Retiring a recoverable window route recurses until the stack overflows

[ejc3]

Summary

Retiring a recoverable window route can recurse until the stack overflows. One crash report shows 511 frames of tabManagerFor(tabId:) → contextContainingTabId → recoverableRouteWorkspaceIdsForRemoteTeardown → retireRecoverableMainWindowRouteIfCurrent → recoverableMainWindowRoute(windowId:) → validatedRecoverableMainWindow → windowForMainWindowId → recoverableMainWindowRouteSnapshot(for:), repeating.

recoverableMainWindowRouteSnapshot(for:) resolved the route's window before checking whether the route's tab manager can still own it. Resolving the window goes through validatedRecoverableMainWindow to recoverableMainWindowRoute(windowId:), which retires a route whose manager can no longer own it. Retirement filters the route's workspaces through tabManagerFor(tabId:), which snapshots orphaned routes again, so the cycle repeated.

The snapshot now checks the manager before resolving the window. A route whose manager cannot own it has no snapshot either way, so the result is unchanged, and looking up a workspace's tab manager no longer retires routes as a side effect.

Testing

• In a full run of the unit tests at main 4638e5b1ea plus the compile fixes in Fix the compile errors that keep main and its unit tests from building #12584, through scripts/ci/run-app-host-xcodebuild.sh in 12 batches the way CI runs app-host tests, one crash report shows 511 frames of this recursion.
• The app and the unit-test target build with this change on an EC2 Mac (Xcode 26.6, macOS 15.7). I could not tie the crash to one test, so there is no focused reproduction.
• The same 12 batches with all seven crash fixes applied, run again after the review changes: macOS saved one crash report during the run, for an unrelated index-out-of-range, and none for this recursion.

Demo Video

Not applicable. The change prevents a crash and has no visible effect.

Review Trigger (Copy/Paste as PR comment)

@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review

Checklist

• I tested the change locally
• I added or updated tests for behavior changes
• I updated docs/changelog if needed
• I requested bot reviews after my latest commit (copy/paste block above or equivalent)
• All code review bot comments are resolved
• All human review comments are resolved

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes a stack overflow crash when retiring a recoverable window route. The snapshot now validates the route's tab manager before resolving the window, breaking the recursion cycle. Route lookup no longer retires routes as a side effect; the snapshot result is unchanged.

