From 5ae2d6d3f64289d8e623e05e812aa96afd1391c5 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 24 Jul 2026 14:18:02 -0400 Subject: [PATCH] refactor(core): settle declined tool calls durably with typed reasons (#38734) --- packages/core/src/permission.ts | 5 ++ packages/core/src/session/model-request.ts | 32 +++++++-- packages/core/src/session/runner/llm.ts | 72 ++++++++++++++----- .../src/session/runner/publish-llm-event.ts | 27 ++++--- packages/core/src/tool/AGENTS.md | 2 +- packages/core/src/tool/question.ts | 3 + packages/core/test/session-runner.test.ts | 4 +- 7 files changed, 109 insertions(+), 36 deletions(-) diff --git a/packages/core/src/permission.ts b/packages/core/src/permission.ts index 1a93f4f58da..8b276833dd3 100644 --- a/packages/core/src/permission.ts +++ b/packages/core/src/permission.ts @@ -218,6 +218,11 @@ const layer = Layer.effect( if (result.effect === "allow") return const item = yield* create(request(input), input.agent) return yield* restore(Deferred.await(item.deferred)).pipe( + // Deliberate defect tunnel: leaves wrap execution in blanket `mapError`, which + // must not convert a user's decline into model-facing tool output. The decline + // resurfaces as a typed failure at SessionModelRequest.executeTool. A decline + // WITH feedback (CorrectedError) intentionally stays typed so the leaf can turn + // it into ToolFailure and the model continues. Effect.catchTag("PermissionV2.DeclinedError", (error) => Effect.die(error)), Effect.ensuring( Effect.sync(() => { diff --git a/packages/core/src/session/model-request.ts b/packages/core/src/session/model-request.ts index af9ac71a864..b62fdc8f3f7 100644 --- a/packages/core/src/session/model-request.ts +++ b/packages/core/src/session/model-request.ts @@ -2,11 +2,14 @@ export * as SessionModelRequest from "./model-request" import { LLM, Message, SystemPart, type LLMRequest, type ToolContent } from "@opencode-ai/ai" import { SessionError } from "@opencode-ai/schema/session-error" -import { Context, Effect, Layer } from "effect" +import { Cause, Context, Effect, Layer, Result } from "effect" import { makeLocationNode } from "@opencode-ai/util/effect/app-node" import { App } from "../app" import { ModelV2 } from "../model" +import { PermissionV2 } from "../permission" import { PluginHooks } from "../plugin/hooks" +import { QuestionTool } from "../tool/question" +import { ToolOutputStore } from "../tool-output-store" import { ToolRegistry } from "../tool/registry" import { SessionContext } from "./context" import { SessionModelHeaders } from "./model-headers" @@ -14,13 +17,32 @@ import { MAX_STEPS_PROMPT } from "./runner/max-steps" import PROMPT_DEFAULT from "./runner/prompt/base.txt" import { toLLMMessages } from "./runner/to-llm-message" +/** Failures a prepared execution can surface: infrastructure errors plus user declines resurfaced from the defect tunnel. */ +export type ExecuteError = ToolOutputStore.Error | PermissionV2.DeclinedError | QuestionTool.CancelledError + +// User declines dive under the leaves' blanket `mapError` as defects (the deliberate +// tunnel entered in PermissionV2.assert and the question tool), so a user's "no" can +// never become model-facing tool output. They resurface as typed failures exactly once, +// here at the seam the runner executes through. +const declineDefect = (cause: Cause.Cause) => { + const decline = cause.reasons.flatMap((reason) => + Cause.isDieReason(reason) && + (reason.defect instanceof PermissionV2.DeclinedError || reason.defect instanceof QuestionTool.CancelledError) + ? [reason.defect] + : [], + )[0] + return decline ? Result.succeed(decline) : Result.fail(cause) +} + interface Prepared { readonly request: LLMRequest /** * One request-scoped execution operation. Unknown, hook-removed, and * step-limit-violating calls fail individually through the same seam. */ - readonly executeTool: ToolRegistry.ToolSet["execute"] + readonly executeTool: ( + input: ToolRegistry.ExecuteInput, + ) => Effect.Effect /** True when this request is the final Step; violating calls are rejected and no continuation follows. */ readonly stepLimitReached: boolean } @@ -134,7 +156,7 @@ export const layer = Layer.effect( tools: hookedTools, toolChoice: stepLimitReached ? "none" : undefined, }) - const executeTool: ToolRegistry.ToolSet["execute"] = (executeInput) => { + const executeTool: Prepared["executeTool"] = (executeInput) => { if (stepLimitReached) return Effect.succeed({ status: "error", @@ -145,7 +167,9 @@ export const layer = Layer.effect( status: "error", error: { type: "tool.unknown", message: `Tool is not available for this request: ${executeInput.call.name}` }, }) - return toolSet.execute(executeInput) + return toolSet + .execute(executeInput) + .pipe(Effect.catchCauseFilter(declineDefect, (decline) => Effect.fail(decline))) } return { request, diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index 0762e5f2465..9f0fce8942b 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -36,27 +36,44 @@ type CallOutcome = Data.TaggedEnum<{ const CallOutcome = Data.taggedEnum() // Declining an interactive prompt halts the drain instead of becoming model-facing tool output. -const isUserDeclined = (cause: Cause.Cause) => - cause.reasons.some( - (reason) => - Cause.isDieReason(reason) && - (reason.defect instanceof PermissionV2.DeclinedError || reason.defect instanceof QuestionTool.CancelledError), - ) +const isDecline = ( + error: SessionModelRequest.ExecuteError, +): error is PermissionV2.DeclinedError | QuestionTool.CancelledError => + error._tag === "PermissionV2.DeclinedError" || error._tag === "QuestionTool.CancelledError" /** - * Classifies how the owned tool fibers ended. Interrupts and interactive declines abort - * the step; a defect from a tool implementation becomes a failed tool call the model can - * read; a typed infrastructure failure must fail the assistant and then the drain. + * Classifies how the owned tool fibers ended. Interrupts abort the step; a user decline + * settles its own call and then aborts the step; a defect from a tool implementation + * becomes a failed tool call the model can read; a typed infrastructure failure must + * fail the assistant and then the drain. */ -const classifyToolExits = (settled: Exit.Exit>, never>) => { +const classifyToolExits = ( + settled: Exit.Exit>, never>, + calls: ReadonlyArray, +) => { + // Exits align with calls by construction: one owned fiber per accepted local call. + const exits = settled._tag === "Success" ? settled.value : [] + const declines = exits.flatMap((exit, index) => + exit._tag === "Failure" + ? exit.cause.reasons.flatMap((reason) => + Cause.isFailReason(reason) && isDecline(reason.error) ? [{ call: calls[index], reason: reason.error }] : [], + ) + : [], + ) const causes = - settled._tag === "Failure" - ? [settled.cause] - : settled.value.flatMap((exit) => (exit._tag === "Failure" ? [exit.cause] : [])) - const failure = causes.find((cause) => !Cause.hasInterrupts(cause) && !isUserDeclined(cause)) + settled._tag === "Failure" ? [settled.cause] : exits.flatMap((exit) => (exit._tag === "Failure" ? [exit.cause] : [])) + // The first non-interrupt, non-decline failure, rebuilt without decline reasons so the + // drain's error channel never carries a decline. + const failure = causes.flatMap((cause) => { + if (Cause.hasInterrupts(cause)) return [] + const reasons = cause.reasons.flatMap((reason): Array> => + Cause.isFailReason(reason) ? (isDecline(reason.error) ? [] : [Cause.makeFailReason(reason.error)]) : [reason], + ) + return reasons.length > 0 ? [Cause.fromReasons(reasons)] : [] + })[0] return { interrupted: causes.some(Cause.hasInterrupts), - declined: causes.some(isUserDeclined), + declines, failure, infraError: failure === undefined ? undefined : Option.getOrUndefined(Cause.findErrorOption(failure)), } @@ -204,7 +221,10 @@ const layer = Layer.effect( step: currentStep, }) // Every local tool call forked here is owned until it reaches one durable settlement. - const toolRuns: Array<{ readonly call: ToolCall; readonly fiber: Fiber.Fiber }> = [] + const toolRuns: Array<{ + readonly call: ToolCall + readonly fiber: Fiber.Fiber + }> = [] const interruptTools = Effect.suspend(() => Fiber.interruptAll(toolRuns.map((run) => run.fiber))) let needsContinuation = false const startSnapshot = yield* snapshots.capture() @@ -348,9 +368,23 @@ const layer = Layer.effect( Effect.forEach(toolRuns, (run) => Fiber.await(run.fiber), { concurrency: "unbounded" }), ).pipe(Effect.exit) if (settled._tag === "Failure") yield* interruptTools - const tools = classifyToolExits(settled) + const tools = classifyToolExits( + settled, + toolRuns.map((run) => run.call), + ) - if (tools.declined || streamInterrupted || tools.interrupted) { + // A declined call settles durably with its reason before the generic sweeps. + for (const decline of tools.declines) + yield* serialized( + publisher.failTool(decline.call.id, { + type: "aborted", + message: + decline.reason._tag === "QuestionTool.CancelledError" + ? decline.reason.message + : "The user declined this tool call", + }), + ) + if (tools.declines.length > 0 || streamInterrupted || tools.interrupted) { yield* serialized(publisher.failUnsettledTools({ type: "aborted", message: "Tool execution interrupted" })) yield* serialized(publisher.failAssistant({ type: "aborted", message: "Step interrupted" })) } @@ -390,7 +424,7 @@ const layer = Layer.effect( } if (stream._tag === "Failure") return yield* Effect.failCause(stream.cause) - if (tools.declined) return yield* Effect.interrupt + if (tools.declines.length > 0) return yield* Effect.interrupt if ((tools.interrupted || tools.infraError !== undefined) && tools.failure) return yield* Effect.failCause(tools.failure) if (tools.interrupted && settled._tag === "Failure") return yield* Effect.failCause(settled.cause) diff --git a/packages/core/src/session/runner/publish-llm-event.ts b/packages/core/src/session/runner/publish-llm-event.ts index cefe60da869..cee1380048d 100644 --- a/packages/core/src/session/runner/publish-llm-event.ts +++ b/packages/core/src/session/runner/publish-llm-event.ts @@ -232,21 +232,27 @@ export const createLLMEventPublisher = (events: Pick providerFailed, diff --git a/packages/core/src/tool/AGENTS.md b/packages/core/src/tool/AGENTS.md index 7c1e3f15069..2c1e9292bc0 100644 --- a/packages/core/src/tool/AGENTS.md +++ b/packages/core/src/tool/AGENTS.md @@ -24,7 +24,7 @@ const source = { } ``` -Leaves own resolution, permission, and side-effect ordering. Translate only expected typed errors into `ToolFailure`; do not use `catchCause`, because interruption and defects must survive. +Leaves own resolution, permission, and side-effect ordering. Translate only expected typed errors into `ToolFailure`; do not use `catchCause`, because interruption and defects must survive. User declines from `PermissionV2.assert` and question dismissals travel as defects beneath leaf `mapError` blankets and resurface as typed failures at `SessionModelRequest.executeTool`; leaves must never catch or convert them. A decline with feedback (`PermissionV2.CorrectedError`) stays typed so the leaf converts it into `ToolFailure` and the model continues. ## Registration diff --git a/packages/core/src/tool/question.ts b/packages/core/src/tool/question.ts index cdb80a0000f..7a19f711c8d 100644 --- a/packages/core/src/tool/question.ts +++ b/packages/core/src/tool/question.ts @@ -91,6 +91,9 @@ export const Plugin = { .pipe(Effect.orDie), ), Effect.flatMap((state) => { + // Deliberate defect tunnel (see PermissionV2.assert): a dismissal must dodge + // leaf `mapError` blankets so it never becomes model-facing tool output; it + // resurfaces as a typed failure at SessionModelRequest.executeTool. if (state.status === "cancelled") return Effect.die(new CancelledError()) const output = { answers: input.questions.map((_, index): QuestionV2.Answer => { diff --git a/packages/core/test/session-runner.test.ts b/packages/core/test/session-runner.test.ts index b4a937b8b51..88230860460 100644 --- a/packages/core/test/session-runner.test.ts +++ b/packages/core/test/session-runner.test.ts @@ -3569,7 +3569,7 @@ describe("SessionRunnerLLM", () => { { type: "tool", id: "call-declined", - state: { status: "error", error: { message: "Tool execution interrupted" } }, + state: { status: "error", error: { type: "aborted", message: "The user declined this tool call" } }, }, ], }, @@ -3721,7 +3721,7 @@ describe("SessionRunnerLLM", () => { { type: "tool", id: "call-question", - state: { status: "error", error: { type: "aborted", message: "Tool execution interrupted" } }, + state: { status: "error", error: { type: "aborted", message: "The user dismissed this question" } }, }, ], },