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.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/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..30f98d89cc6 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'); }, ); @@ -371,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({ 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(