Skip to content

Commit 6d0d13d

Browse files
Adib234jerop
andauthored
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>
1 parent 7bf1287 commit 6d0d13d

2 files changed

Lines changed: 41 additions & 14 deletions

File tree

packages/cli/src/config/config.test.ts

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,6 +1344,36 @@ describe('Approval mode tool exclusion logic', () => {
13441344
'Invalid approval mode: invalid_mode. Valid values are: yolo, auto_edit, plan, default',
13451345
);
13461346
});
1347+
1348+
it('should fall back to default approval mode if plan mode is requested but not enabled', async () => {
1349+
process.argv = ['node', 'script.js'];
1350+
const settings = createTestMergedSettings({
1351+
general: {
1352+
defaultApprovalMode: 'plan',
1353+
},
1354+
experimental: {
1355+
plan: false,
1356+
},
1357+
});
1358+
const argv = await parseArguments(settings);
1359+
const config = await loadCliConfig(settings, 'test-session', argv);
1360+
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
1361+
});
1362+
1363+
it('should allow plan approval mode if experimental plan is enabled', async () => {
1364+
process.argv = ['node', 'script.js'];
1365+
const settings = createTestMergedSettings({
1366+
general: {
1367+
defaultApprovalMode: 'plan',
1368+
},
1369+
experimental: {
1370+
plan: true,
1371+
},
1372+
});
1373+
const argv = await parseArguments(settings);
1374+
const config = await loadCliConfig(settings, 'test-session', argv);
1375+
expect(config.getApprovalMode()).toBe(ApprovalMode.PLAN);
1376+
});
13471377
});
13481378

13491379
describe('loadCliConfig with allowed-mcp-server-names', () => {
@@ -2556,19 +2586,17 @@ describe('loadCliConfig approval mode', () => {
25562586
},
25572587
});
25582588

2559-
await expect(loadCliConfig(settings, 'test-session', argv)).rejects.toThrow(
2560-
'Approval mode "plan" is only available when experimental.plan is enabled.',
2561-
);
2589+
const config = await loadCliConfig(settings, 'test-session', argv);
2590+
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
25622591
});
25632592

25642593
it('should throw error when --approval-mode=plan is used but experimental.plan setting is missing', async () => {
25652594
process.argv = ['node', 'script.js', '--approval-mode', 'plan'];
25662595
const argv = await parseArguments(createTestMergedSettings());
25672596
const settings = createTestMergedSettings({});
25682597

2569-
await expect(loadCliConfig(settings, 'test-session', argv)).rejects.toThrow(
2570-
'Approval mode "plan" is only available when experimental.plan is enabled.',
2571-
);
2598+
const config = await loadCliConfig(settings, 'test-session', argv);
2599+
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
25722600
});
25732601

25742602
// --- Untrusted Folder Scenarios ---
@@ -2678,11 +2706,8 @@ describe('loadCliConfig approval mode', () => {
26782706
experimental: { plan: false },
26792707
});
26802708
const argv = await parseArguments(settings);
2681-
await expect(
2682-
loadCliConfig(settings, 'test-session', argv),
2683-
).rejects.toThrow(
2684-
'Approval mode "plan" is only available when experimental.plan is enabled.',
2685-
);
2709+
const config = await loadCliConfig(settings, 'test-session', argv);
2710+
expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT);
26862711
});
26872712
});
26882713
});

packages/cli/src/config/config.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,11 +555,13 @@ export async function loadCliConfig(
555555
break;
556556
case 'plan':
557557
if (!(settings.experimental?.plan ?? false)) {
558-
throw new Error(
559-
'Approval mode "plan" is only available when experimental.plan is enabled.',
558+
debugLogger.warn(
559+
'Approval mode "plan" is only available when experimental.plan is enabled. Falling back to "default".',
560560
);
561+
approvalMode = ApprovalMode.DEFAULT;
562+
} else {
563+
approvalMode = ApprovalMode.PLAN;
561564
}
562-
approvalMode = ApprovalMode.PLAN;
563565
break;
564566
case 'default':
565567
approvalMode = ApprovalMode.DEFAULT;

0 commit comments

Comments
 (0)