diff --git a/extensions/clickclack/src/outbound.test.ts b/extensions/clickclack/src/outbound.test.ts index c9e4cf662cee..9598cd0e4520 100644 --- a/extensions/clickclack/src/outbound.test.ts +++ b/extensions/clickclack/src/outbound.test.ts @@ -658,6 +658,32 @@ describe("reconcileClickClackUnknownSend", () => { expect(loadOutboundMediaFromUrl).not.toHaveBeenCalled(); }); + it("recovers one attachment when legacy and plural URLs describe the same upload", async () => { + findUploadByNonce.mockResolvedValueOnce({ id: "upl_existing", filename: "proof.ts" }); + findMessageByNonce.mockResolvedValueOnce({ + id: "msg_existing", + attachments: [{ id: "upl_existing" }], + }); + const mediaUrl = "/workspace/proof.ts"; + + const result = await reconcileClickClackUnknownSend({ + cfg, + queueId: "queue-media", + channel: "clickclack", + to: "channel:general", + enqueuedAt: 1, + retryCount: 0, + payloads: [{ text: "proof", mediaUrl, mediaUrls: [mediaUrl] }], + }); + + expect(result.status).toBe("sent"); + expect(findUploadByNonce).toHaveBeenCalledOnce(); + expect(findMessageByNonce).toHaveBeenCalledOnce(); + if (result.status === "sent") { + expect(result.receipt.platformMessageIds).toEqual(["msg_existing"]); + } + }); + it("replays normally when uploads exist but messages do not", async () => { findUploadByNonce .mockResolvedValueOnce({ id: "upl_first", filename: "first.png" }) diff --git a/extensions/clickclack/src/outbound.ts b/extensions/clickclack/src/outbound.ts index 65d1b7e8ae05..9149c77edd66 100644 --- a/extensions/clickclack/src/outbound.ts +++ b/extensions/clickclack/src/outbound.ts @@ -13,6 +13,7 @@ import { loadOutboundMediaFromUrl, type OutboundMediaLoadOptions, } from "openclaw/plugin-sdk/outbound-media"; +import { resolveSendableOutboundReplyParts } from "openclaw/plugin-sdk/reply-payload"; import { FormatCapabilityProfile, renderMarkdownWithMarkers, @@ -305,9 +306,7 @@ function collectReconciliationMediaUrls(ctx: ChannelMessageUnknownSendContext): return planned.map((url) => url.trim()).filter(Boolean); } const payload = ctx.payloads[0]; - return [payload?.mediaUrl, ...(payload?.mediaUrls ?? [])] - .map((url) => url?.trim()) - .filter((url): url is string => Boolean(url)); + return payload ? resolveSendableOutboundReplyParts(payload).mediaUrls : []; } /** diff --git a/src/infra/outbound/reply-payload-parts.ts b/src/auto-reply/reply-payload-parts.ts similarity index 95% rename from src/infra/outbound/reply-payload-parts.ts rename to src/auto-reply/reply-payload-parts.ts index 239f11dbefb0..d56c3463aab4 100644 --- a/src/infra/outbound/reply-payload-parts.ts +++ b/src/auto-reply/reply-payload-parts.ts @@ -1,4 +1,4 @@ -import { normalizeStringEntries } from "../../../packages/normalization-core/src/string-normalization.js"; +import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization"; /** Derived sendability facts for text/media outbound payload delivery. */ export type SendableOutboundReplyParts = { diff --git a/src/channels/message/rendered-batch.test.ts b/src/channels/message/rendered-batch.test.ts index 626bde144be3..1a5ee16c1d16 100644 --- a/src/channels/message/rendered-batch.test.ts +++ b/src/channels/message/rendered-batch.test.ts @@ -2,6 +2,32 @@ import { describe, expect, it } from "vitest"; import { createRenderedMessageBatchPlan } from "./rendered-batch.js"; describe("createRenderedMessageBatchPlan", () => { + it.each([ + { + name: "matching legacy and plural attachments", + payload: { mediaUrl: " /tmp/image.png ", mediaUrls: [" /tmp/image.png "] }, + expected: ["/tmp/image.png"], + }, + { + name: "plural attachments superseding a legacy attachment", + payload: { + mediaUrl: "/tmp/obsolete.png", + mediaUrls: [" /tmp/first.png ", "", "/tmp/second.png"], + }, + expected: ["/tmp/first.png", "/tmp/second.png"], + }, + { + name: "an empty plural attachment list", + payload: { mediaUrl: " /tmp/image.png ", mediaUrls: [] }, + expected: ["/tmp/image.png"], + }, + ])("uses canonical media precedence for $name", ({ payload, expected }) => { + const plan = createRenderedMessageBatchPlan([payload]); + + expect(plan.mediaCount).toBe(expected.length); + expect(plan.items[0]?.mediaUrls).toEqual(expected); + }); + it("keeps aggregate media counts aligned with normalized media items", () => { const plan = createRenderedMessageBatchPlan([ { diff --git a/src/channels/message/rendered-batch.ts b/src/channels/message/rendered-batch.ts index a5fbecf9f57e..cc562969b045 100644 --- a/src/channels/message/rendered-batch.ts +++ b/src/channels/message/rendered-batch.ts @@ -3,6 +3,7 @@ * * Summarizes reply payloads so delivery can pick adapter paths and recovery metadata. */ +import { resolveSendableOutboundReplyParts } from "../../auto-reply/reply-payload-parts.js"; import type { ReplyPayload } from "../../auto-reply/reply-payload.js"; import type { RenderedMessageBatch, @@ -11,22 +12,12 @@ import type { RenderedMessageBatchPlanKind, } from "./types.js"; -function countMedia(payload: ReplyPayload): number { - return collectMediaUrls(payload).length; -} - -function collectMediaUrls(payload: ReplyPayload): string[] { - return [payload.mediaUrl, ...(payload.mediaUrls ?? [])] - .map((url) => url?.trim()) - .filter((url): url is string => Boolean(url)); -} - function createRenderedMessageBatchPlanItem( payload: ReplyPayload, index: number, ): RenderedMessageBatchPlanItem { const text = payload.text?.trim(); - const mediaUrls = collectMediaUrls(payload); + const mediaUrls = resolveSendableOutboundReplyParts(payload).mediaUrls; const presentationBlockCount = payload.presentation?.blocks?.length ?? 0; const kinds: RenderedMessageBatchPlanKind[] = []; if (text) { @@ -62,9 +53,9 @@ export function createRenderedMessageBatchPlan( ): RenderedMessageBatchPlan { const items = payloads.map(createRenderedMessageBatchPlanItem); return payloads.reduce( - (plan, payload) => { + (plan, payload, index) => { const text = payload.text?.trim(); - const mediaCount = countMedia(payload); + const mediaCount = items[index]?.mediaUrls.length ?? 0; return { payloadCount: plan.payloadCount + 1, textCount: plan.textCount + (text ? 1 : 0), diff --git a/src/infra/outbound/deliver-queue.exact-reconciliation.integration.test.ts b/src/infra/outbound/deliver-queue.exact-reconciliation.integration.test.ts index 9ad174964ec5..dc50cef48cff 100644 --- a/src/infra/outbound/deliver-queue.exact-reconciliation.integration.test.ts +++ b/src/infra/outbound/deliver-queue.exact-reconciliation.integration.test.ts @@ -1,6 +1,9 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { createMessageReceiptFromOutboundResults } from "../../channels/message/receipt.js"; -import type { ChannelMessageSendTextContext } from "../../channels/message/types.js"; +import type { + ChannelMessageSendMediaContext, + ChannelMessageSendTextContext, +} from "../../channels/message/types.js"; import type { OpenClawConfig } from "../../config/config.js"; import { createEmptyPluginRegistry } from "../../plugins/registry.js"; import { resetPluginRuntimeStateForTest, setActivePluginRegistry } from "../../plugins/runtime.js"; @@ -98,4 +101,62 @@ describe("exact Matrix delivery queue reconciliation", () => { expect(sendText).toHaveBeenCalledOnce(); }, ); + + it.each(["automatic", "explicit"] as const)( + "delivers one normalized attachment with %s exact reconciliation", + async (reconciliation) => { + process.env.OPENCLAW_STATE_DIR = tmpDir; + const mediaUrl = "https://example.invalid/image.png"; + const sendMedia = vi.fn(async (ctx: ChannelMessageSendMediaContext) => { + await ctx.onPlatformSendDispatch?.(); + return { + messageId: "media-1", + receipt: createMessageReceiptFromOutboundResults({ + results: [{ channel: "matrix", messageId: "media-1" }], + kind: "media", + }), + }; + }); + + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "matrix", + source: "test", + plugin: { + ...createOutboundTestPlugin({ id: "matrix", outbound: matrixOutboundForQueueTest }), + message: { + id: "matrix", + durableFinal: { + ...(reconciliation === "automatic" + ? { automaticUnknownSendReconciliation: true } + : {}), + capabilities: { text: true, media: true, reconcileUnknownSend: true }, + reconcileUnknownSendKinds: { media: true }, + reconcileUnknownSend: async () => ({ status: "not_sent" as const }), + }, + send: { text: vi.fn(), media: sendMedia }, + }, + }, + }, + ]), + ); + + await expect( + deliverOutboundPayloads({ + cfg: {} as OpenClawConfig, + channel: "matrix", + to: "!room:example", + payloads: [{ text: "caption", mediaUrl, mediaUrls: [mediaUrl] }], + queuePolicy: "required", + ...(reconciliation === "explicit" ? { requireUnknownSendReconciliation: true } : {}), + }), + ).resolves.toMatchObject([{ messageId: "media-1" }]); + + expect(sendMedia).toHaveBeenCalledOnce(); + expect(sendMedia).toHaveBeenCalledWith( + expect.objectContaining({ mediaUrl, deliveryPartIndex: 0, deliveryPartCount: 1 }), + ); + }, + ); }); diff --git a/src/plugin-sdk/reply-payload.ts b/src/plugin-sdk/reply-payload.ts index 961d3918688b..56bb231088de 100644 --- a/src/plugin-sdk/reply-payload.ts +++ b/src/plugin-sdk/reply-payload.ts @@ -1,15 +1,15 @@ // Reply payload helpers normalize plugin reply targets, text, media, and approval metadata. import { normalizeLowercaseStringOrEmpty } from "../../packages/normalization-core/src/string-coerce.js"; -import type { ReplyPayload as InternalReplyPayload } from "../auto-reply/reply-payload.js"; -import type { ChannelOutboundAdapter } from "../channels/plugins/outbound.types.js"; -import { normalizeOutboundReplyPayloadCore as normalizeCoreOutboundReplyPayload } from "../infra/outbound/reply-payload-normalize.js"; import { countOutboundMedia, hasOutboundMedia, hasOutboundText, resolveOutboundMediaUrls, resolveSendableOutboundReplyParts, -} from "../infra/outbound/reply-payload-parts.js"; +} from "../auto-reply/reply-payload-parts.js"; +import type { ReplyPayload as InternalReplyPayload } from "../auto-reply/reply-payload.js"; +import type { ChannelOutboundAdapter } from "../channels/plugins/outbound.types.js"; +import { normalizeOutboundReplyPayloadCore as normalizeCoreOutboundReplyPayload } from "../infra/outbound/reply-payload-normalize.js"; import { createReplyToFanout } from "../infra/outbound/reply-policy.js"; import { hasReplyPayloadContent } from "../interactive/payload.js"; @@ -74,7 +74,7 @@ export type ReasoningReplyPayload = { }; /** Derived sendability facts for text/media outbound payload delivery. */ -export type { SendableOutboundReplyParts } from "../infra/outbound/reply-payload-parts.js"; +export type { SendableOutboundReplyParts } from "../auto-reply/reply-payload-parts.js"; export { countOutboundMedia, hasOutboundMedia, diff --git a/src/tts/tts-payload.ts b/src/tts/tts-payload.ts index 58c30d9bb82b..615f99aeddfe 100644 --- a/src/tts/tts-payload.ts +++ b/src/tts/tts-payload.ts @@ -1,3 +1,4 @@ +import { resolveSendableOutboundReplyParts } from "../auto-reply/reply-payload-parts.js"; import { getReplyPayloadMetadata, markReplyPayloadAsTtsSupplement, @@ -6,7 +7,6 @@ import { import { getChannelPlugin } from "../channels/plugins/registry.js"; import type { OpenClawConfig } from "../config/types.js"; import { isVerbose, logVerbose } from "../globals.js"; -import { resolveSendableOutboundReplyParts } from "../infra/outbound/reply-payload-parts.js"; import { hasReplyPayloadContent } from "../interactive/payload.js"; import { truncateUtf16Safe } from "../utils.js"; import { normalizeMessageChannel } from "../utils/message-channel-core.js";