diff --git a/packages/codemode/src/codemode.ts b/packages/codemode/src/codemode.ts index 842209dac57d..059786f683da 100644 --- a/packages/codemode/src/codemode.ts +++ b/packages/codemode/src/codemode.ts @@ -49,6 +49,8 @@ export type ExecuteOptions = {}> = { onToolCallStart?: (call: ToolRuntime.ToolCallStarted) => Effect.Effect> /** Observes each admitted tool call as it settles, with outcome and duration. */ onToolCallEnd?: (call: ToolRuntime.ToolCallEnded) => Effect.Effect> + /** Explains an unknown tool path the host recognizes from outside Code Mode; replaces the default suggestions. */ + unknownToolHint?: (path: ReadonlyArray) => string | undefined } /** A JSON value that can cross the confined interpreter boundary. */ diff --git a/packages/codemode/src/interpreter/runtime.ts b/packages/codemode/src/interpreter/runtime.ts index 093f57776510..9cb93171507f 100644 --- a/packages/codemode/src/interpreter/runtime.ts +++ b/packages/codemode/src/interpreter/runtime.ts @@ -95,13 +95,13 @@ import { coerceToNumber, coerceToString, compoundOperators, - createErrorValue, - errorBrandName, errorConstructors, invokeCoercion, valueConstructors, } from "../stdlib/value.js" import { + createErrorValue, + errorBrandName, isSandboxValue, SandboxDate, SandboxMap, @@ -112,8 +112,20 @@ import { SandboxURLSearchParams, } from "../values.js" +const PROGRAM_PREFIX = "async function __codemode__() {\n" + +// Without a position the retry re-emits a long script blind; the offset counts from inside the wrapper. +const formatParsePosition = (code: string, start: number | undefined): string => { + if (start === undefined) return "" + const offset = start - PROGRAM_PREFIX.length + const lineStart = code.lastIndexOf("\n", offset - 1) + 1 + const line = code.slice(0, lineStart).split("\n").length + const excerpt = code.slice(Math.max(lineStart, offset - 40), offset + 20).replace(/\s+/g, " ") + return ` (line ${line}, col ${offset - lineStart + 1}) near: ${excerpt}` +} + const parseProgram = (code: string): ProgramNode => { - const transpiled = transpileModule(`async function __codemode__() {\n${code}\n}`, { + const transpiled = transpileModule(`${PROGRAM_PREFIX}${code}\n}`, { reportDiagnostics: true, compilerOptions: { target: ScriptTarget.ESNext, @@ -124,7 +136,7 @@ const parseProgram = (code: string): ProgramNode => { if (diagnostic) { throw new InterpreterRuntimeError( - `Failed to parse TypeScript: ${flattenDiagnosticMessageText(diagnostic.messageText, "\n")}`, + `Failed to parse TypeScript: ${flattenDiagnosticMessageText(diagnostic.messageText, "\n")}${formatParsePosition(code, diagnostic.start)}`, undefined, "ParseError", ) @@ -3345,6 +3357,7 @@ export const executeWithLimits = >( const hooks = { ...(options.onToolCallStart === undefined ? {} : { onToolCallStart: options.onToolCallStart }), ...(options.onToolCallEnd === undefined ? {} : { onToolCallEnd: options.onToolCallEnd }), + ...(options.unknownToolHint === undefined ? {} : { unknownToolHint: options.unknownToolHint }), } const tools = ToolRuntime.make( (options.tools ?? {}) as HostTools>, diff --git a/packages/codemode/src/stdlib/value.ts b/packages/codemode/src/stdlib/value.ts index 17ca8b1c486f..0ed51248e174 100644 --- a/packages/codemode/src/stdlib/value.ts +++ b/packages/codemode/src/stdlib/value.ts @@ -12,24 +12,17 @@ export const valueConstructors = new Set(["Date", "RegExp", "Map", "Set", "URL", export const compoundOperators = new Set(["+=", "-=", "*=", "/=", "%=", "**=", "&=", "|=", "^=", "<<=", ">>=", ">>>="]) -const ErrorBrand: unique symbol = Symbol("codemode.error") - -export const createErrorValue = (name: string, message: string): SafeObject => { - const value = Object.assign(Object.create(null) as SafeObject, { name, message }) - Object.defineProperty(value, ErrorBrand, { value: name }) - return value -} - -export const errorBrandName = (value: unknown): string | undefined => - value !== null && typeof value === "object" - ? ((value as Record)[ErrorBrand] as string | undefined) - : undefined - export const boundedData = (value: unknown, label: string): unknown => copyIn(value, label, true) export const coerceToString = (value: unknown): string => { if (value === null) return "null" if (value === undefined) return "undefined" + // `catch (e) { return String(e) }` is the common idiom; "[object Object]" would discard the tool's message. + const errorName = errorBrandName(value) + if (errorName !== undefined) { + const message = (value as { message?: unknown }).message + return typeof message === "string" && message !== "" ? `${errorName}: ${message}` : errorName + } if (value instanceof SandboxDate) return Number.isFinite(value.time) ? new Date(value.time).toISOString() : "Invalid Date" if (value instanceof SandboxRegExp) return `/${value.regex.source}/${value.regex.flags}` @@ -73,13 +66,10 @@ export const invokeCoercion = (ref: CoercionFunction, args: Array, node if (ref.name === "parseFloat") return parseFloat(coerceToString(value)) return coerceToString(value) } +import { type AstNode, CoercionFunction, InterpreterRuntimeError } from "../interpreter/model.js" +import { copyIn } from "../tool-runtime.js" import { - type AstNode, - CoercionFunction, - InterpreterRuntimeError, -} from "../interpreter/model.js" -import { copyIn, type SafeObject } from "../tool-runtime.js" -import { + errorBrandName, isSandboxValue, SandboxDate, SandboxMap, diff --git a/packages/codemode/src/tool-runtime.ts b/packages/codemode/src/tool-runtime.ts index f4ccc61d4c49..fcb1f7a97373 100644 --- a/packages/codemode/src/tool-runtime.ts +++ b/packages/codemode/src/tool-runtime.ts @@ -10,6 +10,7 @@ import { } from "./tool-schema.js" import { isDefinition as isToolDefinition, type Definition } from "./tool.js" import { + errorBrandName, SandboxDate, SandboxMap, SandboxPromise, @@ -71,6 +72,8 @@ export type ToolCallEnded = { export type ToolCallHooks = { readonly onToolCallStart?: ((call: ToolCallStarted) => Effect.Effect) | undefined readonly onToolCallEnd?: ((call: ToolCallEnded) => Effect.Effect) | undefined + /** Explains an unknown tool path the host recognizes from outside Code Mode; replaces the default suggestions. */ + readonly unknownToolHint?: ((path: ReadonlyArray) => string | undefined) | undefined } /** Model-visible description of one schema-backed tool. */ @@ -211,14 +214,15 @@ const copyBounded = ( if (preserveSandboxValues) { // Intra-sandbox checkpoints keep sandbox value instances alive as leaves; their contents // are never walked here (Map/Set members are validated where mutation happens, and the - // real boundary still serializes them below). + // real boundary still serializes them below). A caught error keeps its brand the same way. if ( value instanceof SandboxDate || value instanceof SandboxRegExp || value instanceof SandboxMap || value instanceof SandboxSet || value instanceof SandboxURL || - value instanceof SandboxURLSearchParams + value instanceof SandboxURLSearchParams || + errorBrandName(value) !== undefined ) { return value } @@ -383,6 +387,33 @@ const termForms = (term: string): Array => { return forms } +// Field-weighted score summed across terms: exact path or segment 20, path substring 8, description 4, +// any searchable text (input names and descriptions) 2. Entries matching no term are dropped unless the query is empty. +const rankTools = (entries: ReadonlyArray, query: string): ReadonlyArray => { + const terms = tokenize(query).map(termForms) + return entries + .map((entry) => { + const path = entry.description.path.toLowerCase() + const description = entry.description.description.toLowerCase() + const score = terms.reduce( + (total, forms) => + total + + (forms.some((form) => path === form || path.endsWith(`.${form}`)) ? 20 : 0) + + (forms.some((form) => path.includes(form)) ? 8 : 0) + + (forms.some((form) => description.includes(form)) ? 4 : 0) + + (forms.some((form) => entry.searchText.includes(form)) ? 2 : 0), + 0, + ) + return { entry, score } + }) + .filter(({ score }) => terms.length === 0 || score > 0) + .sort( + (left, right) => + right.score - left.score || left.entry.description.path.localeCompare(right.entry.description.path), + ) + .map(({ entry }) => entry) +} + const makeSearchTool = (searchIndex: ReadonlyArray): Definition => ({ _tag: "CodeModeTool", description: "Search available Code Mode tools", @@ -407,34 +438,7 @@ const makeSearchTool = (searchIndex: ReadonlyArray): Definition => : scoped.find( (entry) => entry.description.path === pathQuery || toolExpression(entry.description.path) === trimmed, ) - const terms = tokenize(query).map(termForms) - // Additive field-weighted scoring, summed across terms: exact path or path segment - // (20) > path substring (8) > description substring (4) > any searchable text, - // including input parameter names and descriptions (2). - const ranked = - exact !== undefined - ? [exact] - : scoped - .map((entry) => { - const path = entry.description.path.toLowerCase() - const description = entry.description.description.toLowerCase() - const score = terms.reduce( - (total, forms) => - total + - (forms.some((form) => path === form || path.endsWith(`.${form}`)) ? 20 : 0) + - (forms.some((form) => path.includes(form)) ? 8 : 0) + - (forms.some((form) => description.includes(form)) ? 4 : 0) + - (forms.some((form) => entry.searchText.includes(form)) ? 2 : 0), - 0, - ) - return { entry, score } - }) - .filter(({ score }) => terms.length === 0 || score > 0) - .sort( - (left, right) => - right.score - left.score || left.entry.description.path.localeCompare(right.entry.description.path), - ) - .map(({ entry }) => entry) + const ranked = exact !== undefined ? [exact] : rankTools(scoped, query) const items = ranked.slice(offset, offset + (request.limit ?? defaultSearchLimit)).map(({ description }) => ({ ...description, path: toolExpression(description.path), @@ -673,7 +677,11 @@ const namespaceKeys = (tools: HostTools, path: ReadonlyArray): Rea return Object.keys(value) } -const resolve = (tools: HostTools, path: ReadonlyArray): HostTool | Definition => { +const resolve = ( + tools: HostTools, + path: ReadonlyArray, + explain: (path: ReadonlyArray) => ReadonlyArray, +): HostTool | Definition => { let value: HostTool | Definition | HostTools = tools for (const segment of path) { @@ -683,9 +691,7 @@ const resolve = (tools: HostTools, path: ReadonlyArray): HostTool< isDefinition(value) || !Object.hasOwn(value, segment) ) { - throw new ToolRuntimeError("UnknownTool", `Unknown tool '${path.join(".")}'.`, [ - "Use tools.$codemode.search({ query }) to find available described tools.", - ]) + throw new ToolRuntimeError("UnknownTool", `Unknown tool '${path.join(".")}'.`, explain(path)) } value = value[segment] as HostTool | Definition | HostTools } @@ -745,6 +751,24 @@ export const make = ( catch: () => new ToolRuntimeError("InvalidToolOutput", `Invalid output from tool '${name}'.`), }) + // A wrong name otherwise costs the model a search and a retry. The host speaks first: it may + // know the name as one of its own tools, which no search inside Code Mode would find. + const explainUnknownTool = (path: ReadonlyArray): ReadonlyArray => { + const hostHint = hooks?.unknownToolHint?.(path) + if (hostHint !== undefined) return [hostHint] + const [namespace = ""] = path + const name = path.at(-1) ?? "" + const inNamespace = searchIndex.filter((entry) => entry.namespace === namespace) + // Models often repeat the namespace in the name (`bedrock.bedrock_send`). + const query = name.startsWith(`${namespace}_`) ? name.slice(namespace.length + 1) : name + const [closest, ...others] = rankTools(inNamespace.length > 0 ? inNamespace : searchIndex, query).slice(0, 3) + if (closest === undefined) return ["Use tools.$codemode.search({ query }) to find available described tools."] + const hints = [`Did you mean: ${closest.description.signature}`] + if (others.length > 0) + hints.push(`Other close matches: ${others.map((entry) => toolExpression(entry.description.path)).join(", ")}`) + return hints + } + const recordCall = (call: ToolCall): void => { if (maxToolCalls !== undefined && calls.length >= maxToolCalls) { throw new ToolRuntimeError("ToolCallLimitExceeded", `Execution exceeded its tool-call limit of ${maxToolCalls}.`) @@ -766,13 +790,14 @@ export const make = ( recordCall(call) return calls.length - 1 }).pipe(Effect.tap((index) => hooks?.onToolCallStart?.({ index, name, input }) ?? Effect.void)) - const tool = resolve(callableTools, path) + const tool = resolve(callableTools, path, explainUnknownTool) let describedInput: unknown if (isDefinition(tool)) { - if (externalArgs.length !== 1) + if (externalArgs.length > 1) throw new ToolRuntimeError("InvalidToolInput", `Tool '${name}' expects exactly one input object.`) describedInput = yield* Effect.try({ - try: () => decodeToolInput(tool, externalArgs[0]), + // A call with no arguments means "no options"; the tool's own schema still rejects a missing required field. + try: () => decodeToolInput(tool, externalArgs[0] ?? {}), catch: (cause) => new ToolRuntimeError("InvalidToolInput", `Invalid input for tool '${name}': ${String(cause)}`), }) diff --git a/packages/codemode/src/values.ts b/packages/codemode/src/values.ts index 4ca305d815eb..789a61bbcd0a 100644 --- a/packages/codemode/src/values.ts +++ b/packages/codemode/src/values.ts @@ -47,3 +47,16 @@ export const isSandboxValue = ( value instanceof SandboxSet || value instanceof SandboxURL || value instanceof SandboxURLSearchParams + +const ErrorBrand: unique symbol = Symbol("codemode.error") + +export const createErrorValue = (name: string, message: string): Record => { + const value = Object.assign(Object.create(null) as Record, { name, message }) + Object.defineProperty(value, ErrorBrand, { value: name }) + return value +} + +export const errorBrandName = (value: unknown): string | undefined => + value !== null && typeof value === "object" + ? ((value as Record)[ErrorBrand] as string | undefined) + : undefined diff --git a/packages/codemode/test/recoverable-mistakes.test.ts b/packages/codemode/test/recoverable-mistakes.test.ts new file mode 100644 index 000000000000..8ef2ab84da40 --- /dev/null +++ b/packages/codemode/test/recoverable-mistakes.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, test } from "bun:test" +import { Effect, Schema } from "effect" +import { CodeMode, Tool, toolError } from "../src/index.js" + +const text = (description: string, value: string) => + Tool.make({ + description, + input: Schema.Struct({ query: Schema.optionalKey(Schema.String) }), + output: Schema.String, + run: () => Effect.succeed(value), + }) + +const tools = { + bedrock: { + registry_find_items: text("Find registry items", "found"), + generate_image: text("Generate an image", "job"), + list_sites: text("List sites", "sites"), + reject: Tool.make({ + description: "Reject a record", + input: Schema.Struct({}), + output: Schema.String, + run: () => Effect.fail(toolError("Field contact_phone must be a valid phone number")), + }), + }, +} + +const execute = (code: string, options: Omit, "tools"> = {}) => + Effect.runPromise(CodeMode.make({ tools, ...options }).execute(code)) + +// Each of these is a mistake models make daily. The reply has to carry the fix, because every +// extra round trip to discover it costs a full model turn. +describe("CodeMode recoverable mistakes", () => { + test("an unknown tool names the closest real tool with its signature", async () => { + const result = await execute(`return await tools.bedrock.find_registry_items({ query: "cro" })`) + + expect(result.ok ? undefined : result.error.kind).toBe("UnknownTool") + expect(result.ok ? undefined : result.error.suggestions?.[0]).toStartWith( + "Did you mean: tools.bedrock.registry_find_items(", + ) + }) + + test("a namespace repeated in the name still finds the tool", async () => { + const result = await execute(`return await tools.bedrock.bedrock_generate_image({ query: "hero" })`) + + expect(result.ok ? undefined : result.error.suggestions?.[0]).toStartWith( + "Did you mean: tools.bedrock.generate_image(", + ) + }) + + test("a name nothing resembles still points at search", async () => { + const result = await execute(`return await tools.bedrock.zzz({})`) + + expect(result.ok ? undefined : result.error.suggestions).toStrictEqual([ + "Use tools.$codemode.search({ query }) to find available described tools.", + ]) + }) + + test("the host can claim an unknown name as one of its own tools", async () => { + const hint = "'set-project-context' is one of your regular tools. Call it directly." + const result = await execute(`return await tools.bedrock.set_project_context({})`, { + unknownToolHint: (path) => (path.at(-1) === "set_project_context" ? hint : undefined), + }) + + expect(result.ok ? undefined : result.error.suggestions).toStrictEqual([hint]) + }) + + test("a call with no arguments sends an empty input object", async () => { + const result = await execute(`return await tools.bedrock.list_sites()`) + + expect(result.ok ? result.value : result.error).toBe("sites") + }) + + test("a second argument is still rejected", async () => { + const result = await execute(`return await tools.bedrock.list_sites({}, {})`) + + expect(result.ok ? undefined : result.error.kind).toBe("InvalidToolInput") + }) + + test("a parse error says where it is", async () => { + const result = await execute(`const sites = 1\nreturn await tools.bedrock.list_sites({)`) + + expect(result.ok ? undefined : result.error.kind).toBe("ParseError") + expect(result.ok ? undefined : result.error.message).toMatch(/\(line 2, col \d+\) near: .*list_sites/) + }) + + test("a caught tool failure stringifies to its message", async () => { + const result = await execute( + `try { await tools.bedrock.reject({}) } catch (e) { return [String(e), \`failed: \${e}\`, "" + e] }`, + ) + + expect(result.ok ? result.value : result.error).toStrictEqual([ + "Error: Field contact_phone must be a valid phone number", + "failed: Error: Field contact_phone must be a valid phone number", + "Error: Field contact_phone must be a valid phone number", + ]) + }) +}) diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index 815a87818f7e..2c9ad3da6679 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -62,7 +62,13 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { abort: options.abortSignal!, messageID: input.processor.message.id, callID: options.toolCallId, - extra: { model: input.model, bypassAgentCheck: input.bypassAgentCheck, promptOps: input.promptOps }, + extra: { + model: input.model, + bypassAgentCheck: input.bypassAgentCheck, + promptOps: input.promptOps, + // Lets code mode tell the model that a name it tried inside a script is one of these instead. + toolIDs: Object.keys(tools), + }, agent: input.agent.name, messages: input.messages, metadata: (val) => diff --git a/packages/opencode/src/tool/code-mode.ts b/packages/opencode/src/tool/code-mode.ts index 931618c568d5..20c8b54085b8 100644 --- a/packages/opencode/src/tool/code-mode.ts +++ b/packages/opencode/src/tool/code-mode.ts @@ -115,6 +115,16 @@ function projectMcpResult(result: CallToolResult, collect: (attachment: Attachme return null } +// The model reaches for an agent tool such as `find-registry-items` inside a script as +// `tools..find_registry_items`; no search of the MCP catalog can correct that. +function agentToolHint(path: ReadonlyArray, toolIDs: unknown): string | undefined { + const fold = (name: string) => name.toLowerCase().replaceAll(/[-_]/g, "") + const wanted = fold(path.at(-1) ?? "") + const match = Array.isArray(toolIDs) ? toolIDs.find((id) => id !== CODE_MODE_TOOL && fold(id) === wanted) : undefined + if (match === undefined) return + return `'${match}' is one of your regular tools, not a Code Mode tool. Call it directly, outside ${CODE_MODE_TOOL}.` +} + type Run = (input: unknown) => Effect.Effect function toolTree(catalog: readonly CatalogEntry[], run: (entry: CatalogEntry) => Run) { @@ -248,6 +258,7 @@ export const CodeModeTool = Tool.define( const runtime = CodeMode.make({ tools: toolTree(catalog, callTool), + unknownToolHint: (path) => agentToolHint(path, ctx.extra?.toolIDs), onToolCallStart: ({ index, name, input }) => Effect.suspend(() => { const shown = (() => { diff --git a/packages/opencode/test/tool/code-mode.test.ts b/packages/opencode/test/tool/code-mode.test.ts index 3011a0a0fdfa..f398a90f4b98 100644 --- a/packages/opencode/test/tool/code-mode.test.ts +++ b/packages/opencode/test/tool/code-mode.test.ts @@ -348,6 +348,23 @@ describe("code mode execute", () => { expect(error.message).toContain("Unknown tool 'known.missing'") }) + test("an unknown tool names the closest MCP tool so the retry needs no search", async () => { + const tool = await build({ bedrock_registry_find_items: mcpTool("registry_find_items", () => "ok") }) + const error = await failure(tool.execute({ code: "return await tools.bedrock.find_registry_items({})" }, ctx)) + expect(error.message).toContain("Did you mean: tools.bedrock.registry_find_items(") + }) + + test("a regular agent tool called inside a script is sent back to a direct call", async () => { + const tool = await build({ bedrock_list_sites: mcpTool("list_sites", () => "ok") }) + const error = await failure( + tool.execute( + { code: "return await tools.bedrock.set_project_context({})" }, + { ...ctx, extra: { toolIDs: ["read", "set-project-context", CODE_MODE_TOOL] } }, + ), + ) + expect(error.message).toContain("'set-project-context' is one of your regular tools") + }) + test("propagates an MCP tool error into the program as a catchable failure", async () => { const tool = await build({ bad_tool: mcpTool("tool", () => ({ isError: true, content: [{ type: "text", text: "server exploded" }] })),