From d994ed9f2281dc5e4eea7d3a33b570766c98e334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BA=B3=E4=BF=A1?= Date: Fri, 24 Jul 2026 22:50:05 +0800 Subject: [PATCH 1/2] fix(core): tell the model when the user manually exits plan mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While plan mode is active the plan-mode system reminder is re-injected on every model-bound turn, so after a manual exit (Shift+Tab, /approval-mode, /plan, ACP mode switch) the reminder just silently stops appearing — a non-signal models do not reliably notice. The model's most recent context still says plan mode is active, so it keeps calling exit_plan_mode and gets stuck (#7671, problem 1). Config.setApprovalMode now queues a one-shot notice on every PLAN -> non-PLAN transition except the approved exit_plan_mode flow (which passes the new fromApprovedPlanExit option); re-entering plan mode clears a stale notice. GeminiClient's system-reminder assembly consumes the flag on the next turn and injects an explicit "the user has manually switched out of plan mode" reminder naming the new mode. Fixes the first half of #7671; the deny-message half is #7673. Co-Authored-By: Claude Fable 5 --- packages/core/src/config/config.test.ts | 45 +++++++++++++ packages/core/src/config/config.ts | 36 +++++++++- packages/core/src/core/client.test.ts | 69 ++++++++++++++++++++ packages/core/src/core/client.ts | 10 +++ packages/core/src/core/prompts.ts | 16 +++++ packages/core/src/tools/exitPlanMode.test.ts | 4 +- packages/core/src/tools/exitPlanMode.ts | 8 ++- 7 files changed, 182 insertions(+), 6 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 358575e1f20..40cf682cab1 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -6460,6 +6460,51 @@ describe('setApprovalMode with folder trust', () => { expect(config.getApprovalModeRevision()).toBe(initialRevision + 2); }); + it('queues a one-shot manual plan-exit notice on a manual exit', () => { + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + config.setApprovalMode(ApprovalMode.PLAN); + config.setApprovalMode(ApprovalMode.DEFAULT); + + expect(config.consumePendingManualPlanExitNotice()).toBe(true); + // One-shot: consumed on first read. + expect(config.consumePendingManualPlanExitNotice()).toBe(false); + }); + + it('does not queue the exit notice for an approved plan exit', () => { + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + config.setApprovalMode(ApprovalMode.PLAN); + config.setApprovalMode(ApprovalMode.DEFAULT, { + fromApprovedPlanExit: true, + }); + + expect(config.consumePendingManualPlanExitNotice()).toBe(false); + }); + + it('clears a stale exit notice when plan mode is re-entered', () => { + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + config.setApprovalMode(ApprovalMode.PLAN); + config.setApprovalMode(ApprovalMode.DEFAULT); + config.setApprovalMode(ApprovalMode.PLAN); + + expect(config.consumePendingManualPlanExitNotice()).toBe(false); + }); + + it('does not queue the exit notice for non-plan mode changes', () => { + const config = new Config(baseParams); + vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); + + config.setApprovalMode(ApprovalMode.AUTO_EDIT); + config.setApprovalMode(ApprovalMode.DEFAULT); + + expect(config.consumePendingManualPlanExitNotice()).toBe(false); + }); + it('records prePlanMode=yolo for a Shift+Tab cycle into plan mode', () => { const config = new Config(baseParams); vi.spyOn(config, 'isTrustedFolder').mockReturnValue(true); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 53201c3bd14..931ef45cb29 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1761,6 +1761,7 @@ export class Config { private approvalMode: ApprovalMode; private prePlanMode?: ApprovalMode; private approvalModeRevision = 0; + private pendingManualPlanExitNotice = false; private autoModeDenialState: AutoModeDenialState = createDenialState(); private readonly accessibility: AccessibilitySettings; private readonly showResponseTokensPerSecond: boolean; @@ -5403,10 +5404,18 @@ export class Config { setApprovalMode( mode: ApprovalMode, - /** @deprecated Model origin no longer changes plan-exit approval. */ - options?: { enteredByModel?: boolean }, + options?: { + /** @deprecated Model origin no longer changes plan-exit approval. */ + enteredByModel?: boolean; + /** + * Set by ExitPlanModeTool for user/leader-approved plan exits. Every + * other PLAN → non-PLAN transition (Shift+Tab, /approval-mode, /plan, + * ACP setSessionMode, confirm-and-switch) is a manual exit the model + * was never told about, and queues a one-shot system reminder. + */ + fromApprovedPlanExit?: boolean; + }, ): void { - void options; if ( !this.isTrustedFolder() && mode !== ApprovalMode.DEFAULT && @@ -5435,8 +5444,18 @@ export class Config { // succeeded, so callers never observe a partially applied mode change. if (mode === ApprovalMode.PLAN && fromMode !== ApprovalMode.PLAN) { this.prePlanMode = fromMode; + // A stale exit notice must not survive a re-entry: the plan-mode + // reminder takes over again on the next turn. + this.pendingManualPlanExitNotice = false; } else if (mode !== ApprovalMode.PLAN && fromMode === ApprovalMode.PLAN) { this.prePlanMode = undefined; + if (!options?.fromApprovedPlanExit) { + // While in plan mode the model is told "plan mode is active" on + // every turn; on a manual exit that reminder just stops appearing, + // which models do not reliably notice (#7671). Queue an explicit + // one-shot exit notice for the next turn's reminder assembly. + this.pendingManualPlanExitNotice = true; + } } // Any deliberate mode change invalidates the AUTO denialTracking signal. if (fromMode !== mode) { @@ -5448,6 +5467,17 @@ export class Config { } } + /** + * One-shot: returns whether a manual (non-approved) plan-mode exit is + * pending model notification, and clears the flag. Consumed by the + * system-reminder assembly in `GeminiClient` on the next model-bound turn. + */ + consumePendingManualPlanExitNotice(): boolean { + const pending = this.pendingManualPlanExitNotice; + this.pendingManualPlanExitNotice = false; + return pending; + } + /** * Returns the directory where this session's plan file is stored. */ diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 0546004926b..4982c3151c9 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -566,6 +566,7 @@ describe('Gemini Client (client.ts)', () => { getNoBrowser: vi.fn().mockReturnValue(false), getUsageStatisticsEnabled: vi.fn().mockReturnValue(true), getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), + consumePendingManualPlanExitNotice: vi.fn().mockReturnValue(false), getSdkMode: vi.fn().mockReturnValue(false), getExperimentalZedIntegration: vi.fn().mockReturnValue(false), isInteractive: vi.fn().mockReturnValue(false), @@ -5964,6 +5965,74 @@ hello ); }); + it('injects a one-shot exit notice after a manual plan-mode exit', async () => { + vi.mocked(mockConfig.getApprovalMode).mockReturnValue( + ApprovalMode.DEFAULT, + ); + vi.mocked( + mockConfig.consumePendingManualPlanExitNotice, + ).mockReturnValueOnce(true); + const mockStream = (async function* () { + yield { type: 'content', value: 'Continuing' }; + })(); + mockTurnRunFn.mockReturnValue(mockStream); + client['chat'] = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + } as unknown as GeminiChat; + + const stream = client.sendMessageStream( + [{ text: 'Now implement it' }], + new AbortController().signal, + 'prompt-id-manual-plan-exit', + ); + for await (const _ of stream) { + // consume stream + } + + expect(mockTurnRunFn).toHaveBeenCalledWith( + 'test-model', + expect.arrayContaining([ + expect.stringContaining( + 'The user has manually switched out of plan mode', + ), + ]), + expect.any(AbortSignal), + ); + }); + + it('does not inject the exit notice when none is pending', async () => { + vi.mocked(mockConfig.getApprovalMode).mockReturnValue( + ApprovalMode.DEFAULT, + ); + const mockStream = (async function* () { + yield { type: 'content', value: 'Reply' }; + })(); + mockTurnRunFn.mockReturnValue(mockStream); + client['chat'] = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + } as unknown as GeminiChat; + + const stream = client.sendMessageStream( + [{ text: 'Hello' }], + new AbortController().signal, + 'prompt-id-no-plan-exit-notice', + ); + for await (const _ of stream) { + // consume stream + } + + const requestArg = mockTurnRunFn.mock.calls.at(-1)![1] as string[]; + expect( + requestArg.some( + (part) => + typeof part === 'string' && + part.includes('manually switched out of plan mode'), + ), + ).toBe(false); + }); + it('uses the subagent plan reminder when a subagent inherits PLAN mode', async () => { vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); vi.mocked(mockConfig.getSdkMode).mockReturnValue(false); diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index e3fd2d7403c..666c390c7db 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -52,6 +52,7 @@ import { getArenaSystemReminder, getCoreSystemPrompt, getCustomSystemPrompt, + getManualPlanExitSystemReminder, getPlanModeSystemReminder, resolveInteractionMode, } from './prompts.js'; @@ -2452,6 +2453,15 @@ export class GeminiClient { this.config.getSdkMode(), ), ); + } else if (this.config.consumePendingManualPlanExitNotice()) { + // One-shot counterpart to the reminder above: the model was told + // "plan mode is active" on every turn, so a manual exit + // (Shift+Tab, /approval-mode, /plan) needs an explicit signal — + // the reminder silently disappearing goes unnoticed and the + // model keeps calling exit_plan_mode (#7671). + systemReminders.push( + getManualPlanExitSystemReminder(this.config.getApprovalMode()), + ); } // add arena system reminder if an arena session is active diff --git a/packages/core/src/core/prompts.ts b/packages/core/src/core/prompts.ts index 77a9ef31420..3f9839cc3bb 100644 --- a/packages/core/src/core/prompts.ts +++ b/packages/core/src/core/prompts.ts @@ -1098,6 +1098,22 @@ Your plan is ready when you have addressed all ambiguities and it covers: what t `; } +/** + * One-shot reminder injected on the first model-bound turn after the user + * manually exits plan mode (Shift+Tab, `/approval-mode`, `/plan`, ACP mode + * switch). While plan mode is active {@link getPlanModeSystemReminder} is + * re-injected every turn, so on a manual exit the model's most recent + * context still says "plan mode is active" — the reminder silently + * disappearing is not a signal models reliably notice (#7671). + * + * @param currentMode - The approval mode the user switched to + */ +export function getManualPlanExitSystemReminder(currentMode: string): string { + return ` +The user has manually switched out of plan mode (current approval mode: ${currentMode}). You are no longer in plan mode. Do NOT call ${ToolNames.EXIT_PLAN_MODE} — there is no plan approval pending. Continue working in the current mode; previous plan-mode restrictions on edits and state-modifying tools no longer apply. +`; +} + /** * Generates a system reminder about an active Arena session. * diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts index 2499f238537..012b1a6ab59 100644 --- a/packages/core/src/tools/exitPlanMode.test.ts +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -115,7 +115,9 @@ describe('ExitPlanModeTool', () => { expect(result.error).toBeUndefined(); expect(approvalMode).toBe(targetMode); - expect(config.setApprovalMode).toHaveBeenCalledWith(targetMode); + expect(config.setApprovalMode).toHaveBeenCalledWith(targetMode, { + fromApprovedPlanExit: true, + }); expect(config.savePlan).toHaveBeenCalledWith('Approved plan'); }, ); diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index 6afcbb03f37..6c864d50ff4 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -234,7 +234,9 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< this.savePlanBestEffort(snapshot.plan); try { - this.config.setApprovalMode(targetMode); + this.config.setApprovalMode(targetMode, { + fromApprovedPlanExit: true, + }); } catch (error) { const message = error instanceof Error ? error.message : String(error); debugLogger.error( @@ -331,7 +333,9 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< this.savePlanBestEffort(plan); try { - this.config.setApprovalMode(decision.targetMode); + this.config.setApprovalMode(decision.targetMode, { + fromApprovedPlanExit: true, + }); } catch (error) { const message = error instanceof Error ? error.message : String(error); return this.errorResult( From 18bb51ba2746e6130ecbd88ea6830abd7715e6ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=BA=B3=E4=BF=A1?= Date: Sat, 25 Jul 2026 01:11:48 +0800 Subject: [PATCH 2/2] test(core): pin the leader-approval exit option and the exit-reminder text Adds the two review-suggested pins: a successful leader-approved exit asserts setApprovalMode receives { fromApprovedPlanExit: true } (only the regular approval site was pinned), and getManualPlanExitSystemReminder gets dedicated prompts tests covering the rendered mode name, the exit_plan_mode prohibition, and the reminder envelope. Co-Authored-By: Claude Fable 5 --- packages/core/src/core/prompts.test.ts | 22 +++++++++++++++ packages/core/src/tools/exitPlanMode.test.ts | 29 ++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/packages/core/src/core/prompts.test.ts b/packages/core/src/core/prompts.test.ts index 5f389f585f6..b3d28df8531 100644 --- a/packages/core/src/core/prompts.test.ts +++ b/packages/core/src/core/prompts.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { getCoreSystemPrompt, getCustomSystemPrompt, + getManualPlanExitSystemReminder, getPlanModeSystemReminder, resolvePathFromEnv, getCompressionPrompt, @@ -786,6 +787,27 @@ describe('getPlanModeSystemReminder', () => { }); }); +describe('getManualPlanExitSystemReminder', () => { + it('should name the new mode and forbid exit_plan_mode', () => { + const result = getManualPlanExitSystemReminder('default'); + + expect(result).toMatch(/^[\s\S]*<\/system-reminder>$/); + expect(result).toContain('manually switched out of plan mode'); + expect(result).toContain('current approval mode: default'); + expect(result).toContain('Do NOT call exit_plan_mode'); + expect(result).toContain('no longer apply'); + }); + + it('should render whichever mode the user switched to', () => { + expect(getManualPlanExitSystemReminder('yolo')).toContain( + 'current approval mode: yolo', + ); + expect(getManualPlanExitSystemReminder('auto-edit')).toContain( + 'current approval mode: auto-edit', + ); + }); +}); + describe('resolvePathFromEnv helper function', () => { beforeEach(() => { vi.resetAllMocks(); diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts index 012b1a6ab59..30f98d89cc6 100644 --- a/packages/core/src/tools/exitPlanMode.test.ts +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -373,6 +373,35 @@ describe('ExitPlanModeTool', () => { expect(config.setApprovalMode).not.toHaveBeenCalled(); }); + it('marks a successful leader-approved exit as an approved plan exit', async () => { + vi.mocked(config.getTeamManager).mockReturnValue({ + requestPlanApproval: vi.fn(async () => ({ + action: 'approve', + targetMode: ApprovalMode.DEFAULT, + })), + } as never); + const invocation = tool.build({ plan: 'Teammate plan' }); + + const result = await runWithTeammateIdentity( + { + agentId: 'planner@test', + agentName: 'planner', + teamName: 'test', + isTeamLead: false, + planModeRequired: true, + }, + () => invocation.execute(new AbortController().signal), + ); + + expect(result.error).toBeUndefined(); + expect(approvalMode).toBe(ApprovalMode.DEFAULT); + // Must mirror the regular approval call site: without the option, a + // leader-approved exit would queue the manual plan-exit reminder. + expect(config.setApprovalMode).toHaveBeenCalledWith(ApprovalMode.DEFAULT, { + fromApprovedPlanExit: true, + }); + }); + it('saves a leader-approved plan when the teammate transition fails', async () => { transitionError = new Error('mode locked'); vi.mocked(config.getTeamManager).mockReturnValue({