mcpcompat/server: refuse dispatch of a request whose per-request context is gone

オープン
#414 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

@jhrozek がすでに取り組んでいます。

2026年10月26日 から。

  • #415 @jhrozek による — オープン

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
54/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
go
領域
backend

調査の方向性

Start in mcpcompat/server/server.go at sessionDispatchMiddleware and trace how it gets the request nonce and identifies notifications. Read the pending context handling in mcpcompat/server/transports.go, then use the existing context bridge test and rehydration_test.go to build coverage for queued, abandoned requests and notifications. Done means missing-context requests are refused before hooks on local and rehydrated sessions, while notifications and messages without a nonce retain their current behavior.

索引モデルが issue の本文から書いたものです。

説明

needs-triage

Summary

When a client disconnects after its POST is queued but before the message is dispatched, mcpcompat/server still dispatches it, on the session's base context instead of the request's own context. Refuse that dispatch instead.

This issue is written so it can be picked up as a self-contained task. Line numbers are against main at 79572a9.

Current behaviour

  1. StreamableHTTPServer.ServeHTTP stores the POST's request context under a random nonce and stamps it on the request (mcpcompat/server/transports.go:487-489), then clears it when ServeHTTP returns (transports.go:502, defer s.mcp.clearPendingRequestContext(nonce)).

  2. go-sdk (v1.8.0, mcp/streamable.go, servePOST) puts the message on the session's incoming queue and returns as soon as the client goes away; it does not drop queued messages when the request context is cancelled. go-sdk handles everything except initialize and notifications asynchronously (mcp/server.go around line 2000), so dispatch can run after ServeHTTP has returned.

  3. At dispatch, sessionDispatchMiddleware (mcpcompat/server/server.go:821) bridges the stored context only if the entry still exists (server.go:838-844). If it is gone, it silently keeps the session's base context:

    if re := req.GetExtra(); re != nil {
        if nonce := re.Header.Get(reqNonceHeader); nonce != "" {
            if reqCtx := s.pendingRequestContext(nonce); reqCtx != nil {
                ctx = &valueBridgeContext{Context: ctx, values: reqCtx}
            }
        }
    }
    
  4. The base context belongs to a different request: the initialize POST for a session created on this replica (go-sdk connectStreamable(req.Context(), ...)), or the first request that reached this replica for a rehydrated session (transports.go:773, srv.Connect(context.WithoutCancel(r.Context()), ...)). The before-hooks (fireBeforeHooks, server.go:850) and the handler then run with that other request's context values (caller identity, audit and telemetry attributes), for a call whose client is no longer there to receive the result.

The existing comment at transports.go:490-501 documents the notification case of this as a known limitation; it does not cover requests.

Proposed change

In sessionDispatchMiddleware, when a message carries X-MCP-Req-Nonce and pendingRequestContext(nonce) returns nil, and the message is a JSON-RPC request (not a notification), return an error instead of continuing. Do this before fireBeforeHooks, so neither hooks nor the handler run.

Details to get right:

  • Notifications keep today's behaviour. Notification-only POSTs are acknowledged with 202 before dispatch, so they routinely lose the entry (the limitation at transports.go:490-501). Refusing them would drop notifications/initialized and break every session. Decide request vs notification from the method/request type available in the middleware (for example the notifications/ method prefix, or go-sdk's request type), and add a test that notifications/initialized still works.
  • No nonce means no change. Messages without the header (anything not coming through StreamableHTTPServer.ServeHTTP, for example in-process or stdio use) must behave exactly as today.
  • The nonce cannot be spoofed. ServeHTTP always overwrites the header with crand.Text() (transports.go:487-489), so "nonce present, entry missing" reliably means the request was abandoned. Keep it that way.
  • Stateless mode. ServeHTTP short-circuits to s.handler for stateless servers after storing the nonce (transports.go:503-519). Check whether a stateless request can be dispatched after its ServeHTTP returns; if it can, the same refusal applies; if it cannot, note why in a comment.
  • Rehydrated sessions. Requests on a rehydrated session go through serveRehydrated with the same nonce bridge. The refusal must apply there too; see rehydration_test.go for the harness.
  • What to return. The client is gone, so the response is not delivered; the point is that the handler does not run. Return a JSON-RPC error (an internal error with a short message such as "request context no longer available") so go-sdk completes the request cleanly. Log at DEBUG, not WARN: ordinary client cancellations trigger this at request rate.
  • Spec note. This treats a disconnect before dispatch as cancellation, which differs from the MCP 2025-11-25 transport note that disconnection SHOULD NOT be interpreted as cancellation. State this deliberate deviation in a comment at the change.
  • Update the comments at transports.go:490-501 and server.go:829-837 so they describe the new behaviour for requests.

Tests

Add to mcpcompat/server (the bridge is already covered by TestPerRequestContext_NotClobberedConcurrently, server_test.go:~258, which is a good starting point for the harness):

  1. Abandoned request is refused. The ordering has to be forced, and a slow tool call will not do it: go-sdk handles calls asynchronously, so they do not hold the session queue. Only initialize and notifications are handled synchronously. A recipe that works (it is how the equivalent toolhive test does it):
    • Register an OnRegisterSession hook (hooks.go:29-30, WithHooks, server.go:409) that records the session ID and then blocks on a channel. The hook runs inside the initialize dispatch, so while it blocks, every later message on that session waits in the queue.
    • Send initialize in a goroutine and wait until the hook has recorded the session ID.
    • Send a tools/call with that Mcp-Session-Id by calling the server's ServeHTTP in-process (httptest.NewRecorder plus a request built with httptest.NewRequest(...).WithContext(ctx) where ctx is already cancelled). go-sdk queues the message and ServeHTTP returns at once, which clears the nonce entry.
    • Release the hook, let initialize finish, and assert the tool handler and the before-hooks never ran for that call (use channels or counters, with a bounded wait; no bare sleeps). Also assert a positive signal that the dispatch did happen and was refused, for example the new DEBUG log line captured with a test slog handler or a counter, so the test cannot pass just because the message was never dispatched.
  2. Session still works afterwards. A normal tools/call on the same session succeeds.
  3. Notifications unaffected. notifications/initialized (and a notification sent with an already-cancelled context) is still processed as today.
  4. Rehydrated session. The same refusal on a session served by serveRehydrated.
  5. Mutation check. Removing the new branch makes test 1 fail.

Run task test (race detector on) and task lint.

Acceptance criteria

  • A request whose stored per-request context is gone at dispatch is never handed to before-hooks or the handler, on local and rehydrated sessions.
  • Notifications and messages without a nonce behave as before.
  • Tests above pass under -race; comments updated; spec deviation stated in code.
  • After release, bump github.com/stacklok/toolhive-core in stacklok/toolhive (go.mod) and run its vMCP tests (task test, LABEL_FILTER=vmcp task test-e2e).

Out of scope

Anything beyond refusing abandoned request dispatches.

主要言語
Go
スター
2
フォーク
6
平均マージ
23時間 44分
マージ済み PR(30日)
133

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

stacklok/toolhive-core のほかの issue

stacklok/toolhive-core の issue をすべて見る

似ている issue

Go の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。