Skip to content

Commit ff4a733

Browse files
authored
fix(cloud-task): relay questions over the durable event stream and park unanswerable ones (#3199)
1 parent e8e5ed5 commit ff4a733

4 files changed

Lines changed: 170 additions & 14 deletions

File tree

packages/agent/src/adapters/claude/permissions/permission-handlers.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,3 +392,44 @@ describe("canUseTool auto mode hands-off approval", () => {
392392
},
393393
);
394394
});
395+
396+
describe("AskUserQuestion cancelled outcomes", () => {
397+
const QUESTION_INPUT = {
398+
question: "Which license should I use?",
399+
options: [{ label: "MIT" }, { label: "Apache 2.0" }],
400+
};
401+
402+
it("denies with the parked-question message when cancelled carries one", async () => {
403+
const context = createContext("AskUserQuestion", {
404+
toolInput: QUESTION_INPUT,
405+
client: {
406+
sessionUpdate: vi.fn().mockResolvedValue(undefined),
407+
requestPermission: vi.fn().mockResolvedValue({
408+
outcome: { outcome: "cancelled" },
409+
_meta: { message: "Waiting for the user to answer." },
410+
}),
411+
},
412+
});
413+
414+
const result = await canUseTool(context);
415+
416+
expect(result.behavior).toBe("deny");
417+
if (result.behavior === "deny") {
418+
expect(result.message).toBe("Waiting for the user to answer.");
419+
}
420+
});
421+
422+
it("aborts the tool use on a bare cancelled outcome", async () => {
423+
const context = createContext("AskUserQuestion", {
424+
toolInput: QUESTION_INPUT,
425+
client: {
426+
sessionUpdate: vi.fn().mockResolvedValue(undefined),
427+
requestPermission: vi.fn().mockResolvedValue({
428+
outcome: { outcome: "cancelled" },
429+
}),
430+
},
431+
});
432+
433+
await expect(canUseTool(context)).rejects.toThrow("Tool use aborted");
434+
});
435+
});

packages/agent/src/adapters/claude/permissions/permission-handlers.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,14 +305,28 @@ async function handleAskUserQuestionTool(
305305
},
306306
});
307307

308+
// A cancelled outcome carrying a message is a deliberate "park the
309+
// question" response (Slack relay, unattended cloud run) — deliver it to
310+
// the model as a denial so it knows to wait for the user instead of
311+
// deciding on its own. A bare cancel remains a tool-use abort.
312+
const customMessage = (response._meta as Record<string, unknown> | undefined)
313+
?.message;
314+
if (
315+
!context.signal?.aborted &&
316+
response.outcome?.outcome === "cancelled" &&
317+
typeof customMessage === "string"
318+
) {
319+
return {
320+
behavior: "deny",
321+
message: customMessage,
322+
};
323+
}
324+
308325
if (context.signal?.aborted || response.outcome?.outcome === "cancelled") {
309326
throw new Error("Tool use aborted");
310327
}
311328

312329
if (response.outcome?.outcome !== "selected") {
313-
const customMessage = (
314-
response._meta as Record<string, unknown> | undefined
315-
)?.message;
316330
return {
317331
behavior: "deny",
318332
message:

packages/agent/src/server/agent-server.ts

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3083,36 +3083,68 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions}
30833083
}
30843084
}
30853085

3086-
// Relay permission requests to the desktop app when:
3086+
// Relay permission requests to the connected client when:
30873087
// - Plan approvals: always relay because they gate autonomy changes
30883088
// that require human confirmation (buffered until desktop connects)
3089-
// - Questions: relay when desktop is connected
3089+
// - Questions: relay when any client can receive and answer them
30903090
// - Edit/bash in "default" mode: relay for manual approval
30913091
// Other modes auto-approve. No client connected → auto-approve
3092-
// (except plan approvals, which wait for a desktop).
3092+
// (except plan approvals, which wait for a desktop, and questions,
3093+
// which are parked for the user instead of being answered blindly).
30933094
{
30943095
const isQuestion = codeToolKind === "question";
30953096
const sessionPermissionMode = this.getSessionPermissionMode();
3096-
const needsDesktopApproval =
3097-
isQuestion ||
3098-
this.shouldRelayPermissionToClient(sessionPermissionMode);
3097+
const needsDesktopApproval = this.shouldRelayPermissionToClient(
3098+
sessionPermissionMode,
3099+
);
3100+
3101+
// With durable event ingest nothing connects to GET /events, so
3102+
// hasDesktopConnected stays false even while the web/desktop task
3103+
// views follow the run through the agent-proxy stream. Those views
3104+
// render permission_request frames and answer via
3105+
// permission_response, so an active event stream counts as a
3106+
// reachable client for questions.
3107+
const hasReachableClient =
3108+
Boolean(this.session?.hasDesktopConnected) ||
3109+
this.eventStreamSender !== null;
30993110

31003111
// A background run has no human to answer a relayed approval
31013112
// (hasDesktopConnected is true from the event-relay reader), so
3102-
// auto-approve rather than hang on it.
3113+
// auto-approve non-question permissions rather than hang on them.
3114+
// Questions are parked (cancelled with message) below so the model
3115+
// does not pick an answer on the user's behalf.
31033116
if (
31043117
mode !== "background" &&
31053118
(isPlanApproval ||
3119+
(isQuestion && hasReachableClient) ||
31063120
(needsDesktopApproval && this.session?.hasDesktopConnected))
31073121
) {
31083122
this.logger.debug("Relaying permission request", {
31093123
kind: params.toolCall?.kind,
31103124
isQuestion,
31113125
hasDesktopConnected: this.session?.hasDesktopConnected ?? false,
3126+
hasReachableClient,
31123127
sessionPermissionMode,
31133128
});
31143129
return this.relayPermissionToClient(params);
31153130
}
3131+
3132+
// A question that cannot be relayed must never fall through to
3133+
// auto-approve: the auto-selected option carries no answers, so the
3134+
// tool would fail with "User did not provide answers" and the model
3135+
// would answer on the user's behalf. Park it for the user instead.
3136+
if (isQuestion) {
3137+
return {
3138+
outcome: { outcome: "cancelled" as const },
3139+
_meta: {
3140+
message:
3141+
"No user is available to answer this question right now. " +
3142+
"Do NOT pick an answer yourself and do NOT re-ask via this tool. " +
3143+
"Restate the question and its options in your response, then end " +
3144+
"your turn so the user can answer when they are back.",
3145+
},
3146+
};
3147+
}
31163148
}
31173149

