Skip to content

fix(agent): resolve credential pool key ambiguity for shared base_url#19682

Closed
Cyrene963 wants to merge 1 commit into
NousResearch:mainfrom
Cyrene963:fix/credential-pool-key-mixup
Closed

fix(agent): resolve credential pool key ambiguity for shared base_url#19682
Cyrene963 wants to merge 1 commit into
NousResearch:mainfrom
Cyrene963:fix/credential-pool-key-mixup

Conversation

@Cyrene963

@Cyrene963 Cyrene963 commented May 4, 2026

Copy link
Copy Markdown

Summary

Fixes #19083

When multiple custom_providers share the same base_url but have different api_key values, get_custom_provider_pool_key() previously returned the pool key for the first match in iteration order. This caused the wrong credential pool to be loaded, resulting in 401 Unauthorized errors.

Root Cause

get_custom_provider_pool_key(base_url) only matched by base_url, ignoring which provider name was actually requested. Two providers with the same base_url always resolved to the first one's pool key.

Fix

Three targeted changes:

  1. agent/credential_pool.py — get_custom_provider_pool_key(): Added optional provider_name parameter. When provided (and not bare "custom"), matches by provider name first for disambiguation. Falls back to base_url matching when name is not available.

  2. hermes_cli/runtime_provider.py — _try_resolve_from_custom_pool(): Passes provider_name through to get_custom_provider_pool_key() so named provider lookups are unambiguous.

  3. agent/credential_pool.py — _seed_custom_pool(): Replaced indirect get_custom_provider_pool_key(model_base_url) call with direct comparison against the current pool_key's own config entry, eliminating first-match ambiguity entirely for this path.

Testing

  • All 41 existing test_credential_pool.py tests pass
  • All 109 existing test_runtime_provider_resolution.py tests pass

Closes #19083

fix(agent): match custom provider by name to resolve pool key ambiguity
1a2f9f7 
When multiple custom_providers share the same base_url,
get_custom_provider_pool_key() previously returned the first match,
causing the wrong credential pool to be loaded.

Changes:
- Add optional provider_name parameter to get_custom_provider_pool_key()
  for name-based disambiguation
- Update _try_resolve_from_custom_pool() to pass provider_name through
- Fix _seed_custom_pool() to compare base_url against the specific
  pool_key's config instead of calling get_custom_provider_pool_key()

Fixes NousResearch#19083
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent loop, run_agent.py, prompt builder area/auth Authentication, OAuth, credential pools labels May 4, 2026
@alt-glitch

alt-glitch commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Likely duplicate of #19122 — same disambiguation fix (add provider_name param to get_custom_provider_pool_key). Also see #19094. All address #19083/#14141.

@alt-glitch

alt-glitch commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Likely duplicate of #19122

@Cyrene963

Cyrene963 commented May 4, 2026

Copy link
Copy Markdown
Author

Closing as duplicate of #19122 (Bartok9) — same fix for credential pool key disambiguation. Their PR was submitted earlier.

Local patch remains active in Cyrene963/hermes-patches until upstream resolves #19083.

@Cyrene963 Cyrene963 closed this May 4, 2026
@Cyrene963 Cyrene963 reopened this May 25, 2026
@Cyrene963

Cyrene963 commented May 25, 2026

Copy link
Copy Markdown
Author

Re-evaluating closure status for #19682

I closed this as a duplicate earlier, but I rechecked the referenced upstream PR(s) and none of them are merged yet:

Because the underlying fix does not appear to have landed upstream, closing this solely as a duplicate may have been premature. I am reopening this PR so it can remain trackable unless maintainers prefer a different canonical PR.

@teknium1

teknium1 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

This looks implemented on current main by the later merged salvage PR #21219, so this duplicate PR can be closed by the automated hermes-sweeper review.

Evidence:

  • agent/credential_pool.py:378 now defines get_custom_provider_pool_key(base_url, provider_name=None) and prefers a normalized provider-name match before falling back to base_url matching.
  • hermes_cli/runtime_provider.py:445 threads provider_name through _try_resolve_from_custom_pool, and hermes_cli/runtime_provider.py:739 passes the named custom provider's name into that lookup.
  • tests/agent/test_credential_pool.py:2041 covers the reported repro: two custom providers with the same base_url, with provider_name="provider-b" resolving to custom:provider-b.
  • Merged PR fix(credential_pool): resolve key mix-up when custom providers share base_url (salvage #19094) #21219 landed as merge commit e38ea38079b8683fba48a245c19ff5a2a8f50d39 and explicitly fixed Credential pool key mix-up when multiple custom providers share the same base_url #19083; git merge-base --is-ancestor e38ea38079b8683fba48a245c19ff5a2a8f50d39 HEAD confirms it is contained in current main.

Thanks for keeping this trackable after the earlier duplicate PRs were closed unmerged.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent loop, run_agent.py, prompt builder P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Credential pool key mix-up when multiple custom providers share the same base_url

3 participants

@Cyrene963 @alt-glitch @teknium1