Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown> | 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<string, unknown> | undefined
)?.message;
return {
behavior: "deny",
message:
Expand Down
46 changes: 39 additions & 7 deletions packages/agent/src/server/agent-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
77 changes: 73 additions & 4 deletions packages/agent/src/server/question-relay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> };
}>;
};
questionRelayedToSlack: boolean;
Expand Down Expand Up @@ -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<typeof vi.fn> } | 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<string, string>,
) => 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 () => {
Expand Down
Loading