feat(plan): support configuring custom plans storage directory (#19577)

This commit is contained in:
Jerop Kipruto
2026-02-19 17:47:08 -05:00
committed by GitHub
parent 2cba2ab37a
commit 537e56ffae
24 changed files with 337 additions and 58 deletions

View File

@@ -737,6 +737,42 @@ describe('Server Config (config.ts)', () => {
);
});
describe('Plan Settings', () => {
const testCases = [
{
name: 'should pass custom plan directory to storage',
planSettings: { directory: 'custom-plans' },
expected: 'custom-plans',
},
{
name: 'should call setCustomPlansDir with undefined if directory is not provided',
planSettings: {},
expected: undefined,
},
{
name: 'should call setCustomPlansDir with undefined if planSettings is not provided',
planSettings: undefined,
expected: undefined,
},
];
testCases.forEach(({ name, planSettings, expected }) => {
it(`${name}`, () => {
const setCustomPlansDirSpy = vi.spyOn(
Storage.prototype,
'setCustomPlansDir',
);
new Config({
...baseParams,
planSettings,
});
expect(setCustomPlansDirSpy).toHaveBeenCalledWith(expected);
setCustomPlansDirSpy.mockRestore();
});
});
});
describe('Telemetry Settings', () => {
it('should return default telemetry target if not provided', () => {
const params: ConfigParameters = {
@@ -2501,7 +2537,7 @@ describe('Plans Directory Initialization', () => {
await config.initialize();
const plansDir = config.storage.getProjectTempPlansDir();
const plansDir = config.storage.getPlansDir();
expect(fs.promises.mkdir).toHaveBeenCalledWith(plansDir, {
recursive: true,
});
@@ -2518,7 +2554,7 @@ describe('Plans Directory Initialization', () => {
await config.initialize();
const plansDir = config.storage.getProjectTempPlansDir();
const plansDir = config.storage.getPlansDir();
expect(fs.promises.mkdir).not.toHaveBeenCalledWith(plansDir, {
recursive: true,
});

View File

@@ -141,6 +141,10 @@ export interface SummarizeToolOutputSettings {
tokenBudget?: number;
}
export interface PlanSettings {
directory?: string;
}
export interface TelemetrySettings {
enabled?: boolean;
target?: TelemetryTarget;
@@ -483,6 +487,7 @@ export interface ConfigParameters {
toolOutputMasking?: Partial<ToolOutputMaskingConfig>;
disableLLMCorrection?: boolean;
plan?: boolean;
planSettings?: PlanSettings;
modelSteering?: boolean;
onModelChange?: (model: string) => void;
mcpEnabled?: boolean;
@@ -836,6 +841,7 @@ export class Config {
this.extensionManagement = params.extensionManagement ?? true;
this.enableExtensionReloading = params.enableExtensionReloading ?? false;
this.storage = new Storage(this.targetDir, this.sessionId);
this.storage.setCustomPlansDir(params.planSettings?.directory);
this.fakeResponses = params.fakeResponses;
this.recordResponses = params.recordResponses;
@@ -949,7 +955,7 @@ export class Config {
// Add plans directory to workspace context for plan file storage
if (this.planEnabled) {
const plansDir = this.storage.getProjectTempPlansDir();
const plansDir = this.storage.getPlansDir();
await fs.promises.mkdir(plansDir, { recursive: true });
this.workspaceContext.addDirectory(plansDir);
}

View File

@@ -12,12 +12,14 @@ vi.unmock('./storageMigration.js');
import * as os from 'node:os';
import * as path from 'node:path';
import * as fs from 'node:fs';
vi.mock('fs', async (importOriginal) => {
const actual = await importOriginal<typeof import('fs')>();
return {
...actual,
mkdirSync: vi.fn(),
realpathSync: vi.fn(actual.realpathSync),
};
});
@@ -61,12 +63,11 @@ describe('Storage initialize', () => {
).toHaveBeenCalledWith(projectRoot);
// Verify migration calls
const shortId = 'project-slug';
// We can't easily get the hash here without repeating logic, but we can verify it's called twice
expect(StorageMigration.migrateDirectory).toHaveBeenCalledTimes(2);
// Verify identifier is set by checking a path
expect(storage.getProjectTempDir()).toContain(shortId);
expect(storage.getProjectTempDir()).toContain(PROJECT_SLUG);
});
});
@@ -105,6 +106,12 @@ describe('Storage additional helpers', () => {
const projectRoot = '/tmp/project';
const storage = new Storage(projectRoot);
beforeEach(() => {
ProjectRegistry.prototype.getShortId = vi
.fn()
.mockReturnValue(PROJECT_SLUG);
});
it('getWorkspaceSettingsPath returns project/.gemini/settings.json', () => {
const expected = path.join(projectRoot, GEMINI_DIR, 'settings.json');
expect(storage.getWorkspaceSettingsPath()).toBe(expected);
@@ -172,6 +179,101 @@ describe('Storage additional helpers', () => {
const expected = path.join(tempDir, sessionId, 'plans');
expect(storageWithSession.getProjectTempPlansDir()).toBe(expected);
});
describe('getPlansDir', () => {
interface TestCase {
name: string;
customDir: string | undefined;
expected: string | (() => string);
expectedError?: string;
setup?: () => () => void;
}
const testCases: TestCase[] = [
{
name: 'custom relative path',
customDir: '.my-plans',
expected: path.resolve(projectRoot, '.my-plans'),
},
{
name: 'custom absolute path outside throws',
customDir: '/absolute/path/to/plans',
expected: '',
expectedError:
"Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '/tmp/project'.",
},
{
name: 'absolute path that happens to be inside project root',
customDir: path.join(projectRoot, 'internal-plans'),
expected: path.join(projectRoot, 'internal-plans'),
},
{
name: 'relative path that stays within project root',
customDir: 'subdir/../plans',
expected: path.resolve(projectRoot, 'plans'),
},
{
name: 'dot path',
customDir: '.',
expected: projectRoot,
},
{
name: 'default behavior when customDir is undefined',
customDir: undefined,
expected: () => storage.getProjectTempPlansDir(),
},
{
name: 'escaping relative path throws',
customDir: '../escaped-plans',
expected: '',
expectedError:
"Custom plans directory '../escaped-plans' resolves to '/tmp/escaped-plans', which is outside the project root '/tmp/project'.",
},
{
name: 'hidden directory starting with ..',
customDir: '..plans',
expected: path.resolve(projectRoot, '..plans'),
},
{
name: 'security escape via symbolic link throws',
customDir: 'symlink-to-outside',
setup: () => {
vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => {
if (p.toString().includes('symlink-to-outside')) {
return '/outside/project/root';
}
return p.toString();
});
return () => vi.mocked(fs.realpathSync).mockRestore();
},
expected: '',
expectedError:
"Custom plans directory 'symlink-to-outside' resolves to '/outside/project/root', which is outside the project root '/tmp/project'.",
},
];
testCases.forEach(({ name, customDir, expected, expectedError, setup }) => {
it(`should handle ${name}`, async () => {
const cleanup = setup?.();
try {
if (name.includes('default behavior')) {
await storage.initialize();
}
storage.setCustomPlansDir(customDir);
if (expectedError) {
expect(() => storage.getPlansDir()).toThrow(expectedError);
} else {
const expectedValue =
typeof expected === 'function' ? expected() : expected;
expect(storage.getPlansDir()).toBe(expectedValue);
}
} finally {
cleanup?.();
}
});
});
});
});
describe('Storage - System Paths', () => {

View File

@@ -12,6 +12,8 @@ import {
GEMINI_DIR,
homedir,
GOOGLE_ACCOUNTS_FILENAME,
isSubpath,
resolveToRealPath,
} from '../utils/paths.js';
import { ProjectRegistry } from './projectRegistry.js';
import { StorageMigration } from './storageMigration.js';
@@ -26,12 +28,17 @@ export class Storage {
private readonly sessionId: string | undefined;
private projectIdentifier: string | undefined;
private initPromise: Promise<void> | undefined;
private customPlansDir: string | undefined;
constructor(targetDir: string, sessionId?: string) {
this.targetDir = targetDir;
this.sessionId = sessionId;
}
setCustomPlansDir(dir: string | undefined): void {
this.customPlansDir = dir;
}
static getGlobalGeminiDir(): string {
const homeDir = homedir();
if (!homeDir) {
@@ -253,6 +260,26 @@ export class Storage {
return path.join(this.getProjectTempDir(), 'plans');
}
getPlansDir(): string {
if (this.customPlansDir) {
const resolvedPath = path.resolve(
this.getProjectRoot(),
this.customPlansDir,
);
const realProjectRoot = resolveToRealPath(this.getProjectRoot());
const realResolvedPath = resolveToRealPath(resolvedPath);
if (!isSubpath(realProjectRoot, realResolvedPath)) {
throw new Error(
`Custom plans directory '${this.customPlansDir}' resolves to '${realResolvedPath}', which is outside the project root '${realProjectRoot}'.`,
);
}
return resolvedPath;
}
return this.getProjectTempPlansDir();
}
getProjectTempTasksDir(): string {
if (this.sessionId) {
return path.join(this.getProjectTempDir(), this.sessionId, 'tasks');

View File

@@ -89,9 +89,7 @@ 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'),
getPlansDir: vi.fn().mockReturnValue('/tmp/project-temp/plans'),
},
isInteractive: vi.fn().mockReturnValue(true),
isInteractiveShellEnabled: vi.fn().mockReturnValue(true),
@@ -509,9 +507,7 @@ describe('Core System Prompt (prompts.ts)', () => {
vi.mocked(mockConfig.getApprovalMode).mockReturnValue(
ApprovalMode.PLAN,
);
vi.mocked(mockConfig.storage.getProjectTempPlansDir).mockReturnValue(
'/tmp/plans',
);
vi.mocked(mockConfig.storage.getPlansDir).mockReturnValue('/tmp/plans');
});
it('should include approved plan path when set in config', () => {

View File

@@ -38,9 +38,7 @@ describe('PromptProvider', () => {
getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project-temp'),
getProjectTempPlansDir: vi
.fn()
.mockReturnValue('/tmp/project-temp/plans'),
getPlansDir: vi.fn().mockReturnValue('/tmp/project-temp/plans'),
},
isInteractive: vi.fn().mockReturnValue(true),
isInteractiveShellEnabled: vi.fn().mockReturnValue(true),

View File

@@ -172,7 +172,7 @@ export class PromptProvider {
'planningWorkflow',
() => ({
planModeToolsList,
plansDir: config.storage.getProjectTempPlansDir(),
plansDir: config.storage.getPlansDir(),
approvedPlanPath: config.getApprovedPlanPath(),
}),
isPlanMode,

View File

@@ -24,7 +24,7 @@ describe('EnterPlanModeTool', () => {
mockConfig = {
setApprovalMode: vi.fn(),
storage: {
getProjectTempPlansDir: vi.fn().mockReturnValue('/mock/plans/dir'),
getPlansDir: vi.fn().mockReturnValue('/mock/plans/dir'),
} as unknown as Config['storage'],
};
tool = new EnterPlanModeTool(

View File

@@ -45,7 +45,7 @@ describe('ExitPlanModeTool', () => {
setApprovalMode: vi.fn(),
setApprovedPlanPath: vi.fn(),
storage: {
getProjectTempPlansDir: vi.fn().mockReturnValue(mockPlansDir),
getPlansDir: vi.fn().mockReturnValue(mockPlansDir),
} as unknown as Config['storage'],
};
tool = new ExitPlanModeTool(

View File

@@ -57,7 +57,7 @@ export class ExitPlanModeTool extends BaseDeclarativeTool<
private config: Config,
messageBus: MessageBus,
) {
const plansDir = config.storage.getProjectTempPlansDir();
const plansDir = config.storage.getPlansDir();
const definition = getExitPlanModeDefinition(plansDir);
super(
EXIT_PLAN_MODE_TOOL_NAME,
@@ -78,9 +78,7 @@ export class ExitPlanModeTool extends BaseDeclarativeTool<
// Since validateToolParamValues is synchronous, we use a basic synchronous check
// for path traversal safety. High-level async validation is deferred to shouldConfirmExecute.
const plansDir = resolveToRealPath(
this.config.storage.getProjectTempPlansDir(),
);
const plansDir = resolveToRealPath(this.config.storage.getPlansDir());
const resolvedPath = path.resolve(
this.config.getTargetDir(),
params.plan_path,
@@ -111,7 +109,7 @@ export class ExitPlanModeTool extends BaseDeclarativeTool<
}
override getSchema(modelId?: string) {
const plansDir = this.config.storage.getProjectTempPlansDir();
const plansDir = this.config.storage.getPlansDir();
return resolveToolDeclaration(getExitPlanModeDefinition(plansDir), modelId);
}
}
@@ -141,7 +139,7 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
const pathError = await validatePlanPath(
this.params.plan_path,
this.config.storage.getProjectTempPlansDir(),
this.config.storage.getPlansDir(),
this.config.getTargetDir(),
);
if (pathError) {