Prevent duplicate structured media delivery from tool results

By adding structuredMediaUrls into the onToolResult payload here, structured attachments are now emitted once in tool-output callbacks and again later from pending media state in verbose full runs. handleToolExecutionEnd still queues structured media via queuePendingToolMedia(...) (src/agents/pi-embedded-subscribe.handlers.tools.ts:315-326), and dispatch logic still forwards media-only tool results (src/auto-reply/reply/dispatch-from-config.ts:645-651), so channels that handle both tool-result and block-reply streams can receive the same media twice for a single tool execution.

Useful? React with 👍 / 👎.

Potential double delivery of structured media when verboseLevel is "full"

When shouldEmitToolOutput() is true AND hasStructuredMedia is true (i.e. details.media is present), emitToolResultOutput in handlers.tools.ts calls ctx.emitToolOutput(...) (which now routes through the updated emitToolResultMessage including structured media via mergedMediaUrls) and then falls through to queuePendingToolMedia with the same structured URLs.

// handlers.tools.ts (unchanged)
if (ctx.shouldEmitToolOutput()) {
    const outputText = extractToolResultText(sanitizedResult);
    if (outputText) {
        ctx.emitToolOutput(toolName, meta, outputText, result); // structured media now included here
    }
    if (!hasStructuredMedia) {
        return;   // skipped when structured media IS present
    }
}
// falls through to:
queuePendingToolMedia(ctx, { mediaUrls, ... });   // same media queued again

onToolResult and onBlockReply are different callbacks, but if the outer caller routes both to the same channel (e.g. Telegram), the generated image could be delivered twice. Before this PR, structured media only reached queuePendingToolMedia; onToolResult never carried it. Now it can arrive through both paths simultaneously.

Could you confirm whether callers that wire up both onToolResult and onBlockReply/pending-block-reply to the same delivery channel are guarded against duplicates, or whether the queuePendingToolMedia path is intentionally left unchanged here? When shouldEmitToolOutput() is true and the result has details.media, structured media now flows through both emitToolResultMessage → onToolResult and queuePendingToolMedia → onBlockReply. Is there de-duplication at the channel-delivery layer, or does the outer caller always use only one of these two paths for a given flow?

This is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.ts
Line: 353-368

Comment:
**Potential double delivery of structured media when verboseLevel is "full"**

When `shouldEmitToolOutput()` is true AND `hasStructuredMedia` is true (i.e. `details.media` is present), `emitToolResultOutput` in `handlers.tools.ts` calls `ctx.emitToolOutput(...)` (which now routes through the updated `emitToolResultMessage` including structured media via `mergedMediaUrls`) **and** then falls through to `queuePendingToolMedia` with the same structured URLs.

```
// handlers.tools.ts (unchanged)
if (ctx.shouldEmitToolOutput()) {
    const outputText = extractToolResultText(sanitizedResult);
    if (outputText) {
        ctx.emitToolOutput(toolName, meta, outputText, result); // structured media now included here
    }
    if (!hasStructuredMedia) {
        return;   // skipped when structured media IS present
    }
}
// falls through to:
queuePendingToolMedia(ctx, { mediaUrls, ... });   // same media queued again
```

`onToolResult` and `onBlockReply` are different callbacks, but if the outer caller routes both to the same channel (e.g. Telegram), the generated image could be delivered twice. Before this PR, structured media only reached `queuePendingToolMedia`; `onToolResult` never carried it. Now it can arrive through both paths simultaneously.

Could you confirm whether callers that wire up both `onToolResult` and `onBlockReply`/pending-block-reply to the same delivery channel are guarded against duplicates, or whether the `queuePendingToolMedia` path is intentionally left unchanged here? When shouldEmitToolOutput() is true and the result has details.media, structured media now flows through both emitToolResultMessage → onToolResult and queuePendingToolMedia → onBlockReply. Is there de-duplication at the channel-delivery layer, or does the outer caller always use only one of these two paths for a given flow?

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

audioAsVoice flag can attach to non-audio media in a mixed-source edge case

structuredArtifact?.audioAsVoice === true is spread unconditionally based solely on the artifact, regardless of whether any of its URLs survived filterToolResultMediaUrls. If all structured-media URLs are filtered away (e.g. local path from an untrusted tool) but filteredMediaUrls (inline MEDIA:) provides surviving URLs, the audioAsVoice: true flag would be forwarded together with those non-audio inline media URLs.

In practice this is unlikely because audioAsVoice is only set by trusted tools (e.g. tts) that would also pass the local-path trust filter. But if this invariant ever breaks, the downstream channel could mishandle image URLs as voice audio.

