Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions evals/plan_mode.eval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,32 @@ describe('plan_mode', () => {
expect(wasToolCalled, 'Expected exit_plan_mode tool to be called').toBe(
true,
);

const toolLogs = rig.readToolLogs();
const exitPlanCall = toolLogs.find(
(log) => log.toolRequest.name === 'exit_plan_mode',
);
expect(
exitPlanCall,
'Expected to find exit_plan_mode in tool logs',
).toBeDefined();

const args = JSON.parse(exitPlanCall!.toolRequest.args);
expect(args.plan_filename, 'plan_filename should be a string').toBeTypeOf(
'string',
);
expect(args.plan_filename, 'plan_filename should end with .md').toMatch(
/\.md$/,
);
expect(
args.plan_filename,
'plan_filename should not be a path',
).not.toContain('/');
expect(
args.plan_filename,
'plan_filename should not be a path',
).not.toContain('\\');

assertModelHasOutput(result);
},
});
Expand Down Expand Up @@ -199,6 +225,30 @@ describe('plan_mode', () => {
await rig.waitForTelemetryReady();
const toolLogs = rig.readToolLogs();

const exitPlanCall = toolLogs.find(
(log) => log.toolRequest.name === 'exit_plan_mode',
);
expect(
exitPlanCall,
'Expected to find exit_plan_mode in tool logs',
).toBeDefined();

const args = JSON.parse(exitPlanCall!.toolRequest.args);
expect(args.plan_filename, 'plan_filename should be a string').toBeTypeOf(
'string',
);
expect(args.plan_filename, 'plan_filename should end with .md').toMatch(
/\.md$/,
);
expect(
args.plan_filename,
'plan_filename should not be a path',
).not.toContain('/');
expect(
args.plan_filename,
'plan_filename should not be a path',
).not.toContain('\\');

// Check if plan was written
const planWrite = toolLogs.find(
(log) =>
Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/ui/components/ExitPlanModeDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ function usePlanContent(planPath: string, config: Config): PlanContentState {
const pathError = await validatePlanPath(
planPath,
config.storage.getPlansDir(),
config.getTargetDir(),
);
if (ignore) return;
if (pathError) {
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2288,6 +2288,10 @@ export class Config implements McpContext, AgentLoopContext {
return this.policyEngine.getApprovalMode();
}

isPlanMode(): boolean {
return this.getApprovalMode() === ApprovalMode.PLAN;
}

getPolicyUpdateConfirmationRequest():
| PolicyUpdateConfirmationRequest
| undefined {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/tools/confirmation-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe('Tool Confirmation Policy Updates', () => {
getDisableLLMCorrection: () => true,
getIdeMode: () => false,
getActiveModel: () => 'test-model',
isPlanMode: () => false,
getWorkspaceContext: () => ({
isPathWithinWorkspace: () => true,
getDirectories: () => [rootDir],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,13 @@ exports[`coreTools snapshots for specific models > Model: gemini-2.5-pro > snaps
"name": "exit_plan_mode",
"parametersJsonSchema": {
"properties": {
"plan_path": {
"description": "The file path to the finalized plan (e.g., "/mock/plans/feature-x.md"). This path MUST be within the designated plans directory: /mock/plans/",
"plan_filename": {
"description": "The filename of the finalized plan (e.g., "feature-x.md"). Do not provide an absolute path.",
"type": "string",
},
},
"required": [
"plan_path",
"plan_filename",
],
"type": "object",
},
Expand Down Expand Up @@ -958,13 +958,13 @@ exports[`coreTools snapshots for specific models > Model: gemini-3-pro-preview >
"name": "exit_plan_mode",
"parametersJsonSchema": {
"properties": {
"plan_path": {
"description": "The file path to the finalized plan (e.g., "/mock/plans/feature-x.md"). This path MUST be within the designated plans directory: /mock/plans/",
"plan_filename": {
"description": "The filename of the finalized plan (e.g., "feature-x.md"). Do not provide an absolute path.",
"type": "string",
},
},
"required": [
"plan_path",
"plan_filename",
],
"type": "object",
},
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tools/definitions/base-declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export const ASK_USER_OPTION_PARAM_DESCRIPTION = 'description';

// -- exit_plan_mode --
export const EXIT_PLAN_MODE_TOOL_NAME = 'exit_plan_mode';
export const EXIT_PLAN_PARAM_PLAN_PATH = 'plan_path';
export const EXIT_PLAN_PARAM_PLAN_FILENAME = 'plan_filename';

// -- enter_plan_mode --
export const ENTER_PLAN_MODE_TOOL_NAME = 'enter_plan_mode';
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/tools/definitions/coreTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export {
ASK_USER_OPTION_PARAM_LABEL,
ASK_USER_OPTION_PARAM_DESCRIPTION,
PLAN_MODE_PARAM_REASON,
EXIT_PLAN_PARAM_PLAN_PATH,
EXIT_PLAN_PARAM_PLAN_FILENAME,
SKILL_PARAM_NAME,
} from './base-declarations.js';

Expand Down Expand Up @@ -244,10 +244,10 @@ export function getShellDefinition(
};
}

export function getExitPlanModeDefinition(plansDir: string): ToolDefinition {
export function getExitPlanModeDefinition(): ToolDefinition {
return {
base: getExitPlanModeDeclaration(plansDir),
overrides: (modelId) => getToolSet(modelId).exit_plan_mode(plansDir),
base: getExitPlanModeDeclaration(),
overrides: (modelId) => getToolSet(modelId).exit_plan_mode(),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('coreTools snapshots for specific models', () => {
{ name: 'enter_plan_mode', definition: ENTER_PLAN_MODE_DEFINITION },
{
name: 'exit_plan_mode',
definition: getExitPlanModeDefinition('/mock/plans'),
definition: getExitPlanModeDefinition(),
},
{
name: 'activate_skill',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
PARAM_DESCRIPTION,
PARAM_DIR_PATH,
SHELL_PARAM_IS_BACKGROUND,
EXIT_PLAN_PARAM_PLAN_PATH,
EXIT_PLAN_PARAM_PLAN_FILENAME,
SKILL_PARAM_NAME,
} from './base-declarations.js';

Expand Down Expand Up @@ -118,20 +118,18 @@ export function getShellDeclaration(
/**
* Returns the FunctionDeclaration for exiting plan mode.
*/
export function getExitPlanModeDeclaration(
plansDir: string,
): FunctionDeclaration {
export function getExitPlanModeDeclaration(): FunctionDeclaration {
return {
name: EXIT_PLAN_MODE_TOOL_NAME,
description:
'Finalizes the planning phase and transitions to implementation by presenting the plan for user approval. This tool MUST be used to exit Plan Mode before any source code edits can be performed. Call this whenever a plan is ready or the user requests implementation.',
parametersJsonSchema: {
type: 'object',
required: [EXIT_PLAN_PARAM_PLAN_PATH],
required: [EXIT_PLAN_PARAM_PLAN_FILENAME],
properties: {
[EXIT_PLAN_PARAM_PLAN_PATH]: {
[EXIT_PLAN_PARAM_PLAN_FILENAME]: {
type: 'string',
description: `The file path to the finalized plan (e.g., "${plansDir}/feature-x.md"). This path MUST be within the designated plans directory: ${plansDir}/`,
description: `The filename of the finalized plan (e.g., "feature-x.md"). Do not provide an absolute path.`,
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,6 @@ The agent did not use the todo list because this task could be completed by a ti
},
},

exit_plan_mode: (plansDir) => getExitPlanModeDeclaration(plansDir),
exit_plan_mode: () => getExitPlanModeDeclaration(),
activate_skill: (skillNames) => getActivateSkillDeclaration(skillNames),
};
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,6 @@ The agent did not use the todo list because this task could be completed by a ti
},
},

exit_plan_mode: (plansDir) => getExitPlanModeDeclaration(plansDir),
exit_plan_mode: () => getExitPlanModeDeclaration(),
activate_skill: (skillNames) => getActivateSkillDeclaration(skillNames),
};
2 changes: 1 addition & 1 deletion packages/core/src/tools/definitions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,6 @@ export interface CoreToolSet {
get_internal_docs: FunctionDeclaration;
ask_user: FunctionDeclaration;
enter_plan_mode: FunctionDeclaration;
exit_plan_mode: (plansDir: string) => FunctionDeclaration;
exit_plan_mode: () => FunctionDeclaration;
activate_skill: (skillNames: string[]) => FunctionDeclaration;
}
40 changes: 40 additions & 0 deletions packages/core/src/tools/edit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,10 @@ describe('EditTool', () => {
isInteractive: () => false,
getDisableLLMCorrection: vi.fn(() => true),
getExperiments: () => {},
isPlanMode: vi.fn(() => false),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
getPlansDir: vi.fn().mockReturnValue('/tmp/plans'),
},
isPathAllowed(this: Config, absolutePath: string): boolean {
const workspaceContext = this.getWorkspaceContext();
Expand Down Expand Up @@ -1299,4 +1301,42 @@ function doIt() {
);
});
});

describe('plan mode', () => {
it('should allow edits to plans directory when isPlanMode is true', async () => {
const mockProjectTempDir = path.join(tempDir, 'project');
fs.mkdirSync(mockProjectTempDir);
vi.mocked(mockConfig.storage.getProjectTempDir).mockReturnValue(
mockProjectTempDir,
);

const plansDir = path.join(mockProjectTempDir, 'plans');
fs.mkdirSync(plansDir);

vi.mocked(mockConfig.isPlanMode).mockReturnValue(true);
vi.mocked(mockConfig.storage.getPlansDir).mockReturnValue(plansDir);

const filePath = path.join(rootDir, 'test-file.txt');
const planFilePath = path.join(plansDir, 'test-file.txt');
const initialContent = 'some initial content';
fs.writeFileSync(planFilePath, initialContent, 'utf8');

const params: EditToolParams = {
file_path: filePath,
instruction: 'Replace initial with new',
old_string: 'initial',
new_string: 'new',
};

const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);

expect(result.llmContent).toMatch(/Successfully modified file/);

// Verify plan file is written with new content
expect(fs.readFileSync(planFilePath, 'utf8')).toBe('some new content');

fs.rmSync(plansDir, { recursive: true, force: true });
});
});
});
8 changes: 7 additions & 1 deletion packages/core/src/tools/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,13 @@ class EditToolInvocation
true,
() => this.config.getApprovalMode(),
);
if (!path.isAbsolute(this.params.file_path)) {
if (this.config.isPlanMode()) {
const safeFilename = path.basename(this.params.file_path);
this.resolvedPath = path.join(
this.config.storage.getPlansDir(),
safeFilename,
);
} else if (!path.isAbsolute(this.params.file_path)) {
const result = correctPath(this.params.file_path, this.config);
if (result.success) {
this.resolvedPath = result.correctedPath;
Expand Down
Loading
Loading