[kanban] archive_task: kill the worker process instead of leaving it orphaned by PINKIIILQWQ · Pull Request #42858 · NousResearch/hermes-agent

PINKIIILQWQ · 2026-06-09T13:29:55Z

Problem

archive_task() is a pure-DB operation — it UPDATE status='archived', worker_pid=NULL but never sends SIGTERM to the OS process. The worker stays alive as an orphan until it next calls kanban_complete/kanban_block and discovers its task was archived, burning API quota and compute.

Root cause: archive_task() was written as a counterpart to delete_archived_task() — a data-layer operation that only touches rows. The kill responsibility was never added.

Compare with reclaim_task() (L3273) which correctly snapshots worker_pid+claim_lock before the DB update and calls _terminate_reclaimed_worker() — sending SIGTERM → 5s wait → SIGKILL if needed.

Fix

18 lines added to archive_task(), following the same order as reclaim_task:

Snapshot worker_pid+claim_lock via SELECT before the write_txn clears them
Kill via _terminate_reclaimed_worker(pid, lock) — same function, same SIGTERM→SIGKILL sequence
Log termination metadata (prev_pid, terminated, sigkill, host_local) in the 'archived' event payload

Non-running / non-local tasks are safe no-ops (_terminate_reclaimed_worker returns immediately when pid is None or claim_lock doesn't match the local host prefix).

Risk

Low. Pattern is identical to reclaim_task (production-stable). No schema change, no API change, no test breakage.

archive_task() was a pure DB operation — it cleared worker_pid, claim_lock, and status from the tasks row but never sent SIGTERM to the actual OS process. A running worker stayed alive until it next called kanban_complete/kanban_block and discovered it was archived, burning API quota and compute resources. Fix: snapshot pid+claim_lock before the write_txn clears them, then call _terminate_reclaimed_worker() — same function reclaim_task uses — which sends SIGTERM, waits 5s, then SIGKILL if still alive. The termination metadata is included in the 'archived' event so operators can see what happened. Order matches reclaim_task: terminate first, then DB update. For non-running / non-local tasks, _terminate_reclaimed_worker returns immediately as a no-op. Closes NousResearch#33774 reprise: the scratch-workspace side was fixed in fc8afd5, but the orphaned-process side was never addressed.

tonydwb

Code Review: Approved. Good bug fix - archive_task now kills the worker process instead of leaving it orphaned. Clean implementation.

release_stale_claims and detect_stale_running call _terminate_reclaimed_worker and then release the task claim unconditionally, even when the termination did not actually kill the worker. _terminate_reclaimed_worker already reports this via its "terminated" flag, but the callers ignore it. When a worker is parked in uninterruptible (D) state — for example throttled by a cgroup memory.high limit — a pending SIGTERM/SIGKILL cannot be delivered until the throttle lifts, so the kill is a no-op. The dispatcher then frees the claim and spawns a fresh worker beside the still-alive one. Repeated every dispatch tick this accumulates duplicate workers without bound, deepening the memory pressure that caused the throttle in the first place — a self-reinforcing runaway. Fix: gate both automatic reclaim paths on _worker_survived_termination(). When we attempted to kill our own host-local worker and it is still alive, defer the reclaim (_defer_reclaim_for_live_worker extends the claim a short grace and emits a reclaim_deferred event) instead of releasing. This guarantees at most one live worker per task and is self-correcting: not spawning a duplicate is what relieves the pressure so the pending signal lands and the worker dies, and the next tick reclaims cleanly. Non-host-local claims and the operator-driven reclaim_task() path keep their existing force-release behaviour. Related: #41448 (concurrent dispatchers amplify this by doubling reclaim frequency); #42858 (kill the worker rather than orphan it on archive). Tests: defer-when-worker-survives, reclaim-when-killed, release-when-not-host-local, and the detect_stale_running path.

release_stale_claims and detect_stale_running call _terminate_reclaimed_worker and then release the task claim unconditionally, even when the termination did not actually kill the worker. _terminate_reclaimed_worker already reports this via its "terminated" flag, but the callers ignore it. When a worker is parked in uninterruptible (D) state — for example throttled by a cgroup memory.high limit — a pending SIGTERM/SIGKILL cannot be delivered until the throttle lifts, so the kill is a no-op. The dispatcher then frees the claim and spawns a fresh worker beside the still-alive one. Repeated every dispatch tick this accumulates duplicate workers without bound, deepening the memory pressure that caused the throttle in the first place — a self-reinforcing runaway. Fix: gate both automatic reclaim paths on _worker_survived_termination(). When we attempted to kill our own host-local worker and it is still alive, defer the reclaim (_defer_reclaim_for_live_worker extends the claim a short grace and emits a reclaim_deferred event) instead of releasing. This guarantees at most one live worker per task and is self-correcting: not spawning a duplicate is what relieves the pressure so the pending signal lands and the worker dies, and the next tick reclaims cleanly. Non-host-local claims and the operator-driven reclaim_task() path keep their existing force-release behaviour. Related: NousResearch#41448 (concurrent dispatchers amplify this by doubling reclaim frequency); NousResearch#42858 (kill the worker rather than orphan it on archive). Tests: defer-when-worker-survives, reclaim-when-killed, release-when-not-host-local, and the detect_stale_running path.

teknium1

Thanks for addressing a real cleanup gap: current archive_task() clears worker_pid without invoking the existing termination helper (hermes_cli/kanban_db.py:5431-5449).

Problems

The new call at hermes_cli/kanban_db.py:4617 occurs after an unlocked snapshot but before the archive UPDATE. A concurrent caller can win the archive transition while this caller still signals the previously read PID, even though its UPDATE later affects zero rows. Please make the snapshot/termination contingent on this invocation winning the archive transition.
The PR changes only hermes_cli/kanban_db.py; it adds no regression coverage. Existing archive-run coverage (tests/hermes_cli/test_kanban_core_functionality.py:1952) checks closure of the run, not worker termination or archived-event metadata.

Suggested changes

Add a monkeypatched termination test for a local PID/claim and assert the archived event payload; cover a non-local claim as a no-op.

Automated hermes-sweeper review.

teknium1 · 2026-07-14T08:49:34Z

+    ).fetchone()
+    prev_pid = row["worker_pid"] if row else None
+    prev_lock = row["claim_lock"] if row else None
+    # Terminate the worker process first (same order as reclaim_task).


