From 06d63f1639143bc18c599e69eb5545ba5f8e11e3 Mon Sep 17 00:00:00 2001 From: Sayt-0 Date: Mon, 24 Aug 2026 10:29:01 +0200 Subject: [PATCH 1/2] feat(mcp): support protocol revision 2026-07-28 --- .golangci.yml | 9 + cmd/root/mcp.go | 8 +- cmd/root/mcp_test.go | 46 +++ docs/features/cli/index.md | 2 +- docs/features/mcp-mode/index.md | 4 +- go.mod | 2 +- go.sum | 4 +- pkg/mcp/server.go | 26 +- pkg/mcp/server_test.go | 325 ++++++++++++++++++ .../builtin/mcpcatalog/mcpcatalog_test.go | 10 +- pkg/tools/mcp/remote_test.go | 14 +- 11 files changed, 438 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 553606efb5..80c39403e5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -187,6 +187,15 @@ linters: linters: - staticcheck text: "SA1019:.*adka2a.*deprecated" + # MCP sampling is deprecated by spec 2026-07-28 (SEP-2577) but stays + # functional during the >=12-month deprecation window; migrating our + # sampling support is a separate effort. Scoped to the sampling + # implementation and its tests so new code cannot silently rely on + # the deprecated SDK surface. + - path: ^(pkg/runtime/|pkg/tools/mcp/|pkg/tools/sampling\.go|pkg/tools/codemode/codemode_test\.go|e2e/sampling_test\.go) + linters: + - staticcheck + text: "SA1019:.*the sampling feature is deprecated" # Vendored verbatim from moby/moby; keep its //nolint:gosec directives # even though our config excludes gosec G404 globally. - path: pkg/worktree/namesgenerator/ diff --git a/cmd/root/mcp.go b/cmd/root/mcp.go index 1aef897d2f..8d4475cfeb 100644 --- a/cmd/root/mcp.go +++ b/cmd/root/mcp.go @@ -51,7 +51,7 @@ func newMCPCmd() *cobra.Command { cmd.PersistentFlags().StringVar(&flags.authToken, "auth-token", "", "Bearer token required for HTTP MCP requests; only valid with --http") cmd.PersistentFlags().BoolVar(&flags.insecureNoAuth, "insecure-no-auth", false, "Allow unauthenticated non-loopback HTTP binding (insecure); only valid with --http") cmd.PersistentFlags().StringVar(&flags.runConfig.MCPToolName, "tool-name", "", "Override the MCP tool identifier clients call (defaults to agent name); only valid when exposing a single agent") - cmd.PersistentFlags().DurationVar(&flags.runConfig.MCPKeepAlive, "mcp-keepalive", 0, "Interval between MCP keep-alive pings (e.g. 30s); 0 disables keep-alive") + cmd.PersistentFlags().DurationVar(&flags.runConfig.MCPKeepAlive, "mcp-keepalive", 0, "Interval between MCP keep-alive pings (e.g. 30s); 0 disables keep-alive; only valid when serving an agent over stdio (not --http or --attach)") addRuntimeConfigFlags(cmd, &flags.runConfig) return cmd @@ -68,12 +68,18 @@ func (f *mcpFlags) runMCPCommand(cmd *cobra.Command, args []string) (commandErr if f.http || f.safety != "" || f.authToken != "" || f.insecureNoAuth { return errors.New("--http-only safety and authentication flags cannot be used with --attach") } + if f.runConfig.MCPKeepAlive != 0 { + return errors.New("--mcp-keepalive cannot be used with --attach: the attach proxy ignores runtime configuration") + } return f.runAttach(ctx) } if !f.http && (f.safety != "" || f.authToken != "" || f.insecureNoAuth) { return errors.New("--safety, --auth-token, and --insecure-no-auth require --http") } + if f.http && f.runConfig.MCPKeepAlive != 0 { + return errors.New("--mcp-keepalive is not supported with --http: stateless HTTP MCP does not support server-initiated keep-alive pings; use the stdio transport instead") + } if err := validateSafetyFlag(f.safety); err != nil { return err } diff --git a/cmd/root/mcp_test.go b/cmd/root/mcp_test.go index 8f2e35bf11..7199be5624 100644 --- a/cmd/root/mcp_test.go +++ b/cmd/root/mcp_test.go @@ -1,6 +1,8 @@ package root import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -37,3 +39,47 @@ func TestMCPHTTPRejectsUnauthenticatedNonLoopbackBind(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "require --auth-token or --insecure-no-auth") } + +func TestMCPKeepAliveRejectedWithHTTP(t *testing.T) { + t.Parallel() + + // The stateless HTTP transport (MCP 2026-07-28) rejects server-initiated + // requests, so a keep-alive ping interval must be refused before the + // server starts listening. + cmd := newMCPCmd() + cmd.SetArgs([]string{"agent.yaml", "--http", "--mcp-keepalive", "30s"}) + err := cmd.ExecuteContext(t.Context()) + require.Error(t, err) + assert.Contains(t, err.Error(), "--mcp-keepalive") + assert.Contains(t, err.Error(), "stateless HTTP") +} + +func TestMCPKeepAliveRejectedWithAttach(t *testing.T) { + t.Parallel() + + // --attach proxies a running TUI session through its own MCP server, + // which never sees the runtime configuration: the flag must be refused + // rather than silently ignored. + cmd := newMCPCmd() + cmd.SetArgs([]string{"--attach", "--mcp-keepalive", "30s"}) + err := cmd.ExecuteContext(t.Context()) + require.Error(t, err) + assert.Contains(t, err.Error(), "--mcp-keepalive") + assert.Contains(t, err.Error(), "--attach") +} + +func TestMCPKeepAliveAcceptedForStdio(t *testing.T) { + t.Parallel() + + // An unparsable agent file makes the stdio path fail during config + // loading, which is past flag validation: the keep-alive flag itself + // must not be rejected without --http. + agentFile := filepath.Join(t.TempDir(), "broken.yaml") + require.NoError(t, os.WriteFile(agentFile, []byte("not: [valid"), 0o600)) + + cmd := newMCPCmd() + cmd.SetArgs([]string{agentFile, "--mcp-keepalive", "30s"}) + err := cmd.ExecuteContext(t.Context()) + require.Error(t, err) + assert.NotContains(t, err.Error(), "--mcp-keepalive") +} diff --git a/docs/features/cli/index.md b/docs/features/cli/index.md index 70df4d6c48..86880717de 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -322,7 +322,7 @@ $ docker agent serve mcp [flags] | `--auth-token ` | (none) | Required Bearer token for HTTP MCP requests. | | `--insecure-no-auth` | `false` | Permit unauthenticated non-loopback HTTP MCP binding. | | `-l, --listen ` | `127.0.0.1:8081` | Address to listen on (only used with `--http`). | -| `--mcp-keepalive `| `0` (disabled) | Interval between MCP keep-alive pings (e.g. `30s`). | +| `--mcp-keepalive ` | `0` (disabled) | Interval between MCP keep-alive pings (e.g. `30s`). Only when serving an agent over stdio — rejected with `--http` (the stateless HTTP transport, MCP `2026-07-28`, has no server-initiated ping) and with `--attach`. | | `--attach [target]` | (none) | Attach to a running TUI run by pid, address, or session id; given without a value, selects the most recent run. | All [runtime configuration flags](#runtime-configuration-flags) are also accepted. diff --git a/docs/features/mcp-mode/index.md b/docs/features/mcp-mode/index.md index 9e3ae0bf69..d251dd4e6d 100644 --- a/docs/features/mcp-mode/index.md +++ b/docs/features/mcp-mode/index.md @@ -58,10 +58,12 @@ $ docker agent serve mcp ./agent.yaml --http --listen 0.0.0.0:9090 --auth-token | `--auth-token` | (none) | Require this Bearer token for HTTP requests. Required for non-loopback HTTP unless explicitly overridden. | | `--insecure-no-auth` | `false` | Permit unauthenticated non-loopback HTTP. Use only behind a trusted authentication boundary. | | `--safety` | `restricted` | Tool safety policy for HTTP requests. CLI value overrides agent/runtime configuration. | -| `--mcp-keepalive` | `0` | Interval between MCP keep-alive pings (e.g. `30s`); `0` disables keep-alive. | +| `--mcp-keepalive` | `0` | Interval between MCP keep-alive pings (e.g. `30s`); `0` disables keep-alive. Only when serving an agent over stdio — rejected with `--http` and `--attach`. | Runtime configuration flags such as `--working-dir`, `--env-from-file`, `--models-gateway`, and hook flags are also available — see the [CLI reference](../cli/index.md). +The HTTP transport is **stateless**, per MCP spec revision `2026-07-28`: modern clients negotiate via `server/discover`, no `Mcp-Session-Id` is issued, and only POST requests are served (GET and DELETE answer `405 Method Not Allowed`). Clients speaking older protocol revisions keep working — the legacy `initialize` handshake is accepted with per-request state. However, older stateful clients that depend on a standalone GET stream or session `DELETE` teardown must upgrade to (or switch to) a client compatible with stateless streaming HTTP. Because the stateless transport has no server-initiated ping, `--mcp-keepalive` is rejected together with `--http`; keep-alive remains available on the stdio transport. + ## HTTP security HTTP MCP defaults to loopback binding. A non-loopback `--listen` address requires `--auth-token`; use `--insecure-no-auth` only when a trusted reverse proxy or network boundary authenticates clients. The safety policy is resolved in this order: `--safety`, agent configuration, runtime configuration, then `restricted`. These HTTP-only flags do not affect stdio or `--attach` operation. diff --git a/go.mod b/go.mod index cde6b55b3d..5f892b5925 100644 --- a/go.mod +++ b/go.mod @@ -53,7 +53,7 @@ require ( github.com/labstack/echo/v4 v4.15.4 github.com/mattn/go-isatty v0.0.24 github.com/mattn/go-runewidth v0.0.28 - github.com/modelcontextprotocol/go-sdk v1.6.1 + github.com/modelcontextprotocol/go-sdk v1.7.0 github.com/muesli/cancelreader v0.2.2 github.com/natefinch/atomic v1.0.1 github.com/openai/openai-go/v3 v3.52.0 diff --git a/go.sum b/go.sum index dc2b14165c..3fcdd3d898 100644 --- a/go.sum +++ b/go.sum @@ -344,8 +344,8 @@ github.com/moby/sys/sequential v0.6.0 h1:qrx7XFUd/5DxtqcoH1h438hF5TmOvzC/lspjy7z github.com/moby/sys/sequential v0.6.0/go.mod h1:uyv8EUTrca5PnDsdMGXhZe6CCe8U/UiTWd+lL+7b/Ko= github.com/moby/term v0.5.2 h1:6qk3FJAFDs6i/q3W/pQ97SX192qKfZgGjCQqfCJkgzQ= github.com/moby/term v0.5.2/go.mod h1:d3djjFCrjnB+fl8NJux+EJzu0msscUP+f8it8hPkFLc= -github.com/modelcontextprotocol/go-sdk v1.6.1 h1:0zOSupjKUxPKSocPT1Wtago+mUHU2/uZ4xSOY0FGReU= -github.com/modelcontextprotocol/go-sdk v1.6.1/go.mod h1:kzm3kzFL1/+AziGOE0nUs3gvPoNxMCvkxokMkuFapXQ= +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/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= diff --git a/pkg/mcp/server.go b/pkg/mcp/server.go index 7d0a548abd..ceccdc041d 100644 --- a/pkg/mcp/server.go +++ b/pkg/mcp/server.go @@ -63,6 +63,13 @@ func StartMCPServer(ctx context.Context, agentFilename, agentName string, runCon // StartHTTPServer starts a streaming HTTP MCP server on the given listener func StartHTTPServer(ctx context.Context, agentFilename, agentName string, runConfig *config.RuntimeConfig, ln net.Listener, options HTTPOptions) error { + // Fail fast, before any config or team loading: the stateless HTTP + // transport (MCP 2026-07-28) rejects server-initiated requests such as + // ping, so keep-alive can never work here. + if runConfig.MCPKeepAlive != 0 { + return errors.New("MCP keep-alive is not supported over stateless HTTP; use the stdio transport instead") + } + slog.DebugContext(ctx, "Starting HTTP MCP server", "agent", agentFilename, "addr", ln.Addr()) agentSource, err := config.Resolve(agentFilename, nil) @@ -98,9 +105,7 @@ func StartHTTPServer(ctx context.Context, agentFilename, agentName string, runCo fmt.Printf("MCP HTTP server listening on http://%s\n", ln.Addr()) - handler := http.Handler(mcp.NewStreamableHTTPHandler(func(_ *http.Request) *mcp.Server { - return server - }, nil)) + handler := newStreamableHTTPHandler(server) if options.AuthToken != "" { handler = httpsec.BearerAuth(options.AuthToken)(handler) } @@ -135,6 +140,16 @@ func StartHTTPServer(ctx context.Context, agentFilename, agentName string, runCo } } +// newStreamableHTTPHandler builds the streamable HTTP handler used in +// production. Stateless mode implements the sessionless MCP 2026-07-28 +// transport: no Mcp-Session-Id header, GET/DELETE rejected with 405, and +// request-local state synthesized for legacy initialize-based clients. +func newStreamableHTTPHandler(server *mcp.Server) http.Handler { + return mcp.NewStreamableHTTPHandler(func(_ *http.Request) *mcp.Server { + return server + }, &mcp.StreamableHTTPOptions{Stateless: true}) +} + func createMCPServer(ctx context.Context, agentFilename, agentName string, runConfig *config.RuntimeConfig) (*mcp.Server, func(), error) { agentSource, err := config.Resolve(agentFilename, nil) if err != nil { @@ -161,7 +176,10 @@ func createMCPServer(ctx context.Context, agentFilename, agentName string, runCo } func createMCPServerForTeam(ctx context.Context, t *team.Team, agentFilename, agentName string, runConfig *config.RuntimeConfig, safety session.SafetyPolicy) (*mcp.Server, error) { - // The SDK only starts keep-alive when KeepAlive > 0. + // The SDK only starts keep-alive when KeepAlive > 0. StartHTTPServer (and + // the CLI, for early UX) rejects a nonzero keep-alive, so this only ever + // takes effect for stdio: the stateless HTTP transport (MCP 2026-07-28) + // rejects server-initiated requests such as ping. server := mcp.NewServer(&mcp.Implementation{ Name: "docker agent", Version: version.Version, diff --git a/pkg/mcp/server_test.go b/pkg/mcp/server_test.go index 543ce42efe..b96a8b786e 100644 --- a/pkg/mcp/server_test.go +++ b/pkg/mcp/server_test.go @@ -1,13 +1,23 @@ package mcp import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io" "net" "net/http" "net/http/httptest" + "strings" + "sync" "testing" + "time" + "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" "github.com/docker/docker-agent/pkg/agent" "github.com/docker/docker-agent/pkg/config" @@ -37,6 +47,23 @@ func TestStartHTTPServer_RejectsAutonomousYAMLSafety(t *testing.T) { require.ErrorContains(t, err, "--safety autonomous") } +func TestStartHTTPServer_RejectsKeepAlive(t *testing.T) { + t.Parallel() + + var lc net.ListenConfig + ln, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0") + require.NoError(t, err) + defer ln.Close() + + // A nonexistent agent file proves the guard fires before config and team + // loading: reaching them would fail with a different error. + runConfig := &config.RuntimeConfig{} + runConfig.MCPKeepAlive = 30 * time.Second + err = StartHTTPServer(t.Context(), "testdata/does-not-exist.yaml", "root", runConfig, ln, HTTPOptions{}) + require.ErrorContains(t, err, "keep-alive") + require.ErrorContains(t, err, "stdio transport") +} + func TestCreateMCPServer_AcceptsAutonomousYAMLSafetyForStdio(t *testing.T) { t.Setenv("OPENAI_API_KEY", "DUMMY") @@ -180,3 +207,301 @@ func TestCreateMCPServer_ToolNameRejectsMultipleAgents(t *testing.T) { assert.Contains(t, err.Error(), "--tool-name") assert.Contains(t, err.Error(), "exactly one agent") } + +// newTestHTTPHandler builds the production stateless handler around a server +// created through the production config-loading path. +func newTestHTTPHandler(t *testing.T) http.Handler { + t.Helper() + + server, cleanup, err := createMCPServer(t.Context(), "testdata/autonomous.yaml", "root", &config.RuntimeConfig{}) + require.NoError(t, err) + t.Cleanup(cleanup) + + return newStreamableHTTPHandler(server) +} + +// recordingRoundTripper captures the JSON-RPC method of every outgoing +// request and any Mcp-Session-Id response header, so tests can assert on the +// exact wire exchange the production handler produces. +type recordingRoundTripper struct { + mu sync.Mutex + methods []string + sessionIDs []string +} + +func (rt *recordingRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + var method string + if req.Body != nil && req.Body != http.NoBody { + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + req.Body = io.NopCloser(bytes.NewReader(body)) + var msg struct { + Method string `json:"method"` + } + _ = json.Unmarshal(body, &msg) + method = msg.Method + } + + resp, err := http.DefaultTransport.RoundTrip(req) + if err != nil { + return nil, err + } + + rt.mu.Lock() + defer rt.mu.Unlock() + if method != "" { + rt.methods = append(rt.methods, method) + } + if id := resp.Header.Get("Mcp-Session-Id"); id != "" { + rt.sessionIDs = append(rt.sessionIDs, id) + } + return resp, nil +} + +func (rt *recordingRoundTripper) recordedMethods() []string { + rt.mu.Lock() + defer rt.mu.Unlock() + return append([]string(nil), rt.methods...) +} + +func (rt *recordingRoundTripper) recordedSessionIDs() []string { + rt.mu.Lock() + defer rt.mu.Unlock() + return append([]string(nil), rt.sessionIDs...) +} + +// TestStreamableHTTPHandler_StatelessNegotiates20260728 proves the production +// HTTP handler negotiates the sessionless MCP 2026-07-28 revision with an SDK +// v1.7 client: negotiation happens via server/discover (never the legacy +// initialize handshake), tool listing succeeds, and no response ever carries +// an Mcp-Session-Id header. +func TestStreamableHTTPHandler_StatelessNegotiates20260728(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "DUMMY") + + httpSrv := httptest.NewServer(newTestHTTPHandler(t)) + defer httpSrv.Close() + + rec := &recordingRoundTripper{} + client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "0.0.1"}, nil) + session, err := client.Connect(t.Context(), &mcp.StreamableClientTransport{ + Endpoint: httpSrv.URL, + HTTPClient: &http.Client{Transport: rec}, + }, nil) + require.NoError(t, err) + defer session.Close() + + assert.Equal(t, "2026-07-28", session.InitializeResult().ProtocolVersion, + "stateless handler must negotiate the 2026-07-28 revision with a v1.7 client") + + toolsRes, err := session.ListTools(t.Context(), nil) + require.NoError(t, err) + require.Len(t, toolsRes.Tools, 1) + assert.Equal(t, "root", toolsRes.Tools[0].Name) + + methods := rec.recordedMethods() + assert.Contains(t, methods, "server/discover", + "a v1.7 client must negotiate via server/discover") + assert.NotContains(t, methods, "initialize", + "the client must not fall back to the legacy initialize handshake") + assert.Empty(t, rec.recordedSessionIDs(), + "stateless responses must not carry an Mcp-Session-Id header") +} + +// TestStreamableHTTPHandler_StatelessRejectsGETAndDELETE pins the stateless +// transport contract: only POST is served; GET (standalone SSE) and DELETE +// (session teardown) answer 405 with an explicit Allow header. +func TestStreamableHTTPHandler_StatelessRejectsGETAndDELETE(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "DUMMY") + + httpSrv := httptest.NewServer(newTestHTTPHandler(t)) + defer httpSrv.Close() + + for _, method := range []string{http.MethodGet, http.MethodDelete} { + req, err := http.NewRequestWithContext(t.Context(), method, httpSrv.URL, http.NoBody) + require.NoError(t, err) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode, method) + assert.Equal(t, "POST", resp.Header.Get("Allow"), method) + } +} + +// TestStreamableHTTPHandler_LegacyInitializeStillAccepted drives the raw +// legacy handshake a pre-2026-07-28 client performs: initialize, the +// initialized notification, then tools/list — all without a session ID. The +// stateless handler must synthesize request-local state and serve them. +func TestStreamableHTTPHandler_LegacyInitializeStillAccepted(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "DUMMY") + + httpSrv := httptest.NewServer(newTestHTTPHandler(t)) + defer httpSrv.Close() + + initResp := postJSONRPC(t, httpSrv.URL, `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"legacy-client","version":"0.0.1"}}}`) + require.Empty(t, string(initResp.rpcError), "unexpected JSON-RPC error") + assert.Empty(t, initResp.header.Get("Mcp-Session-Id"), + "stateless initialize must not assign a session") + var initResult struct { + ProtocolVersion string `json:"protocolVersion"` + } + require.NoError(t, json.Unmarshal(initResp.result, &initResult)) + assert.Equal(t, "2025-06-18", initResult.ProtocolVersion, + "legacy initialize must keep the client's protocol revision") + + // notifications/initialized has no id; a 202 acknowledges it. + notifResp := doJSONRPCRequest(t, httpSrv.URL, `{"jsonrpc":"2.0","method":"notifications/initialized"}`) + notifResp.Body.Close() + assert.Equal(t, http.StatusAccepted, notifResp.StatusCode) + + listResp := postJSONRPC(t, httpSrv.URL, `{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}`) + require.Empty(t, string(listResp.rpcError), "unexpected JSON-RPC error") + var listResult struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } + require.NoError(t, json.Unmarshal(listResp.result, &listResult)) + require.Len(t, listResult.Tools, 1, + "tools/list without a session ID must be served with synthesized state") + assert.Equal(t, "root", listResult.Tools[0].Name) +} + +type jsonRPCResponse struct { + header http.Header + result json.RawMessage + rpcError json.RawMessage +} + +func doJSONRPCRequest(t *testing.T, url, body string) *http.Response { + t.Helper() + + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, url, strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json, text/event-stream") + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + return resp +} + +// postJSONRPC sends a raw JSON-RPC request and decodes the single response +// message, which the handler delivers as JSON or as an SSE stream. +func postJSONRPC(t *testing.T, url, body string) jsonRPCResponse { + t.Helper() + + resp := doJSONRPCRequest(t, url, body) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + + var payload []byte + contentType := resp.Header.Get("Content-Type") + switch { + case strings.HasPrefix(contentType, "text/event-stream"): + scanner := bufio.NewScanner(resp.Body) + for scanner.Scan() { + if data, ok := strings.CutPrefix(scanner.Text(), "data: "); ok { + payload = []byte(data) + break + } + } + require.NoError(t, scanner.Err()) + case strings.HasPrefix(contentType, "application/json"): + var err error + payload, err = io.ReadAll(resp.Body) + require.NoError(t, err) + default: + t.Fatalf("unexpected Content-Type %q", contentType) + } + require.NotEmpty(t, payload, "no JSON-RPC message in response") + + var msg struct { + Result json.RawMessage `json:"result"` + Error json.RawMessage `json:"error"` + } + require.NoError(t, json.Unmarshal(payload, &msg)) + return jsonRPCResponse{header: resp.Header, result: msg.Result, rpcError: msg.Error} +} + +// TestStreamableHTTPHandler_StatelessToolsCallInvalidInput drives a real +// stateless tools/call through the production handler. The arguments fail +// input-schema validation (message must be a string), so the transport and +// RPC dispatch run end to end while the agent handler — and any model call — +// is never reached. +func TestStreamableHTTPHandler_StatelessToolsCallInvalidInput(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "DUMMY") + + httpSrv := httptest.NewServer(newTestHTTPHandler(t)) + defer httpSrv.Close() + + resp := postJSONRPC(t, httpSrv.URL, `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"root","arguments":{"message":123}}}`) + require.Empty(t, string(resp.rpcError), + "input validation failures must surface as a tool error result, not a protocol error") + assert.Empty(t, resp.header.Get("Mcp-Session-Id"), + "stateless tools/call must not assign a session") + + var callResult struct { + IsError bool `json:"isError"` + Content []struct { + Type string `json:"type"` + Text string `json:"text"` + } `json:"content"` + } + require.NoError(t, json.Unmarshal(resp.result, &callResult)) + assert.True(t, callResult.IsError) + require.NotEmpty(t, callResult.Content) + assert.Contains(t, callResult.Content[0].Text, `validating "arguments"`) +} + +// TestStreamableHTTPHandler_ConcurrentStatelessClients runs several +// independent clients against the shared production *mcp.Server at once; +// with -race this pins the stateless handler's thread-safety. +func TestStreamableHTTPHandler_ConcurrentStatelessClients(t *testing.T) { + t.Setenv("OPENAI_API_KEY", "DUMMY") + + httpSrv := httptest.NewServer(newTestHTTPHandler(t)) + defer httpSrv.Close() + + g, ctx := errgroup.WithContext(t.Context()) + for range 8 { + g.Go(func() error { + client := mcp.NewClient(&mcp.Implementation{Name: "concurrent-client", Version: "0.0.1"}, nil) + session, err := client.Connect(ctx, &mcp.StreamableClientTransport{Endpoint: httpSrv.URL}, nil) + if err != nil { + return err + } + defer session.Close() + res, err := session.ListTools(ctx, nil) + if err != nil { + return err + } + if len(res.Tools) != 1 || res.Tools[0].Name != "root" { + return fmt.Errorf("unexpected tools: %+v", res.Tools) + } + return nil + }) + } + require.NoError(t, g.Wait()) +} + +// TestAgentToolAnnotationsJSONKeepsFalseHints pins the SDK v1.7 (spec +// 2026-07-28) serialization change: false readOnlyHint and idempotentHint +// are emitted explicitly instead of omitted. +func TestAgentToolAnnotationsJSONKeepsFalseHints(t *testing.T) { + t.Parallel() + + ag := agent.New("test", "test agent", agent.WithTools( + tools.Tool{Name: "writer", Annotations: annot(false, false, nil, nil)}, + )) + annotations, err := agentToolAnnotations(t.Context(), ag) + require.NoError(t, err) + require.False(t, annotations.ReadOnlyHint) + require.False(t, annotations.IdempotentHint) + + data, err := json.Marshal(annotations) + require.NoError(t, err) + assert.Contains(t, string(data), `"readOnlyHint":false`) + assert.Contains(t, string(data), `"idempotentHint":false`) +} diff --git a/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go b/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go index c6745d2802..966e84392f 100644 --- a/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go +++ b/pkg/tools/builtin/mcpcatalog/mcpcatalog_test.go @@ -1623,6 +1623,12 @@ func TestToolsAuthRequiredIsDeferred(t *testing.T) { // Server-Sent Events. We only need to respond to two methods (initialize // and tools/list) for a successful handshake, then immediately close the // stream so the client moves on. +// +// The fake is intentionally a *legacy* stateful fixture: it answers the +// SDK v1.7 client's server/discover probe with an empty result (the +// default branch below), which makes the client fall back to the legacy +// initialize handshake, and it assigns an Mcp-Session-Id so the +// pre-2026-07-28 stateful behavior keeps being exercised. func newFakeMCPServer(t *testing.T) *httptest.Server { t.Helper() @@ -1673,7 +1679,9 @@ func mcpHandler(t *testing.T, _ bool) http.HandlerFunc { switch body.Method { case "initialize": writeJSONRPC(t, w, body.ID, map[string]any{ - "protocolVersion": "2025-03-26", + // The latest revision that still supports initialize + // (2026-07-28 removed it in favor of server/discover). + "protocolVersion": "2025-11-25", "capabilities": map[string]any{}, "serverInfo": map[string]any{ "name": "fake", diff --git a/pkg/tools/mcp/remote_test.go b/pkg/tools/mcp/remote_test.go index 7e4a133b39..081f0ba9ac 100644 --- a/pkg/tools/mcp/remote_test.go +++ b/pkg/tools/mcp/remote_test.go @@ -2,6 +2,7 @@ package mcp import ( "context" + "encoding/json" "fmt" "net" "net/http" @@ -124,10 +125,21 @@ func TestRemoteClientHeadersWithStreamable(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { capturedRequest = r + // Echo the request id: the v1.7 SDK client probes server/discover + // before falling back to initialize, so a hardcoded id would leave + // the fallback request unanswered and hang the client. + var req struct { + ID json.RawMessage `json:"id"` + } + _ = json.NewDecoder(r.Body).Decode(&req) + if len(req.ID) == 0 { + req.ID = json.RawMessage("null") + } + // Send a minimal response w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - fmt.Fprintf(w, `{"jsonrpc":"2.0","result":{"protocolVersion":"1.0.0","capabilities":{},"serverInfo":{"name":"test","version":"1.0.0"}},"id":1}`) + fmt.Fprintf(w, `{"jsonrpc":"2.0","result":{"protocolVersion":"1.0.0","capabilities":{},"serverInfo":{"name":"test","version":"1.0.0"}},"id":%s}`, req.ID) select { case requestCaptured <- true: From 835349e6d33ec0d6831c9750c71ab69f8447d0fc Mon Sep 17 00:00:00 2001 From: Sayt-0 Date: Mon, 24 Aug 2026 14:24:57 +0200 Subject: [PATCH 2/2] test(mcp): cover MRTR elicitation retry --- pkg/tools/mcp/remote_test.go | 108 +++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/pkg/tools/mcp/remote_test.go b/pkg/tools/mcp/remote_test.go index 1a452ad374..7d78b0df53 100644 --- a/pkg/tools/mcp/remote_test.go +++ b/pkg/tools/mcp/remote_test.go @@ -19,6 +19,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/docker/docker-agent/pkg/tools" "github.com/docker/docker-agent/pkg/upstream" ) @@ -606,6 +607,113 @@ func TestRemoteClientCallToolAbortsOnContextDeadline(t *testing.T) { } } +// TestRemoteClientCallToolMRTRElicitation covers MCP 2026-07-28 +// multi-round-trip elicitation (SEP-2322) through the real remote client +// wiring. The server runs in stateless mode — the transport mode our own +// HTTP server ships with — where mid-call server-initiated JSON-RPC requests +// are impossible, so the tool handler returns CallToolResult.InputRequests +// plus an opaque RequestState instead. The go-sdk client middleware must +// fulfill that request through the repository-level tools.ElicitationHandler +// wired via SetElicitationHandler, retry the call with the elicitation +// response and the echoed RequestState, and surface only the completed +// result to the caller. +func TestRemoteClientCallToolMRTRElicitation(t *testing.T) { + t.Parallel() + + const ( + elicitMessage = "Deploy to production?" + requestState = "deploy-state-42" + ) + + // The tool handler runs on HTTP server goroutines, so no require/t.Fatal + // here: observations travel over a buffered channel and atomics to the + // test goroutine, which asserts after CallTool returns. + type retryCall struct { + requestState string + response gomcp.InputResponse + } + var toolCalls atomic.Int32 + retryCh := make(chan retryCall, 1) + + server := gomcp.NewServer(&gomcp.Implementation{Name: "test-server", Version: "1.0.0"}, nil) + gomcp.AddTool(server, &gomcp.Tool{Name: "deploy", Description: "asks for confirmation"}, + func(_ context.Context, req *gomcp.CallToolRequest, _ struct{}) (*gomcp.CallToolResult, any, error) { + toolCalls.Add(1) + if len(req.Params.InputResponses) == 0 { + return &gomcp.CallToolResult{ + InputRequests: gomcp.InputRequestMap{"confirm": &gomcp.ElicitParams{Message: elicitMessage}}, + RequestState: requestState, + }, nil, nil + } + select { + case retryCh <- retryCall{ + requestState: req.Params.RequestState, + response: req.Params.InputResponses["confirm"], + }: + default: + } + return &gomcp.CallToolResult{Content: []gomcp.Content{&gomcp.TextContent{Text: "deployed"}}}, nil, nil + }) + + httpServer := httptest.NewServer(gomcp.NewStreamableHTTPHandler( + func(*http.Request) *gomcp.Server { return server }, + &gomcp.StreamableHTTPOptions{Stateless: true}, + )) + defer httpServer.Close() + + client := newRemoteClient(httpServer.URL, "streamable", nil, NewInMemoryTokenStore(), nil, false, nil) + + var elicitCalls atomic.Int32 + elicitMessages := make(chan string, 1) + client.SetElicitationHandler(func(_ context.Context, req *gomcp.ElicitParams) (tools.ElicitationResult, error) { + elicitCalls.Add(1) + select { + case elicitMessages <- req.Message: + default: + } + return tools.ElicitationResult{ + Action: tools.ElicitationActionAccept, + Content: map[string]any{"confirmed": true}, + }, nil + }) + + _, err := client.Initialize(t.Context(), nil) + require.NoError(t, err) + defer func() { _ = client.Close(context.WithoutCancel(t.Context())) }() + + result, err := client.CallTool(t.Context(), &gomcp.CallToolParams{Name: "deploy"}) + require.NoError(t, err) + require.NotNil(t, result) + + require.Equal(t, int32(1), elicitCalls.Load(), "elicitation handler must run exactly once") + select { + case msg := <-elicitMessages: + assert.Equal(t, elicitMessage, msg, "handler must receive the server's elicitation message") + default: + t.Fatal("elicitation handler never recorded the message it received") + } + + require.Equal(t, int32(2), toolCalls.Load(), "tool handler must run exactly twice: initial call + middleware retry") + var retry retryCall + select { + case retry = <-retryCh: + default: + t.Fatal("server never observed the retry carrying the input responses") + } + assert.Equal(t, requestState, retry.requestState, "retry must echo the exact RequestState of the input-required result") + elicitResult, ok := retry.response.(*gomcp.ElicitResult) + require.True(t, ok, "retry must carry the keyed *gomcp.ElicitResult, got %T", retry.response) + assert.Equal(t, string(tools.ElicitationActionAccept), elicitResult.Action) + assert.Equal(t, map[string]any{"confirmed": true}, elicitResult.Content) + + assert.False(t, result.NeedsInput(), "final result must be complete, not input-required") + assert.False(t, result.IsError) + require.Len(t, result.Content, 1) + text, ok := result.Content[0].(*gomcp.TextContent) + require.True(t, ok, "expected text content, got %T", result.Content[0]) + assert.Equal(t, "deployed", text.Text) +} + // mutableEnvProvider is a context-aware, mutable environment.Provider for // tests that need to change a value between two requests on the same // connection.