mcp: best-effort cancellation notification blocks the caller's return past its deadline

[inhuman]

Summary

call() awaits cancelCall() before returning, so a cancelled call does not return to its caller until the best-effort notifications/cancelled message has been delivered — or has itself timed out. On v1.7.0 that means a caller's context deadline can be overrun by up to notifyCancellationTimeout (5 s). Before #885 the notification context carried no deadline at all and the overrun was unbounded.

go-sdk/mcp/transport.go

Lines 271 to 309 in 958dfcc

	func call(ctx context.Context, conn *jsonrpc2.Connection, method string, params Params, result Result) error {
	// The "%w"s in this function expose jsonrpc.Error as part of the API.
	call := conn.Call(ctx, method, params)
	err := call.Await(ctx, result)
	switch {
	case errors.Is(err, jsonrpc2.ErrClientClosing), errors.Is(err, jsonrpc2.ErrServerClosing):
	return fmt.Errorf("%w: calling %q: %v", ErrConnectionClosed, method, err)
	case ctx.Err() != nil:
	err := cancelCall(ctx, conn, call)
	return errors.Join(ctx.Err(), err)
	case err != nil:
	return fmt.Errorf("calling %q: %w", method, err)
	}
	return nil
	}
	// cancelCall sends a "notifications/cancelled" notification for call and eagerly
	// retires it from conn.
	//
	// By default, the jsonrpc2 library waits for graceful shutdown when the
	// connection is closed, meaning it expects all outgoing and incoming requests
	// to complete. However, for MCP this expectation is unrealistic, and can lead
	// to hanging shutdown. For example, if a streamable client is killed, the
	// server will not be able to detect this event, except via keepalive pings (if
	// they are configured), and so outgoing calls may hang indefinitely.
	//
	// Therefore, we choose to eagerly retire calls, removing them from the
	// outgoingCalls map, when the caller context is cancelled: if the caller will
	// never receive the response, there's no need to track it.
	func cancelCall(ctx context.Context, conn *jsonrpc2.Connection, call *jsonrpc2.AsyncCall) error {
	notifyCtx, cancelNotify := context.WithTimeout(context.WithoutCancel(ctx), notifyCancellationTimeout)
	defer cancelNotify()
	err := conn.Notify(notifyCtx, notificationCancelled, &CancelledParams{
	Reason: ctx.Err().Error(),
	RequestID: call.ID().Raw(),
	})
	conn.Retire(call, ctx.Err())
	return err
	}

The invariant is already written down in this file, one constant above the code that breaks it:

// notifyCancellationTimeout bounds the cancellation notification we send to
// the peer when the caller's context is cancelled. The notification is
// best-effort: a degraded connection (e.g. an OAuth flow that has been
// abandoned) must not be able to block the caller's return path or
// re-trigger expensive recovery on its behalf. See issue #882.
const notifyCancellationTimeout = 5 * time.Second

The bound makes the blocking finite. It does not stop the notification from blocking the caller's return path — the code still does precisely what the comment forbids, just for no longer than five seconds.

Why this is a defect and not a tuning question

The message being delivered is "I have changed my mind about this request". It is sent synchronously, on the critical path of the caller who has already changed their mind. That caller's deadline is a promise it made to its own caller; spending that budget on a courtesy message to an unresponsive peer breaks the promise. And there is nothing to wait for: a best-effort notification produces no result the caller can act on. Today its error is joined into the returned error, where it is pure noise — see the reproduction output below.

Two things are wrong here, and only the second one is the bug:

1. the notification's context had no deadline whatsoever — best-effort delivery with infinite patience (addressed by mcp: do not re-prompt OAuth after cancelled Authorize #885, now 5 s);
2. the caller's return is gated on that delivery (still present).

(1) is what made (2) spectacular. (2) is what is actually wrong. Stated positively: a best-effort notification must not block the caller's return, and must carry a bounded context. Half of that now holds.

Reproduction

go-sdk v1.7.0, Go 1.26, linux. A Streamable HTTP server that is slow to accept the cancellation POST — which is the normal case, since it is by construction busy with the very request being cancelled:

