fix(agent): reword SKILLS_GUIDANCE trigger and stop mislabelling its 400 as billing - #86677
Conversation
…400 as billing On an Anthropic subscription OAuth credential, every request failed with HTTP 400 "You're out of extra usage. Add more at claude.ai/settings/usage". That is not a billing condition: Anthropic's server-side content filter rejects the first sentence of Hermes' own built-in SKILLS_GUIDANCE prompt, and the rejection is surfaced with a billing-shaped message. Because the message points at the usage settings page, it reliably sends people to buy quota they do not need — the reporter lost three debugging sessions to it. Bisected against the live API with the real 71,721-char assembled prompt: the first SKILLS_GUIDANCE sentence alone reproduces the 400 and removing it alone clears it. Size was ruled out (20 KB of unrelated filler returns 200) and so was the system[0] identity gate (that returns 429, a different failure). Three changes, all serving the same outcome — a subscription user can no longer be misdirected by this 400: - agent/prompt_builder.py: reword the triggering sentence to the phrasing the reporter verified returns 200. Meaning, the skill_manage reference, and the ## Skill Safety Rule block are all preserved. The reword is empirically validated rather than understood, so a comment records the bisect and warns that any rewrite must be re-verified against an OAuth token, not an API key. - agent/conversation_loop.py: the Anthropic branch of the billing guidance no longer asserts exhaustion as fact. It hedges the opening line, names the content-filter alternative, and gives the operator a way to tell the two apart (if the usage page still shows quota, suspect a content rejection). It also points at `hermes auth reset anthropic`, because the credential exhaustion latch replays the stored error for ~60 min without issuing a request — which makes a real fix look like it did not work. - hermes_cli/auth.py: document that CLAUDE_CODE_OAUTH_TOKEN is an OAuth token, not an API key, despite auth_type="api_key". It stays in api_key_env_vars because that tuple doubles as the credential-discovery list; removing it would stop Hermes finding a `claude setup-token` credential at all. Docs updated to match the reworded prompt. Fixes #82154
…00 through classification, cooldown, and terminal surfaces
Review follow-up (egilewski): the previous commit only hedged the guidance
text; the exact Anthropic 400 was still classified, persisted, and surfaced
as confirmed billing exhaustion. Carry the ambiguity all the way through:
- agent/error_classifier.py: 'out of extra usage' matches on the 400 and
status-less paths now attach error_context {billing_unverified,
possible_content_filter}. Reason stays FailoverReason.billing (rotation +
fallback remain the right recovery either way); ClassifiedError grows a
billing_unverified property.
- agent/credential_pool.py: new FAILURE_REASON_BILLING_UNVERIFIED. An
unverified billing exhaustion gets the short transient cooldown instead of
the one-hour bench, regardless of pool size: a content-filter rejection
leaves the credential healthy and fails identically on every key, and the
hour-long sole-credential latch is what replayed the stored error and made
real fixes look ineffective. A true 402 keeps the full bench. The marker
persists with the entry so a restart cannot upgrade it back to a bench.
- agent/agent_runtime_helpers.py + run_agent.py: recover_with_credential_pool
threads billing_unverified and hands the pool 'billing_unverified' as the
persisted failure_reason.
- agent/conversation_loop.py: the fallback-switch status, max-retries status,
terminal label, and both structured terminal results hedge when the verdict
is unverified. New _billing_terminal_label + _billing_failure_result build
the returned terminal response in one place; the result dict now carries
billing_unverified and the billing_block gains 'unverified': true. The
confirmed-billing path (a real 402 or an API-key credit depletion) keeps
the original assertive wording, so the caveat no longer dilutes it.
Regression tests: classifier marking (400 + status-less + unambiguous-body
negative), pool cooldown TTLs + persistence round-trip, pool failure_reason
plumbing, and the returned terminal response for both unverified and
confirmed verdicts.
Note: tests/agent/test_credential_pool_routing.py::TestFailureAttribution::
test_unmatched_key_does_not_retry_only_pool_entry fails identically on
current main without this change (pre-existing, unrelated).
alt-glitch
added
type/bug
૮ >ﻌ< ა ci reviewran on fc2ca75 — fix(agent): carry the ambiguity of Anthropic's 'out of extra
|
trevorgordon981
left a comment
trevorgordon981
left a comment
There was a problem hiding this comment.
Sound, well-tested fix with a clean two-part structure and honest documentation of the empirical limitation. Two concrete fragility points worth hardening; neither is blocking.
1. Case-sensitive substring is fragile
_UNVERIFIED_BILLING_PATTERNS = ("out of extra usage",) matches against raw error_msg with no .lower() normalization — unlike _BILLING_PATTERNS, which normalizes case/punctuation. If Anthropic rewrites or localizes the body, or it arrives as "Out of Extra Usage," the marker silently disappears and the bug regresses to benching a healthy key for an hour. A defensive error_msg.lower() in _billing_ambiguity_context is a one-line hardening, and there's no test asserting the absence of the marker to catch a misspelling.
Also note the ambiguity marker is keyed purely on message text, not provider. The pool TTL downgrade (recover_with_credential_pool) is provider-agnostic — only the UI hedge is gated to provider == "anthropic". A non-Anthropic provider that happens to return a billing 400 containing "out of extra usage" would inherit a shortened cooldown it doesn't warrant.
2. Classifier ordering dependency is untested
The marker is applied only inside the _BILLING_PATTERNS branch of _classify_400, which sits after the rate-limit / image-too-large / context-overflow / provider-policy branches — all of which early-return before reaching it. It's correct for the current body, but a future reordering (or a body that trips an earlier pattern) silently loses the unverified marker. Worth a comment plus a test pinning that a body matching both a prior pattern and "out of extra usage" still routes to billing + unverified.
3. billing_unverified reads non-locally in _rotate_failed_credential
effective_reason is read via closure at call time, but billing_unverified is a parameter of recover_with_credential_pool, not a captured variable. Consistent today since the parameter is immutable, but it reads non-locally within the same function — a maintenance trap if the helper is refactored. Bind it into the closure explicitly.
4. Cooldown-vs-degradation tradeoff is documented and acceptable
On a genuine depletion, the short cooldown re-latches and re-fails on the next attempt — bounded churn, not a correctness bug, and the error_code != 402 guard correctly prevents a real 402 from being downgraded to a short cooldown.
Tests
The strongest part of the PR — 117 passing, new suites cover both 400 sub-branches, the status-less path, structured carrythrough, both pool paths, the 402 guard, and pin the trigger substrings (parametrized over 5 fragments). Gaps: no downgrade-order test for _classify_400 (finding #2), and the block-level block["unverified"] carry isn't hard-pinned. Approve after optionally hardening #1/#2.
4 participants
Summary
Salvage of #82177 (@jackulau) onto current main — both commits cherry-pick cleanly with authorship preserved. Fixes #82154; partially addresses #65365 (the SKILLS_GUIDANCE half of that cluster — the
session_search/memoryschema-alias half remains with #78025/#76807).On Anthropic subscription OAuth, every request failed
HTTP 400 "You're out of extra usage". This is not billing: Anthropic's server-side content filter rejects the first sentence of Hermes' built-inSKILLS_GUIDANCEprompt, and the rejection wears a billing-shaped body pointing users at the usage-settings page to buy quota they don't need.Two halves:
agent/prompt_builder.py): the flagged sentence becomes "When you work out a non-trivial workflow, record it with skill_manage for future reuse." — semantically equivalent (save reusable workflows via skill_manage), bisect-verified against the live API in the issue's repro. A code comment records the bisect and warns future editors to re-verify against an OAuth token, not an API key. System-prompt text changes affect prompt cache only at session start — acceptable.error_classifier.pymarks "out of extra usage" matchesbilling_unverified: true / possible_content_filter: true(reason staysbilling— rotation+fallback remain the right recovery either way); the credential pool applies a short cooldown instead of the 1-hour billing bench; every terminal/fallback surface hedges ("may be a content-filter rejection") while still surfacing the wire error — the friendly text never replaces the diagnosable one.Validation
test_anthropic_billing_guidance,test_billing_unverified_carrythrough,test_credential_pool_sole_cooldown,test_error_classifier,test_skills_guidance_content_filter.Fixes #82154. Supersedes #82177 (authorship preserved).
Infographic