Repository navigation
fix(desktop): Windows update-preflight family — deferral evidence, hindsight daemon reap, gateway stop/restore (salvage #98350/#75477/#76057) - #100124
Merged
Conversation
…lockers The Desktop venv-blocker scan (since #99724) defers ledger-verified serve/dashboard holders to the CLI updater's stop+relaunch rungs, but the scan output only carried an opaque deferred_backends count — nothing explained WHICH holders the deferral consumed or why they vanished from processes. Add sanitized decision evidence (#98350): deferred_backend_evidence lists structured ledger identity only (pid, purpose, recorded port) — never the command line, which can carry tokens or private endpoints. Adds a desktop parser contract fixture proving the consumer tolerates the diagnostics while keeping blocked/processes authoritative. Salvaged from PR #98350; the exemption half of that PR was independently consolidated on main via #99724 (_is_updater_owned_backend).
…ff (Windows) The pre-handoff teardown tree-kills only the backends the Desktop owns (backendConnectionState + backendPool). The memory plugin's hindsight daemon is spawned DETACHED off venv\Scripts\pythonw.exe, so it survives the teardown, keeps venv files mapped, and either dead-ends the venv-blocker scan with no in-app remedy or (pre-#74805 shim-only gate) raced the updater into a half-updated venv. Add a narrowly-scoped reap: kill only processes whose exe lives under venv\Scripts (ordinal case-insensitive prefix — no PowerShell -like wildcard hazards) AND whose cmdline references hindsight_api.main. External holders (user terminals, unrelated scripts) are never killed — scanVenvBlockers still reports them and the hand-off aborts, per existing design. Selection logic is a pure DI'd module with unit tests. Salvaged from PR #75477 (scoped per review: the narrow daemon kill; the PR's generic every-exe kill was rejected as over-broad, its updateInFlight half was superseded by #75778/#73822, and its generic lock-probe half by the #74805 release gate + #99724 scanner classification).
…e gate (#70337) releaseBackendLock tree-kills only the Desktop's own backends (backendConnectionState + backendPool). A messaging gateway launched by the gateway-launcher desktop plugin via /api/gateway/start lives outside those structures; on Windows its launcher (venv\Scripts\python.exe) keeps the venv mandatory-locked, so the 15s release gate aborts the hand-off BEFORE the venv-blocker scan — the scanner's pausable-gateway exemption and the CLI updater's pause machinery never get their chance. Delegate to `hermes gateway stop --all` (launcher + worker discovery across every profile; gateway.pid records only the uv WORKER and taskkill /T from the worker never reaches its parent). Per review, adds the drain-semantics counterpart: every applyUpdates abort path (lock-held, venv-blocked, probe-failed, updater-spawn-failed) restores gateways via `gateway start --all`, so a failed update no longer strands every profile's gateway stopped. Pure DI'd module + tests. Salvaged from PR #76057 (issue #70337; overlap credit: #70477 by @JonthanaHanh targeted the same symptom earlier via ZIP-dir preservation).
૮ >ﻌ< ა ci reviewran on a5e891f — fix(desktop): stop plugin-launched gateways before the Windo
|
alt-glitch
added
type/bug
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
5 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.
Summary
Wave-3 salvage of the venv-preflight scanner family — three surviving external PRs consolidated into one PR on the same Windows update-preflight path, each contributor's authorship preserved via cherry-pick-style commits.
1. Sanitized deferral evidence — @liuhao1024 (PR #98350)
The exemption half of #98350 was independently consolidated on main by #99724 (
_is_updater_owned_backenddefers ledger-verified serve/dashboard holders). What survived is the evidence half: the scan output carried only an opaquedeferred_backendscount. Nowdeferred_backend_evidencereports structured ledger identity (pid, purpose, recorded port) — never argv, which can carry tokens or private endpoints. Includes liuhao1024's desktop parser contract fixture (adapted to main's field names) proving the consumer tolerates the diagnostics whileblocked/processesstay authoritative.2. Reap detached hindsight venv daemon — @xy952666680 (PR #75477)
The memory plugin's hindsight daemon is spawned DETACHED off
venv\Scripts\pythonw.exe, so it survives the Desktop's backend tree-kill and keeps venv files mapped — dead-ending the venv-blocker scan with no in-app remedy. Salvaged the narrowly-scoped daemon reap (exe undervenv\Scriptsby ordinal path-prefix AND cmdline referencinghindsight_api.main; pure DI'dvenv-holder-selectmodule with tests). Per the triage review, the PR's other halves were dropped: the generic every-exe kill (over-broad — external holders must abort, not die), theupdateInFlightmarker gate (superseded by #75778/#73822), and the generic lock probe (superseded by the #74805 PID-exit release gate + #99724 scanner classification).3. Stop plugin-launched gateways before the release gate — @Sergey0515 (PR #76057, fixes #70337)
A gateway launched by the gateway-launcher plugin via
/api/gateway/startis not inbackendConnectionState/backendPool, soreleaseBackendLock's teardown never touches it; its launcher keeps the venv locked and the 15s gate aborts before the scanner's pausable-gateway exemption can run. Salvaged thehermes gateway stop --alldelegation (launcher+worker discovery, all profiles) and addressed the review's drain-semantics concern: everyapplyUpdatesabort path (lock-held, venv-blocked, probe-failed, updater-spawn-failed) now restores gateways viagateway start --all, so a failed update no longer strands gateways stopped. Overlap credit: #70477 by @JonthanaHanh targeted the same symptom earlier.Testing
tests/hermes_cli/test_scan_venv_blockers.py— 38 passed (evidence assertions added; sanitization proven: no--host/argv in evidence)apps/desktop: vitest electron project —venv-blocker-scan.test.ts(27),venv-holder-select.test.ts(6),gateway-stop-before-update.test.ts(7),backend-release-gate.test.ts— all greentsc -p tsconfig.electron.json --noEmitclean; eslint clean; ruff clean_scan_venv_blockers.main()with a ledgered serve + a foreign holder → serve deferred with sanitized evidence, foreign holder still blocksLive repro: real-import
_scan_venv_blockers.main()harness on origin/main — before: deferred serve vanished fromprocesseswith only an opaque count (no way to tell WHICH holder was consumed, argv redaction unverifiable); after:deferred_backend_evidence=[{"pid":78,"purpose":"serve","port":9119}]with the foreign holder (pid 92) still reported as the sole blocker. Windows-native halves (daemon reap, gateway stop): wine2e live lane run https://github.com/NousResearch/hermes-agent/actions/runs/33486696978 (venv-holder live E2E on windows-latest against this head).Infographic