fix(kanban): don't permanently block tasks that hit a provider rate limit (#38223)
A kanban worker that exhausted its retries purely on a provider rate limit / quota wall (e.g. opencode-go's 5-hour window) exited with code 1. The dispatcher counted that as a crash, and with DEFAULT_FAILURE_LIMIT=2 two quota-wall hits permanently blocked the card. Fanning out many workers against one shared quota made this routine. Now a rate-limited worker exits with EX_TEMPFAIL (75); the dispatcher classifies that as a 'rate_limited' exit, releases the task back to 'ready' WITHOUT incrementing consecutive_failures (the breaker can't trip on a transient throttle), and the respawn guard defers the next attempt on a cooldown (default 5min, HERMES_KANBAN_RATE_LIMIT_COOLDOWN_SECONDS) until the quota window clears. Genuine crashes still count and trip the breaker as before. The 120s Retry-After cap is unchanged — no worker parks for hours holding a slot. - conversation_loop.py: surface failure_reason in the exhaustion return - cli.py: kanban worker picks exit 75 on rate_limit/billing failure - kanban_db.py: rate_limited exit kind, no-count requeue, cooldown guard
This commit is contained in:
@@ -679,6 +679,207 @@ def test_resolve_crash_grace_seconds_handles_bad_env(monkeypatch):
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Rate-limit requeue: a worker that bails on a provider quota wall must be
|
||||
# released back to ``ready`` WITHOUT counting a failure, so a long (e.g.
|
||||
# 5-hour) quota window can't trip the circuit breaker and permanently block
|
||||
# the card. The respawn guard then defers it on a cooldown until quota
|
||||
# returns. Regression coverage for the kanban-rate-limit-failure report.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _exited_status(code: int) -> int:
|
||||
"""Raw wait-status for a WIFEXITED child with the given exit code."""
|
||||
return code << 8
|
||||
|
||||
|
||||
def test_classify_worker_exit_recognizes_rate_limit_sentinel(kanban_home):
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
pid = 31337
|
||||
_kb._record_worker_exit(pid, _exited_status(_kb.KANBAN_RATE_LIMIT_EXIT_CODE))
|
||||
kind, code = _kb._classify_worker_exit(pid)
|
||||
assert kind == "rate_limited"
|
||||
assert code == _kb.KANBAN_RATE_LIMIT_EXIT_CODE
|
||||
|
||||
# Plain non-zero exit is still a normal crash, not rate-limited.
|
||||
_kb._record_worker_exit(pid + 1, _exited_status(1))
|
||||
assert _kb._classify_worker_exit(pid + 1) == ("nonzero_exit", 1)
|
||||
|
||||
|
||||
def test_rate_limit_exit_requeues_without_counting_failure(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
"""A rate-limit sentinel exit releases the task to ``ready`` and leaves
|
||||
``consecutive_failures`` untouched — the breaker must never trip on a
|
||||
transient throttle, even across many quota-wall hits."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: False)
|
||||
monkeypatch.setenv("HERMES_KANBAN_CRASH_GRACE_SECONDS", "0")
|
||||
|
||||
with kb.connect() as conn:
|
||||
host = _kb._claimer_id().split(":", 1)[0]
|
||||
tid = kb.create_task(conn, title="rl", assignee="a")
|
||||
|
||||
# Simulate FAR more quota-wall hits than DEFAULT_FAILURE_LIMIT (2).
|
||||
# If any of these counted as a failure the task would be blocked.
|
||||
for i in range(6):
|
||||
pid = 70000 + i
|
||||
# Claim to open a real run (so detect_crashed_workers can close
|
||||
# it with a rate_limited outcome), then point the claim at this
|
||||
# host + a dead pid so the crash path acts on it.
|
||||
kb.claim_task(conn, tid, claimer=f"{host}:w{i}")
|
||||
conn.execute(
|
||||
"UPDATE tasks SET worker_pid=?, consecutive_failures=? "
|
||||
"WHERE id=?",
|
||||
(pid, 0, tid),
|
||||
)
|
||||
conn.commit()
|
||||
_kb._record_worker_exit(
|
||||
pid, _exited_status(_kb.KANBAN_RATE_LIMIT_EXIT_CODE)
|
||||
)
|
||||
|
||||
crashed = kb.detect_crashed_workers(conn)
|
||||
# Rate-limited requeues are NOT crashes.
|
||||
assert tid not in crashed
|
||||
rl = getattr(_kb.detect_crashed_workers, "_last_rate_limited", [])
|
||||
assert tid in rl
|
||||
|
||||
task = kb.get_task(conn, tid)
|
||||
assert task.status == "ready", (
|
||||
f"hit {i}: should requeue ready, got {task.status}"
|
||||
)
|
||||
assert task.consecutive_failures == 0, (
|
||||
f"hit {i}: rate-limit must not count a failure, "
|
||||
f"got {task.consecutive_failures}"
|
||||
)
|
||||
|
||||
# Last failure error stamped so the respawn guard recognizes the
|
||||
# quota wall.
|
||||
assert task.last_failure_error and "rate-limited" in task.last_failure_error
|
||||
|
||||
# A ``rate_limited`` run outcome was recorded (not ``crashed``).
|
||||
outcomes = [
|
||||
r["outcome"] for r in conn.execute(
|
||||
"SELECT outcome FROM task_runs WHERE task_id=?", (tid,),
|
||||
).fetchall()
|
||||
]
|
||||
assert "rate_limited" in outcomes
|
||||
assert "crashed" not in outcomes
|
||||
|
||||
|
||||
def test_real_crash_still_counts_and_trips_breaker(kanban_home, monkeypatch):
|
||||
"""Sanity: a genuine non-zero crash (not the sentinel) still increments
|
||||
the failure counter and trips the breaker — the rate-limit carve-out is
|
||||
surgical, not a blanket "never count crashes"."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
monkeypatch.setattr(_kb, "_pid_alive", lambda _pid: False)
|
||||
|
||||
with kb.connect() as conn:
|
||||
host = _kb._claimer_id().split(":", 1)[0]
|
||||
tid = kb.create_task(conn, title="crash", assignee="a")
|
||||
|
||||
for i in range(2): # DEFAULT_FAILURE_LIMIT == 2
|
||||
pid = 60000 + i
|
||||
conn.execute(
|
||||
"UPDATE tasks SET status='running', worker_pid=?, "
|
||||
"claim_lock=? WHERE id=?",
|
||||
(pid, f"{host}:w{i}", tid),
|
||||
)
|
||||
conn.commit()
|
||||
_kb._record_worker_exit(pid, _exited_status(1)) # generic failure
|
||||
kb.detect_crashed_workers(conn)
|
||||
|
||||
task = kb.get_task(conn, tid)
|
||||
assert task.status == "blocked", (
|
||||
f"genuine crashes should still trip the breaker, got {task.status}"
|
||||
)
|
||||
|
||||
|
||||
def test_respawn_guard_defers_rate_limited_within_cooldown(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
"""Within the cooldown after a rate-limit requeue, the guard defers the
|
||||
respawn; after the cooldown it allows a probe — and crucially does NOT
|
||||
fall into ``blocker_auth`` (which would defer forever)."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
monkeypatch.setenv("HERMES_KANBAN_RATE_LIMIT_COOLDOWN_SECONDS", "300")
|
||||
now = 5_000_000
|
||||
|
||||
with kb.connect() as conn:
|
||||
tid = kb.create_task(conn, title="rl-guard", assignee="a")
|
||||
# Seed a rate_limited run that just ended + the stamped error.
|
||||
kb.claim_task(conn, tid)
|
||||
run_id = kb.get_task(conn, tid).current_run_id
|
||||
conn.execute(
|
||||
"UPDATE task_runs SET outcome='rate_limited', status='rate_limited', "
|
||||
"ended_at=? WHERE id=?",
|
||||
(now, run_id),
|
||||
)
|
||||
conn.execute(
|
||||
"UPDATE tasks SET status='ready', current_run_id=NULL, "
|
||||
"claim_lock=NULL, claim_expires=NULL, worker_pid=NULL, "
|
||||
"last_failure_error=? WHERE id=?",
|
||||
("pid 1 exited rate-limited (quota wall) — requeued", tid),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
# Inside cooldown → defer with the rate-limit-specific reason.
|
||||
monkeypatch.setattr(_kb.time, "time", lambda: now + 100)
|
||||
assert kb.check_respawn_guard(conn, tid) == "rate_limit_cooldown"
|
||||
|
||||
# Past cooldown → allowed (None), NOT trapped by blocker_auth even
|
||||
# though last_failure_error contains "rate-limited".
|
||||
monkeypatch.setattr(_kb.time, "time", lambda: now + 400)
|
||||
assert kb.check_respawn_guard(conn, tid) is None
|
||||
|
||||
|
||||
def test_respawn_guard_rate_limit_cooldown_zero_allows_immediately(
|
||||
kanban_home, monkeypatch,
|
||||
):
|
||||
"""Cooldown of 0 disables the wait — task is spawnable on the next tick,
|
||||
and the stamped rate-limit text does not re-trap it via blocker_auth."""
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
monkeypatch.setenv("HERMES_KANBAN_RATE_LIMIT_COOLDOWN_SECONDS", "0")
|
||||
now = 6_000_000
|
||||
|
||||
with kb.connect() as conn:
|
||||
tid = kb.create_task(conn, title="rl-zero", assignee="a")
|
||||
kb.claim_task(conn, tid)
|
||||
run_id = kb.get_task(conn, tid).current_run_id
|
||||
conn.execute(
|
||||
"UPDATE task_runs SET outcome='rate_limited', status='rate_limited', "
|
||||
"ended_at=? WHERE id=?",
|
||||
(now, run_id),
|
||||
)
|
||||
conn.execute(
|
||||
"UPDATE tasks SET status='ready', current_run_id=NULL, "
|
||||
"claim_lock=NULL, last_failure_error=? WHERE id=?",
|
||||
("pid 1 exited rate-limited (quota wall)", tid),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
monkeypatch.setattr(_kb.time, "time", lambda: now + 1)
|
||||
assert kb.check_respawn_guard(conn, tid) is None
|
||||
|
||||
|
||||
def test_resolve_rate_limit_cooldown_handles_bad_env(monkeypatch):
|
||||
import hermes_cli.kanban_db as _kb
|
||||
|
||||
for bad_val in ("notanumber", "-5", ""):
|
||||
monkeypatch.setenv(
|
||||
"HERMES_KANBAN_RATE_LIMIT_COOLDOWN_SECONDS", bad_val
|
||||
)
|
||||
assert (
|
||||
_kb._resolve_rate_limit_cooldown_seconds()
|
||||
== _kb.DEFAULT_RATE_LIMIT_COOLDOWN_SECONDS
|
||||
)
|
||||
|
||||
|
||||
def test_max_runtime_uses_current_run_start_after_retry(kanban_home, monkeypatch):
|
||||
"""A retry should get a fresh max-runtime window.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user