diff --git a/agent/turn_recovery.py b/agent/turn_recovery.py index c411e1ad82..32b388ff98 100644 --- a/agent/turn_recovery.py +++ b/agent/turn_recovery.py @@ -381,18 +381,48 @@ def _refresh_credentials_after_401( return False -def _is_lingering_codex_token_expired(agent: Any, api_error: Exception, _retry: TurnRetryState) -> bool: - """401 ``token_expired`` that survived the one-shot Codex OAuth refresh (#88510). The Codex - backend rejects a stale replayed ``encrypted_content`` blob with this auth signature, so a - persisted session loops on "sign in again" while a fresh session on the same bearer works. - Once the credential path has had its turn, the caller treats it like +def _is_codex_token_expired(agent: Any, api_error: Exception) -> bool: + """401 ``token_expired`` from the Codex backend (#88510). It rejects a stale replayed + ``encrypted_content`` blob with this auth signature, so a persisted session loops on "sign + in again" while a fresh session on the same bearer works. The caller treats it like ``invalid_encrypted_content`` — but only while cached reasoning items remain to strip.""" - if getattr(api_error, "status_code", None) != 401 or not _retry.codex_auth_retry_attempted: + if getattr(api_error, "status_code", None) != 401: return False reason = agent._extract_api_error_context(api_error).get("reason") return isinstance(reason, str) and reason.strip().lower() == "token_expired" +def _recover_stale_codex_reasoning(agent: Any, _retry: TurnRetryState, messages: List[Dict[str, Any]]) -> bool: + """Stale ``codex_reasoning_items`` blob rejected by the provider: disable replay for the + session, strip cached items (mutates persisted ``messages``), retry once.""" + if ( + _retry.invalid_encrypted_content_retry_attempted + or agent.api_mode != "codex_responses" + or not bool(getattr(agent, "_codex_reasoning_replay_enabled", True)) + or not any( + isinstance(_m, dict) + and _m.get("role") == "assistant" + and isinstance(_m.get("codex_reasoning_items"), list) + and _m.get("codex_reasoning_items") + for _m in messages + ) + ): + return False + _retry.invalid_encrypted_content_retry_attempted = True + replay_stats = agent._disable_codex_reasoning_replay(messages) + _vlines( + agent, + f"⚠️ Encrypted reasoning replay was rejected by the provider — " + f"disabled replay and stripped {replay_stats['items']} item(s) from " + f"{replay_stats['messages']} message(s), retrying...", + ) + logger.warning( + "%sInvalid encrypted reasoning recovery: disabled replay and stripped %d items from %d messages", + agent.log_prefix, replay_stats["items"], replay_stats["messages"], + ) + return True + + def _recover_format_errors( agent: Any, api_error: Exception, classified: Any, _retry: TurnRetryState, messages: List[Dict[str, Any]], api_messages: Any, @@ -418,37 +448,11 @@ def _recover_format_errors( ) return True - # 400 ``invalid_encrypted_content`` on a stale ``codex_reasoning_items`` blob — or the same - # rejection wearing a 401 ``token_expired`` after the credential refresh changed nothing: - # disable replay for the session, strip cached items, retry once. - if ( - ( - classified.reason == FailoverReason.invalid_encrypted_content - or _is_lingering_codex_token_expired(agent, api_error, _retry) - ) - and not _retry.invalid_encrypted_content_retry_attempted - and agent.api_mode == "codex_responses" - and bool(getattr(agent, "_codex_reasoning_replay_enabled", True)) - and any( - isinstance(_m, dict) - and _m.get("role") == "assistant" - and isinstance(_m.get("codex_reasoning_items"), list) - and _m.get("codex_reasoning_items") - for _m in messages - ) + # 400 ``invalid_encrypted_content`` on a stale ``codex_reasoning_items`` blob (the 401 + # ``token_expired`` twin is taken ahead of the credential pool in the caller). + if classified.reason == FailoverReason.invalid_encrypted_content and _recover_stale_codex_reasoning( + agent, _retry, messages ): - _retry.invalid_encrypted_content_retry_attempted = True - replay_stats = agent._disable_codex_reasoning_replay(messages) - _vlines( - agent, - f"⚠️ Encrypted reasoning replay was rejected by the provider — " - f"disabled replay and stripped {replay_stats['items']} item(s) from " - f"{replay_stats['messages']} message(s), retrying...", - ) - logger.warning( - "%sInvalid encrypted reasoning recovery: disabled replay and stripped %d items from %d messages", - agent.log_prefix, replay_stats["items"], replay_stats["messages"], - ) return True # Structured 400 naming ``context_management``: disable native compaction for the @@ -556,15 +560,23 @@ def recover_after_classification( ) -> Tuple[bool, bool]: """One-shot recovery chain that runs AFTER ``classify_api_error`` and before the generic retry path. Order is load-bearing (each branch may ``return`` early): - Nous paid-entitlement refresh → credential-pool rotation → image shrink → - multimodal-tool-content strip → corrupt-image strip → Anthropic OAuth 1M-beta - disable → per-provider 401 credential refresh → format-recovery strips. + Nous paid-entitlement refresh → Codex stale-reasoning strip on 401 ``token_expired`` → + credential-pool rotation → image shrink → multimodal-tool-content strip → corrupt-image + strip → Anthropic OAuth 1M-beta disable → per-provider 401 credential refresh → + format-recovery strips. Returns ``(retry_now, recovered_with_pool)``; the latter feeds the Nous rate-limit guard.""" from agent.conversation_loop import _is_nous_inference_route if _recover_welcome_tier(agent, classified, _retry): return True, False + # 401 ``token_expired`` while the transcript still carries ``codex_reasoning_items`` is a + # stale replayed blob far more often than a dead bearer (#88510): strip BEFORE the pool + # refreshes/benches every healthy entry over a session-state problem. A real expiry pays + # one extra round-trip and then takes the credential path below as before. + if _is_codex_token_expired(agent, api_error) and _recover_stale_codex_reasoning(agent, _retry, messages): + return True, False + if ( classified.reason == FailoverReason.billing and _is_nous_inference_route( diff --git a/tests/agent/test_codex_token_expired_replay_recovery.py b/tests/agent/test_codex_token_expired_replay_recovery.py index da19e83fc4..699e765962 100644 --- a/tests/agent/test_codex_token_expired_replay_recovery.py +++ b/tests/agent/test_codex_token_expired_replay_recovery.py @@ -2,10 +2,11 @@ Issue #88510: the Codex backend rejects a stale replayed ``encrypted_content`` blob with the auth signature (401 ``token_expired``), so a resumed session failed on every prompt while a -fresh session on the same bearer worked. After the one-shot OAuth refresh changed nothing, -``recover_after_classification`` must strip cached ``codex_reasoning_items`` and retry once, -exactly like the 400 ``invalid_encrypted_content`` path; a 401 without cached reasoning is a -real expiry and stays on the credential path. +fresh session on the same bearer worked. ``recover_after_classification`` must strip cached +``codex_reasoning_items`` and retry once, exactly like the 400 ``invalid_encrypted_content`` +path — ahead of the credential pool and the one-shot OAuth refresh, so a session-state problem +never burns a refresh token or benches healthy pool entries; a 401 without cached reasoning is +a real expiry and stays on the credential path. """ from __future__ import annotations @@ -79,17 +80,18 @@ def _recover(agent, err, retry, messages): ) -def test_lingering_token_expired_strips_cached_reasoning_once_after_refresh(): +def test_token_expired_strips_cached_reasoning_once_before_the_credential_path(): agent, retry, messages = _Agent(), TurnRetryState(), _cached_history() retried, _ = _recover(agent, _Codex401(), retry, messages) assert retried is True - assert agent.refresh_calls == 1 # the credential path ran first and changed nothing + assert agent.refresh_calls == 0 # no refresh token burned on a session-state problem assert agent._codex_reasoning_replay_enabled is False assert not any("codex_reasoning_items" in m for m in messages) - # A second identical 401 in the same turn is a real auth failure: no second strip. + # A second identical 401 in the same turn is a real auth failure: refresh once, no second strip. assert _recover(agent, _Codex401(), retry, messages) == (False, False) + assert agent.refresh_calls == 1 @pytest.mark.parametrize("err, history", [ @@ -103,3 +105,26 @@ def test_token_expired_without_cached_reasoning_stays_on_auth_path(err, history) assert agent.refresh_calls == 1 assert agent._codex_reasoning_replay_enabled is True assert retry.invalid_encrypted_content_retry_attempted is False + + +def test_pooled_token_expired_strips_cached_reasoning_before_benching_entries(): + """A stale blob is a session-state problem: the strip must run before the pool benches + every healthy entry (STATUS_EXHAUSTED) over a 401 the credentials did not cause.""" + from agent.agent_runtime_helpers import recover_with_credential_pool + from agent.credential_pool import CredentialPool, PooledCredential + + agent, retry, messages = _Agent(), TurnRetryState(), _cached_history() + entries = [ + PooledCredential(provider="openai-codex", id=f"acct-{i}", label=f"acct-{i}", auth_type="oauth", + priority=i, source=f"acct-{i}", access_token=token) + for i, token in enumerate(["same-bearer", "other-bearer"]) + ] + pool = CredentialPool("openai-codex", entries) + agent._credential_pool = pool + agent._recover_with_credential_pool = lambda **kw: recover_with_credential_pool(agent, **kw) + + retried, recovered_with_pool = _recover(agent, _Codex401(), retry, messages) + + assert (retried, recovered_with_pool) == (True, False) + assert not any("codex_reasoning_items" in m for m in messages) + assert [e.last_status for e in pool.entries()] == [None, None] # nobody benched diff --git a/website/docs/developer-guide/provider-runtime.md b/website/docs/developer-guide/provider-runtime.md index 203965c833..38f4970223 100644 --- a/website/docs/developer-guide/provider-runtime.md +++ b/website/docs/developer-guide/provider-runtime.md @@ -152,6 +152,7 @@ Codex uses a separate Responses API path: - `api_mode = codex_responses` - dedicated credential resolution and auth store support +- a resumed session whose lingering Codex reasoning items (`encrypted_content`) are rejected — as a 400 `invalid_encrypted_content` or as a 401 `token_expired` — self-heals by stripping the cached items and replaying once, before any credential refresh or pool rotation ## Auxiliary model routing