Skip to content

fix(deps): update whatsapp-rust-bridge to 0.7.0 - #26

Merged
jlucaso1 merged 7 commits into
mainfrom
claude/bridge-0-7-0-upgrade-kripwv
Aug 8, 2026
Merged

jlucaso1 merged 7 commits into
mainfrom
claude/bridge-0-7-0-upgrade-kripwv

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Aug 8, 2026 •

Copy link
Copy Markdown
Member

Summary

@oxidezap/whatsapp-rust-bridge 0.6.5 to 0.7.0.

Nothing new is exposed. 0.7.0 adds roughly fifty operations (newsletter, labels, quick replies, business catalog, server queries, privacy and tokens); none of them is wired up here. This PR is only the bump plus the four runtime behavior changes that came with it.

Observable for a consumer of this package:

  • A mistyped argument to a bridge-backed socket method now rejects with invalid-argument naming the parameter, where it used to leave the promise pending forever and end the Node process with an uncaught exception.
  • An app-state mutation rejected for the caller's own input (mute, pin, archive, star, contact names) now reports invalid-argument instead of internal.
  • logged_out.reason is now a written-down string rather than a Debug rendering. The values did not change, so no consumer comparison breaks.
  • encryptMediaStream rejects a non-stream instead of accepting it and failing much deeper.

Type surface: I diffed the emitted .d.ts between the two versions with comments stripped. There is no removal, no signature change and no shape change to any exported type. The only deletions are wasm-bindgen glue symbols in InitOutput. npm run lint (which runs tsc) passed on the first try across the roughly 60 import sites, and the WhatsAppEvent union is byte-identical, so Bridge/__tests__/dts-drift.test.ts needed nothing.

Behavior changes absorbed

Public strings no longer come from Debug (bridge #28)

logged_out.reason, newsletter verification/state, and newsletter follower role used to cross as format!("{:?}", ...). Debug is not a stable API, so a variant renamed in the core would have silently changed a string consumers compare, with nothing failing to build on either side.

The important detail, and the one the pre-research guessed the other way: this is a change of source, not of value. The mappings emit exactly what Debug emitted ("LoggedOut", "TempBanned", "Verified", "Active", "Owner", and Unknown(499)-style values for the core's own fallback). Nothing arriving here changed.

What this repository does with it: evt.reason is consumed in one place, src/Socket/events.ts:392, where it is interpolated into the close message. It decides nothing; the status code is always DisconnectReason.loggedOut. connect_failure.reason is a separate field and a number, so isReconnectableConnectFailure is untouched.

Because the string is now a contract rather than an accident, src/__tests__/auto-reconnect-terminal-close.test.ts pins all fifteen spellings against the close message a consumer sees, plus the no-reason case. The events are synthetic, so to be precise about what that buys: it pins our half of the contract, that the dispatcher passes the reason through untouched, that the message is exactly the literal Logged out: followed by the reason, and that the status code stays loggedOut whatever the reason says. A change on the bridge side of the line is caught by the bridge's own tests, which assert those spellings literally. This test therefore passes on 0.6.5 too, since the values are the same on both.

A mistyped parameter rejects instead of escaping the caller (bridge #25)

Twenty-four typed parameters were deserialized by tsify's own FromWasmAbi, inside the async shim, where a throw escapes as an uncaught exception. The result was the worst of both: the promise never settled, and the process died. 0.7.0 takes them as JsValue and deserializes explicitly, rejecting with invalid-argument and the parameter name.

The accepted set of values is unchanged (same serde deserialization, different call site), so no valid call started failing.

Our own call sites are type-correct and I checked each one that touches an affected parameter: readMessages/markPlayed go through receiptMessageKeys, which already projects to the exact {remoteJid, id, participant?} shape and drops keys missing either required field; addLidPnMappings filters before calling; muteChat, sendPresence, sendChatState, the group and block actions, and the media-type parameters are all pass-throughs of typed values. Nothing here needed changing.

src/__tests__/bridge-argument-rejection.test.ts covers the new contract: four probes against a real offline bridge client, in a child process so a rejection can be told apart from a call that ends the runner. All of them fail on 0.6.5 and pass on 0.7.0.

They drive the bridge boundary directly rather than the socket methods layered over it, which matters for reading them honestly: sock.readMessages filters its input through receiptMessageKeys first, so that particular bad value resolves there without ever reaching the bridge. The boundary is still consumer-facing, since MediaGenerationOptions.processMedia hands the raw client over.

A bad argument reports invalid-argument, not internal (bridge #29)

Six rejections that were about the caller's input said internal. The broad one is AppStateError::InvalidRequest, which covers every app-state mutation: mute, pin, archive, star, labels, contact names.

Nothing in src/ branches on a bridge error kind; I grepped for kind, serverCode and code comparisons, including the tests. src/Utils/generics.ts's getCodeFromWSError reads transport errors, not bridge errors. So the impact here is entirely in what an end user sees, and no code changed.

Two narrower cases in the same PR do change what a call does. muteChat now range-checks its timestamp before the as i64 cast, so Infinity rejects with invalid-argument on muteUntil instead of saturating to i64::MAX and passing every check downstream. That one is covered in the rejection test above.

encryptMediaStream rejects a non-stream (bridge #30)

input and output were imported class types, which wasm-bindgen casts unchecked, so a plain object was accepted at the boundary and blew up deep inside the stream machinery as another uncaught exception with the call pending forever. 0.7.0 probes each one by calling getReader()/getWriter() and releasing the lock, and rejects naming the parameter.

This repository never calls encryptMediaStream itself. It reaches consumers through MediaGenerationOptions.processMedia, which is handed waClient typed as a Pick of WasmWhatsAppClient over uploadMedia, encryptMediaStream and uploadEncryptedMediaStream (src/Types/Message.ts:359). The rejection test covers the parameter contract; the happy path with real web streams lives in src/__tests__/e2e/send-receive-message.test-e2e.ts, which I could not run locally but which CI ran green on this branch.

Note for anyone passing streams through processMedia: the probe consumes and releases a reader/writer, so an already-locked stream is now rejected rather than accepted.

Bugs this surfaced

One test failure, and it was not a bug in this repository.

src/__tests__/bridge-free-safety.test.ts asserted that free() with a call in flight dies with dlmalloc's assertion failed: psize <= size + max_overhead. On 0.7.0 it dies with RuntimeError: memory access out of bounds instead.

I checked before touching it. The assertion string is still compiled into both wasm builds, and both crashes are deterministic across repeated runs (5/5 each). The hazard is unchanged: free() with a pending call still corrupts the heap, still ends the process, and awaiting disconnect() first still makes it safe, which the suite's other two cases confirm on both versions. What changed is which memory check trips first, a property of the larger 0.7.0 heap layout, not of the invariant end() depends on.

So the test was pinning a build detail. It now accepts either fault and additionally requires the stack to come from inside wasm, which the old assertion did not check. That second condition is new coverage, not slack: a crash raised in JS would previously not have been distinguished.

No assertion was loosened elsewhere, no timeout raised, nothing skipped. The other 645 tests passed unchanged.

Reviewed and no change needed

  • src/Types/Newsletter.ts unions. The pre-research suspected verification?: 'VERIFIED' | 'UNVERIFIED' and mute_state?: 'ON' | 'OFF' never matched what the bridge sends, and that 0.7.0 might fix a latent bug. Neither is the case. NewsletterMetadata in that file is a mirror of upstream Baileys' type, kept for API compatibility, and is not what sock.newsletterMetadata() returns: that returns the bridge's NewsletterMetadataResult, where verification and state are plain string. Nothing in src/ compares them to those literals. The unions are correct as upstream mirrors and wrong to "fix" here; the real gap is that newsletterMetadata() does not project onto them, which belongs to the newsletter domain, not to a version bump.
  • connect_failure.reason. A number (ConnectFailureReason wire code), untouched by feat(privacy): add call and message privacy controls #28. isReconnectableConnectFailure and mapConnectFailureToDisconnect in src/Socket/events.ts keep working on the same values.
  • Error-code handling. No path in src/ reads a bridge error's kind, so fix(chats): stop silently dropping unsupported chatModify variants #29 cannot change control flow here. The one test that asserts on a bridge error (src/__tests__/group-operations-compatibility.test.ts) checks serverCode, which fix(chats): stop silently dropping unsupported chatModify variants #29 does not touch.
  • getMediaConn doc drift. The 0.6.5 doc comment claimed hosts carried maxContentLengthBytes; 0.7.0 corrected the comment. MediaHost itself is { hostname: string } in both, so nothing here was relying on a field that was never there.
  • Module split (bridge fix(deps): update whatsapp-rust-bridge to 0.7.0 #26). wasm_client.rs was split per domain. Every type still resolves from the same import path: dist/whatsapp_rust_bridge.d.ts is the only declaration file that changed at all, and the comment-stripped diff shows no declaration moved, was removed, or changed shape.
  • src/Socket/index.ts logout path. Builds its own 'Logged out' Boom independently of evt.reason, so the dispatcher's message and this one stay separate as before.

Coverage

npm run compat:audit before: 97.15% (21717/22354)
npm run compat:audit after: 97.15% (21717/22354)

Identical across all four categories (exports 85.31%, functions 96.04%, fields 98.00%, events 100.00%). It should be: the audit measures this package's public surface against upstream Baileys, and this PR adds no public method. A move here would have meant a new surface slipped in.

Follow-ups

Deliberately unused, one domain per PR:

Also worth knowing when those land: assertSessions' force and getUSyncDevices' useCache/ignoreZeroDevices are documented in 0.7.0 as having no effect. The parameters existed before; only the documentation is new.

Release type

The bump is committed as fix(deps), not chore(deps), and the PR title matches. .github/workflows/release.yml records the reason: only feat/fix/perf produce a release, and a chore-typed bridge bump would leave the upgrade sitting on main until an unrelated commit dragged it out.

Validation

Run locally and green:

  • npm run format
  • npm run lint (tsc plus oxlint)
  • npm test: 666 tests across 167 suites, 0 failures. 645 across 165 before this PR, so 21 added and none removed.
  • npm run compat:audit

npm run test:e2e could not run locally, since it needs a mock server this environment does not have. CI does have it, and the E2E workflow passed on this branch, which is what covers the encryptMediaStream happy path with real ReadableStream/WritableStream values. All four workflows (Build, Run Tests, Linting, E2E) are green.

Cross-version checks done by hand, against real bridge builds:

  • The four rejection probes: 5 failures on 0.6.5, 5 passes on 0.7.0.
  • The free-safety crash signature: 3 runs on 0.6.5 all dlmalloc, 5 runs on 0.7.0 all out-of-bounds, and the reworked test passes on both.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

Next review available in: 34 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 @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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 53eb8f20-fd1f-4e0f-b05d-585720001e22

📥 Commits

Reviewing files that changed from the base of the PR and between 3a80f10 and 3d4b4c6.

📒 Files selected for processing (3)
  • src/__tests__/auto-reconnect-terminal-close.test.ts
  • src/__tests__/bridge-argument-rejection.test.ts
  • src/__tests__/bridge-free-safety.test.ts
📝 Walkthrough

Walkthrough

The bridge dependency was upgraded to 0.7.0. Tests now cover invalid bridge arguments, logged-out reason formatting, and multiple wasm memory-fault messages.

Changes

Bridge validation

Layer / File(s) Summary
Bridge upgrade and argument validation
package.json, src/__tests__/bridge-argument-rejection.test.ts
The bridge dependency uses version 0.7.0. Child-process tests verify invalid arguments reject with invalid-argument while the process remains alive.
Logout close-message coverage
src/__tests__/auto-reconnect-terminal-close.test.ts
Tests preserve supported logged_out reasons in close messages and verify the Logged out fallback.
Wasm fault compatibility assertions
src/__tests__/bridge-free-safety.test.ts
Tests accept known allocator, bounds, and unreachable wasm faults while requiring wasm crash markers in stderr.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A rabbit checked the bridge at dawn,
And found each bad input neatly drawn.
Logout words stayed clear and bright,
Wasm faults wore names just right.
“Hop onward—tests now guard the path!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main change: upgrading whatsapp-rust-bridge to version 0.7.0.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/__tests__/bridge-argument-rejection.test.ts`:
- Around line 96-109: Update the probe flow around probe so that when
Promise.race reports a pending outcome, it records that diagnostic and exits the
child immediately before any subsequent bridge probe or c.free() call can run.
Preserve normal continuation and timer cleanup for resolved or rejected probes.
- Around line 117-138: Update runProbes to handle the spawned ChildProcess error
event by clearing the timer and resolving with the error outcome instead of
leaving the promise pending or raising an uncaught exception. Also include the
returned stderr in the first probe assertion’s failure message, using the
appropriate assert API or import rather than the local expect helper.
- Around line 79-93: Update the child process spawn configuration in
bridge-argument-rejection.test.ts to set its cwd explicitly to the repository
root, ensuring bare imports such as `@oxidezap/whatsapp-rust-bridge` and pino
resolve independently of the test runner’s working directory. Preserve the
existing child script and spawn behavior otherwise.

In `@src/__tests__/bridge-free-safety.test.ts`:
- Around line 109-117: Update WASM_MEMORY_FAULTS to use the exact V8 wording
“RuntimeError: unreachable”, or remove that entry unless its producing bridge
build is documented; revise the surrounding comments to describe the actual
allowlist count and identified builds. In the assertion near the test’s failure
check, include outcome.stderr in the assertion message and import the assertion
helper required for message support.
🪄 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: 7258b167-d861-435b-b0df-5ac3ba818ae7

📥 Commits

Reviewing files that changed from the base of the PR and between d76e660 and 3a80f10.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • package.json
  • src/__tests__/auto-reconnect-terminal-close.test.ts
  • src/__tests__/bridge-argument-rejection.test.ts
  • src/__tests__/bridge-free-safety.test.ts

Comment thread src/__tests__/bridge-argument-rejection.test.ts
Comment thread src/__tests__/bridge-argument-rejection.test.ts
Comment thread src/__tests__/bridge-argument-rejection.test.ts
Comment thread src/__tests__/bridge-free-safety.test.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a80f10d20

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

describe('logged out: the reason reaches the consumer verbatim', () => {
for (const reason of LOGGED_OUT_REASONS) {
it(`${reason} → 'Logged out: ${reason}'`, () => {
const result = dispatch({ type: 'logged_out', data: { reason } })

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exercise the bridge when pinning logout reason strings

When a later bridge release changes its logged_out.reason mapping, this test will still pass because it injects each expected string directly into the dispatcher rather than obtaining an event from the bridge. It only verifies that the dispatcher preserves arbitrary input verbatim, so it does not provide the claimed regression protection for the dependency contract; derive the values through a bridge serialization/event path or validate the bridge mapping itself.

Useful? React with 👍 / 👎.

Comment thread package.json
"dependencies": {
"@hapi/boom": "^9.1.4",
"@oxidezap/whatsapp-rust-bridge": "0.6.5",
"@oxidezap/whatsapp-rust-bridge": "0.7.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use a release-producing commit type for this dependency bump

When this lands with the current chore(deps): update whatsapp-rust-bridge to 0.7.0 subject, consumers will not receive the upgraded dependency: the release policy documented in .github/workflows/release.yml lines 3-11 says only feat, fix, and perf commits produce a release and explicitly requires bridge bumps to use fix(deps). Release Please will therefore create no version/tag for this commit, causing the workflow's verify and publish jobs to remain skipped until an unrelated release-producing change lands.

Useful? React with 👍 / 👎.

const PROBES: ProbeSpec[] = [
{
label: 'readMessages with a string where the key list goes',
call: `c.readMessages('not-an-array')`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Run consumer-contract probes through the socket API

For consumers calling the advertised socket surface, this probe does not establish the asserted rejection behavior because it invokes the raw bridge client. In particular, Socket/messages.ts passes public readMessages input through receiptMessageKeys; for 'not-an-array', that helper iterates the characters, drops all of them for lacking remoteJid and id, and the public method resolves without calling the bridge. The adjacent sendPresence probe likewise bypasses the exposed sendPresenceUpdate wrapper, so these raw-client successes cannot support the stated package-level contract; exercise makeWASocket or add validation at its boundary.

Useful? React with 👍 / 👎.


await c.disconnect()
c.free()
process.exit(0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Flush probe reports before forcing the child to exit

On POSIX CI, the child's stdout is a pipe and its writes may still be pending when this unconditional process.exit(0) runs; process.exit() terminates without waiting for asynchronous stdout writes to finish. Under pipe backpressure or a loaded runner, one or more PROBE records can therefore be truncated even though every call rejected correctly, and the parent then fails the label assertion as if the bridge regressed. Let the child exit naturally after cleanup or wait for the stdout write callbacks/drain before exiting.

Useful? React with 👍 / 👎.

claude added 6 commits August 8, 2026 20:23
The TypeScript surface is additive between 0.6.5 and 0.7.0: no exported
type, interface or method signature was removed or changed, and the
`WhatsAppEvent` union is byte-identical, so the adapter drift check keeps
passing untouched. What did change is runtime behavior, absorbed in the
commits that follow.

None of the roughly fifty operations 0.7.0 adds is wired up here. They
land one domain at a time so a behavior change and a new surface never
arrive in the same review.

Typed `fix` rather than `chore` on purpose: only feat/fix/perf produce a
release, and release.yml records that a chore-typed bridge bump leaves the
upgrade sitting on main until an unrelated commit drags it out.
`free()` while a bridge call is in flight still corrupts the wasm heap
and still ends the process, and awaiting `disconnect()` first still makes
it safe. Only the diagnostic changed: 0.6.5 tripped dlmalloc's own
consistency assertion, 0.7.0's larger heap faults on an out-of-bounds
access before dlmalloc gets to look. Both strings are compiled into both
builds, so which one fires is a property of the layout, not of the
hazard.

Pinning one allocator message meant the suite failed for a reason that
says nothing about the invariant `end()` depends on. It now accepts
either fault and additionally requires the stack to come from inside
wasm, so a crash raised in JS can no longer satisfy it by accident.
Bridge 0.7.0 stopped deriving `logged_out.reason` from `Debug` and writes
the spellings out instead, keeping every value it already emitted. The
values are unchanged, but their status is not: a variant renamed in the
core used to change this string silently, and now cannot.

The reason is interpolated into the close message, which makes it the
text a consumer logs and greps. It steers nothing, the status code is
always `loggedOut`, so nothing here failed. That is exactly why it needed
a test: the last representation change would have gone unnoticed, and so
would the next one.
…rocess

A typed parameter of the wrong shape used to be deserialized inside the
async shim, where the throw escaped as an uncaught exception: the promise
never settled and Node ended the process. 0.7.0 rejects with
`invalid-argument` naming the parameter, and does the same for a stream
parameter that cannot do what a stream does.

Every method covered is reachable from this package's public surface.
`readMessages` and `muteChat` are socket methods, `encryptMediaStream` is
handed to consumers through `MediaGenerationOptions.processMedia`. Our
own code passes none of them a bad argument, so what this pins is the
contract a consumer gets.

The probes run in a child process on purpose: the old failure is fatal,
and only a separate process can tell a rejection from a call that took
the test runner down with it. All five fail on 0.6.5 and pass on 0.7.0.
… after it

Three problems in the new coverage, all about the diagnostic rather than
the assertion.

A probe that timed out left its call in flight and the loop carried on,
so the run reached `free()` with a pending call: the exact heap
corruption `bridge-free-safety` documents. The child then died and the
suite reported the exit code, burying the stalled probe that caused it.
It now exits at the first `pending`.

`spawn` failures emitted `error` with nobody listening, which throws in
the runner and leaves the promise pending until the suite timeout.

And a failed run printed `expected false to be true` with the child's
stderr collected but never shown. Both assertions now carry it, which is
the only account of why a child died before reporting.

Also drops the unmatched `unreachable` entry from the fault allowlist:
the other two name the build that emits them, that one named none.
The child wrote its reports with console.log and then called
process.exit(), which does not wait for a pipe's pending writes. A loaded
runner could truncate the reports and the parent would read the short run
as a regression. The writes are awaited now.

Two doc comments also claimed more than the tests deliver. The rejection
probes drive the bridge boundary directly, not the socket methods over
it: `sock.readMessages` filters its input through `receiptMessageKeys`
first, so the bad value under test never reaches the bridge that way. And
the logged-out reasons are injected as synthetic events, so they pin our
pass-through and message format, not the bridge's mapping, which the
bridge asserts literally in its own tests.
@greptile-apps

greptile-apps Bot commented Aug 8, 2026 •

Copy link
Copy Markdown

Greptile Summary

The PR upgrades @oxidezap/whatsapp-rust-bridge from 0.6.5 to 0.7.0 and adds regression coverage for its changed argument validation, logout-reason strings, and wasm crash signature.

  • Pins the dependency and lockfile to bridge 0.7.0.
  • Adds child-process tests confirming invalid bridge arguments reject without terminating the test runner.
  • Adds logout-reason pass-through coverage.
  • Updates the free-safety test to recognize both documented wasm memory-fault signatures while retaining wasm-origin and process-failure checks.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
package.json Pins the bridge runtime dependency to version 0.7.0.
package-lock.json Resolves bridge 0.7.0 with its updated registry integrity metadata.
src/tests/bridge-argument-rejection.test.ts Adds isolated regression probes verifying malformed bridge arguments reject with structured invalid-argument errors.
src/tests/auto-reconnect-terminal-close.test.ts Pins logout reason strings, messages, and disconnect status behavior.
src/tests/bridge-free-safety.test.ts Updates heap-corruption coverage for the crash signature emitted by bridge 0.7.0 while preserving origin and termination assertions.

Reviews (3): Last reviewed commit: "test(bridge): name which probes stand fo..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot previously approved these changes Aug 8, 2026
@jlucaso1
jlucaso1 force-pushed the claude/bridge-0-7-0-upgrade-kripwv branch from 71903fc to 249cbe0 Compare August 8, 2026 20:24
@greptile-apps
greptile-apps Bot dismissed their stale review August 8, 2026 20:24

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

@jlucaso1 jlucaso1 changed the title chore(deps): update whatsapp-rust-bridge to 0.7.0 fix(deps): update whatsapp-rust-bridge to 0.7.0 Aug 8, 2026

jlucaso1 commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Retyped the bump from chore(deps) to fix(deps), title included. The Codex P1 was right and it is the one finding here with a shipping consequence: release.yml says in so many words that a chore-typed bridge bump releases nothing and leaves the upgrade sitting on main until an unrelated commit drags it out. Same tree, only the messages changed, so the force-push carries no content.

While retyping I also moved fix(tests): describe the free() hazard... to test(bridge): .... It changes no shipped code, and fix would have listed it in the user-facing changelog as a bug fix.

Everything else that landed since the first push:

  • A stalled probe now exits the child immediately. It used to leave its call in flight and carry on to free(), which is the heap corruption the sibling suite documents, so the crash got reported instead of the probe that caused it.
  • Added a child.on('error') handler. A failed spawn was throwing in the runner and leaving the promise pending until the suite timeout.
  • Both assertions now carry the child's stderr. It was being collected and never shown, so a child that died during setup failed with expected false to be true.
  • Reports are written with an awaited write callback instead of console.log. stdout is a pipe, and process.exit() does not wait for it, so a loaded runner could truncate the run and the parent would read that as a regression.
  • Dropped unreachable from the fault allowlist. The other two entries name the build that emits them; that one named none.

Two comments I did not act on, with reasons.

Deriving the logged-out reasons from the bridge instead of injecting them. Fair as stated, and I have corrected the comment rather than the test: a real logged_out event needs a server saying so, which is e2e territory. What the test can pin is our half, and that is now what it claims to pin: the dispatcher passes the reason through untouched, the message is exactly Logged out: <reason>, and the status code stays loggedOut regardless. The bridge asserts the spellings literally in its own tests.

Running the rejection probes through makeWASocket. The reading of receiptMessageKeys is correct: sock.readMessages('not-an-array') iterates the characters, drops every one for lacking remoteJid and id, and resolves without touching the bridge. So going through the socket would test our normalizer, not the bridge change that is the subject here. I have reworded the comment to say the probes drive the boundary directly and to name that filtering explicitly, and to note the boundary is still consumer-facing because MediaGenerationOptions.processMedia hands the raw client over.

Setting an explicit cwd on the spawned child. Skipped. Node resolves the bare specifiers by walking up from the eval base, so any cwd inside the repo finds node_modules, and a cwd outside it means node --test matched no test files to begin with. Deriving the root as SRC/../.. would add an assumption about where the file sits that the code does not currently make.

npm run lint and npm test (666 tests) green on the retyped branch.


Generated by Claude Code

greptile-apps[bot]
greptile-apps Bot previously approved these changes Aug 8, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found and verified against the latest diff

Confidence score: 4/5

  • In package.json, the bridge dependency bump is tagged as chore(deps), which can be ignored by Release Please under the stated policy; the practical risk is shipping code that never gets a release cut — retag as fix(deps) (or update release rules) so the bump is actually published.
  • In src/__tests__/bridge-argument-rejection.test.ts, calling c.readMessages/c.sendPresence directly bypasses readMessages/sendPresenceUpdate wrapper filtering, so wrapper-level regressions could slip through while tests still pass — move these probes through the public wrappers or add parallel wrapper-path assertions.
  • In src/__tests__/auto-reconnect-terminal-close.test.ts, the test injects the reason string instead of exercising wasm bridge output, so it may not catch upstream reason-format changes despite the comment’s claim — validate the reason via the real bridge path (or adjust the test/comment to match what is actually covered).
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="package.json">

<violation number="1" location="package.json:78">
P2: This dependency bump uses a `chore(deps)` commit type, but per the release policy only `feat`, `fix`, and `perf` commits trigger a Release Please release (and bridge bumps specifically require `fix(deps)`). With the current commit type, this upgrade won't produce a release/tag, leaving consumers without the fix.</violation>
</file>

<file name="src/__tests__/auto-reconnect-terminal-close.test.ts">

<violation number="1" location="src/__tests__/auto-reconnect-terminal-close.test.ts:268">
P3: The comment claims this test is 'where the contract is observed' and that without it an upstream reason-spelling change would be 'caught', but the test injects the reason string itself and never touches the wasm bridge, so it only asserts JS interpolation (`Logged out: ${evt.reason}` at events.ts:392) is verbatim. A renamed or re-spelled `ConnectFailureReason` variant in bridge 0.8 would leave these assertions green, so the stated protective purpose isn't actually met. Consider having the test derive its expected values from the real bridge output (an e2e/integration path) or rewrite the comment to say it pins the JS interpolation only, not bridge spellings.</violation>
</file>

<file name="src/__tests__/bridge-argument-rejection.test.ts">

<violation number="1" location="src/__tests__/bridge-argument-rejection.test.ts:53">
P2: These probes call the raw bridge client (`c.readMessages`, `c.sendPresence`) directly instead of going through the public socket wrappers (`readMessages`, `sendPresenceUpdate`). Those wrappers apply their own input filtering before reaching the bridge (e.g. `receiptMessageKeys` silently drops invalid entries for `'not-an-array'`), so these probes don't actually exercise the argument-rejection contract for consumers using the public API surface.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread package.json
"dependencies": {
"@hapi/boom": "^9.1.4",
"@oxidezap/whatsapp-rust-bridge": "0.6.5",
"@oxidezap/whatsapp-rust-bridge": "0.7.0",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This dependency bump uses a chore(deps) commit type, but per the release policy only feat, fix, and perf commits trigger a Release Please release (and bridge bumps specifically require fix(deps)). With the current commit type, this upgrade won't produce a release/tag, leaving consumers without the fix.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 78:

<comment>This dependency bump uses a `chore(deps)` commit type, but per the release policy only `feat`, `fix`, and `perf` commits trigger a Release Please release (and bridge bumps specifically require `fix(deps)`). With the current commit type, this upgrade won't produce a release/tag, leaving consumers without the fix.</comment>

<file context>
@@ -75,7 +75,7 @@
   "dependencies": {
     "@hapi/boom": "^9.1.4",
-    "@oxidezap/whatsapp-rust-bridge": "0.6.5",
+    "@oxidezap/whatsapp-rust-bridge": "0.7.0",
     "long": "^5.3.2",
     "pino": "^10.3.1",
</file context>

const PROBES: ProbeSpec[] = [
{
label: 'readMessages with a string where the key list goes',
call: `c.readMessages('not-an-array')`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: These probes call the raw bridge client (c.readMessages, c.sendPresence) directly instead of going through the public socket wrappers (readMessages, sendPresenceUpdate). Those wrappers apply their own input filtering before reaching the bridge (e.g. receiptMessageKeys silently drops invalid entries for 'not-an-array'), so these probes don't actually exercise the argument-rejection contract for consumers using the public API surface.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/__tests__/bridge-argument-rejection.test.ts, line 51:

<comment>These probes call the raw bridge client (`c.readMessages`, `c.sendPresence`) directly instead of going through the public socket wrappers (`readMessages`, `sendPresenceUpdate`). Those wrappers apply their own input filtering before reaching the bridge (e.g. `receiptMessageKeys` silently drops invalid entries for `'not-an-array'`), so these probes don't actually exercise the argument-rejection contract for consumers using the public API surface.</comment>

<file context>
@@ -0,0 +1,171 @@
+const PROBES: ProbeSpec[] = [
+	{
+		label: 'readMessages with a string where the key list goes',
+		call: `c.readMessages('not-an-array')`,
+		field: 'keys',
+		why: 'a typed list parameter'
</file context>

it(`${reason} → 'Logged out: ${reason}'`, () => {
const result = dispatch({ type: 'logged_out', data: { reason } })

expect((result.close?.lastDisconnect?.error as Boom | undefined)?.message).toBe(`Logged out: ${reason}`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The comment claims this test is 'where the contract is observed' and that without it an upstream reason-spelling change would be 'caught', but the test injects the reason string itself and never touches the wasm bridge, so it only asserts JS interpolation (Logged out: ${evt.reason} at events.ts:392) is verbatim. A renamed or re-spelled ConnectFailureReason variant in bridge 0.8 would leave these assertions green, so the stated protective purpose isn't actually met. Consider having the test derive its expected values from the real bridge output (an e2e/integration path) or rewrite the comment to say it pins the JS interpolation only, not bridge spellings.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/__tests__/auto-reconnect-terminal-close.test.ts, line 267:

<comment>The comment claims this test is 'where the contract is observed' and that without it an upstream reason-spelling change would be 'caught', but the test injects the reason string itself and never touches the wasm bridge, so it only asserts JS interpolation (`Logged out: ${evt.reason}` at events.ts:392) is verbatim. A renamed or re-spelled `ConnectFailureReason` variant in bridge 0.8 would leave these assertions green, so the stated protective purpose isn't actually met. Consider having the test derive its expected values from the real bridge output (an e2e/integration path) or rewrite the comment to say it pins the JS interpolation only, not bridge spellings.</comment>

<file context>
@@ -227,6 +227,56 @@ describe('connection.update: terminal disconnects', () => {
+		it(`${reason} → 'Logged out: ${reason}'`, () => {
+			const result = dispatch({ type: 'logged_out', data: { reason } })
+
+			expect((result.close?.lastDisconnect?.error as Boom | undefined)?.message).toBe(`Logged out: ${reason}`)
+			expect(result.statusCode).toBe(DisconnectReason.loggedOut)
+		})
</file context>

…o not

The comment lumped all four together as "not the socket methods", which
undersold two of them and left the other two vague. Per probe now:
`muteChat` is a bare pass-through on the socket, so that probe is the
public path exactly; `encryptMediaStream` reaches consumers raw through
`processMedia`; `readMessages` is absorbed by `receiptMessageKeys` and
`sendPresence` by `sendPresenceUpdate`, which turns an off-union status
into a missing-target-jid Boom first.

Comment only, no assertion changed.
@greptile-apps
greptile-apps Bot dismissed their stale review August 8, 2026 20:41

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

jlucaso1 commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Checked all three against the branch as it stands.

chore(deps) on the bump — already fixed, before this review landed. The commit is d6034c1 fix(deps): update whatsapp-rust-bridge to 0.7.0 and the PR title matches. The finding reads the file diff, which carries no commit type, so it cannot see this either way. Worth saying that the underlying point was right and it was Codex that caught it first; it is the one issue in this PR that would have cost a release.

The logout-reason comment — already rewritten in 249cbe0. The quoted phrases ("where the contract is observed", the claim that a bridge-side change would be caught) are gone. What is there now says the events are synthetic and that this pins our half only: the dispatcher passes the reason through untouched, the message format, and the status code staying loggedOut. Which is the same correction the finding asks for.

The probes — half right, and I have made the comment exact rather than restructured the test. Checked each of the four against its socket wrapper:

  • muteChat is a bare pass-through (chat-actions.ts:9), so sock.muteChat(jid, Infinity) reaches the bridge with the bad argument intact. That probe is the public path.
  • encryptMediaStream is handed to consumers raw through MediaGenerationOptions.processMedia.
  • readMessages is absorbed by receiptMessageKeys, exactly as described.
  • sendPresence is absorbed by sendPresenceUpdate, though not by filtering: 'nope' is neither available nor unavailable, so it falls through to the branch that throws a missing-target-jid Boom (index.ts:890) before the bridge is reached.

So the blanket claim does not hold for half the probes, and the previous comment undersold them by lumping all four together. 3d4b4c6 names each one. I am not routing the absorbed two through makeWASocket: that would test our normalizers, not the bridge change this file is about, and for readMessages it would assert a silent no-op rather than a rejection.

Assertions unchanged, npm run lint and npm test green.


Generated by Claude Code

@jlucaso1
jlucaso1 merged commit 10471a4 into main Aug 8, 2026
7 checks passed
@jlucaso1
jlucaso1 deleted the claude/bridge-0-7-0-upgrade-kripwv branch September 9, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants