From c16b26fb56ffd757b58f50e0de44aae0dca7ede2 Mon Sep 17 00:00:00 2001 From: Noel Hidalgo Date: Tue, 21 Jul 2026 12:19:56 -0400 Subject: [PATCH 1/3] refactor: extract tool definitions and argument schemas into src/tools.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 --- src/index.ts | 137 ++++-------------------------------- src/tools.ts | 133 ++++++++++++++++++++++++++++++++++ test/strict-schema.test.mjs | 96 +++++++++++++++++++++++++ 3 files changed, 241 insertions(+), 125 deletions(-) create mode 100644 src/tools.ts create mode 100644 test/strict-schema.test.mjs diff --git a/src/index.ts b/src/index.ts index bd78b32..aacb438 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,6 @@ import { ListToolsRequestSchema, } from "@modelcontextprotocol/sdk/types.js"; import { createRequire } from "node:module"; -import { z } from "zod"; import { searchNotices, getNoticesByAgency, @@ -15,12 +14,8 @@ import { getPublicHearings, getOpenSolicitations, getNoticesByDateRange, - NOTICE_TYPES, } from "./city-record.js"; - -const isoDate = z - .string() - .regex(/^\d{4}-\d{2}-\d{2}$/, "must be a date in YYYY-MM-DD format"); +import { TOOLS, parseToolArgs } from "./tools.js"; const require = createRequire(import.meta.url); const { version } = require("../package.json") as { version: string }; @@ -30,100 +25,7 @@ const server = new Server( { capabilities: { tools: {} } } ); -server.setRequestHandler(ListToolsRequestSchema, async () => ({ - tools: [ - { - name: "search_notices", - description: - "Full-text search across all NYC City Record notices. Returns recent matching notices sorted by date.", - inputSchema: { - type: "object", - properties: { - query: { type: "string", description: "Search term" }, - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - required: ["query"], - }, - }, - { - name: "get_notices_by_agency", - description: - "Get City Record notices published by a specific city agency (partial name match).", - inputSchema: { - type: "object", - properties: { - agency_name: { type: "string", description: "Agency name or partial name, e.g. 'DCAS', 'Parks'" }, - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - required: ["agency_name"], - }, - }, - { - name: "get_notices_by_type", - description: - "Get notices filtered by type. Valid types: Solicitation, Award, Intent to Award, Intent to Negotiate, Public Hearings, Public Comment, Meeting, Notice, Vendor List, Sale.", - inputSchema: { - type: "object", - properties: { - notice_type: { - type: "string", - enum: [...NOTICE_TYPES], - description: "Notice type", - }, - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - required: ["notice_type"], - }, - }, - { - name: "get_procurement_notices", - description: - "Get recent procurement-related notices: solicitations, awards, intent to award, vendor lists. Useful for tracking open contracts and recent awards.", - inputSchema: { - type: "object", - properties: { - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - }, - }, - { - name: "get_public_hearings", - description: - "Get recent public hearings, public comment periods, and agency meetings from the City Record.", - inputSchema: { - type: "object", - properties: { - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - }, - }, - { - name: "get_open_solicitations", - description: - "Get active solicitations (RFPs, RFQs, IFBs) where the due date has not yet passed. Sorted by due date ascending — soonest deadlines first.", - inputSchema: { - type: "object", - properties: { - limit: { type: "number", description: "Max results (default 25, max 100)" }, - }, - }, - }, - { - name: "get_notices_by_date_range", - description: - "Get all City Record notices published within a date range.", - inputSchema: { - type: "object", - properties: { - start_date: { type: "string", description: "Start date, YYYY-MM-DD" }, - end_date: { type: "string", description: "End date, YYYY-MM-DD" }, - limit: { type: "number", description: "Max results (default 50, max 200)" }, - }, - required: ["start_date", "end_date"], - }, - }, - ], -})); +server.setRequestHandler(ListToolsRequestSchema, async () => ({ tools: TOOLS })); server.setRequestHandler(CallToolRequestSchema, async (request) => { const { name, arguments: args } = request.params; @@ -131,61 +33,46 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { try { switch (name) { case "search_notices": { - const { query, limit } = z - .object({ query: z.string(), limit: z.number().max(100).optional() }) - .parse(args); + const { query, limit } = parseToolArgs("search_notices", args); const results = await searchNotices(query, limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_notices_by_agency": { - const { agency_name, limit } = z - .object({ agency_name: z.string(), limit: z.number().max(100).optional() }) - .parse(args); + const { agency_name, limit } = parseToolArgs("get_notices_by_agency", args); const results = await getNoticesByAgency(agency_name, limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_notices_by_type": { - const { notice_type, limit } = z - .object({ notice_type: z.enum(NOTICE_TYPES), limit: z.number().max(100).optional() }) - .parse(args); + const { notice_type, limit } = parseToolArgs("get_notices_by_type", args); const results = await getNoticesByType(notice_type, limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_procurement_notices": { - const { limit } = z - .object({ limit: z.number().max(100).optional() }) - .parse(args ?? {}); + const { limit } = parseToolArgs("get_procurement_notices", args); const results = await getProcurementNotices(limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_public_hearings": { - const { limit } = z - .object({ limit: z.number().max(100).optional() }) - .parse(args ?? {}); + const { limit } = parseToolArgs("get_public_hearings", args); const results = await getPublicHearings(limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_open_solicitations": { - const { limit } = z - .object({ limit: z.number().max(100).optional() }) - .parse(args ?? {}); + const { limit } = parseToolArgs("get_open_solicitations", args); const results = await getOpenSolicitations(limit ?? 25); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } case "get_notices_by_date_range": { - const { start_date, end_date, limit } = z - .object({ - start_date: isoDate, - end_date: isoDate, - limit: z.number().max(200).optional(), - }) - .parse(args); + const { start_date, end_date, limit } = parseToolArgs( + "get_notices_by_date_range", + args + ); const results = await getNoticesByDateRange(start_date, end_date, limit ?? 50); return { content: [{ type: "text", text: JSON.stringify(results, null, 2) }] }; } diff --git a/src/tools.ts b/src/tools.ts new file mode 100644 index 0000000..c6cbc46 --- /dev/null +++ b/src/tools.ts @@ -0,0 +1,133 @@ +import type { Tool } from "@modelcontextprotocol/sdk/types.js"; +import { z } from "zod"; +import { NOTICE_TYPES } from "./city-record.js"; + +const isoDate = z + .string() + .regex(/^\d{4}-\d{2}-\d{2}$/, "must be a date in YYYY-MM-DD format"); + +/** Tool definitions advertised to clients by the ListTools handler. */ +export const TOOLS: Tool[] = [ + { + name: "search_notices", + description: + "Full-text search across all NYC City Record notices. Returns recent matching notices sorted by date.", + inputSchema: { + type: "object", + properties: { + query: { type: "string", description: "Search term" }, + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + required: ["query"], + }, + }, + { + name: "get_notices_by_agency", + description: + "Get City Record notices published by a specific city agency (partial name match).", + inputSchema: { + type: "object", + properties: { + agency_name: { type: "string", description: "Agency name or partial name, e.g. 'DCAS', 'Parks'" }, + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + required: ["agency_name"], + }, + }, + { + name: "get_notices_by_type", + description: + "Get notices filtered by type. Valid types: Solicitation, Award, Intent to Award, Intent to Negotiate, Public Hearings, Public Comment, Meeting, Notice, Vendor List, Sale.", + inputSchema: { + type: "object", + properties: { + notice_type: { + type: "string", + enum: [...NOTICE_TYPES], + description: "Notice type", + }, + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + required: ["notice_type"], + }, + }, + { + name: "get_procurement_notices", + description: + "Get recent procurement-related notices: solicitations, awards, intent to award, vendor lists. Useful for tracking open contracts and recent awards.", + inputSchema: { + type: "object", + properties: { + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + }, + }, + { + name: "get_public_hearings", + description: + "Get recent public hearings, public comment periods, and agency meetings from the City Record.", + inputSchema: { + type: "object", + properties: { + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + }, + }, + { + name: "get_open_solicitations", + description: + "Get active solicitations (RFPs, RFQs, IFBs) where the due date has not yet passed. Sorted by due date ascending — soonest deadlines first.", + inputSchema: { + type: "object", + properties: { + limit: { type: "number", description: "Max results (default 25, max 100)" }, + }, + }, + }, + { + name: "get_notices_by_date_range", + description: "Get all City Record notices published within a date range.", + inputSchema: { + type: "object", + properties: { + start_date: { type: "string", description: "Start date, YYYY-MM-DD" }, + end_date: { type: "string", description: "End date, YYYY-MM-DD" }, + limit: { type: "number", description: "Max results (default 50, max 200)" }, + }, + required: ["start_date", "end_date"], + }, + }, +]; + +/** Argument schemas, one per tool name in TOOLS. */ +export const SCHEMAS = { + search_notices: z.object({ + query: z.string(), + limit: z.number().max(100).optional(), + }), + get_notices_by_agency: z.object({ + agency_name: z.string(), + limit: z.number().max(100).optional(), + }), + get_notices_by_type: z.object({ + notice_type: z.enum(NOTICE_TYPES), + limit: z.number().max(100).optional(), + }), + get_procurement_notices: z.object({ limit: z.number().max(100).optional() }), + get_public_hearings: z.object({ limit: z.number().max(100).optional() }), + get_open_solicitations: z.object({ limit: z.number().max(100).optional() }), + get_notices_by_date_range: z.object({ + start_date: isoDate, + end_date: isoDate, + limit: z.number().max(200).optional(), + }), +} satisfies Record; + +export type ToolName = keyof typeof SCHEMAS; + +export function parseToolArgs( + name: K, + args: unknown +): z.infer<(typeof SCHEMAS)[K]> { + return SCHEMAS[name].parse(args ?? {}); +} diff --git a/test/strict-schema.test.mjs b/test/strict-schema.test.mjs new file mode 100644 index 0000000..f49b89d --- /dev/null +++ b/test/strict-schema.test.mjs @@ -0,0 +1,96 @@ +// Unknown tool parameters must be rejected, not silently dropped (issue #10). +// +// Runs entirely offline: it exercises the argument-validation layer, which is +// where the drop happened, and never reaches the Socrata City Record API. +import test from "node:test"; +import assert from "node:assert/strict"; +import { TOOLS, SCHEMAS, parseToolArgs } from "../dist/tools.js"; + +// Durable: holds for tools added later, not just today's seven. +test("every advertised tool sets additionalProperties: false", () => { + assert.ok(TOOLS.length > 0, "TOOLS must not be empty"); + for (const tool of TOOLS) { + assert.equal( + tool.inputSchema.additionalProperties, + false, + `${tool.name} must set additionalProperties:false so unknown params are rejected` + ); + } +}); + +test("every advertised tool has an argument schema, and vice versa", () => { + assert.deepEqual( + TOOLS.map((t) => t.name).sort(), + Object.keys(SCHEMAS).sort() + ); +}); + +// The behavioral test from issue #10: today the call succeeds and returns +// normal results; after the fix it raises. +test("a call carrying an undeclared parameter is rejected", () => { + assert.throws( + () => + parseToolArgs("get_open_solicitations", { + limit: 1, + bogus_unknown_param: "SHOULD_REJECT", + }), + /unrecognized|unknown|not permitted/i, + "an undeclared parameter must raise, not be silently dropped" + ); +}); + +test("the rejection names the bad key and the accepted parameters", () => { + assert.throws( + () => parseToolArgs("search_notices", { query: "x", council_district: 10 }), + (err) => { + assert.match(err.message, /council_district/); + assert.match(err.message, /\bquery\b/); + assert.match(err.message, /\blimit\b/); + return true; + } + ); +}); + +test("every tool rejects an undeclared parameter", () => { + const valid = { + search_notices: { query: "x" }, + get_notices_by_agency: { agency_name: "DCAS" }, + get_notices_by_type: { notice_type: "Solicitation" }, + get_procurement_notices: {}, + get_public_hearings: {}, + get_open_solicitations: {}, + get_notices_by_date_range: { start_date: "2026-01-01", end_date: "2026-01-31" }, + }; + for (const [name, args] of Object.entries(valid)) { + assert.throws( + () => parseToolArgs(name, { ...args, bogus_unknown_param: "x" }), + /unrecognized|unknown|not permitted/i, + `${name} accepted an undeclared parameter` + ); + } +}); + +// Regression guard: do not over-correct into rejecting valid calls. +test("valid calls still parse", () => { + assert.deepEqual(parseToolArgs("get_open_solicitations", { limit: 1 }), { limit: 1 }); + assert.deepEqual(parseToolArgs("get_open_solicitations", undefined), {}); + assert.deepEqual(parseToolArgs("search_notices", { query: "rfp", limit: 5 }), { + query: "rfp", + limit: 5, + }); + assert.deepEqual( + parseToolArgs("get_notices_by_date_range", { + start_date: "2026-01-01", + end_date: "2026-01-31", + }), + { start_date: "2026-01-01", end_date: "2026-01-31" } + ); +}); + +// Ordinary validation errors must keep their own messages. +test("a bad value for a declared parameter still reports normally", () => { + assert.throws( + () => parseToolArgs("get_notices_by_date_range", { start_date: "Jan 1", end_date: "2026-01-31" }), + /YYYY-MM-DD/ + ); +}); From b25a1895d6aa20ed4752e1f5766ab142e36512a9 Mon Sep 17 00:00:00 2001 From: Noel Hidalgo Date: Tue, 21 Jul 2026 12:21:04 -0400 Subject: [PATCH 2/3] fix: reject unknown tool parameters instead of silently dropping them 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 --- CHANGELOG.md | 9 ++++++ src/tools.ts | 82 ++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e98eb..144f53b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Reject unknown tool parameters instead of silently dropping them. Every + tool's advertised `inputSchema` now sets `additionalProperties: false`, and + every argument schema is `zod.strict()`, so an invented filter raises an + error naming the bad key and the tool's accepted parameters rather than + returning unfiltered results for a different question ([#10]) + ## [1.0.2] - 2026-07-06 ### Fixed @@ -65,3 +73,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#4]: https://github.com/BetaNYC/nyc-record-mcp/pull/4 [#6]: https://github.com/BetaNYC/nyc-record-mcp/pull/6 [#7]: https://github.com/BetaNYC/nyc-record-mcp/pull/7 +[#10]: https://github.com/BetaNYC/nyc-record-mcp/issues/10 diff --git a/src/tools.ts b/src/tools.ts index c6cbc46..5514658 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -19,6 +19,7 @@ export const TOOLS: Tool[] = [ limit: { type: "number", description: "Max results (default 25, max 100)" }, }, required: ["query"], + additionalProperties: false, }, }, { @@ -32,6 +33,7 @@ export const TOOLS: Tool[] = [ limit: { type: "number", description: "Max results (default 25, max 100)" }, }, required: ["agency_name"], + additionalProperties: false, }, }, { @@ -49,6 +51,7 @@ export const TOOLS: Tool[] = [ limit: { type: "number", description: "Max results (default 25, max 100)" }, }, required: ["notice_type"], + additionalProperties: false, }, }, { @@ -60,6 +63,7 @@ export const TOOLS: Tool[] = [ properties: { limit: { type: "number", description: "Max results (default 25, max 100)" }, }, + additionalProperties: false, }, }, { @@ -71,6 +75,7 @@ export const TOOLS: Tool[] = [ properties: { limit: { type: "number", description: "Max results (default 25, max 100)" }, }, + additionalProperties: false, }, }, { @@ -82,6 +87,7 @@ export const TOOLS: Tool[] = [ properties: { limit: { type: "number", description: "Max results (default 25, max 100)" }, }, + additionalProperties: false, }, }, { @@ -95,39 +101,71 @@ export const TOOLS: Tool[] = [ limit: { type: "number", description: "Max results (default 50, max 200)" }, }, required: ["start_date", "end_date"], + additionalProperties: false, }, }, ]; -/** Argument schemas, one per tool name in TOOLS. */ +/** + * Argument schemas, one per tool name in TOOLS. + * + * Every schema is `.strict()`. zod strips unknown keys by default, which meant + * an invented filter (`council_district`, `vendor`, …) was silently discarded + * and the tool answered a different question with real, plausible data — see + * issue #10. Strict turns that into a loud failure. + */ export const SCHEMAS = { - search_notices: z.object({ - query: z.string(), - limit: z.number().max(100).optional(), - }), - get_notices_by_agency: z.object({ - agency_name: z.string(), - limit: z.number().max(100).optional(), - }), - get_notices_by_type: z.object({ - notice_type: z.enum(NOTICE_TYPES), - limit: z.number().max(100).optional(), - }), - get_procurement_notices: z.object({ limit: z.number().max(100).optional() }), - get_public_hearings: z.object({ limit: z.number().max(100).optional() }), - get_open_solicitations: z.object({ limit: z.number().max(100).optional() }), - get_notices_by_date_range: z.object({ - start_date: isoDate, - end_date: isoDate, - limit: z.number().max(200).optional(), - }), + search_notices: z + .object({ query: z.string(), limit: z.number().max(100).optional() }) + .strict(), + get_notices_by_agency: z + .object({ agency_name: z.string(), limit: z.number().max(100).optional() }) + .strict(), + get_notices_by_type: z + .object({ + notice_type: z.enum(NOTICE_TYPES), + limit: z.number().max(100).optional(), + }) + .strict(), + get_procurement_notices: z + .object({ limit: z.number().max(100).optional() }) + .strict(), + get_public_hearings: z.object({ limit: z.number().max(100).optional() }).strict(), + get_open_solicitations: z + .object({ limit: z.number().max(100).optional() }) + .strict(), + get_notices_by_date_range: z + .object({ + start_date: isoDate, + end_date: isoDate, + limit: z.number().max(200).optional(), + }) + .strict(), } satisfies Record; export type ToolName = keyof typeof SCHEMAS; +/** + * Parse a tool call's arguments, rejecting any parameter the tool does not + * declare. zod's own strict message names the offending key but offers no + * alternative, so the accepted names are read back off the advertised schema. + */ export function parseToolArgs( name: K, args: unknown ): z.infer<(typeof SCHEMAS)[K]> { - return SCHEMAS[name].parse(args ?? {}); + const result = SCHEMAS[name].safeParse(args ?? {}); + if (result.success) return result.data; + + const unknown = result.error.issues.find((i) => i.code === "unrecognized_keys"); + if (unknown) { + const accepted = Object.keys(TOOLS.find((t) => t.name === name)?.inputSchema.properties ?? {}); + throw new Error( + `${name} received unrecognized parameter(s): ${unknown.keys.join(", ")}. ` + + `It accepts only: ${accepted.join(", ")}. The call was rejected rather ` + + `than run without that filter, because dropping it silently would ` + + `return results for a different question.` + ); + } + throw result.error; } From 3c828e0d508ccc0b77772f9bc2e2cf7dd1a7e331 Mon Sep 17 00:00:00 2001 From: Noel Hidalgo Date: Tue, 21 Jul 2026 13:03:09 -0400 Subject: [PATCH 3/3] chore: release 1.1.0 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 --- CHANGELOG.md | 2 +- package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 144f53b..156448b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [1.1.0] - unreleased ### Fixed diff --git a/package-lock.json b/package-lock.json index 8d8ed25..58600b2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@betanyc/nyc-record-mcp", - "version": "1.0.2", + "version": "1.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@betanyc/nyc-record-mcp", - "version": "1.0.2", + "version": "1.1.0", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.0.0", diff --git a/package.json b/package.json index 3b2422f..80a2618 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@betanyc/nyc-record-mcp", - "version": "1.0.2", + "version": "1.1.0", "description": "MCP server for NYC City Record notices via NYC Open Data", "keywords": [ "mcp",