Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix(inventory): normalize owned metadata
Canonicalize JSON metadata at builder boundaries, report invalid values from Build, and preserve wire semantics for typed mutable values.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1e4a1ca6-53f7-4158-af22-35d2448d0b13
  • Loading branch information
SamMorrowDrums and Copilot committed Sep 2, 2026
commit e070a779d16b0749692054a591dde2f1f4cdcd61
53 changes: 48 additions & 5 deletions pkg/inventory/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ const mcpAppsFeatureFlag FeatureFlag = "remote_mcp_ui_apps"
type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error)

// Builder builds a Registry with the specified configuration. SetTools,
// SetResources, and SetPrompts copy the feature and metadata state retained by
// the inventory.
// SetResources, and SetPrompts normalize retained metadata through JSON so the
// inventory owns it; Build reports metadata that cannot be represented as JSON.
// Use NewBuilder to create a builder, chain configuration methods,
// then call Build() to create the final inventory.
//
Expand All @@ -45,6 +45,9 @@ type Builder struct {
tools []ServerTool
resourceTemplates []ServerResourceTemplate
prompts []ServerPrompt
toolsErr error
resourcesErr error
promptsErr error
deprecatedAliases map[string]string

// Configuration options (processed at Build time)
Expand All @@ -68,30 +71,66 @@ func NewBuilder() *Builder {
// SetTools sets the tools for the inventory. Returns self for chaining.
func (b *Builder) SetTools(tools []ServerTool) *Builder {
b.tools = slices.Clone(tools)
b.toolsErr = nil
for i := range b.tools {
b.tools[i] = cloneServerTool(b.tools[i])
tool, err := ownServerTool(b.tools[i])
if err != nil {
b.toolsErr = errors.Join(b.toolsErr, fmt.Errorf("tool %q metadata: %w", b.tools[i].Tool.Name, err))
}
b.tools[i] = tool
}
return b
}

// SetResources sets the resource templates for the inventory. Returns self for chaining.
func (b *Builder) SetResources(resources []ServerResourceTemplate) *Builder {
b.resourceTemplates = slices.Clone(resources)
b.resourcesErr = nil
for i := range b.resourceTemplates {
b.resourceTemplates[i] = cloneResourceTemplate(b.resourceTemplates[i])
resource, err := ownResourceTemplate(b.resourceTemplates[i])
if err != nil {
b.resourcesErr = errors.Join(b.resourcesErr, fmt.Errorf("resource template %q metadata: %w", b.resourceTemplates[i].Template.Name, err))
}
b.resourceTemplates[i] = resource
}
return b
}

// SetPrompts sets the prompts for the inventory. Returns self for chaining.
func (b *Builder) SetPrompts(prompts []ServerPrompt) *Builder {
b.prompts = slices.Clone(prompts)
b.promptsErr = nil
for i := range b.prompts {
b.prompts[i] = clonePrompt(b.prompts[i])
prompt, err := ownPrompt(b.prompts[i])
if err != nil {
b.promptsErr = errors.Join(b.promptsErr, fmt.Errorf("prompt %q metadata: %w", b.prompts[i].Prompt.Name, err))
}
b.prompts[i] = prompt
}
return b
}

func ownServerTool(tool ServerTool) (ServerTool, error) {
meta, err := normalizeMeta(tool.Tool.Meta)
tool.Tool.Meta = meta
tool.FeatureRule = tool.FeatureRule.clone()
return tool, err
}

func ownResourceTemplate(resource ServerResourceTemplate) (ServerResourceTemplate, error) {
meta, err := normalizeMeta(resource.Template.Meta)
resource.Template.Meta = meta
resource.FeatureRule = resource.FeatureRule.clone()
return resource, err
}

func ownPrompt(prompt ServerPrompt) (ServerPrompt, error) {
meta, err := normalizeMeta(prompt.Prompt.Meta)
prompt.Prompt.Meta = meta
prompt.FeatureRule = prompt.FeatureRule.clone()
return prompt, err
}

func cloneServerTool(tool ServerTool) ServerTool {
tool.Tool.Meta = cloneMeta(tool.Tool.Meta)
tool.FeatureRule = tool.FeatureRule.clone()
Expand Down Expand Up @@ -234,6 +273,10 @@ func cleanTools(tools []string) []string {
// (i.e., they don't exist in the tool set and are not deprecated aliases).
// This ensures invalid tool configurations fail fast at build time.
func (b *Builder) Build() (*Inventory, error) {
if err := errors.Join(b.toolsErr, b.resourcesErr, b.promptsErr); err != nil {
return nil, fmt.Errorf("invalid inventory metadata: %w", err)
}

tools := b.tools

filters := b.filters
Expand Down
24 changes: 20 additions & 4 deletions pkg/inventory/metadata.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
package inventory

import (
"slices"
"encoding/json"
"fmt"

"github.com/modelcontextprotocol/go-sdk/mcp"
)

func normalizeMeta(meta mcp.Meta) (mcp.Meta, error) {
if meta == nil {
return nil, nil
}
data, err := json.Marshal(meta)
if err != nil {
return nil, fmt.Errorf("marshal metadata: %w", err)
}
var normalized mcp.Meta
if err := json.Unmarshal(data, &normalized); err != nil {
Comment thread
Copilot marked this conversation as resolved.
Outdated
return nil, fmt.Errorf("unmarshal metadata: %w", err)
}
return normalized, nil
}

func cloneMeta(meta mcp.Meta) mcp.Meta {
if meta == nil {
return nil
Expand Down Expand Up @@ -33,9 +49,9 @@ func cloneMetaValue(value any) any {
clone[i] = cloneMetaValue(item)
}
return clone
case []string:
return slices.Clone(value)
default:
case nil, bool, float64, string:
return value
default:
panic(fmt.Sprintf("metadata value %T is not normalized", value))
}
}
116 changes: 95 additions & 21 deletions pkg/inventory/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package inventory

import (
"context"
"encoding/base64"
"encoding/json"
"fmt"
"testing"
Expand Down Expand Up @@ -1309,6 +1310,9 @@ func TestBuilderOwnsFeatureMetadataInputs(t *testing.T) {
prompts := []ServerPrompt{mockPrompt("prompt", "toolset1")}
prompts[0].FeatureRule = promptRule
prompts[0].Prompt.Meta = nestedTestMeta("prompt", "prompt")
expectedToolMeta := mustMarshalJSON(t, tools[0].Tool.Meta)
expectedResourceMeta := mustMarshalJSON(t, resources[0].Template.Meta)
expectedPromptMeta := mustMarshalJSON(t, prompts[0].Prompt.Meta)

checker := func(_ context.Context, flag FeatureFlag) (bool, error) {
return flag == "tool" || flag == "resource" || flag == "prompt" || flag == mcpAppsFeatureFlag, nil
Expand All @@ -1320,16 +1324,16 @@ func TestBuilderOwnsFeatureMetadataInputs(t *testing.T) {
WithToolsets([]string{"all"}).
WithFeatureChecker(checker)

setNestedTestMeta(tools[0].Tool.Meta, "ui", "after-set")
setNestedTestMeta(resources[0].Template.Meta, "resource", "after-set")
setNestedTestMeta(prompts[0].Prompt.Meta, "prompt", "after-set")
mutateSourceTestMeta(tools[0].Tool.Meta, "ui", "after-set", 's')
mutateSourceTestMeta(resources[0].Template.Meta, "resource", "after-set", 's')
mutateSourceTestMeta(prompts[0].Prompt.Meta, "prompt", "after-set", 's')
inv := mustBuild(t, builder)
tools[0].FeatureRule.features[0] = "changed"
setNestedTestMeta(tools[0].Tool.Meta, "ui", "after-build")
mutateSourceTestMeta(tools[0].Tool.Meta, "ui", "after-build", 'b')
resources[0].FeatureRule.features[0] = "changed"
setNestedTestMeta(resources[0].Template.Meta, "resource", "after-build")
mutateSourceTestMeta(resources[0].Template.Meta, "resource", "after-build", 'b')
prompts[0].FeatureRule.features[0] = "changed"
setNestedTestMeta(prompts[0].Prompt.Meta, "prompt", "after-build")
mutateSourceTestMeta(prompts[0].Prompt.Meta, "prompt", "after-build", 'b')

require.Equal(t, []FeatureFlag{"prompt", "remote_mcp_ui_apps", "resource", "tool"}, inv.RequiredFeatures())
availableTools := inv.AvailableTools(context.Background())
Expand All @@ -1342,18 +1346,18 @@ func TestBuilderOwnsFeatureMetadataInputs(t *testing.T) {
requireNestedTestMeta(t, availableResources[0].Template.Meta, "resource", "resource")
requireNestedTestMeta(t, availablePrompts[0].Prompt.Meta, "prompt", "prompt")

setNestedTestMeta(availableTools[0].Tool.Meta, "ui", "after-available")
mutateNormalizedTestMeta(availableTools[0].Tool.Meta, "ui", "after-available")
availableTools[0].FeatureRule.features[0] = "changed"
setNestedTestMeta(availableResources[0].Template.Meta, "resource", "after-available")
mutateNormalizedTestMeta(availableResources[0].Template.Meta, "resource", "after-available")
availableResources[0].FeatureRule.features[0] = "changed"
setNestedTestMeta(availablePrompts[0].Prompt.Meta, "prompt", "after-available")
mutateNormalizedTestMeta(availablePrompts[0].Prompt.Meta, "prompt", "after-available")
availablePrompts[0].FeatureRule.features[0] = "changed"

allTools := inv.AllTools()
setNestedTestMeta(allTools[0].Tool.Meta, "ui", "after-all")
mutateNormalizedTestMeta(allTools[0].Tool.Meta, "ui", "after-all")
foundTool, _, err := inv.FindToolByName("tool")
require.NoError(t, err)
setNestedTestMeta(foundTool.Tool.Meta, "ui", "after-find")
mutateNormalizedTestMeta(foundTool.Tool.Meta, "ui", "after-find")

server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil)
inv.RegisterAll(context.Background(), server, nil)
Expand All @@ -1369,40 +1373,110 @@ func TestBuilderOwnsFeatureMetadataInputs(t *testing.T) {
registeredTools, err := clientSession.ListTools(context.Background(), nil)
require.NoError(t, err)
requireNestedTestMeta(t, registeredTools.Tools[0].Meta, "ui", "tool")
require.JSONEq(t, string(expectedToolMeta), string(mustMarshalJSON(t, registeredTools.Tools[0].Meta)))
registeredResources, err := clientSession.ListResourceTemplates(context.Background(), nil)
require.NoError(t, err)
requireNestedTestMeta(t, registeredResources.ResourceTemplates[0].Meta, "resource", "resource")
require.JSONEq(t, string(expectedResourceMeta), string(mustMarshalJSON(t, registeredResources.ResourceTemplates[0].Meta)))
registeredPrompts, err := clientSession.ListPrompts(context.Background(), nil)
require.NoError(t, err)
requireNestedTestMeta(t, registeredPrompts.Prompts[0].Meta, "prompt", "prompt")
require.JSONEq(t, string(expectedPromptMeta), string(mustMarshalJSON(t, registeredPrompts.Prompts[0].Meta)))
}

type testMetaMap map[string]string
type testMetaSlice []string
type testMetaPointer struct {
Value string `json:"value"`
}

func nestedTestMeta(key, value string) mcp.Meta {
return mcp.Meta{
key: map[string]any{
"objects": []any{map[string]any{"value": value}},
"strings": []string{value},
"objects": []any{map[string]any{"value": value}},
"strings": []string{value},
"typed_map": testMetaMap{"value": value},
"typed_slice": testMetaSlice{value},
"bytes": []byte(value),
"pointer": &testMetaPointer{Value: value},
},
}
}

func setNestedTestMeta(meta mcp.Meta, key, value string) {
func mutateSourceTestMeta(meta mcp.Meta, key, value string, marker byte) {
nested := meta[key].(map[string]any)
nested["objects"].([]any)[0].(map[string]any)["value"] = value
nested["strings"].([]string)[0] = value
nested["typed_map"].(testMetaMap)["value"] = value
nested["typed_slice"].(testMetaSlice)[0] = value
nested["bytes"].([]byte)[0] = marker
nested["pointer"].(*testMetaPointer).Value = value
}

func mutateNormalizedTestMeta(meta mcp.Meta, key, value string) {
nested := meta[key].(map[string]any)
nested["objects"].([]any)[0].(map[string]any)["value"] = value
nested["strings"].([]any)[0] = value
nested["typed_map"].(map[string]any)["value"] = value
nested["typed_slice"].([]any)[0] = value
nested["bytes"] = value
nested["pointer"].(map[string]any)["value"] = value
}

func requireNestedTestMeta(t *testing.T, meta mcp.Meta, key, value string) {
t.Helper()
nested := meta[key].(map[string]any)
require.Equal(t, value, nested["objects"].([]any)[0].(map[string]any)["value"])
switch strings := nested["strings"].(type) {
case []string:
require.Equal(t, value, strings[0])
case []any:
require.Equal(t, value, strings[0])
default:
require.Failf(t, "unexpected strings metadata", "type %T", strings)
require.Equal(t, value, nested["strings"].([]any)[0])
require.Equal(t, value, nested["typed_map"].(map[string]any)["value"])
require.Equal(t, value, nested["typed_slice"].([]any)[0])
require.Equal(t, base64.StdEncoding.EncodeToString([]byte(value)), nested["bytes"])
require.Equal(t, value, nested["pointer"].(map[string]any)["value"])
}

func mustMarshalJSON(t *testing.T, value any) []byte {
t.Helper()
data, err := json.Marshal(value)
require.NoError(t, err)
return data
}

func TestBuildRejectsInvalidMetadata(t *testing.T) {
tests := []struct {
name string
builder *Builder
errorText string
}{
{
name: "tool",
builder: NewBuilder().SetTools([]ServerTool{{
Tool: mcp.Tool{Name: "tool", Meta: mcp.Meta{"invalid": make(chan int)}},
}}),
errorText: `tool "tool" metadata`,
},
{
name: "resource",
builder: NewBuilder().SetResources([]ServerResourceTemplate{{
Template: mcp.ResourceTemplate{Name: "resource", Meta: mcp.Meta{"invalid": make(chan int)}},
}}),
errorText: `resource template "resource" metadata`,
},
{
name: "prompt",
builder: NewBuilder().SetPrompts([]ServerPrompt{{
Prompt: mcp.Prompt{Name: "prompt", Meta: mcp.Meta{"invalid": make(chan int)}},
}}),
errorText: `prompt "prompt" metadata`,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, err := tt.builder.Build()
require.ErrorContains(t, err, "invalid inventory metadata")
require.ErrorContains(t, err, tt.errorText)
require.ErrorContains(t, err, "unsupported type: chan int")
})
}
}

Expand Down