Skip to content

Preserve Tailscale access across Nightly Iroh upgrades - #8457

Merged
4 commits merged into
mainfrom
feat-iroh-tailscale-skew
Jul 19, 2026
Merged

4 commits merged into
mainfrom
feat-iroh-tailscale-skew

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 19, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • preserve the historical legacy mobile listener default for dev and Nightly Macs when no preference has ever been written
  • keep explicit current and legacy preferences authoritative, including an explicit off
  • keep stable builds opt-in and retain the new iOS fail-closed rule for bearer tokens over raw Tailscale TCP

This is compatibility ingress only. An older iOS app can still reach an updated Nightly Mac, where the Mac performs same-account Stack authentication. A new iOS app still requires Iroh from a truly old Mac because that Mac has no cryptographic challenge that can attest a raw Tailscale route.

Verification

  • regression-only commit 9a688df060 fails before the policy overload exists
  • xcodebuild -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination "platform=macOS" -derivedDataPath /tmp/cmux-tsmigr test -only-testing:cmuxTests/MobileHostServiceSettingsTests CODE_SIGNING_ALLOWED=NO (14 passed)
  • swift test --package-path Packages/iOS/CmuxMobileTransport --filter CmxNetworkByteTransportFactorySecurityTests (5 passed)
  • swift test --package-path Packages/iOS/CmuxMobileShell --filter IrohReconnectRouteSelectionTests (24 passed)
  • cloud tagged build tsmigr succeeded: https://github.com/manaflow-ai/cmux/actions/runs/29676343291
  • tagged runtime reported the legacy listener running and published both IPv4 and IPv6 Tailscale routes

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Preserves Tailscale pairing access across Nightly Iroh upgrades by keeping the legacy mobile listener on for dev/nightly builds when no preference exists. Explicit settings still win, and stable builds remain opt-in.

  • Bug Fixes
    • Default listener depends on build flavor: dev/nightly on, stable off when no preference is set.
    • Explicit current or legacy preference overrides, including an explicit off.
    • Made build flavor explicit via isListeningEnabled(defaults:buildFlavor:) for deterministic behavior; added tests for nightly compatibility, explicit and legacy disables, and stable default.

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

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Fixed listener-enable behavior to consistently respect explicit current and legacy preferences.
    • Updated defaults by build type: nightly retains the prior behavior, while stable remains opt-in to avoid local network permission prompts.
  • Tests
    • Added/expanded unit tests for build-flavor-specific defaults, including nightly behavior, explicit true/false handling, legacy overrides, and stable-build opt-in.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

MobileHostService now resolves listener enablement from current or legacy preferences before applying build-flavor defaults. Dev and nightly builds remain enabled by default, while stable builds require explicit opt-in. Tests cover these defaults and overrides.

Changes

Listener enablement behavior

Layer / File(s) Summary
Build-flavor listener resolution and coverage
Sources/Mobile/MobileHostService.swift, cmuxTests/MobileHostServiceSettingsTests.swift
isListeningEnabled delegates to a BuildFlavor-aware overload, preserves current and legacy preference overrides, applies non-stable defaults when unset, and tests dev, nightly, and stable behavior.

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

