fix: reject unknown tool parameters instead of silently dropping them (#19)

[noneck]

DO NOT MERGE BEFORE 2026-07-27 — live demos run on this server until then.

Gonzalez (Wed) and Maloney (Mon) run against these MCPs. Strict schemas turn silently-succeeding calls into throwing ones, which is the point, but it is not a change to land underneath a live meeting. Merge window opens Monday 2026-07-27.

The bug

zod strips unknown keys by default. An undeclared parameter vanished with no error and the tool returned unfiltered results — real, correctly formatted, correctly sourced data answering a different question, with nothing in the response for a calling model to detect.

search_contracts(vendor="Community League of the Heights", page_size=3)
  → 5,755,099 unrelated contract records

vendor_name is the declared parameter, and passing that correctly already throws VENDOR_NAME_UNSUPPORTED_MESSAGE naming three alternatives. But vendor is undeclared, so zod stripped it and the guard never fired. A one-word typo defeated a deliberate guard.

The fix — two layers

(a) Advertised contract. Worth stating plainly, because it corrects the issue's premise for this repo: tools/list already emitted additionalProperties: false for all ten tools under @modelcontextprotocol/sdk 1.29.0 — zod-to-json-schema emits it for a plain strip-mode z.object too. So the advertised contract was already honest and the server was contradicting it, which is the worse of the two failure shapes. .strict() preserves that output, and test/strict-schema.test.mjs now pins it so an SDK bump inside the ^1.0.0 range cannot drop it silently.

(b) Server-side enforcement. This is the layer that was actually missing. An unknown key now raises Input validation error before the handler runs, upstream request included — the API is never reached.

(c) Alias hint. search_contracts maps the observed guess vendor → vendor_name and surfaces VENDOR_NAME_UNSUPPORTED_MESSAGE, so a caller who typos into the undeclared name lands on the same three alternatives rather than 5.7M rows:

MCP error -32602: Input validation error: Invalid arguments for tool search_contracts: [
  { "code": "unrecognized_keys", "keys": ["vendor"],
    "message": "Unrecognized key(s) in object: 'vendor'. 'vendor' is not a parameter of
    search_contracts — the declared parameter is 'vendor_name'. search_contracts cannot
    filter by vendor name: ... (1) pass vendor_code ... (2) use search_spending with
    payee_name ... (3) use smart_search ..." }
]

Only search_contracts carries an alias. Every other tool gets plain .strict() — zod's default already names the offending key loudly, and speculative alias entries are not worth the surface.

The raw-shape → ZodObject change

This repo is the odd one out in the fleet: it uses server.registerTool(...) from McpServer with a zod raw shape as inputSchema, and you cannot call .strict() on a raw shape. So each inputSchema: { ... } becomes inputSchema: strictSchema({ ... }), where strictSchema returns a full ZodObject with .strict() and an optional per-schema errorMap for aliases.

Passing a ZodObject directly is supported: registerTool routes inputSchema through getZodSchemaObject (node_modules/@modelcontextprotocol/sdk/dist/esm/server/mcp.js:861), which returns a schema instance unchanged and only wraps bare raw shapes. tools/list separately routes through normalizeObjectSchema, which likewise passes a _def-bearing schema through untouched. Verified empirically against SDK 1.29.0, not just read.

.default() values still apply through .strict() — a regression test asserts status/category still reach the request XML.

Tests

New test/strict-schema.test.mjs (5 tests), fixture-backed via a stubbed globalThis.fetch, zero network calls to Checkbook NYC. New fixture test/fixtures/contracts-response.xml.

Before the fix (the two behavioral tests fail, which is the point):

✔ every tool advertises additionalProperties:false (#19)
✖ search_contracts with the undeclared 'vendor' key errors and names vendor_name (#19)
✖ an unknown key on any other tool is refused too (#19)
✔ a valid search_contracts call still runs and returns records (#19)
✔ vendor_name still returns VENDOR_NAME_UNSUPPORTED_MESSAGE (#19 regression on #16)
ℹ pass 3   ℹ fail 2

  AssertionError: an undeclared parameter must raise, not be silently dropped and answered
  + actual - expected
  + undefined
  - true

After the fix, full suite (npm test = npm run build && node --test "test/*.test.mjs", local build, not the npx cache):

ℹ tests 40
ℹ pass 40
ℹ fail 0
ℹ duration_ms 134.346125

Not in this PR

• No version bump. PR Add UNVERIFIED contracts filters behind live-verify gate (v1.3.2, #6/#8/#10) — supersedes #14 #18 already moves to 1.3.2 and is open; bumping here would collide. CHANGELOG entry sits under [Unreleased].
• No new dependencies (zod is already ^3.23.0). No parameter renamed or removed.
• Not published, not merged, not /mcp-update-ed.

Follow-up when this lands

Presenter notes in BetaNYC/grounding-ai-with-ny-open-data document the current, broken behavior as verified. When this merges those notes go stale, and a stale presenter note is worse than none because someone will trust it in a meeting. Update the Reinvent Albany script's worked example in the same pass.

Refs #19

🤖 Generated with Claude Code

[noneck]

Version collision to watch with #18.

This PR is now versioned 1.4.0 per the fleet-wide minor-bump decision (2026-07-21). #18 targets 1.3.2.

If this merges first, #18 needs re-versioning to 1.4.1 or 1.5.0 — shipping 1.3.2 after 1.4.0 would be a downgrade. If #18 merges first, this is unaffected.

Also folded in here: the node --test glob is now unquoted. Quoted, node does the matching itself, and Node 20 does not support glob patterns there at all (added in 22) — it treats the pattern as a literal path and runs zero tests. This was caught by CI in nyc-record-mcp (#12), which is the only repo in the fleet whose CI actually runs its test suite.

⚠️ This repo's CI is still build-only, so the 40 tests here do not run on PRs yet. Worth fixing separately — the same one-step change as nyc-record-mcp#12, plus test/** in the path filters.

[noneck]

Correction to my earlier comment on this PR. I said this repo's CI was build-only and that the 40 tests did not run. That was wrong. The workflow already had a Test step running a bare node --test; the job was merely named "Build", which is what I read off the check name without opening the file. All 40 tests were already running on both Node 20 and 22 — verified in the run logs, not inferred.

Bare node --test with no arguments discovers test files by walking the directory, and that has worked since Node 18. It is only glob patterns as arguments that need Node 22, which is the separate bug fixed in the commit above.

The real gap, now closed: CI ran node --test while developers run npm test. Different commands — and that divergence is exactly why CI stayed green while npm test was broken on Node 20. A gate cannot vouch for a command it does not run. CI now runs npm test, plus test/** is in the path filters so a test-only PR no longer skips the gate silently.

Apologies for the noise. Reading a check name instead of the workflow file is the same mistake as trusting a plausible response instead of reading the schema, which is the bug this PR exists to fix.