From 011731ea9e5a7c2d2a1edc65d600f4f8526118f6 Mon Sep 17 00:00:00 2001 From: Devansh Jain <31609257+devanshrj@users.noreply.github.com> Date: Wed, 18 Mar 2026 17:03:32 -0700 Subject: [PATCH] fix(plan-mode): require manual ExitPlanMode approval in yolo (#1440) --- src/cli/App.tsx | 29 ++++--- src/cli/components/ApprovalSwitch.tsx | 5 ++ src/cli/components/StaticPlanApproval.tsx | 83 ++++++++++++------- src/headless.ts | 4 +- .../cli/permission-mode-retry-wiring.test.ts | 44 ++++++++++ src/tests/tools/interactivepolicy.test.ts | 2 +- src/tools/interactivePolicy.ts | 2 +- 7 files changed, 118 insertions(+), 51 deletions(-) diff --git a/src/cli/App.tsx b/src/cli/App.tsx index 8f20f81..ca49d3e 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -12603,9 +12603,14 @@ ${SYSTEM_REMINDER_CLOSE} // acceptEdits/yolo), keep their chosen mode instead of downgrading. const currentMode = permissionMode.getMode(); if (currentMode === "plan") { - const restoreMode = acceptEdits - ? "acceptEdits" - : (permissionMode.getModeBeforePlan() ?? "default"); + const previousMode = permissionMode.getModeBeforePlan(); + const restoreMode = + // If the user was in YOLO before entering plan mode, always restore it. + previousMode === "bypassPermissions" + ? "bypassPermissions" + : acceptEdits + ? "acceptEdits" + : (previousMode ?? "default"); permissionMode.setMode(restoreMode); setUiPermissionMode(restoreMode); } else { @@ -12717,13 +12722,12 @@ ${SYSTEM_REMINDER_CLOSE} const hasUsablePlan = planFileExists(fallbackPlanPath); if (mode !== "plan") { + if (hasUsablePlan) { + // Keep approval flow alive and let user manually approve. + return; + } + if (mode === "bypassPermissions") { - if (hasUsablePlan) { - // YOLO mode with a plan file — auto-approve ExitPlanMode. - lastAutoHandledExitPlanToolCallIdRef.current = approval.toolCallId; - handlePlanApprove(); - return; - } // YOLO mode but no plan file yet — tell agent to write it first. const planFilePath = activePlanPath ?? fallbackPlanPath; const plansDir = join(homedir(), ".letta", "plans"); @@ -12734,10 +12738,6 @@ ${SYSTEM_REMINDER_CLOSE} ); return; } - if (hasUsablePlan) { - // Other modes: keep approval flow alive and let user manually approve. - return; - } // Plan mode state was lost and no plan file is recoverable (e.g., CLI restart) const statusId = uid("status"); @@ -12793,7 +12793,6 @@ ${SYSTEM_REMINDER_CLOSE} }, [ pendingApprovals, approvalResults.length, - handlePlanApprove, handlePlanKeepPlanning, refreshDerived, queueApprovalResults, @@ -12970,7 +12969,7 @@ If using apply_patch, use this exact relative patch path: ${applyPatchRelativePa // Guard EnterPlanMode: // When in bypassPermissions (YOLO) mode, auto-approve EnterPlanMode and stay // in YOLO — the agent gets plan instructions but keeps full permissions. - // The existing ExitPlanMode guard then auto-approves the exit too. + // ExitPlanMode still requires explicit user approval. useEffect(() => { const currentIndex = approvalResults.length; const approval = pendingApprovals[currentIndex]; diff --git a/src/cli/components/ApprovalSwitch.tsx b/src/cli/components/ApprovalSwitch.tsx index 7f14875..e21e772 100644 --- a/src/cli/components/ApprovalSwitch.tsx +++ b/src/cli/components/ApprovalSwitch.tsx @@ -1,4 +1,5 @@ import { memo } from "react"; +import { permissionMode } from "../../permissions/mode"; import type { AdvancedDiffSuccess } from "../helpers/diff"; import type { ApprovalRequest } from "../helpers/stream"; import { @@ -230,12 +231,16 @@ export const ApprovalSwitch = memo( // 1. ExitPlanMode → StaticPlanApproval if (toolName === "ExitPlanMode" && onPlanApprove && onPlanKeepPlanning) { + const showAcceptEditsOption = + permissionMode.getMode() === "plan" && + permissionMode.getModeBeforePlan() !== "bypassPermissions"; return ( onPlanApprove(false)} onApproveAndAcceptEdits={() => onPlanApprove(true)} onKeepPlanning={onPlanKeepPlanning} onCancel={onCancel ?? (() => {})} + showAcceptEditsOption={showAcceptEditsOption} isFocused={isFocused} planContent={planContent} planFilePath={planFilePath} diff --git a/src/cli/components/StaticPlanApproval.tsx b/src/cli/components/StaticPlanApproval.tsx index 65f56b7..c4118e1 100644 --- a/src/cli/components/StaticPlanApproval.tsx +++ b/src/cli/components/StaticPlanApproval.tsx @@ -12,6 +12,7 @@ type Props = { onApproveAndAcceptEdits: () => void; onKeepPlanning: (reason: string) => void; onCancel: () => void; // For CTRL-C to queue denial (like other approval screens) + showAcceptEditsOption?: boolean; isFocused?: boolean; planContent?: string; planFilePath?: string; @@ -35,6 +36,7 @@ export const StaticPlanApproval = memo( onApproveAndAcceptEdits, onKeepPlanning, onCancel, + showAcceptEditsOption = true, isFocused = true, planContent, planFilePath, @@ -69,9 +71,10 @@ export const StaticPlanApproval = memo( }); }, [planContent, planFilePath, agentName]); - const customOptionIndex = 2; + const customOptionIndex = showAcceptEditsOption ? 2 : 1; const maxOptionIndex = customOptionIndex; - const isOnCustomOption = selectedOption === customOptionIndex; + const effectiveSelectedOption = Math.min(selectedOption, maxOptionIndex); + const isOnCustomOption = effectiveSelectedOption === customOptionIndex; const customOptionPlaceholder = "Type here to tell Letta Code what to change"; @@ -127,9 +130,9 @@ export const StaticPlanApproval = memo( // When on regular options if (key.return) { - if (selectedOption === 0) { + if (showAcceptEditsOption && effectiveSelectedOption === 0) { onApproveAndAcceptEdits(); - } else if (selectedOption === 1) { + } else { onApprove(); } return; @@ -141,10 +144,14 @@ export const StaticPlanApproval = memo( // Number keys for quick selection (only for fixed options, not custom text input) if (input === "1") { - onApproveAndAcceptEdits(); + if (showAcceptEditsOption) { + onApproveAndAcceptEdits(); + } else { + onApprove(); + } return; } - if (input === "2") { + if (showAcceptEditsOption && input === "2") { onApprove(); return; } @@ -169,51 +176,63 @@ export const StaticPlanApproval = memo( {/* Options */} - {/* Option 1: Yes, and auto-accept edits */} + {/* Option 1 */} - {selectedOption === 0 ? "❯" : " "} 1. + {effectiveSelectedOption === 0 ? "❯" : " "} 1. - Yes, and auto-accept edits + {showAcceptEditsOption + ? "Yes, and auto-accept edits" + : "Yes, proceed (bypassPermissions / yolo mode)"} {/* Option 2: Yes, and manually approve edits */} - - - - {selectedOption === 1 ? "❯" : " "} 2. - + {showAcceptEditsOption && ( + + + + {effectiveSelectedOption === 1 ? "❯" : " "} 2. + + + + + Yes, and manually approve edits + + - - - Yes, and manually approve edits - - - + )} {/* Option 3: Custom input */} @@ -221,7 +240,7 @@ export const StaticPlanApproval = memo( - {isOnCustomOption ? "❯" : " "} 3. + {isOnCustomOption ? "❯" : " "} {customOptionIndex + 1}. diff --git a/src/headless.ts b/src/headless.ts index 80a7eac..d2d200f 100644 --- a/src/headless.ts +++ b/src/headless.ts @@ -1944,8 +1944,8 @@ ${SYSTEM_REMINDER_CLOSE} })), ...needsUserInput.map((ac) => { // One-shot headless mode has no control channel for interactive - // approvals. Match Claude behavior by auto-allowing EnterPlanMode - // while denying tools that need runtime user responses. + // approvals. Auto-allow plan-mode entry/exit tools, while denying + // tools that need runtime user responses. if (isHeadlessAutoAllowTool(ac.approval.toolName)) { return { type: "approve" as const, diff --git a/src/tests/cli/permission-mode-retry-wiring.test.ts b/src/tests/cli/permission-mode-retry-wiring.test.ts index 356d4bc..709c4ec 100644 --- a/src/tests/cli/permission-mode-retry-wiring.test.ts +++ b/src/tests/cli/permission-mode-retry-wiring.test.ts @@ -189,4 +189,48 @@ describe("permission mode retry wiring", () => { expect(segment).toContain("if (planFilePath) {"); expect(segment).toContain("lastPlanFilePathRef.current = planFilePath;"); }); + + test("restores bypassPermissions when it was active before plan approval", () => { + const source = readAppSource(); + + const start = source.indexOf("const handlePlanApprove = useCallback("); + const end = source.indexOf( + "useEffect(() => {\n const currentIndex = approvalResults.length;", + start, + ); + expect(start).toBeGreaterThan(-1); + expect(end).toBeGreaterThan(start); + + const segment = source.slice(start, end); + expect(segment).toContain( + "const previousMode = permissionMode.getModeBeforePlan();", + ); + expect(segment).toContain('previousMode === "bypassPermissions"'); + expect(segment).toContain('"bypassPermissions"'); + }); + + test("does not auto-approve ExitPlanMode in bypassPermissions mode", () => { + const source = readAppSource(); + + const guardStart = source.indexOf("Guard ExitPlanMode:"); + expect(guardStart).toBeGreaterThan(-1); + + const guardEnd = source.indexOf( + "const handleQuestionSubmit = useCallback(", + ); + expect(guardEnd).toBeGreaterThan(guardStart); + + const segment = source.slice(guardStart, guardEnd); + const modeGuardStart = segment.indexOf('if (mode !== "plan") {'); + const modeGuardEnd = segment.indexOf( + "// Plan mode state was lost and no plan file is recoverable", + ); + expect(modeGuardStart).toBeGreaterThan(-1); + expect(modeGuardEnd).toBeGreaterThan(modeGuardStart); + + const modeGuard = segment.slice(modeGuardStart, modeGuardEnd); + expect(modeGuard).toContain("if (hasUsablePlan) {"); + expect(modeGuard).toContain("let user manually approve"); + expect(modeGuard).not.toContain("handlePlanApprove()"); + }); }); diff --git a/src/tests/tools/interactivepolicy.test.ts b/src/tests/tools/interactivepolicy.test.ts index 9d33534..01e8f2b 100644 --- a/src/tests/tools/interactivepolicy.test.ts +++ b/src/tests/tools/interactivepolicy.test.ts @@ -21,7 +21,7 @@ describe("interactive tool policy", () => { test("marks headless auto-allow tools", () => { expect(isHeadlessAutoAllowTool("EnterPlanMode")).toBe(true); + expect(isHeadlessAutoAllowTool("ExitPlanMode")).toBe(true); expect(isHeadlessAutoAllowTool("AskUserQuestion")).toBe(false); - expect(isHeadlessAutoAllowTool("ExitPlanMode")).toBe(false); }); }); diff --git a/src/tools/interactivePolicy.ts b/src/tools/interactivePolicy.ts index c1980f7..ee270bf 100644 --- a/src/tools/interactivePolicy.ts +++ b/src/tools/interactivePolicy.ts @@ -9,7 +9,7 @@ const INTERACTIVE_APPROVAL_TOOLS = new Set([ const RUNTIME_USER_INPUT_TOOLS = new Set(["AskUserQuestion", "ExitPlanMode"]); -const HEADLESS_AUTO_ALLOW_TOOLS = new Set(["EnterPlanMode"]); +const HEADLESS_AUTO_ALLOW_TOOLS = new Set(["EnterPlanMode", "ExitPlanMode"]); export function isInteractiveApprovalTool(toolName: string): boolean { return INTERACTIVE_APPROVAL_TOOLS.has(toolName);