Files
openclaw/src/agents/pi-embedded-subscribe.handlers.tools.test.ts
Gustavo Madeira Santana 0ef9383487 fix(approvals): make exec approval fallback guidance channel-specific (#61424)
Merged via squash.

Prepared head SHA: cb5d3c249c
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com>
Reviewed-by: @gumadeiras
2026-04-05 14:26:46 -04:00

800 lines
23 KiB
TypeScript

import type { AgentEvent } from "@mariozechner/pi-agent-core";
import { describe, expect, it, vi } from "vitest";
import type { MessagingToolSend } from "./pi-embedded-messaging.js";
import {
handleToolExecutionEnd,
handleToolExecutionStart,
} from "./pi-embedded-subscribe.handlers.tools.js";
import type {
ToolCallSummary,
ToolHandlerContext,
} from "./pi-embedded-subscribe.handlers.types.js";
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>;
onAgentEvent: ReturnType<typeof vi.fn>;
} {
const onBlockReplyFlush = vi.fn();
const onAgentEvent = vi.fn();
const warn = vi.fn();
const ctx: ToolHandlerContext = {
params: {
runId: "run-test",
onBlockReplyFlush,
onAgentEvent,
onToolResult: undefined,
},
flushBlockReplyBuffer: vi.fn(),
hookRunner: undefined,
log: {
debug: vi.fn(),
warn,
},
state: {
toolMetaById: new Map<string, ToolCallSummary>(),
toolMetas: [],
toolSummaryById: new Set<string>(),
itemActiveIds: new Set<string>(),
itemStartedCount: 0,
itemCompletedCount: 0,
pendingMessagingTargets: new Map<string, MessagingToolSend>(),
pendingMessagingTexts: new Map<string, string>(),
pendingMessagingMediaUrls: new Map<string, string[]>(),
pendingToolMediaUrls: [],
pendingToolAudioAsVoice: false,
messagingToolSentTexts: [],
messagingToolSentTextsNormalized: [],
messagingToolSentMediaUrls: [],
messagingToolSentTargets: [],
successfulCronAdds: 0,
deterministicApprovalPromptSent: false,
},
shouldEmitToolResult: () => false,
shouldEmitToolOutput: () => false,
emitToolSummary: vi.fn(),
emitToolOutput: vi.fn(),
trimMessagingToolSent: vi.fn(),
};
return { ctx, warn, onBlockReplyFlush, onAgentEvent };
}
describe("handleToolExecutionStart read path checks", () => {
it("does not warn when read tool uses file_path alias", async () => {
const { ctx, warn, onBlockReplyFlush } = createTestContext();
const evt: ToolExecutionStartEvent = {
type: "tool_execution_start",
toolName: "read",
toolCallId: "tool-1",
args: { file_path: "/tmp/example.txt" },
};
await handleToolExecutionStart(ctx, evt);
expect(onBlockReplyFlush).toHaveBeenCalledTimes(1);
expect(warn).not.toHaveBeenCalled();
});
it("warns when read tool has neither path nor file_path", async () => {
const { ctx, warn } = createTestContext();
const evt: ToolExecutionStartEvent = {
type: "tool_execution_start",
toolName: "read",
toolCallId: "tool-2",
args: {},
};
await handleToolExecutionStart(ctx, evt);
expect(warn).toHaveBeenCalledTimes(1);
expect(String(warn.mock.calls[0]?.[0] ?? "")).toContain("read tool called without path");
});
it("awaits onBlockReplyFlush before continuing tool start processing", async () => {
const { ctx, onBlockReplyFlush } = createTestContext();
let releaseFlush: (() => void) | undefined;
onBlockReplyFlush.mockImplementation(
() =>
new Promise<void>((resolve) => {
releaseFlush = resolve;
}),
);
const evt: ToolExecutionStartEvent = {
type: "tool_execution_start",
toolName: "exec",
toolCallId: "tool-await-flush",
args: { command: "echo hi" },
};
const pending = handleToolExecutionStart(ctx, evt);
// Let the async function reach the awaited flush Promise.
await Promise.resolve();
// If flush isn't awaited, tool metadata would already be recorded here.
expect(ctx.state.toolMetaById.has("tool-await-flush")).toBe(false);
expect(releaseFlush).toBeTypeOf("function");
releaseFlush?.();
await pending;
expect(ctx.state.toolMetaById.has("tool-await-flush")).toBe(true);
expect(ctx.state.itemStartedCount).toBe(2);
expect(ctx.state.itemActiveIds.has("tool:tool-await-flush")).toBe(true);
expect(ctx.state.itemActiveIds.has("command:tool-await-flush")).toBe(true);
});
});
describe("handleToolExecutionEnd cron.add commitment tracking", () => {
it("increments successfulCronAdds when cron add succeeds", async () => {
const { ctx } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "cron",
toolCallId: "tool-cron-1",
args: { action: "add", job: { name: "reminder" } },
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "cron",
toolCallId: "tool-cron-1",
isError: false,
result: { details: { status: "ok" } },
} as never,
);
expect(ctx.state.successfulCronAdds).toBe(1);
});
it("does not increment successfulCronAdds when cron add fails", async () => {
const { ctx } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "cron",
toolCallId: "tool-cron-2",
args: { action: "add", job: { name: "reminder" } },
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "cron",
toolCallId: "tool-cron-2",
isError: true,
result: { details: { status: "error" } },
} as never,
);
expect(ctx.state.successfulCronAdds).toBe(0);
expect(ctx.state.itemCompletedCount).toBe(1);
expect(ctx.state.itemActiveIds.size).toBe(0);
});
});
describe("handleToolExecutionEnd mutating failure recovery", () => {
it("clears edit failure when the retry succeeds through common file path aliases", async () => {
const { ctx } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "edit",
toolCallId: "tool-edit-1",
args: {
file_path: "/tmp/demo.txt",
old_string: "beta stale",
new_string: "beta fixed",
},
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "edit",
toolCallId: "tool-edit-1",
isError: true,
result: { error: "Could not find the exact text in /tmp/demo.txt" },
} as never,
);
expect(ctx.state.lastToolError?.toolName).toBe("edit");
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "edit",
toolCallId: "tool-edit-2",
args: {
file: "/tmp/demo.txt",
oldText: "beta",
newText: "beta fixed",
},
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "edit",
toolCallId: "tool-edit-2",
isError: false,
result: { ok: true },
} as never,
);
expect(ctx.state.lastToolError).toBeUndefined();
});
});
describe("handleToolExecutionEnd timeout metadata", () => {
it("records timeout metadata for failed exec results", async () => {
const { ctx } = createTestContext();
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-timeout",
isError: true,
result: {
content: [
{
type: "text",
text: "Command timed out after 1800 seconds.",
},
],
details: {
status: "failed",
timedOut: true,
exitCode: null,
durationMs: 1_800_000,
aggregated: "",
},
},
} as never,
);
expect(ctx.state.lastToolError).toMatchObject({
toolName: "exec",
timedOut: true,
});
});
});
describe("handleToolExecutionEnd exec approval prompts", () => {
it("emits a deterministic approval payload and marks assistant output suppressed", async () => {
const { ctx } = createTestContext();
const onToolResult = vi.fn();
ctx.params.onToolResult = onToolResult;
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-approval",
isError: false,
result: {
details: {
status: "approval-pending",
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
expiresAtMs: 1_800_000_000_000,
host: "gateway",
command: "npm view diver name version description",
cwd: "/tmp/work",
warningText: "Warning: heredoc execution requires explicit approval in allowlist mode.",
},
},
} as never,
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.stringContaining("```txt\n/approve 12345678 allow-once\n```"),
channelData: {
execApproval: expect.objectContaining({
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
approvalKind: "exec",
allowedDecisions: ["allow-once", "allow-always", "deny"],
}),
},
interactive: expect.objectContaining({
blocks: expect.any(Array),
}),
}),
);
expect(ctx.state.deterministicApprovalPromptSent).toBe(true);
});
it("preserves filtered approval decisions from tool details", async () => {
const { ctx } = createTestContext();
const onToolResult = vi.fn();
ctx.params.onToolResult = onToolResult;
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-approval-ask-always",
isError: false,
result: {
details: {
status: "approval-pending",
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
expiresAtMs: 1_800_000_000_000,
allowedDecisions: ["allow-once", "deny"],
host: "gateway",
command: "npm view diver name version description",
},
},
} as never,
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.not.stringContaining("allow-always"),
channelData: {
execApproval: expect.objectContaining({
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
approvalKind: "exec",
allowedDecisions: ["allow-once", "deny"],
}),
},
interactive: expect.objectContaining({
blocks: expect.any(Array),
}),
}),
);
});
it("emits a deterministic unavailable payload when the initiating surface cannot approve", async () => {
const { ctx } = createTestContext();
const onToolResult = vi.fn();
ctx.params.onToolResult = onToolResult;
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-unavailable",
isError: false,
result: {
details: {
status: "approval-unavailable",
reason: "initiating-platform-disabled",
channel: "discord",
channelLabel: "Discord",
accountId: "work",
},
},
} as never,
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.stringContaining("native chat exec approvals are not configured on Discord"),
}),
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.not.stringContaining("/approve"),
}),
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.not.stringContaining("Pending command:"),
}),
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.not.stringContaining("Host:"),
}),
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: expect.not.stringContaining("CWD:"),
}),
);
expect(ctx.state.deterministicApprovalPromptSent).toBe(true);
});
it("emits the shared approver-DM notice when another approval client received the request", async () => {
const { ctx } = createTestContext();
const onToolResult = vi.fn();
ctx.params.onToolResult = onToolResult;
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-unavailable-dm-redirect",
isError: false,
result: {
details: {
status: "approval-unavailable",
reason: "initiating-platform-disabled",
channelLabel: "Telegram",
sentApproverDms: true,
},
},
} as never,
);
expect(onToolResult).toHaveBeenCalledWith(
expect.objectContaining({
text: "Approval required. I sent approval DMs to the approvers for this account.",
}),
);
expect(ctx.state.deterministicApprovalPromptSent).toBe(true);
});
it("does not suppress assistant output when deterministic prompt delivery rejects", async () => {
const { ctx } = createTestContext();
ctx.params.onToolResult = vi.fn(async () => {
throw new Error("delivery failed");
});
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-approval-reject",
isError: false,
result: {
details: {
status: "approval-pending",
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
expiresAtMs: 1_800_000_000_000,
host: "gateway",
command: "npm view diver name version description",
cwd: "/tmp/work",
},
},
} as never,
);
expect(ctx.state.deterministicApprovalPromptSent).toBe(false);
});
it("emits approval + blocked command item events when exec needs approval", async () => {
const { ctx, onAgentEvent } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "exec",
toolCallId: "tool-exec-approval-events",
args: { command: "npm test" },
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-approval-events",
isError: false,
result: {
details: {
status: "approval-pending",
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
host: "gateway",
command: "npm test",
},
},
} as never,
);
expect(onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "approval",
data: expect.objectContaining({
phase: "requested",
status: "pending",
itemId: "command:tool-exec-approval-events",
approvalId: "12345678-1234-1234-1234-123456789012",
approvalSlug: "12345678",
}),
}),
);
expect(onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "item",
data: expect.objectContaining({
itemId: "command:tool-exec-approval-events",
phase: "end",
status: "blocked",
summary: "Awaiting approval before command can run.",
}),
}),
);
});
});
describe("handleToolExecutionEnd derived tool events", () => {
it("emits command output events for exec results", async () => {
const { ctx, onAgentEvent } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "exec",
toolCallId: "tool-exec-output",
args: { command: "ls" },
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "exec",
toolCallId: "tool-exec-output",
isError: false,
result: {
details: {
status: "completed",
aggregated: "README.md",
exitCode: 0,
durationMs: 10,
cwd: "/tmp/work",
},
},
} as never,
);
expect(onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "command_output",
data: expect.objectContaining({
itemId: "command:tool-exec-output",
phase: "end",
output: "README.md",
exitCode: 0,
cwd: "/tmp/work",
}),
}),
);
});
it("emits patch summary events for apply_patch results", async () => {
const { ctx, onAgentEvent } = createTestContext();
await handleToolExecutionStart(
ctx as never,
{
type: "tool_execution_start",
toolName: "apply_patch",
toolCallId: "tool-patch-summary",
args: { patch: "*** Begin Patch" },
} as never,
);
await handleToolExecutionEnd(
ctx as never,
{
type: "tool_execution_end",
toolName: "apply_patch",
toolCallId: "tool-patch-summary",
isError: false,
result: {
details: {
summary: {
added: ["a.ts"],
modified: ["b.ts"],
deleted: ["c.ts"],
},
},
},
} as never,
);
expect(onAgentEvent).toHaveBeenCalledWith(
expect.objectContaining({
stream: "patch",
data: expect.objectContaining({
itemId: "patch:tool-patch-summary",
added: ["a.ts"],
modified: ["b.ts"],
deleted: ["c.ts"],
summary: "1 added, 1 modified, 1 deleted",
}),
}),
);
});
});
describe("messaging tool media URL tracking", () => {
it("tracks media 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", media: "file:///img.jpg" },
};
await handleToolExecutionStart(ctx, evt);
expect(ctx.state.pendingMessagingMediaUrls.get("tool-m1")).toEqual(["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", media: "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("commits mediaUrls from tool result payload", async () => {
const { ctx } = createTestContext();
const startEvt: ToolExecutionStartEvent = {
type: "tool_execution_start",
toolName: "message",
toolCallId: "tool-m2b",
args: { action: "send", to: "channel:123", content: "hi" },
};
await handleToolExecutionStart(ctx, startEvt);
const endEvt: ToolExecutionEndEvent = {
type: "tool_execution_end",
toolName: "message",
toolCallId: "tool-m2b",
isError: false,
result: {
content: [
{
type: "text",
text: JSON.stringify({
mediaUrls: ["file:///img-a.jpg", "file:///img-b.jpg"],
}),
},
],
},
};
await handleToolExecutionEnd(ctx, endEvt);
expect(ctx.state.messagingToolSentMediaUrls).toEqual([
"file:///img-a.jpg",
"file:///img-b.jpg",
]);
});
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", media: "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", media: "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);
});
});