fix(aux-client): resolve named-provider credential when base_url overrides endpoint (#16290) by briandevans · Pull Request #16308 · NousResearch/hermes-agent

briandevans · 2026-04-27T02:13:16Z

Summary

When auxiliary.vision.provider: zai (or any PROVIDER_REGISTRY provider) is set alongside auxiliary.vision.base_url, the credential pool for zai was never consulted — the api_key silently fell through to OPENAI_API_KEY or \"no-key-required\", causing a 401 at the API.
_resolve_task_provider_model() now looks up the named provider's credentials when cfg_base_url is set and cfg_api_key is empty.
Explicit api_key in config.yaml always takes precedence. provider: custom and provider: auto are excluded from the lookup.

The bug

_resolve_task_provider_model() returns "custom" as the provider name whenever cfg_base_url is set, discarding cfg_provider. Downstream, resolve_provider_client("custom", ..., explicit_api_key="") has no knowledge of the original "zai" intent and falls through to OPENAI_API_KEY or "no-key-required" — producing a 401 even when ZAI_API_KEY is correctly configured in .env.

The fix

In the config-reading branch of _resolve_task_provider_model(), when cfg_base_url is set and cfg_api_key is blank, attempt a one-shot resolve_api_key_provider_credentials(cfg_provider) lookup for the named provider. The resolved key is then forwarded as explicit_api_key into resolve_provider_client("custom", ...), which uses it directly for the custom endpoint.

The change is isolated to the config-read path (the if task: block) and applies uniformly to all auxiliary task types (vision, compression, review, etc.).

Test plan

Before: _resolve_task_provider_model("vision") with provider=zai, base_url=..., api_key="" returns api_key=None → 401
After: returns api_key="zai-test-key-123" (the env-var value)
Regression guard: reverted fix → test_named_provider_with_base_url_resolves_env_key fails; restored → 6 tests pass
Explicit api_key in config is never overwritten (test_explicit_api_key_in_config_takes_precedence)
Unknown providers (not in PROVIDER_REGISTRY) return None without raising (test_unknown_provider_in_registry_leaves_key_none)
provider: custom skips the registry lookup (test_custom_provider_name_skips_registry_lookup)
Missing credentials return None gracefully (test_missing_credentials_do_not_raise)
Fix applies to non-vision tasks too (test_fix_applies_to_non_vision_tasks_too)
Full adjacent suite: 160 passed, 0 failures (tests/agent/test_auxiliary_client.py, test_auxiliary_config_bridge.py, test_auxiliary_named_custom_providers.py, test_auxiliary_main_first.py, test_vision_resolved_args.py, test_minimax_auxiliary_url.py)

Pull request overview

Fixes auxiliary client credential resolution when a named provider is configured with a custom base_url but no explicit api_key, ensuring the named provider’s credential pool is consulted instead of silently falling back to OPENAI_API_KEY / "no-key-required".

Changes:

Update _resolve_task_provider_model() to attempt a credential lookup for the configured named provider when cfg_base_url is set and api_key is blank.
Add regression tests covering named provider + base_url credential resolution, precedence of explicit config keys, and behavior for unknown/custom providers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`agent/auxiliary_client.py`	Adds conditional credential lookup for named providers when a task-level `base_url` override is present.
`tests/agent/test_aux_vision_named_provider_key.py`	Introduces regression tests for the named-provider + custom `base_url` credential resolution behavior.

Comments suppressed due to low confidence (1)

tests/agent/test_aux_vision_named_provider_key.py:137

The new tests cover API-key providers and unknown/custom providers, but they don't cover the case where cfg_provider is in PROVIDER_REGISTRY yet not an API-key provider (e.g. nous, openai-codex, copilot-acp). Adding a test that sets provider to one of these with base_url set and asserts _resolve_task_provider_model() does not raise (and leaves api_key as None) would protect against regressions in the new lookup logic.

    def test_fix_applies_to_non_vision_tasks_too(self, monkeypatch):
        """Same fix applies to compression/review/other auxiliary tasks."""
        from agent.auxiliary_client import _resolve_task_provider_model

        monkeypatch.setattr(
            "agent.auxiliary_client._get_auxiliary_task_config",
            lambda task: _make_task_config("deepseek", "https://api.deepseek.com/v1") if task == "compression" else {},
        )

        fake_creds = {"api_key": "deepseek-key-456"}
        with patch(
            "hermes_cli.auth.resolve_api_key_provider_credentials",
            return_value=fake_creds,
        ):
            provider, model, base_url, api_key, api_mode = _resolve_task_provider_model("compression")

        assert provider == "custom"
        assert base_url == "https://api.deepseek.com/v1"
        assert api_key == "deepseek-key-456"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T02:16:42Z

+                        creds = resolve_api_key_provider_credentials(cfg_provider)
+                        resolved_key = str(creds.get("api_key", "")).strip() or None


cfg_provider in PROVIDER_REGISTRY is not sufficient before calling resolve_api_key_provider_credentials(): that function raises AuthError for providers whose auth_type is not api_key (e.g. nous, openai-codex, copilot-acp). With auxiliary.<task>.base_url set and api_key blank, this code path can raise and break auxiliary routing. Gate the call on PROVIDER_REGISTRY[cfg_provider].auth_type == "api_key" and/or catch AuthError (or a broader exception) so non-api-key providers simply leave resolved_key as None.

Suggested change

creds = resolve_api_key_provider_credentials(cfg_provider)

resolved_key = str(creds.get("api_key", "")).strip() or None

provider_config = PROVIDER_REGISTRY[cfg_provider]

if getattr(provider_config, "auth_type", None) == "api_key":

try:

creds = resolve_api_key_provider_credentials(cfg_provider)

resolved_key = str(creds.get("api_key", "")).strip() or None

except Exception:

pass

Copilot · 2026-04-27T02:16:42Z

+                    if cfg_provider in PROVIDER_REGISTRY:
+                        creds = resolve_api_key_provider_credentials(cfg_provider)


This lookup is case-sensitive (if cfg_provider in PROVIDER_REGISTRY:), but cfg_provider is taken verbatim from config (only .strip()). If a user sets provider: ZAI (or any non-lowercase variant), downstream resolve_provider_client() would normalize it, but this pre-lookup will fail and resolved_key stays None, reintroducing the 401. Consider normalizing to lowercase for the registry membership check and credential resolution call.

Suggested change

if cfg_provider in PROVIDER_REGISTRY:

creds = resolve_api_key_provider_credentials(cfg_provider)

registry_provider = cfg_provider.lower()

if registry_provider in PROVIDER_REGISTRY:

creds = resolve_api_key_provider_credentials(registry_provider)

briandevans · 2026-04-27T03:08:06Z

@copilot Both findings addressed in commit 1a3623af:

Finding 1 (case-sensitivity, line 2693): cfg_provider was taken verbatim from config without lowercasing. Added norm_provider = cfg_provider.lower() so ZAI, Zai, etc. all resolve correctly to zai in the registry.

Finding 2 (auth_type guard, line 2694): Gated the resolve_api_key_provider_credentials() call on pconfig.auth_type == "api_key" before calling. Non-api-key providers (nous, openai-codex, copilot-acp, bedrock) now silently return api_key=None instead of raising AuthError.

Two new tests: test_uppercase_provider_name_normalizes_to_registry_key and test_non_api_key_provider_does_not_raise.

…rides endpoint (NousResearch#16290) When auxiliary.vision.provider is set to a named provider (e.g. zai) and auxiliary.vision.base_url is also set, _resolve_task_provider_model() returned "custom" with an empty api_key, causing resolve_provider_client() to fall through to OPENAI_API_KEY or "no-key-required" and receive a 401. Consult the provider's PROVIDER_REGISTRY credential pool when cfg_base_url is set and cfg_api_key is empty and cfg_provider is a known named provider. The explicit api_key in config.yaml always takes precedence; "custom" and "auto" provider names are excluded from the lookup. The fix applies to all auxiliary tasks (vision, compression, review, etc.) via the shared helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…oviders Addresses two Copilot inline findings on NousResearch#16308: 1. cfg_provider is read verbatim from config (only .strip()), so a user writing `ZAI` or `Zai` caused a silent miss in PROVIDER_REGISTRY and left resolved_key=None. Normalise to lowercase before the lookup. 2. resolve_api_key_provider_credentials() raises AuthError for providers whose auth_type != "api_key" (nous, openai-codex, copilot-acp, bedrock). Gate the call on pconfig.auth_type == "api_key" so non-api-key providers silently return api_key=None instead of raising. Two new tests added: - test_uppercase_provider_name_normalizes_to_registry_key - test_non_api_key_provider_does_not_raise Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briandevans · 2026-05-18T00:11:08Z

Closing to keep the queue clean — happy to reopen if this is still useful.

Copilot AI review requested due to automatic review settings April 27, 2026 02:13

Copilot started reviewing on behalf of briandevans April 27, 2026 02:13 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

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/config Config system, migrations, profiles labels Apr 27, 2026

briandevans and others added 2 commits April 30, 2026 13:10

briandevans force-pushed the fix/aux-vision-provider-key-lookup-16290 branch from 1a3623a to df26e00 Compare April 30, 2026 20:11

brucecbi mentioned this pull request May 13, 2026

[Bug]: api_key_env alias silently ignored in fallback_providers entries — falls through to "no-key-required" placeholder #25091

Closed

briandevans closed this May 18, 2026

This was referenced May 29, 2026

fix(auxiliary): preserve named provider identity when base_url is passed to _resolve_task_provider_model #34652

Closed

fix(aux): resolve key_env/api_key_env in auxiliary task config #36080

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(aux-client): resolve named-provider credential when base_url overrides endpoint (#16290)#16308

fix(aux-client): resolve named-provider credential when base_url overrides endpoint (#16290)#16308
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/aux-vision-provider-key-lookup-16290

briandevans commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

briandevans commented Apr 27, 2026

Uh oh!

briandevans commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		creds = resolve_api_key_provider_credentials(cfg_provider)
		resolved_key = str(creds.get("api_key", "")).strip() or None

		if cfg_provider in PROVIDER_REGISTRY:
		creds = resolve_api_key_provider_credentials(cfg_provider)

-                    if cfg_provider in PROVIDER_REGISTRY:
-                        creds = resolve_api_key_provider_credentials(cfg_provider)
+                    registry_provider = cfg_provider.lower()
+                    if registry_provider in PROVIDER_REGISTRY:
+                        creds = resolve_api_key_provider_credentials(registry_provider)

Conversation

briandevans commented Apr 27, 2026

Summary

The bug

The fix

Test plan

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

briandevans commented Apr 27, 2026

Uh oh!

briandevans commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants