fix(server): stop a steered prompt from deadlocking on a permission (#3585)
* fix(server): stop a steered prompt from deadlocking on a permission Claude parks its turn inside the blocking canUseTool callback while a plan approval or a tool prompt waits for an answer. The SDK stops to read its input stream there, so a message sent at the open card is queued and never read. The user must reject the card or interrupt the agent by hand, and then send the message a second time. Deny the pending permissions after the steer is admitted. The prompt is already in the SDK input queue when the permission callback returns, so Claude reads it in the same turn. Plan mode stays on and no turn restarts. One pass is not enough. A turn that made several tool calls opens its next request 10 to 300 milliseconds after the previous one is answered, so a single snapshot advances the batch by one and parks the turn again. The Claude adapter already tracks the steers the SDK has not read, so expose that as an optional AgentSession.hasUnreadSteer and answer each new request while that method reports the prompt as unread. The release stops the moment the prompt is read, so a permission the agent asks for after it acts on the message still reaches the user. Only the two entrypoints that carry a message from a person set the flag. A scheduled prompt, a child-finished notification, and an agent-to-agent prompt must not answer a card the user is looking at. Denial left no timeline row, so a denied plan disappeared from the transcript. Claude and Codex now push a plan_approval row with approved: false. For Codex the row comes from resolvePlanPermission, which is the path that dismissPendingPlanApprovals also uses. In the app, queue mode held the message until the turn ended, and a parked turn never ends. The composer now sends while a permission is pending, and the button label agrees with the submit path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(server): keep a post-prompt permission out of the release The release loop answered the pending requests before it asked whether the agent had read the prompt. The agent can read the prompt during a poll interval and then open a permission request because of it. The next pass denied that request, so a decision that belongs to the user was made for them. The loop now asks first and answers second. The first pass stays unconditional, because a provider that does not report unread steers must still get one release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(server): make steer permission clearing provider-owned Keep the daemon policy explicit while letting each provider queue the steer and release permissions that block it. This removes timing-based polling and preserves rejected plans as canonical timeline rows. * fix(server): preserve omitted steer options Only construct provider steer options when permission clearing is requested. System prompts otherwise keep the existing undefined options contract. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 5
Mohamed Boudra
parent
975dbd433b
commit
c6b04f302a
@@ -35,6 +35,7 @@ import {
|
||||
type AgentPromptInput,
|
||||
type AgentProvider,
|
||||
type AgentRunOptions,
|
||||
type AgentSteerOptions,
|
||||
type AgentRunResult,
|
||||
type AgentSession,
|
||||
type AgentSessionConfig,
|
||||
@@ -294,6 +295,12 @@ export type ActiveTurnSteerDispatchResult =
|
||||
| { status: "inactive" | "steered" }
|
||||
| { status: "replaced"; iterator: AsyncGenerator<AgentStreamEvent> };
|
||||
|
||||
function stripSteerOptions(options?: AgentSteerOptions): AgentRunOptions | undefined {
|
||||
if (!options) return undefined;
|
||||
const { clearPendingPermissions: _, ...runOptions } = options;
|
||||
return runOptions;
|
||||
}
|
||||
|
||||
export interface WaitForAgentOptions {
|
||||
signal?: AbortSignal;
|
||||
waitForActive?: boolean;
|
||||
@@ -2395,7 +2402,7 @@ export class AgentManager {
|
||||
async steerAgentRun(
|
||||
agentId: string,
|
||||
prompt: AgentPromptInput,
|
||||
options?: AgentRunOptions,
|
||||
options?: AgentSteerOptions,
|
||||
): Promise<SteerResult> {
|
||||
const agent = this.requireSessionAgent(agentId);
|
||||
const expectedTurnId = agent.activeForegroundTurnId ?? agent.activeTurnId;
|
||||
@@ -2423,7 +2430,7 @@ export class AgentManager {
|
||||
async steerOrReplaceActiveTurn(
|
||||
agentId: string,
|
||||
prompt: AgentPromptInput,
|
||||
options?: AgentRunOptions,
|
||||
options?: AgentSteerOptions,
|
||||
): Promise<ActiveTurnSteerDispatchResult> {
|
||||
const agent = this.requireSessionAgent(agentId);
|
||||
const expectedTurnId = agent.activeForegroundTurnId ?? agent.activeTurnId;
|
||||
@@ -2457,7 +2464,12 @@ export class AgentManager {
|
||||
this.assertSteerAdmissionOwnsTurn(agent, expectedTurnId);
|
||||
return {
|
||||
status: "replaced",
|
||||
iterator: await this.replaceAdmittedForegroundTurn(agent, expectedTurnId, prompt, options),
|
||||
iterator: await this.replaceAdmittedForegroundTurn(
|
||||
agent,
|
||||
expectedTurnId,
|
||||
prompt,
|
||||
stripSteerOptions(options),
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -27,6 +27,8 @@ export interface StartAgentRunOptions {
|
||||
replaceRunning?: boolean;
|
||||
activeTurnBehavior?: ActiveTurnBehavior;
|
||||
runOptions?: AgentRunOptions;
|
||||
/** Ask the provider to deny permissions blocking this steer. */
|
||||
clearPendingPermissions?: boolean;
|
||||
}
|
||||
|
||||
export type PromptDispatchDisposition = "out_of_band" | "steered" | "turn_started";
|
||||
@@ -47,7 +49,10 @@ async function steerOrReplaceActiveRun(
|
||||
if (options?.activeTurnBehavior !== "steer") {
|
||||
return null;
|
||||
}
|
||||
const result = await agentManager.steerOrReplaceActiveTurn(agentId, prompt, options.runOptions);
|
||||
const steerOptions = options.clearPendingPermissions
|
||||
? { ...options.runOptions, clearPendingPermissions: true }
|
||||
: options.runOptions;
|
||||
const result = await agentManager.steerOrReplaceActiveTurn(agentId, prompt, steerOptions);
|
||||
if (result.status === "steered") {
|
||||
return { disposition: "steered" };
|
||||
}
|
||||
@@ -193,6 +198,8 @@ export interface SendPromptToAgentParams {
|
||||
* schedule fires, notify-on-finish).
|
||||
*/
|
||||
unarchive?: boolean;
|
||||
/** See {@link StartAgentRunOptions.clearPendingPermissions}. */
|
||||
clearPendingPermissions?: boolean;
|
||||
logger: Logger;
|
||||
}
|
||||
|
||||
@@ -262,6 +269,7 @@ export async function sendPromptToAgent(
|
||||
return await startAgentRun(params.agentManager, params.agentId, params.prompt, params.logger, {
|
||||
replaceRunning: true,
|
||||
activeTurnBehavior: params.activeTurnBehavior,
|
||||
clearPendingPermissions: params.clearPendingPermissions,
|
||||
runOptions,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -214,9 +214,14 @@ export interface AgentRunOptions {
|
||||
clientMessageId?: string;
|
||||
}
|
||||
|
||||
export interface AgentSteerOptions extends AgentRunOptions {
|
||||
/** Deny permissions that block this steer. An accepted steer must honor this contract. */
|
||||
clearPendingPermissions?: boolean;
|
||||
}
|
||||
|
||||
export type SteerResult = { status: "accepted" } | { status: "unavailable" };
|
||||
|
||||
export interface SteerActiveTurnOptions extends AgentRunOptions {
|
||||
export interface SteerActiveTurnOptions extends AgentSteerOptions {
|
||||
expectedTurnId: string;
|
||||
}
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@ import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { afterEach, describe, expect, test, vi } from "vitest";
|
||||
import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk";
|
||||
import type { PermissionResult, SDKMessage, SDKUserMessage } from "@anthropic-ai/claude-agent-sdk";
|
||||
|
||||
import { createTestLogger } from "../../../../test-utils/test-logger.js";
|
||||
import * as executableUtils from "../../../../executable-resolution/executable-resolution.js";
|
||||
@@ -877,6 +877,110 @@ describe("ClaudeAgentSession features", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("a human steer supersedes blocking permissions until Claude reads it", async () => {
|
||||
const { queryFactory } = createQueryMock();
|
||||
const client = new ClaudeAgentClient({
|
||||
logger,
|
||||
queryFactory,
|
||||
resolveBinary: async () => "/test/claude/bin",
|
||||
});
|
||||
const session = await client.createSession({ provider: "claude", cwd: process.cwd() });
|
||||
const internal = session as unknown as {
|
||||
handlePermissionRequest(
|
||||
name: string,
|
||||
input: Record<string, unknown>,
|
||||
options: Record<string, unknown>,
|
||||
): Promise<PermissionResult>;
|
||||
translateMessageToEvents(message: SDKMessage): AgentStreamEvent[];
|
||||
};
|
||||
|
||||
try {
|
||||
const { turnId } = await session.startTurn("first turn");
|
||||
const input = queryFactory.mock.calls[0]?.[0].prompt as AsyncIterable<SDKUserMessage>;
|
||||
const iterator = input[Symbol.asyncIterator]();
|
||||
await iterator.next();
|
||||
|
||||
const firstPermission = internal.handlePermissionRequest(
|
||||
"ExitPlanMode",
|
||||
{ plan: "First plan" },
|
||||
{ toolUseID: "tool-1" },
|
||||
);
|
||||
expect(session.getPendingPermissions()).toHaveLength(1);
|
||||
|
||||
await expect(
|
||||
session.steerActiveTurn?.("review this instead", {
|
||||
expectedTurnId: turnId,
|
||||
clearPendingPermissions: true,
|
||||
}),
|
||||
).resolves.toEqual({ status: "accepted" });
|
||||
await expect(firstPermission).resolves.toMatchObject({
|
||||
behavior: "deny",
|
||||
interrupt: undefined,
|
||||
message: expect.stringContaining("message instead of approving"),
|
||||
});
|
||||
|
||||
const steer = await iterator.next();
|
||||
const steerUuid = steer.value?.uuid;
|
||||
expect(steerUuid).toEqual(expect.any(String));
|
||||
|
||||
await expect(
|
||||
internal.handlePermissionRequest(
|
||||
"Write",
|
||||
{ file_path: "SECOND.md" },
|
||||
{ toolUseID: "tool-2" },
|
||||
),
|
||||
).resolves.toMatchObject({ behavior: "deny", interrupt: undefined });
|
||||
|
||||
internal.translateMessageToEvents({
|
||||
type: "command_lifecycle",
|
||||
command_uuid: steerUuid,
|
||||
state: "started",
|
||||
} as unknown as SDKMessage);
|
||||
const laterPermission = internal.handlePermissionRequest(
|
||||
"Write",
|
||||
{ file_path: "LATER.md" },
|
||||
{ toolUseID: "tool-3" },
|
||||
);
|
||||
expect(session.getPendingPermissions()).toHaveLength(1);
|
||||
const requestId = session.getPendingPermissions()[0]!.id;
|
||||
await session.respondToPermission(requestId, { behavior: "deny", message: "test cleanup" });
|
||||
await expect(laterPermission).resolves.toMatchObject({ behavior: "deny" });
|
||||
} finally {
|
||||
await session.close();
|
||||
}
|
||||
});
|
||||
|
||||
test("a non-human steer leaves a pending permission for the user", async () => {
|
||||
const { queryFactory } = createQueryMock();
|
||||
const client = new ClaudeAgentClient({
|
||||
logger,
|
||||
queryFactory,
|
||||
resolveBinary: async () => "/test/claude/bin",
|
||||
});
|
||||
const session = await client.createSession({ provider: "claude", cwd: process.cwd() });
|
||||
const internal = session as unknown as {
|
||||
handlePermissionRequest(
|
||||
name: string,
|
||||
input: Record<string, unknown>,
|
||||
options: Record<string, unknown>,
|
||||
): Promise<PermissionResult>;
|
||||
};
|
||||
|
||||
try {
|
||||
const { turnId } = await session.startTurn("first turn");
|
||||
const permission = internal.handlePermissionRequest("Write", {}, { toolUseID: "tool-1" });
|
||||
await expect(
|
||||
session.steerActiveTurn?.("system notification", { expectedTurnId: turnId }),
|
||||
).resolves.toEqual({ status: "accepted" });
|
||||
expect(session.getPendingPermissions()).toHaveLength(1);
|
||||
const requestId = session.getPendingPermissions()[0]!.id;
|
||||
await session.respondToPermission(requestId, { behavior: "deny", message: "test cleanup" });
|
||||
await expect(permission).resolves.toMatchObject({ behavior: "deny" });
|
||||
} finally {
|
||||
await session.close();
|
||||
}
|
||||
});
|
||||
|
||||
test.each([
|
||||
["supported model", "claude-opus-4-8", { type: "disabled" }, undefined],
|
||||
["unsupported model", "claude-fable-5", { type: "adaptive" }, "high"],
|
||||
@@ -1162,6 +1266,64 @@ describe("normalizeClaudeAskUserQuestionUpdatedInput", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("denying a plan leaves the plan readable in the timeline", async () => {
|
||||
const client = new ClaudeAgentClient({
|
||||
logger: createTestLogger(),
|
||||
resolveBinary: async () => "/test/claude/bin",
|
||||
});
|
||||
const session = await client.createSession({
|
||||
provider: "claude",
|
||||
cwd: process.cwd(),
|
||||
});
|
||||
|
||||
const request = {
|
||||
id: "permission-plan-1",
|
||||
provider: "claude",
|
||||
name: "ExitPlanMode",
|
||||
kind: "plan",
|
||||
input: { plan: "Ship the thing" },
|
||||
};
|
||||
|
||||
const events: AgentStreamEvent[] = [];
|
||||
const unsubscribe = session.subscribe((event) => events.push(event));
|
||||
|
||||
new Promise<unknown>((resolve, reject) => {
|
||||
(
|
||||
session as unknown as {
|
||||
pendingPermissions: Map<
|
||||
string,
|
||||
{
|
||||
request: typeof request;
|
||||
resolve: (value: unknown) => void;
|
||||
reject: (error: Error) => void;
|
||||
}
|
||||
>;
|
||||
}
|
||||
).pendingPermissions.set(request.id, { request, resolve, reject });
|
||||
}).catch(() => undefined);
|
||||
|
||||
try {
|
||||
await session.respondToPermission(request.id, {
|
||||
behavior: "deny",
|
||||
message: "The user answered with a message instead of approving.",
|
||||
});
|
||||
|
||||
const planRow = events.find(
|
||||
(event) =>
|
||||
event.type === "timeline" &&
|
||||
event.item.type === "tool_call" &&
|
||||
event.item.name === "plan_approval",
|
||||
);
|
||||
expect(planRow).toBeDefined();
|
||||
const item = (planRow as { item: Extract<AgentTimelineItem, { type: "tool_call" }> }).item;
|
||||
expect(item.detail).toEqual({ type: "plan", text: "Ship the thing" });
|
||||
expect(item.metadata).toMatchObject({ approved: false });
|
||||
} finally {
|
||||
unsubscribe();
|
||||
await session.close();
|
||||
}
|
||||
});
|
||||
|
||||
test("respondToPermission maps other answer text back to Claude question keys", async () => {
|
||||
const client = new ClaudeAgentClient({
|
||||
logger: createTestLogger(),
|
||||
|
||||
@@ -365,6 +365,8 @@ const CLAUDE_ROOT_ONLY_COMMANDS = new Set([
|
||||
const INTERRUPT_TOOL_USE_PLACEHOLDER = "[Request interrupted by user for tool use]";
|
||||
const INTERRUPT_PLACEHOLDER_PATTERN = /^\[Request interrupted by user(?:[^\]]*)\]$/;
|
||||
const NO_RESPONSE_REQUESTED_PLACEHOLDER = "No response requested.";
|
||||
const STEER_SUPERSEDED_PERMISSION_MESSAGE =
|
||||
"The user answered with a message instead of approving. Their message follows.";
|
||||
const UUID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
|
||||
|
||||
interface SlashCommandInvocation {
|
||||
@@ -2032,6 +2034,8 @@ class ClaudeAgentSession implements AgentSession {
|
||||
* UUIDs are provider-private; never let them escape the adapter boundary.
|
||||
*/
|
||||
private readonly queuedSteerUuids = new Set<string>();
|
||||
/** Human steers whose text has not reached Claude yet and therefore supersede blocking cards. */
|
||||
private readonly permissionClearingSteerUuids = new Set<string>();
|
||||
private claudeSessionId: string | null;
|
||||
private persistence: AgentPersistenceHandle | null;
|
||||
private currentMode: PermissionMode;
|
||||
@@ -2295,13 +2299,34 @@ class ClaudeAgentSession implements AgentSession {
|
||||
) {
|
||||
return { status: "unavailable" };
|
||||
}
|
||||
if (message.uuid) {
|
||||
this.queuedSteerUuids.add(message.uuid);
|
||||
}
|
||||
input.push(message);
|
||||
this.enqueueSteer(input, message, options.clearPendingPermissions === true);
|
||||
return { status: "accepted" };
|
||||
}
|
||||
|
||||
private enqueueSteer(
|
||||
input: AsyncMessageInput<SDKUserMessage>,
|
||||
message: SDKUserMessage,
|
||||
clearPendingPermissions: boolean,
|
||||
): void {
|
||||
const uuid = message.uuid;
|
||||
if (uuid) this.queuedSteerUuids.add(uuid);
|
||||
if (uuid && clearPendingPermissions) {
|
||||
this.permissionClearingSteerUuids.add(uuid);
|
||||
}
|
||||
try {
|
||||
input.push(message);
|
||||
if (clearPendingPermissions) {
|
||||
this.denyPendingPermissionsSupersededBySteer();
|
||||
}
|
||||
} catch (error) {
|
||||
if (uuid) {
|
||||
this.queuedSteerUuids.delete(uuid);
|
||||
this.permissionClearingSteerUuids.delete(uuid);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
subscribe(callback: (event: AgentStreamEvent) => void): () => void {
|
||||
this.subscribers.add(callback);
|
||||
return () => {
|
||||
@@ -2469,6 +2494,83 @@ class ClaudeAgentSession implements AgentSession {
|
||||
return Array.from(this.pendingPermissions.values()).map((entry) => entry.request);
|
||||
}
|
||||
|
||||
/**
|
||||
* A denied request is the only record the transcript gets. Plans especially:
|
||||
* the pending card is the only place the plan text lives, so losing it means
|
||||
* the user can no longer read what they just declined.
|
||||
*/
|
||||
private recordDeniedPermissionTimeline(
|
||||
request: AgentPermissionRequest,
|
||||
response: Extract<AgentPermissionResponse, { behavior: "deny" }>,
|
||||
): void {
|
||||
if (request.kind === "tool") {
|
||||
this.pushToolCall(
|
||||
mapClaudeFailedToolCall({
|
||||
name: request.name,
|
||||
callId:
|
||||
(typeof request.metadata?.toolUseId === "string" ? request.metadata.toolUseId : null) ??
|
||||
request.id,
|
||||
input: request.input ?? null,
|
||||
output: null,
|
||||
error: { message: response.message ?? "Permission denied" },
|
||||
}),
|
||||
);
|
||||
return;
|
||||
}
|
||||
if (request.kind === "plan") {
|
||||
let planText: string | null = null;
|
||||
if (typeof request.metadata?.planText === "string") {
|
||||
planText = request.metadata.planText;
|
||||
} else if (typeof request.input?.plan === "string") {
|
||||
planText = request.input.plan;
|
||||
}
|
||||
if (!planText) return;
|
||||
this.pushToolCall({
|
||||
type: "tool_call",
|
||||
name: "plan_approval",
|
||||
callId: request.id,
|
||||
status: "completed",
|
||||
error: null,
|
||||
detail: { type: "plan", text: planText },
|
||||
metadata: {
|
||||
approved: false,
|
||||
actionId: response.selectedActionId ?? "reject",
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
private resolveDeniedPermission(
|
||||
request: AgentPermissionRequest,
|
||||
response: Extract<AgentPermissionResponse, { behavior: "deny" }>,
|
||||
): PermissionResult {
|
||||
this.recordDeniedPermissionTimeline(request, response);
|
||||
this.pushEvent({
|
||||
type: "permission_resolved",
|
||||
provider: "claude",
|
||||
requestId: request.id,
|
||||
resolution: response,
|
||||
});
|
||||
return {
|
||||
behavior: "deny",
|
||||
message: response.message ?? "Permission request denied",
|
||||
interrupt: response.interrupt,
|
||||
};
|
||||
}
|
||||
|
||||
private denyPendingPermissionsSupersededBySteer(): void {
|
||||
for (const [requestId, pending] of this.pendingPermissions) {
|
||||
this.pendingPermissions.delete(requestId);
|
||||
pending.cleanup?.();
|
||||
pending.resolve(
|
||||
this.resolveDeniedPermission(pending.request, {
|
||||
behavior: "deny",
|
||||
message: STEER_SUPERSEDED_PERMISSION_MESSAGE,
|
||||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async respondToPermission(requestId: string, response: AgentPermissionResponse): Promise<void> {
|
||||
const pending = this.pendingPermissions.get(requestId);
|
||||
if (!pending) {
|
||||
@@ -2512,26 +2614,8 @@ class ClaudeAgentSession implements AgentSession {
|
||||
};
|
||||
pending.resolve(result);
|
||||
} else {
|
||||
if (pending.request.kind === "tool") {
|
||||
this.pushToolCall(
|
||||
mapClaudeFailedToolCall({
|
||||
name: pending.request.name,
|
||||
callId:
|
||||
(typeof pending.request.metadata?.toolUseId === "string"
|
||||
? pending.request.metadata.toolUseId
|
||||
: null) ?? pending.request.id,
|
||||
input: pending.request.input ?? null,
|
||||
output: null,
|
||||
error: { message: response.message ?? "Permission denied" },
|
||||
}),
|
||||
);
|
||||
}
|
||||
const result: PermissionResult = {
|
||||
behavior: "deny",
|
||||
message: response.message ?? "Permission request denied",
|
||||
interrupt: response.interrupt,
|
||||
};
|
||||
pending.resolve(result);
|
||||
pending.resolve(this.resolveDeniedPermission(pending.request, response));
|
||||
return;
|
||||
}
|
||||
|
||||
this.pushEvent({
|
||||
@@ -3917,6 +4001,7 @@ class ClaudeAgentSession implements AgentSession {
|
||||
private async discardQueuedSteers(query: Query): Promise<void> {
|
||||
const uuids = [...this.queuedSteerUuids];
|
||||
this.queuedSteerUuids.clear();
|
||||
this.permissionClearingSteerUuids.clear();
|
||||
if (uuids.length === 0) return;
|
||||
// The SDK runtime supports this, but its public Query type has not caught up. Keep the
|
||||
// compatibility escape hatch inside the Claude adapter.
|
||||
@@ -4020,6 +4105,7 @@ class ClaudeAgentSession implements AgentSession {
|
||||
const lifecycle = readClaudeCommandLifecycle(message);
|
||||
if (!lifecycle || lifecycle.state === "queued") return;
|
||||
this.queuedSteerUuids.delete(lifecycle.commandUuid);
|
||||
this.permissionClearingSteerUuids.delete(lifecycle.commandUuid);
|
||||
}
|
||||
|
||||
private appendTaskStateEvent(message: SDKMessage, events: AgentStreamEvent[]): void {
|
||||
@@ -4526,6 +4612,13 @@ class ClaudeAgentSession implements AgentSession {
|
||||
request,
|
||||
});
|
||||
|
||||
if (this.permissionClearingSteerUuids.size > 0) {
|
||||
return this.resolveDeniedPermission(request, {
|
||||
behavior: "deny",
|
||||
message: STEER_SUPERSEDED_PERMISSION_MESSAGE,
|
||||
});
|
||||
}
|
||||
|
||||
return await new Promise<PermissionResult>((resolve, reject) => {
|
||||
const cleanupFns: Array<() => void> = [];
|
||||
const cleanup = () => {
|
||||
|
||||
@@ -175,6 +175,119 @@ function deferred<T>() {
|
||||
}
|
||||
|
||||
describe("Codex active-turn steering admission", () => {
|
||||
test("a steer without the clearing contract leaves permissions open", async () => {
|
||||
const appServer = createFakeCodexAppServer({
|
||||
"turn/steer": () => ({ turn: { id: "native-A" } }),
|
||||
});
|
||||
const { session, paseoTurnId } = await startPublicSteeringSession(appServer);
|
||||
castInternals<{ emitSyntheticPlanApprovalRequest: (planText: string) => void }>(
|
||||
session,
|
||||
).emitSyntheticPlanApprovalRequest("Ship the thing");
|
||||
|
||||
await expect(
|
||||
session.steerActiveTurn!("background notification", { expectedTurnId: paseoTurnId }),
|
||||
).resolves.toEqual({ status: "accepted" });
|
||||
expect(session.getPendingPermissions()).toHaveLength(1);
|
||||
|
||||
const requestId = session.getPendingPermissions()[0]!.id;
|
||||
await session.respondToPermission(requestId, { behavior: "deny", message: "test cleanup" });
|
||||
await session.close();
|
||||
appServer.assertNoErrors();
|
||||
});
|
||||
|
||||
test("a clearing steer denies every pending permission through its provider handler", async () => {
|
||||
const appServer = createFakeCodexAppServer({
|
||||
"turn/steer": () => ({ turn: { id: "native-A" } }),
|
||||
});
|
||||
const { session, paseoTurnId } = await startPublicSteeringSession(appServer);
|
||||
const events: AgentStreamEvent[] = [];
|
||||
session.subscribe((event) => events.push(event));
|
||||
|
||||
const commandPermission = waitForNextPermission(session);
|
||||
appServer.requestCommandApproval({
|
||||
itemId: "command-1",
|
||||
threadId: "thread-1",
|
||||
turnId: "native-A",
|
||||
command: "git status",
|
||||
cwd: "/workspace/project",
|
||||
reason: "Needs approval",
|
||||
});
|
||||
await commandPermission;
|
||||
|
||||
const filePermission = waitForNextPermission(session);
|
||||
appServer.requestFileChangeApproval({
|
||||
itemId: "file-1",
|
||||
threadId: "thread-1",
|
||||
turnId: "native-A",
|
||||
reason: "Apply the patch",
|
||||
});
|
||||
await filePermission;
|
||||
|
||||
const questionPermission = waitForNextPermission(session);
|
||||
appServer.requestUserInput({
|
||||
itemId: "question-1",
|
||||
threadId: "thread-1",
|
||||
turnId: "native-A",
|
||||
questions: [
|
||||
{
|
||||
id: "choice",
|
||||
header: "Choice",
|
||||
question: "Which option?",
|
||||
options: [{ label: "One" }, { label: "Two" }],
|
||||
},
|
||||
],
|
||||
});
|
||||
await questionPermission;
|
||||
|
||||
const mcpPermission = waitForNextPermission(session);
|
||||
appServer.requestMcpElicitation({
|
||||
threadId: "thread-1",
|
||||
turnId: "native-A",
|
||||
serverName: "browser",
|
||||
message: "Open this page?",
|
||||
requestedSchema: { type: "object", properties: {} },
|
||||
});
|
||||
await mcpPermission;
|
||||
|
||||
castInternals<{ emitSyntheticPlanApprovalRequest: (planText: string) => void }>(
|
||||
session,
|
||||
).emitSyntheticPlanApprovalRequest("Ship the thing");
|
||||
expect(session.getPendingPermissions()).toHaveLength(5);
|
||||
|
||||
await expect(
|
||||
session.steerActiveTurn!("review this instead", {
|
||||
expectedTurnId: paseoTurnId,
|
||||
clearPendingPermissions: true,
|
||||
}),
|
||||
).resolves.toEqual({ status: "accepted" });
|
||||
|
||||
expect(session.getPendingPermissions()).toEqual([]);
|
||||
await expect(appServer.waitForApprovalDecision("command-1")).resolves.toEqual({
|
||||
decision: "decline",
|
||||
});
|
||||
await expect(appServer.waitForApprovalDecision("file-1")).resolves.toEqual({
|
||||
decision: "decline",
|
||||
});
|
||||
await expect(appServer.waitForApprovalDecision("question-1")).resolves.toEqual({ answers: {} });
|
||||
await expect(appServer.waitForMcpElicitationDecision()).resolves.toEqual({
|
||||
action: "decline",
|
||||
content: null,
|
||||
_meta: null,
|
||||
});
|
||||
expect(events).toContainEqual(
|
||||
expect.objectContaining({
|
||||
type: "timeline",
|
||||
item: expect.objectContaining({
|
||||
name: "plan_approval",
|
||||
detail: { type: "plan", text: "Ship the thing" },
|
||||
metadata: expect.objectContaining({ approved: false }),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
await session.close();
|
||||
appServer.assertNoErrors();
|
||||
});
|
||||
|
||||
test("does not steer B when A completes while command resolution is pending", async () => {
|
||||
const commandResolution = deferred<{ commandName: string } | null>();
|
||||
const resolverEntered = deferred<void>();
|
||||
@@ -5549,3 +5662,64 @@ describe("Codex importable sessions", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Codex denied plan approvals", () => {
|
||||
function planApprovalRows(events: AgentStreamEvent[]) {
|
||||
return events.filter(
|
||||
(event) =>
|
||||
event.type === "timeline" &&
|
||||
event.item.type === "tool_call" &&
|
||||
event.item.name === "plan_approval",
|
||||
);
|
||||
}
|
||||
|
||||
function createPlanSession(): { session: CodexTestSession; events: AgentStreamEvent[] } {
|
||||
const session = createSession();
|
||||
const events: AgentStreamEvent[] = [];
|
||||
session.subscribe((event) => events.push(event));
|
||||
castInternals<{ emitSyntheticPlanApprovalRequest: (planText: string) => void }>(
|
||||
session,
|
||||
).emitSyntheticPlanApprovalRequest("Ship the thing");
|
||||
return { session, events };
|
||||
}
|
||||
|
||||
function pendingPlanRequestId(session: CodexTestSession): string {
|
||||
const [request] = session.getPendingPermissions?.() ?? [];
|
||||
if (!request) throw new Error("expected a pending plan permission");
|
||||
return request.id;
|
||||
}
|
||||
|
||||
test("answering the card with a denial records the plan decision", async () => {
|
||||
const { session, events } = createPlanSession();
|
||||
const requestId = pendingPlanRequestId(session);
|
||||
|
||||
await session.respondToPermission?.(requestId, {
|
||||
behavior: "deny",
|
||||
message: "The user answered with a message instead of approving.",
|
||||
});
|
||||
|
||||
const [row] = planApprovalRows(events);
|
||||
expect(row).toBeDefined();
|
||||
expect((row as { item: { detail: unknown; metadata?: unknown } }).item).toMatchObject({
|
||||
detail: { type: "plan", text: "Ship the thing" },
|
||||
metadata: { approved: false },
|
||||
});
|
||||
});
|
||||
|
||||
test("a plan superseded by a new prompt records the same decision", () => {
|
||||
const { session, events } = createPlanSession();
|
||||
|
||||
castInternals<{ dismissPendingPlanApprovals: (message: string) => void }>(
|
||||
session,
|
||||
).dismissPendingPlanApprovals("Dismissed by a new prompt");
|
||||
|
||||
// dismissPendingPlanApprovals goes straight to resolvePlanPermission, so a
|
||||
// row emitted from the response handler would miss this route entirely.
|
||||
const [row] = planApprovalRows(events);
|
||||
expect(row).toBeDefined();
|
||||
expect((row as { item: { detail: unknown; metadata?: unknown } }).item).toMatchObject({
|
||||
detail: { type: "plan", text: "Ship the thing" },
|
||||
metadata: { approved: false },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4163,6 +4163,9 @@ export class CodexAppServerAgentSession implements AgentSession {
|
||||
if (acknowledgedTurnId !== nativeTurnId) {
|
||||
throw new Error("Codex returned an invalid steer acknowledgement");
|
||||
}
|
||||
if (options.clearPendingPermissions) {
|
||||
await this.clearPendingPermissionsForSteer();
|
||||
}
|
||||
return { status: "accepted" };
|
||||
} catch (error) {
|
||||
if (isDefinitiveCodexSteerRejection(error)) return { status: "unavailable" };
|
||||
@@ -4461,7 +4464,41 @@ export class CodexAppServerAgentSession implements AgentSession {
|
||||
}
|
||||
}
|
||||
|
||||
private async clearPendingPermissionsForSteer(): Promise<void> {
|
||||
const requestIds = Array.from(this.pendingPermissionHandlers.keys());
|
||||
for (const requestId of requestIds) {
|
||||
if (!this.pendingPermissionHandlers.has(requestId)) continue;
|
||||
await this.respondToPermission(requestId, {
|
||||
behavior: "deny",
|
||||
message: "The user answered with a message instead of approving. Their message follows.",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
private resolvePlanPermission(requestId: string, resolution: AgentPermissionResponse): void {
|
||||
if (resolution.behavior === "deny") {
|
||||
// Every route into a denial lands here — the response handler, a new
|
||||
// prompt, and an accepted steer — so the transcript record belongs here
|
||||
// rather than in handlePlanPermissionResponse.
|
||||
const planText =
|
||||
this.pendingPermissionHandlers.get(requestId)?.planText ??
|
||||
this.pendingPermissions.get(requestId)?.metadata?.planText;
|
||||
if (typeof planText === "string") {
|
||||
this.emitEvent({
|
||||
type: "timeline",
|
||||
provider: CODEX_PROVIDER,
|
||||
item: {
|
||||
type: "tool_call",
|
||||
callId: requestId,
|
||||
name: "plan_approval",
|
||||
status: "completed",
|
||||
error: null,
|
||||
detail: { type: "plan", text: planText },
|
||||
metadata: { approved: false },
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
this.pendingPermissionHandlers.delete(requestId);
|
||||
this.pendingPermissions.delete(requestId);
|
||||
this.resolvedPermissionRequests.add(requestId);
|
||||
|
||||
@@ -92,6 +92,19 @@ export interface FakeCodexAppServer {
|
||||
reason: string;
|
||||
}): void;
|
||||
waitForCommandApprovalDecision(itemId: string): Promise<unknown>;
|
||||
requestFileChangeApproval(params: {
|
||||
itemId: string;
|
||||
threadId: string;
|
||||
turnId: string;
|
||||
reason: string;
|
||||
}): void;
|
||||
requestUserInput(params: {
|
||||
itemId: string;
|
||||
threadId: string;
|
||||
turnId: string;
|
||||
questions: Array<Record<string, unknown>>;
|
||||
}): void;
|
||||
waitForApprovalDecision(itemId: string): Promise<unknown>;
|
||||
requestMcpElicitation(params: {
|
||||
threadId: string;
|
||||
turnId: string | null;
|
||||
@@ -516,6 +529,35 @@ export function createFakeCodexAppServer(
|
||||
);
|
||||
},
|
||||
async waitForCommandApprovalDecision(itemId) {
|
||||
return await this.waitForApprovalDecision(itemId);
|
||||
},
|
||||
requestFileChangeApproval(params) {
|
||||
const requestId = nextServerRequestId;
|
||||
nextServerRequestId += 1;
|
||||
approvalRequestIds.set(params.itemId, requestId);
|
||||
child.stdout.write(
|
||||
`${JSON.stringify({
|
||||
jsonrpc: "2.0",
|
||||
id: requestId,
|
||||
method: "item/fileChange/requestApproval",
|
||||
params,
|
||||
})}\n`,
|
||||
);
|
||||
},
|
||||
requestUserInput(params) {
|
||||
const requestId = nextServerRequestId;
|
||||
nextServerRequestId += 1;
|
||||
approvalRequestIds.set(params.itemId, requestId);
|
||||
child.stdout.write(
|
||||
`${JSON.stringify({
|
||||
jsonrpc: "2.0",
|
||||
id: requestId,
|
||||
method: "item/tool/requestUserInput",
|
||||
params,
|
||||
})}\n`,
|
||||
);
|
||||
},
|
||||
async waitForApprovalDecision(itemId) {
|
||||
const requestId = approvalRequestIds.get(itemId);
|
||||
if (requestId === undefined) {
|
||||
throw new Error(`No pending fake Codex app-server approval for ${itemId}`);
|
||||
|
||||
@@ -3372,6 +3372,9 @@ export class Session {
|
||||
prompt,
|
||||
messageId,
|
||||
runOptions,
|
||||
// A typed or spoken message from the human answers any permission the
|
||||
// agent is blocked on.
|
||||
clearPendingPermissions: true,
|
||||
logger: this.sessionLogger,
|
||||
});
|
||||
return { ok: true };
|
||||
@@ -7192,6 +7195,7 @@ export class Session {
|
||||
prompt,
|
||||
messageId: msg.messageId,
|
||||
activeTurnBehavior: msg.activeTurnBehavior ?? "interrupt",
|
||||
clearPendingPermissions: true,
|
||||
logger: this.sessionLogger,
|
||||
});
|
||||
} catch (error) {
|
||||
|
||||
Reference in New Issue
Block a user