Skip to content

fix(update): coordinate Windows gateway pause across three layers during desktop update (#74386) - #74419

Closed
webtecnica wants to merge 4 commits into
NousResearch:mainfrom
webtecnica:fix/74386-windows-update-gateway-coordination
Closed

fix(update): coordinate Windows gateway pause across three layers during desktop update (#74386)#74419
webtecnica wants to merge 4 commits into
NousResearch:mainfrom
webtecnica:fix/74386-windows-update-gateway-coordination

Conversation

@webtecnica

@webtecnica webtecnica commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #74386 — on Windows gateway-enabled installs, the desktop Update button triggers a three-layer chain (Electron → Rust bootstrapper → Python CLI) where each layer has a different gap in gateway process coordination, guaranteeing that the desktop updater can never complete on a gateway-enabled install without manual hermes gateway stop intervention.

Changes

Layer 1 — Electron (apps/desktop/electron/main.ts)

Add pauseWindowsGatewaysForUpdate / resumeWindowsGatewaysAfterUpdate functions that run the Python _gateway_update_lock subprocess bridge to pause gateways before the venv-blocker scan. The pre-flight blocker scan was finding the running gateway and aborting before the CLI updater even started.

The resume token is passed to the spawned CLI updater via HERMES_WINDOWS_GATEWAY_RESUME_TOKEN env var so it does not double-pause. On error paths (blocked/probe-failure), gateways are resumed before the UI shows the error.

Layer 2 — Fallback

stopVenvBlockers in venv-blocker-scan.ts provides a TypeScript-side fallback: when the blocker scan finds processes, isGatewayProcess() classifies them by cmdline, and gateway PIDs are force-killed via taskkill /F before re-scanning. Non-gateway processes are still reported as blockers.

Layer 3 — Python CLI (hermes_cli/update_cmd.py, hermes_cli/_gateway_update_lock.py)

Two fixes:

  1. Env-var token passthrough: Before calling _pause_windows_gateways_for_update(), check HERMES_WINDOWS_GATEWAY_RESUME_TOKEN. When set by the Electron preflight, use the Electron's token as the resume token (skip re-pause — gateways are already gone) but still register the atexit resume handler.

  2. Graceful gateway handling in venv-holder check: When _detect_venv_python_processes() finds remaining holders after gateway pause, classify each by cmdline. If all are gateways (e.g. gateways spawned via --replace or shim-wrapped children that find_gateway_pids() missed), force-kill them with terminate_pid(force=True) and proceed instead of aborting. Only abort when a non-gateway holder (the Desktop backend) remains.

New module: hermes_cli/_gateway_update_lock.py

Subprocess-callable bridge invoked by Electron:

  • python -m hermes_cli._gateway_update_lock pause — calls _pause_windows_gateways_for_update(), prints JSON resume token
  • python -m hermes_cli._gateway_update_lock resume <json-token> — calls _resume_windows_gateways_after_update()

Files changed

File Change
apps/desktop/electron/main.ts +Layer 1: gateway pause/resume helpers, integrated into applyUpdates preflight
apps/desktop/electron/venv-blocker-scan.ts +isGatewayProcess + stopVenvBlockers helpers
apps/desktop/electron/venv-blocker-scan.test.ts +tests for new helpers
hermes_cli/_gateway_update_lock.py +new module: subprocess bridge for pause/resume
hermes_cli/update_cmd.py +Layer 3: env-var token passthrough + graceful gateway handling

Related

…rting (NousResearch#74326)

The desktop's Update button has been unusable on gateway-enabled Windows installs since the updater redesign. Every click kills the app's own backend, then the venv-blocker preflight finds the always-running gateway and aborts. The CLI updater already has pause/resume logic, but the desktop preflight never reaches it.

Add a two-layer gateway-aware preflight:
1. Pre-pause gateways via _gateway_update_lock.py (subprocess bridge to native CLI logic)
2. Fallback taskkill-based cleanup for gateways the pre-pause missed

New files:
- hermes_cli/_gateway_update_lock.py: subprocess-callable gateway pause/resume

Modified files:
- apps/desktop/electron/main.ts: pauseWindowsGatewaysForUpdate/resumeWindowsGatewaysAfterUpdate + modified preflight
- apps/desktop/electron/venv-blocker-scan.ts: stopVenvBlockers/isGatewayProcess
- apps/desktop/electron/venv-blocker-scan.test.ts: tests for new functions
…lpers (NousResearch#74326)

Add GATEWAY_CMDLINE_MARKER constant, isGatewayProcess() to detect gateway processes from blocker scan output, and stopVenvBlockers() to force-kill them via taskkill. Full test coverage.
…se on Windows (NousResearch#74386)

The CLI updater's venv-holder check after gateway pause treats ANY
remaining venv process as a fatal blocker, even when those processes
are gateways that _pause_windows_gateways_for_update() missed (e.g.
gateways spawned via --replace or shim-wrapped children outside the
PID-file registry). This undoes the pause (calls _resume) and aborts.

Two fixes:
1. Check  env var before pausing
   gateways. When set by the Desktop Electron preflight, use the
   Electron's token as the resume token instead of pausing again (the
   gateways are already gone). The CLI updater must still resume on
   exit using the same token.

2. When _detect_venv_python_processes() finds remaining holders after
   pause, classify each by cmdline. If ALL are gateways, force-kill
   them with terminate_pid(force=True) and proceed — the resume token
   will respawn them post-update. Only abort when a non-gateway holder
   (e.g. the Desktop backend) remains.

Closes NousResearch#74386
@webtecnica webtecnica changed the title fix(desktop): stop Windows gateway in update preflight instead of aborting (#74326) fix(update): coordinate Windows gateway pause across three layers during desktop update (#74386) Jul 29, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) platform/windows Native Windows-specific behavior or breakage area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 29, 2026
@apoapostolov

Copy link
Copy Markdown

Thanks for picking this up @webtecnica — I filed #74386 and walked the three-layer chain on a live gateway-enabled Windows install. This PR hits the right shape: Electron pre-pause before scanVenvBlockers, token handoff so CLI does not double-pause, and a second-line kill for gateways the PID-file pause missed.

I read the full diff. A few notes from that path:

Resume token drop on the taskkill path (real regression risk)

In applyUpdates, when the first scan is still blocked, stopVenvBlockers clears the rest, and the re-scan is clear, the code sets noGatewayResumeNeeded = true and then withholds HERMES_WINDOWS_GATEWAY_RESUME_TOKEN from the spawned updater.

That is wrong when pauseWindowsGatewaysForUpdate already returned a useful token. Pause snapshots profile PIDs / unmapped argv / cold_start_if_installed so resume can bring gateways back. Force-killing a leftover gateway does not mean "nothing to resume" — it means "extra PIDs that were not in the registry are gone." Dropping the token leaves the profiles that were paused permanently down until the user runs hermes gateway start or logs in again.

Suggested rule: if gatewayToken is non-null (and not a useless empty object), always pass it to the CLI, even after the taskkill fallback. Only skip the env var when there was never a resume-worthy token.

Classification mismatch (minor)

Electron uses cmdline.includes("gateway run"). Layer 3 CLI uses "gateway" in cmdline.lower(). Prefer the same marker on both sides so a process cannot be "gateway" for one layer and "real blocker" for the other.

Empty / no-gateway token from the bridge

_gateway_update_lock pause emits {"ok": true, "token": {}} when the helper returns None. JSON.stringify({}) is truthy, so CLI may treat that as _gateway_was_pre_paused with an empty dict. Resume already no-ops when resume_needed is missing, so it is mostly harmless, but Electron could treat "ok + empty/missing resume fields" as null and not set the env var. That keeps CLI's own pause path for the no-gateway case.

Heavy import on the preflight path

The bridge imports _pause_windows_gateways_for_update from hermes_cli.main. That import is not free on Windows. Related work in #74599 is shrinking venv-scan cost under Desktop's ~15s preflight budget. Worth keeping an eye on total preflight time (pause subprocess + scan) so we do not trade "always blocked on gateway" for "probe timeout / exit -1". Importing the helpers from the lighter update_cmd surface (or a thin module that does not pull full CLI) would help if preflight starts flirting with the timeout again.

Layer 2 (Rust bootstrapper)

Issue also called out force_kill_other_hermes only taskkilling hermes.exe and missing pythonw gateway. This PR hardens Electron + CLI; the bootstrapper gap is still open if something still enters that path. Fine to land Electron/CLI first — just flagging so #74386 is not closed as fully done until that path is either covered or explicitly out of scope.

Happy to re-test on the same Windows gateway install once the resume-token handoff is tightened. Appreciate the thoroughness.

1. Resume token: remove noGatewayResumeNeeded flag on taskkill path
   (token always carries profile data needed for resume)
2. Classification: align gateway marker (gateway run → gateway)
   to match Python CLI's broader detection
3. Empty token: return null when no gateways running, not {}
4. Import: hermes_cli.main → hermes_cli.update_cmd (lighter import)
@webtecnica

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review @apoapostolov! Addressed all 4 points in 3d9d05a:

1. Resume token (🔴 regression risk) — FIXED
Removed noGatewayResumeNeeded = true on the taskkill path. The token is now ALWAYS passed to the CLI even after stopVenvBlockers. Resume already no-ops when there's nothing to do, so this is safe.

2. Classification mismatch (🟡 minor) — FIXED
Changed GATEWAY_CMDLINE_MARKER from 'gateway run' to 'gateway' in venv-blocker-scan.ts, matching the Python CLI's \"gateway\" in low check. Both sides now use the same marker.

3. Empty token (🟢 harmless) — FIXED
_gateway_update_lock no longer returns {} for the no-gateway case. Electron now checks Object.keys(t).length > 0 before treating the token as valid.

4. Heavy import (🟡 perf) — FIXED
Python bridge now imports directly from hermes_cli.update_cmd instead of hermes_cli.main, avoiding the full CLI module load on the preflight path.

Happy to iterate if anything needs tightening further!

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the Electron and CLI sides of this Windows update failure. The underlying premise is still present on current main: apps/desktop/electron/main.ts:2936-2945 aborts any blocked venv scan, and hermes_cli/update_cmd.py:3128-3132 aborts any remaining holder after pause.

Problems

  • apps/desktop/electron/main.ts:2754 fails the required desktop lint check because the new untyped Promise is inferred as {}, so stdout is not a valid property.
  • apps/desktop/electron/venv-blocker-scan.ts:236 treats any command line containing gateway as killable. The added test at venv-blocker-scan.test.ts:250 demonstrates the mismatch: git log --grep=gateway is expected to be non-gateway but is classified true in CI.

Suggested changes

  • Give the pause subprocess Promise a concrete { stdout: string } result type (or use a typed promisified API).
  • Parse a Hermes gateway invocation rather than using a free-text substring, and retain coverage for arguments that merely mention gateway.
  • Add coverage for the Python pre-paused-token and mixed-holder paths. The current Rust fallback remains limited to hermes.exe at apps/bootstrap-installer/src-tauri/src/update.rs:738-758; scope or cover that residual path explicitly.

Automated hermes-sweeper review.

}

try {
const { stdout } = await new Promise((resolve, reject) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This untyped Promise is inferred as Promise<{}>, so destructuring stdout fails the required desktop lint check. Give the resolved payload an explicit { stdout: string } type or use a typed promisified execFile result.

* classification in ``update_cmd.py``).
*/
export function isGatewayProcess(proc: VenvBlockerProcess): boolean {
return proc.cmdline.toLowerCase().includes(GATEWAY_CMDLINE_MARKER)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A free-text gateway match is too broad for a force-kill decision: it makes the added git log --grep=gateway test fail in CI and can match non-gateway command arguments. Parse/validate the Hermes gateway invocation instead.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Closing in favor of #74618, which we're salvaging for the Windows gateway-pause dead-end (#74326/#74386).

The Electron-side pre-pause can't reach already-bricked installs (it ships via the very updater that's broken), the desktop typecheck was failing, and the loose gateway substring marker misclassifies innocent commands — adding a third gateway matcher in TS is the same matcher-drift class that caused the original bug. #74618 fixes the preflight veto in repo Python the existing build invokes fresh on every scan, sharing one canonical matcher (gateway.status.looks_like_gateway_command_line).

Appreciate the deep three-layer analysis — it maps the coordination problem better than the issue itself did.

@teknium1 teknium1 closed this Aug 1, 2026
@apoapostolov

Copy link
Copy Markdown

Makes sense — thanks for the clear call.

The “already-bricked install” constraint is the decider: anything that only lands after a successful desktop rebuild cannot unstick the path that is currently broken. Putting the preflight exemption + single canonical matcher in repo Python (looks_like_gateway_command_line) so the existing Electron build picks it up on the next scan is the right cut.

I’ll watch #74618 for #74386 / #74326. Appreciate you and @iso2kx pushing the CLI-side salvage rather than stacking another TS matcher.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P1 High — major feature broken, no workaround platform/windows Native Windows-specific behavior or breakage sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

4 participants