Skip to content
Merged
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
17 changes: 16 additions & 1 deletion apps/server/src/shared/git/worktree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,33 @@ export async function createGitWorktree(
if (createNewBranch) {
await ensureBranchDoesNotExist(repoRoot, branchName, commandRunner);

// Pass --no-track so `git worktree add` does NOT write branch tracking
// config. When the start-point is a remote-tracking ref (origin/main) git
// would otherwise auto-write `branch.<name>.{remote,merge}` into
// .git/config, and that write takes the repo-wide .git/config lock. Two
// worktree creations against the same repo at the same instant then collide
// ("error: could not lock config file .git/config: File exists"), which git
// treats as fatal and fails the whole `worktree add` (exit 255). We set the
// exact same upstream explicitly below, so the auto-write is redundant and
// safe to drop.
await commandRunner("git", [
"-C",
repoRoot,
"worktree",
"add",
"--no-track",
"-b",
branchName,
worktreePath,
baseRef,
]);

// Set upstream tracking so archival checks know which branch to compare against
// Set upstream tracking so archival checks know which branch to compare
// against. This is now the only .git/config writer in the creation path.
// Tolerate exit 1/128: git returns exit 1 (not a hard failure) when the
// config lock is momentarily held by a concurrent worktree creation. A
// missing upstream is harmless here — worktree-status and base-ref both
// fall back to origin/<baseBranch>, which is exactly what we'd have tracked.
await commandRunner(
"git",
["-C", worktreePath, "branch", "--set-upstream-to", baseRef, branchName],
Expand Down
116 changes: 114 additions & 2 deletions apps/server/test/git-worktree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe("git worktree services", () => {
case `-C ${repoRoot} show-ref --verify --quiet refs/heads/feature-auth-flow`:
case `-C ${repoRoot} show-ref --verify --quiet refs/remotes/origin/feature-auth-flow`:
return { exitCode: 1, stdout: "", stderr: "" };
case `-C ${repoRoot} worktree add -b feature-auth-flow ${expectedWorktreePath} origin/main`:
case `-C ${repoRoot} worktree add --no-track -b feature-auth-flow ${expectedWorktreePath} origin/main`:
case `-C ${expectedWorktreePath} branch --set-upstream-to origin/main feature-auth-flow`:
return { exitCode: 0, stdout: "", stderr: "" };
default:
Expand All @@ -73,6 +73,118 @@ describe("git worktree services", () => {
});
});

it("creates the worktree with --no-track and sets upstream explicitly", async () => {
// Regression: `git worktree add -b <branch> <path> origin/main` used to let
// git auto-write branch tracking config, which takes the .git/config lock
// and fails ("could not lock config file") when two worktrees are created
// against the same repo concurrently. --no-track keeps that racy write out
// of `worktree add`; upstream is set by the explicit follow-up command.
const repoRoot = path.join(tempRoot, "repo");
const expectedWorktreePath = path.join(tempRoot, "repo-feature-auth-flow");
const calls: string[] = [];

vi.mocked(runCommand).mockImplementation(async (_command, args) => {
const key = args.join(" ");
calls.push(key);
switch (key) {
case `-C ${repoRoot} rev-parse --show-toplevel`:
return { exitCode: 0, stdout: repoRoot, stderr: "" };
case `-C ${repoRoot} remote get-url origin`:
return {
exitCode: 0,
stdout: "[email protected]:test/repo.git",
stderr: "",
};
case `-C ${repoRoot} fetch origin main --quiet`:
return { exitCode: 0, stdout: "", stderr: "" };
case `-C ${repoRoot} rev-parse --verify origin/main`:
return { exitCode: 0, stdout: "abc123", stderr: "" };
case `-C ${repoRoot} show-ref --verify --quiet refs/heads/feature-auth-flow`:
case `-C ${repoRoot} show-ref --verify --quiet refs/remotes/origin/feature-auth-flow`:
return { exitCode: 1, stdout: "", stderr: "" };
case `-C ${repoRoot} worktree add --no-track -b feature-auth-flow ${expectedWorktreePath} origin/main`:
case `-C ${expectedWorktreePath} branch --set-upstream-to origin/main feature-auth-flow`:
return { exitCode: 0, stdout: "", stderr: "" };
default:
throw new Error(`Unexpected command: ${key}`);
}
});

await createGitWorktree({
cwd: repoRoot,
name: "Feature Auth Flow",
createNewBranch: true,
});

const addCall = calls.find((c) => c.includes("worktree add"));
expect(addCall).toContain("worktree add --no-track -b feature-auth-flow");
expect(
calls.some((c) =>
c.includes("branch --set-upstream-to origin/main feature-auth-flow")
)
).toBe(true);
});

it("succeeds when a contended .git/config lock blocks the upstream write", async () => {
// Regression for the concurrency bug: `git worktree add --no-track` no
// longer writes config, so it can't lose the lock race. The follow-up
// `git branch --set-upstream-to` still writes config, and real git exits 1
// ("could not lock config file .git/config: File exists") when a concurrent
// creation holds the lock. Exit 1 is tolerated, so creation must still
// succeed — a missing upstream degrades to origin/<base>, which is correct.
const repoRoot = path.join(tempRoot, "repo");
const expectedWorktreePath = path.join(tempRoot, "repo-feature-auth-flow");
let upstreamCalls = 0;

vi.mocked(runCommand).mockImplementation(
async (_command, args, options) => {
const key = args.join(" ");
switch (key) {
case `-C ${repoRoot} rev-parse --show-toplevel`:
return { exitCode: 0, stdout: repoRoot, stderr: "" };
case `-C ${repoRoot} remote get-url origin`:
return {
exitCode: 0,
stdout: "[email protected]:test/repo.git",
stderr: "",
};
case `-C ${repoRoot} fetch origin main --quiet`:
return { exitCode: 0, stdout: "", stderr: "" };
case `-C ${repoRoot} rev-parse --verify origin/main`:
return { exitCode: 0, stdout: "abc123", stderr: "" };
case `-C ${repoRoot} show-ref --verify --quiet refs/heads/feature-auth-flow`:
case `-C ${repoRoot} show-ref --verify --quiet refs/remotes/origin/feature-auth-flow`:
return { exitCode: 1, stdout: "", stderr: "" };
case `-C ${repoRoot} worktree add --no-track -b feature-auth-flow ${expectedWorktreePath} origin/main`:
return { exitCode: 0, stdout: "", stderr: "" };
case `-C ${expectedWorktreePath} branch --set-upstream-to origin/main feature-auth-flow`:
upstreamCalls += 1;
// The caller must tolerate exit 1 (must not throw); assert it does.
expect(options?.allowedExitCodes).toContain(1);
return {
exitCode: 1,
stdout: "",
stderr:
"error: could not lock config file .git/config: File exists",
};
default:
throw new Error(`Unexpected command: ${key}`);
}
}
);

const result = await createGitWorktree({
cwd: repoRoot,
name: "Feature Auth Flow",
createNewBranch: true,
});

// Attempted exactly once (no retry loop) and creation still succeeded.
expect(upstreamCalls).toBe(1);
expect(result.branchName).toBe("feature-auth-flow");
expect(result.baseRef).toBe("origin/main");
});

it("derives a hashed worktree-path slug when createNewBranch is false (review #1161)", async () => {
// `feature/x` and `feature-x` slug-collapse to `feature-x` with the
// legacy mapping. The hash discriminator added for createNewBranch=false
Expand Down Expand Up @@ -243,7 +355,7 @@ describe("git worktree services", () => {
case `-C ${repoRoot} show-ref --verify --quiet refs/heads/feature-local`:
case `-C ${repoRoot} show-ref --verify --quiet refs/remotes/origin/feature-local`:
return { exitCode: 1, stdout: "", stderr: "" };
case `-C ${repoRoot} worktree add -b feature-local ${expectedWorktreePath} main`:
case `-C ${repoRoot} worktree add --no-track -b feature-local ${expectedWorktreePath} main`:
case `-C ${expectedWorktreePath} branch --set-upstream-to main feature-local`:
return { exitCode: 0, stdout: "", stderr: "" };
default:
Expand Down
Loading