Skip to content

fix(update): ignore Windows system venv holder noise - #57852

Open
yungchentang wants to merge 1 commit into
NousResearch:mainfrom
yungchentang:codex/fix-windows-venv-holders-57829
Open

fix(update): ignore Windows system venv holder noise#57852
yungchentang wants to merge 1 commit into
NousResearch:mainfrom
yungchentang:codex/fix-windows-venv-holders-57829

Conversation

@yungchentang

Copy link
Copy Markdown
Contributor

Summary

  • Ignore Windows pseudo/system processes when checking live venv Python holders during update.
  • Keep user-space Python/Hermes trampoline detection intact.

Root Cause

  • _detect_venv_python_processes() treated noisy Windows system process cwd/cmdline metadata as proof of a Hermes venv holder, even when the executable was a system process.

Tests

  • python -m pytest tests/hermes_cli/test_update_venv_health.py -q
  • python -m py_compile hermes_cli/main.py tests/hermes_cli/test_update_venv_health.py
  • git diff --check

Fixes #57829

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P2 Medium — degraded but workaround exists labels Jul 3, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows update fix. Current main still promotes any process whose cmdline contains the venv path (hermes_cli/main.py:9082) or whose hermes_cli.main invocation has a cwd under the install root (hermes_cli/main.py:9084-9086), matching the failure mode reported in #57829. The PR filters the reported system-process class before those fallbacks and its added test retains detection of the user-space Python trampoline.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026
@yungchentang
yungchentang force-pushed the codex/fix-windows-venv-holders-57829 branch from 9c8a46b to 50ea329 Compare July 15, 2026 18:00
@yungchentang

Copy link
Copy Markdown
Contributor Author

Rebased the original focused fix onto current main (1f89f3102f) and force-pushed the PR branch as 50ea329ab4. The implementation still filters the Windows system-process class before the venv/cwd fallbacks, while retaining the user-space Python trampoline regression.

Verification:

  • scripts/run_tests.sh tests/hermes_cli/test_update_venv_health.py -q — 19 passed
  • git diff --check origin/main...HEAD — clean

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

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard 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-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.

[Bug]: _find_venv_holders false positive on Windows - flags system processes as Hermes processes, blocking update

3 participants