fix(agent-core): fix plan mode auto review bug

.changeset/exit-plan-mode-auto-review.md

@@ -0,0 +1,5 @@
+---
+"@moonshot-ai/kimi-code": minor
+---
+
+Exit plan mode now requires reviewing the current plan, even in auto permission mode. Approve or revise the plan when prompted.

packages/agent-core/src/agent/permission/policies/exit-plan-mode-review-ask.ts

@@ -19,7 +19,6 @@ export class ExitPlanModeReviewAskPermissionPolicy implements PermissionPolicy {
 
   evaluate(context: PermissionPolicyContext): PermissionPolicyResult | undefined {
     if (context.toolCall.name !== 'ExitPlanMode') return;
-    if (this.agent.permission.mode === 'auto') return;
     if (!this.agent.planMode.isActive) return;
     const display = context.execution.display;
     if (display?.kind !== 'plan_review') return;

packages/agent-core/src/agent/permission/policies/index.ts

@@ -37,6 +37,8 @@ export function createPermissionDecisionPolicies(agent: Agent): PermissionPolicy
     new PlanModeGuardDenyPermissionPolicy(agent),
     // User-configured deny rule matches → deny.
     new UserConfiguredDenyPermissionPolicy(agent),
+    // ExitPlanMode with active plan_review + non-empty plan → ask (tracks plan_submitted/plan_resolved itself). Runs before session history so a stale session approval can't bypass review of a new plan body and before auto mode permission policy so auto mode cannot bypass review of a new plan body.
+    new ExitPlanModeReviewAskPermissionPolicy(agent),
     // auto mode → approve (any auto-mode block must be a deny rule above this).
     new AutoModeApprovePermissionPolicy(agent),
     // Approve-for-session memorized rule matches → approve. Runs before user-configured ask rules so an in-session grant beats a still-matching ask rule on later calls.
@@ -45,8 +47,6 @@ export function createPermissionDecisionPolicies(agent: Agent): PermissionPolicy
     new UserConfiguredAskPermissionPolicy(agent),
     // User-configured allow rule matches → approve.
     new UserConfiguredAllowPermissionPolicy(agent),
-    // ExitPlanMode with active plan_review + non-empty plan + non-auto → ask (tracks plan_submitted/plan_resolved itself). Runs before session history so a stale session approval can't bypass review of a new plan body.
-    new ExitPlanModeReviewAskPermissionPolicy(agent),
     // CreateGoal (non-auto) → ask with the same start menu as /goal: choose the
     // permission mode to run the goal under, or decline. Applies the mode, then
     // lets the tool create the goal.

packages/agent-core/src/tools/builtin/planning/exit-plan-mode.ts

@@ -136,7 +136,7 @@ export class ExitPlanModeTool implements BuiltinTool<ExitPlanModeInput> {
     const failed = this.exitPlanMode();
     if (failed !== undefined) return failed;
 
-    this.agent.telemetry.track('plan_resolved', { outcome: 'auto_approved' });
+    this.agent.telemetry.track('plan_resolved', { outcome: 'approved' });
 
     return {
       isError: false,

packages/agent-core/test/agent/permission.test.ts

@@ -704,11 +704,11 @@ describe('Permission policy chain', () => {
       'auto-mode-ask-user-question-deny',
       'plan-mode-guard-deny',
       'user-configured-deny',
+      'exit-plan-mode-review-ask',
       'auto-mode-approve',
       'session-approval-history',
       'user-configured-ask',
       'user-configured-allow',
-      'exit-plan-mode-review-ask',
       'goal-start-review-ask',
       'plan-mode-tool-approve',
       'sensitive-file-access-ask',
@@ -1842,18 +1842,31 @@ describe('ExitPlanMode permission policy', () => {
     { label: 'Approach B', description: 'Use the complete refactor.' },
   ];
 
-  it('allows ExitPlanMode directly in auto mode without requesting approval', async () => {
-    const { manager, requestApproval } = makePlanPermissionManager({
+  it('requests plan-review approval in auto mode and returns formatted output', async () => {
+    const { manager, requestApproval, exit } = makePlanPermissionManager({
       mode: 'auto',
       plan: '# Auto Plan',
+      path: '/tmp/plan.md',
+      approval: { decision: 'approved' },
     });
 
     const result = await manager.beforeToolCall(
-      hookContext({ id: 'call_exit', toolName: 'ExitPlanMode', args: {} }),
+      hookContext({
+        id: 'call_exit',
+        toolName: 'ExitPlanMode',
+        args: {},
+        execution: planReviewExecution({ plan: '# Auto Plan', path: '/tmp/plan.md' }),
+      }),
     );
 
-    expect(result).toBeUndefined();
-    expect(requestApproval).not.toHaveBeenCalled();
+    expect(requestApproval).toHaveBeenCalledTimes(1);
+    expect(exit).toHaveBeenCalled();
+    expect(result).toMatchObject({
+      syntheticResult: {
+        isError: false,
+        output: expect.stringContaining('Exited plan mode.'),
+      },
+    });
   });
 
   it('requests plan-review approval in yolo mode and returns formatted output', async () => {
@@ -1910,7 +1923,7 @@ describe('ExitPlanMode permission policy', () => {
     });
   });
 
-  it('reuses session approval for ExitPlanMode without re-prompting plan review', async () => {
+  it('re-prompts plan review for a new plan body even when a session approval exists', async () => {
     const { manager, requestApproval, exit } = makePlanPermissionManager({
       mode: 'manual',
       plan: '# Updated Plan',
@@ -1934,9 +1947,14 @@ describe('ExitPlanMode permission policy', () => {
       }),
     );
 
-    expect(requestApproval).not.toHaveBeenCalled();
-    expect(exit).not.toHaveBeenCalled();
-    expect(result).toBeUndefined();
+    expect(requestApproval).toHaveBeenCalledTimes(1);
+    expect(exit).toHaveBeenCalled();
+    expect(result).toMatchObject({
+      syntheticResult: {
+        isError: false,
+        output: expect.stringContaining('Exited plan mode.'),
+      },
+    });
   });
 
   it('returns a synthetic stop-turn result when the user rejects the plan', async () => {

packages/agent-core/test/agent/plan.test.ts

@@ -128,7 +128,7 @@ describe('plan clear', () => {
 });
 
 describe('plan exit tool', () => {
-  it('reads the current plan file and exits plan mode directly in auto mode', async () => {
+  it('requests plan review before exiting plan mode in auto mode', async () => {
     const files = new Map<string, string>();
     const readText = vi.fn(async (path: string) => files.get(path) ?? '');
     const ctx = testAgent({
@@ -152,10 +152,10 @@ describe('plan exit tool', () => {
     ctx.mockNextResponse({ type: 'text', text: 'I can execute after approval.' });
     await ctx.rpc.prompt({ input: [{ type: 'text', text: 'Show the plan' }] });
 
+    const approval = await ctx.takeApprovalRequest();
+    approval.respond({ decision: 'approved' });
+
     await ctx.untilTurnEnd();
-    expect(
-      ctx.allEvents.some((event) => event.type === '[rpc]' && event.event === 'requestApproval'),
-    ).toBe(false);
     expect(readText).toHaveBeenCalledWith(planPath);
     expect(ctx.agent.planMode.isActive).toBe(false);
     const llmInput = ctx.llmCalls[1]!;

packages/agent-core/test/tools/planning/exit-plan-mode-telemetry.test.ts

@@ -123,7 +123,7 @@ function permissionContext(args: ExitPlanModeInput): PermissionPolicyContext {
 }
 
 describe('ExitPlanMode telemetry', () => {
-  it('tracks submitted without options and auto approval', async () => {
+  it('tracks submitted without options and review approval in auto mode', async () => {
     const { agent, telemetryTrack, exitPlanMode } = makeAgent({ mode: 'auto' });
 
     const result = await execute(agent);
@@ -132,7 +132,7 @@ describe('ExitPlanMode telemetry', () => {
     expect(exitPlanMode).toHaveBeenCalledTimes(1);
     expect(telemetryTrack).toHaveBeenCalledWith('plan_submitted', { has_options: false });
     expect(telemetryTrack).toHaveBeenCalledWith('plan_resolved', {
-      outcome: 'auto_approved',
+      outcome: 'approved',
     });
   });