fix: preserve original JSON schema in listTools operation without Zod… #141
Conversation
WalkthroughThis change refactors the MCP Client's Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant McpClient
participant MCP_Server
User->>McpClient: listTools()
McpClient->>MCP_Server: fetch tools with schemas
MCP_Server-->>McpClient: return tools with inputSchema
McpClient->>McpClient: log received schemas (debug)
McpClient->>McpClient: normalize and validate tools
McpClient->>McpClient: log returned schemas (debug)
McpClient-->>User: return tools with original inputSchema
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
nodes/McpClient/__tests__/listTools.schema.test.ts (1)
6-67:⚠️ Potential issueTest duplicates an identical suite elsewhere
There is an identical test file
nodes/McpClient/listTools.schema.test.ts.
Running both doubles execution time and provides no additional coverage.
Delete one or move common helpers to a shared util to avoid redundancy.
♻️ Duplicate comments (1)
nodes/McpClient/listTools.schema.test.ts (1)
6-71: Same concern as above – duplicated and indirect testsThis file is an exact copy of
__tests__/listTools.schema.test.ts.
Keep a single source-of-truth test and remove the copy to prevent maintenance drift.
🧹 Nitpick comments (5)
schemabug.md (2)
38-41: Avoid unnecessary bold-hyphen coupling“deeply-nested” does not need a hyphen because “deeply” already ends with -ly.
-**Preserve the original JSON Schema from the MCP server.** +**Preserve the original JSON Schema from the MCP server.**
146-147: Tone – add “please” for reviewer friendlinessThe sentence “If you need a PR template or further test scaffolding, let me know!” is flagged by LanguageTool.
Consider:-If you need a PR template or further test scaffolding, let me know! +If you need a PR template or further test scaffolding, please let me know!🧰 Tools
🪛 LanguageTool
[style] ~147-~147: Consider using polite language here.
Context: ...R template or further test scaffolding, let me know!(INSERT_PLEASE)
nodes/McpClient/McpClient.node.ts (2)
425-446: Large JSON pretty-prints may explode n8n logs
JSON.stringify(rawTools, null, 2)and the similar call foroutputToolsstringify full nested schemas.
On large tool catalogs this can flood n8n’s log buffer and degrade performance.- this.logger.debug(`[MCP][listTools] Received tools from server: ${JSON.stringify(rawTools, null, 2)}`); + this.logger.debug(() => `[MCP][listTools] Received tools from server (count=${tools?.length ?? 0})`);Most loggers (including n8n’s) accept a lazy-evaluated function/string. Consider logging only length or first N chars.
441-445: Variable reuse reduces readabilityThe alias
toolsis reused for the mapped array:const tools = …; … const outputTools = tools.map(…)
tools→rawToolArrayandoutputTools→normalizedToolswould read clearer.docs/PR_TEMPLATE_mcp-schema-fix.md (1)
22-25: Missing determiner“Downstream consumers will now receive full, correct schema” → “the full, correct schema”.
-Downstream consumers will now receive the full, correct schema as provided by the MCP server. +Downstream consumers will now receive the full, correct schema as provided by the MCP server.🧰 Tools
🪛 LanguageTool
[uncategorized] ~23-~23: You might be missing the article “the” here.
Context: ...ed. ## Impact - No breaking changes to API or interface. - Downstream consumers wi...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/PR_TEMPLATE_mcp-schema-fix.md(1 hunks)nodes/McpClient/McpClient.node.ts(2 hunks)nodes/McpClient/__tests__/listTools.schema.test.ts(1 hunks)nodes/McpClient/listTools.schema.test.ts(1 hunks)schemabug.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/PR_TEMPLATE_mcp-schema-fix.md
[uncategorized] ~23-~23: You might be missing the article “the” here.
Context: ...ed. ## Impact - No breaking changes to API or interface. - Downstream consumers wi...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
schemabug.md
[uncategorized] ~13-~13: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...aude Desktop) correctly fetch the full, deeply-nested schema from the same MCP server. ## 3....
(HYPHENATED_LY_ADVERB_ADJECTIVE)
[style] ~147-~147: Consider using polite language here.
Context: ...R template or further test scaffolding, let me know!
(INSERT_PLEASE)
🔇 Additional comments (1)
nodes/McpClient/McpClient.node.ts (1)
428-435: Defensive parsing could mis-detect object-shaped responses
typeof rawTools?.tools === 'object' && rawTools.tools !== null ? Object.values(rawTools.tools) : []
will treat a plain object with arbitrary keys as list of tools, losing the key names.If the SDK guarantees
rawTools.toolsis either array or object keyed byname, that’s fine; otherwise you risk silently discarding data. Add an explicit fallback / validation.
|
Hi @havebeenfitz, @iamfiscus. This PR solves a big issue I'm currently facing, as reported in #129. Do you think this can be merged/fixed? Appreciate it. Maycon |
3 participants
… conversion
Description
This PR fixes a critical bug in the MCP Client node's
listToolsoperation, where tool parameter schemas from the MCP server were being truncated and losing fidelity. The root cause was a lossy conversion from JSON Schema to Zod and back, resulting in the loss of nested object structures, enums, constraints, and defaults. The fix removes this conversion and passes the server's JSON Schema through unchanged, preserving all schema details for downstream consumers and UI.Related Issue
Closes #129
Type of Change
How Has This Been Tested?
nodes/McpClient/__tests__/listTools.schema.test.ts) that simulates a complex, deeply-nested tool schema from the MCP server and verifies that the schema is returned without modification or truncation.npm testto ensure all existing and new tests pass.listToolsoperation to confirm full fidelity of the schema.Checklist
schemabug.mdand PR template)Release Notes
listTools, enabling correct UI generation, validation, and downstream usage.Screenshots (if applicable)
N/A (schema validation and passthrough tested via Jest)
Summary by CodeRabbit