From aad3e4b148143c75e64450f64fdbc816262c9c6f Mon Sep 17 00:00:00 2001 From: Mahima Shanware Date: Tue, 14 Apr 2026 05:45:22 +0000 Subject: [PATCH] feat(core): implement JIT plan directory provisioning with path safety --- .../core/src/tools/enter-plan-mode.test.ts | 109 +++++++++++++++++- packages/core/src/tools/enter-plan-mode.ts | 48 ++++++-- 2 files changed, 145 insertions(+), 12 deletions(-) diff --git a/packages/core/src/tools/enter-plan-mode.test.ts b/packages/core/src/tools/enter-plan-mode.test.ts index 0b477d7be6..ff584aaafc 100644 --- a/packages/core/src/tools/enter-plan-mode.test.ts +++ b/packages/core/src/tools/enter-plan-mode.test.ts @@ -39,6 +39,8 @@ describe('EnterPlanModeTool', () => { mockConfig = { setApprovalMode: vi.fn(), + getExtensions: vi.fn().mockReturnValue([]), + getExtensionSetting: vi.fn(), storage: { getPlansDir: vi.fn().mockReturnValue('/mock/plans/dir'), } as unknown as Config['storage'], @@ -132,15 +134,114 @@ describe('EnterPlanModeTool', () => { expect(result.returnDisplay).toBe('Switching to Plan mode'); }); - it('should create plans directory if it does not exist', async () => { - const invocation = tool.build({}); - vi.mocked(fs.existsSync).mockReturnValue(false); + it('should create custom plan directories for active extensions, handling overrides and fallbacks', async () => { + vi.mocked(mockConfig.getExtensions!).mockReturnValue([ + { + name: 'ext-user-override', + isActive: true, + plan: { directory: '.manifest-dir' }, // Manifest default exists + } as import('../config/config.js').GeminiCLIExtension, + { + name: 'ext-manifest-fallback', + isActive: true, + plan: { directory: '.manifest-dir' }, // Only manifest default + } as import('../config/config.js').GeminiCLIExtension, + { + name: 'ext-no-custom', + isActive: true, + } as import('../config/config.js').GeminiCLIExtension, + { + name: 'ext-inactive', + isActive: false, + plan: { directory: '.inactive-dir' }, + } as import('../config/config.js').GeminiCLIExtension, + ]); + vi.mocked(mockConfig.getExtensionSetting!).mockImplementation( + (name, setting) => { + if (name === 'ext-user-override' && setting === 'plan.directory') { + return '.user-override-dir'; // User setting wins + } + return undefined; + }, + ); + + vi.mocked(mockConfig.storage!.getPlansDir).mockImplementation( + (customDir?: string) => { + if (customDir === '.user-override-dir') + return '/mock/plans/user-override'; + if (customDir === '.manifest-dir') + return '/mock/plans/manifest-default'; + return '/mock/plans/global-default'; + }, + ); + + const invocation = tool.build({}); await invocation.execute({ abortSignal: new AbortController().signal }); - expect(fs.mkdirSync).toHaveBeenCalledWith('/mock/plans/dir', { + // 1. Global default should be created + expect(fs.mkdirSync).toHaveBeenCalledWith('/mock/plans/global-default', { recursive: true, }); + // 2. User override should be created for ext-user-override + expect(fs.mkdirSync).toHaveBeenCalledWith('/mock/plans/user-override', { + recursive: true, + }); + // 3. Manifest default should be created for ext-manifest-fallback + expect(fs.mkdirSync).toHaveBeenCalledWith( + '/mock/plans/manifest-default', + { + recursive: true, + }, + ); + // 4. No folder should be created for ext-no-custom + // 5. No folder should be created for ext-inactive + expect(fs.mkdirSync).not.toHaveBeenCalledWith( + '/mock/plans/inactive-dir', + { + recursive: true, + }, + ); + + expect(fs.mkdirSync).toHaveBeenCalledTimes(3); + }); + + it('should ignore validation failures for extension-specific plan directories and continue', async () => { + vi.mocked(mockConfig.getExtensions!).mockReturnValue([ + { + name: 'ext-invalid', + isActive: true, + plan: { directory: '../illegal' }, + } as import('../config/config.js').GeminiCLIExtension, + { + name: 'ext-valid', + isActive: true, + plan: { directory: '.valid' }, + } as import('../config/config.js').GeminiCLIExtension, + ]); + + vi.mocked(mockConfig.storage!.getPlansDir).mockImplementation( + (customDir?: string) => { + if (customDir === '../illegal') + throw new Error('Path traversal detected'); + if (customDir === '.valid') return '/mock/plans/valid'; + return '/mock/plans/global-default'; + }, + ); + + const invocation = tool.build({}); + await invocation.execute({ abortSignal: new AbortController().signal }); + + // Should create global default + expect(fs.mkdirSync).toHaveBeenCalledWith('/mock/plans/global-default', { + recursive: true, + }); + // Should create valid extension dir + expect(fs.mkdirSync).toHaveBeenCalledWith('/mock/plans/valid', { + recursive: true, + }); + // Should NOT have crashed on illegal dir + expect(fs.mkdirSync).toHaveBeenCalledTimes(2); }); it('should include optional reason in output display but not in llmContent', async () => { diff --git a/packages/core/src/tools/enter-plan-mode.ts b/packages/core/src/tools/enter-plan-mode.ts index 81c3f095ce..072e300ca1 100644 --- a/packages/core/src/tools/enter-plan-mode.ts +++ b/packages/core/src/tools/enter-plan-mode.ts @@ -125,16 +125,48 @@ export class EnterPlanModeInvocation extends BaseToolInvocation< this.config.setApprovalMode(ApprovalMode.PLAN); - // Ensure plans directory exists so that the agent can write the plan file. + // Ensure plans directories exist so that the agent can write plan files. // In sandboxed environments, the plans directory must exist on the host // before it can be bound/allowed in the sandbox. - const plansDir = this.config.storage.getPlansDir(); - if (!fs.existsSync(plansDir)) { - try { - fs.mkdirSync(plansDir, { recursive: true }); - } catch (e) { - // Log error but don't fail; write_file will try again later - debugLogger.error(`Failed to create plans directory: ${plansDir}`, e); + const dirsToCreate = new Set(); + + // Always ensure the default plans directory exists + try { + dirsToCreate.add(this.config.storage.getPlansDir(undefined)); + } catch { + // Ignore if default somehow throws (unlikely) + } + + // Ensure extension-specific plan directories exist + for (const ext of this.config.getExtensions()) { + if (!ext.isActive) continue; + + // Check for user-defined custom plan directory setting first. + // If not set, fallback to the default directory defined in the extension's manifest. + const customDir = + this.config.getExtensionSetting(ext.name, 'plan.directory') ?? + ext.plan?.directory; + + if (customDir) { + try { + dirsToCreate.add(this.config.storage.getPlansDir(customDir)); + } catch (e) { + debugLogger.warn( + `Invalid custom plan directory '${customDir}' for extension '${ext.name}':`, + e, + ); + } + } + } + + for (const dir of dirsToCreate) { + if (!fs.existsSync(dir)) { + try { + fs.mkdirSync(dir, { recursive: true }); + } catch (e) { + // Log error but don't fail; write_file will try again later + debugLogger.error(`Failed to create plans directory: ${dir}`, e); + } } }