From 95f20517c25ee418da5337f4ead347008baaa2b3 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Tue, 22 Sep 2026 20:36:52 +0530 Subject: [PATCH] fix(codex): key the catalog cache on token state too, and drop the inner digest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An expired stored token makes _codex_catalog serve the static fallback (no Astra). Under the principal-only key that fallback outlived the token refresh for the whole cache TTL — before this stack the auth.json rewrite busted it. The identity now has an "expired" state so the refresh to a live token for the same principal is a cache miss, as it was. The helper also hashed the principal itself; _credential_fingerprint blake2b-hashes the joined parts one frame up (they already carry raw API-key env values), so the second digest bought nothing. It returns the principal (or the opaque token) directly, and the empty-token case is one early return. --- hermes_cli/codex_models.py | 28 ++++++++++++++---------- tests/hermes_cli/test_model_cache_swr.py | 12 +++++++++- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/hermes_cli/codex_models.py b/hermes_cli/codex_models.py index 4125dab0aa..3d555343fe 100644 --- a/hermes_cli/codex_models.py +++ b/hermes_cli/codex_models.py @@ -2,7 +2,6 @@ from __future__ import annotations -import hashlib import json import logging import os @@ -100,23 +99,28 @@ def _drop_undiscovered_astra(model_ids: List[str]) -> List[str]: def codex_catalog_credential_identity() -> str: - """Stable, non-secret identity of the credential used for live discovery. + """Identity of the credential live discovery would use right now, for the catalog cache key. - OAuth access and refresh tokens rotate in place, while the account-scoped - model catalog remains authoritative for the same ChatGPT principal. Unknown - token formats fall back to a token digest so a credential replacement still - invalidates the cache conservatively. + Access/refresh tokens rotate in place while the account-scoped catalog stays authoritative for + the same ChatGPT principal, so the key is ``(chatgpt_account_id, sub)``, not the token. An + expired token is its own state: ``_codex_catalog`` serves the static fallback for it, and that + fallback must not outlive the refresh under the healthy principal's key. Opaque non-JWT tokens + fall back to the token itself (the caller hashes every part before anything is persisted). """ - try: - from agent.credential_pool import _codex_principal_identity - from hermes_cli.auth import resolve_codex_runtime_credentials + from hermes_cli.auth import _codex_access_token_is_expiring, resolve_codex_runtime_credentials + try: token = str(resolve_codex_runtime_credentials(read_only=True).get("api_key") or "") - except Exception: + except Exception: # AuthError (no/exhausted creds) or the pytest seat belt: no live catalog either way + token = "" + if not token: return "missing" + if _codex_access_token_is_expiring(token, 0): + return "expired" + from agent.credential_pool import _codex_principal_identity + principal = _codex_principal_identity(token) - basis = json.dumps(principal, separators=(",", ":")).encode() if principal else token.encode() - return hashlib.blake2b(basis, digest_size=8).hexdigest() if basis else "missing" + return "/".join(principal) if principal else token def _ranked_slugs(entries: object) -> List[str]: diff --git a/tests/hermes_cli/test_model_cache_swr.py b/tests/hermes_cli/test_model_cache_swr.py index 02cae1af68..2830f8338e 100644 --- a/tests/hermes_cli/test_model_cache_swr.py +++ b/tests/hermes_cli/test_model_cache_swr.py @@ -95,7 +95,7 @@ class TestProviderModelsSWR: ): import hermes_cli.models as mod - def jwt(account_id, subject, nonce): + def jwt(account_id, subject, nonce, exp=None): def segment(value): raw = json.dumps(value, separators=(",", ":")).encode() return base64.urlsafe_b64encode(raw).rstrip(b"=").decode() @@ -104,6 +104,7 @@ class TestProviderModelsSWR: "sub": subject, "nonce": nonce, "https://api.openai.com/auth": {"chatgpt_account_id": account_id}, + **({"exp": exp} if exp is not None else {}), } return f"{segment({'alg': 'none'})}.{segment(claims)}.sig" @@ -148,6 +149,15 @@ class TestProviderModelsSWR: assert mod.cached_provider_model_ids("openai-codex", non_blocking=True) == [] spawn.assert_called_once_with("openai-codex") + # An expired token only ever yields the static fallback (no Astra); the refresh to a live + # token for the same principal must bust that row instead of serving it for the whole TTL. + write_auth(jwt("account-b", "user-b", "stale", exp=time.time() - 60), 0, 5_000_000_000) + mod.update_provider_cache_entry("openai-codex", ["gpt-5.6-sol"]) + write_auth(jwt("account-b", "user-b", "fresh", exp=time.time() + 3600), 0, 6_000_000_000) + with patch.object(mod, "_spawn_swr_refresh") as spawn: + assert mod.cached_provider_model_ids("openai-codex", non_blocking=True) == [] + spawn.assert_called_once_with("openai-codex") + def test_force_refresh_bypasses_swr(self): import hermes_cli.models as mod