From 08a62ed53790f8a74892909ee6f54449b23260f3 Mon Sep 17 00:00:00 2001 From: nikita-ashihmin Date: Thu, 30 Jul 2026 20:21:11 +0400 Subject: [PATCH] Expose structured permission changes in ACP metadata (#930) ## Summary Expose Claude Code permission-update suggestions through the same versioned option-level metadata contract used by other ACP agents: ```json { "_meta": { "permission": { "version": 1, "changes": [] } } } ``` The approval actions are presented as `Deny`, `Allow Once`, and `Always Allow`. Exact scope and policy details live in metadata instead of being encoded into the button label. ## Why ACP already transports button semantics via `PermissionOption.kind`, display text via `name`, and the selected value via `optionId`. It does not currently expose the exact persistent policy update. Claude Code suggestions can do considerably more than add one allow rule: they may add, remove, or replace allow/deny/ask rule sets, add or remove directories, change permission mode, and target different storage destinations. Flattening those suggestions into labels or a list of scopes loses mutation semantics and can incorrectly describe a narrow or restrictive update as permission for the whole tool. ## Contract Every change carries a producer-supplied human-readable `description`. Structured fields preserve: - ordered `add`, `remove`, and atomic `replace` operations - `allow`, `deny`, and `ask` rule sets through `ruleBehavior` - whole-tool targets and provider-rule matchers - filesystem directory targets - permission-mode changes - session, process, user, project, and project-local destinations as lifetime/storage Example scoped Bash rule: ```json { "type": "policy_rule", "operation": "add", "ruleBehavior": "allow", "description": "Allow Bash calls matching npm test:*", "lifetime": { "scope": "session" }, "targets": [ { "type": "tool", "toolName": "Bash", "matcher": { "type": "provider_rule", "provider": "claudeCode", "value": "npm test:*" } } ] } ``` A tool target without a matcher means the whole tool. A provider rule is intentionally opaque; clients should show `description` rather than reverse-engineering Claude Code rule syntax. When Claude Code supplies no suggestions, the adapter preserves its existing fallback behavior by describing the exact session rule it returns: allow the whole requested tool for the session. ## Compatibility This remains on the current ACP protocol and uses only `_meta`. Clients that do not understand `_meta.permission` continue rendering and dispatching the standard options. Option IDs and permission outcomes are unchanged. ## Tests - full suite: 682 passed, 20 skipped - lint passed - Prettier check passed - TypeScript build passed - coverage includes whole-tool fallback, scoped rules, directories, replace/remove, deny rules, destinations, and mode changes --- src/acp-agent.ts | 154 ++++++++++++++------ src/tests/acp-agent.test.ts | 271 ++++++++++++++++++++++++++---------- 2 files changed, 310 insertions(+), 115 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 766ca778..88a94f56 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -1194,48 +1194,122 @@ export function resolvePermissionMode( return mapped; } -/** - * Builds the label for the "Always Allow" permission option so the user can see - * the exact scope they are committing to. Uses the SDK-provided suggestions - * when available (e.g. `Bash(npm test:*)`) and falls back to naming the whole - * tool so "Always Allow" is never a blank check without disclosure. - */ -export function describeAlwaysAllow( +function permissionLifetime(destination: PermissionUpdate["destination"]): Record { + switch (destination) { + case "session": + return { scope: "session" }; + case "cliArg": + return { scope: "process", storage: "cli_argument" }; + case "userSettings": + return { scope: "persistent", storage: "user" }; + case "projectSettings": + return { scope: "persistent", storage: "project" }; + case "localSettings": + return { scope: "persistent", storage: "project_local" }; + default: + return { scope: "unknown" }; + } +} + +function permissionMetadataForAlwaysAllow( suggestions: PermissionUpdate[] | undefined, toolName: string, -): string { - if (!suggestions || suggestions.length === 0) { - return `Always Allow all ${toolName}`; - } +): Record { + const effectiveSuggestions = + suggestions && suggestions.length > 0 + ? suggestions + : [ + { + type: "addRules" as const, + rules: [{ toolName }], + behavior: "allow" as const, + destination: "session" as const, + }, + ]; + const changes: Array> = []; - const ruleLabels: string[] = []; - const directories: string[] = []; - - for (const update of suggestions) { - if (update.type === "addRules" && update.behavior === "allow") { - for (const rule of update.rules) { - ruleLabels.push( - rule.ruleContent ? `${rule.toolName}(${rule.ruleContent})` : `all ${rule.toolName}`, - ); + for (const update of effectiveSuggestions) { + switch (update.type) { + case "addRules": + case "removeRules": + case "replaceRules": { + const operation = + update.type === "addRules" ? "add" : update.type === "removeRules" ? "remove" : "replace"; + const targets = update.rules.map((rule) => ({ + type: "tool", + toolName: rule.toolName, + ...(rule.ruleContent + ? { + matcher: { + type: "provider_rule", + provider: "claudeCode", + value: rule.ruleContent, + }, + } + : {}), + })); + const renderedRules = update.rules + .map((rule) => + rule.ruleContent + ? `${rule.toolName} calls matching ${rule.ruleContent}` + : `all ${rule.toolName} calls`, + ) + .join(", "); + const verb = + operation === "add" + ? update.behavior === "allow" + ? "Allow" + : update.behavior === "deny" + ? "Deny" + : "Ask before" + : operation === "remove" + ? `Remove ${update.behavior} rules for` + : `Replace ${update.behavior} rules with`; + changes.push({ + type: "policy_rule", + operation, + ruleBehavior: update.behavior, + description: `${verb} ${renderedRules}`, + lifetime: permissionLifetime(update.destination), + targets, + }); + break; } - } else if (update.type === "addDirectories") { - directories.push(...update.directories); + case "addDirectories": + case "removeDirectories": { + const operation = update.type === "addDirectories" ? "add" : "remove"; + changes.push({ + type: "policy_rule", + operation, + ruleBehavior: "allow", + description: + operation === "add" + ? `Allow filesystem access under ${update.directories.join(", ")}` + : `Remove additional filesystem access under ${update.directories.join(", ")}`, + lifetime: permissionLifetime(update.destination), + targets: update.directories.map((path) => ({ + type: "filesystem", + matcher: { type: "directory", path }, + })), + }); + break; + } + case "setMode": + changes.push({ + type: "permission_mode", + operation: "set", + provider: "claudeCode", + mode: update.mode, + description: `Set Claude Code permission mode to ${update.mode}`, + lifetime: permissionLifetime(update.destination), + }); + break; + default: + break; } } - const parts: string[] = []; - if (ruleLabels.length > 0) { - parts.push(ruleLabels.join(", ")); - } - if (directories.length > 0) { - parts.push(`access to ${directories.join(", ")}`); - } - - if (parts.length === 0) { - return `Always Allow all ${toolName}`; - } - - return `Always Allow ${parts.join(" and ")}`; + return { version: 1, changes }; } /** @@ -4536,7 +4610,6 @@ export class ClaudeAcpAgent { toolInput, { signal, suggestions, toolUseID, agentID, matchedAskRule }, ) => { - const alwaysAllowLabel = describeAlwaysAllow(suggestions, toolName); const supportsTerminalOutput = this.clientCapabilities?._meta?.["terminal_output"] === true; const session = this.sessions[sessionId]; if (!session) { @@ -4691,13 +4764,16 @@ export class ClaudeAcpAgent { const response = await this.requestPermissionFromClient( { options: [ + { kind: "reject_once", name: "Deny", optionId: "reject" }, + { kind: "allow_once", name: "Allow Once", optionId: "allow" }, { kind: "allow_always", - name: alwaysAllowLabel, + name: "Always Allow", optionId: "allow_always", + _meta: { + permission: permissionMetadataForAlwaysAllow(suggestions, toolName), + }, }, - { kind: "allow_once", name: "Allow", optionId: "allow" }, - { kind: "reject_once", name: "Reject", optionId: "reject" }, ], sessionId, toolCall: { diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 42c82c40..1c420244 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -34,7 +34,6 @@ import { stripLocalCommandMetadata, ClaudeAcpAgent, claudeCliPath, - describeAlwaysAllow, streamEventToAcpNotifications, messageIdForGrouping, buildConfigOptions, @@ -2000,81 +1999,6 @@ describe("permission requests", () => { expect(Array.isArray(requestStructure.toolCall.content)).toBe(true); } }); - - describe("describeAlwaysAllow", () => { - it("falls back to naming the whole tool when no suggestions are provided", () => { - expect(describeAlwaysAllow(undefined, "Bash")).toBe("Always Allow all Bash"); - expect(describeAlwaysAllow([], "Read")).toBe("Always Allow all Read"); - }); - - it("includes the scoped rule content from a suggestion", () => { - const label = describeAlwaysAllow( - [ - { - type: "addRules", - rules: [{ toolName: "Bash", ruleContent: "npm test:*" }], - behavior: "allow", - destination: "session", - }, - ], - "Bash", - ); - expect(label).toBe("Always Allow Bash(npm test:*)"); - }); - - it("indicates a tool-wide rule when the suggestion has no ruleContent", () => { - const label = describeAlwaysAllow( - [ - { - type: "addRules", - rules: [{ toolName: "Read" }], - behavior: "allow", - destination: "session", - }, - ], - "Read", - ); - expect(label).toBe("Always Allow all Read"); - }); - - it("joins multiple rules and directory suggestions", () => { - const label = describeAlwaysAllow( - [ - { - type: "addRules", - rules: [ - { toolName: "Bash", ruleContent: "git status" }, - { toolName: "Bash", ruleContent: "git diff:*" }, - ], - behavior: "allow", - destination: "session", - }, - { - type: "addDirectories", - directories: ["/tmp/work"], - destination: "session", - }, - ], - "Bash", - ); - expect(label).toBe("Always Allow Bash(git status), Bash(git diff:*) and access to /tmp/work"); - }); - - it("ignores non-allow rules and falls back when nothing is left", () => { - const label = describeAlwaysAllow( - [ - { - type: "addRules", - rules: [{ toolName: "Bash", ruleContent: "rm -rf:*" }], - behavior: "deny", - destination: "session", - }, - ], - "Bash", - ); - expect(label).toBe("Always Allow all Bash"); - }); - }); }); describe("permission request cancellation", () => { @@ -2173,6 +2097,201 @@ describe("permission request cancellation", () => { } as any), ).rejects.toThrow("Tool use aborted"); }); + + it("offers the approval actions in deny, once, always order with human-readable labels", async () => { + let request: RequestPermissionRequest | undefined; + const mockClient = { + sessionUpdate: async () => {}, + requestPermission: async (params: RequestPermissionRequest) => { + request = params; + return { outcome: { outcome: "selected", optionId: "reject" } }; + }, + } as unknown as AcpClient; + const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} }); + injectSession(agent, "session-1"); + + await agent.canUseTool("session-1")("Bash", { command: "ls" }, { + signal: new AbortController().signal, + suggestions: [], + toolUseID: "tool-1", + } as any); + + expect(request?.options).toEqual([ + { kind: "reject_once", name: "Deny", optionId: "reject" }, + { kind: "allow_once", name: "Allow Once", optionId: "allow" }, + { + kind: "allow_always", + name: "Always Allow", + optionId: "allow_always", + _meta: { + permission: { + version: 1, + changes: [ + { + type: "policy_rule", + operation: "add", + ruleBehavior: "allow", + description: "Allow all Bash calls", + lifetime: { scope: "session" }, + targets: [{ type: "tool", toolName: "Bash" }], + }, + ], + }, + }, + }, + ]); + }); + + it("attaches structured permission changes to the always-allow ACP option", async () => { + let request: RequestPermissionRequest | undefined; + const mockClient = { + sessionUpdate: async () => {}, + requestPermission: async (params: RequestPermissionRequest) => { + request = params; + return { outcome: { outcome: "selected", optionId: "reject" } }; + }, + } as unknown as AcpClient; + const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} }); + injectSession(agent, "session-1"); + + await agent.canUseTool("session-1")("Bash", { command: "npm test" }, { + signal: new AbortController().signal, + suggestions: [ + { + type: "addRules", + rules: [{ toolName: "Bash", ruleContent: "npm test:*" }], + behavior: "allow", + destination: "session", + }, + { + type: "addDirectories", + directories: ["/tmp/work"], + destination: "session", + }, + ], + toolUseID: "tool-1", + } as any); + + expect(request?.options.find((option) => option.optionId === "allow_always")?._meta).toEqual({ + permission: { + version: 1, + changes: [ + { + type: "policy_rule", + operation: "add", + ruleBehavior: "allow", + description: "Allow Bash calls matching npm test:*", + lifetime: { scope: "session" }, + targets: [ + { + type: "tool", + toolName: "Bash", + matcher: { + type: "provider_rule", + provider: "claudeCode", + value: "npm test:*", + }, + }, + ], + }, + { + type: "policy_rule", + operation: "add", + ruleBehavior: "allow", + description: "Allow filesystem access under /tmp/work", + lifetime: { scope: "session" }, + targets: [ + { + type: "filesystem", + matcher: { type: "directory", path: "/tmp/work" }, + }, + ], + }, + ], + }, + }); + }); + + it("preserves replace, remove, deny, destination, and mode suggestion semantics", async () => { + let request: RequestPermissionRequest | undefined; + const mockClient = { + sessionUpdate: async () => {}, + requestPermission: async (params: RequestPermissionRequest) => { + request = params; + return { outcome: { outcome: "selected", optionId: "reject" } }; + }, + } as unknown as AcpClient; + const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} }); + injectSession(agent, "session-1"); + + await agent.canUseTool("session-1")("Bash", { command: "rm generated.txt" }, { + signal: new AbortController().signal, + suggestions: [ + { + type: "replaceRules", + rules: [{ toolName: "Bash", ruleContent: "rm generated.txt" }], + behavior: "deny", + destination: "projectSettings", + }, + { + type: "removeDirectories", + directories: ["/tmp/old"], + destination: "localSettings", + }, + { + type: "setMode", + mode: "default", + destination: "session", + }, + ], + toolUseID: "tool-1", + } as any); + + expect( + (request?.options.find((option) => option.optionId === "allow_always")?._meta as any) + ?.permission.changes, + ).toEqual([ + { + type: "policy_rule", + operation: "replace", + ruleBehavior: "deny", + description: "Replace deny rules with Bash calls matching rm generated.txt", + lifetime: { scope: "persistent", storage: "project" }, + targets: [ + { + type: "tool", + toolName: "Bash", + matcher: { + type: "provider_rule", + provider: "claudeCode", + value: "rm generated.txt", + }, + }, + ], + }, + { + type: "policy_rule", + operation: "remove", + ruleBehavior: "allow", + description: "Remove additional filesystem access under /tmp/old", + lifetime: { scope: "persistent", storage: "project_local" }, + targets: [ + { + type: "filesystem", + matcher: { type: "directory", path: "/tmp/old" }, + }, + ], + }, + { + type: "permission_mode", + operation: "set", + provider: "claudeCode", + mode: "default", + description: "Set Claude Code permission mode to default", + lifetime: { scope: "session" }, + }, + ]); + }); }); describe("tool_call emitted before permission request", () => {