Skip to content

fix(windows): native Windows correctness fixes + green CLI test suite - #57016

Closed
lEWFkRAD wants to merge 6 commits into
NousResearch:mainfrom
lEWFkRAD:fix/windows-native-test-suite
Closed

fix(windows): native Windows correctness fixes + green CLI test suite#57016
lEWFkRAD wants to merge 6 commits into
NousResearch:mainfrom
lEWFkRAD:fix/windows-native-test-suite

Conversation

@lEWFkRAD

@lEWFkRAD lEWFkRAD commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Running the test suite natively on Windows (not WSL) surfaced 16 failures across tests/cli, tests/gateway, and tests/tools. Triaging them found 4 real product bugs plus a handful of POSIX assumptions in the tests themselves. With this PR, pytest tests/cli tests/gateway/test_status.py tests/tools/test_windows_native_support.py tests/test_windows_subprocess_no_window_flags.py is fully green on native Windows (1180 passed, 5 legitimately skipped) and stays green on POSIX.

Product fixes

  1. Gateway profile liveness mis-match on mixed path separators (gateway/status.py, hermes_cli/gateway.py): _command_line_belongs_to_profile compared str(Path) (backslashes on Windows) against HERMES_HOME= argv values that may carry forward slashes (wmic/CIM output, Git Bash, JSON configs) — a live named-profile gateway could be reported not-running by the dashboard's cross-profile check. Both sides now normalize to forward slashes.
  2. cprint crashes when stdout has no console (hermes_cli/banner.py): prompt_toolkit's Win32Output raises NoConsoleScreenBufferError under pythonw.exe, CI, or redirected stdout. A display helper should never crash its caller — it now degrades to plain print, matching the fallback cli._cprint already has.
  3. file:///C:/... drops don't resolve (cli.py): urlparse keeps the leading slash (/C:/...); it is now stripped for drive-letter paths so dragging a file into the CLI works on Windows.
  4. POSIX-literal paths joined with os.path.join (cli.py termux hint, hermes_cli/browser_connect.py WSL chrome candidates): produced /sdcard\Pictures\cat.png on native Windows. Joined with literal forward slashes / posixpath.join.

Test-suite fixes

  • New tests/cli/conftest.py: 9 test modules build a CLI via importlib.reload(cli) with prompt_toolkit stubbed in sys.modules. reload() re-executes into the same module dict, so the MagicMock bindings (_pt_print, _PT_ANSI, ...) survive the patch.dict context and silently swallow _cprint output in any later test (this is what made test_resume_quiet_stderr order-dependent). An autouse module-scoped fixture restores the real bindings when the pollution is detected.
  • Symlink tests skip when the process lacks symlink privileges (WinError 1314 without Developer Mode), reusing the existing _can_symlink() convention from test_symlink_prefix_confusion.py.
  • Tilde-expansion tests set USERPROFILE alongside HOME (ntpath.expanduser ignores HOME since Python 3.8).
  • test_windows_native_support.py: two tests violated the file's own "every test mocks the platform" contract — the no-op test now mocks is_windows(), and the real-SIGKILL assertion is skipped on win32.
  • Cross-platform regression test for the mixed-separator profile match.

Related Issue

