fix(codemode): retry an MCP call once with decoded JSON-string args when the server rejects a string for a structure - #38
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/codemode/src/tool-schema.ts">
<violation number="1" location="packages/codemode/src/tool-schema.ts:316">
P2: When an unrelated tool failure contains this validation phrase, the regex triggers a second `tool.run` invocation and can duplicate side effects. Require a structured validation marker or verify the rejected property before retrying.</violation>
<violation number="2" location="packages/codemode/src/tool-schema.ts:336">
P2: When only one field is rejected, this line also converts every other JSON-looking string field, so valid string data such as `toolName: "[]"` becomes an array on retry. Restrict decoding to the rejected property or preserve unrelated string fields.</violation>
</file>
<file name="packages/codemode/src/tool-runtime.ts">
<violation number="1" location="packages/codemode/src/tool-runtime.ts:791">
P2: When a rejected JSON string encodes deeply nested data or blocked properties, the retry bypasses the runtime's `copyIn` boundary and passes it directly to the host tool. Copy the decoded value through `copyIn` before the retry so retried inputs receive the same depth and plain-data validation as normal arguments.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * in both the zod issue form (`"expected": "object", "received": "string"`) and the message form | ||
| * (`Expected object, received string`). | ||
| */ | ||
| const STRING_FOR_STRUCTURE_REJECTION = /expected"?:?\s*"?(?:object|array)"?,?\s*"?received"?:?\s*"?string/i |
There was a problem hiding this comment.
P2: When an unrelated tool failure contains this validation phrase, the regex triggers a second tool.run invocation and can duplicate side effects. Require a structured validation marker or verify the rejected property before retrying.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/codemode/src/tool-schema.ts, line 316:
<comment>When an unrelated tool failure contains this validation phrase, the regex triggers a second `tool.run` invocation and can duplicate side effects. Require a structured validation marker or verify the rejected property before retrying.</comment>
<file context>
@@ -286,18 +286,61 @@ export const outputTypeScript = <R>(definition: Definition<R>, pretty = false):
+ * in both the zod issue form (`"expected": "object", "received": "string"`) and the message form
+ * (`Expected object, received string`).
+ */
+const STRING_FOR_STRUCTURE_REJECTION = /expected"?:?\s*"?(?:object|array)"?,?\s*"?received"?:?\s*"?string/i
+
+const rejectionText = (error: unknown, depth = 0): string => {
</file context>
| let changed = false | ||
| const decoded = Object.fromEntries( | ||
| Object.entries(value).map(([key, entry]) => { | ||
| const next = parseJsonStructure(entry, "structure") |
There was a problem hiding this comment.
P2: When only one field is rejected, this line also converts every other JSON-looking string field, so valid string data such as toolName: "[]" becomes an array on retry. Restrict decoding to the rejected property or preserve unrelated string fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/codemode/src/tool-schema.ts, line 336:
<comment>When only one field is rejected, this line also converts every other JSON-looking string field, so valid string data such as `toolName: "[]"` becomes an array on retry. Restrict decoding to the rejected property or preserve unrelated string fields.</comment>
<file context>
@@ -286,18 +286,61 @@ export const outputTypeScript = <R>(definition: Definition<R>, pretty = false):
+ let changed = false
+ const decoded = Object.fromEntries(
+ Object.entries(value).map(([key, entry]) => {
+ const next = parseJsonStructure(entry, "structure")
+ if (next !== entry) changed = true
+ return [key, next]
</file context>
| Effect.catch((error: ToolError) => { | ||
| // The tool rejected a JSON string where it wanted a structure: retry once, decoded. | ||
| const retried = decodeInputOnRejection(describedInput, error) | ||
| return retried === undefined ? Effect.fail(error) : runHost(Effect.suspend(() => tool.run(retried))) |
There was a problem hiding this comment.
P2: When a rejected JSON string encodes deeply nested data or blocked properties, the retry bypasses the runtime's copyIn boundary and passes it directly to the host tool. Copy the decoded value through copyIn before the retry so retried inputs receive the same depth and plain-data validation as normal arguments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/codemode/src/tool-runtime.ts, line 791:
<comment>When a rejected JSON string encodes deeply nested data or blocked properties, the retry bypasses the runtime's `copyIn` boundary and passes it directly to the host tool. Copy the decoded value through `copyIn` before the retry so retried inputs receive the same depth and plain-data validation as normal arguments.</comment>
<file context>
@@ -783,7 +784,13 @@ export const make = <R>(
+ Effect.catch((error: ToolError) => {
+ // The tool rejected a JSON string where it wanted a structure: retry once, decoded.
+ const retried = decodeInputOnRejection(describedInput, error)
+ return retried === undefined ? Effect.fail(error) : runHost(Effect.suspend(() => tool.run(retried)))
+ }),
+ )
</file context>
| return retried === undefined ? Effect.fail(error) : runHost(Effect.suspend(() => tool.run(retried))) | |
| return retried === undefined | |
| ? Effect.fail(error) | |
| : runHost(Effect.suspend(() => tool.run(copyIn(retried, `Arguments for tool '${name}'`)))) |
5344a9b to
8bb342b
Compare
…hen the server rejects a string for a structure The schema-driven decode from #32 only fires when the declared property type is object or array. next-devtools-mcp advertises nextjs_call's args as type string while validating it as an object, so every args: "{}" call still fails with Invalid argument 'args' ... expected object, received string. decodeRejectedInput recognises that rejection text (zod issue and message forms) and re-parses every string property that encodes an object or array; invokeChildTool retries client.callTool once with the result, so the before hook and the permission ask run once and no non-MCP tool is affected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8bb342b to
f7ca366
Compare
The agent stops losing
executeturns toargs: "{}"on MCP tools whose advertised schema is wrong. Where #32 decoded a JSON string only when the schema declared an object or array, this retries the MCP call once, decoded, when the server itself rejects a string for a structure.Context
#32 shipped in v1.17.14-8 (andytown anomalyco#26456) and did not move the number. On production data since the bump, 100% of the
MCP error -32603: Invalid argument 'args': [ ... "expected": "object", "received": "string" ]failures (50-83/day, external users) come fromtools["next-devtools"].nextjs_call({ port, toolName, args: "{}" }), and they persist on sandboxes verified to runReplopencode v1.17.14-8.The cause is upstream of the decode: next-devtools-mcp 0.4.0 hand-rolls its zod-to-JSON-schema conversion and falls through on
ZodRecord, sotools/listadvertisesargsas{ "type": "string" }whileparseToolArgsvalidates it as an object. The model follows the advertised schema and is rejected. A decode keyed on the declared type can never fire here, and 0.4.0 is still the latest npm release. A schema-agnostic pre-decode was rejected: it would corrupt tools that legitimately take JSON in a string (write_filecontent, HTTP bodies).What changed
codemode/tool-schema.ts:decodeRejectedInput(input, error)matches the string-where-structure-expected rejection (expected object|array ... received string, zod issue or message form) and returns the input with every top-level string property that encodes an object or array parsed, orundefinedwhen the rejection is unrelated or nothing changes. The JSON-parse step is shared with the existing schema-drivendecodeJsonString, which now takes a predicate instead of re-checking the schema type.opencode/tool/code-mode.tsinvokeChildTool: the transport call is a localcallTool(args); on failure, one retry withdecodeRejectedInput's result, otherwise the original error rethrows. The retry wraps onlyclient.callTool, so thetool.execute.beforehook and the permission ask run once per child call, and OpenAPI/native CodeMode tools are untouched.What to scrutinize
The retry fires only after the server reported an input-validation rejection, so a tool that executed and failed for another reason is never re-invoked. The regex is deliberately narrow; a server that rejects with different wording keeps the old behaviour. All top-level JSON-string properties are re-parsed, not only the one the error names: server error formats differ too much to extract the key reliably, and a wrongly parsed sibling fails the server's validation again rather than executing. Each call to a mis-advertised tool still pays one failed round trip; teaching the catalog the corrected type after a successful retry is the deeper follow-up, deliberately left out of this PR.
Testing done
packages/opencodetest/tool/code-mode.test.ts: 38 pass, including the new retry test (watched failing first): the server sees"{}"then{}, the execute succeeds, and permission is asked once.packages/codemode: 265 pass, including a unit test ofdecodeRejectedInputcovering decode, no-op on non-JSON, and no-op on an unrelated error.bun run typecheckin both packages: clean. Prettier on changed files: clean.executeparts whosestate.errorstarts withMCP error -32603: Invalid argument 'args'on the production read replica (tracked by theexecute-tool-success-morning-checkroutine); expect 0 on fresh sandboxes within a day.🤖 Generated with Claude Code