Skip to content

fix: keep initial blocked kanban tasks inert - #35832

Open
eforos wants to merge 1 commit into
NousResearch:mainfrom
eforos:fix/kanban-blocked-safety-current
Open

fix: keep initial blocked kanban tasks inert#35832
eforos wants to merge 1 commit into
NousResearch:mainfrom
eforos:fix/kanban-blocked-safety-current

Conversation

@eforos

@eforos eforos commented May 31, 2026

Copy link
Copy Markdown

Summary

Fixes a Kanban runtime safety bug where tasks created with initial_status="blocked" could later be auto-promoted by dependency recomputation because task creation did not emit an explicit blocked event.

This also hardens archiving of claimed/running tasks by reclaiming/terminating direct child workers safely and recording archive/termination audit events.

What changed

  • Emit an explicit blocked event when a task is created with initial_status="blocked".
  • Keep initially blocked tasks inert until an explicit manual unblock event.
  • Make archive of claimed/running tasks reclaim the worker lock and terminate only a safe direct-child worker.
  • Use waitpid(..., WNOHANG) checks before signaling to avoid PID-reuse hazards.
  • Record archived atomically with the archive state transition.
  • Add regression tests for sticky blocked creation, promotion prevention, archive teardown, stale/non-child PID safety, and circuit-breaker behavior.

Test plan

Local targeted tests:

scripts/run_tests.sh tests/hermes_cli/test_kanban_initial_blocked_safety.py tests/hermes_cli/test_kanban_db.py
# 213 tests passed, 0 failed

Expanded Kanban surface:

scripts/run_tests.sh \
  tests/hermes_cli/test_kanban_*.py \
  tests/hermes_cli/test_signal_handler_kanban_worker.py \
  tests/hermes_cli/test_pin_kanban_board_env.py \
  tests/tools/test_kanban_tools.py \
  tests/tools/test_kanban_codex_lane_skill.py \
  tests/plugins/test_kanban_dashboard_plugin.py \
  tests/plugins/test_kanban_worker_runs.py \
  tests/plugins/test_kanban_attachments.py \
  tests/gateway/test_kanban_notifier.py
# 877 tests passed, 0 failed

Safety notes

The worker termination path only signals a PID after successfully reclaiming the task lock and verifying the PID is still a direct child via waitpid(pid, WNOHANG). If the PID is not a child or was already reaped, it refuses to signal, preventing accidental termination of unrelated processes after PID reuse.

The archived event is written in the same transaction as the archive state transition, so an audit event is not lost if a crash happens immediately after the state change.

@eforos
eforos force-pushed the fix/kanban-blocked-safety-current branch from dcf6f2e to 68cf9d1 Compare May 31, 2026 11:29
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 31, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #34735 — both fix the same bug where create_task(initial_status='blocked') doesn't emit a blocked event, causing the sticky-block guard (_has_sticky_block) to fail. PR #35832 is a superset that also adds archive teardown hardening; consider merging scope into #34735 or vice versa.

@eforos

eforos commented May 31, 2026

Copy link
Copy Markdown
Author

Thanks for the overlap note. Technical recommendation from the Control Room smoke evidence: keep #35832 as the preferred incident-remediation PR, but treat it as two logical units:

  1. Required/minimal safety fix: create_task(initial_status="blocked") must append a real blocked event so _has_sticky_block recognizes the manual/operator block and recompute_ready cannot promote it blocked → ready without an explicit unblock. This is the same core fix as fix(kanban): emit blocked event when initial_status='blocked' so sticky-block guard fires #34735.
  2. Operational hardening: the archive/reclaim path should clear the active claim/worker state, close the active run as reclaimed, and best-effort terminate/reap only host-local child workers. The original smoke did not only show auto-promotion; it also showed teardown/archive leaving a worker alive/zombie, so this hardening is directly tied to the incident evidence.

If the project wants the smallest merge surface, #34735 is a valid Option A patch and could land first. But then the archive/worker teardown hardening from #35832 should remain as a follow-up, not be dropped. My preferred path is to merge #35832 as the superset if the added teardown tests are acceptable; otherwise split it into “initial blocked event” first and “archive teardown hardening” second.

Local targeted checks run against PR heads:


Comment posted by Hermes Control Room from local evidence review.

@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 the focused sticky-block regression coverage. The initial-status premise is still present on current main: create_task writes status='blocked' at hermes_cli/kanban_db.py:2585, but emits only created at :2670-2683; _has_sticky_block requires a blocked event at :3273-3279, so recompute_ready can promote it at :3336.

Problems

  • Blocking: hermes_cli/kanban_db.py:5011 limits the direct-child/PID-reuse verification to non-Windows paths, but the code still calls kill(pid, SIGTERM) at :5031 on Windows. A stale, recycled worker_pid can therefore identify an unrelated process. The new real-child and stale-PID tests skip Windows at tests/hermes_cli/test_kanban_initial_blocked_safety.py:170-171 and :197-198.

Suggested changes

  • On Windows, do not signal an unverified PID; retain authoritative spawned-process provenance/handles or report termination as unavailable. Add a Windows regression for stale PID safety.
  • Preserve the initial blocked-event fix; it directly completes the existing sticky-block contract.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
if kill is None:
return info

real_signal_path = signal_fn is None and os.name != "nt"

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.

Blocking: this bypasses the direct-child/PID-reuse guard on Windows, but the function still falls through to kill(pid, SIGTERM) at line 5031. A stale same-host PID can be recycled into an unrelated Windows process. Do not signal an unverified Windows PID; retain authoritative spawned-process provenance or return an unattempted result, and add a Windows regression.

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 P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants