[juliusmarminge]

Problem

Opening the branch menu sends vcs.refreshStatus with localOnly: true. When no remote status was cached, or the branch had changed, that refresh cached an empty remote entry ({ value: null }) and published remoteUpdated: null. Every reader takes that entry as "the remote was read and has no PR":

• Overlap: if the subscription's first remote read is still running, the refresh overwrites the PR, upstream and ahead/behind counts that read just cached. They stay empty until the next poll, or until a full refresh when automatic fetch is set to 0.
• Before any remote read: the turn-end refreshPullRequestStatus returns without reading, because it skips when cached.remote?.value === null.
• After a branch switch: the turn-end PR refresh skips in the same way. A subscription that opens afterwards finds a cached remote and waits a full interval before its first read.

Change

The local-only path of VcsStatusBroadcaster.refreshStatus now refreshes local status only and returns the cached parts. It no longer writes a remote entry, invalidates the remote cache, or takes the remote write lock.

• Branch switch: updateCachedLocalStatus already drops the old branch's remote and publishes remoteUpdated: null.
• Invalidation: readRemoteStatus already invalidates the remote cache before every read.

The only caller (the branch menu) ignores the RPC result and gets status from the stream.

Two existing tests used a fake remote that ignores the branch and relied on the empty entry. Their fakes now return the new branch's remote, and their call counts include the remote read getStatus now makes for the new branch. Their assertions are otherwise unchanged.

Scope and approval

Fixes #33256

Verification

• vp test run apps/server/src/vcs/VcsStatusBroadcaster.test.ts: 29/29 pass. Three new tests reproduce the three cases above and fail on main's code:
  • a local-only refresh that overlaps the first remote poll keeps its PR: expected null to deeply equal { number: 2978, … }
  • a local-only refresh before any remote read leaves the PR refresh able to read: expected undefined to deeply equal { number: 2978, … }
  • a local-only refresh after a branch switch leaves the PR refresh able to read: expected undefined to deeply equal { number: 3001, … }
• vp run check:changed on 456327574c: format, lint, knip, typecheck (3 packages) and the 2 changed test files (31 tests) pass.

Evidence

Server-only change, so the evidence is real-server event logs rather than screenshots. Each row ran each side's own apps/server/src/bin.ts serve on a fresh base dir (T3CODE_DEV_SEED=0), driven over the WebSocket RPC with the same driver and a synthetic repo with a local bare remote. Turn-end rows use the Replay (dev) provider. Overlap rows hold the remote git fetch open with a gate. P1 uses a read-only shallow checkout of this PR's branch, so the server's PR lookup finds the real open PR. Before = main 4c361199f4, after = 456327574c. R1 was recorded at c6ca605630 / 33cb900503, and the fix is unchanged since.

Row	Scenario	Before 4c361199f4	After 456327574c	Logs
R1	Cold local-only refresh, then first subscription (30 s interval)	no remote read within 8 s	upstream/ahead 0.45 s later	transcript below
R2	Cold local-only refresh, then a turn ends; automatic fetch 0	no remote read after the turn; remote stays null	turn-end refresh reads; upstream/ahead published	before · after
R3	Warm cache, same branch (control)	remote kept	remote kept (unchanged)	before · after
R4	Branch A → B, local-only refresh, new subscription	two clears, no B read in window	one clear, B's remote read immediately	before · after
R5	Branch A → B, local-only refresh, turn ends; fetch 0	turn-end refresh skips; B stays null	B's remote recovered after the turn	before · after
R6	First remote read in flight, local-only refresh overlaps	remote published, then erased to null	remote published and kept	before · after
R7	Branch switch while a remote read is in flight	stale A read discarded; null cached for B, no B read in window	stale A read discarded; B read immediately	before · after
R8	Cold local-only refresh, new subscription; fetch 0	remote stays null	remote read without a fetch	before · after
R9	Non-repository directory (control)	null	null (unchanged)	before · after
P1	R6 ordering on a checkout of this PR's branch with a real GitHub remote	open PR https://git.cafe/pingdotgg/t3code/pulls/33263 published, then erased	open PR https://git.cafe/pingdotgg/t3code/pulls/33263 kept	before · after

