mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-18 01:51:20 -07:00
feat(plan): add persistent plan file storage (#17563)
This commit is contained in:
@@ -57,7 +57,9 @@ vi.mock('fs', async (importOriginal) => {
|
||||
|
||||
return {
|
||||
...actualFs,
|
||||
mkdirSync: vi.fn(),
|
||||
mkdirSync: vi.fn((p) => {
|
||||
mockPaths.add(p.toString());
|
||||
}),
|
||||
writeFileSync: vi.fn(),
|
||||
existsSync: vi.fn((p) => mockPaths.has(p.toString())),
|
||||
statSync: vi.fn((p) => {
|
||||
|
||||
@@ -324,6 +324,117 @@ describe('Policy Engine Integration Tests', () => {
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should allow write_file to plans directory in Plan mode', async () => {
|
||||
const settings: Settings = {};
|
||||
|
||||
const config = await createPolicyEngineConfig(
|
||||
settings,
|
||||
ApprovalMode.PLAN,
|
||||
);
|
||||
const engine = new PolicyEngine(config);
|
||||
|
||||
// Valid plan file path (64-char hex hash, .md extension, safe filename)
|
||||
const validPlanPath =
|
||||
'/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/my-plan.md';
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: validPlanPath } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
|
||||
// Valid plan with underscore in filename
|
||||
const validPlanPath2 =
|
||||
'/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/feature_auth.md';
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: validPlanPath2 } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should deny write_file outside plans directory in Plan mode', async () => {
|
||||
const settings: Settings = {};
|
||||
|
||||
const config = await createPolicyEngineConfig(
|
||||
settings,
|
||||
ApprovalMode.PLAN,
|
||||
);
|
||||
const engine = new PolicyEngine(config);
|
||||
|
||||
// Write to workspace (not plans dir) should be denied
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: '/project/src/file.ts' } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
|
||||
// Write to plans dir but wrong extension should be denied
|
||||
const wrongExtPath =
|
||||
'/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/script.js';
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: wrongExtPath } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
|
||||
// Path traversal attempt should be denied (filename contains /)
|
||||
const traversalPath =
|
||||
'/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/../../../etc/passwd.md';
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: traversalPath } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
|
||||
// Invalid hash length should be denied
|
||||
const shortHashPath = '/home/user/.gemini/tmp/abc123/plans/plan.md';
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: shortHashPath } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should deny write_file to subdirectories in Plan mode', async () => {
|
||||
const settings: Settings = {};
|
||||
|
||||
const config = await createPolicyEngineConfig(
|
||||
settings,
|
||||
ApprovalMode.PLAN,
|
||||
);
|
||||
const engine = new PolicyEngine(config);
|
||||
|
||||
// Write to subdirectory should be denied
|
||||
const subdirPath =
|
||||
'/home/user/.gemini/tmp/a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2/plans/subdir/plan.md';
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'write_file', args: { file_path: subdirPath } },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should verify priority ordering works correctly in practice', async () => {
|
||||
const settings: Settings = {
|
||||
tools: {
|
||||
|
||||
@@ -14,6 +14,7 @@ import { ApprovalMode } from '../policy/types.js';
|
||||
import type { HookDefinition } from '../hooks/types.js';
|
||||
import { HookType, HookEventName } from '../hooks/types.js';
|
||||
import * as path from 'node:path';
|
||||
import * as fs from 'node:fs';
|
||||
import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js';
|
||||
import {
|
||||
DEFAULT_TELEMETRY_TARGET,
|
||||
@@ -2232,3 +2233,55 @@ describe('Config JIT Initialization', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('Plans Directory Initialization', () => {
|
||||
const baseParams: ConfigParameters = {
|
||||
sessionId: 'test-session',
|
||||
targetDir: '/tmp/test',
|
||||
debugMode: false,
|
||||
model: 'test-model',
|
||||
cwd: '/tmp/test',
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(fs.promises, 'mkdir').mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.mocked(fs.promises.mkdir).mockRestore();
|
||||
});
|
||||
|
||||
it('should create plans directory and add it to workspace context when plan is enabled', async () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: true,
|
||||
});
|
||||
|
||||
await config.initialize();
|
||||
|
||||
const plansDir = config.storage.getProjectTempPlansDir();
|
||||
expect(fs.promises.mkdir).toHaveBeenCalledWith(plansDir, {
|
||||
recursive: true,
|
||||
});
|
||||
|
||||
const context = config.getWorkspaceContext();
|
||||
expect(context.getDirectories()).toContain(plansDir);
|
||||
});
|
||||
|
||||
it('should NOT create plans directory or add it to workspace context when plan is disabled', async () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: false,
|
||||
});
|
||||
|
||||
await config.initialize();
|
||||
|
||||
const plansDir = config.storage.getProjectTempPlansDir();
|
||||
expect(fs.promises.mkdir).not.toHaveBeenCalledWith(plansDir, {
|
||||
recursive: true,
|
||||
});
|
||||
|
||||
const context = config.getWorkspaceContext();
|
||||
expect(context.getDirectories()).not.toContain(plansDir);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { inspect } from 'node:util';
|
||||
import process from 'node:process';
|
||||
@@ -696,6 +697,7 @@ export class Config {
|
||||
this.extensionManagement = params.extensionManagement ?? true;
|
||||
this.enableExtensionReloading = params.enableExtensionReloading ?? false;
|
||||
this.storage = new Storage(this.targetDir);
|
||||
|
||||
this.fakeResponses = params.fakeResponses;
|
||||
this.recordResponses = params.recordResponses;
|
||||
this.enablePromptCompletion = params.enablePromptCompletion ?? false;
|
||||
@@ -794,6 +796,13 @@ export class Config {
|
||||
this.workspaceContext.addDirectory(dir);
|
||||
}
|
||||
|
||||
// Add plans directory to workspace context for plan file storage
|
||||
if (this.planEnabled) {
|
||||
const plansDir = this.storage.getProjectTempPlansDir();
|
||||
await fs.promises.mkdir(plansDir, { recursive: true });
|
||||
this.workspaceContext.addDirectory(plansDir);
|
||||
}
|
||||
|
||||
// Initialize centralized FileDiscoveryService
|
||||
const discoverToolsHandle = startupProfiler.start('discover_tools');
|
||||
this.getFileService();
|
||||
|
||||
@@ -78,4 +78,10 @@ describe('Storage – additional helpers', () => {
|
||||
const expected = path.join(os.homedir(), GEMINI_DIR, 'tmp', 'bin');
|
||||
expect(Storage.getGlobalBinDir()).toBe(expected);
|
||||
});
|
||||
|
||||
it('getProjectTempPlansDir returns ~/.gemini/tmp/<hash>/plans', () => {
|
||||
const tempDir = storage.getProjectTempDir();
|
||||
const expected = path.join(tempDir, 'plans');
|
||||
expect(storage.getProjectTempPlansDir()).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -155,6 +155,10 @@ export class Storage {
|
||||
return path.join(this.getProjectTempDir(), 'logs');
|
||||
}
|
||||
|
||||
getProjectTempPlansDir(): string {
|
||||
return path.join(this.getProjectTempDir(), 'plans');
|
||||
}
|
||||
|
||||
getExtensionsDir(): string {
|
||||
return path.join(this.getGeminiDir(), 'extensions');
|
||||
}
|
||||
|
||||
@@ -182,6 +182,11 @@ You are operating in **Plan Mode** - a structured planning workflow for designin
|
||||
## Available Tools
|
||||
The following read-only tools are available in Plan Mode:
|
||||
|
||||
- \`write_file\` - Save plans to the plans directory (see Plan Storage below)
|
||||
|
||||
## Plan Storage
|
||||
- Save your plans as Markdown (.md) files directly to: \`/tmp/project-temp/plans/\`
|
||||
- Use descriptive filenames: \`feature-name.md\` or \`bugfix-description.md\`
|
||||
|
||||
## Workflow Phases
|
||||
|
||||
@@ -201,7 +206,7 @@ The following read-only tools are available in Plan Mode:
|
||||
- Only begin this phase after exploration is complete
|
||||
- Create a detailed implementation plan with clear steps
|
||||
- Include file paths, function signatures, and code snippets where helpful
|
||||
- Present the plan for review
|
||||
- After saving the plan, present the full content of the markdown file to the user for review
|
||||
|
||||
### Phase 4: Review & Approval
|
||||
- Ask the user if they approve the plan, want revisions, or want to reject it
|
||||
|
||||
@@ -65,6 +65,9 @@ describe('Core System Prompt (prompts.ts)', () => {
|
||||
getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
|
||||
storage: {
|
||||
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project-temp'),
|
||||
getProjectTempPlansDir: vi
|
||||
.fn()
|
||||
.mockReturnValue('/tmp/project-temp/plans'),
|
||||
},
|
||||
isInteractive: vi.fn().mockReturnValue(true),
|
||||
isInteractiveShellEnabled: vi.fn().mockReturnValue(true),
|
||||
|
||||
@@ -146,6 +146,8 @@ export function getCoreSystemPrompt(
|
||||
.map((toolName) => `- \`${toolName}\``)
|
||||
.join('\n');
|
||||
|
||||
const plansDir = config.storage.getProjectTempPlansDir();
|
||||
|
||||
approvalModePrompt = `
|
||||
# Active Approval Mode: Plan
|
||||
|
||||
@@ -154,6 +156,11 @@ You are operating in **Plan Mode** - a structured planning workflow for designin
|
||||
## Available Tools
|
||||
The following read-only tools are available in Plan Mode:
|
||||
${planModeToolsList}
|
||||
- \`${WRITE_FILE_TOOL_NAME}\` - Save plans to the plans directory (see Plan Storage below)
|
||||
|
||||
## Plan Storage
|
||||
- Save your plans as Markdown (.md) files directly to: \`${plansDir}/\`
|
||||
- Use descriptive filenames: \`feature-name.md\` or \`bugfix-description.md\`
|
||||
|
||||
## Workflow Phases
|
||||
|
||||
@@ -173,7 +180,7 @@ ${planModeToolsList}
|
||||
- Only begin this phase after exploration is complete
|
||||
- Create a detailed implementation plan with clear steps
|
||||
- Include file paths, function signatures, and code snippets where helpful
|
||||
- Present the plan for review
|
||||
- After saving the plan, present the full content of the markdown file to the user for review
|
||||
|
||||
### Phase 4: Review & Approval
|
||||
- Ask the user if they approve the plan, want revisions, or want to reject it
|
||||
|
||||
@@ -68,3 +68,11 @@ modes = ["plan"]
|
||||
toolName = "SubagentInvocation"
|
||||
decision = "allow"
|
||||
priority = 50
|
||||
|
||||
# Allow write_file for .md files in plans directory
|
||||
[[rule]]
|
||||
toolName = "write_file"
|
||||
decision = "allow"
|
||||
priority = 50
|
||||
modes = ["plan"]
|
||||
argsPattern = "\"file_path\":\"[^\"]+/\\.gemini/tmp/[a-f0-9]{64}/plans/[a-zA-Z0-9_-]+\\.md\""
|
||||
|
||||
@@ -47,6 +47,7 @@ import {
|
||||
} from '../test-utils/mock-message-bus.js';
|
||||
|
||||
const rootDir = path.resolve(os.tmpdir(), 'gemini-cli-test-root');
|
||||
const plansDir = path.resolve(os.tmpdir(), 'gemini-cli-test-plans');
|
||||
|
||||
// --- MOCKS ---
|
||||
vi.mock('../core/client.js');
|
||||
@@ -84,7 +85,7 @@ const mockConfigInternal = {
|
||||
getBaseLlmClient: vi.fn(), // Initialize as a plain mock function
|
||||
getFileSystemService: () => fsService,
|
||||
getIdeMode: vi.fn(() => false),
|
||||
getWorkspaceContext: () => new WorkspaceContext(rootDir),
|
||||
getWorkspaceContext: () => new WorkspaceContext(rootDir, [plansDir]),
|
||||
getApiKey: () => 'test-key',
|
||||
getModel: () => 'test-model',
|
||||
getSandbox: () => false,
|
||||
@@ -126,10 +127,13 @@ describe('WriteFileTool', () => {
|
||||
tempDir = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), 'write-file-test-external-'),
|
||||
);
|
||||
// Ensure the rootDir for the tool exists
|
||||
// Ensure the rootDir and plansDir for the tool exists
|
||||
if (!fs.existsSync(rootDir)) {
|
||||
fs.mkdirSync(rootDir, { recursive: true });
|
||||
}
|
||||
if (!fs.existsSync(plansDir)) {
|
||||
fs.mkdirSync(plansDir, { recursive: true });
|
||||
}
|
||||
|
||||
// Setup GeminiClient mock
|
||||
mockGeminiClientInstance = new (vi.mocked(GeminiClient))(
|
||||
@@ -206,6 +210,9 @@ describe('WriteFileTool', () => {
|
||||
if (fs.existsSync(rootDir)) {
|
||||
fs.rmSync(rootDir, { recursive: true, force: true });
|
||||
}
|
||||
if (fs.existsSync(plansDir)) {
|
||||
fs.rmSync(plansDir, { recursive: true, force: true });
|
||||
}
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
@@ -813,6 +820,24 @@ describe('WriteFileTool', () => {
|
||||
/File path must be within one of the workspace directories/,
|
||||
);
|
||||
});
|
||||
|
||||
it('should allow paths within the plans directory', () => {
|
||||
const params = {
|
||||
file_path: path.join(plansDir, 'my-plan.md'),
|
||||
content: '# My Plan',
|
||||
};
|
||||
expect(() => tool.build(params)).not.toThrow();
|
||||
});
|
||||
|
||||
it('should reject paths that try to escape the plans directory', () => {
|
||||
const params = {
|
||||
file_path: path.join(plansDir, '..', 'escaped.txt'),
|
||||
content: 'malicious',
|
||||
};
|
||||
expect(() => tool.build(params)).toThrow(
|
||||
/File path must be within one of the workspace directories/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('specific error types for write failures', () => {
|
||||
|
||||
Reference in New Issue
Block a user