diff --git a/tests/tools/test_bot_failure_reasons.py b/tests/tools/test_bot_failure_reasons.py new file mode 100644 index 0000000000..096b94ae33 --- /dev/null +++ b/tests/tools/test_bot_failure_reasons.py @@ -0,0 +1,105 @@ +"""Tests: typed failure-reason codes (tools/bot_failure_reasons.py, #93091). + +Pins the closed reason vocabulary, the ordered classifier (incl. the +auth-beats-quota precedence seen in real Anthropic 401 bodies), the three +real-world fixtures from live bot runs, and the auto-retryable set. +""" + +import pytest + +from tools import bot_failure_reasons as fr + +# Real error text captured from live bot turns. +FIXTURE_ANTHROPIC_401 = ( + "Error code: 401 - {'type': 'error', 'error': {'type': 'authentication_error', " + "'message': 'Your API key is invalid, blocked or out of funds...'}}" +) +FIXTURE_NO_PROVIDER = ( + "agent init failed: No LLM provider configured. Run `hermes model` to select " + "a provider, or run `hermes setup` for first-time configuration." +) +FIXTURE_NO_TOKEN = "agent init failed: No access token found for Nous Portal login." + + +def test_closed_vocabulary_contains_every_code(): + assert fr.ALL_REASONS == { + "runtime_offline", + "queued_expired", + "delivery_timeout", + "agent_blocked", + "cancelled", + "provider_auth_or_access", + "provider_quota_limit", + "provider_rate_limit", + "provider_server_error", + "context_overflow", + "missing_config", + "model_unavailable", + "unknown", + } + # constants match their string values + assert fr.RUNTIME_OFFLINE == "runtime_offline" + assert fr.PROVIDER_AUTH_OR_ACCESS == "provider_auth_or_access" + assert fr.UNKNOWN == "unknown" + + +@pytest.mark.parametrize( + ("text", "code"), + [ + ("Error code: 403 - forbidden", fr.PROVIDER_AUTH_OR_ACCESS), + ("Invalid API key provided", fr.PROVIDER_AUTH_OR_ACCESS), + ("Error code: 402 - payment required", fr.PROVIDER_QUOTA_LIMIT), + ("insufficient balance, top up your account", fr.PROVIDER_QUOTA_LIMIT), + ("You exceeded your current quota", fr.PROVIDER_QUOTA_LIMIT), + ("Error code: 429 - Too Many Requests", fr.PROVIDER_RATE_LIMIT), + ("Rate limit reached for gpt-4o", fr.PROVIDER_RATE_LIMIT), + ("Error code: 500 - internal server error", fr.PROVIDER_SERVER_ERROR), + ("Error code: 529 - overloaded_error: Overloaded", fr.PROVIDER_SERVER_ERROR), + ("This model's maximum context length is 128000 tokens", fr.CONTEXT_OVERFLOW), + ("context_overflow: prompt too large", fr.CONTEXT_OVERFLOW), + ("missing config: no provider block in config.yaml", fr.MISSING_CONFIG), + ("model 'gpt-9' not found", fr.MODEL_UNAVAILABLE), + ("The model `foo-bar` does not exist", fr.MODEL_UNAVAILABLE), + ("something inexplicable happened", fr.UNKNOWN), + ("", fr.UNKNOWN), + (None, fr.UNKNOWN), + ], +) +def test_classify_agent_error_rules(text, code): + assert fr.classify_agent_error(text) == code + + +def test_fixture_anthropic_401_auth_beats_quota(): + # The live 401 body ALSO says "out of funds" — auth wins by precedence. + assert "out of funds" in FIXTURE_ANTHROPIC_401 + assert fr.classify_agent_error(FIXTURE_ANTHROPIC_401) == fr.PROVIDER_AUTH_OR_ACCESS + + +def test_precedence_authentication_error_type_alone_beats_quota_words(): + text = "authentication_error: account is out of funds" + assert fr.classify_agent_error(text) == fr.PROVIDER_AUTH_OR_ACCESS + # but plain quota text without any auth marker classifies as quota + assert fr.classify_agent_error("account is out of funds") == fr.PROVIDER_QUOTA_LIMIT + + +def test_fixture_no_provider_configured_is_missing_config(): + assert fr.classify_agent_error(FIXTURE_NO_PROVIDER) == fr.MISSING_CONFIG + + +def test_fixture_no_access_token_is_missing_config(): + assert fr.classify_agent_error(FIXTURE_NO_TOKEN) == fr.MISSING_CONFIG + + +def test_auto_retryable_set_and_predicate(): + assert fr.AUTO_RETRYABLE == { + fr.RUNTIME_OFFLINE, + fr.DELIVERY_TIMEOUT, + fr.PROVIDER_RATE_LIMIT, + fr.PROVIDER_SERVER_ERROR, + } + for code in fr.AUTO_RETRYABLE: + assert fr.is_auto_retryable(code) + for code in fr.ALL_REASONS - fr.AUTO_RETRYABLE: + assert not fr.is_auto_retryable(code) + assert not fr.is_auto_retryable("") + assert not fr.is_auto_retryable("nonsense") diff --git a/tests/tools/test_bot_relay.py b/tests/tools/test_bot_relay.py index d87861fcd6..28b334e48b 100644 --- a/tests/tools/test_bot_relay.py +++ b/tests/tools/test_bot_relay.py @@ -129,6 +129,21 @@ def test_write_reply_validates_envelope_id(root): assert data["reply"] == "pong" and not data["error"] +def test_write_reply_reason_passthrough_and_classification(root): + # explicit reason is persisted verbatim + path = bot_relay.write_reply(root, "c" * 32, error="boom", reason="delivery_timeout") + data = json.loads(Path(path).read_text(encoding="utf-8")) + assert data["reason"] == "delivery_timeout" and data["error"] == "boom" + # no reason given → classified from error text + path = bot_relay.write_reply(root, "d" * 32, error="Error code: 429 - rate limit") + data = json.loads(Path(path).read_text(encoding="utf-8")) + assert data["reason"] == "provider_rate_limit" + # success reply carries an empty reason + path = bot_relay.write_reply(root, "e" * 32, reply="ok") + data = json.loads(Path(path).read_text(encoding="utf-8")) + assert data["reason"] == "" and data["reply"] == "ok" + + def test_waiter_command_quotes_and_targets_reply_file(root): env = {"id": "b" * 32, "target_handle": "researcher", "target_connection": "ssh-vps"} cmd = bot_relay.waiter_command(root, env) diff --git a/tools/bot_failure_reasons.py b/tools/bot_failure_reasons.py new file mode 100644 index 0000000000..c278a9d880 --- /dev/null +++ b/tools/bot_failure_reasons.py @@ -0,0 +1,127 @@ +"""Typed failure-reason codes for bot turns and relay replies (#93091). + +A closed vocabulary of machine-readable reason codes carried ALONGSIDE the +existing free-text ``error`` fields (additive schema — old consumers keep +working). Platform-side codes are assigned by the transport/relay layer; +agent-side codes are derived from raw agent/provider error text via +``classify_agent_error``. + +Classifier precedence (deterministic, documented, tested): + 1. auth — an explicit ``authentication_error`` type, a 401/403 status, + or "invalid api key" wins over everything else. Rationale: real + provider 401 bodies (e.g. Anthropic) say "invalid, blocked or out of + funds" — quota words inside an auth error must not misclassify it. + 2. quota — 402 / out of funds / quota / balance. + 3. rate — 429 / rate limit. + 4. server — 5xx / server error / overloaded. + 5. context — context length / context_overflow / maximum context. + 6. config — No LLM provider configured / missing config / No access token. + 7. model — model not found / does not exist. + 8. unknown — anything else (including empty text). +""" + +from __future__ import annotations + +import re + +# ── platform-side reason codes ─────────────────────────────────────────────── +RUNTIME_OFFLINE = "runtime_offline" +QUEUED_EXPIRED = "queued_expired" +DELIVERY_TIMEOUT = "delivery_timeout" +AGENT_BLOCKED = "agent_blocked" +CANCELLED = "cancelled" + +# ── agent-side reason codes ────────────────────────────────────────────────── +PROVIDER_AUTH_OR_ACCESS = "provider_auth_or_access" +PROVIDER_QUOTA_LIMIT = "provider_quota_limit" +PROVIDER_RATE_LIMIT = "provider_rate_limit" +PROVIDER_SERVER_ERROR = "provider_server_error" +CONTEXT_OVERFLOW = "context_overflow" +MISSING_CONFIG = "missing_config" +MODEL_UNAVAILABLE = "model_unavailable" +UNKNOWN = "unknown" + +ALL_REASONS = frozenset( + { + RUNTIME_OFFLINE, + QUEUED_EXPIRED, + DELIVERY_TIMEOUT, + AGENT_BLOCKED, + CANCELLED, + PROVIDER_AUTH_OR_ACCESS, + PROVIDER_QUOTA_LIMIT, + PROVIDER_RATE_LIMIT, + PROVIDER_SERVER_ERROR, + CONTEXT_OVERFLOW, + MISSING_CONFIG, + MODEL_UNAVAILABLE, + UNKNOWN, + } +) + +#: Reasons a supervisor may retry automatically without human intervention. +AUTO_RETRYABLE = frozenset( + {RUNTIME_OFFLINE, DELIVERY_TIMEOUT, PROVIDER_RATE_LIMIT, PROVIDER_SERVER_ERROR} +) + + +def is_auto_retryable(reason: str) -> bool: + """True when ``reason`` is safe to retry automatically.""" + return reason in AUTO_RETRYABLE + + +# Ordered (pattern, code) rules — first match wins. See module docstring for +# the precedence rationale (auth beats quota by design). +_RULES: tuple[tuple[re.Pattern[str], str], ...] = ( + ( + re.compile( + r"authentication_error|invalid api key" + r"|(?:error code:?\s*|status(?:\s*code)?:?\s*|\b)(?:401|403)\b", + re.IGNORECASE, + ), + PROVIDER_AUTH_OR_ACCESS, + ), + ( + re.compile(r"\b402\b|out of funds|quota|balance", re.IGNORECASE), + PROVIDER_QUOTA_LIMIT, + ), + ( + re.compile(r"\b429\b|rate.?limit", re.IGNORECASE), + PROVIDER_RATE_LIMIT, + ), + ( + re.compile(r"\b5\d{2}\b|server error|overloaded", re.IGNORECASE), + PROVIDER_SERVER_ERROR, + ), + ( + re.compile(r"context length|context_overflow|maximum context", re.IGNORECASE), + CONTEXT_OVERFLOW, + ), + ( + re.compile( + r"no llm provider configured|missing config|no access token", + re.IGNORECASE, + ), + MISSING_CONFIG, + ), + ( + re.compile(r"model .*(not found|does not exist)|model_not_found", re.IGNORECASE), + MODEL_UNAVAILABLE, + ), +) + + +def classify_agent_error(text: str) -> str: + """Map raw agent/provider error text to a closed reason code. + + First matching rule in ``_RULES`` wins; anything unmatched (or empty) + is ``unknown``. Auth intentionally outranks quota: a 401 body that also + mentions "out of funds" is still an auth/access failure. + """ + raw = str(text or "") + if not raw.strip(): + return UNKNOWN + for pattern, code in _RULES: + if pattern.search(raw): + return code + return UNKNOWN diff --git a/tools/bot_mode_dm.py b/tools/bot_mode_dm.py index 2eda26eadb..03819dcc49 100644 --- a/tools/bot_mode_dm.py +++ b/tools/bot_mode_dm.py @@ -227,7 +227,9 @@ def _resolve_local_name(target: str, roster: list[str]) -> Optional[str]: def _err(message: str, *, roster: list[str] | None = None, peers: list[str] | None = None) -> str: - payload: dict[str, Any] = {"error": message} + from tools.bot_failure_reasons import classify_agent_error + + payload: dict[str, Any] = {"error": message, "reason": classify_agent_error(message)} if roster is not None: payload["teammates"] = roster if peers is not None: diff --git a/tools/bot_relay.py b/tools/bot_relay.py index 6913bd2a8a..444df02186 100644 --- a/tools/bot_relay.py +++ b/tools/bot_relay.py @@ -249,19 +249,31 @@ def claim_pending_envelopes(root: Path | str) -> list[dict]: def write_reply( - root: Path | str, envelope_id: str, *, reply: str = "", error: str = "" + root: Path | str, envelope_id: str, *, reply: str = "", error: str = "", reason: str = "" ) -> Path: - """Persist the relayed reply (or delivery error) for the waiter.""" + """Persist the relayed reply (or delivery error) for the waiter. + + ``reason`` is an optional typed failure code (see + ``tools.bot_failure_reasons``); when omitted and ``error`` is non-empty + it is classified from the error text. + """ base = _ensure_dirs(root) safe = str(envelope_id or "").strip() if not re.match(r"^[0-9a-f]{32}$", safe): raise ValueError(f"invalid envelope id: {envelope_id!r}") + err = str(error or "") + code = str(reason or "") + if not code and err: + from tools.bot_failure_reasons import classify_agent_error + + code = classify_agent_error(err) path = base / REPLIES_DIR / f"{safe}.json" payload = { "id": safe, "at": int(time.time()), "reply": str(reply or ""), - "error": str(error or ""), + "error": err, + "reason": code, } fd, tmp = tempfile.mkstemp(dir=str(base / REPLIES_DIR), prefix=".rep-", suffix=".tmp") with os.fdopen(fd, "w", encoding="utf-8") as f: diff --git a/tui_gateway/methods_bot_relay.py b/tui_gateway/methods_bot_relay.py index 9570235077..ae266c6731 100644 --- a/tui_gateway/methods_bot_relay.py +++ b/tui_gateway/methods_bot_relay.py @@ -138,7 +138,8 @@ def _(rid, params: dict) -> dict: def _(rid, params: dict) -> dict: """Write a relayed reply (or delivery error) for a sender-side waiter. - Params: ``id`` (envelope id), ``reply`` and/or ``error``. + Params: ``id`` (envelope id), ``reply`` and/or ``error``, optional + ``reason`` (typed failure code, see ``tools.bot_failure_reasons``). """ envelope_id = str(params.get("id") or "").strip() if not envelope_id: @@ -156,6 +157,7 @@ def _(rid, params: dict) -> dict: envelope_id, reply=str(params.get("reply") or ""), error=str(params.get("error") or ""), + reason=str(params.get("reason") or ""), ) return _ok(rid, {"ok": True}) except ValueError as e: