Skip to content

fix(windows): normalize MSYS Hermes environment paths - #93938

Closed
chrisluersen wants to merge 1 commit into
NousResearch:mainfrom
chrisluersen:fix/msys-env-path-normalization
Closed

chrisluersen wants to merge 1 commit into
NousResearch:mainfrom
chrisluersen:fix/msys-env-path-normalization

Conversation

@chrisluersen

Copy link
Copy Markdown

What does this PR do?

Fixes HERMES_HOME (and LOCALAPPDATA) path mangling when Hermes runs under git-bash / MSYS on Windows. MSYS rewrites Win32 env vars to POSIX form when spawning native Python, so HERMES_HOME arrives as /c/Users/<user>/AppData/Local/hermes instead of C:/Users/<user>/.... A bare Path(value) then mangles the leading /c into a rooted relative path (\c\Users\...), silently pointing at a non-existent shadow tree — which breaks board enumeration and can create junk C:\c\... directories.

Adds _normalize_msys_env_path() to hermes_constants.py: on win32 only, it converts the intentional MSYS drive form /<ASCII drive letter>/...<UPPER DRIVE>:/... before Path construction. Native drive paths, UNC paths, custom root-relative paths, Unicode non-drive paths, a bare /c, and every non-Windows value pass through unchanged. Applied at the three Path() construction sites for HERMES_HOME and LOCALAPPDATA (native default home, env-resolved home, and get_default_hermes_root()), preserving memoization and profile unwrapping.

Related Issue

None (no issue opened; cross-platform compatibility fix).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_constants.py — add _normalize_msys_env_path(); apply it at the three env-path Path() sites (_get_platform_default_hermes_home, _hermes_home_from_env, get_default_hermes_root).
  • tests/test_hermes_constants.py — 13 focused tests: direct MSYS HERMES_HOME; MSYS profile path resolving to the native root; MSYS LOCALAPPDATA with HERMES_HOME unset; native slash/backslash drive paths; UNC path; /custom/hermes and /é/not-a-drive pass-through; bare /c unchanged; non-Windows pass-through.

How to Test

  1. python -m pytest tests/test_hermes_constants.py -k "Msys or msys or normalize or Normalization" -q13 passed, 66 deselected.
  2. Ruff: ruff check hermes_constants.py tests/test_hermes_constants.py → clean.
  3. Under git-bash on Windows, launch Hermes and confirm HERMES_HOME resolves to the correct C:\Users\...\AppData\Local\hermes tree (no shadow \c\Users\...).

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — not fully run: the canonical full-suite run on this Windows host is non-diagnostic (untouched tests fail from missing async-test support, Pyright init timeouts, and Windows symlink privilege limits). Focused tests/test_hermes_constants.py passes green; ruff is clean.
  • I've added tests for my changes
  • I've tested on my platform: Windows 11 (git-bash/MSYS)

Documentation & Housekeeping

  • Updated docstrings for the new normalization helper
  • Updated cli-config.yaml.example if config keys changed — N/A (no new config)
  • Considered cross-platform impact (Windows-only normalization; POSIX pass-through) per the compatibility guide

Screenshots / Logs

N/A (no visual change).

git-bash/MSYS rewrites Win32 env vars to POSIX form when spawning native
Python, so HERMES_HOME and LOCALAPPDATA arrive as /c/Users/... instead of
C:/Users/... A bare Path() mangles the leading /c into a rooted relative
path (\c\Users\...) pointing at a non-existent shadow tree, which
breaks board enumeration and can create junk C:\c\... dirs.

