mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
fix(core): Enforce Plan mode entry boundary (#7248)
* fix(core): Enforce Plan mode entry boundary Co-authored-by: Qwen-Coder <[email protected]> * test: Cover concurrent Plan entry siblings Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
1 parent
20f6977393
commit
2a9f678521
12 files changed
+650
-16
No files matched your search
@@ -0,0 +1,33 @@
|
||||
# Plan mode mid-turn guidance and entry boundary
|
||||
|
||||
## Problem
|
||||
|
||||
`enter_plan_mode` changes the approval mode while a model turn is still being processed. Before this change, a successful invocation returned only a short sentence, so the model did not receive the full Plan mode constraints until a later turn. Sibling tool calls from the same model response could also execute on either side of the mode transition: calls before the entry ran under the previous mode, while calls after it ran after Plan mode became active without being rescheduled against the new boundary.
|
||||
|
||||
## Contract
|
||||
|
||||
A successful or already-active `enter_plan_mode` invocation returns the same complete reminder produced by `getPlanModeSystemReminder()`. SDK sessions receive the SDK-specific variant. Unsolicited YOLO entry, transition failures, and subagent or teammate rejection retain their existing results because no Plan mode transition occurred.
|
||||
|
||||
When a post-deduplication executable batch contains more than one call and one call is `enter_plan_mode`, the first entry call is an execution boundary. Only that call is eligible to execute. Every other executable sibling, regardless of whether it appeared before or after the entry, receives a terminal `EXECUTION_DENIED` response instructing the model to retry in the next turn after observing the new approval mode. Entry failure or idempotent success does not release the siblings.
|
||||
|
||||
Existing terminal decisions take precedence. Loop detection still rejects the whole batch first. Duplicate provider responses are emitted in their original positions but are not executable siblings. In structured-output mode, the existing structured-output pre-scan remains terminal and suppresses `enter_plan_mode` along with other non-structured calls.
|
||||
|
||||
`exit_plan_mode` is not an execution boundary in this change. Its explicit user approval and stale-context protections are independent.
|
||||
|
||||
## Integration
|
||||
|
||||
The core scheduler applies the boundary after call-ID deduplication and canonical-name resolution, before permission checks, registry lookup, hooks, or invocation construction. Skipped calls therefore do not request permissions or run per-tool hooks. They remain terminal batch results so the existing completion callback, recording, telemetry, and `PostToolBatch` audit path observe a complete response for every accepted call ID. Runtime-specific content-generator views are cleaned with the other terminal results.
|
||||
|
||||
ACP applies the same policy after loop and duplicate-provider handling and before executing its sequential or Agent batches. Duplicate responses remain ordered. ACP does not introduce a `PostToolBatch` hook because that path intentionally does not support one.
|
||||
|
||||
Headless mode applies the policy after duplicate and structured-output filtering. Skipped calls are emitted and returned as denied tool results in their original order, but do not consume `--max-tool-calls` budget. The entry itself follows the normal budget and abort behavior.
|
||||
|
||||
## Output preservation
|
||||
|
||||
The reminder is lifecycle policy, not ordinary tool payload. `enter_plan_mode` declares an infinite per-tool output limit, is exempt from the scheduler's persistence spill gate, and is not a candidate for aggregate batch offloading. These three protections prevent the policy from being truncated, replaced with a file pointer, or reduced to a preview before the next model turn.
|
||||
|
||||
## Validation
|
||||
|
||||
Unit coverage verifies exact DEFAULT and SDK reminders, success and idempotent entry, first-entry selection, sibling denial on both sides, duplicate-provider ordering, headless budget accounting, full-reminder preservation under deliberately tiny output thresholds, `PostToolBatch` visibility, and runtime-view cleanup. Existing scheduler, ACP, and headless suites cover the surrounding permission, loop, duplicate, structured-output, and abort behavior.
|
||||
|
||||
Managed-host validation should confirm that the ACP client receives one result for every tool call and that the next model request contains the complete reminder plus sibling denial responses. This validation requires a deployed build and a host session ID; it is not simulated by changing production routing in this PR.
|
||||
@@ -13258,6 +13258,100 @@ describe('Session', () => {
|
||||
};
|
||||
}
|
||||
|
||||
it('isolates enter_plan_mode from executable ACP siblings while preserving duplicate responses', async () => {
|
||||
const writeExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'wrote',
|
||||
returnDisplay: 'wrote',
|
||||
});
|
||||
const enterExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'entered plan mode',
|
||||
returnDisplay: 'entered plan mode',
|
||||
});
|
||||
const readExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'read',
|
||||
returnDisplay: 'read',
|
||||
});
|
||||
mockToolRegistry.getTool.mockImplementation((name: string) => {
|
||||
const execute =
|
||||
name === core.ToolNames.ENTER_PLAN_MODE
|
||||
? enterExecute
|
||||
: name === core.ToolNames.WRITE_FILE
|
||||
? writeExecute
|
||||
: readExecute;
|
||||
return mockAllowedTool(name, execute);
|
||||
});
|
||||
const historyIds = new Set(['duplicate_read']);
|
||||
vi.mocked(mockChat.getHistoryFunctionResponseIds).mockReturnValue(
|
||||
historyIds,
|
||||
);
|
||||
const [duplicatePart] = core.normalizeModelToolCallIds(
|
||||
[
|
||||
{
|
||||
functionCall: {
|
||||
id: 'duplicate_read',
|
||||
name: core.ToolNames.READ_FILE,
|
||||
args: { file_path: 'duplicate.ts' },
|
||||
},
|
||||
},
|
||||
],
|
||||
historyIds,
|
||||
new Set<string>(),
|
||||
);
|
||||
|
||||
const result = await (
|
||||
session as unknown as ToolCallInternals
|
||||
).runToolCalls(new AbortController().signal, 'prompt-plan-boundary', [
|
||||
{
|
||||
id: 'write_before_entry',
|
||||
name: core.ToolNames.WRITE_FILE,
|
||||
args: { file_path: 'before.txt' },
|
||||
},
|
||||
duplicatePart.functionCall!,
|
||||
{
|
||||
id: 'enter_plan',
|
||||
name: core.ToolNames.ENTER_PLAN_MODE,
|
||||
args: {},
|
||||
},
|
||||
{
|
||||
id: 'read_after_entry',
|
||||
name: core.ToolNames.READ_FILE,
|
||||
args: { file_path: 'after.ts' },
|
||||
},
|
||||
]);
|
||||
|
||||
expect(writeExecute).not.toHaveBeenCalled();
|
||||
expect(enterExecute).toHaveBeenCalledOnce();
|
||||
expect(readExecute).not.toHaveBeenCalled();
|
||||
expect(result.parts.map((part) => part.functionResponse?.id)).toEqual([
|
||||
'write_before_entry',
|
||||
'duplicate_read__qwen_dup_2',
|
||||
'enter_plan',
|
||||
'read_after_entry',
|
||||
]);
|
||||
expect(result.parts[0].functionResponse?.response).toEqual({
|
||||
error: core.PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
});
|
||||
expect(result.parts[1].functionResponse?.response).toEqual({
|
||||
error: expect.stringContaining(
|
||||
'Duplicate provider tool call id "duplicate_read"',
|
||||
),
|
||||
});
|
||||
expect(result.parts[2].functionResponse?.response).toEqual({
|
||||
output: 'entered plan mode',
|
||||
});
|
||||
expect(result.parts[3].functionResponse?.response).toEqual({
|
||||
error: core.PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
});
|
||||
expect(mockChatRecordingService.recordToolResult).toHaveBeenCalledWith(
|
||||
[result.parts[0]],
|
||||
expect.objectContaining({
|
||||
callId: 'write_before_entry',
|
||||
status: 'error',
|
||||
errorType: core.ToolErrorType.EXECUTION_DENIED,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps a structured timeout as an error after a later parent abort', async () => {
|
||||
const parentController = new AbortController();
|
||||
const messageBus = {
|
||||
|
||||
@@ -51,8 +51,10 @@ import {
|
||||
convertToFunctionErrorResponse,
|
||||
convertToFunctionResponse,
|
||||
createDuplicateProviderToolCallResponse,
|
||||
findPlanModeEntryBatchBoundaryIndex,
|
||||
findRepeatedDuplicateProviderToolCall,
|
||||
markDuplicateProviderToolCallResponseSent,
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
createDebugLogger,
|
||||
DiscoveredMCPTool,
|
||||
StreamEventType,
|
||||
@@ -5374,6 +5376,7 @@ export class Session implements SessionContext {
|
||||
fc: FunctionCall,
|
||||
message = PERMISSION_CANCEL_SKIP_MESSAGE,
|
||||
emitStart = true,
|
||||
errorType?: ToolErrorType,
|
||||
): Promise<Part> => {
|
||||
const toolName = fc.name ?? 'unknown_tool';
|
||||
const callId = fc.id ?? `${toolName}-skip-${++skippedToolCallCounter}`;
|
||||
@@ -5391,7 +5394,7 @@ export class Session implements SessionContext {
|
||||
status: 'error',
|
||||
resultDisplay: undefined,
|
||||
error,
|
||||
errorType: undefined,
|
||||
errorType,
|
||||
});
|
||||
if (emitStart) {
|
||||
await this.toolCallEmitter.emitStart({
|
||||
@@ -5551,6 +5554,17 @@ export class Session implements SessionContext {
|
||||
}
|
||||
}
|
||||
|
||||
const executableCalls = batches.flatMap((batch) =>
|
||||
batch.kind === 'execute' ? batch.calls : [],
|
||||
);
|
||||
const planModeEntryBoundaryIndex = findPlanModeEntryBatchBoundaryIndex(
|
||||
executableCalls.map((call) => call.name),
|
||||
);
|
||||
const planModeEntryBoundary =
|
||||
planModeEntryBoundaryIndex === undefined
|
||||
? undefined
|
||||
: executableCalls[planModeEntryBoundaryIndex];
|
||||
|
||||
const appendSkippedAfter = async (
|
||||
parts: Part[],
|
||||
fc: FunctionCall,
|
||||
@@ -5712,6 +5726,22 @@ export class Session implements SessionContext {
|
||||
parts.push(...batch.response.responseParts);
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
planModeEntryBoundary &&
|
||||
!batch.calls.includes(planModeEntryBoundary)
|
||||
) {
|
||||
for (const fc of batch.calls) {
|
||||
parts.push(
|
||||
await recordSkippedToolCall(
|
||||
fc,
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
true,
|
||||
ToolErrorType.EXECUTION_DENIED,
|
||||
),
|
||||
);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (batch.concurrent && batch.calls.length > 1) {
|
||||
const batchAbortController = new AbortController();
|
||||
let batchStopAfterPermissionCancel = false;
|
||||
|
||||
@@ -32,6 +32,8 @@ import {
|
||||
LOOP_SENTINEL_DYNAMIC,
|
||||
TeamEventType,
|
||||
ToolConfirmationOutcome,
|
||||
ToolNames,
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import type { Part } from '@google/genai';
|
||||
import { EventEmitter } from 'node:events';
|
||||
@@ -1205,6 +1207,94 @@ describe('runNonInteractive', () => {
|
||||
}));
|
||||
}
|
||||
|
||||
it('isolates enter_plan_mode from headless siblings without charging skipped calls to the budget', async () => {
|
||||
setupMetricsMock();
|
||||
vi.mocked(mockConfig.getMaxToolCalls).mockReturnValue(1);
|
||||
vi.mocked(mockToolRegistry.getTool).mockImplementation(
|
||||
(name: string) =>
|
||||
({
|
||||
kind:
|
||||
name === ToolNames.READ_FILE
|
||||
? Kind.Read
|
||||
: name === ToolNames.WRITE_FILE
|
||||
? Kind.Edit
|
||||
: Kind.Other,
|
||||
}) as unknown as ReturnType<typeof mockToolRegistry.getTool>,
|
||||
);
|
||||
mockCoreExecuteToolCall.mockImplementation(
|
||||
async (
|
||||
_config: unknown,
|
||||
request: { callId: string; name: string },
|
||||
) => ({
|
||||
responseParts: [
|
||||
{
|
||||
functionResponse: {
|
||||
id: request.callId,
|
||||
name: request.name,
|
||||
response: { output: 'entered plan mode' },
|
||||
},
|
||||
},
|
||||
],
|
||||
resultDisplay: 'entered plan mode',
|
||||
}),
|
||||
);
|
||||
|
||||
mockGeminiClient.sendMessageStream
|
||||
.mockReturnValueOnce(
|
||||
createStreamFromEvents([
|
||||
...toolCallEvents(
|
||||
['write-before-entry'],
|
||||
ToolNames.WRITE_FILE,
|
||||
'p-plan-boundary',
|
||||
),
|
||||
...toolCallEvents(
|
||||
['enter-plan'],
|
||||
ToolNames.ENTER_PLAN_MODE,
|
||||
'p-plan-boundary',
|
||||
),
|
||||
...toolCallEvents(
|
||||
['read-after-entry-1', 'read-after-entry-2'],
|
||||
ToolNames.READ_FILE,
|
||||
'p-plan-boundary',
|
||||
),
|
||||
]),
|
||||
)
|
||||
.mockReturnValueOnce(createStreamFromEvents(finishTurn));
|
||||
|
||||
await runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'enter plan mode',
|
||||
'p-plan-boundary',
|
||||
);
|
||||
|
||||
expect(mockCoreExecuteToolCall).toHaveBeenCalledOnce();
|
||||
expect(mockCoreExecuteToolCall.mock.calls[0][1]).toMatchObject({
|
||||
callId: 'enter-plan',
|
||||
name: ToolNames.ENTER_PLAN_MODE,
|
||||
});
|
||||
const nextTurnParts = mockGeminiClient.sendMessageStream.mock
|
||||
.calls[1][0] as Part[];
|
||||
expect(nextTurnParts.map((part) => part.functionResponse?.id)).toEqual([
|
||||
'write-before-entry',
|
||||
'enter-plan',
|
||||
'read-after-entry-1',
|
||||
'read-after-entry-2',
|
||||
]);
|
||||
expect(nextTurnParts[0].functionResponse?.response).toEqual({
|
||||
error: PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
});
|
||||
expect(nextTurnParts[1].functionResponse?.response).toEqual({
|
||||
output: 'entered plan mode',
|
||||
});
|
||||
expect(nextTurnParts[2].functionResponse?.response).toEqual({
|
||||
error: PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
});
|
||||
expect(nextTurnParts[3].functionResponse?.response).toEqual({
|
||||
error: PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
});
|
||||
});
|
||||
|
||||
it('runs a batch of concurrency-safe tool calls concurrently', async () => {
|
||||
setupMetricsMock();
|
||||
// Kind.Read is concurrency-safe, so the whole batch is one parallel
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
ApprovalMode,
|
||||
ToolConfirmationOutcome,
|
||||
createDuplicateProviderToolCallResponse,
|
||||
findPlanModeEntryBatchBoundaryIndex,
|
||||
isSystemReminderContent,
|
||||
markDuplicateProviderToolCallResponseSent,
|
||||
findRepeatedDuplicateProviderToolCall,
|
||||
@@ -46,6 +47,8 @@ import {
|
||||
canonicalToolName,
|
||||
parsePositiveIntegerEnv,
|
||||
partitionByConcurrencySafety,
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
ToolErrorType,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import type { Content, Part, PartListUnion } from '@google/genai';
|
||||
import type { CLIUserMessage, PermissionMode } from './nonInteractive/types.js';
|
||||
@@ -1167,6 +1170,13 @@ export async function runNonInteractive(
|
||||
(r) => r.name === ToolNames.STRUCTURED_OUTPUT,
|
||||
);
|
||||
}
|
||||
const planModeEntryBoundaryIndex = findPlanModeEntryBatchBoundaryIndex(
|
||||
requestsToExecute.map((request) => request.name),
|
||||
);
|
||||
const planModeEntryBoundary =
|
||||
planModeEntryBoundaryIndex === undefined
|
||||
? undefined
|
||||
: requestsToExecute[planModeEntryBoundaryIndex];
|
||||
const executedRequests = new Set<ToolCallRequestInfo>(
|
||||
respondedRequests,
|
||||
);
|
||||
@@ -1333,6 +1343,30 @@ export async function runNonInteractive(
|
||||
return false;
|
||||
};
|
||||
|
||||
const finalizePlanModeEntrySiblingSkip = (
|
||||
requestInfo: ToolCallRequestInfo,
|
||||
): void => {
|
||||
const error = new Error(PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE);
|
||||
const responseParts: Part[] = [
|
||||
{
|
||||
functionResponse: {
|
||||
id: requestInfo.callId,
|
||||
name: requestInfo.name,
|
||||
response: { error: error.message },
|
||||
},
|
||||
},
|
||||
];
|
||||
adapter.emitToolResult(requestInfo, {
|
||||
callId: requestInfo.callId,
|
||||
responseParts,
|
||||
resultDisplay: error.message,
|
||||
error,
|
||||
errorType: ToolErrorType.EXECUTION_DENIED,
|
||||
});
|
||||
executedRequests.add(requestInfo);
|
||||
toolResponseParts.push(...responseParts);
|
||||
};
|
||||
|
||||
const maxToolConcurrency = parsePositiveIntegerEnv(
|
||||
process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'],
|
||||
10,
|
||||
@@ -1355,6 +1389,13 @@ export async function runNonInteractive(
|
||||
}> = [];
|
||||
const inFlight = new Set<Promise<void>>();
|
||||
for (const requestInfo of batch.calls) {
|
||||
if (
|
||||
planModeEntryBoundary &&
|
||||
requestInfo !== planModeEntryBoundary
|
||||
) {
|
||||
finalizePlanModeEntrySiblingSkip(requestInfo);
|
||||
continue;
|
||||
}
|
||||
if (!isBudgetExempt(requestInfo)) {
|
||||
budgetEnforcer.tickToolCall();
|
||||
}
|
||||
@@ -1420,6 +1461,13 @@ export async function runNonInteractive(
|
||||
// Sequential batch (a single tool, or a side-effecting tool):
|
||||
// identical to the pre-parallelisation behaviour.
|
||||
for (const requestInfo of batch.calls) {
|
||||
if (
|
||||
planModeEntryBoundary &&
|
||||
requestInfo !== planModeEntryBoundary
|
||||
) {
|
||||
finalizePlanModeEntrySiblingSkip(requestInfo);
|
||||
continue;
|
||||
}
|
||||
if (!isBudgetExempt(requestInfo)) {
|
||||
budgetEnforcer.tickToolCall();
|
||||
}
|
||||
|
||||
@@ -35,6 +35,7 @@ import { SkillTool } from '../tools/skill.js';
|
||||
import { StructuredToolError } from '../tools/priorReadEnforcement.js';
|
||||
import { ToolNames, ToolNamesMigration } from '../tools/tool-names.js';
|
||||
import type {
|
||||
CompletedToolCall,
|
||||
ExecutingToolCall,
|
||||
ToolCall,
|
||||
WaitingToolCall,
|
||||
@@ -71,6 +72,8 @@ import {
|
||||
} from '../agents/runtime/agent-context.js';
|
||||
import { runWithTeammateIdentity } from '../agents/team/identity.js';
|
||||
import { normalizeToolNameForProvider } from '../utils/tool-name-utils.js';
|
||||
import { getPlanModeSystemReminder } from './prompts.js';
|
||||
import { PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE } from './plan-mode-entry-policy.js';
|
||||
|
||||
type ToolSpanRecord = {
|
||||
name: string;
|
||||
@@ -707,6 +710,8 @@ describe('CoreToolScheduler', () => {
|
||||
onToolCallsUpdate?: ReturnType<typeof vi.fn>;
|
||||
memoryMonitor?: { scheduleCheck: () => void };
|
||||
toolOutputBatchBudget?: number;
|
||||
truncateToolOutputThreshold?: number;
|
||||
truncateToolOutputLines?: number;
|
||||
}) {
|
||||
const ensureTool = vi.fn(
|
||||
async (name: string) =>
|
||||
@@ -754,8 +759,10 @@ describe('CoreToolScheduler', () => {
|
||||
getToolResultBytesWritten: () => 0,
|
||||
trackToolResultBytes: vi.fn(),
|
||||
getTruncateToolOutputThreshold: () =>
|
||||
options.truncateToolOutputThreshold ??
|
||||
DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD,
|
||||
getTruncateToolOutputLines: () => DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES,
|
||||
getTruncateToolOutputLines: () =>
|
||||
options.truncateToolOutputLines ?? DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES,
|
||||
getToolOutputBatchBudget: () =>
|
||||
options.toolOutputBatchBudget ?? Number.POSITIVE_INFINITY,
|
||||
getToolRegistry: () => mockToolRegistry,
|
||||
@@ -800,6 +807,221 @@ describe('CoreToolScheduler', () => {
|
||||
};
|
||||
}
|
||||
|
||||
it('isolates enter_plan_mode as a batch boundary and preserves its full reminder', async () => {
|
||||
const reminder = getPlanModeSystemReminder(false);
|
||||
const writeExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'wrote',
|
||||
returnDisplay: 'wrote',
|
||||
});
|
||||
const enterExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: reminder,
|
||||
returnDisplay: 'Entered plan mode.',
|
||||
});
|
||||
const readExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'read',
|
||||
returnDisplay: 'read',
|
||||
});
|
||||
const messageBus = {
|
||||
request: vi.fn().mockImplementation(
|
||||
async (request: {
|
||||
eventName: string;
|
||||
}): Promise<HookExecutionResponse> => ({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: `${request.eventName}-hook`,
|
||||
success: true,
|
||||
output: { decision: 'allow' },
|
||||
}),
|
||||
),
|
||||
};
|
||||
const toolsByName = new Map<string, MockTool>([
|
||||
[
|
||||
ToolNames.WRITE_FILE,
|
||||
new MockTool({ name: ToolNames.WRITE_FILE, execute: writeExecute }),
|
||||
],
|
||||
[
|
||||
ToolNames.ENTER_PLAN_MODE,
|
||||
new MockTool({
|
||||
name: ToolNames.ENTER_PLAN_MODE,
|
||||
maxOutputChars: Number.POSITIVE_INFINITY,
|
||||
execute: enterExecute,
|
||||
}),
|
||||
],
|
||||
[
|
||||
ToolNames.READ_FILE,
|
||||
new MockTool({ name: ToolNames.READ_FILE, execute: readExecute }),
|
||||
],
|
||||
]);
|
||||
const { scheduler, ensureTool, onAllToolCallsComplete } =
|
||||
createSchedulerForLegacyToolTests({
|
||||
toolsByName,
|
||||
messageBus,
|
||||
disableHooks: false,
|
||||
truncateToolOutputThreshold: 1,
|
||||
truncateToolOutputLines: 1,
|
||||
toolOutputBatchBudget: 1,
|
||||
});
|
||||
const runtimeView = {
|
||||
contentGenerator: {},
|
||||
contentGeneratorConfig: { model: 'vision-agent' },
|
||||
} as RuntimeContentGeneratorView;
|
||||
|
||||
await scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'write-before-entry',
|
||||
name: ToolNames.WRITE_FILE,
|
||||
args: { file_path: 'before.txt' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-plan-boundary',
|
||||
},
|
||||
{
|
||||
callId: 'enter-plan',
|
||||
name: ToolNames.ENTER_PLAN_MODE,
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-plan-boundary',
|
||||
},
|
||||
{
|
||||
callId: 'read-after-entry',
|
||||
name: ToolNames.READ_FILE,
|
||||
args: { file_path: 'after.txt' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-plan-boundary',
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
runtimeView,
|
||||
);
|
||||
|
||||
expect(ensureTool).toHaveBeenCalledOnce();
|
||||
expect(ensureTool).toHaveBeenCalledWith(ToolNames.ENTER_PLAN_MODE);
|
||||
expect(writeExecute).not.toHaveBeenCalled();
|
||||
expect(enterExecute).toHaveBeenCalledOnce();
|
||||
expect(readExecute).not.toHaveBeenCalled();
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalledOnce();
|
||||
});
|
||||
const completedCalls = onAllToolCallsComplete.mock
|
||||
.calls[0][0] as CompletedToolCall[];
|
||||
expect(completedCalls.map((call) => call.request.callId)).toEqual([
|
||||
'write-before-entry',
|
||||
'enter-plan',
|
||||
'read-after-entry',
|
||||
]);
|
||||
expect(completedCalls.map((call) => call.status)).toEqual([
|
||||
'error',
|
||||
'success',
|
||||
'error',
|
||||
]);
|
||||
const [writeCall, enterCall, readCall] = completedCalls;
|
||||
expect(writeCall.response.error?.message).toBe(
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
);
|
||||
expect(writeCall.response.errorType).toBe(ToolErrorType.EXECUTION_DENIED);
|
||||
expect(
|
||||
enterCall.response.responseParts[0]?.functionResponse?.response?.[
|
||||
'output'
|
||||
],
|
||||
).toBe(reminder);
|
||||
expect(readCall.response.error?.message).toBe(
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
);
|
||||
expect(readCall.response.errorType).toBe(ToolErrorType.EXECUTION_DENIED);
|
||||
|
||||
const postBatchRequest = messageBus.request.mock.calls.find(
|
||||
([request]) => request.eventName === 'PostToolBatch',
|
||||
)?.[0];
|
||||
expect(
|
||||
postBatchRequest.input.tool_calls.map(
|
||||
(call: { status: string }) => call.status,
|
||||
),
|
||||
).toEqual(['error', 'success', 'error']);
|
||||
expect(
|
||||
(
|
||||
scheduler as unknown as {
|
||||
runtimeContentGeneratorViews: Map<
|
||||
string,
|
||||
RuntimeContentGeneratorView
|
||||
>;
|
||||
}
|
||||
).runtimeContentGeneratorViews.size,
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
it('keeps siblings suppressed when enter_plan_mode itself fails', async () => {
|
||||
const enterExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'Failed to enter plan mode: transition failed',
|
||||
returnDisplay: 'Failed to enter plan mode: transition failed',
|
||||
error: {
|
||||
message: 'Failed to enter plan mode: transition failed',
|
||||
type: ToolErrorType.EXECUTION_FAILED,
|
||||
},
|
||||
});
|
||||
const writeExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'wrote',
|
||||
returnDisplay: 'wrote',
|
||||
});
|
||||
const { scheduler, ensureTool, onAllToolCallsComplete } =
|
||||
createSchedulerForLegacyToolTests({
|
||||
toolsByName: new Map([
|
||||
[
|
||||
ToolNames.ENTER_PLAN_MODE,
|
||||
new MockTool({
|
||||
name: ToolNames.ENTER_PLAN_MODE,
|
||||
execute: enterExecute,
|
||||
}),
|
||||
],
|
||||
[
|
||||
ToolNames.WRITE_FILE,
|
||||
new MockTool({
|
||||
name: ToolNames.WRITE_FILE,
|
||||
execute: writeExecute,
|
||||
}),
|
||||
],
|
||||
]),
|
||||
});
|
||||
|
||||
await scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'failed-entry',
|
||||
name: ToolNames.ENTER_PLAN_MODE,
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-failed-entry',
|
||||
},
|
||||
{
|
||||
callId: 'write-after-failed-entry',
|
||||
name: ToolNames.WRITE_FILE,
|
||||
args: { file_path: 'blocked.txt' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-failed-entry',
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalledOnce();
|
||||
});
|
||||
expect(ensureTool).toHaveBeenCalledOnce();
|
||||
expect(enterExecute).toHaveBeenCalledOnce();
|
||||
expect(writeExecute).not.toHaveBeenCalled();
|
||||
const completedCalls = onAllToolCallsComplete.mock
|
||||
.calls[0][0] as CompletedToolCall[];
|
||||
expect(completedCalls.map((call) => call.status)).toEqual([
|
||||
'error',
|
||||
'error',
|
||||
]);
|
||||
expect(completedCalls[0].response.error?.message).toBe(
|
||||
'Failed to enter plan mode: transition failed',
|
||||
);
|
||||
expect(completedCalls[1].response.error?.message).toBe(
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps interaction-required tools awaiting approval despite YOLO and an allow hook', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'executed',
|
||||
|
||||
@@ -89,6 +89,10 @@ import {
|
||||
validatePlanModeShellApproval,
|
||||
validatePlanModeShellContext,
|
||||
} from './plan-mode-shell-policy.js';
|
||||
import {
|
||||
findPlanModeEntryBatchBoundaryIndex,
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
} from './plan-mode-entry-policy.js';
|
||||
import {
|
||||
applyAutoModeDecision,
|
||||
evaluateAutoMode,
|
||||
@@ -181,14 +185,14 @@ function dedupeRequestsByCallId(
|
||||
// the headroom ensures the gate only fires for genuinely un-truncated output
|
||||
// and must exceed the stub size (~2.3K) to avoid cascading re-persistence.
|
||||
const GATE_HEADROOM = 3000;
|
||||
// Tools that bound their own output and must bypass the persistence gate.
|
||||
// read_file pages/truncates itself; read_mcp_resource caps text in
|
||||
// formatMcpResourceContents and sets maxOutputChars=Infinity — but this gate
|
||||
// runs first, so without the exemption a 28k–100k resource is spilled to a
|
||||
// stub before that self-cap takes effect and the model never sees the body.
|
||||
// Tools whose output must bypass the persistence gate. read_file pages itself,
|
||||
// and read_mcp_resource caps text in formatMcpResourceContents. enter_plan_mode
|
||||
// returns lifecycle policy that must remain inline. This gate runs before
|
||||
// per-tool limits, so each requires an explicit exemption here.
|
||||
const GATE_EXEMPT_TOOLS = new Set<string>([
|
||||
ToolNames.READ_FILE,
|
||||
ToolNames.READ_MCP_RESOURCE,
|
||||
ToolNames.ENTER_PLAN_MODE,
|
||||
]);
|
||||
|
||||
function extractTextFromPartListUnion(c: PartListUnion): string {
|
||||
@@ -2083,6 +2087,9 @@ export class CoreToolScheduler {
|
||||
const requestsToProcess = dedupeRequestsByCallId(
|
||||
Array.isArray(request) ? request : [request],
|
||||
);
|
||||
const planModeEntryBoundaryIndex = findPlanModeEntryBatchBoundaryIndex(
|
||||
requestsToProcess.map((item) => canonicalToolName(item.name)),
|
||||
);
|
||||
|
||||
// Prune validation retry state per-tool, not wholesale. Keys are
|
||||
// "<toolName>:<errorMessage>"; retain counters only for tools actually
|
||||
@@ -2102,7 +2109,24 @@ export class CoreToolScheduler {
|
||||
}
|
||||
|
||||
const newToolCalls: ToolCall[] = [];
|
||||
for (const reqInfo of requestsToProcess) {
|
||||
for (const [requestIndex, reqInfo] of requestsToProcess.entries()) {
|
||||
if (
|
||||
planModeEntryBoundaryIndex !== undefined &&
|
||||
requestIndex !== planModeEntryBoundaryIndex
|
||||
) {
|
||||
newToolCalls.push({
|
||||
status: 'error',
|
||||
request: reqInfo,
|
||||
response: createErrorResponse(
|
||||
reqInfo,
|
||||
new Error(PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE),
|
||||
ToolErrorType.EXECUTION_DENIED,
|
||||
),
|
||||
durationMs: 0,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const canonicalName = canonicalToolName(reqInfo.name);
|
||||
|
||||
// Check if the tool is excluded due to permissions/environment restrictions
|
||||
@@ -4966,7 +4990,7 @@ export class CoreToolScheduler {
|
||||
toolName: string,
|
||||
content: PartListUnion,
|
||||
): Promise<PartListUnion> {
|
||||
if (GATE_EXEMPT_TOOLS.has(toolName)) return content;
|
||||
if (GATE_EXEMPT_TOOLS.has(canonicalToolName(toolName))) return content;
|
||||
|
||||
const text = extractTextFromPartListUnion(content);
|
||||
if (!text || isAlreadyTruncated(text)) return content;
|
||||
@@ -5086,6 +5110,10 @@ export class CoreToolScheduler {
|
||||
private async offloadCallOutput(
|
||||
call: CompletedToolCall,
|
||||
): Promise<CompletedToolCall | null> {
|
||||
if (canonicalToolName(call.request.name) === ToolNames.ENTER_PLAN_MODE) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const parts = call.response.responseParts;
|
||||
if (parts.length !== 1) return null;
|
||||
const fr = parts[0]?.functionResponse;
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { ToolNames } from '../tools/tool-names.js';
|
||||
import { findPlanModeEntryBatchBoundaryIndex } from './plan-mode-entry-policy.js';
|
||||
|
||||
describe('findPlanModeEntryBatchBoundaryIndex', () => {
|
||||
it.each([
|
||||
{ names: [], expected: undefined },
|
||||
{ names: [ToolNames.ENTER_PLAN_MODE], expected: undefined },
|
||||
{ names: [ToolNames.READ_FILE], expected: undefined },
|
||||
{
|
||||
names: [ToolNames.ENTER_PLAN_MODE, ToolNames.READ_FILE],
|
||||
expected: 0,
|
||||
},
|
||||
{
|
||||
names: [ToolNames.WRITE_FILE, ToolNames.ENTER_PLAN_MODE],
|
||||
expected: 1,
|
||||
},
|
||||
{
|
||||
names: [
|
||||
ToolNames.READ_FILE,
|
||||
ToolNames.ENTER_PLAN_MODE,
|
||||
ToolNames.WRITE_FILE,
|
||||
],
|
||||
expected: 1,
|
||||
},
|
||||
{
|
||||
names: [ToolNames.ENTER_PLAN_MODE, ToolNames.ENTER_PLAN_MODE],
|
||||
expected: 0,
|
||||
},
|
||||
{
|
||||
names: [undefined, ToolNames.ENTER_PLAN_MODE],
|
||||
expected: 1,
|
||||
},
|
||||
])('returns $expected for $names', ({ names, expected }) => {
|
||||
expect(findPlanModeEntryBatchBoundaryIndex(names)).toBe(expected);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Qwen
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { ToolNames } from '../tools/tool-names.js';
|
||||
|
||||
export const PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE =
|
||||
'Tool call skipped because enter_plan_mode is an execution boundary. Retry it in the next model turn after observing the resulting approval mode.';
|
||||
|
||||
export function findPlanModeEntryBatchBoundaryIndex(
|
||||
toolNames: ReadonlyArray<string | undefined>,
|
||||
): number | undefined {
|
||||
if (toolNames.length <= 1) return undefined;
|
||||
|
||||
const index = toolNames.indexOf(ToolNames.ENTER_PLAN_MODE);
|
||||
return index === -1 ? undefined : index;
|
||||
}
|
||||
@@ -75,6 +75,11 @@ export {
|
||||
decoratePlanModeShellConfirmation,
|
||||
validatePlanModeShellApproval,
|
||||
} from './core/plan-mode-shell-policy.js';
|
||||
/** @internal */
|
||||
export {
|
||||
PLAN_MODE_ENTRY_SIBLING_SKIP_MESSAGE,
|
||||
findPlanModeEntryBatchBoundaryIndex,
|
||||
} from './core/plan-mode-entry-policy.js';
|
||||
export * from './core/geminiChat.js';
|
||||
export * from './core/geminiRequest.js';
|
||||
export * from './core/inlineMediaLimit.js';
|
||||
|
||||
@@ -9,6 +9,7 @@ import { EnterPlanModeTool } from './enterPlanMode.js';
|
||||
import { ApprovalMode, type Config } from '../config/config.js';
|
||||
import { runWithAgentContext } from '../agents/runtime/agent-context.js';
|
||||
import { runWithTeammateIdentity } from '../agents/team/identity.js';
|
||||
import { getPlanModeSystemReminder } from '../core/prompts.js';
|
||||
|
||||
describe('EnterPlanModeTool', () => {
|
||||
let tool: EnterPlanModeTool;
|
||||
@@ -31,6 +32,7 @@ describe('EnterPlanModeTool', () => {
|
||||
isInteractive: vi.fn(() => true),
|
||||
getExperimentalZedIntegration: vi.fn(() => false),
|
||||
getInputFormat: vi.fn(() => undefined),
|
||||
getSdkMode: vi.fn(() => false),
|
||||
} as unknown as Config;
|
||||
|
||||
tool = new EnterPlanModeTool(mockConfig);
|
||||
@@ -67,6 +69,10 @@ describe('EnterPlanModeTool', () => {
|
||||
expect(tool.shouldDefer).toBe(false);
|
||||
});
|
||||
|
||||
it('should exempt the complete reminder from output truncation', () => {
|
||||
expect(tool.maxOutputChars).toBe(Number.POSITIVE_INFINITY);
|
||||
});
|
||||
|
||||
it('should expose only the userRequested flag in its schema', () => {
|
||||
expect(tool.schema.parametersJsonSchema).toEqual({
|
||||
type: 'object',
|
||||
@@ -101,7 +107,7 @@ describe('EnterPlanModeTool', () => {
|
||||
);
|
||||
expect(approvalMode).toBe(ApprovalMode.PLAN);
|
||||
expect(savedPrePlanMode).toBe(ApprovalMode.DEFAULT);
|
||||
expect(result.llmContent).toContain('Plan mode is now active');
|
||||
expect(result.llmContent).toBe(getPlanModeSystemReminder(false));
|
||||
});
|
||||
|
||||
it('should switch from AUTO_EDIT to PLAN', async () => {
|
||||
@@ -168,7 +174,7 @@ describe('EnterPlanModeTool', () => {
|
||||
);
|
||||
expect(approvalMode).toBe(ApprovalMode.PLAN);
|
||||
expect(savedPrePlanMode).toBe(ApprovalMode.DEFAULT);
|
||||
expect(result.llmContent).toContain('Plan mode is now active');
|
||||
expect(result.llmContent).toBe(getPlanModeSystemReminder(false));
|
||||
});
|
||||
|
||||
it('should switch from YOLO to PLAN when the user explicitly requested it', async () => {
|
||||
@@ -186,7 +192,7 @@ describe('EnterPlanModeTool', () => {
|
||||
);
|
||||
expect(approvalMode).toBe(ApprovalMode.PLAN);
|
||||
expect(savedPrePlanMode).toBe(ApprovalMode.YOLO);
|
||||
expect(result.llmContent).toContain('Plan mode is now active');
|
||||
expect(result.llmContent).toBe(getPlanModeSystemReminder(false));
|
||||
});
|
||||
|
||||
it('should honour a user-requested YOLO entry in an ACP session', async () => {
|
||||
@@ -206,7 +212,19 @@ describe('EnterPlanModeTool', () => {
|
||||
ApprovalMode.PLAN,
|
||||
);
|
||||
expect(approvalMode).toBe(ApprovalMode.PLAN);
|
||||
expect(result.llmContent).not.toContain('non-interactive');
|
||||
expect(result.llmContent).toBe(getPlanModeSystemReminder(false));
|
||||
});
|
||||
|
||||
it('should return plan-only guidance in SDK mode', async () => {
|
||||
(mockConfig.getSdkMode as ReturnType<typeof vi.fn>).mockReturnValue(true);
|
||||
const invocation = tool.build({});
|
||||
const result = await invocation.execute(new AbortController().signal);
|
||||
|
||||
expect(result.llmContent).toBe(getPlanModeSystemReminder(true));
|
||||
expect(result.llmContent).toContain('Present your plan directly');
|
||||
expect(result.llmContent).not.toContain(
|
||||
'by calling the exit_plan_mode tool',
|
||||
);
|
||||
});
|
||||
|
||||
it('should be idempotent: already in PLAN does not call setApprovalMode', async () => {
|
||||
@@ -217,7 +235,7 @@ describe('EnterPlanModeTool', () => {
|
||||
|
||||
expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
|
||||
expect(savedPrePlanMode).toBe(ApprovalMode.AUTO);
|
||||
expect(result.llmContent).toContain('Plan mode is now active');
|
||||
expect(result.llmContent).toBe(getPlanModeSystemReminder(false));
|
||||
});
|
||||
|
||||
it('should return error when setApprovalMode throws', async () => {
|
||||
|
||||
@@ -17,6 +17,7 @@ import {
|
||||
buildSubagentPlanToolBlockedResult,
|
||||
isPlanLifecycleToolUnavailableInSubagent,
|
||||
} from '../agents/runtime/subagent-plan-tool-policy.js';
|
||||
import { getPlanModeSystemReminder } from '../core/prompts.js';
|
||||
|
||||
const debugLogger = createDebugLogger('ENTER_PLAN_MODE');
|
||||
|
||||
@@ -182,8 +183,7 @@ class EnterPlanModeToolInvocation extends BaseToolInvocation<
|
||||
}
|
||||
|
||||
return {
|
||||
llmContent:
|
||||
'Plan mode is now active. Continue with read-only investigation, ask the user when needed, and use exit_plan_mode when the plan is ready.',
|
||||
llmContent: getPlanModeSystemReminder(this.config.getSdkMode()),
|
||||
returnDisplay: 'Entered plan mode.',
|
||||
};
|
||||
}
|
||||
@@ -195,6 +195,10 @@ export class EnterPlanModeTool extends BaseDeclarativeTool<
|
||||
> {
|
||||
static readonly Name: string = ToolNames.ENTER_PLAN_MODE;
|
||||
|
||||
override get maxOutputChars(): number {
|
||||
return Number.POSITIVE_INFINITY;
|
||||
}
|
||||
|
||||
constructor(private readonly config: Config) {
|
||||
super(
|
||||
EnterPlanModeTool.Name,
|
||||
|
||||
Reference in new issue
Block a user