Просмотр исходного кода

fix(acp): handle acp-next permission events (#29656)

Shoubhit Dash 3 месяцев назад
Родитель
Сommit
aa553dea94

+ 10 - 2
packages/opencode/src/acp-next/event.ts

@@ -11,6 +11,7 @@ import type {
 } from "@opencode-ai/sdk/v2"
 import { Effect } from "effect"
 import { ACPNextSession } from "./session"
+import { ACPNextPermission } from "./permission"
 import {
   duplicateRunningToolUpdate,
   errorToolUpdate,
@@ -22,7 +23,8 @@ import {
 
 const log = Log.create({ service: "acp-next-event" })
 
-type Connection = Pick<AgentSideConnection, "sessionUpdate">
+type Connection = Pick<AgentSideConnection, "sessionUpdate"> &
+  Partial<Pick<AgentSideConnection, "requestPermission" | "writeTextFile">>
 type GlobalEventEnvelope = {
   payload?: Event
 }
@@ -40,6 +42,7 @@ export class Subscription {
   private readonly abort = new AbortController()
   private readonly shellSnapshots = new Map<string, string>()
   private readonly toolStarts = new Set<string>()
+  private readonly permission: ACPNextPermission.Handler
   private started = false
 
   constructor(
@@ -48,7 +51,9 @@ export class Subscription {
       connection: Connection
       session: ACPNextSession.Interface
     },
-  ) {}
+  ) {
+    this.permission = new ACPNextPermission.Handler(input)
+  }
 
   start() {
     if (this.started) return
@@ -65,6 +70,9 @@ export class Subscription {
 
   async handle(event: Event) {
     switch (event.type) {
+      case "permission.asked":
+        this.permission.handle(event)
+        return
       case "message.part.updated":
         return this.handlePartUpdated(event)
       case "message.part.delta":

+ 149 - 0
packages/opencode/src/acp-next/permission.ts

@@ -0,0 +1,149 @@
+import type {
+  AgentSideConnection,
+  PermissionOption,
+  RequestPermissionResponse,
+} from "@agentclientprotocol/sdk"
+import * as Log from "@opencode-ai/core/util/log"
+import type { Event, OpencodeClient } from "@opencode-ai/sdk/v2"
+import { applyPatch } from "diff"
+import { exists, readText } from "@/util/filesystem"
+import type { ACPNextSession } from "./session"
+import { toLocations, toToolKind, type ToolInput } from "./tool"
+import { Effect } from "effect"
+
+const log = Log.create({ service: "acp-next-permission" })
+
+type PermissionEvent = Extract<Event, { type: "permission.asked" }>
+type Reply = "once" | "always" | "reject"
+type Connection = Partial<Pick<AgentSideConnection, "requestPermission" | "writeTextFile">>
+
+const permissionOptions: PermissionOption[] = [
+  { optionId: "once", kind: "allow_once", name: "Allow once" },
+  { optionId: "always", kind: "allow_always", name: "Always allow" },
+  { optionId: "reject", kind: "reject_once", name: "Reject" },
+]
+
+export class Handler {
+  private readonly queues = new Map<string, Promise<void>>()
+
+  constructor(
+    private readonly input: {
+      sdk: OpencodeClient
+      connection: Connection
+      session: ACPNextSession.Interface
+    },
+  ) {}
+
+  handle(event: PermissionEvent) {
+    const permission = event.properties
+    const previous = this.queues.get(permission.sessionID) ?? Promise.resolve()
+    const next = previous
+      .then(() => this.process(event))
+      .catch((error: unknown) => {
+        log.error("failed to handle permission", { error, permissionID: permission.id })
+      })
+      .finally(() => {
+        if (this.queues.get(permission.sessionID) === next) {
+          this.queues.delete(permission.sessionID)
+        }
+      })
+    this.queues.set(permission.sessionID, next)
+  }
+
+  private async process(event: PermissionEvent) {
+    const permission = event.properties
+    const session = await Effect.runPromise(this.input.session.tryGet(permission.sessionID))
+    if (!session) return
+
+    if (!this.input.connection.requestPermission) {
+      log.error("ACP connection cannot request permission", {
+        permissionID: permission.id,
+        sessionID: permission.sessionID,
+      })
+      await this.reply(permission.id, "reject", session.cwd)
+      return
+    }
+
+    const result = await this.input.connection
+      .requestPermission({
+        sessionId: permission.sessionID,
+        toolCall: {
+          toolCallId: permission.tool?.callID ?? permission.id,
+          status: "pending",
+          title: permission.permission,
+          rawInput: permission.metadata,
+          kind: toToolKind(permission.permission),
+          locations: toLocations(permission.permission, permission.metadata),
+        },
+        options: permissionOptions,
+      })
+      .catch(async (error: unknown) => {
+        log.error("failed to request permission from ACP", {
+          error,
+          permissionID: permission.id,
+          sessionID: permission.sessionID,
+        })
+        await this.reply(permission.id, "reject", session.cwd)
+        return undefined
+      })
+
+    if (!result) return
+
+    const reply = selectedReply(result)
+    if (reply !== "once" && reply !== "always") {
+      await this.reply(permission.id, "reject", session.cwd)
+      return
+    }
+
+    if (permission.permission === "edit") {
+      await this.writeProposedEdit(session.id, permission.metadata).catch((error: unknown) => {
+        log.error("failed to write proposed edit through ACP", {
+          error,
+          permissionID: permission.id,
+          sessionID: permission.sessionID,
+        })
+      })
+    }
+
+    await this.reply(permission.id, reply, session.cwd)
+  }
+
+  private async reply(requestID: string, reply: Reply, directory: string) {
+    await this.input.sdk.permission.reply({
+      requestID,
+      reply,
+      directory,
+    })
+  }
+
+  private async writeProposedEdit(sessionId: string, metadata: ToolInput) {
+    const filepath = stringValue(metadata.filepath)
+    const diff = stringValue(metadata.diff)
+    if (!filepath || !diff || !this.input.connection.writeTextFile) return
+
+    const content = (await exists(filepath)) ? await readText(filepath) : ""
+    const next = applyPatch(content, diff)
+    if (next === false) {
+      log.error("Failed to apply unified diff (context mismatch)")
+      return
+    }
+
+    void this.input.connection.writeTextFile({
+      sessionId,
+      path: filepath,
+      content: next,
+    })
+  }
+}
+
+function selectedReply(result: RequestPermissionResponse): Reply {
+  if (result.outcome.outcome !== "selected") return "reject"
+  if (result.outcome.optionId === "once" || result.outcome.optionId === "always") return result.outcome.optionId
+  return "reject"
+}
+
+function stringValue(value: unknown) {
+  return typeof value === "string" ? value : undefined
+}
+
+export * as ACPNextPermission from "./permission"

+ 2 - 1
packages/opencode/src/acp-next/service.ts

@@ -69,7 +69,8 @@ export class Service extends Context.Service<Service, Interface>()("@opencode/AC
 
 export function make(input: {
   sdk: OpencodeClient
-  connection?: Pick<AgentSideConnection, "sessionUpdate">
+  connection?: Pick<AgentSideConnection, "sessionUpdate"> &
+    Partial<Pick<AgentSideConnection, "requestPermission" | "writeTextFile">>
   directory?: Directory.Interface
   session?: ACPNextSession.Interface
   eventSubscription?: (subscription: ACPNextEvent.Subscription) => void

+ 1 - 1
packages/opencode/src/acp-next/tool.ts

@@ -74,7 +74,7 @@ export function toLocations(toolName: string, input: ToolInput): ToolCallLocatio
     case "read":
     case "edit":
     case "write":
-      return locationFrom(input.filePath)
+      return locationFrom(input.filePath ?? input.filepath)
 
     case "grep":
     case "glob":

+ 232 - 0
packages/opencode/test/acp-next/permission.test.ts

@@ -0,0 +1,232 @@
+import { describe, expect, it } from "bun:test"
+import type {
+  AgentSideConnection,
+  RequestPermissionRequest,
+  RequestPermissionResponse,
+  SessionUpdate,
+} from "@agentclientprotocol/sdk"
+import type { Event, OpencodeClient } from "@opencode-ai/sdk/v2"
+import { Effect, ManagedRuntime } from "effect"
+import { ACPNextEvent } from "@/acp-next/event"
+import { ACPNextSession } from "@/acp-next/session"
+
+type PermissionEvent = Extract<Event, { type: "permission.asked" }>
+type PermissionReplyParams = Parameters<OpencodeClient["permission"]["reply"]>[0]
+type SessionUpdateParams = Parameters<AgentSideConnection["sessionUpdate"]>[0]
+
+const pollUntil = async (
+  check: () => boolean | Promise<boolean>,
+  message: string,
+  opts?: { timeoutMs?: number; intervalMs?: number },
+) => {
+  const started = Date.now()
+  while (true) {
+    if (await check()) return
+    if (Date.now() - started > (opts?.timeoutMs ?? 2000)) throw new Error(message)
+    await new Promise((resolve) => setTimeout(resolve, opts?.intervalMs ?? 5))
+  }
+}
+
+function makeSessionService() {
+  return ManagedRuntime.make(ACPNextSession.defaultLayer).runSync(
+    ACPNextSession.Service.use((service) => Effect.succeed(service)),
+  )
+}
+
+function createHarness(
+  requestPermission: (params: RequestPermissionRequest) => Promise<RequestPermissionResponse> = () =>
+    Promise.resolve({ outcome: { outcome: "selected", optionId: "once" } }),
+) {
+  const replies: PermissionReplyParams[] = []
+  const requests: RequestPermissionRequest[] = []
+  const updates: SessionUpdateParams[] = []
+  const session = makeSessionService()
+  const sdk = {
+    permission: {
+      reply: (params: PermissionReplyParams) => {
+        replies.push(params)
+        return Promise.resolve({ data: true })
+      },
+    },
+    session: {
+      message: () => Promise.resolve({ data: undefined }),
+    },
+  } as unknown as OpencodeClient
+  const connection = {
+    requestPermission: (params: RequestPermissionRequest) => {
+      requests.push(params)
+      return requestPermission(params)
+    },
+    sessionUpdate: (params: SessionUpdateParams) => {
+      updates.push(params)
+      return Promise.resolve()
+    },
+  } satisfies Pick<AgentSideConnection, "requestPermission" | "sessionUpdate">
+  const subscription = new ACPNextEvent.Subscription({ sdk, connection, session })
+
+  return { connection, replies, requests, sdk, session, subscription, updates }
+}
+
+async function createSession(session: ACPNextSession.Interface, sessionId: string, cwd = "/workspace") {
+  await Effect.runPromise(session.create({ id: sessionId, cwd }))
+}
+
+async function createKnownTextPart(session: ACPNextSession.Interface, sessionId: string, messageId: string, partId: string) {
+  await Effect.runPromise(
+    session.recordPartMetadata({
+      sessionId,
+      messageId,
+      partId,
+      partType: "text",
+      role: "assistant",
+    }),
+  )
+}
+
+function permissionAsked(
+  sessionID: string,
+  id: string,
+  input: {
+    permission?: string
+    metadata?: Record<string, unknown>
+    tool?: { messageID: string; callID: string }
+  } = {},
+) {
+  return {
+    id: `evt_${id}`,
+    type: "permission.asked",
+    properties: {
+      id,
+      sessionID,
+      permission: input.permission ?? "bash",
+      patterns: ["*"],
+      metadata: input.metadata ?? { command: "printf hello" },
+      always: [],
+      ...(input.tool ? { tool: input.tool } : {}),
+    },
+  } as PermissionEvent
+}
+
+function textDelta(sessionID: string, messageID: string, partID: string, delta: string) {
+  return {
+    id: `evt_${sessionID}_${messageID}_${partID}`,
+    type: "message.part.delta",
+    properties: {
+      sessionID,
+      messageID,
+      partID,
+      field: "text",
+      delta,
+    },
+  } as Event
+}
+
+function textFromUpdates(updates: SessionUpdateParams[], sessionId: string) {
+  return updates
+    .filter((item) => item.sessionId === sessionId)
+    .map((item) => item.update)
+    .filter((update): update is Extract<SessionUpdate, { sessionUpdate: "agent_message_chunk" }> => {
+      return update.sessionUpdate === "agent_message_chunk"
+    })
+    .map((update) => (update.content.type === "text" ? update.content.text : ""))
+    .join("")
+}
+
+describe("acp-next permissions", () => {
+  it("sends requestPermission and replies with the selected outcome", async () => {
+    const harness = createHarness()
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(permissionAsked("ses_a", "perm_1", { tool: { messageID: "msg_1", callID: "call_1" } }))
+
+    await pollUntil(() => harness.replies.length === 1, "permission was never replied")
+
+    expect(harness.requests[0]).toMatchObject({
+      sessionId: "ses_a",
+      toolCall: {
+        toolCallId: "call_1",
+        status: "pending",
+        title: "bash",
+        rawInput: { command: "printf hello" },
+        kind: "execute",
+        locations: [],
+      },
+      options: [
+        { optionId: "once", kind: "allow_once", name: "Allow once" },
+        { optionId: "always", kind: "allow_always", name: "Always allow" },
+        { optionId: "reject", kind: "reject_once", name: "Reject" },
+      ],
+    })
+    expect(harness.replies).toEqual([{ requestID: "perm_1", reply: "once", directory: "/workspace" }])
+  })
+
+  it("rejects non-selected outcomes", async () => {
+    const harness = createHarness(() => Promise.resolve({ outcome: { outcome: "cancelled" } }))
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(permissionAsked("ses_a", "perm_cancelled"))
+
+    await pollUntil(() => harness.replies.length === 1, "cancelled permission was never replied")
+
+    expect(harness.replies[0]).toMatchObject({ requestID: "perm_cancelled", reply: "reject" })
+  })
+
+  it("rejects when requestPermission fails", async () => {
+    const harness = createHarness(() => Promise.reject(new Error("client permission UI failed")))
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(permissionAsked("ses_a", "perm_failed"))
+
+    await pollUntil(() => harness.replies.length === 1, "failed permission was never rejected")
+
+    expect(harness.replies[0]).toMatchObject({ requestID: "perm_failed", reply: "reject" })
+  })
+
+  it("does not let a blocked session A permission block session B message updates", async () => {
+    let releasePermission: (() => void) | undefined
+    const blocked = new Promise<RequestPermissionResponse>((resolve) => {
+      releasePermission = () => resolve({ outcome: { outcome: "selected", optionId: "once" } })
+    })
+    const harness = createHarness(() => blocked)
+    await createSession(harness.session, "ses_a")
+    await createSession(harness.session, "ses_b")
+    await createKnownTextPart(harness.session, "ses_b", "msg_b", "part_b")
+
+    harness.subscription.handle(permissionAsked("ses_a", "perm_blocked"))
+    await pollUntil(() => harness.requests.length === 1, "blocked permission was never requested")
+
+    await harness.subscription.handle(textDelta("ses_b", "msg_b", "part_b", "session_b_message"))
+
+    expect(textFromUpdates(harness.updates, "ses_b")).toBe("session_b_message")
+    expect(harness.replies).toHaveLength(0)
+
+    releasePermission?.()
+    await pollUntil(() => harness.replies.length === 1, "blocked permission was never replied after release")
+  })
+
+  it("serializes permission requests per session", async () => {
+    let releaseFirst: (() => void) | undefined
+    const first = new Promise<RequestPermissionResponse>((resolve) => {
+      releaseFirst = () => resolve({ outcome: { outcome: "selected", optionId: "once" } })
+    })
+    const harness = createHarness(() =>
+      harness.requests.length === 1 ? first : Promise.resolve({ outcome: { outcome: "selected", optionId: "always" } }),
+    )
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(permissionAsked("ses_a", "perm_1"))
+    harness.subscription.handle(permissionAsked("ses_a", "perm_2"))
+
+    await pollUntil(() => harness.requests.length === 1, "first permission was never requested")
+    expect(harness.requests.map((request) => request.toolCall.toolCallId)).toEqual(["perm_1"])
+
+    releaseFirst?.()
+    await pollUntil(() => harness.requests.length === 2, "second permission was not requested after first resolved")
+    await pollUntil(() => harness.replies.length === 2, "serialized permissions were not both replied")
+
+    expect(harness.replies.map((reply) => [reply.requestID, reply.reply])).toEqual([
+      ["perm_1", "once"],
+      ["perm_2", "always"],
+    ])
+  })
+})