fix(adapter): honor explicit provider config — fail fast when not in registry (#180) #4
101
adapter.py
101
adapter.py
@ -280,13 +280,28 @@ def _project_vendor_auth(provider: dict) -> None:
|
||||
return
|
||||
|
||||
|
||||
def _resolve_provider(model: str, providers: tuple) -> dict:
|
||||
def _resolve_provider(
|
||||
model: str,
|
||||
providers: tuple,
|
||||
explicit_provider: str = None,
|
||||
) -> dict:
|
||||
"""Return the provider entry matching this model id.
|
||||
|
||||
Match is case-insensitive: prefix wins over alias when both could
|
||||
apply. Unknown ids fall back to the first provider in the registry
|
||||
(by convention, the OAuth/safest default — anthropic-oauth in both
|
||||
_BUILTIN_PROVIDERS and the shipped config.yaml).
|
||||
If ``explicit_provider`` is given (set via the ``provider:`` field in
|
||||
workspace config.yaml or runtime_config), look up by name first. If the
|
||||
named provider is not in the registry, RAISE ``ValueError`` with an
|
||||
actionable message — silent fallback to ``providers[0]`` is the bug
|
||||
that motivated #180 (workspace operator picks ``provider: minimax``
|
||||
in the canvas Config tab, the adapter ignores it, the Claude SDK
|
||||
silently keeps using ``CLAUDE_CODE_OAUTH_TOKEN`` and the operator has
|
||||
no way to tell from the canvas that their provider switch did
|
||||
nothing).
|
||||
|
||||
Without an explicit name: match is case-insensitive, prefix wins over
|
||||
alias when both could apply, and unknown ids fall back to the first
|
||||
provider in the registry (by convention, the OAuth/safest default —
|
||||
``anthropic-oauth`` in both _BUILTIN_PROVIDERS and the shipped
|
||||
config.yaml).
|
||||
|
||||
Pre-condition: ``providers`` is non-empty. _load_providers always
|
||||
returns at least one entry (built-ins when YAML is missing or every
|
||||
@ -298,6 +313,44 @@ def _resolve_provider(model: str, providers: tuple) -> dict:
|
||||
"_load_providers must always return at least one entry "
|
||||
"(falling back to _BUILTIN_PROVIDERS when needed)"
|
||||
)
|
||||
|
||||
# Explicit provider name takes precedence — fail fast if it's not in
|
||||
# the registry. Anything else would silently route the operator's
|
||||
# picked provider through the wrong auth/base_url path. The error
|
||||
# message tells them exactly which two paths fix it.
|
||||
if explicit_provider:
|
||||
ep_lower = explicit_provider.lower()
|
||||
for provider in providers:
|
||||
if provider["name"].lower() == ep_lower:
|
||||
return provider
|
||||
names = ", ".join(p["name"] for p in providers)
|
||||
raise ValueError(
|
||||
f"claude-code adapter: workspace config picks "
|
||||
f"provider='{explicit_provider}' but it is not in the "
|
||||
f"providers registry.\n"
|
||||
f"\n"
|
||||
f"Known providers: {names}\n"
|
||||
f"\n"
|
||||
f"Two ways to fix:\n"
|
||||
f" (a) Add '{explicit_provider}' to /configs/config.yaml as a "
|
||||
f"providers: entry. Required keys:\n"
|
||||
f" providers:\n"
|
||||
f" - name: {explicit_provider}\n"
|
||||
f" auth_mode: third_party_anthropic_compat\n"
|
||||
f" base_url: https://... # provider's Anthropic-compat endpoint\n"
|
||||
f" auth_env: [{explicit_provider.upper()}_API_KEY]\n"
|
||||
f" model_prefixes: [...]\n"
|
||||
f" (b) Switch the workspace runtime template to one that "
|
||||
f"natively supports {explicit_provider} (CrewAI, LangGraph, or "
|
||||
f"DeepAgents read provider/model from runtime_config and route "
|
||||
f"directly without needing an Anthropic-compat shim).\n"
|
||||
f"\n"
|
||||
f"Note: claude-code SDK speaks the Anthropic API protocol. "
|
||||
f"Providers that only expose OpenAI-compatible endpoints "
|
||||
f"(MiniMax, GLM, Kimi, DeepSeek native APIs) need either an "
|
||||
f"Anthropic-compat proxy in front, or option (b)."
|
||||
)
|
||||
|
||||
if not model:
|
||||
return providers[0]
|
||||
m = model.lower()
|
||||
@ -401,8 +454,36 @@ class ClaudeCodeAdapter(BaseAdapter):
|
||||
rc = config.runtime_config
|
||||
if isinstance(rc, dict):
|
||||
picked_model = rc.get("model") or "sonnet"
|
||||
explicit_provider_name = rc.get("provider")
|
||||
else:
|
||||
picked_model = getattr(rc, "model", None) or "sonnet"
|
||||
explicit_provider_name = getattr(rc, "provider", None)
|
||||
|
||||
# Also honor the top-level `provider:` field in /configs/config.yaml.
|
||||
# The canvas Config-tab Provider dropdown writes there (not into
|
||||
# runtime_config) on some legacy paths. Either source is canonical;
|
||||
# whichever is set wins. Root cause of #180: the adapter used to
|
||||
# ignore both, silently routing every non-Anthropic provider pick
|
||||
# through anthropic-oauth.
|
||||
if not explicit_provider_name:
|
||||
yaml_path = os.path.join(config.config_path, "config.yaml")
|
||||
try:
|
||||
import yaml # transitive dep via molecule-ai-workspace-runtime
|
||||
with open(yaml_path, "r") as f:
|
||||
data = yaml.safe_load(f) or {}
|
||||
if isinstance(data, dict):
|
||||
val = data.get("provider")
|
||||
if isinstance(val, str) and val.strip():
|
||||
explicit_provider_name = val.strip()
|
||||
except FileNotFoundError:
|
||||
pass
|
||||
except Exception as exc: # noqa: BLE001 — defensive: never block boot
|
||||
logger.warning(
|
||||
"providers: failed to read top-level provider: from %s (%s); "
|
||||
"falling back to model-based resolution",
|
||||
yaml_path, exc,
|
||||
)
|
||||
|
||||
# NOTE: do NOT strip the provider prefix here. The pre-fix routing
|
||||
# behavior — `anthropic:claude-opus-4-7` falls through to
|
||||
# providers[0] (anthropic-oauth) when no model_prefixes match — is
|
||||
@ -411,7 +492,15 @@ class ClaudeCodeAdapter(BaseAdapter):
|
||||
# `anthropic-api` provider and the CLI then hangs at `initialize`
|
||||
# because ANTHROPIC_API_KEY isn't set. The strip belongs only at
|
||||
# the CLI invocation site (create_executor below).
|
||||
provider = _resolve_provider(picked_model, providers)
|
||||
#
|
||||
# Pass the explicit provider name through so _resolve_provider
|
||||
# raises ValueError with an actionable message (instead of silently
|
||||
# routing to providers[0]) when an operator picks a provider that
|
||||
# isn't in the registry. See #180.
|
||||
provider = _resolve_provider(
|
||||
picked_model, providers,
|
||||
explicit_provider=explicit_provider_name,
|
||||
)
|
||||
auth_env_options = provider["auth_env"]
|
||||
|
||||
# Project the per-vendor API key (MINIMAX_API_KEY, GLM_API_KEY,
|
||||
|
||||
89
tests/conftest.py
Normal file
89
tests/conftest.py
Normal file
@ -0,0 +1,89 @@
|
||||
"""Shared pytest fixtures + import shims for the adapter test suite.
|
||||
|
||||
`adapter.py` imports at module load:
|
||||
- molecule_runtime.adapters.base (BaseAdapter, AdapterConfig, RuntimeCapabilities)
|
||||
- molecule_runtime.plugins (lazy in setup(), but stubbed proactively)
|
||||
- a2a.server.agent_execution (AgentExecutor)
|
||||
- claude_sdk_executor (lazy in create_executor(), stubbed proactively)
|
||||
|
||||
In production those arrive transitively via molecule-ai-workspace-runtime.
|
||||
The CI runner only installs `pytest pytest-asyncio pyyaml`, so the import
|
||||
chain would fail with ModuleNotFoundError before any test collects —
|
||||
exactly the failure that broke CI on the #180 fix branch (PR #4) and
|
||||
caused the merge wall to block on a green local but red Gitea CI.
|
||||
|
||||
Putting the stub installer here (collected before any test module is
|
||||
imported, per pytest semantics) means every test file can do
|
||||
`from adapter import ...` at module top without a per-file boilerplate
|
||||
copy. It also forces a single shape for the stubs so two files can't
|
||||
silently disagree on whether `BaseAdapter` has
|
||||
`install_plugins_via_registry` (see test_adapter_prevalidate's
|
||||
async-setup tests, which need the method to exist on the parent class).
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import types
|
||||
from dataclasses import dataclass
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
|
||||
@dataclass
|
||||
class _StubRuntimeCapabilities:
|
||||
provides_native_session: bool = False
|
||||
|
||||
|
||||
@dataclass
|
||||
class _StubAdapterConfig:
|
||||
runtime_config: object = None
|
||||
config_path: str = "/tmp/configs"
|
||||
system_prompt: str = ""
|
||||
heartbeat: object = None
|
||||
|
||||
|
||||
class _StubBaseAdapter:
|
||||
async def install_plugins_via_registry(self, *_args, **_kwargs):
|
||||
pass
|
||||
|
||||
|
||||
def _install_stubs() -> None:
|
||||
"""Install the smallest set of import shims that adapter.py needs."""
|
||||
if "molecule_runtime" not in sys.modules:
|
||||
mr = types.ModuleType("molecule_runtime")
|
||||
mr.adapters = types.ModuleType("molecule_runtime.adapters")
|
||||
mr.adapters.base = types.ModuleType("molecule_runtime.adapters.base")
|
||||
mr.adapters.base.BaseAdapter = _StubBaseAdapter
|
||||
mr.adapters.base.AdapterConfig = _StubAdapterConfig
|
||||
mr.adapters.base.RuntimeCapabilities = _StubRuntimeCapabilities
|
||||
mr.plugins = types.ModuleType("molecule_runtime.plugins")
|
||||
mr.plugins.load_plugins = lambda **_kwargs: []
|
||||
sys.modules["molecule_runtime"] = mr
|
||||
sys.modules["molecule_runtime.adapters"] = mr.adapters
|
||||
sys.modules["molecule_runtime.adapters.base"] = mr.adapters.base
|
||||
sys.modules["molecule_runtime.plugins"] = mr.plugins
|
||||
if "a2a" not in sys.modules:
|
||||
a2a = types.ModuleType("a2a")
|
||||
a2a.server = types.ModuleType("a2a.server")
|
||||
a2a.server.agent_execution = types.ModuleType("a2a.server.agent_execution")
|
||||
a2a.server.agent_execution.AgentExecutor = type("AgentExecutor", (), {})
|
||||
sys.modules["a2a"] = a2a
|
||||
sys.modules["a2a.server"] = a2a.server
|
||||
sys.modules["a2a.server.agent_execution"] = a2a.server.agent_execution
|
||||
if "claude_sdk_executor" not in sys.modules:
|
||||
mod = types.ModuleType("claude_sdk_executor")
|
||||
mod.ClaudeSDKExecutor = MagicMock(name="ClaudeSDKExecutor")
|
||||
sys.modules["claude_sdk_executor"] = mod
|
||||
|
||||
|
||||
# Run at conftest import time — pytest collects conftest.py before any
|
||||
# test module, so the stubs are in sys.modules before `from adapter
|
||||
# import ...` ever executes.
|
||||
_install_stubs()
|
||||
|
||||
# adapter.py lives in the parent dir of tests/ (template root). pytest's
|
||||
# `--import-mode=importlib` + tests/pytest.ini anchoring rootdir at
|
||||
# tests/ means the parent isn't on sys.path automatically. Add it here
|
||||
# once so every test file can do `from adapter import ...` cleanly.
|
||||
_PARENT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
if _PARENT_DIR not in sys.path:
|
||||
sys.path.insert(0, _PARENT_DIR)
|
||||
146
tests/test_provider_resolution.py
Normal file
146
tests/test_provider_resolution.py
Normal file
@ -0,0 +1,146 @@
|
||||
"""Tests for the provider-resolution path that was silent-failing on #180.
|
||||
|
||||
Regression coverage: when an operator picks a provider in the canvas Config
|
||||
tab that isn't in the registry, the adapter must raise ValueError with an
|
||||
actionable message — NOT silently fall through to providers[0]
|
||||
(anthropic-oauth) and then have the Claude SDK hit the user's OAuth quota
|
||||
under a different name.
|
||||
|
||||
These tests mirror the production failure mode reported by Hongming
|
||||
2026-05-07 17:35: workspace config.yaml had `provider: minimax` set, the
|
||||
adapter ignored it entirely, the SDK kept calling the Anthropic API with
|
||||
CLAUDE_CODE_OAUTH_TOKEN, hit the OAuth quota, and the canvas surfaced
|
||||
"Agent error (Exception)" with no clue why.
|
||||
|
||||
Import-shim setup (sys.path + molecule_runtime / a2a / claude_sdk_executor
|
||||
stubs) lives in tests/conftest.py — shared with test_adapter_prevalidate
|
||||
so the two stub installers can't disagree on shape (e.g. BaseAdapter
|
||||
having install_plugins_via_registry).
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from adapter import (
|
||||
_BUILTIN_PROVIDERS,
|
||||
_resolve_provider,
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_with_no_explicit_provider_falls_back_to_model_match():
|
||||
"""No explicit provider → model-based prefix/alias matching, default to providers[0]."""
|
||||
p = _resolve_provider("claude-opus-4-7", _BUILTIN_PROVIDERS)
|
||||
assert p["name"] == "anthropic-api" # matches model_prefixes=("claude-",)
|
||||
|
||||
|
||||
def test_resolve_with_no_explicit_provider_falls_back_to_default():
|
||||
"""Unknown model + no explicit provider → providers[0] (anthropic-oauth)."""
|
||||
p = _resolve_provider("unknown-model", _BUILTIN_PROVIDERS)
|
||||
assert p["name"] == "anthropic-oauth"
|
||||
|
||||
|
||||
def test_resolve_with_explicit_provider_in_registry_returns_match():
|
||||
"""Explicit name lookup wins over model-based resolution."""
|
||||
# Even though "claude-opus-4-7" would normally resolve to anthropic-api
|
||||
# via prefix matching, the explicit provider name wins.
|
||||
p = _resolve_provider(
|
||||
"claude-opus-4-7", _BUILTIN_PROVIDERS,
|
||||
explicit_provider="anthropic-oauth",
|
||||
)
|
||||
assert p["name"] == "anthropic-oauth"
|
||||
|
||||
|
||||
def test_resolve_with_explicit_provider_case_insensitive():
|
||||
"""Provider name match is case-insensitive (operators write 'Anthropic-OAuth' etc)."""
|
||||
p = _resolve_provider(
|
||||
"sonnet", _BUILTIN_PROVIDERS,
|
||||
explicit_provider="ANTHROPIC-OAUTH",
|
||||
)
|
||||
assert p["name"] == "anthropic-oauth"
|
||||
|
||||
|
||||
def test_resolve_with_explicit_provider_not_in_registry_raises():
|
||||
"""The #180 regression test: explicit non-registry provider must raise, not fall through."""
|
||||
with pytest.raises(ValueError) as exc_info:
|
||||
_resolve_provider(
|
||||
"MiniMax-M2.7-highspeed", _BUILTIN_PROVIDERS,
|
||||
explicit_provider="minimax",
|
||||
)
|
||||
msg = str(exc_info.value)
|
||||
# Must name the bad provider so operator knows what they typed
|
||||
assert "minimax" in msg
|
||||
# Must list known providers so operator knows what's available
|
||||
assert "anthropic-oauth" in msg
|
||||
assert "anthropic-api" in msg
|
||||
# Must give actionable next steps — NOT just "not found"
|
||||
assert "providers:" in msg or "Add" in msg
|
||||
assert "Switch" in msg or "runtime" in msg
|
||||
|
||||
|
||||
def test_resolve_with_explicit_provider_does_not_silent_fallback():
|
||||
"""Specifically: must not return providers[0] when explicit_provider is bogus.
|
||||
|
||||
This is the exact silent-fallback path that caused the user-visible
|
||||
bug: operator picks 'minimax' → adapter returns anthropic-oauth →
|
||||
SDK uses CLAUDE_CODE_OAUTH_TOKEN → hits quota.
|
||||
"""
|
||||
with pytest.raises(ValueError):
|
||||
result = _resolve_provider(
|
||||
"anything", _BUILTIN_PROVIDERS,
|
||||
explicit_provider="minimax",
|
||||
)
|
||||
# If the implementation regresses to silent fallback, this would
|
||||
# have returned providers[0] (anthropic-oauth) instead of raising.
|
||||
# Defense-in-depth: guard against accidental "return" inside the
|
||||
# error path.
|
||||
assert result["name"] not in {"anthropic-oauth", "anthropic-api"}, (
|
||||
"REGRESSION: silent fallback to default provider when explicit "
|
||||
"provider name is not in registry — this is the #180 bug."
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_with_explicit_provider_in_custom_registry():
|
||||
"""When operator adds a third-party provider to the registry, explicit lookup finds it."""
|
||||
custom_registry = _BUILTIN_PROVIDERS + (
|
||||
{
|
||||
"name": "minimax",
|
||||
"auth_mode": "third_party_anthropic_compat",
|
||||
"model_prefixes": ("minimax-",),
|
||||
"model_aliases": (),
|
||||
"base_url": "https://api.minimaxi.com/anthropic-compat",
|
||||
"auth_env": ("MINIMAX_API_KEY",),
|
||||
},
|
||||
)
|
||||
p = _resolve_provider(
|
||||
"MiniMax-M2.7-highspeed", custom_registry,
|
||||
explicit_provider="minimax",
|
||||
)
|
||||
assert p["name"] == "minimax"
|
||||
assert p["base_url"] == "https://api.minimaxi.com/anthropic-compat"
|
||||
assert "MINIMAX_API_KEY" in p["auth_env"]
|
||||
|
||||
|
||||
def test_resolve_empty_providers_raises():
|
||||
"""Pre-condition: providers must be non-empty (existing behavior preserved)."""
|
||||
with pytest.raises(ValueError, match="empty providers tuple"):
|
||||
_resolve_provider("anything", ())
|
||||
|
||||
|
||||
def test_resolve_explicit_empty_string_treated_as_no_explicit():
|
||||
"""`provider: ''` (empty string) → fall back to model-based resolution, not raise."""
|
||||
# This shape can happen when the canvas writes an empty provider field.
|
||||
# Treating it as "no explicit pick" is more forgiving than raising,
|
||||
# since the user clearly didn't intend to break their workspace.
|
||||
p = _resolve_provider(
|
||||
"claude-opus-4-7", _BUILTIN_PROVIDERS,
|
||||
explicit_provider="",
|
||||
)
|
||||
assert p["name"] == "anthropic-api" # fell through to model-based
|
||||
|
||||
|
||||
def test_resolve_explicit_none_treated_as_no_explicit():
|
||||
"""`explicit_provider=None` (default) → fall back to model-based resolution."""
|
||||
p = _resolve_provider(
|
||||
"claude-opus-4-7", _BUILTIN_PROVIDERS,
|
||||
explicit_provider=None,
|
||||
)
|
||||
assert p["name"] == "anthropic-api"
|
||||
Loading…
Reference in New Issue
Block a user