Skip to content

Require Computer Use onboarding wiring - #13627

Merged
teamleaderleo merged 1 commit into
mainfrom
fix/computer-use-onboarding-wiring-followup
Sep 22, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
fix/computer-use-onboarding-wiring-followup

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

HostSettingsActions previously carried a default no-op Computer Use onboarding callback. Production patched that callback later from AppDelegate.configure(), while tests could inject it manually. That let the host wiring disappear without making construction fail.

Result

The onboarding action is now a required HostSettingsActions initializer dependency.

  • production supplies the real ComputerUseUXCoordinator.presentOnboardingFromSettings(startingAt:) route at composition time;
  • the mutable post-construction setter and silent no-op default are gone;
  • direct permission requests and the granted-permission refresh path exercise the injected action through executable behavior tests;
  • the source-text regression was removed.

The composition now makes the dependency explicit: a production HostSettingsActions instance cannot be created while omitting Computer Use onboarding routing.

Validation

Focused app-host/CI checks are running on the repaired head. No runtime behavior is intentionally changed; this turns an existing host connection into a required dependency and pins the behavior through executable paths.

Related: #13599.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed accessibility and screen-recording permission requests from settings so they open the appropriate computer-use onboarding step.
    • Improved the reliability of launching computer-use onboarding from settings.
  • Refactor

    • Updated settings actions to require a valid computer-use onboarding handler, ensuring requests are routed consistently.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 655d514d-072e-4662-a405-1e743b588af3

📥 Commits

Reviewing files that changed from the base of the PR and between 38ced03 and fffdc72.

📒 Files selected for processing (5)
  • Sources/AppDelegate.swift
  • Sources/HostSettingsActions.swift
  • Sources/cmuxApp.swift
  • cmuxTests/ComputerUseUXTests.swift
  • cmuxTests/HostSettingsShortcutNotificationTests.swift
💤 Files with no reviewable changes (1)
  • Sources/AppDelegate.swift

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


📝 Walkthrough

Walkthrough

HostSettingsActions now receives its computer-use onboarding callback during initialization. cmuxApp supplies the callback, the old setter wiring is removed, and tests verify accessibility and screen-recording onboarding starting points.

Changes

Computer-use onboarding wiring

Layer / File(s) Summary
Required onboarding action contract
Sources/HostSettingsActions.swift
The onboarding callback is now a required immutable initializer dependency. The setter and default no-op closure are removed.
Application onboarding wiring
Sources/cmuxApp.swift, Sources/AppDelegate.swift
cmuxApp routes the callback to presentOnboardingFromSettings(startingAt:). The previous AppDelegate settings handler is removed.
Onboarding routing validation
cmuxTests/ComputerUseUXTests.swift, cmuxTests/HostSettingsShortcutNotificationTests.swift
Tests verify accessibility and screen-recording starting points. Test helpers provide the required initializer callback.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to fffdc

