MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-05-15 14:23:02 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): ensure JIT provisioning of plan directory only occurs in active plan mode

Browse Source 
Address review bot feedback to prevent EACCES errors during startup by restricting filesystem mutation and workspace context registration to when the user is actively in an interactive plan session. Non-plan tool registration now uses the safely resolved path without attempting to create the directory.
This commit is contained in:
Mahima Shanware
2026-04-07 21:00:43 +00:00
parent 073ecc1d4c
commit fd8cfad6f9
2 changed files with 62 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/core/src/config/config.test.ts
+42 -8
View File
@@ -3367,7 +3367,7 @@ describe('Plans Directory Initialization', () => {
expect(context.getDirectories()).not.toContain(plansDir);
});
it('should create plans directory and add it to workspace context when getPlansDir is called', async () => {
it('should create plans directory and add it to workspace context when getPlansDir is called and ApprovalMode.PLAN is active', async () => {
vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
const config = new Config({
...baseParams,
@@ -3375,6 +3375,7 @@ describe('Plans Directory Initialization', () => {
});
await config.initialize();
config.setApprovalMode(ApprovalMode.PLAN);
const plansDir = config.getPlansDir();
expect(fs.mkdirSync).toHaveBeenCalledWith(plansDir, {
@@ -3385,7 +3386,7 @@ describe('Plans Directory Initialization', () => {
expect(context.getDirectories()).toContain(plansDir);
});
it('should gracefully handle existing directories by relying on mkdirSync recursive: true', async () => {
it('should NOT create plans directory if ApprovalMode is not PLAN even if plan is enabled', async () => {
vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
const config = new Config({
...baseParams,
@@ -3393,6 +3394,23 @@ describe('Plans Directory Initialization', () => {
});
await config.initialize();
// Default mode is DEFAULT, not PLAN
const plansDir = config.getPlansDir();
expect(fs.mkdirSync).not.toHaveBeenCalled();
const context = config.getWorkspaceContext();
expect(context.getDirectories()).not.toContain(plansDir);
});
it('should gracefully handle existing directories by relying on mkdirSync recursive: true when in PLAN mode', async () => {
vi.spyOn(fs, 'mkdirSync').mockReturnValue(undefined);
const config = new Config({
...baseParams,
plan: true,
});
await config.initialize();
config.setApprovalMode(ApprovalMode.PLAN);
const plansDir = config.getPlansDir();
// mkdirSync should be called unconditionally
@@ -3403,7 +3421,10 @@ describe('Plans Directory Initialization', () => {
expect(context.getDirectories()).toContain(plansDir);
});
it('should throw an error if the plan directory path is blocked by an existing file (EEXIST)', async () => {
it('should log a warning if the plan directory path is blocked by an existing file (EEXIST) in PLAN mode', async () => {
const writeSpy = vi
.spyOn(process.stderr, 'write')
.mockImplementation(() => true);
vi.spyOn(fs, 'mkdirSync').mockImplementation(() => {
const err = new Error('File exists') as NodeJS.ErrnoException;
err.code = 'EEXIST';
@@ -3415,13 +3436,21 @@ describe('Plans Directory Initialization', () => {
});
await config.initialize();
config.setApprovalMode(ApprovalMode.PLAN);
config.getPlansDir();
expect(() => config.getPlansDir()).toThrow(
/Failed to initialize active plan directory.*File exists/,
expect(writeSpy).toHaveBeenCalledWith(
expect.stringMatching(
/Failed to initialize active plan directory.*File exists/,
),
);
writeSpy.mockRestore();
});
it('should throw an error if mkdirSync fails during getPlansDir (e.g. EACCES)', async () => {
it('should log a warning if mkdirSync fails during getPlansDir (e.g. EACCES) in PLAN mode', async () => {
const writeSpy = vi
.spyOn(process.stderr, 'write')
.mockImplementation(() => true);
vi.spyOn(fs, 'mkdirSync').mockImplementation(() => {
const err = new Error('Permission denied') as NodeJS.ErrnoException;
err.code = 'EACCES';
@@ -3433,10 +3462,15 @@ describe('Plans Directory Initialization', () => {
});
await config.initialize();
config.setApprovalMode(ApprovalMode.PLAN);
config.getPlansDir();
expect(() => config.getPlansDir()).toThrow(
/Failed to initialize active plan directory.*Permission denied/,
expect(writeSpy).toHaveBeenCalledWith(
expect.stringMatching(
/Failed to initialize active plan directory.*Permission denied/,
),
);
writeSpy.mockRestore();
});
it('should NOT create plans directory or add it to workspace context when plan is disabled', async () => {
packages/core/src/config/config.ts
+20 -15
View File
@@ -2269,26 +2269,31 @@ export class Config implements McpContext, AgentLoopContext {
return plansDir;
}
try {
fs.mkdirSync(plansDir, { recursive: true });
let realPlansDir = plansDir;
if (this.planEnabled && this.isPlanMode()) {
try {
const resolved = resolveToRealPath(plansDir);
if (resolved) {
realPlansDir = resolved;
fs.mkdirSync(plansDir, { recursive: true });
let realPlansDir = plansDir;
try {
const resolved = resolveToRealPath(plansDir);
if (resolved) {
realPlansDir = resolved;
}
} catch {
// Ignore failures in mock environments
}
} catch {
// Ignore failures in mock environments
this.workspaceContext.addDirectory(realPlansDir);
this.initializedPlanDirs.add(plansDir);
} catch (e: unknown) {
const errorMessage = e instanceof Error ? e.message : String(e);
process.stderr.write(
`Failed to initialize active plan directory at '${plansDir}': ${errorMessage}\n`,
);
}
this.workspaceContext.addDirectory(realPlansDir);
} else if (!this.planEnabled) {
this.initializedPlanDirs.add(plansDir);
} 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;
}