← Back to PRs

#22087: Preserve assistant reply when exec fails under suppressToolErrors

by graysurf open 2026-02-20 18:00 View on GitHub →
agents size: XS
## Summary - Preserve user-facing assistant replies when `messages.suppressToolErrors=true` and an `exec`/mutating tool call fails. - Add a regression test covering the `assistantTexts + exec failure + suppressToolErrors` path. ## Expected vs Actual - Expected: with `suppressToolErrors=true`, assistant reply text remains visible; tool warning should not override it. - Actual (before): mutating tool warning path could still append error warnings even when a user-facing reply already existed. ## Impact - Prevents noisy mutating-tool warnings from dominating final output in runs that already produced a user-visible answer. - Keeps existing safety behavior for no-reply cases unchanged. ## Reproduction 1. Set `messages.suppressToolErrors: true`. 2. Trigger a run that produces assistant text and also hits an `exec` failure. 3. Observe that only assistant text is kept after this patch. ## Issues Found | ID | Issue | Severity | Status | | --- | --- | --- | --- | | PR-22087-BUG-01 | `suppressToolErrors` did not suppress mutating/exec warning when assistant text already existed. | medium | fixed | ## Fix Approach - In `shouldShowToolErrorWarning`, short-circuit to `false` when both conditions are true: - `suppressToolErrors` is enabled - a user-facing assistant reply already exists - Leave mutating-tool warning behavior unchanged for no-reply runs. ## Testing - `pnpm test src/agents/pi-embedded-runner/run/payloads.test.ts` Fixes #19820 <!-- greptile_comment --> <h3>Greptile Summary</h3> Added logic to preserve assistant replies when `suppressToolErrors` is enabled and a mutating tool (like `exec`) fails. The fix prevents noisy tool error warnings from overriding user-facing assistant text when the user has explicitly requested error suppression. Key changes: - Added short-circuit condition in `shouldShowToolErrorWarning` that returns false when both `suppressToolErrors` is enabled and a user-facing reply exists - Preserves existing safety behavior for mutating tool failures when no assistant reply is present - Added regression test covering the `assistantTexts + exec failure + suppressToolErrors` scenario The implementation is clean and the logic placement is correct - it handles the specific case without breaking existing behavior for other scenarios. <h3>Confidence Score: 5/5</h3> - Safe to merge with no risk - The logic is well-placed and preserves all existing safety behaviors while fixing the specific issue. The test coverage is appropriate and verifies the fix works as intended. The change is minimal, focused, and doesn't introduce any edge cases or breaking changes. - No files require special attention <sub>Last reviewed commit: 6db89f3</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs