Skip to content

Refresh same-account Iroh discovery on iOS startup - #9430

Merged
azooz2003-bit merged 5 commits into
mainfrom
prep-app-store-review-mac
Aug 6, 2026
Merged

azooz2003-bit merged 5 commits into
mainfrom
prep-app-store-review-mac

Conversation

@azooz2003-bit

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

Copy link
Copy Markdown
Collaborator

Summary

  • force every explicit iOS same-account Iroh discovery request to perform a fresh broker refresh before returning live Mac candidates
  • remove the cached discovery-generation fast path that could return an old or empty route catalog after login, foreground, reinstall, or signing/bundle identity changes
  • keep the App Review Mac reviewer instructions current
  • revert the secondary stream-end reconnect patch because the corrected scope is first-launch/new-signing discovery

Why

  • Fresh installs and newly signed app instances start with no paired-Mac rows, so Auto-Connect depends on zero-touch Iroh discovery. The startup reconnect path now waits for the current runtime to ask the broker for a fresh live-Mac snapshot before deciding there are no same-account Macs.
  • This is signal-driven: login/startup/foreground still call the existing reconnect/discovery path, and that path now performs an explicit broker refresh instead of consuming a cached discovery generation.

Verification

  • swift test --package-path Packages/iOS/CmuxMobileShell --filter IrohZeroTouchDiscoveryTests, 12 tests passed
  • swift test --package-path Packages/iOS/CmuxMobileShell --filter IrohConnectionRecoveryOwnerTests, 19 tests passed after reverting the secondary reconnect patch
  • macOS tagged cloud reload: asrm, run 30895906008, BUILD_OK
  • Hosted regression run for the new cmuxFeature test: https://github.com/manaflow-ai/cmux/actions/runs/31054057933 ran cmuxFeatureTests/MobileIrohRuntimeCompositionCooldownTests on the iPhone simulator lane. All 11 tests passed, including liveMacDiscoveryForcesFreshBrokerSnapshotAfterActivation, and xcodebuild reported TEST SUCCEEDED. The run used throwaway branch ci-asrm-cooldown-verify (this PR head plus the iOS: unbreak MobileInjectedAttachStartupTests so the xctestplan compiles #9668 test-compile fix, branch deleted after the run) because the compile break it fixes still exists on origin/main.
  • The job itself is marked red only by the zero-tests gate, which does not count Swift Testing output; filed as iOS workflow zero-tests gate fails Swift Testing package filters that ran and passed #9671. The package-conventions lint failure in the same run is the pre-existing baseline set in files this PR does not touch.
  • End-to-end simulator preflight of the exact fixed path (fleet build, tag asrm, isolated sim cmux-dev-asrm): fresh install of dev.cmux.ios.asrm (new signing, empty container), signed in via mobile-dev-launch with --no-attach so no pairing ticket existed. Within 6 seconds the app discovered the same-account Mac through the forced broker refresh and established a live session (debug log: macupdate.hint version=0.64.22, sync.subscribe_ok topics=12, stable sync.liveness probes), and the workspace list rendered the discovered Mac with no Add Computer step. Screenshot captured in the worktree at cmux-assets/prep-app-store-review-mac/asrm-fresh-install-discovery.png.

Pending or blocked verification

  • Local swift test --package-path ios/cmuxPackage --filter MobileIrohRuntimeCompositionCooldownTests/liveMacDiscoveryForcesFreshBrokerSnapshotAfterActivation is blocked because ios/cmuxPackage pulls iOS-only UIKit terminal packages into a macOS SwiftPM test build.
  • iPhone: dev.cmux.ios.asrm 1.0.4 is installed on the target phone (offline queue auto-drained after reconnect) and a signed-in launch was verified. Injected attach to the workstation-local tagged Mac times out on the physical device because the device build bakes staging origins while that Mac's attach route is workstation-local; this is expected for the phone path, which relies on the same-account staging discovery this PR fixes. Remaining step is hands-on dogfood; the device is currently locked, so relaunch after unlock with scripts/mobile-dev-launch.sh --tag asrm --device --device-id --no-attach.

Notes

  • no App Store Connect mutation, upload, submission, or release was performed
  • no autoreview, Codex review, Claude review, or second-model review was invoked

Summary by CodeRabbit

  • New Features

    • Added automatic review-Mac discovery with a manual pairing fallback.
    • Updated App Store review setup for Mailinator email-code authentication.
    • Added review-only network access guidance and external-network verification steps.
  • Bug Fixes

    • Improved live Mac discovery by refreshing current results and clearing stale results when refreshes fail.
    • Added runtime validation to ensure displayed devices remain current.
  • Documentation

    • Identified reviewer-setup.md as the canonical App Store Connect notes template.
    • Clarified that review-notes.md is reference-only.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Review-Mac Discovery and Setup