Environment: Linux 6.8 x64, Node 26; automatic fetch interval 30 s, or 0 s for R2/R5/R8. Index · pass-1 matrix (its "missing" rows were captured afterwards; the index is current) · scripts: capture.ts, capture-hosted.ts, seed.mjs, seed-hosted.mjs, git-wrapper.py, run-matrix.py, R1 driver.

What else changed: the overlapping local-only refresh no longer waits for the remote writer. Branch switches publish one clear instead of two. The immediate refreshStatus result still carries default remote fields while a remote read is in flight, the same as before; the only caller ignores it. Controls R3 and R9 show no change.

Not checked: the web toolbar's PR badge in a browser. P1 shows the real PR payload surviving over the RPC the toolbar subscribes to.

R1 transcript (before c6ca605630, after 33cb900503):

Before (c6ca605)

[+  147ms] seeded repo: ## feature/demo...origin/feature/demo [ahead 1]
[+  165ms] server pid 1070005 checkout c6ca605630
[+ 7814ms] -> vcs.refreshStatus { localOnly: true }   (branch menu opened)
[+ 8160ms] <- refreshStatus result: branch feature/demo {"hasUpstream":false,"aheadCount":0,"behindCount":0,"pr":null}
[+ 8160ms] -> subscribeVcsStatus (branch toolbar mounts), watching 8 s
[+ 8198ms] <- snapshot  remote: null
[+16171ms] done watching

After (33cb900)

[+  144ms] seeded repo: ## feature/demo...origin/feature/demo [ahead 1]
[+  158ms] server pid 1073114 checkout 33cb900503
[+ 8220ms] -> vcs.refreshStatus { localOnly: true }   (branch menu opened)
[+ 8598ms] <- refreshStatus result: branch feature/demo {"hasUpstream":false,"aheadCount":0,"behindCount":0,"pr":null}
[+ 8598ms] -> subscribeVcsStatus (branch toolbar mounts), watching 8 s
[+ 8624ms] <- snapshot  remote: null
[+ 9049ms] <- remoteUpdated: {"hasUpstream":true,"aheadCount":1,"behindCount":0,"pr":null}
[+16608ms] done watching

Verification log

• Main merged into the branch with no conflicts (head 456327574c); the fix diff is unchanged.
• Review round 1: A found no code bug and asked for runtime evidence of the overlap, turn-end and branch-switch paths with a real PR (captured above as R2–R9 and P1); B: merge.
• Review round 2, with the evidence: A merge, B merge. No open findings.
• Proof audit: pass 1 insufficient (R2, R4–R8 missing); pass 2 sufficient on all 10 rows, with R6 rebuilt independently from the uploaded scripts (32/32 events match).
• Tests: 29/29 in VcsStatusBroadcaster.test.ts; the 3 new tests fail on main's broadcaster.
• CI: all checks green on 456327574c.
• Unverified surfaces: none.

AI-assisted; verified with the tests and tools listed above.

[macroscopeapp]

Approvability

Verdict: Approved at 0b75603

Macroscope's review found this PR approvable — Local-only refreshes stop recording a synthetic no-PR result, preventing branch-menu interactions from wiping PR status. Existing branch-change cache clearing remains in place, and the change is limited to transient status handling.

No code changes detected at 4563275. Prior analysis still applies.

^{You can add or adjust custom eligibility rules. Learn more.}

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	21.5 KiB	21.5 KiB	0 B (0.0%)	24.4 KiB	✅
Codex	Live turn messages	1	1	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.9 KiB	21.9 KiB	0 B (0.0%)	24.4 KiB	✅
Claude	Live turn messages	1	1	0 (0.0%)	8	✅

Baseline: 4c36119 · PR result: 4563275 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 108.5 KiB
• Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

refreshStatus now refreshes local status before handling local-only requests. Those requests return merged cached status without invalidating or writing remote status. Tests cover overlapping remote polls, later PR refreshes, and branch switches.

Changes: Local-only status refresh

Layer / File(s)	Summary
Refresh behavior and validation apps/server/src/vcs/VcsStatusBroadcaster.ts, apps/server/src/vcs/VcsStatusBroadcaster.test.ts	Local-only refreshes return merged cached status without invalidating or writing remote status. Tests check PR preservation during overlapping remote reads and verify later remote reads after local-only refreshes and branch switches.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• pingdotgg/t3code#22807: Introduced the localOnly option and the branch-change behavior that this refresh path uses.
• pingdotgg/t3code#32054: Changes the same localOnly handling and local-refresh ordering in refreshStatus.

