Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions apps/server/src/vcs/GitVcsDriverCore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1345,6 +1345,77 @@ it.layer(TestLayer)("GitVcsDriver core integration", (it) => {
assert.equal(result.branch, current);
}),
);

it.effect("explains checkout failures caused by uncommitted changes", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
const { initialBranch } = yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;

yield* git(cwd, ["checkout", "-b", "feature/target"]);
yield* writeTextFile(cwd, "README.md", "# target\n");
yield* git(cwd, ["add", "."]);
yield* git(cwd, ["commit", "-m", "target commit"]);
yield* git(cwd, ["checkout", initialBranch]);
yield* writeTextFile(cwd, "README.md", "# dirty\n");

const error = yield* driver.switchRef({ cwd, refName: "feature/target" }).pipe(Effect.flip);

assert.deepInclude(error, {
_tag: "GitCommandError",
operation: "GitVcsDriver.switchRef.checkout",
cwd,
});
assert.include(error.detail, "uncommitted changes would be overwritten");
assert.include(error.detail, "Commit, stash, or discard");
assert.notProperty(error, "stderr");
}),
);

it.effect("explains checkout failures when the branch is checked out in another worktree", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const worktreesRoot = yield* makeTmpDir("git-vcs-driver-worktrees-");
const fileSystem = yield* FileSystem.FileSystem;
const pathService = yield* Path.Path;
const worktreePath = pathService.join(worktreesRoot, "linked-worktree");
yield* fileSystem.makeDirectory(worktreesRoot, { recursive: true });
const driver = yield* GitVcsDriver.GitVcsDriver;

yield* git(cwd, ["worktree", "add", "-b", "feature/linked", worktreePath]);

const error = yield* driver.switchRef({ cwd, refName: "feature/linked" }).pipe(Effect.flip);

assert.deepInclude(error, {
_tag: "GitCommandError",
operation: "GitVcsDriver.switchRef.checkout",
cwd,
});
assert.include(error.detail, "already checked out in another worktree");
assert.notProperty(error, "stderr");
}),
);

it.effect("explains checkout failures for unknown refs", () =>
Effect.gen(function* () {
const cwd = yield* makeTmpDir();
yield* initRepoWithCommit(cwd);
const driver = yield* GitVcsDriver.GitVcsDriver;

const error = yield* driver
.switchRef({ cwd, refName: "does/not-exist-xyz" })
.pipe(Effect.flip);

assert.deepInclude(error, {
_tag: "GitCommandError",
operation: "GitVcsDriver.switchRef.checkout",
cwd,
});
assert.include(error.detail, "not found locally or on remotes");
assert.notProperty(error, "stderr");
}),
);
});

describe("worktree operations", () => {
Expand Down
52 changes: 48 additions & 4 deletions apps/server/src/vcs/GitVcsDriverCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,27 @@ function isMissingWorktreeStderr(stderr: string): boolean {
);
}

// Classifies `git checkout` stderr into a closed set of static sentences.
// Raw stderr never leaves the driver (it can carry remote URLs with embedded
// credentials), so every branch returns a fixed string that names the cause
// and the fix instead of echoing git's output.
function classifySwitchRefCheckoutError(stderr: string): string {
const normalized = stderr.toLowerCase();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium vcs/GitVcsDriverCore.ts:451

switchRef returns the generic git checkout failed detail for translated worktree, overwrite, and missing-path diagnostics, so users lose the specific remediation this classifier is intended to provide. classifySwitchRefCheckoutError only matches English substrings, while switchRef runs checkout through executeGit without forcing LC_ALL: "C"; use executeGitWithStableDiagnostics or otherwise set that locale before parsing stderr.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/vcs/GitVcsDriverCore.ts around line 451:

`switchRef` returns the generic `git checkout failed` detail for translated worktree, overwrite, and missing-path diagnostics, so users lose the specific remediation this classifier is intended to provide. `classifySwitchRefCheckoutError` only matches English substrings, while `switchRef` runs `checkout` through `executeGit` without forcing `LC_ALL: "C"`; use `executeGitWithStableDiagnostics` or otherwise set that locale before parsing stderr.

if (normalized.includes("used by worktree")) {
return "git checkout failed because the branch is already checked out in another worktree. Switch in that worktree or check out a different branch.";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worktree matcher misses older Git

Medium Severity

classifySwitchRefCheckoutError only matches used by worktree, the Git 2.43+ wording. Older Git still emits is already checked out at, so worktree conflicts stay as the generic git checkout failed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 71f3b27. Configure here.

}
if (
normalized.includes("would be overwritten by checkout") ||
normalized.includes("stash them before you switch")
) {
return "git checkout failed because uncommitted changes would be overwritten. Commit, stash, or discard those changes, then try again.";
}
if (normalized.includes("did not match any file") || normalized.includes("pathspec")) {
return "git checkout failed because the ref was not found locally or on remotes. Fetch the latest refs and check the branch name, then try again.";
}
return "git checkout failed";
}

interface Trace2Monitor {
readonly env: NodeJS.ProcessEnv;
readonly flush: Effect.Effect<void, never>;
Expand Down Expand Up @@ -3207,10 +3228,33 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
? ["checkout", localTrackingBranch]
: ["checkout", input.refName];

yield* executeGit("GitVcsDriver.switchRef.checkout", input.cwd, checkoutArgs, {
timeoutMs: 10_000,
fallbackErrorDetail: "git checkout failed",
});
const checkoutResult = yield* executeGit(
"GitVcsDriver.switchRef.checkout",
input.cwd,
checkoutArgs,
{
timeoutMs: 10_000,
allowNonZeroExit: true,
},
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Classifier skips stable Git locale

Medium Severity

switchRef classifies English git checkout stderr without LC_ALL=C, so a localized Git still surfaces the generic git checkout failed message. Nearby removeWorktree already uses executeGitWithStableDiagnostics for the same style of matching.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 71f3b27. Configure here.

if (checkoutResult.exitCode !== 0) {
// Raw stderr stays out of the wire error (it can carry secrets); the
// classified detail names the cause and the fix instead.
yield* Effect.logWarning(
`GitVcsDriver.switchRef: git checkout exited with code ${checkoutResult.exitCode} (stderr length ${checkoutResult.stderr.length}).`,
);
return yield* new GitCommandError({
...gitCommandContext({
operation: "GitVcsDriver.switchRef.checkout",
cwd: input.cwd,
args: checkoutArgs,
}),
detail: classifySwitchRefCheckoutError(checkoutResult.stderr),
...(checkoutResult.exitCode === null ? {} : { exitCode: checkoutResult.exitCode }),
stdoutLength: checkoutResult.stdout.length,
stderrLength: checkoutResult.stderr.length,
});
}

const refName = yield* runGitStdout("GitVcsDriver.switchRef.currentBranch", input.cwd, [
"branch",
Expand Down
Loading