diff --git a/packages/codemode/src/index.ts b/packages/codemode/src/index.ts index 92019825004b..799fc7a39667 100644 --- a/packages/codemode/src/index.ts +++ b/packages/codemode/src/index.ts @@ -2,3 +2,4 @@ export * as CodeMode from "./codemode.js" export * as Tool from "./tool.js" export * as OpenAPI from "./openapi/index.js" export { ToolError, toolError } from "./tool-error.js" +export { decodeRejectedInput } from "./tool-schema.js" diff --git a/packages/codemode/src/tool-schema.ts b/packages/codemode/src/tool-schema.ts index 5b8f3e2e75e2..42b17f960123 100644 --- a/packages/codemode/src/tool-schema.ts +++ b/packages/codemode/src/tool-schema.ts @@ -286,18 +286,43 @@ export const outputTypeScript = (definition: Definition, pretty = false): const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value) -/** Parses a JSON string standing in for a declared object/array (e.g. `args: "{}"`); every other value is untouched. */ -const decodeJsonString = (value: unknown, schema: JsonSchema | undefined): unknown => { - if (typeof value !== "string" || (schema?.type !== "object" && schema?.type !== "array")) return value +type Accept = (parsed: unknown) => boolean +const isStructure: Accept = (value) => isRecord(value) || Array.isArray(value) +const acceptsFor: Partial> = { object: isRecord, array: Array.isArray } + +/** Parses a JSON string whose value satisfies `accept`; anything else is untouched. */ +const parseJsonString = (value: unknown, accept: Accept): unknown => { + if (typeof value !== "string" || !/^\s*[[{]/.test(value)) return value try { const parsed: unknown = JSON.parse(value) - if (schema.type === "array" ? Array.isArray(parsed) : isRecord(parsed)) return parsed + if (accept(parsed)) return parsed } catch { // not JSON: keep the original string } return value } +/** Parses a JSON string standing in for a declared object/array (e.g. `args: "{}"`); every other value is untouched. */ +const decodeJsonString = (value: unknown, schema: JsonSchema | undefined): unknown => { + const accept = typeof schema?.type === "string" ? acceptsFor[schema.type] : undefined + return accept ? parseJsonString(value, accept) : value +} + +/** A tool's validation text for a JSON string where it wanted a structure, in zod's issue and message forms. */ +const REJECTS_STRING_FOR_STRUCTURE = /expected\W+(?:object|array)\W+received\W+string/i + +/** Re-parses every JSON-string property that encodes a structure once a tool has rejected one; undefined when unrelated or unchanged. */ +export const decodeRejectedInput = (input: unknown, error: unknown): Record | undefined => { + const message = error instanceof Error ? error.message : String(error) + if (!isRecord(input) || !REJECTS_STRING_FOR_STRUCTURE.test(message)) return undefined + let decoded: Record | undefined + for (const [key, value] of Object.entries(input)) { + const next = parseJsonString(value, isStructure) + if (next !== value) (decoded ??= { ...input })[key] = next + } + return decoded +} + /** * Decodes tool input before `run` is invoked. Effect Schemas validate (throwing on failure); JSON-Schema * inputs pass through unvalidated (render-only), after decoding JSON strings standing in for declared objects/arrays. diff --git a/packages/codemode/test/codemode.test.ts b/packages/codemode/test/codemode.test.ts index 773db85a94f4..e9ecab4b21b2 100644 --- a/packages/codemode/test/codemode.test.ts +++ b/packages/codemode/test/codemode.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test" import { Cause, Effect, Schema } from "effect" -import { CodeMode, Tool, toolError } from "../src/index.js" +import { CodeMode, Tool, decodeRejectedInput, toolError } from "../src/index.js" const run = (tool: Tool.Definition) => Effect.runPromise(CodeMode.make({ tools: { host: { call: tool } } }).execute("return await tools.host.call({})")) @@ -469,6 +469,13 @@ describe("CodeMode schema flexibility", () => { ]) }) + test("decodeRejectedInput re-parses JSON-string structures only after a string-for-structure rejection", () => { + const rejection = new Error("MCP error -32603: Invalid argument 'args': Expected object, received string") + expect(decodeRejectedInput({ args: "{}", toolName: "x" }, rejection)).toStrictEqual({ args: {}, toolName: "x" }) + expect(decodeRejectedInput({ args: "not json" }, rejection)).toBeUndefined() + expect(decodeRejectedInput({ args: "{}" }, new Error("MCP error -32001: Request timed out"))).toBeUndefined() + }) + test("renders JSON Schema outputs and $defs references", async () => { const lookup = Tool.make({ description: "Look up a user", diff --git a/packages/opencode/src/tool/code-mode.ts b/packages/opencode/src/tool/code-mode.ts index 332d4b43f150..931618c568d5 100644 --- a/packages/opencode/src/tool/code-mode.ts +++ b/packages/opencode/src/tool/code-mode.ts @@ -1,7 +1,7 @@ import * as Tool from "./tool" import { CallToolResultSchema, type CallToolResult } from "@modelcontextprotocol/sdk/types.js" import { Cause, Effect, Schema } from "effect" -import { CodeMode, Tool as SandboxTool, toolError } from "@opencode-ai/codemode" +import { CodeMode, Tool as SandboxTool, decodeRejectedInput, toolError } from "@opencode-ai/codemode" import { MCP } from "@/mcp" import { McpCatalog } from "@/mcp/catalog" import { Agent } from "@/agent/agent" @@ -146,9 +146,9 @@ const invokeChildTool = Effect.fn("CodeMode.invokeChildTool")(function* (input: const result: CallToolResult = yield* Effect.gen(function* () { yield* input.ctx.ask({ permission: input.entry.key, metadata: {}, patterns: ["*"], always: ["*"] }) // Deliberately mirrors McpCatalog.convertTool's transport call so the MCP service stays free of tool-loop concerns. - return yield* Effect.promise(async () => { + const callTool = async (args: Record) => { const raw = await input.entry.tool.client.callTool( - { name: input.entry.tool.def.name, arguments: input.args }, + { name: input.entry.tool.def.name, arguments: args }, CallToolResultSchema, { resetTimeoutOnProgress: true, @@ -166,6 +166,16 @@ const invokeChildTool = Effect.fn("CodeMode.invokeChildTool")(function* (input: .join("\n\n") || "MCP tool returned an error", ) return raw + } + return yield* Effect.promise(async () => { + try { + return await callTool(input.args) + } catch (error) { + // A server whose advertised schema says string but validates an object rejects `args: "{}"`; retry once decoded. + const decoded = decodeRejectedInput(input.args, error) + if (!decoded) throw error + return await callTool(decoded) + } }) }).pipe( Effect.withSpan("Tool.execute", { diff --git a/packages/opencode/test/tool/code-mode.test.ts b/packages/opencode/test/tool/code-mode.test.ts index 34b3faa610d7..3011a0a0fdfa 100644 --- a/packages/opencode/test/tool/code-mode.test.ts +++ b/packages/opencode/test/tool/code-mode.test.ts @@ -358,6 +358,34 @@ describe("code mode execute", () => { expect(output.output).toBe("caught: server exploded") }) + test("retries an MCP call once with decoded JSON-string args after the server rejects a string for an object", async () => { + const seen: unknown[] = [] + const asked: unknown[] = [] + const permissionCtx: Tool.Context = { ...ctx, ask: (req) => Effect.sync(() => void asked.push(req)) } + const tool = await build({ + devtools_call: mcpTool( + "call", + (args) => { + seen.push(args.args) + if (typeof args.args === "string") + throw new Error( + 'MCP error -32603: Invalid argument \'args\': [{"expected": "object", "received": "string"}]', + ) + return { content: [{ type: "text", text: "ok" }] } + }, + { type: "object", properties: { args: { type: "string" } } }, + ), + }) + + const output = await Effect.runPromise( + tool.execute({ code: 'return await tools.devtools.call({ args: "{}" })' }, permissionCtx), + ) + + expect(output.metadata.error).toBeUndefined() + expect(seen).toStrictEqual(["{}", {}]) + expect(asked).toHaveLength(1) + }) + test("asks permission before each child tool call", async () => { const asked: unknown[] = [] const permissionCtx: Tool.Context = { ...ctx, ask: (req) => Effect.sync(() => void asked.push(req)) }