Skip to content

fix(ios): Add Computer sheets never appeared on Iroh setups - #11022

Merged
azooz2003-bit merged 4 commits into
mainfrom
fix-ios-add-computer-sheet
Aug 28, 2026
Merged

azooz2003-bit merged 4 commits into
mainfrom
fix-ios-add-computer-sheet

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Aug 27, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

On an Iroh/Auto-Connect setup, tapping Add Computer anywhere on iOS (the Computers sheet's toolbar +, its end-of-list row, the disconnected shell's +) dismissed the covering sheet and presented nothing. Screen recording from dogfood (08-27): the Computers sheet just closes, no pairing form.

Cause

#9957 gated Add Computer to Tailscale twice: at render time (addComputerAction returned nil) and as a runtime re-check inside showAddDevice(). #10437 deliberately removed the render gate — Add Computer is always available, only the Tailscale pairing-code scanner keeps a method gate — but left the runtime re-check behind. So on tailscaleSetupStatus == .notSelected the affordance rendered, the tap silently returned, and DeviceTreeView.addComputer() still dismissed the Computers sheet.

Fix

Remove the stale currentlyAllowsManualPairing guard from showAddDevice() only. The scanner entrypoints keep their gate. The root presentation state machine already sequences the sheet handoff (.dismissingChild(child, pendingPairing:) → present pairing on childDidDismiss) once the request is allowed through, matching the HIG Sheets guidance to close the first sheet before displaying the one it triggers (https://developer.apple.com/design/human-interface-guidelines/sheets, "Display only one sheet at a time"; page consulted for this change). No new UI, no string changes (localization audit: none needed).

Tests

Two-commit regression pattern: commit 1 updates cmuxUITests.testAutomaticConnectionMethodPresentsAddComputer (renamed from ...HidesAddComputer, which still encoded the pre-#10437 hidden policy) and extends testAutomaticAttachVersionApprovalDoesNotExposeManualPairing to tap Add Computer inside the Computers sheet and require the manual form to appear — red without the fix. Commit 2 is the fix.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes Add Computer on iOS never presenting its pairing form on Iroh/Auto-Connect setups — the tap dismissed the covering sheet and silently did nothing.

  • Removes the stale runtime re-check in showAddDevice(); the Tailscale scanner entrypoints keep their method gate.
  • Updates cmuxUITests to tap Add Computer under Auto-Connect and from the Computers sheet, asserting the manual pairing form presents and cancels.
  • Drops the empty-state copy and Settings-scanner assertions from the Auto-Connect test, which failed on main's mock lane for unrelated reasons.

Written for commit eaf542c. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • The Add Computer option now consistently opens the manual pairing form across connection methods.
    • Prevented visible Add Computer controls from becoming unresponsive in certain setups.
  • Tests

    • Expanded coverage for opening and dismissing the manual pairing form from the toolbar and Computers screen.
    • Clarified scanner availability separately from manual pairing.

azooz2003-bit and others added 2 commits August 27, 2026 16:27
Reproduces the report that Add Computer sheets never appear on iOS: on
an Iroh/Auto-Connect setup, tapping Add Computer (Computers sheet + or
row, disconnected-shell toolbar +) dismissed the covering sheet and
presented nothing.

Updates the two tests that still encoded the pre-#10437 policy (Add
Computer hidden outside Tailscale) to the current policy from #10437
(always available; only the Tailscale scanner keeps a method gate), and
extends them to tap the affordance and require the manual pairing form
to actually appear.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR #9957 gated Add Computer to Tailscale twice: at render time
(addComputerAction returned nil) and as a runtime re-check inside
showAddDevice(). PR #10437 deliberately removed the render gate — Add
Computer is always available, only the Tailscale pairing-code scanner
keeps a method gate — but left the runtime re-check behind. On Iroh
setups (tailscaleSetupStatus == .notSelected) every Add Computer
entrypoint therefore rendered but silently no-opped: the Computers
sheet dismissed itself and the pairing form never presented.

Remove the re-check from showAddDevice() only; the scanner entrypoints
keep theirs. The root presentation state machine already sequences the
sheet handoff (dismissingChild -> pendingPairing) once the request is
allowed through.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 27, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c5873589-32e8-4e01-ab09-840504b71fdb

📥 Commits

Reviewing files that changed from the base of the PR and between 41cbcd3 and eaf542c.

📒 Files selected for processing (1)
  • ios/cmuxUITests/cmuxUITests.swift

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


📝 Walkthrough

Walkthrough

The manual Add Computer entrypoint now presents the pairing form without the manual-pairing gate. Scanner entrypoints retain the gate. UI tests cover toolbar and Computers sheet access.

Changes

Manual pairing availability

Layer / File(s) Summary
Manual entrypoint and UI coverage
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift, ios/cmuxUITests/cmuxUITests.swift
showAddDevice() always presents manual pairing. UI tests verify access from the toolbar and Computers sheet, form presentation, and dismissal. Test documentation reflects the updated behavior.

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

Merge Risk: ⚪ Minimal · up to eaf54

The PR makes manual Add Computer present correctly on non-Tailscale setups while retaining the scanner restriction. No actionable merge-blocking risk remains, and the change is ready after normal checks and review.

Suggested reviewers: lawrencecchen

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary fix: Add Computer sheets now appear on Iroh setups.
Description check ✅ Passed The description provides detailed problem, cause, fix, and testing information. It is mostly complete, although it does not use the repository template headings and omits the demo video and checklist …
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.
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 production diff only removes the pre-existing currentlyAllowsManualPairing guard in showAddDevice() and adds documentation. It adds no actor annotations, Sendable types, background tas…
Cmux Swift Blocking Runtime ✅ Passed PASS: The only production Swift change adds a comment and removes the currentlyAllowsManualPairing guard from showAddDevice(). It introduces no semaphore, blocking wait, sleep, delayed dispatch, p…
Cmux Browser Automation Off-Main ✅ Passed PASS. The pull request changes only iOS UI code and iOS UI tests. Sources/TerminalController.swift and `Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.…
Cmux Expensive Synchronous Load ✅ Passed PASS: The production diff only removes the currentlyAllowsManualPairing guard from showAddDevice() and adds a comment. It does not add or move any agent-history loader, file read, directory scan, …
Cmux Cache Substitution Correctness ✅ Passed PASS: The production diff only removes the currentlyAllowsManualPairing guard from showAddDevice() and keeps the scanner guards. It does not replace an authoritative read with a cache, and it does…
Cmux No Hacky Sleeps ✅ Passed PASS. The pull request changes only Swift files: CMUXMobileRootView.swift and cmuxUITests.swift. The production change removes a Swift guard and adds a comment. The test changes use XCTest existen…
Cmux Algorithmic Complexity ✅ Passed PASS: The production diff changes only CMUXMobileRootView.showAddDevice() by removing a boolean guard and calling the existing presentPairing(.manual) path. It adds no loop, collection scan, sort,…
Cmux Swift Concurrency ✅ Passed PASS. The PR diff changes CMUXMobileRootView.showAddDevice() by removing a pairing-method guard and adding a comment. It does not add or expand DispatchQueue, Combine, completion-handler APIs, or …
Cmux Swift @Concurrent ✅ Passed PASS. The PR changes showAddDevice() only as a synchronous function by removing a guard. It does not add or alter async, nonisolated, @concurrent, actor isolation, or heavy async work. The sca…
Cmux Swift Package Boundaries ✅ Passed PASS: The only production change removes one guard from CMUXMobileRootView.showAddDevice() and adds a comment. This is SwiftUI presentation routing in CmuxMobileShellUI, an existing SwiftPM target…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The PR changes only CMUXMobileRootView.swift and ios/cmuxUITests/cmuxUITests.swift. It changes no Package.swift, Package.resolved, .gitignore, Xcode project, workspace, or workflow fil…
Cmux Swift Logging ✅ Passed PASS — The cumulative diff changes only CMUXMobileRootView.swift and ios/cmuxUITests/cmuxUITests.swift. The production change removes a guard and adds comments; it adds no print, debugPrint, `…
Cmux User-Facing Error Privacy ✅ Passed PASS: The PR changes only pairing presentation logic in production and a developer comment. It adds no user-facing error, alert, command output, API error body, or recovery copy. The added “Iroh” and …
Cmux Full Internationalization ✅ Passed PASS: The PR changes one production Swift function by removing a gate and adding a developer-only comment. It does not add or modify user-facing text, localization keys, catalogs, web messages, or loc…
Cmux Swiftui State Layout ✅ Passed PASS: The production diff only removes the currentlyAllowsManualPairing guard from showAddDevice() and adds a comment. The UI diff changes XCTest coverage only. No new ObservableObject, `@Publis…
Cmux Architecture Rethink ✅ Passed PASS. The production diff removes one stale guard from showAddDevice() and leaves the existing root presentation owner unchanged. showAddDevice() still routes through presentPairing(.manual) and…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS — The Swift diff changes CMUXMobileRootView.showAddDevice() and iOS UI tests only. The changed code routes through the existing SwiftUI .sheet presentation state machine. It adds no `NSWindow…
Cmux Source Artifacts ✅ Passed PASS: The PR changes only Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift and ios/cmuxUITests/cmuxUITests.swift. The diff adds no artifact paths, artifact-like ex…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The PR changes one production Swift file, Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift. Its only code change removes a pre-existing guard from `showAddDevi…
Cmux No Ambient Global State ✅ Passed PASS: The production Swift diff only adds a comment and removes a guard from private func showAddDevice() in CMUXMobileRootView. The function remains inside struct CMUXMobileRootView; the diff a…
Full details: Description check

Explanation

The description provides detailed problem, cause, fix, and testing information. It is mostly complete, although it does not use the repository template headings and omits the demo video and checklist sections.

Full details: Cmux Swift Actor Isolation

Explanation

PASS. The production diff only removes the pre-existing currentlyAllowsManualPairing guard in showAddDevice() and adds documentation. It adds no actor annotations, Sendable types, background tasks, or cross-actor calls. The existing presentPairing(.manual) UI path remains unchanged. The other changed file is ios/cmuxUITests/cmuxUITests.swift, which is test code. Therefore the diff does not introduce or materially worsen the specified Swift actor-isolation mistakes.

Full details: Cmux Swift Blocking Runtime

Explanation

PASS: The only production Swift change adds a comment and removes the currentlyAllowsManualPairing guard from showAddDevice(). It introduces no semaphore, blocking wait, sleep, delayed dispatch, polling loop, synchronous main-queue call, or manual lock. The other changes are UI-test-only scaffolding.

Full details: Cmux Browser Automation Off-Main

Explanation

PASS. The pull request changes only iOS UI code and iOS UI tests. Sources/TerminalController.swift and Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swift are unchanged. The diff contains no browser.* socket commands, WebKit waits, worker routing, or policy coverage changes, so the custom check does not apply.

Full details: Cmux Expensive Synchronous Load

Explanation

PASS: The production diff only removes the currentlyAllowsManualPairing guard from showAddDevice() and adds a comment. It does not add or move any agent-history loader, file read, directory scan, syscall, or JSON parsing. showAddDevice() only calls the existing presentPairing(.manual) state transition. The other changed file is ios/cmuxUITests/cmuxUITests.swift, which is test code and contains no expensive loader change.

Full details: Cmux Cache Substitution Correctness

Explanation

PASS: The production diff only removes the currentlyAllowsManualPairing guard from showAddDevice() and keeps the scanner guards. It does not replace an authoritative read with a cache, and it does not change persistence, history, undo, or snapshot behavior. The UI-test edits are not production changes. The cache-substitution failure condition is therefore not applicable.

Full details: Cmux No Hacky Sleeps

Explanation

PASS. The pull request changes only Swift files: CMUXMobileRootView.swift and cmuxUITests.swift. The production change removes a Swift guard and adds a comment. The test changes use XCTest existence waits, not fixed sleep or timer primitives. The runtime-no-hacky-sleeps check applies only to non-Swift TypeScript, JavaScript, shell, and build/runtime scripts, so no failure condition is introduced.

Full details: Cmux Algorithmic Complexity

Explanation

PASS: The production diff changes only CMUXMobileRootView.showAddDevice() by removing a boolean guard and calling the existing presentPairing(.manual) path. It adds no loop, collection scan, sort, filter, join, batch action, or slower algorithm. The other changed file is ios/cmuxUITests/cmuxUITests.swift, which is test-only and exempt by the rule.

Full details: Cmux Swift Concurrency

Explanation

PASS. The PR diff changes CMUXMobileRootView.showAddDevice() by removing a pairing-method guard and adding a comment. It does not add or expand DispatchQueue, Combine, completion-handler APIs, or fire-and-forget Task work. The UI test changes add XCTest interactions and assertions only. Existing Task and queue occurrences remain unchanged, and the repository rule explicitly allows XCTest and existing nearby legacy code.

Full details: Cmux Swift `@Concurrent`

Explanation

PASS. The PR changes showAddDevice() only as a synchronous function by removing a guard. It does not add or alter async, nonisolated, @concurrent, actor isolation, or heavy async work. The scanner guards remain unchanged. The UI test remains @MainActor; its existing async server setup is not changed. No stated concurrency failure condition is introduced.

Full details: Cmux Swift Package Boundaries

Explanation

PASS: The only production change removes one guard from CMUXMobileRootView.showAddDevice() and adds a comment. This is SwiftUI presentation routing in CmuxMobileShellUI, an existing SwiftPM target with its own test target. It does not introduce reusable domain, provider, protocol, persistence, or independently testable logic in an app-target root. The change fits the rule's allowed UI and app-lifecycle composition cases.

Full details: Cmux Swiftpm Lockfiles

Explanation

PASS: The PR changes only CMUXMobileRootView.swift and ios/cmuxUITests/cmuxUITests.swift. It changes no Package.swift, Package.resolved, .gitignore, Xcode project, workspace, or workflow file. The changed package's .gitignore contains only .build/, and its Package.resolved is tracked. Therefore no SwiftPM dependency or Xcode package-reference change requires a lockfile diff.

Full details: Cmux Swift Logging

Explanation

PASS — The cumulative diff changes only CMUXMobileRootView.swift and ios/cmuxUITests/cmuxUITests.swift. The production change removes a guard and adds comments; it adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or secret-bearing diagnostics. Existing print calls are confined to the UI test file and are unchanged. The logging check therefore finds no introduced or materially changed violation.

Full details: Cmux User-Facing Error Privacy

Explanation

PASS: The PR changes only pairing presentation logic in production and a developer comment. It adds no user-facing error, alert, command output, API error body, or recovery copy. The added “Iroh” and “Tailscale” references are in comments and tests, which the rule explicitly allows. The existing pairing error plumbing is unchanged.

Full details: Cmux Full Internationalization

Explanation

PASS: The PR changes one production Swift function by removing a gate and adding a developer-only comment. It does not add or modify user-facing text, localization keys, catalogs, web messages, or locale data. The other changed file is an XCTest file; its comments and assertion messages are explicitly allowed test/developer text. The full diff from the PR base contains only these two files, and git diff --check is clean.

Full details: Cmux Swiftui State Layout

Explanation

PASS: The production diff only removes the currentlyAllowsManualPairing guard from showAddDevice() and adds a comment. The UI diff changes XCTest coverage only. No new ObservableObject, @Published, @StateObject, @EnvironmentObject, GeometryReader, lazy/list row store reference, or render-time state mutation is introduced. Existing @State and @Bindable declarations in CMUXMobileRootView are unchanged.

Full details: Cmux Architecture Rethink

Explanation

PASS. The production diff removes one stale guard from showAddDevice() and leaves the existing root presentation owner unchanged. showAddDevice() still routes through presentPairing(.manual) and handleRootPresentation; the state machine already represents the child-dismissal handoff with dismissingChild(..., pendingPairing:) and then pairing(...). All Add Computer surfaces use the shared addComputerAction, while scanner entrypoints retain their method guards. The PR adds no sleeps, delayed dispatch, polling, locks, observers, mutable state, side channels, duplicate action wiring, or new UI lifecycle owner. The other changes are UI-test synchronization and assertions, which the rule allows.

Full details: Cmux Swift Auxiliary Window Close Shortcuts

Explanation

PASS — The Swift diff changes CMUXMobileRootView.showAddDevice() and iOS UI tests only. The changed code routes through the existing SwiftUI .sheet presentation state machine. It adds no NSWindow, NSPanel, NSWindowController, Window, or WindowGroup, and it adds no window identifier or close-shortcut handling. This is an allowed sheet change.

Full details: Cmux Source Artifacts

Explanation

PASS: The PR changes only Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift and ios/cmuxUITests/cmuxUITests.swift. The diff adds no artifact paths, artifact-like extensions, generated logs, screenshots, recordings, caches, build output, or scratch directories. Both changed files are intentional product source and UI test files, which the rule explicitly permits.

Full details: Cmux No Test Or Debug Seam In Production Source

Explanation

PASS. The PR changes one production Swift file, Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CMUXMobileRootView.swift. Its only code change removes a pre-existing guard from showAddDevice() and adds explanatory comments. It adds no #if DEBUG test/debug member, test hook, debug-named accessor, or visibility widening. Existing debug blocks are unchanged. The other changed file is UI test code under ios/cmuxUITests/, not a production Sources/ path.

Full details: Cmux No Ambient Global State

Explanation

PASS: The production Swift diff only adds a comment and removes a guard from private func showAddDevice() in CMUXMobileRootView. The function remains inside struct CMUXMobileRootView; the diff adds no file-scope function or mutable variable, static namespace, or singleton. The UI test changes are outside the production Swift scope of this check.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-ios-add-computer-sheet

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.

azooz2003-bit and others added 2 commits August 27, 2026 21:38
…ction

The empty-description recheck after cancelling the pairing form depends
on post-cancel shell state and flaked in CI; assert the copy first (as
the test originally did), then exercise the Add Computer regression tap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The inherited empty-state copy and Settings-scanner assertions predate
the current mock-data shell and fail on main's broken mock lane for
reasons unrelated to this fix; keep the test on its regression contract
(tap Add Computer -> manual form presents -> cancel dismisses).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@azooz2003-bit
azooz2003-bit merged commit 2c6fd70 into main Aug 28, 2026
6 checks passed
rustybret pushed a commit to rustybret/bmux that referenced this pull request Aug 28, 2026
8910e63 cmux-tui: index cached surface exits (manaflow-ai#11000)
ed19cfa ios: reserve unread badge overflow before the group header chevron (manaflow-ai#11018)
c1e7f09 Fix premature Codex completion notifications (manaflow-ai#10838)
2c6fd70 fix(ios): Add Computer sheets never appeared on Iroh setups (manaflow-ai#11022)
8d71d72 fix(cmux-tui): surface remote transport loss instead of impersonating an empty session (manaflow-ai#11045)
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