Skip to content

refactor: deprecate AIGatewayRoutingEnabled, remove direct chat routing - #26862

Merged
johnstcn merged 23 commits into
mainfrom
cian/codagt-682-remove-aigatewayroutingenabled-flag-and-dead-direct-routing
Jul 1, 2026
Merged

johnstcn merged 23 commits into
mainfrom
cian/codagt-682-remove-aigatewayroutingenabled-flag-and-dead-direct-routing

Conversation

@johnstcn

@johnstcn johnstcn commented Jun 30, 2026 •

Copy link
Copy Markdown
Member

Closes https://linear.app/codercom/issue/CODAGT-682
Closes https://linear.app/codercom/issue/CODAGT-598

This PR removes the now-dead direct-routing code:

  • Deletes the direct routing implementation.
  • Collapses the resolvedModelRoute discriminated union into aiGatewayModelRoute.
  • Removes the dead providerKeys cascade.
  • Deletes the preferredShortTextCandidates quickgen function.
  • Simplifies the advisor override error handling.

It also deprecates the AIGatewayRoutingEnabled deployment option as a no-op instead of hard-deleting it, so deployments that still carry the flag in YAML or CLI args do not break on upgrade.

Once direct routing was gone, the AI Gateway became mandatory for chat, which surfaced gaps in how the product behaves with the gateway disabled:

  • Expose ai-gateway-enabled to the frontend via embedded page metadata (same mechanism as tasks-tab-visible) instead of a field on the models response.
  • Disable the chat composer via the existing AgentSetupNotice when the gateway is disabled, for both new and existing chats.
  • Fix nil/typed-nil chatDaemon panics on startup and shutdown (git sync worker method value, Close(), ContextDirtyMarker interface assignment) when the gateway is disabled.
  • Fix getChat incorrectly 503ing (making existing chats look not-found in the UI) even though it's a pure DB read; stop the chat WebSocket from retrying the still-gated stream endpoint forever when the gateway is disabled.

@linear-code

linear-code Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

CODAGT-682

CODAGT-598

@johnstcn
johnstcn force-pushed the cian/codagt-682-remove-aigatewayroutingenabled-flag-and-dead-direct-routing branch from 952120f to c4efb77 Compare June 30, 2026 12:19
@johnstcn johnstcn changed the title refactor(coderd/x/chatd): deprecate AIGatewayRoutingEnabled, remove direct routing refactor: deprecate AIGatewayRoutingEnabled, remove direct chat routing Jun 30, 2026
@johnstcn
johnstcn force-pushed the cian/codagt-682-remove-aigatewayroutingenabled-flag-and-dead-direct-routing branch from 877f496 to 590bdfe Compare June 30, 2026 21:06
johnstcn added 5 commits June 30, 2026 21:10
…irect routing

Deprecate the AIGatewayRoutingEnabled deployment option as a no-op
instead of hard-deleting it, so deployments with the flag in their YAML
config or CLI flags don't break on upgrade. Delete the direct routing
implementation, collapse the resolvedModelRoute discriminated union
into aiGatewayModelRoute, remove the dead providerKeys cascade, delete
the preferredShortTextCandidates quickgen function, and simplify the
advisor override error handling.

The option will be fully removed in a future release.
…itle path

gofmt advisor_internal_test.go and drop the dead chatprovider import in
title_override_internal_test.go. Remove the providerKeys parameter from
generateManualTitleCandidate, proposeChatTitleWithStore, and
regenerateChatTitleWithStore, plus the upstream resolveUserProviderAPIKeys
calls in RegenerateChatTitle and ProposeChatTitle that fed them. The
manual title path stopped reading keys when title_override.go dropped
its keys parameter.
- coderd: chat daemon only constructs when BridgeConfig.Enabled. Chat
  HTTP handlers return 503 with a clear remediation message via a new
  requireChatDaemon guard.
- codersdk: ChatModelsResponse gains ai_gateway_disabled. listChatModels
  short-circuits with the signal so the UI can render a banner.
- site: AgentsPageView renders a warning banner with copy:
  "AI Gateway must be enabled for Coder Agents functionality." Plumbed
  from AgentsPage via the new SDK field. Storybook AIGatewayDisabled
  story added.
- cli: warnDeprecatedChatAIGatewayRoutingFlag logs once at startup when
  the operator explicitly set the deprecated flag.
