fix(update): defer Windows runtime repair when the updater holds the venv

[Finn763]

What does this PR do?

On Windows, hermes update launched from the install's own venv (the standard layout) can never complete the managed-runtime repair. The cutover must rename venv → venv.stale.runtime-<token>, but Windows refuses to rename a directory containing an executable mapped by a running process — and the updater itself is executing venv\Scripts\python.exe (with the venv\Scripts\hermes.exe launcher still mapped as its parent). The pre-flight holder scan (_windows_runtime_holders → _detect_venv_python_processes) deliberately excludes the calling process and its ancestors, so the guard reports "no holders", the repair provisions a whole new runtime generation, and then _rename_with_retry burns its 5 retries against a lock that cannot be released while the updater lives. The failure is non-fatal, so hermes update retries forever without converging — while the misleading "The next hermes update will retry" text implies it will.

This PR adds a Windows self-lock pre-flight that is checked before provisioning:

• New _windows_runtime_self_lock(live) in hermes_cli/managed_uv.py detects the one holder the generic scan is blind to — the calling process itself (sys.executable under the live venv) or a launcher ancestor (venv\Scripts\hermes.exe) started from the venv.
• repair_vulnerable_runtime defers with an actionable message instead of staging a candidate for a doomed cutover: it names the mapped executable, explains that retrying from inside the venv cannot converge, and points at the escape hatch (run the updater from an interpreter outside the venv).
• The deferral returns skipped (like the existing holders guard), so no caller contract changes, and happens before provisioning — the incomplete generation-* leftovers the reporter observed are no longer produced.
• POSIX is untouched: renaming a tree while this process maps files from it works fine there, and the detector is a no-op off Windows.

This mirrors the existing _defer_update_for_self_lock() pattern used by the dependency-sync path, which already bails out cleanly when the updater itself holds the lock.

Root cause

