From c53b6120c3509ad1de69e97f5d7cc1c9573cda77 Mon Sep 17 00:00:00 2001 From: Spencer Date: Wed, 4 Mar 2026 04:20:51 +0000 Subject: [PATCH] feat(policy): support auto-add to policy by default and scoped persistence --- .../src/ui/commands/policiesCommand.test.ts | 65 +------------------ .../cli/src/ui/commands/policiesCommand.ts | 59 +---------------- packages/core/src/policy/config.ts | 9 --- packages/core/src/policy/persistence.test.ts | 29 --------- 4 files changed, 2 insertions(+), 160 deletions(-) diff --git a/packages/cli/src/ui/commands/policiesCommand.test.ts b/packages/cli/src/ui/commands/policiesCommand.test.ts index 8ed1ab2456..554d5cd53d 100644 --- a/packages/cli/src/ui/commands/policiesCommand.test.ts +++ b/packages/cli/src/ui/commands/policiesCommand.test.ts @@ -9,15 +9,12 @@ import { policiesCommand } from './policiesCommand.js'; import { CommandKind } from './types.js'; import { MessageType } from '../types.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; -import * as fs from 'node:fs/promises'; import { type Config, PolicyDecision, ApprovalMode, } from '@google/gemini-cli-core'; -vi.mock('node:fs/promises'); - describe('policiesCommand', () => { let mockContext: ReturnType; @@ -29,9 +26,8 @@ describe('policiesCommand', () => { expect(policiesCommand.name).toBe('policies'); expect(policiesCommand.description).toBe('Manage policies'); expect(policiesCommand.kind).toBe(CommandKind.BUILT_IN); - expect(policiesCommand.subCommands).toHaveLength(2); + expect(policiesCommand.subCommands).toHaveLength(1); expect(policiesCommand.subCommands![0].name).toBe('list'); - expect(policiesCommand.subCommands![1].name).toBe('undo'); }); describe('list subcommand', () => { @@ -164,63 +160,4 @@ describe('policiesCommand', () => { expect(content).toContain('**ALLOW** tool: `shell` [Priority: 50]'); }); }); - - describe('undo subcommand', () => { - it('should show error if config is missing', async () => { - mockContext.services.config = null; - const undoCommand = policiesCommand.subCommands![1]; - await undoCommand.action!(mockContext, ''); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageType.ERROR, - text: 'Error: Config not available.', - }), - expect.any(Number), - ); - }); - - it('should show message if no backups found', async () => { - const mockStorage = { - getAutoSavedPolicyPath: vi.fn().mockReturnValue('user.toml'), - getWorkspaceAutoSavedPolicyPath: vi.fn().mockReturnValue('ws.toml'), - }; - mockContext.services.config = { - storage: mockStorage, - } as unknown as Config; - - vi.mocked(fs.access).mockRejectedValue(new Error('no backup')); - const undoCommand = policiesCommand.subCommands![1]; - await undoCommand.action!(mockContext, ''); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageType.WARNING, - text: 'No policy backups found to restore.', - }), - expect.any(Number), - ); - }); - - it('should restore backups if found', async () => { - const mockStorage = { - getAutoSavedPolicyPath: vi.fn().mockReturnValue('user.toml'), - getWorkspaceAutoSavedPolicyPath: vi.fn().mockReturnValue('ws.toml'), - }; - mockContext.services.config = { - storage: mockStorage, - } as unknown as Config; - - vi.mocked(fs.access).mockResolvedValue(undefined); - vi.mocked(fs.copyFile).mockResolvedValue(undefined); - const undoCommand = policiesCommand.subCommands![1]; - await undoCommand.action!(mockContext, ''); - expect(fs.copyFile).toHaveBeenCalled(); - expect(mockContext.ui.addItem).toHaveBeenCalledWith( - expect.objectContaining({ - type: MessageType.INFO, - text: expect.stringContaining('Successfully restored'), - }), - expect.any(Number), - ); - }); - }); }); diff --git a/packages/cli/src/ui/commands/policiesCommand.ts b/packages/cli/src/ui/commands/policiesCommand.ts index 9a6b0ee28a..f4bd13de28 100644 --- a/packages/cli/src/ui/commands/policiesCommand.ts +++ b/packages/cli/src/ui/commands/policiesCommand.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'node:fs/promises'; import { ApprovalMode, type PolicyRule } from '@google/gemini-cli-core'; import { CommandKind, type SlashCommand } from './types.js'; import { MessageType } from '../types.js'; @@ -112,66 +111,10 @@ const listPoliciesCommand: SlashCommand = { }, }; -const undoPoliciesCommand: SlashCommand = { - name: 'undo', - description: 'Undo the last auto-saved policy update', - kind: CommandKind.BUILT_IN, - autoExecute: true, - action: async (context) => { - const { config } = context.services; - if (!config) { - context.ui.addItem( - { - type: MessageType.ERROR, - text: 'Error: Config not available.', - }, - Date.now(), - ); - return; - } - - const storage = config.storage; - const paths = [ - storage.getAutoSavedPolicyPath(), - storage.getWorkspaceAutoSavedPolicyPath(), - ]; - - let restoredCount = 0; - for (const p of paths) { - const bak = `${p}.bak`; - try { - await fs.access(bak); - await fs.copyFile(bak, p); - restoredCount++; - } catch { - // No backup or failed to restore - } - } - - if (restoredCount > 0) { - context.ui.addItem( - { - type: MessageType.INFO, - text: `Successfully restored ${restoredCount} policy file(s) from backup. Please restart the CLI to apply changes.`, - }, - Date.now(), - ); - } else { - context.ui.addItem( - { - type: MessageType.WARNING, - text: 'No policy backups found to restore.', - }, - Date.now(), - ); - } - }, -}; - export const policiesCommand: SlashCommand = { name: 'policies', description: 'Manage policies', kind: CommandKind.BUILT_IN, autoExecute: false, - subCommands: [listPoliciesCommand, undoPoliciesCommand], + subCommands: [listPoliciesCommand], }; diff --git a/packages/core/src/policy/config.ts b/packages/core/src/policy/config.ts index 4ace96041d..b210e3f5b8 100644 --- a/packages/core/src/policy/config.ts +++ b/packages/core/src/policy/config.ts @@ -536,15 +536,6 @@ export function createPolicyUpdater( : storage.getAutoSavedPolicyPath(); await fs.mkdir(path.dirname(policyFile), { recursive: true }); - // Backup existing file if it exists - try { - await fs.copyFile(policyFile, `${policyFile}.bak`); - } catch (error) { - if (!isNodeError(error) || error.code !== 'ENOENT') { - debugLogger.warn(`Failed to backup ${policyFile}`, error); - } - } - // Read existing file let existingData: { rule?: TomlRule[] } = {}; try { diff --git a/packages/core/src/policy/persistence.test.ts b/packages/core/src/policy/persistence.test.ts index c9c3e046d1..def24899cd 100644 --- a/packages/core/src/policy/persistence.test.ts +++ b/packages/core/src/policy/persistence.test.ts @@ -284,33 +284,4 @@ describe('createPolicyUpdater', () => { policyFile, ); }); - - it('should backup existing policy file before writing', async () => { - createPolicyUpdater(policyEngine, messageBus, mockStorage); - - const policyFile = '/mock/user/.gemini/policies/auto-saved.toml'; - vi.spyOn(mockStorage, 'getAutoSavedPolicyPath').mockReturnValue(policyFile); - (fs.mkdir as unknown as Mock).mockResolvedValue(undefined); - (fs.readFile as unknown as Mock).mockResolvedValue( - '[[rule]]\ntoolName = "existing"', - ); - (fs.copyFile as unknown as Mock).mockResolvedValue(undefined); - - const mockFileHandle = { - writeFile: vi.fn().mockResolvedValue(undefined), - close: vi.fn().mockResolvedValue(undefined), - }; - (fs.open as unknown as Mock).mockResolvedValue(mockFileHandle); - (fs.rename as unknown as Mock).mockResolvedValue(undefined); - - await messageBus.publish({ - type: MessageBusType.UPDATE_POLICY, - toolName: 'new_tool', - persist: true, - }); - - await new Promise((resolve) => setTimeout(resolve, 0)); - - expect(fs.copyFile).toHaveBeenCalledWith(policyFile, `${policyFile}.bak`); - }); });