Provider credential fixes and shared custom-provider registration

[melissa-barca]

This PR fills some gaps in the ai-lib's authentication process to bring it to parity with Positron's Authentication extension and adds shared custom-provider registration for Positron's headless service.

This carries the ai-lib changes from #114 that don't depend on the auth id rename.

• Two windows refreshing the same OAuth credential no longer overwrite each other's tokens. A refresh commits only while the stored record still holds the generation it read. The device-code poll also logs the error code an attempt ends with.
• If GEMINI_API_KEY is unset, the Gemini provider reads GOOGLE_API_KEY.
• Google Vertex accepts a service account from GOOGLE_CLIENT_EMAIL and GOOGLE_PRIVATE_KEY, and tries it before Application Default Credentials. If Google rejects the credentials, the provider reports an auth error that names the two variables.
• On Posit Workbench, an admin-provisioned Databricks profile takes precedence over a DATABRICKS_TOKEN in the user's shell.
• The bridge registers providers.custom entries by kind, with shared helpers for a custom entry's auth mapping and for writing Google Cloud and AWS session tokens, for Positron's headless language model service and Assistant to share.
• When AWS_WEB_IDENTITY_TOKEN_FILE is set, the AWS credential provider points the STS token exchange at the configured region.
• The catalog loader accepts defaults from the host, and ai-config can normalize a Microsoft Foundry base URL, both for Assistant's upcoming provider path.

Cherry-picked from #114. Where they conflicted, I kept main's OpenCode entries and #119's "Reload model list" wording.

[wch]

Looks good overall! Here's some stuff turned up by /assistant-review:

• critical (correctness) — packages/ai-lib/packages/ai-credentials/src/store-backend/StoreBackend.ts:605-609: Two windows can still overwrite each other's refreshed OAuth tokens. The new generation check reads the record and then calls store.set separately; withLock is now explicitly allowed to lock only within one window. Both windows can read generation g, receive different refresh outcomes, and both pass the check before either writes. A late write can replace a valid rotated token with an obsolete token or a refresh_failed tombstone. The same read-then-write race applies to interactive compareAndWrite at lines 582–588. The updated memory-bank/aiCredentialStore.md:89-94 claims the weaker backing is safe, but a generation field alone is not atomic compare-and-set. Require exclusion across every writer of these keys, or supply a backing with a genuinely atomic conditional write before advertising multi-window safety; add a test using two independently locked backings over the same record.

• critical (correctness) — packages/ai-lib/packages/ai-credentials/src/store-backend/envCredentialResolver.ts:38-43: Workbench's admin-provisioned Databricks profile does not outrank a shell token on hosts that capture secrets at startup. The new guard checks DATABRICKS_CONFIG_FILE, but providerEnvRegistry.ts:88-95 declares only the token and M2M variables. captureProviderEnvironment therefore retains DATABRICKS_TOKEN without the config-file marker, and NodeAuthService resolves against that captured snapshot (packages/node/src/platform/services/NodeAuthService.ts:284-296); the shell PAT is selected. Capture the config-file marker without scrubbing it, and test capture → backend resolution with both values present. This is a credential-precedence regression for the exact managed-host case the change targets.

• critical (correctness) — packages/ai-lib/packages/ai-provider-bridge/src/providers/google-vertex-provider.ts:480-488: Inline Vertex service-account credentials can list models but cannot authenticate chat. Discovery mints a token from the captured GOOGLE_CLIENT_EMAIL and GOOGLE_PRIVATE_KEY at lines 87–113. The client factory forwards only a brokered access token or ADC filename; GoogleVertexClient.googleAuthOptions() uses only those two inputs (model-clients/GoogleVertexClient.ts:71-80). On a host that scrubs the private key, a user with only the inline service account sees models but the SDK has no credential for either Gemini or Anthropic requests. Feed the same captured service-account material into the SDK's renewable auth path, and test discovery followed by chat without ADC.

• important (correctness) — packages/ai-lib/packages/ai-provider-bridge/src/providers/google-vertex-provider.ts:105-110: A temporary token-service failure is reported as rejected service-account credentials. Every error from getAccessToken() is converted to InlineServiceAccountError, and lines 401–418 turn that name into inline_service_account_rejected with instructions to replace or unset the variables. A timeout or connection reset therefore blames valid keys and clears cached models as an auth failure. Distinguish actual credential rejection from network/service errors and retain the existing network-error/stale-cache handling for the latter; cover a transient exchange failure.

• important (simplification) — packages/ai-lib/packages/ai-provider-bridge/src/register-all-providers.ts:155-182: The new complete custom-kind registrar table duplicates the existing table in packages/node/src/platform/providers/register-all-providers.ts:65-95. Node still calls its own registerCustomProviders, and Positron's catalog adapter calls that Node helper too (packages/positron/src/positron-ai/catalog-adapter.ts:468-479). Adding a kind or changing its callback/credential environment now requires synchronized edits to both tables, contrary to the stated goal of shared registration. Have the Node adapter pass its catalog-derived customProviders and prebuilt callbacks to the bridge's registerAllProviders, and have the Positron adapter use that same bridge entry; delete the Node registrar table/helper when consumers are updated. The bridge then owns one provider-kind mapping, while the adapters retain only host-specific selection and callback construction.

