streamable client: Connect blocks far past its context deadline against an unresponsive server (detached-context cleanup)

[ibetitsmike]

What

Client.Connect with a context deadline can block far past that deadline against a server that accepts TCP connections but never responds (black-holed peer). Observed: a 2s deadline returns after ~12s with the default HTTP client in a local test; in production, with connections crossing a NAT that silently dropped packets, a 10s deadline stretched to ~125s (kernel TCP timeout).

The caller's deadline bounds the initialize request itself, but not the error-path cleanup that follows. StreamableClientTransport.Connect detaches the incoming context for the connection's lifecycle (connCtx, cancel := context.WithCancel(xcontext.Detach(ctx)), mcp/streamable.go ~L2045 in v1.7.0). That detachment is intentional and documented for keeping the standalone SSE stream alive past a short connect deadline. However, when initialize fails at the deadline, the cleanup work that runs before Connect returns (the notifications/cancelled write for the abandoned call and session.Close()) issues HTTP requests bound to the detached connection context, not to the caller's expired one. Against a black-holed server those requests block until the HTTP transport or the kernel gives up, so Connect overshoots its deadline by minutes with a default http.Client (no ResponseHeaderTimeout).

Related: #1183 covers the same detached-context ordering problem for Close() specifically. This report is about Connect not honoring its own deadline on the error path.

Repro

func TestConnectOvershootsDeadline(t *testing.T) {
	// Accepts TCP connections, never responds to any request.
	ln, err := net.Listen("tcp", "127.0.0.1:0")
	if err != nil {
		t.Fatal(err)
	}
	defer ln.Close()
	go func() {
		for {
			conn, err := ln.Accept()
			if err != nil {
				return
			}
			defer conn.Close()
		}
	}()

	client := mcp.NewClient(&mcp.Implementation{Name: "c", Version: "0"}, nil)
	tr := &mcp.StreamableClientTransport{Endpoint: "http://" + ln.Addr().String()}
	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	defer cancel()

	start := time.Now()
	_, err = client.Connect(ctx, tr, nil)
	elapsed := time.Since(start)
	t.Logf("Connect returned after %s, err=%v", elapsed, err)
	if elapsed > 4*time.Second {
		t.Fatalf("Connect blocked %s past a 2s deadline", elapsed)
	}
}

With go-sdk v1.7.0 and Go 1.24 this fails: Connect returned after 12.03s, err=context deadline exceeded; sending "notifications/cancelled": rejected by transport: Post "http://127.0.0.1:...": context deadline exceeded.

Expected

Connect(ctx, ...) returns within (approximately) ctx's deadline regardless of server behavior. Error-path cleanup for a session that never initialized could be best-effort/asynchronous, or bounded by the caller's context rather than the detached connection context.

Workaround

We now run Connect+ListTools in a goroutine and select on the deadline, abandoning the goroutine on timeout, plus a cloned http.Transport with ResponseHeaderTimeout and a short dial timeout (coder/coder#28400).

🤖 Filed by Mux (AI agent) on behalf of @ibetitsmike.

[guglielmo-san]

@ibetitsmike thank you for the report, I think this issue has been recently addressed.
PR #1151 (mcp: allow asynchronous cancel notification, already on main) reworked that: conn.Retire runs synchronously and the cancellation notify is spawned in a goroutine. Because Retire removes the call from outgoingCalls before cs.Close() runs, Close()'s updateInFlight typically sets connClosing = true while the state is already idle, closes the underlying conn, and returns.
Could you re-run the repro against main (or a build that includes #1151) and check the elapsed time?

[Haaaarry]

Confirmation data on whether this is fixed on main, as requested: not fully.

Ran the reporter's black-hole repro test on current main (2732839, includes #1151 and #1190), Go 1.25.5, -count=3: Connect with a 2s deadline returns after 7.00s, 7.00s, 7.00s — very stable, still overshooting the deadline ~3.5x (v1.7.0 overshot to ~12s).

Two experiments to localize what's left:

1. MCPGODEBUG=blockingcancelnotify=1 restores exactly the v1.7.0 behavior (12.00s = deadline + 2×5s). This confirms mcp: allow asynchronous cancel notification #1151 moved one 5s block (the call-path notifications/cancelled in call()) off the return path, but a second ~5s synchronous step remains in Connect's error path.
2. Ruled out the session-termination DELETE: patched closeDeleteTimeout down to 1s locally and the total stayed 7.00s — consistent with Close() skipping the DELETE when no session ID was ever established.

So the remaining overshoot is a ~5s synchronous step in the handshake error path (after the caller's ctx expires, before Connect returns), not the Close() cleanup. Happy to keep digging toward a specific line if that would help.

[omkar619-dev]

Ran the repro against main — go-sdk v1.7.1-0.20260828140610-21c18c6229e1, Go 1.25.

default: Connect returned after 7.0007s (2s deadline) MCPGODEBUG=blockingcancelnotify=1: Connect returned after 12.0016s

So it's improved but not fixed i believe . The 5.000s delta between those two runs is exactly notifyCancellationTimeout, which confirms #1151 is doing its job — the cancel-notify no longer blocks the return. The blocking run also surfaces sending "notifications/cancelled": rejected by transport: Post ... which is absent from the default run. That leaves the remaining 5s as closeDeleteTimeout, so the breakdown on
main is 2s deadline + 5s session-teardown DELETE.

Btw I think the thread was missing half the picture. #1189 describes two blocking cleanup steps and #1151 only covers one: it made the notifications/cancelled write asynchronous in transport.go (conn.Retire, then a goroutine with context.WithoutCancel +
notifyCancellationTimeout). The session.Close() DELETE stayed on the connection context until #1184 gave it closeDeleteTimeout = 5s, and that only merged on the 21st. Both halves are on main now but arrived through separate PRs under separate issues. The original report
was against v1.7.0, released 28 Jul, which predates #1151 by ten days — and its 12.03s matches today's blocking-notify figure, which fits.

The bound itself looks right. What's left is that Connect's error path still waits on session teardown before returning, rather than that teardown being bounded by the caller's context or moved off the return path. So Connect still can't honour its own deadline, it just overshoots by 5s now instead of by minutes.

Also worth flagging: nothing on main asserts the end-to-end invariant. #1184 added TestStreamableClientCloseUnresponsiveDelete for the Close path and #1151 added TestCallCancellation_FastReturn for the call path, but there's no test that Connect returns within its own deadline against an unresponsive peer. Since the detach at streamable.go ~L2045 is unchanged, any cleanup step added to that error path later would reintroduce this silently.

Happy to send a PR — the repro above as a regression test, plus whichever disposition you prefer for the teardown wait.