fix(plan-mode): require manual ExitPlanMode approval in yolo (#1440)
This commit is contained in:
@@ -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];
|
||||
|
||||
@@ -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 (
|
||||
<StaticPlanApproval
|
||||
onApprove={() => onPlanApprove(false)}
|
||||
onApproveAndAcceptEdits={() => onPlanApprove(true)}
|
||||
onKeepPlanning={onPlanKeepPlanning}
|
||||
onCancel={onCancel ?? (() => {})}
|
||||
showAcceptEditsOption={showAcceptEditsOption}
|
||||
isFocused={isFocused}
|
||||
planContent={planContent}
|
||||
planFilePath={planFilePath}
|
||||
|
||||
@@ -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 */}
|
||||
<Box marginTop={1} flexDirection="column">
|
||||
{/* Option 1: Yes, and auto-accept edits */}
|
||||
{/* Option 1 */}
|
||||
<Box flexDirection="row">
|
||||
<Box width={5} flexShrink={0}>
|
||||
<Text
|
||||
color={
|
||||
selectedOption === 0 ? colors.approval.header : undefined
|
||||
effectiveSelectedOption === 0
|
||||
? colors.approval.header
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
{selectedOption === 0 ? "❯" : " "} 1.
|
||||
{effectiveSelectedOption === 0 ? "❯" : " "} 1.
|
||||
</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - 5)}>
|
||||
<Text
|
||||
wrap="wrap"
|
||||
color={
|
||||
selectedOption === 0 ? colors.approval.header : undefined
|
||||
effectiveSelectedOption === 0
|
||||
? colors.approval.header
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
Yes, and auto-accept edits
|
||||
{showAcceptEditsOption
|
||||
? "Yes, and auto-accept edits"
|
||||
: "Yes, proceed (bypassPermissions / yolo mode)"}
|
||||
</Text>
|
||||
</Box>
|
||||
</Box>
|
||||
|
||||
{/* Option 2: Yes, and manually approve edits */}
|
||||
<Box flexDirection="row">
|
||||
<Box width={5} flexShrink={0}>
|
||||
<Text
|
||||
color={
|
||||
selectedOption === 1 ? colors.approval.header : undefined
|
||||
}
|
||||
>
|
||||
{selectedOption === 1 ? "❯" : " "} 2.
|
||||
</Text>
|
||||
{showAcceptEditsOption && (
|
||||
<Box flexDirection="row">
|
||||
<Box width={5} flexShrink={0}>
|
||||
<Text
|
||||
color={
|
||||
effectiveSelectedOption === 1
|
||||
? colors.approval.header
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
{effectiveSelectedOption === 1 ? "❯" : " "} 2.
|
||||
</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - 5)}>
|
||||
<Text
|
||||
wrap="wrap"
|
||||
color={
|
||||
effectiveSelectedOption === 1
|
||||
? colors.approval.header
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
Yes, and manually approve edits
|
||||
</Text>
|
||||
</Box>
|
||||
</Box>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - 5)}>
|
||||
<Text
|
||||
wrap="wrap"
|
||||
color={
|
||||
selectedOption === 1 ? colors.approval.header : undefined
|
||||
}
|
||||
>
|
||||
Yes, and manually approve edits
|
||||
</Text>
|
||||
</Box>
|
||||
</Box>
|
||||
)}
|
||||
|
||||
{/* Option 3: Custom input */}
|
||||
<Box flexDirection="row">
|
||||
@@ -221,7 +240,7 @@ export const StaticPlanApproval = memo(
|
||||
<Text
|
||||
color={isOnCustomOption ? colors.approval.header : undefined}
|
||||
>
|
||||
{isOnCustomOption ? "❯" : " "} 3.
|
||||
{isOnCustomOption ? "❯" : " "} {customOptionIndex + 1}.
|
||||
</Text>
|
||||
</Box>
|
||||
<Box flexGrow={1} width={Math.max(0, columns - 5)}>
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user