fix(pair-code): keep a phone-number link alive past the QR rotation

[jlucaso1]

What

The pair-code (link_code_companion_reg) flow shares a connection with QR rotation but not its lifetime, and four places treated the two as one. Cross-checked against the captured WhatsApp Web bundle — WAWeb/Alt/DeviceLinking{Api,Algorithm,HandleNotification}.js, WAWeb/Handle/PairDevice.js, WAWeb/Handle/CompanionReqRefreshNotification.js, WAWeb/Link/DevicePhoneNumberCodeScreen.react.js.

The stage-1/stage-2 crypto was re-audited first and is not at fault: a vector produced by PairCodeUtils was replayed through an independent WebCrypto implementation of the primary's half (PBKDF2 2<<16, AES-CTR length: 64, HKDF for the bundle key and adv_secret, AES-GCM), which decrypts the key bundle, finds both identity keys in the expected slots, and derives a byte-identical adv_secret. What was wrong is the flow's lifetime, and the symptom it produces is a companion_finish the server accepts (type="result") followed by silence: a primary that cannot open the key bundle reports a failed link to its own user and tells the companion nothing.

1. Exhausted QR refs no longer take the connection down with them

Handle/PairDevice.js waits 60s on the first ref (u = 6e4) and 20s on each of the other five (c = 20 * 1e3) — the same budget we use — and when they run out it cancels its own timer and reports UNPAIRED_IDLE. It does not close the socket; reloading is left to the surface above (SocketUtils.refreshQR, a user action). Every stopComms call site in the bundle belongs to a stream error, a failure, or an explicit user action — none to ref exhaustion.

