feat(acp): Handle ACP request cancellation signals (#801)

2026-10-27 04:30:59 +00:00 · 2026-06-22 14:48:42 +02:00
parent d5e1b7fa72
commit 9013d1d468
5 changed files with 296 additions and 52 deletions
--- a/package-lock.json
+++ b/package-lock.json
@@ -9,7 +9,7 @@
      "version": "0.49.0",
      "license": "Apache-2.0",
      "dependencies": {
-        "@agentclientprotocol/sdk": "0.28.1",
+        "@agentclientprotocol/sdk": "0.29.0",
        "@anthropic-ai/claude-agent-sdk": "0.3.185",
        "zod": "^3.25.0 || ^4.0.0"
      },
@@ -19,6 +19,7 @@
      "devDependencies": {
        "@anthropic-ai/sdk": "0.105.0",
        "@eslint/js": "10.0.1",
+        "@tsconfig/node22": "^22.0.5",
        "@types/node": "26.0.0",
        "@typescript-eslint/eslint-plugin": "8.61.1",
        "@typescript-eslint/parser": "8.61.1",
@@ -29,12 +30,15 @@
        "ts-node": "10.9.2",
        "typescript": "6.0.3",
        "vitest": "4.1.9"
+      },
+      "engines": {
+        "node": ">=22"
      }
    },
    "node_modules/@agentclientprotocol/sdk": {
-      "version": "0.28.1",
-      "resolved": "https://registry.npmjs.org/@agentclientprotocol/sdk/-/sdk-0.28.1.tgz",
-      "integrity": "sha512-Z2Frs6YtPhnZZ+XwFXyQkRDXY0fn8FjCalEs0W4yUhQnY4TztmNq0/RnfzWdFN3vqT3h0jTz5klzYbZHGxCDyQ==",
+      "version": "0.29.0",
+      "resolved": "https://registry.npmjs.org/@agentclientprotocol/sdk/-/sdk-0.29.0.tgz",
+      "integrity": "sha512-Sb8xJ/cIhqlvxeJiB0+1scBk0LAkVAwJ9WAFG9XCe3LEI5JWAyOxbqaCa8O1HmNnrk4+qHN11eaKH6zL2iHIYA==",
      "license": "Apache-2.0",
      "peerDependencies": {
        "zod": "^3.25.0 || ^4.0.0"
@@ -872,6 +876,13 @@
      "dev": true,
      "license": "MIT"
    },
+    "node_modules/@tsconfig/node22": {
+      "version": "22.0.5",
+      "resolved": "https://registry.npmjs.org/@tsconfig/node22/-/node22-22.0.5.tgz",
+      "integrity": "sha512-hLf2ld+sYN/BtOJjHUWOk568dvjFQkHnLNa6zce25GIH+vxKfvTgm3qpaH6ToF5tu/NN0IH66s+Bb5wElHrLcw==",
+      "dev": true,
+      "license": "MIT"
+    },
    "node_modules/@tybys/wasm-util": {
      "version": "0.10.2",
      "resolved": "https://registry.npmjs.org/@tybys/wasm-util/-/wasm-util-0.10.2.tgz",
--- a/package.json
+++ b/package.json
@@ -59,14 +59,18 @@
  },
  "author": "Zed Industries",
  "license": "Apache-2.0",
+  "engines": {
+    "node": ">=22"
+  },
  "dependencies": {
-    "@agentclientprotocol/sdk": "0.28.1",
+    "@agentclientprotocol/sdk": "0.29.0",
    "@anthropic-ai/claude-agent-sdk": "0.3.185",
    "zod": "^3.25.0 || ^4.0.0"
  },
  "devDependencies": {
    "@anthropic-ai/sdk": "0.105.0",
    "@eslint/js": "10.0.1",
+    "@tsconfig/node22": "22.0.5",
    "@types/node": "26.0.0",
    "@typescript-eslint/eslint-plugin": "8.61.1",
    "@typescript-eslint/parser": "8.61.1",
--- a/src/acp-agent.ts
+++ b/src/acp-agent.ts
@@ -635,10 +635,21 @@ export function describeAlwaysAllow(
 */
 export interface AcpClient {
  sessionUpdate(params: SessionNotification): Promise<void>;
-  requestPermission(params: RequestPermissionRequest): Promise<RequestPermissionResponse>;
+  /** `signal`, when aborted, sends `$/cancel_request` for the in-flight
+   *  permission request so the client can dismiss its prompt (and settle our
+   *  await) instead of leaving the dialog open after the turn was cancelled. */
+  requestPermission(
+    params: RequestPermissionRequest,
+    signal?: AbortSignal,
+  ): Promise<RequestPermissionResponse>;
  readTextFile(params: ReadTextFileRequest): Promise<ReadTextFileResponse>;
  writeTextFile(params: WriteTextFileRequest): Promise<WriteTextFileResponse>;
-  unstable_createElicitation(params: CreateElicitationRequest): Promise<CreateElicitationResponse>;
+  /** `signal`, when aborted, sends `$/cancel_request` for the in-flight
+   *  elicitation so the client can dismiss its prompt and settle our await. */
+  unstable_createElicitation(
+    params: CreateElicitationRequest,
+    signal?: AbortSignal,
+  ): Promise<CreateElicitationResponse>;
  unstable_completeElicitation(params: CompleteElicitationNotification): Promise<void>;
  /** Send a custom (extension) notification, e.g. `_claude/sdkMessage`. */
  extNotification(method: string, params: Record<string, unknown>): Promise<void>;
@@ -657,8 +668,13 @@ class ClientConnection implements AcpClient {
    return this.ctx.notify(methods.client.session.update, params);
  }

-  requestPermission(params: RequestPermissionRequest): Promise<RequestPermissionResponse> {
-    return this.ctx.request(methods.client.session.requestPermission, params);
+  requestPermission(
+    params: RequestPermissionRequest,
+    signal?: AbortSignal,
+  ): Promise<RequestPermissionResponse> {
+    return this.ctx.request(methods.client.session.requestPermission, params, {
+      cancellationSignal: signal,
+    });
  }

  readTextFile(params: ReadTextFileRequest): Promise<ReadTextFileResponse> {
@@ -669,8 +685,13 @@ class ClientConnection implements AcpClient {
    return this.ctx.request(methods.client.fs.writeTextFile, params);
  }

-  unstable_createElicitation(params: CreateElicitationRequest): Promise<CreateElicitationResponse> {
-    return this.ctx.request(methods.client.elicitation.create, params);
+  unstable_createElicitation(
+    params: CreateElicitationRequest,
+    signal?: AbortSignal,
+  ): Promise<CreateElicitationResponse> {
+    return this.ctx.request(methods.client.elicitation.create, params, {
+      cancellationSignal: signal,
+    });
  }

  unstable_completeElicitation(params: CompleteElicitationNotification): Promise<void> {
@@ -2442,6 +2463,27 @@ export class ClaudeAcpAgent {
    return response;
  }

+  /** Forward a permission request to the client, wiring the tool call's
+   *  `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 the client settles the request (a `cancelled`
+   *  outcome or a `requestCancelled` rejection). Either way we surface the same
+   *  "Tool use aborted" the callers already expect, so a cancelled dialog no
+   *  longer leaves the `await` hanging. */
+  private async requestPermissionFromClient(
+    params: RequestPermissionRequest,
+    signal: AbortSignal,
+  ): Promise<RequestPermissionResponse> {
+    try {
+      return await this.client.requestPermission(params, signal);
+    } catch (error) {
+      if (signal.aborted) {
+        throw new Error("Tool use aborted", { cause: error });
+      }
+      throw error;
+    }
+  }
+
  canUseTool(sessionId: string): CanUseTool {
    return async (toolName, toolInput, { signal, suggestions, toolUseID }) => {
      const alwaysAllowLabel = describeAlwaysAllow(suggestions, toolName);
@@ -2489,19 +2531,22 @@ export class ClaudeAcpAgent {
          session.modes.availableModes.some((m) => m.id === o.optionId),
        );

-        const response = await this.client.requestPermission({
-          options,
-          sessionId,
-          toolCall: {
-            toolCallId: toolUseID,
-            rawInput: toolInput,
-            ...toolInfoFromToolUse(
-              { name: toolName, input: toolInput, id: toolUseID },
-              supportsTerminalOutput,
-              session?.cwd,
-            ),
+        const response = await this.requestPermissionFromClient(
+          {
+            options,
+            sessionId,
+            toolCall: {
+              toolCallId: toolUseID,
+              rawInput: toolInput,
+              ...toolInfoFromToolUse(
+                { name: toolName, input: toolInput, id: toolUseID },
+                supportsTerminalOutput,
+                session?.cwd,
+              ),
+            },
          },
-        });
+          signal,
+        );

        if (signal.aborted || response.outcome?.outcome === "cancelled") {
          throw new Error("Tool use aborted");
@@ -2550,27 +2595,30 @@ export class ClaudeAcpAgent {
        };
      }

-      const response = await this.client.requestPermission({
-        options: [
-          {
-            kind: "allow_always",
-            name: alwaysAllowLabel,
-            optionId: "allow_always",
+      const response = await this.requestPermissionFromClient(
+        {
+          options: [
+            {
+              kind: "allow_always",
+              name: alwaysAllowLabel,
+              optionId: "allow_always",
+            },
+            { kind: "allow_once", name: "Allow", optionId: "allow" },
+            { kind: "reject_once", name: "Reject", optionId: "reject" },
+          ],
+          sessionId,
+          toolCall: {
+            toolCallId: toolUseID,
+            rawInput: toolInput,
+            ...toolInfoFromToolUse(
+              { name: toolName, input: toolInput, id: toolUseID },
+              supportsTerminalOutput,
+              session?.cwd,
+            ),
          },
-          { kind: "allow_once", name: "Allow", optionId: "allow" },
-          { kind: "reject_once", name: "Reject", optionId: "reject" },
-        ],
-        sessionId,
-        toolCall: {
-          toolCallId: toolUseID,
-          rawInput: toolInput,
-          ...toolInfoFromToolUse(
-            { name: toolName, input: toolInput, id: toolUseID },
-            supportsTerminalOutput,
-            session?.cwd,
-          ),
        },
-      });
+        signal,
+      );
      if (signal.aborted || response.outcome?.outcome === "cancelled") {
        throw new Error("Tool use aborted");
      }
@@ -2624,12 +2672,17 @@ export class ClaudeAcpAgent {
      }

      try {
-        const response = await this.client.unstable_createElicitation(createRequest);
+        const response = await this.client.unstable_createElicitation(createRequest, signal);
        if (signal.aborted) {
          return { action: "cancel" };
        }
        return createElicitationResponseToElicitResult(response);
      } catch (error) {
+        // A cancellation we requested (signal aborted) settles as a cancel, not
+        // a hard decline — the elicitation was abandoned, not refused.
+        if (signal.aborted) {
+          return { action: "cancel" };
+        }
        this.logger.error(`Failed to forward MCP elicitation: ${error}`);
        return { action: "decline" };
      }
@@ -2655,8 +2708,13 @@ export class ClaudeAcpAgent {
    const createRequest = askUserQuestionsToCreateRequest(questions, sessionId, toolUseID);
    let response;
    try {
-      response = await this.client.unstable_createElicitation(createRequest);
+      response = await this.client.unstable_createElicitation(createRequest, signal);
    } catch (error) {
+      // A cancellation we requested (signal aborted) settles as an aborted tool
+      // use, matching the post-response check below.
+      if (signal.aborted) {
+        throw new Error("Tool use aborted", { cause: error });
+      }
      this.logger.error(`Failed to present AskUserQuestion elicitation: ${error}`);
      return { behavior: "deny", message: "Could not present the question to the user." };
    }
@@ -4408,6 +4466,38 @@ export function streamEventToAcpNotifications(
  }
 }

+/** Run a `session/prompt` while honoring `$/cancel_request` for it. ACP clients
+ *  normally stop a turn with the `session/cancel` notification, but `signal`
+ *  (the prompt request's abort signal) also fires when the client sends the
+ *  generic `$/cancel_request` for this prompt — the protocol's complementary
+ *  cancellation fallback. Route that to the same `agent.cancel` path so a client
+ *  using only the generic mechanism still stops the turn (and the prompt
+ *  resolves "cancelled" instead of running to completion).
+ *
+ *  The listener is scoped to this call: once the prompt settles it is removed,
+ *  so a later teardown-time abort of the (per-request) signal can't cancel a
+ *  subsequent turn. `signal` also aborts on connection close, in which case
+ *  cancelling the in-flight turn is the desired behavior anyway. */
+export async function runPromptWithCancellation(
+  agent: Pick<ClaudeAcpAgent, "prompt" | "cancel" | "logger">,
+  params: PromptRequest,
+  signal: AbortSignal,
+): Promise<PromptResponse> {
+  const onAbort = () => {
+    // Fire-and-forget: nothing awaits this listener, so swallow (and log) any
+    // rejection rather than surfacing it as an unhandled rejection.
+    agent.cancel({ sessionId: params.sessionId }).catch((error) => {
+      agent.logger.error(`Failed to cancel prompt via $/cancel_request: ${error}`);
+    });
+  };
+  signal.addEventListener("abort", onAbort, { once: true });
+  try {
+    return await agent.prompt(params);
+  } finally {
+    signal.removeEventListener("abort", onAbort);
+  }
+}
+
 export function runAcp() {
  const input = nodeToWebWritable(process.stdout);
  const output = nodeToWebReadable(process.stdin);
@@ -4436,7 +4526,9 @@ export function runAcp() {
      agent.setSessionConfigOption(ctx.params),
    )
    .onRequest(methods.agent.authenticate, (ctx) => agent.authenticate(ctx.params))
-    .onRequest(methods.agent.session.prompt, (ctx) => agent.prompt(ctx.params))
+    .onRequest(methods.agent.session.prompt, (ctx) =>
+      runPromptWithCancellation(agent, ctx.params, ctx.signal),
+    )
    .onNotification(methods.agent.session.cancel, (ctx) => agent.cancel(ctx.params))
    .connect(stream);

--- a/src/tests/acp-agent.test.ts
+++ b/src/tests/acp-agent.test.ts
@@ -38,6 +38,7 @@ import {
  messageIdForGrouping,
  buildConfigOptions,
  discoverCustomAgents,
+  runPromptWithCancellation,
  type AcpClient,
  type SDKMessageFilter,
 } from "../acp-agent.js";
@@ -1783,6 +1784,148 @@ describe("permission requests", () => {
  });
 });

+describe("permission request cancellation", () => {
+  function injectSession(agent: ClaudeAcpAgent, sessionId: string) {
+    function* empty() {}
+    const gen = Object.assign(empty(), { interrupt: vi.fn(), close: vi.fn() });
+    agent.sessions[sessionId] = {
+      query: gen as any,
+      input: new Pushable(),
+      cancelled: false,
+      cwd: "/test",
+      sessionFingerprint: JSON.stringify({ cwd: "/test", mcpServers: [] }),
+      modes: { currentModeId: "default", availableModes: [] },
+      models: { currentModelId: "default", availableModels: [] },
+      modelInfos: [],
+      settingsManager: { dispose: vi.fn() } as any,
+      accumulatedUsage: {
+        inputTokens: 0,
+        outputTokens: 0,
+        cachedReadTokens: 0,
+        cachedWriteTokens: 0,
+      },
+      configOptions: [],
+      agents: [],
+      currentAgent: "default",
+      abortController: new AbortController(),
+      emitRawSDKMessages: false,
+      contextWindowSize: 200000,
+      taskState: new Map(),
+      toolUseCache: {},
+      messageIdToUuid: new Map(),
+    } as any;
+    return agent.sessions[sessionId]!;
+  }
+
+  it("forwards the tool-call signal so a pending permission request is cancelled on abort", async () => {
+    let receivedSignal: AbortSignal | undefined;
+    const mockClient = {
+      sessionUpdate: async () => {},
+      // A `$/cancel_request`-aware client settles the request once the agent
+      // aborts it; model that by rejecting when the forwarded signal fires.
+      requestPermission: (_params: RequestPermissionRequest, signal?: AbortSignal) => {
+        receivedSignal = signal;
+        return new Promise<RequestPermissionResponse>((_resolve, reject) => {
+          signal?.addEventListener("abort", () => reject(new Error("Request cancelled")), {
+            once: true,
+          });
+        });
+      },
+    } as unknown as AcpClient;
+    const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} });
+    injectSession(agent, "session-1");
+
+    const controller = new AbortController();
+    const pending = agent.canUseTool("session-1")("Bash", { command: "ls" }, {
+      signal: controller.signal,
+      suggestions: [],
+      toolUseID: "tool-1",
+    } as any);
+    // Let canUseTool reach the awaited requestPermission before cancelling.
+    await Promise.resolve();
+
+    // The tool-call signal is threaded through as the cancellation signal.
+    expect(receivedSignal).toBe(controller.signal);
+
+    controller.abort();
+
+    await expect(pending).rejects.toThrow("Tool use aborted");
+  });
+
+  it("treats a cancelled permission outcome as an aborted tool use", async () => {
+    const mockClient = {
+      sessionUpdate: async () => {},
+      requestPermission: async () => ({ outcome: { outcome: "cancelled" } }),
+    } as unknown as AcpClient;
+    const agent = new ClaudeAcpAgent(mockClient, { log: () => {}, error: () => {} });
+    injectSession(agent, "session-1");
+
+    await expect(
+      agent.canUseTool("session-1")("Bash", { command: "ls" }, {
+        signal: new AbortController().signal,
+        suggestions: [],
+        toolUseID: "tool-1",
+      } as any),
+    ).rejects.toThrow("Tool use aborted");
+  });
+});
+
+describe("runPromptWithCancellation", () => {
+  function deferred<T>() {
+    let resolve!: (value: T) => void;
+    const promise = new Promise<T>((r) => {
+      resolve = r;
+    });
+    return { promise, resolve };
+  }
+
+  it("cancels the in-flight prompt when the request signal aborts ($/cancel_request)", async () => {
+    const promptResult = deferred<PromptResponse>();
+    const cancel = vi.fn(async () => {});
+    const agent = {
+      prompt: vi.fn(() => promptResult.promise),
+      cancel,
+      logger: { log: () => {}, error: () => {} },
+    } as any;
+
+    const controller = new AbortController();
+    const params = { sessionId: "session-1", prompt: [] } as any;
+    const pending = runPromptWithCancellation(agent, params, controller.signal);
+
+    // No cancel yet — the turn is running.
+    expect(cancel).not.toHaveBeenCalled();
+
+    // Client sends $/cancel_request -> the SDK aborts this request's signal.
+    controller.abort();
+    expect(cancel).toHaveBeenCalledWith({ sessionId: "session-1" });
+
+    // The prompt settles "cancelled" through the normal cancel path.
+    promptResult.resolve({ stopReason: "cancelled" });
+    await expect(pending).resolves.toEqual({ stopReason: "cancelled" });
+  });
+
+  it("does not cancel after the prompt settles normally", async () => {
+    const promptResult = deferred<PromptResponse>();
+    const cancel = vi.fn(async () => {});
+    const agent = {
+      prompt: vi.fn(() => promptResult.promise),
+      cancel,
+      logger: { log: () => {}, error: () => {} },
+    } as any;
+
+    const controller = new AbortController();
+    const params = { sessionId: "session-1", prompt: [] } as any;
+    const pending = runPromptWithCancellation(agent, params, controller.signal);
+
+    promptResult.resolve({ stopReason: "end_turn" });
+    await expect(pending).resolves.toEqual({ stopReason: "end_turn" });
+
+    // A late abort (e.g. per-request signal cleanup) must not cancel a later turn.
+    controller.abort();
+    expect(cancel).not.toHaveBeenCalled();
+  });
+});
+
 describe("stop reason propagation", () => {
  function createMockAgent() {
    const mockClient = {
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,13 +1,7 @@
 {
+  "extends": "@tsconfig/node22/tsconfig.json",
  "compilerOptions": {
-    "target": "ES2021",
-    "module": "NodeNext",
-    "moduleResolution": "NodeNext",
-    "lib": ["ES2021"],
-    "strict": true,
-    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
-    "skipLibCheck": true,
    "outDir": "dist",
    "rootDir": "src",
    "declaration": true,