diff --git a/packages/core/src/session/runner/llm.ts b/packages/core/src/session/runner/llm.ts index 6f46931332..2d31338b90 100644 --- a/packages/core/src/session/runner/llm.ts +++ b/packages/core/src/session/runner/llm.ts @@ -48,7 +48,7 @@ import { SessionRunnerSystemPrompt } from "./system-prompt" import { Snapshot } from "../../snapshot" import { makeLocationNode } from "../../effect/app-node" import { llmClient } from "../../effect/app-node-platform" -import { AgentNotFoundError, StepFailedError, UserInterruptedError } from "../error" +import { AgentNotFoundError, StepFailedError } from "../error" import { toSessionError } from "../to-session-error" import { SessionRunnerRetry } from "./retry" import type { SessionHooks } from "@opencode-ai/plugin/v2/effect/session" @@ -256,8 +256,8 @@ const layer = Layer.effect( tools: toolMaterialization?.definitions ?? [], toolChoice: isLastStep ? "none" : undefined, }) - const toolFibers = yield* FiberSet.make() - const ownedToolFibers: Array> = [] + const toolFibers = yield* FiberSet.make() + const ownedToolFibers: Array> = [] let needsContinuation = false const availableTools = new Map(request.tools.map((tool) => [tool.name, tool])) const requestEvent: SessionHooks["request"] = { @@ -363,13 +363,6 @@ const layer = Layer.effect( output: settlement.output, }), settlement.error, - ).pipe( - // Terminal cleanup must own failAssistant because the provider may still be streaming another tool input. - Effect.andThen( - settlement.error?.type === "permission.rejected" - ? Effect.fail(new StepFailedError({ error: settlement.error })) - : Effect.void, - ), ), ), ), @@ -463,26 +456,18 @@ const layer = Layer.effect( : settled.value.flatMap((exit) => (exit._tag === "Failure" ? [exit.cause] : [])) const toolsInterrupted = settledCauses.some(Cause.hasInterrupts) const userDeclined = settledCauses.some(isUserDeclined) - const permissionRejected = settledCauses - .map((cause) => Option.getOrUndefined(Cause.findErrorOption(cause))) - .find( - (error): error is StepFailedError => - error instanceof StepFailedError && error.error.type === "permission.rejected", - ) - if (userDeclined || permissionRejected || streamInterrupted || toolsInterrupted) { + if (userDeclined || streamInterrupted || toolsInterrupted) { yield* FiberSet.clear(toolFibers) yield* serialized(publisher.failUnsettledTools({ type: "aborted", message: "Tool execution interrupted" })) - yield* serialized( - publisher.failAssistant(permissionRejected?.error ?? { type: "aborted", message: "Step interrupted" }), - ) + yield* serialized(publisher.failAssistant({ type: "aborted", message: "Step interrupted" })) } // A settled tool fiber failure is one of two things. A defect from a tool // implementation becomes a failed tool call the model can read, and the step still // settles so the model may recover. A typed infrastructure failure (tool output // could not be persisted) also fails the assistant and then fails the drain. const settledFailure = settledCauses.find( - (cause) => !Cause.hasInterrupts(cause) && !isUserDeclined(cause) && !permissionRejected, + (cause) => !Cause.hasInterrupts(cause) && !isUserDeclined(cause), ) const infraError = settledFailure === undefined ? undefined : Option.getOrUndefined(Cause.findErrorOption(settledFailure)) @@ -534,7 +519,6 @@ const layer = Layer.effect( if (stream._tag === "Failure") return yield* Effect.failCause(stream.cause) if (userDeclined) return yield* Effect.interrupt - if (permissionRejected) return yield* new UserInterruptedError() if ((toolsInterrupted || infraError !== undefined) && settledFailure) return yield* Effect.failCause(settledFailure) if (toolsInterrupted && settled._tag === "Failure") return yield* Effect.failCause(settled.cause) diff --git a/packages/core/test/session-runner.test.ts b/packages/core/test/session-runner.test.ts index 86e9af37a9..f870c5b2b7 100644 --- a/packages/core/test/session-runner.test.ts +++ b/packages/core/test/session-runner.test.ts @@ -64,7 +64,7 @@ import { Location } from "@opencode-ai/core/location" import { ProviderV2 } from "@opencode-ai/core/provider" import { Cause, DateTime, Deferred, Effect, Exit, Fiber, Layer, Schema, Stream } from "effect" import { TestClock } from "effect/testing" -import { and, asc, eq } from "drizzle-orm" +import { asc, eq } from "drizzle-orm" import { testEffect } from "./lib/effect" const requests: LLMRequest[] = [] @@ -518,22 +518,6 @@ const recordedEventTypes = (id: SessionV2.ID) => ) }) -const recordedToolInputEnds = (id: SessionV2.ID, callID: string) => - Effect.gen(function* () { - const { db } = yield* Database.Service - return (yield* db - .select({ data: EventTable.data }) - .from(EventTable) - .where( - and( - eq(EventTable.aggregate_id, id), - eq(EventTable.type, EventV2.versionedType(SessionEvent.Tool.Input.Ended.type, 1)), - ), - ) - .all() - .pipe(Effect.orDie)).filter((event) => event.data.callID === callID) - }) - const recordedStepSettlementEvents = (id: SessionV2.ID, assistantMessageID: SessionMessage.ID) => Effect.gen(function* () { const { db } = yield* Database.Service @@ -2864,7 +2848,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("returns policy-blocked tools to the model and continues", () => + it.effect("returns tool-wrapped policy blocks to the model and continues", () => Effect.gen(function* () { const session = yield* setup const registry = yield* ToolRegistry.Service @@ -3006,7 +2990,7 @@ describe("SessionRunnerLLM", () => { }), ) - it.effect("preserves permission rejection and stops before continuation", () => + it.effect("returns configured permission denials to the model and continues", () => Effect.gen(function* () { const session = yield* setup const registry = yield* ToolRegistry.Service @@ -3017,19 +3001,13 @@ describe("SessionRunnerLLM", () => { [LLMEvent.stepStart({ index: 0 }), LLMEvent.stepFinish({ index: 0, reason: "stop" })], ] - const exit = yield* session.resume(sessionID).pipe(Effect.exit) + yield* session.resume(sessionID) - expect(exit._tag).toBe("Failure") - expect(requests).toHaveLength(1) + expect(requests).toHaveLength(2) expect(yield* session.context(sessionID)).toMatchObject([ { type: "user" }, { type: "assistant", - finish: "error", - error: { - type: "permission.rejected", - message: "Permission denied: edit", - }, content: [ { type: "tool", @@ -3044,93 +3022,12 @@ describe("SessionRunnerLLM", () => { }, ], }, + { type: "assistant", finish: "stop" }, ]) - expect(yield* recordedEventTypes(sessionID)).not.toContain("session.step.ended.1") + expect(yield* recordedEventTypes(sessionID)).not.toContain("session.step.failed.1") }), ) - const rejectPermissionWhileToolInputStreams = (lateEvent: LLMEvent) => - Effect.gen(function* () { - const session = yield* setup - const registry = yield* ToolRegistry.Service - const releaseLateEvent = yield* Deferred.make() - yield* registry.register({ permissionfail: permissionFail }) - const events = yield* EventV2.Service - const permissionFailed = yield* events.subscribe(SessionEvent.Tool.Failed).pipe( - Stream.filter((event) => event.data.sessionID === sessionID && event.data.callID === "call-permission"), - Stream.runHead, - Effect.forkScoped({ startImmediately: true }), - ) - yield* admit(session, "Reject permission while another tool input streams") - responseStream = Stream.concat( - Stream.fromIterable([ - LLMEvent.stepStart({ index: 0 }), - LLMEvent.toolInputStart({ id: "call-streaming", name: "echo" }), - LLMEvent.toolCall({ id: "call-permission", name: "permissionfail", input: {} }), - ]), - Stream.fromEffect(Deferred.await(releaseLateEvent)).pipe(Stream.flatMap(() => Stream.make(lateEvent))), - ) - - const run = yield* session.resume(sessionID).pipe(Effect.forkChild) - yield* Fiber.join(permissionFailed).pipe(Effect.timeout("1 second")) - yield* Effect.yieldNow - const inputEndsBeforeRelease = yield* recordedToolInputEnds(sessionID, "call-streaming") - yield* Deferred.succeed(releaseLateEvent, undefined) - const exit = yield* Fiber.await(run) - return { - exit, - inputEndsBeforeRelease, - context: yield* session.context(sessionID), - inputEnds: yield* recordedToolInputEnds(sessionID, "call-streaming"), - } - }) - - for (const testCase of [ - { - name: "does not end concurrent tool input when permission is rejected", - event: LLMEvent.toolInputDelta({ - id: "call-streaming", - name: "echo", - text: '{"text":"still streaming"}', - }), - defect: "Tool input delta after end: call-streaming", - }, - { - name: "does not duplicate concurrent tool input end when permission is rejected", - event: LLMEvent.toolInputEnd({ id: "call-streaming", name: "echo" }), - defect: "Duplicate tool input end: call-streaming", - }, - ]) { - it.effect(testCase.name, () => - Effect.gen(function* () { - const result = yield* rejectPermissionWhileToolInputStreams(testCase.event) - - expect(result.inputEndsBeforeRelease).toHaveLength(0) - expect(Exit.isFailure(result.exit)).toBe(true) - if (Exit.isFailure(result.exit)) { - expect(Cause.pretty(result.exit.cause)).not.toContain(testCase.defect) - expect(Cause.hasDies(result.exit.cause)).toBe(false) - } - expect(result.context).toMatchObject([ - { type: "user" }, - { - type: "assistant", - error: { type: "permission.rejected", message: "Permission denied: edit" }, - content: [ - { - type: "tool", - id: "call-streaming", - state: { status: "error", error: { type: "aborted", message: "Tool execution interrupted" } }, - }, - { type: "tool", id: "call-permission", state: { status: "error" } }, - ], - }, - ]) - expect(result.inputEnds).toHaveLength(1) - }), - ) - } - it.effect("interrupts runner continuation when a question is cancelled", () => Effect.gen(function* () { const session = yield* setup