diff --git a/packages/opencode/src/tool/tool.ts b/packages/opencode/src/tool/tool.ts index 2e631b216e..8dafc6ed1d 100644 --- a/packages/opencode/src/tool/tool.ts +++ b/packages/opencode/src/tool/tool.ts @@ -1,5 +1,5 @@ import { PermissionV1 } from "@opencode-ai/core/v1/permission" -import { Effect, Schema } from "effect" +import { Effect, Option, Schema } from "effect" import { SessionV1 } from "@opencode-ai/core/v1/session" import type { JSONSchema7 } from "@ai-sdk/provider" import type { SessionID, MessageID } from "../session/schema" @@ -107,10 +107,13 @@ export type InferDef = /** * The OpenAI tools contract requires `parameters` to be a JSON Schema object. * A root-level combinator (anyOf/oneOf/allOf) is outside that contract: - * OpenAI tolerates it, DeepSeek rejects it with a schema error, and GLM - * silently emits empty tool arguments. Tools that need a discriminated union - * must nest it under a property (e.g. `{ params: }`). Violations fail - * at construction time here instead of degrading at provider runtime. + * OpenAI tolerates it, DeepSeek rejects it with a schema error, GLM silently + * emits empty tool arguments, and qwen-family models string-encode property + * values whose schema is a nested union (issue #297 — repaired by the + * retry pass in the execute wrapper below). Tools that need a discriminated + * union must nest it under a property (e.g. `{ params: }`). + * Violations fail at construction time here instead of degrading at provider + * runtime. */ function assertObjectRootedParameters(id: string, toolInfo: DefWithoutID | { parameters: unknown; jsonSchema?: unknown }) { const root = toolInfo.jsonSchema ?? ToolJsonSchema.fromSchema(toolInfo.parameters as Schema.Top) @@ -162,16 +165,19 @@ function wrap, Result extends Metadat "message.id": ctx.messageID, ...(ctx.callID ? { "tool.call_id": ctx.callID } : {}), } + const invalidArguments = (error: unknown) => + new InvalidArgumentsError({ + tool: id, + detail: toolInfo.formatValidationError ? toolInfo.formatValidationError(error) : String(error), + }) return Effect.gen(function* () { - const decoded = yield* decode(args).pipe( - Effect.mapError( - (error) => - new InvalidArgumentsError({ - tool: id, - detail: toolInfo.formatValidationError ? toolInfo.formatValidationError(error) : String(error), - }), - ), - ) + // Strict decode first; the lenient retry only runs once strict + // decoding already failed, so legitimate string arguments that look + // like JSON are never re-parsed. + const strict = yield* decode(args).pipe(Effect.option) + const decoded = Option.isSome(strict) + ? strict.value + : yield* decode(repairStringifiedContainers(args)).pipe(Effect.mapError(invalidArguments)) const result = yield* execute(decoded as Schema.Schema.Type, ctx) if (result.metadata.truncated !== undefined) { return result @@ -193,6 +199,33 @@ function wrap, Result extends Metadat }) } +// Some models string-encode a tool-argument container whose schema is a +// nested union (qwen family, issue #297): the wire carries {"params": +// "{\"action\": \"list\"}"} instead of a nested object. Re-parse strings that +// sit where a container is expected and let the strict decode judge the +// result. Runs only after the strict decode failed, so plain string +// parameters are never touched. +function repairStringifiedContainers(value: unknown): unknown { + if (typeof value === "string") { + const trimmed = value.trim() + if (!trimmed.startsWith("{") && !trimmed.startsWith("[")) return value + try { + const parsed: unknown = JSON.parse(trimmed) + if (typeof parsed === "object" && parsed !== null) return repairStringifiedContainers(parsed) + } catch { + return value + } + return value + } + if (Array.isArray(value)) return value.map(repairStringifiedContainers) + if (typeof value === "object" && value !== null) { + return Object.fromEntries( + Object.entries(value).map(([key, item]) => [key, repairStringifiedContainers(item)]), + ) + } + return value +} + export function define< Parameters extends Schema.Decoder, Result extends Metadata, diff --git a/packages/opencode/test/tool/tool-define.test.ts b/packages/opencode/test/tool/tool-define.test.ts index 8a6afe39de..16c4c90d47 100644 --- a/packages/opencode/test/tool/tool-define.test.ts +++ b/packages/opencode/test/tool/tool-define.test.ts @@ -106,6 +106,96 @@ describe("Tool.define", () => { }), ) + // Regression for #297: qwen-family models string-encode a nested-union + // property value ({"params": "{\"action\": \"list\"}"}). The execute wrap + // must retry with the container re-parsed so the tool still runs. + it.effect("stringified container arguments decode through the lenient retry", () => + Effect.gen(function* () { + const parameters = Schema.Struct({ + params: Schema.Union([ + Schema.Struct({ action: Schema.Literal("list") }), + Schema.Struct({ action: Schema.Literal("validate"), spec_path: Schema.String }), + ]), + }) + const calls: Array> = [] + const info = yield* Tool.define( + "test-repair", + Effect.succeed({ + description: "test tool", + parameters, + parseOptions: { onExcessProperty: "error" }, + execute(args: Schema.Schema.Type) { + calls.push(args) + return Effect.succeed({ title: "test", output: "ok", metadata: { truncated: false } }) + }, + }), + ) + const ctx = makeCtx() + const tool = yield* info.init() + const execute = tool.execute as unknown as (args: unknown, ctx: Tool.Context) => ReturnType + + yield* execute({ params: '{"action": "list"}' }, ctx) + yield* execute({ params: '{"action": "validate", "spec_path": "spec.yaml"}' }, ctx) + yield* execute({ params: { action: "list" } }, ctx) + + expect(calls).toEqual([ + { params: { action: "list" } }, + { params: { action: "validate", spec_path: "spec.yaml" } }, + { params: { action: "list" } }, + ]) + }), + ) + + it.effect("unrepairable arguments still surface as InvalidArgumentsError", () => + Effect.gen(function* () { + const parameters = Schema.Struct({ + params: Schema.Union([Schema.Struct({ action: Schema.Literal("list") })]), + }) + const info = yield* Tool.define( + "test-repair-fail", + Effect.succeed({ + description: "test tool", + parameters, + execute() { + return Effect.succeed({ title: "test", output: "ok", metadata: { truncated: false } }) + }, + }), + ) + const tool = yield* info.init() + const execute = tool.execute as unknown as (args: unknown, ctx: Tool.Context) => ReturnType + + const exit = yield* execute({ params: "not a container" }, makeCtx()).pipe(Effect.exit) + expect(Exit.isFailure(exit)).toBe(true) + if (!Exit.isFailure(exit)) return + const die = exit.cause.reasons.find(Cause.isDieReason) + expect(die?.defect).toBeInstanceOf(Tool.InvalidArgumentsError) + }), + ) + + it.effect("plain string parameters that look like JSON are not re-parsed", () => + Effect.gen(function* () { + const parameters = Schema.Struct({ note: Schema.String }) + const calls: Array> = [] + const info = yield* Tool.define( + "test-string-passthrough", + Effect.succeed({ + description: "test tool", + parameters, + execute(args: Schema.Schema.Type) { + calls.push(args) + return Effect.succeed({ title: "test", output: "ok", metadata: { truncated: false } }) + }, + }), + ) + const tool = yield* info.init() + const execute = tool.execute as unknown as (args: unknown, ctx: Tool.Context) => ReturnType + + yield* execute({ note: '{"kept": "string"}' }, makeCtx()) + + expect(calls).toEqual([{ note: '{"kept": "string"}' }]) + }), + ) + // Regression for #28438: the wrap is the canonical "untyped → typed" boundary. // When the LLM emits a tool call with a payload that fails the parameter // schema, the wrap must surface a typed `Tool.InvalidArgumentsError` whose