From 908bd198c2e418231470fa0d0595f78b874c8a5e Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 12:45:49 +0800 Subject: [PATCH 1/8] feat: bump go-sdk to v1.7.0 and move elicitation to multi-round-trip input requests --- examples/tasks/e2e_elicitation_legacy_test.go | 174 ++++++++++++++++++ examples/tasks/e2e_elicitation_test.go | 4 +- go.mod | 6 +- go.sum | 12 +- internal/gen/generator.go | 42 ++++- internal/gen/generator_test.go | 15 +- internal/gen/templates/tool.go.tmpl | 27 ++- pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go | 29 ++- 8 files changed, 275 insertions(+), 34 deletions(-) create mode 100644 examples/tasks/e2e_elicitation_legacy_test.go diff --git a/examples/tasks/e2e_elicitation_legacy_test.go b/examples/tasks/e2e_elicitation_legacy_test.go new file mode 100644 index 0000000..9514d53 --- /dev/null +++ b/examples/tasks/e2e_elicitation_legacy_test.go @@ -0,0 +1,174 @@ +// Legacy-protocol coverage for the elicitation gate. Every other +// elicitation test in this package rides in-memory transports, which +// negotiate protocol 2026-07-28 via server/discover and therefore +// exercise only the go-sdk's client-side multi-round-trip middleware. +// Pre-2026 clients instead hit the SDK's server-side compatibility shim: +// it intercepts the InputRequests result, performs a real session.Elicit +// round-trip, and re-invokes the handler with the answer. That shim is +// the entire backward-compatibility story for generated elicitation +// gates, so it gets its own suite here. +// +// Streamable HTTP without Stateless pins sessions to 2025-11-25 (the +// transport refuses to advertise 2026-07-28), which is exactly the +// legacy path. connectLegacyHTTP asserts the negotiated version so this +// suite fails loudly if the transport ever starts speaking the new +// protocol and the shim silently stops being exercised. +package tasks_test + +import ( + "context" + "encoding/json" + "fmt" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + tasksv1 "github.com/akuity/protomcp/pkg/api/gen/examples/tasks/v1" + "github.com/akuity/protomcp/pkg/protomcp" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// legacyProtocolCutoff is the first protocol revision on which the +// server-side elicitation shim no longer applies. +const legacyProtocolCutoff = "2026-07-28" + +// connectLegacyHTTP serves srv over Streamable HTTP (non-stateless) and +// connects an MCP client to it, returning a session guaranteed to speak +// a pre-2026-07-28 protocol revision. +func connectLegacyHTTP(ctx context.Context, t *testing.T, srv *protomcp.Server, opts *mcp.ClientOptions) *mcp.ClientSession { + t.Helper() + ts := httptest.NewServer(srv) + t.Cleanup(ts.Close) + client := mcp.NewClient(&mcp.Implementation{Name: "legacy-test-client", Version: "0.0.1"}, opts) + cs, err := client.Connect(ctx, &mcp.StreamableClientTransport{Endpoint: ts.URL}, nil) + if err != nil { + t.Fatalf("client connect: %v", err) + } + t.Cleanup(func() { _ = cs.Close() }) + if v := cs.InitializeResult().ProtocolVersion; v >= legacyProtocolCutoff { + t.Fatalf("negotiated protocol %q >= %q: this suite must exercise the legacy server-side elicitation shim", v, legacyProtocolCutoff) + } + return cs +} + +// TestDeleteTask_LegacyShimAccept drives confirm-then-delete over the +// legacy path: the server-side shim turns the handler's InputRequests +// result into a session.Elicit round-trip, the client accepts, and the +// backend observes the Delete. +func TestDeleteTask_LegacyShimAccept(t *testing.T) { + ctx := context.Background() + grpcClient := startGRPC(t) + srv := protomcp.New("tasks", "0.1.0") + tasksv1.RegisterTasksMCPTools(srv, grpcClient) + + var seenMessage atomic.Value // string + var elicitCalls atomic.Int32 + cs := connectLegacyHTTP(ctx, t, srv, &mcp.ClientOptions{ + ElicitationHandler: func(_ context.Context, req *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + elicitCalls.Add(1) + if req != nil && req.Params != nil { + seenMessage.Store(req.Params.Message) + } + return &mcp.ElicitResult{Action: "accept"}, nil + }, + }) + + var created tasksv1.Task + callTool(ctx, t, cs, "Tasks_CreateTask", + `{"task":{"title":"delete-me-legacy","done":false}}`, &created) + if created.Id == "" { + t.Fatalf("Create: empty id") + } + + var del tasksv1.DeleteTaskResponse + callTool(ctx, t, cs, "Tasks_DeleteTask", + fmt.Sprintf(`{"id":%q}`, created.Id), &del) + if !del.Existed { + t.Errorf("Delete: Existed = false, want true (task was present before the delete)") + } + if got := elicitCalls.Load(); got != 1 { + t.Errorf("elicitation handler called %d times, want 1", got) + } + msg, _ := seenMessage.Load().(string) + if !strings.Contains(msg, created.Id) { + t.Errorf("elicitation message %q does not contain task id %q", msg, created.Id) + } +} + +// TestDeleteTask_LegacyShimDecline asserts the decline path through the +// shim: an IsError tool result (not a protocol error) and an untouched +// backend. +func TestDeleteTask_LegacyShimDecline(t *testing.T) { + ctx := context.Background() + grpcClient := startGRPC(t) + srv := protomcp.New("tasks", "0.1.0") + tasksv1.RegisterTasksMCPTools(srv, grpcClient) + + cs := connectLegacyHTTP(ctx, t, srv, &mcp.ClientOptions{ + ElicitationHandler: func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + return &mcp.ElicitResult{Action: "decline"}, nil + }, + }) + + var created tasksv1.Task + callTool(ctx, t, cs, "Tasks_CreateTask", + `{"task":{"title":"keep-me-legacy","done":false}}`, &created) + + out, err := cs.CallTool(ctx, &mcp.CallToolParams{ + Name: "Tasks_DeleteTask", + Arguments: json.RawMessage(fmt.Sprintf(`{"id":%q}`, created.Id)), + }) + if err != nil { + t.Fatalf("CallTool: transport error: %v", err) + } + if !out.IsError { + t.Fatalf("Delete: want IsError, got success: %+v", out) + } + + // Backend untouched. + var got tasksv1.Task + callTool(ctx, t, cs, "Tasks_GetTask", + fmt.Sprintf(`{"id":%q}`, created.Id), &got) + if got.Id != created.Id { + t.Errorf("Get after declined delete: got %q, want %q", got.Id, created.Id) + } +} + +// TestDeleteTask_LegacyShimNoHandler pins the upgrade-note behavior: a +// client with no ElicitationHandler now gets a hard protocol error from +// the elicitation round-trip — under go-sdk v1.5.0 this surfaced as a +// graceful IsError tool result instead. The backend must stay untouched +// either way. +func TestDeleteTask_LegacyShimNoHandler(t *testing.T) { + ctx := context.Background() + grpcClient := startGRPC(t) + srv := protomcp.New("tasks", "0.1.0") + tasksv1.RegisterTasksMCPTools(srv, grpcClient) + + cs := connectLegacyHTTP(ctx, t, srv, nil) + + var created tasksv1.Task + callTool(ctx, t, cs, "Tasks_CreateTask", + `{"task":{"title":"keep-me-nohandler","done":false}}`, &created) + + out, err := cs.CallTool(ctx, &mcp.CallToolParams{ + Name: "Tasks_DeleteTask", + Arguments: json.RawMessage(fmt.Sprintf(`{"id":%q}`, created.Id)), + }) + if err == nil { + t.Fatalf("CallTool: want hard error without an ElicitationHandler, got result %+v", out) + } + if !strings.Contains(err.Error(), "elicitation") { + t.Errorf("CallTool error %q does not mention elicitation", err) + } + + // Backend untouched. + var got tasksv1.Task + callTool(ctx, t, cs, "Tasks_GetTask", + fmt.Sprintf(`{"id":%q}`, created.Id), &got) + if got.Id != created.Id { + t.Errorf("Get after failed delete: got %q, want %q", got.Id, created.Id) + } +} diff --git a/examples/tasks/e2e_elicitation_test.go b/examples/tasks/e2e_elicitation_test.go index d502e4b..3cb6e5b 100644 --- a/examples/tasks/e2e_elicitation_test.go +++ b/examples/tasks/e2e_elicitation_test.go @@ -2,7 +2,9 @@ // DeleteTask carries both the destructive tool hint and a confirmation // elicitation in tasks.proto; the generated handler must: // -// - fire session.Elicit before the upstream gRPC call +// - publish the confirmation as a multi-round-trip InputRequest before +// the upstream gRPC call (the SDK fulfills it through the client's +// ElicitationHandler and re-invokes the handler with the answer) // - render {{id}} into the prompt so the user sees *which* task // - run the gRPC call only when action=="accept" // - return an IsError CallToolResult with a clear message when the diff --git a/go.mod b/go.mod index 4c47d57..9835a3c 100644 --- a/go.mod +++ b/go.mod @@ -5,8 +5,8 @@ go 1.26.2 require ( buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.11-20260415201107-50325440f8f2.1 github.com/cbroglie/mustache v1.4.0 - github.com/google/jsonschema-go v0.4.2 - github.com/modelcontextprotocol/go-sdk v1.5.0 + github.com/google/jsonschema-go v0.4.3 + github.com/modelcontextprotocol/go-sdk v1.7.0 github.com/yosida95/uritemplate/v3 v3.0.2 google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 google.golang.org/grpc v1.80.0 @@ -18,8 +18,10 @@ require ( github.com/segmentio/encoding v0.5.4 // indirect golang.org/x/net v0.49.0 // indirect golang.org/x/oauth2 v0.35.0 // indirect + golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.41.0 // indirect golang.org/x/text v0.33.0 // indirect + golang.org/x/time v0.15.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d // indirect google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.1 // indirect ) diff --git a/go.sum b/go.sum index c12c70d..7f4435d 100644 --- a/go.sum +++ b/go.sum @@ -14,12 +14,12 @@ github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= -github.com/google/jsonschema-go v0.4.2 h1:tmrUohrwoLZZS/P3x7ex0WAVknEkBZM46iALbcqoRA8= -github.com/google/jsonschema-go v0.4.2/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= +github.com/google/jsonschema-go v0.4.3 h1:/DBOLZTfDow7pe2GmaJNhltueGTtDKICi8V8p+DQPd0= +github.com/google/jsonschema-go v0.4.3/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/modelcontextprotocol/go-sdk v1.5.0 h1:CHU0FIX9kpueNkxuYtfYQn1Z0slhFzBZuq+x6IiblIU= -github.com/modelcontextprotocol/go-sdk v1.5.0/go.mod h1:gggDIhoemhWs3BGkGwd1umzEXCEMMvAnhTrnbXJKKKA= +github.com/modelcontextprotocol/go-sdk v1.7.0 h1:yqjY2dsbKAC0LSuWZVBMrHgiG8ukXv6NRo0JiALay44= +github.com/modelcontextprotocol/go-sdk v1.7.0/go.mod h1:dL7u98E/zjJTGzEq+j30jQ8K2k1mb6LeAH4inEcSGts= github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfvNt0= @@ -42,10 +42,14 @@ golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= golang.org/x/oauth2 v0.35.0 h1:Mv2mzuHuZuY2+bkyWXIHMfhNdJAdwW3FuWeCPYN5GVQ= golang.org/x/oauth2 v0.35.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.41.0 h1:Ivj+2Cp/ylzLiEU89QhWblYnOE9zerudt9Ftecq2C6k= golang.org/x/sys v0.41.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= +golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= +golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= diff --git a/internal/gen/generator.go b/internal/gen/generator.go index dffb088..0ef2887 100644 --- a/internal/gen/generator.go +++ b/internal/gen/generator.go @@ -131,12 +131,13 @@ type toolTemplateData struct { // Elicitation is non-nil when the method also carries a // protomcp.v1.elicitation annotation; the tool template then emits - // a session.Elicit(...) call before the upstream gRPC invocation. + // a multi-round-trip confirmation gate before the upstream gRPC + // invocation. Elicitation *elicitationTemplateData } // elicitationTemplateData carries the per-RPC context the tool template -// needs to emit a session.Elicit call. +// needs to emit a multi-round-trip elicitation input request. type elicitationTemplateData struct { // MessageExpr is a Go source expression that evaluates at runtime // to the fully-rendered confirmation prompt. For literal strings @@ -145,7 +146,14 @@ type elicitationTemplateData struct { // ...) field getters. MessageExpr string - QMCPElicitParams string + // RequestID is the server-assigned key under which the elicitation + // is published in the result's InputRequests map, and which the + // client echoes back in inputResponses on the retry. + RequestID string + + QMCPElicitParams string + QMCPInputRequestMap string + QMCPElicitResult string } // commonQuals bundles qualified identifiers every template site needs. @@ -704,9 +712,10 @@ func buildToolTemplateData( } // buildElicitationTemplateData validates the elicitation annotation and -// computes the tool template context for the session.Elicit(...) gate. -// Validation rejects empty messages, unresolved Mustache variables, and -// unsupported Mustache forms (sections, partials). +// computes the tool template context for the multi-round-trip +// confirmation gate. Validation rejects empty messages, unresolved +// Mustache variables, and unsupported Mustache forms (sections, +// partials). func buildElicitationTemplateData( g *protogen.GeneratedFile, m *protogen.Method, @@ -738,12 +747,29 @@ func buildElicitationTemplateData( GoName: "ElicitParams", GoImportPath: importMCP, }) + qInputRequestMap := g.QualifiedGoIdent(protogen.GoIdent{ + GoName: "InputRequestMap", + GoImportPath: importMCP, + }) + qElicitResult := g.QualifiedGoIdent(protogen.GoIdent{ + GoName: "ElicitResult", + GoImportPath: importMCP, + }) return &elicitationTemplateData{ - MessageExpr: expr, - QMCPElicitParams: qElicitParams, + MessageExpr: expr, + RequestID: elicitationRequestID, + QMCPElicitParams: qElicitParams, + QMCPInputRequestMap: qInputRequestMap, + QMCPElicitResult: qElicitResult, }, nil } +// elicitationRequestID is the server-assigned key for the confirmation +// input request. Servers choose these IDs freely; clients echo them back +// verbatim in inputResponses, so any stable value works. One elicitation +// per tool call means a single fixed key is sufficient. +const elicitationRequestID = "confirm" + // deriveToolName implements the ToolOptions.Name algorithm: explicit // override > synthesized _ > service-level prefix // applied on top. diff --git a/internal/gen/generator_test.go b/internal/gen/generator_test.go index f3c91ef..d7e5b30 100644 --- a/internal/gen/generator_test.go +++ b/internal/gen/generator_test.go @@ -100,8 +100,10 @@ func TestGenerate_BadStreams_BidiErrors(t *testing.T) { // TestGenerate_Elicit covers the happy path where a method carries both a // tool and an elicitation annotation: the generated source must emit the -// mcp.ElicitParams struct, the Mustache-rendered message expression, the -// accept-path guard, and the decline-path IsError short-circuit. +// multi-round-trip confirmation gate — an InputRequests result carrying +// mcp.ElicitParams on the first invocation, the inputResponses lookup on +// the retry — plus the Mustache-rendered message expression and the +// decline-path IsError short-circuit. func TestGenerate_Elicit(t *testing.T) { out := runGenerate(t, "elicit.proto") @@ -109,6 +111,15 @@ func TestGenerate_Elicit(t *testing.T) { {"register function", true, "RegisterElicitMCPTools"}, {"Delete tool name", true, `"Elicit_Delete"`}, {"ElicitParams struct literal", true, "&mcp.ElicitParams{"}, + // First invocation publishes the confirmation under the fixed + // server-assigned request ID. + {"InputRequests map literal", true, "InputRequests: mcp.InputRequestMap{"}, + // The retry reads the client's echoed answer back by the same key. + {"inputResponses lookup", true, `req.Params.InputResponses["confirm"]`}, + {"answer type assertion", true, "*mcp.ElicitResult"}, + // The old direct server-initiated request must be gone: it hard-fails + // on protocol >= 2026-07-28 sessions. + {"no direct Elicit call", false, ".Elicit(ctx"}, // The literal prefix up to the first Mustache var appears as a Go // string literal in the emitted Sprintf concatenation. {"rendered message prefix", true, `"Delete item with id "`}, diff --git a/internal/gen/templates/tool.go.tmpl b/internal/gen/templates/tool.go.tmpl index 1851e31..9598af9 100644 --- a/internal/gen/templates/tool.go.tmpl +++ b/internal/gen/templates/tool.go.tmpl @@ -33,16 +33,27 @@ g.Metadata.Set(srv.ProgressTokenHeader(), {{.QProtomcpSanitizeMetadataValue}}({{.QFmtSprintf}}("%v", tok))) } {{- if .Elicitation}} - // Confirm with the client before making the upstream call. - // A nil session is tolerated for unit-test harnesses. + // Confirm with the client before making the upstream call, via + // the multi-round-trip protocol (SEP-2322): the first invocation + // returns an elicitation InputRequest, and the SDK re-invokes + // this handler with the client's answer (client middleware on + // >= 2026-07-28 sessions, the SDK's server-side shim for older + // clients). A nil session is tolerated for unit-test harnesses. if req.Session != nil { - elicitResult, elicitErr := req.Session.Elicit(ctx, &{{.Elicitation.QMCPElicitParams}}{ - Message: {{.Elicitation.MessageExpr}}, - }) - if elicitErr != nil { - return srv.FinishToolCall(ctx, req, g, nil, elicitErr) + answer, answered := req.Params.InputResponses[{{printf "%q" .Elicitation.RequestID}}] + if !answered { + // Intermediate protocol result, not a tool result: skip + // FinishToolCall so ToolResultProcessors cannot attach + // Content, which the SDK rejects alongside InputRequests. + return &{{.QMCPCallResult}}{ + InputRequests: {{.Elicitation.QMCPInputRequestMap}}{ + {{printf "%q" .Elicitation.RequestID}}: &{{.Elicitation.QMCPElicitParams}}{ + Message: {{.Elicitation.MessageExpr}}, + }, + }, + }, nil, nil } - if elicitResult == nil || elicitResult.Action != "accept" { + if er, ok := answer.(*{{.Elicitation.QMCPElicitResult}}); !ok || er == nil || er.Action != "accept" { // Explicit refusal; the gRPC call does NOT run. return srv.FinishToolCall(ctx, req, g, &{{.QMCPCallResult}}{ IsError: true, diff --git a/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go b/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go index be5b23a..120125d 100644 --- a/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go +++ b/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go @@ -268,16 +268,27 @@ func RegisterTasksMCPTools(srv *protomcp.Server, client TasksClient) { if tok := req.Params.GetProgressToken(); tok != nil { g.Metadata.Set(srv.ProgressTokenHeader(), protomcp.SanitizeMetadataValue(fmt.Sprintf("%v", tok))) } - // Confirm with the client before making the upstream call. - // A nil session is tolerated for unit-test harnesses. + // Confirm with the client before making the upstream call, via + // the multi-round-trip protocol (SEP-2322): the first invocation + // returns an elicitation InputRequest, and the SDK re-invokes + // this handler with the client's answer (client middleware on + // >= 2026-07-28 sessions, the SDK's server-side shim for older + // clients). A nil session is tolerated for unit-test harnesses. if req.Session != nil { - elicitResult, elicitErr := req.Session.Elicit(ctx, &mcp.ElicitParams{ - Message: "Delete task with id " + fmt.Sprintf("%v", (&in).GetId()) + "? This cannot be undone.", - }) - if elicitErr != nil { - return srv.FinishToolCall(ctx, req, g, nil, elicitErr) - } - if elicitResult == nil || elicitResult.Action != "accept" { + answer, answered := req.Params.InputResponses["confirm"] + if !answered { + // Intermediate protocol result, not a tool result: skip + // FinishToolCall so ToolResultProcessors cannot attach + // Content, which the SDK rejects alongside InputRequests. + return &mcp.CallToolResult{ + InputRequests: mcp.InputRequestMap{ + "confirm": &mcp.ElicitParams{ + Message: "Delete task with id " + fmt.Sprintf("%v", (&in).GetId()) + "? This cannot be undone.", + }, + }, + }, nil, nil + } + if er, ok := answer.(*mcp.ElicitResult); !ok || er == nil || er.Action != "accept" { // Explicit refusal; the gRPC call does NOT run. return srv.FinishToolCall(ctx, req, g, &mcp.CallToolResult{ IsError: true, From 3511740f2a98e42d457c65e90cf6c9e9daf79874 Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 12:45:50 +0800 Subject: [PATCH 2/8] fix(server): keep cross-origin protection on by default under go-sdk v1.7.0 --- pkg/protomcp/server.go | 11 +++++ pkg/protomcp/server_http_test.go | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 pkg/protomcp/server_http_test.go diff --git a/pkg/protomcp/server.go b/pkg/protomcp/server.go index 36ade4f..7475534 100644 --- a/pkg/protomcp/server.go +++ b/pkg/protomcp/server.go @@ -159,6 +159,17 @@ func New(name, version string, opts ...ServerOption) *Server { s.httpInner = mcp.NewStreamableHTTPHandler(func(*http.Request) *mcp.Server { return s.sdk }, s.httpOpts) + // go-sdk v1.7.0 stopped installing cross-origin protection when + // StreamableHTTPOptions.CrossOriginProtection is nil, and deprecated + // the field in favor of wrapping the handler. Restore the v1.5.0 + // always-on default unless the caller supplied their own instance via + // WithHTTPOptions. Reading the deprecated field is the only way to + // detect that; when the SDK removes it, wrap the caller's instance + // here instead of passing it through. + callerCOP := s.httpOpts != nil && s.httpOpts.CrossOriginProtection != nil //nolint:staticcheck // SA1019: reading the deprecated field is the only way to detect a caller-supplied instance. + if !callerCOP { + s.httpInner = http.NewCrossOriginProtection().Handler(s.httpInner) + } return s } diff --git a/pkg/protomcp/server_http_test.go b/pkg/protomcp/server_http_test.go new file mode 100644 index 0000000..2d33a67 --- /dev/null +++ b/pkg/protomcp/server_http_test.go @@ -0,0 +1,84 @@ +package protomcp + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// postInitialize fires a raw initialize POST at the server, optionally +// tagged with a Sec-Fetch-Site header, and returns the status code. +func postInitialize(t *testing.T, url, secFetchSite string) int { + t.Helper() + body := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"c","version":"0.0.1"}}}` + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, url, bytes.NewBufferString(body)) + if err != nil { + t.Fatalf("new request: %v", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + if secFetchSite != "" { + req.Header.Set("Sec-Fetch-Site", secFetchSite) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("post: %v", err) + } + t.Cleanup(func() { _ = resp.Body.Close() }) + return resp.StatusCode +} + +// TestCrossOriginProtection_OnByDefault locks in the v1.5.0 behavior we +// restore on top of go-sdk v1.7.0: with no HTTP options at all, a +// cross-site POST is rejected while a same-origin one is served. The SDK +// stopped installing this protection itself in v1.7.0. +func TestCrossOriginProtection_OnByDefault(t *testing.T) { + s := New("t", "0.0.1") + ts := httptest.NewServer(s) + t.Cleanup(ts.Close) + + if got := postInitialize(t, ts.URL, "cross-site"); got != http.StatusForbidden { + t.Errorf("cross-site POST status = %d, want %d (cross-origin protection missing)", got, http.StatusForbidden) + } + if got := postInitialize(t, ts.URL, ""); got != http.StatusOK { + t.Errorf("same-origin POST status = %d, want %d", got, http.StatusOK) + } +} + +// TestCrossOriginProtection_UserHTTPOptsStillWrapped verifies that +// supplying unrelated StreamableHTTPOptions does not lose the default +// protection: only a caller-supplied CrossOriginProtection opts out. +func TestCrossOriginProtection_UserHTTPOptsStillWrapped(t *testing.T) { + s := New("t", "0.0.1", + WithHTTPOptions(&mcp.StreamableHTTPOptions{JSONResponse: true}), + ) + ts := httptest.NewServer(s) + t.Cleanup(ts.Close) + + if got := postInitialize(t, ts.URL, "cross-site"); got != http.StatusForbidden { + t.Errorf("cross-site POST status = %d, want %d (wrap dropped by unrelated HTTP options)", got, http.StatusForbidden) + } +} + +// TestCrossOriginProtection_CallerSuppliedWins verifies the opt-out: a +// caller-supplied CrossOriginProtection passes through to the SDK +// untouched, so a permissive policy admits cross-site requests. +func TestCrossOriginProtection_CallerSuppliedWins(t *testing.T) { + permissive := http.NewCrossOriginProtection() + permissive.AddInsecureBypassPattern("/") + s := New("t", "0.0.1", + WithHTTPOptions(&mcp.StreamableHTTPOptions{ + CrossOriginProtection: permissive, + }), + ) + ts := httptest.NewServer(s) + t.Cleanup(ts.Close) + + if got := postInitialize(t, ts.URL, "cross-site"); got != http.StatusOK { + t.Errorf("cross-site POST status = %d, want %d (caller-supplied protection not honored)", got, http.StatusOK) + } +} From f1dffad4c23337443362cef500449e07da22d8d4 Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 12:45:50 +0800 Subject: [PATCH 3/8] docs: update elicitation, subscriptions, and HTTP semantics for go-sdk v1.7.0 --- AGENTS.md | 2 ++ CONTRIBUTING.md | 2 +- README.md | 32 ++++++++++++++++++++++++-------- examples/subscriptions/README.md | 14 ++++++++++++-- 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index afc239d..57cd054 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,6 +51,8 @@ LSP diagnostics go stale after `buf generate`; trust `go build` / `go test` over - **proto message copy-locks.** Generated message types embed `protoimpl.MessageState` which contains a `sync.Mutex`. Don't struct-copy them (`c := *t`); use `proto.Clone`. - **`paths=source_relative`.** Our `buf.gen.yaml` uses `paths=source_relative`, meaning the output directory mirrors the proto source tree. Do not change without updating every `go_package` option and every import path. - **protojson vs json.** MCP tool content is protojson. Use `protojson.Unmarshal` in tests that decode into generated proto types, plain `json.Unmarshal` breaks on Timestamp, Duration, enum-as-name, and int64-as-string. +- **Elicitation is multi-round-trip (SEP-2322).** Generated elicitation-gated handlers run **twice** per confirmed call: first invocation returns an `InputRequests` result, the retry carries the answer in `req.Params.InputResponses["confirm"]`. A harness that invokes a generated handler directly with a non-nil session must supply that map entry or it will get the input-required result, not the tool result. +- **Subscription registration is asynchronous on protocol ≥ 2026-07-28.** The client's `subscriptions/listen` call dispatches without awaiting a response, so `Subscribe`/`Connect` return before the server has recorded the subscription. Tests that connect and immediately mutate can miss notifications; poll or wait for a first event instead of assuming registration completed. ## Where design decisions live diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fb3ae1f..1b7451b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -130,7 +130,7 @@ Any deviation is a hard error citing the `service.method`. No fallbacks. ### 7. Schema mismatches are dev-time failures -`AddTool` is called with both input and output schemas. The SDK validates responses against the output schema before delivering them, a proto server returning a response that fails our generated schema is a codegen bug we want the test suite to catch, not something to silence. +`AddTool` is called with both input and output schemas. The SDK validates responses against the output schema before delivering them, a proto server returning a response that fails our generated schema is a codegen bug we want the test suite to catch, not something to silence. The one exception is by SDK design: input-required results (multi-round-trip `InputRequests`, as emitted by the generated elicitation gate) skip output-schema validation entirely — they are protocol intermediates, not tool results. ## Style diff --git a/README.md b/README.md index 7b5870a..5921419 100644 --- a/README.md +++ b/README.md @@ -262,7 +262,7 @@ Full runnable version: [`examples/greeter/cmd/greeter/main.go`](examples/greeter | **Resource (list_changed)** | The server, when the resource list mutates | Background watcher over a server-streaming RPC → `notifications/resources/list_changed` per received event (SDK debounces bursts) | `(protomcp.v1.resource_list_changed) = {}` on a server-streaming RPC | | **Resource (subscribe)** | The user, asking the client to track a URI | User-wired `SubscribeHandler` / `UnsubscribeHandler` + `srv.SDK().ResourceUpdated(...)` | *(no annotation, see [Resource subscriptions](#resource-subscriptions-are-user-wired-not-an-annotation))* | | **Prompt** | The user, picking a slash-command | `prompts/get` renders a Mustache template against the gRPC response | `(protomcp.v1.prompt) = {…}` | -| **Elicitation** | (modifier) the server, mid-tool-call, asking the user to confirm | `session.Elicit(...)` gate before the gRPC call runs | `(protomcp.v1.elicitation) = {…}` *(requires a `tool` on the same RPC)* | +| **Elicitation** | (modifier) the server, mid-tool-call, asking the user to confirm | Multi-round-trip confirmation `InputRequest` (SEP-2322) before the gRPC call runs; the SDK shims legacy clients transparently | `(protomcp.v1.elicitation) = {…}` *(requires a `tool` on the same RPC)* | Multiple primitives on one RPC are legal and additive. A `GetTask` RPC annotated with `tool` + `resource_template` becomes both a tool the LLM can invoke and a URI the user can attach. @@ -363,6 +363,8 @@ Lifecycle is explicit via `ctx` (not `Server.Close()`-driven) so it mirrors `htt **Implementation note.** The MCP Go SDK currently only fires `notifications/resources/list_changed` as a side effect of mutating its static resource registry. `Server.NotifyResourceListChanged()` triggers it by adding then immediately removing a sentinel resource template (`protomcp-internal-list-changed-trigger://{_}`); the two SDK calls coalesce into one wire notification via the 10 ms debounce. A client that runs `resources/templates/list` in the sub-millisecond window between the add and remove could observe the sentinel, which is a documented caveat of the current workaround. +**Delivery on protocol ≥ 2026-07-28.** Legacy sessions (anything negotiated through `initialize`) receive list-changed notifications unconditionally. Sessions on protocol revision 2026-07-28 or later receive them **only** over a `subscriptions/listen` stream that opted into the matching notification type. The go-sdk client opens that stream automatically when a `ResourceListChangedHandler` (or the tools/prompts equivalent) is registered — so a 2026-07-28 client with **no such handler silently stops receiving `NotifyResourceListChanged`** where a legacy client would still have seen the wire notification. Register the handler client-side if you depend on the signal. + ### Resource subscriptions are user-wired, not an annotation protomcp deliberately does not codegen subscribe handlers. MCP's `resources/subscribe` is a per-URI fanout the server delivers to every interested session; gRPC server-streaming is the opposite shape, a per-request stream owned by one caller. The two models do not map cleanly, and real backends also deliver change events from places that are not gRPC streams at all (pub/sub, CDC, polling, webhooks). An annotation would pick a wrong default for most users. @@ -371,13 +373,15 @@ The MCP Go SDK already handles the mechanics. Supply `ServerOptions.SubscribeHan The subscribe/unsubscribe handlers only act as a gate: the SDK calls them first (`return err` to reject, `return nil` to allow), and on allow unconditionally records the session in its subscriptions map. `ResourceUpdated` always reads from that same map, so the same fan-out works with either no-op handlers or custom ones. The handler type decides **which URIs are accepted** and **what extra work runs on subscribe/unsubscribe** (starting an upstream feed, cleaning it up), not whether `ResourceUpdated` delivers. +How a subscription arrives depends on the negotiated protocol revision. Legacy sessions call `resources/subscribe` / `resources/unsubscribe` directly. Sessions on revision 2026-07-28 or later use a long-lived `subscriptions/listen` stream instead (`resources/subscribe` itself is rejected there) — but the SDK routes both paths through the same internal subscription machinery, so **your `SubscribeHandler`/`UnsubscribeHandler` still fire per URI either way**, and `ResourceUpdated` delivers over whichever channel the session holds. The go-sdk client picks the right mechanism transparently. + There are two common shapes: 1. **Push from the write path (no-op handlers).** Supply `func(...) error { return nil }` for both handlers and call `ResourceUpdated` from wherever mutations happen. Best when events originate inside the same process as the MCP server. 2. **Wrap an external source.** Real handlers that start/stop upstream delivery per subscription (open a gRPC stream, run a PG `LISTEN`, subscribe to a Redis/Kafka topic, register a webhook). Best when events come from outside the process. Still call `ResourceUpdated` on each incoming event. -[`examples/subscriptions`](examples/subscriptions) ships both as runnable demos: `cmd/subscriptions-simple` for the push-from-write-path pattern (about 30 lines of wiring) and `cmd/subscriptions` for the external-source pattern with a reusable `Hub` + `Manager` and race-tested session-close cleanup. +[`examples/subscriptions`](examples/subscriptions) ships both as runnable demos: `cmd/subscriptions-simple` for the push-from-write-path pattern (about 30 lines of wiring) and `cmd/subscriptions` for the external-source pattern, with per-principal subscribe authorization and a watcher wrapped in `protomcp.RetryLoop`. ### `protomcp.v1.prompt`, method option @@ -395,7 +399,11 @@ Enum-typed and `buf.validate.string.in`-constrained prompt arguments automatical | Field | Required? | Effect | |---|---|---| -| `message` | required | Mustache template over the tool's **request** message; rendered into the confirmation prompt. `session.Elicit(...)` runs before the gRPC call, any non-accept action returns `IsError: true` and the tool never executes. | +| `message` | required | Mustache template over the tool's **request** message; rendered into the confirmation prompt shown to the user before the gRPC call runs. Any non-accept answer returns `IsError: true` and the tool never executes. | + +The gate uses the multi-round-trip protocol (SEP-2322). The generated handler's first invocation returns a `CallToolResult` carrying a single elicitation `InputRequest` under the fixed key `"confirm"` — not a tool result. The SDK then obtains the user's answer and re-invokes the handler with it echoed in `inputResponses`; only an `action == "accept"` answer lets the gRPC call run. On sessions speaking protocol ≥ 2026-07-28 the client's middleware fulfills the request through its `ElicitationHandler` and retries automatically; on older sessions the SDK's server-side shim performs a classic `elicitation/create` round-trip and re-invokes the handler in place. One annotation serves both. Note the handler body runs twice per confirmed call. + +**Clients must register an `ElicitationHandler`.** A client without one fails the tool call with a hard JSON-RPC error (`client does not support elicitation`) — the error never reaches your `ToolErrorHandler`, and there is no `IsError` result for the LLM to read. (Under go-sdk v1.5.0 this case surfaced as a graceful `IsError` tool result instead.) Hard codegen error when used without a companion `tool`, or on a streaming RPC. @@ -450,7 +458,7 @@ Each example is standalone, runnable, and has its own README. |---|---| | [`examples/greeter`](examples/greeter) | Tool primitive surface, unary + server-streaming RPCs, progress notifications with monotonic counter, **progress-token gRPC-metadata propagation**, `ToolErrorHandler`, `ToolResultProcessor` redaction, `ToolMiddleware` request mutation, SDK options pass-through, **`field_schema.exclude` schema masking round-trip** | | [`examples/tasks`](examples/tasks) | **Every declarative MCP primitive end-to-end.** Tools with `read_only` / `idempotent` / `destructive` hints + `OUTPUT_ONLY` stripping, **two `resource_template` annotations (`tasks://{id}`, `tags://{id}`)**, **a single `resource_list` that enumerates both types via `{type}://{id}` with `OffsetPagination`**, **prompts (`tasks_review`)**, **elicitation (confirm `DeleteTask`)**, plus `@example` markers and `enumDescriptions` on `TaskStatus` | -| [`examples/subscriptions`](examples/subscriptions) | **User-wired resource subscriptions** on top of the Tasks resource template. In-process `Hub` + `Manager` + `SubscribeHandler`/`UnsubscribeHandler` + `ss.Wait()` session-close watchdog. Race-tested. | +| [`examples/subscriptions`](examples/subscriptions) | **User-wired resource subscriptions** on top of the Tasks resource template. Per-principal subscribe authorization in `SubscribeHandler`/`UnsubscribeHandler`, plus a watcher wrapped in `protomcp.RetryLoop` pushing `ResourceUpdated`. Race-tested. | | [`examples/auth`](examples/auth) | Two-layer auth: SDK-native bearer middleware **or** custom HTTP middleware, both writing gRPC metadata for the upstream | Cmd directories inside each example hold the runnable binaries: @@ -462,7 +470,7 @@ Cmd directories inside each example hold the runnable binaries: - [`examples/greeter/cmd/errorhandler`](examples/greeter/cmd/errorhandler), custom `ErrorHandler` - [`examples/greeter/cmd/sdkopts`](examples/greeter/cmd/sdkopts), pass `mcp.ServerOptions` / `mcp.StreamableHTTPOptions` - [`examples/tasks/cmd/tasks`](examples/tasks/cmd/tasks), CRUD -- [`examples/subscriptions/cmd/subscriptions`](examples/subscriptions/cmd/subscriptions), Hub-driven subscribe wiring +- [`examples/subscriptions/cmd/subscriptions`](examples/subscriptions/cmd/subscriptions), authorization-gated subscribe wiring - [`examples/auth/cmd/auth`](examples/auth/cmd/auth), custom HTTP middleware → ctx → metadata - [`examples/auth/cmd/sdkauth`](examples/auth/cmd/sdkauth), MCP Go SDK's `auth.RequireBearerToken` → `TokenInfoFromContext` → metadata @@ -564,6 +572,8 @@ func injectTenant(next protomcp.ToolHandler) protomcp.ToolHandler { Override with `protomcp.WithToolErrorHandler(...)`, mirrors `grpc-gateway`'s `runtime.WithErrorHandler` but produces JSON-RPC shapes. +**Error-code landscape (go-sdk ≥ v1.7.0).** protomcp's own JSON-RPC codes are unchanged: `Unauthenticated → -32001`, `PermissionDenied → -32002`, `Canceled → -32003`, `DeadlineExceeded → -32004` (all in the implementation-defined `-32000..-32019` range). The SDK's resource-not-found error moved from `-32002` to `-32602` (`mcp.CodeResourceNotFound` is now a deprecated `var` aliasing `jsonrpc.CodeInvalidParams`), so `-32002` no longer collides with any SDK response — but `-32602` is now shared between the SDK's resource-not-found / invalid-params errors and protomcp's invalid-cursor mapping. protomcp does not set the `MCPGODEBUG=customresnotfounderrcode` escape hatch. + ### ResultProcessor, mutate responses before the client sees them ```go @@ -609,7 +619,7 @@ srv = protomcp.New("svc", "0.1.0", ) ``` -An invalid client-supplied cursor surfaces as JSON-RPC error `-32602` (Invalid params). Callers that build their own middleware can return `&protomcp.InvalidCursorError{Err: …}` to get the same mapping. +An invalid client-supplied cursor surfaces as JSON-RPC error `-32602` (Invalid params). Callers that build their own middleware can return `&protomcp.InvalidCursorError{Err: …}` to get the same mapping. Note that since go-sdk v1.7.0 the SDK also uses `-32602` for its own resource-not-found and malformed-params errors, so clients cannot distinguish those cases from an invalid cursor by code alone. The runnable `examples/tasks/cmd/tasks` command wires `OffsetPagination` (default `page-size=3`) so `resources/list` pages live; `examples/tasks/e2e_resources_test.go` asserts the three-page round-trip. @@ -716,6 +726,12 @@ srv := protomcp.New("svc", "0.1.0", ) ``` +**Cross-origin protection.** go-sdk v1.7.0 stopped installing cross-origin protection when `StreamableHTTPOptions.CrossOriginProtection` is nil (and deprecated the field). protomcp keeps the protection **on by default** by wrapping its HTTP handler with `http.NewCrossOriginProtection()`. To customize or relax the policy, supply your own `CrossOriginProtection` via `WithHTTPOptions` — a caller-supplied instance passes through to the SDK untouched and disables protomcp's wrap. (The SDK's own escape hatch for the old default is `MCPGODEBUG=enableoriginverification=1`; protomcp does not rely on it.) + +**Request body limit.** Since go-sdk v1.7.0 the streamable HTTP transport caps request bodies at 4 MiB by default (`mcp.DefaultMaxRequestBodyBytes`), returning HTTP 413 above the limit. Raise or disable it via `StreamableHTTPOptions.MaxRequestBodyBytes` (negative disables). + +**`MCPGODEBUG` flags.** go-sdk v1.7.0 gates several behavior changes behind environment escape hatches (`enableoriginverification`, `allowsessionsinstateless`, `hintomitempty`, `customresnotfounderrcode`, `nowrapinvalidparams`, `noprotocolerrorbody`, `disablecontenttypecheck`, `disablecompleteparamsvalidation`, `disablelocalhostprotection`). protomcp relies on **none** of them — they are temporary and slated for removal in later SDK releases. + --- ## Scope & limitations @@ -846,10 +862,10 @@ We surveyed every Go-based proto → MCP project we could find before starting. | **`google.api.field_behavior`** | `REQUIRED` + `OUTPUT_ONLY` (recursive runtime clear) | `REQUIRED` only | ❌ (via `buf.validate.required` only) | `REQUIRED` + `OUTPUT_ONLY` (codegen only, no runtime clear) | | **Resources (templates + list)** | ✅ `resource_template` (multiple per server, served via `resources/templates/list`) + single `resource_list` with `{type}://{id}`-style multi-type enumeration; `OffsetPagination` / `PageTokenPagination` helpers | ❌ | ❌ | ❌ | | **Resources (list_changed)** | ✅ `resource_list_changed` annotation on a server-streaming RPC → auto-generated reconnecting watcher that fires `notifications/resources/list_changed` per backend event (SDK debounces) | ❌ | ❌ | ❌ | -| **Resources (subscribe)** | user-wired via `SubscribeHandler` / `UnsubscribeHandler` + `srv.SDK().ResourceUpdated(...)`, same SDK surface the upstream Go SDK exposes, with a reference `Hub` + `Manager` + session-close watchdog in [`examples/subscriptions`](examples/subscriptions) | ❌ | ❌ | ❌ | +| **Resources (subscribe)** | user-wired via `SubscribeHandler` / `UnsubscribeHandler` + `srv.SDK().ResourceUpdated(...)`, same SDK surface the upstream Go SDK exposes, with reference authorization-gated wiring in [`examples/subscriptions`](examples/subscriptions) | ❌ | ❌ | ❌ | | **Prompts** | ✅ `prompt` annotation with Mustache template; `prompts/list` + `prompts/get` | ❌ | ❌ | ❌ | | **Prompt argument completion** | ✅ auto-wired for enum and `buf.validate.string.in` args | ❌ | ❌ | ❌ | -| **Elicitation** | ✅ `elicitation` modifier; `session.Elicit()` gate before tool execution | ❌ | ❌ | ❌ | +| **Elicitation** | ✅ `elicitation` modifier; multi-round-trip confirmation `InputRequest` (SEP-2322) before tool execution, SDK-shimmed for legacy clients | ❌ | ❌ | ❌ | | **Progress-token metadata forwarding** | ✅ `mcp-progress-token` forwarded as outgoing gRPC metadata | ❌ | ❌ | ❌ | | **`@example` schema hints** | ✅ from proto `// @example ` comment markers | ❌ | ❌ | ❌ | | **`enumDescriptions` schema hints** | ✅ from enum-value leading comments | ❌ | ❌ | ❌ | diff --git a/examples/subscriptions/README.md b/examples/subscriptions/README.md index 7b5d3f4..1683922 100644 --- a/examples/subscriptions/README.md +++ b/examples/subscriptions/README.md @@ -25,14 +25,24 @@ internally and fans each `ResourceUpdated` call out to only the sessions that asked for that URI. **The subscribe/unsubscribe handlers only act as a gate.** When a -`resources/subscribe` request arrives, the SDK calls your handler -first (`return err` to reject, `return nil` to allow); on allow, it +subscription request arrives, the SDK calls your handler first +(`return err` to reject, `return nil` to allow); on allow, it **unconditionally** records the session in its internal subscriptions map. `ResourceUpdated` always reads from that same map, so it fans out to subscribed sessions regardless of whether your handlers are no-ops or doing real upstream work. The handler type decides **which URIs are accepted**, not whether fan-out happens. +**How subscriptions arrive depends on the protocol revision.** Legacy +sessions (negotiated through `initialize`, up to 2025-11-25) send +`resources/subscribe` / `resources/unsubscribe` requests. Sessions on +revision 2026-07-28 or later hold a long-lived `subscriptions/listen` +stream instead, and `resources/subscribe` is rejected there — but the +SDK routes each listed URI through the same internal subscribe path, +so your `SubscribeHandler`/`UnsubscribeHandler` fire per URI on both +paths and `ResourceUpdated` delivers over whichever channel the +session holds. The go-sdk client picks the mechanism transparently. + ## Pick the pattern that matches your event source ### Pattern A: push from the write path (simpler, more common) From b932ee3c6b6ca705aff0f356ef256539e0d47cf5 Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 14:05:45 +0800 Subject: [PATCH 4/8] fix(gen): bind elicitation answers to the originating call via RequestState --- examples/tasks/e2e_elicitation_legacy_test.go | 9 ++ examples/tasks/e2e_elicitation_test.go | 95 +++++++++++++++++++ internal/gen/generator.go | 19 +++- internal/gen/generator_test.go | 12 ++- internal/gen/templates/tool.go.tmpl | 45 +++++---- pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go | 45 +++++---- pkg/protomcp/elicitation.go | 26 +++++ 7 files changed, 200 insertions(+), 51 deletions(-) create mode 100644 pkg/protomcp/elicitation.go diff --git a/examples/tasks/e2e_elicitation_legacy_test.go b/examples/tasks/e2e_elicitation_legacy_test.go index 9514d53..456a291 100644 --- a/examples/tasks/e2e_elicitation_legacy_test.go +++ b/examples/tasks/e2e_elicitation_legacy_test.go @@ -41,6 +41,15 @@ func connectLegacyHTTP(ctx context.Context, t *testing.T, srv *protomcp.Server, t.Helper() ts := httptest.NewServer(srv) t.Cleanup(ts.Close) + // Disable the client-side multi-round-trip middleware: it fires on + // any inputRequests result regardless of protocol version, so left + // enabled it would fulfill the elicitation itself and mask a + // server-side shim regression — exactly what this suite exists to + // catch. With it off, only the shim can complete the round-trip. + if opts == nil { + opts = &mcp.ClientOptions{} + } + opts.MultiRoundTrip = &mcp.MultiRoundTripOptions{Disabled: true} client := mcp.NewClient(&mcp.Implementation{Name: "legacy-test-client", Version: "0.0.1"}, opts) cs, err := client.Connect(ctx, &mcp.StreamableClientTransport{Endpoint: ts.URL}, nil) if err != nil { diff --git a/examples/tasks/e2e_elicitation_test.go b/examples/tasks/e2e_elicitation_test.go index 3cb6e5b..700a5d0 100644 --- a/examples/tasks/e2e_elicitation_test.go +++ b/examples/tasks/e2e_elicitation_test.go @@ -148,6 +148,101 @@ func TestDeleteTask_ElicitationDecline(t *testing.T) { } } +// TestDeleteTask_ElicitationReusedParamsRePrompts is the regression test +// for answer/request binding. The SDK's client middleware mutates the +// caller's CallToolParams in place on a fulfilled elicitation +// (setMultiRoundTripRetryParams), so a client that reuses one params +// struct across calls carries a stale inputResponses["confirm"] into the +// next call. Without RequestState binding that stale answer silently +// confirmed a delete of a *different* task; with it, every distinct call +// re-prompts. +func TestDeleteTask_ElicitationReusedParamsRePrompts(t *testing.T) { + ctx := context.Background() + grpcClient := startGRPC(t) + srv := protomcp.New("tasks", "0.1.0") + tasksv1.RegisterTasksMCPTools(srv, grpcClient) + + var elicitCalls atomic.Int32 + cs := connectWith(ctx, t, srv, &mcp.ClientOptions{ + ElicitationHandler: func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + elicitCalls.Add(1) + return &mcp.ElicitResult{Action: "accept"}, nil + }, + }) + + var first, second tasksv1.Task + callTool(ctx, t, cs, "Tasks_CreateTask", `{"task":{"title":"one","done":false}}`, &first) + callTool(ctx, t, cs, "Tasks_CreateTask", `{"task":{"title":"two","done":false}}`, &second) + + // One params struct, reused across both deletes: after the first + // call the SDK has stuffed InputResponses + RequestState into it. + params := &mcp.CallToolParams{ + Name: "Tasks_DeleteTask", + Arguments: json.RawMessage(fmt.Sprintf(`{"id":%q}`, first.Id)), + } + if out, err := cs.CallTool(ctx, params); err != nil || out.IsError { + t.Fatalf("first Delete: err=%v out=%+v", err, out) + } + if got := elicitCalls.Load(); got != 1 { + t.Fatalf("after first delete: elicitation handler called %d times, want 1", got) + } + + params.Arguments = json.RawMessage(fmt.Sprintf(`{"id":%q}`, second.Id)) + if out, err := cs.CallTool(ctx, params); err != nil || out.IsError { + t.Fatalf("second Delete: err=%v out=%+v", err, out) + } + if got := elicitCalls.Load(); got != 2 { + t.Errorf("after second delete: elicitation handler called %d times, want 2 (stale answer must re-prompt, not confirm)", got) + } +} + +// TestDeleteTask_ElicitationPrePopulatedAnswerRePrompts asserts that an +// inputResponses entry supplied on the very first call, without the +// matching RequestState, does not skip the confirmation: the gate +// re-prompts, and a declining user still blocks the delete. +func TestDeleteTask_ElicitationPrePopulatedAnswerRePrompts(t *testing.T) { + ctx := context.Background() + grpcClient := startGRPC(t) + srv := protomcp.New("tasks", "0.1.0") + tasksv1.RegisterTasksMCPTools(srv, grpcClient) + + var elicitCalls atomic.Int32 + cs := connectWith(ctx, t, srv, &mcp.ClientOptions{ + ElicitationHandler: func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + elicitCalls.Add(1) + return &mcp.ElicitResult{Action: "decline"}, nil + }, + }) + + var created tasksv1.Task + callTool(ctx, t, cs, "Tasks_CreateTask", `{"task":{"title":"keep-me","done":false}}`, &created) + + out, err := cs.CallTool(ctx, &mcp.CallToolParams{ + Name: "Tasks_DeleteTask", + Arguments: json.RawMessage(fmt.Sprintf(`{"id":%q}`, created.Id)), + InputResponses: mcp.InputResponseMap{ + "confirm": &mcp.ElicitResult{Action: "accept"}, + }, + }) + if err != nil { + t.Fatalf("CallTool: transport error: %v", err) + } + if !out.IsError { + t.Fatalf("Delete: want IsError (user declined the re-prompt), got success: %+v", out) + } + if got := elicitCalls.Load(); got != 1 { + t.Errorf("elicitation handler called %d times, want 1 (pre-populated answer must trigger a real prompt)", got) + } + + // Task survived. + var got tasksv1.Task + callTool(ctx, t, cs, "Tasks_GetTask", + fmt.Sprintf(`{"id":%q}`, created.Id), &got) + if got.Id != created.Id { + t.Errorf("Get after blocked delete: got %q, want %q", got.Id, created.Id) + } +} + // TestDeleteTask_ElicitationCancel asserts that action=cancel behaves the // same as decline, it is a non-accept action, so the tool must // short-circuit with IsError and leave the backend untouched. diff --git a/internal/gen/generator.go b/internal/gen/generator.go index 0ef2887..d8ffcbd 100644 --- a/internal/gen/generator.go +++ b/internal/gen/generator.go @@ -154,6 +154,10 @@ type elicitationTemplateData struct { QMCPElicitParams string QMCPInputRequestMap string QMCPElicitResult string + + // QProtomcpElicitationState qualifies the runtime helper that binds + // an elicitation answer to the tool call that prompted it. + QProtomcpElicitationState string } // commonQuals bundles qualified identifiers every template site needs. @@ -755,12 +759,17 @@ func buildElicitationTemplateData( GoName: "ElicitResult", GoImportPath: importMCP, }) + qElicitationState := g.QualifiedGoIdent(protogen.GoIdent{ + GoName: "ElicitationState", + GoImportPath: importProtomcp, + }) return &elicitationTemplateData{ - MessageExpr: expr, - RequestID: elicitationRequestID, - QMCPElicitParams: qElicitParams, - QMCPInputRequestMap: qInputRequestMap, - QMCPElicitResult: qElicitResult, + MessageExpr: expr, + RequestID: elicitationRequestID, + QMCPElicitParams: qElicitParams, + QMCPInputRequestMap: qInputRequestMap, + QMCPElicitResult: qElicitResult, + QProtomcpElicitationState: qElicitationState, }, nil } diff --git a/internal/gen/generator_test.go b/internal/gen/generator_test.go index d7e5b30..e5ae8b8 100644 --- a/internal/gen/generator_test.go +++ b/internal/gen/generator_test.go @@ -112,14 +112,18 @@ func TestGenerate_Elicit(t *testing.T) { {"Delete tool name", true, `"Elicit_Delete"`}, {"ElicitParams struct literal", true, "&mcp.ElicitParams{"}, // First invocation publishes the confirmation under the fixed - // server-assigned request ID. + // server-assigned request ID, bound to the call via RequestState. {"InputRequests map literal", true, "InputRequests: mcp.InputRequestMap{"}, - // The retry reads the client's echoed answer back by the same key. + {"request state derived from call", true, "protomcp.ElicitationState("}, + {"request state on the result", true, "RequestState: elicitState"}, + // The retry reads the client's echoed answer back by the same key + // and requires the echoed state to match. {"inputResponses lookup", true, `req.Params.InputResponses["confirm"]`}, - {"answer type assertion", true, "*mcp.ElicitResult"}, + {"request state verified on retry", true, "req.Params.RequestState != elicitState"}, + {"answer type assertion", true, "elicitAnswer.(*mcp.ElicitResult)"}, // The old direct server-initiated request must be gone: it hard-fails // on protocol >= 2026-07-28 sessions. - {"no direct Elicit call", false, ".Elicit(ctx"}, + {"no direct Elicit call", false, "Session.Elicit("}, // The literal prefix up to the first Mustache var appears as a Go // string literal in the emitted Sprintf concatenation. {"rendered message prefix", true, `"Delete item with id "`}, diff --git a/internal/gen/templates/tool.go.tmpl b/internal/gen/templates/tool.go.tmpl index 9598af9..7b7fea8 100644 --- a/internal/gen/templates/tool.go.tmpl +++ b/internal/gen/templates/tool.go.tmpl @@ -38,28 +38,31 @@ // returns an elicitation InputRequest, and the SDK re-invokes // this handler with the client's answer (client middleware on // >= 2026-07-28 sessions, the SDK's server-side shim for older - // clients). A nil session is tolerated for unit-test harnesses. - if req.Session != nil { - answer, answered := req.Params.InputResponses[{{printf "%q" .Elicitation.RequestID}}] - if !answered { - // Intermediate protocol result, not a tool result: skip - // FinishToolCall so ToolResultProcessors cannot attach - // Content, which the SDK rejects alongside InputRequests. - return &{{.QMCPCallResult}}{ - InputRequests: {{.Elicitation.QMCPInputRequestMap}}{ - {{printf "%q" .Elicitation.RequestID}}: &{{.Elicitation.QMCPElicitParams}}{ - Message: {{.Elicitation.MessageExpr}}, - }, + // clients). RequestState binds the answer to this exact tool + // call: a stale or replayed answer (e.g. from a CallToolParams + // the SDK mutated in place on an earlier call) re-prompts + // instead of silently confirming. + elicitState := {{.Elicitation.QProtomcpElicitationState}}({{printf "%q" .ToolName}}, raw) + elicitAnswer, elicitAnswered := req.Params.InputResponses[{{printf "%q" .Elicitation.RequestID}}] + if !elicitAnswered || req.Params.RequestState != elicitState { + // Intermediate protocol result, not a tool result: skip + // FinishToolCall so ToolResultProcessors cannot attach + // Content, which the SDK rejects alongside InputRequests. + return &{{.QMCPCallResult}}{ + RequestState: elicitState, + InputRequests: {{.Elicitation.QMCPInputRequestMap}}{ + {{printf "%q" .Elicitation.RequestID}}: &{{.Elicitation.QMCPElicitParams}}{ + Message: {{.Elicitation.MessageExpr}}, }, - }, nil, nil - } - if er, ok := answer.(*{{.Elicitation.QMCPElicitResult}}); !ok || er == nil || er.Action != "accept" { - // Explicit refusal; the gRPC call does NOT run. - return srv.FinishToolCall(ctx, req, g, &{{.QMCPCallResult}}{ - IsError: true, - Content: []{{.QMCPContent}}{&{{.QMCPTextContent}}{Text: "User declined to proceed."}}, - }, nil) - } + }, + }, nil, nil + } + if er, ok := elicitAnswer.(*{{.Elicitation.QMCPElicitResult}}); !ok || er == nil || er.Action != "accept" { + // Explicit refusal; the gRPC call does NOT run. + return srv.FinishToolCall(ctx, req, g, &{{.QMCPCallResult}}{ + IsError: true, + Content: []{{.QMCPContent}}{&{{.QMCPTextContent}}{Text: "User declined to proceed."}}, + }, nil) } {{- end}} {{if .IsServerStreaming}} diff --git a/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go b/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go index 120125d..4f38c06 100644 --- a/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go +++ b/pkg/api/gen/examples/tasks/v1/tasks.mcp.pb.go @@ -273,28 +273,31 @@ func RegisterTasksMCPTools(srv *protomcp.Server, client TasksClient) { // returns an elicitation InputRequest, and the SDK re-invokes // this handler with the client's answer (client middleware on // >= 2026-07-28 sessions, the SDK's server-side shim for older - // clients). A nil session is tolerated for unit-test harnesses. - if req.Session != nil { - answer, answered := req.Params.InputResponses["confirm"] - if !answered { - // Intermediate protocol result, not a tool result: skip - // FinishToolCall so ToolResultProcessors cannot attach - // Content, which the SDK rejects alongside InputRequests. - return &mcp.CallToolResult{ - InputRequests: mcp.InputRequestMap{ - "confirm": &mcp.ElicitParams{ - Message: "Delete task with id " + fmt.Sprintf("%v", (&in).GetId()) + "? This cannot be undone.", - }, + // clients). RequestState binds the answer to this exact tool + // call: a stale or replayed answer (e.g. from a CallToolParams + // the SDK mutated in place on an earlier call) re-prompts + // instead of silently confirming. + elicitState := protomcp.ElicitationState("Tasks_DeleteTask", raw) + elicitAnswer, elicitAnswered := req.Params.InputResponses["confirm"] + if !elicitAnswered || req.Params.RequestState != elicitState { + // Intermediate protocol result, not a tool result: skip + // FinishToolCall so ToolResultProcessors cannot attach + // Content, which the SDK rejects alongside InputRequests. + return &mcp.CallToolResult{ + RequestState: elicitState, + InputRequests: mcp.InputRequestMap{ + "confirm": &mcp.ElicitParams{ + Message: "Delete task with id " + fmt.Sprintf("%v", (&in).GetId()) + "? This cannot be undone.", }, - }, nil, nil - } - if er, ok := answer.(*mcp.ElicitResult); !ok || er == nil || er.Action != "accept" { - // Explicit refusal; the gRPC call does NOT run. - return srv.FinishToolCall(ctx, req, g, &mcp.CallToolResult{ - IsError: true, - Content: []mcp.Content{&mcp.TextContent{Text: "User declined to proceed."}}, - }, nil) - } + }, + }, nil, nil + } + if er, ok := elicitAnswer.(*mcp.ElicitResult); !ok || er == nil || er.Action != "accept" { + // Explicit refusal; the gRPC call does NOT run. + return srv.FinishToolCall(ctx, req, g, &mcp.CallToolResult{ + IsError: true, + Content: []mcp.Content{&mcp.TextContent{Text: "User declined to proceed."}}, + }, nil) } final := func(ctx context.Context, _ *mcp.CallToolRequest, g *protomcp.GRPCData) (*mcp.CallToolResult, error) { diff --git a/pkg/protomcp/elicitation.go b/pkg/protomcp/elicitation.go new file mode 100644 index 0000000..f32482e --- /dev/null +++ b/pkg/protomcp/elicitation.go @@ -0,0 +1,26 @@ +package protomcp + +import ( + "crypto/sha256" + "encoding/hex" +) + +// ElicitationState derives the opaque RequestState value that binds a +// multi-round-trip elicitation answer to the exact tool call that +// prompted it. Generated elicitation gates publish it on the +// input-required result and require the retry to echo it back alongside +// a matching answer; a stale or replayed answer (for example from a +// CallToolParams struct the SDK's client middleware mutated in place on +// an earlier call) therefore re-prompts instead of silently confirming. +// +// The value is a plain content hash, not an authenticator: a client +// willing to lie about the user's answer can compute it. Elicitation is +// a UX confirmation for honest clients, not a server-enforced +// authorization control — enforce authorization server-side. +func ElicitationState(toolName string, rawArgs []byte) string { + h := sha256.New() + h.Write([]byte(toolName)) + h.Write([]byte{0}) + h.Write(rawArgs) + return hex.EncodeToString(h.Sum(nil)) +} From 7dcf635066214f9a3797cb328d98ad2ddf63007e Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 14:05:45 +0800 Subject: [PATCH 5/8] test(server): prove caller-supplied cross-origin policy is enforced, not just permitted --- pkg/protomcp/server_http_test.go | 39 +++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/pkg/protomcp/server_http_test.go b/pkg/protomcp/server_http_test.go index 2d33a67..941bbab 100644 --- a/pkg/protomcp/server_http_test.go +++ b/pkg/protomcp/server_http_test.go @@ -11,8 +11,9 @@ import ( ) // postInitialize fires a raw initialize POST at the server, optionally -// tagged with a Sec-Fetch-Site header, and returns the status code. -func postInitialize(t *testing.T, url, secFetchSite string) int { +// tagged with Sec-Fetch-Site and Origin headers, and returns the status +// code. +func postInitialize(t *testing.T, url, secFetchSite, origin string) int { t.Helper() body := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"c","version":"0.0.1"}}}` req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, url, bytes.NewBufferString(body)) @@ -24,6 +25,9 @@ func postInitialize(t *testing.T, url, secFetchSite string) int { if secFetchSite != "" { req.Header.Set("Sec-Fetch-Site", secFetchSite) } + if origin != "" { + req.Header.Set("Origin", origin) + } resp, err := http.DefaultClient.Do(req) if err != nil { t.Fatalf("post: %v", err) @@ -41,10 +45,10 @@ func TestCrossOriginProtection_OnByDefault(t *testing.T) { ts := httptest.NewServer(s) t.Cleanup(ts.Close) - if got := postInitialize(t, ts.URL, "cross-site"); got != http.StatusForbidden { + if got := postInitialize(t, ts.URL, "cross-site", ""); got != http.StatusForbidden { t.Errorf("cross-site POST status = %d, want %d (cross-origin protection missing)", got, http.StatusForbidden) } - if got := postInitialize(t, ts.URL, ""); got != http.StatusOK { + if got := postInitialize(t, ts.URL, "", ""); got != http.StatusOK { t.Errorf("same-origin POST status = %d, want %d", got, http.StatusOK) } } @@ -59,26 +63,35 @@ func TestCrossOriginProtection_UserHTTPOptsStillWrapped(t *testing.T) { ts := httptest.NewServer(s) t.Cleanup(ts.Close) - if got := postInitialize(t, ts.URL, "cross-site"); got != http.StatusForbidden { + if got := postInitialize(t, ts.URL, "cross-site", ""); got != http.StatusForbidden { t.Errorf("cross-site POST status = %d, want %d (wrap dropped by unrelated HTTP options)", got, http.StatusForbidden) } } -// TestCrossOriginProtection_CallerSuppliedWins verifies the opt-out: a -// caller-supplied CrossOriginProtection passes through to the SDK -// untouched, so a permissive policy admits cross-site requests. +// TestCrossOriginProtection_CallerSuppliedWins verifies the opt-out in +// both directions: a caller-supplied CrossOriginProtection replaces +// protomcp's default wrap, so its trusted origin is admitted where the +// default would 403 — and its policy still rejects untrusted origins. +// The rejection leg matters: it fails if a future SDK release keeps the +// deprecated field but stops honoring it, which would otherwise leave +// the server with no protection at all while this test stayed green. func TestCrossOriginProtection_CallerSuppliedWins(t *testing.T) { - permissive := http.NewCrossOriginProtection() - permissive.AddInsecureBypassPattern("/") + cop := http.NewCrossOriginProtection() + if err := cop.AddTrustedOrigin("https://trusted.example"); err != nil { + t.Fatalf("AddTrustedOrigin: %v", err) + } s := New("t", "0.0.1", WithHTTPOptions(&mcp.StreamableHTTPOptions{ - CrossOriginProtection: permissive, + CrossOriginProtection: cop, }), ) ts := httptest.NewServer(s) t.Cleanup(ts.Close) - if got := postInitialize(t, ts.URL, "cross-site"); got != http.StatusOK { - t.Errorf("cross-site POST status = %d, want %d (caller-supplied protection not honored)", got, http.StatusOK) + if got := postInitialize(t, ts.URL, "cross-site", "https://trusted.example"); got != http.StatusOK { + t.Errorf("trusted-origin cross-site POST status = %d, want %d (caller-supplied protection not honored)", got, http.StatusOK) + } + if got := postInitialize(t, ts.URL, "cross-site", "https://evil.example"); got != http.StatusForbidden { + t.Errorf("untrusted-origin cross-site POST status = %d, want %d (caller-supplied policy not enforced)", got, http.StatusForbidden) } } From ee3779c9ce8cab901e0eeccb061417b88d10b254 Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 14:05:45 +0800 Subject: [PATCH 6/8] docs: correct ResultProcessor coverage claims and error-code range for the MRTR gate --- AGENTS.md | 2 +- README.md | 12 +++++++----- pkg/protomcp/result.go | 6 +++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 57cd054..37ba7da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,7 +51,7 @@ LSP diagnostics go stale after `buf generate`; trust `go build` / `go test` over - **proto message copy-locks.** Generated message types embed `protoimpl.MessageState` which contains a `sync.Mutex`. Don't struct-copy them (`c := *t`); use `proto.Clone`. - **`paths=source_relative`.** Our `buf.gen.yaml` uses `paths=source_relative`, meaning the output directory mirrors the proto source tree. Do not change without updating every `go_package` option and every import path. - **protojson vs json.** MCP tool content is protojson. Use `protojson.Unmarshal` in tests that decode into generated proto types, plain `json.Unmarshal` breaks on Timestamp, Duration, enum-as-name, and int64-as-string. -- **Elicitation is multi-round-trip (SEP-2322).** Generated elicitation-gated handlers run **twice** per confirmed call: first invocation returns an `InputRequests` result, the retry carries the answer in `req.Params.InputResponses["confirm"]`. A harness that invokes a generated handler directly with a non-nil session must supply that map entry or it will get the input-required result, not the tool result. +- **Elicitation is multi-round-trip (SEP-2322).** Generated elicitation-gated handlers run **twice** per confirmed call: first invocation returns an `InputRequests` result carrying a `RequestState` bound to the call (`protomcp.ElicitationState(toolName, rawArgs)`), the retry must echo that state and the answer in `req.Params.InputResponses["confirm"]`. A harness that invokes a generated handler directly needs non-nil `req.Params` with both the map entry and the matching `RequestState`, or it will get the input-required result, not the tool result. - **Subscription registration is asynchronous on protocol ≥ 2026-07-28.** The client's `subscriptions/listen` call dispatches without awaiting a response, so `Subscribe`/`Connect` return before the server has recorded the subscription. Tests that connect and immediately mutate can miss notifications; poll or wait for a first event instead of assuming registration completed. ## Where design decisions live diff --git a/README.md b/README.md index 5921419..9a0e022 100644 --- a/README.md +++ b/README.md @@ -403,6 +403,8 @@ Enum-typed and `buf.validate.string.in`-constrained prompt arguments automatical The gate uses the multi-round-trip protocol (SEP-2322). The generated handler's first invocation returns a `CallToolResult` carrying a single elicitation `InputRequest` under the fixed key `"confirm"` — not a tool result. The SDK then obtains the user's answer and re-invokes the handler with it echoed in `inputResponses`; only an `action == "accept"` answer lets the gRPC call run. On sessions speaking protocol ≥ 2026-07-28 the client's middleware fulfills the request through its `ElicitationHandler` and retries automatically; on older sessions the SDK's server-side shim performs a classic `elicitation/create` round-trip and re-invokes the handler in place. One annotation serves both. Note the handler body runs twice per confirmed call. +**Answers are bound to the call that prompted them.** The input-required result carries a `RequestState` derived from the tool name and raw arguments (`protomcp.ElicitationState`), and the retry must echo it back alongside the answer or the gate re-prompts. This matters because the SDK's client middleware mutates the caller's `CallToolParams` in place when it fulfills an elicitation: a client reusing one params struct across calls would otherwise carry a stale `"confirm"` answer into the next call and silently skip the confirmation. The binding is a content hash, not an authenticator — a client willing to lie about the user's answer can compute it, just as it could return `accept` from its handler without asking anyone. Elicitation is a UX confirmation for honest clients, not a server-enforced authorization control; enforce authorization server-side. + **Clients must register an `ElicitationHandler`.** A client without one fails the tool call with a hard JSON-RPC error (`client does not support elicitation`) — the error never reaches your `ToolErrorHandler`, and there is no `IsError` result for the LLM to read. (Under go-sdk v1.5.0 this case surfaced as a graceful `IsError` tool result instead.) Hard codegen error when used without a companion `tool`, or on a streaming RPC. @@ -572,24 +574,24 @@ func injectTenant(next protomcp.ToolHandler) protomcp.ToolHandler { Override with `protomcp.WithToolErrorHandler(...)`, mirrors `grpc-gateway`'s `runtime.WithErrorHandler` but produces JSON-RPC shapes. -**Error-code landscape (go-sdk ≥ v1.7.0).** protomcp's own JSON-RPC codes are unchanged: `Unauthenticated → -32001`, `PermissionDenied → -32002`, `Canceled → -32003`, `DeadlineExceeded → -32004` (all in the implementation-defined `-32000..-32019` range). The SDK's resource-not-found error moved from `-32002` to `-32602` (`mcp.CodeResourceNotFound` is now a deprecated `var` aliasing `jsonrpc.CodeInvalidParams`), so `-32002` no longer collides with any SDK response — but `-32602` is now shared between the SDK's resource-not-found / invalid-params errors and protomcp's invalid-cursor mapping. protomcp does not set the `MCPGODEBUG=customresnotfounderrcode` escape hatch. +**Error-code landscape (go-sdk ≥ v1.7.0).** protomcp's own JSON-RPC codes are unchanged: `Unauthenticated → -32001`, `PermissionDenied → -32002`, `Canceled → -32003`, `DeadlineExceeded → -32004` (all in JSON-RPC's implementation-defined `-32000..-32099` range, below the `-32020` and up band the SDK claims for its own codes). The SDK's resource-not-found error moved from `-32002` to `-32602` (`mcp.CodeResourceNotFound` is now a deprecated `var` aliasing `jsonrpc.CodeInvalidParams`), so `-32002` no longer collides with any SDK response — but `-32602` is now shared between the SDK's resource-not-found / invalid-params errors and protomcp's invalid-cursor mapping. protomcp does not set the `MCPGODEBUG=customresnotfounderrcode` escape hatch. ### ResultProcessor, mutate responses before the client sees them ```go -func scrubEmails(_ context.Context, _ *mcp.CallToolRequest, r *mcp.CallToolResult) (*mcp.CallToolResult, error) { - for _, c := range r.Content { +func scrubEmails(_ context.Context, _ *protomcp.GRPCData, m *protomcp.MCPData[*mcp.CallToolRequest, *mcp.CallToolResult]) (*mcp.CallToolResult, error) { + for _, c := range m.Output.Content { if tc, ok := c.(*mcp.TextContent); ok { tc.Text = emailRE.ReplaceAllString(tc.Text, "[email]") } } - return r, nil + return m.Output, nil } protomcp.New("svc", "0.1.0", protomcp.WithToolResultProcessor(scrubEmails)) ``` -Processors run on **both** success and `IsError` results, so a single redaction rule covers every response path. +Processors run on **both** success and `IsError` results, so a single redaction rule covers both of those response paths. They do **not** run on input-required (multi-round-trip) intermediates such as the generated elicitation confirmation — the SDK rejects results carrying both `Content` and `InputRequests`, so those bypass the pipeline finish. In particular, the elicitation prompt (rendered from request fields) reaches the client unredacted; keep sensitive values out of `elicitation.message` templates. ### RetryLoop diff --git a/pkg/protomcp/result.go b/pkg/protomcp/result.go index 15f6e47..3086550 100644 --- a/pkg/protomcp/result.go +++ b/pkg/protomcp/result.go @@ -9,7 +9,11 @@ import ( // ToolResultProcessor inspects or mutates a CallToolResult before it // reaches the client. Processors see IsError results synthesized by // ToolErrorHandler, so a redaction processor covers both success and -// failure paths. +// failure paths. They do NOT see input-required (multi-round-trip) +// intermediates such as the generated elicitation confirmation: those +// bypass the pipeline finish because the SDK rejects results carrying +// both Content and InputRequests, so anything a processor could attach +// would be invalid there. // // Alias for ResultProcessor[*mcp.CallToolRequest, *mcp.CallToolResult]. type ToolResultProcessor = ResultProcessor[*mcp.CallToolRequest, *mcp.CallToolResult] From e3a0e149aa1b5d030c9f9c14c0d9ae32b455c9a3 Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Tue, 4 Aug 2026 14:25:47 +0800 Subject: [PATCH 7/8] test: pin ElicitationState contract and document identical-replay semantics --- README.md | 2 +- internal/gen/generator_test.go | 5 +++-- pkg/protomcp/elicitation.go | 10 ++++++---- pkg/protomcp/elicitation_test.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 pkg/protomcp/elicitation_test.go diff --git a/README.md b/README.md index 9a0e022..ce269e6 100644 --- a/README.md +++ b/README.md @@ -403,7 +403,7 @@ Enum-typed and `buf.validate.string.in`-constrained prompt arguments automatical The gate uses the multi-round-trip protocol (SEP-2322). The generated handler's first invocation returns a `CallToolResult` carrying a single elicitation `InputRequest` under the fixed key `"confirm"` — not a tool result. The SDK then obtains the user's answer and re-invokes the handler with it echoed in `inputResponses`; only an `action == "accept"` answer lets the gRPC call run. On sessions speaking protocol ≥ 2026-07-28 the client's middleware fulfills the request through its `ElicitationHandler` and retries automatically; on older sessions the SDK's server-side shim performs a classic `elicitation/create` round-trip and re-invokes the handler in place. One annotation serves both. Note the handler body runs twice per confirmed call. -**Answers are bound to the call that prompted them.** The input-required result carries a `RequestState` derived from the tool name and raw arguments (`protomcp.ElicitationState`), and the retry must echo it back alongside the answer or the gate re-prompts. This matters because the SDK's client middleware mutates the caller's `CallToolParams` in place when it fulfills an elicitation: a client reusing one params struct across calls would otherwise carry a stale `"confirm"` answer into the next call and silently skip the confirmation. The binding is a content hash, not an authenticator — a client willing to lie about the user's answer can compute it, just as it could return `accept` from its handler without asking anyone. Elicitation is a UX confirmation for honest clients, not a server-enforced authorization control; enforce authorization server-side. +**Answers are bound to the call that prompted them.** The input-required result carries a `RequestState` derived from the tool name and raw arguments (`protomcp.ElicitationState`), and the retry must echo it back alongside the answer or the gate re-prompts. This matters because the SDK's client middleware mutates the caller's `CallToolParams` in place when it fulfills an elicitation: a client reusing one params struct across calls would otherwise carry a stale `"confirm"` answer into the next call and silently skip the confirmation. The binding is a content hash, not an authenticator or a nonce — a client willing to lie about the user's answer can compute it, just as it could return `accept` from its handler without asking anyone, and a byte-identical call re-issued with the already-echoed answer (same tool, same arguments, same state) will not re-prompt. Elicitation is a UX confirmation for honest clients, not a server-enforced authorization control; enforce authorization — and, for operations where even an identical replay must be re-confirmed, idempotency — server-side. **Clients must register an `ElicitationHandler`.** A client without one fails the tool call with a hard JSON-RPC error (`client does not support elicitation`) — the error never reaches your `ToolErrorHandler`, and there is no `IsError` result for the LLM to read. (Under go-sdk v1.5.0 this case surfaced as a graceful `IsError` tool result instead.) diff --git a/internal/gen/generator_test.go b/internal/gen/generator_test.go index e5ae8b8..bf3208e 100644 --- a/internal/gen/generator_test.go +++ b/internal/gen/generator_test.go @@ -101,8 +101,9 @@ func TestGenerate_BadStreams_BidiErrors(t *testing.T) { // TestGenerate_Elicit covers the happy path where a method carries both a // tool and an elicitation annotation: the generated source must emit the // multi-round-trip confirmation gate — an InputRequests result carrying -// mcp.ElicitParams on the first invocation, the inputResponses lookup on -// the retry — plus the Mustache-rendered message expression and the +// mcp.ElicitParams and a RequestState bound to the call on the first +// invocation, the inputResponses lookup plus state verification on the +// retry — plus the Mustache-rendered message expression and the // decline-path IsError short-circuit. func TestGenerate_Elicit(t *testing.T) { out := runGenerate(t, "elicit.proto") diff --git a/pkg/protomcp/elicitation.go b/pkg/protomcp/elicitation.go index f32482e..410fcfb 100644 --- a/pkg/protomcp/elicitation.go +++ b/pkg/protomcp/elicitation.go @@ -13,10 +13,12 @@ import ( // CallToolParams struct the SDK's client middleware mutated in place on // an earlier call) therefore re-prompts instead of silently confirming. // -// The value is a plain content hash, not an authenticator: a client -// willing to lie about the user's answer can compute it. Elicitation is -// a UX confirmation for honest clients, not a server-enforced -// authorization control — enforce authorization server-side. +// The value is a plain content hash, not an authenticator or a nonce: a +// client willing to lie about the user's answer can compute it, and a +// byte-identical call re-issued with an already-echoed answer will not +// re-prompt. Elicitation is a UX confirmation for honest clients, not a +// server-enforced authorization control — enforce authorization (and, +// where identical replays matter, idempotency) server-side. func ElicitationState(toolName string, rawArgs []byte) string { h := sha256.New() h.Write([]byte(toolName)) diff --git a/pkg/protomcp/elicitation_test.go b/pkg/protomcp/elicitation_test.go new file mode 100644 index 0000000..7c1dd5b --- /dev/null +++ b/pkg/protomcp/elicitation_test.go @@ -0,0 +1,28 @@ +package protomcp + +import "testing" + +// TestElicitationState pins the helper's contract: deterministic hex +// output, sensitivity to both inputs, and domain separation between +// them. Generated elicitation gates and any test harness invoking a +// gated handler directly both depend on recomputing identical values. +func TestElicitationState(t *testing.T) { + base := ElicitationState("Tasks_DeleteTask", []byte(`{"id":"a"}`)) + if len(base) != 64 { // hex-encoded sha256 + t.Fatalf("ElicitationState length = %d, want 64 hex chars", len(base)) + } + if got := ElicitationState("Tasks_DeleteTask", []byte(`{"id":"a"}`)); got != base { + t.Errorf("not deterministic: %q != %q", got, base) + } + if got := ElicitationState("Tasks_DeleteTask", []byte(`{"id":"b"}`)); got == base { + t.Errorf("argument change did not change the state") + } + if got := ElicitationState("Other_Tool", []byte(`{"id":"a"}`)); got == base { + t.Errorf("tool-name change did not change the state") + } + // The 0x00 separator must keep the (toolName, args) boundary + // unambiguous: shifting bytes across it has to change the state. + if ElicitationState("ab", []byte("c")) == ElicitationState("a", []byte("bc")) { + t.Errorf("domain separation broken: boundary shift produced the same state") + } +} From 0212a8b5464c7c9e229295e13772967d88449ab3 Mon Sep 17 00:00:00 2001 From: Haoxin Yang Date: Wed, 5 Aug 2026 13:40:24 +0800 Subject: [PATCH 8/8] test: pin identical-replay elicitation regression, gated on the go-sdk params-mutation fix (modelcontextprotocol/go-sdk#1145) --- README.md | 2 +- examples/tasks/e2e_elicitation_test.go | 63 ++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ce269e6..0c0f1d8 100644 --- a/README.md +++ b/README.md @@ -403,7 +403,7 @@ Enum-typed and `buf.validate.string.in`-constrained prompt arguments automatical The gate uses the multi-round-trip protocol (SEP-2322). The generated handler's first invocation returns a `CallToolResult` carrying a single elicitation `InputRequest` under the fixed key `"confirm"` — not a tool result. The SDK then obtains the user's answer and re-invokes the handler with it echoed in `inputResponses`; only an `action == "accept"` answer lets the gRPC call run. On sessions speaking protocol ≥ 2026-07-28 the client's middleware fulfills the request through its `ElicitationHandler` and retries automatically; on older sessions the SDK's server-side shim performs a classic `elicitation/create` round-trip and re-invokes the handler in place. One annotation serves both. Note the handler body runs twice per confirmed call. -**Answers are bound to the call that prompted them.** The input-required result carries a `RequestState` derived from the tool name and raw arguments (`protomcp.ElicitationState`), and the retry must echo it back alongside the answer or the gate re-prompts. This matters because the SDK's client middleware mutates the caller's `CallToolParams` in place when it fulfills an elicitation: a client reusing one params struct across calls would otherwise carry a stale `"confirm"` answer into the next call and silently skip the confirmation. The binding is a content hash, not an authenticator or a nonce — a client willing to lie about the user's answer can compute it, just as it could return `accept` from its handler without asking anyone, and a byte-identical call re-issued with the already-echoed answer (same tool, same arguments, same state) will not re-prompt. Elicitation is a UX confirmation for honest clients, not a server-enforced authorization control; enforce authorization — and, for operations where even an identical replay must be re-confirmed, idempotency — server-side. +**Answers are bound to the call that prompted them.** The input-required result carries a `RequestState` derived from the tool name and raw arguments (`protomcp.ElicitationState`), and the retry must echo it back alongside the answer or the gate re-prompts. This matters because the SDK's client middleware mutates the caller's `CallToolParams` in place when it fulfills an elicitation: a client reusing one params struct across calls would otherwise carry a stale `"confirm"` answer into the next call and silently skip the confirmation. The binding is a content hash, not an authenticator or a nonce — a client willing to lie about the user's answer can compute it, just as it could return `accept` from its handler without asking anyone, and a byte-identical call re-issued with the already-echoed answer (same tool, same arguments, same state) will not re-prompt. Elicitation is a UX confirmation for honest clients, not a server-enforced authorization control; enforce authorization — and, for operations where even an identical replay must be re-confirmed, idempotency — server-side. The reused-struct hazard originates in the SDK: go-sdk's client middleware leaves the fulfilled `inputResponses`/`requestState` on the caller's params ([modelcontextprotocol/go-sdk#1144](https://github.com/modelcontextprotocol/go-sdk/issues/1144)). With the upstream fix ([modelcontextprotocol/go-sdk#1145](https://github.com/modelcontextprotocol/go-sdk/pull/1145)) retry state travels on a copy, a reused struct arrives clean, and the gate re-prompts even for a byte-identical call; protomcp will pin the first go-sdk release containing it, and `TestDeleteTask_ElicitationIdenticalReplayRePrompts` activates against that release. **Clients must register an `ElicitationHandler`.** A client without one fails the tool call with a hard JSON-RPC error (`client does not support elicitation`) — the error never reaches your `ToolErrorHandler`, and there is no `IsError` result for the LLM to read. (Under go-sdk v1.5.0 this case surfaced as a graceful `IsError` tool result instead.) diff --git a/examples/tasks/e2e_elicitation_test.go b/examples/tasks/e2e_elicitation_test.go index 700a5d0..45458cc 100644 --- a/examples/tasks/e2e_elicitation_test.go +++ b/examples/tasks/e2e_elicitation_test.go @@ -196,6 +196,69 @@ func TestDeleteTask_ElicitationReusedParamsRePrompts(t *testing.T) { } } +// TestDeleteTask_ElicitationIdenticalReplayRePrompts pins the +// byte-identical replay property end to end: re-issuing the SAME +// CallToolParams struct — same tool, same arguments — after a confirmed +// call must prompt again rather than ride the previous answer. The +// server-side RequestState is a recomputable content hash, so this +// protection lives client-side: a fixed SDK never writes retry state +// into the caller's params, the replay arrives with no answer, and the +// gate re-prompts. go-sdk <= v1.7.0 instead leaves the fulfilled answer +// and matching state on the caller's struct +// (modelcontextprotocol/go-sdk#1144), so the replay skips the prompt; +// on such SDKs this test detects the mutation and skips. Once the +// pinned go-sdk contains the fix (modelcontextprotocol/go-sdk#1145), +// the skip never triggers — turn it into a hard failure then, so an +// SDK regression cannot silently reopen the replay window. +func TestDeleteTask_ElicitationIdenticalReplayRePrompts(t *testing.T) { + ctx := context.Background() + grpcClient := startGRPC(t) + srv := protomcp.New("tasks", "0.1.0") + tasksv1.RegisterTasksMCPTools(srv, grpcClient) + + // Accept the first prompt, decline any later one: if the replay + // correctly re-prompts, it must then short-circuit with IsError. + var elicitCalls atomic.Int32 + cs := connectWith(ctx, t, srv, &mcp.ClientOptions{ + ElicitationHandler: func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) { + if elicitCalls.Add(1) == 1 { + return &mcp.ElicitResult{Action: "accept"}, nil + } + return &mcp.ElicitResult{Action: "decline"}, nil + }, + }) + + var created tasksv1.Task + callTool(ctx, t, cs, "Tasks_CreateTask", `{"task":{"title":"replay-me","done":false}}`, &created) + + params := &mcp.CallToolParams{ + Name: "Tasks_DeleteTask", + Arguments: json.RawMessage(fmt.Sprintf(`{"id":%q}`, created.Id)), + } + if out, err := cs.CallTool(ctx, params); err != nil || out.IsError { + t.Fatalf("first Delete: err=%v out=%+v", err, out) + } + if got := elicitCalls.Load(); got != 1 { + t.Fatalf("after first delete: elicitation handler called %d times, want 1", got) + } + + if params.RequestState != "" || params.InputResponses != nil { + t.Skipf("go-sdk left multi-round-trip state on caller params (modelcontextprotocol/go-sdk#1144, fixed by #1145): a byte-identical replay would skip the prompt until the fixed SDK is pinned") + } + + // Replay the SAME struct, byte-identical arguments included. + out, err := cs.CallTool(ctx, params) + if err != nil { + t.Fatalf("replayed Delete: transport error: %v", err) + } + if got := elicitCalls.Load(); got != 2 { + t.Errorf("after replay: elicitation handler called %d times, want 2 (identical replay must re-prompt)", got) + } + if !out.IsError { + t.Errorf("replayed Delete: want IsError (user declined the re-prompt), got success: %+v", out) + } +} + // TestDeleteTask_ElicitationPrePopulatedAnswerRePrompts asserts that an // inputResponses entry supplied on the very first call, without the // matching RequestState, does not skip the confirmation: the gate