fix(subdirectory_hints): catch RuntimeError from Path.expanduser() - #29433
fix(subdirectory_hints): catch RuntimeError from Path.expanduser()#29433udatny wants to merge 1 commit into
Conversation
alexzhu0
left a comment
There was a problem hiding this comment.
LGTM — strong candidate for merge
Reproduced the symptom on main against openai/gpt-5.1-codex and minimax-cn/MiniMax-M3 (this session) — a terminal command with ~500-700 in a heredoc blew up the tool dispatcher with the misleading "Could not determine home directory" error string. The author's three-line fix in subdirectory_hints.py resolves it.
Strengths
- Reproduces a real LLM output pattern (
~as "approximately"), not a contrived edge case - Three test cases cover: buggy case, edge case, regression guard
- The companion fix to
Path.home()(lines 198/202) is a nice catch — same exception class affectsrelative_to(Path.home()) - Conventional Commits + clean checklist
Suggestions (non-blocking)
-
Rebase needed —
mainadvanced 5 days since this branch was cut;mergeable_state: blockedlikely from required CI checks rather than conflicts, but a rebase onto currentmainwill rule that out. -
Broader blast radius — while reading the diff I grep'd the repo for other
Path(...).expanduser()call sites that don't catchRuntimeError. Three adjacent ones worth considering for a follow-up PR (would happily file any/all if you want to keep this one focused):agent/tool_dispatch_helpers.py:158—expanded = Path(raw_path).expanduser()in_get_path_target. This is on the hot path for every path-scoped tool call (read_file/write_file/edit_file/...). Same bug class, higher impact than subdirectory_hints because the path comes straight fromfunction_args["path"].agent/skill_commands.py:63—identifier_path = Path(raw_identifier).expanduser()in slash-command resolver.raw_identifieris user-supplied.hermes_cli/gateway.py:2095-2101—Path.home()itself can raiseRuntimeError, onlyValueErroris caught.
-
Optional: a lint rule — would prevent future regressions. A simple
flake8-no-runtime-warnplugin or a one-off AST check that flags barePath(...).expanduser()not wrapped intry/except (..., RuntimeError).
Verdict
LGTM. The PR is correctly scoped, well-tested, and addresses a real production pain point that misleads users into chasing model/environment issues. Recommending merge once rebased and CI green.
|
Confirming this is the right fix — reproduced on Two things worth folding in before this lands:
Closing my duplicate #43970 in favor of this earlier PR. |
…ied, build exit-0, 210 tests pass single-worktree); review-comment walk all 41 PRs — 1 genuine duplicate (NousResearch#50049 vs NousResearch#29433) ADDRESSED on-PR + operator-deferred-close, rest are triage cross-refs affirming non-redundant
…ndle) This PR previously bundled 100 files as a "cross-PR integration regression suite", but 94 of those duplicated other open PRs — which made it the primary blocker when combining the PR set onto a later release (it conflicted on every overlapping file). **Slimmed to the 4 files genuinely unique to this PR:** ``` hermes_cli/auth.py # copilot-opus-context auth path hermes_cli/runtime_provider.py # runtime provider resolution tests/agent/conftest.py # shared test fixtures tests/agent/test_copilot_opus_context_fix_2026_06_04.py # the regression test ``` The 94 duplicate files are owned by their topical feature PRs already (autopilot NousResearch#49917, reasoning NousResearch#48024, copilot identity NousResearch#50064, etc.). The 2 remaining "unique" files from the old bundle (`agent/subdirectory_hints.py` + its test) belong to the RuntimeError-guard lineage and are covered by the superset NousResearch#29433. Built on v0.17.0 (`2bd1977d8`); all 4 files compile; 0 private-provenance leaks. Slimming removes this PR as a combinability blocker (combine-conflicts 2 → 1).
…LE) + reproducible delta map - GITHUB-MERGEABLE-AUDIT.md: GitHub mergeable=40/41 (only NousResearch#50111 manifest conflicts). 6 PRs (NousResearch#50296/NousResearch#49644/NousResearch#50041/NousResearch#50073/NousResearch#50064/NousResearch#50033) genuinely conflicted on current origin/main (drifted past v0.17.0); each rebased (1-file complementary conflict), now MERGEABLE. - DELTA-MAP-v017.md: reproducible per-file map (PR diffs vs v0.17.0, fresh tips): 160 = 137 in-PR + 21 DISCARD + 2 upstream-NousResearch#29433 + 0 orphans, sum verified.
…elta=165, 0 orphans) - diff_coverage_proof.sh + DIFF-COVERAGE-PROOF.txt: PROVEN delta(165) = in-PR(138) + 9 .bak + 12 .project-intel + 4 transcripts + 2 upstream-NousResearch#29433, 0 orphans. CORRECTION: delta is 165 (not 160); 4 transcripts/ eval-captures were unclassified orphans, now DISCARD. DISCARD total = 25. - PINNED-SHAS-API.txt: GitHub-API head SHAs (50053/50111 fixes confirmed landed). - EVIDENCE-BUNDLE.md: 41-vs-40 reconciliation + CI-gated status + 0-leak scan.
Adds the "xai" -> "xAI" entry to _LABEL_OVERRIDES so the provider list shows the proper casing. Scoped to this single label change; the subdirectory_hints.py RuntimeError guard previously bundled here is deferred to the maintainer-preferred open PR NousResearch#29433 (same fix), keeping this PR to one logical change.
…hint dup to upstream NousResearch#29433 NousResearch#50626 force-pushed to exactly hermes_cli/providers.py (+"xai":"xAI", 1 line) — its subdirectory_hints.py RuntimeError guard duplicated maintainer-preferred open NousResearch#29433 (which ships the same fix + its own test), so it's deferred there (one-PR-per-change). The 2 subdir-hint files move to a new SUPERSEDED-by-upstream coverage class. Coverage re-balances: 165 = 129 in PRs + 25 DISCARD + 9 WITHDRAWN + 2 SUPERSEDED + 0 orphans. Trimmed NousResearch#50626 verified applies-clean on v0.17.0. Maps reconciled.
974a5a1 to
2351002
Compare
|
Rebased onto current Keeping this PR scoped to
Happy to consolidate into one omnibus PR instead if maintainers prefer — let me know. |
`pathlib.Path('~user').expanduser()` raises RuntimeError when the
tilde-expansion can't resolve the user (e.g. `~500-700` where the LLM
meant "approximately 500-700" rather than a path). The hint walker's
existing `except (OSError, ValueError):` clauses do not catch
RuntimeError, so it escapes through the tool dispatcher and surfaces
in the conversation loop as a misleading
Error during OpenAI-compatible API call #N:
Could not determine home directory.
Reproduced across three unrelated models (openai/gpt-5-mini,
openai/gpt-5.1-codex, deepseek/deepseek-v4-flash) on terminal-tool
commands containing literal tildes in non-path contexts — common in
LLM output ("~500 agencies", "~45,000 CVEs", "~80/hr blended rate").
Reproduction (one-liner):
>>> from pathlib import Path
>>> Path("~500-700").expanduser()
RuntimeError: Could not determine home directory.
Fix: extend the three `except` clauses in
agent/subdirectory_hints.py to also catch RuntimeError:
line 138 (_add_path_candidate's outer catch around the Path().expanduser() call)
lines 198+202 (_load_hints_for_directory's nested catches around hint_path.relative_to(Path.home()))
Tests: tests/agent/test_subdirectory_hints_tilde.py adds three cases
covering: tilde-as-approximately in heredoc commands, ~unknown_user paths,
and a regression guard that legitimate ~/path expansion still works.
2351002 to
fef4124
Compare
|
Small follow-up: the test file as originally submitted referenced a Fixed by switching the three tests to pytest's built-in Verified on Linux: Also independently confirmed the bug-vs-fix behaviour on |
|
Thanks for the focused fix. This is already implemented on current Automated hermes-sweeper review evidence:
|
What does this PR do?
Fixes a silent agent failure where any tool-call command containing a literal
~in a non-path context (e.g.~500-700 agencies,~45,000 CVEs,~80/hr blended rate— common LLM output meaning "approximately") causes the entire tool invocation to surface asError during OpenAI-compatible API call #N: Could not determine home directory.from inside the conversation loop's catch-all.The actual root cause is in
agent/subdirectory_hints.py:Path(token).expanduser()raisesRuntimeErrorwhen the tilde-expansion can't resolve to a real user, and the existingexcept (OSError, ValueError):clauses do not catch it. The exception bubbles up through the tool dispatcher and gets misreported as an API call error, masking the bug and making it look model-specific.Related Issue
Fixes
Type of Change
Changes Made
agent/subdirectory_hints.py:138—except (OSError, ValueError)→except (OSError, ValueError, RuntimeError)(in_add_path_candidate, which callsPath(raw_path).expanduser()).agent/subdirectory_hints.py:198—except ValueError→except (ValueError, RuntimeError)(aroundhint_path.relative_to(self.working_dir), which also touches the tilde-expansion path internally).agent/subdirectory_hints.py:202— same as 198 aroundhint_path.relative_to(Path.home())(wherePath.home()itself can raiseRuntimeErrorif HOME is not resolvable in the calling context).tests/agent/test_subdirectory_hints.py— three new tests in aTestSubdirectoryHintTrackerTildeRobustnessclass:test_tilde_approximately_in_command_does_not_crash: simulates an LLM heredoc with~500-700/~45,000-style tokens. Without the fix, raisesRuntimeError. With the fix, returns silently.test_tilde_with_unknown_user_does_not_crash:~nonexistent_user/path(POSIX), same expectation.test_valid_tilde_user_still_works: regression guard so legitimate~/Documentspaths still work.How to Test
Repro on
main(without the fix):Inside Hermes (without the fix), the symptom is:
After applying the fix:
pytest tests/agent/test_subdirectory_hints.py -q # All TestSubdirectoryHintTrackerTildeRobustness tests pass.Affected models we've reproduced this on (it is not model-specific):
openai/gpt-5-minivia OpenRouteropenai/gpt-5.1-codexvia OpenRouter (mid-pipeline, ~33 turns in, on a heredoc containing~80/hr blended rate)deepseek/deepseek-v4-flashvia OpenRouterChecklist
Code
fix(subdirectory_hints): catch RuntimeError from Path.expanduser()pytest tests/agent/test_subdirectory_hints.py -qpassesnousresearch/hermes-agent:latest), Python 3.13Documentation & Housekeeping
Why this is hard to notice without the fix
The surfaced error string is "Error during OpenAI-compatible API call #N: Could not determine home directory." which strongly suggests an API-side or environment-side problem. In reality it's a pathlib exception three frames deep in the hint walker, wrapped twice by the conversation loop's catch-all. Once you know to look at
subdirectory_hints.py, the one-line nature of the fix is obvious. Hopefully this PR saves the next user the few hours we spent tracing it.