Skip to content

fix(whatsapp): notify user when trailing media send fails instead of silent drop#93334

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
rushindrasinha:fix/whatsapp-trailing-media-silent-drop
Jun 15, 2026
Merged

fix(whatsapp): notify user when trailing media send fails instead of silent drop#93334
vincentkoc merged 1 commit into
openclaw:mainfrom
rushindrasinha:fix/whatsapp-trailing-media-silent-drop

Conversation

@rushindrasinha

@rushindrasinha rushindrasinha commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Problem

When a reply contains multiple media attachments and a non-first send fails, the onError callback in deliver-reply.ts returns early without any user notification:

onError: async ({ error, mediaUrl, caption, isFirst }) => {
  whatsappOutboundLog.error(...);
  replyLogger.warn(...);
  if (!isFirst) {
    return;  // ← silent drop: user never knows the attachment failed
  }
  // first-item fallback logic...

The error is logged server-side only. The recipient sees the first media attachment (or its text fallback) but the second, third, etc. simply never arrive with no indication of failure.

Fix

Replace the silent return with a brief text reply so the user knows the attachment was unavailable:

if (!isFirst) {
  whatsappOutboundLog.warn(`Trailing media failed; sent warning to ${msg.from}`);
  rememberSendResult(
    await sendWithRetry(
      () => msg.platform.reply("⚠️ Media unavailable.", getQuote()),
      "media:fallback-unavailable",
    ),
  );
  return;
}

Test

Added deliver-reply.test.ts: "notifies user when a non-first media send fails instead of dropping silently"

Sets up two-media reply where first send succeeds and second fails (non-retryable error). Asserts reply is called exactly once with "⚠️ Media unavailable".

All 29 deliver-reply tests pass.

Observed on a production deployment where multi-attachment replies occasionally lost trailing attachments without any visible error to the recipient.

Real behavior proof

  • Behavior or issue addressed: In deliverWebReply, when a reply has multiple media attachments and a non-first send fails, the onError callback returned early with no user notification — the recipient silently never received the trailing attachment(s).
  • Real environment tested: OpenClaw checkout on macOS (Darwin arm64, Node v22). Ran the actual patched deliverWebReply from this branch via node --import tsx, loading two real local image files through the real loadWebMedia path (files placed under the allowed local root /tmp/openclaw).
  • Exact steps or command run after this patch: built a reply with two media URLs, wired a transport whose sendMedia accepts the first send and rejects the second (trailing) send with an upload error, then invoked delivery:
    node --import tsx proof-media.mts
  • Evidence after fix: live stdout from the patched delivery path:
$ node --import tsx proof-media.mts
platform.sendMedia() call #1 -> OK
[whatsapp] Sent media reply to +10000000000 (0.00MB)
platform.sendMedia() call #2 -> FAILS (simulated upload error)
[whatsapp] Failed sending web media to +10000000000: upload failed
[whatsapp] Trailing media failed; sent warning to +10000000000
platform.reply() called -> "⚠️ Media unavailable."

replies sent to user: ["⚠️ Media unavailable."]
RESULT: trailing media failure now NOTIFIES the user (pre-fix this was silently dropped)
  • Observed result after fix: when the trailing media send failed, the patched code sent the user a ⚠️ Media unavailable. reply instead of returning silently. The raw error string (upload failed) was kept server-side only and not leaked to the user.
  • What was not tested: delivery against a live WhatsApp socket to a real device was not exercised; the failure was injected at the transport boundary (platform.sendMedia) so the real deliverWebReply error-handling branch executed end to end.

@rushindrasinha
fix(whatsapp): notify user when trailing media send fails instead of …
0666e59 
…silent drop

When a multi-media reply contains more than one attachment and a
non-first send fails, the onError callback returned early without
notifying the user. The error was logged server-side only; from the
recipient's perspective the attachment simply never arrived.

Replace the silent return with a brief text fallback so the user
knows the attachment was unavailable.

Co-Authored-By: Dr Rushindra Sinha <5796457+rushindrasinha@users.noreply.github.com>
@openclaw-barnacle openclaw-barnacle Bot added channel: whatsapp-web Channel integration: whatsapp-web size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 11:54 AM ET / 15:54 UTC.

Summary
The PR changes WhatsApp Web media delivery so failed non-first media attachments send a short unavailable warning instead of returning silently, with a regression test for the two-attachment failure path.

PR surface: Source +9, Tests +41. Total +50 across 2 files.

Reproducibility: yes. Current main has a clear source path: a multi-media WhatsApp reply whose non-first media send fails enters onError with isFirst: false and returns after logging.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • No automated repair lane is needed; this is a focused implementation PR with no blocking findings from the read-only review.

Security
Cleared: The diff only changes WhatsApp delivery behavior and a colocated test, with no CI, dependency, secret, or supply-chain surface touched.

Review details

Best possible solution:

Land the focused WhatsApp-plugin fallback after normal maintainer review and required checks, keeping the shared media helper channel-agnostic.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main has a clear source path: a multi-media WhatsApp reply whose non-first media send fails enters onError with isFirst: false and returns after logging.

Is this the best way to solve the issue?

Yes. deliverWebReply already owns WhatsApp-specific media fallback text, while the shared helper only provides first/trailing metadata and should stay channel-agnostic.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 794bd89fa030.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live terminal output from the patched deliverWebReply path with real local media loading and a send failure injected at the transport boundary.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live terminal output from the patched deliverWebReply path with real local media loading and a send failure injected at the transport boundary.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: The PR fixes a bounded WhatsApp media-delivery failure notification with limited channel-specific blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live terminal output from the patched deliverWebReply path with real local media loading and a send failure injected at the transport boundary.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live terminal output from the patched deliverWebReply path with real local media loading and a send failure injected at the transport boundary.
Evidence reviewed

PR surface:

Source +9, Tests +41. Total +50 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 9 0 +9
Tests 1 41 0 +41
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 50 0 +50

What I checked:

Likely related people:

  • mcaxtr: GitHub commit history for the WhatsApp deliver-reply files shows recent WhatsApp auto-reply and socket-operation work by Marcus Castro, including nearby runtime and test changes. (role: recent area contributor; confidence: high; commits: eebcb100b835, 7d8b000bf78a, 008d785a80fb; files: extensions/whatsapp/src/auto-reply/deliver-reply.ts, extensions/whatsapp/src/auto-reply/deliver-reply.test.ts)
  • steipete: GitHub commit history shows repeated WhatsApp delivery, reply-payload helper, and colocated test maintenance by Peter Steinberger across the same files. (role: historical area contributor; confidence: high; commits: eab402493429, 9ffe7644169b, 8d73bc77fa5d; files: extensions/whatsapp/src/auto-reply/deliver-reply.ts, extensions/whatsapp/src/auto-reply/deliver-reply.test.ts, src/plugin-sdk/reply-payload.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. labels Jun 15, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 15, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 15, 2026
@vincentkoc vincentkoc self-assigned this Jun 15, 2026
vincentkoc
vincentkoc approved these changes Jun 15, 2026
View reviewed changes

@vincentkoc vincentkoc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed live state, focused WhatsApp delivery diff, and green checks. The fallback notification is scoped and covered by regression tests.

@vincentkoc vincentkoc merged commit 00c58b6 into openclaw:main Jun 15, 2026
241 of 258 checks passed
@yetval yetval mentioned this pull request Jun 17, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jun 18, 2026
Open
@Hansen1018 Hansen1018 mentioned this pull request Jun 25, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vincentkoc vincentkoc vincentkoc approved these changes

Assignees

@vincentkoc vincentkoc

Labels

channel: whatsapp-web Channel integration: whatsapp-web P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rushindrasinha @vincentkoc