• minor (correctness) — packages/ai-lib/packages/ai-config/src/base-url.ts:160-171: The Foundry URL normalizer strips queries but not fragments. For https://r.openai.azure.com/openai/v1#section, the # prevents the existing /openai/v1 recognition and the result becomes https://r.openai.azure.com/openai/v1#section/openai/v1, not a usable API root. Strip URL fragments along with queries, and add a pasted-URL case. This helper is exported for a future host caller, not yet called by production code in this checkout.

• minor (convention) — packages/ai-lib/packages/ai-provider-bridge/src/register-all-providers.ts:95: customProviders[].clientKind accepts arbitrary string despite the exhaustive SupportedCustomClientKind table, forcing a Partial<Record<string, ...>> cast at lines 204–207. Narrow the public field to SupportedCustomClientKind so typed callers cannot accidentally request an unsupported kind; keep a runtime guard if untyped inputs can reach this API.

• minor (clarity) — packages/ai-lib/packages/ai-config/src/node/load-catalog.ts:35-39: The new host-default source and inline Vertex credential path are absent from their architectural guidance. packages/ai-lib/memory-bank/aiConfig.md:158-168 still enumerates the old loader sources, while packages/ai-lib/memory-bank/providerGuide.md:261-290 describes Vertex as ADC-only. Update those sections with the host-vs-env default ordering and inline service-account behavior so subsequent integrations do not rebuild the old assumptions.

[melissa-barca]

Thanks @wch I addressed your feedback, ran /assistant-review, fixed a few more findings, and performed my smoke tests. I think this is ready to merge.

• OAuth refresh race: I corrected the StoreBackend comment and aiCredentialStore.md to say it narrows the race under a per-window lock rather than closing it. I also added tests for the guard itself: a record another writer renewed is neither overwritten nor tombstoned. Nothing uses SecretStorage-backed OAuth until Assistant owns Posit AI Pass on Positron, so I'd like to handle cross-window exclusion as part of that change.
• DATABRICKS_CONFIG_FILE is now captured, without scrubbing, through
• Transient token failures: 429, 5xx, request timeouts and connection errors now take the network-error path and keep the stale cache. Other 4xx responses and key-parsing failures still report rejected credentials.
• Duplicate registrar table: posit-dev/assistant#2578 deletes Assistant's copy and registers custom entries through its table. The per-kind wiring test that lived in Assistant is now in ai-lib.
• Foundry fragments: stripped, with a test.
• clientKind: typed as SupportedCustomClientKind, with a new isSupportedCustomClientKind in ai-config.
• Docs: aiConfig.md, providerGuide.md, credentialResolver.md and architecture.md are updated.

[wch]

I ran another review on it, and the first and third ones definitely seem worth fixing. Since I had the review and checkout right here in front of me, I went ahead and implemented and pushed fixes, and after fixing those two issues, I kept going and addressed the rest of them as well -- I hope that's OK!

Details

• important (correctness) — packages/ai-credentials/src/store-backend/envCredentialResolver.ts:42-48, packages/ai-credentials/src/store-backend/StoreBackend.ts:271-290: On Workbench, Databricks M2M variables in the user's shell still override the admin-managed profile. The new check lives in the generic resolveCredentialsFromEnv, and that function only handles DATABRICKS_TOKEN for Databricks. The Databricks branch of StoreBackend.environmentResolution reads DATABRICKS_CLIENT_ID, DATABRICKS_CLIENT_SECRET and DATABRICKS_HOST directly and never consults the Workbench marker:

  Workbench, shell has DATABRICKS_TOKEN                -> resolveCredentialsFromEnv -> null -> "none"   (profile wins)
  Workbench, shell has CLIENT_ID + SECRET + HOST        -> environmentResolution builds an oauth-m2m source -> sourceContext returns it -> used
  Workbench, TOKEN + DATABRICKS_AUTH_TYPE=oauth-m2m     -> same M2M path -> used
  

  The code comment ("the admin credential must not be overridable from the shell") and memory-bank/credentialResolver.md:150-151 ("the environment resolves nothing") both describe the stricter behavior, not the one that ships. If the intent is really just DATABRICKS_TOKEN, as the PR description says, narrow the comment and the doc. Otherwise, move the check to the top of the providerId === "databricks" branch in environmentResolution, which already owns Databricks env precedence, and return { kind: "none" } there. That covers PAT and M2M in one place and keeps a provider-specific rule out of the generic resolver. hasEnvCredentials is exported and returns whatever resolveCredentialsFromEnv does, so keep the PAT-level check there too if hosts use it for status (or have both call one isWorkbenchManagedDatabricks(env) predicate). Add a test with M2M variables alongside the Workbench marker.