Suggested labels: bugfix

Suggested reviewers: justyannicc, macroscopeapp

Merge Risk: Minimal · up to 0b756

Local-only refreshes preserve cached remote status, and an initially empty remote result is accepted by the response contract. No merge-blocking issue was identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title states the main change: opening the branch menu no longer clears PR status. It follows the conventional commit format and matches the raw summary, which removes the remote-entry write for lo…
Linked Issues check		Linked issue #33256 is the only direct target. Its coding requirements are: (1) a local-only refresh must not write a { value: null } remote entry; (2) it must not remove a remote status cached by a…
Out of Scope Changes check		The whole-PR diff (merge base c73b38b to head 0b75603) changes two files. In VcsStatusBroadcaster.ts, the diff removes the local-only remote write, invalidation, and lock, which is the fix for #33…
Description check		The description covers the problem, change, linked issue, and focused verification results. It also states what was not checked. The required sections are present and mostly complete.

Full details: Title check

Explanation

The title states the main change: opening the branch menu no longer clears PR status. It follows the conventional commit format and matches the raw summary, which removes the remote-entry write for local-only refreshes. The title is specific and does not use vague terms.

Full details: Linked Issues check

Explanation

Linked issue #33256 is the only direct target. Its coding requirements are: (1) a local-only refresh must not write a { value: null } remote entry; (2) it must not remove a remote status cached by an overlapping first remote read; (3) refreshPullRequestStatus must still read after a local-only refresh, both before any remote read and after a branch switch; (4) regression tests in VcsStatusBroadcaster.test.ts.

Evidence at head 0b75603:

• Req 1 and 2: The local-only branch of refreshStatus now calls refreshLocalStatusCore, then returns mergeGitStatusParts of getStatusParts. It no longer writes a remote entry, calls invalidateRemoteStatus, or takes withRemoteWriteLock. An in-flight read keeps its cached PR and upstream.
• Req 3, branch switch: updateCachedLocalStatus drops a remote whose branch differs from the new local.refName, and publishes remoteUpdated: null when publish is set. getStatusParts returns remote: null when no remote is cached, so getStatus and the PR refresh path read the remote for the new branch.
• Req 3, before any remote read: with no cached remote, the cache holds no { value: null } entry, so the skip condition described in the issue (cached.remote?.value === null) does not hold. The read proceeds.
• Req 4: Three new tests cover overlap, before-first-read, and branch-switch cases, matching the issue's cases. Existing fakes that ignored the branch were adjusted, and their call counts now include the read for the new branch.

Unresolved uncertainty: the truncated read of VcsStatusBroadcaster.ts did not show readRemoteStatus or the refreshPullRequestStatus body. The invalidation-before-read behavior and the skip condition rest on the issue text and the test changes (remoteInvalidationCalls drops from 2 to 1 only because the removed local-only invalidation was redundant). The PR's reported test run was not re-executed here.

No poller readNow is added. An already-open subscription after a branch switch still waits for its normal poll interval. This matches main for that case and is not a requirement stated for open subscriptions in #33256. The issue's requirement for subscriptions opened after the switch is met by getStatus reading when no remote is cached.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff (merge base c73b38b to head 0b75603) changes two files. In VcsStatusBroadcaster.ts, the diff removes the local-only remote write, invalidation, and lock, which is the fix for #33256. In VcsStatusBroadcaster.test.ts, it adds three regression tests for #33256 and adjusts call-count assertions and two fakes. The fake and count adjustments follow directly from removing the null entry: the removed local-only invalidation accounts for the change from 2 to 1 invalidations, and the added getStatus read accounts for the extra remote read. The non-local-only path is unchanged. No unrelated code, configuration, or dependency change appears in the diff. The PR description's runtime transcript, driver script, and verification notes are not code changes in the diff.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: fix/issue-33256
• File: .coderabbit.config.ts
• Commit: 0b75603
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Team
• Run ID: 9a61663e-404e-472e-af13-95e38a1a7854

Commits

Reviewing files that changed from the base of the PR and between c73b38b and 0b75603.

Files selected for processing: 2

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

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.