Skip to content

fix(runner): whitelist Windows profile and temp vars in hermetic env - #67407

Closed
dany-aspire wants to merge 1 commit into
NousResearch:mainfrom
dany-aspire:fix/run-tests-windows-hermetic-env
Closed

fix(runner): whitelist Windows profile and temp vars in hermetic env#67407
dany-aspire wants to merge 1 commit into
NousResearch:mainfrom
dany-aspire:fix/run-tests-windows-hermetic-env

Conversation

@dany-aspire

Copy link
Copy Markdown

Fixes #67385.

Problem

On native Windows, fails because strips , , , , , and variables, causing test collection crashes whenever tests touch or write temporary files. Consoles also fail when encoding tick glyphs.

Fix

  • Appended conditional passthrough of , , , , , and in using syntax (ensuring macOS/Linux environments remain byte-identical).
  • Added to force UTF-8 stdout encoding on Windows, resolving charmap encode errors.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #67387 already covers this environment passthrough and additionally supports the native Windows venv layout. This focused subset remains related rather than a duplicate so maintainers can choose the preferred scope.

@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 isolating the Windows environment allowlist issue.

Problems

  • scripts/run_tests.sh:46-53 only recognizes a POSIX venv (bin/activate, then bin/python). A standard native-Windows venv uses Scripts/activate and Scripts/python.exe, so the runner reaches the error at scripts/run_tests.sh:61-64 before the new env -i passthrough at this PR's added line 101 can execute. The member-linked #67387 already carries the required Scripts/ probe alongside the same allowlist change.

Suggested changes

  • Include a guarded Scripts/activate / Scripts/python.exe venv path, then keep the proposed Windows environment passthrough and PYTHONUTF8=1 change.

Automated hermes-sweeper review.

Comment thread scripts/run_tests.sh
LANG=C.UTF-8 \
LC_ALL=C.UTF-8 \
PYTHONHASHSEED=0 \
PYTHONUTF8=1 \

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.

This allowlist is reached only after venv discovery. Current scripts/run_tests.sh:46-53 requires bin/activate and bin/python, so a standard native-Windows Scripts/activate venv exits at lines 61-64 before this line runs unless HERMES_PYTHON is supplied. Please add the guarded Scripts/ probe included by related #67387.

@dany-aspire dany-aspire reopened this Jul 19, 2026
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 19, 2026
@dany-aspire

Copy link
Copy Markdown
Author

Closing in favor of #67387, which implements the complete fix for #67385 (including the Windows Scripts/ venv probe this PR was missing). Thanks!

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 needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: run_tests.sh cannot run tests touching Path.home() on native Windows — env -i strips USERPROFILE

3 participants