From d634fa079ea398fc4f58701c913c3d1c32d8cf4e Mon Sep 17 00:00:00 2001 From: EloquentBrush0x <283442588+EloquentBrush0x@users.noreply.github.com> Date: Mon, 18 May 2026 11:47:38 +0300 Subject: [PATCH] fix(pool): sync anthropic entry on access_token change, not just refresh_token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_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`. --- agent/credential_pool.py | 27 ++++-- tests/agent/test_credential_pool.py | 136 ++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 6 deletions(-) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index ea15d790905..8d10bbb1cbf 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -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() diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index ad9cbfcbdba..461fd243a45 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -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