perf(server): PR status comes back sooner after a branch switch

apps/server/src/git/GitManager.test.ts

@@ -1054,7 +1054,10 @@ it.layer(layerGitManagerTest)("GitManager", (it) => {
     }),
   );
 
-  it.effect("a git read in flight during a T3 git action 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);
@@ -1097,7 +1100,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);
+      yield* manager.invalidateStatus(repoDir, options);
       yield* Deferred.succeed(gate.release, undefined);
       yield* Fiber.join(inFlight);
 
@@ -3015,6 +3018,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",
     () =>

apps/server/src/git/GitManager.ts

@@ -164,7 +164,11 @@ export class GitManager extends Context.Service<
     }) => Effect.Effect<boolean>;
     readonly invalidateLocalStatus: (cwd: string) => Effect.Effect<void, never>;
     readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect<void, never>;
-    readonly invalidateStatus: (cwd: string) => Effect.Effect<void, never>;
+    /** keepPullRequestLookups is for changes that cannot affect a PR, such as a checkout. */
+    readonly invalidateStatus: (
+      cwd: string,
+      options?: { readonly keepPullRequestLookups?: boolean },
+    ) => Effect.Effect<void, never>;
     readonly resolvePullRequest: (
       input: GitPullRequestRefInput,
     ) => Effect.Effect<GitResolvePullRequestResult, GitManagerServiceError>;
@@ -1257,6 +1261,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<string, number>();
+  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: {
@@ -2621,10 +2634,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);
@@ -2964,10 +2980,11 @@ 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* bumpPrLookupGitStateEpoch(cwd);
       yield* forgetPrLookupGitState(cwd);
     },
   );

apps/server/src/git/GitWorkflowService.ts

@@ -69,7 +69,7 @@ export class GitWorkflowService extends Context.Service<
     readonly invalidateLocalStatus: (cwd: string) => Effect.Effect<void, never>;
     readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect<void, never>;
     readonly readRefSnapshot: (cwd: string) => Effect.Effect<string | null>;
-    readonly invalidateStatus: (cwd: string) => Effect.Effect<void, never>;
+    readonly invalidateStatus: GitManager.GitManager["Service"]["invalidateStatus"];
     /** The checkout's own HEAD file, inside a linked worktree's admin directory when there is one. */
     readonly headPath: (cwd: string) => Effect.Effect<string | null, never>;
     readonly pullCurrentBranch: (cwd: string) => Effect.Effect<VcsPullResult, GitCommandError>;

apps/server/src/vcs/VcsStatusBroadcaster.test.ts

@@ -26,6 +26,7 @@ import type {
   VcsStatusStreamEvent,
 } from "@t3tools/contracts";
 import { GitCommandError, GitManagerError } from "@t3tools/contracts";
+import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit";
 import { applyGitStatusStreamEvent } from "@t3tools/shared/git";
 
 import * as VcsAutomaticFetchInterval from "./VcsAutomaticFetchInterval.ts";
@@ -78,6 +79,8 @@ function layerTestFor(state: {
   localInvalidationCalls: number;
   remoteInvalidationCalls: number;
   remoteStatusRefreshUpstreamValues?: Array<boolean | undefined>;
+  remoteStatusInteractiveValues?: Array<boolean>;
+  invalidateStatusKeepLookupValues?: Array<boolean | undefined>;
   localStatusBranchChanges?: Array<boolean>;
   refSnapshot?: string | null;
   remoteStatusError?: GitManagerError;
@@ -163,6 +166,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,
               ),
@@ -176,10 +186,11 @@ 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);
           }),
         ...(state.pullCurrentBranch ? { pullCurrentBranch: state.pullCurrentBranch } : {}),
         headPath: () =>
@@ -1124,6 +1135,118 @@ 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<typeof layerTestFor>[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<void>();
+      const subscribed = yield* Deferred.make<void>();
+      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<typeof layerTestFor>[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<void>();
+      const firstRemote = yield* Deferred.make<void>();
+      const secondRemote = yield* Deferred.make<void>();
+      const thirdRemote = yield* Deferred.make<void>();
+      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;
+          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,
+      );
+      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.refSnapshot = "fetched";
+        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,
+      );
+
+      // 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.refSnapshot = "fetched late";
+        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);
+  });
+
   it.effect("a remote poll refreshes Changes totals when divergence counts stay the same", () => {
     const state: Parameters<typeof layerTestFor>[0] = {
       currentLocalStatus: {

apps/server/src/vcs/VcsStatusBroadcaster.ts

@@ -25,6 +25,7 @@ import type {
   VcsStatusSubscriptionInput,
 } from "@t3tools/contracts";
 import { DEFAULT_AUTOMATIC_GIT_FETCH_INTERVAL } from "@t3tools/contracts";
+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";
@@ -687,15 +688,46 @@ export const make = Effect.gen(function* () {
     return yield* refreshRemoteAndLocalTotals(cwd, rawCwd, refsBeforeLocalRead);
   }, Effect.scoped);
 
+  const refreshRemoteAfterCheckout = Effect.fn("VcsStatusBroadcaster.refreshRemoteAfterCheckout")(
+    function* (cwd: string, policyCwd: string, refsBeforeLocalRead: string | null) {
+      yield* withRemoteWriteLock(
+        cwd,
+        Effect.gen(function* () {
+          const refreshUpstream = !Duration.isZero(yield* automaticFetchInterval);
+          yield* workflow.invalidateStatus(cwd, { keepPullRequestLookups: true });
+          const remoteRead = yield* readRemoteStatus(cwd, { refreshUpstream });
+          // 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);
+          }
+          // 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) {
     const cwd = yield* withFileSystem(normalizeCwd(rawCwd));
+    // Taken before the local read, so a fetch that finishes after it still triggers a re-read.
+    const refsBeforeLocalRead = yield* workflow.readRefSnapshot(cwd);
     const localDone = yield* Deferred.make<void>();
     // 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, null)),
+      Effect.andThen(refreshRemoteAfterCheckout(cwd, rawCwd, refsBeforeLocalRead)),
       Effect.ignoreCause({ log: true }),
       Effect.forkIn(broadcasterScope),
     );