Skip to content

fix(whatsapp): deliver tool replies that include media#60968

Merged
mcaxtr merged 2 commits intoopenclaw:mainfrom
adaclaw:fix/whatsapp-deliver-tool-media-payloads
Apr 25, 2026
Merged

fix(whatsapp): deliver tool replies that include media#60968
mcaxtr merged 2 commits intoopenclaw:mainfrom
adaclaw:fix/whatsapp-deliver-tool-media-payloads

Conversation

@adaclaw
Copy link
Copy Markdown
Contributor

@adaclaw adaclaw commented Apr 4, 2026

Problem

WhatsApp outbound delivery suppressed every kind === "tool" payload in shouldSuppressWhatsAppPayload, so tool results that only carried media (e.g. generated image paths) never reached WhatsApp.

Solution

For tool lifecycle payloads, suppress only when resolveSendableOutboundReplyParts(payload).hasMedia is false. Text-only tool chatter stays suppressed; tool payloads with media are delivered like other media replies.

Tests

  • Updated inbound-dispatch.test.ts: mock derives hasMedia from mediaUrls / mediaUrl; assert tool+media calls deliverReply.

Note on the Pi / tool_execution_end scheduling change

We also investigated removing { detach: true } from tool_execution_end scheduling. That ordering interacts with deferred after_tool_call / microtask work and can race with lastToolError clearing (see subscribeEmbeddedPiSession mutating-failure test). Shipping that safely likely needs awaiting or restructuring those hooks—not included here.

Verification

pnpm exec vitest run extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.test.ts (17 passed).

Made with Cursor

@openclaw-barnacle openclaw-barnacle Bot added channel: whatsapp-web Channel integration: whatsapp-web size: XS labels Apr 4, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 4, 2026

Greptile Summary

This PR fixes WhatsApp outbound delivery so that kind === \"tool\" reply payloads which carry media (e.g. a generated image path) are no longer unconditionally suppressed — they now go through deliverReply when resolveSendableOutboundReplyParts(payload).hasMedia is true. Text-only tool chatter remains suppressed.

The suppression fix itself (shouldSuppressWhatsAppPayload) is correct and the test update properly validates the new path. There is one logical gap worth addressing before landing:

  • didQueueVisibleReply does not count tool deliveries (extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.ts line 317). For a turn that contains only a tool+media payload, counts.block === 0, counts.final === 0, and queuedFinal === false, so didQueueVisibleReply is false. The function then returns false at line 323 even though didSendReply was set to true by the delivery. Callers would incorrectly conclude that the turn was silent. Adding counts.tool > 0 to the visibility check closes the gap.
  • The test's getCapturedDeliver helper narrows the payload type in a way that excludes mediaUrls/mediaUrl, requiring an as never cast at line 274; widening that type would make the assertion type-safe.

Confidence Score: 3/5

Safe to merge with a small fix — the suppression change is correct but the visibility-check gap means tool-only-media turns report false to callers despite delivering media.

The core one-line fix is right and tests confirm the deliver callback is reached for tool+media payloads. However, didQueueVisibleReply (line 317) still ignores counts.tool, so dispatchWhatsAppBufferedReply returns false for a turn whose only payload is a tool+media reply — the exact scenario this PR targets. That discrepancy in the return value is a logical gap introduced by this change and warrants addressing before landing.

extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.ts — specifically the didQueueVisibleReply check at line 317.

Comments Outside Diff (2)

  1. extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.ts, line 317-323 (link)

    P1 Tool-media turns hit the early return false, discarding delivery

    didQueueVisibleReply only checks queuedFinal || counts.block > 0 || counts.final > 0counts.tool is never included. Before this PR that was fine because tool payloads were always suppressed and didSendReply could never be set by them. After this PR, a tool payload that carries media goes through deliverReply and sets didSendReply = true, but if it's the only payload in the turn (counts.block === 0, counts.final === 0, queuedFinal === false) then didQueueVisibleReply is still false and the function takes the early return false path at line 323, throwing away the true value of didSendReply.

    This means callers receive false (i.e. "no visible reply sent") even though a media attachment was just delivered to WhatsApp. The scenario is precisely the one this PR targets — a tool result that produces only media.

    To fix, include counts.tool > 0 in the visibility check:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.ts
    Line: 317-323
    
    Comment:
    **Tool-media turns hit the early `return false`, discarding delivery**
    
    `didQueueVisibleReply` only checks `queuedFinal || counts.block > 0 || counts.final > 0``counts.tool` is never included. Before this PR that was fine because tool payloads were always suppressed and `didSendReply` could never be set by them. After this PR, a tool payload that carries media goes through `deliverReply` and sets `didSendReply = true`, but if it's the only payload in the turn (`counts.block === 0`, `counts.final === 0`, `queuedFinal === false`) then `didQueueVisibleReply` is still `false` and the function takes the early `return false` path at line 323, throwing away the `true` value of `didSendReply`.
    
    This means callers receive `false` (i.e. "no visible reply sent") even though a media attachment was just delivered to WhatsApp. The scenario is precisely the one this PR targets — a tool result that produces only media.
    
    To fix, include `counts.tool > 0` in the visibility check:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.test.ts, line 99-110 (link)

    P2 getCapturedDeliver payload type doesn't include mediaUrls / mediaUrl

    The narrow payload type returned by getCapturedDeliver doesn't include mediaUrls or mediaUrl, which forces the test at line 274 to cast with as never to pass those fields. Widening the type here would make the tool-media test case type-safe without a cast:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.test.ts
    Line: 99-110
    
    Comment:
    **`getCapturedDeliver` payload type doesn't include `mediaUrls` / `mediaUrl`**
    
    The narrow payload type returned by `getCapturedDeliver` doesn't include `mediaUrls` or `mediaUrl`, which forces the test at line 274 to cast with `as never` to pass those fields. Widening the type here would make the tool-media test case type-safe without a cast:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.ts
