fix(server): name the cause of a failed git command - #8645
Conversation
Git states why a command failed on stderr, but stderr is deliberately kept off GitCommandError — it echoes argv and remote URLs, which can carry credentials. Callers were left with "git fetch origin failed" and no way to tell a tag conflict from an auth failure without re-running git by hand. Match stderr at the driver against a fixed set of well-known failures and carry the result as a closed set of tags. The message each tag renders is a static sentence that quotes none of the matched text, so the existing redaction guarantee is unchanged: stderr still never leaves the driver. Fixes pingdotgg#4380
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
| if (options.allowNonZeroExit || result.exitCode === 0) { | ||
| return Effect.succeed(result); | ||
| } | ||
| const reason = classifyGitFailure(result.stderr); |
There was a problem hiding this comment.
🟡 Medium vcs/GitVcsDriverCore.ts:930
classifyGitFailure(result.stderr) labels hook-generated text as a Git failure reason, so a failing pre-push hook that prints authentication failed receives reason: "authentication_failed" even though no credentials were attempted. Restrict classification to Git diagnostic lines or exclude hook output before assigning a remediation reason.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/vcs/GitVcsDriverCore.ts around line 930:
`classifyGitFailure(result.stderr)` labels hook-generated text as a Git failure reason, so a failing `pre-push` hook that prints `authentication failed` receives `reason: "authentication_failed"` even though no credentials were attempted. Restrict classification to Git diagnostic lines or exclude hook output before assigning a remediation reason.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 64e5038. Configure here.
| return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}`; | ||
| const reason = | ||
| this.reason === undefined ? "" : ` ${gitCommandFailureReasonMessage(this.reason)}`; | ||
| return `Git command failed in ${this.operation} (${this.cwd}): ${this.detail}${reason}`; |
There was a problem hiding this comment.
Reason text runs into detail
Low Severity
The message getter appends the reason sentence with only a leading space, so it reads as one run-on clause whenever detail does not already end with a period. createWorktree and fetchRemote use fallback details like git worktree add failed and git fetch origin failed, which produces text such as git worktree add failed That branch is already checked out in another worktree.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 64e5038. Configure here.
ApprovabilityVerdict: Would Approve Macroscope's review found this PR approvable — This is a localized Git error-enrichment fix with an optional, backward-compatible reason field, fixed safe messages, and focused integration tests; successful command behavior remains unchanged. A remaining risk is that hook-generated stderr may occasionally be mistaken for Git diagnostics, while the separate fallback-message formatting issue is minor. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |


What Changed
GitCommandErrorgains an optionalreason: a closed set of tags for well-known gitfailures, recognized from stderr inside the driver. Each tag renders a fixed sentence that
the error's
messagegetter appends after the existing detail.Before and after, for the same failing command (real git, captured from the driver):
Raw stderr still never leaves the driver.
Why
Fixes #4380. Every git precondition failure collapsed into one string, so a tag conflict, an
auth failure and a branch already checked out elsewhere were indistinguishable without
re-running the command by hand.
The issue offers two options. The first — carrying a bounded
stderrExcerpt— is the one Idid not take:
apps/server/src/vcs/GitVcsDriverCore.test.tsalready assertsnotProperty(error, "stderr")and that a secret passed in argv never reacheserror.message, so dropping stderr is deliberate, and shipping an excerpt would meandeleting a security test. This is the issue's second option, the parsed reason, which keeps
that guarantee intact: the tags are a closed literal union, the sentences are static, and
nothing matched from stderr is ever interpolated. The existing redaction test is extended
with
notProperty(error, "reason").Classification happens at the two shared funnels every git call routes through
(
executeGitand the non-zero-exit branch of the raw executor), which coversfetchRemoteandcreateWorktree— the two operations named in the issue.Two limits, stated rather than hidden:
executeGitinherits the process locale, so under anon-English
LANGgit's wording does not match and the error simply keeps today'sbehavior — no reason, no regression. Forcing
LC_ALL=Cfor every git command would fixthat, but it changes the environment of every call in the driver and belongs in its own
change. The new tests pin
LC_ALL: "C"so they do not depend on the runner's locale.GitCommandErrorconstructions hold stderr in hand and stay unclassified.Adding them is one line each and deliberately left out to keep this to one concern.
UI Changes
None. Server-side error metadata; no rendered change. The improved text surfaces wherever
an existing git error message is already shown.
Verification
The three new tests drive real git through the real driver: a branch already checked out in
another worktree, a command outside a repository, and a tag collision that must stay
unclassified. All three fail without the source change.
Checklist
to
GitCommandErrorfields exists in any client)Model: Claude Opus 5 (1M context). Harness: Claude Code.
Note
Low Risk
Additive optional contract field and localized error enrichment; stderr redaction behavior is preserved and covered by tests.
Overview
GitCommandErrornow carries an optionalreasontag (closed union in contracts) for common git failures. The VCS driver classifies non-zero exits by matching stderr against ordered regex patterns insideGitVcsDriverCoreonly—raw stderr still never leaves the driver.When a reason is set,
GitCommandError.messageappends a fixed user-facing sentence fromgitCommandFailureReasonMessage; callers can also branch onreason(e.g. worktree branch conflict, not a repository, auth, remote unreachable). Unmatched cases (including duplicate tag creation) stay unclassified so tag/ref collisions are not mislabeled as path conflicts.Integration tests cover worktree add conflicts, commands outside a repo, tag collisions, and extend the redaction test to assert
reasonis absent when classification does not apply.Reviewed by Cursor Bugbot for commit 64e5038. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add
reasonfield toGitCommandErrorto classify common git failuresGitCommandFailureReasonin git.ts: a closed set of reasons (e.g.not_a_repository,branch_checked_out_in_worktree,remote_unreachable) with a user-facing message map.classifyGitFailurein GitVcsDriverCore.ts that scans git stderr against orderedGIT_FAILURE_REASON_PATTERNSand attaches areasontoGitCommandErroron non-zero exits.GitCommandError.messagenow appends an explanatory sentence whenreasonis set; unmatched errors remain unchanged.GitCommandError.messagegains an extra sentence for classified failures, and instances now carry an optionalreasonfield that was absent before.📊 Macroscope summarized 64e5038. 2 files reviewed, 1 issue evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues