fix(recv): retry (not NACK) InvalidSignedPreKeyId on the 1:1 decrypt path

[jlucaso1]

What

Handle SignalProtocolError::InvalidSignedPreKeyId on the session (pkmsg/msg) decrypt path by sending a retry receipt (RetryReason::InvalidKeyId) instead of letting it fall to the catch-all 500 NACK.

Why (bug)

When a PreKeySignalMessage names a signed prekey we've rotated past SIGNED_PRE_KEY_RETENTION (src/features/rotate_key.rs), SignedPreKeyAdapter::get_signed_pre_key returns InvalidSignedPreKeyId. The session-decrypt match in src/message/receive.rs handled SessionNotFound, BadMac/InvalidMessage, InvalidPreKeyId, UntrustedIdentity, DuplicatedMessage — but not InvalidSignedPreKeyId, so it hit the else catch-all → spawn_nack(UnhandledError). A 500 NACK clears the stanza from the offline queue → permanent, silent 1:1 message loss.

WhatsApp Web classifies this as a SignalDecryptionError → SignalRetryable → retry receipt (WAWeb/Msg/ProcessingDecryptionHandler.js); the receipt carries our live bundle so the peer rebuilds against the current signed prekey. The sibling InvalidPreKeyId arm already retries — this was an inconsistent drop for the signed-prekey case.

How

• Add an else if matches!(e, InvalidSignedPreKeyId) arm before the catch-all that routes to the existing handle_decrypt_failure(RetryReason::InvalidKeyId, …), mirroring the adjacent InvalidPreKeyId arm. The catch-all 500 NACK is preserved for genuinely non-Signal errors.

Tests

• test_invalid_signed_prekey_id_sends_retry_receipt — builds a bundle whose signed-prekey id the client never provisioned (the id is not signed, so the bundle is still accepted), so decrypt hits InvalidSignedPreKeyId; asserts a retry receipt with RetryReason::InvalidKeyId is emitted (pre-fix this took the terminal NACK path, which never bumps the retry caches).
• Refactors the shared bobs_prekey_bundle test helper into bobs_prekey_bundle_with_spk_id(client, spk_id); the original delegates with spk_id = 1 (no behavior change for existing callers).
• cargo fmt, cargo clippy -p whatsapp-rust --lib --tests clean; new test passes.

🤖 Generated with Claude Code

---

Generated by Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Warning

Review limit reached

@jlucaso1, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5c5f9aa3-96dd-4c99-8e71-71cf053de5e7

📥 Commits

Reviewing files that changed from the base of the PR and between a8ba6fd and dabdf63.

📒 Files selected for processing (2)

• src/message/receive.rs
• src/message/tests.rs

📝 Walkthrough

Walkthrough

The InvalidPreKeyId retry-handling call in receive.rs is reformatted for multiline style without behavior change. In tests.rs, the prekey bundle helper is parameterized to accept a signed prekey id, and a new test verifies that an unprovisioned signed prekey id triggers a retry receipt with RetryReason::InvalidKeyId.

Changes

Retry Path and Test Coverage

Layer / File(s)	Summary
Reformat InvalidPreKeyId retry arm src/message/receive.rs	The handle_decrypt_failure call in the InvalidPreKeyId match arm is reformatted into a multiline call, preserving the same failure-marking and retry behavior.
Parameterized prekey bundle and InvalidSignedPreKeyId regression test src/message/tests.rs	bobs_prekey_bundle now delegates to a new bobs_prekey_bundle_with_spk_id helper that drives the signed prekey id via a parameter, and a new async test confirms decrypting with an unprovisioned signed prekey id fails and emits a retry receipt with RetryReason::InvalidKeyId.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• oxidezap/whatsapp-rust#254: Overlaps on the failure→retry-receipt flow, since #254 changes retry-receipt construction/key inclusion based on retry reason.
• oxidezap/whatsapp-rust#664: Both touch process_session_enc_batch's decryption-failure path and retry routing via handle_decrypt_failure.
• oxidezap/whatsapp-rust#983: Both touch process_session_enc_batch's RetryReason handling in receive.rs affecting the same routing paths.

Suggested reviewers: greptile-apps

Look, this needs to be airtight — key mismatch handling is the kind of thing that breaks trust at scale, and trust is everything. The test coverage here is a good start, but I want to make sure this pattern gets extended everywhere it matters. Ship it once it's clean.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main fix: retrying InvalidSignedPreKeyId instead of NACK on the 1:1 decrypt path.
Description check	✅ Passed	The description directly matches the changeset and explains the bug, fix, and test coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/fix-invalid-signed-prekey-retry

---

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

Adds an else if arm for SignalProtocolError::InvalidSignedPreKeyId in the 1:1 session-decrypt error chain so that messages from peers using a rotated signed prekey trigger a retry receipt (RetryReason::InvalidKeyId) rather than a terminal 500 NACK that silently drops the stanza from the offline queue.

• src/message/receive.rs: New else if arm inserted between InvalidPreKeyId and the catch-all; calls the same handle_decrypt_failure(RetryReason::InvalidKeyId) path already used by the sibling arm.
• src/message/tests.rs: Refactors bobs_prekey_bundle into a bobs_prekey_bundle_with_spk_id(client, spk_id) helper and adds test_invalid_signed_prekey_id_sends_retry_receipt to assert a retry receipt is emitted (rather than a terminal NACK) when the message carries an unprovisioned signed-prekey id.

Confidence Score: 5/5

Safe to merge; the change is a targeted, additive fix to a previously uncovered error arm.