This change wires Settings-triggered computer-use onboarding through application construction and adds coverage for its starting points. No remaining merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 23 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description explains the problem and resulting dependency change, but it does not follow the repository template. It omits the required Summary, Testing, Demo Video, Review Trigger, and Checklist … Update the description to use the required template sections. Add concrete completed test commands and results, provide a demo video or attachment for this behavior change, include the review-trigger comment block, and complete the checklis…
✅ Passed checks (23 passed)
Check name Status Explanation
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 Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request changes only Computer Use Settings wiring and related tests in five Swift files. The diff introduces no Cloud terminal creation, cmux-tui transport, manual pane, PTY readiness, …
Cmux Swift Actor Isolation ✅ Passed PASS. The production diff preserves explicit MainActor isolation. HostSettingsActions remains @MainActor, and its new stored onboarding callback is an @MainActor closure. The wiring targets `App…
Cmux Swift Blocking Runtime ✅ Passed PASS. The diff only changes onboarding callback injection and adds deterministic test assertions. It does not add semaphores, blocking waits, sleeps, delayed dispatch, polling, main-queue synchronizat…
Cmux Browser Automation Off-Main ✅ Passed The check is not triggered. The PR changes only Computer Use onboarding wiring and related tests in five non-browser files. No changed path is Sources/TerminalController.swift or `ControlCommandExec…
Cmux Expensive Synchronous Load ✅ Passed PASS: The production diff only changes Computer Use onboarding callback wiring. It removes the setter, injects a @MainActor closure, and routes to the existing `presentOnboardingFromSettings(startin…
Cmux Cache Substitution Correctness ✅ Passed The production diff only changes Computer Use onboarding callback wiring. It replaces a mutable no-op callback plus setter with an initializer-injected closure and routes it to `presentOnboardingFromS…
Cmux No Hacky Sleeps ✅ Passed PASS. The pull request changes only Swift files. The added production code wires an onboarding closure, and the added test code performs synchronous assertions. The authoritative diff introduces no sl…
Cmux Algorithmic Complexity ✅ Passed PASS. The production diff only changes Computer Use onboarding callback wiring in Sources/HostSettingsActions.swift, Sources/cmuxApp.swift, and Sources/AppDelegate.swift. It adds no loops, colle…
Cmux Swift Concurrency ✅ Passed PASS. The diff does not introduce background Dispatch queues, Combine state, fire-and-forget Tasks, or completion-handler async APIs. It replaces a stored no-op callback plus later setter with a requi…
Cmux Swift @Concurrent ✅ Passed PASS. The diff adds no @concurrent or nonisolated async declaration and does not change the isolation or call sites of HostSettingsActions.refreshComputerUsePermissions() async. The new onboardi…
Cmux Swift Package Boundaries ✅ Passed PASS. The production diff only changes app-level wiring: it injects a required onboarding callback into HostSettingsActions, removes the post-construction setter from AppDelegate, and connects the…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The authoritative diff changes only five Swift source/test files. It contains no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project package-reference changes. Therefore, no …
Cmux Swift Logging ✅ Passed PASS — The PR adds no logging statements or logging configuration. The production diff only injects the Computer Use onboarding closure and removes the old setter wiring. Existing Logger, StartupBread…
Cmux User-Facing Error Privacy ✅ Passed The production diff changes only Computer Use onboarding callback wiring and constructor requirements. It adds no user-facing error, alert, command output, API body, or recovery text. The callback pre…
Cmux Full Internationalization ✅ Passed PASS. The authoritative diff changes only onboarding callback wiring, initializer storage, and tests. It adds no user-facing Swift text and changes no app catalogs, Info.plist localization, web messag…
Cmux Swiftui State Layout ✅ Passed PASS. The authoritative diff changes AppKit host wiring and test setup only. It adds no SwiftUI view, ObservableObject, @Published, GeometryReader, lazy/list row store reference, or render-time state …
Cmux Architecture Rethink ✅ Passed PASS. The diff is a small dependency-injection correctness fix. It replaces a mutable optional callback and post-construction setter with a required let callback supplied at cmuxApp construction. …
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR does not add or materially change a standalone window. Its production changes only inject the existing onboarding callback and route it to presentOnboardingFromSettings; the added tests use c…
Cmux Source Artifacts ✅ Passed All five changed paths are hand-written Swift source or test files. The diff adds no logs, screenshots, recordings, caches, build output, dependency checkouts, hidden scratch directories, or broad art…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The production diff adds no test/debug seam. Sources/HostSettingsActions.swift adds a private let runComputerUseOnboardingAction callback, and its production methods invoke it for real Setti…
Title check ✅ Passed The title clearly describes the primary change: making Computer Use onboarding wiring a required dependency.
Full details: Description check

Explanation

The description explains the problem and resulting dependency change, but it does not follow the repository template. It omits the required Summary, Testing, Demo Video, Review Trigger, and Checklist sections, and it does not provide completed test results.

Resolution

Update the description to use the required template sections. Add concrete completed test commands and results, provide a demo video or attachment for this behavior change, include the review-trigger comment block, and complete the checklist. Reconcile the description with the actual implementation and PR objective if they differ more than intended.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

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

@github-actions

Copy link
Copy Markdown
Contributor

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

@cursor

cursor Bot commented Sep 22, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot 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.

@teamleaderleo teamleaderleo changed the title Fix Computer Use onboarding production wiring test: pin Computer Use onboarding production wiring Sep 22, 2026
@teamleaderleo
teamleaderleo force-pushed the fix/computer-use-onboarding-wiring-followup branch from 0437d0c to fffdc72 Compare September 22, 2026 09:42
@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 22, 2026 09:42
@teamleaderleo teamleaderleo changed the title test: pin Computer Use onboarding production wiring Require Computer Use onboarding wiring Sep 22, 2026
@teamleaderleo
teamleaderleo merged commit 121bf02 into main Sep 22, 2026
56 of 58 checks passed
Sign up for free to 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.

1 participant