feat(cli): add --force-kill flag for Windows update (#63300)

[Phoenix0531-sudo]

Summary

On Windows, hermes update fails when Hermes-related Python processes
(headroom proxy, tui_gateway workers) are running. These python.exe
processes hold .pyd file locks that the dependency installer (uv/pip)
cannot release, causing the update to fail partway and leave a broken
install.

The existing --force flag only skips the hermes.exe shim-lock
detector — it does not address the .pyd lock problem, and the existing
--force-venv flag merely bypasses the detection without actually
stopping the lock-holding processes.

This PR adds --force-kill, which detects and terminates all
Hermes-related venv Python processes before proceeding with the update.

Changes

File	Change
hermes_cli/subcommands/update.py	Add --force-kill CLI argument
hermes_cli/main.py	Add _kill_hermes_python_processes()
hermes_cli/main.py	Wire --force-kill into _cmd_update_impl()
hermes_cli/main.py	Update error message to mention --force-kill

Key design decisions:

• Reuses the existing _detect_venv_python_processes() rather than
  duplicating detection logic
• Graceful → force kill pattern: taskkill sends WM_CLOSE first,
  then taskkill /F for survivors
• Runs before the existing --force / --force-venv checks, so
  after killing, those guards pass naturally

Backward compatibility

• --force continues to work unchanged (skips .exe lock check only)
• --force-venv continues to work unchanged (skips venv python check)
• --force-kill is additive; no existing invocation is affected
• Off-Windows: no behavioural change (all new code is behind _is_windows())

Testing

• Python syntax check: py_compile on both modified files passes
• Ruff format: update.py is clean; main.py pre-existing formatting
  issues are unchanged
• Import test: from hermes_cli.main import _kill_hermes_python_processes
  resolves correctly

Related issue

Closes #63300

[teknium1]

Thanks for addressing a real Windows update failure mode. The venv-holder guard still exists on current main (hermes_cli/main.py:9604-9609), so the underlying report remains relevant.

Problems

• The new call at hermes_cli/main.py:9620 kills the generic detector result before _pause_windows_gateways_for_update(). Current main deliberately snapshots unmapped gateway argv before force-killing so it can relaunch it (hermes_cli/main.py:9209-9214); the proposed ordering loses that state. The detector also explicitly says Desktop backends respawn and should be refused rather than killed (hermes_cli/main.py:9020-9022).
• The new taskkill invocations build "/PID <pid>" as one argv element (hermes_cli/main.py:9153, :9159). The established primitive uses separate "/PID", str(pid) arguments plus /T /F (gateway/status.py:112).
• No tests are included for the new flag or termination/update-resume behavior.

Suggested changes

• Preserve the existing gateway pause/snapshot/resume flow, and narrowly target only processes that can safely be stopped.
• Reuse the established Windows termination behavior and add Windows-gated regression coverage.

Automated hermes-sweeper review.

[teknium1]

Please use the established taskkill argv shape: separate "/PID", str(pid) elements (and the existing tree-kill/no-window behavior where applicable). gateway/status.py:112 is the current Windows termination primitive and avoids encoding a switch and its value into one argv item.

[teknium1]

This runs before _pause_windows_gateways_for_update(), so a detected gateway is killed before that flow can snapshot its argv and arrange a restart. Current main treats that snapshot as load-bearing (hermes_cli/main.py:9209-9214); preserve the pause/resume flow and only target processes that can safely be stopped.

[Phoenix0531-sudo]

Rebased onto latest main and addressed the three points from the July-16 sweeper review on the previous push:

1. Ordering: kill happens after gateway pause/snapshot, not before. _kill_hermes_python_processes() now lives in update_cmd.py and is wired in _cmd_update_impl() after _pause_windows_gateways_for_update() returns, so the gateway relaunch argv snapshot is preserved. (update_cmd.py around the --force-kill branch.)

2. Reuses the established termination primitive instead of inline taskkill. Each PID goes through gateway.status.terminate_pid(pid, force=True) — same /T /F tree-kill the rest of the codebase uses — no bespoke subprocess.run(["taskkill", "/PID", str(pid), ...]).

3. Tests added. 6 cases in tests/hermes_cli/test_update_concurrent_quarantine.py, covering: no-op when no holders, no-op off-Windows, every non-Desktop holder gets a force tree-kill, survives individual failures, Desktop backend is refused (not killed), and the --force-kill decision logic. 17/17 pass on CI-parity run.

On the sweeper's note that "the detector says Desktop backends respawn and should be refused rather than killed" — that contract from _detect_venv_python_processes's docstring is now honoured: PIDs whose cmdline contains serve or dashboard (the same classification _format_venv_python_holders_message uses for its ← Hermes Desktop backend hint) are skipped with a printed ⚠ Skipping Hermes Desktop backend — close the Desktop app first message; only gateway / headroom proxy / tui_gateway slash_worker PIDs are force-killed. This keeps --force-kill useful for the issue's two named cases while leaving the Desktop backend alone (the app supervises it and would respawn mid-update).

Diff is 4 files / +259 −2. py_compile, ruff check and the targeted pytest run are all green.

[GottZ]

This was generated by AI during triage.

Summary

One PR, #64386, addresses issue #63300 by adding a Windows --force-kill update path that stops non-Desktop venv Python lock holders while preserving the existing gateway pause/snapshot flow.

Related pull requests

• feat(cli): add --force-kill flag for Windows update (#63300) #64386 best fix — (+259/-2) — n/a: Adds --force-kill, runs termination after gateway pause/snapshot, reuses terminate_pid(pid, force=True), refuses Desktop-supervised backends, and adds focused tests for termination and flag behavior. The current diff addresses the contributor's keep_open review findings on ordering, the termination primitive, missing tests, and Desktop respawn handling.

Suggested consolidation

Keep #64386 open with a salvage path: retain its narrowly targeted non-Desktop lock-holder termination, preserved gateway relaunch state, established tree-kill primitive, and focused tests. Request contributor re-review of the revised diff; there are no duplicate PRs to close.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I63300(["issue #63300 (open)"])
    P64386["PR #64386 (open)"]
    P64386 -->|best fix| I63300
    class I63300 open
    class P64386 open
    class P64386 best
    class P64386 target
    click I63300 "https://github.com/NousResearch/hermes-agent/issues/63300"
    click P64386 "https://github.com/NousResearch/hermes-agent/pull/64386"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (2 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[Phoenix0531-sudo]

Friendly nudge on this one. The previous push (7aa5d05e) addressed the three findings from the July-16 sweeper review and GottZ's triage pass on Aug-3 flagged it as best fix for #63300. No CI has been queued on the branch since then — likely needs the fork-workflow approval to spin up the lint/pytest suite. A quick review (or just workflow approval so CI gives you a green signal to review against) would unblock this.