Fix shell tool truncating real output that fit within max_output_length - #5027
arpankernel wants to merge 1 commit into
Conversation
`ShellAction.execute` truncated shell output twice against the same budget. `truncate_shell_outputs(normalized, max_output_length)` already bounds the combined stdout+stderr payload to the budget. `render_shell_outputs` then adds decoration (the `$ <command>` prefix, `stderr:`, `exit code:` lines, and blank separators), and the code re-clamped that decorated string with `output_text[:max_output_length]` — charging the decoration against the same budget and cutting real output that already fit. With a command set and a small budget the entire payload could be lost, leaving only the `$ <command>` prefix. `max_output_length` bounds the output streams (per `truncate_shell_outputs`), not the human-readable framing, so drop the second clamp for positive budgets. The only load-bearing case for it was `max_output_length == 0`, where `render_shell_outputs` emits a "(no output)" placeholder; that is preserved by collapsing to an empty string. Add a regression test.
|
One design note for reviewers, since this touches the meaning of This PR treats If you instead intend |
Summary
ShellAction.executetruncates shell output twice against the same budget, so a command whose output already fits withinmax_output_lengthcan still have real output chopped — or lost entirely.In
src/agents/run_internal/tool_actions.py(theShellResultbranch):truncate_shell_outputsalready bounds the combined stdout+stderr tomax_output_length(its docstring: "Truncate shell output streams to a maximum combined length").render_shell_outputsthen prepends the$ <command>line and other framing, so the secondoutput_text[:max_output_length]charges that framing against the same budget and cuts real output that already fit. With a command set and a small budget, the entire payload is dropped and the model sees only the$ <command>prefix.Fix
max_output_lengthbounds the output streams, not the human-readable framing, so drop the redundant second clamp for positive budgets. The only load-bearing case for it wasmax_output_length == 0, whererender_shell_outputssubstitutes a(no output)placeholder for an empty command; that is preserved by collapsing to an empty string (matching the prior behavior).Test plan
test_shell_tool_max_output_length_does_not_count_command_decoration: a command (echo hi) withstdout="0123456789"andmax_output_length=6now yields"$ echo hi\n012345"(stdout bounded to the 6-char budget, decoration not counted). Before the fix this test produces"$ echo"— the whole real stdout is lost.tests/test_shell_tool.py,tests/test_shell_call_serialization.pypass (46), including the existing zero/negativemax_output_lengthcases.ruff check,ruff format --check, andmypyare clean on the changed files.Issue number if applicable
None — found by inspection; no existing issue.
Checks
make lint/make mypyon the changed files