Line: 317-323

Comment:
**Tool-media turns hit the early `return false`, discarding delivery**

`didQueueVisibleReply` only checks `queuedFinal || counts.block > 0 || counts.final > 0``counts.tool` is never included. Before this PR that was fine because tool payloads were always suppressed and `didSendReply` could never be set by them. After this PR, a tool payload that carries media goes through `deliverReply` and sets `didSendReply = true`, but if it's the only payload in the turn (`counts.block === 0`, `counts.final === 0`, `queuedFinal === false`) then `didQueueVisibleReply` is still `false` and the function takes the early `return false` path at line 323, throwing away the `true` value of `didSendReply`.

This means callers receive `false` (i.e. "no visible reply sent") even though a media attachment was just delivered to WhatsApp. The scenario is precisely the one this PR targets — a tool result that produces only media.

To fix, include `counts.tool > 0` in the visibility check:

```suggestion
  const didQueueVisibleReply = queuedFinal || counts.block > 0 || counts.final > 0 || counts.tool > 0;
  if (!didQueueVisibleReply) {
    if (params.shouldClearGroupHistory) {
      params.groupHistories.set(params.groupHistoryKey, []);
    }
    logVerbose("Skipping auto-reply: silent token or no text/media returned from resolver");
    return false;
  }
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/whatsapp/src/auto-reply/monitor/inbound-dispatch.test.ts
Line: 99-110

Comment:
**`getCapturedDeliver` payload type doesn't include `mediaUrls` / `mediaUrl`**

The narrow payload type returned by `getCapturedDeliver` doesn't include `mediaUrls` or `mediaUrl`, which forces the test at line 274 to cast with `as never` to pass those fields. Widening the type here would make the tool-media test case type-safe without a cast:

```suggestion
function getCapturedDeliver() {
  return (
    capturedDispatchParams as {
      dispatcherOptions?: {
        deliver?: (
          payload: {
            text?: string;
            isReasoning?: boolean;
            isCompactionNotice?: boolean;
            mediaUrls?: string[];
            mediaUrl?: string;
          },
          info: { kind: "tool" | "block" | "final" },
        ) => Promise<void>;
      };
    }
  )?.dispatcherOptions?.deliver;
}
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(whatsapp): deliver tool replies that..." | Re-trigger Greptile

@mcaxtr mcaxtr self-assigned this Apr 25, 2026
@mcaxtr mcaxtr force-pushed the fix/whatsapp-deliver-tool-media-payloads branch from 3ec54ac to 8d8d58b Compare April 25, 2026 02:06
@mcaxtr mcaxtr force-pushed the fix/whatsapp-deliver-tool-media-payloads branch 4 times, most recently from b3b8ce0 to e1559a5 Compare April 25, 2026 02:43
@mcaxtr mcaxtr force-pushed the fix/whatsapp-deliver-tool-media-payloads branch from e1559a5 to 2670402 Compare April 25, 2026 02:44
@mcaxtr mcaxtr merged commit 413e407 into openclaw:main Apr 25, 2026
9 checks passed
@mcaxtr
Copy link
Copy Markdown
Member

mcaxtr commented Apr 25, 2026

Merged via squash.

Thanks @adaclaw!

Angfr95 pushed a commit to Angfr95/openclaw that referenced this pull request Apr 25, 2026
Merged via squash.

Prepared head SHA: 2670402
Co-authored-by: adaclaw <266167987+adaclaw@users.noreply.github.com>
Co-authored-by: mcaxtr <7562095+mcaxtr@users.noreply.github.com>
Reviewed-by: @mcaxtr
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
Merged via squash.

Prepared head SHA: 2670402
Co-authored-by: adaclaw <266167987+adaclaw@users.noreply.github.com>
Co-authored-by: mcaxtr <7562095+mcaxtr@users.noreply.github.com>
Reviewed-by: @mcaxtr
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
Merged via squash.

Prepared head SHA: 2670402
Co-authored-by: adaclaw <266167987+adaclaw@users.noreply.github.com>
Co-authored-by: mcaxtr <7562095+mcaxtr@users.noreply.github.com>
Reviewed-by: @mcaxtr
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: whatsapp-web Channel integration: whatsapp-web size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants