From e6f21f95cdc3118075c9a0a8981572dc13b28ea0 Mon Sep 17 00:00:00 2001 From: Aiden Cline <63023139+rekram1-node@users.noreply.github.com> Date: Thu, 12 Nov 2026 18:06:23 -0600 Subject: [PATCH] revert(core): restore tool dispatch before nested repair (#63630) --- packages/codemode/README.md | 15 +- packages/codemode/src/runtime/driver.ts | 21 +- packages/codemode/src/scheduler.ts | 9 +- packages/codemode/src/tool-runtime.ts | 204 ++++---- packages/codemode/test/suspend.test.ts | 77 --- packages/codemode/test/tool-paths.test.ts | 96 ---- packages/core/README.md | 8 - packages/core/src/codemode/tool.ts | 191 +++----- packages/core/src/plugin/hooks.ts | 4 +- packages/core/src/plugin/tool-input-repair.ts | 53 +- packages/core/src/tool.ts | 109 ++--- packages/core/src/tool/prepare.ts | 76 --- packages/core/src/tool/runtime.ts | 3 - .../tool-input-repair-integration.test.ts | 453 +----------------- packages/core/test/tool-execute.test.ts | 92 ---- .../src/message/current-tool-state.test.ts | 42 +- .../src/message/current-tool-state.ts | 7 +- 17 files changed, 263 insertions(+), 1197 deletions(-) delete mode 100644 packages/core/src/tool/prepare.ts diff --git a/packages/codemode/README.md b/packages/codemode/README.md index 0cdf20ff03e..f5a2730f67f 100644 --- a/packages/codemode/README.md +++ b/packages/codemode/README.md @@ -94,26 +94,13 @@ runtime.execute(source) // Effect ``` The Effect environment is inferred from the supplied tools. `hooks` surround every call the program makes into the -host: `tool.before`/`tool.after` receive `{ name, input, key }` with the call's `:` +host: `tool.before`/`tool.after` receive `{ name, input, key }` with decoded input and the call's `:` idempotency key, and `extension.before`/`extension.after` receive `{ extension, name, args }`. An `after` hook also receives how the call ended (`success` with its value, `failure` with its error, or `interrupted`). A `before` hook runs before the call reaches the host, and a failing one denies it: the host function is never called, and the program catches the failure as a thrown error. A denied call fails the way the call itself would: a tool call or asynchronous global rejects its promise, and a synchronous global throws. -The runtime counts a tool attempt toward `maxToolCalls` under its requested name before lookup or validation. -The optional `tool.resolve` hook can rewrite the name and input, including an unknown path. Return the original -name to keep the catalog's lookup and suggestions. The built-in `search` skips resolution. `tool.before` sees the -decoded input. `tool.after` sees the same call object even if lookup, validation, or a hook failed; in that case -it carries the last name and input reached. A veto from `tool.resolve` or `tool.before` rejects the script's call -with `ToolFailure`. - -A redirect must preserve the execution mode (plain, idempotent, or durable) because the driver chooses how to -suspend the call from its requested path. Unknown paths can redirect to plain tools. For resumable calls the -runtime saves the resolved name and JSON input before schema decoding. Resume reuses a completed resolution; -if suspension interrupted an idempotent call during resolution, the hook runs again with the same key. -Detaching to suspend leaves `tool.after` pending until the call settles. - ### Suspending and resuming a run `CodeMode.make` returns a `CodeMode.Runtime`, which can also run a program step by step: diff --git a/packages/codemode/src/runtime/driver.ts b/packages/codemode/src/runtime/driver.ts index 8e0d553b525..e9039c1d43f 100644 --- a/packages/codemode/src/runtime/driver.ts +++ b/packages/codemode/src/runtime/driver.ts @@ -164,18 +164,8 @@ export class Driver { call.status = "running" const request = call.request if (request.kind === "tool") { - const options = { - key: call.key, - admit: (name: string) => this.admit(call.id, name), - detaching: () => this.suspending, - resolved: request.resolved, - save: (name: string, input: Json | undefined) => { - const current = this.vm.call(call.id) - if (current) current.request = { kind: "tool", path: name.split("."), args: [input], resolved: true } - }, - } if (this.tools.mode(request.path)?._tag === "CodeModeDurable") { - const started = this.tools.start(request.path, request.args, options) + const started = this.tools.start(request.path, request.args, call.key, (name) => this.admit(call.id, name)) return this.fork(call.id, started, (data) => { const current = this.vm.call(call.id) if (current === undefined) return @@ -185,7 +175,14 @@ export class Driver { this.wait(current) }) } - return this.fork(call.id, this.tools.execute(request.path, request.args, options)) + return this.fork( + call.id, + this.tools.execute(request.path, request.args, { + key: call.key, + admit: (name) => this.admit(call.id, name), + detaching: () => this.suspending, + }), + ) } if (request.kind === "search") { return this.fork( diff --git a/packages/codemode/src/scheduler.ts b/packages/codemode/src/scheduler.ts index d805e4e376a..3d372013db8 100644 --- a/packages/codemode/src/scheduler.ts +++ b/packages/codemode/src/scheduler.ts @@ -71,12 +71,7 @@ export type Job = * extension function's call (see `ExtensionRequest`). */ export type HostRequest = - | { - readonly kind: "tool" - readonly path: ReadonlyArray - readonly args: Array - readonly resolved?: true - } + | { readonly kind: "tool"; readonly path: ReadonlyArray; readonly args: Array } | { readonly kind: "search"; readonly args: Array } | { readonly kind: "extension"; readonly call: ExtensionRequest } @@ -115,7 +110,7 @@ export class HostCall { readonly id: number, /** The call's stable idempotency key, `:`. */ readonly key: string, - public request: HostRequest, + readonly request: HostRequest, /** What runs: a tool path, or an extension function's label. */ readonly label: string, /** The running job waits for it: its outcome continues the frame that asked, before any other job. */ diff --git a/packages/codemode/src/tool-runtime.ts b/packages/codemode/src/tool-runtime.ts index ac6da218c0d..8910a44c6b8 100644 --- a/packages/codemode/src/tool-runtime.ts +++ b/packages/codemode/src/tool-runtime.ts @@ -34,7 +34,7 @@ export type ToolCall = { readonly name: string } -/** A tool call the program is making. Host resolution updates its path and input; `tool.before` sees decoded input. */ +/** A tool call the program is making, with its decoded input. */ export type ToolInvocation = { readonly name: string readonly input: unknown @@ -58,16 +58,8 @@ export type CallResult = | { readonly status: "failure"; readonly error: unknown } | { readonly status: "interrupted" } -function callResult(exit: Exit.Exit): CallResult { - if (Exit.isSuccess(exit)) return { status: "success", value: exit.value } - if (Cause.hasInterruptsOnly(exit.cause)) return { status: "interrupted" } - return { status: "failure", error: Cause.squash(exit.cause) } -} - /** Hooks around every call the program makes into the host. A failing `before` denies the call. */ export type Hooks = { - /** Resolve a registered or unknown tool path before validation. Redirects must preserve the call's execution mode. */ - readonly "tool.resolve"?: (call: ToolInvocation) => Effect.Effect, unknown, R> readonly "tool.before"?: ((call: ToolInvocation) => Effect.Effect) | undefined readonly "tool.after"?: ((call: ToolInvocation, result: CallResult) => Effect.Effect) | undefined readonly "extension.before"?: ((call: ExtensionInvocation) => Effect.Effect) | undefined @@ -89,7 +81,9 @@ export const hooked = ( ? run : Effect.onExit(run, (exit) => { if (detaching?.()) return Effect.void - return after(call, callResult(exit)) + if (Exit.isSuccess(exit)) return after(call, { status: "success", value: exit.value }) + if (Cause.hasInterruptsOnly(exit.cause)) return after(call, { status: "interrupted" }) + return after(call, { status: "failure", error: Cause.squash(exit.cause) }) }) return before === undefined ? observed : Effect.andThen(before(call), observed) } @@ -531,12 +525,10 @@ const namespaceKeys = (root: ToolNode, path: ReadonlyArray): Reado return Array.from(node.children.keys()) } -const resolve = (prepared: Prepared, path: ReadonlyArray): Tool => { - const root = prepared.root +const resolve = (root: ToolNode, path: ReadonlyArray, index: ReadonlyArray): Tool => { const segments = canonicalSegments(path) const node = lookup(root, segments) if (node === undefined) { - const index = prepared.searchIndex const name = segments.join(".") const ns = segments.length > 1 && root.children.has(segments[0]) ? segments[0] : undefined const closest = rank( @@ -576,9 +568,6 @@ export type ExecuteCallOptions = { readonly key: string readonly admit: (name: string) => void readonly detaching?: (() => boolean) | undefined - /** A completed resolution saved for replay, before schema decoding. */ - readonly resolved?: boolean | undefined - readonly save?: (name: string, input: Json | undefined) => void } /** The tool bridge of one execution. Arguments arrive and results leave as JSON; program values never enter. */ @@ -605,7 +594,8 @@ export type ToolRuntime = { readonly start: ( path: ReadonlyArray, args: Array, - options: ExecuteCallOptions, + key: string, + admit: (name: string) => void, ) => Effect.Effect /** * Waits for a durable tool's running work: its output, as `execute` gives it. `detaching` says whether an @@ -640,22 +630,22 @@ export const make = ( calls.push(call) } - const argument = (args: Array) => (args.length === 0 ? {} : args[0]) - - const wrap = (tool: Tool | undefined, input: unknown) => { - const field = tool && typeof input === "string" ? stringParameter(tool)?.name : undefined - return field === undefined ? input : { [field]: input } - } - - const decode = (name: string, tool: Tool, input: unknown) => - Effect.try({ - try: () => decodeToolInput(tool, wrap(tool, input)), - catch: (cause) => - new ToolRuntimeError( - "InvalidToolInput", - `Invalid input for tool '${name}': ${String(cause)}`, - name === "search" ? [] : ["The signature may have changed. Use search to get the current signature."], - ), + const decode = (name: string, tool: Tool, args: Array) => + Effect.gen(function* () { + const normalized = args.length === 0 ? [{}] : args + if (normalized.length !== 1) + throw new ToolRuntimeError("InvalidToolInput", `Tool '${name}' expects at most one input object.`) + const input = normalized[0] + const field = typeof input === "string" ? stringParameter(tool)?.name : undefined + return yield* Effect.try({ + try: () => decodeToolInput(tool, field === undefined ? input : { [field]: input }), + catch: (cause) => + new ToolRuntimeError( + "InvalidToolInput", + `Invalid input for tool '${name}': ${String(cause)}`, + name === "search" ? [] : ["The signature may have changed. Use search to get the current signature."], + ), + }) }) // A tool's own failure becomes a ToolError; interruption passes through. @@ -683,76 +673,26 @@ export const make = ( catch: (cause) => new ToolRuntimeError("InvalidToolOutput", `Invalid output from tool '${name}': ${cause}`), }) - const invoke = (tool: Tool, call: ToolInvocation) => + const executeTool = (name: string, tool: Tool, args: Array, options: ExecuteCallOptions) => Effect.gen(function* () { if ("_tag" in tool.execute && tool.execute._tag === "CodeModeDurable") { - throw new TypeError(`Tool '${call.name}' is a durable tool; start it with ToolRuntime.start.`) + throw new TypeError(`Tool '${name}' is a durable tool; start it with ToolRuntime.start.`) } const run = "_tag" in tool.execute ? tool.execute.execute : tool.execute - const raw = yield* failure(Effect.suspend(() => run(call.input, { key: call.key }))) - return yield* output(call.name, tool, raw) - }) - - const mode = (tool: Tool | undefined) => (tool && "_tag" in tool.execute ? tool.execute : undefined) - - const attempt = ( - path: ReadonlyArray, - args: Array, - options: ExecuteCallOptions, - run: (tool: Tool, call: ToolInvocation) => Effect.Effect, - search = false, - ) => { - const name = path.join(".") - const original = search ? makeSearchTool(prepared.searchIndex) : lookup(root, path)?.tool - const execution = mode(original) - const rewrite = search || options.resolved ? undefined : hooks["tool.resolve"] - return Effect.acquireUseRelease( - Effect.sync(() => { - options.admit(name) - const input: unknown = args.length > 1 ? args : argument(args) - return { name, key: options.key, input } - }), - (call) => + const key = options.key + const input = yield* decode(name, tool, args) + yield* Effect.sync(() => options.admit(name)) + return yield* hooked( + { name, input, key }, + hooks["tool.before"], + hooks["tool.after"], Effect.gen(function* () { - if (args.length > 1) - throw new ToolRuntimeError("InvalidToolInput", `Tool '${name}' expects at most one input object.`) - // A replay must read the saved JSON even if a hook or the executor edits its input in place. - if (execution) call.input = structuredClone(call.input) - if (rewrite) { - const target = yield* failure(Effect.suspend(() => rewrite(call))) - call.name = target.name - call.input = target.input - } - const tool = (call.name === name ? original : undefined) ?? resolve(prepared, [call.name]) - if (execution?._tag !== mode(tool)?._tag) - throw new ToolRuntimeError( - "InvalidToolInput", - `Cannot redirect '${name}' to '${call.name}' across execution modes.`, - ) - if (rewrite && execution) { - if (call.input !== undefined && !Schema.is(Schema.Json)(call.input)) - throw new ToolRuntimeError( - "InvalidToolInput", - `Resolved input for resumable tool '${call.name}' must be JSON.`, - ) - options.save?.(call.name, structuredClone(call.input)) - } - call.input = yield* decode(call.name, tool, call.input) - const before = hooks["tool.before"] - if (before) yield* failure(Effect.suspend(() => before(call))) - return yield* run(tool, call) + const raw = yield* failure(Effect.suspend(() => run(input, { key }))) + return yield* output(name, tool, raw) }), - (call, exit) => { - if (options.detaching?.()) return Effect.void - // A durable start hands settlement to wait, including after resume. - if (execution?._tag === "CodeModeDurable" && Exit.isSuccess(exit)) { - if (hooks["tool.after"]) invocations.set(call.key, call) - return Effect.void - } - return hooks["tool.after"]?.(call, callResult(exit)) ?? Effect.void - }, - ) - } + options.detaching, + ) + }) const durableTool = ( path: ReadonlyArray, @@ -776,31 +716,55 @@ export const make = ( invocations, record: (name) => recordCall({ name }), keys: (path) => namespaceKeys(root, path), - search: (args, options) => attempt(["search"], args, options, invoke, true), + search: (args, options) => + Effect.suspend(() => executeTool("search", makeSearchTool(prepared.searchIndex), args, options)), execute: (path, args, options) => Effect.suspend(() => { const segments = canonicalSegments(path) // Models often write `tools.search(...)` for the bare `search(...)`; honor it unless a tool owns that path. - const search = segments.length === 1 && segments[0] === "search" && lookup(root, segments) === undefined - return attempt(segments, args, options, invoke, search) + if (segments.length === 1 && segments[0] === "search" && lookup(root, segments) === undefined) + return executeTool("search", makeSearchTool(prepared.searchIndex), args, options) + return executeTool(segments.join("."), resolve(root, path, prepared.searchIndex), args, options) + }), + mode: (path) => { + const segments = canonicalSegments(path) + if (segments.length === 1 && segments[0] === "search" && lookup(root, segments) === undefined) return undefined + const execute = lookup(root, segments)?.tool?.execute + return execute !== undefined && "_tag" in execute ? execute : undefined + }, + start: (path, args, key, admit) => + Effect.suspend(() => { + const found = durableTool(path) + return Effect.gen(function* () { + const input = yield* decode(found.name, found.tool, args) + yield* Effect.sync(() => admit(found.name)) + const invocation: ToolInvocation = { name: found.name, input, key } + invocations.set(key, invocation) + const before = hooks["tool.before"] + const after = hooks["tool.after"] + const run = failure(Effect.suspend(() => found.durable.start(input, { key }))).pipe( + Effect.flatMap((raw) => + Effect.try({ + try: (): Json => { + const text = JSON.stringify(raw) + return text === undefined ? null : JSON.parse(text) + }, + catch: (cause) => + new ToolRuntimeError("InvalidToolOutput", `Invalid start data from tool '${found.name}': ${cause}`), + }), + ), + ) + const observed = + after === undefined + ? run + : Effect.onError(run, (cause) => { + invocations.delete(key) + if (Cause.hasInterruptsOnly(cause)) return after(invocation, { status: "interrupted" }) + return after(invocation, { status: "failure", error: Cause.squash(cause) }) + }) + return yield* before === undefined ? observed : Effect.andThen(before(invocation), observed) + }) }), - mode: (path) => mode(lookup(root, canonicalSegments(path))?.tool), - start: (path, args, options) => - attempt(canonicalSegments(path), args, options, (tool, call) => - Effect.gen(function* () { - const durable = mode(tool) - if (durable?._tag !== "CodeModeDurable") throw new TypeError(`Tool '${call.name}' is not a durable tool.`) - const raw = yield* failure(Effect.suspend(() => durable.start(call.input, { key: call.key }))) - return yield* Effect.try({ - try: (): Json => { - const text = JSON.stringify(raw) - return text === undefined ? null : JSON.parse(text) - }, - catch: (cause) => - new ToolRuntimeError("InvalidToolOutput", `Invalid start data from tool '${call.name}': ${cause}`), - }) - }), - ), wait: (path, args, key, data, detaching) => Effect.suspend(() => { const found = durableTool(path) @@ -816,7 +780,7 @@ export const make = ( known !== undefined ? Effect.succeed(known) : Effect.map( - decode(found.name, found.tool, argument(args)), + decode(found.name, found.tool, args), (input): ToolInvocation => ({ name: found.name, input, @@ -827,7 +791,9 @@ export const make = ( Effect.onExit(run, (exit) => { if (detaching()) return Effect.void invocations.delete(key) - return after(call, callResult(exit)) + if (Exit.isSuccess(exit)) return after(call, { status: "success", value: exit.value }) + if (Cause.hasInterruptsOnly(exit.cause)) return after(call, { status: "interrupted" }) + return after(call, { status: "failure", error: Cause.squash(exit.cause) }) }), ) }), diff --git a/packages/codemode/test/suspend.test.ts b/packages/codemode/test/suspend.test.ts index 77d875c4920..7471f9b55d6 100644 --- a/packages/codemode/test/suspend.test.ts +++ b/packages/codemode/test/suspend.test.ts @@ -714,83 +714,6 @@ describe("resume", () => { }) describe("idempotent and durable tools", () => { - test("suspend during idempotent resolution retries the hook under its original key", async () => { - const entered = latch() - const keys: string[] = [] - const after: string[] = [] - const runtime = CodeMode.make({ - tools: { - read: Tool.make({ description: "Read", input: Value, output: Value, execute: Tool.idempotent(Effect.succeed) }), - }, - limits: { maxToolCalls: 1 }, - hooks: { - "tool.resolve": (call) => Effect.gen(function* () { - keys.push(call.key) - if (keys.length > 1) return call - entered.resolve() - return yield* Effect.never - }), - "tool.after": (_, result) => Effect.sync(() => void after.push(result.status)), - }, - }) - const run = background(runtime.program(`return await tools.read({ value: 7 })`, undefined, { runId: "policy" })) - await entered.promise - const saved = suspendedOf(await Effect.runPromise(run.suspend)) - expect(after).toEqual([]) - expect(await Effect.runPromise(background(runtime.resume(CodeMode.toBytes(saved.state))).result)).toEqual({ - ok: true, value: { value: 7 }, toolCalls: [{ name: "read" }], - }) - expect(keys).toEqual(["policy:0", "policy:0"]) - expect(after).toEqual(["success"]) - }) - - test("resume uses the resolved tools and inputs for replay and durable wait", async () => { - const read = idempotentManual() - const jobs = durableJobs() - const resolved: string[] = [] - const after: Array<[string, unknown, string]> = [] - const runtime = () => - CodeMode.make({ - tools: { oldRead: idempotentManual().tool, oldJob: durableJobs().tool, read: read.tool, job: jobs.tool }, - limits: { maxToolCalls: 2 }, - hooks: { - "tool.resolve": (call) => - Effect.sync(() => { - resolved.push(call.name) - return { name: call.name === "oldRead" ? "read" : "job", input: { value: 21 } } - }), - "tool.after": (call, result) => Effect.sync(() => void after.push([call.name, call.input, result.status])), - }, - }) - const run = background( - runtime().program(`return await Promise.all([tools.oldRead(), tools.oldJob()])`, undefined, { runId: "rewrite" }), - ) - await Promise.all([read.called(), jobs.waiting()]) - const saved = suspendedOf(await Effect.runPromise(run.suspend)) - expect(after).toEqual([]) - read.reset() - jobs.reset() - const resumed = background(runtime().resume(CodeMode.toBytes(saved.state))) - await Promise.all([read.called(), jobs.waiting()]) - expect(resolved).toEqual(["oldRead", "oldJob"]) - expect(read.attempts.map(({ key, input }) => [key, input])).toEqual([ - ["rewrite:0", 21], - ["rewrite:0", 21], - ]) - expect(jobs.log).toEqual(["start rewrite:1", "wait rewrite:1 rewrite:1", "wait rewrite:1 rewrite:1"]) - read.attempts[1]!.answer(21) - jobs.finish("rewrite:1", 21) - expect(await Effect.runPromise(resumed.result)).toEqual({ - ok: true, - value: [{ value: 21 }, { value: 21 }], - toolCalls: [{ name: "oldRead" }, { name: "oldJob" }], - }) - expect(after.toSorted(([a], [b]) => a.localeCompare(b))).toEqual([ - ["job", { value: 21 }, "success"], - ["read", { value: 21 }, "success"], - ]) - }) - test("an idempotent tool in flight can be suspended, is interrupted without firing after, and reruns on resume with the same key", async () => { const read = idempotentManual() const events: Array = [] diff --git a/packages/codemode/test/tool-paths.test.ts b/packages/codemode/test/tool-paths.test.ts index e1f812c0b97..a739d651510 100644 --- a/packages/codemode/test/tool-paths.test.ts +++ b/packages/codemode/test/tool-paths.test.ts @@ -77,102 +77,6 @@ describe("dotted tool names", () => { }) }) -describe("host resolution", () => { - test.each([false, true])( - "settles each admitted lookup, validation and veto failure (resolver: %s)", - async (resolve) => { - const settled: Array<[string, unknown, string]> = [] - const runtime = CodeMode.make({ - tools: { - "issues.list": Tool.make({ - description: "List issues", - input: Schema.Struct({ page: Schema.NumberFromString }), - execute: () => Effect.die("denied call reached the tool"), - }), - }, - hooks: { - ...(resolve && { "tool.resolve": (call: CodeMode.ToolInvocation) => Effect.succeed(call) }), - "tool.before": () => Effect.fail(new Error("policy denied this call")), - "tool.after": (call, result) => Effect.sync(() => void settled.push([call.name, call.input, result.status])), - }, - }) - const cases = [ - ["issues.get_list", "{}", "UnknownTool", "Did you mean tools.issues.list?"], - ["issues.list", "{ page: 1 }", "InvalidToolInput", "Expected string"], - ["issues.list", "{ page: '2' }", "ToolFailure", "policy denied this call"], - ] as const - for (const [name, args, kind, message] of cases) { - const result = await Effect.runPromise(runtime.execute(`await tools.${name}(${args})`)) - expect(result).toMatchObject({ - ok: false, - toolCalls: [{ name }], - error: { kind, message: expect.stringContaining(message) }, - }) - } - expect(settled).toEqual([ - ["issues.get_list", {}, "failure"], - ["issues.list", { page: 1 }, "failure"], - ["issues.list", { page: 2 }, "failure"], - ]) - }, - ) - - test("a resolver veto rejects the script promise as a tool failure", async () => { - const runtime = CodeMode.make({ hooks: { "tool.resolve": () => Effect.fail(new Error("unavailable here")) } }) - expect(await value(runtime, `return tools.missing().catch(e => e.message)`)).toBe("unavailable here") - expect(await failure(runtime, `await tools.missing()`)).toMatchObject({ - kind: "ToolFailure", - message: "unavailable here", - }) - }) - - test("rejects a redirect that would change how the call suspends", async () => { - const runtime = CodeMode.make({ - tools: { - read: Tool.make({ description: "Read", input: Schema.Struct({}), execute: Tool.idempotent(() => Effect.void) }), - }, - hooks: { "tool.resolve": () => Effect.succeed({ name: "read", input: {} }) }, - }) - expect(await failure(runtime, `await tools.missing()`)).toMatchObject({ - kind: "InvalidToolInput", - message: "Cannot redirect 'missing' to 'read' across execution modes.", - }) - }) - - test("reserves the last slot before an async rewrite and counts the attempted call once", async () => { - const attempts: string[] = [] - const calls = new Set() - const settled: string[] = [] - const runtime = CodeMode.make({ - tools: { available: echo("Available", "ok") }, - limits: { maxToolCalls: 1 }, - hooks: { - "tool.resolve": (call) => - Effect.gen(function* () { - attempts.push(call.name) - calls.add(call) - yield* Effect.yieldNow - return { name: "available", input: call.input } - }), - "tool.before": (call) => Effect.sync(() => void calls.add(call)), - "tool.after": (call, result) => - Effect.sync(() => { - calls.add(call) - settled.push(`${call.name}:${result.status}`) - }), - }, - }) - const result = await Effect.runPromise( - runtime.execute(`return await Promise.allSettled([tools.first(), tools.second()])`), - ) - expect(attempts).toEqual(["first"]) - expect(calls.size).toBe(1) - expect(settled).toEqual(["available:success"]) - expect(result.toolCalls).toEqual([{ name: "first" }]) - expect(result.ok && result.value).toMatchObject([{ status: "fulfilled", value: "ok" }, { status: "rejected" }]) - }) -}) - describe("callable namespaces", () => { const runtime = CodeMode.make({ tools: { issues: echo("All issues", "all"), "issues.list": echo("List issues", "list") }, diff --git a/packages/core/README.md b/packages/core/README.md index dac2d733805..7ea8334eae1 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -1,11 +1,3 @@ # @opencode/core Core runtime services for OpenCode. - -### Tool input repair - -`tool.execute.before` hooks run in registration order. The built-in input repair hook applies the selected tool's schema at its place in that order. For example, a later hook sees `{ count: 2 }` for an integer field supplied as `"2"`. It can inspect or edit that input, or reject the call before the handler runs. - -When a hook changes the tool name, Core prepares the arguments for the newly selected visible tool before calling the next hook. A name-only redirect uses the original arguments, so freeform text moves to the new tool's required string field. If a hook also edits the input, the edited value supplies the new arguments. Once repair has run, edits that keep the same tool go directly to validation. - -Inside Code Mode, scripts call catalog paths, such as `tools.msg["send-text"](text)`. A hook can redirect a call using either `msg.send-text` or its registration ID, `msg_send-text`. In a scheduled check the target must also be on the check's allowlist. Leaving an unknown path unchanged preserves the usual lookup error and suggestions. diff --git a/packages/core/src/codemode/tool.ts b/packages/core/src/codemode/tool.ts index 2d24511228b..4976c40ef3b 100644 --- a/packages/core/src/codemode/tool.ts +++ b/packages/core/src/codemode/tool.ts @@ -1,22 +1,19 @@ export * as CodeModeTool from "./tool.js" import { ToolFailure } from "@opencode/ai/effect" -import { CodeMode, Namespace, Tool, toolExpression } from "@opencode/codemode" +import { CodeMode, Namespace, Tool } from "@opencode/codemode" import type { Content, Context as ToolContext, + Error, Info, Metadata, Namespace as ToolNamespace, Result, } from "@opencode/schema/tool" -import { Error } from "@opencode/schema/tool" -import { isRecord } from "@opencode/util/record" import { Clock, Context, Effect, Ref, Schema, Semaphore } from "effect" -import { isDeepStrictEqual } from "node:util" import { toSessionError } from "../session/to-session-error.js" -import { aliasNote, definition, normalizedName } from "../tool/runtime.js" -import { prepare, type Target } from "../tool/prepare.js" +import { definition, normalizedName } from "../tool/runtime.js" import { Permission } from "../permission.js" import { CodeModeCatalog } from "./catalog.js" import { CodeModeExecution } from "./execution.js" @@ -60,19 +57,12 @@ type Tools = { [name: string]: Tool.Tool | Namespace.Namespace | Tools } -type Before = (name: string, input: unknown, context: C) => Effect.Effect<{ tool: string; input: unknown }, Error> - // Per-execution parts of a runtime shared by every execution against the same inventory. class Run extends Context.Service< Run, { - readonly executeTool: (name: string, tool: Info, input: unknown, key: string) => Effect.Effect - readonly before?: Before - readonly alias?: (key: string, note: string) => void - readonly progress?: ( - call: CodeMode.ToolInvocation | CodeMode.ExtensionInvocation, - status: ExecuteCall["status"], - ) => Effect.Effect + readonly executeTool: (name: string, tool: Info, input: unknown) => Effect.Effect + readonly hooks: CodeMode.Hooks } >()("@opencode/CodeModeTool/Run") {} @@ -96,7 +86,6 @@ export const create = ( inventory: Inventory, executeTool: (name: string, tool: Info, input: unknown, context: ToolContext) => Effect.Effect, permission: Permission.Interface, - before?: Before, ) => { return { name: "execute", @@ -109,8 +98,6 @@ export const create = ( const callIndex = yield* Ref.make(0) const files = yield* Ref.make>([]) const calls = yield* Ref.make>([]) - const pendingNotes = new Map() - const notes = new Set() const lock = Semaphore.makeUnsafe(1) // Each update replaces the last, so send at most one per 100 ms; a trailing one carries the latest rows. // A success drops a pending one when the scope closes, since the result carries the final rows. @@ -125,10 +112,7 @@ export const create = ( const record = (update: (items: Array) => Array) => lock.withPermit( Effect.gen(function* () { - const previous = yield* Ref.get(calls) - const next = update(previous) - if (previous === next) return - yield* Ref.set(calls, next) + yield* Ref.update(calls, update) if (yield* Ref.get(trailing)) return const wait = (yield* Ref.get(publishedAt)) + 100 - (yield* Clock.currentTimeMillis) if (wait <= 0) return yield* publish @@ -140,16 +124,8 @@ export const create = ( .execute(code) .pipe( Effect.provideService(Run, { - before: before && ((name, input) => before(name, input, context)), - alias: (key, note) => pendingNotes.set(key, note), - executeTool: (name, tool, input, key) => + executeTool: (name, tool, input) => Effect.gen(function* () { - // Resolution or validation can still reject a repair before the target runs. - const note = pendingNotes.get(key) - if (note !== undefined) { - pendingNotes.delete(key) - notes.add(note) - } const index = yield* Ref.getAndUpdate(callIndex, (index) => index + 1) // The program sees the same failure text the model gets for a direct call. const executed = yield* executeTool(name, tool, input, { @@ -167,7 +143,7 @@ export const create = ( yield* Ref.update(files, (items) => [...items, { index, files: outputFileParts }]) return resultValue(executed) }), - progress: progressRows(record), + hooks: progressHooks(record), }), // A failed or interrupted call keeps its last progress, so send pending rows before it settles. Effect.onError(() => @@ -181,15 +157,15 @@ export const create = ( const collected = (yield* Ref.get(files)) .toSorted((left, right) => left.index - right.index) .flatMap((item) => item.files) + const output = formatResult(result) const value: typeof ExecuteOutput.Type = { - output: formatResult(result), + output, toolCalls, files: collected, ...(result.ok ? {} : { error: true }), } const content: Array = [ { type: "text", text: value.output }, - ...Array.from(notes, (text) => ({ type: "text" as const, text })), ...value.files.map((file) => ({ type: "file" as const, uri: `data:${file.mime};base64,${file.data}`, @@ -210,20 +186,40 @@ export const create = ( } satisfies Info } -// The runtime carries one call object from admission to settlement, including across a repair. -function progressRows(record: (update: (items: Array) => Array) => Effect.Effect) { +// Rows appear in start order; the same call object arrives at both hooks, so a call finds its row again. +function progressHooks(record: (update: (items: Array) => Array) => Effect.Effect) { const rows = new WeakMap() - return (call: CodeMode.ToolInvocation | CodeMode.ExtensionInvocation, status: ExecuteCall["status"]) => { - if ("extension" in call && call.name !== "fetch") return Effect.void - const input = displayInput(call) - const entry = { tool: call.name, status, ...(input ? { input } : {}) } - return record((items) => { - const index = rows.get(call) - if (index !== undefined) return isDeepStrictEqual(items[index], entry) ? items : items.with(index, entry) + const start = (call: object, entry: ExecuteCall) => + record((items) => { rows.set(call, items.length) return [...items, entry] }) + const settle = (call: object, result: CodeMode.CallResult) => { + const index = rows.get(call) + if (index === undefined) return Effect.void + return record((items) => { + const next = [...items] + next[index] = { ...items[index], status: result.status === "success" ? "completed" : "error" } + return next + }) } + return { + "tool.before": (call) => { + const shown = displayInput(call.input) + return start(call, { tool: call.name, status: "running", ...(shown ? { input: shown } : {}) }) + }, + "tool.after": settle, + // Only listed extension functions get a row; anything else stays out of the TUI. + "extension.before": (call) => { + switch (call.name) { + case "fetch": + return start(call, { tool: call.name, status: "running", input: CodeModeFetch.display(call.args) }) + default: + return Effect.void + } + }, + "extension.after": settle, + } satisfies CodeMode.Hooks } export const catalog = (inventory: Inventory) => { @@ -279,10 +275,11 @@ const runtimes = new WeakMap({ - ...entries, + const tools = inventoryTools(inventory) + const created = CodeMode.make({ + tools, extensions: [CodeModeFetch.extension], + hooks: runHooks, }) runtimes.set(inventory, created) return created @@ -296,42 +293,36 @@ export const check = ( code: string, bindings: Readonly>, limits: CodeMode.ExecutionLimits, - executeTool: (name: string, tool: Info, input: unknown, key: string) => Effect.Effect, - before?: Before, + executeTool: (name: string, tool: Info, input: unknown) => Effect.Effect, ) => CodeMode.make({ - ...inventoryTools(inventory), + tools: inventoryTools(inventory), + hooks: runHooks, limits, }) .execute(code, bindings) .pipe( Effect.provideService(Run, { - before, - executeTool: (name, tool, input, key) => executeTool(name, tool, input, key).pipe(Effect.map(resultValue)), + executeTool: (name, tool, input) => executeTool(name, tool, input).pipe(Effect.map(resultValue)), + hooks: {}, }), ) function inventoryTools(inventory: Inventory) { // A path may carry namespace metadata, a callable tool, child tools, or all three. const root: ToolNode = { children: new Map() } - const paths = new Map() - const ids = new Map() for (const namespace of inventory.namespaces?.values() ?? []) getNode(root, namespace.name).namespace = namespace for (const [name, registration] of inventory.tools) { const child = definition(registration) - const path = qualifiedName(registration) - const target = { tool: registration, definition: child } - paths.set(path, target) - ids.set(name, target) - getNode(root, path).tool = Tool.make({ + getNode(root, qualifiedName(registration)).tool = Tool.make({ description: child.description, input: child.inputSchema, freeform: registration.freeform !== undefined, output: child.outputSchema ?? Schema.NullOr(Schema.String), - execute: (input, { key }) => Run.use((run) => run.executeTool(name, registration, input, key)), + execute: (input) => Run.use((run) => run.executeTool(name, registration, input)), }) } - return { tools: renderTools(root), hooks: runHooks(paths, ids) } + return renderTools(root) } function resultContent(executed: Result) { @@ -348,44 +339,12 @@ function resultValue(executed: Result) { return text === "" ? null : text } -function runHooks(paths: ReadonlyMap, ids: ReadonlyMap): CodeMode.Hooks { - const running = (call: CodeMode.ToolInvocation | CodeMode.ExtensionInvocation) => - Run.use((run) => run.progress?.(call, "running") ?? Effect.void) - const settled = (call: CodeMode.ToolInvocation | CodeMode.ExtensionInvocation, result: CodeMode.CallResult) => - Run.use((run) => run.progress?.(call, result.status === "success" ? "completed" : "error") ?? Effect.void) - return { - "tool.resolve": (call) => - Run.use((run) => - Effect.gen(function* () { - if (run.progress) yield* run.progress(call, "running") - const before = run.before - if (!before) return call - const original = paths.get(call.name) - const requested = original?.definition.name ?? call.name - const selected = yield* prepare( - requested, - call.input, - (name) => { - // The script must use a catalog path. Only a plugin redirect can select by registration ID. - return name === requested ? original : (ids.get(name) ?? paths.get(name)) - }, - (name, input) => before(name, input, call.key), - ) - if (!selected.target) { - if (selected.name === requested) return { name: call.name, input: selected.input } - return yield* new Error({ message: `Tool is not available for this request: ${selected.name}` }) - } - const name = qualifiedName(selected.target.tool) - if (original === undefined) run.alias?.(call.key, aliasNote(toolExpression(call.name), toolExpression(name))) - return { name, input: selected.input } - }), - ), - "tool.before": running, - "tool.after": settled, - "extension.before": running, - "extension.after": settled, - } -} +const runHooks = { + "tool.before": (call) => Run.use((run) => run.hooks["tool.before"]?.(call) ?? Effect.void), + "tool.after": (call, result) => Run.use((run) => run.hooks["tool.after"]?.(call, result) ?? Effect.void), + "extension.before": (call) => Run.use((run) => run.hooks["extension.before"]?.(call) ?? Effect.void), + "extension.after": (call, result) => Run.use((run) => run.hooks["extension.after"]?.(call, result) ?? Effect.void), +} satisfies CodeMode.Hooks function getNode(root: Node, path: string) { return path.split(".").reduce((parent, name) => { @@ -447,36 +406,12 @@ export function qualifiedName(registration: Info) { return `${registration.options.namespace}.${normalized}` } -function displayInput(call: CodeMode.ToolInvocation | CodeMode.ExtensionInvocation): ExecuteCall["input"] { - try { - if ("extension" in call) return CodeModeFetch.display(call.args) - const value = snapshot(call.input, new Set()) - if (value === undefined || value === null) return - if (!isRecord(value)) return { input: value } - return Object.keys(value).length ? value : undefined - } catch { - // An opaque plugin value can reject inspection. The status row is still useful without its input. - return undefined - } -} - -// Copy only data properties: displaying a plugin object must not invoke its getters or toJSON method. -function snapshot(value: unknown, parents: Set): Schema.Json | undefined { - if (value === null || typeof value === "string" || typeof value === "boolean") return value - if (typeof value === "number") return Number.isFinite(value) ? value : null - if (typeof value !== "object" || parents.has(value)) return - parents.add(value) - const property = (key: string | number) => snapshot(Object.getOwnPropertyDescriptor(value, key)?.value, parents) - const result = Array.isArray(value) - ? Array.from({ length: value.length }, (_, i) => property(i) ?? null) - : Object.fromEntries( - Object.keys(value).flatMap((key) => { - const item = property(key) - return item === undefined ? [] : [[key, item]] - }), - ) - parents.delete(value) - return result +// Tool inputs arrive as parsed JSON, so the JSON value cast is a boundary fact. +function displayInput(input: unknown): Record | undefined { + if (input === null || input === undefined) return + if (typeof input !== "object" || Array.isArray(input)) return { input: input as typeof Schema.Json.Type } + if (Object.keys(input).length === 0) return + return input as Record } function formatResult(result: CodeMode.Result) { diff --git a/packages/core/src/plugin/hooks.ts b/packages/core/src/plugin/hooks.ts index fa62caca550..1325837f9d5 100644 --- a/packages/core/src/plugin/hooks.ts +++ b/packages/core/src/plugin/hooks.ts @@ -55,7 +55,6 @@ export interface Interface { domain: Domain, name: Name, event: Domains[Domain][Name], - afterEach?: (event: Domains[Domain][Name]) => void, ) => Effect.Effect } @@ -86,12 +85,11 @@ const layer = Layer.effect( }, ) - const trigger: Interface["trigger"] = Effect.fnUntraced(function* (domain, name, event, afterEach) { + const trigger: Interface["trigger"] = Effect.fnUntraced(function* (domain, name, event) { for (const entry of callbacks.get(key(domain, name)) ?? []) { if (entry.options?.providerID !== undefined && entry.options.providerID !== eventProviderID(event)) continue const result: Effect.Effect = entry.callback(event) yield* result - afterEach?.(event) } return event }) diff --git a/packages/core/src/plugin/tool-input-repair.ts b/packages/core/src/plugin/tool-input-repair.ts index b9c88cd1de9..c2e1f03b362 100644 --- a/packages/core/src/plugin/tool-input-repair.ts +++ b/packages/core/src/plugin/tool-input-repair.ts @@ -1,9 +1,9 @@ export * as ToolInputRepairPlugin from "./tool-input-repair.js" +import { ToolFailure } from "@opencode/ai/effect" import { define } from "@opencode/plugin/effect/plugin" import { Effect, JsonSchema, Option, Predicate, Schema } from "effect" -import { Selection } from "../tool/prepare.js" -import { definition } from "../tool/runtime.js" +import { definition, normalizeContent } from "../tool/runtime.js" // Repairs apply only when the input schema unambiguously supports them: // - Stringified root or nested object: '{"limit":"20"}' -> { limit: 20 } @@ -27,31 +27,50 @@ export const Plugin = define({ id: "opencode.tool.input.repair", effect: (ctx) => Effect.gen(function* () { + const aliased = new Map() yield* ctx.tool.hook("execute.before", (event) => Effect.gen(function* () { - const selection = yield* Selection - if (selection) { - const alias = aliases.get(event.tool) - if (alias && !selection.has(event.tool) && selection.has(alias)) event.tool = alias - selection.repair(event, repairInput) + // The outer Code Mode tool is built per snapshot rather than registered, so it cannot be + // looked up here. Its `{ code }` input is trivial; the tools it calls are repaired normally. + if (event.tool === "execute") return + const tools = yield* ctx.tool.list() + const alias = aliases.get(event.tool) + const tool = + tools.find((tool) => tool.id === event.tool) ?? + (alias === undefined ? undefined : tools.find((tool) => tool.id === alias)) + if (!tool) return + if (tool.id !== event.tool) { + aliased.set(event.id, event.tool) + event.tool = tool.id + } + const schema = definition(tool).inputSchema + if (schema.type !== "object") return + event.input = repair(event.input, schema, schema, 0) + }), + ) + yield* ctx.tool.hook("execute.after", (event) => + Effect.sync(() => { + const called = aliased.get(event.id) + if (called === undefined) return + aliased.delete(event.id) + const note = `There is no "${called}" tool, so this ran as "${event.tool}". Call "${event.tool}" directly.` + if (event.status === "error") { + event.error = new ToolFailure({ + message: `${event.error.message}\n\n${note}`, + error: event.error.error, + metadata: event.error.metadata, + }) return } - // Standalone hook callers have no selection phase. - const tools = yield* ctx.tool.list() - const tool = - tools.find((tool) => tool.id === event.tool) ?? tools.find((tool) => tool.id === aliases.get(event.tool)) - if (tool) { - event.tool = tool.id - event.input = repairInput(event.input, definition(tool).inputSchema) + event.result = { + ...event.result, + content: [...normalizeContent(event.result.content, event.result.output), { type: "text", text: note }], } }), ) }), }) -const repairInput = (input: unknown, schema: JsonSchema.JsonSchema) => - schema.type === "object" ? repair(input, schema, schema, 0) : input - function repair(value: unknown, schema: JsonSchema.JsonSchema, root: JsonSchema.JsonSchema, depth: number): unknown { if (depth > maxDepth) return value diff --git a/packages/core/src/tool.ts b/packages/core/src/tool.ts index b4b9281acfe..30d071beb3c 100644 --- a/packages/core/src/tool.ts +++ b/packages/core/src/tool.ts @@ -2,10 +2,10 @@ export * as Tool from "./tool.js" export { CallID, Content, Error, FileContent, TextContent } from "@opencode/schema/tool" export type { Context, Metadata, Namespace, Options, Result } from "@opencode/schema/tool" -import { ToolDefinition, ToolFailure, type ToolCall } from "@opencode/ai/effect" +import { ToolDefinition, type ToolCall } from "@opencode/ai/effect" import type { CodeMode } from "@opencode/codemode" import { Tool } from "@opencode/schema/tool" -import { Context, Effect, Layer, Result, Schema, SchemaIssue, Types } from "effect" +import { Context, Effect, Layer, Ref, Result, Schema, SchemaIssue, Types } from "effect" import { makeLocationNode } from "@opencode/util/effect/app-node" import type { Agent } from "./agent.js" import { CodeModeCatalog } from "./codemode/catalog.js" @@ -16,8 +16,7 @@ import { PluginHooks } from "./plugin/hooks.js" import { SessionMessage } from "./session/message.js" import { SessionSchema } from "./session/schema.js" import { State } from "./state.js" -import { aliasNote, definition, effectiveName, execute, normalizedName, normalizeContent } from "./tool/runtime.js" -import { prepare, Selection, type Target } from "./tool/prepare.js" +import { definition, effectiveName, execute, normalizedName, normalizeContent } from "./tool/runtime.js" import { Wildcard } from "./util/wildcard.js" export class RegistrationError extends Schema.TaggedError()("Tool.RegistrationError", { @@ -121,29 +120,21 @@ const layer = Layer.effect( ] }) - const beforeExecute = Effect.fnUntraced(function* (name: string, input: unknown, context: Tool.Context) { - const selection = yield* Selection - return yield* hooks.trigger( - "tool", - "execute.before", - { - tool: name, - sessionID: context.sessionID, - agent: context.agent, - messageID: context.messageID, - id: context.id, - input, - }, - selection?.redirected, - ) - }) + const beforeExecute = (name: string, input: unknown, context: Tool.Context) => + hooks.trigger("tool", "execute.before", { + tool: name, + sessionID: context.sessionID, + agent: context.agent, + messageID: context.messageID, + id: context.id, + input, + }) const executeTool = Effect.fn("Tool.execute")(function* ( tool: Tool.Info, name: string, input: unknown, context: Tool.Context, - note?: string, ) { yield* Effect.annotateCurrentSpan({ "gen_ai.tool.name": name, @@ -170,8 +161,6 @@ const layer = Layer.effect( error: execution.failure, } yield* hooks.trigger("tool", "execute.after", afterEvent) - if (note) - return yield* new ToolFailure({ ...afterEvent.error, message: `${afterEvent.error.message}\n\n${note}` }) return yield* afterEvent.error } const afterEvent: PluginHooks.Domains["tool"]["execute.after"] = { @@ -187,11 +176,14 @@ const layer = Layer.effect( const afterContent = yield* normalizeImages(normalizeContent(afterEvent.result.content, afterEvent.result.output)) return { ...(afterEvent.result.output === undefined ? {} : { output: afterEvent.result.output }), - content: note ? [...afterContent, { type: "text" as const, text: note }] : afterContent, + content: afterContent, ...(afterEvent.result.metadata === undefined ? {} : { metadata: afterEvent.result.metadata }), } }) + const run = (tool: Tool.Info, name: string, input: unknown, context: Tool.Context) => + beforeExecute(name, input, context).pipe(Effect.flatMap((event) => executeTool(tool, name, event.input, context))) + let catalog: | { data: Data @@ -283,22 +275,14 @@ const layer = Layer.effect( const codeModeTool = codeModeEnabled ? CodeModeTool.create( codeModeInventory, - (name, tool, input, context) => executeTool(tool, name, input, context), + (name, tool, input, context) => run(tool, name, input, context), permission, - beforeExecute, ) : undefined const codeModeCatalog = !codeModeEnabled ? undefined : (cached?.value ?? CodeModeTool.catalog(codeModeInventory)) const codeModeDefinition = codeModeTool && (cached?.definition ?? definition(codeModeTool)) - const targets = new Map( - Array.from(direct) - .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0)) - .map(([name, tool]) => [name, { tool, definition: data.definitions.get(name)! }]), - ) - if (codeModeTool && codeModeDefinition) - targets.set(codeModeTool.name, { tool: codeModeTool, definition: codeModeDefinition }) if (codeModeCatalog && codeModeDefinition) catalog = { data, @@ -309,7 +293,12 @@ const layer = Layer.effect( } return { ...(codeModeCatalog === undefined ? {} : { codeModeCatalog }), - definitions: Array.from(targets.values(), (target) => target.definition), + definitions: [ + ...Array.from(direct) + .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0)) + .map(([name]) => data.definitions.get(name)!), + ...(codeModeDefinition ? [codeModeDefinition] : []), + ], execute: Effect.fnUntraced(function* (input: Parameters[0]) { const context: Tool.Context = { sessionID: input.sessionID, @@ -319,32 +308,18 @@ const layer = Layer.effect( progress: input.progress ?? (() => Effect.void), cancel: input.cancel, } - const lookup = (advertised: string) => { - const name = input.definitions ? input.definitions.get(advertised)?.name : advertised - if (name === undefined) return - return targets.get(name) - } - const called = input.call.name - const selected = yield* prepare(called, input.call.input, lookup, (name, args) => - beforeExecute(name, args, context), - ) - if (selected.target) { - const note = - called !== selected.name && !lookup(called) - ? aliasNote(JSON.stringify(called), JSON.stringify(selected.name)) - : undefined - return yield* executeTool( - selected.target.tool, - selected.target.definition.name, - selected.input, - context, - note, - ) - } - if (input.definitions && targets.has(selected.name)) - return yield* new Tool.Error({ message: `Tool is not available for this request: ${selected.name}` }) + const event = yield* beforeExecute(input.call.name, input.call.input, context) + const requested = input.definitions?.get(event.tool) + // Preserve session context removal and alias resolution, now after the repair hook. + if (!requested && input.definitions && (direct.has(event.tool) || codeModeTool?.name === event.tool)) + return yield* new Tool.Error({ message: `Tool is not available for this request: ${event.tool}` }) + const name = requested?.name ?? event.tool + if (name === "execute" && codeModeTool) + return yield* executeTool(codeModeTool, name, event.input, context) + const tool = direct.get(name) + if (tool) return yield* executeTool(tool, name, event.input, context) return yield* new Tool.Error({ - message: `No tool named "${selected.name}" is currently available. Please use a tool from the available tool list.`, + message: `No tool named "${name}" is currently available. Please use a tool from the available tool list.`, }) }), } @@ -368,17 +343,21 @@ const layer = Layer.effect( !whollyDisabled(tool.options?.permission ?? name, input.permissions), ), ) - const context = (key: string): Tool.Context => ({ - ...input.context, - id: Tool.CallID.make(`call_${input.context.messageID}_${key}`), - }) + const callSeq = yield* Ref.make(0) return yield* CodeModeTool.check( { tools, namespaces: data.namespaces }, input.code, input.bindings, input.limits, - (name, tool, toolInput, key) => executeTool(tool, name, toolInput, context(key)), - (name, toolInput, key) => beforeExecute(name, toolInput, context(key)), + (name, tool, toolInput) => + Effect.gen(function* () { + const index = yield* Ref.getAndUpdate(callSeq, (value) => value + 1) + const context: Tool.Context = { + ...input.context, + id: Tool.CallID.make(`call_${input.context.messageID}_${index}`), + } + return yield* run(tool, name, toolInput, context) + }), ) }), }) diff --git a/packages/core/src/tool/prepare.ts b/packages/core/src/tool/prepare.ts deleted file mode 100644 index 612dc25e9df..00000000000 --- a/packages/core/src/tool/prepare.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { ToolDefinition } from "@opencode/ai/effect" -import { Tool } from "@opencode/schema/tool" -import { Context, Effect, Result } from "effect" -import { isDeepStrictEqual } from "node:util" - -type Repair = (input: unknown, schema: ToolDefinition["inputSchema"]) => unknown -type Draft = { tool: string; input: unknown } - -export type Target = { readonly tool: Tool.Info; readonly definition: ToolDefinition } - -// Each invocation resolves names against its own visible registrations. -export const Selection = Context.Reference< - | { - readonly has: (name: string) => boolean - readonly repair: (event: Draft, repair: Repair) => void - readonly redirected: (event: Draft) => void - } - | undefined ->("@opencode/Tool/Selection", { defaultValue: () => undefined }) - -export const prepare = Effect.fnUntraced(function* ( - requested: string, - input: unknown, - lookup: (name: string) => Target | undefined, - before: (name: string, input: unknown) => Effect.Effect, -) { - let repair: Repair | undefined - const select = (name: string, source: unknown) => { - const target = lookup(name) - const schema = target?.definition.inputSchema - const field = - schema && target.definition.freeform && typeof source === "string" - ? ToolDefinition.freeformField(schema) - : undefined - const wrapped = field === undefined ? source : { [field]: source } - const input = repair && schema ? repair(wrapped, schema) : wrapped - // Repair can share nested objects with the source. Snapshot only transformed values - // so an in-place hook edit can be distinguished from a name-only redirect. - const baseline = input === source ? undefined : Result.try(() => structuredClone(input)) - return { name, source, input, baseline } - } - let selected = select(requested, input) - const reselect = (event: Draft) => { - // A name-only redirect starts from the original arguments: the old schema may have - // dropped fields or wrapped text under a different key. Explicit edits take precedence. - const unchanged = - event.input === selected.input && - selected.baseline && - Result.isSuccess(selected.baseline) && - isDeepStrictEqual(event.input, selected.baseline.success) - selected = select(event.tool, unchanged ? selected.source : event.input) - event.input = selected.input - } - const event = yield* before(requested, selected.input).pipe( - Effect.provideService(Selection, { - has: (name) => lookup(name) !== undefined, - repair: (event, fn) => { - repair = fn - reselect(event) - }, - redirected: (event) => { - if (event.tool !== selected.name) reselect(event) - }, - }), - ) - // A later hook that replaces the prepared object must supply the handler's input shape. - // In particular a bare string here must not be interpreted again as Code Mode shorthand. - const target = lookup(event.tool) - if ( - typeof event.input === "string" && - target?.definition.freeform && - ToolDefinition.freeformField(target.definition.inputSchema) !== undefined - ) - return yield* new Tool.Error({ message: `Expected an input object for freeform tool ${event.tool}` }) - return { name: event.tool, target, input: event.input } -}) diff --git a/packages/core/src/tool/runtime.ts b/packages/core/src/tool/runtime.ts index ce0b8f4ae84..5e41681abca 100644 --- a/packages/core/src/tool/runtime.ts +++ b/packages/core/src/tool/runtime.ts @@ -7,9 +7,6 @@ import { $ZodType, toJSONSchema } from "zod/v4/core" const formatEffectIssues = SchemaIssue.makeFormatterStandardSchemaV1() -export const aliasNote = (called: string, target: string) => - `There is no ${called} tool, so this ran as ${target}. Call ${target} directly.` - // Codecs for raw JSON Schema inputs (mostly MCP tools), built once per schema object; schemas are not edited after // registration. Not an Effect `Cache`: each entry keeps its lookup fiber, which keeps the caller's stack frames and // through them the `Effect.fn` closures over its Location's services, so evicted Locations stayed alive. diff --git a/packages/core/test/plugin/tool-input-repair-integration.test.ts b/packages/core/test/plugin/tool-input-repair-integration.test.ts index f27aae35274..38578f105c8 100644 --- a/packages/core/test/plugin/tool-input-repair-integration.test.ts +++ b/packages/core/test/plugin/tool-input-repair-integration.test.ts @@ -1,5 +1,5 @@ import { expect } from "bun:test" -import { Deferred, Effect, Predicate, Schema } from "effect" +import { Effect, Schema } from "effect" import { Agent } from "@opencode/core/agent" import { Plugin } from "@opencode/core/plugin" import { ToolInputRepairPlugin } from "@opencode/core/plugin/tool-input-repair" @@ -21,20 +21,8 @@ it.effect("repairs tool input before validating its original schema", () => const plugins = yield* Plugin.Service const registry = yield* Tool.Service const executed: unknown[] = [] - const observed: unknown[] = [] yield* plugins.activate([ { ...ToolInputRepairPlugin.Plugin, revision: "1" }, - { - id: "inspect-count", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - Effect.sync(() => { - observed.push(event.input) - if (event.id === "call-edited") event.input = { count: "5", enabled: true } - }), - ), - }, { id: "repairable-tool", revision: "1", @@ -61,16 +49,6 @@ it.effect("repairs tool input before validating its original schema", () => }, }) expect(executed).toEqual([{ count: 2, enabled: true }]) - expect(observed).toEqual(executed) - const error = yield* snapshot - .execute({ - ...identity, - call: { type: "tool-call", id: "call-edited", name: "repairable", input: { count: "3", enabled: true } }, - }) - .pipe(Effect.flip) - expect(error.message).toContain("count") - expect(observed.at(-1)).toEqual({ count: 3, enabled: true }) - expect(executed).toHaveLength(1) yield* registry.transform((draft) => { draft.update("repairable", (tool) => { @@ -92,26 +70,6 @@ it.effect("repairs tool input before validating its original schema", () => { count: false, enabled: true }, ]) - // An advertised name can differ from the registry ID. Resolve it against the original snapshot. - const advertised = new Map( - snapshot.definitions.filter((tool) => tool.name === "repairable").map((tool) => ["counter", tool]), - ) - yield* snapshot.execute({ - ...identity, - definitions: advertised, - call: { type: "tool-call", id: "call-advertised", name: "counter", input: { count: "4", enabled: "true" } }, - }) - expect(executed.at(-1)).toEqual({ count: 4, enabled: true }) - expect( - (yield* snapshot - .execute({ - ...identity, - definitions: advertised, - call: { type: "tool-call", id: "call-original", name: "repairable", input: {} }, - }) - .pipe(Effect.flip)).message, - ).toBe("Tool is not available for this request: repairable") - yield* registry.transform((draft) => draft.remove("repairable")) const removed = yield* registry.snapshot() expect( @@ -166,411 +124,32 @@ it.effect("runs a call to another harness's tool name as the matching tool and s }), ) -it.effect("lets a later hook decide whether the nested echo handler receives the repaired count", () => +it.effect("repairs namespaced inner tool input called from Code Mode", () => Effect.gen(function* () { const plugins = yield* Plugin.Service const registry = yield* Tool.Service const executed: unknown[] = [] - yield* plugins.activate([ - { ...ToolInputRepairPlugin.Plugin, revision: "1" }, - { - id: "inspect-echo", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - event.tool === "example_echo" && Predicate.isObject(event.input) && event.input.count === 4 - ? Effect.fail(new Tool.Error({ message: "skip this count" })) - : Effect.void, - ), - }, - ]) - yield* registry.transform((draft) => - draft.add({ - name: "echo", - options: { namespace: "example" }, - description: "Echo a count", - input: Schema.Struct({ count: Schema.Int }), - execute: (input) => Effect.sync(() => executed.push(input)).pipe(Effect.as({ content: String(input.count) })), - }), - ) - - const snapshot = yield* registry.snapshot() - const result = yield* program( - snapshot, - `const count = await tools.example.echo({ count: "3" }); - try { await tools.example.echo({ count: "4" }) } catch (error) { return [count, error.message] }`, - ) - expect(result.output).toMatchObject({ output: expect.stringContaining("skip this count") }) - expect(executed).toEqual([{ count: 3 }]) - }), -) - -for (const order of ["before", "after"] as const) { - it.effect(`prepares the selected tool's final input when a redirect is registered ${order} built-in repair`, () => - Effect.gen(function* () { - const plugins = yield* Plugin.Service - const registry = yield* Tool.Service - const raw = ' { "text": "héllo", "extra": 1 }\n' - const calls: string[] = [] - const ran: unknown[] = [] - const observed: unknown[] = [] - const rewrite: Plugin.Generation = { - id: "redirect", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - Effect.sync(() => { - calls.push(event.tool) - if (event.tool === "draft" && Predicate.isObject(event.input) && event.input.body === "edit") { - delete event.input.body - event.input.text = "in-place edit" - } - if (event.tool === "post" || event.tool === "draft") event.tool = "msg.send-text" - if (event.tool === "replace") { - event.tool = "msg_send-text" - event.input = { text: "edited" } - } - if (event.tool === "countByPath") event.tool = "msg.count" - if (event.tool === "countByID") { - event.tool = "msg_count" - event.input = { count: "7" } - } - }), - ), - } - const repair = { ...ToolInputRepairPlugin.Plugin, revision: "1" } - yield* plugins.activate([ - ...(order === "before" ? [rewrite, repair] : [repair, rewrite]), - { - id: "inspect-selected-input", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - Effect.sync(() => { - if (event.tool !== "execute") observed.push(event.input) - }), - ), - }, - ]) - yield* registry.transform((draft) => { - draft.add({ - name: "draft", - description: "Draft", - freeform: { type: "text" }, - input: Schema.Struct({ body: Schema.String }), - execute: () => Effect.die("redirected"), - }) - draft.add({ - name: "send-text", - options: { namespace: "msg" }, - description: "Send", - freeform: { type: "text" }, - input: Schema.Struct({ text: Schema.String }), - execute: ({ text }) => - Effect.sync(() => { - ran.push(text) - return { content: text } - }), - }) - draft.add({ - name: "count", - options: { namespace: "msg" }, - description: "Count", - input: Schema.Struct({ count: Schema.Int }), - execute: ({ count }) => - Effect.sync(() => { - ran.push(count) - return { content: String(count) } - }), - }) - }) - const snapshot = yield* registry.snapshot() - const text = JSON.stringify(raw) - const result = yield* program( - snapshot, - `return [await tools.msg["send-text"](${text}), await tools.post(${text}), await tools.draft(${text}), await tools.replace(${text}), await tools.draft("edit")]`, - ) - expect(result.output).toMatchObject({ output: JSON.stringify([raw, raw, raw, "edited", "in-place edit"]) }) - expect(result.content).toContainEqual({ - type: "text", - text: 'There is no tools.post tool, so this ran as tools.msg["send-text"]. Call tools.msg["send-text"] directly.', - }) - const checked = yield* registry.check({ - code: 'return [await tools.countByPath({count:"3"}), await tools.countByID({})]', - tools: ["msg.count"], - bindings: {}, - permissions: [], - limits: {}, - context: { ...identity, progress: () => Effect.void }, - }) - expect(checked).toMatchObject({ ok: true, value: ["3", "7"] }) - expect(calls).toEqual([ - "execute", - "msg_send-text", - "post", - "draft", - "replace", - "draft", - "countByPath", - "countByID", - ]) - expect(ran).toEqual([raw, raw, raw, "edited", "in-place edit", 3, 7]) - expect(observed).toEqual([ - { text: raw }, - { text: raw }, - { text: raw }, - { text: "edited" }, - { text: "in-place edit" }, - { count: 3 }, - { count: 7 }, - ]) - }), - ) -} - -function program(snapshot: Tool.Snapshot, code: string) { - return snapshot.execute({ - ...identity, - call: { type: "tool-call", id: "call-program", name: "execute", input: { code } }, - }) -} - -it.effect("validates a hook's final freeform edit as handler input", () => - Effect.gen(function* () { - const plugins = yield* Plugin.Service - const registry = yield* Tool.Service - yield* plugins.activate([ - { ...ToolInputRepairPlugin.Plugin, revision: "1" }, - { - id: "edit-echo", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - Effect.sync(() => { - if (event.tool === "echo") event.input = "edited" - }), - ), - }, - ]) - const echoed: string[] = [] - yield* registry.transform((draft) => - draft.add({ - name: "echo", - description: "Echo text", - freeform: { type: "text" }, - input: Schema.Struct({ text: Schema.String }), - execute: ({ text }) => Effect.sync(() => echoed.push(text)).pipe(Effect.as({ content: text })), - }), - ) - const result = yield* program( - yield* registry.snapshot(), - 'try { await tools.echo("hello") } catch (error) { return error.message }', - ) - expect(result.output).toMatchObject({ output: expect.stringContaining("Expected an input object") }) - expect(echoed).toEqual([]) - }), -) - -const redirected = Effect.gen(function* () { - const plugins = yield* Plugin.Service - const registry = yield* Tool.Service - const before: string[] = [] - const executed: string[] = [] - yield* plugins.activate([ - { - id: "redirect-tool", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - Effect.sync(() => { - if (event.tool === "execute") return - before.push(event.tool) - if (event.tool !== "example.missing" && event.tool !== "example_original") return - event.tool = "example_target" - event.input = { count: 7 } - }), - ), - }, - ]) - yield* registry.transform((draft) => { - for (const name of ["original", "target"]) { - draft.add({ - name, - options: { namespace: "example" }, - description: name, - input: Schema.Struct({ count: Schema.Int }), - execute: ({ count }, ctx) => - Effect.sync(() => { - executed.push(name) - return { content: `${ctx.id}:${count}` } - }), - }) - } - }) - return { registry, snapshot: yield* registry.snapshot(), before, executed } -}) - -for (const [path, hookName] of [ - ["example.missing", "example.missing"], - ["example.original", "example_original"], -]) { - it.effect(`redirects tools.${path} to a visible namespaced target before validation`, () => - Effect.gen(function* () { - const fixture = yield* redirected - const result = yield* program(fixture.snapshot, `return await tools.${path}({})`) - expect(result.output).toMatchObject({ output: "call-program:7" }) - expect(result.metadata).toEqual({ - toolCalls: [{ tool: "example.target", input: { count: 7 }, status: "completed" }], - }) - expect(fixture.before).toEqual([hookName]) - expect(fixture.executed).toEqual(["target"]) - }), - ) -} - -it.effect("records an unknown underscore spelling when the program catches the lookup error", () => - Effect.gen(function* () { - const fixture = yield* redirected - const result = yield* program( - fixture.snapshot, - "try { await tools.example_target({ count: 4 }) } catch (e) { return e.message }", - ) - expect(result.output).toMatchObject({ - output: expect.stringContaining("Unknown tool 'example_target'"), - }) - expect(result.metadata).toEqual({ toolCalls: [{ tool: "example_target", input: { count: 4 }, status: "error" }] }) - expect(fixture.before).toEqual(["example_target"]) - expect(fixture.executed).toEqual([]) - }), -) - -it.effect("a program can catch a plugin veto and return successfully with one failed tool call", () => - Effect.gen(function* () { - const plugins = yield* Plugin.Service - const registry = yield* Tool.Service - yield* plugins.activate([ - { - id: "deny-tool", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - event.tool === "blocked" ? Effect.fail(new Tool.Error({ message: "denied by plugin" })) : Effect.void, - ), - }, - ]) - const executed: unknown[] = [] - yield* registry.transform((draft) => - draft.add({ - name: "blocked", - description: "Blocked tool", - input: Schema.Struct({ value: Schema.Int }), - execute: (input) => Effect.sync(() => executed.push(input)).pipe(Effect.as({ content: "unexpected" })), - }), - ) - const result = yield* program( - yield* registry.snapshot(), - "try { await tools.blocked({ value: 2 }) } catch (e) { return e.message }", - ) - expect(result.output).toMatchObject({ output: expect.stringContaining("denied by plugin") }) - expect(result.metadata).toEqual({ toolCalls: [{ tool: "blocked", input: { value: 2 }, status: "error" }] }) - const checked = yield* registry.check({ - code: "return await tools.blocked({ value: 2 })", - tools: ["blocked"], - bindings: {}, - permissions: [], - limits: {}, - context: { ...identity, progress: () => Effect.void }, - }) - expect(checked).toMatchObject({ - ok: false, - error: { kind: "ToolFailure", message: expect.stringContaining("denied by plugin") }, - }) - expect(executed).toEqual([]) - }), -) - -it.effect("rejects a redirect outside a scheduled check's allowlist", () => - Effect.gen(function* () { - const fixture = yield* redirected - const result = yield* fixture.registry.check({ - code: "return await tools.example.original({ count: 1 })", - tools: ["example.original"], - bindings: {}, - permissions: [], - limits: {}, - context: { ...identity, progress: () => Effect.void }, - }) - expect(result.ok).toBe(false) - expect(fixture.before).toEqual(["example_original"]) - expect(fixture.executed).toEqual([]) - }), -) - -it.effect("keeps a direct-only shell hidden from Code Mode alias repair", () => - Effect.gen(function* () { - const plugins = yield* Plugin.Service - const registry = yield* Tool.Service - const executed: string[] = [] yield* plugins.activate([{ ...ToolInputRepairPlugin.Plugin, revision: "1" }]) yield* registry.transform((draft) => draft.add({ - name: "shell", - options: { codemode: false }, - description: "Shell", - input: Schema.Struct({ command: Schema.String }), - execute: ({ command }) => Effect.sync(() => executed.push(command)).pipe(Effect.as({ content: "done" })), + name: "count", + options: { namespace: "example" }, + description: "Record a count", + input: Schema.Struct({ count: Schema.Int }), + execute: (input) => Effect.sync(() => executed.push(input)).pipe(Effect.as({ content: "ok" })), }), ) - const result = yield* program(yield* registry.snapshot(), 'return await tools.bash({ command: "pwd" })') - expect(result.output).toMatchObject({ error: true }) - expect(executed).toEqual([]) - }), -) -it.effect("keeps parallel text results clean and reports the alias on the outer execute result", () => - Effect.gen(function* () { - const plugins = yield* Plugin.Service - const registry = yield* Tool.Service - const siblingFinished = yield* Deferred.make() - yield* plugins.activate([ - { ...ToolInputRepairPlugin.Plugin, revision: "1" }, - { - id: "wait-for-sibling", - revision: "1", - effect: (ctx) => - ctx.tool.hook("execute.before", (event) => - event.tool === "shell" ? Deferred.await(siblingFinished) : Effect.void, - ), - }, - ]) - yield* registry.transform((draft) => { - draft.add({ - name: "shell", - description: "Shell", - input: Schema.Struct({}), - execute: () => Effect.succeed({ content: "shell text" }), - }) - draft.add({ - name: "sibling", - description: "Sibling", - input: Schema.Struct({}), - execute: () => Deferred.succeed(siblingFinished, undefined).pipe(Effect.as({ content: "sibling text" })), - }) - }) const snapshot = yield* registry.snapshot() - const result = yield* program(snapshot, "return await Promise.all([tools.bash({}), tools.sibling({})])") - expect(result.output).toMatchObject({ output: '["shell text","sibling text"]' }) - expect(result.metadata).toEqual({ - toolCalls: [ - { tool: "shell", status: "completed" }, - { tool: "sibling", status: "completed" }, - ], + yield* snapshot.execute({ + ...identity, + call: { + type: "tool-call", + id: "call-codemode-repair", + name: "execute", + input: { code: 'return await tools.example.count({ count: "3" })' }, + }, }) - expect(result.content).toContainEqual({ - type: "text", - text: "There is no tools.bash tool, so this ran as tools.shell. Call tools.shell directly.", - }) - const direct = yield* program(snapshot, "return await tools.shell({})") - expect(direct.content).toEqual([{ type: "text", text: "shell text" }]) + expect(executed).toEqual([{ count: 3 }]) }), ) diff --git a/packages/core/test/tool-execute.test.ts b/packages/core/test/tool-execute.test.ts index 4aa0ab68b4a..b5d7880129b 100644 --- a/packages/core/test/tool-execute.test.ts +++ b/packages/core/test/tool-execute.test.ts @@ -501,98 +501,6 @@ const progressCodeMode = (started: Deferred.Deferred, wait: Effect.Effect< const row = (tool: string, status: string) => ({ tool, status }) -it.effect("execute returns a caught argument error with JSON-safe progress and output", () => - Effect.gen(function* () { - const progress: Array = [] - const result = yield* execute( - createCodeMode(new Map()), - { code: 'try { await tools.echo(undefined, "extra") } catch (e) { return e.message }' }, - { ...context, progress: (update) => Effect.sync(() => progress.push(update)) }, - ) - const toolCalls = [{ tool: "echo", status: "error", input: { input: [null, "extra"] } }] - expect(progress).toStrictEqual([{ toolCalls }]) - expect(result.metadata).toStrictEqual({ toolCalls }) - expect(result.output).toEqual({ - output: expect.stringContaining("expects at most one input object"), - toolCalls, - files: [], - }) - }), -) - -it.effect("execute snapshots plugin inputs while passing optional and noncloneable values to the tool", () => - Effect.gen(function* () { - const input = { optional: undefined, record: { keep: "before", skip: undefined }, callback: () => "ok" } - const tool: Info = { - name: "inspect", - description: "Inspect plugin input", - input: Schema.Unknown, - execute: (received) => { - expect(received).toBe(input) - return Effect.succeed({ content: input.callback() }) - }, - } - const mode = CodeModeTool.create( - { tools: new Map([[tool.name, tool]]) }, - (_, registration, input, context) => execute(registration, input, context), - allowAll, - (tool) => Effect.succeed({ tool, input }), - ) - const result = yield* execute(mode, { code: "return await tools.inspect({})" }, context) - input.record.keep = "after" - expect(result.metadata).toStrictEqual({ - toolCalls: [{ tool: "inspect", status: "completed", input: { record: { keep: "before" } } }], - }) - expect(result.content).toEqual([{ type: "text", text: "ok" }]) - }), -) - -it.effect("publishes one running row for an unchanged call while the tool waits", () => - Effect.gen(function* () { - const started = yield* Deferred.make() - const progress: Array = [] - const fiber = yield* progressCodeMode(started, Effect.never) - .execute( - { code: "await tools.wait({})" }, - { - ...context, - progress: (update) => Effect.sync(() => progress.push(update)), - }, - ) - .pipe(Effect.forkChild({ startImmediately: true })) - yield* Deferred.await(started) - yield* TestClock.adjust("100 millis") - expect(progress).toEqual([{ toolCalls: [row("wait", "running")] }]) - yield* Fiber.interrupt(fiber) - }), -) - -it.effect("interrupting execute during asynchronous repair settles the admitted row", () => - Effect.gen(function* () { - const repairing = yield* Deferred.make() - const progress: Array = [] - const codeMode = CodeModeTool.create( - { tools: new Map() }, - () => Effect.die("unexpected handler"), - allowAll, - () => Deferred.succeed(repairing, undefined).pipe(Effect.andThen(Effect.never)), - ) - const fiber = yield* codeMode - .execute( - { code: "await tools.unknown()" }, - { - ...context, - progress: (update) => Effect.sync(() => progress.push(update)), - }, - ) - .pipe(Effect.forkChild({ startImmediately: true })) - yield* Deferred.await(repairing) - expect(progress).toEqual([{ toolCalls: [row("unknown", "running")] }]) - yield* Fiber.interrupt(fiber) - expect(progress.at(-1)).toEqual({ toolCalls: [row("unknown", "error")] }) - }), -) - it.effect("execute sends at most one progress update per 100 ms with the latest rows", () => Effect.gen(function* () { const started = yield* Deferred.make() diff --git a/packages/session-ui/src/message/current-tool-state.test.ts b/packages/session-ui/src/message/current-tool-state.test.ts index b167aa5f17e..a799c2eaf1c 100644 --- a/packages/session-ui/src/message/current-tool-state.test.ts +++ b/packages/session-ui/src/message/current-tool-state.test.ts @@ -1,13 +1,6 @@ import { describe, expect, test } from "bun:test" import type { SessionMessageAssistantTool } from "@opencode/client/promise" -import { - currentToolError, - currentToolFailed, - currentToolInput, - currentToolMetadata, - currentToolOutput, - executeToolCalls, -} from "./current-tool-state" +import { currentToolError, currentToolInput, currentToolMetadata, currentToolOutput } from "./current-tool-state" const time = { created: 1 } @@ -76,39 +69,6 @@ describe("current tool state", () => { expect(currentToolMetadata(tool)).toBe(tool.state.metadata) }) - test("keeps a caught or interrupted call marked as an error when execute succeeds", () => { - // Both kinds of child termination are persisted as an error row. - const metadata = { toolCalls: [{ tool: "fetch", status: "error" }] } - - const tool = { - type: "tool", - id: "tool_execute", - name: "execute", - state: { status: "completed", input: {}, content: [{ type: "text", text: "recovered" }], metadata }, - time, - } satisfies SessionMessageAssistantTool - - expect(currentToolFailed(tool)).toBe(false) - expect(executeToolCalls(metadata)).toEqual([{ tool: "fetch", status: "error" }]) - }) - - test("marks an execute script failure even without recorded child calls", () => { - const tool = { - type: "tool", - id: "tool_execute", - name: "execute", - state: { - status: "completed", - input: {}, - content: [{ type: "text", text: "Code cannot be empty." }], - metadata: { error: true }, - }, - time, - } satisfies SessionMessageAssistantTool - - expect(currentToolFailed(tool)).toBe(true) - }) - test("keeps structured errors at the current boundary", () => { const tool = { type: "tool", diff --git a/packages/session-ui/src/message/current-tool-state.ts b/packages/session-ui/src/message/current-tool-state.ts index e89864a469b..4322bedd657 100644 --- a/packages/session-ui/src/message/current-tool-state.ts +++ b/packages/session-ui/src/message/current-tool-state.ts @@ -40,8 +40,7 @@ export function currentToolError(tool: SessionMessageAssistantTool) { export function currentToolFailed(tool: SessionMessageAssistantTool) { return ( tool.state.status === "error" || - // A script can succeed after catching a child error or leaving a call pending. - (tool.name === "execute" && currentToolMetadata(tool).error === true) || + (tool.name === "execute" && executeToolFailed(currentToolMetadata(tool))) || (tool.name === "shell" && tool.state.status === "completed" && shellResultFailed(currentToolMetadata(tool))) ) } @@ -51,6 +50,10 @@ export function shellResultFailed(metadata: Schema.JsonObject) { return metadata.timeout === true || (Predicate.isNumber(metadata.exit) && metadata.exit !== 0) } +function executeToolFailed(metadata: Schema.JsonObject) { + return metadata.error === true || executeToolCalls(metadata).some((call) => call.status === "error") +} + const decodeExecuteCall = Schema.decodeUnknownOption( Schema.Struct({ tool: Schema.String,