- chatd/advisor: restore SuccessReturnsOverrideModelAndConfig and
  AIProviderIDResolvesOverrideProviderKeys to test the actual success
  path by wiring a mock transport factory.
- chatd/quickgen: inline the single-element candidates loop in
  maybeGenerateChatTitle and generateTurnStatusLabel; the multi-candidate
  retry was specific to direct routing.
@johnstcn
johnstcn force-pushed the cian/codagt-682-remove-aigatewayroutingenabled-flag-and-dead-direct-routing branch from 590bdfe to c0cd83f Compare June 30, 2026 21:16
@github-actions

Copy link
Copy Markdown
Contributor

Docs preview

📖 View docs preview for docs/reference/api/chats.md

…re-chatd tests

PR #26658 only migrated full-server tests to AI Gateway routing; this
migrates the bare-chatd tests that build *chatd.Server directly via
chatd.New/newActiveTestServer/newTestServer/newInternalTestServer,
which PR #26862 left without a transport factory once direct routing
was deleted.

- 78 tests wired to an existing mock provider URL via
  chattest.NewMockAIBridgeTransport, mirroring the pattern already
  used in 10 passing tests.
- TestProviderSwitchSanitizesAndRestoresPEToolHistory and
  TestComputerUseSubagentToolsAndModel each drive two providers in
  one chat; added a small providerRoutedTransportFactory that
  dispatches by provider name instead of extending chattest's
  single-target mock.
- 6 TestSpawnAgent_* tests had an unrelated bug: they call
  runSubagentTool directly and never set
  aibridge.WithDelegatedAPIKeyID on ctx, so they failed before
  transport wiring was ever reached. Added the missing ctx line,
  copying the established sibling pattern.
- newInternalTestServer gained one new
  withInternalTestServerTransportFactory option, mirroring its
  existing With* options, for the two internal-package tests that
  needed it.
- TestRegenerateChatTitle_PersistsAndBroadcasts and its idle variant
  exposed a real call-sequence change: PR #26862 deleted the
  pre-lock resolveUserProviderAPIKeys call these tests' gomock
  expectations still modeled, and title-generation routing now goes
  through GetAIProviderByID instead of
  GetAIProviders/GetAIProviderKeysByProviderIDs. Rewrote the mock
  expectations and AIProviderID wiring to match the current code
  path; chatd.go itself is untouched.

No production code changed.
Comment thread coderd/exp_chats.go Outdated
…iring sweep

- Restore the outgoing-request model assertion that was lost when
  TestRegenerateChatTitle_PersistsAndBroadcasts and its idle variant
  were rewritten to use a synthesized RoundTripper response instead
  of a real chattest.NewOpenAI server. The previous rewrite returned
  a canned response without checking what model was actually
  requested.
- Drop the PR-number reference from the explanatory comment in both
  tests; state the current routing invariant instead.
- Use the existing withSubagentDelegatedKey helper consistently at
  the two TestSpawnAgent_ComputerUse* call sites instead of spelling
  out aibridge.WithDelegatedAPIKeyID inline, matching the other four
  call sites added in the same change.
@johnstcn

This comment was marked as resolved.

johnstcn added 2 commits June 30, 2026 23:27
…ta, not chat models response

Replace codersdk.ChatModelsResponse.AIGatewayDisabled with a new
ai-gateway-enabled meta tag on the SPA shell HTML, mirroring the
existing HideAITasks/tasks-tab-visible mechanism exactly. This is a
deployment config boolean gating an AI feature for all users, not
just deployment admins, so /api/v2/deployment/config (which requires
ResourceDeploymentConfig read) isn't a usable home for it; the
embedded-metadata mechanism already serves this exact purpose.

- listChatModels drops its AI-Gateway-disabled special case and
  falls through to its normal catalog logic unconditionally, same
  as before this branch added the special case.
- AgentsPage reads the new metadata key, defaulting to enabled (not
  disabled) when the tag is unavailable in dev/Storybook, so the
  banner doesn't show by default outside production.
- AgentsPageView's own prop contract and its AIGatewayDisabled story
  are unchanged; only the data source moves.
- Removed now-dead ai_gateway_disabled fixture lines forced by the
  type change across 5 story/test files.
…isabled derivation

Compute aiGatewayDisabled directly from the metadata instead of
deriving an intermediate aiGatewayEnabled local and inverting it at
the prop site, per deep-review feedback.
@johnstcn

This comment was marked as resolved.

@johnstcn

Copy link
Copy Markdown
Member Author

/coder-agents-review

@coder-agents-review

coder-agents-review Bot commented Jun 30, 2026 •

Copy link
Copy Markdown
Contributor

Chat: Review posted | View chat
Requested: 2026-06-30 23:43 UTC by @johnstcn
Spend: $7.80 / $100.00

Review history
  • R1 (2026-07-01), 1 P0, 1 P3, COMMENT. Review

deep-review v0.9.0 | Round 1 | 6b8c38b..0f4c702

Last posted: Round 1, 2 findings (1 P0, 1 P3), COMMENT. Review

Finding inventory

Finding inventory

Findings

# Sev Status Location Summary Round Reviewer Posted
CRF-1 P0 Open coderd/coderd.go:2351 Nil pointer dereference on shutdown when AI Gateway disabled: chatDaemon.Close() unconditional, gitSyncWorker captures nil chatDaemon.PublishDiffStatusChange R1 Netero Yes
CRF-2 P3 Open cli/server.go:2937 No test for warnDeprecatedChatAIGatewayRoutingFlag deprecation warning R1 Netero Yes

Round log

Round 1

Netero-only. 1 P0, 1 P3. Reviewed against 6b8c38b..0f4c702.

About deep-review

CRF = Coder Review Finding (P0-P4, Nit, Note)

Reviewer Focus
Bisky tests
Chopper ops/errors
Churn-guard change verification
Ging language modernization
Gon naming
Hisoka edge cases
Killua perf
Kite change integrity
Knov contracts
Knuckle SQL
Komugi flake/determinism
Kurapika security
Law decomposition
Leorio docs
Luffy product
Mafu-san process
Mafuuu contracts
Melody dispatch/pairing
Meruem structural
Nami frontend
Netero mechanical checks
Pariston premise testing
Pen-botter product gaps
Razor verification
Robin duplication
Ryosuke Go arch
Takumi concurrency
Zoro shape

🤖 Managed by Coder Agents.

Comment thread site/src/pages/AgentsPage/AgentsPageView.tsx Outdated

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First-pass review (Netero). The full review panel has not yet reviewed this PR; it will run after these findings are addressed.

The deprecation approach is sound: keeping the option in the OptionSet for YAML upgrade safety while removing the underlying routing path is the right call. The test sweep across 81 bare-chatd tests is thorough, and the embedded-metadata pattern for surfacing gateway state to the frontend mirrors the existing tasks-tab-visible pattern cleanly.

1 P0, 1 P3.

"This crashes the process on shutdown."


coderd/coderd.go:2344-2351

P0 [CRF-1] chatDaemon is now conditionally initialized (line 832, guarded by BridgeConfig.Enabled), but gitSyncWorker at line 869 is created unconditionally within the outer block. When AI Gateway is disabled, chatDaemon is nil, so api.chatDaemon.PublishDiffStatusChange (line 871) captures a nil receiver. When the worker processes a diff status change, calling the stored method value panics.

The same nil receiver causes a second crash here: api.chatDaemon.Close() (line 2351) is called unconditionally during shutdown, and Close() immediately calls p.closeInflightAdmission(), which dereferences the nil receiver.

Before this PR, chatd.New() was always called, so chatDaemon was never nil.

Fix: move the gitSyncWorker creation (lines 862-876) inside the if BridgeConfig.Enabled block, and add if api.chatDaemon != nil guards on api.chatDaemon.Close() (line 2351) and api.gitSyncWorker.Done() (line 2346). (Netero)

🤖

🤖 This review was automatically generated with Coder Agents.

Comment thread cli/server.go Outdated
johnstcn added 2 commits July 1, 2026 00:25
…extual notice when AI Gateway is disabled

Previously the top-of-page banner in AgentsPageView was the only
signal that AI Gateway was disabled deployment-wide, and it didn't
disable anything. Since this session's listChatModels simplification
made /models reflect DB config regardless of runtime gateway state,
a deployment with providers/models configured but AI Gateway off
would show a chat input that looks usable but cannot actually work.

- AgentSetupNotice gains an aiGatewayDisabled branch, checked before
  its existing provider/model checks (it can be true even when the
  catalog has entries), reusing the existing admin-aware messaging
  convention instead of a disconnected page banner.
- AgentChatInput forwards the flag into its existing setup-notice
  logic.
- Both composer entry points (AgentChatPage for existing chats,
  AgentCreateForm for new chats, reached via AgentsPage and
  AgentCreatePage) now fold aiGatewayDisabled into their existing
  disabled-state checks and read the ai-gateway-enabled embedded
  metadata added earlier this session.
- Removed the now-redundant top-of-page banner and its story from
  AgentsPageView; AgentsPage no longer needs the metadata read it
  only fed.
…teway-disabled derivation

- AgentChatInput picked the isAdmin/non-admin AgentSetupNotice branch
  based on providerCount/modelCount being defined, but
  aiGatewayDisabled resolves synchronously from embedded metadata
  while those counts are still loading. An admin would briefly see
  'Contact your admin to enable it' before it flipped to the correct
  admin copy. The branch now short-circuits on aiGatewayDisabled
  directly, independent of the counts.
- Extracted the repeated ai-gateway-enabled metadata read/inversion
  from AgentChatPage.tsx and AgentCreatePage.tsx into a single
  useAIGatewayDisabled() hook.
- Tightened AgentChatInput's AIGatewayDisabledShowsSetupNotice story
  assertion, which matched a substring common to both the admin and
  non-admin copy and couldn't catch a wiring regression between the
  two; added a non-admin counterpart story.

Per deep review.
@johnstcn

This comment was marked as resolved.

johnstcn added 2 commits July 1, 2026 00:49
…isabled

Two crashes when AI.BridgeConfig.Enabled is false (chatDaemon stays
nil):

- The git sync worker captured api.chatDaemon.PublishDiffStatusChange
  as a method value unconditionally. A method value on a nil pointer
  receiver is itself non-nil, so the worker's own nil check on its
  callback never caught it; calling it later dereferenced the nil
  receiver and panicked.
- api.chatDaemon.Close() was called unconditionally during shutdown,
  immediately dereferencing the nil receiver in
  closeInflightAdmission().

Only bind the git sync worker's publish callback when chatDaemon
exists, and guard the shutdown Close() call. Added
TestAIGatewayDisabledStartupAndShutdown, which panics without this
fix (verified by reverting it locally) and passes with it.

Also adds Test_warnDeprecatedChatAIGatewayRoutingFlag, covering the
three explicit ValueSource cases plus the default/absent cases, per
review feedback.

Addresses coder-agents-review P0 and P3 findings on this PR.
… fix

- Found and fixed a third, previously-unreported crash of the same
  class: workspaceAgentRPC assigned api.chatDaemon (a *chatd.Server,
  nil when AI Gateway is disabled) directly into the interface-typed
  ContextDirtyMarker field. A nil pointer assigned to an interface
  produces a non-nil interface value (a typed nil), so agentapi's own
  'if DirtyMarker != nil' check didn't catch it, and calling
  HydrateAndMarkChatsDirty on the nil receiver would panic. The
  removed comment claiming chatDaemon is 'always constructed' was
  stale; extracted a small nil-safe constructor instead.
- Extracted the git-sync-worker callback guard into
  chatDaemonPublishDiffStatusChangeFunc so it's directly unit
  testable (TestChatDaemonPublishDiffStatusChangeFunc) without
  needing to drive the worker's real-time tick loop. Verified this
  test fails against the naive unconditional-method-value version of
  the guard, and passes with the fix, per two reviewers converging on
  the same coverage gap in TestAIGatewayDisabledStartupAndShutdown.
@johnstcn

This comment was marked as resolved.

…scan

warnDeprecatedChatAIGatewayRoutingFlag looped over the whole option
set with a continue to find one flag by name; serpent.OptionSet
already ships ByFlag for exactly this lookup. Also drops one of two
test cases (Default, None) that exercised the identical no-op
branch of the deprecation switch.

Per ponytail review.
Comment thread site/src/pages/AgentsPage/components/AgentSetupNotice.tsx Outdated
Comment thread site/src/hooks/useEmbeddedMetadata.ts Outdated
Comment thread codersdk/deployment_test.go Outdated
Comment thread coderd/coderd_test.go Outdated
Comment thread coderd/coderd_internal_test.go
johnstcn added 4 commits July 1, 2026 08:36
- Drop the redundant NonNilChatDaemon subtest; a real non-nil
  chatDaemon's PublishDiffStatusChange is already exercised in
  coderd/exp_chats_test.go. Keep only the NilChatDaemon case, which
  is the actual non-obvious behavior this guard protects.
