feat(pair-code): report a refused pair-code request to the consumer by jlucaso1 · Pull Request #1191 · oxidezap/whatsapp-rust

jlucaso1 · 2026-07-29T23:13:54Z

Fixes #1190. The report is correct on every point I checked, including the negative one: there is no other surface a consumer could observe this on. Event::PairError is dispatched only from pair.rs:347/:506, the return value is discarded by the detached task, and nothing else touches the bus on this path.

Where it goes

bot.rs:601-628 is where the error dies, but it is not where the fix belongs — see below.

Err(e) => { warn!(target: "Bot/PairCode", "Failed to request pair code: {}", e); }

Worth noting the log line the issue quotes is also worse than it needs to be. PairError::RequestFailed was #[error("pair-code IQ request failed")], so that warn! printed exactly that sentence — the 429 in the issue's paste comes from the other line, the err(Debug) on the #[tracing::instrument]. The crate::error module already states the convention this variant was breaking ("a wrapping variant renders exactly what it wraps"), and PairError::PairCode next to it already follows it.

What WhatsApp Web actually does

Queried the whatspec IR first, then restored the pinned bundle set behind 2.3000.1043899084 — 565 files fetched and matched against generated/bundles.lock.json — because the IR describes shape and the question here is control flow.

1. The refusal set is closed, and the IR has it

iq/index.json, the CompanionHelloResponseError variant of makeCompanionHelloRequest, carries errorArms in full:

Arm	Code	Text
`IQErrorInternalServerError`	500	`internal-server-error`
`IQErrorBadRequest`	400	`bad-request`
`IQErrorFeatureNotAvailable`	452	`feature-not-available`
`IQErrorRateOverlimit`	429	`rate-overlimit`
`IQErrorForbidden`	403	`forbidden`

WASmaxInMdIqMixinErrors.parseIqMixinErrors tries them in that order and falls through to errorMixinDisjunction — i.e. anything else fails to parse and the RPC throws. Each mixin asserts the code and the text as paired literals (literal(attrInt, e, "code", 429) beside literal(attrString, e, "text", "rate-overlimit")), which is why the classification below keys on both.

2. Their error carries the status structurally, not as a string

WAWebAltDeviceLinkingIq.sendCompanionHello:

if (u.name === "CompanionHelloResponseNotifyCompanion")
  return u.value.linkCodeCompanionRegLinkCodePairingRefElementValue;
throw u.name === "CompanionHelloResponseError"
  ? new CompanionHelloError("alt pairing: Got an error from alt paring: companion hello: " + u.value.errorIqMixinErrors.name,
                            u.value.errorIqMixinErrors)   // <- second ctor arg becomes `.type`
  : new CompanionHelloError("alt pairing: Got an unknown error from alt paring: companion hello");

The name is interpolated into the message and the parsed arm is kept on .type. That is exactly the split the issue asks for.

3. They branch on it — two statuses, distinctly

DevicePhoneNumberCodeScreen, catching around genLinkDeviceCodeForPhoneNumber:

} catch(e) {
  addPointToCurrentMarker("gen_code_exception");
  yield resetLinkDeviceState({linkDeviceMethod: PairingType.ALT_DEVICE_LINKING});
  if (e instanceof CompanionHelloError) {
    WARN`[alt pairing] companion-hello err starting flow: ${e}`;
    switch (e.type?.name) {                       // <- the discriminant, not the message
      case "IQErrorFeatureNotAvailable": setStep(STEP1_PHONE_NUMBER_ENTRY); k(); return;
      case "IQErrorRateOverlimit":       setStep(STEP1_PHONE_NUMBER_ENTRY); I(); return;
    }
  }
  ERROR`alt pairing: unexpected error happened while starting flow: ${e}`.sendLogs(...);
  E();
}

The three modals are the whole policy:

I() — "Too many attempts / There were too many attempts to link a device. Please try again later."
k() — "Limited availability / This feature is not available to you yet. It's coming soon. Link with QR code instead.", button Link with QR code
E() — generic, "Please try again or link with the QR code.", for 400/403/500 and anything unparseable

Three things follow, and all three are load-bearing here:

It never log-and-drops. Every refusal reaches the person, and the two that have a specific correct response get a specific message.
It resets the flow first, unconditionally, before deciding what to say. Our claim_guard.release_now() on the stage-1 error path is the same move, and it already existed.
It never retries on its own, and never reads backoff. I grepped the alt-linking module for it: backoff appears only in unrelated backoffOptions. So the issue's second suggestion is right to expose the hint, but I have documented it as the server's advice rather than as a schedule WA Web is known to follow — claiming the latter would be inventing evidence.

I also checked abprops for a pair-code rate limit or retry budget. There is none; the throttle is server-side only.

The fix

The status

PairCodeRejection in wacore, an int-mode WireEnum keyed on code — the same shape as NackReason and TempBanReason, #[wire_fallback] Unknown(i32) and no #[non_exhaustive], matching both. A code outside WA Web's five keeps its number rather than collapsing into a named arm.