Follow-up to the native-Windows work in #43933 and #54565.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • gateway/status.py, hermes_cli/gateway.py — separator-normalized profile matching
  • hermes_cli/banner.py — cprint console fallback
  • cli.py — file URI drive-letter fix, termux hint posix join
  • hermes_cli/browser_connect.py — posixpath.join for WSL candidates
  • tests/cli/conftest.py (new) — un-pollute cli module after stubbed reloads
  • tests/cli/*, tests/gateway/test_status.py, tests/tools/test_windows_native_support.py — Windows-safe tests + regression coverage

How to Test

  1. On native Windows: python -m pytest tests/cli tests/gateway/test_status.py tests/tools/test_windows_native_support.py tests/test_windows_subprocess_no_window_flags.py → previously 16 failures, now green (1180 passed, 5 skipped).
  2. On Linux/macOS: same command — no behavior change (the new conftest fixture is a no-op when no pollution occurred; product changes only affect Windows-only or WSL-literal code paths).
  3. python scripts/check-windows-footguns.py --all → clean.

🤖 Generated with Claude Code

lEWFkRAD and others added 4 commits July 2, 2026 07:33
On Windows, str(Path) renders backslashes while a HERMES_HOME= value on
an argv (wmic/CIM output, Git Bash, JSON configs) may carry forward
slashes -- the substring match in _command_line_belongs_to_profile and
its mirror _matches_current_profile never hit, so a live named-profile
gateway was reported as not running by the cross-profile liveness check.
Normalize both sides to forward slashes before comparing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* _detect_file_drop: file:///C:/... parses to path /C:/... -- strip
  the leading slash on Windows so dropped file URIs resolve.
* _termux_example_image_path: join with literal forward slashes; the
  Android roots are POSIX paths regardless of host OS.
* get_chrome_debug_candidates: join WSL /mnt/c/... candidates with
  posixpath.join -- os.path.join emits backslashes on native Windows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
prompt_toolkit's Win32Output raises NoConsoleScreenBufferError when
stdout is redirected or absent (pythonw.exe desktop backend, CI,
piped output). Degrade to a plain print instead of crashing the
caller, matching the fallback cli._cprint already has.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* tests/cli/conftest.py (new): several modules reload cli with
  prompt_toolkit stubbed in sys.modules; reload() re-executes into the
  same module dict, so the MagicMock bindings survive the patch context
  and silently swallow cprint output in later tests. Restore the real
  bindings at module boundaries when the pollution is detected.
* Skip symlink tests when the process lacks symlink privileges
  (WinError 1314), reusing the existing _can_symlink() convention.
* Set USERPROFILE alongside HOME in tilde-expansion tests --
  ntpath.expanduser ignores HOME since Python 3.8.
* test_windows_native_support: mock is_windows() in the no-op test and
  skip the real-SIGKILL assertion on win32, per the file's own
  every-test-mocks-the-platform contract.
* test_status: regression test for mixed-separator profile matching.
* test_resume_quiet_stderr: reset prompt_toolkit's cached AppSession
  output so creation happens under capsys.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 2, 2026
lEWFkRAD added a commit to lEWFkRAD/hermes-agent that referenced this pull request Jul 10, 2026
…6 update wipe

Squashed re-apply of the 4 commits from fork branch
fix/windows-native-test-suite (profile command-line matching across path
separators, drive-letter file:// URIs, cprint no-console fallback, CLI
test-suite Windows skips + conftest isolation). Patch guard flagged the
wipe at 10:46Z; NousResearch#57016 is still open upstream so the fix must live
locally until merge.

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

Thanks for the native-Windows cleanup. The reported production defects are still present on current main: profile checks compare unnormalized paths in gateway/status.py:361-370 and hermes_cli/gateway.py:357-371; banner.cprint has no fallback at hermes_cli/banner.py:39-43; and the URI/WSL joins remain at cli.py:2774-2777, cli.py:2841-2847, and hermes_cli/browser_connect.py:91-124.

Problems

  • The PR does not add direct regression coverage for the drive-letter URI, no-console cprint, or WSL candidate-join fixes. The current file-URI test only covers a POSIX Path.as_uri() form (tests/cli/test_cli_file_drop.py:202-207), and no browser-connect or banner test is changed in the PR diff.

Suggested changes

  • Add focused tests for file:///C:/... under the Windows path branch, cprint fallback after a prompt_toolkit output exception, and forward-slash WSL candidates when path joining runs on an nt host.

Automated hermes-sweeper review.

Comment thread hermes_cli/banner.py
@@ -41,7 +41,14 @@ def cprint(text: str):
"""Print ANSI-colored text through prompt_toolkit's renderer."""
from prompt_toolkit import print_formatted_text as _pt_print
from prompt_toolkit.formatted_text import ANSI as _PT_ANSI

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.

Please add a focused regression test that makes print_formatted_text raise and verifies this fallback emits through plain print; no banner test is changed by this PR.

@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 15, 2026
lEWFkRAD added a commit to lEWFkRAD/hermes-agent that referenced this pull request Jul 25, 2026
…6 update wipe

Squashed re-apply of the 4 commits from fork branch
fix/windows-native-test-suite (profile command-line matching across path
separators, drive-letter file:// URIs, cprint no-console fallback, CLI
test-suite Windows skips + conftest isolation). Patch guard flagged the
wipe at 10:46Z; NousResearch#57016 is still open upstream so the fix must live
locally until merge.
teknium1 pushed a commit that referenced this pull request Jul 30, 2026
…ner, and WSL browser paths

Salvaged from #57016 by @lEWFkRAD:
- cli.py: handle file:///C:/... drive-letter URIs on nt (strip the
  leading slash urlparse leaves); join Termux example paths with literal
  forward slashes so hints stay POSIX on Windows.
- gateway/status.py + hermes_cli/gateway.py: normalize backslashes to
  forward slashes before the HERMES_HOME substring match so separator
  style cannot defeat profile ownership detection.
- hermes_cli/banner.py: cprint degrades to plain print when
  prompt_toolkit has no console (NoConsoleScreenBufferError on
  redirected/absent Windows stdout).
- hermes_cli/browser_connect.py: posixpath.join for WSL /mnt/c/... bases
  (os.path.join would emit backslashes on nt).
- Test hardening: symlink skip-guards, USERPROFILE alongside HOME for
  ntpath.expanduser, SIGKILL absence skipif fixed via monkeypatch,
  drive-letter URI / separator-normalization / banner-fallback coverage.

Dropped from the original PR: tests/cli/conftest.py fixture and the
AppSession _output monkeypatch — main's merged tests/cli/conftest.py
already handles that prompt_toolkit pollution.
@teknium1

Copy link
Copy Markdown
Contributor

Merged in part via the round-4 closeout PR #74614 (commit fbb93fa, your authorship preserved). Landed: the four verified Windows product fixes — drive-letter file:///C:/ URI handling (cli.py), backslash→slash HERMES_HOME normalization before profile matching (gateway/status.py + hermes_cli/gateway.py), banner cprint NoConsole fallback, posixpath.join for WSL browser paths — plus the symlink skip-guards and USERPROFILE-aware test hardening. Dropped: the CLI conftest fixture and AppSession monkeypatch (the pollution they addressed was root-fixed on main via #68872), and hunks superseded by the merged Windows-native work. Between this, #57066, #63981, #61598, #57119 and #57182, your whole cluster has now landed or been resolved — thanks for the sustained, high-quality diagnosis work @lEWFkRAD!

@teknium1 teknium1 closed this Jul 30, 2026
SSC-ENG pushed a commit to SSC-Engineering/hermes-agent that referenced this pull request Jul 30, 2026
…ner, and WSL browser paths

Salvaged from NousResearch#57016 by @lEWFkRAD:
- cli.py: handle file:///C:/... drive-letter URIs on nt (strip the
  leading slash urlparse leaves); join Termux example paths with literal
  forward slashes so hints stay POSIX on Windows.
- gateway/status.py + hermes_cli/gateway.py: normalize backslashes to
  forward slashes before the HERMES_HOME substring match so separator
  style cannot defeat profile ownership detection.
- hermes_cli/banner.py: cprint degrades to plain print when
  prompt_toolkit has no console (NoConsoleScreenBufferError on
  redirected/absent Windows stdout).
- hermes_cli/browser_connect.py: posixpath.join for WSL /mnt/c/... bases
  (os.path.join would emit backslashes on nt).
- Test hardening: symlink skip-guards, USERPROFILE alongside HOME for
  ntpath.expanduser, SIGKILL absence skipif fixed via monkeypatch,
  drive-letter URI / separator-normalization / banner-fallback coverage.

Dropped from the original PR: tests/cli/conftest.py fixture and the
AppSession _output monkeypatch — main's merged tests/cli/conftest.py
already handles that prompt_toolkit pollution.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…ner, and WSL browser paths

Salvaged from NousResearch#57016 by @lEWFkRAD:
- cli.py: handle file:///C:/... drive-letter URIs on nt (strip the
  leading slash urlparse leaves); join Termux example paths with literal
  forward slashes so hints stay POSIX on Windows.
- gateway/status.py + hermes_cli/gateway.py: normalize backslashes to
  forward slashes before the HERMES_HOME substring match so separator
  style cannot defeat profile ownership detection.
- hermes_cli/banner.py: cprint degrades to plain print when
  prompt_toolkit has no console (NoConsoleScreenBufferError on
  redirected/absent Windows stdout).
- hermes_cli/browser_connect.py: posixpath.join for WSL /mnt/c/... bases
  (os.path.join would emit backslashes on nt).
- Test hardening: symlink skip-guards, USERPROFILE alongside HOME for
  ntpath.expanduser, SIGKILL absence skipif fixed via monkeypatch,
  drive-letter URI / separator-normalization / banner-fallback coverage.

Dropped from the original PR: tests/cli/conftest.py fixture and the
AppSession _output monkeypatch — main's merged tests/cli/conftest.py
already handles that prompt_toolkit pollution.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…ner, and WSL browser paths

Salvaged from NousResearch#57016 by @lEWFkRAD:
- cli.py: handle file:///C:/... drive-letter URIs on nt (strip the
  leading slash urlparse leaves); join Termux example paths with literal
  forward slashes so hints stay POSIX on Windows.
- gateway/status.py + hermes_cli/gateway.py: normalize backslashes to
  forward slashes before the HERMES_HOME substring match so separator
  style cannot defeat profile ownership detection.
- hermes_cli/banner.py: cprint degrades to plain print when
  prompt_toolkit has no console (NoConsoleScreenBufferError on
  redirected/absent Windows stdout).
- hermes_cli/browser_connect.py: posixpath.join for WSL /mnt/c/... bases
  (os.path.join would emit backslashes on nt).
- Test hardening: symlink skip-guards, USERPROFILE alongside HOME for
  ntpath.expanduser, SIGKILL absence skipif fixed via monkeypatch,
  drive-letter URI / separator-normalization / banner-fallback coverage.

Dropped from the original PR: tests/cli/conftest.py fixture and the
AppSession _output monkeypatch — main's merged tests/cli/conftest.py
already handles that prompt_toolkit pollution.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants