fix(auth): refresh provider credentials per request (ENG-2116) - #429
Conversation
Refs: ENG-2116
…y seam AsyncAzureOpenAI._prepare_options never chains to super(), so a callable api_key is never awaited and the client sends an empty api-key header on every request. Refuse the combination at construction instead of failing on the first call. Add the test for the composition the fix actually depends on: a 401 on the stale token, the credential rotating mid-flight, and a 200 on the retry, asserting the header sequence rather than just the recovery. Both halves were tested in isolation and neither could catch the two drifting apart. Also cover the structured-output role stamps, and record why a confirmed verifier 401 propagates where 402 and 403 on the same call latch quietly.
There was a problem hiding this comment.
🟡 Changes recommended
OpenAIProvider now supports api_key_provider, but it doesn’t validate that a construction-time api_key is present for export_connection_info()/scratchpad subprocess configuration, which can silently configure the scratchpad with an empty key.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses stale provider credentials in long-running turns by introducing per-request credential refresh for OpenAI-compatible providers and adding a typed, bounded confirmation flow for provider-auth (HTTP 401) refusals, so a single isolated refusal can recover without discarding a turn.
Changes:
- Introduces
ProviderAuthErrorand updates OpenAI/Anthropic status mappers to raise it for HTTP 401, enabling canonical downstream handling. - Adds a one-time confirmation retry for
ProviderAuthErrorat theLLMClientboundary (including streaming, without replay after output) and preserves role attribution on terminal refusals. - Bumps the OpenAI Python SDK dependency to
openai>=2.21.0and adds targeted tests covering dynamic API key refresh + confirmation, router behavior, verifier propagation, and session re-raise semantics.
File summaries
| File | Description |
|---|---|
pyproject.toml |
Bumps OpenAI SDK minimum version to support async API-key suppliers. |
uv.lock |
Updates locked dependency spec for OpenAI SDK. |
anton/core/llm/provider.py |
Adds ProviderAuthError type (401 credential refusal) with optional role attribute. |
anton/core/llm/openai.py |
Adds async api_key_provider support for per-request key refresh; maps 401 → ProviderAuthError; refuses Azure supplier case. |
anton/core/llm/anthropic.py |
Maps 401 → ProviderAuthError for consistent typed auth handling. |
anton/core/llm/client.py |
Implements bounded auth confirmation (one retry) for both non-streaming and streaming calls; stamps terminal refusals with the failing role. |
anton/core/session.py |
Switches provider-auth detection to canonical type; ensures confirmed verifier 401 propagates (not swallowed by broader handlers). |
anton/cli.py |
Updates runtime dependency check to require openai>=2.21.0. |
tests/test_openai_dynamic_api_key.py |
Adds regression tests for dynamic key refresh + retry confirmation, and Azure supplier refusal. |
tests/test_client.py |
Adds extensive tests for confirmation, role stamping, and streaming replay behavior. |
tests/test_status_error_mapper.py |
Updates mapper tests to assert canonical ProviderAuthError behavior for 401s. |
tests/test_session_auth_error_reraise.py |
Updates session tests to assert typed provider-auth propagation and no extra retries. |
tests/test_chat_error_action_default.py |
Pins default action differences between typed auth refusal vs plain ConnectionError text. |
tests/test_thalamus.py |
Adds test ensuring router auth refusal falls back to planning after confirmation fails. |
tests/test_verifier_truncation.py |
Adds test ensuring confirmed verifier auth refusal propagates as terminal. |
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… kwarg AsyncAzureOpenAI never awaits a callable api_key: the base __init__ has already replaced it with "", so the client sends an empty api-key header on every request and 401s forever. The guard caught that only when the value arrived through api_key_provider. `api_key` is annotated str but nothing enforces it at runtime, and a supplier passed through it reaches the same SDK path with the same result. Key the refusal on the resolved value instead. Refs: ENG-2116
alecantu7
left a comment
There was a problem hiding this comment.
Approving
I reviewed and approved the staging counterpart (anton#421) across three rounds. This carries that content, and I re-verified against this head rather than relying on the earlier one — the branch moved twice while I was looking at it.
Delta from the approved staging content: comments only, plus one broadened guard.
b4471472 was comment/docstring corrections. I checked the new wording is true of the code rather than aspirational: it says a refusal is terminal after a failed confirmation or immediately once a stream has emitted, and client.py:69 is if yielded or not confirmation.take(): raise. Accurate.
303bf7c6 broadens the Azure refusal from api_key_provider is not None to callable(client_api_key) — so a callable arriving through the api_key parameter is caught too, which matters because that parameter is annotated str and nothing enforces it at runtime. Mutation-verified: reverting to the narrow guard reddens test_azure_refuses_a_callable_passed_as_the_static_api_key. 4 passed at head.
Everything from the earlier rounds still stands — the wire-level recovery (['Bearer token-a', 'Bearer token-b']), the guard armed by two dying tests, and the classifier differential showing 402/403/429/5xx byte-identical to the base.
The pin sequence is a precondition on merge, not on this approval. cowork-server#444 pins anton to a rev on an unmerged branch, and its own comment says so. Order: merge anton#429 → re-pin #444 to the resulting anton main commit (or branch = "main") → regenerate the lockfile → merge #444 → merge cowork#806. The image installs via uv sync --frozen, so a rev on an unmerged branch reaching a cowork-server main build is a broken image, not a stale one.\n\nBoth tracks are still open. anton#421 / cowork-server#428 / cowork#778 target staging and are also approved. Whichever ships, the other wants closing or rebasing deliberately — main → staging is a manual merge here.
Approved on Alejandro's call. Merging is his, and this one is first.
User story
As a signed-in Cowork user
I want an active turn to adopt refreshed MindsHub credentials and recover from one isolated authentication refusal
So that long-running work is not discarded or blamed on my account
Hotfix scope
This is #421 replayed onto
mainso the fix can ship without waiting for the 180-commitstagingbacklog. The three commits are byte-identical to the ones onfix/eng-2116-refresh-active-jwt, confirmed withgit range-diff; nothing fromstagingcame along. #421 stays open againststaging, and the backmerge resolves it.One commit here is not yet on #421:
fix(auth): clarify terminal provider refusal semantics, which was still local when this branch was cut.Why this matters
A long-running turn can retain a ten-minute MindsHub JWT after the desktop refreshes it. The next model call then fails as if the user's session were invalid, discarding substantial work and suggesting a reconnect that cannot repair the stale in-memory credential. Seven observed failures discarded about 1.47 million tokens, including work from a paid external user.
Acceptance criteria
provider_auth; optional probes remain fail-open after confirmation.provider_auth.How to test
ConnectionError, auth-looking text, a bare 401, and non-auth gateway statuses and verify their existing mappings remain intact.Notes for the reviewer
Provider instances stay alive while credentials rotate. A runtime supplier feeds the OpenAI SDK before each main-process request. Static provider settings remain static, and exported scratchpad subprocess configuration keeps its construction-time credential because subprocess hot-swap needs a separate IPC contract.
Confirmation is typed and bounded. Only
ProviderAuthErroris retried, only once, and streaming retries only before the first event. Required planning, coding, and verifier calls propagate a confirmed refusal; optional router, history-summary, and background-memory probes retain their fail-open contract.Azure refuses an
api_key_providerat construction.AsyncAzureOpenAI._prepare_optionsnever chains to super, so the supplier is never awaited and the client would send an emptyapi-keyon every request.The terminal error carries the failing role. cowork-server uses that role to preserve the correct Reconnect or update-key action in mixed-provider configurations.
Merge this first. cowork-server's hotfix pins this branch head and has to be re-pinned to the resulting
maincommit before it merges.Verified locally, against
mainRun in a worktree cut from
origin/main(b94ee7e1) with these three commits replayed on top.BRAVE_API_KEY/EXA_API_KEY/MINDS_API_KEY/OPENAI_API_KEY/ANTHROPIC_API_KEY/MINDSHUB_API_KEY. None import-error skips, none in the touched pathProviderAuthErroris introduced by the diff itself;EndpointConfigurationError, used by the new Azure refusal, was already imported inopenai.pyon maingit range-diffagainst #421303bf7c6)api_key, not only one passed asapi_key_provider, with a testShips with
Merge order: this PR first, then cowork-server, then cowork desktop.