fix(auth): harden Codex credential pool recovery - #31032
Conversation
|
CI follow-up: the first run exposed a real missing helper in Local verification after the fix:
|
8398204 to
03f47be
Compare
|
Nix Ubuntu failed on a transient GitHub API 401 while fetching a public flake input ( |
|
Reviewed this PR focusing on security, error handling, race conditions, and test coverage. Overall: well-structured fix. The credential_id threading through the recovery pipeline is clean. No secrets leak into logs (entries identified by label/id prefix only). JWT claim decoding for quota identity is purely local -- no network exposure. One WARNING: Positive notes: (1) Co-Authored-By: Claude Opus 4.7 noreply@anthropic.com |
|
CI is green after the no-diff retrigger: tests, ruff/ty, Nix on Ubuntu/macOS, supply-chain, history/attribution, docs/docker jobs all pass on |
03f47be to
451f388
Compare
|
Rebased on current Changes after rebase:
Local verification:
CI has been retriggered on |
|
Follow-up from the live Codex pool reproduction on vps_new: Responses/Codex returned a concrete future reset window for Pushed
Validation:
|
|
Follow-up from live production repro on 2026-05-26:
Production gateway has the same fix cherry-picked as 14c149340 and the stale pool entries were reconciled; #2 and #5 are back in rotation. |
bca95db to
ad4bff3
Compare
3864a60 to
00fcf09
Compare
00fcf09 to
ee4bac5
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the thorough recovery coverage. The central premise is still present on current main: hermes_cli/runtime_provider.py:1698-1705 leaves an exhausted pool with no selected entry, then :1784-1795 falls through to singleton Codex auth. The live-usage reconciliation and exact-entry recovery work therefore remain valuable.
Problems
agent/credential_pool.py:379addsHERMES_CODEX_USAGE_TIMEOUT_SECONDS. The project policy atAGENTS.md:102-106reserves.env/HERMES_*for secrets; behavioral timeouts belong inconfig.yaml. Given the code caps this probe at two seconds, a fixed internal constant appears sufficient.tests/agent/test_credential_pool.py:18and:38both define_jwt_with_claims; the latter silently overwrites the former.
Suggested changes
- Remove the new environment-variable configuration path, or wire a documented config setting if it must be user-configurable.
- Deduplicate the test helper before salvage.
Automated hermes-sweeper review.
ee4bac5 to
30615ce
Compare
|
Queue hygiene note after portfolio refresh (2026-07-19): This PR is still valuable, but it is the broadest dirty item in my open set ( I refreshed the narrower dirty PRs first. I’ll do a dedicated rebase/salvage pass for this one next rather than force a risky mega-resolve in the same batch. |
Rebase the still-needed Codex pool recovery behavior onto current main: - stop exhausted pools falling through to singleton OAuth auth - reconcile stale Codex exhaustion via live usage probes - keep concrete usage-limit reset windows authoritative until they elapse - clear cooldowns when usage reports allowed=true - group shared workspace members by (chatgpt_account_id, JWT sub) - mark exact credentials via credential_pool_entry_id recovery plumbing - surface DEAD credentials in hermes auth list
30615ce to
ea30288
Compare
|
Dedicated current-main salvage/rebase completed in The three conflicts were resolved by preserving current
Independent verification after conflict resolution:
|
When credential selection runs under a live asyncio loop, offload Codex live-usage reconciliation to a worker and clear stale pool entry IDs on provider rebinds so recovery targets the active credential.
Late findings follow-upPushed
Local verification: credential-pool + switch/fallback/restore suites green. |
Keep CredentialPool.select synchronous so a live Codex usage probe can recover an entry before callers decide the pool is exhausted. Offload all async gateway runtime resolution instead, and bind switched/fallback pools to the exact entry that supplied the resolved API key.
Final independent-review follow-upPushed
Local verification: 189 passed; ruff clean. |
|
Fresh production reproduction of the stale Codex exhaustion path on Hermes v0.19.0 (2026.7.20), upstream Observed
Recovery and verificationI ran the supported reset: hermes auth reset openai-codex
# Reset status on 1 openai-codex credentialsI then copied the config/auth store into an isolated temporary hermes chat -q 'Reply with exactly CODEX_OK and nothing else.' \
--provider openai-codex -m gpt-5.6-solIt returned: No reauthentication or token replacement was required. Clearing only the persisted exhaustion fields restored the same OAuth credential immediately. Why this is useful evidence for this PRThis independently reproduces the false-positive exhaustion case described in the PR: a concrete future The manual workaround is effective but non-obvious: hermes auth reset openai-codexThis reproduction supports the PR's live-usage reconciliation work and its requirement that an explicit usable/allowed result clear stale persisted cooldown state. It also demonstrates direct billing impact when the configured fallback is a metered provider. |
What does this PR do?
Fixes a cluster of Codex credential-pool failure modes where Hermes can keep reporting
openai-codexas exhausted even after a usable pooled credential exists.The root cause is that pool exhaustion state is persistent, but runtime recovery had a few gaps:
usage_limit_reachedstatus could stay persisted incorrectly when no concrete reset window exists, while concrete future reset timestamps from failed model requests must remain authoritative;chatgpt_account_idcan identify a workspace/account, not the user-level usage bucket, so workspace members need to be grouped by(chatgpt_account_id, JWT sub)for quota propagation.This is intentionally a single draft PR because the final workspace-member quota fix depends on the live-usage reconciliation path introduced earlier in this stack.
Related Issue
Related to #27448 and #30802. No separate issue yet; this was reproduced from live
openai-codexpool state and covered with regression tests.Type of Change
Changes Made
hermes_cli/runtime_provider.py: report exhausted pools explicitly instead of silently falling through to singleton OAuth, and include Codex duplicate quota identity diagnostics.hermes_cli/auth_commands.py: surface DEAD credentials inhermes auth listinstead of making them look selectable.agent/credential_pool.py: reconcile exhausted Codex entries against live usage, keep concrete 429 reset windows authoritative until their reset timestamps elapse, quarantine invalidated Codex tokens, and group shared workspace entries by quota identity.agent/agent_init.py,run_agent.py,gateway/run.py, cron/TUI/CLI helpers): preservecredential_pool_entry_idso 429 recovery marks the exact credential used by the failed request.How to Test
/usr/local/lib/hermes-agent/venv/bin/python -m pytest -q -o addopts='' tests/agent/test_credential_pool.py tests/hermes_cli/test_runtime_provider_resolution.py tests/hermes_cli/test_auth_commands.py tests/run_agent/test_run_agent.py::TestCredentialPoolRecovery/usr/local/lib/hermes-agent/venv/bin/python -m ruff check agent/credential_pool.py hermes_cli/runtime_provider.py tests/agent/test_credential_pool.py tests/hermes_cli/test_runtime_provider_resolution.py tests/hermes_cli/test_auth_commands.py tests/run_agent/test_run_agent.py agent/agent_init.py agent/agent_runtime_helpers.py gateway/run.pyatobarrientos@gmail.comandaashish44khanal@gmail.com (2)together because they sharedchatgpt_account_id=1538301a-...; live usage showed differentuser_id/usage windows, and after this changehermes auth list openai-codexselected the usable entry instead of reporting all 8 pool entries unavailable.Checklist
Code
pytest tests/ -qand all tests passDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AScreenshots / Logs
Focused local validation: