Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion hermes_cli/kanban.py
Original file line number Diff line number Diff line change
Expand Up @@ -1830,13 +1830,23 @@ def _cmd_reclaim(args: argparse.Namespace) -> int:
conn, args.task_id,
reason=getattr(args, "reason", None),
)
task = kb.get_task(conn, args.task_id) if ok else None
if not ok:
print(
f"cannot reclaim {args.task_id} (not running or unknown id)",
file=sys.stderr,
)
return 1
print(f"Reclaimed {args.task_id}")
status = task.status if task else None
if status in ("blocked", "triage", "scheduled"):
# A reclaim releases the claim; it does not promote. Say so, or the
# operator assumes the card is queued and waits for a worker forever.
print(
f"Reclaimed {args.task_id} (status held at {status!r} — "
f"run 'hermes kanban unblock {args.task_id}' to re-queue it)"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triage is included in this branch, but unblock_task only accepts blocked and scheduled (hermes_cli/kanban_db.py:5808). This prints a recovery command that fails for triage; direct triage to its specify/decompose transition instead.

)
else:
print(f"Reclaimed {args.task_id}" + (f" ({status})" if status else ""))
return 0


Expand Down
46 changes: 43 additions & 3 deletions hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -3183,6 +3183,22 @@ def create_task(
},
)
_inherit_notify_subs(conn, task_id, parents, created_at=now)
if task_status == "blocked":
# Parking a card in blocked at creation time is an explicit
# operator decision (the documented purpose of
# ``--initial-status blocked`` is the human-ops gate). Emit
# the same "blocked" event ``block_task`` emits so
# ``_has_sticky_block`` treats it as sticky — otherwise
# ``recompute_ready`` auto-promotes a parentless blocked
# card to ready on the next dispatcher tick and a worker
# gets spawned, defeating the flag entirely.
# ``unblock_task`` remains the legitimate exit.
_append_event(
conn,
task_id,
"blocked",
{"reason": "created with initial_status=blocked"},
)
return task_id
except sqlite3.IntegrityError:
if attempt == 1:
Expand Down Expand Up @@ -4459,6 +4475,12 @@ def reclaim_task(
when an operator wants to abort a running worker without waiting
for the TTL to expire (e.g. after seeing a hallucination warning).

A reclaim releases a *claim*; it never promotes. Rows parked in
``blocked`` / ``triage`` / ``scheduled`` keep that status and only shed
their stale claim residue — promoting them here would feed them straight
back to the dispatcher and defeat the block. Use :func:`unblock_task` or
:func:`promote_task` to actually re-queue one.

Returns True if a reclaim happened, False if the task isn't in a
reclaimable state (not running, or doesn't exist).
"""
Expand All @@ -4475,13 +4497,27 @@ def reclaim_task(
termination = _terminate_reclaimed_worker(
row["worker_pid"], prev_lock, signal_fn=signal_fn,
)
# A reclaim RELEASES A CLAIM; it is not a promotion. Laundering a
# ``blocked`` / ``triage`` / ``scheduled`` row into ``ready`` here hands it
# straight to the dispatcher, which claims and spawns a worker on the next
# tick — silently defeating ``--initial-status blocked``, a worker's
# ``review-required`` handoff, and the spawn-failure circuit breaker alike.
# Those rows keep their status and only lose the stale claim residue.
# Reclaiming a blocked card is also invisible to ``_has_sticky_block`` (it
# emits ``reclaimed``, never ``unblocked``), so the pre-fix path produced a
# genuinely incoherent row: ``status='ready'`` while still sticky-blocked,
# which ``recompute_ready`` and the dispatcher then disagree about.
# ``unblock_task`` / ``promote_task`` remain the only ways out of blocked.
held_status = row["status"]
preserve_status = held_status in ("blocked", "triage", "scheduled")
with write_txn(conn):
cur = conn.execute(
"UPDATE tasks SET status = 'ready', claim_lock = NULL, "
"UPDATE tasks SET status = ?, claim_lock = NULL, "
"claim_expires = NULL, worker_pid = NULL "
"WHERE id = ? AND status IN ('running', 'ready', 'blocked') "
"WHERE id = ? AND status IN ('running', 'ready', 'blocked', "
"'triage', 'scheduled') "
"AND claim_lock IS ?",
(task_id, prev_lock),
(held_status if preserve_status else "ready", task_id, prev_lock),
)
if cur.rowcount != 1:
return False
Expand All @@ -4499,6 +4535,10 @@ def reclaim_task(
"reason": reason,
"prev_lock": prev_lock,
}
if preserve_status:
# Make the non-promotion explicit in board history so an operator
# who reclaims a blocked card can see why it did NOT go ready.
payload["status_preserved"] = held_status
payload.update(termination)
_append_event(
conn, task_id, "reclaimed",
Expand Down
85 changes: 85 additions & 0 deletions tests/hermes_cli/test_kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -4999,3 +4999,88 @@ def test_bare_connect_does_not_close_on_context_exit(tmp_path):
# Still usable after with-block exit (the leak).
conn.execute("SELECT 1").fetchone()
conn.close() # explicit close to avoid leaking THIS test


def test_initial_status_blocked_is_sticky_across_recompute(kanban_home):
"""A task created with ``initial_status='blocked'`` must survive
``recompute_ready`` — the flag's documented purpose is parking a card for
human ops, so the dispatcher must not auto-promote it.

Regression: ``create_task`` set ``status='blocked'`` but emitted no
``blocked`` event, so ``_has_sticky_block`` returned False and
``recompute_ready`` promoted the (parentless) card to ready on the next
dispatcher tick, spawning a worker for a card that explicitly asked not to
be dispatched.
"""
with kb.connect() as conn:
t = kb.create_task(conn, title="parked", initial_status="blocked")
assert kb.get_task(conn, t).status == "blocked"
promoted = kb.recompute_ready(conn)
assert promoted == 0
assert kb.get_task(conn, t).status == "blocked"
# The card must be sticky by the same predicate an operator block uses.
assert kb._has_sticky_block(conn, t) is True
row = conn.execute(
"SELECT kind FROM task_events WHERE task_id = ? "
"AND kind IN ('blocked', 'unblocked') ORDER BY id DESC LIMIT 1",
(t,),
).fetchone()
assert row is not None and row["kind"] == "blocked"


def test_initial_status_blocked_survives_a_real_dispatcher_pass(
kanban_home, all_assignees_spawnable
):
"""E2E: the dispatcher must not spawn a worker on a created-blocked card.

``recompute_ready`` runs inside ``dispatch_once``, so the promotion above
is not academic: it hands the card to the claim + spawn path on the very
next tick.
"""
spawned = []

def fake_spawn(task, workspace):
spawned.append(task.id)

with kb.connect() as conn:
t = kb.create_task(
conn, title="do not dispatch me", assignee="alice",
initial_status="blocked",
)
kb.dispatch_once(conn, spawn_fn=fake_spawn)
assert spawned == [], (
"a card created with initial_status=blocked must never be spawned"
)
assert kb.get_task(conn, t).status == "blocked"


def test_initial_status_blocked_unblock_releases(kanban_home):
"""``unblock_task`` is the legitimate exit from a created-blocked card —
after an explicit unblock, normal flow may promote it."""
with kb.connect() as conn:
t = kb.create_task(conn, title="parked", initial_status="blocked")
assert kb.unblock_task(conn, t)
assert kb.get_task(conn, t).status in ("ready", "todo")
# And it stays promotable: recompute must not re-block it.
kb.recompute_ready(conn)
assert kb.get_task(conn, t).status in ("ready", "todo")
assert kb._has_sticky_block(conn, t) is False


def test_initial_status_blocked_with_done_parents_stays_blocked(kanban_home):
"""Sticky beats the parents-are-done promotion rule.

``recompute_ready`` promotes a blocked task whose parents are all done.
An explicitly parked card must not be promoted by that rule either --
otherwise completing an unrelated parent silently releases the gate.
"""
with kb.connect() as conn:
parent = kb.create_task(conn, title="parent")
kb.claim_task(conn, parent)
kb.complete_task(conn, parent)
child = kb.create_task(
conn, title="parked child", parents=[parent],
initial_status="blocked",
)
kb.recompute_ready(conn)
assert kb.get_task(conn, child).status == "blocked"
Loading
Loading