From 60c7388da9b5b3f15cff68bd18736d9b00bac945 Mon Sep 17 00:00:00 2001 From: Sam Morrow Date: Thu, 21 May 2026 14:33:10 +0200 Subject: [PATCH] refactor: address review feedback on PR #2450 - Remove unrelated X-Custom-Auth-Headers from CORS allowlist - Replace per-type sort functions with generic sortByToolset[T] - Add doc comments explaining why WithoutFeatureFiltering is needed (HTTP mode feature flags arrive per-request, so static schema must include all variants as an upper bound) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/http/middleware/cors.go | 1 - pkg/inventory/filters.go | 54 +++++++++++++++---------------------- 2 files changed, 22 insertions(+), 33 deletions(-) diff --git a/pkg/http/middleware/cors.go b/pkg/http/middleware/cors.go index fb8e8e548d..2eaf4227b4 100644 --- a/pkg/http/middleware/cors.go +++ b/pkg/http/middleware/cors.go @@ -17,7 +17,6 @@ func SetCorsHeaders(h http.Handler) http.Handler { "Mcp-Session-Id", "Mcp-Protocol-Version", "Last-Event-ID", - "X-Custom-Auth-Headers", headers.AuthorizationHeader, headers.MCPReadOnlyHeader, headers.MCPToolsetsHeader, diff --git a/pkg/inventory/filters.go b/pkg/inventory/filters.go index 20a157de63..8af3a68168 100644 --- a/pkg/inventory/filters.go +++ b/pkg/inventory/filters.go @@ -103,12 +103,15 @@ func (r *Inventory) isToolEnabledWithFeatureFlags(ctx context.Context, tool *Ser return true } -func sortTools(tools []ServerTool) { - sort.Slice(tools, func(i, j int) bool { - if tools[i].Toolset.ID != tools[j].Toolset.ID { - return tools[i].Toolset.ID < tools[j].Toolset.ID +// sortByToolset sorts a slice in place by toolset ID then item name. +func sortByToolset[T any](items []T, key func(T) (ToolsetID, string)) { + sort.Slice(items, func(i, j int) bool { + idI, nameI := key(items[i]) + idJ, nameJ := key(items[j]) + if idI != idJ { + return idI < idJ } - return tools[i].Tool.Name < tools[j].Tool.Name + return nameI < nameJ }) } @@ -125,13 +128,16 @@ func (r *Inventory) AvailableTools(ctx context.Context) []ServerTool { } // Sort deterministically: by toolset ID, then by tool name - sortTools(result) + sortByToolset(result, func(t ServerTool) (ToolsetID, string) { return t.Toolset.ID, t.Tool.Name }) return result } // AvailableToolsWithoutFeatureFiltering returns tools that pass every filter -// except FeatureFlagEnable/FeatureFlagDisable. +// except FeatureFlagEnable/FeatureFlagDisable. This is needed for the HTTP +// handler's static schema: feature flags arrive per-request via X-MCP-Features +// header, so the static tool list must include all feature-gated variants as an +// upper bound that per-request evaluation can narrow. func (r *Inventory) AvailableToolsWithoutFeatureFiltering(ctx context.Context) []ServerTool { var result []ServerTool for i := range r.tools { @@ -141,20 +147,11 @@ func (r *Inventory) AvailableToolsWithoutFeatureFiltering(ctx context.Context) [ } } - sortTools(result) + sortByToolset(result, func(t ServerTool) (ToolsetID, string) { return t.Toolset.ID, t.Tool.Name }) return result } -func sortResourceTemplates(resourceTemplates []ServerResourceTemplate) { - sort.Slice(resourceTemplates, func(i, j int) bool { - if resourceTemplates[i].Toolset.ID != resourceTemplates[j].Toolset.ID { - return resourceTemplates[i].Toolset.ID < resourceTemplates[j].Toolset.ID - } - return resourceTemplates[i].Template.Name < resourceTemplates[j].Template.Name - }) -} - // AvailableResourceTemplates returns resource templates that pass all current filters, // sorted deterministically by toolset ID, then template name. // The context is used for feature flag evaluation. @@ -172,13 +169,14 @@ func (r *Inventory) AvailableResourceTemplates(ctx context.Context) []ServerReso } // Sort deterministically: by toolset ID, then by template name - sortResourceTemplates(result) + sortByToolset(result, func(r ServerResourceTemplate) (ToolsetID, string) { return r.Toolset.ID, r.Template.Name }) return result } // AvailableResourceTemplatesWithoutFeatureFiltering returns resource templates -// that pass every filter except FeatureFlagEnable/FeatureFlagDisable. +// that pass every filter except FeatureFlagEnable/FeatureFlagDisable. See +// AvailableToolsWithoutFeatureFiltering for rationale. func (r *Inventory) AvailableResourceTemplatesWithoutFeatureFiltering(_ context.Context) []ServerResourceTemplate { var result []ServerResourceTemplate for i := range r.resourceTemplates { @@ -188,20 +186,11 @@ func (r *Inventory) AvailableResourceTemplatesWithoutFeatureFiltering(_ context. } } - sortResourceTemplates(result) + sortByToolset(result, func(r ServerResourceTemplate) (ToolsetID, string) { return r.Toolset.ID, r.Template.Name }) return result } -func sortPrompts(prompts []ServerPrompt) { - sort.Slice(prompts, func(i, j int) bool { - if prompts[i].Toolset.ID != prompts[j].Toolset.ID { - return prompts[i].Toolset.ID < prompts[j].Toolset.ID - } - return prompts[i].Prompt.Name < prompts[j].Prompt.Name - }) -} - // AvailablePrompts returns prompts that pass all current filters, // sorted deterministically by toolset ID, then prompt name. // The context is used for feature flag evaluation. @@ -219,13 +208,14 @@ func (r *Inventory) AvailablePrompts(ctx context.Context) []ServerPrompt { } // Sort deterministically: by toolset ID, then by prompt name - sortPrompts(result) + sortByToolset(result, func(p ServerPrompt) (ToolsetID, string) { return p.Toolset.ID, p.Prompt.Name }) return result } // AvailablePromptsWithoutFeatureFiltering returns prompts that pass every filter -// except FeatureFlagEnable/FeatureFlagDisable. +// except FeatureFlagEnable/FeatureFlagDisable. See +// AvailableToolsWithoutFeatureFiltering for rationale. func (r *Inventory) AvailablePromptsWithoutFeatureFiltering(_ context.Context) []ServerPrompt { var result []ServerPrompt for i := range r.prompts { @@ -235,7 +225,7 @@ func (r *Inventory) AvailablePromptsWithoutFeatureFiltering(_ context.Context) [ } } - sortPrompts(result) + sortByToolset(result, func(p ServerPrompt) (ToolsetID, string) { return p.Toolset.ID, p.Prompt.Name }) return result }