From e0dee645a09f76814ee44dc37c07569b0a5b6ec4 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 00:10:08 +0800 Subject: [PATCH 01/11] feat(core): Restrict subagent plan mode tools Co-authored-by: Qwen-Coder --- .../src/agents/runtime/agent-core.test.ts | 162 +++++++++++++++++- .../core/src/agents/runtime/agent-core.ts | 20 ++- .../runtime/subagent-plan-tool-policy.ts | 36 ++++ .../runtime/workflow-orchestrator.test.ts | 29 +++- .../agents/runtime/workflow-orchestrator.ts | 11 +- .../core/src/core/coreToolScheduler.test.ts | 47 +++++ packages/core/src/core/coreToolScheduler.ts | 3 +- packages/core/src/tools/enterPlanMode.test.ts | 36 ++++ packages/core/src/tools/enterPlanMode.ts | 14 ++ packages/core/src/tools/exitPlanMode.test.ts | 50 ++++++ packages/core/src/tools/exitPlanMode.ts | 18 ++ packages/core/src/tools/tool-search.test.ts | 78 +++++++++ packages/core/src/tools/tool-search.ts | 16 ++ packages/core/src/tools/workflow/workflow.ts | 2 +- 14 files changed, 499 insertions(+), 23 deletions(-) create mode 100644 packages/core/src/agents/runtime/subagent-plan-tool-policy.ts diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index 9e4b0a577fb..5287977dfae 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -33,6 +33,7 @@ import type { ContentGenerator, ContentGeneratorConfig, } from '../../core/contentGenerator.js'; +import { ToolNames } from '../../tools/tool-names.js'; describe('AgentCore.runInAgentFrames', () => { // The deferred-approval `respond` callback that AgentCore hands to the @@ -307,13 +308,18 @@ describe('AgentCore.prepareTools', () => { ): { core: AgentCore; getFunctionDeclarationsSpy: ReturnType; + getFunctionDeclarationsFilteredSpy: ReturnType; } { const getFunctionDeclarationsSpy = vi.fn().mockReturnValue(fnDeclarations); + const getFunctionDeclarationsFilteredSpy = vi.fn((names: string[]) => + fnDeclarations.filter((d) => d.name && names.includes(d.name)), + ); const config = { + getDebugLogger: vi.fn().mockReturnValue({ debug: vi.fn() }), getToolRegistry: vi.fn().mockReturnValue({ warmAll: vi.fn().mockResolvedValue(undefined), getFunctionDeclarations: getFunctionDeclarationsSpy, - getFunctionDeclarationsFiltered: vi.fn().mockReturnValue([]), + getFunctionDeclarationsFiltered: getFunctionDeclarationsFilteredSpy, }), } as unknown as Config; @@ -325,7 +331,11 @@ describe('AgentCore.prepareTools', () => { { max_turns: 1 }, toolConfig, ); - return { core, getFunctionDeclarationsSpy }; + return { + core, + getFunctionDeclarationsSpy, + getFunctionDeclarationsFilteredSpy, + }; } it('wildcard tools:["*"] inherits deferred tools (passes includeDeferred: true)', async () => { @@ -358,17 +368,26 @@ describe('AgentCore.prepareTools', () => { it('absent toolConfig also inherits deferred tools (default = wildcard)', async () => { const fnDecls: FunctionDeclaration[] = [ { name: 'lsp', description: 'language server' } as FunctionDeclaration, + { + name: ToolNames.ENTER_PLAN_MODE, + description: 'enter plan mode', + } as FunctionDeclaration, + { + name: ToolNames.EXIT_PLAN_MODE, + description: 'exit plan mode', + } as FunctionDeclaration, ]; const { core, getFunctionDeclarationsSpy } = buildAgentForTools( undefined, fnDecls, ); - await core.prepareTools(); + const tools = await core.prepareTools(); expect(getFunctionDeclarationsSpy).toHaveBeenCalledWith({ includeDeferred: true, }); + expect(tools.map((t) => t.name)).toEqual(['lsp']); }); it('explicit tools list does NOT use the wildcard inherit path', async () => { @@ -385,4 +404,141 @@ describe('AgentCore.prepareTools', () => { expect(getFunctionDeclarationsSpy).not.toHaveBeenCalled(); }); + + it('excludes plan lifecycle tools from wildcard/default subagent tools', async () => { + const fnDecls: FunctionDeclaration[] = [ + { name: 'core_tool', description: 'core' } as FunctionDeclaration, + { + name: ToolNames.ENTER_PLAN_MODE, + description: 'enter plan mode', + } as FunctionDeclaration, + { + name: ToolNames.EXIT_PLAN_MODE, + description: 'exit plan mode', + } as FunctionDeclaration, + ]; + const { core } = buildAgentForTools({ tools: ['*'] }, fnDecls); + + const tools = await core.prepareTools(); + + expect(tools.map((t) => t.name)).toEqual(['core_tool']); + }); + + it('does not re-enable plan lifecycle tools via explicit tool names', async () => { + const fnDecls: FunctionDeclaration[] = [ + { name: ToolNames.READ_FILE, description: 'read' } as FunctionDeclaration, + { + name: ToolNames.ENTER_PLAN_MODE, + description: 'enter plan mode', + } as FunctionDeclaration, + { + name: ToolNames.EXIT_PLAN_MODE, + description: 'exit plan mode', + } as FunctionDeclaration, + ]; + const { core, getFunctionDeclarationsFilteredSpy } = buildAgentForTools( + { + tools: [ + ToolNames.READ_FILE, + ToolNames.ENTER_PLAN_MODE, + ToolNames.EXIT_PLAN_MODE, + ], + }, + fnDecls, + ); + + const tools = await core.prepareTools(); + + expect(getFunctionDeclarationsFilteredSpy).toHaveBeenCalledWith([ + ToolNames.READ_FILE, + ]); + expect(tools.map((t) => t.name)).toEqual([ToolNames.READ_FILE]); + }); + + it('filters inline plan lifecycle declarations too', async () => { + const inlineSafe = { + name: 'inline_safe', + description: 'safe inline tool', + } as FunctionDeclaration; + const { core } = buildAgentForTools( + { + tools: [ + { name: ToolNames.ENTER_PLAN_MODE } as FunctionDeclaration, + { name: ToolNames.EXIT_PLAN_MODE } as FunctionDeclaration, + inlineSafe, + ], + }, + [], + ); + + const tools = await core.prepareTools(); + + expect(tools).toEqual([inlineSafe]); + }); + + it('keeps teammate coordination tools but excludes plan lifecycle tools', async () => { + const fnDecls: FunctionDeclaration[] = [ + { + name: ToolNames.SEND_MESSAGE, + description: 'send message', + } as FunctionDeclaration, + { + name: ToolNames.TASK_UPDATE, + description: 'task update', + } as FunctionDeclaration, + { + name: ToolNames.ENTER_PLAN_MODE, + description: 'enter plan mode', + } as FunctionDeclaration, + { + name: ToolNames.EXIT_PLAN_MODE, + description: 'exit plan mode', + } as FunctionDeclaration, + ]; + const { core } = buildAgentForTools({ tools: ['*'] }, fnDecls); + + let tools: FunctionDeclaration[] = []; + await runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + async () => { + tools = await core.prepareTools(); + }, + ); + + expect(tools.map((t) => t.name)).toEqual([ + ToolNames.SEND_MESSAGE, + ToolNames.TASK_UPDATE, + ]); + }); + + it.each([ToolNames.ENTER_PLAN_MODE, ToolNames.EXIT_PLAN_MODE])( + 'returns a dedicated message when filtered %s is called directly', + async (toolName) => { + const { core } = buildAgentForTools(undefined, []); + + const result = await core.processFunctionCalls( + [ + { + name: toolName, + args: { plan: 'Plan from filtered tool' }, + id: 'call-1', + }, + ], + new AbortController(), + 'prompt-filtered-plan-tool', + 1, + [{ name: ToolNames.READ_FILE } as FunctionDeclaration], + ); + + const response = result.messages[0]?.parts?.[0]?.functionResponse + ?.response as { error?: string } | undefined; + expect(response?.error).toContain('not available inside subagents'); + expect(response?.error).toContain('return your plan'); + }, + ); }); diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 71dcbb682ba..77a5520de1e 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -91,6 +91,11 @@ import { runWithTeammateIdentity, } from '../team/identity.js'; import type { TeammateIdentity } from '../team/types.js'; +import { + getSubagentPlanToolUnavailableMessage, + isSubagentPlanLifecycleTool, + SUBAGENT_PLAN_LIFECYCLE_TOOLS, +} from './subagent-plan-tool-policy.js'; /** * Result of a single reasoning loop invocation. @@ -109,6 +114,8 @@ import type { TeammateIdentity } from '../team/types.js'; * non-team Agent subagent has no teammate identity, so isTeammate() * returns false and these tools would treat it as the leader — letting * it delete or rewrite the active team. + * - Plan lifecycle tools are owned by the caller/main session. A subagent + * should return its plan to the caller instead of entering or exiting mode. */ export const EXCLUDED_TOOLS_FOR_SUBAGENTS: ReadonlySet = new Set([ ToolNames.AGENT, @@ -122,6 +129,7 @@ export const EXCLUDED_TOOLS_FOR_SUBAGENTS: ReadonlySet = new Set([ ToolNames.TASK_CREATE, ToolNames.TASK_UPDATE, ToolNames.TASK_LIST, + ...SUBAGENT_PLAN_LIFECYCLE_TOOLS, // Worktree management belongs to the parent session — a subagent must // never enter or exit the user's worktree state independently. ToolNames.ENTER_WORKTREE, @@ -136,6 +144,7 @@ export const EXCLUDED_TOOLS_FOR_SUBAGENTS: ReadonlySet = new Set([ * Tools excluded from teammates. Teammates need send_message and the * task_* coordination tools to do their job, but they must not be able * to create or destroy the team itself — only the leader can do that. + * Plan lifecycle tools remain caller-owned for teammates too. */ const EXCLUDED_TOOLS_FOR_TEAMMATES: ReadonlySet = new Set([ ToolNames.AGENT, @@ -145,6 +154,7 @@ const EXCLUDED_TOOLS_FOR_TEAMMATES: ReadonlySet = new Set([ ToolNames.TASK_STOP, ToolNames.TEAM_CREATE, ToolNames.TEAM_DELETE, + ...SUBAGENT_PLAN_LIFECYCLE_TOOLS, // Worktree management belongs to the parent session. ToolNames.ENTER_WORKTREE, ToolNames.EXIT_WORKTREE, @@ -469,10 +479,6 @@ export class AgentCore { const excludedFromSubagents = isTeammate() ? EXCLUDED_TOOLS_FOR_TEAMMATES : EXCLUDED_TOOLS_FOR_SUBAGENTS; - // When a subagent has an explicit tools list (not wildcard), only the - // recursive-spawn guard (AgentTool) is enforced. - const recursionGuardOnly = new Set([ToolNames.AGENT]); - if (this.toolConfig) { const asStrings = this.toolConfig.tools.filter( (t): t is string => typeof t === 'string', @@ -509,7 +515,7 @@ export class AgentCore { // Also filter inline FunctionDeclaration[] passed directly in toolConfig. toolsList.push( ...onlyInlineDecls.filter( - (d) => !(d.name && recursionGuardOnly.has(d.name)), + (d) => !(d.name && excludedFromSubagents.has(d.name)), ), ); } else { @@ -1205,7 +1211,9 @@ export class AgentCore { const args = (fc.args ?? {}) as Record; if (!allowedToolNames.has(fc.name)) { - const errorMessage = `Tool "${toolName}" not found. Tools must use the exact names provided.`; + const errorMessage = isSubagentPlanLifecycleTool(toolName) + ? getSubagentPlanToolUnavailableMessage(toolName) + : `Tool "${toolName}" not found. Tools must use the exact names provided.`; const functionResponsePart = { functionResponse: { id: callId, diff --git a/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts b/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts new file mode 100644 index 00000000000..729d26cf5bc --- /dev/null +++ b/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts @@ -0,0 +1,36 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ToolNames } from '../../tools/tool-names.js'; +import { isTeammate } from '../team/identity.js'; +import { getCurrentAgentId } from './agent-context.js'; + +export const SUBAGENT_PLAN_LIFECYCLE_TOOLS: ReadonlySet = new Set([ + ToolNames.ENTER_PLAN_MODE, + ToolNames.EXIT_PLAN_MODE, +]); + +export function isSubagentLikeExecutionContext(): boolean { + return getCurrentAgentId() !== null || isTeammate(); +} + +export function isSubagentPlanLifecycleTool(toolName: string): boolean { + return SUBAGENT_PLAN_LIFECYCLE_TOOLS.has(toolName); +} + +export function isPlanLifecycleToolUnavailableInSubagent( + toolName: string, +): boolean { + return ( + isSubagentLikeExecutionContext() && isSubagentPlanLifecycleTool(toolName) + ); +} + +export function getSubagentPlanToolUnavailableMessage( + toolName: string, +): string { + return `${toolName} is not available inside subagents or team agents. Plan mode is owned by the caller/main session; return your plan, findings, or constraints to the caller in your normal response instead of entering or exiting plan mode.`; +} diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index f1e7cf5562c..3933294fa71 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -1156,13 +1156,14 @@ describe('createProductionDispatch', () => { }); }); - // T11: disallow SendMessage / ExitPlanMode to mirror upstream Tg8. - it('disallows SendMessage and ExitPlanMode for workflow subagents', async () => { + // T11: disallow SendMessage plus plan lifecycle tools to mirror upstream Tg8. + it('disallows SendMessage and plan lifecycle tools for workflow subagents', async () => { const dispatch = createProductionDispatch(fakeConfig()); await dispatch('hello', { label: 'h1' }); expect(created[0]!.toolConfig?.tools).toEqual(['*']); expect(created[0]!.toolConfig?.disallowedTools).toEqual([ 'send_message', + 'enter_plan_mode', 'exit_plan_mode', ]); }); @@ -1932,9 +1933,13 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( expect(result).toBe('explore-output'); expect(calls).toHaveLength(1); expect(calls[0].config.name).toBe('Explore'); - // Workflow floor [SendMessage, ExitPlanMode] must be unioned in. + // Workflow floor [SendMessage, EnterPlanMode, ExitPlanMode] must be unioned in. expect(calls[0].config.disallowedTools).toEqual( - expect.arrayContaining(['send_message', 'exit_plan_mode']), + expect.arrayContaining([ + 'send_message', + 'enter_plan_mode', + 'exit_plan_mode', + ]), ); }); @@ -2008,9 +2013,14 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( const dispatch = createProductionDispatch(config); await dispatch('hi', { agentType: 'Permissive' }); const disallowed = calls[0].config.disallowedTools ?? []; - // Union: Foo (from agentType) + send_message + exit_plan_mode (floor). + // Union: Foo (from agentType) + send_message + plan lifecycle floor. expect(disallowed).toEqual( - expect.arrayContaining(['Foo', 'send_message', 'exit_plan_mode']), + expect.arrayContaining([ + 'Foo', + 'send_message', + 'enter_plan_mode', + 'exit_plan_mode', + ]), ); }); @@ -2404,7 +2414,12 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( }); const disallowed = calls[0].config.disallowedTools ?? []; expect(disallowed).toEqual( - expect.arrayContaining(['Foo', 'send_message', 'exit_plan_mode']), + expect.arrayContaining([ + 'Foo', + 'send_message', + 'enter_plan_mode', + 'exit_plan_mode', + ]), ); }); diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index 5bb3157df1c..0f165fe20f8 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -151,15 +151,16 @@ const WORKFLOW_SUBAGENT_MAX_TURNS = 50; const WORKFLOW_SUBAGENT_MAX_TIME_MINUTES = 10; /** - * disallowedTools mirror the upstream `Tg8` workflow-subagent config — both + * disallowedTools mirror the upstream `Tg8` workflow-subagent config. These * tools would let a subagent break the "final text IS the return value" - * contract. SendMessage would deliver the answer to the user instead of - * the calling script; ExitPlanMode would interrupt the workflow's plan-mode - * intent. Defense-in-depth alongside the §XmO system prompt that already - * documents both restrictions. + * contract: SendMessage would deliver the answer to the user instead of + * the calling script, and plan lifecycle tools would interrupt the workflow's + * plan-mode intent. Defense-in-depth alongside the §XmO system prompt that + * already documents these restrictions. */ const WORKFLOW_SUBAGENT_DISALLOWED_TOOLS: string[] = [ ToolNames.SEND_MESSAGE, + ToolNames.ENTER_PLAN_MODE, ToolNames.EXIT_PLAN_MODE, ]; diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 73eef1bc298..a33134ba9fd 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -60,6 +60,7 @@ import { WriteFileTool } from '../tools/write-file.js'; import { ShellTool, ShellToolInvocation } from '../tools/shell.js'; import type { ShellToolParams } from '../tools/shell.js'; import type { ShellExecutionConfig } from '../services/shellExecutionService.js'; +import { runWithAgentContext } from '../agents/runtime/agent-context.js'; type ToolSpanRecord = { name: string; @@ -5991,6 +5992,52 @@ 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'); + } + }); + + it('should tell subagents to return the plan directly when plan mode blocks a tool', async () => { + const editTool = new MockTool({ + name: 'write_file', + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: MOCK_TOOL_GET_CONFIRMATION_DETAILS, + }); + const onAllToolCallsComplete = vi.fn(); + const onToolCallsUpdate = vi.fn(); + const scheduler = createPlanModeScheduler( + editTool, + onAllToolCallsComplete, + onToolCallsUpdate, + ); + + const abortController = new AbortController(); + const request = { + callId: '1', + name: 'write_file', + args: {}, + isClientInitiated: false, + prompt_id: 'prompt-plan-subagent-blocked', + }; + + await runWithAgentContext('agent-1', () => + scheduler.schedule([request], abortController.signal), + ); + + await vi.waitFor(() => { + expect(onAllToolCallsComplete).toHaveBeenCalled(); + }); + + const completedCalls = onAllToolCallsComplete.mock + .calls[0][0] as ToolCall[]; + expect(completedCalls[0].status).toBe('error'); + if (completedCalls[0].status === 'error') { + const responseText = JSON.stringify( + completedCalls[0].response.responseParts, + ); + expect(responseText).toContain('Present your plan directly'); + expect(responseText).not.toContain('exit_plan_mode tool'); } }); diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 3a8a358c48c..d245b2bf854 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -110,6 +110,7 @@ import levenshtein from 'fast-levenshtein'; import { getPlanModeSystemReminder } from './prompts.js'; import { ShellToolInvocation } from '../tools/shell.js'; import { IdeClient } from '../ide/ide-client.js'; +import { isSubagentLikeExecutionContext } from '../agents/runtime/subagent-plan-tool-policy.js'; import { safeSetStatus } from '../telemetry/tracer.js'; import { SpanStatusCode, type Span } from '@opentelemetry/api'; import { @@ -2268,7 +2269,7 @@ export class CoreToolScheduler { responseParts: convertToFunctionResponse( reqInfo.name, reqInfo.callId, - getPlanModeSystemReminder(), + getPlanModeSystemReminder(isSubagentLikeExecutionContext()), ), resultDisplay: 'Plan mode blocked a non-read-only tool call.', error: undefined, diff --git a/packages/core/src/tools/enterPlanMode.test.ts b/packages/core/src/tools/enterPlanMode.test.ts index be000cd515d..8e505bcc951 100644 --- a/packages/core/src/tools/enterPlanMode.test.ts +++ b/packages/core/src/tools/enterPlanMode.test.ts @@ -7,6 +7,8 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; 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'; describe('EnterPlanModeTool', () => { let tool: EnterPlanModeTool; @@ -157,5 +159,39 @@ describe('EnterPlanModeTool', () => { expect(result.llmContent).toContain('Failed to enter plan mode'); expect(result.llmContent).toContain('trust gate'); }); + + it('rejects inside subagent context without changing approval mode', async () => { + approvalMode = ApprovalMode.DEFAULT; + const invocation = tool.build({}); + + const result = await runWithAgentContext('agent-1', () => + invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain('not available inside subagents'); + expect(result.llmContent).toContain('return your plan'); + expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); + expect(approvalMode).toBe(ApprovalMode.DEFAULT); + }); + + it('rejects inside teammate context without changing approval mode', async () => { + approvalMode = ApprovalMode.AUTO_EDIT; + const invocation = tool.build({}); + + const result = await runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + () => invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain('not available inside subagents'); + expect(result.llmContent).toContain('return your plan'); + expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); + expect(approvalMode).toBe(ApprovalMode.AUTO_EDIT); + }); }); }); diff --git a/packages/core/src/tools/enterPlanMode.ts b/packages/core/src/tools/enterPlanMode.ts index 42edb12d327..264b83fba87 100644 --- a/packages/core/src/tools/enterPlanMode.ts +++ b/packages/core/src/tools/enterPlanMode.ts @@ -13,6 +13,10 @@ import { ApprovalMode } from '../config/config.js'; import { ToolDisplayNames, ToolNames } from './tool-names.js'; import { InputFormat } from '../output/types.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getSubagentPlanToolUnavailableMessage, + isPlanLifecycleToolUnavailableInSubagent, +} from '../agents/runtime/subagent-plan-tool-policy.js'; const debugLogger = createDebugLogger('ENTER_PLAN_MODE'); @@ -64,6 +68,16 @@ class EnterPlanModeToolInvocation extends BaseToolInvocation< } async execute(_signal: AbortSignal): Promise { + if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.ENTER_PLAN_MODE)) { + const message = getSubagentPlanToolUnavailableMessage( + ToolNames.ENTER_PLAN_MODE, + ); + return { + llmContent: message, + returnDisplay: message, + }; + } + // In headless (non-interactive) mode without ACP support, the gate // exit paths require user interaction that cannot be fulfilled. const isAcpMode = diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts index 81c35e7f5df..84c515b310e 100644 --- a/packages/core/src/tools/exitPlanMode.test.ts +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -14,6 +14,8 @@ import { import { ToolConfirmationOutcome } from './tools.js'; import { runPlanApprovalGate } from '../plan-gate/planApprovalGate.js'; import type { GateDecision, MergedGateFinding } from '../plan-gate/types.js'; +import { runWithAgentContext } from '../agents/runtime/agent-context.js'; +import { runWithTeammateIdentity } from '../agents/team/identity.js'; vi.mock('../plan-gate/planApprovalGate.js', async (importOriginal) => { const actual = @@ -364,6 +366,54 @@ describe('ExitPlanModeTool', () => { const invocation = tool.build(params); expect(invocation.toolLocations()).toEqual([]); }); + + it('allows by default inside subagent context to avoid approval UI', async () => { + const invocation = tool.build({ plan: 'Subagent plan' }); + + const permission = await runWithAgentContext('agent-1', () => + invocation.getDefaultPermission(), + ); + + expect(permission).toBe('allow'); + }); + + it('rejects inside subagent context without saving or changing mode', async () => { + approvalMode = ApprovalMode.PLAN; + const invocation = tool.build({ plan: 'Subagent plan' }); + + const result = await runWithAgentContext('agent-1', () => + invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain('not available inside subagents'); + expect(result.llmContent).toContain('return your plan'); + expect(mockConfig.savePlan).not.toHaveBeenCalled(); + expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); + expect(mockedRunPlanApprovalGate).not.toHaveBeenCalled(); + expect(approvalMode).toBe(ApprovalMode.PLAN); + }); + + it('rejects inside teammate context without saving or changing mode', async () => { + approvalMode = ApprovalMode.PLAN; + const invocation = tool.build({ plan: 'Teammate plan' }); + + const result = await runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + () => invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain('not available inside subagents'); + expect(result.llmContent).toContain('return your plan'); + expect(mockConfig.savePlan).not.toHaveBeenCalled(); + expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); + expect(mockedRunPlanApprovalGate).not.toHaveBeenCalled(); + expect(approvalMode).toBe(ApprovalMode.PLAN); + }); }); describe('tool description', () => { diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index 4db18bbe880..d109082429d 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -26,6 +26,10 @@ import { } from '../plan-gate/planApprovalGate.js'; import type { EvidenceBundle } from '../plan-gate/types.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getSubagentPlanToolUnavailableMessage, + isPlanLifecycleToolUnavailableInSubagent, +} from '../agents/runtime/subagent-plan-tool-policy.js'; const debugLogger = createDebugLogger('EXIT_PLAN_MODE'); @@ -118,6 +122,10 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< * (issue #5574). */ override async getDefaultPermission(): Promise { + if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)) { + return 'allow'; + } + const prePlanMode = this.config.getPrePlanMode(); const gateState = this.config.getPlanGateState(); if ( @@ -195,6 +203,16 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< } async execute(signal: AbortSignal): Promise { + if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)) { + const message = getSubagentPlanToolUnavailableMessage( + ToolNames.EXIT_PLAN_MODE, + ); + return { + llmContent: message, + returnDisplay: message, + }; + } + const { plan, originalRequest, researchSummary, resolutionSummary } = this.params; const prePlanMode = this.config.getPrePlanMode(); diff --git a/packages/core/src/tools/tool-search.test.ts b/packages/core/src/tools/tool-search.test.ts index 523e4ffd811..fbba1ac79d4 100644 --- a/packages/core/src/tools/tool-search.test.ts +++ b/packages/core/src/tools/tool-search.test.ts @@ -16,6 +16,8 @@ import { CronCreateTool } from './cron-create.js'; import { CronDeleteTool } from './cron-delete.js'; import { CronListTool } from './cron-list.js'; import { LoopWakeupTool } from './loop-wakeup.js'; +import { ToolNames } from './tool-names.js'; +import { runWithAgentContext } from '../agents/runtime/agent-context.js'; const baseConfigParams: ConfigParameters = { cwd: '/tmp', @@ -587,6 +589,82 @@ describe('ToolSearchTool', () => { expect(setToolsSpy).not.toHaveBeenCalled(); }); + it('select: exit_plan_mode remains inspectable in the main session', async () => { + registry.registerTool( + new MockTool({ + name: ToolNames.EXIT_PLAN_MODE, + shouldDefer: true, + alwaysLoad: true, + }), + ); + const setToolsSpy = vi.fn().mockResolvedValue(undefined); + vi.spyOn(config, 'getGeminiClient').mockReturnValue({ + setTools: setToolsSpy, + refreshStartupContextReminder: vi.fn().mockResolvedValue(undefined), + } as never); + + const tool = new ToolSearchTool(config); + const result = await tool + .build({ query: `select:${ToolNames.EXIT_PLAN_MODE}` }) + .execute(new AbortController().signal); + + expect(String(result.llmContent)).toContain( + `"name":"${ToolNames.EXIT_PLAN_MODE}"`, + ); + expect(registry.isDeferredToolRevealed(ToolNames.EXIT_PLAN_MODE)).toBe( + false, + ); + expect(setToolsSpy).not.toHaveBeenCalled(); + }); + + it.each<{ + toolName: string; + shouldDefer: boolean; + alwaysLoad: boolean; + }>([ + { + toolName: ToolNames.ENTER_PLAN_MODE, + shouldDefer: false, + alwaysLoad: false, + }, + { + toolName: ToolNames.EXIT_PLAN_MODE, + shouldDefer: true, + alwaysLoad: true, + }, + ])( + 'select: rejects $toolName inside subagent context without revealing or syncing tools', + async ({ toolName, shouldDefer, alwaysLoad }) => { + registry.registerTool( + new MockTool({ + name: toolName, + shouldDefer, + alwaysLoad, + }), + ); + const setToolsSpy = vi.fn().mockResolvedValue(undefined); + vi.spyOn(config, 'getGeminiClient').mockReturnValue({ + setTools: setToolsSpy, + refreshStartupContextReminder: vi.fn().mockResolvedValue(undefined), + } as never); + + const tool = new ToolSearchTool(config); + const result = await runWithAgentContext('agent-1', () => + tool + .build({ query: `select:${toolName}` }) + .execute(new AbortController().signal), + ); + + expect(String(result.llmContent)).toContain( + 'not available inside subagents', + ); + expect(String(result.llmContent)).toContain('return your plan'); + expect(String(result.llmContent)).not.toContain(`"name":"${toolName}"`); + expect(registry.isDeferredToolRevealed(toolName)).toBe(false); + expect(setToolsSpy).not.toHaveBeenCalled(); + }, + ); + it('+must-word filters candidates whose name does not contain the required term', async () => { // Both tools would match on "send" in description; only one has "slack" // in its name. The +slack prefix should narrow the result to that one. diff --git a/packages/core/src/tools/tool-search.ts b/packages/core/src/tools/tool-search.ts index d17c5ff7253..b86cf640617 100644 --- a/packages/core/src/tools/tool-search.ts +++ b/packages/core/src/tools/tool-search.ts @@ -30,6 +30,10 @@ import { ToolNames, ToolDisplayNames } from './tool-names.js'; import type { Config } from '../config/config.js'; import { DiscoveredMCPTool } from './mcp-tool.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getSubagentPlanToolUnavailableMessage, + isPlanLifecycleToolUnavailableInSubagent, +} from '../agents/runtime/subagent-plan-tool-policy.js'; const debugLogger = createDebugLogger('TOOL_SEARCH'); @@ -265,6 +269,7 @@ class ToolSearchInvocation extends BaseToolInvocation< const registry = this.config.getToolRegistry(); const loaded: AnyDeclarativeTool[] = []; const missing: string[] = []; + const blocked: string[] = []; // Case-insensitive lookup across all known names (instance names + factory // names). Preserve the user-supplied casing in the error list so the @@ -284,6 +289,10 @@ class ToolSearchInvocation extends BaseToolInvocation< missing.push(requested); continue; } + if (isPlanLifecycleToolUnavailableInSubagent(canonical)) { + blocked.push(canonical); + continue; + } // Treat ensureTool throws the same as a null return: log + report // missing. Without this, an exception mid-batch would propagate // out of the loop with previous tools already revealed but never @@ -426,6 +435,12 @@ class ToolSearchInvocation extends BaseToolInvocation< const header = llmContent ? '\n\n' : ''; llmContent += `${header}Not found: ${missing.join(', ')}`; } + if (blocked.length > 0) { + const header = llmContent ? '\n\n' : ''; + llmContent += `${header}Unavailable: ${blocked + .map((name) => getSubagentPlanToolUnavailableMessage(name)) + .join('\n')}`; + } if (truncated.length > 0) { // Surface the dropped names so the model knows it must re-issue // another ToolSearch for them — without this, the model would @@ -438,6 +453,7 @@ class ToolSearchInvocation extends BaseToolInvocation< const displayParts: string[] = []; if (loaded.length > 0) displayParts.push(`Loaded ${loaded.length} tool(s)`); if (missing.length > 0) displayParts.push(`${missing.length} missing`); + if (blocked.length > 0) displayParts.push(`${blocked.length} unavailable`); if (truncated.length > 0) displayParts.push(`${truncated.length} truncated`); const returnDisplay = displayParts.join(', ') || 'No tools loaded'; diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 3a713cec33f..89817185ebc 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -112,7 +112,7 @@ const WORKFLOW_PARAM_SCHEMA = { 'in this build" (parity with upstream). isolation=worktree refuses to ' + 'run when the parent working tree has uncommitted changes (the subagent ' + 'would see a stale HEAD). ' + - 'Workflow subagents always have SendMessage / ExitPlanMode in their ' + + 'Workflow subagents always have SendMessage / EnterPlanMode / ExitPlanMode in their ' + 'disallowed-tool floor regardless of agentType. ' + 'Concurrency: `parallel([() => agent(...), ...])` runs thunks ' + 'through a shared per-run window (default ' + From 539c87799c51baba6b20679ba12d2ec2890672e4 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 00:52:57 +0800 Subject: [PATCH 02/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- packages/core/src/tools/enterPlanMode.test.ts | 2 ++ packages/core/src/tools/enterPlanMode.ts | 1 + packages/core/src/tools/exitPlanMode.test.ts | 2 ++ packages/core/src/tools/exitPlanMode.ts | 1 + 4 files changed, 6 insertions(+) diff --git a/packages/core/src/tools/enterPlanMode.test.ts b/packages/core/src/tools/enterPlanMode.test.ts index 8e505bcc951..8f918d4927f 100644 --- a/packages/core/src/tools/enterPlanMode.test.ts +++ b/packages/core/src/tools/enterPlanMode.test.ts @@ -170,6 +170,7 @@ describe('EnterPlanModeTool', () => { expect(result.llmContent).toContain('not available inside subagents'); expect(result.llmContent).toContain('return your plan'); + expect(result.error?.message).toBe(result.llmContent); expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); expect(approvalMode).toBe(ApprovalMode.DEFAULT); }); @@ -190,6 +191,7 @@ describe('EnterPlanModeTool', () => { expect(result.llmContent).toContain('not available inside subagents'); expect(result.llmContent).toContain('return your plan'); + expect(result.error?.message).toBe(result.llmContent); expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); expect(approvalMode).toBe(ApprovalMode.AUTO_EDIT); }); diff --git a/packages/core/src/tools/enterPlanMode.ts b/packages/core/src/tools/enterPlanMode.ts index 264b83fba87..21b27f04db6 100644 --- a/packages/core/src/tools/enterPlanMode.ts +++ b/packages/core/src/tools/enterPlanMode.ts @@ -75,6 +75,7 @@ class EnterPlanModeToolInvocation extends BaseToolInvocation< return { llmContent: message, returnDisplay: message, + error: { message }, }; } diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts index 84c515b310e..24157356995 100644 --- a/packages/core/src/tools/exitPlanMode.test.ts +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -387,6 +387,7 @@ describe('ExitPlanModeTool', () => { expect(result.llmContent).toContain('not available inside subagents'); expect(result.llmContent).toContain('return your plan'); + expect(result.error?.message).toBe(result.llmContent); expect(mockConfig.savePlan).not.toHaveBeenCalled(); expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); expect(mockedRunPlanApprovalGate).not.toHaveBeenCalled(); @@ -409,6 +410,7 @@ describe('ExitPlanModeTool', () => { expect(result.llmContent).toContain('not available inside subagents'); expect(result.llmContent).toContain('return your plan'); + expect(result.error?.message).toBe(result.llmContent); expect(mockConfig.savePlan).not.toHaveBeenCalled(); expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); expect(mockedRunPlanApprovalGate).not.toHaveBeenCalled(); diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index d109082429d..72e559f7887 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -210,6 +210,7 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< return { llmContent: message, returnDisplay: message, + error: { message }, }; } From 585aa259d771c838d1310c1063a55ca0725fe069 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 01:45:46 +0800 Subject: [PATCH 03/11] codex: address follow-up PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../src/agents/runtime/agent-core.test.ts | 32 +++++++++------- .../core/src/agents/runtime/agent-core.ts | 4 +- .../agents/runtime/workflow-orchestrator.ts | 4 +- packages/core/src/core/client.test.ts | 37 ++++++++++++++++++- packages/core/src/core/client.ts | 5 ++- 5 files changed, 63 insertions(+), 19 deletions(-) diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index 5287977dfae..294be2124a1 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -455,7 +455,7 @@ describe('AgentCore.prepareTools', () => { expect(tools.map((t) => t.name)).toEqual([ToolNames.READ_FILE]); }); - it('filters inline plan lifecycle declarations too', async () => { + it('filters inline declarations using the full subagent exclusion floor', async () => { const inlineSafe = { name: 'inline_safe', description: 'safe inline tool', @@ -463,6 +463,8 @@ describe('AgentCore.prepareTools', () => { const { core } = buildAgentForTools( { tools: [ + { name: ToolNames.SEND_MESSAGE } as FunctionDeclaration, + { name: ToolNames.TASK_UPDATE } as FunctionDeclaration, { name: ToolNames.ENTER_PLAN_MODE } as FunctionDeclaration, { name: ToolNames.EXIT_PLAN_MODE } as FunctionDeclaration, inlineSafe, @@ -521,18 +523,22 @@ describe('AgentCore.prepareTools', () => { async (toolName) => { const { core } = buildAgentForTools(undefined, []); - const result = await core.processFunctionCalls( - [ - { - name: toolName, - args: { plan: 'Plan from filtered tool' }, - id: 'call-1', - }, - ], - new AbortController(), - 'prompt-filtered-plan-tool', - 1, - [{ name: ToolNames.READ_FILE } as FunctionDeclaration], + const result = await runWithAgentContext('test-subagent', () => + core.runInAgentFrames(() => + core.processFunctionCalls( + [ + { + name: toolName, + args: { plan: 'Plan from filtered tool' }, + id: 'call-1', + }, + ], + new AbortController(), + 'prompt-filtered-plan-tool', + 1, + [{ name: ToolNames.READ_FILE } as FunctionDeclaration], + ), + ), ); const response = result.messages[0]?.parts?.[0]?.functionResponse diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 77a5520de1e..62001439edf 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -93,7 +93,7 @@ import { import type { TeammateIdentity } from '../team/types.js'; import { getSubagentPlanToolUnavailableMessage, - isSubagentPlanLifecycleTool, + isPlanLifecycleToolUnavailableInSubagent, SUBAGENT_PLAN_LIFECYCLE_TOOLS, } from './subagent-plan-tool-policy.js'; @@ -1211,7 +1211,7 @@ export class AgentCore { const args = (fc.args ?? {}) as Record; if (!allowedToolNames.has(fc.name)) { - const errorMessage = isSubagentPlanLifecycleTool(toolName) + const errorMessage = isPlanLifecycleToolUnavailableInSubagent(toolName) ? getSubagentPlanToolUnavailableMessage(toolName) : `Tool "${toolName}" not found. Tools must use the exact names provided.`; const functionResponsePart = { diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index 0f165fe20f8..fdd16a53e7a 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -45,6 +45,7 @@ import { WorkspaceContext } from '../../utils/workspaceContext.js'; import { SyntheticOutputTool } from '../../tools/syntheticOutput.js'; import { rebuildToolRegistryOnOverride } from '../../tools/agent/agent.js'; import { toModelVisibleSubagentResult } from '../subagent-result.js'; +import { SUBAGENT_PLAN_LIFECYCLE_TOOLS } from './subagent-plan-tool-policy.js'; /** * Default ceiling on total `agent()` calls per workflow run (matches upstream @@ -160,8 +161,7 @@ const WORKFLOW_SUBAGENT_MAX_TIME_MINUTES = 10; */ const WORKFLOW_SUBAGENT_DISALLOWED_TOOLS: string[] = [ ToolNames.SEND_MESSAGE, - ToolNames.ENTER_PLAN_MODE, - ToolNames.EXIT_PLAN_MODE, + ...SUBAGENT_PLAN_LIFECYCLE_TOOLS, ]; /** diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 7a310244d03..d48770cc70c 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -55,7 +55,11 @@ vi.mock('../utils/retry.js', () => ({ retryWithBackoff: vi.fn(async (fn) => await fn()), isUnattendedMode: vi.fn(() => false), })); -import { getCoreSystemPrompt, getCustomSystemPrompt } from './prompts.js'; +import { + getCoreSystemPrompt, + getCustomSystemPrompt, + getPlanModeSystemReminder, +} from './prompts.js'; import { DEFAULT_QWEN_FLASH_MODEL } from '../config/models.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { promptIdContext } from '../utils/promptIdContext.js'; @@ -75,6 +79,7 @@ import { setActiveGoal, } from '../goals/activeGoalStore.js'; import type { FileHistorySnapshot } from '../services/fileHistoryService.js'; +import { runWithAgentContext } from '../agents/runtime/agent-context.js'; // Mock fs module to prevent actual file system operations during tests const mockFileSystem = new Map(); @@ -481,6 +486,7 @@ describe('Gemini Client (client.ts)', () => { getNoBrowser: vi.fn().mockReturnValue(false), getUsageStatisticsEnabled: vi.fn().mockReturnValue(true), getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT), + getSdkMode: vi.fn().mockReturnValue(false), getIdeModeFeature: vi.fn().mockReturnValue(false), getIdeMode: vi.fn().mockReturnValue(true), getDebugMode: vi.fn().mockReturnValue(false), @@ -4702,6 +4708,35 @@ hello ); }); + 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); + vi.mocked(getPlanModeSystemReminder).mockReturnValue( + 'return plan to caller', + ); + const mockStream = (async function* () { + yield { type: 'content', value: 'Plan ready' }; + })(); + mockTurnRunFn.mockReturnValue(mockStream); + client['chat'] = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + } as unknown as GeminiChat; + + await runWithAgentContext('agent-1', async () => { + const stream = client.sendMessageStream( + [{ text: 'Plan this change' }], + new AbortController().signal, + 'prompt-id-subagent-plan-reminder', + ); + for await (const _ of stream) { + // consume stream + } + }); + + expect(getPlanModeSystemReminder).toHaveBeenCalledWith(true); + }); + it('should not inject duplicate date on the same day', async () => { client['lastInjectedDate'] = undefined; vi.setSystemTime(new Date('2026-06-05T12:00:00Z')); diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 6760428d464..fe10bc520a8 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -117,6 +117,7 @@ import { subagentNameContext } from '../utils/subagentNameContext.js'; import { escapeSystemReminderTags } from '../utils/xml.js'; import { ApiRetryEvent } from '../telemetry/types.js'; import { logApiRetry } from '../telemetry/loggers.js'; +import { isSubagentLikeExecutionContext } from '../agents/runtime/subagent-plan-tool-policy.js'; // Hook types and utilities import { @@ -2103,7 +2104,9 @@ export class GeminiClient { // add plan mode system reminder if approval mode is plan if (this.config.getApprovalMode() === ApprovalMode.PLAN) { systemReminders.push( - getPlanModeSystemReminder(this.config.getSdkMode()), + getPlanModeSystemReminder( + isSubagentLikeExecutionContext() || this.config.getSdkMode(), + ), ); } From 2efcd918b27a1ddd7d801df284ed5fb6d97d9225 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 02:45:50 +0800 Subject: [PATCH 04/11] codex: address plan tool observability feedback (#6087) Co-authored-by: Qwen-Coder --- packages/core/src/tools/enterPlanMode.ts | 3 +++ packages/core/src/tools/exitPlanMode.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/core/src/tools/enterPlanMode.ts b/packages/core/src/tools/enterPlanMode.ts index 21b27f04db6..0731e8b9604 100644 --- a/packages/core/src/tools/enterPlanMode.ts +++ b/packages/core/src/tools/enterPlanMode.ts @@ -72,6 +72,9 @@ class EnterPlanModeToolInvocation extends BaseToolInvocation< const message = getSubagentPlanToolUnavailableMessage( ToolNames.ENTER_PLAN_MODE, ); + debugLogger.warn( + `[EnterPlanModeTool] Blocked plan lifecycle tool call from subagent: ${ToolNames.ENTER_PLAN_MODE}`, + ); return { llmContent: message, returnDisplay: message, diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index 72e559f7887..5b6088340cf 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -207,6 +207,9 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< const message = getSubagentPlanToolUnavailableMessage( ToolNames.EXIT_PLAN_MODE, ); + debugLogger.warn( + `[ExitPlanModeTool] Blocked plan lifecycle tool call from subagent: ${ToolNames.EXIT_PLAN_MODE}`, + ); return { llmContent: message, returnDisplay: message, From 0a62a85d77f1cb8f7748d069257f0f4d2dacf2a4 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 04:45:54 +0800 Subject: [PATCH 05/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../src/agents/runtime/agent-core.test.ts | 1 + .../runtime/workflow-orchestrator.test.ts | 15 ++++++++++++- .../agents/runtime/workflow-orchestrator.ts | 21 ++++++++++++++++--- packages/core/src/core/client.ts | 2 ++ packages/core/src/core/coreToolScheduler.ts | 2 ++ packages/core/src/tools/exitPlanMode.ts | 2 ++ packages/core/src/tools/tool-search.test.ts | 2 ++ packages/core/src/tools/tool-search.ts | 13 ++++++++---- 8 files changed, 50 insertions(+), 8 deletions(-) diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index 294be2124a1..c9a2d29be3d 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -545,6 +545,7 @@ describe('AgentCore.prepareTools', () => { ?.response as { error?: string } | undefined; expect(response?.error).toContain('not available inside subagents'); expect(response?.error).toContain('return your plan'); + expect(response?.error).not.toContain('not found'); }, ); }); diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index 3933294fa71..afdbe3c79e0 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -40,6 +40,7 @@ const { promptConfigSystemPrompt?: string; runConfig?: { max_turns?: number; max_time_minutes?: number }; toolConfig?: { tools?: string[]; disallowedTools?: string[] }; + agentId?: string | null; }>, nextFinalText: { value: undefined as string | undefined }, // T10 (PR #4732 R1): the production dispatch checks getTerminateMode() and @@ -133,6 +134,7 @@ vi.mock('./agent-headless.js', () => ({ ctx: { get: (k: string) => unknown }, signal?: AbortSignal, ) => { + const { getCurrentAgentId } = await import('./agent-context.js'); created.push({ name, prompt: ctx.get('task_prompt') as string, @@ -140,6 +142,7 @@ vi.mock('./agent-headless.js', () => ({ promptConfigSystemPrompt: promptConfig.systemPrompt, runConfig, toolConfig, + agentId: getCurrentAgentId(), }); if ( !promptConfig.systemPrompt?.includes('subagent spawned by a workflow') @@ -1093,6 +1096,7 @@ describe('createProductionDispatch', () => { expect(created.length).toBe(1); expect(created[0]!.name).toBe('h1'); expect(created[0]!.prompt).toBe('hello'); + expect(created[0]!.agentId).toMatch(/^workflow-agent-[0-9a-f]{8}$/); }); it('strips internal tags from fast-path final text', async () => { @@ -1775,6 +1779,7 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( runtimeContextSame: boolean; options?: { runConfigOverrides?: unknown }; eventEmitterAttached: boolean; + executeAgentId?: string | null; }; function fakeConfigWithMgr(opts: { @@ -1861,6 +1866,10 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( _ctx: unknown, signal?: AbortSignal, ): Promise => { + const { getCurrentAgentId } = await import( + './agent-context.js' + ); + call.executeAgentId = getCurrentAgentId(); if (outcome.runWithEmitter && options?.eventEmitter) { outcome.runWithEmitter( options.eventEmitter as { @@ -1929,10 +1938,14 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( }), }); const dispatch = createProductionDispatch(config); - const result = await dispatch('find foo', { agentType: 'Explore' }); + const result = await dispatch('find foo', { + agentType: 'Explore', + label: 'explore-1', + }); expect(result).toBe('explore-output'); expect(calls).toHaveLength(1); expect(calls[0].config.name).toBe('Explore'); + expect(calls[0].executeAgentId).toMatch(/^workflow-agent-[0-9a-f]{8}$/); // Workflow floor [SendMessage, EnterPlanMode, ExitPlanMode] must be unioned in. expect(calls[0].config.disallowedTools).toEqual( expect.arrayContaining([ diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index fdd16a53e7a..36efb045c5c 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -46,6 +46,7 @@ import { SyntheticOutputTool } from '../../tools/syntheticOutput.js'; import { rebuildToolRegistryOnOverride } from '../../tools/agent/agent.js'; import { toModelVisibleSubagentResult } from '../subagent-result.js'; import { SUBAGENT_PLAN_LIFECYCLE_TOOLS } from './subagent-plan-tool-policy.js'; +import { runWithAgentContext } from './agent-context.js'; /** * Default ceiling on total `agent()` calls per workflow run (matches upstream @@ -412,6 +413,7 @@ async function runSingleDispatch( const { AgentHeadless, ContextState } = await import('./agent-headless.js'); const ctx = new ContextState(); ctx.set('task_prompt', prompt); + const workflowAgentId = `workflow-agent-${randomBytes(4).toString('hex')}`; if ( opts.agentType === undefined && @@ -453,7 +455,9 @@ async function runSingleDispatch( // is valid inside the throw path because AgentHeadless's own // outer `finally` finalizes stats before propagating the error. try { - await subagent.execute(ctx, attemptSignal); + await runWithAgentContext(workflowAgentId, () => + subagent.execute(ctx, attemptSignal), + ); } finally { reportTokens(subagent, opts, onTokens); } @@ -471,7 +475,15 @@ async function runSingleDispatch( return toModelVisibleSubagentResult(subagent.getFinalText(), mode); } - return runOverridePath(config, ctx, opts, attemptSignal, onTokens, emitter); + return runOverridePath( + config, + ctx, + opts, + attemptSignal, + workflowAgentId, + onTokens, + emitter, + ); } /** @@ -541,6 +553,7 @@ async function runOverridePath( ctx: ContextState, opts: WorkflowAgentOpts, signal: AbortSignal | undefined, + workflowAgentId: string, /** * P5: forwarded from createProductionDispatch. The override path * builds its own AgentHeadless and runs subagent.execute(); the @@ -758,7 +771,9 @@ async function runOverridePath( // AgentHeadless's own outer `finally` finalizes stats before // propagating. try { - await subagent.execute(ctx, dispatchSignal); + await runWithAgentContext(workflowAgentId, () => + subagent.execute(ctx, dispatchSignal), + ); } finally { reportTokens(subagent, opts, onTokens); } diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index fe10bc520a8..4b21292b239 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -2104,6 +2104,8 @@ export class GeminiClient { // add plan mode system reminder if approval mode is plan if (this.config.getApprovalMode() === ApprovalMode.PLAN) { systemReminders.push( + // SDK clients do not receive the interactive exit-plan flow, so + // they need plan-only guidance even outside subagent contexts. getPlanModeSystemReminder( isSubagentLikeExecutionContext() || this.config.getSdkMode(), ), diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index d245b2bf854..d61b89de5d9 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -2269,6 +2269,8 @@ export class CoreToolScheduler { responseParts: convertToFunctionResponse( reqInfo.name, reqInfo.callId, + // Main SDK sessions can still exit plan mode explicitly; + // subagents cannot, so only subagents get plan-only guidance. getPlanModeSystemReminder(isSubagentLikeExecutionContext()), ), resultDisplay: 'Plan mode blocked a non-read-only tool call.', diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index 5b6088340cf..9c839d95d28 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -123,6 +123,8 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< */ override async getDefaultPermission(): Promise { if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)) { + // Avoid showing an approval UI for a subagent-only rejection; execute() + // still returns before saving the plan or changing approval mode. return 'allow'; } diff --git a/packages/core/src/tools/tool-search.test.ts b/packages/core/src/tools/tool-search.test.ts index fbba1ac79d4..a120d6096e0 100644 --- a/packages/core/src/tools/tool-search.test.ts +++ b/packages/core/src/tools/tool-search.test.ts @@ -659,6 +659,8 @@ describe('ToolSearchTool', () => { 'not available inside subagents', ); expect(String(result.llmContent)).toContain('return your plan'); + expect(result.error?.message).toContain('not available inside subagents'); + expect(result.error?.message).toContain('return your plan'); expect(String(result.llmContent)).not.toContain(`"name":"${toolName}"`); expect(registry.isDeferredToolRevealed(toolName)).toBe(false); expect(setToolsSpy).not.toHaveBeenCalled(); diff --git a/packages/core/src/tools/tool-search.ts b/packages/core/src/tools/tool-search.ts index b86cf640617..f1004a37188 100644 --- a/packages/core/src/tools/tool-search.ts +++ b/packages/core/src/tools/tool-search.ts @@ -435,11 +435,12 @@ class ToolSearchInvocation extends BaseToolInvocation< const header = llmContent ? '\n\n' : ''; llmContent += `${header}Not found: ${missing.join(', ')}`; } + const blockedMessages = blocked.map((name) => + getSubagentPlanToolUnavailableMessage(name), + ); if (blocked.length > 0) { const header = llmContent ? '\n\n' : ''; - llmContent += `${header}Unavailable: ${blocked - .map((name) => getSubagentPlanToolUnavailableMessage(name)) - .join('\n')}`; + llmContent += `${header}Unavailable: ${blockedMessages.join('\n')}`; } if (truncated.length > 0) { // Surface the dropped names so the model knows it must re-issue @@ -458,7 +459,11 @@ class ToolSearchInvocation extends BaseToolInvocation< displayParts.push(`${truncated.length} truncated`); const returnDisplay = displayParts.join(', ') || 'No tools loaded'; - return { llmContent, returnDisplay }; + const result: ToolResult = { llmContent, returnDisplay }; + if (blockedMessages.length > 0) { + result.error = { message: blockedMessages.join('\n') }; + } + return result; } } From 1bb8db86bc5022bf0d83a9cdbdaad92e0708be82 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 06:21:02 +0800 Subject: [PATCH 06/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../runtime/subagent-plan-tool-policy.test.ts | 85 +++++++++++++++++++ .../runtime/subagent-plan-tool-policy.ts | 24 ++++-- packages/core/src/core/client.test.ts | 27 ++++++ packages/core/src/tools/enterPlanMode.ts | 14 +-- packages/core/src/tools/exitPlanMode.test.ts | 29 +++++++ packages/core/src/tools/exitPlanMode.ts | 28 +++--- packages/core/src/tools/tool-search.test.ts | 37 ++++++++ packages/core/src/tools/tool-search.ts | 14 +-- 8 files changed, 224 insertions(+), 34 deletions(-) create mode 100644 packages/core/src/agents/runtime/subagent-plan-tool-policy.test.ts diff --git a/packages/core/src/agents/runtime/subagent-plan-tool-policy.test.ts b/packages/core/src/agents/runtime/subagent-plan-tool-policy.test.ts new file mode 100644 index 00000000000..accea87b1c0 --- /dev/null +++ b/packages/core/src/agents/runtime/subagent-plan-tool-policy.test.ts @@ -0,0 +1,85 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import { ToolNames } from '../../tools/tool-names.js'; +import { runWithTeammateIdentity } from '../team/identity.js'; +import { runWithAgentContext } from './agent-context.js'; +import { + buildSubagentPlanToolBlockedResult, + getSubagentPlanToolUnavailableMessage, + isPlanLifecycleToolUnavailableInSubagent, + isSubagentLikeExecutionContext, + SUBAGENT_PLAN_LIFECYCLE_TOOLS, +} from './subagent-plan-tool-policy.js'; + +describe('subagent plan tool policy', () => { + it('recognizes subagent and teammate execution contexts', async () => { + expect(isSubagentLikeExecutionContext()).toBe(false); + + await runWithAgentContext('agent-1', async () => { + expect(isSubagentLikeExecutionContext()).toBe(true); + }); + + runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + () => { + expect(isSubagentLikeExecutionContext()).toBe(true); + }, + ); + }); + + it('blocks only plan lifecycle tools inside subagent-like contexts', async () => { + expect(SUBAGENT_PLAN_LIFECYCLE_TOOLS.has(ToolNames.ENTER_PLAN_MODE)).toBe( + true, + ); + expect(SUBAGENT_PLAN_LIFECYCLE_TOOLS.has(ToolNames.EXIT_PLAN_MODE)).toBe( + true, + ); + expect( + isPlanLifecycleToolUnavailableInSubagent(ToolNames.ENTER_PLAN_MODE), + ).toBe(false); + + await runWithAgentContext('agent-1', async () => { + expect( + isPlanLifecycleToolUnavailableInSubagent(ToolNames.ENTER_PLAN_MODE), + ).toBe(true); + expect( + isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE), + ).toBe(true); + expect( + isPlanLifecycleToolUnavailableInSubagent(ToolNames.READ_FILE), + ).toBe(false); + }); + }); + + it('builds a logged blocked result with caller guidance', () => { + const logger = { warn: vi.fn() }; + + const result = buildSubagentPlanToolBlockedResult( + ToolNames.EXIT_PLAN_MODE, + 'ExitPlanModeTool', + logger, + ); + + const message = getSubagentPlanToolUnavailableMessage( + ToolNames.EXIT_PLAN_MODE, + ); + expect(result).toEqual({ + llmContent: message, + returnDisplay: message, + error: { message }, + }); + expect(logger.warn).toHaveBeenCalledWith( + `[ExitPlanModeTool] Blocked plan lifecycle tool call from subagent: ${ToolNames.EXIT_PLAN_MODE}`, + ); + }); +}); diff --git a/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts b/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts index 729d26cf5bc..fde058f45d3 100644 --- a/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts +++ b/packages/core/src/agents/runtime/subagent-plan-tool-policy.ts @@ -5,6 +5,7 @@ */ import { ToolNames } from '../../tools/tool-names.js'; +import type { ToolResult } from '../../tools/tools.js'; import { isTeammate } from '../team/identity.js'; import { getCurrentAgentId } from './agent-context.js'; @@ -17,15 +18,12 @@ export function isSubagentLikeExecutionContext(): boolean { return getCurrentAgentId() !== null || isTeammate(); } -export function isSubagentPlanLifecycleTool(toolName: string): boolean { - return SUBAGENT_PLAN_LIFECYCLE_TOOLS.has(toolName); -} - export function isPlanLifecycleToolUnavailableInSubagent( toolName: string, ): boolean { return ( - isSubagentLikeExecutionContext() && isSubagentPlanLifecycleTool(toolName) + isSubagentLikeExecutionContext() && + SUBAGENT_PLAN_LIFECYCLE_TOOLS.has(toolName) ); } @@ -34,3 +32,19 @@ export function getSubagentPlanToolUnavailableMessage( ): string { return `${toolName} is not available inside subagents or team agents. Plan mode is owned by the caller/main session; return your plan, findings, or constraints to the caller in your normal response instead of entering or exiting plan mode.`; } + +export function buildSubagentPlanToolBlockedResult( + toolName: string, + logTag: string, + logger: { warn(message: string): void }, +): ToolResult { + const message = getSubagentPlanToolUnavailableMessage(toolName); + logger.warn( + `[${logTag}] Blocked plan lifecycle tool call from subagent: ${toolName}`, + ); + return { + llmContent: message, + returnDisplay: message, + error: { message }, + }; +} diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index d48770cc70c..5a925efe2ee 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -4737,6 +4737,33 @@ hello expect(getPlanModeSystemReminder).toHaveBeenCalledWith(true); }); + it('uses the main-session plan reminder outside subagent and SDK mode', async () => { + vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); + vi.mocked(mockConfig.getSdkMode).mockReturnValue(false); + vi.mocked(getPlanModeSystemReminder).mockReturnValue( + 'call exit_plan_mode', + ); + const mockStream = (async function* () { + yield { type: 'content', value: 'Plan ready' }; + })(); + mockTurnRunFn.mockReturnValue(mockStream); + client['chat'] = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + } as unknown as GeminiChat; + + const stream = client.sendMessageStream( + [{ text: 'Plan this change' }], + new AbortController().signal, + 'prompt-id-main-plan-reminder', + ); + for await (const _ of stream) { + // consume stream + } + + expect(getPlanModeSystemReminder).toHaveBeenCalledWith(false); + }); + it('should not inject duplicate date on the same day', async () => { client['lastInjectedDate'] = undefined; vi.setSystemTime(new Date('2026-06-05T12:00:00Z')); diff --git a/packages/core/src/tools/enterPlanMode.ts b/packages/core/src/tools/enterPlanMode.ts index 0731e8b9604..e48507dd912 100644 --- a/packages/core/src/tools/enterPlanMode.ts +++ b/packages/core/src/tools/enterPlanMode.ts @@ -14,7 +14,7 @@ import { ToolDisplayNames, ToolNames } from './tool-names.js'; import { InputFormat } from '../output/types.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { - getSubagentPlanToolUnavailableMessage, + buildSubagentPlanToolBlockedResult, isPlanLifecycleToolUnavailableInSubagent, } from '../agents/runtime/subagent-plan-tool-policy.js'; @@ -69,17 +69,11 @@ class EnterPlanModeToolInvocation extends BaseToolInvocation< async execute(_signal: AbortSignal): Promise { if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.ENTER_PLAN_MODE)) { - const message = getSubagentPlanToolUnavailableMessage( + return buildSubagentPlanToolBlockedResult( ToolNames.ENTER_PLAN_MODE, + 'EnterPlanModeTool', + debugLogger, ); - debugLogger.warn( - `[EnterPlanModeTool] Blocked plan lifecycle tool call from subagent: ${ToolNames.ENTER_PLAN_MODE}`, - ); - return { - llmContent: message, - returnDisplay: message, - error: { message }, - }; } // In headless (non-interactive) mode without ACP support, the gate diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts index 24157356995..86ccccf22fa 100644 --- a/packages/core/src/tools/exitPlanMode.test.ts +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -377,6 +377,35 @@ describe('ExitPlanModeTool', () => { expect(permission).toBe('allow'); }); + it('allows by default inside teammate context to avoid approval UI', async () => { + const invocation = tool.build({ plan: 'Teammate plan' }); + + const permission = runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + () => invocation.getDefaultPermission(), + ); + + await expect(permission).resolves.toBe('allow'); + }); + + it('falls back to generic confirmation inside subagent context', async () => { + const invocation = tool.build({ plan: 'Subagent plan' }); + + const confirmation = await runWithAgentContext('agent-1', () => + invocation.getConfirmationDetails(new AbortController().signal), + ); + + expect(confirmation.type).toBe('info'); + await confirmation.onConfirm(ToolConfirmationOutcome.ProceedOnce); + expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); + expect(approvalMode).toBe(ApprovalMode.PLAN); + }); + it('rejects inside subagent context without saving or changing mode', async () => { approvalMode = ApprovalMode.PLAN; const invocation = tool.build({ plan: 'Subagent plan' }); diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index 9c839d95d28..86b26fc9493 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -4,7 +4,11 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { ToolPlanConfirmationDetails, ToolResult } from './tools.js'; +import type { + ToolCallConfirmationDetails, + ToolPlanConfirmationDetails, + ToolResult, +} from './tools.js'; import type { PermissionDecision } from '../permissions/types.js'; import { BaseDeclarativeTool, @@ -27,7 +31,7 @@ import { import type { EvidenceBundle } from '../plan-gate/types.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { - getSubagentPlanToolUnavailableMessage, + buildSubagentPlanToolBlockedResult, isPlanLifecycleToolUnavailableInSubagent, } from '../agents/runtime/subagent-plan-tool-policy.js'; @@ -142,8 +146,12 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< } override async getConfirmationDetails( - _abortSignal: AbortSignal, - ): Promise { + abortSignal: AbortSignal, + ): Promise { + if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)) { + return super.getConfirmationDetails(abortSignal); + } + const prePlanMode = this.config.getPrePlanMode(); const details: ToolPlanConfirmationDetails = { type: 'plan', @@ -206,17 +214,11 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< async execute(signal: AbortSignal): Promise { if (isPlanLifecycleToolUnavailableInSubagent(ToolNames.EXIT_PLAN_MODE)) { - const message = getSubagentPlanToolUnavailableMessage( + return buildSubagentPlanToolBlockedResult( ToolNames.EXIT_PLAN_MODE, + 'ExitPlanModeTool', + debugLogger, ); - debugLogger.warn( - `[ExitPlanModeTool] Blocked plan lifecycle tool call from subagent: ${ToolNames.EXIT_PLAN_MODE}`, - ); - return { - llmContent: message, - returnDisplay: message, - error: { message }, - }; } const { plan, originalRequest, researchSummary, resolutionSummary } = diff --git a/packages/core/src/tools/tool-search.test.ts b/packages/core/src/tools/tool-search.test.ts index a120d6096e0..b4ac3286736 100644 --- a/packages/core/src/tools/tool-search.test.ts +++ b/packages/core/src/tools/tool-search.test.ts @@ -661,12 +661,49 @@ describe('ToolSearchTool', () => { expect(String(result.llmContent)).toContain('return your plan'); expect(result.error?.message).toContain('not available inside subagents'); expect(result.error?.message).toContain('return your plan'); + expect(String(result.returnDisplay)).toContain('1 unavailable'); expect(String(result.llmContent)).not.toContain(`"name":"${toolName}"`); expect(registry.isDeferredToolRevealed(toolName)).toBe(false); expect(setToolsSpy).not.toHaveBeenCalled(); }, ); + it('select: loads allowed tools while rejecting plan lifecycle tools inside subagent context', async () => { + registry.registerTool( + new MockTool({ + name: ToolNames.READ_FILE, + shouldDefer: false, + }), + ); + registry.registerTool( + new MockTool({ + name: ToolNames.ENTER_PLAN_MODE, + shouldDefer: false, + }), + ); + + const tool = new ToolSearchTool(config); + const result = await runWithAgentContext('agent-1', () => + tool + .build({ + query: `select:${ToolNames.READ_FILE},${ToolNames.ENTER_PLAN_MODE}`, + }) + .execute(new AbortController().signal), + ); + + expect(String(result.llmContent)).toContain( + `"name":"${ToolNames.READ_FILE}"`, + ); + expect(String(result.llmContent)).not.toContain( + `"name":"${ToolNames.ENTER_PLAN_MODE}"`, + ); + expect(String(result.llmContent)).toContain( + 'not available inside subagents', + ); + expect(result.error?.message).toContain(ToolNames.ENTER_PLAN_MODE); + expect(result.returnDisplay).toBe('Loaded 1 tool(s), 1 unavailable'); + }); + it('+must-word filters candidates whose name does not contain the required term', async () => { // Both tools would match on "send" in description; only one has "slack" // in its name. The +slack prefix should narrow the result to that one. diff --git a/packages/core/src/tools/tool-search.ts b/packages/core/src/tools/tool-search.ts index f1004a37188..665d0999ce9 100644 --- a/packages/core/src/tools/tool-search.ts +++ b/packages/core/src/tools/tool-search.ts @@ -435,12 +435,14 @@ class ToolSearchInvocation extends BaseToolInvocation< const header = llmContent ? '\n\n' : ''; llmContent += `${header}Not found: ${missing.join(', ')}`; } - const blockedMessages = blocked.map((name) => - getSubagentPlanToolUnavailableMessage(name), - ); + let blockedErrorMessage: string | undefined; if (blocked.length > 0) { + const blockedMessages = blocked.map((name) => + getSubagentPlanToolUnavailableMessage(name), + ); + blockedErrorMessage = blockedMessages.join('\n'); const header = llmContent ? '\n\n' : ''; - llmContent += `${header}Unavailable: ${blockedMessages.join('\n')}`; + llmContent += `${header}Unavailable: ${blockedErrorMessage}`; } if (truncated.length > 0) { // Surface the dropped names so the model knows it must re-issue @@ -460,8 +462,8 @@ class ToolSearchInvocation extends BaseToolInvocation< const returnDisplay = displayParts.join(', ') || 'No tools loaded'; const result: ToolResult = { llmContent, returnDisplay }; - if (blockedMessages.length > 0) { - result.error = { message: blockedMessages.join('\n') }; + if (blockedErrorMessage) { + result.error = { message: blockedErrorMessage }; } return result; } From d8a3730fdf13d664a49a8f192d12334e2467fbbf Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 07:23:18 +0800 Subject: [PATCH 07/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../src/agents/runtime/agent-core.test.ts | 19 +++++++++++++++++-- .../core/src/agents/runtime/agent-core.ts | 14 +++++++++++--- packages/core/src/tools/tool-search.test.ts | 2 +- packages/core/src/tools/tool-search.ts | 2 +- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index c9a2d29be3d..dcf8be3de13 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -307,15 +307,17 @@ describe('AgentCore.prepareTools', () => { fnDeclarations: FunctionDeclaration[], ): { core: AgentCore; + debugSpy: ReturnType; getFunctionDeclarationsSpy: ReturnType; getFunctionDeclarationsFilteredSpy: ReturnType; } { + const debugSpy = vi.fn(); const getFunctionDeclarationsSpy = vi.fn().mockReturnValue(fnDeclarations); const getFunctionDeclarationsFilteredSpy = vi.fn((names: string[]) => fnDeclarations.filter((d) => d.name && names.includes(d.name)), ); const config = { - getDebugLogger: vi.fn().mockReturnValue({ debug: vi.fn() }), + getDebugLogger: vi.fn().mockReturnValue({ debug: debugSpy }), getToolRegistry: vi.fn().mockReturnValue({ warmAll: vi.fn().mockResolvedValue(undefined), getFunctionDeclarations: getFunctionDeclarationsSpy, @@ -333,6 +335,7 @@ describe('AgentCore.prepareTools', () => { ); return { core, + debugSpy, getFunctionDeclarationsSpy, getFunctionDeclarationsFilteredSpy, }; @@ -460,7 +463,7 @@ describe('AgentCore.prepareTools', () => { name: 'inline_safe', description: 'safe inline tool', } as FunctionDeclaration; - const { core } = buildAgentForTools( + const { core, debugSpy } = buildAgentForTools( { tools: [ { name: ToolNames.SEND_MESSAGE } as FunctionDeclaration, @@ -476,6 +479,18 @@ describe('AgentCore.prepareTools', () => { const tools = await core.prepareTools(); expect(tools).toEqual([inlineSafe]); + expect(debugSpy).toHaveBeenCalledWith( + `[prepareTools] Filtered inline declaration "${ToolNames.SEND_MESSAGE}" from subagent tool list`, + ); + expect(debugSpy).toHaveBeenCalledWith( + `[prepareTools] Filtered inline declaration "${ToolNames.TASK_UPDATE}" from subagent tool list`, + ); + expect(debugSpy).toHaveBeenCalledWith( + `[prepareTools] Filtered inline declaration "${ToolNames.ENTER_PLAN_MODE}" from subagent tool list`, + ); + expect(debugSpy).toHaveBeenCalledWith( + `[prepareTools] Filtered inline declaration "${ToolNames.EXIT_PLAN_MODE}" from subagent tool list`, + ); }); it('keeps teammate coordination tools but excludes plan lifecycle tools', async () => { diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 62001439edf..7ebf54a617a 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -514,9 +514,17 @@ export class AgentCore { } // Also filter inline FunctionDeclaration[] passed directly in toolConfig. toolsList.push( - ...onlyInlineDecls.filter( - (d) => !(d.name && excludedFromSubagents.has(d.name)), - ), + ...onlyInlineDecls.filter((d) => { + if (d.name && excludedFromSubagents.has(d.name)) { + this.runtimeContext + .getDebugLogger() + ?.debug( + `[prepareTools] Filtered inline declaration "${d.name}" from subagent tool list`, + ); + return false; + } + return true; + }), ); } else { // Inherit all available tools by default when not specified — see the diff --git a/packages/core/src/tools/tool-search.test.ts b/packages/core/src/tools/tool-search.test.ts index b4ac3286736..21c37b5d0e9 100644 --- a/packages/core/src/tools/tool-search.test.ts +++ b/packages/core/src/tools/tool-search.test.ts @@ -700,7 +700,7 @@ describe('ToolSearchTool', () => { expect(String(result.llmContent)).toContain( 'not available inside subagents', ); - expect(result.error?.message).toContain(ToolNames.ENTER_PLAN_MODE); + expect(result.error).toBeUndefined(); expect(result.returnDisplay).toBe('Loaded 1 tool(s), 1 unavailable'); }); diff --git a/packages/core/src/tools/tool-search.ts b/packages/core/src/tools/tool-search.ts index 665d0999ce9..4a70178882b 100644 --- a/packages/core/src/tools/tool-search.ts +++ b/packages/core/src/tools/tool-search.ts @@ -462,7 +462,7 @@ class ToolSearchInvocation extends BaseToolInvocation< const returnDisplay = displayParts.join(', ') || 'No tools loaded'; const result: ToolResult = { llmContent, returnDisplay }; - if (blockedErrorMessage) { + if (blockedErrorMessage && loaded.length === 0) { result.error = { message: blockedErrorMessage }; } return result; From d0670df799d101500967ee45ab3daa45ecd90a32 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 08:15:16 +0800 Subject: [PATCH 08/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../agents/runtime/workflow-orchestrator.test.ts | 14 ++++++++++---- .../src/agents/runtime/workflow-orchestrator.ts | 8 +++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index afdbe3c79e0..929549c6d5d 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -1160,13 +1160,15 @@ describe('createProductionDispatch', () => { }); }); - // T11: disallow SendMessage plus plan lifecycle tools to mirror upstream Tg8. - it('disallows SendMessage and plan lifecycle tools for workflow subagents', async () => { + // T11: disallow SendMessage plus tools that break workflow return/cleanup + // contracts. + it('disallows workflow-only floor tools for workflow subagents', async () => { const dispatch = createProductionDispatch(fakeConfig()); await dispatch('hello', { label: 'h1' }); expect(created[0]!.toolConfig?.tools).toEqual(['*']); expect(created[0]!.toolConfig?.disallowedTools).toEqual([ 'send_message', + 'monitor', 'enter_plan_mode', 'exit_plan_mode', ]); @@ -1946,10 +1948,12 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( expect(calls).toHaveLength(1); expect(calls[0].config.name).toBe('Explore'); expect(calls[0].executeAgentId).toMatch(/^workflow-agent-[0-9a-f]{8}$/); - // Workflow floor [SendMessage, EnterPlanMode, ExitPlanMode] must be unioned in. + // Workflow floor [SendMessage, Monitor, EnterPlanMode, ExitPlanMode] must + // be unioned in. expect(calls[0].config.disallowedTools).toEqual( expect.arrayContaining([ 'send_message', + 'monitor', 'enter_plan_mode', 'exit_plan_mode', ]), @@ -2026,11 +2030,12 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( const dispatch = createProductionDispatch(config); await dispatch('hi', { agentType: 'Permissive' }); const disallowed = calls[0].config.disallowedTools ?? []; - // Union: Foo (from agentType) + send_message + plan lifecycle floor. + // Union: Foo (from agentType) + workflow-only floor. expect(disallowed).toEqual( expect.arrayContaining([ 'Foo', 'send_message', + 'monitor', 'enter_plan_mode', 'exit_plan_mode', ]), @@ -2430,6 +2435,7 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( expect.arrayContaining([ 'Foo', 'send_message', + 'monitor', 'enter_plan_mode', 'exit_plan_mode', ]), diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index 36efb045c5c..e7e08ad662b 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -156,12 +156,14 @@ const WORKFLOW_SUBAGENT_MAX_TIME_MINUTES = 10; * disallowedTools mirror the upstream `Tg8` workflow-subagent config. These * tools would let a subagent break the "final text IS the return value" * contract: SendMessage would deliver the answer to the user instead of - * the calling script, and plan lifecycle tools would interrupt the workflow's - * plan-mode intent. Defense-in-depth alongside the §XmO system prompt that - * already documents these restrictions. + * the calling script, plan lifecycle tools would interrupt the workflow's + * plan-mode intent, and MonitorTool depends on AgentTool-owned notification + * callbacks that workflow subagents do not register. Defense-in-depth alongside + * the workflow system prompt's return-value contract. */ const WORKFLOW_SUBAGENT_DISALLOWED_TOOLS: string[] = [ ToolNames.SEND_MESSAGE, + ToolNames.MONITOR, ...SUBAGENT_PLAN_LIFECYCLE_TOOLS, ]; From 476ec7a8620da5b4e8bc467433b0a69a5b6f1e7e Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 08:22:43 +0800 Subject: [PATCH 09/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../src/agents/runtime/agent-core.test.ts | 27 ++++++++------ .../core/src/agents/runtime/agent-core.ts | 15 ++++++-- .../runtime/workflow-orchestrator.test.ts | 35 ++++++++++--------- .../agents/runtime/workflow-orchestrator.ts | 15 +++++--- 4 files changed, 58 insertions(+), 34 deletions(-) diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index dcf8be3de13..f7f0d451ead 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -439,16 +439,17 @@ describe('AgentCore.prepareTools', () => { description: 'exit plan mode', } as FunctionDeclaration, ]; - const { core, getFunctionDeclarationsFilteredSpy } = buildAgentForTools( - { - tools: [ - ToolNames.READ_FILE, - ToolNames.ENTER_PLAN_MODE, - ToolNames.EXIT_PLAN_MODE, - ], - }, - fnDecls, - ); + const { core, debugSpy, getFunctionDeclarationsFilteredSpy } = + buildAgentForTools( + { + tools: [ + ToolNames.READ_FILE, + ToolNames.ENTER_PLAN_MODE, + ToolNames.EXIT_PLAN_MODE, + ], + }, + fnDecls, + ); const tools = await core.prepareTools(); @@ -456,6 +457,12 @@ describe('AgentCore.prepareTools', () => { ToolNames.READ_FILE, ]); expect(tools.map((t) => t.name)).toEqual([ToolNames.READ_FILE]); + expect(debugSpy).toHaveBeenCalledWith( + `[prepareTools] Filtered "${ToolNames.ENTER_PLAN_MODE}" from explicit subagent tool list`, + ); + expect(debugSpy).toHaveBeenCalledWith( + `[prepareTools] Filtered "${ToolNames.EXIT_PLAN_MODE}" from explicit subagent tool list`, + ); }); it('filters inline declarations using the full subagent exclusion floor', async () => { diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 7ebf54a617a..4db58280166 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -506,10 +506,19 @@ export class AgentCore { // the recursion guard). This prevents control-plane tools // (CRON_CREATE, TASK_STOP, SEND_MESSAGE, etc.) from leaking into // explicitly-configured subagents that happen to list them. + const allowedNames = asStrings.filter((name) => { + if (excludedFromSubagents.has(name)) { + this.runtimeContext + .getDebugLogger() + ?.debug( + `[prepareTools] Filtered "${name}" from explicit subagent tool list`, + ); + return false; + } + return true; + }); toolsList.push( - ...toolRegistry.getFunctionDeclarationsFiltered( - asStrings.filter((name) => !excludedFromSubagents.has(name)), - ), + ...toolRegistry.getFunctionDeclarationsFiltered(allowedNames), ); } // Also filter inline FunctionDeclaration[] passed directly in toolConfig. diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index 929549c6d5d..36b3ce5df26 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -1096,7 +1096,7 @@ describe('createProductionDispatch', () => { expect(created.length).toBe(1); expect(created[0]!.name).toBe('h1'); expect(created[0]!.prompt).toBe('hello'); - expect(created[0]!.agentId).toMatch(/^workflow-agent-[0-9a-f]{8}$/); + expect(created[0]!.agentId).toMatch(/^workflow-agent-[0-9a-f]{16}$/); }); it('strips internal tags from fast-path final text', async () => { @@ -1178,20 +1178,19 @@ describe('createProductionDispatch', () => { // subagent terminates with a non-GOAL mode. Without this, `await agent(...)` // would resolve to '' on user cancel and the script would keep running. it.each([ - ['CANCELLED', /terminate mode: CANCELLED/], - ['MAX_TURNS', /terminate mode: MAX_TURNS/], - ['TIMEOUT', /terminate mode: TIMEOUT/], - ['ERROR', /terminate mode: ERROR/], - ])( - 'throws when subagent terminate mode is %s', - async (mode, expectedMessage) => { - nextTerminateMode.value = mode; - const dispatch = createProductionDispatch(fakeConfig()); - await expect(dispatch('hello', { label: 'h1' })).rejects.toThrow( - expectedMessage, - ); - }, - ); + ['CANCELLED'], + ['MAX_TURNS'], + ['TIMEOUT'], + ['ERROR'], + ])('throws when subagent terminate mode is %s', async (mode) => { + nextTerminateMode.value = mode; + const dispatch = createProductionDispatch(fakeConfig()); + await expect(dispatch('hello', { label: 'h1' })).rejects.toThrow( + new RegExp( + `workflow-agent-[0-9a-f]{16} did not complete \\(terminate mode: ${mode}\\)\\.`, + ), + ); + }); // ── R1 (#1 + #3): token reporting across all terminate modes ────────── @@ -1947,7 +1946,7 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( expect(result).toBe('explore-output'); expect(calls).toHaveLength(1); expect(calls[0].config.name).toBe('Explore'); - expect(calls[0].executeAgentId).toMatch(/^workflow-agent-[0-9a-f]{8}$/); + expect(calls[0].executeAgentId).toMatch(/^workflow-agent-[0-9a-f]{16}$/); // Workflow floor [SendMessage, Monitor, EnterPlanMode, ExitPlanMode] must // be unioned in. expect(calls[0].config.disallowedTools).toEqual( @@ -3033,7 +3032,9 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( await expect( dispatch('extract', { schema: { type: 'object' } }), ).rejects.toThrow( - new RegExp(`did not complete \\(terminate mode: ${mode}\\)\\.`), + new RegExp( + `workflow-agent-[0-9a-f]{16} did not complete \\(terminate mode: ${mode}\\)\\.`, + ), ); }, ); diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index e7e08ad662b..64a3657aae5 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -415,7 +415,8 @@ async function runSingleDispatch( const { AgentHeadless, ContextState } = await import('./agent-headless.js'); const ctx = new ContextState(); ctx.set('task_prompt', prompt); - const workflowAgentId = `workflow-agent-${randomBytes(4).toString('hex')}`; + const workflowAgentId = `workflow-agent-${randomBytes(8).toString('hex')}`; + debugLogger.debug(`[workflow] Dispatch ${workflowAgentId}`); if ( opts.agentType === undefined && @@ -457,6 +458,9 @@ async function runSingleDispatch( // is valid inside the throw path because AgentHeadless's own // outer `finally` finalizes stats before propagating the error. try { + // runWithAgentContext is load-bearing for workflow subagents: it + // establishes the ALS frame that isSubagentLikeExecutionContext() reads, + // so plan lifecycle tools remain blocked if tool filtering changes. await runWithAgentContext(workflowAgentId, () => subagent.execute(ctx, attemptSignal), ); @@ -471,7 +475,7 @@ async function runSingleDispatch( const mode = subagent.getTerminateMode(); if (mode !== AgentTerminateMode.GOAL) { throw new Error( - `Workflow subagent did not complete (terminate mode: ${mode}).`, + `Workflow subagent ${workflowAgentId} did not complete (terminate mode: ${mode}).`, ); } return toModelVisibleSubagentResult(subagent.getFinalText(), mode); @@ -773,6 +777,9 @@ async function runOverridePath( // AgentHeadless's own outer `finally` finalizes stats before // propagating. try { + // runWithAgentContext is load-bearing for workflow subagents: it + // establishes the ALS frame that isSubagentLikeExecutionContext() reads, + // so plan lifecycle tools remain blocked if tool filtering changes. await runWithAgentContext(workflowAgentId, () => subagent.execute(ctx, dispatchSignal), ); @@ -822,7 +829,7 @@ async function runOverridePath( mode !== AgentTerminateMode.CANCELLED ) { throw new Error( - `Workflow subagent did not complete (terminate mode: ${mode}).`, + `Workflow subagent ${workflowAgentId} did not complete (terminate mode: ${mode}).`, ); } // The dispatch aborts via schemaState.abortController on the @@ -849,7 +856,7 @@ async function runOverridePath( const mode = subagent.getTerminateMode(); if (mode !== AgentTerminateMode.GOAL) { throw new Error( - `Workflow subagent did not complete (terminate mode: ${mode}).`, + `Workflow subagent ${workflowAgentId} did not complete (terminate mode: ${mode}).`, ); } let finalText: WorkflowAgentResult = toModelVisibleSubagentResult( From 93d69db448ab1c1eab652d1f6adaa75b1de46266 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 08:35:10 +0800 Subject: [PATCH 10/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- packages/core/src/core/client.test.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 5a925efe2ee..1bd4e01e05d 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -4737,6 +4737,33 @@ hello expect(getPlanModeSystemReminder).toHaveBeenCalledWith(true); }); + it('uses the subagent plan reminder when SDK mode is active', async () => { + vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); + vi.mocked(mockConfig.getSdkMode).mockReturnValue(true); + vi.mocked(getPlanModeSystemReminder).mockReturnValue( + 'return plan to caller', + ); + const mockStream = (async function* () { + yield { type: 'content', value: 'Plan ready' }; + })(); + mockTurnRunFn.mockReturnValue(mockStream); + client['chat'] = { + addHistory: vi.fn(), + getHistory: vi.fn().mockReturnValue([]), + } as unknown as GeminiChat; + + const stream = client.sendMessageStream( + [{ text: 'Plan this change' }], + new AbortController().signal, + 'prompt-id-sdk-plan-reminder', + ); + for await (const _ of stream) { + // consume stream + } + + expect(getPlanModeSystemReminder).toHaveBeenCalledWith(true); + }); + it('uses the main-session plan reminder outside subagent and SDK mode', async () => { vi.mocked(mockConfig.getApprovalMode).mockReturnValue(ApprovalMode.PLAN); vi.mocked(mockConfig.getSdkMode).mockReturnValue(false); From 80695fc3a52f115ebf208ba7ae28059d49a26fd5 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Wed, 1 Jul 2026 09:39:55 +0800 Subject: [PATCH 11/11] codex: address PR review feedback (#6087) Co-authored-by: Qwen-Coder --- .../core/src/core/coreToolScheduler.test.ts | 109 ++++++++++++------ packages/core/src/core/coreToolScheduler.ts | 9 +- packages/core/src/tools/tool-search.test.ts | 60 +++++++--- packages/core/src/tools/workflow/workflow.ts | 4 +- 4 files changed, 122 insertions(+), 60 deletions(-) diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index a33134ba9fd..6514f8f54e4 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -61,6 +61,7 @@ import { ShellTool, ShellToolInvocation } from '../tools/shell.js'; import type { ShellToolParams } from '../tools/shell.js'; import type { ShellExecutionConfig } from '../services/shellExecutionService.js'; import { runWithAgentContext } from '../agents/runtime/agent-context.js'; +import { runWithTeammateIdentity } from '../agents/team/identity.js'; type ToolSpanRecord = { name: string; @@ -5800,6 +5801,7 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => { tool: MockTool, onAllToolCallsComplete: ReturnType, onToolCallsUpdate: ReturnType, + options: { sdkMode?: boolean } = {}, ) { const mockToolRegistry = { getTool: () => tool, @@ -5821,6 +5823,7 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => { getUsageStatisticsEnabled: () => true, getDebugMode: () => false, getApprovalMode: () => ApprovalMode.PLAN, + getSdkMode: () => options.sdkMode ?? false, getPermissionsAllow: () => [], getContentGeneratorConfig: () => ({ model: 'test-model', @@ -5998,48 +6001,77 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => { } }); - it('should tell subagents to return the plan directly when plan mode blocks a tool', async () => { - const editTool = new MockTool({ - name: 'write_file', - getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, - getConfirmationDetails: MOCK_TOOL_GET_CONFIRMATION_DETAILS, - }); - const onAllToolCallsComplete = vi.fn(); - const onToolCallsUpdate = vi.fn(); - const scheduler = createPlanModeScheduler( - editTool, - onAllToolCallsComplete, - onToolCallsUpdate, - ); + it.each([ + { + label: 'subagents', + promptId: 'prompt-plan-subagent-blocked', + run: (schedule: () => Promise) => + runWithAgentContext('agent-1', schedule), + }, + { + label: 'teammates', + promptId: 'prompt-plan-teammate-blocked', + run: (schedule: () => Promise) => + runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + schedule, + ), + }, + { + label: 'SDK callers', + promptId: 'prompt-plan-sdk-blocked', + schedulerOptions: { sdkMode: true }, + run: (schedule: () => Promise) => schedule(), + }, + ])( + 'should tell $label to return the plan directly when plan mode blocks a tool', + async ({ promptId, run, schedulerOptions }) => { + const editTool = new MockTool({ + name: 'write_file', + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: MOCK_TOOL_GET_CONFIRMATION_DETAILS, + }); + const onAllToolCallsComplete = vi.fn(); + const onToolCallsUpdate = vi.fn(); + const scheduler = createPlanModeScheduler( + editTool, + onAllToolCallsComplete, + onToolCallsUpdate, + schedulerOptions, + ); - const abortController = new AbortController(); - const request = { - callId: '1', - name: 'write_file', - args: {}, - isClientInitiated: false, - prompt_id: 'prompt-plan-subagent-blocked', - }; + const abortController = new AbortController(); + const request = { + callId: '1', + name: 'write_file', + args: {}, + isClientInitiated: false, + prompt_id: promptId, + }; - await runWithAgentContext('agent-1', () => - scheduler.schedule([request], abortController.signal), - ); + await run(() => scheduler.schedule([request], abortController.signal)); - await vi.waitFor(() => { - expect(onAllToolCallsComplete).toHaveBeenCalled(); - }); + await vi.waitFor(() => { + expect(onAllToolCallsComplete).toHaveBeenCalled(); + }); - const completedCalls = onAllToolCallsComplete.mock - .calls[0][0] as ToolCall[]; - expect(completedCalls[0].status).toBe('error'); - if (completedCalls[0].status === 'error') { - const responseText = JSON.stringify( - completedCalls[0].response.responseParts, - ); - expect(responseText).toContain('Present your plan directly'); - expect(responseText).not.toContain('exit_plan_mode tool'); - } - }); + const completedCalls = onAllToolCallsComplete.mock + .calls[0][0] as ToolCall[]; + expect(completedCalls[0].status).toBe('error'); + if (completedCalls[0].status === 'error') { + const responseText = JSON.stringify( + completedCalls[0].response.responseParts, + ); + expect(responseText).toContain('Present your plan directly'); + expect(responseText).not.toContain('exit_plan_mode tool'); + } + }, + ); it('should allow info confirmation tools in plan mode after approval', async () => { const onConfirmSpy = vi.fn().mockResolvedValue(undefined); @@ -8142,6 +8174,7 @@ describe('CoreToolScheduler telemetry spans', () => { getUsageStatisticsEnabled: () => true, getDebugMode: () => false, getApprovalMode: () => ApprovalMode.PLAN, + getSdkMode: () => false, getPermissionsAllow: () => [], getContentGeneratorConfig: () => ({}), getShellExecutionConfig: () => ({ diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index d61b89de5d9..fd93f83c389 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -2269,9 +2269,12 @@ export class CoreToolScheduler { responseParts: convertToFunctionResponse( reqInfo.name, reqInfo.callId, - // Main SDK sessions can still exit plan mode explicitly; - // subagents cannot, so only subagents get plan-only guidance. - getPlanModeSystemReminder(isSubagentLikeExecutionContext()), + // SDK and subagent-like callers should return the plan + // directly instead of entering an interactive approval flow. + getPlanModeSystemReminder( + isSubagentLikeExecutionContext() || + this.config.getSdkMode(), + ), ), resultDisplay: 'Plan mode blocked a non-read-only tool call.', error: undefined, diff --git a/packages/core/src/tools/tool-search.test.ts b/packages/core/src/tools/tool-search.test.ts index 21c37b5d0e9..7281e282aff 100644 --- a/packages/core/src/tools/tool-search.test.ts +++ b/packages/core/src/tools/tool-search.test.ts @@ -12,12 +12,14 @@ import { ToolRegistry } from './tool-registry.js'; import { DiscoveredMCPTool } from './mcp-tool.js'; import { MockTool } from '../test-utils/mock-tool.js'; import { ToolSearchTool, scoreTool, tokenize } from './tool-search.js'; +import type { ToolResult } from './tools.js'; import { CronCreateTool } from './cron-create.js'; import { CronDeleteTool } from './cron-delete.js'; import { CronListTool } from './cron-list.js'; import { LoopWakeupTool } from './loop-wakeup.js'; import { ToolNames } from './tool-names.js'; import { runWithAgentContext } from '../agents/runtime/agent-context.js'; +import { runWithTeammateIdentity } from '../agents/team/identity.js'; const baseConfigParams: ConfigParameters = { cwd: '/tmp', @@ -633,7 +635,7 @@ describe('ToolSearchTool', () => { alwaysLoad: true, }, ])( - 'select: rejects $toolName inside subagent context without revealing or syncing tools', + 'select: rejects $toolName inside subagent-like context without revealing or syncing tools', async ({ toolName, shouldDefer, alwaysLoad }) => { registry.registerTool( new MockTool({ @@ -649,22 +651,46 @@ describe('ToolSearchTool', () => { } as never); const tool = new ToolSearchTool(config); - const result = await runWithAgentContext('agent-1', () => - tool - .build({ query: `select:${toolName}` }) - .execute(new AbortController().signal), - ); - - expect(String(result.llmContent)).toContain( - 'not available inside subagents', - ); - expect(String(result.llmContent)).toContain('return your plan'); - expect(result.error?.message).toContain('not available inside subagents'); - expect(result.error?.message).toContain('return your plan'); - expect(String(result.returnDisplay)).toContain('1 unavailable'); - expect(String(result.llmContent)).not.toContain(`"name":"${toolName}"`); - expect(registry.isDeferredToolRevealed(toolName)).toBe(false); - expect(setToolsSpy).not.toHaveBeenCalled(); + const contextCases: Array<{ + run: (callback: () => Promise) => Promise; + }> = [ + { + run: (callback) => runWithAgentContext('agent-1', callback), + }, + { + run: (callback) => + runWithTeammateIdentity( + { + agentId: 'agent@test', + agentName: 'agent', + teamName: 'test', + isTeamLead: false, + }, + callback, + ), + }, + ]; + + for (const { run } of contextCases) { + const result = await run(() => + tool + .build({ query: `select:${toolName}` }) + .execute(new AbortController().signal), + ); + + expect(String(result.llmContent)).toContain( + 'not available inside subagents', + ); + expect(String(result.llmContent)).toContain('return your plan'); + expect(result.error?.message).toContain( + 'not available inside subagents', + ); + expect(result.error?.message).toContain('return your plan'); + expect(String(result.returnDisplay)).toContain('1 unavailable'); + expect(String(result.llmContent)).not.toContain(`"name":"${toolName}"`); + expect(registry.isDeferredToolRevealed(toolName)).toBe(false); + expect(setToolsSpy).not.toHaveBeenCalled(); + } }, ); diff --git a/packages/core/src/tools/workflow/workflow.ts b/packages/core/src/tools/workflow/workflow.ts index 89817185ebc..0895275f598 100644 --- a/packages/core/src/tools/workflow/workflow.ts +++ b/packages/core/src/tools/workflow/workflow.ts @@ -112,8 +112,8 @@ const WORKFLOW_PARAM_SCHEMA = { 'in this build" (parity with upstream). isolation=worktree refuses to ' + 'run when the parent working tree has uncommitted changes (the subagent ' + 'would see a stale HEAD). ' + - 'Workflow subagents always have SendMessage / EnterPlanMode / ExitPlanMode in their ' + - 'disallowed-tool floor regardless of agentType. ' + + 'Workflow subagents always have SendMessage / Monitor / EnterPlanMode / ExitPlanMode ' + + 'in their disallowed-tool floor regardless of agentType. ' + 'Concurrency: `parallel([() => agent(...), ...])` runs thunks ' + 'through a shared per-run window (default ' + '`max(1, min(16, cpus-2))` agents in flight; override via ' +