Skip to content

fix(chat): keep MCP server connections alive across their lifetime#42

Merged
mudler merged 1 commit into
masterfrom
fix/mcp-connect-context-cancel
Jul 1, 2026
Merged

fix(chat): keep MCP server connections alive across their lifetime#42
mudler merged 1 commit into
masterfrom
fix/mcp-connect-context-cancel

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

Summary

  • ReconcileMCPServers connected user-configured MCP servers with a 30s-timeout context and called cancel() immediately after Connect() returned.
  • The go-sdk's mcp.Client.Connect stores the context it's given for the connection's entire lifetime, not just the initial handshake — StreamableClientTransport.Connect derives a cancellable context from it to drive the background "hanging GET" SSE listener and reconnect machinery.
  • Cancelling that context right after connect tore down the background SSE listener, so a later tool call on an otherwise-connected, healthy server that needed to re-establish its SSE stream failed with the real, user-reported error chain:
    connection closed: calling "tools/call": client is closing:
    hanging GET: failed to reconnect (session ID: ...):
    connection failed after 5 attempts: Get "https://<server>":
    context canceled
    
  • Fix: hand Connect the session's own long-lived context (s.ctx) directly, with no per-connect timeout — mirroring the built-in host-tool connect path in NewSession, which has never exhibited this bug. Deleting only the cancel() call would not have been sufficient: the WithTimeout wrapper itself would still auto-cancel the connection 30s after connecting.
  • Found via live debugging of a real nib-desktop user report, after two earlier, legitimate fixes (transport selection, built-in-tools allowlist scoping — this repo's PR fix(chat): built-in tools allowlist no longer filters user-configured MCP servers #41) resolved the symptom of no tools showing up, but this context-lifetime bug is what actually broke tool calls on an already-connected server.

Testing

  • Added TestReconcileMCPServersKeepsConnectionAliveForToolCalls (chat/session_reload_test.go): stands up a real streamable-HTTP MCP server and asserts the background hanging-GET SSE stream is not torn down once ReconcileMCPServers returns. Confirmed it fails on the pre-fix code and passes on the fix.
  • go build ./..., go vet ./..., go test ./... — 100% green across all packages.

🤖 Generated with Claude Code

ReconcileMCPServers connected config MCP servers with a 30s-timeout
context and called cancel() immediately after Connect() returned. The
go-sdk's mcp.Client.Connect stores the context it is given for the
connection's ENTIRE lifetime (StreamableClientTransport.Connect derives
a cancellable context from it to drive the background "hanging GET" SSE
listener and reconnect machinery), not just the initial handshake.

Cancelling that context right after connect tore down the background SSE
listener, so a later tool call on an otherwise-connected, healthy server
that needed to re-establish its SSE stream failed with the real,
user-reported error chain:

  connection closed: calling "tools/call": client is closing:
  hanging GET: failed to reconnect (session ID: ...):
  connection failed after 5 attempts: Get "https://<server>":
  context canceled

Fix: hand Connect the session's own long-lived context (s.ctx) directly
with no per-connect timeout, mirroring the built-in host-tool connect
path in NewSession, which has never exhibited this bug. Deleting only the
cancel() call would not suffice: the WithTimeout wrapper would itself
auto-cancel the connection 30s after connecting.

Adds a regression test that stands up a real streamable-HTTP MCP server
and asserts the background hanging-GET SSE stream is not torn down once
ReconcileMCPServers returns (it fails on the pre-fix code, passes now).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mudler mudler merged commit 74135fa into master Jul 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants