Skip to content

Fix remote project binding races during concurrent open and switch - #356

Merged
arul28 merged 15 commits into
mainfrom
cursor/critical-bug-detection-c1b3
May 26, 2026
Merged

Fix remote project binding races during concurrent open and switch#356
arul28 merged 15 commits into
mainfrom
cursor/critical-bug-detection-c1b3

Conversation

@cursor

@cursorcursorBot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Cron critical-bug audit found a high-severity correctness issue in Harden Remote Connections (#353) remote project routing that is not covered by open draft PR #354 (sync RPC reconnect retry).

Bug and impact

Impact: While switching between remote projects (or opening two projects in quick succession), mutating sync/lane/git RPCs could run against the previous remote projectId because:

  1. remoteRuntime.openProject did not clear the preload binding or participate in the project-transition guard.
  2. Overlapping openProject calls applied whichever response finished last, even if a newer switch had already started.
  3. Stale switchRemoteProject completions could overwrite renderer state after a faster switch to a different project.

Trigger: Remote runtime → open project A → quickly switch tab/command palette to project B (or double-open) → during the in-flight openProject, UI or background code calls sync.setPin, sync.connectToBrain, lane create, git push, etc.

Root cause

Preload cached currentProjectBinding across remote switches without generation checks or transition blocking; renderer switchRemoteProject applied every completion unconditionally.

Fix

  • Wrap remoteRuntime.openProject in runProjectRuntimeTransition, clear binding at start, only rememberProjectBinding for the latest generation.
  • Block mutating sync methods and non-read-only runtime actions during transitions (same guard pattern as mutating chat).
  • Ignore stale switchRemoteProject store updates when a newer switch has started.

Validation

  • npm run test -- --run src/preload/preload.test.ts -t "remote project binding" (2 tests)
  • npm run test -- --run src/renderer/state/appStore.test.ts -t "stale switchRemoteProject" (1 test)

Related open PRs (not duplicated)

Open in WebView Automation

ADEOpen in ADE · cursor/critical-bug-detection-c1b3 branch · PR #356

Greptile Summary

This PR hardens remote project binding during concurrent openProject calls and rapid project switching by introducing generation counters at three layers (preload, renderer store, and IPC bridge) so stale completions never overwrite state set by a newer switch. It also blocks mutating runtime and sync actions during transitions and adds isCommitInLaneHistory with branch-scoped undo tracking.

  • Generation guards at three independent layers: openRemoteProjectGeneration in the preload, remoteProjectSwitchGeneration in appStore, and remoteOpenProjectGenerations in runtimeBridge each independently discard stale responses, with runProjectRuntimeTransition blocking concurrent mutating calls.
  • Mutating action guard expanded: MUTATING_SYNC_METHODS, READ_ONLY_RUNTIME_ACTION_PREFIXES, and shouldBypassProjectRuntimeDuringTransition extend the existing chat-only guard to all runtime domains, with read-only actions falling through to IPC when a remote open is in flight.
  • Branch-scoped undo / VM lane refactoring: getLatestUndoableHeadChange now filters by currentBranchRef and stops at checkout boundaries; wireMacosVmLanePlacement is extracted into a shared helper and VM launch cache entries are keyed by (projectRoot, laneId) to prevent cross-project collisions.

Confidence Score: 5/5

Safe to merge. The three independent generation counters correctly discard stale remote-project completions, and the mutating-action guards are wired consistently across sync and runtime call paths.

The race-condition fix is structurally sound: each layer independently tracks the latest generation and ignores stale completions without touching shared state. Tests cover overlapping opens, mutating-sync blocking, and stale store updates. The two findings are minor: isCommitInLaneHistory is misclassified as mutating (masked by .catch(() => false) at its only call site), and stale switch errors still propagate to the original caller (a cosmetic UX concern, not a state-corruption risk).

apps/desktop/src/preload/preload.ts — the read-only action set and the newly added isCommitInLaneHistory bridge method.

Important Files Changed

FilenameOverview
apps/desktop/src/preload/preload.tsCore of the fix: adds openRemoteProjectGeneration counter, shouldBypassProjectRuntimeDuringTransition, and MUTATING_SYNC_METHODS guard. New isCommitInLaneHistory bridge method is misclassified as mutating by the prefix check, causing spurious throws during transitions (mitigated by caller's .catch).
apps/desktop/src/renderer/state/appStore.tsAdds remoteProjectSwitchGeneration counter to switchRemoteProject; stale completions return early on success and skip state updates on error. Logic is correct and well-tested.
apps/desktop/src/main/services/ipc/runtimeBridge.tsAdds per-window/sender generation tracking to remoteRuntimeOpenProject IPC handler; stale completions skip bindRemoteProject. canBindRemoteProjectToSender guard prevents binding to destroyed windows.
apps/desktop/src/main/services/lanes/laneLaunchContext.tsVM launch cache keys now include projectRoot to prevent cross-project collisions. pickActiveVmRecord correctly scopes by laneId. syncMacosVmLaunchCacheFromEvent added to keep cache aligned with VM lifecycle events.
apps/desktop/src/main/services/lanes/laneService.tsExtracts wireMacosVmLanePlacement into a shared helper with previousPlacement:"none" for the creation path. Adds cleanupCreatedWorktreeLaneAfterVmWireFailure (returns never) ensuring creation rollback always throws.
apps/desktop/src/main/services/git/gitOperationsService.tsBranch-scoped undo logic: getLatestUndoableHeadChange now filters by currentBranchRef and stops at HEAD_CHANGE_UNDO_BOUNDARY_KINDS. Adds isCommitInLaneHistory and normalizeCommitShaArg helper.
apps/desktop/src/renderer/components/history/HistoryPage.tsxAdds commitOnLaneHistory state tracked via isCommitInLaneHistory. Fixes lane focus synchronization with selected lane and adds guard against guessing a lane for commit-only deeplinks.
apps/desktop/src/preload/preload.test.tsAdds three new preload tests covering overlapping openProject calls, mutating sync blocking during transitions, and file mutation blocking. Good coverage of the race condition scenarios.
apps/desktop/src/renderer/state/appStore.test.tsAdds stale switchRemoteProject test verifying that a slow project-A switch completing after project-B doesn't overwrite renderer state. Test correctly validates both store state and binding.

Sequence Diagram

sequenceDiagram
participant UI as Renderer (appStore)
participant PL as Preload (openProject)
participant IPC as IPC Bridge (runtimeBridge)
participant Remote as Remote Runtime
Note over UI,Remote: Concurrent project switch scenario
UI->>UI: "switchRemoteProject(A) gen=1"
UI->>UI: set(projectTransition)
UI->>PL: remoteRuntime.openProject(A)
PL->>PL: "generation=1, rememberBinding(null)"
UI->>UI: "switchRemoteProject(B) gen=2"
UI->>UI: set(projectTransition)
UI->>PL: remoteRuntime.openProject(B)
PL->>PL: "generation=2, rememberBinding(null)"
IPC->>Remote: "connect(A) gen=1"
IPC->>Remote: "connect(B) gen=2"
Remote-->>IPC: bindingB latest → bindRemoteProject(B)
IPC-->>PL: bindingB
PL->>PL: "gen(2)==gen(2) → rememberBinding(B), clearActive"
PL-->>UI: bindingB
UI->>UI: "switchGen(2)==latest(2) → set(project=B, transition=null)"
Remote-->>IPC: bindingA stale → isLatest? No → skip bindRemoteProject
IPC-->>PL: bindingA
PL->>PL: gen(1)≠gen(2) → skip rememberBinding
PL-->>UI: bindingA stale binding returned
UI->>UI: switchGen(1)≠latest(2) → return early no state change
Note over UI,Remote: Mutating call during transition
UI->>PL: sync.setPin() during openProject
PL->>PL: "MUTATING_SYNC_METHODS + transitionDepth>0"
PL-->>UI: throws Project is switching...
Loading

Fix All in CursorFix All in CodexFix All in Claude Code

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---### Issue 1 of 2
apps/desktop/src/preload/preload.ts:1225-1232
`git.isCommitInLaneHistory` is the one new action added in this PR that doesn't match any prefix in `READ_ONLY_RUNTIME_ACTION_PREFIXES` (it starts with `"is"`, not `"get"`, `"list"`, etc.), so `isMutatingRuntimeAction("git", "isCommitInLaneHistory")` returns `true`. During any project transition `callProjectRuntimeActionIfBound` throws "Project is switching…" before the IPC fallback is reached. The only current caller wraps it with `.catch(() => false)`, which masks the error, but any future caller without that guard will receive a spurious transition error at runtime.
```suggestionconst READ_ONLY_RUNTIME_ACTIONS = new Set([ "chat.codexFuzzyFileSearch", "chat.fileSearch", "chat.modelCatalog", "file.quickOpen", "git.isCommitInLaneHistory", "terminal.activeForChat", "terminal.preview",]);```### Issue 2 of 2
apps/desktop/src/renderer/state/appStore.ts:1650-1657
**Stale error still throws to original caller**
In the catch block the `throw error` runs unconditionally regardless of `switchGeneration`. If switch A (gen=1) fails after switch B (gen=2) has already succeeded, A's error is re-thrown to A's original caller, which may display an error toast or set local component error state even though the project switch ultimately succeeded. Consider swallowing the error when the generation is stale: `if (switchGeneration !== remoteProjectSwitchGeneration) return;` before the rethrow.

Reviews (5): Last reviewed commit: "ship: iteration 1 — fix corrupt CI cache..." | Re-trigger Greptile

@vercel

vercelBot commented May 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
adeIgnoredIgnoredPreviewMay 26, 2026 1:18am

@arul28

Copy link
Copy Markdown
Owner

@copilot review but do not make fixes

CopilotAI commented May 25, 2026

Copy link
Copy Markdown
Contributor

@copilot review but do not make fixes

Reviewed the PR changes as requested (no fixes made). I don’t see any new blocking issues in the remote project binding race handling as currently implemented.

CopilotAI requested a review from arul28May 25, 2026 23:28
@arul28
arul28 marked this pull request as ready for review May 25, 2026 23:31

@cursorcursorBot left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

PR Review

Scope: 63 file(s), +2800 / −277
Verdict: Looks good

This change hardens concurrent remote project open/switch, VM launch-cache scoping, Work tab session hydration, History git safety, and sync schema handling. The generation guards in runtimeBridge.ts, preload, and appStore line up with the new tests and look correct for out-of-order completions.


Notes

  • Good pattern: pairing main-process remoteOpenProjectGenerations / preload openRemoteProjectGeneration with renderer remoteProjectSwitchGeneration so stale async opens cannot re-bind the window or store.
  • HistoryPage intentionally avoids guessing laneId on commit-only deeplinks; destructive actions are gated by isCommitInLaneHistory and disabled reasons — worth a quick manual pass on ade:// commit links without laneId to confirm the UX reads clearly.
  • useWorkSessions project-switch refs (pendingProjectSwitchRef, hasAuthoritativeSessionsRef) address a subtle one-render cache poison risk; did not run the desktop suite in this environment.
  • Review is based on the PR commit range in the workspace; could not fetch live PR metadata via gh (auth unavailable here).
Open in WebView Automation

Sent by Cursor Automation: BUGBOT in Versic

Comment threadapps/desktop/src/preload/preload.ts Outdated
Comment threadapps/desktop/src/preload/preload.ts
Comment threadapps/desktop/src/main/services/git/gitOperationsService.ts
Comment threadapps/desktop/src/main/services/lanes/laneService.ts
@arul28

Copy link
Copy Markdown
Owner

@copilot review but do not make fixes

@capy-ai

capy-aiBot commented May 25, 2026

Copy link
Copy Markdown

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

CopilotAI commented May 25, 2026

Copy link
Copy Markdown
Contributor

@copilot review but do not make fixes

Reviewed again without making fixes. Current race-handling changes look sound in the touched paths (preload generation/transition guard, renderer stale-switch suppression, and runtime bridge latest-open binding). I also checked CI logs: the most recent failure on this branch was test-desktop (8) from run 26421166350 due to a failing orchestrator test (aiOrchestratorService.test.ts), while the latest completed CI run for this PR branch succeeded.

@arul28
arul28force-pushed the cursor/critical-bug-detection-c1b3 branch from 92d8931 to 76394b9CompareMay 26, 2026 00:18
@arul28

Copy link
Copy Markdown
Owner

@copilot review but do not make fixes

CopilotAI commented May 26, 2026

Copy link
Copy Markdown
Contributor

@copilot review but do not make fixes

Reviewed again without making fixes. The core remote-binding race handling still looks correct across preload/appStore/runtimeBridge. I do see two adjacent (non-core) edge risks still present: HistoryPage keeps commitOnLaneHistory from the previous lane until async history check resolves, and isCommitInLaneHistory still falls through to false on non-1 git exit codes (e.g. unknown-object errors). CI check: latest completed CI on this branch succeeded, while the most recent failed run (26421166350) was still the orchestrator paused-run test failure (aiOrchestratorService.test.ts); current newer runs are in progress.

Comment threadapps/desktop/src/main/services/git/gitOperationsService.ts
cursoragentand others added 14 commits May 25, 2026 20:40
- Clear preload binding during remote openProject and ignore stale completions
- Run openProject inside project runtime transition guard
- Block mutating sync and non-read-only runtime actions during transitions
- Ignore stale switchRemoteProject UI updates when a newer switch starts
Regression tests in preload and appStore.
Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
Add regression tests for dropLegacyUnifiedMemoriesSchema and
lane_linear_issue_links dedupe/index cleanup during openKvDb.
Co-authored-by: Arul Sharma <arul28@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Remove dead finally block in preload remote project open
- Add camelCase boundary check to read-only action prefix matching
- Remove unreachable git_undo_head_change from boundary set
- Use "none" as previousPlacement for VM lane creation path
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arul28
arul28force-pushed the cursor/critical-bug-detection-c1b3 branch from 76394b9 to 7f84199CompareMay 26, 2026 00:42
@arul28

Copy link
Copy Markdown
Owner

@copilot review but do not make fixes

- Bust stale node_modules cache (nm- → nm-v2-) caused by parallel
install race where npm ci ECONNRESET was silently swallowed
- Fix parallel install to track PIDs and fail if any npm ci exits non-zero
- Skip CRDT changes for tables removed from schema instead of throwing
- Align test expectations for unknown-table handling
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arul28

Copy link
Copy Markdown
Owner

@copilot review but do not make fixes

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@arul28@cursoragent