Repository navigation
Conversation
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>
jbayardo
changed the title
jbayardo
marked this pull request as ready for review
October 14, 2026 23:07
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Breaking structured-output and exported Go type changes require maintainer confirmation of downstream compatibility.
2 open findings
What changed in this PR
Reduces duplicate payloads in modern MCP tool results while preserving legacy responses.
Changes:
- Keeps file bodies in
content, with only resource metadata in structured output. - Removes HTML escaping from modern structured JSON text.
- Updates tests, schema snapshots, and documentation.
| File | Description |
|---|---|
| pkg/inventory/typed_output.go | Normalizes HTML escapes in modern JSON text. |
| pkg/inventory/typed_output_test.go | Tests HTML encoding and escape handling. |
| pkg/github/typed_repository_outputs_test.go | Verifies metadata-only structured resources. |
| pkg/github/repository_tool_types.go | Removes resource bodies from exported types and schemas. |
| pkg/github/__toolsnaps__/get_file_contents.snap | Records the metadata-only output schema. |
| docs/typed-tool-schemas.md | Documents serialization and resource-body behavior. |
🧠 Review effort: Max
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+400
to
+401
| if !bytes.Contains(data, []byte(`\u00`)) { | ||
| return data |
Comment on lines
+79
to
+80
| `get_file_contents` keeps file bodies in the embedded resource and reports only | ||
| its `uri` and `mimeType` in `structuredContent`, so clients that forward both |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On protocol
2026-07-28,get_file_contentsreturned every file twice: once as the embedded resource incontentand again astext/blobinstructuredContent. Structured output now carries only the resourceuriandmimeType; the body stays incontent. Modern JSON text is also encoded without HTML escaping, so it equalsJSON.stringify(structuredContent).Why
Clients that forward both
contentandstructuredContentto the model pay for each file twice. In Copilot CLI the mirror is ~41.5% ofget_file_contentstokens: ~1.07 GB/day of structured content across 81K of 241K daily calls. Without it, ~9.4K/day of the 19.7K results that spill past 20 KiB would stay inline.Separately,
json.Marshalescapes<,>and&as\u003c,\u003eand\u0026. Copilot CLI only drops the structured JSON when the text is byte-equal toJSON.stringify(structuredContent), so all-text results containing those characters (e.g.pull_request_read,search_code,web_search,search_repositories) were also sent twice.MCP spec context: modelcontextprotocol/modelcontextprotocol#1624, modelcontextprotocol/modelcontextprotocol#2200.
Fixes #
What changed
RepositoryResourceOutputkeeps onlyuriandmimeType.repositoryContentOutputno longer copies the file body, and thetext/bloboneOfis removed from the output schema.applyModernStructuredTextencodes withSetEscapeHTML(false)and then unescapes\u003c/\u003e/\u0026inside JSON strings. The second step is needed because the SDK hands over an already-escapedjson.RawMessage. Escaped backslashes are handled, so a literal\u003cin the data stays as-is.get_file_contentswire test now expects structured output equal to the legacy blocks minus the resource bodies, with notextorblob.get_file_contents.snapregenerated;docs/typed-tool-schemas.mddocuments both rules.Options considered
outputSchemafor discovery. The file body is no longer duplicated, and every client still receives it incontent.outputSchemaforget_file_contentsPreserveHandlerContentstructuredContentis still emitted, so it does not fix the duplication.structuredContentonly for file readsoutputSchema, the spec requiresstructuredContent, and the TS SDK client throws when it is missing.Measurements (real API, stdio binary,
github/github-mcp-server@main, protocol2026-07-28)Bytes of
content+structuredContent:docs/typed-tool-schemas.mdREADME.mdpkg/octicons/icons/repo-light.pngpkg/github(directory)pull_request_read#3437: text equalsJSON.stringify(structuredContent)Legacy protocol (
2025-11-25) results are byte-identical before and after.MCP impact
get_file_contentsoutput schema andstructuredContentno longer includetext/blob. This is a contract change for consumers that read file bodies fromstructuredContent(typed outputs shipped in v2.0.0); they should read the embedded resource incontent. Go library consumers loseRepositoryResourceOutput.Textand.Blob. Modern JSON text no longer HTML-escapes<,>and&; it parses to the same value.Prompts tested (tool changes only)
Exercised over stdio with a JSON-RPC harness (protocol
2026-07-28and2025-11-25), not through an LLM client:pull_request_read; HTML escaping)In every modern-protocol case,
structuredContentvalidated against the advertisedoutputSchemawith Ajv (the TS SDK's validator). I also confirmed in the Copilot CLI source that it still sends the embedded resource body to the model.Security / limits
get_file_contentsresult size on the modern protocol and lets clients dedupe all-text results. No data is newly exposed or filtered.Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint:gofmt -s -lclean;golangci-lint run(v2.14.0, matching CI) on./pkg/inventory/... ./pkg/github/...reported 0 issues../script/test:go test ./... -count=1passed. I ran it without-racebecause cgo isn't available on this Windows machine; CI runs-race.go run ./cmd/github-mcp-server generate-docsproduced no changes.Docs
docs/typed-tool-schemas.mdnow covers non-HTML-escaped modern text and metadata-only structured output for resource results. README needed no changes.