From 5b5c79a8ef4317146b0db7cc2bbb7495c1748e63 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Thu, 25 Jun 2026 21:46:58 -0700 Subject: [PATCH] feat(kanban): typed block reasons + unblock-loop breaker (#52848) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(kanban): typed block reasons + unblock-loop breaker Stops the kanban blocked-task loop: a worker blocks a task, a cron unblocks it, the worker re-blocks for the same reason, repeat forever. block_task now takes a typed kind and a persistent block_recurrences counter on the tasks table: - kind=dependency routes to todo (parent-gated, auto-resumed), never the human 'blocked' bucket a cron would keep unblocking. - needs_input/capability/transient/untyped land in blocked; each same-cause re-block after an unblock increments block_recurrences, and at BLOCK_RECURRENCE_LIMIT (default 2) the task routes to triage for a human instead of blocked. - unblock_task no longer resets block_recurrences (the amnesia that let the loop run unbounded); complete_task clears it on success. Wired through the worker kanban_block tool (new kind arg) and the hermes kanban block --kind CLI flag, both reporting where the task actually landed. Docs + 11 new tests; 536 existing kanban tests green. * test(kanban): make second-block notify test use a distinct block cause test_notifier_second_blocked_delivers blocked the same task twice with the same (untyped) reason, which now trips the new unblock-loop breaker and routes the second block to triage instead of blocked — so only one 'blocked' notification fired. The test's actual intent is that TWO distinct block cycles each notify; give the two cycles different kinds (needs_input then capability) so they're genuinely separate blocks. The same-cause loop→triage path is covered by test_kanban_block_kinds.py. --- hermes_cli/kanban.py | 27 +- hermes_cli/kanban_db.py | 328 +++++++++++++++++--- tests/hermes_cli/test_kanban_block_kinds.py | 202 ++++++++++++ tests/hermes_cli/test_kanban_notify.py | 12 +- tools/kanban_tools.py | 47 ++- website/docs/user-guide/features/kanban.md | 8 +- 6 files changed, 563 insertions(+), 61 deletions(-) create mode 100644 tests/hermes_cli/test_kanban_block_kinds.py diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 347165b6269..7fc7bf94895 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -559,6 +559,16 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_block.add_argument("reason", nargs="*", help="Reason (also appended as a comment)") p_block.add_argument("--ids", nargs="+", default=None, help="Additional task ids to block with the same reason (bulk mode)") + p_block.add_argument( + "--kind", default=None, choices=sorted(kb.VALID_BLOCK_KINDS), + help=( + "Typed block reason. 'dependency' waits in todo (auto-promoted " + "when parents finish, no human); 'needs_input'/'capability' go to " + "blocked for a human; 'transient' marks a maybe-flaky failure. " + "Repeated same-kind re-blocks after unblock route the task to " + "triage to break unblock loops. Omit for a generic block." + ), + ) p_schedule = sub.add_parser("schedule", help="Park one or more tasks in Scheduled (waiting on time, not human input)") p_schedule.add_argument("task_id") @@ -1928,6 +1938,7 @@ def _cmd_edit(args: argparse.Namespace) -> int: def _cmd_block(args: argparse.Namespace) -> int: reason = " ".join(args.reason).strip() if args.reason else None + kind = getattr(args, "kind", None) author = _profile_author() ids = [args.task_id] + list(getattr(args, "ids", None) or []) failed: list[str] = [] @@ -1939,12 +1950,26 @@ def _cmd_block(args: argparse.Namespace) -> int: conn, tid, reason=reason, + kind=kind, expected_run_id=_worker_run_id_for(tid), ): failed.append(tid) print(f"cannot block {tid}", file=sys.stderr) else: - print(f"Blocked {tid}" + (f": {reason}" if reason else "")) + # Report where the task actually landed — dependency blocks go + # to todo, and a tripped unblock-loop breaker routes to triage. + landed = kb.get_task(conn, tid) + where = landed.status if landed else "blocked" + suffix = f": {reason}" if reason else "" + if where == "todo": + print(f"{tid} → todo (dependency wait){suffix}") + elif where == "triage": + print( + f"{tid} → triage (unblock loop detected — needs a " + f"human decision){suffix}" + ) + else: + print(f"Blocked {tid}{suffix}") return 0 if not failed else 1 diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 5e014975589..4b031287f12 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -100,6 +100,37 @@ _log = logging.getLogger(__name__) VALID_STATUSES = {"triage", "todo", "scheduled", "ready", "running", "blocked", "review", "done", "archived"} VALID_INITIAL_STATUSES = {"running", "blocked"} + +# Typed block reasons. Distinguishes the two fundamentally different things a +# worker (or human) means by "blocked", so each can be routed differently +# instead of all landing in one undifferentiated ``blocked`` bucket that a cron +# unblocks → worker re-blocks → cron unblocks … forever. +# +# * ``dependency`` — can't proceed until another task finishes. Routed to +# ``todo`` (NOT ``blocked``) so the existing +# parent-gating / ``recompute_ready`` machinery promotes +# it automatically once parents are done. No human, no +# cron, no retry storm. +# * ``needs_input`` — needs a human decision/answer it cannot derive. +# * ``capability`` — hit a hard wall (no access, missing creds, an action no +# AI agent can perform). Genuinely human-only. +# * ``transient`` — a flaky/temporary failure that may clear on retry. +# +# ``needs_input`` and ``capability`` are "truly blocked": they go to ``blocked`` +# for a human, and the unblock-loop breaker (see ``block_task`` / +# ``BLOCK_RECURRENCE_LIMIT``) escalates them to ``triage`` if a cron keeps +# unblocking them only to have the worker re-block for the same reason. +# ``None`` = legacy/un-typed block (treated as a generic human blocker). +VALID_BLOCK_KINDS = {"dependency", "needs_input", "capability", "transient"} + +# After a task has been blocked, unblocked, and re-blocked this many times for +# the same (truly-blocked) reason, the unblock-loop breaker stops trusting the +# unblocker (usually a cron) and routes the task to ``triage`` instead of back +# to ``blocked`` — breaking the infinite unblock↔re-block loop and forcing a +# human-in-the-loop decision. Mirrors the dispatcher's ``DEFAULT_FAILURE_LIMIT`` +# spirit (default 2) but counts a different signal: manual unblock recurrences, +# not dispatcher spawn/crash/timeout failures. +BLOCK_RECURRENCE_LIMIT = 2 VALID_WORKSPACE_KINDS = {"scratch", "worktree", "dir"} KNOWN_TOOLSET_NAMES = frozenset(name.casefold() for name in get_toolset_names()) _IS_WINDOWS = sys.platform == "win32" @@ -838,6 +869,13 @@ class Task: # set the env var. Lets clients render a per-session board without # relying on tenant + time-window heuristics. session_id: Optional[str] = None + # Typed block reason (one of VALID_BLOCK_KINDS) or None for legacy/un-typed + # blocks. Set by ``block_task``; preserved across unblock so a re-block for + # the same kind is recognisable as an unblock↔re-block loop. + block_kind: Optional[str] = None + # Unblock-loop counter. See the column comment in SCHEMA_SQL and + # ``BLOCK_RECURRENCE_LIMIT``. Reset only on successful completion. + block_recurrences: int = 0 @classmethod def from_row(cls, row: sqlite3.Row) -> "Task": @@ -914,6 +952,14 @@ class Task: session_id=( row["session_id"] if "session_id" in keys else None ), + block_kind=( + row["block_kind"] if "block_kind" in keys and row["block_kind"] else None + ), + block_recurrences=( + int(row["block_recurrences"]) + if "block_recurrences" in keys and row["block_recurrences"] is not None + else 0 + ), ) @@ -1078,7 +1124,20 @@ CREATE TABLE IF NOT EXISTS tasks ( -- for tasks created from the CLI, dashboard, or any path that doesn't -- set the env var. Indexed so per-session list queries stay cheap on -- larger boards. - session_id TEXT + session_id TEXT, + -- Typed block reason set by ``block_task`` (one of VALID_BLOCK_KINDS, or + -- NULL for legacy/un-typed blocks). Drives routing: ``dependency`` never + -- sits in ``blocked`` (goes to ``todo`` for parent-gating); the others go + -- to ``blocked`` for a human. Preserved across unblock so a re-block for + -- the SAME kind can be recognised as a loop. + block_kind TEXT, + -- Unblock-loop counter. Incremented each time a task is re-blocked for the + -- same truly-blocked reason after having been unblocked. When it reaches + -- BLOCK_RECURRENCE_LIMIT the task is routed to ``triage`` instead of + -- ``blocked`` so a cron can't spin it forever. Reset to 0 only on a + -- successful completion — NOT on unblock (resetting on unblock is exactly + -- the amnesia that let the loop run unbounded). + block_recurrences INTEGER NOT NULL DEFAULT 0 ); CREATE TABLE IF NOT EXISTS task_links ( @@ -1873,6 +1932,22 @@ def _migrate_add_optional_columns(conn: sqlite3.Connection) -> None: conn, "tasks", "session_id", "session_id TEXT" ) + if "block_kind" not in cols: + # Typed block reason (VALID_BLOCK_KINDS) or NULL for legacy/un-typed + # blocks. Existing blocked rows get NULL, which is treated as a + # generic human blocker — same behaviour they had before the column. + _add_column_if_missing(conn, "tasks", "block_kind", "block_kind TEXT") + + if "block_recurrences" not in cols: + # Unblock-loop counter. Existing rows start at 0, so the loop breaker + # only begins counting from the first re-block after this migration. + _add_column_if_missing( + conn, + "tasks", + "block_recurrences", + "block_recurrences INTEGER NOT NULL DEFAULT 0", + ) + # Indexes over additive ``tasks`` columns must be created after the # columns exist. Keeping them in SCHEMA_SQL breaks legacy boards: SQLite # parses each statement in ``executescript`` against the live schema, so a @@ -3898,7 +3973,9 @@ def complete_task( completed_at = ?, claim_lock = NULL, claim_expires= NULL, - worker_pid = NULL + worker_pid = NULL, + block_kind = NULL, + block_recurrences = 0 WHERE id = ? AND status IN ('running', 'ready', 'blocked') """, @@ -3913,7 +3990,9 @@ def complete_task( completed_at = ?, claim_lock = NULL, claim_expires= NULL, - worker_pid = NULL + worker_pid = NULL, + block_kind = NULL, + block_recurrences = 0 WHERE id = ? AND status IN ('running', 'ready', 'blocked') AND current_run_id = ? @@ -4385,53 +4464,204 @@ def block_task( task_id: str, *, reason: Optional[str] = None, + kind: Optional[str] = None, expected_run_id: Optional[int] = None, ) -> bool: - """Transition ``running -> blocked``.""" - with write_txn(conn): - if expected_run_id is None: - cur = conn.execute( - """ - UPDATE tasks - SET status = 'blocked', - claim_lock = NULL, - claim_expires= NULL, - worker_pid = NULL - WHERE id = ? - AND status IN ('running', 'ready') - """, - (task_id,), - ) - else: - cur = conn.execute( - """ - UPDATE tasks - SET status = 'blocked', - claim_lock = NULL, - claim_expires= NULL, - worker_pid = NULL - WHERE id = ? - AND status IN ('running', 'ready') - AND current_run_id = ? - """, - (task_id, int(expected_run_id)), - ) - if cur.rowcount != 1: - return False - run_id = _end_run( - conn, task_id, - outcome="blocked", status="blocked", - summary=reason, + """Transition ``running``/``ready`` → ``blocked`` (or route elsewhere). + + ``kind`` (one of :data:`VALID_BLOCK_KINDS`, or ``None`` for a legacy + un-typed block) drives routing instead of every block landing in one + undifferentiated ``blocked`` bucket: + + * ``dependency`` — the task is only waiting on another task. It does NOT + sit in ``blocked`` (where a cron would keep "unblocking" it); it goes to + ``todo`` so the existing parent-gating / ``recompute_ready`` machinery + promotes it automatically once its parents finish. No human, no cron, no + retry storm. This is Dale's "Type 2 — dependency blocked". + + * ``needs_input`` / ``capability`` / ``None`` — "truly blocked" (Dale's + "Type 1"). Lands in ``blocked`` for a human. BUT: each time such a task + is re-blocked for the SAME kind after having been unblocked, the + unblock-loop counter (``block_recurrences``) increments. When it reaches + :data:`BLOCK_RECURRENCE_LIMIT`, the task is routed to ``triage`` instead + of ``blocked`` — breaking the cron-unblock ↔ worker-re-block loop and + forcing a human-in-the-loop triage decision. + + * ``transient`` — treated like a generic block for routing, but a worker + can use it to signal "this might clear on its own"; it still participates + in the loop breaker so a forever-flaky task eventually escalates. + + Returns True on any successful transition (to ``blocked``, ``todo``, or + ``triage``), False when the task wasn't in a blockable state. + """ + if kind is not None and kind not in VALID_BLOCK_KINDS: + raise ValueError( + f"block kind must be one of {sorted(VALID_BLOCK_KINDS)} or None" ) - # Synthesize a run when blocking a never-claimed task so the - # reason is preserved in attempt history. - if run_id is None and reason: - run_id = _synthesize_ended_run( + routed_to = "blocked" + recurrences = 0 + with write_txn(conn): + cur_row = conn.execute( + "SELECT status, block_kind, block_recurrences FROM tasks WHERE id = ?", + (task_id,), + ).fetchone() + if cur_row is None: + return False + prev_kind = cur_row["block_kind"] if "block_kind" in cur_row.keys() else None + prev_recurrences = ( + int(cur_row["block_recurrences"]) + if "block_recurrences" in cur_row.keys() + and cur_row["block_recurrences"] is not None + else 0 + ) + + # Dependency blocks never enter the human ``blocked`` bucket — they + # wait in ``todo`` and let ``recompute_ready`` gate on parents. Routing + # here (rather than ``blocked``) is what keeps a cron from ever seeing + # a dependency-wait as something to "unblock". + if kind == "dependency": + cur = conn.execute( + """ + UPDATE tasks + SET status = 'todo', + claim_lock = NULL, + claim_expires = NULL, + worker_pid = NULL, + block_kind = ? + WHERE id = ? + AND status IN ('running', 'ready') + """ + ("" if expected_run_id is None else " AND current_run_id = ?"), + (kind, task_id) if expected_run_id is None + else (kind, task_id, int(expected_run_id)), + ) + if cur.rowcount != 1: + return False + run_id = _end_run( conn, task_id, - outcome="blocked", + outcome="blocked", status="blocked", summary=reason, ) - _append_event(conn, task_id, "blocked", {"reason": reason}, run_id=run_id) + if run_id is None and reason: + run_id = _synthesize_ended_run( + conn, task_id, outcome="blocked", summary=reason, + ) + _append_event( + conn, task_id, "dependency_wait", + {"reason": reason, "kind": kind}, run_id=run_id, + ) + routed_to = "todo" + _blocked_task = get_task(conn, task_id) + _fire_kanban_lifecycle_hook( + "kanban_task_blocked", + task_id, + board=get_current_board(), + assignee=_blocked_task.assignee if _blocked_task else None, + run_id=run_id, + reason=reason, + ) + return True + + # Truly-blocked kinds. Increment the unblock-loop counter when this is a + # re-block for the SAME reason after a prior unblock. block_task only + # fires from running/ready (i.e. AFTER an unblock returned the task to + # the work pool), so a stored block_kind that matches the incoming kind + # means: blocked → unblocked → about-to-re-block for the same cause. + # An un-typed (None) block compares as "same" to a prior un-typed block. + same_cause = prev_kind == kind + recurrences = prev_recurrences + 1 if same_cause else 1 + + if recurrences >= BLOCK_RECURRENCE_LIMIT: + # Loop detected — stop letting the unblocker spin this task. Route + # to triage for a human-in-the-loop decision instead of blocked. + cur = conn.execute( + """ + UPDATE tasks + SET status = 'triage', + claim_lock = NULL, + claim_expires = NULL, + worker_pid = NULL, + block_kind = ?, + block_recurrences = ? + WHERE id = ? + AND status IN ('running', 'ready') + """ + ("" if expected_run_id is None else " AND current_run_id = ?"), + (kind, recurrences, task_id) if expected_run_id is None + else (kind, recurrences, task_id, int(expected_run_id)), + ) + if cur.rowcount != 1: + return False + run_id = _end_run( + conn, task_id, + outcome="blocked", status="blocked", + summary=reason, + ) + if run_id is None and reason: + run_id = _synthesize_ended_run( + conn, task_id, outcome="blocked", summary=reason, + ) + _append_event( + conn, task_id, "block_loop_detected", + { + "reason": reason, + "kind": kind, + "recurrences": recurrences, + "limit": BLOCK_RECURRENCE_LIMIT, + }, + run_id=run_id, + ) + routed_to = "triage" + else: + if expected_run_id is None: + cur = conn.execute( + """ + UPDATE tasks + SET status = 'blocked', + claim_lock = NULL, + claim_expires = NULL, + worker_pid = NULL, + block_kind = ?, + block_recurrences = ? + WHERE id = ? + AND status IN ('running', 'ready') + """, + (kind, recurrences, task_id), + ) + else: + cur = conn.execute( + """ + UPDATE tasks + SET status = 'blocked', + claim_lock = NULL, + claim_expires = NULL, + worker_pid = NULL, + block_kind = ?, + block_recurrences = ? + WHERE id = ? + AND status IN ('running', 'ready') + AND current_run_id = ? + """, + (kind, recurrences, task_id, int(expected_run_id)), + ) + if cur.rowcount != 1: + return False + run_id = _end_run( + conn, task_id, + outcome="blocked", status="blocked", + summary=reason, + ) + # Synthesize a run when blocking a never-claimed task so the + # reason is preserved in attempt history. + if run_id is None and reason: + run_id = _synthesize_ended_run( + conn, task_id, + outcome="blocked", + summary=reason, + ) + _append_event( + conn, task_id, "blocked", + {"reason": reason, "kind": kind, "recurrences": recurrences}, + run_id=run_id, + ) _blocked_task = get_task(conn, task_id) _fire_kanban_lifecycle_hook( "kanban_task_blocked", @@ -4556,6 +4786,16 @@ def unblock_task(conn: sqlite3.Connection, task_id: str) -> bool: (task_id,), ).fetchone() new_status = "todo" if undone_parents else "ready" + # NOTE: deliberately does NOT touch ``block_recurrences`` or + # ``block_kind``. Resetting the recurrence counter on unblock is exactly + # the amnesia that let a cron unblock → worker re-block loop run + # unbounded (Dale's report). The counter survives the unblock so that a + # subsequent same-cause ``block_task`` can detect the loop and route to + # triage at ``BLOCK_RECURRENCE_LIMIT``. It is reset to 0 only on a + # successful completion (see ``complete_task``). ``consecutive_failures`` + # (the *dispatcher* spawn/crash/timeout counter — a different signal) is + # still reset here, which is correct: a deliberate unblock is a fresh + # start for the dispatcher's retry budget. cur = conn.execute( "UPDATE tasks SET status = ?, current_run_id = NULL, " "consecutive_failures = 0, last_failure_error = NULL " diff --git a/tests/hermes_cli/test_kanban_block_kinds.py b/tests/hermes_cli/test_kanban_block_kinds.py new file mode 100644 index 00000000000..f18dedfae64 --- /dev/null +++ b/tests/hermes_cli/test_kanban_block_kinds.py @@ -0,0 +1,202 @@ +"""Tests for typed block reasons + the unblock-loop breaker. + +Covers the built-in fix for the kanban "blocked loop" — a worker blocks a +task, a cron unblocks it, the worker re-blocks for the same reason, repeat +forever. The fix gives ``block_task`` a typed ``kind`` and a persistent +``block_recurrences`` counter: + +* ``dependency`` blocks route to ``todo`` (parent-gated, auto-resumed) and + never enter the human ``blocked`` bucket a cron would keep unblocking. +* ``needs_input`` / ``capability`` / un-typed blocks land in ``blocked``; + each same-cause re-block after an unblock increments ``block_recurrences``, + and at ``BLOCK_RECURRENCE_LIMIT`` the task routes to ``triage`` for a human. +* ``unblock_task`` deliberately does NOT reset ``block_recurrences`` (the + amnesia that let the loop run unbounded). +* A successful ``complete_task`` resets the loop memory. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from hermes_cli import kanban_db as kb + + +@pytest.fixture +def kanban_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + home = tmp_path / ".hermes" + home.mkdir() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + kb.init_db() + return home + + +def _running_task(conn, title="t"): + """Create a task and drive it to ``running`` so block_task can act.""" + tid = kb.create_task(conn, title=title, assignee="worker") + with kb.write_txn(conn): + conn.execute("UPDATE tasks SET status='ready' WHERE id=?", (tid,)) + claimed = kb.claim_task(conn, tid, claimer="worker") + assert claimed is not None + return tid + + +def _make_running_again(conn, tid): + with kb.write_txn(conn): + conn.execute("UPDATE tasks SET status='ready' WHERE id=?", (tid,)) + assert kb.claim_task(conn, tid, claimer="worker") is not None + + +# --------------------------------------------------------------------------- +# Loop breaker +# --------------------------------------------------------------------------- + + +def test_first_typed_block_lands_in_blocked(kanban_home: Path) -> None: + with kb.connect_closing() as conn: + tid = _running_task(conn) + assert kb.block_task(conn, tid, reason="which key?", kind="needs_input") + t = kb.get_task(conn, tid) + assert t.status == "blocked" + assert t.block_kind == "needs_input" + assert t.block_recurrences == 1 + + +def test_unblock_does_not_reset_recurrence_counter(kanban_home: Path) -> None: + """The crux of the fix: unblock must preserve the loop counter.""" + with kb.connect_closing() as conn: + tid = _running_task(conn) + kb.block_task(conn, tid, reason="x", kind="needs_input") + assert kb.get_task(conn, tid).block_recurrences == 1 + assert kb.unblock_task(conn, tid) + t = kb.get_task(conn, tid) + assert t.status == "ready" + assert t.block_recurrences == 1 # NOT reset to 0 + assert t.block_kind == "needs_input" # kind preserved for comparison + + +def test_same_cause_reblock_routes_to_triage(kanban_home: Path) -> None: + """Dale's loop: block → unblock → re-block same kind → triage.""" + with kb.connect_closing() as conn: + tid = _running_task(conn) + kb.block_task(conn, tid, reason="need creds", kind="needs_input") + kb.unblock_task(conn, tid) + _make_running_again(conn, tid) + kb.block_task(conn, tid, reason="still need creds", kind="needs_input") + t = kb.get_task(conn, tid) + assert t.status == "triage" + assert t.block_recurrences == 2 + + +def test_untyped_block_loop_also_protected(kanban_home: Path) -> None: + """Legacy un-typed blocks (kind=None) still trip the breaker.""" + with kb.connect_closing() as conn: + tid = _running_task(conn) + kb.block_task(conn, tid, reason="a") + kb.unblock_task(conn, tid) + _make_running_again(conn, tid) + kb.block_task(conn, tid, reason="a again") + assert kb.get_task(conn, tid).status == "triage" + + +def test_different_kinds_do_not_compound(kanban_home: Path) -> None: + """A re-block for a DIFFERENT reason resets the counter to 1.""" + with kb.connect_closing() as conn: + tid = _running_task(conn) + kb.block_task(conn, tid, reason="a", kind="needs_input") + kb.unblock_task(conn, tid) + _make_running_again(conn, tid) + kb.block_task(conn, tid, reason="b", kind="capability") + t = kb.get_task(conn, tid) + assert t.status == "blocked" + assert t.block_recurrences == 1 + + +def test_block_loop_detected_event_emitted(kanban_home: Path) -> None: + with kb.connect_closing() as conn: + tid = _running_task(conn) + kb.block_task(conn, tid, reason="x", kind="capability") + kb.unblock_task(conn, tid) + _make_running_again(conn, tid) + kb.block_task(conn, tid, reason="x", kind="capability") + events = [e for e in kb.list_events(conn, tid) + if e.kind == "block_loop_detected"] + assert events, "expected a block_loop_detected event" + payload = events[-1].payload or {} + assert payload.get("recurrences") == 2 + assert payload.get("kind") == "capability" + + +# --------------------------------------------------------------------------- +# Dependency routing +# --------------------------------------------------------------------------- + + +def test_dependency_block_routes_to_todo(kanban_home: Path) -> None: + """Dependency waits never enter the human 'blocked' bucket.""" + with kb.connect_closing() as conn: + tid = _running_task(conn) + assert kb.block_task(conn, tid, reason="need X first", kind="dependency") + t = kb.get_task(conn, tid) + assert t.status == "todo" + assert t.block_kind == "dependency" + + +def test_dependency_then_parent_done_promotes(kanban_home: Path) -> None: + """A dependency-parked child becomes ready once its parent completes.""" + with kb.connect_closing() as conn: + parent = kb.create_task(conn, title="parent", assignee="worker") + child = _running_task(conn, title="child") + kb.link_tasks(conn, parent_id=parent, child_id=child) + kb.block_task(conn, child, reason="wait", kind="dependency") + assert kb.get_task(conn, child).status == "todo" + # Finish the parent, then let recompute_ready run. + with kb.write_txn(conn): + conn.execute("UPDATE tasks SET status='ready' WHERE id=?", (parent,)) + kb.claim_task(conn, parent, claimer="worker") + kb.complete_task(conn, parent, result="done") + kb.recompute_ready(conn) + assert kb.get_task(conn, child).status == "ready" + + +# --------------------------------------------------------------------------- +# Completion resets loop memory +# --------------------------------------------------------------------------- + + +def test_completion_clears_block_memory(kanban_home: Path) -> None: + with kb.connect_closing() as conn: + tid = _running_task(conn) + kb.block_task(conn, tid, reason="x", kind="capability") + kb.unblock_task(conn, tid) + assert kb.get_task(conn, tid).block_recurrences == 1 + kb.complete_task(conn, tid, result="done") + t = kb.get_task(conn, tid) + assert t.status == "done" + assert t.block_recurrences == 0 + assert t.block_kind is None + + +# --------------------------------------------------------------------------- +# Validation + back-compat +# --------------------------------------------------------------------------- + + +def test_invalid_kind_rejected(kanban_home: Path) -> None: + with kb.connect_closing() as conn: + tid = _running_task(conn) + with pytest.raises(ValueError): + kb.block_task(conn, tid, reason="x", kind="bogus") + + +def test_block_without_kind_is_backward_compatible(kanban_home: Path) -> None: + """Existing callers that pass no kind keep the old single-block behaviour.""" + with kb.connect_closing() as conn: + tid = _running_task(conn) + assert kb.block_task(conn, tid, reason="legacy") + t = kb.get_task(conn, tid) + assert t.status == "blocked" + assert t.block_kind is None diff --git a/tests/hermes_cli/test_kanban_notify.py b/tests/hermes_cli/test_kanban_notify.py index f8109416cb5..07b1fa201ba 100644 --- a/tests/hermes_cli/test_kanban_notify.py +++ b/tests/hermes_cli/test_kanban_notify.py @@ -186,8 +186,8 @@ async def test_notifier_second_blocked_delivers(kanban_home): tid = kb.create_task(conn, title="test task", assignee="worker1") kb.add_notify_sub(conn, task_id=tid, platform="telegram", chat_id="chat1") - # Cycle 1: blocked - kb.block_task(conn, tid, reason="first block") + # Cycle 1: blocked for one reason + kb.block_task(conn, tid, reason="first block", kind="needs_input") finally: conn.close() @@ -197,14 +197,18 @@ async def test_notifier_second_blocked_delivers(kanban_home): timeout=10.0, ) - # Cycle 2: unblock → block run again + # Cycle 2: unblock → block again for a DIFFERENT reason. A distinct + # block cause must still notify. (A *same*-cause re-block instead trips + # the unblock-loop breaker and routes to triage — covered by + # test_kanban_block_kinds.py; here we exercise two genuinely different + # blocks, which is the case the user wants notified twice.) runner._running = True tick_count = 0 conn = kb.connect() try: kb.unblock_task(conn, tid) - kb.block_task(conn, tid, reason="second block") + kb.block_task(conn, tid, reason="second block", kind="capability") finally: conn.close() diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 1e4e70f7a4f..c78317f63f8 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -623,13 +623,20 @@ def _handle_block(args: dict, **kw) -> str: if not reason or not str(reason).strip(): return tool_error("reason is required — explain what input you need") reason = redact_sensitive_text(str(reason), force=True) + kind = args.get("kind") board = args.get("board") try: kb, conn = _connect(board=board) + if kind is not None and kind not in kb.VALID_BLOCK_KINDS: + conn.close() + return tool_error( + f"kind must be one of {sorted(kb.VALID_BLOCK_KINDS)} (or omit it)" + ) try: ok = kb.block_task( conn, tid, reason=reason, + kind=kind, expected_run_id=_worker_run_id(tid), ) if not ok: @@ -638,7 +645,15 @@ def _handle_block(args: dict, **kw) -> str: f"running/ready)" ) run = kb.latest_run(conn, tid) - return _ok(task_id=tid, run_id=run.id if run else None) + # Tell the worker where the task actually landed so it doesn't + # assume it's sitting in 'blocked' when routing sent it elsewhere. + landed = kb.get_task(conn, tid) + return _ok( + task_id=tid, + run_id=run.id if run else None, + status=landed.status if landed else "blocked", + block_kind=kind, + ) finally: conn.close() except ValueError as e: @@ -1192,11 +1207,16 @@ KANBAN_COMPLETE_SCHEMA = { KANBAN_BLOCK_SCHEMA = { "name": "kanban_block", "description": ( - "Transition the task to blocked because you need human input " - "to proceed. ``reason`` will be shown to the human on the " - "board and included in context when someone unblocks you. " - "Use for genuine blockers only — don't block on things you can " - "resolve yourself." + "Stop work on this task and route it according to WHY you're stuck. " + "Set ``kind`` to say which: 'dependency' (waiting on another task — " + "goes to todo and auto-resumes when that task finishes, no human " + "needed), 'needs_input' (you need a human decision/answer), " + "'capability' (a hard wall: no access, missing credentials, an action " + "no agent can do), or 'transient' (a flaky failure that may clear). " + "``reason`` is shown to the human on the board. If a task keeps " + "getting unblocked and re-blocked for the same reason, it is " + "auto-escalated to triage. Use for genuine blockers only — don't " + "block on things you can resolve yourself." ), "parameters": { "type": "object", @@ -1208,9 +1228,18 @@ KANBAN_BLOCK_SCHEMA = { "reason": { "type": "string", "description": ( - "What you need answered, in one or two sentences. " - "Don't paste the whole conversation; the human has " - "the board and can ask follow-ups via comments." + "What you need answered or what stopped you, in one or " + "two sentences. Don't paste the whole conversation; the " + "human has the board and can ask follow-ups via comments." + ), + }, + "kind": { + "type": "string", + "enum": ["dependency", "needs_input", "capability", "transient"], + "description": ( + "Why you're blocked. 'dependency' waits in todo and " + "resumes automatically; the others surface to a human. " + "Omit only if none apply." ), }, "board": _board_schema_prop(), diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index c2fe8a0a88b..bd8bd27e3bf 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -268,7 +268,7 @@ hermes kanban block t_abc "need input" --ids t_def t_hij | `kanban_show` | Read the current task (title, body, prior attempts, parent handoffs, comments, full pre-formatted `worker_context`). Defaults to the env's task id. | — | | `kanban_list` | List task summaries with filters for `assignee`, `status`, `tenant`, archived visibility, and limit. Intended for orchestrators discovering board work. | — | | `kanban_complete` | Finish with `summary` + `metadata` structured handoff. | at least one of `summary` / `result` | -| `kanban_block` | Escalate for human input with a `reason`. | `reason` | +| `kanban_block` | Stop work and route by why: `kind=dependency` (waits in `todo`, auto-resumes), `needs_input`/`capability`/`transient` (surface to a human). Repeated same-kind re-blocks auto-escalate to `triage`. | `reason` | | `kanban_heartbeat` | Signal liveness during long operations. Pure side-effect. | — | | `kanban_comment` | Append a durable note to the task thread. | `task_id`, `body` | | `kanban_create` | (Orchestrators) fan out into child tasks with an `assignee`, optional `parents`, `skills`, etc. | `title`, `assignee` | @@ -899,8 +899,10 @@ Every transition appends a row to `task_events`. Each row carries an optional `r | `promoted` | — | `todo → ready` because all parents hit `done`. `run_id` is `NULL`. | | `claimed` | `{lock, expires, run_id}` | Dispatcher atomically claimed a `ready` task for spawn. | | `completed` | `{result_len, summary?}` | Worker wrote `--result` / `--summary` and task hit `done`. `summary` is the first-line handoff (400-char cap); full version lives on the run row. If `complete_task` is called on a never-claimed task with handoff fields, a zero-duration run is synthesized so `run_id` still points at something. | -| `blocked` | `{reason}` | Worker or human flipped the task to `blocked`. Synthesizes a zero-duration run when called on a never-claimed task with `--reason`. | -| `unblocked` | — | `blocked → ready`, either manually or via `/unblock`. `run_id` is `NULL`. | +| `blocked` | `{reason, kind, recurrences}` | Worker or human flipped the task to `blocked`. `kind` is the typed block reason (`needs_input`, `capability`, `transient`, or `null` for a generic block); `recurrences` is the unblock-loop counter. Synthesizes a zero-duration run when called on a never-claimed task with `--reason`. | +| `dependency_wait` | `{reason, kind}` | Worker blocked with `kind=dependency` — the task is only waiting on another task, so it routes to `todo` (parent-gated, auto-promoted) instead of `blocked`. No human needed. | +| `block_loop_detected` | `{reason, kind, recurrences, limit}` | A task was unblocked and re-blocked for the same reason `BLOCK_RECURRENCE_LIMIT` times (default 2). Instead of landing in `blocked` again — where a cron would keep unblocking it — it routes to `triage` for a human decision, breaking the unblock↔re-block loop. | +| `unblocked` | — | `blocked → ready` (or `todo` if parents are still open), either manually or via `/unblock`. Resets the dispatcher's `consecutive_failures` but deliberately preserves `block_recurrences` so the loop breaker keeps its memory. `run_id` is `NULL`. | | `archived` | — | Hidden from the default board. If the task was still running, carries the `run_id` of the run that was reclaimed as a side effect. | **Edits** (human-driven changes that aren't transitions):