From a68ff9961c3cee5f8e076cbfdafda16c15bc2294 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 7 Aug 2026 11:14:49 -0700 Subject: [PATCH] fix(discord): surface inbound attachment download failures (#120269) * fix(discord): surface inbound attachment download failures * fix(discord): carry the media-unavailable notice into the agent text --- .../monitor/message-handler.context.test.ts | 64 +++++++++++++++++++ .../src/monitor/message-handler.context.ts | 20 +++++- .../discord/src/monitor/message-media.ts | 14 +++- 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/extensions/discord/src/monitor/message-handler.context.test.ts b/extensions/discord/src/monitor/message-handler.context.test.ts index 8bac40bd7982..b7a180a9b1fe 100644 --- a/extensions/discord/src/monitor/message-handler.context.test.ts +++ b/extensions/discord/src/monitor/message-handler.context.test.ts @@ -174,6 +174,70 @@ describe("discord buildDiscordMessageProcessContext sender bot status", () => { expect(result.ctxPayload.InboundHistory).toHaveLength(2); }); + it("records an unavailable-attachment notice for path-less media facts", async () => { + // Failed downloads produce path-less facts that core drops from the media + // projection; the body notice is the model's only record of the attachment. + const ctx = await createBaseDiscordMessageContext(); + + const result = await buildDiscordMessageProcessContext({ + ctx, + text: "look at this", + mediaList: [ + { contentType: "image/png", kind: "image" }, + { path: "/tmp/ok.png", contentType: "image/png", kind: "image" }, + ], + }); + if (!result) { + throw new Error("expected a built Discord message context"); + } + + expect(result.ctxPayload.Body).toContain("look at this"); + expect(result.ctxPayload.Body).toContain("[discord attachment unavailable]"); + // BodyForAgent is what the model reads; Body alone would leave it silent. + // It derives from the raw message text (harness baseText), not the envelope. + expect(result.ctxPayload.BodyForAgent).toContain("hi"); + expect(result.ctxPayload.BodyForAgent).toContain("[discord attachment unavailable]"); + }); + + it("keeps audio-transcript precedence in the agent text when media fails", async () => { + const ctx = await createBaseDiscordMessageContext({ + preflightAudioTranscript: "spoken words", + }); + + const result = await buildDiscordMessageProcessContext({ + ctx, + text: "look at this", + mediaList: [{ contentType: "image/png", kind: "image" }], + }); + if (!result) { + throw new Error("expected a built Discord message context"); + } + + expect(result.ctxPayload.BodyForAgent).toContain("spoken words"); + expect(result.ctxPayload.BodyForAgent).toContain("[discord attachment unavailable]"); + }); + + it("pluralizes the unavailable notice and skips it when all media resolved", async () => { + const ctx = await createBaseDiscordMessageContext(); + + const failedTwice = await buildDiscordMessageProcessContext({ + ctx, + text: "two broken", + mediaList: [ + { contentType: "image/png", kind: "image" }, + { contentType: "video/mp4", kind: "video" }, + ], + }); + expect(failedTwice?.ctxPayload.Body).toContain("[discord 2 attachments unavailable]"); + + const allResolved = await buildDiscordMessageProcessContext({ + ctx: await createBaseDiscordMessageContext(), + text: "fine", + mediaList: [{ path: "/tmp/ok.png", contentType: "image/png", kind: "image" }], + }); + expect(allResolved?.ctxPayload.Body).not.toContain("unavailable"); + }); + it("does not inject stale pending history when history is disabled", async () => { const guildHistories = new Map([ ["c1", [historyEntry({ id: "stale", senderId: "111", sender: "Alice", body: "stale body" })]], diff --git a/extensions/discord/src/monitor/message-handler.context.ts b/extensions/discord/src/monitor/message-handler.context.ts index c458aea00329..35cded25aa95 100644 --- a/extensions/discord/src/monitor/message-handler.context.ts +++ b/extensions/discord/src/monitor/message-handler.context.ts @@ -2,6 +2,7 @@ import { buildChannelInboundEventContext, formatInboundEnvelope, + formatInboundMediaUnavailableText, resolveEnvelopeFormatOptions, toHistoryMediaEntries, toInboundMediaFactsWithMetadata, @@ -167,11 +168,24 @@ export async function buildDiscordMessageProcessContext(params: { }); const channelHistory = createChannelHistoryWindow({ historyMap: guildHistories }); let visibleChannelHistory: DiscordHistoryEntry[] | undefined; + // Failed downloads (CDN error, SSRF block, size cap, timeout) produce + // path-less facts that core drops from the media projection. Record the + // outcome in the body like sibling channels so the turn never silently + // ignores an attachment the user sent. + const unavailableMediaCount = mediaList.filter((media) => !media.path).length; + const appendMediaUnavailableNotice = (body: string | undefined) => + unavailableMediaCount > 0 + ? formatInboundMediaUnavailableText({ + body, + notice: `[discord ${unavailableMediaCount > 1 ? `${unavailableMediaCount} attachments` : "attachment"} unavailable]`, + }) + : body; + const bodyWithMediaNotice = appendMediaUnavailableNotice(text) ?? text; let combinedBody = formatInboundEnvelope({ channel: "Discord", from: fromLabel, timestamp: resolveTimestampMs(message.timestamp), - body: text, + body: bodyWithMediaNotice, chatType: isDirectMessage ? "direct" : "channel", senderLabel, previousTimestamp, @@ -392,7 +406,9 @@ export async function buildDiscordMessageProcessContext(params: { inboundEventKind: ctx.inboundEventKind, body: combinedBody, rawBody: preflightAudioTranscript ?? baseText, - bodyForAgent: preflightAudioTranscript ?? baseText ?? text, + // BodyForAgent wins over Body for the model's text, so the notice has to + // ride the agent-facing source too — keeping transcript precedence. + bodyForAgent: appendMediaUnavailableNotice(preflightAudioTranscript ?? baseText ?? text), commandBody: preflightAudioTranscript ?? baseText, inboundHistory, }, diff --git a/extensions/discord/src/monitor/message-media.ts b/extensions/discord/src/monitor/message-media.ts index 4d6c8a83871b..2ca4c480147c 100644 --- a/extensions/discord/src/monitor/message-media.ts +++ b/extensions/discord/src/monitor/message-media.ts @@ -6,7 +6,7 @@ import { } from "openclaw/plugin-sdk/channel-inbound"; import { getFileExtension, normalizeMimeType } from "openclaw/plugin-sdk/media-mime"; import { saveRemoteMedia, type FetchLike } from "openclaw/plugin-sdk/media-runtime"; -import { logVerbose } from "openclaw/plugin-sdk/runtime-env"; +import { getChildLogger, logVerbose } from "openclaw/plugin-sdk/runtime-env"; import type { SsrFPolicy } from "openclaw/plugin-sdk/ssrf-runtime"; import { normalizeLowercaseStringOrEmpty, @@ -401,7 +401,12 @@ async function appendResolvedMediaFromAttachments(params: { }); } catch (err) { const id = attachment.id ?? attachmentUrl; - logVerbose(`${params.errorPrefix} ${id}: ${String(err)}`); + // Warn on the default path: the failed download becomes a path-less fact + // that core drops from the media projection, so this log plus the body + // notice are the only records of the missing attachment. + getChildLogger({ module: "discord-media" }).warn( + `${params.errorPrefix} ${id}: ${String(err)}`, + ); const classification = resolveDiscordMediaClassification({ attachment }); params.out.push({ ...classification, @@ -506,7 +511,10 @@ async function appendResolvedMediaFromStickers(params: { } } if (lastError) { - logVerbose(`${params.errorPrefix} ${sticker.id}: ${formatStickerError(lastError)}`); + // Same visibility contract as failed attachments: path-less fact + warn. + getChildLogger({ module: "discord-media" }).warn( + `${params.errorPrefix} ${sticker.id}: ${formatStickerError(lastError)}`, + ); const fallback = candidates[0]; if (fallback) { params.out.push({