fix(ai): lock provider selection to config.ai.provider, hard-fail on miss (#535)

Closes a silent-fallback bug: when config.ai.provider="anthropic"
was set, LLMAbstraction still initialized OpenAI alongside Anthropic
whenever an OpenAI key was present in config or env (via
${OPENAI_API_KEY} substitution in config_marcus.json). OpenAI then
joined the fallback chain. When Anthropic momentarily failed,
Marcus cascaded to OpenAI silently, and real cost rows landed under
provider="openai" for a user who believed they had locked the
system to Anthropic.

The earlier code gated only the ENV-VAR fallback by
configured_provider — the init block itself ran whenever the
substituted key passed validation. This change inverts the gate:
each provider's init is wrapped in _allowed(name) which is True
only when configured_provider is empty (legacy auto-discovery) or
matches the provider's name.

Also adds a hard-fail at the end of _initialize_providers: when
configured_provider is set but didn't make it into self.providers,
raise RuntimeError instead of silently allowing whatever else
loaded. Surfaces misconfig immediately at startup.

Tests: 3 new regression cases in TestProviderLockdownRegression
covering the leak, symmetric case, and hard-fail behavior. Updated
one existing test's regex match to the new (more specific) error
message.

Refs: #533 (graceful-degradation follow-up)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: lwgray

src/ai/providers/llm_abstraction.py

@@ -185,79 +185,107 @@ def _initialize_providers(self) -> None:
 
             config = MarcusConfig()
 
-        # Check if user explicitly configured a provider
-        # If so, only initialize that provider (no fallbacks to env vars)
+        # Provider lockdown (Marcus #531). When the user explicitly sets
+        # ``config.ai.provider``, ONLY that provider initializes. Other
+        # providers never enter ``self.providers`` and never become
+        # fallback candidates — even if their credentials happen to be
+        # present in config or in the environment.
+        #
+        # The earlier code gated only the ENV-VAR fallback by
+        # ``configured_provider``, but left the init block gated only
+        # on key validity. Config substitution (``"openai_api_key":
+        # "${OPENAI_API_KEY}"`` in config_marcus.json) put a real OpenAI
+        # key into ``config.ai.openai_api_key`` whenever the env var was
+        # exported in the user's shell — so OpenAI silently joined the
+        # fallback chain even when ``provider: anthropic`` was set, and
+        # cascaded billing to OpenAI when Anthropic momentarily failed.
         configured_provider = config.ai.provider or ""
 
-        # Try to initialize Anthropic provider
-        anthropic_key = config.ai.anthropic_api_key or ""
-        # Only fall back to env var if no specific provider is configured
-        # or if anthropic is the configured provider
-        should_fallback_anthropic = (
-            not configured_provider or configured_provider == "anthropic"
-        )
-        if not anthropic_key and should_fallback_anthropic:
-            anthropic_key = os.getenv("CLAUDE_API_KEY", "").strip()
-
-        if (
-            anthropic_key
-            and anthropic_key.startswith("sk-ant-")
-            and len(anthropic_key) > 10
-            and anthropic_key != "sk-ant-your-api-key-here"
-        ):
-            try:
-                from .anthropic_provider import AnthropicProvider
-
-                # Pass key directly to the provider — never write into
-                # os.environ. ANTHROPIC_API_KEY in the env would force
-                # Claude Code subprocesses (Epictetus, project creator,
-                # workers, monitor) to bill the API instead of using the
-                # user's Claude Code subscription.
-                self.providers["anthropic"] = AnthropicProvider(api_key=anthropic_key)
-                self.fallback_providers.append("anthropic")
-                logger.info("Successfully initialized Anthropic provider")
-            except Exception as e:
-                logger.warning(f"Failed to initialize Anthropic provider: {e}")
-        else:
-            logger.debug(
-                f"Skipping Anthropic provider - no valid API key configured "
-                f"(key present: {bool(anthropic_key)})"
-            )
+        def _allowed(name: str) -> bool:
+            """Return True iff ``name`` may initialize under the current config.
+
+            When ``configured_provider`` is set, only the matching name
+            is allowed. When it's empty (legacy auto-discovery), every
+            provider with valid credentials is allowed.
+            """
+            return not configured_provider or configured_provider == name
+
+        # ----- Anthropic -----------------------------------------------------
+        if _allowed("anthropic"):
+            anthropic_key = config.ai.anthropic_api_key or ""
+            if not anthropic_key:
+                anthropic_key = os.getenv("CLAUDE_API_KEY", "").strip()
+
+            if (
+                anthropic_key
+                and anthropic_key.startswith("sk-ant-")
+                and len(anthropic_key) > 10
+                and anthropic_key != "sk-ant-your-api-key-here"
+            ):
+                try:
+                    from .anthropic_provider import AnthropicProvider
+
+                    # Pass key directly to the provider — never write into
+                    # os.environ. ANTHROPIC_API_KEY in the env would force
+                    # Claude Code subprocesses (Epictetus, project creator,
+                    # workers, monitor) to bill the API instead of using the
+                    # user's Claude Code subscription.
+                    self.providers["anthropic"] = AnthropicProvider(
+                        api_key=anthropic_key
+                    )
+                    self.fallback_providers.append("anthropic")
+                    logger.info("Successfully initialized Anthropic provider")
+                except Exception as e:
+                    logger.warning(f"Failed to initialize Anthropic provider: {e}")
+            else:
+                logger.debug(
+                    f"Skipping Anthropic provider - no valid API key configured "
+                    f"(key present: {bool(anthropic_key)})"
+                )
 
-        # Only try OpenAI if we have a valid API key
-        openai_key = config.ai.openai_api_key or ""
-        # Only fall back to env var if no specific provider is configured
-        # or if openai is the configured provider
-        should_fallback_openai = (
-            not configured_provider or configured_provider == "openai"
-        )
-        if not openai_key and should_fallback_openai:
-            openai_key = os.getenv("OPENAI_API_KEY", "").strip()
-
-        if (
-            openai_key
-            and openai_key.startswith("sk-")
-            and len(openai_key) > 10
-            and openai_key != "sk-your-openai-key-here"
-        ):
-            try:
-                from .openai_provider import OpenAIProvider
+        # ----- OpenAI --------------------------------------------------------
+        if _allowed("openai"):
+            openai_key = config.ai.openai_api_key or ""
+            if not openai_key:
+                openai_key = os.getenv("OPENAI_API_KEY", "").strip()
+
+            if (
+                openai_key
+                and openai_key.startswith("sk-")
+                and len(openai_key) > 10
+                and openai_key != "sk-your-openai-key-here"
+            ):
+                try:
+                    from .openai_provider import OpenAIProvider
 
-                # Temporarily set env var for the provider
-                os.environ["OPENAI_API_KEY"] = openai_key
-                self.providers["openai"] = OpenAIProvider()
-                self.fallback_providers.append("openai")
-                logger.info("Successfully initialized OpenAI provider")
-            except Exception as e:
-                logger.warning(f"Failed to initialize OpenAI provider: {e}")
-        else:
-            logger.debug(
-                f"Skipping OpenAI provider - no valid API key configured "
-                f"(key present: {bool(openai_key)})"
+                    # Temporarily set env var for the provider
+                    os.environ["OPENAI_API_KEY"] = openai_key
+                    self.providers["openai"] = OpenAIProvider()
+                    self.fallback_providers.append("openai")
+                    logger.info("Successfully initialized OpenAI provider")
+                except Exception as e:
+                    logger.warning(f"Failed to initialize OpenAI provider: {e}")
+            else:
+                logger.debug(
+                    f"Skipping OpenAI provider - no valid API key configured "
+                    f"(key present: {bool(openai_key)})"
+                )
+        elif config.ai.openai_api_key or os.getenv("OPENAI_API_KEY", "").strip():
+            # Diagnostic only: user has an OpenAI key available somewhere
+            # but `config.ai.provider` excludes openai. Tell them loudly
+            # so they know we deliberately ignored the key.
+            logger.info(
+                "OpenAI key present but provider=%r — OpenAI deliberately "
+                "NOT initialized. To use OpenAI, set ai.provider='openai' "
+                "in config_marcus.json.",
+                configured_provider,
             )
 
-        # Add cloud provider if configured
-        if configured_provider == "cloud":
+        # ----- Cloud ---------------------------------------------------------
+        # Cloud was already gated correctly (only inits when explicitly
+        # configured), so the existing check stays. Comment kept for
+        # consistency with the rewritten anthropic/openai blocks above.
+        if _allowed("cloud") and configured_provider == "cloud":
             cloud_key = config.ai.cloud_api_key or ""
             if not cloud_key:
                 cloud_key = os.getenv("MARCUS_CLOUD_LLM_KEY", "").strip()
@@ -294,28 +322,38 @@ def _initialize_providers(self) -> None:
                     bool(cloud_model),
                 )
 
-        # Add local provider if configured
-        local_model_path = config.ai.local_model or ""
-        # Only fall back to env var if no specific provider is configured
-        # or if local is the configured provider
-        should_fallback_local = (
-            not configured_provider or configured_provider == "local"
-        )
-        if not local_model_path and should_fallback_local:
-            local_model_path = os.getenv("MARCUS_LOCAL_LLM_PATH", "").strip()
+        # ----- Local ---------------------------------------------------------
+        if _allowed("local"):
+            local_model_path = config.ai.local_model or ""
+            if not local_model_path:
+                local_model_path = os.getenv("MARCUS_LOCAL_LLM_PATH", "").strip()
 
-        if local_model_path:
-            try:
-                from .local_provider import LocalLLMProvider
+            if local_model_path:
+                try:
+                    from .local_provider import LocalLLMProvider
 
-                self.providers["local"] = LocalLLMProvider(local_model_path)
-                self.fallback_providers.append("local")
-                logger.info(
-                    f"Successfully initialized local LLM provider "
-                    f"with model: {local_model_path}"
-                )
-            except Exception as e:
-                logger.warning(f"Failed to initialize local LLM provider: {e}")
+                    self.providers["local"] = LocalLLMProvider(local_model_path)
+                    self.fallback_providers.append("local")
+                    logger.info(
+                        f"Successfully initialized local LLM provider "
+                        f"with model: {local_model_path}"
+                    )
+                except Exception as e:
+                    logger.warning(f"Failed to initialize local LLM provider: {e}")
+
+        # Hard-fail when the user explicitly set a provider and it didn't
+        # initialize. The earlier code logged a warning and silently
+        # cascaded to whichever provider happened to be available — that
+        # caused real cost rows to land under the "wrong" provider after a
+        # silent fallback. Surface the gap immediately. (Marcus #531)
+        if configured_provider and configured_provider not in self.providers:
+            raise RuntimeError(
+                f"config.ai.provider={configured_provider!r} is set but the "
+                f"provider failed to initialize. Refusing to silently fall "
+                f"back to another provider. Check that the corresponding "
+                f"credentials are present and valid in config_marcus.json "
+                f"or the matching environment variable, then restart Marcus."
+            )
 
         # Initialize provider stats only for successfully loaded providers
         self.provider_stats = {

tests/unit/ai/test_llm_provider_selection.py

@@ -10,6 +10,8 @@
 
 import pytest
 
+pytestmark = pytest.mark.unit
+
 from src.ai.providers.llm_abstraction import LLMAbstraction
 
 
@@ -244,8 +246,9 @@ def test_anthropic_api_key_env_var_is_NOT_read(
 
         With only ANTHROPIC_API_KEY in env (no CLAUDE_API_KEY, no config key),
         the Anthropic provider must NOT initialize. ``_initialize_providers``
-        raises ``RuntimeError`` when nothing initializes, which is the
-        correct outcome — that proves the env var was ignored.
+        raises ``RuntimeError`` when the configured provider failed to init
+        (Marcus #531), which is the correct outcome — that proves the env
+        var was ignored.
         """
         os.environ["ANTHROPIC_API_KEY"] = "sk-ant-this-should-be-ignored-completely"
 
@@ -258,7 +261,7 @@ def test_anthropic_api_key_env_var_is_NOT_read(
 
         with patch("src.config.marcus_config.get_config", return_value=mock_config):
             llm = LLMAbstraction()
-            with pytest.raises(RuntimeError, match="No LLM providers"):
+            with pytest.raises(RuntimeError, match="ai.provider='anthropic' is set"):
                 llm._initialize_providers()
 
         # Even after the failed initialization attempt, the providers dict
@@ -267,3 +270,92 @@ def test_anthropic_api_key_env_var_is_NOT_read(
             "Marcus must not fall back to ANTHROPIC_API_KEY — that env var "
             "belongs to Claude Code's subscription auth."
         )
+
+
+class TestProviderLockdownRegression:
+    """Regression: real OpenAI key in config must not leak past
+    ``provider: anthropic`` setting (Marcus #531).
+
+    The earlier code gated only the env-var fallback by configured
+    provider; the init block ran whenever
+    ``config.ai.openai_api_key`` was non-empty — and Marcus's
+    ``config_marcus.json`` substitutes ``${OPENAI_API_KEY}`` into
+    that field, so a shell-exported OpenAI key silently joined the
+    fallback chain alongside Anthropic. When Anthropic momentarily
+    failed, Marcus cascaded to OpenAI and billed real tokens to the
+    OpenAI key without the user knowing.
+
+    These tests reproduce the exact leak condition (config carries
+    BOTH keys; provider explicitly set to one) and assert the other
+    provider stays out of ``self.providers``.
+    """
+
+    @pytest.fixture
+    def env_with_real_openai_key(self):
+        """Reproduce the user's environment: real openai key in env."""
+        prev = os.environ.get("OPENAI_API_KEY")
+        os.environ["OPENAI_API_KEY"] = "sk-proj-s8abcdef1234567890abcdef"
+        yield
+        if prev is None:
+            os.environ.pop("OPENAI_API_KEY", None)
+        else:
+            os.environ["OPENAI_API_KEY"] = prev
+
+    def test_openai_excluded_when_provider_is_anthropic(self, env_with_real_openai_key):
+        """Provider=anthropic + real OpenAI key in config: no OpenAI provider."""
+        mock_config = Mock()
+        mock_config.ai.provider = "anthropic"
+        # Reproduce config_marcus.json's substituted state: anthropic
+        # AND openai keys both present.
+        mock_config.ai.anthropic_api_key = "sk-ant-api03-fake-for-test-1234567890"
+        mock_config.ai.openai_api_key = "sk-proj-s8abcdef1234567890abcdef"
+        mock_config.ai.model = "claude-sonnet-4-6"
+        mock_config.ai.local_model = None
+
+        with patch("src.config.marcus_config.get_config", return_value=mock_config):
+            llm = LLMAbstraction()
+            llm._initialize_providers()
+            assert "anthropic" in llm.providers
+            assert "openai" not in llm.providers, (
+                "OpenAI provider must NOT initialize when provider=anthropic, "
+                "even if openai_api_key is non-empty"
+            )
+            assert "openai" not in llm.fallback_providers
+
+    def test_anthropic_excluded_when_provider_is_openai(self, env_with_real_openai_key):
+        """Provider=openai + real Anthropic key in config: no Anthropic provider."""
+        mock_config = Mock()
+        mock_config.ai.provider = "openai"
+        mock_config.ai.anthropic_api_key = "sk-ant-api03-fake-for-test-1234567890"
+        mock_config.ai.openai_api_key = "sk-proj-s8abcdef1234567890abcdef"
+        mock_config.ai.model = "gpt-4o-mini"
+        mock_config.ai.local_model = None
+
+        with patch("src.config.marcus_config.get_config", return_value=mock_config):
+            llm = LLMAbstraction()
+            llm._initialize_providers()
+            assert "openai" in llm.providers
+            assert "anthropic" not in llm.providers
+            assert "anthropic" not in llm.fallback_providers
+
+    def test_hard_fail_when_configured_provider_does_not_initialize(self):
+        """Refusing silent fallback: missing anthropic key + provider=anthropic raises."""
+        mock_config = Mock()
+        mock_config.ai.provider = "anthropic"
+        # Explicitly no anthropic key but a real openai key present —
+        # the earlier code would silently fall back to OpenAI.
+        mock_config.ai.anthropic_api_key = None
+        mock_config.ai.openai_api_key = "sk-proj-s8abcdef1234567890abcdef"
+        mock_config.ai.model = "claude-sonnet-4-6"
+        mock_config.ai.local_model = None
+
+        # Clear CLAUDE_API_KEY too so the env-var fallback can't satisfy it.
+        prev = os.environ.pop("CLAUDE_API_KEY", None)
+        try:
+            with patch("src.config.marcus_config.get_config", return_value=mock_config):
+                with pytest.raises(RuntimeError, match="ai.provider='anthropic'"):
+                    llm = LLMAbstraction()
+                    llm._initialize_providers()
+        finally:
+            if prev is not None:
+                os.environ["CLAUDE_API_KEY"] = prev