mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-07 07:11:39 +00:00
test(media-dedup): add missing coverage for Discord media dedup wiring
Cover three integration points where media dedup could silently regress: - trimMessagingToolSent FIFO cap at 200 entries - buildReplyPayloads media filter wiring (new test file) - followup-runner messagingToolSentMediaUrls filtering
This commit is contained in:
committed by
Peter Steinberger
parent
4640999e77
commit
c7681c3cff
@@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- iOS/Talk: harden mobile talk config handling by ignoring redacted/env-placeholder API keys, support secure local keychain override, improve accessibility motion/contrast behavior in status UI, and tighten ATS to local-network allowance. (#18163) Thanks @mbelinky.
|
- iOS/Talk: harden mobile talk config handling by ignoring redacted/env-placeholder API keys, support secure local keychain override, improve accessibility motion/contrast behavior in status UI, and tighten ATS to local-network allowance. (#18163) Thanks @mbelinky.
|
||||||
- iOS/Location: restore the significant location monitor implementation (service hooks + protocol surface + ATS key alignment) after merge drift so iOS builds compile again. (#18260) Thanks @ngutman.
|
- iOS/Location: restore the significant location monitor implementation (service hooks + protocol surface + ATS key alignment) after merge drift so iOS builds compile again. (#18260) Thanks @ngutman.
|
||||||
- Telegram: keep DM-topic replies and draft previews in the originating private-chat topic by preserving positive `message_thread_id` values for DM threads. (#18586) Thanks @sebslight.
|
- Telegram: keep DM-topic replies and draft previews in the originating private-chat topic by preserving positive `message_thread_id` values for DM threads. (#18586) Thanks @sebslight.
|
||||||
|
- Discord: prevent duplicate media delivery when the model uses the `message send` tool with media, by skipping media extraction from messaging tool results since the tool already sent the message directly. (#18270)
|
||||||
- Telegram: keep draft-stream preview replies attached to the user message for `replyToMode: "all"` in groups and DMs, preserving threaded reply context from preview through finalization. (#17880) Thanks @yinghaosang.
|
- Telegram: keep draft-stream preview replies attached to the user message for `replyToMode: "all"` in groups and DMs, preserving threaded reply context from preview through finalization. (#17880) Thanks @yinghaosang.
|
||||||
- Telegram: prevent streaming final replies from being overwritten by later final/error payloads, and suppress fallback tool-error warnings when a recovered assistant answer already exists after tool calls. (#17883) Thanks @Marvae and @obviyus.
|
- Telegram: prevent streaming final replies from being overwritten by later final/error payloads, and suppress fallback tool-error warnings when a recovered assistant answer already exists after tool calls. (#17883) Thanks @Marvae and @obviyus.
|
||||||
- Telegram: disable block streaming when `channels.telegram.streamMode` is `off`, preventing newline/content-block replies from splitting into multiple messages. (#17679) Thanks @saivarunk.
|
- Telegram: disable block streaming when `channels.telegram.streamMode` is `off`, preventing newline/content-block replies from splitting into multiple messages. (#17679) Thanks @saivarunk.
|
||||||
|
|||||||
@@ -1,13 +1,26 @@
|
|||||||
|
import type { AgentEvent } from "@mariozechner/pi-agent-core";
|
||||||
import { describe, expect, it, vi } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
import type { MessagingToolSend } from "./pi-embedded-messaging.js";
|
||||||
|
import type {
|
||||||
|
ToolCallSummary,
|
||||||
|
ToolHandlerContext,
|
||||||
|
} from "./pi-embedded-subscribe.handlers.types.js";
|
||||||
import {
|
import {
|
||||||
handleToolExecutionEnd,
|
handleToolExecutionEnd,
|
||||||
handleToolExecutionStart,
|
handleToolExecutionStart,
|
||||||
} from "./pi-embedded-subscribe.handlers.tools.js";
|
} from "./pi-embedded-subscribe.handlers.tools.js";
|
||||||
|
|
||||||
function createTestContext() {
|
type ToolExecutionStartEvent = Extract<AgentEvent, { type: "tool_execution_start" }>;
|
||||||
|
type ToolExecutionEndEvent = Extract<AgentEvent, { type: "tool_execution_end" }>;
|
||||||
|
|
||||||
|
function createTestContext(): {
|
||||||
|
ctx: ToolHandlerContext;
|
||||||
|
warn: ReturnType<typeof vi.fn>;
|
||||||
|
onBlockReplyFlush: ReturnType<typeof vi.fn>;
|
||||||
|
} {
|
||||||
const onBlockReplyFlush = vi.fn();
|
const onBlockReplyFlush = vi.fn();
|
||||||
const warn = vi.fn();
|
const warn = vi.fn();
|
||||||
const ctx = {
|
const ctx: ToolHandlerContext = {
|
||||||
params: {
|
params: {
|
||||||
runId: "run-test",
|
runId: "run-test",
|
||||||
onBlockReplyFlush,
|
onBlockReplyFlush,
|
||||||
@@ -21,20 +34,24 @@ function createTestContext() {
|
|||||||
warn,
|
warn,
|
||||||
},
|
},
|
||||||
state: {
|
state: {
|
||||||
toolMetaById: new Map<string, string | undefined>(),
|
toolMetaById: new Map<string, ToolCallSummary>(),
|
||||||
|
toolMetas: [],
|
||||||
toolSummaryById: new Set<string>(),
|
toolSummaryById: new Set<string>(),
|
||||||
pendingMessagingTargets: new Map<string, unknown>(),
|
pendingMessagingTargets: new Map<string, MessagingToolSend>(),
|
||||||
pendingMessagingTexts: new Map<string, string>(),
|
pendingMessagingTexts: new Map<string, string>(),
|
||||||
|
pendingMessagingMediaUrls: new Map<string, string>(),
|
||||||
messagingToolSentTexts: [],
|
messagingToolSentTexts: [],
|
||||||
messagingToolSentTextsNormalized: [],
|
messagingToolSentTextsNormalized: [],
|
||||||
|
messagingToolSentMediaUrls: [],
|
||||||
messagingToolSentTargets: [],
|
messagingToolSentTargets: [],
|
||||||
successfulCronAdds: 0,
|
successfulCronAdds: 0,
|
||||||
toolMetas: [],
|
|
||||||
},
|
},
|
||||||
shouldEmitToolResult: () => false,
|
shouldEmitToolResult: () => false,
|
||||||
|
shouldEmitToolOutput: () => false,
|
||||||
emitToolSummary: vi.fn(),
|
emitToolSummary: vi.fn(),
|
||||||
|
emitToolOutput: vi.fn(),
|
||||||
trimMessagingToolSent: vi.fn(),
|
trimMessagingToolSent: vi.fn(),
|
||||||
} as const;
|
};
|
||||||
|
|
||||||
return { ctx, warn, onBlockReplyFlush };
|
return { ctx, warn, onBlockReplyFlush };
|
||||||
}
|
}
|
||||||
@@ -43,15 +60,14 @@ describe("handleToolExecutionStart read path checks", () => {
|
|||||||
it("does not warn when read tool uses file_path alias", async () => {
|
it("does not warn when read tool uses file_path alias", async () => {
|
||||||
const { ctx, warn, onBlockReplyFlush } = createTestContext();
|
const { ctx, warn, onBlockReplyFlush } = createTestContext();
|
||||||
|
|
||||||
await handleToolExecutionStart(
|
const evt: ToolExecutionStartEvent = {
|
||||||
ctx as never,
|
type: "tool_execution_start",
|
||||||
{
|
toolName: "read",
|
||||||
type: "tool_execution_start",
|
toolCallId: "tool-1",
|
||||||
toolName: "read",
|
args: { file_path: "/tmp/example.txt" },
|
||||||
toolCallId: "tool-1",
|
};
|
||||||
args: { file_path: "/tmp/example.txt" },
|
|
||||||
} as never,
|
await handleToolExecutionStart(ctx, evt);
|
||||||
);
|
|
||||||
|
|
||||||
expect(onBlockReplyFlush).toHaveBeenCalledTimes(1);
|
expect(onBlockReplyFlush).toHaveBeenCalledTimes(1);
|
||||||
expect(warn).not.toHaveBeenCalled();
|
expect(warn).not.toHaveBeenCalled();
|
||||||
@@ -60,15 +76,14 @@ describe("handleToolExecutionStart read path checks", () => {
|
|||||||
it("warns when read tool has neither path nor file_path", async () => {
|
it("warns when read tool has neither path nor file_path", async () => {
|
||||||
const { ctx, warn } = createTestContext();
|
const { ctx, warn } = createTestContext();
|
||||||
|
|
||||||
await handleToolExecutionStart(
|
const evt: ToolExecutionStartEvent = {
|
||||||
ctx as never,
|
type: "tool_execution_start",
|
||||||
{
|
toolName: "read",
|
||||||
type: "tool_execution_start",
|
toolCallId: "tool-2",
|
||||||
toolName: "read",
|
args: {},
|
||||||
toolCallId: "tool-2",
|
};
|
||||||
args: {},
|
|
||||||
} as never,
|
await handleToolExecutionStart(ctx, evt);
|
||||||
);
|
|
||||||
|
|
||||||
expect(warn).toHaveBeenCalledTimes(1);
|
expect(warn).toHaveBeenCalledTimes(1);
|
||||||
expect(String(warn.mock.calls[0]?.[0] ?? "")).toContain("read tool called without path");
|
expect(String(warn.mock.calls[0]?.[0] ?? "")).toContain("read tool called without path");
|
||||||
@@ -128,3 +143,127 @@ describe("handleToolExecutionEnd cron.add commitment tracking", () => {
|
|||||||
expect(ctx.state.successfulCronAdds).toBe(0);
|
expect(ctx.state.successfulCronAdds).toBe(0);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("messaging tool media URL tracking", () => {
|
||||||
|
it("tracks mediaUrl arg from messaging tool as pending", async () => {
|
||||||
|
const { ctx } = createTestContext();
|
||||||
|
|
||||||
|
const evt: ToolExecutionStartEvent = {
|
||||||
|
type: "tool_execution_start",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-m1",
|
||||||
|
args: { action: "send", to: "channel:123", content: "hi", mediaUrl: "file:///img.jpg" },
|
||||||
|
};
|
||||||
|
|
||||||
|
await handleToolExecutionStart(ctx, evt);
|
||||||
|
|
||||||
|
expect(ctx.state.pendingMessagingMediaUrls.get("tool-m1")).toBe("file:///img.jpg");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("commits pending media URL on tool success", async () => {
|
||||||
|
const { ctx } = createTestContext();
|
||||||
|
|
||||||
|
// Simulate start
|
||||||
|
const startEvt: ToolExecutionStartEvent = {
|
||||||
|
type: "tool_execution_start",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-m2",
|
||||||
|
args: { action: "send", to: "channel:123", content: "hi", mediaUrl: "file:///img.jpg" },
|
||||||
|
};
|
||||||
|
|
||||||
|
await handleToolExecutionStart(ctx, startEvt);
|
||||||
|
|
||||||
|
// Simulate successful end
|
||||||
|
const endEvt: ToolExecutionEndEvent = {
|
||||||
|
type: "tool_execution_end",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-m2",
|
||||||
|
isError: false,
|
||||||
|
result: { ok: true },
|
||||||
|
};
|
||||||
|
|
||||||
|
await handleToolExecutionEnd(ctx, endEvt);
|
||||||
|
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls).toContain("file:///img.jpg");
|
||||||
|
expect(ctx.state.pendingMessagingMediaUrls.has("tool-m2")).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("trims messagingToolSentMediaUrls to 200 on commit (FIFO)", async () => {
|
||||||
|
const { ctx } = createTestContext();
|
||||||
|
|
||||||
|
// Replace mock with a real trim that replicates production cap logic.
|
||||||
|
const MAX = 200;
|
||||||
|
ctx.trimMessagingToolSent = () => {
|
||||||
|
if (ctx.state.messagingToolSentTexts.length > MAX) {
|
||||||
|
const overflow = ctx.state.messagingToolSentTexts.length - MAX;
|
||||||
|
ctx.state.messagingToolSentTexts.splice(0, overflow);
|
||||||
|
ctx.state.messagingToolSentTextsNormalized.splice(0, overflow);
|
||||||
|
}
|
||||||
|
if (ctx.state.messagingToolSentTargets.length > MAX) {
|
||||||
|
const overflow = ctx.state.messagingToolSentTargets.length - MAX;
|
||||||
|
ctx.state.messagingToolSentTargets.splice(0, overflow);
|
||||||
|
}
|
||||||
|
if (ctx.state.messagingToolSentMediaUrls.length > MAX) {
|
||||||
|
const overflow = ctx.state.messagingToolSentMediaUrls.length - MAX;
|
||||||
|
ctx.state.messagingToolSentMediaUrls.splice(0, overflow);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
// Pre-fill with 200 URLs (url-0 .. url-199)
|
||||||
|
for (let i = 0; i < 200; i++) {
|
||||||
|
ctx.state.messagingToolSentMediaUrls.push(`file:///img-${i}.jpg`);
|
||||||
|
}
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls).toHaveLength(200);
|
||||||
|
|
||||||
|
// Commit one more via start → end
|
||||||
|
const startEvt: ToolExecutionStartEvent = {
|
||||||
|
type: "tool_execution_start",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-cap",
|
||||||
|
args: { action: "send", to: "channel:123", content: "hi", mediaUrl: "file:///img-new.jpg" },
|
||||||
|
};
|
||||||
|
await handleToolExecutionStart(ctx, startEvt);
|
||||||
|
|
||||||
|
const endEvt: ToolExecutionEndEvent = {
|
||||||
|
type: "tool_execution_end",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-cap",
|
||||||
|
isError: false,
|
||||||
|
result: { ok: true },
|
||||||
|
};
|
||||||
|
await handleToolExecutionEnd(ctx, endEvt);
|
||||||
|
|
||||||
|
// Should be capped at 200, oldest removed, newest appended.
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls).toHaveLength(200);
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls[0]).toBe("file:///img-1.jpg");
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls[199]).toBe("file:///img-new.jpg");
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls).not.toContain("file:///img-0.jpg");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("discards pending media URL on tool error", async () => {
|
||||||
|
const { ctx } = createTestContext();
|
||||||
|
|
||||||
|
const startEvt: ToolExecutionStartEvent = {
|
||||||
|
type: "tool_execution_start",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-m3",
|
||||||
|
args: { action: "send", to: "channel:123", content: "hi", mediaUrl: "file:///img.jpg" },
|
||||||
|
};
|
||||||
|
|
||||||
|
await handleToolExecutionStart(ctx, startEvt);
|
||||||
|
|
||||||
|
const endEvt: ToolExecutionEndEvent = {
|
||||||
|
type: "tool_execution_end",
|
||||||
|
toolName: "message",
|
||||||
|
toolCallId: "tool-m3",
|
||||||
|
isError: true,
|
||||||
|
result: "Error: failed",
|
||||||
|
};
|
||||||
|
|
||||||
|
await handleToolExecutionEnd(ctx, endEvt);
|
||||||
|
|
||||||
|
expect(ctx.state.messagingToolSentMediaUrls).toHaveLength(0);
|
||||||
|
expect(ctx.state.pendingMessagingMediaUrls.has("tool-m3")).toBe(false);
|
||||||
|
>>>>>>> 018297172 (test(media-dedup): add missing coverage for Discord media dedup wiring)
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
46
src/auto-reply/reply/agent-runner-payloads.test.ts
Normal file
46
src/auto-reply/reply/agent-runner-payloads.test.ts
Normal file
@@ -0,0 +1,46 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { buildReplyPayloads } from "./agent-runner-payloads.js";
|
||||||
|
|
||||||
|
const baseParams = {
|
||||||
|
isHeartbeat: false,
|
||||||
|
didLogHeartbeatStrip: false,
|
||||||
|
blockStreamingEnabled: false,
|
||||||
|
blockReplyPipeline: null,
|
||||||
|
replyToMode: "off" as const,
|
||||||
|
};
|
||||||
|
|
||||||
|
describe("buildReplyPayloads media filter integration", () => {
|
||||||
|
it("strips media URL from payload when in messagingToolSentMediaUrls", () => {
|
||||||
|
const { replyPayloads } = buildReplyPayloads({
|
||||||
|
...baseParams,
|
||||||
|
payloads: [{ text: "hello", mediaUrl: "file:///tmp/photo.jpg" }],
|
||||||
|
messagingToolSentMediaUrls: ["file:///tmp/photo.jpg"],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(replyPayloads).toHaveLength(1);
|
||||||
|
expect(replyPayloads[0].mediaUrl).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves media URL when not in messagingToolSentMediaUrls", () => {
|
||||||
|
const { replyPayloads } = buildReplyPayloads({
|
||||||
|
...baseParams,
|
||||||
|
payloads: [{ text: "hello", mediaUrl: "file:///tmp/photo.jpg" }],
|
||||||
|
messagingToolSentMediaUrls: ["file:///tmp/other.jpg"],
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(replyPayloads).toHaveLength(1);
|
||||||
|
expect(replyPayloads[0].mediaUrl).toBe("file:///tmp/photo.jpg");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("applies media filter after text filter", () => {
|
||||||
|
const { replyPayloads } = buildReplyPayloads({
|
||||||
|
...baseParams,
|
||||||
|
payloads: [{ text: "hello world!", mediaUrl: "file:///tmp/photo.jpg" }],
|
||||||
|
messagingToolSentTexts: ["hello world!"],
|
||||||
|
messagingToolSentMediaUrls: ["file:///tmp/photo.jpg"],
|
||||||
|
});
|
||||||
|
|
||||||
|
// Text filter removes the payload entirely (text matched), so nothing remains.
|
||||||
|
expect(replyPayloads).toHaveLength(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -257,6 +257,47 @@ describe("createFollowupRunner messaging tool dedupe", () => {
|
|||||||
expect(onBlockReply).not.toHaveBeenCalled();
|
expect(onBlockReply).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("drops media URL from payload when messaging tool already sent it", async () => {
|
||||||
|
const onBlockReply = vi.fn(async () => {});
|
||||||
|
runEmbeddedPiAgentMock.mockResolvedValueOnce({
|
||||||
|
payloads: [{ mediaUrl: "/tmp/img.png" }],
|
||||||
|
messagingToolSentMediaUrls: ["/tmp/img.png"],
|
||||||
|
meta: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
const runner = createFollowupRunner({
|
||||||
|
opts: { onBlockReply },
|
||||||
|
typing: createMockTypingController(),
|
||||||
|
typingMode: "instant",
|
||||||
|
defaultModel: "anthropic/claude-opus-4-5",
|
||||||
|
});
|
||||||
|
|
||||||
|
await runner(baseQueuedRun());
|
||||||
|
|
||||||
|
// Media stripped → payload becomes non-renderable → not delivered.
|
||||||
|
expect(onBlockReply).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("delivers media payload when not a duplicate", async () => {
|
||||||
|
const onBlockReply = vi.fn(async () => {});
|
||||||
|
runEmbeddedPiAgentMock.mockResolvedValueOnce({
|
||||||
|
payloads: [{ mediaUrl: "/tmp/img.png" }],
|
||||||
|
messagingToolSentMediaUrls: ["/tmp/other.png"],
|
||||||
|
meta: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
const runner = createFollowupRunner({
|
||||||
|
opts: { onBlockReply },
|
||||||
|
typing: createMockTypingController(),
|
||||||
|
typingMode: "instant",
|
||||||
|
defaultModel: "anthropic/claude-opus-4-5",
|
||||||
|
});
|
||||||
|
|
||||||
|
await runner(baseQueuedRun());
|
||||||
|
|
||||||
|
expect(onBlockReply).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
it("persists usage even when replies are suppressed", async () => {
|
it("persists usage even when replies are suppressed", async () => {
|
||||||
const storePath = path.join(
|
const storePath = path.join(
|
||||||
await fs.mkdtemp(path.join(tmpdir(), "openclaw-followup-usage-")),
|
await fs.mkdtemp(path.join(tmpdir(), "openclaw-followup-usage-")),
|
||||||
|
|||||||
Reference in New Issue
Block a user