mcpcompat/server: refuse dispatch of a request whose per-request context is gone
Los mantenedores suelen responder en 1 día
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 54/100
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
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
-
StreamableHTTPServer.ServeHTTPstores the POST's request context under a random nonce and stamps it on the request (mcpcompat/server/transports.go:487-489), then clears it whenServeHTTPreturns (transports.go:502,defer s.mcp.clearPendingRequestContext(nonce)). -
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 exceptinitializeand notifications asynchronously (mcp/server.goaround line 2000), so dispatch can run afterServeHTTPhas returned. -
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} } } } -
The base context belongs to a different request: the
initializePOST for a session created on this replica (go-sdkconnectStreamable(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 dropnotifications/initializedand break every session. Decide request vs notification from the method/request type available in the middleware (for example thenotifications/method prefix, or go-sdk's request type), and add a test thatnotifications/initializedstill 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.
ServeHTTPalways overwrites the header withcrand.Text()(transports.go:487-489), so "nonce present, entry missing" reliably means the request was abandoned. Keep it that way. - Stateless mode.
ServeHTTPshort-circuits tos.handlerfor stateless servers after storing the nonce (transports.go:503-519). Check whether a stateless request can be dispatched after itsServeHTTPreturns; if it can, the same refusal applies; if it cannot, note why in a comment. - Rehydrated sessions. Requests on a rehydrated session go through
serveRehydratedwith the same nonce bridge. The refusal must apply there too; seerehydration_test.gofor 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-501andserver.go:829-837so 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):
- 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
initializeand notifications are handled synchronously. A recipe that works (it is how the equivalent toolhive test does it):- Register an
OnRegisterSessionhook (hooks.go:29-30,WithHooks,server.go:409) that records the session ID and then blocks on a channel. The hook runs inside theinitializedispatch, so while it blocks, every later message on that session waits in the queue. - Send
initializein a goroutine and wait until the hook has recorded the session ID. - Send a
tools/callwith thatMcp-Session-Idby calling the server'sServeHTTPin-process (httptest.NewRecorderplus a request built withhttptest.NewRequest(...).WithContext(ctx)wherectxis already cancelled). go-sdk queues the message andServeHTTPreturns at once, which clears the nonce entry. - Release the hook, let
initializefinish, 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 testsloghandler or a counter, so the test cannot pass just because the message was never dispatched.
- Register an
- Session still works afterwards. A normal
tools/callon the same session succeeds. - Notifications unaffected.
notifications/initialized(and a notification sent with an already-cancelled context) is still processed as today. - Rehydrated session. The same refusal on a session served by
serveRehydrated. - 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-coreinstacklok/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
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de stacklok/toolhive-core
-
needs-triage
Dificultad 3/5 1-2 días Aptitud para principiantes 66/100
stacklok/toolhive-core#426 ·
Los mantenedores suelen responder en 1 día
-
needs-triage
Dificultad 5/5 Más de una semana Aptitud para principiantes 8/100
stacklok/toolhive-core#343 ·
Los mantenedores suelen responder en 1 día
-
needs-triage
Dificultad 5/5 Más de una semana Aptitud para principiantes 12/100
stacklok/toolhive-core#342 ·
Los mantenedores suelen responder en 1 día
-
needs-triage
Dificultad 5/5 Más de una semana Aptitud para principiantes 15/100
stacklok/toolhive-core#341 ·
Los mantenedores suelen responder en 1 día
-
needs-triage
Dificultad 4/5 3-5 días Aptitud para principiantes 48/100
stacklok/toolhive-core#239 ·
Los mantenedores suelen responder en 1 día
Todos los issues de stacklok/toolhive-core
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 83/100
txn2/mcp-data-platform#2493 ·
Los mantenedores suelen responder en 1 día
-
area/docs framework/hugo help wanted language/markdown
Dificultad 1/5 Menos de una hora Aptitud para principiantes 88/100
Los mantenedores suelen responder en 1 día
-
clawsweeper:fix-shape-clear clawsweeper:queueable-fix clawsweeper:source-repro impact:other issue-rating: 🦞 diamond lobster no-stale P2
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
openclaw/crabbox#3060 · 1 comentario · 1 reacción ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 74/100
radius-project/radius#13698 · 1 comentario ·
Los mantenedores suelen responder en 1 día