mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-15 06:12:50 -07:00
fix(core): strip redundant plan directory prefix in plan mode
This fix updates `resolveAndValidatePlanPath` to detect and strip redundant prefixes from file paths when the agent provides a path that begins with the basename of the plans directory (e.g., stripping `conductor/` when writing to `conductor/product.md` inside the `conductor` plan directory). This prevents the creation of nested directories (e.g., `conductor/conductor/product.md`) while maintaining path compatibility between Plan Mode and Execution Mode.
This commit is contained in:
@@ -8,7 +8,11 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import path from 'node:path';
|
||||
import * as fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import { validatePlanPath, validatePlanContent } from './planUtils.js';
|
||||
import {
|
||||
validatePlanPath,
|
||||
validatePlanContent,
|
||||
resolveAndValidatePlanPath,
|
||||
} from './planUtils.js';
|
||||
|
||||
describe('planUtils', () => {
|
||||
let tempRootDir: string;
|
||||
@@ -29,6 +33,56 @@ describe('planUtils', () => {
|
||||
}
|
||||
});
|
||||
|
||||
describe('resolveAndValidatePlanPath', () => {
|
||||
it('should strip redundant prefix matching plansDir basename', () => {
|
||||
const result = resolveAndValidatePlanPath('plans/test.md', plansDir);
|
||||
expect(result).toBe(path.join(plansDir, 'test.md'));
|
||||
});
|
||||
|
||||
it('should strip redundant prefix when path starts with ./', () => {
|
||||
const result = resolveAndValidatePlanPath('./plans/test.md', plansDir);
|
||||
expect(result).toBe(path.join(plansDir, 'test.md'));
|
||||
});
|
||||
|
||||
it('should strip redundant prefix matching plansDir basename (with nested path)', () => {
|
||||
const result = resolveAndValidatePlanPath(
|
||||
'plans/nested/test.md',
|
||||
plansDir,
|
||||
);
|
||||
expect(result).toBe(path.join(plansDir, 'nested/test.md'));
|
||||
});
|
||||
|
||||
it('should handle standard paths without the prefix', () => {
|
||||
const result = resolveAndValidatePlanPath('test.md', plansDir);
|
||||
expect(result).toBe(path.join(plansDir, 'test.md'));
|
||||
});
|
||||
|
||||
it('should handle standard paths with ./ prefix', () => {
|
||||
const result = resolveAndValidatePlanPath('./test.md', plansDir);
|
||||
expect(result).toBe(path.join(plansDir, 'test.md'));
|
||||
});
|
||||
|
||||
it('should throw if path is empty after stripping prefix', () => {
|
||||
expect(() => resolveAndValidatePlanPath('plans', plansDir)).toThrowError(
|
||||
/must include a filename/,
|
||||
);
|
||||
expect(() => resolveAndValidatePlanPath('plans/', plansDir)).toThrowError(
|
||||
/must include a filename/,
|
||||
);
|
||||
expect(() =>
|
||||
resolveAndValidatePlanPath('./plans', plansDir),
|
||||
).toThrowError(/must include a filename/);
|
||||
});
|
||||
|
||||
it('should handle mixed separators', () => {
|
||||
const result = resolveAndValidatePlanPath(
|
||||
'plans\\windows-style.md',
|
||||
plansDir,
|
||||
);
|
||||
expect(result).toBe(path.join(plansDir, 'windows-style.md'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('validatePlanPath', () => {
|
||||
it('should return null for a valid path within plans directory', async () => {
|
||||
const planPath = 'test.md';
|
||||
|
||||
@@ -39,7 +39,33 @@ export function resolveAndValidatePlanPath(
|
||||
throw new Error('Plan file path must be non-empty.');
|
||||
}
|
||||
|
||||
const resolvedPath = path.resolve(plansDir, trimmedPath);
|
||||
// Normalize separators to forward slashes for easier manipulation
|
||||
const normalizedInput = trimmedPath.replace(/\\/g, '/');
|
||||
|
||||
// Prevent redundant nesting if the agent includes the plans directory name in the path.
|
||||
// E.g. plansDir='/repo/conductor', planPath='conductor/test.md' -> we want '/repo/conductor/test.md'
|
||||
// Also handle './conductor/test.md' or 'conductor/nested/test.md'
|
||||
let normalizedPlanPath = normalizedInput;
|
||||
const plansDirName = path.basename(plansDir);
|
||||
|
||||
// Split into segments and remove empty or '.' segments from the beginning
|
||||
const segments = normalizedInput.split('/').filter((s) => s !== '');
|
||||
if (segments[0] === '.') {
|
||||
segments.shift();
|
||||
}
|
||||
|
||||
if (segments[0] === plansDirName) {
|
||||
// Strip the redundant prefix.
|
||||
segments.shift();
|
||||
normalizedPlanPath = segments.join('/');
|
||||
|
||||
// If the path was EXACTLY just the directory name (e.g. "conductor"), it's invalid for a file.
|
||||
if (!normalizedPlanPath) {
|
||||
throw new Error('Plan file path must include a filename.');
|
||||
}
|
||||
}
|
||||
|
||||
const resolvedPath = path.resolve(plansDir, normalizedPlanPath);
|
||||
const realPath = resolveToRealPath(resolvedPath);
|
||||
const realPlansDir = resolveToRealPath(plansDir);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user