From 0d1d10d807115f4f7d618d187cc74b3065ff4f44 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 7 Jul 2026 12:21:28 +0100 Subject: [PATCH] fix(cloud-task): relay questions over durable event stream With durable event ingest enabled, cloud task views receive permission_request frames through the durable stream instead of the sandbox SSE relay. Treat an active event stream sender as a reachable client for AskUserQuestion and permission requests. When a question still cannot be relayed, return a cancelled outcome that tells the model to restate the question and wait for the user instead of selecting an answer automatically. Cancelled outcomes carrying a message now reach the model as a denial so the parked-question instruction is delivered. Generated-By: PostHog Code --- .../permissions/permission-handlers.test.ts | 41 ++++++++++ .../claude/permissions/permission-handlers.ts | 20 ++++- packages/agent/src/server/agent-server.ts | 46 +++++++++-- .../agent/src/server/question-relay.test.ts | 77 ++++++++++++++++++- 4 files changed, 170 insertions(+), 14 deletions(-) diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts index 3137059271..46cebe33f1 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts @@ -392,3 +392,44 @@ describe("canUseTool auto mode hands-off approval", () => { }, ); }); + +describe("AskUserQuestion cancelled outcomes", () => { + const QUESTION_INPUT = { + question: "Which license should I use?", + options: [{ label: "MIT" }, { label: "Apache 2.0" }], + }; + + it("denies with the parked-question message when cancelled carries one", async () => { + const context = createContext("AskUserQuestion", { + toolInput: QUESTION_INPUT, + client: { + sessionUpdate: vi.fn().mockResolvedValue(undefined), + requestPermission: vi.fn().mockResolvedValue({ + outcome: { outcome: "cancelled" }, + _meta: { message: "Waiting for the user to answer." }, + }), + }, + }); + + const result = await canUseTool(context); + + expect(result.behavior).toBe("deny"); + if (result.behavior === "deny") { + expect(result.message).toBe("Waiting for the user to answer."); + } + }); + + it("aborts the tool use on a bare cancelled outcome", async () => { + const context = createContext("AskUserQuestion", { + toolInput: QUESTION_INPUT, + client: { + sessionUpdate: vi.fn().mockResolvedValue(undefined), + requestPermission: vi.fn().mockResolvedValue({ + outcome: { outcome: "cancelled" }, + }), + }, + }); + + await expect(canUseTool(context)).rejects.toThrow("Tool use aborted"); + }); +}); diff --git a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts index 485c266c80..44933130bd 100644 --- a/packages/agent/src/adapters/claude/permissions/permission-handlers.ts +++ b/packages/agent/src/adapters/claude/permissions/permission-handlers.ts @@ -305,14 +305,28 @@ async function handleAskUserQuestionTool( }, }); + // A cancelled outcome carrying a message is a deliberate "park the + // question" response (Slack relay, unattended cloud run) — deliver it to + // the model as a denial so it knows to wait for the user instead of + // deciding on its own. A bare cancel remains a tool-use abort. + const customMessage = (response._meta as Record | undefined) + ?.message; + if ( + !context.signal?.aborted && + response.outcome?.outcome === "cancelled" && + typeof customMessage === "string" + ) { + return { + behavior: "deny", + message: customMessage, + }; + } + if (context.signal?.aborted || response.outcome?.outcome === "cancelled") { throw new Error("Tool use aborted"); } if (response.outcome?.outcome !== "selected") { - const customMessage = ( - response._meta as Record | undefined - )?.message; return { behavior: "deny", message: diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index f1d4385ee7..ef1be6a671 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -3083,36 +3083,68 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} } } - // Relay permission requests to the desktop app when: + // Relay permission requests to the connected client when: // - Plan approvals: always relay because they gate autonomy changes // that require human confirmation (buffered until desktop connects) - // - Questions: relay when desktop is connected + // - Questions: relay when any client can receive and answer them // - Edit/bash in "default" mode: relay for manual approval // Other modes auto-approve. No client connected → auto-approve - // (except plan approvals, which wait for a desktop). + // (except plan approvals, which wait for a desktop, and questions, + // which are parked for the user instead of being answered blindly). { const isQuestion = codeToolKind === "question"; const sessionPermissionMode = this.getSessionPermissionMode(); - const needsDesktopApproval = - isQuestion || - this.shouldRelayPermissionToClient(sessionPermissionMode); + const needsDesktopApproval = this.shouldRelayPermissionToClient( + sessionPermissionMode, + ); + + // With durable event ingest nothing connects to GET /events, so + // hasDesktopConnected stays false even while the web/desktop task + // views follow the run through the agent-proxy stream. Those views + // render permission_request frames and answer via + // permission_response, so an active event stream counts as a + // reachable client for questions. + const hasReachableClient = + Boolean(this.session?.hasDesktopConnected) || + this.eventStreamSender !== null; // A background run has no human to answer a relayed approval // (hasDesktopConnected is true from the event-relay reader), so - // auto-approve rather than hang on it. + // auto-approve non-question permissions rather than hang on them. + // Questions are parked (cancelled with message) below so the model + // does not pick an answer on the user's behalf. if ( mode !== "background" && (isPlanApproval || + (isQuestion && hasReachableClient) || (needsDesktopApproval && this.session?.hasDesktopConnected)) ) { this.logger.debug("Relaying permission request", { kind: params.toolCall?.kind, isQuestion, hasDesktopConnected: this.session?.hasDesktopConnected ?? false, + hasReachableClient, sessionPermissionMode, }); return this.relayPermissionToClient(params); } + + // A question that cannot be relayed must never fall through to + // auto-approve: the auto-selected option carries no answers, so the + // tool would fail with "User did not provide answers" and the model + // would answer on the user's behalf. Park it for the user instead. + if (isQuestion) { + return { + outcome: { outcome: "cancelled" as const }, + _meta: { + message: + "No user is available to answer this question right now. " + + "Do NOT pick an answer yourself and do NOT re-ask via this tool. " + + "Restate the question and its options in your response, then end " + + "your turn so the user can answer when they are back.", + }, + }; + } } if (this.shouldBlockPublishPermission(params)) { diff --git a/packages/agent/src/server/question-relay.test.ts b/packages/agent/src/server/question-relay.test.ts index fe1a14be7f..6c0a58c22d 100644 --- a/packages/agent/src/server/question-relay.test.ts +++ b/packages/agent/src/server/question-relay.test.ts @@ -17,8 +17,8 @@ interface TestableAgentServer { options: unknown[]; toolCall: unknown; }) => Promise<{ - outcome: { outcome: string }; - _meta?: { message?: string }; + outcome: { outcome: string; optionId?: string }; + _meta?: { message?: string; answers?: Record }; }>; }; questionRelayedToSlack: boolean; @@ -281,15 +281,84 @@ describe("Question relay", () => { delete process.env.POSTHOG_CODE_INTERACTION_ORIGIN; }); - it("auto-approves question tools (no Slack relay)", async () => { - const client = server.createCloudClient(TEST_PAYLOAD); + it.each([ + [ + "no client can receive them", + { eventStreamActive: false, mode: "interactive" }, + ], + [ + "the run is in background mode even with the event stream active", + { eventStreamActive: true, mode: "background" }, + ], + ])("parks question tools when %s", async (_label, config) => { + const srv = server as TestableAgentServer & { + eventStreamSender: { enqueue: ReturnType } | null; + }; + if (config.eventStreamActive) { + srv.eventStreamSender = { enqueue: vi.fn() }; + } + const client = srv.createCloudClient({ + ...TEST_PAYLOAD, + mode: config.mode, + }); const result = await client.requestPermission({ options: ALLOW_OPTIONS, toolCall: { _meta: QUESTION_META }, }); + expect(result.outcome.outcome).toBe("cancelled"); + expect(result._meta?.message).toContain( + "Do NOT pick an answer yourself", + ); + }); + + it("relays question tools when the durable event stream is active", async () => { + const appendRawLine = vi.fn(); + const enqueue = vi.fn(); + const srv = server as TestableAgentServer & { + eventStreamSender: { enqueue: typeof enqueue } | null; + resolvePermission: ( + requestId: string, + optionId: string, + customInput?: string, + answers?: Record, + ) => boolean; + }; + srv.session = { + payload: TEST_PAYLOAD, + sseController: null, + hasDesktopConnected: false, + logWriter: { appendRawLine }, + }; + srv.eventStreamSender = { enqueue }; + + const client = srv.createCloudClient(TEST_PAYLOAD); + const pending = client.requestPermission({ + options: ALLOW_OPTIONS, + toolCall: { toolCallId: "question-1", _meta: QUESTION_META }, + }); + + const request = appendRawLine.mock.calls + .map(([, line]) => JSON.parse(line)) + .find((n) => n?.method === "_posthog/permission_request"); + expect(request).toBeDefined(); + expect(enqueue).toHaveBeenCalledWith( + expect.objectContaining({ type: "permission_request" }), + ); + + srv.resolvePermission( + request.params.requestId as string, + "option_0", + undefined, + { "Which license should I use?": "MIT" }, + ); + + const result = await pending; expect(result.outcome.outcome).toBe("selected"); + expect(result._meta?.answers).toEqual({ + "Which license should I use?": "MIT", + }); }); it("keeps auto-approving permissions after SSE send failures", async () => {