From 85e4bb438dfe963e6032c82717ffaee41acfa049 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 18 Jul 2026 05:57:45 +0100 Subject: [PATCH] fix: generated images no longer post twice (#110366) * fix(agents): prevent duplicate generated media delivery * fix(ios): sync reasoning activity translations * fix(media): preserve malformed parent paths * fix(media): distinguish network media references --- .../media/native-image-generation.yaml | 13 +++- .../delivery-evidence.ts | 5 +- src/agents/subagent-announce-delivery.test.ts | 42 ++++++++++++ src/agents/subagent-announce-delivery.ts | 8 ++- src/auto-reply/reply/reply-payloads-dedupe.ts | 26 ++----- .../chat-send-command-replies.ts | 24 +------ .../server-restart-sentinel-agent-delivery.ts | 19 ++++-- src/gateway/server-restart-sentinel.test.ts | 36 ++++++++++ src/media/media-reference-comparison.test.ts | 67 +++++++++++++++++++ src/media/media-reference-comparison.ts | 64 ++++++++++++++++++ 10 files changed, 249 insertions(+), 55 deletions(-) create mode 100644 src/media/media-reference-comparison.test.ts create mode 100644 src/media/media-reference-comparison.ts diff --git a/qa/scenarios/media/native-image-generation.yaml b/qa/scenarios/media/native-image-generation.yaml index fd7520671333..7da28aeccbf4 100644 --- a/qa/scenarios/media/native-image-generation.yaml +++ b/qa/scenarios/media/native-image-generation.yaml @@ -8,11 +8,12 @@ scenario: - media.image-generation secondary: - tools.native-image-generation - objective: Verify image_generate appears when configured and returns a real saved media artifact. + objective: Verify image_generate appears when configured, returns a real saved media artifact, and delivers it once. successCriteria: - image_generate appears in the effective tool inventory. - Agent triggers native image_generate. - Tool output returns a saved MEDIA path and the file exists. + - The generated-media completion produces exactly one outbound channel delivery. docsRefs: - docs/tools/image-generation.md - docs/providers/openai.md @@ -85,4 +86,14 @@ flow: - assert: expr: "typeof generatedPath === 'string' && generatedPath.length > 0" message: image generation did not produce a saved media path + - call: sleep + args: + - 3000 + - set: outboundMessages + value: + expr: "state.getSnapshot().messages.filter((candidate) => candidate.direction === 'outbound' && candidate.conversation.id === 'qa-operator')" + - assert: + expr: "outboundMessages.length === 1" + message: + expr: "`expected exactly one generated-media delivery, saw ${outboundMessages.length}; transcript=${formatTransportTranscript(state, { conversationId: 'qa-operator' })}`" detailsExpr: "`${outbound.text}\\nIMAGE_PATH:${generatedPath}`" diff --git a/src/agents/embedded-agent-runner/delivery-evidence.ts b/src/agents/embedded-agent-runner/delivery-evidence.ts index 97e4a273d4b1..79e2d1c86676 100644 --- a/src/agents/embedded-agent-runner/delivery-evidence.ts +++ b/src/agents/embedded-agent-runner/delivery-evidence.ts @@ -1,3 +1,4 @@ +import { normalizeMediaReferenceForComparison } from "../../media/media-reference-comparison.js"; /** * Extracts visible delivery evidence from embedded-agent run results. */ @@ -356,10 +357,10 @@ export function hasCompleteAutomaticMediaDeliveryOutcomeEvidence( classifiedIndexes.add(index); } } - const expected = new Set(expectedMediaUrls); + const expected = new Set(expectedMediaUrls.map(normalizeMediaReferenceForComparison)); return payloads.every((payload, index) => { const containsExpectedMedia = collectDeliveredMediaUrls({ payloads: [payload] }).some((url) => - expected.has(url), + expected.has(normalizeMediaReferenceForComparison(url)), ); return !containsExpectedMedia || classifiedIndexes.has(index); }); diff --git a/src/agents/subagent-announce-delivery.test.ts b/src/agents/subagent-announce-delivery.test.ts index fba02dd0170d..68dd2dcb757b 100644 --- a/src/agents/subagent-announce-delivery.test.ts +++ b/src/agents/subagent-announce-delivery.test.ts @@ -4039,6 +4039,48 @@ describe("deliverSubagentAnnouncement completion delivery", () => { expect(sendMessage).not.toHaveBeenCalled(); }); + it("does not resend generated media when delivery evidence uses an equivalent file URL", async () => { + const callGateway = createGatewayMock({ + result: { + payloads: [], + messagingToolSentMediaUrls: ["file:///tmp/generated%20night%20drive.mp3"], + }, + }); + const sendMessage = createSendMessageMock(); + const result = await deliverSlackChannelAnnouncement({ + callGateway, + sendMessage, + sessionId: "requester-session-channel", + isActive: false, + expectsCompletionMessage: true, + directIdempotencyKey: "announce-channel-media-normalized-message-tool", + sourceTool: "music_generate", + runtimeConfig: { messages: { groupChat: { visibleReplies: "message_tool" } } }, + internalEvents: [ + { + type: "task_completion", + source: "music_generation", + childSessionKey: "music_generate:task-123", + childSessionId: "task-123", + announceType: "music generation task", + taskLabel: "night-drive synthwave", + status: "ok", + statusLabel: "completed successfully", + result: "Generated 1 track.\nMEDIA:/tmp/generated night drive.mp3", + mediaUrls: ["/tmp/generated night drive.mp3"], + replyInstruction: + "Tell the user the music is ready and send it through the message tool.", + }, + ], + }); + + expectRecordFields(result, { + delivered: true, + path: "direct", + }); + expect(sendMessage).not.toHaveBeenCalled(); + }); + it("accepts payload-only generated media when message tool sent text only", async () => { const callGateway = createGatewayMock({ result: { diff --git a/src/agents/subagent-announce-delivery.ts b/src/agents/subagent-announce-delivery.ts index 2ab1b92bd826..34e49b627d24 100644 --- a/src/agents/subagent-announce-delivery.ts +++ b/src/agents/subagent-announce-delivery.ts @@ -24,6 +24,7 @@ import { releaseSessionDeliveryClaim, type SessionDeliveryRoute, } from "../infra/session-delivery-queue.js"; +import { normalizeMediaReferenceForComparison } from "../media/media-reference-comparison.js"; import { stringifyRouteThreadId } from "../plugin-sdk/channel-route.js"; import { normalizeAccountId } from "../routing/session-key.js"; import { defaultRuntime } from "../runtime.js"; @@ -1245,7 +1246,7 @@ function resolveGeneratedMediaDirectFallbackUrls(params: { return expected; } const delivered = new Set( - params.requiresMessageToolDelivery + (params.requiresMessageToolDelivery ? collectMessagingToolDeliveredMediaUrlsForTarget(result, params.deliveryTarget) : collectAutomaticCompletionDeliveredMediaUrls({ result, @@ -1253,9 +1254,10 @@ function resolveGeneratedMediaDirectFallbackUrls(params: { automaticDeliveryRequested: params.automaticDeliveryRequested, automaticDeliveryFailed: params.automaticDeliveryFailed === true, expectedMediaCount: expected.length, - }), + }) + ).map(normalizeMediaReferenceForComparison), ); - return expected.filter((url) => !delivered.has(url)); + return expected.filter((url) => !delivered.has(normalizeMediaReferenceForComparison(url))); } function collectAutomaticCompletionDeliveredMediaUrls(params: { diff --git a/src/auto-reply/reply/reply-payloads-dedupe.ts b/src/auto-reply/reply/reply-payloads-dedupe.ts index b3b1b5548913..95d5e0928e63 100644 --- a/src/auto-reply/reply/reply-payloads-dedupe.ts +++ b/src/auto-reply/reply/reply-payloads-dedupe.ts @@ -9,6 +9,7 @@ import { getChannelPlugin } from "../../channels/plugins/index.js"; import { getLoadedChannelPluginForRead } from "../../channels/plugins/registry-loaded.js"; import { normalizeAnyChannelId } from "../../channels/registry.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { normalizeMediaReferenceForComparison } from "../../media/media-reference-comparison.js"; import { channelRouteTargetsMatchExact, stringifyRouteThreadId, @@ -46,7 +47,7 @@ export function filterMessagingToolMediaDuplicates(params: { } const sentSet = new Set(); for (const sentMediaUrl of sentMediaUrls) { - const normalized = normalizeMediaForDedupe(sentMediaUrl); + const normalized = normalizeMediaReferenceForComparison(sentMediaUrl); if (normalized) { sentSet.add(normalized); } @@ -67,13 +68,13 @@ export function filterMessagingToolMediaDuplicates(params: { } const mediaUrl = payload.mediaUrl; const mediaUrls = payload.mediaUrls; - const stripSingle = mediaUrl && sentSet.has(normalizeMediaForDedupe(mediaUrl)); + const stripSingle = mediaUrl && sentSet.has(normalizeMediaReferenceForComparison(mediaUrl)); let filteredUrls: string[] | undefined; let strippedMediaUrls = false; if (mediaUrls?.length) { for (const [mediaIndex, url] of mediaUrls.entries()) { - if (sentSet.has(normalizeMediaForDedupe(url))) { + if (sentSet.has(normalizeMediaReferenceForComparison(url))) { strippedMediaUrls = true; if (!filteredUrls) { filteredUrls = mediaUrls.slice(0, mediaIndex); @@ -117,25 +118,6 @@ export function hasEnabledDeliveryOperation(payload: ReplyPayload): boolean { return pin === true || (typeof pin === "object" && pin.enabled); } -function normalizeMediaForDedupe(value: string): string { - const trimmed = value.trim(); - if (!trimmed) { - return ""; - } - if (!normalizeLowercaseStringOrEmpty(trimmed).startsWith("file://")) { - return trimmed; - } - try { - const parsed = new URL(trimmed); - if (parsed.protocol === "file:") { - return decodeURIComponent(parsed.pathname || ""); - } - } catch { - // Keep fallback below for non-URL-like inputs. - } - return trimmed.replace(/^file:\/\//i, ""); -} - function normalizeProviderForComparison(value?: string): string | undefined { const trimmed = normalizeOptionalString(value); if (!trimmed) { diff --git a/src/gateway/server-methods/chat-send-command-replies.ts b/src/gateway/server-methods/chat-send-command-replies.ts index 3bb792265b75..fd04a0aaf256 100644 --- a/src/gateway/server-methods/chat-send-command-replies.ts +++ b/src/gateway/server-methods/chat-send-command-replies.ts @@ -1,7 +1,6 @@ -import path from "node:path"; -import { fileURLToPath } from "node:url"; import { resolveSendableOutboundReplyParts } from "openclaw/plugin-sdk/reply-payload"; import type { ReplyPayload } from "../../auto-reply/reply-payload.js"; +import { normalizeMediaReferenceForComparison } from "../../media/media-reference-comparison.js"; import { parseInlineDirectives, sanitizeReplyDirectiveId } from "../../utils/directive-tags.js"; import { sanitizeAssistantDisplayText } from "./chat-assistant-content.js"; @@ -20,27 +19,8 @@ function replyMediaUrls(payload: ReplyPayload): string[] { return resolveSendableOutboundReplyParts(payload).mediaUrls; } -function normalizeCommandMediaDedupeKey(value: string): string { - const trimmed = value.trim(); - if (!trimmed) { - return ""; - } - if (!trimmed.toLowerCase().startsWith("file://")) { - return path.isAbsolute(trimmed) ? path.normalize(trimmed) : trimmed; - } - try { - const parsed = new URL(trimmed); - if (parsed.protocol === "file:") { - return path.normalize(fileURLToPath(parsed)); - } - } catch { - // Keep malformed file URL-like values comparable with the fallback below. - } - return trimmed.replace(/^file:\/\//iu, ""); -} - function replyMediaDedupeKeys(payload: ReplyPayload): string[] { - return replyMediaUrls(payload).map((mediaUrl) => normalizeCommandMediaDedupeKey(mediaUrl)); + return replyMediaUrls(payload).map((mediaUrl) => normalizeMediaReferenceForComparison(mediaUrl)); } function canonicalizeReplyMedia(payload: ReplyPayload): ReplyPayload { diff --git a/src/gateway/server-restart-sentinel-agent-delivery.ts b/src/gateway/server-restart-sentinel-agent-delivery.ts index 8b5e8b542da5..376cbb0f9c3a 100644 --- a/src/gateway/server-restart-sentinel-agent-delivery.ts +++ b/src/gateway/server-restart-sentinel-agent-delivery.ts @@ -31,6 +31,7 @@ import { type SessionDeliveryRoute, } from "../infra/session-delivery-queue.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; +import { normalizeMediaReferenceForComparison } from "../media/media-reference-comparison.js"; import { INTERNAL_MESSAGE_CHANNEL } from "../utils/message-channel.js"; import { dispatchGatewayMethodInProcess } from "./server-plugins.js"; import { loadSessionEntry } from "./session-utils.js"; @@ -173,10 +174,16 @@ async function evaluateQueuedGeneratedMediaAgentResult(params: { ); } const expectedMediaUrls = params.entry.expectedMediaUrls ?? []; - const deliveredMediaUrls = new Set(collectQueuedDeliveredMediaUrls(params)); - const missingMediaUrls = expectedMediaUrls.filter((url) => !deliveredMediaUrls.has(url)); - const provenExpectedMediaUrls = expectedMediaUrls.filter((url) => deliveredMediaUrls.has(url)); - const ambiguousMediaUrls = new Set(collectAmbiguousAutomaticMediaUrls(params.result)); + const deliveredMediaUrls = new Set( + collectQueuedDeliveredMediaUrls(params).map(normalizeMediaReferenceForComparison), + ); + const isDelivered = (url: string) => + deliveredMediaUrls.has(normalizeMediaReferenceForComparison(url)); + const missingMediaUrls = expectedMediaUrls.filter((url) => !isDelivered(url)); + const provenExpectedMediaUrls = expectedMediaUrls.filter(isDelivered); + const ambiguousMediaUrls = new Set( + collectAmbiguousAutomaticMediaUrls(params.result).map(normalizeMediaReferenceForComparison), + ); const deliveryFailure = getAgentCommandDeliveryFailure(params.result); const replySatisfied = expectedMediaUrls.length > 0 @@ -251,7 +258,9 @@ async function evaluateQueuedGeneratedMediaAgentResult(params: { !hasCompleteAutomaticMediaDeliveryOutcomeEvidence(params.result, missingMediaUrls); if ( incompletePartialFailureEvidence || - missingMediaUrls.some((url) => ambiguousMediaUrls.has(url)) + missingMediaUrls.some((url) => + ambiguousMediaUrls.has(normalizeMediaReferenceForComparison(url)), + ) ) { log.warn("queued generated-media delivery has ambiguous attachment side effects", { queueId: params.entry.id, diff --git a/src/gateway/server-restart-sentinel.test.ts b/src/gateway/server-restart-sentinel.test.ts index 08c29d620380..0644f31ca8fc 100644 --- a/src/gateway/server-restart-sentinel.test.ts +++ b/src/gateway/server-restart-sentinel.test.ts @@ -1555,6 +1555,42 @@ describe("scheduleRestartSentinelWake", () => { ); }); + it("accepts normalized generated-media evidence without a bare retry", async () => { + mocks.dispatchGatewayMethodInProcess.mockResolvedValueOnce({ + status: "ok", + result: { + payloads: [{ text: "ready", mediaUrls: ["/tmp/generated image.png"] }], + deliveryStatus: { status: "sent" }, + }, + }); + + await deliverQueuedSessionDelivery({ + deps: {} as never, + entry: { + id: "session-delivery-media-normalized", + kind: "agentTurn", + sessionKey: "agent:main:main", + message: "generated image ready", + messageId: "image:task-normalized:agent-loop", + idempotencyKey: "image:task-normalized:agent-loop", + enqueuedAt: 1, + retryCount: 0, + route: { channel: "discord", to: "channel:123", chatType: "channel" }, + inputProvenance: { + kind: "inter_session", + sourceChannel: "webchat", + sourceTool: "image_generate", + }, + sourceReplyDeliveryMode: "automatic", + expectedMediaUrls: ["file:///tmp/generated%20image.png"], + }, + }); + + expect(mocks.advanceSessionDeliveryAgentRun).not.toHaveBeenCalled(); + expect(mocks.failSessionDelivery).not.toHaveBeenCalled(); + expect(mocks.deferSessionDelivery).not.toHaveBeenCalled(); + }); + it("accepts a generated-media reply committed only to the owning transcript", async () => { mocks.dispatchGatewayMethodInProcess.mockResolvedValueOnce({ status: "ok", diff --git a/src/media/media-reference-comparison.test.ts b/src/media/media-reference-comparison.test.ts new file mode 100644 index 000000000000..d7de2551a77c --- /dev/null +++ b/src/media/media-reference-comparison.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { normalizeMediaReferenceForComparison } from "./media-reference-comparison.js"; + +describe("normalizeMediaReferenceForComparison", () => { + it("matches encoded file URLs with equivalent absolute paths", () => { + expect(normalizeMediaReferenceForComparison("file:///tmp/generated%20image.png")).toBe( + normalizeMediaReferenceForComparison("/tmp/generated image.png"), + ); + }); + + it("keeps parent segments distinct without resolving filesystem identity", () => { + expect(normalizeMediaReferenceForComparison("/tmp/output/../generated.png")).toBe( + "/tmp/output/../generated.png", + ); + }); + + it("applies URL parent-segment semantics before filesystem comparison", () => { + expect(normalizeMediaReferenceForComparison("file:///tmp/output/../generated.png")).toBe( + normalizeMediaReferenceForComparison("/tmp/generated.png"), + ); + expect(normalizeMediaReferenceForComparison("file:///tmp/output/%2e%2e/generated.png")).toBe( + normalizeMediaReferenceForComparison("/tmp/generated.png"), + ); + }); + + it("normalizes safe absolute path syntax symmetrically", () => { + expect(normalizeMediaReferenceForComparison("/tmp/output/./generated.png")).toBe( + normalizeMediaReferenceForComparison("file:///tmp/output/./generated.png"), + ); + }); + + it("preserves remote and relative references after trimming", () => { + expect(normalizeMediaReferenceForComparison(" https://example.test/a/../b.png ")).toBe( + "https://example.test/a/../b.png", + ); + expect(normalizeMediaReferenceForComparison("./output/../generated.png")).toBe( + "./output/../generated.png", + ); + expect(normalizeMediaReferenceForComparison("file://server/share.png")).not.toBe( + normalizeMediaReferenceForComparison("server/share.png"), + ); + expect(normalizeMediaReferenceForComparison("//cdn.example/share.png")).toBe( + "//cdn.example/share.png", + ); + expect(normalizeMediaReferenceForComparison("//cdn.example/share.png")).not.toBe( + normalizeMediaReferenceForComparison("/cdn.example/share.png"), + ); + expect(normalizeMediaReferenceForComparison("file:////cdn.example/share.png")).not.toBe( + normalizeMediaReferenceForComparison("//cdn.example/share.png"), + ); + expect(normalizeMediaReferenceForComparison("file:////cdn.example/share.png")).not.toBe( + normalizeMediaReferenceForComparison("/cdn.example/share.png"), + ); + expect(normalizeMediaReferenceForComparison("file:////cdn.example/a//share.png")).toBe( + normalizeMediaReferenceForComparison("file:////cdn.example/a/share.png"), + ); + }); + + it("keeps malformed local file URLs comparable with their paths", () => { + expect(normalizeMediaReferenceForComparison("file:///tmp/100%.png")).toBe( + normalizeMediaReferenceForComparison("/tmp/100%.png"), + ); + expect(normalizeMediaReferenceForComparison("file:///tmp/link/../asset%.png")).toBe( + "/tmp/link/../asset%.png", + ); + }); +}); diff --git a/src/media/media-reference-comparison.ts b/src/media/media-reference-comparison.ts new file mode 100644 index 000000000000..9d444f95ca43 --- /dev/null +++ b/src/media/media-reference-comparison.ts @@ -0,0 +1,64 @@ +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const PATH_PARENT_SEGMENT_RE = /(?:^|[\\/])\.\.(?:[\\/]|$)/u; +const FORWARD_NETWORK_PATH_PREFIX_RE = /^\/\//u; +const FILE_URL_PREFIX_LENGTH = "file://".length; +const FILE_URL_LOCAL_NETWORK_KEY_PREFIX = "\0file-url-local-network:"; + +function normalizeAbsoluteLocalPath(value: string): string { + if ( + !path.isAbsolute(value) || + PATH_PARENT_SEGMENT_RE.test(value) || + FORWARD_NETWORK_PATH_PREFIX_RE.test(value) + ) { + return value; + } + return path.normalize(value); +} + +function normalizeFileUrlLocalPath(value: string): string { + if (!value.startsWith("//")) { + return normalizeAbsoluteLocalPath(value); + } + const normalized = PATH_PARENT_SEGMENT_RE.test(value) + ? value + : `//${path.normalize(value.slice(2))}`; + return `${FILE_URL_LOCAL_NETWORK_KEY_PREFIX}${normalized}`; +} + +function normalizeMalformedLocalFileUrl(value: string): string | undefined { + const remainder = value.slice(FILE_URL_PREFIX_LENGTH); + let localPath: string; + if (remainder.startsWith("/")) { + localPath = remainder; + } else if (/^localhost(?:\/|$)/iu.test(remainder)) { + localPath = remainder.slice("localhost".length); + } else { + return undefined; + } + if (process.platform === "win32" && /^\/[a-z]:[\\/]/iu.test(localPath)) { + localPath = localPath.slice(1); + } + return normalizeFileUrlLocalPath(localPath); +} + +/** Canonicalizes equivalent local media references without resolving the filesystem. */ +export function normalizeMediaReferenceForComparison(value: string): string { + const trimmed = value.trim(); + if (!trimmed) { + return ""; + } + if (!trimmed.toLowerCase().startsWith("file://")) { + return normalizeAbsoluteLocalPath(trimmed); + } + try { + const parsed = new URL(trimmed); + if (parsed.protocol === "file:") { + return normalizeFileUrlLocalPath(fileURLToPath(parsed)); + } + } catch { + // Preserve the historical fallback for malformed local URLs without conflating remote hosts. + } + return normalizeMalformedLocalFileUrl(trimmed) ?? trimmed; +}