feat: Implement Plan Mode for Safe Code Planning (#658)

This commit is contained in:
tanzhenxin
2025-09-24 14:26:17 +08:00
committed by GitHub
parent 8379bc4d81
commit 4e7a7e2656
43 changed files with 2895 additions and 281 deletions

View File

@@ -269,7 +269,7 @@ describe('Configuration Integration Tests', () => {
parseArguments = parseArgs;
});
it('should parse --approval-mode=auto_edit correctly through the full argument parsing flow', async () => {
it('should parse --approval-mode=auto-edit correctly through the full argument parsing flow', async () => {
const originalArgv = process.argv;
try {
@@ -277,7 +277,7 @@ describe('Configuration Integration Tests', () => {
'node',
'script.js',
'--approval-mode',
'auto_edit',
'auto-edit',
'-p',
'test',
];
@@ -285,7 +285,30 @@ describe('Configuration Integration Tests', () => {
const argv = await parseArguments({} as Settings);
// Verify that the argument was parsed correctly
expect(argv.approvalMode).toBe('auto_edit');
expect(argv.approvalMode).toBe('auto-edit');
expect(argv.prompt).toBe('test');
expect(argv.yolo).toBe(false);
} finally {
process.argv = originalArgv;
}
});
it('should parse --approval-mode=plan correctly through the full argument parsing flow', async () => {
const originalArgv = process.argv;
try {
process.argv = [
'node',
'script.js',
'--approval-mode',
'plan',
'-p',
'test',
];
const argv = await parseArguments({} as Settings);
expect(argv.approvalMode).toBe('plan');
expect(argv.prompt).toBe('test');
expect(argv.yolo).toBe(false);
} finally {

View File

@@ -262,9 +262,9 @@ describe('parseArguments', () => {
});
it('should allow --approval-mode without --yolo', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit'];
process.argv = ['node', 'script.js', '--approval-mode', 'auto-edit'];
const argv = await parseArguments({} as Settings);
expect(argv.approvalMode).toBe('auto_edit');
expect(argv.approvalMode).toBe('auto-edit');
expect(argv.yolo).toBe(false);
});
@@ -1087,6 +1087,32 @@ describe('Approval mode tool exclusion logic', () => {
expect(excludedTools).toContain(WriteFileTool.Name);
});
it('should exclude all interactive tools in non-interactive mode with plan approval mode', async () => {
process.argv = [
'node',
'script.js',
'--approval-mode',
'plan',
'-p',
'test',
];
const argv = await parseArguments({} as Settings);
const settings: Settings = {};
const extensions: Extension[] = [];
const config = await loadCliConfig(
settings,
extensions,
'test-session',
argv,
);
const excludedTools = config.getExcludeTools();
expect(excludedTools).toContain(ShellTool.Name);
expect(excludedTools).toContain(EditTool.Name);
expect(excludedTools).toContain(WriteFileTool.Name);
});
it('should exclude all interactive tools in non-interactive mode with explicit default approval mode', async () => {
process.argv = [
'node',
@@ -1113,12 +1139,12 @@ describe('Approval mode tool exclusion logic', () => {
expect(excludedTools).toContain(WriteFileTool.Name);
});
it('should exclude only shell tools in non-interactive mode with auto_edit approval mode', async () => {
it('should exclude only shell tools in non-interactive mode with auto-edit approval mode', async () => {
process.argv = [
'node',
'script.js',
'--approval-mode',
'auto_edit',
'auto-edit',
'-p',
'test',
];
@@ -1189,8 +1215,9 @@ describe('Approval mode tool exclusion logic', () => {
const testCases = [
{ args: ['node', 'script.js'] }, // default
{ args: ['node', 'script.js', '--approval-mode', 'plan'] },
{ args: ['node', 'script.js', '--approval-mode', 'default'] },
{ args: ['node', 'script.js', '--approval-mode', 'auto_edit'] },
{ args: ['node', 'script.js', '--approval-mode', 'auto-edit'] },
{ args: ['node', 'script.js', '--approval-mode', 'yolo'] },
{ args: ['node', 'script.js', '--yolo'] },
];
@@ -1215,12 +1242,12 @@ describe('Approval mode tool exclusion logic', () => {
}
});
it('should merge approval mode exclusions with settings exclusions in auto_edit mode', async () => {
it('should merge approval mode exclusions with settings exclusions in auto-edit mode', async () => {
process.argv = [
'node',
'script.js',
'--approval-mode',
'auto_edit',
'auto-edit',
'-p',
'test',
];
@@ -1238,8 +1265,8 @@ describe('Approval mode tool exclusion logic', () => {
const excludedTools = config.getExcludeTools();
expect(excludedTools).toContain('custom_tool'); // From settings
expect(excludedTools).toContain(ShellTool.Name); // From approval mode
expect(excludedTools).not.toContain(EditTool.Name); // Should be allowed in auto_edit
expect(excludedTools).not.toContain(WriteFileTool.Name); // Should be allowed in auto_edit
expect(excludedTools).not.toContain(EditTool.Name); // Should be allowed in auto-edit
expect(excludedTools).not.toContain(WriteFileTool.Name); // Should be allowed in auto-edit
});
it('should throw an error for invalid approval mode values in loadCliConfig', async () => {
@@ -1262,7 +1289,7 @@ describe('Approval mode tool exclusion logic', () => {
invalidArgv as CliArgs,
),
).rejects.toThrow(
'Invalid approval mode: invalid_mode. Valid values are: yolo, auto_edit, default',
'Invalid approval mode: invalid_mode. Valid values are: plan, default, auto-edit, yolo',
);
});
});
@@ -1929,6 +1956,13 @@ describe('loadCliConfig approval mode', () => {
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
});
it('should set PLAN approval mode when --approval-mode=plan', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'plan'];
const argv = await parseArguments({} as Settings);
const config = await loadCliConfig({}, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.PLAN);
});
it('should set YOLO approval mode when --yolo flag is used', async () => {
process.argv = ['node', 'script.js', '--yolo'];
const argv = await parseArguments({} as Settings);
@@ -1950,8 +1984,8 @@ describe('loadCliConfig approval mode', () => {
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
});
it('should set AUTO_EDIT approval mode when --approval-mode=auto_edit', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit'];
it('should set AUTO_EDIT approval mode when --approval-mode=auto-edit', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'auto-edit'];
const argv = await parseArguments({} as Settings);
const config = await loadCliConfig({}, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.AUTO_EDIT);
@@ -1964,6 +1998,33 @@ describe('loadCliConfig approval mode', () => {
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.YOLO);
});
it('should use approval mode from settings when CLI flags are not provided', async () => {
process.argv = ['node', 'script.js'];
const argv = await parseArguments({} as Settings);
const settings: Settings = { approvalMode: 'plan' };
const config = await loadCliConfig(settings, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.PLAN);
});
it('should normalize approval mode values from settings', async () => {
process.argv = ['node', 'script.js'];
const argv = await parseArguments({} as Settings);
const settings: Settings = { approvalMode: 'AutoEdit' };
const config = await loadCliConfig(settings, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.AUTO_EDIT);
});
it('should throw when approval mode in settings is invalid', async () => {
process.argv = ['node', 'script.js'];
const argv = await parseArguments({} as Settings);
const settings: Settings = { approvalMode: 'invalid_mode' };
await expect(
loadCliConfig(settings, [], 'test-session', argv),
).rejects.toThrow(
'Invalid approval mode: invalid_mode. Valid values are: plan, default, auto-edit, yolo',
);
});
it('should prioritize --approval-mode over --yolo when both would be valid (but validation prevents this)', async () => {
// Note: This test documents the intended behavior, but in practice the validation
// prevents both flags from being used together
@@ -1995,8 +2056,8 @@ describe('loadCliConfig approval mode', () => {
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
});
it('should override --approval-mode=auto_edit to DEFAULT', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit'];
it('should override --approval-mode=auto-edit to DEFAULT', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'auto-edit'];
const argv = await parseArguments({} as Settings);
const config = await loadCliConfig({}, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
@@ -2015,6 +2076,13 @@ describe('loadCliConfig approval mode', () => {
const config = await loadCliConfig({}, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
});
it('should allow PLAN approval mode in untrusted folders', async () => {
process.argv = ['node', 'script.js', '--approval-mode', 'plan'];
const argv = await parseArguments({} as Settings);
const config = await loadCliConfig({}, [], 'test-session', argv);
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.PLAN);
});
});
});

View File

@@ -52,6 +52,39 @@ const logger = {
error: (...args: any[]) => console.error('[ERROR]', ...args),
};
const VALID_APPROVAL_MODE_VALUES = [
'plan',
'default',
'auto-edit',
'yolo',
] as const;
function formatApprovalModeError(value: string): Error {
return new Error(
`Invalid approval mode: ${value}. Valid values are: ${VALID_APPROVAL_MODE_VALUES.join(
', ',
)}`,
);
}
function parseApprovalModeValue(value: string): ApprovalMode {
const normalized = value.trim().toLowerCase();
switch (normalized) {
case 'plan':
return ApprovalMode.PLAN;
case 'default':
return ApprovalMode.DEFAULT;
case 'yolo':
return ApprovalMode.YOLO;
case 'auto_edit':
case 'autoedit':
case 'auto-edit':
return ApprovalMode.AUTO_EDIT;
default:
throw formatApprovalModeError(value);
}
}
export interface CliArgs {
model: string | undefined;
sandbox: boolean | string | undefined;
@@ -147,9 +180,9 @@ export async function parseArguments(settings: Settings): Promise<CliArgs> {
})
.option('approval-mode', {
type: 'string',
choices: ['default', 'auto_edit', 'yolo'],
choices: ['plan', 'default', 'auto-edit', 'yolo'],
description:
'Set the approval mode: default (prompt for approval), auto_edit (auto-approve edit tools), yolo (auto-approve all tools)',
'Set the approval mode: plan (plan only), default (prompt for approval), auto-edit (auto-approve edit tools), yolo (auto-approve all tools)',
})
.option('telemetry', {
type: 'boolean',
@@ -438,30 +471,21 @@ export async function loadCliConfig(
// Determine approval mode with backward compatibility
let approvalMode: ApprovalMode;
if (argv.approvalMode) {
// New --approval-mode flag takes precedence
switch (argv.approvalMode) {
case 'yolo':
approvalMode = ApprovalMode.YOLO;
break;
case 'auto_edit':
approvalMode = ApprovalMode.AUTO_EDIT;
break;
case 'default':
approvalMode = ApprovalMode.DEFAULT;
break;
default:
throw new Error(
`Invalid approval mode: ${argv.approvalMode}. Valid values are: yolo, auto_edit, default`,
);
}
approvalMode = parseApprovalModeValue(argv.approvalMode);
} else if (argv.yolo) {
approvalMode = ApprovalMode.YOLO;
} else if (settings.approvalMode) {
approvalMode = parseApprovalModeValue(settings.approvalMode);
} else {
// Fallback to legacy --yolo flag behavior
approvalMode =
argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT;
approvalMode = ApprovalMode.DEFAULT;
}
// Force approval mode to default if the folder is not trusted.
if (!trustedFolder && approvalMode !== ApprovalMode.DEFAULT) {
if (
!trustedFolder &&
approvalMode !== ApprovalMode.DEFAULT &&
approvalMode !== ApprovalMode.PLAN
) {
logger.warn(
`Approval mode overridden to "default" because the current folder is not trusted.`,
);
@@ -474,6 +498,7 @@ export async function loadCliConfig(
const extraExcludes: string[] = [];
if (!interactive && !argv.experimentalAcp) {
switch (approvalMode) {
case ApprovalMode.PLAN:
case ApprovalMode.DEFAULT:
// In default non-interactive mode, all tools that require approval are excluded.
extraExcludes.push(ShellTool.Name, EditTool.Name, WriteFileTool.Name);

View File

@@ -892,6 +892,16 @@ export const SETTINGS_SCHEMA = {
description: 'Disable all loop detection checks (streaming and LLM).',
showInDialog: true,
},
approvalMode: {
type: 'string',
label: 'Default Approval Mode',
category: 'General',
requiresRestart: false,
default: 'default',
description:
'Default approval mode for tool usage. Valid values: plan, default, auto-edit, yolo.',
showInDialog: true,
},
enableWelcomeBack: {
type: 'boolean',
label: 'Enable Welcome Back',