fix(core): exit_plan_mode returns guidance error from execute() instead of permission deny

docs/design/explicit-plan-exit-approval.md

@@ -22,7 +22,7 @@ ACP sends no mode update when permission is pending or when confirmation, hooks,
 
 ## Failure behavior
 
-- Calls outside Plan mode are denied before a confirmation is constructed.
+- Calls outside Plan mode fail safely with actionable state guidance at whichever boundary observes the mode change. `execute()` returns a guidance error when the session is outside Plan mode and there is no approval snapshot. `getConfirmationDetails()` throws the same guidance when called outside Plan mode (e.g. via a PM `ask` rule or a Plan-to-non-Plan switch between permission evaluation and confirmation construction). The default permission is `allow` — this is a state issue, not a security issue.
 - Invalid confirmation outcomes, cancellation, aborts, stale revisions, and transition failures leave Plan mode active.
 - Two exits approved against the same revision cannot both succeed.
 - If an ACP host cannot present `switch_mode`, Plan mode remains active and the error directs the user to the host mode selector or `/plan exit`.

packages/core/src/core/coreToolScheduler.test.ts

@@ -35,6 +35,7 @@ import { pathToFileURL } from 'node:url';
 import { SkillTool } from '../tools/skill.js';
 import { StructuredToolError } from '../tools/priorReadEnforcement.js';
 import { ToolNames, ToolNamesMigration } from '../tools/tool-names.js';
