From 9ae238d2fd5c8c42480005caa5d8331d36a83a5f Mon Sep 17 00:00:00 2001 From: daixin1204 Date: Sat, 9 May 2026 15:21:32 +0800 Subject: [PATCH] fix(kanban): route unblock through dependency gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit unblock_task no longer jumps directly to ready. It sets the task to todo and calls recompute_ready, so the same parent-dependency gate that guards todo→ready (all parents done/archived) is re-evaluated. This closes the only bypass that could land a task in ready without passing the parent-completion check, and naturally handles the case where parents were archived while the task was blocked. --- hermes_cli/kanban_db.py | 39 ++++-- .../test_kanban_core_functionality.py | 114 ++++++++++++++++++ 2 files changed, 141 insertions(+), 12 deletions(-) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 06b4f96cebc4..b1ca357c8fb8 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -1763,7 +1763,7 @@ def recompute_ready(conn: sqlite3.Connection) -> int: "WHERE l.child_id = ?", (task_id,), ).fetchall() - if all(p["status"] == "done" for p in parents): + if all(p["status"] in ("done", "archived") for p in parents): conn.execute( "UPDATE tasks SET status = 'ready' WHERE id = ? AND status = 'todo'", (task_id,), @@ -2465,14 +2465,25 @@ def block_task( def unblock_task(conn: sqlite3.Connection, task_id: str) -> bool: - """Transition ``blocked -> ready``. - - Defensively closes any stale ``current_run_id`` pointer before flipping - status. In the common path (``block_task`` closed the run already) this - is a no-op. If a future or external write left the pointer dangling, - the leaked run is closed as ``reclaimed`` inside the same txn so the - runs invariant (``current_run_id IS NULL`` ⇔ run row in terminal - state) holds for the rest of this function's lifetime. + """Transition ``blocked -> todo``, then re-run dependency gating. + + Instead of jumping directly to ``ready``, this sets the task to + ``todo`` and calls :func:`recompute_ready` so the parent-dependency + gate is re-evaluated. If all parents are terminal (``done`` or + ``archived``) the task is promoted to ``ready`` in the same call; + otherwise it stays in ``todo`` until its remaining parents complete. + + This avoids the ``unblock_task`` bypass of dependency gating and + naturally handles edge cases where parents were archived after the + task was last in ``ready`` / ``running``. + + Defensively closes any stale ``current_run_id`` pointer before + flipping status. In the common path (``block_task`` closed the run + already) this is a no-op. If a future or external write left the + pointer dangling, the leaked run is closed as ``reclaimed`` inside + the same txn so the runs invariant (``current_run_id IS NULL`` ⇔ run + row in terminal state) holds for the rest of this function's + lifetime. """ now = int(time.time()) with write_txn(conn): @@ -2493,14 +2504,18 @@ def unblock_task(conn: sqlite3.Connection, task_id: str) -> bool: (now, int(stale["current_run_id"])), ) cur = conn.execute( - "UPDATE tasks SET status = 'ready', current_run_id = NULL " + "UPDATE tasks SET status = 'todo', current_run_id = NULL " "WHERE id = ? AND status = 'blocked'", (task_id,), ) if cur.rowcount != 1: return False - _append_event(conn, task_id, "unblocked", None) - return True + _append_event(conn, task_id, "unblocked", {"to_status": "todo"}) + # Re-run dependency gating outside the txn (separate txn so + # recompute_ready sees the committed todo status). Same pattern as + # complete_task at the end of its txn. + recompute_ready(conn) + return True def specify_triage_task( diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index 45d457630e16..a5e0fa9f8957 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -2205,6 +2205,120 @@ def test_unblock_normal_path_no_spurious_run(kanban_home): conn.close() +# ── unblock_task → todo + recompute_ready dependency gating ────────────── + +def test_unblock_rechecks_parent_dependencies(kanban_home): + """unblock_task must go through recompute_ready: blocked→todo, + then promoted to ready only when all parents are terminal.""" + conn = kb.connect() + try: + parent = kb.create_task(conn, title="dep", assignee="worker") + child = kb.create_task( + conn, title="depends on dep", assignee="worker", parents=[parent], + ) + # child starts as todo (parent not done), stays todo + assert kb.get_task(conn, child).status == "todo" + + # Complete parent → recompute_ready promotes child to ready + kb.claim_task(conn, parent) + kb.complete_task(conn, parent, summary="ok") + assert kb.get_task(conn, child).status == "ready" + + # Block child, then unblock — parents are all done, so + # recompute_ready should promote todo→ready immediately. + kb.claim_task(conn, child) + kb.block_task(conn, child, reason="need clarification") + assert kb.get_task(conn, child).status == "blocked" + + assert kb.unblock_task(conn, child) is True + assert kb.get_task(conn, child).status == "ready" + finally: + conn.close() + + +def test_unblock_stays_todo_when_parents_not_done(kanban_home): + """unblock_task must leave the task in todo when a parent is still + running (not done/archived).""" + conn = kb.connect() + try: + parent = kb.create_task(conn, title="slow parent", assignee="worker") + child = kb.create_task( + conn, title="depends on slow", assignee="worker", parents=[parent], + ) + # child starts as todo because parent is not done + assert kb.get_task(conn, child).status == "todo" + + # Force child to ready (simulate what happens after a human + # manually moves it), then block it. + conn.execute("UPDATE tasks SET status = 'ready' WHERE id = ?", (child,)) + conn.commit() + kb.claim_task(conn, child) + kb.block_task(conn, child, reason="need input") + assert kb.get_task(conn, child).status == "blocked" + + # Unblock — parent is still in todo, so recompute_ready + # should leave child in todo. + assert kb.unblock_task(conn, child) is True + assert kb.get_task(conn, child).status == "todo", ( + "child must stay todo when parent is not done" + ) + finally: + conn.close() + + +def test_unblock_promotes_when_parent_archived(kanban_home): + """archived parents satisfy dependencies (superset of done).""" + conn = kb.connect() + try: + parent = kb.create_task(conn, title="done-then-archived", assignee="worker") + child = kb.create_task( + conn, title="depends on archived", assignee="worker", parents=[parent], + ) + assert kb.get_task(conn, child).status == "todo" + + # Complete and archive parent. + kb.claim_task(conn, parent) + kb.complete_task(conn, parent, summary="done") + assert kb.get_task(conn, parent).status == "done" + kb.archive_task(conn, parent) + assert kb.get_task(conn, parent).status == "archived" + + # recompute_ready should now see parent as terminal and promote child. + kb.recompute_ready(conn) + assert kb.get_task(conn, child).status == "ready", ( + "archived parent must satisfy dependency" + ) + + # Block and unblock — should go todo→ready in one call. + kb.claim_task(conn, child) + kb.block_task(conn, child, reason="need input") + assert kb.get_task(conn, child).status == "blocked" + + assert kb.unblock_task(conn, child) is True + assert kb.get_task(conn, child).status == "ready", ( + "unblock must promote to ready when archived parent satisfies dep" + ) + finally: + conn.close() + + +def test_unblock_without_parents_still_goes_ready(kanban_home): + """Tasks with no parents go blocked→todo→ready (all([]) == True).""" + conn = kb.connect() + try: + tid = kb.create_task(conn, title="no deps", assignee="worker") + kb.claim_task(conn, tid) + kb.block_task(conn, tid, reason="need input") + assert kb.get_task(conn, tid).status == "blocked" + + assert kb.unblock_task(conn, tid) is True + assert kb.get_task(conn, tid).status == "ready", ( + "task with no parents must go ready after unblock" + ) + finally: + conn.close() + + def test_migration_backfill_idempotent_under_re_run(tmp_path, monkeypatch): """init_db must be safe to re-run repeatedly. Each call should leave at most one run row per in-flight task, even if called while a