🤖 Save bash tool overflow output to temp file #104
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When bash tool output exceeds limits (100 lines, 16KB, or 1KB/line), save the full output to a temp file instead of failing with no output. This prevents expensive commands from needing to be re-run. Key design decisions: - NO output shown when truncated - avoids overwhelming context - Short 8-char hex file IDs (bash-a1b2c3d4.txt) for easy recall - Helpful filtering examples (grep, head, tail, sed) - Reminder to clean up temp file when done - Reduce max_lines to 100 (from 1000) to trigger overflow more reasonably Changes: - Continue collecting lines even after truncation triggered - Write full output to /tmp/bash-<8hex>.txt on overflow - Return error with file path and filtering instructions - Update tests to verify overflow file creation and format - Reduce BASH_DEFAULT_MAX_LINES and BASH_HARD_MAX_LINES to 100 _Generated with `cmux`_
ammar-agent
force-pushed
the
bash-overflow-temp-file
branch
from
b9a2eb8 to
4cff873
Compare
ammario
requested changes
Oct 8, 2025
src/services/tools/bash.ts
Outdated
Comment on lines
221
to
222
| stdoutReader.close(); | ||
| stderrReader.close(); |
Member
There was a problem hiding this comment.
would a using pattern clean up this duplication / bug risk?
Collaborator
Author
There was a problem hiding this comment.
Fixed! Added a triggerTruncation() helper function that eliminates the duplication. This ensures consistent cleanup and reduces bug risk across all 6 call sites.
…mit to 300 Changes: - Add triggerTruncation() helper to eliminate code duplication (6 instances) - Increase max_lines from 100 to 300 (better balance for common use cases) - Update overflow message examples to reflect 300 line limit - Update test to verify 300+ lines collected Addresses review comment about duplication and bug risk. _Generated with `cmux`_
Collaborator
Author
|
✅ Fixed! Added a |
- Replace dynamic import with static fs import at top of file - Use String(err) in template literal to satisfy type checker - Use RegExp.exec() instead of String.match() _Generated with `cmux`_
ammario
approved these changes
Oct 8, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When the bash tool's output exceeds the
max_lineslimit, instead of discarding all output, this PR saves it to a temporary file and includes the file path in the truncated output message.This prevents expensive commands from needing to be re-run just to see their full output.
Changes
/tmp/bash-overflow-<timestamp>.txtwhen truncation occursExample
Before:
After:
Testing
Generated with
cmux