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