fix(kanban): dispatcher blocks a card on the first terminal provider error
Before, a worker killed by a revoked credential or a missing model was booked as an ordinary crash and re-spawned into the identical failure until kanban.failure_limit / max_retries was spent — burning worker slots and the retry budget on something a retry cannot fix (#114587). Now KANBAN_TERMINAL_PROVIDER_EXIT_CODE (78) is its own exit kind, `terminal_provider`: `_classify_dead_worker_exit` books the run crashed with the provider's words appended, and `_account_crashes` force-trips the breaker on that first death (sticky, so recompute_ready does not resume it before the operator fixes the provider). Same booking in the implementation and the review lane — the review worker dies through the same sweep. Transient failures (429 / 5xx / timeout) keep the existing rate_limited requeue and the consecutive_failures budget. `hermes kanban show` / the dashboard diagnostics now fire for that trip below the repeated-failure threshold ("Provider rejected this profile's credential or model — blocked after one attempt") with the fix path. No new columns, no separate review-lane counter, no new config: option B of #114587. The terminal-vs-transient split was proposed in #114589 by @TFOjojo; its regex-on-error-text classifier is replaced by the worker's own FailoverReason verdict. Part of #114587 Co-authored-by: TFOjojo <279183633+TFOjojo@users.noreply.github.com>
This commit is contained in:
@@ -75,7 +75,10 @@ zero outside a kanban task (footprint ladder rung 3).
|
||||
Isolation: **board** is the hard boundary — workers get `HERMES_KANBAN_BOARD` pinned in their env and
|
||||
cannot see other boards; **tenant** is a soft namespace within a board (workspace-path + memory-key
|
||||
isolation, one fleet serving several businesses). After `kanban.failure_limit` consecutive
|
||||
non-success attempts on a task (default 2) the dispatcher auto-blocks it to stop spin loops.
|
||||
non-success attempts on a task (default 2) the dispatcher auto-blocks it to stop spin loops; a
|
||||
worker exit of `KANBAN_TERMINAL_PROVIDER_EXIT_CODE` (78 — credential revoked, model gone; the
|
||||
worker's own `failure_reason` classification via `cli._TERMINAL_PROVIDER_REASONS`) trips it on
|
||||
the first attempt, sticky, because no retry can heal it (#114587).
|
||||
Process-identity note: `kanban --preserve-cache` contains "serve" — never classify processes by argv
|
||||
substring (root). Worker liveness is `(worker_pid, worker_started_at)` — the start-time fingerprint
|
||||
(`gateway.status.get_process_start_time`) recorded at claim time — never bare PID existence, or a
|
||||
|
||||
@@ -246,6 +246,8 @@ def _exit_code_kind(code: int) -> "tuple[str, int]":
|
||||
return ("clean_exit", 0)
|
||||
if code == _kb.KANBAN_RATE_LIMIT_EXIT_CODE:
|
||||
return ("rate_limited", code)
|
||||
if code == _kb.KANBAN_TERMINAL_PROVIDER_EXIT_CODE:
|
||||
return ("terminal_provider", code)
|
||||
return ("nonzero_exit", code)
|
||||
|
||||
|
||||
@@ -1016,6 +1018,9 @@ class _DeadWorker:
|
||||
event_payload: dict
|
||||
protocol_violation: bool = False
|
||||
rate_limited: bool = False
|
||||
terminal_provider: bool = False
|
||||
"""``KANBAN_TERMINAL_PROVIDER_EXIT_CODE``: the provider rejected the worker's
|
||||
credential/model — trips the breaker on this first occurrence."""
|
||||
|
||||
@property
|
||||
def run_outcome(self) -> str:
|
||||
@@ -1084,6 +1089,18 @@ def _classify_dead_worker_exit(
|
||||
{"pid": pid, "claimer": claimer, "exit_code": code},
|
||||
rate_limited=True,
|
||||
)
|
||||
if kind == "terminal_provider":
|
||||
# The worker classified its own provider failure as unhealable (credential
|
||||
# revoked, model gone): every further spawn would hit the same wall, so
|
||||
# ``_account_crashes`` trips the breaker now instead of after ``failure_limit``.
|
||||
return _DeadWorker(
|
||||
kind, code,
|
||||
f"pid {pid} exited on a terminal provider error (exit {code}): the provider rejected "
|
||||
"this profile's credential or model — fix the configuration, then unblock.",
|
||||
"crashed",
|
||||
{"pid": pid, "claimer": claimer, "exit_kind": kind, "exit_code": code, "terminal_provider": True},
|
||||
terminal_provider=True,
|
||||
)
|
||||
if kind == "nonzero_exit":
|
||||
error_text = f"pid {pid} exited with code {code}"
|
||||
elif kind == "signaled":
|
||||
@@ -1103,9 +1120,9 @@ class _CrashSweep:
|
||||
|
||||
crashed: list[str] = field(default_factory=list)
|
||||
rate_limited: list[str] = field(default_factory=list)
|
||||
# ``(task_id, pid, claimer, protocol_violation, error_text)``: accounted
|
||||
# after the txn via ``_record_task_failure`` (needs its own write_txn).
|
||||
crash_details: list[tuple[str, int, str, bool, str]] = field(default_factory=list)
|
||||
# ``(task_id, pid, claimer, dead_worker)``: accounted after the txn via
|
||||
# ``_record_task_failure`` (needs its own write_txn).
|
||||
crash_details: list[tuple[str, int, str, _DeadWorker]] = field(default_factory=list)
|
||||
# Worker-exit observer payloads, fired only after every reclaim/accounting
|
||||
# txn has committed.
|
||||
exited_hook_payloads: list[dict] = field(default_factory=list)
|
||||
@@ -1177,9 +1194,7 @@ def _reclaim_dead_workers(conn: sqlite3.Connection, board: Optional[str] = None)
|
||||
sweep.rate_limited.append(row["id"])
|
||||
else:
|
||||
sweep.crashed.append(row["id"])
|
||||
sweep.crash_details.append(
|
||||
(row["id"], pid, row["claim_lock"], dead.protocol_violation, dead.error_text)
|
||||
)
|
||||
sweep.crash_details.append((row["id"], pid, row["claim_lock"], dead))
|
||||
return sweep
|
||||
|
||||
|
||||
@@ -1188,16 +1203,18 @@ def _account_crashes(conn: sqlite3.Connection, crash_details: list) -> list[str]
|
||||
|
||||
Protocol violations get a BOUNDED violation-only budget independent of
|
||||
``consecutive_failures`` (per-task ``max_retries`` takes precedence);
|
||||
systemic same-error crashes (>= 3 identical fingerprints this tick) trip
|
||||
immediately.
|
||||
systemic same-error crashes (>= 3 identical fingerprints this tick) and
|
||||
terminal provider errors (credential revoked, model gone — a retry cannot
|
||||
heal them) trip immediately.
|
||||
"""
|
||||
auto_blocked: list[str] = []
|
||||
fp_counts: dict[str, int] = {}
|
||||
for _, _, _, _, err_text in crash_details:
|
||||
fp = _error_fingerprint(err_text)
|
||||
for _, _, _, dead in crash_details:
|
||||
fp = _error_fingerprint(dead.error_text)
|
||||
fp_counts[fp] = fp_counts.get(fp, 0) + 1
|
||||
for tid, pid, claimer, protocol_violation, error_text in crash_details:
|
||||
if protocol_violation:
|
||||
for tid, pid, claimer, dead in crash_details:
|
||||
error_text = dead.error_text
|
||||
if dead.protocol_violation:
|
||||
streak = _protocol_violation_streak(conn, tid)
|
||||
trow = conn.execute("SELECT max_retries FROM tasks WHERE id = ?", (tid,)).fetchone()
|
||||
if trow is None:
|
||||
@@ -1227,6 +1244,20 @@ def _account_crashes(conn: sqlite3.Connection, crash_details: list) -> list[str]
|
||||
"protocol_violation_limit": violation_limit,
|
||||
},
|
||||
)
|
||||
elif dead.terminal_provider:
|
||||
# A retry cannot heal a revoked credential or a missing model, so
|
||||
# the whole ``failure_limit`` budget would be spent on identical
|
||||
# failures. ``force_trip`` blocks now, sticky: ``recompute_ready``
|
||||
# must not auto-resume it before the operator fixes the provider.
|
||||
tripped = _record_task_failure(
|
||||
conn, tid,
|
||||
error=error_text,
|
||||
outcome="crashed",
|
||||
force_trip=True,
|
||||
release_claim=False,
|
||||
end_run=False,
|
||||
event_payload_extra={"pid": pid, "claimer": claimer, "terminal_provider": True},
|
||||
)
|
||||
else:
|
||||
is_systemic = fp_counts.get(_error_fingerprint(error_text), 0) >= 3
|
||||
extra = {"pid": pid, "claimer": claimer}
|
||||
|
||||
@@ -112,6 +112,18 @@ def _latest_event_ts(events: Iterable[Any], kinds: set[str]) -> int:
|
||||
return max([0, *(_event_ts(ev) for ev in events if _event_kind(ev) in kinds)])
|
||||
|
||||
|
||||
def _latest_gave_up_is_terminal_provider(events: Iterable[Any]) -> bool:
|
||||
"""True when the most recent breaker trip was a terminal provider error (credential
|
||||
revoked, model gone) and nothing has resumed the task since."""
|
||||
for ev in reversed(list(events)):
|
||||
kind = _event_kind(ev)
|
||||
if kind == "gave_up":
|
||||
return bool(_parse_payload(ev).get("terminal_provider"))
|
||||
if kind in {"unblocked", "promoted", "completed", "claimed"}:
|
||||
return False
|
||||
return False
|
||||
|
||||
|
||||
def _cli_hint(label: str, command: str, *, suggested: bool = False) -> DiagnosticAction:
|
||||
return DiagnosticAction(kind="cli_hint", label=label, payload={"command": command},
|
||||
suggested=suggested)
|
||||
@@ -369,8 +381,12 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]:
|
||||
threshold = _positive_int(_failure_threshold(cfg), 3)
|
||||
failure_limit = _positive_int(cfg.get("failure_limit"), threshold)
|
||||
failures = _first_field(task, "consecutive_failures", "spawn_failures", 0)
|
||||
if failures is None or failures < threshold:
|
||||
# A terminal provider error (credential revoked, model gone) blocks the card after ONE
|
||||
# attempt, below any threshold; it still needs an operator, so diagnose it now.
|
||||
terminal_trip = _latest_gave_up_is_terminal_provider(events)
|
||||
if not terminal_trip and (failures is None or failures < threshold):
|
||||
return []
|
||||
failures = failures or 0
|
||||
last_err = _first_field(task, "last_failure_error", "last_spawn_error")
|
||||
assignee = _task_field(task, "assignee")
|
||||
|
||||
@@ -397,7 +413,15 @@ def _rule_repeated_failures(task, events, runs, now, cfg) -> list[Diagnostic]:
|
||||
severity = "critical" if failures >= threshold * 2 else "error"
|
||||
err_snippet = _error_snippet(last_err)
|
||||
outcome_label = _OUTCOME_LABELS.get(most_recent_outcome or "", "failure")
|
||||
if err_snippet:
|
||||
if terminal_trip:
|
||||
title = "Provider rejected this profile's credential or model — blocked after one attempt"
|
||||
detail = (
|
||||
f"The worker's provider call failed with an error a retry cannot fix (revoked or invalid "
|
||||
f"API key, model not found), so the dispatcher blocked the task instead of spending the "
|
||||
f"{failure_limit}-attempt retry budget on it. Full last error:\n\n{err_snippet}\n\n"
|
||||
f"Fix the assignee profile's provider credentials/model, then unblock the task."
|
||||
)
|
||||
elif err_snippet:
|
||||
title = f"Agent {outcome_label} x{failures}: {err_snippet.splitlines()[0][:160]}"
|
||||
detail = (
|
||||
f"This task has failed {failures} times in a row (most recent: {outcome_label}). Full "
|
||||
|
||||
@@ -387,6 +387,50 @@ def test_rate_limit_exit_requeues_without_counting_failure(
|
||||
assert "crashed" not in outcomes
|
||||
|
||||
|
||||
@pytest.mark.parametrize("lane", ["ready", "review"])
|
||||
def test_terminal_provider_exit_blocks_after_one_attempt_in_either_lane(kanban_home, monkeypatch, lane):
|
||||
"""A worker that exits ``KANBAN_TERMINAL_PROVIDER_EXIT_CODE`` (credential revoked, model
|
||||
gone) parks the card ``blocked`` on the FIRST death — well below ``failure_limit`` and the
|
||||
per-task ``max_retries`` — with the provider error as the reason, sticky against
|
||||
``recompute_ready``. Same booking for the implementation and the review lane (#114587)."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
from hermes_cli import kanban_db_dispatch as _kbd
|
||||
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: False)
|
||||
monkeypatch.setenv("HERMES_KANBAN_CRASH_GRACE_SECONDS", "0")
|
||||
|
||||
with kbc.connect() as conn:
|
||||
host = _kb._claimer_id().split(":", 1)[0]
|
||||
tid = kb.create_task(conn, title="terminal", assignee="a", max_retries=5)
|
||||
claimed = kb.claim_task(conn, tid, claimer=f"{host}:w0")
|
||||
if lane == "review":
|
||||
assert kb.request_review(conn, tid, summary="done", reviewer="r",
|
||||
expected_run_id=claimed.current_run_id)
|
||||
assert kb.claim_review_task(conn, tid, claimer=f"{host}:r0") is not None
|
||||
pid = 71000
|
||||
conn.execute("UPDATE tasks SET worker_pid=? WHERE id=?", (pid, tid))
|
||||
conn.commit()
|
||||
_kbd._record_worker_exit(pid, _exited_status(_kb.KANBAN_TERMINAL_PROVIDER_EXIT_CODE))
|
||||
|
||||
crashed = kbd.detect_crashed_workers(conn)
|
||||
assert tid in crashed
|
||||
assert tid in getattr(_kbd.detect_crashed_workers, "_last_auto_blocked", [])
|
||||
|
||||
task = kb.get_task(conn, tid)
|
||||
assert task.status == "blocked"
|
||||
assert task.consecutive_failures == 1 # one spawn, not failure_limit / max_retries of them
|
||||
assert "terminal provider error" in (task.last_failure_error or "")
|
||||
gave_up = conn.execute(
|
||||
"SELECT payload FROM task_events WHERE task_id=? AND kind='gave_up'", (tid,),
|
||||
).fetchone()
|
||||
assert json.loads(gave_up["payload"])["terminal_provider"] is True
|
||||
|
||||
# Sticky: the breaker did not reach its counter limit, yet the card must stay parked
|
||||
# until an operator fixes the provider and unblocks it.
|
||||
kb.recompute_ready(conn)
|
||||
assert kb.get_task(conn, tid).status == "blocked"
|
||||
|
||||
|
||||
|
||||
|
||||
def test_respawn_guard_defers_rate_limited_within_cooldown(
|
||||
|
||||
@@ -561,11 +561,24 @@ is part of the worker
|
||||
protocol. If the worker process exits with status 0 while the task is still
|
||||
`running`, the dispatcher treats that as a protocol violation and emits a
|
||||
`protocol_violation` event. A dispatcher-spawned worker whose turn failed
|
||||
therefore exits non-zero: `1` for an ordinary failure, and `75`
|
||||
therefore exits non-zero: `1` for an ordinary failure, `75`
|
||||
(`EX_TEMPFAIL`) when the provider was rate-limited, overloaded, returning
|
||||
5xx or timing out, or the account hit a billing/quota wall — the dispatcher records that run as `rate_limited` and
|
||||
requeues the task without counting a failure, so a quota window is never
|
||||
booked as a protocol violation. The worker also writes its exit code as the
|
||||
booked as a protocol violation — and `78` (`EX_CONFIG`) when the provider
|
||||
rejected something a retry cannot fix: the profile's credential (401/403,
|
||||
revoked or invalid key), the model (404 / model not found) or the TLS chain.
|
||||
That **terminal provider error** trips the circuit breaker on the first
|
||||
occurrence: the dispatcher records the run as `crashed` with
|
||||
`exit_kind: terminal_provider`, emits `gave_up` with `terminal_provider: true`
|
||||
and parks the card `blocked` (sticky — `recompute_ready` will not auto-resume
|
||||
it) with the provider's own words in `last_failure_error`, instead of
|
||||
re-spawning into the same wall until `kanban.failure_limit` / `max_retries`
|
||||
is spent. Both lanes get the same booking: a reviewer worker that dies on a
|
||||
revoked key parks the card exactly like an implementer. `hermes kanban show`
|
||||
surfaces it as *Provider rejected this profile's credential or model — blocked
|
||||
after one attempt*; fix the assignee profile's provider (`hermes -p <profile>
|
||||
auth` / `setup`), then `hermes kanban unblock <id>`. The worker also writes its exit code as the
|
||||
last line of its own log (`[kanban-worker-exit] rc=<code>`), so a per-tick
|
||||
`hermes kanban dispatch` process — which never reaped the worker and cannot
|
||||
read its exit status — books the same death the same way the gateway-embedded
|
||||
@@ -1407,7 +1420,7 @@ Every transition appends a row to `task_events`. Each row carries an optional `r
|
||||
| `respawn_guarded` | `{reason}` | Dispatcher refused to re-spawn this ready task this tick. Reasons: `infrastructure_cooldown` (the host refused the last spawn — no restart-safe systemd scope — and the cooldown has not elapsed; never counted against the card), `rate_limit_cooldown` (the last run hit a quota wall; same cooldown, never counted), `blocker_auth` (last failure was a quota/auth/429 error — wait for the rate window to reset), `recent_success` (a completed run happened in the last hour — wait for review before re-running), `active_pr` (a GitHub PR URL appears in a recent comment — a prior worker already opened a PR). The task stays in `ready`; the next tick gets another chance to spawn. If the underlying condition persists, the normal `consecutive_failures` circuit breaker will auto-block via `gave_up` after `failure_limit` failures. |
|
||||
| `spawn_failed` | `{error, failures}` | One spawn attempt failed (missing PATH, workspace unmountable, …). Counter increments; task returns to `ready` for retry. |
|
||||
| `protocol_violation` | `{pid, claimer, exit_code, protocol_violation, worker_output?}` | Worker exited successfully while the task was still `running`, usually because it answered without a terminal board call (`kanban_complete`, `kanban_request_review` or `kanban_block`). Emitted on every violation (the payload's `protocol_violation: true` marker is copied into the run metadata and feeds the violation-only retry budget). Below the budget — up to `_PROTOCOL_VIOLATION_FAILURE_LIMIT` (default 3) *consecutive* violations, per-task `max_retries` overriding — the task simply returns to `ready` for another attempt; when the streak reaches the bound the dispatcher also emits `gave_up` and auto-blocks. `worker_output` carries the worker's own last printed text (usually its explanation of why it stopped), also folded into `last_failure_error` and shown to the retry worker as the prior-attempt error. |
|
||||
| `gave_up` | `{failures, effective_limit, limit_source, error}` | Circuit breaker fired after N consecutive non-successful attempts. Task auto-blocks with the last error. The effective limit resolves as task `max_retries`, then dispatcher `failure_limit` / `kanban.failure_limit`, then the built-in default. |
|
||||
| `gave_up` | `{failures, effective_limit, limit_source, error, terminal_provider?}` | Circuit breaker fired after N consecutive non-successful attempts. Task auto-blocks with the last error. The effective limit resolves as task `max_retries`, then dispatcher `failure_limit` / `kanban.failure_limit`, then the built-in default. `terminal_provider: true` means the worker exited `78` on a provider error a retry cannot fix (credential revoked, model gone) and the breaker fired on that first attempt, sticky, regardless of the limit. |
|
||||
|
||||
`hermes kanban tail <id>` shows these for a single task. `hermes kanban watch` streams them board-wide.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user