fix(server): update branch labels after terminal checkouts#23422

open13
main
fix/issue-7409
opened3d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 3d ago
Selected thread: apps/server/src/vcs/VcsStatusBroadcaster.ts:535 — <!-- MURMUR_IGNORE --> 🟠 **High** `vcs/VcsStatusBroadcaster.ts:535` When `localStatus` fails for the displayed branch, this fallback caches the new remote counts without refreshing the cached local status. Later polls see `branchShown && !countsMoved` and skip the local read, so local totals can remain stale even after the transient error clears. Preserve a pending local refresh or otherwise retry the read after this fallback. No longer relevant as of 2a04d5e1cabf9c9f9af28f06b460b8422e6932aa <!-- macroscope-meta: {"kind":"code_review"} -->
0 / 6 viewed6 files changedLine totals unavailable

apps/server/src/vcs/VcsStatusBroadcaster.ts · Earlier revision c8075386 · right 535

Original diff
@@ -484,23 +495,55 @@ export const make = Effect.gen(function* () {
         if (options?.refreshUpstream !== false) {
           yield* workflow.invalidateRemoteStatus(cwd);
         }
-        const previousRemote = (yield* getCachedStatus(cwd))?.remote?.value;
-        const remote = yield* workflow.remoteStatus({ cwd }, options);
-        const pulled =
-          options?.refreshUpstream === false
-            ? null
-            : yield* maybeAutoPull(cwd, remote, options?.policyCwds ?? [cwd]);
-        if (pulled !== null) return pulled.remote;
-        if (
-          remote &&
-          (!previousRemote ||
+        // A checkout outside T3 Code can land between these reads, and the remote result does
+        // not name its branch. Reading the branch around it keeps one branch's PR and counts
+        // from reaching clients beside another branch's name.
+        for (let attempt = 1; attempt <= CHECKOUT_RACE_ATTEMPTS; attempt += 1) {
+          // The first read may hold the previous branch's result in the remote cache.
+          if (attempt > 1) yield* workflow.invalidateRemoteStatus(cwd);
+          const branch = yield* workflow.checkedOutBranch(cwd);
+          const remote = yield* workflow.remoteStatus(
+            { cwd },
+            attempt === 1 ? options : { ...options, refreshUpstream: false },
+          );
+          if ((yield* workflow.checkedOutBranch(cwd)) !== branch) continue;
+          const pulled =
+            options?.refreshUpstream === false
+              ? null
+              : yield* maybeAutoPull(cwd, remote, options?.policyCwds ?? [cwd]);
+          if (pulled !== null) return pulled.remote;
+          if (remote === null) {
+            return yield* updateCachedRemoteStatus(cwd, remote, { publish: true });
+          }
+          const cached = yield* getCachedStatus(cwd);
+          const previousRemote = cached?.remote?.value;
+          const branchShown = cached?.local?.value.refName === branch;
+          const countsMoved =
+            !previousRemote ||
             previousRemote.aheadCount !== remote.aheadCount ||
             previousRemote.behindCount !== remote.behindCount ||
-            previousRemote.aheadOfDefaultCount !== remote.aheadOfDefaultCount)
-        ) {
-          yield* refreshLocalStatusCore(cwd);
+            previousRemote.aheadOfDefaultCount !== remote.aheadOfDefaultCount;
+          if (branchShown && !countsMoved) {
+            return yield* updateCachedRemoteStatus(cwd, remote, { publish: true });
+          }
+          yield* workflow.invalidateLocalStatus(cwd);
+          const local = yield* workflow.localStatus({ cwd }).pipe(
+            // A commit in an editor can hold index.lock. The remote still belongs to the
+            // branch clients show, but must not go out under the name of a branch they don't.
+            Effect.catch((error) => (branchShown ? Effect.succeed(null) : Effect.fail(error))),
+          );
+          if (local === null) {

scripts/fixtures/issue-7409/seed.py · Earlier revision c8075386 · right 26

Original diff
@@ -0,0 +1,98 @@
+#!/usr/bin/env python3
+"""Create synthetic Git repositories and optional clean T3 projection fixtures.
+
+Run with a NEW --root. For --state, stop the isolated server first. The schema
+is read from --schema-from; only migration bookkeeping is copied. No user data
+or credentials are copied. The original database must never be the destination.
+Example: python3 seed.py --root /tmp/checkout-proof --schema-from /tmp/schema.sqlite \
+    --state /tmp/isolated/userdata/statev2.sqlite
+Reset captures with git -C <root>/regular switch main (and similarly linked).
+"""
+import argparse
+import json
+import os
+from pathlib import Path
+import sqlite3
+import subprocess
+
+parser = argparse.ArgumentParser(description=__doc__)
+parser.add_argument('--root', type=Path, required=True)
+parser.add_argument('--schema-from', type=Path)
+parser.add_argument('--state', type=Path)
+args = parser.parse_args()
+data = json.loads(Path(__file__).with_name('data.json').read_text())
+root = args.root.resolve()
+root.mkdir(parents=True, exist_ok=False)
+env = dict(os.environ, GIT_CONFIG_NOSYSTEM='1', GIT_CONFIG_GLOBAL='/dev/null',

apps/server/src/vcs/VcsStatusBroadcaster.ts · Earlier revision 58e9f471 · right 788

Original diff
@@ -722,11 +760,92 @@ export const make = Effect.gen(function* () {
     }
   });
 
+  // A checkout outside T3 Code, such as `git switch` in the terminal, calls no refresh RPC.
+  // Rereading remote too, without a fetch, sends the new branch with its own PR and counts.
+  const refreshCheckedOutRef = Effect.fn("VcsStatusBroadcaster.refreshCheckedOutRef")(function* (
+    cwd: string,
+  ) {
+    if ((yield* getCachedStatus(cwd))?.remote) {
+      const remote = yield* refreshRemoteStatus(cwd, { refreshUpstream: false });
+      if (remote) return;
+    }
+    yield* refreshLocalStatusCore(cwd);
+  });
+
+  // Runs for the subscription lifetime, so tracing would leave a span open indefinitely.
+  const watchHead = Effect.fnUntraced(function* (cwd: string, ready: Deferred.Deferred<void>) {
+    const headPath = yield* workflow.headPath(cwd);
+    if (headPath === null) return;
+    // Git replaces HEAD by renaming HEAD.lock, which a watch on the file itself would lose.
+    yield* fs
+      .watch(path.dirname(headPath), {
+        onReady: () => Deferred.doneUnsafe(ready, Exit.void),
+      })
+      .pipe(
+        Stream.filter((event) => path.basename(event.path) === "HEAD"),
+        // A rebase moves HEAD many times; refresh once for whatever arrived during a refresh.
+        Stream.buffer({ capacity: 1, strategy: "sliding" }),
+        Stream.runForEach(() => refreshCheckedOutRef(cwd).pipe(Effect.ignoreCause({ log: true }))),

apps/server/src/vcs/VcsStatusBroadcaster.ts · Earlier revision bd7fbe65 · right 797

Original diff
@@ -722,11 +766,98 @@ export const make = Effect.gen(function* () {
     }
   });
 
+  // A checkout outside T3 Code, such as `git switch` in the terminal, calls no refresh RPC.
+  // Rereading remote too, without a fetch, sends the new branch with its own PR and counts.
+  const refreshCheckedOutRef = Effect.fn("VcsStatusBroadcaster.refreshCheckedOutRef")(function* (
+    cwd: string,
+  ) {
+    if ((yield* getCachedStatus(cwd))?.remote) {
+      const remote = yield* refreshRemoteStatus(cwd, { refreshUpstream: false });
+      if (remote) return;
+    }
+    yield* refreshLocalStatusCore(cwd);
+  });
+
+  // Runs for the subscription lifetime, so tracing would leave a span open indefinitely.
+  const watchHead = Effect.fnUntraced(function* (cwd: string, ready: Deferred.Deferred<void>) {
+    const headPath = yield* workflow.headPath(cwd);
+    if (headPath === null) return;
+    // Git replaces HEAD by renaming HEAD.lock, which a watch on the file itself would lose.
+    yield* fs
+      .watch(path.dirname(headPath), {
+        onReady: () => Deferred.doneUnsafe(ready, Exit.void),
+      })
+      .pipe(
+        Stream.filter((event) => path.basename(event.path) === "HEAD"),
+        // A rebase moves HEAD many times; refresh once for whatever arrived during a refresh.
+        Stream.buffer({ capacity: 1, strategy: "sliding" }),
+        Stream.runForEach(() =>
+          refreshCheckedOutRef(cwd).pipe(
+            Effect.retry(CHECKOUT_REFRESH_RETRY),
+            Effect.ignoreCause({ log: true }),