Preserve pull request status across branch checkouts - #5407
Preserve pull request status across branch checkouts#5407juliusmarminge wants to merge 2 commits into
Conversation
- Add branch-keyed PR lookup RPC and client query - Serialize VCS refreshes to prevent stale status races
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces a new feature for preserving PR status across branch checkouts, including a new RPC endpoint, concurrency controls, and caching changes across multiple layers. Two open review comments identify potential race conditions and badge flickering bugs in the new logic that warrant human review. You can customize Macroscope's approvability policy. Learn more. |
- Reconcile VCS refreshes after mid-refresh checkouts - Use branch-keyed PR lookups for thread indicators - Keep git actions tied to the active checkout
| // When the status describes the thread's own branch it is authoritative | ||
| // about that branch's PR — including when it reports none, so this must not | ||
| // fall through to a warm branch lookup that would resurrect a stale badge. | ||
| if (status !== null && thread.branch !== null && status.refName === thread.branch) { |
There was a problem hiding this comment.
🟡 Medium state/use-thread-pr.ts:49
When the VCS status stream reports a matching refName with pr: null before the remote PR lookup resolves, useThreadPr returns null instead of falling through to the already-subscribed branchPr result. This causes the PR badge to flicker off immediately after checkout and can leave it permanently absent if that remote refresh later fails. The guard at line 49 treats any matching status as authoritative, including the transient state where the PR field is still pending, so the branch-keyed fallback is never reached. Consider distinguishing a status that has confirmed no PR from one whose PR field is still pending before short-circuiting.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/mobile/src/state/use-thread-pr.ts around line 49:
When the VCS status stream reports a matching `refName` with `pr: null` before the remote PR lookup resolves, `useThreadPr` returns `null` instead of falling through to the already-subscribed `branchPr` result. This causes the PR badge to flicker off immediately after checkout and can leave it permanently absent if that remote refresh later fails. The guard at line 49 treats any matching status as authoritative, including the transient state where the PR field is still pending, so the branch-keyed fallback is never reached. Consider distinguishing a status that has confirmed no PR from one whose PR field is still pending before short-circuiting.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON, but a cloud agent failed to start.
Reviewed by Cursor Bugbot for commit 4a42ee1. Configure here.
| return mergeGitStatusParts( | ||
| cached?.local?.value ?? local, | ||
| cached?.remote ? cached.remote.value : remote, | ||
| ); |
There was a problem hiding this comment.
Stale remote paired after checkout
Medium Severity
refreshStatusCore prefers cached local status after a mid-refresh checkout, but when that checkout cleared the cached remote half it falls back to the in-flight remote value. A checkout that lands after refreshRemoteStatus succeeds but before getCachedStatus can therefore return the new refName merged with the previous branch’s ahead/behind and PR data, even though the cache itself already dropped that remote half.
Reviewed by Cursor Bugbot for commit 4a42ee1. Configure here.
1 participant
What Changed
Why
PR status was derived from the current checkout, causing thread badges and PR actions to show stale or incorrect information after switching branches. Branch-keyed lookups keep each thread associated with its own PR while preserving fast stream-fed status for the active checkout. Refresh serialization prevents slow remote lookups from publishing results for a previous branch.
The typography cleanup keeps monospace surfaces consistent and removes unused advanced-font preference logic.
UI Changes
The sidebar, chat, branch selector, and Git actions now continue to display and act on the correct PR after switching checkouts. No visual layout changes were made, so before/after screenshots are not applicable.
Checklist
Note
Medium Risk
Touches VCS caching, concurrency, and checkout/switch RPC ordering where stale or mismatched PR/ref data could briefly surface; changes are localized to git status/PR resolution with added test coverage.
Overview
PR indicators and thread badges now follow each thread’s recorded branch, not whatever is checked out in the repo. A new
vcs.branchPrRPC (and cached client query) resolves PR/MR state from git branch config and upstream tracking, reusing the samelookupStatusPrpath as live status.Web and mobile replace checkout-only
resolveThreadPrwithuseThreadPr/selectBranchPr: streamed status wins whenrefNamematches the thread branch (including “no PR”), otherwise the branch-keyed query keeps the badge. Sidebar, chat, and the branch toolbar use this;resolveBranchToolbarPrBranchis removed. Git actions stay gated on checkout status only.VcsStatusBroadcastergets per-cwd local/remote semaphores, drops cached remote data onrefNamechange, discards superseded in-flight remote reads, and publishes a local-only snapshot (remote: null) on checkout change before async remote refresh.vcsSwitchReftriggersrefreshLocalStatusbefore the full refresh so clients don’t see the old branch after await.Contracts add
VcsBranchPrInput/VcsBranchPrResult; tests cover refresh races and branch PR selection.Reviewed by Cursor Bugbot for commit 4a42ee1. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Preserve pull request status by branch across checkout changes
vcs.branchPrRPC endpoint that resolves a PR for any branch (not necessarily checked out) by reading git config to infer upstream and default branch, without requiring an active checkout.selectBranchPranduseThreadPrutilities that prefer live git status when it matches the thread's branch, but fall back to the branch-keyedvcs.branchPrquery when the checkout has moved away.useThreadPrinstead of the removedresolveThreadPrfunction, so badges persist after a checkout change.VcsStatusBroadcasterto prevent stale remote or local status from overriding newer state during concurrent refreshes; remote halves are cleared on ref changes and repopulated asynchronously.Macroscope summarized 4a42ee1.