From 1a18981a84df893d41527df99f6429baca84c13d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 26 Aug 2026 22:48:07 -0700 Subject: [PATCH] fix: preserve explicit MIME types on attachment actions (#130714) Resolve attachment MIME aliases once and carry normalized metadata through send preservation and structured hydration. Remove stale nested-MIME publication while preserving filenames, sniffing and staging. Closes #130711. --- docs/nodes/images.md | 9 ++ .../message-action-params.attachments.test.ts | 120 +++++++++++------- .../outbound/message-action-params.test.ts | 52 ++++++-- src/infra/outbound/message-action-params.ts | 21 ++- .../outbound/message-action-send.test.ts | 22 ++-- 5 files changed, 145 insertions(+), 79 deletions(-) diff --git a/docs/nodes/images.md b/docs/nodes/images.md index d32a4fcff9f7..f16646e0ffa2 100644 --- a/docs/nodes/images.md +++ b/docs/nodes/images.md @@ -28,6 +28,15 @@ portable formats, byte limits, and lazy transcoding, see - `--dry-run` — print the resolved payload and skip sending. - `--json` — print the result as JSON: `{ action, channel, dryRun, handledBy, messageId?, payload }` (`payload` carries the channel-specific send result, including any media reference). +## Message tool attachment metadata + +For `buffer` attachments, `contentType` takes precedence over `mimeType`; a data URL's +MIME type is used only when neither is supplied. For `reply`, `sendAttachment`, +`upload-file`, and `setGroupIcon`, top-level MIME metadata also takes precedence over +the selected `attachments[]` entry. Hydration carries that choice as `contentType` +and uses it to infer a missing filename. Explicit filenames are preserved. This +metadata precedence does not change MIME detection when media bytes are loaded or staged. + ## WhatsApp Web channel behavior - Input: local file path **or** HTTP(S) URL. diff --git a/src/infra/outbound/message-action-params.attachments.test.ts b/src/infra/outbound/message-action-params.attachments.test.ts index 264926efe193..083752246bf7 100644 --- a/src/infra/outbound/message-action-params.attachments.test.ts +++ b/src/infra/outbound/message-action-params.attachments.test.ts @@ -150,27 +150,36 @@ describe("runMessageAction media behavior", () => { }, ); - it.each(["send", "sendAttachment", "reply", "upload-file", "setGroupIcon"] as const)( - "keeps explicit content type authoritative for %s data URLs", - async (action) => { - const args: Record = { - buffer: parameterizedPngDataUrl, - contentType: "image/jpeg", - }; + it.each( + (["send", "sendAttachment", "reply", "upload-file", "setGroupIcon"] as const).flatMap( + (action) => [ + { action, name: "contentType", metadata: { contentType: "image/jpeg" } }, + { action, name: "mimeType", metadata: { mimeType: "image/jpeg" } }, + { + action, + name: "both aliases", + metadata: { contentType: "image/jpeg", mimeType: "image/webp" }, + }, + ], + ), + )("keeps $name authoritative for $action data URLs", async ({ action, metadata }) => { + const args: Record = { + buffer: parameterizedPngDataUrl, + ...metadata, + }; - await hydrateAttachmentParamsForAction({ - cfg: {}, - channel: "imessage", - args, - action, - dryRun: true, - mediaPolicy: { mode: "host" }, - }); + await hydrateAttachmentParamsForAction({ + cfg: {}, + channel: "imessage", + args, + action, + dryRun: true, + mediaPolicy: { mode: "host" }, + }); - expect(args.contentType).toBe("image/jpeg"); - expect(args.filename).toBe("attachment.jpg"); - }, - ); + expect(args.contentType).toBe("image/jpeg"); + expect(args.filename).toBe("attachment.jpg"); + }); it.each([ ["duplicate marker", "image/png;base64;base64"], @@ -641,33 +650,56 @@ describe("runMessageAction media behavior", () => { expect(canonicalizeBase64(String(handlerParams.buffer))).toBe(onePixelPngBase64); }); - it("hydrates buffer and metadata from attachments[] before the reply handler runs", async () => { - const result = await runMessageAction({ - cfg, - action: "reply", - params: { - channel: "replychat", - target: "+15551234567", - messageId: "parent-id", - text: "look at this", - attachments: [ - { - url: "https://example.com/pic.png", - name: "reply.png", - mimeType: "image/png", - }, - ], + it.each([ + { name: "nested MIME", metadata: {}, contentType: "image/png" }, + { + name: "explicit MIME alias", + metadata: { mimeType: "text/plain" }, + contentType: "text/plain", + }, + { + name: "contentType before mimeType", + metadata: { + contentType: "text/plain", + mimeType: "application/json", + filename: "explicit.txt", }, - }); + contentType: "text/plain", + }, + ])( + "passes $name from attachments[] to the reply handler", + async ({ metadata, contentType }) => { + const result = await runMessageAction({ + cfg, + action: "reply", + params: { + channel: "replychat", + target: "+15551234567", + messageId: "parent-id", + text: "look at this", + ...metadata, + attachments: [ + { + url: "https://example.com/pic.png", + name: "reply.png", + mimeType: "image/png", + }, + ], + }, + }); - expect(result.kind).toBe("action"); - expect(loadWebMedia).toHaveBeenCalledWith("https://example.com/pic.png", expect.any(Object)); - expect(handleActionMock).toHaveBeenCalledTimes(1); - const handlerParams = firstMockArg(handleActionMock, "handleAction"); - expect(handlerParams.buffer).toBe(Buffer.from("hello").toString("base64")); - expect(handlerParams.filename).toBe("reply.png"); - expect(handlerParams.contentType).toBe("image/png"); - }); + expect(result.kind).toBe("action"); + expect(loadWebMedia).toHaveBeenCalledWith( + "https://example.com/pic.png", + expect.any(Object), + ); + expect(handleActionMock).toHaveBeenCalledTimes(1); + const handlerParams = firstMockArg(handleActionMock, "handleAction"); + expect(handlerParams.buffer).toBe(Buffer.from("hello").toString("base64")); + expect(handlerParams.filename).toBe(metadata.filename ?? "reply.png"); + expect(handlerParams.contentType).toBe(contentType); + }, + ); it("does not copy metadata from attachments[] when top-level media wins", async () => { await runMessageAction({ diff --git a/src/infra/outbound/message-action-params.test.ts b/src/infra/outbound/message-action-params.test.ts index c40469265567..a0a038aa83ce 100644 --- a/src/infra/outbound/message-action-params.test.ts +++ b/src/infra/outbound/message-action-params.test.ts @@ -635,13 +635,26 @@ describe("message action media helpers", () => { expect(args.filename).toBe("cute.png"); }); - it("hydrates reply attachments from the first structured attachment source", async () => { + it.each([ + { name: "nested MIME", metadata: {}, contentType: "application/octet-stream" }, + { + name: "explicit MIME alias", + metadata: { mimeType: "text/plain" }, + contentType: "text/plain", + }, + { + name: "contentType before mimeType", + metadata: { contentType: "text/plain", mimeType: "application/json" }, + contentType: "text/plain", + }, + ])("hydrates structured reply attachments with $name", async ({ metadata, contentType }) => { const args: Record = { + ...metadata, attachments: [ { - url: "https://example.com/cute.png", - mimeType: "image/png", - name: "cute.png", + media: "https://example.invalid/note", + mimeType: "application/octet-stream", + name: "note.txt", }, ], }; @@ -655,8 +668,9 @@ describe("message action media helpers", () => { mediaPolicy: { mode: "host" }, }); - expect(args.filename).toBe("cute.png"); - expect(args.contentType).toBe("image/png"); + expect(args.filename).toBe("note.txt"); + expect(args.contentType).toBe(contentType); + expect(args.buffer).toBeUndefined(); }); it("does not hydrate ignored structured attachments when plugin media params win", async () => { @@ -707,12 +721,12 @@ describe("message action media helpers", () => { expect(args.caption).toBeUndefined(); }); - it("hydrates buffer-only send params into outbound media paths", async () => { + it.each(["contentType", "mimeType"])("stages buffer-only sends with %s metadata", async (key) => { await withTempOpenClawStateDir(async () => { const args: Record = { buffer: Buffer.from("artifact bytes").toString("base64"), filename: "artifact.txt", - contentType: "text/plain", + [key]: "text/plain", }; await hydrateAttachmentParamsForAction({ @@ -727,6 +741,8 @@ describe("message action media helpers", () => { expect(args.mediaUrl).toBe(args.media); expect(args.mediaUrls).toEqual([args.media]); expect(args.buffer).toBeUndefined(); + expect(args.contentType).toBe("text/plain"); + expect(args.filename).toBe("artifact.txt"); await expect(fs.readFile(String(args.media), "utf8")).resolves.toBe("artifact bytes"); }); }); @@ -806,27 +822,35 @@ describe("message action media helpers", () => { }); }); - it("previews dry-run buffer-only sends without writing outbound media files", async () => { + it.each( + ["dry-run", "preserve-buffer"].flatMap((mode) => [ + { mode, buffer: "SGVsbG8=", name: "raw base64" }, + { mode, buffer: "data:application/octet-stream;base64,SGVsbG8=", name: "data URL" }, + ]), + )("keeps explicit MIME for $mode $name without staging", async ({ mode, buffer }) => { await withTempOpenClawStateDir(async (stateDir) => { const args: Record = { - buffer: Buffer.from("preview").toString("base64"), + buffer, filename: "preview.txt", - contentType: "text/plain", + mimeType: "text/plain", }; await hydrateAttachmentParamsForAction({ cfg, - channel: "workspace", + channel: "imessage", args, action: "send", - dryRun: true, + dryRun: mode === "dry-run", + preserveSendBuffer: mode === "preserve-buffer", mediaPolicy: { mode: "host" }, }); expect(args.media).toBe("buffer://message-send/attachment"); expect(args.mediaUrl).toBe("buffer://message-send/attachment"); expect(args.mediaUrls).toEqual(["buffer://message-send/attachment"]); - expect(args.buffer).toBeUndefined(); + expect(args.buffer).toBe(mode === "preserve-buffer" ? buffer : undefined); + expect(args.contentType).toBe("text/plain"); + expect(args.filename).toBe("preview.txt"); await expect(fs.readdir(path.join(stateDir, "media", "outbound"))).rejects.toThrow(); }); }); diff --git a/src/infra/outbound/message-action-params.ts b/src/infra/outbound/message-action-params.ts index e19135cf293e..1baf5d0b8202 100644 --- a/src/infra/outbound/message-action-params.ts +++ b/src/infra/outbound/message-action-params.ts @@ -335,19 +335,17 @@ async function hydrateSendBufferMediaParams(params: { } const normalized = normalizeBase64Payload({ base64: rawBuffer, - contentType: readToolStringParam(params.args, "contentType") ?? undefined, + contentType: + readToolStringParam(params.args, "contentType") ?? + readToolStringParam(params.args, "mimeType"), }); if (!normalized.base64) { return; } - const contentType = - readToolStringParam(params.args, "contentType") ?? - readToolStringParam(params.args, "mimeType") ?? - normalized.contentType; const filename = readToolStringParam(params.args, "filename") ?? inferAttachmentFilename({ - contentType: contentType ?? undefined, + contentType: normalized.contentType, }); const maxBytes = resolveSendBufferMaxBytes(params); if (params.dryRun || params.preserveBuffer) { @@ -376,7 +374,7 @@ async function hydrateSendBufferMediaParams(params: { }), maxBytes, { - contentType: contentType ?? undefined, + contentType: normalized.contentType, filename, }, ); @@ -503,9 +501,9 @@ async function hydrateAttachmentPayload(params: { }); if (normalized.base64 !== rawBuffer && normalized.base64) { params.args.buffer = normalized.base64; - if (normalized.contentType && !contentTypeParam) { - params.args.contentType = normalized.contentType; - } + } + if (normalized.contentType && !readToolStringParam(params.args, "contentType")) { + params.args.contentType = normalized.contentType; } const filename = readToolStringParam(params.args, "filename"); @@ -648,9 +646,6 @@ async function hydrateAttachmentActionPayload(params: { if (attachmentSource?.filename && !readToolStringParam(params.args, "filename")) { params.args.filename = attachmentSource.filename; } - if (attachmentSource?.contentType && !readToolStringParam(params.args, "contentType")) { - params.args.contentType = attachmentSource.contentType; - } if (params.allowMessageCaptionFallback) { const caption = readToolStringParam(params.args, "caption", { allowEmpty: true })?.trim(); diff --git a/src/infra/outbound/message-action-send.test.ts b/src/infra/outbound/message-action-send.test.ts index a435dc251828..89c4d7a7c0f9 100644 --- a/src/infra/outbound/message-action-send.test.ts +++ b/src/infra/outbound/message-action-send.test.ts @@ -70,7 +70,10 @@ describe("runMessageAction plugin dispatch", () => { vi.clearAllMocks(); vi.unstubAllEnvs(); }); - it("preserves buffer-only send bytes for gateway-side materialization", async () => { + it.each([ + { name: "raw base64", buffer: "SGVsbG8=" }, + { name: "data URL", buffer: "data:application/octet-stream;base64,SGVsbG8=" }, + ])("preserves $name bytes and MIME for gateway-side materialization", async ({ buffer }) => { const gatewayPlugin = createGatewayActionPlugin({ pluginId: "gatewaychat", label: "Gateway Chat", @@ -101,9 +104,9 @@ describe("runMessageAction plugin dispatch", () => { params: { channel: "gatewaychat", target: "user-123", - buffer: Buffer.from("gateway bytes").toString("base64"), + buffer, filename: "gateway.txt", - contentType: "text/plain", + mimeType: "text/plain", }, gateway: { clientName: "cli", @@ -123,7 +126,7 @@ describe("runMessageAction plugin dispatch", () => { media: "buffer://message-send/attachment", mediaUrl: "buffer://message-send/attachment", mediaUrls: ["buffer://message-send/attachment"], - buffer: Buffer.from("gateway bytes").toString("base64"), + buffer, filename: "gateway.txt", contentType: "text/plain", }, @@ -132,7 +135,10 @@ describe("runMessageAction plugin dispatch", () => { expect(mocks.executeSendAction).not.toHaveBeenCalled(); }); - it("preserves buffer-only send bytes for gateway delivery-mode channels", async () => { + it.each([ + { name: "raw base64", buffer: "SGVsbG8=" }, + { name: "data URL", buffer: "data:application/octet-stream;base64,SGVsbG8=" }, + ])("preserves $name bytes and MIME for gateway delivery-mode channels", async ({ buffer }) => { const gatewayDeliveryPlugin: ChannelPlugin = { id: "gatewaydeliver", meta: { @@ -175,9 +181,9 @@ describe("runMessageAction plugin dispatch", () => { params: { channel: "gatewaydeliver", target: "user-123", - buffer: Buffer.from("gateway delivery bytes").toString("base64"), + buffer, filename: "delivery.txt", - contentType: "text/plain", + mimeType: "text/plain", }, gateway: { clientName: "cli", @@ -191,7 +197,7 @@ describe("runMessageAction plugin dispatch", () => { { mediaUrl: "buffer://message-send/attachment", mediaUrls: ["buffer://message-send/attachment"], - buffer: Buffer.from("gateway delivery bytes").toString("base64"), + buffer, filename: "delivery.txt", contentType: "text/plain", },