Stabilize PR status lookups and provider session lifecycle#4281
Conversation
- Cache GitHub PR lookups while preserving the last known result on failures - Support older gh repository response fields - Reuse remotes across SSH and HTTPS URL variants - Route refreshes through full VCS status invalidation
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| // ensureRemote reuses an existing remote instead of adding an ssh/https | ||
| // duplicate (git@host:owner/repo, ssh://git@host/owner/repo, | ||
| // https://host/owner/repo all compare equal). | ||
| const scpLike = /^(?:ssh:\/\/)?(?:[^@/:]+@)([^:/]+)[:/](.+)$/.exec(normalized); |
There was a problem hiding this comment.
🟡 Medium vcs/GitVcsDriverCore.ts:248
normalizeRemoteUrl treats the port in an SSH URL as part of the repository path. ssh://git@github.com:22/owner/repo.git normalizes to github.com/22/owner/repo, while the equivalent https://github.com/owner/repo.git normalizes to github.com/owner/repo. As a result, ensureRemote fails to match the existing remote and adds a duplicate whenever the SSH URL explicitly includes a port.
The scpLike regex captures host:port/path in group 2 instead of excluding the port. It needs to separate an optional :port from the host before constructing host/path.
- const scpLike = /^(?:ssh:\/\/)?(?:[^@/:]+@)([^:/]+)[:/](.+)$/.exec(normalized);
+ const scpLike = /^(?:ssh:\/\/)?(?:[^@/:]+@)([^:/]+)(?::\d+)?[:/](.+)$/.exec(normalized);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/vcs/GitVcsDriverCore.ts around line 248:
`normalizeRemoteUrl` treats the port in an SSH URL as part of the repository path. `ssh://git@github.com:22/owner/repo.git` normalizes to `github.com/22/owner/repo`, while the equivalent `https://github.com/owner/repo.git` normalizes to `github.com/owner/repo`. As a result, `ensureRemote` fails to match the existing remote and adds a duplicate whenever the SSH URL explicitly includes a port.
The `scpLike` regex captures `host:port/path` in group 2 instead of excluding the port. It needs to separate an optional `:port` from the host before constructing `host/path`.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces new caching and fallback behavior for PR status lookups, representing significant runtime changes rather than simple fixes. Two unresolved review comments also identify potential bugs in the new logic that should be addressed. You can customize Macroscope's approvability policy. Learn more. |
…fallback, complete mocks - Replace the hand-rolled normalizeRemoteUrl in GitVcsDriverCore with the shared normalizeGitRemoteUrl, which correctly drops explicit ssh ports (ssh://git@host:22/owner/repo now matches https://host/owner/repo). - Key the last-known-PR fallback by (cwd, branch) only so an upstream change (e.g. first push -u) does not orphan the sticky value. - Implement invalidateStatus in the remaining GitWorkflowService/GitManager test mocks; the missing method made the background-refresh server test hang for 120s in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…upstream The last-known-PR fallback (added to avoid blanking the badge on a transient gh failure) is keyed by branch only, so a retarget to a different upstream/fork mid-failure could surface a PR that no longer matches the current head. Track the upstream the fallback was resolved against and withhold it when both the cached and current upstreamRef are known and differ, while still tolerating an upstream merely appearing or disappearing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 c0b9cee. Configure here.
| // without invalidating the fallback when it still targets the same repo. | ||
| if (lastKnown.headRemoteUrlKey !== null || current.headRemoteUrlKey !== null) { | ||
| return lastKnown.headRemoteUrlKey === current.headRemoteUrlKey ? lastKnown.pr : null; | ||
| } |
There was a problem hiding this comment.
Sticky PR dropped on null URL key
Medium Severity
resolveLastKnownPr clears sticky PR badges prematurely. Transient failures (e.g., git config read errors, gh API issues) can cause headRemoteUrlKey mismatches or null values. The function's strict headRemoteUrlKey comparison then incorrectly invalidates the remembered PR, even if the branch's remote identity is unchanged, affecting fork workflows.
Reviewed by Cursor Bugbot for commit c0b9cee. Configure here.
…#4281) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…dotgg#2284) (#184) * Sidebar v2 beta: flat thread list with a server-backed settled lifecycle (pingdotgg#4026) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: maria-rcks <maria@kuuro.net> (cherry picked from commit 32c6012) * fix(settings): validate the add-provider wizard step before advancing (pingdotgg#2813) (pingdotgg#3100) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 282ecb3) * fix(claude): isolate capability probe from user MCP servers (pingdotgg#4015) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit aa5ec80) * Preserve connecting status while a turn starts (pingdotgg#4101) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 783692a) * fix(server): stop restoring stale OpenCode models (pingdotgg#4095) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 4e09cdd) * [codex] keep scoped package references as text (pingdotgg#4167) Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit c7b21ff) * fix(web): default provider selection for users without Codex (pingdotgg#4117) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit b6e1b39) * Unify temporary worktree branch naming (pingdotgg#4278) (cherry picked from commit 571a8b4) * fix(web): use message-square icon for settled icon-less project threads in sidebar v2 (pingdotgg#4279) (cherry picked from commit 020179c) * Stabilize sidebar settling animations (pingdotgg#4280) (cherry picked from commit 18b4688) * Restore Copy Link in chat link context menu (pingdotgg#4161) Co-authored-by: Julius Marminge <julius0216@outlook.com> (cherry picked from commit e5fba26) * fix(desktop): handle EPIPE errors on stdout/stderr to prevent crash dialog (pingdotgg#4213) (cherry picked from commit f74eb62) * Preserve draft thread highlighting during promotion (pingdotgg#4283) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 18fa89c) * Move mobile working timer into the thread timeline (pingdotgg#4285) (cherry picked from commit 7e2bb47) * Stabilize PR status lookups and provider session lifecycle (pingdotgg#4281) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 376c149) * fix: open command palette instead of custom dialog for new thread picker in SidebarV2 (pingdotgg#4269) (cherry picked from commit 9fe4832) * fix(server): don't drop sticky PR fallback when remote URL can't be resolved (pingdotgg#4289) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 9a0a071) * feat(web): copy branch name via right-click in the branch selector (pingdotgg#4275) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 78a0ea5) * Add remote server updates and standalone service management (pingdotgg#4286) Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit ab4a883) * Refine light-mode sidebar surfaces (pingdotgg#4268) Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 593289c) * fix(mobile): don't mark Android VPN/Tailscale as offline when connected (pingdotgg#3949) (cherry picked from commit bc9428a) * improve and prevent silent thread branch drift and PR fetching (pingdotgg#2284) Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> (cherry picked from commit 2d31cb0) * fix(sync): restore fork migration IDs under Sidebar v2 settled Upstream pingdotgg#4026 replaced Migrations.ts with linear upstream numbering and dropped fork-only migrations (NormalizeLegacyProviderKinds, RepairProposedPlanColumns, BackfillForkProviderInstanceIds). Restore the fork registry and append ProjectionThreadsSettled as ID 36. Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com> --------- Co-authored-by: Theo Browne <me@t3.gg> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: maria-rcks <maria@kuuro.net> Co-authored-by: Leonel Rivas <encriptandost@gmail.com> Co-authored-by: Julius Marminge <julius0216@outlook.com> Co-authored-by: codex <codex@users.noreply.github.com> Co-authored-by: Jaret Bottoms <jaretbottoms@gmail.com> Co-authored-by: Ishan <ishansachu1@gmail.com> Co-authored-by: Yukun Shan <92423096+nateEc@users.noreply.github.com> Co-authored-by: Maxwell Young <maxtheyoung@gmail.com> Co-authored-by: Miklós Fazekas <mfazekas@szemafor.com> Co-authored-by: Utkarsh Patil <73941998+UtkarshUsername@users.noreply.github.com> Co-authored-by: Henry Zhang <113233555+caezium@users.noreply.github.com> Co-authored-by: Rushikesh Gaikwad <81632222+Wraient@users.noreply.github.com> Co-authored-by: legs <145564979+justsomelegs@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: aaditagrawal <aaditagrawal@users.noreply.github.com>


Summary
Testing
Note
Medium Risk
Changes visible PR badge behavior on errors and caches PR association separately from other status fields; incorrect sticky fallback rules could show wrong PRs until upstream/remote keys change.
Overview
PR status now uses a separate slow cache for hosting-provider PR lookups (ahead/behind still refresh on the fast status cadence). Transient
ghfailures return the last known PR instead of clearing the badge, with guards so retargeted upstreams, repointed remotes, or fork changes do not show a stale association.Explicit
invalidateStatus(user refresh, git actions) bumps a PR lookup epoch so live lookups run immediately;VcsStatusBroadcaster.refreshStatuscalls that full invalidation instead of only local/remote caches.GitHub PR list decoding tolerates older
gh(< 2.47) payloads missingheadRepository.nameWithOwner/ owner login by reconstructingowner/repowhen possible.ensureRemotereuses an existing remote when URLs match after sharednormalizeGitRemoteUrl(ssh/https, ports,.git, casing). Branch head context tracksheadRemoteUrlKeyfor the sticky-PR matching rules.Reviewed by Cursor Bugbot for commit c0b9cee. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Stabilize PR status lookups with caching, fallback state, and provider session lifecycle fixes
findLatestPrcalls on every remote status read.cwdepoch on explicit invalidation to force a fresh lookup.VcsStatusBroadcaster.refreshStatusnow calls a singleinvalidateStatusinstead of separate local/remote invalidations, ensuring the PR cache epoch is also bypassed on manual refresh.GitHubPullRequestSchemato tolerate olderghoutput that omitsheadRepository.nameWithOwner, reconstructing it from owner login and repo name when available.GitVcsDriver.ensureRemotenow uses sharednormalizeGitRemoteUrlto recognize equivalent remotes across http/ssh variants, schemes, ports, and.gitsuffix differences.Macroscope summarized c0b9cee.