• important (tests) — packages/ai-credentials/src/store-backend/StoreBackend.ts:602-637: The headline fix, "two windows refreshing the same OAuth credential no longer overwrite each other's tokens", has no test. No test file under ai-credentials exercises withRefreshTransaction or the generation comparison in persistRefreshedTokens/persistRefreshError. The only reference is a mock hook in acquisition.test.ts:727, which bypasses the StoreBackend implementation entirely. Removing either current.generation !== refreshGenerations.get(providerId) line would pass CI. Add a StoreBackend test that uses in-memory storage whose withLock does not exclude a second writer. Start a refresh, hold the token-endpoint response, write a new authenticated record (standing in for the other window), then release the response. Assert that the other window's record survives. Add the terminal-error variant too: invalid_grant after the other window rotated must not tombstone the fresh record. That is the case that previously logged users out.

• important (simplification) — packages/ai-provider-bridge/src/providers/google-vertex-provider.ts:128-156, 433-435, 516, packages/ai-provider-bridge/src/model-clients/GoogleVertexClient.ts:74-87: The Vertex credential precedence (brokered token, then inline service account, then ADC/keyfile) is now written out separately in three places:

  1. getAccessToken / resolveGoogleVertexAccessToken, for model discovery.
  2. GoogleVertexClient.googleAuthOptions, for chat, fed by inlineServiceAccount(...) and googleApplicationCredentials in the client factory.
  3. The isInlineAuth/isBrokeredAuth ternary that picks the error message and code.

  Each place re-derives the source from credentials.accessToken and the environment. That drift has already happened twice inside this PR: c834de7 ("Authenticate Vertex chat with the inline service account, not only discovery") and caa84f0 ("Choose the Vertex auth-error guidance from the credential source in use") fix spots that fell out of sync. A single resolveVertexCredentialSource(accessToken, env) returning { kind: "brokered", token } | { kind: "inline", serviceAccount } | { kind: "adc", keyFilename? } would give one source of truth. The token resolver, the client's googleAuthOptions and the auth-message selection would all switch on it, and the repeated readSdkCredentialEnvironment(credentialEnvironment ?? process.env) calls would go away. Behavior stays the same.

• minor (correctness) — packages/ai-credentials/src/store-backend/StoreBackend.ts:624-637, packages/ai-credentials/src/acquisition.ts:449: The generation is read at a different moment from the tokens the refresh actually uses, and it is passed through a side-channel Map. withRefreshTransaction reads the record and notes its generation. The operation then calls readTokens separately. Under a per-window lock, another window can write between those two reads:

  note generation g1 -> other window writes g2 -> readTokens returns g2's refresh token -> refresh (server rotates g2's token)
  -> persist sees g2 != g1 -> rotated tokens dropped -> stored g2 refresh token is now dead -> next refresh: invalid_grant -> tombstone
  

  The window is narrow, but the result is a self-inflicted sign-out. The refreshGenerations map also means persistRefreshedTokens/persistRefreshError behave correctly only when called inside withRefreshTransaction. Outside it, the lookup is undefined and every generation-bearing record is silently skipped. Take the generation from the same read that supplies the tokens (have the transaction pass that snapshot, generation included, to the operation) so the check and the tokens always agree and the coupling becomes explicit. Separately, when the terminal branch's persistRefreshError is skipped, acquisition.ts:474 still logs "stored tokens removed", which is now sometimes false.

• minor (correctness) — packages/ai-provider-bridge/src/register-all-providers.ts:208-216: One unsupported custom kind throws after all built-ins and every earlier custom entry are already registered. Later valid entries are then skipped, and the caller's whole registration call fails. The guard exists for untyped callers such as kinds read over IPC, which is exactly where one stale entry would take down every custom provider. Either validate every entry before registering any, or log a warning and skip the bad one. Update the "rejects a custom entry" test to match.

• minor (convention) — packages/ai-provider-bridge/src/providers/google-vertex-provider.ts:86-97: isTransientTokenError uses three as casts to probe response.status, name and code. Narrow with "response" in error / "code" in error plus typeof checks instead (or reuse gaxios's GaxiosError type guard if the bridge already depends on it), in line with the repo's no-casts rule.

• minor (correctness) — packages/ai-config/src/base-url.ts:157-173, packages/ai-config/src/__tests__/base-url.test.ts:61: A pasted https://r.openai.azure.com/openai becomes https://r.openai.azure.com/openai/openai/v1, because only /openai/deployments/... and /openai/v1... are stripped. The test also pins .../openai/v10 → .../openai/v10/openai/v1, which locks in a nonsensical output as if it were a contract. Consider also stripping a trailing bare /openai segment, and replacing the v10 case with one that asserts behavior you actually want to promise (for example, that a non-v1 path is not truncated).

[melissa-barca]

More thank ok, thank you Winston!