fix(agents): prevent duplicate generated media delivery

This commit is contained in:
Peter Steinberger
2026-07-18 04:34:12 +01:00
parent 28c2f4c075
commit 5879f097bd
20 changed files with 197 additions and 37 deletions
@@ -115,7 +115,22 @@ describe("constrainRestartRecoveryDeliveryPayloads", () => {
],
[" /tmp/missing.png ", "/tmp/missing.png"],
),
).toEqual([{ text: "ready" }, { mediaUrls: ["/tmp/missing.png"], trustedLocalMedia: true }]);
).toEqual([{ text: "ready", mediaUrls: ["/tmp/missing.png"], trustedLocalMedia: true }]);
});
it("strips duplicate MEDIA directives before attaching the host-owned media", () => {
expect(
constrainRestartRecoveryDeliveryPayloads(
[{ text: "Fresh from the decks\nMEDIA:/tmp/generated.png" }],
["/tmp/generated.png"],
),
).toEqual([
{
text: "Fresh from the decks",
mediaUrls: ["/tmp/generated.png"],
trustedLocalMedia: true,
},
]);
});
it("strips all model media from a text-only notice", () => {
+17 -2
View File
@@ -1,6 +1,7 @@
import type { ReplyPayload } from "../auto-reply/reply-payload.js";
import type { RestartRecoveryTerminalDeliveryEvidenceResult } from "../config/sessions/restart-recovery-types.js";
import type { SessionEntry } from "../config/sessions/types.js";
import { splitMediaFromOutput } from "../media/parse.js";
import type { DeliveryContext } from "../utils/delivery-context.shared.js";
import {
collectDeliveredMediaUrls,
@@ -47,7 +48,10 @@ export function constrainRestartRecoveryDeliveryPayloads(
for (const payload of payloads ?? []) {
const constrainedPayload: ReplyPayload = {};
if (!suppressText && typeof payload.text === "string") {
constrainedPayload.text = payload.text;
// The host appends the canonical media set below. Strip model-emitted
// MEDIA directives first or the outbound parser re-materializes the same
// attachment in both this caption payload and the host-owned payload.
constrainedPayload.text = splitMediaFromOutput(payload.text).text;
}
if (payload.isError === true) {
constrainedPayload.isError = true;
@@ -78,7 +82,18 @@ export function constrainRestartRecoveryDeliveryPayloads(
new Set(mediaUrls.map((url) => url.trim()).filter((url) => url.length > 0)),
);
if (exactMediaUrls.length > 0) {
constrained.push({ mediaUrls: exactMediaUrls, trustedLocalMedia: true });
const captionIndex = constrained.findIndex(
(payload) => payload.isReasoning !== true && Boolean(payload.text?.trim()),
);
if (captionIndex >= 0) {
constrained[captionIndex] = {
...constrained[captionIndex],
mediaUrls: exactMediaUrls,
trustedLocalMedia: true,
};
} else {
constrained.push({ mediaUrls: exactMediaUrls, trustedLocalMedia: true });
}
}
return constrained;
}
@@ -2854,12 +2854,10 @@ describe("agentCommand LiveSessionModelSwitchError retry", () => {
"delivery params",
);
expect(requireRecord(deliveryParams.result, "delivery result").payloads).toEqual([
{ text: "ready" },
{ mediaUrls: ["/tmp/missing.png"], trustedLocalMedia: true },
{ text: "ready", mediaUrls: ["/tmp/missing.png"], trustedLocalMedia: true },
]);
expect(deliveryParams.payloads).toEqual([
{ text: "ready" },
{ mediaUrls: ["/tmp/missing.png"], trustedLocalMedia: true },
{ text: "ready", mediaUrls: ["/tmp/missing.png"], trustedLocalMedia: true },
]);
expect(
state.persistSessionEntryMock.mock.calls.some((call) => {
+24
View File
@@ -1880,6 +1880,30 @@ describe("deliverAgentCommandResult payload normalization", () => {
expect(outcome?.stage).toBe("platform_send");
});
it("accepts an already-owned internal delivery intent during replay", async () => {
deliverOutboundPayloadsMock.mockRejectedValueOnce(
new Error("Stable delivery intent is already queued: image:task-1:generated-media-direct"),
);
const delivered = await deliverMediaReplyForTest(
{
key: "agent:tester:slack:direct:alice",
agentId: "tester",
} as never,
{ internalDeliveryIdempotencyKey: "image:task-1:generated-media-direct" },
);
expect(delivered.deliverySucceeded).toBe(true);
expectDeliveryStatusFields(delivered, {
requested: true,
attempted: true,
status: "suppressed",
succeeded: true,
reason: "delivery_intent_already_owned",
resultCount: 0,
});
});
it("marks no-payload deliveryStatus as terminal delivery success", async () => {
const delivered = await deliverAgentCommandResult({
cfg: {} as OpenClawConfig,
+27 -3
View File
@@ -941,6 +941,12 @@ export async function deliverAgentCommandResult(
threadId: resolvedThreadTarget ?? null,
bestEffort: bestEffortDeliver,
durability: bestEffortDeliver ? "best_effort" : "required",
...(opts.internalDeliveryIdempotencyKey
? {
deliveryIntentId: opts.internalDeliveryIdempotencyKey,
completionRetention: "permanent" as const,
}
: {}),
signal: restartAbort.signal,
onDeliveryIntent: restartAbort.dispose,
onError: logDeliveryError,
@@ -953,8 +959,25 @@ export async function deliverAgentCommandResult(
if (restartAbort.signal?.aborted && send.status === "failed") {
throw restartAbort.signal.reason;
}
deliveryStatus = deliveryStatusFromDurableSend(send);
if (!bestEffortDeliver && (send.status === "failed" || send.status === "partial_failed")) {
const stableIntentAlreadyOwned =
send.status === "failed" &&
Boolean(opts.internalDeliveryIdempotencyKey) &&
/Stable delivery intent is already queued:/i.test(formatErrorMessage(send.error));
deliveryStatus = stableIntentAlreadyOwned
? {
requested: true,
attempted: true,
status: "suppressed",
succeeded: true,
reason: "delivery_intent_already_owned",
resultCount: 0,
}
: deliveryStatusFromDurableSend(send);
if (
!stableIntentAlreadyOwned &&
!bestEffortDeliver &&
(send.status === "failed" || send.status === "partial_failed")
) {
emitJsonEnvelope(deliveryStatus);
captureDeliveryResult(
buildDeliveryResult({
@@ -967,7 +990,8 @@ export async function deliverAgentCommandResult(
);
throw send.error;
}
deliverySucceeded = send.status === "sent" || send.status === "suppressed";
deliverySucceeded =
stableIntentAlreadyOwned || send.status === "sent" || send.status === "suppressed";
}
}
if (deliver && !deliveryStatus) {
+1
View File
@@ -153,6 +153,7 @@ export type AgentCommandOpts = {
forceRestartSafeTools?: boolean;
/** Host-owned exact media set for a scoped automatic recovery delivery. */
internalDeliveryMediaUrls?: string[];
internalDeliveryIdempotencyKey?: string;
internalDeliverySuppressText?: boolean;
/** Gateway ingress that already persisted visible activity can skip the duplicate pre-run touch. */
skipInitialSessionTouch?: boolean;
+47 -2
View File
@@ -3428,6 +3428,8 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
content: "The generated music is ready.",
mediaUrls: ["/tmp/generated-night-drive.mp3"],
idempotencyKey: "announce-dm-fallback-empty:generated-media-direct",
deliveryIntentId: "announce-dm-fallback-empty:generated-media-direct",
completionRetention: "permanent",
}),
);
});
@@ -3909,6 +3911,46 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
);
});
it("treats a shared generated-media delivery intent as already delivered", async () => {
const callGateway = createGatewayMock({
result: { payloads: [{ text: "The track is ready." }] },
});
const sendMessage = vi.fn(async () => {
throw new Error(
"Stable delivery intent is already queued: announce-dm-fallback-empty:generated-media-direct",
);
}) as unknown as typeof runtimeSendMessage;
const result = await deliverDiscordDirectMessageCompletion({
callGateway,
sendMessage,
sourceTool: "music_generate",
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.",
mediaUrls: ["/tmp/generated-night-drive.mp3"],
replyInstruction: "Deliver the generated music.",
},
],
});
expectRecordFields(result, { delivered: true, path: "direct" });
expect(generatedMediaWakeMocks.wakeSessionForGeneratedMediaDirectDelivery).toHaveBeenCalledWith(
expect.objectContaining({
mediaLabel: "music",
contextKey: "announce-dm-fallback-empty:generated-media-direct",
}),
);
});
it("allows visible direct delivery for media generation failure summaries without generated media", async () => {
const callGateway = createGatewayMock({
result: {
@@ -4850,7 +4892,9 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
expect.objectContaining({
kind: "agentTurn",
sessionKey: "agent:main:slack:channel:C123",
message: expect.stringContaining("generated-locked.png"),
message: expect.stringMatching(
/generated-locked\.png[\s\S]*completion delivery system owns the generated media/,
),
messageId: "announce-channel-media-handoff-locked:agent-loop",
route: {
channel: "slack",
@@ -4863,8 +4907,9 @@ describe("deliverSubagentAnnouncement completion delivery", () => {
sourceChannel: "webchat",
sourceTool: "image_generate",
},
sourceReplyDeliveryMode: "message_tool_only",
sourceReplyDeliveryMode: "automatic",
expectedMediaUrls: ["/tmp/generated-locked.png"],
deliveryIdempotencyKey: "announce-channel-media-handoff-locked:generated-media-direct",
idempotencyKey: "announce-channel-media-handoff-locked:agent-loop",
}),
expect.any(Number),
+36 -25
View File
@@ -1056,6 +1056,8 @@ async function deliverGeneratedMediaCompletionDirect(params: {
content: params.content ?? `The generated ${mediaLabel} is ready.`,
mediaUrls: Array.from(params.mediaUrls),
idempotencyKey,
deliveryIntentId: idempotencyKey,
completionRetention: "permanent",
mirror: {
sessionKey: params.requesterSessionKey,
agentId,
@@ -1082,6 +1084,24 @@ async function deliverGeneratedMediaCompletionDirect(params: {
path: "direct",
};
} catch (err) {
if (/Stable delivery intent is already queued:/i.test(summarizeDeliveryError(err))) {
if (params.wakeAfterDelivery) {
wakeSessionForGeneratedMediaDirectDelivery({
cfg: params.cfg,
sessionKey: params.requesterSessionKey,
mediaLabel,
status: params.status ?? "ok",
deliveryContext: {
channel: params.deliveryTarget.channel,
to: params.deliveryTarget.to,
accountId: params.deliveryTarget.accountId,
threadId: params.deliveryTarget.threadId,
},
contextKey: idempotencyKey,
});
}
return { delivered: true, path: "direct" };
}
const terminal = hasAnnounceSendEvidence(err);
return {
delivered: false,
@@ -2202,36 +2222,26 @@ export async function deliverSubagentAnnouncement(params: {
directOrigin: params.directOrigin,
requesterSessionOrigin: params.requesterSessionOrigin,
});
const { requesterSessionOrigin, effectiveDirectOrigin } = resolveCompletionDeliveryOrigins({
expectsCompletionMessage: params.expectsCompletionMessage,
completionDirectOrigin: params.completionDirectOrigin,
directOrigin: params.directOrigin,
requesterSessionOrigin: params.requesterSessionOrigin,
});
const requesterEntry = subagentAnnounceDeliveryDeps.loadRequesterSessionEntry(
params.targetRequesterSessionKey,
).entry;
// No external route exists for an internal-only handoff. Let the normal
// agent final enter the owning transcript instead of requiring a message tool target.
const sourceReplyDeliveryMode =
queuedRoute.route.channel === INTERNAL_MESSAGE_CHANNEL
? "automatic"
: completionRequiresMessageToolDelivery({
cfg,
requesterSessionKey: params.requesterSessionKey,
targetRequesterSessionKey: canonicalSessionKey,
requesterEntry,
directOrigin: effectiveDirectOrigin,
requesterSessionOrigin,
})
? "message_tool_only"
: "automatic";
// Durable generated-media handoffs are host-owned: the model supplies
// only caption text while the queue fixes the route and exact attachment.
// Replay dispatch also enforces disableMessageTool=true.
const sourceReplyDeliveryMode = "automatic";
const queued = await enqueueClaimedSessionDelivery(
{
kind: "agentTurn",
sessionKey: canonicalSessionKey,
message:
formatAgentInternalEventsForPrompt(params.internalEvents) || params.triggerMessage,
formatAgentInternalEventsForPrompt(
params.internalEvents?.map((event) =>
event.status === "ok" && collectExpectedMediaFromInternalEvents([event]).length > 0
? {
...event,
replyInstruction:
"The completion delivery system owns the generated media and will attach it exactly once to this reply. Do not call the message tool and do not include MEDIA lines or otherwise resend the attachment. Write only the short caption or normal conversational reply.",
}
: event,
),
) || params.triggerMessage,
messageId: `${params.directIdempotencyKey}:agent-loop`,
route: queuedRoute.route,
...(queuedRoute.deliveryContext ? { deliveryContext: queuedRoute.deliveryContext } : {}),
@@ -2243,6 +2253,7 @@ export async function deliverSubagentAnnouncement(params: {
},
sourceReplyDeliveryMode,
expectedMediaUrls: collectExpectedMediaFromInternalEvents(params.internalEvents),
deliveryIdempotencyKey: `${params.directIdempotencyKey}:generated-media-direct`,
idempotencyKey: `${params.directIdempotencyKey}:agent-loop`,
},
resolveSubagentAnnounceTimeoutMs(cfg) + 5_000,
@@ -1084,6 +1084,8 @@ describe("createMediaGenerationTaskLifecycle", () => {
const announceParams = subagentAnnounceDeliveryMocks.deliverSubagentAnnouncement.mock
.calls[0]?.[0] as { triggerMessage?: string; internalEvents?: unknown[] } | undefined;
expect(announceParams?.triggerMessage).toContain("MEDIA:/tmp/generated-night-drive.mp3");
expect(announceParams?.triggerMessage).toContain('call message(action="send")');
expect(announceParams?.triggerMessage).toContain("final-reply MEDIA lines");
expect(announceParams?.internalEvents).toEqual([
expect.objectContaining({
mediaUrls: ["/tmp/generated-night-drive.mp3"],
@@ -359,6 +359,7 @@ export function startAgentRunExecution(params: {
disableMessageTool: params.request.disableMessageTool,
forceRestartSafeTools: params.request.forceRestartSafeTools,
internalDeliveryMediaUrls: params.client?.internal?.internalDeliveryMediaUrls,
internalDeliveryIdempotencyKey: params.client?.internal?.internalDeliveryIdempotencyKey,
internalDeliverySuppressText: params.client?.internal?.internalDeliverySuppressText,
suppressPromptPersistence:
params.requestedPromptPersistenceSuppression ||
+1
View File
@@ -862,6 +862,7 @@ export const sendHandlers: GatewayRequestHandlers = {
gatewayClientScopes: client?.connect?.scopes ?? [],
silent: request.silent,
formatting: request.parseMode ? { parseMode: request.parseMode } : undefined,
deliveryIntentId: idem,
mirror: outboundSessionKey
? {
sessionKey: outboundSessionKey,
@@ -82,6 +82,7 @@ export type GatewayClient = {
agentRunTracking?: "plugin_subagent";
/** Host-owned exact media set for a scoped automatic recovery delivery. */
internalDeliveryMediaUrls?: string[];
internalDeliveryIdempotencyKey?: string;
internalDeliverySuppressText?: boolean;
/** Plugin-owned tools authorized for this internal subagent run. */
runtimePluginToolGrant?: RuntimePluginToolGrant;
@@ -17,6 +17,7 @@ export function createSyntheticPluginRuntimeClient(params?: {
agentRunTracking?: "plugin_subagent";
cronRunContinuation?: boolean;
internalDeliveryMediaUrls?: string[];
internalDeliveryIdempotencyKey?: string;
internalDeliverySuppressText?: boolean;
pluginRuntimeOwnerId?: string;
runtimePluginToolGrant?: RuntimePluginToolGrant;
@@ -46,6 +47,9 @@ export function createSyntheticPluginRuntimeClient(params?: {
...(params?.internalDeliveryMediaUrls
? { internalDeliveryMediaUrls: [...params.internalDeliveryMediaUrls] }
: {}),
...(params?.internalDeliveryIdempotencyKey
? { internalDeliveryIdempotencyKey: params.internalDeliveryIdempotencyKey }
: {}),
...(params?.internalDeliverySuppressText === true
? { internalDeliverySuppressText: true }
: {}),
+2
View File
@@ -241,6 +241,7 @@ type DispatchGatewayMethodInProcessOptions = {
expectFinal?: boolean;
forceSyntheticClient?: boolean;
internalDeliveryMediaUrls?: string[];
internalDeliveryIdempotencyKey?: string;
internalDeliverySuppressText?: boolean;
onAccepted?: (payload: unknown) => void;
pluginRuntimeOwnerId?: string;
@@ -280,6 +281,7 @@ export async function dispatchGatewayMethodInProcessRaw(
agentRunTracking: options?.agentRunTracking,
cronRunContinuation: options?.allowSyntheticCronRunContinuation === true,
internalDeliveryMediaUrls: options?.internalDeliveryMediaUrls,
internalDeliveryIdempotencyKey: options?.internalDeliveryIdempotencyKey,
internalDeliverySuppressText: options?.internalDeliverySuppressText,
...(pluginRuntimeOwnerId ? { pluginRuntimeOwnerId } : {}),
...(options?.runtimePluginToolGrant
@@ -317,6 +317,7 @@ export async function deliverQueuedGeneratedMediaAgentTurn(params: {
}
const queuedRunId = resolveQueuedAgentRunId(entry);
const deliveryIdempotencyKey = entry.deliveryIdempotencyKey;
const deliveryMode = resolveDurableCompletionDeliveryMode(entry.sourceReplyDeliveryMode);
if (deliveryMode === "host_owned" && route.channel === INTERNAL_MESSAGE_CHANNEL) {
return await deadLetterSessionDelivery(
@@ -439,6 +440,9 @@ export async function deliverQueuedGeneratedMediaAgentTurn(params: {
expectFinal: true,
forceSyntheticClient: true,
internalDeliveryMediaUrls: entry.expectedMediaUrls ?? [],
...(deliveryIdempotencyKey
? { internalDeliveryIdempotencyKey: deliveryIdempotencyKey }
: {}),
...(entry.suppressTextDelivery === true ? { internalDeliverySuppressText: true } : {}),
onAccepted: () => {
accepted = true;
@@ -1163,6 +1163,7 @@ describe("scheduleRestartSentinelWake", () => {
},
sourceReplyDeliveryMode: "automatic",
expectedMediaUrls: ["/tmp/proof.png"],
deliveryIdempotencyKey: "image:task-automatic:generated-media-direct",
suppressTextDelivery: true,
},
});
@@ -1178,6 +1179,7 @@ describe("scheduleRestartSentinelWake", () => {
expectFinal: true,
forceSyntheticClient: true,
internalDeliveryMediaUrls: ["/tmp/proof.png"],
internalDeliveryIdempotencyKey: "image:task-automatic:generated-media-direct",
internalDeliverySuppressText: true,
onAccepted: expect.any(Function),
},
+3
View File
@@ -777,6 +777,8 @@ type DeliverOutboundPayloadsCoreParams = {
deliveryQueueId?: string;
/** @internal Stable producer id used to make queue creation idempotent across crashes. */
deliveryIntentId?: string;
/** @internal Preserve the completed producer receipt for permanent deduplication. */
completionRetention?: "permanent";
/** @internal Serializable owner state finalized after live or recovered delivery. */
deliveryCompletion?: DurableDeliveryCompletion;
/** @internal Channel-valid id reserved before a correlated conversation turn is sent. */
@@ -1591,6 +1593,7 @@ export async function deliverOutboundPayloadsInternal(
session: params.session,
gatewayClientScopes: params.gatewayClientScopes,
preparedMessageId: params.preparedMessageId,
completionRetention: params.completionRetention,
deliveryCompletion: params.deliveryCompletion,
};
if (params.deliveryIntentId) {
+2
View File
@@ -349,6 +349,7 @@ describe("sendMessage", () => {
queuePolicy: "required",
requireUnknownSendReconciliation: false,
deliveryIntentId: "operation-1",
completionRetention: "permanent",
deliveryCompletion: {
kind: "conversation",
agentId: "main",
@@ -361,6 +362,7 @@ describe("sendMessage", () => {
const deliveryParams = expectDeliveryCallFields({
queuePolicy: "required",
deliveryIntentId: "operation-1",
completionRetention: "permanent",
deliveryCompletion: {
kind: "conversation",
agentId: "main",
+3
View File
@@ -98,6 +98,8 @@ type MessageSendParams = {
gatewayOwnedDelivery?: boolean;
/** @internal Stable producer id for idempotent durable queue creation. */
deliveryIntentId?: string;
/** @internal Preserve the completed producer receipt for permanent deduplication. */
completionRetention?: "permanent";
/** @internal Serializable owner state finalized by live send or recovery. */
deliveryCompletion?: DurableDeliveryCompletion;
/** @internal Override provider unknown-send reconciliation independently from queue durability. */
@@ -444,6 +446,7 @@ export async function sendMessage(params: MessageSendParams): Promise<MessageSen
formatting: params.parseMode ? { parseMode: params.parseMode } : undefined,
preparedMessageId: params.preparedMessageId,
deliveryIntentId: params.deliveryIntentId,
completionRetention: params.completionRetention,
deliveryCompletion: params.deliveryCompletion,
...(params.onDeliveryIntent ? { onDeliveryIntent: params.onDeliveryIntent } : {}),
...(params.onDeliveryResult ? { onDeliveryResult: params.onDeliveryResult } : {}),
@@ -64,6 +64,8 @@ export type QueuedSessionDeliveryPayload =
inputProvenance?: InputProvenance;
sourceReplyDeliveryMode?: SourceReplyDeliveryMode;
expectedMediaUrls?: string[];
/** Stable outbound ownership shared with generated-media direct fallback. */
deliveryIdempotencyKey?: string;
suppressTextDelivery?: true;
idempotencyKey?: string;
} & SessionDeliveryRetryPolicy);