Skip to content

fix(tools): preserve host PATH in Windows terminal snapshots - #71433

Open
OpenSpek wants to merge 2 commits into
NousResearch:mainfrom
OpenSpek:fix/windows-preserve-host-path
Open

OpenSpek wants to merge 2 commits into
NousResearch:mainfrom
OpenSpek:fix/windows-preserve-host-path

Conversation

@OpenSpek

Copy link
Copy Markdown

What does this PR do?

On Windows, build the LocalEnvironment snapshot with non-login Git Bash instead of bash -l.

Git for Windows' /etc/profile can replace the native parent PATH. Desktop and collaboration hosts commonly inject a bundled CLI directory into that parent environment before launching an ACP agent. The login shell discarded that directory before Hermes captured its terminal snapshot, so a CLI that existed and was present in the ACP process environment became command not found inside the terminal tool.

Configured shell init files are already prepended explicitly. This change keeps that behavior while limiting the non-login snapshot to Windows; POSIX retains the existing login-shell path.

Real integration proof

Reproduced on Windows 10 with:

Before the change, native and non-login Git Bash could resolve buzz, while the login snapshot could not. After the change, a fresh process against current Hermes main returned:

/c/.../buzz-pr2773-pilot/build/target/release/buzz
BUZZ_CLI_OK

The full ACP turn then published a visible, attributed Buzz thread reply with the exact requested token.

The separate Git Bash startup-probe hang encountered in the same proof is already covered by #69083 and is intentionally not duplicated here.

Tests

  • New Windows/POSIX shell-mode contract: 2 passed
  • ruff check tools/environments/local.py tests/tools/test_local_env_windows_msys.py: passed
  • Full Windows/MSYS file, baseline: 4 failed, 36 passed
  • Full Windows/MSYS file, patched: 4 failed, 38 passed

The four failures are unchanged Windows-host baseline assumptions in existing tests; this patch introduced no new failures.

Type of change

  • Bug fix
  • Tests

Scope

  • No configuration or new environment variables
  • No provider/model changes
  • No POSIX shell behavior change
  • No third-party integration added to Hermes core

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets tool/terminal Terminal execution and process management platform/windows sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 25, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused Windows reproduction and narrow terminal change.

Problems

  • tools/environments/local.py in this PR changes every Windows login=True invocation to bash -c. On current main, BaseEnvironment.init_session() deliberately requests a login snapshot at tools/environments/base.py:584; the documented contract includes loading /etc/profile, ~/.bash_profile, and ~/.profile (website/docs/reference/faq.md:164). The new condition therefore removes more than the problematic PATH rewrite.
  • The explicit-init prelude does not cover the default profile case on Windows: tools/environments/local.py:1306 only auto-resolves profile files when not _IS_WINDOWS. The added unit test supplies an explicit .bashrc, so it cannot detect loss of default login-profile exports.

Suggested changes

  • Keep Windows login initialization and preserve the inherited host PATH after profile processing instead of making every Windows snapshot non-login.
  • Add a regression that proves both a host-injected PATH entry and a login-profile PATH entry survive the snapshot.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@alt-glitch alt-glitch added os/windows Native Windows behaviour or breakage: PowerShell install, Windows paths, cmd.exe, WSL-vs-native diff and removed platform/windows labels Oct 17, 2026

This branch has not been deployed

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

Labels

comp/tools Tool registry, model_tools, toolsets os/windows Native Windows behaviour or breakage: PowerShell install, Windows paths, cmd.exe, WSL-vs-native diff P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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 tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants