From dd5c7bd09e83bbeb7c3f63aa787e6ffdd3cc6221 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Wed, 14 Oct 2026 05:15:50 -0700 Subject: [PATCH] fix(server): new worktrees no longer fail when a background fetch runs at the same time (#19411) Co-authored-by: Claude Opus 5.5 --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 46 +++++++++++++++- apps/server/src/vcs/GitVcsDriverCore.ts | 56 ++++++++++++-------- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 45b1348a1a841b6062864b25045a49bbceabeacd..d98da188562f94f4cadc5f3673d28269e986887c 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -998,11 +998,25 @@ it.effect.each([ name: "reference lock", stderr: "error: cannot lock ref 'refs/remotes/origin/main': is at abc but expected def", expected: "could not update a local reference", + attempts: 2, }, { name: "lock file", stderr: "fatal: Unable to create '/repo/.git/FETCH_HEAD.lock': File exists.", expected: "could not update a local reference", + attempts: 2, + }, + { + name: "reference race on Git 2.53", + stderr: "error: fetching ref refs/remotes/origin/main failed: incorrect old value provided", + expected: "could not update a local reference", + attempts: 2, + }, + { + name: "held reference lock on Git 2.53", + stderr: "error: fetching ref refs/remotes/origin/main failed: reference already exists", + expected: "could not update a local reference", + attempts: 2, }, { name: "unrelated remote chatter", @@ -1063,7 +1077,37 @@ it.effect.each([ assert.notInclude(yield* encodeGitCommandError(error), secret); assert.notProperty(error, "stderr"); assert.notProperty(error, "args"); - assert.equal(yield* Ref.get(attempts), 1); + assert.equal(yield* Ref.get(attempts), "attempts" in scenario ? scenario.attempts : 1); + }).pipe(Effect.provide(layerServerConfig.pipe(Layer.provideMerge(NodeServices.layer)))), +); + +it.effect("retries a fetch that lost a ref update to another Git process", () => + Effect.gen(function* () { + const attempts = yield* Ref.make(0); + const spawner = ChildProcessSpawner.make((command) => + Effect.gen(function* () { + if (!ChildProcess.isStandardCommand(command)) + return yield* Effect.die("expected Git command"); + if (command.args[0] !== "fetch") return makeNonRepositoryHandle(); + const attempt = yield* Ref.updateAndGet(attempts, (count) => count + 1); + if (attempt > 1) return makeSuccessfulHandle(""); + return ChildProcessSpawner.makeHandle({ + ...makeNonRepositoryHandle(), + exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(1)), + stderr: Stream.encodeText( + Stream.make( + "error: fetching ref refs/remotes/origin/main failed: incorrect old value provided", + ), + ), + }); + }), + ); + const driver = yield* makeGitVcsDriverCore().pipe( + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + ); + const cwd = yield* makeTmpDir(); + yield* driver.fetchRemote({ cwd, remoteName: "origin", refName: "main" }); + assert.equal(yield* Ref.get(attempts), 2); }).pipe(Effect.provide(layerServerConfig.pipe(Layer.provideMerge(NodeServices.layer)))), ); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index ed68e44632dcfeef5fabc488a6bbc004ad5b5c23..439c6a55ac783937abc836872e80fd080e68b166 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -579,18 +579,24 @@ function fetchFailureDetail(stderr: string): string | undefined { ) { return "Git could not access the remote repository. Check the remote URL and repository permissions on the server."; } - if ( - lines.some((line) => - /^(?:(?:error|fatal): cannot lock ref\b|fatal: Unable to create ['"].+\.lock['"]:)/i.test( - line, - ), - ) - ) { + if (isRefLockFailure(stderr)) { return "Git could not update a local reference. Another Git operation or a stale lock may be blocking the fetch; check the repository on the server, then retry."; } return undefined; } +// Matches a fetch that lost a ref update to another Git process. Git 2.53+ +// reports it as "fetching ref failed: ..."; older Git says "cannot lock ref". +function isRefLockFailure(stderr: string): boolean { + return stderr + .split(/\r?\n/) + .some((line) => + /^(?:(?:error|fatal): cannot lock ref\b|fatal: Unable to create ['"].+\.lock['"]:|error: fetching ref \S+ failed: (?:incorrect old value provided|reference already exists)$)/i.test( + line.trim(), + ), + ); +} + interface Trace2Monitor { readonly env: NodeJS.ProcessEnv; readonly flush: Effect.Effect; @@ -3713,15 +3719,26 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* env: STATUS_UPSTREAM_REFRESH_ENV, fallbackErrorDetail: `git fetch ${input.remoteName} failed`, }; - const fetchAll = executeGitWithStableDiagnostics( - "GitVcsDriver.fetchRemote", - input.cwd, - args, - { - ...options, - allowNonZeroExit: true, - }, - ).pipe( + // Another fetch in the same repository, such as the background status + // poll, can win the race for a remote-tracking ref. The loser fails, but + // the ref is free again by the time a rerun needs it, so retry once. + const runFetch = (fetchArgs: readonly string[]) => { + const run = executeGitWithStableDiagnostics( + "GitVcsDriver.fetchRemote", + input.cwd, + fetchArgs, + { + ...options, + allowNonZeroExit: true, + }, + ); + return run.pipe( + Effect.flatMap((result) => + result.exitCode !== 0 && isRefLockFailure(result.stderr) ? run : Effect.succeed(result), + ), + ); + }; + const fetchAll = runFetch(args).pipe( Effect.flatMap((result) => result.exitCode === 0 ? Effect.void @@ -3750,12 +3767,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ...args, `+refs/heads/${branch}:refs/remotes/${input.remoteName}/${branch}`, ]; - const result = yield* executeGitWithStableDiagnostics( - "GitVcsDriver.fetchRemote", - input.cwd, - scopedArgs, - { ...options, allowNonZeroExit: true }, - ); + const result = yield* runFetch(scopedArgs); if (result.exitCode === 0) return; if ( result.stderr -- 2.54.0