diff --git a/adapter.py b/adapter.py index 57357f8..ae68efe 100644 --- a/adapter.py +++ b/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, diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..4b6d747 --- /dev/null +++ b/tests/conftest.py @@ -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) diff --git a/tests/test_provider_resolution.py b/tests/test_provider_resolution.py new file mode 100644 index 0000000..cb51343 --- /dev/null +++ b/tests/test_provider_resolution.py @@ -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"