fix(update): honesty train — HEAD-move gate, surfaced restart aborts, verified cold-start, self-lock defence - #86687
Merged
Merged
Conversation
A detached/pinned checkout can report 'N new commit(s)' against origin, run the ff-only merge successfully, and still sit on the old commit afterward (the branch-switch step re-detaches to the raw SHA). Before this guard 'hermes update' printed '✓ Code updated!' and reinstalled deps + rebuilt the desktop app against the stale tree - no error, no warning, 'hermes doctor' healthy. Compare pre-pull and post-pull HEAD; if they match, fail loudly with a reattach hint instead of claiming success.
The gateway auto-restart phase in `hermes update` was wrapped in a blanket `except Exception` that only logged at debug level. When the phase raised early — e.g. importing `hermes_cli.gateway` from the freshly pulled checkout inside a process that already loaded pre-update modules — every drain and restart line vanished from the update output, the update printed "Update complete!" and exited 0, and the still-running gateway kept serving pre-update modules against replaced source files. The next Telegram turn died with `ImportError: cannot import name 'is_trivial_prompt'`. The handler now probes for surviving gateway processes and, unless it can positively prove none are running, prints the cause plus a manual recovery command and marks the fleet restart incomplete — which exits nonzero and writes the gateway-mode exit-code marker, matching the existing failed-or-stale-unit path. Fixes #78574
…ivor probe Review follow-up (#78574): the aborted-restart handler only flagged the fleet stale when the post-failure survivor probe was None or non-empty. A positive empty probe was treated as proof-of-safety — but `[]` is only safe when nothing was running before the phase. If a gateway was discovered, stopped (SIGTERM/drain), and its replacement never came back, the probe is empty at exactly that unsafe moment and the update reported success — the fail-open contract this fix exists to close. Snapshot the pre-restart gateway PIDs before any stop/drain and route the handler decision through a pure _restart_phase_failure_is_incomplete() helper that fails closed on an empty survivor set whenever a gateway existed pre-restart (or the pre-state could not be read). Add decision-level regression tests covering the stopped-without-replacement gap, unknown pre-state, and the truly-no-gateway positive control.
…ing success _cold_start_windows_gateway_after_update() printed the success line off a successful Popen return alone, which only proves CreateProcess succeeded, not that the child survived. On Windows, a job object denying CREATE_BREAKAWAY_FROM_JOB hard-kills the child during updater teardown before it logs anything, yet the updater still printed "Starting Windows gateway after update (PID ...)" — leaving Telegram/Discord/etc. offline with no indication anything failed (#84185). Route the success report through gateway_windows._report_gateway_start(), the same post-spawn liveness poll every other _spawn_detached() caller already uses, so a dead child is reported as a failure with a manual-recovery hint instead of a false success.
… base interpreter is uv-managed Two gaps left every Windows git-checkout install unable to recover from the exact failure state #83569 reports: 1. Self-lock detection. _detect_venv_python_processes() always excludes the calling process by design — a CLI hermes update IS the venv python. An updater that had already imported a native venv extension (the canonical one being cryptography.hazmat.bindings._rust, mapped while hermes_cli.main resolved external secret sources) passed every preflight and then died mid-sync with os error 5 when uv tried to rewrite the mapped .pyd, stranding the venv half-updated. A new preflight now refuses the sync before touching the checkout, writes the update-incomplete marker so the next fresh launch completes the install, and exits 2. Verified on a live Windows 11 host: after importing hermes_cli.main, tasklist /m _rust.pyd shows the .pyd mapped in the caller, and a peer process cannot open it read-write (Permission denied) — while a rename succeeds, matching how uv/pip actually fail (truncate+write, not rename). 2. Early-recovery install path. _early_recovery._run_repair_install used sys.executable -m pip unconditionally. Windows git checkouts install on a uv-managed base interpreter (python-build-standalone), whose EXTERNALLY-MANAGED marker makes plain pip abort with externally-managed-environment — the repair no-oped and the venv stayed broken. The repair now detects the PEP 668 marker, prefers uv pip install with VIRTUAL_ENV pointed at the project venv, and falls back to pip --break-system-packages when no uv binary exists. Both fixes ship with subprocess/unit regressions (sabotage-verified): the new tests fail on pre-fix code and pass with it. Complements #77517, which keeps the updater from importing cryptography in the first place; this PR is the defence-in-depth when any future path loads it anyway. Fixes #83569
…self-lock loop fix) Reviewer egilewski found the original defer was circular (#83590 comment): the self-lock preflight wrote .update-incomplete and exited, but the next launch only ran the full recovery AFTER main.py's third-party imports — so a healthy venv's probes made the early pass a no-op, main.py imported cryptography eagerly, the .pyd got mapped again, and the deferred install re-hit the exact self-lock it was meant to escape. Close the loop by making the marker guarantee the install runs BEFORE any native extension can be imported: - hermes_cli/_install_repair.py (new, stdlib-only): single source of truth for the core .[all] reinstall — ensurepip bootstrap, uv-pip/pip resolution with VIRTUAL_ENV, Termux env stripping, Windows hermes*.exe quarantine, per-extra fallback ladder, and fd1→fd2 routing for acp safety. Deliberately free of managed_uv/hermes_constants imports so it stays importable in the corrupted-venv state it exists to repair. - hermes_cli/_early_recovery.py: recover_if_needed now completes a pending .update-incomplete install BEFORE the import probes, on every launch that sees the marker (unless argv is update). Success clears the marker; failure bumps an attempts counter inside the marker body and keeps it. A 3-attempt ceiling stops a persistently-failing install from reinstall-hammering every launch (hermes acp included) — past the ceiling the late post-import recovery takes over with its manual recovery instructions. Single-flight lock shared with the late path. - hermes_cli/main.py: _recover_core_update_marker_locked delegates the install to the shared executor (no duplicated logic); ensure_uv stays in the late path so a venv whose uv vanished mid-update still bootstraps it. - tests: 7 new regressions — the reviewer's exact case (marker + healthy venv → install runs while sys.modules has no cryptography), failure keeps marker + increments attempts, retry ceiling, lazy marker does not trigger core install (#58004 invariant), argv-update skip, and corrupt/missing marker bodies. The key test was sabotage-verified: removing the pre-import branch makes it fail with zero install calls, while a lone-lazy-marker test still passes; restoring the branch makes it pass again. Refs #83569
…rfaced The gateway auto-restart phase used to swallow every exception at debug level, so tests driving cmd_update end-to-end never noticed it touching real gateway discovery. With #78574 surfacing an aborted restart as a failed update, an unmocked find_gateway_pids on a box with a live gateway hits the conftest live-system guard and turns into a spurious sys.exit(1). Add an autouse fixture in test_cmd_update.py (discovery returns nothing, systemd unsupported) and the same seams in test_update_head_moved_gate's helper so the phase is a clean no-op for tests that do not assert on gateway restarts.
alt-glitch
added
type/bug
Contributor
૮ >ﻌ< ა ci reviewran on 7a42bcb — fix(update): use canonical venv_bin_dir in _install_repair (
|
…oded Scripts/bin)
This was referenced Aug 15, 2026
Closed
Closed
This was referenced Aug 15, 2026
Halldrix
added a commit
to Halldrix/hermes-agent
that referenced
this pull request
Aug 15, 2026
…-lock on Windows The secrets_cli import in main() was eager, which loaded agent.secret_sources.bitwarden and its cryptography.* dependencies before cmd_update() ran. On Windows, the updater process itself then mapped cryptography._rust.pyd into its own address space, triggering the self-lock detector (_detect_self_loaded_native_modules) and causing a defer/exit-2 loop that blocked updates entirely. Move the secrets_cli import inside the _dispatch_secrets function so it only pays for itself when the user actually runs a secrets subcommand. This keeps hermes update (and all other commands) free of the cryptography._rust.pyd eager load. Refs NousResearch#83569, NousResearch#83590, NousResearch#86687 Test: 3 new regression tests verify cryptography._rust stays out of sys.modules during main() and the update path.
teknium1
pushed a commit
that referenced
this pull request
Aug 15, 2026
…-lock on Windows The secrets_cli import in main() was eager, which loaded agent.secret_sources.bitwarden and its cryptography.* dependencies before cmd_update() ran. On Windows, the updater process itself then mapped cryptography._rust.pyd into its own address space, triggering the self-lock detector (_detect_self_loaded_native_modules) and causing a defer/exit-2 loop that blocked updates entirely. Move the secrets_cli import inside the _dispatch_secrets function so it only pays for itself when the user actually runs a secrets subcommand. This keeps hermes update (and all other commands) free of the cryptography._rust.pyd eager load. Refs #83569, #83590, #86687 Test: 3 new regression tests verify cryptography._rust stays out of sys.modules during main() and the update path.
teknium1
added a commit
that referenced
this pull request
Aug 15, 2026
…swap is at risk (#86735, #86780, #86781) The #86687 self-lock preflight fired on every Windows `hermes update`: bitwarden.py's module-level cryptography import (fixed in #86782 / #86826-class change) meant cryptography._rust was ALWAYS mapped by the time the preflight ran, so the update exited 2 before even fetching and looped forever — including the Desktop in-app update (#86780). Two structural fixes so the guard can never re-brick the flow it protects: 1. Version-gated detection: _detect_self_loaded_native_modules() now consults _dependency_sync_would_rewrite(dist) — installed version vs the on-disk pyproject pins (base deps + all extras, env markers honored). A loaded module whose distribution the sync will not touch is no lock risk and is not reported. Unknown → fail closed. 2. Relocated deferral: the check no longer runs pre-fetch. It runs via _abort_dependency_sync_if_self_locked() immediately before each venv rewrite (git-path dep sync, ZIP-path dep sync, current-checkout venv repair) — AFTER the code swap. A deferral now leaves the user on NEW code with only the dependency install pending (completed by the next launch's marker recovery), instead of stranding them on the old checkout in an exit-2 loop. PyYAML's _yaml extension (loaded by every CLI process) joins the registry — with version gating it is now safe to list. Tests: version-gate unit coverage (no-change skip, stale pin, missing dist, extras, markers, fail-closed None), deferral wiring (marker + gateway resume + exit 2), placement guards (no detector call pre-fetch; guard present at git/ZIP sync), and subprocess-verified import hygiene (import hermes_cli.main and the update --check dispatch never load cryptography._rust). Follow-up to #86687 (Halldrix's #83590 salvage — the preflight's intent stands as defence-in-depth; this makes it fire only when true). Fixes #86735 Fixes #86780 Fixes #86781
teknium1
added a commit
that referenced
this pull request
Aug 15, 2026
…swap is at risk (#86735, #86780, #86781) The #86687 self-lock preflight fired on every Windows `hermes update`: bitwarden.py's module-level cryptography import (fixed in #86782 / #86826-class change) meant cryptography._rust was ALWAYS mapped by the time the preflight ran, so the update exited 2 before even fetching and looped forever — including the Desktop in-app update (#86780). Two structural fixes so the guard can never re-brick the flow it protects: 1. Version-gated detection: _detect_self_loaded_native_modules() now consults _dependency_sync_would_rewrite(dist) — installed version vs the on-disk pyproject pins (base deps + all extras, env markers honored). A loaded module whose distribution the sync will not touch is no lock risk and is not reported. Unknown → fail closed. 2. Relocated deferral: the check no longer runs pre-fetch. It runs via _abort_dependency_sync_if_self_locked() immediately before each venv rewrite (git-path dep sync, ZIP-path dep sync, current-checkout venv repair) — AFTER the code swap. A deferral now leaves the user on NEW code with only the dependency install pending (completed by the next launch's marker recovery), instead of stranding them on the old checkout in an exit-2 loop. PyYAML's _yaml extension (loaded by every CLI process) joins the registry — with version gating it is now safe to list. Tests: version-gate unit coverage (no-change skip, stale pin, missing dist, extras, markers, fail-closed None), deferral wiring (marker + gateway resume + exit 2), placement guards (no detector call pre-fetch; guard present at git/ZIP sync), and subprocess-verified import hygiene (import hermes_cli.main and the update --check dispatch never load cryptography._rust). Follow-up to #86687 (Halldrix's #83590 salvage — the preflight's intent stands as defence-in-depth; this makes it fire only when true). Fixes #86735 Fixes #86780 Fixes #86781
This was referenced Aug 15, 2026
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Consolidated
hermes updatehonesty train for the Windows/macOS install-update bug class — four fixes that sharehermes_cli/update_cmd.py/main.py, sequenced to compose cleanly (each later commit was retested against the earlier ones):✓ Code updated!on HEAD actually moving. A detached/pinned checkout can passmerge --ff-onlyyet stay on the old SHA; the update then reinstalled deps and rebuilt the desktop against the stale tree while claiming success. Now it compares pre/post-pull SHAs and fails loudly with the reattach command.except Exception: logger.debug(...), so an early failure silently left gateways serving pre-update modules while the update printed "Update complete!" and exited 0. Now: pre-restart PID snapshot + survivor probe + pure fail-closed decision helper ([]after a stopped gateway is NOT proof of safety), recovery warning,.update_exit_code1 in gateway mode, exit 1. Includes egilewski's empty-survivor fail-open fix; monerostar verified on native Win11 (10 tests) and Linux.✓ Starting Windows gateway after update (PID n)straight off a successfulPopen— a job object denying breakaway kills the child before it logs anything (Halldrix's harness provedCREATE_BREAKAWAY_FROM_JOBcan be silently ignored). The cold-start success line is now gated on the same_report_gateway_startpost-spawn liveness poll every other_spawn_detachedcaller uses. This closes the false-success reporting half of [Bug]: Windows: gateway cold-started after 'hermes update' dies silently (no logs, no PID file, no exit record) — offline until manual restart #84185; the deeper schtasks-based spawn escape discussed in that thread is follow-up scope.cryptography._rust.pyd), write.update-incomplete, and complete the pending core install in_early_recoverybefore any native import on the next launch (new stdlib-onlyhermes_cli/_install_repair.pyshared by early and late recovery paths; attempts-counter backoff; PEP 668/uv-managed base interpreter repair viauv pip/--break-system-packages). Addresses egilewski's circular-handoff review with the pre-import completion regression.Follow-up commits by me: contributor email mapping, plus test seam updates — with the restart phase now surfaced (fix 2), tests that drive
cmd_updateend-to-end must mock gateway discovery or they reach the conftest live-system guard on a box with a live gateway (autouse fixture intest_cmd_update.py, same seams intest_update_head_moved_gate.py).Related Issues
Fixes #79678, #78574, #83569. Addresses the reporting half of #84185. Supersedes #79734, #78590, #84212, #83590 (all cherry-picked here with authorship preserved).
Verification (Linux)
scripts/run_tests.shover the 10 touched/adjacent test files:test_cmd_update.py,test_update_autostash.py,test_lazy_refresh_venv_repair.py,test_update_concurrent_quarantine.py,test_update_head_moved_gate.py,test_update_gateway_restart_aborted.py,test_update_cold_start_gateway_liveness.py,test_update_self_lock.py,test_early_recovery.py,test_update_interrupted_recovery.py— 87 passed, 0 failed.python3 scripts/audit_pr_attribution.py— all contributor emails mapped.Needs Windows smoke (not executable here): live
hermes updateon a Windows host with a running gateway (job-object teardown path) and a cryptography version bump (self-lock path). The Linux-testable logic (marker lifecycle, decision helpers, liveness-gated reporting, uv/PEP 668 branches) is covered by the suites above.Infographic