Skip to content

fix(kanban): floor consecutive_failures at effective_limit on breaker trip (t_21f59f6d) - #23

Merged
SSC-ENG merged 2 commits into
mainfrom
fix/kanban-parents-terminal-floor-failures
Jul 31, 2026
Merged

fix(kanban): floor consecutive_failures at effective_limit on breaker trip (t_21f59f6d)#23
SSC-ENG merged 2 commits into
mainfrom
fix/kanban-parents-terminal-floor-failures

Conversation

@SSC-ENG

@SSC-ENG SSC-ENG commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Kanban card: t_21f59f6d — "DISPATCH BUG: blocked cards spuriously re-promoted to ready via empty-parents 'parents_terminal' trigger (t_342c4c9f loop, 100+ wasted spawns)".

Linear issue: none exists for this card (kanban-native dispatcher infra bug; no user-facing Linear-tracked feature/fix maps to it).

Root cause

_record_task_failure's breaker-trip path (hermes_cli/kanban_db.py) persisted the raw per-call failures counter on consecutive_failures instead of flooring it at the effective_limit that actually tripped the breaker.

For force_trip=True callers (protocol-violation streak, systemic crash fingerprint), the streak that trips the breaker is often unrelated to the unified consecutive_failures column. If that column was reset to 0 by a prior unblock_task, the very first force-trip call computes failures=1 even when tripping on, say, failure_limit=3.

Storing that raw 1 let recompute_ready — invoked later in the SAME dispatch tick, re-resolving its own (often lower/unrelated) effective_limit — see 1 < 2 and immediately promote the task straight back to ready, undoing the trip within the same tick.

