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).
This commit is contained in:
kshitij
2026-09-12 11:55:00 +05:30
parent fe6351000f
commit ea42884e99
2 changed files with 30 additions and 20 deletions

View File

@@ -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]]:

View File

@@ -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
# ---------------------------------------------------------------------------