1. repair_vulnerable_runtime() provisions a fixed runtime, then _cut_over_candidate() parks the live venv via _rename_with_retry(live, backup) — any OSError becomes "could not park the existing venv".
2. On Windows a directory containing an executable mapped by a running process cannot be renamed; hermes update runs from venv\Scripts\python.exe / hermes.exe, so the rename always fails with ERROR_ACCESS_DENIED and the retries can never help while the process lives.
3. The pre-flight guard _windows_runtime_holders() delegates to _detect_venv_python_processes(), which explicitly excludes the calling process and its ancestors (skip.add(os.getpid())) — correct for the dependency-sync path (a fresh child process doesn't map the .pyd files), but it makes the guard blind to exactly the holder that matters for a whole-venv rename.
4. Same Windows lock family as [Bug]: hermes update fails on Windows with Access is denied (os error 5) during uv pip install. #23327 / [Bug]: Windows hermes update fails when Gateway/Dashboard lock native .pyd dependencies #38789, whose fixes (rename hermes.exe before install, detect venv python workers) cover only the dependency-sync path. The runtime-repair cutover had no equivalent self-lock handling.

Evidence

• Repro on Windows 11: with sys.executable pointed inside the live venv and no other holders, the pre-fix repair walks straight into provisioning and cutover (regression test was red against the pre-fix code — see the sabotage check below), matching the reporter's could not park the existing venv: [WinError 5] and the two observed consecutive failures.
• Empirical check on this Windows 11 dev machine: _windows_runtime_self_lock(<our real venv>) → True with detail naming ...\Scripts\python.exe; an unrelated venv → False.
• Sabotage run: with the guard neutralized (self_locked, self_detail = False, ""), test_self_lock_defers_repair_before_provisioning FAILS (repair proceeds to provisioning); with the fix restored it passes — the regression test bites.
• venv.rollback-bak-* from the report is not produced by any current code (grep: zero references; leftover of older updater releases), and the incomplete generation-* class stops being produced by this bug because the deferral happens before provisioning. No cleanup sweep added — out of scope.

Test plan

• New TestWindowsRuntimeSelfLock (4 tests, host-independent via mocks, run on Windows locally and on POSIX CI):
  • self-locked updater → skipped deferral, no provisioning, no .hermes-runtime dir, output has actionable guidance and no "will retry";
  • non-self-locked updater → repair proceeds (guard fails open, no always-defer loop);
  • detector is a no-op off Windows;
  • a venv\Scripts launcher ancestor is detected as a self-lock.
• pytest tests/hermes_cli/test_managed_uv.py — 26 passed; the 7 failures on this Windows host are pre-existing Windows-layout failures of POSIX-oriented tests (verified identical at origin/main, untouched by this diff).
• pytest tests/hermes_cli/test_scan_venv_blockers.py tests/hermes_cli/test_update_orphan_backend_reap.py — 47 passed.
• pytest tests/hermes_cli/test_update_self_lock.py tests/hermes_cli/test_update_shim_self_lock.py tests/hermes_cli/test_update_venv_health.py tests/hermes_cli/test_update_secret_import_lock.py — 55 passed.
• ruff check on both files — clean.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Related

• [Bug]: hermes update fails on Windows with Access is denied (os error 5) during uv pip install. #23327, [Bug]: Windows hermes update fails when Gateway/Dashboard lock native .pyd dependencies #38789 (same Windows lock family, dependency-sync path)
• PR fix(update): rename hermes.exe before uv/pip install on Windows #23353 / fix(update): rename hermes.exe before uv/pip install on Windows #23408, PR fix(cli): detect venv python workers before update to prevent .pyd lock failures (#38789) #39037 (the pip/sync-path fixes)
• fix(update): repoint Windows managed runtime without renaming live venv #88836 (open PR taking the alternative cutover approach — repointing pyvenv.cfg instead of renaming; this PR's deferral is complementary: it stops the doomed rename and gives honest guidance, while fix(update): repoint Windows managed runtime without renaming live venv #88836 would actually complete the repair on Windows)

Review round 1 — Enough1122 feedback (2026-08-24)

1. Assertion-message idiom — fixed. test_self_lock_defers_repair_before_provisioning used mock_install.assert_not_called(), ("…"): a bare tuple expression whose parenthetical never becomes a failure message. Now assert mock_install.call_count == 0, ("a self-locked updater must not provision a candidate it can never cut over"). Verified both directions with the guard forced to fail: the old form raises only the bare mock error (Expected '_install_safe_python_generation' to not have been called. Called 1 times.), the new form surfaces the message. Unforced, the test passes as before.
2. psutil unconditional import — no change. Agreed with the review: it is the module's only psutil import and it sits inside the existing fail-open try/except Exception wrapper (managed_uv.py:1078-1097), so a missing psutil skips the ancestor scan instead of failing the repair. Left as-is.
3. Escape-hatch cd {root} — no change, verified. The printed root is Path(project_root) if project_root is not None else _PROJECT_ROOT, with _PROJECT_ROOT = Path(__file__).resolve().parents[1] — the checkout containing hermes_cli/, not the HERMES_HOME data dir. Neither production call site (managed_uv.py:232, :369) passes project_root, so the printed directory always contains hermes_cli/. hermes_cli/main.py registers the update subcommand and runs main() under if __name__ == "__main__", so <system Python> -m hermes_cli.main update resolves the package from the printed cwd. Guidance stands as written.

Closes #93032

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Correct diagnosis and clean fix: distinguishing the structural self-lock (Windows maps the executable of every running process, so the updater inside the venv it must rename can never succeed, retries included) from the transient locks the generic scan handles — and deferring before provisioning so no orphan candidate generation leaks — is exactly right. The escape-hatch guidance honestly avoids promising retries will help, POSIX is properly no-op'd, and the test matrix covers defer-before-provision, fail-open for outside interpreters, the platform gate, and the hermes.exe ancestor-shim case. Small items:

1. Broken assertion-message idiom in the regression test: mock_install.assert_not_called(), ("a self-locked updater …") (tests/hermes_cli/test_managed_uv.py, test_self_lock_defers_repair_before_provisioning) is a bare tuple expression, not an assert-with-message — the parenthetical never evaluates into a failure message. It works today only because assert_not_called() itself raises; wrap it properly (assert mock_install.call_count == 0, "…") or drop the string.

2. _windows_runtime_self_lock resolves both sides per call and walks all psutil ancestors on every repair attempt — negligible here (repair is rare), just noting the psutil import is unconditional inside the try while the rest of the module treats psutil as optional; consistent enough.

3. The suggested escape hatch assumes <system Python> -m hermes_cli.main update works after cd {root} — true when run from the checkout (cwd on sys.path), but for installs where the checkout lives elsewhere than the managed runtime root, verify {root} printed is actually the checkout containing hermes_cli/, not the HERMES_HOME data dir — otherwise the advice sends users to a directory where the module won't import.

[Finn763]

Thanks for the review — all three points addressed.

1. Assertion-message idiom — fixed. test_self_lock_defers_repair_before_provisioning no longer uses the bare tuple mock_install.assert_not_called(), ("…"); it is now assert mock_install.call_count == 0, ("a self-locked updater must not provision a candidate it can never cut over"). Verified both directions by forcing the guard to fail:

• old form (fix stashed): FAILs with only the bare mock error — Expected '_install_safe_python_generation' to not have been called. Called 1 times. — the parenthetical never appears;
• new form: FAILs with AssertionError: a self-locked updater must not provision a candidate it can never cut over — the message now surfaces.

Unforced, the test passes as before: pytest tests/hermes_cli/test_managed_uv.py -k "self_lock or venv_launcher" → 4 passed, 41 deselected. The full file is unchanged from the body's numbers: 26 passed, 12 skipped plus the same 7 pre-existing Windows-layout failures, which I re-ran at the base commit with the fix stashed and reproduced identically (fake non-PE uv executables → WinError 216, a POSIX-only branch test, and the bin/python vs Scripts/python.exe fixture layout) — untouched by this diff. ruff check on the file is clean.

2. psutil unconditional import — agreed, left as-is. It is the module's only psutil import and it sits inside the existing fail-open try/except Exception wrapper (hermes_cli/managed_uv.py:1078-1097), so a missing psutil skips the ancestor scan rather than failing the repair — consistent enough for a rare path, exactly as you noted.

3. Escape-hatch cd {root} — verified, no change needed. What gets printed is root = Path(project_root) if project_root is not None else _PROJECT_ROOT, and _PROJECT_ROOT = Path(__file__).resolve().parents[1] — the directory containing hermes_cli/, i.e. the checkout, never the HERMES_HOME data dir. Neither production call site (managed_uv.py:232 and :369) passes project_root, so the printed directory always contains the package. And hermes_cli/main.py registers the update subcommand and calls main() under if __name__ == "__main__", so <system Python> -m hermes_cli.main update resolves hermes_cli from the cwd the message points at. The guidance stands as written.

Thanks again for the careful read.

[teknium1]

Salvaged onto current origin/main as PR #99711 — both commits cherry-picked cleanly with your authorship preserved (noreply email, auto-resolved by attribution CI).

Extra proof added on top: the on-demand windows-latest lane (.github/workflows/windows-venv-e2e.yml) now runs TestWindowsRuntimeSelfLock, and it executed green on the salvage head: https://github.com/NousResearch/hermes-agent/actions/runs/33428856773

Checked overlap with merged #99525: that PR handles the Desktop updater resume side (scripts/desktop-update/*.ps1); nothing on main produces the CLI runtime-repair self-lock deferral your fix adds — complementary, no conflict.

This PR will be closed with credit once #99711 merges.

[teknium1]

Merged via PR #99711 — your two commits were cherry-picked onto current main with your authorship preserved in git log, plus the on-demand windows-latest lane executed your regression suite green (run 33431300968). Thanks for the precise structural-lock diagnosis and fix!