PairCodeRejection::from_server(code, text) -> Option<Self> classifies on both attributes, since that is what the mixins above assert. A text that contradicts the code yields None — no classification is honest — which lands the consumer on the same generic path WA Web takes.

None rather than Unknown(code) for that case, because Unknown could not carry it. The wire form of this enum is code(), so Unknown(429) serializes to 429 and rehydrates as RateOverlimit; since PairingCodeError derives Serialize, a consumer persisting or forwarding the event would get the demotion silently undone and reapply throttling. Verified, not assumed:

code() = 429   json = 429   back = RateOverlimit   aliases = true

There is no in-band value that both records the code and refuses to alias the arm it came from. The code is not lost — PairError::RequestFailed renders what it wraps, and names it.

An absent text is not a contradiction and the code alone decides. Deliberately laxer than WA Web, which would reject it: refusing to classify a bare 429 would also clear is_throttled() and turn the one refusal this PR exists to make actionable back into a silent one.

is_throttled() is deliberately wider than 429: it also covers BadRequest, because PairError::RequestFailed's existing doc comment already records that the server throttles pair-code requests per phone number under bad-request rather than rate-overlimit. The method says plainly that a bad-request may equally be invalid content and that the two are indistinguishable on the wire, so a true means "back off, then bounded retries", not "this will eventually succeed".

The event

Event::PairingCodeError { rejection: Option<PairCodeRejection>, backoff: Option<Duration>, error: String }.

Named for the Pairing* family that already owns this flow (PairingCode, PairingCodeRefresh) rather than the issue's PairCodeError — that name is taken by wacore::pair_code::PairCodeError, and both are re-exported from whatsapp_rust::pair_code, so the collision would have been in consumers' faces rather than only in ours.

rejection is None when there is no typed status to act on: nothing was refused (local validation, no connection, timeout), or the refusal could not be classified. Both mean the same thing to a consumer — read error.

The EventKind variant is appended at the end, per the stability rule above that enum: the discriminant doubles as an EventInterest bit index and is what a consumer persists, so inserting mid-enum re-points every stored mask after it. The Event variant stays beside PairingCode/PairingCodeRefresh, where it reads best — only the kind's discriminant is load-bearing.

Where it is dispatched

In pair_with_code, wrapping the whole flow, not at the Err arm in bot.rs as the issue suggests. Three reasons:

Stage 1 fails from a dozen places, several of them before the IQ exists. Wrapping is what makes a validation failure report too, and the issue's own motivation covers it: PhoneNumberTooShort needs to be as visible as 429, just with the opposite response.
It is what stops a later early return from silently going unreported.
It mirrors the success path, which already both returns the code and dispatches Event::PairingCode.

bot.rs also dispatches from the wait_for_socket timeout, which returns before pair_with_code is ever called — a consumer's most likely "waiting forever", so it cannot be the gap. The is_logged_in() early return between them deliberately does not dispatch: that is a success by another route (QR or an existing session won), and an error event there would be a false alarm.

What is not reported, and why it is a state question

The event means "no code is coming". So the question at dispatch time is not how the request failed but whether a code is nonetheless on its way — which is a property of the pair-code state, not of the error variant. Review found four separate failures that could reach the dispatch while a flow was live (a duplicate request, a withdrawn one, its IQ timing out, and a second caller passing a bad number beside a live code), so the suppression asks the state:

async fn failure_is_not_this_flows_to_report(self: &Arc<Self>, e: &PairError) -> bool {
    if e.lost_the_flow_to_another_request() { return true; }
    // A failing request that still owned the slot has released it by now, so
    // an outstanding flow here belongs to somebody else.
    self.pair_code_state.lock().await.is_outstanding(wacore::time::now_secs())
}

Both branches are load-bearing, verified by removing each in turn. The state check catches anything that fails while another flow owns the slot. The variant check catches what the state cannot see: a cancellation with no replacement leaves the slot idle, yet the caller asked for exactly that and does not need telling.

Supporting that, both post-await branches in stage 1 now recheck claim ownership. The success branch already did; the IQ-error branch did not, so a 30 s timeout or a server rejection outliving a cancel_pair_code returned RequestFailed and reported against the replacement. Losing the slot outranks how the request happened to end.

A direct caller still gets the Err in every one of these cases.

Also

