Fix Anthropic OAuth stale token shadowing - #5101
Conversation
|
Good diagnosis and a clean fix. The stale-token-shadowing bug is a real footgun — users doing OAuth twice on Anthropic ending up with an expired What I like:
Three things to consider before merge: 1. try:
from agent.anthropic_adapter import resolve_anthropic_token
anthropic_key = resolve_anthropic_token()
except Exception:
anthropic_key = os.getenv("ANTHROPIC_TOKEN") or os.getenv("ANTHROPIC_API_KEY")If 2. Migration hint for users with stale env tokens: After this PR, a user with Consider a one-shot info log when logger.info(
"Using refreshable %s credentials; your .env contains a static ANTHROPIC_TOKEN "
"that is being ignored. You can remove it to avoid confusion.",
label,
)Or surface this in 3. Atomicity of dual-store writes in anthropic_mod._save_hermes_oauth_credentials(...)
anthropic_mod._write_claude_code_credentials(...)If the first write succeeds and the second fails (disk full, permission error), the stores diverge. Not catastrophic — Test coverage question: does the test suite cover the case where both stores have expired creds AND refresh fails for both? The fallback path to Ship it once the doctor/status exception handling is addressed — everything else is polish. |
|
Addressed the review blocker around the broad exception fallbacks in doctor/status.
Validation: |
|
@mattsegura FSA there are some merge conflicts on this branch that'll probably need to get touched up before this is hopefully reviewed/merged |
|
Thanks @mattsegura. Closing — heavy overlap with @5park1e's #12971 which landed via #15175 (commit e110677). #12971 addresses the same core bug (stale OAuth token → Your PR's 12-file scope included touches to |
Fixes #6346
Summary
Fix Anthropic OAuth flows getting stuck behind stale
ANTHROPIC_TOKENvalues.Changes
hermes modelAnthropic OAuth flowhermes auth add anthropicReproduction (before fix)
~/.hermes/.env:export ANTHROPIC_TOKEN=stale-env-tokenFor example:
hermes model.ANTHROPIC_TOKENcould still win during credential resolution and shadow the fresh OAuth credentials.Observed pre-fix behavior:
hermes auth add anthropicdid not consistently persist the right credential source for later useWhy
Users could end up with an invalid or stale
ANTHROPIC_TOKENin.env, causing Anthropic 401s even after reauth. The refreshable credential stores were not consistently preferred or persisted across flows.Validation
pytest -q -o addopts='' tests/test_anthropic_oauth_flow.py tests/test_anthropic_adapter.py tests/test_credential_pool.py tests/test_auth_commands.py tests/test_anthropic_provider_persistence.py python3 -m compileall agent/anthropic_adapter.py hermes_cli/main.py hermes_cli/auth_commands.py hermes_cli/status.py hermes_cli/doctor.pyResult:
148 passed