From 1279984ef264ec2aeda474781cda001a22e52453 Mon Sep 17 00:00:00 2001 From: mkorwel Date: Wed, 18 Feb 2026 07:59:44 -0600 Subject: [PATCH 1/5] feat(plan): enforce read-only constraints and designated plan storage in Plan Mode - Update Plan Mode policy to explicitly deny write_file and replace operations on source code. - Restrict ToolRegistry to only allow read-only tools and plan-related write operations when in Plan Mode. - Enhance tool schemas in Plan Mode with descriptions explicitly limiting write_file and replace to the designated plans directory. - Update Plan Mode system prompts to clarify behavioral rules and the requirement for approved plans before source code modifications. - Standardize formatting in strict-development-rules.md. --- .gemini/commands/strict-development-rules.md | 154 ++++++++++++++----- packages/core/src/policy/policies/plan.toml | 8 + packages/core/src/prompts/snippets.ts | 2 +- packages/core/src/tools/tool-registry.ts | 44 +++++- 4 files changed, 168 insertions(+), 40 deletions(-) diff --git a/.gemini/commands/strict-development-rules.md b/.gemini/commands/strict-development-rules.md index 54c8ff80af4..9c01860091f 100644 --- a/.gemini/commands/strict-development-rules.md +++ b/.gemini/commands/strict-development-rules.md @@ -1,64 +1,142 @@ # Gemini CLI Strict Development Rules -These rules apply strictly to all code modifications and additions within the Gemini CLI project. +These rules apply strictly to all code modifications and additions within the +Gemini CLI project. ## Testing Guidelines -* **Async/Await**: Always use `waitFor` from `packages/cli/src/test-utils/async.ts` instead of `vi.waitFor` for all `waitFor` calls within `packages/cli`. NEVER use fixed waits (e.g., `await delay(100)`). Always use `waitFor` with a predicate to ensure tests are stable and fast. Using the wrong `waitFor` can result in flaky tests and `act` warnings. -* **React Testing**: Use `act` to wrap all blocks in tests that change component state. Use `render` or `renderWithProviders` from `packages/cli/src/test-utils/render.tsx` instead of `render` from `ink-testing-library` directly. This prevents spurious `act` warnings. If test cases specify providers directly, consider whether the existing `renderWithProviders` should be modified. -* **Snapshots**: Use `toMatchSnapshot` to verify that rendering works as expected rather than matching against the raw content of the output. When modifying snapshots, verify the changes are intentional and do not hide underlying bugs. -* **Parameterized Tests**: Use parameterized tests where it reduces duplicated lines. Give the parameters explicit types to ensure the tests are type-safe. -* **Mocks Management**: - * Mock critical dependencies (`fs`, `os`, `child_process`) ONLY at the top of the file. Ideally, avoid mocking these dependencies altogether. - * Reuse existing mocks and fakes rather than creating new ones. - * Avoid mocking the file system whenever possible. If using the real file system is too difficult, consider writing an integration test instead. - * Always call `vi.restoreAllMocks()` in `afterEach` to prevent test pollution. - * Use `vi.useFakeTimers()` for tests involving time-based logic to avoid flakiness. -* **Typing in Tests**: Avoid using `any` in tests; prefer proper types or `unknown` with narrowing. +- **Async/Await**: Always use `waitFor` from + `packages/cli/src/test-utils/async.ts` instead of `vi.waitFor` for all + `waitFor` calls within `packages/cli`. NEVER use fixed waits (e.g., + `await delay(100)`). Always use `waitFor` with a predicate to ensure tests are + stable and fast. Using the wrong `waitFor` can result in flaky tests and `act` + warnings. +- **React Testing**: Use `act` to wrap all blocks in tests that change component + state. Use `render` or `renderWithProviders` from + `packages/cli/src/test-utils/render.tsx` instead of `render` from + `ink-testing-library` directly. This prevents spurious `act` warnings. If test + cases specify providers directly, consider whether the existing + `renderWithProviders` should be modified. +- **Snapshots**: Use `toMatchSnapshot` to verify that rendering works as + expected rather than matching against the raw content of the output. When + modifying snapshots, verify the changes are intentional and do not hide + underlying bugs. +- **Parameterized Tests**: Use parameterized tests where it reduces duplicated + lines. Give the parameters explicit types to ensure the tests are type-safe. +- **Mocks Management**: + - Mock critical dependencies (`fs`, `os`, `child_process`) ONLY at the top of + the file. Ideally, avoid mocking these dependencies altogether. + - Reuse existing mocks and fakes rather than creating new ones. + - Avoid mocking the file system whenever possible. If using the real file + system is too difficult, consider writing an integration test instead. + - Always call `vi.restoreAllMocks()` in `afterEach` to prevent test pollution. + - Use `vi.useFakeTimers()` for tests involving time-based logic to avoid + flakiness. +- **Typing in Tests**: Avoid using `any` in tests; prefer proper types or + `unknown` with narrowing. ## React Guidelines (`packages/cli`) -* **`setState` and Side Effects**: NEVER trigger side effects from within the body of a `setState` callback. Use a reducer or `useRef` if necessary. These cases have historically introduced multiple bugs; typically, they should be resolved using a reducer. -* **Rendering**: Do not introduce infinite rendering loops. Avoid synchronous file I/O in React components as it will hang the UI. Do not implement new logic for custom string measurement or string truncation. Use Ink layout instead, leveraging `ResizeObserver` as needed. -* **Keyboard Handling**: Keyboard handling MUST go through `useKeyPress.ts` from the Gemini CLI package rather than the standard ink library. This library supports reporting multiple keyboard events sequentially in the same React frame (critical for slow terminals). Handling this correctly often requires reducers to ensure multiple state updates are handled gracefully without overriding values. Refer to `text-buffer.ts` for a canonical example. -* **Logging**: Do not leave `console.log`, `console.warn`, or `console.error` in the code. -* **State & Effects**: Ensure state initialization is explicit (e.g., use `undefined` rather than `true` as a default if the state is truly unknown). Carefully manage `useEffect` dependencies. Prefer a reducer whenever practical. NEVER disable `react-hooks/exhaustive-deps`; fix the code to correctly declare dependencies instead. -* **Context & Props**: Avoid excessive property drilling. Leverage existing providers, extend them, or propose a new one if necessary. Only use providers for properties that are consistent across the entire application. -* **Code Structure**: Avoid complex `if` statements where `switch` statements could be used. Keep `AppContainer` minimal; refactor complex logic into React hooks. Evaluate whether business logic should be added to `hookSystem.ts` or integrated into `packages/core` rather than `packages/cli`. +- **`setState` and Side Effects**: NEVER trigger side effects from within the + body of a `setState` callback. Use a reducer or `useRef` if necessary. These + cases have historically introduced multiple bugs; typically, they should be + resolved using a reducer. +- **Rendering**: Do not introduce infinite rendering loops. Avoid synchronous + file I/O in React components as it will hang the UI. Do not implement new + logic for custom string measurement or string truncation. Use Ink layout + instead, leveraging `ResizeObserver` as needed. +- **Keyboard Handling**: Keyboard handling MUST go through `useKeyPress.ts` from + the Gemini CLI package rather than the standard ink library. This library + supports reporting multiple keyboard events sequentially in the same React + frame (critical for slow terminals). Handling this correctly often requires + reducers to ensure multiple state updates are handled gracefully without + overriding values. Refer to `text-buffer.ts` for a canonical example. +- **Logging**: Do not leave `console.log`, `console.warn`, or `console.error` in + the code. +- **State & Effects**: Ensure state initialization is explicit (e.g., use + `undefined` rather than `true` as a default if the state is truly unknown). + Carefully manage `useEffect` dependencies. Prefer a reducer whenever + practical. NEVER disable `react-hooks/exhaustive-deps`; fix the code to + correctly declare dependencies instead. +- **Context & Props**: Avoid excessive property drilling. Leverage existing + providers, extend them, or propose a new one if necessary. Only use providers + for properties that are consistent across the entire application. +- **Code Structure**: Avoid complex `if` statements where `switch` statements + could be used. Keep `AppContainer` minimal; refactor complex logic into React + hooks. Evaluate whether business logic should be added to `hookSystem.ts` or + integrated into `packages/core` rather than `packages/cli`. ## Core Guidelines (`packages/core`) -* **Services**: Implement services as classes with clear lifecycle management (e.g., `initialize()` methods). Services should be stateless where possible, or use the centralized `Storage` service for persistence. -* **Cross-Service Communication**: Prefer using the `coreEvents` bus (from `packages/core/src/utils/events.ts`) for asynchronous communication between services or to notify the UI of state changes. Avoid tight coupling between services. -* **Utilities**: Use `debugLogger` from `packages/core/src/utils/debugLogger.ts` for internal logging instead of `console`. Ensure all shell operations use `spawnAsync` from `packages/core/src/utils/shell-utils.ts` for consistent error handling and promise management. Handle filesystem errors gracefully using `isNodeError` from `packages/core/src/utils/errors.ts`. -* **Exports & Tooling**: Add new tools to `packages/core/src/tools/` and register them in `packages/core/src/tools/tool-registry.ts`. Export all new public services, utilities, and types from `packages/core/src/index.ts`. +- **Services**: Implement services as classes with clear lifecycle management + (e.g., `initialize()` methods). Services should be stateless where possible, + or use the centralized `Storage` service for persistence. +- **Cross-Service Communication**: Prefer using the `coreEvents` bus (from + `packages/core/src/utils/events.ts`) for asynchronous communication between + services or to notify the UI of state changes. Avoid tight coupling between + services. +- **Utilities**: Use `debugLogger` from `packages/core/src/utils/debugLogger.ts` + for internal logging instead of `console`. Ensure all shell operations use + `spawnAsync` from `packages/core/src/utils/shell-utils.ts` for consistent + error handling and promise management. Handle filesystem errors gracefully + using `isNodeError` from `packages/core/src/utils/errors.ts`. +- **Exports & Tooling**: Add new tools to `packages/core/src/tools/` and + register them in `packages/core/src/tools/tool-registry.ts`. Export all new + public services, utilities, and types from `packages/core/src/index.ts`. ## Architectural Audit (Package Boundaries) -* **Logic Placement**: Non-UI logic (e.g., model orchestration, tool implementation, git/filesystem operations) MUST reside in `packages/core`. `packages/cli` should ONLY contain UI/Ink components, command-line argument parsing, and user interaction logic. -* **Environment Isolation**: Core logic must not assume a TUI environment. Use the `ConfirmationBus` or `Output` abstractions for communicating with the user from Core. -* **Decoupling**: Actively look for opportunities to decouple services using `coreEvents`. If a service imports another just to notify it of a change, use an event instead. +- **Logic Placement**: Non-UI logic (e.g., model orchestration, tool + implementation, git/filesystem operations) MUST reside in `packages/core`. + `packages/cli` should ONLY contain UI/Ink components, command-line argument + parsing, and user interaction logic. +- **Environment Isolation**: Core logic must not assume a TUI environment. Use + the `ConfirmationBus` or `Output` abstractions for communicating with the user + from Core. +- **Decoupling**: Actively look for opportunities to decouple services using + `coreEvents`. If a service imports another just to notify it of a change, use + an event instead. ## General Gemini CLI Design Principles -* **Settings**: Use settings for user-configurable options rather than adding new command line arguments. Add new settings to `packages/cli/src/config/settingsSchema.ts`. If a setting has `showInDialog: true`, it MUST be documented in `docs/get-started/configuration.md`. Ensure `requiresRestart` is correctly set. -* **Logging**: Use `debugLogger` for rethrown errors to avoid duplicate logging. -* **Keyboard Shortcuts**: Define all new keyboard shortcuts in `packages/cli/src/config/keyBindings.ts` and document them in `docs/cli/keyboard-shortcuts.md`. Be careful of keybindings that require the `Meta` key, as only certain meta key shortcuts are supported on Mac. Avoid function keys and shortcuts commonly bound in VSCode. +- **Settings**: Use settings for user-configurable options rather than adding + new command line arguments. Add new settings to + `packages/cli/src/config/settingsSchema.ts`. If a setting has + `showInDialog: true`, it MUST be documented in + `docs/get-started/configuration.md`. Ensure `requiresRestart` is correctly + set. +- **Logging**: Use `debugLogger` for rethrown errors to avoid duplicate logging. +- **Keyboard Shortcuts**: Define all new keyboard shortcuts in + `packages/cli/src/config/keyBindings.ts` and document them in + `docs/cli/keyboard-shortcuts.md`. Be careful of keybindings that require the + `Meta` key, as only certain meta key shortcuts are supported on Mac. Avoid + function keys and shortcuts commonly bound in VSCode. ## TypeScript Best Practices -* Use `checkExhaustive` in the `default` clause of `switch` statements to ensure all cases are handled. -* Avoid using the non-null assertion operator (`!`) unless absolutely necessary. -* **STRICT TYPING**: Strictly forbid `any` and `unknown` in both CLI and Core packages. `unknown` is only allowed if it is immediately narrowed using type guards or Zod validation. -* NEVER disable `@typescript-eslint/no-floating-promises`. -* Avoid making types nullable unless strictly necessary, as it hurts readability. +- Use `checkExhaustive` in the `default` clause of `switch` statements to ensure + all cases are handled. +- Avoid using the non-null assertion operator (`!`) unless absolutely necessary. +- **STRICT TYPING**: Strictly forbid `any` and `unknown` in both CLI and Core + packages. `unknown` is only allowed if it is immediately narrowed using type + guards or Zod validation. +- NEVER disable `@typescript-eslint/no-floating-promises`. +- Avoid making types nullable unless strictly necessary, as it hurts + readability. ## TUI Best Practices -* **Terminal Compatibility**: Consider how changes might behave differently across terminals (e.g., VSCode terminal, SSH, Kitty, default Mac terminal, iTerm2, Windows terminal). If modifying keyboard handling, integrate deeply with existing files like `KeypressContext.tsx` and `terminalCapabilityManager.ts`. -* **iTerm**: Be aware that `ITERM_SESSION_ID` may be present when users run VSCode from within iTerm, even if the terminal is not iTerm. +- **Terminal Compatibility**: Consider how changes might behave differently + across terminals (e.g., VSCode terminal, SSH, Kitty, default Mac terminal, + iTerm2, Windows terminal). If modifying keyboard handling, integrate deeply + with existing files like `KeypressContext.tsx` and + `terminalCapabilityManager.ts`. +- **iTerm**: Be aware that `ITERM_SESSION_ID` may be present when users run + VSCode from within iTerm, even if the terminal is not iTerm. ## Code Cleanup -* **Refactoring**: Actively clean up code duplication, technical debt, and boilerplate ("AI Slop") when working in the codebase. -* **Prompts**: Be aware that changes can impact the prompts sent to Gemini CLI and affect overall quality. +- **Refactoring**: Actively clean up code duplication, technical debt, and + boilerplate ("AI Slop") when working in the codebase. +- **Prompts**: Be aware that changes can impact the prompts sent to Gemini CLI + and affect overall quality. diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 12648fec5f6..ec666e66a09 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -54,3 +54,11 @@ decision = "allow" priority = 70 modes = ["plan"] argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+/plans/[a-zA-Z0-9_-]+\\.md\"" + +# Explicitly Deny other write operations in Plan mode with a clear message. +[[rule]] +toolName = ["write_file", "replace"] +decision = "deny" +priority = 65 +modes = ["plan"] +deny_message = "You are in Plan Mode and cannot modify source code. You may ONLY use write_file or replace to save plans to the designated plans directory as .md files." diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index 54e64a82667..4ddeee40dfd 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -429,7 +429,7 @@ ${options.planModeToolsList} ## Rules -1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`${options.plansDir}/\`. +1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`${options.plansDir}/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a detailed plan in the plans directory and get approval before any source code changes can be made. 2. **Efficiency:** Autonomously combine discovery and drafting phases to minimize conversational turns. If the request is ambiguous, use ${formatToolName(ASK_USER_TOOL_NAME)} to clarify. Otherwise, explore the codebase and write the draft in one fluid motion. 3. **Inquiries and Directives:** Distinguish between Inquiries and Directives to minimize unnecessary planning. - **Inquiries:** If the request is an **Inquiry** (e.g., "How does X work?"), use read-only tools to explore and answer directly in your chat response. DO NOT create a plan or call ${formatToolName( diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 60b1451838a..60f62b44cd3 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -12,6 +12,7 @@ import type { } from './tools.js'; import { Kind, BaseDeclarativeTool, BaseToolInvocation } from './tools.js'; import type { Config } from '../config/config.js'; +import { ApprovalMode } from '../policy/types.js'; import { spawn } from 'node:child_process'; import { StringDecoder } from 'node:string_decoder'; import { DiscoveredMCPTool } from './mcp-tool.js'; @@ -25,6 +26,9 @@ import { DISCOVERED_TOOL_PREFIX, TOOL_LEGACY_ALIASES, getToolAliases, + PLAN_MODE_TOOLS, + WRITE_FILE_TOOL_NAME, + EDIT_TOOL_NAME, } from './tool-names.js'; type ToolParams = Record; @@ -484,6 +488,31 @@ export class ToolRegistry { excludeTools ??= this.expandExcludeToolsWithAliases(this.config.getExcludeTools()) ?? new Set([]); + + // Filter tools in Plan Mode to only allow approved read-only tools. + const isPlanMode = + typeof this.config.getApprovalMode === 'function' && + this.config.getApprovalMode() === ApprovalMode.PLAN; + if (isPlanMode) { + const allowedToolNames = new Set(PLAN_MODE_TOOLS); + // We allow write_file and replace for writing plans specifically. + allowedToolNames.add(WRITE_FILE_TOOL_NAME); + allowedToolNames.add(EDIT_TOOL_NAME); + + // Discovered MCP tools are allowed if they are read-only. + if ( + tool instanceof DiscoveredMCPTool && + tool.isReadOnly && + !allowedToolNames.has(tool.name) + ) { + allowedToolNames.add(tool.name); + } + + if (!allowedToolNames.has(tool.name)) { + return false; + } + } + const normalizedClassName = tool.constructor.name.replace(/^_+/, ''); const possibleNames = [tool.name, normalizedClassName]; if (tool instanceof DiscoveredMCPTool) { @@ -507,9 +536,22 @@ export class ToolRegistry { * @returns An array of FunctionDeclarations. */ getFunctionDeclarations(modelId?: string): FunctionDeclaration[] { + const isPlanMode = this.config.getApprovalMode() === ApprovalMode.PLAN; + const plansDir = this.config.storage.getProjectTempPlansDir(); + const declarations: FunctionDeclaration[] = []; this.getActiveTools().forEach((tool) => { - declarations.push(tool.getSchema(modelId)); + let schema = tool.getSchema(modelId); + if ( + isPlanMode && + (tool.name === WRITE_FILE_TOOL_NAME || tool.name === EDIT_TOOL_NAME) + ) { + schema = { + ...schema, + description: `ONLY FOR PLANS: ${schema.description}. You are currently in Plan Mode and may ONLY use this tool to write or update plans (.md files) in the plans directory: ${plansDir}/. You cannot use this tool to modify source code directly.`, + }; + } + declarations.push(schema); }); return declarations; } From ad35c74bfa15568ceafcb0010998c8ec9a6c212a Mon Sep 17 00:00:00 2001 From: mkorwel Date: Thu, 19 Feb 2026 13:13:22 -0600 Subject: [PATCH 2/5] fix(plan): refine read-only constraints and address review feedback --- .gemini/commands/strict-development-rules.md | 154 +++++-------------- packages/core/src/policy/policies/plan.toml | 8 - packages/core/src/prompts/snippets.ts | 2 +- packages/core/src/tools/tool-registry.ts | 44 +----- 4 files changed, 40 insertions(+), 168 deletions(-) diff --git a/.gemini/commands/strict-development-rules.md b/.gemini/commands/strict-development-rules.md index 9c01860091f..54c8ff80af4 100644 --- a/.gemini/commands/strict-development-rules.md +++ b/.gemini/commands/strict-development-rules.md @@ -1,142 +1,64 @@ # Gemini CLI Strict Development Rules -These rules apply strictly to all code modifications and additions within the -Gemini CLI project. +These rules apply strictly to all code modifications and additions within the Gemini CLI project. ## Testing Guidelines -- **Async/Await**: Always use `waitFor` from - `packages/cli/src/test-utils/async.ts` instead of `vi.waitFor` for all - `waitFor` calls within `packages/cli`. NEVER use fixed waits (e.g., - `await delay(100)`). Always use `waitFor` with a predicate to ensure tests are - stable and fast. Using the wrong `waitFor` can result in flaky tests and `act` - warnings. -- **React Testing**: Use `act` to wrap all blocks in tests that change component - state. Use `render` or `renderWithProviders` from - `packages/cli/src/test-utils/render.tsx` instead of `render` from - `ink-testing-library` directly. This prevents spurious `act` warnings. If test - cases specify providers directly, consider whether the existing - `renderWithProviders` should be modified. -- **Snapshots**: Use `toMatchSnapshot` to verify that rendering works as - expected rather than matching against the raw content of the output. When - modifying snapshots, verify the changes are intentional and do not hide - underlying bugs. -- **Parameterized Tests**: Use parameterized tests where it reduces duplicated - lines. Give the parameters explicit types to ensure the tests are type-safe. -- **Mocks Management**: - - Mock critical dependencies (`fs`, `os`, `child_process`) ONLY at the top of - the file. Ideally, avoid mocking these dependencies altogether. - - Reuse existing mocks and fakes rather than creating new ones. - - Avoid mocking the file system whenever possible. If using the real file - system is too difficult, consider writing an integration test instead. - - Always call `vi.restoreAllMocks()` in `afterEach` to prevent test pollution. - - Use `vi.useFakeTimers()` for tests involving time-based logic to avoid - flakiness. -- **Typing in Tests**: Avoid using `any` in tests; prefer proper types or - `unknown` with narrowing. +* **Async/Await**: Always use `waitFor` from `packages/cli/src/test-utils/async.ts` instead of `vi.waitFor` for all `waitFor` calls within `packages/cli`. NEVER use fixed waits (e.g., `await delay(100)`). Always use `waitFor` with a predicate to ensure tests are stable and fast. Using the wrong `waitFor` can result in flaky tests and `act` warnings. +* **React Testing**: Use `act` to wrap all blocks in tests that change component state. Use `render` or `renderWithProviders` from `packages/cli/src/test-utils/render.tsx` instead of `render` from `ink-testing-library` directly. This prevents spurious `act` warnings. If test cases specify providers directly, consider whether the existing `renderWithProviders` should be modified. +* **Snapshots**: Use `toMatchSnapshot` to verify that rendering works as expected rather than matching against the raw content of the output. When modifying snapshots, verify the changes are intentional and do not hide underlying bugs. +* **Parameterized Tests**: Use parameterized tests where it reduces duplicated lines. Give the parameters explicit types to ensure the tests are type-safe. +* **Mocks Management**: + * Mock critical dependencies (`fs`, `os`, `child_process`) ONLY at the top of the file. Ideally, avoid mocking these dependencies altogether. + * Reuse existing mocks and fakes rather than creating new ones. + * Avoid mocking the file system whenever possible. If using the real file system is too difficult, consider writing an integration test instead. + * Always call `vi.restoreAllMocks()` in `afterEach` to prevent test pollution. + * Use `vi.useFakeTimers()` for tests involving time-based logic to avoid flakiness. +* **Typing in Tests**: Avoid using `any` in tests; prefer proper types or `unknown` with narrowing. ## React Guidelines (`packages/cli`) -- **`setState` and Side Effects**: NEVER trigger side effects from within the - body of a `setState` callback. Use a reducer or `useRef` if necessary. These - cases have historically introduced multiple bugs; typically, they should be - resolved using a reducer. -- **Rendering**: Do not introduce infinite rendering loops. Avoid synchronous - file I/O in React components as it will hang the UI. Do not implement new - logic for custom string measurement or string truncation. Use Ink layout - instead, leveraging `ResizeObserver` as needed. -- **Keyboard Handling**: Keyboard handling MUST go through `useKeyPress.ts` from - the Gemini CLI package rather than the standard ink library. This library - supports reporting multiple keyboard events sequentially in the same React - frame (critical for slow terminals). Handling this correctly often requires - reducers to ensure multiple state updates are handled gracefully without - overriding values. Refer to `text-buffer.ts` for a canonical example. -- **Logging**: Do not leave `console.log`, `console.warn`, or `console.error` in - the code. -- **State & Effects**: Ensure state initialization is explicit (e.g., use - `undefined` rather than `true` as a default if the state is truly unknown). - Carefully manage `useEffect` dependencies. Prefer a reducer whenever - practical. NEVER disable `react-hooks/exhaustive-deps`; fix the code to - correctly declare dependencies instead. -- **Context & Props**: Avoid excessive property drilling. Leverage existing - providers, extend them, or propose a new one if necessary. Only use providers - for properties that are consistent across the entire application. -- **Code Structure**: Avoid complex `if` statements where `switch` statements - could be used. Keep `AppContainer` minimal; refactor complex logic into React - hooks. Evaluate whether business logic should be added to `hookSystem.ts` or - integrated into `packages/core` rather than `packages/cli`. +* **`setState` and Side Effects**: NEVER trigger side effects from within the body of a `setState` callback. Use a reducer or `useRef` if necessary. These cases have historically introduced multiple bugs; typically, they should be resolved using a reducer. +* **Rendering**: Do not introduce infinite rendering loops. Avoid synchronous file I/O in React components as it will hang the UI. Do not implement new logic for custom string measurement or string truncation. Use Ink layout instead, leveraging `ResizeObserver` as needed. +* **Keyboard Handling**: Keyboard handling MUST go through `useKeyPress.ts` from the Gemini CLI package rather than the standard ink library. This library supports reporting multiple keyboard events sequentially in the same React frame (critical for slow terminals). Handling this correctly often requires reducers to ensure multiple state updates are handled gracefully without overriding values. Refer to `text-buffer.ts` for a canonical example. +* **Logging**: Do not leave `console.log`, `console.warn`, or `console.error` in the code. +* **State & Effects**: Ensure state initialization is explicit (e.g., use `undefined` rather than `true` as a default if the state is truly unknown). Carefully manage `useEffect` dependencies. Prefer a reducer whenever practical. NEVER disable `react-hooks/exhaustive-deps`; fix the code to correctly declare dependencies instead. +* **Context & Props**: Avoid excessive property drilling. Leverage existing providers, extend them, or propose a new one if necessary. Only use providers for properties that are consistent across the entire application. +* **Code Structure**: Avoid complex `if` statements where `switch` statements could be used. Keep `AppContainer` minimal; refactor complex logic into React hooks. Evaluate whether business logic should be added to `hookSystem.ts` or integrated into `packages/core` rather than `packages/cli`. ## Core Guidelines (`packages/core`) -- **Services**: Implement services as classes with clear lifecycle management - (e.g., `initialize()` methods). Services should be stateless where possible, - or use the centralized `Storage` service for persistence. -- **Cross-Service Communication**: Prefer using the `coreEvents` bus (from - `packages/core/src/utils/events.ts`) for asynchronous communication between - services or to notify the UI of state changes. Avoid tight coupling between - services. -- **Utilities**: Use `debugLogger` from `packages/core/src/utils/debugLogger.ts` - for internal logging instead of `console`. Ensure all shell operations use - `spawnAsync` from `packages/core/src/utils/shell-utils.ts` for consistent - error handling and promise management. Handle filesystem errors gracefully - using `isNodeError` from `packages/core/src/utils/errors.ts`. -- **Exports & Tooling**: Add new tools to `packages/core/src/tools/` and - register them in `packages/core/src/tools/tool-registry.ts`. Export all new - public services, utilities, and types from `packages/core/src/index.ts`. +* **Services**: Implement services as classes with clear lifecycle management (e.g., `initialize()` methods). Services should be stateless where possible, or use the centralized `Storage` service for persistence. +* **Cross-Service Communication**: Prefer using the `coreEvents` bus (from `packages/core/src/utils/events.ts`) for asynchronous communication between services or to notify the UI of state changes. Avoid tight coupling between services. +* **Utilities**: Use `debugLogger` from `packages/core/src/utils/debugLogger.ts` for internal logging instead of `console`. Ensure all shell operations use `spawnAsync` from `packages/core/src/utils/shell-utils.ts` for consistent error handling and promise management. Handle filesystem errors gracefully using `isNodeError` from `packages/core/src/utils/errors.ts`. +* **Exports & Tooling**: Add new tools to `packages/core/src/tools/` and register them in `packages/core/src/tools/tool-registry.ts`. Export all new public services, utilities, and types from `packages/core/src/index.ts`. ## Architectural Audit (Package Boundaries) -- **Logic Placement**: Non-UI logic (e.g., model orchestration, tool - implementation, git/filesystem operations) MUST reside in `packages/core`. - `packages/cli` should ONLY contain UI/Ink components, command-line argument - parsing, and user interaction logic. -- **Environment Isolation**: Core logic must not assume a TUI environment. Use - the `ConfirmationBus` or `Output` abstractions for communicating with the user - from Core. -- **Decoupling**: Actively look for opportunities to decouple services using - `coreEvents`. If a service imports another just to notify it of a change, use - an event instead. +* **Logic Placement**: Non-UI logic (e.g., model orchestration, tool implementation, git/filesystem operations) MUST reside in `packages/core`. `packages/cli` should ONLY contain UI/Ink components, command-line argument parsing, and user interaction logic. +* **Environment Isolation**: Core logic must not assume a TUI environment. Use the `ConfirmationBus` or `Output` abstractions for communicating with the user from Core. +* **Decoupling**: Actively look for opportunities to decouple services using `coreEvents`. If a service imports another just to notify it of a change, use an event instead. ## General Gemini CLI Design Principles -- **Settings**: Use settings for user-configurable options rather than adding - new command line arguments. Add new settings to - `packages/cli/src/config/settingsSchema.ts`. If a setting has - `showInDialog: true`, it MUST be documented in - `docs/get-started/configuration.md`. Ensure `requiresRestart` is correctly - set. -- **Logging**: Use `debugLogger` for rethrown errors to avoid duplicate logging. -- **Keyboard Shortcuts**: Define all new keyboard shortcuts in - `packages/cli/src/config/keyBindings.ts` and document them in - `docs/cli/keyboard-shortcuts.md`. Be careful of keybindings that require the - `Meta` key, as only certain meta key shortcuts are supported on Mac. Avoid - function keys and shortcuts commonly bound in VSCode. +* **Settings**: Use settings for user-configurable options rather than adding new command line arguments. Add new settings to `packages/cli/src/config/settingsSchema.ts`. If a setting has `showInDialog: true`, it MUST be documented in `docs/get-started/configuration.md`. Ensure `requiresRestart` is correctly set. +* **Logging**: Use `debugLogger` for rethrown errors to avoid duplicate logging. +* **Keyboard Shortcuts**: Define all new keyboard shortcuts in `packages/cli/src/config/keyBindings.ts` and document them in `docs/cli/keyboard-shortcuts.md`. Be careful of keybindings that require the `Meta` key, as only certain meta key shortcuts are supported on Mac. Avoid function keys and shortcuts commonly bound in VSCode. ## TypeScript Best Practices -- Use `checkExhaustive` in the `default` clause of `switch` statements to ensure - all cases are handled. -- Avoid using the non-null assertion operator (`!`) unless absolutely necessary. -- **STRICT TYPING**: Strictly forbid `any` and `unknown` in both CLI and Core - packages. `unknown` is only allowed if it is immediately narrowed using type - guards or Zod validation. -- NEVER disable `@typescript-eslint/no-floating-promises`. -- Avoid making types nullable unless strictly necessary, as it hurts - readability. +* Use `checkExhaustive` in the `default` clause of `switch` statements to ensure all cases are handled. +* Avoid using the non-null assertion operator (`!`) unless absolutely necessary. +* **STRICT TYPING**: Strictly forbid `any` and `unknown` in both CLI and Core packages. `unknown` is only allowed if it is immediately narrowed using type guards or Zod validation. +* NEVER disable `@typescript-eslint/no-floating-promises`. +* Avoid making types nullable unless strictly necessary, as it hurts readability. ## TUI Best Practices -- **Terminal Compatibility**: Consider how changes might behave differently - across terminals (e.g., VSCode terminal, SSH, Kitty, default Mac terminal, - iTerm2, Windows terminal). If modifying keyboard handling, integrate deeply - with existing files like `KeypressContext.tsx` and - `terminalCapabilityManager.ts`. -- **iTerm**: Be aware that `ITERM_SESSION_ID` may be present when users run - VSCode from within iTerm, even if the terminal is not iTerm. +* **Terminal Compatibility**: Consider how changes might behave differently across terminals (e.g., VSCode terminal, SSH, Kitty, default Mac terminal, iTerm2, Windows terminal). If modifying keyboard handling, integrate deeply with existing files like `KeypressContext.tsx` and `terminalCapabilityManager.ts`. +* **iTerm**: Be aware that `ITERM_SESSION_ID` may be present when users run VSCode from within iTerm, even if the terminal is not iTerm. ## Code Cleanup -- **Refactoring**: Actively clean up code duplication, technical debt, and - boilerplate ("AI Slop") when working in the codebase. -- **Prompts**: Be aware that changes can impact the prompts sent to Gemini CLI - and affect overall quality. +* **Refactoring**: Actively clean up code duplication, technical debt, and boilerplate ("AI Slop") when working in the codebase. +* **Prompts**: Be aware that changes can impact the prompts sent to Gemini CLI and affect overall quality. diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index ec666e66a09..12648fec5f6 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -54,11 +54,3 @@ decision = "allow" priority = 70 modes = ["plan"] argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+/plans/[a-zA-Z0-9_-]+\\.md\"" - -# Explicitly Deny other write operations in Plan mode with a clear message. -[[rule]] -toolName = ["write_file", "replace"] -decision = "deny" -priority = 65 -modes = ["plan"] -deny_message = "You are in Plan Mode and cannot modify source code. You may ONLY use write_file or replace to save plans to the designated plans directory as .md files." diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index 4ddeee40dfd..54e64a82667 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -429,7 +429,7 @@ ${options.planModeToolsList} ## Rules -1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`${options.plansDir}/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a detailed plan in the plans directory and get approval before any source code changes can be made. +1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`${options.plansDir}/\`. 2. **Efficiency:** Autonomously combine discovery and drafting phases to minimize conversational turns. If the request is ambiguous, use ${formatToolName(ASK_USER_TOOL_NAME)} to clarify. Otherwise, explore the codebase and write the draft in one fluid motion. 3. **Inquiries and Directives:** Distinguish between Inquiries and Directives to minimize unnecessary planning. - **Inquiries:** If the request is an **Inquiry** (e.g., "How does X work?"), use read-only tools to explore and answer directly in your chat response. DO NOT create a plan or call ${formatToolName( diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 60f62b44cd3..60b1451838a 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -12,7 +12,6 @@ import type { } from './tools.js'; import { Kind, BaseDeclarativeTool, BaseToolInvocation } from './tools.js'; import type { Config } from '../config/config.js'; -import { ApprovalMode } from '../policy/types.js'; import { spawn } from 'node:child_process'; import { StringDecoder } from 'node:string_decoder'; import { DiscoveredMCPTool } from './mcp-tool.js'; @@ -26,9 +25,6 @@ import { DISCOVERED_TOOL_PREFIX, TOOL_LEGACY_ALIASES, getToolAliases, - PLAN_MODE_TOOLS, - WRITE_FILE_TOOL_NAME, - EDIT_TOOL_NAME, } from './tool-names.js'; type ToolParams = Record; @@ -488,31 +484,6 @@ export class ToolRegistry { excludeTools ??= this.expandExcludeToolsWithAliases(this.config.getExcludeTools()) ?? new Set([]); - - // Filter tools in Plan Mode to only allow approved read-only tools. - const isPlanMode = - typeof this.config.getApprovalMode === 'function' && - this.config.getApprovalMode() === ApprovalMode.PLAN; - if (isPlanMode) { - const allowedToolNames = new Set(PLAN_MODE_TOOLS); - // We allow write_file and replace for writing plans specifically. - allowedToolNames.add(WRITE_FILE_TOOL_NAME); - allowedToolNames.add(EDIT_TOOL_NAME); - - // Discovered MCP tools are allowed if they are read-only. - if ( - tool instanceof DiscoveredMCPTool && - tool.isReadOnly && - !allowedToolNames.has(tool.name) - ) { - allowedToolNames.add(tool.name); - } - - if (!allowedToolNames.has(tool.name)) { - return false; - } - } - const normalizedClassName = tool.constructor.name.replace(/^_+/, ''); const possibleNames = [tool.name, normalizedClassName]; if (tool instanceof DiscoveredMCPTool) { @@ -536,22 +507,9 @@ export class ToolRegistry { * @returns An array of FunctionDeclarations. */ getFunctionDeclarations(modelId?: string): FunctionDeclaration[] { - const isPlanMode = this.config.getApprovalMode() === ApprovalMode.PLAN; - const plansDir = this.config.storage.getProjectTempPlansDir(); - const declarations: FunctionDeclaration[] = []; this.getActiveTools().forEach((tool) => { - let schema = tool.getSchema(modelId); - if ( - isPlanMode && - (tool.name === WRITE_FILE_TOOL_NAME || tool.name === EDIT_TOOL_NAME) - ) { - schema = { - ...schema, - description: `ONLY FOR PLANS: ${schema.description}. You are currently in Plan Mode and may ONLY use this tool to write or update plans (.md files) in the plans directory: ${plansDir}/. You cannot use this tool to modify source code directly.`, - }; - } - declarations.push(schema); + declarations.push(tool.getSchema(modelId)); }); return declarations; } From 65d51183efd33aba15982732b180a6d41bab20d6 Mon Sep 17 00:00:00 2001 From: mkorwel Date: Thu, 19 Feb 2026 13:18:20 -0600 Subject: [PATCH 3/5] Revert "fix(plan): refine read-only constraints and address review feedback" This reverts commit ad35c74bfa15568ceafcb0010998c8ec9a6c212a. --- .gemini/commands/strict-development-rules.md | 154 ++++++++++++++----- packages/core/src/policy/policies/plan.toml | 8 + packages/core/src/prompts/snippets.ts | 2 +- packages/core/src/tools/tool-registry.ts | 44 +++++- 4 files changed, 168 insertions(+), 40 deletions(-) diff --git a/.gemini/commands/strict-development-rules.md b/.gemini/commands/strict-development-rules.md index 54c8ff80af4..9c01860091f 100644 --- a/.gemini/commands/strict-development-rules.md +++ b/.gemini/commands/strict-development-rules.md @@ -1,64 +1,142 @@ # Gemini CLI Strict Development Rules -These rules apply strictly to all code modifications and additions within the Gemini CLI project. +These rules apply strictly to all code modifications and additions within the +Gemini CLI project. ## Testing Guidelines -* **Async/Await**: Always use `waitFor` from `packages/cli/src/test-utils/async.ts` instead of `vi.waitFor` for all `waitFor` calls within `packages/cli`. NEVER use fixed waits (e.g., `await delay(100)`). Always use `waitFor` with a predicate to ensure tests are stable and fast. Using the wrong `waitFor` can result in flaky tests and `act` warnings. -* **React Testing**: Use `act` to wrap all blocks in tests that change component state. Use `render` or `renderWithProviders` from `packages/cli/src/test-utils/render.tsx` instead of `render` from `ink-testing-library` directly. This prevents spurious `act` warnings. If test cases specify providers directly, consider whether the existing `renderWithProviders` should be modified. -* **Snapshots**: Use `toMatchSnapshot` to verify that rendering works as expected rather than matching against the raw content of the output. When modifying snapshots, verify the changes are intentional and do not hide underlying bugs. -* **Parameterized Tests**: Use parameterized tests where it reduces duplicated lines. Give the parameters explicit types to ensure the tests are type-safe. -* **Mocks Management**: - * Mock critical dependencies (`fs`, `os`, `child_process`) ONLY at the top of the file. Ideally, avoid mocking these dependencies altogether. - * Reuse existing mocks and fakes rather than creating new ones. - * Avoid mocking the file system whenever possible. If using the real file system is too difficult, consider writing an integration test instead. - * Always call `vi.restoreAllMocks()` in `afterEach` to prevent test pollution. - * Use `vi.useFakeTimers()` for tests involving time-based logic to avoid flakiness. -* **Typing in Tests**: Avoid using `any` in tests; prefer proper types or `unknown` with narrowing. +- **Async/Await**: Always use `waitFor` from + `packages/cli/src/test-utils/async.ts` instead of `vi.waitFor` for all + `waitFor` calls within `packages/cli`. NEVER use fixed waits (e.g., + `await delay(100)`). Always use `waitFor` with a predicate to ensure tests are + stable and fast. Using the wrong `waitFor` can result in flaky tests and `act` + warnings. +- **React Testing**: Use `act` to wrap all blocks in tests that change component + state. Use `render` or `renderWithProviders` from + `packages/cli/src/test-utils/render.tsx` instead of `render` from + `ink-testing-library` directly. This prevents spurious `act` warnings. If test + cases specify providers directly, consider whether the existing + `renderWithProviders` should be modified. +- **Snapshots**: Use `toMatchSnapshot` to verify that rendering works as + expected rather than matching against the raw content of the output. When + modifying snapshots, verify the changes are intentional and do not hide + underlying bugs. +- **Parameterized Tests**: Use parameterized tests where it reduces duplicated + lines. Give the parameters explicit types to ensure the tests are type-safe. +- **Mocks Management**: + - Mock critical dependencies (`fs`, `os`, `child_process`) ONLY at the top of + the file. Ideally, avoid mocking these dependencies altogether. + - Reuse existing mocks and fakes rather than creating new ones. + - Avoid mocking the file system whenever possible. If using the real file + system is too difficult, consider writing an integration test instead. + - Always call `vi.restoreAllMocks()` in `afterEach` to prevent test pollution. + - Use `vi.useFakeTimers()` for tests involving time-based logic to avoid + flakiness. +- **Typing in Tests**: Avoid using `any` in tests; prefer proper types or + `unknown` with narrowing. ## React Guidelines (`packages/cli`) -* **`setState` and Side Effects**: NEVER trigger side effects from within the body of a `setState` callback. Use a reducer or `useRef` if necessary. These cases have historically introduced multiple bugs; typically, they should be resolved using a reducer. -* **Rendering**: Do not introduce infinite rendering loops. Avoid synchronous file I/O in React components as it will hang the UI. Do not implement new logic for custom string measurement or string truncation. Use Ink layout instead, leveraging `ResizeObserver` as needed. -* **Keyboard Handling**: Keyboard handling MUST go through `useKeyPress.ts` from the Gemini CLI package rather than the standard ink library. This library supports reporting multiple keyboard events sequentially in the same React frame (critical for slow terminals). Handling this correctly often requires reducers to ensure multiple state updates are handled gracefully without overriding values. Refer to `text-buffer.ts` for a canonical example. -* **Logging**: Do not leave `console.log`, `console.warn`, or `console.error` in the code. -* **State & Effects**: Ensure state initialization is explicit (e.g., use `undefined` rather than `true` as a default if the state is truly unknown). Carefully manage `useEffect` dependencies. Prefer a reducer whenever practical. NEVER disable `react-hooks/exhaustive-deps`; fix the code to correctly declare dependencies instead. -* **Context & Props**: Avoid excessive property drilling. Leverage existing providers, extend them, or propose a new one if necessary. Only use providers for properties that are consistent across the entire application. -* **Code Structure**: Avoid complex `if` statements where `switch` statements could be used. Keep `AppContainer` minimal; refactor complex logic into React hooks. Evaluate whether business logic should be added to `hookSystem.ts` or integrated into `packages/core` rather than `packages/cli`. +- **`setState` and Side Effects**: NEVER trigger side effects from within the + body of a `setState` callback. Use a reducer or `useRef` if necessary. These + cases have historically introduced multiple bugs; typically, they should be + resolved using a reducer. +- **Rendering**: Do not introduce infinite rendering loops. Avoid synchronous + file I/O in React components as it will hang the UI. Do not implement new + logic for custom string measurement or string truncation. Use Ink layout + instead, leveraging `ResizeObserver` as needed. +- **Keyboard Handling**: Keyboard handling MUST go through `useKeyPress.ts` from + the Gemini CLI package rather than the standard ink library. This library + supports reporting multiple keyboard events sequentially in the same React + frame (critical for slow terminals). Handling this correctly often requires + reducers to ensure multiple state updates are handled gracefully without + overriding values. Refer to `text-buffer.ts` for a canonical example. +- **Logging**: Do not leave `console.log`, `console.warn`, or `console.error` in + the code. +- **State & Effects**: Ensure state initialization is explicit (e.g., use + `undefined` rather than `true` as a default if the state is truly unknown). + Carefully manage `useEffect` dependencies. Prefer a reducer whenever + practical. NEVER disable `react-hooks/exhaustive-deps`; fix the code to + correctly declare dependencies instead. +- **Context & Props**: Avoid excessive property drilling. Leverage existing + providers, extend them, or propose a new one if necessary. Only use providers + for properties that are consistent across the entire application. +- **Code Structure**: Avoid complex `if` statements where `switch` statements + could be used. Keep `AppContainer` minimal; refactor complex logic into React + hooks. Evaluate whether business logic should be added to `hookSystem.ts` or + integrated into `packages/core` rather than `packages/cli`. ## Core Guidelines (`packages/core`) -* **Services**: Implement services as classes with clear lifecycle management (e.g., `initialize()` methods). Services should be stateless where possible, or use the centralized `Storage` service for persistence. -* **Cross-Service Communication**: Prefer using the `coreEvents` bus (from `packages/core/src/utils/events.ts`) for asynchronous communication between services or to notify the UI of state changes. Avoid tight coupling between services. -* **Utilities**: Use `debugLogger` from `packages/core/src/utils/debugLogger.ts` for internal logging instead of `console`. Ensure all shell operations use `spawnAsync` from `packages/core/src/utils/shell-utils.ts` for consistent error handling and promise management. Handle filesystem errors gracefully using `isNodeError` from `packages/core/src/utils/errors.ts`. -* **Exports & Tooling**: Add new tools to `packages/core/src/tools/` and register them in `packages/core/src/tools/tool-registry.ts`. Export all new public services, utilities, and types from `packages/core/src/index.ts`. +- **Services**: Implement services as classes with clear lifecycle management + (e.g., `initialize()` methods). Services should be stateless where possible, + or use the centralized `Storage` service for persistence. +- **Cross-Service Communication**: Prefer using the `coreEvents` bus (from + `packages/core/src/utils/events.ts`) for asynchronous communication between + services or to notify the UI of state changes. Avoid tight coupling between + services. +- **Utilities**: Use `debugLogger` from `packages/core/src/utils/debugLogger.ts` + for internal logging instead of `console`. Ensure all shell operations use + `spawnAsync` from `packages/core/src/utils/shell-utils.ts` for consistent + error handling and promise management. Handle filesystem errors gracefully + using `isNodeError` from `packages/core/src/utils/errors.ts`. +- **Exports & Tooling**: Add new tools to `packages/core/src/tools/` and + register them in `packages/core/src/tools/tool-registry.ts`. Export all new + public services, utilities, and types from `packages/core/src/index.ts`. ## Architectural Audit (Package Boundaries) -* **Logic Placement**: Non-UI logic (e.g., model orchestration, tool implementation, git/filesystem operations) MUST reside in `packages/core`. `packages/cli` should ONLY contain UI/Ink components, command-line argument parsing, and user interaction logic. -* **Environment Isolation**: Core logic must not assume a TUI environment. Use the `ConfirmationBus` or `Output` abstractions for communicating with the user from Core. -* **Decoupling**: Actively look for opportunities to decouple services using `coreEvents`. If a service imports another just to notify it of a change, use an event instead. +- **Logic Placement**: Non-UI logic (e.g., model orchestration, tool + implementation, git/filesystem operations) MUST reside in `packages/core`. + `packages/cli` should ONLY contain UI/Ink components, command-line argument + parsing, and user interaction logic. +- **Environment Isolation**: Core logic must not assume a TUI environment. Use + the `ConfirmationBus` or `Output` abstractions for communicating with the user + from Core. +- **Decoupling**: Actively look for opportunities to decouple services using + `coreEvents`. If a service imports another just to notify it of a change, use + an event instead. ## General Gemini CLI Design Principles -* **Settings**: Use settings for user-configurable options rather than adding new command line arguments. Add new settings to `packages/cli/src/config/settingsSchema.ts`. If a setting has `showInDialog: true`, it MUST be documented in `docs/get-started/configuration.md`. Ensure `requiresRestart` is correctly set. -* **Logging**: Use `debugLogger` for rethrown errors to avoid duplicate logging. -* **Keyboard Shortcuts**: Define all new keyboard shortcuts in `packages/cli/src/config/keyBindings.ts` and document them in `docs/cli/keyboard-shortcuts.md`. Be careful of keybindings that require the `Meta` key, as only certain meta key shortcuts are supported on Mac. Avoid function keys and shortcuts commonly bound in VSCode. +- **Settings**: Use settings for user-configurable options rather than adding + new command line arguments. Add new settings to + `packages/cli/src/config/settingsSchema.ts`. If a setting has + `showInDialog: true`, it MUST be documented in + `docs/get-started/configuration.md`. Ensure `requiresRestart` is correctly + set. +- **Logging**: Use `debugLogger` for rethrown errors to avoid duplicate logging. +- **Keyboard Shortcuts**: Define all new keyboard shortcuts in + `packages/cli/src/config/keyBindings.ts` and document them in + `docs/cli/keyboard-shortcuts.md`. Be careful of keybindings that require the + `Meta` key, as only certain meta key shortcuts are supported on Mac. Avoid + function keys and shortcuts commonly bound in VSCode. ## TypeScript Best Practices -* Use `checkExhaustive` in the `default` clause of `switch` statements to ensure all cases are handled. -* Avoid using the non-null assertion operator (`!`) unless absolutely necessary. -* **STRICT TYPING**: Strictly forbid `any` and `unknown` in both CLI and Core packages. `unknown` is only allowed if it is immediately narrowed using type guards or Zod validation. -* NEVER disable `@typescript-eslint/no-floating-promises`. -* Avoid making types nullable unless strictly necessary, as it hurts readability. +- Use `checkExhaustive` in the `default` clause of `switch` statements to ensure + all cases are handled. +- Avoid using the non-null assertion operator (`!`) unless absolutely necessary. +- **STRICT TYPING**: Strictly forbid `any` and `unknown` in both CLI and Core + packages. `unknown` is only allowed if it is immediately narrowed using type + guards or Zod validation. +- NEVER disable `@typescript-eslint/no-floating-promises`. +- Avoid making types nullable unless strictly necessary, as it hurts + readability. ## TUI Best Practices -* **Terminal Compatibility**: Consider how changes might behave differently across terminals (e.g., VSCode terminal, SSH, Kitty, default Mac terminal, iTerm2, Windows terminal). If modifying keyboard handling, integrate deeply with existing files like `KeypressContext.tsx` and `terminalCapabilityManager.ts`. -* **iTerm**: Be aware that `ITERM_SESSION_ID` may be present when users run VSCode from within iTerm, even if the terminal is not iTerm. +- **Terminal Compatibility**: Consider how changes might behave differently + across terminals (e.g., VSCode terminal, SSH, Kitty, default Mac terminal, + iTerm2, Windows terminal). If modifying keyboard handling, integrate deeply + with existing files like `KeypressContext.tsx` and + `terminalCapabilityManager.ts`. +- **iTerm**: Be aware that `ITERM_SESSION_ID` may be present when users run + VSCode from within iTerm, even if the terminal is not iTerm. ## Code Cleanup -* **Refactoring**: Actively clean up code duplication, technical debt, and boilerplate ("AI Slop") when working in the codebase. -* **Prompts**: Be aware that changes can impact the prompts sent to Gemini CLI and affect overall quality. +- **Refactoring**: Actively clean up code duplication, technical debt, and + boilerplate ("AI Slop") when working in the codebase. +- **Prompts**: Be aware that changes can impact the prompts sent to Gemini CLI + and affect overall quality. diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index 12648fec5f6..ec666e66a09 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -54,3 +54,11 @@ decision = "allow" priority = 70 modes = ["plan"] argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-]+/plans/[a-zA-Z0-9_-]+\\.md\"" + +# Explicitly Deny other write operations in Plan mode with a clear message. +[[rule]] +toolName = ["write_file", "replace"] +decision = "deny" +priority = 65 +modes = ["plan"] +deny_message = "You are in Plan Mode and cannot modify source code. You may ONLY use write_file or replace to save plans to the designated plans directory as .md files." diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index 54e64a82667..4ddeee40dfd 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -429,7 +429,7 @@ ${options.planModeToolsList} ## Rules -1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`${options.plansDir}/\`. +1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`${options.plansDir}/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a detailed plan in the plans directory and get approval before any source code changes can be made. 2. **Efficiency:** Autonomously combine discovery and drafting phases to minimize conversational turns. If the request is ambiguous, use ${formatToolName(ASK_USER_TOOL_NAME)} to clarify. Otherwise, explore the codebase and write the draft in one fluid motion. 3. **Inquiries and Directives:** Distinguish between Inquiries and Directives to minimize unnecessary planning. - **Inquiries:** If the request is an **Inquiry** (e.g., "How does X work?"), use read-only tools to explore and answer directly in your chat response. DO NOT create a plan or call ${formatToolName( diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 60b1451838a..60f62b44cd3 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -12,6 +12,7 @@ import type { } from './tools.js'; import { Kind, BaseDeclarativeTool, BaseToolInvocation } from './tools.js'; import type { Config } from '../config/config.js'; +import { ApprovalMode } from '../policy/types.js'; import { spawn } from 'node:child_process'; import { StringDecoder } from 'node:string_decoder'; import { DiscoveredMCPTool } from './mcp-tool.js'; @@ -25,6 +26,9 @@ import { DISCOVERED_TOOL_PREFIX, TOOL_LEGACY_ALIASES, getToolAliases, + PLAN_MODE_TOOLS, + WRITE_FILE_TOOL_NAME, + EDIT_TOOL_NAME, } from './tool-names.js'; type ToolParams = Record; @@ -484,6 +488,31 @@ export class ToolRegistry { excludeTools ??= this.expandExcludeToolsWithAliases(this.config.getExcludeTools()) ?? new Set([]); + + // Filter tools in Plan Mode to only allow approved read-only tools. + const isPlanMode = + typeof this.config.getApprovalMode === 'function' && + this.config.getApprovalMode() === ApprovalMode.PLAN; + if (isPlanMode) { + const allowedToolNames = new Set(PLAN_MODE_TOOLS); + // We allow write_file and replace for writing plans specifically. + allowedToolNames.add(WRITE_FILE_TOOL_NAME); + allowedToolNames.add(EDIT_TOOL_NAME); + + // Discovered MCP tools are allowed if they are read-only. + if ( + tool instanceof DiscoveredMCPTool && + tool.isReadOnly && + !allowedToolNames.has(tool.name) + ) { + allowedToolNames.add(tool.name); + } + + if (!allowedToolNames.has(tool.name)) { + return false; + } + } + const normalizedClassName = tool.constructor.name.replace(/^_+/, ''); const possibleNames = [tool.name, normalizedClassName]; if (tool instanceof DiscoveredMCPTool) { @@ -507,9 +536,22 @@ export class ToolRegistry { * @returns An array of FunctionDeclarations. */ getFunctionDeclarations(modelId?: string): FunctionDeclaration[] { + const isPlanMode = this.config.getApprovalMode() === ApprovalMode.PLAN; + const plansDir = this.config.storage.getProjectTempPlansDir(); + const declarations: FunctionDeclaration[] = []; this.getActiveTools().forEach((tool) => { - declarations.push(tool.getSchema(modelId)); + let schema = tool.getSchema(modelId); + if ( + isPlanMode && + (tool.name === WRITE_FILE_TOOL_NAME || tool.name === EDIT_TOOL_NAME) + ) { + schema = { + ...schema, + description: `ONLY FOR PLANS: ${schema.description}. You are currently in Plan Mode and may ONLY use this tool to write or update plans (.md files) in the plans directory: ${plansDir}/. You cannot use this tool to modify source code directly.`, + }; + } + declarations.push(schema); }); return declarations; } From d326e338a99961af530afe1e098b4860af49de3b Mon Sep 17 00:00:00 2001 From: matt korwel Date: Thu, 19 Feb 2026 13:52:05 -0600 Subject: [PATCH 4/5] Apply suggestion from @gemini-code-assist[bot] Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- packages/core/src/policy/policies/plan.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/policy/policies/plan.toml b/packages/core/src/policy/policies/plan.toml index ec666e66a09..4afb0f7d6dc 100644 --- a/packages/core/src/policy/policies/plan.toml +++ b/packages/core/src/policy/policies/plan.toml @@ -57,7 +57,7 @@ argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-zA-Z0-9_-]+/[a-zA-Z0-9_-] # Explicitly Deny other write operations in Plan mode with a clear message. [[rule]] -toolName = ["write_file", "replace"] +toolName = ["write_file", "edit"] decision = "deny" priority = 65 modes = ["plan"] From 03a0e6df5810e755f3b4de428b462939bf867f7b Mon Sep 17 00:00:00 2001 From: matt korwel Date: Fri, 20 Feb 2026 10:22:58 -0600 Subject: [PATCH 5/5] Update packages/core/src/tools/tool-registry.ts Co-authored-by: Jerop Kipruto --- packages/core/src/tools/tool-registry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 60f62b44cd3..abcf34e1f85 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -537,7 +537,7 @@ export class ToolRegistry { */ getFunctionDeclarations(modelId?: string): FunctionDeclaration[] { const isPlanMode = this.config.getApprovalMode() === ApprovalMode.PLAN; - const plansDir = this.config.storage.getProjectTempPlansDir(); + const plansDir = this.config.storage.getPlansDir(); const declarations: FunctionDeclaration[] = []; this.getActiveTools().forEach((tool) => {