Skip to content

Kill Kanban worker process trees on Windows timeout - #49374

Open
sdgdllc wants to merge 5 commits into
NousResearch:mainfrom
sdgdllc:hermes/windows-git-bash-shim-fix
Open

Kill Kanban worker process trees on Windows timeout#49374
sdgdllc wants to merge 5 commits into
NousResearch:mainfrom
sdgdllc:hermes/windows-git-bash-shim-fix

Conversation

@sdgdllc

@sdgdllc sdgdllc commented Jun 20, 2026

Copy link
Copy Markdown

Summary

  • On Windows, terminate Kanban worker process trees with taskkill /T /F when max-runtime or reclaim cleanup kills a worker.
  • Preserve the existing signal_fn hook path for unit tests and non-Windows behavior.
  • Record process-tree cleanup metadata in timeout/reclaim event payloads.

Why

A dispatcher timeout can kill only the recorded worker PID while leaving stdio MCP descendants alive (node, npx, Playwright MCP, AgentMemory MCP, local runtime MCP). That leaks tool servers after a Kanban card times out and degrades the host.

Verification

  • C:\Users\m_t_c\.hermes-sdgd\venv\Scripts\python.exe -m pytest -o addopts= -p no:timeout tests\hermes_cli\test_kanban_db.py -k "max_runtime or terminate_worker_pid"

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management backend/local Local shell execution P3 Low — cosmetic, nice to have labels Jun 20, 2026

@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 addressing a real Windows cleanup gap: current main still uses direct PID signaling in hermes_cli/kanban_db.py:6163-6197 and :6358-6378, while the established Windows tree-kill convention is taskkill /T /F in gateway/status.py:98-125.

Problems

  • hermes_cli/kanban_db.py:5148 uses subprocess.CREATE_NO_WINDOW directly. The new test sets _IS_WINDOWS=True while running under Linux (tests/hermes_cli/test_kanban_db.py:941-945), where that attribute does not exist, so the test raises AttributeError before reaching the mocked taskkill call.
  • The PR diff includes unrelated channel-directory, Discord, and Git-Bash fixes. gh pr diff 49374 --name-only lists nine files, while commit 8ef3f040245c itself changes only the Kanban implementation and its test.

Suggested changes

  • Use the platform-safe hide-flags helper already used by gateway/status.py:108-117, then keep the mocked-Windows test runnable on Linux.
  • Salvage the focused Kanban change onto current main, applying it to both enforce_max_runtime and _terminate_reclaimed_worker so timeout and reclaim cleanup share the same tree-kill behavior.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
stderr=subprocess.DEVNULL,
timeout=10,
check=False,
creationflags=subprocess.CREATE_NO_WINDOW,

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.

CREATE_NO_WINDOW is absent from Linux's subprocess module. Because the new test sets _IS_WINDOWS=True on Linux, this line raises AttributeError before the mocked taskkill runs. Use the platform-safe hide-flags helper or a guarded lookup so the test remains cross-platform.

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

Labels

backend/local Local shell execution comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants