Repository navigation
iOS: reconnecting never exits the user's workspace or invalidates their current task - #11091
azooz2003-bit wants to merge 2 commits into
Conversation
On iPhone, the moment an iroh reconnect began the mounted workspace detail popped back to the list. Mechanism, reproduced by these tests: 1. Recovery's redial teardown (`clearRemoteConnectionContext`) drops every secondary Mac's `workspacesByMac` entry. Going from N Macs to 1 flips multi-Mac row-id scoping, so every derived row id changes raw value and the selection remaps to a different id, re-keying the pushed navigation route and destroying the mounted detail. 2. A failed first dial (immediate when the network just dropped) runs the teardown again with `foregroundMacDeviceID` already nil, so the retention filter keys on the anonymous sentinel and wipes the whole list. Two tests fail on main; the preserving-mode baseline passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Connection teardown (`clearRemoteConnectionContext`) no longer removes `workspacesByMac` rows. Teardown is a transport event; rows are removed only by authoritative events (healthy list reconcile, unpair/hide, sign-out, team change). This fixes both halves of the reconnect pop: secondary entries survive so multi-Mac row-id scoping never flips mid-recovery (the pushed navigation route keeps its identity), and the failed-dial re-teardown that used to wipe the whole list through the anonymous-sentinel retention filter now only downgrades statuses, which is idempotent. All retained entries downgrade to `.unavailable` so the Computers dots and list chrome stay truthful, and the post-reconnect secondary refresh already re-syncs them. Defense-in-depth at the navigation layer: the compact path policy takes `listIsAuthoritative`. While the workspace list is reconnecting or disconnected, a cleared selection or a list hole keeps the mounted detail on its last-known snapshot; pops still happen normally when a healthy list confirms a deletion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change retains workspace rows during connection teardown and prevents compact navigation from removing mounted detail routes when workspace-list data is not authoritative. Tests cover repeated teardown, selection retention, status changes, and navigation behavior. ChangesWorkspace retention and navigation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The PR keeps users in their workspace during transient reconnects while preserving authoritative removal behavior. It is mergeable with owner awareness because concurrent refreshes or failed non-foreground switches could still leave workspace state or secondary updates stale in edge cases. Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Description checkExplanation The description provides a detailed summary, rationale, implementation scope, testing results, and follow-up context. It omits the template's explicit Testing, Demo Video, Review Trigger, and Checklist sections, but the core change and verification information are present. Full details: Docstring CoverageExplanation Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. (1 skipped: 1 too large.) Full details: Cmux Swift Actor IsolationExplanation PASS. The production diff does not introduce or materially worsen an actor-isolation mistake. Full details: Cmux Swift Blocking RuntimeExplanation PASS — The PR introduces no blocking or timing-based synchronization in production Swift. The production diff changes workspace retention, status updates, navigation policy parameters, and comments only. Added production lines contain no semaphore waits, sleeps, delayed dispatch, polling, main-queue sync, or manual locks. The new Swift tests are deterministic and contain no timing or blocking primitives, which the policy allows. Full details: Cmux Browser Automation Off-MainExplanation PASS: The PR changes only iOS workspace teardown and navigation files. The two rule-target files, Full details: Cmux Expensive Synchronous LoadExplanation PASS: The pull request adds no expensive synchronous agent-history load. The production diff only changes workspace retention and navigation state. Added production lines contain no Full details: Cmux Cache Substitution CorrectnessExplanation PASS — the PR does not replace a fresh persistence, history, undo, or snapshot read with a cache. The production diff only retains in-memory Full details: Cmux No Hacky SleepsExplanation PASS: The complete PR range ( Full details: Cmux Algorithmic ComplexityExplanation No algorithmic-complexity failure is introduced. In Full details: Cmux Swift ConcurrencyExplanation PASS: The parent-to-HEAD diff adds no legacy Swift concurrency pattern. The changes are synchronous workspace-state retention, a Boolean navigation-policy parameter, call-site plumbing, comments, and synchronous tests. The changed files contain existing Full details: Cmux Swift `@Concurrent`Explanation PASS. The full PR diff adds no Full details: Cmux Swift Package BoundariesExplanation PASS. The production diff does not keep the changed logic in the app target. The changed store code is in the Full details: Cmux Swiftpm LockfilesExplanation PASS: The PR diff from d1e6aaa to a0c781b changes only Swift source and test files. It does not change Package.swift, Package.resolved, .gitignore, workflow files, or Xcode project files. Therefore, no SwiftPM lockfile or package-reference policy condition applies. Full details: Cmux Swift LoggingExplanation PASS. The PR diff adds or materially changes no logging. The production changes contain only teardown logic, navigation state, and comments. The added tests and comments contain no Full details: Cmux User-Facing Error PrivacyExplanation PASS: The complete PR range changes connection-state retention and navigation behavior, plus developer comments and tests. No production user-facing error, alert, command output, API error body, or recovery copy is added or materially changed. The only added Swift string literals are test fixtures such as workspace and Mac identifiers, which the rule explicitly allows. The changed comments are developer-only comments. Full details: Cmux Full InternationalizationExplanation PASS: The PR adds no user-facing Swift text, web copy, metadata, or localization/catalog changes. Production additions are connection-state and navigation logic plus developer-only comments. The new Swift strings are confined to regression-test fixtures, which the rule allows. The diff changes only six Swift source/test files and adds no locale or string-catalog entries. Full details: Cmux Swiftui State LayoutExplanation PASS. The SwiftUI diff only adds the Full details: Cmux Architecture RethinkExplanation PASS. The diff applies a small store correctness fix with an explicit invariant: transport teardown retains Full details: Cmux Swift Auxiliary Window Close ShortcutsExplanation PASS. The pull request changes only connection-state retention, workspace navigation policy, and tests. The complete diff adds or modifies no NSWindow, NSPanel, NSWindowController, SwiftUI Window/WindowGroup, close-shortcut routing, or auxiliary-window identifier code. WorkspaceShellView remains a main workspace view, which the rule allows. The new window-specific tests are not present. Full details: Cmux Source ArtifactsExplanation PASS: The aggregate diff changes six Swift source/test paths only. The new Full details: Cmux No Test Or Debug Seam In Production SourceExplanation PASS: The PR adds no test/debug seam to production Swift source. The production diff only changes teardown behavior/comments, adds the private Full details: Cmux No Ambient Global StateExplanation PASS: The PR diff adds no ambient global state in production Swift.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
This branch was successfully deployed
1 participant
On iPhone (iroh, latest INTERNAL/NIGHTLY), the moment a reconnect began the mounted workspace detail was exited. Reproduced at the store level by the new regression tests (commit 1, red): recovery's redial teardown dropped every secondary Mac's
workspacesByMacentry, which flips multi-Mac row-id scoping, changes every derived row id's raw value, remaps the selection, and re-keys the pushed navigation route (destroying the mounted detail); a failed first dial then re-entered the teardown withforegroundMacDeviceIDalready nil, so the retention filter keyed on the anonymous sentinel and wiped the whole list.The fix is the general invariant the symptom kept violating: connection state changes never terminate or invalidate the user's current context.
clearRemoteConnectionContextno longer removes workspace rows. Teardown is a transport event; rows are removed only by authoritative events (healthy list reconcile, unpair/hide in the secondary reconcile pass, sign-out, team change), all of which already do their own explicit removal. All retained entries downgrade to.unavailableso the Computers dots and list chrome stay truthful, the teardown is idempotent (a second clear is a status no-op), and the per-Mac entry count stays stable so row-id scoping and route identity survive reconnects. The post-reconnect secondary refresh re-syncs the retained rows.WorkspaceShellCompactNavigationPolicytakeslistIsAuthoritative. While the list is reconnecting/disconnected, a cleared selection or a list hole keeps the mounted detail rendering its last-known snapshot (rename dialogs and sheets survive because the detail view identity survives); deletions confirmed by a healthy list still pop normally.Commit 1 adds the failing store tests only (CI red); commit 2 adds the fix plus policy tests (CI green). Full
CmuxMobileWorkspacesuite passes (45/45); fullCmuxMobileShellsuite run included in the verification pass.HIG: Feedback — status feedback should reach people "without having to take action or leave their current context"; only alerts may deliberately disrupt context. Transient connectivity is status, so it is shown in place (existing title spinner from #10821) instead of ejecting the user.
Follow-up (out of scope): genuine Mac removal while N→1 still flips row-id scoping and re-keys a mounted route; making scoping sticky would remove that last identity change, but it changes persisted-selection semantics and deserves its own pass.
Summary by cubic
Fixes iOS reconnects so reconnecting never exits the user's workspace or invalidates their current task. Connection teardown no longer removes workspace rows or retargets the selection, and compact navigation keeps the mounted detail while the list is reconnecting or disconnected.
Store
clearRemoteConnectionContextkeeps allworkspacesByMacrows, downgrading their status to.unavailableso the Computers dots and list chrome stay truthful.Navigation
WorkspaceShellCompactNavigationPolicytakeslistIsAuthoritative; while the list is reconnecting or disconnected, a cleared selection or list hole keeps the mounted detail on its last-known snapshot.Written for commit a0c781b. Summary will update on new commits.
Summary by CodeRabbit