Skip to content

fix(cli): restore YOLO mode after exiting Plan mode-new #19625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Manik-Khajuria-5 wants to merge 1 commit into from
Closed

fix(cli): restore YOLO mode after exiting Plan mode-new #19625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 105 additions & 16 deletions packages/cli/src/ui/hooks/useApprovalModeIndicator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ describe('useApprovalModeIndicator', () => {
}),
);
expect(result.current).toBe(ApprovalMode.AUTO_EDIT);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(1);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(2);
});

it('should initialize with ApprovalMode.DEFAULT if config.getApprovalMode returns ApprovalMode.DEFAULT', () => {
Expand All @@ -157,7 +157,7 @@ describe('useApprovalModeIndicator', () => {
}),
);
expect(result.current).toBe(ApprovalMode.DEFAULT);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(1);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(2);
});

it('should initialize with ApprovalMode.YOLO if config.getApprovalMode returns ApprovalMode.YOLO', () => {
Expand All @@ -169,7 +169,8 @@ describe('useApprovalModeIndicator', () => {
}),
);
expect(result.current).toBe(ApprovalMode.YOLO);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(1);
// Hook calls getApprovalMode twice on mount: (1) for useState initial value, (2) in effect that captures "mode at CLI open" for Plan exit restore
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(2);
});

it('should cycle the indicator and update config when Shift+Tab or Ctrl+Y is pressed', () => {
Expand Down Expand Up @@ -202,19 +203,19 @@ describe('useApprovalModeIndicator', () => {
);
expect(result.current).toBe(ApprovalMode.YOLO);

// Shift+Tab cycles back to AUTO_EDIT (from YOLO)
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.YOLO);
act(() => {
capturedUseKeypressHandler({
name: 'tab',
shift: true,
} as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.AUTO_EDIT,
ApprovalMode.DEFAULT,
);
expect(result.current).toBe(ApprovalMode.AUTO_EDIT);
expect(result.current).toBe(ApprovalMode.DEFAULT);

// Ctrl+Y toggles YOLO
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT);
act(() => {
capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key);
});
Expand All @@ -223,17 +224,17 @@ describe('useApprovalModeIndicator', () => {
);
expect(result.current).toBe(ApprovalMode.YOLO);

// Shift+Tab from YOLO jumps to AUTO_EDIT
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.YOLO);
act(() => {
capturedUseKeypressHandler({
name: 'tab',
shift: true,
} as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.AUTO_EDIT,
ApprovalMode.DEFAULT,
);
expect(result.current).toBe(ApprovalMode.AUTO_EDIT);
expect(result.current).toBe(ApprovalMode.DEFAULT);
});

it('should not toggle if only one key or other keys combinations are pressed', () => {
Expand Down Expand Up @@ -316,7 +317,7 @@ describe('useApprovalModeIndicator', () => {
addItem: vi.fn(),
});
expect(result.current).toBe(ApprovalMode.AUTO_EDIT);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(3);
expect(mockConfigInstance.getApprovalMode).toHaveBeenCalledTimes(4);
});

describe('in untrusted folders', () => {
Expand Down Expand Up @@ -679,7 +680,7 @@ describe('useApprovalModeIndicator', () => {
capturedUseKeypressHandler({ name: 'y', ctrl: true } as Key);
});

// Switch to AUTO_EDIT
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.YOLO);
act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
Expand All @@ -691,12 +692,12 @@ describe('useApprovalModeIndicator', () => {
);
expect(mockOnApprovalModeChange).toHaveBeenNthCalledWith(
2,
ApprovalMode.AUTO_EDIT,
ApprovalMode.DEFAULT,
);
});

it('should cycle to PLAN when allowPlanMode is true', () => {
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.AUTO_EDIT);
it('should cycle to PLAN when allowPlanMode is true (default sequence: DEFAULT -> PLAN)', () => {
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT);

renderHook(() =>
useApprovalModeIndicator({
Expand All @@ -706,7 +707,6 @@ describe('useApprovalModeIndicator', () => {
}),
);

// AUTO_EDIT -> PLAN
act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
Expand Down Expand Up @@ -734,4 +734,93 @@ describe('useApprovalModeIndicator', () => {
ApprovalMode.DEFAULT,
);
});

it('should restore YOLO when exiting Plan via Shift+Tab after YOLO -> DEFAULT -> AUTO_EDIT -> PLAN (allowPlanMode true)', () => {
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.YOLO);

const { result } = renderHook(() =>
useApprovalModeIndicator({
config: mockConfigInstance as unknown as ActualConfigType,
addItem: vi.fn(),
allowPlanMode: true,
}),
);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.DEFAULT,
);
expect(result.current).toBe(ApprovalMode.DEFAULT);

mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.AUTO_EDIT,
);
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.AUTO_EDIT);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.PLAN,
);
expect(result.current).toBe(ApprovalMode.PLAN);

mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.PLAN);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.YOLO,
);
expect(result.current).toBe(ApprovalMode.YOLO);
});

it('should complete default sequence: DEFAULT then PLAN then AUTO_EDIT then DEFAULT when allowPlanMode is true', () => {
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.DEFAULT);

const { result } = renderHook(() =>
useApprovalModeIndicator({
config: mockConfigInstance as unknown as ActualConfigType,
addItem: vi.fn(),
allowPlanMode: true,
}),
);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenNthCalledWith(
1,
ApprovalMode.PLAN,
);
expect(result.current).toBe(ApprovalMode.PLAN);
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.PLAN);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenNthCalledWith(
2,
ApprovalMode.AUTO_EDIT,
);
expect(result.current).toBe(ApprovalMode.AUTO_EDIT);
mockConfigInstance.getApprovalMode.mockReturnValue(ApprovalMode.AUTO_EDIT);

act(() => {
capturedUseKeypressHandler({ name: 'tab', shift: true } as Key);
});
expect(mockConfigInstance.setApprovalMode).toHaveBeenNthCalledWith(
3,
ApprovalMode.DEFAULT,
);
expect(result.current).toBe(ApprovalMode.DEFAULT);
});
});
35 changes: 25 additions & 10 deletions packages/cli/src/ui/hooks/useApprovalModeIndicator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { useState, useEffect } from 'react';
import { useState, useEffect, useRef } from 'react';
import {
ApprovalMode,
type Config,
Expand Down Expand Up @@ -32,11 +32,20 @@ export function useApprovalModeIndicator({
}: UseApprovalModeIndicatorArgs): ApprovalMode {
const currentConfigValue = config.getApprovalMode();
const [showApprovalMode, setApprovalMode] = useState(currentConfigValue);
const initialModeRef = useRef<ApprovalMode | null>(null);

useEffect(() => {
setApprovalMode(currentConfigValue);
}, [currentConfigValue]);

useEffect(() => {
if (initialModeRef.current === null) {
const mode = config.getApprovalMode();
initialModeRef.current =
mode === ApprovalMode.PLAN ? ApprovalMode.DEFAULT : mode;
}
}, [config]);

useKeypress(
(key) => {
let nextApprovalMode: ApprovalMode | undefined;
Expand Down Expand Up @@ -72,20 +81,29 @@ export function useApprovalModeIndicator({
: ApprovalMode.YOLO;
} else if (keyMatchers[Command.CYCLE_APPROVAL_MODE](key)) {
const currentMode = config.getApprovalMode();
const initial = initialModeRef.current ?? ApprovalMode.DEFAULT;
const fromYolo = initial === ApprovalMode.YOLO;

switch (currentMode) {
case ApprovalMode.DEFAULT:
nextApprovalMode = ApprovalMode.AUTO_EDIT;
nextApprovalMode =
allowPlanMode && !fromYolo
? ApprovalMode.PLAN
: ApprovalMode.AUTO_EDIT;
break;
case ApprovalMode.AUTO_EDIT:
nextApprovalMode = allowPlanMode
? ApprovalMode.PLAN
: ApprovalMode.DEFAULT;
nextApprovalMode =
fromYolo && allowPlanMode
? ApprovalMode.PLAN
: ApprovalMode.DEFAULT;
break;
case ApprovalMode.PLAN:
nextApprovalMode = ApprovalMode.DEFAULT;
nextApprovalMode = fromYolo
? ApprovalMode.YOLO
: ApprovalMode.AUTO_EDIT;
break;
case ApprovalMode.YOLO:
nextApprovalMode = ApprovalMode.AUTO_EDIT;
nextApprovalMode = ApprovalMode.DEFAULT;
break;
default:
}
Expand All @@ -94,10 +112,7 @@ export function useApprovalModeIndicator({
if (nextApprovalMode) {
try {
config.setApprovalMode(nextApprovalMode);
// Update local state immediately for responsiveness
setApprovalMode(nextApprovalMode);

// Notify the central handler about the approval mode change
onApprovalModeChange?.(nextApprovalMode);
} catch (e) {
if (addItem) {
Expand Down