Omit absent JSON-RPC ids instead of emitting null

pkg/mcp/batch.go

@@ -49,9 +49,11 @@ func IsBatchRequest(body []byte) bool {
 }
 
 // WriteBatchUnsupportedError writes an HTTP 400 response carrying a JSON-RPC
-// "Invalid Request" error for a rejected batch. The JSON-RPC id is null: a
-// batch has no single request id to echo. Use this where an http.ResponseWriter
-// is available (the streamable proxy, ParsingMiddleware).
+// "Invalid Request" error for a rejected batch. A batch has no single request
+// id to echo, so the "id" key is omitted entirely (schema/2025-11-25 types the
+// error response's id as optional, not nullable; see session.HasJSONRPCID).
+// Use this where an http.ResponseWriter is available (the streamable proxy,
+// ParsingMiddleware).
 func WriteBatchUnsupportedError(w http.ResponseWriter) {
 	body := classificationErrorBody(nil, batchUnsupported)
 	w.Header().Set("Content-Type", "application/json")
@@ -63,7 +65,7 @@ func WriteBatchUnsupportedError(w http.ResponseWriter) {
 // BatchUnsupportedResponse builds an *http.Response carrying the batch-rejection
 // JSON-RPC error, for proxies that intercept at the RoundTripper layer (the
 // transparent proxy) where no http.ResponseWriter is available. Mirrors
-// ClassificationErrorResponse; the JSON-RPC id is null.
+// ClassificationErrorResponse; the JSON-RPC id is omitted, not null.
 func BatchUnsupportedResponse(req *http.Request) *http.Response {
 	return jsonRPCErrorResponse(req, nil, batchUnsupported)
 }

pkg/mcp/batch_test.go

@@ -62,15 +62,22 @@ func TestWriteBatchUnsupportedError(t *testing.T) {
 
 	var resp struct {
 		JSONRPC string `json:"jsonrpc"`
-		ID      any    `json:"id"`
 		Error   struct {
 			Code    int64  `json:"code"`
 			Message string `json:"message"`
 		} `json:"error"`
 	}
 	require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
 	assert.Equal(t, "2.0", resp.JSONRPC)
-	assert.Nil(t, resp.ID)
 	assert.Equal(t, CodeInvalidRequest, resp.Error.Code)
 	assert.NotEmpty(t, resp.Error.Message)
+
+	// A batch has no single request id to echo. A tagged-struct decode
+	// (above) can't tell an absent "id" key from a present null -- both
+	// decode to the zero value -- so the omission is checked via a map
+	// decode instead.
+	var asMap map[string]any
+	require.NoError(t, json.Unmarshal(w.Body.Bytes(), &asMap))
+	_, hasID := asMap["id"]
+	assert.False(t, hasID, `"id" key must be omitted, not present as null`)
 }

pkg/mcp/classification_response.go

@@ -10,6 +10,8 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+
+	"github.com/stacklok/toolhive/pkg/transport/session"
 )
 
 // WriteClassificationError writes an HTTP 400 response with a JSON-RPC error
@@ -65,6 +67,11 @@ func jsonRPCErrorResponse(req *http.Request, requestID any, err error) *http.Res
 // the error implements CodedError, falling back to the standard JSON-RPC
 // Invalid Params code otherwise -- a fallback that is currently unreachable,
 // since every error ClassifyRevision returns implements CodedError.
+//
+// MCP narrows base JSON-RPC 2.0 here: schema/2025-11-25 types the error
+// response as `id?: RequestId` where RequestId = string | number, so an
+// absent id is encoded by omitting the "id" key, never as null. See
+// session.HasJSONRPCID.
 func classificationErrorBody(requestID any, err error) []byte {
 	code := CodeInvalidParams
 	var coded CodedError
@@ -84,14 +91,16 @@ func classificationErrorBody(requestID any, err error) []byte {
 	resp := map[string]any{
 		"jsonrpc": "2.0",
 		"error":   errBody,
-		"id":      requestID,
+	}
+	if session.HasJSONRPCID(requestID) {
+		resp["id"] = requestID
 	}
 
 	body, marshalErr := json.Marshal(resp)
 	if marshalErr != nil {
 		// This should never happen with simple map types, but return a
 		// hand-crafted fallback to guarantee a valid JSON-RPC error.
-		return []byte(`{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params"},"id":null}`)
+		return []byte(`{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params"}}`)
 	}
 	return body
 }

pkg/mcp/classification_response_test.go

@@ -10,59 +10,106 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestClassificationError(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name      string
-		requestID any
-		err       error
-		wantCode  int64
-		wantData  bool
+		name       string
+		requestID  any
+		err        error
+		wantCode   int64
+		wantData   bool
+		wantIDKey  bool
+		wantIDEcho string // substring the body must contain, verbatim, when wantIDKey
 	}{
 		{
-			name:      "header mismatch",
-			requestID: "req-1",
-			err:       &HeaderMismatchError{Header: "2026-07-28", Body: "2025-11-25"},
-			wantCode:  CodeHeaderMismatch,
-			wantData:  true,
+			name:       "header mismatch",
+			requestID:  "req-1",
+			err:        &HeaderMismatchError{Header: "2026-07-28", Body: "2025-11-25"},
+			wantCode:   CodeHeaderMismatch,
+			wantData:   true,
+			wantIDKey:  true,
+			wantIDEcho: `"id":"req-1"`,
 		},
 		{
-			name:      "unsupported version",
-			requestID: float64(42),
-			err:       &UnsupportedVersionError{Requested: "1999-01-01", Supported: []string{MCPVersionModern}},
-			wantCode:  CodeUnsupportedProtocolVersion,
-			wantData:  true,
+			name:       "unsupported version",
+			requestID:  float64(42),
+			err:        &UnsupportedVersionError{Requested: "1999-01-01", Supported: []string{MCPVersionModern}},
+			wantCode:   CodeUnsupportedProtocolVersion,
+			wantData:   true,
+			wantIDKey:  true,
+			wantIDEcho: `"id":42`,
 		},
 		{
-			name:      "missing client capability",
-			requestID: "req-3",
-			err:       &MissingClientCapabilityError{RequiredCapabilities: map[string]any{}},
-			wantCode:  CodeMissingClientCapability,
-			wantData:  true,
+			name:       "missing client capability",
+			requestID:  "req-3",
+			err:        &MissingClientCapabilityError{RequiredCapabilities: map[string]any{}},
+			wantCode:   CodeMissingClientCapability,
+			wantData:   true,
+			wantIDKey:  true,
+			wantIDEcho: `"id":"req-3"`,
 		},
 		{
-			name:      "missing modern metadata",
-			requestID: "req-4",
-			err:       &MissingModernMetadataError{},
-			wantCode:  CodeInvalidParams,
-			wantData:  false, // Data() returns an empty (non-nil) map, so len(data) == 0
+			name:       "missing modern metadata",
+			requestID:  "req-4",
+			err:        &MissingModernMetadataError{},
+			wantCode:   CodeInvalidParams,
+			wantData:   false, // Data() returns an empty (non-nil) map, so len(data) == 0
+			wantIDKey:  true,
+			wantIDEcho: `"id":"req-4"`,
 		},
 		{
-			name:      "nil request id",
+			// A nil requestID means the caller couldn't determine an id at
+			// all (e.g. the request failed to parse). MCP narrows base
+			// JSON-RPC to omit the "id" key here rather than emit
+			// "id":null: see session.HasJSONRPCID.
+			name:      "nil request id omits the id key",
 			requestID: nil,
 			err:       &HeaderMismatchError{Header: "a", Body: "b"},
 			wantCode:  CodeHeaderMismatch,
 			wantData:  true,
+			wantIDKey: false,
+		},
+		{
+			// The transparent proxy threads the incoming id through as raw
+			// bytes; literal null must be treated the same as a missing id.
+			name:      "raw null request id omits the id key",
+			requestID: json.RawMessage(`null`),
+			err:       &HeaderMismatchError{Header: "a", Body: "b"},
+			wantCode:  CodeHeaderMismatch,
+			wantData:  true,
+			wantIDKey: false,
+		},
+		{
+			name:      "nil RawMessage omits the id key",
+			requestID: json.RawMessage(nil),
+			err:       &HeaderMismatchError{Header: "a", Body: "b"},
+			wantCode:  CodeHeaderMismatch,
+			wantData:  true,
+			wantIDKey: false,
+		},
+		{
+			name:       "zero request id is still present",
+			requestID:  float64(0),
+			err:        &HeaderMismatchError{Header: "a", Body: "b"},
+			wantCode:   CodeHeaderMismatch,
+			wantData:   true,
+			wantIDKey:  true,
+			wantIDEcho: `"id":0`,
 		},
 		{
-			name:      "plain non-coded error falls back to invalid params",
-			requestID: "req-6",
-			err:       errors.New("boom"),
-			wantCode:  CodeInvalidParams,
-			wantData:  false,
+			name:       "plain non-coded error falls back to invalid params",
+			requestID:  "req-6",
+			err:        errors.New("boom"),
+			wantCode:   CodeInvalidParams,
+			wantData:   false,
+			wantIDKey:  true,
+			wantIDEcho: `"id":"req-6"`,
 		},
 	}
 
@@ -115,7 +162,6 @@ func TestClassificationError(t *testing.T) {
 					Message string         `json:"message"`
 					Data    map[string]any `json:"data"`
 				} `json:"error"`
-				ID any `json:"id"`
 			}
 			if err := json.Unmarshal(wireBody, &decoded); err != nil {
 				t.Fatalf("unmarshaling response body: %v", err)
@@ -137,13 +183,19 @@ func TestClassificationError(t *testing.T) {
 				t.Errorf("expected no data, got %v", decoded.Error.Data)
 			}
 
-			gotID, wantID := decoded.ID, tt.requestID
-			if wantID == nil {
-				if gotID != nil {
-					t.Errorf("id = %v, want nil", gotID)
-				}
-			} else if gotID != wantID {
-				t.Errorf("id = %v (%T), want %v (%T)", gotID, gotID, wantID, wantID)
+			// Decoding into map[string]any (rather than a tagged struct) is
+			// the only way to tell an absent "id" key apart from a present
+			// null: a struct field would read as the zero value either way.
+			var asMap map[string]any
+			require.NoError(t, json.Unmarshal(wireBody, &asMap))
+			_, hasID := asMap["id"]
+			assert.Equal(t, tt.wantIDKey, hasID, `"id" key presence`)
+			if tt.wantIDKey {
+				require.NotEmpty(t, tt.wantIDEcho, "table entry must set wantIDEcho when wantIDKey is true")
+				// A map decode would coerce a numeric id to float64, losing
+				// the verbatim-echo guarantee #5945 pinned -- so check the
+				// raw wire bytes instead.
+				assert.Contains(t, string(wireBody), tt.wantIDEcho)
 			}
 		})
 	}
