From c6b04f302a2238e9c134d444f92842905af52ecd Mon Sep 17 00:00:00 2001 From: Christoph Leiter Date: Thu, 20 Aug 2026 14:02:00 +0200 Subject: [PATCH] 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 * 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 * 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 Co-authored-by: Mohamed Boudra --- docs/glossary.md | 2 +- docs/providers.md | 2 + .../e2e/browser/permission-steer.real.spec.ts | 83 +++++++++ packages/app/src/composer/index.tsx | 17 +- packages/app/src/composer/input/state.test.ts | 7 + packages/app/src/composer/input/state.ts | 7 + .../server/src/server/agent/agent-manager.ts | 18 +- .../server/src/server/agent/agent-prompt.ts | 10 +- .../src/server/agent/agent-sdk-types.ts | 7 +- .../agent/providers/claude/agent.test.ts | 164 ++++++++++++++++- .../server/agent/providers/claude/agent.ts | 141 +++++++++++--- .../providers/codex-app-server-agent.test.ts | 174 ++++++++++++++++++ .../agent/providers/codex-app-server-agent.ts | 37 ++++ .../codex/test-utils/fake-app-server.ts | 42 +++++ packages/server/src/server/session.ts | 4 + 15 files changed, 683 insertions(+), 32 deletions(-) create mode 100644 packages/app/e2e/browser/permission-steer.real.spec.ts diff --git a/docs/glossary.md b/docs/glossary.md index 84d3e4d31..e19005307 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -43,7 +43,7 @@ Authoritative terminology. UI label wins. Don't invent synonyms; use what's here - **Agent controls** — Provider, model, mode, thinking, and provider-feature controls for an agent or draft agent. Code: `AgentControls` / `DraftAgentControls` (`packages/app/src/composer/agent-controls/index.tsx`). Forbidden: "Agent status bar". - **Composer footer** — Optional area rendered below the composer input but still inside the keyboard-shifted composer layout. Code: `Composer.footer` (`packages/app/src/composer/index.tsx`). - **Composer track** — A contextual lane at the foot of a pane, by the composer input. Specific tracks use the ` track` form: **Queue track**, **Subagents track**, **Tasks track**. The subagents and tasks tracks are pills floating over the transcript, each opening a panel with its rows; the queue track is a lane inside the composer itself. Code: queue track inside `Composer` (`packages/app/src/composer/index.tsx`), pill and panel frame in `packages/app/src/composer/tracks.tsx`, `SubagentsTrack` (`packages/app/src/subagents/track.tsx`), `AgentTaskList` (`packages/app/src/composer/task-list/index.tsx`). -- **Steer** — A follow-up admitted into an agent's already-running turn instead of interrupting it. Interrupting the turn discards steers the provider has not read yet; there is no take-back short of that, and no provider offers one. The stop button and Escape both interrupt. A steer the provider never read keeps its transcript row, matching Codex. UI: "Send and steer". Code: `steerActiveTurn` (`packages/server/src/server/agent/agent-sdk-types.ts`). Don't confuse with **Queue track**, which holds messages client-side until the turn ends and never enters the running turn. Forbidden: "Delivery", "Queued prompt". +- **Steer** — A follow-up admitted into an agent's already-running turn instead of interrupting it. Interrupting the turn discards steers the provider has not read yet; there is no take-back short of that, and no provider offers one. The stop button and Escape both interrupt. A steer the provider never read keeps its transcript row, matching Codex. Sending while the agent waits on a permission denies that request first, so the message reaches the same turn instead of stalling behind it. UI: "Send and steer". Code: `steerActiveTurn` (`packages/server/src/server/agent/agent-sdk-types.ts`). Don't confuse with **Queue track**, which holds messages client-side until the turn ends and never enters the running turn. Forbidden: "Delivery", "Queued prompt". - **Subagent** — User-facing term for an agent session related to a parent agent session. Use **subagents** in UI copy and docs. Internal daemon/provider plumbing may say "child agent" or `child_session`, especially for provider-managed imports; do not surface "child agent" as a product term. - **Attachment tray** — The selected-attachments row inside the composer input, above the text input. Code: `renderAttachmentTray` (`packages/app/src/composer/index.tsx`). Forbidden: "Attachment bar". - **Fork** — Starting a NEW agent session seeded with a copy of another session's curated chat history. The history rides along as a `chat_history` composer attachment on a draft; the fork does **not** resume or branch the provider-side session, and the source session keeps running untouched. Two targets, one surface, one RPC (`agent.fork_context`, gated on the `agentForkContext` host feature). UI: the fork menu lives in the assistant turn footer and offers "Fork in a new tab" / "Fork in a new workspace". On a **completed** turn it sits beside the copy button and pins a boundary to that turn, so the fork stops where the turn ended. On the **in-flight** turn it sits beside the progress loader and elapsed timer, and omits the boundary entirely — `selectForkContextRows` projects the whole timeline when no boundary is given, so the fork captures everything **up to now**, including the response still streaming. That omission is what makes mid-run forking work; pinning a boundary there would silently drop the live response. Forbidden on provider-subagent (read-only) panes. Code: `useForkAgent` (`packages/app/src/hooks/use-fork-agent.ts`), `selectForkContextRows` (`packages/server/src/server/agent/activity-curator.ts`). Don't confuse with: git fork (unrelated; Paseo has no such concept), **Worktree** (filesystem isolation, not history copying). diff --git a/docs/providers.md b/docs/providers.md index 304070583..ae3fbd904 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -85,6 +85,8 @@ Active-turn steering is an optional `AgentSession.steerActiveTurn` operation. Th A steering adapter also owes its interrupt: stopping a turn must discard the steers the provider has not read yet, or one of them resumes the turn the user just stopped. Codex clears pending input when it aborts a turn; Claude does not, so its adapter cancels the SDK messages it queued before calling `query.interrupt()`. +`SteerActiveTurnOptions.clearPendingPermissions` makes permission release part of the provider contract. A provider that accepts such a steer queues it first, denies permissions blocking its delivery, and stops once the steer is read. Steers without the flag leave permissions open. A denied plan remains in the timeline because the pending card was the only other copy of its text. + Rewind accepts the canonical wire `messageId` and resolves it to the provider identity before calling the adapter. A submitted prompt cannot be rewound until its provider echo supplies that identity. Submitted user-message wire items carry the same Paseo ID in `messageId` and `clientMessageId`. Provider adapters attach `clientMessageId` only to the echo for that foreground submission; provider history and externally initiated user rows do not have a Paseo client ID. diff --git a/packages/app/e2e/browser/permission-steer.real.spec.ts b/packages/app/e2e/browser/permission-steer.real.spec.ts new file mode 100644 index 000000000..802ae763a --- /dev/null +++ b/packages/app/e2e/browser/permission-steer.real.spec.ts @@ -0,0 +1,83 @@ +import { mkdtempSync, realpathSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { expect, test } from "../support/fixtures"; +import { submitMessage } from "../support/helpers/composer"; +import { waitForPermissionPrompt } from "../support/helpers/permissions"; +import { cleanupRewindFlow, type AgentHandle, launchAgent } from "../support/helpers/rewind-flow"; + +const scenarios = [ + { + provider: "claude" as const, + providerConfig: { model: "haiku", modeId: "plan" }, + planPrompt: + "Do not ask questions. Produce a concise plan to append the line 'Hello' to README.md, then present it for approval. Do not implement it.", + planText: "README.md", + steerPrompt: "Do not implement anything. Reply exactly CLAUDE_STEER_RECEIVED and stop.", + reply: "CLAUDE_STEER_RECEIVED", + }, + { + provider: "codex" as const, + providerConfig: { featureValues: { plan_mode: true } }, + planPrompt: + "Produce a concise implementation plan with exactly these steps: Inspect permission steering; Implement permission steering; Verify permission steering. Do not implement it.", + planText: "Inspect permission steering", + steerPrompt: "Do not produce another plan. Reply exactly CODEX_STEER_RECEIVED and stop.", + reply: "CODEX_STEER_RECEIVED", + }, +]; + +test.describe("composer steer supersedes plan approval", () => { + for (const scenario of scenarios) { + test(`${scenario.provider} keeps the rejected plan in the timeline`, async ({ + page, + }, testInfo) => { + test.setTimeout(420_000); + const cwd = realpathSync( + mkdtempSync(path.join(tmpdir(), `paseo-permission-steer-${scenario.provider}-`)), + ); + let handle: AgentHandle | undefined; + + try { + handle = await launchAgent({ + page, + provider: scenario.provider, + cwd, + mode: "full-access", + providerConfig: scenario.providerConfig, + }); + await submitMessage(page, scenario.planPrompt); + await waitForPermissionPrompt(page, 180_000); + + const pendingPlan = page.getByTestId("permission-plan-card"); + await expect(pendingPlan).toContainText(scenario.planText); + const pendingScreenshot = testInfo.outputPath(`${scenario.provider}-pending-plan.png`); + await page.screenshot({ path: pendingScreenshot }); + await testInfo.attach(`${scenario.provider} pending plan`, { + path: pendingScreenshot, + contentType: "image/png", + }); + + await submitMessage(page, scenario.steerPrompt); + await expect(pendingPlan).toHaveCount(0, { timeout: 30_000 }); + const rejectedPlan = page.getByTestId("timeline-plan-card"); + await expect(rejectedPlan).toBeVisible({ timeout: 30_000 }); + await expect(rejectedPlan).toContainText(scenario.planText); + await expect( + page.getByTestId("assistant-message").filter({ hasText: scenario.reply }), + ).toBeVisible({ + timeout: 180_000, + }); + + const rejectedScreenshot = testInfo.outputPath(`${scenario.provider}-rejected-plan.png`); + await page.screenshot({ path: rejectedScreenshot }); + await testInfo.attach(`${scenario.provider} rejected plan`, { + path: rejectedScreenshot, + contentType: "image/png", + }); + } finally { + await cleanupRewindFlow({ handle, cwd }); + } + }); + } +}); diff --git a/packages/app/src/composer/index.tsx b/packages/app/src/composer/index.tsx index dc2547d4e..a8f43a3e8 100644 --- a/packages/app/src/composer/index.tsx +++ b/packages/app/src/composer/index.tsx @@ -96,6 +96,7 @@ import { } from "@/attachments/service"; import { resolveAgentControlsMode } from "@/composer/agent-controls/mode"; import { resolveComposerInputMode, type ComposerInputMode } from "@/composer/input-mode"; +import { resolveActiveSendBehavior } from "./input/state"; import { useKeyboardShiftStyle } from "@/hooks/use-keyboard-shift-style"; import { useKeyboardActionHandler } from "@/hooks/use-keyboard-action-handler"; import type { KeyboardActionDefinition } from "@/keyboard/keyboard-action-dispatcher"; @@ -1476,6 +1477,20 @@ function ComposerContentImpl({ const beginAgentCancellation = useSessionStore((state) => state.beginAgentCancellation); const settleAgentCancellation = useSessionStore((state) => state.settleAgentCancellation); const isAgentRunning = hasActiveTurn; + // Queueing behind a permission prompt would strand the message: the turn is + // parked until the request is answered. + const hasPendingPermission = useSessionStore((state) => { + const pendingPermissions = state.sessions[serverId]?.pendingPermissions; + if (!pendingPermissions) return false; + for (const permission of pendingPermissions.values()) { + if (permission.agentId === agentId) return true; + } + return false; + }); + const activeSendBehavior = resolveActiveSendBehavior( + appSettings.sendBehavior, + hasPendingPermission, + ); const hasAgent = agentState.status !== null; const queueWriter = useMemo( @@ -2302,7 +2317,7 @@ function ComposerContentImpl({ voiceServerId={serverId} voiceAgentId={agentId} isAgentRunning={isAgentRunning} - defaultSendBehavior={appSettings.sendBehavior} + defaultSendBehavior={activeSendBehavior} onQueue={handleQueue} onSubmitLoadingPress={submitLoadingPressHandler} onKeyPress={handleCommandKeyPress} diff --git a/packages/app/src/composer/input/state.test.ts b/packages/app/src/composer/input/state.test.ts index aede454e8..5503ea151 100644 --- a/packages/app/src/composer/input/state.test.ts +++ b/packages/app/src/composer/input/state.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { applyDictationTranscript, computeCanStartDictation, + resolveActiveSendBehavior, resolveComposerSurfacePresentation, runAlternateSendAction, runDefaultSendAction, @@ -188,6 +189,12 @@ describe("dictation transcript behavior", () => { }); describe("composer send behavior", () => { + it("sends immediately when queue mode cannot advance past a permission", () => { + expect(resolveActiveSendBehavior("queue", true)).toBe("interrupt"); + expect(resolveActiveSendBehavior("queue", false)).toBe("queue"); + expect(resolveActiveSendBehavior("steer", true)).toBe("steer"); + }); + function actions() { const calls: string[] = []; return { diff --git a/packages/app/src/composer/input/state.ts b/packages/app/src/composer/input/state.ts index 6239fd822..a63aeb02d 100644 --- a/packages/app/src/composer/input/state.ts +++ b/packages/app/src/composer/input/state.ts @@ -5,6 +5,13 @@ import type { MessageInputKeyboardActionKind } from "@/keyboard/actions"; export type SendBehavior = ActiveTurnBehavior | "queue"; +export function resolveActiveSendBehavior( + sendBehavior: SendBehavior, + hasPendingPermission: boolean, +): SendBehavior { + return sendBehavior === "queue" && hasPendingPermission ? "interrupt" : sendBehavior; +} + interface ComposerSurfaceState { opacity: 0 | 1; pointerEvents: "auto" | "none"; diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 9a1838a78..c8e209643 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -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 }; +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 { 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 { 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), + ), }; } diff --git a/packages/server/src/server/agent/agent-prompt.ts b/packages/server/src/server/agent/agent-prompt.ts index 05650efd1..07c7b4f57 100644 --- a/packages/server/src/server/agent/agent-prompt.ts +++ b/packages/server/src/server/agent/agent-prompt.ts @@ -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, }); } diff --git a/packages/server/src/server/agent/agent-sdk-types.ts b/packages/server/src/server/agent/agent-sdk-types.ts index f8e5a3753..b94043c96 100644 --- a/packages/server/src/server/agent/agent-sdk-types.ts +++ b/packages/server/src/server/agent/agent-sdk-types.ts @@ -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; } diff --git a/packages/server/src/server/agent/providers/claude/agent.test.ts b/packages/server/src/server/agent/providers/claude/agent.test.ts index 95ae40f09..402307954 100644 --- a/packages/server/src/server/agent/providers/claude/agent.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.test.ts @@ -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, + options: Record, + ): Promise; + translateMessageToEvents(message: SDKMessage): AgentStreamEvent[]; + }; + + try { + const { turnId } = await session.startTurn("first turn"); + const input = queryFactory.mock.calls[0]?.[0].prompt as AsyncIterable; + 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, + options: Record, + ): Promise; + }; + + 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((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 }).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(), diff --git a/packages/server/src/server/agent/providers/claude/agent.ts b/packages/server/src/server/agent/providers/claude/agent.ts index d16e1187f..fb9726e12 100644 --- a/packages/server/src/server/agent/providers/claude/agent.ts +++ b/packages/server/src/server/agent/providers/claude/agent.ts @@ -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(); + /** Human steers whose text has not reached Claude yet and therefore supersede blocking cards. */ + private readonly permissionClearingSteerUuids = new Set(); 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, + 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, + ): 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, + ): 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 { 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 { 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((resolve, reject) => { const cleanupFns: Array<() => void> = []; const cleanup = () => { diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts index a3cc15b54..d9093a885 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.test.ts @@ -175,6 +175,119 @@ function deferred() { } 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(); @@ -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 }, + }); + }); +}); diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 18df2d8e5..1019cde52 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -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 { + 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); diff --git a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts index 1ae3204fc..3f4a7af01 100644 --- a/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts +++ b/packages/server/src/server/agent/providers/codex/test-utils/fake-app-server.ts @@ -92,6 +92,19 @@ export interface FakeCodexAppServer { reason: string; }): void; waitForCommandApprovalDecision(itemId: string): Promise; + requestFileChangeApproval(params: { + itemId: string; + threadId: string; + turnId: string; + reason: string; + }): void; + requestUserInput(params: { + itemId: string; + threadId: string; + turnId: string; + questions: Array>; + }): void; + waitForApprovalDecision(itemId: string): Promise; 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}`); diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 014d3e727..b0ebb7f24 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -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) {