From bde38e9d102ee4b7653edbb05cff0efdea784c5c Mon Sep 17 00:00:00 2001
From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com>
Date: Tue, 11 Aug 2026 23:51:00 +0200
Subject: [PATCH] fix(coderd/x/chatd): synchronize aibridgeTestFactory recorded
 fields (#28031)

Fixes the data race in the chatd test helper `aibridgeTestFactory`
reported in CODAGT-917 (`test-go-race-pg` flake in
`TestAwaitSubagentCompletion/Timeout`).

`TransportFor` recorded `providerName` and `source` with plain field
writes. Tests that start the chat worker share one factory between
concurrently running chat runners (parent chat and spawned subagent), so
two runners resolving models at the same time raced on those writes.

The fix guards the recorded fields with a mutex and reads them through a
locked `recorded()` accessor at the three asserting call sites.

Verified with a red-green repro: concurrent `TransportFor` calls on one
factory failed `go test -race` with the exact CI signature (lines 37-38)
before the fix and pass after it. Also ran `go test -race
./coderd/x/chatd -run TestAwaitSubagentCompletion -count=10` and the
full `go test -race ./coderd/x/chatd` package, both clean.

Audited every other `aibridge.TransportFactory` implementation and
`aibridgeTestFactory` use site for the same defect:
`chattest.MockAIBridgeTransport` is already mutex-guarded,
`stubTransportFactory` (coderd/aibridge_test.go) records via a channel,
`providerRoutedTransportFactory` (chatd_test.go) is a stateless lookup,
and the production factories keep no recorded state. No other occurrence
exists.

Closes CODAGT-917.

> Mux acted on Mike's behalf to create this PR.
---
 coderd/x/chatd/model_routing_internal_test.go | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/coderd/x/chatd/model_routing_internal_test.go b/coderd/x/chatd/model_routing_internal_test.go
index 10aa66e566..6a6d1324ee 100644
--- a/coderd/x/chatd/model_routing_internal_test.go
+++ b/coderd/x/chatd/model_routing_internal_test.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"net/http"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"testing"
 
@@ -27,6 +28,7 @@ import (
 )
 
 type aibridgeTestFactory struct {
+	mu           sync.Mutex
 	providerName string
 	source       aibridge.Source
 	err          error
@@ -34,6 +36,8 @@ type aibridgeTestFactory struct {
 }
 
 func (f *aibridgeTestFactory) TransportFor(providerName string, source aibridge.Source) (http.RoundTripper, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
 	f.providerName = providerName
 	f.source = source
 	if f.err != nil {
@@ -42,6 +46,12 @@ func (f *aibridgeTestFactory) TransportFor(providerName string, source aibridge.
 	return f.rt, nil
 }
 
+func (f *aibridgeTestFactory) recorded() (providerName string, source aibridge.Source) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.providerName, f.source
+}
+
 type roundTripFunc func(*http.Request) (*http.Response, error)
 
 func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
@@ -617,8 +627,9 @@ func TestAIBridgeGatewayProviderTypesPreserveSlashModelID(t *testing.T) {
 			got := <-seen
 			require.NotEmpty(t, got.path)
 			require.Equal(t, modelName, got.model)
-			require.Equal(t, tt.providerName, factory.providerName)
-			require.Equal(t, aibridge.SourceAgents, factory.source)
+			gotProvider, gotSource := factory.recorded()
+			require.Equal(t, tt.providerName, gotProvider)
+			require.Equal(t, aibridge.SourceAgents, gotSource)
 		})
 	}
 }
@@ -655,8 +666,9 @@ func TestAIBridgeComputerUseModelUsesRoute(t *testing.T) {
 	require.False(t, debugEnabled)
 	require.EqualValues(t, codersdk.ChatComputerUseProviderOpenAI, resolvedProvider)
 	require.Equal(t, modelName, resolvedModel)
-	require.Equal(t, "primary-openai", factory.providerName)
-	require.Equal(t, aibridge.SourceAgents, factory.source)
+	gotProvider, gotSource := factory.recorded()
+	require.Equal(t, "primary-openai", gotProvider)
+	require.Equal(t, aibridge.SourceAgents, gotSource)
 }
 
 // The computer-use model is a hardcoded default with no config of its own, so
@@ -776,8 +788,9 @@ func TestAIBridgeDelegatedContextPropagation(t *testing.T) {
 	require.NoError(t, err)
 
 	got := <-seen
-	require.Equal(t, "primary-openai", factory.providerName)
-	require.Equal(t, aibridge.SourceAgents, factory.source)
+	gotProvider, gotSource := factory.recorded()
+	require.Equal(t, "primary-openai", gotProvider)
+	require.Equal(t, aibridge.SourceAgents, gotSource)
 	require.True(t, got.ok)
 	require.Equal(t, "/v1/responses", got.path)
 	require.Equal(t, apiKeyID, got.apiKeyID)