Jelajahi Sumber

fix(core): continue after configured permission denial (#36155)

Aiden Cline 1 bulan lalu
induk
melakukan
629b304c48

+ 6 - 22
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<void, ToolOutputStore.Error | StepFailedError>()
-      const ownedToolFibers: Array<Fiber.Fiber<void, ToolOutputStore.Error | StepFailedError>> = []
+      const toolFibers = yield* FiberSet.make<void, ToolOutputStore.Error>()
+      const ownedToolFibers: Array<Fiber.Fiber<void, ToolOutputStore.Error>> = []
       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)

+ 7 - 110
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<void>()
-      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