#20382: fix: move suppressToolErrors check before mutating tool check
agents
size: XS
Cluster:
Tool Execution and Error Handling
## Summary
The `shouldShowToolErrorWarning()` function was checking for mutating tools *before* checking `suppressToolErrors`, which caused exec tool errors (and other mutating tool errors) to always bypass the suppression setting.
## Root Cause
In the current logic:
```js
if (isMutatingToolError) return true; // Always runs first for exec
if (params.suppressToolErrors) return false; // Never reached!
```
Since `exec` is in `MUTATING_TOOL_NAMES`, the function always returns `true` for exec errors, never reaching the `suppressToolErrors` check.
## Fix
Move the `suppressToolErrors` check to run *before* the mutating tool check:
```js
if (params.suppressToolErrors) return false; // Takes precedence
if (isMutatingToolError) return true;
```
## Testing
Tested with `messages.suppressToolErrors: true` — exec tool errors are now properly suppressed in Telegram.
Fixes #20140
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR reorders the condition checks in `shouldShowToolErrorWarning()` so that `suppressToolErrors` takes precedence over the mutating tool error check. While the intent is to allow suppression of `exec` tool errors via `messages.suppressToolErrors`, the change breaks a **safety-critical invariant**: mutating tool errors (from `exec`, `write`, `bash`, etc.) were intentionally shown even when `suppressToolErrors` is set, to prevent users from thinking a destructive action succeeded when it actually failed.
- The existing test `"still shows mutating tool errors when messages.suppressToolErrors is enabled"` (`payloads.e2e.test.ts:244`) directly contradicts this change and will fail.
- The codebase already has `suppressToolErrorWarnings` as the nuclear override that suppresses all errors including mutating ones — this is the correct flag for the use case described in the PR.
- A more targeted fix (e.g., a separate config option or adjusting which tools are classified as mutating) would be safer than changing the global precedence order.
<h3>Confidence Score: 1/5</h3>
- This PR breaks a safety invariant that prevents silent suppression of mutating tool errors, and will cause an existing test to fail.
- Score of 1 reflects that the change directly contradicts an existing test assertion ("still shows mutating tool errors when messages.suppressToolErrors is enabled") and breaks a documented safety invariant (line 258 comment: "Always surface mutating tool failures so we do not silently confirm actions that did not happen"). The existing `suppressToolErrorWarnings` flag already provides the nuclear override capability the PR is trying to achieve.
- `src/agents/pi-embedded-runner/run/payloads.ts` — the reordered condition checks break the intended suppression hierarchy
<sub>Last reviewed commit: c3b65ef</sub>
<!-- greptile_other_comments_section -->
<sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub>
<!-- /greptile_comment -->
Most Similar PRs
#19632: fix: suppressToolErrors now suppresses exec tool failure notifications
by Gitjay11 · 2026-02-18
89.5%
#18466: fix: suppress recoverable mutating tool errors when agent already r...
by stijnhoste · 2026-02-16
85.8%
#22087: Preserve assistant reply when exec fails under suppressToolErrors
by graysurf · 2026-02-20
85.0%
#17552: fix(agents): suppress tool error warnings when assistant already re...
by AytuncYildizli · 2026-02-15
84.5%
#18992: fix: suppress spurious tool error warnings for read-only exec commands
by Phineas1500 · 2026-02-17
83.1%
#18415: fix(agents): suppress benign exec exit code 1 from tool error surfa...
by aldoeliacim · 2026-02-16
82.9%
#19235: fix(telegram): tool error warnings no longer overwrite streamed rep...
by gatewaybuddy · 2026-02-17
81.9%
#19375: telegram: align tool-error summaries
by NorthyIE · 2026-02-17
80.1%
#18708: feat(messages): add suppressToolErrorWarnings config option
by codexGW · 2026-02-17
78.7%
#9861: fix(agents): re-run tool_use/tool_result repair after limitHistoryT...
by CyberSinister · 2026-02-05
78.6%