From 0e15805e25c0bf386bf31b2390e0ff2d7064ccc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E8=88=AA?= Date: Tue, 21 Jul 2026 19:17:02 +0800 Subject: [PATCH] fix(credential-pool): exhaust all entries sharing a failed API key on 402 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 402/429/401 is an API-key–level failure (account out of balance, rate-limited, or key rejected), but the same key can back more than one pool entry — e.g. an explicit pool entry plus a `model_config` entry auto-seeded from `model.api_key`, both carrying the identical `runtime_api_key`. `mark_exhausted_and_rotate(api_key_hint=...)` only marked the *first* matching entry, leaving the sibling OK. `_select_unlocked()` then kept handing back the same depleted key, so the billing-recovery `continue` loop in the conversation retry path never converged: the request hung until the client disconnected (~2.5min observed against DeepSeek), emitting only `response.created` with no 402 ever surfaced to the user. Mark every entry sharing the failed key so the pool can reach the "no available entries" state and let the error propagate immediately. Adds a regression test covering two entries backed by the same key. --- agent/credential_pool.py | 17 ++++++++ tests/agent/test_credential_pool.py | 64 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/agent/credential_pool.py b/agent/credential_pool.py index f8109a1aa458..7d6adc147096 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -1763,6 +1763,23 @@ class CredentialPool: return None _label = entry.label or entry.id[:8] self._mark_exhausted(entry, status_code, error_context) + # A 402/429/401 is an API-key–level failure: the account is out of + # balance, rate-limited, or its key is rejected. The same key can + # back more than one pool entry (e.g. an explicit pool entry plus a + # ``model_config`` entry auto-seeded from ``model.api_key`` — both + # carry the identical ``runtime_api_key``). Marking only the first + # match leaves the sibling entries OK, so ``_select_unlocked()`` + # keeps handing back the same depleted key and rotation never + # converges — the caller ``continue``s forever until the client + # disconnects (a ~2.5min hang with no error surfaced to the user). + # Mark every entry sharing the failed key so the pool can reach the + # "no available entries" state and let the error propagate. + if api_key_hint: + for sibling in self._entries: + if sibling.id == entry.id: + continue + if sibling.runtime_api_key == api_key_hint: + self._mark_exhausted(sibling, status_code, error_context) # Re-read the updated entry to log the correct terminal state. updated_entry = next( (e for e in self._entries if e.id == entry.id), entry, diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index 3e76470fbace..2a21062d66a5 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -379,6 +379,70 @@ def test_mark_exhausted_and_rotate_persists_status(tmp_path, monkeypatch): assert persisted["last_error_code"] == 402 +def test_billing_rotation_marks_all_entries_sharing_failed_key(tmp_path, monkeypatch): + """A 402 must exhaust every pool entry backed by the same API key. + + Regression: the same key can back more than one pool entry — e.g. an + explicit pool entry plus a ``model_config`` entry auto-seeded from + ``model.api_key`` (both carry the identical ``runtime_api_key``). When + ``mark_exhausted_and_rotate`` is called with ``api_key_hint`` it matched + only the *first* such entry, leaving the sibling OK. ``_select_unlocked()`` + then kept handing back the same depleted key, so the billing-recovery + ``continue`` loop in the conversation retry path never converged — the + request hung ~2.5min until the client disconnected, with no 402 ever + surfaced to the user. All entries sharing the failed key must be + exhausted so the pool reaches "no available entries" and the error + propagates immediately. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + shared_key = "sk-deepseek-shared" + _write_auth_store( + tmp_path, + { + "version": 1, + "credential_pool": { + "custom": [ + { + "id": "cred-explicit", + "label": "520555", + "auth_type": "api_key", + "priority": 0, + "source": "manual", + "access_token": shared_key, + "base_url": "https://api.deepseek.com", + }, + { + "id": "cred-model-config", + "label": "model_config", + "auth_type": "api_key", + "priority": 1, + "source": "manual", + "access_token": shared_key, + "base_url": "https://api.deepseek.com", + }, + ] + }, + }, + ) + + from agent.credential_pool import load_pool, STATUS_EXHAUSTED + + pool = load_pool("custom") + + # First 402 on the shared key: rotation must NOT hand back a sibling + # entry that wraps the same depleted key — it must converge to None. + next_entry = pool.mark_exhausted_and_rotate( + status_code=402, + api_key_hint=shared_key, + ) + assert next_entry is None + + # Both entries are now exhausted (not just the first match). + statuses = {entry.id: entry.last_status for entry in pool.entries()} + assert statuses["cred-explicit"] == STATUS_EXHAUSTED + assert statuses["cred-model-config"] == STATUS_EXHAUSTED + + def test_token_invalidated_marks_credential_dead(tmp_path, monkeypatch): """OpenAI Codex token_invalidated must mark the credential DEAD, not exhausted.