From e4bd7a189582cd1fe026238bad78c0453c369d7f Mon Sep 17 00:00:00 2001 From: 3ae3ae Date: Mon, 2 Nov 2026 14:19:26 +0900 Subject: [PATCH] fix(acp): clear permission requests a finished turn abandoned An ACP agent can give up on a permission request itself and end its turn normally. Paseo kept that request pending, so the approval card stayed up with nothing listening, and the next prompt reported the agent as waiting. Cancel a turn's open requests whenever that turn ends, cancel a request when its tool reaches a terminal status, and publish the resolved event in both cases. Requests raised outside a turn stay answerable. Closes #7664 Co-Authored-By: Claude Opus 5.5 --- .../src/server/agent/providers/acp-agent.ts | 27 ++-- ...-acp-agent.permission-turn-failure.test.ts | 117 +++++++++++++++--- 2 files changed, 117 insertions(+), 27 deletions(-) diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index 339c491e46..9b5f92399a 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -643,6 +643,7 @@ interface PendingPermission { resolve: (response: RequestPermissionResponse) => void; reject: (error: Error) => void; turnId: string | null; + toolCallId: string; } interface PendingUserMessage { @@ -2644,6 +2645,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { resolve, reject, turnId: this.activeForegroundTurnId, + toolCallId: params.toolCall.toolCallId, }); }); @@ -3207,6 +3209,9 @@ export class ACPAgentSession implements AgentSession, ACPClient { snapshot = this.toolSnapshotTransformer(snapshot); } this.toolCalls.set(toolCallId, snapshot); + if (update.status === "completed" || update.status === "failed") { + this.cancelPendingPermissions((pending) => pending.toolCallId === toolCallId); + } return [this.wrapTimeline(mapToolSnapshotToTimeline(snapshot, this.terminalEntries))]; } @@ -3414,11 +3419,7 @@ export class ACPAgentSession implements AgentSession, ACPClient { event: Extract, ): void { this.deliverTranslatedEvents(this.flushPendingUserMessage()); - // A turn that ends without completing leaves no way to answer its open - // permission requests, so tell the agent they were cancelled. - if (event.type !== "turn_completed") { - this.cancelPendingPermissions(); - } + this.cancelPendingPermissions((pending) => pending.turnId === event.turnId); this.activeForegroundTurnId = null; this.fallbackAssistantMessageId = null; if (this.submittedUserMessageTurnId === event.turnId) { @@ -3427,11 +3428,21 @@ export class ACPAgentSession implements AgentSession, ACPClient { this.pushEvent(event); } - private cancelPendingPermissions(): void { - for (const pending of this.pendingPermissions.values()) { + private cancelPendingPermissions( + matches: (pending: PendingPermission) => boolean = () => true, + ): void { + for (const [requestId, pending] of this.pendingPermissions) { + if (!matches(pending)) continue; + this.pendingPermissions.delete(requestId); pending.resolve({ outcome: { outcome: "cancelled" } }); + this.pushEvent({ + type: "permission_resolved", + provider: this.provider, + requestId, + resolution: { behavior: "deny", message: "Permission request canceled" }, + turnId: pending.turnId ?? undefined, + }); } - this.pendingPermissions.clear(); } private emitBootstrapThreadEvent(): void { diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.permission-turn-failure.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.permission-turn-failure.test.ts index aaa6a5110e..b263529347 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.permission-turn-failure.test.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.permission-turn-failure.test.ts @@ -12,6 +12,7 @@ describe("GenericACPAgentClient permission requests when a turn ends early", () test.each([ { ending: "fail", terminalEvent: "turn_failed" }, { ending: "cancel", terminalEvent: "turn_canceled" }, + { ending: "end_turn", terminalEvent: "turn_completed" }, ] as const)( "answers a permission request left open by a $terminalEvent turn with cancelled", async ({ ending, terminalEvent }) => { @@ -25,6 +26,16 @@ describe("GenericACPAgentClient permission requests when a turn ends early", () await waitForEvent(events, terminalEvent); expect(hasEvent(events, "permission_requested")).toBe(true); expect(session.getPendingPermissions()).toEqual([]); + const requested = permissionRequests(events)[0]!; + expect(resolvedEvents(events)).toEqual([ + { + type: "permission_resolved", + provider: "acp", + requestId: requested.request.id, + resolution: { behavior: "deny", message: "Permission request canceled" }, + turnId: requested.turnId, + }, + ]); const replyStart = events.length; await session.startTurn("turn two"); @@ -39,6 +50,46 @@ describe("GenericACPAgentClient permission requests when a turn ends early", () }); }, ); + + test.each(["completed", "failed"] as const)( + "cancels only the matching permission when its tool becomes %s", + async (status) => { + await withFakeACPAgent(status, async (command, cwd) => { + const client = new GenericACPAgentClient({ logger: createTestLogger(), command }); + const session = await client.createSession({ provider: "acp", cwd }); + try { + const events = collectEvents(session); + await session.startTurn("turn one"); + await waitForToolUpdate(events); + expect(pendingToolIds(session)).toEqual(["tool-2"]); + expect(resolvedEvents(events)).toEqual([ + expect.objectContaining({ requestId: permissionRequests(events)[0]!.request.id }), + ]); + } finally { + await session.close(); + } + }); + }, + ); + + test("keeps a permission raised outside a turn answerable after another turn completes", async () => { + await withFakeACPAgent("outside_turn", async (command, cwd) => { + const client = new GenericACPAgentClient({ logger: createTestLogger(), command }); + const session = await client.createSession({ provider: "acp", cwd }); + try { + const outside = session.getPendingPermissions(); + expect(pendingToolIds(session)).toEqual(["tool-outside"]); + const events = collectEvents(session); + await session.startTurn("turn one"); + await waitForEvent(events, "turn_completed"); + expect(session.getPendingPermissions()).toEqual(outside); + await session.respondToPermission(outside[0]!.id, { behavior: "allow" }); + expect(session.getPendingPermissions()).toEqual([]); + } finally { + await session.close(); + } + }); + }); }); function collectEvents(session: AgentSession): AgentStreamEvent[] { @@ -77,8 +128,24 @@ function assistantText(events: AgentStreamEvent[]): string { .join(""); } +function permissionRequests(events: AgentStreamEvent[]) { + return events.filter((event) => event.type === "permission_requested"); +} + +function resolvedEvents(events: AgentStreamEvent[]) { + return events.filter((event) => event.type === "permission_resolved"); +} + +function pendingToolIds(session: AgentSession) { + return session.getPendingPermissions().map((request) => request.metadata?.toolCallId); +} + +async function waitForToolUpdate(events: AgentStreamEvent[]): Promise { + await expect.poll(() => assistantText(events)).toBe("tool terminal"); +} + async function withFakeACPAgent( - ending: "fail" | "cancel", + ending: "fail" | "cancel" | "end_turn" | "completed" | "failed" | "outside_turn", run: (command: [string, ...string[]], cwd: string) => Promise, ): Promise { const testDir = await mkdtemp(path.join(tmpdir(), "paseo-acp-permission-turn-failure-")); @@ -91,8 +158,7 @@ async function withFakeACPAgent( } } -// Turn one asks for permission, then fails or is cancelled while the request is -// still open. +// Turn one ends while its permission request is still open. // Later turns report whether that request was ever answered. const fakeACPAgentScript = ` const readline = require("node:readline"); @@ -106,10 +172,27 @@ function write(message) { process.stdout.write(JSON.stringify({ jsonrpc: "2.0", ...message }) + "\\n"); } +function ask(sessionId, requestId, toolCallId) { + const toolCall = { toolCallId, title: "echo hello", kind: "execute", status: "pending" }; + write({ method: "session/update", params: { sessionId, update: { sessionUpdate: "tool_call", ...toolCall } } }); + write({ + id: requestId, + method: "session/request_permission", + params: { + sessionId, + toolCall, + options: [ + { optionId: "allow", name: "Allow once", kind: "allow_once" }, + { optionId: "reject", name: "Reject", kind: "reject_once" }, + ], + }, + }); +} + rl.on("line", (line) => { const message = JSON.parse(line); - if (message.method === undefined && message.id === "perm-1") { - permissionAnswer = message.result ?? { error: message.error }; + if (message.method === undefined) { + if (message.id === "perm-1") permissionAnswer = message.result ?? { error: message.error }; return; } if (message.method === "initialize") { @@ -117,6 +200,7 @@ rl.on("line", (line) => { return; } if (message.method === "session/new") { + if (ending === "outside_turn") ask("session-1", "perm-outside", "tool-outside"); write({ id: message.id, result: { sessionId: "session-1" } }); return; } @@ -124,23 +208,18 @@ rl.on("line", (line) => { const sessionId = message.params.sessionId; promptCount += 1; if (promptCount === 1) { - const toolCall = { toolCallId: "tool-1", title: "echo hello", kind: "execute", status: "pending" }; - write({ method: "session/update", params: { sessionId, update: { sessionUpdate: "tool_call", ...toolCall } } }); - write({ - id: "perm-1", - method: "session/request_permission", - params: { - sessionId, - toolCall, - options: [ - { optionId: "allow", name: "Allow once", kind: "allow_once" }, - { optionId: "reject", name: "Reject", kind: "reject_once" }, - ], - }, - }); + ask(sessionId, "perm-1", "tool-1"); + if (ending === "completed" || ending === "failed") { + ask(sessionId, "perm-2", "tool-2"); + write({ method: "session/update", params: { sessionId, update: { sessionUpdate: "tool_call_update", toolCallId: "tool-1", status: ending } } }); + write({ method: "session/update", params: { sessionId, update: { sessionUpdate: "agent_message_chunk", content: { type: "text", text: "tool terminal" } } } }); + return; + } setTimeout(() => { if (ending === "cancel") { write({ id: message.id, result: { stopReason: "cancelled" } }); + } else if (ending === "end_turn" || ending === "outside_turn") { + write({ id: message.id, result: { stopReason: "end_turn" } }); } else { write({ id: message.id, error: { code: -32603, message: "turn failed with a permission open" } }); }