fix(anthropic): consult credential_pool in resolve_anthropic_token (#26344) - #26356
Closed
briandevans wants to merge 1 commit into
Closed
fix(anthropic): consult credential_pool in resolve_anthropic_token (#26344)#26356briandevans wants to merge 1 commit into
briandevans wants to merge 1 commit into
Conversation
…ousResearch#26344) `hermes auth add anthropic --type oauth` (PKCE) stores its token only in `~/.hermes/auth.json::credential_pool["anthropic"]` — not in env vars and not in `~/.claude/.credentials.json`. `resolve_anthropic_token()` walked four sources (ANTHROPIC_TOKEN, CLAUDE_CODE_OAUTH_TOKEN, Claude Code creds file, ANTHROPIC_API_KEY) but never consulted the pool, so cron jobs that route through `resolve_runtime_provider` raised `AuthError: No Anthropic credentials found` even after `hermes auth status anthropic` reported `logged in`. Add a 4th step that reads the pool via `read_credential_pool("anthropic")` and returns the first entry's `access_token`, skipping entries currently in exhaustion cooldown (so the resolver doesn't hand back a token the pool already knows is rate-limited). Rotation/refresh/heartbeat semantics still belong to `CredentialPool.select()` — this is a lightweight read-only fallback for callers that only need a single token string (auxiliary client fallback, cron, account-usage probe, model picker). The new step is purely additive: env-var and Claude Code paths still win in their original priority order. Pool lookup wraps the read in try/except so the test seat belt in `_auth_file_path` and any malformed-JSON / missing-file case degrades cleanly to "no token" rather than crashing the caller. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a credential-pool fallback in resolve_anthropic_token() so tokens stored only via hermes auth add anthropic (especially PKCE OAuth) are resolvable from contexts that don't go through CredentialPool (cron jobs, debug commands).
Changes:
- New helper
_resolve_anthropic_token_from_pool()reads~/.hermes/auth.jsoncredential pool, returns the first usable access token, skipping entries in exhaustion cooldown. resolve_anthropic_token()consults the pool as priority 4, ahead of the legacyANTHROPIC_API_KEYfallback; docstring updated.- New tests cover PKCE-only fallback, exhausted-entry skip, and env-var precedence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| agent/anthropic_adapter.py | Adds pool lookup helper and wires it into the resolver priority chain. |
| tests/agent/test_anthropic_adapter.py | Adds three regression tests for pool fallback, exhaustion skip, and env precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+981
to
+994
| for entry in entries: | ||
| if not isinstance(entry, dict): | ||
| continue | ||
| token = (entry.get("access_token") or "").strip() | ||
| if not token: | ||
| continue | ||
| # Skip entries currently in exhaustion cooldown — pool selection | ||
| # would also skip them, and returning an exhausted token would | ||
| # produce a misleading 401 rather than a clean "no creds" error. | ||
| if entry.get("last_status") == "exhausted": | ||
| reset_at = entry.get("last_error_reset_at") | ||
| if isinstance(reset_at, (int, float)) and reset_at > now_s: | ||
| continue | ||
| return token |
Comment on lines
+979
to
+980
| import time as _time | ||
| now_s = _time.time() |
Comment on lines
+966
to
+973
| for callers that only need a single token string. Returns None on any | ||
| error (pool unavailable, malformed entries, test seat belt tripped). | ||
| """ | ||
| try: | ||
| from hermes_cli.auth import read_credential_pool | ||
| entries = read_credential_pool("anthropic") | ||
| except Exception as e: | ||
| logger.debug("Credential pool lookup failed: %s", e) |
Contributor
Author
|
CI audit — all 7 test failures are pre-existing baselines on clean
All seven reproduce on clean |
Collaborator
alt-glitch
added
P1
Contributor
Author
|
Closing — superseded by @LeonSGP43's #26351, which was opened 11 minutes earlier and covers the same root cause (consult Anthropic |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
hermes auth add anthropic --type oauth(PKCE) stores its token only in the credential_pool.resolve_anthropic_token()never consulted that pool, so cron jobs (and any other call site that doesn't go throughCredentialPooldirectly) raisedAuthError: No Anthropic credentials foundeven whenhermes auth status anthropicreportedlogged in.ANTHROPIC_API_KEYfallback that readsread_credential_pool(\"anthropic\")and returns the first usableaccess_token, skipping entries in exhaustion cooldown.The bug
resolve_anthropic_token()atagent/anthropic_adapter.py:953walked four sources:ANTHROPIC_TOKENenv varCLAUDE_CODE_OAUTH_TOKENenv var~/.claude/.credentials.json(Claude Code OAuth file)ANTHROPIC_API_KEYenv varThe Hermes-native PKCE flow (`hermes auth add anthropic --type oauth`) does not write to any of those —
run_hermes_oauth_login_pure()returns the credential dict, andauth_commands.pystores it as aPooledCredentialwithsource=\"manual:hermes_pkce\"directly into~/.hermes/auth.json::credential_pool[\"anthropic\"][0].For runtime paths that resolve credentials via
resolve_anthropic_token()rather thanCredentialPool.select()— cron viahermes_cli/runtime_provider.py:830, the auxiliary client atagent/auxiliary_client.py:1801/2423,hermes_cli/models.py:2307,agent/account_usage.py:176— the token is invisible. The cron failure trace from the issue:```
cron/scheduler.py:1281 run_job
→ hermes_cli/runtime_provider.py:1194 resolve_runtime_provider
→ agent/anthropic_adapter.py raise AuthError("No Anthropic credentials found…")
```
The fix
Add a 4th step that consults the credential pool. New helper
_resolve_anthropic_token_from_pool():read_credential_pool(\"anthropic\")raw (noCredentialPoolinstantiation — keeps it cheap and side-effect-free; rotation/refresh/heartbeat still belong toCredentialPool.select()).last_status == \"exhausted\"whoselast_error_reset_atis still in the future, so the resolver doesn't hand back a token the pool already knows is rate-limited._auth_file_path, malformed-JSON, missing-file, and any other failure degrade cleanly to "no pool token" rather than crashing the resolver.The new step is purely additive — env-var and Claude Code paths still win in their original priority order, so the existing test matrix (env precedence, refreshable-Claude preference) is unchanged.
Test plan
tests/agent/test_anthropic_adapter.py::TestResolveAnthropicToken::test_falls_back_to_credential_pool_when_only_hermes_pkce_exists— reproduces the cron scenario (no env vars, no~/.claude/, only amanual:hermes_pkceentry in the pool) and asserts the token is returned.test_pool_lookup_skips_exhausted_entries— pool with one exhausted-with-future-reset entry and one fresh entry returns the fresh one.test_env_anthropic_token_still_wins_over_pool—ANTHROPIC_TOKENenv wins over a pool entry, so users can override without editingauth.json.tests/agent/test_credential_pool.py(54 passed) andtests/hermes_cli/test_auth_profile_fallback.py— confirm pool reads still work under profile + global fallback._resolve_anthropic_token_from_poolcall makestest_falls_back_to_credential_pool_when_only_hermes_pkce_existsreturn None — proving the test exercises the new code path, not a coincidence.Note: 2 existing tests (
test_falls_back_to_claude_code_credentials,test_prefers_refreshable_claude_code_credentials_over_static_anthropic_token) fail on my macOS dev box becauseread_claude_code_credentials()hits the real Keychain before the file fixture. They fail on origin/main without my changes (verified viagit stash) and pass on Linux CI.Related
hermes debug) and /usage shows no Codex account quota; _read_codex_tokens() ignores credential_pool #15167 (/usage), but those address diagnostic surfaces. This addresses the runtime path that was actually failing cron.hermes debugdiagnostic viahermes_cli/dump.py; complementary, not a duplicate — different file, different code path.