The fix adds a single well-scoped arm to an existing if/else chain, using the same call path already proven by the InvalidPreKeyId arm. The new test confirms the retry receipt is emitted and that the terminal NACK path is no longer taken. No pre-existing arm is modified, and the catch-all for genuinely unhandled errors is preserved.

No files require special attention.

Important Files Changed

Filename	Overview
src/message/receive.rs	Inserts a correctly structured else if InvalidSignedPreKeyId arm before the catch-all NACK, routing to handle_decrypt_failure(RetryReason::InvalidKeyId) — identical to the existing InvalidPreKeyId arm.
src/message/tests.rs	Extracts bobs_prekey_bundle_with_spk_id from the existing helper (no behavior change for existing callers), and adds a focused end-to-end test that verifies the retry receipt path for InvalidSignedPreKeyId.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[process_session_enc_batch] --> B{decrypt succeeds?}
    B -- yes --> C[outcome.decrypted = true]
    B -- no --> D{error type?}
    D -- SessionNotFound --> E[try PN→LID migration]
    E -- Decrypted --> C
    E -- NotDecrypted --> F[retry receipt: NoSession]
    D -- BadMac / InvalidMessage --> G[try PN→LID migration]
    G -- Decrypted --> C
    G -- NotDecrypted --> H[retry receipt: BadMac / InvalidMessage]
    D -- InvalidPreKeyId --> I[try PN→LID migration]
    I -- Decrypted --> C
    I -- NotDecrypted --> J[retry receipt: InvalidKeyId]
    D -- InvalidSignedPreKeyId --> K["NEW: retry receipt: InvalidKeyId"]
    D -- DuplicatedMessage --> L[duplicate = true]
    D -- UntrustedIdentity --> M[retry receipt: identity]
    D -- other --> N["catch-all: 500 NACK (UnhandledError)"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[process_session_enc_batch] --> B{decrypt succeeds?}
    B -- yes --> C[outcome.decrypted = true]
    B -- no --> D{error type?}
    D -- SessionNotFound --> E[try PN→LID migration]
    E -- Decrypted --> C
    E -- NotDecrypted --> F[retry receipt: NoSession]
    D -- BadMac / InvalidMessage --> G[try PN→LID migration]
    G -- Decrypted --> C
    G -- NotDecrypted --> H[retry receipt: BadMac / InvalidMessage]
    D -- InvalidPreKeyId --> I[try PN→LID migration]
    I -- Decrypted --> C
    I -- NotDecrypted --> J[retry receipt: InvalidKeyId]
    D -- InvalidSignedPreKeyId --> K["NEW: retry receipt: InvalidKeyId"]
    D -- DuplicatedMessage --> L[duplicate = true]
    D -- UntrustedIdentity --> M[retry receipt: identity]
    D -- other --> N["catch-all: 500 NACK (UnhandledError)"]

Loading

_{Reviews (2): Last reviewed commit: "docs(recv): tighten InvalidSignedPreKeyI..." | Re-trigger Greptile}

[cubic-dev-ai]

No issues found across 2 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

Not reviewed (too large): src/message/tests.rs (~77 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

<sub>Auto-approved: Adds retry handling for InvalidSignedPreKeyId, mirroring existing InvalidPreKeyId handling. Isolated change with tests.

Re-trigger cubic</sub>

[github-actions]

📦 Binary size report

Metric	main	PR	Δ
bin size (stripped)	10.78 MiB	10.78 MiB	-352 B (-0.00%) 🔽
bin .text	8.79 MiB	8.78 MiB	-384 B (-0.00%) 🔽
bin allocated (text+data+bss)	10.78 MiB	10.78 MiB	-4.00 KiB (-0.04%) 🔽
llvm-lines wacore	504,289	504,289	0
llvm-lines wacore copies	17,275	17,275	0
llvm-lines whatsapp-rust lib	750,924	751,079	+155 (+0.02%) 🔺
llvm-lines whatsapp-rust lib copies	24,486	24,487	+1 (+0.00%) 🔺
deps crates (Cargo.lock)	466	466	0

.text per crate

Crate	main	PR	Δ
.text whatsapp_rust	1.61 MiB	1.61 MiB	-396 B (-0.02%) 🔽
.text wacore	530.38 KiB	530.38 KiB	0
.text wacore_binary	157.49 KiB	157.49 KiB	0
.text wacore_libsignal	178.73 KiB	178.73 KiB	0
.text wacore_appstate	156.42 KiB	156.42 KiB	0
.text wacore_noise	26.05 KiB	26.05 KiB	0
.text waproto	1.60 MiB	1.60 MiB	0
.text whatsapp_rust_sqlite_storage	512.98 KiB	512.98 KiB	0
.text whatsapp_rust_tokio_transport	43.61 KiB	43.61 KiB	0
.text whatsapp_rust_ureq_http_client	10.47 KiB	10.47 KiB	0
.text std	1022.23 KiB	1022.24 KiB	+16 B (+0.00%) 🔺
.text other deps	2.95 MiB	2.95 MiB	0

Baseline: 8414b4304 (latest main run) · Head: 2c8868b81 · Graphs

[cubic-dev-ai]

0 issues found across 2 files (changes from recent commits).

Not reviewed (too large): src/message/tests.rs (~8 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

<sub>Auto-approved: Bug fix: adds missing handling for InvalidSignedPreKeyId decrypt error to send retry receipt instead of terminal NACK, plus test coverage. Low risk, isolated change.

Re-trigger cubic</sub>