From 65ade4580cd37dc0e686492e0b64ab7bef6164de Mon Sep 17 00:00:00 2001 From: Brad Harris Date: Fri, 17 Jul 2026 20:21:01 -0600 Subject: [PATCH] Fix .git/config lock race when creating worktrees concurrently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git worktree add -b origin/main` auto-writes branch tracking config (branch..{remote,merge}) into .git/config, which takes the repo-wide config lock. When two worktree agents are created against the same repo at the same instant, the second `worktree add` loses the lock race and dies: error: could not lock config file .git/config: File exists error: unable to write upstream branch configuration git treats that as fatal (exit 255) and fails the whole command, so the agent-create API returns a 500. Surfaced as an intermittent E2E failure in worktree.spec.ts whose setup creates several worktree agents in parallel. Fix: pass --no-track to `git worktree add` so it no longer writes tracking config — that removes the only .git/config write from the non-idempotent command, so it can't lose the race. Upstream is still set explicitly right after via `git branch --set-upstream-to` (unchanged), which already tolerates exit 1/128; git returns exit 1 when that write hits a contended lock, and a missing upstream degrades correctly to origin/ in worktree-status and base-ref, so creation succeeds either way. Adds regression tests: `worktree add` carries --no-track, and creation succeeds when the upstream write is blocked by a contended config lock (exit 1 + lock stderr), matching real git behavior. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/server/src/shared/git/worktree.ts | 17 +++- apps/server/test/git-worktree.test.ts | 116 ++++++++++++++++++++++++- 2 files changed, 130 insertions(+), 3 deletions(-) diff --git a/apps/server/src/shared/git/worktree.ts b/apps/server/src/shared/git/worktree.ts index 80c20f5c..1ab8b7d2 100644 --- a/apps/server/src/shared/git/worktree.ts +++ b/apps/server/src/shared/git/worktree.ts @@ -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..{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/, which is exactly what we'd have tracked. await commandRunner( "git", ["-C", worktreePath, "branch", "--set-upstream-to", baseRef, branchName], diff --git a/apps/server/test/git-worktree.test.ts b/apps/server/test/git-worktree.test.ts index bf2b4a25..1c977a87 100644 --- a/apps/server/test/git-worktree.test.ts +++ b/apps/server/test/git-worktree.test.ts @@ -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: @@ -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 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: "git@github.com: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/, 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: "git@github.com: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 @@ -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: