fix(skill-mcp): handle pre-parsed object arguments in parseArguments #675
Conversation
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
Some LLM tool frameworks pre-parse JSON string arguments into objects before passing them to the tool. This causes parseArguments to fail with 'Invalid arguments JSON' error when it receives [object Object]. This fix adds a type check to handle both: - String arguments (original behavior, parsed via JSON.parse) - Object arguments (already parsed by framework, returned directly) Fixes issue where skill_mcp fails with Playwright and other MCP tools when arguments are passed as JSON objects by the LLM framework.
Contributor
|
All contributors have signed the CLA. Thank you! ✅ |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 3/5
- Runtime and type expectations diverge in
src/tools/skill-mcp/types.ts, so widened object inputs will still be rejected and users can’t successfully call the tool with objects. - Impact is direct: the API now advertises support for objects but the schema remains string-only, creating a regression for callers who trust the types.
- Pay close attention to
src/tools/skill-mcp/types.ts- align the runtime schema with the widened type before merging.
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/types.ts">
<violation number="1" location="src/tools/skill-mcp/types.ts:6">
P1: Type widened to allow object arguments but runtime tool schema still only accepts strings, so object inputs will be rejected before execution.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| resource_name?: string | ||
| prompt_name?: string | ||
| arguments?: string | ||
| arguments?: string | Record<string, unknown> |
There was a problem hiding this comment.
P1: Type widened to allow object arguments but runtime tool schema still only accepts strings, so object inputs will be rejected before execution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/skill-mcp/types.ts, line 6:
<comment>Type widened to allow object arguments but runtime tool schema still only accepts strings, so object inputs will be rejected before execution.</comment>
<file context>
@@ -3,6 +3,6 @@ export interface SkillMcpArgs {
resource_name?: string
prompt_name?: string
- arguments?: string
+ arguments?: string | Record<string, unknown>
grep?: string
}
</file context>
renekris
added a commit
to renekris/oh-my-glm
that referenced
this pull request
Jan 10, 2026
renekris
pushed a commit
to renekris/oh-my-glm
that referenced
this pull request
Jan 11, 2026
kdcokenny
pushed a commit
that referenced
this pull request
Jan 13, 2026
kdcokenny
pushed a commit
that referenced
this pull request
Jan 13, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Fixes an issue where
skill_mcpfails withInvalid arguments JSONerror when OpenCode's tool invocation system passes arguments as pre-parsed objects instead of JSON strings.Problem
OpenCode's plugin tool system can pass arguments as already-parsed JavaScript objects rather than raw JSON strings. When this happens, the
parseArgumentsfunction receives an object instead of a string, causing:This breaks
skill_mcpusage with Playwright and other MCP tools.Solution
Added a type check in
parseArgumentsto handle both cases:JSON.parse()Changes
src/tools/skill-mcp/tools.ts: Added object type check inparseArgumentssrc/tools/skill-mcp/types.ts: UpdatedSkillMcpArgs.argumentstype to accept both string and objectTesting
Tested with Playwright MCP browser automation -
browser_navigate,browser_evaluate, and other tools now work correctly when arguments are passed as objects by OpenCode.