fix: reject unknown tool parameters instead of silently dropping them - #11
Merged
Conversation
…s.ts No behavior change. src/index.ts connects a stdio transport at import time, so nothing in it can be imported by a test without starting a server. Moving the tool array and the per-tool zod schemas into their own module makes both testable offline, which the strict-parameter fix (#10) needs. Refs #10 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
zod strips unknown keys by default, so a tool call carrying an invented filter parsed cleanly with the filter removed and returned real, correctly formatted, unfiltered data. Nothing in the response signalled the drop, so a consuming model could not detect it. In the sibling budget repo the same defect returned $47.5M of citywide awards for a question scoped to one council district. Two layers: - `additionalProperties: false` on every advertised inputSchema, so the calling model knows an invented parameter is invalid before it calls. - `.strict()` on every argument schema, so anything that slips past the advertised contract raises server-side. The error names the offending key and the parameters that tool does accept, read back off the advertised schema rather than a hand-maintained table. Refs #10 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Minor rather than patch, matching the fleet-wide decision: tool-call behavior visibly changes for any caller that was passing an undeclared parameter. No declared parameter was renamed or removed. Co-Authored-By: Claude Opus 4.8 <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.
DO NOT MERGE BEFORE 2026-07-27 — live demos run on this server until then.
The defect
zod strips unknown keys by default. A tool call carrying a parameter the tool does not declare parsed cleanly with that parameter removed, and the handler returned real, correctly formatted, correctly sourced data — just not the data that was asked for. Nothing in the response signalled the drop, so a consuming model could not detect it.
Agents guess parameter names. That is normal. The correct response is to reject the guess loudly, not to answer a different question.
The fix — two layers
1.
additionalProperties: falseon every advertisedinputSchema. This is the layer that actually prevents the bad call: it tells the calling model, before it calls, that an invented parameter is invalid. Verified end to end over stdio —tools/listreturnsadditionalProperties: falseon all seven tools after SDK serialization.2.
.strict()on every argument schema. Server-side enforcement, so anything slipping past the advertised contract raises rather than being silently stripped.The error names the right parameter. zod's default strict message names the bad key but offers no alternative.
parseToolArgscatches theunrecognized_keysissue and reports the accepted names, read back off that tool's advertised schema rather than a hand-maintained table:This follows the spirit of
VENDOR_NAME_UNSUPPORTED_MESSAGEinnyc-checkbook-mcp: name the limitation, then name the supported alternatives, in plain prose.Structural change
src/index.tsconnects a stdio transport at import time, so nothing in it can be imported by a test without starting a server. The tool array and the per-tool zod schemas move tosrc/tools.ts(first commit, no behavior change) so both are testable offline.index.tsshrinks: each dispatch case is now a singleparseToolArgs(...)call with type inference preserved.Test plan
New
test/strict-schema.test.mjs, run offline — it exercises the argument-validation layer where the drop happened and never reaches the Socrata City Record API.additionalProperties: false, andTOOLSandSCHEMAScover the same tool names.bogus_unknown_paramraises, for every one of the seven tools.undefinedargs), and an ordinary bad value for a declared parameter still reports its own message rather than the unknown-key one.Before the fix (extraction commit built and run, so the failure is real, not a missing import):
Direct reproduction against that same build:
After the fix, clean build (
rm -rf dist && npm test), Node 26.5.0:All eight pre-existing SoQL and encoding tests still pass. No new dependencies; no parameter renamed or removed.
Refs #10