diff --git a/cron/AGENTS.md b/cron/AGENTS.md index ec38f3dcdb..cddd8e5fc6 100644 --- a/cron/AGENTS.md +++ b/cron/AGENTS.md @@ -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 diff --git a/hermes_cli/kanban_db_dispatch.py b/hermes_cli/kanban_db_dispatch.py index 5f30da9ef0..1bd402d613 100644 --- a/hermes_cli/kanban_db_dispatch.py +++ b/hermes_cli/kanban_db_dispatch.py @@ -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} diff --git a/hermes_cli/kanban_diagnostics.py b/hermes_cli/kanban_diagnostics.py index 9e255ab9f1..de123d9c7a 100644 --- a/hermes_cli/kanban_diagnostics.py +++ b/hermes_cli/kanban_diagnostics.py @@ -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 " diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index 4dd3739162..f344984dbe 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -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( diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 99fd8c8c03..5fc2d4c3af 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -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 +auth` / `setup`), then `hermes kanban unblock `. The worker also writes its exit code as the last line of its own log (`[kanban-worker-exit] rc=`), 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 ` shows these for a single task. `hermes kanban watch` streams them board-wide.