feat(events)!: move to bridge 0.10.0 and deliver the events it added - #55
Conversation
Version bump plus the two integration points its breaking change moves.
Incomplete on purpose: the boundary coercions that keep the send path
Baileys-compatible are not in yet, so the fuzz suite is still red on
`wire:upstream-readable` and `pure:encodeNewsletterMessage`.
The reader's 64-bit hooks are renamed `*Number()` to `*Value()` and
widened from `number` to `number | Long`, so a value too wide to be
exact as a double survives instead of failing the whole message. The
compatibility facade keeps materialising every 64-bit word as a long.js
Long whatever the magnitude — upstream's types declare Long there, and a
consumer calling `.toNumber()` must not have that work only below 2^53.
The `as unknown as number` casts the old signature forced are gone.
`inviteExpirationNumber` called `.toNumber()` on its argument. A 64-bit
field now crosses as a plain `{ low, high, unsigned }` once it is wide,
and that shape carries no methods, so it goes through `toNumber`, which
reads both forms and reconstructs the high word rather than dropping it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
0.8.0 refuses an empty string in a 64-bit field and an unpaired surrogate in a text field, where 0.7.2 wrote 0 and U+FFFD. Upstream Baileys still encodes both, so passing the strictness through would break the send path for callers and lose Baileys compatibility. This absorbs it at the boundary instead. Repair on failure, not check on write. The ordinary encode runs exactly the code it ran before: a message the codec accepts never reaches the repair, and one that does not was already going to throw. Checking every string for an unpaired surrogate on the way in would cost a scan per text field on the send path, and subclassing the writer is not available — `BinaryWriter` is not exported from the package root. The retry hangs off `finish` because the bridge's writer is lazy: encode queues the fields and finish is what writes them, so a refused value surfaces there rather than at the encode call. Re-encoding from the repaired message is safe for the same reason repairing inside an overridden `string()` would not be — that would have to resume after a tag and a length were already written. Verified: a lone surrogate now encodes to efbfbd, and fileLength '' encodes to 0, both matching what 0.7.2 sent. `pure:encodeNewsletterMessage` is green. Incomplete: `relayMessage` calls the bridge's `encodeProto` directly rather than going through this facade, so the send path is untouched and `wire:upstream-readable` is still red. That seam is next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
`relayMessage` and its siblings call the neutral `encodeProto` directly rather than going through the compatibility facade, so the repair added for `proto.X.encode` never reached them and the send path stayed red. `encodeProtoCompat` wraps that call with the same coercion, and the five call sites in Socket/messages.ts and Socket/index.ts now use it. The repair itself is lifted out of the facade class so both entry points share one implementation, addressed by type name for the send path and by schema index for the facade. The path-to-index map is built on first use: only the repair needs it, and the repair only runs after an encode has already failed. `wire:upstream-readable` goes from 134 divergences to none, and `pure:encodeNewsletterMessage` from 11 to none. `proto-empty-string-for-numeric-field` is removed. Verified by deleting it and re-running: nothing resurfaced, because the bridge now refuses the empty string outright and this side coerces before it gets there, so the divergence it described no longer occurs on any target. `newsletter-encode-lone-surrogate` stays — deleting it did bring back 11 divergences, since upstream writes the surrogate raw as WTF-8 (edbfbf) where this side writes U+FFFD (efbfbd). Its comment now says the substitution is deliberate rather than incidental. Fourteen tests cover both entry points, with hand-written expected bytes rather than bytes derived from the codec. Each was verified to fail with the repair disabled — separately for the send path and for the facade, since they have different entry points and a single switch would have left one of them vacuous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds protobuf repair and compatibility encoding, expands bridge event normalization and public event types, improves wide integer handling, and centralizes schema-aware fuzz validation with new divergence cases. ChangesProtocol and bridge behavior
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
`npm run lint` runs the full `tsc`, which the bump broke: the bridge's
neutral `Int64` — `number | { low, high, unsigned }`, a shape with none
of Long's methods — leaked through every type derived from the proto
namespace, and `MinimalMessage` stopped being assignable to upstream's.
That is the codec's type, not this library's. The compatibility facade
supplies a reader that materialises every 64-bit word as a long.js Long
whatever its magnitude, and the published declaration has always said so,
so a consumer calling `.toNumber()` is right to expect one. `WAMessage`
and `AuthenticationCreds` restate the two fields that carry it into a
comparison against upstream.
Typing the whole namespace as the shipped facade was tried first and
abandoned: the facade is a namespace declaration, so casting the value
loses `proto.IMessage` as a type namespace and costs 131 errors.
Also pins ContextInfo.quotedMessage.videoMessage.mediaKeyDomain, measured
after the bump. It is one of the eleven fields the bridge never writes,
at the path a generative draw puts it — the shape the decode-parity entry
is built to fail on until someone has looked at it. proto:decode-parity
is green again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
The robustness classifier splits a mutated payload two ways: well-formed protobuf read differently is a codec bug, bytes that do not frame are undefined behaviour. It decided that with `canonicalWire(bytes)` and no schema, so it framed only the top level — while the real decoders descend into submessages using the schema. `wire.ts` states the rule it broke: every caller that has a schema passes it, and this one has `path`. Passing the schema alone changed nothing, because `scanFrom` fell back to the raw hex whenever a nested scan failed. That fallback is right without a schema — the payload could be a string or a `bytes` field that merely looks like protobuf — and wrong with one: a field the schema declares a message, whose contents do not frame, makes the record malformed. Both halves were needed. `schemaAt` and the field-number recovery it depends on move out of the codec differential into `harness/schema-context.ts`, which is what lets a second fuzzer have a schema at all. One of the four findings reclassifies to proto:mutation-interpretation, where the entry beside it already calls that undefined. I predicted three would move; only one did, because my probe treated every length-delimited field as a submessage where the schema descends only into declared ones — in the other two the corruption sits in a `bytes` or `string` field, which is genuinely opaque. The three that remain frame at the top level and through every declared submessage. Two decoders reading well-formed protobuf differently is a parser differential, and no allowlist entry is written for it: a predicate loose enough to cover it would have to excuse "same key, different value" on mutated input, which is the property this target exists to protect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
|
The only failing target is They are a parser differential in the bridge codec, not something this repository can fix:
All three frame as protobuf at the top level and through every submessage the schema declares — checked with the schema, after fixing the classifier that had been framing only the top level. Well-formed protobuf has one interpretation, so two decoders producing different objects from it is a genuine defect, and in the first case they assign different field boundaries: keys appear on one side and not the other. None of this reproduces under bridge 0.7.2, so 0.8.0 introduced it. A prompt has gone to that repository with the reproducing bytes. No allowlist entry was written, deliberately. A predicate loose enough to cover these would have to excuse "same key, different value" on mutated input — exactly the property this target exists to detect. Making CI green that way would disable the check that found the problem, which is worse than a red target with a written-down cause. The PR stays a draft until the bridge ships a fix. If the cause is the new 64-bit decode path consuming a different number of bytes at some edge — the hypothesis in the prompt — all three should clear together on the next bump. Generated by Claude Code |
0.8.1 carries the fix the parser-differential report asked for: a singular message field read twice now merges, as the wire format defines — "for embedded message fields, the parser merges multiple instances of the same field, as if with the Message::MergeFrom method" — where the generated code assigned and dropped whatever the earlier instance carried. The count went 3 to 8 rather than to zero, and five of the eight are the fix working. On a payload carrying the same message field twice the bridge now returns the merged object and protobufjs returns only the last one, so this is the one entry in the registry where the *reference* implementation is the non-conforming side. Intended rather than open: aligning would mean deliberately dropping fields the wire format says are there. The predicate is directional and total — upstream may not carry a key the bridge lacks, no shared value may differ, and something has to have been kept — so it cannot excuse a misread or a loss, only a retention. Six counter-cases pin it, including a loss in one branch beside a retention in another. Three remain, and one of the original three cleared into the merge class: lying-length on Message.ImageMessage, truncate → nesting-bomb on HistorySync, and a new concatenate → concatenate on Message.PollCreationMessage. Still parser differentials, still not excused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
jlucaso1
changed the title
|
Updating the blocker above — it described the 0.8.0 state and two of its three cases have changed.
0.8.1 did land the fix this PR reported (#44 there). It went 3 → 8 rather than to zero, and five of the eight are the fix working: a singular message field read twice now merges as the wire format defines, so on a concatenated payload the bridge returns the merged object where protobufjs returns only the last copy. Those five are documented as Three remain, and they are not the same three:
None of the three is a retention: two carry a value that differs where both sides hold the key, one has a field only upstream produced. So the merge predicate does not reach them and should not — they are still parser differentials on payloads that frame at the top level and through every declared submessage. One concrete lead for the next report: Still no allowlist entry for these three, for the reason given above: a predicate loose enough would have to excuse "same key, different value" on mutated input. Generated by Claude Code |
…erns Protobuf merges concatenated messages, and for repeated fields merging means concatenating rather than replacing. The predicate only handled the singular half: it bailed whenever two arrays differed in length, so a Message.PollCreationMessage carrying three copies — where contextInfo.mentionedJid holds both elements here and only the last one upstream — was reported as an unexplained parser differential when it is the same rule, in the same direction, with the same conformant side. Kept strictly directional. The bridge's array has to be upstream's with elements appended: every element upstream produced must match at its own index, and upstream being the longer side is the bridge losing elements, which still fails. Three counter-cases pin it. proto:mutation-agreement goes from three to two, and the two that remain are the ones 0.8.0 introduced: lying-length on Message.ImageMessage and truncate → nesting-bomb on HistorySync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
|
Correction to the comment above: two, not three. Verified on
What remains are the two that 0.8.0 introduced:
A second report has gone to the bridge with the reproducing bytes for both, and with two separate hypotheses for the Generated by Claude Code |
The bump the draft was aiming at, two releases on. 0.9.0 and 0.10.0 landed between: generated declarations that resolve, an app-state mutation moved onto the proto serializer, and eleven core events the bridge had been dropping. Seven of those reach JavaScript for the first time, and the compiler asked for each: `AdapterMap` is a mapped type over the bridge's union, so a new event is a build error until someone decides what it means. `app_state_sync_failed` is the one with a reason to be first. The engine stopped withholding a session whose critical app-state sync came back degraded — it announces the connection and reports what is missing — and this library delivered only the first half. A consumer was told a session with no push name was healthy, with nothing to read that said otherwise. It crosses as `app-state-sync.failed`, which upstream does not have because upstream never withholds a session on app state and so has nothing to report. Two more map onto channels upstream already has: `message_label_association_update` is the per-message half of `labels.association`. The chat half was already here and its comment said the message one was "a separate path"; this is that path. `pairing_qr_codes_exhausted` becomes a terminal close with `timedOut`, which is what upstream's own QR timer does when it gives up, so the canonical reconnect handler needs to learn nothing new. The remaining five are acknowledged with their reason recorded rather than translated: upstream has no channel for a contact deletion, the account-wide link-preview setting, quick replies, a call placed on the phone, or a pairing-code rejection that arrives after the code was displayed. Three checks had to be re-argued, and none of them was adapted for convenience: The renamed `faviconMMSMetadata` (field 33, now `faviconMmsMetadata`) is a schema rename WhatsApp made and baileys 7.0.0-rc13 has not picked up. The wire is identical and the divergence closes when upstream regenerates, so it is recorded as open rather than reconciled: accepting the old spelling on encode means walking the message tree on every send, for a link-preview favicon. `Message.PaymentExtendedMetadata.messageParamsJson` stopped being a wire gap — the same schema release added it — so it leaves the audit baseline. The two mutation-agreement divergences are both under deliberately corrupted bytes. One was already argued for `nesting-bomb`, and only escaped its entry because the chain reads `truncate → nesting-bomb`; that predicate now reads the last mutator instead of demanding an exact match, which is what its own comment argues for. The other, `lying-length`, is recorded as open with what is undecided about it.
jlucaso1
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/__fuzz__/harness/divergence.ts`:
- Around line 930-977: Restrict src/__fuzz__/harness/divergence.ts lines 930-977
so the favicon divergence matches only the exact Message.ExtendedTextMessage
path with the specific faviconMMSMetadata versus faviconMmsMetadata key
difference, not arbitrary favicon text or URLs. In
src/__fuzz__/harness/divergence.ts lines 1432-1444, require
lastMutator(divergence.input) to equal concatenate before applying retention
matching. In src/__fuzz__/harness/__tests__/harness.test.ts lines 617-642, add
rejection cases covering non-concatenate retention divergences and unrelated
text or URL values containing favicon.
In `@src/Bridge/types.ts`:
- Around line 447-459: Move the existing batched app-state sync failure
documentation from CanonicalQrCodesExhausted to CanonicalAppStateSyncFailed,
then document CanonicalQrCodesExhausted specifically as the QR-code exhaustion
event.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aaabd1f3-1f74-4bd1-b6da-840f9d3a7e43
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
package.jsonscripts/compatibility/proto-runtime-audit.tssrc/Bridge/primitives.tssrc/Bridge/schema.tssrc/Bridge/types.tssrc/Compatibility/encode-proto.tssrc/Compatibility/proto-runtime.tssrc/Socket/events.tssrc/Socket/groups.tssrc/Socket/index.tssrc/Socket/messages.tssrc/Types/Auth.tssrc/Types/Events.tssrc/Types/Message.tssrc/__fuzz__/bridge-events.fuzz.test.tssrc/__fuzz__/harness/__tests__/harness.test.tssrc/__fuzz__/harness/divergence.tssrc/__fuzz__/harness/schema-context.tssrc/__fuzz__/harness/wire.tssrc/__fuzz__/proto-codec.fuzz.test.tssrc/__fuzz__/proto-robustness.fuzz.test.tssrc/__tests__/encode-proto-compat.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08d6cd5c73
ℹ️ 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 reviewers caught the same thing independently: the entry matched on the retention *shape* alone, so any mutator that made the bridge invent a field or an array element while the rest of upstream's output was unchanged got filed as the intended concatenation difference. Merge semantics is the justification and it only applies to a payload that carries the message twice. Scoped to the chain *containing* `concatenate`, not ending with it. Both reviewers proposed the final step and that is too strict — concatenation is what puts the message in twice, and a byte corrupted afterwards does not undo it. Measured rather than argued: scoping to the last step left `concatenate → flip-bit` and `concatenate → replace-byte` unexcused, and both are pure retentions with nothing else differing. Four counter-cases pin it: a retention from `flip-bit` alone and from `lying-length` alone are rejected, and the three chain positions — concatenation first, last, and twice — are accepted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RvtvVVWQs8AeBqSzS1JpCg
Three predicates matched more than their reason argued for, and an allowlist that is wider than its argument is how a real regression gets excused. The favicon entry matched anything mentioning the word, anywhere in the finding. It now judges each sweep on its own terms: an omission names field 33, the name sweeps name the two spellings, and a decode has to agree once both spellings come off — and to have disagreed before. Anything else through the same field is still a finding. `lying-length` matched on the mutator label alone, so the next real misdecode reached through that mutation would have been recorded as known. It now also requires the path and the shape the reason describes. The concatenation entry, which predates this branch, never checked that anything was concatenated: any mutator producing the same retention shape was read as intended merge semantics. It requires the mutator now — anywhere in the chain, since concatenating is what creates the merge and a byte flipped afterwards does not undo it, which is a different question from the wire-type entry that reads only the last step. Also moves the app-state doc onto the interface it describes; it had ended up above `CanonicalQrCodesExhausted`, which now documents itself.
|
Review comments addressed, except one, and the remaining test failure is tracked in #60. Narrowed the three predicates. The favicon entry judges each sweep on its own terms now — an omission names field 33, the name sweeps name the two spellings, and a decode has to agree once both spellings come off and to have disagreed before. Moved the app-state doc onto the interface it describes. Not addressed: the map-entry skip in The |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1ba9a3333
ℹ️ 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".
|
Measurements for the The three findings decompose like this (structural diff of
Every one is documented; none carries an undocumented difference. On "stripping the second cause would hide a third": it does not, provided stripping means remove these named things, then require exact equality — which is what Measured rather than argued. I chained the other documented spellings into the renumbering entry's comparison — Then reverted it; the branch is untouched. Two things that experiment taught me, one of which contradicts what I expected:
So the shape of a fix is a shared "strip every documented spelling and omission" normaliser that the matching entry applies before its own equality check — not a per-entry patch. That is more than a line, which is probably why it belongs in #60 rather than in this PR, and I agree with holding it here. Happy to take it if you want it; otherwise the numbers above are yours. Generated by Claude Code |
…y models renames The round-trip target reported nine findings after the bump, and every cause in them was already documented — what was missing was that they arrived together, which no single allowlist entry could explain. None of it needed a new entry. The harness already models this exactly: `faviconMMSMetadata` → `faviconMmsMetadata` is the fourth rename, so it joins RENAMED_PROTO_FIELDS beside the three that were there. `proto-field-renamed-and-dropped` then covers every view of it, including the ones that arrive with a second known gap, because that entry already composes the two. It also joins the not-encoded and omitted-field lists, which is what the encode sweeps measure: handed upstream's spelling, the bridge writes nothing. What remained after undoing the renames was the same two gaps — a `mediaKeyDomain` the bridge never writes, and the `pollResultSnapshotMessageV3` renumbering — reached through paths the older schema never put a generative draw at. DECODE_OMITTED_PATHS enumerates by path for exactly this reason, so the four new paths are listed there. `Message.PaymentExtendedMetadata.messageParamsJson` leaves the not-encoded list: schema 2.3000.1044659339 added the field and the bridge writes it now. An earlier attempt at this added a favicon entry of its own with a predicate that matched the word anywhere in a finding, then grew a second field into it to cover the pairs. Both are gone: a wider entry would have excused the next real defect that happened to travel with a favicon.
There was a problem hiding this comment.
💡 Codex Review
baileyrs/src/__fuzz__/harness/divergence.ts
Line 473 in 20cea47
Remove the restored payment field from the omission allowlist
This update removes Message.PaymentExtendedMetadata.messageParamsJson from NOT_ENCODED_FIELDS and from the runtime audit because bridge 0.10.0 now writes it, but KNOWN_OMITTED_FIELDS still contains its Message.PaymentExtendedMetadata#3 identity. If a later codec change drops this restored field again in a generated encode case, byteTarget classifies it as proto:field-omission and this stale member silently allowlists the regression; remove it here along with the other baseline removals.
ℹ️ 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 were acknowledged as `noop` under a shared comment claiming upstream has
no channel for them. It has one for each.
`settings.update` already declares the `disableLinkPreviews` arm, and upstream
reaches it through app state (`Utils/chat-utils.ts` branches on
`privacySettingDisableLinkPreviewsAction` and emits the action as the value).
The bridge delivers the same action over a different pipe, so it lands on the
same channel with the same payload — a consumer that read the setting from
app state reads it here unchanged. Dropping it left the local setting stale
with nothing to say so.
`pairing_code_error` leaves the socket exactly where `pair_error` does: alive,
with the engine ready to take another request, and with a code on screen that
is now spent. It adapts to the same canonical event, so the same dispatcher
logs it and emits `connecting` with an explicit `qr: undefined` — a pairing
code surfaces as `qr`, so that is what stops it being offered. The server's
rejection code and backoff ride along for the log; neither changes what a
consumer does, but a throttle the server named itself is the difference
between a code that will come back and one that will not.
`extractAction` now tolerates a missing `data` slot. The two entries read the
action before any other guard, and `bridge:adapt-total` caught the adapter
throwing rather than dropping on `{ type: 'disable_link_previews_update' }`.
Measured, 500k iterations, median of five rounds, GC scavenge counts (wall
clock on this machine varies ±32% between runs, scavenges do not):
extractAction data.action 0 scavenges 5.2 ns/op
extractAction data?.action 0 scavenges 8.5 ns/op
link previews noop (before) 5 scavenges 19.5 ns/op
link previews settingUpdate 10 scavenges 32.2 ns/op
pairing error noop (before) 6 scavenges 22.2 ns/op
pairing error pairError 4 scavenges 23.8 ns/op
The optional chain allocates nothing and costs ~3 ns on a call that happens
once per sync-action event; order-swapped to confirm it is not a warm-up
artifact. The link-preview value is one more object than a `noop` was, on an
event that arrives when another device changes an account setting.
…0.10.0 closed
Three findings, all measured before and after.
**The wire scan asked only one encoder.** `factsFor` recovered each field
number by encoding through upstream, but the payloads these scans read are
produced by `encodeProto`. Swept both across the whole schema:
DIFF Message.pollResultSnapshotMessageV3 upstream 114 bridge 115
{ checked: 2421, diffs: 1, bridgeUnwritable: 17 }
One field, the documented one — and it meant the bridge's spelling of that
submessage framed as opaque bytes, so a corruption inside it read as
well-formed and routed to `proto:mutation-agreement` rather than
`proto:mutation-interpretation`. Both numbers are recorded now. Nothing else
in `Message` claims either on either side, so this cannot make a different
field misframe. Not hardcoded for the one known case: a hand-kept exception is
what the field-number sweep exists to replace.
Building the fact map for all 498 message types goes from 42 ms to 84 ms. It
is lazy and cached per path, so a run pays only for the types it looks at.
**Two allowlist members no longer reproduce.** Bridge 0.10.0 writes
`PaymentExtendedMetadata.messageParamsJson` — both encoders emit `1a 00` for
an explicit empty string — so `Message.PaymentExtendedMetadata#3` left
`KNOWN_OMITTED_FIELDS` and the field left `PRESENCE_DROPPED_FIELDS`, which
takes the presence count from ten to nine. It had already left
`NOT_ENCODED_FIELDS`; these two were missed. A member that no longer
reproduces excuses nothing today and silently excuses a regression the day the
field breaks again. Swept the other members of all three lists against 0.10.0
to check none of them had gone the same way; they had not.
`faviconMMSMetadata` was listed twice in `NOT_ENCODED_FIELDS`, with two
near-identical comments. Deduplicated. `KNOWN_OMITTED_FIELDS` stays at twelve:
the favicon rename joined as `#33` while the payment field left.
**The encode wrapper minted its own writer.** It returned a fresh `{ finish }`
rather than the writer the codec produced, which would drop anything the
bridge later adds to that surface. It shadows `finish` on the codec's own
writer now. (The reported break — `encode(m).ldelim()` — does not reproduce:
the bridge already returns a bare `{ finish }`, so that surface was never
there. The `$protobuf.Writer` declaration in `WAProto/index.d.ts` has not
matched the runtime since the bridge became the codec.)
|
On the review comment about Measured rather than reasoned about. Bridge 0.10.0 writes the field, identically to upstream: So two members were stale, not one:
Both removed. Swept the rest of all three lists against 0.10.0 while there, to check none of the others had gone the same way. They had not — every remaining member still reproduces: Two other things that fell out of reading these lists closely:
And the doc on All of this is in Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e67bc3d97
ℹ️ 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".
The entry excused any `lying-length` divergence on `Message.ImageMessage` where both decoders returned a non-empty object. That is true of any misread reached through the same mutator, so a real regression would have been filed as known. Deleted the entry and ran the sweep to see what it was actually covering: one divergence in 10001 runs. Reading it showed the mechanism is not quite what the entry's prose said, and is directional in a way that can be checked. protobufjs decodes greedily — a lead byte swallows the bytes after it into one code point, ASCII ones included. The bridge rejects the sequence and substitutes per byte, so every ASCII byte it met survives. On the real case upstream produced 88 ASCII characters against the bridge's 98, and upstream's are a subsequence of the bridge's. So the difference now has to *be* the salvage: retention in the structural direction the merge walker already checks, plus any differing string having upstream's ASCII as an ordered subsequence of ours. Anything else — a key only upstream produced, a changed number, a string that moved bytes both sides could decode — still fails. `keptFieldsUpstreamDropped` grows one optional seam for this: a caller may name the specific leaf difference its entry documents. The default explains nothing, so the concatenation entry keeps the strict reading unchanged. An ASCII-skeleton equality check was tried first and rejected the real divergence, which is what surfaced the greedy-decode asymmetry rather than leaving it assumed. Eleven cases pin it: the real shape and each documented half alone are excused; an upstream-only key, a changed scalar, a changed ASCII string, a dropped character, ASCII upstream has that we lack, reordered ASCII, another mutator, another type, and a no-op are all rejected.
There was a problem hiding this comment.
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/__fuzz__/harness/schema-context.ts`:
- Around line 151-156: Update the field-number processing loop around
numbersFor, repeated, and messages to detect conflicting assignments instead of
silently overwriting them. Before adding a number to repeated or setting its
nested message path, validate any existing mapping or repeated classification
and fail loudly when it conflicts; preserve idempotent assignments for the same
field metadata and keep the existing behavior for unclaimed numbers.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aa4552e2-e5d8-4348-b3bf-68d75a189096
📒 Files selected for processing (9)
src/Bridge/schema.tssrc/Bridge/types.tssrc/Compatibility/proto-runtime.tssrc/Socket/events.tssrc/__fuzz__/bridge-events.fuzz.test.tssrc/__fuzz__/harness/__tests__/harness.test.tssrc/__fuzz__/harness/divergence.tssrc/__fuzz__/harness/schema-context.tssrc/__tests__/regressions.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7e6f4d27d
ℹ️ 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".
…nst a guess Reading both encoders' numbers is only safe while no number is claimed by two different fields. Measured across the whole schema: 2422 claims, no collisions — so the ambiguity is theoretical. But the thing that would create one is a schema regeneration, which is exactly the moment nobody re-reads the comment asserting it holds. Two halves. The quiet one is in `factsFor`: a number two fields claim is recorded by neither, so the scan treats it as opaque bytes — the answer it gave before the file knew about nested messages at all. Conservative beats confidently framing a payload against the wrong submessage, which would report a difference at a location that does not exist. The loud one is a test that sweeps every message type and fails on the first collision, naming the type and the number. A degradation that nobody notices is the same silence, one level down.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6032d89fd0
ℹ️ 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".
…any depth Two findings, both real, both measured. **The repeated-field direction was backwards.** The merge entry compared upstream's array against ours index by index, on the reasoning that the bridge appends the later copy — a prefix. But the repeated field that actually diverges sits inside a *singular* submessage the payload carries twice: merging concatenates every copy's elements, while upstream replaces the submessage and keeps only the last copy's. Upstream's array is therefore our tail. Measured on the deep sweep, 18 array pairs stand in that relation against 2 the prefix reading matched: SyncActionValue.primaryFeature.flags local ["xxx…","","","👍"] upstream ["👍"] …deviceListMetadata.senderKeyIndexes local ["2147483647","127"] upstream ["127"] Reading it as a suffix takes proto:mutation-agreement from 57 unexcused to 43, resolving all fourteen concatenate findings and unexcusing nothing. The test used duplicated `'a'` values, which pass under either reading and are what hid this; the cases now use distinct ones and pin the direction from both ends. **The repair stopped at a fixed depth.** `MAX_REPAIR_DEPTH` was 24, so a refused value below that was left alone and the retry rethrew — 12 nested `ephemeralMessage.message` wrappers is exactly 24 levels, and an empty-string int64 under them threw instead of being coerced. Recursive protobuf messages have no depth limit, so the guard is now the thing it was standing in for: an ancestor set, refusing only a message that contains itself. The ancestor path, not everything visited — the same object reached twice in different branches is legitimate and is still repaired in both. Verified: 200 wrappers repair, a cycle returns unchanged without spinning, a cycle carrying a repairable value beside it still repairs. Measured at 8 wrappers, where both implementations traverse the whole tree (median of five, 50k iterations): depth counter shallow 1.74 µs 116 scavenges/50k deep 13.98 µs ancestor set shallow 1.99 µs 123 scavenges/50k deep 15.84 µs ~13% on a walk that only runs after an encode has already thrown.
An extra blank line left where MAX_REPAIR_DEPTH was removed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b369a76cb1
ℹ️ 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".
…lone Two gaps the previous tightening left, both real. The predicate asked only that both sides carry *some* non-ASCII with matching ASCII skeletons. `a雪b` against `aéb` satisfies that while being an ordinary decoder disagreement — nothing establishes either side substituted anything. The bridge's side must now carry U+FFFD, which is the substitution itself; measured on the one case this entry covers, 25 of them. And the seam returned 1, so a string difference satisfied the entry with no retained field at all — an arbitrary Unicode decoding regression on this route would have excused itself. It returns 0 now: an explained leaf is permitted, never sufficient. The retention is what satisfies the entry, and the substitution only rides beside it. Both checked against the real divergence, which still qualifies on both counts (25 substitutions, 2 retained fields), and against the counter-example.
2 participants
Summary
Bridge
0.7.2→0.10.0. The draft was aiming at 0.8.1; 0.9.0 and 0.10.0 landed since, and the blocker it stopped on is gone.Three things arrive together: the reader and strictness contracts from 0.8.x that this branch already absorbed, generated declarations that actually resolve (bridge #52), and eleven core events the bridge had been dropping (bridge #54).
The events
Seven reach JavaScript for the first time, and the compiler asked for every one —
AdapterMapis a mapped type over the bridge's union, so a new event is a build error until someone decides what it means.app_state_sync_failedis the one with a reason to be first. The engine stopped withholding a session whose critical app-state sync came back degraded (whatsapp-rust#1291): it announces the connection and reports what is missing. This library delivered only the first half, so a consumer was told a session with no push name was healthy and had nothing to read that said otherwise. It crosses asapp-state-sync.failed.That event has no upstream counterpart, and cannot: upstream never withholds a session on app state, so it has nothing to report there. It is additive, so no existing handler changes, and the declaration auditor counts an extra entry as
extrarather than a gap.Two map onto channels upstream already has:
message_label_association_updatelabels.association,type: Messagepairing_qr_codes_exhaustedconnection.updateclose,timedOutFive are acknowledged with the reason recorded, not translated. Upstream has no channel for a contact deletion (
contacts.updateonly upserts), the account-wide link-preview setting, quick replies, a call placed on the phone, or a pairing-code rejection that arrives after the code was displayed. Each is registered in the fuzz coverage tables, so a future decision to expose one is a deliberate edit rather than a silent drift.The blocker is gone, and what replaced it
The draft stopped on three
proto:mutation-agreementdivergences. Two remain, and one of those was already argued for.nesting-bombhad an entry whose predicate demanded an exact mutator match; the chain now readstruncate → nesting-bomb, so it reads the last mutator of the chain instead. That is what the entry's own comment argues for — the concern was a mutator running after the bomb, which the tail check still excludes.lying-lengthis recorded as open, with what is undecided about it: whether the bridge should refuse a submessage whose declared length disagrees with its content.Both are under deliberately corrupted bytes that no wire this library sends produces.
Two more checks re-argued
faviconMMSMetadata. WhatsApp renamed field 33 tofaviconMmsMetadatain schema 2.3000.1044659339 and baileys 7.0.0-rc13 has not picked it up. The wire is identical — same field number — and the divergence closes on its own when upstream regenerates its proto. Recorded as open rather than reconciled: accepting the old spelling on encode means walking the message tree on every send, for a link-preview favicon.Message.PaymentExtendedMetadata.messageParamsJsonstopped being a wire gap; the same schema release added the field. It leaves the audit baseline, which is whatresolvedKnownGapsexists to catch.Validation
npm test— 1267 passing, 0 failing.tsc,oxlint,oxfmtclean.proto-runtime-audit --strictclean.E2E left to CI, which is also where the lifecycle contract recording runs: with the engine announcing degraded sessions,
LC-009andLC-011may have changed state, and that suite fails deliberately if a pinned violation starts holding.Summary by CodeRabbit
New Features
Bug Fixes
Chores