docs(agents): correct stale agent docs and add a WA Web verification path

AGENTS.md

@@ -1,50 +1,58 @@
 # WhatsApp-Rust
 
-Rust implementation of the WhatsApp protocol, inspired by **whatsmeow** (Go), **Baileys** (TypeScript), and real **WhatsApp Web** behavior. Covers QR pairing, E2E encrypted messaging (1-on-1 + group), media upload/download, and connection management.
+Rust implementation of the WhatsApp protocol: QR pairing, E2E encrypted messaging (1-on-1 + group), media, VoIP, connection management.
 
-## Crate Structure
+Ground truth for protocol behavior is WhatsApp Web itself: query the structured [whatspec](https://github.com/oxidezap/whatspec) IR first, drop to the raw bundle in `docs/captured-js/` when it can't answer, and treat **whatsmeow** (Go) and **Baileys** (TypeScript) as second opinions. See `agent_docs/wa_web_reference.md`.
 
-- **wacore** — Platform-agnostic core: binary protocol, crypto, IQ types, state traits. No Tokio dependency.
-- **waproto** — Protobuf definitions (`whatsapp.proto`) compiled via prost. No feature logic here.
-- **whatsapp-rust** — Main client: Tokio runtime, SQLite persistence (Diesel), high-level API.
+## Crates
 
-## Build & Verify
+- **wacore** — platform-agnostic core: binary protocol, crypto, IQ types, state traits. Also builds for wasm32 and ESP32, so no Tokio here.
+- **waproto** — prost-generated protobufs from `whatsapp.proto`. No feature logic.
+- **whatsapp-rust** — Tokio runtime, SQLite persistence (Diesel), high-level API.
+
+## Build & verify
 
 ```bash
 cargo fmt --all
-cargo clippy --all --tests
-cargo test --all
-cargo test -p e2e-tests          # requires mock server running
+cargo test -p <touched crate> --lib                     # fast local loop
+cargo clippy --workspace --all-targets -- -D warnings   # what CI enforces
 ```
 
-## Rust Style
+Workspace clippy takes minutes — pushing and letting CI parallelize the matrix is usually faster. E2E tests (`cargo test -p e2e-tests`) need the mock server running; see `agent_docs/e2e_testing.md`.
+
+## Gotchas
+
+Things that look correct and are not:
 
-- **Collapsible if**: Always use let-chains (`if let Some(x) = foo && let Some(y) = x.bar { ... }`) instead of nested `if let` blocks. Clippy's `collapsible_if` lint will reject the nested form.
-- **No real PII in tests**: Use fictitious phone numbers and JIDs in test code. Never commit real user numbers.
+- **Device state.** Never mutate `Device` directly, not even in tests — a write-lock mutation bypasses the cached snapshot. Mutate through `DeviceCommand` + `PersistenceManager::process_command()`; read through `get_device_snapshot()`, which returns a cached `Arc<Device>` (sync, refcount-cheap, safe per message) — hold it and borrow fields instead of cloning them. `get_device_arc()` exists only for store adapters that need `&mut Device` trait access.
+- **Locks.** `session_locks` serializes Signal encrypt/decrypt per protocol address; `chat_lanes` (`ChatLane::enqueue_lock` in `src/client.rs`) serializes *incoming* processing per chat. Outgoing sends are deliberately not per-chat locked — WA Web doesn't lock them either.
+- **Wire-tagged enums.** 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. In tagged mode it generates a sibling `<Name>Tag`; parsers must dispatch on `<Name>Tag::try_from(node.tag.as_ref())` rather than string literals, so renaming a tag stays a one-attribute change. Modes and attributes: `agent_docs/protocol_architecture.md`.
+- **Event payloads are a frozen API.** Sealed with `#[non_exhaustive]` + `#[derive(bon::Builder)]` and constructed via `Type::builder()…build()`; a maybe-absent field is `Option<T>`, never an empty-string or zero sentinel. The full stability policy is the `Event` doc comment in `wacore/src/types/events.rs`.
+- **Blocking work** — `ureq`, heavy CPU — belongs in `tokio::task::spawn_blocking`; it shares a runtime with the read loop.
+- **let-chains**, never nested `if let`. Clippy's `collapsible_if` is denied in CI.
+- **No real PII in tests**, including vectors derived from production captures. Regenerate them from fictitious JIDs and numbers.
+- **Errors**: `thiserror` for typed errors, `anyhow` where several failure kinds meet. No `.unwrap()` outside tests.
 
-## Critical Conventions
+## Adding a feature
 
-- **State**: Never modify Device state directly (not even in tests — a write-lock mutation bypasses the cached snapshot). Use `DeviceCommand` + `PersistenceManager::process_command()` (or `modify_device` internally). Read via `get_device_snapshot()` — it returns a cached `Arc<Device>` (sync, refcount-cheap, safe to call per message); borrow fields from the held snapshot instead of cloning them. `get_device_arc()` is only for store adapters that need `&mut Device` trait access.
-- **Async**: All I/O uses Tokio. Wrap blocking I/O (`ureq`) and heavy CPU work in `tokio::task::spawn_blocking`.
-- **Concurrency**: `session_locks` serializes per-sender Signal encrypt/decrypt. `message_enqueue_locks` serializes per-chat incoming message processing. Outgoing sends are not per-chat locked (matches WA Web).
-- **Errors**: `thiserror` for typed errors, `anyhow` for multi-failure functions. No `.unwrap()` outside tests.
-- **Protocol**: Cross-reference **whatsmeow**, **Baileys**, and captured WhatsApp Web JS (`docs/captured-js/`) to verify implementations.
-- **IQ Requests**: Use `client.execute(Spec::new(&jid)).await?` pattern. IqSpec constructors take `&Jid` not `Jid`.
-- **New features**: Expose via `src/features/mod.rs`, re-export in `src/lib.rs`.
-- **Event payloads**: Seal each payload struct with `#[non_exhaustive]` + `#[derive(bon::Builder)]` so fields can be added without breaking consumers; construct via the generated builder (`Type::builder()…build()`), not a struct literal. Model a maybe-absent field as `Option<T>` (gets a `maybe_*` setter), never an empty-string/zero sentinel. Every event payload is sealed this way (unit-marker events too, as empty sealed structs built via `X::builder().build()`); see the `Event` doc in `wacore/src/types/events.rs` for the full stability policy.
-- **Wire-tagged enums**: Every protocol enum uses `#[derive(WireEnum)]`. The `#[wire = "..."]` (or `#[wire = NUM]` for int mode) attribute is the SINGLE source of truth for each variant's wire value. Do NOT also derive `serde::Serialize`/`Deserialize` or add `#[serde(rename_all)]` — the derive owns both. Three modes: unit-string (default), tagged-with-payload (`#[wire(tag = "type")]` on the enum, optional `#[wire_alias = "..."]` and `#[wire(skip)]` on fields, `#[wire_fallback]` for catch-all), and int (`#[wire(kind = "int")]`). In tagged mode the derive auto-generates a sibling `<Name>Tag` enum; parsers must dispatch via `<Name>Tag::try_from(node.tag.as_ref())` instead of matching string literals, so renaming a wire tag stays a single-attribute change.
+Find the wire format before designing anything — see `agent_docs/feature_implementation.md`. IQ requests go through `client.execute(Spec::new(&jid)).await?`, and `IqSpec` constructors take `&Jid` so callers need not clone. Public surface is `pub use` in `src/features/*.rs`, re-exported from `src/features/mod.rs` and `src/lib.rs`.
 
-## Detailed Docs
+Comments carry the *why* of a decision, at the single point where it is made. Repeating a rationale at call sites is how it goes stale.
 
-Read these when working on the relevant area:
+## Detailed docs
 
-- `agent_docs/protocol_architecture.md` — ProtocolNode, IqSpec, derive macros, node parsing
-- `agent_docs/feature_implementation.md` — Step-by-step feature implementation flow
-- `agent_docs/e2e_testing.md` — E2E test patterns, file organization, event-driven waiting
-- `agent_docs/debugging.md` — evcxr REPL, binary protocol debugging
-- `agent_docs/binary_size_ci.md` — size-tracking CI: metrics, budgets, baseline semantics
-- `agent_docs/observability.md` — per-session stats (I/O, memory report, TaskInstrument/CPU), design rules
-- `agent_docs/plugin_architecture.md` — native plugin host, lifecycle/capability invariants, future foreign adapter seam
-- `agent_docs/signal_durability.md` — Signal counter leases, pre-wire gates, crash recovery, review checklist
+Read the one that covers what you are touching:
 
-When adding comments to the code, dont be so verbose, also only explain why, not what
+| Doc | Read it when |
+| --- | --- |
+| `agent_docs/wa_web_reference.md` | Confirming any protocol behavior, limit, enum value, or stanza shape against real WA Web |
+| `agent_docs/protocol_architecture.md` | Building or parsing stanzas: `ProtocolNode`, `IqSpec`, derive macros, node helpers |
+| `agent_docs/noise_handshake.md` | Connection setup: XX/IK/fallback selection, server cert cache, failure classification |
+| `agent_docs/feature_implementation.md` | Starting a feature and needing its wire format from captured WA Web JS |
+| `agent_docs/signal_durability.md` | Any code that reads, mutates, persists, or sends Signal state |
+| `agent_docs/e2e_testing.md` | Writing or fixing tests under `tests/e2e/` |
+| `agent_docs/observability.md` | Adding a cache, counter, or anything reported by `memory_report()` / `stats()` |
+| `agent_docs/plugin_architecture.md` | Touching the `plugins` / `client-lifecycle` feature surface |
+| `agent_docs/voip_audio_codecs.md` | VoIP media: codec profiles, negotiation, encoded audio API |
+| `agent_docs/binary_size_ci.md` | A size gate failed, or a change adds dependencies or generic instantiations |
+| `agent_docs/debugging.md` | Decoding raw binary-protocol bytes by hand |

agent_docs/debugging.md

@@ -62,4 +62,4 @@ WhatsApp binary protocol uses nibble encoding for numeric strings. Each byte con
 // Encode: "100000000000001" -> "100000000000001f"
 ```
 
-See `wacore/binary/src/nibble.rs` for the implementation.
+The implementation is split across `wacore/binary/src/encoder.rs` and `decoder.rs`. The full token dictionaries WhatsApp Web uses are in whatspec's `generated/tokens/index.json` (see `wa_web_reference.md`) when a byte doesn't decode to what you expect.

agent_docs/e2e_testing.md

@@ -1,121 +1,55 @@
-# E2E Testing Best Practices
+# E2E tests
 
-E2E tests live in `tests/e2e/` and run against a mock WhatsApp server. They test real connection flows, encryption, and event delivery.
+`tests/e2e/` runs real connection, encryption, and event-delivery flows against a mock WhatsApp server. `tests/e2e/src/lib.rs` holds `TestClient`, which connects, waits for pairing and sync, and provides the event-based assertions.
 
-## Test Infrastructure
+## The one rule
 
-- **`tests/e2e/src/lib.rs`**: `TestClient` helper — connects to mock server, waits for pairing + sync, provides event-based assertions.
-- Each `TestClient` owns an isolated `InMemoryBackend`; the mock server is shared.
-- Libtest runs tests within a test binary **in parallel** by default. Never rely on test order.
-- Use `unique_push_name()` for server-side account isolation. For a multi-device test,
-  create one unique name and pass it only to that test's related clients.
-- CI pins the mock-server image by digest. Update it deliberately with the matching
-  server change so an unchanged client commit always runs against the same protocol peer.
-- Local runs must start the mock with `CHATSTATE_TTL_SECS=3`; `chatstate_ttl.rs`
-  intentionally uses the same shortened expiry as CI.
+**Never synchronize on a fixed sleep.** Long enough to be reliable is slow; short enough to be fast is flaky. Wait on the condition itself — an event, or a bounded poll that fails with a clear message.
 
-## File Organization
-
-Split test files by domain so ownership and failures stay clear. Do not use file boundaries
-as a synchronization mechanism; correctness must not depend on how Cargo schedules test targets.
-
-```
-tests/e2e/tests/
-├── chat_actions.rs      # Pin, mute, archive, star
-├── connection.rs        # Connect, reconnect
-├── groups.rs            # Group CRUD, admin, settings
-├── media.rs             # Upload, download, send media
-├── messaging.rs         # Send/receive text messages
-├── chatstate_ttl.rs     # Chatstate expiry with the CI's 3-second mock TTL
-├── offline_groups.rs    # Offline group notifications
-├── offline_messages.rs  # Offline message queuing + delivery
-├── offline_receipts.rs  # Offline receipt + presence delivery
-├── presence.rs          # Typing indicators, availability
-├── profile.rs           # Push name, status text
-├── profile_picture.rs   # Profile picture CRUD
-└── receipts.rs          # Online receipt routing
+```rust
+// Returns as soon as the event arrives
+let event = client_b
+    .wait_for_event(15, |e| e.messages().any(|m| m.message.conversation.as_deref() == Some("hello")))
+    .await?;
 ```
 
-When adding new tests, place them in the file matching their domain. If a file grows beyond ~10-15 tests, consider splitting further.
-
-## Recovery and Race Regressions
-
-Make recovery tests deterministic with narrow `test-util` fault hooks, then wait for an
-observable event, stanza, or bounded state transition. Do not depend on CPU load to hit a race, and
-do not raise a readiness timeout to hide a failed bootstrap phase. `app_state.rs`'s missing-key
-test is the reference pattern: remove exactly the required state, trigger a real sync, and
-assert that recovery reaches the wire.
+`groups.rs` uses zero sleeps and runs at ~2.2s per test. For state with no corresponding event, poll it with a deadline; raising a readiness timeout to make a failing bootstrap pass is not a fix.
 
-## Event-Driven Waiting (Preferred)
+Timeouts that hold up in practice: 10-15s for event waits in online flows (events normally arrive in under a second), 30s after an offline reconnect (reconnect plus queue drain), 3-5s for negative assertions, 5s for `wait_for_disconnected`.
 
-Use `wait_for_event()` with predicates instead of arbitrary sleeps. This is both faster and more reliable:
+## Isolation
 
-```rust
-// GOOD: event-driven — returns as soon as the event arrives
-let event = client_b
-    .wait_for_event(15, |e| e.messages().any(|m| m.message.conversation.as_deref() == Some("hello")))
-    .await?;
+Each `TestClient` owns an isolated `InMemoryBackend`; the mock server is shared. Libtest runs tests inside a binary in parallel, so nothing may depend on test order — and file boundaries are organization, not synchronization.
 
-// BAD: arbitrary sleep — wastes time or causes flaky failures
-tokio::time::sleep(Duration::from_secs(2)).await;
-```
+`unique_push_name()` gives server-side account isolation. In a multi-device test, create one unique name and pass it only to the clients that must share an account.
 
-Reference: `groups.rs` uses zero sleeps and runs at ~2.2s/test. Follow this pattern for new tests.
+CI pins the mock-server image by digest, so an unchanged client commit always runs against the same protocol peer; bump it deliberately, together with the matching server change. Local runs need `CHATSTATE_TTL_SECS=3` on the mock — `chatstate_ttl.rs` depends on the same shortened expiry CI uses.
 
-## Offline Testing Pattern
+## Connect, disconnect, reconnect
 
-`reconnect()` tears the socket down in a background task, so the client is still
-online when it returns. Poll for the transition with `wait_for_disconnected()`
-instead of sleeping — `Event::Disconnected` is suppressed for expected
-disconnects, so the connection flag is the observable:
+The distinction that catches people: **`reconnect()` tears the socket down in a background task**, so the client is still online when it returns. `Event::Disconnected` is suppressed for expected disconnects, which makes the connection flag the only observable.
 
 ```rust
-// Client goes offline (triggers auto-reconnect in background)
 client_b.client.reconnect().await;
 client_b.wait_for_disconnected(5).await?;
 
-// Now send while client is offline — server queues it
+// Now offline — the server queues this
 client_a.client.send_message(jid_b.clone(), message).await?;
 
-// Client reconnects automatically and receives from offline queue
+// Auto-reconnect drains the offline queue
 let event = client_b.wait_for_event(30, |e| matches!(e, Event::Messages(_))).await?;
 ```
 
-Full disconnects need nothing: `TestClient::disconnect()` awaits the run task, so
-the client is normally already offline when it returns. It caps that wait at 5s
-and warns instead of failing, so a test that depends on the client being offline
-afterwards should still assert it rather than assume it.
-
-```rust
-client_b.disconnect().await;
-```
-
-## `reconnect_and_wait()` Helper
-
-Use `TestClient::reconnect_and_wait()` when you need the client back online (not testing offline behavior):
-
-```rust
-// Reconnects and waits for Connected event — no arbitrary sleep needed
-client_b.reconnect_and_wait().await?;
-```
+`TestClient::disconnect()` awaits the run task, so the client is normally already offline on return — but it caps that wait at 5s and warns rather than failing, so a test that depends on being offline afterwards should assert it.
 
-Do NOT use this for offline tests — it waits for the client to be back online, defeating the purpose.
+`reconnect_and_wait()` waits for the client to come back online. Using it in an offline test defeats the test.
 
-## Timeout Guidelines
+## Recovery and race regressions
 
-- **Event waits in online flows**: 10-15s (events arrive in <1s normally)
-- **Event waits after offline reconnect**: 30s (reconnect + offline queue drain)
-- **Negative assertions** (event should NOT arrive): 3-5s
-- **Going offline** (`wait_for_disconnected`): 5s
+Make these deterministic with narrow `test-util` fault hooks, then wait for an observable event, stanza, or bounded state transition. Never depend on CPU load to hit a race. The reference pattern is `app_state.rs`'s missing-key test: remove exactly the required state, trigger a real sync, assert that recovery reaches the wire.
 
-Never synchronize on a fixed sleep: long enough to be reliable is slow, short
-enough to be fast is flaky. Wait on the condition itself — an event, or a poll
-with a bounded deadline that fails with a clear message.
+## Writing a new test
 
-## Writing New E2E Tests
+Put it in the file matching its domain, or add one — if a file passes ~10-15 tests, split it. Use `TestClient::connect("unique_prefix")` with a prefix unique per client per test, return `anyhow::Result<()>`, `disconnect()` every client at the end, and initialize logging with `let _ = env_logger::builder().is_test(true).try_init();`.
 
-1. Use `TestClient::connect("unique_prefix")` with a unique prefix per client per test.
-2. Use `wait_for_event()` for all assertions; for state with no event, poll it with a bounded deadline. Never sleep.
-3. Always call `disconnect()` on all clients at the end (cleanup).
-4. Return `anyhow::Result<()>` for clean error propagation.
-5. Use `env_logger` for debug output: `let _ = env_logger::builder().is_test(true).try_init();`
+Cover the failure alongside the success: a guard with two conditions needs both negatives.