mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(pool): sync anthropic entry on access_token change, not just refresh_token
`_sync_anthropic_entry_from_credentials_file` only checked whether the
refresh_token in ~/.claude/.credentials.json differed from the pool
entry's refresh_token. This missed the case where the CLI performs a
silent access-token re-issue — returning a new access_token alongside
the *same* refresh_token. The pool entry's stale bearer token was never
updated, causing 401 errors on every request until the exhausted-TTL
(5 min) expired.
Bring this function to parity with its Codex and xAI OAuth siblings:
- Check either access_token *or* refresh_token changed (dual-field guard).
- Use `file_X or entry.X` fallbacks so a partial file can't blank a field.
- Clear all six status/error fields on sync (last_error_reason,
last_error_message, last_error_reset_at were previously omitted),
ensuring an exhausted entry becomes available immediately.
Spotted via parity review against commit 569bc94b5 which fixed the same
pattern in `_sync_nous_entry_from_auth_store`.
This commit is contained in:
parent
c510f48680
commit
d634fa079e
2 changed files with 157 additions and 6 deletions
|
|
@ -616,17 +616,32 @@ class CredentialPool:
|
|||
file_refresh = creds.get("refreshToken", "")
|
||||
file_access = creds.get("accessToken", "")
|
||||
file_expires = creds.get("expiresAt", 0)
|
||||
# If the credentials file has a different token pair, sync it
|
||||
if file_refresh and file_refresh != entry.refresh_token:
|
||||
logger.debug("Pool entry %s: syncing tokens from credentials file (refresh token changed)", entry.id)
|
||||
# Sync when either token changed. Access tokens can be re-issued
|
||||
# without a new refresh token (silent re-issue path), so checking
|
||||
# only refresh_token misses that case and leaves a stale
|
||||
# access_token in the pool → 401 on every request until the pool
|
||||
# entry's exhausted TTL expires.
|
||||
entry_access = entry.access_token or ""
|
||||
entry_refresh = entry.refresh_token or ""
|
||||
if (file_access or file_refresh) and (
|
||||
(file_access and file_access != entry_access)
|
||||
or (file_refresh and file_refresh != entry_refresh)
|
||||
):
|
||||
logger.debug(
|
||||
"Pool entry %s: syncing tokens from credentials file (tokens changed)",
|
||||
entry.id,
|
||||
)
|
||||
updated = replace(
|
||||
entry,
|
||||
access_token=file_access,
|
||||
refresh_token=file_refresh,
|
||||
expires_at_ms=file_expires,
|
||||
access_token=file_access or entry.access_token,
|
||||
refresh_token=file_refresh or entry.refresh_token,
|
||||
expires_at_ms=file_expires or entry.expires_at_ms,
|
||||
last_status=None,
|
||||
last_status_at=None,
|
||||
last_error_code=None,
|
||||
last_error_reason=None,
|
||||
last_error_message=None,
|
||||
last_error_reset_at=None,
|
||||
)
|
||||
self._replace_entry(entry, updated)
|
||||
self._persist()
|
||||
|
|
|
|||
|
|
@ -3146,3 +3146,139 @@ def test_remove_index_does_not_resurrect_via_disk_merge(tmp_path, monkeypatch):
|
|||
final = json.loads((tmp_path / "hermes" / "auth.json").read_text())
|
||||
final_ids = [entry["id"] for entry in final["credential_pool"]["anthropic"]]
|
||||
assert final_ids == ["cred-A"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _sync_anthropic_entry_from_credentials_file — parity fix tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _make_anthropic_claude_code_pool(tmp_path, monkeypatch, *, access_token, refresh_token, expires_at_ms=9_999_999_999_000):
|
||||
"""Helper: load an Anthropic pool seeded with a single claude_code entry."""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||
monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)
|
||||
monkeypatch.delenv("ANTHROPIC_TOKEN", raising=False)
|
||||
monkeypatch.delenv("CLAUDE_CODE_OAUTH_TOKEN", raising=False)
|
||||
_write_auth_store(tmp_path, {"version": 1, "credential_pool": {}})
|
||||
monkeypatch.setattr("hermes_cli.auth.is_provider_explicitly_configured", lambda pid: pid == "anthropic")
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_hermes_oauth_credentials",
|
||||
lambda: None,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_claude_code_credentials",
|
||||
lambda: {"accessToken": access_token, "refreshToken": refresh_token, "expiresAt": expires_at_ms},
|
||||
)
|
||||
from agent.credential_pool import load_pool
|
||||
pool = load_pool("anthropic")
|
||||
entry = pool.select()
|
||||
assert entry is not None
|
||||
assert entry.source == "claude_code"
|
||||
return pool, entry
|
||||
|
||||
|
||||
def test_sync_anthropic_entry_access_token_only_changed(tmp_path, monkeypatch):
|
||||
"""Sync must trigger when access_token rotates but refresh_token stays the same.
|
||||
|
||||
This is the parity-fix case: the old code checked only refresh_token,
|
||||
so a silent access_token re-issue left the pool with a stale bearer token.
|
||||
"""
|
||||
pool, entry = _make_anthropic_claude_code_pool(
|
||||
tmp_path, monkeypatch,
|
||||
access_token="old-access",
|
||||
refresh_token="shared-refresh",
|
||||
)
|
||||
|
||||
# Credentials file: new access_token, same refresh_token
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_claude_code_credentials",
|
||||
lambda: {"accessToken": "new-access", "refreshToken": "shared-refresh", "expiresAt": 9_999_999_999_000},
|
||||
)
|
||||
|
||||
synced = pool._sync_anthropic_entry_from_credentials_file(entry)
|
||||
|
||||
assert synced is not entry, "sync must return a new entry object"
|
||||
assert synced.access_token == "new-access"
|
||||
assert synced.refresh_token == "shared-refresh"
|
||||
|
||||
|
||||
def test_sync_anthropic_entry_refresh_token_changed(tmp_path, monkeypatch):
|
||||
"""Sync must trigger when refresh_token rotates (single-use rotation path)."""
|
||||
pool, entry = _make_anthropic_claude_code_pool(
|
||||
tmp_path, monkeypatch,
|
||||
access_token="access-v1",
|
||||
refresh_token="refresh-v1",
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_claude_code_credentials",
|
||||
lambda: {"accessToken": "access-v2", "refreshToken": "refresh-v2", "expiresAt": 9_999_999_999_000},
|
||||
)
|
||||
|
||||
synced = pool._sync_anthropic_entry_from_credentials_file(entry)
|
||||
|
||||
assert synced is not entry
|
||||
assert synced.access_token == "access-v2"
|
||||
assert synced.refresh_token == "refresh-v2"
|
||||
|
||||
|
||||
def test_sync_anthropic_entry_tokens_unchanged_no_op(tmp_path, monkeypatch):
|
||||
"""Sync must be a no-op when credentials file matches the pool entry."""
|
||||
pool, entry = _make_anthropic_claude_code_pool(
|
||||
tmp_path, monkeypatch,
|
||||
access_token="same-access",
|
||||
refresh_token="same-refresh",
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_claude_code_credentials",
|
||||
lambda: {"accessToken": "same-access", "refreshToken": "same-refresh", "expiresAt": 9_999_999_999_000},
|
||||
)
|
||||
|
||||
synced = pool._sync_anthropic_entry_from_credentials_file(entry)
|
||||
|
||||
assert synced is entry, "no-op sync must return the original entry object"
|
||||
|
||||
|
||||
def test_sync_anthropic_entry_clears_all_error_fields(tmp_path, monkeypatch):
|
||||
"""Syncing fresh tokens must clear all six error/status fields on the entry.
|
||||
|
||||
Before the fix, last_error_reason / last_error_message / last_error_reset_at
|
||||
were left set, so a previously-exhausted entry could stay stuck even after
|
||||
fresh tokens arrived from the credentials file.
|
||||
"""
|
||||
from dataclasses import replace as dc_replace
|
||||
from agent.credential_pool import STATUS_EXHAUSTED
|
||||
|
||||
pool, entry = _make_anthropic_claude_code_pool(
|
||||
tmp_path, monkeypatch,
|
||||
access_token="stale-access",
|
||||
refresh_token="stale-refresh",
|
||||
)
|
||||
|
||||
now = time.time()
|
||||
exhausted = dc_replace(
|
||||
entry,
|
||||
last_status=STATUS_EXHAUSTED,
|
||||
last_status_at=now,
|
||||
last_error_code=401,
|
||||
last_error_reason="token_expired",
|
||||
last_error_message="Access token has expired",
|
||||
last_error_reset_at=now + 300,
|
||||
)
|
||||
pool._replace_entry(entry, exhausted)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"agent.anthropic_adapter.read_claude_code_credentials",
|
||||
lambda: {"accessToken": "fresh-access", "refreshToken": "fresh-refresh", "expiresAt": 9_999_999_999_000},
|
||||
)
|
||||
|
||||
synced = pool._sync_anthropic_entry_from_credentials_file(exhausted)
|
||||
|
||||
assert synced is not exhausted
|
||||
assert synced.access_token == "fresh-access"
|
||||
assert synced.last_status is None
|
||||
assert synced.last_status_at is None
|
||||
assert synced.last_error_code is None
|
||||
assert synced.last_error_reason is None
|
||||
assert synced.last_error_message is None
|
||||
assert synced.last_error_reset_at is None
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue