Skip to content

Commit 314493c

Browse files
mahimashanwareadamfweidman
authored andcommitted
fix(core): resolve nested plan directory duplication and relative path policies (#25138)
1 parent 0ed38ff commit 314493c

17 files changed

Lines changed: 282 additions & 75 deletions

docs/cli/plan-mode.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,6 @@ Storage whenever Gemini CLI exits Plan Mode to start the implementation.
331331
#!/usr/bin/env bash
332332
# Extract the plan filename from the tool input JSON
333333
plan_filename=$(jq -r '.tool_input.plan_filename // empty')
334-
plan_filename=$(basename -- "$plan_filename")
335334

336335
# Construct the absolute path using the GEMINI_PLANS_DIR environment variable
337336
plan_path="$GEMINI_PLANS_DIR/$plan_filename"
@@ -360,7 +359,7 @@ To register this `AfterTool` hook, add it to your `settings.json`:
360359
{
361360
"name": "archive-plan",
362361
"type": "command",
363-
"command": "./.gemini/hooks/archive-plan.sh"
362+
"command": "~/.gemini/hooks/archive-plan.sh"
364363
}
365364
]
366365
}

evals/plan_mode.eval.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ describe('plan_mode', () => {
305305
settings,
306306
},
307307
prompt:
308-
'Enter plan mode and plan to create a new module called foo. The plan should be saved as foo-plan.md. Then, exit plan mode.',
308+
'I agree with your strategy. Please enter plan mode and draft the plan to create a new module called foo. The plan should be saved as foo-plan.md. Then, exit plan mode.',
309309
assert: async (rig, result) => {
310310
const enterPlanCalled = await rig.waitForToolCall('enter_plan_mode');
311311
expect(
@@ -376,4 +376,48 @@ describe('plan_mode', () => {
376376
assertModelHasOutput(result);
377377
},
378378
});
379+
380+
evalTest('USUALLY_PASSES', {
381+
name: 'should handle nested plan directories correctly',
382+
suiteName: 'plan_mode',
383+
suiteType: 'behavioral',
384+
approvalMode: ApprovalMode.PLAN,
385+
params: {
386+
settings,
387+
},
388+
prompt:
389+
'Please create a new architectural plan in a nested folder called "architecture/frontend-v2.md" within the plans directory. The plan should contain the text "# Frontend V2 Plan". Then, exit plan mode',
390+
assert: async (rig, result) => {
391+
await rig.waitForTelemetryReady();
392+
const toolLogs = rig.readToolLogs();
393+
394+
const writeCalls = toolLogs.filter((log) =>
395+
['write_file', 'replace'].includes(log.toolRequest.name),
396+
);
397+
398+
const wroteToNestedPath = writeCalls.some((log) => {
399+
try {
400+
const args = JSON.parse(log.toolRequest.args);
401+
if (!args.file_path) return false;
402+
// In plan mode, paths can be passed as relative (architecture/frontend-v2.md)
403+
// or they might be resolved as absolute by the tool depending on the exact mock state.
404+
// We strictly ensure it ends exactly with the expected nested path and doesn't contain extra nesting.
405+
const normalizedPath = args.file_path.replace(/\\/g, '/');
406+
return (
407+
normalizedPath === 'architecture/frontend-v2.md' ||
408+
normalizedPath.endsWith('/plans/architecture/frontend-v2.md')
409+
);
410+
} catch {
411+
return false;
412+
}
413+
});
414+
415+
expect(
416+
wroteToNestedPath,
417+
'Expected model to successfully target the nested plan file path',
418+
).toBe(true);
419+
420+
assertModelHasOutput(result);
421+
},
422+
});
379423
});

packages/cli/src/ui/components/ExitPlanModeDialog.test.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ Implement a comprehensive authentication system with multiple providers.
159159
isTrustedFolder: () => true,
160160
getPreferredEditor: () => undefined,
161161
getSessionId: () => 'test-session-id',
162+
getProjectRoot: () => mockTargetDir,
162163
storage: {
163164
getPlansDir: () => mockPlansDir,
164165
},
@@ -466,6 +467,7 @@ Implement a comprehensive authentication system with multiple providers.
466467
getIdeMode: () => false,
467468
isTrustedFolder: () => true,
468469
getSessionId: () => 'test-session-id',
470+
getProjectRoot: () => mockTargetDir,
469471
storage: {
470472
getPlansDir: () => mockPlansDir,
471473
},

