Skip to content

fix(kanban): terminate live workers on manual exits from running - #31365

Open
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/kanban-terminate-orphan-workers
Open

fix(kanban): terminate live workers on manual exits from running#31365
Dusk1e wants to merge 1 commit into
NousResearch:mainfrom
Dusk1e:fix/kanban-terminate-orphan-workers

Conversation

@Dusk1e

@Dusk1e Dusk1e commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Manual running -> done/blocked/scheduled/archived transitions now best-effort terminate the live claimed worker before clearing run metadata.
  • Added a shared helper in hermes_cli/kanban_db.py so CLI/core paths and dashboard direct-status moves use the same worker termination logic.
  • Preserved the self-completing worker path with a guard that skips killing the current process when the worker is closing its own run.
  • Added regression coverage for complete_task, archive_task, and dashboard drag-drop off running.

Validation

  • PASS: python -m pytest tests/hermes_cli/test_kanban_db.py tests/plugins/test_kanban_dashboard_plugin.py -q --timeout-method=thread -k "complete_task_terminates_foreign_running_worker or complete_task_skips_terminating_self_worker or archive_task_terminates_foreign_running_worker or patch_status_ready_terminates_running_worker or patch_status_archive_closes_running_run or patch_status_done_with_summary_and_metadata"
    Result: 6 passed, 257 deselected
  • PASS: manual smoke repro with a real sleeping subprocess
    Result: complete_ok=True, status=done, worker_pid=None, run_outcome=completed, proc_exited=True, returncode=15

Additional broader check

  • PARTIAL: python -m pytest tests/hermes_cli/test_kanban_db.py tests/plugins/test_kanban_dashboard_plugin.py --timeout-method=thread -vv --maxfail=3
    Result before stop: 125 passed, 3 failed
  • The 3 failures were unrelated to this change and came from existing _resolve_hermes_argv expectations on this Windows/Python shim environment:
    • test_resolve_hermes_argv_prefers_path_shim
    • test_resolve_hermes_argv_hermes_bin_bare_name_uses_path
    • test_resolve_hermes_argv_falls_back_to_module_form_when_no_path_shim

Notes

  • I had to run pytest with --timeout-method=thread because this Windows environment does not support the default SIGALRM path used by pytest-timeout.
  • There is also a non-blocking pytest temp cleanup PermissionError at process exit on this machine; it does not affect the kanban fix itself.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for addressing a real worker-lifecycle gap. Current complete_task clears worker_pid while moving a task to done (hermes_cli/kanban_db.py:4053-4069), and the same gap remains in archive, schedule, and dashboard direct-status transitions (hermes_cli/kanban_db.py:5431-5449, 5812-5854; plugins/kanban/dashboard/plugin_api.py:1023-1039).

Problems

  • The diff adds termination behavior to block_task and schedule_task, but its new tests cover only complete, archive, and dashboard drag-drop. Current block_task has separate dependency, loop-routing, and ordinary branches that clear worker_pid (hermes_cli/kanban_db.py:4825-4967), so these paths need explicit coverage too.

Suggested changes

  • Add foreign-worker and self-worker regression coverage for manual running -> blocked and running -> scheduled transitions when salvaging this into the current implementation.
  • The base has substantially moved; port the helper deliberately into current lifecycle code rather than applying the old hunks mechanically.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins 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.

3 participants