fix(inventory): clone schema before header annotation to avoid shared-map race

AnnotateHeaderParams mutated the *jsonschema.Schema (and per-property Extra
maps) shared with the original tool definition via the caller's shallow copy.
Under per-request registration (remote server), concurrent requests could race
on — and fatally panic from — the same Extra map. Now clone only what we touch
(schema value, Properties map, annotated property schemas + their Extra maps);
the original is never written. Adds a no-mutation test and a 64-goroutine
race regression (go test -race clean).

Addresses Copilot review on #2787.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Copilot

pkg/inventory/server_tool.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 
 	"github.com/github/github-mcp-server/pkg/octicons"
 	"github.com/google/jsonschema-go/jsonschema"
@@ -130,25 +131,49 @@ func (st *ServerTool) RegisterFunc(s *mcp.Server, deps any) {
 // for every tool; the enforcement test in pkg/github guards full coverage.
 var HeaderParams = map[string]string{"owner": "owner", "repo": "repo"}
 
-// AnnotateHeaderParams adds an "x-mcp-header" annotation to matching top-level
-// input properties, which the SDK projects onto Mcp-Param-{name} request headers.
+// AnnotateHeaderParams returns a copy of tool whose routing-relevant input
+// properties (per HeaderParams) carry an "x-mcp-header" annotation, which the
+// SDK projects onto Mcp-Param-{name} request headers. It never mutates the
+// input tool's schema or any map shared with the original tool definition:
+// callers shallow-copy ServerTool.Tool, so the *jsonschema.Schema (and its
+// per-property Extra maps) are shared, and per-request registration must not
+// race on them. Only the schema, its Properties map, and the specific property
+// schemas/Extra maps that gain an annotation are cloned.
 func AnnotateHeaderParams(tool *mcp.Tool) {
 	schema, ok := tool.InputSchema.(*jsonschema.Schema)
 	if !ok || schema == nil {
 		return
 	}
-	for prop, header := range HeaderParams {
-		ps := schema.Properties[prop]
-		if ps == nil {
-			continue
-		}
-		if ps.Extra == nil {
-			ps.Extra = map[string]any{}
-		}
-		if _, exists := ps.Extra["x-mcp-header"]; !exists {
-			ps.Extra["x-mcp-header"] = header
+
+	// Collect params that actually need an annotation, so a tool without
+	// owner/repo (or already annotated) is left untouched and unCloned.
+	var toAnnotate []string
+	for prop := range HeaderParams {
+		if ps := schema.Properties[prop]; ps != nil {
+			if _, exists := ps.Extra["x-mcp-header"]; !exists {
+				toAnnotate = append(toAnnotate, prop)
+			}
 		}
 	}
+	if len(toAnnotate) == 0 {
+		return
+	}
+
+	// Clone only what we mutate: a fresh schema value, a fresh Properties map,
+	// and fresh property schemas with fresh Extra maps. The original schema and
+	// its maps are never written to, so concurrent per-request registration is
+	// race-free and deterministic.
+	schemaCopy := *schema
+	schemaCopy.Properties = maps.Clone(schema.Properties)
+	for _, prop := range toAnnotate {
+		propCopy := *schemaCopy.Properties[prop]
+		extra := make(map[string]any, len(propCopy.Extra)+1)
+		maps.Copy(extra, propCopy.Extra)
+		extra["x-mcp-header"] = HeaderParams[prop]
+		propCopy.Extra = extra
+		schemaCopy.Properties[prop] = &propCopy
+	}
+	tool.InputSchema = &schemaCopy
 }
 
 // NewServerToolWithContextHandler creates a ServerTool with a handler that receives deps via context.

pkg/inventory/server_tool_test.go

@@ -3,6 +3,7 @@ package inventory
 import (
 	"context"
 	"encoding/json"
+	"sync"
 	"testing"
 
 	"github.com/google/jsonschema-go/jsonschema"
@@ -99,3 +100,39 @@ func TestAnnotateHeaderParams(t *testing.T) {
 	AnnotateHeaderParams(&mcp.Tool{InputSchema: &jsonschema.Schema{Properties: map[string]*jsonschema.Schema{"x": {}}}})
 	AnnotateHeaderParams(&mcp.Tool{InputSchema: json.RawMessage(`{}`)})
 }
+
+func TestAnnotateHeaderParams_DoesNotMutateOriginal(t *testing.T) {
+	orig := &jsonschema.Schema{
+		Type:       "object",
+		Properties: map[string]*jsonschema.Schema{"owner": {Type: "string"}, "repo": {Type: "string"}},
+	}
+	tool := &mcp.Tool{InputSchema: orig}
+	AnnotateHeaderParams(tool)
+
+	// Original schema and its property Extra maps must be untouched.
+	require.Nil(t, orig.Properties["owner"].Extra, "must not mutate original owner schema")
+	require.Nil(t, orig.Properties["repo"].Extra, "must not mutate original repo schema")
+	// Returned copy carries the annotation.
+	got := tool.InputSchema.(*jsonschema.Schema)
+	require.NotSame(t, orig, got, "must replace InputSchema with a copy")
+	require.Equal(t, "owner", got.Properties["owner"].Extra["x-mcp-header"])
+}
+
+func TestAnnotateHeaderParams_ConcurrentRegistrationIsRaceFree(t *testing.T) {
+	// Shared base schema, as ServerTool.Tool is shallow-copied per registration.
+	base := &jsonschema.Schema{
+		Type:       "object",
+		Properties: map[string]*jsonschema.Schema{"owner": {Type: "string"}, "repo": {Type: "string"}},
+	}
+	var wg sync.WaitGroup
+	for range 64 {
+		wg.Go(func() {
+			tool := mcp.Tool{InputSchema: base} // shallow copy shares *Schema
+			AnnotateHeaderParams(&tool)
+			got := tool.InputSchema.(*jsonschema.Schema)
+			require.Equal(t, "repo", got.Properties["repo"].Extra["x-mcp-header"])
+		})
+	}
+	wg.Wait()
+	require.Nil(t, base.Properties["owner"].Extra, "shared base must remain unmutated")
+}