refactor(connection): dedupe fetch-tools' tool mapping, fix STDIO output-schema leniency - #5555
Open
pedrofrxncx wants to merge 1 commit into
Open
refactor(connection): dedupe fetch-tools' tool mapping, fix STDIO output-schema leniency#5555pedrofrxncx wants to merge 1 commit into
pedrofrxncx wants to merge 1 commit into
Conversation
…put-schema leniency Reduction: HTTP/SSE/STDIO each hand-rolled the identical MCP-tool-to-ToolDefinition mapping (3 near-identical blocks) — collapsed into one mapListedTools() helper. While unifying it, fixed a real inconsistency it exposed: HTTP/SSE relaxed outputSchema with additionalProperties: true, but STDIO stored it verbatim. MCP clients re-validate tool structuredContent with Ajv (additionalProperties: false by default per apps/api/src/tools/registry/schema.ts's documented pattern), so a STDIO-backed tool returning any field its outputSchema doesn't model gets rejected with -32602 "Structured content does not match the tool's output schema" — a failure HTTP/SSE tools don't hit for the same shape. Net: -38/+24 in fetch-tools.ts, plus a new unit test for the extracted helper.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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
1 participant
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.
Source: reduction + bug found while auditing
apps/api/src/tools/connection/fetch-tools.tsfor hardening issues (SSRF is already covered by open PR #5516 — not duplicated here).What/why: The HTTP, SSE, and STDIO tool-fetch branches each hand-rolled an identical block mapping MCP-listed tools to our stored
ToolDefinitionshape. Collapsed the three into onemapListedTools()helper (net -38/+24 in the file). Unifying it surfaced a real inconsistency: HTTP and SSE relaxedoutputSchemawithadditionalProperties: true, but STDIO stored it verbatim.Failure scenario / regression:
apps/api/src/tools/registry/schema.tsdocuments why this matters — MCP clients re-validate a tool'sstructuredContentwith Ajv, which enforcesadditionalProperties: falseby default and rejects extra fields with-32602: Structured content does not match the tool's output schema. A STDIO-backed dev connection (local-mode only) whose tool returns any field not modeled in its declaredoutputSchemawould hit that rejection, while the identical HTTP/SSE tool would not — same tool shape, transport-dependent breakage. Now all three transports get the same leniency.Verification: added
apps/api/src/tools/connection/fetch-tools.test.ts, a unit test for the extracted puremapListedTools()helper covering: empty list → null, outputSchema getsadditionalProperties: truemerged in, and no outputSchema stays undefined. Reviewer can run:Locally ran:
bun run fmt,cd apps/api && bunx tsc --noEmit(clean), the targeted test above (3 pass), andbunx oxlinton both changed files (0 warnings/errors). Full CI validates the rest.Summary by cubic
Deduped MCP tool-to-ToolDefinition mapping in
fetch-toolsand alignedoutputSchemaleniency across HTTP, SSE, and STDIO. Prevents STDIO-only Ajv revalidation errors when tools return extra fields.Bug Fixes
outputSchemawithadditionalProperties: true, matching HTTP/SSE and avoiding-32602rejections for extra fields.Refactors
mapListedTools()and reused it in all transports to remove three near-identical mappings.apps/api/src/tools/connection/fetch-tools.test.tsto cover empty lists, schema leniency, and absentoutputSchema.Written for commit ee51c35. Summary will update on new commits.