diff --git a/anton/cli.py b/anton/cli.py index 45e20d9d..0e07ba5a 100644 --- a/anton/cli.py +++ b/anton/cli.py @@ -133,7 +133,7 @@ def _reexec() -> None: # Core dependencies from pyproject.toml that anton needs at runtime _REQUIRED_PACKAGES: dict[str, str] = { "anthropic": "anthropic>=0.42.0", - "openai": "openai>=1.0", + "openai": "openai>=2.21.0", "pydantic": "pydantic>=2.0", "pydantic_settings": "pydantic-settings>=2.0", "prompt_toolkit": "prompt-toolkit>=3.0", diff --git a/anton/core/llm/anthropic.py b/anton/core/llm/anthropic.py index ccddf477..72654c59 100644 --- a/anton/core/llm/anthropic.py +++ b/anton/core/llm/anthropic.py @@ -13,6 +13,7 @@ ContextOverflowError, LLMProvider, LLMResponse, + ProviderAuthError, ProviderConnectionInfo, StreamComplete, StreamEvent, @@ -47,7 +48,7 @@ def _raise_for_status_error( failures are classified first; only what's left is offered to the transient classifier, and the generic "unavailable" copy is the last resort. - - 401 → ConnectionError (invalid-key copy; cowork-server keys on this phrase). + - 401 → ProviderAuthError (canonical provider-credential refusal). - 429 WITH a quota ``detail`` → TokenLimitExceeded (keeps its own card). - 402/429 with an M3 gate wallet code (``wallet_empty`` / ``included_allowance_exhausted``, body or X-MindsHub-Reason header) @@ -60,7 +61,7 @@ def _raise_for_status_error( - anything else → the generic "temporarily unavailable" ConnectionError. """ if exc.status_code == 401: - raise ConnectionError( + raise ProviderAuthError( "Invalid API key — check your ANTHROPIC_API_KEY environment variable." ) from exc diff --git a/anton/core/llm/client.py b/anton/core/llm/client.py index d92f494b..7d2e09fe 100644 --- a/anton/core/llm/client.py +++ b/anton/core/llm/client.py @@ -1,14 +1,75 @@ from __future__ import annotations import logging -from collections.abc import AsyncIterator -from typing import TYPE_CHECKING +from collections.abc import AsyncIterator, Awaitable, Callable +from typing import TYPE_CHECKING, TypeVar -from .provider import LLMProvider, LLMResponse, StreamComplete, StreamEvent +from .provider import ( + LLMProvider, + LLMResponse, + ProviderAuthError, + StreamComplete, + StreamEvent, +) if TYPE_CHECKING: from anton.config.settings import AntonSettings +_T = TypeVar("_T") + + +class _ProviderAuthConfirmation: + """One immediate retry budget for a single logical provider call.""" + + def __init__(self) -> None: + self._available = True + + def take(self) -> bool: + if not self._available: + return False + self._available = False + return True + + +async def _call_with_auth_confirmation( + operation: Callable[[], Awaitable[_T]], + *, + role: str, +) -> _T: + """Retry one typed provider-auth refusal, then propagate the second.""" + confirmation = _ProviderAuthConfirmation() + while True: + try: + return await operation() + except ProviderAuthError as exc: + if not confirmation.take(): + exc.role = role + raise + + +async def _stream_with_auth_confirmation( + operation: Callable[[], AsyncIterator[_T]], + *, + role: str, +) -> AsyncIterator[_T]: + """Retry a pre-stream auth refusal without replaying emitted events.""" + confirmation = _ProviderAuthConfirmation() + while True: + yielded = False + try: + async for event in operation(): + yielded = True + yield event + return + except ProviderAuthError as exc: + # ProviderAuthError is produced from an HTTP 401 before a response + # stream starts. Keep the guard anyway: if a future provider maps a + # mid-stream event to this type, replaying would duplicate visible + # text or tool-use events. + if yielded or not confirmation.take(): + exc.role = role + raise + def _resolve_openai_compatible_flavor(settings: AntonSettings) -> str: """Distinguish MindsHub/mdb.ai passthrough from a generic openai-compatible @@ -63,6 +124,7 @@ def __init__( # Defaults to the coding role so hosts that construct LLMClient # directly (cowork-server) get behavior-preserving summarization # with no changes. + self._router_auth_role = "router" if router_provider is not None else "coding" self._router_provider = router_provider or coding_provider self._router_model = router_model or coding_model self._max_tokens = max_tokens @@ -113,13 +175,16 @@ async def plan( native_web_tools: set[str] | None = None, ) -> LLMResponse: listener = self.usage_listener - response = await self._planning_provider.complete( - model=self._planning_model, - system=system, - messages=messages, - tools=tools, - max_tokens=max_tokens or self._max_tokens, - native_web_tools=native_web_tools, + response = await _call_with_auth_confirmation( + lambda: self._planning_provider.complete( + model=self._planning_model, + system=system, + messages=messages, + tools=tools, + max_tokens=max_tokens or self._max_tokens, + native_web_tools=native_web_tools, + ), + role="planning", ) self._notify_usage("planning", self._planning_model, response.usage, listener) self._record_served(response) @@ -135,13 +200,16 @@ async def plan_stream( native_web_tools: set[str] | None = None, ) -> AsyncIterator[StreamEvent]: listener = self.usage_listener - async for event in self._planning_provider.stream( - model=self._planning_model, - system=system, - messages=messages, - tools=tools, - max_tokens=max_tokens or self._max_tokens, - native_web_tools=native_web_tools, + async for event in _stream_with_auth_confirmation( + lambda: self._planning_provider.stream( + model=self._planning_model, + system=system, + messages=messages, + tools=tools, + max_tokens=max_tokens or self._max_tokens, + native_web_tools=native_web_tools, + ), + role="planning", ): if isinstance(event, StreamComplete): self._notify_usage( @@ -214,13 +282,16 @@ async def code( native_web_tools: set[str] | None = None, ) -> LLMResponse: listener = self.usage_listener - response = await self._coding_provider.complete( - model=self._coding_model, - system=system, - messages=messages, - tools=tools, - max_tokens=max_tokens or self._max_tokens, - native_web_tools=native_web_tools, + response = await _call_with_auth_confirmation( + lambda: self._coding_provider.complete( + model=self._coding_model, + system=system, + messages=messages, + tools=tools, + max_tokens=max_tokens or self._max_tokens, + native_web_tools=native_web_tools, + ), + role="coding", ) self._notify_usage("coding", self._coding_model, response.usage, listener) return response @@ -239,11 +310,14 @@ async def summarize( this is behavior-preserving unless a distinct model is selected. """ listener = self.usage_listener - response = await self._router_provider.complete( - model=self._router_model, - system=system, - messages=messages, - max_tokens=max_tokens or self._max_tokens, + response = await _call_with_auth_confirmation( + lambda: self._router_provider.complete( + model=self._router_model, + system=system, + messages=messages, + max_tokens=max_tokens or self._max_tokens, + ), + role=self._router_auth_role, ) self._notify_usage("router", self._router_model, response.usage, listener) return response @@ -263,13 +337,16 @@ async def gate( only answer from context or delegate. """ listener = self.usage_listener - response = await self._router_provider.complete( - model=self._router_model, - system=system, - messages=messages, - tools=tools, - tool_choice=tool_choice, - max_tokens=max_tokens or self._max_tokens, + response = await _call_with_auth_confirmation( + lambda: self._router_provider.complete( + model=self._router_model, + system=system, + messages=messages, + tools=tools, + tool_choice=tool_choice, + max_tokens=max_tokens or self._max_tokens, + ), + role=self._router_auth_role, ) self._notify_usage("router", self._router_model, response.usage, listener) return response @@ -304,13 +381,16 @@ async def _generate_object_with( budget = max_tokens or self._max_tokens listener = self.usage_listener - response = await provider.complete( - model=model, - system=system, - messages=messages, - tools=[tool], - tool_choice={"type": "tool", "name": tool["name"]}, - max_tokens=budget, + response = await _call_with_auth_confirmation( + lambda: provider.complete( + model=model, + system=system, + messages=messages, + tools=[tool], + tool_choice={"type": "tool", "name": tool["name"]}, + max_tokens=budget, + ), + role=role, ) # Count BEFORE the no-tool-call raise below: a structured call that # failed (and its bigger-budget retry) still spent real tokens diff --git a/anton/core/llm/openai.py b/anton/core/llm/openai.py index 2eb9506d..c5357e47 100644 --- a/anton/core/llm/openai.py +++ b/anton/core/llm/openai.py @@ -3,7 +3,7 @@ import json import logging import os -from collections.abc import AsyncIterator +from collections.abc import AsyncIterator, Awaitable, Callable from typing import NoReturn import openai @@ -19,6 +19,7 @@ LLMProvider, LLMResponse, ModelUnavailableError, + ProviderAuthError, ProviderConnectionInfo, StreamComplete, StreamEvent, @@ -42,6 +43,8 @@ logger = logging.getLogger(__name__) +AsyncAPIKeyProvider = Callable[[], Awaitable[str]] + def _raise_for_status_error(exc: "openai.APIStatusError", model: str) -> NoReturn: """Map a provider HTTP error onto anton's typed/curated exceptions. @@ -51,8 +54,7 @@ def _raise_for_status_error(exc: "openai.APIStatusError", model: str) -> NoRetur previous four copy-pasted blocks had already diverged in wording. Mapping policy: - - 401 → ConnectionError with the invalid-key copy (cowork-server's - provider_auth detection keys on this exact phrase). + - 401 → ProviderAuthError with the invalid-key copy. - 403 with a structured gateway code (``model_access_denied`` / ``model_disabled``) → ModelUnavailableError carrying the code + model, with actionable copy. Detection is code-exact on purpose: @@ -74,8 +76,8 @@ def _raise_for_status_error(exc: "openai.APIStatusError", model: str) -> NoRetur ``code`` sits at top level. The gateway's 429 is FastAPI-style (``{"detail": …}``, no envelope) and passes through untouched. Both fields are therefore read from the top level first, with an envelope - fallback for clients that deliver the wire shape unmodified (anton's - pyproject allows ``openai>=1.0``, and proxies exist that re-wrap). + fallback for clients that deliver the wire shape unmodified (anton requires + ``openai>=2.21.0``, and proxies exist that re-wrap). The originally shipped ENG-598 mapper read only ``body["error"]["code"]`` — a key the SDK had already peeled off — which left the model-403 card dead in production. @@ -93,7 +95,7 @@ def _raise_for_status_error(exc: "openai.APIStatusError", model: str) -> NoRetur _foreign = origin_is_known_third_party(exc) if exc.status_code == 401: - raise ConnectionError( + raise ProviderAuthError( "Invalid API key — check your OpenAI API key configuration." ) from exc @@ -827,7 +829,12 @@ def __init__( vision_format: str = "openai", flavor: str = FLAVOR_OPENAI_COMPATIBLE_GENERIC, reasoning_effort: str | None = None, + api_key_provider: AsyncAPIKeyProvider | None = None, ) -> None: + # Keep the construction-time value separate from the live supplier. + # ChatSession passes export_connection_info() into ScratchpadManager, + # whose subprocess/env boundary cannot carry a callable. The main-process + # async OpenAI client can ask the supplier for a fresh bearer per request. self._api_key = api_key self._base_url = base_url self._ssl_verify = ssl_verify @@ -860,12 +867,31 @@ def __init__( import httpx + client_api_key = api_key_provider if api_key_provider is not None else api_key if api_version and _is_azure_endpoint(base_url): # Azure OpenAI: use the dedicated client which handles deployment # URL construction and api-version automatically. + if callable(client_api_key): + # AsyncAzureOpenAI._prepare_options fully overrides the base + # hook and never chains to super(), so AsyncOpenAI's + # _refresh_api_key never runs and the supplier is never + # awaited. The base __init__ has already replaced a callable + # api_key with "", so the client would send an empty api-key + # header on every request and 401 forever. Refuse at + # construction instead of failing on the first call. + # + # Keyed on the resolved value, not on `api_key_provider`: the + # `api_key` parameter is annotated `str` but nothing enforces + # that at runtime, and a callable arriving through it reaches + # the same SDK path with the same empty header. + raise EndpointConfigurationError( + "Azure OpenAI cannot refresh credentials per request: " + "AsyncAzureOpenAI ignores a callable api_key. Pass a " + "static api_key for Azure endpoints." + ) azure_kwargs: dict = {"api_version": api_version} - if api_key: - azure_kwargs["api_key"] = api_key + if client_api_key: + azure_kwargs["api_key"] = client_api_key if base_url: azure_kwargs["azure_endpoint"] = base_url if not ssl_verify: @@ -873,8 +899,8 @@ def __init__( self._client = AsyncAzureOpenAI(**azure_kwargs) else: kwargs: dict = {} - if api_key: - kwargs["api_key"] = api_key + if client_api_key: + kwargs["api_key"] = client_api_key if base_url: kwargs["base_url"] = base_url if not ssl_verify: diff --git a/anton/core/llm/provider.py b/anton/core/llm/provider.py index 3035c495..5cca667f 100644 --- a/anton/core/llm/provider.py +++ b/anton/core/llm/provider.py @@ -482,6 +482,23 @@ class TokenLimitExceeded(Exception): """Raised when the LLM returns 429 due to billing/token limits.""" +class ProviderAuthError(ConnectionError): + """Raised when a provider rejects its credential with HTTP 401. + + The type distinguishes an authentication refusal from unrelated + ``ConnectionError`` failures. It remains a ``ConnectionError`` subclass so + in-process callers written against the previous 401 mapping keep working. + The client stamps ``role`` on a terminal refusal so hosts can attribute + the recovery action to the provider that actually failed. A refusal is + terminal after a failed confirmation, or immediately when a stream has + emitted an event and replay would duplicate output. + """ + + def __init__(self, message: str, *, role: str | None = None): + super().__init__(message) + self.role = role + + class StructuredOutputError(ValueError): """Raised when a forced-tool-call structured-output call yields no usable call. diff --git a/anton/core/session.py b/anton/core/session.py index a2d98567..40e8c789 100644 --- a/anton/core/session.py +++ b/anton/core/session.py @@ -44,6 +44,7 @@ EndpointConfigurationError, LLMResponse, ModelUnavailableError, + ProviderAuthError, ProviderOverloadedError, StreamComplete, StreamContextCompacted, @@ -587,18 +588,14 @@ def _verifier_error_type(exc: BaseException | None) -> str: def _is_provider_auth_error(exc: BaseException) -> bool: - """A provider-auth 401 — anton's "Invalid API key — …" copy from - `openai.py`/`anthropic.py` (ENG-1310): the credential is wrong, not the - request, so retrying can't succeed either. The substring match mirrors - cowork-server's `turn_errors.is_auth_error()`; the `isinstance` check is - an anton-only narrowing on top of it (both 401 raise sites always type - it this way, so it's a no-op in practice) — anything else (a bare - "temporarily unavailable" ConnectionError) is a different failure. - - Shared by both `turn_stream` re-raise sites so the check can't drift - between them (review feedback on ENG-1310). + """Whether ``exc`` is the canonical provider HTTP-401 mapping. + + ``LLMClient`` propagates this after a failed confirmation, or immediately + when a stream has emitted an event and cannot be replayed. The session must + propagate either terminal refusal, while unrelated ``ConnectionError`` + values keep their existing recovery behavior. """ - return isinstance(exc, ConnectionError) and "invalid api key" in str(exc).lower() + return isinstance(exc, ProviderAuthError) # Shared closing instruction for every path that hands control back to the @@ -4181,8 +4178,10 @@ async def _turn_stream_inner( ): raise - # Same reasoning applies to a provider-auth 401 (ENG-1310) - # — see _is_provider_auth_error. + # LLMClient propagates a typed provider-auth 401 after a + # failed confirmation, or immediately after stream output + # to avoid replay. Either terminal refusal must reach the + # host's reconnect/update-key card. if _is_provider_auth_error(_agent_exc): raise @@ -4389,13 +4388,9 @@ async def _turn_stream_inner( # mapping exists server-side. raise if _is_provider_auth_error(e): - # Same reasoning for a provider-auth 401 — see - # _is_provider_auth_error. cowork-server's - # turn_errors.is_auth_error() matches this exact - # text and renders the "Reconnect MindsHub" / - # BYOK-key action card, but only if the exception - # propagates instead of being flattened into chat - # text here (ENG-1310). + # Preserve a refusal after failed confirmation, + # or the first refusal after stream output, for + # the host's auth-error mapping. raise fallback = f"An unexpected error occurred: {e}. Please try again or rephrase your request." assistant_text_parts.append(fallback) @@ -5499,6 +5494,23 @@ async def _stream_and_handle_tools( ) if not retrying: break + except ProviderAuthError: + # The client already made the one bounded confirmation + # attempt. ProviderAuthError subclasses ConnectionError, + # so it must stay ahead of the broad transient-verdict + # catch below or a confirmed verifier 401 is swallowed. + # + # This is deliberately unlike the wallet 402 and model 403 + # on this same call, which break and latch quietly per the + # aux-surface reasoning at _DENIED_VERDICT_ERRORS: those + # say the request was refused, while a confirmed 401 says + # the credential is dead for every later call too. ENG-2116 + # requires a confirmed refusal on a required planning, + # coding, or verifier call to reach the host as + # provider_auth so the reconnect action survives. The cost + # is that a turn whose reply already streamed still ends in + # an auth error. + raise except _DENIED_VERDICT_ERRORS as exc: # Deterministic denial — see _DENIED_VERDICT_ERRORS (and the # ordering note there: this clause must stay ahead of the diff --git a/pyproject.toml b/pyproject.toml index 2b08eb59..3ddabc5b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,8 @@ classifiers = [ ] dependencies = [ "anthropic>=0.42.0", - "openai>=1.0", + # Async API-key suppliers are part of the OpenAI 2.21 client contract. + "openai>=2.21.0", # Imported at module scope; arrived transitively via anthropic/openai until # both moved to httpx2. "httpx>=0.27,<1", diff --git a/tests/test_chat_error_action_default.py b/tests/test_chat_error_action_default.py index efccacf5..dfefd7db 100644 --- a/tests/test_chat_error_action_default.py +++ b/tests/test_chat_error_action_default.py @@ -6,19 +6,28 @@ provider-auth 401 is equally deterministic (ENG-1310 made it propagate instead of flattening into chat text), but nothing steered its default away from "retry" — a gap three past PRs (#236, #247, #288) flagged for this -exact ConnectionError-defaults-to-retry pattern. This pins the fix. +error-default pattern. This pins the typed distinction. """ from __future__ import annotations from anton.chat import _default_turn_error_action -from anton.core.llm.provider import EndpointConfigurationError, ModelUnavailableError, TokenLimitExceeded +from anton.core.llm.provider import ( + EndpointConfigurationError, + ModelUnavailableError, + ProviderAuthError, + TokenLimitExceeded, +) _AUTH_ERROR_MESSAGE = "Invalid API key — check your OpenAI API key configuration." def test_provider_auth_error_defaults_to_setup(): - assert _default_turn_error_action(ConnectionError(_AUTH_ERROR_MESSAGE)) == "setup" + assert _default_turn_error_action(ProviderAuthError(_AUTH_ERROR_MESSAGE)) == "setup" + + +def test_invalid_key_text_without_the_canonical_type_defaults_to_retry(): + assert _default_turn_error_action(ConnectionError(_AUTH_ERROR_MESSAGE)) == "retry" def test_generic_connection_error_still_defaults_to_retry(): diff --git a/tests/test_client.py b/tests/test_client.py index d93fa572..9ba919e8 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,12 +1,26 @@ from __future__ import annotations -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest +from pydantic import BaseModel + from anton.config.settings import AntonSettings from anton.core.llm.client import LLMClient -from anton.core.llm.provider import LLMProvider, LLMResponse, Usage +from anton.core.llm.provider import ( + LLMProvider, + LLMResponse, + ProviderAuthError, + StreamTextDelta, + Usage, +) + + +class _Schema(BaseModel): + """Minimal forced-tool-call schema for the structured-output role tests.""" + + answer: str @pytest.fixture() @@ -72,6 +86,166 @@ async def test_plan_passes_tools(self, mock_providers): call_kwargs = planning.complete.call_args.kwargs assert call_kwargs["tools"] == tools + async def test_plan_confirms_one_auth_refusal_then_returns_success( + self, mock_providers + ): + planning, coding = mock_providers + planning.complete = AsyncMock( + side_effect=[ + ProviderAuthError("Invalid API key"), + LLMResponse(content="recovered", usage=Usage()), + ] + ) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + response = await client.plan(system="sys", messages=[]) + + assert response.content == "recovered" + assert planning.complete.await_count == 2 + + async def test_plan_propagates_second_auth_refusal(self, mock_providers): + planning, coding = mock_providers + planning.complete = AsyncMock( + side_effect=ProviderAuthError("Invalid API key") + ) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ProviderAuthError) as err: + await client.plan(system="sys", messages=[]) + + assert planning.complete.await_count == 2 + assert err.value.role == "planning" + + async def test_plan_does_not_retry_unrelated_connection_error( + self, mock_providers + ): + planning, coding = mock_providers + planning.complete = AsyncMock(side_effect=ConnectionError("network down")) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ConnectionError, match="network down"): + await client.plan(system="sys", messages=[]) + + planning.complete.assert_awaited_once() + + async def test_code_marks_a_confirmed_auth_failure_with_its_role( + self, mock_providers + ): + planning, coding = mock_providers + coding.complete = AsyncMock(side_effect=ProviderAuthError("Invalid API key")) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ProviderAuthError) as err: + await client.code(system="sys", messages=[]) + + assert coding.complete.await_count == 2 + assert err.value.role == "coding" + + async def test_plan_stream_confirms_auth_before_first_event( + self, mock_providers + ): + planning, coding = mock_providers + calls = 0 + + async def stream(**kwargs): + nonlocal calls + calls += 1 + if calls == 1: + raise ProviderAuthError("Invalid API key") + yield StreamTextDelta(text="recovered") + + planning.stream = MagicMock(side_effect=stream) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + events = [ + event + async for event in client.plan_stream(system="sys", messages=[]) + ] + + assert events == [StreamTextDelta(text="recovered")] + assert calls == 2 + + async def test_plan_stream_propagates_second_auth_refusal( + self, mock_providers + ): + planning, coding = mock_providers + calls = 0 + + async def stream(**kwargs): + nonlocal calls + calls += 1 + raise ProviderAuthError("Invalid API key") + yield # pragma: no cover - preserve the async-iterator protocol + + planning.stream = MagicMock(side_effect=stream) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ProviderAuthError) as err: + async for _ in client.plan_stream(system="sys", messages=[]): + pass + + assert calls == 2 + assert err.value.role == "planning" + + async def test_plan_stream_never_replays_after_an_event_was_yielded( + self, mock_providers + ): + planning, coding = mock_providers + calls = 0 + + async def stream(**kwargs): + nonlocal calls + calls += 1 + yield StreamTextDelta(text="partial") + raise ProviderAuthError("Invalid API key") + + planning.stream = MagicMock(side_effect=stream) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + events = [] + + with pytest.raises(ProviderAuthError) as err: + async for event in client.plan_stream(system="sys", messages=[]): + events.append(event) + + assert events == [StreamTextDelta(text="partial")] + assert calls == 1 + assert err.value.role == "planning" + class TestRouterRole: """Summarization runs on the router role, which defaults to the coding @@ -92,6 +266,24 @@ async def test_summarize_defaults_to_coding_role(self, mock_providers): assert coding.complete.call_args.kwargs["model"] == "model-b" assert result.content == "code" + async def test_summarize_fallback_attributes_auth_to_coding( + self, mock_providers + ): + planning, coding = mock_providers + coding.complete = AsyncMock(side_effect=ProviderAuthError("Invalid API key")) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ProviderAuthError) as err: + await client.summarize(system="sys", messages=[]) + + assert coding.complete.await_count == 2 + assert err.value.role == "coding" + async def test_summarize_uses_distinct_router_model(self, mock_providers): planning, coding = mock_providers router = AsyncMock(spec=LLMProvider) @@ -116,6 +308,50 @@ async def test_summarize_uses_distinct_router_model(self, mock_providers): assert client.router_provider is router assert client.router_model == "model-c" + async def test_gate_confirms_one_router_auth_refusal(self, mock_providers): + planning, coding = mock_providers + router = AsyncMock(spec=LLMProvider) + router.complete = AsyncMock( + side_effect=[ + ProviderAuthError("Invalid API key"), + LLMResponse(content="delegate", usage=Usage()), + ] + ) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + router_provider=router, + router_model="model-c", + ) + + response = await client.gate(system="sys", messages=[]) + + assert response.content == "delegate" + assert router.complete.await_count == 2 + + async def test_gate_propagates_second_router_auth_refusal(self, mock_providers): + planning, coding = mock_providers + router = AsyncMock(spec=LLMProvider) + router.complete = AsyncMock( + side_effect=ProviderAuthError("Invalid API key") + ) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + router_provider=router, + router_model="model-c", + ) + + with pytest.raises(ProviderAuthError) as err: + await client.gate(system="sys", messages=[]) + + assert router.complete.await_count == 2 + assert err.value.role == "router" + def test_router_accessors_fall_back_to_coding(self, mock_providers): planning, coding = mock_providers client = LLMClient( @@ -128,6 +364,48 @@ def test_router_accessors_fall_back_to_coding(self, mock_providers): assert client.router_model == "model-b" +class TestStructuredOutputRole: + """The forced-tool-call path shares `_generate_object_with`, so its auth + confirmation and role stamp are separate from the plan/code call sites + tested above. Without these, dropping the wrapper or swapping the two role + strings leaves the suite green while the host attributes a reconnect card + to a provider that did not fail.""" + + async def test_generate_object_confirms_and_attributes_to_planning( + self, mock_providers + ): + planning, coding = mock_providers + planning.complete = AsyncMock(side_effect=ProviderAuthError("Invalid API key")) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ProviderAuthError) as err: + await client.generate_object(_Schema, system="sys", messages=[]) + + assert planning.complete.await_count == 2 + assert err.value.role == "planning" + + async def test_generate_object_code_attributes_to_coding(self, mock_providers): + planning, coding = mock_providers + coding.complete = AsyncMock(side_effect=ProviderAuthError("Invalid API key")) + client = LLMClient( + planning_provider=planning, + planning_model="model-a", + coding_provider=coding, + coding_model="model-b", + ) + + with pytest.raises(ProviderAuthError) as err: + await client.generate_object_code(_Schema, system="sys", messages=[]) + + assert coding.complete.await_count == 2 + assert err.value.role == "coding" + + class TestLLMClientFromSettings: def test_from_settings_creates_client(self): from anton.core.llm.anthropic import AnthropicProvider diff --git a/tests/test_openai_dynamic_api_key.py b/tests/test_openai_dynamic_api_key.py new file mode 100644 index 00000000..5f09af1e --- /dev/null +++ b/tests/test_openai_dynamic_api_key.py @@ -0,0 +1,197 @@ +from __future__ import annotations + +from unittest.mock import patch + +import httpx +import openai +import pytest + +from anton.core.llm.client import LLMClient +from anton.core.llm.openai import OpenAIProvider +from anton.core.llm.provider import EndpointConfigurationError + + +async def test_live_requests_reread_api_key_without_rebuilding_provider(): + current_token = "token-a" + authorization_headers: list[str] = [] + + async def api_key_provider() -> str: + return current_token + + def handler(request: httpx.Request) -> httpx.Response: + authorization_headers.append(request.headers["authorization"]) + return httpx.Response( + 200, + json={ + "id": "chatcmpl-test", + "object": "chat.completion", + "created": 0, + "model": "test-model", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": "ok"}, + "finish_reason": "stop", + } + ], + "usage": { + "prompt_tokens": 1, + "completion_tokens": 1, + "total_tokens": 2, + }, + }, + ) + + real_client = openai.AsyncOpenAI + transport = httpx.MockTransport(handler) + + def build_client(**kwargs): + kwargs["http_client"] = httpx.AsyncClient(transport=transport) + return real_client(**kwargs) + + with patch( + "anton.core.llm.openai.openai.AsyncOpenAI", side_effect=build_client + ): + provider = OpenAIProvider( + api_key="token-a", + api_key_provider=api_key_provider, + base_url="https://gateway.test/v1", + ) + + try: + await provider.complete(model="test-model", system="sys", messages=[]) + current_token = "token-b" + await provider.complete(model="test-model", system="sys", messages=[]) + finally: + await provider._client.close() + + assert authorization_headers == ["Bearer token-a", "Bearer token-b"] + assert provider.export_connection_info().api_key == "token-a" + + +def _completion_body() -> dict: + return { + "id": "chatcmpl-test", + "object": "chat.completion", + "created": 0, + "model": "test-model", + "choices": [ + { + "index": 0, + "message": {"role": "assistant", "content": "done"}, + "finish_reason": "stop", + } + ], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2}, + } + + +async def test_a_401_on_the_stale_token_is_confirmed_with_the_rotated_one(): + """The two halves of ENG-2116 are only useful together. + + ``test_live_requests_reread_api_key_without_rebuilding_provider`` proves the + per-request re-read but never returns a 401, and ``tests/test_client.py`` + proves the one bounded retry against mocks that carry no credential. Neither + can assert that the confirmation attempt used the ROTATED token, which is + the whole mechanism. Refactoring either half apart from the other would keep + both green while the stale-JWT symptom returned. + + The 401 body is the production shape: Keycloak answered ``invalid_token`` + with reason "Token is not active" for all seven named ENG-2116 failures. + """ + live = {"credential": "token-a"} + authorization_headers: list[str] = [] + + async def api_key_provider() -> str: + return live["credential"] + + def handler(request: httpx.Request) -> httpx.Response: + authorization = request.headers["authorization"] + authorization_headers.append(authorization) + if authorization == "Bearer token-a": + # The desktop refresh lands between the refusal and the retry. + live["credential"] = "token-b" + return httpx.Response( + 401, + json={ + "error": { + "message": "Token is not active", + "code": "invalid_token", + } + }, + ) + return httpx.Response(200, json=_completion_body()) + + real_client = openai.AsyncOpenAI + transport = httpx.MockTransport(handler) + + def build_client(**kwargs): + kwargs["http_client"] = httpx.AsyncClient(transport=transport) + return real_client(**kwargs) + + with patch( + "anton.core.llm.openai.openai.AsyncOpenAI", side_effect=build_client + ): + provider = OpenAIProvider( + api_key="token-a", + api_key_provider=api_key_provider, + base_url="https://gateway.test/v1", + ) + + client = LLMClient( + planning_provider=provider, + planning_model="test-model", + coding_provider=provider, + coding_model="test-model", + ) + try: + response = await client.plan(system="sys", messages=[]) + finally: + await provider._client.close() + + assert response.content == "done" + # The header sequence is what separates "retried and got lucky" from + # "retried with the rotated credential". + assert authorization_headers == ["Bearer token-a", "Bearer token-b"] + + +async def test_azure_refuses_a_credential_supplier_it_cannot_await(): + """AsyncAzureOpenAI never awaits a callable api_key. + + Its ``_prepare_options`` override does not chain to super(), so + ``AsyncOpenAI._refresh_api_key`` never runs, and the base ``__init__`` has + already replaced the callable with "". Constructing such a client would send + an empty ``api-key`` header on every request and 401 forever, with no typed + error to explain it. + """ + + async def api_key_provider() -> str: # pragma: no cover - never awaited + return "token-a" + + with pytest.raises(EndpointConfigurationError, match="callable api_key"): + OpenAIProvider( + api_key="static-key", + api_key_provider=api_key_provider, + api_version="2024-06-01", + base_url="https://example.openai.azure.com", + ) + + +async def test_azure_refuses_a_callable_passed_as_the_static_api_key(): + """``api_key`` is annotated ``str``, but nothing enforces that at runtime. + + A supplier handed through ``api_key`` instead of ``api_key_provider`` reaches + the same ``AsyncAzureOpenAI`` path and produces the same permanently empty + ``api-key`` header, so the refusal keys on the resolved value rather than on + which parameter carried it. + """ + + async def api_key_provider() -> str: # pragma: no cover - never awaited + return "token-a" + + with pytest.raises(EndpointConfigurationError, match="callable api_key"): + OpenAIProvider( + api_key=api_key_provider, # type: ignore[arg-type] + api_version="2024-06-01", + base_url="https://example.openai.azure.com", + ) diff --git a/tests/test_session_auth_error_reraise.py b/tests/test_session_auth_error_reraise.py index 8e420695..8906a5c4 100644 --- a/tests/test_session_auth_error_reraise.py +++ b/tests/test_session_auth_error_reraise.py @@ -1,22 +1,19 @@ -"""ENG-1310 — a persistent provider-auth failure must propagate, not flatten. +"""A terminal provider-auth failure propagates without flattening. -A `ConnectionError` (anton's "Invalid API key — …" copy for a 401 from the -LLM gateway, see `openai.py`/`anthropic.py`) used to fall into a generic +A `ProviderAuthError` (anton's typed 401 mapping from +`openai.py`/`anthropic.py`) used to fall into a generic `except Exception` branch and get dumped into the chat as "An unexpected error occurred: Invalid API key … Please try again or rephrase your request." instead of reaching cowork-server's `turn_errors.is_auth_error()`, which already renders the correct "Reconnect MindsHub" / BYOK-key card. -Two sites in `turn_stream` needed the same auth-shaped check, mirroring how -ENG-1139 treats `EndpointConfigurationError` (also deterministic — retrying -can't fix it): +`LLMClient` confirms a typed refusal once before output. A streaming refusal +after output propagates immediately to avoid replay. The session must propagate +either terminal refusal without spending its generic retry budget or injecting +misleading recovery history. -1. The immediate re-raise at the top of the retry loop — an invalid key - fails on the FIRST attempt instead of burning the count-based retry - budget on doomed retries. -2. The retry-exhaustion fallback's own wrap-up call — belt-and-suspenders - for the case where retries were legitimately spent on a DIFFERENT - failure and the key only turns out to be bad on the final summary call. +Both session re-raise sites remain covered: the main turn loop and the final +wrap-up call made after unrelated errors exhausted their own retry budget. """ from __future__ import annotations @@ -28,7 +25,7 @@ from tests.conftest import make_mock_llm -from anton.core.llm.provider import EndpointConfigurationError +from anton.core.llm.provider import EndpointConfigurationError, ProviderAuthError from anton.core.session import ChatSession, ChatSessionConfig, _is_provider_auth_error _AUTH_ERROR_MESSAGE = "Invalid API key — check your OpenAI API key configuration." @@ -83,19 +80,17 @@ async def _run_turn(session: ChatSession, prompt: str = "what's in my inbox?"): return events -async def test_persistent_auth_failure_fails_immediately_without_wasting_retries(workspace): - """An invalid key can't be fixed by retrying — it must fail on the first - attempt, the same way EndpointConfigurationError (ENG-1139) does, not - after burning the count-based retry budget on doomed re-attempts.""" +async def test_session_does_not_retry_a_confirmed_auth_failure(workspace): + """The LLMClient boundary already confirmed this refusal once.""" mock_llm = make_mock_llm() - script = _AlwaysRaisingPlanStream(ConnectionError(_AUTH_ERROR_MESSAGE)) + script = _AlwaysRaisingPlanStream(ProviderAuthError(_AUTH_ERROR_MESSAGE)) mock_llm.plan_stream = script session = ChatSession(ChatSessionConfig(llm_client=mock_llm, workspace=workspace)) - with pytest.raises(ConnectionError, match="Invalid API key"): + with pytest.raises(ProviderAuthError, match="Invalid API key"): await _run_turn(session) - assert script.calls == 1, "an auth failure must not be retried" + assert script.calls == 1, "the session must not add a third provider attempt" async def test_auth_failure_on_the_final_wrapup_call_still_reraises(workspace): @@ -107,12 +102,12 @@ async def test_auth_failure_on_the_final_wrapup_call_still_reraises(workspace): mock_llm = make_mock_llm() script = _ScriptedExceptionPlanStream( [RuntimeError("boom"), RuntimeError("boom"), RuntimeError("boom"), - ConnectionError(_AUTH_ERROR_MESSAGE)] + ProviderAuthError(_AUTH_ERROR_MESSAGE)] ) mock_llm.plan_stream = script session = ChatSession(ChatSessionConfig(llm_client=mock_llm, workspace=workspace)) - with pytest.raises(ConnectionError, match="Invalid API key"): + with pytest.raises(ProviderAuthError, match="Invalid API key"): await _run_turn(session) # 3 retry attempts (max_auto_retries=2) on the unrelated RuntimeError, @@ -120,11 +115,9 @@ async def test_auth_failure_on_the_final_wrapup_call_still_reraises(workspace): assert script.calls == 4 -def test_is_provider_auth_error_matches_only_the_invalid_key_copy(): - """The predicate both re-raise sites share — pinned directly so the two - call sites can't drift from each other (review feedback on ENG-1310).""" - assert _is_provider_auth_error(ConnectionError(_AUTH_ERROR_MESSAGE)) - assert _is_provider_auth_error(ConnectionError("INVALID API KEY — case insensitive")) +def test_is_provider_auth_error_matches_only_the_canonical_type(): + assert _is_provider_auth_error(ProviderAuthError(_AUTH_ERROR_MESSAGE)) + assert not _is_provider_auth_error(ConnectionError(_AUTH_ERROR_MESSAGE)) assert not _is_provider_auth_error(ConnectionError("temporarily unavailable")) assert not _is_provider_auth_error(RuntimeError(_AUTH_ERROR_MESSAGE)) @@ -149,7 +142,7 @@ async def test_endpoint_configuration_error_on_the_final_wrapup_call_still_rerai async def test_generic_connection_error_still_falls_back_to_chat_text(workspace): - """Only the auth-shaped message re-raises — an unrelated ConnectionError + """Only the typed auth refusal re-raises — an unrelated ConnectionError (e.g. the generic 'temporarily unavailable' case) keeps the existing fallback-text behavior instead of failing the turn.""" mock_llm = make_mock_llm() diff --git a/tests/test_status_error_mapper.py b/tests/test_status_error_mapper.py index 529a3bc3..f3ed985d 100644 --- a/tests/test_status_error_mapper.py +++ b/tests/test_status_error_mapper.py @@ -3,8 +3,7 @@ One mapper (`openai._raise_for_status_error`) serves all four call paths (chat/stream × completions/responses). These tests pin the mapping policy: -- 401 → ConnectionError with the exact invalid-key copy (cowork-server's - provider_auth detection string-matches it). +- 401 → ProviderAuthError with the exact invalid-key copy. - 429 + quota detail → TokenLimitExceeded (and it outranks any 403 logic). - 403 + structured gateway code → ModelUnavailableError carrying code+model, with actionable copy per code. @@ -32,6 +31,7 @@ ContentValidationError, EndpointConfigurationError, ModelUnavailableError, + ProviderAuthError, TokenLimitExceeded, TransientProviderError, classify_transient, @@ -96,24 +96,23 @@ def test_sdk_unwraps_error_envelope(): # ── 401 ─────────────────────────────────────────────────────────────── -def test_401_maps_to_invalid_key_connection_error(): +def test_401_json_maps_to_provider_auth_error(): exc = _sdk_error(401, json_body={"error": {"message": "bad key"}}) - with pytest.raises(ConnectionError) as err: + with pytest.raises(ProviderAuthError) as err: _raise_for_status_error(exc, "sonnet") - # cowork-server's provider_auth detection keys on this exact phrase. + # Keep the user-facing copy stable even though downstream classification + # now keys on the canonical exception type. assert "Invalid API key" in str(err.value) assert not isinstance(err.value, ModelUnavailableError) - # session.py's own re-raise checks (ENG-1310) key on this predicate, not - # the raw text — pin against the REAL mapper output so an edit to this - # copy that drops "invalid api key" fails here too, not just silently in - # production (review feedback on ENG-1310). + # Pin the real mapper output to the same canonical predicate used by the + # session; unrelated ConnectionError text must not enter this path. assert _is_provider_auth_error(err.value) def test_401_html_body_maps_to_invalid_key(): # nginx auth walls return HTML — the 401 branch must not need a body. exc = _sdk_error(401, text_body="401 Authorization Required") - with pytest.raises(ConnectionError) as err: + with pytest.raises(ProviderAuthError) as err: _raise_for_status_error(exc, "sonnet") assert "Invalid API key" in str(err.value) assert _is_provider_auth_error(err.value) @@ -261,7 +260,7 @@ def _wire_shaped_error(status_code, body): body — bypassing the SDK's parse-and-unwrap on purpose. This is the only way to hand the mapper an envelope-shaped ``exc.body``: the pinned SDK always peels ``error`` (see test_sdk_unwraps_error_envelope), but - anton's pyproject allows ``openai>=1.0`` and proxies exist that re-wrap, + anton's pyproject allows newer OpenAI 2.x releases and proxies can re-wrap, so the mapper's envelope fallback must stay pinned by a test that the MockTransport route physically cannot produce.""" # A real MindsHub host — the origin is load-bearing since ENG-1693, and @@ -483,8 +482,8 @@ def test_unrelated_invalid_request_error_falls_through_to_generic(): def test_content_validation_error_is_a_connection_error(): - # Subclasses ConnectionError so call sites that only know the legacy - # mapping (string-matching "invalid api key" etc.) keep working unchanged. + # Preserve generic ConnectionError compatibility for legacy callers while + # typed readers distinguish content-shape failures from provider auth. assert issubclass(ContentValidationError, ConnectionError) @@ -598,7 +597,7 @@ def test_wallet_denial_code_reads_both_dialects(): # ── the anthropic twin (ENG-1169) ───────────────────────────────────── -def test_anthropic_401_maps_to_invalid_key_connection_error(): +def test_anthropic_401_maps_to_provider_auth_error(): # No real-SDK 401 coverage existed for the anthropic mapper before this # (review feedback on ENG-1310) — only openai's 401 was pinned against # actual SDK output; anthropic's own "Invalid API key — …" copy was @@ -607,7 +606,7 @@ def test_anthropic_401_maps_to_invalid_key_connection_error(): "type": "authentication_error", "message": "invalid x-api-key", }}) - with pytest.raises(ConnectionError) as err: + with pytest.raises(ProviderAuthError) as err: _raise_anthropic(exc, model="claude-sonnet") assert "Invalid API key" in str(err.value) assert _is_provider_auth_error(err.value) diff --git a/tests/test_thalamus.py b/tests/test_thalamus.py index 9bcf4381..4e865b89 100644 --- a/tests/test_thalamus.py +++ b/tests/test_thalamus.py @@ -11,9 +11,12 @@ from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock +from anton.core.llm.client import LLMClient from anton.core.session import ChatSession, ChatSessionConfig from anton.core.llm.provider import ( + LLMProvider, LLMResponse, + ProviderAuthError, StreamComplete, StreamTextDelta, ToolCall, @@ -354,6 +357,28 @@ async def test_thalamus_failure_falls_back_to_planning(self): assert reply == "Handled anyway." llm.plan.assert_called_once() + async def test_confirmed_router_auth_failure_falls_back_to_planning(self): + planning = AsyncMock(spec=LLMProvider) + planning.complete = AsyncMock(return_value=_response("Handled anyway.")) + coding = AsyncMock(spec=LLMProvider) + router = AsyncMock(spec=LLMProvider) + router.complete = AsyncMock(side_effect=ProviderAuthError("Invalid API key")) + llm = LLMClient( + planning_provider=planning, + planning_model="planning-model", + coding_provider=coding, + coding_model="coding-model", + router_provider=router, + router_model="router-model", + ) + session = ChatSession(ChatSessionConfig(llm_client=llm, router_enabled=True)) + + reply = await session.turn("hi") + + assert reply == "Handled anyway." + assert router.complete.await_count == 2 + planning.complete.assert_awaited_once() + async def test_image_turns_skip_thalamus(self): llm = make_mock_llm() llm.gate = AsyncMock() diff --git a/tests/test_verifier_truncation.py b/tests/test_verifier_truncation.py index 2c802527..3ca673b3 100644 --- a/tests/test_verifier_truncation.py +++ b/tests/test_verifier_truncation.py @@ -29,6 +29,7 @@ from anton.core.llm.client import LLMClient from anton.core.llm.provider import ( LLMResponse, + ProviderAuthError, StreamComplete, StructuredOutputError, ToolCall, @@ -457,6 +458,24 @@ async def fake_verdict(_schema, *, system, messages, max_tokens): assert calls == [_VERIFIER_TOKEN_BUDGETS[0]], "must not pay for a hopeless retry" +async def test_confirmed_verifier_auth_failure_propagates(workspace): + """A second typed 401 is terminal even though the type is also an OSError.""" + mock_llm = make_mock_llm() + mock_llm.generate_object_code = AsyncMock( + side_effect=ProviderAuthError("Invalid API key") + ) + + session = _session_that_uses_a_tool(mock_llm, workspace) + try: + with pytest.raises(ProviderAuthError, match="Invalid API key"): + async for _ in session.turn_stream("build me a dashboard"): + pass + finally: + await session.close() + + mock_llm.generate_object_code.assert_awaited_once() + + async def test_attempts_are_bounded_and_repeat_each_turn(workspace): """Truncation retries are bounded by the budget list — and are re-tried on a later turn rather than latched off. diff --git a/uv.lock b/uv.lock index 863b56bb..54c3b5ec 100644 --- a/uv.lock +++ b/uv.lock @@ -199,7 +199,7 @@ requires-dist = [ { name = "anthropic", specifier = ">=0.42.0" }, { name = "dill", specifier = "==0.3.8" }, { name = "httpx", specifier = ">=0.27,<1" }, - { name = "openai", specifier = ">=1.0" }, + { name = "openai", specifier = ">=2.21.0" }, { name = "packaging", specifier = ">=21.0" }, { name = "pillow", marker = "extra == 'clipboard'", specifier = ">=12.3.0" }, { name = "prompt-toolkit", specifier = ">=3.0" },