diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index a614ebdfb5687..2604d59c3d1de 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -67,6 +67,7 @@ export class GitWorkflowService extends Context.Service< ) => Effect.Effect; readonly invalidateLocalStatus: (cwd: string) => Effect.Effect; readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect; + readonly readRefSnapshot: (cwd: string) => Effect.Effect; readonly invalidateStatus: (cwd: string) => Effect.Effect; /** The checkout's own HEAD file, inside a linked worktree's admin directory when there is one. */ readonly headPath: (cwd: string) => Effect.Effect; @@ -390,6 +391,7 @@ export const make = Effect.gen(function* () { }), invalidateLocalStatus: gitManager.invalidateLocalStatus, invalidateRemoteStatus: gitManager.invalidateRemoteStatus, + readRefSnapshot: git.readRefSnapshot, invalidateStatus: gitManager.invalidateStatus, headPath: Effect.fn("GitWorkflowService.headPath")(function (cwd) { return git diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index 9e0c2b50f2743..ba77fb2a099c3 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -337,6 +337,8 @@ export class GitVcsDriver extends Context.Service< cwd: string, options?: GitRemoteStatusOptions, ) => Effect.Effect; + /** Refs, HEAD and shallow boundaries. Null when the snapshot cannot be read. */ + readonly readRefSnapshot: (cwd: string) => Effect.Effect; readonly prepareCommitContext: ( cwd: string, filePaths?: readonly string[], diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 4f5f6b059c711..b5feb7db70b91 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -5040,6 +5040,100 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("a ref snapshot detects branch identity, ref and ancestry changes", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-vcs-driver-remote-"); + const updater = yield* makeTmpDir("git-vcs-driver-updater-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "-u", "origin", initialBranch]); + yield* git(updater, ["clone", remote, "."]); + yield* git(updater, ["config", "user.email", "test@test.com"]); + yield* git(updater, ["config", "user.name", "Test"]); + const driver = yield* GitVcsDriver.GitVcsDriver; + const snapshot = driver.readRefSnapshot(cwd); + + // Newer Git can create origin/HEAD on the first fetch. + yield* git(cwd, ["fetch", "origin"]); + const initial = yield* snapshot; + assert.isNotNull(initial); + yield* git(cwd, ["fetch", "origin"]); + assert.equal(yield* snapshot, initial); + + yield* git(cwd, ["branch", "same-commit"]); + const beforeCheckout = yield* snapshot; + const refsBeforeCheckout = yield* git(cwd, ["show-ref", "--head"]); + yield* git(cwd, ["checkout", "same-commit"]); + assert.equal(yield* git(cwd, ["show-ref", "--head"]), refsBeforeCheckout); + const afterCheckout = yield* snapshot; + assert.isNotNull(afterCheckout); + assert.notEqual(afterCheckout, beforeCheckout); + yield* git(cwd, ["checkout", "--detach"]); + assert.equal(yield* git(cwd, ["show-ref", "--head"]), refsBeforeCheckout); + const detached = yield* snapshot; + assert.isNotNull(detached); + assert.notEqual(detached, beforeCheckout); + assert.notEqual(detached, afterCheckout); + assert.equal(yield* snapshot, detached); + yield* git(cwd, ["checkout", initialBranch]); + assert.equal(yield* snapshot, beforeCheckout); + yield* git(cwd, ["branch", "-d", "same-commit"]); + + yield* writeTextFile(updater, "remote.txt", "remote\n"); + yield* git(updater, ["add", "remote.txt"]); + yield* git(updater, ["commit", "-m", "remote commit"]); + yield* git(updater, ["push", "origin", initialBranch]); + assert.equal(yield* snapshot, initial); + yield* git(cwd, ["fetch", "origin"]); + const fetched = yield* snapshot; + assert.notEqual(fetched, initial); + + const shallow = yield* makeTmpDir("git-vcs-driver-shallow-"); + yield* git(shallow, ["clone", "--depth=1", NodeURL.pathToFileURL(remote).href, "."]); + const shallowRefs = yield* git(shallow, ["show-ref", "--head"]); + const shallowSnapshot = yield* driver.readRefSnapshot(shallow); + assert.isNotNull(shallowSnapshot); + const shallowWorktree = yield* makeTmpDir("git-vcs-driver-shallow-worktree-"); + yield* git(shallow, ["worktree", "add", "--detach", shallowWorktree]); + const worktreeSnapshot = yield* driver.readRefSnapshot(shallowWorktree); + assert.isNotNull(worktreeSnapshot); + yield* git(shallow, ["fetch", "--unshallow", "origin"]); + assert.equal(yield* git(shallow, ["show-ref", "--head"]), shallowRefs); + const unshallowSnapshot = yield* driver.readRefSnapshot(shallow); + const unshallowWorktreeSnapshot = yield* driver.readRefSnapshot(shallowWorktree); + assert.isNotNull(unshallowSnapshot); + assert.isNotNull(unshallowWorktreeSnapshot); + assert.notEqual(unshallowSnapshot, shallowSnapshot); + assert.notEqual(unshallowWorktreeSnapshot, worktreeSnapshot); + + yield* git(cwd, ["checkout", "--detach"]); + yield* git(cwd, ["commit", "--allow-empty", "-m", "detached"]); + assert.notEqual(yield* snapshot, fetched); + + const head = yield* git(cwd, ["rev-parse", "HEAD"]); + yield* driver.execute({ + operation: "GitVcsDriver.test.git", + cwd, + args: ["update-ref", "--stdin"], + stdin: Array.from( + { length: 16_000 }, + (_, index) => `create refs/heads/snapshot-${String(index).padStart(5, "0")} ${head}\n`, + ).join(""), + }); + yield* git(cwd, ["pack-refs", "--all"]); + const large = yield* snapshot; + assert.isNotNull(large); + assert.isAbove(Buffer.byteLength(large ?? "", "utf8"), 1_000_000); + assert.equal(yield* snapshot, large); + yield* git(cwd, ["update-ref", "refs/heads/snapshot-00000", "HEAD~1"]); + assert.notEqual(yield* snapshot, large); + + assert.isNull(yield* driver.readRefSnapshot(yield* makeTmpDir())); + }), + ); + it.effect("does not start Git auto-maintenance from background upstream fetches", () => Effect.gen(function* () { const cwd = yield* makeTmpDir(); diff --git a/apps/server/src/vcs/GitVcsDriverCore.ts b/apps/server/src/vcs/GitVcsDriverCore.ts index 71487a327fc1f..dabcd1112ab06 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -83,6 +83,7 @@ const FETCH_REMOTE_TIMEOUT_MS = Duration.toMillis(Duration.minutes(5)); const WORKTREE_INDEX_REFRESH_TIMEOUT_MS = Duration.toMillis(Duration.minutes(5)); const WORKTREE_REMOVE_TIMEOUT_MS = Duration.toMillis(Duration.minutes(5)); const DEFAULT_MAX_OUTPUT_BYTES = 1_000_000; +const REF_SNAPSHOT_MAX_OUTPUT_BYTES = 16 * 1024 * 1024; const OUTPUT_TRUNCATED_MARKER = "\n\n[truncated]"; const PREPARED_COMMIT_PATCH_MAX_OUTPUT_BYTES = 49_000; const RANGE_COMMIT_SUMMARY_MAX_OUTPUT_BYTES = 19_000; @@ -1733,6 +1734,38 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ), }); + const readRefSnapshot: GitVcsDriver.GitVcsDriver["Service"]["readRefSnapshot"] = Effect.fn( + "GitVcsDriver.readRefSnapshot", + )( + function* (cwd) { + const result = yield* executeGit( + "GitVcsDriver.readRefSnapshot", + cwd, + ["show-ref", "--head"], + { + allowNonZeroExit: true, + maxOutputBytes: REF_SNAPSHOT_MAX_OUTPUT_BYTES, + }, + ); + if (result.exitCode !== 0) return null; + const symbolicHead = yield* executeGit( + "GitVcsDriver.readRefSnapshot", + cwd, + ["symbolic-ref", "--quiet", "HEAD"], + { allowNonZeroExit: true }, + ); + // Exit 1 means detached HEAD; other failures require a full status read. + if (symbolicHead.exitCode !== 0 && symbolicHead.exitCode !== 1) return null; + // Deepening a shallow clone changes ancestry without moving refs. + const shallowPath = path.join(yield* resolveGitCommonDir(cwd), "shallow"); + const shallow = (yield* fileSystem.exists(shallowPath)) + ? yield* fileSystem.readFileString(shallowPath) + : ""; + return `${result.stdout}\0${symbolicHead.stdout}\0${shallow}`; + }, + Effect.orElseSucceed(() => null), + ); + const refreshStatusUpstreamIfStale = Effect.fn("refreshStatusUpstreamIfStale")(function* ( cwd: string, ) { @@ -5929,6 +5962,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* statusDetails, statusDetailsLocal, statusDetailsRemote, + readRefSnapshot, prepareCommitContext, commit: Effect.fnUntraced(function* (cwd, subject, body, options) { return yield* withListRefsInvalidation(cwd, commit(cwd, subject, body, options)); diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts index cbe3dafd8fc5b..852c9f9ba0631 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -78,6 +78,7 @@ function layerTestFor(state: { localInvalidationCalls: number; remoteInvalidationCalls: number; remoteStatusRefreshUpstreamValues?: Array; + refSnapshot?: string | null; remoteStatusError?: GitManagerError; backgroundWorkEnabled?: boolean; automaticFetchInterval?: Effect.Effect; @@ -135,6 +136,8 @@ function layerTestFor(state: { : (state.beforeCurrentBranch ?? Effect.void).pipe( Effect.andThen(Effect.sync(() => state.currentLocalStatus.refName)), ), + readRefSnapshot: () => + Effect.sync(() => (state.refSnapshot === undefined ? "refs" : state.refSnapshot)), upstreamDivergence: () => Effect.sync(() => state.currentRemoteStatus?.hasUpstream && state.currentLocalStatus.refName !== null @@ -748,6 +751,7 @@ describe("VcsStatusBroadcaster", () => { Layer.mock(GitWorkflowService.GitWorkflowService)({ localStatus: () => Effect.succeed(localStatus), currentBranch: () => Effect.succeed(localStatus.refName), + readRefSnapshot: () => Effect.succeed("refs"), remoteStatus: () => Effect.succeed(remoteStatus), invalidateLocalStatus: () => Effect.void, invalidateRemoteStatus: () => Effect.void, @@ -844,6 +848,7 @@ describe("VcsStatusBroadcaster", () => { Layer.mock(GitWorkflowService.GitWorkflowService)({ localStatus: () => Effect.succeed(baseLocalStatus), currentBranch: () => Effect.succeed(baseLocalStatus.refName), + readRefSnapshot: () => Effect.succeed("refs"), remoteStatus: () => Effect.gen(function* () { remoteReads += 1; @@ -890,6 +895,7 @@ describe("VcsStatusBroadcaster", () => { Layer.mock(GitWorkflowService.GitWorkflowService)({ localStatus: () => Effect.succeed(baseLocalStatus), currentBranch: () => Effect.succeed(baseLocalStatus.refName), + readRefSnapshot: () => Effect.succeed("refs"), remoteStatus: () => Effect.gen(function* () { remoteReads += 1; @@ -943,6 +949,7 @@ describe("VcsStatusBroadcaster", () => { return state.currentLocalStatus; }), currentBranch: () => Effect.succeed(state.currentLocalStatus.refName), + readRefSnapshot: () => Effect.succeed("refs"), remoteStatus: () => Effect.suspend(() => { state.remoteStatusCalls += 1; @@ -1160,6 +1167,7 @@ describe("VcsStatusBroadcaster", () => { branchChanges: { baseRef: "origin/main", insertions: 745, deletions: 24 }, }; let cachedLocal: VcsStatusLocalResult | undefined; + let refSnapshot = "before"; const readLocalStatus = () => Effect.sync(() => (cachedLocal ??= local)); const layerTest = VcsStatusBroadcaster.layer.pipe( Layer.provideMerge(NodeServices.layer), @@ -1177,6 +1185,7 @@ describe("VcsStatusBroadcaster", () => { }), currentBranch: () => Effect.succeed(baseLocalStatus.refName), invalidateRemoteStatus: () => Effect.void, + readRefSnapshot: () => Effect.sync(() => refSnapshot), remoteStatus: () => Deferred.succeed(fetchStarted, undefined).pipe( Effect.andThen(Deferred.await(finishFetch)), @@ -1195,6 +1204,7 @@ describe("VcsStatusBroadcaster", () => { ...baseLocalStatus, branchChanges: { baseRef: "origin/main", insertions: 0, deletions: 0 }, }; + refSnapshot = "after"; yield* Deferred.succeed(finishFetch, undefined); const status = yield* Fiber.join(refresh); assert.deepStrictEqual(status.branchChanges, local.branchChanges); @@ -1202,6 +1212,58 @@ describe("VcsStatusBroadcaster", () => { }); }); + it.effect("an explicit refresh re-reads local totals only after refs moved", () => { + const totals = (insertions: number): VcsStatusLocalResult => ({ + ...baseLocalStatus, + branchChanges: { baseRef: "origin/main", insertions, deletions: 0 }, + }); + const state: Parameters[0] = { + currentLocalStatus: totals(745), + currentRemoteStatus: remoteStatusWithPr, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const subscribed = yield* Deferred.make(); + const events = yield* broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }).pipe( + Stream.tap((event) => + event._tag === "snapshot" ? Deferred.succeed(subscribed, undefined) : Effect.void, + ), + Stream.filter((event) => event._tag !== "snapshot"), + Stream.take(3), + Stream.runCollect, + Effect.forkScoped, + ); + yield* Deferred.await(subscribed); + + state.localStatusCalls = 0; + yield* broadcaster.refreshStatus("/repo"); + assert.equal(state.localStatusCalls, 1); + + state.localStatusCalls = 0; + state.currentRemoteStatus = baseRemoteStatus; + state.beforeRemoteStatus = Effect.sync(() => { + state.currentLocalStatus = totals(0); + state.refSnapshot = "moved"; + }); + const moved = yield* broadcaster.refreshStatus("/repo"); + assert.equal(state.localStatusCalls, 2); + assert.deepStrictEqual(moved.branchChanges, totals(0).branchChanges); + assert.deepStrictEqual( + [...(yield* Fiber.join(events))].map((event) => event._tag), + ["remoteUpdated", "remoteUpdated", "localUpdated"], + ); + + state.localStatusCalls = 0; + state.refSnapshot = null; + yield* broadcaster.refreshStatus("/repo"); + assert.equal(state.localStatusCalls, 2); + }).pipe(Effect.provide(layerTestFor(state)), Effect.scoped); + }); + it.effect("publishes local status while an earlier lookup holds the remote lock", () => { const state: Parameters[0] = { currentLocalStatus: baseLocalStatus, diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index eeb2b6cc982b9..8e69679d50638 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -591,7 +591,7 @@ export const make = Effect.gen(function* () { }); const refreshRemoteAndLocalTotals = Effect.fn("VcsStatusBroadcaster.refreshRemoteAndLocalTotals")( - function* (cwd: string, policyCwd: string) { + function* (cwd: string, policyCwd: string, refsBeforeLocalRead: string | null) { return yield* withRemoteWriteLock( cwd, Effect.gen(function* () { @@ -599,14 +599,19 @@ export const make = Effect.gen(function* () { // Bump the PR lookup epoch after earlier remote writers have finished. yield* workflow.invalidateStatus(cwd); const remoteRead = yield* readRemoteStatus(cwd, { refreshUpstream }); - // Fetch can move the base used for local Changes totals. - yield* refreshLocalStatusCore(cwd); // Without a fetch, the cached behind count cannot justify an automatic pull. const pulled = refreshUpstream ? yield* maybeAutoPull(cwd, remoteRead, [policyCwd]) : null; if (pulled !== null) return mergeGitStatusParts(pulled.local, pulled.remote); yield* updateCachedRemoteStatus(cwd, remoteRead, { publish: true }); + // A fetch, ours or not, can move the base used for local Changes totals. + if ( + refsBeforeLocalRead === null || + (yield* workflow.readRefSnapshot(cwd)) !== refsBeforeLocalRead + ) { + yield* refreshLocalStatusCore(cwd); + } const parts = yield* getStatusParts(cwd); return mergeGitStatusParts(parts.local, parts.remote); }), @@ -618,13 +623,14 @@ export const make = Effect.gen(function* () { "VcsStatusBroadcaster.refreshStatus", )(function* (rawCwd, options) { const cwd = yield* withFileSystem(normalizeCwd(rawCwd)); + const refsBeforeLocalRead = options?.localOnly ? null : yield* workflow.readRefSnapshot(cwd); yield* refreshLocalStatusCore(cwd); if (options?.localOnly) { // Never cache a remote here: readers take `value: null` as "read and found no PR". const parts = yield* getStatusParts(cwd); return mergeGitStatusParts(parts.local, parts.remote); } - return yield* refreshRemoteAndLocalTotals(cwd, rawCwd); + return yield* refreshRemoteAndLocalTotals(cwd, rawCwd, refsBeforeLocalRead); }); const refreshAfterCheckout: VcsStatusBroadcaster["Service"]["refreshAfterCheckout"] = Effect.fn( @@ -635,7 +641,7 @@ export const make = Effect.gen(function* () { // Owned by the broadcaster so a client disconnecting mid-switch still leaves status correct. yield* refreshLocalStatusCore(cwd).pipe( Effect.ensuring(Deferred.succeed(localDone, undefined)), - Effect.andThen(refreshRemoteAndLocalTotals(cwd, rawCwd)), + Effect.andThen(refreshRemoteAndLocalTotals(cwd, rawCwd, null)), Effect.ignoreCause({ log: true }), Effect.forkIn(broadcasterScope), );