This signal is sent before the archive UPDATE establishes that this invocation won the transition. Two concurrent archive requests can both read the old PID; the loser can still terminate it even when its conditional UPDATE returns zero rows. Please serialize the snapshot with the successful archive transition so termination is only performed for the state change this call owns.

release_stale_claims and detect_stale_running call _terminate_reclaimed_worker and then release the task claim unconditionally, even when the termination did not actually kill the worker. _terminate_reclaimed_worker already reports this via its "terminated" flag, but the callers ignore it. When a worker is parked in uninterruptible (D) state — for example throttled by a cgroup memory.high limit — a pending SIGTERM/SIGKILL cannot be delivered until the throttle lifts, so the kill is a no-op. The dispatcher then frees the claim and spawns a fresh worker beside the still-alive one. Repeated every dispatch tick this accumulates duplicate workers without bound, deepening the memory pressure that caused the throttle in the first place — a self-reinforcing runaway. Fix: gate both automatic reclaim paths on _worker_survived_termination(). When we attempted to kill our own host-local worker and it is still alive, defer the reclaim (_defer_reclaim_for_live_worker extends the claim a short grace and emits a reclaim_deferred event) instead of releasing. This guarantees at most one live worker per task and is self-correcting: not spawning a duplicate is what relieves the pressure so the pending signal lands and the worker dies, and the next tick reclaims cleanly. Non-host-local claims and the operator-driven reclaim_task() path keep their existing force-release behaviour. Related: NousResearch#41448 (concurrent dispatchers amplify this by doubling reclaim frequency); NousResearch#42858 (kill the worker rather than orphan it on archive). Tests: defer-when-worker-survives, reclaim-when-killed, release-when-not-host-local, and the detect_stale_running path.

PINKIIILQWQ force-pushed the fix/archive-kill-worker-process branch from 2049784 to a93fb37 Compare June 9, 2026 13:36

PINKIIILQWQ marked this pull request as ready for review June 9, 2026 13:38

alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 9, 2026

tonydwb approved these changes Jun 10, 2026

View reviewed changes

Sahil-SS9 mentioned this pull request Jun 12, 2026

fix(kanban): hold reclaim while the worker is still alive #44909

Closed

This was referenced Jul 3, 2026

fix(kanban): terminate worker process on block/archive/complete (#57596) #57602

Closed

fix(kanban): terminate workers when tasks are blocked or archived #57713

Closed

teknium1 reviewed Jul 14, 2026

View reviewed changes

Navigation Menu

Uh oh!

[kanban] archive_task: kill the worker process instead of leaving it orphaned - #42858

[kanban] archive_task: kill the worker process instead of leaving it orphaned#42858
PINKIIILQWQ wants to merge 1 commit into
NousResearch:mainfrom
PINKIIILQWQ:fix/archive-kill-worker-process

PINKIIILQWQ commented Jun 9, 2026

Uh oh!

tonydwb left a comment

Uh oh!

teknium1 left a comment

Uh oh!

teknium1 Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

PINKIIILQWQ commented Jun 9, 2026

Problem

Fix

Risk

Related

Uh oh!

tonydwb left a comment

Choose a reason for hiding this comment

Uh oh!

teknium1 left a comment

Choose a reason for hiding this comment

Problems

Suggested changes

Uh oh!

teknium1 Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants