diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 2991ffaa4e5f..dc250c21139c 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -930,6 +930,7 @@ def recover_with_credential_pool( has_retried_429: bool, classified_reason: Optional[FailoverReason] = None, error_context: Optional[Dict[str, Any]] = None, + billing_unverified: bool = False, ) -> tuple[bool, bool]: """Attempt credential recovery via pool rotation. @@ -944,6 +945,12 @@ def recover_with_credential_pool( providers that surface billing/rate-limit/auth conditions under a different status code, such as Anthropic returning HTTP 400 for "out of extra usage". + + `billing_unverified` marks a billing verdict that rests on an ambiguous + body (``ClassifiedError.billing_unverified``, #82154): the pool persists + it as ``billing_unverified`` so the exhausted entry gets a short cooldown + instead of the one-hour billing bench — the same 400 can be a + content-filter rejection that leaves the credential healthy. """ pool = agent._credential_pool if pool is None: @@ -1036,7 +1043,13 @@ def _rotate_failed_credential(rotate_status: int): # cooldowns — the pool can only tell them apart if we say which. # ``effective_reason`` is resolved below; this closure runs after. if effective_reason is not None: - kwargs["failure_reason"] = effective_reason.value + _failure_reason = effective_reason.value + if effective_reason == FailoverReason.billing and billing_unverified: + # Ambiguous billing body (#82154): persist the ambiguity so + # the cooldown is sized as transient, not a 1-hour bench. + from agent.credential_pool import FAILURE_REASON_BILLING_UNVERIFIED + _failure_reason = FAILURE_REASON_BILLING_UNVERIFIED + kwargs["failure_reason"] = _failure_reason return pool.mark_exhausted_and_rotate(**kwargs) effective_reason = classified_reason diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index c05167d7685d..0d441e9308d4 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -512,6 +512,7 @@ def _billing_or_entitlement_message( provider: str, base_url: str, model: str, + unverified: bool = False, ) -> str: if _is_nous_inference_route(provider, base_url): return _nous_entitlement_message(capability) @@ -526,16 +527,45 @@ def _billing_or_entitlement_message( # apply to a subscription — the user waits for the reset or switches to an # API key. if (provider or "").strip().lower() == "anthropic": - lines = [ - ( - f"{provider_label} reported that your Claude subscription usage is " - f"exhausted for {model_label} (included quota + extra-usage credits)." - ), - "Options: wait for the billing cycle to reset, or add extra usage at " - "https://claude.ai/settings/usage", - "You can also switch to an Anthropic API key or another provider with " - "/model --provider .", - ] + # ``unverified`` (ClassifiedError.billing_unverified, #82154): the + # "out of extra usage" 400 is ambiguous — Anthropic returns the same + # body when its server-side content filter rejects part of the request + # on a subscription OAuth token, so the message reliably misdirects + # diagnosis toward buying quota. Hedge the claim and name the other + # cause. A confirmed verdict (e.g. a real 402 or an API-key credit + # depletion) keeps the assertive wording. + if unverified: + lines = [ + ( + f"{provider_label} reported that your Claude subscription usage may be " + f"exhausted for {model_label} (included quota + extra-usage credits) — " + "but this specific error is not proof of a billing problem." + ), + "If https://claude.ai/settings/usage still shows quota remaining, this is " + "probably NOT a billing problem: on a Claude subscription (OAuth) token " + "Anthropic returns this same message when its content filter rejects part " + "of the request — typically a phrase in the system prompt.", + "If usage really is exhausted: wait for the billing cycle to reset, or add " + "extra usage at https://claude.ai/settings/usage", + "You can also switch to an Anthropic API key or another provider with " + "/model --provider .", + # The exhaustion latch replays the stored error without issuing + # a request, so a real fix looks like it didn't work. + "Retry with a fresh credential state: `hermes auth reset anthropic`. Until " + "that cooldown clears, this error can be replayed from cache without " + "contacting the API.", + ] + else: + lines = [ + ( + f"{provider_label} reported that your Claude subscription usage is " + f"exhausted for {model_label} (included quota + extra-usage credits)." + ), + "Options: wait for the billing cycle to reset, or add extra usage at " + "https://claude.ai/settings/usage", + "You can also switch to an Anthropic API key or another provider with " + "/model --provider .", + ] return "\n".join(lines) # Provider-agnostic billing URL derivation (OpenAI, DeepSeek, xAI, Groq, @@ -564,16 +594,84 @@ def _billing_or_entitlement_message( return "\n".join(lines) -def _billing_block_dict(provider, base_url, model, message="") -> Optional[dict]: +def _billing_block_dict( + provider, base_url, model, message="", *, unverified: bool = False +) -> Optional[dict]: """Best-effort structured billing descriptor (None if billing_links is unavailable).""" try: from agent.billing_links import build_billing_block - return build_billing_block( + block = build_billing_block( provider=provider, base_url=str(base_url), model=model, message=message ).to_dict() except Exception: return None + if block is not None and unverified: + # Carry the classifier's ambiguity into the structured descriptor so + # every surface rendering the block can hedge too (#82154). + block["unverified"] = True + return block + + +def _billing_terminal_label(summary: str, unverified: bool) -> str: + """Terminal-failure prefix for a billing-classified error. + + ``unverified`` (#82154): the Anthropic "out of extra usage" 400 can be a + content-filter rejection, so the terminal line must not assert billing + exhaustion as fact. + """ + if unverified: + return ( + "Provider reported usage/credit exhaustion (unverified — the same " + f"error can be a content-filter rejection, not billing): {summary}" + ) + return f"Billing or credits exhausted: {summary}" + + +def _billing_failure_result( + *, + classified, + summary: str, + messages, + api_call_count: int, + provider: str, + base_url, + model: str, + guidance: Optional[str] = None, +) -> dict: + """Structured terminal result for a billing-classified failure. + + Single construction point for the returned terminal response so the + label, guidance, structured block, and ambiguity flag stay consistent + across the non-retryable abort and max-retries paths (#82154). + """ + unverified = bool(getattr(classified, "billing_unverified", False)) + if guidance is None: + guidance = _billing_or_entitlement_message( + capability="model access", + provider=provider, + base_url=str(base_url), + model=model, + unverified=unverified, + ) + final = _billing_terminal_label(summary, unverified) + if guidance: + final += f"\n\n{guidance}" + return { + "final_response": final, + "messages": messages, + "api_calls": api_call_count, + "completed": False, + "failed": True, + "error": summary, + "failure_reason": classified.reason.value, + # The billing verdict may rest on an ambiguous body (#82154) — carry + # that through the structured result, not just the prose. + "billing_unverified": unverified, + "billing_block": _billing_block_dict( + provider, base_url, model, guidance, unverified=unverified + ), + } def _print_billing_or_entitlement_guidance( @@ -583,12 +681,14 @@ def _print_billing_or_entitlement_guidance( provider: str, base_url: str, model: str, + unverified: bool = False, ) -> bool: message = _billing_or_entitlement_message( capability=capability, provider=provider, base_url=base_url, model=model, + unverified=unverified, ) if not message: return False @@ -4334,6 +4434,7 @@ def _perform_api_call(next_api_kwargs): has_retried_429=_retry.has_retried_429, classified_reason=classified.reason, error_context=error_context, + billing_unverified=classified.billing_unverified, ) if recovered_with_pool: continue @@ -4978,9 +5079,17 @@ def _perform_api_call(next_api_kwargs): "switching to fallback model..." ) elif classified.reason == FailoverReason.billing: - agent._buffer_status( - "⚠️ Billing or credits exhausted — switching to fallback provider..." - ) + if classified.billing_unverified: + # Ambiguous body (#82154) — don't assert billing. + agent._buffer_status( + "⚠️ Provider reported usage/credit exhaustion " + "(unverified — may be a content-filter rejection) " + "— switching to fallback provider..." + ) + else: + agent._buffer_status( + "⚠️ Billing or credits exhausted — switching to fallback provider..." + ) elif _is_transport_failure: agent._buffer_status( "⚠️ Provider unreachable — switching to fallback provider..." @@ -5677,6 +5786,7 @@ def _perform_api_call(next_api_kwargs): provider=_provider, base_url=str(_base), model=_model, + unverified=classified.billing_unverified, ): pass elif _provider == "nous" and _print_nous_entitlement_guidance( @@ -5803,26 +5913,15 @@ def _perform_api_call(next_api_kwargs): # the max-retries path so every surface (CLI, TUI, desktop) # renders one consistent billing signal. if classified.reason == FailoverReason.billing: - _ce_guidance = _billing_or_entitlement_message( - capability="model access", + return _billing_failure_result( + classified=classified, + summary=_nonretryable_summary, + messages=messages, + api_call_count=api_call_count, provider=_provider, - base_url=str(_base), + base_url=_base, model=_model, ) - _ce_final = f"Billing or credits exhausted: {_nonretryable_summary}" - if _ce_guidance: - _ce_final += f"\n\n{_ce_guidance}" - _ce_block = _billing_block_dict(_provider, _base, _model, _ce_guidance) - return { - "final_response": _ce_final, - "messages": messages, - "api_calls": api_call_count, - "completed": False, - "failed": True, - "error": _nonretryable_summary, - "failure_reason": classified.reason.value, - "billing_block": _ce_block, - } return { "final_response": _nonretryable_summary, "messages": messages, @@ -5866,12 +5965,20 @@ def _perform_api_call(next_api_kwargs): _final_summary = agent._summarize_api_error(api_error) _billing_guidance = "" if classified.reason == FailoverReason.billing: - agent._emit_status(f"❌ Billing or credits exhausted — {_final_summary}") + if classified.billing_unverified: + # Ambiguous body (#82154) — hedge the terminal line. + agent._emit_status( + "❌ Provider reported usage/credit exhaustion " + f"(unverified — may be a content-filter rejection) — {_final_summary}" + ) + else: + agent._emit_status(f"❌ Billing or credits exhausted — {_final_summary}") _billing_guidance = _billing_or_entitlement_message( capability="model access", provider=_provider, base_url=str(_base), model=_model, + unverified=classified.billing_unverified, ) _print_billing_or_entitlement_guidance( agent, @@ -5879,6 +5986,7 @@ def _perform_api_call(next_api_kwargs): provider=_provider, base_url=str(_base), model=_model, + unverified=classified.billing_unverified, ) elif is_rate_limited: agent._emit_status(f"❌ Rate limited after {max_retries} retries — {_final_summary}") @@ -5985,13 +6093,20 @@ def _perform_api_call(next_api_kwargs): ) agent._persist_session(messages, conversation_history) _billing_block = None + _billing_unverified = False if classified.reason == FailoverReason.billing: - _final_response = f"Billing or credits exhausted: {_final_summary}" + _billing_unverified = classified.billing_unverified + _final_response = _billing_terminal_label( + _final_summary, _billing_unverified + ) if _billing_guidance: _final_response += f"\n\n{_billing_guidance}" # Structured recovery descriptor so every surface renders # the same link + label from one signal (see helper). - _billing_block = _billing_block_dict(_provider, _base, _model, _billing_guidance) + _billing_block = _billing_block_dict( + _provider, _base, _model, _billing_guidance, + unverified=_billing_unverified, + ) else: _final_response = f"API call failed after {max_retries} retries: {_final_summary}" if _is_thinking_timeout: @@ -6031,6 +6146,9 @@ def _perform_api_call(next_api_kwargs): # different exit code. ``rate_limit`` / ``billing`` here # mean "quota wall, not a task error". "failure_reason": classified.reason.value, + # True when the billing verdict rests on an ambiguous + # body (#82154) — may be a content-filter rejection. + "billing_unverified": _billing_unverified, # Present only for billing walls: structured recovery # descriptor (provider, billing_url, is_nous, message). "billing_block": _billing_block, diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 072140134e76..84c5b6834b96 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -136,6 +136,14 @@ def _load_config_safe() -> Optional[dict]: # the classifier), so the value is duplicated here rather than referenced. FAILURE_REASON_BILLING = "billing" +# Billing verdict that rests on an ambiguous body (#82154): Anthropic's +# "out of extra usage" 400 is returned both for genuine overage depletion and +# for a server-side content-filter rejection of the request. The latter leaves +# the credential perfectly healthy, so an unverified billing exhaustion gets +# the short transient cooldown instead of the one-hour billing bench — a +# genuine depletion simply re-latches on the next attempt. +FAILURE_REASON_BILLING_UNVERIFIED = "billing_unverified" + # Throttle window for the "no available entries" INFO line. Credential # selection runs on a hot path (every model call, plus auxiliary tasks like # compression/moa/titles), so when a pool is empty or fully exhausted the @@ -332,6 +340,15 @@ def _exhausted_ttl( if error_code == 401: return EXHAUSTED_TTL_401_SECONDS base = EXHAUSTED_TTL_429_SECONDS if error_code == 429 else EXHAUSTED_TTL_DEFAULT_SECONDS + # Unverified billing (#82154): the same 400 body can be a content-filter + # rejection of the request itself, in which case the credential is healthy + # and an hour-long bench just blocks it (and, for a sole credential, + # replays the stored error for the full hour — making a real fix look like + # it did not work). Short cooldown regardless of pool size; a genuine + # depletion re-latches on the next attempt. A true 402 stays a full bench + # even if something mislabeled it unverified. + if failure_reason == FAILURE_REASON_BILLING_UNVERIFIED and error_code != 402: + return min(base, EXHAUSTED_TTL_SOLE_CREDENTIAL_SECONDS) # Sole credential: shorten only TRANSIENT throttles (429 rate-limit, 403 # edge-throttle, 5xx server, or unknown). Billing exhaustion — whether # classified as such or self-evident from a 402 — is a genuine depletion diff --git a/agent/error_classifier.py b/agent/error_classifier.py index e8df941361ec..8198888813e2 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -102,6 +102,15 @@ class ClassifiedError: def is_auth(self) -> bool: return self.reason in {FailoverReason.auth, FailoverReason.auth_permanent} + @property + def billing_unverified(self) -> bool: + """True when a ``billing`` verdict rests on an ambiguous body. + + Anthropic's "out of extra usage" 400 can also be a content-filter + rejection (#82154); surfaces must hedge rather than assert exhaustion. + """ + return bool(self.error_context.get("billing_unverified")) + # ── Provider-specific patterns ────────────────────────────────────────── @@ -131,6 +140,25 @@ def is_auth(self) -> bool: "not available on the free tier", ] +# Billing-pattern matches that are NOT proof of billing exhaustion. Anthropic +# returns the identical "out of extra usage" body on a subscription OAuth +# token both when the overage bucket is genuinely depleted AND when its +# server-side content filter rejects part of the request (#82154) — the two +# are indistinguishable from the response. Classification stays ``billing`` +# (rotation + fallback remain the right recovery either way), but the +# ambiguity is carried in ``error_context`` so downstream surfaces hedge +# instead of asserting exhaustion as fact, and the credential pool applies a +# short cooldown instead of the one-hour billing bench (a content-filter +# rejection leaves the credential perfectly healthy). +_UNVERIFIED_BILLING_PATTERNS = ("out of extra usage",) + + +def _billing_ambiguity_context(error_msg: str) -> Dict[str, Any]: + """error_context marking a billing verdict as unverified (see above).""" + if any(p in error_msg for p in _UNVERIFIED_BILLING_PATTERNS): + return {"billing_unverified": True, "possible_content_filter": True} + return {} + # xAI's explicit Grok credit-exhaustion code. Keep the HTTP 403 special case # provider-scoped: other providers' generic billing codes historically remain # auth failures when they arrive as 403. @@ -1511,6 +1539,10 @@ def _classify_400( retryable=False, should_rotate_credential=True, should_fallback=True, + # "out of extra usage" on a 400 is ambiguous — it can also be a + # content-filter rejection (#82154). Mark the verdict unverified + # so downstream hedges and the pool skips the 1-hour bench. + error_context=_billing_ambiguity_context(error_msg), ) # Generic 400 + large session → probable context overflow @@ -1692,6 +1724,10 @@ def _classify_by_message( retryable=False, should_rotate_credential=True, should_fallback=True, + # Status-less path: adapters can strip the HTTP status from the + # Anthropic "out of extra usage" 400, so the same ambiguity + # marking applies here (#82154). + error_context=_billing_ambiguity_context(error_msg), ) # Rate limit patterns diff --git a/agent/prompt_builder.py b/agent/prompt_builder.py index a9e11c7abb71..ccfa217f4e45 100644 --- a/agent/prompt_builder.py +++ b/agent/prompt_builder.py @@ -197,10 +197,21 @@ def _strip_yaml_frontmatter(content: str) -> str: "asking them to repeat themselves." ) +# NOTE (#82154): the opening sentence is worded deliberately. Anthropic's +# server-side content filter rejects the previous phrasing ("After completing a +# complex task (5+ tool calls), fixing a tricky error, or discovering a +# non-trivial workflow, save the approach as a skill with skill_manage so you +# can reuse it next time.") on subscription OAuth credentials, and surfaces that +# rejection as a billing-shaped HTTP 400 ("You're out of extra usage"), which +# sends users to buy quota they do not need. Bisected against the live API: that +# sentence alone reproduces the 400 and removing it alone clears it; size and +# the system[0] identity gate were both ruled out. The reword is empirically +# validated, not understood — if you rewrite this sentence, re-verify against a +# subscription OAuth token, not an sk-ant-api… key, which does not hit the +# filter. SKILLS_GUIDANCE = ( - "After completing a complex task (5+ tool calls), fixing a tricky error, " - "or discovering a non-trivial workflow, save the approach as a " - "skill with skill_manage so you can reuse it next time.\n" + "When you work out a non-trivial workflow, record it with skill_manage " + "for future reuse.\n" "When using a skill and finding it outdated, incomplete, or wrong, " "patch it immediately with skill_manage(action='patch') — don't wait to be asked. " "Skills that aren't maintained become liabilities.\n" diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 3da395831f43..9f43d1ce3af5 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -395,6 +395,15 @@ class ProviderConfig: name="Anthropic", auth_type="api_key", inference_base_url="https://api.anthropic.com", + # CLAUDE_CODE_OAUTH_TOKEN is NOT an API key, despite auth_type="api_key" + # and its place in this tuple (#82154). `claude setup-token` yields an + # `sk-ant-oat01…` OAuth token: sent as `x-api-key` it 401s, and sent as a + # bare Bearer it 429s. It is listed here because this tuple doubles as the + # credential-DISCOVERY list (agent/credential_pool.py builds its env scan + # from it), so removing it would stop Hermes finding a setup-token + # credential at all. The adapter routes such a value down the OAuth path + # on the strength of its prefix, not on this entry. Only ANTHROPIC_API_KEY + # and ANTHROPIC_TOKEN are usable as literal API keys. api_key_env_vars=("ANTHROPIC_API_KEY", "ANTHROPIC_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"), base_url_env_var="ANTHROPIC_BASE_URL", ), diff --git a/run_agent.py b/run_agent.py index 6b1854e817f7..aa3d1bebf5ec 100644 --- a/run_agent.py +++ b/run_agent.py @@ -6129,10 +6129,11 @@ def _recover_with_credential_pool( has_retried_429: bool, classified_reason: Optional[FailoverReason] = None, error_context: Optional[Dict[str, Any]] = None, + billing_unverified: bool = False, ) -> tuple[bool, bool]: """Forwarder — see ``agent.agent_runtime_helpers.recover_with_credential_pool``.""" from agent.agent_runtime_helpers import recover_with_credential_pool - return recover_with_credential_pool(self, status_code=status_code, has_retried_429=has_retried_429, classified_reason=classified_reason, error_context=error_context) + return recover_with_credential_pool(self, status_code=status_code, has_retried_429=has_retried_429, classified_reason=classified_reason, error_context=error_context, billing_unverified=billing_unverified) def _credential_pool_may_recover_rate_limit(self) -> bool: """Whether a rate-limit retry should wait for same-provider credentials.""" diff --git a/tests/agent/test_anthropic_billing_guidance.py b/tests/agent/test_anthropic_billing_guidance.py index 142b8b04c2f9..3d49b4aed341 100644 --- a/tests/agent/test_anthropic_billing_guidance.py +++ b/tests/agent/test_anthropic_billing_guidance.py @@ -8,6 +8,11 @@ guidance ("add credits with that provider") is wrong for a subscription — the user waits for the cycle reset or switches to an API key. This branch gives Anthropic-specific, actionable guidance (folds in PR #40073's UX). + +#82154 adds the ``unverified`` axis: the same 400 body is also returned when +Anthropic's server-side content filter rejects part of the request, so an +unverified billing verdict must hedge and name the other cause, while a +confirmed verdict keeps the assertive wording. """ from __future__ import annotations @@ -44,3 +49,81 @@ def test_non_anthropic_billing_guidance_unaffected(): assert "claude.ai/settings/usage" not in msg # Generic path still surfaces the OpenRouter credits link. assert "openrouter.ai/settings/credits" in msg + + +# ── #82154: an UNVERIFIED billing 400 is not proof of a billing problem ────── +# Anthropic returns the same "out of extra usage" body when its server-side +# content filter rejects part of the request on a subscription OAuth token. +# Asserting exhaustion outright cost one reporter three debugging sessions and +# sent them at the billing page. When the classifier marks the verdict +# unverified, the guidance must hedge and name the other cause. + + +def _anthropic_msg(*, unverified: bool) -> str: + return _billing_or_entitlement_message( + capability="model access", + provider="anthropic", + base_url="https://api.anthropic.com", + model="claude-opus-5", + unverified=unverified, + ) + + +def test_unverified_guidance_names_the_content_filter_alternative(): + msg = _anthropic_msg(unverified=True).lower() + assert "content filter" in msg + # Must give the operator a way to tell the two apart, not just hedge. + assert "still shows quota remaining" in msg + assert "system prompt" in msg + + +def test_unverified_guidance_does_not_assert_exhaustion_as_fact(): + """The opening line must hedge. 'is exhausted' is the claim that misdirected + diagnosis; 'may be exhausted' keeps the billing lead without asserting it.""" + first_line = _anthropic_msg(unverified=True).splitlines()[0].lower() + assert "may be exhausted" in first_line + assert "is exhausted" not in first_line + + +def test_unverified_guidance_warns_about_the_cached_exhaustion_replay(): + """After a failure the credential is latched exhausted and the stored error + is replayed without issuing a request — so a real fix looks like it didn't + work. Point at the reset before the user concludes that.""" + msg = _anthropic_msg(unverified=True) + assert "hermes auth reset anthropic" in msg + assert "without contacting the API" in msg + + +def test_unverified_guidance_keeps_the_billing_remedies(): + """The caveats are additive — the billing remedies stay available.""" + msg = _anthropic_msg(unverified=True) + assert "https://claude.ai/settings/usage" in msg + assert "reset" in msg.lower() + assert "/model" in msg + assert "claude-opus-5" in msg + + +def test_confirmed_guidance_stays_assertive_without_the_caveat(): + """A CONFIRMED billing verdict (e.g. a real 402) must not be diluted by + content-filter lore that only applies to the ambiguous 400 body.""" + msg = _anthropic_msg(unverified=False) + first_line = msg.splitlines()[0].lower() + assert "is exhausted" in first_line + assert "may be exhausted" not in first_line + lowered = msg.lower() + assert "content filter" not in lowered + assert "hermes auth reset" not in lowered + + +def test_content_filter_caveat_is_anthropic_only(): + """A generic provider must not inherit Anthropic-specific classifier lore, + even when the verdict is marked unverified.""" + msg = _billing_or_entitlement_message( + capability="model access", + provider="openrouter", + base_url="https://openrouter.ai/api/v1", + model="anthropic/claude-opus-4.7", + unverified=True, + ).lower() + assert "content filter" not in msg + assert "hermes auth reset" not in msg diff --git a/tests/agent/test_billing_unverified_carrythrough.py b/tests/agent/test_billing_unverified_carrythrough.py new file mode 100644 index 000000000000..775b6adca472 --- /dev/null +++ b/tests/agent/test_billing_unverified_carrythrough.py @@ -0,0 +1,171 @@ +"""#82154: an unverified billing verdict must carry its ambiguity through +every downstream surface — the returned terminal response, the structured +result fields, the credential-pool failure_reason, and the persisted entry — +not just the explanatory guidance text. + +Anthropic returns the identical "out of extra usage" HTTP 400 body on a +subscription OAuth token both for genuine overage depletion and for a +server-side content-filter rejection of the request. The classifier marks +that verdict ``billing_unverified``; these tests pin that the marking is not +dropped on the way out. +""" + +from __future__ import annotations + +from types import SimpleNamespace +from unittest.mock import MagicMock + +from agent.conversation_loop import _billing_failure_result, _billing_terminal_label +from agent.error_classifier import FailoverReason, classify_api_error + + +class MockAPIError(Exception): + def __init__(self, message, status_code=None, body=None): + super().__init__(message) + self.status_code = status_code + self.body = body + + +_EXTRA_USAGE_BODY = ( + "You're out of extra usage. Add more at claude.ai/settings/usage and keep going." +) + + +def _classified_unverified(): + e = MockAPIError( + _EXTRA_USAGE_BODY, + status_code=400, + body={"error": {"type": "invalid_request_error", "message": _EXTRA_USAGE_BODY}}, + ) + return classify_api_error(e, provider="anthropic") + + +def _classified_confirmed(): + e = MockAPIError( + "Your credit balance is too low to access the Anthropic API.", + status_code=400, + body={"error": { + "type": "invalid_request_error", + "message": "Your credit balance is too low to access the Anthropic API.", + }}, + ) + return classify_api_error(e, provider="anthropic") + + +# ── Returned terminal response ─────────────────────────────────────────────── + + +class TestTerminalResponse: + def test_unverified_terminal_response_does_not_assert_billing(self): + """The exact ambiguous 400 must not produce an unhedged + 'Billing or credits exhausted' terminal response.""" + result = _billing_failure_result( + classified=_classified_unverified(), + summary="HTTP 400: out of extra usage", + messages=[], + api_call_count=3, + provider="anthropic", + base_url="https://api.anthropic.com", + model="claude-opus-5", + ) + final = result["final_response"] + assert not final.startswith("Billing or credits exhausted") + assert "unverified" in final + assert "content-filter" in final or "content filter" in final + # The guidance must ride along and hedge too. + assert "still shows quota remaining" in final + + def test_unverified_terminal_response_structured_fields(self): + """The structured result carries the ambiguity, not just the prose.""" + result = _billing_failure_result( + classified=_classified_unverified(), + summary="HTTP 400: out of extra usage", + messages=[], + api_call_count=3, + provider="anthropic", + base_url="https://api.anthropic.com", + model="claude-opus-5", + ) + assert result["failed"] is True + assert result["failure_reason"] == "billing" + assert result["billing_unverified"] is True + block = result["billing_block"] + if block is not None: # None only if billing_links is unavailable + assert block.get("unverified") is True + + def test_confirmed_terminal_response_stays_assertive(self): + """A confirmed billing verdict keeps the original terminal label and + carries no ambiguity flag.""" + result = _billing_failure_result( + classified=_classified_confirmed(), + summary="HTTP 400: credit balance too low", + messages=[], + api_call_count=1, + provider="anthropic", + base_url="https://api.anthropic.com", + model="claude-opus-5", + ) + assert result["final_response"].startswith("Billing or credits exhausted") + assert result["billing_unverified"] is False + block = result["billing_block"] + if block is not None: + assert "unverified" not in block + + def test_terminal_label_contract(self): + assert _billing_terminal_label("boom", False) == "Billing or credits exhausted: boom" + hedged = _billing_terminal_label("boom", True) + assert "unverified" in hedged + assert "content-filter" in hedged + assert not hedged.startswith("Billing or credits exhausted") + + +# ── Credential-pool plumbing ───────────────────────────────────────────────── + + +class TestPoolFailureReason: + def _run_recovery(self, *, billing_unverified: bool) -> dict: + """Drive recover_with_credential_pool with a billing classification and + capture what the pool is told.""" + from agent.agent_runtime_helpers import recover_with_credential_pool + + captured: dict = {} + next_entry = SimpleNamespace(label="secondary") + + class _Pool: + provider = "anthropic" + + def current(self): + return None + + def entries(self): + return [] + + def mark_exhausted_and_rotate(self, **kwargs): + captured.update(kwargs) + return next_entry + + agent = SimpleNamespace( + provider="anthropic", + base_url="https://api.anthropic.com", + api_key="sk-ant-oat01-test", + _credential_pool=_Pool(), + _credential_pool_entry_id=None, + _swap_credential=MagicMock(), + ) + recovered, _ = recover_with_credential_pool( + agent, + status_code=400, + has_retried_429=False, + classified_reason=FailoverReason.billing, + billing_unverified=billing_unverified, + ) + assert recovered is True + return captured + + def test_unverified_billing_reaches_pool_as_unverified(self): + captured = self._run_recovery(billing_unverified=True) + assert captured["failure_reason"] == "billing_unverified" + + def test_confirmed_billing_reaches_pool_as_billing(self): + captured = self._run_recovery(billing_unverified=False) + assert captured["failure_reason"] == "billing" diff --git a/tests/agent/test_credential_pool_sole_cooldown.py b/tests/agent/test_credential_pool_sole_cooldown.py index 26a2ebe4e780..2ff2f034b94f 100644 --- a/tests/agent/test_credential_pool_sole_cooldown.py +++ b/tests/agent/test_credential_pool_sole_cooldown.py @@ -16,7 +16,7 @@ def _write_auth_store(tmp_path, payload: dict) -> None: hermes_home = tmp_path / "hermes" hermes_home.mkdir(parents=True, exist_ok=True) - (hermes_home / "auth.json").write_text(json.dumps(payload, indent=2)) + (hermes_home / "auth.json").write_text(json.dumps(payload, indent=2), encoding="utf-8") def _entry( @@ -161,3 +161,82 @@ def test_multi_key_429_keeps_full_bench(tmp_path, monkeypatch): ) assert pool.has_available() is False assert pool.select() is None + + +# ── #82154: UNVERIFIED billing must not keep the one-hour bench ────────────── +# Anthropic's "out of extra usage" 400 is ambiguous: the same body is returned +# when the server-side content filter rejects part of the request, leaving the +# credential perfectly healthy. An hour-long bench on that verdict blocks a +# healthy key and (sole-credential case) replays the stored error for the full +# hour — making a real fix look like it did not work. + + +def test_sole_credential_unverified_billing_400_recovers_quickly(tmp_path, monkeypatch): + """An unverified billing 400 gets the short transient cooldown, not the + one-hour billing bench.""" + pool = _load( + tmp_path, + monkeypatch, + [_entry(400, age_seconds=90, failure_reason="billing_unverified")], + ) + entry = pool.select() + assert entry is not None + assert entry.last_status == "ok" + + +def test_multi_key_unverified_billing_400_recovers_quickly(tmp_path, monkeypatch): + """The short cooldown applies regardless of pool size: a content-filter + rejection fails identically on EVERY credential, so benching each rotated + key for an hour would take the whole pool offline for nothing.""" + pool = _load( + tmp_path, + monkeypatch, + [ + _entry(400, age_seconds=90, cred_id="cred-1", priority=0, + failure_reason="billing_unverified"), + _entry(400, age_seconds=90, cred_id="cred-2", priority=1, + failure_reason="billing_unverified"), + ], + ) + entry = pool.select() + assert entry is not None + assert entry.last_status == "ok" + + +def test_unverified_billing_ttl_values(tmp_path, monkeypatch): + """Direct TTL contract: unverified billing is transient-sized; confirmed + billing keeps the full bench; a true 402 wins over a stray unverified tag.""" + from agent.credential_pool import ( + EXHAUSTED_TTL_DEFAULT_SECONDS, + EXHAUSTED_TTL_SOLE_CREDENTIAL_SECONDS, + _exhausted_ttl, + ) + + assert ( + _exhausted_ttl(400, sole_credential=True, failure_reason="billing_unverified") + == EXHAUSTED_TTL_SOLE_CREDENTIAL_SECONDS + ) + assert ( + _exhausted_ttl(400, sole_credential=False, failure_reason="billing_unverified") + == EXHAUSTED_TTL_SOLE_CREDENTIAL_SECONDS + ) + assert ( + _exhausted_ttl(400, sole_credential=True, failure_reason="billing") + == EXHAUSTED_TTL_DEFAULT_SECONDS + ) + assert ( + _exhausted_ttl(402, sole_credential=True, failure_reason="billing_unverified") + == EXHAUSTED_TTL_DEFAULT_SECONDS + ) + + +def test_unverified_billing_survives_reload(tmp_path, monkeypatch): + """The unverified marker persists with the entry, so a restart keeps the + short cooldown instead of upgrading it to a billing bench.""" + pool = _load( + tmp_path, + monkeypatch, + [_entry(400, age_seconds=10, failure_reason="billing_unverified")], + ) + entry = pool.entries()[0] + assert entry.failure_reason == "billing_unverified" diff --git a/tests/agent/test_error_classifier.py b/tests/agent/test_error_classifier.py index 8dd8d2bbad1d..59d2cd517683 100644 --- a/tests/agent/test_error_classifier.py +++ b/tests/agent/test_error_classifier.py @@ -812,7 +812,13 @@ def test_400_anthropic_extra_usage_exhausted(self): """Anthropic returns 400 with 'out of extra usage' when the user's extra-usage allowance is depleted. Must classify as billing so the fallback chain engages (with credential rotation) instead of the - generic format_error path, which never rotates. (#11736, #13170)""" + generic format_error path, which never rotates. (#11736, #13170) + + #82154: the identical body is ALSO returned when Anthropic's content + filter rejects part of the request on a subscription OAuth token, so + the billing verdict must be marked unverified — downstream surfaces + hedge instead of asserting exhaustion, and the credential pool skips + the one-hour billing bench.""" e = MockAPIError( "You're out of extra usage. Add more at claude.ai/settings/usage and keep going.", status_code=400, @@ -826,6 +832,33 @@ def test_400_anthropic_extra_usage_exhausted(self): assert result.should_fallback is True assert result.retryable is False assert result.should_rotate_credential is True + assert result.billing_unverified is True + assert result.error_context.get("possible_content_filter") is True + + def test_400_unambiguous_billing_body_is_not_marked_unverified(self): + """A 400 whose billing evidence is NOT the ambiguous 'out of extra + usage' body keeps a confirmed verdict (#82154).""" + e = MockAPIError( + "Your credit balance is too low to access the Anthropic API.", + status_code=400, + body={"error": { + "type": "invalid_request_error", + "message": "Your credit balance is too low to access the Anthropic API.", + }}, + ) + result = classify_api_error(e, provider="anthropic") + assert result.reason == FailoverReason.billing + assert result.billing_unverified is False + + def test_statusless_extra_usage_is_marked_unverified(self): + """Adapters can strip the HTTP status from the Anthropic 400; the + message-only path must carry the same ambiguity marking (#82154).""" + e = Exception( + "You're out of extra usage. Add more at claude.ai/settings/usage and keep going." + ) + result = classify_api_error(e, provider="anthropic") + assert result.reason == FailoverReason.billing + assert result.billing_unverified is True def test_200_with_error_body(self): """200 status with error in body — should be unknown, not crash.""" diff --git a/tests/agent/test_skills_guidance_content_filter.py b/tests/agent/test_skills_guidance_content_filter.py new file mode 100644 index 000000000000..ad23b83a47c5 --- /dev/null +++ b/tests/agent/test_skills_guidance_content_filter.py @@ -0,0 +1,95 @@ +"""SKILLS_GUIDANCE must not carry the phrasing Anthropic's content filter rejects. + +#82154: on a subscription OAuth credential, Anthropic's server-side content +filter rejected the first sentence of the built-in ``SKILLS_GUIDANCE`` prompt +and surfaced the rejection as ``HTTP 400 "You're out of extra usage."`` — +a billing-shaped message that sent users to buy quota they did not need. + +Bisected against the live API against the full 71,721-char assembled prompt: +that sentence alone reproduced the 400, and removing it alone cleared it. +Size (20 KB of filler → 200) and the ``system[0]`` identity gate (a 429, not a +400) were both ruled out. + +These tests pin the reword. They deliberately assert on the *trigger substrings* +rather than on an exact replacement string, so a future rewording is free to +change the prose as long as it does not reintroduce the rejected phrasing or +drop the behaviour the sentence exists to produce. +""" + +from __future__ import annotations + +import re + +import pytest + +from agent.prompt_builder import SKILLS_GUIDANCE + + +# Substrings unique to the rejected sentence. The bisect showed the trigger +# survives removal of the "(5+ tool calls)" clause, so the clause alone is not +# a sufficient guard — the surrounding phrasing is pinned too. +REJECTED_FRAGMENTS = ( + "After completing a complex task", + "5+ tool calls", + "fixing a tricky error", + "save the approach as a", + "so you can reuse it next time", +) + + +class TestRejectedPhrasingIsGone: + @pytest.mark.parametrize("fragment", REJECTED_FRAGMENTS) + def test_trigger_fragment_absent(self, fragment): + assert fragment not in SKILLS_GUIDANCE, ( + f"{fragment!r} is part of the phrasing Anthropic's content filter " + "rejects on subscription OAuth tokens (#82154)" + ) + + def test_first_sentence_is_the_verified_reword(self): + # The reporter verified this replacement returns 200 where the original + # returned 400. Pin the first line so a refactor can't silently revert it. + first_line = SKILLS_GUIDANCE.split("\n", 1)[0] + assert first_line == ( + "When you work out a non-trivial workflow, record it with skill_manage " + "for future reuse." + ) + + +class TestBehaviourIsPreserved: + """The reword must not cost the prompt its meaning — it still has to tell + the model to record workflows as skills and to patch stale ones.""" + + def test_still_instructs_recording_a_workflow_as_a_skill(self): + first_line = SKILLS_GUIDANCE.split("\n", 1)[0].lower() + assert "skill_manage" in first_line + assert "workflow" in first_line + assert "reuse" in first_line + + def test_patch_stale_skills_sentence_untouched(self): + assert "skill_manage(action='patch')" in SKILLS_GUIDANCE + assert "Skills that aren't maintained become liabilities." in SKILLS_GUIDANCE + + def test_skill_safety_rule_block_untouched(self): + # Guarded independently by tests/agent/test_ghost_skill_pruning.py; asserted + # here too so a reword of the guidance can't quietly take the block with it. + assert "## Skill Safety Rule" in SKILLS_GUIDANCE + for rule in ("UNAVAILABLE", "RELOAD", "WAIT", "DEDUP"): + assert rule in SKILLS_GUIDANCE + + def test_real_newlines_and_line_count_preserved(self): + # test_ghost_skill_pruning.py asserts count("\n") >= 6; the reword must + # not drop a line separator on its way past that bound. + assert "\\n" not in SKILLS_GUIDANCE + assert SKILLS_GUIDANCE.count("\n") >= 6 + + +class TestGuidanceReachesTheSystemPrompt: + def test_guidance_is_wired_into_tool_guidance(self): + # A reword is worthless if the constant stopped being appended. Assert the + # wiring rather than trusting the constant in isolation. + import inspect + + import agent.system_prompt as system_prompt + + source = inspect.getsource(system_prompt) + assert re.search(r"tool_guidance\.append\(\s*SKILLS_GUIDANCE\s*\)", source) diff --git a/website/docs/user-guide/features/skills.md b/website/docs/user-guide/features/skills.md index 40bf6d33dcdc..79c693380dfd 100644 --- a/website/docs/user-guide/features/skills.md +++ b/website/docs/user-guide/features/skills.md @@ -470,10 +470,12 @@ below lets you require human review before those changes land. ### When the Agent Creates Skills -- After completing a complex task (5+ tool calls) successfully +The system prompt asks the agent to record a non-trivial workflow with `skill_manage` for +future reuse. In practice that covers: + +- When it worked out a multi-step workflow worth repeating - When it hit errors or dead ends and found the working path - When the user corrected its approach -- When it discovered a non-trivial workflow ### Actions