From d61c8b8bbd940dd0011b2ac5fb9dfb308b879f73 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:24:41 -0700 Subject: [PATCH] Leave Claude plan mode when the user approves a plan (#34549) * Leave Claude plan mode when the user approves a plan Approving a proposed plan left Claude in plan mode, so it could not carry the plan out. On an approved plan, while Claude reports plan mode, ask it to return to the chat's own approval mode. The request is written before the approval and never awaited, so its failure cannot hold or reject the approval. Tracked in STA-6663. * Use checked types for Claude plan approval mode * Restore Claude plan approvals by protocol action and mode * Normalize missing Claude plan approval mode before narrowing --- .../claude/claude-plan-approval-mode.test.ts | 231 ++++++++++++++++++ src/main/claude/claude-plan-approval-mode.ts | 52 ++++ src/main/claude/claude-structured-options.ts | 4 +- .../claude-structured-prompt-ownership.ts | 2 + .../claude-structured-session-test-support.ts | 2 + 5 files changed, 289 insertions(+), 2 deletions(-) create mode 100644 src/main/claude/claude-plan-approval-mode.test.ts create mode 100644 src/main/claude/claude-plan-approval-mode.ts diff --git a/src/main/claude/claude-plan-approval-mode.test.ts b/src/main/claude/claude-plan-approval-mode.test.ts new file mode 100644 index 00000000000..49063fd1750 --- /dev/null +++ b/src/main/claude/claude-plan-approval-mode.test.ts @@ -0,0 +1,231 @@ +import type { + PermissionMode, + PermissionResult, + Query, + SDKMessage +} from '@anthropic-ai/claude-agent-sdk' +import { describe, expect, it, vi } from 'vitest' +import { createClaudeControlSurface } from './claude-agent-sdk-control-requests' +import { claudePlanApprovalMode, leaveClaudePlanModeOnApproval } from './claude-plan-approval-mode' +import type { ClaudePendingPrompt } from './claude-prompt-registry' +import { invokeCanUseTool } from './claude-can-use-tool-test-support' +import { + USER_MESSAGE, + acquired, + claudeFrame, + fakeClaude +} from './claude-structured-session-test-support' + +const PLAN_PROMPT: Pick = { toolName: 'ExitPlanMode' } +const ALLOW: PermissionResult = { behavior: 'allow', updatedInput: {}, toolUseID: 'tool-plan' } + +type PlanSession = Parameters[0] + +function planSession( + setPermissionMode: (mode: string) => Promise, + overrides: Partial = {} +): PlanSession { + return { + connection: { setPermissionMode }, + reportedOptions: { reportedPermissionMode: 'plan' }, + launchIntent: { permissionMode: 'acceptEdits' }, + ...overrides + } +} + +function recordingQuery(setPermissionMode: Query['setPermissionMode']): Query { + function unusedControl(): never { + throw new Error('unexpected query control') + } + const messages = (async function* (): AsyncGenerator { + yield* [] + })() + return Object.assign(messages, { + setPermissionMode, + interrupt: unusedControl, + setMcpPermissionModeOverride: unusedControl, + setModel: unusedControl, + setMaxThinkingTokens: unusedControl, + applyFlagSettings: unusedControl, + updateSettings: unusedControl, + initializationResult: unusedControl, + reinitialize: unusedControl, + supportedCommands: unusedControl, + supportedModels: unusedControl, + supportedAgents: unusedControl, + mcpServerStatus: unusedControl, + getContextUsage: unusedControl, + usage_EXPERIMENTAL_MAY_CHANGE_DO_NOT_RELY_ON_THIS_API_YET: unusedControl, + readFile: unusedControl, + reloadPlugins: unusedControl, + reloadSkills: unusedControl, + reloadOutputStyles: unusedControl, + accountInfo: unusedControl, + rewindFiles: unusedControl, + seedReadState: unusedControl, + reconnectMcpServer: unusedControl, + toggleMcpServer: unusedControl, + readMcpResource: unusedControl, + setMcpServers: unusedControl, + streamInput: unusedControl, + stopTask: unusedControl, + backgroundTasks: unusedControl, + close: unusedControl + }) +} + +async function pendingPlanApproval(input: Record, initPermissionMode?: string) { + const claude = fakeClaude({ replayUuid: 'turn-1', initPermissionMode }) + const adapter = await acquired(claude) + await adapter.dispatch({ + sessionId: 'session-1', + clientMessageId: 'client-turn-1', + body: USER_MESSAGE, + fence: 7 + }) + const connection = claude.connections[0] + if (!connection) { + throw new Error('expected Claude connection') + } + claudeFrame(connection, { type: 'system', subtype: 'status', permissionMode: 'plan' }) + const request = new AbortController() + const answered = invokeCanUseTool(connection, 'ExitPlanMode', 'permission-plan', 'tool-plan', { + input, + signal: request.signal + }) + adapter.bindPromptItemId('session-1', 'journal-plan', 'permission-plan') + return { adapter, connection, answered, request } +} + +describe('leaving Claude plan mode on an approved plan', () => { + it.each<[Parameters[0], PermissionMode]>([ + [{ launchIntent: { permissionMode: 'acceptEdits' } }, 'acceptEdits'], + [{ launchIntent: { permissionMode: 'bypassPermissions' } }, 'bypassPermissions'], + [{ launchIntent: {}, firstRootInit: { permissionMode: 'auto' } }, 'auto'], + [{ launchIntent: {}, firstRootInit: { permissionMode: 'dontAsk' } }, 'dontAsk'], + [{ launchIntent: {}, firstRootInit: { permissionMode: 'unknown' } }, 'default'], + // The user's own CLI settings started in plan: approving still lets Claude edit, asking each time. + [{ launchIntent: {}, firstRootInit: { permissionMode: 'plan' } }, 'default'], + [{}, 'default'] + ])('returns to the chat’s own mode (%j)', (session, mode) => { + expect(claudePlanApprovalMode(session)).toBe(mode) + }) + + it('changes nothing for Keep planning, other approvals, or a chat already out of plan', () => { + const setPermissionMode = vi.fn(async () => {}) + const deny: PermissionResult = { behavior: 'deny', message: 'keep planning', toolUseID: 't' } + leaveClaudePlanModeOnApproval(planSession(setPermissionMode), PLAN_PROMPT, deny) + leaveClaudePlanModeOnApproval(planSession(setPermissionMode), { toolName: 'Bash' }, ALLOW) + leaveClaudePlanModeOnApproval( + planSession(setPermissionMode, { reportedOptions: { reportedPermissionMode: 'ask' } }), + PLAN_PROMPT, + ALLOW + ) + expect(setPermissionMode).not.toHaveBeenCalled() + }) + + it.each([ + { label: 'plan text', input: { plan: '# Add multiply' }, mode: 'default', failure: 'none' }, + { label: 'empty input', input: {}, mode: 'default', failure: 'none' }, + { label: 'own dontAsk', input: {}, mode: 'dontAsk', failure: 'none' }, + { label: 'rejected mode request', input: {}, mode: 'default', failure: 'async' }, + { label: 'closed connection', input: {}, mode: 'default', failure: 'sync' } + ])( + 'settles the real plan callback after its mode request: $label', + async ({ input, mode, failure }) => { + const { adapter, connection, answered } = await pendingPlanApproval(input, mode) + const order: string[] = [] + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const controls = createClaudeControlSurface( + recordingQuery(async (requestedMode) => { + order.push(`set_permission_mode:${requestedMode}`) + if (failure === 'async') { + throw new Error('mode rejected') + } + }) + ) + const setMode = vi.spyOn(connection, 'setPermissionMode').mockImplementation( + failure === 'sync' + ? (requestedMode) => { + order.push(`set_permission_mode:${requestedMode}`) + throw new Error('closed') + } + : controls.setPermissionMode + ) + const callback = answered.promise.then((result) => { + order.push('control_response') + return result + }) + try { + await adapter.answerPrompt({ + sessionId: 'session-1', + itemId: 'journal-plan', + kind: 'approval', + response: { kind: 'option', optionId: 'allow' }, + fence: 7, + commit: async () => { + expect(setMode).not.toHaveBeenCalled() + expect(answered.settled()).toBe(false) + } + }) + + await expect(callback).resolves.toMatchObject({ + behavior: 'allow', + updatedInput: input, + toolUseID: 'tool-plan' + }) + expect(order).toEqual([`set_permission_mode:${mode}`, 'control_response']) + expect(setMode).toHaveBeenCalledOnce() + if (failure !== 'none') { + await vi.waitFor(() => expect(warn).toHaveBeenCalledOnce()) + } + } finally { + warn.mockRestore() + setMode.mockRestore() + } + } + ) + + it.each(['failed commit', 'lost ownership'])( + 'does not change mode when approval has %s', + async (failure) => { + const { adapter, connection, answered, request } = await pendingPlanApproval({}) + const setMode = vi.spyOn(connection, 'setPermissionMode') + await expect( + adapter.answerPrompt({ + sessionId: 'session-1', + itemId: 'journal-plan', + kind: 'approval', + response: { kind: 'option', optionId: 'allow' }, + fence: 7, + commit: async () => { + expect(setMode).not.toHaveBeenCalled() + expect(answered.settled()).toBe(false) + if (failure === 'failed commit') { + throw new Error('journal write failed') + } + request.abort() + } + }) + ).rejects.toThrow() + expect(setMode).not.toHaveBeenCalled() + if (failure === 'lost ownership') { + await expect(answered.promise).resolves.toBeNull() + } else { + expect(answered.settled()).toBe(false) + // The failed write released its claim, so a later approval still works. + await adapter.answerPrompt({ + sessionId: 'session-1', + itemId: 'journal-plan', + kind: 'approval', + response: { kind: 'option', optionId: 'allow' }, + fence: 7, + commit: async () => {} + }) + await expect(answered.promise).resolves.toMatchObject({ behavior: 'allow' }) + expect(setMode).toHaveBeenCalledOnce() + } + setMode.mockRestore() + } + ) +}) diff --git a/src/main/claude/claude-plan-approval-mode.ts b/src/main/claude/claude-plan-approval-mode.ts new file mode 100644 index 00000000000..e8e47566032 --- /dev/null +++ b/src/main/claude/claude-plan-approval-mode.ts @@ -0,0 +1,52 @@ +import type { PermissionMode, PermissionResult } from '@anthropic-ai/claude-agent-sdk' +import type { ClaudePendingPrompt } from './claude-prompt-registry' +import type { ClaudeSession } from './structured/claude-structured-session-state' + +/** This process's requested mode, else its first init's; anything else asks for each edit. */ +export function claudePlanApprovalMode( + session: Pick +): PermissionMode { + const mode = + session.launchIntent?.permissionMode ?? session.firstRootInit?.permissionMode ?? 'default' + switch (mode) { + case 'default': + case 'acceptEdits': + case 'dontAsk': + case 'auto': + case 'bypassPermissions': + return mode + default: + return 'default' + } +} + +function logPlanExitFailure(error: unknown): void { + try { + console.warn('[claude] leaving Plan mode after an approved plan failed:', error) + } catch { + // The approval stands whatever happens to this log line. + } +} + +/** Claude stays in plan after an approved ExitPlanMode until told. The request is written + * before the allow and never awaited, so its failure cannot hold or reject the approval. */ +export function leaveClaudePlanModeOnApproval( + session: Pick & { + connection: Pick + }, + prompt: Pick, + reply: PermissionResult +): void { + if ( + prompt.toolName !== 'ExitPlanMode' || + reply.behavior !== 'allow' || + session.reportedOptions.reportedPermissionMode !== 'plan' + ) { + return + } + try { + session.connection.setPermissionMode(claudePlanApprovalMode(session)).catch(logPlanExitFailure) + } catch (error) { + logPlanExitFailure(error) + } +} diff --git a/src/main/claude/claude-structured-options.ts b/src/main/claude/claude-structured-options.ts index 7b23a785c23..b21891f2b00 100644 --- a/src/main/claude/claude-structured-options.ts +++ b/src/main/claude/claude-structured-options.ts @@ -22,8 +22,8 @@ const EFFORT_LEVELS = ['low', 'medium', 'high', 'xhigh', 'max'] as const const OPTION_ORDER = ['model', 'effort', 'fastMode'] as const -/** Approval mode is fixed at startup: a fallback mode is dropped only by reopening for a newly picked model. - * Compares with what Orca itself launched, so the reopened child never matches again. */ +/** A fallback approval mode is dropped only by reopening for a newly picked model (only an approved plan + * changes the live mode). Compares with what Orca itself launched, so the reopened child never matches again. */ export function claudeModelTransitionRestart( sessions: ReadonlyMap ): NonNullable { diff --git a/src/main/claude/claude-structured-prompt-ownership.ts b/src/main/claude/claude-structured-prompt-ownership.ts index 9dab987b431..79ec9ca85e7 100644 --- a/src/main/claude/claude-structured-prompt-ownership.ts +++ b/src/main/claude/claude-structured-prompt-ownership.ts @@ -5,6 +5,7 @@ import { } from '../native-chat/agent-session-wire/structured-agent-session-adapter' import type { StructuredAgentSessionAdapterStop } from '../native-chat/agent-session-wire/structured-agent-session-adapter-stop' import { answerClaudePrompt, cancelClaudeTurn } from './claude-structured-control-actions' +import { leaveClaudePlanModeOnApproval } from './claude-plan-approval-mode' import type { ClaudeLateDispatchSettlement } from './claude-replay-turn-resolution' import { buildClaudePromptReply, claudePromptDismissal } from './claude-structured-prompt-replies' import type { ClaudeSession } from './structured/claude-structured-session-state' @@ -157,6 +158,7 @@ export async function answerClaudeStructuredPrompt(input: { ) { throw new AgentSessionPromptUnavailableError(request.itemId) } + leaveClaudePlanModeOnApproval(session, claim.found.prompt, reply) await answerClaudePrompt(session, claim, reply) } catch (error) { session.prompts.releaseClaim(claim) diff --git a/src/main/claude/claude-structured-session-test-support.ts b/src/main/claude/claude-structured-session-test-support.ts index 99fd760b1e6..b664c69fed6 100644 --- a/src/main/claude/claude-structured-session-test-support.ts +++ b/src/main/claude/claude-structured-session-test-support.ts @@ -56,6 +56,7 @@ export function fakeClaude( initSessionId?: string initUuid?: string initModel?: string + initPermissionMode?: string /** Default: live SessionStart proof; 'none': no hook; 'init': unmeasured startup shape. */ initProof?: 'init' | 'session-start' | 'none' initAccount?: unknown @@ -92,6 +93,7 @@ export function fakeClaude( session_id: options.initSessionId ?? PROVIDER_SESSION_ID, uuid: options.initUuid ?? 'init-uuid', model: options.initModel ?? 'claude-sonnet-5', + ...(options.initPermissionMode ? { permissionMode: options.initPermissionMode } : {}), apiKeySource: 'none', ...(options.capabilities ? { capabilities: options.capabilities } : {}) })