Repository navigation
Fix disabled iOS controls after forgetting a computer - #10010
Conversation
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
📝 WalkthroughWalkthroughAuthenticated iOS shell routing retains the shell required by active or dismissing child presentations. The change adds child-host derivation, shell-surface retention, root-view integration, and tests for reconnection, restoration, pairing, and final computer removal. ChangesChild Host Shell Retention
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant cmuxUITests
participant UITestConfig
participant CMUXMobileRootScene
participant CMUXMobileShellStore
participant CMUXMobileRootView
cmuxUITests->>UITestConfig: enable successful computer forget
UITestConfig-->>CMUXMobileRootScene: resolve successful forget stub
CMUXMobileRootScene->>CMUXMobileShellStore: inject forget implementation
cmuxUITests->>CMUXMobileShellStore: forget final saved computer
CMUXMobileShellStore-->>CMUXMobileRootView: show disconnected shell
CMUXMobileRootView->>CMUXMobileRootView: retain required child host
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/cmuxPackage/Sources/cmuxFeature/CMUXMobileRootScene.swift`:
- Around line 23-35: Move SuccessfulComputerForgetUITestStub out of
CMUXMobileRootScene.swift into a dedicated debug-only source file or folder,
preserving its MobileIrohMacForgetting conformance and behavior. Remove the
test-only `#if` DEBUG seam from the production root-scene file so
CMUXMobileRootScene.swift contains only runtime composition.
In `@ios/cmuxUITests/cmuxUITests.swift`:
- Around line 2474-2478: Update the status wait in the test around
MobileTerminalMacConnectionStatus to verify a post-disconnect state rather than
only element existence. Wait for the element, then poll its label or hint until
it indicates “Reconnecting” or “Disconnected,” and assert that state before
proceeding with the Forget operation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5031905f-fdb5-40b0-8e70-498cde51a8f4
📒 Files selected for processing (3)
Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/UITestConfig.swiftios/cmuxPackage/Sources/cmuxFeature/CMUXMobileRootScene.swiftios/cmuxUITests/cmuxUITests.swift
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. |
c7b125a to
79f4020
Compare
Summary:
Why:
Forgetting the final saved computer refreshes the store to the disconnected shell while the Computers sheet is still presented. That unmounts the sheet presenter before onDismiss can release the shared modal slot, so Settings and Add Computer requests are rejected forever.
Verification:
The two fix runs are shown as failed because the workflow zero-test guard only recognizes XCTest summaries. The selected Swift Testing suites executed and passed in the job logs.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Prevents disabled iOS toolbar controls after forgetting the final computer by moving the Computers screen to a root-owned modal and keeping its presenter mounted through dismissal. Previously a workspace-owned presenter unmounted during the final-forget transition and orphaned the modal; now Done/Add/Forget route through the root so Settings and Add Computer remain responsive.
.computerstoMobileRootPresentationStatewithpresentComputers/dismissComputers;CMUXMobileRootViewpresents/dismisses, selects workspaces, and dismisses after a final forget.WorkspaceShellView/WorkspaceShellHost/WorkspaceListViewreplace localdeviceTreePresentationwithshowComputers(previews fall back to the local sheet).DeviceTreeViewaddsdismissActionanddidForgetComputer; Done/Add/Forget dismiss via the root owner. Tests cover Computers ownership, transition to pairing, and regressions for Settings/Add Computer after deleting the last computer. DEBUG-onlySuccessfulComputerForgetUITestStubenables UI tests without external revoke; mock runs suppress push authorization prompts.Written for commit 79f4020. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests