From adf4eb487b367e9765820a07162d5aaef363fd3e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 23:35:55 +0000 Subject: [PATCH] fix(signal): forward all inbound attachments from #39212 (thanks @joeykrug) Co-authored-by: Joey Krug --- CHANGELOG.md | 1 + .../event-handler.inbound-contract.test.ts | 33 +++++++ .../event-handler.mention-gating.test.ts | 28 ++++++ src/signal/monitor/event-handler.ts | 96 ++++++++++++++----- 4 files changed, 136 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 533e75d7297..60a4a8748bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -304,6 +304,7 @@ Docs: https://docs.openclaw.ai - Control UI/auth error reporting: map generic browser `Fetch failed` websocket close errors back to actionable gateway auth messages (`gateway token mismatch`, `authentication failed`, `retry later`) so dashboard disconnects stop hiding credential problems. Landed from contributor PR #28608 by @KimGLee. Thanks @KimGLee. - Media/mime unknown-kind handling: return `undefined` (not `"unknown"`) for missing/unrecognized MIME kinds and use document-size fallback caps for unknown remote media, preventing phantom `` Signal events from being treated as real messages. (#39199) Thanks @nicolasgrasset. - Nodes/system.run allow-always persistence: honor shell comment semantics during allowlist analysis so `#`-tailed payloads that never execute are not persisted as trusted follow-up commands. Thanks @tdjackey for reporting. +- Signal/inbound attachment fan-in: forward all successfully fetched inbound attachments through `MediaPaths`/`MediaUrls`/`MediaTypes` (instead of only the first), and improve multi-attachment placeholder summaries in mention-gated pending history. (#39212) Thanks @joeykrug. ## 2026.3.2 diff --git a/src/signal/monitor/event-handler.inbound-contract.test.ts b/src/signal/monitor/event-handler.inbound-contract.test.ts index 84075523655..88be22ea5b4 100644 --- a/src/signal/monitor/event-handler.inbound-contract.test.ts +++ b/src/signal/monitor/event-handler.inbound-contract.test.ts @@ -173,6 +173,39 @@ describe("signal createSignalEventHandler inbound contract", () => { expect(capture.ctx?.CommandAuthorized).toBe(false); }); + it("forwards all fetched attachments via MediaPaths/MediaTypes", async () => { + const handler = createSignalEventHandler( + createBaseSignalEventHandlerDeps({ + cfg: { + messages: { inbound: { debounceMs: 0 } }, + channels: { signal: { dmPolicy: "open", allowFrom: ["*"] } }, + }, + ignoreAttachments: false, + fetchAttachment: async ({ attachment }) => ({ + path: `/tmp/${String(attachment.id)}.dat`, + contentType: attachment.id === "a1" ? "image/jpeg" : undefined, + }), + historyLimit: 0, + }), + ); + + await handler( + createSignalReceiveEvent({ + dataMessage: { + message: "", + attachments: [{ id: "a1", contentType: "image/jpeg" }, { id: "a2" }], + }, + }), + ); + + expect(capture.ctx).toBeTruthy(); + expect(capture.ctx?.MediaPath).toBe("/tmp/a1.dat"); + expect(capture.ctx?.MediaType).toBe("image/jpeg"); + expect(capture.ctx?.MediaPaths).toEqual(["/tmp/a1.dat", "/tmp/a2.dat"]); + expect(capture.ctx?.MediaUrls).toEqual(["/tmp/a1.dat", "/tmp/a2.dat"]); + expect(capture.ctx?.MediaTypes).toEqual(["image/jpeg", "application/octet-stream"]); + }); + it("drops own UUID inbound messages when only accountUuid is configured", async () => { const ownUuid = "123e4567-e89b-12d3-a456-426614174000"; const handler = createSignalEventHandler( diff --git a/src/signal/monitor/event-handler.mention-gating.test.ts b/src/signal/monitor/event-handler.mention-gating.test.ts index 403f36c1ab8..38dedf5a813 100644 --- a/src/signal/monitor/event-handler.mention-gating.test.ts +++ b/src/signal/monitor/event-handler.mention-gating.test.ts @@ -171,6 +171,34 @@ describe("signal mention gating", () => { expect(entries[0].body).toBe(""); }); + it("summarizes multiple skipped attachments with stable file count wording", async () => { + capturedCtx = undefined; + const groupHistories = new Map(); + const handler = createSignalEventHandler( + createBaseSignalEventHandlerDeps({ + cfg: createSignalConfig({ requireMention: true }), + historyLimit: 5, + groupHistories, + ignoreAttachments: false, + fetchAttachment: async ({ attachment }) => ({ + path: `/tmp/${String(attachment.id)}.bin`, + }), + }), + ); + + await handler( + makeGroupEvent({ + message: "", + attachments: [{ id: "a1" }, { id: "a2" }], + }), + ); + + expect(capturedCtx).toBeUndefined(); + const entries = groupHistories.get("g1"); + expect(entries).toHaveLength(1); + expect(entries[0].body).toBe("[2 files attached]"); + }); + it("records quote text in pending history for skipped quote-only group messages", async () => { await expectSkippedGroupHistory({ message: "", quoteText: "quoted context" }, "quoted context"); }); diff --git a/src/signal/monitor/event-handler.ts b/src/signal/monitor/event-handler.ts index 7369a166add..abba2d0778e 100644 --- a/src/signal/monitor/event-handler.ts +++ b/src/signal/monitor/event-handler.ts @@ -56,6 +56,26 @@ import type { SignalReceivePayload, } from "./event-handler.types.js"; import { renderSignalMentions } from "./mentions.js"; + +function formatAttachmentKindCount(kind: string, count: number): string { + if (kind === "attachment") { + return `${count} file${count > 1 ? "s" : ""}`; + } + return `${count} ${kind}${count > 1 ? "s" : ""}`; +} + +function formatAttachmentSummaryPlaceholder(contentTypes: Array): string { + const kindCounts = new Map(); + for (const contentType of contentTypes) { + const kind = kindFromMime(contentType) ?? "attachment"; + kindCounts.set(kind, (kindCounts.get(kind) ?? 0) + 1); + } + const parts = [...kindCounts.entries()].map(([kind, count]) => + formatAttachmentKindCount(kind, count), + ); + return `[${parts.join(" + ")} attached]`; +} + export function createSignalEventHandler(deps: SignalEventHandlerDeps) { type SignalInboundEntry = { senderName: string; @@ -71,6 +91,8 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { messageId?: string; mediaPath?: string; mediaType?: string; + mediaPaths?: string[]; + mediaTypes?: string[]; commandAuthorized: boolean; wasMentioned?: boolean; }; @@ -170,6 +192,9 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { MediaPath: entry.mediaPath, MediaType: entry.mediaType, MediaUrl: entry.mediaPath, + MediaPaths: entry.mediaPaths, + MediaUrls: entry.mediaPaths, + MediaTypes: entry.mediaTypes, WasMentioned: entry.isGroup ? entry.wasMentioned === true : undefined, CommandAuthorized: entry.commandAuthorized, OriginatingChannel: "signal" as const, @@ -311,7 +336,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { return shouldDebounceTextInbound({ text: entry.bodyText, cfg: deps.cfg, - hasMedia: Boolean(entry.mediaPath || entry.mediaType), + hasMedia: Boolean(entry.mediaPath || entry.mediaType || entry.mediaPaths?.length), }); }, onFlush: async (entries) => { @@ -335,6 +360,8 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { bodyText: combinedText, mediaPath: undefined, mediaType: undefined, + mediaPaths: undefined, + mediaTypes: undefined, }); }, onError: (err) => { @@ -632,6 +659,12 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { if (deps.ignoreAttachments) { return ""; } + const attachmentTypes = (dataMessage.attachments ?? []).map((attachment) => + typeof attachment?.contentType === "string" ? attachment.contentType : undefined, + ); + if (attachmentTypes.length > 1) { + return formatAttachmentSummaryPlaceholder(attachmentTypes); + } const firstContentType = dataMessage.attachments?.[0]?.contentType; const pendingKind = kindFromMime(firstContentType ?? undefined); return pendingKind ? `` : ""; @@ -655,32 +688,49 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { let mediaPath: string | undefined; let mediaType: string | undefined; + const mediaPaths: string[] = []; + const mediaTypes: string[] = []; let placeholder = ""; - const firstAttachment = dataMessage.attachments?.[0]; - if (firstAttachment?.id && !deps.ignoreAttachments) { - try { - const fetched = await deps.fetchAttachment({ - baseUrl: deps.baseUrl, - account: deps.account, - attachment: firstAttachment, - sender: senderRecipient, - groupId, - maxBytes: deps.mediaMaxBytes, - }); - if (fetched) { - mediaPath = fetched.path; - mediaType = fetched.contentType ?? firstAttachment.contentType ?? undefined; + const attachments = dataMessage.attachments ?? []; + if (!deps.ignoreAttachments) { + for (const attachment of attachments) { + if (!attachment?.id) { + continue; + } + try { + const fetched = await deps.fetchAttachment({ + baseUrl: deps.baseUrl, + account: deps.account, + attachment, + sender: senderRecipient, + groupId, + maxBytes: deps.mediaMaxBytes, + }); + if (fetched) { + mediaPaths.push(fetched.path); + mediaTypes.push( + fetched.contentType ?? attachment.contentType ?? "application/octet-stream", + ); + if (!mediaPath) { + mediaPath = fetched.path; + mediaType = fetched.contentType ?? attachment.contentType ?? undefined; + } + } + } catch (err) { + deps.runtime.error?.(danger(`attachment fetch failed: ${String(err)}`)); } - } catch (err) { - deps.runtime.error?.(danger(`attachment fetch failed: ${String(err)}`)); } } - const kind = kindFromMime(mediaType ?? undefined); - if (kind) { - placeholder = ``; - } else if (dataMessage.attachments?.length) { - placeholder = ""; + if (mediaPaths.length > 1) { + placeholder = formatAttachmentSummaryPlaceholder(mediaTypes); + } else { + const kind = kindFromMime(mediaType ?? undefined); + if (kind) { + placeholder = ``; + } else if (attachments.length) { + placeholder = ""; + } } const bodyText = messageText || placeholder || dataMessage.quote?.text?.trim() || ""; @@ -730,6 +780,8 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { messageId, mediaPath, mediaType, + mediaPaths: mediaPaths.length > 0 ? mediaPaths : undefined, + mediaTypes: mediaTypes.length > 0 ? mediaTypes : undefined, commandAuthorized, wasMentioned: effectiveWasMentioned, });