Repository navigation
Fix iOS discovery of sibling Mac builds - #9403
Closed
azooz2003-bit wants to merge 3 commits into
Closed
azooz2003-bit wants to merge 3 commits into
azooz2003-bit wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe change adds deferred zero-touch Iroh candidate authentication during secondary aggregation. Validated candidates are persisted with controlled activation, authenticated display names are retained, and tests cover multiple sibling builds and route-specific transport behavior. ChangesZero-touch Iroh pairing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant StoredMac
participant MobileShellComposite
participant ZeroTouchIroh
participant PairedMacPersistence
StoredMac->>MobileShellComposite: report discovered candidates
MobileShellComposite->>MobileShellComposite: stage candidates by scope
MobileShellComposite->>ZeroTouchIroh: authenticate candidates
ZeroTouchIroh->>PairedMacPersistence: submit validated ticket
MobileShellComposite->>MobileShellComposite: retain authenticated display name
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 inconclusive)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
`@Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite`+ZeroTouchIroh.swift:
- Around line 125-148: The candidate loop around
authenticateZeroTouchIrohCandidate must not return before trailing cleanup
executes. Label the loop, replace the Task.isCancelled/current-scope and
.superseded returns with breaks from that loop, and preserve the existing
cleanup afterward for clearing pendingZeroTouchIrohCandidateScope, loading
paired Macs when didPersistCandidate is true, and scheduling retries for
transientFailureMacIDs.
🪄 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: b5bf74d4-4104-4de7-b577-fdaa56585211
📒 Files selected for processing (5)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+PairedMacPersistence.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ZeroTouchIroh.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/SecondaryClientHandle.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/IrohZeroTouchDiscoveryTests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ZeroTouchIroh.swift (2)
102-106: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not treat a store-read failure as an empty store.
try? ... ?? []makesstoredPairingIDsempty whenpairedMacStore.loadAllfails. The loop then treats every candidate as new and may persist a pairing without confirming the authoritative store state. Preserve the candidates and route the read failure through the transient retry path.As per coding guidelines: correctness-critical state must use one reliable source and fail closed when that source is unavailable.
🤖 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`+ZeroTouchIroh.swift around lines 102 - 106, Update the storedPairingIDs loading in the zero-touch pairing flow to distinguish a successful empty result from a failed pairedMacStore.loadAll read. Preserve the authoritative store-read error and route it through the existing transient retry path, preventing candidate persistence when the store state is unavailable; only evaluate new candidates after a successful load.Sources: Coding guidelines, Path instructions
194-201: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the authenticated MAC identity for the final hidden check.
candidate.macDeviceIDandcandidate.instanceTagare broker discovery data, but this path already relies onhandle.ticket/handle.authenticatedInstanceTagfor build compatibility and persistence. Use the authenticated identity forisHiddenMacDeviceID; if it is unavailable or mismatches the dial hint, reject the candidate instead of applying hidden filtering on stale discovery data.🤖 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`+ZeroTouchIroh.swift around lines 194 - 201, Update the guard in the zero-touch candidate validation flow to call isHiddenMacDeviceID with the authenticated MAC identity from handle.ticket and handle.authenticatedInstanceTag, rather than candidate.macDeviceID and candidate.instanceTag. Reject the candidate when the authenticated identity is unavailable or does not match the dial hint, while preserving the existing hidden-device filtering behavior for valid authenticated identities.Source: Path instructions
🤖 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`+ZeroTouchIroh.swift:
- Around line 102-106: Update the storedPairingIDs loading in the zero-touch
pairing flow to distinguish a successful empty result from a failed
pairedMacStore.loadAll read. Preserve the authoritative store-read error and
route it through the existing transient retry path, preventing candidate
persistence when the store state is unavailable; only evaluate new candidates
after a successful load.
- Around line 194-201: Update the guard in the zero-touch candidate validation
flow to call isHiddenMacDeviceID with the authenticated MAC identity from
handle.ticket and handle.authenticatedInstanceTag, rather than
candidate.macDeviceID and candidate.instanceTag. Reject the candidate when the
authenticated identity is unavailable or does not match the dial hint, while
preserving the existing hidden-device filtering behavior for valid authenticated
identities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 337a34f6-5f80-4c77-8b74-26aaf26f9048
📒 Files selected for processing (1)
Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite+ZeroTouchIroh.swift
lawrencecchen
added
the
stale-revisit
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Tests:
Hosted evidence:
The fixed hosted run reaches unrelated current-main failures in package-conventions-lint and WorkspaceListTableCoordinatorDropTests. The full shell-package process also hits existing concurrent timing failures and does not exit before the job limit; the isolated changed suite passes.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes iOS zero-touch discovery so sibling Mac builds (e.g., stable + nightly) are authenticated and saved as secondary pairings without taking over the active connection. This stabilizes multi-Mac aggregation and prevents missing siblings.
CmuxMobileShell.markActive: false) and preserve already-active pairings on upsert so the foreground build stays active.markActiveto paired-mac persistence andauthenticatedDisplayNametoSecondaryClientHandle.Written for commit ce91b32. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests