diff --git a/src/agents/pi-embedded-runner/run/payloads.ts b/src/agents/pi-embedded-runner/run/payloads.ts index 3dd4411ba31..ad5d99d0a8b 100644 --- a/src/agents/pi-embedded-runner/run/payloads.ts +++ b/src/agents/pi-embedded-runner/run/payloads.ts @@ -18,38 +18,10 @@ import { extractAssistantThinking, formatReasoningMessage, } from "../../pi-embedded-utils.js"; +import { isLikelyMutatingToolName } from "../../tool-mutation.js"; type ToolMetaEntry = { toolName: string; meta?: string }; -const MUTATING_TOOL_NAMES = new Set([ - "write", - "edit", - "apply_patch", - "exec", - "bash", - "process", - "message", - "sessions_send", - "cron", - "gateway", - "canvas", - "nodes", - "session_status", -]); - -function isLikelyMutatingTool(toolName: string): boolean { - const normalized = toolName.trim().toLowerCase(); - if (!normalized) { - return false; - } - return ( - MUTATING_TOOL_NAMES.has(normalized) || - normalized.endsWith("_actions") || - normalized.startsWith("message_") || - normalized.includes("send") - ); -} - export function buildEmbeddedRunPayloads(params: { assistantTexts: string[]; toolMetas: ToolMetaEntry[]; @@ -259,7 +231,8 @@ export function buildEmbeddedRunPayloads(params: { errorLower.includes("needs") || errorLower.includes("requires"); const isMutatingToolError = - params.lastToolError.mutatingAction ?? isLikelyMutatingTool(params.lastToolError.toolName); + params.lastToolError.mutatingAction ?? + isLikelyMutatingToolName(params.lastToolError.toolName); const shouldShowToolError = isMutatingToolError || (!hasUserFacingReply && !isRecoverableError); // Always surface mutating tool failures so we do not silently confirm actions that did not happen. diff --git a/src/agents/pi-embedded-subscribe.handlers.tools.ts b/src/agents/pi-embedded-subscribe.handlers.tools.ts index 96cfe250ffc..a733e4f147b 100644 --- a/src/agents/pi-embedded-subscribe.handlers.tools.ts +++ b/src/agents/pi-embedded-subscribe.handlers.tools.ts @@ -16,170 +16,21 @@ import { sanitizeToolResult, } from "./pi-embedded-subscribe.tools.js"; import { inferToolMetaFromArgs } from "./pi-embedded-utils.js"; +import { buildToolMutationState, isSameToolMutationAction } from "./tool-mutation.js"; import { normalizeToolName } from "./tool-policy.js"; /** Track tool execution start times and args for after_tool_call hook */ const toolStartData = new Map(); -const READ_ONLY_ACTIONS = new Set([ - "get", - "list", - "read", - "status", - "show", - "fetch", - "search", - "query", - "view", - "poll", - "log", - "inspect", - "check", - "probe", -]); -const PROCESS_MUTATING_ACTIONS = new Set(["write", "send_keys", "submit", "paste", "kill"]); -const MESSAGE_MUTATING_ACTIONS = new Set([ - "send", - "reply", - "thread_reply", - "threadreply", - "edit", - "delete", - "react", - "pin", - "unpin", -]); - -function asRecord(value: unknown): Record | undefined { - return value && typeof value === "object" ? (value as Record) : undefined; -} - -function normalizeActionName(value: unknown): string | undefined { - if (typeof value !== "string") { - return undefined; - } - const normalized = value - .trim() - .toLowerCase() - .replace(/[\s-]+/g, "_"); - return normalized || undefined; -} - -function normalizeFingerprintValue(value: unknown): string | undefined { - if (typeof value !== "string") { - return undefined; - } - const normalized = value.trim(); - return normalized ? normalized.toLowerCase() : undefined; -} - -function isMutatingToolCall(toolName: string, args: unknown): boolean { - const normalized = toolName.trim().toLowerCase(); - const record = asRecord(args); - const action = normalizeActionName(record?.action); - - switch (normalized) { - case "write": - case "edit": - case "apply_patch": - case "exec": - case "bash": - case "sessions_send": - return true; - case "process": - return action != null && PROCESS_MUTATING_ACTIONS.has(action); - case "message": - return ( - (action != null && MESSAGE_MUTATING_ACTIONS.has(action)) || - typeof record?.content === "string" || - typeof record?.message === "string" - ); - case "session_status": - return typeof record?.model === "string" && record.model.trim().length > 0; - default: { - if (normalized === "cron" || normalized === "gateway" || normalized === "canvas") { - return action == null || !READ_ONLY_ACTIONS.has(action); - } - if (normalized === "nodes") { - return action == null || action !== "list"; - } - if (normalized.endsWith("_actions")) { - return action == null || !READ_ONLY_ACTIONS.has(action); - } - if (normalized.startsWith("message_") || normalized.includes("send")) { - return true; - } - return false; - } - } -} - -function buildActionFingerprint( - toolName: string, - args: unknown, - meta?: string, -): string | undefined { - if (!isMutatingToolCall(toolName, args)) { - return undefined; - } - const normalizedTool = toolName.trim().toLowerCase(); - const record = asRecord(args); - const action = normalizeActionName(record?.action); - const parts = [`tool=${normalizedTool}`]; - if (action) { - parts.push(`action=${action}`); - } - for (const key of [ - "path", - "filePath", - "oldPath", - "newPath", - "to", - "target", - "messageId", - "sessionKey", - "jobId", - "id", - "model", - ]) { - const value = normalizeFingerprintValue(record?.[key]); - if (value) { - parts.push(`${key.toLowerCase()}=${value}`); - } - } - const normalizedMeta = meta?.trim().replace(/\s+/g, " ").toLowerCase(); - if (normalizedMeta) { - parts.push(`meta=${normalizedMeta}`); - } - return parts.join("|"); -} - function buildToolCallSummary(toolName: string, args: unknown, meta?: string): ToolCallSummary { - const actionFingerprint = buildActionFingerprint(toolName, args, meta); + const mutation = buildToolMutationState(toolName, args, meta); return { meta, - mutatingAction: actionFingerprint != null, - actionFingerprint, + mutatingAction: mutation.mutatingAction, + actionFingerprint: mutation.actionFingerprint, }; } -function sameToolAction( - existing: { toolName: string; meta?: string; actionFingerprint?: string }, - toolName: string, - meta?: string, - actionFingerprint?: string, -): boolean { - if (existing.actionFingerprint != null || actionFingerprint != null) { - // For mutating flows, fail closed: only clear when both fingerprints exist and match. - return ( - existing.actionFingerprint != null && - actionFingerprint != null && - existing.actionFingerprint === actionFingerprint - ); - } - return existing.toolName === toolName && (existing.meta ?? "") === (meta ?? ""); -} - function extendExecMeta(toolName: string, args: unknown, meta?: string): string | undefined { const normalized = toolName.trim().toLowerCase(); if (normalized !== "exec" && normalized !== "bash") { @@ -347,7 +198,13 @@ export async function handleToolExecutionEnd( } else if (ctx.state.lastToolError) { // Keep unresolved mutating failures until the same action succeeds. if (ctx.state.lastToolError.mutatingAction) { - if (sameToolAction(ctx.state.lastToolError, toolName, meta, callSummary?.actionFingerprint)) { + if ( + isSameToolMutationAction(ctx.state.lastToolError, { + toolName, + meta, + actionFingerprint: callSummary?.actionFingerprint, + }) + ) { ctx.state.lastToolError = undefined; } } else { diff --git a/src/agents/tool-mutation.test.ts b/src/agents/tool-mutation.test.ts new file mode 100644 index 00000000000..3eb417a71b2 --- /dev/null +++ b/src/agents/tool-mutation.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import { + buildToolActionFingerprint, + buildToolMutationState, + isLikelyMutatingToolName, + isMutatingToolCall, + isSameToolMutationAction, +} from "./tool-mutation.js"; + +describe("tool mutation helpers", () => { + it("treats session_status as mutating only when model override is provided", () => { + expect(isMutatingToolCall("session_status", { sessionKey: "agent:main:main" })).toBe(false); + expect( + isMutatingToolCall("session_status", { + sessionKey: "agent:main:main", + model: "openai/gpt-4o", + }), + ).toBe(true); + }); + + it("builds stable fingerprints for mutating calls and omits read-only calls", () => { + const writeFingerprint = buildToolActionFingerprint( + "write", + { path: "/tmp/demo.txt", id: 42 }, + "write /tmp/demo.txt", + ); + expect(writeFingerprint).toContain("tool=write"); + expect(writeFingerprint).toContain("path=/tmp/demo.txt"); + expect(writeFingerprint).toContain("id=42"); + expect(writeFingerprint).toContain("meta=write /tmp/demo.txt"); + + const readFingerprint = buildToolActionFingerprint("read", { path: "/tmp/demo.txt" }); + expect(readFingerprint).toBeUndefined(); + }); + + it("exposes mutation state for downstream payload rendering", () => { + expect( + buildToolMutationState("message", { action: "send", to: "telegram:1" }).mutatingAction, + ).toBe(true); + expect(buildToolMutationState("browser", { action: "list" }).mutatingAction).toBe(false); + }); + + it("matches tool actions by fingerprint and fails closed on asymmetric data", () => { + expect( + isSameToolMutationAction( + { toolName: "write", actionFingerprint: "tool=write|path=/tmp/a" }, + { toolName: "write", actionFingerprint: "tool=write|path=/tmp/a" }, + ), + ).toBe(true); + expect( + isSameToolMutationAction( + { toolName: "write", actionFingerprint: "tool=write|path=/tmp/a" }, + { toolName: "write", actionFingerprint: "tool=write|path=/tmp/b" }, + ), + ).toBe(false); + expect( + isSameToolMutationAction( + { toolName: "write", actionFingerprint: "tool=write|path=/tmp/a" }, + { toolName: "write" }, + ), + ).toBe(false); + }); + + it("keeps legacy name-only mutating heuristics for payload fallback", () => { + expect(isLikelyMutatingToolName("sessions_send")).toBe(true); + expect(isLikelyMutatingToolName("browser_actions")).toBe(true); + expect(isLikelyMutatingToolName("message_slack")).toBe(true); + expect(isLikelyMutatingToolName("browser")).toBe(false); + }); +}); diff --git a/src/agents/tool-mutation.ts b/src/agents/tool-mutation.ts new file mode 100644 index 00000000000..22b0e7af9d8 --- /dev/null +++ b/src/agents/tool-mutation.ts @@ -0,0 +1,201 @@ +const MUTATING_TOOL_NAMES = new Set([ + "write", + "edit", + "apply_patch", + "exec", + "bash", + "process", + "message", + "sessions_send", + "cron", + "gateway", + "canvas", + "nodes", + "session_status", +]); + +const READ_ONLY_ACTIONS = new Set([ + "get", + "list", + "read", + "status", + "show", + "fetch", + "search", + "query", + "view", + "poll", + "log", + "inspect", + "check", + "probe", +]); + +const PROCESS_MUTATING_ACTIONS = new Set(["write", "send_keys", "submit", "paste", "kill"]); + +const MESSAGE_MUTATING_ACTIONS = new Set([ + "send", + "reply", + "thread_reply", + "threadreply", + "edit", + "delete", + "react", + "pin", + "unpin", +]); + +export type ToolMutationState = { + mutatingAction: boolean; + actionFingerprint?: string; +}; + +export type ToolActionRef = { + toolName: string; + meta?: string; + actionFingerprint?: string; +}; + +function asRecord(value: unknown): Record | undefined { + return value && typeof value === "object" ? (value as Record) : undefined; +} + +function normalizeActionName(value: unknown): string | undefined { + if (typeof value !== "string") { + return undefined; + } + const normalized = value + .trim() + .toLowerCase() + .replace(/[\s-]+/g, "_"); + return normalized || undefined; +} + +function normalizeFingerprintValue(value: unknown): string | undefined { + if (typeof value === "string") { + const normalized = value.trim(); + return normalized ? normalized.toLowerCase() : undefined; + } + if (typeof value === "number" || typeof value === "bigint" || typeof value === "boolean") { + return String(value).toLowerCase(); + } + return undefined; +} + +export function isLikelyMutatingToolName(toolName: string): boolean { + const normalized = toolName.trim().toLowerCase(); + if (!normalized) { + return false; + } + return ( + MUTATING_TOOL_NAMES.has(normalized) || + normalized.endsWith("_actions") || + normalized.startsWith("message_") || + normalized.includes("send") + ); +} + +export function isMutatingToolCall(toolName: string, args: unknown): boolean { + const normalized = toolName.trim().toLowerCase(); + const record = asRecord(args); + const action = normalizeActionName(record?.action); + + switch (normalized) { + case "write": + case "edit": + case "apply_patch": + case "exec": + case "bash": + case "sessions_send": + return true; + case "process": + return action != null && PROCESS_MUTATING_ACTIONS.has(action); + case "message": + return ( + (action != null && MESSAGE_MUTATING_ACTIONS.has(action)) || + typeof record?.content === "string" || + typeof record?.message === "string" + ); + case "session_status": + return typeof record?.model === "string" && record.model.trim().length > 0; + default: { + if (normalized === "cron" || normalized === "gateway" || normalized === "canvas") { + return action == null || !READ_ONLY_ACTIONS.has(action); + } + if (normalized === "nodes") { + return action == null || action !== "list"; + } + if (normalized.endsWith("_actions")) { + return action == null || !READ_ONLY_ACTIONS.has(action); + } + if (normalized.startsWith("message_") || normalized.includes("send")) { + return true; + } + return false; + } + } +} + +export function buildToolActionFingerprint( + toolName: string, + args: unknown, + meta?: string, +): string | undefined { + if (!isMutatingToolCall(toolName, args)) { + return undefined; + } + const normalizedTool = toolName.trim().toLowerCase(); + const record = asRecord(args); + const action = normalizeActionName(record?.action); + const parts = [`tool=${normalizedTool}`]; + if (action) { + parts.push(`action=${action}`); + } + for (const key of [ + "path", + "filePath", + "oldPath", + "newPath", + "to", + "target", + "messageId", + "sessionKey", + "jobId", + "id", + "model", + ]) { + const value = normalizeFingerprintValue(record?.[key]); + if (value) { + parts.push(`${key.toLowerCase()}=${value}`); + } + } + const normalizedMeta = meta?.trim().replace(/\s+/g, " ").toLowerCase(); + if (normalizedMeta) { + parts.push(`meta=${normalizedMeta}`); + } + return parts.join("|"); +} + +export function buildToolMutationState( + toolName: string, + args: unknown, + meta?: string, +): ToolMutationState { + const actionFingerprint = buildToolActionFingerprint(toolName, args, meta); + return { + mutatingAction: actionFingerprint != null, + actionFingerprint, + }; +} + +export function isSameToolMutationAction(existing: ToolActionRef, next: ToolActionRef): boolean { + if (existing.actionFingerprint != null || next.actionFingerprint != null) { + // For mutating flows, fail closed: only clear when both fingerprints exist and match. + return ( + existing.actionFingerprint != null && + next.actionFingerprint != null && + existing.actionFingerprint === next.actionFingerprint + ); + } + return existing.toolName === next.toolName && (existing.meta ?? "") === (next.meta ?? ""); +}