Skip to content

fix(kanban): reap a superseded run's live worker before respawn - #109747

Open
tahleen wants to merge 1 commit into
NousResearch:mainfrom
tahleen:fix/kanban-reap-superseded-workers
Open

tahleen wants to merge 1 commit into
NousResearch:mainfrom
tahleen:fix/kanban-reap-superseded-workers

Conversation

@tahleen

@tahleen tahleen commented Sep 13, 2026

Copy link
Copy Markdown

What this fixes

A kanban run can reach a terminal handoff (kanban_request_review, kanban_request_changes, kanban_complete) and close in the DB while its worker process is still alive — the turn loop keeps calling the model against a run that no longer exists, and the process keeps holding tool calls. When the dispatcher then claims the same card again (review lane, then the fix lane), the previous process is still there: two workers mutate one card.

Measured on 2026-09-07: a worker whose kanban_request_review had already succeeded kept emitting tool calls for ~30 minutes (heartbeats, diagnostics, board calls rejected with expected_run_id mismatch). The dispatcher stacked runs behind it — 4–6 concurrent workers on one card (t_e46a5fea, runs 30 then 32-36), each one holding a claim slot. The reap closes that window at the source: before respawning a task, kill whatever is left over from its already-closed runs.

What it changes (dispatcher side only)

hermes_cli/kanban_db_dispatch.py:

  • _spawned_event_pids(conn, task_id) — (run_id, pid) pairs read back from the task's append-only spawned events, newest first. This is the only durable trace of a run's PID: _end_run() clears task_runs.worker_pid when the run closes, so a monitor cannot find the stale process from the run row.
  • _worker_query(task_id) — single source of the task-scoped query _worker_argv() passes to chat -q, so the guard below cannot drift from what is actually spawned.
  • _cmdline_belongs_to_kanban_worker(pid, task_id) — PID-recycling guard: signal only a PID whose live cmdline still carries that task-scoped query, never a process that merely inherited the number.
  • _reap_superseded_run_workers(conn, task_id, keep_run_id=...) — SIGTERM then SIGKILL, only for runs that are already closed (task_runs.outcome IS NOT NULL), emitting a reaped_superseded_worker event.
  • Call site: _dispatch_lane_task(), immediately before the spawn, inside a try/except logged at debug level, so a failing reap can never block the dispatch path.

Existing termination paths on main (claim-expiry reclaim, operator reclaim, enforce_max_runtime, crash detection, archive_task) all key off the task leaving/staying in a state they can see; none of them covers "a new run of this card is about to spawn while the previous process is still alive", which is exactly the goal-mode review/fix cycle.

Tests

tests/hermes_cli/test_kanban_reap_superseded_workers.py (new, 4 tests):

  • a closed run's live worker is SIGTERMed; an open run's worker is not;
  • an innocent live PID (alive, but not this task's worker) is never signalled — asserted by making the terminate helper fail the test if it is called;
  • no-op when the task has no spawned events;
  • test_full_review_fix_cycle_keeps_single_live_worker: replays implementation → request_review → request_changes → request_review → complete through the real dispatch_once, with fake workers that do not exit after their terminal tool (the bug), asserting one live worker after every tick.

Local evidence on base origin/main b6b53c69a6:

  • scripts/run_tests.sh tests/hermes_cli/test_kanban_reap_superseded_workers.py → 4 passed
  • scripts/run_tests.sh tests/hermes_cli/test_kanban*.py → 53 files, 342 passed, 0 failed, 1 skipped (the skip is a windows_only test on Linux)
  • uvx ruff check on both files → clean; py_compile clean

Related prior art

Notes

Opened from a fork (tahleen/hermes-agent), so there is no CI on the branch — the evidence above is local, on a fresh rebase of origin/main.

A run that reaches a terminal handoff (request_review / request_changes /
complete_task) closes in the DB while its worker process can still be alive:
the turn loop keeps calling the model against a run that no longer exists.
When the dispatcher then claims the same task again (review lane, then the
fix lane), the previous process is still there, so two workers mutate one
card concurrently.

Observed on 2026-09-07: a worker whose kanban_request_review had succeeded
kept emitting tool calls for ~30 minutes, and one card accumulated runs
30 and 32-36, each of them holding a claim slot.

task_runs.worker_pid cannot be used to find the stale process: _end_run
clears it when the run closes. The append-only `spawned` event payload is
the only durable trace of a run's worker PID, so the reap reads it back and
terminates those processes before the replacement spawns:

- _spawned_event_pids(): (run_id, pid) pairs from the task's `spawned`
  events, newest first.
- _cmdline_belongs_to_kanban_worker(): PID-recycling guard — only signal a
  PID whose live cmdline still carries the task-scoped query built by
  _worker_query(), i.e. the very string _worker_argv passes to `chat -q`.
- _reap_superseded_run_workers(): SIGTERM then SIGKILL, only for runs that
  are already closed, emitting `reaped_superseded_worker`.
- Call site: _dispatch_lane_task, right before the spawn, best-effort inside
  a try/except so a reap failure can never block the dispatch path.

Tests (tests/hermes_cli/test_kanban_reap_superseded_workers.py):
- a closed run's live worker is terminated, an open run's is not;
- an innocent PID (alive, but not this task's worker) is never signalled;
- noop without spawned events;
- the full implementation -> review -> fix -> review -> complete cycle
  replayed through the real dispatch_once, with fake workers that do NOT exit
  after their terminal tool (the bug), asserting a single live worker after
  every tick.

This branch has not been deployed

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

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants