From 7ef0cb303caa38ba559b40121e270f2c22a689d0 Mon Sep 17 00:00:00 2001 From: Ronald Ekambi Date: Tue, 1 Sep 2026 16:55:32 -0400 Subject: [PATCH 1/3] mcp: label resultType on results that bypass the dispatcher Protocol revision 2026-07-28 requires resultType on every result. CallToolResult, GetPromptResult and ReadResourceResult hold it in an unexported field, since they can be complete or input_required and so cannot embed completeResultWithType as the seven types in #1060 do. Only handleMultiRoundTripResult writes that field, and it runs inside Server.callTool, getPrompt and readResource. A receiving middleware that returns a result instead of calling next -- an auth, rate limit or entitlement gate -- never reaches it, so the response ships without the field and strict clients reject it as malformed. Move the choice of complete versus input_required into setMultiRoundTripResultType, and call it from both places that need it. setCompleteResultType, which now labels results it did not before, is renamed annotateResultType and picks a branch by type switch. It reads inputRequests() rather than assuming complete, which matters because setResultType is unexported: populating InputRequests is a middleware's only way to ask for input, and labeling it complete would make a conforming client drop the elicitation. Recomputing is safe because a result that did reach the dispatcher was labeled from the same test. Embedding completeResultWithType instead would label every result complete and clobber input_required, breaking multi-round-trip flows. A middleware can also return a typed nil, which satisfies Result, and annotating one panics. That panic predates this change; it happened one frame further on, in annotateServerInfo. Result gains isNil, matching what Params already does, and ServerSession.handle skips annotation for a nil result. Result is sealed by isResult, so adding an unexported method to it is not a breaking change. ServerSession.handle still gates on the request's _meta while handleMultiRoundTripResult gates on the session's negotiated version, so a session that negotiated 2026-07-28 and sends no _meta gets no resultType from a short-circuited request. Only a client that mixes protocol versions can reach that, so TestServerSessionHandle_ResultTypeGate records it rather than changing behavior to cover it. Also correct ten comments naming an exported ResultType field and ResultTypeComplete/ResultTypeInputRequired constants, none of which exist. Two were unresolvable [CallToolResult.ResultType] doc links and now point at [CallToolResult.NeedsInput]. The exported API is unchanged and the conformance goldens do not move. Fixes #1225 Co-Authored-By: Claude Opus 5 (1M context) --- mcp/mrtr.go | 16 ++- mcp/protocol.go | 86 +++++++++------ mcp/server.go | 6 +- mcp/server_test.go | 270 ++++++++++++++++++++++++++++++++++++++++----- mcp/shared.go | 7 +- 5 files changed, 317 insertions(+), 68 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index 008c0f84e..38dff366f 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -53,15 +53,21 @@ func handleMultiRoundTripResult(ss *ServerSession, logger *slog.Logger, res mult if clientSupportsMultiRoundTrip(ss) { // For older clients the resultType is left unset. Input requests will be handled // by serverMultiRoundTripMiddleware client calls and handler reinvocation. - if hasInputRequests { - res.setResultType(resultTypeInputRequired) - } else { - res.setResultType(resultTypeComplete) - } + setMultiRoundTripResultType(res) } return nil } +// setMultiRoundTripResultType labels res complete or input_required by +// whether it carries input requests. +func setMultiRoundTripResultType(res multiRoundTripResponse) { + if res.inputRequests() != nil { + res.setResultType(resultTypeInputRequired) + } else { + res.setResultType(resultTypeComplete) + } +} + func clientSupportsMultiRoundTrip(ss *ServerSession) bool { protocolVersion := latestProtocolVersion if iparams := ss.InitializeParams(); iparams != nil { diff --git a/mcp/protocol.go b/mcp/protocol.go index 4867e5440..d905acdf8 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -19,7 +19,7 @@ type resultType string const ( // resultTypeComplete indicates the result is final. - // This is the default when ResultType is empty. + // This is the default when resultType is empty. resultTypeComplete resultType = "complete" // resultTypeInputRequired indicates the server needs additional client @@ -40,9 +40,16 @@ type completeResultResponse interface { isCompleteResult() } -func setCompleteResultType(res Result) { - if r, ok := res.(completeResultResponse); ok { +// annotateResultType sets the resultType that protocol 2026-07-28 requires on +// every result. +func annotateResultType(res Result) { + switch r := res.(type) { + case completeResultResponse: r.setResultType(resultTypeComplete) + case multiRoundTripResponse: + // A middleware can return one of these without reaching the + // dispatcher, leaving the field unset. + setMultiRoundTripResultType(r) } } @@ -226,9 +233,8 @@ type CallToolParams struct { // marshaled to JSON. Arguments any `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when retrying + // a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -249,9 +255,8 @@ type CallToolParamsRaw struct { // Arguments (see [AddTool]). Arguments json.RawMessage `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when retrying + // a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -305,7 +310,7 @@ type CallToolResult struct { IsError bool `json:"isError,omitempty"` // InputRequests is a map of server-assigned IDs to input requests. - // Populated only when ResultType is ResultTypeInputRequired. + // Set only when the result needs more client input. // The client must fulfill these and echo the IDs back in InputResponses // when retrying the call. InputRequests InputRequestMap `json:"inputRequests,omitempty"` @@ -317,10 +322,10 @@ type CallToolResult struct { // Unauthenticated servers must encrypt, sign and verify this value. RequestState string `json:"requestState,omitempty"` - // ResultType indicates whether this result is complete or requires further - // client input. Empty or ResultTypeComplete means the call succeeded - // normally. ResultTypeInputRequired means the client should fulfill the - // InputRequests and retry the call. + // resultType records whether the call finished or needs more client input. + // Empty or resultTypeComplete means it finished. resultTypeInputRequired + // means the client should fulfill InputRequests and retry. + // See [CallToolResult.NeedsInput]. resultType resultType // The error passed to setError, if any. // It is not marshaled, and therefore it is only visible on the server. @@ -358,7 +363,8 @@ func (r *CallToolResult) GetError() error { return r.err } -func (*CallToolResult) isResult() {} +func (*CallToolResult) isResult() {} +func (x *CallToolResult) isNil() bool { return x == nil } func (r *CallToolResult) setResultType(rt resultType) { r.resultType = rt } func (r *CallToolResult) requestState() string { return r.RequestState } @@ -373,7 +379,7 @@ func (r *CallToolResult) hasContent() bool { } // NeedsInput reports whether this result requires further client input. -// This is true when the server returned ResultType "input_required". +// This is true when the server returned a resultType of "input_required". // When NeedsInput returns true, check InputRequests for the set of // requests the server needs fulfilled before retrying the call. // An empty InputRequests with NeedsInput true indicates load-shedding. @@ -665,7 +671,8 @@ type CompleteResult struct { Completion CompletionResultDetails `json:"completion"` } -func (*CompleteResult) isResult() {} +func (*CompleteResult) isResult() {} +func (x *CompleteResult) isNil() bool { return x == nil } // CreateMessageParams holds parameters for a sampling/createMessage request. // @@ -849,6 +856,7 @@ type CreateMessageResult struct { } func (*CreateMessageResult) isResult() {} +func (x *CreateMessageResult) isNil() bool { return x == nil } func (*CreateMessageResult) isInputResponse() {} func (r *CreateMessageResult) UnmarshalJSON(data []byte) error { type result CreateMessageResult // avoid recursion @@ -900,6 +908,7 @@ var createMessageWithToolsResultAllow = map[string]bool{ } func (*CreateMessageWithToolsResult) isResult() {} +func (x *CreateMessageWithToolsResult) isNil() bool { return x == nil } func (*CreateMessageWithToolsResult) isInputResponse() {} // MarshalJSON marshals the result. When Content has a single element, it is @@ -960,9 +969,8 @@ type GetPromptParams struct { // The name of the prompt or prompt template. Name string `json:"name"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when retrying + // a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -982,19 +990,20 @@ type GetPromptResult struct { Description string `json:"description,omitempty"` Messages []*PromptMessage `json:"messages"` - // InputRequests is populated when ResultType is ResultTypeInputRequired. + // InputRequests is set when the result needs more client input. // See [CallToolResult.InputRequests]. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is the opaque state for multi-round-trip retries. // See [CallToolResult.RequestState]. RequestState string `json:"requestState,omitempty"` - // ResultType indicates whether this result is complete or requires further - // client input. See [CallToolResult.ResultType] for details. + // resultType records whether the call finished or needs more client input. + // See [CallToolResult.NeedsInput]. resultType resultType } -func (*GetPromptResult) isResult() {} +func (*GetPromptResult) isResult() {} +func (x *GetPromptResult) isNil() bool { return x == nil } func (r *GetPromptResult) setResultType(rt resultType) { r.resultType = rt } func (r *GetPromptResult) requestState() string { return r.RequestState } @@ -1104,7 +1113,8 @@ type InitializeResult struct { ServerInfo *Implementation `json:"serverInfo"` } -func (*InitializeResult) isResult() {} +func (*InitializeResult) isResult() {} +func (x *InitializeResult) isNil() bool { return x == nil } type InitializedParams struct { // Meta is reserved by the protocol to allow clients and servers to attach @@ -1147,7 +1157,8 @@ type DiscoverResult struct { Instructions string `json:"instructions,omitempty"` } -func (*DiscoverResult) isResult() {} +func (*DiscoverResult) isResult() {} +func (x *DiscoverResult) isNil() bool { return x == nil } func (x *ListPromptsParams) isParams() {} func (x *ListPromptsParams) isNil() bool { return x == nil } @@ -1217,6 +1228,7 @@ type ListPromptsResult struct { } func (x *ListPromptsResult) isResult() {} +func (x *ListPromptsResult) isNil() bool { return x == nil } func (x *ListPromptsResult) nextCursorPtr() *string { return &x.NextCursor } type ListResourceTemplatesParams struct { @@ -1248,6 +1260,7 @@ type ListResourceTemplatesResult struct { } func (x *ListResourceTemplatesResult) isResult() {} +func (x *ListResourceTemplatesResult) isNil() bool { return x == nil } func (x *ListResourceTemplatesResult) nextCursorPtr() *string { return &x.NextCursor } type ListResourcesParams struct { @@ -1279,6 +1292,7 @@ type ListResourcesResult struct { } func (x *ListResourcesResult) isResult() {} +func (x *ListResourcesResult) isNil() bool { return x == nil } func (x *ListResourcesResult) nextCursorPtr() *string { return &x.NextCursor } // ListRootsParams holds parameters for a roots/list request. @@ -1315,6 +1329,7 @@ type ListRootsResult struct { } func (*ListRootsResult) isResult() {} +func (x *ListRootsResult) isNil() bool { return x == nil } func (*ListRootsResult) isInputResponse() {} type ListToolsParams struct { @@ -1346,6 +1361,7 @@ type ListToolsResult struct { } func (x *ListToolsResult) isResult() {} +func (x *ListToolsResult) isNil() bool { return x == nil } func (x *ListToolsResult) nextCursorPtr() *string { return &x.NextCursor } // The severity of a log message. @@ -1585,9 +1601,8 @@ type ReadResourceParams struct { // the server how to interpret it. URI string `json:"uri"` - // InputResponses maps input request IDs to responses, provided when - // retrying a call after receiving a result with ResultType - // ResultTypeInputRequired. + // InputResponses maps input request IDs to responses. Set it when retrying + // a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -1606,19 +1621,20 @@ type ReadResourceResult struct { Cacheable Contents []*ResourceContents `json:"contents"` - // InputRequests is populated when ResultType is ResultTypeInputRequired. + // InputRequests is set when the result needs more client input. // See [CallToolResult.InputRequests]. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is the opaque state for multi-round-trip retries. // See [CallToolResult.RequestState]. RequestState string `json:"requestState,omitempty"` - // ResultType indicates whether this result is complete or requires further - // client input. See [CallToolResult.ResultType] for details. + // resultType records whether the call finished or needs more client input. + // See [CallToolResult.NeedsInput]. resultType resultType } -func (*ReadResourceResult) isResult() {} +func (*ReadResourceResult) isResult() {} +func (x *ReadResourceResult) isNil() bool { return x == nil } func (r *ReadResourceResult) setResultType(rt resultType) { r.resultType = rt } func (r *ReadResourceResult) requestState() string { return r.RequestState } @@ -2124,7 +2140,8 @@ type SubscriptionsListenResult struct { Meta `json:"_meta"` } -func (*SubscriptionsListenResult) isResult() {} +func (*SubscriptionsListenResult) isResult() {} +func (x *SubscriptionsListenResult) isNil() bool { return x == nil } // TODO(jba): add CompleteRequest and related types. @@ -2201,6 +2218,7 @@ type ElicitResult struct { } func (*ElicitResult) isResult() {} +func (x *ElicitResult) isNil() bool { return x == nil } func (*ElicitResult) isInputResponse() {} // ElicitationCompleteParams is sent from the server to the client, informing it that an out-of-band elicitation interaction has completed. diff --git a/mcp/server.go b/mcp/server.go index 5014a4f33..15fe08f15 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -2039,8 +2039,10 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, if err != nil { return nil, err } - if validatedMeta.usesNewProtocol { - setCompleteResultType(res) + // A middleware can return a typed nil, which satisfies Result but panics + // when annotated. + if validatedMeta.usesNewProtocol && res != nil && !res.isNil() { + annotateResultType(res) annotateServerInfo(res, ss.server.impl) } return res, nil diff --git a/mcp/server_test.go b/mcp/server_test.go index 11e75e232..b467439f7 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -1399,6 +1399,22 @@ func TestServerSessionHandle_RejectsInitializeOnNewProtocol(t *testing.T) { }) } +// newProtocolParams merges fields into the _meta that opts a request into the +// sessionless protocol (SEP-2575). +func newProtocolParams(fields map[string]any) map[string]any { + params := map[string]any{ + "_meta": map[string]any{ + MetaKeyProtocolVersion: protocolVersion20260728, + MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, + MetaKeyClientCapabilities: map[string]any{}, + }, + } + for k, v := range fields { + params[k] = v + } + return params +} + func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { server := NewServer(testImpl, &ServerOptions{ CompletionHandler: func(context.Context, *CompleteRequest) (*CompleteResult, error) { @@ -1424,19 +1440,19 @@ func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Continue?"}}, }, nil }) - newProtocolParams := func(fields map[string]any) map[string]any { - params := map[string]any{ - "_meta": map[string]any{ - MetaKeyProtocolVersion: protocolVersion20260728, - MetaKeyClientInfo: map[string]any{"name": "c", "version": "1"}, - MetaKeyClientCapabilities: map[string]any{}, - }, - } - for k, v := range fields { - params[k] = v - } - return params - } + AddTool(server, &Tool{Name: "toolComplete"}, func(context.Context, *CallToolRequest, struct{}) (*CallToolResult, any, error) { + return &CallToolResult{Content: []Content{&TextContent{Text: "ok"}}}, nil, nil + }) + server.AddPrompt(&Prompt{Name: "promptComplete"}, func(context.Context, *GetPromptRequest) (*GetPromptResult, error) { + return &GetPromptResult{ + Messages: []*PromptMessage{{Role: "assistant", Content: &TextContent{Text: "ok"}}}, + }, nil + }) + server.AddResource(&Resource{URI: "test://resource-complete", Name: "resourceComplete"}, func(context.Context, *ReadResourceRequest) (*ReadResourceResult, error) { + return &ReadResourceResult{ + Contents: []*ResourceContents{{URI: "test://resource-complete", Text: "ok"}}, + }, nil + }) tests := []struct { name string @@ -1507,40 +1523,242 @@ func TestServerSessionHandle_SetsResultTypeOnNewProtocol(t *testing.T) { params: newProtocolParams(map[string]any{"uri": "test://resource"}), want: resultTypeInputRequired, }, + { + name: "tool complete", + method: methodCallTool, + params: newProtocolParams(map[string]any{"name": "toolComplete", "arguments": map[string]any{}}), + want: resultTypeComplete, + }, + { + name: "prompt complete", + method: methodGetPrompt, + params: newProtocolParams(map[string]any{"name": "promptComplete"}), + want: resultTypeComplete, + }, + { + name: "resource complete", + method: methodReadResource, + params: newProtocolParams(map[string]any{"uri": "test://resource-complete"}), + want: resultTypeComplete, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := resultTypeOf(t, server, nil, tc.method, tc.params); got != string(tc.want) { + t.Errorf("resultType = %q, want %q", got, tc.want) + } + }) + } +} + +// TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits is a +// regression test for #1225. User middleware wraps outside +// serverMultiRoundTripMiddleware, so a middleware that returns a result +// without calling next bypasses both that shim and the dispatcher. +func TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits(t *testing.T) { + tests := []struct { + name string + method string + params map[string]any + result Result + want resultType + }{ + { + name: "tools/call", + method: methodCallTool, + params: newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}}), + result: &CallToolResult{Content: []Content{&TextContent{Text: "denied"}}}, + want: resultTypeComplete, + }, + { + name: "prompts/get", + method: methodGetPrompt, + params: newProtocolParams(map[string]any{"name": "prompt"}), + result: &GetPromptResult{ + Messages: []*PromptMessage{{Role: "assistant", Content: &TextContent{Text: "denied"}}}, + }, + want: resultTypeComplete, + }, + { + name: "resources/read", + method: methodReadResource, + params: newProtocolParams(map[string]any{"uri": "test://resource"}), + result: &ReadResourceResult{ + Contents: []*ResourceContents{{URI: "test://resource", Text: "denied"}}, + }, + want: resultTypeComplete, + }, + { + // setResultType is unexported, so populating InputRequests is + // the only way a middleware can ask for input. + name: "tools/call input required", + method: methodCallTool, + params: newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}}), + result: &CallToolResult{ + InputRequests: InputRequestMap{"confirm": &ElicitParams{Message: "Continue?"}}, + }, + want: resultTypeInputRequired, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { + // Nothing is registered: if the middleware fell through to next, + // dispatch would fail rather than return a result. + server := NewServer(testImpl, nil) + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(ctx context.Context, method string, req Request) (Result, error) { + if method == tc.method { + return tc.result, nil + } + return next(ctx, method, req) + } + }) + if got := resultTypeOf(t, server, nil, tc.method, tc.params); got != string(tc.want) { + t.Errorf("resultType = %q, want %q", got, tc.want) + } + }) + } +} + +// TestServerSessionHandle_ResultTypeGate pins when a result carries resultType. +// handle gates on the request's _meta, while handleMultiRoundTripResult gates +// on the session's negotiated version, so the last case below gets no +// resultType even though the session speaks 2026-07-28. Only a client that +// mixes protocol versions reaches that, so it is recorded rather than fixed. +func TestServerSessionHandle_ResultTypeGate(t *testing.T) { + version := func(v string) func(*ServerSessionState) { + return func(s *ServerSessionState) { + s.InitializeParams = &InitializeParams{ProtocolVersion: v} + } + } + callParams := map[string]any{"name": "tool", "arguments": map[string]any{}} + + tests := []struct { + name string + state func(*ServerSessionState) + params map[string]any + want resultType // "" means the field must be absent + }{ + { + name: "legacy session, no _meta", + state: version(protocolVersion20251125), + params: callParams, + }, + { + name: "legacy session, new-protocol _meta", + state: version(protocolVersion20251125), + params: newProtocolParams(callParams), + want: resultTypeComplete, + }, + { + name: "new-protocol session, no _meta", + state: version(protocolVersion20260728), + params: callParams, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := NewServer(testImpl, nil) + // Short-circuit so this measures handle's gate, not the + // dispatcher's. + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(context.Context, string, Request) (Result, error) { + return &CallToolResult{Content: []Content{&TextContent{Text: "ok"}}}, nil + } + }) + if got := resultTypeOf(t, server, tc.state, methodCallTool, tc.params); got != string(tc.want) { + t.Errorf("resultType = %q, want %q", got, tc.want) + } + }) + } +} + +// TestServerSessionHandle_NilResultFromMiddleware checks that a middleware +// returning no result does not panic. A typed nil satisfies Result, so the +// annotation helpers would otherwise dereference it. +func TestServerSessionHandle_NilResultFromMiddleware(t *testing.T) { + tests := []struct { + name string + result Result + }{ + {"nil interface", nil}, + {"typed nil", (*CallToolResult)(nil)}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + server := NewServer(testImpl, nil) + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(context.Context, string, Request) (Result, error) { + return tc.result, nil + } + }) ss := &ServerSession{server: server} id, err := jsonrpc.MakeID("test") if err != nil { t.Fatal(err) } - result, err := ss.handle(context.Background(), &jsonrpc.Request{ + res, err := ss.handle(context.Background(), &jsonrpc.Request{ ID: id, - Method: tc.method, - Params: mustMarshal(tc.params), + Method: methodCallTool, + Params: mustMarshal(newProtocolParams(map[string]any{"name": "tool", "arguments": map[string]any{}})), }) if err != nil { - t.Fatal(err) + t.Fatalf("handle: %v", err) } - data, err := json.Marshal(result) + data, err := json.Marshal(res) if err != nil { t.Fatal(err) } - var got struct { - ResultType string `json:"resultType"` - } - if err := json.Unmarshal(data, &got); err != nil { - t.Fatal(err) - } - if got.ResultType != string(tc.want) { - t.Fatalf("resultType = %q, want %q; response = %s", got.ResultType, tc.want, data) + if got := string(data); got != "null" { + t.Errorf("result = %s, want null", got) } }) } } +// resultTypeOf handles one request on a transportless session and reports the +// resultType of the marshaled result, or "" if the field is absent. +func resultTypeOf(t *testing.T, server *Server, state func(*ServerSessionState), method string, params map[string]any) string { + t.Helper() + ss := &ServerSession{server: server} + if state != nil { + ss.updateState(state) + } + id, err := jsonrpc.MakeID("test") + if err != nil { + t.Fatal(err) + } + result, err := ss.handle(context.Background(), &jsonrpc.Request{ + ID: id, + Method: method, + Params: mustMarshal(params), + }) + if err != nil { + t.Fatal(err) + } + data, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + t.Fatal(err) + } + raw, ok := fields["resultType"] + if !ok { + return "" + } + var got string + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("unmarshaling resultType from %s: %v", data, err) + } + return got +} + // TestServerSessionHandle_RejectsRemovedMethodsOnNewProtocol verifies that // the methods removed by SEP-2575 (`initialize`, `notifications/initialized`, // `ping`) all return Method not found when the request opts into the new diff --git a/mcp/shared.go b/mcp/shared.go index 22587526c..6637a256a 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -809,6 +809,9 @@ type Result interface { // isResult discourages implementation of Result outside of this package. isResult() + // isNil returns true if the underlying value is nil. + isNil() bool + // GetMeta returns metadata from a value. GetMeta() map[string]any // SetMeta sets the metadata on a value. @@ -827,13 +830,15 @@ type ResultBase struct { Meta `json:"_meta,omitempty"` } -func (*ResultBase) isResult() {} +func (*ResultBase) isResult() {} +func (x *ResultBase) isNil() bool { return x == nil } // emptyResult is returned by methods that have no result, like ping. // Those methods cannot return nil, because jsonrpc2 cannot handle nils. type emptyResult struct{} func (*emptyResult) isResult() {} +func (x *emptyResult) isNil() bool { return x == nil } func (*emptyResult) GetMeta() map[string]any { panic("should never be called") } func (*emptyResult) SetMeta(map[string]any) { panic("should never be called") } From 3b1b7040b003b8579c7d00f3c83886ed9be2e784 Mon Sep 17 00:00:00 2001 From: Ronald Ekambi Date: Wed, 2 Sep 2026 15:54:04 -0400 Subject: [PATCH 2/3] mcp: simplify comment wording in resultType labeling code Applies ASD-STE100 Simplified Technical English to the comments touched in the resultType labeling change: complete sentences with explicit subjects, no -ing verb clauses, and "must" instead of "should" for the InputRequests requirement. --- mcp/protocol.go | 24 ++++++++++++------------ mcp/server.go | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/mcp/protocol.go b/mcp/protocol.go index d905acdf8..e710e9630 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -48,7 +48,7 @@ func annotateResultType(res Result) { r.setResultType(resultTypeComplete) case multiRoundTripResponse: // A middleware can return one of these without reaching the - // dispatcher, leaving the field unset. + // dispatcher. This leaves the field unset. setMultiRoundTripResultType(r) } } @@ -233,8 +233,8 @@ type CallToolParams struct { // marshaled to JSON. Arguments any `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses. Set it when retrying - // a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -255,8 +255,8 @@ type CallToolParamsRaw struct { // Arguments (see [AddTool]). Arguments json.RawMessage `json:"arguments,omitempty"` - // InputResponses maps input request IDs to responses. Set it when retrying - // a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. // The client must echo this back when retrying. @@ -310,9 +310,9 @@ type CallToolResult struct { IsError bool `json:"isError,omitempty"` // InputRequests is a map of server-assigned IDs to input requests. - // Set only when the result needs more client input. + // It is set only when the result needs more client input. // The client must fulfill these and echo the IDs back in InputResponses - // when retrying the call. + // when it retries the call. InputRequests InputRequestMap `json:"inputRequests,omitempty"` // RequestState is an opaque string the client must echo back when @@ -324,7 +324,7 @@ type CallToolResult struct { // resultType records whether the call finished or needs more client input. // Empty or resultTypeComplete means it finished. resultTypeInputRequired - // means the client should fulfill InputRequests and retry. + // means the client must fulfill InputRequests and retry. // See [CallToolResult.NeedsInput]. resultType resultType // The error passed to setError, if any. @@ -969,8 +969,8 @@ type GetPromptParams struct { // The name of the prompt or prompt template. Name string `json:"name"` - // InputResponses maps input request IDs to responses. Set it when retrying - // a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` @@ -1601,8 +1601,8 @@ type ReadResourceParams struct { // the server how to interpret it. URI string `json:"uri"` - // InputResponses maps input request IDs to responses. Set it when retrying - // a call that asked for input. See [CallToolResult.NeedsInput]. + // InputResponses maps input request IDs to responses. Set it when the + // client retries a call that asked for input. See [CallToolResult.NeedsInput]. InputResponses InputResponseMap `json:"inputResponses,omitempty"` // RequestState is the opaque state from the previous input-required result. RequestState string `json:"requestState,omitempty"` diff --git a/mcp/server.go b/mcp/server.go index 15fe08f15..f909daee7 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -2039,8 +2039,8 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any, if err != nil { return nil, err } - // A middleware can return a typed nil, which satisfies Result but panics - // when annotated. + // A middleware can return a typed nil value that satisfies the Result + // interface. Annotating that value panics. if validatedMeta.usesNewProtocol && res != nil && !res.isNil() { annotateResultType(res) annotateServerInfo(res, ss.server.impl) From 5e5198c1008586d1e2a6f2e8785fc565911f7c9c Mon Sep 17 00:00:00 2001 From: Ronald Ekambi Date: Tue, 8 Sep 2026 10:08:00 -0400 Subject: [PATCH 3/3] mcp: make annotateResultType the only resultType setter handleMultiRoundTripResult also labeled resultType, so the field was set in two places under two different conditions: the session's negotiated version there, and the request's _meta in ServerSession.handle. The dispatcher read the field it had just set. callTool used it to keep a nil Content as "null", and readResource used it to return early before the nil Contents error. Both now test InputRequests directly, which is what the label was derived from, so neither depends on the labeling. handleMultiRoundTripResult is left only rejecting a result that carries both content and input requests, so rename it to validateMultiRoundTripResult and drop the now-unused session parameter. Co-Authored-By: Claude Opus 5 (1M context) --- mcp/mrtr.go | 25 +++++-------------------- mcp/protocol.go | 10 +++++++--- mcp/server.go | 10 +++++----- mcp/server_test.go | 9 +++++---- 4 files changed, 22 insertions(+), 32 deletions(-) diff --git a/mcp/mrtr.go b/mcp/mrtr.go index 38dff366f..fdf98ece8 100644 --- a/mcp/mrtr.go +++ b/mcp/mrtr.go @@ -36,38 +36,23 @@ type multiRoundTripResponse interface { hasContent() bool } -func handleMultiRoundTripResult(ss *ServerSession, logger *slog.Logger, res multiRoundTripResponse) error { +// validateMultiRoundTripResult rejects a result that carries both content and +// input requests. [annotateResultType] labels the result later, in +// [ServerSession.handle], so this function does not set resultType. +func validateMultiRoundTripResult(logger *slog.Logger, res multiRoundTripResponse) error { if res == nil { return nil } - hasInputRequests := res.inputRequests() != nil - - if hasInputRequests && res.hasContent() { + if res.inputRequests() != nil && res.hasContent() { logger.Warn("handler returned both content and inputRequests") return &jsonrpc.Error{ Code: jsonrpc.CodeInternalError, Message: "server bug: result has both content and inputRequests", } } - - if clientSupportsMultiRoundTrip(ss) { - // For older clients the resultType is left unset. Input requests will be handled - // by serverMultiRoundTripMiddleware client calls and handler reinvocation. - setMultiRoundTripResultType(res) - } return nil } -// setMultiRoundTripResultType labels res complete or input_required by -// whether it carries input requests. -func setMultiRoundTripResultType(res multiRoundTripResponse) { - if res.inputRequests() != nil { - res.setResultType(resultTypeInputRequired) - } else { - res.setResultType(resultTypeComplete) - } -} - func clientSupportsMultiRoundTrip(ss *ServerSession) bool { protocolVersion := latestProtocolVersion if iparams := ss.InitializeParams(); iparams != nil { diff --git a/mcp/protocol.go b/mcp/protocol.go index 7472406c5..be37a1838 100644 --- a/mcp/protocol.go +++ b/mcp/protocol.go @@ -47,9 +47,13 @@ func annotateResultType(res Result) { case completeResultResponse: r.setResultType(resultTypeComplete) case multiRoundTripResponse: - // A middleware can return one of these without reaching the - // dispatcher. This leaves the field unset. - setMultiRoundTripResultType(r) + // These results are complete or input_required, so label them by + // whether the handler asked for more client input. + if r.inputRequests() != nil { + r.setResultType(resultTypeInputRequired) + } else { + r.setResultType(resultTypeComplete) + } } } diff --git a/mcp/server.go b/mcp/server.go index f909daee7..183a51922 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -910,7 +910,7 @@ func (s *Server) getPrompt(ctx context.Context, req *GetPromptRequest) (*GetProm } res, err := prompt.handler(ctx, req) if err == nil && res != nil { - if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { + if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { return nil, err } } @@ -1012,10 +1012,10 @@ func (s *Server) callTool(ctx context.Context, req *CallToolRequest) (*CallToolR } res, err := st.handler(ctx, req) if err == nil && res != nil { - if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { + if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { return nil, err } - if res.Content == nil && res.resultType != resultTypeInputRequired { + if res.Content == nil && res.InputRequests == nil { res2 := *res res2.Content = []Content{} // avoid "null" res = &res2 @@ -1080,11 +1080,11 @@ func (s *Server) readResource(ctx context.Context, req *ReadResourceRequest) (*R if res == nil { return nil, fmt.Errorf("reading resource %s: read handler returned nil information", uri) } - if err := handleMultiRoundTripResult(req.Session, s.opts.Logger, res); err != nil { + if err := validateMultiRoundTripResult(s.opts.Logger, res); err != nil { return nil, err } s.resolveCacheable(ctx, req, &res.Cacheable) - if res.resultType == resultTypeInputRequired { + if res.InputRequests != nil { return res, nil } if res.Contents == nil { diff --git a/mcp/server_test.go b/mcp/server_test.go index b467439f7..77dcb10b1 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -1623,10 +1623,11 @@ func TestServerSessionHandle_SetsResultTypeWhenMiddlewareShortCircuits(t *testin } // TestServerSessionHandle_ResultTypeGate pins when a result carries resultType. -// handle gates on the request's _meta, while handleMultiRoundTripResult gates -// on the session's negotiated version, so the last case below gets no -// resultType even though the session speaks 2026-07-28. Only a client that -// mixes protocol versions reaches that, so it is recorded rather than fixed. +// annotateResultType is the only setter, and handle gates it on the request's +// _meta rather than on the version the session negotiated. So the last case +// below gets no resultType even though the session speaks 2026-07-28. Only a +// client that mixes protocol versions reaches that, so it is recorded rather +// than fixed. func TestServerSessionHandle_ResultTypeGate(t *testing.T) { version := func(v string) func(*ServerSessionState) { return func(s *ServerSessionState) {