Observed in production:

  • Escalation card t_d1994b5b hit this 96+ times, feeding a BEL-ESCALATION storm of wasted dispatch/spawn/crash cycles.
  • Card t_342c4c9f flapped blocked -> ready -> crashed repeatedly with the identical signature: {"trigger": "parents_terminal", "satisfied_parent_ids": []} on a task with zero parents (a zero-parent task's parent list is vacuously "all done", so any recompute_ready re-check trivially satisfies the promotion condition once the under-reported counter clears whatever limit it re-resolves).
  • This bug's own tracking card, t_21f59f6d, reproduced it live during investigation: gave_up at failures=2/effective_limit=2, immediately re-promoted via parents_terminal with satisfied_parent_ids=[].

Fix

Persist max(failures, effective_limit) on the stored counter at trip time. Any later re-check computing the same-or-lower limit now correctly sees the task as still over threshold. A genuinely higher configured limit (e.g. a raised max_retries) can still recover the task — unified counter semantics preserved, just no longer allowed to under-report.

Tests

New regression test tests/hermes_cli/test_kanban_blocked_sticky.py::test_force_trip_floors_consecutive_failures_at_effective_limit:

  • Force-trips with failure_limit=3 on a freshly-created task (counter starts at 0).
  • Asserts the stored consecutive_failures is floored at 3, not the raw per-call count of 1.
  • Simulates the same-tick recompute_ready(failure_limit=2) call and asserts it does NOT re-promote (would have, pre-fix, since 1 < 2).

Verified the test fails without the source fix (assert 1 == 3 — got 1) and passes with it.

Full kanban test slice (tests/hermes_cli/ -k kanban): 269 passed / 9 pre-existing failures, identical on the unpatched fork/main baseline (environment-only: missing httpx2, decompose-message-format drift, lifecycle-hook fixture, write-guard real-root fixture) — zero net-new failures introduced by this change.

Not done / explicitly out of scope

  • Does NOT restart the live gateway process. Python does not hot-reload an already-imported module, so any dispatcher gateway process that imported the old _record_task_failure/recompute_ready before this merge will keep exhibiting the bug until it is restarted/replaced after merge. Whoever merges this should schedule that restart.
  • Does NOT address the separate rc=0 protocol-violation classification issue (already fixed independently in fix(kanban): stop miscounting forced kills and init failures as protocol_violation #22 / merged 996ac78c4).
  • NOT release-ready by self-declaration — awaiting TRC (Tessa Cole) technical review per the standing merge lane.

Card: t_21f59f6d.

SSC-ENG added 2 commits July 30, 2026 20:54
… trip

_record_task_failure's force_trip / threshold-trip path was storing the
raw `failures` counter instead of flooring it at the effective_limit
that actually tripped the breaker. For force_trip callers (protocol-
violation streak, systemic crash fingerprint) the streak that trips the
breaker often has nothing to do with the unified consecutive_failures
column — e.g. a violation streak of 3 force-trips with failure_limit=3
while the column itself sat at 0 (reset by an earlier unblock), so
`failures` computed inside this call is only 1.

Storing that raw `1` let recompute_ready — which runs later in the
SAME dispatch tick and re-resolves its own (often lower/unrelated)
effective_limit — see "1 < 2" and immediately promote the task straight
back to ready, undoing the trip within the same tick.

Observed in production as an infinite blocked -> ready -> crash loop on
zero-parent tasks, surfaced as a promoted event carrying
{"trigger": "parents_terminal", "satisfied_parent_ids": []} because a
zero-parent task's parent list is vacuously "all done". Escalation card
t_d1994b5b hit this 96+ times feeding a BEL-ESCALATION storm; card
t_342c4c9f flapped blocked->ready->crashed repeatedly with the same
signature; card t_21f59f6d (this bug's own tracking card) reproduced it
live during investigation (gave_up at failures=2/effective_limit=2,
immediately re-promoted via parents_terminal with satisfied_parent_ids=[]).

Fix: persist max(failures, effective_limit) on the stored counter. Any
later re-check computing the same-or-lower limit now correctly sees the
task as still over threshold; a genuinely higher configured limit can
still recover it (unified counter semantics preserved, just no longer
allowed to under-report).

Card: t_21f59f6d.
…f6d)

Reproduces the exact bug shape: force_trip=True computes a raw
per-call failures count of 1 (column reset to 0 by a prior unblock)
while tripping on a higher effective_limit=3. Before the fix, the
under-reported counter let a subsequent recompute_ready call with a
lower effective_limit=2 satisfy 1 < 2 and re-promote the task within
the same tick -- exactly the parents_terminal/satisfied_parent_ids=[]
loop seen on t_d1994b5b (96+ times) and t_342c4c9f.

Verified: fails without the kanban_db.py fix (asserts 1 == 3, got 1),
passes with it. Full kanban test slice: 269 passed / 9 pre-existing
environment-only failures (identical on unpatched fork/main baseline,
zero net-new failures introduced).
@SSC-ENG

SSC-ENG commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

TRC Technical Review — PR #23 fix/kanban-parents-terminal-floor-failures

Intake: structurally present — committed on named branch, pushed, open PR #23 (SSC-ENG/hermes-agent), base main@996ac78c, head e990d8ba6a04b4a2c3049face3e021bc36f3e3d3, linked card t_21f59f6d. mergeStateStatus=CLEAN, mergeable=MERGEABLE, state=OPEN. Re-queried immediately before posting this verdict — head unchanged.

1. Diff scope

git diff 996ac78c...e990d8ba — exactly 2 files, +92/-2:

  • hermes_cli/kanban_db.py +25/-2: adds stored_failures = max(failures, effective_limit) inside the breaker-trip branch of _record_task_failure, used in both the spawn-path and timeout/crash-path UPDATE statements in place of the raw failures value. No other lines touched. Matches the described fix exactly, no scope creep.
  • tests/hermes_cli/test_kanban_blocked_sticky.py +67/-0: one new regression test, test_force_trip_floors_consecutive_failures_at_effective_limit.

2. Regression test genuinely reproduces the bug — independently re-verified

Checked out exact head e990d8b, confirmed test passes (3 passed). Then reverted hermes_cli/kanban_db.py alone to the pre-fix blob at base 996ac78 (keeping the new test), re-ran the single new test:

FAILED ...test_force_trip_floors_consecutive_failures_at_effective_limit
AssertionError: consecutive_failures must be floored at effective_limit (3), got 1 ...
assert 1 == 3

Confirms the test fails without the source fix and passes with it — genuine reproduction, not a tautology. Restored the fixed file afterward (working tree clean, verified via git diff --stat).

3. Full kanban test slice, fresh at exact head

Ran uv run pytest tests/hermes_cli/ -k kanban -q on a clean clone at head e990d8b (Python 3.12 venv, uv sync):

9 failed, 269 passed, 1 skipped, 3476 deselected

Matches the expected 269 passed / 9 pre-existing failures exactly. Ran the identical command on the unpatched fork/main baseline (996ac78) in a separate worktree to confirm the 9 failures are pre-existing and environment-only, not introduced by this PR:

9 failed, 268 passed, 1 skipped, 3476 deselected

Same 9 failing test IDs on both runs (test_rate_limit_exit_requeues_without_counting_failure, test_connect_works_when_wal_is_silently_refused, 4x decompose fanout tests, test_claim_fires_hook, 2x write-guard real-root tests) — httpx2-deprecation-adjacent, lifecycle-hook fixture, and write-guard real-root fixture issues, identical on both heads. The 269 vs 268 delta is exactly the one new regression test. No fresh-run discrepancy to flag.

4. CI status at exact head

gh pr checks 23: all required checks pass. Python tests slices 1/8–8/8 pass, Python lints pass, OSV/supply-chain scans pass, Desktop E2E pass. "All required checks pass" summary check: pass.

5. Scope-creep check

Confirmed via git diff 996ac78c...e990d8ba -- hermes_cli/kanban_db.py: the diff touches only the _record_task_failure breaker-trip block. It does not touch the rc=0 protocol-violation classification path (that landed separately in #22/996ac78c and is untouched here — verified by diff, zero lines outside _record_task_failure).

Operational flag (does not block TRC PASS, but is load-bearing): this fix is code-only. The live dispatcher gateway (pid 32061, --profile 01-max-headroom gateway run --replace, started 2026-07-30 15:47:32, confirmed still running) started before this fix existed and will keep running the old _record_task_failure in-process until it is merged AND the gateway process is restarted/replaced. A merge alone will look done on GitHub while the board keeps flapping on the same blocked→ready→crash loop until the gateway reloads. This is explicitly handed to the gated merge-lane task (t_3b2e98ec, assignee rhea-ramos) as step 3 of its procedure — flagging here per the review ask, not re-litigating ownership.

Verdict

GO. Diff is scoped exactly to the described fix + one regression test. Regression test independently re-verified to fail pre-fix / pass post-fix. Full kanban slice fresh at exact head matches expected 269/9 split, cross-checked against baseline to confirm the 9 failures are pre-existing and environment-only. CI green at exact head. No scope creep into the separately-merged rc=0 classification work. Gateway-restart operational dependency named explicitly for the merge lane.

GATEWAY-VERDICT: TRC=PASS head=e990d8ba6a04b4a2c3049face3e021bc36f3e3d3

@SSC-ENG
SSC-ENG merged commit e3d4e0a into main Jul 31, 2026
37 checks passed
SSC-ENG added a commit that referenced this pull request Aug 1, 2026
…t dispatcher auto-promotion as a legitimate unblock (t_e2b1f62a) (#25)

Kanban card: t_e2b1f62a (rhea-ramos) — second, distinct dispatch-routing
defect discovered while verifying PR #23's fix for t_21f59f6d.

Linear issue: none exists for this card (kanban-native dispatcher infra
bug; no user-facing Linear-tracked feature/fix maps to it — same
convention as PR #23/t_21f59f6d).

Root cause (a): block_task()'s unblock-loop breaker incremented
block_recurrences on any same-kind re-block, with no check on WHAT put
the task back in running/ready. A dispatcher-side auto-promotion
(recompute_ready's parents_terminal trigger, or any other non-
unblock_task exit from blocked) looked identical, at this layer, to a
genuine human/cron unblock_task -> worker re-block ping-pong. This
inflated t_342c4c9f's counter to 3 (limit=2) purely from residual churn
caused by the now-fixed t_21f59f6d bug, tripping block_loop_detected and
routing an already-reviewed, correctly-blocked card to triage instead of
blocked.

Fix: block_task now checks the most recent status-exit event kind before
counting a same-block_kind re-block as the same-cause loop signal. Only
an explicit unblock_task call (which emits "unblocked") re-arms the
counter; a dispatcher "promoted" event resets it to a fresh 1.

Root cause (b): decompose_task() (and the gateway's auto-decompose
sweep via list_triage_ids()) treated every triage card identically,
including ones routed there by block_loop_detected specifically to
force a human decision. auto-decomposer picked such a card up, made a
cosmetic title/body edit, and immediately promoted it straight back to
ready via parents_terminal (with REAL, all-done parent ids this time —
a distinct defect from t_21f59f6d) — defeating the loop breaker's whole
purpose within ~90 seconds.

Fix: decompose_task() now checks whether the triage card's most recent
event is block_loop_detected and refuses to specify/promote it (no LLM
call made), leaving it for an explicit human/operator action.

Ask (c) confirmed structurally: the active_pr respawn guard
(_RESPAWN_GUARD_PR_WINDOW = 86400s) is NOT indefinite — without fixes
(a)/(b) the wasted-spawn loop would have resumed after 24h at a slower
cadence, not stayed blocked forever. This makes (a)/(b) the real fix.

## Tests

- test_kanban_blocked_sticky.py: added
  test_dispatcher_repromotion_does_not_inflate_block_recurrences
  (reproduces the t_342c4c9f loop across 4 dispatcher-style
  re-promotion cycles; asserts recurrences stay at 1 and status stays
  blocked) and test_genuine_unblock_reblock_loop_still_trips_breaker
  (sanity check the original Dale's-Type-1 loop-breaker behavior is
  preserved: BLOCK_RECURRENCE_LIMIT=2 still trips on the 2nd genuine
  same-cause unblock->reblock).
- test_kanban_decompose.py: added
  test_decompose_skips_triage_card_from_block_loop_detected (asserts
  ok=False, no LLM call, card stays in triage untouched).

All new/modified tests pass (12/12). Ran full tests/hermes_cli -k kanban
suite on this branch (276 passed, 9 pre-existing failures) and diffed
1:1 against the same suite on unmodified fork/main (273 passed, same 9
failures) — confirmed the 9 failures are pre-existing test-isolation
issues unrelated to this change, and this diff adds exactly 3 new
passing tests with zero regressions.

🤖 Generated with Hermes Agent (rhea-ramos)

Co-authored-by: SSC-ENG <225143396+SSC-ENG@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant