Kanban worker iteration-budget exhaustion and retry loop fixes

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #35220 fix(kanban): stop budget-exhaustion retry loop; align recovery guard with configured failure_limit

  merged to main

  files: hermes_cli/kanban_db.py, tests/hermes_cli/test_kanban_blocked_sticky.py, tests/hermes_cli/test_kanban_db.py

  Show the diff

  • hermes_cli/kanban_db.py

    @@ -2457,7 +2457,9 @@ def _has_sticky_block(conn: sqlite3.Connection, task_id: str) -> bool:
         return bool(row) and row["kind"] == "blocked"
     
     
    -def recompute_ready(conn: sqlite3.Connection) -> int:
    +def recompute_ready(
    +    conn: sqlite3.Connection, failure_limit: int = None,
    +) -> int:
         """Promote ``todo`` tasks to ``ready`` when all parents are ``done`` or ``archived``.
     
         Returns the number of tasks promoted.  Safe to call inside or outside
    @@ -2465,17 +2467,34 @@ def recompute_ready(conn: sqlite3.Connection) -> int:
     
         ``blocked`` tasks are also considered for promotion (so a task
         blocked purely by a parent dependency unblocks itself when the
    -    parent completes), *except* when the most recent block event was a
    -    worker-initiated ``kanban_block`` — those stay blocked until an
    -    explicit ``kanban_unblock`` (#28712).  Without that guard, a
    -    ``review-required`` handoff would auto-respawn, the fresh worker
    -    would find nothing to do, exit cleanly, get recorded as a protocol
    -    violation, and the cycle would repeat indefinitely.
    +    parent completes), *except* in two cases:
    +
    +    1. The most recent block event was a worker-initiated
    +       ``kanban_block`` — those stay blocked until an explicit
    +       ``kanban_unblock`` (#28712).
    +
    +    2. The task's ``consecutive_failures`` has reached the effective
    +       failure limit.  This prevents infinite retry loops when a task
    +       repeatedly exhausts its iteration budget: without this guard the
    +       counter would reset on every recovery cycle and the circuit
    +       breaker could never trip (#35072).
    +
    +    The effective failure limit resolves in the same order as the
    +    circuit breaker in ``_record_task_failure`` so the two never
    +    disagree about when a task is permanently blocked:
    +
    +      1. per-task ``max_retries`` if set
    +      2. caller-supplied ``failure_limit`` (the dispatcher passes the
    +         ``kanban.failure_limit`` config value through ``dispatch_once``)
    +      3. ``DEFAULT_FAILURE_LIMIT``
         """
    +    if failure_limit is None:
    +        failure_limit = DEFAULT_FAILURE_LIMIT
         promoted = 0
         with write_txn(conn):
             todo_rows = conn.execute(
    -            "SELECT id, status FROM tasks WHERE status IN ('todo', 'blocked')"
    +            "SELECT id, status, consecutive_failures, max_retries "
    +            "FROM tasks WHERE status IN ('todo', 'blocked')"
             ).fetchall()
             for row in todo_rows:
                 task_id = row["id"]
    @@ -2493,13 +2512,25 @@ def recompute_ready(conn: sqlite3.Connection) -> int:
                     (task_id,),
                 ).fetchall()
                 if all(p["status"] in ("done", "archived") for p in parents):
    -                # Blocked tasks also get their failure counters reset —
    -                # this is effectively an auto-unblock (circuit-breaker
    -                # recovery; worker-initiated blocks are skipped above).
                     if cur_status == "blocked":
    +                    # Don't auto-recover tasks that have hit the
    +                    # circuit-breaker failure limit.  Without this
    +                    # guard, a task that repeatedly exhausts its
    +                    # iteration budget would cycle forever:
    +                    # block → auto-recover → respawn → budget
    +                    # exhausted → block → …  The counter must also
    +                    # be preserved so the breaker can accumulate
    +                    # across recovery cycles.
    +                    failures = int(row["consecutive_failures"] or 0)
    +                    task_limit = row["max_retries"]
    +                    effective_limit = (
    +                        int(task_limit) if task_limit is not None
    +                        else int(failure_limit)
    +                    )
    +                    if failures >= effective_limit:
    +                        continue
                         conn.execute(
    -                        "UPDATE tasks SET status = 'ready', "
    -                        "consecutive_failures = 0, last_failure_error = NULL "
    +                        "UPDATE tasks SET status = 'ready' "
                             "WHERE id = ? AND status = 'blocked'",
                             (task_id,),
                         )
    @@ -5424,7 +5455,7 @@ def dispatch_once(
         if _crash_auto_blocked:
             result.auto_blocked.extend(_crash_auto_blocked)
         result.timed_out = enforce_max_runtime(conn)
    -    result.promoted = recompute_ready(conn)
    +    result.promoted = recompute_ready(conn, failure_limit=failure_limit)
     
         # Count tasks already running so max_spawn enforces concurrency rather
         # than a per-tick spawn budget. See the docstring above for the full

  • tests/hermes_cli/test_kanban_blocked_sticky.py

    @@ -106,20 +106,30 @@ def test_worker_block_on_child_with_done_parents_is_still_sticky(kanban_home: Pa
     
     def test_circuit_breaker_block_still_auto_promotes(kanban_home: Path) -> None:
         """A child that was put into ``blocked`` *without* a worker-issued
    -    ``kanban_block`` (e.g. circuit-breaker after repeated spawn
    -    failures, manual DB triage) must still get auto-promoted when its
    -    parents complete — preserves the pre-#28712 recovery semantics."""
    +    ``kanban_block`` (e.g. a transient crash, manual DB triage) and whose
    +    ``consecutive_failures`` is still *below* the circuit-breaker limit
    +    must get auto-promoted when its parents complete — preserves the
    +    pre-#28712 recovery semantics for genuinely transient failures.
    +
    +    The complementary case — a block whose failure count has *reached*
    +    the limit must stay blocked — is covered by
    +    ``test_kanban_db.py::test_recompute_ready_skips_tasks_at_failure_limit``
    +    (#35072).  Together they pin the contract: ``recompute_ready`` defers
    +    the give-up decision to the same effective limit the breaker uses, so
    +    the two never disagree.
    +    """
         with kb.connect() as conn:
             parent = kb.create_task(conn, title="parent")
             child = kb.create_task(conn, title="child", parents=[parent])
             kb.complete_task(conn, parent, result="ok")
     
    -        # Simulate a circuit-breaker / direct triage that flips status
    -        # without emitting a ``blocked`` event — exactly what
    -        # ``_record_task_failure`` does after a ``gave_up``.
    +        # Simulate a transient circuit-breaker / direct triage that flips
    +        # status without emitting a ``blocked`` event — exactly what
    +        # ``_record_task_failure`` does below the limit.  One failure is
    +        # under the default limit (2), so recovery is still correct.
             conn.execute(
    -            "UPDATE tasks SET status='blocked', consecutive_failures=5, "
    -            "last_failure_error='persistent error' WHERE id=?",
    +            "UPDATE tasks SET status='blocked', consecutive_failures=1, "
    +            "last_failure_error='transient error' WHERE id=?",
                 (child,),
             )
             conn.commit()
    @@ -128,8 +138,9 @@ def test_circuit_breaker_block_still_auto_promotes(kanban_home: Path) -> None:
             assert promoted == 1
             task = kb.get_task(conn, child)
             assert task.status == "ready"
    -        assert task.consecutive_failures == 0
    -        assert task.last_failure_error is None
    +        # Counter is preserved across recovery (not reset) so the breaker
    +        # can still accumulate if the task keeps failing (#35072).
    +        assert task.consecutive_failures == 1
     
     
     def test_gave_up_event_alone_does_not_make_block_sticky(kanban_home: Path) -> None:

  • tests/hermes_cli/test_kanban_db.py

    @@ -307,7 +307,8 @@ def test_recompute_ready_cascades_through_chain(kanban_home):
     
     
     def test_recompute_ready_promotes_blocked_with_done_parents(kanban_home):
    -    """blocked tasks with all parents done should be promoted to ready."""
    +    """blocked tasks with all parents done should be promoted to ready,
    +    unless the circuit-breaker failure limit has been reached."""
         with kb.connect() as conn:
             parent = kb.create_task(conn, title="parent", assignee="a")
             child = kb.create_task(
    @@ -316,16 +317,16 @@ def test_recompute_ready_promotes_blocked_with_done_parents(kanban_home):
             # Complete the parent
             kb.claim_task(conn, parent)
             kb.complete_task(conn, parent, result="ok")
    -        # Manually block the child (simulates a worker that failed
    -        # after the parent finished)
    +        # Manually block the child with zero failures (simulates a
    +        # dependency block, not a circuit-breaker block).
             conn.execute(
    -            "UPDATE tasks SET status='blocked', consecutive_failures=5, "
    -            "last_failure_error='persistent error' WHERE id=?",
    +            "UPDATE tasks SET status='blocked', consecutive_failures=0, "
    +            "last_failure_error=NULL WHERE id=?",
                 (child,),
             )
             conn.commit()
             assert kb.get_task(conn, child).status == "blocked"
    -        # recompute_ready should promote blocked → ready and reset failures
    +        # recompute_ready should promote blocked → ready
             promoted = kb.recompute_ready(conn)
             assert promoted == 1
             task = kb.get_task(conn, child)
    @@ -815,6 +816,149 @@ def test_unblock_resets_failure_counters(kanban_home):
             assert task.last_failure_error is None
     
     
    +def test_recompute_ready_skips_tasks_at_failure_limit(kanban_home):
    +    """recompute_ready must not auto-recover tasks whose consecutive_failures
    +    has reached the circuit-breaker limit (#35072).
    +
    +    Without this guard, a task that repeatedly exhausts its iteration
    +    budget would cycle forever: block → auto-recover (counter reset)
    +    → respawn → budget exhausted → block → …
    +    """
    +    with kb.connect() as conn:
    +        parent = kb.create_task(conn, title="parent", assignee="a")
    +        child = kb.create_task(conn, title="child", assignee="a",
    +                               parents=[parent])
    +        # Complete the parent so the child's dependencies are satisfied.
    +        kb.claim_task(conn, parent)
    +        kb.complete_task(conn, parent, summary="done")
    +
    +        # Simulate the child having exhausted its budget twice,
    +        # hitting the default failure limit (2).
    +        kb.claim_task(conn, child)
    +        kb._record_task_failure(
    +            conn, child, error="budget exhausted 1",
    +            outcome="timed_out", release_claim=True, end_run=True,
    +            failure_limit=2,
    +        )
    +        kb._record_task_failure(
    +            conn, child, error="budget exhausted 2",
    +            outcome="timed_out", release_claim=True, end_run=True,
    +            failure_limit=2,
    +        )
    +        task = kb.get_task(conn, child)
    +        assert task.status == "blocked"
    +        assert task.consecutive_failures >= 2
    +
    +        # recompute_ready must NOT promote this task — the circuit
    +        # breaker has tripped and it should stay blocked.
    +        promoted = kb.recompute_ready(conn)
    +        assert promoted == 0
    +        assert kb.get_task(conn, child).status == "blocked"
    +
    +        # Explicit unblock should still work and reset the counter.
    +        assert kb.unblock_task(conn, child)
    +        task = kb.get_task(conn, child)
    +        assert task.status == "ready"
    +        assert task.consecutive_failures == 0
    +
    +
    +def test_recompute_ready_recovers_below_limit(kanban_home):
    +    """recompute_ready auto-recovers blocked tasks that haven't hit the
    +    failure limit yet — the counter is preserved across recovery."""
    +    with kb.connect() as conn:
    +        t = kb.create_task(conn, title="task", assignee="a")
    +        kb.claim_task(conn, t)
    +        # One failure, below the default limit of 2.
    +        kb._record_task_failure(
    +            conn, t, error="budget exhausted 1",
    +            outcome="timed_out", release_claim=True, end_run=True,
    +            failure_limit=2,
    +        )
    +        task = kb.get_task(conn, t)
    +        assert task.status == "ready"
    +        assert task.consecutive_failures == 1
    +
    +        # Simulate being blocked by something else (not circuit breaker).
    +        conn.execute(
    +            "UPDATE tasks SET status = 'blocked' WHERE id = ?", (t,),
    +        )
    +        conn.commit()
    +
    +        promoted = kb.recompute_ready(conn)
    +        assert promoted == 1
    +        task = kb.get_task(conn, t)
    +        assert task.status == "ready"
    +        # Counter must be preserved, not reset.
    +        assert task.consecutive_failures == 1
    +
    +
    +def test_recompute_ready_honours_dispatcher_failure_limit(kanban_home):
    +    """The guard's effective limit must follow the same resolution order
    +    as the circuit breaker (#35072): per-task max_retries → dispatcher
    +    failure_limit → DEFAULT_FAILURE_LIMIT.
    +
    +    Without threading the dispatcher's ``kanban.failure_limit`` through,
    +    the guard falls back to DEFAULT_FAILURE_LIMIT and disagrees with the
    +    breaker — sticking a task prematurely (config limit > default) or
    +    letting a tripped task escape (config limit < default).
    +    """
    +    with kb.connect() as conn:
    +        # Config allows MORE retries than the default. A task blocked
    +        # with failures below the configured limit must still recover.
    +        t = kb.create_task(conn, title="lenient", assignee="a")
    +        conn.execute(
    +            "UPDATE tasks SET status='blocked', consecutive_failures=? "
    +            "WHERE id=?",
    +            (kb.DEFAULT_FAILURE_LIMIT, t),
    +        )
    +        conn.commit()
    +        # Default-limit call would stick it (failures >= default).
    +        assert kb.recompute_ready(conn) == 0
    +        assert kb.get_task(conn, t).status == "blocked"
    +        # Dispatcher configured a higher limit → recover, preserve counter.
    +        promoted = kb.recompute_ready(
    +            conn, failure_limit=kb.DEFAULT_FAILURE_LIMIT + 2
    +        )
    +        assert promoted == 1
    +        task = kb.get_task(conn, t)
    +        assert task.status == "ready"
    +        assert task.consecutive_failures == kb.DEFAULT_FAILURE_LIMIT
    +
    +        # Config allows FEWER retries than the default. A task at the
    +        # stricter limit must stay blocked even though it's below default.
    +        t2 = kb.create_task(conn, title="strict", assignee="a")
    +        conn.execute(
    +            "UPDATE tasks SET status='blocked', consecutive_failures=1 "
    +            "WHERE id=?",
    +            (t2,),
    +        )
    +        conn.commit()
    +        # Default-limit (2) would recover it (1 < 2).
    +        # Stricter config limit (1) must keep it blocked (1 >= 1).
    +        assert kb.recompute_ready(conn, failure_limit=1) == 0
    +        assert kb.get_task(conn, t2).status == "blocked"
    +
    +
    +def test_recompute_ready_per_task_max_retries_overrides_dispatcher(kanban_home):
    +    """A per-task ``max_retries`` wins over the dispatcher failure_limit,
    +    matching ``_record_task_failure``'s resolution order."""
    +    with kb.connect() as conn:
    +        t = kb.create_task(conn, title="per-task", assignee="a")
    +        # Per-task allows 4 retries; dispatcher config says 2.
    +        conn.execute(
    +            "UPDATE tasks SET status='blocked', consecutive_failures=2, "
    +            "max_retries=4 WHERE id=?",
    +            (t,),
    +        )
    +        conn.commit()
    +        # failures(2) < per-task limit(4) → recover, despite dispatcher=2.
    +        promoted = kb.recompute_ready(conn, failure_limit=2)
    +        assert promoted == 1
    +        task = kb.get_task(conn, t)
    +        assert task.status == "ready"
    +        assert task.consecutive_failures == 2
    +
    +
     # ---------------------------------------------------------------------------
     # Parent-completion invariant at the claim gate (RCA t_a6acd07d)
     # ---------------------------------------------------------------------------