mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-15 14:23:02 -07:00
feat(core): dynamic MRU plan directory resolution and lazy initialization
Introduces active extension context tracking in config to support dynamic switching of plan directories. Resolves circular dependency in storage by deferring plan directory creation until on-demand use, preventing ENOENT errors on non-existent paths.
This commit is contained in:
@@ -3345,11 +3345,19 @@ describe('Plans Directory Initialization', () => {
|
||||
|
||||
afterEach(() => {
|
||||
vi.mocked(fs.promises.mkdir).mockRestore();
|
||||
vi.mocked(fs.promises.access).mockRestore?.();
|
||||
vi.mocked(fs.mkdirSync).mockRestore?.();
|
||||
vi.mocked(fs.existsSync).mockReturnValue(true); // Reset to default mock behavior
|
||||
});
|
||||
|
||||
it('should add plans directory to workspace context if it exists', async () => {
|
||||
vi.spyOn(fs.promises, 'access').mockResolvedValue(undefined);
|
||||
it('should not eagerly create plans directory during initialization', async () => {
|
||||
let planDirExists = false;
|
||||
vi.spyOn(fs, 'existsSync').mockImplementation((path) =>
|
||||
String(path).includes('plans') ? planDirExists : true,
|
||||
);
|
||||
vi.spyOn(fs, 'mkdirSync').mockImplementation((path) => {
|
||||
if (String(path).includes('plans')) planDirExists = true;
|
||||
return undefined;
|
||||
});
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: true,
|
||||
@@ -3357,18 +3365,66 @@ describe('Plans Directory Initialization', () => {
|
||||
|
||||
await config.initialize();
|
||||
|
||||
const plansDir = config.storage.getPlansDir();
|
||||
// Should NOT create the directory eagerly
|
||||
expect(fs.promises.mkdir).not.toHaveBeenCalled();
|
||||
// Should check if it exists
|
||||
expect(fs.promises.access).toHaveBeenCalledWith(plansDir);
|
||||
expect(fs.mkdirSync).not.toHaveBeenCalled();
|
||||
|
||||
// Using storage directly to avoid triggering creation
|
||||
const plansDir = config.storage.getPlansDir();
|
||||
const context = config.getWorkspaceContext();
|
||||
expect(context.getDirectories()).not.toContain(plansDir);
|
||||
});
|
||||
|
||||
it('should create plans directory and add it to workspace context when getPlansDir is called', async () => {
|
||||
let planDirExists = false;
|
||||
vi.spyOn(fs, 'existsSync').mockImplementation((path) =>
|
||||
String(path).includes('plans') ? planDirExists : true,
|
||||
);
|
||||
vi.spyOn(fs, 'mkdirSync').mockImplementation((path) => {
|
||||
if (String(path).includes('plans')) planDirExists = true;
|
||||
return undefined;
|
||||
});
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: true,
|
||||
});
|
||||
|
||||
await config.initialize();
|
||||
const plansDir = config.getPlansDir();
|
||||
|
||||
expect(fs.mkdirSync).toHaveBeenCalledWith(plansDir, {
|
||||
recursive: true,
|
||||
});
|
||||
|
||||
const context = config.getWorkspaceContext();
|
||||
expect(context.getDirectories()).toContain(plansDir);
|
||||
});
|
||||
|
||||
it('should NOT add plans directory to workspace context if it does not exist', async () => {
|
||||
vi.spyOn(fs.promises, 'access').mockRejectedValue({ code: 'ENOENT' });
|
||||
it('should add plans directory to workspace context even if it already exists', async () => {
|
||||
vi.spyOn(fs, 'existsSync').mockReturnValue(true);
|
||||
vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: true,
|
||||
});
|
||||
|
||||
await config.initialize();
|
||||
const plansDir = config.getPlansDir();
|
||||
|
||||
// Should NOT try to create it if it exists
|
||||
expect(fs.mkdirSync).not.toHaveBeenCalled();
|
||||
|
||||
// But MUST still register it
|
||||
const context = config.getWorkspaceContext();
|
||||
expect(context.getDirectories()).toContain(plansDir);
|
||||
});
|
||||
|
||||
it('should throw an error if mkdirSync fails during getPlansDir', async () => {
|
||||
vi.spyOn(fs, 'existsSync').mockImplementation(
|
||||
(path) => !String(path).includes('plans'),
|
||||
);
|
||||
vi.spyOn(fs, 'mkdirSync').mockImplementation(() => {
|
||||
throw { code: 'EACCES', message: 'Permission denied' };
|
||||
});
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: true,
|
||||
@@ -3376,15 +3432,16 @@ describe('Plans Directory Initialization', () => {
|
||||
|
||||
await config.initialize();
|
||||
|
||||
const plansDir = config.storage.getPlansDir();
|
||||
expect(fs.promises.mkdir).not.toHaveBeenCalled();
|
||||
expect(fs.promises.access).toHaveBeenCalledWith(plansDir);
|
||||
|
||||
const context = config.getWorkspaceContext();
|
||||
expect(context.getDirectories()).not.toContain(plansDir);
|
||||
expect(() => config.getPlansDir()).toThrow(
|
||||
/Failed to initialize active plan directory/,
|
||||
);
|
||||
});
|
||||
|
||||
it('should NOT create plans directory or add it to workspace context when plan is disabled', async () => {
|
||||
vi.spyOn(fs, 'existsSync').mockImplementation(
|
||||
(path) => !String(path).includes('plans'),
|
||||
);
|
||||
vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
plan: false,
|
||||
@@ -3393,9 +3450,10 @@ describe('Plans Directory Initialization', () => {
|
||||
await config.initialize();
|
||||
|
||||
const plansDir = config.storage.getPlansDir();
|
||||
expect(fs.promises.mkdir).not.toHaveBeenCalledWith(plansDir, {
|
||||
recursive: true,
|
||||
});
|
||||
expect(fs.mkdirSync).not.toHaveBeenCalled();
|
||||
expect(config.getWorkspaceContext().getDirectories()).not.toContain(
|
||||
plansDir,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -711,6 +711,7 @@ export interface ConfigParameters {
|
||||
plan?: boolean;
|
||||
tracker?: boolean;
|
||||
planSettings?: PlanSettings;
|
||||
extensionPlanDirs?: Record<string, string>;
|
||||
worktreeSettings?: WorktreeSettings;
|
||||
modelSteering?: boolean;
|
||||
onModelChange?: (model: string) => void;
|
||||
@@ -774,6 +775,8 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
private readonly extensionsEnabled: boolean;
|
||||
private mcpServers: Record<string, MCPServerConfig> | undefined;
|
||||
private readonly mcpEnablementCallbacks?: McpEnablementCallbacks;
|
||||
private activeExtensionContext?: string;
|
||||
private readonly extensionPlanDirs: Record<string, string>;
|
||||
private userMemory: string | HierarchicalMemory;
|
||||
private geminiMdFileCount: number;
|
||||
private geminiMdFilePaths: string[];
|
||||
@@ -1038,6 +1041,7 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
this.mcpServerCommand = params.mcpServerCommand;
|
||||
this.mcpServers = params.mcpServers;
|
||||
this.mcpEnablementCallbacks = params.mcpEnablementCallbacks;
|
||||
this.extensionPlanDirs = params.extensionPlanDirs ?? {};
|
||||
this.mcpEnabled = params.mcpEnabled ?? true;
|
||||
this.extensionsEnabled = params.extensionsEnabled ?? true;
|
||||
this.allowedMcpServers = params.allowedMcpServers ?? [];
|
||||
@@ -1409,20 +1413,6 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
this.workspaceContext.addDirectory(dir);
|
||||
}
|
||||
|
||||
// Add plans directory to workspace context for plan file storage
|
||||
if (this.planEnabled) {
|
||||
const plansDir = this.storage.getPlansDir();
|
||||
try {
|
||||
await fs.promises.access(plansDir);
|
||||
this.workspaceContext.addDirectory(plansDir);
|
||||
} catch {
|
||||
// Directory does not exist yet, so we don't add it to the workspace context.
|
||||
// It will be created when the first plan is written. Since custom plan
|
||||
// directories must be within the project root, they are automatically
|
||||
// covered by the project-wide file discovery once created.
|
||||
}
|
||||
}
|
||||
|
||||
// Initialize centralized FileDiscoveryService
|
||||
const discoverToolsHandle = startupProfiler.start('discover_tools');
|
||||
this.getFileService();
|
||||
@@ -2252,6 +2242,51 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
return this.mcpEnabled;
|
||||
}
|
||||
|
||||
getActiveExtensionContext(): string | undefined {
|
||||
return this.activeExtensionContext;
|
||||
}
|
||||
|
||||
setActiveExtensionContext(context: string | undefined): void {
|
||||
this.activeExtensionContext = context;
|
||||
}
|
||||
|
||||
hasExtensionPlanDir(name: string): boolean {
|
||||
return !!this.extensionPlanDirs[name];
|
||||
}
|
||||
|
||||
getActiveExtensionPlanDir(): string | undefined {
|
||||
if (this.activeExtensionContext) {
|
||||
return this.extensionPlanDirs[this.activeExtensionContext];
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
getPlansDir(): string {
|
||||
const plansDir = this.storage.getPlansDir(this.getActiveExtensionPlanDir());
|
||||
try {
|
||||
if (!fs.existsSync(plansDir)) {
|
||||
fs.mkdirSync(plansDir, { recursive: true });
|
||||
}
|
||||
|
||||
let realPlansDir = plansDir;
|
||||
try {
|
||||
const resolved = resolveToRealPath(plansDir);
|
||||
if (resolved) {
|
||||
realPlansDir = resolved;
|
||||
}
|
||||
} catch {
|
||||
// Ignore failures in mock environments
|
||||
}
|
||||
this.workspaceContext.addDirectory(realPlansDir);
|
||||
} catch (e: unknown) {
|
||||
const errorMessage = e instanceof Error ? e.message : String(e);
|
||||
throw new Error(
|
||||
`Failed to initialize active plan directory at '${plansDir}': ${errorMessage}`,
|
||||
);
|
||||
}
|
||||
return plansDir;
|
||||
}
|
||||
|
||||
getMcpEnablementCallbacks(): McpEnablementCallbacks | undefined {
|
||||
return this.mcpEnablementCallbacks;
|
||||
}
|
||||
|
||||
@@ -320,18 +320,31 @@ export class Storage {
|
||||
return path.join(this.getProjectTempDir(), 'tracker');
|
||||
}
|
||||
|
||||
getPlansDir(): string {
|
||||
if (this.customPlansDir) {
|
||||
const resolvedPath = path.resolve(
|
||||
this.getProjectRoot(),
|
||||
this.customPlansDir,
|
||||
);
|
||||
getPlansDir(extensionPlanDir?: string): string {
|
||||
const customDir = extensionPlanDir || this.customPlansDir;
|
||||
if (customDir) {
|
||||
const resolvedPath = path.resolve(this.getProjectRoot(), customDir);
|
||||
const realProjectRoot = resolveToRealPath(this.getProjectRoot());
|
||||
const realResolvedPath = resolveToRealPath(resolvedPath);
|
||||
let realResolvedPath = resolvedPath;
|
||||
|
||||
try {
|
||||
realResolvedPath = resolveToRealPath(resolvedPath);
|
||||
} catch (e: unknown) {
|
||||
if (
|
||||
!(
|
||||
e &&
|
||||
typeof e === 'object' &&
|
||||
'code' in e &&
|
||||
(e.code === 'ENOENT' || e.code === 'EISDIR')
|
||||
)
|
||||
) {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
if (!isSubpath(realProjectRoot, realResolvedPath)) {
|
||||
throw new Error(
|
||||
`Custom plans directory '${this.customPlansDir}' resolves to '${realResolvedPath}', which is outside the project root '${realProjectRoot}'.`,
|
||||
`Custom plans directory '${customDir}' resolves to '${realResolvedPath}', which is outside the project root '${realProjectRoot}'.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user