From 5823ea50b4f47c330e8658af622b7761e1d94983 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 16:12:43 +0800 Subject: [PATCH 1/8] fix(web,server): the branch label no longer flips back after a switch Co-Authored-By: Claude Opus 5.5 --- .../src/vcs/VcsStatusBroadcaster.test.ts | 43 +++++++++++++ apps/server/src/vcs/VcsStatusBroadcaster.ts | 64 ++++++++++++++----- apps/server/src/ws.ts | 14 +++- .../components/BranchToolbar.logic.test.ts | 19 ++++++ .../web/src/components/BranchToolbar.logic.ts | 17 +++++ .../BranchToolbarBranchSelector.tsx | 20 +++++- 6 files changed, 156 insertions(+), 21 deletions(-) diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts index 042f8274543e5..e199a1fbcaf1c 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -656,6 +656,49 @@ describe("VcsStatusBroadcaster", () => { }, ); + it.effect("publishes a checkout's branch before returning, ahead of the remote read", () => { + const state: Parameters[0] = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + yield* broadcaster.getStatus({ cwd: "/repo" }); + const subscribed = yield* Deferred.make(); + const localPublished = yield* Deferred.make(); + const remotePublished = yield* Deferred.make(); + yield* broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }).pipe( + Stream.runForEach((event) => { + if (event._tag === "snapshot") return Deferred.succeed(subscribed, undefined); + if (event._tag === "localUpdated") return Deferred.succeed(localPublished, event.local); + return event.remote === null + ? Effect.void + : Deferred.succeed(remotePublished, event.remote); + }), + Effect.forkScoped, + ); + yield* Deferred.await(subscribed); + + const releaseRemote = yield* Deferred.make(); + state.beforeRemoteStatus = Deferred.await(releaseRemote); + state.currentLocalStatus = { ...baseLocalStatus, refName: "main" }; + state.currentRemoteStatus = remoteStatusWithPr; + yield* broadcaster.refreshAfterCheckout("/repo"); + + assert.equal((yield* Deferred.await(localPublished)).refName, "main"); + assert.isFalse(yield* Deferred.isDone(remotePublished)); + + yield* Deferred.succeed(releaseRemote, undefined); + assert.deepStrictEqual(yield* Deferred.await(remotePublished), remoteStatusWithPr); + assert.equal((yield* broadcaster.getStatus({ cwd: "/repo" })).pr?.number, 2978); + }).pipe(Effect.provide(layerTestFor(state)), Effect.scoped); + }); + it.effect("an explicit refresh reads local totals after the fetch", () => { const state: Parameters[0] = { currentLocalStatus: baseLocalStatus, diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index a8839d0c1cd0c..e3bbec22e1163 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -1,5 +1,6 @@ import * as Cause from "effect/Cause"; import * as Context from "effect/Context"; +import * as Deferred from "effect/Deferred"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; @@ -211,6 +212,11 @@ export class VcsStatusBroadcaster extends Context.Service< readonly refreshLocalStatus: ( cwd: string, ) => Effect.Effect; + /** + * For a checkout someone is waiting on. Returns once the new branch is published, so the + * caller's reply does not outrun it, and finishes the slower remote read in the background. + */ + readonly refreshAfterCheckout: (cwd: string) => Effect.Effect; readonly refreshStatus: ( cwd: string, options?: { readonly localOnly?: boolean }, @@ -499,6 +505,31 @@ export const make = Effect.gen(function* () { ); }); + const refreshRemoteAndLocalTotals = Effect.fn("VcsStatusBroadcaster.refreshRemoteAndLocalTotals")( + function* (cwd: string, policyCwd: string) { + return yield* withRemoteWriteLock( + cwd, + Effect.gen(function* () { + const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval); + // 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. + const local = yield* workflow.localStatus({ cwd }); + yield* updateCachedLocalStatus(cwd, local, { publish: true }); + // 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 }); + const parts = yield* getStatusParts(cwd); + return mergeGitStatusParts(parts.local, parts.remote); + }), + ); + }, + ); + const refreshStatus: VcsStatusBroadcaster["Service"]["refreshStatus"] = Effect.fn( "VcsStatusBroadcaster.refreshStatus", )(function* (rawCwd, options) { @@ -522,24 +553,22 @@ export const make = Effect.gen(function* () { }), ); } - return yield* withRemoteWriteLock( - cwd, - Effect.gen(function* () { - const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval); - // 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. - const local = yield* workflow.localStatus({ cwd }); - yield* updateCachedLocalStatus(cwd, local, { publish: true }); - // Without a fetch, the cached behind count cannot justify an automatic pull. - const pulled = refreshUpstream ? yield* maybeAutoPull(cwd, remoteRead, [rawCwd]) : null; - if (pulled !== null) return mergeGitStatusParts(pulled.local, pulled.remote); - yield* updateCachedRemoteStatus(cwd, remoteRead, { publish: true }); - const parts = yield* getStatusParts(cwd); - return mergeGitStatusParts(parts.local, parts.remote); - }), + return yield* refreshRemoteAndLocalTotals(cwd, rawCwd); + }); + + const refreshAfterCheckout: VcsStatusBroadcaster["Service"]["refreshAfterCheckout"] = Effect.fn( + "VcsStatusBroadcaster.refreshAfterCheckout", + )(function* (rawCwd) { + const cwd = yield* withFileSystem(normalizeCwd(rawCwd)); + const localDone = yield* Deferred.make(); + // 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.ignoreCause({ log: true }), + Effect.forkIn(broadcasterScope), ); + yield* Deferred.await(localDone); }); const refreshPullRequestStatus: VcsStatusBroadcaster["Service"]["refreshPullRequestStatus"] = @@ -772,6 +801,7 @@ export const make = Effect.gen(function* () { return VcsStatusBroadcaster.of({ getStatus, refreshLocalStatus, + refreshAfterCheckout, refreshStatus, refreshPullRequestStatus, streamStatus, diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 8373bbfae0c55..44ba76497b686 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -3028,9 +3028,19 @@ export const layerWsRpc = ( [WS_METHODS.vcsRemoveWorktree]: (input) => gitWorkflow.removeWorktree(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), [WS_METHODS.vcsCreateRef]: (input) => - gitWorkflow.createRef(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), + gitWorkflow + .createRef(input) + .pipe( + Effect.tap(() => + input.switchRef + ? vcsStatusBroadcaster.refreshAfterCheckout(input.cwd) + : refreshGitStatus(input.cwd), + ), + ), [WS_METHODS.vcsSwitchRef]: (input) => - gitWorkflow.switchRef(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), + gitWorkflow + .switchRef(input) + .pipe(Effect.tap(() => vcsStatusBroadcaster.refreshAfterCheckout(input.cwd))), [WS_METHODS.vcsInit]: (input) => vcsProvisioning.initRepository(input).pipe(Effect.tap(() => refreshGitStatus(input.cwd))), [WS_METHODS.reviewGetBranchCommits]: (input) => review.getBranchCommits(input), diff --git a/apps/web/src/components/BranchToolbar.logic.test.ts b/apps/web/src/components/BranchToolbar.logic.test.ts index 08e6f7633dee3..1b7f6887373ee 100644 --- a/apps/web/src/components/BranchToolbar.logic.test.ts +++ b/apps/web/src/components/BranchToolbar.logic.test.ts @@ -10,6 +10,7 @@ import { resolveLocalCheckoutBranchMismatch, sanitizeNewRefName, shouldIncludeBranchPickerItem, + isBranchHoldCurrent, } from "./BranchToolbar.logic"; describe("resolveDraftEnvModeAfterBranchChange", () => { @@ -235,6 +236,24 @@ describe("shouldIncludeBranchPickerItem", () => { }); }); +describe("isBranchHoldCurrent", () => { + const hold = { branch: "feature", replaces: "main", cwd: "/repo" }; + + it("keeps showing the checked-out branch while status still reports the old one", () => { + expect(isBranchHoldCurrent(hold, { canonicalBranch: "main", cwd: "/repo" })).toBe(true); + }); + + it("lets go once status moves, even to a different branch than the checkout", () => { + expect(isBranchHoldCurrent(hold, { canonicalBranch: "feature", cwd: "/repo" })).toBe(false); + expect(isBranchHoldCurrent(hold, { canonicalBranch: "other", cwd: "/repo" })).toBe(false); + expect(isBranchHoldCurrent(hold, { canonicalBranch: null, cwd: "/repo" })).toBe(false); + }); + + it("does not apply to another checkout that happens to be on the old branch", () => { + expect(isBranchHoldCurrent(hold, { canonicalBranch: "main", cwd: "/other" })).toBe(false); + }); +}); + describe("sanitizeNewRefName", () => { it("collapses a run of whitespace into a single dash", () => { expect(sanitizeNewRefName("new branch")).toBe("new-branch"); diff --git a/apps/web/src/components/BranchToolbar.logic.ts b/apps/web/src/components/BranchToolbar.logic.ts index e22dcfaddedeb..4895c7f34ea7c 100644 --- a/apps/web/src/components/BranchToolbar.logic.ts +++ b/apps/web/src/components/BranchToolbar.logic.ts @@ -196,6 +196,23 @@ export function resolveBranchToolbarValue(input: { return currentGitBranch ?? activeThreadBranch; } +export interface BranchHold { + readonly branch: string | null; + readonly replaces: string | null; + readonly cwd: string; +} + +/** + * A finished checkout's branch stays on screen until status reports any change, because the status + * stream and the RPC reply are not ordered and showing status first would flip back to the old branch. + */ +export function isBranchHoldCurrent( + hold: BranchHold, + current: { readonly canonicalBranch: string | null; readonly cwd: string | null }, +): boolean { + return hold.cwd === current.cwd && hold.replaces === current.canonicalBranch; +} + export function resolveBranchTriggerLabel(input: { activeWorktreePath: string | null; effectiveEnvMode: EnvMode; diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 8f6ffdf0a119e..129f94e2d38cf 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -46,7 +46,9 @@ import { parsePullRequestReference } from "@t3tools/client-runtime/pull-request- import { getSourceControlPresentation } from "../sourceControlPresentation"; import { useComposerMenuProps } from "./chat/composerEventScope"; import { + type BranchHold, deriveLocalBranchNameFromRemoteRef, + isBranchHoldCurrent, resolveBranchTriggerLabel, resolveBranchToolbarPrBranch, resolveBranchSelectionTarget, @@ -286,8 +288,13 @@ export function BranchToolbarBranchSelector({ activeThreadBranch, currentGitBranch, }); + const [branchHold, setBranchHold] = useState(null); + const branchHoldCurrent = + branchHold !== null && + isBranchHoldCurrent(branchHold, { canonicalBranch: canonicalActiveBranch, cwd: branchCwd }); + if (branchHold !== null && !branchHoldCurrent) setBranchHold(null); const [resolvedActiveBranch, setOptimisticBranch] = useOptimistic( - canonicalActiveBranch, + branchHoldCurrent ? branchHold.branch : canonicalActiveBranch, (_currentBranch: string | null, optimisticBranch: string | null) => optimisticBranch, ); const listedActiveBranch = @@ -376,7 +383,6 @@ export function BranchToolbarBranchSelector({ startBranchActionTransition(async () => { await action(); branchRefState.refresh(); - branchStatusQuery.refresh(); }); }; @@ -425,6 +431,11 @@ export function BranchToolbarBranchSelector({ ? (checkoutResult.value.refName ?? selectedBranchName) : selectedBranchName; setOptimisticBranch(nextBranchName); + setBranchHold({ + branch: nextBranchName, + replaces: canonicalActiveBranch, + cwd: selectionTarget.checkoutCwd, + }); setThreadBranch(nextBranchName, selectionTarget.nextWorktreePath); return; } @@ -462,6 +473,11 @@ export function BranchToolbarBranchSelector({ }); if (createBranchResult._tag === "Success") { setOptimisticBranch(createBranchResult.value.refName); + setBranchHold({ + branch: createBranchResult.value.refName, + replaces: canonicalActiveBranch, + cwd: branchCwd, + }); setThreadBranch(createBranchResult.value.refName, activeWorktreePath); return; } From c63e30b009b936cd9c1a74de9e2ec468e8739c57 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 17:19:19 +0800 Subject: [PATCH 2/8] fix(web): a branch hold does not carry over to another environment Co-Authored-By: Claude Opus 5.5 --- .../components/BranchToolbar.logic.test.ts | 22 +++++++++++++------ .../web/src/components/BranchToolbar.logic.ts | 13 +++++++++-- .../BranchToolbarBranchSelector.tsx | 8 ++++++- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/apps/web/src/components/BranchToolbar.logic.test.ts b/apps/web/src/components/BranchToolbar.logic.test.ts index 1b7f6887373ee..6819e8242a3cd 100644 --- a/apps/web/src/components/BranchToolbar.logic.test.ts +++ b/apps/web/src/components/BranchToolbar.logic.test.ts @@ -1,4 +1,4 @@ -import { type VcsRef } from "@t3tools/contracts"; +import { EnvironmentId, type VcsRef } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; import { dedupeRemoteBranchesWithLocalMatches } from "@t3tools/shared/git"; import { @@ -237,20 +237,28 @@ describe("shouldIncludeBranchPickerItem", () => { }); describe("isBranchHoldCurrent", () => { - const hold = { branch: "feature", replaces: "main", cwd: "/repo" }; + const environmentId = EnvironmentId.make("env-a"); + const hold = { branch: "feature", replaces: "main", environmentId, cwd: "/repo" }; + const current = { canonicalBranch: "main", environmentId, cwd: "/repo" }; it("keeps showing the checked-out branch while status still reports the old one", () => { - expect(isBranchHoldCurrent(hold, { canonicalBranch: "main", cwd: "/repo" })).toBe(true); + expect(isBranchHoldCurrent(hold, current)).toBe(true); }); it("lets go once status moves, even to a different branch than the checkout", () => { - expect(isBranchHoldCurrent(hold, { canonicalBranch: "feature", cwd: "/repo" })).toBe(false); - expect(isBranchHoldCurrent(hold, { canonicalBranch: "other", cwd: "/repo" })).toBe(false); - expect(isBranchHoldCurrent(hold, { canonicalBranch: null, cwd: "/repo" })).toBe(false); + for (const canonicalBranch of ["feature", "other", null]) { + expect(isBranchHoldCurrent(hold, { ...current, canonicalBranch })).toBe(false); + } }); it("does not apply to another checkout that happens to be on the old branch", () => { - expect(isBranchHoldCurrent(hold, { canonicalBranch: "main", cwd: "/other" })).toBe(false); + expect(isBranchHoldCurrent(hold, { ...current, cwd: "/other" })).toBe(false); + }); + + it("does not apply to the same path on another environment", () => { + expect( + isBranchHoldCurrent(hold, { ...current, environmentId: EnvironmentId.make("env-b") }), + ).toBe(false); }); }); diff --git a/apps/web/src/components/BranchToolbar.logic.ts b/apps/web/src/components/BranchToolbar.logic.ts index 4895c7f34ea7c..e882daa386a9f 100644 --- a/apps/web/src/components/BranchToolbar.logic.ts +++ b/apps/web/src/components/BranchToolbar.logic.ts @@ -199,6 +199,7 @@ export function resolveBranchToolbarValue(input: { export interface BranchHold { readonly branch: string | null; readonly replaces: string | null; + readonly environmentId: EnvironmentId; readonly cwd: string; } @@ -208,9 +209,17 @@ export interface BranchHold { */ export function isBranchHoldCurrent( hold: BranchHold, - current: { readonly canonicalBranch: string | null; readonly cwd: string | null }, + current: { + readonly canonicalBranch: string | null; + readonly environmentId: EnvironmentId; + readonly cwd: string | null; + }, ): boolean { - return hold.cwd === current.cwd && hold.replaces === current.canonicalBranch; + return ( + hold.environmentId === current.environmentId && + hold.cwd === current.cwd && + hold.replaces === current.canonicalBranch + ); } export function resolveBranchTriggerLabel(input: { diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 129f94e2d38cf..80ff1970888ef 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -291,7 +291,11 @@ export function BranchToolbarBranchSelector({ const [branchHold, setBranchHold] = useState(null); const branchHoldCurrent = branchHold !== null && - isBranchHoldCurrent(branchHold, { canonicalBranch: canonicalActiveBranch, cwd: branchCwd }); + isBranchHoldCurrent(branchHold, { + canonicalBranch: canonicalActiveBranch, + environmentId, + cwd: branchCwd, + }); if (branchHold !== null && !branchHoldCurrent) setBranchHold(null); const [resolvedActiveBranch, setOptimisticBranch] = useOptimistic( branchHoldCurrent ? branchHold.branch : canonicalActiveBranch, @@ -434,6 +438,7 @@ export function BranchToolbarBranchSelector({ setBranchHold({ branch: nextBranchName, replaces: canonicalActiveBranch, + environmentId, cwd: selectionTarget.checkoutCwd, }); setThreadBranch(nextBranchName, selectionTarget.nextWorktreePath); @@ -476,6 +481,7 @@ export function BranchToolbarBranchSelector({ setBranchHold({ branch: createBranchResult.value.refName, replaces: canonicalActiveBranch, + environmentId, cwd: branchCwd, }); setThreadBranch(createBranchResult.value.refName, activeWorktreePath); From 264613434457256d67edbf722e9788d59dbf7ff4 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 16:13:02 +0800 Subject: [PATCH 3/8] perf(server): PR status comes back sooner after a branch switch Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 36 +++++++ apps/server/src/git/GitManager.ts | 10 +- apps/server/src/git/GitWorkflowService.ts | 4 +- apps/server/src/vcs/GitVcsDriver.ts | 2 + apps/server/src/vcs/GitVcsDriverCore.test.ts | 25 +++++ apps/server/src/vcs/GitVcsDriverCore.ts | 18 ++++ .../src/vcs/VcsStatusBroadcaster.test.ts | 102 +++++++++++++++++- apps/server/src/vcs/VcsStatusBroadcaster.ts | 30 +++++- 8 files changed, 221 insertions(+), 6 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index a71f19e6a973b..d60d25cfa172c 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -2433,6 +2433,42 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect("a checkout's invalidation keeps the branch's cached PR lookup", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + yield* runGit(repoDir, ["checkout", "-b", "feature/pr-cached"]); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/pr-cached"]); + + const { manager } = yield* makeManager({ + providerScenario: { + prListSequence: [ + [ + { + number: 215, + title: "Cached PR", + url: "https://github.com/pingdotgg/codething-mvp/pull/215", + baseRefName: "main", + headRefName: "feature/pr-cached", + }, + ], + [], + ], + }, + }); + + expect((yield* manager.status({ cwd: repoDir })).pr?.number).toBe(215); + + yield* manager.invalidateStatus(repoDir, { keepPullRequestLookups: true }); + expect((yield* manager.status({ cwd: repoDir })).pr?.number).toBe(215); + + yield* manager.invalidateStatus(repoDir); + expect((yield* manager.status({ cwd: repoDir })).pr).toBeNull(); + }), + ); + it.effect( "status does not reuse a stale PR after the branch is retargeted to a different upstream", () => diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 95f5187b1a303..86c94ddc21412 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -140,7 +140,11 @@ export class GitManager extends Context.Service< ) => Effect.Effect; readonly invalidateLocalStatus: (cwd: string) => Effect.Effect; readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect; - readonly invalidateStatus: (cwd: string) => Effect.Effect; + /** keepPullRequestLookups is for changes that cannot affect a PR, such as a checkout. */ + readonly invalidateStatus: ( + cwd: string, + options?: { readonly keepPullRequestLookups?: boolean }, + ) => Effect.Effect; readonly resolvePullRequest: ( input: GitPullRequestRefInput, ) => Effect.Effect; @@ -2665,10 +2669,10 @@ export const make = Effect.gen(function* () { yield* invalidateRemoteStatusResultCache(cwd); }); const invalidateStatus: GitManager["Service"]["invalidateStatus"] = Effect.fn("invalidateStatus")( - function* (cwd) { + function* (cwd, options) { yield* invalidateLocalStatusResultCache(cwd); yield* invalidateRemoteStatusResultCache(cwd); - yield* bumpPrLookupEpoch(cwd); + if (!options?.keepPullRequestLookups) yield* bumpPrLookupEpoch(cwd); yield* forgetPrLookupGitState(cwd); }, ); diff --git a/apps/server/src/git/GitWorkflowService.ts b/apps/server/src/git/GitWorkflowService.ts index fdc00849af2d0..89ae147ce9f29 100644 --- a/apps/server/src/git/GitWorkflowService.ts +++ b/apps/server/src/git/GitWorkflowService.ts @@ -59,7 +59,8 @@ export class GitWorkflowService extends Context.Service< ) => Effect.Effect; readonly invalidateLocalStatus: (cwd: string) => Effect.Effect; readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect; - readonly invalidateStatus: (cwd: string) => Effect.Effect; + readonly invalidateStatus: GitManager.GitManager["Service"]["invalidateStatus"]; + readonly statusUpstreamFetchCount: GitVcsDriver.GitVcsDriver["Service"]["statusUpstreamFetchCount"]; readonly pullCurrentBranch: (cwd: string) => Effect.Effect; readonly runStackedAction: ( input: GitRunStackedActionInput, @@ -369,6 +370,7 @@ export const make = Effect.gen(function* () { invalidateLocalStatus: gitManager.invalidateLocalStatus, invalidateRemoteStatus: gitManager.invalidateRemoteStatus, invalidateStatus: gitManager.invalidateStatus, + statusUpstreamFetchCount: git.statusUpstreamFetchCount, pullCurrentBranch: Effect.fn("GitWorkflowService.pullCurrentBranch")(function (cwd) { return ensureGitCommand("GitWorkflowService.pullCurrentBranch", cwd).pipe( Effect.andThen(git.pullCurrentBranch(cwd)), diff --git a/apps/server/src/vcs/GitVcsDriver.ts b/apps/server/src/vcs/GitVcsDriver.ts index fe49a6b1391af..65dc2d183a3ce 100644 --- a/apps/server/src/vcs/GitVcsDriver.ts +++ b/apps/server/src/vcs/GitVcsDriver.ts @@ -310,6 +310,8 @@ export class GitVcsDriver extends Context.Service< cwd: string, options?: GitRemoteStatusOptions, ) => Effect.Effect; + /** Changes whenever a status read fetches this repository, so callers can tell if refs may have moved. */ + readonly statusUpstreamFetchCount: (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 d9a48457f8a38..534f66321554f 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -3386,6 +3386,31 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("counts a status read's upstream fetch only when one actually ran", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-vcs-driver-remote-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "-u", "origin", initialBranch]); + + const driver = yield* GitVcsDriver.GitVcsDriver; + const initial = yield* driver.statusUpstreamFetchCount(cwd); + + yield* driver.statusDetailsRemote(cwd, { refreshUpstream: false }); + assert.equal(yield* driver.statusUpstreamFetchCount(cwd), initial); + + yield* driver.statusDetailsRemote(cwd); + const afterFetch = yield* driver.statusUpstreamFetchCount(cwd); + assert.equal(afterFetch, initial + 1); + + // Still inside the refresh interval, so this read reuses the last fetch. + yield* driver.statusDetailsRemote(cwd); + assert.equal(yield* driver.statusUpstreamFetchCount(cwd), afterFetch); + }), + ); + 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 11580f4c26e18..c415f207a9285 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -1621,10 +1621,20 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } } }); + const statusUpstreamFetches = new Map(); const refreshStatusRemoteCacheEntry = Effect.fn("refreshStatusRemoteCacheEntry")(function* ( cacheKey: StatusRemoteRefreshCacheKey, ) { return yield* fetchRemoteForStatus(cacheKey.gitCommonDir, cacheKey.remoteName).pipe( + // Counted on failure too: a fetch can move some refs before it fails. + Effect.ensuring( + Effect.sync(() => { + statusUpstreamFetches.set( + cacheKey.gitCommonDir, + (statusUpstreamFetches.get(cacheKey.gitCommonDir) ?? 0) + 1, + ); + }), + ), Effect.as(true as const), ); }); @@ -1640,6 +1650,13 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ), }); + const statusUpstreamFetchCount: GitVcsDriver.GitVcsDriver["Service"]["statusUpstreamFetchCount"] = + (cwd) => + resolveGitCommonDir(cwd).pipe( + Effect.map((gitCommonDir) => statusUpstreamFetches.get(gitCommonDir) ?? 0), + Effect.orElseSucceed(() => 0), + ); + const refreshStatusUpstreamIfStale = Effect.fn("refreshStatusUpstreamIfStale")(function* ( cwd: string, ) { @@ -4936,6 +4953,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* statusDetails, statusDetailsLocal, statusDetailsRemote, + statusUpstreamFetchCount, 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 e199a1fbcaf1c..00518490f47b6 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -23,6 +23,7 @@ import type { VcsStatusStreamEvent, } from "@t3tools/contracts"; import { GitCommandError, GitManagerError } from "@t3tools/contracts"; +import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; import * as VcsStatusBroadcaster from "./VcsStatusBroadcaster.ts"; import * as BackgroundPolicy from "../background/BackgroundPolicy.ts"; @@ -72,6 +73,9 @@ function layerTestFor(state: { localInvalidationCalls: number; remoteInvalidationCalls: number; remoteStatusRefreshUpstreamValues?: Array; + remoteStatusInteractiveValues?: Array; + invalidateStatusKeepLookupValues?: Array; + upstreamFetchCount?: number; remoteStatusError?: GitManagerError; backgroundWorkEnabled?: boolean; automaticFetchInterval?: Effect.Effect; @@ -113,6 +117,13 @@ function layerTestFor(state: { return state.currentRemoteStatus; }), ), + Effect.tap(() => + Effect.gen(function* () { + state.remoteStatusInteractiveValues?.push( + yield* SourceControlRateLimit.Interactive, + ); + }), + ), Effect.tap(() => state.remoteStatusError ? Effect.fail(state.remoteStatusError) : Effect.void, ), @@ -126,11 +137,13 @@ function layerTestFor(state: { Effect.sync(() => { state.remoteInvalidationCalls += 1; }), - invalidateStatus: () => + invalidateStatus: (_cwd, options) => Effect.sync(() => { state.localInvalidationCalls += 1; state.remoteInvalidationCalls += 1; + state.invalidateStatusKeepLookupValues?.push(options?.keepPullRequestLookups); }), + statusUpstreamFetchCount: () => Effect.sync(() => state.upstreamFetchCount ?? 0), ...(state.pullCurrentBranch ? { pullCurrentBranch: state.pullCurrentBranch } : {}), }), ), @@ -699,6 +712,93 @@ describe("VcsStatusBroadcaster", () => { }).pipe(Effect.provide(layerTestFor(state)), Effect.scoped); }); + it.effect("a checkout's PR lookup is a user action that reuses cached lookups", () => { + const state: Parameters[0] = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + remoteStatusInteractiveValues: [], + invalidateStatusKeepLookupValues: [], + }; + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const remotePublished = yield* Deferred.make(); + const subscribed = yield* Deferred.make(); + yield* broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }).pipe( + Stream.runForEach((event) => { + if (event._tag === "snapshot") return Deferred.succeed(subscribed, undefined); + return event._tag === "remoteUpdated" && event.remote !== null + ? Deferred.succeed(remotePublished, undefined) + : Effect.void; + }), + Effect.forkScoped, + ); + yield* Deferred.await(subscribed); + + yield* broadcaster.refreshAfterCheckout("/repo"); + yield* Deferred.await(remotePublished); + + assert.deepStrictEqual(state.remoteStatusInteractiveValues, [true]); + assert.deepStrictEqual(state.invalidateStatusKeepLookupValues, [true]); + }).pipe(Effect.provide(layerTestFor(state)), Effect.scoped); + }); + + it.effect("a checkout re-reads local status only when a fetch ran", () => { + const state: Parameters[0] = { + currentLocalStatus: baseLocalStatus, + currentRemoteStatus: baseRemoteStatus, + localStatusCalls: 0, + remoteStatusCalls: 0, + localInvalidationCalls: 0, + remoteInvalidationCalls: 0, + }; + + return Effect.gen(function* () { + const broadcaster = yield* VcsStatusBroadcaster.VcsStatusBroadcaster; + const subscribed = yield* Deferred.make(); + const firstRemote = yield* Deferred.make(); + const secondRemote = yield* Deferred.make(); + yield* broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }).pipe( + Stream.runForEach((event) => { + if (event._tag === "snapshot") return Deferred.succeed(subscribed, undefined); + if (event._tag !== "remoteUpdated" || event.remote === null) return Effect.void; + return event.remote.pr === null + ? Deferred.succeed(firstRemote, undefined) + : Deferred.succeed(secondRemote, undefined); + }), + Effect.forkScoped, + ); + yield* Deferred.await(subscribed); + const readsBeforeCheckout = state.localStatusCalls; + + yield* broadcaster.refreshAfterCheckout("/repo"); + yield* Deferred.await(firstRemote); + assert.equal(state.localStatusCalls, readsBeforeCheckout + 1); + + const fetchedLocal: VcsStatusLocalResult = { + ...baseLocalStatus, + branchChanges: { baseRef: "origin/main", insertions: 0, deletions: 0 }, + }; + state.currentRemoteStatus = remoteStatusWithPr; + state.beforeRemoteStatus = Effect.sync(() => { + state.upstreamFetchCount = (state.upstreamFetchCount ?? 0) + 1; + state.currentLocalStatus = fetchedLocal; + }); + yield* broadcaster.refreshAfterCheckout("/repo"); + yield* Deferred.await(secondRemote); + + assert.equal(state.localStatusCalls, readsBeforeCheckout + 3); + assert.deepStrictEqual( + (yield* broadcaster.getStatus({ cwd: "/repo" })).branchChanges, + fetchedLocal.branchChanges, + ); + }).pipe(Effect.provide(layerTestFor(state)), Effect.scoped); + }); + it.effect("an explicit refresh reads local totals after the fetch", () => { const state: Parameters[0] = { currentLocalStatus: baseLocalStatus, diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index e3bbec22e1163..254d5bdc0f041 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -24,6 +24,7 @@ import type { } from "@t3tools/contracts"; import { DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL } from "@t3tools/contracts"; import { resolveServerBackgroundActivitySettings } from "@t3tools/shared/backgroundActivitySettings"; +import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; import { mergeGitStatusParts } from "@t3tools/shared/git"; import * as KeyedLock from "@t3tools/shared/KeyedLock"; import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; @@ -556,6 +557,33 @@ export const make = Effect.gen(function* () { return yield* refreshRemoteAndLocalTotals(cwd, rawCwd); }); + const refreshRemoteAfterCheckout = Effect.fn("VcsStatusBroadcaster.refreshRemoteAfterCheckout")( + function* (cwd: string, policyCwd: string) { + yield* withRemoteWriteLock( + cwd, + Effect.gen(function* () { + const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval); + yield* workflow.invalidateStatus(cwd, { keepPullRequestLookups: true }); + const fetchesBefore = yield* workflow.statusUpstreamFetchCount(cwd); + const remoteRead = yield* readRemoteStatus(cwd, { refreshUpstream }); + // Only a fetch can move the base used for local Changes totals. + if ((yield* workflow.statusUpstreamFetchCount(cwd)) !== fetchesBefore) { + 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) { + yield* updateCachedRemoteStatus(cwd, remoteRead, { publish: true }); + } + }), + ); + }, + // Someone just picked this branch, so its PR lookup skips the background batching delay. + Effect.provideService(SourceControlRateLimit.Interactive, true), + ); + const refreshAfterCheckout: VcsStatusBroadcaster["Service"]["refreshAfterCheckout"] = Effect.fn( "VcsStatusBroadcaster.refreshAfterCheckout", )(function* (rawCwd) { @@ -564,7 +592,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(refreshRemoteAfterCheckout(cwd, rawCwd)), Effect.ignoreCause({ log: true }), Effect.forkIn(broadcasterScope), ); From e54393def59082f1796e027bc0dfaa0ee6069ad0 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 17:12:20 +0800 Subject: [PATCH 4/8] fix(server): count status fetches when the process exits A waiter can time out while the fetch keeps running, so counting at that point records the fetch before refs move and can skip the local re-read after it. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 35 ++++++++++++++++++++ apps/server/src/vcs/GitVcsDriverCore.ts | 17 ++++------ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 534f66321554f..64eb9afcba3e1 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -1,6 +1,8 @@ // @effect-diagnostics nodeBuiltinImport:off - realpathSync.native resolves Windows 8.3 short names, which the Effect realPath does not. +import * as NodeChildProcess from "node:child_process"; import * as NodeFS from "node:fs"; import * as NodeOS from "node:os"; +import * as NodePath from "node:path"; import * as NodeURL from "node:url"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as HostProcess from "@t3tools/shared/HostProcess"; @@ -3411,6 +3413,39 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); + it.effect("does not count a status fetch that is still running after its wait times out", () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-vcs-driver-remote-"); + const control = yield* makeTmpDir("git-vcs-driver-control-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "-u", "origin", initialBranch]); + + const started = NodePath.join(control, "started"); + const release = NodePath.join(control, "release"); + NodeChildProcess.execFileSync("mkfifo", [started, release]); + const uploadPack = NodePath.join(control, "upload-pack.sh"); + NodeFS.writeFileSync( + uploadPack, + `#!/bin/sh\necho started > "${started}"\ncat "${release}" > /dev/null\nexec git-upload-pack "$@"\n`, + { mode: 0o755 }, + ); + yield* git(cwd, ["config", "remote.origin.uploadpack", uploadPack]); + + const driver = yield* GitVcsDriver.GitVcsDriver; + const initial = yield* driver.statusUpstreamFetchCount(cwd); + const status = yield* driver.statusDetailsRemote(cwd).pipe(Effect.forkScoped); + yield* Effect.promise(() => NodeFS.promises.readFile(started)); + yield* TestClock.adjust("6 seconds"); + yield* Fiber.join(status); + + assert.equal(yield* driver.statusUpstreamFetchCount(cwd), initial); + yield* Effect.promise(() => NodeFS.promises.writeFile(release, "go")); + }), + ); + 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 c415f207a9285..e89c885d87e58 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.ts @@ -1326,6 +1326,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* ); }); + const statusUpstreamFetches = new Map(); const fetchRemoteForStatus = Effect.fn("GitVcsDriver.fetchRemoteForStatus.wait")(function* ( gitCommonDir: string, remoteName: string, @@ -1374,6 +1375,12 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* Effect.onExit(() => Effect.sync(() => { statusFetches.delete(key); + // Counted at exit, not when a waiter stops waiting: a waiter can time out before refs + // move. Failures count too, because a failed fetch can still move some refs. + statusUpstreamFetches.set( + gitCommonDir, + (statusUpstreamFetches.get(gitCommonDir) ?? 0) + 1, + ); }), ), Effect.forkIn(serviceScope, { startImmediately: false, uninterruptible: false }), @@ -1621,20 +1628,10 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function* } } }); - const statusUpstreamFetches = new Map(); const refreshStatusRemoteCacheEntry = Effect.fn("refreshStatusRemoteCacheEntry")(function* ( cacheKey: StatusRemoteRefreshCacheKey, ) { return yield* fetchRemoteForStatus(cacheKey.gitCommonDir, cacheKey.remoteName).pipe( - // Counted on failure too: a fetch can move some refs before it fails. - Effect.ensuring( - Effect.sync(() => { - statusUpstreamFetches.set( - cacheKey.gitCommonDir, - (statusUpstreamFetches.get(cacheKey.gitCommonDir) ?? 0) + 1, - ); - }), - ), Effect.as(true as const), ); }); From 2f0c8d33a2b154618f1700a2a458e17c693fac66 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 17:19:57 +0800 Subject: [PATCH 5/8] fix(server): a git read in flight during a checkout does not outlive it Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 51 ++++++++++++++++++++++++++ apps/server/src/git/GitManager.ts | 17 ++++++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index d60d25cfa172c..09f919ecbc66a 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -1020,6 +1020,57 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); + it.effect("a git read in flight during a checkout does not outlive it", () => + Effect.gen(function* () { + const repoDir = yield* makeTempDir("t3code-git-manager-"); + yield* initRepo(repoDir); + const remoteDir = yield* createBareRemote(); + yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); + yield* runGit(repoDir, ["checkout", "-b", "feature/racing-upstream"]); + yield* runGit(repoDir, ["push", "-u", "origin", "feature/racing-upstream"]); + yield* runGit(repoDir, ["push", "origin", "HEAD:refs/heads/feature/raced-head"]); + const pullRequest = (number: number, headRefName: string): FakeChangeRequest[] => [ + { + number, + title: headRefName, + url: `https://github.com/pingdotgg/codething-mvp/pull/${number}`, + baseRefName: "main", + headRefName, + state: "open", + updatedAt: "2026-04-07T15:00:00Z", + }, + ]; + const gate = { + matches: (args: ReadonlyArray) => + args[0] === "for-each-ref" && args.at(-1) === "refs/heads/feature/racing-upstream", + reached: yield* Deferred.make(), + release: yield* Deferred.make(), + }; + const { manager } = yield* makeManager({ + gitSpawnGate: gate, + providerScenario: { + prListByHeadSelector: { + "feature/racing-upstream": pullRequest(237, "feature/racing-upstream"), + "feature/raced-head": pullRequest(238, "feature/raced-head"), + }, + }, + }); + const lookup = manager.branchPullRequest({ + cwd: repoDir, + branch: "feature/racing-upstream", + }); + + const inFlight = yield* Effect.forkChild(lookup); + yield* Deferred.await(gate.reached); + yield* runGit(repoDir, ["branch", "--set-upstream-to=origin/feature/raced-head"]); + yield* manager.invalidateStatus(repoDir, { keepPullRequestLookups: true }); + yield* Deferred.succeed(gate.release, undefined); + yield* Fiber.join(inFlight); + + expect((yield* lookup)?.number).toBe(238); + }), + ); + it.effect("branch PR lookup notifies linked threads when draft status changes", () => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); diff --git a/apps/server/src/git/GitManager.ts b/apps/server/src/git/GitManager.ts index 86c94ddc21412..396bcc430252d 100644 --- a/apps/server/src/git/GitManager.ts +++ b/apps/server/src/git/GitManager.ts @@ -1155,6 +1155,15 @@ export const make = Effect.gen(function* () { prLookupEpochByCwd.set(cacheKey, prLookupEpoch(cacheKey) + 1); }), ); + // Separate from the PR lookup epoch because a checkout keeps PR lookups but changes git state. + const prLookupGitStateEpochByCwd = new Map(); + const prLookupGitStateEpoch = (cwd: string) => prLookupGitStateEpochByCwd.get(cwd) ?? 0; + const bumpPrLookupGitStateEpoch = (cwd: string) => + normalizeStatusCacheKey(cwd).pipe( + Effect.map((cacheKey) => { + prLookupGitStateEpochByCwd.set(cacheKey, prLookupGitStateEpoch(cacheKey) + 1); + }), + ); const prLookupCacheKey = ( cwd: string, details: { @@ -2407,10 +2416,13 @@ export const make = Effect.gen(function* () { } // invalidateStatus bumps the epoch. A read that started before a T3 git // action, or before a newer read finished, must not replace their state. - const epoch = prLookupEpoch(cwd); + const epoch = prLookupGitStateEpoch(cwd); const value = yield* read; const latest = prLookupGitStateByKey.get(entryKey); - if (prLookupEpoch(cwd) !== epoch || (latest !== undefined && latest.readAtMs > readAtMs)) { + if ( + prLookupGitStateEpoch(cwd) !== epoch || + (latest !== undefined && latest.readAtMs > readAtMs) + ) { return value; } prLookupGitStateByKey.delete(entryKey); @@ -2673,6 +2685,7 @@ export const make = Effect.gen(function* () { yield* invalidateLocalStatusResultCache(cwd); yield* invalidateRemoteStatusResultCache(cwd); if (!options?.keepPullRequestLookups) yield* bumpPrLookupEpoch(cwd); + yield* bumpPrLookupGitStateEpoch(cwd); yield* forgetPrLookupGitState(cwd); }, ); From 69c7f0bc748b40a0873be9f1c1b619725a898c5d Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 17:21:25 +0800 Subject: [PATCH 6/8] test(server): the held-fetch test runs on more machines Co-Authored-By: Claude Opus 5.5 --- apps/server/src/vcs/GitVcsDriverCore.test.ts | 64 +++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/apps/server/src/vcs/GitVcsDriverCore.test.ts b/apps/server/src/vcs/GitVcsDriverCore.test.ts index 64eb9afcba3e1..c9e71c222c24b 100644 --- a/apps/server/src/vcs/GitVcsDriverCore.test.ts +++ b/apps/server/src/vcs/GitVcsDriverCore.test.ts @@ -3413,37 +3413,41 @@ it.layer(layerTest)("GitVcsDriver core integration", (it) => { }), ); - it.effect("does not count a status fetch that is still running after its wait times out", () => - Effect.gen(function* () { - const cwd = yield* makeTmpDir(); - const remote = yield* makeTmpDir("git-vcs-driver-remote-"); - const control = yield* makeTmpDir("git-vcs-driver-control-"); - const { initialBranch } = yield* initRepoWithCommit(cwd); - yield* git(remote, ["init", "--bare"]); - yield* git(cwd, ["remote", "add", "origin", remote]); - yield* git(cwd, ["push", "-u", "origin", initialBranch]); - - const started = NodePath.join(control, "started"); - const release = NodePath.join(control, "release"); - NodeChildProcess.execFileSync("mkfifo", [started, release]); - const uploadPack = NodePath.join(control, "upload-pack.sh"); - NodeFS.writeFileSync( - uploadPack, - `#!/bin/sh\necho started > "${started}"\ncat "${release}" > /dev/null\nexec git-upload-pack "$@"\n`, - { mode: 0o755 }, - ); - yield* git(cwd, ["config", "remote.origin.uploadpack", uploadPack]); - - const driver = yield* GitVcsDriver.GitVcsDriver; - const initial = yield* driver.statusUpstreamFetchCount(cwd); - const status = yield* driver.statusDetailsRemote(cwd).pipe(Effect.forkScoped); - yield* Effect.promise(() => NodeFS.promises.readFile(started)); - yield* TestClock.adjust("6 seconds"); - yield* Fiber.join(status); + // Needs mkfifo to hold the fetch open at a known point. + it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")( + "does not count a status fetch that is still running after its wait times out", + () => + Effect.gen(function* () { + const cwd = yield* makeTmpDir(); + const remote = yield* makeTmpDir("git-vcs-driver-remote-"); + const control = yield* makeTmpDir("git-vcs-driver-control-"); + const { initialBranch } = yield* initRepoWithCommit(cwd); + yield* git(remote, ["init", "--bare"]); + yield* git(cwd, ["remote", "add", "origin", remote]); + yield* git(cwd, ["push", "-u", "origin", initialBranch]); + + const started = NodePath.join(control, "started"); + const release = NodePath.join(control, "release"); + NodeChildProcess.execFileSync("mkfifo", [started, release]); + const uploadPack = NodePath.join(control, "upload-pack.sh"); + NodeFS.writeFileSync( + uploadPack, + `#!/bin/sh\necho started > "${started}"\ncat "${release}" > /dev/null\nexec git-upload-pack "$@"\n`, + { mode: 0o755 }, + ); + // Git runs this setting through a shell, and the temp path can contain spaces. + yield* git(cwd, ["config", "remote.origin.uploadpack", `'${uploadPack}'`]); - assert.equal(yield* driver.statusUpstreamFetchCount(cwd), initial); - yield* Effect.promise(() => NodeFS.promises.writeFile(release, "go")); - }), + const driver = yield* GitVcsDriver.GitVcsDriver; + const initial = yield* driver.statusUpstreamFetchCount(cwd); + const status = yield* driver.statusDetailsRemote(cwd).pipe(Effect.forkScoped); + yield* Effect.promise(() => NodeFS.promises.readFile(started)); + yield* TestClock.adjust("6 seconds"); + yield* Fiber.join(status); + + assert.equal(yield* driver.statusUpstreamFetchCount(cwd), initial); + yield* Effect.promise(() => NodeFS.promises.writeFile(release, "go")); + }), ); it.effect("does not start Git auto-maintenance from background upstream fetches", () => From 20369433f5cabb9020032760d10203232cd358da Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 17:41:15 +0800 Subject: [PATCH 7/8] test(server): share the in-flight git read test across invalidations Co-Authored-By: Claude Opus 5.5 --- apps/server/src/git/GitManager.test.ts | 58 +++----------------------- 1 file changed, 5 insertions(+), 53 deletions(-) diff --git a/apps/server/src/git/GitManager.test.ts b/apps/server/src/git/GitManager.test.ts index 09f919ecbc66a..34132bc844c7e 100644 --- a/apps/server/src/git/GitManager.test.ts +++ b/apps/server/src/git/GitManager.test.ts @@ -969,58 +969,10 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { }), ); - it.effect("a git read in flight during a T3 git action does not outlive it", () => - Effect.gen(function* () { - const repoDir = yield* makeTempDir("t3code-git-manager-"); - yield* initRepo(repoDir); - const remoteDir = yield* createBareRemote(); - yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]); - yield* runGit(repoDir, ["checkout", "-b", "feature/racing-upstream"]); - yield* runGit(repoDir, ["push", "-u", "origin", "feature/racing-upstream"]); - yield* runGit(repoDir, ["push", "origin", "HEAD:refs/heads/feature/raced-head"]); - const pullRequest = (number: number, headRefName: string): FakeChangeRequest[] => [ - { - number, - title: headRefName, - url: `https://github.com/pingdotgg/codething-mvp/pull/${number}`, - baseRefName: "main", - headRefName, - state: "open", - updatedAt: "2026-04-07T15:00:00Z", - }, - ]; - const gate = { - matches: (args: ReadonlyArray) => - args[0] === "for-each-ref" && args.at(-1) === "refs/heads/feature/racing-upstream", - reached: yield* Deferred.make(), - release: yield* Deferred.make(), - }; - const { manager } = yield* makeManager({ - gitSpawnGate: gate, - providerScenario: { - prListByHeadSelector: { - "feature/racing-upstream": pullRequest(237, "feature/racing-upstream"), - "feature/raced-head": pullRequest(238, "feature/raced-head"), - }, - }, - }); - const lookup = manager.branchPullRequest({ - cwd: repoDir, - branch: "feature/racing-upstream", - }); - - const inFlight = yield* Effect.forkChild(lookup); - yield* Deferred.await(gate.reached); - yield* runGit(repoDir, ["branch", "--set-upstream-to=origin/feature/raced-head"]); - yield* manager.invalidateStatus(repoDir); - yield* Deferred.succeed(gate.release, undefined); - yield* Fiber.join(inFlight); - - expect((yield* lookup)?.number).toBe(238); - }), - ); - - it.effect("a git read in flight during a checkout does not outlive it", () => + it.effect.each([ + { action: "a T3 git action", options: undefined }, + { action: "a checkout", options: { keepPullRequestLookups: true } }, + ])("a git read in flight during $action does not outlive it", ({ options }) => Effect.gen(function* () { const repoDir = yield* makeTempDir("t3code-git-manager-"); yield* initRepo(repoDir); @@ -1063,7 +1015,7 @@ it.layer(layerGitManagerTest)("GitManager", (it) => { const inFlight = yield* Effect.forkChild(lookup); yield* Deferred.await(gate.reached); yield* runGit(repoDir, ["branch", "--set-upstream-to=origin/feature/raced-head"]); - yield* manager.invalidateStatus(repoDir, { keepPullRequestLookups: true }); + yield* manager.invalidateStatus(repoDir, options); yield* Deferred.succeed(gate.release, undefined); yield* Fiber.join(inFlight); From 2a1504888b5ebbf821e8e0d3a73442b7778f2106 Mon Sep 17 00:00:00 2001 From: Yuruzuu Date: Sun, 1 Nov 2026 21:56:22 +0800 Subject: [PATCH 8/8] fix(server): a fetch ending during a checkout's first read updates totals The fetch count was taken after the first local read, and after waiting for the remote lock. A fetch finishing in that gap was missed, leaving Changes totals on the old base until the next refresh. Co-Authored-By: Claude Opus 5.5 --- .../src/vcs/VcsStatusBroadcaster.test.ts | 29 +++++++++++++++++-- apps/server/src/vcs/VcsStatusBroadcaster.ts | 7 +++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts index 56fd2fbf6c023..c1ec6dad69afc 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.test.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.test.ts @@ -762,12 +762,14 @@ describe("VcsStatusBroadcaster", () => { const subscribed = yield* Deferred.make(); const firstRemote = yield* Deferred.make(); const secondRemote = yield* Deferred.make(); + const thirdRemote = yield* Deferred.make(); yield* broadcaster.streamStatus({ cwd: "/repo", includeRemote: false }).pipe( Stream.runForEach((event) => { if (event._tag === "snapshot") return Deferred.succeed(subscribed, undefined); if (event._tag !== "remoteUpdated" || event.remote === null) return Effect.void; - return event.remote.pr === null - ? Deferred.succeed(firstRemote, undefined) + if (event.remote.pr === null) return Deferred.succeed(firstRemote, undefined); + return event.remote.pr.number === 2979 + ? Deferred.succeed(thirdRemote, undefined) : Deferred.succeed(secondRemote, undefined); }), Effect.forkScoped, @@ -796,6 +798,29 @@ describe("VcsStatusBroadcaster", () => { (yield* broadcaster.getStatus({ cwd: "/repo" })).branchChanges, fetchedLocal.branchChanges, ); + + // A fetch finishing after the first local read took its values still needs a re-read. + const lateFetchLocal: VcsStatusLocalResult = { + ...baseLocalStatus, + branchChanges: { baseRef: "origin/main", insertions: 7, deletions: 0 }, + }; + state.beforeRemoteStatus = Effect.void; + state.currentRemoteStatus = { + ...remoteStatusWithPr, + pr: remoteStatusWithPr.pr && { ...remoteStatusWithPr.pr, number: 2979 }, + }; + state.beforeLocalStatus = Effect.sync(() => { + state.beforeLocalStatus = Effect.void; + state.upstreamFetchCount = (state.upstreamFetchCount ?? 0) + 1; + state.currentLocalStatus = lateFetchLocal; + }); + yield* broadcaster.refreshAfterCheckout("/repo"); + yield* Deferred.await(thirdRemote); + + assert.deepStrictEqual( + (yield* broadcaster.getStatus({ cwd: "/repo" })).branchChanges, + lateFetchLocal.branchChanges, + ); }).pipe(Effect.provide(layerTestFor(state)), Effect.scoped); }); diff --git a/apps/server/src/vcs/VcsStatusBroadcaster.ts b/apps/server/src/vcs/VcsStatusBroadcaster.ts index 74cc54f45268e..f3d8018a56cba 100644 --- a/apps/server/src/vcs/VcsStatusBroadcaster.ts +++ b/apps/server/src/vcs/VcsStatusBroadcaster.ts @@ -548,13 +548,12 @@ export const make = Effect.gen(function* () { }); const refreshRemoteAfterCheckout = Effect.fn("VcsStatusBroadcaster.refreshRemoteAfterCheckout")( - function* (cwd: string, policyCwd: string) { + function* (cwd: string, policyCwd: string, fetchesBefore: number) { yield* withRemoteWriteLock( cwd, Effect.gen(function* () { const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval); yield* workflow.invalidateStatus(cwd, { keepPullRequestLookups: true }); - const fetchesBefore = yield* workflow.statusUpstreamFetchCount(cwd); const remoteRead = yield* readRemoteStatus(cwd, { refreshUpstream }); // Only a fetch can move the base used for local Changes totals. if ((yield* workflow.statusUpstreamFetchCount(cwd)) !== fetchesBefore) { @@ -578,11 +577,13 @@ export const make = Effect.gen(function* () { "VcsStatusBroadcaster.refreshAfterCheckout", )(function* (rawCwd) { const cwd = yield* withFileSystem(normalizeCwd(rawCwd)); + // Taken before the local read, so a fetch that finishes after it still triggers a re-read. + const fetchesBefore = yield* workflow.statusUpstreamFetchCount(cwd); const localDone = yield* Deferred.make(); // 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(refreshRemoteAfterCheckout(cwd, rawCwd)), + Effect.andThen(refreshRemoteAfterCheckout(cwd, rawCwd, fetchesBefore)), Effect.ignoreCause({ log: true }), Effect.forkIn(broadcasterScope), );