Conversation
The `documents` field of the `rerank-documents` MCP tool used a `z.union([...])`, which emits `anyOf` in the generated JSON Schema. Claude tool schemas reject composition keywords (anyOf/oneOf/allOf), so the tool failed to register — the same class of bug fixed for `upsert-records` in #89. Replace the union with `z.any().refine(isDocumentSet, ...)`, mirroring the #89 approach. Runtime validation is unchanged: it still accepts an array of text documents or an array of string-valued records, and still rejects non-arrays, mixed arrays, and records with non-string values. Add regression coverage that asserts no composition keywords appear in the generated input schema, plus runtime accept/reject cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#89 and #93 each fixed one MCP tool whose schema emitted `anyOf` (which Claude tool schemas reject), and each shipped a per-tool regression test. That approach is whack-a-mole: the next `z.union(...)` reintroduces the bug silently until someone remembers to add another per-tool assertion, and the per-tool tests converted Zod via `z.toJSONSchema` rather than the path the MCP SDK actually uses. Replace them with a single guard that boots the real server and inspects every tool's `inputSchema` exactly as a client receives it over `tools/list`. This covers all current and future tools automatically and has no fidelity gap. Remove the now-redundant per-tool composition tests from upsert-records and rerank-documents, keeping their runtime validation coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0046eaf. Configure here.
|
|
||
| // Sanity check: fail loudly if tool discovery is broken, so a green run | ||
| // can't mean "no tools were checked". | ||
| expect(tools.length).toBeGreaterThan(0); |
There was a problem hiding this comment.
Guard skips database tools silently
Medium Severity
The sanity check only asserts tools.length > 0, but search-docs always registers even when PINECONE_API_KEY is missing, so the guard can pass while skipping every database tool. That undercuts the stated goal of failing loudly when nothing meaningful was checked, and matches the exact false-green risk this test was meant to prevent.
Triggered by project rule: Code Review Guidance
Reviewed by Cursor Bugbot for commit 0046eaf. Configure here.
| .refine(isDocumentSet, { | ||
| message: | ||
| 'Documents must be an array of text documents (strings) or an array of records with string values.', | ||
| }) |
There was a problem hiding this comment.
Documents schema drops array type
Medium Severity
DOCUMENTS_SCHEMA is built on z.any().refine(...), so the emitted JSON Schema loses type: array and becomes unconstrained. Clients like Claude then get no structural hint that documents must be an array, which increases invalid tool calls even though an array base schema would avoid anyOf while preserving that constraint.
Reviewed by Cursor Bugbot for commit 0046eaf. Configure here.
2 participants
Overview
Debt-reduction follow-up to #89 and #93. Both of those PRs fixed a single MCP tool whose generated JSON Schema emitted
anyOf(which Claude tool schemas reject), and each shipped its own per-tool regression test with a copy-pastedfindSchemaCompositionKeywordshelper.That pattern is whack-a-mole:
z.union(...)added to any tool reintroduces the bug silently until someone remembers to write yet another per-tool assertion.z.toJSONSchema(full Zod,io: 'output'), not the path the MCP SDK actually uses at registration (zod/v4-mini'stoJSONSchemawithio: 'input') — so they could pass while the shipped tool still breaks.Change
Add a single guard (
src/tools/tool-schemas.test.ts) that boots the real server over an in-memory transport and inspects every tool'sinputSchemaexactly as a client receives it viatools/list. This:I verified it has teeth: temporarily reintroducing a
z.unionmakes it fail with{ name: 'rerank-documents', keywords: ['anyOf'] }.The now-redundant per-tool composition tests (and their duplicated helpers) are removed from
upsert-records.test.tsandrerank-documents.test.ts; their runtime validation coverage is kept.The guard only passes once both tool fixes are in
main, so this branch includes them. Until #89 and #93 merge, this PR's diff will show their commits too. The only new change to review here is the final commit (test: guard all tool schemas…). Merge order: #89 → #93 → this. It will rebase cleanly ontomainonce they land.Type
Validation
npm run lintnpm test(97 passing; guard verified to fail on a reintroduced union)npm run build🤖 Generated with Claude Code
Note
Low Risk
Test infrastructure plus schema-shape refactors that preserve validation semantics; no production API or auth changes.
Overview
Adds
src/tools/tool-schemas.test.ts, which starts the real MCP server (in-memory transport) and asserts every registered tool’sinputSchemafromtools/listcontains noanyOf,oneOf, orallOf—the keywords Claude rejects—so new tools are covered without per-tool copy-paste tests.rerank-documentsandupsert-recordsreplacez.union(...)withz.any().refine(...)forDOCUMENTS_SCHEMAandFIELD_VALUE_SCHEMAso runtime validation stays the same but emitted JSON Schema avoids unions. Per-tool tests drop duplicated composition-keyword checks and keep runtime Zod parse cases, with comments pointing at the central guard.Reviewed by Cursor Bugbot for commit 0046eaf. Bugbot is set up for automated code reviews on this repo. Configure here.