fix(tools): harden bash tool argument handling#269
Open
SuperMarioYL wants to merge 3 commits into
Open
Conversation
The bash tool computes `timeout = min(args.get("timeout") or
DEFAULT_TIMEOUT, MAX_TIMEOUT)` outside the handler's try block. The tool
schema declares `timeout` as an integer, but model providers occasionally
serialize schema-typed integers as JSON strings (e.g. "120"). In that case
`min("120", 36000)` raises an uncaught TypeError that aborts the whole
agent turn instead of returning a recoverable tool error.
Coerce the value with int() and fall back to DEFAULT_TIMEOUT on a
non-numeric input, then clamp with MAX_TIMEOUT. Behavior for valid integer
input is unchanged.
Add tests/unit/test_local_tools.py with regression coverage for string,
missing, integer, oversized, oversized-string, and non-numeric timeouts.
A non-existent `work_dir` reached `subprocess.run(cwd=...)` and surfaced a raw `FileNotFoundError` string. Check the directory up front and return an actionable tool error so the model can correct the path. Extends tests/unit/test_local_tools.py with work_dir coverage.
A negative or zero `timeout` (e.g. "-5" or "0") survived coercion and was passed straight to `subprocess.run`, which then timed out instantly with a misleading message. Fall back to DEFAULT_TIMEOUT for any non-positive value.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
_bash_handler(the bash tool) had two argument-handling defects. This PR hardens both and adds regression tests.1. A string
timeoutcrashed the agent turntimeoutis computed before the handler'stryblock:The bash tool schema declares
timeoutas an integer, but models occasionally emit schema-typed integers as JSON strings ("120").min("120", 36000)then raises an uncaughtTypeErrorthat aborts the whole turn instead of returning a recoverable tool error:The value is now coerced with
int(), falling back toDEFAULT_TIMEOUTon a non-numeric input. A non-positive value (e.g."-5"or"0") — which would otherwise makesubprocess.runtime out instantly with a misleading message — also falls back to the default.2. An invalid
work_dirsurfaced a rawOSErrorA non-existent
work_dirreachedsubprocess.run(cwd=...)and surfaced a rawFileNotFoundErrorstring. It is now validated up front with a clear, actionable message so the model can correct the path.Tests
Adds
tests/unit/test_local_tools.py(10 tests) covering string / missing / integer / oversized / oversized-string / non-numeric / non-positivetimeout, and missing / file / validwork_dir. The string-timeouttest is red onmainand green here.uv run pytest tests/unit/test_local_tools.py-> 10 passed.ruff check/ruff format --checkclean.Notes
Behavior for valid integer input is unchanged. This is the bash-tool sibling of the read-tool coercion in #110 — it touches a different function (
_bash_handler, not_read_handler) and does not overlap that PR.