mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 19:35:28 -06:00
fix(channels): deliver normalized attachments exactly once
This commit is contained in:
@@ -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" })
|
||||
|
||||
@@ -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 : [];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 = {
|
||||
@@ -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([
|
||||
{
|
||||
|
||||
@@ -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<RenderedMessageBatchPlan>(
|
||||
(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),
|
||||
|
||||
@@ -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 }),
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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";
|
||||
|
||||
Reference in New Issue
Block a user