mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
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
This commit is contained in:
154
src/acp-agent.ts
154
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<string, string> {
|
||||
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<string, unknown> {
|
||||
const effectiveSuggestions =
|
||||
suggestions && suggestions.length > 0
|
||||
? suggestions
|
||||
: [
|
||||
{
|
||||
type: "addRules" as const,
|
||||
rules: [{ toolName }],
|
||||
behavior: "allow" as const,
|
||||
destination: "session" as const,
|
||||
},
|
||||
];
|
||||
const changes: Array<Record<string, unknown>> = [];
|
||||
|
||||
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: {
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user