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

Abierto
#414 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

@jhrozek ya está trabajando en esto.

Desde el 26/10/2026.

  • #415 de @jhrozek — abierto

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
54/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
go
Área
backend

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Go
Estrellas
2
Forks
5
Merge medio
1 d 2 h
PR fusionados (30 d)
124

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de stacklok/toolhive-core

Todos los issues de stacklok/toolhive-core

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.