iOS/macOS transport: survive wake-time broker 401s without endpoint teardown - #9263
Closed
azooz2003-bit wants to merge 5 commits into
Closed
azooz2003-bit wants to merge 5 commits into
azooz2003-bit wants to merge 5 commits into
Conversation
A broker 401 at app wake (token pair rotated by another lane between capture and server validation) must not tear down the verified iroh runtime, and the Mac being redialed must not be dialed a second time as a background-control aggregation candidate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
At app wake the relay-policy refresh races the RPC lane's force token refresh: the pair captured coherently a moment earlier reaches the broker after rotation and gets a 401. That single 401 used to fail the endpoint, clear routes and the offline cache (or tear down the whole runtime on warm wakes), and nap 30-36s of flat backoff, turning a seconds-long token race into the 30s-2.5min reconnect outages visible in every wake ring. Four changes: - CmxIrohTrustBrokerClient recovers exactly once from a 401: the token source re-captures (force-minting only when the rejected access token is unchanged) and the request retries with the recovered pair. Frozen pinned sources (sign-out revocation) opt out by default. - 401/403 now preserve verified policy during refresh, and 401 retries initial activation; resolvePolicy falls back to the verified offline bootstrap on auth rejections like it already did for connectivity, so LAN and cached-relay dials keep working while auth settles. - The relay-policy refresh loop retries authorization failures on a 2s..120s ladder instead of the flat 30s+jitter schedule. - The Mac being redialed is excluded from secondary aggregation while a stored-Mac reconnect is in flight, removing the duplicate background-control dial (and its drain wait) from every recovery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Trust Broker now recovers rejected credentials and retries unauthorized requests once. Policy fallback distinguishes availability failures from authenticated denials. Relay retries use authorization-specific delays. Mac aggregation excludes the active recovery target until reconnection completes. ChangesTrust Broker recovery
Recovery-aware Mac aggregation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BrokerTokenSource
participant TrustBroker
participant CredentialRecovery
BrokerTokenSource->>TrustBroker: Send authenticated request
TrustBroker-->>BrokerTokenSource: Return HTTP 401
BrokerTokenSource->>CredentialRecovery: Request recovered credentials
CredentialRecovery-->>BrokerTokenSource: Return account-pinned credentials
BrokerTokenSource->>TrustBroker: Retry request once
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (22 passed)
✨ Finishing Touches 💡 1📝 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 |
Collaborator
Author
|
Heads-up: this composes semantically with #9259 (transient token-miss → connectivity) — 9259 covers the launch/revalidation-window half of the fail=15 signature, this PR covers the server-401-after-rotation half plus teardown/backoff/double-dial. They textually conflict in |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift`:
- Around line 2473-2476: Update the macOS host relay-policy refresh retry-delay
calculation in the settings-control flow to use the same cause-aware
authorization backoff as MobileIrohRuntimeComposition’s
Self.relayPolicyRetrySchedule(for:) helper. Preserve the existing failureCount
and retryAfterSeconds inputs, ensuring authorization failures follow the shared
2–120 second schedule instead of only CmxIrohRetrySchedule().
In
`@Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime`+Lifecycle.swift:
- Around line 109-127: Update CmxIrohClientRuntime.recoversWithCachedPolicy to
return true only for connectivity or other explicitly supported transient
failures, and remove the 401/403 CmxIrohTrustBrokerClientError rejection
fallback. Ensure rejected broker responses continue through the existing
failure-closed lifecycle/sign-out path.
In `@Sources/Mobile/MobileHostIrohRuntime`+Activation.swift:
- Around line 126-149: Extract the duplicated account-pinned credential recovery
sequence into one shared helper or static factory near CmxIrohBrokerTokenSource,
preserving session re-capture, access-token rotation reuse, one force-refresh
fallback, account-ID validation, and fail-closed behavior. In
Sources/Mobile/MobileHostIrohRuntime+Activation.swift lines 126-149 and
ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift lines
2879-2902, replace each recoveredCredentialPair closure body with calls to that
shared implementation, passing auth/session snapshot, expected account ID, and
force-refresh behavior.
🪄 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: 7c9748d9-195b-4800-9381-d68ce513f219
📒 Files selected for processing (11)
Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime+Lifecycle.swiftPackages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohClientRuntime+Policy.swiftPackages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohTrustBrokerClient.swiftPackages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohTrustBrokerClientError.swiftPackages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohTrustBrokerClientAuthClassifierTests.swiftPackages/Shared/CmuxIrohTransport/Tests/CmuxIrohTransportTests/CmxIrohTrustBrokerClientAuthRecoveryTests.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileMacConnectionPoolTests.swiftSources/Mobile/MobileHostIrohRuntime+Activation.swiftios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swiftios/cmuxPackage/Tests/cmuxFeatureTests/MobileIrohRuntimeCompositionTests.swift
Comment on lines
+2473
to
2476
| let retryDelay = Self.relayPolicyRetrySchedule(for: error).delay( | ||
| failureCount: failureCount, | ||
| retryAfterSeconds: (error as? any CmxRetryAfterProviding)? | ||
| .retryAfterSeconds, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every scheduleRelayPolicyRefresh implementation and check
# whether each uses a cause-aware retry schedule for authorization failures.
rg -n --type=swift -B3 -A5 'func scheduleRelayPolicyRefresh'
rg -n --type=swift 'relayPolicyRetrySchedule\('Repository: manaflow-ai/cmux
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -u
echo "Swift files matching target patterns:"
git ls-files | rg '(^|/)(MobileIrohRuntimeComposition\.swift|MobileHostIrohRuntime\+Activation\.swift)$' || true
echo
echo "Search scheduleRelayPolicyRefresh by name string:"
rg -n -S 'scheduleRelayPolicyRefresh' . || true
echo
echo "Search relayPolicyRetrySchedule by name string:"
rg -n -S 'relayPolicyRetrySchedule' . || true
echo
echo "Search CmxIrohRetrySchedule:"
rg -n -S 'CmxIrohRetrySchedule|RetrySchedule|retryAfterSeconds|authorizationFailed' . || trueRepository: manaflow-ai/cmux
Length of output: 41828
🏁 Script executed:
#!/bin/bash
set -u
echo "MobileHostIrohRuntime+SettingsControl scheduleRelayPolicyRefresh:"
sed -n '320,370p' Sources/Mobile/MobileHostIrohRuntime+SettingsControl.swift
echo
echo "MobileHostIrohRuntime+Activation activate scheduleRelayPolicyRefresh call:"
sed -n '460,510p' Sources/Mobile/MobileHostIrohRuntime+Activation.swift
echo
echo "MobileHostIrohRuntime policy schedule property:"
sed -n '100,140p' Sources/Mobile/MobileHostIrohRuntime.swift
echo
echo "Shared host runtime policy refresh retry delay:"
sed -n '1,80p' Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohHostRuntime+PolicyRefresh.swift
echo
echo "Shared host runtime initializer:"
sed -n '60,150p' Packages/Shared/CmuxIrohTransport/Sources/CmuxIrohTransport/CmxIrohHostRuntime.swiftRepository: manaflow-ai/cmux
Length of output: 14153
🏁 Script executed:
#!/bin/bash
set -u
echo "MobileIrohRuntimeComposition scheduleRelayPolicyRefresh definition:"
sed -n '2350,2520p' ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift
echo
echo "MobileHostIrohRuntime+Lifecycle scheduleRelayPolicyRefresh implementation:"
rg -n -A120 -B20 'private func scheduleRelayPolicyRefresh|func scheduleRelayPolicyRefresh\(' Sources/Mobile/MobileHostIrohRuntime+Lifecycle.swift Sources/Mobile/MobileHostIrohRuntime+SettingsControl.swift Sources/Mobile/MobileHostIrohRuntime+Activation.swift
echo
echo "Any relayPolicyRetrySchedule usage outside iOS cmuxFeature:"
rg -n -S 'relayPolicyRetrySchedule|diagnosticFailureKind\(' Sources/Mobile Packages/Shared ios/cmuxPackage/Sources | grep -v 'Tests/cmuxFeatureTests' | head -n 120Repository: manaflow-ai/cmux
Length of output: 25251
Apply the authorization backoff to the macOS host relay-policy refresh.
Sources/Mobile/MobileHostIrohRuntime+SettingsControl.swift:347-351 still computes the retry delay with CmxIrohRetrySchedule() only, while ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift:2491-2507 has the cause-aware helper. Use the same authorization-aware schedule here so iOS and macOS get the same 2s-120s retry path.
Also applies to: 2491-2509
🤖 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 `@ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift`
around lines 2473 - 2476, Update the macOS host relay-policy refresh retry-delay
calculation in the settings-control flow to use the same cause-aware
authorization backoff as MobileIrohRuntimeComposition’s
Self.relayPolicyRetrySchedule(for:) helper. Preserve the existing failureCount
and retryAfterSeconds inputs, ensuring authorization failures follow the shared
2–120 second schedule instead of only CmxIrohRetrySchedule().
Comment on lines
+126
to
149
| }, | ||
| recoveredCredentialPair: { [weak auth] rejected in | ||
| guard let auth else { return nil } | ||
| // Re-capture first: when another lane already rotated the | ||
| // session, the fresh snapshot differs from the rejected | ||
| // pair and no extra mint is needed. Only an unchanged | ||
| // access token forces a mint; the SDK store dedups | ||
| // concurrent refreshes. | ||
| if let session = try? await auth.authenticatedSessionSnapshot(), | ||
| session.accountID == accountID, | ||
| session.accessToken != rejected.accessToken { | ||
| return CmxIrohBrokerCredentials( | ||
| accessToken: session.accessToken, | ||
| refreshToken: session.refreshToken | ||
| ) | ||
| } | ||
| guard (try? await auth.forceRefreshAccessToken()) != nil, | ||
| let session = try? await auth.authenticatedSessionSnapshot(), | ||
| session.accountID == accountID else { return nil } | ||
| return CmxIrohBrokerCredentials( | ||
| accessToken: session.accessToken, | ||
| refreshToken: session.refreshToken | ||
| ) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Extract the duplicated account-pinned credential recovery logic into one shared helper.
Both sites implement the identical exactly-once recovery sequence: re-capture the session and reuse it if the access token already rotated, otherwise force-refresh and re-capture, failing closed on any account mismatch. This is security-relevant authentication logic; duplicating it across the macOS host runtime and the iOS client runtime means a future correction to one path can silently miss the other.
Sources/Mobile/MobileHostIrohRuntime+Activation.swift#L126-L149: replace this closure body with a call to a shared helper that takesauth, the expected account ID, and the rejected pair.ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift#L2879-L2902: replace this closure body with the same shared helper call.
Consider adding a static factory such as CmxIrohBrokerTokenSource.accountPinned(expectedAccountID:sessionSnapshot:forceRefresh:) in CmuxIrohTransport, or a helper in the shared auth package, so both platforms call one implementation.
♻️ Sketch of a shared extraction
extension CmxIrohBrokerTokenSource {
/// Account-pinned token source with exactly-once 401 recovery: reuses an
/// already-rotated session, otherwise force-refreshes once. Fails closed
/// on any account mismatch.
static func accountPinned(
expectedAccountID: String,
sessionSnapshot: `@escaping` `@Sendable` () async -> (
accountID: String, accessToken: String, refreshToken: String
)?,
forceRefresh: `@escaping` `@Sendable` () async -> Bool
) -> CmxIrohBrokerTokenSource {
func pair(_ session: (accountID: String, accessToken: String, refreshToken: String)) -> CmxIrohBrokerCredentials {
CmxIrohBrokerCredentials(accessToken: session.accessToken, refreshToken: session.refreshToken)
}
return CmxIrohBrokerTokenSource(
credentialPair: {
guard let session = await sessionSnapshot(),
session.accountID == expectedAccountID else { return nil }
return pair(session)
},
recoveredCredentialPair: { rejected in
if let session = await sessionSnapshot(),
session.accountID == expectedAccountID,
session.accessToken != rejected.accessToken {
return pair(session)
}
guard await forceRefresh(),
let session = await sessionSnapshot(),
session.accountID == expectedAccountID else { return nil }
return pair(session)
}
)
}
}📍 Affects 2 files
Sources/Mobile/MobileHostIrohRuntime+Activation.swift#L126-L149(this comment)ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift#L2879-L2902
🤖 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 `@Sources/Mobile/MobileHostIrohRuntime`+Activation.swift around lines 126 -
149, Extract the duplicated account-pinned credential recovery sequence into one
shared helper or static factory near CmxIrohBrokerTokenSource, preserving
session re-capture, access-token rotation reuse, one force-refresh fallback,
account-ID validation, and fail-closed behavior. In
Sources/Mobile/MobileHostIrohRuntime+Activation.swift lines 126-149 and
ios/cmuxPackage/Sources/cmuxFeature/MobileIrohRuntimeComposition.swift lines
2879-2902, replace each recoveredCredentialPair closure body with calls to that
shared implementation, passing auth/session snapshot, expected account ID, and
force-refresh behavior.
foregroundTerminalBrokerFailureRevokesLocalPolicy used a 401 as its terminal failure; auth rejections are deliberately no longer terminal, so the revocation case now uses a genuinely terminal 400 and 401/403 join the parameterized preserved-set coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The preserved-policy widening must not cross the security boundary the never-consults-offline tests pin: an authenticated denial (401/403) keeps already-verified in-memory state during a refresh, but it must never unlock the offline policy store or dial-time cached grants — a revoked account or binding stops dialing at the next dial, not at grant expiry. resolvePolicy's offline bootstrap stays connectivity-only, and the registry context provider and host cached-policy fallbacks now key on a strict isAvailabilityFailure classifier instead of the widened preserve set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
Merged
Collaborator
Author
|
Superseded by #9344: same fixes redesigned as a single credential authority on post-connectivity-v2 main (this branch conflicted after the v2 merge). The backoff-schedule piece landed separately with the v2 closeout. |
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
1 participant
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.
Problem
Field cmuxdiag rings from build 20260731034828 (Aziz's phone, 2026-07-31) show the dominant reconnect-flakiness mechanism at app wake: the relay-policy refresh fires with a credential pair that another lane (the RPC wake force-refresh) rotated between capture and server validation. The broker answers 401. Today that single 401:
endpointFailed authorizationFailed) or, on warm wakes, tears down the whole verified runtime including routes and the offline policy cache (CmxIrohClientRuntime+PolicyRefresh), andretryScheduled ms=32331/36124/36683in the rings).Every dial in that window then fails
policyUnavailable/endpointUnavailable/noRoute; observed outages ran 30s to 2.5 minutes per wake. Ring stats: 39 dials, 10 connected, 29 recovery attempts in a 19-minute trace.The rings also show every recovery dialing the target Mac twice: recovery nils
foregroundMacDeviceID, which makes the recovering Mac eligible for secondary (background-control) aggregation; the warm dial then has to be drained by the foreground redial (controlOwnerReleased+ fresh dial, plus up to the 3s handoff-drain wait).Fix
CmxIrohTrustBrokerClientrecovers exactly once from a 401.CmxIrohBrokerTokenSourcegains an optionalrecoveredCredentialPairclosure invoked with the rejected pair; the client retries the request once with the recovered pair. The iOS composition and the Mac host wire it to re-capture the session snapshot (force-minting viaAuthCoordinator.forceRefreshAccessToken()only when the rejected access token is unchanged, so concurrent rejections cannot stampede the minter). Frozen pinned sources (sign-out revocation flows) keep the default nil closure and never switch credentials.preservesVerifiedPolicyDuringRefreshnow includes 401/403,retriesInitialActivationincludes 401, andresolvePolicyfalls back to the verified offline bootstrap on auth rejections exactly as it already did for connectivity failures. LAN and cached-relay dials keep working while auth settles; a genuinely dead session still exits through the auth coordinator's state clear, which owns runtime teardown.relayPolicyRetrySchedule(for:)) instead of the flat 30s+jitter schedule.secondaryAggregationCandidateMacsexcludes the in-flight recovery target (recoveryTargetMacDeviceID/recoveryTargetInstanceTag, gated onisReconnectingStoredMac || connectionRecoveryOwner.isActive) the same way it excludes a live foreground.Regression structure
Relationship to in-flight PRs
Complementary to #9256 (pooled-session wake validation, deferred recovery, dial gating) and #9250 (health-gated recovery, discovery freshness): neither touches the broker auth lane or the aggregation exclusion. The zombie/corpse-dial signatures in the same rings are owned by #9256.
Test plan
swift test --package-path Packages/Shared/CmuxIrohTransport(full suite)swift test --package-path Packages/iOS/CmuxMobileShell --filter MobileMacConnectionPoolTests(75/75)cmuxFeatureTests.relayPolicyRetryScheduleShortensAuthorizationFailuresruns in the hosted iOS-simulator package job (package does not build for macOS)wkauth) + iPhone dogfood🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes wake-time broker 401s without tearing down the endpoint, preserving verified state so LAN and existing routes stay online while auth settles. Also prevents duplicate background-control dials during recovery and shortens auth-failure backoff.
Written for commit 7063d0f. Summary will update on new commits.
Summary by CodeRabbit