| author | |
| committer | |
| log | dd5c7bd09e83bbeb7c3f63aa787e6ffdd3cc6221 |
| tree | 3c6fe1a84fe127484e55907fc8bfce41c49c0086 |
| parent | df8df0d53b39bfd6a5d8d69fabade19bd668e0a5 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>2 files changed, 79 insertions(+), 23 deletions(-)
apps/server/src/vcs/GitVcsDriverCore.test.ts+45-1| ... | @@ -998,11 +998,25 @@ it.effect.each([ | ... | @@ -998,11 +998,25 @@ it.effect.each([ |
| 998 | name: "reference lock", | 998 | name: "reference lock", |
| 999 | stderr: "error: cannot lock ref 'refs/remotes/origin/main': is at abc but expected def", | 999 | stderr: "error: cannot lock ref 'refs/remotes/origin/main': is at abc but expected def", |
| 1000 | expected: "could not update a local reference", | 1000 | expected: "could not update a local reference", |
| 1001 | attempts: 2, | ||
| 1001 | }, | 1002 | }, |
| 1002 | { | 1003 | { |
| 1003 | name: "lock file", | 1004 | name: "lock file", |
| 1004 | stderr: "fatal: Unable to create '/repo/.git/FETCH_HEAD.lock': File exists.", | 1005 | stderr: "fatal: Unable to create '/repo/.git/FETCH_HEAD.lock': File exists.", |
| 1005 | expected: "could not update a local reference", | 1006 | expected: "could not update a local reference", |
| 1007 | attempts: 2, | ||
| 1008 | }, | ||
| 1009 | { | ||
| 1010 | name: "reference race on Git 2.53", | ||
| 1011 | stderr: "error: fetching ref refs/remotes/origin/main failed: incorrect old value provided", | ||
| 1012 | expected: "could not update a local reference", | ||
| 1013 | attempts: 2, | ||
| 1014 | }, | ||
| 1015 | { | ||
| 1016 | name: "held reference lock on Git 2.53", | ||
| 1017 | stderr: "error: fetching ref refs/remotes/origin/main failed: reference already exists", | ||
| 1018 | expected: "could not update a local reference", | ||
| 1019 | attempts: 2, | ||
| 1006 | }, | 1020 | }, |
| 1007 | { | 1021 | { |
| 1008 | name: "unrelated remote chatter", | 1022 | name: "unrelated remote chatter", |
| ... | @@ -1063,7 +1077,37 @@ it.effect.each([ | ... | @@ -1063,7 +1077,37 @@ it.effect.each([ |
| 1063 | assert.notInclude(yield* encodeGitCommandError(error), secret); | 1077 | assert.notInclude(yield* encodeGitCommandError(error), secret); |
| 1064 | assert.notProperty(error, "stderr"); | 1078 | assert.notProperty(error, "stderr"); |
| 1065 | assert.notProperty(error, "args"); | 1079 | assert.notProperty(error, "args"); |
| 1066 | assert.equal(yield* Ref.get(attempts), 1); | 1080 | assert.equal(yield* Ref.get(attempts), "attempts" in scenario ? scenario.attempts : 1); |
| 1081 | }).pipe(Effect.provide(layerServerConfig.pipe(Layer.provideMerge(NodeServices.layer)))), | ||
| 1082 | ); | ||
| 1083 | |||
| 1084 | it.effect("retries a fetch that lost a ref update to another Git process", () => | ||
| 1085 | Effect.gen(function* () { | ||
| 1086 | const attempts = yield* Ref.make(0); | ||
| 1087 | const spawner = ChildProcessSpawner.make((command) => | ||
| 1088 | Effect.gen(function* () { | ||
| 1089 | if (!ChildProcess.isStandardCommand(command)) | ||
| 1090 | return yield* Effect.die("expected Git command"); | ||
| 1091 | if (command.args[0] !== "fetch") return makeNonRepositoryHandle(); | ||
| 1092 | const attempt = yield* Ref.updateAndGet(attempts, (count) => count + 1); | ||
| 1093 | if (attempt > 1) return makeSuccessfulHandle(""); | ||
| 1094 | return ChildProcessSpawner.makeHandle({ | ||
| 1095 | ...makeNonRepositoryHandle(), | ||
| 1096 | exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(1)), | ||
| 1097 | stderr: Stream.encodeText( | ||
| 1098 | Stream.make( | ||
| 1099 | "error: fetching ref refs/remotes/origin/main failed: incorrect old value provided", | ||
| 1100 | ), | ||
| 1101 | ), | ||
| 1102 | }); | ||
| 1103 | }), | ||
| 1104 | ); | ||
| 1105 | const driver = yield* makeGitVcsDriverCore().pipe( | ||
| 1106 | Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), | ||
| 1107 | ); | ||
| 1108 | const cwd = yield* makeTmpDir(); | ||
| 1109 | yield* driver.fetchRemote({ cwd, remoteName: "origin", refName: "main" }); | ||
| 1110 | assert.equal(yield* Ref.get(attempts), 2); | ||
| 1067 | }).pipe(Effect.provide(layerServerConfig.pipe(Layer.provideMerge(NodeServices.layer)))), | 1111 | }).pipe(Effect.provide(layerServerConfig.pipe(Layer.provideMerge(NodeServices.layer)))), |
| 1068 | ); | 1112 | ); |
| 1069 | 1113 |
apps/server/src/vcs/GitVcsDriverCore.ts+34-22| ... | @@ -579,18 +579,24 @@ function fetchFailureDetail(stderr: string): string | undefined { | ... | @@ -579,18 +579,24 @@ function fetchFailureDetail(stderr: string): string | undefined { |
| 579 | ) { | 579 | ) { |
| 580 | return "Git could not access the remote repository. Check the remote URL and repository permissions on the server."; | 580 | return "Git could not access the remote repository. Check the remote URL and repository permissions on the server."; |
| 581 | } | 581 | } |
| 582 | if ( | 582 | if (isRefLockFailure(stderr)) { |
| 583 | lines.some((line) => | ||
| 584 | /^(?:(?:error|fatal): cannot lock ref\b|fatal: Unable to create ['"].+\.lock['"]:)/i.test( | ||
| 585 | line, | ||
| 586 | ), | ||
| 587 | ) | ||
| 588 | ) { | ||
| 589 | 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."; | 583 | 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."; |
| 590 | } | 584 | } |
| 591 | return undefined; | 585 | return undefined; |
| 592 | } | 586 | } |
| 593 | 587 | ||
| 588 | // Matches a fetch that lost a ref update to another Git process. Git 2.53+ | ||
| 589 | // reports it as "fetching ref <ref> failed: ..."; older Git says "cannot lock ref". | ||
| 590 | function isRefLockFailure(stderr: string): boolean { | ||
| 591 | return stderr | ||
| 592 | .split(/\r?\n/) | ||
| 593 | .some((line) => | ||
| 594 | /^(?:(?: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( | ||
| 595 | line.trim(), | ||
| 596 | ), | ||
| 597 | ); | ||
| 598 | } | ||
| 599 | |||
| 594 | interface Trace2Monitor { | 600 | interface Trace2Monitor { |
| 595 | readonly env: NodeJS.ProcessEnv; | 601 | readonly env: NodeJS.ProcessEnv; |
| 596 | readonly flush: Effect.Effect<void, never>; | 602 | readonly flush: Effect.Effect<void, never>; |
| ... | @@ -3713,15 +3719,26 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* | ... | @@ -3713,15 +3719,26 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* |
| 3713 | env: STATUS_UPSTREAM_REFRESH_ENV, | 3719 | env: STATUS_UPSTREAM_REFRESH_ENV, |
| 3714 | fallbackErrorDetail: `git fetch ${input.remoteName} failed`, | 3720 | fallbackErrorDetail: `git fetch ${input.remoteName} failed`, |
| 3715 | }; | 3721 | }; |
| 3716 | const fetchAll = executeGitWithStableDiagnostics( | 3722 | // Another fetch in the same repository, such as the background status |
| 3717 | "GitVcsDriver.fetchRemote", | 3723 | // poll, can win the race for a remote-tracking ref. The loser fails, but |
| 3718 | input.cwd, | 3724 | // the ref is free again by the time a rerun needs it, so retry once. |
| 3719 | args, | 3725 | const runFetch = (fetchArgs: readonly string[]) => { |
| 3720 | { | 3726 | const run = executeGitWithStableDiagnostics( |
| 3721 | ...options, | 3727 | "GitVcsDriver.fetchRemote", |
| 3722 | allowNonZeroExit: true, | 3728 | input.cwd, |
| 3723 | }, | 3729 | fetchArgs, |
| 3724 | ).pipe( | 3730 | { |
| 3731 | ...options, | ||
| 3732 | allowNonZeroExit: true, | ||
| 3733 | }, | ||
| 3734 | ); | ||
| 3735 | return run.pipe( | ||
| 3736 | Effect.flatMap((result) => | ||
| 3737 | result.exitCode !== 0 && isRefLockFailure(result.stderr) ? run : Effect.succeed(result), | ||
| 3738 | ), | ||
| 3739 | ); | ||
| 3740 | }; | ||
| 3741 | const fetchAll = runFetch(args).pipe( | ||
| 3725 | Effect.flatMap((result) => | 3742 | Effect.flatMap((result) => |
| 3726 | result.exitCode === 0 | 3743 | result.exitCode === 0 |
| 3727 | ? Effect.void | 3744 | ? Effect.void |
| ... | @@ -3750,12 +3767,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* | ... | @@ -3750,12 +3767,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* |
| 3750 | ...args, | 3767 | ...args, |
| 3751 | `+refs/heads/${branch}:refs/remotes/${input.remoteName}/${branch}`, | 3768 | `+refs/heads/${branch}:refs/remotes/${input.remoteName}/${branch}`, |
| 3752 | ]; | 3769 | ]; |
| 3753 | const result = yield* executeGitWithStableDiagnostics( | 3770 | const result = yield* runFetch(scopedArgs); |
| 3754 | "GitVcsDriver.fetchRemote", | ||
| 3755 | input.cwd, | ||
| 3756 | scopedArgs, | ||
| 3757 | { ...options, allowNonZeroExit: true }, | ||
| 3758 | ); | ||
| 3759 | if (result.exitCode === 0) return; | 3771 | if (result.exitCode === 0) return; |
| 3760 | if ( | 3772 | if ( |
| 3761 | result.stderr | 3773 | result.stderr |