Skip to content

Commit 2490828

Browse files
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
1 parent 8566d48 commit 2490828

3 files changed

Lines changed: 163 additions & 30 deletions

File tree

pkg/inventory/builder.go

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ const mcpAppsFeatureFlag FeatureFlag = "remote_mcp_ui_apps"
2424
type ToolFilter func(ctx context.Context, tool *ServerTool) (bool, error)
2525

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

5053
// Configuration options (processed at Build time)
@@ -68,30 +71,66 @@ func NewBuilder() *Builder {
6871
// SetTools sets the tools for the inventory. Returns self for chaining.
6972
func (b *Builder) SetTools(tools []ServerTool) *Builder {
7073
b.tools = slices.Clone(tools)
74+
b.toolsErr = nil
7175
for i := range b.tools {
72-
b.tools[i] = cloneServerTool(b.tools[i])
76+
tool, err := ownServerTool(b.tools[i])
77+
if err != nil {
78+
b.toolsErr = errors.Join(b.toolsErr, fmt.Errorf("tool %q metadata: %w", b.tools[i].Tool.Name, err))
79+
}
80+
b.tools[i] = tool
7381
}
7482
return b
7583
}
7684

7785
// SetResources sets the resource templates for the inventory. Returns self for chaining.
7886
func (b *Builder) SetResources(resources []ServerResourceTemplate) *Builder {
7987
b.resourceTemplates = slices.Clone(resources)
88+
b.resourcesErr = nil
8089
for i := range b.resourceTemplates {
81-
b.resourceTemplates[i] = cloneResourceTemplate(b.resourceTemplates[i])
90+
resource, err := ownResourceTemplate(b.resourceTemplates[i])
91+
if err != nil {
92+
b.resourcesErr = errors.Join(b.resourcesErr, fmt.Errorf("resource template %q metadata: %w", b.resourceTemplates[i].Template.Name, err))
93+
}
94+
b.resourceTemplates[i] = resource
8295
}
8396
return b
8497
}
8598

8699
// SetPrompts sets the prompts for the inventory. Returns self for chaining.
87100
func (b *Builder) SetPrompts(prompts []ServerPrompt) *Builder {
88101
b.prompts = slices.Clone(prompts)
102+
b.promptsErr = nil
89103
for i := range b.prompts {
90-
b.prompts[i] = clonePrompt(b.prompts[i])
104+
prompt, err := ownPrompt(b.prompts[i])
105+
if err != nil {
106+
b.promptsErr = errors.Join(b.promptsErr, fmt.Errorf("prompt %q metadata: %w", b.prompts[i].Prompt.Name, err))
107+
}
108+
b.prompts[i] = prompt
91109
}
92110
return b
93111
}
94112

113+
func ownServerTool(tool ServerTool) (ServerTool, error) {
114+
meta, err := normalizeMeta(tool.Tool.Meta)
115+
tool.Tool.Meta = meta
116+
tool.FeatureRule = tool.FeatureRule.clone()
117+
return tool, err
118+
}
119+
120+
func ownResourceTemplate(resource ServerResourceTemplate) (ServerResourceTemplate, error) {
121+
meta, err := normalizeMeta(resource.Template.Meta)
122+
resource.Template.Meta = meta
123+
resource.FeatureRule = resource.FeatureRule.clone()
124+
return resource, err
125+
}
126+
127+
func ownPrompt(prompt ServerPrompt) (ServerPrompt, error) {
128+
meta, err := normalizeMeta(prompt.Prompt.Meta)
129+
prompt.Prompt.Meta = meta
130+
prompt.FeatureRule = prompt.FeatureRule.clone()
131+
return prompt, err
132+
}
133+
95134
func cloneServerTool(tool ServerTool) ServerTool {
96135
tool.Tool.Meta = cloneMeta(tool.Tool.Meta)
97136
tool.FeatureRule = tool.FeatureRule.clone()
@@ -234,6 +273,10 @@ func cleanTools(tools []string) []string {
234273
// (i.e., they don't exist in the tool set and are not deprecated aliases).
235274
// This ensures invalid tool configurations fail fast at build time.
236275
func (b *Builder) Build() (*Inventory, error) {
276+
if err := errors.Join(b.toolsErr, b.resourcesErr, b.promptsErr); err != nil {
277+
return nil, fmt.Errorf("invalid inventory metadata: %w", err)
278+
}
279+
237280
tools := b.tools
238281

239282
filters := b.filters

pkg/inventory/metadata.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,27 @@
11
package inventory
22

33
import (
4-
"slices"
4+
"encoding/json"
5+
"fmt"
56

67
"github.com/modelcontextprotocol/go-sdk/mcp"
78
)
89

10+
func normalizeMeta(meta mcp.Meta) (mcp.Meta, error) {
11+
if meta == nil {
12+
return nil, nil
13+
}
14+
data, err := json.Marshal(meta)
15+
if err != nil {
16+
return nil, fmt.Errorf("marshal metadata: %w", err)
17+
}
18+
var normalized mcp.Meta
19+
if err := json.Unmarshal(data, &normalized); err != nil {
20+
return nil, fmt.Errorf("unmarshal metadata: %w", err)
21+
}
22+
return normalized, nil
23+
}
24+
925
func cloneMeta(meta mcp.Meta) mcp.Meta {
1026
if meta == nil {
1127
return nil
@@ -33,9 +49,9 @@ func cloneMetaValue(value any) any {
3349
clone[i] = cloneMetaValue(item)
3450
}
3551
return clone
36-
case []string:
37-
return slices.Clone(value)
38-
default:
52+
case nil, bool, float64, string:
3953
return value
54+
default:
55+
panic(fmt.Sprintf("metadata value %T is not normalized", value))
4056
}
4157
}

pkg/inventory/registry_test.go

Lines changed: 95 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package inventory
22

33
import (
44
"context"
5+
"encoding/base64"
56
"encoding/json"
67
"fmt"
78
"testing"
@@ -1309,6 +1310,9 @@ func TestBuilderOwnsFeatureMetadataInputs(t *testing.T) {
13091310
prompts := []ServerPrompt{mockPrompt("prompt", "toolset1")}
13101311
prompts[0].FeatureRule = promptRule
13111312
prompts[0].Prompt.Meta = nestedTestMeta("prompt", "prompt")
1313+
expectedToolMeta := mustMarshalJSON(t, tools[0].Tool.Meta)
1314+
expectedResourceMeta := mustMarshalJSON(t, resources[0].Template.Meta)
1315+
expectedPromptMeta := mustMarshalJSON(t, prompts[0].Prompt.Meta)
13121316

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

1323-
setNestedTestMeta(tools[0].Tool.Meta, "ui", "after-set")
1324-
setNestedTestMeta(resources[0].Template.Meta, "resource", "after-set")
1325-
setNestedTestMeta(prompts[0].Prompt.Meta, "prompt", "after-set")
1327+
mutateSourceTestMeta(tools[0].Tool.Meta, "ui", "after-set", 's')
1328+
mutateSourceTestMeta(resources[0].Template.Meta, "resource", "after-set", 's')
1329+
mutateSourceTestMeta(prompts[0].Prompt.Meta, "prompt", "after-set", 's')
13261330
inv := mustBuild(t, builder)
13271331
tools[0].FeatureRule.features[0] = "changed"
1328-
setNestedTestMeta(tools[0].Tool.Meta, "ui", "after-build")
1332+
mutateSourceTestMeta(tools[0].Tool.Meta, "ui", "after-build", 'b')
13291333
resources[0].FeatureRule.features[0] = "changed"
1330-
setNestedTestMeta(resources[0].Template.Meta, "resource", "after-build")
1334+
mutateSourceTestMeta(resources[0].Template.Meta, "resource", "after-build", 'b')
13311335
prompts[0].FeatureRule.features[0] = "changed"
1332-
setNestedTestMeta(prompts[0].Prompt.Meta, "prompt", "after-build")
1336+
mutateSourceTestMeta(prompts[0].Prompt.Meta, "prompt", "after-build", 'b')
13331337

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

1345-
setNestedTestMeta(availableTools[0].Tool.Meta, "ui", "after-available")
1349+
mutateNormalizedTestMeta(availableTools[0].Tool.Meta, "ui", "after-available")
13461350
availableTools[0].FeatureRule.features[0] = "changed"
1347-
setNestedTestMeta(availableResources[0].Template.Meta, "resource", "after-available")
1351+
mutateNormalizedTestMeta(availableResources[0].Template.Meta, "resource", "after-available")
13481352
availableResources[0].FeatureRule.features[0] = "changed"
1349-
setNestedTestMeta(availablePrompts[0].Prompt.Meta, "prompt", "after-available")
1353+
mutateNormalizedTestMeta(availablePrompts[0].Prompt.Meta, "prompt", "after-available")
13501354
availablePrompts[0].FeatureRule.features[0] = "changed"
13511355

13521356
allTools := inv.AllTools()
1353-
setNestedTestMeta(allTools[0].Tool.Meta, "ui", "after-all")
1357+
mutateNormalizedTestMeta(allTools[0].Tool.Meta, "ui", "after-all")
13541358
foundTool, _, err := inv.FindToolByName("tool")
13551359
require.NoError(t, err)
1356-
setNestedTestMeta(foundTool.Tool.Meta, "ui", "after-find")
1360+
mutateNormalizedTestMeta(foundTool.Tool.Meta, "ui", "after-find")
13571361

13581362
server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil)
13591363
inv.RegisterAll(context.Background(), server, nil)
@@ -1369,40 +1373,110 @@ func TestBuilderOwnsFeatureMetadataInputs(t *testing.T) {
13691373
registeredTools, err := clientSession.ListTools(context.Background(), nil)
13701374
require.NoError(t, err)
13711375
requireNestedTestMeta(t, registeredTools.Tools[0].Meta, "ui", "tool")
1376+
require.JSONEq(t, string(expectedToolMeta), string(mustMarshalJSON(t, registeredTools.Tools[0].Meta)))
13721377
registeredResources, err := clientSession.ListResourceTemplates(context.Background(), nil)
13731378
require.NoError(t, err)
13741379
requireNestedTestMeta(t, registeredResources.ResourceTemplates[0].Meta, "resource", "resource")
1380+
require.JSONEq(t, string(expectedResourceMeta), string(mustMarshalJSON(t, registeredResources.ResourceTemplates[0].Meta)))
13751381
registeredPrompts, err := clientSession.ListPrompts(context.Background(), nil)
13761382
require.NoError(t, err)
13771383
requireNestedTestMeta(t, registeredPrompts.Prompts[0].Meta, "prompt", "prompt")
1384+
require.JSONEq(t, string(expectedPromptMeta), string(mustMarshalJSON(t, registeredPrompts.Prompts[0].Meta)))
1385+
}
1386+
1387+
type testMetaMap map[string]string
1388+
type testMetaSlice []string
1389+
type testMetaPointer struct {
1390+
Value string `json:"value"`
13781391
}
13791392

13801393
func nestedTestMeta(key, value string) mcp.Meta {
13811394
return mcp.Meta{
13821395
key: map[string]any{
1383-
"objects": []any{map[string]any{"value": value}},
1384-
"strings": []string{value},
1396+
"objects": []any{map[string]any{"value": value}},
1397+
"strings": []string{value},
1398+
"typed_map": testMetaMap{"value": value},
1399+
"typed_slice": testMetaSlice{value},
1400+
"bytes": []byte(value),
1401+
"pointer": &testMetaPointer{Value: value},
13851402
},
13861403
}
13871404
}
13881405

1389-
func setNestedTestMeta(meta mcp.Meta, key, value string) {
1406+
func mutateSourceTestMeta(meta mcp.Meta, key, value string, marker byte) {
13901407
nested := meta[key].(map[string]any)
13911408
nested["objects"].([]any)[0].(map[string]any)["value"] = value
13921409
nested["strings"].([]string)[0] = value
1410+
nested["typed_map"].(testMetaMap)["value"] = value
1411+
nested["typed_slice"].(testMetaSlice)[0] = value
1412+
nested["bytes"].([]byte)[0] = marker
1413+
nested["pointer"].(*testMetaPointer).Value = value
1414+
}
1415+
1416+
func mutateNormalizedTestMeta(meta mcp.Meta, key, value string) {
1417+
nested := meta[key].(map[string]any)
1418+
nested["objects"].([]any)[0].(map[string]any)["value"] = value
1419+
nested["strings"].([]any)[0] = value
1420+
nested["typed_map"].(map[string]any)["value"] = value
1421+
nested["typed_slice"].([]any)[0] = value
1422+
nested["bytes"] = value
1423+
nested["pointer"].(map[string]any)["value"] = value
13931424
}
13941425

13951426
func requireNestedTestMeta(t *testing.T, meta mcp.Meta, key, value string) {
13961427
t.Helper()
13971428
nested := meta[key].(map[string]any)
13981429
require.Equal(t, value, nested["objects"].([]any)[0].(map[string]any)["value"])
1399-
switch strings := nested["strings"].(type) {
1400-
case []string:
1401-
require.Equal(t, value, strings[0])
1402-
case []any:
1403-
require.Equal(t, value, strings[0])
1404-
default:
1405-
require.Failf(t, "unexpected strings metadata", "type %T", strings)
1430+
require.Equal(t, value, nested["strings"].([]any)[0])
1431+
require.Equal(t, value, nested["typed_map"].(map[string]any)["value"])
1432+
require.Equal(t, value, nested["typed_slice"].([]any)[0])
1433+
require.Equal(t, base64.StdEncoding.EncodeToString([]byte(value)), nested["bytes"])
1434+
require.Equal(t, value, nested["pointer"].(map[string]any)["value"])
1435+
}
1436+
1437+
func mustMarshalJSON(t *testing.T, value any) []byte {
1438+
t.Helper()
1439+
data, err := json.Marshal(value)
1440+
require.NoError(t, err)
1441+
return data
1442+
}
1443+
1444+
func TestBuildRejectsInvalidMetadata(t *testing.T) {
1445+
tests := []struct {
1446+
name string
1447+
builder *Builder
1448+
errorText string
1449+
}{
1450+
{
1451+
name: "tool",
1452+
builder: NewBuilder().SetTools([]ServerTool{{
1453+
Tool: mcp.Tool{Name: "tool", Meta: mcp.Meta{"invalid": make(chan int)}},
1454+
}}),
1455+
errorText: `tool "tool" metadata`,
1456+
},
1457+
{
1458+
name: "resource",
1459+
builder: NewBuilder().SetResources([]ServerResourceTemplate{{
1460+
Template: mcp.ResourceTemplate{Name: "resource", Meta: mcp.Meta{"invalid": make(chan int)}},
1461+
}}),
1462+
errorText: `resource template "resource" metadata`,
1463+
},
1464+
{
1465+
name: "prompt",
1466+
builder: NewBuilder().SetPrompts([]ServerPrompt{{
1467+
Prompt: mcp.Prompt{Name: "prompt", Meta: mcp.Meta{"invalid": make(chan int)}},
1468+
}}),
1469+
errorText: `prompt "prompt" metadata`,
1470+
},
1471+
}
1472+
1473+
for _, tt := range tests {
1474+
t.Run(tt.name, func(t *testing.T) {
1475+
_, err := tt.builder.Build()
1476+
require.ErrorContains(t, err, "invalid inventory metadata")
1477+
require.ErrorContains(t, err, tt.errorText)
1478+
require.ErrorContains(t, err, "unsupported type: chan int")
1479+
})
14061480
}
14071481
}
14081482

0 commit comments

Comments
 (0)