[robbyslmt]

What does this PR do?

Fixes a P1 Windows gateway startup crash (#122183): on installs where a pre-PM in-tree venv/ was built by a different CPython version than the PM-managed runtime, _ensure_windows_gateway_venv_imports() injected that venv's site-packages into the gateway process anyway. The partial injection then broke every later import of a compiled package — the hosted_room_worker died 5× in rapid succession at every boot with:

ModuleNotFoundError: No module named 'pydantic_core._pydantic_core'

(the gateway core survives because its own imports finish before the injection; tui_gateway.server in the worker imports pydantic_core after it).

Two changes to _ensure_windows_gateway_venv_imports():

1. ABI guard — read the candidate venv's pyvenv.cfg and never inject a venv whose Python version differs from the running interpreter. An ABI mismatch there is the crash; unknown layouts (no readable pyvenv.cfg) keep legacy behavior so nothing that worked before stops working.
2. PM preference — when a committed PM environment exists (pm.environments.committed_venv), it is the sole candidate, replacing ambient VIRTUAL_ENV / the in-tree venv. The committed generation is what PM selected for this install; the pre-PM venv is a stale artifact that loses to it.

Related Issue

Fixes #122183 (duplicates: #122556, #122736, #122400)

Type of Change

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

Changes Made

• gateway/run.py — _ensure_windows_gateway_venv_imports(): version guard (_pyvenv_version() helper) + committed-PM-first candidate selection, with logger.debug on every skipped/failed path. Legacy fallback preserved when no PM environment is committed.
• tests/gateway/test_windows_venv_import_guard.py — new: 3 tests covering (a) mismatched venv never injected, (b) matching venv still injected (legacy installs), (c) PM environment wins over a mismatched ambient venv. Red on main, green with this patch, skipped off-Windows.

How to Test

1. Reproduce on main (Windows, PM-managed runtime + an in-tree venv from an older Python): gateway boots, then Supervised task hosted_room_worker died: ModuleNotFoundError("No module named 'pydantic_core._pydantic_core'") × 5 in gateway.log, then giving up restarts.
2. Apply this PR, hermes gateway restart: worker stays up; log shows no pydantic_core errors.
3. pytest tests/gateway/test_windows_venv_import_guard.py -q → 3 passed (Windows; skipped elsewhere).

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass (targeted suites: this file + tests/hermes_cli/test_gateway_windows.py green; 4 unrelated pre-existing environmental failures on this machine are listed in the PR notes below)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11, Python 3.14.7 PM runtime + 3.11 in-tree venv

Documentation & Housekeeping

• I've updated relevant documentation (docstrings carry the WHY) — or N/A
• I've considered cross-platform impact: guard is Windows-path-only (_ensure_windows_gateway_venv_imports early-returns elsewhere); tests skip off-Windows; no POSIX behavior change

Notes for reviewers

• Test environment quirk handled in the tests: the suite's home_io_guard refuses file I/O under the real Hermes home, and site.addsitedir stats every sys.path entry — on default installs the interpreter's own stdlib lives under the home, so the tests stub addsitedir (the unit under test is candidate selection, not stdlib site processing). This makes the tests hermetic on any layout.
• Pre-existing unrelated failures seen on this machine (not introduced here, same before/after): test_exec_schtasks_round_trips_non_ascii_task_argument_live (schtasks needs elevation), test_load_enabled_toolsets_* / test_load_cfg_raw_sees_replacement_with_pinned_mtime_and_size (toolset-catalog test-data drift + mtime race).

[kiramakes]

🤖 Hermes Agent automated review: PR reviewed. Diff analyzed (196 lines changed). Check CI status and manual review recommended.

[kiramakes]

Summary

Ready to merge. Well-scoped fix for a real Windows gateway crash (pydantic_core ABI mismatch), with a comprehensive test suite.

Positives

• The version-guard logic is correctly placed: it checks pyvenv.cfg version before injecting, and skips on mismatch — exactly the right choke point.
• Committed PM environment is preferred over ambient candidates, with a clean fallback path when PM is absent.
• Test coverage is thorough: version-mismatched rejection, matching-venv legacy path, and PM-priority-over-ambient. The outside_home_tmp fixture is a nice touch given the suite's home_io_guard.
• Comments cite the specific incident (hosted_room_worker crashes, 2026-09-26) — good historical context for future readers.

Suggestions

• test_pm_environment_wins_over_mismatched_ambient_venv creates the PM venv with version f"{sys.version_info[0]}.{sys.version_info[1]}.0" — the .0 patch is arbitrary but fine since the guard only checks major.minor. Consider using the same .99 suffix as the other test for visual consistency.
• The _pyvenv_version parser splits on . twice and ignores anything after the second segment. This is correct for "3.11.99" → (3, 11), but a pyvenv.cfg with "version = 3.11" (no third segment) also works because rest.partition(".") on an empty string yields ("", "", ""). No bug, just noting the parser is robust by accident rather than by guard — might be worth a brief comment.

Questions

• None. The contract is clear and the tests validate it.

[ehz0ah]

Verdict: REQUEST_CHANGES. Two P1 blockers remain at this exact head. The production guard does not parse the format written by uv, and the new regression file is absent from the Windows CI selector. No native Windows validation is claimed.

[ehz0ah]

P1 — Parse the uv version_info key before accepting this environment. Hermes PM creates these environments with uv venv, and real uv pyvenv.cfg files use version_info = X.Y.Z. This parser accepts only version, so it returns None for the normal PM format. The check below treats None as an unknown layout and injects the environment anyway. A mismatched uv environment can therefore recreate the compiled-extension ABI crash this PR is intended to stop. Please use the shared parser or accept both keys, read with utf-8-sig, and add a regression that writes the real uv format.

[ehz0ah]

P1 — Use pytest.mark.platforms("windows") so these tests run in CI. The native Windows jobs build their file list with scripts/ci/list_os_marked_tests.py, which selects only files containing platforms(...). This bare skipif file is omitted from both Windows jobs, while Linux and macOS skip all three tests. I confirmed the selector does not list this file at the exact PR head.

[andrexibiza]

Preferring the committed dependency environment over a stale in-tree venv addresses the reported Windows failure. The selection-error path currently undoes that protection, and the late import adjustment needs to preserve the process's already-activated generation.

Reviewed 6c4ae6da656079ba86a162263b07393740644999.

P1 — a broken committed selection is converted into permission to use legacy dependencies

The new except Exception branch turns every committed_venv() failure into pm_env=None, then considers VIRTUAL_ENV and the in-tree venv. But pm.environments._recorded_venv() deliberately distinguishes no record from an unreadable, malformed, missing-tree or out-of-install committed selection. Those latter states raise rather than silently selecting an older dependency set.

A malformed facts.json, a permission error, or a committed generation whose tree vanished therefore reaches the ambient fallback here. If that old venv has the same major/minor Python version, the new version guard accepts it. An error reading the canonical owner has become authority to load a different dependency generation—the exact precedence this PR is trying to prevent.

Allow the legacy path only when canonical selection explicitly reports that no PM/payload environment exists. Preserve selection failures as failures before site.addsitedir, sys.path or child environment mutation. Do not repair this by catching the same error under a narrower log message.

Add separate no-selection, malformed-record, unreadable-record, vanished-selected-tree and outside-install-selection cases. For every invalid committed selection, assert that no legacy site directory is added and neither VIRTUAL_ENV nor PYTHONPATH is rewritten. Retain the allowed legacy-only and valid-PM-precedence controls.

P2 — Python-version compatibility does not prove the active dependency generation

The helper runs after gateway module imports, then prepends the selected site's packages and rewrites the child environment. Equal Python major/minor versions can still describe two dependency trees with different package versions. Modules already imported from A stay in sys.modules while late imports can resolve under B.

This matters if the process activated A at boot and the install's committed selection changed to B before this helper reads it. The existing activate_dependencies contract explicitly makes activation a pre-import boot decision and says already-running processes are not switched after a dependency install. The new PM preference must not become a second, later activation decision.

Use the existing activation/environment identity for the running process, or perform the canonical selection before third-party imports. A same-version comparison remains useful as a rejection check; it is not a replacement for generation identity. Add a same-ABI A/B selection-change case with an early module from A and a late dependency, and assert that one process never composes the two generations. This is a source-level lifetime concern, not a claim that changing only the Python patch number is itself an ABI bug.

Existing review, tests and acceptance

ehz0ah's existing review separately identifies version_info parsing and Windows CI selection. Those are already reported and are not duplicated as new inline findings here. The current source still checks only version, and the test file still uses a bare Windows skipif. Coordinate those corrections with this selection/error fix rather than introducing another venv parser.

This audit re-read the complete two-file diff, full helper, canonical PM selection and activation code, test fixtures and discussion. I did not rerun the three tests, process real .pth files, exercise native Windows startup or perform independent 5×2×3 verification. The tests deliberately stub site.addsitedir; their candidate-selection results are not proof of real import compatibility or compiled-package realization.

Exact-head CI 36196076193, Docker 36196076020, and Nix 36196076146 are action_required. No every-commit-green or merge acceptance is claimed. I cannot authorize workflows. The earlier facade-size concern remains a repository acceptance item; this pass did not independently rerun that scanner.

Keep robbyslmt's implementation and the #122183/#122556/#122736/#122400 incident lineage. The needed correction belongs in the existing PM activation/selection seam, not another fallback owner in gateway/run.py. The PR is open, non-draft and reported mergeable at the unchanged head, with recorded base 6d70abc87134a6f1243b90512530e201f08b02c2.

[andrexibiza]

P1 — do not turn a rejected committed environment into legacy fallback. committed_venv() returns None for no selection but raises for unreadable/invalid facts or a missing/out-of-install generation. This catch collapses those errors into None, allowing same-version ambient/in-tree packages to be injected. Preserve the canonical refusal before path/env mutation; legacy fallback should require an explicit no-selection result. Test corrupted/unreadable facts and a vanished selected tree with a valid-looking legacy venv present.

[andrexibiza]

P2 — retain the boot-activated generation, not just its Python version. The helper runs after imports and later prepends site-packages. A booted process on generation A can see a newly committed same-ABI B here, retaining early modules from A and resolving later ones from B. pm.environments.activate_dependencies explicitly prohibits switching an already-running process after install. Bind this adjustment to the running activation or move selection before third-party imports; add a same-ABI A→B selection-change test. Patch-version differences alone are not the finding.

[kshitijk4poor]

Thanks @robbyslmt. The gateway side lands through #123996. Your committed-generation preference is the core of it, and you're credited as co-author (Co-authored-by).

How the open review points were handled:

• @andrexibiza P1, a broken selection allowing a legacy fallback: committed_venv() errors now propagate (fail-closed, the same as hermes_bootstrap). There's no except Exception turning them into None.
• @andrexibiza P2, keep the boot-activated generation: when a generation is committed, the helper returns before any sys.path change. A running process never gets a newly committed generation prepended late.
• @ehz0ah P1, uv version_info: the pyvenv.cfg parser is gone. The committed-generation check makes a version guard unnecessary, and non-PM installs keep the existing overlay unchanged.
• @ehz0ah P1, Windows CI selector: the regression file uses pytest.mark.platforms("windows"), and list_os_marked_tests.py windows selects it.
• @kiramakes: thanks for the reviews.

Closing in favour of #123996. I'll post the merge SHA here when it lands.

[kshitijk4poor]

Merged: #123996 landed on main as 86f449b.