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

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

@@ -969,7 +969,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);
@@ -1012,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);
+      yield* manager.invalidateStatus(repoDir, options);
       yield* Deferred.succeed(gate.release, undefined);
       yield* Fiber.join(inFlight);
 
@@ -2433,6 +2436,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

@@ -140,7 +140,11 @@ export class GitManager extends Context.Service<
     ) => Effect.Effect<GitBranchPullRequest | null, GitManagerServiceError>;
     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>;
@@ -1151,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<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: {
@@ -2403,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);
@@ -2665,10 +2681,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

@@ -59,7 +59,8 @@ export class GitWorkflowService extends Context.Service<
     ) => Effect.Effect<VcsStatusRemoteResult | null, GitManagerServiceError>;
     readonly invalidateLocalStatus: (cwd: string) => Effect.Effect<void, never>;
     readonly invalidateRemoteStatus: (cwd: string) => Effect.Effect<void, never>;
-    readonly invalidateStatus: (cwd: string) => Effect.Effect<void, never>;
+    readonly invalidateStatus: GitManager.GitManager["Service"]["invalidateStatus"];
+    readonly statusUpstreamFetchCount: GitVcsDriver.GitVcsDriver["Service"]["statusUpstreamFetchCount"];
     readonly pullCurrentBranch: (cwd: string) => Effect.Effect<VcsPullResult, GitCommandError>;
     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)),

apps/server/src/vcs/GitVcsDriver.ts

@@ -310,6 +310,8 @@ export class GitVcsDriver extends Context.Service<
       cwd: string,
       options?: GitRemoteStatusOptions,
     ) => Effect.Effect<GitRemoteStatusDetails, GitCommandError>;
+    /** Changes whenever a status read fetches this repository, so callers can tell if refs may have moved. */
+    readonly statusUpstreamFetchCount: (cwd: string) => Effect.Effect<number>;
     readonly prepareCommitContext: (
       cwd: string,
       filePaths?: readonly string[],

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";
@@ -3386,6 +3388,68 @@ 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);
+      }),
+    );
+
+    // 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}'`]);
+
+          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();

apps/server/src/vcs/GitVcsDriverCore.ts

@@ -1326,6 +1326,7 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
     );
   });
 
+  const statusUpstreamFetches = new Map<string, number>();
   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 }),
@@ -1640,6 +1647,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 +4950,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));