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
1 change: 1 addition & 0 deletions PATCHES.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,4 @@ default; do **not** retire such a row on a PR-merge signal. See the #44338 row.
| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Stop the kanban block-loop breaker from false-tripping on a healthy multi-round review cycle. The loop breaker in `block_task` (`hermes_cli/kanban_db.py`) increments `block_recurrences` whenever a task is re-blocked for the SAME `block_kind` after an unblock, routing to `triage` at `BLOCK_RECURRENCE_LIMIT` (2) to break a cron-unblock↔worker-re-block loop. But a legitimate review cycle (`review-changes-requested` → rework → a NEW, different finding → `review-changes-requested` again) is blocked with the same kind every round, so the second healthy round hit the limit and routed the card to `triage`/`needs_input` — taking it OFF the `auto_route_review_bounce` path (which scans `WHERE status='blocked'`), stranding it with no worker (observed live twice, 2026-07-02 and 2026-07-05 on card `t_0d57d36d`; required manual block→unblock rescue both times). The classifier — not the budget — was the bug (raising the limit only delays the false-trip). Fix: keying on the reviewer's `review-changes-requested` bounce reason, a review re-block whose normalized finding text MATERIALLY DIFFERS from the prior round's `blocked` event reason is treated as a fresh cause (`same_cause=False`, counter resets to 1) — a distinct finding is progress, not a loop. A review bounce repeating the IDENTICAL finding still counts (a reviewer bouncing the same unfixed finding forever IS a real loop worth escalating). Two small helpers added next to the existing `_is_review_bounce_reason`: `_last_block_reason` (most-recent `blocked` event reason, NOT gated on stickiness — the prior round was already unblocked, so `_latest_sticky_block_reason` returns None) and `_review_bounce_finding` (strips the prefix + separator, collapses whitespace, lowercases so trivial jitter is not a new finding). Non-review `needs_input`/`capability`/un-typed blocks are entirely unaffected (the reset is gated on `_is_review_bounce_reason(reason)`), so the breaker's real purpose is preserved. Single-file change in `hermes_cli/kanban_db.py` + 6 regression tests in `tests/hermes_cli/test_kanban_block_kinds.py` (distinct-finding round stays blocked; 3-round distinct cycle never triages; distinct-then-same escalates; same-finding repeat still triages; whitespace/case jitter is not a material change; non-review same-kind loop still trips). Upstream has no equivalent (no `block_recurrences`/`review-changes-requested`/typed-block concept in upstream `hermes_cli/*.py`) — cwest-team review-loop tooling. **Retire trigger:** never auto-retires (permanent-local); remove only if the fork's typed-block loop-breaker / review-lane dispatch is retired. | permanent-local | main@9be292f1e |
| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Provide a sanctioned recovery for a card whose `block_recurrences` loop counter is ALREADY inflated (follow-up to the block-loop-breaker row above). The breaker resets `block_recurrences` ONLY inside `complete_task`, so a card whose counter was inflated by prior buggy-code runs or an operator's repeated block→unblock while diagnosing had NO sanctioned way back to the normal flow — every `block`→`unblock` re-tripped straight to `triage` (live symptom 2026-07-05 on card `t_0d57d36d`, counter stuck at 5; the only escape was the non-obvious `move_card`-to-`blocked`-then-`unblock` workaround that bypasses `block_task`'s counter logic). Two sanctioned paths, both in `hermes_cli/kanban_db.py` + CLI, both keyed on the SAME PR #48 `_review_bounce_finding` classifier so the breaker is never weakened: (1) new core API `reset_block_recurrences(conn, id, *, actor, reason)` zeroes the counter and emits a `block_recurrences_reset` audit event (does NOT touch status/block_kind/claim state — a pure counter reset), surfaced as `hermes kanban unblock --reset-loop <id>` which resets FIRST (so a card already escalated to `triage`, where `unblock_task` does not apply, is still recovered) then attempts the normal unblock; (2) `auto_route_review_bounce` now resets the counter to 0 when a genuine author-rework transition routes the card back — i.e. when the current bounce's finding MATERIALLY DIFFERS from the prior round's (a fresh cycle), while a bounce repeating the IDENTICAL finding LEAVES the counter intact so a real same-unfixed-finding loop still escalates to triage. New helper `_prior_block_reason` returns the SECOND-most-recent `blocked` event reason (the router reads a card already re-blocked, so the newest `blocked` event is the CURRENT round; the prior round is the one before it — distinct from `block_task`'s use of `_last_block_reason`, which fires BEFORE the new block event is written). A plain `unblock` (no `--reset-loop`) is byte-for-byte unchanged: the counter deliberately survives an ordinary unblock (the existing anti-amnesia design). Tests: `tests/hermes_cli/test_kanban_reset_block_loop.py` (10 — reset zeroes + audits; missing-task no-op; already-zero still audits; inflated card recovers to `blocked` not `triage` after reset; same-finding loop STILL escalates; auto-route resets on different finding / preserves on same; CLI `--reset-loop` recovers a blocked card, recovers a triage card, and plain unblock leaves the counter). Real `kanban_db`, temp board, no mocks. **Retire trigger:** open the upstream PR (phase 2), then auto-retire when it merges in a tagged release ≥ base; until then carry as upstream-pending, dropped only when the fork's block-loop-breaker / review-lane dispatch is retired. | permanent-local | main@9be292f1e |
| fork PR (TBD — no upstream PR; fork-internal dispatcher semantics) | Fix the two SIBLING crash-classification paths #47 missed, so the kanban dispatcher stops false-flagging cleanly-finished and transiently-failed workers as `crashed`/`gave_up`. #47 (`426a8a9c7`) carved out the provably-done case for a reap-registry `clean_exit` (rc=0 captured), but two same-class paths in `detect_crashed_workers` (`hermes_cli/kanban_db.py`) still counted a false failure: **(A) transient endpoint-unreachable failures** — a run that died on `APIConnectionError` / connection-refused / provider 5xx (classified `timeout`/`overloaded`/`server_error`/`upstream_rate_limit` after in-process retries) exited plain `1`, which the reaper read as a real crash toward `failure_limit`; a proxy blip thus `gave_up` a card that would otherwise succeed. Fix: new `KANBAN_TRANSIENT_EXIT_CODE` (69, EX_UNAVAILABLE) sentinel + a single-source-of-truth `kanban_worker_exit_code(failure_reason)` helper the CLI kanban-worker exit path delegates to (replacing the inline rate-limit-only mapping); `_classify_worker_exit` maps 69 → a new `transient` kind; `detect_crashed_workers` treats it exactly like the rate-limit carve-out (requeue to `ready`/`review`, NO failure counted, distinct `transient` run outcome + event, stamped `last_failure_error` so the respawn-guard cooldown spaces the retry), surfaced via `DispatchResult.transient` + the `_last_transient` side-channel; `check_respawn_guard`'s cooldown gate now recognizes both `rate_limited` and `transient` outcomes. **(B) clean exit misread as a crash on the `pid not alive`/`unknown` reap path** — #47's provably-done carve-out was gated on `kind == "clean_exit"` only, so a worker that finished its lane cleanly (draft-PR handoff / edit-in-place card) but whose exit was NOT captured in the reap registry (reaped by init, or gone between the reap tick and the liveness check → `_classify_worker_exit` returns `unknown`) fell into the generic `crashed` branch and counted a failure. Fix: extend the `_lane_work_provably_done` carve-out from `kind == "clean_exit"` to `kind in ("clean_exit", "unknown")`, using the SAME durable proof signals (recent_success / active_pr) #47 trusts. Non-regression preserved: a genuine mid-work death with NO proof still `crashed`→`gave_up` after the limit, and a real non-zero crash (not the sentinel) still counts. Two-file change (`hermes_cli/kanban_db.py` + `cli.py`) + 8 behavior-contract tests in `tests/hermes_cli/test_kanban_db.py` (transient-sentinel classify; transient requeue-without-failure across 6 hits; transient cooldown defer/allow; unknown-exit-after-completed-run and unknown-exit-after-draft-PR not counted; unknown-exit-without-proof still crashes; real-crash non-regression; exit-code mapping). **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release ≥ base; until then carry as upstream-pending. | upstream-pending | main@9be292f1e |
| fork PR (TBD — no upstream PR; fork-internal review-lane semantics) | Make `unblock_task` also transition a card out of `triage`, so the OUTER feedback loop (Casey-feedback -> author on a card carrying an open PR) self-heals the `active_pr` respawn-guard wedge that only the INNER review-bounce loop was covered for (composes with rows 84/85 -- the block-loop breaker + reset-recovery -- extending them to the non-review author-bounce case). Live symptom 2026-07-11: a writing card (`t_f6126dcd`, author orwell) carrying an OPEN PR needed to bounce back to its author for a Casey-feedback revision; the hand-move `blocked->todo`+author did not clear the `active_pr` guard (a raw status flip emits no `unblocked` cutoff event), so `hermes kanban dispatch` refused to spawn the author every tick (`respawn_guarded {active_pr}`). Repeated churn inflated `block_recurrences` past `BLOCK_RECURRENCE_LIMIT`, escalating the card to `triage`, where the standard block->unblock cutoff SILENTLY no-oped -- `unblock_task` (`hermes_cli/kanban_db.py`) matched only `status IN ('blocked','scheduled')`, so a triage card matched zero rows, returned False, emitted no `unblocked` event, and the guard stayed tripped forever (only a hand `reset_block_recurrences` + status=blocked + `unblock_task` rescued it). Root cause: the INNER review loop self-heals via `auto_route_review_bounce` (reassigns + `unblock_task`, emitting the `unblocked` cutoff that `check_respawn_guard` honors), but the outer feedback loop has no `review-changes-requested` reason so it is never routed, and the only skill-layer primitive (`onecard_common.move_card`) is a raw status flip that emits no cutoff. Fix (minimal, well-scoped): add `'triage'` to both `WHERE status IN (...)` clauses in `unblock_task` -- the stale-run-pointer SELECT and the status UPDATE -- so a card escalated to `triage` transitions back to `ready`/`todo` (parent-gate re-checked) and emits the `unblocked` cutoff, clearing `active_pr` the same way a normal block->unblock does. Preserves the loop breaker: `block_recurrences` is deliberately NOT reset (counter still survives the unblock; reset only on completion or the explicit `reset_block_recurrences` API), so a genuine same-finding loop still escalates. This supersedes row 85's parenthetical about a card already escalated to `triage` where `unblock_task` did not apply -- with this patch `unblock --reset-loop` fully recovers a triaged card (reset + lane flip). Single-file change in `hermes_cli/kanban_db.py` (+ stale-comment sync in `hermes_cli/kanban.py`) + 5 behavior-contract regression tests in `tests/hermes_cli/test_kanban_reset_block_loop.py` (RED-then-GREEN reproducing the exact triage-with-open-PR wedge; parent-gate re-check; counter preserved; the pre-fix wedge documented). Clean upstream candidate -- a sanctioned unblock that no-ops from triage is a real bug -- but tracked fork-internal for now. **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release >= base; until then carry as permanent-local. | permanent-local | main@9be292f1e |
13 changes: 8 additions & 5 deletions hermes_cli/kanban.py
Original file line number Diff line number Diff line change
Expand Up @@ -2038,8 +2038,10 @@ def _cmd_unblock(args: argparse.Namespace) -> int:
kb.add_comment(conn, tid, author, f"UNBLOCK: {reason}")
# Zero the unblock-loop counter FIRST (independent of the lane
# transition) so a card stuck by an inflated block_recurrences is
# recovered even when it is parked in triage (where unblock_task does
# not apply). Emits its own auditable block_recurrences_reset event.
# returned to a clean cycle. ``unblock_task`` transitions from triage
# too, so the reset + unblock together fully recover a card the
# loop breaker escalated to triage. Emits its own auditable
# block_recurrences_reset event.
if reset_loop:
if not kb.reset_block_recurrences(
conn, tid, actor=author or "user", reason=reason,
Expand All @@ -2051,14 +2053,15 @@ def _cmd_unblock(args: argparse.Namespace) -> int:
if not kb.unblock_task(conn, tid):
if reset_loop:
# The counter reset succeeded; only the lane flip did not
# apply (e.g. the card is in triage, not blocked/scheduled).
# apply (the card was not in a transitionable status —
# blocked/scheduled/triage).
print(
f"Reset loop counter for {tid} "
f"(not blocked/scheduled — left in place)"
f"(not blocked/scheduled/triage — left in place)"
)
else:
failed.append(tid)
print(f"cannot unblock {tid} (not blocked/scheduled?)", file=sys.stderr)
print(f"cannot unblock {tid} (not blocked/scheduled/triage?)", file=sys.stderr)
else:
suffix = f": {reason}" if reason else ""
prefix = "Unblocked + reset loop counter" if reset_loop else "Unblocked"
Expand Down
20 changes: 17 additions & 3 deletions hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -5897,19 +5897,33 @@ def promote_task(


def unblock_task(conn: sqlite3.Connection, task_id: str) -> bool:
"""Transition ``blocked``/``scheduled`` -> ready or todo.
"""Transition ``blocked``/``scheduled``/``triage`` -> ready or todo.

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.

``triage`` is a transitionable source status alongside ``blocked``/
``scheduled``: the block-loop breaker escalates a card to ``triage`` once
``block_recurrences`` reaches :data:`BLOCK_RECURRENCE_LIMIT` (see
:func:`block_task`), and the OUTER feedback loop (Casey-feedback -> author on
a card carrying an open PR) has no ``review-changes-requested`` reason, so
:func:`auto_route_review_bounce` never routes it. Without ``triage`` here, a
card churned into triage could never emit the ``unblocked`` cutoff event that
:func:`check_respawn_guard` honors to clear the ``active_pr`` guard — so it
wedged forever (live 2026-07-11 symptom). Transitioning from ``triage`` makes
the sanctioned unblock self-heal that class. It deliberately does NOT reset
``block_recurrences`` (see the note below), so the loop breaker is preserved:
the counter still survives the unblock and is reset only on completion or via
the explicit :func:`reset_block_recurrences` recovery API.
"""
now = int(time.time())
with write_txn(conn):
stale = conn.execute(
"SELECT current_run_id FROM tasks WHERE id = ? AND status IN ('blocked', 'scheduled')",
"SELECT current_run_id FROM tasks WHERE id = ? AND status IN ('blocked', 'scheduled', 'triage')",
(task_id,),
).fetchone()
if stale and stale["current_run_id"]:
Expand Down Expand Up @@ -5950,7 +5964,7 @@ def unblock_task(conn: sqlite3.Connection, task_id: str) -> bool:
cur = conn.execute(
"UPDATE tasks SET status = ?, current_run_id = NULL, "
"consecutive_failures = 0, last_failure_error = NULL "
"WHERE id = ? AND status IN ('blocked', 'scheduled')",
"WHERE id = ? AND status IN ('blocked', 'scheduled', 'triage')",
(new_status, task_id),
)
if cur.rowcount != 1:
Expand Down
Loading
Loading