Recover a Cloud machine graph stuck on an equal-cursor conflict

[teamleaderleo]

Summary

A Cloud or SSH machine's graph can wedge for the life of the app. A full snapshot and the graph the deltas built can disagree at the same cursor. When they do, installSnapshotIfNewer refuses the snapshot as an "equal-cursor conflict", and no later read at that cursor ever succeeds. refreshCurrentGraph never becomes current, and opens on that machine fail until relaunch. #15205 fixes one known cause (exited terminals' tab fields). Any other strictly compared field that differs at the same revision still wedges: cwd, lifecycle, or keys the model doesn't include.

Now:

• The first conflict from a current full refresh keeps the installed graph, records the cursor, and schedules the existing bounded state-recovery read.
• A second conflict at the same cursor adopts the daemon's full snapshot and leaves a cloud.state.equalCursorConflictAdopted breadcrumb. A full read at the current cursor is the daemon's own answer. So a wedge always breaks, and a single race never discards the graph.
• Only a current full refresh can arm or adopt. Event-feed snapshots and reads that started before a newer install keep today's behavior (refused).
• Which side wins: adoption replaces the whole graph, like any fresh install. A field or key the daemon does not send is absent afterwards rather than kept from the delta-built graph. App-side overlays (pending renames, pending creations) live outside cloudState and are reapplied on publish. A pending rename's predecessor is still refused outright and never arms recovery.
• Recovery budget: a successful adoption resets the recovery count, so it does not use up the budget event-feed barriers rely on.

Composes with #15205 and #15283: git merge-tree shows no source conflict with either. The only overlap with #15283 is both PRs adding a test file to project.pbxproj, which catch-up regenerates.

Testing

• New cmuxTests/CloudEqualCursorConflictRecoveryTests.swift, committed failing before the fix:
  • the first full-refresh conflict keeps the graph and arms recovery, and a second adopts
  • an event-feed conflict never adopts
  • a stale read never adopts
  • a newer install clears an armed conflict, and a conflict at the new cursor starts over
