From b8e8046979a9ed381aafd03faab3e4f5ffd80003 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 2 Jul 2026 10:26:10 +0800 Subject: [PATCH 1/9] feat(core): allow bounded nested sub-agent spawning via maxSubagentDepth Sub-agents may now spawn sub-agents up to a configurable maximum nesting depth (default 5; 1 reproduces the previous no-nesting behavior). Enforced in two layers sharing one predicate: prepareTools() hides the agent tool from leaf-depth sub-agents, and AgentTool.execute() rejects over-depth spawns as an authoritative backstop. Teammates, forks, and the workflow tool remain excluded from nesting. Launch depth is persisted in the agent meta sidecar and restored on resume (including deferred-approval continuations and in-process AgentInteractive frames) so a resumed nested agent cannot regain spawn capacity. See knowledge/qwen-code/design/nested-subagents.md. --- packages/cli/src/config/config.ts | 2 + packages/cli/src/config/settingsSchema.ts | 10 ++ packages/core/src/agents/agent-transcript.ts | 8 ++ .../src/agents/background-agent-resume.ts | 6 +- .../src/agents/runtime/agent-context.test.ts | 26 ++-- .../core/src/agents/runtime/agent-context.ts | 46 +++++- .../src/agents/runtime/agent-core.test.ts | 135 +++++++++++++++++- .../core/src/agents/runtime/agent-core.ts | 70 ++++++++- .../agents/runtime/agent-interactive.test.ts | 42 ++++++ .../src/agents/runtime/agent-interactive.ts | 43 +++++- packages/core/src/config/config.test.ts | 39 +++++ packages/core/src/config/config.ts | 21 +++ .../src/tools/agent/agent-override.test.ts | 4 + packages/core/src/tools/agent/agent.test.ts | 105 +++++++++++++- packages/core/src/tools/agent/agent.ts | 118 +++++++++++---- .../schemas/settings.schema.json | 5 + 16 files changed, 623 insertions(+), 57 deletions(-) diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 42896814869..a5d1f1745e7 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -1996,6 +1996,8 @@ export async function loadCliConfig( argv.maxSessionTurns ?? settings.model?.maxSessionTurns ?? -1, maxWallTimeSeconds: resolveMaxWallTimeSeconds(argv, settings), maxToolCalls: resolveMaxToolCalls(argv, settings), + // Undefined flows through to Config's default (5) and clamp logic. + maxSubagentDepth: settings.model?.maxSubagentDepth, experimentalZedIntegration: argv.acp || argv.experimentalAcp || false, cronEnabled: settings.experimental?.cron ?? true, agentTeamEnabled: settings.experimental?.agentTeam ?? false, diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index cac410f8ad3..7effa86743d 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1360,6 +1360,16 @@ const SETTINGS_SCHEMA = { 'Cumulative tool-call budget for a run (counts every executed tool, success or failure; structured_output under --json-schema is exempt). -1 means unlimited; 0 means "no tool calls allowed" (first call aborts). Capped at 1,000,000 to catch typos. Overridable via --max-tool-calls.', showInDialog: false, }, + maxSubagentDepth: { + type: 'number', + label: 'Max Sub-agent Nesting Depth', + category: 'Model', + requiresRestart: false, + default: 5, + description: + 'Maximum number of nested sub-agent levels. 1 disables nesting (sub-agents cannot spawn sub-agents); the default 5 allows a sub-agent to spawn sub-agents up to five levels deep. Values below 1 are clamped to 1. Teammates, forks, and the workflow tool are excluded from nesting.', + showInDialog: false, + }, chatCompression: { type: 'object', label: 'Chat Compression', diff --git a/packages/core/src/agents/agent-transcript.ts b/packages/core/src/agents/agent-transcript.ts index b6f62afe68b..3949279d75c 100644 --- a/packages/core/src/agents/agent-transcript.ts +++ b/packages/core/src/agents/agent-transcript.ts @@ -121,6 +121,8 @@ export interface AgentMeta { agentColor?: string; /** Number of explicit resume attempts performed so far. */ resumeCount?: number; + /** Nesting depth at launch time; restored on background/foreground resume. */ + depth?: number; /** Last terminal error, if any. */ lastError?: string; } @@ -135,6 +137,12 @@ export interface AgentPersistedCliFlags { model?: string; maxSessionTurns?: number; maxToolCalls?: number; + /** + * Launch-time nesting cap. Interprets the persisted `depth` — without it a + * nested agent launched under a lower cap would resume after restart under + * the new session's (or default) cap and regain spawn capacity. + */ + maxSubagentDepth?: number; } /** diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index 898460aa92a..5cf7a4a1a3f 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -1028,7 +1028,11 @@ export class BackgroundAgentResumeService { } }; - const framedRunBody = () => runWithAgentContext(meta.agentId, runBody); + // Restore the persisted launch depth so a resumed nested agent keeps + // its original nesting level (and spawn eligibility) instead of + // recomputing to depth 0 from this top-level resume frame. + const framedRunBody = () => + runWithAgentContext(meta.agentId, runBody, meta.depth); void (target.isFork ? runInForkContext(framedRunBody) : framedRunBody()); return entry; } catch (error) { diff --git a/packages/core/src/agents/runtime/agent-context.test.ts b/packages/core/src/agents/runtime/agent-context.test.ts index fabefd9dc84..dfb454a4b34 100644 --- a/packages/core/src/agents/runtime/agent-context.test.ts +++ b/packages/core/src/agents/runtime/agent-context.test.ts @@ -200,16 +200,20 @@ describe('agent-context (depth) — #3731 Phase 3', () => { }); }); - it('callers do not pass depth — it is computed from parent frame only', async () => { - // Defensive: confirm `runWithAgentContext`'s signature still takes - // only (agentId, fn). Phase 3 depth tracking must remain a - // caller-invisible internal concern. - await runWithAgentContext('outer', async () => { - const before = getCurrentAgentDepth(); - // No way to pass depth in — the helper computes it. - await runWithAgentContext('inner', async () => { - expect(getCurrentAgentDepth()).toBe(before + 1); - }); - }); + it('depthOverride pins the depth for background/foreground resume', async () => { + // Resume runs from a top-level frame; without the override a resumed + // nested agent would recompute to depth 0 and regain spawn capacity. + // Passing the persisted launch depth restores the original level, and a + // child frame still auto-increments from the restored value. + await runWithAgentContext( + 'resumed', + async () => { + expect(getCurrentAgentDepth()).toBe(3); + await runWithAgentContext('child', async () => { + expect(getCurrentAgentDepth()).toBe(4); + }); + }, + 3, + ); }); }); diff --git a/packages/core/src/agents/runtime/agent-context.ts b/packages/core/src/agents/runtime/agent-context.ts index ef0636bbaef..fa1189797d4 100644 --- a/packages/core/src/agents/runtime/agent-context.ts +++ b/packages/core/src/agents/runtime/agent-context.ts @@ -46,12 +46,16 @@ const storage = new AsyncLocalStorage(); export function runWithAgentContext( agentId: string, fn: () => Promise, + depthOverride?: number, ): Promise { const current = storage.getStore() ?? {}; // Auto-increment depth: top-level = 0, nested = parent+1. No caller has // to know about it; telemetry reads it back via getCurrentAgentDepth - // (#3731 Phase 3 subagent spans). - const depth = (current.depth ?? -1) + 1; + // (#3731 Phase 3 subagent spans). depthOverride restores the original + // launch depth on background/foreground resume — otherwise the frame + // recomputes from a top-level (depth 0) parent and a resumed nested agent + // would regain spawn capacity it should not have. + const depth = depthOverride ?? (current.depth ?? -1) + 1; return storage.run({ ...current, agentId, depth }, fn); } @@ -89,3 +93,41 @@ export function getRuntimeContentGenerator(): | undefined { return storage.getStore()?.runtimeView; } + +/** + * True when there is no active agent frame — i.e. we are in the top-level + * user session, not inside a sub-agent. The canonical "top-level only" + * predicate for gating capabilities (teammate spawning, forking) that must + * not be reachable from a nested sub-agent. + */ +export function isTopLevelSession(): boolean { + return getCurrentAgentId() === null; +} + +/** + * The 0-based depth a child spawned by the current invoker would have: + * 0 when spawning from the top-level session (no agent frame), parent + * depth + 1 inside a sub-agent frame. Single source of the launch-depth + * formula — used for subagent telemetry spans, persisted in AgentMeta so + * background/foreground resume can restore the original nesting level (via + * the runWithAgentContext depthOverride), and underlying + * {@link canSpawnNestedAgent}. + */ +export function childLaunchDepth(): number { + return getCurrentAgentId() !== null ? getCurrentAgentDepth() + 1 : 0; +} + +/** + * Whether the current invoker may spawn a nested sub-agent given the + * configured maximum nesting depth (1-based levels; a top-level sub-agent is + * level 1). Single source of truth for the depth relationship, shared by + * AgentCore.prepareTools() (schema gating) and AgentTool.execute() (runtime + * guard) so the two cannot drift apart. + * + * The would-be child sits at level `childLaunchDepth() + 1` (levels are + * 1-based, depths 0-based), which must not exceed `maxDepth`. See + * knowledge/qwen-code/design/nested-subagents.md. + */ +export function canSpawnNestedAgent(maxDepth: number): boolean { + return childLaunchDepth() + 1 <= maxDepth; +} diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index bb793b6b44f..db64f98fa49 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect, vi } from 'vitest'; import type { FunctionDeclaration } from '@google/genai'; import { AgentCore } from './agent-core.js'; import { + getCurrentAgentDepth, getCurrentAgentId, getRuntimeContentGenerator, runWithAgentContext, @@ -15,6 +16,7 @@ import { type RuntimeContentGeneratorView, } from './agent-context.js'; import { subagentNameContext } from '../../utils/subagentNameContext.js'; +import { ToolNames } from '../../tools/tool-names.js'; import { getAgentName, getTeammateContext, @@ -33,7 +35,6 @@ 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 @@ -221,6 +222,49 @@ describe('AgentCore.runInAgentFrames', () => { expect(observedAgentId).toBe('agent-123'); }); + it('restores the nesting depth for deferred-approval continuations', async () => { + // Regression (codex review): the respond closure captured only the agent + // id, so runWithAgentContext recomputed depth 0 from the UI's frame-less + // chain — a deferred-approved `agent` tool call from a leaf-depth + // sub-agent would then bypass maxSubagentDepth. The closure must carry + // the depth captured at emit time. + const core = makeCore('approval-agent'); + + let respondClosure: (() => Promise) | undefined; + let observedDepth: number | null = null; + const onConfirm = async () => { + observedDepth = getCurrentAgentDepth(); + }; + + // Emit from a nested frame (depth 2), mirroring a sub-agent of a + // sub-agent whose tool call parks for approval. + await runWithAgentContext('lvl1', () => + runWithAgentContext('lvl2', () => + runWithAgentContext('lvl3', async () => { + const inheritedAgentId = getCurrentAgentId(); + const inheritedAgentDepth = getCurrentAgentDepth(); + expect(inheritedAgentDepth).toBe(2); + respondClosure = () => + core.runInAgentFrames( + onConfirm, + undefined, + inheritedAgentId ?? undefined, + undefined, + inheritedAgentDepth, + ); + }), + ), + ); + + // The frames are gone; respond fires from a fresh chain like the UI. + expect(getCurrentAgentId()).toBeNull(); + await new Promise((resolve) => setImmediate(resolve)); + + await respondClosure!(); + + expect(observedDepth).toBe(2); + }); + it('restores the teammate identity for deferred-approval continuations', async () => { // Regression: a teammate's `send_message`/`task_update` that requires // confirmation resumes from the UI's async chain, outside the @@ -305,6 +349,7 @@ describe('AgentCore.prepareTools', () => { function buildAgentForTools( toolConfig: ToolConfig | undefined, fnDeclarations: FunctionDeclaration[], + maxSubagentDepth = 5, ): { core: AgentCore; debugSpy: ReturnType; @@ -323,6 +368,7 @@ describe('AgentCore.prepareTools', () => { getFunctionDeclarations: getFunctionDeclarationsSpy, getFunctionDeclarationsFiltered: getFunctionDeclarationsFilteredSpy, }), + getMaxSubagentDepth: vi.fn().mockReturnValue(maxSubagentDepth), } as unknown as Config; const core = new AgentCore( @@ -607,4 +653,91 @@ describe('AgentCore.prepareTools', () => { expect(response?.error).not.toContain('not found'); }, ); + + // ─── Nested sub-agents ────────────────────────────────────────── + // The AgentTool is depth-gated: available to a sub-agent only while + // maxSubagentDepth still permits another level. prepareTools() reads the + // sub-agent's own 0-based depth from getCurrentAgentDepth(); a child sits + // at level (depth + 2), which must not exceed the cap. + const nestingDecls = (): FunctionDeclaration[] => [ + { name: 'read_file', description: 'read' } as FunctionDeclaration, + { + name: ToolNames.AGENT, + description: 'spawn subagent', + } as FunctionDeclaration, + ]; + + it('nesting: includes AgentTool for a shallow subagent when depth permits', async () => { + const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 5); + // One frame → getCurrentAgentDepth() === 0 (a top-level sub-agent). + const tools = await runWithAgentContext('lvl1', () => core.prepareTools()); + expect(tools.map((t) => t.name)).toContain(ToolNames.AGENT); + }); + + it('nesting: excludes AgentTool at the leaf depth', async () => { + const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 2); + // Two nested frames → depth === 1 → level 2 → the leaf when max === 2. + const tools = await runWithAgentContext('lvl1', () => + runWithAgentContext('lvl2', () => core.prepareTools()), + ); + const names = tools.map((t) => t.name); + expect(names).not.toContain(ToolNames.AGENT); + expect(names).toContain('read_file'); + }); + + it('nesting: maxSubagentDepth=1 reproduces the old no-nesting behavior', async () => { + const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 1); + const tools = await runWithAgentContext('lvl1', () => core.prepareTools()); + expect(tools.map((t) => t.name)).not.toContain(ToolNames.AGENT); + }); + + it('nesting: frameless prepareTools fails closed — AgentTool excluded', async () => { + // prepareTools() only ever serves agents, never the top-level session. + // A missing agent frame means the launch path forgot runWithAgentContext + // (codex review: AgentInteractive.start() before it established its + // frame); such an agent must not be depth-gated as the top-level session. + const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 5); + const tools = await core.prepareTools(); + const names = tools.map((t) => t.name); + expect(names).not.toContain(ToolNames.AGENT); + expect(names).toContain('read_file'); + }); + + it('nesting: teammates never receive the AgentTool regardless of depth', async () => { + const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 5); + const identity: TeammateIdentity = { + agentId: 'scribe@demo', + agentName: 'scribe', + teamName: 'demo', + isTeamLead: false, + }; + const tools = await runWithTeammateIdentity(identity, () => + runWithAgentContext('lvl1', () => core.prepareTools()), + ); + expect(tools.map((t) => t.name)).not.toContain(ToolNames.AGENT); + }); + + it('nesting: explicit tools list includes AgentTool only when nesting is allowed', async () => { + const allowed = buildAgentForTools( + { tools: ['read_file', ToolNames.AGENT] }, + nestingDecls(), + 5, + ); + const allowedTools = await runWithAgentContext('lvl1', () => + allowed.core.prepareTools(), + ); + expect(allowedTools.map((t) => t.name)).toContain(ToolNames.AGENT); + + const denied = buildAgentForTools( + { tools: ['read_file', ToolNames.AGENT] }, + nestingDecls(), + 1, + ); + const deniedTools = await runWithAgentContext('lvl1', () => + denied.core.prepareTools(), + ); + const deniedNames = deniedTools.map((t) => t.name); + expect(deniedNames).not.toContain(ToolNames.AGENT); + expect(deniedNames).toContain('read_file'); + }); }); diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 1326e8c8e97..ad9445f5112 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -22,12 +22,16 @@ import { reportError } from '../../utils/errorReporting.js'; import { subagentNameContext } from '../../utils/subagentNameContext.js'; import type { Config } from '../../config/config.js'; import { + canSpawnNestedAgent, + getCurrentAgentDepth, getCurrentAgentId, getRuntimeContentGenerator, + isTopLevelSession, runWithAgentContext, runWithRuntimeContentGenerator, type RuntimeContentGeneratorView, } from './agent-context.js'; +import { isInForkExecution } from '../../tools/agent/fork-subagent.js'; import { createDuplicateProviderToolCallResponse, findRepeatedDuplicateProviderToolCall, @@ -493,6 +497,37 @@ export class AgentCore { const toolsList: FunctionDeclaration[] = []; const excludedFromSubagents = getExcludedToolsForCurrentContext(); + + // Nested sub-agents: the AgentTool is normally excluded to prevent + // recursive spawning, but when maxSubagentDepth permits another level we + // let it back in. prepareTools() runs inside this sub-agent's own + // AsyncLocalStorage frame (see AgentHeadless.run / AgentInteractive), so + // canSpawnNestedAgent() reads this agent's own depth. Teammates do not + // nest in v1, and forks must never spawn sub-agents (the fork contract is + // context-sharing, not isolation). See + // knowledge/qwen-code/design/nested-subagents.md. + // + // !isTopLevelSession() fails closed: prepareTools() only ever serves + // agents — never the top-level user session — so a missing agent frame + // means the launch path forgot runWithAgentContext. Without this check + // such an agent would be depth-gated as the top-level session and receive + // the AgentTool even at maxSubagentDepth=1 (codex review: frame-less + // AgentInteractive.start(), since fixed to establish its frame). + const nestingAllowed = + !isTopLevelSession() && + !isTeammate() && + !isInForkExecution() && + canSpawnNestedAgent(this.runtimeContext.getMaxSubagentDepth()); + + // Effective exclusion test. AgentTool is depth-gated (allowed only when + // this sub-agent is shallow enough to spawn another level); every other + // control-plane tool follows the static exclusion set unchanged. + const isExcluded = (name: string | undefined): boolean => { + if (!name) return false; + if (name === ToolNames.AGENT) return !nestingAllowed; + return excludedFromSubagents.has(name); + }; + if (this.toolConfig) { const asStrings = this.toolConfig.tools.filter( (t): t is string => typeof t === 'string', @@ -513,7 +548,7 @@ export class AgentCore { toolsList.push( ...toolRegistry .getFunctionDeclarations({ includeDeferred: true }) - .filter((t) => !(t.name && excludedFromSubagents.has(t.name))), + .filter((t) => !isExcluded(t.name)), ); } else { // Explicit tool list: apply the full subagent exclusion set (not just @@ -521,7 +556,7 @@ export class AgentCore { // (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)) { + if (isExcluded(name)) { this.runtimeContext .getDebugLogger() ?.debug( @@ -535,10 +570,14 @@ export class AgentCore { ...toolRegistry.getFunctionDeclarationsFiltered(allowedNames), ); } - // Also filter inline FunctionDeclaration[] passed directly in toolConfig. + // Also filter inline FunctionDeclaration[] passed directly in toolConfig + // through the same exclusion test as the registry branches, so the + // depth-gated AgentTool and every other control-plane tool are handled + // uniformly (the inline form must not become a leak path for + // workflow/cron/team tools into a subagent). toolsList.push( ...onlyInlineDecls.filter((d) => { - if (d.name && excludedFromSubagents.has(d.name)) { + if (isExcluded(d.name)) { this.runtimeContext .getDebugLogger() ?.debug( @@ -555,7 +594,7 @@ export class AgentCore { toolsList.push( ...toolRegistry .getFunctionDeclarations({ includeDeferred: true }) - .filter((t) => !(t.name && excludedFromSubagents.has(t.name))), + .filter((t) => !isExcluded(t.name)), ); } @@ -642,7 +681,10 @@ export class AgentCore { * `inheritedAgentId` does the same for logical agent ownership. It is * needed by deferred approval because the user's approval response runs * from the parent UI chain, after the subagent's AsyncLocalStorage frame - * has unwound. + * has unwound. `inheritedAgentDepth` accompanies it: without the original + * nesting depth the restored frame recomputes to depth 0, letting a + * deferred-approved `agent` tool call from a leaf-depth sub-agent bypass + * maxSubagentDepth. * * `inheritedTeammateIdentity` restores the in-process teammate identity * frame (`teammateIdentityStore`). Deferred approval needs it for the @@ -661,12 +703,21 @@ export class AgentCore { inheritedView?: RuntimeContentGeneratorView, inheritedAgentId?: string, inheritedTeammateIdentity?: TeammateIdentity, + inheritedAgentDepth?: number, ): Promise { const runInner = () => subagentNameContext.run(this.name, () => { const runWithView = () => this.withRuntimeView(fn, inheritedView); + // inheritedAgentDepth restores the agent's original nesting depth. + // Without it the frame recomputes from the UI's frame-less async + // chain to depth 0, and an approved `agent` tool call from a + // leaf-depth sub-agent would bypass maxSubagentDepth. return inheritedAgentId - ? runWithAgentContext(inheritedAgentId, runWithView) + ? runWithAgentContext( + inheritedAgentId, + runWithView, + inheritedAgentDepth, + ) : runWithView(); }); return inheritedTeammateIdentity @@ -1449,6 +1500,10 @@ export class AgentCore { // restore it. See `runInAgentFrames` for the wiring. const inheritedView = getRuntimeContentGenerator(); const inheritedAgentId = getCurrentAgentId(); + // Depth pairs with the id: the continuation must re-enter the + // frame at this agent's original depth, or the depth guard on a + // deferred-approved `agent` tool call would see depth 0. + const inheritedAgentDepth = getCurrentAgentDepth(); // Capture the teammate identity frame too, while the loop // frame is still live, so the deferred-approval continuation // can restore it. See `runInAgentFrames` for why this matters @@ -1484,6 +1539,7 @@ export class AgentCore { inheritedView, inheritedAgentId ?? undefined, inheritedTeammateIdentity, + inheritedAgentDepth, ); }, timestamp: Date.now(), diff --git a/packages/core/src/agents/runtime/agent-interactive.test.ts b/packages/core/src/agents/runtime/agent-interactive.test.ts index d8513c37337..4c63c0d6579 100644 --- a/packages/core/src/agents/runtime/agent-interactive.test.ts +++ b/packages/core/src/agents/runtime/agent-interactive.test.ts @@ -17,6 +17,7 @@ import type { import { ContextState } from './agent-headless.js'; import type { AgentInteractiveConfig } from './agent-types.js'; import { AgentStatus } from './agent-types.js'; +import { getCurrentAgentDepth, getCurrentAgentId } from './agent-context.js'; function createMockChat() { return { @@ -209,6 +210,47 @@ describe('AgentInteractive', () => { expect(agent.getStatus()).toBe('completed'); }); + it('runs start() and the message loop inside the agent identity frame', async () => { + // Regression (codex review): start() called prepareTools() outside any + // runWithAgentContext frame, so in-process interactive agents (Arena, + // in-process teammates) were depth-gated as the top-level session — + // receiving the `agent` tool even at maxSubagentDepth=1 — and their + // later `agent` calls were counted as top-level spawns. + const { core } = createMockCore(); + let prepId: string | null = null; + let prepDepth = -1; + let loopId: string | null = null; + let loopDepth = -1; + (core.prepareTools as ReturnType).mockImplementation(() => { + prepId = getCurrentAgentId(); + prepDepth = getCurrentAgentDepth(); + return []; + }); + (core.runReasoningLoop as ReturnType).mockImplementation( + async () => { + loopId = getCurrentAgentId(); + loopDepth = getCurrentAgentDepth(); + return { text: 'Done', terminateMode: null, turnsUsed: 1 }; + }, + ); + + const agent = new AgentInteractive( + createConfig({ initialTask: 'go' }), + core, + ); + await agent.start(context); + await vi.waitFor(() => { + expect(agent.getStatus()).toBe('idle'); + }); + + // Spawned from the top-level session → the agent's own frame is depth 0 + // (a level-1 agent), for schema preparation and reasoning rounds alike. + expect(prepId).toBe('agent-1'); + expect(prepDepth).toBe(0); + expect(loopId).toBe('agent-1'); + expect(loopDepth).toBe(0); + }); + it('should process initialTask immediately on start', async () => { const { core } = createMockCore(); const config = createConfig({ initialTask: 'Do something' }); diff --git a/packages/core/src/agents/runtime/agent-interactive.ts b/packages/core/src/agents/runtime/agent-interactive.ts index 816d3bc01ff..0c2c524d835 100644 --- a/packages/core/src/agents/runtime/agent-interactive.ts +++ b/packages/core/src/agents/runtime/agent-interactive.ts @@ -15,6 +15,7 @@ import { createAbortController, createChildAbortController, } from '../../utils/abortController.js'; +import { childLaunchDepth, runWithAgentContext } from './agent-context.js'; import { createDebugLogger } from '../../utils/debugLogger.js'; import { type AgentEventEmitter, AgentEventType } from './agent-events.js'; import type { @@ -57,6 +58,18 @@ export class AgentInteractive { private readonly core: AgentCore; private readonly queue = new AsyncMessageQueue(); + /** + * This agent's nesting depth, captured from the spawner's ambient frame at + * construction (0 when spawned from the top-level session). start() and + * runLoop() re-enter the agent identity frame pinned at this depth, so + * prepareTools()' depth gating and the AgentTool's runtime guards see an + * in-process interactive agent (Arena, in-process teammate) as an agent — + * not as the top-level session. Pinning (rather than auto-increment) + * matters because runLoop() restarts itself from its own finally block and + * enqueueMessage() may be called from arbitrary chains. + */ + private readonly agentDepth: number; + private status: AgentStatus = AgentStatus.INITIALIZING; private error: string | undefined; private lastRoundError: string | undefined; @@ -78,6 +91,7 @@ export class AgentInteractive { constructor(config: AgentInteractiveConfig, core: AgentCore) { this.config = config; this.core = core; + this.agentDepth = childLaunchDepth(); this.setupEventListeners(); } @@ -85,9 +99,19 @@ export class AgentInteractive { /** * Start the agent. Initializes the chat session, then kicks off - * processing if an initialTask is configured. + * processing if an initialTask is configured. Runs inside this agent's + * identity frame so prepareTools() depth-gates the AgentTool correctly + * (see agentDepth). */ - async start(context: ContextState): Promise { + start(context: ContextState): Promise { + return runWithAgentContext( + this.config.agentId, + () => this.startInner(context), + this.agentDepth, + ); + } + + private async startInner(context: ContextState): Promise { this.setStatus(AgentStatus.INITIALIZING); this.chat = await this.core.createChat(context, { @@ -118,9 +142,20 @@ export class AgentInteractive { /** * Run loop: process all pending messages, then settle status. - * Exits when the queue is empty or the agent is aborted. + * Exits when the queue is empty or the agent is aborted. Runs inside this + * agent's identity frame (pinned at agentDepth) so tool bodies — including + * a nested `agent` spawn and its depth guard — attribute to this agent + * rather than the top-level session. */ - private async runLoop(): Promise { + private runLoop(): Promise { + return runWithAgentContext( + this.config.agentId, + () => this.runLoopInner(), + this.agentDepth, + ); + } + + private async runLoopInner(): Promise { this.processing = true; try { let message = this.queue.dequeue(); diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 17874296cb6..3636d42170f 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -500,6 +500,45 @@ describe('Server Config (config.ts)', () => { ); }); + describe('getMaxSubagentDepth', () => { + it('defaults to 5 when unset', () => { + expect(new Config(baseParams).getMaxSubagentDepth()).toBe(5); + }); + + it('respects an explicit value', () => { + expect( + new Config({ + ...baseParams, + maxSubagentDepth: 3, + }).getMaxSubagentDepth(), + ).toBe(3); + }); + + it('clamps values below 1 up to 1 (never disables sub-agents)', () => { + expect( + new Config({ + ...baseParams, + maxSubagentDepth: 0, + }).getMaxSubagentDepth(), + ).toBe(1); + expect( + new Config({ + ...baseParams, + maxSubagentDepth: -4, + }).getMaxSubagentDepth(), + ).toBe(1); + }); + + it('floors fractional values', () => { + expect( + new Config({ + ...baseParams, + maxSubagentDepth: 3.9, + }).getMaxSubagentDepth(), + ).toBe(3); + }); + }); + describe('getTeamMemoryEnabled', () => { const prevEnv = process.env['QWEN_CODE_MEMORY_TEAM']; afterEach(() => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 630802d43e0..1875342fdef 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -925,6 +925,16 @@ export interface ConfigParameters { model?: string; outputLanguageFilePath?: string; maxSessionTurns?: number; + /** + * Maximum number of nested sub-agent levels (1-based). `1` reproduces the + * pre-nesting behavior — level-1 sub-agents exist but cannot themselves + * spawn sub-agents. The default `5` lets a sub-agent spawn sub-agents up to + * five levels deep. Values `< 1` are clamped to `1`. This governs *nesting* + * only; it never disables sub-agents. Teammates, forks, and the workflow + * tool are excluded from nesting in v1. See + * knowledge/qwen-code/design/nested-subagents.md. + */ + maxSubagentDepth?: number; /** * Wall-clock budget for an unattended run, in seconds. `-1` (default) * means no limit. Enforced by the CLI's non-interactive run loop @@ -1472,6 +1482,7 @@ export class Config { private ideMode: boolean; private readonly maxSessionTurns: number; + private readonly maxSubagentDepth: number; private readonly maxWallTimeSeconds: number; private readonly maxToolCalls: number; private readonly clearContextOnIdle: ClearContextOnIdleSettings; @@ -1698,6 +1709,12 @@ export class Config { this.fileDiscoveryService = params.fileDiscoveryService ?? null; this.bugCommand = params.bugCommand; this.maxSessionTurns = params.maxSessionTurns ?? -1; + // Default 5 (nesting on). Explicit values below 1 clamp to 1 so the knob + // never disables sub-agents outright — it only bounds nesting. + this.maxSubagentDepth = + params.maxSubagentDepth == null + ? 5 + : Math.max(1, Math.floor(params.maxSubagentDepth)); this.maxWallTimeSeconds = params.maxWallTimeSeconds ?? -1; this.maxToolCalls = params.maxToolCalls ?? -1; const clearContextOnIdle = params.clearContextOnIdle; @@ -3410,6 +3427,10 @@ export class Config { return this.maxSessionTurns; } + getMaxSubagentDepth(): number { + return this.maxSubagentDepth; + } + getMaxWallTimeSeconds(): number { return this.maxWallTimeSeconds; } diff --git a/packages/core/src/tools/agent/agent-override.test.ts b/packages/core/src/tools/agent/agent-override.test.ts index 794ce31db7f..21e1f054fd5 100644 --- a/packages/core/src/tools/agent/agent-override.test.ts +++ b/packages/core/src/tools/agent/agent-override.test.ts @@ -473,6 +473,7 @@ describe('createApprovalModeOverride bound-tool isolation', () => { model: 'agent-model', maxSessionTurns: 7, maxToolCalls: 11, + maxSubagentDepth: 2, }, }, ); @@ -483,6 +484,9 @@ describe('createApprovalModeOverride bound-tool isolation', () => { expect(child.getModel()).toBe('agent-model'); expect(child.getMaxSessionTurns()).toBe(7); expect(child.getMaxToolCalls()).toBe(11); + // Launch-time nesting cap survives resume even when the resuming + // session's own cap differs (codex review). + expect(child.getMaxSubagentDepth()).toBe(2); await child.getToolRegistry().warmAll(); expect(child.getToolRegistry().getAllToolNames()).not.toContain( diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index d486c80294f..b257408ad88 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -18,7 +18,11 @@ import { type Config, ApprovalMode } from '../../config/config.js'; import { SubagentManager } from '../../subagents/subagent-manager.js'; import type { SubagentConfig } from '../../subagents/types.js'; import { BUBBLE_APPROVAL_MODE } from '../../subagents/types.js'; -import { FORK_AGENT, FORK_DEFAULT_MAX_TURNS } from './fork-subagent.js'; +import { + FORK_AGENT, + FORK_DEFAULT_MAX_TURNS, + runInForkContext, +} from './fork-subagent.js'; import { AgentTerminateMode } from '../../agents/runtime/agent-types.js'; import { AgentHeadless, @@ -181,6 +185,7 @@ describe('AgentTool', () => { getSandbox: vi.fn().mockReturnValue(undefined), getScreenReader: vi.fn().mockReturnValue(false), getMaxSessionTurns: vi.fn().mockReturnValue(-1), + getMaxSubagentDepth: vi.fn().mockReturnValue(5), getMaxToolCalls: vi.fn().mockReturnValue(-1), isTrustedFolder: vi.fn().mockReturnValue(true), isInteractive: vi.fn().mockReturnValue(false), @@ -735,6 +740,103 @@ describe('AgentTool', () => { }); }); + describe('nesting depth guard', () => { + it('rejects a spawn that would exceed maxSubagentDepth', async () => { + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(1); + const invocation = agentTool.build({ + description: 'Spawn deeper', + prompt: 'Do work', + subagent_type: 'file-search', + }); + // One agent frame → invoker is a level-1 sub-agent; its child would be + // level 2, which exceeds max=1 → rejected before any subagent load. + const result = await runWithAgentContext('sub-1', () => + invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain('nesting depth limit reached'); + expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); + }); + + it('allows a spawn from the top-level session at maxSubagentDepth=1', async () => { + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(1); + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue(null); + const invocation = agentTool.build({ + description: 'Spawn helper', + prompt: 'Do work', + subagent_type: 'file-search', + }); + // No agent frame → invoker level 0 → child level 1 ≤ 1 → allowed; the + // guard lets execution proceed to subagent resolution. + await invocation.execute(new AbortController().signal); + + expect(mockSubagentManager.loadSubagent).toHaveBeenCalledWith( + 'file-search', + ); + }); + + it('does not route a nested sub-agent to a teammate even with a name + active team', async () => { + // Regression: nested sub-agents now carry the AgentTool. A sub-agent + // passing `name` must NOT reach executeTeammate (which would bypass the + // depth guard and the v1 "teammates do not nest" rule). With max depth 1 + // and one agent frame, the spawn falls through team routing to the depth + // guard and is rejected — proving executeTeammate was not taken. + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(1); + vi.mocked(config.getTeamManager).mockReturnValue({} as never); + const invocation = agentTool.build({ + description: 'Spawn teammate from within a sub-agent', + prompt: 'Do work', + subagent_type: 'file-search', + name: 'helper', + }); + const result = await runWithAgentContext('sub-1', () => + invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain('nesting depth limit reached'); + expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); + }); + + it('blocks a fork child from spawning any sub-agent', async () => { + // Forks must never spawn sub-agents. The runtime guard blocks ALL agent + // calls from within a fork (not just fork-in-fork), catching the + // wildcard/fallback tool path that could otherwise re-add `agent`. + const invocation = agentTool.build({ + description: 'Spawn from within a fork', + prompt: 'Do work', + subagent_type: 'file-search', + }); + const result = await runInForkContext(() => + invocation.execute(new AbortController().signal), + ); + + expect(result.llmContent).toContain( + 'Cannot spawn sub-agents from within a fork', + ); + expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); + }); + + it('allows nesting while depth remains under the cap', async () => { + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(5); + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue(null); + const invocation = agentTool.build({ + description: 'Spawn deeper', + prompt: 'Do work', + subagent_type: 'file-search', + }); + // Two frames → invoker is level 2; child would be level 3 ≤ 5 → allowed. + await runWithAgentContext('sub-1', () => + runWithAgentContext('sub-2', () => + invocation.execute(new AbortController().signal), + ), + ); + + expect(mockSubagentManager.loadSubagent).toHaveBeenCalledWith( + 'file-search', + ); + }); + }); + describe('refreshSubagents', () => { it('should refresh when change listener fires', async () => { const newSubagents: SubagentConfig[] = [ @@ -3295,6 +3397,7 @@ describe('AgentTool', () => { model: 'parent-model', maxSessionTurns: -1, maxToolCalls: -1, + maxSubagentDepth: 5, }), }), ); diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index e0d8b3d9674..9f63a5c3254 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -53,8 +53,10 @@ import { import { FileDiscoveryService } from '../../services/fileDiscoveryService.js'; import { WorkspaceContext } from '../../utils/workspaceContext.js'; import { - getCurrentAgentDepth, + canSpawnNestedAgent, + childLaunchDepth, getCurrentAgentId, + isTopLevelSession, runWithAgentContext, } from '../../agents/runtime/agent-context.js'; import { trace, context as otelContext } from '@opentelemetry/api'; @@ -437,6 +439,9 @@ function applyPersistedCliFlagOverrides( if (flags.maxToolCalls !== undefined) { ov.getMaxToolCalls = () => flags.maxToolCalls; } + if (flags.maxSubagentDepth !== undefined) { + ov.getMaxSubagentDepth = () => flags.maxSubagentDepth; + } } function capturePersistedCliFlags( @@ -452,6 +457,7 @@ function capturePersistedCliFlags( model: config.getModel(), maxSessionTurns: config.getMaxSessionTurns(), maxToolCalls: config.getMaxToolCalls(), + maxSubagentDepth: config.getMaxSubagentDepth(), }; } @@ -1515,18 +1521,13 @@ class AgentToolInvocation extends BaseToolInvocation { ? undefined : trace.getSpan(otelContext.active())?.spanContext(); // Capture parent identity BEFORE we enter the child's runWithAgentContext - // frame inside `body`. The parent's depth is `getCurrentAgentDepth()` (0 - // outside any frame, N inside frame at depth N); the subagent itself - // lives one level deeper, hence the +1 — but only when a parent frame - // exists. Without a parent the subagent is top-level (depth 0). The - // `getCurrentAgentId() !== null` test discriminates "no frame" from - // "frame at depth 0", which `getCurrentAgentDepth()` alone cannot. - // Review wenshao @ #4410. + // frame inside `body` — childLaunchDepth() reads the invoker's frame, not + // the child's. Review wenshao @ #4410. const parentAgentId = getCurrentAgentId(); const span = startSubagentSpan({ ...spec, parentAgentId: parentAgentId ?? undefined, - depth: parentAgentId !== null ? getCurrentAgentDepth() + 1 : 0, + depth: childLaunchDepth(), invokingRequestId: this.callId, sessionId: this.config.getSessionId(), invokerSpanContext, @@ -1750,6 +1751,29 @@ class AgentToolInvocation extends BaseToolInvocation { } } + /** + * Failure ToolResult for a spawn blocked by an execute()-entry guard + * (nesting depth limit, fork containment). Keeps the two guards' result + * shape in lockstep. + */ + private buildSpawnBlockedResult( + llmContent: string, + terminateReason: string, + ): ToolResult { + return { + llmContent, + returnDisplay: { + type: 'task_execution' as const, + subagentName: + this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE, + taskDescription: this.params.description, + taskPrompt: this.params.prompt, + status: 'failed' as const, + terminateReason, + }, + }; + } + async execute( signal?: AbortSignal, updateOutput?: (output: ToolResultDisplay) => void, @@ -1783,7 +1807,13 @@ class AgentToolInvocation extends BaseToolInvocation { // A name only means "spawn a teammate" while a team is active. Older // prompts may still pass it without a team; treat that as a normal // one-shot agent instead of failing the whole task. - if (this.params.name && !isTeammate()) { + // + // isTopLevelSession() restricts teammate spawning to the top-level + // session. Nested sub-agents now carry the AgentTool, so without this a + // sub-agent could pass `name` and reach executeTeammate, bypassing both + // the v1 "teammates do not nest" rule and the depth guard below. A nested + // sub-agent's `name` falls through to the normal path. + if (this.params.name && !isTeammate() && isTopLevelSession()) { if (!this.config.getTeamManager()) { debugLogger.debug( `[AgentTool] Ignoring teammate name "${this.params.name}" because no team is active.`, @@ -1793,6 +1823,39 @@ class AgentToolInvocation extends BaseToolInvocation { } } + // ─── Nesting depth guard ────────────────────────────────────── + // Authoritative runtime backstop for sub-agent nesting: reject the spawn + // when it would exceed maxSubagentDepth. AgentCore.prepareTools() also + // hides the AgentTool from leaf-depth sub-agents (same canSpawnNestedAgent + // check), so a well-behaved model never reaches here. Teammate spawns + // returned above and are not gated by depth. See + // knowledge/qwen-code/design/nested-subagents.md. + const maxSubagentDepth = this.config.getMaxSubagentDepth(); + if (!canSpawnNestedAgent(maxSubagentDepth)) { + return this.buildSpawnBlockedResult( + `Error: sub-agent nesting depth limit reached ` + + `(max ${maxSubagentDepth} level${maxSubagentDepth === 1 ? '' : 's'}). ` + + `Complete this task directly with your own tools instead of ` + + `spawning another sub-agent.`, + `Nesting depth limit reached (max ${maxSubagentDepth})`, + ); + } + + // ─── Fork spawn guard ───────────────────────────────────────── + // Forks must never spawn sub-agents — the fork contract is + // context-sharing, not isolation. AgentCore.prepareTools() already strips + // `agent` from fork contexts, but wildcard/fallback tool lists make this + // runtime backstop necessary. Blocks ALL agent calls from a fork child + // (not just fork-in-fork). This runs before the fork branch below, so the + // recursive-fork case is subsumed here. See + // knowledge/qwen-code/design/nested-subagents.md. + if (isInForkExecution()) { + return this.buildSpawnBlockedResult( + 'Error: Cannot spawn sub-agents from within a fork. Please execute tasks directly.', + 'Sub-agent spawning is not allowed inside a fork', + ); + } + // ── Isolation state hoisted to the outermost scope ──────────── // The outer try/catch in this method is the last line of defence // against pre-execution failures (e.g. createApprovalModeOverride @@ -1941,7 +2004,15 @@ class AgentToolInvocation extends BaseToolInvocation { const requestedType = this.params.subagent_type; const isForkRequested = requestedType?.toLowerCase() === FORK_SUBAGENT_TYPE; - const isFork = isForkRequested && isForkSubagentEnabled(this.config); + const isFork = + isForkRequested && + isForkSubagentEnabled(this.config) && + // v1: fork is a top-level-only capability. A sub-agent — which may now + // carry the AgentTool via nesting — that requests a fork falls back to + // the awaitable general-purpose sub-agent (via effectiveSubagentType + // below) instead of opening a nested fork. Fork nesting is deferred + // past v1. See knowledge/qwen-code/design/nested-subagents.md. + isTopLevelSession(); const effectiveSubagentType = isFork ? undefined : isForkRequested @@ -1953,25 +2024,6 @@ class AgentToolInvocation extends BaseToolInvocation { if (isFork) { subagentConfig = FORK_AGENT; - - // Recursive-fork guard. A fork child's reasoning loop runs inside - // an AsyncLocalStorage frame set by `runInForkContext`; when its - // model calls the `agent` tool, this check fires before any history - // or config is touched. - if (isInForkExecution()) { - return { - llmContent: - 'Error: Cannot create a fork from within an existing fork child. Please execute tasks directly.', - returnDisplay: { - type: 'task_execution' as const, - subagentName: FORK_AGENT.name, - taskDescription: this.params.description, - taskPrompt: this.params.prompt, - status: 'failed' as const, - terminateReason: 'Recursive forking is not allowed', - }, - }; - } } else { const loadedConfig = await this.subagentManager.loadSubagent( effectiveSubagentType!, @@ -2518,6 +2570,9 @@ class AgentToolInvocation extends BaseToolInvocation { subagentName: subagentConfig.name, agentColor: subagentConfig.color, resumeCount: 0, + // Persisted so resume restores the original nesting level; see + // childLaunchDepth() for the rationale. + depth: childLaunchDepth(), }); // Subscribe to the subagent's tool-call event stream so the @@ -3080,6 +3135,9 @@ class AgentToolInvocation extends BaseToolInvocation { subagentName: subagentConfig.name, agentColor: subagentConfig.color, resumeCount: 0, + // Persisted so resume restores the original nesting level; see + // childLaunchDepth() for the rationale. + depth: childLaunchDepth(), }); const stopHookWarning = await runFramed(); diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 35f0039f900..d5e52506a2b 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -592,6 +592,11 @@ "type": "number", "default": -1 }, + "maxSubagentDepth": { + "description": "Maximum number of nested sub-agent levels. 1 disables nesting (sub-agents cannot spawn sub-agents); the default 5 allows a sub-agent to spawn sub-agents up to five levels deep. Values below 1 are clamped to 1. Teammates, forks, and the workflow tool are excluded from nesting.", + "type": "number", + "default": 5 + }, "chatCompression": { "description": "Chat compression settings.", "type": "object", From 2c500771c7a1fdfb9b3044fbc4fe8ae9a3bf9b9b Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 2 Jul 2026 13:07:18 +0000 Subject: [PATCH 2/9] fix(core): address review findings on nested sub-agent spawning - Deny the agent tool to workflow-spawned subagents: depth gating would otherwise re-admit it, letting a workflow leaf spawn outside the orchestrator's concurrency cap, agent accounting, and token budget. - Reject non-finite maxSubagentDepth values (JSON 1e309 parses to Infinity and would unbound the recursion cap; NaN would silently block all nesting) and cap the knob at 100 to catch typos. - Add a --max-subagent-depth CLI flag mirroring sibling budget flags, with loud validation for flag typos, and document the setting. - Log guard rejections (depth, fork containment) and silent fork-to-subagent downgrades through the agent debug logger. - Refresh stale comments (depthOverride resume pinning, depth-gated AgentTool exclusion) and drop references to a design doc that lives outside the repository. - Fill review-noted test gaps: nesting predicate primitives, fork-context prepareTools, persisted-depth restoration on background resume, nested AgentInteractive depth pinning, nested fork fallback, and the blocked-spawn returnDisplay shape. --- docs/users/configuration/settings.md | 1 + packages/cli/src/config/config.ts | 34 +++++++- .../agents/background-agent-resume.test.ts | 77 +++++++++++++++++++ .../src/agents/runtime/agent-context.test.ts | 51 ++++++++++++ .../core/src/agents/runtime/agent-context.ts | 7 +- .../src/agents/runtime/agent-core.test.ts | 13 ++++ .../core/src/agents/runtime/agent-core.ts | 9 ++- .../agents/runtime/agent-interactive.test.ts | 39 +++++++++- .../runtime/workflow-orchestrator.test.ts | 34 ++++---- .../agents/runtime/workflow-orchestrator.ts | 7 ++ packages/core/src/config/config.test.ts | 39 ++++++++-- packages/core/src/config/config.ts | 69 +++++++++++------ packages/core/src/tools/agent/agent.test.ts | 28 +++++++ packages/core/src/tools/agent/agent.ts | 23 ++++-- 14 files changed, 372 insertions(+), 59 deletions(-) diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index e334d19c7ad..47d88f4f3b8 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -160,6 +160,7 @@ Settings are organized into categories. Most settings should be placed within th | `model.maxSessionTurns` | number | Maximum number of user/model/tool turns to keep in a session. -1 means unlimited. | `-1` | | `model.maxWallTimeSeconds` | number | Wall-clock budget for headless / unattended runs, in seconds. `-1` means unlimited. Overridable per-invocation via `--max-wall-time`, which requires a positive duration (`90`, `30s`, `5m`, `1h`, `1.5h`); the minimum is 1 second — sub-second values (`500ms`, `0.5`) are rejected as typos. Omit the flag to fall back to this setting. Aborts with exit code 55 when exceeded. | `-1` | | `model.maxToolCalls` | number | Cumulative tool-call budget for a run (counts every executed tool, success or failure; `structured_output` under `--json-schema` is exempt). `-1` means unlimited; `0` means "no tool calls allowed". Capped at 1,000,000 to catch typos. Overridable via `--max-tool-calls`. Aborts with exit code 55 when exceeded. | `-1` | +| `model.maxSubagentDepth` | number | Maximum sub-agent nesting depth (1-based levels: a top-level sub-agent is level 1). `1` keeps sub-agents available but disables nesting — the pre-nesting behavior. Values clamp to the range 1–100; non-finite values fall back to the default. Teammates, forks, and workflow-spawned agents never nest regardless of this setting. Overridable via `--max-subagent-depth`. | `5` | | `model.generationConfig` | object | Advanced overrides passed to the underlying content generator. Supports request controls such as `timeout`, `maxRetries`, `enableCacheControl`, `splitToolMedia` (default `true`; splits tool-returned media — including images read by the built-in read_file — into a follow-up user message instead of the spec-violating `role: "tool"` message, so strict OpenAI-compatible servers like doubao / new-api / LM Studio can see it; set `false` to restore the legacy embed-in-tool behavior), `toolResultContentFormat` (default `"parts"`; set `"string"` only for legacy OpenAI-compatible runtimes whose tool templates ignore text content parts), `contextWindowSize` (override model's context window size), `modalities` (override auto-detected input modalities), `customHeaders` (custom HTTP headers for API requests), and `extra_body` (additional body parameters for OpenAI-compatible API requests only), along with fine-tuning knobs under `samplingParams` (for example `temperature`, `top_p`, `max_tokens`). Leave unset to rely on provider defaults. | `undefined` | | `model.chatCompression.contextPercentageThreshold` | number | **REMOVED.** Replaced by `context.autoCompactThreshold` (see `#### context` section below). Auto-compaction now uses a three-tier threshold ladder (warn / auto / hard) computed internally from the model's context window via the `computeThresholds()` function. The old setting is silently ignored (no startup warning). See PR #4345 / `docs/design/auto-compaction-threshold-redesign.md` for the redesign rationale. | `N/A` | | `model.chatCompression.maxRecentFilesToRetain` | number | Number of most-recently-touched files whose current content is restored (embedded if small, otherwise referenced by path) into history after auto-compaction. `0` restores none. Env override: `QWEN_COMPACT_MAX_RECENT_FILES`. | `5` | diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index a5d1f1745e7..68c74c4d0f1 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -206,6 +206,7 @@ export interface CliArgs { maxSessionTurns: number | undefined; maxWallTime: string | undefined; maxToolCalls: number | undefined; + maxSubagentDepth: number | undefined; coreTools: string[] | undefined; excludeTools: string[] | undefined; disabledSlashCommands: string[] | undefined; @@ -887,6 +888,11 @@ export async function parseArguments(): Promise { description: 'Maximum cumulative tool calls executed during the run (success or failure; `structured_output` under --json-schema is exempt). Aborts with exit code 55 when exceeded. -1 / unset means no limit; 0 means "no tool calls allowed" (first call aborts). Capped at 1,000,000 to catch typos.', }) + .option('max-subagent-depth', { + type: 'number', + description: + 'Maximum sub-agent nesting depth (1-based levels). 1 keeps sub-agents available but disables nesting; capped at 100. Overrides model.maxSubagentDepth from settings. Defaults to 5.', + }) .option('core-tools', { type: 'array', string: true, @@ -1235,6 +1241,32 @@ function resolveMaxToolCalls(argv: CliArgs, settings: Settings): number { return -1; } +/** + * Resolves the sub-agent nesting cap. Order of precedence: + * `--max-subagent-depth` flag, then `model.maxSubagentDepth` from settings, + * else undefined (Config applies the default of 5). + * + * Yargs accepts `NaN` from non-numeric flag values, and Config's clamp + * would silently fall back to the default — validate up front so a typo + * fails loudly. Settings values stay lenient (Config clamps them) so a bad + * settings.json cannot break startup. + */ +function resolveMaxSubagentDepth( + argv: CliArgs, + settings: Settings, +): number | undefined { + const value = argv.maxSubagentDepth; + if (value !== undefined && value !== null) { + if (!Number.isInteger(value) || value < 1 || value > 100) { + throw new Error( + `--max-subagent-depth must be an integer between 1 and 100; got ${value}.`, + ); + } + return value; + } + return settings.model?.maxSubagentDepth; +} + export function isDebugMode(argv: CliArgs): boolean { if (argv.debug) return true; const debugVal = process.env['DEBUG']; @@ -1997,7 +2029,7 @@ export async function loadCliConfig( maxWallTimeSeconds: resolveMaxWallTimeSeconds(argv, settings), maxToolCalls: resolveMaxToolCalls(argv, settings), // Undefined flows through to Config's default (5) and clamp logic. - maxSubagentDepth: settings.model?.maxSubagentDepth, + maxSubagentDepth: resolveMaxSubagentDepth(argv, settings), experimentalZedIntegration: argv.acp || argv.experimentalAcp || false, cronEnabled: settings.experimental?.cron ?? true, agentTeamEnabled: settings.experimental?.agentTeam ?? false, diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index 7fbdf9e1900..857182da7af 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -19,6 +19,7 @@ import { } from './agent-transcript.js'; import { AgentTerminateMode } from './runtime/agent-types.js'; import { AgentEventEmitter } from './runtime/agent-events.js'; +import { getCurrentAgentDepth } from './runtime/agent-context.js'; import { AgentHeadless } from './runtime/agent-headless.js'; import { FORK_DEFAULT_MAX_TURNS, @@ -1122,6 +1123,82 @@ describe('BackgroundAgentResumeService', () => { expect(overriddenConfig.getMaxToolCalls()).toBe(11); }, 20000); + it('restores the persisted launch depth so a resumed nested agent keeps its level', async () => { + // Resume happens from a top-level frame (depth would recompute to 0); + // the persisted meta.depth must be pinned via the runWithAgentContext + // depthOverride, or a resumed nested agent would regain spawn capacity. + const sessionId = 'session-depth'; + const agentId = 'agent-depth'; + const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); + const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); + + writeAgentMeta(metaPath, { + agentId, + agentType: 'researcher', + description: 'Resume nested agent', + parentSessionId: sessionId, + parentAgentId: 'agent-parent', + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + depth: 2, + }); + fs.writeFileSync( + outputFile, + JSON.stringify({ + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-20T00:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'Resume nested agent' }] }, + }) + '\n', + 'utf8', + ); + + registry.register({ + agentId, + description: 'Resume nested agent', + subagentType: 'researcher', + status: 'paused', + startTime: Date.now(), + abortController: new AbortController(), + prompt: 'Resume nested agent', + outputFile, + metaPath, + isBackgrounded: true, + }); + + let observedDepth = -1; + const createAgentHeadless = vi.fn().mockResolvedValue({ + subagent: { + execute: vi.fn(async () => { + observedDepth = getCurrentAgentDepth(); + }), + setExternalMessageProvider: vi.fn(), + getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), + getExecutionSummary: () => ({ + totalTokens: 0, + outputTokens: 0, + totalDurationMs: 0, + }), + getTerminateMode: () => AgentTerminateMode.GOAL, + getFinalText: () => 'done', + }, + dispose: vi.fn().mockResolvedValue(undefined), + }); + + const { service, subagentManager } = createService(); + subagentManager.createAgentHeadless = createAgentHeadless; + + const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); + + expect(resumed).toBeDefined(); + await vi.waitFor(() => { + expect(observedDepth).toBe(2); + }); + }, 20000); + it('coalesces concurrent resume calls into a single running agent', async () => { const sessionId = 'session-double'; const agentId = 'agent-double'; diff --git a/packages/core/src/agents/runtime/agent-context.test.ts b/packages/core/src/agents/runtime/agent-context.test.ts index dfb454a4b34..3d4a2b5b15e 100644 --- a/packages/core/src/agents/runtime/agent-context.test.ts +++ b/packages/core/src/agents/runtime/agent-context.test.ts @@ -6,9 +6,12 @@ import { describe, expect, it } from 'vitest'; import { + canSpawnNestedAgent, + childLaunchDepth, getCurrentAgentDepth, getCurrentAgentId, getRuntimeContentGenerator, + isTopLevelSession, runWithAgentContext, runWithRuntimeContentGenerator, type RuntimeContentGeneratorView, @@ -217,3 +220,51 @@ describe('agent-context (depth) — #3731 Phase 3', () => { ); }); }); + +describe('agent-context (nesting predicates)', () => { + it('isTopLevelSession: true outside any frame, false inside one', async () => { + expect(isTopLevelSession()).toBe(true); + await runWithAgentContext('sub', async () => { + expect(isTopLevelSession()).toBe(false); + }); + expect(isTopLevelSession()).toBe(true); + }); + + it('childLaunchDepth: 0 from the top level, parent depth + 1 inside frames', async () => { + expect(childLaunchDepth()).toBe(0); + await runWithAgentContext('lvl1', async () => { + expect(childLaunchDepth()).toBe(1); + await runWithAgentContext('lvl2', async () => { + expect(childLaunchDepth()).toBe(2); + }); + }); + }); + + it('canSpawnNestedAgent: child level (1-based) must not exceed maxDepth', async () => { + // Top level: the child would be a level-1 agent — allowed at max 1. + expect(canSpawnNestedAgent(1)).toBe(true); + await runWithAgentContext('lvl1', async () => { + // Inside a level-1 agent: the child would be level 2. + expect(canSpawnNestedAgent(1)).toBe(false); + expect(canSpawnNestedAgent(2)).toBe(true); + await runWithAgentContext('lvl2', async () => { + // Inside a level-2 agent: the child would be level 3. + expect(canSpawnNestedAgent(2)).toBe(false); + expect(canSpawnNestedAgent(3)).toBe(true); + }); + }); + }); + + it('canSpawnNestedAgent respects a depthOverride-pinned frame', async () => { + // A resumed agent pinned at depth 4 must not spawn at max 5 (child would + // be level 6) even though the resume itself runs from the top level. + await runWithAgentContext( + 'resumed', + async () => { + expect(canSpawnNestedAgent(5)).toBe(false); + expect(canSpawnNestedAgent(6)).toBe(true); + }, + 4, + ); + }); +}); diff --git a/packages/core/src/agents/runtime/agent-context.ts b/packages/core/src/agents/runtime/agent-context.ts index fa1189797d4..b5b5ba4b06f 100644 --- a/packages/core/src/agents/runtime/agent-context.ts +++ b/packages/core/src/agents/runtime/agent-context.ts @@ -35,7 +35,9 @@ interface AgentContext { /** * Nesting depth — 0 for a top-level subagent (called from a user's * top-level interaction), +1 per nested `runWithAgentContext` frame. - * Auto-incremented; callers do not pass it. Read via + * Auto-incremented by default; resume paths (background resume, + * AgentInteractive, deferred approvals) pass `depthOverride` to restore + * the original launch depth instead. Read via * {@link getCurrentAgentDepth} for telemetry (#3731 Phase 3). */ readonly depth?: number; @@ -125,8 +127,7 @@ export function childLaunchDepth(): number { * guard) so the two cannot drift apart. * * The would-be child sits at level `childLaunchDepth() + 1` (levels are - * 1-based, depths 0-based), which must not exceed `maxDepth`. See - * knowledge/qwen-code/design/nested-subagents.md. + * 1-based, depths 0-based), which must not exceed `maxDepth`. */ export function canSpawnNestedAgent(maxDepth: number): boolean { return childLaunchDepth() + 1 <= maxDepth; diff --git a/packages/core/src/agents/runtime/agent-core.test.ts b/packages/core/src/agents/runtime/agent-core.test.ts index db64f98fa49..feb68aa74b6 100644 --- a/packages/core/src/agents/runtime/agent-core.test.ts +++ b/packages/core/src/agents/runtime/agent-core.test.ts @@ -16,6 +16,7 @@ import { type RuntimeContentGeneratorView, } from './agent-context.js'; import { subagentNameContext } from '../../utils/subagentNameContext.js'; +import { runInForkContext } from '../../tools/agent/fork-subagent.js'; import { ToolNames } from '../../tools/tool-names.js'; import { getAgentName, @@ -703,6 +704,18 @@ describe('AgentCore.prepareTools', () => { expect(names).toContain('read_file'); }); + it('nesting: fork execution contexts never receive the AgentTool', async () => { + // The fork contract is context-sharing, not isolation — forks must not + // spawn. Depth would otherwise permit nesting here (one frame, max 5). + const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 5); + const tools = await runInForkContext(() => + runWithAgentContext('lvl1', () => core.prepareTools()), + ); + const names = tools.map((t) => t.name); + expect(names).not.toContain(ToolNames.AGENT); + expect(names).toContain('read_file'); + }); + it('nesting: teammates never receive the AgentTool regardless of depth', async () => { const { core } = buildAgentForTools({ tools: ['*'] }, nestingDecls(), 5); const identity: TeammateIdentity = { diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index ad9445f5112..7b0c9de6666 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -110,7 +110,11 @@ import { /** * Tools that must never be available to non-team subagents (including * forked agents spawned via the Agent tool). - * - AgentTool prevents recursive subagent spawning. + * - AgentTool is depth-gated rather than unconditionally excluded: + * `isExcluded()` in `prepareTools()` re-admits it while + * `canSpawnNestedAgent()` permits another nesting level, and consults + * this set only for every other tool. The entry here remains the + * fail-closed floor for consumers of the raw set. * - Cron tools are session-scoped and should only run from the main session. * - TaskStop and SendMessage are parent-side control-plane tools for managing * background subagents; subagents have no agent IDs to manage natively, so @@ -504,8 +508,7 @@ export class AgentCore { // AsyncLocalStorage frame (see AgentHeadless.run / AgentInteractive), so // canSpawnNestedAgent() reads this agent's own depth. Teammates do not // nest in v1, and forks must never spawn sub-agents (the fork contract is - // context-sharing, not isolation). See - // knowledge/qwen-code/design/nested-subagents.md. + // context-sharing, not isolation). // // !isTopLevelSession() fails closed: prepareTools() only ever serves // agents — never the top-level user session — so a missing agent frame diff --git a/packages/core/src/agents/runtime/agent-interactive.test.ts b/packages/core/src/agents/runtime/agent-interactive.test.ts index 4c63c0d6579..2ff6d965bf6 100644 --- a/packages/core/src/agents/runtime/agent-interactive.test.ts +++ b/packages/core/src/agents/runtime/agent-interactive.test.ts @@ -17,7 +17,11 @@ import type { import { ContextState } from './agent-headless.js'; import type { AgentInteractiveConfig } from './agent-types.js'; import { AgentStatus } from './agent-types.js'; -import { getCurrentAgentDepth, getCurrentAgentId } from './agent-context.js'; +import { + getCurrentAgentDepth, + getCurrentAgentId, + runWithAgentContext, +} from './agent-context.js'; function createMockChat() { return { @@ -251,6 +255,39 @@ describe('AgentInteractive', () => { expect(loopDepth).toBe(0); }); + it('pins the construction-time depth when built inside a sub-agent frame', async () => { + // A nested in-process interactive agent captures childLaunchDepth() at + // construction. start() runs OUTSIDE the parent's frame here, so a + // measured depth of 1 proves the pin — without it the agent would be + // framed as depth 0 (top-level spawn) and regain spawn capacity. + const { core } = createMockCore(); + let prepDepth = -1; + let loopDepth = -1; + (core.prepareTools as ReturnType).mockImplementation(() => { + prepDepth = getCurrentAgentDepth(); + return []; + }); + (core.runReasoningLoop as ReturnType).mockImplementation( + async () => { + loopDepth = getCurrentAgentDepth(); + return { text: 'Done', terminateMode: null, turnsUsed: 1 }; + }, + ); + + const agent = await runWithAgentContext( + 'parent-agent', + async () => + new AgentInteractive(createConfig({ initialTask: 'go' }), core), + ); + await agent.start(context); + await vi.waitFor(() => { + expect(agent.getStatus()).toBe('idle'); + }); + + expect(prepDepth).toBe(1); + expect(loopDepth).toBe(1); + }); + it('should process initialTask immediately on start', async () => { const { core } = createMockCore(); const config = createConfig({ initialTask: 'Do something' }); diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index acf5b01eb51..482b0514ad4 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -1179,26 +1179,25 @@ describe('createProductionDispatch', () => { 'monitor', 'enter_plan_mode', 'exit_plan_mode', + 'agent', ]); }); // T10 (PR #4732 R1): the production dispatch must throw when the // 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'], - ['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}\\)\\.`, - ), - ); - }); + it.each([['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 ────────── @@ -1956,14 +1955,15 @@ 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]{16}$/); - // Workflow floor [SendMessage, Monitor, EnterPlanMode, ExitPlanMode] must - // be unioned in. + // Workflow floor [SendMessage, Monitor, EnterPlanMode, ExitPlanMode, + // Agent] must be unioned in. expect(calls[0].config.disallowedTools).toEqual( expect.arrayContaining([ 'send_message', 'monitor', 'enter_plan_mode', 'exit_plan_mode', + 'agent', ]), ); }); @@ -2046,6 +2046,7 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( 'monitor', 'enter_plan_mode', 'exit_plan_mode', + 'agent', ]), ); }); @@ -2446,6 +2447,7 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( 'monitor', 'enter_plan_mode', 'exit_plan_mode', + 'agent', ]), ); }); diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index 49759b0c4c5..c2caeeef58d 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -165,6 +165,13 @@ const WORKFLOW_SUBAGENT_DISALLOWED_TOOLS: string[] = [ ToolNames.SEND_MESSAGE, ToolNames.MONITOR, ...SUBAGENT_PLAN_LIFECYCLE_TOOLS, + // AgentTool: workflow subagents must not spawn sub-agents even where + // maxSubagentDepth would permit nesting — a leaf-spawned agent would run + // outside the orchestrator's concurrency cap, agent counter, and token + // budget, and its result would bypass the script's return-value contract. + // The WorkflowTool itself is already excluded from every subagent; this + // closes the same loop for plain `agent` calls (review #6189). + ToolNames.AGENT, ]; /** diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 3636d42170f..98724bf757f 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -537,6 +537,32 @@ describe('Server Config (config.ts)', () => { }).getMaxSubagentDepth(), ).toBe(3); }); + + it('falls back to the default on non-finite values', () => { + // JSON `1e309` parses to Infinity — must not disable the recursion cap. + expect( + new Config({ + ...baseParams, + maxSubagentDepth: Infinity, + }).getMaxSubagentDepth(), + ).toBe(5); + // NaN comparisons are always false — must not silently block nesting. + expect( + new Config({ + ...baseParams, + maxSubagentDepth: NaN, + }).getMaxSubagentDepth(), + ).toBe(5); + }); + + it('caps absurdly large values at 100', () => { + expect( + new Config({ + ...baseParams, + maxSubagentDepth: 5000, + }).getMaxSubagentDepth(), + ).toBe(100); + }); }); describe('getTeamMemoryEnabled', () => { @@ -3530,7 +3556,8 @@ describe('Server Config (config.ts)', () => { const lastCall = vi.mocked(loadServerHierarchicalMemory).mock.calls.at(-1); const options = lastCall?.at(-1) as - LoadServerHierarchicalMemoryOptions | undefined; + | LoadServerHierarchicalMemoryOptions + | undefined; expect(options?.onInstructionsLoaded).toEqual(expect.any(Function)); await options?.onInstructionsLoaded?.({ @@ -5874,8 +5901,9 @@ describe('Model Switching and Config Updates', () => { } it('resolves getters to the runtime view inside the frame, instance fields outside', async () => { - const { runWithRuntimeContentGenerator } = - await import('../agents/runtime/agent-context.js'); + const { runWithRuntimeContentGenerator } = await import( + '../agents/runtime/agent-context.js' + ); const config = new Config(baseParams); const parentGenerator = { generateContentStream: vi.fn(), @@ -5922,8 +5950,9 @@ describe('Model Switching and Config Updates', () => { }); it('falls back to the parent model id when the runtime view config has no model', async () => { - const { runWithRuntimeContentGenerator } = - await import('../agents/runtime/agent-context.js'); + const { runWithRuntimeContentGenerator } = await import( + '../agents/runtime/agent-context.js' + ); const config = new Config(baseParams); setInstanceFields( config, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 1875342fdef..273720ad862 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -637,7 +637,10 @@ export type McpServerScope = 'project' | 'workspace' | 'system'; * - `pending_approval`: a gated server awaiting approval (#4615). */ export type McpServerUnavailableReason = - 'removed' | 'not_allowed' | 'excluded' | 'pending_approval'; + | 'removed' + | 'not_allowed' + | 'excluded' + | 'pending_approval'; /** * Scopes whose servers are checked-in / shareable and therefore untrusted: they @@ -930,9 +933,8 @@ export interface ConfigParameters { * pre-nesting behavior — level-1 sub-agents exist but cannot themselves * spawn sub-agents. The default `5` lets a sub-agent spawn sub-agents up to * five levels deep. Values `< 1` are clamped to `1`. This governs *nesting* - * only; it never disables sub-agents. Teammates, forks, and the workflow - * tool are excluded from nesting in v1. See - * knowledge/qwen-code/design/nested-subagents.md. + * only; it never disables sub-agents. Teammates, forks, and + * workflow-spawned agents are excluded from nesting in v1. */ maxSubagentDepth?: number; /** @@ -1346,7 +1348,8 @@ export class Config { private skillManager: SkillManager | null = null; private permissionManager: PermissionManager | null = null; private modelInvocableCommandsProvider: - (() => ReadonlyArray<{ name: string; description: string }>) | null = null; + | (() => ReadonlyArray<{ name: string; description: string }>) + | null = null; private modelInvocableCommandsExecutor: | (( name: string, @@ -1381,7 +1384,8 @@ export class Config { private readonly excludeTools: string[] | undefined; private readonly disabledSlashCommands: readonly string[]; private readonly disabledSkillNamesProvider: - (() => ReadonlySet) | null; + | (() => ReadonlySet) + | null; // `disabledTools` is set at construction // time but can be re-synced by the daemon mutation surface // (`setWorkspaceToolEnabled` propagates through ACP) so a subsequent @@ -1409,7 +1413,8 @@ export class Config { */ private readonly recentlyRemovedMcpServers = new Set(); private readonly topTierMcpServers: - Record | undefined; + | Record + | undefined; private readonly runtimeMcpServers = new Map(); private readonly lspEnabled: boolean; private lspClient?: LspClient; @@ -1521,7 +1526,8 @@ export class Config { private shellExecutionConfig: ShellExecutionConfig; private arenaManager: ArenaManager | null = null; private arenaManagerChangeCallback: - ((manager: ArenaManager | null) => void) | null = null; + | ((manager: ArenaManager | null) => void) + | null = null; private readonly arenaAgentClient: ArenaAgentClient | null; private teamManager: TeamManager | null = null; private teamManagerChangeCallbacks = new Set< @@ -1710,11 +1716,15 @@ export class Config { this.bugCommand = params.bugCommand; this.maxSessionTurns = params.maxSessionTurns ?? -1; // Default 5 (nesting on). Explicit values below 1 clamp to 1 so the knob - // never disables sub-agents outright — it only bounds nesting. + // never disables sub-agents outright — it only bounds nesting. This + // setting is the recursion guardrail, so non-finite values (NaN would + // silently block all nesting, Infinity would unbound it) fall back to the + // default, and 100 caps typos the way maxToolCalls' ceiling does. this.maxSubagentDepth = - params.maxSubagentDepth == null + params.maxSubagentDepth == null || + !Number.isFinite(params.maxSubagentDepth) ? 5 - : Math.max(1, Math.floor(params.maxSubagentDepth)); + : Math.min(100, Math.max(1, Math.floor(params.maxSubagentDepth))); this.maxWallTimeSeconds = params.maxWallTimeSeconds ?? -1; this.maxToolCalls = params.maxToolCalls ?? -1; const clearContextOnIdle = params.clearContextOnIdle; @@ -2048,7 +2058,8 @@ export class Config { (input['permission_mode'] as PermissionMode) || PermissionMode.Default, (input['permission_suggestions'] as - PermissionSuggestion[] | undefined) || undefined, + | PermissionSuggestion[] + | undefined) || undefined, signal, ); break; @@ -3160,7 +3171,8 @@ export class Config { * the bridge at an unreachable, or non-image-capable, model. */ private resolveVisionModelSelection(): - VisionBridgeModelSelection | undefined { + | VisionBridgeModelSelection + | undefined { if (!this.visionModel) return undefined; const visionModelForLog = formatVisionModelSettingForLog(this.visionModel); const parsedSetting = parseVisionModelSetting(this.visionModel); @@ -3908,7 +3920,8 @@ export class Config { } getMcpTransportPool(): - import('../tools/mcp-transport-pool.js').McpTransportPool | undefined { + | import('../tools/mcp-transport-pool.js').McpTransportPool + | undefined { return this.mcpTransportPool; } @@ -5735,7 +5748,8 @@ export class Config { * has been registered (e.g., in SDK mode). */ getModelInvocableCommandsProvider(): - (() => ReadonlyArray<{ name: string; description: string }>) | null { + | (() => ReadonlyArray<{ name: string; description: string }>) + | null { return this.modelInvocableCommandsProvider; } @@ -5869,8 +5883,9 @@ export class Config { if (options?.forSubAgent) return; const schema = this.jsonSchema; await registerLazy(ToolNames.STRUCTURED_OUTPUT, async () => { - const { SyntheticOutputTool } = - await import('../tools/syntheticOutput.js'); + const { SyntheticOutputTool } = await import( + '../tools/syntheticOutput.js' + ); return new SyntheticOutputTool(schema); }); }; @@ -5905,8 +5920,9 @@ export class Config { return new ToolSearchTool(this); }); await registerLazy(ToolNames.READ_MCP_RESOURCE, async () => { - const { ReadMcpResourceTool } = - await import('../tools/read-mcp-resource.js'); + const { ReadMcpResourceTool } = await import( + '../tools/read-mcp-resource.js' + ); return new ReadMcpResourceTool(this); }); await registerLazy(ToolNames.AGENT, async () => { @@ -5994,8 +6010,9 @@ export class Config { return new TodoWriteTool(this); }); await registerLazy(ToolNames.ASK_USER_QUESTION, async () => { - const { AskUserQuestionTool } = - await import('../tools/askUserQuestion.js'); + const { AskUserQuestionTool } = await import( + '../tools/askUserQuestion.js' + ); return new AskUserQuestionTool(this); }); if (!this.sdkMode) { @@ -6022,8 +6039,9 @@ export class Config { }); if (this.isArtifactEnabled()) { await registerLazy(ToolNames.ARTIFACT, async () => { - const { ArtifactTool } = - await import('../tools/artifact/artifact-tool.js'); + const { ArtifactTool } = await import( + '../tools/artifact/artifact-tool.js' + ); return new ArtifactTool(this); }); } @@ -6112,8 +6130,9 @@ export class Config { // built-in also gates these. Direct registry.registerFactory() would // bypass coreTools allowlist + whole-tool deny rules. if (this.isComputerUseEnabled()) { - const { registerComputerUseTools } = - await import('../tools/computer-use/index.js'); + const { registerComputerUseTools } = await import( + '../tools/computer-use/index.js' + ); await registerComputerUseTools(registerLazy, this); } diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index b257408ad88..5f64e290455 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -755,6 +755,14 @@ describe('AgentTool', () => { ); expect(result.llmContent).toContain('nesting depth limit reached'); + // The user-facing display mirrors a failed task execution, not a bare + // string, so the UI renders it like any other failed sub-agent run. + expect(result.returnDisplay).toMatchObject({ + type: 'task_execution', + status: 'failed', + subagentName: 'file-search', + terminateReason: 'Nesting depth limit reached (max 1)', + }); expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); }); @@ -835,6 +843,26 @@ describe('AgentTool', () => { 'file-search', ); }); + + it('falls back to a regular general-purpose sub-agent when a nested sub-agent requests a fork', async () => { + // Fork is a top-level-only capability in v1. A nested sub-agent + // requesting `subagent_type: "fork"` must get the awaitable + // general-purpose sub-agent, not an error and not a nested fork. + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(5); + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue(null); + const invocation = agentTool.build({ + description: 'Fork from a nested sub-agent', + prompt: 'Do work', + subagent_type: 'fork', + }); + await runWithAgentContext('sub-1', () => + invocation.execute(new AbortController().signal), + ); + + expect(mockSubagentManager.loadSubagent).toHaveBeenCalledWith( + 'general-purpose', + ); + }); }); describe('refreshSubagents', () => { diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 9f63a5c3254..28e65a93fb6 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -1828,10 +1828,12 @@ class AgentToolInvocation extends BaseToolInvocation { // when it would exceed maxSubagentDepth. AgentCore.prepareTools() also // hides the AgentTool from leaf-depth sub-agents (same canSpawnNestedAgent // check), so a well-behaved model never reaches here. Teammate spawns - // returned above and are not gated by depth. See - // knowledge/qwen-code/design/nested-subagents.md. + // returned above and are not gated by depth. const maxSubagentDepth = this.config.getMaxSubagentDepth(); if (!canSpawnNestedAgent(maxSubagentDepth)) { + debugLogger.debug( + `[AgentTool] Nesting depth guard blocked spawn: childLevel=${childLaunchDepth() + 1} max=${maxSubagentDepth} type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, + ); return this.buildSpawnBlockedResult( `Error: sub-agent nesting depth limit reached ` + `(max ${maxSubagentDepth} level${maxSubagentDepth === 1 ? '' : 's'}). ` + @@ -1847,9 +1849,11 @@ class AgentToolInvocation extends BaseToolInvocation { // `agent` from fork contexts, but wildcard/fallback tool lists make this // runtime backstop necessary. Blocks ALL agent calls from a fork child // (not just fork-in-fork). This runs before the fork branch below, so the - // recursive-fork case is subsumed here. See - // knowledge/qwen-code/design/nested-subagents.md. + // recursive-fork case is subsumed here. if (isInForkExecution()) { + debugLogger.debug( + `[AgentTool] Fork containment guard blocked spawn: type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, + ); return this.buildSpawnBlockedResult( 'Error: Cannot spawn sub-agents from within a fork. Please execute tasks directly.', 'Sub-agent spawning is not allowed inside a fork', @@ -2011,7 +2015,7 @@ class AgentToolInvocation extends BaseToolInvocation { // carry the AgentTool via nesting — that requests a fork falls back to // the awaitable general-purpose sub-agent (via effectiveSubagentType // below) instead of opening a nested fork. Fork nesting is deferred - // past v1. See knowledge/qwen-code/design/nested-subagents.md. + // past v1. isTopLevelSession(); const effectiveSubagentType = isFork ? undefined @@ -2020,6 +2024,15 @@ class AgentToolInvocation extends BaseToolInvocation { // fall back to the awaitable general-purpose subagent. DEFAULT_BUILTIN_SUBAGENT_TYPE : (requestedType ?? DEFAULT_BUILTIN_SUBAGENT_TYPE); + if (isForkRequested && !isFork) { + debugLogger.debug( + `[AgentTool] Fork request downgraded to a regular sub-agent (${ + isTopLevelSession() + ? 'forking unavailable in this session' + : 'forks do not nest' + }).`, + ); + } let subagentConfig: SubagentConfig; if (isFork) { From 819739a815ce444f2eedd16e6d270cdb2a4008fe Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 2 Jul 2026 13:18:26 +0000 Subject: [PATCH 3/9] fix(cli): add maxSubagentDepth to the CliArgs test literal The exhaustive CliArgs mock in gemini.test.tsx missed the new field, failing CI's clean tsc build (the local incremental build skipped the test file). --- packages/cli/src/gemini.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 1e4198afbde..e35b9b85bdc 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -1005,6 +1005,7 @@ describe('gemini.tsx main function kitty protocol', () => { maxSessionTurns: undefined, maxWallTime: undefined, maxToolCalls: undefined, + maxSubagentDepth: undefined, experimentalLsp: undefined, channel: undefined, chatRecording: undefined, From 07c002b9bea6e78e9a93fde00d4df122dc592b82 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 2 Jul 2026 22:56:17 +0000 Subject: [PATCH 4/9] fix(core): add a teammate backstop to the agent spawn guards execute() backstopped depth and fork containment but not the teammate exclusion, so its guards covered less than prepareTools() gates. A teammate spawn call that slipped past schema-hiding would have nested. Block it symmetrically with the fork guard, log the rejection, and pin the behavior in a test. --- packages/core/src/tools/agent/agent.test.ts | 29 +++++++++++++++++++++ packages/core/src/tools/agent/agent.ts | 21 +++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 5f64e290455..7ef8eb489d7 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -39,6 +39,7 @@ import { partToString } from '../../utils/partUtils.js'; import type { HookSystem } from '../../hooks/hookSystem.js'; import { PermissionMode } from '../../hooks/types.js'; import { runWithAgentContext } from '../../agents/runtime/agent-context.js'; +import { runWithTeammateIdentity } from '../../agents/team/identity.js'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -805,6 +806,34 @@ describe('AgentTool', () => { expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); }); + it('blocks a teammate from spawning any sub-agent', async () => { + // Teammates do not nest in v1. The schema layer strips `agent` from + // teammate tool lists; this pins the symmetric runtime backstop for a + // hallucinated call that slips past schema-hiding. Depth would permit + // the spawn here (max 5), so a pass proves the teammate guard fired. + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(5); + const invocation = agentTool.build({ + description: 'Spawn from a teammate', + prompt: 'Do work', + subagent_type: 'file-search', + }); + const result = await runWithTeammateIdentity( + { + agentId: 'scribe@demo', + agentName: 'scribe', + teamName: 'demo', + isTeamLead: false, + }, + () => + runWithAgentContext('teammate-1', () => + invocation.execute(new AbortController().signal), + ), + ); + + expect(result.llmContent).toContain('Teammates cannot spawn sub-agents'); + expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); + }); + it('blocks a fork child from spawning any sub-agent', async () => { // Forks must never spawn sub-agents. The runtime guard blocks ALL agent // calls from within a fork (not just fork-in-fork), catching the diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 28e65a93fb6..446fa4ea128 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -1843,6 +1843,27 @@ class AgentToolInvocation extends BaseToolInvocation { ); } + // ─── Teammate spawn guard ───────────────────────────────────── + // Teammates do not nest in v1: prepareTools() strips `agent` from every + // teammate schema, and this backstop rejects a hallucinated spawn call + // that slips past schema-hiding — symmetric with the fork guard below so + // the execute() backstops cover the same exclusions as prepareTools(). + // (execute() intentionally has no isTopLevelSession() term: the + // top-level session is the normal spawn path, while prepareTools() only + // ever serves agents and uses that check to fail closed on a missing + // frame — that asymmetry is why the two layers share canSpawnNestedAgent + // rather than one full predicate.) Teammate spawns via `name` returned + // in the team-routing branch above, which requires !isTeammate(). + if (isTeammate()) { + debugLogger.debug( + `[AgentTool] Teammate containment guard blocked spawn: type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, + ); + return this.buildSpawnBlockedResult( + 'Error: Teammates cannot spawn sub-agents. Complete this task directly with your own tools, or coordinate through send_message.', + 'Sub-agent spawning is not allowed from a teammate context', + ); + } + // ─── Fork spawn guard ───────────────────────────────────────── // Forks must never spawn sub-agents — the fork contract is // context-sharing, not isolation. AgentCore.prepareTools() already strips From 547716407000fd3e35019144ebf23d16ee01870f Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 2 Jul 2026 23:05:03 +0000 Subject: [PATCH 5/9] fix(core): normalize persisted maxSubagentDepth on resume MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resume path trusted the raw sidecar value, bypassing the Config clamp — a tampered or malformed sidecar (1e309 parses to Infinity; JSON.stringify turns Infinity into null) would remove the nesting cap for resumed agents. Extract the clamp into a shared normalizeMaxSubagentDepth used by both the Config constructor and the flag-restore path, and refresh the stale settings schema description (clamp range, non-finite fallback, workflow-agent wording). --- packages/cli/src/config/settingsSchema.ts | 2 +- .../agents/background-agent-resume.test.ts | 5 +++ packages/core/src/config/config.ts | 31 +++++++++++++------ packages/core/src/tools/agent/agent.ts | 13 ++++++-- .../schemas/settings.schema.json | 2 +- 5 files changed, 39 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 7effa86743d..b08b29f3602 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1367,7 +1367,7 @@ const SETTINGS_SCHEMA = { requiresRestart: false, default: 5, description: - 'Maximum number of nested sub-agent levels. 1 disables nesting (sub-agents cannot spawn sub-agents); the default 5 allows a sub-agent to spawn sub-agents up to five levels deep. Values below 1 are clamped to 1. Teammates, forks, and the workflow tool are excluded from nesting.', + 'Maximum sub-agent nesting depth (1-based levels: a top-level sub-agent is level 1). 1 keeps sub-agents available but disables nesting; the default 5 allows nesting up to five levels deep. Values clamp to the range 1-100; non-finite values fall back to the default. Teammates, forks, and workflow-spawned agents never nest regardless of this setting. Overridable via --max-subagent-depth.', showInDialog: false, }, chatCompression: { diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index 857182da7af..3c77224b981 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -1059,6 +1059,10 @@ describe('BackgroundAgentResumeService', () => { model: 'agent-model', maxSessionTurns: 7, maxToolCalls: 11, + // Deliberately out of range: the resume path must re-normalize + // persisted values with Config semantics (clamp to 1–100), so a + // malformed or tampered sidecar cannot bypass the nesting cap. + maxSubagentDepth: 5000, }, }); fs.writeFileSync( @@ -1121,6 +1125,7 @@ describe('BackgroundAgentResumeService', () => { expect(overriddenConfig.getModel()).toBe('agent-model'); expect(overriddenConfig.getMaxSessionTurns()).toBe(7); expect(overriddenConfig.getMaxToolCalls()).toBe(11); + expect(overriddenConfig.getMaxSubagentDepth()).toBe(100); }, 20000); it('restores the persisted launch depth so a resumed nested agent keeps its level', async () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 273720ad862..b0672fff71e 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1216,6 +1216,26 @@ function loadMemoryPressureConfig(): MemoryPressureConfig { return config; } +/** + * Normalizes a maxSubagentDepth value: absent or non-finite values fall back + * to the default of 5 (NaN would silently block all nesting, Infinity — e.g. + * JSON `1e309` — would unbound the recursion cap), and finite values floor + * and clamp to 1–100. Values below 1 clamp up so the knob never disables + * sub-agents outright — it only bounds nesting; 100 caps typos the way + * maxToolCalls' ceiling does. + * + * Shared by the Config constructor and the resume path that restores + * persisted launch flags, so a malformed or tampered agent sidecar cannot + * bypass the nesting cap. + */ +export function normalizeMaxSubagentDepth( + value: number | null | undefined, +): number { + return value == null || !Number.isFinite(value) + ? 5 + : Math.min(100, Math.max(1, Math.floor(value))); +} + function readMemoryPressureRatioEnv(envName: string, fallback: number): number { const raw = process.env[envName]; if (!raw) { @@ -1715,16 +1735,7 @@ export class Config { this.fileDiscoveryService = params.fileDiscoveryService ?? null; this.bugCommand = params.bugCommand; this.maxSessionTurns = params.maxSessionTurns ?? -1; - // Default 5 (nesting on). Explicit values below 1 clamp to 1 so the knob - // never disables sub-agents outright — it only bounds nesting. This - // setting is the recursion guardrail, so non-finite values (NaN would - // silently block all nesting, Infinity would unbound it) fall back to the - // default, and 100 caps typos the way maxToolCalls' ceiling does. - this.maxSubagentDepth = - params.maxSubagentDepth == null || - !Number.isFinite(params.maxSubagentDepth) - ? 5 - : Math.min(100, Math.max(1, Math.floor(params.maxSubagentDepth))); + this.maxSubagentDepth = normalizeMaxSubagentDepth(params.maxSubagentDepth); this.maxWallTimeSeconds = params.maxWallTimeSeconds ?? -1; this.maxToolCalls = params.maxToolCalls ?? -1; const clearContextOnIdle = params.clearContextOnIdle; diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 446fa4ea128..9174a803419 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -91,7 +91,11 @@ import { formatStopHookBlockingCapWarning, } from '../../hooks/stopHookCap.js'; import { toModelVisibleSubagentResult } from '../../agents/subagent-result.js'; -import { ApprovalMode, Config } from '../../config/config.js'; +import { + ApprovalMode, + Config, + normalizeMaxSubagentDepth, +} from '../../config/config.js'; import { createDenialState } from '../../permissions/denialTracking.js'; import { isTeammate } from '../../agents/team/identity.js'; import { isSubagentLikeExecutionContext } from '../../agents/runtime/subagent-plan-tool-policy.js'; @@ -440,7 +444,12 @@ function applyPersistedCliFlagOverrides( ov.getMaxToolCalls = () => flags.maxToolCalls; } if (flags.maxSubagentDepth !== undefined) { - ov.getMaxSubagentDepth = () => flags.maxSubagentDepth; + // Re-normalize across the serialization boundary: a malformed or + // tampered sidecar (JSON `1e309` parses to Infinity; JSON.stringify + // turns Infinity into null) must not bypass the nesting cap for + // resumed agents. Same semantics as the Config constructor. + const maxSubagentDepth = normalizeMaxSubagentDepth(flags.maxSubagentDepth); + ov.getMaxSubagentDepth = () => maxSubagentDepth; } } diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index d5e52506a2b..b7dbbe85c1a 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -593,7 +593,7 @@ "default": -1 }, "maxSubagentDepth": { - "description": "Maximum number of nested sub-agent levels. 1 disables nesting (sub-agents cannot spawn sub-agents); the default 5 allows a sub-agent to spawn sub-agents up to five levels deep. Values below 1 are clamped to 1. Teammates, forks, and the workflow tool are excluded from nesting.", + "description": "Maximum sub-agent nesting depth (1-based levels: a top-level sub-agent is level 1). 1 keeps sub-agents available but disables nesting; the default 5 allows nesting up to five levels deep. Values clamp to the range 1-100; non-finite values fall back to the default. Teammates, forks, and workflow-spawned agents never nest regardless of this setting. Overridable via --max-subagent-depth.", "type": "number", "default": 5 }, From ed1177bd992be09c95f2a5c4eb850bb49e6d1901 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Thu, 2 Jul 2026 23:59:14 +0000 Subject: [PATCH 6/9] test(core): pin null-to-default normalization of resumed maxSubagentDepth JSON.stringify(Infinity) === 'null', so a sidecar can legitimately carry null; widen the persisted flag type to admit it and parameterize the resume test over both the clamp (5000 -> 100) and the null fallback (null -> 5). --- packages/core/src/agents/agent-transcript.ts | 6 +- .../agents/background-agent-resume.test.ts | 81 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/packages/core/src/agents/agent-transcript.ts b/packages/core/src/agents/agent-transcript.ts index 3949279d75c..4d6a1172e47 100644 --- a/packages/core/src/agents/agent-transcript.ts +++ b/packages/core/src/agents/agent-transcript.ts @@ -141,8 +141,12 @@ export interface AgentPersistedCliFlags { * Launch-time nesting cap. Interprets the persisted `depth` — without it a * nested agent launched under a lower cap would resume after restart under * the new session's (or default) cap and regain spawn capacity. + * + * `null` occurs when a non-finite in-memory value crossed JSON + * serialization (JSON.stringify(Infinity) === 'null'); the resume path + * normalizes it back to the default. */ - maxSubagentDepth?: number; + maxSubagentDepth?: number | null; } /** diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index 3c77224b981..fb2cc74fb68 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -1128,6 +1128,87 @@ describe('BackgroundAgentResumeService', () => { expect(overriddenConfig.getMaxSubagentDepth()).toBe(100); }, 20000); + it.each([ + // Out-of-range values clamp with Config semantics. + { persisted: 5000, expected: 100 }, + // JSON.stringify(Infinity) === 'null' — the sidecar can legitimately + // carry null for a non-finite in-memory value; it must fall back to + // the default, not leak through the getter override. + { persisted: null, expected: 5 }, + ])( + 'normalizes persisted maxSubagentDepth $persisted to $expected on resume', + async ({ persisted, expected }) => { + const sessionId = `session-depth-norm-${expected}`; + const agentId = `agent-depth-norm-${expected}`; + const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); + const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); + + writeAgentMeta(metaPath, { + agentId, + agentType: 'researcher', + description: 'Resume with persisted depth cap', + parentSessionId: sessionId, + parentAgentId: null, + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + persistedCliFlags: { maxSubagentDepth: persisted }, + }); + fs.writeFileSync( + outputFile, + JSON.stringify({ + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-20T00:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'Resume' }] }, + }) + '\n', + 'utf8', + ); + + registry.register({ + agentId, + description: 'Resume with persisted depth cap', + subagentType: 'researcher', + status: 'paused', + startTime: Date.now(), + abortController: new AbortController(), + prompt: 'Resume with persisted depth cap', + outputFile, + metaPath, + isBackgrounded: true, + }); + + const createAgentHeadless = vi.fn().mockResolvedValue({ + subagent: { + execute: vi.fn(async () => undefined), + setExternalMessageProvider: vi.fn(), + getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), + getExecutionSummary: () => ({ + totalTokens: 0, + outputTokens: 0, + totalDurationMs: 0, + }), + getTerminateMode: () => AgentTerminateMode.GOAL, + getFinalText: () => 'done', + }, + dispose: vi.fn().mockResolvedValue(undefined), + }); + + const { service, subagentManager } = createService(); + subagentManager.createAgentHeadless = createAgentHeadless; + + const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); + + expect(resumed).toBeDefined(); + expect(createAgentHeadless).toHaveBeenCalledTimes(1); + const [, overriddenConfig] = createAgentHeadless.mock.calls[0]!; + expect(overriddenConfig.getMaxSubagentDepth()).toBe(expected); + }, + 20000, + ); + it('restores the persisted launch depth so a resumed nested agent keeps its level', async () => { // Resume happens from a top-level frame (depth would recompute to 0); // the persisted meta.depth must be pinned via the runWithAgentContext From 4b0c5ac667756a9e8be28c8a1157e1adcd13a882 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Fri, 3 Jul 2026 01:28:54 +0000 Subject: [PATCH 7/9] fix(core): harden nesting depth edges from final review pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Normalize persisted meta.depth on resume: the sidecar is untrusted JSON, and a tampered negative depth (or -1e309 → -Infinity) would pin the resumed frame below zero and pass canSpawnNestedAgent for every cap. Invalid values fail closed to the depth ceiling — the agent keeps running but cannot spawn. - Register monitor notification routing for in-process interactive agents: framing runLoop() made their monitors agent-owned, and owned dispatch has no session fallback, so notifications were silently dropped. InProcessBackend now routes them into the agent's message queue and tears the routing down on release. - Downgrade background spawn requests from nested launchers to awaited foreground runs: a nested launcher cannot honor the background completion contract (send_message/task_stop excluded, notifications session-scoped), which orphaned the child's results. - Extract spawnBlockReason() as the single spawn-exclusion policy shared by prepareTools() and execute(), replacing two hand-kept copies of the depth/teammate/fork rules. - Share DEFAULT_MAX_SUBAGENT_DEPTH / MAX_SUBAGENT_DEPTH_LIMIT across the core normalizer, the CLI flag validator, and the settings schema. - Log dropped teammate names from nested spawns; revert the impossible |null persisted-flag widening to an honest tampered-sidecar framing; document the constructor-time depth capture invariant. --- packages/cli/src/config/config.ts | 9 +- packages/cli/src/config/settingsSchema.ts | 3 +- .../core/src/agents/agent-transcript.test.ts | 26 +++ packages/core/src/agents/agent-transcript.ts | 38 ++++- .../agents/backends/InProcessBackend.test.ts | 49 ++++++ .../src/agents/backends/InProcessBackend.ts | 23 +++ .../agents/background-agent-resume.test.ts | 150 ++++++++++-------- .../src/agents/background-agent-resume.ts | 11 +- .../src/agents/runtime/agent-context.test.ts | 28 ++++ .../core/src/agents/runtime/agent-context.ts | 30 ++++ .../core/src/agents/runtime/agent-core.ts | 12 +- .../src/agents/runtime/agent-interactive.ts | 8 + packages/core/src/config/config.ts | 17 +- packages/core/src/tools/agent/agent.test.ts | 56 +++++++ packages/core/src/tools/agent/agent.ts | 129 ++++++++------- 15 files changed, 435 insertions(+), 154 deletions(-) diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 68c74c4d0f1..f543fd0876b 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -36,6 +36,7 @@ import { SchemaValidator, type ConfigParameters, type MCPServerConfig, + MAX_SUBAGENT_DEPTH_LIMIT, } from '@qwen-code/qwen-code-core'; import { extensionsCommand } from '../commands/extensions.js'; import { hooksCommand } from '../commands/hooks.js'; @@ -1257,9 +1258,13 @@ function resolveMaxSubagentDepth( ): number | undefined { const value = argv.maxSubagentDepth; if (value !== undefined && value !== null) { - if (!Number.isInteger(value) || value < 1 || value > 100) { + if ( + !Number.isInteger(value) || + value < 1 || + value > MAX_SUBAGENT_DEPTH_LIMIT + ) { throw new Error( - `--max-subagent-depth must be an integer between 1 and 100; got ${value}.`, + `--max-subagent-depth must be an integer between 1 and ${MAX_SUBAGENT_DEPTH_LIMIT}; got ${value}.`, ); } return value; diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index b08b29f3602..96e1a4d4e7b 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -16,6 +16,7 @@ import type { } from '@qwen-code/qwen-code-core'; import { ApprovalMode, + DEFAULT_MAX_SUBAGENT_DEPTH, DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH, DEFAULT_QWEN_CUSTOM_IGNORE_FILE_NAMES, DEFAULT_STOP_HOOK_BLOCK_CAP, @@ -1365,7 +1366,7 @@ const SETTINGS_SCHEMA = { label: 'Max Sub-agent Nesting Depth', category: 'Model', requiresRestart: false, - default: 5, + default: DEFAULT_MAX_SUBAGENT_DEPTH, description: 'Maximum sub-agent nesting depth (1-based levels: a top-level sub-agent is level 1). 1 keeps sub-agents available but disables nesting; the default 5 allows nesting up to five levels deep. Values clamp to the range 1-100; non-finite values fall back to the default. Teammates, forks, and workflow-spawned agents never nest regardless of this setting. Overridable via --max-subagent-depth.', showInDialog: false, diff --git a/packages/core/src/agents/agent-transcript.test.ts b/packages/core/src/agents/agent-transcript.test.ts index 49b31e65480..78261b6fdaa 100644 --- a/packages/core/src/agents/agent-transcript.test.ts +++ b/packages/core/src/agents/agent-transcript.test.ts @@ -13,6 +13,7 @@ import { getAgentJsonlPath, getAgentMetaPath, attachJsonlTranscriptWriter, + normalizeResumedAgentDepth, readAgentMeta, readLastTranscriptRecordUuidSync, writeAgentMeta, @@ -582,4 +583,29 @@ describe('agent-transcript', () => { expect(readLastTranscriptRecordUuidSync(jsonlPath)).toBe(records[1].uuid); }); }); + + describe('normalizeResumedAgentDepth', () => { + it('passes through valid persisted depths and absent values', () => { + expect(normalizeResumedAgentDepth(undefined)).toBeUndefined(); + expect(normalizeResumedAgentDepth(0)).toBe(0); + expect(normalizeResumedAgentDepth(3)).toBe(3); + expect(normalizeResumedAgentDepth(100)).toBe(100); + }); + + it('fails closed (no spawn capacity) on tampered or corrupt values', () => { + // A negative depth would make canSpawnNestedAgent() pass for every + // cap; clamping down to 0 would likewise fail open by granting full + // spawn capacity. Anything invalid pins to the ceiling instead. + expect(normalizeResumedAgentDepth(-50)).toBe(100); + // JSON `-1e309` parses to -Infinity. + expect(normalizeResumedAgentDepth(-Infinity)).toBe(100); + expect(normalizeResumedAgentDepth(Infinity)).toBe(100); + expect(normalizeResumedAgentDepth(NaN)).toBe(100); + expect(normalizeResumedAgentDepth(2.5)).toBe(100); + expect(normalizeResumedAgentDepth(101)).toBe(100); + expect(normalizeResumedAgentDepth(null as unknown as number)).toBe( + undefined, + ); + }); + }); }); diff --git a/packages/core/src/agents/agent-transcript.ts b/packages/core/src/agents/agent-transcript.ts index 4d6a1172e47..b4081015691 100644 --- a/packages/core/src/agents/agent-transcript.ts +++ b/packages/core/src/agents/agent-transcript.ts @@ -33,6 +33,7 @@ import type { AgentBootstrapRecordPayload, ChatRecord, } from '../services/chatRecordingService.js'; +import { MAX_SUBAGENT_DEPTH_LIMIT } from '../config/config.js'; import type { SandboxConfig } from '../config/config.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { _recoverObjectsFromLine } from '../utils/jsonl-utils.js'; @@ -121,7 +122,10 @@ export interface AgentMeta { agentColor?: string; /** Number of explicit resume attempts performed so far. */ resumeCount?: number; - /** Nesting depth at launch time; restored on background/foreground resume. */ + /** + * Nesting depth at launch time; restored on background/foreground resume + * via {@link normalizeResumedAgentDepth} — never trust the raw value. + */ depth?: number; /** Last terminal error, if any. */ lastError?: string; @@ -142,11 +146,35 @@ export interface AgentPersistedCliFlags { * nested agent launched under a lower cap would resume after restart under * the new session's (or default) cap and regain spawn capacity. * - * `null` occurs when a non-finite in-memory value crossed JSON - * serialization (JSON.stringify(Infinity) === 'null'); the resume path - * normalizes it back to the default. + * Always a normalized 1–100 integer when written by this codebase; the + * resume path still re-normalizes because the sidecar is a plain JSON + * file a malformed or hand-edited copy of which can carry anything. */ - maxSubagentDepth?: number | null; + maxSubagentDepth?: number; +} + +/** + * Normalizes a persisted launch depth read from an agent sidecar before it + * is pinned via the `runWithAgentContext` depthOverride. The sidecar is a + * plain JSON file, so a malformed or hand-edited value must not mint spawn + * capacity: a negative depth (or `-1e309`, which parses to -Infinity) would + * make `canSpawnNestedAgent()` pass for every cap. + * + * Absent values return undefined (the resume frame derives its depth as a + * fresh launch). Anything but an integer within 0–{@link + * MAX_SUBAGENT_DEPTH_LIMIT} fails CLOSED to the limit: the resumed agent + * keeps running but cannot spawn — clamping a corrupt value down to 0 would + * fail open by granting full spawn capacity. + */ +export function normalizeResumedAgentDepth( + value: number | undefined, +): number | undefined { + if (value == null) return undefined; + return Number.isInteger(value) && + value >= 0 && + value <= MAX_SUBAGENT_DEPTH_LIMIT + ? value + : MAX_SUBAGENT_DEPTH_LIMIT; } /** diff --git a/packages/core/src/agents/backends/InProcessBackend.test.ts b/packages/core/src/agents/backends/InProcessBackend.test.ts index 9fe0bf5c57f..c081252f974 100644 --- a/packages/core/src/agents/backends/InProcessBackend.test.ts +++ b/packages/core/src/agents/backends/InProcessBackend.test.ts @@ -132,6 +132,10 @@ function createMockConfig() { return { getModel: vi.fn().mockReturnValue('test-model'), getToolRegistry: vi.fn().mockReturnValue(registry), + getMonitorRegistry: vi.fn().mockReturnValue({ + setAgentNotificationCallback: vi.fn(), + cancelRunningForOwner: vi.fn(), + }), getSessionId: vi.fn().mockReturnValue('test-session'), getPlansDir: vi.fn().mockReturnValue('/tmp/plans'), getApprovalMode: vi.fn().mockReturnValue(DEFAULT_MODE), @@ -220,6 +224,51 @@ describe('InProcessBackend', () => { expect(backend.getAgent('agent-1')).toBeDefined(); }); + it('routes owned monitor notifications into the agent message queue', async () => { + // AgentInteractive frames tool bodies under the agent identity, so a + // monitor it starts is agent-owned — and owned-monitor dispatch has no + // session fallback. Spawn must register the routing; stop must tear it + // down and cancel anything still running. + const mockConfig = createMockConfig(); + const monitorRegistry = ( + mockConfig as unknown as { + getMonitorRegistry: () => { + setAgentNotificationCallback: ReturnType; + cancelRunningForOwner: ReturnType; + }; + } + ).getMonitorRegistry(); + const localBackend = new InProcessBackend( + mockConfig as unknown as ConstructorParameters< + typeof InProcessBackend + >[0], + ); + await localBackend.init(); + await localBackend.spawnAgent(createSpawnConfig('agent-1')); + + expect(monitorRegistry.setAgentNotificationCallback).toHaveBeenCalledWith( + 'agent-1', + expect.any(Function), + ); + const callback = monitorRegistry.setAgentNotificationCallback.mock + .calls[0]![1] as (displayText: string, modelText: string) => void; + + const agent = localBackend.getAgent('agent-1')!; + const enqueueSpy = vi.spyOn(agent, 'enqueueMessage'); + callback('display text', 'model text'); + expect(enqueueSpy).toHaveBeenCalledWith('model text'); + + localBackend.stopAgent('agent-1'); + expect(monitorRegistry.cancelRunningForOwner).toHaveBeenCalledWith( + 'agent-1', + { notify: false }, + ); + expect(monitorRegistry.setAgentNotificationCallback).toHaveBeenCalledWith( + 'agent-1', + undefined, + ); + }); + it('should set first spawned agent as active', async () => { await backend.init(); await backend.spawnAgent(createSpawnConfig('agent-1')); diff --git a/packages/core/src/agents/backends/InProcessBackend.ts b/packages/core/src/agents/backends/InProcessBackend.ts index 9f0fca05d2e..cf7ffdd9acb 100644 --- a/packages/core/src/agents/backends/InProcessBackend.ts +++ b/packages/core/src/agents/backends/InProcessBackend.ts @@ -157,6 +157,22 @@ export class InProcessBackend implements Backend { this.agents.set(config.agentId, interactive); this.agentOrder.push(config.agentId); + // Route owned monitor notifications into this agent's message queue. + // AgentInteractive frames every tool body under the agent identity, so a + // `monitor` started by this agent is stamped with its ownerAgentId — and + // MonitorRegistry.dispatchNotification routes owned monitors ONLY + // through agentNotificationCallbacks, with no session fallback. Without + // this registration (the in-process analogue of AgentTool's + // registerOwnedMonitorNotifications) those notifications are silently + // dropped and a start-only Monitor can never report back to the agent. + // enqueueMessage self-wakes the message pump, so no lifecycle callback + // is needed. Unregistered in releaseAgentResources. + this.runtimeContext + .getMonitorRegistry() + .setAgentNotificationCallback(config.agentId, (_displayText, modelText) => + interactive.enqueueMessage(modelText), + ); + // Set first agent as active if (this.activeAgentId === null) { this.activeAgentId = config.agentId; @@ -402,6 +418,13 @@ export class InProcessBackend implements Backend { agentId: string, registry = this.agentRegistries.get(agentId), ): void { + // Tear down monitor routing registered in spawnAgent: stop any monitors + // the agent left running and drop its notification callback. Safe to + // run twice (the terminal watcher and stopAgent both funnel here). + const monitorRegistry = this.runtimeContext.getMonitorRegistry(); + monitorRegistry.cancelRunningForOwner(agentId, { notify: false }); + monitorRegistry.setAgentNotificationCallback(agentId, undefined); + const cleanup = this.agentApprovalCleanups.get(agentId); if (cleanup) { this.agentApprovalCleanups.delete(agentId); diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index fb2cc74fb68..0492b38b5b4 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -1131,10 +1131,10 @@ describe('BackgroundAgentResumeService', () => { it.each([ // Out-of-range values clamp with Config semantics. { persisted: 5000, expected: 100 }, - // JSON.stringify(Infinity) === 'null' — the sidecar can legitimately - // carry null for a non-finite in-memory value; it must fall back to - // the default, not leak through the getter override. - { persisted: null, expected: 5 }, + // This codebase never writes null, but the sidecar is a plain JSON + // file — a malformed or hand-edited copy can carry it; it must fall + // back to the default, not leak through the getter override. + { persisted: null as unknown as number, expected: 5 }, ])( 'normalizes persisted maxSubagentDepth $persisted to $expected on resume', async ({ persisted, expected }) => { @@ -1209,81 +1209,91 @@ describe('BackgroundAgentResumeService', () => { 20000, ); - it('restores the persisted launch depth so a resumed nested agent keeps its level', async () => { + it.each([ // Resume happens from a top-level frame (depth would recompute to 0); // the persisted meta.depth must be pinned via the runWithAgentContext // depthOverride, or a resumed nested agent would regain spawn capacity. - const sessionId = 'session-depth'; - const agentId = 'agent-depth'; - const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); - const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); + { persisted: 2, expected: 2 }, + // The sidecar is untrusted input: a tampered negative depth must fail + // closed to the depth ceiling (no spawn capacity), not pin the frame + // at a level that passes canSpawnNestedAgent() for every cap. + { persisted: -50, expected: 100 }, + ])( + 'restores persisted launch depth $persisted as $expected on resume', + async ({ persisted, expected }) => { + const sessionId = `session-depth-${expected}`; + const agentId = `agent-depth-${expected}`; + const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); + const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); - writeAgentMeta(metaPath, { - agentId, - agentType: 'researcher', - description: 'Resume nested agent', - parentSessionId: sessionId, - parentAgentId: 'agent-parent', - createdAt: '2026-04-20T00:00:00.000Z', - status: 'running', - subagentName: 'researcher', - depth: 2, - }); - fs.writeFileSync( - outputFile, - JSON.stringify({ - uuid: 'u1', - parentUuid: null, - sessionId, - timestamp: '2026-04-20T00:00:00.000Z', - type: 'user', - message: { role: 'user', parts: [{ text: 'Resume nested agent' }] }, - }) + '\n', - 'utf8', - ); + writeAgentMeta(metaPath, { + agentId, + agentType: 'researcher', + description: 'Resume nested agent', + parentSessionId: sessionId, + parentAgentId: 'agent-parent', + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + depth: persisted, + }); + fs.writeFileSync( + outputFile, + JSON.stringify({ + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-20T00:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'Resume nested agent' }] }, + }) + '\n', + 'utf8', + ); - registry.register({ - agentId, - description: 'Resume nested agent', - subagentType: 'researcher', - status: 'paused', - startTime: Date.now(), - abortController: new AbortController(), - prompt: 'Resume nested agent', - outputFile, - metaPath, - isBackgrounded: true, - }); + registry.register({ + agentId, + description: 'Resume nested agent', + subagentType: 'researcher', + status: 'paused', + startTime: Date.now(), + abortController: new AbortController(), + prompt: 'Resume nested agent', + outputFile, + metaPath, + isBackgrounded: true, + }); - let observedDepth = -1; - const createAgentHeadless = vi.fn().mockResolvedValue({ - subagent: { - execute: vi.fn(async () => { - observedDepth = getCurrentAgentDepth(); - }), - setExternalMessageProvider: vi.fn(), - getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), - getExecutionSummary: () => ({ - totalTokens: 0, - outputTokens: 0, - totalDurationMs: 0, - }), - getTerminateMode: () => AgentTerminateMode.GOAL, - getFinalText: () => 'done', - }, - dispose: vi.fn().mockResolvedValue(undefined), - }); + let observedDepth = -1; + const createAgentHeadless = vi.fn().mockResolvedValue({ + subagent: { + execute: vi.fn(async () => { + observedDepth = getCurrentAgentDepth(); + }), + setExternalMessageProvider: vi.fn(), + getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), + getExecutionSummary: () => ({ + totalTokens: 0, + outputTokens: 0, + totalDurationMs: 0, + }), + getTerminateMode: () => AgentTerminateMode.GOAL, + getFinalText: () => 'done', + }, + dispose: vi.fn().mockResolvedValue(undefined), + }); - const { service, subagentManager } = createService(); - subagentManager.createAgentHeadless = createAgentHeadless; + const { service, subagentManager } = createService(); + subagentManager.createAgentHeadless = createAgentHeadless; - const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); + const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); - expect(resumed).toBeDefined(); - await vi.waitFor(() => { - expect(observedDepth).toBe(2); - }); - }, 20000); + expect(resumed).toBeDefined(); + await vi.waitFor(() => { + expect(observedDepth).toBe(expected); + }); + }, + 20000, + ); it('coalesces concurrent resume calls into a single running agent', async () => { const sessionId = 'session-double'; diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index 5cf7a4a1a3f..b1471b1f269 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -19,6 +19,7 @@ import { AgentTerminateMode } from './runtime/agent-types.js'; import { AgentHeadless, ContextState } from './runtime/agent-headless.js'; import { getSubagentSessionDir, + normalizeResumedAgentDepth, readAgentMeta, patchAgentMeta, attachJsonlTranscriptWriter, @@ -1030,9 +1031,15 @@ export class BackgroundAgentResumeService { // Restore the persisted launch depth so a resumed nested agent keeps // its original nesting level (and spawn eligibility) instead of - // recomputing to depth 0 from this top-level resume frame. + // recomputing to depth 0 from this top-level resume frame. Normalized + // because the sidecar is untrusted input — a tampered negative depth + // would otherwise mint unbounded spawn capacity. const framedRunBody = () => - runWithAgentContext(meta.agentId, runBody, meta.depth); + runWithAgentContext( + meta.agentId, + runBody, + normalizeResumedAgentDepth(meta.depth), + ); void (target.isFork ? runInForkContext(framedRunBody) : framedRunBody()); return entry; } catch (error) { diff --git a/packages/core/src/agents/runtime/agent-context.test.ts b/packages/core/src/agents/runtime/agent-context.test.ts index 3d4a2b5b15e..71e11a419c1 100644 --- a/packages/core/src/agents/runtime/agent-context.test.ts +++ b/packages/core/src/agents/runtime/agent-context.test.ts @@ -14,8 +14,11 @@ import { isTopLevelSession, runWithAgentContext, runWithRuntimeContentGenerator, + spawnBlockReason, type RuntimeContentGeneratorView, } from './agent-context.js'; +import { runWithTeammateIdentity } from '../team/identity.js'; +import { runInForkContext } from '../../tools/agent/fork-subagent.js'; import { AuthType, type ContentGenerator, @@ -267,4 +270,29 @@ describe('agent-context (nesting predicates)', () => { 4, ); }); + + it('spawnBlockReason: depth wins first, then teammate, then fork, else null', async () => { + expect(spawnBlockReason(5)).toBeNull(); + await runWithAgentContext('lvl1', async () => { + expect(spawnBlockReason(5)).toBeNull(); + expect(spawnBlockReason(1)).toBe('depth'); + await runWithTeammateIdentity( + { + agentId: 'scribe@demo', + agentName: 'scribe', + teamName: 'demo', + isTeamLead: false, + }, + async () => { + expect(spawnBlockReason(5)).toBe('teammate'); + // A teammate at leaf depth reports depth — the reason order is + // execute()'s guard order, keeping the user-facing message stable. + expect(spawnBlockReason(1)).toBe('depth'); + }, + ); + await runInForkContext(async () => { + expect(spawnBlockReason(5)).toBe('fork'); + }); + }); + }); }); diff --git a/packages/core/src/agents/runtime/agent-context.ts b/packages/core/src/agents/runtime/agent-context.ts index b5b5ba4b06f..2a7bc4276f5 100644 --- a/packages/core/src/agents/runtime/agent-context.ts +++ b/packages/core/src/agents/runtime/agent-context.ts @@ -23,6 +23,8 @@ import type { ContentGenerator, ContentGeneratorConfig, } from '../../core/contentGenerator.js'; +import { isTeammate } from '../team/identity.js'; +import { isInForkExecution } from '../../tools/agent/fork-subagent.js'; export interface RuntimeContentGeneratorView { readonly contentGenerator: ContentGenerator; @@ -132,3 +134,31 @@ export function childLaunchDepth(): number { export function canSpawnNestedAgent(maxDepth: number): boolean { return childLaunchDepth() + 1 <= maxDepth; } + +/** + * Single source of the sub-agent spawn exclusion policy, shared by + * `AgentCore.prepareTools()` (schema gating) and `AgentTool.execute()` + * (runtime guards) so the two layers cannot drift: a rule missed on the + * runtime side is a silent spawn bypass, missed on the schema side it burns + * model turns on guaranteed-rejected calls. + * + * Returns the first blocking reason — evaluated in `execute()`'s guard + * order, so the winning reason (and its user-facing message) is stable for + * contexts that trip several rules (e.g. a teammate at leaf depth) — or + * null when a spawn is permitted. All four inputs are pure + * AsyncLocalStorage reads, so the composition is order-insensitive for the + * schema side's boolean use. + * + * Callers gate on different frame requirements on top of this: + * `prepareTools()` adds `!isTopLevelSession()` to fail closed on a missing + * agent frame (it only ever serves agents), while `execute()` must allow + * the top-level session — that is the normal spawn path. + */ +export function spawnBlockReason( + maxDepth: number, +): 'depth' | 'teammate' | 'fork' | null { + if (!canSpawnNestedAgent(maxDepth)) return 'depth'; + if (isTeammate()) return 'teammate'; + if (isInForkExecution()) return 'fork'; + return null; +} diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 7b0c9de6666..8ec00c15a3d 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -22,16 +22,15 @@ import { reportError } from '../../utils/errorReporting.js'; import { subagentNameContext } from '../../utils/subagentNameContext.js'; import type { Config } from '../../config/config.js'; import { - canSpawnNestedAgent, getCurrentAgentDepth, getCurrentAgentId, getRuntimeContentGenerator, isTopLevelSession, runWithAgentContext, runWithRuntimeContentGenerator, + spawnBlockReason, type RuntimeContentGeneratorView, } from './agent-context.js'; -import { isInForkExecution } from '../../tools/agent/fork-subagent.js'; import { createDuplicateProviderToolCallResponse, findRepeatedDuplicateProviderToolCall, @@ -506,9 +505,8 @@ export class AgentCore { // recursive spawning, but when maxSubagentDepth permits another level we // let it back in. prepareTools() runs inside this sub-agent's own // AsyncLocalStorage frame (see AgentHeadless.run / AgentInteractive), so - // canSpawnNestedAgent() reads this agent's own depth. Teammates do not - // nest in v1, and forks must never spawn sub-agents (the fork contract is - // context-sharing, not isolation). + // spawnBlockReason() reads this agent's own depth and context — the same + // shared predicate AgentTool.execute() backstops at runtime. // // !isTopLevelSession() fails closed: prepareTools() only ever serves // agents — never the top-level user session — so a missing agent frame @@ -518,9 +516,7 @@ export class AgentCore { // AgentInteractive.start(), since fixed to establish its frame). const nestingAllowed = !isTopLevelSession() && - !isTeammate() && - !isInForkExecution() && - canSpawnNestedAgent(this.runtimeContext.getMaxSubagentDepth()); + spawnBlockReason(this.runtimeContext.getMaxSubagentDepth()) === null; // Effective exclusion test. AgentTool is depth-gated (allowed only when // this sub-agent is shallow enough to spawn another level); every other diff --git a/packages/core/src/agents/runtime/agent-interactive.ts b/packages/core/src/agents/runtime/agent-interactive.ts index 0c2c524d835..0fdb8014989 100644 --- a/packages/core/src/agents/runtime/agent-interactive.ts +++ b/packages/core/src/agents/runtime/agent-interactive.ts @@ -91,6 +91,14 @@ export class AgentInteractive { constructor(config: AgentInteractiveConfig, core: AgentCore) { this.config = config; this.core = core; + // Ambient capture: reads the SPAWNER's AsyncLocalStorage frame, so this + // is correct only while construction stays on the spawner's await chain + // (today: InProcessBackend.spawnAgent, whose entry paths are top-level + // gated). A future factory/queue/deferred construction would silently + // record depth 0 — the same deferral-loses-frame failure the resume + // path solves explicitly by persisting AgentMeta.depth. If construction + // ever moves off the spawn chain, thread the depth through + // AgentInteractiveConfig instead. this.agentDepth = childLaunchDepth(); this.setupEventListeners(); } diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index b0672fff71e..6d618d0d4d4 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1216,13 +1216,18 @@ function loadMemoryPressureConfig(): MemoryPressureConfig { return config; } +/** Default sub-agent nesting cap (1-based levels). */ +export const DEFAULT_MAX_SUBAGENT_DEPTH = 5; +/** Ceiling for the nesting cap — catches typos the way maxToolCalls' does. */ +export const MAX_SUBAGENT_DEPTH_LIMIT = 100; + /** * Normalizes a maxSubagentDepth value: absent or non-finite values fall back - * to the default of 5 (NaN would silently block all nesting, Infinity — e.g. + * to the default (NaN would silently block all nesting, Infinity — e.g. * JSON `1e309` — would unbound the recursion cap), and finite values floor - * and clamp to 1–100. Values below 1 clamp up so the knob never disables - * sub-agents outright — it only bounds nesting; 100 caps typos the way - * maxToolCalls' ceiling does. + * and clamp to the 1–{@link MAX_SUBAGENT_DEPTH_LIMIT} range. Values below 1 + * clamp up so the knob never disables sub-agents outright — it only bounds + * nesting. * * Shared by the Config constructor and the resume path that restores * persisted launch flags, so a malformed or tampered agent sidecar cannot @@ -1232,8 +1237,8 @@ export function normalizeMaxSubagentDepth( value: number | null | undefined, ): number { return value == null || !Number.isFinite(value) - ? 5 - : Math.min(100, Math.max(1, Math.floor(value))); + ? DEFAULT_MAX_SUBAGENT_DEPTH + : Math.min(MAX_SUBAGENT_DEPTH_LIMIT, Math.max(1, Math.floor(value))); } function readMemoryPressureRatioEnv(envName: string, fallback: number): number { diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 7ef8eb489d7..6a814eb2acd 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -873,6 +873,33 @@ describe('AgentTool', () => { ); }); + it('ignores a teammate name from a nested sub-agent and spawns a regular sub-agent', async () => { + // With a team active and depth permitting the spawn, a nested + // sub-agent's `name` must not reach executeTeammate (teammates do not + // nest in v1) — the spawn proceeds as a regular one-shot agent of the + // requested type. Pins the silent-success path at the default cap. + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(5); + const spawnTeammate = vi.fn().mockResolvedValue(undefined); + vi.mocked(config.getTeamManager).mockReturnValue({ + spawnTeammate, + } as never); + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue(null); + const invocation = agentTool.build({ + description: 'Spawn with a name from a nested sub-agent', + prompt: 'Do work', + subagent_type: 'file-search', + name: 'helper', + }); + await runWithAgentContext('sub-1', () => + invocation.execute(new AbortController().signal), + ); + + expect(spawnTeammate).not.toHaveBeenCalled(); + expect(mockSubagentManager.loadSubagent).toHaveBeenCalledWith( + 'file-search', + ); + }); + it('falls back to a regular general-purpose sub-agent when a nested sub-agent requests a fork', async () => { // Fork is a top-level-only capability in v1. A nested sub-agent // requesting `subagent_type: "fork"` must get the awaitable @@ -3112,6 +3139,35 @@ describe('AgentTool', () => { expect(mockRegistry.register).toHaveBeenCalled(); }); + it('downgrades a background request from a nested sub-agent to an awaited foreground run', async () => { + // Background delegation is top-level-only in v1: a nested launcher + // cannot honor the background completion contract (send_message / + // task_stop are excluded from its toolset, and completion + // notifications go to the top-level session). The run must complete + // inline instead of orphaning the child's results. + vi.mocked(config.getMaxSubagentDepth).mockReturnValue(5); + const params: AgentParams = { + description: 'Start monitor from a nested sub-agent', + prompt: 'Watch for changes', + subagent_type: 'monitor', + run_in_background: true, + }; + + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation(params); + const result = await runWithAgentContext('sub-1', () => + invocation.execute(), + ); + + const llmText = partToString(result.llmContent); + expect(llmText).not.toContain('Background agent launched'); + expect(llmText).toContain('Monitor done'); + expect(mockRegistry.register).toHaveBeenCalledWith( + expect.objectContaining({ isBackgrounded: false }), + ); + }); + it('returns registry registration errors to the model without launching the background body', async () => { const errorMessage = 'Cannot start background agent: maximum concurrent background agents ' + diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 9174a803419..15d5cc0888e 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -41,7 +41,6 @@ import { buildForkedMessages, buildChildMessage, buildWorktreeNotice, - isInForkExecution, isForkSubagentEnabled, runInForkContext, } from './fork-subagent.js'; @@ -53,11 +52,11 @@ import { import { FileDiscoveryService } from '../../services/fileDiscoveryService.js'; import { WorkspaceContext } from '../../utils/workspaceContext.js'; import { - canSpawnNestedAgent, childLaunchDepth, getCurrentAgentId, isTopLevelSession, runWithAgentContext, + spawnBlockReason, } from '../../agents/runtime/agent-context.js'; import { trace, context as otelContext } from '@opentelemetry/api'; import { @@ -444,10 +443,11 @@ function applyPersistedCliFlagOverrides( ov.getMaxToolCalls = () => flags.maxToolCalls; } if (flags.maxSubagentDepth !== undefined) { - // Re-normalize across the serialization boundary: a malformed or - // tampered sidecar (JSON `1e309` parses to Infinity; JSON.stringify - // turns Infinity into null) must not bypass the nesting cap for - // resumed agents. Same semantics as the Config constructor. + // Re-normalize across the serialization boundary: this codebase only + // ever persists a normalized 1-100 integer, but the sidecar is a plain + // JSON file — a malformed or hand-edited copy (out-of-range numbers, + // `1e309` → Infinity, or a literal null) must not bypass the nesting + // cap for resumed agents. Same semantics as the Config constructor. const maxSubagentDepth = normalizeMaxSubagentDepth(flags.maxSubagentDepth); ov.getMaxSubagentDepth = () => maxSubagentDepth; } @@ -628,7 +628,7 @@ export class AgentTool extends BaseDeclarativeTool { run_in_background: { type: 'boolean', description: - 'Set to true to run this agent in the background. You will be notified when it completes.', + 'Set to true to run this agent in the background. You will be notified when it completes. Top-level session only: from within a sub-agent the task runs in the foreground and returns its result inline.', }, ...(config.isAgentTeamEnabled() ? { @@ -1830,64 +1830,57 @@ class AgentToolInvocation extends BaseToolInvocation { } else { return this.executeTeammate(this.params.name, signal, updateOutput); } - } - - // ─── Nesting depth guard ────────────────────────────────────── - // Authoritative runtime backstop for sub-agent nesting: reject the spawn - // when it would exceed maxSubagentDepth. AgentCore.prepareTools() also - // hides the AgentTool from leaf-depth sub-agents (same canSpawnNestedAgent - // check), so a well-behaved model never reaches here. Teammate spawns - // returned above and are not gated by depth. - const maxSubagentDepth = this.config.getMaxSubagentDepth(); - if (!canSpawnNestedAgent(maxSubagentDepth)) { - debugLogger.debug( - `[AgentTool] Nesting depth guard blocked spawn: childLevel=${childLaunchDepth() + 1} max=${maxSubagentDepth} type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, - ); - return this.buildSpawnBlockedResult( - `Error: sub-agent nesting depth limit reached ` + - `(max ${maxSubagentDepth} level${maxSubagentDepth === 1 ? '' : 's'}). ` + - `Complete this task directly with your own tools instead of ` + - `spawning another sub-agent.`, - `Nesting depth limit reached (max ${maxSubagentDepth})`, - ); - } - - // ─── Teammate spawn guard ───────────────────────────────────── - // Teammates do not nest in v1: prepareTools() strips `agent` from every - // teammate schema, and this backstop rejects a hallucinated spawn call - // that slips past schema-hiding — symmetric with the fork guard below so - // the execute() backstops cover the same exclusions as prepareTools(). - // (execute() intentionally has no isTopLevelSession() term: the - // top-level session is the normal spawn path, while prepareTools() only - // ever serves agents and uses that check to fail closed on a missing - // frame — that asymmetry is why the two layers share canSpawnNestedAgent - // rather than one full predicate.) Teammate spawns via `name` returned - // in the team-routing branch above, which requires !isTeammate(). - if (isTeammate()) { + } else if (this.params.name && !isTeammate()) { + // Nested sub-agent passing `name`: the parameter is dropped and a + // regular one-shot agent spawns at the next level. Log it — the + // silent fall-through is otherwise invisible to operators debugging + // why an expected teammate never appeared. debugLogger.debug( - `[AgentTool] Teammate containment guard blocked spawn: type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, - ); - return this.buildSpawnBlockedResult( - 'Error: Teammates cannot spawn sub-agents. Complete this task directly with your own tools, or coordinate through send_message.', - 'Sub-agent spawning is not allowed from a teammate context', + `[AgentTool] Ignoring teammate name "${this.params.name}" from a nested sub-agent; spawning a regular sub-agent instead.`, ); } - // ─── Fork spawn guard ───────────────────────────────────────── - // Forks must never spawn sub-agents — the fork contract is - // context-sharing, not isolation. AgentCore.prepareTools() already strips - // `agent` from fork contexts, but wildcard/fallback tool lists make this - // runtime backstop necessary. Blocks ALL agent calls from a fork child - // (not just fork-in-fork). This runs before the fork branch below, so the - // recursive-fork case is subsumed here. - if (isInForkExecution()) { + // ─── Spawn guards ───────────────────────────────────────────── + // Authoritative runtime backstop for the shared spawn exclusion policy + // (spawnBlockReason — the same predicate prepareTools() uses to hide the + // AgentTool, so the two layers cannot drift). A well-behaved model never + // reaches here, but wildcard/fallback tool lists and hallucinated calls + // make the runtime check load-bearing: depth bounds nesting, teammates + // do not nest in v1, and forks must never spawn (the fork contract is + // context-sharing, not isolation — blocking ALL agent calls here, before + // the fork branch below, subsumes the recursive-fork case). Teammate + // spawns via `name` returned in the team-routing branch above, which + // requires !isTeammate(). + const maxSubagentDepth = this.config.getMaxSubagentDepth(); + const spawnBlocked = spawnBlockReason(maxSubagentDepth); + if (spawnBlocked !== null) { debugLogger.debug( - `[AgentTool] Fork containment guard blocked spawn: type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, - ); - return this.buildSpawnBlockedResult( - 'Error: Cannot spawn sub-agents from within a fork. Please execute tasks directly.', - 'Sub-agent spawning is not allowed inside a fork', + `[AgentTool] Spawn blocked (${spawnBlocked}): childLevel=${childLaunchDepth() + 1} max=${maxSubagentDepth} type=${this.params.subagent_type ?? DEFAULT_BUILTIN_SUBAGENT_TYPE}`, ); + switch (spawnBlocked) { + case 'depth': + return this.buildSpawnBlockedResult( + `Error: sub-agent nesting depth limit reached ` + + `(max ${maxSubagentDepth} level${maxSubagentDepth === 1 ? '' : 's'}). ` + + `Complete this task directly with your own tools instead of ` + + `spawning another sub-agent.`, + `Nesting depth limit reached (max ${maxSubagentDepth})`, + ); + case 'teammate': + return this.buildSpawnBlockedResult( + 'Error: Teammates cannot spawn sub-agents. Complete this task directly with your own tools, or coordinate through send_message.', + 'Sub-agent spawning is not allowed from a teammate context', + ); + case 'fork': + return this.buildSpawnBlockedResult( + 'Error: Cannot spawn sub-agents from within a fork. Please execute tasks directly.', + 'Sub-agent spawning is not allowed inside a fork', + ); + default: { + const exhaustive: never = spawnBlocked; + throw new Error(`Unhandled spawn block reason: ${exhaustive}`); + } + } } // ── Isolation state hoisted to the outermost scope ──────────── @@ -2102,9 +2095,25 @@ class AgentToolInvocation extends BaseToolInvocation { } // OR the tool parameter with the agent definition's background flag. - const shouldRunInBackground = + // + // Background delegation is top-level-only in v1, mirroring the fork + // downgrade above: a nested launcher would be handed a completion + // contract it cannot honor — the success guidance names send_message + // and task_stop (both excluded from sub-agent toolsets), and + // BackgroundTaskRegistry's single session-level notification callback + // would inject the child's completion into the top-level conversation + // while the launcher (typically finished by then) never hears back. + // Downgrade to an awaited foreground run instead of orphaning the + // child's results. + const backgroundRequested = this.params.run_in_background === true || subagentConfig.background === true; + const shouldRunInBackground = backgroundRequested && isTopLevelSession(); + if (backgroundRequested && !shouldRunInBackground) { + debugLogger.debug( + `[AgentTool] Background request downgraded to a foreground run for a nested sub-agent (type=${subagentConfig.name}).`, + ); + } // Preflight: fast-fail before expensive worktree/subagent setup. // This is not redundant with registry.register() below — that call From 6598ab8f0aaf47294959614b435f8fee0fe3abb7 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Fri, 3 Jul 2026 01:39:59 +0000 Subject: [PATCH 8/9] test(cli): add DEFAULT_MAX_SUBAGENT_DEPTH to core package mocks settingsSchema.ts now imports the shared constant, so CLI tests that mock @qwen-code/qwen-code-core with an explicit export list need the new export. --- packages/cli/src/acp-integration/acpAgent.worktree.test.ts | 1 + .../cli/src/serve/workspace-service/__tests__/facade.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts index 2646a0c5973..c7424816300 100644 --- a/packages/cli/src/acp-integration/acpAgent.worktree.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.worktree.test.ts @@ -101,6 +101,7 @@ vi.mock('@qwen-code/qwen-code-core', () => ({ APPROVAL_MODE_INFO: {}, APPROVAL_MODES: [], DEFAULT_STOP_HOOK_BLOCK_CAP: 8, + DEFAULT_MAX_SUBAGENT_DEPTH: 5, DEFAULT_SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH: 1024 * 1024, SENSITIVE_SPAN_ATTRIBUTE_MAX_LENGTH_LIMIT: 100 * 1024 * 1024, DEFAULT_TOOL_RESULTS_TOTAL_CHARS_THRESHOLD: 500_000, diff --git a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts index e5921dcc26e..9e81b71dea4 100644 --- a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts +++ b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts @@ -59,6 +59,7 @@ vi.mock('@qwen-code/qwen-code-core', () => { YOLO: 'yolo', }, DEFAULT_STOP_HOOK_BLOCK_CAP: 5, + DEFAULT_MAX_SUBAGENT_DEPTH: 5, DEFAULT_TOOL_OUTPUT_BATCH_BUDGET: 100_000, DEFAULT_TOOL_RESULTS_TOTAL_CHARS_THRESHOLD: 100_000, DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES: 2000, From 699cb26678747a3340297d20978cfdb9d5b1694e Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Fri, 3 Jul 2026 09:56:53 +0800 Subject: [PATCH 9/9] feat(cli): display nested sub-agents as a tree in the TUI (#6191) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(core): allow bounded nested sub-agent spawning via maxSubagentDepth Sub-agents may now spawn sub-agents up to a configurable maximum nesting depth (default 5; 1 reproduces the previous no-nesting behavior). Enforced in two layers sharing one predicate: prepareTools() hides the agent tool from leaf-depth sub-agents, and AgentTool.execute() rejects over-depth spawns as an authoritative backstop. Teammates, forks, and the workflow tool remain excluded from nesting. Launch depth is persisted in the agent meta sidecar and restored on resume (including deferred-approval continuations and in-process AgentInteractive frames) so a resumed nested agent cannot regain spawn capacity. See knowledge/qwen-code/design/nested-subagents.md. * feat(cli): display nested sub-agents as a tree in the TUI Render nested agents depth-first with indent + dim '↳' in the live agent panel and background tasks view; promote orphaned children to root with a '· from ' annotation. Detail view gains a level badge, Parent breadcrumb, and Sub-agents section. The [blocking] tag and two-step cancel confirm now apply only to provably user-blocking foreground chains. Parent completion summaries carry a '· N sub-agents' tail (guard-rejected spawns now record as failed tool calls so the count stays honest). Also fixes the live-panel Enter-for-detail order mismatch by sharing one display order between the panel render and the composer keyboard mapping. * fix(core): address round-1 review on nested sub-agent spawning - Derive launch metadata (hooks, spans, task rows, meta sidecar) from the resolved subagent config instead of the raw requested type, so a fork request that falls back to the awaitable path no longer reports "fork". - Pin the blocked-spawn failure contract in tests: error is set and returnDisplay.status is 'failed' for both the depth and fork guards; also document the failure-path routing at buildSpawnBlockedResult. - Drop source-comment references to private knowledge/ design docs that do not exist in this repository. * test: address round-2 review on sub-agent counting and fork fallback - Exercise the legacy 'task' alias in the scrollback sub-agent count so the migration-aware name set is covered, not just the canonical name. - Pin the nested-fork downgrade: a sub-agent requesting a fork falls back to the awaitable general-purpose subagent even in interactive mode. - Drop a duplicated 'nesting depth guard' describe block left behind by the automated base-branch merge (kept the copy with the failure-shape assertions). * fix(core): keep actionable guidance in blocked-spawn error messages The scheduler's failure path sends only error.message to the model and the scrollback, discarding llmContent. With the terse terminateReason as the message, a blocked spawn lost its "do the task yourself instead" instruction, inviting retry loops. Carry the full guidance text in error.message and keep terminateReason for the display card. * test(cli): pin the tree indent clamp at depth beyond TREE_INDENT_MAX_LEVELS Maintainer mutation-testing on the PR found that removing the clamp in treeRowPrefix survived the suite. Assert a depth-4 row indents 3 levels (12 spaces), plus the base marker/indent behavior. --------- Co-authored-by: qwen-code-dev-bot --- .../src/ui/components/InputPrompt.test.tsx | 11 +- .../cli/src/ui/components/InputPrompt.tsx | 17 +- .../BackgroundTasksDialog.test.tsx | 244 +++++++++++++++-- .../background-view/BackgroundTasksDialog.tsx | 157 ++++++++++- .../background-view/LiveAgentPanel.test.tsx | 60 ++++ .../background-view/LiveAgentPanel.tsx | 63 ++++- .../background-view/agent-forest.test.ts | 241 ++++++++++++++++ .../background-view/agent-forest.ts | 258 ++++++++++++++++++ .../liveAgentPanelVisibility.ts | 9 + .../components/messages/ToolMessage.test.tsx | 91 ++++++ .../ui/components/messages/ToolMessage.tsx | 22 ++ .../cli/src/ui/hooks/useBackgroundTaskView.ts | 79 +++--- .../src/agents/background-agent-resume.ts | 6 + .../core/src/agents/background-tasks.test.ts | 44 +++ packages/core/src/agents/background-tasks.ts | 27 ++ packages/core/src/tools/agent/agent.test.ts | 51 ++++ packages/core/src/tools/agent/agent.ts | 25 +- 17 files changed, 1316 insertions(+), 89 deletions(-) create mode 100644 packages/cli/src/ui/components/background-view/agent-forest.test.ts create mode 100644 packages/cli/src/ui/components/background-view/agent-forest.ts diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index 0c51da6e32f..b5070e53121 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -5097,7 +5097,12 @@ describe('InputPrompt', () => { unmount(); }); - it('Enter on the live panel maps visible agents back to their bg entry index', async () => { + it('Enter on the live panel maps the DISPLAYED row back to its bg entry index', async () => { + // The snapshot is newest-first but the panel renders oldest-first + // (panelDisplayOrder). Selecting panel row 2 must open the agent the + // user sees there — 'first-live-agent' (snapshot index 1) — not the + // mirrored snapshot position. Pins the fix for the wrong-detail bug + // found during the nested-subagents-ui E2E dry-run. mockedUseBackgroundTaskViewState.mockReturnValue({ entries: [ { kind: 'shell', shellId: 'bg-shell' }, @@ -5126,7 +5131,9 @@ describe('InputPrompt', () => { stdin.write('\r'); // Enter await wait(); - expect(mockViewActions.setBgSelectedIndex).toHaveBeenCalledWith(3); + // Display order: main, second-live-agent (row 1), first-live-agent + // (row 2). Row 2 → snapshot index 1. + expect(mockViewActions.setBgSelectedIndex).toHaveBeenCalledWith(1); expect(mockViewActions.enterBgDetailFromPanel).toHaveBeenCalled(); expect(mockViewActions.setLivePanelFocused).toHaveBeenCalledWith(false); unmount(); diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index fcbba2452b6..64173908c31 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -58,7 +58,11 @@ import { useBackgroundTaskViewState, useBackgroundTaskViewActions, } from '../contexts/BackgroundTaskViewContext.js'; -import { isLiveAgentPanelVisibleEntry } from './background-view/liveAgentPanelVisibility.js'; +import { + isLiveAgentPanelVisibleEntry, + LIVE_AGENT_PANEL_MAX_ROWS, +} from './background-view/liveAgentPanelVisibility.js'; +import { panelDisplayOrder } from './background-view/agent-forest.js'; import { FEEDBACK_DIALOG_KEYS } from '../FeedbackDialog.js'; import { BaseTextInput } from './BaseTextInput.js'; import type { RenderLineOptions } from './BaseTextInput.js'; @@ -234,8 +238,17 @@ export const InputPrompt: React.FC = ({ setPillFocused: setBgPillFocused, } = useBackgroundTaskViewActions(); const hasAgents = agents.size > 0; + // panelDisplayOrder + the maxRows tail-window mirror LiveAgentPanel's + // rendered rows exactly (oldest-first, nested agents grouped under + // their parent, windowed to the last LIVE_AGENT_PANEL_MAX_ROWS) so + // `livePanelSelectedIndex - 1` indexes the same agent the user sees + // highlighted. Filtering alone (snapshot order, newest-first, unsliced) + // opened the wrong agent's detail on Enter. const getVisibleBgAgents = useCallback( - () => bgEntries.filter((e) => isLiveAgentPanelVisibleEntry(e, Date.now())), + () => + panelDisplayOrder( + bgEntries.filter((e) => isLiveAgentPanelVisibleEntry(e, Date.now())), + ).slice(-LIVE_AGENT_PANEL_MAX_ROWS), [bgEntries], ); const hasActiveToolConfirmation = useMemo( diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx index 91e60a535c1..8e57919d7ea 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -24,32 +24,18 @@ import { } from '../../hooks/useBackgroundTaskView.js'; import { useKeypress } from '../../hooks/useKeypress.js'; -vi.mock('../../hooks/useBackgroundTaskView.js', () => ({ - useBackgroundTaskView: vi.fn(), - // Re-export the helper so Dialog renderers can still resolve it under the - // mocked module. Inline impl keeps the test independent of the hook - // module while preserving the discriminator-based id contract. - entryId: (entry: DialogEntry): string => { - switch (entry.kind) { - case 'agent': - return entry.agentId; - case 'shell': - return entry.shellId; - case 'monitor': - return entry.monitorId; - case 'workflow': - return entry.runId; - case 'dream': - return entry.dreamId; - default: { - const _exhaustive: never = entry; - throw new Error( - `entryId: unknown DialogEntry kind: ${JSON.stringify(_exhaustive)}`, - ); - } - } - }, -})); +vi.mock('../../hooks/useBackgroundTaskView.js', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('../../hooks/useBackgroundTaskView.js') + >(); + // Only the hook itself is stubbed (the Harness feeds entries directly); + // the module's pure helpers (entryId, compareActiveThenTerminal) keep + // their real implementations so renderer behavior can't silently drift + // from production — a hand-inlined copy here previously risked exactly + // that. + return { ...actual, useBackgroundTaskView: vi.fn() }; +}); vi.mock('../../hooks/useKeypress.js', () => ({ useKeypress: vi.fn(), @@ -158,7 +144,11 @@ function setup(initial: readonly DialogEntry[]): Harness { ); return match; }, + // The detail view lists an agent's children via `.getAll()` + // (Sub-agents section of the nested-agent tree display). + getAll: () => currentEntries.filter((e) => e.kind === 'agent'), }), + getMaxSubagentDepth: () => 5, getMonitorRegistry: () => ({ cancel: monitorCancel, // Resolve `.get(monitorId)` against the snapshot so the dialog's @@ -361,6 +351,35 @@ describe('BackgroundTasksDialog', () => { expect(h.cancel).toHaveBeenCalledWith('bg-1'); }); + it('foreground child of a background parent cancels on the first `x` (chain-aware confirm)', () => { + // The two-step confirm exists to protect the USER's turn. A nested + // foreground child awaited by a background parent blocks that parent, + // not the user — same chain verdict as the [blocking] row prefix — + // so it cancels one-shot like any background entry. + const bgParent = entry({ + id: 'p', + agentId: 'p', + description: 'bg parent work', + isBackgrounded: true, + }); + const fgChild = entry({ + id: 'c', + agentId: 'c', + description: 'awaited child work', + status: 'running', + isBackgrounded: false, + parentAgentId: 'p', + depth: 1, + }); + const h = setup([bgParent, fgChild]); + + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.moveSelectionDown()); + + h.pressKey({ sequence: 'x' }); + expect(h.cancel).toHaveBeenCalledWith('c'); + }); + it('ignores `x` on a terminal foreground entry (no arm, no cancel call)', () => { // A foreground entry briefly stays visible after settling but before // the tool-call's finally path unregisters it. The dialog's hint @@ -829,9 +848,7 @@ describe('BackgroundTasksDialog', () => { // ── R2 #15: WorkflowDetailBody budget chip rendering ──────────────── - function workflowEntry( - overrides: Partial = {}, - ): DialogEntry { + function workflowEntry(overrides: Partial = {}): DialogEntry { const base = { id: 'wf_test1234', kind: 'workflow' as const, @@ -925,4 +942,173 @@ describe('BackgroundTasksDialog', () => { expect(f).toContain('420t'); }); }); + + describe('nested sub-agent display', () => { + // Entries are passed pre-grouped (parent before child) — in production + // useBackgroundTaskView applies reorderChildrenUnderParents before the + // snapshot reaches the dialog; this suite mocks the hook, so grouping + // is the fixture's job and the dialog only owns indent/labels. + it('indents a nested child row with ↳ in the list', () => { + const parent = entry({ + id: 'p', + agentId: 'p', + description: 'parent work', + }); + const child = entry({ + id: 'c', + agentId: 'c', + description: 'child work', + parentAgentId: 'p', + depth: 1, + }); + const h = setup([parent, child]); + h.call(() => h.probe.current!.actions.openDialog()); + const frame = h.lastFrame() ?? ''; + expect(frame).toContain('↳'); + expect(frame.indexOf('parent work')).toBeLessThan( + frame.indexOf('child work'), + ); + }); + + it('tags [blocking] only when the whole chain is foreground', () => { + const bgParent = entry({ + id: 'p', + agentId: 'p', + description: 'bg parent work', + isBackgrounded: true, + }); + const fgChild = entry({ + id: 'c', + agentId: 'c', + description: 'awaited child work', + isBackgrounded: false, + parentAgentId: 'p', + depth: 1, + }); + const fgTop = entry({ + id: 't', + agentId: 't', + description: 'top-level fg work', + isBackgrounded: false, + }); + const h = setup([bgParent, fgChild, fgTop]); + h.call(() => h.probe.current!.actions.openDialog()); + const lines = (h.lastFrame() ?? '').split('\n'); + const childLine = lines.find((l) => l.includes('awaited child')) ?? ''; + const topLine = lines.find((l) => l.includes('top-level fg')) ?? ''; + // The child blocks its background parent, not the user's turn. + expect(childLine).not.toContain('[blocking]'); + // Regression guard: a plain foreground top-level agent keeps the tag. + expect(topLine).toContain('[blocking]'); + }); + + it('shows the level badge and Parent breadcrumb in a nested child detail', () => { + const parent = entry({ + id: 'p', + agentId: 'p', + description: 'parent work', + subagentType: 'researcher', + }); + const child = entry({ + id: 'c', + agentId: 'c', + description: 'child work', + parentAgentId: 'p', + depth: 1, + }); + const h = setup([parent, child]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.moveSelectionDown()); + h.call(() => h.probe.current!.actions.enterDetail()); + const frame = h.lastFrame() ?? ''; + expect(frame).toContain('level 2 of 5'); + expect(frame).toContain('Parent'); + expect(frame).toContain('main › researcher — parent work'); + }); + + it('lists live children in the parent detail Sub-agents section', () => { + const parent = entry({ + id: 'p', + agentId: 'p', + description: 'parent work', + }); + const child = entry({ + id: 'c', + agentId: 'c', + description: 'child work', + parentAgentId: 'p', + depth: 1, + subagentType: 'explore', + }); + const h = setup([parent, child]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + const frame = h.lastFrame() ?? ''; + expect(frame).toContain('Sub-agents'); + expect(frame).toContain('○ explore — child work'); + // The parent itself is top-level: no Parent section, no badge. + expect(frame).not.toContain('level 1 of'); + }); + + it('sorts the Sub-agents roster active-first so the cap cannot hide running children', () => { + // getAll() is insertion-ordered (oldest spawn first). With more than + // five children, the raw order would fill the capped roster with the + // oldest completed children and hide the newest still-running one — + // the row the user most wants to see. Same two-bucket ordering as + // the main list (compareActiveThenTerminal). + const parent = entry({ + id: 'p', + agentId: 'p', + description: 'parent work', + }); + const doneChildren = [1, 2, 3, 4, 5].map((n) => + entry({ + id: `done-${n}`, + agentId: `done-${n}`, + description: `done-${n} work`, + status: 'completed', + startTime: n, + endTime: 100 + n, + parentAgentId: 'p', + depth: 1, + }), + ); + const runningChild = entry({ + id: 'live-6', + agentId: 'live-6', + description: 'live-6 work', + status: 'running', + startTime: 6, + parentAgentId: 'p', + depth: 1, + }); + const h = setup([parent, ...doneChildren, runningChild]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + const frame = h.lastFrame() ?? ''; + expect(frame).toContain('Sub-agents'); + // The running child is visible despite being the 6th by insertion… + expect(frame).toContain('live-6 work'); + // …and the oldest-finished child is the one that fell past the cap. + expect(frame).not.toContain('done-1 work'); + expect(frame).toContain('1 more'); + }); + + it('falls back to the launch-time parent name when the parent is gone', () => { + const orphan = entry({ + id: 'c', + agentId: 'c', + description: 'child work', + parentAgentId: 'gone', + parentName: 'researcher', + depth: 1, + }); + const h = setup([orphan]); + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + const frame = h.lastFrame() ?? ''; + expect(frame).toContain('researcher'); + expect(frame).toContain('no longer running'); + }); + }); }); diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 3bbc23fd1cf..c23c8c20052 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -41,9 +41,17 @@ import { type AgentDialogEntry, type DialogEntry, type DreamDialogEntry, + compareActiveThenTerminal, entryId, } from '../../hooks/useBackgroundTaskView.js'; import { localizeToolDisplayName, t } from '../../../i18n/index.js'; +import { + ancestorChain, + computeAgentTreeInfo, + computeUserBlockingIds, + statusGlyph, + treeRowPrefix, +} from './agent-forest.js'; // `DialogEntry['status']` widens the shell status union with the agent-only // `paused` state, so dialog handlers can switch on a single combined enum. @@ -170,13 +178,16 @@ function terminalStatusPresentation( const FOREGROUND_ROW_PREFIX = '[blocking]'; const SHELL_ROW_PREFIX = '[shell]'; -function rowLabel(entry: DialogEntry): string { +function rowLabel(entry: DialogEntry, userBlocking: boolean): string { switch (entry.kind) { case 'agent': { const label = buildBackgroundEntryLabel(entry, { includePrefix: false }); - const base = entry.isBackgrounded - ? label - : `${FOREGROUND_ROW_PREFIX} ${label}`; + // `[blocking]` warns that cancelling ends the USER's turn. That is + // only true when the whole ancestor chain is foreground — a nested + // foreground child awaited by a background parent blocks that + // parent, not the user. The caller resolves the verdict via + // computeUserBlockingIds. + const base = userBlocking ? `${FOREGROUND_ROW_PREFIX} ${label}` : label; // Flag agents with a parked approval so the user can spot which row to // open from the list without entering each detail view. return entry.pendingApprovals?.length @@ -214,6 +225,35 @@ function rowLabel(entry: DialogEntry): string { } } +/** + * The detail view's Parent line for a nested agent: a breadcrumb of the + * present ancestor chain (`main › researcher — `), rooted at + * '…' when the chain breaks before the top level, or the launch-time + * `parentName` when the immediate parent is already gone. Undefined for + * top-level agents (they get no Parent section). + */ +function formatParentBreadcrumb( + entry: AgentDialogEntry, + lookup: (id: string) => AgentTask | undefined, +): string | undefined { + if (entry.parentAgentId == null) return undefined; + const { chain, terminatedBy } = ancestorChain(entry, lookup); + if (chain.length === 0) { + return `${entry.parentName ?? t('unknown agent')} · ${t('no longer running')}`; + } + // ancestorChain returns nearest-first; the breadcrumb reads root-first. + const rootFirst = [...chain].reverse(); + const crumbs = [ + terminatedBy === 'root' ? 'main' : '…', + ...rootFirst.map((p) => p.subagentType ?? 'agent'), + ].join(' › '); + const immediateParent = chain[0]; + const desc = immediateParent.description + ? ` — ${immediateParent.description}` + : ''; + return `${crumbs}${desc}`; +} + function elapsedFor(entry: { startTime: number; endTime?: number }): string { const elapsedMs = Math.max( 0, @@ -295,6 +335,14 @@ const ListBody: React.FC<{ const hiddenBelow = entries.length - windowEnd; const visible = entries.slice(windowStart, windowEnd); + // Nested-agent affordances, computed over the FULL roster (not the + // window) so indent and the [blocking] verdict don't change as the + // selection scrolls a parent out of view. The entries arrive already + // grouped depth-first (useBackgroundTaskView applies + // reorderChildrenUnderParents), so indentation lines up with position. + const treeInfo = computeAgentTreeInfo(entries); + const blockingIds = computeUserBlockingIds(entries); + return ( @@ -318,13 +366,25 @@ const ListBody: React.FC<{ : terminal ? terminal.labelColor : theme.text.primary; + const treePrefix = + entry.kind === 'agent' + ? treeRowPrefix(entry, treeInfo.get(entry.agentId)) + : ''; return ( {isSelected ? '> ' : ' '} + {treePrefix !== '' && ( + {treePrefix} + )} - {escapeAnsiCtrlCodes(rowLabel(entry))} + {escapeAnsiCtrlCodes( + rowLabel( + entry, + entry.kind === 'agent' && blockingIds.has(entry.agentId), + ), + )} ); @@ -588,6 +648,7 @@ const AgentDetailBody: React.FC<{ maxHeight: number; maxWidth: number; }> = ({ entry, maxHeight, maxWidth }) => { + const config = useConfig(); const title = escapeAnsiCtrlCodes( `${entry.subagentType ?? 'Agent'} \u203A ${buildBackgroundEntryLabel(entry, { includePrefix: false })}`, ); @@ -604,6 +665,34 @@ const AgentDetailBody: React.FC<{ if (entry.stats?.toolUses !== undefined) { dimSubtitleParts.push(formatToolUseCount(entry.stats.toolUses)); } + // Nesting badge: launch depth is 0-based, user-facing levels are 1-based + // (a top-level sub-agent is level 1 and gets no badge). + if ((entry.depth ?? 0) > 0) { + dimSubtitleParts.push( + t('nested \u00B7 level {{level}} of {{max}}', { + level: String((entry.depth ?? 0) + 1), + max: String(config.getMaxSubagentDepth()), + }), + ); + } + + // Parent breadcrumb + live children, resolved from the registry at + // render time (the detail body re-renders on activity ticks, so both + // stay current). Every lookup tolerates eviction: a missing ancestor + // truncates the breadcrumb with '\u2026', a fully-gone parent falls back to + // the launch-time parentName captured at registration. + const registry = config.getBackgroundTaskRegistry(); + const parentLine = formatParentBreadcrumb(entry, (id) => registry.get(id)); + // Same active-first / newest-first ordering as the main roster: getAll() + // is insertion-ordered, so without the sort a parent with more than five + // children would hide its still-running newest child behind its oldest + // completed ones. + const childAgents = registry + .getAll() + .filter((task) => task.parentAgentId === entry.agentId) + .sort(compareActiveThenTerminal); + const visibleChildAgents = childAgents.slice(0, 5); + const hiddenChildCount = childAgents.length - visibleChildAgents.length; // Registry stores activities newest-last; keep that order so the live // row sits at the bottom of the Progress block. Cap at 5 in case the @@ -646,6 +735,52 @@ const AgentDetailBody: React.FC<{ + {parentLine !== undefined && ( + + + + + {t('Parent')} + + + + + {` ${escapeAnsiCtrlCodes(parentLine)}`} + + + + )} + + {visibleChildAgents.length > 0 && ( + + + + + {t('Sub-agents')} + {` (${childAgents.length})`} + + + {visibleChildAgents.map((child) => ( + + + {` ${statusGlyph(child.status)} ${escapeAnsiCtrlCodes( + `${child.subagentType ?? 'agent'} \u2014 ${child.description}`, + )} \u00B7 ${elapsedFor(child)}`} + + + ))} + {hiddenChildCount > 0 && ( + + + {` \u2026 ${t('{{count}} more', { count: String(hiddenChildCount) })}`} + + + )} + + )} + {activities.length > 0 && ( @@ -1320,9 +1455,15 @@ export const BackgroundTasksDialog: React.FC = ({ selectedEntry.kind === 'agent' && selectedEntry.status === 'paused'; if (!isCancelable && !isAbandonable) return; const entryKey = entryId(selectedEntry); - const isForegroundAgent = - selectedEntry.kind === 'agent' && !selectedEntry.isBackgrounded; - if (isForegroundAgent && pendingCancelEntryId !== entryKey) { + // Two-step confirm only when cancelling would end the USER's turn — + // the same chain-aware verdict as the `[blocking]` row prefix. A + // foreground child awaited by a *background* parent unblocks that + // parent, not the user, so it cancels on the first press like any + // background entry. + const isUserBlockingAgent = + selectedEntry.kind === 'agent' && + computeUserBlockingIds(entries).has(selectedEntry.agentId); + if (isUserBlockingAgent && pendingCancelEntryId !== entryKey) { setPendingCancelEntryId(entryKey); return; } diff --git a/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx b/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx index 2308b72c358..3ca663b9aff 100644 --- a/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx +++ b/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx @@ -702,4 +702,64 @@ describe('', () => { expect(lastFrame() ?? '').toContain('researcher'); expect(lastFrame() ?? '').toContain('snapshot-only path'); }); + + describe('nested sub-agent tree', () => { + it('indents a nested child with ↳ beneath its parent', () => { + // Snapshot order is newest-first (child registered after parent); + // the panel renders oldest-first with the child grouped under it. + const parent = agentEntry({ + agentId: 'parent', + description: 'parent work', + }); + const child = agentEntry({ + agentId: 'child', + description: 'child work', + parentAgentId: 'parent', + depth: 1, + }); + const { lastFrame } = renderPanel({ entries: [child, parent] }); + const frame = lastFrame() ?? ''; + expect(frame).toContain('↳ ○ child work'); + expect(frame.indexOf('parent work')).toBeLessThan( + frame.indexOf('child work'), + ); + }); + + it('groups a child under its parent past an interleaving sibling', () => { + // Registration order P, Q, then P's child — newest-first snapshot + // is [child, q, p]. The tree pass must pull the child directly + // beneath P instead of leaving it flat after Q. + const p = agentEntry({ agentId: 'p', description: 'p work' }); + const q = agentEntry({ agentId: 'q', description: 'q work' }); + const child = agentEntry({ + agentId: 'c', + description: 'c work', + parentAgentId: 'p', + depth: 1, + }); + const { lastFrame } = renderPanel({ entries: [child, q, p] }); + const frame = lastFrame() ?? ''; + expect(frame.indexOf('p work')).toBeLessThan(frame.indexOf('c work')); + expect(frame.indexOf('c work')).toBeLessThan(frame.indexOf('q work')); + }); + + it('promotes an orphaned child to root level with a from-parent note', () => { + // Parent already evicted: the child renders at root indent but keeps + // the ↳ marker and says who launched it. + const orphan = agentEntry({ + agentId: 'o', + description: 'orphan work', + parentAgentId: 'gone', + parentName: 'researcher', + depth: 2, + }); + const { lastFrame } = renderPanel({ entries: [orphan] }); + const frame = lastFrame() ?? ''; + expect(frame).toContain('↳ ○ orphan work'); + expect(frame).toContain('· from researcher'); + // Root gutter only (paddingX 2 + prefix 2 = 4 cols) — no tree indent + // despite the launch depth of 2, which would add 4+ more columns. + expect(frame).not.toContain(' ↳'); + }); + }); }); diff --git a/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx b/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx index 454225c6232..ff80d6de685 100644 --- a/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx +++ b/packages/cli/src/ui/components/background-view/LiveAgentPanel.tsx @@ -47,7 +47,17 @@ import type { AgentDialogEntry, DialogEntry, } from '../../hooks/useBackgroundTaskView.js'; -import { isLiveAgentPanelVisibleEntry } from './liveAgentPanelVisibility.js'; +import { + isLiveAgentPanelVisibleEntry, + LIVE_AGENT_PANEL_MAX_ROWS, +} from './liveAgentPanelVisibility.js'; +import { + type AgentTreeInfo, + computeAgentTreeInfo, + panelDisplayOrder, + statusGlyph, + treeRowPrefix, +} from './agent-forest.js'; interface LiveAgentPanelProps { /** @@ -64,7 +74,10 @@ interface LiveAgentPanelProps { width?: number; } -const DEFAULT_MAX_ROWS = 12; +// Sourced from liveAgentPanelVisibility so the composer's panel-focus +// keyboard handler windows its candidate list identically — see the +// constant's doc for the lockstep contract. +const DEFAULT_MAX_ROWS = LIVE_AGENT_PANEL_MAX_ROWS; // Re-export under a panel-local alias so the source of truth stays // in `subagents/builtin-agents.ts` (a backend rename of the default // type would otherwise silently re-introduce the redundant @@ -92,10 +105,9 @@ function isAgentEntry(entry: DialogEntry): entry is AgentDialogEntry { return entry.kind === 'agent'; } -// Bullet glyphs mirror Claude Code's CoordinatorTaskPanel — `○` for -// active slots (running / paused) so the row reads as a uniform list, -// terminal states keep distinct check / cross marks so they're easy -// to scan at a glance. +// Glyph vocabulary lives in agent-forest's statusGlyph (shared with the +// dialog's Sub-agents roster so the two surfaces can't drift); this adds +// the panel's color mapping plus the synthesized-row special case on top. function statusIcon(entry: AgentDialogEntry & { synthesized?: boolean }): { glyph: string; color: string; @@ -106,19 +118,18 @@ function statusIcon(entry: AgentDialogEntry & { synthesized?: boolean }): { // we don't lie about success. return { glyph: '·', color: theme.text.secondary }; } + const glyph = statusGlyph(entry.status); switch (entry.status) { case 'running': - return { glyph: '○', color: theme.status.warning }; case 'paused': - return { glyph: '⏸', color: theme.status.warning }; + case 'cancelled': + return { glyph, color: theme.status.warning }; case 'completed': - return { glyph: '✔', color: theme.status.success }; + return { glyph, color: theme.status.success }; case 'failed': - return { glyph: '✖', color: theme.status.error }; - case 'cancelled': - return { glyph: '✖', color: theme.status.warning }; + return { glyph, color: theme.status.error }; default: - return { glyph: '○', color: theme.text.secondary }; + return { glyph, color: theme.text.secondary }; } } @@ -399,7 +410,14 @@ const LiveAgentPanelBody: React.FC<{ if (visibleAgents.length === 0) return null; - const visibleAgentsAsc = [...visibleAgents].reverse(); + // Snapshot order is newest-first (dialog convention); panelDisplayOrder + // renders oldest-first with each nested agent grouped under its parent + // (and keeps the composer's panel-focus keyboard handler in lockstep). + // Tree metadata is computed on the full visible set (not the maxRows + // window) so a row's indent doesn't shift when the window scrolls past + // its parent. + const visibleAgentsAsc = panelDisplayOrder(visibleAgents); + const treeInfo = computeAgentTreeInfo(visibleAgentsAsc); const overflow = Math.max(0, visibleAgentsAsc.length - maxRows); const visible = overflow > 0 ? visibleAgentsAsc.slice(-maxRows) : visibleAgentsAsc; @@ -430,6 +448,7 @@ const LiveAgentPanelBody: React.FC<{ entry={entry} now={now} selected={focused && clampedIndex === idx + 1} + tree={treeInfo.get(entry.agentId)} /> ))} {focused && ( @@ -447,7 +466,8 @@ const AgentRow: React.FC<{ entry: AgentDialogEntry; now: number; selected?: boolean; -}> = ({ entry, now, selected = false }) => { + tree?: AgentTreeInfo; +}> = ({ entry, now, selected = false, tree }) => { const { glyph, color } = statusIcon(entry); // ANSI sanitize every user-controlled string before it reaches Ink. // `subagentType` comes from subagent config (user-authored or model- @@ -497,6 +517,15 @@ const AgentRow: React.FC<{ // between the description and the right-pinned elapsed. const tail = ` ▶ ${elapsed}${tokenSuffix}`; const prefix = selected ? '▸ ' : ' '; + // Tree gutter (indent + ↳) comes from the shared agent-forest helper so + // the panel and the dialog list can't drift; the orphan additionally + // says who launched it. + const treePrefix = treeRowPrefix(entry, tree); + const orphanNote = tree?.orphaned + ? entry.parentName + ? ` · from ${escapeAnsiCtrlCodes(entry.parentName)}` + : ' · nested' + : ''; return ( @@ -506,6 +535,9 @@ const AgentRow: React.FC<{ + {treePrefix !== '' && ( + {treePrefix} + )} {`${glyph} `} {showType && ( <> @@ -517,6 +549,7 @@ const AgentRow: React.FC<{ {activity && ( {` (${activity})`} )} + {orphanNote && {orphanNote}} diff --git a/packages/cli/src/ui/components/background-view/agent-forest.test.ts b/packages/cli/src/ui/components/background-view/agent-forest.test.ts new file mode 100644 index 00000000000..383bfc02fd7 --- /dev/null +++ b/packages/cli/src/ui/components/background-view/agent-forest.test.ts @@ -0,0 +1,241 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + type AgentForestNode, + computeAgentTreeInfo, + computeUserBlockingIds, + reorderChildrenUnderParents, + treeRowPrefix, +} from './agent-forest.js'; + +interface TestNode extends AgentForestNode { + kind: string; + agentId?: string; + label?: string; +} + +function agent(agentId: string, overrides: Partial = {}): TestNode { + return { kind: 'agent', agentId, ...overrides }; +} + +function idsOf(entries: readonly TestNode[]): Array { + return entries.map((e) => (e.kind === 'agent' ? e.agentId : e.kind)); +} + +describe('reorderChildrenUnderParents', () => { + it('keeps a flat roster unchanged', () => { + const entries = [agent('a'), agent('b'), agent('c')]; + expect(idsOf(reorderChildrenUnderParents(entries))).toEqual([ + 'a', + 'b', + 'c', + ]); + }); + + it('pulls a child from behind an interleaving sibling up under its parent', () => { + // Registration order: P, Q, P-child — the panel bug this feature fixes. + const entries = [ + agent('p'), + agent('q'), + agent('p-child', { parentAgentId: 'p' }), + ]; + expect(idsOf(reorderChildrenUnderParents(entries))).toEqual([ + 'p', + 'p-child', + 'q', + ]); + }); + + it('orders a multi-level tree depth-first with siblings in input order', () => { + const entries = [ + agent('root'), + agent('child-1', { parentAgentId: 'root' }), + agent('other-root'), + agent('grandchild', { parentAgentId: 'child-1' }), + agent('child-2', { parentAgentId: 'root' }), + ]; + expect(idsOf(reorderChildrenUnderParents(entries))).toEqual([ + 'root', + 'child-1', + 'grandchild', + 'child-2', + 'other-root', + ]); + }); + + it('keeps non-agent entries at their exact positions', () => { + const shell: TestNode = { kind: 'shell' }; + const dream: TestNode = { kind: 'dream' }; + const entries = [ + agent('p'), + shell, + agent('q'), + agent('p-child', { parentAgentId: 'p' }), + dream, + ]; + const result = reorderChildrenUnderParents(entries); + expect(idsOf(result)).toEqual(['p', 'shell', 'p-child', 'q', 'dream']); + }); + + it('treats an agent with an evicted parent as a root in input order', () => { + const entries = [ + agent('a'), + agent('orphan', { parentAgentId: 'gone' }), + agent('b'), + ]; + expect(idsOf(reorderChildrenUnderParents(entries))).toEqual([ + 'a', + 'orphan', + 'b', + ]); + }); + + it('does not drop members of a parent cycle', () => { + const entries = [ + agent('x', { parentAgentId: 'y' }), + agent('y', { parentAgentId: 'x' }), + agent('z'), + ]; + const result = reorderChildrenUnderParents(entries); + expect(result).toHaveLength(3); + expect(new Set(idsOf(result))).toEqual(new Set(['x', 'y', 'z'])); + }); + + it('ignores a self-parent reference', () => { + const entries = [agent('a', { parentAgentId: 'a' }), agent('b')]; + expect(idsOf(reorderChildrenUnderParents(entries))).toEqual(['a', 'b']); + }); +}); + +describe('computeAgentTreeInfo', () => { + it('computes visible depth along the present parent chain', () => { + const entries = [ + agent('root'), + agent('child', { parentAgentId: 'root' }), + agent('grandchild', { parentAgentId: 'child' }), + ]; + const info = computeAgentTreeInfo(entries); + expect(info.get('root')).toEqual({ visibleDepth: 0, orphaned: false }); + expect(info.get('child')).toEqual({ visibleDepth: 1, orphaned: false }); + expect(info.get('grandchild')).toEqual({ + visibleDepth: 2, + orphaned: false, + }); + }); + + it('promotes an agent with an evicted parent to visible root and flags it', () => { + const entries = [agent('orphan', { parentAgentId: 'gone' })]; + expect(computeAgentTreeInfo(entries).get('orphan')).toEqual({ + visibleDepth: 0, + orphaned: true, + }); + }); + + it('counts depth up to the nearest missing ancestor only', () => { + // grandparent evicted: parent is an orphaned root, child sits at depth 1. + const entries = [ + agent('parent', { parentAgentId: 'gone' }), + agent('child', { parentAgentId: 'parent' }), + ]; + const info = computeAgentTreeInfo(entries); + expect(info.get('parent')).toEqual({ visibleDepth: 0, orphaned: true }); + expect(info.get('child')).toEqual({ visibleDepth: 1, orphaned: false }); + }); + + it('terminates on parent cycles', () => { + // reorderChildrenUnderParents appends cycle members flat, so their + // depth must be 0 — otherwise they'd indent under no rendered parent. + const entries = [ + agent('x', { parentAgentId: 'y' }), + agent('y', { parentAgentId: 'x' }), + ]; + const info = computeAgentTreeInfo(entries); + expect(info.get('x')).toEqual({ visibleDepth: 0, orphaned: false }); + expect(info.get('y')).toEqual({ visibleDepth: 0, orphaned: false }); + }); +}); + +describe('computeUserBlockingIds', () => { + it('tags a top-level foreground agent', () => { + const entries = [ + agent('fg', { isBackgrounded: false, parentAgentId: null }), + ]; + expect(computeUserBlockingIds(entries)).toEqual(new Set(['fg'])); + }); + + it('never tags background agents', () => { + const entries = [ + agent('bg', { isBackgrounded: true, parentAgentId: null }), + ]; + expect(computeUserBlockingIds(entries).size).toBe(0); + }); + + it('does not tag a foreground child awaited by a background parent', () => { + const entries = [ + agent('bg-parent', { isBackgrounded: true, parentAgentId: null }), + agent('fg-child', { isBackgrounded: false, parentAgentId: 'bg-parent' }), + ]; + expect(computeUserBlockingIds(entries).size).toBe(0); + }); + + it('tags a fully-foreground chain down from the top-level session', () => { + const entries = [ + agent('fg-parent', { isBackgrounded: false, parentAgentId: null }), + agent('fg-child', { isBackgrounded: false, parentAgentId: 'fg-parent' }), + ]; + expect(computeUserBlockingIds(entries)).toEqual( + new Set(['fg-parent', 'fg-child']), + ); + }); + + it('does not tag when an ancestor is missing from the visible set', () => { + const entries = [ + agent('fg-child', { isBackgrounded: false, parentAgentId: 'gone' }), + ]; + expect(computeUserBlockingIds(entries).size).toBe(0); + }); + + it('does not tag members of a parent cycle', () => { + const entries = [ + agent('x', { isBackgrounded: false, parentAgentId: 'y' }), + agent('y', { isBackgrounded: false, parentAgentId: 'x' }), + ]; + expect(computeUserBlockingIds(entries).size).toBe(0); + }); + + it('treats a legacy entry without parentAgentId as top-level', () => { + const entries = [agent('legacy', { isBackgrounded: false })]; + expect(computeUserBlockingIds(entries)).toEqual(new Set(['legacy'])); + }); +}); + +describe('treeRowPrefix', () => { + it('indents by visible depth with a marker on spawned rows', () => { + expect( + treeRowPrefix(agent('root'), { visibleDepth: 0, orphaned: false }), + ).toBe(''); + expect( + treeRowPrefix(agent('child', { parentAgentId: 'root' }), { + visibleDepth: 1, + orphaned: false, + }), + ).toBe(' ↳ '); + }); + + it('clamps the indent at TREE_INDENT_MAX_LEVELS for deep trees', () => { + // Maintainer mutation-testing on #6191 found removing the clamp + // survived the suite. A depth-4 row must indent 3 levels (12 spaces), + // not 4 (16) — the detail view's level badge carries the exact depth. + expect( + treeRowPrefix(agent('deep', { parentAgentId: 'p3' }), { + visibleDepth: 4, + orphaned: false, + }), + ).toBe(' ↳ '); + }); +}); diff --git a/packages/cli/src/ui/components/background-view/agent-forest.ts b/packages/cli/src/ui/components/background-view/agent-forest.ts new file mode 100644 index 00000000000..8e710adaa6d --- /dev/null +++ b/packages/cli/src/ui/components/background-view/agent-forest.ts @@ -0,0 +1,258 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * agent-forest — pure helpers for rendering nested sub-agents as a tree. + * + * Sub-agents may spawn sub-agents (bounded by `maxSubagentDepth`), and every + * agent in the tree registers in the same flat `BackgroundTaskRegistry`. These + * helpers turn that flat roster into a parent-grouped display order plus + * per-row tree metadata, without assuming the full tree is visible: rows + * appear and evict independently (terminal rows leave the LiveAgentPanel + * after a few seconds, registries cap retained terminal entries), so every + * function here treats a missing parent as a normal case, not an error. + * + * The reorder is a post-pass over an already-sorted list on purpose. The + * surfaces sort differently (the dialog sorts active-then-terminal, the + * panel renders oldest-first) and a tree can span those buckets — a running + * parent with a just-completed child, or vice versa. Grouping children under + * whichever position their parent already earned preserves each surface's + * sort semantics for roots while keeping trees contiguous. + */ + +/** + * Minimal structural view of a task entry — satisfied by both `AgentTask` + * (registry entries) and the CLI's `DialogEntry` union, so the helpers can + * be shared without importing either type (and without a type cycle between + * this module and the view-model hook). + */ +export interface AgentForestNode { + kind: string; + agentId?: string; + parentAgentId?: string | null; + isBackgrounded?: boolean; +} + +/** Per-agent tree metadata for row rendering. */ +export interface AgentTreeInfo { + /** + * Structural depth among the *visible* entries (0 = rendered at root + * level). An agent whose ancestors were evicted renders closer to the + * root than its launch depth — the tree indents only what the user can + * actually see, so connectors never dangle. + */ + visibleDepth: number; + /** + * True when the entry claims a parent (`parentAgentId` set) that is not + * in the visible set — the row is promoted to root level and annotated + * ("from ") instead of indented under nothing. + */ + orphaned: boolean; +} + +function isAgentNode( + e: T, +): e is T & { agentId: string } { + return e.kind === 'agent' && typeof e.agentId === 'string'; +} + +/** + * Regroups agent entries so each agent renders directly beneath its parent + * (depth-first), while every non-agent entry keeps its exact position: the + * k-th agent slot in the input is filled by the k-th agent of the grouped + * order. Agents whose parent is absent (top-level, evicted parent, or a + * parent-cycle) keep their original relative order as roots; siblings keep + * their original relative order under their parent. + */ +export function reorderChildrenUnderParents( + entries: readonly T[], +): T[] { + const agents = entries.filter(isAgentNode); + + const byId = new Map(); + for (const agent of agents) byId.set(agent.agentId, agent); + + const childrenOf = new Map>(); + const roots: Array = []; + for (const agent of agents) { + const pid = agent.parentAgentId; + if (pid != null && pid !== agent.agentId && byId.has(pid)) { + const siblings = childrenOf.get(pid); + if (siblings) siblings.push(agent); + else childrenOf.set(pid, [agent]); + } else { + roots.push(agent); + } + } + if (childrenOf.size === 0) return [...entries]; + + const ordered: T[] = []; + const visited = new Set(); + const visit = (node: T & { agentId: string }) => { + if (visited.has(node.agentId)) return; + visited.add(node.agentId); + ordered.push(node); + for (const child of childrenOf.get(node.agentId) ?? []) visit(child); + }; + for (const root of roots) visit(root); + // Parent-cycles never reach a root; append the members in input order + // rather than dropping rows. + for (const agent of agents) { + if (!visited.has(agent.agentId)) { + visited.add(agent.agentId); + ordered.push(agent); + } + } + + let next = 0; + return entries.map((e) => (isAgentNode(e) ? ordered[next++] : e)); +} + +/** + * The LiveAgentPanel's row order: the snapshot arrives newest-first (the + * dialog convention), the panel renders oldest-first, then groups each + * nested agent under its parent. Shared with the composer's panel-focus + * keyboard handler (InputPrompt), which resolves `livePanelSelectedIndex` + * against this list — the two MUST use the same transform or Enter opens + * the wrong agent's detail. + */ +export function panelDisplayOrder( + visibleNewestFirst: readonly T[], +): T[] { + return reorderChildrenUnderParents([...visibleNewestFirst].reverse()); +} + +/** How an ancestor walk ended — see {@link ancestorChain}. */ +export type AncestorTermination = 'root' | 'missing' | 'cycle'; + +/** + * Walks `node`'s parent chain through `lookup`, collecting the ancestors + * that are actually present (immediate parent first). The walk stops — + * without error, per the module contract — at a top-level ancestor + * (`'root'`), at an evicted/unknown parent id (`'missing'`), or on a + * repeated id (`'cycle'`). Single home of the eviction/cycle policy shared + * by tree depth, the `[blocking]` verdict, and the detail-view breadcrumb. + */ +export function ancestorChain( + node: AgentForestNode, + lookup: (id: string) => T | undefined, +): { chain: T[]; terminatedBy: AncestorTermination } { + const chain: T[] = []; + const seen = new Set(node.agentId != null ? [node.agentId] : []); + let pid = node.parentAgentId; + for (;;) { + if (pid == null) return { chain, terminatedBy: 'root' }; + if (seen.has(pid)) return { chain, terminatedBy: 'cycle' }; + const parent = lookup(pid); + if (!parent) return { chain, terminatedBy: 'missing' }; + seen.add(pid); + chain.push(parent); + pid = parent.parentAgentId; + } +} + +/** + * Computes {@link AgentTreeInfo} for every agent entry in the visible set. + * Depth is the length of the parent chain that is actually present. + */ +export function computeAgentTreeInfo( + entries: readonly AgentForestNode[], +): Map { + const byId = new Map(); + for (const e of entries) if (isAgentNode(e)) byId.set(e.agentId, e); + + const info = new Map(); + for (const e of byId.values()) { + const { chain, terminatedBy } = ancestorChain(e, (id) => byId.get(id)); + info.set(e.agentId!, { + // Cycle members are appended flat at root level by + // reorderChildrenUnderParents — mirror that here, otherwise the row + // would indent under no rendered parent. + visibleDepth: terminatedBy === 'cycle' ? 0 : chain.length, + // Orphaned = claims a parent but its IMMEDIATE parent is gone. A + // deeper break (grandparent evicted) still renders indented under + // the present parent, so only a zero-length 'missing' chain counts. + orphaned: chain.length === 0 && terminatedBy === 'missing', + }); + } + return info; +} + +/** + * The agent ids whose cancellation would end the USER's current turn — a + * foreground entry whose entire ancestor chain is foreground up to the + * top-level session. A foreground child awaited by a *background* parent + * blocks that parent, not the user, so it is not tagged; the dialog's + * `[blocking]` warning exists solely to flag "cancelling this ends your + * turn". When the chain cannot be proven (evicted ancestor, cycle), the + * entry is not tagged — a missing warning is a milder failure than a wrong + * one. + */ +export function computeUserBlockingIds( + entries: readonly AgentForestNode[], +): Set { + const byId = new Map(); + for (const e of entries) if (isAgentNode(e)) byId.set(e.agentId, e); + + const blocking = new Set(); + for (const e of byId.values()) { + if (e.isBackgrounded !== false) continue; + const { chain, terminatedBy } = ancestorChain(e, (id) => byId.get(id)); + if ( + terminatedBy === 'root' && + chain.every((ancestor) => ancestor.isBackgrounded === false) + ) { + blocking.add(e.agentId!); + } + } + return blocking; +} + +// ─── Row presentation shared by LiveAgentPanel and BackgroundTasksDialog ─── + +// Indent per nesting level, clamped so deep trees don't starve the +// description column on narrow terminals; the detail dialog's level badge +// carries the exact depth beyond the clamp. +export const TREE_INDENT_PER_LEVEL = ' '; +export const TREE_INDENT_MAX_LEVELS = 3; + +/** + * The tree gutter rendered before a row's status glyph: indent by visible + * depth plus a `↳` marker on any row spawned by another agent. The marker + * is kept even for orphans (parent already evicted, depth back at 0) so + * "this was a nested agent" stays legible. + */ +export function treeRowPrefix( + entry: AgentForestNode, + tree: AgentTreeInfo | undefined, +): string { + const indent = TREE_INDENT_PER_LEVEL.repeat( + Math.min(tree?.visibleDepth ?? 0, TREE_INDENT_MAX_LEVELS), + ); + const marker = entry.parentAgentId != null ? '↳ ' : ''; + return `${indent}${marker}`; +} + +/** + * Status → glyph vocabulary shared by the panel rows and the detail view's + * Sub-agents roster: `○` for active slots (running keeps the list visually + * uniform), distinct marks for terminal states, `⏸` for paused. + */ +export function statusGlyph(status: string): string { + switch (status) { + case 'running': + return '○'; + case 'paused': + return '⏸'; + case 'completed': + return '✔'; + case 'failed': + case 'cancelled': + return '✖'; + default: + return '○'; + } +} diff --git a/packages/cli/src/ui/components/background-view/liveAgentPanelVisibility.ts b/packages/cli/src/ui/components/background-view/liveAgentPanelVisibility.ts index 22895542232..2cd3d4021b1 100644 --- a/packages/cli/src/ui/components/background-view/liveAgentPanelVisibility.ts +++ b/packages/cli/src/ui/components/background-view/liveAgentPanelVisibility.ts @@ -13,6 +13,15 @@ import type { // must be something the live panel can actually render. export const TERMINAL_VISIBLE_MS = 8000; +// Row budget the panel renders (it windows to the LAST rows of its +// display-ordered list when the roster outgrows this). Shared with the +// composer's panel-focus keyboard handler, which slices its candidate +// list by the same amount so `livePanelSelectedIndex` always addresses +// a row that is actually on screen. No caller overrides the panel's +// `maxRows` prop today; if one ever does, it must thread the same value +// to the keyboard handler. +export const LIVE_AGENT_PANEL_MAX_ROWS = 12; + export function isLiveAgentPanelVisibleEntry( entry: DialogEntry, nowMs: number, diff --git a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx index b10037e13d6..11123040c02 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.test.tsx @@ -429,6 +429,7 @@ describe('', () => { status: 'running' | 'completed' | 'failed' | 'cancelled'; pendingConfirmation?: object; terminateReason?: string; + executionSummary?: object; }; isFocused?: boolean; isPending?: boolean; @@ -501,6 +502,96 @@ describe('', () => { expect(output).not.toContain('MockApprovalPrompt'); }); + it('counts successful agent-tool calls as sub-agents in the summary tail', () => { + // Direct children = successful AgentTool calls from the per-tool + // usage stats. The failed call (a guard-blocked spawn) must not + // count. Stats key on the raw request name, so the legacy 'task' + // alias must count alongside the canonical 'agent'. + const { lastFrame } = renderWithContext( + , + StreamingState.Idle, + ); + const output = lastFrame() ?? ''; + expect(output).toContain('7 tools'); + expect(output).toContain('3 sub-agents'); + }); + + it('renders no sub-agent segment when the agent spawned none', () => { + const { lastFrame } = renderWithContext( + , + StreamingState.Idle, + ); + const output = lastFrame() ?? ''; + expect(output).toContain('2 tools'); + expect(output).not.toContain('sub-agent'); + }); + it('live (`isPending`) terminal subagent → renders summary inline (panel snapshot already dropped)', () => { // After `unregisterForeground`'s post-delete emit (#3921 swap- // order), the panel snapshot drops the foreground entry as soon diff --git a/packages/cli/src/ui/components/messages/ToolMessage.tsx b/packages/cli/src/ui/components/messages/ToolMessage.tsx index 52ccdffae6d..ba453b237e2 100644 --- a/packages/cli/src/ui/components/messages/ToolMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolMessage.tsx @@ -24,6 +24,7 @@ import type { McpToolProgressData, FileDiff, } from '@qwen-code/qwen-code-core'; +import { ToolNames, ToolNamesMigration } from '@qwen-code/qwen-code-core'; import { ToolConfirmationMessage } from './ToolConfirmationMessage.js'; import { PlanSummaryDisplay } from '../PlanSummaryDisplay.js'; import { ShellInputPrompt } from '../ShellInputPrompt.js'; @@ -47,6 +48,17 @@ import { } from '../shared/ToolStatusIndicator.js'; import { ToolElapsedTime } from '../shared/ToolElapsedTime.js'; +// Names that resolve to the agent tool: the canonical name plus whatever +// legacy request aliases core's migration map declares (e.g. 'task'). +// Tool-usage stats key on the raw request name, so the scrollback +// sub-agent count must accept all of them. +const AGENT_TOOL_NAMES: ReadonlySet = new Set([ + ToolNames.AGENT, + ...Object.entries(ToolNamesMigration) + .filter(([, canonical]) => canonical === ToolNames.AGENT) + .map(([legacy]) => legacy), +]); + const STATIC_HEIGHT = 1; const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc. const MIN_LINES_SHOWN = 2; // show at least this many lines @@ -396,6 +408,16 @@ const SubagentScrollbackSummary: React.FC<{ `${stats.totalToolCalls} tool${stats.totalToolCalls === 1 ? '' : 's'}`, ); } + // Direct children this agent spawned = its successful AgentTool calls + // (per-tool usage already rides in executionSummary — no extra + // plumbing). Blocked spawns (depth/fork guards) return an error result + // and land in `failure`, so they don't count. + const subagentSpawns = (stats?.toolUsage ?? []) + .filter((tu) => AGENT_TOOL_NAMES.has(tu.name)) + .reduce((sum, tu) => sum + tu.success, 0); + if (subagentSpawns > 0) { + parts.push(`${subagentSpawns} sub-agent${subagentSpawns === 1 ? '' : 's'}`); + } if (stats?.totalDurationMs !== undefined) { parts.push( formatDuration(stats.totalDurationMs, { hideTrailingZeros: true }), diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts index 7f1bad68393..fdd1479a844 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts @@ -35,6 +35,7 @@ import { type TaskState, type WorkflowTask, } from '@qwen-code/qwen-code-core'; +import { reorderChildrenUnderParents } from '../components/background-view/agent-forest.js'; // Cap on retained terminal dream entries surfaced via the dialog. // `MemoryManager.tasks` has no eviction; without this cap the list @@ -108,6 +109,42 @@ export interface UseBackgroundTaskViewResult { entries: readonly DialogEntry[]; } +/** + * Two-bucket roster ordering so "new OR running tasks should appear at the + * top" (the literal phrasing of the issue this view-model serves). A pure + * startTime DESC sort surfaces the newest LAUNCH but lets an older + * long-running / paused entry fall below a batch of newer terminal entries + * — the user opens the dialog wanting to check the running work, and finds + * it buried under noise. + * + * bucket 1 — active (running + paused), sorted by startTime DESC so the + * most recent launch sits at the very top. + * bucket 2 — terminal (completed / failed / cancelled), sorted by + * endTime DESC so the most recently FINISHED entry is the + * first terminal row (matches "what changed while I wasn't + * looking" intuition; startTime would put a long-running task + * that just settled below an old quick task that finished + * hours ago). + * + * Shared by the dialog snapshot and the detail view's Sub-agents roster so + * a capped child list can't hide a running child behind older completed + * siblings. + */ +export function compareActiveThenTerminal( + a: { status: string; startTime: number; endTime?: number }, + b: { status: string; startTime: number; endTime?: number }, +): number { + const aActive = a.status === 'running' || a.status === 'paused'; + const bActive = b.status === 'running' || b.status === 'paused'; + if (aActive !== bActive) return aActive ? -1 : 1; + if (aActive) return b.startTime - a.startTime; + // Terminal bucket: fall back to startTime when an entry has no endTime + // (defensive — the registries stamp endTime on every running → terminal + // transition, so this only matters for synthetic / partially-restored + // entries). + return (b.endTime ?? b.startTime) - (a.endTime ?? a.startTime); +} + /** Stable id of an entry regardless of kind — used as React key + lookup. */ export function entryId(entry: DialogEntry): string { switch (entry.kind) { @@ -225,45 +262,18 @@ export function useBackgroundTaskView( : undefined, }; }); - // Two-bucket merge so "new OR running tasks should appear at the - // top" (the literal phrasing of the issue this view-model serves). - // A pure startTime DESC sort surfaces the newest LAUNCH but lets - // an older long-running / paused entry fall below a batch of - // newer terminal entries — the user opens the dialog wanting to - // check the running work, and finds it buried under noise. - // - // bucket 1 — active (running + paused), sorted by startTime DESC - // so the most recent launch sits at the very top. - // bucket 2 — terminal (completed / failed / cancelled), sorted - // by endTime DESC so the most recently FINISHED entry - // is the first terminal row (matches "what changed - // while I wasn't looking" intuition; startTime would - // put a long-running task that just settled below an - // old quick task that finished hours ago). - // - // Entries falling out the bottom of bucket 2 are eventually - // pruned by each registry's terminal-entry cap (see + // Two-bucket merge — see compareActiveThenTerminal for the full + // rationale. Entries falling out the bottom of the terminal bucket + // are eventually pruned by each registry's terminal-entry cap (see // `MAX_RETAINED_TERMINAL_AGENTS` / `MAX_RETAINED_TERMINAL_SHELLS` // / `MAX_RETAINED_TERMINAL_MONITORS`). - const isActive = (entry: DialogEntry): boolean => - entry.status === 'running' || entry.status === 'paused'; const merged = [ ...agentEntries, ...shellEntries, ...monitorEntries, ...workflowEntries, ...dreamEntries, - ].sort((a, b) => { - const aActive = isActive(a); - const bActive = isActive(b); - if (aActive !== bActive) return aActive ? -1 : 1; - if (aActive) return b.startTime - a.startTime; - // Terminal bucket: fall back to startTime when an entry has no - // endTime yet (defensive — the registries stamp endTime on - // every running → terminal transition, so this only matters - // for synthetic / partially-restored entries). - return (b.endTime ?? b.startTime) - (a.endTime ?? a.startTime); - }); + ].sort(compareActiveThenTerminal); // Cache the dream signature derived from the freshly-built // entries — the memory listener uses this to skip redundant // setEntries calls when an extract notify fires (extract has no @@ -271,7 +281,12 @@ export function useBackgroundTaskView( // from the same `allDreams` snapshot used to build dreamEntries // so the gate value can never desync from what's on screen. lastDreamSig = computeDreamSig(allDreams); - setEntries(merged); + // Nested sub-agents: group each agent under its parent (depth-first) + // while non-agent entries and root agents keep the bucket positions + // assigned above. A tree can legitimately span the two buckets (a + // running parent with a just-completed child), so grouping must be a + // post-pass on the sorted list rather than part of the comparator. + setEntries(reorderChildrenUnderParents(merged)); }; // Wrap registry callbacks in a thunk so React's setStatusChange diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index b1471b1f269..e28f507d0c4 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -444,6 +444,12 @@ export class BackgroundAgentResumeService { error: meta.lastError === resumeBlockedReason ? undefined : meta.lastError, resumeBlockedReason, + // Restore nesting lineage from the sidecar so a restart-recovered + // nested agent keeps its place in the tree display. parentName is + // not persisted (the parent is usually gone after a restart); the + // UI falls back to its generic orphan annotation. + parentAgentId: meta.parentAgentId, + depth: meta.depth, }; const entry = registry.register(registration); recovered.push(entry); diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index a34d798ecdf..3e42eb25b80 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -74,6 +74,50 @@ describe('BackgroundTaskRegistry', () => { expect(registry.get('test-1')).toBe(entry); }); + it('resolves parentName from the registered parent at registration time', () => { + registry.register({ + agentId: 'parent-1', + description: 'parent agent', + subagentType: 'researcher', + status: 'running', + startTime: Date.now(), + abortController: new AbortController(), + isBackgrounded: true, + outputFile: '/tmp/parent.jsonl', + }); + + const child = registry.register({ + agentId: 'child-1', + description: 'child agent', + status: 'running', + startTime: Date.now(), + abortController: new AbortController(), + isBackgrounded: false, + outputFile: '/tmp/child.jsonl', + parentAgentId: 'parent-1', + depth: 1, + }); + + // Captured eagerly so the UI's orphan annotation survives the + // parent's later eviction. + expect(child.parentName).toBe('researcher'); + + // Unknown parent (e.g. restart-resume of a nested agent whose parent + // is gone): parentName stays undefined, no throw. + const orphan = registry.register({ + agentId: 'orphan-1', + description: 'orphan agent', + status: 'running', + startTime: Date.now(), + abortController: new AbortController(), + isBackgrounded: true, + outputFile: '/tmp/orphan.jsonl', + parentAgentId: 'gone', + depth: 2, + }); + expect(orphan.parentName).toBeUndefined(); + }); + it('completes a background agent and sends notification', () => { const callback = vi.fn(); registry.setNotificationCallback(callback); diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index c070b1a3528..b069cd2b00b 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -228,6 +228,25 @@ export interface AgentTask extends TaskBase { */ agentId: string; subagentType?: string; + /** + * AgentId of the sub-agent that spawned this one; null when launched + * from the top-level session. Drives the nested-agent tree display in + * the LiveAgentPanel and BackgroundTasksDialog. Mirrors + * `AgentMeta.parentAgentId`. + */ + parentAgentId?: string | null; + /** + * Display name (`subagentType`) of the spawning sub-agent, captured at + * registration time. Display-only: lets the orphan annotation + * ("· from ") survive the parent's eviction from the registry. + */ + parentName?: string; + /** + * Launch depth (0-based; 0 = spawned by the top-level session). Same + * value as `AgentMeta.depth` / `childLaunchDepth()`. User-facing level + * = depth + 1. + */ + depth?: number; /** * True if the task is running asynchronously (parent has moved on, the * task persists across turns and emits a terminal XML notification). @@ -432,6 +451,14 @@ export class BackgroundTaskRegistry { ? ((registration as AgentTask).notified ?? false) : false; entry.pendingMessages = registration.pendingMessages ?? []; + // Resolve the parent's display name at registration time — before the + // parent can evict — so the UI's orphan annotation survives it. Owned + // here rather than at call sites so every registration path that + // carries a parentAgentId (spawn, resume, future flavors) gets it + // without remembering to. A caller-provided name wins. + if (entry.parentName === undefined && entry.parentAgentId != null) { + entry.parentName = this.agents.get(entry.parentAgentId)?.subagentType; + } this.agents.set(entry.agentId, entry); debugLogger.info(`Registered background agent: ${entry.agentId}`); diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 6a814eb2acd..8fef74e8835 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -765,6 +765,16 @@ describe('AgentTool', () => { terminateReason: 'Nesting depth limit reached (max 1)', }); expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); + // A blocked spawn must take the scheduler's failure path: `error` keeps + // tool-usage stats from counting it as a spawned sub-agent (and fires + // failure-path hooks), and the display row reports it as failed. The + // scheduler sends only `error.message` to the model (llmContent is + // discarded on the failure path), so the message must carry the full + // actionable guidance, not just the terse reason label. + expect(result.error?.message).toContain('nesting depth limit reached'); + expect(result.error?.message).toContain('Complete this task directly'); + const display = result.returnDisplay as AgentResultDisplay; + expect(display.status).toBe('failed'); }); it('allows a spawn from the top-level session at maxSubagentDepth=1', async () => { @@ -851,6 +861,14 @@ describe('AgentTool', () => { 'Cannot spawn sub-agents from within a fork', ); expect(mockSubagentManager.loadSubagent).not.toHaveBeenCalled(); + // Same failure-path contract as the depth guard above: the model-facing + // message keeps the actionable instruction. + expect(result.error?.message).toContain( + 'Cannot spawn sub-agents from within a fork', + ); + expect(result.error?.message).toContain('execute tasks directly'); + const display = result.returnDisplay as AgentResultDisplay; + expect(display.status).toBe('failed'); }); it('allows nesting while depth remains under the cap', async () => { @@ -1785,6 +1803,39 @@ describe('AgentTool', () => { expect(AgentHeadless.create).not.toHaveBeenCalled(); }); + it('falls back to general-purpose when a nested sub-agent requests a fork', async () => { + // v1: fork is a top-level-only capability. A sub-agent — which may + // carry the AgentTool via nesting — that requests a fork downgrades to + // the awaitable general-purpose subagent instead of opening a nested + // fork, even in interactive mode where fork is otherwise enabled. + const mockLoadedSubagent: SubagentConfig = { + name: 'general-purpose', + description: 'General-purpose agent', + systemPrompt: 'You are a general-purpose agent.', + level: 'builtin', + filePath: '', + }; + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue( + mockLoadedSubagent, + ); + + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation({ + description: 'fork task', + prompt: 'do the thing', + subagent_type: 'fork', + }); + // One agent frame → the invoker is a level-1 sub-agent, not the + // top-level session, so the fork request must take the fallback path. + await runWithAgentContext('parent-sub', () => invocation.execute()); + + expect(mockSubagentManager.loadSubagent).toHaveBeenCalledWith( + 'general-purpose', + ); + expect(AgentHeadless.create).not.toHaveBeenCalled(); + }); + it('should call AgentHeadless.create directly and execute without options', async () => { const params: AgentParams = { description: 'fork task', diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 15d5cc0888e..412ace47ecb 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -1771,6 +1771,17 @@ class AgentToolInvocation extends BaseToolInvocation { ): ToolResult { return { llmContent, + // `error` marks the call failed in the scheduler, so tool-usage stats + // record a failure — a blocked spawn must not count as a spawned + // sub-agent (the scrollback summary derives its sub-agent count from + // the AgentTool's success count). This deliberately routes the denial + // through the scheduler's failure path (error-formatted model + // response, failure-path hooks): a blocked spawn IS a failed tool call. + // The failure path sends ONLY `error.message` to the model and the + // scrollback (`llmContent` is discarded there), so the message must be + // the full guidance text — the terse `terminateReason` would strip the + // "do the task yourself instead" instruction and invite retry loops. + error: { message: llmContent }, returnDisplay: { type: 'task_execution' as const, subagentName: @@ -2348,7 +2359,11 @@ class AgentToolInvocation extends BaseToolInvocation { const agentIdSuffix = this.callId ?? randomUUID().slice(0, 8); const hookOpts = { agentId: `${subagentConfig.name}-${agentIdSuffix}`, - agentType: this.params.subagent_type || subagentConfig.name, + // Resolved config name, not the raw requested type: a fork request + // that fell back to the awaitable path (nested / non-interactive) + // must report the agent that actually runs — hooks, spans, task + // rows, and the meta sidecar all read this field. + agentType: subagentConfig.name, resolvedMode, signal, updateOutput, @@ -2525,6 +2540,10 @@ class AgentToolInvocation extends BaseToolInvocation { prompt: this.params.prompt, outputFile: jsonlPath, metaPath, + // Nested-agent lineage (mirrors the meta sidecar); register() + // resolves the parent's display name from parentAgentId. + parentAgentId: getCurrentAgentId(), + depth: childLaunchDepth(), }); } catch (error) { const errorMessage = @@ -3169,6 +3188,10 @@ class AgentToolInvocation extends BaseToolInvocation { toolUseId: this.callId, outputFile: fgJsonlPath, metaPath: fgMetaPath, + // Nested-agent lineage (mirrors the meta sidecar); register() + // resolves the parent's display name from parentAgentId. + parentAgentId: getCurrentAgentId(), + depth: childLaunchDepth(), }); writeAgentMeta(fgMetaPath, { agentId: hookOpts.agentId,