Skip to content

refactor(coderd/x/chatd): drop the remaining direct provider clients - #30929

Draft
ibetitsmike wants to merge 2 commits into
mike/chatd-gateway-only-providersfrom
mike/chatd-gateway-only-provider-clients
Draft

ibetitsmike wants to merge 2 commits into
mike/chatd-gateway-only-providersfrom
mike/chatd-gateway-only-provider-clients

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

After #30928, chatprovider.ModelFromConfig still built direct Azure, Google, OpenRouter, and Vercel clients. The AI Gateway path never asks for them, because fantasyConfigForAIBridge sends those provider types through the OpenAI-compatible client. This PR deletes those four arms and their missing-key error cases, the per-provider region plumbing that only the deleted Bedrock client read, and ProviderAPIKeys.Empty, which has no callers left.

Why it is safe

  • The only production caller of ModelFromConfig passes a hint from fantasyConfigForAIBridge (see refactor(coderd/x/chatd/chatprovider): drop the direct Bedrock client #30928), and no test builds these four clients.
  • Region flowed from ConfiguredProvider.Region through ResolveUserProviderKeys into ProviderAPIKeys.RegionByProvider, and only the Bedrock arm read it. The gateway reads Bedrock and Claude Platform regions from the provider settings itself (aibridgedserver.go).
  • chatd.go still marks Claude Platform on AWS as supporting ambient credentials. TestResolveQuickgenModel_TitleGenerationOverrideAmbientCredentials fails when that flag is forced to false.
  • Empty lost its last production caller when direct chat routing was removed (refactor: deprecate AIGatewayRoutingEnabled, remove direct chat routing #26862). Its only test checked the region term.
  • Deleted tests (TestProviderAPIKeysEmpty_RegionCountsAsConfigured, TestResolveUserProviderKeys_PreservesProviderRegions, the DisabledProviderRegionsRemoved prune row) checked region and Empty code that is gone. One also checked that a keyless Bedrock provider is available; TestResolveUserProviderKeys still fails when the Bedrock ambient case or the empty-key rule in setResolvedProviderAPIKey is mutated.
  • rg -n 'RegionByProvider|setRegion|keys\.Region\(|\.Empty\(\)' coderd/x/chatd finds nothing.

Review focus

The aiProviderConfigFromKeys hunk in chatd.go, the only change outside chatprovider: it drops the region reads and keeps the ambient credential flag.

Validation

  • go test ./coderd/x/chatd/chatprovider/... ./coderd/x/chatd/chatopenai/...: pass.
  • go test ./coderd/x/chatd/ -count=1 -p 4 -parallel 4: pass.
  • go build ./... and go vet on every package that imports chatprovider: pass.
  • make pre-commit: pass.

Stack

This stack removes provider code that only direct, non-gateway clients used, so chatprovider builds just the three clients the AI Gateway path needs, then derives the BYOK auth header from that same client choice.

  1. refactor(coderd/x/chatd/chatprovider): drop the direct Bedrock client #30928 Drop the direct Bedrock client
  2. refactor(coderd/x/chatd): drop the remaining direct provider clients #30929 Drop the remaining direct provider clients
  3. refactor(coderd/x/chatd/chatprovider): drop provider options only direct clients read #30958 Drop provider options only direct clients read
  4. refactor(coderd/x/chatd): drop transport branches only direct clients used #30966 Drop transport branches only direct clients used
  5. test(coderd/x/chatd): cover BYOK header choice for Bedrock and OpenAI-compatible routes #30978 Cover the BYOK header choice for Bedrock and OpenAI-compatible routes
  6. Share the AI Gateway client choice with BYOK auth (next)

Xum, an AI agent, opened this PR for @ibetitsmike and wrote this description.

ModelFromConfig still built direct Azure, Google, OpenRouter, and
Vercel clients, although the AI Gateway path only asks for the
Anthropic, OpenAI, and OpenAI-compatible clients. Delete those arms
and their missing-key error cases, the per-provider region plumbing
that only the direct Bedrock client read, and ProviderAPIKeys.Empty,
which has no callers left.
@ibetitsmike

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T15:38:59.777717Z 5cf6b25 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 5cf6b255bf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant