Skip to content

test(agent): pin Windows HOME variables in tilde-dependent tests - #63562

Open
LceAn wants to merge 1 commit into
NousResearch:mainfrom
LceAn:win-expanduser-home
Open

test(agent): pin Windows HOME variables in tilde-dependent tests#63562
LceAn wants to merge 1 commit into
NousResearch:mainfrom
LceAn:win-expanduser-home

Conversation

@LceAn

@LceAn LceAn commented Jul 13, 2026

Copy link
Copy Markdown

What & why

On POSIX, expanduser("~") honors HOME; on Windows it prefers USERPROFILE, then HOMEDRIVE/HOMEPATH. Tests that set only HOME can therefore resolve ~ against the real Windows profile instead of their temporary directory.

Change

Apply the portable test-home setup to all seven reviewed tilde-dependent tests:

  • set HOME and USERPROFILE to the temporary test directory;
  • clear HOMEDRIVE and HOMEPATH fallbacks;
  • compare the two mixed-separator path results semantically with Path.

No production code changes are involved.

Validation

The seven focused behaviors pass on native Windows (six pytest targets plus direct MCP certificate-path verification); lint and diff checks pass.

Related / duplicate analysis

Complementary to #67196, which changes canonical runner/Hindsight isolation. Not a duplicate of #34374/#61571/#61668: those change production image-routing recognition, while this PR only repairs test fixtures.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows test-isolation fix. The two target failures are real on current main: tests/agent/lsp/test_workspace.py:136-139 sets only HOME before agent/lsp/workspace.py:41 calls os.path.expanduser, and tests/agent/test_context_references.py:334-355 similarly exercises the home-sensitive guard at agent/context_references.py:366.

Problems

  • The same test defect remains in unmodified sibling paths: tests/agent/test_shell_hooks_consent.py:203agent/shell_hooks.py:880; tests/agent/test_image_routing.py:541agent/image_routing.py:122; tests/cli/test_cli_file_drop.py:214 and tests/cli/test_cli_image_command.py:76cli.py:2850; tests/tools/test_mcp_client_cert.py:87tools/mcp_tool.py:992. Each relies on HOME alone while asserting ~ resolves under its temp directory.

Suggested changes

  • Extend this portable-home setup to those sibling tilde tests. tests/tools/test_checkpoint_manager.py:57-60 is existing repository precedent.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows duplicate This issue or pull request already exists labels Jul 15, 2026
@alt-glitch

alt-glitch commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #49118: this PR now expands the same Windows HOME-profile isolation to seven tests, so it is a broader, complementary test fix rather than a duplicate.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 16, 2026
@LceAn
LceAn force-pushed the win-expanduser-home branch from 9eb3d83 to e6825b0 Compare July 18, 2026 07:51
@LceAn

LceAn commented Jul 18, 2026

Copy link
Copy Markdown
Author

Updated per review. Added the portable HOME/USERPROFILE setup to all five named sibling tests (seven total), and changed the two mixed-separator string assertions to Path semantic comparisons.

Focused validation: all 7 target nodes passed.

The broader canonical seven-file run still exposes independent existing Windows clean-env baselines: CLI files fail collection because the runner removes USERPROFILE/LOCALAPPDATA before cli import, test_image_routing.py has six unrelated absolute-Windows-path parser failures, and the runner itself raises GBK UnicodeEncodeError while printing . I kept those separate from this focused HOME/tilde PR.

@alt-glitch alt-glitch added comp/cli CLI entry point, hermes_cli/, setup wizard tool/mcp MCP client and OAuth comp/lsp Language Server Protocol integration (P2 policy) needs-decision Awaiting maintainer decision before any implementation and removed duplicate This issue or pull request already exists labels Jul 18, 2026
@LceAn
LceAn force-pushed the win-expanduser-home branch from e6825b0 to ec72a8e Compare July 20, 2026 02:59
@LceAn LceAn changed the title test(agent): pin USERPROFILE so HOME-dependent tests resolve ~ on Windows (#49118) test(agent): pin USERPROFILE for HOME-dependent tilde tests on Windows (#49118) Jul 20, 2026
@LceAn

LceAn commented Jul 20, 2026

Copy link
Copy Markdown
Author

Maintenance update: rebased onto current main, normalized all seven touched files back to LF, and refreshed the body with the related-PR analysis. The seven reviewed portable-home fixtures remain intact; the diff is now +29/-5. #67196 is complementary, and the duplicate image-routing PRs target production regex code rather than these fixtures.

@alt-glitch alt-glitch removed the needs-decision Awaiting maintainer decision before any implementation label Jul 20, 2026
Pin USERPROFILE alongside HOME and clear HOMEDRIVE/HOMEPATH in all
seven tests that assert tilde expansion resolves under a temporary home.
Use Path-based assertions for the two helpers that preserve a mixed
separator after ntpath.expanduser, so the tests compare path semantics
rather than platform-specific string formatting.

Addresses review feedback on NousResearch#63562.
@LceAn
LceAn force-pushed the win-expanduser-home branch from ec72a8e to 4d41a5f Compare July 20, 2026 05:14
@LceAn LceAn changed the title test(agent): pin USERPROFILE for HOME-dependent tilde tests on Windows (#49118) test(agent): pin Windows HOME variables in tilde-dependent tests Jul 20, 2026
@LceAn

LceAn commented Jul 20, 2026

Copy link
Copy Markdown
Author

Second-pass maintenance: kept the seven per-test HOME/USERPROFILE fixtures as the narrow fix; no global fixture was introduced. The broader runner and production image-routing PRs remain complementary, not duplicates.

@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 20, 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 comp/cli CLI entry point, hermes_cli/, setup wizard comp/lsp Language Server Protocol integration (P2 policy) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows tool/mcp MCP client and OAuth type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants