fix(acp): enrich permission prompts (#34079)

packages/opencode/src/acp/permission.ts

@@ -1,9 +1,16 @@
-import type { AgentSideConnection, PermissionOption, RequestPermissionResponse } from "@agentclientprotocol/sdk"
+import type {
+  AgentSideConnection,
+  PermissionOption,
+  RequestPermissionResponse,
+  ToolCallContent,
+  ToolCallLocation,
+  ToolCallUpdate,
+} from "@agentclientprotocol/sdk"
 import type { Event, OpencodeClient } from "@opencode-ai/sdk/v2"
 import { applyPatch } from "diff"
 import { exists, readText } from "@/util/filesystem"
 import type { ACPSession } from "./session"
-import { toLocations, toToolKind, type ToolInput } from "./tool"
+import { pendingToolCall, toLocations, type ToolInput } from "./tool"
 import { Effect } from "effect"
 
 type PermissionEvent = Extract<Event, { type: "permission.asked" }>
@@ -54,14 +61,11 @@ export class Handler {
     const result = await this.input.connection
       .requestPermission({
         sessionId: permission.sessionID,
-        toolCall: {
+        toolCall: await permissionToolCall({
           toolCallId: permission.tool?.callID ?? permission.id,
-          status: "pending",
+          toolName: permission.permission,
-          title: permission.permission,
+          input: permission.metadata,
-          rawInput: permission.metadata,
+        }),
-          kind: toToolKind(permission.permission),
-          locations: toLocations(permission.permission, permission.metadata),
-        },
         options: permissionOptions,
       })
       .catch(async () => {
@@ -111,6 +115,107 @@ export class Handler {
   }
 }
 
+async function permissionToolCall(input: {
+  readonly toolCallId: string
+  readonly toolName: string
+  readonly input: ToolInput
+}): Promise<ToolCallUpdate> {
+  const toolCall = pendingToolCall({
+    toolCallId: input.toolCallId,
+    toolName: input.toolName,
+    state: {
+      input: input.input,
+      title: permissionTitle(input.toolName, input.input),
+    },
+  })
+  const content = await permissionContent(input.toolName, input.input)
+  return {
+    ...toolCall,
+    locations: permissionLocations(input.toolName, input.input),
+    ...(content.length ? { content } : {}),
+  }
+}
+
+function permissionTitle(toolName: string, input: ToolInput) {
+  const tool = toolName.toLocaleLowerCase()
+  switch (tool) {
+    case "external_directory":
+      return stringValue(input.description) ?? stringValue(input.command) ?? stringValue(input.parentDir)
+
+    case "webfetch":
+      return stringValue(input.url)
+
+    case "websearch":
+      return stringValue(input.query)
+
+    case "grep":
+    case "glob":
+      return stringValue(input.pattern)
+
+    case "read":
+    case "edit":
+    case "write":
+      return editTitle(input)
+
+    default:
+      return undefined
+  }
+}
+
+function editTitle(input: ToolInput) {
+  const files = fileMetadata(input)
+  if (files.length === 1) return files[0]?.relativePath ?? files[0]?.filePath
+  if (files.length > 1) return `${files.length} files`
+  return stringValue(input.filePath) ?? stringValue(input.filepath) ?? stringValue(input.path)
+}
+
+function permissionLocations(toolName: string, input: ToolInput): ToolCallLocation[] {
+  const files = fileMetadata(input)
+  if (files.length) {
+    return Array.from(
+      new Set(files.flatMap((file) => [file.filePath, file.movePath].filter((path): path is string => !!path))),
+      (path) => ({ path }),
+    )
+  }
+  return toLocations(toolName, input)
+}
+
+async function permissionContent(toolName: string, input: ToolInput): Promise<ToolCallContent[]> {
+  if (toolName.toLocaleLowerCase() !== "edit") return []
+
+  const files = fileMetadata(input)
+  if (files.length) return diffContentForFiles(files)
+
+  const filepath = stringValue(input.filepath) ?? stringValue(input.filePath)
+  const diff = stringValue(input.diff)
+  if (!filepath || !diff) return []
+  const content = await diffContentForPatch(filepath, diff)
+  return content ? [content] : []
+}
+
+async function diffContentForFiles(files: PermissionFileMetadata[]) {
+  const content = await Promise.all(
+    files.map(async (file) => {
+      if (!file.patch) return []
+      const content = await diffContentForPatch(file.filePath, file.patch, file.movePath)
+      return content ? [content] : []
+    }),
+  )
+  return content.flat()
+}
+
+async function diffContentForPatch(filepath: string, diff: string, displayPath = filepath) {
+  const content = (await exists(filepath)) ? await readText(filepath) : ""
+  const next = applyPatch(content, diff)
+  if (next === false) return undefined
+  return {
+    type: "diff" as const,
+    path: displayPath,
+    oldText: content,
+    newText: 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
@@ -121,4 +226,29 @@ function stringValue(value: unknown) {
   return typeof value === "string" ? value : undefined
 }
 
+type PermissionFileMetadata = {
+  readonly filePath: string
+  readonly relativePath?: string
+  readonly movePath?: string
+  readonly patch?: string
+}
+
+function fileMetadata(input: ToolInput): PermissionFileMetadata[] {
+  if (!Array.isArray(input.files)) return []
+  return input.files.flatMap((file): PermissionFileMetadata[] => {
+    if (!file || typeof file !== "object") return []
+    const info = file as Record<string, unknown>
+    const filePath = stringValue(info.filePath)
+    if (!filePath) return []
+    return [
+      {
+        filePath,
+        relativePath: stringValue(info.relativePath),
+        movePath: stringValue(info.movePath),
+        patch: stringValue(info.patch),
+      },
+    ]
+  })
+}
+
 export * as ACPPermission from "./permission"

packages/opencode/test/acp/permission.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it } from "bun:test"
+import { afterEach, describe, expect, it } from "bun:test"
 import type {
   AgentSideConnection,
   RequestPermissionRequest,
@@ -6,13 +6,22 @@ import type {
   SessionUpdate,
 } from "@agentclientprotocol/sdk"
 import type { Event, OpencodeClient } from "@opencode-ai/sdk/v2"
+import { createTwoFilesPatch } from "diff"
 import { Effect, ManagedRuntime } from "effect"
+import { mkdtemp, rm } from "node:fs/promises"
+import { tmpdir } from "node:os"
+import path from "node:path"
 import { ACPEvent } from "@/acp/event"
 import { ACPSession } from "@/acp/session"
 
 type PermissionEvent = Extract<Event, { type: "permission.asked" }>
 type PermissionReplyParams = Parameters<OpencodeClient["permission"]["reply"]>[0]
 type SessionUpdateParams = Parameters<AgentSideConnection["sessionUpdate"]>[0]
+const cleanupDirs: string[] = []
+
+afterEach(async () => {
+  await Promise.all(cleanupDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true })))
+})
 
 const pollUntil = async (
   check: () => boolean | Promise<boolean>,
@@ -137,6 +146,14 @@ function textFromUpdates(updates: SessionUpdateParams[], sessionId: string) {
     .join("")
 }
 
+async function tempFile(name: string, content: string) {
+  const dir = await mkdtemp(path.join(tmpdir(), "opencode-acp-permission-"))
+  cleanupDirs.push(dir)
+  const file = path.join(dir, name)
+  await Bun.write(file, content)
+  return file
+}
+
 describe("acp permissions", () => {
   it("sends requestPermission and replies with the selected outcome", async () => {
     const harness = createHarness()
@@ -151,7 +168,7 @@ describe("acp permissions", () => {
       toolCall: {
         toolCallId: "call_1",
         status: "pending",
-        title: "bash",
+        title: "printf hello",
         rawInput: { command: "printf hello" },
         kind: "execute",
         locations: [],
@@ -165,6 +182,116 @@ describe("acp permissions", () => {
     expect(harness.replies).toEqual([{ requestID: "perm_1", reply: "once", directory: "/workspace" }])
   })
 
+  it("uses permission metadata for non-shell titles", async () => {
+    const harness = createHarness()
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(
+      permissionAsked("ses_a", "perm_fetch", {
+        permission: "webfetch",
+        metadata: {
+          url: "https://example.com/docs",
+          format: "markdown",
+        },
+        tool: { messageID: "msg_1", callID: "call_1" },
+      }),
+    )
+
+    await pollUntil(() => harness.replies.length === 1, "webfetch permission was never replied")
+
+    expect(harness.requests[0]?.toolCall).toMatchObject({
+      toolCallId: "call_1",
+      title: "https://example.com/docs",
+      kind: "fetch",
+      rawInput: { url: "https://example.com/docs", format: "markdown" },
+    })
+  })
+
+  it("includes a diff content block for edit permission metadata", async () => {
+    const filepath = await tempFile("file.ts", "before\n")
+    const harness = createHarness()
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(
+      permissionAsked("ses_a", "perm_edit", {
+        permission: "edit",
+        metadata: {
+          filepath,
+          diff: createTwoFilesPatch(filepath, filepath, "before\n", "after\n"),
+        },
+        tool: { messageID: "msg_1", callID: "call_1" },
+      }),
+    )
+
+    await pollUntil(() => harness.replies.length === 1, "edit permission was never replied")
+
+    expect(harness.requests[0]?.toolCall).toMatchObject({
+      toolCallId: "call_1",
+      title: filepath,
+      kind: "edit",
+      locations: [{ path: filepath }],
+      content: [
+        {
+          type: "diff",
+          path: filepath,
+          oldText: "before\n",
+          newText: "after\n",
+        },
+      ],
+    })
+  })
+
+  it("includes per-file diff blocks and locations for apply_patch permission metadata", async () => {
+    const first = await tempFile("first.ts", "one\n")
+    const second = await tempFile("second.ts", "alpha\n")
+    const harness = createHarness()
+    await createSession(harness.session, "ses_a")
+
+    harness.subscription.handle(
+      permissionAsked("ses_a", "perm_patch", {
+        permission: "edit",
+        metadata: {
+          filepath: "first.ts, second.ts",
+          files: [
+            {
+              filePath: first,
+              relativePath: "first.ts",
+              patch: createTwoFilesPatch(first, first, "one\n", "two\n"),
+            },
+            {
+              filePath: second,
+              relativePath: "second.ts",
+              patch: createTwoFilesPatch(second, second, "alpha\n", "beta\n"),
+            },
+          ],
+        },
+        tool: { messageID: "msg_1", callID: "call_1" },
+      }),
+    )
+
+    await pollUntil(() => harness.replies.length === 1, "apply_patch permission was never replied")
+
+    expect(harness.requests[0]?.toolCall).toMatchObject({
+      toolCallId: "call_1",
+      title: "2 files",
+      locations: [{ path: first }, { path: second }],
+      content: [
+        {
+          type: "diff",
+          path: first,
+          oldText: "one\n",
+          newText: "two\n",
+        },
+        {
+          type: "diff",
+          path: second,
+          oldText: "alpha\n",
+          newText: "beta\n",
+        },
+      ],
+    })
+  })
+
   it("forwards external_directory metadata and locations to requestPermission", async () => {
     const harness = createHarness()
     await createSession(harness.session, "ses_a")
@@ -189,7 +316,7 @@ describe("acp permissions", () => {
       toolCall: {
         toolCallId: "call_1",
         status: "pending",
-        title: "external_directory",
+        title: "Create external directory",
         rawInput: {
           command: "mkdir -p /tmp/outside",
           description: "Create external directory",