[omxhub]

Symptom

On a Windows package-manager install, the gateway connected to every platform and then failed every agent turn. Telegram replied with the raw error text:

RuntimeError: Failed to initialize OpenAI client: ModuleNotFoundError: No module named 'pydantic_core'

The supervised tui_gateway worker (tui_gateway.server, which imports pydantic) died five times in a row on the same import.

Root cause

The gateway deliberately runs the package manager's store interpreter, and activation pops VIRTUAL_ENV. _ensure_windows_gateway_venv_imports() therefore fell through to the pre-PM in-tree <root>/venv — a tree built for a different interpreter — and prepended its site-packages to sys.path with no interpreter check:

if os.environ.get("VIRTUAL_ENV"):
    candidates.append(Path(os.environ["VIRTUAL_ENV"]))
legacy_venv = project_root / "venv"
candidates.append(legacy_venv)   # unconditional: no ABI check

Pure-Python distribution contents imported fine, so the adapters stayed healthy and Telegram still connected — but every compiled submodule raised. A cp311 pydantic_core*.pyd cannot load into a cp314 interpreter, and the foreign tree shadowed the correct one at sys.path[0].

Fix

• Resolve pm.environments.committed_venv first. The committed generation is the contract for "which environment this install is running" and is invisible to sys.prefix / VIRTUAL_ENV, so it must win before any fallback.
• Gate every fallback candidate on the running interpreter's ABI (_windows_venv_matches_running_python, reading pyvenv.cfg) so a foreign tree can never reach sys.path.

Tests

tests/gateway/test_gateway_venv_imports.py — 8 contract tests:

• the committed generation wins over the in-tree venv;
• a foreign-ABI VIRTUAL_ENV is refused;
• a matching legacy venv is still adopted (backward compatibility);
• a pyvenv.cfg that cannot be read is not treated as a match-by-default.

Verification

On the affected install (store CPython 3.14 plus a cp311 in-tree venv):

check	before	after
the failing frame, process_bootstrap.OpenAI(...), in the gateway's launch environment	ModuleNotFoundError: No module named 'pydantic_core'	builds a client
supervised tui_gateway worker at boot	died 5×	starts
inbound Telegram turn	error returned in ~1s	replies normally

[Enough1122]

AI code review — automated review for reference; please use your judgment.

Summary

This PR gates gateway venv adoption behind an ABI check, which is the right direction, but the gate as written does not fire for the venvs that actually exist in the wild, and the two tests that would have caught that are vacuous. The core change is in gateway/run.py (_windows_venv_matches_running_python at gateway/run.py:422, call sites at gateway/run.py:478 and gateway/run.py:481) with a new contract suite in tests/gateway/test_gateway_venv_imports.py.

The headline: the parser looks for the version_info key, but CPython's own venv module writes version (not version_info) into pyvenv.cfg. Only uv venv writes version_info. So the guard is a no-op for every stdlib-created venv, and falls through to the permissive "cannot be judged, so allow" branch. The exact regression this PR describes still reproduces.

Findings

1. [blocker] The ABI guard only parses version_info, which CPython's venv never writes

gateway/run.py:437 matches on the key version_info and nothing else:

key, _, value = line.partition("=")
if key.strip().lower() != "version_info":
    continue

version_info is a uv-specific key. CPython's stdlib venv writes version instead — Lib/venv/__init__.py:237 in the standard library is literally:

f.write('version = %d.%d.%d\n' % sys.version_info[:3])

I verified both shapes on this machine (CPython 3.14.7, uv 0.12.17):

$ python -m venv /tmp/_realvenv && cat /tmp/_realvenv/pyvenv.cfg
home = C:\Python314
include-system-site-packages = false
version = 3.14.7
executable = C:\Python314\python.exe
command = C:\Python314\python.exe -m venv ...

$ uv venv /tmp/_uvvenv && cat /tmp/_uvvenv/pyvenv.cfg
home = C:\...\uv\python\cpython-3.14-windows-x86_64-none
implementation = CPython
uv = 0.12.17
version_info = 3.14
include-system-site-packages = false

Feeding those real manifests to the new function under a 3.14 interpreter:

stdlib-venv style (version=)     FOREIGN 3.11  -> matches=True   <-- guard did not fire
stdlib-venv style (version=)     MATCH  3.14   -> matches=True
virtualenv style (version_info=) FOREIGN 3.11  -> matches=False
uv style         (version_info=) FOREIGN 3.11  -> matches=False

The stdlib case returns True for a foreign 3.11 tree because the loop finds no version_info key, exhausts the lines, and hits the final return True at gateway/run.py:443 — the "cannot be judged, so allow" path. That is the same fail-open shape the PR intends to close.

