From 6268548d2e4d11c2214dce3e597be46c61fa434e Mon Sep 17 00:00:00 2001 From: Julian Bayardo Spadafora Date: Tue, 13 Oct 2026 17:21:57 -0700 Subject: [PATCH] Stop mirroring file bodies in get_file_contents structured output get_file_contents returned each file twice on protocol 2026-07-28: once in the embedded resource and again as text/blob in structuredContent. Clients that forward both to the model paid for every file twice. Structured output now carries only the resource uri and mimeType; the body stays in content. Modern JSON text for all-text results is also encoded without HTML escaping, so it matches JSON.stringify(structuredContent) and clients can dedupe it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/typed-tool-schemas.md | 9 ++++ .../__toolsnaps__/get_file_contents.snap | 52 +----------------- pkg/github/repository_tool_types.go | 28 ++++------ pkg/github/typed_repository_outputs_test.go | 24 +++++++-- pkg/inventory/typed_output.go | 54 ++++++++++++++++++- pkg/inventory/typed_output_test.go | 46 ++++++++++++++++ 6 files changed, 139 insertions(+), 74 deletions(-) diff --git a/docs/typed-tool-schemas.md b/docs/typed-tool-schemas.md index 2065872be0..c9da66b3a7 100644 --- a/docs/typed-tool-schemas.md +++ b/docs/typed-tool-schemas.md @@ -70,3 +70,12 @@ request-dependent output such as CSV. This is for intentional non-DTO responses such as resources, errors, or CSV; ordinary declared structured success must retain shared DTO serialization, with modern JSON text equal to `structuredContent`, even when legacy handler text was a plain mutation message. +That text is encoded without HTML escaping (`<`, `>` and `&` stay literal), so it +matches a standard re-serialization such as `JSON.stringify(structuredContent)` +and clients can recognize it as a duplicate. + +Content blocks are kept as-is whenever any block is not text. Structured output +for such results should describe the blocks rather than repeat their payloads: +`get_file_contents` keeps file bodies in the embedded resource and reports only +its `uri` and `mimeType` in `structuredContent`, so clients that forward both +fields do not send each file to the model twice. diff --git a/pkg/github/__toolsnaps__/get_file_contents.snap b/pkg/github/__toolsnaps__/get_file_contents.snap index fa845c7d50..7e00161888 100644 --- a/pkg/github/__toolsnaps__/get_file_contents.snap +++ b/pkg/github/__toolsnaps__/get_file_contents.snap @@ -181,61 +181,11 @@ "properties": { "resource": { "additionalProperties": false, - "oneOf": [ - { - "not": { - "required": [ - "blob" - ] - }, - "required": [ - "text" - ] - }, - { - "not": { - "required": [ - "text" - ] - }, - "required": [ - "blob" - ] - }, - { - "not": { - "anyOf": [ - { - "required": [ - "text" - ] - }, - { - "required": [ - "blob" - ] - } - ] - } - } - ], + "description": "Metadata for the embedded resource. The file body is returned only in the result content, not repeated here.", "properties": { - "blob": { - "description": "Base64-encoded file bytes.", - "type": [ - "null", - "string" - ] - }, "mimeType": { "type": "string" }, - "text": { - "type": [ - "null", - "string" - ] - }, "uri": { "type": "string" } diff --git a/pkg/github/repository_tool_types.go b/pkg/github/repository_tool_types.go index 4f3b584983..a91e9909fe 100644 --- a/pkg/github/repository_tool_types.go +++ b/pkg/github/repository_tool_types.go @@ -1,7 +1,6 @@ package github import ( - "encoding/base64" "encoding/json" "fmt" "maps" @@ -212,8 +211,9 @@ func (out ForkRepositoryOutput) MarshalJSON() ([]byte, error) { } // Repository contents can be a projected directory listing or a heterogeneous -// MCP content response. The latter is represented losslessly as typed content -// blocks, including the human-readable status and embedded text/blob or link. +// MCP content response. The latter is represented as typed content blocks, +// including the human-readable status and resource or link metadata. File +// bodies stay in the result content and are not repeated in structured output. type RepositoryContentsOutput struct { Directory []*RepositoryDirectoryEntryOutput Content *RepositoryContentOutput @@ -324,14 +324,15 @@ type RepositoryTextBlock struct { type RepositoryResourceBlock struct { Type string `json:"type"` - Resource RepositoryResourceOutput `json:"resource"` + Resource RepositoryResourceOutput `json:"resource" jsonschema:"Metadata for the embedded resource. The file body is returned only in the result content, not repeated here."` } +// RepositoryResourceOutput describes an embedded file resource without its +// body, so clients that forward both content and structured output to a model +// do not receive every file twice. type RepositoryResourceOutput struct { - URI string `json:"uri"` - MIMEType string `json:"mimeType,omitempty"` - Text *string `json:"text,omitempty"` - Blob *string `json:"blob,omitempty" jsonschema:"Base64-encoded file bytes."` + URI string `json:"uri"` + MIMEType string `json:"mimeType,omitempty"` } type RepositoryLinkBlock struct { @@ -353,12 +354,6 @@ func repositoryContentOutput(result *mcp.CallToolResult) (*RepositoryContentsOut out.Content = append(out.Content, RepositoryContentBlock{Text: &RepositoryTextBlock{Type: "text", Text: block.Text}}) case *mcp.EmbeddedResource: resource := RepositoryResourceOutput{URI: block.Resource.URI, MIMEType: block.Resource.MIMEType} - if block.Resource.Blob != nil { - blob := base64.StdEncoding.EncodeToString(block.Resource.Blob) - resource.Blob = &blob - } else if block.Resource.Text != "" { - resource.Text = &block.Resource.Text - } out.Content = append(out.Content, RepositoryContentBlock{Resource: &RepositoryResourceBlock{Type: "resource", Resource: resource}}) case *mcp.ResourceLink: out.Content = append(out.Content, RepositoryContentBlock{Link: &RepositoryLinkBlock{ @@ -521,11 +516,6 @@ func repositoryContentsOutputSchema() *jsonschema.Schema { text.Properties["type"].Const = new(any("text")) resource := repositoryOutputSchema[RepositoryResourceBlock]() resource.Properties["type"].Const = new(any("resource")) - resource.Properties["resource"].OneOf = []*jsonschema.Schema{ - {Required: []string{"text"}, Not: &jsonschema.Schema{Required: []string{"blob"}}}, - {Required: []string{"blob"}, Not: &jsonschema.Schema{Required: []string{"text"}}}, - {Not: &jsonschema.Schema{AnyOf: []*jsonschema.Schema{{Required: []string{"text"}}, {Required: []string{"blob"}}}}}, - } link := repositoryOutputSchema[RepositoryLinkBlock]() link.Properties["type"].Const = new(any("resource_link")) content := &jsonschema.Schema{ diff --git a/pkg/github/typed_repository_outputs_test.go b/pkg/github/typed_repository_outputs_test.go index 672837c711..5d9012503a 100644 --- a/pkg/github/typed_repository_outputs_test.go +++ b/pkg/github/typed_repository_outputs_test.go @@ -373,7 +373,11 @@ func TestTypedRepositoryOutputs(t *testing.T) { first := result.Content[0].(*mcp.TextContent).Text switch { case call.name == "get_file_contents" && !strings.HasPrefix(first, "["): - assert.JSONEq(t, `{"content":`+mustMarshalJSON(t, legacy.Content)+`}`, mustMarshalJSON(t, result.StructuredContent)) + structured := mustMarshalJSON(t, result.StructuredContent) + assert.JSONEq(t, `{"content":`+legacyContentWithoutResourceBodies(t, legacy.Content)+`}`, structured, + "structured output repeats content metadata but not file bodies") + assert.NotContains(t, structured, `"text":"hello\n"`) + assert.NotContains(t, structured, `"blob"`) case call.name == "get_file_contents": var entries []*RepositoryDirectoryEntryOutput require.NoError(t, json.Unmarshal([]byte(first), &entries)) @@ -439,6 +443,19 @@ func hasOnlyTextContent(content []mcp.Content) bool { return true } +func legacyContentWithoutResourceBodies(t *testing.T, content []mcp.Content) string { + t.Helper() + var blocks []map[string]any + require.NoError(t, json.Unmarshal([]byte(mustMarshalJSON(t, content)), &blocks)) + for _, block := range blocks { + if resource, ok := block["resource"].(map[string]any); ok { + delete(resource, "text") + delete(resource, "blob") + } + } + return mustMarshalJSON(t, blocks) +} + func assertLegacyRepositoryText(t *testing.T, name string, args map[string]any, result *mcp.CallToolResult) { t.Helper() switch name { @@ -546,9 +563,10 @@ func TestRepositoryOutputUnions(t *testing.T) { "contents", repositoryContentsOutputSchema(), []string{`null`, `[]`, `[{"name":"file"}]`, `{"content":[]}`, `{"content":[{"type":"text","text":"status"}]}`, `{"content":[{"type":"resource","resource":{"uri":"repo://file","mimeType":"text/plain"}}]}`, - `{"content":[{"type":"resource","resource":{"uri":"repo://file","blob":"AAEC"}}]}`, `{"content":[{"type":"resource_link","uri":"repo://file","name":"file","size":1048576}]}`}, - []string{`{}`, `{"content":[{"type":"text"}]}`, `{"content":[{"type":"resource","resource":{"uri":"repo://file","text":"x","blob":"AA=="}}]}`, + []string{`{}`, `{"content":[{"type":"text"}]}`, + `{"content":[{"type":"resource","resource":{"uri":"repo://file","text":"x"}}]}`, + `{"content":[{"type":"resource","resource":{"uri":"repo://file","blob":"AAEC"}}]}`, `{"content":[{"type":"opaque","data":{}}]}`}, }, } { diff --git a/pkg/inventory/typed_output.go b/pkg/inventory/typed_output.go index f6ec5d1ea2..e3aa2b40b9 100644 --- a/pkg/inventory/typed_output.go +++ b/pkg/inventory/typed_output.go @@ -361,7 +361,7 @@ func applyModernStructuredText(result *mcp.CallToolResult) error { if result == nil || result.IsError || result.InputRequests != nil || result.StructuredContent == nil { return nil } - encoded, err := json.Marshal(result.StructuredContent) + encoded, err := marshalStructuredText(result.StructuredContent) if err != nil { return fmt.Errorf("marshal structured tool output for text content: %w", err) } @@ -376,6 +376,58 @@ func applyModernStructuredText(result *mcp.CallToolResult) error { return nil } +// marshalStructuredText encodes structured output for its JSON text block +// without HTML escaping, so the text matches a standard re-serialization of +// structuredContent (such as JSON.stringify) and clients can detect it as a +// duplicate. +func marshalStructuredText(value any) ([]byte, error) { + var buffer bytes.Buffer + encoder := json.NewEncoder(&buffer) + encoder.SetEscapeHTML(false) + if err := encoder.Encode(value); err != nil { + return nil, err + } + // Pre-encoded json.RawMessage values, including SDK typed output, keep + // their HTML escapes even when the encoder does not add new ones. + return unescapeJSONHTML(bytes.TrimSuffix(buffer.Bytes(), []byte("\n"))), nil +} + +// unescapeJSONHTML replaces the \u003c, \u003e and \u0026 escapes that +// encoding/json emits inside strings with the literal characters. In valid JSON +// a backslash only starts a string escape, so consuming escapes pairwise +// cannot misread an escaped backslash followed by "u003c". +func unescapeJSONHTML(data []byte) []byte { + if !bytes.Contains(data, []byte(`\u00`)) { + return data + } + out := make([]byte, 0, len(data)) + for i := 0; i < len(data); i++ { + if data[i] != '\\' || i+1 == len(data) { + out = append(out, data[i]) + continue + } + if data[i+1] == 'u' && i+6 <= len(data) { + switch string(data[i+2 : i+6]) { + case "003c", "003C": + out = append(out, '<') + i += 5 + continue + case "003e", "003E": + out = append(out, '>') + i += 5 + continue + case "0026": + out = append(out, '&') + i += 5 + continue + } + } + out = append(out, data[i], data[i+1]) + i++ + } + return out +} + func removeSDKOutputFallback(result *mcp.CallToolResult, state *typedCallState) error { if result.StructuredContent == nil || len(result.Content) <= state.contentLengthBeforeSDK { return nil diff --git a/pkg/inventory/typed_output_test.go b/pkg/inventory/typed_output_test.go index 99b6b30ed3..23996bba7b 100644 --- a/pkg/inventory/typed_output_test.go +++ b/pkg/inventory/typed_output_test.go @@ -1686,6 +1686,52 @@ func TestTypedScalarOutputProtocolGatePreservesHandlerText(t *testing.T) { assert.Equal(t, "custom legacy text", result.Content[0].(*mcp.TextContent).Text) } +func TestTypedModernTextDoesNotEscapeHTML(t *testing.T) { + type output struct { + HTML string `json:"html"` + Literal string `json:"literal"` + } + tool := NewServerToolWithContextHandler( + mcp.Tool{Name: "typed_html_tool"}, + testToolsetMetadata("test"), + func(context.Context, *mcp.CallToolRequest, struct{}) (*mcp.CallToolResult, output, error) { + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: "legacy text"}}, + }, output{HTML: "a & b", Literal: `\u003c`}, nil + }, + ) + server := mcp.NewServer(&mcp.Implementation{Name: "test-server", Version: "v0.0.1"}, nil) + tool.RegisterFunc(server, nil) + modern := connectTypedTestClient(t, server, "") + + result, err := modern.CallTool(context.Background(), &mcp.CallToolParams{Name: "typed_html_tool"}) + require.NoError(t, err) + require.Len(t, result.Content, 1) + text := result.Content[0].(*mcp.TextContent).Text + assert.Equal(t, `{"html":"a & b","literal":"\\u003c"}`, text, + "modern text matches a standard re-serialization of structuredContent") + + var reserialized bytes.Buffer + encoder := json.NewEncoder(&reserialized) + encoder.SetEscapeHTML(false) + require.NoError(t, encoder.Encode(result.StructuredContent)) + assert.Equal(t, strings.TrimSuffix(reserialized.String(), "\n"), text) +} + +func TestUnescapeJSONHTML(t *testing.T) { + for _, test := range []struct{ input, want string }{ + {`{"a":"plain"}`, `{"a":"plain"}`}, + {`"\u003cb\u003e \u0026 \u003C\u003E"`, `" & <>"`}, + {`"\\u003c"`, `"\\u003c"`}, + {`"\\\u003c"`, `"\\<"`}, + {`"\u00e9\u2028\"\u0026"`, `"\u00e9\u2028\"&"`}, + {`"\u003`, `"\u003`}, + {`"\`, `"\`}, + } { + assert.Equal(t, test.want, string(unescapeJSONHTML([]byte(test.input))), test.input) + } +} + func TestTypedLegacyOutputGateRemovesFallbackWithEmptyHandlerContent(t *testing.T) { tool := NewServerToolWithContextHandler( mcp.Tool{Name: "typed_empty_content_tool"},