Add _normalize_msys_env_path() to convert only the intentional MSYS drive
form /<ASCII drive letter>/... to <UPPER DRIVE>:/... on win32, applied at
the three Path() construction sites for HERMES_HOME and LOCALAPPDATA.
Native drive paths, UNC paths, custom root-relative paths, Unicode
non-drive paths, a bare /c, and all non-Windows values pass through
unchanged.
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint platform/windows Native Windows-specific behavior or breakage area/profiles Multi-profile isolation, HERMES_HOME scoping P2 Medium — degraded but workaround exists sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 24, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Nicely restrained fix: converting only the exact MSYS drive form (/<ascii letter>/…) on win32 avoids the classic over-normalizer trap (UNC, /custom/..., Unicode roots all pass through), the three call sites that consume Windows-shaped env values are covered, and the test matrix — including the profile-path round-trip through get_default_hermes_root() and non-win32 passthrough — is exactly right. Small notes only:

  1. hermes_constants.py:_normalize_msys_env_path — consider the /cygdrive/c/... spelling for awareness if not handling. Cygwin (as opposed to git-bash/MSYS2) rewrites env vars to /cygdrive/<letter>/..., which this helper intentionally leaves untouched and which will hit the same shadow-tree mangling. If Cygwin users are out of scope, a docstring sentence saying so prevents a future "why doesn't this work" issue; if they're in scope, the same narrow pattern extended by prefix check covers it.

  2. Coverage gap — mixed separators. MSYS can emit /c/Users/alex/AppData/Local/hermes (handled) but tools sometimes pass C:/Users/... already normalized or HERMES_HOME=/c/Users/../.. with dot segments; Path() handles dots fine post-conversion, so this is really just asking for one test pinning /c/./x/../yC:/./x/../y passes through the converter untouched (normalization stays Path's job).

  3. Trivial: tests/test_hermes_constants.py ends without a trailing newline (diff shows \ No newline at end of file) — most linters/editors will flag it eventually.

No blocking issues.

@monerostar monerostar 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.

Native Win11 + Git Bash verification from a live Hermes host (not CI).

Bug is real here. Unfixed Path("/c/Users/Admin/AppData/Local/hermes").resolve() lands on C:\c\Users\Admin\AppData\Local\hermes. That shadow tree already exists on this machine (created ~2026-07-17) with kanban.db (118784 bytes), cron/, sessions/, profiles/, etc. Current install hermes_constants.py still has no helper.

This patch works on this host. From the PR tree:

helper /c/Users/Admin/AppData/Local/hermes -> C:/Users/Admin/AppData/Local/hermes
get_process_hermes_home(HERMES_HOME=/c/Users/Admin/AppData/Local/hermes/profiles/main)
  -> C:\Users\Admin\AppData\Local\hermes\profiles\main  (exists, not the C:\c shadow)

python -m pytest tests/test_hermes_constants.py -k "Msys or msys or Normalization" -o addopts=13 passed on this Win11 box.

CI risk: several helper tests (test_direct_msys_hermes_home, lowercase-drive, native/UNC/passthrough-except-linux) do not monkeypatch sys.platform. On win32 they pass (as they did here). On Linux they call the helper which returns the input unchanged, so test_direct_msys_hermes_home would expect C:/Users/alex/... and get /c/Users/alex/.... gh pr checks currently reports no checks on this branch. Please pin sys.platform == "win32" on the conversion assertions (the process-home tests already do this).

Scope: converting only /<ascii letter>/... is the right narrow cut. Sibling #39910 is a much larger un-mangle (cygdrive/mnt + cron/logging). This PR is the smaller HERMES_HOME/LOCALAPPDATA fix; they should not both land as-is without a shared helper.

Nits already noted by the automated review: missing trailing newline in the test file; /cygdrive/c/... intentionally untouched (fine if git-bash-only).

Happy to re-run the focused tests after the platform guards land.

@monerostar

Copy link
Copy Markdown
Contributor

Native Win11 + git-bash verify (monerostar)

Host: Windows 11 (10.0.26200), Python 3.11.15 (native win32 venv), shell is git-bash (MSYSTEM=MINGW64, OSTYPE=cygwin). Checkout: 00a6fead2a.

This is exactly the environment the PR describes. In this Hermes/cron process HERMES_HOME / LOCALAPPDATA already arrive as native C:\... (Hermes set them), but the pathlib mangle is trivial to show with the MSYS form:

Path('/c/Users/Admin/AppData/Local/hermes')
  str        = \c\Users\Admin\AppData\Local\hermes
  is_absolute = False
  drive/root  = '' / '\'
  .absolute() = C:\c\Users\Admin\AppData\Local\hermes

That shadow tree is not hypothetical on this machine: C:\c\Users\Admin\AppData\Local\hermes already exists (profiles dir included). Native C:\Users\Admin\AppData\Local\hermes is the real home.

Tests

pytest tests/test_hermes_constants.py::TestMsysEnvPathNormalization -o addopts=
  13 passed

All conversion / passthrough / profile-unwrap / LOCALAPPDATA cases passed on native win32, including the linux monkeypatch passthrough.

Notes (not blockers)

  • Sibling: fix(windows): unmangle MSYS Hermes paths #39910 is a larger unmangle (canonicalize_hermes_path, and it drops .resolve() on cron stores because Path.resolve() re-introduces C:\c\...). This PR is the smaller, right-place fix for HERMES_HOME / LOCALAPPDATA construction in hermes_constants. They overlap; they should not both land as competing helpers.
  • Helper is correctly win32-only and narrow (/<ASCII letter>/...X:/...). Bare /c, /custom/hermes, UNC, and native C:\ / C:/ forms are left alone — matches what I want from a git-bash host.
  • Does not by itself stop other call sites that do Path('/c/...').resolve() on a raw MSYS string. That's the fix(windows): unmangle MSYS Hermes paths #39910 leftover. Fine for this scope.

The bug is real here. The tests match the live pathlib behavior.

@chrisluersen

Copy link
Copy Markdown
Author

Superseded by #115116 — same fix, rebased onto current main (this branch was 11,954 commits behind and conflicting). The refresh also re-targets the call site upstream moved (_hermes_home_from_envget_process_hermes_home) and makes the tests host-independent instead of faking sys.platform, so they no longer fail the Linux CI lane.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants