test: extract AI Gateway test helpers for chatd by johnstcn · Pull Request #26639 · coder/coder

johnstcn · 2026-06-23T19:26:32Z

Extracts test infrastructure for AI Gateway routing into shared helpers so both AGPL and enterprise tests can use them.

Two helpers:

aibridgedtest.StartTestAIBridgeDaemon (in coderd/aibridgedtest/aibridgedtest.go, !slim build tag) — a real in-process aibridged daemon wired to fake upstream providers. Extracted from enterprise/coderd/aibridge_reload_test.go's startTestAIBridgeDaemon. Stands up the real aibridge stack: cached bridge pool, DRPC backend, provider reload subscription, in-memory HTTP handler. Tests that create AI provider rows with BaseURL pointing at httptest.Server URLs get their requests proxied through the real aibridged transport: auth header injection, SSE streaming, request recording, and path rewriting all run as they would in production. No mock maintenance, no drift.

Signature: StartTestAIBridgeDaemon(ctx context.Context, t testing.TB, api *coderd.API, metrics *aibridged.Metrics). Caller controls context cancellation and owns the metrics instance (pass aibridged.NewMetrics(prometheus.NewRegistry()) or nil for a throwaway).

Lives in a separate coderd/aibridgedtest package to avoid an import cycle: coderdtest is imported by cli test files, and the helper needs cli.BuildProviders, so it cannot live in coderdtest.
chattest.MockAIBridgeTransport (in coderd/x/chatd/chattest/mock_aibridge_transport.go) — a mock aibridge.TransportFactory for the 3 bare-chatd tests that use newActiveTestServer (no full coderd API, can't call CreateInMemoryAIBridgeServer). Extracted from chatd_test.go's chatAIGatewayTestFactory. Redirects requests to a target URL (mock provider server) and records each request via a cloned *http.Request. Mirrors the real aibridged transport's delegated API key invariant. Uses functional options: NewMockAIBridgeTransport(t, url, opts...) with WithPreservePath() option.

Why both: The 19 full-server tests have a coderd.API available and can use the real daemon via api.RegisterInMemoryAIBridgedHTTPHandler. The 3 bare-chatd tests use newActiveTestServer (no coderd API) and can't call CreateInMemoryAIBridgeServer, so they need the mock transport. The real daemon approach uses real aibridge.Provider implementations pointing at httptest.Server URLs — the "fake" is the upstream HTTP server, not a mocked interface. No drift risk.

No production code changes. All existing tests pass.

This is PR 1 of a 3-PR stack that removes the AIGatewayRoutingEnabled flag and its dead direct routing paths.

…icking fields

… mock transport

…process aibridge

…ridge helpers

johnstcn · 2026-06-24T11:21:53Z

/coder-agents-review

coder-agents-review · 2026-06-24T11:21:57Z

Chat: Review posted | View chat
Requested: 2026-06-24 12:20 UTC by @johnstcn
Spend: $52.74 / $100.00

Review history

R1 (2026-06-24), 1 P2, 1 P3, COMMENT. Review
R2 (2026-06-24): 17 reviewers, 3 Nit, 1 P2, 3 P3, COMMENT. Review
R3 (2026-06-24): 10 reviewers, 6 Nit, 1 P2, 3 P3, APPROVE. Review

deep-review v0.9.0 | Round 3 | 44f2a77..28189d1

Last posted: Round 3, 10 findings (1 P2, 3 P3, 6 Nit), APPROVE. Review

Finding inventory

Findings

#	Sev	Status	Location	Summary	Round	Reviewer	Posted
CRF-1	P2	Author fixed (`d858ca7`)	aibridgedtest.go:1	Missing `//go:build !slim` build tag breaks slim compilation	R1	Netero	Yes
CRF-2	P3	Author fixed (`d858ca7`)	chatd_test.go:9745	TestProcessChat_AIGatewayRoutingUsesDelegatedAPIKey weakened from loop-all to first-only assertions	R1	Netero	Yes
CRF-3	P3	Author fixed (`28189d1`)	aibridgedtest.go:95	`defer RecordReloadAttempt()` inverts metric temporal contract vs production	R2	Chopper P3, Knov P3	Yes
CRF-4	P3	Author fixed (`28189d1`)	aibridgedtest.go:23	Doc is 19 lines but skips `api` parameter; parameters documented out of signature order	R2	Gon P2, Leorio P3	Yes
CRF-5	Nit	Author fixed (`28189d1`)	aibridgedtest.go:3	Package doc says "wired to fake upstream providers" but the package does not wire fakes	R2	Gon P2	Yes
CRF-6	Nit	Author fixed (`28189d1`)	mock_aibridge_transport.go:82	`append([]RecordedRequest(nil), ...)` regressed from `slices.Clone`	R2	Ging-Go	Yes
CRF-7	Nit	Author fixed (`28189d1`)	aibridgedtest.go:44	`*testing.T` where `testing.TB` suffices; inconsistent with `NewMockAIBridgeTransport`	R2	Zoro, Knov	Yes
CRF-8	Nit	Dropped by orchestrator (naming convention is established; renaming would cascade)	mock_aibridge_transport.go:50	`MockAIBridgeTransport` should be `MockAIBridgeTransportFactory`	R2	Gon	No
CRF-9	P2	Dropped by orchestrator (one redundant sentence in 2-sentence doc; not worth posting)	mock_aibridge_transport.go:15	RecordedRequest doc restates struct layout	R2	Gon	No
CRF-10	P2	Dropped by orchestrator (usage context helps first-time readers; not diluting)	mock_aibridge_transport.go:41	MockAIBridgeTransport type doc carries usage examples	R2	Gon	No

| CRF-11 | Nit | Open | mock_aibridge_transport.go:29 | "when one was set" implies APIKeyID can be empty; RoundTrip errors before recording when key is absent | R3 | Gon P3, Leorio Nit | Yes |
| CRF-12 | Nit | Open | aibridgedtest.go:32 | "t provides cleanup and fatal helpers" restates testing.TB | R3 | Gon | Yes |
| CRF-13 | Nit | Open | aibridgedtest.go:38 | metrics doc omits that nil is accepted | R3 | Leorio | Yes |

Round log

Round 1

Netero-only. 1 P2, 1 P3. Reviewed against 44f2a77..0c0ee00.

Round 2

Panel (17 reviewers). CRF-1, CRF-2 addressed. 2 P3, 3 Nit new. 3 dropped. Reviewed against 44f2a77..d858ca7.

Round 3

Panel (10 reviewers). CRF-3 through CRF-7 addressed. 3 Nit new. Reviewed against 44f2a77..28189d1.

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.

coder-agents-review

First-pass review (Netero). This is a mechanical review only; the full review panel has not yet reviewed this PR and will review after these findings are addressed.

Severity summary: 1 P2, 1 P3.

The extraction itself is clean. Both new packages compile, tests pass, and the deleted code has no orphaned references. Two things to address before the panel reviews.

🤖 This review was automatically generated with Coder Agents.

johnstcn · 2026-06-24T11:39:29Z

/coder-agents-review

coder-agents-review

Panel review (17 reviewers). CRF-1 and CRF-2 from round 1 are verified fixed in d858ca7.

Severity summary: 2 P3, 3 Nit.

Clean extraction. The PR description's framing (sharing test helpers across package boundaries, avoiding import cycles) matches what the code does. The secondary improvements are all directionally correct: caller-controlled context and metrics on the daemon helper, enforced delegated API key invariant in the mock transport, and *http.Request recording that eliminates future struct-change churn. The RecordedRequest redesign is a genuine improvement over the flat struct it replaced.

Notes from the panel:

(Ryosuke) coderd/aibridgedtest imports cli, reaching upward in the dependency graph. No cycle, but the direction is unusual. If the 3-PR stack makes aibridgedtest a long-term fixture, relocating BuildProviders to coderd/aibridged (or a sibling) would eliminate the upward reach. Not a problem for this PR.
(Ryosuke) RecordedRequest.Request is a shallow clone; the body reader is shared with the forwarded request. Current tests only inspect headers and URL, so this is inert. The doc says "shallow clone" which is accurate. A future test inspecting .Body would get EOF.
(Gon) MockAIBridgeTransport implements aibridge.TransportFactory but is named Transport. The existing convention (stubTransportFactory in coderd/aibridge_test.go) uses the interface name. Not filed as a finding because the name is established and referenced in the PR; renaming would cascade.
(Leorio) The PR description shows StartTestAIBridgeDaemon(t *testing.T, ctx context.Context, ...) but the actual signature is (ctx context.Context, t *testing.T, ...) after the revive lint fix. Worth updating the description.
(Leorio) "That mock aibridged transport error message is a masterpiece of test diagnostics" (paraphrased): names the constraint, the fix function, and where it belongs.
(Komugi) Stress-tested all four affected tests with -race -count=10 at GOMAXPROCS=1 and GOMAXPROCS=2. All green.

🤖 This review was automatically generated with Coder Agents.

johnstcn · 2026-06-24T12:20:48Z

/coder-agents-review

coder-agents-review

All prior findings (CRF-1 through CRF-7) verified fixed across three rounds. 10 reviewers, 7 returned no findings. CI pending.

Clean extraction. The fixes over the three rounds were all genuine: build tag added, loop-all assertions restored, defer ordering corrected, docs rewritten with parameter coverage, slices.Clone restored, testing.TB adopted. No fix introduced a regression.

3 Nits below for optional polish. None blocking.

"A mock that silently accepts missing invariants is costume jewelry pretending to be a safety net." (Bisky, on the delegated API key enforcement upgrade)

🤖 This review was automatically generated with Coder Agents.

test(coderd/x/chatd/chattest): extract mock AI Gateway transport factory

dae7687

github-actions Bot assigned johnstcn Jun 23, 2026

johnstcn changed the title ~~test(coderd/x/chatd/chattest): extract mock AI Gateway transport factory~~ chore(coderd/x/chatd/chattest): extract mock AI Gateway transport factory Jun 23, 2026

johnstcn changed the title ~~chore(coderd/x/chatd/chattest): extract mock AI Gateway transport factory~~ chore(coderd/x/chatd): extract mock AI Gateway transport factory Jun 23, 2026

johnstcn added 7 commits June 23, 2026 19:35

test(coderd/x/chatd/chattest): clone http.Request instead of cherry-p…

bd50eec

…icking fields

test(coderd/x/chatd/chattest): enforce delegated API key invariant in…

503bbdb

… mock transport

test(coderd/coderdtest): extract StartTestAIBridgeDaemon for real in-…

b3f51d5

…process aibridge

test: functional options and caller-supplied ctx/metrics for test aib…

1dbd6d7

…ridge helpers

test: rename MockTransportFactory to MockAIBridgeTransport

3623d5b

test: drop metrics return from StartTestAIBridgeDaemon, caller owns it

6aa469b

test: move StartTestAIBridgeDaemon to aibridgedtest to fix import cycle

b8cfda4

johnstcn changed the title ~~chore(coderd/x/chatd): extract mock AI Gateway transport factory~~ test: extract AI Gateway test helpers for chatd Jun 24, 2026

test: fix context.Context parameter order for revive lint

0c0ee00

coder-agents-review Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread coderd/aibridgedtest/aibridgedtest.go Outdated

Comment thread coderd/x/chatd/chatd_test.go

test: fix build tag and restore loop-all assertions from review

d858ca7

johnstcn mentioned this pull request Jun 24, 2026

test: migrate chatd tests to AI Gateway routing #26658

Draft

coder-agents-review Bot reviewed Jun 24, 2026

View reviewed changes

test: address review findings (defer, docs, slices.Clone, testing.TB)

28189d1

johnstcn commented Jun 24, 2026

View reviewed changes

Comment thread coderd/aibridgedtest/aibridgedtest.go

johnstcn commented Jun 24, 2026

View reviewed changes

Comment thread coderd/aibridgedtest/aibridgedtest.go Outdated

johnstcn commented Jun 24, 2026

View reviewed changes

Comment thread coderd/x/chatd/chattest/mock_aibridge_transport.go Outdated

coder-agents-review Bot approved these changes Jun 24, 2026

View reviewed changes

Comment thread coderd/x/chatd/chattest/mock_aibridge_transport.go

Comment thread coderd/aibridgedtest/aibridgedtest.go

Comment thread coderd/aibridgedtest/aibridgedtest.go

test: address review comments on docs and clone wording

f8f1131

Conversation

johnstcn commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnstcn commented Jun 24, 2026

Uh oh!

coder-agents-review Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Findings

Round log

Round 1

Round 2

Round 3

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

johnstcn commented Jun 24, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnstcn commented Jun 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

johnstcn commented Jun 23, 2026 •

edited

Loading

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

edited

Loading