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
30 changes: 23 additions & 7 deletions packages/core/src/core/coreToolScheduler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6233,9 +6233,18 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => {
expect(completedCalls[0].response.resultDisplay).toBe(
'Plan mode blocked a non-read-only tool call.',
);
expect(
JSON.stringify(completedCalls[0].response.responseParts),
).toContain('exit_plan_mode tool');
// Response must use error key (not output) so LLM recognizes it as a failure
const responseParts = completedCalls[0].response.responseParts;
const responseJson = JSON.stringify(responseParts);
expect(responseJson).toContain('"error"');
expect(responseJson).toContain('Tool blocked by plan mode');
expect(responseJson).toContain('write_file');
// Plan-required teammates get exit_plan_mode hint
expect(responseJson).toContain('exit_plan_mode');
expect(completedCalls[0].response.error).toBeInstanceOf(Error);
expect(completedCalls[0].response.errorType).toBe(
ToolErrorType.EXECUTION_DENIED,
);
}
});

Expand Down Expand Up @@ -6302,11 +6311,18 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => {
.calls[0][0] as ToolCall[];
expect(completedCalls[0].status).toBe('error');
if (completedCalls[0].status === 'error') {
const responseText = JSON.stringify(
completedCalls[0].response.responseParts,
// SDK, subagent, and teammate paths all get the same error format
// but different guidance: SDK/subagents get "Present your plan directly"
const responseParts = completedCalls[0].response.responseParts;
const responseJson = JSON.stringify(responseParts);
expect(responseJson).toContain('"error"');
expect(responseJson).toContain('Tool blocked by plan mode');
expect(responseJson).toContain('Present your plan directly');
expect(responseJson).not.toContain('exit_plan_mode');
expect(completedCalls[0].response.error).toBeInstanceOf(Error);
expect(completedCalls[0].response.errorType).toBe(
ToolErrorType.EXECUTION_DENIED,
);
expect(responseText).toContain('Present your plan directly');
expect(responseText).not.toContain('exit_plan_mode tool');
}
},
);
Expand Down
32 changes: 18 additions & 14 deletions packages/core/src/core/coreToolScheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ import {
} from '../tools/modifiable-tool.js';
import * as Diff from 'diff';
import levenshtein from 'fast-levenshtein';
import { getPlanModeSystemReminder } from './prompts.js';
import { ShellToolInvocation } from '../tools/shell.js';
import { IdeClient } from '../ide/ide-client.js';
import {
Expand Down Expand Up @@ -2458,22 +2457,27 @@ export class CoreToolScheduler {
isEnterPlanModeTool,
)
) {
// SDK and ordinary subagent-like callers should return plans
// directly; they do not have exit_plan_mode available. Plan-required
// teammates have a dedicated exit_plan_mode approval path.
const isPlanRequiredTeammate =
!shouldUsePlanOnlyReminderInSubagentContext() &&
!this.config.getSdkMode();

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] Setting error and errorType here is the correct fix, but it causes a behavioral side effect: BaseJsonOutputAdapter.ts (lines 1039-1049) records into permission_denials whenever response.error is truthy and errorType === EXECUTION_DENIED. Plan-mode-blocked tool calls were previously invisible to that tracker (both fields were undefined). They will now appear alongside actual user-declined permission denials in SDK/non-interactive JSON output, with no discriminator field (e.g., reason: "plan_mode" vs reason: "user_declined").

If any SDK consumer iterates permission_denials to decide whether to re-prompt the user or abort, plan-mode blocks will be misclassified as user rejections.

Consider either filtering plan-mode blocks out of permissionDenials, or adding a discriminator to CLIPermissionDenial.

— qwen3.7-max via Qwen Code /review

const planModeError = new Error(
`Tool blocked by plan mode: "${reqInfo.name}" is not a read-only tool. ` +
`Only read-only tools (read_file, grep_search, glob, list_directory, ` +
`web_fetch, etc.) are allowed in plan mode.` +
(isPlanRequiredTeammate
? ` Call exit_plan_mode to exit plan mode and execute this tool.`
: ` Present your plan directly to the caller instead of executing this tool.`),
);
this.setStatusInternal(reqInfo.callId, 'error', {
callId: reqInfo.callId,
responseParts: convertToFunctionResponse(
reqInfo.name,
reqInfo.callId,
// SDK callers and ordinary subagent-like callers should
// return plans directly. Plan-required teammates have a
// dedicated exit_plan_mode approval path instead.
getPlanModeSystemReminder(
shouldUsePlanOnlyReminderInSubagentContext() ||
this.config.getSdkMode(),
),
...createErrorResponse(
reqInfo,
planModeError,
ToolErrorType.EXECUTION_DENIED,
),
resultDisplay: 'Plan mode blocked a non-read-only tool call.',
error: undefined,
errorType: undefined,
});
setToolSpanFailure(
toolSpan,
Expand Down
Loading