Skip to content

fix: preserve known provider when base_url is set in auxiliary task resolution - #39732

Closed
tino-chen wants to merge 1 commit into
NousResearch:mainfrom
tino-chen:fix/auxiliary-vision-provider-routing
Closed

fix: preserve known provider when base_url is set in auxiliary task resolution#39732
tino-chen wants to merge 1 commit into
NousResearch:mainfrom
tino-chen:fix/auxiliary-vision-provider-routing

Conversation

@tino-chen

Copy link
Copy Markdown

Summary

Fixes a bug where auxiliary tasks (vision, compression, etc.) fail with 401 authentication errors when both provider and base_url are configured for a known provider (e.g., alibaba with DashScope).

Problem

When auxiliary.vision config has both provider: alibaba and base_url: https://dashscope.aliyuncs.com/..., vision calls fail with:

Error code: 401 - {'error': {'message': 'Incorrect API key provided', ...}}

Root Cause

A double-resolution trap in _resolve_task_provider_model:

  1. call_llm calls _resolve_task_provider_model once → returns ("alibaba", model, base_url, ...)
  2. call_llm passes the resolved base_url as an explicit argument to resolve_vision_provider_client
  3. resolve_vision_provider_client calls _resolve_task_provider_model again with base_url as an explicit argument
  4. The second call sees base_url is present and unconditionally routes to "custom"
  5. The "custom" provider path reads OPENAI_API_KEY instead of DASHSCOPE_API_KEY
  6. DashScope receives an invalid API key and returns 401

Fix

In _resolve_task_provider_model, when both provider (explicit) and base_url (explicit) are set, preserve the provider name if it's a known PROVIDER_REGISTRY entry instead of routing to "custom". The resolve_provider_client function already handles explicit_base_url correctly for known providers (line ~3746).

if base_url:
    # NEW: preserve known provider to avoid double-resolution trap
    if provider and provider not in {"", "auto", "custom"}:
        return provider, resolved_model, base_url, api_key, resolved_api_mode
    return "custom", resolved_model, base_url, api_key, resolved_api_mode

Testing

Added 5 regression tests in TestResolveTaskProviderModelPreservesKnownProvider:

  • ✅ Known provider + base_url → provider preserved (the bug fix)
  • ✅ Auto + base_url → custom (unchanged behavior)
  • ✅ No provider + base_url → custom (unchanged behavior)
  • ✅ Direct API alias (e.g., 'openai') → custom (unchanged behavior)
  • ✅ Config-derived provider + base_url → provider preserved (the bug fix)

All 198 existing tests pass (excluding async tests that require pytest-asyncio plugin).

Impact

This fix resolves authentication failures for users who configure auxiliary tasks with both provider and base_url for known providers like:

  • alibaba (DashScope)
  • deepseek
  • gemini
  • anthropic
  • etc.

The fix is minimal and backward-compatible: providers not in PROVIDER_REGISTRY (like "auto", "", "custom") still route to "custom" as before.

Workaround

Until this is merged, users can work around the bug by removing base_url from their auxiliary.vision config if they have DASHSCOPE_BASE_URL (or equivalent) set in .env.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels Jun 5, 2026
@sacwooky

Copy link
Copy Markdown

Independent confirmation of this bug + fix from a separate deployment.

