Make pairing and adoption survive a flaky network - #952
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
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 |
A re-pair overwrote the device's working secret the moment the host answered, two round trips before the device could persist the reply. A drop in that gap left the host holding credentials the phone never saw, and the only way back was another PIN typed at the Mac -- so on a flaky network, retrying was what broke you. A re-pair now stages: the committed secret stays live and the replacement waits in a pending slot for 10 minutes, promoted by the first hello that authenticates with it. Proof of possession IS the commit, so no new wire message is needed and clients that predate this get the protection unchanged. `pairing_result` carries an advisory `rotation` field for diagnosis only. Only PIN pairing stages; account adoption returns its secret inside hello_ok with no acknowledged follow-up, and the local OS/SSH path hands it back in-process. Also: - Bucket failed PIN attempts by device as well as address, and raise the global breaker to 25. Five fumbled PINs on one phone used to block pairing and account adoption for every device in the house. - Give each of the collapsed hello rejections its own actionable message; "Sync authentication failed." covered fourteen distinct causes with fourteen different fixes. - Fold two copies of the pair-failure limiter into one shared tracker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ilure Account adoption walked [lan, tailnet, relay] strictly in order, paying a socket open plus a 3s identity challenge per route. On cellular, where no direct route can ever succeed, that is 10-20s of dead time before relay is even dialed. It now races every route at once through the same machinery the paired reconnect path uses (#948): one plan, relay joining behind the leading direct candidate rather than after it, one 10s budget. Each candidate owns its socket and mailbox, so only the winner touches app state; the challenge crypto is unchanged. Any adoption failure also latched `autoReconnectPausedByUser`, which is persisted and blocks reconnecting forever -- so one bad connect left the app opening on a dead "Disconnected" screen for good. It is now written only by an explicit user action, which stamps a source alongside it; a paused flag with no source is provably fallout from an older build and is cleared once. Also: - A host naming an adoption cipher this build does not implement now fails that route instead of aborting the attempt with an identity verification error. The cipher is still never used. - Persist a PIN pairing secret before the hello, so a lost hello_ok cannot strand the phone on a secret the Mac already retired. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Negotiate explicit pairing commits for clients that persist replacement credentials before hello. Keep legacy hello-as-commit behavior for older clients and bind acknowledgements to the exact staged secret so stale sockets cannot promote a newer rotation. Extend host coverage for flaky re-pairs, commit expiry and races, per-device limiting, and actionable authentication errors. Add pure iOS coverage for persistence ordering and commit negotiation without running an iOS build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ten tries and a two-minute cooldown per device instead of five tries and ten minutes. Fumbling a 6-digit code while setting up a phone is the common case, not an attack; a two-minute wait still caps a LAN brute force at roughly seven codes a minute against a million-code space. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two suites disagreed about a staged re-pair. The desktop copy asserted a renamed device shows its new name straight away; the new ade-cli coverage asserted the whole staged record stays hidden. Splitting them by what is actually a credential settles it: the secret, its DPoP binding, runtime-host grant, and account ownership wait for the acknowledgement, while peerName, platform, and device type apply immediately. A user who renames a phone should not wait on a handshake they cannot see. Also key the two PIN-limit loops off PAIR_FAILURE_THRESHOLD instead of hardcoding the old value, so retuning the threshold cannot silently break them again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arul28
force-pushed
the
ade/pr4-pairing-adoption-e5aafbb4
branch
from
July 29, 2026 20:56
a81d608 to
0171833
Compare
Staging protected more than the secret. A PIN re-pair is supposed to declassify an account-owned pairing, but staging that left the committed record account-owned, so the next account switch revoked the pairing the user had just re-established at the Mac. A withdrawn runtime-host grant had the same shape: authority the re-pair removed stayed readable until an acknowledgement that may never arrive. Split by direction instead of by field: elevations wait for proof, reductions land at once. Costs at most one re-grant; the alternative is a privilege leak and a destroyed pairing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Why
Three defects made first-connect and re-pair fragile, all found during a live mobile-connection audit:
What changed
autoReconnectPausedByUseronly latches on an explicit user disconnect, with a one-time migration that clears failure-fallout pauses.Verification
testDeepLinkRouter*cases that fail identically on clean main (tracked separately).🤖 Generated with Claude Code