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

anton/chat.py

@@ -20,7 +20,7 @@
     replace_at_image_paths,
     save_clipboard_image,
 )
-from anton.core.session import ChatSession, ChatSessionConfig
+from anton.core.session import ChatSession, ChatSessionConfig, _is_provider_auth_error
 from anton.core.llm.prompt_builder import SystemPromptContext
 from anton.core.llm.provider import (
     EndpointConfigurationError,
@@ -1279,6 +1279,24 @@ def _desktop_greeting(console: Console, settings) -> None:
 
 
 
+def _default_turn_error_action(exc: BaseException) -> str:
+    """Which action the turn-failure prompt should default to.
+
+    ModelUnavailableError (plan/kill-switch gate) and EndpointConfigurationError
+    (wrong base URL / route) are deterministic for the identical request —
+    retrying re-sends a doomed call — so the default steers to "setup" (switch
+    model / provider / fix endpoint). A provider-auth 401 (ENG-1310) is
+    equally deterministic — see `_is_provider_auth_error` — and is included
+    here for the same reason: three past PRs (#236, #247, #288) flagged that a
+    bare ConnectionError defaults to "retry" even when the failure can't
+    possibly be fixed by retrying (review feedback on ENG-1310). Any other
+    ConnectionError (transient) keeps "retry" as the default.
+    """
+    if isinstance(exc, (ModelUnavailableError, EndpointConfigurationError)) or _is_provider_auth_error(exc):
+        return "setup"
+    return "retry" if isinstance(exc, ConnectionError) else "setup"
+
+
 def run_chat(
     console: Console, settings: AntonSettings, *, resume: bool = False, first_run: bool = False, desktop_first_run: bool = False
 ) -> None:
@@ -2017,17 +2035,7 @@ def _bottom_toolbar():
                     "  (anton) Switch LLM provider, update API key, or retry?",
                     choices=["setup", "retry", "s", "r"],
                     choices_display="setup/retry",
-                    # ModelUnavailableError (plan/kill-switch gate) and
-                    # EndpointConfigurationError (wrong base URL / route) are both
-                    # deterministic for the identical request — retrying re-sends a
-                    # doomed call — so steer the default to "setup" (switch model /
-                    # provider / fix endpoint). Other ConnectionErrors (transient)
-                    # keep retry as the default.
-                    default=(
-                        "setup"
-                        if isinstance(exc, (ModelUnavailableError, EndpointConfigurationError))
-                        else ("retry" if isinstance(exc, ConnectionError) else "setup")
-                    ),
+                    default=_default_turn_error_action(exc),
                 )
                 if choice in ("setup", "s"):
                     session = await handle_setup_models(

anton/core/session.py

@@ -366,6 +366,21 @@ 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. 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).
+    """
+    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
@@ -2912,6 +2927,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 —
@@ -3025,16 +3045,35 @@ 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
+                                # token_limit/model_unavailable to actionable cards,
+                                # 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). NOTE:
+                                # cowork-server has no dedicated card for
+                                # EndpointConfigurationError yet (grepped — zero
+                                # hits, friendly_turn_error falls through to the
+                                # generic message for it); re-raising it here still
+                                # stops the misleading "adjust your approach" prose,
+                                # it just doesn't get a *better* card until that
+                                # 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).
+                                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_chat_error_action_default.py

@@ -0,0 +1,37 @@
+"""`_default_turn_error_action` — which action the interactive CLI's
+turn-failure prompt defaults to (review feedback on ENG-1310).
+
+ModelUnavailableError and EndpointConfigurationError are deterministic for
+the identical request, so the CLI already steered them to "setup". A
+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.
+"""
+
+from __future__ import annotations
+
+from anton.chat import _default_turn_error_action
+from anton.core.llm.provider import EndpointConfigurationError, ModelUnavailableError, 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"
+
+
+def test_generic_connection_error_still_defaults_to_retry():
+    assert _default_turn_error_action(ConnectionError("temporarily unavailable")) == "retry"
+
+
+def test_model_unavailable_error_defaults_to_setup():
+    assert _default_turn_error_action(ModelUnavailableError("blocked", code="model_access_denied", model="sonnet")) == "setup"
+
+
+def test_endpoint_configuration_error_defaults_to_setup():
+    assert _default_turn_error_action(EndpointConfigurationError("bad endpoint")) == "setup"
+
+
+def test_non_connection_error_defaults_to_setup():
+    assert _default_turn_error_action(TokenLimitExceeded("out of tokens")) == "setup"

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

tests/test_status_error_mapper.py

@@ -36,6 +36,7 @@
     classify_transient,
     wallet_denial_code,
 )
+from anton.core.session import _is_provider_auth_error
 
 
 def _sdk_error(status_code, json_body=None, text_body=None, headers=None):
@@ -98,6 +99,11 @@ def test_401_maps_to_invalid_key_connection_error():
     # cowork-server's provider_auth detection keys on this exact phrase.
     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).
+    assert _is_provider_auth_error(err.value)
 
 
 def test_401_html_body_maps_to_invalid_key():
@@ -106,6 +112,7 @@ def test_401_html_body_maps_to_invalid_key():
     with pytest.raises(ConnectionError) as err:
         _raise_for_status_error(exc, "sonnet")
     assert "Invalid API key" in str(err.value)
+    assert _is_provider_auth_error(err.value)
 
 
 # ── 429 (quota) ───────────────────────────────────────────────────────
@@ -498,6 +505,20 @@ def test_wallet_denial_code_reads_both_dialects():
 
 # ── the anthropic twin (ENG-1169) ─────────────────────────────────────
 
+def test_anthropic_401_maps_to_invalid_key_connection_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
+    # untested except via a hand-built ConnectionError.
+    exc = _anthropic_sdk_error(401, json_body={"type": "error", "error": {
+        "type": "authentication_error",
+        "message": "invalid x-api-key",
+    }})
+    with pytest.raises(ConnectionError) as err:
+        _raise_anthropic(exc, model="claude-sonnet")
+    assert "Invalid API key" in str(err.value)
+    assert _is_provider_auth_error(err.value)
+
 def _anthropic_sdk_error(status_code, json_body=None, headers=None):
     """Real `anthropic.APIStatusError` from the pinned SDK — the anthropic
     twin of `_sdk_error`. The anthropic SDK does NOT unwrap the error