Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('ToolConfirmationMessage', () => {
vi.mocked(useToolActions).mockReturnValue({
confirm: mockConfirm,
cancel: vi.fn(),
isDiffingEnabled: false,
});

const mockConfig = {
Expand Down Expand Up @@ -274,4 +275,92 @@ describe('ToolConfirmationMessage', () => {
expect(lastFrame()).toContain('Allow for all future sessions');
});
});

describe('Modify with external editor option', () => {
const editConfirmationDetails: ToolCallConfirmationDetails = {
type: 'edit',
title: 'Confirm Edit',
fileName: 'test.txt',
filePath: '/test.txt',
fileDiff: '...diff...',
originalContent: 'a',
newContent: 'b',
onConfirm: vi.fn(),
};

it('should show "Modify with external editor" when NOT in IDE mode', () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => false,
} as unknown as Config;

vi.mocked(useToolActions).mockReturnValue({
confirm: vi.fn(),
cancel: vi.fn(),
isDiffingEnabled: false,
});

const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={editConfirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);

expect(lastFrame()).toContain('Modify with external editor');
});

it('should show "Modify with external editor" when in IDE mode but diffing is NOT enabled', () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => true,
} as unknown as Config;

vi.mocked(useToolActions).mockReturnValue({
confirm: vi.fn(),
cancel: vi.fn(),
isDiffingEnabled: false,
});

const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={editConfirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);

expect(lastFrame()).toContain('Modify with external editor');
});

it('should NOT show "Modify with external editor" when in IDE mode AND diffing is enabled', () => {
const mockConfig = {
isTrustedFolder: () => true,
getIdeMode: () => true,
} as unknown as Config;

vi.mocked(useToolActions).mockReturnValue({
confirm: vi.fn(),
cancel: vi.fn(),
isDiffingEnabled: true,
});

const { lastFrame } = renderWithProviders(
<ToolConfirmationMessage
callId="test-call-id"
confirmationDetails={editConfirmationDetails}
config={mockConfig}
availableTerminalHeight={30}
terminalWidth={80}
/>,
);

expect(lastFrame()).not.toContain('Modify with external editor');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const ToolConfirmationMessage: React.FC<
availableTerminalHeight,
terminalWidth,
}) => {
const { confirm } = useToolActions();
const { confirm, isDiffingEnabled } = useToolActions();

const settings = useSettings();
const allowPermanentApproval =
Expand Down Expand Up @@ -111,9 +111,9 @@ export const ToolConfirmationMessage: React.FC<
});
}
}
// We hide "Modify with external editor" if IDE mode is active, assuming
// the IDE provides a better interface (diff view) for this.
if (!config.getIdeMode()) {
// We hide "Modify with external editor" if IDE mode is active AND
// the IDE is actually capable of showing a diff (connected).
if (!config.getIdeMode() || !isDiffingEnabled) {
options.push({
label: 'Modify with external editor',
value: ToolConfirmationOutcome.ModifyWithEditor,
Expand Down Expand Up @@ -210,7 +210,13 @@ export const ToolConfirmationMessage: React.FC<
});
}
return options;
}, [confirmationDetails, isTrustedFolder, allowPermanentApproval, config]);
}, [
confirmationDetails,
isTrustedFolder,
allowPermanentApproval,
config,
isDiffingEnabled,
]);

const availableBodyContentHeight = useCallback(() => {
if (availableTerminalHeight === undefined) {
Expand Down
40 changes: 40 additions & 0 deletions packages/cli/src/ui/contexts/ToolActionsContext.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,44 @@ describe('ToolActionsContext', () => {
throw new Error('Expected onConfirm to be present');
}
});

it('updates isDiffingEnabled when IdeClient status changes', async () => {
let statusListener: () => void = () => {};
const mockIdeClient = {
isDiffingEnabled: vi.fn().mockReturnValue(false),
addStatusChangeListener: vi.fn().mockImplementation((listener) => {
statusListener = listener;
}),
removeStatusChangeListener: vi.fn(),
} as unknown as IdeClient;

vi.mocked(IdeClient.getInstance).mockResolvedValue(mockIdeClient);
vi.mocked(mockConfig.getIdeMode).mockReturnValue(true);

const { result } = renderHook(() => useToolActions(), { wrapper });

// Wait for initialization
await act(async () => {
await vi.waitFor(() => expect(IdeClient.getInstance).toHaveBeenCalled());
await new Promise((resolve) => setTimeout(resolve, 0));
});

expect(result.current.isDiffingEnabled).toBe(false);

// Simulate connection change
vi.mocked(mockIdeClient.isDiffingEnabled).mockReturnValue(true);
await act(async () => {
statusListener();
});

expect(result.current.isDiffingEnabled).toBe(true);

// Simulate disconnection
vi.mocked(mockIdeClient.isDiffingEnabled).mockReturnValue(false);
await act(async () => {
statusListener();
});

expect(result.current.isDiffingEnabled).toBe(false);
});
});
27 changes: 22 additions & 5 deletions packages/cli/src/ui/contexts/ToolActionsContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ interface ToolActionsContextValue {
payload?: ToolConfirmationPayload,
) => Promise<void>;
cancel: (callId: string) => Promise<void>;
isDiffingEnabled: boolean;
}

const ToolActionsContext = createContext<ToolActionsContextValue | null>(null);
Expand All @@ -55,12 +56,28 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (

// Hoist IdeClient logic here to keep UI pure
const [ideClient, setIdeClient] = useState<IdeClient | null>(null);
const [isDiffingEnabled, setIsDiffingEnabled] = useState(false);

useEffect(() => {
let isMounted = true;
if (config.getIdeMode()) {
IdeClient.getInstance()
.then((client) => {
if (isMounted) setIdeClient(client);
if (!isMounted) return;
setIdeClient(client);
setIsDiffingEnabled(client.isDiffingEnabled());

const handleStatusChange = () => {
if (isMounted) {
setIsDiffingEnabled(client.isDiffingEnabled());
}
};

client.addStatusChangeListener(handleStatusChange);
// Return a cleanup function for the listener
return () => {
client.removeStatusChangeListener(handleStatusChange);
};
})
.catch((error) => {
debugLogger.error('Failed to get IdeClient instance:', error);
Expand Down Expand Up @@ -88,12 +105,12 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (
// 1. Handle Side Effects (IDE Diff)
if (
details?.type === 'edit' &&
ideClient?.isDiffingEnabled() &&
isDiffingEnabled &&
'filePath' in details // Check for safety
) {
const cliOutcome =
outcome === ToolConfirmationOutcome.Cancel ? 'rejected' : 'accepted';
await ideClient.resolveDiffFromCli(details.filePath, cliOutcome);
await ideClient?.resolveDiffFromCli(details.filePath, cliOutcome);
}

// 2. Dispatch
Expand Down Expand Up @@ -125,7 +142,7 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (

debugLogger.warn(`ToolActions: No confirmation mechanism for ${callId}`);
},
[config, ideClient, toolCalls],
[config, ideClient, toolCalls, isDiffingEnabled],
);

const cancel = useCallback(
Expand All @@ -136,7 +153,7 @@ export const ToolActionsProvider: React.FC<ToolActionsProviderProps> = (
);

return (
<ToolActionsContext.Provider value={{ confirm, cancel }}>
<ToolActionsContext.Provider value={{ confirm, cancel, isDiffingEnabled }}>
{children}
</ToolActionsContext.Provider>
);
Expand Down
Loading