31183150
if (this.shouldBlockPublishPermission(params)) {

packages/agent/src/server/question-relay.test.ts

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ interface TestableAgentServer {
1717
options: unknown[];
1818
toolCall: unknown;
1919
}) => Promise<{
20-
outcome: { outcome: string };
21-
_meta?: { message?: string };
20+
outcome: { outcome: string; optionId?: string };
21+
_meta?: { message?: string; answers?: Record<string, string> };
2222
}>;
2323
};
2424
questionRelayedToSlack: boolean;
@@ -281,15 +281,84 @@ describe("Question relay", () => {
281281
delete process.env.POSTHOG_CODE_INTERACTION_ORIGIN;
282282
});
283283

284-
it("auto-approves question tools (no Slack relay)", async () => {
285-
const client = server.createCloudClient(TEST_PAYLOAD);
284+
it.each([
285+
[
286+
"no client can receive them",
287+
{ eventStreamActive: false, mode: "interactive" },
288+
],
289+
[
290+
"the run is in background mode even with the event stream active",
291+
{ eventStreamActive: true, mode: "background" },
292+
],
293+
])("parks question tools when %s", async (_label, config) => {
294+
const srv = server as TestableAgentServer & {
295+
eventStreamSender: { enqueue: ReturnType<typeof vi.fn> } | null;
296+
};
297+
if (config.eventStreamActive) {
298+
srv.eventStreamSender = { enqueue: vi.fn() };
299+
}
286300

301+
const client = srv.createCloudClient({
302+
...TEST_PAYLOAD,
303+
mode: config.mode,
304+
});
287305
const result = await client.requestPermission({
288306
options: ALLOW_OPTIONS,
289307
toolCall: { _meta: QUESTION_META },
290308
});
291309

310+
expect(result.outcome.outcome).toBe("cancelled");
311+
expect(result._meta?.message).toContain(
312+
"Do NOT pick an answer yourself",
313+
);
314+
});
315+
316+
it("relays question tools when the durable event stream is active", async () => {
317+
const appendRawLine = vi.fn();
318+
const enqueue = vi.fn();
319+
const srv = server as TestableAgentServer & {
320+
eventStreamSender: { enqueue: typeof enqueue } | null;
321+
resolvePermission: (
322+
requestId: string,
323+
optionId: string,
324+
customInput?: string,
325+
answers?: Record<string, string>,
326+
) => boolean;
327+
};
328+
srv.session = {
329+
payload: TEST_PAYLOAD,
330+
sseController: null,
331+
hasDesktopConnected: false,
332+
logWriter: { appendRawLine },
333+
};
334+
srv.eventStreamSender = { enqueue };
335+
336+
const client = srv.createCloudClient(TEST_PAYLOAD);
337+
const pending = client.requestPermission({
338+
options: ALLOW_OPTIONS,
339+
toolCall: { toolCallId: "question-1", _meta: QUESTION_META },
340+
});
341+
342+
const request = appendRawLine.mock.calls
343+
.map(([, line]) => JSON.parse(line))
344+
.find((n) => n?.method === "_posthog/permission_request");
345+
expect(request).toBeDefined();
346+
expect(enqueue).toHaveBeenCalledWith(
347+
expect.objectContaining({ type: "permission_request" }),
348+
);
349+
350+
srv.resolvePermission(
351+
request.params.requestId as string,
352+
"option_0",
353+
undefined,
354+
{ "Which license should I use?": "MIT" },
355+
);
356+
357+
const result = await pending;
292358
expect(result.outcome.outcome).toBe("selected");
359+
expect(result._meta?.answers).toEqual({
360+
"Which license should I use?": "MIT",
361+
});
293362
});
294363

295364
it("keeps auto-approving permissions after SSE send failures", async () => {

0 commit comments

Comments
 (0)