fix(coderd/x/chatd/mcpclient): enforce MCP connect budget and unblock session cleanup (#28400)

## Problem

During the Aug 19 dev.coder.com incident, chats stalled for up to ~15
minutes per turn. chatd reconnects to every configured MCP server on
every generation step, and one configured server
(`registry.coder.com/mcp`) was black-holing requests from the
deployment's egress IPs: TCP/requests were silently dropped, and each
connect attempt hung until the kernel gave up (~125s), far past the
nominal 10s connect budget.

The budget does not hold because go-sdk v1.7.0 detaches the context
inside `StreamableClientTransport.Connect`, and its error paths block on
HTTP work bound to that detached context. Reproduced in a test: a bare
`Connect` with a 2s deadline against a black-holed server returns after
**12s**. Step-end session cleanup (`session.Close()`) runs synchronously
in the generation loop and blocks on the same detached requests, so a
server that wedges mid-turn stalls every step boundary too.

## Changes

- **Enforce the connect budget externally** (`connectOne`): run
`Connect` + `ListTools` in a goroutine and select on the budget context.
On timeout, abandon the goroutine and leave a reaper that drains its
late result and closes any session that still materialized, so nothing
leaks and the caller returns within the budget.
- **Stop using bare `http.DefaultTransport`**: MCP traffic now rides a
cloned transport with a 5s dial timeout (converts SYN black-holes into
fast errors) and a 60s `ResponseHeaderTimeout`. `http.Client.Timeout`
stays unset so long-lived SSE streams are unaffected.
- **Close sessions in detached goroutines during cleanup**
(`ConnectAll`'s cleanup func): the sessions are discarded regardless,
and a wedged `Close` (DELETE bound to the SDK's detached context) must
not stall the generation loop at step boundaries.

One deliberate deviation from the incident plan: `ResponseHeaderTimeout`
is 60s (matching `toolCallTimeout`) instead of ~10s. The same HTTP
client serves tool-call POSTs, and a JSON-response MCP server sends no
headers until the tool finishes, so a 10s bound would falsely kill
legitimate 10-60s tools that fit today's tool-call budget. The real 10s
connect budget is enforced by the external select, not the transport.

## Tests

- Acceptance: with a black-holed server plus a healthy one configured,
`ConnectAll` returns within the budget and the healthy server's tools
are present; closing the black-holed connections makes the reaper exit
(red without the fix: 1s budget took 11s).
- A slow-but-alive server (300ms/request) still connects.
- A server that only responds after the budget: `ConnectAll` returns
promptly, the late result is reaped.
- Cleanup returns promptly while the session-teardown DELETE is wedged
server-side, and the teardown still happens in the background (red
without the fix: cleanup blocked 60s).
- Transport guard test pinning the dial/response-header bounds.

`go test ./coderd/x/chatd/...` passes (including goleak in `chatd`).

Part of the MCP connect-stall incident follow-up; observability (connect
durations in logs/debug runs) comes in a stacked follow-up PR.

> 🤖 Mux authored this PR on Mike's behalf.

<!-- mux-attribution: model=anthropic:claude-opus-4-6 thinking=high -->

coderd/x/chatd/mcpclient/export_test.go

@@ -1,9 +1,36 @@
 package mcpclient
 
+import (
+	"context"
+	"time"
+
+	"charm.land/fantasy"
+	"github.com/google/uuid"
+
+	"cdr.dev/slog/v3"
+	"github.com/coder/coder/v2/coderd/database"
+)
+
 // ConvertCallResultForTest exposes convertCallResult for external
 // tests.
 var ConvertCallResultForTest = convertCallResult
 
+// ConnectAllForTest exposes connectAll with an injectable connect
+// timeout and a reaperDone hook that fires after an abandoned
+// connect goroutine has been drained and its late session closed.
+func ConnectAllForTest(
+	ctx context.Context,
+	logger slog.Logger,
+	configs []database.MCPServerConfig,
+	timeout time.Duration,
+	reaperDone func(),
+) ([]fantasy.AgentTool, func()) {
+	return connectAllWithHooks(
+		ctx, logger, configs, nil, uuid.Nil, nil, nil,
+		timeout, connectHooks{reaperDone: reaperDone},
+	)
+}
+
 // BuildAuthHeadersForTest exposes buildAuthHeaders for external
 // tests.
 var BuildAuthHeadersForTest = buildAuthHeaders

coderd/x/chatd/mcpclient/mcpclient.go

@@ -84,6 +84,32 @@ func ConnectAll(
 	userID uuid.UUID,
 	oidcSrc UserOIDCTokenSource,
 	coderHeaders map[string]string,
+) ([]fantasy.AgentTool, func()) {
+	return connectAllWithHooks(
+		ctx, logger, configs, tokens, userID, oidcSrc, coderHeaders,
+		connectTimeout, connectHooks{},
+	)
+}
+
+// connectHooks carries test-only instrumentation for connect
+// internals. The zero value is used in production.
+type connectHooks struct {
+	// reaperDone, when non-nil, is called after an abandoned
+	// connect goroutine's late result has been drained and any
+	// late session closed.
+	reaperDone func()
+}
+
+func connectAllWithHooks(
+	ctx context.Context,
+	logger slog.Logger,
+	configs []database.MCPServerConfig,
+	tokens []database.MCPServerUserToken,
+	userID uuid.UUID,
+	oidcSrc UserOIDCTokenSource,
+	coderHeaders map[string]string,
+	timeout time.Duration,
+	hooks connectHooks,
 ) ([]fantasy.AgentTool, func()) {
 	// Index tokens by server config ID so auth header
 	// construction is O(1) per server.
@@ -101,14 +127,20 @@ func ConnectAll(
 	)
 
 	// Build cleanup eagerly so it always closes any sessions
-	// that connected, even if a later connection fails.
+	// that connected, even if a later connection fails. Each
+	// close runs in a detached goroutine: the sessions are
+	// discarded either way, and Close on a server that stopped
+	// responding mid-turn can block until the transport abandons
+	// the connection (the SDK detaches the request context), which
+	// must not stall the generation loop at step boundaries.
 	cleanup := func() {
 		mu.Lock()
-		defer mu.Unlock()
-		for _, s := range sessions {
-			_ = s.Close()
-		}
+		toClose := sessions
 		sessions = nil
+		mu.Unlock()
+		for _, s := range toClose {
+			go func() { _ = s.Close() }()
+		}
 	}
 
 	var eg errgroup.Group
@@ -120,6 +152,7 @@ func ConnectAll(
 		eg.Go(func() error {
 			serverTools, session, connectErr := connectOne(
 				ctx, logger, cfg, tokensByConfigID, userID, oidcSrc, coderHeaders,
+				timeout, hooks,
 			)
 			if connectErr != nil {
 				logger.Warn(ctx,
@@ -211,6 +244,8 @@ func connectOne(
 	userID uuid.UUID,
 	oidcSrc UserOIDCTokenSource,
 	coderHeaders map[string]string,
+	timeout time.Duration,
+	hooks connectHooks,
 ) ([]fantasy.AgentTool, *mcp.ClientSession, error) {
 	headers := buildAuthHeaders(ctx, logger, cfg, tokensByConfigID, userID, oidcSrc)
 
@@ -250,21 +285,64 @@ func connectOne(
 	// The timeout covers the entire connect+list sequence, not
 	// each phase individually. The SDK negotiates the protocol
 	// version during Connect; the session outlives connectCtx.
-	connectCtx, cancel := context.WithTimeout(
-		ctx, connectTimeout,
-	)
+	connectCtx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
 
-	session, err := mcpClient.Connect(connectCtx, tr, nil)
-	if err != nil {
-		return nil, nil, xerrors.Errorf("connect: %w", err)
+	// Run the connect+list sequence in a goroutine and enforce the
+	// budget externally. The SDK's streamable transport detaches
+	// the context after starting HTTP requests, and its error-path
+	// session.Close blocks on those detached requests, so a
+	// black-holed server can block Connect far past connectCtx's
+	// deadline. The select below guarantees the caller gets an
+	// answer within the budget regardless.
+	type connectResult struct {
+		session *mcp.ClientSession
+		tools   *mcp.ListToolsResult
+		err     error
 	}
+	resCh := make(chan connectResult, 1)
+	go func() {
+		session, err := mcpClient.Connect(connectCtx, tr, nil)
+		if err != nil {
+			resCh <- connectResult{err: xerrors.Errorf("connect: %w", err)}
+			return
+		}
+		toolsResult, err := session.ListTools(connectCtx, nil)
+		if err != nil {
+			// Deliver the result before closing: Close sends a
+			// DELETE on the SDK's detached context and can wedge,
+			// which would otherwise convert a fast ListTools
+			// failure into a budget timeout for the caller.
+			resCh <- connectResult{err: xerrors.Errorf("list tools: %w", err)}
+			_ = session.Close()
+			return
+		}
+		resCh <- connectResult{session: session, tools: toolsResult}
+	}()
 
-	toolsResult, err := session.ListTools(connectCtx, nil)
-	if err != nil {
-		_ = session.Close()
-		return nil, nil, xerrors.Errorf("list tools: %w", err)
+	var res connectResult
+	select {
+	case res = <-resCh:
+	case <-connectCtx.Done():
+		// Abandon the wedged goroutine; it exits once the
+		// transport's dial or response-header timeout fires. The
+		// reaper drains its late result and closes any session
+		// that still materialized so nothing leaks. It must not
+		// hold locks or block the caller.
+		go func() {
+			if late := <-resCh; late.session != nil {
+				_ = late.session.Close()
+			}
+			if hooks.reaperDone != nil {
+				hooks.reaperDone()
+			}
+		}()
+		return nil, nil, xerrors.Errorf("connect: %w", connectCtx.Err())
 	}
+	if res.err != nil {
+		return nil, nil, res.err
+	}
+	session, toolsResult := res.session, res.tools
 
 	var tools []fantasy.AgentTool
 	for _, mcpTool := range toolsResult.Tools {
@@ -286,7 +364,11 @@ func connectOne(
 	}
 
 	if len(tools) == 0 {
-		_ = session.Close()
+		// Close the discarded session asynchronously: Close sends
+		// a DELETE on the SDK's detached context, so a server that
+		// wedges after a successful connect would otherwise hold
+		// the caller far past the connect budget.
+		go func() { _ = session.Close() }()
 		return nil, nil, nil
 	}
 

coderd/x/chatd/mcpclient/mcpclient_connect_test.go

@@ -0,0 +1,292 @@
+package mcpclient_test
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+	"github.com/stretchr/testify/require"
+
+	"cdr.dev/slog/v3/sloggers/slogtest"
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/x/chatd/mcpclient"
+)
+
+// blackHoleListener accepts TCP connections and never responds,
+// simulating a server (or an edge in front of it) that silently
+// drops requests. Returned connections are tracked so the test can
+// terminate them.
+type blackHoleListener struct {
+	ln net.Listener
+
+	mu    sync.Mutex
+	conns []net.Conn
+}
+
+func newBlackHoleListener(t *testing.T) *blackHoleListener {
+	t.Helper()
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	bh := &blackHoleListener{ln: ln}
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			bh.mu.Lock()
+			bh.conns = append(bh.conns, conn)
+			bh.mu.Unlock()
+		}
+	}()
+	t.Cleanup(bh.close)
+	return bh
+}
+
+func (b *blackHoleListener) close() {
+	_ = b.ln.Close()
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	for _, c := range b.conns {
+		_ = c.Close()
+	}
+	b.conns = nil
+}
+
+func (b *blackHoleListener) url() string {
+	return "http://" + b.ln.Addr().String()
+}
+
+// TestConnectAll_BlackHoledServerBudget is the acceptance test for
+// the connect budget: one black-holed server must not delay turn
+// preparation beyond the budget, and healthy servers' tools must
+// still be discovered. Without external budget enforcement the SDK
+// blocks several times past the context deadline (observed: 12s
+// for a 2s deadline) because its transport detaches the context.
+func TestConnectAll_BlackHoledServerBudget(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	bh := newBlackHoleListener(t)
+	healthy := newTestMCPServer(t, echoTool())
+
+	reaperDone := make(chan struct{}, 2)
+	timeout := 1 * time.Second
+
+	start := time.Now()
+	tools, cleanup := mcpclient.ConnectAllForTest(ctx, logger,
+		[]database.MCPServerConfig{
+			makeConfig("blackhole", bh.url()),
+			makeConfig("healthy", healthy.URL),
+		},
+		timeout,
+		func() { reaperDone <- struct{}{} },
+	)
+	elapsed := time.Since(start)
+	t.Cleanup(cleanup)
+
+	// The budget must hold: well under the SDK's unbounded
+	// behavior (6x the deadline), with margin for slow CI.
+	require.Less(t, elapsed, 4*timeout,
+		"ConnectAll took %s, budget was %s", elapsed, timeout)
+	require.Equal(t, []string{"healthy__echo"}, toolNames(tools))
+
+	// Terminating the black-holed connections unblocks the
+	// abandoned connect goroutine; the reaper must then drain its
+	// result and exit.
+	bh.close()
+	select {
+	case <-reaperDone:
+	case <-time.After(30 * time.Second):
+		t.Fatal("reaper did not exit after black-holed connections were closed")
+	}
+}
+
+// TestConnectAll_SlowServerStillConnects proves that a server that
+// is slow but within the budget still connects and serves tools.
+func TestConnectAll_SlowServerStillConnects(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	srv := mcp.NewServer(&mcp.Implementation{Name: "slow", Version: "1.0.0"}, nil)
+	tool := echoTool()
+	srv.AddTool(tool.tool, tool.handler)
+	handler := mcp.NewStreamableHTTPHandler(
+		func(*http.Request) *mcp.Server { return srv },
+		&mcp.StreamableHTTPOptions{Stateless: true},
+	)
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(300 * time.Millisecond)
+		handler.ServeHTTP(w, r)
+	}))
+	t.Cleanup(ts.Close)
+
+	cfg := makeConfig("slow", ts.URL)
+	tools, cleanup := mcpclient.ConnectAll(ctx, logger, []database.MCPServerConfig{cfg}, nil, uuid.Nil, nil, nil)
+	t.Cleanup(cleanup)
+
+	require.Equal(t, []string{"slow__echo"}, toolNames(tools))
+}
+
+// TestConnectAll_LateServerReaped proves that when a server only
+// responds after the budget expired, ConnectAll has long returned
+// and the abandoned connect goroutine's late result is drained by
+// the reaper so nothing leaks.
+func TestConnectAll_LateServerReaped(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	srv := mcp.NewServer(&mcp.Implementation{Name: "late", Version: "1.0.0"}, nil)
+	tool := echoTool()
+	srv.AddTool(tool.tool, tool.handler)
+	handler := mcp.NewStreamableHTTPHandler(
+		func(*http.Request) *mcp.Server { return srv },
+		&mcp.StreamableHTTPOptions{Stateless: true},
+	)
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(2 * time.Second)
+		handler.ServeHTTP(w, r)
+	}))
+	t.Cleanup(ts.Close)
+
+	reaperDone := make(chan struct{}, 1)
+	timeout := 500 * time.Millisecond
+
+	start := time.Now()
+	tools, cleanup := mcpclient.ConnectAllForTest(ctx, logger,
+		[]database.MCPServerConfig{makeConfig("late", ts.URL)},
+		timeout,
+		func() { reaperDone <- struct{}{} },
+	)
+	elapsed := time.Since(start)
+	t.Cleanup(cleanup)
+
+	require.Less(t, elapsed, 4*timeout,
+		"ConnectAll took %s, budget was %s", elapsed, timeout)
+	require.Empty(t, tools)
+
+	select {
+	case <-reaperDone:
+	case <-time.After(30 * time.Second):
+		t.Fatal("reaper did not exit after the late server responded")
+	}
+}
+
+// TestConnectAll_CleanupPromptWhenServerWedges proves that the
+// cleanup function returns promptly even when a connected session's
+// server has stopped responding, so a wedged server cannot stall
+// the generation loop at step boundaries. The session teardown
+// DELETE is held server-side while cleanup must already have
+// returned.
+func TestConnectAll_CleanupPromptWhenServerWedges(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	srv := mcp.NewServer(&mcp.Implementation{Name: "wedge", Version: "1.0.0"}, nil)
+	tool := echoTool()
+	srv.AddTool(tool.tool, tool.handler)
+	// Stateful handler so closing the session sends a DELETE.
+	handler := mcp.NewStreamableHTTPHandler(
+		func(*http.Request) *mcp.Server { return srv }, nil,
+	)
+
+	var deleteArrived atomic.Bool
+	releaseDelete := make(chan struct{})
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodDelete {
+			deleteArrived.Store(true)
+			select {
+			case <-releaseDelete:
+			case <-r.Context().Done():
+			}
+		}
+		handler.ServeHTTP(w, r)
+	}))
+	t.Cleanup(ts.Close)
+
+	var releaseOnce sync.Once
+	release := func() { releaseOnce.Do(func() { close(releaseDelete) }) }
+	// Registered after ts.Close so it runs before it, letting the
+	// wedged DELETE finish and the session unwind before the
+	// server waits for outstanding requests.
+	t.Cleanup(release)
+
+	cfg := makeConfig("wedge", ts.URL)
+	tools, cleanup := mcpclient.ConnectAll(ctx, logger, []database.MCPServerConfig{cfg}, nil, uuid.Nil, nil, nil)
+	require.Equal(t, []string{"wedge__echo"}, toolNames(tools))
+
+	start := time.Now()
+	cleanup()
+	elapsed := time.Since(start)
+	require.Less(t, elapsed, 1*time.Second,
+		"cleanup took %s with a wedged server", elapsed)
+
+	// The teardown must still happen in the background: the wedged
+	// DELETE arrives even though cleanup already returned.
+	require.Eventually(t, deleteArrived.Load,
+		10*time.Second, 10*time.Millisecond,
+		"session close DELETE never reached the server")
+	release()
+}
+
+// TestConnectAll_NoToolsWedgedCloseWithinBudget proves that a
+// server whose session yields no usable tools cannot stall
+// ConnectAll past the connect budget when its teardown DELETE
+// wedges. The discarded session must be closed off the caller's
+// path, and the teardown must still happen in the background.
+func TestConnectAll_NoToolsWedgedCloseWithinBudget(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
+
+	// Stateful handler with no registered tools so closing the
+	// discarded session sends a DELETE.
+	srv := mcp.NewServer(&mcp.Implementation{Name: "notools", Version: "1.0.0"}, nil)
+	handler := mcp.NewStreamableHTTPHandler(
+		func(*http.Request) *mcp.Server { return srv }, nil,
+	)
+
+	var deleteArrived atomic.Bool
+	releaseDelete := make(chan struct{})
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodDelete {
+			deleteArrived.Store(true)
+			select {
+			case <-releaseDelete:
+			case <-r.Context().Done():
+			}
+		}
+		handler.ServeHTTP(w, r)
+	}))
+	t.Cleanup(ts.Close)
+
+	var releaseOnce sync.Once
+	release := func() { releaseOnce.Do(func() { close(releaseDelete) }) }
+	t.Cleanup(release)
+
+	cfg := makeConfig("notools", ts.URL)
+	start := time.Now()
+	tools, cleanup := mcpclient.ConnectAll(ctx, logger, []database.MCPServerConfig{cfg}, nil, uuid.Nil, nil, nil)
+	elapsed := time.Since(start)
+	t.Cleanup(cleanup)
+
+	require.Empty(t, tools)
+	require.Less(t, elapsed, 5*time.Second,
+		"ConnectAll took %s with a wedged no-tools teardown", elapsed)
+
+	require.Eventually(t, deleteArrived.Load,
+		10*time.Second, 10*time.Millisecond,
+		"discarded session close DELETE never reached the server")
+	release()
+}