Layer / File(s) Summary
Fresh live-Mac discovery
ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift, ios/cmuxPackage/Tests/cmuxFeatureTests/MobileIrohRuntimeCompositionCooldownTests.swift
Live discovery refreshes the runtime, removes cached candidate reuse, validates runtime identity, and verifies a fresh broker request after activation.
Reviewer authentication and pairing
ios/AppStoreReview/reviewer-setup.md, ios/AppStoreReview/review-notes.md
Review instructions cover Mailinator code authentication, automatic prepared-Mac discovery, Tailscale fallback pairing, review-only controls, and interaction testing.
Submission notes and checklist
ios/AppStoreReview/README.md, ios/AppStoreReview/metadata-screenshots-checklist.md, ios/AppStoreReview/review-notes.md
Documentation identifies reviewer-setup.md as canonical, marks review-notes.md as reference-only, and updates App Store Connect submission checks.

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

Possibly related PRs

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 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 Swift Actor Isolation ✅ Passed The production Swift diff only removes cached discovery state and reads. MobileIrohRuntimeComposition was already @MainActor; the only added Swift code is a @MainActor test.
Cmux Swift Blocking Runtime ✅ Passed The production Swift diff removes cached-generation logic and adds no blocking or timing primitive; the only new Swift code is an async test, which is allowed scaffolding.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only App Review documentation and iOS Iroh discovery files; both scoped browser automation files are unchanged, so no off-main browser wait or missing policy test is introduced.
Cmux Expensive Synchronous Load ✅ Passed The production diff removes cached discovery state and uses an existing async CmxIrohClientRuntime actor refresh; it adds no agent-history loader, file scan, JSON parse, or synchronous I/O path.
Cmux Cache Substitution Correctness ✅ Passed The Swift diff removes the cached discovery-generation fast path and calls refreshLiveDiscoveryOutcome before reading candidates; failed refreshes clear live candidates, so no fresh read was replac...
Cmux No Hacky Sleeps ✅ Passed The PR changes only Markdown and Swift files; it introduces no TypeScript, JavaScript, shell, or build/runtime script changes covered by this check.
Cmux Algorithmic Complexity ✅ Passed The production Swift diff only removes a cached-generation fast path and forces refresh; it adds no nested or per-target collection scan. Existing route-catalog filtering/sorting is unchanged, and...
Cmux Swift Concurrency ✅ Passed The PR-range Swift diff removes cached discovery state and adds one async XCTest; it adds no Dispatch, Combine, completion-handler, or unowned fire-and-forget pattern.
Cmux Swift @Concurrent ✅ Passed The Swift diff only removes the cached-generation path and adds a test. The network refresh remains an explicit await to actor-isolated CmxIrohClientRuntime from @MainActor; no invalid annotation o...
Cmux Swift Package Boundaries ✅ Passed The production diff only removes cached-discovery state and a fast path from the existing @MainActor cmuxFeature SwiftPM composition target; the added code is test-only.
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only docs, Swift source, and tests; it changes no Package.swift, .gitignore, workflow, Xcode package reference, or lockfile, and cmuxPackage/.gitignore does not ignore Package.resolved.
Cmux Swift Logging ✅ Passed The Swift diff only removes discovery-cache logic and adds a test; it adds no prohibited logging, and the existing Logger is unchanged and declared nonisolated private.
Cmux User-Facing Error Privacy ✅ Passed Production changes only remove cached discovery control flow and add a broker-count test; provider terms are confined to App Store review docs, which the rule allows.
Cmux Full Internationalization ✅ Passed The PR adds no production user-facing Swift text; the only added Swift is a test, and the Markdown changes are internal App Store Review operational docs, an allowed case.
Cmux Swiftui State Layout ✅ Passed The net Swift diff only removes discovery-cache state and adds a broker-count test; changed files import no SwiftUI and add no prohibited state, layout, or render-time mutation patterns.
Cmux Architecture Rethink ✅ Passed The Swift diff removes the composition's consumed-generation cache and always uses the runtime's refresh owner; it adds only a focused request-count test and no timing or side-channel repair.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR Swift diff only changes Iroh discovery state and adds a test; it introduces no standalone NSWindow, NSPanel, Window, WindowGroup, or close-shortcut routing.
Cmux Source Artifacts ✅ Passed All six changed paths are intentional Markdown docs, Swift source, or a Swift test; no artifact, cache, build-output, scratch, or dependency-checkout path appears.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Production Swift diff only removes cached discovery state and fast-path code; it adds no test/debug guard or seam-shaped member. The new test is under Tests/.
Cmux No Ambient Global State ✅ Passed The production Swift diff only removes discovery cache state and changes scoped methods; it adds no top-level functions, mutable globals, static-only namespaces, or new singletons.
Title check ✅ Passed The title clearly summarizes the primary change: refreshing same-account Iroh discovery on iOS startup.
Description check ✅ Passed The description clearly covers the change, rationale, testing, limitations, and review scope, but omits the template's Demo Video, Review Trigger, and Checklist sections.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch prep-app-store-review-mac

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/AppStoreReview/metadata-screenshots-checklist.md`:
- Around line 18-23: Establish one canonical App Store Connect notes template
and ensure fallback pairing details are included whenever automatic discovery
may fail. In ios/AppStoreReview/metadata-screenshots-checklist.md:18-23,
reference only the canonical pasteable block; in
ios/AppStoreReview/README.md:87-88, identify that template and mark other
documentation reference-only; in ios/AppStoreReview/reviewer-setup.md:63-67,
label the Pasteable notes block canonical and define the fallback rule; in
ios/AppStoreReview/review-notes.md:13-31, remove the competing pasteable flow or
convert the file into the finalized canonical template without unresolved
placeholders.
🪄 Autofix (Beta)

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: e3904f52-46e2-4bba-8e12-af7fa0b3cded

📥 Commits

Reviewing files that changed from the base of the PR and between 84f5755 and b7b4dc3.

📒 Files selected for processing (4)
  • ios/AppStoreReview/README.md
  • ios/AppStoreReview/metadata-screenshots-checklist.md
  • ios/AppStoreReview/review-notes.md
  • ios/AppStoreReview/reviewer-setup.md

Comment thread ios/AppStoreReview/metadata-screenshots-checklist.md Outdated
@azooz2003-bit
azooz2003-bit force-pushed the prep-app-store-review-mac branch from b7b4dc3 to 8389509 Compare August 4, 2026 07:32
@azooz2003-bit azooz2003-bit changed the title Update App Review Mac reviewer instructions Stabilize App Review Iroh auto-connect Aug 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift (1)

10907-10910: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Move the probe response parsing off the main actor.

sendRequest returns raw JSON data, and the current transport limit is on total frame-buffer size, not a small probe response. Keep JSONSerialization.jsonObject(with:) off Task { @mainactor ... } to avoid blocking input/rendering on one probe result.

🤖 Prompt for 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.

In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`
around lines 10907 - 10910, Move the JSONSerialization.jsonObject(with:) call
out of the Task { `@MainActor` ... } closure in the probe response handling near
the guard, parsing the raw data before entering the main-actor task. Pass the
parsed [String: Any] result into the actor-isolated logic while preserving the
existing stream_id and subscribed validation behavior.

