perf(server,web): branch switches stop flipping back and redoing PR lookups

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",
     () =>

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>;
@@ -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);
     },
   );

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

@@ -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();

apps/server/src/vcs/GitVcsDriverCore.ts

@@ -1621,10 +1621,20 @@ export const makeGitVcsDriverCore = Effect.fn("makeGitVcsDriverCore")(function*
       }
     }
   });
+  const statusUpstreamFetches = new Map<string, number>();
   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));

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<boolean | undefined>;
+  remoteStatusInteractiveValues?: Array<boolean>;
+  invalidateStatusKeepLookupValues?: Array<boolean | undefined>;
+  upstreamFetchCount?: number;
   remoteStatusError?: GitManagerError;
   backgroundWorkEnabled?: boolean;
   automaticFetchInterval?: Effect.Effect<Duration.Duration>;
@@ -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 } : {}),
       }),
     ),
@@ -656,6 +669,136 @@ describe("VcsStatusBroadcaster", () => {
     },
   );
 
+  it.effect("publishes a checkout's branch before returning, ahead of the remote read", () => {
+    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;
+      yield* broadcaster.getStatus({ cwd: "/repo" });
+      const subscribed = yield* Deferred.make<void>();
+      const localPublished = yield* Deferred.make<VcsStatusLocalResult>();
+      const remotePublished = yield* Deferred.make<VcsStatusRemoteResult | null>();
+      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<void>();
+      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("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>();
+      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<typeof layerTestFor>[0] = {
       currentLocalStatus: baseLocalStatus,