Feat/output schemas 2026 07 28#2945
Draft
olaservo wants to merge 6 commits into
Draft
Conversation
Adds feature-gated MCP `outputSchema` and `structuredContent`, targeting the tools whose response shape varies by their `method` argument — the ones both existing upstream attempts (github#2382, github#2468) explicitly set aside as "multi-method tool returning different shapes". Protocol revision 2026-07-28 (SEP-2106) is what makes these expressible. 2025-11-25 typed `outputSchema` as a closed object shape restricted to `type: "object"` at the root, and `structuredContent` as a JSON object; the draft widens both to any valid JSON Schema 2020-12 and any JSON value. Two orthogonal gates: - `output_schemas` feature flag controls rollout. - The negotiated protocol version, read per request, controls which schema shapes are legal. Object-root schemas ship to every client; non-object roots and non-object structuredContent are withheld below 2026-07-28. Unions use `anyOf`, never `oneOf`. `oneOf` requires exactly one branch to match and provably fails here: actions_run_trigger has four structurally identical branches, an empty array satisfies every array branch, and an issue_read `get` on a sub-issue also satisfies the `get_parent` branch. A test asserts no schema ever contains "oneOf". Handlers are unchanged. Rather than thread typed values out of ~20 call sites, structuredContent is mirrored from the serialized-JSON text block each handler already emits — the relationship the spec itself describes. This is byte-exact, so it cannot drift from the text content and preserves number formatting a re-marshal would lose. It applies only to tools that declare a schema, leaving the other ~120 tools byte-identical on the wire. Because the schema lives on ServerTool and is copied onto a duplicate mcp.Tool at registration, Tool.OutputSchema stays nil in the default surface and none of the 123 committed toolsnaps change. Tools covered: actions_get, actions_list, actions_run_trigger, discussion_comment_write, issue_dependency_read (single shape, not a union), issue_read, pull_request_read. Also fixes three adjacent bugs found along the way: - GetSubIssues could emit the literal `null`: go-github declares `var subIssues []*SubIssue` and leaves it nil on an empty body, which marshals to null rather than []. Every sibling method normalises; this one did not. - sub_issue_write's `method` property had no `enum`, so its values existed only in prose while dispatch accepted exactly add/remove/reprioritize. - convertJSONTextResultToCSV cleared StructuredContent, which would have made csv_output silently disable output_schemas for every list_* tool. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tent Adds `structured_content_only`, which turns the main cost of output schemas into a saving. With `output_schemas` alone, a schema-bearing tool sends its payload twice: once as the serialized-JSON text block and once as structuredContent. That duplication works against csv_output, minimal_output and the fields param, all of which exist to make responses smaller. The spec's "a tool that returns structured content SHOULD also return the serialized JSON in a TextContent block" is a backwards-compatibility clause. The Go SDK is explicit about this where it synthesises that block on its typed path: the fallback exists "so that pre-SEP-2106 clients can recover the structured payload from unstructured content". A client that negotiated 2026-07-28 is not such a client, so for those the block is pure duplication. Measured on a small payload (TestStructuredContentOnlyRoughlyHalvesTheResult): no output schema 272 bytes output_schemas 405 bytes (+49%) output_schemas,structured_content_only 254 bytes (-7%) The result is smaller than having no output schema at all, because a text block embeds JSON as an escaped string — every quote becomes \" — while structuredContent carries it raw. The saving grows with payload size. `content` stays present as an empty array: the draft schema still lists it in CallToolResult's required set, and the SDK normalises an empty slice to `[]` rather than `null`. The text is kept whenever it is not genuinely redundant: older or unknown protocol version, error results, non-JSON or multi-part results (raw diffs, logs, CSV-converted output), and results where the handler set structuredContent itself — in that last case the server did not author the text/structured pairing and cannot assume the two match. Deliberately a separate opt-in, conjoined with output_schemas rather than independent. Negotiating 2026-07-28 does not prove a client reads structuredContent — no capability advertises it — and a client that ignored it would see an empty result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four of the seven schemas accepted arbitrary objects, so the unions
documented and validated nothing:
actions_get accepted {} and {"junk":1}
actions_list accepted {} and {"junk":1}
issue_read accepted [{}] and [{"junk":1}]
pull_request_read accepted {}
The cause is that these branches describe go-github structs whose every
field is omitempty, leaving no required key anywhere.
JSON Schema 2020-12 has no dedicated "at least one of these keys" keyword;
the idiom is anyOf over singleton required:
{"anyOf":[{"required":["id"]},{"required":["number"]}, ...]}
which reads as "must carry at least one recognised field". It is the right
tool here precisely because it says nothing about UNKNOWN properties: a
go-github bump that adds a struct field cannot make the server emit a key
its own schema forbids.
That is also why additionalProperties:false is now removed from
pull_request_read's get_status branch and its repoStatus $def. It achieved
non-vacuity but coupled the schema to go-github's exact field set.
get_status needed one extra step, because an all-nil *github.CombinedStatus
(every field pointer+omitempty) really does marshal to {}. Rather than leave
{} globally acceptable, that branch is anyOf over a documented
{"maxProperties":0} arm plus the singleton required arms, so {} stays valid
only through the branch that can actually produce it while {"junk":1} is
rejected.
For array branches the constraint goes inside "items", so an empty array
still validates — get_comments on an issue with no comments returns [], and
that must keep working.
Adds three permanent guards:
- TestPolymorphicOutputSchemasAreNotVacuous, which whitelists the single
legitimate {} by tool and requires a justification rather than allowing
it silently
- TestPolymorphicOutputSchemasDoNotForbidUnknownFields, which keeps
additionalProperties:false from creeping back
- the existing NeverUseOneOf guard, unchanged
Verified that nothing was over-tightened: real handler output still conforms
(including sparse issues with every optional absent, and empty collections),
unknown fields alongside recognised ones still validate, and [] still
validates for every array branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment-to-code ratio across the four new files was 0.89, with the worst offender being 24 comment lines above a one-line function body. Cuts the "what" and keeps the non-obvious "why": that oneOf is wrong for these unions, that the version gate must copy on write because the SDK shares tool pointers, that dropping text is opt-in because no capability advertises structuredContent support. Much of what went was duplicating docs/feature-flags.md, which did not exist when these were written. Ratio is now 0.48, against 0.62 for pkg/inventory/registry.go and 1.23 for pkg/ifc/ifc.go. Blocks of 7+ lines drop from 12 to 4. No code changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Extends TestIssueReadOutputValidatesAgainstDeclaredSchema to the remaining six tools, taking method coverage from 3 of 28 to 28 of 28. Each tool gets its own file; each subtest runs the real handler against the existing mock harness, then validates the emitted payload against the schema the tool advertises. Validating the text block is equivalent to validating what a client receives, because the structured-content mirror publishes those exact bytes as structuredContent. Beyond the happy path, the tests cover the cases that actually exercise a schema's required sets: empty collections, sparse objects with every optional field absent, and — for actions_get — a different single surviving key per method, so a non-first branch of each anyOf is exercised rather than always the first. Fixes a latent bug the tests surfaced. go-github decodes GetWorkflowRunByID, GetWorkflowJobByID and GetWorkflowRunUsageByID into a *T, so a 200 carrying a null body leaves the pointer nil and json.Marshal emits the literal `null`, which cannot validate against an object-rooted schema. Same class as the sub-issues bug already fixed here; these three now report the anomaly instead of returning a payload no caller can use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The synthetic 272-byte fixture undersells the effect, because the saving comes from JSON-string escaping overhead, which scales with payload size. Measured against the live GitHub API on github/github-mcp-server, comparing the full serialized tools/call result: pull_request_read get_files 99,990 -> 48,999 bytes (-51%) actions_list list_workflows 30,407 -> 15,839 bytes (-48%) actions_get get_workflow_run 30,229 -> 15,801 bytes (-48%) pull_request_read get_check_runs 10,551 -> 6,240 bytes (-41%) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Adds feature-gated MCP
outputSchemaandstructuredContentfor the tools whose response shape varies by theirmethodargument, and adds a second flag that makes those responses smaller than they are today rather than larger.Why
Output schemas are what let a client generate typed bindings for a tool. Two attempts are already open and both deliberately skip the method-dispatched tools. #2382's own description lists them under "Tools unchanged (Out=any, no structured output)":
This PR reuses #2468's API surface so the two can be reconciled.
What makes the unions expressible now
Under 2025-11-25 the normative schema typed
outputSchemaas a closed shape whose only root keywords were$schema/type/properties/required, with the comment "Currently restricted to type:objectat the root level", and typedstructuredContentas{ [key: string]: unknown }. Protocol revision 2026-07-28 (SEP-2106) widensoutputSchemato{ $schema?: string; [key: string]: unknown }— "any valid JSON Schema 2020-12" — andstructuredContenttounknown.So a union spanning objects and arrays, which is what
issue_readandpull_request_readneed, was not previously expressible.What changed
output_schemascontrols rollout; the negotiated protocol version, read per request via(*ServerRequest[P]).ProtocolVersion(), controls which schema shapes are legal for a given client.>= 2026-07-28structured_content_only(requiresoutput_schemas). With output schemas alone a tool sends its payload twice. The spec's "SHOULD also return the serialized JSON in a TextContent block" is a backwards-compatibility clause — the SDK says so where it synthesises that block on its typed path, "so that pre-SEP-2106 clients can recover the structured payload from unstructured content".output_schemasoutput_schemas,structured_content_onlyA text block embeds JSON as an escaped string (every
"becomes\") whilestructuredContentcarries it raw. The saving scales with payload size, as measured against the live API on this repo:output_schemas+ structured_content_onlypull_request_readget_filesactions_listlist_workflowsactions_getget_workflow_runpull_request_readget_check_runsText is kept whenever it isn't genuinely redundant: older/unknown protocol version, error results, non-JSON or multi-part results (raw diffs, logs, CSV-converted output), and results where the handler set
structuredContentitself.It's a separate opt-in rather than automatic because negotiating 2026-07-28 does not prove a client reads
structuredContent— no capability advertises it, and a client that ignored it would see an empty result.anyOf, neveroneOfoneOfrequires exactly one branch to match, which fails on real payloads here:actions_run_triggerhas four structurally identical branches,[]satisfies every array branch at once, and anissue_readgeton a sub-issue also satisfies theget_parentbranch.anyOfaccepts all three while still rejecting values matching no branch. A test asserts no schema ever contains"oneOf".Branches are kept non-vacuous with
anyOfover singletonrequired("at least one recognised field") rather thanadditionalProperties: false, so a go-github bump that adds a struct field can't make the server emit a key its own schema forbids.Four incidental bug fixes
GetSubIssuescould emit literalnull. go-github declaresvar subIssues []*SubIssueand leaves it nil on an empty body, which marshals tonull, not[]. Every sibling method normalises withmake; this one didn't. Affects anyone parsing that array today, independent of this feature.sub_issue_write.methodhad noenum— values lived only in prose while dispatch accepted exactlyadd/remove/reprioritize. The one toolsnap in this PR.convertJSONTextResultToCSVclearedStructuredContent, which would have madecsv_outputsilently disableoutput_schemasfor everylist_*tool.actions_getcould emit literalnullfor three methods. go-github decodesGetWorkflowRunByID,GetWorkflowJobByIDandGetWorkflowRunUsageByIDinto a*T, so a 200 with a null body leaves the pointer nil. Surfaced by the conformance tests; same class as the sub-issues bug.MCP impact
output_schemasis enabled. With both flags off, the wire format is byte-identical: no tool advertisesoutputSchema, no result carriesstructuredContent, and none of the 123 committed toolsnaps change (the schema lives onServerTooland is copied onto a duplicatemcp.Toolat registration, soTool.OutputSchemastays nil in the default surface). The single snapshot change in this PR is thesub_issue_writeenum fix.Prompts tested (tool changes only)
Run against the live GitHub API on
github/github-mcp-server, with the server in--read-only --toolsets=all --features=output_schemas,issue_dependencies. Each response'sstructuredContentwas validated against theoutputSchemathe server advertised for that tool over the wire. 23 calls, 23 validated, 0 failures.issue_readgetissue_readget_commentsissue_readget_sub_issuesissue_readget_labelsissue_readget_parentpull_request_readgetpull_request_readget_filespull_request_readget_commitspull_request_readget_statuspull_request_readget_reviewspull_request_readget_commentspull_request_readget_review_commentspull_request_readget_check_runsactions_listlist_workflowsactions_listlist_workflow_runsactions_listlist_workflow_jobsactions_listlist_workflow_run_artifactsactions_getget_workflow_runactions_getget_workflow_run_usageactions_getget_workflow_run_logs_urlactions_getget_workflow_jobissue_dependency_readget_blocked_byissue_dependency_readget_blockingThe whole suite was then re-run with
structured_content_onlyadded: 23/23 still validate, confirming the flag doesn't change what a client can parse.Not exercised live:
actions_run_triggeranddiscussion_comment_writeare the only two schema-bearing tools withReadOnlyHint: false— they trigger/cancel workflow runs, delete logs, and create/delete real discussion comments. They're covered by the mocked conformance tests only, and the harness ran with--read-onlyso they were never even registered.Security / limits
output_schemasalone increases it (~+49%); withstructured_content_onlyit drops below baseline. No new data is exposed —structuredContentcarries the exact bytes the text block already carried.structuredContent, sooutputSchemadescribes only the success shape.Tool renaming
deprecated_tool_aliases.goLint & tests
./script/lint— 0 issues./script/testDocs
docs/feature-flags.md— sections for both flags, including the version-gating table and the size measurements.