Skip to content
Merged
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
63 changes: 57 additions & 6 deletions hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -5407,6 +5407,25 @@ def _has_sticky_block(conn: sqlite3.Connection, task_id: str) -> bool:
return bool(row) and row["kind"] == "blocked"


def _most_recent_event_kind(conn: sqlite3.Connection, task_id: str) -> Optional[str]:
"""Return the ``kind`` of the single most recent ``task_events`` row for
``task_id``, or ``None`` if the task has no events.

Used to detect that a ``triage`` card landed there via the
``block_loop_detected`` routing (see ``block_task`` /
``BLOCK_RECURRENCE_LIMIT``) rather than via normal raw-intake — that
routing exists specifically to force a human decision, so callers such
as the auto-decomposer must not treat it as an ordinary triage card to
sweep and re-promote (t_e2b1f62a).
"""
row = conn.execute(
"SELECT kind FROM task_events WHERE task_id = ? "
"ORDER BY id DESC LIMIT 1",
(task_id,),
).fetchone()
return row["kind"] if row else None


def recompute_ready(
conn: sqlite3.Connection, failure_limit: int = None,
) -> int:
Expand Down Expand Up @@ -7158,12 +7177,44 @@ def block_task(
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
# re-block for the SAME reason after a prior *legitimate* unblock.
# block_task only fires from running/ready (i.e. AFTER something
# returned the task to the work pool), so a stored block_kind that
# matches the incoming kind COULD mean: blocked -> unblocked ->
# about-to-re-block for the same cause (Dale's genuine cron/human
# unblock <-> worker re-block ping-pong — this is what the counter is
# meant to catch). An un-typed (None) block compares as "same" to a
# prior un-typed block.
#
# But "blocked -> ready/running" can ALSO happen without anyone
# actually unblocking the task for a reason: ``recompute_ready``'s
# dispatcher-side ``parents_terminal`` sweep (or any other
# non-``unblock_task`` transition) can flip a non-sticky
# circuit-breaker ``blocked`` state back to ``ready`` on its own.
# That is a completely different signal from "a human/cron decided
# this was resolved" — treating it the same way conflates "re-promoted
# by dispatcher machinery unrelated to the block reason" with
# "re-blocked for the same cause after a legitimate unblock", and
# inflates ``block_recurrences`` on every dispatcher hiccup until the
# loop breaker trips and routes an otherwise-correctly-blocked,
# already-reviewed card to ``triage`` for no reason (t_e2b1f62a).
#
# Disambiguate by checking what the most recent "exit from blocked"
# event actually was: only an explicit ``unblock_task`` call emits
# ``"unblocked"``; a dispatcher auto-promotion emits ``"promoted"``
# (trigger ``parents_terminal`` or otherwise). Only count this as a
# same-cause re-block when the intervening exit was a genuine
# ``unblocked`` event.
last_exit_row = conn.execute(
"SELECT kind FROM task_events WHERE task_id = ? "
"AND kind IN ('unblocked', 'promoted') "
"ORDER BY id DESC LIMIT 1",
(task_id,),
).fetchone()
exited_via_legitimate_unblock = bool(
last_exit_row and last_exit_row["kind"] == "unblocked"
)
same_cause = prev_kind == kind and exited_via_legitimate_unblock
recurrences = prev_recurrences + 1 if same_cause else 1

if recurrences >= BLOCK_RECURRENCE_LIMIT:
Expand Down
26 changes: 22 additions & 4 deletions hermes_cli/kanban_decompose.py
Original file line number Diff line number Diff line change
Expand Up @@ -570,11 +570,29 @@ def decompose_task(
"""
with kb.connect_closing() as conn:
task = kb.get_task(conn, task_id)
if task is None:
return DecomposeOutcome(task_id, False, "unknown task id")
if task.status != "triage":
if task is None:
return DecomposeOutcome(task_id, False, "unknown task id")
if task.status != "triage":
return DecomposeOutcome(
task_id, False, f"task is not in triage (status={task.status!r})"
)
# A triage card whose most recent event is ``block_loop_detected`` was
# routed here specifically to force a HUMAN decision (see
# ``block_task``/``BLOCK_RECURRENCE_LIMIT`` in kanban_db.py) — the
# unblock<->reblock loop breaker tripped because a worker kept
# re-blocking it for the same cause. Blindly re-"specifying" and
# promoting such a card back to ``todo``/``ready`` hands it straight
# back into the same loop the breaker exists to interrupt (t_e2b1f62a):
# the card's disposition is already correct and signed, it just needs a
# human triage call, not another decomposer pass. Skip it here; a human
# (or an explicit ``kanban specify``/``kanban promote`` call) is the
# only legitimate way out of this state.
most_recent_kind = kb._most_recent_event_kind(conn, task_id)
if most_recent_kind == "block_loop_detected":
return DecomposeOutcome(
task_id, False, f"task is not in triage (status={task.status!r})"
task_id, False,
"skipped: most recent event is block_loop_detected — "
"awaiting human triage decision, not auto-decompose",
)

cfg = _load_config()
Expand Down
112 changes: 112 additions & 0 deletions tests/hermes_cli/test_kanban_blocked_sticky.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

from __future__ import annotations

import json
import time
from pathlib import Path

Expand Down Expand Up @@ -228,3 +229,114 @@ def test_force_trip_floors_consecutive_failures_at_effective_limit(
"floored failure count still exceeds a lower effective_limit"
)
assert kb.get_task(conn, tid).status == "blocked"


# ---------------------------------------------------------------------------
# block_task's recurrence counter must not treat a dispatcher-side
# auto-promotion (parents_terminal or any other non-unblock_task exit from
# ``blocked``) the same as a genuine human/cron unblock -> worker re-block
# cycle (t_e2b1f62a).
# ---------------------------------------------------------------------------


def test_dispatcher_repromotion_does_not_inflate_block_recurrences(
kanban_home: Path,
) -> None:
"""Reproduces the t_342c4c9f loop: a card is correctly re-blocked for
the SAME cause after the dispatcher (not a human/cron) flipped it back
to ready/running via a ``parents_terminal``-triggered ``promoted``
event. This must NOT count toward ``block_recurrences`` — only an
explicit ``unblock_task`` call re-arms the same-cause counter.
"""
with kb.connect() as conn:
tid = kb.create_task(conn, title="t_342c4c9f style review-required card")
kb.claim_task(conn, tid)

assert kb.block_task(
conn, tid, kind="needs_input",
reason="REJECTED-INTAKE: awaiting producer resubmission",
expected_run_id=kb.get_task(conn, tid).current_run_id,
)
task = kb.get_task(conn, tid)
assert task.status == "blocked"
assert task.block_recurrences == 1

# Simulate the dispatcher-side bug: something (parents_terminal
# sweep, or any non-unblock_task path) flips the card back to
# ready/running WITHOUT going through unblock_task — so no
# "unblocked" event is emitted, only a "promoted" one.
for cycle in range(4):
conn.execute(
"UPDATE tasks SET status = 'running' WHERE id = ?", (tid,),
)
conn.execute(
"INSERT INTO task_events (task_id, kind, payload, created_at) "
"VALUES (?, 'promoted', ?, ?)",
(
tid,
json.dumps({
"from_status": "blocked", "to_status": "ready",
"trigger": "parents_terminal", "satisfied_parent_ids": [],
}),
int(time.time()) + cycle,
),
)
conn.commit()

assert kb.block_task(
conn, tid, kind="needs_input",
reason="REJECTED-INTAKE: awaiting producer resubmission",
expected_run_id=None,
)
task = kb.get_task(conn, tid)
# The card must land back in 'blocked' (never 'triage') and the
# recurrence counter must stay at 1 — each re-block after a
# dispatcher auto-promotion is treated as a fresh same-cause
# block (recurrences reset to 1), not an accumulating loop.
assert task.status == "blocked", (
f"cycle {cycle}: dispatcher-side re-promotion inflated the "
f"loop-breaker into routing to {task.status!r} instead of "
"staying blocked"
)
assert task.block_recurrences == 1, (
f"cycle {cycle}: block_recurrences={task.block_recurrences}, "
"expected 1 — a parents_terminal-triggered re-entry must not "
"count toward the same-cause loop-breaker counter"
)


def test_genuine_unblock_reblock_loop_still_trips_breaker(
kanban_home: Path,
) -> None:
"""Sanity check that the fix above does not defang the original
loop-breaker: a REAL unblock_task -> re-block cycle for the same cause
must still accumulate recurrences and eventually route to triage.
"""
with kb.connect() as conn:
tid = kb.create_task(conn, title="genuine ping-pong reproducer")
kb.claim_task(conn, tid)

assert kb.block_task(
conn, tid, kind="needs_input", reason="waiting on X",
expected_run_id=kb.get_task(conn, tid).current_run_id,
)
assert kb.get_task(conn, tid).block_recurrences == 1

assert kb.unblock_task(conn, tid)
assert kb.get_task(conn, tid).status == "ready"
conn.execute("UPDATE tasks SET status = 'running' WHERE id = ?", (tid,))
conn.commit()

assert kb.block_task(
conn, tid, kind="needs_input", reason="waiting on X",
expected_run_id=None,
)
task = kb.get_task(conn, tid)
# BLOCK_RECURRENCE_LIMIT is 2, so the loop breaker trips on THIS
# (second) same-cause re-block, routing straight to triage rather
# than back to blocked — it does not take a third cycle.
assert task.status == "triage", (
"a genuine repeated unblock -> re-block cycle for the same "
"cause must still trip the loop breaker and route to triage"
)
assert task.block_recurrences == kb.BLOCK_RECURRENCE_LIMIT == 2
37 changes: 37 additions & 0 deletions tests/hermes_cli/test_kanban_decompose.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,40 @@ def test_decompose_returns_false_when_task_not_triage(kanban_home):
assert "not in triage" in outcome.reason


def test_decompose_skips_triage_card_from_block_loop_detected(kanban_home):
"""A triage card whose most recent event is ``block_loop_detected`` was
routed there specifically to force a human decision (see
``block_task``/``BLOCK_RECURRENCE_LIMIT`` in kanban_db.py — t_e2b1f62a).
``decompose_task`` must refuse to auto-specify/promote it, and must not
invoke the auxiliary LLM at all for such a card.
"""
with kb.connect() as conn:
tid = kb.create_task(conn, title="review-required card", triage=True)
kb._append_event(
conn, tid, "block_loop_detected",
{"reason": "REJECTED-INTAKE", "kind": "needs_input",
"recurrences": 2, "limit": kb.BLOCK_RECURRENCE_LIMIT},
)

patches = _patch_list_profiles(["orchestrator"])
for p in patches:
p.start()
try:
with _patch_aux_client("{}") as mock_call_llm, _patch_extra_body():
outcome = decomp.decompose_task(tid, author="me")
finally:
for p in patches:
p.stop()

assert outcome.ok is False
assert "block_loop_detected" in outcome.reason
mock_call_llm.assert_not_called()
with kb.connect() as conn:
task = kb.get_task(conn, tid)
assert task.status == "triage", (
"a block_loop_detected card must stay in triage untouched, "
"not be re-specified or promoted"
)



Loading