mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-22 15:01:13 +00:00
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
This commit is contained in:
committed by
GitHub
parent
40e7447546
commit
85e4bb438d
@@ -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}`"
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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<string>();
|
||||
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) {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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",
|
||||
|
||||
67
src/media/media-reference-comparison.test.ts
Normal file
67
src/media/media-reference-comparison.test.ts
Normal file
@@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
64
src/media/media-reference-comparison.ts
Normal file
64
src/media/media-reference-comparison.ts
Normal file
@@ -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;
|
||||
}
|
||||
Reference in New Issue
Block a user