PairError::rejection() / PairError::backoff() for direct callers, built on the existing ErrorChainExt::server_rejection() rather than a second way to walk the chain. (Imported inside the method bodies — at module scope its as_dyn_error is ambiguous with thiserror's AsDynError for every #[from] in the file.)
BotBuilder::on_pair_code_error, alongside on_pair_code / on_pair_code_refresh.
PairError::RequestFailed renders what it wraps, so the warn! in bot.rs now names the code and text without the reader reaching for the Debug form.

Tests

rejection_codes_match_wa_web pins all five against parseIqMixinErrors, plus from(418) == Unknown(418). Same idea as nack_reason_codes_match_wa_web.
rate_overlimit_is_recoverable_as_a_typed_status — the point of the change: 429 recovers as RateOverlimit with a 30 s backoff and reads as throttled, and Display carries both the code and the text.
a_contradicting_text_yields_no_classification / an_absent_text_still_classifies_by_code — the two halves of the code/text pairing, including the deliberate deviation.
pair_code_rejections_do_not_alias_on_a_round_trip — every reachable value survives serde as itself, and keeps Unknown(429).code() == RateOverlimit.code() as a live demonstration so the reason from_server returns Option cannot be refactored away as redundant.
feature_not_available_is_not_throttled — the one refusal retrying cannot fix.
local_failure_reports_no_rejection — no invented status for a failure that never reached the server.
failed_request_dispatches_pairing_code_error drives the real pair_with_code and asserts on the bus, via a validation failure — no server needed, and it covers the harder half: the dispatch wraps the whole flow, so a path returning before the IQ is built still reports.
Four suppression tests, one per way a stale failure could speak for a live flow: an_outstanding_code_is_not_reported_as_a_failure, a_superseded_request_is_not_reported_as_a_failure, a_withdrawn_request_reports_cancellation_not_its_iq_failure, and a_validation_failure_beside_a_live_code_is_not_reported. Each was verified to discriminate by reverting its fix — e.g. losing the slot outranks how the request ended, got RequestFailed(ServerError { code: 429, … }) and a live code must not be reported as failed by an unrelated bad request.
event_kind_discriminants_are_append_only pins the discriminants by value. Absolute rather than relative to a neighbour: an offset check still passes when a kind is inserted before the pair, shifting all three together.

The pre-existing a_rejected_request_frees_the_slot, a_failed_request_hands_the_claim_back_before_it_returns and a_cancelled_request_does_not_install_its_flow still pass, which is what says the wrapper and the suppression did not disturb claim handling.

Verification

cargo test: 1321 wacore, 1299 whatsapp-rust — 0 failed. cargo clippy -p wacore -p whatsapp-rust --all-targets -- -D warnings clean. cargo fmt --all. RUSTDOCFLAGS="-D warnings" cargo doc clean.

Two red/absent signals, neither from this diff:

Semver Checks (informational) is already failure on main at 414fedaa6, this PR's merge base (run) — the workflow still reports green because the job carries continue-on-error: true, documented in supply-chain.yml as "Advisory only. The workspace is pre-1.0 and intentionally breaks API between minors." All 10 failing lints are waproto::whatsapp::* against the published waproto-0.6.0; not one comes from wacore or whatsapp-rust, and this diff touches zero waproto files.
cargo check --workspace --all-targets does not complete in my container: alsa-sys fails its build script for want of ALSA headers, which the voip-cli example pulls in. -p whatsapp-rust -p wacore -p whatsapp-rust-plugin-metrics --all-targets is clean, and I am relying on CI for the rest of the matrix.

Binary size gate green: +5.97 KiB (+0.06%) whole-binary, +1.83 KiB in wacore and +3.28 KiB in whatsapp-rust.

Caveats

No new stanza and no wire-format change; this is entirely consumer-facing surface.
I have not reproduced the reporter's incident against a live server. The 429 handling is exercised by constructing the IqError, not by being refused — what I verified against WhatsApp is the refusal set and the branching policy, both from the bundle.
Nothing here retries or backs off on the consumer's behalf. That is a policy decision, and WA Web does not make it either.
The wait_for_socket dispatch has no test: it is a hardcoded 30 s inside a task start_background detaches, so covering it needs either a real 30 s wait or plumbing the timeout out purely for testability.

Review history

Eight things in this PR were wrong before review caught them. All eight are worth naming:

The EventKind variant was inserted mid-enum, shifting every discriminant from QrScannedWithoutMultidevice on — Messages and Receipt among them. The rule is stated directly above the enum and ServerAck/PairingQrCodesExhausted already sit at the end for it. Now enforced by a test rather than a comment.
The wait_for_socket timeout dispatched nothing, leaving the failure a consumer is least able to distinguish from "still waiting" as the one silent path — the exact shape Pair-code request failures are logged and dropped, so a consumer cannot see (or back off from) a 429 rate-overlimit #1190 is about.
rejection() classified on code alone, looser than the parser this description cites as its own evidence.
CodeAlreadyOutstanding rode the catch-all dispatch, and the event doc claimed the claim was always released — wrong for the case a consumer is most likely to hit, and an invitation to retry into the same refusal.
Cancelled rode it too, which is worse: it resolves after a replacement may own the slot, so the event is uncorrelated with the running flow rather than merely wrong.
The IQ-error branch never rechecked claim ownership, unlike the success branch beside it, so (5) was only half closed.
The contradicting-pair demotion did not survive serialization. Unknown(429) serialized to 429 and rehydrated as RateOverlimit, so (3)'s fix held only in-process — the half that does not matter for a gateway persisting events.
A validation failure beside a live code still reported, because validation runs before the outstanding-flow check and so never reaches the suppressed variants.

Items 4 → 8 are one class, found four times. Patching variants one at a time was the wrong shape and I should have seen it by the second: the event's meaning is a claim about state, so the suppression now asks the state. The variant check remains only for the case the state genuinely cannot see.

A ninth, self-inflicted and caught before pushing: the test for (5) initially passed with the fix reverted, because cargo fmt had collapsed the match arm and the revert silently missed the pattern. Every suppression test is now verified against the real text.

A pair-code request configured through `BotBuilder::with_pair_code` runs in a detached task, so its `Err` reaches nobody: the failure was logged and dropped. A server `429 rate-overlimit` — the one signal that should make a consumer slow down — was invisible, and indistinguishable from "the user has not typed the code yet". Dispatch `Event::PairingCodeError` on every failure path, carrying the refusal as a matchable `PairCodeRejection` rather than only a formatted string, plus the server's `backoff` hint when it named one. The same status is reachable from a direct caller via `PairError::rejection()`/`backoff()`. `PairCodeRejection`'s five named arms are the complete set WA Web's own `companion_hello` response parser accepts (`WASmaxInMdIqMixinErrors.parseIqMixinErrors`); a code outside it keeps its number in `Unknown` instead of collapsing into a named arm. WA Web branches on exactly two of them in `DevicePhoneNumberCodeScreen` — via `CompanionHelloError.type.name`, not the message — so `rate-overlimit` becomes "try again later" and `feature-not-available` sends the user to the QR code. It never retries on its own and never reads `backoff`, so the hint is exposed as the server's advice, not as a schedule. Dispatching wraps the whole flow rather than sitting at each `return Err`, so a validation failure reports too and a later early return cannot go unreported. `PairError::RequestFailed` now renders what it wraps, per the `crate::error` rendering convention, so the code and text reach a log line that prints only the error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

coderabbitai · 2026-07-29T23:14:25Z

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

Summary by CodeRabbit

New Features
- Added a pairing-code error event that includes the error message plus optional server rejection reason and retry backoff.
- Added a builder callback to react to pairing-code errors.
- Exposed pairing rejection helpers to classify throttling and extract related retry timing details.
Bug Fixes
- Automatic pairing-code requests now emit an error event on socket timeouts.
- Improved pairing-by-code error reporting to avoid misclassifying flow ownership during stage-1 send failures.
Documentation
- Clarified detached pairing-code behavior and how to distinguish “still waiting” from “no code will be issued.”
Tests
- Added unit tests for rejection mapping, throttling classification, backoff extraction, demotion scenarios, and emitted event contents.

Walkthrough

Pair-code failures now emit typed PairingCodeError events with server rejection and backoff details. Detached requests report timeouts, and BotBuilder exposes a callback for handling these events.

Changes

Pair-code error reporting

Layer / File(s)	Summary
Pairing-code error event contract `wacore/src/types/events.rs`	Adds the event kind, payload, serialization behavior, capacity assertion, and compatibility tests.
Typed refusal and error accessors `wacore/src/pair_code.rs`, `src/pair_code.rs`	Adds refusal-code mapping, throttling classification, display formatting, public re-exports, and `PairError` accessors.
Pair-code failure dispatch `src/pair_code.rs`	Dispatches `PairingCodeError` on failed requests while preserving flow ownership and returning the original error; tests cover metadata and delivery.
Detached-task callback integration `src/bot.rs`	Reports socket timeouts, adds `on_pair_code_error`, and documents event-based handling for detached requests.

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

Sequence Diagram(s)

sequenceDiagram
  participant PairCodeTask
  participant Client
  participant EventBus
  participant Callback
  PairCodeTask->>Client: pair_with_code(options)
  Client->>EventBus: dispatch PairingCodeError(rejection, backoff, error)
  EventBus->>Callback: invoke on_pair_code_error
  Callback-->>Client: receive payload and Arc<Client>

Possibly related PRs

oxidezap/whatsapp-rust#202: Introduces the base pair-code flow extended by this change.
oxidezap/whatsapp-rust#1004: Updates event discriminants and event-interest capacity used by the new event.
oxidezap/whatsapp-rust#1163: Shares the pair-code lifecycle and cancellation paths used by this dispatch logic.

Suggested labels: api-design, size-increase-ok

Suggested reviewers: greptile-apps

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	`#1190`'s requirements are implemented: failures dispatch an event, expose typed rejection/backoff, and avoid Event::PairError.
Out of Scope Changes check	✅ Passed	The changes appear scoped to pair-code failure reporting, supporting APIs, and tests/docs with no unrelated additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly summarizes the main change: reporting refused pair-code requests to consumers.
Description check	✅ Passed	The description is directly about pair-code failure reporting and matches the changeset.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/whatsapp-bundle-whatspec-w0hf5k

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

Actionable comments posted: 2

🤖 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/bot.rs`:
- Around line 625-628: Update the socket-wait failure branch around
wait_for_socket in src/bot.rs lines 625-628 to dispatch Event::PairingCodeError
with rejection: None, backoff: None, and error: e.to_string() before returning.
Keep the documentation around src/bot.rs lines 1194-1199 only once the earlier
failure path is covered, avoiding duplicate or outdated commentary.

In `@wacore/src/types/events.rs`:
- Around line 1258-1263: Update the documentation near the code-claim event to
clarify that CodeAlreadyOutstanding is dispatched before acquiring a new claim
while the previous code remains live; state that only a claim held by the failed
request is released, so callers must cancel or wait for the existing code to
expire before retrying.

🪄 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: d881f65e-c488-4cb1-852b-c031a535f9eb

📥 Commits

Reviewing files that changed from the base of the PR and between 414feda and 8fd95c4.

📒 Files selected for processing (4)

src/bot.rs
src/pair_code.rs
wacore/src/pair_code.rs
wacore/src/types/events.rs

github-actions · 2026-07-29T23:21:30Z

📦 Binary size report

Metric	main	PR	Δ
bin size (stripped)	10.10 MiB	10.10 MiB	+6.56 KiB (+0.06%) 🔺
bin .text	8.13 MiB	8.13 MiB	+5.69 KiB (+0.07%) 🔺
bin allocated (text+data+bss)	10.09 MiB	10.10 MiB	+8.71 KiB (+0.08%) 🔺
llvm-lines wacore	510,083	510,468	+385 (+0.08%) 🔺
llvm-lines wacore copies	16,634	16,636	+2 (+0.01%) 🔺
llvm-lines whatsapp-rust lib	723,794	725,142	+1,348 (+0.19%) 🔺
llvm-lines whatsapp-rust lib copies	22,818	22,867	+49 (+0.21%) 🔺
deps crates (Cargo.lock)	471	471	0

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.84 MiB	1.85 MiB	+8.63 KiB (+0.46%) 🔺
.text wacore	691.65 KiB	693.17 KiB	+1.53 KiB (+0.22%) 🔺
.text wacore_binary	91.40 KiB	91.40 KiB	0
.text wacore_libsignal	165.82 KiB	165.82 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	40.36 KiB	40.36 KiB	0
.text whatsapp_rust_ureq_http_client	11.83 KiB	11.83 KiB	0
.text std	1.08 MiB	1.08 MiB	+402 B (+0.04%) 🔺
.text other deps	1.91 MiB	1.90 MiB	-4.96 KiB (-0.25%) 🔽

Top movers (cargo-bloat attribution)

Crate	main	PR	Δ
whatsapp_rust	1.84 MiB	1.85 MiB	+8.63 KiB (+0.46%)
metrics_exporter_prometheus	4.96 KiB	(removed)	-4.96 KiB (-100.00%)
wacore	691.65 KiB	693.17 KiB	+1.53 KiB (+0.22%)

Baseline: 414fedaa6 (latest main run) · Head: 2472f0f02 · Graphs

…laim note Review caught a gap the event's own doc had promised away. When `with_pair_code`'s task times out waiting for the socket, `pair_with_code` never runs, so nothing dispatches — the one failure left that a consumer could not tell from "still waiting", which is the shape the issue was about. Dispatch it from that branch: `rejection` stays `None`, because nothing was refused. Also correct the claim-release note. Only a claim the failed request itself took is released; `CodeAlreadyOutstanding` is refused *because* an earlier code is still live, and that one is deliberately untouched, so "call `pair_with_code` again" was wrong for exactly the case a consumer is most likely to hit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8fd95c4403

ℹ️ 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

Caution

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

⚠️ Outside diff range comments (1)

wacore/src/types/events.rs (1)
225-225: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Append PairingCodeError instead of inserting it.

EventInterest uses EventKind discriminants as bit positions, so this insertion shifts every later bit. Existing persisted or externally constructed masks can then subscribe to the wrong events after upgrading. Keep the Event variant and kind() arm, but append this EventKind variant after the current last variant.
Proposed fix
     PairingCodeRefresh,
-    PairingCodeError,
     QrScannedWithoutMultidevice,
...
     PairingQrCodesExhausted,
+    PairingCodeError,
🤖 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/types/events.rs` at line 225, Move the EventKind variant
PairingCodeError from its current inserted position to after the existing final
variant in the EventKind enum. Preserve the Event variant and its kind() arm,
ensuring all previously defined EventKind discriminants and corresponding
EventInterest bit positions remain unchanged.

🤖 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 `@wacore/src/types/events.rs`:
- Line 225: Move the EventKind variant PairingCodeError from its current
inserted position to after the existing final variant in the EventKind enum.
Preserve the Event variant and its kind() arm, ensuring all previously defined
EventKind discriminants and corresponding EventInterest bit positions remain
unchanged.

ℹ️ Review info

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b4032f6e-866a-430a-b8e0-e554160eed9c

📥 Commits

Reviewing files that changed from the base of the PR and between 8fd95c4 and 293ebf1.

📒 Files selected for processing (2)

src/bot.rs
wacore/src/types/events.rs

…text Two review findings, both real. `EventKind`'s discriminant doubles as an `EventInterest` bit index and is what a consumer persists or transmits, so a kind inserted mid-enum re-points every stored mask after it at the wrong events — `Messages` and `Receipt` among them. The rule is stated directly above the enum and `ServerAck`/`PairingQrCodesExhausted` already sit at the end for it; this did not follow it. `PairingCodeError` moves to the end (the `Event` variant stays where it reads best), the overflow tripwire re-points at the new last variant, and a test now pins the discriminants by value so the rule is enforced rather than only documented. `PairError::rejection()` also classified on `code` alone while WA Web asserts `code` and `text` as a pair and drops to its generic error path when they disagree. `PairCodeRejection::from_server` now demotes a contradicting pairing to `Unknown`. An *absent* text is deliberately still classified by code — laxer than WA Web, because demoting a bare 429 would clear `is_throttled` and put the silent failure back for the one refusal that most needs acting on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

coderabbitai

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 `@wacore/src/types/events.rs`:
- Around line 1937-1941: Update the ordering assertions for EventKind so
PairingCodeError is asserted to immediately follow PairingQrCodesExhausted,
preserving the existing ServerAck ordering check and pinning PairingCodeError’s
persisted index.

🪄 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: f325ed89-12f0-4e3a-b61a-6fc7a26acf25

📥 Commits

Reviewing files that changed from the base of the PR and between 293ebf1 and c6d9dc0.

📒 Files selected for processing (3)

src/pair_code.rs
wacore/src/pair_code.rs
wacore/src/types/events.rs

`CodeAlreadyOutstanding` was riding the catch-all dispatch, which made the event say the opposite of what happened: the request was refused *because* a code is live, and the consumer already holds it from the `PairingCode` that minted it. An `on_pair_code_error` handler seeing it could only retry, into the same refusal, until `cancel_pair_code` or expiry cleared the slot. Excluded via `PairError::leaves_a_code_outstanding()`, so the event keeps its one meaning — no code is coming — and a direct caller still gets the `Err`. A concurrent request that has not yet produced a code still resolves on its own, into `PairingCode` or into this event, so refusing the duplicate strands nobody. Also pin the new kind's discriminant absolutely rather than as an offset from its neighbour: a relative assertion still passes when a kind is inserted before the pair, which shifts all three together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

coderabbitai

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 `@src/pair_code.rs`:
- Line 985: Update the phone_number fixture in the relevant pair-code test to
use a valid fictional NANP number with a real area code and the 555 exchange,
such as 12025550111, while preserving the existing string format.

🪄 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: 939bbdf5-792e-4bc7-82b5-8e86f0c44251

📥 Commits

Reviewing files that changed from the base of the PR and between c6d9dc0 and 4ef497a.

📒 Files selected for processing (2)

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: 4ef497ab74

ℹ️ 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".

…eplacement Second instance of the same class as the `CodeAlreadyOutstanding` exclusion, and the more dangerous one. A request cancelled while its `companion_hello` is in flight returns `Cancelled` only once the response lands — by which point a replacement may already own the slot and be about to deliver a code. Dispatching the predecessor's failure puts an uncorrelated "no code is coming" on the bus against a flow that is running, so a consumer could tear down the live code or retry over it. `leaves_a_code_outstanding` becomes `lost_the_flow_to_another_request` and covers both: neither failure means no code is coming, and both follow from something the caller did, so neither is news. A direct caller still gets the `Err`. The new test was worthless as first written — it passed with the fix reverted, because `cargo fmt` had collapsed the match arm and the revert silently missed. Re-checked against the real text: it now fails with "a withdrawn request must not report against the flow that replaced it". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 11a6145e62

ℹ️ 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".

Both from review, both confirmed by a test that fails without the fix. The IQ-error branch never rechecked claim ownership, unlike the success branch beside it that turns the same lost claim into `Cancelled`. A 30s timeout or a server rejection easily outlives a `cancel_pair_code` plus its replacement, so the withdrawn request reported `RequestFailed` and the catch-all dispatched it against the flow that now owns the slot. It now makes the same recheck: losing the slot outranks how the request ended. `from_server` returned `Unknown(code)` for a contradicting code/text pair, which could not hold. The wire form of the enum *is* `code()`, so `Unknown(429)` serializes to `429` and rehydrates as `RateOverlimit` — verified — and `PairingCodeError` derives `Serialize`, so persisting or forwarding one silently undid the demotion and reapplied throttling. No in-band value can both record the code and refuse to alias the arm it came from, so it returns `None`: refused-but-unclassifiable and nothing-refused mean the same thing to a consumer, and the code still reaches them through the error's rendering. A round-trip test now pins that no reachable value rehydrates as a different one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

Left stale by the previous commit: `from_server` now needs both attributes and can decline to classify, so "the code alone identifies the refusal" said the opposite of what the type does. Names the constructor to use, and states that `code()` is the whole wire form — the fact that makes `Unknown` unable to carry a demotion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df0d1f9648

ℹ️ 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 · 2026-07-30T00:29:02Z

+            Err(e) => {
+                self.core.event_bus.dispatch(Event::PairingCodeError(


Avoid reporting validation failures against a live code

When a code is already outstanding and a second caller supplies an invalid phone number or custom code, pair_with_code_inner returns the validation error before checking pair_code_state; this catch-all then dispatches PairingCodeError even though the original code remains live. Fresh evidence beyond the fixed CodeAlreadyOutstanding case is that these pre-claim validation errors never reach that suppressed variant, so an uncorrelated handler can still treat the active flow as failed and tear it down or retry. Check for an outstanding flow before validation, or suppress pre-claim failures when another flow owns the slot.

Useful? React with 👍 / 👎.

Valid, and this is the fourth instance of one class you have surfaced — duplicate request, withdrawn request, its IQ failing, and now a bad number beside a live code. Four is enough to say the approach was wrong rather than incomplete: I kept enumerating error variants when the event's meaning is a statement about state.

The event says "no code is coming". The question was therefore never how the request failed but whether a code is nonetheless on its way, and only the state answers that. In 6f46563 the suppression asks it:

async fn failure_is_not_this_flows_to_report(self: &Arc<Self>, e: &PairError) -> bool { if e.lost_the_flow_to_another_request() { return true; } // A failing request that still owned the slot has released it by now, so // an outstanding flow here belongs to somebody else. self.pair_code_state.lock().await.is_outstanding(wacore::time::now_secs()) }

That closes your case and any future variant that can reach the dispatch while a flow is live, without me having to predict which.

Both branches are load-bearing, which I checked rather than assumed by removing each in turn:

Drop the state check → a_validation_failure_beside_a_live_code_is_not_reported fails with a live code must not be reported as failed by an unrelated bad request.

Drop the variant check → the two cancellation tests fail. A cancellation with no replacement leaves the slot idle, so nothing is live and the state check cannot see it; the caller asked for exactly that and does not need telling.

Not reordering validation to run after the outstanding check, which was your other option. That would change which error a caller gets for a genuinely bad number — CodeAlreadyOutstanding instead of PhoneNumberTooShort — hiding the input bug behind a transient one. Validation should keep winning that race; it is only the reporting that was wrong.

1321 wacore / 1299 whatsapp-rust green, clippy and rustdoc clean.

Generated by Claude Code

coderabbitai

Caution

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

⚠️ Outside diff range comments (1)

wacore/src/pair_code.rs (1)
647-671: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Don’t use WireEnum as a JSON/serde serialization helper.

PairCodeRejection is classified from <error code, text>, not dispatched through the #[wire] values; the existing derive just couples its JSON shape to attributes used for protocol wire mapping. Keep this manual: code(), Display, and serde implementations tied only to the needed persistence contract, not #[wire]. Add a regression test that named rehydrated values do not silently alias, instead of relying on #[wire] to guard that across Serialize/Deserialize.
🤖 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 647 - 671, Remove the WireEnum-based
serialization from PairCodeRejection and decouple its serde behavior from the
#[wire] mappings. Preserve or implement explicit code(), Display, Serialize, and
Deserialize behavior for the required persistence contract, including faithful
handling of named variants and Unknown(i32). Add a regression test that
serializes and rehydrates named PairCodeRejection values and verifies they
remain the same variant rather than aliasing.
Sources: Coding guidelines, Learnings

🤖 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 `@wacore/src/pair_code.rs`:
- Around line 647-671: Remove the WireEnum-based serialization from
PairCodeRejection and decouple its serde behavior from the #[wire] mappings.
Preserve or implement explicit code(), Display, Serialize, and Deserialize
behavior for the required persistence contract, including faithful handling of
named variants and Unknown(i32). Add a regression test that serializes and
rehydrates named PairCodeRejection values and verifies they remain the same
variant rather than aliasing.

ℹ️ Review info

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a7110cc-3582-4ebf-b34a-cf20ae9048d3

📥 Commits

Reviewing files that changed from the base of the PR and between 4ef497a and 32bef92.

📒 Files selected for processing (3)

src/pair_code.rs
wacore/src/pair_code.rs
wacore/src/types/events.rs

…iant Fourth instance of one class, so this fixes the class. Validation runs before the outstanding-flow check, so a second caller passing a bad phone number while a code is live fails as `PhoneNumberTooShort` — never reaching the suppressed variants — and reported "no code is coming" against a code that was on screen. Enumerating variants was the wrong shape and has now been wrong four times (duplicate request, withdrawn request, its IQ failing, and a bad number beside a live code). The event means "no code is coming", so the question was never *how* the request failed but whether a code is nonetheless on its way, which is a property of the state. A failing request that owned the slot has released it by the time this runs, so an outstanding flow here belongs to somebody else. The variant check stays, and is still load-bearing: a cancellation with no replacement leaves the slot idle, so nothing is live, yet the caller asked for exactly that and does not need telling. Verified by removing each check in turn — dropping the state check fails the new test, dropping the variant check fails the two cancellation tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E3jMXeFFcJepp2HNXoWGGM

jlucaso1 · 2026-07-30T00:36:32Z

Declining CodeRabbit's out-of-diff finding on wacore/src/pair_code.rs:647-671 ("don't use WireEnum as a JSON/serde serialization helper; keep code(), Display, and serde manual"), because it asks for exactly what AGENTS.md forbids:

Every protocol enum derives WireEnum, and its #[wire = ...] attribute is the single source of truth for the wire value. Do not also derive serde::Serialize/Deserialize or add #[serde(rename_all)] — the derive owns both.

PairCodeRejection is a protocol enum in the ordinary sense: its values are the literal code attribute of the server's <error> node, not an internal classification we invented. It is the same shape as NackReason and TempBanReason, which both derive WireEnum with #[wire(kind = "int")] and a #[wire_fallback] Unknown(i32). Hand-rolling serde here would make it the one protocol enum in the tree that owns its own JSON, which is the drift the rule exists to prevent.

The premise is also not quite right: the #[wire] values are what dispatch happens on. from_server classifies through From<i32>, which the derive generates from those attributes — the code/text pairing is a validation on top, not a separate mapping.

The second half of the suggestion is already done. pair_code_rejections_do_not_alias_on_a_round_trip (added in 32bef92) serializes and rehydrates every reachable value and asserts each stays the same variant — which is what surfaced the Unknown(429) → RateOverlimit aliasing that made from_server return Option in the first place.

If the coupling is genuinely unwanted, the right change is to the rule in AGENTS.md and to all the enums under it, not to this one type in this PR.

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f465638fa

ℹ️ 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 Bot added the api-design label Jul 29, 2026

coderabbitai Bot requested changes Jul 29, 2026

View reviewed changes

Comment thread src/bot.rs

Comment thread wacore/src/types/events.rs Outdated

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread wacore/src/types/events.rs Outdated

Comment thread src/pair_code.rs

Comment thread src/bot.rs

Comment thread src/pair_code.rs Outdated

coderabbitai Bot reviewed Jul 29, 2026

View reviewed changes

coderabbitai Bot approved these changes Jul 29, 2026

View reviewed changes

coderabbitai Bot requested changes Jul 29, 2026

View reviewed changes

Comment thread wacore/src/types/events.rs Outdated

coderabbitai Bot requested changes Jul 29, 2026

View reviewed changes

Comment thread src/pair_code.rs

coderabbitai Bot approved these changes Jul 29, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

Comment thread src/pair_code.rs

chatgpt-codex-connector Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread src/pair_code.rs Outdated

Comment thread wacore/src/pair_code.rs Outdated

coderabbitai Bot added the size-increase-ok Accepted binary-size increase: downgrades the per-PR size gate to a warning label Jul 30, 2026

chatgpt-codex-connector Bot reviewed Jul 30, 2026

View reviewed changes

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jul 30, 2026

View reviewed changes

Comment thread wacore/src/pair_code.rs

jlucaso1 merged commit ec363de into main Jul 30, 2026
28 of 29 checks passed

jlucaso1 deleted the claude/whatsapp-bundle-whatspec-w0hf5k branch July 30, 2026 01:02

jlucaso1 mentioned this pull request Jul 30, 2026

docs(pair-code): document PairingCodeError, PairCodeRejection, and on_pair_code_error oxidezap/whatsapp-rust-docs#468

Merged

coderabbitai Bot mentioned this pull request Jul 30, 2026

fix(pair-code): read the companion_finish answer instead of assuming it #1198

Merged

		Err(e) => {
		self.core.event_bus.dispatch(Event::PairingCodeError(

Uh oh!

Uh oh!

Conversation

jlucaso1 commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Where it goes

What WhatsApp Web actually does

1. The refusal set is closed, and the IR has it

2. Their error carries the status structurally, not as a string

3. They branch on it — two statuses, distinctly

The fix

The status

The event

Where it is dispatched

What is not reported, and why it is a state question

Also

Tests

Verification

Caveats

Review history

Uh oh!

coderabbitai Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Binary size report

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

jlucaso1 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

jlucaso1 commented Jul 29, 2026 •

edited

Loading

coderabbitai Bot commented Jul 29, 2026 •

edited

Loading

github-actions Bot commented Jul 29, 2026 •

edited

Loading