// delay any POST carrying notifications/cancelled by d
func stallCancelNotify(h http.Handler, d time.Duration) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if r.Method == http.MethodPost && r.Body != nil {
			body, _ := io.ReadAll(r.Body)
			r.Body = io.NopCloser(bytes.NewReader(body))
			if bytes.Contains(body, []byte("notifications/cancelled")) {
				time.Sleep(d)
			}
		}
		h.ServeHTTP(w, r)
	})
}

func main() {
	srv := mcp.NewServer(&mcp.Implementation{Name: "repro", Version: "v0"}, nil)
	mcp.AddTool(srv, &mcp.Tool{Name: "slow"},
		func(ctx context.Context, _ *mcp.CallToolRequest, _ struct{}) (*mcp.CallToolResult, struct{}, error) {
			select {
			case <-ctx.Done():
			case <-time.After(60 * time.Second):
			}
			return &mcp.CallToolResult{}, struct{}{}, nil
		})

	handler := mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return srv }, nil)
	ts := httptest.NewServer(stallCancelNotify(handler, 20*time.Second))
	defer ts.Close()

	client := mcp.NewClient(&mcp.Implementation{Name: "repro-client", Version: "v0"}, nil)
	cs, _ := client.Connect(context.Background(), &mcp.StreamableClientTransport{Endpoint: ts.URL}, nil)
	defer cs.Close()

	callCtx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
	defer cancel()

	start := time.Now()
	_, err := cs.CallTool(callCtx, &mcp.CallToolParams{Name: "slow"})
	fmt.Printf("returned after %s\nerr: %v\n", time.Since(start).Round(time.Millisecond), err)
}

Output:

caller deadline: 200ms
  [server] holding notifications/cancelled for 20s
CallTool returned after 5.202s (overshoot 5.002s)
err: context deadline exceeded
sending "notifications/cancelled": rejected by transport: Post "http://127.0.0.1:35295": context deadline exceeded

The overshoot is exactly notifyCancellationTimeout, and the caller's error now carries a second line about a delivery it never asked to wait for.

For scale, the same mechanism on v0.6.0 (before the bound existed): a declared 30 s tool-call limit was measured returning at 155.06–155.08 s in production against a busy Streamable HTTP MCP server — repeatedly, with the same figure to two decimal places.

Related: the eager retire is delayed by the very thing it protects against

cancelCall's doc comment justifies eager retirement as the mitigation for hanging shutdown when a peer cannot be reached. But conn.Retire(call, ctx.Err()) runs after the notify, so the eager retire is itself delayed by up to 5 s by exactly the unresponsive peer it was introduced to handle.

Suggested direction

Retire first, deliver off the caller's path. The pattern already exists a few lines above, in callSubscriptionsListen:

go func() {
	<-ctx.Done()
	_ = cancelCall(ctx, conn, call)
}()

Applied to call(), roughly:

case ctx.Err() != nil:
	conn.Retire(call, ctx.Err())
	go func() {
		notifyCtx, stop := context.WithTimeout(context.WithoutCancel(ctx), notifyCancellationTimeout)
		defer stop()
		_ = conn.Notify(notifyCtx, notificationCancelled, &CancelledParams{...})
	}()
	return ctx.Err()

with the notify error logged rather than joined into the caller's error. context.WithoutCancel(ctx) still preserves values (tracing, auth) without preserving a dead deadline, exactly as #885 established — the bounded context stays; only the waiting goes away. If some callers genuinely want to wait for delivery, that reads better as an explicit option than as the default for everyone.

Environment

• github.com/modelcontextprotocol/go-sdk v1.7.0; same code path on main @ 958dfcc
• Go 1.26, linux
• Originally observed through github.com/inhuman/mcp-multiplexer on go-sdk v0.6.0

Workaround, for anyone else hitting this

Race the SDK call against your own ctx.Done() and return on your own deadline. The price is an orphaned goroutine holding one connection until the SDK unwinds, so it is worth counting those rather than hiding them.

[guglielmo-san]

@inhuman thank you for reporting this issue, PR #1151 should have fixed that