coderd/x/chatd/mcpclient/mcphttpclient.go

@@ -2,11 +2,55 @@ package mcpclient
 
 import (
 	"flag"
+	"net"
 	"net/http"
+	"time"
 )
 
+// dialTimeout bounds TCP connection establishment to an MCP
+// server. Without it, a server that silently drops SYNs (for
+// example an edge mitigation black-holing this deployment's
+// egress IPs) blocks the dial until the kernel gives up
+// retransmitting, roughly two minutes on Linux.
+const dialTimeout = 5 * time.Second
+
+// responseHeaderTimeout bounds how long a server may take to send
+// response headers once a request is written. It matches
+// toolCallTimeout rather than connectTimeout because the same
+// client serves tool-call POSTs, and a JSON-response MCP server
+// sends no headers until the tool finishes, so a lower value would
+// kill legitimate slow tools that fit the tool-call budget.
+// Long-lived SSE streams are unaffected; only their headers must
+// arrive within this window. http.Client.Timeout is deliberately
+// unset because it would cap the stream body too.
+const responseHeaderTimeout = toolCallTimeout
+
+// mcpSharedTransport is the transport for all production MCP
+// connections. MCP traffic must not use http.DefaultTransport
+// directly: the default has no dial or response-header bounds, so
+// a black-holed server would hold connections for minutes.
+var mcpSharedTransport = newMCPTransport()
+
+// newMCPTransport clones http.DefaultTransport when possible,
+// preserving proxy and connection-pool settings, and tightens its
+// failure timeouts so an unresponsive MCP server fails in seconds.
+func newMCPTransport() *http.Transport {
+	tr, ok := http.DefaultTransport.(*http.Transport)
+	if ok {
+		tr = tr.Clone()
+	} else {
+		tr = &http.Transport{Proxy: http.ProxyFromEnvironment}
+	}
+	tr.DialContext = (&net.Dialer{
+		Timeout:   dialTimeout,
+		KeepAlive: 30 * time.Second,
+	}).DialContext
+	tr.ResponseHeaderTimeout = responseHeaderTimeout
+	return tr
+}
+
 func httpClientWithHeaders(headers map[string]string) *http.Client {
-	base := http.DefaultTransport
+	var base http.RoundTripper = mcpSharedTransport
 	if isolated := mcpHTTPClient(); isolated != nil {
 		base = isolated.Transport
 	}
@@ -33,20 +77,13 @@ func (h *headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, error
 }
 
 // mcpHTTPClient returns an isolated *http.Client when running
-// inside tests, or nil for production. During tests,
-// httptest.Server.Close() calls
-// http.DefaultTransport.CloseIdleConnections(), which disrupts
-// any MCP client sharing that transport. When DefaultTransport
-// is a *http.Transport it is cloned; otherwise a minimal
-// transport with ProxyFromEnvironment is created as a fallback.
+// inside tests, or nil for production. During tests each client
+// gets a fresh transport so closed httptest servers cannot leave
+// stale pooled connections behind for later tests that reuse the
+// same address.
 func mcpHTTPClient() *http.Client {
 	if flag.Lookup("test.v") == nil {
 		return nil
 	}
-	if dt, ok := http.DefaultTransport.(*http.Transport); ok {
-		return &http.Client{Transport: dt.Clone()}
-	}
-	return &http.Client{Transport: &http.Transport{
-		Proxy: http.ProxyFromEnvironment,
-	}}
+	return &http.Client{Transport: newMCPTransport()}
 }

coderd/x/chatd/mcpclient/mcphttpclient_internal_test.go

@@ -0,0 +1,27 @@
+package mcpclient
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestMCPTransportTimeouts guards the transport hardening: MCP
+// traffic must never ride a transport without dial and
+// response-header bounds, or a black-holed server holds
+// connections until the kernel gives up (about two minutes).
+func TestMCPTransportTimeouts(t *testing.T) {
+	t.Parallel()
+
+	shared := mcpSharedTransport
+	require.NotNil(t, shared.DialContext)
+	require.Equal(t, responseHeaderTimeout, shared.ResponseHeaderTimeout)
+	// The response-header bound must not undercut the tool-call
+	// budget, or slow JSON-response tools within budget would be
+	// killed at the HTTP layer.
+	require.GreaterOrEqual(t, responseHeaderTimeout, toolCallTimeout)
+
+	isolated := mcpHTTPClient()
+	require.NotNil(t, isolated, "must be isolated under test")
+	require.NotSame(t, shared, isolated.Transport)
+}