refactor(events): complete the event-payload API freeze - #1004
Conversation
Final tranche of the pre-1.0 freeze (after ServerAck in #1002 and the notification/sync payloads in #1003). Seals every remaining event payload so fields can be added later without breaking consumers, and clears the related tech debt in the same pass. Sealing (#[non_exhaustive] + #[derive(bon::Builder)]): - 25 field-bearing payloads: Receipt, InboundMessage, MessageBatch, UndecryptableMessage, LoggedOut, Disconnected, PairSuccess, PairError, PairPasskey{Request,Confirmation,Error}, DeviceListUpdate + DeviceNotificationInfo, IdentityChange, BusinessStatusUpdate, DisappearingModeChanged, MexNotification, NewsletterLiveUpdate(+Message, +Reaction), TemporaryBan, ConnectFailure, StreamError, OfflineSyncPreview, OfflineSyncCompleted. - 3 inline Event variants converted to sealed newtypes: PairingQrCode, PairingCode, PairingCodeRefresh. Event::kind() and every construct/match site updated accordingly. - 4 unit-marker events converted to empty sealed structs: Connected, ClientOutdated, QrScannedWithoutMultidevice, StreamReplaced — built via X::builder().build(). Tech debt cleared along the way: - EventInterest bitmask migrated u64 -> u128, doubling the EventKind ceiling from 64 to 128 (was at 58/64); CAPACITY and the build-time tripwire updated. All ~62 cross-crate construction/match sites in whatsapp-rust plus the in-crate wacore test sites and one e2e-tests reader migrated to the builder / newtype form. Behavior-preserving: same values, same control flow; Option fields routed through maybe_* setters. Compiler-enforced end to end (E0639 on any missed literal, typestate build() on any missed required field). Deliberately not added: a trybuild compile-fail guard. The repo builds on nightly, where trybuild .stderr snapshots drift on every toolchain bump; the seal is already enforced structurally by E0639, so a snapshot test would be net-negative maintenance debt. AGENTS.md and the Event doc updated: the freeze is complete, not rolling.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughEvent payloads are sealed and built through ChangesEvent payload sealing and dispatch migration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| wacore/src/types/events.rs | Core change: seals all remaining payload structs with #[non_exhaustive] + bon::Builder, extracts three inline Event variants to newtype structs, converts four unit-marker events to empty sealed structs, migrates EventInterest from u64 to u128, and corrects ConnectFailure::message from String to Option. All consistent and correct. |
| src/client/node_io.rs | Updates all ConnectFailure, LoggedOut, StreamReplaced, StreamError, TemporaryBan, and ClientOutdated construction sites to use builders; correctly routes the optional message attribute through maybe_message(Option) instead of unwrap_or(""). No behavior changes other than the sentinel→Option fix. |
| src/pair.rs | Migrates PairSuccess, PairError, and PairingQrCode construction to builder form; match sites updated to newtype pattern. Mechanical and correct. |
| src/pair_code.rs | Migrates PairingCode and PairingCodeRefresh dispatch and match sites to builder/newtype form, including two test assertions. Clean. |
| src/receipt.rs | Both Receipt construction sites migrated to builder, using r#type(…) raw-identifier setter correctly. Behavior preserved. |
| src/handlers/notification/device.rs | DeviceListUpdate and DeviceNotificationInfo construction migrated to builders; Option fields (lid_user, key_index, contact_hash) correctly routed through maybe_* setters. |
| src/bot.rs | All PairingCode and PairingCodeRefresh match and construction sites updated to newtype pattern; test helper pairing_code_event migrated to builder. Clean. |
| AGENTS.md | Event payload policy updated from 'rolling out per struct' to 'every payload is sealed', accurately reflecting the completed freeze. |
| tests/e2e/src/lib.rs | Single PairingQrCode match site updated to newtype pattern. Correct. |
| src/passkey/flow.rs | PairPasskeyRequest, PairPasskeyConfirmation, and PairPasskeyError construction migrated to builder at all five dispatch/return sites. Clean. |
Class Diagram
%%{init: {'theme': 'neutral'}}%%
classDiagram
class Event {
<<non_exhaustive enum>>
+PairingQrCode(PairingQrCode)
+PairingCode(PairingCode)
+PairingCodeRefresh(PairingCodeRefresh)
+Connected(Connected)
+ClientOutdated(ClientOutdated)
+QrScannedWithoutMultidevice(QrScannedWithoutMultidevice)
+StreamReplaced(StreamReplaced)
+LoggedOut(LoggedOut)
+Disconnected(Disconnected)
+ConnectFailure(ConnectFailure)
+StreamError(StreamError)
+Messages(MessageBatch)
+Receipt(Receipt)
+PairSuccess(PairSuccess)
+PairError(PairError)
+kind() EventKind
}
class EventInterest {
<<u128 bitmask>>
+ALL: EventInterest
+CAPACITY: u8 = 128
+of(kinds) EventInterest
+with(kind) EventInterest
+wants(kind) bool
}
class ConnectFailure {
<<non_exhaustive>>
+reason: ConnectFailureReason
+message: Option~String~
+raw: Option~Node~
}
class MessageBatch {
<<non_exhaustive>>
+messages: Arc~[InboundMessage]~
+origin: BatchOrigin
}
class InboundMessage {
<<non_exhaustive>>
+message: Arc~wa::Message~
+info: Arc~MessageInfo~
}
Event --> EventInterest : filtered by
Event --> ConnectFailure
Event --> MessageBatch
MessageBatch --> InboundMessage
%%{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"}}}%%
classDiagram
class Event {
<<non_exhaustive enum>>
+PairingQrCode(PairingQrCode)
+PairingCode(PairingCode)
+PairingCodeRefresh(PairingCodeRefresh)
+Connected(Connected)
+ClientOutdated(ClientOutdated)
+QrScannedWithoutMultidevice(QrScannedWithoutMultidevice)
+StreamReplaced(StreamReplaced)
+LoggedOut(LoggedOut)
+Disconnected(Disconnected)
+ConnectFailure(ConnectFailure)
+StreamError(StreamError)
+Messages(MessageBatch)
+Receipt(Receipt)
+PairSuccess(PairSuccess)
+PairError(PairError)
+kind() EventKind
}
class EventInterest {
<<u128 bitmask>>
+ALL: EventInterest
+CAPACITY: u8 = 128
+of(kinds) EventInterest
+with(kind) EventInterest
+wants(kind) bool
}
class ConnectFailure {
<<non_exhaustive>>
+reason: ConnectFailureReason
+message: Option~String~
+raw: Option~Node~
}
class MessageBatch {
<<non_exhaustive>>
+messages: Arc~[InboundMessage]~
+origin: BatchOrigin
}
class InboundMessage {
<<non_exhaustive>>
+message: Arc~wa::Message~
+info: Arc~MessageInfo~
}
Event --> EventInterest : filtered by
Event --> ConnectFailure
Event --> MessageBatch
MessageBatch --> InboundMessage
Reviews (3): Last reviewed commit: "Merge branch 'main' into claude/whatsapp..." | Re-trigger Greptile
📦 Binary size report
.text per crate
Top movers (cargo-bloat attribution)
Baseline: |
There was a problem hiding this comment.
2 issues found across 22 files
Confidence score: 3/5
- In
wacore/src/types/events.rs, unit-marker events now serialize as{}instead ofnull, which can break observers that rely on the previous JSON shape and cause downstream parsing/regression issues in public event consumers — restore the prior wire format (or add explicit compatibility handling/versioning) before merging. - In
src/client/node_io.rs, mapping a missing<failure message>to""collapses two different states (“absent” vs “empty”), so clients may lose error semantics and make incorrect retry/reporting decisions — preserve presence information in the model (e.g., optional message) or add a compatibility-safe encoding before merging.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Two follow-ups on the freeze PR surfaced by CI and review:
- examples/benchmark.rs matched the old inline `Event::PairingQrCode
{ code, .. }`; under `--all-targets` (both the default Build & Test and
the all-features job) this failed to compile. Updated to the newtype
pattern. The earlier local check missed it because a plain
`cargo build -p whatsapp-rust` does not compile examples and the
workspace `--all-targets` check aborted on the voip-cli `alsa-sys`
system dep first.
- ConnectFailure.message changed String -> Option<String>. It was
populated with `unwrap_or("")`, an empty-string sentinel for an absent
server `message` attribute — exactly the sentinel the freeze convention
forbids, and this is the point it would have been locked in. Now built
via maybe_message, so absent stays None. No consumer read the field, so
the change is contained to the one construction site.
Verified: cargo build -p whatsapp-rust --all-targets (default) and
clippy -p whatsapp-rust --all-features --all-targets -- -D warnings both
clean; wacore 1074 + whatsapp-rust 954 tests pass; fmt clean.
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
What
Final tranche of the pre-1.0 event-payload API freeze, after
ServerAck(#1002) and the notification/sync payloads (#1003). It seals every remaining event payload so fields can be added later without breaking consumers, and clears the related tech debt in the same pass. After this, the wholeEventsurface is frozen.Sealing (
#[non_exhaustive]+#[derive(bon::Builder)])Receipt,InboundMessage,MessageBatch,UndecryptableMessage,LoggedOut,Disconnected,PairSuccess,PairError,PairPasskeyRequest/Confirmation/Error,DeviceListUpdate+ nestedDeviceNotificationInfo,IdentityChange,BusinessStatusUpdate,DisappearingModeChanged,MexNotification,NewsletterLiveUpdate(+Message, +Reaction),TemporaryBan,ConnectFailure,StreamError,OfflineSyncPreview,OfflineSyncCompleted.Eventvariants converted to sealed newtypes:PairingQrCode,PairingCode,PairingCodeRefresh.Event::kind()and every construct/match site updated.Connected,ClientOutdated,QrScannedWithoutMultidevice,StreamReplaced— built viaX::builder().build().Tech debt cleared along the way
EventInterestbitmask migratedu64tou128, doubling theEventKindceiling from 64 to 128 (it was at 58/64, so this was the next real wall).CAPACITYand the build-time tripwire updated.Migration
All ~62 cross-crate construction/match sites in
whatsapp-rust, plus the in-cratewacoretest sites and onee2e-testsreader, moved to the builder / newtype form. Behavior-preserving: same values, same control flow;Optionfields routed throughmaybe_*setters. Compiler-enforced end to end —E0639on any missed struct literal, andbon's typestatebuild()rejects any missed required field at compile time.Deliberately not added: a
trybuildguardThe repo builds on nightly, where
trybuild.stderrsnapshots drift on every toolchain bump. The seal is already enforced structurally byE0639on every literal, so a snapshot test would be net-negative maintenance debt rather than a real safety gain.Verification
cargo build -p whatsapp-rust/-p wacoreclean.cargo clippy -p whatsapp-rust -p wacore --testsclean.cargo check -p e2e-tests -p bench-integration --all-targetsclean.cargo fmt --all --checkclean.cargo test -p wacore --lib→ 1074 passed;cargo test -p whatsapp-rust --lib→ 954 passed, 0 failed.voip-cliwas not checked here — it fails to build in this environment on thealsa-syssystem dependency (missing ALSA headers), unrelated to this change.Follow-ups
None for the freeze itself — the
Eventsurface is fully sealed.AGENTS.mdand theEventdoc now state the policy as complete rather than rolling out.