Source: Coding guidelines

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

Outside diff comments:
In
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 10907-10910: Move the JSONSerialization.jsonObject(with:) call out
of the Task { `@MainActor` ... } closure in the probe response handling near the
guard, parsing the raw data before entering the main-actor task. Pass the parsed
[String: Any] result into the actor-isolated logic while preserving the existing
stream_id and subscribed validation behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: adc50f31-36fa-4a28-84f4-96ddde10bdc0

📥 Commits

Reviewing files that changed from the base of the PR and between b7b4dc3 and 8389509.

📒 Files selected for processing (3)
  • Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ConnectionRecovery.swift
  • Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift
  • Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/IrohConnectionRecoveryOwnerTests.swift

@azooz2003-bit
azooz2003-bit force-pushed the prep-app-store-review-mac branch from 8389509 to 1a6cf89 Compare August 4, 2026 08:00
@cursor

cursor Bot commented Aug 4, 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.

@azooz2003-bit azooz2003-bit changed the title Stabilize App Review Iroh auto-connect Refresh same-account Iroh discovery on iOS startup Aug 4, 2026
azooz2003-bit and others added 2 commits August 4, 2026 02:14
CodeRabbit flagged conflicting instructions: the checklist said to paste
all of review-notes.md into App Store Connect while reviewer-setup.md had
its own pasteable block with the manual pairing fallback. The
reviewer-setup.md block is now the single canonical template and
review-notes.md is marked reference-only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@azooz2003-bit
azooz2003-bit merged commit fe18ae9 into main Aug 6, 2026
6 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