We disconnected at 60s + 5×20s = 160s. A pair code is advertised as valid for 180s (code_validity, matching WA Web's I = 180), so the teardown revoked a code we had just told the consumer was still good — and any primary_hello for it then arrived at a session the server had already dropped. Request the code on a later ref and the gap is worse: on the sixth, the socket had 20s left against an advertised 180.

Now the rotation task reports Event::PairingQrCodesExhausted { disconnected } and only disconnects when no pair-code flow is outstanding, so QR-only consumers keep the reconnect they rely on to get fresh refs.

2. One code at a time

startAltLinkingFlow is guarded by invariant(stage === Initialized) (Alt/DeviceLinkingApi.js): a second companion_hello is only reachable through an explicit initializeAltDeviceLinking(). That guard is load-bearing, because a second code does not replace the first for the phone — the server never sees the code and routes primary_hello by number, so whoever is still reading the older one reaches stage 2 and is answered with a key bundle their code cannot open.

We overwrote pair_code_state silently, with nothing in the logs to say the displayed code had just been retired. pair_with_code now fails with PairCodeError::CodeAlreadyOutstanding { remaining } while a code is live, and Client::cancel_pair_code() is the explicit reset — our initializeAltDeviceLinking(). Expired codes strand nobody and still let a fresh request through.

The module docs now also state the rule this follows from: QR rotation is not a reason to request a new pair code. WA Web mints one per user action and regenerates only on the server's refresh_code, on force_manual_refresh, or on its own expiry timers — never on a ref rotating.

3. An unanswered companion_finish now times out

Link/DevicePhoneNumberCodeScreen.react.js arms 1 * MINUTE_MILLISECONDS on primary_hello_received and regenerates the code when it fires (primary_hello_expire), with a 3.25-minute TTL behind it and a cap of 5 regenerations. The primary having read the code says nothing about the outcome, and a primary that failed goes quiet — silence is the only signal there is.

We had no timeout at all: after Sent companion_finish, waiting for pair-success the flow simply sat there. Event::PairingCodeRefresh now covers both triggers (the server's refresh_code and this timer), and in both cases the flow is cleared before the event fires, so a consumer acting on it is not rejected by the very flow it was told to replace.

4. <notification type="companion_reg_refresh">

Handle/CompanionReqRefreshNotification.js accepts the stanza with either a companion_reg_refresh or a pair-device-rotate-qr child, rejects it when neither is present, and re-mints the ADV secret key. We routed it to the catch-all, so the QR we kept advertising carried a secret the server had retired.

One deliberate divergence: WA Web rotates unconditionally, but past stage 2 a phone-number flow has already derived the secret the pending pair-success HMAC is computed over. Rotating there converts a link that was about to succeed into one that cannot, so the refresh yields to an outstanding pair code — the QR it is about is what that code is replacing anyway.

5. The ack no longer waits on stage-2 crypto

Alt/DeviceLinkingHandleNotification.js starts handlePrimaryHello(i) without awaiting it and returns the ack in the same expression. We awaited the whole of stage 2 — a 131k-round PBKDF2, a DH, a persist and a send — before the notification was acknowledged.

Stage 2 moves into a task. The cheap guards (ref, validity window, attempt cap) stay on the notification path so the return value still reflects them, the pair_code_state lock still serializes derive → persist → send as #976 established, and the task re-checks the ref it was spawned for, so a cancellation or pair-success landing while it waits for the lock stops it.

Follow-ups from review

Six races the first push had, all reachable, all in code this branch introduced:

• The one-code guard was not atomic. It checked the state and released the lock across the stage-1 round trip, so two concurrent callers both found it idle and both minted a code — the overlap the guard exists to prevent. The slot is claimed under the same lock that reads it, through a new PairCodeState::RequestingCode handed back when stage 1 fails.
• Stage 2 matched the state variant, not the ref. A flow that had been replaced rather than retired read as its own, and would have persisted a retired adv secret over the replacement's and put a companion_finish on the wire for a ref nobody holds. (The first push's description claimed this check was there; it was not.)
• companion_reg_refresh yielded only to a code inside its validity window. A primary_hello accepted near the end of that window leaves the link pending for up to a minute longer with the adv secret already derived, so the refresh could rotate straight through a pending pair-success. awaiting_pair_success tracks that half of the flow on its own clock.
• QR payloads were built once and queued. Since this PR gives companion_reg_refresh the power to re-mint the adv secret, every queued payload would have advertised a retired one and a scan would fail verification. Each payload is now built when its ref is published — which is also what WA Web does, since it renders from current state rather than a precomputed list.
• The pair-success timeout was armed after a successful send, so a stage 2 that failed armed nothing. WA Web starts the clock on the notification (primaryHelloReceivedAltLinking fires before handlePrimaryHelloInternal), so it is armed on acceptance now.
• A test used poll_until(.., || true), a no-op that let its negative assertion pass without the timer task ever being polled.

Second round

Seven more, from a re-review of the fixes above:

• A claim identified by the second it started in is not an identity. Cancel a request and start its replacement inside the same second and the first one's late response installs its code over the replacement's claim. Claims carry a token.
• Three places confused the code's clock with the link's. pair_with_code and the QR-exhaustion guard both read a flow as gone once the 180s window closed, though an accepted primary_hello leaves companion_finish pending for up to a minute more — one would start a second flow racing the pending pair-success, the other would tear down the socket carrying it. Both ask is_outstanding now.
• companion_reg_refresh had the opposite error, deferring to a code that was merely displayed. That flow derives its own adv secret when the phone answers, so deferring protected nothing while leaving the QR on the same connection advertising retired material for the whole validity window. It defers only to a pending pair-success — which narrows the divergence from WA Web to the one case that needs it.
• The pair-success timers were keyed on the shared ref, so a retry accepted partway through the first attempt's window was retired by the first attempt's timer. They carry the attempt they belong to.
• Rotating the adv secret reached the queued QR payloads but not the one on screen, which stayed scannable against a retired secret for the rest of its ref. The rotation task re-renders the current ref in place — one path serves both a ref change and an adv-secret change in WA Web (Link/DeviceQrcode.react.js listens on advSecretEventEmitter).
• A pair-code flow outlived its connection. The ref and any in-flight companion_hello belong to a socket the server has dropped; left behind, they made the outstanding-code guard reject the request reconnecting exists to make. Cleared in connection teardown.

Not changed, with reason. An authenticated pair-success is honoured even when the flow that produced it was retired: the HMAC verifying is proof the server finished registering this device, so refusing it would leave us without credentials for a device the server already has — worse than the state it avoids, and not recoverable without a logout. cancel_pair_code withdraws the local intent; it cannot undo what the phone confirmed. WA Web does not gate this either (Handle/PairSuccess.js checks a reentrancy flag and isRegistered(), nothing about the linking flow), and once a replacement reaches stage two the rotated secret makes the stale response fail verification anyway.

Also unchanged: the pair-success timer waits on the same pair_code_state lock run_stage_two holds from derive through send, so a stage 2 slower than a minute delays it. Releasing that lock earlier would reopen the adv-secret race #976 closed, and the delay is bounded by stage 2 itself — the invariant is worth more than the bound. The pre-wire claim recheck also has no deterministic test: the derivation it races is real CPU work, so the predicate it rechecks is tested instead.

Third round

Seven more, and one portability miss of my own:

• The refresh sender was published after the rotation task was spawned, so a rotation arriving in that window found no sender and was dropped. Both channels are published before the spawn.
• The rotation handler released the state lock between deciding and writing. Stage 2 derives its secret and builds companion_finish under that same lock, so a primary_hello landing in the gap had its secret overwritten and its pair-success verified against the wrong one.
• A deferred rotation was discarded, not deferred. If the flow it yielded to then failed, the retired secret stayed the one the QR advertised. It is recorded and applied when the flow gives up.
• pair-success retired the flow before authenticating it, so a response arriving after its own flow was written off cancelled the replacement that succeeded it — and then failed verification against the secret that replacement derived. It retires the flow once the crypto checks out.
• A dropped request future orphaned its claim, rejecting every later request for the rest of the validity window. Ownership is released by guard, armed until the flow is installed, and stage 1 rechecks it before reaching the wire.
• The connection-scoped reset ran before the generation was retired, leaving a window where a new request could claim the slot against a connection already going away. It runs after the inner teardown.
• The rotation loop body is indented to its own block.

And the miss: the re-render loop reached for tokio::time::sleep_until, which the wasm32 and no-default-features builds do not have — three checks caught it that a native-only local run did not. It tracks elapsed time on the runtime's clock and sleeps through the Runtime trait, and the two CI build commands are part of the local loop now.

Fourth round, and one reversal

• The deferred-refresh queue is gone. I added it last round so a companion_reg_refresh that had to yield to a pending pair-success would still be applied. Review then found it wrong on three counts in one pass: it rotated without the state lock a pair-success had just been authenticated under, it survived the teardown of the connection that requested it, and refresh_code retired the flow without draining it. Getting it right means being atomic with pair-success completing, with the flow being retired on any of four paths, and with the connection going away — four synchronisation points to recover a QR during the window where the phone-number flow is the one being used. The handler drops the request instead; if that flow fails, the QR advertises a retired secret until the next reconnect, which is where the server asks again.
• cleanup_connection_state returns early when the client-lifecycle feature is compiled in but no lifecycle is installed, so the connection-scoped reset never ran on that path — which is the path cargo test --workspace takes, and how CI caught what -p whatsapp-rust --lib could not.
• A drop(state) that dropped nothing, and the block that no longer scoped anything.

Fifth round

• A failed stage 1 released its claim only through Drop, which schedules a detached task — the future was ready before it ran, so a caller retrying the moment it saw the failure got CodeAlreadyOutstanding for a request that had given up. Error paths hand the claim back before returning; Drop stays as the backstop for a caller that drops the future and never sees the error.
• The QR ref measured its deadline on the global monotonic clock while sleeping on the runtime's. A Runtime with an independent clock would recompute nearly the full TTL on every re-render and extend the ref past the deadline the server set. One sleep covers the ref and is polled across re-renders rather than recreated by them, so the runtime's clock alone decides when it is spent.

Sixth round

• EventKind::PairingQrCodesExhausted moved to the end of the enum. Its discriminant is what a consumer persists and what indexes the EventInterest bitmask, so inserting it beside its Event variant renumbered every kind after it. ServerAck already sits at the end for the same reason; the doc said "declaration order" and no longer does.
• handle_pair_success stopped the QR rotation before parsing or verifying the response, so a pair-success arriving after its own flow was written off took the displayed code down with it and was then rejected — nothing on screen, no exhaustion event. It stops alongside the state transition, once the identity and HMAC check out.

Breaking changes

• EventKind gains PairingQrCodesExhausted at the end, so no existing discriminant moves.
• wacore::pair_code::PairCodeState gains RequestingCode { code_generation_ts, claim }. Exhaustive matches on it need a new arm; the state is the stage-1 window and is neither a live code nor idle. live_flow_remaining already folds it in for callers that only ask whether a flow is in progress.
• pair_with_code now fails with PairCodeError::CodeAlreadyOutstanding where it used to silently replace the outstanding code, and with PairCodeError::Cancelled when the flow is withdrawn while stage 1 is in flight. Callers that re-request on a schedule need cancel_pair_code() first — or, better, to stop re-requesting on a schedule.

Testing

Written test-first; each fix landed against a failing test.

Area	Tests
QR exhaustion	socket stays up with a pair code outstanding; the exhaustion event reports itself and a QR-only flow still disconnects
One code at a time	live code refuses a replacement; cancel_pair_code clears the way; an expired code does not block; a server refresh_code clears the flow it asks to replace
live_flow_remaining (wacore)	no flow; countdown across the window; the exact boundary is still live and one second past it is not; a backwards clock does not underflow
pair-success timeout	an unpaired primary_hello asks for a new code and leaves the state ready for it; a completed pairing silences the timer
companion_reg_refresh	rotates on either accepted child; ignored with neither; yields to an outstanding pair code
Early ack	the handler returns before companion_finish reaches the transport
Claim atomicity	a request racing another is refused; a rejected request frees the slot; a cancelled one does not install its flow
Stage-2 identity	a task does not answer for the flow that replaced it
Pending pair-success	companion_reg_refresh waits for it even after the code expired; the timeout is armed even when stage 2 cannot run
QR payload freshness	each payload carries the adv secret of its own moment; rotating re-emits the ref on screen
Claim identity	a claim outlives the second it started in; the replacement is the flow left standing
Link vs code clock	a pending pair-success owns the slot, keeps the socket up, and defers the rotation; a merely displayed code does not defer it
Attempt windows	a retry gets its own response window
Connection scope	a teardown does not leave the slot claimed
Claim lifetime	a dropped request hands its claim back; a failed one does so before returning; a withdrawn claim stops being owned

Clock-driven tests run under tokio::time::pause and only step the clock once the timer they are testing has had a turn to arm itself — a jump taken earlier just moves the deadline out of reach. Nothing waits on real time (~2s for the pair suite, stable over 10 consecutive runs). The existing stage-2 tests now poll for the adv rotation the spawned task performs.

cargo test --workspace --exclude e2e-tests green (the command CI runs, and the one whose feature unification exposed the teardown path above), cargo check --workspace --all-targets, --no-default-features, and the wasm32 release build clean, cargo clippy -p whatsapp-rust -p wacore --all-targets -- -D warnings clean, cargo fmt --all applied.

Not in scope

No change to the pair-code crypto, the QR pairing crypto, or the pair-success path. WA Web's 3.25-minute code TTL and 5-regeneration cap live in its React screen rather than its protocol layer, and are left to the consumer; the one-minute post-primary_hello timer is included because nothing above this layer can observe what it reacts to. No QPL markers or telemetry. Semver Checks is red here and on main alike, with identical counts (30/2/10 failures across the three checked crates, enum_variant_added 3 on both) — baseline drift against the last published release, nothing this branch adds. The PairCodeState variant is still a breaking change in Rust terms and is called out above regardless of what the lint counts.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR adds companion registration refresh handling, coordinates ADV secret rotation with active pair-code flows, prevents overlapping live pairing codes, moves stage-two processing off the acknowledgment path, and adds QR exhaustion events with conditional disconnection.

Changes

Pairing refresh flow

Layer / File(s)	Summary
Pairing state and event contracts wacore/src/pair_code.rs, wacore/src/types/events.rs	Pair-code claims, validity helpers, timeout configuration, cancellation errors, and QR exhaustion events are added.
Pair-code lifecycle and asynchronous stage two src/pair_code.rs	Live codes cannot be superseded without cancellation, stage two runs asynchronously with flow checks, refreshes retire flows, and timeout-driven regeneration is tested.
Companion registration refresh handling src/handlers/notification/...	The dispatcher validates refresh stanzas, preserves pending pair-success flows, rotates the ADV secret, and requests QR re-rendering.
QR rendering and exhaustion coordination src/pair.rs, src/client.rs, src/client/lifecycle.rs	QR payloads use current device state, refresh signals re-render the displayed QR, teardown resets pair-code state, and exhaustion conditionally disconnects while emitting status events.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• oxidezap/whatsapp-rust#202: The pair-code state and pair-success timing semantics directly support refresh deferral.
• oxidezap/whatsapp-rust#404: Both changes persist ADV secret rotation through DeviceCommand::SetAdvSecretKey.
• oxidezap/whatsapp-rust#593: Both changes modify the QR payload generation and re-rendering pipeline.

Suggested labels: api-design

Sequence Diagram(s)

sequenceDiagram
  participant NotificationDispatcher
  participant RefreshHandler
  participant PairCodeState
  participant DeviceStore
  participant QRRotationTask
  NotificationDispatcher->>RefreshHandler: dispatch companion_reg_refresh
  RefreshHandler->>PairCodeState: check pending pair-success
  alt no pending pair-success
    RefreshHandler->>DeviceStore: persist new ADV secret
    RefreshHandler->>QRRotationTask: request displayed QR re-render
  else pending pair-success
    RefreshHandler-->>NotificationDispatcher: preserve ADV secret
  end

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Title check	✅ Passed	The title clearly summarizes the pair-code lifecycle fix around QR rotation and matches the main change.
Description check	✅ Passed	The description is directly related to the implemented pair-code, QR refresh, timeout, and lifecycle changes.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/pair-code-lifecycle-wa-web-parity

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

The PR separates pair-code lifetime from QR rotation and expands pairing-flow recovery behavior.

• Keeps the connection alive after QR exhaustion while a live phone-number flow exists and publishes a dedicated exhaustion event.
• Prevents sequential replacement of live pair codes, adds explicit cancellation, and refreshes stalled flows after an unanswered companion_finish.
• Handles companion registration refresh notifications and acknowledges primary_hello before detached stage-two crypto.
• Adds shared pair-code lifetime helpers, errors, events, and regression tests.

Confidence Score: 2/5

The PR should not merge until the concurrent initialization and stale detached-stage ownership races are fixed.

Concurrent pair_with_code calls can still create and overwrite multiple live flows, while a stage-two task from a cancelled flow can mistake a replacement flow for its own and persist or send stale cryptographic material.

Files Needing Attention: src/pair_code.rs

Important Files Changed

Filename	Overview
src/pair_code.rs	Adds live-flow exclusion, cancellation, detached stage-two processing, and timeout recovery, but leaves races in concurrent initialization and stale-task ownership checks.
src/pair.rs	Changes QR exhaustion to preserve connections carrying a live pair-code flow and emits a typed exhaustion event.
src/handlers/notification/companion_reg.rs	Adds validated companion registration refresh handling with an intentional live-pair-code exception.
src/handlers/notification/mod.rs	Routes companion_reg_refresh notifications and adds coverage for accepted and rejected child tags.
wacore/src/pair_code.rs	Adds pair-code lifetime calculation, timeout configuration, and the outstanding-code error contract.
wacore/src/types/events.rs	Adds frozen event payloads for exhausted QR references and broadens pair-code refresh semantics.

Sequence Diagram

sequenceDiagram
  participant App
  participant Client
  participant WA as WhatsApp Server
  participant Stage2 as Detached Stage 2
  App->>Client: pair_with_code()
  Client->>WA: companion_hello
  WA-->>Client: pairing_ref
  Client-->>App: display code
  WA->>Client: primary_hello
  Client-->>WA: notification ack
  Client->>Stage2: derive and persist keys
  Stage2->>WA: companion_finish
  alt pair-success received
    WA-->>Client: pair-success
    Client-->>App: PairSuccess
  else no response for 60 seconds
    Client-->>App: PairingCodeRefresh
  end

Loading

Prompt To Fix All With AI

### Issue 1
src/pair_code.rs:195-202
**Concurrent requests bypass code guard**

When two callers invoke `pair_with_code` while the state is idle, both release the mutex before awaiting their independent `companion_hello` requests and can both pass the one-code guard. Their responses then overwrite the same `WaitingForPhoneConfirmation` state, so entering the earlier returned code uses the later flow's key material and fails to link.

### Issue 2
src/pair_code.rs:539-546
**Stale stage-two task changes replacement**

When a flow is cancelled or refreshed after scheduling stage two and a replacement reaches `WaitingForPhoneConfirmation`, this variant-only check treats the replacement as the captured flow. The stale task then persists its retired ADV secret and sends its old `companion_finish`, causing the valid replacement to fail verification or time out.

```suggestion
    let state_guard = client.pair_code_state.lock().await;
    // Pair-success, cancellation, or replacement may have landed while this
    // task waited for the lock.
    if !matches!(
        &*state_guard,
        PairCodeState::WaitingForPhoneConfirmation {
            pairing_ref: current_ref,
            ..
        } if current_ref.as_slice() == pairing_ref.as_slice()
    ) {
        return;
    }
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(pair-code): keep a phone-number link..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@src/handlers/notification/mod.rs`:
- Around line 93-95: Move the rationale comment from the `use companion_reg::*;`
re-export to directly above `use device::*;`, which re-exports
`handle_local_identity_change`. Keep the comment text and re-export behavior
unchanged.

In `@src/pair_code.rs`:
- Around line 1284-1291: Replace the no-op poll_until call in the
completed-pairing test with an explicit async yield that gives the timer task a
chance to run before the assertion. Keep the existing assertion and its expected
no-refresh behavior unchanged.
- Around line 539-546: Update the re-entry check in the pairing task around the
state_guard to require that WaitingForPhoneConfirmation contains the same
pairing ref captured by this task, rather than matching the variant alone.
Mirror the still_ours comparison used by start_pair_success_timeout, returning
early for a different or retired ref before deriving secrets or sending
companion_finish.
- Around line 195-202: Make the check-and-reservation in pair_with_code atomic
so concurrent calls cannot both mint a code. Update the flow around
live_flow_remaining and the later pair_code_state write to use a dedicated
serialization lock or reserve the slot while holding the same pair_code_state
lock before PBKDF2/send_iq; preserve the CodeAlreadyOutstanding error for
subsequent callers and ensure the reservation is finalized or released
appropriately on failure.

🪄 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: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a784ba5d-5c4b-48a6-9572-e2df0bbc4b8c

📥 Commits

Reviewing files that changed from the base of the PR and between 4c343ba and d9545c9.

📒 Files selected for processing (6)

• src/handlers/notification/companion_reg.rs
• src/handlers/notification/mod.rs
• src/pair.rs
• src/pair_code.rs
• wacore/src/pair_code.rs
• wacore/src/types/events.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9545c9a58

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

📦 Binary size report

Metric	main	PR	Δ
bin size (stripped)	10.05 MiB	10.07 MiB	+19.50 KiB (+0.19%) 🔺
bin .text	8.08 MiB	8.10 MiB	+13.75 KiB (+0.17%) 🔺
bin allocated (text+data+bss)	10.05 MiB	10.06 MiB	+16.48 KiB (+0.16%) 🔺
llvm-lines wacore	495,629	496,063	+434 (+0.09%) 🔺
llvm-lines wacore copies	16,413	16,417	+4 (+0.02%) 🔺
llvm-lines whatsapp-rust lib	721,105	724,470	+3,365 (+0.47%) 🔺
llvm-lines whatsapp-rust lib copies	22,717	22,808	+91 (+0.40%) 🔺
deps crates (Cargo.lock)	471	471	0

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.84 MiB	1.84 MiB	+8.71 KiB (+0.46%) 🔺
.text wacore	665.61 KiB	667.21 KiB	+1.60 KiB (+0.24%) 🔺
.text wacore_binary	89.33 KiB	89.33 KiB	0
.text wacore_libsignal	166.27 KiB	166.27 KiB	0
.text wacore_appstate	22.35 KiB	22.35 KiB	0
.text wacore_noise	21.79 KiB	21.79 KiB	0
.text waproto	1.74 MiB	1.74 MiB	0
.text whatsapp_rust_sqlite_storage	515.98 KiB	515.98 KiB	0
.text whatsapp_rust_tokio_transport	39.91 KiB	39.91 KiB	0
.text whatsapp_rust_ureq_http_client	10.33 KiB	10.33 KiB	0
.text std	1.07 MiB	1.07 MiB	+2.32 KiB (+0.21%) 🔺
.text other deps	1.90 MiB	1.90 MiB	+974 B (+0.05%) 🔺

Top movers (cargo-bloat attribution)

Crate	main	PR	Δ
whatsapp_rust	1.84 MiB	1.84 MiB	+8.71 KiB (+0.46%)
std	1.07 MiB	1.07 MiB	+2.32 KiB (+0.21%)
wacore	665.61 KiB	667.21 KiB	+1.60 KiB (+0.24%)

Baseline: c27aae0c5 (latest main run) · Head: 38af2be83 · Graphs

[greptile-apps]

Greptile has paused reviews on this repository — it used its 750 free open-source review credits for this billing period. Reviews resume automatically on August 2. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 108437b5b3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4cbe76d90a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (2)

src/pair_code.rs (1)

879-890: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Every new test fixture uses 15551234567, which isn't the reserved fictional-number format.

The shared root cause is an NPA of 555. This repo's convention is 1 + a real NPA + the fictional 555 exchange + a line number in 0100-0199 — e.g. 12025550111. 1555... doesn't match that, so these aren't provably unassignable. Cheap to fix now, annoying to chase later once more tests copy the helper.

• src/pair_code.rs#L879-L890: change phone_jid in set_waiting to "12025550111".
• src/pair_code.rs#L1168-L1173: change phone_number in options() to "12025550111".
• src/pair_code.rs#L1560-L1569: change the run_stage_two phone argument to "12025550111".
• src/handlers/notification/mod.rs#L196-L206: change phone_jid in companion_reg_refresh_waits_for_a_pending_pair_success to "12025550111".
• src/handlers/notification/mod.rs#L230-L239: change phone_jid in a_merely_displayed_code_does_not_defer_the_rotation to "12025550111".
• src/pair.rs#L589-L600: change phone_jid in set_pair_code_waiting to "12025550111".
• src/pair.rs#L709-L721: change phone_jid in qr_exhaustion_keeps_the_socket_up_for_a_pending_pair_success to "12025550111".

Based on learnings: "In Rust test code that uses fictional NANP phone numbers, format numbers as: 1 (country code) + a real NPA (not 555) + the fictional 555 exchange + a 4-digit line number between 0100 and 0199. ... Do not use an NPA of 555 (e.g., 1555...), since it does not match the reserved fictional-number format."

🤖 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 `@src/pair_code.rs` around lines 879 - 890, Replace every test fixture phone
number 15551234567 with the reserved fictional number 12025550111: update
set_waiting, options(), and run_stage_two in src/pair_code.rs (lines 879-890,
1168-1173, and 1560-1569),
companion_reg_refresh_waits_for_a_pending_pair_success and
a_merely_displayed_code_does_not_defer_the_rotation in
src/handlers/notification/mod.rs (lines 196-206 and 230-239), and
set_pair_code_waiting and
qr_exhaustion_keeps_the_socket_up_for_a_pending_pair_success in src/pair.rs
(lines 589-600 and 709-721).

Source: Learnings

wacore/src/pair_code.rs (1)

224-256: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Two doc blocks got merged onto the wrong function.

Lines 225-239 describe the code-validity window and end with "The boundary matches [PairCodeUtils::code_validity] as applied in stage 2" — that's live_flow_remaining's doc. It's now sitting on top of awaiting_pair_success, and live_flow_remaining at Line 266 has no doc at all. Rustdoc will publish the wrong explanation on the wrong item. This needs to move to where the decision it explains actually lives.

As per coding guidelines: "Place comments explaining the rationale for a decision at the single point where that decision is made."

📝 Proposed fix: move the stranding rationale onto `live_flow_remaining`

 impl PairCodeState {
-    /// The window left on a code someone may still be reading, or `None` when
-    /// there is nothing left to strand.
-    ///
-    /// A second `companion_hello` mints a new code *and* a new ephemeral
-    /// keypair, but the server keeps routing `primary_hello` by phone number —
-    /// it never sees the code itself. So the holder of the superseded code
-    /// still reaches stage 2, and gets a key bundle derived from key material
-    /// their code cannot open: the primary fails to link with no error the
-    /// companion can see. WA Web forbids the overlap outright, guarding
-    /// `startAltLinkingFlow` with `invariant(stage === Initialized)`
-    /// (`Alt/DeviceLinkingApi.js`) so a replacement must follow an explicit
-    /// `initializeAltDeviceLinking()`.
-    ///
-    /// The boundary matches [`PairCodeUtils::code_validity`] as applied in
-    /// stage 2, which rejects only `age > validity`.
     /// Whether a `companion_finish` is out and its `pair-success` still due.

and above live_flow_remaining:

+    /// The window left on a code someone may still be reading, or `None` when
+    /// there is nothing left to strand.
+    ///
+    /// A second `companion_hello` mints a new code *and* a new ephemeral
+    /// keypair, but the server keeps routing `primary_hello` by phone number —
+    /// it never sees the code itself. So the holder of the superseded code
+    /// still reaches stage 2, and gets a key bundle derived from key material
+    /// their code cannot open. WA Web forbids the overlap outright, guarding
+    /// `startAltLinkingFlow` with `invariant(stage === Initialized)`
+    /// (`Alt/DeviceLinkingApi.js`).
+    ///
+    /// The boundary matches [`PairCodeUtils::code_validity`] as applied in
+    /// stage 2, which rejects only `age > validity`.
     pub fn live_flow_remaining(&self, now: i64) -> Option<std::time::Duration> {

🤖 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 `@wacore/src/pair_code.rs` around lines 224 - 256, Move the code-validity and
stranding rationale currently attached to `awaiting_pair_success` onto the
`live_flow_remaining` method, preserving its explanation of the
`PairCodeUtils::code_validity` boundary and stage-2 behavior. Keep only the
pair-success/adv-secret documentation above `awaiting_pair_success`, and ensure
both methods’ rustdoc describes the decision each method implements.

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.

Inline comments:
In `@src/pair.rs`:
- Around line 125-179: Fix the indentation of the loop body in the QR rotation
logic so all statements inside loop, including the snapshot creation, event
dispatch, select handling, and closing braces, are consistently nested under
loop. Ensure the resulting formatting matches cargo fmt without changing
behavior.

---

Outside diff comments:
In `@src/pair_code.rs`:
- Around line 879-890: Replace every test fixture phone number 15551234567 with
the reserved fictional number 12025550111: update set_waiting, options(), and
run_stage_two in src/pair_code.rs (lines 879-890, 1168-1173, and 1560-1569),
companion_reg_refresh_waits_for_a_pending_pair_success and
a_merely_displayed_code_does_not_defer_the_rotation in
src/handlers/notification/mod.rs (lines 196-206 and 230-239), and
set_pair_code_waiting and
qr_exhaustion_keeps_the_socket_up_for_a_pending_pair_success in src/pair.rs
(lines 589-600 and 709-721).

In `@wacore/src/pair_code.rs`:
- Around line 224-256: Move the code-validity and stranding rationale currently
attached to `awaiting_pair_success` onto the `live_flow_remaining` method,
preserving its explanation of the `PairCodeUtils::code_validity` boundary and
stage-2 behavior. Keep only the pair-success/adv-secret documentation above
`awaiting_pair_success`, and ensure both methods’ rustdoc describes the decision
each method implements.

🪄 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: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c09dc710-624c-4db1-a438-298e4e6f680f

📥 Commits

Reviewing files that changed from the base of the PR and between d9545c9 and 4cbe76d.

📒 Files selected for processing (7)

• src/client.rs
• src/client/lifecycle.rs
• src/handlers/notification/companion_reg.rs
• src/handlers/notification/mod.rs
• src/pair.rs
• src/pair_code.rs
• wacore/src/pair_code.rs

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (4)

src/pair.rs (4)

716-716: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Use a valid fictional phone number.

15551234567 uses 555 as the NPA, which violates the repository’s test-data rule. Replace it with a number such as 12025550111 (202-555-0111).

🤖 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 `@src/pair.rs` at line 716, Replace the hard-coded phone_jid value in the
relevant test data with a valid fictional number, such as 12025550111, avoiding
15551234567 because its NPA violates the repository’s test-data rule.

Sources: Coding guidelines, Learnings

---

184-220: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Make exhaustion teardown atomic with login completion.

The initial is_logged_in() check can become stale while awaiting pair_code_state.lock(). A concurrent pair-success may finish during that await, after which this task can still dispatch disconnected = true and close the newly logged-in socket. Recheck at teardown and coordinate the decision with the login transition rather than relying on the earlier sample.

🤖 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 `@src/pair.rs` around lines 184 - 220, Make the QR-exhaustion teardown decision
atomic with login completion in the surrounding pairing task. After awaiting
pair_code_state.lock(), recheck the current login state and coordinate the final
disconnected/close decision with the login transition, so a concurrent
successful pairing cannot dispatch disconnected=true or close the newly
logged-in socket. Preserve the existing behavior for genuinely logged-out
QR-only sessions and outstanding pair-code flows.

---

634-649: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make QR payload tests order-independent.

Concurrent callback delivery makes vector position unreliable.

• src/pair.rs#L634-L649: retain the initial payload/ref and match later events by secret/ref instead of using codes[0] and codes[1].
• src/pair.rs#L682-L698: retain the original ref before refresh and search for the refreshed payload carrying the new secret.

🤖 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 `@src/pair.rs` around lines 634 - 649, Make the QR payload assertions in
src/pair.rs lines 634-649 order-independent by retaining the initial
payload/reference and locating later events by secret or reference instead of
codes[0] and codes[1]. In src/pair.rs lines 682-698, retain the original
reference before refresh and search collected events for the refreshed payload
containing the new secret; update both sites while preserving their existing
assertions.

Source: Learnings

---

92-98: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Install the refresh sender before spawning the rotation task.

A concurrent companion_reg_refresh can call refresh_pairing_qr before pairing_qr_refresh_tx is assigned, so the signal is lost and the displayed QR can retain a retired ADV secret. Store the channels first, then spawn the task; also retire any previous rotation sender when replacing it.

Proposed ordering

+                    *client.pairing_cancellation_tx.lock().await = Some(stop_tx);
+                    *client.pairing_qr_refresh_tx.lock().await = Some(refresh_tx);
+
                     client
                         .runtime
                         .spawn(Box::pin(async move {
                             // ...
                         }))
                         .detach();

-                    *client.pairing_cancellation_tx.lock().await = Some(stop_tx);
-                    *client.pairing_qr_refresh_tx.lock().await = Some(refresh_tx);

Also applies to: 224-225

🤖 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 `@src/pair.rs` around lines 92 - 98, Update the pairing QR refresh setup around
refresh_tx and the spawned rotation task so pairing_qr_refresh_tx is installed
before spawning the task, allowing concurrent refresh_pairing_qr calls from
companion_reg_refresh to signal it. When replacing the sender, retire or close
the previous rotation sender before storing the new one, then spawn the task
using the installed channels.

🤖 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 `@src/pair.rs`:
- Line 716: Replace the hard-coded phone_jid value in the relevant test data
with a valid fictional number, such as 12025550111, avoiding 15551234567 because
its NPA violates the repository’s test-data rule.
- Around line 184-220: Make the QR-exhaustion teardown decision atomic with
login completion in the surrounding pairing task. After awaiting
pair_code_state.lock(), recheck the current login state and coordinate the final
disconnected/close decision with the login transition, so a concurrent
successful pairing cannot dispatch disconnected=true or close the newly
logged-in socket. Preserve the existing behavior for genuinely logged-out
QR-only sessions and outstanding pair-code flows.
- Around line 634-649: Make the QR payload assertions in src/pair.rs lines
634-649 order-independent by retaining the initial payload/reference and
locating later events by secret or reference instead of codes[0] and codes[1].
In src/pair.rs lines 682-698, retain the original reference before refresh and
search collected events for the refreshed payload containing the new secret;
update both sites while preserving their existing assertions.
- Around line 92-98: Update the pairing QR refresh setup around refresh_tx and
the spawned rotation task so pairing_qr_refresh_tx is installed before spawning
the task, allowing concurrent refresh_pairing_qr calls from
companion_reg_refresh to signal it. When replacing the sender, retire or close
the previous rotation sender before storing the new one, then spawn the task
using the installed channels.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6b86a729-62b6-499a-b55d-3c9e6ca2c92b

📥 Commits

Reviewing files that changed from the base of the PR and between 4cbe76d and e27d01a.

📒 Files selected for processing (1)

• src/pair.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e27d01af96

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0bc7763afe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dda93f77e8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (2)

src/pair_code.rs (2)

379-392: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

This doc comment is describing the wrong function.

"Hand back a claim ... when stage 1 failed" is release_code_claim. owns_code_claim hands back nothing — it's a read-only predicate. When you inserted the new fn you left the doc stranded on it, and now the one function that actually mutates the slot is undocumented. In a file where the entire thesis is "claims are identified by token, not by timestamp," the docs have to point at the right thing. This is a one-line move.

As per coding guidelines: "Place comments explaining the rationale for a decision at the single point where that decision is made."

📝 Proposed fix: move the doc onto the function it describes

-    /// Hand back a claim taken by [`Self::pair_with_code`] when stage 1 failed.
-    ///
-    /// Identified by its token, so a claim already superseded — by a
-    /// cancellation, or by the replacement that followed one — is left alone.
+    /// Whether the slot is still held by *this* request's claim.
+    ///
+    /// Rechecked before the wire send and before installing the flow: a
+    /// cancellation or a replacement landing in between must stop stage 1.
     async fn owns_code_claim(self: &Arc<Self>, claim: wacore::pair_code::PairCodeClaim) -> bool {
         matches!(&*self.pair_code_state.lock().await, PairCodeState::RequestingCode { claim: c, .. } if *c == claim)
     }
 
+    /// Hand back a claim taken by [`Self::pair_with_code`] when stage 1 failed.
+    ///
+    /// Identified by its token, so a claim already superseded — by a
+    /// cancellation, or by the replacement that followed one — is left alone.
     async fn release_code_claim(self: &Arc<Self>, claim: wacore::pair_code::PairCodeClaim) {

🤖 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 `@src/pair_code.rs` around lines 379 - 392, Move the existing claim-release doc
comment from owns_code_claim to release_code_claim. Keep owns_code_claim
undocumented as the read-only ownership predicate, and place the comment
immediately above the function that resets the state to Idle.

Source: Coding guidelines

---

926-937: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fictional phone numbers in the new pair-code fixtures don't follow the repo convention. All three sites hardcode 15551234567, which puts 555 in the NPA slot — the convention here is country code 1 + a real NPA + the 555 exchange + a line number in 0100–0199 (e.g. 12025550111). One root cause: the convention wasn't applied when these fixtures were written.

• src/pair_code.rs#L926-L937: change phone_jid in set_waiting to 12025550111.
• src/pair_code.rs#L1215-L1220: change phone_number in the options() helper to 12025550111, since every pair-code test seeds from it.
• src/pair.rs#L716-L750: change phone_jid in qr_exhaustion_keeps_the_socket_up_for_a_pending_pair_success to 12025550111.

Based on learnings: "In Rust test code that uses fictional NANP phone numbers, format numbers as: 1 (country code) + a real NPA (not 555) + the fictional 555 exchange + a 4-digit line number between 0100 and 0199. Do not use an NPA of 555."

🤖 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 `@src/pair_code.rs` around lines 926 - 937, Update the fictional NANP numbers
at src/pair_code.rs:926-937 in set_waiting, src/pair_code.rs:1215-1220 in
options(), and src/pair.rs:716-750 in
qr_exhaustion_keeps_the_socket_up_for_a_pending_pair_success to 12025550111,
preserving the existing fixture behavior.

Source: Learnings

♻️ Duplicate comments (1)

src/pair.rs (1)

183-188: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

The match's closing brace is still under-indented — cargo fmt will rewrite this.

Line 187 closes the match opened at line 165 but sits four spaces shallower than it, so the loop and match closers line up at the same column and neither reads as belonging to anything. This was called out on an earlier commit and mostly fixed; this brace got missed. Run cargo fmt --all and it's done — we shouldn't be spending CI cycles on whitespace.

As per coding guidelines: "Run cargo fmt --all and ensure workspace clippy passes with cargo clippy --workspace --all-targets -- -D warnings before submission."

🧹 Proposed fix

                                         // Same ref and deadline, rebuilt payload.
                                         futures::future::Either::Right((
                                             futures::future::Either::Right(_),
                                             _,
                                         )) => continue,
-                                }
+                                    }
                                 }

🤖 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 `@src/pair.rs` around lines 183 - 188, Correct the indentation of the closing
brace for the match surrounding the futures::future::Either::Right pattern in
src/pair.rs, then run cargo fmt --all to apply standard formatting. Verify the
workspace with cargo clippy --workspace --all-targets -- -D warnings.

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.

Inline comments:
In `@src/client/lifecycle.rs`:
- Around line 930-941: Update clear_connection_scoped_pair_code to also reset
the connection-scoped pending_reg_refresh state while holding the appropriate
lock, alongside resetting pair_code_state to Idle. Ensure teardown leaves no
pending registration refresh for the next socket connection.

In `@src/handlers/notification/companion_reg.rs`:
- Around line 71-91: In the companion registration refresh flow, remove the
now-unnecessary bare block surrounding the pair_code_state lock and delete the
explicit drop(state) after rotate_companion_registration. Keep the lock,
awaiting_pair_success handling, comments, and rotation behavior unchanged; let
the guard release naturally at the end of its scope.

In `@src/pair_code.rs`:
- Around line 400-409: Update handle_refresh_code so the path that transitions a
matching WaitingForPhoneConfirmation state to Idle also calls
apply_pending_reg_refresh at the same cleanup point as
start_pair_success_timeout. In cancel_pair_code, do not return early when the
flow is already retired; preserve the cleanup call so any deferred companion
registration refresh is drained.

---

Outside diff comments:
In `@src/pair_code.rs`:
- Around line 379-392: Move the existing claim-release doc comment from
owns_code_claim to release_code_claim. Keep owns_code_claim undocumented as the
read-only ownership predicate, and place the comment immediately above the
function that resets the state to Idle.
- Around line 926-937: Update the fictional NANP numbers at
src/pair_code.rs:926-937 in set_waiting, src/pair_code.rs:1215-1220 in
options(), and src/pair.rs:716-750 in
qr_exhaustion_keeps_the_socket_up_for_a_pending_pair_success to 12025550111,
preserving the existing fixture behavior.

---

Duplicate comments:
In `@src/pair.rs`:
- Around line 183-188: Correct the indentation of the closing brace for the
match surrounding the futures::future::Either::Right pattern in src/pair.rs,
then run cargo fmt --all to apply standard formatting. Verify the workspace with
cargo clippy --workspace --all-targets -- -D warnings.

🪄 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: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: da8d1957-33e3-4638-800f-62d2e616c755

📥 Commits

Reviewing files that changed from the base of the PR and between e27d01a and dda93f7.

📒 Files selected for processing (6)

• src/client.rs
• src/client/lifecycle.rs
• src/handlers/notification/companion_reg.rs
• src/handlers/notification/mod.rs
• src/pair.rs
• src/pair_code.rs

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eb67af3b22

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52f6126518

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4024b597ce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".