jerry/claude-agent-acp
1
0
Fork 0
mirror of https://github.com/agentclientprotocol/claude-agent-acp.git synced 2026-11-02 15:31:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: forward v2 elicitation values and read dismissed v2 permission prompts as a refusal (#1493)

Browse Source 
**Elicitation content** (agentclientprotocol/agent-client-protocol#2595)
- v2 lets an elicitation answer hold any JSON value, such as an object.
The agent checked accepted content with the v1 guard, which allows only
strings, numbers, booleans and string arrays. A v2 accept with an object
therefore reached the MCP server as `{ action: "accept", content: {} }`.
- The agent now forwards the accepted content unchanged, and the MCP
server checks it against its own schema. If Claude Code's MCP client
still uses the older value types, it rejects such an answer with an
error instead of passing on empty content.
- AskUserQuestion reads only string, number and boolean picks, so it no
longer shows an object as `[object Object]`.
- The MCP OAuth url-mode check now tests `action !== "accept"` instead
of the v1 guard.

**Permission `cancelled` on v2**
(agentclientprotocol/agent-client-protocol#2582, #2658)
- In v2, `cancelled` also means the user dismissed the prompt or the
client declined it by policy.
- When no `session/cancel` is in progress, a foreground v2 `cancelled`
is now denied as "User refused permission to run tool", the same denial
background subagents already get, and the turn goes on. Before, Claude
Code reported it as "Tool permission request failed".
- During a `session/cancel` it is still aborted. v1 is unchanged.
- Unknown v2 outcomes map to `cancelled`, so they are also denied now
instead of aborted.
This commit is contained in:
Ben Brandt
2026-10-30 15:44:34 +01:00
committed by GitHub
parent c3428de1bf
commit 3bc671193c
6 changed files with 135 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
src/acp-agent.ts
View File

@@ -2704,7 +2704,10 @@ async function authenticateMcpServer(
elicitation.then((response) => ({ type: "elicitation" as const, response })),
]);
if (first.type === "elicitation" && !CreateElicitationResponse.isAccept(first.response)) {
// A url-mode accept carries no content to check. The guard
// `CreateElicitationResponse.isAccept` would also reject an accept whose
// `content` holds a v2-only value, such as an object.
if (first.type === "elicitation" && first.response.action !== "accept") {
return;
}
@@ -10273,8 +10276,9 @@ export class ClaudeAcpAgent {
* `signal` through as a `cancellationSignal`. When the turn is cancelled
* while the client's prompt is still open the signal aborts, the SDK sends
* `$/cancel_request`, and our local abort race settles even if the client
* ignores it. A `cancelled` outcome, request rejection, and local abort all
* surface the same "Tool use aborted" the callers already expect. */
* ignores it. A request rejection and a local abort surface the same
* "Tool use aborted" that the callers expect. `canUseTool` decides what a
* `cancelled` outcome means. */
private async requestPermissionFromClient(
params: AcpPermissionRequest,
toolName: string,
@@ -10813,10 +10817,14 @@ export class ClaudeAcpAgent {
// cancel does not stop. Answer such a request with a denial so the
// subagent reads "the user refused" and goes on. Throwing instead makes
// Claude Code deny it as "Tool permission request failed".
// On ACP v2, `cancelled` also answers a prompt that the user dismissed
// or that the client declined by policy. Without a `session/cancel`
// (`session.cancelled` unset), that is the user refusing the tool, so it
// gets the same denial and the turn goes on.
if (
response.outcome?.outcome === "cancelled" &&
this.sessions[sessionId] === session &&
!blocksForeground(session, agentID)
(!blocksForeground(session, agentID) || (this.v2 && !session.cancelled))
) {
return {
behavior: "deny",

48
src/elicitation.ts
View File

@@ -1,8 +1,7 @@
import { randomUUID } from "node:crypto";
import { CreateElicitationResponse } from "@agentclientprotocol/sdk/unstable";
import type {
CreateElicitationRequest,
ElicitationAcceptAction,
CreateElicitationResponse,
ElicitationPropertySchema,
ElicitationSchema,
EnumOption,
@@ -75,17 +74,22 @@ export function mcpElicitationToCreateRequest(
}
/**
* Content of an accepted elicitation response.
* Content of an accepted elicitation response, or `{}` when the response is
* no accept or its `content` is no JSON object.
*
* Uses the SDK's validating guard rather than an `action === "accept"` check:
* the guard both narrows past the union's custom/future variant and validates
* the payload, so a malformed accept (right tag, ill-typed content) yields
* empty content — the same classification the SDK's wire validators apply.
* The values are not checked. v1 allows a string, number, boolean, or string
* array; v2 also allows any other JSON value, such as an object. An MCP
* server checks the content against its own `requestedSchema`, so it gets the
* content as the client sent it: before, an accept whose content held an
* object reached the server as `{ action: "accept", content: {} }`. Callers
* that read a field must check its type.
*/
function acceptedElicitationContent(
response: CreateElicitationResponse,
): NonNullable<ElicitationAcceptAction["content"]> {
return CreateElicitationResponse.isAccept(response) ? (response.content ?? {}) : {};
function acceptedElicitationContent(response: CreateElicitationResponse): Record<string, unknown> {
if (response.action !== "accept") return {};
const content: unknown = (response as { content?: unknown }).content;
return content !== null && typeof content === "object" && !Array.isArray(content)
? (content as Record<string, unknown>)
: {};
}
/**
@@ -97,7 +101,11 @@ export function createElicitationResponseToElicitResult(
): ElicitationResult {
switch (response.action) {
case "accept":
return { action: "accept", content: acceptedElicitationContent(response) };
// `ElicitResult` types the values as v1 does. The MCP server checks them.
return {
action: "accept",
content: acceptedElicitationContent(response) as ElicitationResult["content"],
};
case "decline":
return { action: "decline" };
case "cancel":
@@ -325,13 +333,13 @@ export function applyAskElicitationResponse(
const custom = content[questionCustomFieldKey(index)];
const customText = typeof custom === "string" ? custom.trim() : "";
// A pick is a string, number, or boolean. Other values, such as an object
// that a v2 client may send, are dropped instead of read as "[object Object]".
const value = content[questionFieldKey(index)];
const picks: string[] =
value === undefined || value === null
? []
: Array.isArray(value)
? value.filter((item) => item !== undefined && item !== null && item !== "").map(String)
: [String(value)];
const picks: string[] = (Array.isArray(value) ? value : [value])
.filter(isElicitationScalar)
.map(String)
.filter((item) => item !== "");
// A multi-select is additive, and the form offers the selection and the
// custom box as independent fields — a user who fills both means both, so
@@ -374,6 +382,10 @@ export function applyAskElicitationResponse(
return { action: "answered", updatedInput: withAnnotations(answers) };
}
function isElicitationScalar(value: unknown): value is string | number | boolean {
return typeof value === "string" || typeof value === "number" || typeof value === "boolean";
}
/**
* Coerce an arbitrary MCP `requestedSchema` into an ACP `ElicitationSchema`.
* The two are structurally compatible JSON Schemas; we just guarantee the

27
src/tests/acp-agent.test.ts
View File

@@ -5144,14 +5144,14 @@ describe("permission request cancellation", () => {
});
describe("a cancelled outcome", () => {
function setup() {
function setup(options: { v2?: boolean } = {}) {
const mockClient = {
sessionUpdate: async () => {},
requestPermission: async (): Promise<RequestPermissionResponse> => ({
outcome: { outcome: "cancelled" },
}),
} as unknown as AcpClient;
const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} });
const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} }, options);
const session = injectSession(agent, "session-1");
session.emittedToolCalls.add("tool-1");
const ask = (agentID?: string) =>
@@ -5173,6 +5173,29 @@ describe("permission request cancellation", () => {
await expect(ask("agent-fg")).rejects.toThrow("Tool use aborted");
});
it("denies the tool use of foreground work on v2 without a session/cancel", async () => {
// On v2 the client also answers `cancelled` when the user dismissed the
// prompt or its policy declined it. The turn goes on without the tool.
const { session, ask } = setup({ v2: true });
session.liveBackgroundTasks.set("agent-fg", { isSubagent: true });
for (const agentID of [undefined, "agent-fg"]) {
await expect(ask(agentID)).resolves.toEqual({
behavior: "deny",
message: "User refused permission to run tool",
toolUseID: "tool-1",
decisionClassification: "user_reject",
});
}
});
it("aborts the tool use of foreground work on v2 during a session/cancel", async () => {
const { session, ask } = setup({ v2: true });
session.cancelled = true;
await expect(ask()).rejects.toThrow("Tool use aborted");
});
it("denies the tool use of a background subagent", async () => {
const { session, ask } = setup();
session.liveBackgroundTasks.set("agent-bg", { isSubagent: true, backgrounded: true });

15
src/tests/acp-v2.test.ts
View File

@@ -993,7 +993,7 @@ describe("ACP v2 prompts", () => {
const decisions: unknown[] = [];
scriptTurns(async function* (options) {
// Claude Code asks about each Bash tool use, then returns its result.
for (const toolUseID of ["toolu_allow", "toolu_reject", "toolu_custom"]) {
for (const toolUseID of ["toolu_allow", "toolu_reject", "toolu_dismissed", "toolu_custom"]) {
yield {
type: "assistant",
message: {
@@ -1041,6 +1041,9 @@ describe("ACP v2 prompts", () => {
const answers: v2.RequestPermissionResponse[] = [
{ outcome: { outcome: "selected", optionId: "allow-once" } },
{ outcome: { outcome: "selected", optionId: "reject" } },
// Without a session/cancel, `cancelled` means that the user dismissed
// the prompt or the client declined it by policy.
{ outcome: { outcome: "cancelled" } },
// The draft says an outcome that the agent does not know is no approval.
{ outcome: { outcome: "_later" } },
];
@@ -1061,10 +1064,16 @@ describe("ACP v2 prompts", () => {
{ optionId: "reject", kind: "reject_once" },
],
});
const refused = {
behavior: "deny",
message: "User refused permission to run tool",
decisionClassification: "user_reject",
};
expect(decisions).toEqual([
expect.objectContaining({ behavior: "allow" }),
expect.objectContaining({ behavior: "deny" }),
"Tool use aborted",
expect.objectContaining({ ...refused, toolUseID: "toolu_dismissed" }),
expect.objectContaining({ ...refused, toolUseID: "toolu_custom" }),
]);
expect(turnTrace(client.sessionUpdates)).toEqual([
"user_message",
@@ -1075,6 +1084,8 @@ describe("ACP v2 prompts", () => {
"running",
"requires_action",
"running",
"requires_action",
"running",
"idle end_turn",
]);
});

51
src/tests/elicitation.test.ts
View File

@@ -132,6 +132,31 @@ describe("createElicitationResponseToElicitResult", () => {
});
});
it("forwards content values that only v2 allows, such as an object", () => {
// ACP v2 lets an elicitation value be any JSON value. The MCP server checks
// the content against its own schema, so none of it is dropped here.
const content = {
name: "Ada",
address: { city: "London" },
tags: ["a", 1, null],
};
const response = { action: "accept", content } as unknown as CreateElicitationResponse;
expect(createElicitationResponseToElicitResult(response)).toEqual({
action: "accept",
content,
});
});
it("maps accept content that is no JSON object to an empty object", () => {
for (const content of [null, "Ada", ["Ada"]]) {
const response = { action: "accept", content } as unknown as CreateElicitationResponse;
expect(createElicitationResponseToElicitResult(response)).toEqual({
action: "accept",
content: {},
});
}
});
it("maps decline and cancel", () => {
expect(
createElicitationResponseToElicitResult({ action: "decline" } as CreateElicitationResponse),
@@ -356,6 +381,28 @@ describe("applyAskElicitationResponse", () => {
});
});
it("reads only string, number, and boolean picks from v2 content", () => {
// A v2 client may send an object or a mixed array. Such values are no
// option label, so they count as no pick instead of "[object Object]".
const response = {
action: "accept",
content: {
question_0: { label: "A" },
question_0_custom: { text: "ignored" },
question_1: ["X", { label: "Y" }, null, 2],
},
} as unknown as CreateElicitationResponse;
expect(applyAskElicitationResponse(response, toolInput, questions)).toEqual({
action: "answered",
updatedInput: {
questions,
metadata: { source: "test" },
answers: { "Multi?": "X, 2" },
},
});
});
it("uses a multi-select question's custom answer alone when nothing is selected", () => {
const response = {
action: "accept",
@@ -652,10 +699,12 @@ describe("refusalFallbackResultFromResponse", () => {
{ action: "accept", content: { choice: "cancelled" } },
{ action: "accept", content: {} },
{ action: "accept", content: { choice: "something-new" } },
{ action: "accept", content: { choice: { value: "retry_fallback" } } },
{ action: "accept", content: { choice: ["retry_fallback"] } },
{ action: "accept" },
{ action: "decline" },
{ action: "cancel" },
] as CreateElicitationResponse[];
] as unknown as CreateElicitationResponse[];
for (const response of cases) {
expect(refusalFallbackResultFromResponse(response)).toBe("cancelled");
}

7
src/v2/permission.ts
View File

@@ -33,8 +33,11 @@ export function v2PermissionRequest(request: AcpPermissionRequest): v2.RequestPe
* The v1 form of the answer to a permission request.
*
* v2 adds outcomes that v1 does not know, which the draft says are no
* approval. They become `cancelled`, which the agent reads as an aborted tool
* use, as it does a cancelled prompt.
* approval. They become `cancelled`. In v2, `cancelled` also means that the
* user dismissed the prompt or the client declined it by policy. So the agent
* reads a `cancelled` outcome on v2 as the user refusing the tool, unless a
* `session/cancel` is stopping the session's work: then the tool use is
* aborted, as on v1.
*/
export function v1PermissionResponse(
response: v2.RequestPermissionResponse,