From b370a4b17bb9a8c7872d6c83693cbb23aa89e7ba Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 23 Aug 2026 17:57:43 +0800 Subject: [PATCH 1/7] fix(core): surface nested sub-agent approvals under background parents A tool call needing confirmation inside a nested sub-agent (launched by a background agent or fork) was neither surfaced nor denied: the prompt- avoidance policy was stamped on an Object.create wrapper while the rebuilt tool registry binds to agentConfig, so nested schedulers resolved Config.prototype's false and believed they could prompt; and the nested invocation's emitter was never bridged, so TOOL_WAITING_APPROVAL fired with no listener and the call waited forever - the enclosing agent hung silently. - Stamp getShouldAvoidPermissionPrompts on agentConfig itself (launch and resume paths) so nested launches inherit the real policy through their config prototype chains: hang-forever becomes an explicit deny when bubbling is off. - Bridge nested foreground launches' approval events onto the nearest backgrounded running ancestor's Background-tasks entry (walking the registry's parentAgentId lineage), so they park where the ancestor's own approvals go and the user can answer them from the dialog. - Mark bridged approvals with the nested runtime's subagentId (declared by the bridge caller via nestedSource - runtime ids and registry ids use different suffixes so comparing them cannot work) and show the waiter in the Background tasks dialog. Fixes #9782 --- .../background-view/BackgroundTasksDialog.tsx | 11 ++ .../agents/background-agent-resume.test.ts | 89 +++++++++ .../src/agents/background-agent-resume.ts | 11 +- .../core/src/agents/background-tasks.test.ts | 48 +++++ packages/core/src/agents/background-tasks.ts | 19 ++ packages/core/src/tools/agent/agent.test.ts | 187 ++++++++++++++++++ packages/core/src/tools/agent/agent.ts | 70 ++++++- 7 files changed, 430 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 3a64ac37a76..feedf604f2e 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -1947,6 +1947,17 @@ export const BackgroundTasksDialog: React.FC = ({ ? `[workflow] ${t('needs approval')}` : t('Background agent needs approval')} + {/* subagentId is set only on approvals bridged from a NESTED + agent onto this entry (see AgentTool's nested approval + bridge). Name the actual waiter so the user knows which of + the descendants is blocked. */} + {selectedApproval?.kind === 'agent' && + selectedApproval.approval.subagentId !== undefined && ( + + {t('from nested agent')}:{' '} + {selectedApproval.approval.subagentId} + + )} { expect(subagentManager.loadSubagent).toHaveBeenCalledWith('researcher'); }); + it('stamps the resumed prompt-avoidance policy where nested launches inherit it', async () => { + // The policy must sit on the config the rebuilt tool registry binds to + // (the createToolRegistry receiver) — not on a wrapper above it — so a + // nested AgentTool launched by the resumed agent inherits it through its + // own config prototype chain. Mirrors the launch path in agent.ts. + const sessionId = 'session-policy'; + const agentId = 'agent-policy'; + const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); + const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); + + writeAgentMeta(metaPath, { + agentId, + agentType: 'researcher', + description: 'Resume policy stamp', + parentSessionId: sessionId, + parentAgentId: null, + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + resolvedApprovalMode: 'auto-edit', + }); + 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 policy stamp' }] }, + }) + '\n', + 'utf8', + ); + + registry.register({ + agentId, + description: 'Resume policy stamp', + subagentType: 'researcher', + isBackgrounded: true, + status: 'paused', + startTime: Date.now(), + abortController: new AbortController(), + prompt: 'Resume policy stamp', + outputFile, + metaPath, + }); + + const subagent = { + execute: vi.fn(async () => {}), + setExternalMessageProvider: vi.fn(), + getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), + getExecutionSummary: () => ({ + totalTokens: 0, + outputTokens: 0, + totalDurationMs: 0, + }), + getTerminateMode: () => AgentTerminateMode.GOAL, + getFinalText: () => 'done', + }; + + const { service, subagentManager } = createService(); + subagentManager.createAgentHeadless.mockResolvedValue({ + subagent, + dispose: vi.fn().mockResolvedValue(undefined), + }); + + const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); + expect(resumed).toBeDefined(); + + const createCall = subagentManager.createAgentHeadless.mock.calls.at(-1); + expect(createCall).toBeDefined(); + const bgConfig = createCall![1] as Config; + const contexts = vi.mocked(bgConfig.createToolRegistry).mock.contexts; + // The runtime config IS the createToolRegistry receiver — the config + // whose rebuilt registry a nested AgentTool binds to. A wrapper-only + // stamp would leave these two distinct objects. + expect(contexts[contexts.length - 1]).toBe(bgConfig); + // Non-interactive harness → auto-deny, visible on the tool-bound config + // itself and through any derived config's prototype chain. + expect(bgConfig.getShouldAvoidPermissionPrompts()).toBe(true); + expect( + (Object.create(bgConfig) as Config).getShouldAvoidPermissionPrompts(), + ).toBe(true); + + await vi.waitFor(() => { + expect(registry.get(agentId)?.status).toBe('completed'); + }); + }); + it('fires SubagentStart hooks when resuming and injects hook context', async () => { const sessionId = 'session-resume'; const agentId = 'agent-resume'; diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index 0cb54e3ca3b..56307b52cd2 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -922,8 +922,15 @@ export class BackgroundAgentResumeService { target.subagentConfig?.approvalMode === BUBBLE_APPROVAL_MODE && this.config.isInteractive(), ); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const bgConfig = Object.create(activeAgentConfig) as any; + // Stamp the policy on activeAgentConfig itself — the config whose + // rebuilt tool registry a nested AgentTool binds to — so nested + // launches under the resumed agent inherit it through their own + // config prototype chains. Mirrors the launch path (agent.ts); a + // wrapper-only stamp left nested schedulers resolving + // Config.prototype's `false` and hanging on unanswerable approvals. + // activeAgentConfig is created per-resume by + // createApprovalModeOverride, so the stamp leaks nowhere. + const bgConfig = activeAgentConfig; bgConfig.getShouldAvoidPermissionPrompts = () => !shouldBubble; const records = await jsonl.read(outputFile); diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index 29a4a3085cf..25187a00edb 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -2657,6 +2657,54 @@ describe('BackgroundTaskRegistry', () => { expect(registry.getPendingApprovals('bg-appr-8')).toHaveLength(0); }); + it('stamps subagentId on bridged approvals only for a nestedSource bridge', () => { + // A nested agent's approvals are parked on its backgrounded ancestor's + // entry; the UI needs to name the actual waiter. The entry's OWN + // approvals must stay unstamped — the runtime subagentId and the + // registry agentId use different suffixes, so the bridge caller + // declares the nested case rather than anyone comparing ids. + const makeWaiting = (callId: string, subagentId: string) => ({ + subagentId, + round: 1, + callId, + name: 'Shell', + description: `run ${callId}`, + args: {}, + confirmationDetails: { + type: 'exec', + } as BackgroundApproval['confirmationDetails'], + respond: vi.fn(async () => {}), + timestamp: Date.now(), + }); + + registry.register(makeRegistration('bg-appr-nested')); + + const ownEmitter = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-appr-nested', ownEmitter); + ownEmitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaiting('own-1', 'fork-runtime1'), + ); + + const nestedEmitter = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-appr-nested', nestedEmitter, { + nestedSource: true, + }); + nestedEmitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaiting('nested-1', 'review-agent-abc123'), + ); + + const parked = registry.getPendingApprovals('bg-appr-nested'); + expect(parked).toHaveLength(2); + expect(parked.find((a) => a.callId === 'own-1')?.subagentId).toBe( + undefined, + ); + expect(parked.find((a) => a.callId === 'nested-1')?.subagentId).toBe( + 'review-agent-abc123', + ); + }); + it('cancel() rejects parked approvals before the abort-driven clear (production ordering)', () => { // In production, abort() synchronously unwinds the agent's awaiting // tool batch, which emits a synthetic TOOL_RESULT for the parked call; diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index 333a9f35338..f0bc3c90abf 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -240,6 +240,16 @@ export interface BackgroundApproval { respond: AgentApprovalRequestEvent['respond']; /** Emission timestamp (ms) — newest-first ordering in the UI. */ at: number; + /** + * Set ONLY for approvals bridged from a NESTED agent onto this entry + * (see AgentTool's nested approval bridge): the nested runtime's + * subagentId (`-`), so the UI can say which descendant is + * actually waiting. Undefined for the entry's own approvals — the + * runtime id and the registry agentId use different suffixes, so + * comparing them cannot distinguish own from nested; the bridge caller + * declares it instead. + */ + subagentId?: string; } /** @@ -1141,6 +1151,14 @@ export class BackgroundTaskRegistry { bridgeApprovalEvents( agentId: string, emitter: AgentEventEmitter, + options?: { + /** + * The emitter belongs to a NESTED agent whose approvals are parked + * on this (ancestor) entry. Stamps each parked approval with the + * event's subagentId so the UI can name the actual waiter. + */ + nestedSource?: boolean; + }, ): () => void { const onWaiting = (event: AgentApprovalRequestEvent) => { const parked = this.addPendingApproval(agentId, { @@ -1150,6 +1168,7 @@ export class BackgroundTaskRegistry { confirmationDetails: event.confirmationDetails, respond: event.respond, at: event.timestamp, + ...(options?.nestedSource ? { subagentId: event.subagentId } : {}), }); // If the entry is already gone/terminal we couldn't park it — reject // so the agent's reasoning loop doesn't block forever on this call. diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 2bc85673788..8f62538936a 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { AgentTool, type AgentParams, + findBackgroundedAncestorAgentId, resolveSubagentApprovalMode, } from './agent.js'; import type { Content, Part, PartListUnion } from '@google/genai'; @@ -2244,6 +2245,62 @@ describe('AgentTool', () => { } }); + it('bridges nested approvals to the nearest backgrounded ancestor entry', async () => { + // A foreground launch from inside a background agent (fork) has no + // inline UI: its TOOL_WAITING_APPROVAL fires on the invocation's own + // emitter. The launch path must bridge that emitter onto the + // ancestor's Background-tasks entry (marked nestedSource so the UI + // can name the waiter) and unbridge on completion. + const registry = config.getBackgroundTaskRegistry() as unknown as { + get: ReturnType; + bridgeApprovalEvents: ReturnType; + }; + const cleanupSpy = vi.fn(); + registry.bridgeApprovalEvents.mockReturnValue(cleanupSpy); + registry.get.mockImplementation((id: string) => + id === 'fork-entry' + ? { isBackgrounded: true, status: 'running' } + : undefined, + ); + + const invocation = agentTool.build({ + description: 'Search files', + prompt: 'Find all TypeScript files', + subagent_type: 'file-search', + }); + await runWithAgentContext('fork-entry', () => + invocation.execute(new AbortController().signal), + ); + + expect(registry.bridgeApprovalEvents).toHaveBeenCalledTimes(1); + const [ownerId, emitter, opts] = + registry.bridgeApprovalEvents.mock.calls[0]; + expect(ownerId).toBe('fork-entry'); + expect(emitter).toBe( + (invocation as unknown as { eventEmitter: unknown }).eventEmitter, + ); + expect(opts).toEqual({ nestedSource: true }); + // Unbridged in the foreground finally — no listener leak. + expect(cleanupSpy).toHaveBeenCalledTimes(1); + }); + + it('does not bridge approvals for a top-level foreground launch', async () => { + // No agent-context frame → no backgrounded ancestor → the inline + // confirmation path applies unchanged. + const registry = config.getBackgroundTaskRegistry() as unknown as { + bridgeApprovalEvents: ReturnType; + }; + + const invocation = agentTool.build({ + description: 'Search files', + prompt: 'Find all TypeScript files', + subagent_type: 'file-search', + }); + await invocation.execute(new AbortController().signal); + + expect(registry.bridgeApprovalEvents).not.toHaveBeenCalled(); + }); + it('strips internal analysis and summary tags from subagent result', async () => { vi.mocked(mockAgent.getFinalText).mockReturnValue( [ @@ -6960,6 +7017,62 @@ describe('AgentTool', () => { expect(createdConfig.getShouldAvoidPermissionPrompts()).toBe(true); }); + it('stamps the prompt-avoidance policy where nested launches inherit it', async () => { + // The policy must sit on the config the rebuilt tool registry binds to + // (the createToolRegistry receiver), not on a wrapper above it: a + // nested AgentTool branches its own configs off that receiver via + // Object.create, and a wrapper-only stamp left nested schedulers + // resolving Config.prototype's `false` — believing they could prompt + // and waiting forever on approvals nobody could see. + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue(bgSubagent); + + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation({ + description: 'Start monitor', + prompt: 'Watch for changes', + subagent_type: 'monitor', + }); + await invocation.execute(); + + const contexts = vi.mocked(config.createToolRegistry).mock.contexts; + const toolBoundConfig = contexts[contexts.length - 1] as Config; + // Auto-deny background agent (non-interactive session): the policy is + // visible on the tool-bound config itself... + expect(toolBoundConfig.getShouldAvoidPermissionPrompts()).toBe(true); + // ...and through the prototype chain of any config a nested launch + // derives from it. + const derived = Object.create(toolBoundConfig) as Config; + expect(derived.getShouldAvoidPermissionPrompts()).toBe(true); + }); + + it('keeps prompts allowed through the chain for a bubbling background agent', async () => { + vi.mocked( + config.isInteractive as ReturnType, + ).mockReturnValue(true); + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue({ + ...bgSubagent, + approvalMode: 'bubble', + }); + + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation({ + description: 'Start monitor', + prompt: 'Watch for changes', + subagent_type: 'monitor', + }); + await invocation.execute(); + + const contexts = vi.mocked(config.createToolRegistry).mock.contexts; + const toolBoundConfig = contexts[contexts.length - 1] as Config; + // Bubbling: prompts stay allowed (they park on the entry), and nested + // launches inherit the same policy so THEIR approvals can bubble too. + expect(toolBoundConfig.getShouldAvoidPermissionPrompts()).toBe(false); + const derived = Object.create(toolBoundConfig) as Config; + expect(derived.getShouldAvoidPermissionPrompts()).toBe(false); + }); + it('forwards the scheduler-provided callId as toolUseId on the registry entry', async () => { const params: AgentParams = { description: 'Start monitor', @@ -7144,6 +7257,80 @@ describe('AgentTool', () => { }); }); +describe('findBackgroundedAncestorAgentId', () => { + type Entry = { + isBackgrounded: boolean; + status: string; + parentAgentId?: string | null; + }; + const registryOf = (entries: Record) => ({ + get: (id: string) => + entries[id] as unknown as ReturnType< + Parameters[0]['get'] + >, + }); + + it('returns undefined without a starting agent id', () => { + const registry = registryOf({}); + expect(findBackgroundedAncestorAgentId(registry, undefined)).toBe( + undefined, + ); + expect(findBackgroundedAncestorAgentId(registry, null)).toBe(undefined); + }); + + it('returns a directly backgrounded running ancestor', () => { + const registry = registryOf({ + 'fork-1': { isBackgrounded: true, status: 'running' }, + }); + expect(findBackgroundedAncestorAgentId(registry, 'fork-1')).toBe('fork-1'); + }); + + it('walks foreground lineage to the backgrounded ancestor', () => { + const registry = registryOf({ + 'leaf-fg': { + isBackgrounded: false, + status: 'running', + parentAgentId: 'mid-fg', + }, + 'mid-fg': { + isBackgrounded: false, + status: 'running', + parentAgentId: 'fork-1', + }, + 'fork-1': { isBackgrounded: true, status: 'running' }, + }); + expect(findBackgroundedAncestorAgentId(registry, 'leaf-fg')).toBe('fork-1'); + }); + + it('returns undefined for a terminal backgrounded ancestor', () => { + const registry = registryOf({ + 'fork-1': { isBackgrounded: true, status: 'completed' }, + }); + expect(findBackgroundedAncestorAgentId(registry, 'fork-1')).toBe(undefined); + }); + + it('returns undefined when the lineage breaks', () => { + const registry = registryOf({ + 'leaf-fg': { + isBackgrounded: false, + status: 'running', + parentAgentId: 'gone', + }, + }); + expect(findBackgroundedAncestorAgentId(registry, 'leaf-fg')).toBe( + undefined, + ); + }); + + it('caps the walk on a corrupt (cyclic) lineage', () => { + const registry = registryOf({ + a: { isBackgrounded: false, status: 'running', parentAgentId: 'b' }, + b: { isBackgrounded: false, status: 'running', parentAgentId: 'a' }, + }); + expect(findBackgroundedAncestorAgentId(registry, 'a')).toBe(undefined); + }); +}); + describe('resolveSubagentApprovalMode', () => { it('should return yolo when parent is yolo, regardless of agent config', () => { expect(resolveSubagentApprovalMode(ApprovalMode.YOLO, 'plan', true)).toBe( diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 63fe66e1fd3..1ca1bbe5223 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -125,6 +125,7 @@ import { type AgentPersistedCliFlags, } from '../../agents/agent-transcript.js'; import type { + AgentTask, BackgroundSlotReservation, ResidentBackgroundAgent, } from '../../agents/background-tasks.js'; @@ -321,6 +322,40 @@ function approvalModeToPermissionMode(mode: ApprovalMode): PermissionMode { * - Otherwise, the agent definition's mode applies if set * - Default fallback is auto-edit (sub-agents need autonomy) */ +/** + * Walks a foreground launch's registry lineage (parentAgentId links) to the + * nearest BACKGROUNDED, still-running ancestor entry. + * + * Why: a nested foreground agent has no inline UI. Its scheduler's + * TOOL_WAITING_APPROVAL fires on the invocation's own emitter, which nothing + * bridges — the call would wait forever (the batch promise only resolves on + * completion or abort). When such an ancestor exists, the launch path bridges + * the nested emitter to that ancestor's Background-tasks entry, parking the + * approval exactly where the ancestor's own approvals go. No ancestor (a + * top-level foreground launch from the main session) → undefined → no bridge, + * and the existing inline confirmation path applies unchanged. + * + * Foreground entries stay registered while running and carry parentAgentId + * (register() in the launch path), so the chain is walkable mid-execution. + * The hop cap is defensive: lineage is acyclic by construction, but a corrupt + * entry must not spin. + */ +export function findBackgroundedAncestorAgentId( + registry: { get(agentId: string): AgentTask | undefined }, + startAgentId: string | null | undefined, +): string | undefined { + let id = startAgentId ?? undefined; + for (let hops = 0; id !== undefined && hops < 16; hops++) { + const entry = registry.get(id); + if (!entry) return undefined; + if (entry.isBackgrounded) { + return entry.status === 'running' ? id : undefined; + } + id = entry.parentAgentId ?? undefined; + } + return undefined; +} + export function resolveSubagentApprovalMode( parentApprovalMode: ApprovalMode, agentApprovalMode?: string, @@ -2992,9 +3027,17 @@ class AgentToolInvocation extends BaseToolInvocation { // Background agents have no inline UI. Preserve the resolved approval // mode while overriding only the prompt-avoidance policy used by their // scheduler. - const subagentRuntimeConfig = shouldRunInBackground - ? (Object.create(agentConfig) as Config) - : agentConfig; + // + // Stamp the policy on agentConfig itself — the config the rebuilt tool + // registry's tools (including any nested AgentTool) are bound to — not + // on a wrapper above it. Nested launches branch their own configs off + // agentConfig via Object.create, so the policy must sit where their + // prototype chains can reach it: stamped only on a wrapper, a nested + // scheduler resolved Config.prototype's `false`, believed it could + // prompt, and waited forever on a TOOL_WAITING_APPROVAL nobody could + // see or answer. agentConfig is created per-launch by + // createApprovalModeOverride, so the stamp leaks nowhere. + const subagentRuntimeConfig = agentConfig; if (shouldRunInBackground) { subagentRuntimeConfig.getShouldAvoidPermissionPrompts = () => !shouldBubble; @@ -4011,6 +4054,26 @@ class AgentToolInvocation extends BaseToolInvocation { this.eventEmitter.on(AgentEventType.TOOL_CALL, onFgToolCall); this.eventEmitter.on(AgentEventType.USAGE_METADATA, onFgUsageMetadata); + // Nested foreground launches under a backgrounded ancestor (a fork or + // any background agent that spawns sub-agents) have no inline UI: + // their TOOL_WAITING_APPROVAL fires on this invocation's emitter, + // which nothing else bridges. Park those approvals on the ancestor's + // Background-tasks entry — the same place the ancestor's own approvals + // go — so the user can answer them from the dialog instead of the + // nested call hanging forever. Top-level foreground launches have no + // such ancestor and keep the inline confirmation path unchanged. + // Double answers (dialog + any inline path) are deduped by the + // runtime's per-call responded guard. + const approvalAncestorId = findBackgroundedAncestorAgentId( + registry, + getCurrentAgentId(), + ); + const cleanupNestedApprovalBridge = approvalAncestorId + ? registry.bridgeApprovalEvents(approvalAncestorId, this.eventEmitter, { + nestedSource: true, + }) + : undefined; + try { ({ cleanup: cleanupFgJsonl } = attachJsonlTranscriptWriter( this.eventEmitter, @@ -4128,6 +4191,7 @@ class AgentToolInvocation extends BaseToolInvocation { } this.eventEmitter.off(AgentEventType.TOOL_CALL, onFgToolCall); this.eventEmitter.off(AgentEventType.USAGE_METADATA, onFgUsageMetadata); + cleanupNestedApprovalBridge?.(); signal?.removeEventListener('abort', onParentAbort); cleanupOwnedMonitorNotifications(); // Release the JSONL writer's listeners and close the fd before From febd3d766a7f7a076df92bacedcea5284c3a122e Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 23 Aug 2026 18:00:49 +0800 Subject: [PATCH 2/7] chore(i18n): add 'from nested agent' locale entries --- packages/cli/src/i18n/locales/ca.js | 1 + packages/cli/src/i18n/locales/de.js | 1 + packages/cli/src/i18n/locales/en.js | 1 + packages/cli/src/i18n/locales/fr.js | 1 + packages/cli/src/i18n/locales/ja.js | 1 + packages/cli/src/i18n/locales/pt.js | 1 + packages/cli/src/i18n/locales/ru.js | 1 + packages/cli/src/i18n/locales/zh-TW.js | 1 + packages/cli/src/i18n/locales/zh.js | 1 + 9 files changed, 9 insertions(+) diff --git a/packages/cli/src/i18n/locales/ca.js b/packages/cli/src/i18n/locales/ca.js index 10f51601b97..e967ebb3b76 100644 --- a/packages/cli/src/i18n/locales/ca.js +++ b/packages/cli/src/i18n/locales/ca.js @@ -1593,6 +1593,7 @@ export default { 'rebutjat — editeu la configuració per tornar a aprovar', 'Background agent needs approval': "L'agent en segon pla necessita aprovació", 'Approve or deny the request above': 'Aprova o denega la sol·licitud de dalt', + 'from nested agent': "de l'agent imbricat", Running: 'En execució', Pausing: 'Pausant', Paused: 'En pausa', diff --git a/packages/cli/src/i18n/locales/de.js b/packages/cli/src/i18n/locales/de.js index 26ea3ad40d9..f3589ab1083 100644 --- a/packages/cli/src/i18n/locales/de.js +++ b/packages/cli/src/i18n/locales/de.js @@ -1530,6 +1530,7 @@ export default { 'rejected — edit config to re-approve': 'abgelehnt — Konfiguration bearbeiten, um erneut zu genehmigen', 'Background agent needs approval': 'Hintergrund-Agent wartet auf Genehmigung', + 'from nested agent': 'von verschachteltem Agent', 'Approve or deny the request above': 'Genehmigen oder lehnen Sie die obige Anfrage ab', Running: 'Läuft', diff --git a/packages/cli/src/i18n/locales/en.js b/packages/cli/src/i18n/locales/en.js index 5a7448ccba0..224de556c87 100644 --- a/packages/cli/src/i18n/locales/en.js +++ b/packages/cli/src/i18n/locales/en.js @@ -2049,6 +2049,7 @@ export default { 'rejected — edit config to re-approve', 'Background agent needs approval': 'Background agent needs approval', 'Approve or deny the request above': 'Approve or deny the request above', + 'from nested agent': 'from nested agent', Running: 'Running', Pausing: 'Pausing', Paused: 'Paused', diff --git a/packages/cli/src/i18n/locales/fr.js b/packages/cli/src/i18n/locales/fr.js index 78e45e8c4b0..05fb30ba605 100644 --- a/packages/cli/src/i18n/locales/fr.js +++ b/packages/cli/src/i18n/locales/fr.js @@ -1601,6 +1601,7 @@ export default { 'rejeté — modifiez la configuration pour réapprouver', 'Background agent needs approval': "L'agent en arrière-plan nécessite une approbation", + 'from nested agent': "de l'agent imbriqué", 'Approve or deny the request above': 'Approuvez ou refusez la demande ci-dessus', Running: 'En cours', diff --git a/packages/cli/src/i18n/locales/ja.js b/packages/cli/src/i18n/locales/ja.js index e947225b9b5..a500bb69fab 100644 --- a/packages/cli/src/i18n/locales/ja.js +++ b/packages/cli/src/i18n/locales/ja.js @@ -971,6 +971,7 @@ export default { 'rejected — edit config to re-approve': '拒否済み — 設定を編集して再承認', 'Background agent needs approval': 'バックグラウンドエージェントが承認待ちです', + 'from nested agent': 'ネストされた agent から', 'Approve or deny the request above': '上のリクエストを承認または拒否してください', Running: '実行中', diff --git a/packages/cli/src/i18n/locales/pt.js b/packages/cli/src/i18n/locales/pt.js index e06756833ef..8553bee5f45 100644 --- a/packages/cli/src/i18n/locales/pt.js +++ b/packages/cli/src/i18n/locales/pt.js @@ -1563,6 +1563,7 @@ export default { 'rejeitado — edite a configuração para reaprovar', 'Background agent needs approval': 'Agente em segundo plano precisa de aprovação', + 'from nested agent': 'do agent aninhado', 'Approve or deny the request above': 'Aprove ou negue a solicitação acima', Running: 'Em execução', Pausing: 'Pausando', diff --git a/packages/cli/src/i18n/locales/ru.js b/packages/cli/src/i18n/locales/ru.js index 0cd76aa0a75..97938379fe8 100644 --- a/packages/cli/src/i18n/locales/ru.js +++ b/packages/cli/src/i18n/locales/ru.js @@ -1480,6 +1480,7 @@ export default { 'отклонено — измените конфигурацию для повторного подтверждения', 'Background agent needs approval': 'Фоновый агент требует подтверждения', 'Approve or deny the request above': 'Подтвердите или отклоните запрос выше', + 'from nested agent': 'от вложенного агента', Running: 'Выполняется', Pausing: 'Приостанавливается', Paused: 'Приостановлено', diff --git a/packages/cli/src/i18n/locales/zh-TW.js b/packages/cli/src/i18n/locales/zh-TW.js index 612d1743f2f..c54c6cf2b07 100644 --- a/packages/cli/src/i18n/locales/zh-TW.js +++ b/packages/cli/src/i18n/locales/zh-TW.js @@ -2128,6 +2128,7 @@ export default { 'rejected — edit config to re-approve': '已拒絕 — 編輯設定以重新審批', 'Background agent needs approval': '背景 agent 等待審批', 'Approve or deny the request above': '請核准或拒絕上方的請求', + 'from nested agent': '來自嵌套 agent', Running: '執行中', Pausing: '暫停中', Paused: '已暫停', diff --git a/packages/cli/src/i18n/locales/zh.js b/packages/cli/src/i18n/locales/zh.js index 578c99f0207..c5ca775aa42 100644 --- a/packages/cli/src/i18n/locales/zh.js +++ b/packages/cli/src/i18n/locales/zh.js @@ -2324,6 +2324,7 @@ export default { 'rejected — edit config to re-approve': '已拒绝 — 编辑配置以重新审批', 'Background agent needs approval': '后台 agent 等待审批', 'Approve or deny the request above': '请批准或拒绝上方的请求', + 'from nested agent': '来自嵌套 agent', Running: '运行中', Pausing: '暂停中', Paused: '已暂停', From baf26cd1490e678664e6f546c7ef3bedc2a064ec Mon Sep 17 00:00:00 2001 From: qwen-code-ci-bot Date: Sun, 23 Aug 2026 13:33:17 +0000 Subject: [PATCH 3/7] fix(core): harden nested sub-agent approval bridging (#9793) --- .../BackgroundTasksDialog.test.tsx | 69 ++++++++++++++ .../agents/background-agent-resume.test.ts | 94 +++++++++++++++++++ .../core/src/agents/background-tasks.test.ts | 49 ++++++++++ packages/core/src/agents/background-tasks.ts | 24 ++++- packages/core/src/tools/agent/agent.test.ts | 25 ++++- packages/core/src/tools/agent/agent.ts | 71 +++++++------- 6 files changed, 292 insertions(+), 40 deletions(-) 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 f582bf0a77e..ad1a7b9002e 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -871,6 +871,75 @@ describe('BackgroundTasksDialog', () => { expect(h.probe.current!.state.dialogOpen).toBe(true); }); + it('names the nested agent a bridged approval came from', () => { + // Approvals bridged from a NESTED agent onto a backgrounded ancestor's + // entry are stamped with the nested runtime id (subagentId); the detail + // view must name it so the user can tell which descendant is blocked. + const bridgedApproval: NonNullable< + AgentDialogEntry['pendingApprovals'] + >[number] = { + callId: 'nested-1', + name: 'Shell', + description: 'run nested-1', + confirmationDetails: { + type: 'exec', + title: 'Allow execution', + command: 'echo nested', + rootCommand: 'echo', + } as NonNullable< + AgentDialogEntry['pendingApprovals'] + >[number]['confirmationDetails'], + respond: vi.fn(), + at: Date.now(), + subagentId: 'review-agent-abc123', + }; + const bg = entry({ + agentId: 'bg-bridged', + pendingApprovals: [bridgedApproval], + }); + const h = setup([bg]); + + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + + expect(h.probe.current!.state.dialogMode).toBe('detail'); + expect(h.lastFrame()).toContain('from nested agent: review-agent-abc123'); + }); + + it('omits the nested-agent line for an unstamped approval', () => { + // The entry's OWN parked approvals carry no subagentId — the line must + // not render for them. + const ownApproval: NonNullable< + AgentDialogEntry['pendingApprovals'] + >[number] = { + callId: 'own-1', + name: 'Shell', + description: 'run own-1', + confirmationDetails: { + type: 'exec', + title: 'Allow execution', + command: 'echo own', + rootCommand: 'echo', + } as NonNullable< + AgentDialogEntry['pendingApprovals'] + >[number]['confirmationDetails'], + respond: vi.fn(), + at: Date.now(), + }; + const bg = entry({ + agentId: 'bg-own', + pendingApprovals: [ownApproval], + }); + const h = setup([bg]); + + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + + expect(h.probe.current!.state.dialogMode).toBe('detail'); + expect(h.lastFrame()).toContain('Background agent needs approval'); + expect(h.lastFrame()).not.toContain('from nested agent'); + }); + it('Esc backs out of an armed foreground cancel without closing the dialog', () => { const fg = entry({ agentId: 'fg-1', diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index adbcb743cf0..367711dc42e 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -89,6 +89,7 @@ describe('BackgroundAgentResumeService', () => { name: 'researcher', color: 'cyan', model: undefined as string | undefined, + approvalMode: undefined as string | undefined, } : null, ), @@ -782,6 +783,98 @@ describe('BackgroundAgentResumeService', () => { }); }); + it('keeps prompts allowed through the chain for a resumed bubble-mode agent in an interactive session', async () => { + // Mirror of the launch-path bubble test (agent.ts): a resumed agent of + // a `bubble` definition in an INTERACTIVE session surfaces + // confirmations to the Background-tasks dialog instead of auto-denying + // them, and nested launches under it must inherit the same policy + // through their config prototype chains. + const sessionId = 'session-bubble'; + const agentId = 'agent-bubble'; + const metaPath = getAgentMetaPath(tempDir, sessionId, agentId); + const outputFile = getAgentJsonlPath(tempDir, sessionId, agentId); + + writeAgentMeta(metaPath, { + agentId, + agentType: 'researcher', + description: 'Resume bubble policy', + parentSessionId: sessionId, + parentAgentId: null, + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + resolvedApprovalMode: 'auto-edit', + }); + 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 bubble policy' }] }, + }) + '\n', + 'utf8', + ); + + registry.register({ + agentId, + description: 'Resume bubble policy', + subagentType: 'researcher', + isBackgrounded: true, + status: 'paused', + startTime: Date.now(), + abortController: new AbortController(), + prompt: 'Resume bubble policy', + outputFile, + metaPath, + }); + + const subagent = { + execute: vi.fn(async () => {}), + setExternalMessageProvider: vi.fn(), + getCore: () => ({ getEventEmitter: () => new AgentEventEmitter() }), + getExecutionSummary: () => ({ + totalTokens: 0, + outputTokens: 0, + totalDurationMs: 0, + }), + getTerminateMode: () => AgentTerminateMode.GOAL, + getFinalText: () => 'done', + }; + + const { service, subagentManager, config } = createService(); + config.isInteractive = () => true; + subagentManager.loadSubagent.mockResolvedValue({ + name: 'researcher', + color: 'cyan', + model: undefined, + approvalMode: 'bubble', + }); + subagentManager.createAgentHeadless.mockResolvedValue({ + subagent, + dispose: vi.fn().mockResolvedValue(undefined), + }); + + const resumed = await service.resumeBackgroundAgent(agentId, 'continue'); + expect(resumed).toBeDefined(); + + const createCall = subagentManager.createAgentHeadless.mock.calls.at(-1); + expect(createCall).toBeDefined(); + const bgConfig = createCall![1] as Config; + // Bubbling: prompts stay allowed (they park on the entry), and nested + // launches inherit the same policy through the prototype chain. + expect(bgConfig.getShouldAvoidPermissionPrompts()).toBe(false); + expect( + (Object.create(bgConfig) as Config).getShouldAvoidPermissionPrompts(), + ).toBe(false); + + await vi.waitFor(() => { + expect(registry.get(agentId)?.status).toBe('completed'); + }); + }); + it('fires SubagentStart hooks when resuming and injects hook context', async () => { const sessionId = 'session-resume'; const agentId = 'agent-resume'; @@ -1605,6 +1698,7 @@ describe('BackgroundAgentResumeService', () => { name: 'researcher', color: 'cyan', model: 'configured-model', + approvalMode: undefined, }); subagentManager.createAgentHeadless = createAgentHeadless; diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index 25187a00edb..842b7c6815a 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -2784,6 +2784,55 @@ describe('BackgroundTaskRegistry', () => { cleanup(); }); + it('drops a re-emitted waiting event for an already-parked call silently', async () => { + // The scheduler re-notifies the whole batch on every status + // transition, and agent-core re-emits TOOL_WAITING_APPROVAL for every + // still-awaiting call without dedup — so while one call is parked, a + // sibling's transition re-emits the parked call's event. That + // duplicate must be dropped silently: auto-rejecting it cancels the + // waiting call while its prompt is still visible in the dialog, and + // the runtime's responded set then no-ops the user's real answer. + const emitter = new AgentEventEmitter(); + registry.register(makeRegistration('bg-appr-reemit')); + registry.bridgeApprovalEvents('bg-appr-reemit', emitter); + + const respond = vi.fn(async () => {}); + const waiting = { + subagentId: 'bg-appr-reemit', + round: 1, + callId: 'c1', + name: 'Shell', + description: 'run c1', + args: {}, + confirmationDetails: { + type: 'exec', + } as BackgroundApproval['confirmationDetails'], + respond, + timestamp: Date.now(), + }; + emitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, waiting); + expect(registry.getPendingApprovals('bg-appr-reemit')).toHaveLength(1); + + // A batch-mate status transition re-emits the parked call's event. + emitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, { + ...waiting, + timestamp: Date.now(), + }); + + expect(respond).not.toHaveBeenCalled(); + expect(registry.getPendingApprovals('bg-appr-reemit')).toHaveLength(1); + // The user's dialog answer still reaches the parked call. + await registry.resolvePendingApproval( + 'bg-appr-reemit', + 'c1', + ToolConfirmationOutcome.ProceedOnce, + ); + expect(respond).toHaveBeenCalledWith( + ToolConfirmationOutcome.ProceedOnce, + undefined, + ); + }); + it('auto-rejects a bridged approval that arrives after termination', () => { const emitter = new AgentEventEmitter(); registry.register(makeRegistration('bg-appr-10')); diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index f0bc3c90abf..f675591eeb9 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -1170,12 +1170,26 @@ export class BackgroundTaskRegistry { at: event.timestamp, ...(options?.nestedSource ? { subagentId: event.subagentId } : {}), }); - // If the entry is already gone/terminal we couldn't park it — reject - // so the agent's reasoning loop doesn't block forever on this call. - // `.catch()` rather than try/catch: respond is async and a late - // rejection (frames torn down post-termination) must not escape as - // an unhandledRejection. if (!parked) { + // Expected duplicate: the scheduler re-notifies the whole batch on + // every status transition and agent-core re-emits + // TOOL_WAITING_APPROVAL for every still-awaiting call, so an + // already-parked call's event can arrive again. Leave the parked + // prompt untouched — rejecting here would cancel the waiting call + // while its dialog is still visible, and the runtime's responded + // set would then no-op the user's real answer. + if ( + this.getPendingApprovals(agentId).some( + (a) => a.callId === event.callId, + ) + ) { + return; + } + // Otherwise the entry is already gone/terminal and we couldn't + // park it — reject so the agent's reasoning loop doesn't block + // forever on this call. `.catch()` rather than try/catch: respond + // is async and a late rejection (frames torn down + // post-termination) must not escape as an unhandledRejection. void event.respond(REJECTED_OUTCOME).catch((error) => { debugLogger.error( `Failed to reject unparkable approval ${agentId}/${event.callId}:`, diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 8f62538936a..78dd8d8f715 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -7302,6 +7302,29 @@ describe('findBackgroundedAncestorAgentId', () => { expect(findBackgroundedAncestorAgentId(registry, 'leaf-fg')).toBe('fork-1'); }); + it('reaches the backgrounded ancestor on deep supported lineages', () => { + // maxSubagentDepth is user-configurable up to MAX_SUBAGENT_DEPTH_LIMIT + // (100), so the walk must not encode a smaller depth assumption: on a + // lineage deeper than the cap it used to carry, it walked off the end + // and returned undefined — no bridge, and the nested call hung on an + // approval nobody could see. Cycle protection comes from a visited set, + // not a hop budget. + const entries: Record = { + 'bg-root': { isBackgrounded: true, status: 'running' }, + }; + const depth = 20; + for (let i = 0; i < depth; i++) { + entries[`fg-${i}`] = { + isBackgrounded: false, + status: 'running', + parentAgentId: i === 0 ? 'bg-root' : `fg-${i - 1}`, + }; + } + expect( + findBackgroundedAncestorAgentId(registryOf(entries), `fg-${depth - 1}`), + ).toBe('bg-root'); + }); + it('returns undefined for a terminal backgrounded ancestor', () => { const registry = registryOf({ 'fork-1': { isBackgrounded: true, status: 'completed' }, @@ -7322,7 +7345,7 @@ describe('findBackgroundedAncestorAgentId', () => { ); }); - it('caps the walk on a corrupt (cyclic) lineage', () => { + it('terminates on a corrupt (cyclic) lineage', () => { const registry = registryOf({ a: { isBackgrounded: false, status: 'running', parentAgentId: 'b' }, b: { isBackgrounded: false, status: 'running', parentAgentId: 'a' }, diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 1ca1bbe5223..762545303c0 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -322,40 +322,6 @@ function approvalModeToPermissionMode(mode: ApprovalMode): PermissionMode { * - Otherwise, the agent definition's mode applies if set * - Default fallback is auto-edit (sub-agents need autonomy) */ -/** - * Walks a foreground launch's registry lineage (parentAgentId links) to the - * nearest BACKGROUNDED, still-running ancestor entry. - * - * Why: a nested foreground agent has no inline UI. Its scheduler's - * TOOL_WAITING_APPROVAL fires on the invocation's own emitter, which nothing - * bridges — the call would wait forever (the batch promise only resolves on - * completion or abort). When such an ancestor exists, the launch path bridges - * the nested emitter to that ancestor's Background-tasks entry, parking the - * approval exactly where the ancestor's own approvals go. No ancestor (a - * top-level foreground launch from the main session) → undefined → no bridge, - * and the existing inline confirmation path applies unchanged. - * - * Foreground entries stay registered while running and carry parentAgentId - * (register() in the launch path), so the chain is walkable mid-execution. - * The hop cap is defensive: lineage is acyclic by construction, but a corrupt - * entry must not spin. - */ -export function findBackgroundedAncestorAgentId( - registry: { get(agentId: string): AgentTask | undefined }, - startAgentId: string | null | undefined, -): string | undefined { - let id = startAgentId ?? undefined; - for (let hops = 0; id !== undefined && hops < 16; hops++) { - const entry = registry.get(id); - if (!entry) return undefined; - if (entry.isBackgrounded) { - return entry.status === 'running' ? id : undefined; - } - id = entry.parentAgentId ?? undefined; - } - return undefined; -} - export function resolveSubagentApprovalMode( parentApprovalMode: ApprovalMode, agentApprovalMode?: string, @@ -415,6 +381,43 @@ export function resolveSubagentApprovalMode( return approvalModeToPermissionMode(parentApprovalMode); } +/** + * Walks a foreground launch's registry lineage (parentAgentId links) to the + * nearest BACKGROUNDED, still-running ancestor entry. + * + * Why: a nested foreground agent has no inline UI. Its scheduler's + * TOOL_WAITING_APPROVAL fires on the invocation's own emitter, which nothing + * bridges — the call would wait forever (the batch promise only resolves on + * completion or abort). When such an ancestor exists, the launch path bridges + * the nested emitter to that ancestor's Background-tasks entry, parking the + * approval exactly where the ancestor's own approvals go. No ancestor (a + * top-level foreground launch from the main session) → undefined → no bridge, + * and the existing inline confirmation path applies unchanged. + * + * Foreground entries stay registered while running and carry parentAgentId + * (register() in the launch path), so the chain is walkable mid-execution. + * The walk must reach the ancestor on ANY supported lineage depth + * (maxSubagentDepth is configurable up to MAX_SUBAGENT_DEPTH_LIMIT), so + * cycle protection comes from a visited set rather than a hop budget. + */ +export function findBackgroundedAncestorAgentId( + registry: { get(agentId: string): AgentTask | undefined }, + startAgentId: string | null | undefined, +): string | undefined { + const seen = new Set(); + let id = startAgentId ?? undefined; + while (id !== undefined && !seen.has(id)) { + seen.add(id); + const entry = registry.get(id); + if (!entry) return undefined; + if (entry.isBackgrounded) { + return entry.status === 'running' ? id : undefined; + } + id = entry.parentAgentId ?? undefined; + } + return undefined; +} + /** * Maps PermissionMode back to ApprovalMode. */ From f3607f8f4dbcfe9ab536388911e00efd11755f89 Mon Sep 17 00:00:00 2001 From: qwen-code-ci-bot Date: Sun, 23 Aug 2026 16:26:29 +0000 Subject: [PATCH 4/7] fix(core): tighten nested approval bridging per review round 2 (#9793) - addPendingApproval returns a discriminated result ('parked' | 'duplicate' | 'unavailable') mirroring the workflow registry's parkPendingApproval, so the bridge drops re-emitted events without re-running the dedup scan on the caller side - log a debug line when a re-emitted approval event is dropped, so "approval never appeared" sessions can tell a drop from a lost event - extract stampBackgroundPromptPolicy next to createApprovalModeOverride; both the launch and resume paths now share one stamp + rationale - gate the nested approval bridge on the inherited prompt-avoidance policy like the sibling bridges, instead of wiring a dead subscription under auto-denying ancestors - refresh two JSDoc examples that still cited the deleted Object.create wrapper --- .../src/agents/background-agent-resume.ts | 22 +++---- .../core/src/agents/background-tasks.test.ts | 29 ++++++-- packages/core/src/agents/background-tasks.ts | 65 ++++++++++-------- .../core/src/subagents/subagent-manager.ts | 9 +-- packages/core/src/tools/agent/agent.test.ts | 31 +++++++++ packages/core/src/tools/agent/agent.ts | 66 ++++++++++++------- 6 files changed, 145 insertions(+), 77 deletions(-) diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index 56307b52cd2..8541d1cf381 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -44,7 +44,10 @@ import { } from '../hooks/stopHookCap.js'; import { toModelVisibleSubagentResult } from './subagent-result.js'; import { runWithAgentContext } from './runtime/agent-context.js'; -import { createApprovalModeOverride } from '../tools/agent/agent.js'; +import { + createApprovalModeOverride, + stampBackgroundPromptPolicy, +} from '../tools/agent/agent.js'; import type { ApprovalMode } from '../config/config.js'; import { FORK_AGENT, @@ -922,16 +925,7 @@ export class BackgroundAgentResumeService { target.subagentConfig?.approvalMode === BUBBLE_APPROVAL_MODE && this.config.isInteractive(), ); - // Stamp the policy on activeAgentConfig itself — the config whose - // rebuilt tool registry a nested AgentTool binds to — so nested - // launches under the resumed agent inherit it through their own - // config prototype chains. Mirrors the launch path (agent.ts); a - // wrapper-only stamp left nested schedulers resolving - // Config.prototype's `false` and hanging on unanswerable approvals. - // activeAgentConfig is created per-resume by - // createApprovalModeOverride, so the stamp leaks nowhere. - const bgConfig = activeAgentConfig; - bgConfig.getShouldAvoidPermissionPrompts = () => !shouldBubble; + stampBackgroundPromptPolicy(activeAgentConfig, shouldBubble); const records = await jsonl.read(outputFile); const recovery = recoverTranscript(records); @@ -946,7 +940,7 @@ export class BackgroundAgentResumeService { ] : [ ...( - await getInitialChatHistory(bgConfig as Config, undefined, { + await getInitialChatHistory(activeAgentConfig, undefined, { includeDeferredToolsReminder: false, includeAvailableSkillsReminder: subagentWillHaveSkillTool( target.subagentConfig, @@ -992,7 +986,7 @@ export class BackgroundAgentResumeService { let subagent: AgentHeadless; if (target.isFork) { subagent = await this.createResumedForkSubagent( - bgConfig as Config, + activeAgentConfig, bgEventEmitter, resumeHistory ?? [], currentForkRuntime!, @@ -1005,7 +999,7 @@ export class BackgroundAgentResumeService { : target.subagentConfig!; const result = await this.config .getSubagentManager() - .createAgentHeadless(resumeSubagentConfig, bgConfig as Config, { + .createAgentHeadless(resumeSubagentConfig, activeAgentConfig, { eventEmitter: bgEventEmitter, promptConfigOverrides: { initialMessages: resumeHistory, diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index 842b7c6815a..486caabf845 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -28,6 +28,17 @@ import { AgentEventEmitter, AgentEventType } from './runtime/agent-events.js'; import { ToolConfirmationOutcome } from '../tools/tools.js'; import { todoWorkChainContext } from '../utils/promptIdContext.js'; +const mockDebugLogger = vi.hoisted(() => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +})); + +vi.mock('../utils/debugLogger.js', () => ({ + createDebugLogger: () => mockDebugLogger, +})); + function makeApproval( callId: string, respond: BackgroundApproval['respond'] = vi.fn(async () => {}), @@ -2438,9 +2449,12 @@ describe('BackgroundTaskRegistry', () => { registry.setApprovalChangeCallback(onChange); registry.register(makeRegistration('bg-appr-1')); - const ok = registry.addPendingApproval('bg-appr-1', makeApproval('c1')); + const parked = registry.addPendingApproval( + 'bg-appr-1', + makeApproval('c1'), + ); - expect(ok).toBe(true); + expect(parked).toBe('parked'); expect(registry.getPendingApprovals('bg-appr-1')).toHaveLength(1); expect(registry.get('bg-appr-1')?.pendingApprovals?.[0].callId).toBe( 'c1', @@ -2453,10 +2467,10 @@ describe('BackgroundTaskRegistry', () => { registry.complete('bg-appr-2', 'done'); expect(registry.addPendingApproval('bg-appr-2', makeApproval('c1'))).toBe( - false, + 'unavailable', ); expect(registry.addPendingApproval('missing', makeApproval('c1'))).toBe( - false, + 'unavailable', ); }); @@ -2465,7 +2479,7 @@ describe('BackgroundTaskRegistry', () => { registry.addPendingApproval('bg-appr-3', makeApproval('c1')); expect(registry.addPendingApproval('bg-appr-3', makeApproval('c1'))).toBe( - false, + 'duplicate', ); expect(registry.getPendingApprovals('bg-appr-3')).toHaveLength(1); }); @@ -2821,6 +2835,11 @@ describe('BackgroundTaskRegistry', () => { expect(respond).not.toHaveBeenCalled(); expect(registry.getPendingApprovals('bg-appr-reemit')).toHaveLength(1); + // The drop is logged at debug level so a session where "the approval + // never appeared" can tell a re-emission drop from a lost event. + expect(mockDebugLogger.debug).toHaveBeenCalledWith( + expect.stringContaining('bg-appr-reemit/c1'), + ); // The user's dialog answer still reaches the parked call. await registry.resolvePendingApproval( 'bg-appr-reemit', diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index f675591eeb9..3bae730960b 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -1050,26 +1050,33 @@ export class BackgroundTaskRegistry { } /** - * Park a tool call awaiting user approval ("permission bubbling"). No-op - * (and the call is auto-rejected by the caller) if the entry is not a - * running background agent — late approvals after cancellation must not - * resurrect a parked prompt. Duplicate callIds are ignored so a - * re-emitted event can't double-list the same call. + * Park a tool call awaiting user approval ("permission bubbling"). + * Returns a discriminated result — mirroring the workflow registry's + * `parkPendingApproval` — so the bridge can tell an expected duplicate + * (an already-parked call whose event the scheduler re-emitted) apart + * from an unparkable entry (gone or terminal), which the caller + * auto-rejects so the agent's reasoning loop doesn't block forever. + * Late approvals after cancellation must not resurrect a parked prompt; + * duplicate callIds are ignored so a re-emitted event can't double-list + * the same call. */ - addPendingApproval(agentId: string, approval: BackgroundApproval): boolean { + addPendingApproval( + agentId: string, + approval: BackgroundApproval, + ): 'parked' | 'duplicate' | 'unavailable' { const entry = this.agents.get(agentId); if (!entry || !entry.isBackgrounded || entry.status !== 'running') { - return false; + return 'unavailable'; } const prior = entry.pendingApprovals ?? []; - if (prior.some((a) => a.callId === approval.callId)) return false; + if (prior.some((a) => a.callId === approval.callId)) return 'duplicate'; entry.pendingApprovals = [...prior, approval]; debugLogger.info( `Parked approval for background agent ${agentId} ` + `(call ${approval.callId}, ${entry.pendingApprovals.length} pending)`, ); this.emitApprovalChange(entry); - return true; + return 'parked'; } /** @@ -1170,26 +1177,26 @@ export class BackgroundTaskRegistry { at: event.timestamp, ...(options?.nestedSource ? { subagentId: event.subagentId } : {}), }); - if (!parked) { - // Expected duplicate: the scheduler re-notifies the whole batch on - // every status transition and agent-core re-emits - // TOOL_WAITING_APPROVAL for every still-awaiting call, so an - // already-parked call's event can arrive again. Leave the parked - // prompt untouched — rejecting here would cancel the waiting call - // while its dialog is still visible, and the runtime's responded - // set would then no-op the user's real answer. - if ( - this.getPendingApprovals(agentId).some( - (a) => a.callId === event.callId, - ) - ) { - return; - } - // Otherwise the entry is already gone/terminal and we couldn't - // park it — reject so the agent's reasoning loop doesn't block - // forever on this call. `.catch()` rather than try/catch: respond - // is async and a late rejection (frames torn down - // post-termination) must not escape as an unhandledRejection. + if (parked === 'duplicate') { + // Expected: the scheduler re-notifies the whole batch on every + // status transition and agent-core re-emits TOOL_WAITING_APPROVAL + // for every still-awaiting call, so an already-parked call's event + // can arrive again. Leave the parked prompt untouched — rejecting + // here would cancel the waiting call while its dialog is still + // visible, and the runtime's responded set would then no-op the + // user's real answer. Debug level because re-emissions are + // frequent while any approval is parked. + debugLogger.debug( + `Dropped re-emitted approval event for already-parked call ${agentId}/${event.callId}`, + ); + return; + } + if (parked === 'unavailable') { + // The entry is already gone/terminal — reject so the agent's + // reasoning loop doesn't block forever on this call. `.catch()` + // rather than try/catch: respond is async and a late rejection + // (frames torn down post-termination) must not escape as an + // unhandledRejection. void event.respond(REJECTED_OUTCOME).catch((error) => { debugLogger.error( `Failed to reject unparkable approval ${agentId}/${event.callId}:`, diff --git a/packages/core/src/subagents/subagent-manager.ts b/packages/core/src/subagents/subagent-manager.ts index 810b00895da..8058c17f097 100644 --- a/packages/core/src/subagents/subagent-manager.ts +++ b/packages/core/src/subagents/subagent-manager.ts @@ -990,10 +990,11 @@ export class SubagentManager { * a wrapper above `runtimeContext` already rebuilt one (typically * `agent.ts:createApprovalModeOverride`, which marks itself via a * Symbol-keyed flag — Symbol lookup walks the prototype chain, so - * this also catches wrapper-on-wrapper layering like - * `bgConfig = Object.create(agentConfig)` from the background path). - * Rebuilding twice would waste work, leak listeners on shared - * managers, and split caches across registry layers. + * this also catches wrapper-on-wrapper layering like the background + * launch passing its stamped `createApprovalModeOverride` override + * directly as `runtimeContext`). Rebuilding twice would waste work, + * leak listeners on shared managers, and split caches across registry + * layers. */ private async buildSubagentContextOverride( runtimeContext: Config, diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index 78dd8d8f715..37962584fa7 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -2301,6 +2301,37 @@ describe('AgentTool', () => { expect(registry.bridgeApprovalEvents).not.toHaveBeenCalled(); }); + it('does not bridge nested approvals when the inherited policy auto-denies', async () => { + // Under a non-bubble ancestor the nested launch inherits + // prompt-avoidance through its config chain, so its scheduler + // auto-denies and no TOOL_WAITING_APPROVAL can fire — the wiring is + // gated like the sibling bridges instead of leaving a dead + // subscription on the emitter. + (config as unknown as Record)[ + 'getShouldAvoidPermissionPrompts' + ] = vi.fn().mockReturnValue(true); + const registry = config.getBackgroundTaskRegistry() as unknown as { + get: ReturnType; + bridgeApprovalEvents: ReturnType; + }; + registry.get.mockImplementation((id: string) => + id === 'fork-entry' + ? { isBackgrounded: true, status: 'running' } + : undefined, + ); + + const invocation = agentTool.build({ + description: 'Search files', + prompt: 'Find all TypeScript files', + subagent_type: 'file-search', + }); + await runWithAgentContext('fork-entry', () => + invocation.execute(new AbortController().signal), + ); + + expect(registry.bridgeApprovalEvents).not.toHaveBeenCalled(); + }); + it('strips internal analysis and summary tags from subagent result', async () => { vi.mocked(mockAgent.getFinalText).mockReturnValue( [ diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 762545303c0..ff04420f5ba 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -458,8 +458,8 @@ export const TOOL_REGISTRY_REBUILT: unique symbol = Symbol.for( * * Used by spawn sites that may be called with a wrapper-on-wrapper * argument (e.g. `subagent-manager.ts:buildSubagentContextOverride` - * receiving `bgConfig = Object.create(agentConfig)` from the - * background-agent path) to skip a redundant rebuild. + * receiving the stamped `createApprovalModeOverride` override passed + * directly from the background-agent path) to skip a redundant rebuild. */ export function hasRebuiltToolRegistry(config: Config): boolean { // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -718,6 +718,30 @@ export async function createApprovalModeOverride( return { config: override as Config, cleanup }; } +/** + * Stamps a background agent's prompt-avoidance policy onto its per-launch / + * per-resume override config. Background agents have no inline UI, so their + * scheduler auto-denies calls that still need confirmation — unless the + * agent opts into permission bubbling (`shouldBubble`), in which case + * prompts surface in the parent session's Background-tasks UI instead. + * + * The stamp MUST land on the config the rebuilt tool registry's tools + * (including any nested AgentTool) are bound to — not on a wrapper above + * it. Nested launches branch their own configs off that config via + * Object.create, so the policy must sit where their prototype chains can + * reach it: stamped only on a wrapper, a nested scheduler resolved + * Config.prototype's `false`, believed it could prompt, and waited forever + * on a TOOL_WAITING_APPROVAL nobody could see or answer. Both callers pass + * a config freshly created by {@link createApprovalModeOverride}, so the + * stamp leaks nowhere. + */ +export function stampBackgroundPromptPolicy( + config: Config, + shouldBubble: boolean, +): void { + config.getShouldAvoidPermissionPrompts = () => !shouldBubble; +} + /** * Agent tool that enables primary agents to delegate tasks to specialized agents. * The tool dynamically loads available agents and includes them in its description @@ -3027,23 +3051,8 @@ class AgentToolInvocation extends BaseToolInvocation { subagentConfig.approvalMode === BUBBLE_APPROVAL_MODE && this.config.isInteractive(), ); - // Background agents have no inline UI. Preserve the resolved approval - // mode while overriding only the prompt-avoidance policy used by their - // scheduler. - // - // Stamp the policy on agentConfig itself — the config the rebuilt tool - // registry's tools (including any nested AgentTool) are bound to — not - // on a wrapper above it. Nested launches branch their own configs off - // agentConfig via Object.create, so the policy must sit where their - // prototype chains can reach it: stamped only on a wrapper, a nested - // scheduler resolved Config.prototype's `false`, believed it could - // prompt, and waited forever on a TOOL_WAITING_APPROVAL nobody could - // see or answer. agentConfig is created per-launch by - // createApprovalModeOverride, so the stamp leaks nowhere. - const subagentRuntimeConfig = agentConfig; if (shouldRunInBackground) { - subagentRuntimeConfig.getShouldAvoidPermissionPrompts = () => - !shouldBubble; + stampBackgroundPromptPolicy(agentConfig, shouldBubble); } // Background agents need a dedicated emitter so their transcript never @@ -3072,7 +3081,7 @@ class AgentToolInvocation extends BaseToolInvocation { let subagentDispose: (() => Promise) | undefined; if (isFork) { const fork = await this.createForkSubagent( - subagentRuntimeConfig as Config, + agentConfig, backgroundEventEmitter, ); subagent = fork.subagent; @@ -3082,7 +3091,7 @@ class AgentToolInvocation extends BaseToolInvocation { } else { const result = await this.subagentManager.createAgentHeadless( subagentConfig, - subagentRuntimeConfig as Config, + agentConfig, { eventEmitter: backgroundEventEmitter ?? this.eventEmitter, ...(shouldRunInBackground && subagentModelId @@ -4071,11 +4080,18 @@ class AgentToolInvocation extends BaseToolInvocation { registry, getCurrentAgentId(), ); - const cleanupNestedApprovalBridge = approvalAncestorId - ? registry.bridgeApprovalEvents(approvalAncestorId, this.eventEmitter, { - nestedSource: true, - }) - : undefined; + // Gated like the sibling bridges: under an auto-denying ancestor + // this launch inherits prompt-avoidance through its config chain, so + // no TOOL_WAITING_APPROVAL can fire and the subscription would be + // dead. + const cleanupNestedApprovalBridge = + approvalAncestorId && !this.config.getShouldAvoidPermissionPrompts?.() + ? registry.bridgeApprovalEvents( + approvalAncestorId, + this.eventEmitter, + { nestedSource: true }, + ) + : undefined; try { ({ cleanup: cleanupFgJsonl } = attachJsonlTranscriptWriter( From ee2d57540ec47719e89387730701f2cb294cfb08 Mon Sep 17 00:00:00 2001 From: qwen-code-ci-bot Date: Sun, 23 Aug 2026 18:57:07 +0000 Subject: [PATCH 5/7] fix(core): key parked background approvals on runtime identity (#9793) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated tool-call ids (`call_qwen_N`) are only unique per conversation, so two nested sub-agents whose first id-less call needs confirmation both arrive under the same callId on their shared background ancestor entry. The parked-approval queue keyed identity on callId alone: the second runtime's prompt was dropped as a "duplicate" with nothing in the dialog to answer, and a resolution or TOOL_RESULT for one runtime could clear another runtime's parked prompt. Key identity on (subagentId, callId) — the same composite the workflow run registry already uses — across addPendingApproval's duplicate predicate, resolvePendingApproval, and clearPendingApproval. Own approvals stay unstamped, so same-call re-emissions from the entry's own runtime still dedupe while different runtimes never collide. --- .../BackgroundTasksDialog.test.tsx | 50 +++++- .../background-view/BackgroundTasksDialog.tsx | 1 + .../core/src/agents/background-tasks.test.ts | 142 +++++++++++++++++- packages/core/src/agents/background-tasks.ts | 40 ++++- 4 files changed, 224 insertions(+), 9 deletions(-) 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 ad1a7b9002e..413418a145f 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -112,6 +112,7 @@ interface Harness { abandon: ReturnType; monitorCancel: ReturnType; dreamCancelTask: ReturnType; + backgroundResolvePendingApproval: ReturnType; workflowResolvePendingApproval: ReturnType; workflowPause: ReturnType; workflowResume: ReturnType; @@ -143,6 +144,7 @@ function setup( const abandon = vi.fn(); const monitorCancel = vi.fn(); const dreamCancelTask = vi.fn(); + const backgroundResolvePendingApproval = vi.fn(); const workflowResolvePendingApproval = vi.fn(); const workflowPause = vi.fn(); const workflowResume = vi.fn(); @@ -154,7 +156,7 @@ function setup( const config = { getBackgroundTaskRegistry: () => ({ cancel, - resolvePendingApproval: vi.fn(), + resolvePendingApproval: backgroundResolvePendingApproval, setActivityChangeCallback: vi.fn(), get: (id: string) => { const match = currentEntries.find( @@ -240,6 +242,7 @@ function setup( abandon, monitorCancel, dreamCancelTask, + backgroundResolvePendingApproval, workflowResolvePendingApproval, workflowPause, workflowResume, @@ -940,6 +943,51 @@ describe('BackgroundTasksDialog', () => { expect(h.lastFrame()).not.toContain('from nested agent'); }); + it('routes an agent approval response with the nested subagentId key part', () => { + // Parked-approval identity is (subagentId, callId): approvals bridged + // from nested runtimes can share a generated callId (`call_qwen_N` + // restarts per conversation), so the resolve must carry the nested + // runtime's id — otherwise one answer could resolve another runtime's + // parked prompt. + const bridgedApproval: NonNullable< + AgentDialogEntry['pendingApprovals'] + >[number] = { + callId: 'call_qwen_1', + name: 'Shell', + description: 'run call_qwen_1', + confirmationDetails: { + type: 'exec', + title: 'Allow execution', + command: 'echo nested', + rootCommand: 'echo', + } as NonNullable< + AgentDialogEntry['pendingApprovals'] + >[number]['confirmationDetails'], + respond: vi.fn(), + at: Date.now(), + subagentId: 'search-agent-aaa111', + }; + const bg = entry({ + agentId: 'bg-route-nested', + pendingApprovals: [bridgedApproval], + }); + const h = setup([bg]); + + h.call(() => h.probe.current!.actions.openDialog()); + h.call(() => h.probe.current!.actions.enterDetail()); + + h.pressKeyBroadcast({ name: 'escape' }); + + expect(h.backgroundResolvePendingApproval).toHaveBeenCalledTimes(1); + expect(h.backgroundResolvePendingApproval).toHaveBeenCalledWith( + 'bg-route-nested', + 'call_qwen_1', + ToolConfirmationOutcome.Cancel, + undefined, + 'search-agent-aaa111', + ); + }); + it('Esc backs out of an armed foreground cancel without closing the dialog', () => { const fg = entry({ agentId: 'fg-1', diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index feedf604f2e..b4309cd6be1 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -1542,6 +1542,7 @@ export const BackgroundTasksDialog: React.FC = ({ selectedApproval.approval.callId, outcome, payload, + selectedApproval.approval.subagentId, ); return; } diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index 486caabf845..ead8fb97b29 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -24,7 +24,11 @@ import { runWithRuntimeContentGenerator, } from './runtime/agent-context.js'; import * as transcript from './agent-transcript.js'; -import { AgentEventEmitter, AgentEventType } from './runtime/agent-events.js'; +import { + AgentEventEmitter, + AgentEventType, + type AgentApprovalRequestEvent, +} from './runtime/agent-events.js'; import { ToolConfirmationOutcome } from '../tools/tools.js'; import { todoWorkChainContext } from '../utils/promptIdContext.js'; @@ -71,6 +75,26 @@ function makeRegistration( }; } +function makeWaitingEvent( + subagentId: string, + callId: string, + respond: BackgroundApproval['respond'] = vi.fn(async () => {}), +): AgentApprovalRequestEvent { + return { + subagentId, + round: 1, + callId, + name: 'Shell', + description: `run ${callId}`, + args: {}, + confirmationDetails: { + type: 'exec', + } as BackgroundApproval['confirmationDetails'], + respond, + timestamp: Date.now(), + }; +} + describe('notification emission and agent context (#7156)', () => { it('captures the Todo work-chain owner at registration', () => { const registry = new BackgroundTaskRegistry(); @@ -2852,6 +2876,122 @@ describe('BackgroundTaskRegistry', () => { ); }); + it('parks own and nested approvals that share a generated callId as separate prompts', () => { + // Generated callIds are only unique per conversation + // (`nextGeneratedId` starts every fresh conversation at + // `call_qwen_1`), so the first id-less call of each nested runtime + // arrives on the shared ancestor entry under the SAME callId. Each + // runtime's prompt must park separately — dropping the second leaves + // nothing in the dialog to answer and its respond() never runs. + registry.register(makeRegistration('bg-collide')); + const ownEmitter = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide', ownEmitter); + const nestedA = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide', nestedA, { + nestedSource: true, + }); + const nestedB = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide', nestedB, { + nestedSource: true, + }); + + // The entry's own bridge stamps no subagentId (undefined). + ownEmitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('bg-collide-runtime', 'call_qwen_1'), + ); + nestedA.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1'), + ); + nestedB.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-bbb222', 'call_qwen_1'), + ); + + const parked = registry.getPendingApprovals('bg-collide'); + expect(parked).toHaveLength(3); + expect(parked.map((a) => a.subagentId)).toEqual([ + undefined, + 'search-agent-aaa111', + 'search-agent-bbb222', + ]); + }); + + it('resolves only the matching runtime when parked callIds collide', async () => { + registry.register(makeRegistration('bg-collide-resolve')); + const nestedA = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-resolve', nestedA, { + nestedSource: true, + }); + const nestedB = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-resolve', nestedB, { + nestedSource: true, + }); + const respondA = vi.fn(async () => {}); + const respondB = vi.fn(async () => {}); + nestedA.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respondA), + ); + nestedB.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-bbb222', 'call_qwen_1', respondB), + ); + + const ok = await registry.resolvePendingApproval( + 'bg-collide-resolve', + 'call_qwen_1', + ToolConfirmationOutcome.ProceedOnce, + undefined, + 'search-agent-aaa111', + ); + + expect(ok).toBe(true); + expect(respondA).toHaveBeenCalledTimes(1); + expect(respondB).not.toHaveBeenCalled(); + const remaining = registry.getPendingApprovals('bg-collide-resolve'); + expect(remaining).toHaveLength(1); + expect(remaining[0]?.subagentId).toBe('search-agent-bbb222'); + }); + + it('clears only its own runtime prompt when a TOOL_RESULT collides on callId', () => { + registry.register(makeRegistration('bg-collide-clear')); + const nestedA = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-clear', nestedA, { + nestedSource: true, + }); + const nestedB = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-clear', nestedB, { + nestedSource: true, + }); + const respondA = vi.fn(async () => {}); + const respondB = vi.fn(async () => {}); + nestedA.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respondA), + ); + nestedB.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-bbb222', 'call_qwen_1', respondB), + ); + + // Runtime A's call settled elsewhere; B's same-callId prompt must + // stay parked. + nestedA.emit(AgentEventType.TOOL_RESULT, { + subagentId: 'search-agent-aaa111', + round: 1, + callId: 'call_qwen_1', + success: true, + } as never); + + const remaining = registry.getPendingApprovals('bg-collide-clear'); + expect(remaining).toHaveLength(1); + expect(remaining[0]?.subagentId).toBe('search-agent-bbb222'); + expect(respondA).not.toHaveBeenCalled(); + expect(respondB).not.toHaveBeenCalled(); + }); + it('auto-rejects a bridged approval that arrives after termination', () => { const emitter = new AgentEventEmitter(); registry.register(makeRegistration('bg-appr-10')); diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index 3bae730960b..36ada05b080 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -1069,7 +1069,18 @@ export class BackgroundTaskRegistry { return 'unavailable'; } const prior = entry.pendingApprovals ?? []; - if (prior.some((a) => a.callId === approval.callId)) return 'duplicate'; + // Identity is (subagentId, callId), mirroring the workflow registry's + // source key: generated callIds (`call_qwen_N`) are only unique per + // conversation, so multiple nested runtimes bridged onto one entry can + // share a callId. Own approvals stay unstamped (undefined), so a + // same-call re-emission from the entry's own runtime still dedupes. + if ( + prior.some( + (a) => + a.callId === approval.callId && a.subagentId === approval.subagentId, + ) + ) + return 'duplicate'; entry.pendingApprovals = [...prior, approval]; debugLogger.info( `Parked approval for background agent ${agentId} ` + @@ -1090,15 +1101,18 @@ export class BackgroundTaskRegistry { callId: string, outcome: Parameters[0], payload?: Parameters[1], + subagentId?: string, ): Promise { const entry = this.agents.get(agentId); if (!entry) return false; - const approval = entry.pendingApprovals?.find((a) => a.callId === callId); + const approval = entry.pendingApprovals?.find( + (a) => a.callId === callId && a.subagentId === subagentId, + ); if (!approval) return false; // Remove before responding so a re-entrant read inside the respond // chain (or a racing TOOL_RESULT clear) sees the call already gone. entry.pendingApprovals = (entry.pendingApprovals ?? []).filter( - (a) => a.callId !== callId, + (a) => !(a.callId === callId && a.subagentId === subagentId), ); this.emitApprovalChange(entry); try { @@ -1129,10 +1143,16 @@ export class BackgroundTaskRegistry { * double-answering. Mirrors the foreground `pendingConfirmation` clear in * the Agent tool's TOOL_RESULT handler. */ - clearPendingApproval(agentId: string, callId: string): void { + clearPendingApproval( + agentId: string, + callId: string, + subagentId?: string, + ): void { const entry = this.agents.get(agentId); if (!entry?.pendingApprovals?.length) return; - const next = entry.pendingApprovals.filter((a) => a.callId !== callId); + const next = entry.pendingApprovals.filter( + (a) => !(a.callId === callId && a.subagentId === subagentId), + ); if (next.length === entry.pendingApprovals.length) return; entry.pendingApprovals = next; this.emitApprovalChange(entry); @@ -1207,8 +1227,14 @@ export class BackgroundTaskRegistry { }; const onResult = (event: AgentToolResultEvent) => { // A result for a parked call means it settled elsewhere — clear the - // stale prompt (without responding again). - this.clearPendingApproval(agentId, event.callId); + // stale prompt (without responding again). Stamp the nested runtime + // exactly as onWaiting does so one runtime's result cannot clear a + // colliding callId parked by another runtime. + this.clearPendingApproval( + agentId, + event.callId, + options?.nestedSource ? event.subagentId : undefined, + ); }; emitter.on(AgentEventType.TOOL_WAITING_APPROVAL, onWaiting); emitter.on(AgentEventType.TOOL_RESULT, onResult); From 93c1c0988a412e57380c5cf065d0e335084f9372 Mon Sep 17 00:00:00 2001 From: qwen-code-ci-bot Date: Sun, 23 Aug 2026 21:21:04 +0000 Subject: [PATCH 6/7] fix(core): attribute nested approval logs and pin composite-key dedup (#9793) --- .../core/src/agents/background-tasks.test.ts | 123 +++++++++++++----- packages/core/src/agents/background-tasks.ts | 16 ++- 2 files changed, 101 insertions(+), 38 deletions(-) diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index ead8fb97b29..bbcc43e0684 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -2701,27 +2701,13 @@ describe('BackgroundTaskRegistry', () => { // approvals must stay unstamped — the runtime subagentId and the // registry agentId use different suffixes, so the bridge caller // declares the nested case rather than anyone comparing ids. - const makeWaiting = (callId: string, subagentId: string) => ({ - subagentId, - round: 1, - callId, - name: 'Shell', - description: `run ${callId}`, - args: {}, - confirmationDetails: { - type: 'exec', - } as BackgroundApproval['confirmationDetails'], - respond: vi.fn(async () => {}), - timestamp: Date.now(), - }); - registry.register(makeRegistration('bg-appr-nested')); const ownEmitter = new AgentEventEmitter(); registry.bridgeApprovalEvents('bg-appr-nested', ownEmitter); ownEmitter.emit( AgentEventType.TOOL_WAITING_APPROVAL, - makeWaiting('own-1', 'fork-runtime1'), + makeWaitingEvent('fork-runtime1', 'own-1'), ); const nestedEmitter = new AgentEventEmitter(); @@ -2730,7 +2716,7 @@ describe('BackgroundTaskRegistry', () => { }); nestedEmitter.emit( AgentEventType.TOOL_WAITING_APPROVAL, - makeWaiting('nested-1', 'review-agent-abc123'), + makeWaitingEvent('review-agent-abc123', 'nested-1'), ); const parked = registry.getPendingApprovals('bg-appr-nested'); @@ -2835,27 +2821,17 @@ describe('BackgroundTaskRegistry', () => { registry.bridgeApprovalEvents('bg-appr-reemit', emitter); const respond = vi.fn(async () => {}); - const waiting = { - subagentId: 'bg-appr-reemit', - round: 1, - callId: 'c1', - name: 'Shell', - description: 'run c1', - args: {}, - confirmationDetails: { - type: 'exec', - } as BackgroundApproval['confirmationDetails'], - respond, - timestamp: Date.now(), - }; - emitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, waiting); + emitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('bg-appr-reemit', 'c1', respond), + ); expect(registry.getPendingApprovals('bg-appr-reemit')).toHaveLength(1); // A batch-mate status transition re-emits the parked call's event. - emitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, { - ...waiting, - timestamp: Date.now(), - }); + emitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('bg-appr-reemit', 'c1', respond), + ); expect(respond).not.toHaveBeenCalled(); expect(registry.getPendingApprovals('bg-appr-reemit')).toHaveLength(1); @@ -2876,6 +2852,44 @@ describe('BackgroundTaskRegistry', () => { ); }); + it('drops a re-emitted waiting event on a nested bridge without double-parking', () => { + // The composite dedup key must compare the incoming subagentId with + // the parked one, not only check the parked entry's stamp: a nested + // runtime's scheduler re-emits TOOL_WAITING_APPROVAL on sibling + // status transitions too, so a stamped event can arrive twice on the + // same nested bridge. Deduping on `subagentId === undefined` alone + // would park a second copy, double-listing the nested call in the + // dialog and inflating the pending count. + registry.register(makeRegistration('bg-appr-reemit-nested')); + const nested = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-appr-reemit-nested', nested, { + nestedSource: true, + }); + + const respond = vi.fn(async () => {}); + nested.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respond), + ); + nested.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respond), + ); + + const parked = registry.getPendingApprovals('bg-appr-reemit-nested'); + expect(parked).toHaveLength(1); + expect(parked[0]?.subagentId).toBe('search-agent-aaa111'); + expect(respond).not.toHaveBeenCalled(); + // The park and drop log lines name the nested runtime so a collision + // on a shared generated callId can be attributed in incident analysis. + expect(mockDebugLogger.info).toHaveBeenCalledWith( + expect.stringContaining('nested search-agent-aaa111'), + ); + expect(mockDebugLogger.debug).toHaveBeenCalledWith( + expect.stringContaining('(nested search-agent-aaa111)'), + ); + }); + it('parks own and nested approvals that share a generated callId as separate prompts', () => { // Generated callIds are only unique per conversation // (`nextGeneratedId` starts every fresh conversation at @@ -2955,6 +2969,47 @@ describe('BackgroundTaskRegistry', () => { expect(remaining[0]?.subagentId).toBe('search-agent-bbb222'); }); + it('resolves the second-parked runtime when parked callIds collide', async () => { + // Resolving the SECOND-parked of two colliding entries pins the + // subagentId conjunct in resolvePendingApproval's find: a callId-only + // find resumes the FIRST runtime and strips the second's prompt + // without ever invoking its respond, leaving that call waiting forever. + registry.register(makeRegistration('bg-collide-resolve-2')); + const nestedA = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-resolve-2', nestedA, { + nestedSource: true, + }); + const nestedB = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-resolve-2', nestedB, { + nestedSource: true, + }); + const respondA = vi.fn(async () => {}); + const respondB = vi.fn(async () => {}); + nestedA.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respondA), + ); + nestedB.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-bbb222', 'call_qwen_1', respondB), + ); + + const ok = await registry.resolvePendingApproval( + 'bg-collide-resolve-2', + 'call_qwen_1', + ToolConfirmationOutcome.ProceedOnce, + undefined, + 'search-agent-bbb222', + ); + + expect(ok).toBe(true); + expect(respondB).toHaveBeenCalledTimes(1); + expect(respondA).not.toHaveBeenCalled(); + const remaining = registry.getPendingApprovals('bg-collide-resolve-2'); + expect(remaining).toHaveLength(1); + expect(remaining[0]?.subagentId).toBe('search-agent-aaa111'); + }); + it('clears only its own runtime prompt when a TOOL_RESULT collides on callId', () => { registry.register(makeRegistration('bg-collide-clear')); const nestedA = new AgentEventEmitter(); diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index 36ada05b080..d2d8eb0835b 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -1084,7 +1084,9 @@ export class BackgroundTaskRegistry { entry.pendingApprovals = [...prior, approval]; debugLogger.info( `Parked approval for background agent ${agentId} ` + - `(call ${approval.callId}, ${entry.pendingApprovals.length} pending)`, + `(call ${approval.callId}` + + (approval.subagentId ? `, nested ${approval.subagentId}` : '') + + `, ${entry.pendingApprovals.length} pending)`, ); this.emitApprovalChange(entry); return 'parked'; @@ -1126,7 +1128,9 @@ export class BackgroundTaskRegistry { ); } catch (error) { debugLogger.error( - `Failed to resolve background approval for ${agentId}/${callId}:`, + `Failed to resolve background approval for ${agentId}/${callId}` + + (subagentId ? ` (nested ${subagentId})` : '') + + ':', error, ); this.fail(agentId, `Failed to resolve background approval: ${callId}`); @@ -1207,7 +1211,9 @@ export class BackgroundTaskRegistry { // user's real answer. Debug level because re-emissions are // frequent while any approval is parked. debugLogger.debug( - `Dropped re-emitted approval event for already-parked call ${agentId}/${event.callId}`, + `Dropped re-emitted approval event for already-parked call ` + + `${agentId}/${event.callId}` + + (options?.nestedSource ? ` (nested ${event.subagentId})` : ''), ); return; } @@ -1219,7 +1225,9 @@ export class BackgroundTaskRegistry { // unhandledRejection. void event.respond(REJECTED_OUTCOME).catch((error) => { debugLogger.error( - `Failed to reject unparkable approval ${agentId}/${event.callId}:`, + `Failed to reject unparkable approval ${agentId}/${event.callId}` + + (options?.nestedSource ? ` (nested ${event.subagentId})` : '') + + ':', error, ); }); From c3715afb1bb181765626062746dc94ac8faba052 Mon Sep 17 00:00:00 2001 From: qwen-code-ci-bot Date: Sun, 23 Aug 2026 23:12:36 +0000 Subject: [PATCH 7/7] fix(core): attribute remaining approval failures and pin own-vs-nested collision tests (#9793) --- .../core/src/agents/background-tasks.test.ts | 131 ++++++++++++++++++ packages/core/src/agents/background-tasks.ts | 10 +- 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/packages/core/src/agents/background-tasks.test.ts b/packages/core/src/agents/background-tasks.test.ts index bbcc43e0684..248d64da95a 100644 --- a/packages/core/src/agents/background-tasks.test.ts +++ b/packages/core/src/agents/background-tasks.test.ts @@ -2684,6 +2684,33 @@ describe('BackgroundTaskRegistry', () => { expect(onNotify).toHaveBeenCalledOnce(); }); + it('names the nested runtime in the resolve fail reason', async () => { + // The entry's failure reason is what incident analysis reads when a + // parked call never resumes; with colliding generated callIds it + // must name which runtime's respond() tore down, like the error log. + const respond = vi.fn(async () => { + throw new Error('frames torn down'); + }); + registry.register(makeRegistration('bg-appr-retry-nested')); + registry.addPendingApproval('bg-appr-retry-nested', { + ...makeApproval('c1', respond), + subagentId: 'search-agent-aaa111', + }); + + const ok = await registry.resolvePendingApproval( + 'bg-appr-retry-nested', + 'c1', + ToolConfirmationOutcome.ProceedOnce, + undefined, + 'search-agent-aaa111', + ); + + expect(ok).toBe(false); + expect(registry.get('bg-appr-retry-nested')?.error).toBe( + 'Failed to resolve background approval: c1 (nested search-agent-aaa111)', + ); + }); + it('auto-rejects parked approvals on cancel', () => { const respond = vi.fn(async () => {}); registry.register(makeRegistration('bg-appr-8')); @@ -2695,6 +2722,32 @@ describe('BackgroundTaskRegistry', () => { expect(registry.getPendingApprovals('bg-appr-8')).toHaveLength(0); }); + it('names the nested runtime in the auto-reject error log', async () => { + // rejectPendingApprovals' .catch is the only trace when a parked + // call's respond rejects during teardown; with colliding generated + // callIds the runtime stamp is the only way to attribute it. + const respond = vi.fn(async () => { + throw new Error('frames torn down'); + }); + registry.register(makeRegistration('bg-appr-autorej-nested')); + registry.addPendingApproval('bg-appr-autorej-nested', { + ...makeApproval('c1', respond), + subagentId: 'search-agent-aaa111', + }); + + registry.cancel('bg-appr-autorej-nested', { notify: false }); + // respond rejects asynchronously; let the .catch handler run. + await Promise.resolve(); + + expect(respond).toHaveBeenCalledWith(ToolConfirmationOutcome.Cancel); + expect(mockDebugLogger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'bg-appr-autorej-nested/c1 (nested search-agent-aaa111)', + ), + expect.any(Error), + ); + }); + it('stamps subagentId on bridged approvals only for a nestedSource bridge', () => { // A nested agent's approvals are parked on its backgrounded ancestor's // entry; the UI needs to name the actual waiter. The entry's OWN @@ -3047,6 +3100,84 @@ describe('BackgroundTaskRegistry', () => { expect(respondB).not.toHaveBeenCalled(); }); + it('resolves the unstamped own approval when a stamped callId collides', async () => { + // The dialog resolves an entry's OWN approval with its (absent) + // subagentId. With a nested approval parked FIRST under the same + // generated callId, relaxing the find conjunct to match any parked + // approval when no subagentId is given would resume the nested + // runtime and strip its prompt, leaving the entry's own call + // waiting forever — the silent hang this PR fixes. + registry.register(makeRegistration('bg-collide-own')); + const nested = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-own', nested, { + nestedSource: true, + }); + const ownEmitter = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-own', ownEmitter); + const respondNested = vi.fn(async () => {}); + const respondOwn = vi.fn(async () => {}); + nested.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respondNested), + ); + ownEmitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('bg-collide-own-runtime', 'call_qwen_1', respondOwn), + ); + + const ok = await registry.resolvePendingApproval( + 'bg-collide-own', + 'call_qwen_1', + ToolConfirmationOutcome.ProceedOnce, + ); + + expect(ok).toBe(true); + expect(respondOwn).toHaveBeenCalledTimes(1); + expect(respondNested).not.toHaveBeenCalled(); + const remaining = registry.getPendingApprovals('bg-collide-own'); + expect(remaining).toHaveLength(1); + expect(remaining[0]?.subagentId).toBe('search-agent-aaa111'); + }); + + it('clears the unstamped own prompt without dropping a stamped collision', () => { + registry.register(makeRegistration('bg-collide-clear-own')); + const nested = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-clear-own', nested, { + nestedSource: true, + }); + const ownEmitter = new AgentEventEmitter(); + registry.bridgeApprovalEvents('bg-collide-clear-own', ownEmitter); + const respondNested = vi.fn(async () => {}); + const respondOwn = vi.fn(async () => {}); + nested.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent('search-agent-aaa111', 'call_qwen_1', respondNested), + ); + ownEmitter.emit( + AgentEventType.TOOL_WAITING_APPROVAL, + makeWaitingEvent( + 'bg-collide-clear-own-runtime', + 'call_qwen_1', + respondOwn, + ), + ); + + // The entry's own call settled elsewhere; the nested prompt parked + // under the same callId must stay parked. + ownEmitter.emit(AgentEventType.TOOL_RESULT, { + subagentId: 'bg-collide-clear-own-runtime', + round: 1, + callId: 'call_qwen_1', + success: true, + } as never); + + const remaining = registry.getPendingApprovals('bg-collide-clear-own'); + expect(remaining).toHaveLength(1); + expect(remaining[0]?.subagentId).toBe('search-agent-aaa111'); + expect(respondNested).not.toHaveBeenCalled(); + expect(respondOwn).not.toHaveBeenCalled(); + }); + it('auto-rejects a bridged approval that arrives after termination', () => { const emitter = new AgentEventEmitter(); registry.register(makeRegistration('bg-appr-10')); diff --git a/packages/core/src/agents/background-tasks.ts b/packages/core/src/agents/background-tasks.ts index d2d8eb0835b..da4e861587c 100644 --- a/packages/core/src/agents/background-tasks.ts +++ b/packages/core/src/agents/background-tasks.ts @@ -1133,7 +1133,11 @@ export class BackgroundTaskRegistry { ':', error, ); - this.fail(agentId, `Failed to resolve background approval: ${callId}`); + this.fail( + agentId, + `Failed to resolve background approval: ${callId}` + + (subagentId ? ` (nested ${subagentId})` : ''), + ); entry.abortController.abort(); return false; } @@ -1865,7 +1869,9 @@ export class BackgroundTaskRegistry { // an unhandledRejection). void approval.respond(REJECTED_OUTCOME).catch((error) => { debugLogger.error( - `Failed to auto-reject parked approval ${entry.agentId}/${approval.callId}:`, + `Failed to auto-reject parked approval ${entry.agentId}/${approval.callId}` + + (approval.subagentId ? ` (nested ${approval.subagentId})` : '') + + ':', error, ); });