From 52bf4fafd81b665b5e517688b571cba4a8d4797a Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Fri, 17 Jul 2026 15:44:29 +0000 Subject: [PATCH] fix(mcp): bound MCP session connect so an unreachable server can't hang the widget (#10880) Establishing an MCP session held the session-cache mutex across client.Connect with no per-connect timeout. An unreachable remote server (bounded only by the 360s httpClient timeout) or a stdio server whose initialize handshake never completes therefore blocked the caller and, because the mutex was held, every other MCP request for that model too. In the UI this shows up as the MCP "Servers" widget spinning forever. It is most visible for cloud-proxy models: their chat path bails out before the MCP tool block, so it never warms the session cache in the background. The widget's /v1/mcp/servers/ call is then the first and only code that connects synchronously, in the request foreground. The session, once established, stays bound to the shared context (it is cancelled later via the cached cancel func on eviction/shutdown), so we can't pass a WithTimeout context to Connect: firing the timeout would tear a healthy session down, and cancelling the shared context would also kill sibling servers that already connected. Instead connectMCP runs Connect on the shared context in a goroutine and stops waiting after the discovery timeout, returning an error for that one server without disturbing the others. A stalled goroutine is reaped when the model's sessions are cancelled. Applied to both SessionsFromMCPConfig and NamedSessionsFromMCPConfig. Signed-off-by: Ettore Di Giacinto Assisted-by: Claude:opus-4.8 [Claude Code] --- .../endpoints/mcp/connect_timeout_test.go | 42 +++++++++++++++++ core/http/endpoints/mcp/tools.go | 46 +++++++++++++++++-- 2 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 core/http/endpoints/mcp/connect_timeout_test.go diff --git a/core/http/endpoints/mcp/connect_timeout_test.go b/core/http/endpoints/mcp/connect_timeout_test.go new file mode 100644 index 000000000000..9bb0eb8adb70 --- /dev/null +++ b/core/http/endpoints/mcp/connect_timeout_test.go @@ -0,0 +1,42 @@ +package mcp + +import ( + "context" + "os/exec" + "time" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("connectMCP", func() { + // A stdio server that starts but never completes the MCP initialize + // handshake models the real-world hang from mudler/LocalAI#10880: an + // unreachable/misbehaving MCP server would otherwise block the caller (and, + // because the session-cache mutex is held across connection setup, every + // other MCP request for the model) until the 360s httpClient timeout, which + // surfaces in the UI as the server widget "spinning forever". + It("returns promptly with an error when the handshake never completes", func() { + // `sleep` stays alive but never reads its stdin nor emits an MCP + // initialize response, so client.Connect blocks on the handshake. + // (`cat` would echo the request back and the SDK would treat it as a + // bogus response, returning immediately instead of hanging.) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() // reap the abandoned goroutine/subprocess after the test + + transport := &mcp.CommandTransport{Command: exec.CommandContext(ctx, "sleep", "60")} + + start := time.Now() + session, err := connectMCP(ctx, transport, 200*time.Millisecond) + elapsed := time.Since(start) + + Expect(err).To(HaveOccurred()) + Expect(session).To(BeNil()) + Expect(err.Error()).To(ContainSubstring("timed out")) + // It must return around the timeout, not hang until the 360s httpClient + // timeout. Allow generous slack for slow CI. + Expect(elapsed).To(BeNumerically("<", 5*time.Second)) + }) +}) diff --git a/core/http/endpoints/mcp/tools.go b/core/http/endpoints/mcp/tools.go index e3f2b3944fd1..af4ebe197d71 100644 --- a/core/http/endpoints/mcp/tools.go +++ b/core/http/endpoints/mcp/tools.go @@ -137,6 +137,44 @@ func MCPServersFromMetadata(metadata map[string]string) []string { return servers } +// connectMCP performs the MCP initialize handshake with a bounded timeout. +// +// Without this bound, an unreachable remote server (capped only by the 360s +// httpClient timeout) or a stdio server whose handshake never completes blocks +// the caller indefinitely. Because the session cache mutex is held across +// connection setup, one stalled server also wedges every other MCP request for +// the same model, which surfaces in the UI as the server widget "spinning +// forever" (mudler/LocalAI#10880) - most visibly for cloud-proxy models, whose +// chat path never warms the session cache in the background. +// +// The session, once established, stays bound to the shared ctx (it is cancelled +// later via the cached cancel func on eviction/shutdown), so we cannot pass a +// WithTimeout context to Connect: firing the timeout would tear a healthy +// session down. Instead we run Connect on the shared ctx in a goroutine and stop +// waiting after the timeout. We deliberately do NOT cancel here - the ctx is +// shared with sibling servers that may already have connected. A genuinely +// stalled goroutine holds only that shared ctx and is reaped when the model's +// sessions are cancelled on eviction/shutdown. +func connectMCP(ctx context.Context, transport mcp.Transport, timeout time.Duration) (*mcp.ClientSession, error) { + type result struct { + session *mcp.ClientSession + err error + } + // Buffered so the goroutine can always send and exit, even after we stop + // waiting on the timeout branch. + done := make(chan result, 1) + go func() { + s, err := client.Connect(ctx, transport, nil) + done <- result{session: s, err: err} + }() + select { + case r := <-done: + return r.session, r.err + case <-time.After(timeout): + return nil, fmt.Errorf("timed out after %s establishing MCP session (server unreachable?)", timeout) + } +} + func SessionsFromMCPConfig( name string, remote config.MCPGenericConfig[config.MCPRemoteServers], @@ -187,7 +225,7 @@ func SessionsFromMCPConfig( ) transport := &mcp.StreamableClientTransport{Endpoint: server.URL, HTTPClient: httpClient} - mcpSession, err := client.Connect(ctx, transport, nil) + mcpSession, err := connectMCP(ctx, transport, config.DefaultMCPDiscoveryTimeout) if err != nil { xlog.Error("Failed to connect to MCP server", "error", err, "url", server.URL) continue @@ -205,7 +243,7 @@ func SessionsFromMCPConfig( command.Env = append(command.Env, key+"="+value) } transport := &mcp.CommandTransport{Command: command} - mcpSession, err := client.Connect(ctx, transport, nil) + mcpSession, err := connectMCP(ctx, transport, config.DefaultMCPDiscoveryTimeout) if err != nil { xlog.Error("Failed to start MCP server", "error", err, "command", command) continue @@ -269,7 +307,7 @@ func NamedSessionsFromMCPConfig( ) transport := &mcp.StreamableClientTransport{Endpoint: server.URL, HTTPClient: httpClient} - mcpSession, err := client.Connect(ctx, transport, nil) + mcpSession, err := connectMCP(ctx, transport, config.DefaultMCPDiscoveryTimeout) if err != nil { xlog.Error("Failed to connect to MCP server", "error", err, "name", serverName, "url", server.URL) continue @@ -290,7 +328,7 @@ func NamedSessionsFromMCPConfig( command.Env = append(command.Env, key+"="+value) } transport := &mcp.CommandTransport{Command: command} - mcpSession, err := client.Connect(ctx, transport, nil) + mcpSession, err := connectMCP(ctx, transport, config.DefaultMCPDiscoveryTimeout) if err != nil { xlog.Error("Failed to start MCP server", "error", err, "name", serverName, "command", command) continue