Skip to content

fix(git): make worktree PR detection reliable#4108

Open
tonoizer wants to merge 3 commits into
pingdotgg:mainfrom
tonoizer:codex/fix-reliable-pr-detection
Open

fix(git): make worktree PR detection reliable#4108
tonoizer wants to merge 3 commits into
pingdotgg:mainfrom
tonoizer:codex/fix-reliable-pr-detection

Conversation

@tonoizer

@tonoizer tonoizer commented Jul 17, 2026

Copy link
Copy Markdown

Summary

  • poll change-request metadata independently from automatic Git fetching
  • preserve lookup certainty and branch-correlated cached PR state
  • refresh PR metadata after agent turns and prevent unsafe PR-creation actions
  • surface provider lookup failures in worktree thread UI

Closes #4106

Testing

  • Focused VCS broadcaster, Git manager, reducer, checkpoint reactor, and UI tests
  • vp check
  • vp run typecheck

Before / After

Before: failed lookup could still offer PR creation After: safe push-only action, warning, and disabled PR entry
Before After

Risks

  • Adds lightweight provider polling for subscribed worktrees; polling is ref-counted, single-flight, and concurrency-bounded.
  • Changes the VCS status wire schema; server, shared reducers, web, and mobile consumers are updated together.
  • Provider failures expose only stable categories; detailed diagnostics remain server-side.

Note

Medium Risk
Wire schema and all status producers/consumers must include the new fields; added provider polling and concurrent turn-completion refreshes increase integration surface without exposing raw auth errors to the client.

Overview
VCS status now carries statusRefName and changeRequestLookup (pending / succeeded / failed with stable reasons) so clients can tell “lookup not done” or “lookup failed” apart from “no PR.” GitManager maps provider errors into those categories instead of silently returning pr: null.

VcsStatusBroadcaster adds refreshChangeRequestStatus (no upstream fetch), per-worktree refresh locking, coalescing, and keeps a cached open PR when a same-branch lookup fails. Background polling still runs lightweight change-request refreshes when automatic Git fetch is disabled; upstream fetch timing is split from that polling.

CheckpointReactor refreshes local git status and change-request status in parallel after turn completion (with cwd fallback and optional reconcile when the branch moved).

Shared reducers tie remote PR data to statusRefName and reset remote state when the local branch changes; git action quick actions and menus use canCreateChangeRequest so PR creation is blocked while lookup is pending or failed (cached open PRs stay actionable).

UI shows amber lookup-failure tooltips on sidebar / thread status rows; settings copy notes lightweight MR/PR checks continue without remote fetch.

Reviewed by Cursor Bugbot for commit 4517d76. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix worktree PR detection by tracking change request lookup state in git status

  • Adds a VcsStatusChangeRequestLookup field to remote git status that tracks whether a PR lookup is pending, succeeded, or failed (with structured reasons like authentication_required or provider_unavailable).
  • PR creation actions in menus and quick actions are suppressed until lookup succeeds; labels show 'Checking … status' or '… status unavailable' while lookup is unresolved.
  • Sidebar thread rows and leading status indicators display a warning icon with tooltip when PR lookup fails for the active branch.
  • VcsStatusBroadcaster now coordinates remote refreshes per worktree with locking and deduplication, and retains cached PR data on transient lookup failures for the same branch.
  • On turn completion, CheckpointReactor now concurrently refreshes both local git status and change request status, with a follow-up refresh when the branch has changed.
  • Remote status is ignored when its statusRefName does not match the current local branch, preventing stale cross-branch data from leaking into status merges.
  • Behavioral Change: VcsStatusRemoteResult now requires changeRequestLookup and statusRefName; existing decoders will fail if these fields are absent.

Macroscope summarized 4517d76.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9518dc56-1ea6-47ae-8405-a2e72e06a3d5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XL 500-999 changed lines (additions + deletions). labels Jul 17, 2026
Comment thread apps/server/src/vcs/VcsStatusBroadcaster.ts
Comment thread apps/web/src/components/ThreadStatusIndicators.tsx Outdated
@macroscopeapp

macroscopeapp Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR introduces substantial new runtime behavior (PR lookup state tracking, refresh coalescing, UI indicators) rather than a simple fix. Additionally, there are unresolved HIGH and MEDIUM severity review comments identifying potential race conditions in the branch refresh logic.

You can customize Macroscope's approvability policy. Learn more.

Comment thread apps/server/src/vcs/VcsStatusBroadcaster.ts
tonoizer and others added 3 commits July 20, 2026 18:13
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Issue: pingdotgg#4106

Co-authored-by: codex <codex@users.noreply.github.com>
@juliusmarminge
juliusmarminge force-pushed the codex/fix-reliable-pr-detection branch from 106234c to 4517d76 Compare July 20, 2026 16:35

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4517d76. Configure here.

{ concurrency: "unbounded" },
);
return yield* updateCachedStatus(cwd, local, remote, { publish: true });
return mergeGitStatusParts(local, remote);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale pending after branch refresh race

High Severity

The new branch-correlation guard in applyGitStatusStreamEvent incorrectly filters remoteUpdated events. It prevents updates for new branches from being applied if they arrive before local updates, leaving PR status stuck as pending. It also misinterprets remote: null as a branch mismatch, causing clients to retain stale remote state instead of clearing it.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4517d76. Configure here.

return VcsStatusBroadcaster.of({
getStatus,
refreshLocalStatus,
refreshChangeRequestStatus,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale remote delays PR refresh

Medium Severity

streamStatus only starts the remote poller immediately when no remote cache entry exists. After this PR, local refresh can advance refName while the cached remote still belongs to the previous branch, so a new poller sleeps for the full lightweight interval before looking up the current branch’s PR. Turn-completion reconcile also skips when either concurrent refresh fails, so a branch-changing turn with a failed change-request refresh can stay pending until the next poll.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4517d76. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL 500-999 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Worktree threads can miss existing pull requests

2 participants