Skip to content

Commit b5f40c6

Browse files
backnotpropruomengz
authored andcommitted
fix(core): handle policy ALLOW for exit_plan_mode (#21802)
1 parent e7a2d33 commit b5f40c6

File tree

2 files changed

+30
-2
lines changed

2 files changed

+30
-2
lines changed

packages/core/src/tools/exit-plan-mode.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,26 @@ Ask the user for specific feedback on how to improve the plan.`,
339339
});
340340
});
341341

342+
describe('execute when shouldConfirmExecute is never called', () => {
343+
it('should approve with DEFAULT mode when approvalPayload is null (policy ALLOW skips confirmation)', async () => {
344+
const planRelativePath = createPlanFile('test.md', '# Content');
345+
const invocation = tool.build({ plan_path: planRelativePath });
346+
347+
// Simulate the scheduler's policy ALLOW path: execute() is called
348+
// directly without ever calling shouldConfirmExecute(), leaving
349+
// approvalPayload null.
350+
const result = await invocation.execute(new AbortController().signal);
351+
const expectedPath = path.join(mockPlansDir, 'test.md');
352+
353+
expect(result.llmContent).toContain('Plan approved');
354+
expect(result.returnDisplay).toContain('Plan approved');
355+
expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
356+
ApprovalMode.DEFAULT,
357+
);
358+
expect(mockConfig.setApprovedPlanPath).toHaveBeenCalledWith(expectedPath);
359+
});
360+
});
361+
342362
describe('getApprovalModeDescription (internal)', () => {
343363
it('should handle all valid approval modes', async () => {
344364
const planRelativePath = createPlanFile('test.md', '# Content');

packages/core/src/tools/exit-plan-mode.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,16 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
203203
};
204204
}
205205

206-
const payload = this.approvalPayload;
207-
if (payload?.approved) {
206+
// When a user policy grants `allow` for exit_plan_mode, the scheduler
207+
// skips the confirmation phase entirely and shouldConfirmExecute is never
208+
// called, leaving approvalPayload null. Treat that as an approval with
209+
// the default mode — consistent with the ALLOW branch inside
210+
// shouldConfirmExecute.
211+
const payload = this.approvalPayload ?? {
212+
approved: true,
213+
approvalMode: ApprovalMode.DEFAULT,
214+
};
215+
if (payload.approved) {
208216
const newMode = payload.approvalMode ?? ApprovalMode.DEFAULT;
209217

210218
if (newMode === ApprovalMode.PLAN || newMode === ApprovalMode.YOLO) {

0 commit comments

Comments
 (0)