+import { ExitPlanModeTool } from '../tools/exitPlanMode.js';
 import type {
   CompletedToolCall,
   ExecutingToolCall,
@@ -1171,6 +1172,120 @@ describe('CoreToolScheduler', () => {
     expect(execute).toHaveBeenCalledWith({ plan: 'Original plan' });
   });
 
+  it('returns guidance error through the scheduler when a PM ask rule hits exit_plan_mode outside plan mode (#7671)', async () => {
+    const exitPlanConfig = {
+      getApprovalMode: () => ApprovalMode.DEFAULT,
+      getApprovalModeRevision: () => 7,
+      getPrePlanMode: () => ApprovalMode.DEFAULT,
+      setApprovalMode: vi.fn(),
+      savePlan: vi.fn(),
+      getTeamManager: () => undefined,
+    } as unknown as Config;
+    const realTool = new ExitPlanModeTool(exitPlanConfig);
+    const permissionManager = {
+      isToolEnabled: vi.fn().mockResolvedValue(true),
+      hasRelevantRules: vi.fn().mockReturnValue(true),
+      evaluate: vi.fn().mockResolvedValue('ask'),
+      hasMatchingAskRule: vi.fn().mockReturnValue(true),
+      findMatchingDenyRule: vi.fn(),
+    };
+    const onAllToolCallsComplete = vi.fn();
+    const onToolCallsUpdate = vi.fn();
+    const { scheduler } = createSchedulerForLegacyToolTests({
+      toolsByName: new Map([
+        [ToolNames.EXIT_PLAN_MODE, realTool as unknown as MockTool],
+      ]),
+      approvalMode: ApprovalMode.DEFAULT,
+      onAllToolCallsComplete,
+      onToolCallsUpdate,
+    });
+    Object.assign(
+      (scheduler as unknown as { config: Record<string, unknown> }).config,
+      {
+        getPermissionManager: () => permissionManager,
+        getTargetDir: () => '/repo',
+        getConditionalRulesRegistry: () => undefined,
+        getSkillManager: () => undefined,
+      },
+    );
+
+    await scheduler.schedule(
+      [
+        {
+          callId: 'pm-ask-exit-plan',
+          name: ToolNames.EXIT_PLAN_MODE,
+          args: { plan: 'My plan' },
+          isClientInitiated: false,
+          prompt_id: 'prompt-pm-ask-exit-plan',
+        },
+      ],
+      new AbortController().signal,
+    );
+
+    await vi.waitFor(() => expect(onAllToolCallsComplete).toHaveBeenCalled());
+    const completedCalls = onAllToolCallsComplete.mock
+      .calls[0][0] as CompletedToolCall[];
+    expect(completedCalls[0].status).toBe('error');
+    const errorJson = JSON.stringify(completedCalls[0].response);
+    expect(errorJson).toContain('not in plan mode');
+    expect(errorJson).toContain('Do not call exit_plan_mode again');
+  });
+
+  it('returns guidance error through the scheduler on Plan-to-non-Plan timing boundary (#7671)', async () => {
+    // Simulate: permission evaluation sees PLAN (requiresUserInteraction
+    // returns true, forcing ask), then mode switches before
+    // getConfirmationDetails is called.
+    let approvalModeCallCount = 0;
+    const exitPlanConfig = {
+      getApprovalMode: () => {
+        approvalModeCallCount++;
+        // The first call is requiresUserInteraction() during
+        // evaluatePermissionFlow; subsequent calls (inside
+        // getConfirmationDetails) see DEFAULT.
+        return approvalModeCallCount <= 1
+          ? ApprovalMode.PLAN
+          : ApprovalMode.DEFAULT;
+      },
+      getApprovalModeRevision: () => 7,
+      getPrePlanMode: () => ApprovalMode.DEFAULT,
+      setApprovalMode: vi.fn(),
+      savePlan: vi.fn(),
+      getTeamManager: () => undefined,
+    } as unknown as Config;
+    const realTool = new ExitPlanModeTool(exitPlanConfig);
+    const onAllToolCallsComplete = vi.fn();
+    const onToolCallsUpdate = vi.fn();
+    const { scheduler } = createSchedulerForLegacyToolTests({
+      toolsByName: new Map([
+        [ToolNames.EXIT_PLAN_MODE, realTool as unknown as MockTool],
+      ]),
+      approvalMode: ApprovalMode.PLAN,
+      onAllToolCallsComplete,
+      onToolCallsUpdate,
+    });
+
+    await scheduler.schedule(
+      [
+        {
+          callId: 'timing-boundary-exit-plan',
+          name: ToolNames.EXIT_PLAN_MODE,
+          args: { plan: 'My plan' },
+          isClientInitiated: false,
+          prompt_id: 'prompt-timing-boundary-exit-plan',
+        },
+      ],
+      new AbortController().signal,
+    );
+
+    await vi.waitFor(() => expect(onAllToolCallsComplete).toHaveBeenCalled());
+    const completedCalls = onAllToolCallsComplete.mock
+      .calls[0][0] as CompletedToolCall[];
+    expect(completedCalls[0].status).toBe('error');
+    const errorJson = JSON.stringify(completedCalls[0].response);
+    expect(errorJson).toContain('not in plan mode');
+    expect(errorJson).toContain('Do not call exit_plan_mode again');
+  });
+
   it('does not let AUTO_EDIT approve an interaction-required info tool', async () => {
     const execute = vi.fn().mockResolvedValue({
       llmContent: 'executed',

packages/core/src/tools/exitPlanMode.test.ts

@@ -10,6 +10,8 @@ import { runWithAgentContext } from '../agents/runtime/agent-context.js';
 import { runWithTeammateIdentity } from '../agents/team/identity.js';
 import { ExitPlanModeTool, type ExitPlanModeParams } from './exitPlanMode.js';
 import { ToolConfirmationOutcome } from './tools.js';
+import { StructuredToolError } from './priorReadEnforcement.js';
+import { ToolErrorType } from './tool-error.js';
 
 describe('ExitPlanModeTool', () => {
   let tool: ExitPlanModeTool;
@@ -75,17 +77,46 @@ describe('ExitPlanModeTool', () => {
     const invocation = tool.build({ plan: 'Plan' });
 
     expect(invocation.requiresUserInteraction?.()).toBe(true);
-    await expect(invocation.getDefaultPermission()).resolves.toBe('ask');
+    // getDefaultPermission always returns 'allow'; requiresUserInteraction
+    // forces 'ask' via permissionFlow when in plan mode (#7671).
+    await expect(invocation.getDefaultPermission()).resolves.toBe('allow');
   });
 
-  it('denies outside plan mode without constructing a misleading prompt', async () => {
+  it('allows outside plan mode but execute returns guidance error (#7671)', async () => {
     approvalMode = ApprovalMode.DEFAULT;
     const invocation = tool.build({ plan: 'Plan' });
 
-    await expect(invocation.getDefaultPermission()).resolves.toBe('deny');
-    await expect(
-      invocation.getConfirmationDetails(new AbortController().signal),
-    ).rejects.toThrow('outside plan mode');
+    // Permission layer allows — not a security concern, just wrong state
+    await expect(invocation.getDefaultPermission()).resolves.toBe('allow');
+
+    // No user interaction required outside plan mode — execute() handles it
+    expect(invocation.requiresUserInteraction?.()).toBe(false);
+
+    // Execute layer returns a helpful error with EXECUTION_DENIED type
+    const result = await invocation.execute(new AbortController().signal);
+    expect(result.error).toBeDefined();
+    expect(result.error?.type).toBe(ToolErrorType.EXECUTION_DENIED);
+    expect(result.llmContent).toContain('not in plan mode');
+    expect(result.llmContent).toContain('Do not call exit_plan_mode again');
+  });
+
+  it('getConfirmationDetails throws structured guidance outside plan mode (#7671)', async () => {
+    approvalMode = ApprovalMode.DEFAULT;
+    const invocation = tool.build({ plan: 'Plan' });
+
+    try {
+      await invocation.getConfirmationDetails(new AbortController().signal);
+      expect.unreachable('should have thrown');
+    } catch (error) {
+      expect(error).toBeInstanceOf(StructuredToolError);
+      expect((error as StructuredToolError).errorType).toBe(
+        ToolErrorType.EXECUTION_DENIED,
+      );
+      expect((error as Error).message).toContain('not in plan mode');
+      expect((error as Error).message).toContain(
+        'Do not call exit_plan_mode again',
+      );
+    }
   });
 
   it.each([

packages/core/src/tools/exitPlanMode.ts

@@ -28,6 +28,8 @@ import {
 } from '../agents/runtime/subagent-plan-tool-policy.js';
 import { getTeammateContext } from '../agents/team/identity.js';
 import type { TeamPlanApprovalDecision } from '../agents/team/TeamManager.js';
+import { StructuredToolError } from './priorReadEnforcement.js';
+import { ToolErrorType } from './tool-error.js';
 
 const debugLogger = createDebugLogger('EXIT_PLAN_MODE');
 
@@ -115,20 +117,23 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
   }
 
   override requiresUserInteraction(): boolean {
+    // Outside plan mode, no user interaction is needed — execute() will
+    // return a guidance error directly (#7671).
+    if (this.config.getApprovalMode() !== ApprovalMode.PLAN) {
+      return false;
+    }
     return (
       !isPlanRequiredTeammateContext() &&
       !isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)
     );
   }
 
   override async getDefaultPermission(): Promise<PermissionDecision> {
-    if (
-      isPlanRequiredTeammateContext() ||
-      isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)
-    ) {
-      return 'allow';
-    }
-    return this.config.getApprovalMode() === ApprovalMode.PLAN ? 'ask' : 'deny';
+    // Always allow at the permission layer. Plan-mode gating lives in
+    // requiresUserInteraction() (forces 'ask' via permissionFlow) and
+    // execute() (returns guidance error outside plan mode). A single
+    // source of truth avoids duplicated conditionals (#7671).
+    return 'allow';
   }
 
   override async getConfirmationDetails(
@@ -141,7 +146,10 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
       return super.getConfirmationDetails(abortSignal);
     }
     if (this.config.getApprovalMode() !== ApprovalMode.PLAN) {
-      throw new Error('Cannot request plan approval outside plan mode.');
+      throw new StructuredToolError(
+        this.outsidePlanGuidanceMessage(),
+        ToolErrorType.EXECUTION_DENIED,
+      );
     }
 
     const snapshot: ExitApprovalSnapshot = {
@@ -201,6 +209,17 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
       );
     }
 
+    // Not in plan mode and no approval snapshot — the user may have
+    // manually switched modes. Return a guidance error instead of a
+    // permission deny (#7671). If there IS an approval snapshot, let the
+    // stale-revision check below handle it (concurrent exit scenario).
+    if (this.config.getApprovalMode() !== ApprovalMode.PLAN && !this.approval) {
+      return this.errorResult(
+        this.outsidePlanGuidanceMessage(),
+        ToolErrorType.EXECUTION_DENIED,
+      );
+    }
+
     const { plan, originalRequest, researchSummary } = this.params;
     if (isPlanRequiredTeammateContext()) {
       return this.executePlanRequiredTeammate(
@@ -352,6 +371,15 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
     };
   }
 
+  private outsidePlanGuidanceMessage(): string {
+    const currentMode = this.config.getApprovalMode();
+    return (
+      `You are not in plan mode (current mode: ${currentMode}). ` +
+      `The user may have manually switched modes via Shift+Tab or /approval-mode. ` +
+      `Do not call exit_plan_mode again. Continue working in the current mode.`
+    );
+  }
+
   private savePlanBestEffort(plan: string): void {
     try {
       this.config.savePlan(plan);
@@ -362,11 +390,11 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
     }
   }
 
-  private errorResult(message: string): ToolResult {
+  private errorResult(message: string, type?: ToolErrorType): ToolResult {
     return {
       llmContent: message,
       returnDisplay: message,
-      error: { message },
+      error: { message, type },
     };
   }