• scripts/sync-test-wiring for the pbxproj. scripts/verify-local.py passes. No local native build; CI runs the tests.
• No fleet dogfood. Reproducing needs a daemon whose full snapshot and deltas disagree at one revision (the Keep an exited terminal's tab edge consistent with its revision #15205 bug is one such case). The unit tests drive the exact install path the refresh uses.

Changelog

Fixed: a Cloud or SSH machine whose state got out of sync no longer stays stuck until relaunch; the next refresh recovers it.

🤖 Generated with Claude Code

---

Summary by cubic

Recovers a Cloud or SSH machine's graph that was permanently stuck when a full snapshot disagreed with the delta-built graph at the same cursor. The first such conflict from a full refresh keeps the installed graph and schedules the bounded state-recovery read; a second conflict at the same cursor adopts the daemon's full snapshot, so the wedge always breaks.

• Only the full refresh that armed it schedules that recovery read, so stale or rename-fenced reads can't spend the budget.
• A delta advancing the cursor, a newer or equal-content install, and a feature-flag suspend clear the armed conflict; a successful adoption resets the recovery count.
• Event-feed snapshots and stale reads never arm or adopt; a pending rename's predecessor is still refused outright.
• Adds CloudEqualCursorConflictRecoveryTests covering the arming, adoption, fence, suspend, and clearing paths.

^{Written for commit 7784fef. Summary will update on new commits.}

[coderabbitai]

Warning

Review limit reached

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9d4c8a71-9021-4cf0-8c80-626f99ec5eca

📥 Commits

Reviewing files that changed from the base of the PR and between 62ee70e and 7784fef.

📒 Files selected for processing (3)

• Sources/Surfaces/CmuxTuiSurfaceProviders.swift
• cmux.xcodeproj/project.pbxproj
• cmuxTests/CloudEqualCursorConflictRecoveryTests.swift

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[teamleaderleo]

Review: this is a self-review by the authoring agent, not a separate review subagent. This worker could not spawn one, so a separate subagent review is still owed before merge.

Checked:

• No loop. The recovery read runs through the existing bounded scheduleStateRecoveryRefresh (limit 5). If the race has resolved by then, the normal install path clears the armed conflict. If it repeats, the second read adopts. A delta that advances the cursor makes the armed cursor stale and harmless.
• The stale-read guard is kept. Adoption requires requestVersion == cloudStateInstallVersion, so an event install during the read blocks it.
• The rename fence is kept. A snapshot failing incomingPassesPendingRenameFence is refused before any conflict handling.
• Merge safety. The guard line Fix Cloud graph freezing on daemons that send no cursor #15283 edits is left untouched; the change sits inside its else branch, so merge-tree is clean for Swift. Clean against Keep an exited terminal's tab edge consistent with its revision #15205.

Fixed during self-review:

• A successful adoption resets stateRecoveryCount, so recovering one wedge does not use up the budget event-feed barriers rely on.

Left:

• Event-feed snapshot conflicts still refuse without scheduling recovery. That is today's behavior; the feed's own barrier handling owns that path.
• The stale reason label. scheduleStateRecoveryRefresh marks the graph stale with reason event_feed_recovery, including when a conflict triggered it. It's cosmetic.

[teamleaderleo]

Review (separate subagent, correctness first; the self-review above was not one): nothing serious.

Checked:

• Race safety: arming and adopting both require the read to be current (requestVersion == cloudStateInstallVersion). Event-feed snapshots never arm or adopt.
• Pending renames: the rename fence runs first, so a pending rename's predecessor is still refused.
• The recovery read is a fresh forced pass, not the in-flight pass's result.
• Adopting matches a normal install, and pending creations and renames are reapplied on publish.
• No loop: adopting schedules nothing.
• Wiring: sync_test_wiring.py --check passes.

Fixed:

• Low: a delta that advances the cursor, and a feature-flag suspend, now clear the armed conflict. Before, the first read after resuming could adopt at once.
• Low: only the install that arms the conflict schedules the recovery read (equalCursorConflictArmedByLastInstall). Stale or rename-fenced reads at an armed cursor no longer spend the budget.
• Tests added:
  • only the arming install asks for recovery
  • a pending rename's predecessor is never adopted, however often it conflicts
  • suspending clears an armed conflict

Left:

• Low: when the recovery budget (5) is spent, the conflict stays armed and the next refresh adopts. There is now a comment saying so.
• Info: adopting resets the budget, so a daemon whose full snapshot keeps changing at a fixed cursor costs two reads and a breadcrumb per refresh. That is bounded.
• The cursor-advance clear on the delta path has no test. Driving the event feed from a unit test needs a live link.

[github-actions]

CI failure attribution

CI passes on 7784feffe2 (run 36454893618 attempt 1).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

[teamleaderleo]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[teamleaderleo]

Review (separate subagent) of head 8e2ff57: nothing serious.

• CI red on ef53b82 was a test bug, not the fix: CmuxTuiSurfaceProvider.catalog is unowned, and the suite passed a temporary SurfaceCatalog() that was freed before recordPendingRemoteRename published through it (renameFenceHoldsWhileArmed crashed). The suite now owns the catalog for each test.
• All seven tests traced against the source; no other test in the file holds a freed reference.
• Merged main; the pbxproj conflict kept both group entries, and the test wiring checks pass.

[teamleaderleo]

Review (separate subagent) of head 8e2ff57: no correctness bugs. Arming and adoption are gated on a current full refresh, the rename fence still runs first, and the recovery budget is spent only by the read that armed.

Test coverage is partial, so the unit tests are not enough to stand in for a dogfood:

• Covered: arm then adopt, the stale and event-feed guards, the rename fence, clearing on a newer cursor and on suspend.
• Not covered by a test that fails without the change: the scheduleStateRecoveryRefresh() call from performRefresh (nothing drives performRefresh), the recovery count reset on adoption, clearing on a delta install, clearing on an equal-content install. The suspend and newer-install tests also do not assert a conflict was armed first.

Leaving this open until it can be dogfooded against a Cloud backend.

[teamleaderleo]

Subagent review at 8e2ff57: no correctness bugs (arming and adoption gated on a current full refresh, rename fence first, recovery budget spent only by the arming read). Findings: test catalog lifetime bug, fixed in e975b55; coverage gaps, addressed in 6a7c880 (test only). Only main merges since. Landing: merging main after #15414, then auto-merge.

[github-actions]

Merge receipt for 7784feffe2: every check was green at merge (15 verified; 17 skipped by policy). Full suite runs on main after merge.