Skip to content

fix: parse JSON-string MCP tool arguments before type check #255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edelauna merged 2 commits into from
Jun 6, 2026
Merged

fix: parse JSON-string MCP tool arguments before type check #255

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f9c65f
fix: parse JSON-string MCP tool arguments before type check
pajitosingh May 22, 2026
c92ab3f
test: add coverage for JSON-string MCP tool argument parsing
pajitosingh Jun 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/core/tools/UseMcpToolTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,14 @@ export class UseMcpToolTool extends BaseTool<"use_mcp_tool"> {
return { isValid: false }
}

// Native-only: arguments are already a structured object.
// Some LLMs emit arguments as JSON-encoded strings rather than objects.
// Parse them early so the type check below sees the unwrapped object.
if (typeof params.arguments === "string") {
try {
params.arguments = JSON.parse(params.arguments)
} catch {}
}

@edelauna edelauna May 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a test in useMcpToolTool.spec.ts that passes nativeArgs.arguments as a JSON-encoded string (e.g. '{"headless": true}' as unknown as Record<string, unknown>) and verifies callTool receives the parsed object? This is the primary behavior change and currently has no regression coverage.

@pajitosingh pajitosingh Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edelauna Done! Added the test in useMcpToolTool.spec.ts — the new test "should parse JSON-string arguments and pass parsed object to callTool" passes nativeArgs.arguments as the JSON string '{"headless": true}' and verifies via callToolMock that callTool receives the parsed object { headless: true }, not the raw string.

The test has been pushed to this PR branch (commit c92ab3f). Ready for re-review.


let parsedArguments: Record<string, unknown> | undefined
if (params.arguments !== undefined) {
if (typeof params.arguments !== "object" || params.arguments === null || Array.isArray(params.arguments)) {
Expand Down
Loading