@@ -157,7 +209,7 @@ func TestClassificationError(t *testing.T) {
 func TestClassificationErrorBodyMarshalFallback(t *testing.T) {
 	t.Parallel()
 
-	const want = `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params"},"id":null}`
+	const want = `{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params"}}`
 	got := classificationErrorBody(make(chan int), errors.New("boom"))
 	if string(got) != want {
 		t.Fatalf("fallback body = %s, want %s", got, want)

pkg/mcp/parser_test.go

@@ -284,17 +284,24 @@ func TestParsingMiddlewareRejectsBatch(t *testing.T) {
 
 			var resp struct {
 				JSONRPC string `json:"jsonrpc"`
-				ID      any    `json:"id"`
 				Error   struct {
 					Code    int64  `json:"code"`
 					Message string `json:"message"`
 				} `json:"error"`
 			}
 			require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
 			assert.Equal(t, "2.0", resp.JSONRPC)
-			assert.Nil(t, resp.ID, "batch rejection carries a null JSON-RPC id")
 			assert.Equal(t, CodeInvalidRequest, resp.Error.Code)
 			assert.NotEmpty(t, resp.Error.Message)
+
+			// A batch has no single request id to echo. A tagged-struct
+			// decode (above) can't tell an absent "id" key apart from a
+			// present null -- both decode to the zero value -- so the
+			// omission is checked via a map decode instead.
+			var asMap map[string]any
+			require.NoError(t, json.Unmarshal(w.Body.Bytes(), &asMap))
+			_, hasID := asMap["id"]
+			assert.False(t, hasID, `"id" key must be omitted, not present as null`)
 		})
 	}
 }

