fix(schema): advertise every tool's outputSchema as permissive (propagates apple-mail-mcp#135) - #123
Merged
Merged
Conversation
The MCP CLIENT validates structuredContent against the ADVERTISED JSON Schema, and a bare zod raw shape renders as additionalProperties:false — so any field a handler emits that its schema doesn't enumerate is a hard client-side -32602 that discards an otherwise-correct result. The server never notices, because zod's own parse strips unknown keys rather than failing, which is why the registerTool/outputSchema migration's "all fields optional, no .strict()" read as permissive: it covered optionality, not undeclared keys. Every tool in this repo was advertising additionalProperties:false. All now register through a wrapper applying .passthrough(), and the contract test fails any tool that regresses. Found while fixing the same defect in apple-mail-mcp, where it was not latent — it broke get-mail-stats on every call (sweetrb/apple-mail-mcp#135).
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.
Propagates the schema half of sweetrb/apple-mail-mcp#135 to this repo. Same defect, same fix, verified the same way.
The defect
The MCP client validates a result's
structuredContentagainst the JSON Schema the server advertised — not against the server's own zod object. A bare zod raw shape renders asadditionalProperties: false, so any field a handler emits that its schema doesn't enumerate becomes a hard client-side-32602 … data must NOT have additional properties, discarding a payload the handler computed correctly.The server never notices, because zod's own parse silently strips unknown keys instead of failing. That is exactly why the
registerTool/outputSchemamigration's "all fields optional, no.strict()" was believed permissive: it covered optionality, it did not cover undeclared keys.Why it matters here
In apple-mail-mcp this was not latent — it broke
get-mail-statson every call for anyone with IMAP configured, because that tool's IMAP branch spreads an object carrying aperMailboxkey the schema never declared. Measuring every repo in the family showed the same advertisement everywhere:additionalProperties: false(before)So in this repo it is currently latent: it costs nothing until some handler's payload gains a key its schema doesn't list, at which point that tool fails completely rather than degrading — and CI would stay green while it happened.
The fix
Every tool registers through a wrapper that wraps its shape in
.passthrough(), advertisingadditionalProperties: true— the contract the migration intended. A declared field still documents the shape; an undeclared one is carried through instead of nuking the result. No tool signature, parameter or behaviour changes.The guard
The outputSchema contract test now fails any tool advertising
additionalProperties: false. The existing assertions couldn't see this class: they check that every tool has a schema and that none requires a field, then round-trip only the diagnostic tools — so a tool with an undeclared key passes CI and fails in the user's client. Verified a real guard rather than a tautology: it reports every tool in this repo as an offender before the fix and none after.Verification
typecheck,lint,format:checkclean; full unit suite passesadditionalProperties: falsecount went to 0