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) {