From 1bc6cdd00cbbba6a5f679d147e44cf129427a61d Mon Sep 17 00:00:00 2001 From: Tarun Sukhani Date: Mon, 16 Feb 2026 08:58:04 +0800 Subject: [PATCH] fix: classify read-only exec/bash commands as non-mutating Read-only commands like find, ls, grep no longer trigger forced error messages when they exit with non-zero codes, preserving the LLM's actual response instead of replacing it with a tool error warning. Co-Authored-By: Claude Opus 4.6 --- .../run/payloads.e2e.test.ts | 22 +++ src/agents/tool-mutation.test.ts | 29 ++++ src/agents/tool-mutation.ts | 134 +++++++++++++++++- 3 files changed, 183 insertions(+), 2 deletions(-) diff --git a/src/agents/pi-embedded-runner/run/payloads.e2e.test.ts b/src/agents/pi-embedded-runner/run/payloads.e2e.test.ts index 03a982289d0..7ecccda9da9 100644 --- a/src/agents/pi-embedded-runner/run/payloads.e2e.test.ts +++ b/src/agents/pi-embedded-runner/run/payloads.e2e.test.ts @@ -184,6 +184,28 @@ describe("buildEmbeddedRunPayloads", () => { expect(payloads[0]?.text).toContain("code 1"); }); + it("suppresses exec tool errors when mutatingAction is false and assistant produced a reply", () => { + const payloads = buildPayloads({ + assistantTexts: ["I searched for PDF files but some directories were inaccessible."], + lastAssistant: makeAssistant({ + stopReason: "stop", + errorMessage: undefined, + content: [], + }), + lastToolError: { + toolName: "exec", + error: "Command exited with code 1", + mutatingAction: false, + }, + }); + + expect(payloads).toHaveLength(1); + expect(payloads[0]?.text).toBe( + "I searched for PDF files but some directories were inaccessible.", + ); + expect(payloads[0]?.isError).toBeUndefined(); + }); + it("suppresses recoverable tool errors containing 'required' for non-mutating tools", () => { const payloads = buildPayloads({ lastToolError: { toolName: "browser", error: "url required" }, diff --git a/src/agents/tool-mutation.test.ts b/src/agents/tool-mutation.test.ts index 3eb417a71b2..0d04b3c82c7 100644 --- a/src/agents/tool-mutation.test.ts +++ b/src/agents/tool-mutation.test.ts @@ -61,6 +61,35 @@ describe("tool mutation helpers", () => { ).toBe(false); }); + it("classifies read-only exec/bash commands as non-mutating", () => { + expect(isMutatingToolCall("exec", { command: "find ~ -iname '*.pdf' 2>/dev/null" })).toBe( + false, + ); + expect(isMutatingToolCall("bash", { command: "ls -la" })).toBe(false); + expect(isMutatingToolCall("exec", { command: "grep pattern file.txt" })).toBe(false); + expect(isMutatingToolCall("exec", { command: "echo hello" })).toBe(false); + expect(isMutatingToolCall("bash", { command: "cat file | grep foo" })).toBe(false); + expect(isMutatingToolCall("exec", { command: "FOO=bar find ." })).toBe(false); + expect(isMutatingToolCall("bash", { command: "/usr/bin/find . -name '*.ts'" })).toBe(false); + expect(isMutatingToolCall("exec", { command: "sudo ls /root" })).toBe(false); + expect(isMutatingToolCall("bash", { command: "time grep -r pattern src/" })).toBe(false); + expect(isMutatingToolCall("exec", { command: "jq '.name' package.json" })).toBe(false); + }); + + it("classifies mutating exec/bash commands conservatively", () => { + expect(isMutatingToolCall("exec", { command: "rm -rf /tmp/foo" })).toBe(true); + expect(isMutatingToolCall("bash", { command: "npm install" })).toBe(true); + expect(isMutatingToolCall("exec", { command: "git push origin main" })).toBe(true); + expect(isMutatingToolCall("bash", { command: "mv file1.txt file2.txt" })).toBe(true); + }); + + it("treats empty or missing exec/bash command as mutating (conservative)", () => { + expect(isMutatingToolCall("exec", {})).toBe(true); + expect(isMutatingToolCall("bash", { command: "" })).toBe(true); + expect(isMutatingToolCall("exec", { command: " " })).toBe(true); + expect(isMutatingToolCall("bash", undefined)).toBe(true); + }); + it("keeps legacy name-only mutating heuristics for payload fallback", () => { expect(isLikelyMutatingToolName("sessions_send")).toBe(true); expect(isLikelyMutatingToolName("browser_actions")).toBe(true); diff --git a/src/agents/tool-mutation.ts b/src/agents/tool-mutation.ts index 22b0e7af9d8..d3f83e55dcc 100644 --- a/src/agents/tool-mutation.ts +++ b/src/agents/tool-mutation.ts @@ -1,3 +1,5 @@ +import path from "node:path"; + const MUTATING_TOOL_NAMES = new Set([ "write", "edit", @@ -14,6 +16,131 @@ const MUTATING_TOOL_NAMES = new Set([ "session_status", ]); +const READ_ONLY_EXEC_COMMANDS = new Set([ + "find", + "locate", + "ls", + "dir", + "tree", + "cat", + "head", + "tail", + "less", + "more", + "tac", + "grep", + "egrep", + "fgrep", + "rg", + "ag", + "ack", + "wc", + "sort", + "uniq", + "cut", + "tr", + "fold", + "paste", + "column", + "diff", + "comm", + "cmp", + "which", + "whereis", + "whence", + "type", + "command", + "hash", + "file", + "stat", + "readlink", + "realpath", + "du", + "df", + "free", + "lsblk", + "date", + "cal", + "uptime", + "w", + "who", + "whoami", + "id", + "groups", + "logname", + "uname", + "hostname", + "hostnamectl", + "arch", + "nproc", + "lscpu", + "env", + "printenv", + "locale", + "echo", + "printf", + "test", + "[", + "true", + "false", + "basename", + "dirname", + "seq", + "yes", + "md5sum", + "sha256sum", + "sha1sum", + "shasum", + "cksum", + "strings", + "xxd", + "od", + "hexdump", + "jq", + "yq", + "xq", + "ps", + "pgrep", + "lsof", + "ss", + "netstat", + "dig", + "nslookup", + "host", + "ping", + "curl", + "wget", +]); + +const SKIP_PREFIXES = new Set(["sudo", "nice", "time", "env", "ionice", "strace", "ltrace"]); + +function isReadOnlyShellCommand(command: string): boolean { + if (!command) { + return false; + } + const tokens = command.split(/\s+/); + let i = 0; + // Skip env-var assignments (FOO=bar) and common prefixes + while (i < tokens.length) { + const token = tokens[i]; + if (/^[A-Za-z_][A-Za-z0-9_]*=/.test(token)) { + i++; + continue; + } + if (SKIP_PREFIXES.has(token)) { + i++; + continue; + } + break; + } + const firstCmd = tokens[i]; + if (!firstCmd) { + return false; + } + const baseName = path.basename(firstCmd); + return READ_ONLY_EXEC_COMMANDS.has(baseName); +} + const READ_ONLY_ACTIONS = new Set([ "get", "list", @@ -104,10 +231,13 @@ export function isMutatingToolCall(toolName: string, args: unknown): boolean { case "write": case "edit": case "apply_patch": - case "exec": - case "bash": case "sessions_send": return true; + case "exec": + case "bash": { + const command = typeof record?.command === "string" ? record.command.trim() : ""; + return !isReadOnlyShellCommand(command); + } case "process": return action != null && PROCESS_MUTATING_ACTIONS.has(action); case "message":