packages/cli/src/ui/components/ExitPlanModeDialog.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ function usePlanContent(planPath: string, config: Config): PlanContentState {
8585
const pathError = await validatePlanPath(
8686
planPath,
8787
config.storage.getPlansDir(),
88+
config.getProjectRoot(),
8889
);
8990
if (ignore) return;
9091
if (pathError) {

packages/cli/src/ui/components/ToolConfirmationQueue.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ describe('ToolConfirmationQueue', () => {
5252
getModel: () => 'gemini-pro',
5353
getDebugMode: () => false,
5454
getTargetDir: () => '/mock/target/dir',
55+
getProjectRoot: () => '/mock/project/root',
5556
getFileSystemService: () => ({
5657
readFile: vi.fn().mockResolvedValue('Plan content'),
5758
}),

packages/core/src/core/__snapshots__/prompts.test.ts.snap

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ For example:
9595

9696
# Active Approval Mode: Plan
9797

98-
You are operating in **Plan Mode**. Your goal is to produce an implementation plan in \`/tmp/plans/\` and get user approval before editing source code.
98+
You are operating in **Plan Mode**. Your goal is to produce an implementation plan in \`../plans/\` and get user approval before editing source code.
9999

100100
## Available Tools
101101
The following tools are available in Plan Mode:
@@ -111,8 +111,8 @@ The following tools are available in Plan Mode:
111111
</available_tools>
112112

113113
## Rules
114-
1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`/tmp/plans/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a plan and get approval.
115-
2. **Write Constraint:** \`write_file\` and \`replace\` may ONLY be used to write .md plan files to \`/tmp/plans/\`. They cannot modify source code.
114+
1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`../plans/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a plan and get approval.
115+
2. **Write Constraint:** \`write_file\` and \`replace\` may ONLY be used to write .md plan files to \`../plans/\`. They cannot modify source code.
116116
3. **Efficiency:** Autonomously combine discovery and drafting phases to minimize conversational turns. If the request is ambiguous, use \`ask_user\` to clarify. Use multi-select to offer flexibility and include detailed descriptions for each option to help the user understand the implications of their choice.
117117
4. **Inquiries and Directives:** Distinguish between Inquiries and Directives to minimize unnecessary planning.
118118
- **Inquiries:** If the request is an **Inquiry** (e.g., "How does X work?"), answer directly. DO NOT create a plan.
@@ -136,7 +136,7 @@ The depth of your consultation should be proportional to the task's complexity.
136136
**CRITICAL:** You MUST NOT proceed to Step 3 (Draft) or Step 4 (Review & Approval) in the same turn as your initial strategy proposal. You MUST wait for user feedback and reach a clear agreement before drafting or submitting the plan.
137137

138138
### 3. Draft
139-
Write the implementation plan to \`/tmp/plans/\`. The plan's structure adapts to the task:
139+
Write the implementation plan to \`../plans/\`. The plan's structure adapts to the task:
140140
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
141141
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
142142
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.
@@ -275,7 +275,7 @@ For example:
275275

276276
# Active Approval Mode: Plan
277277

278-
You are operating in **Plan Mode**. Your goal is to produce an implementation plan in \`/tmp/plans/\` and get user approval before editing source code.
278+
You are operating in **Plan Mode**. Your goal is to produce an implementation plan in \`../plans/\` and get user approval before editing source code.
279279

280280
## Available Tools
281281
The following tools are available in Plan Mode:
@@ -291,8 +291,8 @@ The following tools are available in Plan Mode:
291291
</available_tools>
292292

293293
## Rules
294-
1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`/tmp/plans/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a plan and get approval.
295-
2. **Write Constraint:** \`write_file\` and \`replace\` may ONLY be used to write .md plan files to \`/tmp/plans/\`. They cannot modify source code.
294+
1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`../plans/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a plan and get approval.
295+
2. **Write Constraint:** \`write_file\` and \`replace\` may ONLY be used to write .md plan files to \`../plans/\`. They cannot modify source code.
296296
3. **Efficiency:** Autonomously combine discovery and drafting phases to minimize conversational turns. If the request is ambiguous, use \`ask_user\` to clarify. Use multi-select to offer flexibility and include detailed descriptions for each option to help the user understand the implications of their choice.
297297
4. **Inquiries and Directives:** Distinguish between Inquiries and Directives to minimize unnecessary planning.
298298
- **Inquiries:** If the request is an **Inquiry** (e.g., "How does X work?"), answer directly. DO NOT create a plan.
@@ -316,7 +316,7 @@ The depth of your consultation should be proportional to the task's complexity.
316316
**CRITICAL:** You MUST NOT proceed to Step 3 (Draft) or Step 4 (Review & Approval) in the same turn as your initial strategy proposal. You MUST wait for user feedback and reach a clear agreement before drafting or submitting the plan.
317317

318318
### 3. Draft
319-
Write the implementation plan to \`/tmp/plans/\`. The plan's structure adapts to the task:
319+
Write the implementation plan to \`../plans/\`. The plan's structure adapts to the task:
320320
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
321321
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
322322
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.
@@ -326,7 +326,7 @@ Write the implementation plan to \`/tmp/plans/\`. The plan's structure adapts to
326326
ONLY use the \`exit_plan_mode\` tool to present the plan for formal approval AFTER you have reached an informal agreement with the user in the chat regarding the proposed strategy. When called, this tool will present the plan and formally request approval.
327327

328328
## Approved Plan
329-
An approved plan is available for this task at \`/tmp/plans/feature-x.md\`.
329+
An approved plan is available for this task at \`../plans/feature-x.md\`.
330330
- **Read First:** You MUST read this file using the \`read_file\` tool before proposing any changes or starting discovery.
331331
- **Iterate:** Default to refining the existing approved plan.
332332
- **New Plan:** Only create a new plan file if the user explicitly asks for a "new plan".
@@ -576,7 +576,7 @@ For example:
576576

577577
# Active Approval Mode: Plan
578578

579-
You are operating in **Plan Mode**. Your goal is to produce an implementation plan in \`/tmp/project-temp/plans/\` and get user approval before editing source code.
579+
You are operating in **Plan Mode**. Your goal is to produce an implementation plan in \`plans/\` and get user approval before editing source code.
580580

581581
## Available Tools
582582
The following tools are available in Plan Mode:
@@ -592,8 +592,8 @@ The following tools are available in Plan Mode:
592592
</available_tools>
593593

594594
## Rules
595-
1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`/tmp/project-temp/plans/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a plan and get approval.
596-
2. **Write Constraint:** \`write_file\` and \`replace\` may ONLY be used to write .md plan files to \`/tmp/project-temp/plans/\`. They cannot modify source code.
595+
1. **Read-Only:** You cannot modify source code. You may ONLY use read-only tools to explore, and you can only write to \`plans/\`. If the user asks you to modify source code directly, you MUST explain that you are in Plan Mode and must first create a plan and get approval.
596+
2. **Write Constraint:** \`write_file\` and \`replace\` may ONLY be used to write .md plan files to \`plans/\`. They cannot modify source code.
597597
3. **Efficiency:** Autonomously combine discovery and drafting phases to minimize conversational turns. If the request is ambiguous, use \`ask_user\` to clarify. Use multi-select to offer flexibility and include detailed descriptions for each option to help the user understand the implications of their choice.
598598
4. **Inquiries and Directives:** Distinguish between Inquiries and Directives to minimize unnecessary planning.
599599
- **Inquiries:** If the request is an **Inquiry** (e.g., "How does X work?"), answer directly. DO NOT create a plan.
@@ -617,7 +617,7 @@ The depth of your consultation should be proportional to the task's complexity.
617617
**CRITICAL:** You MUST NOT proceed to Step 3 (Draft) or Step 4 (Review & Approval) in the same turn as your initial strategy proposal. You MUST wait for user feedback and reach a clear agreement before drafting or submitting the plan.
618618

619619
### 3. Draft
620-
Write the implementation plan to \`/tmp/project-temp/plans/\`. The plan's structure adapts to the task:
620+
Write the implementation plan to \`plans/\`. The plan's structure adapts to the task:
621621
- **Simple Tasks:** Include a bulleted list of specific **Changes** and **Verification** steps.
622622
- **Standard Tasks:** Include an **Objective**, **Key Files & Context**, **Implementation Steps**, and **Verification & Testing**.
623623
- **Complex Tasks:** Include **Background & Motivation**, **Scope & Impact**, **Proposed Solution**, **Alternatives Considered**, a phased **Implementation Plan**, **Verification**, and **Migration & Rollback** strategies.

packages/core/src/core/prompts.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ describe('Core System Prompt (prompts.ts)', () => {
9393
getToolRegistry: vi.fn().mockReturnValue(mockRegistry),
9494
getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
9595
getSandboxEnabled: vi.fn().mockReturnValue(false),
96+
getProjectRoot: vi.fn().mockReturnValue('/tmp/project-temp'),
9697
storage: {
9798
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project-temp'),
9899
getPlansDir: vi.fn().mockReturnValue('/tmp/project-temp/plans'),

packages/core/src/prompts/promptProvider.test.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
88
import { PromptProvider } from './promptProvider.js';
99
import type { Config } from '../config/config.js';
10+
import { makeRelative } from '../utils/paths.js';
1011
import {
1112
getAllGeminiMdFilenames,
1213
DEFAULT_CONTEXT_FILENAME,
@@ -58,6 +59,7 @@ describe('PromptProvider', () => {
5859
).getToolRegistry?.() as unknown as ToolRegistry;
5960
},
6061
getToolRegistry: vi.fn().mockReturnValue(mockToolRegistry),
62+
getProjectRoot: vi.fn().mockReturnValue('/tmp/project-temp'),
6163
topicState: new TopicState(),
6264
getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
6365
getSandboxEnabled: vi.fn().mockReturnValue(false),
@@ -236,7 +238,14 @@ describe('PromptProvider', () => {
236238
expect(prompt).toContain(
237239
'`write_file` and `replace` may ONLY be used to write .md plan files',
238240
);
239-
expect(prompt).toContain('/tmp/project-temp/plans/');
241+
242+
const expectedRelativePath = makeRelative(
243+
mockConfig.storage.getPlansDir(),
244+
mockConfig.getProjectRoot(),
245+
).replaceAll('\\', '/');
246+
expect(prompt).toContain(
247+
`write .md plan files to \`${expectedRelativePath}/\``,
248+
);
240249
});
241250
});
242251

