Skip to content

Commit d48e3ea

Browse files
author
Mateusz
committed
fix(openai-codex): reduce managed OAuth false positives on refresh and 403
- Branch refresh_access_token on ManagedOAuthRefreshError: skip penalizing rotation for transient and needs_reauth; soft-rotate for other failures. - Reload disk state in rotate_on_auth_failure before saving; skip counter when account already needs_reauth on disk. - Add rotate_away_without_auth_penalty; use for HTTP 403 streaming handshake and handle_forbidden_rotation on connector/credentials. - Add selector and credential tests; wire executor/conftest mocks. Also includes local changes to codex quota notifications, continuation-related tests, acp stale kill timer, tool calls regression, and codex CLI tests. Made-with: Cursor
1 parent 3690470 commit d48e3ea

13 files changed

Lines changed: 676 additions & 57 deletions

src/connectors/_openai_codex_connector.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ def has_static_credentials(self) -> bool:
115115

116116
# Supported Codex models - sourced from official Codex CLI models.json
117117
SUPPORTED_CODEX_MODELS: tuple[str, ...] = (
118+
"gpt-5.5",
118119
"gpt-5.4",
119120
"gpt-5.4-mini",
120121
"gpt-5.3-codex",
@@ -140,6 +141,7 @@ def has_static_credentials(self) -> bool:
140141
DEFAULT_REASONING_EFFORT: str = "medium"
141142
# All current models support xhigh reasoning effort
142143
XHIGH_SUPPORTED_MODELS: tuple[str, ...] = (
144+
"gpt-5.5",
143145
"gpt-5.4",
144146
"gpt-5.4-mini",
145147
"gpt-5.3-codex",
@@ -1809,6 +1811,39 @@ async def _handle_auth_failure_rotation(
18091811
return True
18101812
return False
18111813

1814+
async def _handle_forbidden_rotation(
1815+
self,
1816+
*,
1817+
session_id: str | None = None,
1818+
) -> bool:
1819+
"""Rotate managed OAuth accounts after HTTP 403 without auth-failure penalties."""
1820+
rotate_method = getattr(
1821+
self._credential_manager,
1822+
"handle_forbidden_rotation",
1823+
None,
1824+
)
1825+
if not callable(rotate_method):
1826+
return False
1827+
1828+
try:
1829+
result = rotate_method(session_id=session_id)
1830+
rotated = await result if inspect.isawaitable(result) else bool(result)
1831+
except Exception as exc:
1832+
if logger.isEnabledFor(logging.WARNING):
1833+
logger.warning(
1834+
"Failed to rotate managed OAuth account after HTTP 403: %s",
1835+
exc,
1836+
exc_info=True,
1837+
)
1838+
return False
1839+
1840+
if rotated:
1841+
token = self._credential_manager.get_access_token()
1842+
if token:
1843+
self.api_key = token
1844+
return True
1845+
return False
1846+
18121847
# -----------------------------
18131848
# Health Tracking API (stale token handling pattern)
18141849
# -----------------------------

src/connectors/openai_codex/codex_quota_notifications.py

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,11 @@ async def maybe_notify_codex_quota_remaining_low(
271271
``latch_keys`` uses tuples ``(managed_account_id, limit_kind, threshold)``.
272272
A key is cleared when remaining rises back to ``>=`` that threshold so alerts
273273
can re-fire after recovery.
274+
275+
When several thresholds are violated in one evaluation (e.g. remaining 8%
276+
with alerts at 25% and 10%), at most one desktop notification is sent for
277+
that limit kind, using the tightest newly crossed threshold; all violated
278+
thresholds are latched so they do not each emit a separate bubble.
274279
"""
275280
if notification_service is None or not notification_service.is_enabled:
276281
return
@@ -293,31 +298,43 @@ async def maybe_notify_codex_quota_remaining_low(
293298
if not math.isfinite(float(remaining)):
294299
continue
295300
rem = float(remaining)
301+
296302
for thr in thresholds:
297303
key = (managed_account_id, kind, thr)
298304
if rem >= thr:
299305
latch_keys.discard(key)
300-
continue
301-
if key in latch_keys:
302-
continue
303-
message = build_codex_low_remaining_notification_message(
304-
email=email,
305-
managed_account_id=managed_account_id,
306-
chatgpt_account_id=chatgpt_account_id,
307-
limit_kind=kind,
308-
threshold_percent=thr,
309-
remaining_percent=rem,
306+
307+
violated = [thr for thr in thresholds if rem < thr]
308+
if not violated:
309+
continue
310+
311+
newly_violated = [
312+
thr for thr in violated if (managed_account_id, kind, thr) not in latch_keys
313+
]
314+
if not newly_violated:
315+
continue
316+
317+
worst_new = min(newly_violated)
318+
message = build_codex_low_remaining_notification_message(
319+
email=email,
320+
managed_account_id=managed_account_id,
321+
chatgpt_account_id=chatgpt_account_id,
322+
limit_kind=kind,
323+
threshold_percent=worst_new,
324+
remaining_percent=rem,
325+
)
326+
try:
327+
await notification_service.send_notification(
328+
title=_CODEX_QUOTA_LOW_REMAINING_TITLE,
329+
message=message,
310330
)
311-
try:
312-
await notification_service.send_notification(
313-
title=_CODEX_QUOTA_LOW_REMAINING_TITLE,
314-
message=message,
315-
)
316-
except Exception as exc:
317-
logger.warning(
318-
"Codex low remaining quota notification failed: %s",
319-
exc,
320-
exc_info=True,
321-
)
322-
continue
323-
latch_keys.add(key)
331+
except Exception as exc:
332+
logger.warning(
333+
"Codex low remaining quota notification failed: %s",
334+
exc,
335+
exc_info=True,
336+
)
337+
continue
338+
339+
for thr in violated:
340+
latch_keys.add((managed_account_id, kind, thr))

src/connectors/openai_codex/credentials.py

Lines changed: 101 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,19 @@ async def _managed_has_accounts(self) -> bool:
463463
return False
464464
return await self._managed_storage.has_configured_accounts()
465465

466+
async def _codex_legacy_auth_file_fallback_allowed(self) -> bool:
467+
"""Return True if loading Codex desktop ``auth.json`` is allowed as fallback.
468+
469+
When managed OAuth is enabled and at least one account file exists under the
470+
managed storage path, legacy ``auth.json`` must not be used, even if no
471+
managed account is currently selectable (e.g. all quotas exhausted).
472+
"""
473+
if not self._managed_enabled():
474+
return True
475+
if await self._managed_has_accounts():
476+
return False
477+
return bool(self._managed_config.allow_legacy_fallback)
478+
466479
def _managed_account_to_credentials(
467480
self,
468481
account: ManagedOAuthAccount,
@@ -530,6 +543,18 @@ async def _load_auth(self, force_reload: bool = False) -> bool:
530543
if await self._load_managed_auth(force_reload=force_reload):
531544
return True
532545

546+
if not await self._codex_legacy_auth_file_fallback_allowed():
547+
if self._managed_enabled() and await self._managed_has_accounts():
548+
logger.warning(
549+
"OpenAI Codex: managed OAuth account files exist but no managed "
550+
"account is currently selectable; not using legacy auth.json "
551+
"from the Codex app."
552+
)
553+
self._active_source = "none"
554+
self._managed_current_account = None
555+
self._auth_credentials = None
556+
return False
557+
533558
explicit_legacy_override = (
534559
self._auth_path is not None or self._oauth_dir_override is not None
535560
)
@@ -872,20 +897,63 @@ async def refresh_access_token(self) -> bool:
872897
"Attempting to refresh OpenAI Codex access token after authentication failure."
873898
)
874899
if await self._load_managed_auth(force_reload=True):
875-
if await self._refresh_managed_access_token():
900+
success, err = await self._refresh_managed_access_token()
901+
if success:
876902
return True
877-
return await self._rotate_managed_on_auth_failure()
903+
if err is None:
904+
return False
905+
if err.from_transient_network:
906+
return False
907+
if err.needs_reauth:
908+
await self._managed_selector.reload_accounts()
909+
nxt = await self._managed_selector.get_next_account(
910+
wait_for_rate_limit_recovery=False,
911+
ignore_session_affinity=True,
912+
)
913+
if nxt is None:
914+
return False
915+
self._managed_selector.update_account(nxt)
916+
self._managed_current_account = nxt
917+
self._auth_credentials = self._managed_account_to_credentials(nxt)
918+
self._active_source = "managed"
919+
return bool(nxt.access_token)
920+
rotated = await self._managed_selector.rotate_away_without_auth_penalty(
921+
session_id=None,
922+
)
923+
if rotated is None:
924+
return False
925+
self._managed_current_account = rotated
926+
self._auth_credentials = self._managed_account_to_credentials(rotated)
927+
self._active_source = "managed"
928+
return bool(rotated.access_token)
929+
if not await self._codex_legacy_auth_file_fallback_allowed():
930+
if self._managed_enabled() and await self._managed_has_accounts():
931+
logger.warning(
932+
"OpenAI Codex: managed OAuth account files exist but no "
933+
"managed account is currently selectable; not refreshing "
934+
"from legacy auth.json."
935+
)
936+
return False
878937
return await self._refresh_legacy_access_token()
879938

880-
async def _refresh_managed_access_token(self) -> bool:
939+
async def _refresh_managed_access_token(
940+
self,
941+
) -> tuple[bool, ManagedOAuthRefreshError | None]:
942+
"""Force-refresh the managed OAuth access token for the current (or next) account.
943+
944+
Returns:
945+
``(True, None)`` on success.
946+
``(False, None)`` when no managed account is available to refresh.
947+
``(False, exc)`` when refresh fails.
948+
"""
881949
account = self._managed_selector.get_current_account()
882950
if account is None:
883951
# Refresh path should also avoid blocking on rate-limit recovery sleeps.
884952
account = await self._managed_selector.get_next_account(
885953
wait_for_rate_limit_recovery=False,
886954
)
887955
if account is None:
888-
return False
956+
return False, None
889957

890958
try:
891959
updated = await self._managed_refresh.force_refresh(account)
@@ -903,22 +971,13 @@ async def _refresh_managed_access_token(self) -> bool:
903971
exc,
904972
exc_info=True,
905973
)
906-
return False
974+
return False, exc
907975

908976
self._managed_selector.update_account(updated)
909977
self._managed_current_account = updated
910978
self._auth_credentials = self._managed_account_to_credentials(updated)
911979
self._active_source = "managed"
912-
return True
913-
914-
async def _rotate_managed_on_auth_failure(self) -> bool:
915-
rotated = await self._managed_selector.rotate_on_auth_failure()
916-
if rotated is None:
917-
return False
918-
self._managed_current_account = rotated
919-
self._auth_credentials = self._managed_account_to_credentials(rotated)
920-
self._active_source = "managed"
921-
return True
980+
return True, None
922981

923982
async def _refresh_legacy_access_token(self) -> bool:
924983
await self._load_legacy_auth(force_reload=True)
@@ -1378,6 +1437,14 @@ async def handle_auth_failure(
13781437
"""Rotate away from currently failing managed account on auth denial."""
13791438
async with self._token_refresh_lock:
13801439
if not await self._load_managed_auth(force_reload=True):
1440+
if not await self._codex_legacy_auth_file_fallback_allowed():
1441+
if self._managed_enabled() and await self._managed_has_accounts():
1442+
logger.warning(
1443+
"OpenAI Codex: managed OAuth account files exist but no "
1444+
"managed account is currently selectable; not handling "
1445+
"auth failure via legacy auth.json."
1446+
)
1447+
return False
13811448
return await self._refresh_legacy_access_token()
13821449
rotated = await self._managed_selector.rotate_on_auth_failure(
13831450
session_id=session_id
@@ -1389,6 +1456,25 @@ async def handle_auth_failure(
13891456
self._active_source = "managed"
13901457
return True
13911458

1459+
async def handle_forbidden_rotation(
1460+
self,
1461+
*,
1462+
session_id: str | None = None,
1463+
) -> bool:
1464+
"""Try another managed account after HTTP 403 without auth-failure penalties."""
1465+
async with self._token_refresh_lock:
1466+
if not await self._load_managed_auth(force_reload=True):
1467+
return False
1468+
rotated = await self._managed_selector.rotate_away_without_auth_penalty(
1469+
session_id=session_id,
1470+
)
1471+
if rotated is None:
1472+
return False
1473+
self._managed_current_account = rotated
1474+
self._auth_credentials = self._managed_account_to_credentials(rotated)
1475+
self._active_source = "managed"
1476+
return True
1477+
13921478
async def mark_account_used(self) -> None:
13931479
"""Mark currently selected managed account as used."""
13941480
if self._active_source != "managed":

src/connectors/openai_codex/executor.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ async def _streaming_iterator() -> AsyncIterator[ProcessedResponse]:
609609
except (HTTPException, LLMProxyError) as exc:
610610
status_code, detail = _codex_initiate_streaming_error_view(exc)
611611
if status_code == 403 and attempts_used < max_retries:
612-
rotated = await self._handle_auth_failure_rotation(
612+
rotated = await self._handle_forbidden_rotation(
613613
session_id=context.session_id
614614
)
615615
if rotated:
@@ -2021,6 +2021,34 @@ async def _handle_auth_failure_rotation(self, *, session_id: str | None) -> bool
20212021
return True
20222022
return False
20232023

2024+
async def _handle_forbidden_rotation(self, *, session_id: str | None) -> bool:
2025+
rotate_method = getattr(
2026+
self._base_connector,
2027+
"_handle_forbidden_rotation",
2028+
None,
2029+
)
2030+
if callable(rotate_method):
2031+
result = rotate_method(session_id=session_id)
2032+
rotated = await result if inspect.isawaitable(result) else bool(result)
2033+
return bool(rotated)
2034+
2035+
fallback_rotate = getattr(
2036+
self._credential_manager,
2037+
"handle_forbidden_rotation",
2038+
None,
2039+
)
2040+
if not callable(fallback_rotate):
2041+
return False
2042+
2043+
result = fallback_rotate(session_id=session_id)
2044+
rotated = await result if inspect.isawaitable(result) else bool(result)
2045+
if rotated:
2046+
new_token = self._credential_manager.get_access_token()
2047+
if new_token and hasattr(self._base_connector, "api_key"):
2048+
self._base_connector.api_key = new_token
2049+
return True
2050+
return False
2051+
20242052
async def _mark_account_used(self) -> None:
20252053
mark_used_method = getattr(self._credential_manager, "mark_account_used", None)
20262054
if not callable(mark_used_method):

0 commit comments

Comments
 (0)