fix(agent): distinguish absent credentials from payment errors in auxiliary unhealthy-marking - #64146
Conversation
…iliary unhealthy-marking _mark_provider_unhealthy() hardcoded "(payment / credit error)" at WARNING into its log line, but four of its eight call sites quarantine for non-billing reasons: absent OpenRouter/Nous credentials, a stale fallback credential, or a rate limit. A local-only setup with zero cloud credentials saw repeated WARNINGs claiming billing problems on providers it never configured. Thread a truthful reason (echoed by both the mark log and the _log_skip_unhealthy skip log) and a log level through the function. The absent-credentials sites now log "no credentials configured" at DEBUG; confirmed 402 paths keep "payment / credit error" at WARNING. Quarantine semantics (TTL, skip behavior) are unchanged. Complements NousResearch#59984/NousResearch#59985 (which fix the retry TTL for the same call sites); this fixes the wording and severity.
640ca75 to
2f52a04
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the misleading unhealthy-provider logs. The premise remains valid on current main: absent OpenRouter credentials still call _mark_provider_unhealthy() at agent/auxiliary_client.py:2053, whose mark and skip logs hardcode payment wording at agent/auxiliary_client.py:2884-2890 and 2919-2922.
Problems
agent/auxiliary_client.py:1994also covers an existing but exhausted OpenRouter pool._select_pool_entry()returns(True, None)whenpool.select()cannot choose an entry (agent/auxiliary_client.py:800-813); the pool selector returnsNonewhen all entries are exhausted (agent/credential_pool.py:1543-1548). Calling that stateno credentials configuredis inaccurate.- The changed assertion at
tests/agent/test_auxiliary_client.py:1120-1134encodes that same exhausted-pool state as absent credentials.
Suggested changes
- Split the no-key path by
pool_present, retaining DEBUG/no-configured-credentials only when no pool exists and assigning a truthful non-payment reason to an exhausted pool. - Add a caplog regression test for the exhausted-pool state, including absence of a payment label.
Automated hermes-sweeper review.
| # payment warnings for a provider they never configured. | ||
| _mark_provider_unhealthy( | ||
| "openrouter", ttl=60, | ||
| reason="no credentials configured", level=logging.DEBUG, |
There was a problem hiding this comment.
This branch also handles _select_pool_entry() returning (True, None), which current CredentialPool._select_unlocked() uses when all pool entries are exhausted (agent/credential_pool.py:1543-1548). That is an existing configured credential pool, not "no credentials configured"; split by pool_present and use a distinct truthful non-payment reason.
SummaryOne PR addresses #64144. #64146 replaces hardcoded payment wording with reason-aware unhealthy-state and skip logging while preserving quarantine behavior, but its OpenRouter branch labels both an absent pool and an exhausted existing pool as "no credentials configured." Related pull requests
Suggested consolidationKeep #64146 open with a salvage path: retain its reason-aware tracking, truthful skip logs, absent-credential DEBUG behavior, and confirmed-payment defaults, while splitting the OpenRouter no-key path by pool presence and adding a separate exhausted-pool caplog test as requested in the contributor keep_open review. There are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I64144(["issue #64144 (open)"])
P64146["PR #64146 (open)"]
P64146 -->|best fix| I64144
class I64144 open
class P64146 open
class P64146 best
class P64146 target
click I64144 "https://github.com/NousResearch/hermes-agent/issues/64144"
click P64146 "https://github.com/NousResearch/hermes-agent/pull/64146"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 14 kB of PR diffs, 12 kB of issue/PR text, 1 kB of discussion (1 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
_mark_provider_unhealthy()hardcoded "(payment / credit error)" at WARNINGinto its log line, but four of its eight call sites quarantine a provider for
non-billing reasons: absent OpenRouter/Nous credentials, a stale fallback
credential, or a rate limit. A local-only setup with zero cloud credentials
(e.g. Ollama via a custom endpoint) saw repeated WARNINGs claiming billing
problems on providers it never configured. The skip line in
_log_skip_unhealthy()had the same defect ("recently returned payment error"for every skip).
This PR threads a truthful
reasonand a loglevelthrough_mark_provider_unhealthy()and echoes the recorded reason from the skip log.The absent-credentials branches now log "no credentials configured" at DEBUG
(an unconfigured optional provider is an expected state); the confirmed-402
branches in
call_llm/acall_llmkeep "payment / credit error" at WARNINGvia the default. Quarantine semantics — TTLs, cache, skip behavior — are
unchanged. This composes with #59985/#60357, which fix the retry TTL for the
same call sites; this PR fixes the wording and severity.
Related Issue
Fixes #64144
Type of Change
Changes Made
agent/auxiliary_client.py_mark_provider_unhealthy(provider, ttl=None, reason="payment / credit error", level=logging.WARNING)— new keyword-only-by-convention params; the reason is stored per label (_aux_unhealthy_reason) and interpolated into the mark log line vialogger.log(level, ...). Default preserves the exact current message and level for the confirmed-payment callers._log_skip_unhealthy()— echoes the stored reason instead of hardcoding "recently returned payment error"._is_provider_unhealthy()/_reset_aux_unhealthy_cache()— evict/clear the reason entry alongside the existing dicts._try_openrouterabsent key →reason="no credentials configured", level=logging.DEBUG_try_nousno auth →reason="no credentials configured", level=logging.DEBUG_try_nousno usable inference JWT →reason="no usable credentials", level=logging.DEBUG_try_nouscross-session rate-limit guard →reason="rate limited"(stays WARNING)_call_fallback_candidate_sync/_call_fallback_candidate_async→reason="stale/unrefreshable credential"(stays WARNING)call_llm/acall_llmconfirmed-402 branches → unchanged (default = payment wording at WARNING)tests/agent/test_auxiliary_client.pyTestAuxUnhealthyReasonLabelingclass (5 tests) pinning: absent OpenRouter key and absent Nous auth mark at DEBUG with "no credentials configured" and emit no payment-labeled record; default (confirmed-402) marking still says "payment / credit error" at WARNING; the skip log echoes the mark reason both ways.test_try_openrouter_pool_exhausted_no_env_marks_unhealthy, the stale-fallback-credential test) for the new kwargs.How to Test
scripts/run_tests.sh tests/agent/test_auxiliary_client.py— 305 passed, 0 failed (includes the 5 new tests).provider: custom,base_url: http://127.0.0.1:11434/v1, noOPENROUTER_API_KEY, no Nous auth), start a session, send one prompt, and watch the log.861d69c7b):OpenRouter key) still produces the payment WARNING unchanged.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (viascripts/run_tests.sh)Documentation & Housekeeping
docs/, docstrings) — docstrings updated in_mark_provider_unhealthycli-config.yaml.exampleif I added/changed config keys — N/A (no config changes)CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Trimmed log from the reproduction session (hermes 0.17.0, local-only Ollama
config, one prompt):