Skip to content

feat(utils): add safe_expanduser() — crash-free path expansion for HOME-unset environments - #41870

Closed
rickychen-xm wants to merge 1 commit into
NousResearch:mainfrom
rickychen-xm:fix/safe-expanduser-utility
Closed

feat(utils): add safe_expanduser() — crash-free path expansion for HOME-unset environments#41870
rickychen-xm wants to merge 1 commit into
NousResearch:mainfrom
rickychen-xm:fix/safe-expanduser-utility

Conversation

@rickychen-xm

Copy link
Copy Markdown

What does this PR do?

Adds a shared safe_expanduser() helper to utils.py and adopts it in agent/subdirectory_hints.py as the first production caller. The helper wraps Path.expanduser() so it never raises RuntimeError("Could not determine home directory") — instead it returns the original path (or an explicit default) when HOME cannot be resolved.

Related Issue

No existing issue. Surfaced in real gateway.log traces on a fresh macOS install of Hermes — agent/subdirectory_hints.py:130 crashes whenever a tool call passes a ~/-prefixed path through SubdirectoryHintTracker._add_path_candidates from a context where Python cannot determine HOME. Concrete trigger seen locally:

RuntimeError: Could not determine home directory.
  File "agent/subdirectory_hints.py", line 130, in _add_path_candidates
    p = Path(raw_path).expanduser()

This happens when the macOS launchd-spawned gateway daemon's plist lacks HOME in EnvironmentVariables, AND pwd.getpwuid() lookup fails (rare uid mapping issue). The same class of crash hits Docker/k8s containers with stripped passwd entries and sudo -E invocations.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ♻️ Refactor (introduces shared helper for sweep PR)

Changes Made

  • utils.py (+50 lines): new safe_expanduser(path, default=None) -> Path under a new "Path Helpers" section. Catches RuntimeError and OSError; returns the original path by default, or a caller-provided default when supplied.
  • agent/subdirectory_hints.py (+3, -1): import safe_expanduser from utils; replace the bare Path(raw_path).expanduser() at line 130 with safe_expanduser(raw_path). The surrounding try/except (OSError, ValueError) is left in place — safe_expanduser only neutralizes the HOME-resolution failure mode, not the subsequent resolve() and ancestor-walk errors.
  • tests/test_utils_expanduser.py (+72 lines): 7 tests covering:
    • normal ~/ expansion when HOME is set
    • non-~ paths pass through unchanged
    • Path instances accepted alongside strings
    • default parameter honored (and coerced to Path)
    • regression test: HOME unset + pwd.getpwuid monkeypatched to raise KeyError → original path returned (not a crash)
    • same setup with explicit default → default returned

How to Test

# Run the new tests
pytest tests/test_utils_expanduser.py -v

# Reproduce the original crash and verify the fix
python -c "
import os, pwd
from pathlib import Path
os.environ.pop('HOME', None)
pwd.getpwuid = lambda uid: (_ for _ in ()).throw(KeyError('nope'))

# Old code crashes:
try:
    Path('~/.hermes').expanduser()
    print('ORIGINAL: no crash')
except RuntimeError as e:
    print(f'ORIGINAL: CRASHED — {e}')

# New helper does not:
from utils import safe_expanduser
result = safe_expanduser('~/.hermes')
print(f'SAFE: returned {result!r}')
"

Expected output:

ORIGINAL: CRASHED — Could not determine home directory.
SAFE: returned PosixPath('~/.hermes')

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (feat(utils): ...)
  • I searched for existing PRs — no duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/test_utils_expanduser.py tests/test_utils_truthy_values.py tests/agent/ — 11 utils tests pass, all agent tests pass except one pre-existing failure on main unrelated to this change (test_anthropic_adapter.py::test_prefers_oauth_token_over_api_key is environment-sensitive to host ~/.claude/ state)
  • I've added tests for my changes — 7 new tests including the regression case
  • I've tested on my platform: macOS 26.5.1, Python 3.13

Documentation & Housekeeping

  • Helper has a full docstring explaining the failure mode and when to use it vs bare Path.expanduser() — N/A for separate docs file
  • No config keys added — N/A for cli-config.yaml.example
  • No architecture change — N/A for CONTRIBUTING.md/AGENTS.md
  • Cross-platform: helper is platform-agnostic; failure mode is more common on Unix but Path.expanduser() on Windows can also raise when USERPROFILE/HOMEDRIVE+HOMEPATH are all missing — same catch handles it.
  • No tool descriptions changed — N/A

Follow-up

This PR is intentionally narrow — it ships the helper + one adopter so the API can be reviewed in isolation. A follow-up PR will sweep the ~49 other bare Path().expanduser() / os.path.expanduser() call sites across hermes_cli/, agent/, cron/, gateway/, tools/, acp_adapter/, and plugins/. Splitting it avoids a giant unreviewable diff and lets the API land first.

Path.expanduser() raises RuntimeError('Could not determine home
directory') when HOME is unset and pwd.getpwuid() fails. This
condition surfaces in launchd-managed daemons (common macOS gateway
deployment), containerized runs, and sudo -E invocations.

safe_expanduser() wraps the call in a (RuntimeError, OSError) catch
and returns the original path (or an explicit default) instead of
crashing — matching what virtually every caller in this codebase
wants when HOME can't be resolved.

Changes:
  - utils.py: new safe_expanduser() function with full docstring
  - agent/subdirectory_hints.py: switch from Path().expanduser()
    to safe_expanduser() as the first production adoption
  - tests/test_utils_expanduser.py: 7 tests covering normal,
    edge-case, and failure-path behavior
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels Jun 8, 2026
@udatny

udatny commented Jun 23, 2026

Copy link
Copy Markdown

Hi @rickychen-xm — flagging an overlap so it doesn't surprise either of us at merge time. My PR #29433 (fix(subdirectory_hints): catch RuntimeError from Path.expanduser()) widens the three except clauses in agent/subdirectory_hints.py directly, while your PR introduces a proper safe_expanduser() helper in utils.py and adopts it in the same file as the first caller.

Same root cause, different shape — your helper is the better long-term answer (reusable across the other vulnerable sites the reviewers on #29433 flagged: tool_dispatch_helpers.py, gateway.py, hermes_cli/kanban_db.py, etc.). Mine is just a tactical three-line unblock.

If #29433 lands first (approved + rebased, awaiting maintainer), subdirectory_hints.py will already have the widened excepts — your adoption of safe_expanduser() there would supersede them cleanly. If yours lands first, I'll close #29433 as redundant. Either order works; just wanted to surface this so maintainers have context.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused helper and for documenting the deployment failure mode. The crash-free behavior in this PR’s sole production adoption is already implemented on current main.

Automated hermes-sweeper review evidence:

  • Commit c126a99fc1e2f82a1e23ebe27fb52e26687fdafa added RuntimeError handling around the exact Path(raw_path).expanduser() path in agent/subdirectory_hints.py:147; this prevents the reported exception from escaping the best-effort hint walker.
  • Current regression coverage in tests/agent/test_subdirectory_hints_tilde.py:19 and :38 exercises approximate-tilde and unknown-user tilde tokens through SubdirectoryHintTracker.check_tool_call() without raising.
  • The implementing commit is contained in release v2026.7.1.
  • The overlap noted by @udatny with fix(subdirectory_hints): catch RuntimeError from Path.expanduser() #29433 has therefore resolved in favor of the already-landed focused fix.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants