fix(agent/x/agentmcp): disable standalone SSE for HTTP MCP servers (#30484)

tl;dr is this should fix the storybook mcp server in Coder Agents. the
storybook mcp never responds to the SSE GET, so it never connects, and
reload fails. this disables that SSE GET and should let storybook
reconnect on config reload

---

HTTP workspace MCP servers whose `/mcp` endpoint never answers the
standalone SSE GET (for example, the Storybook dev server) hang the
agent's MCP reload indefinitely. The go-sdk opens that GET inside
`Connect` on a context the agent's 30s connect timeout does not bound,
and because reloads are singleflighted, every later `.mcp.json` change
waits behind the hung one. The server never shows up as connected or
failed, and no MCP config change is picked up until the agent restarts.

Set `DisableStandaloneSSE: true` on the streamable HTTP transport. The
agent's client registers no handlers for server-initiated messages, so
the stream is unused; tool call responses and in-request notifications
still arrive on the POST.

<details>
<summary>Investigation and decision log</summary>

**Symptom:** a `storybook` HTTP server in `.mcp.json` never produced
`storybook__*` tools in chats, while stdio servers worked.

**Findings:**

- Agent log showed the startup connect failing with `connection refused`
(Storybook was not up yet), and a later reload failing with `standalone
SSE request failed (session ID: ...)`, meaning `initialize` had
succeeded and the GET stream was the failure point.
- After touching `.mcp.json` with Storybook running, no reload result
was ever logged (success or warning).
- A standalone probe using go-sdk v1.8.0 and the same transport config
hung in `Connect` past its 30s timeout. Goroutine dump: `Client.Connect
-> sessionUpdated -> connectStandaloneSSE -> connectSSE`, which uses the
connection's lifetime context, not the connect context.
- `curl` against Storybook: `POST initialize` returns a session ID; `GET
/mcp` with that session sends no response headers at all. The spec
requires either `text/event-stream` or 405, so Storybook is
non-compliant, but the agent should not wedge on it.
- With `DisableStandaloneSSE: true`, the same probe connected in ~7ms
and listed all 8 Storybook tools.

**Why disable instead of alternatives:**

- `mcp.NewClient(..., nil)` in `manager.go` registers no
`tools/list_changed`, sampling, elicitation, roots, or logging handlers,
so nothing on the standalone stream is consumed today.
- The spec makes the standalone GET optional for clients; responses to
client requests are never sent on it.
- Bounding `Connect` with our own timeout would require abandoning a
stuck goroutine and session per bad server, since the hang is inside the
SDK call on a context we do not control.
- Tradeoff: live `tools/list_changed` refresh would need the stream
back, opened after `Connect` and off the reload path.

**Out of scope, noted for follow-up:**

- A server that is down when the agent first connects is not retried
until `.mcp.json` changes.
- A server that restarts invalidates the session ID, so the next POST
fails and nothing reconnects the session. Disabling the standalone
stream does not change this.

</details>

---

Generated by Coder Agents on behalf of @jeremyruppel.
This commit is contained in:
Jeremy Ruppel
2026-10-12 16:01:23 -04:00
committed by GitHub
parent ca7d7afea9
commit c4fabe90ae
2 changed files with 67 additions and 0 deletions
+6
View File
@@ -915,6 +915,12 @@ func (m *Manager) createTransport(ctx context.Context, cfg ServerConfig) (mcp.Tr
return &mcp.StreamableClientTransport{
Endpoint: cfg.URL,
HTTPClient: httpClientWithHeaders(cfg.Headers),
// The SDK opens the standalone GET inside Connect on a context
// connectTimeout does not cancel, so a server that never answers
// it hangs Connect, and every later reload, indefinitely. This
// also means server-initiated handlers on the client never fire
// over HTTP.
DisableStandaloneSSE: true,
}, nil
case "sse":
return &mcp.SSEClientTransport{
+61
View File
@@ -6,6 +6,8 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
@@ -272,6 +274,65 @@ func TestConnectServer_StdioProcessSurvivesConnect(t *testing.T) {
assert.Equal(t, "echo", result.Tools[0].Name)
}
// TestConnectServer_HTTPConnectsWhenStandaloneSSEHangs guards against the
// SDK's standalone GET, which connectTimeout does not bound, hanging
// Connect. See DisableStandaloneSSE in createTransport. The GET is only
// sent when the negotiated protocol is below 2026-07-28, so this test
// stops exercising it if an SDK upgrade negotiates that version or later.
func TestConnectServer_HTTPConnectsWhenStandaloneSSEHangs(t *testing.T) {
t.Parallel()
server := mcp.NewServer(&mcp.Implementation{Name: "fake", Version: "1.0.0"}, nil)
server.AddTool(&mcp.Tool{
Name: "echo",
InputSchema: map[string]any{"type": "object"},
}, func(context.Context, *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
return &mcp.CallToolResult{}, nil
})
handler := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return server }, nil)
release := make(chan struct{})
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == http.MethodGet {
select {
case <-release:
case <-r.Context().Done():
}
return
}
handler.ServeHTTP(w, r)
}))
t.Cleanup(srv.Close)
t.Cleanup(func() { close(release) })
ctx := testutil.Context(t, testutil.WaitShort)
m := NewManager(ctx, slogtest.Make(t, nil), agentexec.DefaultExecer, nil, nil, nil, nil)
t.Cleanup(func() { _ = m.Close() })
type connectResult struct {
client *mcp.ClientSession
err error
}
done := make(chan connectResult, 1)
go func() {
client, err := m.connectServer(ctx, ServerConfig{
Name: "fake",
Transport: "http",
URL: srv.URL,
})
done <- connectResult{client: client, err: err}
}()
res := testutil.RequireReceive(ctx, t, done)
require.NoError(t, res.err)
t.Cleanup(func() { _ = res.client.Close() })
result, err := res.client.ListTools(ctx, nil)
require.NoError(t, err)
require.Len(t, result.Tools, 1)
assert.Equal(t, "echo", result.Tools[0].Name)
}
func TestManager_WaitReloadTimeout(t *testing.T) {
t.Parallel()