fix(skill-mcp): handle arguments passed as object instead of JSON string

[sngweizhi]

Summary

• Fix skill_mcp tool failing with "Invalid arguments JSON" error when LLMs pass arguments as an object instead of a JSON string
• Update parseArguments() to handle both object and string inputs
• Add test case for object argument handling

Problem

When invoking skill_mcp tools (like Playwright MCP), some LLM tool-calling interfaces pass the arguments parameter as a JavaScript object instead of a JSON string. This caused failures like:

Error: Invalid arguments JSON: JSON Parse error: Unexpected identifier "object"

Expected a valid JSON object, e.g.: '{"key": "value"}'
Received: [object Object]

The root cause: when JSON.parse() is called on an object, the object is first coerced to a string via .toString(), resulting in "[object Object]" which is invalid JSON.

Solution

Update parseArguments() to check if the input is already an object and return it directly, only falling back to JSON.parse() for string inputs.

Test Plan

• Added test case: "accepts arguments as object when LLM passes object directly"
• All 622 existing tests pass
• Manually verified Playwright MCP works after the fix

---

Summary by cubic

Fixes skill_mcp tools failing with “Invalid arguments JSON” when LLMs pass arguments as objects. parseArguments now accepts both object and JSON string, restoring compatibility with Playwright MCP and similar tools.

• Bug Fixes
  • Updated parseArguments to return objects directly and only JSON.parse strings.
  • Expanded arguments type to string | Record<string, unknown>.
  • Added a test for object arguments; manual check confirms Playwright MCP works.

^{Written for commit 83839cd. Summary will update on new commits.}

[github-actions]

All contributors have signed the CLA. Thank you! ✅
_{Posted by the CLA Assistant Lite bot.}

[greptile-apps]

Greptile Summary

Fixed skill_mcp tool failing when LLMs pass arguments as object instead of JSON string by enhancing parseArguments() to handle both input types.

• Updated parseArguments() in tools.ts:72-76 to check if input is already an object before attempting JSON parsing
• Modified SkillMcpArgs.arguments type from string to string | Record<string, unknown> to reflect both input formats
• Added test case verifying object argument handling with proper mocking and BDD structure
• Schema Mismatch: Tool schema at line 117 still declares arguments as tool.schema.string() but should be updated to tool.schema.any() to match the implementation

Confidence Score: 4/5

• Safe to merge with one schema mismatch that should be fixed
• The implementation correctly handles both string and object inputs, includes proper test coverage, and follows TDD workflow. However, the tool schema definition still declares arguments as string-only, creating a type mismatch between the schema and the actual implementation
• src/tools/skill-mcp/tools.ts - update schema definition at line 117

Important Files Changed

Filename	Overview
src/tools/skill-mcp/types.ts	Updated arguments field to accept both string and object types to handle LLM variations
src/tools/skill-mcp/tools.ts	Enhanced parseArguments() to check if input is already an object before parsing JSON
src/tools/skill-mcp/tools.test.ts	Added test case verifying object argument handling with proper BDD structure

Sequence Diagram

sequenceDiagram
    participant LLM
    participant SkillMcpTool
    participant parseArguments
    participant Manager
    
    Note over LLM: LLM passes arguments as object<br/>instead of JSON string
    
    LLM->>SkillMcpTool: execute({mcp_name, tool_name, arguments: {action: "list"}})
    SkillMcpTool->>parseArguments: parseArguments({action: "list"})
    
    alt arguments is object
        parseArguments->>parseArguments: typeof argsJson === "object"
        parseArguments-->>SkillMcpTool: return argsJson (as-is)
    else arguments is string
        parseArguments->>parseArguments: JSON.parse(argsJson)
        parseArguments-->>SkillMcpTool: return parsed object
    end
    
    SkillMcpTool->>Manager: callTool(info, context, tool_name, parsedArgs)
    Manager-->>SkillMcpTool: result
    SkillMcpTool-->>LLM: formatted output

Loading

[greptile-apps]

Additional Comments (1)

1. src/tools/skill-mcp/tools.ts, line 117 (link)

   logic: schema definition still declares arguments as string-only, but the type definition and implementation now accept objects. update the schema to match:

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[cubic-dev-ai]

No issues found across 3 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

[sngweizhi]

I have read the CLA Document and I hereby sign the CLA

[code-yeongyu]

However, the tool schema definition still declares arguments as string-only, creating a type mismatch between the schema and the actual implementation

@sisyphus-dev-ai hey please fix this, commit and push on here

[sisyphus-dev-ai]

✅ Schema Mismatch Fixed

I've fixed the schema mismatch issue identified by Greptile. The changes include:

What Was Fixed

• Updated tool schema at from tool.schema.string() to tool.schema.any()
• Updated description to reflect that arguments can now be either a JSON string or an object

Changes Made

- arguments: tool.schema.string().optional().describe("JSON string of arguments"),
+ arguments: tool.schema.any().optional().describe("JSON string or object of arguments"),

Verification

✅ All tests passing (12 tests, 16 expect() calls)
✅ Type definitions match implementation (string | Record<string, unknown>)
✅ Schema now correctly reflects the runtime behavior

The schema now properly accepts both string and object arguments, resolving the type mismatch between the schema definition and the actual implementation.

Commit: 83839cd

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/tools/skill-mcp/tools.ts">

<violation number="1" location="src/tools/skill-mcp/tools.ts:117">
P2: Using `any()` is overly permissive - it allows arrays and primitives to bypass schema validation. Since `typeof []` returns `&quot;object&quot;`, arrays would pass through `parseArguments` and be incorrectly cast to `Record&lt;string, unknown&gt;`. Consider using a union type schema if available (e.g., `tool.schema.union([tool.schema.string(), tool.schema.record(...)])`) or add an `!Array.isArray()` check in `parseArguments`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: Using any() is overly permissive - it allows arrays and primitives to bypass schema validation. Since typeof [] returns "object", arrays would pass through parseArguments and be incorrectly cast to Record<string, unknown>. Consider using a union type schema if available (e.g., tool.schema.union([tool.schema.string(), tool.schema.record(...)])) or add an !Array.isArray() check in parseArguments.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/skill-mcp/tools.ts, line 117:

<comment>Using `any()` is overly permissive - it allows arrays and primitives to bypass schema validation. Since `typeof []` returns `&quot;object&quot;`, arrays would pass through `parseArguments` and be incorrectly cast to `Record&lt;string, unknown&gt;`. Consider using a union type schema if available (e.g., `tool.schema.union([tool.schema.string(), tool.schema.record(...)])`) or add an `!Array.isArray()` check in `parseArguments`.</comment>

<file context>
@@ -114,7 +114,7 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition
       resource_name: tool.schema.string().optional().describe(&quot;MCP resource URI to read&quot;),
       prompt_name: tool.schema.string().optional().describe(&quot;MCP prompt to get&quot;),
-      arguments: tool.schema.string().optional().describe(&quot;JSON string of arguments&quot;),
+      arguments: tool.schema.any().optional().describe(&quot;JSON string or object of arguments&quot;),
       grep: tool.schema.string().optional().describe(&quot;Regex pattern to filter output lines (only matching lines returned)&quot;),
     },
</file context>