From ea42884e99bcd8c04fe2cb8098f86929ced794e9 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:55:00 +0530 Subject: [PATCH] refactor(auth): codex cooldown clear reuses the pool ownership rule clear_codex_pool_quota_cooldowns decided "borrow the root?" inside a nested closure via a tri-state Optional[int] return (None = no rows), which forced `cleared or 0` and duplicated the rule persist_pool_entries already owns. Decide once with _profile_owns_pool_provider + _borrowed_single_use_pool_root, then lock/load/clear/save exactly one store. Behaviour is unchanged for every (mode x profile rows x root rows) cell; the pre-lock decision races only a concurrent `hermes auth add` in the profile, whose fresh rows carry no cooldown. Adds the missing negative invariant: a profile that OWNS Codex rows never has the root store touched (0 cleared, root byte-identical). --- hermes_cli/auth_codex.py | 28 ++++++------------- .../hermes_cli/test_auth_profile_fallback.py | 22 +++++++++++++++ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/hermes_cli/auth_codex.py b/hermes_cli/auth_codex.py index a2b02611a0..584ef738fb 100644 --- a/hermes_cli/auth_codex.py +++ b/hermes_cli/auth_codex.py @@ -579,18 +579,16 @@ def clear_codex_pool_quota_cooldowns(access_token: Optional[str] = None) -> int: rate-limited entry does (a redeemed banked reset restores the whole account; a still-exhausted entry just re-freezes with fresh metadata on its next 429). """ - from agent.credential_pool import _borrowed_single_use_pool_root + from agent.credential_pool import _borrowed_single_use_pool_root, _profile_owns_pool_provider from hermes_cli.auth import _auth_store_lock, _load_auth_store, _save_auth_store - - def _clear_in(target: Optional[Path]) -> Optional[int]: - """Clear inside *target* (None = active store); None when that store has no codex rows.""" - cleared = 0 + cleared = 0 + try: + # Same owner rule as ``persist_pool_entries``: a profile with no Codex rows of its own + # borrows the global-root pool, so the cooldown must clear where the rows actually live. + target = None if _profile_owns_pool_provider("openai-codex") else _borrowed_single_use_pool_root() with _auth_store_lock(target_path=target): auth_store = _load_auth_store(target) - entries = _pool_entries(auth_store, "openai-codex") - if not entries: - return None - for entry in _codex_pool_dicts(entries): + for entry in _codex_pool_dicts(_pool_entries(auth_store, "openai-codex")): if access_token and str(entry.get("access_token") or "") != access_token: continue if _entry_is_rate_limit_exhausted(entry): @@ -598,19 +596,9 @@ def clear_codex_pool_quota_cooldowns(access_token: Optional[str] = None) -> int: cleared += 1 if cleared: _save_auth_store(auth_store, target_path=target) - return cleared - - try: - cleared = _clear_in(None) - if cleared is None: - # No rows of its own: this profile borrows the global-root pool (the same fallback - # ``read_credential_pool`` reads through), so the cooldown must clear where the rows live. - root = _borrowed_single_use_pool_root() - cleared = _clear_in(root) if root is not None else 0 - return cleared or 0 except Exception: logger.debug("Failed to clear Codex pool quota cooldowns", exc_info=True) - return 0 + return cleared def _codex_pool_dicts(entries: Optional[List[Any]]) -> Iterator[Dict[str, Any]]: diff --git a/tests/hermes_cli/test_auth_profile_fallback.py b/tests/hermes_cli/test_auth_profile_fallback.py index 73a8bbcf2a..60a58eae40 100644 --- a/tests/hermes_cli/test_auth_profile_fallback.py +++ b/tests/hermes_cli/test_auth_profile_fallback.py @@ -205,6 +205,28 @@ def test_codex_cooldown_clear_writes_to_the_store_that_owns_the_borrowed_pool(pr assert root_rows[0].get("last_error_reset_at") is None +def test_codex_cooldown_clear_never_touches_root_when_profile_owns_rows(profile_env): + """A profile with its own Codex rows is the owner: the root's cooldown state is not ours to + clear, even when none of the profile's rows are exhausted (0 cleared, root byte-identical).""" + from hermes_cli.auth_codex import clear_codex_pool_quota_cooldowns + + root_file = profile_env["global"] / "auth.json" + _write(root_file, _make_auth_store(pool={ + "openai-codex": [{"id": "glob", "auth_type": "oauth", "priority": 0, + "access_token": "global-codex-access-token", "refresh_token": "r", + "last_status": "exhausted", "last_error_reason": "rate_limit", + "last_error_reset_at": 4_102_444_800}], + })) + _write(profile_env["profile"] / "auth.json", _make_auth_store(pool={ + "openai-codex": [{"id": "prof", "auth_type": "oauth", "priority": 0, + "access_token": "profile-codex-access-token", "refresh_token": "r"}], + })) + before = root_file.read_bytes() + + assert clear_codex_pool_quota_cooldowns() == 0 + assert root_file.read_bytes() == before + + # --------------------------------------------------------------------------- # Classic mode — no fallback path should ever trigger # ---------------------------------------------------------------------------