#18992: fix: suppress spurious tool error warnings for read-only exec commands

## Summary Based on #18910 by @gatewaybuddy, with fixes for the review feedback: - **Suppresses spurious `⚠️ Exec failed` warnings** for read-only commands (e.g. `grep` finding no matches, `ls` on nonexistent path) when the assistant has already addressed the error in its reply - **Clears stale `lastToolError`** at the start of each new tool execution to prevent ghost warnings leaking across tool calls - **Prunes the read-only command allowlist** to only truly read-only commands, removing ~20 commands that can mutate state: - `sed` / `awk` (in-place editing with `-i`) - `git` (commit, push, add, delete, etc.) - `python3` / `python` / `node` / `ruby` / `perl` (arbitrary code execution) - `npm` / `pnpm` / `pip` / `cargo` (package management) - `docker` / `kubectl` / `aws` / `sam` (infrastructure mutation) - `sqlite3` / `redis-cli` (database writes) - `openclaw` / `pass` / `gpg` / `ip` / `read` The original PR's allowlist would have silently suppressed error warnings for failed mutating operations like `git commit`, `docker rm`, `npm install`, etc. ## Test plan - [x] New tests for exec/bash command classification (6 tests): - Read-only commands → non-mutating - Absolute path binaries → non-mutating - Unknown commands → mutating - Previously misclassified commands (git, sed, docker, etc.) → mutating - Empty/missing commands → defaults to mutating - [x] Existing tool-mutation tests still pass (4 tests) - [x] Lint and format clean Supersedes #18910 <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds read-only command classification for `exec`/`bash` tool calls, clears stale `lastToolError` at tool execution start, and suppresses mutating tool error warnings when the assistant has already replied. - **Read-only command allowlist** (`tool-mutation.ts`): Well-curated list of ~45 truly read-only commands. However, `write`/`edit`/`apply_patch` now fall through the same switch case and depend on empty-string-not-in-set for correct classification — should be separated. Shell operators (pipes, redirects, `&&`) in command strings bypass the first-token check, which could misclassify mutating pipelines. - **Stale error clearing** (`pi-embedded-subscribe.handlers.tools.ts`): Correctly clears `lastToolError` at tool execution start to prevent ghost warnings. - **Warning suppression** (`payloads.ts`): The `!params.hasUserFacingReply` condition breaks the existing e2e test `"shows mutating tool errors even when assistant output exists"` in `payloads.e2e.test.ts:274`, which asserts the old behavior of always surfacing mutating tool errors. This test was not updated in the PR. - **Tests** (`tool-mutation.test.ts`): Good coverage of the new classification logic (6 test cases), but missing coverage for `write`/`edit`/`apply_patch` correctness after the fallthrough change. <h3>Confidence Score: 2/5</h3> - This PR likely breaks an existing e2e test and has a fragile switch fallthrough pattern that could cause regressions. - Score of 2 reflects: (1) the `payloads.ts` change breaks the existing e2e test "shows mutating tool errors even when assistant output exists" without updating it, (2) `write`/`edit`/`apply_patch` tools accidentally route through bash command parsing logic via fallthrough, and (3) shell operators in command strings bypass the read-only classification. - Pay close attention to `src/agents/pi-embedded-runner/run/payloads.ts` (breaking e2e test) and `src/agents/tool-mutation.ts` (fragile fallthrough and shell operator bypass). _{Last reviewed commit: 25f7ed1} <!-- greptile_other_comments_section --> <!-- /greptile_comment -->