fix(auth): refresh active-turn credentials per request (ENG-2116) - #444
Conversation
Refs: ENG-2116
remote_turn_error keyed only on the typed ProviderAuthError name, but the worker pods run the minds-anton-scratchpad image, pinned in scratchpad-controller and bumped independently of this server's vendored anton. Neither pinned commit carries the type, so every hosted 401 downgraded to the generic code and lost the Reconnect card. Match the legacy copy too, anchored with startswith so a tool 401 cannot select it. Make both anton imports lazy with fallbacks, matching the four siblings in turn_errors.py: a module-scope import turned a version skew into a failed app import rather than one missing error card. Splat api_key_provider like effort_kw for the same reason. Only name a provider on an auth error when the failing role is known, or when the required roles resolve to the same provider anyway. An unstamped refusal previously defaulted to planning, which offers a MindsHub Reconnect card for what may have been a failing BYOK key. Drop the getattr defaults on resolved providers and complete the test fakes instead; the default would turn a rename into a silent False, and False is what disables the desktop wake barrier. Correct the pin comment: this rev is anton#421's head, not a staging commit.
The staging PR pinned anton#421's head, which is cut from Anton `staging` and carries 180 commits this branch does not want. Repoint at anton#429, the same change replayed onto Anton `main`. Anton `main` now defines resolve_artifact_id, canonical_artifact_id and artifact_key, so the pin no longer exists to hold the tree on a staging commit. Rewrite the comment to say that, and to name the re-pin this rev still needs once anton#429 lands. Refs: ENG-2116
PR environment is up ·
|
| Console | https://pr-cowork-server-444.dev.mindshub.ai |
| Auth API | https://auth-pr-cowork-server-444.dev.mindshub.ai |
| Cowork Web | https://cowork-pr-cowork-server-444.dev.mindshub.ai |
| Keycloak Admin | https://auth-pr-cowork-server-444.dev.mindshub.ai/auth/admin/ |
| Inference API | https://api-pr-cowork-server-444.dev.mindshub.ai/v1 |
| Cowork Server API | https://cowork-pr-cowork-server-444.dev.mindshub.ai/api |
| Health | https://cowork-pr-cowork-server-444.dev.mindshub.ai/api/v1/health/ |
| Namespace | pr-cowork-server-444 |
Every service is in this namespace. The ones this PR does not build run the staging image unless you link them with Deploys: in the PR body.
Updated on every push to this PR.
There was a problem hiding this comment.
🟡 Changes recommended
It still locks/pins anton-agent to an unmerged branch head (non-reproducible main build) and the lockfile now makes portalocker depend on Windows-only pywin32 unconditionally, which can break non-Windows installs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates Cowork’s Anton-backed provider wiring so active turns can pick up refreshed MindsHub credentials per request, and so confirmed provider-auth failures are classified/messaged accurately (including by role) without regressing other gateway error mappings.
Changes:
- Add a per-request runtime credential supplier for local MindsHub-backed providers, capability-gated for older Anton versions.
- Switch auth failure detection/mapping to Anton’s typed
ProviderAuthError, while keeping a narrow legacy fallback for remote worker pods. - Extend health reporting and tests to reflect when the desktop runtime Minds credential is required.
File summaries
| File | Description |
|---|---|
cowork/services/providers.py |
Adds runtime credential supplier and capability-gated api_key_provider wiring for MindsHub providers. |
cowork/handlers/turn_errors.py |
Uses typed ProviderAuthError (plus legacy anchored fallback) for auth detection and remote error mapping. |
cowork/handlers/responses.py |
Attributes auth failures to the actual failing role/provider when possible; avoids incorrect defaults in mixed configs. |
cowork/api/v1/endpoints/health.py |
Adds minds_runtime_credential_required flag derived from resolved required roles (planning/coding) and org mode. |
tests/test_build_llm_client.py |
Tests runtime supplier behavior, org/static behavior boundaries, and backward-compat degradation paths. |
tests/test_turn_errors.py |
Tests typed auth detection, legacy fallback behaviors, and correct reconnectability/provider labeling by role. |
tests/test_health_owner.py |
Tests health flag behavior for runtime credential requirement across role/provider combinations and org mode. |
pyproject.toml |
Pins anton-agent to a specific git rev for ENG-2116 capabilities and documents merge-order constraint. |
uv.lock |
Updates locked Anton rev/deps and reflects resolver output (includes a platform-marker regression to fix). |
Review details
- Files reviewed: 8/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
alecantu7
left a comment
There was a problem hiding this comment.
Review — 202fd3b4 (hotfix to main)
I reviewed and approved the staging counterpart (#428 @ a3beb2f3) over four rounds. This carries that content unchanged, so rather than re-derive it I verified equivalence and looked only at what differs.
Verified identical to the approved version
Diffing the ENG-2116 files between a3beb2f3 and this head: responses.py, providers.py, health.py and test_build_llm_client.py are byte-identical. Both findings I raised survive:
- the hosted-401 mapping at
:950-954— typed branch plus thestartswith(LEGACY_AUTH_ERROR_MESSAGE_PREFIX)fallback, which is what keeps the pinned scratchpad pods'ConnectionErrormapping toprovider_auth; - the capability gate at
:1164/:1208—supports_api_key_providerfrominspect.signature, with the warn-once degradation.
The only delta in turn_errors.py is 24 deletions of the WORKER_UNRESPONSIVE block. That is not a removal by this PR: grep on origin/main returns 0 hits and origin/staging returns 2, so it is ENG-2126 work that exists on staging and not on main. Correct for a main-based branch, and worth knowing that it means production keeps conflating "the worker never answered" with "anton raised something we don't recognise" until #427 reaches main by its own route.
The one blocking step
pyproject.toml:102 pins anton-agent to rev = "b4471472…" — anton#429's head, which is not an ancestor of anton main (verified). The comment says exactly this and instructs the re-pin. Restating because it is now a production build: merge anton#429 first, re-pin to the resulting anton main commit or branch = "main", regenerate the lockfile, then merge this. 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.
Two things that apply to the whole hotfix set
Six open PRs now exist for ENG-2116 — anton#421 / cowork-server#428 / cowork#778 against staging (all three approved), and anton#429 / cowork-server#444 / this set against main. Both tracks carry the same change. Whichever ships, the other wants closing or rebasing deliberately, and per the repo convention main → staging gets merged back by hand — nothing does it automatically, so staging will otherwise sit behind production carrying an approved duplicate of what already shipped.
main is production. These merge straight to it, so the usual staging soak does not happen. That raises the cost of the pin step below from "housekeeping" to "the thing that breaks the build".
Verdict: COMMENT. No new findings — the content is what I already approved. Not an approval; the pin step is a real precondition and it is Alejandro's call whether it is satisfied.
The README's credential section presented the settings overlay as the whole mechanism. It now names the per-request re-read, the two static cases that keep their construction-time value, and the scratchpad subprocess that also does. That last one is the limit a reader goes to the README to find, and it lived only in a code comment. remote_turn_error anchors the legacy invalid-key match with startswith so a tool's own 401 cannot borrow the Reconnect card, but no test fed a message that contains the phrase without starting with it. The new one fails against the substring form. Record why the router arm of the auth provider label cannot be reached today: every router call site swallows a confirmed refusal and falls back to planning. Refs: ENG-2116
anton#429 gained a commit in review: the Azure endpoint now refuses any callable api_key, not only one arriving through api_key_provider. Follow its head so the pin keeps naming the PR head its own comment describes, and so the tree this installs is the tree anton main becomes. Refs: ENG-2116
alecantu7
left a comment
There was a problem hiding this comment.
Approving
Head unchanged from the one I reviewed (202fd3b4), all checks complete with zero failures.
This carries the content I approved on cowork-server#428 across four rounds. Verified by diffing the ENG-2116 files rather than re-deriving: responses.py, providers.py, health.py and test_build_llm_client.py are byte-identical. Both findings I raised survive — the hosted-401 mapping at :950-954 (typed branch plus the startswith legacy fallback that keeps the pinned scratchpad pods mapping to provider_auth), and the capability gate at :1164/:1208 with its warn-once degradation.
The only turn_errors.py delta is 24 deletions of the WORKER_UNRESPONSIVE block, which is not a removal by this PR: grep returns 0 hits on origin/main and 2 on origin/staging. It is ENG-2126 work that has not reached main. Correct for a main-based branch — with the consequence that production keeps conflating "the worker never answered" with "anton raised something unrecognised" until #427 arrives by its own route.
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 the step the pin sequence is about.
|
Correction to my approval above. It says "head unchanged from the one I reviewed ( I have now verified the two commits that landed, and both are fine — better than fine, in one case:
That second one resolves the stale-pin half of what I flagged: the pin now tracks its sibling instead of a superseded commit. The other half stands unchanged and is still the merge precondition — The approval stands on |
mindsdb/anton#429 merged as 18b043f6, so Anton main now carries the typed provider-auth error and the per-request API-key supplier alongside the artifact-identity symbols it already had. Neither reason to hold a rev pin survives, so drop back to the branch and let the lockfile record the resolved commit that `uv sync --frozen` installs. Removes the merge-ordering hazard this file has carried since #407: no rev on an unmerged branch can reach a cowork-server main build any more. Refs: ENG-2116
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 #428 replayed onto
mainso the fix can ship without waiting for thestagingbacklog. The three ENG-2116 commits are byte-identical to the ones onfix/eng-2116-refresh-active-jwt, confirmed withgit range-diff; nothing else fromstagingcame along. #428 stays open againststaging, and the backmerge resolves it.One commit is new here, and it is the only difference from #428: the Anton pin.
The Anton pin
anton-agentsourcerev = "095c17ae"— anton#421's head, cut from Antonstagingbranch = "main", lockfile resolving to18b043f6The old pin comment claimed Anton
mainlacksresolve_artifact_id,canonical_artifact_idandartifact_key. That is no longer true: Antonmaindefines all three inanton/core/artifacts/models.pyand exports them. The pin now exists only to reach the ENG-2116 supplier, not to hold the tree on a staging commit. The comment says that.Resolved. anton#429 merged as
18b043f6, so Antonmainnow carries the ENG-2116 supplier alongside the artifact-identity symbols. Both reasons for a rev pin are gone and this tracksbranch = "main"again, which removes the merge-ordering hazard this file has carried since #407.Why this matters
The sidecar previously built long-lived model providers from one startup credential. Even after the desktop refreshed and handed over a MindsHub JWT, an active turn could keep sending the expired value and surface the gateway refusal as a user-session problem.
Acceptance criteria
branch = "main", lockfile at18b043f6.How to test
Notes for the reviewer
The runtime store remains the boundary. The desktop already owns credential rotation and handoff; the sidecar passes an async supplier into local Minds-backed Anton providers so live instances read the store at request time. Static org and user-provided credentials are intentionally unchanged.
The health flag is authoritative for the desktop wake gate. It reflects resolved required planning and coding roles only. Optional router usage cannot delay an otherwise direct-provider turn.
Hosted 401s keep the Reconnect card. The worker pods run the
minds-anton-scratchpadimage, pinned in scratchpad-controller and bumped independently of this PR set, soremote_turn_errormatches the legacy 401 copy as well as the typed name. Both Anton imports are lazy with fallbacks for the same version-skew reason.Verified locally, against
mainRun in a worktree cut from
origin/main(6095d840) with these commits replayed on top.make test)test_build_llm_client.py/test_health_owner.py/test_turn_errors.pyOpenAIProvider(api_key_provider=...)andProviderAuthErrorboth resolve at18b043f6(antonmain), so the capability gates take the live branch instead of silently degradinguv lock --checktests/integrationCOWORK_BASE_URLruntime_credentialand its accessors are already on main, and every file the replayed code depends on is byte-identical betweenmainandstaginggit range-diffagainst #428Ships with
Deploys: #444
Merge order: Anton #429 (merged) → this PR → cowork desktop #806.