Repository navigation
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, localized Git pull error-reporting fix that preserves successful pull behavior and never exposes raw remote output. Targeted integration tests cover the new diagnostics, secret redaction, and unchanged handling of non-fast-forward failures. No code changes detected at You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesPull failure diagnostics
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The pull diagnostics change appears mergeable after normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
A failed Pull only reported "git pull failed". Fetch failures already map authentication, network, missing-repository and reference-lock errors to fixed messages (pingdotgg#12485); pull now uses the same diagnoses, so a dead SSH agent or an unreachable remote is named instead of hidden. Raw Git output still never enters the error, and unrecognised failures keep the generic message.
bc1b535 to
2f2afb0
Compare
|
Force-pushed to drop a merge commit. I'd used the Update branch button, which authored it with a personal email address I don't want public. The branch is now the single fix commit rebased onto current |
What Changed
pullCurrentBranchnow explains a failedgit pull --ff-onlywith the fixed diagnoses #12485 added for fetch. When Git's output matches a known case the error says "Git could not authenticate with the remote…", "Git could not reach the remote…", "Git could not access the remote repository…" or the reference-lock message. Anything else keeps "git pull failed".Raw Git output still never enters the error; only the fixed messages do. To match them on any server,
git pullnow runs withLC_ALL=C, like the fetch path.Why
The triage of #11872 names this as its secondary problem: when a desktop SSH remote loses its forwarded agent, Pull fails with only "git pull failed", and the
Permission denied (publickey)that explains it never reaches the UI. #12485 fixed that for fetches during worktree preparation; the Pull action was left on the generic message.I'm submitting it as a small, focused fix for that defect: one call site, reusing the existing helper, with no change to what Pull does or to any default. It does not fix the agent lifetime bug in #11872 itself.
UI Changes
Clicking Pull in the app, connected over an SSH tunnel to a server on a Linux host that is in the state #11872 describes: it was started inside an ssh session with a forwarded agent, the session closed, and its
SSH_AUTH_SOCKnow points at a socket that no longer exists. Browser zoomed to 200% so the toast is legible.Before (server built from
main):After (server built from this branch):
The same click against a local server on macOS
Real
gitandsshtogit@github.comwith no identity and a dead agent socket.Before:
After:
Verification
To reproduce: in a repository with an upstream branch and at least one commit to pull, set
core.sshCommandtossh -F /dev/null -o BatchMode=yes -o IdentitiesOnly=yes -o IdentityFile=/nonexistent -o IdentityAgent=/tmp/dead.sockwith an SSH remote, then click Pull. That leavessshwith no key and an agent socket that doesn't exist, which is the state #11872 leaves the managed server in.In the app against a local server on macOS: the toast changed from "git pull failed" to the authentication message when I ran the same click on
mainand on this branch (screenshots above).vp test run src/vcs/GitVcsDriverCore.test.tsfromapps/server: 110 passed on currentmain(108 when the Linux and Windows runs below were done;mainhas gained two tests since). Three of those are new, all using real Git:sshprints OpenSSH'sPermission denied (publickey)line and exits 255; the pull is reported as an authentication failure and a marker printed alongside it never appears in the error. Onmainthis fails withexpected 'git pull failed' to include 'could not authenticate'.main.vp run --filter t3 typecheck,vp lintandvp fmt --checkon the two changed files: clean.In the app against a Linux host over SSH (Ubuntu 24.04.3 x86_64, OpenSSH 9.6p1, git 2.43.0, Node 24.21.0), set up the way [Bug]: Desktop SSH remote's managed server inherits a per-session forwarded SSH_AUTH_SOCK that dies with the launching session #11872 describes. Inside an ssh session with a throwaway agent forwarded, I started the server with the line the managed launch uses (
nohup env T3CODE_NO_BROWSER=1 … serve --host 127.0.0.1 --port … < /dev/null &) and let the session close. The running server's/proc/<pid>/environstill hadSSH_AUTH_SOCK=/tmp/ssh-…/agent.N, and that path no longer existed. With the app connected to that server through anssh -N -Ltunnel, clicking Pull on a repository with a GitHub SSH remote gave the before and after screenshots above, once with a server built frommainand once from this branch.On the same host, from a process left in that state:
ssh -vreportedget_agent_identities: ssh_get_authentication_socket: No such file or directory(it had reportedagent returned 1 keyswhile the session was open),git pull --ff-onlyprintedgit@github.com: Permission denied (publickey)., andpullCurrentBranchreturned "git pull failed" withmain's driver and the authentication message with this branch.The same test file on that Linux host: 108 passed.
The same test file on Windows 11 (git 2.53.0.windows.1, Node 24.21.0): the three new tests pass, including the stand-in
sshone. The file as a whole is 106 passed, 1 failed, 1 skipped. The failure is the existing "keeps untracked filenames with pathspec magic in the review" test from fix(server): detect file renames in review diffs #8086, which creates a file named:(exclude)after.tsand getsENOENTbecause Windows doesn't allow:in a filename; the skip is the existing Windows skip for newline paths.pnpm installdidn't finish linking on that machine, so I started the run withnode node_modules\vite-plus\bin\vp test run src/vcs/GitVcsDriverCore.test.ts.Not checked: the desktop app issuing that launch itself. I ran its launch line by hand, so the server ended up in the same state without going through the desktop's SSH environment setup.