^{Written for commit 2a8d6dc. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Improved recoverable window route handling by checking ownership eligibility before resolving a window.
  • Prevents routes from being resolved and retired when their manager cannot own them.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b6a71961-59ec-4d16-8348-aa57bb744200

📥 Commits

Reviewing files that changed from the base of the PR and between 708c758 and 55c8142.

📒 Files selected for processing (1)

• Sources/AppDelegate+RecoverableMainWindowRoutes.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The recoverable main-window route snapshot now checks tab-manager ownership before resolving the cached window. Non-ownable routes return nil without resolving or retiring their window.

Changes

Recoverable route ownership

Layer / File(s)	Summary
Ownership guard before window resolution Sources/AppDelegate+RecoverableMainWindowRoutes.swift	recoverableMainWindowRouteSnapshot(for:) returns nil when the tab manager cannot own the route. Window resolution now occurs only after this check.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 55c81

The ownership guard prevents the recursion path while preserving live routing for eligible managers; no merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the primary change: preventing stack overflow caused by recursive retirement of recoverable window routes.
Description check	✅ Passed	The description is complete and directly explains the cause, fix, testing, and expected behavior. It includes all template sections. The behavior-test, bot-review, and review-resolution checklist item…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	PASS. The PR changes only the control flow in recoverableMainWindowRouteSnapshot(for:); it adds no model, service protocol, Sendable reference type, or background access. AppDelegate, `Recoverab…
Cmux Swift Blocking Runtime	✅ Passed	PASS. The pull request changes one production Swift file and only reorders manager validation before window resolution, adds a comment, and preserves the existing return flow. The added lines introduc…
Cmux Browser Automation Off-Main	✅ Passed	PASS: The PR changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift. The diff reorders recoverable route manager validation before window resolution. It does not change browser.* socke…
Cmux Expensive Synchronous Load	✅ Passed	PASS. The PR changes only recoverableMainWindowRouteSnapshot(for:). It moves the existing windowForMainWindowId lookup after the manager-ownership guard. The diff adds no agent-history loader, fil…
Cmux Cache Substitution Correctness	✅ Passed	PASS. The only changed production file reorders the existing manager-ownership guard ahead of route.window ?? windowForMainWindowId(route.windowId) in recoverableMainWindowRouteSnapshot(for:). It …
Cmux No Hacky Sleeps	✅ Passed	PASS: The authoritative PR diff changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift. The change is Swift, and the custom check applies only to production non-Swift app/runtime changes…
Cmux Algorithmic Complexity	✅ Passed	PASS: The only production change is in Sources/AppDelegate+RecoverableMainWindowRoutes.swift:79-97. It adds an O(1) TabManager ownership guard before window resolution and removes a potentially re…
Cmux Swift Concurrency	✅ Passed	PASS. The PR changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift. The diff adds a synchronous manager guard and moves window resolution after that guard. It adds no DispatchQueue, C…
Cmux Swift @Concurrent	✅ Passed	The PR changes only the synchronous recoverableMainWindowRouteSnapshot(for:) implementation. The diff adds no async, nonisolated, or @concurrent declaration and does not change async call site…
Cmux Swift Package Boundaries	✅ Passed	PASS: The authoritative diff changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift. It reorders an existing manager-ownership guard before windowForMainWindowId(...); it does not intr…
Cmux Swiftpm Lockfiles	✅ Passed	PASS. The reviewed range changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift (+11/-6). It changes route snapshot control flow and does not change Package.swift, Package.resolved, …
Cmux Swift Logging	✅ Passed	The pull request changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift. The added lines contain no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or `cmuxDebugLo…
Cmux User-Facing Error Privacy	✅ Passed	PASS: The pull request changes only internal recoverable-route snapshot control flow in one Swift file. The added text is a developer-only comment, and the diff adds no user-facing error, alert, comma…
Cmux Full Internationalization	✅ Passed	PASS: The PR changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift. The diff changes control flow and adds developer-only comments; it introduces no user-facing Swift text, localization…
Cmux Swiftui State Layout	✅ Passed	PASS: The PR changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift, an AppKit extension that imports AppKit and contains no SwiftUI view, state, layout, or render code. The diff only …
Cmux Architecture Rethink	✅ Passed	PASS. The authoritative diff changes only the ordering in recoverableMainWindowRouteSnapshot(for:) and adds a comment. It performs the existing tabManagerCanOwnRecoverableMainWindowRoute check bef…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS. The PR changes only recoverableMainWindowRouteSnapshot(for:) in Sources/AppDelegate+RecoverableMainWindowRoutes.swift. It reorders tab-manager validation before existing window lookup. The d…
Cmux Source Artifacts	✅ Passed	The PR changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift, an existing hand-written Swift source file. The diff contains source logic and comments only. No logs, media, temporary dir…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS. The PR changes only Sources/AppDelegate+RecoverableMainWindowRoutes.swift, which is production source, but the diff only reorders the existing manager check before window resolution and adds e…
Cmux No Ambient Global State	✅ Passed	PASS. The authoritative diff contains one modified Swift file. The only code change moves the existing tabManagerCanOwnRecoverableMainWindowRoute guard before window resolution inside the existing p…

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard.

[github-actions]

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

[teamleaderleo]

You actually had this one first! You opened it on Sep 14 and we ended up landing the same fix days later without spotting your PR, which is on us. Closing since main has it now, but the credit is yours. Thank you for tracking it down :)