pkg/mcp/tool_filter.go

@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/stacklok/toolhive/pkg/transport/session"
 	"github.com/stacklok/toolhive/pkg/transport/types"
 )
 
@@ -388,21 +389,28 @@ func writeFilteredToolCallError(w http.ResponseWriter, id any) {
 // tool call, modeled on classificationErrorBody: the body is marshaled first
 // (with a hand-crafted fallback on marshal failure) so the caller only writes
 // headers/status once a valid body is ready.
+//
+// MCP narrows base JSON-RPC 2.0 here: schema/2025-11-25 types the error
+// response as `id?: RequestId` where RequestId = string | number, so an
+// absent id is encoded by omitting the "id" key, never as null. See
+// session.HasJSONRPCID.
 func filteredToolCallErrorBody(id any) []byte {
 	resp := map[string]any{
 		"jsonrpc": "2.0",
 		"error": map[string]any{
 			"code":    CodeInvalidParams,
 			"message": filteredToolNotFoundMessage,
 		},
-		"id": id,
+	}
+	if session.HasJSONRPCID(id) {
+		resp["id"] = id
 	}
 
 	body, err := json.Marshal(resp)
 	if err != nil {
 		// This should never happen with simple map types, but return a
 		// hand-crafted fallback to guarantee a valid JSON-RPC error.
-		return []byte(`{"jsonrpc":"2.0","error":{"code":-32602,"message":"tool not found"},"id":null}`)
+		return []byte(`{"jsonrpc":"2.0","error":{"code":-32602,"message":"tool not found"}}`)
 	}
 	return body
 }

pkg/mcp/tool_filter_test.go

@@ -1945,6 +1945,16 @@ func TestNewToolCallMappingMiddleware_FilteredTool(t *testing.T) {
 			expectStatus:  http.StatusOK,
 			expectJSONRPC: true,
 		},
+		{
+			// Zero is a valid JSON-RPC id and must stay present, not be
+			// mistaken for absent.
+			name:          "Accept: application/json - zero id is still present",
+			accept:        "application/json",
+			setAccept:     true,
+			id:            0,
+			expectStatus:  http.StatusOK,
+			expectJSONRPC: true,
+		},
 		{
 			name:          "Accept: application/json, text/event-stream",
 			accept:        "application/json, text/event-stream",
@@ -1968,7 +1978,11 @@ func TestNewToolCallMappingMiddleware_FilteredTool(t *testing.T) {
 			expectJSONRPC: true,
 		},
 		{
-			name:          "null id is echoed as null",
+			// MCP narrows base JSON-RPC 2.0 here: schema/2025-11-25 types the
+			// error response id as optional, not nullable, so a request with
+			// no usable id gets an error body that omits the "id" key
+			// entirely rather than echoing "id":null (session.HasJSONRPCID).
+			name:          "null id omits the id key",
 			accept:        "application/json",
 			setAccept:     true,
 			id:            nil,
@@ -2025,6 +2039,16 @@ func TestNewToolCallMappingMiddleware_FilteredTool(t *testing.T) {
 			assert.NotContains(t, errObj["message"], "filter",
 				"a filtered tool must look the same as a nonexistent one")
 
+			// Map decode is the only way to tell an absent "id" key apart
+			// from a present null -- response["id"] reads back as the same
+			// nil interface value either way.
+			_, hasID := response["id"]
+			if tt.id == nil {
+				assert.False(t, hasID, `"id" key must be omitted for a nil request id, not present as null`)
+				return
+			}
+			assert.True(t, hasID, `"id" key must be present`)
+
 			expectedID, err := json.Marshal(tt.id)
 			require.NoError(t, err)
 			actualID, err := json.Marshal(response["id"])