- Drop the deprecated-option existence/description assertions added
  to TestChatAIGatewayRoutingEnabledDefault; overkill for an upgrade-
  safety no-op contract.
- Collapse AgentSetupNotice's aiGatewayDisabled admin/non-admin
  branches into one message. Unlike the provider/model branches,
  there's no in-app settings page for this deployment-level flag for
  either audience, so the isAdmin distinction didn't gate any real
  capability difference.
- Rename useAIGatewayDisabled to useAIGatewayEnabled, returning the
  metadata value directly instead of inverting it internally, so the
  hook name matches the ai-gateway-enabled metadata key it reads.
  Callers invert locally (!useAIGatewayEnabled()) where they need
  the disabled polarity. Also drops a redundant .available check in
  the hook itself, since registerValue only ever sets available:true
  alongside a defined value.
…n AI Gateway is disabled

getChat is a pure read; gating it behind requireChatDaemon made every
existing chat look not-found once the gateway was disabled. The FE
chat WebSocket also kept reconnecting to the still-gated stream
endpoint forever with no way to know it could never succeed.
Regression test for the chatDaemon nil-guard fix in getChat; every
existing chat test starts the daemon, so nothing exercised this path.
@johnstcn
johnstcn marked this pull request as ready for review July 1, 2026 10:06
@johnstcn

johnstcn commented Jul 1, 2026 •

Copy link
Copy Markdown
Member Author

Generated by Coder Agents on behalf of @johnstcn.

Verified the change using CDP/Playwright using an authenticated admin session:

/agents list: existing chats render normally in the sidebar. The composer shows "AI Gateway is disabled. Enable it in your deployment config to chat with Coder Agents." and is disabled, as expected.

image

