fix(delivery): preserve explicit delivery operations

This commit is contained in:
Vincent Koc
2026-07-12 16:02:50 -07:00
parent 5f69ce8c0d
commit c44d831488
3 changed files with 125 additions and 2 deletions

View File

@@ -855,6 +855,76 @@ describe("normalizeAgentCommandReplyPayloads", () => {
expect(deliverOutboundPayloadsMock).not.toHaveBeenCalled();
});
it("preserves duplicate media needed for a delivery operation", async () => {
deliverOutboundPayloadsMock.mockResolvedValue([{ channel: "slack", messageId: "msg-1" }]);
const delivery = { pin: { enabled: true, required: true } };
const delivered = await deliverAgentCommandResult({
cfg: {} as OpenClawConfig,
deps: {} as CliDeps,
runtime: { log: vi.fn(), error: vi.fn() } as never,
opts: {
message: "completion handoff",
deliver: true,
replyChannel: "slack",
replyTo: "channel:C123",
} as AgentCommandOpts,
outboundSession: undefined,
sessionEntry: undefined,
payloads: [{ mediaUrls: ["/tmp/generated-image.png"], delivery }] as never,
result: {
...createResult(),
messagingToolSentTargets: [
{
tool: "message",
provider: "slack",
to: "channel:C123",
mediaUrls: ["/tmp/generated-image.png"],
},
],
} as RunResult,
});
expect(delivered.deliverySucceeded).toBe(true);
expect(latestOutboundDeliveryArgs().payloads).toEqual([
expect.objectContaining({
mediaUrls: ["/tmp/generated-image.png"],
delivery,
}),
]);
});
it("drops audioAsVoice when its media was already delivered", async () => {
const delivered = await deliverAgentCommandResult({
cfg: {} as OpenClawConfig,
deps: {} as CliDeps,
runtime: { log: vi.fn(), error: vi.fn() } as never,
opts: {
message: "completion handoff",
deliver: true,
replyChannel: "slack",
replyTo: "channel:C123",
} as AgentCommandOpts,
outboundSession: undefined,
sessionEntry: undefined,
payloads: [{ mediaUrls: ["/tmp/voice.ogg"], audioAsVoice: true }],
result: {
...createResult(),
messagingToolSentTargets: [
{
tool: "message",
provider: "slack",
to: "channel:C123",
mediaUrls: ["/tmp/voice.ogg"],
},
],
} as RunResult,
});
expect(delivered.payloads).toEqual([]);
expect(deliverOutboundPayloadsMock).not.toHaveBeenCalled();
});
it("dedupes delivered file media before normalization can add a failure warning", async () => {
createReplyMediaPathNormalizerMock.mockImplementationOnce(
(..._args: unknown[]) =>

View File

@@ -57,6 +57,14 @@ export function filterMessagingToolMediaDuplicates(params: {
let nextPayloads: ReplyPayload[] | undefined;
for (const [index, payload] of payloads.entries()) {
// Delivery operations apply to the message created by this payload. Keep
// its content intact so dedupe cannot silently skip the operation.
if (hasEnabledDeliveryOperation(payload)) {
if (nextPayloads) {
nextPayloads.push(payload);
}
continue;
}
const mediaUrl = payload.mediaUrl;
const mediaUrls = payload.mediaUrls;
const stripSingle = mediaUrl && sentSet.has(normalizeMediaForDedupe(mediaUrl));
@@ -85,10 +93,15 @@ export function filterMessagingToolMediaDuplicates(params: {
continue;
}
const nextMediaUrl = stripSingle ? undefined : mediaUrl;
const nextMediaUrls = filteredUrls?.length ? filteredUrls : undefined;
const nextPayload = copyReplyPayloadMetadata(payload, {
...payload,
mediaUrl: stripSingle ? undefined : mediaUrl,
mediaUrls: filteredUrls?.length ? filteredUrls : undefined,
mediaUrl: nextMediaUrl,
mediaUrls: nextMediaUrls,
...(payload.audioAsVoice === true && !nextMediaUrl && !nextMediaUrls
? { audioAsVoice: undefined }
: {}),
});
if (!nextPayloads) {
nextPayloads = payloads.slice(0, index);
@@ -99,6 +112,11 @@ export function filterMessagingToolMediaDuplicates(params: {
return nextPayloads ?? payloads;
}
function hasEnabledDeliveryOperation(payload: ReplyPayload): boolean {
const pin = payload.delivery?.pin;
return pin === true || (typeof pin === "object" && pin.enabled === true);
}
function normalizeMediaForDedupe(value: string): string {
const trimmed = value.trim();
if (!trimmed) {

View File

@@ -78,6 +78,41 @@ describe("filterMessagingToolMediaDuplicates", () => {
expect(result).toEqual([{ text: "gallery", mediaUrl: undefined, mediaUrls: undefined }]);
});
it("preserves media for payloads with delivery operations", () => {
const delivery = { pin: { enabled: true, required: true } };
const payload = { mediaUrl: "file:///tmp/photo.jpg", delivery };
const result = filterMessagingToolMediaDuplicates({
payloads: [payload],
sentMediaUrls: ["file:///tmp/photo.jpg"],
});
expect(result).toEqual([payload]);
});
it.each([{ delivery: { pin: false } }, { delivery: { pin: { enabled: false } } }])(
"dedupes media for disabled delivery metadata: $delivery",
({ delivery }) => {
const result = filterMessagingToolMediaDuplicates({
payloads: [{ mediaUrl: "file:///tmp/photo.jpg", delivery }],
sentMediaUrls: ["file:///tmp/photo.jpg"],
});
expect(result).toEqual([{ mediaUrl: undefined, mediaUrls: undefined, delivery }]);
},
);
it("clears audioAsVoice when dedupe removes all media", () => {
const result = filterMessagingToolMediaDuplicates({
payloads: [{ mediaUrl: "file:///tmp/voice.ogg", audioAsVoice: true }],
sentMediaUrls: ["file:///tmp/voice.ogg"],
});
expect(result).toEqual([
{
mediaUrl: undefined,
mediaUrls: undefined,
audioAsVoice: undefined,
},
]);
});
it("returns payloads unchanged when no media present", () => {
const payloads = [{ text: "plain text" }];
const result = filterMessagingToolMediaDuplicates({