From 0ba0a9921d1182fb4dbd80dc81d0eea16920713e Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Wed, 3 Jun 2026 13:59:28 +0100 Subject: [PATCH] fix(cloud-task): recover pending permission prompts from the run log on reconnect --- .../features/sessions/service/service.test.ts | 69 ++++++++++++++++++- .../features/sessions/service/service.ts | 51 +++++++++++++- packages/agent/src/acp-extensions.ts | 6 ++ packages/agent/src/server/agent-server.ts | 32 ++++++++- .../agent/src/server/question-relay.test.ts | 51 ++++++++++++++ 5 files changed, 206 insertions(+), 3 deletions(-) diff --git a/apps/code/src/renderer/features/sessions/service/service.test.ts b/apps/code/src/renderer/features/sessions/service/service.test.ts index 1836a6bd4a..c2ad3446a5 100644 --- a/apps/code/src/renderer/features/sessions/service/service.test.ts +++ b/apps/code/src/renderer/features/sessions/service/service.test.ts @@ -313,7 +313,12 @@ vi.mock("@utils/session", async () => { }); import { toast } from "@renderer/utils/toast"; -import { getSessionService, resetSessionService } from "./service"; +import type { StoredLogEntry } from "@shared/types/session-events"; +import { + derivePendingPermissionRequests, + getSessionService, + resetSessionService, +} from "./service"; // --- Test Fixtures --- @@ -4669,3 +4674,65 @@ describe("SessionService", () => { }); }); }); + +describe("derivePendingPermissionRequests", () => { + const request = (requestId: string, toolCallId: string): StoredLogEntry => ({ + type: "notification", + notification: { + method: "_posthog/permission_request", + params: { + requestId, + toolCallId, + toolCall: { toolCallId, title: "Ready to code?" }, + options: [], + }, + }, + }); + const resolved = (requestId: string): StoredLogEntry => ({ + type: "notification", + notification: { + method: "_posthog/permission_resolved", + params: { requestId }, + }, + }); + + it("returns only unanswered requests, carrying their requestId", () => { + const pending = derivePendingPermissionRequests([ + request("r1", "t1"), + resolved("r1"), + request("r2", "t2"), + ]); + + expect(pending.map((p) => p.requestId)).toEqual(["r2"]); + expect(pending[0].toolCall.toolCallId).toBe("t2"); + }); + + it("ignores unrelated entries and requests without a requestId", () => { + const pending = derivePendingPermissionRequests([ + { + type: "notification", + notification: { method: "_posthog/console", params: {} }, + }, + { + type: "notification", + notification: { method: "_posthog/permission_request", params: {} }, + }, + ]); + + expect(pending).toEqual([]); + }); + + it("drops requests missing a toolCall so they never reach the handler", () => { + const pending = derivePendingPermissionRequests([ + { + type: "notification", + notification: { + method: "_posthog/permission_request", + params: { requestId: "r1", options: [] }, + }, + }, + ]); + + expect(pending).toEqual([]); + }); +}); diff --git a/apps/code/src/renderer/features/sessions/service/service.ts b/apps/code/src/renderer/features/sessions/service/service.ts index 554bb91ab2..4faf612c20 100644 --- a/apps/code/src/renderer/features/sessions/service/service.ts +++ b/apps/code/src/renderer/features/sessions/service/service.ts @@ -152,6 +152,42 @@ function hasSessionPromptEvent(events: AcpMessage[]): boolean { ); } +type DerivedPermissionRequest = Pick< + CloudTaskPermissionRequestUpdate, + "requestId" | "toolCall" | "options" +>; + +export function derivePendingPermissionRequests( + entries: StoredLogEntry[], +): DerivedPermissionRequest[] { + const requests = new Map(); + const resolved = new Set(); + for (const entry of entries) { + const method = entry.notification?.method; + if (!method) continue; + const params = (entry.notification?.params ?? {}) as { + requestId?: string; + toolCall?: CloudTaskPermissionRequestUpdate["toolCall"]; + options?: CloudTaskPermissionRequestUpdate["options"]; + }; + if (typeof params.requestId !== "string") continue; + if (isNotification(method, POSTHOG_NOTIFICATIONS.PERMISSION_RESOLVED)) { + resolved.add(params.requestId); + } else if ( + isNotification(method, POSTHOG_NOTIFICATIONS.PERMISSION_REQUEST) && + typeof params.toolCall?.toolCallId === "string" && + Array.isArray(params.options) + ) { + requests.set(params.requestId, { + requestId: params.requestId, + toolCall: params.toolCall, + options: params.options, + }); + } + } + return [...requests.values()].filter((r) => !resolved.has(r.requestId)); +} + function buildCloudDefaultConfigOptions( initialMode: string | undefined, adapter: Adapter = "claude", @@ -1463,7 +1499,7 @@ export class SessionService { private handleCloudPermissionRequest( taskRunId: string, - update: CloudTaskPermissionRequestUpdate, + update: DerivedPermissionRequest, ): void { log.info("Cloud permission request received", { taskRunId, @@ -1497,6 +1533,15 @@ export class SessionService { notifyPermissionRequest(session.taskTitle, session.taskId); } + private surfacePersistedPendingPermissions( + taskRunId: string, + entries: StoredLogEntry[], + ): void { + for (const request of derivePendingPermissionRequests(entries)) { + this.handleCloudPermissionRequest(taskRunId, request); + } + } + // --- Prompt Handling --- /** @@ -3552,6 +3597,10 @@ export class SessionService { } } + if (update.kind === "snapshot" && !isTerminalStatus(update.status)) { + this.surfacePersistedPendingPermissions(taskRunId, update.newEntries); + } + // NOTE: Don't auto-flush on `!isPromptPending && queue.length > 0` here. // Setup-phase log batches (`_posthog/progress`, `_posthog/console`) stream // in BEFORE the agent emits its initial `session/prompt` request, so diff --git a/packages/agent/src/acp-extensions.ts b/packages/agent/src/acp-extensions.ts index 759d778080..e00c95ebb9 100644 --- a/packages/agent/src/acp-extensions.ts +++ b/packages/agent/src/acp-extensions.ts @@ -69,6 +69,12 @@ export const POSTHOG_NOTIFICATIONS = { /** Response to a relayed permission request (plan approval, question) */ PERMISSION_RESPONSE: "_posthog/permission_response", + + /** Permission request raised by the agent, persisted to the log so a reconnecting client can recover its requestId */ + PERMISSION_REQUEST: "_posthog/permission_request", + + /** Permission request resolved, persisted so a reconnecting client can tell it is no longer pending */ + PERMISSION_RESOLVED: "_posthog/permission_resolved", } as const; /** diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 8f6551556d..848755c4a7 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -253,6 +253,7 @@ export class AgentServer { outcome: { outcome: "selected"; optionId: string }; _meta?: Record; }) => void; + toolCallId?: string; } >(); @@ -2508,6 +2509,7 @@ ${signedCommitInstructions} _meta?: Record; }> { const requestId = crypto.randomUUID(); + const toolCallId = params.toolCall?.toolCallId as string | undefined; this.broadcastEvent({ type: "permission_request", @@ -2516,11 +2518,33 @@ ${signedCommitInstructions} toolCall: params.toolCall, }); + // Persist the request so a client that connects after the live event can + // recover the requestId from the log and re-surface the prompt. + this.persistPermissionLifecycle(POSTHOG_NOTIFICATIONS.PERMISSION_REQUEST, { + requestId, + toolCallId, + options: params.options, + toolCall: params.toolCall, + }); + return new Promise((resolve) => { - this.pendingPermissions.set(requestId, { resolve }); + this.pendingPermissions.set(requestId, { resolve, toolCallId }); }); } + private persistPermissionLifecycle( + method: string, + params: Record, + ): void { + if (!this.session) return; + // appendRawLine wraps the line in the {type, timestamp, notification} + // envelope, so pass the bare notification (matching broadcastTurnComplete). + this.session.logWriter.appendRawLine( + this.session.payload.run_id, + JSON.stringify({ jsonrpc: "2.0", method, params }), + ); + } + private resolvePermission( requestId: string, optionId: string, @@ -2532,6 +2556,12 @@ ${signedCommitInstructions} this.pendingPermissions.delete(requestId); + this.persistPermissionLifecycle(POSTHOG_NOTIFICATIONS.PERMISSION_RESOLVED, { + requestId, + toolCallId: pending.toolCallId, + optionId, + }); + const meta: Record = {}; if (customInput) meta.customInput = customInput; if (answers) meta.answers = answers; diff --git a/packages/agent/src/server/question-relay.test.ts b/packages/agent/src/server/question-relay.test.ts index 63692ce56f..073328a40a 100644 --- a/packages/agent/src/server/question-relay.test.ts +++ b/packages/agent/src/server/question-relay.test.ts @@ -386,6 +386,57 @@ describe("Question relay", () => { }); }); + describe("permission lifecycle persisted to log", () => { + it("persists the request (with requestId) and its resolution", async () => { + const appendRawLine = vi.fn(); + const srv = server as TestableAgentServer & { + relayPermissionToClient: (p: { + options: unknown[]; + toolCall?: unknown; + }) => Promise<{ outcome: { outcome: string; optionId: string } }>; + resolvePermission: (requestId: string, optionId: string) => boolean; + session: { + payload: typeof TEST_PAYLOAD; + sseController: null; + logWriter: { appendRawLine: typeof appendRawLine }; + }; + }; + srv.session = { + payload: TEST_PAYLOAD, + sseController: null, + logWriter: { appendRawLine }, + }; + + const logged = (method: string) => + appendRawLine.mock.calls + .map(([, line]) => JSON.parse(line)) + .find((n) => n?.method === method); + + const promise = srv.relayPermissionToClient({ + options: [{ kind: "allow_once", optionId: "allow", name: "Allow" }], + toolCall: { toolCallId: "tool-1", title: "Ready to code?" }, + }); + + const request = logged("_posthog/permission_request"); + expect(request).toBeTruthy(); + expect(typeof request.params.requestId).toBe("string"); + expect(request.params.toolCallId).toBe("tool-1"); + const requestId = request.params.requestId; + + expect(srv.resolvePermission(requestId, "allow")).toBe(true); + + const resolved = logged("_posthog/permission_resolved"); + expect(resolved).toBeTruthy(); + expect(resolved.params.requestId).toBe(requestId); + expect(resolved.params.toolCallId).toBe("tool-1"); + expect(resolved.params.optionId).toBe("allow"); + + await expect(promise).resolves.toMatchObject({ + outcome: { outcome: "selected", optionId: "allow" }, + }); + }); + }); + describe("relayAgentResponse duplicate suppression", () => { it("skips relay when questionRelayedToSlack is set", async () => { const relaySpy = vi