Why this matters here specifically: the tree being guarded is the legacy in-tree <root>/venv, which is exactly the tree most likely to have been created by a pre-PM installer using stdlib venv (the repo's own apps/desktop/electron/main.ts:2777 comment still discusses python -m venv detection for Windows). PM-created generations use uv and do carry version_info, so the fix holds for the generation path and misses the legacy path it was written for. The repo's own fixtures confirm the split: tests/hermes_cli/test_install_lock_timeout.py:38, tests/pm/test_runtime_boot_gc.py:20, tests/scripts/test_payload_digests.py:125 and tests/tools/test_local_env_blocklist.py:307 all use version = , while only tests/hermes_cli/test_gateway_windows.py:133 uses version_info = .

Reproduction of the original symptom at this HEAD. With a stdlib-shaped foreign <root>/venv and no committed selection, running the function directly:

HEAD: foreign-ABI in-tree venv (stdlib 'version = 3.11.9') reached sys.path -> True

Suggested fix: accept either key, and treat a manifest that declares no interpreter version at all as a separate, explicitly-considered case rather than a silent pass:

for line in text.splitlines():
    key, _, value = line.partition("=")
    if key.strip().lower() not in ("version", "version_info"):
        continue
    parts = [p for p in value.strip().split(".") if p.isdigit()]
    if parts[:2] != [str(sys.version_info.major), str(sys.version_info.minor)]:
        return False
    break
else:
    return False  # no version key at all -> unjudgeable, refuse rather than adopt

Note the repo already has two pyvenv.cfg readers to model this on: cron/scheduler_script.py:106 (_read_windows_pyvenv_cfg, which returns a whole dict and keys off uv/home) and agent/runtime_self_protection.py:99 (_pyvenv_home). Both use encoding="utf-8-sig"; the new code uses encoding="utf-8" at gateway/run.py:432, which on a UTF-8-BOM pyvenv.cfg (written by MSYS2/Git-Bash tooling) leaves a leading \ufeffhome — harmless here since only the version key is matched, but the utf-8-sig precedent is worth following for consistency.

2. [blocker] Two of the four behavioural tests assert against an already-restored sys.path, so they pass on unfixed code

tests/gateway/test_gateway_venv_imports.py:58:

def _run(monkeypatch, committed):
    monkeypatch.setattr("pm.environments.committed_venv", lambda _root: committed)
    saved = list(sys.path)
    try:
        gateway_run._ensure_windows_gateway_venv_imports()
        return [p for p in sys.path if p not in saved]
    finally:
        sys.path[:] = saved   # <-- restored before the caller's assert runs

The helper restores sys.path in its finally block, and then the two "must refuse" tests ignore its return value and assert against the global sys.path — which has already been rolled back.

• tests/gateway/test_gateway_venv_imports.py:79 test_foreign_in_tree_venv_is_refused_without_a_selection — calls _run(...) discarding the result, then assert _site(stale) not in sys.path (line 84).
• tests/gateway/test_gateway_venv_imports.py:96 test_foreign_venv_in_the_environment_is_not_adopted — same pattern, _run(...) at line 104, assert _site(stale) not in sys.path at line 105.

Both assert against a list that the helper has just restored, so they can never observe the mutation. Reverse control — head's test file against the base source, which has no guard at all:

test_committed_generation_wins_over_foreign_in_tree_venv   FAILED
test_foreign_in_tree_venv_is_refused_without_a_selection    PASSED   <-- vacuous
test_legacy_venv_is_adopted_when_it_matches_the_running_python PASSED
test_foreign_venv_in_the_environment_is_not_adopted         PASSED   <-- vacuous
... 5 failed, 3 passed

The two vacuous tests pass in isolation on base too, confirming they are not order-dependent. To show what they should have caught, running base's unfixed code directly with a foreign in-tree venv:

AT BASE: actually added to sys.path -> [..., '...\\hermes-agent\\venv\\Lib\\site-packages']
AT BASE: foreign site present?     -> True
AT BASE: after _run() restore, foreign in sys.path? -> False  <-- what the test asserts

The foreign tree was adopted; the assertion simply could not see it. Fix: use the returned added list, as the two working tests do at lines 76 and 94:

added = _run(monkeypatch, None)
assert _site(stale) not in added

This is the same defect class as finding 1 — both leave the intended regression unguarded.

3. [blocker] A committed_venv failure is swallowed into the legacy path, contradicting PM's own contract

gateway/run.py:463-472:

committed: Path | None = None
try:
    from pm.environments import committed_venv
    committed = committed_venv(project_root)
except Exception:
    committed = None
if committed is not None and committed.is_dir():
    candidates.append(committed)
else:
    ...  # legacy fallback: VIRTUAL_ENV and <root>/venv

committed_venv deliberately raises for a broken selection — pm/environments.py:180, :185, :189 and :193 all raise RuntimeError(...) for an unreadable facts file, a malformed record, a non-string path, or a generation that is missing or outside the install. That is a load-bearing signal, and the sibling selected_venv states the contract explicitly at pm/environments.py:153: "A broken committed selection is an error, not permission to load an older dependency set silently."

The blanket except Exception: committed = None collapses "there is no committed selection" (the legitimate None) and "the committed selection is broken" (an error) into the same value, and then falls through to adopting the legacy tree. Confirmed against this HEAD:

HEAD: broken committed record -> legacy foreign-ABI tree adopted? -> True

So the exact outcome PM's contract forbids is what happens. Narrow the except to the import itself (ImportError/ModuleNotFoundError) and let a RuntimeError from a corrupt record propagate or produce an explicit refusal, rather than being downgraded to a fallback.

Note also that committed.is_dir() on line 470 is a second, silent way to drop a valid selection: if the generation is momentarily unreadable, the code again prefers the legacy tree over a committed environment that PM has already designated as authoritative.

4. [non-blocking] The committed generation is adopted without any ABI check at all

gateway/run.py:470-471 appends committed to candidates with no _windows_venv_matches_running_python call, while both fallback candidates are gated. The invariant the PR states — "a foreign-ABI tree must never reach sys.path" (gateway/run.py:425-428) — therefore does not hold on the preferred path. Probed directly:

HEAD: committed 3.11 gen adopted under a 3.14 interpreter? -> True

This is narrower than finding 3: the gate is missing, not wrong. A store-Python minor upgrade ahead of a hermes update re-sync is the plausible trigger, and it would produce precisely the pydantic_core failure the PR is fixing. Gating committed the same way as the fallbacks is a one-line change and makes the stated invariant actually hold.

5. [non-blocking] The new tests will not run in the Windows CI lane

tests/gateway/test_gateway_venv_imports.py:69 gates the class with @pytest.mark.skipif(sys.platform != "win32", ...), but the file carries no pytest.mark.platforms("windows") marker. The Windows lanes in .github/workflows/tests-os.yml:66 build their file list from scripts/ci/list_os_marked_tests.py, which selects only files containing a mark.platforms(...) call (scripts/ci/list_os_marked_tests.py:56, _PLATFORMS_CALL). Confirmed locally:

$ python scripts/ci/list_os_marked_tests.py windows tests | wc -l
168
$ python scripts/ci/list_os_marked_tests.py windows tests | grep gateway_venv
  (no match — this PR's file is not in the list)

So the file is never handed to the Windows runners. The existing sibling test for this same function, tests/gateway/test_windows_venv_import_guard.py (added by the merged #123996), uses the file-level pytestmark = pytest.mark.platforms("windows") and is picked up correctly. The same fix applies here; note that TestVenvMatchesRunningPython at tests/gateway/test_gateway_venv_imports.py:108 is platform-independent, so the marker should go on the Windows-only class rather than the module, or the pure-Python cases would be needlessly excluded on other runners.

6. [minor] The PR description contradicts both the code and its own test

The description's test list claims:

a pyvenv.cfg that cannot be read is not treated as a match-by-default.

The code does the opposite: gateway/run.py:433-434 returns True on OSError, and the docstring at gateway/run.py:429 says so explicitly — "A missing pyvenv.cfg cannot be judged, so it is allowed (never a new refusal)". The test at tests/gateway/test_gateway_venv_imports.py:109 also asserts is True. The description appears to have been written against an earlier revision; worth reconciling so the next reader knows which is intended.

Minor

• gateway/run.py:439 — [p for p in value.strip().split(".") if p.isdigit()] silently drops non-numeric components, so a malformed version_info = 3.x.1 yields ["3"], and ["3"] != ["3", "14"] happens to refuse, but version_info = 3.14.x yields ["3", "14"] and is accepted. Accepting a manifest whose version we could not fully parse is a small hole in an otherwise strict check; prefer refusing when the parsed part count is short.
• tests/gateway/test_gateway_venv_imports.py:129 — no trailing newline at end of file.
• tests/gateway/test_gateway_venv_imports.py:122 and :128 — these two unit tests are the ones that would have caught finding 1, but they are the only fixtures that write version_info. Adding a version = case here is the direct regression test for the bug and costs four lines.

Note on related work

pm/environments.py:163 committed_venv already carries a docstring saying the in-tree venv "is built for whichever interpreter created it, so loading it from PM's store Python mixes ABIs (compiled modules vanish)". That is the same root cause this PR targets, so the fix belongs here; the guard in this PR is the enforcement point that was missing.

[Enough1122]

AI code review — automated follow-up for reference; not a maintainer.

Noting for the record as this closes: the blocker from my review is still the final state of the branch.

The last commit is aaeaa428 (2026-09-25T11:53), my review landed at 14:50 the same day, and you closed at 2026-09-27T01:51 without a reply. So the finding I raised is not addressed — not rejected, just unanswered — and I want that on the thread rather than only in my archive, in case this is picked up later.

The headline, restated so it is actionable without my review: _windows_venv_matches_running_python (gateway/run.py:422) parses the version_info key from pyvenv.cfg, but CPython's own venv module writes version. Only uv venv writes version_info. The guard therefore never fires for a stdlib-created venv, falls into the permissive "cannot be judged, so allow" branch, and the foreign-ABI adoption the PR sets out to prevent still happens for exactly the venvs most installs have. The two tests in tests/gateway/test_gateway_venv_imports.py that would have caught it are vacuous for the same reason — they build their fixture the uv way.

The check is one condition in the right place, and parsing both keys would close it. If you have decided the underlying hazard is handled elsewhere, that is a reasonable position and no reply is needed — I only want to make sure the silence is not read as agreement.

Also fine to note: git log shows the branch was never rebased after aaeaa428, so the shape I reviewed is the shape that existed.