ADR-1288: An MCP tool schema that fails to marshal is fatal, never defaulted¶
- Status: Accepted
- Date: 2026-09-21
- Deciders: Lusoris
- Tags:
mcp,agents,security,api
Context¶
cmd/vmafx-mcp/tools.go builds each tool's JSON-Schema as a Go literal and serialises it at startup. The original helper, mustSchema, panicked on a json.Marshal failure. HISS-07 flags a panic in a production code path (.standards-baseline.json carried cmd/vmafx-mcp/tools.go:962:HISS-07, "Legacy panic invocation in production code path"), so the HISS-21 burn-down had to discharge it.
The first attempt replaced the panic with a logged fallback to {"type":"object"}. That schema is not a degraded version of the real one — it is the absence of one. JSON-Schema validation is the server's only check on tool arguments before a handler runs, and an empty object schema accepts every argument map. The result is a tool that is listed, looks healthy to the client, and has had its declared contract silently switched off; the Go↔Python parity tests would not catch it, because they only compare the tools that are registered. Trading a loud startup failure for a silent, permanently-degraded tool is the wrong direction for a surface that agents call unattended.
Decision¶
A tool input schema that fails to marshal aborts registration and the process. toolSchema returns (json.RawMessage, error); toolRegistrar.add is the only writer of mcp.Tool.InputSchema, so no caller can construct a tool whose schema did not marshal. The first failure is retained, every later add is a no-op, registerTools returns the error, buildServer discards the half-built server, and the fx provider buildMCPServer fails the graph so the process exits non-zero. No default schema is ever substituted, and there is no flag to relax this — the failure is a defect in the server's own literals, not an operator condition.
Alternatives considered¶
| Option | Pros | Cons | Why not chosen |
|---|---|---|---|
| Return the error up the registration chain, fatal at startup (chosen) | No reachable path from a marshal failure to a registered tool; the failure names the tool; satisfies HISS-07 without a panic | buildServer / buildMCPServer grow an error return, and five test call sites check it | — |
Keep panic in mustSchema | Simplest; already fail-fast | Violates HISS-07, which is the invariant being discharged; a panic in an fx provider unwinds without fx's shutdown path | Rejected: the rule exists and a panic is the crudest form of the correct behaviour |
Log and substitute {"type":"object"} | Server keeps serving the other 23 tools | The affected tool stays reachable with no argument validation at all, indistinguishable over the wire from a healthy one; a log line on a stdio server is easily lost | Rejected: this is the finding this ADR exists to correct |
| Register the tool but make its handler refuse every call | Keeps the tool list stable | Same lie to the client (listed, advertised, unusable), with a worse error surface — a per-call failure instead of one at startup | Rejected: a broken tool listed as available is worse than a server that refuses to start |
Marshal all schemas in an init() and log.Fatal | No signature changes | log.Fatal bypasses fx's lifecycle, and init()-time failure cannot say which fx provider it belongs to | Rejected: the fx graph already has a failure channel |
Consequences¶
- Positive: "a listed tool is a validated tool" holds unconditionally. A schema defect surfaces at startup with the tool's name instead of as unexplained handler behaviour later. The
panicis gone, so HISS-07 is discharged structurally rather than by suppression. - Negative: one malformed schema literal takes the whole server down, so a defect in one tool blocks the other 23. That is the intended trade: the schemas are compile-time literals with no runtime input, so the failure is a source bug that must be fixed, not tolerated.
- Neutral / follow-ups:
buildServerreturns(*mcp.Server, error)— thecmd/vmafx-mcp/AGENTS.mdseam note that previously said "do not change signature" now records why the error return is load-bearing.cmd/vmafx-mcp/tool_schema_test.gopins each link of the chain, including that a marshal failure yields neither a permissive schema nor a registered tool.
References¶
cmd/vmafx-mcp/AGENTS.mdinvariants #19 and thebuildServerseam.docs/mcp/index.md§ "Startup contract: all tools or none".docs/rebase-notes.md§ "vmafx-mcp tool schemas fail closed (2026-09-21)".- ADR-1184 — the Go-only gRPC bridge tools that share this registration path.
- HISS-07 ("Checked Errors") in
.config/hiss/coverage.yaml; baseline fingerprintcmd/vmafx-mcp/tools.go:962:HISS-07. - Source: adversarial review of branch
chore/hiss21-go-cmd, paraphrased: a schema that accepts anything is not a safe fallback for a schema that failed to build; "handled" under HISS-07 means propagated or fatal, not swallowed into a permissive default.