From e707e416ed741ddf250760ea6497916bcded9eb6 Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Wed, 3 Jun 2026 19:05:28 +0530 Subject: [PATCH] fix(acp): include external directory permission context (#30567) --- packages/opencode/src/acp/tool.ts | 19 ++++++++-- packages/opencode/src/tool/shell.ts | 23 +++++++++--- packages/opencode/test/acp/permission.test.ts | 36 +++++++++++++++++++ packages/opencode/test/acp/tool.test.ts | 3 ++ packages/opencode/test/tool/shell.test.ts | 6 ++++ 5 files changed, 79 insertions(+), 8 deletions(-) diff --git a/packages/opencode/src/acp/tool.ts b/packages/opencode/src/acp/tool.ts index 095f5047e..bf16a78de 100644 --- a/packages/opencode/src/acp/tool.ts +++ b/packages/opencode/src/acp/tool.ts @@ -78,6 +78,9 @@ export function toLocations(toolName: string, input: ToolInput): ToolCallLocatio case "write": return locationFrom(input.filePath ?? input.filepath) + case "external_directory": + return locationFrom(input.filePath ?? input.filepath, input.parentDir, input.directories) + case "grep": case "glob": case "context": @@ -255,9 +258,19 @@ export const buildDuplicateRunningToolUpdate = duplicateRunningToolUpdate export const buildCompletedToolUpdate = completedToolUpdate export const buildErrorToolUpdate = errorToolUpdate -function locationFrom(value: unknown): ToolCallLocation[] { - const path = stringValue(value) - return path ? [{ path }] : [] +function locationFrom(...values: unknown[]): ToolCallLocation[] { + return Array.from( + new Set( + values.flatMap((value): string[] => { + if (Array.isArray(value)) { + return value.filter((item): item is string => typeof item === "string" && item.length > 0) + } + const path = stringValue(value) + return path ? [path] : [] + }), + ), + (path) => ({ path }), + ) } function diffContent(input: ToolInput): ToolCallContent[] { diff --git a/packages/opencode/src/tool/shell.ts b/packages/opencode/src/tool/shell.ts index 59427b8e7..7726c1bd4 100644 --- a/packages/opencode/src/tool/shell.ts +++ b/packages/opencode/src/tool/shell.ts @@ -263,9 +263,14 @@ const parse = Effect.fn("ShellTool.parse")(function* (command: string, ps: boole return tree }) -const ask = Effect.fn("ShellTool.ask")(function* (ctx: Tool.Context, scan: Scan) { +const ask = Effect.fn("ShellTool.ask")(function* ( + ctx: Tool.Context, + scan: Scan, + input: { command: string; description: string }, +) { if (scan.dirs.size > 0) { - const globs = Array.from(scan.dirs).map((dir) => { + const directories = Array.from(scan.dirs) + const globs = directories.map((dir) => { if (process.platform === "win32") return FSUtil.normalizePathPattern(path.join(dir, "*")) return path.join(dir, "*") }) @@ -273,7 +278,12 @@ const ask = Effect.fn("ShellTool.ask")(function* (ctx: Tool.Context, scan: Scan) permission: "external_directory", patterns: globs, always: globs, - metadata: {}, + metadata: { + command: input.command, + description: input.description, + directories, + patterns: globs, + }, }) } @@ -282,7 +292,10 @@ const ask = Effect.fn("ShellTool.ask")(function* (ctx: Tool.Context, scan: Scan) permission: ShellID.ToolID, patterns: Array.from(scan.patterns), always: Array.from(scan.always), - metadata: {}, + metadata: { + command: input.command, + description: input.description, + }, }) }) @@ -625,7 +638,7 @@ export const ShellTool = Tool.define( ) const scan = yield* collect(tree.rootNode, cwd, ps, shell, instanceCtx) if (!containsPath(cwd, instanceCtx)) scan.dirs.add(cwd) - yield* ask(ctx, scan) + yield* ask(ctx, scan, params) }), ) diff --git a/packages/opencode/test/acp/permission.test.ts b/packages/opencode/test/acp/permission.test.ts index fb86026af..966ff6ff1 100644 --- a/packages/opencode/test/acp/permission.test.ts +++ b/packages/opencode/test/acp/permission.test.ts @@ -165,6 +165,42 @@ describe("acp permissions", () => { expect(harness.replies).toEqual([{ requestID: "perm_1", reply: "once", directory: "/workspace" }]) }) + it("forwards external_directory metadata and locations to requestPermission", async () => { + const harness = createHarness() + await createSession(harness.session, "ses_a") + + harness.subscription.handle( + permissionAsked("ses_a", "perm_external", { + permission: "external_directory", + metadata: { + command: "mkdir -p /tmp/outside", + description: "Create external directory", + directories: ["/tmp/outside"], + patterns: ["/tmp/outside/*"], + }, + tool: { messageID: "msg_1", callID: "call_1" }, + }), + ) + + await pollUntil(() => harness.replies.length === 1, "external_directory permission was never replied") + + expect(harness.requests[0]).toMatchObject({ + sessionId: "ses_a", + toolCall: { + toolCallId: "call_1", + status: "pending", + title: "external_directory", + rawInput: { + command: "mkdir -p /tmp/outside", + description: "Create external directory", + directories: ["/tmp/outside"], + patterns: ["/tmp/outside/*"], + }, + locations: [{ path: "/tmp/outside" }], + }, + }) + }) + it("rejects non-selected outcomes", async () => { const harness = createHarness(() => Promise.resolve({ outcome: { outcome: "cancelled" } })) await createSession(harness.session, "ses_a") diff --git a/packages/opencode/test/acp/tool.test.ts b/packages/opencode/test/acp/tool.test.ts index 17949c861..bc3c7c359 100644 --- a/packages/opencode/test/acp/tool.test.ts +++ b/packages/opencode/test/acp/tool.test.ts @@ -34,6 +34,9 @@ describe("acp tool conversion", () => { expect(toLocations("grep", { path: "/repo/src" })).toEqual([{ path: "/repo/src" }]) expect(toLocations("glob", { path: "/repo/test" })).toEqual([{ path: "/repo/test" }]) expect(toLocations("context7_get_library_docs", { path: "/docs" })).toEqual([{ path: "/docs" }]) + expect(toLocations("external_directory", { directories: ["/tmp/outside"], patterns: ["/tmp/outside/*"] })).toEqual([ + { path: "/tmp/outside" }, + ]) expect(toLocations("bash", { filePath: "/tmp/nope.ts", path: "/tmp" })).toEqual([]) expect(toLocations("read", { path: "/tmp/missing-file-path.ts" })).toEqual([]) }) diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index adb8a3c30..4f6ece584 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -920,6 +920,12 @@ describe("tool.shell permissions", () => { expect(extDirReq).toBeDefined() expect(extDirReq!.patterns).toContain(expected) expect(extDirReq!.always).toContain(expected) + expect(extDirReq!.metadata).toMatchObject({ + command: `cat ${filepath}`, + description: "Read external file", + directories: [outerTmp], + patterns: [expected], + }) }), ) }),