docs: trim over-long comments on the output schema code

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>

Author: olaservo

pkg/github/output_schema.go

@@ -16,11 +16,9 @@ import (
 // MustOutputSchema infers an output schema for T, panicking during package
 // initialization if inference fails.
 //
-// Unlike input schemas, an output schema root need not be `{"type":"object"}`:
-// from protocol version 2026-07-28 (SEP-2106) it may be any valid JSON Schema
-// 2020-12, including a bare array or a bare anyOf. Schemas whose root is not
-// an object are stripped per-request for older clients — see
-// inventory.OutputSchemaVersionGate — so inferring one here is safe.
+// Unlike input schemas, an output schema root need not be `{"type":"object"}`,
+// so T may be a slice or a scalar. Non-object roots are stripped per-request
+// for pre-2026-07-28 clients by inventory.OutputSchemaVersionGate.
 func MustOutputSchema[T any]() *jsonschema.Schema {
 	schema, err := jsonschema.For[T](nil)
 	if err != nil {
@@ -32,36 +30,21 @@ func MustOutputSchema[T any]() *jsonschema.Schema {
 
 // AnyOfSchema builds a union output schema over the given branches.
 //
-// It deliberately emits `anyOf` and never `oneOf`. `oneOf` requires that
-// EXACTLY ONE branch match, which is wrong for essentially every tool in this
-// package whose output shape varies by method:
-//
-//   - Branches are frequently structurally identical. All four of
-//     actions_run_trigger's non-run_workflow methods return the same
-//     {message, run_id, status, status_code} map, so a oneOf over them matches
-//     four branches and therefore always fails.
-//   - Empty collections are ambiguous. issue_read method=get_comments on an
-//     issue with no comments returns [], which vacuously satisfies every array
-//     branch.
-//   - Optional fields overlap. issue_read method=get on a sub-issue populates
-//     `parent`, which also satisfies the get_parent branch.
-//
-// anyOf ("at least one") accepts all three while still rejecting values that
-// match no branch, which is the useful half of the validation.
+// Always anyOf, never oneOf: oneOf requires EXACTLY ONE branch to match, and
+// these unions have structurally identical branches, empty arrays that satisfy
+// every array branch, and overlapping optional fields. anyOf still rejects
+// values matching no branch, which is the useful half of the validation.
+// TestAnyOfAcceptsAmbiguousPayloadsButStillRejectsGarbage demonstrates both.
 func AnyOfSchema(branches ...*jsonschema.Schema) *jsonschema.Schema {
 	return &jsonschema.Schema{AnyOf: branches}
 }
 
-// MustRawOutputSchema wraps a hand-authored JSON Schema document, panicking
-// during package initialization if it does not parse or does not resolve.
-//
-// Hand-authored schemas are kept as json.RawMessage rather than
-// *jsonschema.Schema so they round-trip byte-for-byte to the client and
-// produce deterministic toolsnaps output, independent of jsonschema-go's
-// struct field coverage and marshaling order.
+// MustRawOutputSchema wraps a hand-authored JSON Schema document as raw bytes,
+// so it reaches the client exactly as written rather than through
+// jsonschema-go's struct coverage and field ordering.
 //
-// Resolution is checked here (not just parsing) so a dangling $ref fails the
-// build rather than the request.
+// Resolution is checked, not just parsing, so a dangling $ref fails the build
+// rather than a request.
 func MustRawOutputSchema(raw string) json.RawMessage {
 	var parsed jsonschema.Schema
 	if err := json.Unmarshal([]byte(raw), &parsed); err != nil {
@@ -73,9 +56,8 @@ func MustRawOutputSchema(raw string) json.RawMessage {
 	return json.RawMessage(raw)
 }
 
-// outputSchemasEnabled reports whether the output_schemas feature is on for
-// this request. This is the rollout gate only; see canSendStructuredContent
-// for the protocol-legality gate.
+// outputSchemasEnabled is the rollout gate; canSendStructuredContent is the
+// separate protocol-legality gate.
 func outputSchemasEnabled(ctx context.Context, deps ToolDependencies) bool {
 	return deps.IsFeatureEnabled(ctx, FeatureFlagOutputSchemas)
 }
@@ -85,14 +67,10 @@ func isJSONObject(marshaled []byte) bool {
 	return bytes.HasPrefix(bytes.TrimLeft(marshaled, " \t\r\n"), []byte("{"))
 }
 
-// canSendStructuredContent reports whether a structuredContent value of the
-// given marshaled shape may legally be sent to this client.
-//
-// Under 2025-11-25 and earlier, structuredContent is typed
-// `{ [key: string]: unknown }` — a JSON object. 2026-07-28 widened it to
-// `unknown`, explicitly "any JSON value (object, array, string, number,
-// boolean, or null)". So an object is always safe; anything else requires the
-// newer protocol. req may be nil in tests, in which case only objects are sent.
+// canSendStructuredContent reports whether a structuredContent value of this
+// shape may legally be sent to this client. An object is always safe; anything
+// else needs 2026-07-28, which widened the field from an object to any JSON
+// value. A nil req (tests) sends objects only.
 func canSendStructuredContent(req *mcp.CallToolRequest, marshaled []byte) bool {
 	if isJSONObject(marshaled) {
 		return true
@@ -103,15 +81,10 @@ func canSendStructuredContent(req *mcp.CallToolRequest, marshaled []byte) bool {
 	return req.ProtocolVersion() >= inventory.ProtocolVersionNonObjectOutputSchemas
 }
 
-// structuredTextResult builds a tool result whose text content is the
-// serialized textValue — byte-identical to what the tool returned before
-// output schemas existed — and which additionally carries structured as
-// structuredContent when both gates allow it.
-//
-// The text block is always populated regardless of the gates. The spec calls
-// for this independently: "For backwards compatibility, a tool that returns
-// structured content SHOULD also return the serialized JSON in a TextContent
-// block."
+// structuredTextResult returns a result whose text content is the serialized
+// textValue — byte-identical to the pre-output-schema behaviour, and populated
+// regardless of the gates — plus structured as structuredContent when both the
+// feature flag and the client's protocol version allow it.
 func structuredTextResult(ctx context.Context, deps ToolDependencies, req *mcp.CallToolRequest, textValue, structured any) (*mcp.CallToolResult, error) {
 	data, err := json.Marshal(textValue)
 	if err != nil {

pkg/github/output_schemas_polymorphic.go

@@ -8,24 +8,9 @@ import (
 )
 
 // Output schemas for tools whose response shape varies by the `method`
-// argument.
-//
-// These are hand-authored rather than inferred from Go types, because no
-// single Go type describes a method-dispatched tool's output. They live as
-// .json files so they stay reviewable and diffable, and are embedded rather
-// than pasted into Go string literals (their descriptions contain markdown
-// backticks, which raw string literals cannot hold).
-//
-// Every union uses anyOf, never oneOf. oneOf requires EXACTLY ONE branch to
-// match, which is provably wrong here: actions_run_trigger has four
-// structurally identical branches, an empty array vacuously satisfies every
-// array branch, and an issue_read `get` on a sub-issue also satisfies the
-// `get_parent` branch. See AnyOfSchema in output_schema.go.
-//
-// Schemas whose root is not {"type":"object"} are legal only from protocol
-// version 2026-07-28 (SEP-2106); inventory.OutputSchemaVersionGate strips them
-// per-request for older clients. TestPolymorphicOutputSchemaRootKinds pins
-// which schemas fall on which side of that line.
+// argument. Hand-authored, because no single Go type describes a
+// method-dispatched tool's output, and kept as .json so they stay reviewable
+// and diffable. Every union uses anyOf, never oneOf — see AnyOfSchema.
 //
 //go:embed output_schemas/*.json
 var outputSchemaFS embed.FS
@@ -50,14 +35,11 @@ var (
 )
 
 // mustLoadOutputSchema reads an embedded schema, compacts it, and verifies it
-// resolves — panicking during package initialization on any failure so a
-// malformed schema or dangling $ref fails the build rather than a request.
+// resolves, panicking at init so a malformed schema fails the build.
 //
-// Compacting is not cosmetic: it strips inter-token whitespace, so a checkout
-// that rewrote the files' line endings (git's core.autocrlf does this on
-// Windows, and it is what corrupts pkg/octicons' embedded data URIs) cannot
-// leak stray carriage returns into what is sent to clients. It preserves
-// string contents exactly, so descriptions are untouched.
+// Compacting also strips inter-token whitespace, so a checkout that rewrote
+// line endings (core.autocrlf on Windows, which is what corrupts
+// pkg/octicons' embedded data URIs) cannot leak carriage returns to clients.
 func mustLoadOutputSchema(name string) json.RawMessage {
 	raw, err := outputSchemaFS.ReadFile(fmt.Sprintf("output_schemas/%s.json", name))
 	if err != nil {

pkg/inventory/output_schema_gate.go

@@ -7,31 +7,18 @@ import (
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
-// ProtocolVersionNonObjectOutputSchemas is the first MCP protocol revision that
-// permits a tool's outputSchema to have a root other than `{"type":"object"}`,
-// and correspondingly permits structuredContent to be any JSON value rather
-// than only a JSON object (SEP-2106).
-//
-// Under 2025-11-25 the normative schema typed outputSchema as a closed shape:
-//
-//	outputSchema?: { $schema?: string; type: "object";
-//	                 properties?: { [key: string]: object }; required?: string[]; }
-//
-// with the doc comment "Currently restricted to type: \"object\" at the root
-// level." Both the restriction and the closed shape were removed in
-// 2026-07-28, which types it as `{ $schema?: string; [key: string]: unknown }`
-// and describes it as "any valid JSON Schema 2020-12".
+// ProtocolVersionNonObjectOutputSchemas is the first MCP protocol revision
+// permitting a tool's outputSchema to have a root other than
+// `{"type":"object"}`, and structuredContent to be any JSON value rather than
+// only an object. Before it, the normative schema restricted outputSchema to
+// "type: \"object\" at the root level" (SEP-2106 lifted both).
 const ProtocolVersionNonObjectOutputSchemas = "2026-07-28"
 
-// HasObjectRootOutputSchema reports whether schema marshals to a JSON Schema
-// whose root declares `"type": "object"`. Such a schema is legal under every
-// protocol revision that supports outputSchema at all, so it needs no gating —
-// even when it uses composition keywords like anyOf internally.
-//
-// A schema that fails to marshal, or that declares any other root (a bare
-// anyOf, `"type": "array"`, a $ref) is reported as non-object-root and is
-// therefore gated. Failing closed is deliberate: an unmarshalable schema
-// should not be advertised to a client that may reject the whole tools/list.
+// HasObjectRootOutputSchema reports whether schema's root declares
+// `"type": "object"`, which needs no gating at any version — even when it uses
+// anyOf internally. Anything else (a bare anyOf, an array, a $ref) is gated,
+// as is a schema that fails to marshal: failing closed keeps an unparseable
+// schema from breaking a client's whole tools/list.
 func HasObjectRootOutputSchema(schema any) bool {
 	if schema == nil {
 		return false
@@ -58,11 +45,9 @@ func HasObjectRootOutputSchema(schema any) bool {
 //
 // Object-root schemas pass through untouched at every version.
 //
-// The middleware copies on write. The SDK's listTools appends the *same*
-// *mcp.Tool pointers it holds in its registry (mcp/server.go:936-939), so
-// mutating a tool in place here would strip the schema from the server's
-// stored definition and leak that to every later session on the same server.
-// Only tools that actually need gating are copied.
+// Copies on write: the SDK's listTools hands back the *same* *mcp.Tool
+// pointers it holds in its registry, so mutating one here would strip the
+// schema from the server's stored definition for every later session.
 func OutputSchemaVersionGate() mcp.Middleware {
 	return func(next mcp.MethodHandler) mcp.MethodHandler {
 		return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) {
@@ -78,12 +63,10 @@ func OutputSchemaVersionGate() mcp.Middleware {
 			if !ok {
 				return res, err
 			}
-			// ProtocolVersion reads the per-request _meta for >= 2026-07-28
-			// clients (which no longer send initialize at all, per SEP-2575)
-			// and falls back to the session's InitializeParams for older ones.
-			// An empty version means we could not determine it; gate in that
-			// case, since only a client we know is new can be trusted with a
-			// non-object root.
+			// Reads the per-request _meta for >= 2026-07-28 clients (which no
+			// longer send initialize at all, per SEP-2575), falling back to
+			// the session's InitializeParams. Empty means undeterminable, so
+			// it gates.
 			if listReq.ProtocolVersion() >= ProtocolVersionNonObjectOutputSchemas {
 				return res, err
 			}

pkg/inventory/structured_mirror.go

@@ -7,27 +7,16 @@ import (
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
-// mirrorStructuredContent wraps a tool handler so that a tool which declares an
-// OutputSchema also returns structuredContent, without the handler having to
-// produce it separately.
+// mirrorStructuredContent gives tools that declare an OutputSchema a
+// structuredContent value, taken as the raw bytes of the serialized-JSON text
+// block they already emit. Reusing those bytes rather than re-marshaling keeps
+// the two fields byte-identical, so number formatting and key order cannot
+// drift between them.
 //
-// This exploits the relationship the spec already defines between the two
-// fields: "For backwards compatibility, a tool that returns structured content
-// SHOULD also return the serialized JSON in a TextContent block." Every tool in
-// this package already emits exactly that — one text block holding
-// json.Marshal of the result — so the structured value is recoverable from it
-// exactly, with no re-marshaling and therefore no risk of changing number
-// formatting or key order.
+// Only tools declaring a schema are wrapped, leaving the rest unchanged.
 //
-// The mirrored value is the raw bytes of the text block, so content and
-// structuredContent are byte-identical by construction and cannot drift.
-//
-// It applies only to tools that declared an OutputSchema. Tools without one are
-// untouched, so the wire format of the other ~120 tools is unchanged.
-//
-// When omitRedundantText is set, the now-duplicated text block is dropped for
-// clients new enough to read structuredContent, halving the response instead of
-// doubling it. See dropRedundantTextContent.
+// omitRedundantText additionally drops the now-duplicated text block; see
+// dropRedundantTextContent.
 func mirrorStructuredContent(next mcp.ToolHandler, omitRedundantText bool) mcp.ToolHandler {
 	return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) {
 		res, err := next(ctx, req)
@@ -60,29 +49,12 @@ func mirrorStructuredContent(next mcp.ToolHandler, omitRedundantText bool) mcp.T
 }
 
 // dropRedundantTextContent removes the text block that structuredContent now
-// duplicates byte-for-byte.
-//
-// 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 says as much 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" (mcp/server.go). A client that negotiated
-// 2026-07-28 is not such a client, so for those the block is pure duplication —
-// this server otherwise sends the same JSON twice, which works against
-// csv_output, minimal_output and the fields param, all of which exist to make
-// responses smaller.
-//
-// Callers must only reach here when structuredContent was mirrored from this
-// exact text, so nothing is lost: the bytes survive, they just travel once.
-//
-// content stays present as an empty array rather than being unset — the draft
-// schema still lists it in CallToolResult's required set, and the SDK
-// normalises an empty slice to `[]` rather than `null`.
+// duplicates byte-for-byte. The spec's "SHOULD also return the serialized JSON
+// in a TextContent block" is a backwards-compatibility clause, so for a client
+// that reads structuredContent the block is pure duplication.
 //
-// This is opt-in (see RegisterToolOptions.OmitRedundantTextContent) rather than
-// automatic, because negotiating 2026-07-28 does not prove a client actually
-// reads structuredContent — there is no capability that says so, and a client
-// that ignored it would see an empty result.
+// Callers must only reach here having mirrored from this exact text. Empty
+// rather than unset: content is in CallToolResult's required set.
 func dropRedundantTextContent(res *mcp.CallToolResult) {
 	res.Content = []mcp.Content{}
 }