Suggested reviewers: lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Cmux Swift Package Boundaries ❌ Error The PR adds a pure, testable listener-default policy overload inside app-target MobileHostService; that domain logic belongs in a shared SwiftPM value API instead. Extract the defaulting rule into Packages/Shared/CMUXMobileCore (or another small package) as a MobileHostListenerPolicy/similar value API, then delegate from the app target.
✅ Passed checks (24 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 only adds a nonisolated overload using Sendable BuildFlavor; no new MainActor model/protocol or background UI-store misuse appears.
Cmux Swift Blocking Runtime ✅ Passed The PR only changes listener-default logic and deterministic tests; no new waits, sleeps, syncs, polling, or locks were added.
Cmux Browser Automation Off-Main ✅ Passed Commit a05a0cc only changes cmuxTests/MobileHostServiceSettingsTests.swift; the browser-automation rule targets unrelated browser command files.
Cmux Expensive Synchronous Load ✅ Passed PR only changes MobileHostService preference defaults/tests; no agent-history loaders, JSON/trajectory scans, or main-actor interactive paths were added.
Cmux Cache Substitution Correctness ✅ Passed No cache substitution: the change uses BuildFlavor, not a cached state, and the listener default still honors explicit prefs with a documented build-flavor fallback.
Cmux No Hacky Sleeps ✅ Passed PR only changes Swift files, and the patch adds no sleep/delay/timer/polling primitives in covered runtime code.
Cmux Algorithmic Complexity ✅ Passed New overload is O(1): two UserDefaults lookups and one BuildFlavor comparison; tests are fixed-size and no scalable collection scans were added.
Cmux Swift Concurrency ✅ Passed The PR only changes listener-default logic/tests; it adds no new DispatchQueue, Combine, completion-handler, or fire-and-forget Task patterns.
Cmux Swift @Concurrent ✅ Passed Touched Swift changes are sync nonisolated settings helpers plus tests; no new/changed @concurrent, nonisolated async, or UI-hop issue appears.
Cmux Swiftpm Lockfiles ✅ Passed PR diff only changes Swift source/tests; no Package.swift, Package.resolved, .gitignore, Xcode project, or workflow files were touched.
Cmux Swift Logging ✅ Passed No new or changed print/debugPrint/NSLog/file-stdout logging appears in the PR diff; the Swift changes only alter listener-default logic and tests.
Cmux User-Facing Error Privacy ✅ Passed The diff only changes listener-default logic/comments and tests; no user-facing errors, alerts, API error bodies, or recovery copy were added.
Cmux Full Internationalization ✅ Passed The diff only changes listener-selection logic/tests and a developer comment; no user-facing text, catalogs, plist, or locale files were touched.
Cmux Swiftui State Layout ✅ Passed PR only changes a service and tests; no SwiftUI views, ObservableObject/@published, GeometryReader, lazy-list store refs, or render-time state writes appear in the diff.
Cmux Architecture Rethink ✅ Passed This is a small correctness fix: listener default now derives from explicit prefs or BuildFlavor, with tests only; no new timing, observer, lock, or split-ownership pattern was introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The PR only changes a MobileHostService test file; no NSWindow/NSPanel/WindowGroup code, cmux.* identifiers, or cmuxAuxiliaryWindowIdentifiers changes.
Cmux Source Artifacts ✅ Passed Only hand-written source/test files changed; no logs, temp folders, build output, or other source-control artifacts were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Production diff only changes isListeningEnabled defaulting via BuildFlavor; no #if DEBUG/test-hook names or visibility widening were added.
Cmux No Ambient Global State ✅ Passed The PR only adds a static overload inside MobileHostService and test coverage; no new top-level funcs, mutable globals, or singletons were introduced.
Title check ✅ Passed The title clearly matches the main change: preserving mobile listener/Tailscale access for Nightly Iroh compatibility.
Description check ✅ Passed It includes a clear Summary and Verification section with what changed, why, and test evidence; only template extras are missing.
✨ 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 feat-iroh-tailscale-skew

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.

@greptile-apps

greptile-apps Bot commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes the default behavior of MobileHostService.isListeningEnabled to be build-flavor-aware: dev and nightly builds default to true (preserving the historical listener that allows older iOS apps to reach an updated Mac over Tailscale), while stable builds remain opt-in (false). The change also adds a buildFlavor: parameter overload to make the policy deterministically testable without depending on the runtime bundle.

  • A new isListeningEnabled(defaults:buildFlavor:) overload replaces the previous SettingCatalog().mobile.iOSPairingHost.defaultValue fallback with buildFlavor != .stable, with the convenience wrapper always passing BuildFlavor.current.
  • Four new unit tests cover the nightly compatibility default, explicit disable via both the canonical and legacy key, and the stable opt-in default; an existing test was updated to pass buildFlavor: .dev explicitly so it no longer relies on runtime bundle detection agreeing with the catalog compile-time constant.

Confidence Score: 5/5

The change is narrowly scoped to the listener default policy and all three build flavors are covered by deterministic tests; no regressions were introduced.

The production logic is a straightforward three-branch fallback (canonical key → legacy key → build-flavor default). Explicit overrides continue to win regardless of flavor, the stable opt-in contract is unchanged, and the dev/nightly compatibility default is now tested directly rather than through runtime bundle detection. The stale-assertion issue raised by a previous review is resolved, and the previously missing legacy-key-false-on-nightly test has been added.

No files require special attention.

Important Files Changed

Filename Overview
Sources/Mobile/MobileHostService.swift Adds a build-flavor-aware isListeningEnabled(defaults:buildFlavor:) overload; logic is correct for all three flavors and explicit override precedence is preserved.
cmuxTests/MobileHostServiceSettingsTests.swift Four new tests added for nightly default, canonical disable, legacy disable, and stable default; existing test updated to drive buildFlavor explicitly instead of relying on runtime bundle detection.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["isListeningEnabled(defaults:)"] -->|"passes BuildFlavor.current"| B["isListeningEnabled(defaults:buildFlavor:)"]
    B --> C{"canonical key set?"}
    C -->|yes| D["return override (true/false)"]
    C -->|no| E{"legacy key set?"}
    E -->|yes| F["return legacyOverride (true/false)"]
    E -->|no| G{"buildFlavor != .stable?"}
    G -->|".dev or .nightly"| H["return true (listener on by default)"]
    G -->|".stable"| I["return false (opt-in only)"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["isListeningEnabled(defaults:)"] -->|"passes BuildFlavor.current"| B["isListeningEnabled(defaults:buildFlavor:)"]
    B --> C{"canonical key set?"}
    C -->|yes| D["return override (true/false)"]
    C -->|no| E{"legacy key set?"}
    E -->|yes| F["return legacyOverride (true/false)"]
    E -->|no| G{"buildFlavor != .stable?"}
    G -->|".dev or .nightly"| H["return true (listener on by default)"]
    G -->|".stable"| I["return false (opt-in only)"]
Loading

Reviews (2): Last reviewed commit: "test(mobile): make Tailscale flavor cove..." | Re-trigger Greptile

Comment on lines +66 to +90
@Test func nightlyPreservesLegacyListenerWhenNoSettingWasEverWritten() throws {
let suiteName = "MobileHostServiceSettingsTests.NightlyCompatibility.\(UUID().uuidString)"
let defaults = try #require(UserDefaults(suiteName: suiteName))
defer { defaults.removePersistentDomain(forName: suiteName) }

#expect(MobileHostService.isListeningEnabled(defaults: defaults, buildFlavor: .nightly))
}

@Test func explicitDisableWinsOverNightlyCompatibility() throws {
let suiteName = "MobileHostServiceSettingsTests.NightlyDisabled.\(UUID().uuidString)"
let defaults = try #require(UserDefaults(suiteName: suiteName))
defer { defaults.removePersistentDomain(forName: suiteName) }

defaults.set(false, forKey: MobileHostService.listeningEnabledDefaultsKey)

#expect(!MobileHostService.isListeningEnabled(defaults: defaults, buildFlavor: .nightly))
}

@Test func stableWithoutExplicitOptInKeepsLegacyListenerOff() throws {
let suiteName = "MobileHostServiceSettingsTests.StableDefault.\(UUID().uuidString)"
let defaults = try #require(UserDefaults(suiteName: suiteName))
defer { defaults.removePersistentDomain(forName: suiteName) }

#expect(!MobileHostService.isListeningEnabled(defaults: defaults, buildFlavor: .stable))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No test for legacy-key explicit-false in nightly

explicitDisableWinsOverNightlyCompatibility covers the canonical key set to false, but there is no corresponding test for the legacy key ("cmuxMobilePairingHostEnabled") set to false in a nightly build. The code at line 464 returns legacyOverride immediately (so false wins correctly), but the symmetry gap means a future reordering of the two early-return guards could silently break the "explicit legacy off on nightly" contract without a failing test.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@azooz2003-bit azooz2003-bit closed this pull request by merging all changes into main in 288f1e1 Jul 19, 2026
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