From b903b00e9d60cd75604dc32f8b951bc5462c1c41 Mon Sep 17 00:00:00 2001 From: Spencer Date: Fri, 20 Mar 2026 07:15:23 +0000 Subject: [PATCH] refactor: simplify command allowlist parsing and parameterize tests - Replace non-null assertion with nullish coalescing in `commandAllowlist.ts`. - Parameterize identical test cases in `ToolConfirmationMessage.test.tsx`. --- .../messages/ToolConfirmationMessage.test.tsx | 188 ++++++++---------- packages/core/src/utils/commandAllowlist.ts | 2 +- 2 files changed, 85 insertions(+), 105 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index e2463e9c7c..c41388c4e9 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -22,6 +22,7 @@ import { initializeShellParsers, } from '@google/gemini-cli-core'; import { renderWithProviders } from '../../../test-utils/render.js'; +import '../../../test-utils/customMatchers.js'; import { createMockSettings } from '../../../test-utils/settings.js'; import { useToolActions } from '../../contexts/ToolActionsContext.js'; import { act } from 'react'; @@ -54,122 +55,101 @@ describe('ToolConfirmationMessage', () => { getApprovalMode: () => 'default', } as unknown as Config; - it('shows permanent approval option for safe commands', async () => { - const execSafe: SerializableConfirmationDetails = { - type: 'exec', - title: 'Confirm Execution', + it.each([ + { + description: 'safe commands', command: 'ls -la', rootCommand: 'ls', - rootCommands: ['ls'], - }; + approvalMode: 'default', + }, + { + description: 'edit commands in AUTO_EDIT mode', + command: 'mkdir test', + rootCommand: 'mkdir', + approvalMode: 'autoEdit', + }, + ])( + 'shows permanent approval option for $description', + async ({ command, rootCommand, approvalMode }) => { + const mockConfigDynamic = { + ...mockConfigWithPermanent, + getApprovalMode: () => approvalMode, + } as unknown as Config; - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( - , - { settings: mockSettingsWithPermanent }, - ); + const exec: SerializableConfirmationDetails = { + type: 'exec', + title: 'Confirm Execution', + command, + rootCommand, + rootCommands: [rootCommand], + }; - await waitUntilReady(); - const output = lastFrame(); - expect(output).toContain('Allow this command for all future sessions'); - unmount(); - }); + const { lastFrame, waitUntilReady, unmount } = + await renderWithProviders( + , + { settings: mockSettingsWithPermanent }, + ); - it('hides permanent approval option for unsafe commands', async () => { - const execUnsafe: SerializableConfirmationDetails = { - type: 'exec', - title: 'Confirm Execution', + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + unmount(); + }, + ); + + it.each([ + { + description: 'unsafe commands', command: 'rm -rf /', rootCommand: 'rm', - rootCommands: ['rm'], - }; - - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( - , - { settings: mockSettingsWithPermanent }, - ); - - await waitUntilReady(); - const output = lastFrame(); - expect(output).not.toContain( - 'Allow this command for all future sessions', - ); - unmount(); - }); - - it('shows permanent approval option for edit commands in AUTO_EDIT mode', async () => { - const mockConfigAutoEdit = { - ...mockConfigWithPermanent, - getApprovalMode: () => 'autoEdit', - } as unknown as Config; - - const execEdit: SerializableConfirmationDetails = { - type: 'exec', - title: 'Confirm Execution', + approvalMode: 'default', + }, + { + description: 'edit commands in DEFAULT mode', command: 'mkdir test', rootCommand: 'mkdir', - rootCommands: ['mkdir'], - }; + approvalMode: 'default', + }, + ])( + 'hides permanent approval option for $description', + async ({ command, rootCommand, approvalMode }) => { + const mockConfigDynamic = { + ...mockConfigWithPermanent, + getApprovalMode: () => approvalMode, + } as unknown as Config; - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( - , - { settings: mockSettingsWithPermanent }, - ); + const exec: SerializableConfirmationDetails = { + type: 'exec', + title: 'Confirm Execution', + command, + rootCommand, + rootCommands: [rootCommand], + }; - await waitUntilReady(); - const output = lastFrame(); - expect(output).toContain('Allow this command for all future sessions'); - unmount(); - }); + const { lastFrame, waitUntilReady, unmount } = + await renderWithProviders( + , + { settings: mockSettingsWithPermanent }, + ); - it('hides permanent approval option for edit commands in DEFAULT mode', async () => { - const execEdit: SerializableConfirmationDetails = { - type: 'exec', - title: 'Confirm Execution', - command: 'mkdir test', - rootCommand: 'mkdir', - rootCommands: ['mkdir'], - }; - - const { lastFrame, waitUntilReady, unmount } = renderWithProviders( - , - { settings: mockSettingsWithPermanent }, - ); - - await waitUntilReady(); - const output = lastFrame(); - expect(output).not.toContain( - 'Allow this command for all future sessions', - ); - unmount(); - }); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + unmount(); + }, + ); }); const mockConfirm = vi.fn(); diff --git a/packages/core/src/utils/commandAllowlist.ts b/packages/core/src/utils/commandAllowlist.ts index cea7ac771f..48eb39dc8e 100644 --- a/packages/core/src/utils/commandAllowlist.ts +++ b/packages/core/src/utils/commandAllowlist.ts @@ -77,7 +77,7 @@ export function canShowAutoApproveCheckbox( // EVERY root must be on an allowlist return roots.every((root) => { // Strip path prefixes (e.g., /usr/bin/ls → ls) - const base = root.includes('/') ? root.split('/').pop()! : root; + const base = root.split('/').pop() ?? root; if (safeCommandAllowlist.has(base)) return true; if (isAutoEdit && editCommandAllowlist.has(base)) return true;