A tighter guard would be to only set audioAsVoice when at least one URL came from the structured artifact:

const structuredUrlsInMerged = structuredMediaUrls.length > 0;
...(structuredUrlsInMerged && structuredArtifact?.audioAsVoice === true
  ? { audioAsVoice: true }
  : {}),

This is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.ts
Line: 367

Comment:
**`audioAsVoice` flag can attach to non-audio media in a mixed-source edge case**

`structuredArtifact?.audioAsVoice === true` is spread unconditionally based solely on the artifact, regardless of whether any of its URLs survived `filterToolResultMediaUrls`. If all structured-media URLs are filtered away (e.g. local path from an untrusted tool) but `filteredMediaUrls` (inline `MEDIA:`) provides surviving URLs, the `audioAsVoice: true` flag would be forwarded together with those non-audio inline media URLs.

In practice this is unlikely because `audioAsVoice` is only set by trusted tools (e.g. `tts`) that would also pass the local-path trust filter. But if this invariant ever breaks, the downstream channel could mishandle image URLs as voice audio.

A tighter guard would be to only set `audioAsVoice` when at least one URL came from the structured artifact:

```ts
const structuredUrlsInMerged = structuredMediaUrls.length > 0;
...(structuredUrlsInMerged && structuredArtifact?.audioAsVoice === true
  ? { audioAsVoice: true }
  : {}),
```

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

Keep queuing structured media when tool output text is empty

In full verbose runs, this unconditional return exits before the fallback media queueing path, so structured attachments are dropped whenever a tool result has details.media but no text content blocks. ctx.emitToolOutput(...) is only called when extractToolResultText(...) yields text, so media-only results (for example image results produced without extraText, such as the canvas snapshot path) now reach neither onToolResult nor pendingToolMedia delivery, causing missing outbound media in embedded replies.

Useful? React with 👍 / 👎.

Preserve media-only tool results in full verbose mode

Returning unconditionally from the ctx.shouldEmitToolOutput() branch drops structured media whenever extractToolResultText(sanitizedResult) is empty. In that case ctx.emitToolOutput(...) is never called, and this early return skips the fallback extractToolResultMediaArtifact(...) + queuePendingToolMedia(...) path, so tools that return only details.media (no text blocks) lose attachments entirely in verbose full runs.

Useful? React with 👍 / 👎.

Avoid returning before media queue when tool callback is absent

This early return drops structured tool media whenever shouldEmitToolOutput() is true and extractToolResultText(...) is non-empty but onToolResult is not wired. In that case ctx.emitToolOutput(...) is effectively a no-op (the subscribe layer bails when onToolResult is undefined), and because this branch returns before queuePendingToolMedia(...), media attachments are never forwarded via the block-reply path. This is a regression from the prior flow for consumers that run in verbose=full without an onToolResult handler.

Useful? React with 👍 / 👎.

Suppress structured media when forwarding failed tool output

This new merge path forwards details.media attachments even for failed tool runs in verbose=full: emitToolResultOutput invokes emitToolOutput(...) before the isToolError guard in src/agents/pi-embedded-subscribe.handlers.tools.ts, and this code now unconditionally adds structured artifact URLs to the onToolResult payload. In practice, a tool result that has both error text and details.media.mediaUrls will send media despite the existing error-path intent to avoid media delivery, which can leak or deliver stale/corrupt attachments.

Useful? React with 👍 / 👎.

Preserve MCP provenance when emitting failed tool output

When isToolError is true, this branch now calls emitToolOutput with outputResult set to undefined, which strips details.mcpServer/mcpTool provenance before media filtering runs in emitToolResultMessage. In verbose=full sessions with onToolResult enabled, an external/MCP tool error that includes a MEDIA:/tmp/... line in text can now be treated as a trusted core-tool local path and forwarded, whereas previously the same payload was filtered out as external. Fresh evidence in this commit is the new const outputResult = isToolError ? undefined : result; assignment.

Useful? React with 👍 / 👎.

Prevent error tool output from re-attaching MEDIA paths

Fresh evidence in this commit is the new extractToolResultMediaArtifact(result) merge path in emitToolResultMessage: that helper also parses legacy MEDIA: lines from result.content, so stripping only details.media/details.path for isToolError no longer prevents media forwarding. In verbose=full, a failed trusted tool that returns text like MEDIA:/tmp/... can now emit a local attachment through onToolResult, which contradicts the error-path media suppression and can expose stale or sensitive files.

Useful? React with 👍 / 👎.