-
Notifications
You must be signed in to change notification settings - Fork 116
[ENG-1310] Unable to authenticate against the llm model invalid api key #327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
tino097
merged 7 commits into
staging
from
tino097/eng-1310-unable-to-authenticate-against-the-llm-model-invalid-api-key
Aug 11, 2026
Merged
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8ae7f92
reraise persistent provider-auth failures instead of flattening them …
tino097 a467daa
Add test cases
tino097 aa8dc8d
fail invalid-API-key errors immediately instead of burning retries first
tino097 2da0427
improve the check from review
tino097 849e79a
address code-review feedback on the ENG-1310 fix
tino097 7920b24
Merge remote-tracking branch 'origin/staging' into tino097/eng-1310-u…
tino097 5290783
close the two remaining review items on ENG-1310
tino097 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| """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.session import ChatSession, ChatSessionConfig | ||
|
|
||
| _AUTH_ERROR_MESSAGE = "Invalid API key — check your OpenAI API key configuration." | ||
|
tino097 marked this conversation as resolved.
|
||
|
|
||
|
|
||
| @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 | ||
|
|
||
|
|
||
| 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 | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.