fix(plan): allow safe fallback when experiment setting for plan is not enabled but approval mode at startup is plan (#19439)

Co-authored-by: Jerop Kipruto <jerop@google.com>
This commit is contained in:
Adib234
2026-02-18 14:54:04 -05:00
committed by GitHub
parent e32111e609
commit 9255e69abb
2 changed files with 41 additions and 14 deletions

View File

@@ -1344,6 +1344,36 @@ describe('Approval mode tool exclusion logic', () => {
'Invalid approval mode: invalid_mode. Valid values are: yolo, auto_edit, plan, default',
);
});
it('should fall back to default approval mode if plan mode is requested but not enabled', async () => {
process.argv = ['node', 'script.js'];
const settings = createTestMergedSettings({
general: {
defaultApprovalMode: 'plan',
},
experimental: {
plan: false,
},
});
const argv = await parseArguments(settings);
const config = await loadCliConfig(settings, 'test-session', argv);
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
});
it('should allow plan approval mode if experimental plan is enabled', async () => {
process.argv = ['node', 'script.js'];
const settings = createTestMergedSettings({
general: {
defaultApprovalMode: 'plan',
},
experimental: {
plan: true,
},
});
const argv = await parseArguments(settings);
const config = await loadCliConfig(settings, 'test-session', argv);
expect(config.getApprovalMode()).toBe(ApprovalMode.PLAN);
});
});
describe('loadCliConfig with allowed-mcp-server-names', () => {
@@ -2556,9 +2586,8 @@ describe('loadCliConfig approval mode', () => {
},
});
await expect(loadCliConfig(settings, 'test-session', argv)).rejects.toThrow(
'Approval mode "plan" is only available when experimental.plan is enabled.',
);
const config = await loadCliConfig(settings, 'test-session', argv);
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
});
it('should throw error when --approval-mode=plan is used but experimental.plan setting is missing', async () => {
@@ -2566,9 +2595,8 @@ describe('loadCliConfig approval mode', () => {
const argv = await parseArguments(createTestMergedSettings());
const settings = createTestMergedSettings({});
await expect(loadCliConfig(settings, 'test-session', argv)).rejects.toThrow(
'Approval mode "plan" is only available when experimental.plan is enabled.',
);
const config = await loadCliConfig(settings, 'test-session', argv);
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
});
// --- Untrusted Folder Scenarios ---
@@ -2678,11 +2706,8 @@ describe('loadCliConfig approval mode', () => {
experimental: { plan: false },
});
const argv = await parseArguments(settings);
await expect(
loadCliConfig(settings, 'test-session', argv),
).rejects.toThrow(
'Approval mode "plan" is only available when experimental.plan is enabled.',
);
const config = await loadCliConfig(settings, 'test-session', argv);
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
});
});
});

View File

@@ -555,11 +555,13 @@ export async function loadCliConfig(
break;
case 'plan':
if (!(settings.experimental?.plan ?? false)) {
throw new Error(
'Approval mode "plan" is only available when experimental.plan is enabled.',
debugLogger.warn(
'Approval mode "plan" is only available when experimental.plan is enabled. Falling back to "default".',
);
approvalMode = ApprovalMode.DEFAULT;
} else {
approvalMode = ApprovalMode.PLAN;
}
approvalMode = ApprovalMode.PLAN;
break;
case 'default':
approvalMode = ApprovalMode.DEFAULT;