packages/core/src/prompts/promptProvider.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import fs from 'node:fs';
88
import path from 'node:path';
99
import process from 'node:process';
1010
import type { HierarchicalMemory } from '../config/memory.js';
11-
import { GEMINI_DIR } from '../utils/paths.js';
11+
import { GEMINI_DIR, makeRelative } from '../utils/paths.js';
1212
import { ApprovalMode } from '../policy/types.js';
1313
import * as snippets from './snippets.js';
1414
import * as legacySnippets from './snippets.legacy.js';
@@ -199,8 +199,19 @@ export class PromptProvider {
199199
() => ({
200200
interactive: interactiveMode,
201201
planModeToolsList,
202-
plansDir: context.config.storage.getPlansDir(),
203-
approvedPlanPath: context.config.getApprovedPlanPath(),
202+
plansDir: makeRelative(
203+
context.config.storage.getPlansDir(),
204+
context.config.getProjectRoot(),
205+
).replaceAll('\\', '/'),
206+
approvedPlanPath: (() => {
207+
const approvedPath = context.config.getApprovedPlanPath();
208+
return approvedPath
209+
? makeRelative(
210+
approvedPath,
211+
context.config.getProjectRoot(),
212+
).replaceAll('\\', '/')
213+
: undefined;
214+
})(),
204215
}),
205216
isPlanMode,
206217
),

packages/core/src/tools/edit.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ describe('EditTool', () => {
107107
getGeminiClient: vi.fn().mockReturnValue(geminiClient),
108108
getBaseLlmClient: vi.fn().mockReturnValue(baseLlmClient),
109109
getTargetDir: () => rootDir,
110+
getProjectRoot: () => rootDir,
110111
getApprovalMode: vi.fn(),
111112
setApprovalMode: vi.fn(),
112113
getWorkspaceContext: () => createMockWorkspaceContext(rootDir),
@@ -1336,8 +1337,8 @@ function doIt() {
13361337
vi.mocked(mockConfig.isPlanMode).mockReturnValue(true);
13371338
vi.mocked(mockConfig.storage.getPlansDir).mockReturnValue(plansDir);
13381339

1339-
const filePath = path.join(rootDir, 'test-file.txt');
1340-
const planFilePath = path.join(plansDir, 'test-file.txt');
1340+
const filePath = 'test-file.txt';
1341+
const planFilePath = path.join(plansDir, filePath);
13411342
const initialContent = 'some initial content';
13421343
fs.writeFileSync(planFilePath, initialContent, 'utf8');
13431344

0 commit comments

Comments
 (0)