Skip to content

Fix: Improve bash tool timeout and output handling - #1811

Closed
xhuw wants to merge 1 commit into
anomalyco:devfrom
xhuw:bash-timeout-and-output-handling
Closed

xhuw wants to merge 1 commit into
anomalyco:devfrom
xhuw:bash-timeout-and-output-handling

Conversation

@xhuw

@xhuw xhuw commented Aug 11, 2025 •

Copy link
Copy Markdown

My attempt to resolve #1721

2 key changes:

  1. Detects timeouts and report to the LLM
  2. create separate output for the LLM which identifies the stdout, stderr, exitCode and timeout

10 minute max timeout is still too short. I am not sure why its there though so not changing it

@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch 4 times, most recently from b6f08d6 to 191dc02 Compare August 12, 2025 18:11

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGENTS.md Guidelines Review

I've reviewed this PR against the guidelines in AGENTS.md and found several suggestions for improvement. These are suggestions to help maintain code consistency - you can decide whether to address them.

Main Violations Found:

  1. Multiple let statements (lines 133, 134, 136, 137, 147, 157, 193) - The guidelines recommend avoiding let statements where possible. Consider using const with objects to track mutable state.

  2. Use of else if statements (lines 195-202) - The guidelines recommend avoiding else statements unless necessary.

Suggested Improvements:

For the mutable variables:

const terminatedState = { terminated: false, byTimeout: true }
const outputState = { stderr: "", stdout: "" }  
const sizeState = { stdout: 0, stderr: 0 }

For the conditional logic:

const baseOutput = `<stdout>${stdout}</stdout>\n<stderr>${stderr}</stderr>\n<exitCode>${process.exitCode}</exitCode>`

const timeoutMessage = terminated && terminatedByTimeout 
  ? `\n<timeout>Process was terminated by timeout after ${timeout}ms...</timeout>`
  : ''

const truncatedMessage = terminated && (stdoutSize >= MAX_OUTPUT_LENGTH || stderrSize >= MAX_OUTPUT_LENGTH) && !terminatedByTimeout
  ? `\n<outputTruncated>Output was truncated to ${MAX_OUTPUT_LENGTH} characters...</outputTruncated>`
  : ''

const forLLM = baseOutput + timeoutMessage + truncatedMessage

These are suggestions - feel free to keep the current approach if it works better for your use case!

@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch 3 times, most recently from e9efcbe to 0a52eb8 Compare August 13, 2025 07:51
@xhuw

xhuw commented Aug 13, 2025

Copy link
Copy Markdown
Author

In response to the bot above:

  1. I think let makes sense as they are simple values which I am mutating. not a TS expert though
  2. else is required here as the only way to determine the difference between maxBuffer error and timeout error is if the output size is less than maxBuffer. hence else

@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch 4 times, most recently from b80f207 to eae2424 Compare August 15, 2025 22:34
@xhuw xhuw changed the title Improve bash tool timeout and output handling Fix: Improve bash tool timeout and output handling Aug 15, 2025
@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch from eae2424 to 4ffd9ed Compare August 15, 2025 23:04
@xhuw xhuw mentioned this pull request Aug 18, 2025
@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch 5 times, most recently from 1c05edf to 02cd7ad Compare August 26, 2025 12:22
@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch from 02cd7ad to d106ad1 Compare August 29, 2025 07:22
@xhuw
xhuw force-pushed the bash-timeout-and-output-handling branch from d106ad1 to 21aa7c8 Compare September 17, 2025 07:29
@lukidoescode

Copy link
Copy Markdown

My own two cents

Background

I am working on an Ansible repo right now and some of my playbook runs can easily exceed 30 minutes for the test runs when spinning up a cluster and provisioning it.

Observations

Latest version of opencode: 0.9.9

Currently what happens when I deploy a playbook is that after a while (I have not measured the duration), the bash tool returns. But the process is still running in the background, it is not being timed out correctly. Only the tool call times out. The output is being cut off.

My current workaround is to tell the agent to stream the output into a log file and to periodically check whether the process is still running (ps) and when it is done to analyze the log file.

Claude Code will kill the process after the bash tool times out. It also allows the LLM to set a parameter to override the default timeout, which I find to be a good solution, since I can just tell the agent which duration to expect with which type of command.

10 minute max timeout is still too short. I am not sure why its there though so not changing it

The projects everyone is working on are so diverse that I do not think a hardcoded maximum is valuable at all, maybe as a config option to set:

  • A Rust compilation might take minutes.
  • A small project test suite might take a few seconds.
  • An Ansible playbook might take an hour.
  • Tearing down terraform infra might take 15 minutes.

A tool needs to not be in the way to be useful.

Maybe a bit off topic but you be the judge

Furthermore, for long outputs, other tools display the first and the last e.g. 50 lines and display how many lines have been truncated from the output. As the bash tool is now, it will sometimes just return due to the output having become too long. It is not very useful that way, since head and tail only print either the first or last 50 lines, not both. Also, having to work around that shortcoming uses up valuable context space.

Lmk if you wish me to make my own PR or whether you'd like to collab.

@xhuw

xhuw commented Sep 18, 2025

Copy link
Copy Markdown
Author

Hey @lukidoescode. I have since made this MCP which I use in preference to the bash tool (I disable the bash tool globally) https://github.com/xhuw/async-bash-mcp you might be interested. It runs commands asynchronously and lets the llm poll the output.

I am surprised you see that the process continues running as the "timeout" parameter is passed directly to child_process.exec, from the docs:

If timeout is greater than 0, the parent process will send the signal identified by the killSignal property (the default is 'SIGTERM') if the child process runs longer than timeout milliseconds.

so timeout should kill the process.

The issues this PR is trying to solve is the fact that with the current bash tool, the LLM wont even know that the script has timed out. I saw very odd behaviour after timeouts where it would say things like "Lets wait for it to finish runs sleep 60" even though the process has been killed.

@xhuw

xhuw commented Oct 22, 2025

Copy link
Copy Markdown
Author

closing this one as it looks like develop has improved now (see #2986)

@xhuw xhuw closed this Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bash tool often times out

2 participants