We hit the identical issue routing auxiliary vision through a named custom provider (a local OpenAI-compatible router; key_env: NINEROUTER_KEY). Same mechanism you describe: async_call_llm(task="vision") resolves the task config (getting the provider's base_url), then passes that base_url back into resolve_vision_provider_client(), which re-runs _resolve_task_provider_model() with base_url set → the named provider is demoted to bare "custom"key_env is dropped → resolve_provider_client() falls back to the no-key-required placeholder → HTTP 401 invalid_api_key on every vision call, even though the main agent loop on the same provider works fine.

Isolated it down to the exact two resolution shapes (same key, same request):

  • no base_url arg → provider preserved → key resolves → 200
  • with base_url arg → provider collapses to custom → key no-key-required401

Your guard at the if base_url: line is the right fix and matches the existing config-derived branch (cfg_base_url + cfg_provider already preserves cfg_provider).

I'd independently written the same one-line guard before finding this PR (#47434, now closed in favor of yours). Offering the extra test coverage from it in case it's useful — it asserts a named custom provider (custom:<name>) is preserved with base_url (your tests cover registry/alibaba; this adds the custom:* case) and that anonymous endpoints (""/auto/custom) still collapse:

class TestResolveTaskProviderModelBaseUrlPreservesNamedProvider:
    def test_named_custom_provider_preserved_with_base_url(self):
        from agent.auxiliary_client import _resolve_task_provider_model
        prov, model, base, key, _ = _resolve_task_provider_model(
            task="vision", provider="custom:9router-codex",
            model="vertex/gemini-2.5-flash",
            base_url="http://127.0.0.1:20128/v1", api_key=None,
        )
        assert prov == "custom:9router-codex"
        assert base == "http://127.0.0.1:20128/v1"

    def test_anonymous_endpoint_still_collapses_to_custom(self):
        from agent.auxiliary_client import _resolve_task_provider_model
        for p in (None, "custom", "auto"):
            prov, *_ = _resolve_task_provider_model(
                task="vision", provider=p, base_url="http://localhost:1234/v1")
            assert prov == "custom"

Verified on Python 3.11: these fail on main and pass with this PR's change; the broader tests/agent/test_auxiliary_client.py suite stays green. +1 to merging this.

@tino-chen
tino-chen force-pushed the fix/auxiliary-vision-provider-routing branch 2 times, most recently from 9d8c09e to 6597a52 Compare June 17, 2026 07:26
…esolution

When both provider (e.g. 'alibaba') and base_url are configured for an
auxiliary task (vision, compression, etc.), _resolve_task_provider_model
was unconditionally routing to 'custom' whenever base_url was present as
an explicit argument. This caused a double-resolution trap:

1. call_llm resolves once → gets ('alibaba', model, base_url, ...)
2. call_llm passes resolved base_url as explicit arg to
   resolve_vision_provider_client
3. resolve_vision_provider_client resolves again with base_url as
   explicit arg → provider becomes 'custom'
4. 'custom' path reads OPENAI_API_KEY instead of DASHSCOPE_API_KEY
5. DashScope returns 401 invalid_api_key

Fix: when provider is explicitly set and survived _expand_direct_api_alias
(i.e. it's a known PROVIDER_REGISTRY entry), preserve the provider name
instead of routing to 'custom'. resolve_provider_client already handles
explicit_base_url for known providers correctly.

Added 5 regression tests covering:
- Known provider + base_url preserved
- Auto + base_url → custom (unchanged)
- No provider + base_url → custom (unchanged)
- Direct API alias (e.g. 'openai') → custom (unchanged)
- Config-derived provider + base_url preserved
@tino-chen
tino-chen force-pushed the fix/auxiliary-vision-provider-routing branch from 6597a52 to d9ef634 Compare June 17, 2026 07:33
@tino-chen

tino-chen commented Jun 17, 2026

Copy link
Copy Markdown
Author

@sacwooky Thanks for the detailed confirmation and the additional test cases — really helpful.

I've added them to the PR alongside our existing tests: custom:<name> preserved with base_url, plus the empty/bare "custom" fallback cases. All 8 tests passing now.

@agy590

agy590 commented Jun 26, 2026

Copy link
Copy Markdown

MiniMax CN reproduction + validation of this fix

Just verified this PR fixes the same bug class for the MiniMax China (minimax-cn) provider — confirmed end-to-end with the upstream main branch and PR #39732 patch applied.

Repro on main (pre-fix)

config.yaml:

auxiliary:
  vision:
    provider: minimax-cn
    model: minimax-m3
    base_url: https://api.minimaxi.com/v1
    api_key: ''

.env has MINIMAX_CN_API_KEY=sk-cp-... (125 chars, verified valid via direct curl https://api.minimaxi.com/v1/text/chatcompletion_v2 → HTTP 200).

vision_analyze call → openai.AuthenticationError: 401 — "login fail: Please carry the API secret key in the 'Authorization' field of the request header (1004)".

Trace through _resolve_task_provider_model:

  1. First call (cfg-only path): cfg_provider=minimax-cn, cfg_base_url=https://api.minimaxi.com/v1, cfg_api_key='' → falls to line 4860 branch → returns ("minimax-cn", "minimax-m3", "https://api.minimaxi.com/v1", None, None)
  2. async_call_llm passes (provider="minimax-cn", base_url="https://api.minimaxi.com/v1") to resolve_vision_provider_client
  3. Second call to _resolve_task_provider_model with explicit args → line 4850 unconditional return "custom", ... → provider identity lost
  4. resolve_provider_client("custom", ..., explicit_api_key=None)OPENAI_API_KEY lookup → empty → "no-key-required" placeholder → 401

With PR #39732 applied

Same config, same .env. vision_analyze returns HTTP 200 with full description. Provider identity preserved through the double-resolution.

Suggested workaround for users blocked waiting for merge

Drop auxiliary.vision.base_url entirely — the provider's default base_url (https://api.minimaxi.com/anthropic for minimax-cn) is the canonical endpoint and api_mode="anthropic_messages" is what the plugin profile already declares:

hermes config set auxiliary.vision.base_url ""

Confirmed working without restarting the gateway — load_config() mtime cache invalidates on file change.

Side note on a subtle variant

The leak cleanup script in my setup had replaced base_url: https://api.minimaxi.com/v1 with base_url: 'null' (literal string). This also triggers the same code path (anything truthy in cfg_base_url works the same way). Worth a defensive check in the cleanup script: when scrubbing a URL, delete the line rather than substitute 'null' — the latter is still truthy and silently regresses into this 401.


Happy to run the PR's test suite (tests/agent/test_auxiliary_client.py) against my repro config if useful for additional coverage. This bug is also tracked in issue #9318 (canonical) and #47741 (vision-specific duplicate).

@teknium1

teknium1 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Closing as implemented on main: _preserve_provider_with_base_url() landed in commit a8841e2 (PR #55605, merged Jun 30) and covers this exact scenario — an explicit first-class provider paired with a base_url now keeps its identity instead of being flattened to custom, so credentials resolve from the right env vars. Verified E2E on current main: provider="alibaba" + base_url stays alibaba; a bare base_url still routes to custom.

You were the earliest submitter on this bug and your double-resolution root-cause analysis was spot on. The merged version additionally gates on the provider catalog so unknown provider names still fall back to custom. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants