Skip to content

fix(auth): stop exhausted pools falling through to singleton auth - #27448

Closed
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/pool-exhaustion-runtime-auth
Closed

fix(auth): stop exhausted pools falling through to singleton auth#27448
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/pool-exhaustion-runtime-auth

Conversation

@Qwinty

@Qwinty Qwinty commented May 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Treat a credential pool with stored entries but no selectable entry as an explicit credential_pool_exhausted auth failure.
  • Stop falling through to singleton Codex/OAuth auth when the configured pool is exhausted, which produced the misleading No Codex credentials stored message.
  • Preserve auto provider fallthrough by skipping the exhausted singleton provider and trying environment/default providers instead.

Context

Related to #12058 and follow-up to #17929. #17929 fixed an init-time fallback path, but runtime provider resolution could still hit pool.has_credentials() == true plus pool.select() is None, then continue into singleton auth and misreport the failure as missing Codex credentials.

Test Plan

  • python -m pytest tests/hermes_cli/test_runtime_provider_resolution.py -q
  • python -m pytest tests/gateway/test_auth_fallback.py tests/gateway/test_session_model_override_routing.py::test_gateway_auth_fallback_uses_fallback_model_from_config -q

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools labels May 17, 2026
@Qwinty

Qwinty commented May 17, 2026

Copy link
Copy Markdown
Contributor Author

CI note: the remaining failing test job appears to be an upstream baseline failure, not introduced by this PR.

This PR is based on upstream/main at f36c89cd5. The Tests / test workflow for that exact upstream SHA failed earlier with the same count and same failure set:

Both report 21 failed and include the same failures, e.g. test_agent_json_version_matches_pyproject, Telegram DM topic metadata assertions, Google Chat/Teams fake aiohttp trust_env, custom-provider model catalog drift, _BUILTIN_SUBCOMMANDS missing send, gateway service permission tests, and the same provider/voice/transcription assertions.

The tests directly covering this PR pass locally:

  • python -m pytest tests/hermes_cli/test_runtime_provider_resolution.py -q112 passed
  • python -m pytest tests/gateway/test_auth_fallback.py tests/gateway/test_session_model_override_routing.py::test_gateway_auth_fallback_uses_fallback_model_from_config -q3 passed

@Qwinty

Qwinty commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up from another live pool investigation: this PR still looks correct for the exhausted-pool -> singleton-auth fallthrough, but I found an adjacent native pool bug that is not covered here.

Using only Hermes' own auth.json pool entries, the Codex usage endpoint can report one pool entry as allowed=true while Hermes still has that entry persisted as last_status=exhausted from an earlier 429. In that state hermes auth list openai-codex and runtime selection can falsely treat the whole pool as unavailable until the stored reset timestamp, even though at least one credential is actually usable.

Two changes are needed separately from this PR:

  • carry the selected credential_pool_entry_id into the agent and mark/refresh that exact entry during recovery, instead of relying on mutable pool.current();
  • for openai-codex, reconcile exhausted entries against the live ChatGPT/Codex usage endpoint before declaring the pool unavailable or rendering auth list.

I have this validated locally: after live reconciliation, hermes auth list openai-codex moved the usable entry out of rate-limited usage_limit_reached and hermes status returned to OpenAI Codex ✓ logged in. I will keep this as a separate follow-up because #27448 is focused on the misleading singleton fallback once the pool is already considered exhausted.

@Qwinty

Qwinty commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Post-rebase CI update: the branch is now rebased on current upstream/main and the merge conflict is resolved; GitHub reports it as mergeable. All non-Tests / test checks passed. The full Tests / test job was cancelled by the workflow timeout at ~95% progress rather than completing with a pytest summary: https://github.com/NousResearch/hermes-agent/actions/runs/26059693081/job/76616431243\n\nLocal focused validation for this auth/runtime-provider PR is green:\n\nbash\nvenv/bin/python -m pytest tests/hermes_cli/test_runtime_provider_resolution.py -q -o 'addopts='\n# 112 passed\n

@Qwinty
Qwinty force-pushed the fix/pool-exhaustion-runtime-auth branch from 083efe4 to 9ed7998 Compare May 21, 2026 21:55
@Qwinty

Qwinty commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

Scope update after another live Codex pool reproduction: this PR still covers the narrow exhausted-pool -> singleton-auth fallthrough bug. I split the broader Codex pool recovery stack into draft PR #31032 because the live stale-pool fix needs additional pieces that are not present in this branch: selected credential id propagation, live usage reconciliation, bounded usage-limit cooldowns, invalid-token quarantine, and quota-identity grouping for shared ChatGPT workspaces.\n\nKeeping this PR narrow is still useful if maintainers prefer incremental merge order. If the preference is one cohesive Codex pool fix, #31032 can supersede this one.

@Qwinty

Qwinty commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing this as superseded by draft PR #31032. This branch was useful as the narrow exhausted-pool -> singleton-auth fallback fix, but it is also the PR that mostly changed the user-facing pool-exhaustion error path. The live Codex issue turned out to need the fuller stack in #31032: selected credential id propagation, live Codex usage reconciliation, bounded usage-limit holds, invalid-token quarantine, and workspace quota-identity grouping. Keeping both open would make review noisier and duplicate the auth surface.

@Qwinty Qwinty closed this May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants