[ENG-1310] Unable to authenticate against the llm model invalid api key

anton/core/session.py

@@ -366,6 +366,19 @@ def _safe_error_detail(exc: BaseException) -> str:
     return name
 
 
+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. Narrow substring match
+    mirrors cowork-server's `turn_errors.is_auth_error()` — 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).
+    """
+    return isinstance(exc, ConnectionError) and "invalid api key" in str(exc).lower()
+
+
 # Shared closing instruction for every path that hands control back to the
 # user (STUCK, budget-exhausted, verifier-call failure): a plain self-
 # assessment of solvability, not just a status dump. Without this, a
@@ -2876,6 +2889,11 @@ async def _turn_stream_inner(
                     ):
                         raise
 
+                    # Same reasoning applies to a provider-auth 401 (ENG-1310)
+                    # — see _is_provider_auth_error.
+                    if _is_provider_auth_error(_agent_exc):
+                        raise
+
                     # ENG-673: a mid-stream transient failure that had NO prior
                     # retry (overload smuggled into a 200, or a truncated stream).
                     # Back off and retry the SAME step within a per-turn budget —
@@ -2989,16 +3007,29 @@ async def _turn_stream_inner(
                                 if isinstance(event, StreamTextDelta):
                                     assistant_text_parts.append(event.text)
                                 yield event
-                        except (TokenLimitExceeded, ModelUnavailableError):
-                            # Curated provider failures must FAIL the turn, not
-                            # get wrapped into assistant prose: the server maps
-                            # them to actionable error cards (token_limit /
-                            # model-unavailable), which can only fire when the
-                            # exception propagates. Wrapping them as text is
-                            # how "Server returned 403" ended up mid-chat with
-                            # "please rephrase your request" advice.
-                            raise
                         except Exception as e:
+                            if isinstance(e, (TokenLimitExceeded, ModelUnavailableError, EndpointConfigurationError)):
+                                # Curated provider failures must FAIL the turn, not
+                                # get wrapped into assistant prose: the server maps
+                                # them to actionable error cards (token_limit /
+                                # model-unavailable / endpoint-config), which can
+                                # only fire when the exception propagates. Wrapping
+                                # them as text is how "Server returned 403" ended
+                                # up mid-chat with "please rephrase your request"
+                                # advice. EndpointConfigurationError added here to
+                                # match the immediate re-raise site above — this
+                                # wrap-up call had been the one place it still fell
+                                # through (review feedback on ENG-1310).
+                                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).
+                                raise
                             fallback = f"An unexpected error occurred: {e}. Please try again or rephrase your request."
                             assistant_text_parts.append(fallback)
                             yield StreamTextDelta(text=fallback)

tests/test_session_auth_error_reraise.py

@@ -0,0 +1,168 @@
+"""ENG-1310 — a persistent provider-auth failure must propagate, not flatten.
+
+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
+`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):
+
+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.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+
+from tests.conftest import make_mock_llm
+
+from anton.core.llm.provider import EndpointConfigurationError
+from anton.core.session import ChatSession, ChatSessionConfig, _is_provider_auth_error
+
+_AUTH_ERROR_MESSAGE = "Invalid API key — check your OpenAI API key configuration."
+
+
+@pytest.fixture()
+def workspace():
+    # Keep scratchpad venvs inside the repo workspace (pytest runs sandboxed
+    # and can't write to the real home directory).
+    base = Path(__file__).resolve().parents[1] / ".pytest-workspace"
+    base.mkdir(parents=True, exist_ok=True)
+    return MagicMock(base=base)
+
+
+class _AlwaysRaisingPlanStream:
+    """`plan_stream` fake that raises the same exception on every call —
+    every retry attempt AND the final wrap-up call see the same failure,
+    the way a genuinely invalid key does."""
+
+    def __init__(self, exc: Exception):
+        self._exc = exc
+        self.calls = 0
+
+    def __call__(self, **kwargs):
+        self.calls += 1
+        raise self._exc
+
+
+class _ScriptedExceptionPlanStream:
+    """`plan_stream` fake that raises a scripted sequence of exceptions, one
+    per call, holding on the last entry once the script runs out — so a
+    fixed prefix (e.g. retries that legitimately exhaust the count-based
+    budget) can be followed by a different failure on the final call."""
+
+    def __init__(self, excs: list[Exception]):
+        self._excs = list(excs)
+        self.calls = 0
+
+    def __call__(self, **kwargs):
+        self.calls += 1
+        idx = min(self.calls - 1, len(self._excs) - 1)
+        raise self._excs[idx]
+
+
+async def _run_turn(session: ChatSession, prompt: str = "what's in my inbox?"):
+    events = []
+    try:
+        async for event in session.turn_stream(prompt):
+            events.append(event)
+    finally:
+        await session.close()
+    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."""
+    mock_llm = make_mock_llm()
+    script = _AlwaysRaisingPlanStream(ConnectionError(_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"):
+        await _run_turn(session)
+
+    assert script.calls == 1, "an auth failure must not be retried"
+
+
+async def test_auth_failure_on_the_final_wrapup_call_still_reraises(workspace):
+    """Retries legitimately exhaust on a DIFFERENT, retryable failure — the
+    key only turns out to be bad on the retry-exhaustion fallback's own
+    wrap-up call. That must still propagate instead of flattening into chat
+    text, even though the auth error never triggered the fast-fail path
+    above."""
+    mock_llm = make_mock_llm()
+    script = _ScriptedExceptionPlanStream(
+        [RuntimeError("boom"), RuntimeError("boom"), RuntimeError("boom"),
+         ConnectionError(_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"):
+        await _run_turn(session)
+
+    # 3 retry attempts (max_auto_retries=2) on the unrelated RuntimeError,
+    # then the final direct wrap-up call hits the auth error.
+    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"))
+    assert not _is_provider_auth_error(ConnectionError("temporarily unavailable"))
+    assert not _is_provider_auth_error(RuntimeError(_AUTH_ERROR_MESSAGE))
+
+
+async def test_endpoint_configuration_error_on_the_final_wrapup_call_still_reraises(workspace):
+    """The wrap-up call's except block must treat EndpointConfigurationError
+    (ENG-1139 — also deterministic, also must default to 'setup' not
+    'retry') the same way the immediate re-raise site already does, instead
+    of flattening it into chat text (review feedback on ENG-1310)."""
+    mock_llm = make_mock_llm()
+    script = _ScriptedExceptionPlanStream(
+        [RuntimeError("boom"), RuntimeError("boom"), RuntimeError("boom"),
+         EndpointConfigurationError("The model endpoint returned 404.")]
+    )
+    mock_llm.plan_stream = script
+    session = ChatSession(ChatSessionConfig(llm_client=mock_llm, workspace=workspace))
+
+    with pytest.raises(EndpointConfigurationError):
+        await _run_turn(session)
+
+    assert script.calls == 4
+
+
+async def test_generic_connection_error_still_falls_back_to_chat_text(workspace):
+    """Only the auth-shaped message 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()
+    script = _AlwaysRaisingPlanStream(ConnectionError("temporarily unavailable"))
+    mock_llm.plan_stream = script
+    session = ChatSession(ChatSessionConfig(llm_client=mock_llm, workspace=workspace))
+
+    events = await _run_turn(session)
+
+    from anton.core.llm.provider import StreamTextDelta
+
+    fallback_text = "".join(
+        e.text for e in events if isinstance(e, StreamTextDelta)
+    )
+    assert "temporarily unavailable" in fallback_text
+    assert "unexpected error occurred" in fallback_text