Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 84 additions & 4 deletions packages/core/src/tools/enterPlanMode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ describe('EnterPlanModeTool', () => {
expect(tool.shouldDefer).toBe(false);
});

it('should have empty-object schema', () => {
it('should expose only the userRequested flag in its schema', () => {
expect(tool.schema.parametersJsonSchema).toEqual({
type: 'object',
properties: {},
properties: {
userRequested: {
type: 'boolean',
description: expect.stringContaining('ONLY when the user'),
},
},
additionalProperties: false,
$schema: 'http://json-schema.org/draft-07/schema#',
});
Expand Down Expand Up @@ -124,16 +129,91 @@ describe('EnterPlanModeTool', () => {
expect(savedPrePlanMode).toBe(ApprovalMode.AUTO);
});

it('should switch from YOLO to PLAN', async () => {
it('should not switch from YOLO to PLAN when the entry is unsolicited', async () => {
// Regression: #5970. A YOLO user opted into low-friction execution;
// silently switching to read-only Plan mode surprised them and then
// blocked reads/writes they expected to proceed. A model-initiated
// enter_plan_mode from YOLO must keep the current mode instead.
approvalMode = ApprovalMode.YOLO;
const invocation = tool.build({});
await invocation.execute(new AbortController().signal);
const result = await invocation.execute(new AbortController().signal);

expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
expect(approvalMode).toBe(ApprovalMode.YOLO);
expect(savedPrePlanMode).toBeUndefined();
expect(result.llmContent).toContain('YOLO');
expect(result.llmContent).not.toContain('Plan mode is now active');
// The model must be told how to honour an explicit user request.
expect(result.llmContent).toContain('userRequested: true');
});

it('should not switch from YOLO to PLAN when userRequested is explicitly false', async () => {
approvalMode = ApprovalMode.YOLO;
const invocation = tool.build({ userRequested: false });
const result = await invocation.execute(new AbortController().signal);

expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
expect(approvalMode).toBe(ApprovalMode.YOLO);
expect(result.llmContent).toContain('YOLO');
expect(result.llmContent).not.toContain('Plan mode is now active');
expect(result.returnDisplay).toContain('Stayed in YOLO');
});

it('should treat userRequested as inert outside YOLO (DEFAULT enters PLAN normally)', async () => {
// Defensive: the flag only gates the YOLO no-op. If it ever gained
// significance in other modes, this pins the expected behavior.
approvalMode = ApprovalMode.DEFAULT;
const invocation = tool.build({ userRequested: true });
const result = await invocation.execute(new AbortController().signal);

expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.PLAN,
{ enteredByModel: true },
);
expect(approvalMode).toBe(ApprovalMode.PLAN);
expect(savedPrePlanMode).toBe(ApprovalMode.DEFAULT);
expect(result.llmContent).toContain('Plan mode is now active');
});

it('should switch from YOLO to PLAN when the user explicitly requested it', async () => {
// The tool description instructs the model to call this only after the
// user asks, and `/plan` is interactive-only — so this tool is the only
// door into plan mode for headless/ACP sessions. A blanket YOLO guard
// would make an explicit user request unreachable there.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This test only asserts setApprovalMode was not called and mode stayed YOLO, but does not verify the return value. The sibling "unsolicited" test asserts on llmContent (contains 'YOLO', not 'Plan mode is now active', contains 'userRequested: true'). Both paths share the same return object today, but the weaker test provides no regression safety on the user-facing message.

Suggested change
// would make an explicit user request unreachable there.
expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
expect(approvalMode).toBe(ApprovalMode.YOLO);
expect(result.llmContent).toContain('YOLO');
expect(result.returnDisplay).toContain('Stayed in YOLO');

— qwen3.7-max via Qwen Code /review

approvalMode = ApprovalMode.YOLO;
const invocation = tool.build({ userRequested: true });
const result = await invocation.execute(new AbortController().signal);

// Still flagged as model-initiated so exit_plan_mode runs the Plan
// Approval Gate for the YOLO session (#5574).
expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.PLAN,
{ enteredByModel: true },
);
expect(approvalMode).toBe(ApprovalMode.PLAN);
expect(savedPrePlanMode).toBe(ApprovalMode.YOLO);
expect(result.llmContent).toContain('Plan mode is now active');
});

it('should honour a user-requested YOLO entry in an ACP session', async () => {
// Headless + ACP: no `/plan`, no Shift+Tab. This tool is the only path.
approvalMode = ApprovalMode.YOLO;
(mockConfig.isInteractive as ReturnType<typeof vi.fn>).mockReturnValue(
false,
);
(
mockConfig.getExperimentalZedIntegration as ReturnType<typeof vi.fn>
).mockReturnValue(true);

const invocation = tool.build({ userRequested: true });
const result = await invocation.execute(new AbortController().signal);

expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.PLAN,
{ enteredByModel: true },
);
expect(approvalMode).toBe(ApprovalMode.PLAN);
expect(result.llmContent).not.toContain('non-interactive');
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] All userRequested: true tests start from ApprovalMode.YOLO. No test passes the flag from DEFAULT, AUTO, or AUTO_EDIT. The parameter is inert in non-YOLO modes (the guard never fires), but a defensive test would catch future regressions if the flag accidentally gained significance outside YOLO. Consider adding a test like tool.build({ userRequested: true }) from ApprovalMode.DEFAULT asserting normal plan-mode entry.

— qwen3.7-max via Qwen Code /review

it('should be idempotent: already in PLAN does not call setApprovalMode', async () => {
Expand Down
45 changes: 43 additions & 2 deletions packages/core/src/tools/enterPlanMode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@ import {

const debugLogger = createDebugLogger('ENTER_PLAN_MODE');

export type EnterPlanModeParams = Record<string, never>;
export interface EnterPlanModeParams {
/**
* Set to `true` only when the user explicitly asked for plan mode in this
* turn (or explicitly confirmed they want it). Distinguishes a genuine
* user-requested entry from the model deciding to plan on its own, which
* matters when the session is in YOLO mode — see the guard in `execute()`.
*/
userRequested?: boolean;
}

const enterPlanModeToolDescription = `Use this tool only after the user explicitly asks to switch into plan mode or confirms they want plan mode. Entering plan mode is a privilege reduction, so it does not require user confirmation at execution time.

Expand All @@ -38,7 +46,13 @@ const enterPlanModeToolSchemaData: FunctionDeclaration = {
description: enterPlanModeToolDescription,
parametersJsonSchema: {
type: 'object',
properties: {},
properties: {
userRequested: {
type: 'boolean',
description:
'Set to true ONLY when the user explicitly asked for plan mode in this turn, or explicitly confirmed they want it. Leave unset (or false) when you are deciding to plan on your own without the user asking. In YOLO mode, an explicit user request will not take effect unless this is true.',
},
},
additionalProperties: false,
$schema: 'http://json-schema.org/draft-07/schema#',
},
Expand Down Expand Up @@ -76,6 +90,33 @@ class EnterPlanModeToolInvocation extends BaseToolInvocation<
);
}

// A model-initiated entry from YOLO (not requested by the user this
// turn) is a no-op. The user explicitly chose YOLO for low-friction
// execution; silently switching to the read-only Plan mode surprises
// them and then blocks the reads/writes they expected to proceed
// (#5970). This tool is ALSO the only door into plan mode in
// headless/ACP sessions — `/plan` is `interactive`-only and there is no
// Shift+Tab there — so a blanket YOLO guard would make a genuine,
// explicit user request unreachable in those sessions. `userRequested`
// lets the model tell the two apart: only gate the no-op when the
// entry is NOT user-requested. Keep the current mode and tell the
// model to continue planning without switching, or to retry with
// `userRequested: true` if the user did explicitly ask.
if (
this.config.getApprovalMode() === ApprovalMode.YOLO &&
!this.params.userRequested
) {
debugLogger.info(
'Blocked model-initiated plan entry from YOLO (userRequested=%s)',
this.params.userRequested,
);
return {
llmContent:
'Plan mode was not entered: the session is in YOLO mode, which the user explicitly chose for low-friction execution. Continue investigating and presenting your plan in the current mode without switching. If the user explicitly asked for plan mode in this turn, retry this tool call with userRequested: true.',
returnDisplay: 'Stayed in YOLO mode (plan mode not entered).',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The YOLO guard's no-op return has no debugLogger call. Other early-return paths (subagent block via logger.warn, catch block via debugLogger.error) log — this one should too. When a user reports "I asked for plan mode and nothing happened," the oncall engineer grepping ENTER_PLAN_MODE finds zero evidence the guard fired.

Suggested change
returnDisplay: 'Stayed in YOLO mode (plan mode not entered).',
debugLogger.info(
'Blocked model-initiated plan entry from YOLO (userRequested=%s)',
this.params.userRequested,
);
return {
llmContent:

— qwen3.7-max via Qwen Code /review

};
}

// In headless (non-interactive) mode without ACP support, the gate
// exit paths require user interaction that cannot be fulfilled.
const isAcpMode =
Expand Down
Loading