Existing chat detail (/agents/{uuid}): loads fully, title, complete message history, edit-message affordance all present. GET /api/experimental/chats/{id} returned 200 (previously 503'd and rendered as not-found in the UI).

image

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes the migration to mandatory AI Gateway routing for chat by deprecating (as a no-op) the AIGatewayRoutingEnabled deployment option, removing the direct-routing implementation in chatd, and wiring an ai-gateway-enabled signal through server-rendered embedded metadata so the frontend can disable chat UX when the gateway is off.

Changes:

  • Deprecate chat-ai-gateway-routing-enabled (kept for upgrade safety) and warn when it is explicitly set.
  • Remove direct chat routing code paths in chatd, collapsing routing types to an AI-Gateway-only model route and updating tests accordingly.
  • Gate chat functionality when the AI Gateway is disabled: backend returns clear 503s for chat operations needing chatd, and frontend disables composer/UX via embedded metadata.

Reviewed changes

Copilot reviewed 49 out of 49 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
site/src/testHelpers/entities.ts Add mock value for ai-gateway-enabled embedded metadata tests.
site/src/pages/AgentsPage/components/ChatPageContent.tsx Thread aiGatewayDisabled prop into chat input.
site/src/pages/AgentsPage/components/ChatConversation/useChatStore.ts Prevent chat WebSocket subscription when AI Gateway is disabled.
site/src/pages/AgentsPage/components/ChatConversation/chatStore.test.tsx Add coverage for “no WebSocket when gateway disabled”.
site/src/pages/AgentsPage/components/AgentSetupNotice.tsx Show a dedicated notice when AI Gateway is disabled, independent of model/provider counts.
site/src/pages/AgentsPage/components/AgentSetupNotice.stories.tsx Story to validate the “AI Gateway disabled” notice rendering/copy.
site/src/pages/AgentsPage/components/AgentCreateForm.tsx Disable creation inputs and surface setup notice when AI Gateway is disabled.
site/src/pages/AgentsPage/components/AgentCreateForm.stories.tsx Story asserting create-form input is disabled when gateway is off.
site/src/pages/AgentsPage/components/AgentChatInput.tsx Force setup notice path and pass aiGatewayDisabled through to the notice.
site/src/pages/AgentsPage/components/AgentChatInput.stories.tsx Story to ensure notice appears even while catalog counts are loading.
site/src/pages/AgentsPage/AgentsPageView.tsx Layout refactor (structural wrapper) to support updated Agents page composition.
site/src/pages/AgentsPage/AgentCreatePage.tsx Derive aiGatewayDisabled from embedded metadata and pass to create form.
site/src/pages/AgentsPage/AgentChatPageView.tsx Thread aiGatewayDisabled into chat view composition.
site/src/pages/AgentsPage/AgentChatPage.tsx Disable chat composer and prevent live behavior when AI Gateway is disabled.
site/src/hooks/useEmbeddedMetadata.ts Add ai-gateway-enabled metadata key and useAIGatewayEnabled() helper.
site/src/hooks/useEmbeddedMetadata.test.ts Extend embedded metadata tests to include ai-gateway-enabled.
site/src/api/typesGenerated.ts Mark ai_gateway_routing_enabled as deprecated in generated types.
site/site.go Inject ai-gateway-enabled into HTML embedded metadata state.
site/index.html Emit <meta property="ai-gateway-enabled" ...> for runtime metadata parsing.
codersdk/deployment.go Deprecate AIGatewayRoutingEnabled option description; add deprecation comment to ChatConfig.
coderd/x/chatd/title_override.go Remove direct-routing key cascade from title override resolution; route via AI Gateway only.
coderd/x/chatd/title_override_internal_test.go Update title override tests to use AI Gateway transport factory and provider IDs.
coderd/x/chatd/subagent_internal_test.go Add transport factory override wiring and ensure delegated key is present for gateway routing.
coderd/x/chatd/quickgen.go Remove preferred “cheap model” candidate path and collapse to single AI Gateway route usage.
coderd/x/chatd/quickgen_internal_test.go Remove tests tied to preferred candidate generation under direct routing.
coderd/x/chatd/model_routing.go Remove resolvedModelRoute discriminated union and direct-routing selection logic.
coderd/x/chatd/model_routing_internal_test.go Update routing tests for AI Gateway-only route shape and behavior.
coderd/x/chatd/model_routing_direct.go Delete the direct routing implementation.
coderd/x/chatd/model_routing_aibridge.go Return aiGatewayModelRoute directly (no wrapper kind) and update return types.
coderd/x/chatd/integration_responses_test.go Ensure integration tests provide an AI Bridge transport factory.
coderd/x/chatd/generation.go Remove provider-keys field from generation preparation state.
coderd/x/chatd/generation_preparer.go Resolve chat model via AI Gateway route only; remove provider-keys plumbing.
coderd/x/chatd/generation_preparer_internal_test.go Update tests to provide transport factory and align assertions with removed provider keys.
coderd/x/chatd/computer_use.go Update computer-use model resolution signature to AI Gateway route type.
coderd/x/chatd/chatd.go Remove aiGatewayRoutingEnabled server field and simplify advisor override routing error behavior.
coderd/x/chatd/chatd_test.go Update extensive chatd tests to inject AI Bridge transport factory and remove routing flag usage.
coderd/x/chatd/chatd_retry_test.go Wire mock transport factory for retry tests.
coderd/x/chatd/chatd_internal_test.go Update title regeneration tests to use AI Gateway transport; add request-model assertion helper.
coderd/x/chatd/chatd_debug.go Simplify provider hint handling for debug-aware model creation under AI Gateway routing.
coderd/x/chatd/chatd_chainmode_test.go Inject AI Bridge transport factory into chain-mode tests.
coderd/x/chatd/advisor_internal_test.go Update advisor override tests for AI Gateway-only routing and required delegated API key IDs.
coderd/workspaceagentsrpc.go Fix typed-nil interface assignment when chatDaemon is nil (gateway disabled).
coderd/exp_chats.go Add requireChatDaemon and gate chat operations with 503 when gateway disabled.
coderd/exp_chats_test.go Add regression test ensuring getChat still works with AI Gateway disabled.
coderd/coderd.go Only construct chat daemon when AI Gateway enabled; fix nil method-value callback and guarded shutdown.
coderd/coderd_test.go Regression test for clean startup/shutdown with AI Gateway disabled.
coderd/coderd_internal_test.go Unit test proving chatDaemonPublishDiffStatusChangeFunc(nil) returns a true nil func.
cli/testdata/server-config.yaml.golden Update golden config docs for deprecated routing flag description.
cli/server.go Warn when deprecated chat-ai-gateway-routing-enabled is explicitly set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread coderd/exp_chats.go Outdated

@DanielleMaywood DanielleMaywood left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just giving a review of the frontend for now

Comment thread site/src/pages/AgentsPage/AgentsPageView.tsx
Comment thread site/src/pages/AgentsPage/components/AgentSetupNotice.tsx Outdated
Comment thread site/src/pages/AgentsPage/components/AgentCreateForm.tsx Outdated
Comment thread site/src/hooks/useEmbeddedMetadata.ts Outdated
Comment thread site/src/pages/AgentsPage/AgentCreatePage.tsx
- Collapse a vestigial flex-row wrapper div in AgentsPageView left over
  from a removed banner; restore sm:flex-row on the outer layout div.
- Drop three comments that only restated the code they annotated.
- Tighten requireChatDaemon's doc comment: it no longer covers all
  chat HTTP operations now that pure reads (getChat) are exempt.
@johnstcn
johnstcn requested a review from DanielleMaywood July 1, 2026 11:45

@mafredri mafredri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing critique of this PR against the clear-writing standard, scoped to the PR description and the net-diff comments (superseded intermediate-commit comments excluded). Comment-level findings are inline; description findings are below. Each finding includes full replacement text.

Description

  1. Process theater. The closing line

    All changes reviewed via multi-reviewer deep-review passes; findings addressed inline in review threads.

    announces thoroughness without telling a reader anything about the change or how to act on it. Remove it; the description already states the scope. Nothing replaces it.

  2. Unresolvable shorthand. Depends on #26658 (PR 2 test migration). All tests already run through AI Gateway routing from PR 2. "PR 2" is an internal label a reader of this PR cannot resolve. Replacement:

    Depends on #26658, which migrated the chat tests onto AI Gateway routing. With that merged, every test already exercises the AI Gateway path.

  3. Reader load in the lead paragraph. The headline (deprecate the option as a no-op) and five separate cleanups are chained into one sentence, so the lead does not stand alone and the cleanups are not a scannable set. Replacement for the first paragraph:

    Deprecate the AIGatewayRoutingEnabled deployment option as a no-op instead of hard-deleting it, so deployments that still carry the flag in YAML or CLI args do not break on upgrade.

    This also removes the now-dead direct-routing code:

    • Delete the direct routing implementation.
    • Collapse the resolvedModelRoute discriminated union into aiGatewayModelRoute.
    • Remove the dead providerKeys cascade.
    • Delete the preferredShortTextCandidates quickgen function.
    • Simplify the advisor override error handling.

🤖 This review was automatically generated with Coder Agents.

Comment thread coderd/coderd_internal_test.go Outdated
Comment thread coderd/coderd_test.go Outdated
Comment thread coderd/x/chatd/chatd_internal_test.go Outdated
Comment thread coderd/x/chatd/chatd_internal_test.go Outdated
Comment thread site/src/pages/AgentsPage/components/AgentSetupNotice.stories.tsx Outdated
Comment thread coderd/exp_chats.go Outdated
Comment thread cli/server.go Outdated
Comment thread coderd/x/chatd/chatd_debug.go Outdated
Comment thread coderd/x/chatd/chatd_internal_test.go Outdated
Comment thread coderd/exp_chats.go
johnstcn added 2 commits July 1, 2026 18:31
…ed-flag-and-dead-direct-routing

Resolves conflicts with #26877 (drop chat_model_configs provider column):
- Delete model_routing_direct.go entirely (dead code path this PR removes).
- Drop `Provider` field from database.ChatModelConfig literals (column dropped).
- Replace `route.providerHint()` calls with `string(route.Provider.Type)`
  (method removed alongside the resolvedModelRoute union we collapsed).
- Keep our single-`candidate` shape in quickgen; drop the multi-candidate
  fallback fragments from main.
- Fold main's added `GetAIProvider*` mock expectations into our test wiring
  where needed; preserve our aibridgeTestAIProvider helper (which sets
  Provider.Name, required by newAIGatewayModel validation).
@johnstcn
johnstcn merged commit 4936ff9 into main Jul 1, 2026
32 of 33 checks passed
@johnstcn
johnstcn deleted the cian/codagt-682-remove-aigatewayroutingenabled-flag-and-dead-direct-routing branch July 1, 2026 19:15
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants