Skip to content

ADR 061: The MCP surface runs on the official SDK and every tool names its payload

This page is generated from docs/decisions/*.yaml by task docs:export-adr-markdown. Do not edit manually.

  • Number: 061
  • Title: The MCP surface runs on the official SDK and every tool names its payload
  • Category: architecture
  • Status: accepted
  • Provenance: guided-ai
  • Source: docs/decisions/061-mcp-output-contract-on-the-official-sdk.yaml

Decision

The MCP server is built on github.com/modelcontextprotocol/go-sdk, registering every tool through the generic mcp.AddTool[In, Out]. The input schema is derived from In and the output schema from Out, so a handler cannot return a shape its published schema does not describe. The SDK validates arguments before the handler runs and validates the marshalled result before it leaves the server. Every tool declares an output schema. This is the property that was missing: none of the 24 tools declared one before, so nothing validated what a handler returned. Every Out is a struct with named fields, so structuredContent is always a JSON object. Collections are named for what they hold — branches, tags, commits — and single objects are named too: commit, tag, comment. The pull-request tools already named theirs and set the precedent the rest now follow. The generic items key that wrapped eight list tools is gone, and so is the resultJSON choke point that applied it. A test drives every tool through a real in-process client against fixture responses. It no longer validates the payload against the advertised schema: the SDK derives that schema from the handler output type and applies it to the marshalled result before it leaves the process, so a test doing it afterwards re-checked the framework. What it checks is what the SDK does not -- that structuredContent is a JSON object, because a schema saying "array" and an array result both satisfy the SDK while a pre-SEP-2106 client rejects the response outright, and that a text fallback exists for clients that do not read structuredContent. A second test fails when a tool is added without an argument fixture, so a new tool cannot go uncalled. Tools carry annotations. ReadOnlyHint says whether the tool writes at all and is declared per tool. DestructiveHint is derived from the Safe classification ADR-039 defines rather than declared beside it: the two answer the same question, and two hand-written answers can disagree in the direction that matters -- Safe is what the server enforces, DestructiveHint is advice a client may ignore. A test fails when a tool annotated read-only is withheld without --yolo.

Agent Instructions

Add a tool with mcp.AddTool through the toolSpec helper in internal/mcp/server.go. Define an input struct and an output struct; do not hand-write a schema. Fields without omitempty become required, fields with it become optional, and the jsonschema struct tag carries the description. The output struct always names its payload. A handler returning a collection returns struct{ Commits []T json:"commits" }, never a bare slice and never a generic items key. Name a single object too. A model reads a fresh tool result on every call with no parser holding a field path, so the name is what tells it what it is looking at. Return a Go error for a failure. The SDK packs it into the result content with IsError set, so there is no need to build an error result by hand, and no need to return a nil error alongside one. Add an entry to toolArguments in the client compatibility test under internal/mcp in the same change. TestEveryToolHasCallArguments fails without one, and without it the new tool is never called by any test. Do not add an enum to an input schema for a value the service layer normalises. Pinning the upper-case spelling of a role rejects "author", which works today. Put the permitted values in the field description instead. Enums are for vocabularies the service does not widen. Do not reintroduce a versioned envelope here. The CLI's --json surface has one for a different consumer; see the rationale.

Rationale

PR #414 fixed a real bug — a bare JSON array in structuredContent, which MCP clients validating against a pre-SEP-2106 revision reject outright with "expected record, received array" — by wrapping non-objects under items at a single choke point. That was the right altitude for an unblock and it did not close the class, because the layer had no output contract at all: no declared schemas, no server-side validation, and no test driving a real client, which is why a wire-shape bug reached a user before a test did. The official SDK closes the class as library behaviour rather than as local discipline. Deriving both schemas from the same type parameters the handler uses makes drift structurally impossible; validating the marshalled JSON rather than the Go value handles types with a custom MarshalJSON; typed nils become zero values instead of null; and a serialised-JSON text fallback is guaranteed for clients that do not read structuredContent. Every defect found reviewing #414 is handled there. Migrating also widened client compatibility rather than narrowing it. mark3labs/mcp-go v0.58.0 targets protocol revision 2025-11-25; go-sdk v1.7.0 negotiates 2024-11-05 through 2026-07-28, serving both the old initialize handshake and the new server/discover RPC, and honouring a lagging client's version rather than forcing it forward. Nothing that worked before stopped working. Naming every payload is for agent legibility, which is the property this surface should optimise for. items says nothing about what it holds, and it sat next to handlers that already named theirs. Naming them all means one rule instead of four conventions, leaves room for pagination metadata beside the collection, and makes the pre-SEP-2106 array rejection unreachable by construction rather than by a special case at a choke point. SEP-2106 has since relaxed structuredContent to any JSON value, so a bare array is legal on the current revision; naming is a deliberate choice, not a workaround. ADR-014 requires versioned envelopes for the CLI's --json output because scripts pin field paths there. That reasoning does not transfer. An MCP consumer is a model reading a fresh result on every call with no compiled parser, so forward-compatible parsing buys nothing and the wrapper costs attention on every call. It also means shape changes on this surface are cheap, which is why this landed as an ordinary change rather than a breaking one. Migrating deleted the req.RequireString / GetString / GetInt boilerplate across all 24 tools. That boilerplate discarded its error at every call site, so a missing project reached the API as a request for project "" rather than being rejected. Schema validation now rejects it before the handler runs.

Rejected Alternatives

  • Add WithOutputSchema and output validation to the existing mcp-go server: Closes the immediate gap and leaves the schema hand-maintained beside the handler, which is the drift this repository has repeatedly paid for. It also leaves the server a protocol revision behind, since v0.58.0 targets 2025-11-25 and the current revision is 2026-07-28.
  • Upgrade to mcp-go v1.0.0-beta.1, which does target 2026-07-28: Same protocol coverage, but a beta rather than a stable release, and it still does not derive the output schema from the handler's return type. The schema-drift argument, not the protocol argument, is what decided this.
  • Keep the items wrapper and only add schemas: Leaves an agent reading a key that tells it nothing, and keeps four envelope conventions across 24 tools. The wrapper existed as compatibility with pre-SEP-2106 clients; naming the payload gives the same compatibility and says what the payload is.
  • Adopt the bb.machine {version, data, meta} envelope from ADR-014: Buys forward-compatible parsing for a consumer that does no parsing, in exchange for a wrapper the model sees past on every call. The property earns its keep on the CLI surface, where scripts pin field paths, and nothing downstream of MCP needs it.
  • Return generated OpenAPI types directly as the published schema: Kept, after checking. RestCommit, RestBuildStatus, RestRequiredBuildCondition and RestMinimalRef are all shallow and under fifteen fields, so they publish cleanly. Narrow view types would be a second definition to maintain for no gain. Revisit per-type if a future tool returns an unwieldy generated struct.