Skip to content

fix(auth): return exhausted credential key instead of empty string - #40961

Open
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/credential-pool-exhausted-empty-key
Open

fix(auth): return exhausted credential key instead of empty string#40961
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:fix/credential-pool-exhausted-empty-key

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

Summary

Fixes #40960

When all credential pool entries are in exhaustion cooldown (e.g. after 429), _resolve_api_key_provider_secret() returns an empty api_key via pool.peek() → None → ("", ""). The empty key causes a misleading 401 Unauthorized from the upstream API instead of the real 429/402 error.

Changes

hermes_cli/auth.py — After pool.peek() returns None (all entries in cooldown), fall through to iterate pool._entries and return the first entry's key with a logger.warning. This ensures the upstream API returns the actual error code and message.

Before

429 quota exhausted → key marked exhausted → next session: peek() = None → api_key="" → 401 Unauthorized ❌

After

429 quota exhausted → key marked exhausted → next session: peek() = None → use exhausted key anyway → 429 quota exhausted ✅ (clear message)

Testing

  • All 112 credential pool tests pass (python -m pytest tests/ -k credential_pool)
  • The one pre-existing failure in test_anthropic_adapter.py::TestRunOauthSetupToken is unrelated (MagicMock type issue)

…en all pool entries are in cooldown

When all credential pool entries are in exhaustion cooldown (e.g. after
429 quota exhausted), _resolve_api_key_provider_secret() returns an empty
api_key via pool.peek() -> None -> "", "". The empty key causes a
misleading 401 Unauthorized from the upstream API instead of a clear
429/402 error.

Fix: after peek() returns None, fall through to the first raw pool entry
and return its key anyway. This ensures the real upstream error surfaces
so users see the actual cause (quota exhausted, billing issue) rather
than a confusing auth failure.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 7, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing a live resolver failure. Current main still has the reported path: hermes_cli/auth.py:600-609 calls pool.peek() and returns an empty key when no entry is available.

Problems

  • hermes_cli/auth.py:609 iterates all raw pool entries, including STATUS_DEAD credentials. agent/credential_pool.py:59-64 and 1467-1496 define those as permanently excluded from use; this fallback would bypass that guarantee and can resend a revoked/invalidated credential.
  • The PR adds no regression coverage for the peek() is None path. Existing tests at tests/tools/test_credential_pool_env_fallback.py:181-214 cover only a normal selected entry and an empty pool.

Suggested changes

  • Gate the fallback on entry is None and restrict it to explicitly STATUS_EXHAUSTED entries, never STATUS_DEAD.
  • Add tests for all-exhausted returning a key and all-DEAD remaining unresolved.

This is an automated hermes-sweeper review.

Comment thread hermes_cli/auth.py
"credential pool: all %s entries exhausted; "
"using %s anyway for clearer upstream error",
provider_id, e.label or e.id[:8],
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raw loop also considers STATUS_DEAD entries. CredentialPool._available_entries() deliberately excludes DEAD credentials permanently (agent/credential_pool.py:1467-1496), so restrict this fallback to entries explicitly marked exhausted; otherwise a revoked/invalidated credential can be sent again.

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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] credential pool exhaustion causes misleading 401 instead of real 429/402

3 participants