fix(tool_dispatch_helpers): catch RuntimeError from Path.expanduser() - #37331
fix(tool_dispatch_helpers): catch RuntimeError from Path.expanduser()#37331alexzhu0 wants to merge 1 commit into
Conversation
`Path(...).expanduser()` raises `RuntimeError` when the tilde-prefix
does not resolve to a real user. This happens both for `~unknown_user`
and for LLM-shaped tokens like `~500-700` (used to mean "approximately
500-700").
`_extract_parallel_scope_path` runs on the hot path for every
path-scoped tool call (read_file, write_file, edit_file, search_files,
read_file_v2, ...), so a bare `Path(raw_path).expanduser()` there
crashes the whole tool invocation with the misleading error string
Error during OpenAI-compatible API call #N:
Could not determine home directory.
when an LLM passes an unresolvable tilde prefix. Same bug class as
NousResearch#29433 (subdirectory_hints), but a different call site that also needs
the exception absorbed.
Fix: wrap the `Path(raw_path).expanduser()` call in
`_extract_parallel_scope_path` with a `try/except RuntimeError`.
A path that does not name a real filesystem location returns `None`
and the caller falls back to the no-scope path.
Tests: extend `tests/agent/test_tool_dispatch_helpers.py` with a new
`TestExtractParallelScopePathTildeRobustness` class covering:
- tilde-as-approximately in tool-call path (`~500-700`)
- unknown POSIX user prefix (`~nonexistent_user/path`)
- regression guard for legitimate `~/...` paths
- non-path-scoped tools still return None
- empty / non-string path arguments are absorbed
Repro on `main` (without the fix):
>>> _extract_parallel_scope_path("read_file", {"path": "~500-700"})
RuntimeError: Could not determine home directory.
After this fix, the call returns `None` and the conversation continues.
Refs NousResearch#29433
|
Filing the first of the three follow-up fixes mentioned in my #29433 review — The fix and test coverage mirror the #29433 style:
CI: 20/20 jobs green (ruff + ty, test x6, build amd64/arm64, nix macOS+Ubuntu, Windows footguns, e2e, supply chain, attribution, common-ancestor). The other two from the review list ( Happy to rebase / squash / retitle / move the change into #29433 directly if maintainers prefer a single atomic fix — just say the word. |
|
Closing to keep my open-PR list tidy — no longer actively tracking this one. The change still stands on its own if it's wanted; feel free to cherry-pick it. |
What does this PR do?
Companion fix to PR 29433: catch
RuntimeErrorfromPath(...).expanduser()inagent/tool_dispatch_helpers._extract_parallel_scope_path, which is the path-normalization helper for every path-scoped tool call (read_file, write_file, edit_file, search_files, read_file_v2, ...)._extract_parallel_scope_pathruns on the hot path for tool dispatch. When the LLM passes an unresolvable tilde prefix infunction_args[path]— either~unknown_user/path(POSIX) or an LLM-shaped token like~500-700meaning "approximately 500-700" —Path(raw_path).expanduser()raisesRuntimeError("Could not determine home directory."), which then bubbles up through the conversation loop and is misreported as a model/API error.This is the same bug class as PR 29433 (subdirectory_hints), but a different call site that also needs the exception absorbed. PR 29433 fixes the subdirectory-hints walker; this one fixes the path-scoped tool dispatcher. Both share the same root cause:
Path.expandusercan raiseRuntimeErrorwhen the tilde prefix does not resolve to a real user.Related Issue
Refs #29433
Type of Change
Changes Made
agent/tool_dispatch_helpers.py— wrap the barePath(raw_path).expanduser()call in_extract_parallel_scope_pathwithtry/except RuntimeError. When the tilde prefix is unresolvable, the function returnsNoneand the caller falls back to the no-scope path (i.e. parallel tool calls are allowed as if there were no path constraint).tests/agent/test_tool_dispatch_helpers.py— add a newTestExtractParallelScopePathTildeRobustnessclass with five cases:test_tilde_approximately_does_not_crashtest_tilde_with_unknown_user_does_not_crashtest_legitimate_tilde_home_still_resolves(regression guard)test_non_path_scoped_tool_returns_nonetest_missing_or_empty_path_returns_noneHow to Test
Repro on
main(without the fix):(Use
TILDE-500-700to dodge the literal tilde that would crash this README; the real bug trigger is a tilde-prefixed string in a tool-call path argument.)After applying the fix:
pytest tests/agent/test_tool_dispatch_helpers.py -v # 32 passed (5 new + 27 existing) in about 2.5sWithout the fix,
test_tilde_approximately_does_not_crashandtest_tilde_with_unknown_user_does_not_crashfail withRuntimeError. With the fix, all 5 new tests pass and the 27 pre-existing tests are unaffected.Checklist
Code
pytest tests/agent/test_tool_dispatch_helpers.py tests/agent/test_subdirectory_hints.py -qand all 57 tests passDocumentation and Housekeeping