diff --git a/docs/design/claude-code-web-shell-backend.md b/docs/design/claude-code-web-shell-backend.md new file mode 100644 index 00000000000..e660ea4992a --- /dev/null +++ b/docs/design/claude-code-web-shell-backend.md @@ -0,0 +1,144 @@ +# Claude Code 作为 Qwen 委派子 agent + +状态：选定产品设计；实现与验证仍在进行。 + +本文定义目标契约与验收条件，不是测试报告。接口、分派与执行器正在修订， +代码中存在某个字段或事件不代表端到端行为已经满足契约。 + +## 1. 产品范围 + +Qwen 保持父会话与任务编排者的身份，通过 Agent 工具把任务委派给 Claude Code。 +Claude 通过本机 ACP adapter 运行；父会话接收任务结果，用户观察子任务并可停止它。 +Web Shell 使用子 agent 展示入口，权限决策仍在父会话中完成。 + +本设计不增加可直接聊天的 Claude 对等后端，不增加独立 Claude workspace runtime， +不改变 workspace 的身份键或 HTTP 路由，也不替换 daemon 的 Qwen ACP 子进程。 +**既有 daemon external-tool guard 及其强制握手保持不变。** +Claude 子进程不因此自动获得 Qwen 内部工具策略的保护；两者的安全边界必须区分。 + +支持目标是普通前台、后台委派及同一存活执行器上的后续输入。 +不承诺 Claude 独立会话恢复、跨进程重启续跑、fork、Agent Team / teammate、 +其他厂商执行器或 `qwen/control/*` 扩展的替代实现。 +子任务面板不是独立聊天入口；查看已记录内容不等同于恢复外部执行器。 + +## 2. 定义与启动 + +agent 定义通过独立的 `executor` 字段选择执行器，不能把 `model` 当作后端选择器。 +示例放在 `.qwen/agents/claude-worker.md`；先由用户安装并固定所需 adapter 版本， +确认其可执行文件 `claude-agent-acp` 位于 PATH，并按 adapter 要求完成 Claude 认证： + +```markdown +--- +name: claude-worker +description: Delegate a bounded task to Claude Code +executor: + kind: acp + command: claude-agent-acp + args: [] +permissionMode: default +--- + +Complete the delegated task and report the result and any limitations. +``` + +`command` 是实际启动的 ACP adapter 可执行文件，不是假定存在的 `claude --acp`。 +运行时不得静默通过 `npx` 下载 latest，也不得替用户安装或升级 adapter。 +命令缺失、认证失败、握手失败或宿主未注入执行器都必须明确报错。 +无效的显式 `executor` 定义必须拒绝，不能丢弃该字段后转用 Qwen。 +同样不得在重试、后台续跑或其他分派路径中静默替换后端。 + +## 3. 分层与接口 + +实现边界对应以下文件；这些是代码位置，不是完成状态或测试证据： + +- `packages/core/src/agents/runtime/subagent-executor.ts`：执行器与工厂契约。 +- `packages/core/src/subagents/subagent-manager.ts`：定义解析、运行配置与分派入口。 +- `packages/cli/src/external-agents/acp-subagent-executor.ts`：子进程、ACP 与事件翻译。 + +core 定义 `SubagentExecutor`，CLI 注入 `ExternalAgentExecutor`；core 不承担 ACP transport。 +分派必须在创建 Qwen provider、工具注册表或执行定义 hooks 等副作用之前识别外部定义， +完成可信度与约束校验，再把派生的工作目录、提示、权限意图和运行配置传给 CLI。 +未声明外部执行器的定义继续使用 `AgentHeadless`。 + +执行器公共面包括 `execute`、`executeExternalInputs`、结果文本、终止原因、统计摘要、 +窄化的 `getCore` 视图、外部消息 provider，以及可选等待回调和资源释放方法。 +`getCore` 只提供事件 emitter、模型标签及可选运行视图，不伪造完整 `AgentCore`。 +模型标签必须标明外部执行器，不能把执行归到父 Qwen 模型。 +管理器返回的 `dispose` 必须组合执行器与宿主资源的清理责任。 + +提示模板由宿主渲染后与任务文本交给 adapter；通过 ACP 文本提交的指令 +不等同于 Claude 原生 system prompt。不能声称其优先级或上下文与 Qwen 完全一致。 +Qwen 的工具、skills、MCP、hooks 与模型配置也不会仅因传入接口就被 Claude 执行。 + +## 4. 信任与权限 + +外部定义会启动本机代码。项目来源的定义必须在工作区受信任后才能启动， +未受信任时不得先启动 adapter 再拒绝。子进程使用目标工作目录和过滤后的环境， +不继承 daemon bearer token 等 Qwen 内部秘密；环境过滤本身不是进程沙箱。 + +所有显式配置的限制必须可执行，不能静默忽略。工具白名单、禁用工具、 +执行限制、只读要求、MCP、hooks、模型选择及轮数限制等，若没有可靠映射或执行保障， +必须在启动前拒绝并指出不支持的约束；默认内部配置与用户显式要求须区分。 +仅在提示中要求“不要写文件”不能兑现只读策略。 + +权限模式必须由宿主的有效审批策略决定，子定义不能绕过父会话的限制。 +不能依赖 Claude 本地设置，也不能把 `session/new` 中的 `_meta.permissionMode` +视为已确认生效的协议契约。**计划修复是建会话后、首个 prompt 前调用原生 +`session/set_mode`，确认所需模式可用并设置成功；否则失败关闭。** +未知或不支持的显式模式必须拒绝，不能静默采用更宽松的本地默认值。 + +Claude 的 `session/request_permission` 转为宿主审批事件，经父 bridge session +路由到用户；不能把仅用于展示的 virtual subagent id 当成权限会话身份。 +响应必须选择 adapter 本次实际提供的 optionId；一次授权不得升级为永久授权。 +无监听者、无有效选项、未知审批结果、连接断开或取消时一律拒绝或取消， +并释放等待中的请求，不能挂起到无限期，也不能默认批准。 + +## 5. 生命周期与输入 + +每个外部执行器拥有自己的子进程、ACP 会话、待决权限与运行状态。 +启动、握手和模式设置须有期限，任一步失败都清理已创建资源。 +一次只允许一个受管理的活动 turn；并发输入必须序列化或明确拒绝。 + +已经取消的调用不得发起 prompt。活动取消先发送 ACP cancel， +若 adapter 不响应，须在有限宽限期后终止所拥有的进程及其子进程并等待回收。 +进程退出、连接损坏、超时和 dispose 都必须结束待决调用与权限等待。 +重复清理应安全，不能误杀其他会话；后台任务只在其管理器持有期间保留执行器。 + +后续输入必须使用相同的取消、统计、结果重置与终止事件路径。 +私有 steering 只有在能力明确支持时才可使用，不能让 adapter 自行启动不可跟踪的 turn。 +未支持 mid-turn 输入时不得假装消息已送达；空闲时的后续 prompt 仍须受生命周期管理。 +未知 stop reason、拒绝或截断不能无条件映射为成功完成。 + +## 6. 事件、记录与预算 + +ACP 更新翻译为 Qwen 的文本流、工具调用、结果、审批及终止事件， +供现有 transcript writer、virtual subagent session 和 Web Shell 消费。 +工具结果须保持 callId 与名称关联，重复终态不得重复计数；错误须可见且不崩溃父进程。 +每轮成功、失败和取消都必须产生一致终态，后续轮次不能混入上一轮的最终文本。 + +首期不承诺完整保真：文本和工具状态是基本目标，结构化工具输出、文件 diff、 +多模态内容及专用 edit/exec 审批展示只有经过映射与验收后才可宣称支持。 +只生成文本事件不代表已拥有完整 transcript；持久化、回放和 Stop 必须逐层验证。 + +`usage_update` 的上下文占用不是每轮生成 token 数，不能求和或当作 output tokens。 +未知 token 使用量必须表达为不可用，**不是免费执行或已知的零消耗**。 +若兼容统计结构暂存零，消费者仍须独立识别其不可计量状态。 +首期 workflow `agent()` 一律在启动前拒绝外部执行器定义：尚不能兑现 workflow 的 +预算、schema 和工具限制。普通 Agent 工具委派不受此门控影响。 +仅告警后继续运行不能保护预算；普通委派仍须遵守取消和时间限制。 + +## 7. 验收条件（待执行，不代表已通过） + +- 前台和后台委派确实启动指定 adapter，结果返回父会话，且不创建对等 runtime。 +- 无效定义、缺命令、缺注入、未知配置均明确失败；Qwen 替代执行次数为零。 +- 未受信任项目与不支持的显式约束在启动前失败，且无 hook、provider 或子进程副作用。 +- adapter 使用宽松本地默认时，仍在首个 prompt 前完成宿主要求的原生模式设置。 +- 模式不支持或设置失败时无 prompt；allow/deny、缺监听者与取消均按最小授权处理。 +- 正常完成、预取消、活动取消、不响应 cancel、子进程退出及重复 dispose 均能有限收敛。 +- 后续输入与并发调用不产生脱管 turn；状态、结果、权限等待和工具计数不跨轮污染。 +- Web Shell 可观察文本与工具状态、回放记录并停止任务，审批出现在父会话。 +- 有或没有 token 上限的 workflow 都在外部启动前拒绝；未知使用量不展示为已知零成本。 +- 既有 Qwen 委派、workspace 身份与 daemon guard 行为不变。 + +验收应结合针对性单测、可控制 ACP 对端的协议与失败路径测试，以及固定 adapter 版本的 +真实委派测试；保存可复查结果后再更新实现状态。本文不引用私人运行日志作为完成依据。 diff --git a/integration-tests/terminal-capture/skill-review-harness/text-capture.tsx b/integration-tests/terminal-capture/skill-review-harness/text-capture.tsx index a56c6ca13ab..e2692157d43 100644 --- a/integration-tests/terminal-capture/skill-review-harness/text-capture.tsx +++ b/integration-tests/terminal-capture/skill-review-harness/text-capture.tsx @@ -248,6 +248,7 @@ async function main() { ['subSessionConstants', 'tools/sub-session-constants.ts'], ['toolWriteOrigin', 'services/tool-write-origin.ts'], ['userPromptSubmitContext', 'hooks/user-prompt-submit-context.ts'], ['noFollowOpen', 'utils/no-follow-open.ts'], ['conversationsRuntimeMarker', 'utils/conversations-runtime-marker.ts'], + ['subagentRuntime', 'subagent-runtime.ts'], ['storage', 'config/storage.ts'], ['atomicFileWrite', 'utils/atomicFileWrite.ts'], ['debugLogger', 'utils/debugLogger.ts'], ]); diff --git a/integration-tests/tsconfig.json b/integration-tests/tsconfig.json index 45dfa216bda..482ab6e5c07 100644 --- a/integration-tests/tsconfig.json +++ b/integration-tests/tsconfig.json @@ -63,6 +63,9 @@ "@qwen-code/qwen-code-core/conversationsRuntimeMarker": [ "../packages/core/src/utils/conversations-runtime-marker.ts" ], + "@qwen-code/qwen-code-core/subagentRuntime": [ + "../packages/core/src/subagent-runtime.ts" + ], "@qwen-code/qwen-code-core/storage": [ "../packages/core/src/config/storage.ts" ], diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 6d177585221..94a5f6b224b 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -1194,6 +1194,22 @@ describe('loadCliConfig', () => { ]); }); + it('registers the external agent executor factory so executor definitions dispatch (R1-7)', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + + const config = await loadCliConfig({}, argv); + + // This single host-side registration is what the whole external-subagent + // feature dispatches through. Every dispatch test mocks + // getExternalAgentExecutor, so without this assertion deleting the + // injection would regress every valid executor definition to "registered no + // external agent executor" with the whole suite still green. + const factory = config.getExternalAgentExecutor(); + expect(factory).toBeDefined(); + expect(typeof factory?.create).toBe('function'); + }); + it('enables debug file logging for --debug when QWEN_DEBUG_LOG_FILE is unset', async () => { delete process.env['QWEN_DEBUG_LOG_FILE']; process.argv = ['node', 'script.js', '--debug']; diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 6157d62e686..bfa2ad7902e 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -2475,6 +2475,14 @@ export async function loadCliConfig( const config = new Config(configParams); + // Load the ACP transport only when an external subagent is requested. + config.setExternalAgentExecutor({ + create: (params) => + import('../external-agents/acp-subagent-executor.js').then((module) => + module.acpExternalAgentExecutor.create(params), + ), + }); + if (lspEnabled) { try { const lspService = new NativeLspService( diff --git a/packages/cli/src/external-agents/acp-subagent-executor.test.ts b/packages/cli/src/external-agents/acp-subagent-executor.test.ts new file mode 100644 index 00000000000..62aa5e4f9a4 --- /dev/null +++ b/packages/cli/src/external-agents/acp-subagent-executor.test.ts @@ -0,0 +1,1072 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + AgentEventEmitter, + AgentEventType, + AgentTerminateMode, + ContextState, + ToolConfirmationOutcome, + type ExternalAgentExecutorParams, + type SubagentExecutor, +} from '@qwen-code/qwen-code-core/subagentRuntime'; +import { Config, InputFormat } from '@qwen-code/qwen-code-core'; +import { + acpExternalAgentExecutor, + assertExternalAgentSpawnPlatformSupported, + externalModelLabel, + isExpectedExternalAgentCleanupExit, + isUnprovenExternalAgentTreeExit, + optionKindForOutcome, + resolvePermissionMode, + selectPeerModeId, + selectPermissionOption, + selectRejectOption, +} from './acp-subagent-executor.js'; + +const fixture = String.raw` +import { AgentSideConnection, ndJsonStream } from '@agentclientprotocol/sdk'; +import { Readable, Writable } from 'node:stream'; +import { spawn } from 'node:child_process'; +const scenario = process.argv[1]; +if (scenario === 'term-resistant' || scenario === 'late-after-timeout') + process.on('SIGTERM', () => {}); +let mode; +let prompts = 0; +let permissionCount = 0; +let connection; +const send = (update) => connection.sessionUpdate({sessionId:'fixture', update}); +connection = new AgentSideConnection(() => ({ + initialize: async () => { + if (scenario === 'init-exit') process.exit(3); + if (scenario === 'init-hang') return new Promise(() => {}); + return {protocolVersion:1, agentCapabilities:{}}; + }, + newSession: async () => { + if (scenario === 'session-exit') process.exit(4); + if (scenario === 'session-hang') return new Promise(() => {}); + return {sessionId:'fixture', modes:{currentModeId:'auto', availableModes:scenario === 'no-mode' ? [] : scenario === 'claude-modes' ? [{id:'default',name:'Ask'},{id:'acceptEdits',name:'AcceptEdits'},{id:'bypassPermissions',name:'Bypass'}] : scenario === 'qwen-modes' ? [{id:'plan'},{id:'default'},{id:'auto-edit'},{id:'auto'},{id:'yolo'}] : [{id:'default',name:'Ask'},{id:'plan',name:'Plan'}]}}; + }, + setSessionMode: async (params) => { + if (scenario === 'mode-error') throw new Error('cannot set mode'); + if (scenario === 'mode-hang') return new Promise(() => {}); + mode = params.modeId; + return {}; + }, + authenticate: async () => ({}), + cancel: async () => { await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:'CANCEL_RECEIVED'}}); }, + prompt: async (params) => { + prompts++; + const safeModes = scenario === 'claude-modes' ? ['default','acceptEdits','bypassPermissions'] : scenario === 'qwen-modes' ? ['plan','default','auto-edit','auto','yolo'] : ['default','plan']; + if (!safeModes.includes(mode)) throw new Error('PROMPT BEFORE SAFE MODE'); + if (scenario === 'claude-modes' || scenario === 'qwen-modes') { await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:'MODE:'+mode}}); return {stopReason:'end_turn'}; } + if (scenario === 'prompt-exit') process.exit(5); + if (scenario === 'prompt-close') { process.stdout.end(); return new Promise(() => {}); } + if (scenario === 'prompt-hang') return new Promise(() => {}); + if (scenario === 'unsupported-extension') { + try { + await connection.extMethod('_fixture/unsupported', {sessionId:'fixture'}); + throw new Error('Unsupported method unexpectedly succeeded'); + } catch (error) { + await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:JSON.stringify({code:error.code,message:error.message})}}); + } + return {stopReason:'end_turn'}; + } + if (scenario === 'tree') { + const descendant = spawn(process.execPath, ['-e','process.on("SIGTERM",()=>{});process.send("ready");setInterval(()=>{},1000)'], {stdio:['ignore','ignore','ignore','ipc']}); + await new Promise(resolve => descendant.once('message', resolve)); + await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:String(descendant.pid)}}); + return {stopReason:'end_turn'}; + } + if (scenario === 'tool-open-hang') { + await send({sessionUpdate:'tool_call',toolCallId:'open-1',title:'Shell',kind:'execute',status:'in_progress',rawInput:{command:'sleep'}}); + return new Promise(() => {}); + } + if (scenario === 'late-after-timeout') { + // Outlive the wall-time budget, then emit after the turn was declared + // over (during dispose's SIGTERM grace). The widened onSessionUpdate + // guard must drop these (R8-1). + await new Promise((resolve) => setTimeout(resolve, 400)); + await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:'LATE_AFTER_TIMEOUT'}}); + await send({sessionUpdate:'tool_call',toolCallId:'late-1',title:'Shell',kind:'execute',status:'in_progress',rawInput:{}}); + await send({sessionUpdate:'tool_call_update',toolCallId:'late-1',status:'completed'}); + return {stopReason:'end_turn'}; + } + if (scenario.startsWith('permission')) { + const toolName = scenario === 'permission-question' ? 'AskUserQuestion' : scenario === 'permission-question-snake' ? 'ask_user_question' : 'Write'; + const request = () => connection.requestPermission({sessionId:'fixture',toolCall:{toolCallId:'same',title:'Write',kind:'edit',rawInput:{command:'rm -rf ./build'},_meta:{claudeCode:{toolName}}},options:[{optionId:'once',name:'Once',kind:'allow_once'},{optionId:'always',name:'Always',kind:'allow_always'},{optionId:'no',name:'No',kind:'reject_once'}]}); + const result = scenario === 'permission-duplicate' ? await Promise.all([request(), request()]) : [await request()]; + await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:JSON.stringify(result)}}); + permissionCount++; + return {stopReason: result[0] && result[0].outcome.outcome === 'cancelled' ? 'cancelled' : 'end_turn'}; + } + if (scenario === 'env') { + await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:JSON.stringify(['QWEN_SERVER_TOKEN','QWEN_DAEMON_TOKEN','QWEN_CODE_EXTERNAL_TOOL_GUARD_TOKEN','QWEN_CODE_PRIVATE_ACP_CAPABILITY'].map(k => process.env[k] ?? null))}}); + return {stopReason:'end_turn'}; + } + await send({sessionUpdate:'agent_thought_chunk',content:{type:'text',text:'thinking'}}); + await send({sessionUpdate:'agent_message_chunk',content:{type:'text',text:params.prompt.map(p=>p.text).join('|')}}); + await send({sessionUpdate:'tool_call',toolCallId:'tool-'+prompts,title:'Shell',kind:'execute',status:'in_progress',rawInput:{}}); + await send({sessionUpdate:'tool_call_update',toolCallId:'tool-'+prompts,rawInput:{command:'printf hello'},_meta:{claudeCode:{toolName:'Bash'}},content:[{type:'content',content:{type:'text',text:'hello'}}]}); + await send({sessionUpdate:'tool_call_update',toolCallId:'tool-'+prompts,status:'completed'}); + await send({sessionUpdate:'tool_call_update',toolCallId:'tool-'+prompts,status:'completed'}); + return {stopReason:scenario === 'continuation-refusal' && prompts > 1 ? 'refusal' : scenario === 'max-tokens' ? 'max_tokens' : scenario === 'unknown-stop' ? 'future_reason' : 'end_turn'}; + }, +}), ndJsonStream(Writable.toWeb(process.stdout),Readable.toWeb(process.stdin))); +setInterval(()=>{},1000); +`; + +const executors: SubagentExecutor[] = []; +afterEach(async () => { + await Promise.all( + executors.splice(0).map((executor) => executor.dispose?.()), + ); + vi.restoreAllMocks(); + vi.unstubAllEnvs(); +}); + +function params(scenario = 'normal'): ExternalAgentExecutorParams { + const runtimeContext = new Config({ + sessionId: 'fixture-test', + targetDir: process.cwd(), + cwd: process.cwd(), + model: 'unused', + debugMode: false, + }); + vi.spyOn(runtimeContext, 'isInteractive').mockReturnValue(true); + return { + spec: { + kind: 'acp', + command: process.execPath, + args: ['--input-type=module', '-e', fixture, scenario], + }, + name: 'fixture', + promptConfig: { systemPrompt: '' }, + modelConfig: { model: 'unused' }, + runConfig: {}, + runtimeContext, + eventEmitter: new AgentEventEmitter(), + }; +} + +async function create( + options: ExternalAgentExecutorParams, +): Promise { + const executor = await acpExternalAgentExecutor.create(options); + executors.push(executor); + return executor; +} + +function context(text = 'task'): ContextState { + const state = new ContextState(); + state.set('task_prompt', text); + return state; +} + +describe('permission mapping', () => { + it('honors loader approval precedence and fails unknown modes towards asking', () => { + expect(resolvePermissionMode('bypassPermissions', 'plan')).toBe('plan'); + expect(resolvePermissionMode('auto', 'default')).toBe('default'); + expect(resolvePermissionMode('auto', 'auto-edit')).toBe('acceptEdits'); + expect(resolvePermissionMode(undefined, undefined)).toBe('default'); + expect(resolvePermissionMode('unknown', undefined)).toBe('default'); + }); + it('maps host approval vocabulary to policy tokens, never peer mode ids (R11-3)', () => { + expect(resolvePermissionMode('yolo', undefined)).toBe('bypass'); + expect(resolvePermissionMode('bypassPermissions', undefined)).toBe( + 'bypass', + ); + expect(resolvePermissionMode('auto', undefined)).toBe('acceptEdits'); + expect(resolvePermissionMode('auto-edit', undefined)).toBe('acceptEdits'); + }); + it('selects the peer-advertised alias for a policy, never a host token (R11-3)', () => { + // Claude vocabulary: host auto-edit maps to the peer's acceptEdits, never + // the qwen-only 'auto' (which a Claude peer does not even advertise). + expect( + selectPeerModeId('acceptEdits', [ + 'default', + 'acceptEdits', + 'bypassPermissions', + ]), + ).toBe('acceptEdits'); + // qwen vocabulary: host auto-edit maps to the peer's auto-edit. + expect( + selectPeerModeId('acceptEdits', ['auto-edit', 'auto', 'default']), + ).toBe('auto-edit'); + expect(selectPeerModeId('bypass', ['bypassPermissions'])).toBe( + 'bypassPermissions', + ); + expect(selectPeerModeId('bypass', ['yolo'])).toBe('yolo'); + // No advertised alias → undefined so the caller refuses, not guesses. + expect(selectPeerModeId('bypass', ['default', 'plan'])).toBeUndefined(); + }); + it('never grants for non-approval outcomes or widens a single approval', () => { + expect(optionKindForOutcome(ToolConfirmationOutcome.RestorePrevious)).toBe( + 'reject_once', + ); + expect(optionKindForOutcome(ToolConfirmationOutcome.ModifyWithEditor)).toBe( + 'reject_once', + ); + const options = [ + { optionId: 'always', kind: 'allow_always' }, + { optionId: 'no', kind: 'reject_once' }, + ]; + expect( + selectPermissionOption(options, ToolConfirmationOutcome.ProceedOnce), + ).toBe('no'); + expect( + selectPermissionOption(options, ToolConfirmationOutcome.Cancel), + ).toBeUndefined(); + expect( + selectPermissionOption( + [{ optionId: 'once', kind: 'allow_once' }], + ToolConfirmationOutcome.ProceedAlways, + ), + ).toBe('once'); + expect( + selectPermissionOption( + [ + { optionId: 'same', kind: 'allow_once' }, + { optionId: 'same', kind: 'reject_once' }, + ], + ToolConfirmationOutcome.ProceedOnce, + ), + ).toBeUndefined(); + }); + it('uses an external model label', () => { + expect(externalModelLabel('/usr/bin/claude')).toBe('external-acp:claude'); + expect(externalModelLabel('C:\\tools\\claude')).toBe('external-acp:claude'); + }); + it('selects the narrowest reject option and refuses to guess', () => { + const full = [ + { optionId: 'once', kind: 'allow_once' }, + { optionId: 'always', kind: 'allow_always' }, + { optionId: 'no', kind: 'reject_once' }, + ]; + // Prefers reject_once over reject_always (deny this call, not the session). + expect(selectRejectOption(full)).toBe('no'); + expect( + selectRejectOption([ + { optionId: 'never', kind: 'reject_always' }, + { optionId: 'once', kind: 'allow_once' }, + ]), + ).toBe('never'); + // No reject option at all → undefined, so the caller falls back to cancel + // rather than selecting an allow. + expect( + selectRejectOption([ + { optionId: 'once', kind: 'allow_once' }, + { optionId: 'always', kind: 'allow_always' }, + ]), + ).toBeUndefined(); + expect(selectRejectOption([])).toBeUndefined(); + // Ambiguous duplicate IDs → refuse to guess. + expect( + selectRejectOption([ + { optionId: 'same', kind: 'reject_once' }, + { optionId: 'same', kind: 'allow_once' }, + ]), + ).toBeUndefined(); + }); +}); + +describe('cleanup-exit classification', () => { + it("tolerates the peer's own unclean exit but not the unproven-tree race", () => { + // The peer's own exit status, for ANY code/signal: acp-bridge raises this + // only after the tree is proven gone, so dispose() tolerates it silently + // (R7-2). Letting dispose() reject on it would override the turn's + // classified terminal state. + for (const status of [ + 'code=none, signal=SIGTERM', + 'code=none, signal=SIGKILL', + 'code=5, signal=none', + 'code=1, signal=none', + 'code=none, signal=SIGHUP', + 'code=143, signal=none', + ]) { + const error = new Error( + `ACP child pid=4242 exited uncleanly during shutdown (${status})`, + ); + expect(isExpectedExternalAgentCleanupExit(error)).toBe(true); + expect(isUnprovenExternalAgentTreeExit(error)).toBe(false); + } + // The snapshot-race shape means the tree was NEVER enumerated (a detached + // descendant may survive), so it is reported, not silently swallowed + // (R10-3). isExpectedExternalAgentCleanupExit must be false so dispose() + // does not treat it as a clean exit; isUnprovenExternalAgentTreeExit routes + // it to the report-and-resolve path. + const snapshot = new Error( + 'ACP child pid=2628870 exited before its initial process-tree snapshot completed', + ); + expect(isExpectedExternalAgentCleanupExit(snapshot)).toBe(false); + expect(isUnprovenExternalAgentTreeExit(snapshot)).toBe(true); + }); + it('propagates every genuine cleanup-proof failure', () => { + const mustThrow = [ + // These are raised when the tree could NOT be proven gone — a descendant + // we owned may have survived, so they must never be swallowed. + 'ACP child pid=4242 process-tree snapshot exceeded 512 processes or depth 8', + 'ACP child pid=4242 was absent from the initial process-tree snapshot', + 'ACP child pid=4242 was not an isolated process-group leader', + 'ACP child pid=4242 process-tree snapshot failed: ps exited 1', + 'ACP child pid=4242 could not send SIGKILL to pgid=4242: EPERM', + 'ACP child pid=4242 could not inspect pgid=999: EACCES', + 'ACP child pid=4242 did not exit within 5000ms', + ]; + for (const message of mustThrow) { + // Neither silently tolerated nor report-and-resolved: these must rethrow. + expect(isExpectedExternalAgentCleanupExit(new Error(message))).toBe( + false, + ); + expect(isUnprovenExternalAgentTreeExit(new Error(message))).toBe(false); + } + }); + it('rejects non-Error and near-miss values', () => { + for (const predicate of [ + isExpectedExternalAgentCleanupExit, + isUnprovenExternalAgentTreeExit, + ]) { + expect(predicate(undefined)).toBe(false); + expect(predicate(null)).toBe(false); + } + // The snapshot shape's anchoring and pid validation now live in + // isUnprovenExternalAgentTreeExit (R10-3). + const tail = 'exited before its initial process-tree snapshot completed'; + expect(isUnprovenExternalAgentTreeExit(`ACP child pid=1 ${tail}`)).toBe( + false, + ); + expect( + isUnprovenExternalAgentTreeExit( + new Error(`prefix ACP child pid=1 ${tail}`), + ), + ).toBe(false); + expect( + isUnprovenExternalAgentTreeExit( + new Error(`ACP child pid=notanumber ${tail}`), + ), + ).toBe(false); + }); +}); + +// Real subprocess + real ProcessRegistry tree-kill. On Windows the registry +// kills via taskkill/SIGKILL and Node reports a numeric exit code with +// signalCode null, which dispose() surfaces as an unclean-exit error — the same +// exposure the repo's other real-process suites gate (process-registry.process, +// hook-runner.process). The pure-function suites above run on every platform. +describe('spawn platform guard (R3-1)', () => { + it('refuses with a clear POSIX-only error on win32, allows POSIX', () => { + // The guard throws BEFORE any spawn, so on win32 a user gets a clear + // "POSIX-only" refusal instead of a misleading `spawn ENOENT` (an + // npm-installed launcher resolving to a `.cmd` libuv never finds). + // Removing the win32 throw turns the first assertion red. + expect(() => assertExternalAgentSpawnPlatformSupported('win32')).toThrow( + /POSIX-only/, + ); + expect(() => + assertExternalAgentSpawnPlatformSupported('darwin'), + ).not.toThrow(); + expect(() => + assertExternalAgentSpawnPlatformSupported('linux'), + ).not.toThrow(); + }); + + it('create() rejects on win32 without spawning', async () => { + const realPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32' }); + try { + await expect(create(params())).rejects.toThrow(/POSIX-only/); + } finally { + Object.defineProperty(process, 'platform', { value: realPlatform }); + } + }); +}); +describe.skipIf(process.platform === 'win32')('real ACP subprocess', () => { + it('returns method-not-found -32601 across the real extension request wire', async () => { + const executor = await create(params('unsupported-extension')); + await executor.execute(context()); + expect(JSON.parse(executor.getFinalText())).toMatchObject({ code: -32601 }); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it('sets an advertised asking mode before prompt and preserves text/thought/tool transcript events', async () => { + const options = params(); + const text = vi.fn(); + const stream = vi.fn(); + const results = vi.fn(); + options.eventEmitter!.on(AgentEventType.ROUND_TEXT, text); + options.eventEmitter!.on(AgentEventType.STREAM_TEXT, stream); + options.eventEmitter!.on(AgentEventType.TOOL_RESPONSES_FINALIZED, results); + const executor = await create(options); + await executor.execute(context()); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ text: 'task', thoughtText: 'thinking' }), + ); + expect(stream).toHaveBeenCalledWith( + expect.objectContaining({ text: 'thinking', thought: true }), + ); + expect(results).toHaveBeenCalledTimes(1); + expect(JSON.stringify(results.mock.calls)).toContain('hello'); + expect( + results.mock.calls[0]![0].responses[0].responseParts[0].functionResponse, + ).toMatchObject({ + name: 'execute', + response: { input: { command: 'printf hello' }, toolName: 'Bash' }, + }); + expect(executor.getExecutionSummary()).toMatchObject({ + totalToolCalls: 1, + successfulToolCalls: 1, + rounds: 1, + }); + const duration = executor.getExecutionSummary().totalDurationMs; + await new Promise((resolve) => setTimeout(resolve, 25)); + expect(executor.getExecutionSummary().totalDurationMs).toBe(duration); + }); + it.each(['no-mode', 'mode-error', 'init-exit', 'session-exit'])( + 'fails creation for %s', + async (scenario) => { + await expect(create(params(scenario))).rejects.toThrow(); + }, + ); + it('drives the peer-advertised acceptEdits mode for a host auto-edit approval (R11-3)', async () => { + const options = params('claude-modes'); + options.approvalMode = 'auto-edit'; + const executor = await create(options); + await executor.execute(context()); + // The peer is told 'acceptEdits' (its own vocabulary), never the qwen-only + // 'auto'/'auto-edit' token — the fixture echoes the mode it received. + expect(executor.getFinalText()).toContain('MODE:acceptEdits'); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it('drives the peer-advertised bypassPermissions mode for a host yolo approval (R11-3)', async () => { + const options = params('claude-modes'); + options.approvalMode = 'yolo'; + const executor = await create(options); + await executor.execute(context()); + expect(executor.getFinalText()).toContain('MODE:bypassPermissions'); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it('refuses creation when the peer advertises no mode for the host bypass policy (R11-3)', async () => { + // The default fixture advertises only default+plan; a host yolo approval + // maps to the bypass policy, which has no advertised alias → connect must + // fail loudly (naming the policy), never silently weaken the mode. + const options = params(); + options.approvalMode = 'yolo'; + await expect(create(options)).rejects.toThrow(/bypass/); + }); + it('selects the qwen peer auto-edit mode for a host auto-edit approval (R11-3)', async () => { + // A qwen-vocabulary peer advertises auto-edit (NOT the Claude-only + // acceptEdits), so the old hard-coded 'acceptEdits' id made connect() throw + // and killed the delegation. The policy token + alias selection must find + // the peer's own auto-edit id and connect must succeed. + const options = params('qwen-modes'); + options.approvalMode = 'auto-edit'; + const executor = await create(options); + await executor.execute(context()); + expect(executor.getFinalText()).toContain('MODE:auto-edit'); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it('selects the qwen peer yolo mode for a host yolo approval, never a weaker auto (R11-3)', async () => { + // The old mapping sent host yolo → 'auto', which a qwen peer accepted — + // silently running a YOLO delegation in the strictly weaker AUTO mode (and + // against a Claude peer, 'auto' is not advertised, so it threw). The bypass + // policy must select the peer's yolo id. + const options = params('qwen-modes'); + options.approvalMode = 'yolo'; + const executor = await create(options); + await executor.execute(context()); + expect(executor.getFinalText()).toContain('MODE:yolo'); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it.each(['init-hang', 'session-hang', 'mode-hang'])( + 'bounds and reaps a %s handshake', + async (scenario) => { + await expect(create(params(scenario))).rejects.toThrow('timed out'); + }, + 15000, + ); + it.each(['prompt-exit', 'prompt-close'])( + 'rejects %s without an error listener', + async (scenario) => { + const executor = await create(params(scenario)); + await expect(executor.execute(context())).rejects.toThrow(); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.ERROR); + }, + ); + it.each(['max-tokens', 'unknown-stop'])( + 'never certifies %s as success', + async (scenario) => { + const executor = await create(params(scenario)); + await executor.execute(context()); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.ERROR); + }, + ); + it('preserves both external-input variants and updates continuation lifecycle and stats', async () => { + const options = params('continuation-refusal'); + const finish = vi.fn(); + const external = vi.fn(); + options.eventEmitter!.on(AgentEventType.FINISH, finish); + options.eventEmitter!.on(AgentEventType.EXTERNAL_MESSAGE, external); + const executor = await create(options); + await executor.execute(context('first')); + await executor.executeExternalInputs( + ['second', { kind: 'notification', text: 'notice' }], + undefined, + { resetStats: false }, + ); + expect(executor.getFinalText()).toBe('second|notice'); + // The continuation peer returned ACP `refusal` — the agent declined, which + // is a failure the parent must see, not a user cancel (R6-2). + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.ERROR); + expect(executor.getExecutionSummary()).toMatchObject({ + rounds: 2, + totalToolCalls: 2, + }); + expect(finish).toHaveBeenCalledTimes(2); + expect(external).toHaveBeenCalledTimes(2); + await executor.execute(context('fresh')); + expect(executor.getExecutionSummary()).toMatchObject({ + rounds: 1, + totalToolCalls: 1, + }); + }); + it('records the user-side transcript entry on a continuation execute() turn', async () => { + const options = params(); + const external = vi.fn(); + options.eventEmitter!.on(AgentEventType.EXTERNAL_MESSAGE, external); + const executor = await create(options); + await executor.execute(context('first')); + // The first turn's task is seeded into the transcript as the initial user + // prompt by the dispatcher, so execute() must not double-record it. + expect(external).not.toHaveBeenCalled(); + await executor.execute(context('second')); + // A resident external agent re-invoked per incoming user message must + // record each continuation task, or the JSONL transcript loses every + // message after the first and round 2 shows an answer to an unasked + // question (R6-1). The in-process sibling emits the same event. + expect(external).toHaveBeenCalledTimes(1); + expect(external).toHaveBeenCalledWith( + expect.objectContaining({ kind: 'message', text: 'second' }), + ); + }); + it('flushes a tool call left open when the turn is cancelled', async () => { + const options = params('tool-open-hang'); + const results = vi.fn(); + const finalized = vi.fn(); + options.eventEmitter!.on(AgentEventType.TOOL_RESULT, results); + options.eventEmitter!.on( + AgentEventType.TOOL_RESPONSES_FINALIZED, + finalized, + ); + const abort = new AbortController(); + // Cancel once the peer's in-progress tool_call has been recorded, so the + // turn ends with that tool still open (R7-3). + options.eventEmitter!.on(AgentEventType.TOOL_CALL, () => abort.abort()); + const executor = await create(options); + await executor.execute(context(), abort.signal); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.CANCELLED); + // The open tool gets a terminal failed TOOL_RESULT + a finalized response, + // so the inline frame / Web Shell row stop showing it as executing, the + // JSONL functionCall is paired, and the FINISH totals sum. + expect(results).toHaveBeenCalledWith( + expect.objectContaining({ callId: 'open-1', success: false }), + ); + expect(finalized).toHaveBeenCalledWith( + expect.objectContaining({ + responses: [ + expect.objectContaining({ + callId: 'open-1', + responseParts: [ + expect.objectContaining({ + functionResponse: expect.objectContaining({ id: 'open-1' }), + }), + ], + }), + ], + }), + ); + const summary = executor.getExecutionSummary(); + expect(summary.totalToolCalls).toBe(1); + expect(summary.failedToolCalls).toBe(1); + expect(summary.successfulToolCalls).toBe(0); + }); + it('drains registered external messages before reporting completion', async () => { + const executor = await create(params()); + const provider = vi + .fn() + .mockReturnValueOnce(['queued']) + .mockReturnValue([]); + executor.setExternalMessageProvider(provider); + await executor.execute(context('first')); + expect(executor.getFinalText()).toContain('queued'); + expect(executor.getExecutionSummary().rounds).toBe(2); + }); + it('does not dispatch a continuation prompt once the wall-time budget is spent (R9-1)', async () => { + const options = params(); + // An explicit cap: an absent max_time_minutes now means "no cap" (R10-5), so + // the budget logic this test exercises needs a real value. + options.runConfig.max_time_minutes = 10; + const executor = await create(options); + // Stub the connection so prompt() is countable and resolves end_turn + // immediately; the first prompt also advances the mocked clock past the + // 10-minute budget, simulating a peer that consumed the whole wall time on + // round 1. A message is queued, so without a budget check the loop would + // dispatch round 2 with `remaining` clamped to 0 — billing a prompt it then + // abandons (wait's 0ms timer rejects with ExternalAgentTimeoutError) and + // killing the peer in dispose(), after emitInputs already recorded the + // queued message as delivered. The guard must stop before that dispatch. + const realNow = Date.now(); + let mockNow = realNow; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => mockNow); + const prompt = vi.fn().mockImplementation(() => { + mockNow = realNow + 11 * 60_000; + return Promise.resolve({ stopReason: 'end_turn' }); + }); + ( + executor as unknown as { + connection: { prompt: typeof prompt; cancel: () => Promise }; + } + ).connection = { prompt, cancel: async () => {} }; + executor.setExternalMessageProvider( + vi.fn().mockReturnValueOnce(['queued']).mockReturnValue([]), + ); + await executor.execute(context('first')); + // Removing the budget guard turns both red: round 2 dispatches (prompt is + // called twice) and resolves end_turn against the empty queue, so the turn + // ends GOAL instead of TIMEOUT. + expect(prompt).toHaveBeenCalledTimes(1); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.TIMEOUT); + nowSpy.mockRestore(); + }); + it('does not drain or record a queued message it has no budget to dispatch (R10-4)', async () => { + const options = params(); + options.runConfig.max_time_minutes = 10; + const delivered: unknown[] = []; + options.eventEmitter!.on(AgentEventType.EXTERNAL_MESSAGE, (event) => + delivered.push(event), + ); + const executor = await create(options); + // Spend the budget during round 1 (the R9-1 construction) and back the + // provider with a mutable queue that drains destructively, like + // registry.drainMessages. Round 1 returns end_turn with a message waiting. + const realNow = Date.now(); + let mockNow = realNow; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => mockNow); + const prompt = vi.fn().mockImplementation(() => { + mockNow = realNow + 11 * 60_000; + return Promise.resolve({ stopReason: 'end_turn' }); + }); + ( + executor as unknown as { + connection: { prompt: typeof prompt; cancel: () => Promise }; + } + ).connection = { prompt, cancel: async () => {} }; + const queue = ['USER MESSAGE FROM PARENT']; + const provider = vi.fn().mockImplementation(() => queue.splice(0)); + executor.setExternalMessageProvider(provider); + await executor.execute(context('first')); + // The budget is spent before round 2, so the loop must break BEFORE the + // destructive drain: the queue still holds the message, the provider was + // never called, and no EXTERNAL_MESSAGE certified a delivery that never + // reached the peer. Removing the pre-drain budget/abort check turns these + // red — the queue empties, the provider runs, and the message is recorded as + // delivered while `next` is dropped at the round-top guard. + expect(queue).toEqual(['USER MESSAGE FROM PARENT']); + expect(provider).not.toHaveBeenCalled(); + expect(delivered).toEqual([]); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.TIMEOUT); + nowSpy.mockRestore(); + }); + it('enforces the wall-time budget cumulatively across resetStats:false continuations (R11-2)', async () => { + const options = params(); + options.runConfig.max_time_minutes = 10; + const executor = await create(options); + const realNow = Date.now(); + let mockNow = realNow; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => mockNow); + // Each prompt burns 7 of the 10 budget minutes (mocked clock), resolving + // end_turn immediately so wait() never times out the in-flight round. + const prompt = vi.fn().mockImplementation(() => { + mockNow += 7 * 60_000; + return Promise.resolve({ stopReason: 'end_turn' }); + }); + ( + executor as unknown as { + connection: { prompt: typeof prompt; cancel: () => Promise }; + } + ).connection = { prompt, cancel: async () => {} }; + // Turn 1 (fresh, resetStats default): burns 7min < 10min cap, ends GOAL. + await executor.execute(context('first')); + expect(prompt).toHaveBeenCalledTimes(1); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + // Turn 2 (continuation, stats preserved): the cumulative clock starts at + // 7min, leaving only 3min. Its prompt burns 7min more, pushing the + // cumulative elapsed (14min) past the 10min cap, so the turn is cut off + // TIMEOUT. Without subtracting this.durationMs, turn 2 would get a fresh + // per-turn 10min budget, its 7min prompt would fit, and it would end GOAL — + // a continued agent overrunning the whole-delegation cap. + await executor.execute(context('second'), undefined, { resetStats: false }); + expect(prompt).toHaveBeenCalledTimes(2); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.TIMEOUT); + nowSpy.mockRestore(); + }); + it('sends the system-prompt bundle only on the first turn, not continuations (R12-1)', async () => { + const options = params(); + options.promptConfig.systemPrompt = 'SYSTEM PROMPT SENTINEL'; + const executor = await create(options); + const payloads: string[][] = []; + const prompt = vi + .fn() + .mockImplementation((p: { prompt: Array<{ text: string }> }) => { + payloads.push(p.prompt.map((block) => block.text)); + return Promise.resolve({ stopReason: 'end_turn' }); + }); + ( + executor as unknown as { + connection: { prompt: typeof prompt; cancel: () => Promise }; + } + ).connection = { prompt, cancel: async () => {} }; + // Turn 1: the bundle (system + task) is the only channel the instruction has. + await executor.execute(context('first')); + // Turn 2 (continuation on the same live session): only the task is sent — + // the session already holds the bundle. Re-sending it would duplicate the + // whole bundle per turn. Removing the gate turns this red (turn 2 gets 2 + // blocks again). + await executor.execute(context('second')); + expect(payloads.length).toBe(2); + expect(payloads[0].length).toBe(2); + expect(payloads[0][0]).toContain('SYSTEM PROMPT SENTINEL'); + expect(payloads[0][1]).toBe('first'); + expect(payloads[1]).toEqual(['second']); + }); + it('does not record entry inputs as delivered when the budget guard breaks a continuation (R12-2)', async () => { + const options = params(); + options.runConfig.max_time_minutes = 10; + const executor = await create(options); + const realNow = Date.now(); + let mockNow = realNow; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => mockNow); + // Turn 1's single prompt burns 11 minutes — past the 10-minute cap. + const prompt = vi.fn().mockImplementation(() => { + mockNow += 11 * 60_000; + return Promise.resolve({ stopReason: 'end_turn' }); + }); + ( + executor as unknown as { + connection: { prompt: typeof prompt; cancel: () => Promise }; + } + ).connection = { prompt, cancel: async () => {} }; + const delivered: string[] = []; + options.eventEmitter!.on(AgentEventType.EXTERNAL_MESSAGE, (event) => { + delivered.push(String((event as { text?: unknown }).text)); + }); + // Turn 1 ends via the round-bottom budget guard (TIMEOUT); durationMs=11min. + await executor.execute(context('first')); + expect(prompt).toHaveBeenCalledTimes(1); + // Turn 2 (continuation, resetStats:false): the cumulative budget is already + // spent, so the round-top guard breaks TIMEOUT before any dispatch. The entry + // input ('second') must NOT be recorded as delivered — the peer never gets + // it. Moving the emit back above the guard turns this red (delivered becomes + // ['second']). + await executor.execute(context('second'), undefined, { resetStats: false }); + expect(prompt).toHaveBeenCalledTimes(1); + expect(delivered).toEqual([]); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.TIMEOUT); + nowSpy.mockRestore(); + }); + it('does not send a prompt for an already-aborted turn', async () => { + const executor = await create(params('prompt-exit')); + await executor.execute(context(), AbortSignal.abort()); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.CANCELLED); + expect(executor.getFinalText()).toBe(''); + }); + it('cancels and reaps an uncooperative continuation', async () => { + const executor = await create(params('prompt-hang')); + const abort = new AbortController(); + const running = executor.executeExternalInputs(['continue'], abort.signal); + setTimeout(() => abort.abort(), 50); + await running; + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.CANCELLED); + }); + it('enforces the configured wall time', async () => { + const options = params('prompt-hang'); + options.runConfig.max_time_minutes = 0.001; + const executor = await create(options); + await executor.execute(context()); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.TIMEOUT); + }); + it('drops session updates that arrive after a timeout declared the turn over', async () => { + const options = params('late-after-timeout'); + options.runConfig.max_time_minutes = 0.002; // 120ms budget + const executor = await create(options); + await executor.execute(context()); + // The turn times out at ~120ms; the SIGTERM-resistant peer then emits a + // chunk and a completing tool_call at ~400ms, during dispose's grace window + // while `executing` is still true and `cancelled` is false (the timeout path + // never sets it). The widened guard must drop them, so the late text never + // reaches finalText (the TIMEOUT path hands it to the parent verbatim) and + // the late tool never inflates the FINISH totals (R8-1). + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.TIMEOUT); + expect(executor.getFinalText()).not.toContain('LATE_AFTER_TIMEOUT'); + expect(executor.getExecutionSummary().totalToolCalls).toBe(0); + expect(executor.getExecutionSummary().successfulToolCalls).toBe(0); + }, 15000); + it('installs no wall-time deadline when max_time_minutes is absent (R10-5)', async () => { + // An absent max_time_minutes means "no cap" everywhere else in the codebase + // (agent-core installs no timer when maxTimeMinutes is falsy), so the + // external executor must not borrow a 10-minute default: that would make the + // same definition behave differently based only on whether it declares an + // executor, and would make a TIMEOUT turn — handed to the parent as the + // answer with no truncation marker — reachable by default. wait() already + // treats an undefined budget as "no timer". Reinstating `?? 10` turns this + // red (a ~600000ms timer is installed). + const executor = await create(params()); + const timers = vi.spyOn(globalThis, 'setTimeout'); + await executor.execute(context()); + const tenMinute = timers.mock.calls.find( + ([, delay]) => + typeof delay === 'number' && delay > 599_000 && delay <= 600_000, + ); + expect(tenMinute).toBeUndefined(); + }); + it('reports an unproven process tree on disposal instead of swallowing it (R10-3)', async () => { + const options = params(); + const errors: string[] = []; + options.eventEmitter!.on(AgentEventType.ERROR, (event) => + errors.push(String((event as { error?: unknown }).error)), + ); + const executor = await create(options); + // Stub the tracked child so terminate() reaps the real peer first (no leaked + // subprocess) and then rejects with the snapshot-race proof error — the shape + // acp-bridge raises when the root exited before the tree was enumerated, so a + // detached descendant may survive. dispose() must report it (not swallow it + // silently) and must NOT rethrow: it is awaited between the turn's + // terminal-state classification and the return, so rethrowing would replace a + // classified TIMEOUT/CANCELLED with a thrown ERROR. + const real = ( + executor as unknown as { child: { terminate: () => Promise } } + ).child; + ( + executor as unknown as { child: { terminate: () => Promise } } + ).child = { + terminate: async () => { + await real.terminate().catch(() => {}); + throw new Error( + 'ACP child pid=4242 exited before its initial process-tree snapshot completed', + ); + }, + }; + await expect(executor.dispose!()).resolves.toBeUndefined(); + expect(errors.some((message) => message.includes('not proven gone'))).toBe( + true, + ); + }); + it('refuses an unenforceable internal turn limit', async () => { + const options = params(); + options.runConfig.max_turns = 1; + await expect(create(options)).rejects.toThrow('max_turns'); + }); + it.each([NaN, Infinity, -1, 0, 100_000])( + 'rejects invalid timeout %s before spawning', + async (minutes) => { + const options = params(); + options.spec.command = '/nonexistent/external-agent'; + options.runConfig.max_time_minutes = minutes; + await expect(create(options)).rejects.toThrow('max_time_minutes'); + }, + ); + it('cleans up when a START subscriber throws', async () => { + const options = params(); + options.eventEmitter!.on(AgentEventType.START, () => { + throw new Error('start listener failed'); + }); + const executor = await create(options); + await expect(executor.execute(context())).rejects.toThrow( + 'start listener failed', + ); + await expect(executor.execute(context())).rejects.toThrow('closed'); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.ERROR); + }); + it.each(['permission-question', 'permission-question-snake'])( + 'denies routed interactive question %s', + async (scenario) => { + const options = params(scenario); + const approval = vi.fn(); + options.eventEmitter!.on(AgentEventType.TOOL_WAITING_APPROVAL, approval); + const executor = await create(options); + await executor.execute(context()); + // Denies the TOOL (selects reject_once), not the TURN (`cancelled`). + expect(JSON.parse(executor.getFinalText())).toEqual([ + { outcome: { outcome: 'selected', optionId: 'no' } }, + ]); + expect(approval).not.toHaveBeenCalled(); + }, + ); + it('denies when no approval listener exists', async () => { + const executor = await create(params('permission')); + await executor.execute(context()); + expect(JSON.parse(executor.getFinalText())).toEqual([ + { outcome: { outcome: 'selected', optionId: 'no' } }, + ]); + }); + it('denies the TOOL, not the TURN, when the user explicitly cancels (R8-3)', async () => { + const options = params('permission'); + options.eventEmitter!.on( + AgentEventType.TOOL_WAITING_APPROVAL, + async (event) => event.respond(ToolConfirmationOutcome.Cancel), + ); + const executor = await create(options); + await executor.execute(context()); + // An explicit user rejection of this one tool selects the peer's + // reject_once option, so the peer continues and ends the turn normally. + // Routing Cancel to ACP `cancelled` instead would abort the whole delegated + // turn (the peer answers stopReason `cancelled` → CANCELLED), abandoning the + // remaining work and billing a fresh prompt to re-delegate. + expect(JSON.parse(executor.getFinalText())).toEqual([ + { outcome: { outcome: 'selected', optionId: 'no' } }, + ]); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.GOAL); + }); + it('denies when the runtime disallows permission prompts even with a listener', async () => { + const options = params('permission'); + vi.spyOn( + options.runtimeContext, + 'getShouldAvoidPermissionPrompts', + ).mockReturnValue(true); + const approval = vi.fn(); + options.eventEmitter!.on(AgentEventType.TOOL_WAITING_APPROVAL, approval); + const executor = await create(options); + await executor.execute(context()); + expect(JSON.parse(executor.getFinalText())).toEqual([ + { outcome: { outcome: 'selected', optionId: 'no' } }, + ]); + expect(approval).not.toHaveBeenCalled(); + }); + it('denies plain headless requests despite a display-only approval listener', async () => { + const options = params('permission'); + vi.spyOn(options.runtimeContext, 'isInteractive').mockReturnValue(false); + vi.spyOn( + options.runtimeContext, + 'getExperimentalZedIntegration', + ).mockReturnValue(false); + vi.spyOn(options.runtimeContext, 'getInputFormat').mockReturnValue( + InputFormat.TEXT, + ); + vi.spyOn( + options.runtimeContext, + 'getShouldAvoidPermissionPrompts', + ).mockReturnValue(false); + const displayOnly = vi.fn(); + options.eventEmitter!.on(AgentEventType.TOOL_WAITING_APPROVAL, displayOnly); + const executor = await create(options); + await executor.execute(context()); + expect(JSON.parse(executor.getFinalText())).toEqual([ + { outcome: { outcome: 'selected', optionId: 'no' } }, + ]); + expect(displayOnly).not.toHaveBeenCalled(); + }); + it('allows stream-json approval responders', async () => { + const options = params('permission'); + vi.spyOn(options.runtimeContext, 'isInteractive').mockReturnValue(false); + vi.spyOn(options.runtimeContext, 'getInputFormat').mockReturnValue( + InputFormat.STREAM_JSON, + ); + options.eventEmitter!.on( + AgentEventType.TOOL_WAITING_APPROVAL, + async (event) => event.respond(ToolConfirmationOutcome.ProceedOnce), + ); + const executor = await create(options); + await executor.execute(context()); + expect(executor.getFinalText()).toContain('"optionId":"once"'); + }); + it('carries the action arguments into the approval confirmation (R11-5)', async () => { + const options = params('permission'); + let seenDetails: + | { prompt?: unknown; renderPromptAsPlainText?: unknown } + | undefined; + options.eventEmitter!.on( + AgentEventType.TOOL_WAITING_APPROVAL, + async (event) => { + seenDetails = ( + event as { + confirmationDetails?: { + prompt?: unknown; + renderPromptAsPlainText?: unknown; + }; + } + ).confirmationDetails; + await event.respond(ToolConfirmationOutcome.ProceedOnce); + }, + ); + const executor = await create(options); + await executor.execute(context()); + // The approval dialog must show WHAT the user is authorizing — the action's + // arguments — not just the tool title and the option labels the dialog + // already renders as buttons. The fixture peer asks permission for a Write + // whose rawInput carries {command: 'rm -rf ./build'}. Reverting the prompt + // back to the option-names join turns this red (no command string). + expect(String(seenDetails?.prompt)).toContain('rm -rf ./build'); + // The payload is foreign-process text, so the dialog must render it as plain + // text — otherwise markdown in it (a glob `**`, a `[label](url)`) is eaten + // or mis-rendered, misrepresenting what the user approves. Removing the flag + // turns this red. (R11-5 fix) + expect(seenDetails?.renderPromptAsPlainText).toBe(true); + }); + it('isolates duplicate outstanding tool IDs and ignores stale approval callbacks', async () => { + const options = params('permission-duplicate'); + const callbacks: Array<() => Promise> = []; + options.eventEmitter!.on(AgentEventType.TOOL_WAITING_APPROVAL, (event) => { + callbacks.push(() => event.respond(ToolConfirmationOutcome.ProceedOnce)); + setTimeout( + () => void event.respond(ToolConfirmationOutcome.ProceedOnce), + 30, + ); + }); + const executor = await create(options); + await executor.execute(context()); + expect(JSON.parse(executor.getFinalText())).toEqual([ + { outcome: { outcome: 'selected', optionId: 'once' } }, + { outcome: { outcome: 'cancelled' } }, + ]); + await callbacks[0]!(); + }); + it('releases pending permission requests on cancellation', async () => { + const options = params('permission'); + const abort = new AbortController(); + options.eventEmitter!.on(AgentEventType.TOOL_WAITING_APPROVAL, () => + abort.abort(), + ); + const executor = await create(options); + await executor.execute(context(), abort.signal); + expect(executor.getTerminateMode()).toBe(AgentTerminateMode.CANCELLED); + }); + it('scrubs all internal credentials in the real child environment', async () => { + for (const name of [ + 'QWEN_SERVER_TOKEN', + 'QWEN_DAEMON_TOKEN', + 'QWEN_CODE_EXTERNAL_TOOL_GUARD_TOKEN', + 'QWEN_CODE_PRIVATE_ACP_CAPABILITY', + ]) + vi.stubEnv(name, 'secret'); + const executor = await create(params('env')); + await executor.execute(context()); + expect(JSON.parse(executor.getFinalText())).toEqual([ + null, + null, + null, + null, + ]); + }); + it('awaits verified disposal of a SIGTERM-resistant descendant', async () => { + const executor = await create(params('tree')); + await executor.execute(context()); + const pid = Number(executor.getFinalText()); + expect(pid).toBeGreaterThan(0); + process.kill(pid, 0); + await executor.dispose?.(); + expect(() => process.kill(pid, 0)).toThrow(); + }, 15000); +}); diff --git a/packages/cli/src/external-agents/acp-subagent-executor.ts b/packages/cli/src/external-agents/acp-subagent-executor.ts new file mode 100644 index 00000000000..4aed6b106e7 --- /dev/null +++ b/packages/cli/src/external-agents/acp-subagent-executor.ts @@ -0,0 +1,1163 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { spawn } from 'node:child_process'; +import { Readable, Writable } from 'node:stream'; +import { + ClientSideConnection, + ndJsonStream, + PROTOCOL_VERSION, + RequestError, + type Client, + type ContentBlock, + type RequestPermissionRequest, + type RequestPermissionResponse, + type SessionNotification, + type ToolCall, + type ToolCallUpdate, +} from '@agentclientprotocol/sdk'; +import { + AgentEventEmitter, + AgentEventType, + AgentTerminateMode, + ToolConfirmationOutcome, + renderSubagentSystemPrompt, + type AgentExternalInput, + type AgentStatsSummary, + type ContextState, + type ExternalAgentExecutor, + type ExternalAgentExecutorParams, + type SubagentExecutor, + type SubagentExecutorCore, +} from '@qwen-code/qwen-code-core/subagentRuntime'; +import { InputFormat, sanitizeChildEnv } from '@qwen-code/qwen-code-core'; +import { createStderrForwarder } from '@qwen-code/acp-bridge/spawnChannel'; +import { + ProcessRegistry, + type TrackedChildProcess, +} from '@qwen-code/acp-bridge/processRegistry'; +import { createDebugLogger } from '@qwen-code/qwen-code-core/utils/debugLogger.js'; + +const INIT_TIMEOUT_MS = 10_000; +const debugLogger = createDebugLogger('EXTERNAL_AGENT'); + +export function externalModelLabel(command: string): string { + return `external-acp:${command.split(/[\\/]/).pop() ?? command}`; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +const EXTERNAL_ACTION_PROMPT_MAX = 300; + +function stripControlChars(value: string): string { + // Strip C0/C1 control characters and DEL; the toolCall payload is foreign-process + // data that must never be rendered as markup or terminal escapes. Done by code + // point rather than a control-char regex so the source carries no literal control + // bytes. + let out = ''; + for (const ch of value) { + const code = ch.codePointAt(0) ?? 0; + out += + code >= 0x20 && code !== 0x7f && !(code >= 0x80 && code <= 0x9f) + ? ch + : ' '; + } + return out; +} + +/** + * Renders an external action's arguments into the approval prompt so the user + * sees WHAT they are authorizing, not just the option labels. `rawInput` is an + * unvalidated payload from a foreign process, so the rendering is bounded in + * length and control characters are stripped — never forwarded as markup (R11-5). + */ +export function describeExternalAction( + toolCall: RequestPermissionRequest['toolCall'], +): string { + const title = stripControlChars( + toolCall.title ?? toolCall.kind ?? 'External action', + ); + const raw = toolCall.rawInput; + if (!isRecord(raw)) return title; + const detail = stripControlChars(JSON.stringify(raw)); + const rendered = `${title}: ${detail}`; + return rendered.length > EXTERNAL_ACTION_PROMPT_MAX + ? `${rendered.slice(0, EXTERNAL_ACTION_PROMPT_MAX)}…` + : rendered; +} + +/** + * R3-1: the executor spawn is POSIX-only in this release. On Windows an + * npm-installed launcher resolves to a `.cmd`/`.bat`, which libuv's PATH search + * never finds (bare name + `.exe` only) and which Node >= 18.20.2 refuses to + * spawn without a shell — and the process-tree reaping (detached process-group + * SIGTERM/SIGKILL) is POSIX-specific. Rather than let the spawn fail with a + * misleading `spawn ENOENT` (reporting an installed adapter as missing), + * fail closed here with a clear, actionable message. Windows support (a + * cross-spawn-style PATHEXT resolution + quoted `cmd.exe` arm) is a tracked + * follow-up. + */ +export function assertExternalAgentSpawnPlatformSupported( + platform: NodeJS.Platform = process.platform, +): void { + if (platform === 'win32') { + throw new Error( + 'External ACP agents are not supported on Windows in this release: the executor spawn is POSIX-only. ' + + 'An npm-installed adapter resolves to a `.cmd` launcher that cannot be spawned without a shell, and the process-tree reaping is POSIX-specific. ' + + 'Run the external agent on macOS/Linux (or WSL). Windows support is a tracked follow-up.', + ); + } +} + +/** + * The host approval policy as a peer-vocabulary-independent token. Peer ACP mode + * ids differ per peer (Claude: `acceptEdits` / `bypassPermissions`; qwen: + * `auto-edit` / `auto` / `yolo`), so resolvePermissionMode maps the host policy + * to a TOKEN and connect() picks the id the peer actually advertises via + * `POLICY_MODE_ALIASES` (R11-3) — a new peer vocabulary needs no new case here. + * The mapping never WIDENS the host policy: a weaker host mode must not select a + * stronger peer mode. + */ +export type ExternalPermissionPolicy = + | 'default' + | 'plan' + | 'acceptEdits' + | 'bypass'; + +export function resolvePermissionMode( + permissionMode: string | undefined, + approvalMode: string | undefined, +): ExternalPermissionPolicy { + switch ((approvalMode ?? permissionMode ?? '').trim().toLowerCase()) { + case 'auto': + case 'acceptedits': + case 'auto-edit': + return 'acceptEdits'; + case 'plan': + return 'plan'; + case 'bypasspermissions': + case 'bypass': + case 'yolo': + return 'bypass'; + default: + return 'default'; + } +} + +// The ranked peer-mode-id aliases per host policy token, canonical-first. +// connect() selects the first alias the peer advertises and refuses if none is, +// so a host policy that has no peer counterpart fails loudly (naming the policy) +// instead of silently running the peer in a weaker mode. +const POLICY_MODE_ALIASES: Record = { + bypass: ['bypassPermissions', 'yolo'], + acceptEdits: ['acceptEdits', 'auto-edit', 'auto'], + plan: ['plan'], + default: ['default'], +}; + +export function selectPeerModeId( + policy: ExternalPermissionPolicy, + availableModeIds: readonly string[], +): string | undefined { + return POLICY_MODE_ALIASES[policy].find((id) => + availableModeIds.includes(id), + ); +} + +export function optionKindForOutcome( + outcome: ToolConfirmationOutcome, +): 'allow_once' | 'allow_always' | 'reject_once' { + switch (outcome) { + case ToolConfirmationOutcome.ProceedOnce: + case ToolConfirmationOutcome.ProceedOnceAndSwitchToDefault: + return 'allow_once'; + case ToolConfirmationOutcome.ProceedAlways: + case ToolConfirmationOutcome.ProceedAlwaysServer: + case ToolConfirmationOutcome.ProceedAlwaysTool: + case ToolConfirmationOutcome.ProceedAlwaysProject: + case ToolConfirmationOutcome.ProceedAlwaysUser: + return 'allow_always'; + case ToolConfirmationOutcome.ModifyWithEditor: + case ToolConfirmationOutcome.Cancel: + case ToolConfirmationOutcome.RestorePrevious: + return 'reject_once'; + default: { + const exhaustive: never = outcome; + void exhaustive; + return 'reject_once'; + } + } +} + +export function selectPermissionOption( + options: ReadonlyArray<{ optionId: string; kind: unknown }>, + outcome: ToolConfirmationOutcome, +): string | undefined { + if (outcome === ToolConfirmationOutcome.Cancel) return undefined; + // An ambiguous option ID could turn a selected rejection into a grant. + if ( + new Set(options.map((option) => option.optionId)).size !== options.length + ) { + return undefined; + } + const kind = optionKindForOutcome(outcome); + return ( + options.find((option) => option.kind === kind) ?? + (kind === 'allow_always' + ? options.find((option) => option.kind === 'allow_once') + : undefined) ?? + options.find((option) => option.kind === 'reject_once') ?? + options.find((option) => option.kind === 'reject_always') + )?.optionId; +} + +/** + * Picks the option that denies a single tool call without ending the turn. A + * host-policy denial (no responder, headless, permission-avoidance, a routed + * interactive question) must reject the TOOL, not cancel the TURN: ACP defines + * the `cancelled` outcome as "the prompt turn was cancelled", so answering a + * policy denial with it would abort the whole delegation on its first sensitive + * tool instead of letting the peer continue without that one action — which is + * what the in-process auto-deny path does. Prefers `reject_once` (deny this + * call) over `reject_always` (deny for the session) to keep the scope narrow, + * and returns undefined when no reject option is safely selectable so the caller + * falls back to `cancelled` rather than guessing. + */ +export function selectRejectOption( + options: ReadonlyArray<{ optionId: string; kind: unknown }>, +): string | undefined { + // An ambiguous option ID could turn a denial into the wrong selection. + if ( + new Set(options.map((option) => option.optionId)).size !== options.length + ) { + return undefined; + } + return ( + options.find((option) => option.kind === 'reject_once') ?? + options.find((option) => option.kind === 'reject_always') + )?.optionId; +} + +/** + * The peer's own exit status, raised by acp-bridge as `exited uncleanly during + * shutdown (code=…, signal=…)` ONLY after it has driven every owned process + * group to empty (`survivingGroups` empty). So it reports that a foreign agent + * we do not own exited — a non-zero code from an uncaught exception, an OOM + * kill, a wrapper turning SIGTERM into 143, or the signal we sent — which + * `terminateMode` has already classified, not a cleanup failure. `dispose()` + * tolerates it silently for any code/signal. + * + * Tolerating it matters: `dispose()` is awaited inside `runTurn`'s catch between + * error classification and the return, so letting it reject here would replace + * the turn's declared terminal state — a turn the user CANCELLED would surface + * to the parent as `failed` with the registry's text. + * + * This deliberately does NOT cover the snapshot-race shape (see + * `isUnprovenExternalAgentTreeExit`): that one means the tree was never + * enumerated, so it is reported rather than silently swallowed. Every other + * genuine cleanup-PROOF error still propagates from `dispose()` — a truncated + * snapshot, a root that was absent or was not an isolated process-group leader, + * a failed snapshot/signal/inspect, or an exit deadline exceeded — so a + * descendant we were responsible for cannot survive disposal silently. + */ +export function isExpectedExternalAgentCleanupExit(error: unknown): boolean { + return ( + error instanceof Error && + /^ACP child pid=\d+ exited uncleanly during shutdown \(code=[^,]+, signal=[^)]+\)$/.test( + error.message, + ) + ); +} + +/** + * acp-bridge raises `exited before its initial process-tree snapshot completed` + * when the peer's root exited while the initial snapshot was in flight: + * `mergeAsynchronousSnapshot` records this proof error and returns BEFORE + * `collectOwnership`, so `knownGroups` holds only the root pgid and a detached + * (`setsid`) descendant the peer started — an MCP or dev server in its own + * process group — was never enumerated, signalled or reaped. The tree was + * therefore NOT proven gone, and acp-bridge ranks this proof error above the + * unclean-exit (process-registry.ts:416 before :417). + * + * It is not a hard cleanup failure (the root and its own group are settled), so + * `dispose()` must not rethrow it — rethrowing would convert an already + * classified TIMEOUT/CANCELLED turn into a thrown ERROR — but it must not be + * swallowed silently either, or the parent is told a clean teardown happened + * while a descendant may survive. `dispose()` reports it (debug log + an ERROR + * event) and resolves. + */ +export function isUnprovenExternalAgentTreeExit(error: unknown): boolean { + return ( + error instanceof Error && + /^ACP child pid=\d+ exited before its initial process-tree snapshot completed$/.test( + error.message, + ) + ); +} + +/** + * Thrown by `wait()` when an operation exceeds its deadline. A dedicated type + * lets `runTurn` classify a wall-time expiry by identity instead of re-deriving + * it from a second `Date.now()` read: the timer fires on libuv's monotonic clock + * against a `remaining` budget computed earlier, so comparing realtime elapsed + * with a 1ms-exact `>=` misclassifies a turn that merely reached its budget as + * ERROR whenever the timer wins by under a millisecond. + */ +class ExternalAgentTimeoutError extends Error { + constructor(timeoutMs: number) { + super(`External ACP operation timed out after ${timeoutMs}ms`); + this.name = 'ExternalAgentTimeoutError'; + } +} + +class AcpSubagentExecutor implements SubagentExecutor { + private connection!: ClientSideConnection; + private sessionId = ''; + private finalText = ''; + private thoughtText = ''; + private terminateMode = AgentTerminateMode.ERROR; + private round = 0; + private durationMs = 0; + private turnStartedAt = 0; + private toolCalls = 0; + private toolSucceeded = 0; + private toolFailed = 0; + private executing = false; + private started = false; + private disposed = false; + private cancelled = false; + private disposePromise?: Promise; + private provider?: () => AgentExternalInput[]; + private readonly pendingPermissions = new Map void>(); + private readonly tools = new Map(); + private readonly toolNames = new Map(); + private readonly finishedTools = new Set(); + private readonly failure = new AbortController(); + private readonly coreView: SubagentExecutorCore; + + private constructor( + private readonly params: ExternalAgentExecutorParams, + private readonly child: TrackedChildProcess, + private readonly emitter: AgentEventEmitter, + ) { + this.coreView = { + getEventEmitter: () => emitter, + modelConfig: { + ...params.modelConfig, + model: externalModelLabel(params.spec.command), + }, + }; + void child.exited.then( + () => this.fail(new Error(`external agent "${params.name}" exited`)), + (error: unknown) => this.fail(error), + ); + } + + static async create( + params: ExternalAgentExecutorParams, + ): Promise { + // Fail closed with a clear message on Windows rather than letting the spawn + // below fail with a misleading `spawn ENOENT` (an npm-installed + // launcher resolves to a `.cmd`, which libuv's PATH search never finds and + // Node refuses to spawn without a shell). The executor's spawn and its + // process-tree reaping are POSIX-only in this release. (R3-1) + assertExternalAgentSpawnPlatformSupported(); + if (params.runConfig.max_turns !== undefined) { + throw new Error('External ACP agents cannot enforce max_turns.'); + } + const minutes = params.runConfig.max_time_minutes; + if ( + minutes !== undefined && + (!Number.isFinite(minutes) || + minutes <= 0 || + minutes * 60_000 > 2_147_483_647) + ) { + throw new Error( + 'External ACP max_time_minutes must be positive, finite, and within the Node timer range.', + ); + } + const child = spawn(params.spec.command, params.spec.args ?? [], { + cwd: params.runtimeContext.getTargetDir(), + stdio: ['pipe', 'pipe', 'pipe'], + detached: process.platform !== 'win32', + windowsHide: true, + env: sanitizeChildEnv(process.env), + }); + const tracked = new ProcessRegistry() + .reserve() + .attach(child, { ownsProcessTree: true }); + const executor = new AcpSubagentExecutor( + params, + tracked, + params.eventEmitter ?? new AgentEventEmitter(), + ); + child.on('error', (error) => executor.fail(error)); + child.stdin!.on('error', (error) => executor.fail(error)); + child.stdout!.on('error', (error) => executor.fail(error)); + const forwarder = createStderrForwarder({ + prefix: `[external-agent ${params.name}] `, + }); + child.stderr!.setEncoding('utf8'); + child.stderr!.on('data', forwarder.onData); + child.stderr!.on('end', forwarder.onEnd); + child.stderr!.on('error', () => {}); + try { + executor.connection = new ClientSideConnection( + () => executor.buildClient(), + ndJsonStream( + Writable.toWeb(child.stdin!) as WritableStream, + Readable.toWeb(child.stdout!) as ReadableStream, + ), + ); + void executor.connection.closed.then(() => + executor.fail(new Error('External ACP connection closed')), + ); + await executor.wait(executor.connect(), INIT_TIMEOUT_MS); + return executor; + } catch (error) { + await executor.dispose(); + throw error; + } + } + + private buildClient(): Client { + return { + sessionUpdate: async (params) => this.onSessionUpdate(params), + requestPermission: (params) => this.onRequestPermission(params), + readTextFile: async () => { + throw RequestError.methodNotFound('fs/read_text_file'); + }, + writeTextFile: async () => { + throw RequestError.methodNotFound('fs/write_text_file'); + }, + extMethod: async (method) => { + throw RequestError.methodNotFound(method); + }, + extNotification: async () => {}, + }; + } + + private async connect(): Promise { + const initialized = await this.connection.initialize({ + protocolVersion: PROTOCOL_VERSION, + clientCapabilities: { fs: { readTextFile: false, writeTextFile: false } }, + }); + if (initialized.protocolVersion !== PROTOCOL_VERSION) { + throw new Error('Unsupported external ACP protocol version'); + } + const session = await this.connection.newSession({ + cwd: this.params.runtimeContext.getTargetDir(), + mcpServers: [], + }); + if (!session.sessionId) + throw new Error('External ACP agent returned no sessionId'); + this.sessionId = session.sessionId; + const policy = resolvePermissionMode( + this.params.permissionMode, + this.params.approvalMode, + ); + const availableIds = + session.modes?.availableModes.map((mode: { id: string }) => mode.id) ?? + []; + const modeId = selectPeerModeId(policy, availableIds); + if (!modeId) { + throw new Error( + `External ACP agent does not advertise any mode for the host's ${policy} approval policy (advertised: ${availableIds.join(', ') || 'none'})`, + ); + } + await this.connection.setSessionMode({ sessionId: this.sessionId, modeId }); + } + + private fail(error: unknown): void { + this.failure.abort(error); + this.drainPermissions(); + } + + private async wait( + operation: Promise, + timeoutMs?: number, + signal?: AbortSignal, + ): Promise { + let timer: ReturnType | undefined; + let onFailure = () => {}; + let onAbort = () => {}; + const stopped = new Promise((_resolve, reject) => { + onFailure = () => reject(this.failure.signal.reason); + onAbort = () => + reject(signal?.reason ?? new Error('External agent cancelled')); + this.failure.signal.addEventListener('abort', onFailure, { once: true }); + signal?.addEventListener('abort', onAbort, { once: true }); + if (this.failure.signal.aborted) onFailure(); + if (signal?.aborted) onAbort(); + if (timeoutMs !== undefined) { + timer = setTimeout( + () => reject(new ExternalAgentTimeoutError(timeoutMs)), + timeoutMs, + ); + } + }); + try { + return await Promise.race([operation, stopped]); + } finally { + clearTimeout(timer); + this.failure.signal.removeEventListener('abort', onFailure); + signal?.removeEventListener('abort', onAbort); + } + } + + async execute( + context: ContextState, + signal?: AbortSignal, + options: { resetStats?: boolean } = {}, + ): Promise { + if (context.get('initial_messages_override') !== undefined) { + throw new Error( + 'External ACP agents cannot import in-process conversation history', + ); + } + // A continuation turn re-enters on the same live ACP session, which has held + // the system-prompt bundle since turn 1 (NewSessionRequest carries only cwd/ + // mcpServers — the prompt content block is the bundle's only channel, so the + // FIRST turn must still send it). Re-rendering and re-sending it on every + // continuation would duplicate the whole bundle (system prompt + rules + + // memory hierarchy) per turn, billed by the peer and pushing its context + // toward compaction. Continuations send only the task. (R12-1) + const continuation = this.started; + const system = continuation + ? '' + : renderSubagentSystemPrompt( + this.params.promptConfig, + context, + this.params.runtimeContext, + ); + const task = String(context.get('task_prompt') ?? 'Get Started!'); + // The first turn's task is seeded into the transcript as the initial user + // prompt by the dispatcher; a continuation turn (a resident external agent + // re-invoked per incoming user message) has no such seed, so emit the task + // as a user-side record here or the JSONL transcript loses every message + // after the first — the in-process sibling emits the same EXTERNAL_MESSAGE + // on continuation. + await this.runTurn( + [ + ...(system ? [{ type: 'text' as const, text: system }] : []), + { type: 'text', text: task }, + ], + signal, + options, + continuation ? [task] : undefined, + ); + } + + async executeExternalInputs( + inputs: AgentExternalInput[], + signal?: AbortSignal, + options: { resetStats?: boolean } = {}, + ): Promise { + if (inputs.length === 0) return; + await this.runTurn(this.inputPrompt(inputs), signal, options, inputs); + } + + private inputPrompt(inputs: AgentExternalInput[]): ContentBlock[] { + return inputs.map((input) => ({ + type: 'text', + text: typeof input === 'string' ? input : input.text, + })); + } + + private emitInputs(inputs: AgentExternalInput[]): void { + for (const input of inputs) { + this.emitter.emit(AgentEventType.EXTERNAL_MESSAGE, { + subagentId: this.id, + kind: typeof input === 'string' ? 'message' : input.kind, + text: typeof input === 'string' ? input : input.text, + timestamp: Date.now(), + }); + } + } + + private async runTurn( + prompt: ContentBlock[], + signal?: AbortSignal, + options: { resetStats?: boolean } = {}, + inputs?: AgentExternalInput[], + ): Promise { + if (this.executing) + throw new Error('External ACP agents do not support concurrent turns'); + if (this.disposed || this.failure.signal.aborted) + throw new Error('External ACP agent is closed'); + this.executing = true; + this.started = true; + this.cancelled = false; + this.finalText = ''; + this.thoughtText = ''; + this.terminateMode = AgentTerminateMode.ERROR; + if (options.resetStats !== false) { + this.round = + this.durationMs = + this.toolCalls = + this.toolSucceeded = + this.toolFailed = + 0; + } + this.tools.clear(); + this.toolNames.clear(); + this.finishedTools.clear(); + this.turnStartedAt = Date.now(); + // An absent max_time_minutes means "no wall-time cap", matching the + // in-process sibling (agent-core installs no timer when maxTimeMinutes is + // falsy) and the rest of the codebase. Do NOT borrow a default from + // DEFAULT_WORKFLOW_SUBAGENT_MAX_TIME_MINUTES: workflow agent() hard-rejects + // external-executor definitions upstream, so that constant provably never + // reaches this path — the only route here is the Agent tool, which sets no + // default. A borrowed 10-minute cap would make the same definition behave + // differently based only on whether it declares an executor, and would make + // a TIMEOUT turn (presented to the parent as the answer) reachable by + // default. `wait` already treats an undefined budget as "no timer". + const timeoutMs = + this.params.runConfig.max_time_minutes === undefined + ? undefined + : this.params.runConfig.max_time_minutes * 60_000; + const cancel = () => { + this.cancelled = true; + this.drainPermissions(); + void this.connection + .cancel({ sessionId: this.sessionId }) + .catch(() => {}); + }; + signal?.addEventListener('abort', cancel, { once: true }); + let timedOut = false; + try { + this.emitter.emit(AgentEventType.START, { + subagentId: this.id, + name: this.params.name, + model: this.coreView.modelConfig.model, + tools: [], + timestamp: Date.now(), + }); + if (signal?.aborted) { + this.terminateMode = AgentTerminateMode.CANCELLED; + return; + } + // R12-2: the entry inputs are emitted at the loop's commit point (after + // the round-top budget guard), not here — emitting now would record them + // as delivered even when the guard breaks an over-budget continuation + // without ever dispatching the prompt. + const entryRound = this.round; + let next = prompt; + do { + this.round++; + // The wall-time budget is CUMULATIVE across a resident agent's + // continuation turns, matching the in-process sibling's preserveStats + // base: `durationMs` holds prior turns' elapsed time (reset only when + // `resetStats` is not false), so subtracting it makes max_time_minutes a + // cap on the whole delegation rather than a fresh per-turn budget that a + // continued agent could overrun. (R11-2) + const remaining = + timeoutMs === undefined + ? undefined + : Math.max( + 0, + timeoutMs - this.durationMs - (Date.now() - this.turnStartedAt), + ); + // Stop before dispatching a round whose budget is already spent. Without + // this, `remaining` clamps to 0 and `connection.prompt(...)` is evaluated + // — a new, billed model turn really reaches the peer — one tick before + // `wait`'s 0ms timer rejects with ExternalAgentTimeoutError; the catch + // then disposes (SIGTERM/SIGKILL) a peer that just started work, and the + // message this round already drained and recorded as delivered is never + // processed. Mirrors the in-process sibling, which checks the budget at + // the top of every round and breaks with TIMEOUT before dispatching. Set + // terminateMode directly rather than throwing: the catch classifies + // wall-time expiry by ExternalAgentTimeoutError identity, so a plain + // Error here would be reclassified ERROR and rethrown to the parent. + if (remaining !== undefined && remaining <= 0) { + this.terminateMode = AgentTerminateMode.TIMEOUT; + break; + } + // Record the caller's entry messages as delivered only now that the + // budget guard has committed to dispatching this round, and only on the + // entry round. Keyed on entryRound, NOT this.round === 1, because + // resetStats:false preserves this.round across continuations, so a + // continuation's first round is >= 2. Emitting before the prompt keeps + // user-before-assistant order in the JSONL transcript. (R12-2) + if (inputs && this.round === entryRound + 1) this.emitInputs(inputs); + const result = await this.wait( + this.connection.prompt({ sessionId: this.sessionId, prompt: next }), + remaining, + signal, + ); + this.terminateMode = this.stopMode(result.stopReason); + if (this.terminateMode !== AgentTerminateMode.GOAL) break; + // Gate the destructive drain on the budget and the abort signal BEFORE + // recording anything as delivered — mirroring the in-process sibling, + // which checks the wall-time budget before drainExternalInputs. The + // provider splices the queued messages out of the registry and + // emitInputs writes them to the transcript as delivered; if the budget + // is already spent (the round-top guard would break the next round) or + // the signal aborted (the loop-bottom condition), `next` is discarded + // without ever reaching connection.prompt — the transcript would + // certify delivery of a message that was never sent, and the registry + // has already lost it. Only drain once committed to dispatching. + if (signal?.aborted) { + this.terminateMode = AgentTerminateMode.CANCELLED; + break; + } + const remainingNext = + timeoutMs === undefined + ? undefined + : Math.max( + 0, + timeoutMs - this.durationMs - (Date.now() - this.turnStartedAt), + ); + if (remainingNext !== undefined && remainingNext <= 0) { + this.terminateMode = AgentTerminateMode.TIMEOUT; + break; + } + const queued = this.provider?.() ?? []; + if (queued.length === 0) break; + this.emitter.emit(AgentEventType.ROUND_TEXT, { + subagentId: this.id, + round: this.round, + text: this.finalText, + thoughtText: this.thoughtText, + timestamp: Date.now(), + }); + this.finalText = ''; + this.thoughtText = ''; + this.emitInputs(queued); + next = this.inputPrompt(queued); + } while (!signal?.aborted); + if (signal?.aborted) this.terminateMode = AgentTerminateMode.CANCELLED; + } catch (error) { + // Classify the wall-time expiry by the timer's own error identity, not by + // re-reading Date.now(): a second clock can miss the monotonic deadline by + // <1ms and misreport a budget-reaching turn as ERROR. See + // ExternalAgentTimeoutError. + timedOut = error instanceof ExternalAgentTimeoutError; + this.terminateMode = signal?.aborted + ? AgentTerminateMode.CANCELLED + : timedOut + ? AgentTerminateMode.TIMEOUT + : AgentTerminateMode.ERROR; + if ( + this.emitter.rawListeners(AgentEventType.ERROR).length > 0 && + !signal?.aborted + ) { + this.emitter.emit(AgentEventType.ERROR, { + subagentId: this.id, + error: error instanceof Error ? error.message : String(error), + timestamp: Date.now(), + }); + } + await this.dispose(); + if (!signal?.aborted && !timedOut) throw error; + } finally { + signal?.removeEventListener('abort', cancel); + this.durationMs += Date.now() - this.turnStartedAt; + this.executing = false; + this.drainPermissions(); + this.emitter.emit(AgentEventType.ROUND_TEXT, { + subagentId: this.id, + round: this.round, + text: this.finalText, + thoughtText: this.thoughtText, + timestamp: Date.now(), + }); + // Close out any tool call left open by cancel/timeout/crash before FINISH + // reports the totals, so the counts sum and no consumer is left showing a + // tool that never terminated (R7-3). + this.flushOpenTools(); + this.emitter.emit(AgentEventType.FINISH, { + subagentId: this.id, + terminateReason: this.terminateMode, + rounds: this.round, + totalDurationMs: this.durationMs, + totalToolCalls: this.toolCalls, + successfulToolCalls: this.toolSucceeded, + failedToolCalls: this.toolFailed, + timestamp: Date.now(), + }); + } + } + + private stopMode(reason: string): AgentTerminateMode { + switch (reason) { + case 'end_turn': + return AgentTerminateMode.GOAL; + case 'max_turn_requests': + return AgentTerminateMode.MAX_TURNS; + case 'cancelled': + return AgentTerminateMode.CANCELLED; + case 'refusal': + // A peer that refused the task neither achieved the goal nor was + // cancelled by the user. CANCELLED renders as "cancelled by the user" + // — a false statement about the user that also hides the refusal from + // telemetry — so report it as a failure the parent can react to. The + // ACP schema asks that a refusal be reflected rather than relabelled. + return AgentTerminateMode.ERROR; + default: + return AgentTerminateMode.ERROR; + } + } + + private get id(): string { + return this.params.subagentId ?? this.params.name; + } + getFinalText(): string { + return this.finalText; + } + getTerminateMode(): AgentTerminateMode { + return this.terminateMode; + } + getCore(): SubagentExecutorCore { + return this.coreView; + } + setExternalMessageProvider(provider: () => AgentExternalInput[]): void { + this.provider = provider; + } + // This executor deliberately does NOT implement `setExternalMessageWaiter` / + // `setExternalMessageWaitPredicate`: ACP v1 has no mid-turn injection + // primitive (no `session/steer`), so input that arrives while a prompt is in + // flight cannot be delivered until that prompt resolves. Queued input is + // drained between prompts via the provider above (next-turn-boundary + // delivery). Implementing true mid-turn steering would require cancel + + // re-prompt — re-billing the whole in-flight turn — which is a protocol/billing + // decision, not a safe default. The delegation result surfaces this + // limitation (see agent.ts EXTERNAL_MID_TURN_INPUT_NOTICE). (R3-6) + getExecutionSummary(): AgentStatsSummary { + return { + rounds: this.round, + totalDurationMs: + this.durationMs + + (this.executing ? Date.now() - this.turnStartedAt : 0), + totalToolCalls: this.toolCalls, + successfulToolCalls: this.toolSucceeded, + failedToolCalls: this.toolFailed, + successRate: this.toolCalls + ? (this.toolSucceeded / this.toolCalls) * 100 + : 0, + inputTokens: 0, + outputTokens: 0, + thoughtTokens: 0, + cachedTokens: 0, + totalTokens: 0, + toolUsage: [], + }; + } + + dispose(): Promise { + this.disposed = true; + this.fail(new Error('External ACP agent disposed')); + return (this.disposePromise ??= this.child + .terminate() + .catch((error: unknown) => { + if (isUnprovenExternalAgentTreeExit(error)) { + // The peer's root exited before its process tree was enumerated, so a + // detached descendant may survive. Report it — do NOT rethrow: + // dispose() is awaited between the turn's terminal-state classification + // and its return, so rethrowing would replace a classified + // TIMEOUT/CANCELLED with a thrown ERROR. + const detail = error instanceof Error ? error.message : String(error); + debugLogger.warn( + `External ACP agent process tree was not proven gone after disposal: ${detail}`, + ); + if (this.emitter.rawListeners(AgentEventType.ERROR).length > 0) { + this.emitter.emit(AgentEventType.ERROR, { + subagentId: this.id, + error: `External agent process tree not proven gone after disposal: ${detail}`, + timestamp: Date.now(), + }); + } + return; + } + if (!isExpectedExternalAgentCleanupExit(error)) throw error; + })); + } + + private drainPermissions(): void { + for (const cancel of this.pendingPermissions.values()) cancel(); + this.pendingPermissions.clear(); + } + + private onSessionUpdate({ sessionId, update }: SessionNotification): void { + // Drop updates once the turn is over for ANY reason, not just user cancel. + // On timeout or peer crash the catch awaits dispose() before the finally + // clears `executing`, and `cancelled` stays false, so without the + // disposed/aborted checks the peer's still-flowing stdout would be appended + // to finalText, emitted as STREAM_TEXT/ROUND_TEXT and counted into the + // FINISH totals for up to EXIT_DEADLINE_MS after termination — a false + // transcript record and a partial result handed to the parent verbatim on + // the non-rethrowing TIMEOUT path (R8-1). + if ( + sessionId !== this.sessionId || + !this.executing || + this.cancelled || + this.disposed || + this.failure.signal.aborted + ) + return; + switch (update.sessionUpdate) { + case 'agent_message_chunk': + case 'agent_thought_chunk': { + const thought = update.sessionUpdate === 'agent_thought_chunk'; + const text = + update.content.type === 'text' + ? update.content.text + : JSON.stringify(update.content); + if (thought) this.thoughtText += text; + else this.finalText += text; + this.emitter.emit(AgentEventType.STREAM_TEXT, { + subagentId: this.id, + round: this.round, + text, + thought, + timestamp: Date.now(), + }); + break; + } + case 'tool_call': + this.updateTool(update); + break; + case 'tool_call_update': + this.updateTool(update); + break; + default: + break; + } + } + + private updateTool(update: ToolCall | ToolCallUpdate): void { + const callId = update.toolCallId; + if (this.finishedTools.has(callId)) return; + const previous = this.tools.get(callId); + const tool: ToolCall = { + toolCallId: callId, + title: previous?.title ?? 'External tool', + ...previous, + ...Object.fromEntries( + Object.entries(update).filter( + ([, value]) => value !== null && value !== undefined, + ), + ), + }; + this.tools.set(callId, tool); + const meta = tool._meta?.['claudeCode']; + const reportedName = + isRecord(meta) && typeof meta['toolName'] === 'string' + ? meta['toolName'] + : (tool.kind ?? 'external_tool'); + const name = this.toolNames.get(callId) ?? reportedName; + if (!previous) { + this.toolNames.set(callId, name); + this.toolCalls++; + this.emitter.emit(AgentEventType.TOOL_CALL, { + subagentId: this.id, + round: this.round, + callId, + name, + args: isRecord(tool.rawInput) + ? tool.rawInput + : { input: tool.rawInput }, + description: tool.title, + timestamp: Date.now(), + }); + } + if (tool.status !== 'completed' && tool.status !== 'failed') return; + this.finishedTools.add(callId); + this.tools.delete(callId); + const success = tool.status === 'completed'; + if (success) this.toolSucceeded++; + else this.toolFailed++; + this.toolNames.delete(callId); + const response = { + input: tool.rawInput ?? null, + toolName: reportedName, + output: tool.rawOutput ?? null, + content: tool.content ?? null, + status: tool.status, + }; + const responseParts = [ + { functionResponse: { id: callId, name, response } }, + ]; + this.emitter.emit(AgentEventType.TOOL_RESULT, { + subagentId: this.id, + round: this.round, + callId, + name, + success, + responseParts, + resultDisplay: JSON.stringify(tool.rawOutput ?? tool.content ?? null), + timestamp: Date.now(), + }); + this.emitter.emit(AgentEventType.TOOL_RESPONSES_FINALIZED, { + subagentId: this.id, + round: this.round, + responses: [{ callId, responseParts }], + timestamp: Date.now(), + }); + } + + /** + * Emit a terminal TOOL_RESULT + TOOL_RESPONSES_FINALIZED for every tool call + * still open when the turn ends. Cancel, timeout and peer crash all leave + * entries in `this.tools`: `cancel()` sets `this.cancelled` before the peer's + * own terminal `tool_call_update` can arrive (so `onSessionUpdate` drops it), + * and the next `runTurn` clears the map. Without this flush three consumers + * keep a false record — the inline frame and Web Shell row show the tool + * executing forever (TOOL_RESULT is the only writer of their status), the + * JSONL transcript holds a permanently unpaired `functionCall` + * (TOOL_RESPONSES_FINALIZED is the only writer of its `tool_result`), and + * FINISH's totals do not sum. The in-process sibling closes the same gap via + * `onAllToolCallsComplete`. + */ + private flushOpenTools(): void { + for (const [callId, tool] of this.tools) { + const meta = tool._meta?.['claudeCode']; + const reportedName = + isRecord(meta) && typeof meta['toolName'] === 'string' + ? meta['toolName'] + : (tool.kind ?? 'external_tool'); + const name = this.toolNames.get(callId) ?? reportedName; + this.toolFailed++; + const response = { + input: tool.rawInput ?? null, + toolName: reportedName, + output: null, + content: tool.content ?? null, + status: 'failed', + }; + const responseParts = [ + { functionResponse: { id: callId, name, response } }, + ]; + this.emitter.emit(AgentEventType.TOOL_RESULT, { + subagentId: this.id, + round: this.round, + callId, + name, + success: false, + responseParts, + resultDisplay: JSON.stringify(tool.content ?? null), + timestamp: Date.now(), + }); + this.emitter.emit(AgentEventType.TOOL_RESPONSES_FINALIZED, { + subagentId: this.id, + round: this.round, + responses: [{ callId, responseParts }], + timestamp: Date.now(), + }); + } + this.tools.clear(); + this.toolNames.clear(); + } + + private async onRequestPermission( + params: RequestPermissionRequest, + ): Promise { + const cancelled: RequestPermissionResponse = { + outcome: { outcome: 'cancelled' }, + }; + const runtime = this.params.runtimeContext; + const callId = params.toolCall.toolCallId; + const meta = params.toolCall._meta?.['claudeCode']; + const name = + isRecord(meta) && typeof meta['toolName'] === 'string' + ? meta['toolName'] + : (params.toolCall.kind ?? 'external_tool'); + // Turn-level and request-integrity states: the turn is already over, the + // request is not ours, or it cannot be answered safely (no options, or + // ambiguous duplicate option IDs). ACP `cancelled` is the right response — + // there is no single tool call to deny. + if ( + params.sessionId !== this.sessionId || + !this.executing || + this.cancelled || + this.disposed || + this.failure.signal.aborted || + this.pendingPermissions.has(callId) || + params.options.length === 0 || + new Set(params.options.map((option) => option.optionId)).size !== + params.options.length + ) + return cancelled; + // Host-policy denials: reject the TOOL, not the TURN, so the peer continues + // without this one action (see selectRejectOption). Falls back to + // `cancelled` only when the peer offered no reject option to select. + if ( + name.replaceAll('_', '').toLowerCase() === 'askuserquestion' || + runtime.getShouldAvoidPermissionPrompts() || + (!runtime.isInteractive() && + !runtime.getExperimentalZedIntegration() && + runtime.getInputFormat() !== InputFormat.STREAM_JSON) || + this.emitter.rawListeners(AgentEventType.TOOL_WAITING_APPROVAL).length === + 0 + ) { + const rejectOptionId = selectRejectOption(params.options); + return rejectOptionId + ? { outcome: { outcome: 'selected', optionId: rejectOptionId } } + : cancelled; + } + return new Promise((resolve) => { + const finish = (optionId?: string) => { + if (this.pendingPermissions.get(callId) !== deny) return; + this.pendingPermissions.delete(callId); + resolve( + optionId ? { outcome: { outcome: 'selected', optionId } } : cancelled, + ); + }; + const deny = () => finish(); + this.pendingPermissions.set(callId, deny); + try { + this.emitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, { + subagentId: this.id, + round: this.round, + callId, + name, + args: isRecord(params.toolCall.rawInput) + ? params.toolCall.rawInput + : {}, + description: params.toolCall.title ?? 'External action', + confirmationDetails: { + type: 'info', + title: params.toolCall.title ?? 'External action', + // Carry the action's arguments so the user sees WHAT they are + // authorizing — not just the option labels the dialog already + // renders as buttons (R11-5). Bounded and control-stripped. + prompt: describeExternalAction(params.toolCall), + // The prompt is a foreign-process payload; render it as plain text, + // never markdown — otherwise the dialog would eat glob `**` and + // render links, misrepresenting what the user approves. (R11-5 fix) + renderPromptAsPlainText: true, + hideAlwaysAllow: true, + }, + respond: async (outcome) => + finish( + selectPermissionOption(params.options, outcome) ?? + // An explicit user rejection of THIS tool must deny the tool, + // not cancel the whole turn: `selectPermissionOption` maps + // Cancel to undefined, and ACP `cancelled` ends the prompt turn + // (abandoning the remaining work and billing a fresh prompt to + // re-delegate). Select the peer's `reject_once` option so the + // turn continues without this one action, matching the dialog's + // "suggest changes" promise and the in-process sibling. Fall + // back to `cancelled` only when no reject option is selectable. + (outcome === ToolConfirmationOutcome.Cancel + ? selectRejectOption(params.options) + : undefined), + ), + timestamp: Date.now(), + }); + } catch { + deny(); + } + }); + } +} + +export const acpExternalAgentExecutor: ExternalAgentExecutor = { + create: (params) => AcpSubagentExecutor.create(params), +}; diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json index be0cb3f7934..371b8d34ef1 100644 --- a/packages/cli/tsconfig.json +++ b/packages/cli/tsconfig.json @@ -37,6 +37,9 @@ "@qwen-code/qwen-code-core/conversationsRuntimeMarker": [ "../core/src/utils/conversations-runtime-marker.ts" ], + "@qwen-code/qwen-code-core/subagentRuntime": [ + "../core/src/subagent-runtime.ts" + ], "@qwen-code/qwen-code-core/*": ["../core/src/*"], "@qwen-code/acp-bridge": ["../acp-bridge/src/index.ts"], "@qwen-code/acp-bridge/transcriptReplay": [ diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index f6c244ba088..73bd04d6b04 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -74,6 +74,10 @@ export default defineConfig({ __dirname, '../core/src/utils/conversations-runtime-marker.ts', ), + '@qwen-code/qwen-code-core/subagentRuntime': path.resolve( + __dirname, + '../core/src/subagent-runtime.ts', + ), }), // Mirrors `"@qwen-code/qwen-code-core/*": ["../core/src/*"]` from // packages/cli/tsconfig.json. esbuild reads that paths block when it diff --git a/packages/core/package.json b/packages/core/package.json index 616f06bb867..88247c02c31 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -13,6 +13,10 @@ "types": "./dist/index.d.ts", "import": "./dist/index.js" }, + "./subagentRuntime": { + "types": "./dist/src/subagent-runtime.d.ts", + "import": "./dist/src/subagent-runtime.js" + }, "./transcriptRecords": { "types": "./dist/src/utils/transcript-records.d.ts", "import": "./dist/src/utils/transcript-records.js" diff --git a/packages/core/src/agents/agent-transcript.ts b/packages/core/src/agents/agent-transcript.ts index 9e793b168e9..791c96af869 100644 --- a/packages/core/src/agents/agent-transcript.ts +++ b/packages/core/src/agents/agent-transcript.ts @@ -147,6 +147,8 @@ export interface AgentMeta { persistedCliFlags?: AgentPersistedCliFlags; /** Canonical subagent config name used to recreate this agent. */ subagentName?: string; + /** External launch provenance; transcript replay cannot restore its session. */ + executor?: 'acp'; /** UI hint preserved for resumed task rows. */ agentColor?: string; /** Number of explicit resume attempts performed so far. */ diff --git a/packages/core/src/agents/background-agent-resume.test.ts b/packages/core/src/agents/background-agent-resume.test.ts index b876f8ce5cd..f666c4ba8fe 100644 --- a/packages/core/src/agents/background-agent-resume.test.ts +++ b/packages/core/src/agents/background-agent-resume.test.ts @@ -23,6 +23,7 @@ import { } from './agent-transcript.js'; import { ToolNames } from '../tools/tool-names.js'; import { AgentTerminateMode } from './runtime/agent-types.js'; +import { SubagentError, SubagentErrorCode } from '../subagents/types.js'; import { AgentEventEmitter } from './runtime/agent-events.js'; import { getCurrentAgentDepth } from './runtime/agent-context.js'; import { AgentHeadless } from './runtime/agent-headless.js'; @@ -459,6 +460,61 @@ describe('BackgroundAgentResumeService', () => { ).toBeUndefined(); }); + it.each(['persisted', 'definition', 'legacy-model', 'legacy-flags'] as const)( + 'blocks cold external resume using %s provenance', + async (provenance) => { + const sessionId = 'session-external'; + const agentId = 'external-agent'; + writeAgentMeta(getAgentMetaPath(tempDir, sessionId, agentId), { + agentId, + agentType: 'researcher', + description: 'External task', + parentSessionId: sessionId, + parentAgentId: null, + createdAt: new Date().toISOString(), + status: 'running', + ...(provenance === 'persisted' ? { executor: 'acp' as const } : {}), + ...(provenance === 'legacy-model' + ? { model: 'external-acp:claude' } + : {}), + ...(provenance === 'legacy-flags' + ? { persistedCliFlags: { model: 'external-acp:claude' } } + : {}), + }); + fs.writeFileSync( + getAgentJsonlPath(tempDir, sessionId, agentId), + JSON.stringify({ + uuid: 'u1', + sessionId, + type: 'user', + message: { role: 'user', parts: [{ text: 'External task' }] }, + }) + '\n', + ); + const { service, subagentManager } = createService(); + if (provenance === 'definition') { + subagentManager.loadSubagent.mockResolvedValue({ + name: 'researcher', + color: 'cyan', + model: undefined, + approvalMode: undefined, + executor: { kind: 'acp', command: 'claude' }, + } as Awaited>); + } + const recovered = await service.loadPausedBackgroundAgents(sessionId); + expect(recovered[0]?.resumeBlockedReason).toContain( + 'External subagent session cannot be restored', + ); + expect(await service.resumeBackgroundAgent(agentId)).toBeUndefined(); + // Recheck provenance at execution time, not only during discovery. + recovered[0]!.resumeBlockedReason = undefined; + expect(await service.resumeBackgroundAgent(agentId)).toBeUndefined(); + expect(registry.get(agentId)?.resumeBlockedReason).toContain( + 'External subagent session cannot be restored', + ); + expect(subagentManager.createAgentHeadless).not.toHaveBeenCalled(); + }, + ); + it('preserves model on recovered paused agents for per-model caps', async () => { const sessionId = 'session-model'; const agentId = 'agent-model'; @@ -631,6 +687,55 @@ describe('BackgroundAgentResumeService', () => { expect(subagentManager.loadSubagent).toHaveBeenCalledWith('deleted-agent'); }); + it('keeps a paused agent listed when its same-named definition now fails the executor guard (R12-3)', async () => { + const sessionId = 'session-executor-refusal'; + const agentId = 'agent-executor-refusal'; + writeAgentMeta(getAgentMetaPath(tempDir, sessionId, agentId), { + agentId, + agentType: 'researcher', + description: + 'Background task whose same-named definition now fails to load', + parentSessionId: sessionId, + parentAgentId: null, + createdAt: '2026-04-20T00:00:00.000Z', + status: 'running', + subagentName: 'researcher', + resolvedApprovalMode: 'default', + }); + fs.writeFileSync( + getAgentJsonlPath(tempDir, sessionId, agentId), + JSON.stringify({ + uuid: 'u1', + parentUuid: null, + sessionId, + timestamp: '2026-04-20T00:00:00.000Z', + type: 'user', + message: { role: 'user', parts: [{ text: 'task' }] }, + }) + '\n', + 'utf8', + ); + + const { service, subagentManager } = createService(); + // The R10-2/R11 executor refusal makes loadSubagent THROW for a same-named + // file that failed to load. resolveResumeTarget must convert that throw into + // the existing "unavailable" shape so discovery keeps the row listed with a + // resumeBlockedReason — otherwise the per-sidecar catch swallows it into a + // debug-only warning and the row vanishes from /tasks. Removing the try/catch + // turns this red (recovered is empty). + subagentManager.loadSubagent.mockRejectedValue( + new SubagentError( + 'Agent file /test/project/.qwen/agents/researcher.md has an invalid executor block: it declares an executor but failed to load.', + SubagentErrorCode.INVALID_CONFIG, + 'researcher', + ), + ); + const recovered = await service.loadPausedBackgroundAgents(sessionId); + expect(recovered).toHaveLength(1); + expect(recovered[0]?.resumeBlockedReason).toContain( + 'invalid executor block', + ); + }); + it('keeps paused tasks resumable when they only carry a stale lastError', async () => { const sessionId = 'session-stale-error'; const agentId = 'agent-stale-error'; diff --git a/packages/core/src/agents/background-agent-resume.ts b/packages/core/src/agents/background-agent-resume.ts index 22624bde8d4..6c817df971b 100644 --- a/packages/core/src/agents/background-agent-resume.ts +++ b/packages/core/src/agents/background-agent-resume.ts @@ -17,6 +17,7 @@ import { } from './runtime/agent-events.js'; import { AgentTerminateMode } from './runtime/agent-types.js'; import { AgentHeadless, ContextState } from './runtime/agent-headless.js'; +import type { SubagentExecutor } from './runtime/subagent-executor.js'; import { buildAgentTranscriptAttach, getAgentJsonlPath, @@ -388,7 +389,7 @@ function recoverTranscript(records: ChatRecord[]): TranscriptRecovery { } function getCompletionStats( - subagent: AgentHeadless, + subagent: SubagentExecutor, liveToolCallCount: number, ): AgentCompletionStats { const summary = subagent.getExecutionSummary(); @@ -503,7 +504,12 @@ export class BackgroundAgentResumeService { if (registry.get(meta.agentId)) continue; const subagentName = meta.subagentName ?? meta.agentType; if (typeof subagentName !== 'string' || !subagentName) continue; - const target = await this.resolveResumeTarget(subagentName); + const target = await this.resolveResumeTarget( + subagentName, + meta.executor, + meta.model, + meta.persistedCliFlags?.model, + ); const outputFile = getAgentJsonlPath( projectDir, @@ -863,7 +869,12 @@ export class BackgroundAgentResumeService { try { const subagentName = meta.subagentName ?? meta.agentType; - const target = await this.resolveResumeTarget(subagentName); + const target = await this.resolveResumeTarget( + subagentName, + meta.executor, + meta.model, + meta.persistedCliFlags?.model, + ); if (!target.subagentConfig && !target.isFork) { const reason = target.unavailableReason || @@ -992,7 +1003,7 @@ export class BackgroundAgentResumeService { const bgEventEmitter = new AgentEventEmitter(); const launchModel = meta.model ?? meta.persistedCliFlags?.model; - let subagent: AgentHeadless; + let subagent: SubagentExecutor; if (target.isFork) { subagent = await this.createResumedForkSubagent( activeAgentConfig, @@ -1545,7 +1556,22 @@ export class BackgroundAgentResumeService { private async resolveResumeTarget( subagentName: string, + executor?: AgentMeta['executor'], + ...legacyModels: Array ): Promise { + // Older external runs wrote a synthetic model label instead of provenance. + // It can deny replay, but never authorizes selecting an executor. + if ( + executor !== undefined || + legacyModels.some((model) => model?.startsWith('external-acp:')) + ) { + return { + agentName: subagentName, + isFork: false, + unavailableReason: + 'External subagent session cannot be restored from a Qwen transcript. Start a new agent instead.', + }; + } if (subagentName === FORK_SUBAGENT_TYPE) { return { agentName: FORK_AGENT.name, @@ -1554,9 +1580,26 @@ export class BackgroundAgentResumeService { }; } - const subagentConfig = await this.config - .getSubagentManager() - .loadSubagent(subagentName); + let subagentConfig: SubagentConfig | null; + try { + subagentConfig = await this.config + .getSubagentManager() + .loadSubagent(subagentName); + } catch (error) { + // loadSubagent throws a recorded executor-block refusal (R10-2/R11) when a + // same-named definition failed to load. This is resume *discovery*, not a + // dispatch, so surface it as the existing "unavailable" shape — the row + // stays listed with a resumeBlockedReason — instead of letting the throw + // escape into the per-sidecar catch, which would drop the row entirely. + return { + agentName: subagentName, + isFork: false, + unavailableReason: + error instanceof Error + ? error.message + : `Subagent "${subagentName}" is no longer available.`, + }; + } if (!subagentConfig) { return { agentName: subagentName, @@ -1565,6 +1608,9 @@ export class BackgroundAgentResumeService { }; } + if (subagentConfig.executor !== undefined) { + return this.resolveResumeTarget(subagentName, 'acp'); + } return { agentName: subagentConfig.name, isFork: false, @@ -1784,7 +1830,7 @@ export class BackgroundAgentResumeService { } private async runSubagentStopHookLoop( - subagent: AgentHeadless, + subagent: SubagentExecutor, opts: { agentId: string; agentType: string; diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 3aa9d45ce29..0e7b58c244a 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -391,6 +391,37 @@ export interface ExecutionStats { totalTokens?: number; } +export function renderSubagentSystemPrompt( + promptConfig: PromptConfig, + context: ContextState, + runtimeContext: Config, + interactive?: boolean, +): string { + if (!promptConfig.systemPrompt) { + return ''; + } + + let finalPrompt = templateString(promptConfig.systemPrompt, context); + + // Only add non-interactive instructions when NOT in interactive mode + if (!interactive) { + finalPrompt += ` + +Important Rules: + - You operate in non-interactive mode: do not ask the user questions; proceed with available context. + - Use tools only when necessary to obtain facts or make changes. + - When the task is complete, return the final result as a normal model response (not a tool call) and stop.`; + } + + // Context files (QWEN.md + output-language.md) keep the subagent aligned + // with project conventions; the volatile auto-memory section stays last. + return assembleSystemPrompt({ + base: finalPrompt, + contextFiles: runtimeContext.getUserMemory(), + autoMemory: runtimeContext.getAutoMemoryPrompt(), + }); +} + /** * AgentCore — shared execution engine for model reasoning and tool scheduling. * @@ -2623,29 +2654,12 @@ export class AgentCore { context: ContextState, options?: CreateChatOptions, ): string { - if (!this.promptConfig.systemPrompt) { - return ''; - } - - let finalPrompt = templateString(this.promptConfig.systemPrompt, context); - - // Only add non-interactive instructions when NOT in interactive mode - if (!options?.interactive) { - finalPrompt += ` - -Important Rules: - - You operate in non-interactive mode: do not ask the user questions; proceed with available context. - - Use tools only when necessary to obtain facts or make changes. - - When the task is complete, return the final result as a normal model response (not a tool call) and stop.`; - } - - // Context files (QWEN.md + output-language.md) keep the subagent aligned - // with project conventions; the volatile auto-memory section stays last. - return assembleSystemPrompt({ - base: finalPrompt, - contextFiles: this.runtimeContext.getUserMemory(), - autoMemory: this.runtimeContext.getAutoMemoryPrompt(), - }); + return renderSubagentSystemPrompt( + this.promptConfig, + context, + this.runtimeContext, + options?.interactive, + ); } /** diff --git a/packages/core/src/agents/runtime/agent-headless.ts b/packages/core/src/agents/runtime/agent-headless.ts index 08a00fc3382..2c567d705cb 100644 --- a/packages/core/src/agents/runtime/agent-headless.ts +++ b/packages/core/src/agents/runtime/agent-headless.ts @@ -29,6 +29,7 @@ import type { } from './agent-events.js'; import { AgentEventType } from './agent-events.js'; import type { AgentStatsSummary } from './agent-statistics.js'; +import type { SubagentExecutor } from './subagent-executor.js'; import type { PromptConfig, ModelConfig, @@ -135,7 +136,7 @@ export function templateString( * Each execute() call runs one task through AgentCore's reasoning loop. Calls * must be sequential; later calls reuse the same chat and prepared tools. */ -export class AgentHeadless { +export class AgentHeadless implements SubagentExecutor { private readonly core: AgentCore; private finalText: string = ''; private terminateMode: AgentTerminateMode = AgentTerminateMode.ERROR; diff --git a/packages/core/src/agents/runtime/subagent-executor.ts b/packages/core/src/agents/runtime/subagent-executor.ts new file mode 100644 index 00000000000..80633e4924b --- /dev/null +++ b/packages/core/src/agents/runtime/subagent-executor.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Config } from '../../config/config.js'; +import type { SubagentExecutorSpec } from '../../subagents/types.js'; +import type { RuntimeContentGeneratorView } from './agent-context.js'; +import type { AgentEventEmitter, AgentHooks } from './agent-events.js'; +import type { ContextState } from './agent-headless.js'; +import type { AgentStatsSummary } from './agent-statistics.js'; +import type { + AgentExternalInput, + AgentTerminateMode, + ModelConfig, + PromptConfig, + RunConfig, + ToolConfig, +} from './agent-types.js'; + +export interface SubagentExecutorCore { + getEventEmitter(): AgentEventEmitter; + readonly modelConfig: ModelConfig; + readonly runtimeView?: RuntimeContentGeneratorView; +} + +export interface SubagentExecutor { + execute( + context: ContextState, + externalSignal?: AbortSignal, + options?: { resetStats?: boolean }, + ): Promise; + + executeExternalInputs( + inputs: AgentExternalInput[], + externalSignal?: AbortSignal, + options?: { resetStats?: boolean }, + ): Promise; + + getFinalText(): string; + getTerminateMode(): AgentTerminateMode; + getExecutionSummary(): AgentStatsSummary; + getCore(): SubagentExecutorCore; + + setExternalMessageProvider(provider: () => AgentExternalInput[]): void; + setExternalMessageWaiter?( + waiter: (signal: AbortSignal) => Promise, + ): void; + setExternalMessageWaitPredicate?(predicate: () => boolean): void; + + /** Resolves after owned resources have been released. */ + dispose?(): void | Promise; +} + +export interface ExternalAgentExecutorParams { + spec: SubagentExecutorSpec; + name: string; + approvalMode?: string; + permissionMode?: string; + promptConfig: PromptConfig; + modelConfig: ModelConfig; + runConfig: RunConfig; + toolConfig?: ToolConfig; + eventEmitter?: AgentEventEmitter; + hooks?: AgentHooks; + taskName?: string; + subagentId?: string; + runtimeContext: Config; +} + +export interface ExternalAgentExecutor { + create(params: ExternalAgentExecutorParams): Promise; +} diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index f1c02fe396a..7b1e2a3864c 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -3608,6 +3608,98 @@ describe('WorkflowOrchestrator P3 — agentType / model / isolation / schema', ( }; } + it.each([ + { label: 'plain', options: {}, tokenLimit: null }, + { label: 'budgeted', options: {}, tokenLimit: 100 }, + { + label: 'schema', + options: { schema: { type: 'object' } }, + tokenLimit: null, + }, + { + label: 'budgeted schema', + options: { schema: { type: 'object' } }, + tokenLimit: 100, + }, + { + label: 'worktree', + options: { isolation: 'worktree' as const }, + tokenLimit: null, + }, + { + label: 'pinned worktree', + options: { workingDir: '/fake/repo/worktree' }, + tokenLimit: null, + }, + ])( + 'rejects $label external workflow dispatch before agent creation', + async ({ options, tokenLimit }) => { + const { WorkflowBudgetImpl } = await import('./workflow-budget.js'); + const budget = new WorkflowBudgetImpl(tokenLimit); + const onTokens = vi.fn((tokens: number) => budget.recordSpent(tokens)); + const onCreate = vi.fn(async () => ({ + finalText: 'external output', + terminateMode: 'GOAL', + })); + const { config, calls } = fakeConfigWithMgr({ + findSubagentByName: async () => ({ + name: 'external-agent', + description: 'External agent', + systemPrompt: 'Complete the task.', + level: 'session', + executor: { kind: 'acp', command: 'external-agent' }, + }), + onCreate, + }); + const createRegistry = vi.spyOn(config, 'createToolRegistry'); + const dispatch = createProductionDispatch(config, undefined, onTokens); + + await expect( + dispatch('do work', { agentType: 'external-agent', ...options }), + ).rejects.toThrow( + 'Workflow agent() does not support external-executor agents: ' + + 'token budgets, schema output, and workflow tool restrictions ' + + 'cannot be enforced. Use an in-process agent definition instead.', + ); + + // The manager owns external factory/process creation; never enter it. + expect(calls).toEqual([]); + expect(onCreate).not.toHaveBeenCalled(); + expect(createRegistry).not.toHaveBeenCalled(); + expect(worktreeStubs.instances).toHaveLength(0); + expect(pinStub.seenLabels).toEqual([]); + expect(onTokens).not.toHaveBeenCalled(); + }, + ); + + it('continues to account tokens for an in-process workflow agent', async () => { + const { WorkflowBudgetImpl } = await import('./workflow-budget.js'); + const budget = new WorkflowBudgetImpl(100); + const { config, calls } = fakeConfigWithMgr({ + findSubagentByName: async () => ({ + name: 'ordinary-agent', + description: 'In-process agent', + systemPrompt: 'Complete the task.', + level: 'session', + }), + onCreate: async () => ({ finalText: 'done', terminateMode: 'GOAL' }), + }); + const previousTokens = nextOutputTokens.value; + nextOutputTokens.value = 25; + try { + const dispatch = createProductionDispatch(config, undefined, (tokens) => + budget.recordSpent(tokens), + ); + await expect( + dispatch('do work', { agentType: 'ordinary-agent' }), + ).resolves.toBe('done'); + expect(calls).toHaveLength(1); + expect(budget.remaining()).toBe(75); + } finally { + nextOutputTokens.value = previousTokens; + } + }); + it('agentType resolves SubagentConfig and routes through createAgentHeadless', async () => { const { config, calls } = fakeConfigWithMgr({ findSubagentByName: async () => ({ diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.ts b/packages/core/src/agents/runtime/workflow-orchestrator.ts index 8e070447c26..2de558c4a79 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.ts @@ -530,6 +530,13 @@ export function createProductionDispatch( // agentType definition rides along so the override path reuses it // instead of re-scanning subagent files per attempt. const agentIdentity = await resolveWorkflowAgentIdentity(config, opts); + if (agentIdentity.resolvedAgentType?.executor !== undefined) { + throw new Error( + 'Workflow agent() does not support external-executor agents: ' + + 'token budgets, schema output, and workflow tool restrictions ' + + 'cannot be enforced. Use an in-process agent definition instead.', + ); + } let attempt = 0; return runStallResilient( async (attemptSignal, emitter) => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index d763f33ecbb..6302db57be2 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -46,6 +46,7 @@ import { } from '../core/contentGenerator.js'; import { tokenLimit } from '../core/tokenLimits.js'; import { getRuntimeContentGenerator } from '../agents/runtime/agent-context.js'; +import type { ExternalAgentExecutor } from '../agents/runtime/subagent-executor.js'; import { isTieredEffortWireModel } from '../core/modalityDefaults.js'; import { DashScopeOpenAICompatibleProvider, @@ -2450,6 +2451,12 @@ export class Config { private readonly sessionWorkflowEnabled: boolean; private sessionWorkflowEnabledProvider?: () => boolean; private sessionWorkflowPlanRevision?: SessionWorkflowPlanRevision; + /** + * Host-supplied factory for subagents that declare an `executor` block. + * `packages/core` has no ACP dependency, so the implementation lives in the + * host package. See `ExternalAgentExecutor` and `setExternalAgentExecutor`. + */ + private externalAgentExecutor?: ExternalAgentExecutor; private readonly modelProposedGoals: ModelProposedGoalsMode; private readonly skipWorkflowUsageWarning: boolean = false; private readonly emitToolUseSummaries: boolean = true; @@ -7955,6 +7962,23 @@ export class Config { } } + /** + * Registers the host's factory for subagents that declare an `executor` + * block. `packages/core` has no ACP dependency, so the implementation lives + * in the host package; see `ExternalAgentExecutor`. + * + * A definition that declares an executor while none is registered fails + * loudly at spawn time rather than silently running in-process — silent + * substitution is the exact failure mode this seam exists to prevent. + */ + setExternalAgentExecutor(executor?: ExternalAgentExecutor): void { + this.externalAgentExecutor = executor; + } + + getExternalAgentExecutor(): ExternalAgentExecutor | undefined { + return this.externalAgentExecutor; + } + getSessionWorkflowPlanRevision(): SessionWorkflowPlanRevision | undefined { if (!this.isSessionWorkflowEnabled()) return undefined; return this.sessionWorkflowPlanRevision; diff --git a/packages/core/src/extension/extensionManager.ts b/packages/core/src/extension/extensionManager.ts index 1c4dfa6b739..e1844b9369f 100644 --- a/packages/core/src/extension/extensionManager.ts +++ b/packages/core/src/extension/extensionManager.ts @@ -10,7 +10,7 @@ import type { } from '../config/config.js'; import { Config } from '../config/config.js'; import { validateSkillName, type SkillConfig } from '../skills/types.js'; -import type { SubagentConfig } from '../subagents/types.js'; +import type { SubagentConfig, SubagentError } from '../subagents/types.js'; import type { ClaudeMarketplaceConfig } from './claude-converter.js'; import type { HookEventName, HookDefinition } from '../hooks/types.js'; import { Storage } from '../config/storage.js'; @@ -178,6 +178,10 @@ export interface Extension { commands?: string[]; skills?: SkillConfig[]; agents?: SubagentConfig[]; + // R10-2: executor-block refusals for this extension's agent files, keyed by + // lowercased declared name, recorded at load so a by-name dispatch can refuse + // instead of falling through to a builtin of the same name. + agentExecutorRefusals?: Map; hooks?: { [K in HookEventName]?: HookDefinition[] }; channels?: Record; } @@ -1711,9 +1715,12 @@ export class ExtensionManager { extension.skills = await loadSkillsFromDir( `${effectiveExtensionPath}/skills`, ); + const agentExecutorRefusals = new Map(); extension.agents = await loadSubagentFromDir( `${effectiveExtensionPath}/agents`, + agentExecutorRefusals, ); + extension.agentExecutorRefusals = agentExecutorRefusals; } if ( diff --git a/packages/core/src/subagent-runtime.ts b/packages/core/src/subagent-runtime.ts new file mode 100644 index 00000000000..d78edb4b7b6 --- /dev/null +++ b/packages/core/src/subagent-runtime.ts @@ -0,0 +1,53 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +export type { + ExternalAgentExecutor, + ExternalAgentExecutorParams, + SubagentExecutor, + SubagentExecutorCore, +} from './agents/runtime/subagent-executor.js'; +export { renderSubagentSystemPrompt } from './agents/runtime/agent-core.js'; +export { ContextState } from './agents/runtime/agent-headless.js'; +export { AgentTerminateMode } from './agents/runtime/agent-types.js'; +export type { + AgentExternalInput, + ModelConfig, + PromptConfig, + RunConfig, + ToolConfig, +} from './agents/runtime/agent-types.js'; +export type { SubagentExecutorSpec } from './subagents/types.js'; +export { ToolConfirmationOutcome } from './tools/tools.js'; +export type { ToolCallConfirmationDetails } from './tools/tools.js'; +export type { + AgentStatsSummary, + ToolUsageStats, +} from './agents/runtime/agent-statistics.js'; +export { + AgentEventEmitter, + AgentEventType, +} from './agents/runtime/agent-events.js'; +export type { + AgentApprovalRequestEvent, + AgentConfirmationDetails, + AgentErrorEvent, + AgentEvent, + AgentEventMap, + AgentExternalMessageEvent, + AgentFinishEvent, + AgentHooks, + AgentRoundEvent, + AgentRoundTextEvent, + AgentStartEvent, + AgentStatusChangeEvent, + AgentStreamTextEvent, + AgentToolCallEvent, + AgentToolOutputUpdateEvent, + AgentToolResponsesFinalizedEvent, + AgentToolResultEvent, + AgentUsageEvent, +} from './agents/runtime/agent-events.js'; diff --git a/packages/core/src/subagents/agent-frontmatter-schema.test.ts b/packages/core/src/subagents/agent-frontmatter-schema.test.ts index 80c78ca763c..82eb5cc5705 100644 --- a/packages/core/src/subagents/agent-frontmatter-schema.test.ts +++ b/packages/core/src/subagents/agent-frontmatter-schema.test.ts @@ -9,6 +9,7 @@ import { PERMISSION_MODE_VALUES, COLOR_VALUES, claudePermissionModeToApprovalMode, + parseAgentExecutor, parseAgentHooks, parseAgentMcpServers, parseMaxTurns, @@ -244,4 +245,100 @@ describe('agent-frontmatter-schema', () => { expect(Object.hasOwn(result!, '__proto__')).toBe(true); }); }); + + describe('parseAgentExecutor — qwen-code extension (not mirrored from CC)', () => { + it('parses a minimal legal spec', () => { + expect(parseAgentExecutor({ kind: 'acp', command: 'npx' })).toEqual({ + kind: 'acp', + command: 'npx', + }); + }); + + it('preserves args and trims the command', () => { + expect( + parseAgentExecutor({ + kind: 'acp', + command: ' npx ', + args: ['-y', '@agentclientprotocol/claude-agent-acp'], + }), + ).toEqual({ + kind: 'acp', + command: 'npx', + args: ['-y', '@agentclientprotocol/claude-agent-acp'], + }); + }); + + it('keeps an explicitly empty args array', () => { + expect( + parseAgentExecutor({ kind: 'acp', command: 'x', args: [] }), + ).toEqual({ kind: 'acp', command: 'x', args: [] }); + }); + + it('drops the whole field for an unrecognized or missing kind', () => { + expect( + parseAgentExecutor({ kind: 'subprocess', command: 'npx' }), + ).toBeUndefined(); + expect(parseAgentExecutor({ command: 'npx' })).toBeUndefined(); + expect( + parseAgentExecutor({ kind: 'ACP', command: 'npx' }), + ).toBeUndefined(); + }); + + it('drops the whole field for a missing, non-string, or blank command', () => { + expect(parseAgentExecutor({ kind: 'acp' })).toBeUndefined(); + expect(parseAgentExecutor({ kind: 'acp', command: 42 })).toBeUndefined(); + expect( + parseAgentExecutor({ kind: 'acp', command: ['npx'] }), + ).toBeUndefined(); + expect( + parseAgentExecutor({ kind: 'acp', command: ' ' }), + ).toBeUndefined(); + }); + + it('drops the whole field when args is not an array of strings', () => { + expect( + parseAgentExecutor({ kind: 'acp', command: 'npx', args: '-y' }), + ).toBeUndefined(); + expect( + parseAgentExecutor({ kind: 'acp', command: 'npx', args: {} }), + ).toBeUndefined(); + }); + + it('drops the whole field rather than filtering a partially invalid args array', () => { + // Running with silently-truncated arguments is worse than not running: + // a dropped `-y` or a dropped package specifier changes what executes. + expect( + parseAgentExecutor({ + kind: 'acp', + command: 'npx', + args: ['-y', 42, '@agentclientprotocol/claude-agent-acp'], + }), + ).toBeUndefined(); + }); + + it('returns undefined for a non-object top level', () => { + expect(parseAgentExecutor(undefined)).toBeUndefined(); + expect(parseAgentExecutor(null)).toBeUndefined(); + expect(parseAgentExecutor('npx')).toBeUndefined(); + expect( + parseAgentExecutor([{ kind: 'acp', command: 'npx' }]), + ).toBeUndefined(); + expect(parseAgentExecutor(42)).toBeUndefined(); + }); + + it('does not pass through unrecognized keys', () => { + // The result is rebuilt from known fields only, so a definition cannot + // smuggle extra executor options past the parser into the spawn site. + const result = parseAgentExecutor({ + kind: 'acp', + command: 'npx', + args: ['-y'], + env: { MALICIOUS: '1' }, + cwd: '/etc', + shell: true, + }); + expect(result).toEqual({ kind: 'acp', command: 'npx', args: ['-y'] }); + expect(Object.keys(result!)).toEqual(['kind', 'command', 'args']); + }); + }); }); diff --git a/packages/core/src/subagents/agent-frontmatter-schema.ts b/packages/core/src/subagents/agent-frontmatter-schema.ts index 4305c3fb4f8..634b06e116c 100644 --- a/packages/core/src/subagents/agent-frontmatter-schema.ts +++ b/packages/core/src/subagents/agent-frontmatter-schema.ts @@ -20,6 +20,8 @@ * its semantics would break existing `.qwen/agents/*.md` files. */ +import type { SubagentExecutorSpec } from './types.js'; + /** Permission mode enum (DL7 `$E` / `kc` constant). */ export const PERMISSION_MODE_VALUES = [ 'acceptEdits', @@ -186,3 +188,46 @@ export function parseAgentHooks( } return Object.keys(result).length > 0 ? result : undefined; } + +/** + * Parse a frontmatter `executor` value into a `SubagentExecutorSpec`. + * + * Unlike `mcpServers` and `hooks`, this field is NOT part of the mirrored + * Claude Code schema — it is a qwen-code extension (see + * `docs/design/claude-code-web-shell-backend.md` §9.3). Because it names an + * external process, it is validated strictly enough that a typo cannot + * silently change what runs: + * + * - non-object / array / null → undefined (whole field dropped) + * - `kind` absent or not `'acp'` → undefined + * - `command` absent, non-string, or blank after trim → undefined + * - `args` present but not an array of strings → undefined. Dropped whole + * rather than filtered: running with silently-truncated arguments is + * worse than not running at all. + * + * The caller must reject a provided value when this returns undefined; it + * must never drop the executor and fall back to in-process execution. + */ +export function parseAgentExecutor( + value: unknown, +): SubagentExecutorSpec | undefined { + if (value === undefined || value === null) return undefined; + if (typeof value !== 'object' || Array.isArray(value)) return undefined; + const record = value as Record; + if (record['kind'] !== 'acp') return undefined; + const command = record['command']; + if (typeof command !== 'string') return undefined; + const trimmedCommand = command.trim(); + if (trimmedCommand === '') return undefined; + const rawArgs = record['args']; + if (rawArgs === undefined) { + return { kind: 'acp', command: trimmedCommand }; + } + if (!Array.isArray(rawArgs)) return undefined; + const args: string[] = []; + for (const arg of rawArgs) { + if (typeof arg !== 'string') return undefined; + args.push(arg); + } + return { kind: 'acp', command: trimmedCommand, args }; +} diff --git a/packages/core/src/subagents/subagent-manager.test.ts b/packages/core/src/subagents/subagent-manager.test.ts index c90033f180f..1962eefbf1a 100644 --- a/packages/core/src/subagents/subagent-manager.test.ts +++ b/packages/core/src/subagents/subagent-manager.test.ts @@ -16,6 +16,7 @@ import { } from './types.js'; import type { ToolRegistry } from '../tools/tool-registry.js'; import type { Config } from '../config/config.js'; +import { ApprovalMode } from '../config/approval-mode.js'; import { makeFakeConfig } from '../test-utils/config.js'; import { AuthType } from '../core/contentGenerator.js'; import { ToolNames } from '../tools/tool-names.js'; @@ -380,6 +381,250 @@ You are a helpful assistant. `; describe('parseSubagentContent', () => { + it.each([ + 'null', + 'false', + '0', + '""', + '{ kind: ACP, command: npx }', + '{ kind: acp, command: " " }', + '{ kind: acp, command: npx, args: [null] }', + ])('rejects invalid executor frontmatter %s', async (executor) => { + const content = `---\nname: test-agent\ndescription: Test\nexecutor: ${executor}\n---\nPrompt`; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('rejects an executor whose null argument the shared parser would sanitize away', async () => { + // Pins the load-bearing guard that validates the ORIGINAL YAML node + // (parseDocument) rather than the shared parser's sanitized result. The + // shared parser strips null sequence items, turning `args: [null]` into + // `args: []` — which parseAgentExecutor ACCEPTS, launching the command + // with truncated arguments. Only the original node still carries [null] + // and is rejected. Drive the real shared parser so the stripping actually + // happens; with the guard reverted to the sanitized value this no longer + // throws and the test goes red. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementationOnce(yaml.parse); + const content = + '---\nname: test-agent\ndescription: Test\nexecutor:\n kind: acp\n command: npx\n args:\n - null\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('rejects a definition whose frontmatter has a YAML syntax error rather than trusting a repaired executor node', async () => { + // parseDocument repairs invalid YAML instead of throwing; document.errors + // is the only signal. Without this guard an unterminated quote yields a + // silently repaired executor node that dispatches a different command than + // the file declares. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + "---\nname: test-agent\ndescription: Test\nexecutor:\n kind: acp\n command: 'npx\n---\nPrompt"; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid YAML frontmatter|invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('still loads a non-executor definition whose frontmatter strict YAML rejects', async () => { + // R5-1: the document.errors refusal must be scoped to executor-bearing + // files. A description containing a colon makes strict YAML reject, but + // the shared parser's parseSimple fallback still loads it; with no + // executor block the definition must keep loading exactly as at the merge + // base, or the agent silently vanishes from /agents and subagent_type + // fails as agent-not-found. Drive the real shared parser so the lenient + // fallback is actually exercised; an unconditional document.errors guard + // makes this throw and the test go red. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: reviewer\ndescription: Reviews code: fast and careful\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + const config = await manager.parseSubagentFile( + validConfig.filePath!, + 'project', + ); + expect(config.name).toBe('reviewer'); + expect(config.description).toBe('Reviews code: fast and careful'); + expect(config.executor).toBeUndefined(); + }); + + it('refuses a quoted top-level "executor" key when the frontmatter YAML is malformed (R7-1 under-refusal leg)', async () => { + // A quoted `"executor":` plus a YAML error the lenient parser tolerates + // (an unquoted colon in description) is missed by parseSimple (keeps the + // quotes in the key) and by a repaired parseDocument (nests it). Only the + // column-0 raw-text probe catches it; without that the file loads with no + // executor and silently runs in-process — the exact substitution this PR + // exists to prevent. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: worker\ndescription: Reviews code: fast\n"executor":\n kind: acp\n command: npx\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('refuses a malformed file whose only "executor" token is nested, fail-closed (R10-1 re-scopes the R7-1 over-refusal leg)', async () => { + // R10-1 widened the claim probe to indentation-tolerant so an executor key + // the real AST dropped cannot slip through to a silent in-process run (a + // TAB- or space-indented top-level `executor:` that the column-0 anchor + // missed). The accepted cost: an `executor:` token nested under another + // key, in a file that ALSO has a YAML error, is now treated as a claim the + // AST lost and refused — a visible, user-fixable over-refusal that beats an + // invisible substitution. Reverting the probe to column-0 anchoring turns + // this red (the definition loads again). A nested executor token in a file + // with NO YAML error still loads: the guard is gated on errors.length > 0. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: reviewer\ndescription: Reviews code: fast\nmetadata:\n executor: legacy-note\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('loads a valid executor whose frontmatter has an unrelated tolerated YAML error before it (R7-1 over-refusal, narrowed)', async () => { + // A duplicate `name:` key from a bad merge is a tolerated YAML error that + // sits BEFORE the executor line, so it cannot reach the executor subtree: + // `document.toJS().executor` stays byte-faithful. The old + // `claimsExecutor && document.errors.length > 0` guard hard-refused this, + // deleting a valid external-agent definition from /agents (and breaking + // `subagent_type:`) over an unrelated quirk that loads at the merge base. + // Removing the line-scoping must turn this red, while the quoted-key and + // unterminated-quote refusals (errors at/after the executor line) stay + // green. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: a\nname: b\ndescription: Test\nexecutor:\n kind: acp\n command: claude-agent-acp\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + const config = await manager.parseSubagentFile( + validConfig.filePath!, + 'project', + ); + expect(config.executor).toEqual({ + kind: 'acp', + command: 'claude-agent-acp', + }); + }); + + it('refuses an executor whose node parseDocument dropped via an earlier error, not trusting the parseSimple fallback (R9-2)', async () => { + // A compact-mapping error on an EARLIER line (the unquoted colon in + // description) makes parseDocument drop the whole remainder, so the + // executor node is ABSENT (has('executor') is false) rather than + // byte-faithful — YAML errors are not line-local, so an error before the + // executor line does not mean the executor survived. The lenient + // parseSimple fallback then rebuilds `command: |` plus an indented `npx` + // as `{command:'|', npx:''}` (it is a line-based heuristic, not a YAML + // parser), and parseAgentExecutor would accept `command:'|'`, spawning an + // executable literally named `|` that the file never declared. The refusal + // must key on parseDocument losing the node (!hasExecutor), not on + // parseSimple also missing it. Reverting that leg to + // `!hasExecutor && frontmatter.executor === undefined` turns this red, + // while the duplicate-key load test above (has('executor') true) stays + // green. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: explore\ndescription: Reviews code: fast\nexecutor:\n kind: acp\n command: |\n npx\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('refuses an executor whose frontmatter has an unresolved YAML alias instead of throwing a raw parse error (R9 deferred :1968)', async () => { + // parseDocument tolerates an unresolved alias (`command: *undef`) with an + // EMPTY document.errors, so the errors-based executor guard cannot catch + // it, but document.toJS() throws when it resolves the node. Without the + // try/catch around toJS() the raw "Unresolved alias" YAML error escapes + // parseSubagentContent; with it the definition is refused as an invalid + // executor block. Removing the try/catch turns this red (the rejection + // message becomes the raw YAML error, not /invalid executor block/). + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: explore\ndescription: Test\nexecutor:\n kind: acp\n command: *undefined_anchor\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + await expect( + manager.parseSubagentFile(validConfig.filePath!, 'project'), + ).rejects.toThrow(/invalid executor block/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('loads a block-scalar prose "executor:" as no executor, not a hoisted command (R10-1)', async () => { + // A `description: |` block scalar whose prose contains an `executor:` line, + // plus an unresolved alias elsewhere so the strict parse fails and the + // lenient parseSimple fallback runs. parseSimple hoists the prose + // `executor:` into a top-level key ({kind:acp, command:npx}); the old code + // used that value as executorRaw and loaded the definition as EXTERNAL, + // dispatching `npx` — a command that exists only as prose. parseDocument + // correctly sees no top-level executor (hasExecutor false) and, with no + // document error, the guard does not fire, so the fix loads with executor + // undefined. Restoring the parseSimple fallback for executorRaw turns this + // red (config.executor becomes {kind:'acp', command:'npx'}). + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + const content = + '---\nname: x\ndescription: |\n executor:\n kind: acp\n command: npx\nother: *undefined_anchor\n---\nPrompt'; + vi.mocked(fs.readFile).mockResolvedValue(content); + const config = await manager.parseSubagentFile( + validConfig.filePath!, + 'project', + ); + expect(config.executor).toBeUndefined(); + }); + + it('visibly reports invalid executors while continuing discovery', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.mocked(fs.readdir).mockResolvedValue(['bad.md', 'good.md'] as never); + vi.mocked(fs.readFile).mockImplementation(async (file) => + String(file).endsWith('bad.md') + ? '---\nname: bad\ndescription: Test\nexecutor: { kind: acp, command: " " }\n---\nPrompt' + : validMarkdown, + ); + const agents = await manager.listSubagents({ + level: 'project', + force: true, + }); + expect(agents.map((agent) => agent.name)).toEqual(['test-agent']); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('invalid executor block'), + ); + }); + it('should parse valid markdown content', () => { const config = manager.parseSubagentContent( validMarkdown, @@ -1029,6 +1274,48 @@ You are weird. }); describe('serializeSubagent', () => { + it('preserves the executor through save and reload', async () => { + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockStringifyYaml.mockImplementationOnce(yaml.stringify); + mockParseYaml.mockImplementationOnce(yaml.parse); + const executor = { + kind: 'acp' as const, + command: 'npx', + args: ['-y', 'adapter'], + }; + const serialized = manager.serializeSubagent({ + ...validConfig, + executor, + }); + expect( + manager.parseSubagentContent( + serialized, + validConfig.filePath!, + 'project', + ).executor, + ).toEqual(executor); + }); + + it.each([null, false, 0, '', { kind: 'acp', command: ' ' }])( + 'refuses to save invalid executor %j without writing', + async (executor) => { + vi.mocked(fs.access).mockRejectedValue(new Error('File not found')); + const config = { + ...validConfig, + executor, + } as unknown as SubagentConfig; + expect(() => manager.serializeSubagent(config)).toThrow( + /executor block failed validation/, + ); + await expect( + manager.createSubagent(config, { level: 'project' }), + ).rejects.toThrow(/executor block failed validation/); + expect(fs.writeFile).not.toHaveBeenCalled(); + }, + ); + it('should serialize basic configuration', () => { const serialized = manager.serializeSubagent(validConfig); @@ -1559,6 +1846,172 @@ You are a helpful assistant.`; path.normalize('/test/project/.qwen/agents/misnamed-file.md'), ); }); + + it('refuses a by-name dispatch matching a skipped invalid-executor file instead of falling through to a builtin (R10-2)', async () => { + // A project file declares the builtin name 'Explore' with an invalid + // executor (typo'd `kind: ACP`), so discovery skips it with a warning. + // Without R10-2, loadSubagent('Explore') falls through + // session>project>user>extension>builtin and resolves the BUILTIN Explore + // — an in-process agent under a Qwen model — silently substituting it for + // the external agent the file asked for, with only a console.warn as the + // trace. loadSubagent must instead refuse with the recorded executor error. + // Reverting to skip-and-continue (no recorded refusal) turns this red: the + // call resolves the builtin instead of rejecting. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue(['explore.md'] as any); + vi.mocked(fs.readFile).mockResolvedValue( + '---\nname: Explore\ndescription: Test\nexecutor:\n kind: ACP\n command: npx\n---\nPrompt', + ); + await expect(manager.loadSubagent('Explore')).rejects.toThrow( + /invalid executor block/, + ); + }); + + it('clears a stale executor refusal when the directory later becomes unreadable (R12-4)', async () => { + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + // First scan: a malformed-executor file records a refusal for 'explore'. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue(['explore.md'] as any); + vi.mocked(fs.readFile).mockResolvedValue( + '---\nname: Explore\ndescription: Test\nexecutor:\n kind: ACP\n command: npx\n---\nPrompt', + ); + await expect(manager.loadSubagent('Explore')).rejects.toThrow( + /invalid executor block/, + ); + // The directory then disappears / becomes unreadable (git checkout of a + // branch with no agents dir, rm -rf, an unreadable dir). Without resetting + // the level's refusals on the scan-failure path, loadSubagent keeps + // throwing the stale refusal for a file that no longer exists, leaving the + // builtin permanently unreachable. With the reset, the scan returns [] and + // the dispatch falls through to the builtin again. Deleting the catch reset + // turns this red (the second call rejects with the stale error). + vi.mocked(fs.readdir).mockRejectedValue( + new Error('ENOENT: no such directory'), + ); + const resolved = await manager.loadSubagent('Explore'); + expect(resolved?.isBuiltin).toBe(true); + }); + + it('does not refuse an in-process definition whose block-scalar prose mentions executor: (R12-5)', async () => { + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue(['foo.md'] as any); + // A `description: |` block scalar documents the executor syntax as prose; a + // duplicate `name:` key is a tolerated YAML quirk (control: it loads + // alone). The raw-text probe matches the prose `executor:` line, so without + // the block-scalar exclusion `claimsExecutor` is true and astLostExecutor + // refuses (the duplicate key makes has('executor') false). With the + // exclusion the prose match is not a claim, no refusal is recorded, and the + // in-process definition loads — reverting the exclusion turns this red + // (loadSubagent rejects /invalid executor block/). + vi.mocked(fs.readFile).mockResolvedValue( + '---\nname: foo\nname: foo\ndescription: |\n Reviews code. To run externally use:\n executor: acp\n for details.\n---\nPrompt', + ); + const config = await manager.loadSubagent('foo'); + expect(config).not.toBeNull(); + expect(config!.name).toBe('foo'); + expect(config!.executor).toBeUndefined(); + }); + + it('does not refuse an in-process definition whose folded-block-scalar prose mentions executor: (R12-5)', async () => { + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue(['foo.md'] as any); + // Same as above but with a folded block scalar (`description: >`). + vi.mocked(fs.readFile).mockResolvedValue( + '---\nname: foo\nname: foo\ndescription: >\n Reviews code. To run externally use:\n executor: acp\n for details.\n---\nPrompt', + ); + const config = await manager.loadSubagent('foo'); + expect(config).not.toBeNull(); + expect(config!.executor).toBeUndefined(); + }); + + it('refuses a by-name dispatch for an executor-claiming file that fails an earlier validation (R11-1)', async () => { + // A project file declares an executor but OMITS the description, so it + // fails the earlier required-field validation BEFORE the executor block is + // reached. Without hoisting the claim probe + declared name above those + // validations, the file is skipped with nothing recorded and loadSubagent + // falls through to the builtin Explore — the substitution R10-2 prevents. + // The catch now converts any load failure of an executor-claiming file into + // a named executor refusal. Reverting the catch conversion turns this red + // (loadSubagent resolves the builtin instead of rejecting). + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue(['explore.md'] as any); + vi.mocked(fs.readFile).mockResolvedValue( + '---\nname: Explore\nexecutor:\n kind: acp\n command: npx\n---\nPrompt', + ); + await expect(manager.loadSubagent('Explore')).rejects.toThrow( + /invalid executor block/, + ); + }); + + it('keys the executor refusal by the AST-parsed name, so a quoted name still refuses the dispatch (R11-4)', async () => { + // parseSimple's parseValue strips only double quotes, so `name: 'Explore'` + // (single-quoted) would otherwise be recorded under "'explore'" and miss + // the 'explore' dispatch lookup. The refusal must be keyed by the name the + // real YAML AST sees. The colon in description forces the astLostExecutor + // refusal path; reverting `declaredName ?? name` to `name` turns this red. + const yaml = await vi.importActual< + typeof import('../utils/yaml-parser.js') + >('../utils/yaml-parser.js'); + mockParseYaml.mockImplementation(yaml.parse); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue(['explore.md'] as any); + vi.mocked(fs.readFile).mockResolvedValue( + "---\nname: 'Explore'\ndescription: Reviews code: fast and careful\nexecutor:\n kind: ACP\n command: npx\n---\nPrompt", + ); + await expect(manager.loadSubagent('Explore')).rejects.toThrow( + /invalid executor block/, + ); + }); + + it('refuses a by-name dispatch for an extension-level executor refusal instead of falling through to a builtin (R10-2 extension leg)', async () => { + // Extension agents load via loadSubagentFromDir, which skips + warns on a + // refusal — so the directory-scan recording never ran for them and the + // R10-2 extension leg read an empty map (a no-op). The refusals are now + // carried on the loaded extension and merged into the 'extension' bucket, + // so the fall-through refuses. Reverting the merge (dropping the + // executorRefusals.set('extension', ...)) turns this red: loadSubagent + // resolves the builtin instead of rejecting. + vi.spyOn(mockConfig, 'getActiveExtensions').mockReturnValue([ + { + agents: [], + agentExecutorRefusals: new Map([ + [ + 'explore', + new SubagentError( + 'Agent file /ext/agents/explore.md has an invalid executor block: it declares an executor but failed to load.', + SubagentErrorCode.INVALID_CONFIG, + 'Explore', + ), + ], + ]), + } as never, + ]); + // No project/user file declares 'Explore' — only the extension refusal. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.mocked(fs.readdir).mockResolvedValue([] as any); + await expect(manager.loadSubagent('Explore')).rejects.toThrow( + /invalid executor block/, + ); + }); }); describe('updateSubagent', () => { @@ -2067,6 +2520,26 @@ bad`); describe('Runtime Configuration Methods', () => { describe('convertToRuntimeConfig', () => { + it.each([{ kind: 'acp', command: 'npx' }, null, false, 0, ''])( + 'refuses external executor %j before in-process conversion', + async (executor) => { + await expect( + manager.convertToRuntimeConfig({ + ...validConfig, + tools: ['read_file'], + executor, + } as unknown as SubagentConfig), + ).rejects.toMatchObject({ + code: SubagentErrorCode.INVALID_CONFIG, + message: expect.stringContaining( + 'cannot be converted to an in-process agent', + ), + }); + expect(mockToolRegistry.warmAll).not.toHaveBeenCalled(); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }, + ); + it('should convert basic configuration', async () => { const runtimeConfig = await manager.convertToRuntimeConfig(validConfig); @@ -2283,6 +2756,265 @@ bad`); }); }); + describe('createAgentHeadless — external executor dispatch', () => { + const executorConfig: SubagentConfig = { + name: 'external-agent', + description: 'Runs somewhere else', + systemPrompt: 'You are external.', + level: 'session' as const, + executor: { kind: 'acp', command: 'npx', args: ['-y', 'some-acp'] }, + }; + + afterEach(() => { + mockAgentHeadlessCreate.mockReset(); + vi.restoreAllMocks(); + }); + + it('refuses to run in-process when no executor is registered', async () => { + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue( + undefined, + ); + + await expect( + manager.createAgentHeadless(executorConfig, mockConfig), + ).rejects.toThrow(/registered no external agent executor/); + + // The load-bearing assertion: it must NOT silently substitute the + // in-process executor. A definition that asked for an external agent + // and got AgentHeadless would bill the wrong provider and report the + // wrong agent, with no signal either way. + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + expect(mockToolRegistry.warmAll).not.toHaveBeenCalled(); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + }); + + it.each([null, false, 0, '', {}, { kind: 'ACP', command: 'npx' }])( + 'rejects injected executor %j without side effects', + async (executor) => { + const create = vi.fn(); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + manager.loadSessionSubagents([ + { ...executorConfig, executor } as unknown as SubagentConfig, + ]); + const loaded = await manager.loadSubagent( + executorConfig.name, + 'session', + ); + await expect( + manager.createAgentHeadless(loaded!, mockConfig), + ).rejects.toThrow(/failed validation/); + expect(create).not.toHaveBeenCalled(); + expect(mockToolRegistry.warmAll).not.toHaveBeenCalled(); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { tools: [] }, + { tools: ['read_file'] }, + { disallowedTools: ['write_file'] }, + { mcpServers: { server: { command: 'node' } } }, + { hooks: { PreToolUse: [] } }, + { model: 'anthropic:claude' }, + { maxTurns: 3 }, + { runConfig: { max_turns: 3 } }, + ])( + 'rejects unsupported definition %j before factory or setup', + async (constraint) => { + const create = vi.fn(); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + const hooks = vi.spyOn(mockConfig, 'getHookSystem'); + await expect( + manager.createAgentHeadless( + { ...executorConfig, ...constraint }, + mockConfig, + ), + ).rejects.toThrow(/does not support/); + expect(create).not.toHaveBeenCalled(); + expect(hooks).not.toHaveBeenCalled(); + expect(mockToolRegistry.warmAll).not.toHaveBeenCalled(); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }, + ); + + it.each([ + { toolConfigOverride: { tools: ['structured_output'] } }, + { promptConfigOverrides: { initialMessages: [] } }, + { promptConfigOverrides: { renderedSystemPrompt: 'schema' } }, + { runtimeAuthOverrides: { authType: 'anthropic' } }, + { modelConfigOverrides: { model: 'claude' } }, + { + modelConfigOverrides: { temperature: 0.5 } as unknown as { + model?: string; + }, + }, + { hooks: { onStop: vi.fn() } }, + { runConfigOverrides: { max_turns: 3 } }, + ])('rejects unsupported options %j before factory', async (options) => { + const create = vi.fn(); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + await expect( + manager.createAgentHeadless(executorConfig, mockConfig, options), + ).rejects.toThrow(/does not support/); + expect(create).not.toHaveBeenCalled(); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + }); + + it('refuses project executables in an untrusted workspace', async () => { + const create = vi.fn(); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + vi.spyOn(mockConfig, 'isTrustedFolder').mockReturnValue(false); + await expect( + manager.createAgentHeadless( + { ...executorConfig, level: 'project' }, + mockConfig, + ), + ).rejects.toThrow(/untrusted project/); + expect(create).not.toHaveBeenCalled(); + }); + + it('refuses an external executor in safe mode even in a trusted folder (R8-2)', async () => { + const create = vi.fn(); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + vi.spyOn(mockConfig, 'isTrustedFolder').mockReturnValue(true); + vi.spyOn(mockConfig, 'isSafeMode').mockReturnValue(true); + // Safe mode promises only built-in subagents and no repo-supplied + // execution. Discovery filtering does not stop loadSubagent resolving a + // repo-shipped executor definition from disk, so the dispatch gate must + // refuse it — even at project level in a trusted folder. + await expect( + manager.createAgentHeadless( + { ...executorConfig, level: 'project' }, + mockConfig, + ), + ).rejects.toThrow(/safe mode/); + expect(create).not.toHaveBeenCalled(); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('preserves external factory errors without AgentHeadless labeling', async () => { + const error = new Error('spawn ENOENT'); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create: vi.fn().mockRejectedValue(error), + }); + await expect( + manager.createAgentHeadless(executorConfig, mockConfig), + ).rejects.toBe(error); + }); + + it('composes external disposal and propagates its error', async () => { + const error = new Error('dispose failed'); + const dispose = vi.fn().mockRejectedValue(error); + const create = vi.fn().mockResolvedValue({ dispose }); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + const result = await manager.createAgentHeadless( + { ...executorConfig, model: 'inherit' }, + mockConfig, + { + modelConfigOverrides: {}, + runConfigOverrides: { max_time_minutes: 2 }, + }, + ); + expect(create.mock.calls[0][0]).toMatchObject({ + modelConfig: {}, + runConfig: { max_time_minutes: 2 }, + toolConfig: { disallowedTools: [ToolNames.ASK_USER_QUESTION] }, + }); + expect(mockCreateContentGenerator).not.toHaveBeenCalled(); + await expect(result.dispose()).rejects.toBe(error); + expect(dispose).toHaveBeenCalledOnce(); + }); + + it('derives the peer permission mode from the host-resolved approval policy, not the raw definition', async () => { + const create = vi.fn().mockResolvedValue({}); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + // The definition asks for the most permissive mode; the host has + // clamped the effective policy to DEFAULT (resolveSubagentApprovalMode + // stamps that onto the runtimeContext the Agent tool hands in). + vi.spyOn(mockConfig, 'getApprovalMode').mockReturnValue( + ApprovalMode.DEFAULT, + ); + await manager.createAgentHeadless( + { ...executorConfig, approvalMode: 'yolo' }, + mockConfig, + ); + // The executor must receive the host's clamped policy, so a definition + // cannot escalate the external agent past the parent session's limit. + expect(create.mock.calls[0]![0]).toMatchObject({ + approvalMode: ApprovalMode.DEFAULT, + }); + }); + + it('re-validates the executor block at the consumption point', async () => { + // Session-level subagents are injected as plain objects and spread + // verbatim by loadSessionSubagents, bypassing frontmatter parsing — so + // an arbitrarily shaped executor can reach the dispatch. + const injected = { + ...executorConfig, + executor: { kind: 'acp', command: ' ' }, + } as unknown as SubagentConfig; + + await expect( + manager.createAgentHeadless(injected, mockConfig), + ).rejects.toThrow(/failed validation/); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + }); + + it('dispatches to the registered executor with the validated spec', async () => { + const externalSubagent = { execute: vi.fn() }; + const create = vi.fn().mockResolvedValue(externalSubagent as never); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + }); + + const result = await manager.createAgentHeadless( + executorConfig, + mockConfig, + ); + + expect(result.subagent).toBe(externalSubagent); + expect(mockAgentHeadlessCreate).not.toHaveBeenCalled(); + expect(create.mock.calls[0][0]).toMatchObject({ + spec: { kind: 'acp', command: 'npx', args: ['-y', 'some-acp'] }, + name: 'external-agent', + }); + }); + + it('leaves the in-process path untouched when no executor is declared', async () => { + mockAgentHeadlessCreate.mockResolvedValue({ + execute: vi.fn(), + } as never); + const create = vi.fn(); + vi.spyOn(mockConfig, 'getExternalAgentExecutor').mockReturnValue({ + create, + } as never); + + await manager.createAgentHeadless( + { ...executorConfig, executor: undefined }, + mockConfig, + ); + + expect(mockAgentHeadlessCreate).toHaveBeenCalled(); + expect(create).not.toHaveBeenCalled(); + }); + }); + describe('createAgentHeadless model override', () => { const agentConfig: SubagentConfig = { name: 'model-test-agent', diff --git a/packages/core/src/subagents/subagent-manager.ts b/packages/core/src/subagents/subagent-manager.ts index af2387d6737..286ef5a85db 100644 --- a/packages/core/src/subagents/subagent-manager.ts +++ b/packages/core/src/subagents/subagent-manager.ts @@ -8,6 +8,7 @@ import * as fs from 'fs/promises'; import * as path from 'path'; import * as os from 'os'; import { randomUUID } from 'crypto'; +import { isScalar, parseDocument, visit } from 'yaml'; import { parse as parseYaml, stringify as stringifyYaml, @@ -33,6 +34,7 @@ import { } from './types.js'; import { SubagentValidator } from './validation.js'; import { AgentHeadless } from '../agents/runtime/agent-headless.js'; +import type { SubagentExecutor } from '../agents/runtime/subagent-executor.js'; import type { AgentEventEmitter, AgentHooks, @@ -58,6 +60,7 @@ import { COLOR_VALUES, isColor, isPermissionMode, + parseAgentExecutor, parseAgentHooks, parseAgentMcpServers, parseMaxTurns, @@ -103,6 +106,14 @@ function subagentApprovalModesLabel(): string { export class SubagentManager { private readonly validator: SubagentValidator; private subagentsCache: Map | null = null; + // R10-2: executor-block refusals recorded during a level scan, keyed by level + // then lowercased declared name. loadSubagent consults this so a by-name + // dispatch REFUSES instead of falling through to a lower-precedence in-process + // definition of the same name (the silent substitution this feature prevents). + private readonly executorRefusals = new Map< + SubagentLevel, + Map + >(); private readonly changeListeners: Set<() => void> = new Set(); constructor(private readonly config: Config) { @@ -305,6 +316,10 @@ export class SubagentManager { ); } + // The explicit-level path does not fall through to a lower-precedence + // definition or a builtin, so it cannot silently substitute an in-process + // agent — return the level's match (or null) without the R10-2 refusal. + // Only the by-name fall-through path below (used by dispatch) refuses. return this.findSubagentByNameAtLevel(name, level); } @@ -322,12 +337,14 @@ export class SubagentManager { if (projectConfig) { return projectConfig; } + this.throwRecordedExecutorRefusal('project', lowerName); // Try user level const userConfig = await this.findSubagentByNameAtLevel(name, 'user'); if (userConfig) { return userConfig; } + this.throwRecordedExecutorRefusal('user', lowerName); // Try extension level const extensionConfig = await this.findSubagentByNameAtLevel( @@ -337,11 +354,27 @@ export class SubagentManager { if (extensionConfig) { return extensionConfig; } + this.throwRecordedExecutorRefusal('extension', lowerName); // Try built-in agents as fallback return this.getBuiltinAgent(name); } + // R10-2: refuse a by-name dispatch when this level skipped a file that + // declared `name` but had an invalid executor block, instead of falling + // through to a lower-precedence in-process definition (or a builtin) of the + // same name — which would silently substitute a Qwen-model agent for the + // external one the file asked for, with only a discovery-time console.warn. + // Scoped to executor refusals recorded during the level scan, so an arbitrary + // malformed file cannot disable an unrelated builtin. + private throwRecordedExecutorRefusal( + level: SubagentLevel, + lowerName: string, + ): void { + const refusal = this.executorRefusals.get(level)?.get(lowerName); + if (refusal) throw refusal; + } + /** * Updates an existing subagent configuration. * @@ -786,6 +819,18 @@ export class SubagentManager { frontmatter['hooks'] = config.hooks; } + if (config.executor !== undefined) { + const executor = parseAgentExecutor(config.executor); + if (!executor) { + throw new SubagentError( + `Subagent "${config.name}" executor block failed validation. Refusing to save it without its executor.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } + frontmatter['executor'] = executor; + } + // Serialize to YAML const yamlContent = stringifyYaml(frontmatter, { lineWidth: 0, // Disable line wrapping @@ -837,7 +882,7 @@ export class SubagentManager { /** Stable id used to keep one invocation grouped across resume. */ subagentId?: string; }, - ): Promise<{ subagent: AgentHeadless; dispose: () => Promise }> { + ): Promise<{ subagent: SubagentExecutor; dispose: () => Promise }> { // Track per-spawn cleanup callbacks declared outside the inner // `try/catch` so the catch can fire them on a constructor failure // before the caller ever receives the return value. The successful @@ -870,6 +915,124 @@ export class SubagentManager { }; try { + if (config.executor !== undefined) { + // Safe mode promises that only built-in subagents are available and + // that no repo-supplied execution surface runs. Discovery filtering is + // not enough: `loadSubagent` resolves a `subagent_type` from disk + // (project/user/extension) regardless of safe mode, and a cloned repo + // can ship `.qwen/agents/.md` carrying an `executor` that + // resolves before the built-in of the same name. Refuse at the dispatch + // gate so safe mode never spawns a definition-supplied binary. + if (runtimeContext.isSafeMode()) { + throw new SubagentError( + `Subagent "${config.name}" declares an external executor, which safe mode forbids: safe mode restricts subagents to the built-in set and never spawns a definition-supplied binary.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } + const spec = parseAgentExecutor(config.executor); + if (!spec) { + throw new SubagentError( + `Subagent "${config.name}" executor block failed validation. Refusing to run it in-process.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } + if (config.level === 'project' && !runtimeContext.isTrustedFolder()) { + throw new SubagentError( + `Cannot start external agent "${config.name}" from an untrusted project.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } + const unsupported = [ + ['tools', config.tools], + ['disallowedTools', config.disallowedTools], + ['mcpServers', config.mcpServers], + ['hooks', config.hooks], + ['maxTurns', config.maxTurns], + ['runConfig.max_turns', config.runConfig?.max_turns], + [ + 'runConfigOverrides.max_turns', + options?.runConfigOverrides?.max_turns, + ], + [ + 'model', + config.model && config.model !== 'inherit' + ? config.model + : undefined, + ], + [ + 'modelConfigOverrides', + Object.keys(options?.modelConfigOverrides ?? {}).length > 0 + ? options?.modelConfigOverrides + : undefined, + ], + ['runtimeAuthOverrides', options?.runtimeAuthOverrides], + ['toolConfigOverride', options?.toolConfigOverride], + [ + 'promptConfigOverrides.renderedSystemPrompt', + options?.promptConfigOverrides?.renderedSystemPrompt, + ], + [ + 'promptConfigOverrides.initialMessages', + options?.promptConfigOverrides?.initialMessages, + ], + ['hooks callbacks', options?.hooks], + ].filter(([, value]) => value !== undefined); + if (unsupported.length > 0) { + throw new SubagentError( + `External agent "${config.name}" does not support ${unsupported.map(([key]) => key).join(', ')}.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } + const externalExecutor = runtimeContext.getExternalAgentExecutor(); + if (!externalExecutor) { + throw new SubagentError( + `Subagent "${config.name}" declares an executor but this host registered no external agent executor. Refusing to run it in-process.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } + const subagent = await externalExecutor.create({ + spec, + name: config.name, + // The peer permission mode must be the host's effective, already + // clamped approval policy — not the definition's raw request. The + // Agent tool resolves the definition's approvalMode against the + // parent session's mode and folder trust (resolveSubagentApprovalMode) + // and stamps the result onto this runtimeContext, so reading it back + // here is what stops a definition from escalating the external agent + // past the parent session's limit. config.permissionMode is already + // bridged into that resolved value at parse time. + approvalMode: runtimeContext.getApprovalMode(), + permissionMode: config.permissionMode, + promptConfig: { + systemPrompt: config.systemPrompt, + ...options?.promptConfigOverrides, + }, + modelConfig: {}, + runConfig: { + ...config.runConfig, + ...options?.runConfigOverrides, + }, + toolConfig: { + tools: ['*'], + disallowedTools: [ToolNames.ASK_USER_QUESTION], + }, + eventEmitter: options?.eventEmitter, + taskName: options?.taskName, + subagentId: options?.subagentId, + runtimeContext: deriveConfig(runtimeContext), + }); + return { + subagent, + dispose: async () => { + await subagent.dispose?.(); + }, + }; + } const runtimeConfig = await this.convertToRuntimeConfig( config, runtimeContext, @@ -978,6 +1141,12 @@ export class SubagentManager { throw innerError; } } catch (error) { + // Already-classified errors carry an accurate message; re-wrapping them + // under "Failed to create AgentHeadless" would misreport the executor + // path, which never constructs an AgentHeadless at all. + if (error instanceof SubagentError || config.executor !== undefined) { + throw error; + } if (error instanceof Error) { throw new SubagentError( `Failed to create AgentHeadless: ${error.message}`, @@ -1261,6 +1430,13 @@ export class SubagentManager { config: SubagentConfig, runtimeContext?: Config, ): Promise { + if (config.executor !== undefined) { + throw new SubagentError( + `Subagent "${config.name}" declares an external executor and cannot be converted to an in-process agent.`, + SubagentErrorCode.INVALID_CONFIG, + config.name, + ); + } const promptConfig: PromptConfig = { systemPrompt: config.systemPrompt, }; @@ -1431,6 +1607,18 @@ export class SubagentManager { if (level === 'extension') { const extensions = this.config.getActiveExtensions(); + // R10-2: extension agents load via loadSubagentFromDir, which skips and + // warns on a refusal — so the directory-scan branch below never runs for + // them and nothing would be recorded. Merge each active extension's + // recorded executor-block refusals so the by-name fall-through refuses + // them too instead of resolving a builtin of the same name. + const merged = new Map(); + for (const extension of extensions) { + for (const [name, error] of extension.agentExecutorRefusals ?? []) { + merged.set(name, error); + } + } + this.executorRefusals.set('extension', merged); return extensions.flatMap((extension) => extension.agents || []); } @@ -1441,6 +1629,10 @@ export class SubagentManager { // If project level is requested but project root is same as home directory, // return empty array to avoid conflicts between project and global agents if (level === 'project' && isHomeDirectory) { + // No fresh scan happens on this path, so clear any refusal a prior scan + // recorded for the level — otherwise loadSubagent keeps throwing a stale + // refusal for a file this path no longer reads. (R12-4) + this.executorRefusals.set(level, new Map()); return []; } @@ -1453,6 +1645,7 @@ export class SubagentManager { const files = await fs.readdir(baseDir); const subagents: SubagentConfig[] = []; + const refusals = new Map(); for (const file of files) { if (!file.endsWith('.md')) continue; @@ -1467,13 +1660,29 @@ export class SubagentManager { // mistyped frontmatter or used a reserved name had no way to see // why their agent wasn't loading. warnInvalidSubagentFile(filePath, error); + // R10-2: record an executor-block refusal by declared name so a + // by-name dispatch can REFUSE instead of falling through to a + // lower-precedence in-process definition of the same name. Scoped to + // executor refusals (not parse failures generally) so an arbitrary + // malformed repo file cannot disable an unrelated builtin at dispatch. + if ( + error instanceof SubagentError && + error.subagentName !== undefined && + error.message.includes('invalid executor block') + ) { + refusals.set(error.subagentName.toLowerCase(), error); + } continue; } } + this.executorRefusals.set(level, refusals); return subagents; } catch (_error) { - // Directory doesn't exist or can't be read + // Directory doesn't exist or can't be read. Clear any refusal a prior + // scan recorded for this level so a stale entry cannot keep refusing a + // by-name dispatch for a file that no longer exists or is unreadable. (R12-4) + this.executorRefusals.set(level, new Map()); return []; } } @@ -1510,7 +1719,21 @@ export class SubagentManager { * @returns True if name is available */ async isNameAvailable(name: string, level?: SubagentLevel): Promise { - const existing = await this.loadSubagent(name, level); + let existing: SubagentConfig | null; + try { + existing = await this.loadSubagent(name, level); + } catch (error) { + // R10-2: loadSubagent now refuses a name claimed by an invalid-executor + // file. For availability that name IS taken (the file exists), so report + // it unavailable rather than propagating the dispatch-time refusal. + if ( + error instanceof SubagentError && + error.message.includes('invalid executor block') + ) { + return false; + } + throw error; + } if (!existing) { return true; // Name is available @@ -1526,6 +1749,11 @@ export class SubagentManager { export async function loadSubagentFromDir( baseDir: string, + // R10-2: when provided, executor-block refusals (an agent file that declares + // an executor but failed to load) are recorded keyed by lowercased declared + // name, so a by-name dispatch can refuse instead of falling through to a + // builtin of the same name. + refusals?: Map, ): Promise { try { const files = await fs.readdir(baseDir); @@ -1547,6 +1775,14 @@ export async function loadSubagentFromDir( subagents.push(config); } catch (error) { warnInvalidSubagentFile(filePath, error); + if ( + refusals && + error instanceof SubagentError && + error.subagentName !== undefined && + error.message.includes('invalid executor block') + ) { + refusals.set(error.subagentName.toLowerCase(), error); + } continue; } } @@ -1558,12 +1794,58 @@ export async function loadSubagentFromDir( } } +/** + * R12-5: returns true when a raw-text `executor:` probe match at `matchIndex` + * lies inside a block scalar (`description: |` or `>`), i.e. the match is prose + * documenting the executor syntax, not a real claim. The R10-1 probe is + * indentation-tolerant, so without this a `description: |` line reading + * `executor: acp` would hoist into a claim and hard-refuse an in-process + * definition that merely carries an unrelated tolerated YAML quirk. Fail-closed: + * if the AST walk itself throws, return false so the match stays a claim. + */ +function probeMatchInsideBlockScalar( + document: ReturnType, + matchIndex: number, +): boolean { + try { + let inside = false; + visit(document, (_key, node) => { + if ( + isScalar(node) && + (node.type === 'BLOCK_LITERAL' || node.type === 'BLOCK_FOLDED') && + node.range && + node.range[0] <= matchIndex && + matchIndex < node.range[1] + ) { + inside = true; + } + }); + return inside; + } catch { + // A failed walk must not drop a real claim; leave the match as a claim. + return false; + } +} + function parseSubagentContent( content: string, filePath: string, level: SubagentLevel, validator: SubagentValidator, ): SubagentConfig { + // R11-1/R11-4: the executor claim and the trusted declared name are computed + // at FUNCTION scope so the catch can convert ANY load failure of an + // executor-claiming file into a named executor refusal (recorded by + // listSubagentsAtLevel, refused by loadSubagent's fall-through) — otherwise a + // file that fails an EARLIER validation (missing description, bad approvalMode) + // is skipped with nothing recorded, and a by-name dispatch silently falls + // through to a lower-precedence in-process definition or the builtin. The name + // comes from the real AST (parseDocument strips quotes), NOT the lenient value + // — the lenient parser strips only double quotes, so a single-quoted + // `name: 'Explore'` would otherwise be keyed "'explore'" and miss the + // 'explore' dispatch lookup. + let claimsExecutor = false; + let declaredName: string | undefined; try { const normalizedContent = normalizeContent(content); @@ -1580,6 +1862,38 @@ function parseSubagentContent( // Parse YAML frontmatter const frontmatter = parseYaml(frontmatterYaml) as Record; + // Real-AST executor claim + trusted declared name, computed BEFORE any + // validation can throw so the catch can route an executor-claiming file's + // failure to a named refusal. The executor block below reuses `document`, + // `hasExecutor` and `executorClaimMatch`. + const document = parseDocument(frontmatterYaml); + const hasExecutor = document.has('executor'); + const executorClaimMatch = /^[ \t]*["']?executor["']?[ \t]*:/m.exec( + frontmatterYaml, + ); + // R12-5: a probe match lying inside a block scalar is prose (a + // `description: |`/`>` documenting the executor syntax), not a claim — + // exclude it so the refusal guards below do not fire on an in-process + // definition that only mentions `executor:` in prose. A genuine top-level + // (even TAB-misindented) executor key is not inside a block scalar, so it + // stays a claim and the R10-1/R9-2 refusals still hold. + const probeIsClaim = + executorClaimMatch !== null && + !probeMatchInsideBlockScalar(document, executorClaimMatch.index); + claimsExecutor = hasExecutor || probeIsClaim; + try { + const nodeName = document.has('name') ? document.get('name') : undefined; + if (typeof nodeName === 'string' && nodeName !== '') + declaredName = nodeName; + } catch { + // toJS/node reads can throw on an unresolved alias; keep the lenient name. + } + if (declaredName === undefined) { + const lenientName = frontmatter['name']; + if (typeof lenientName === 'string' && lenientName !== '') + declaredName = lenientName; + } + // Extract required fields and convert to strings const nameRaw = frontmatter['name']; const descriptionRaw = frontmatter['description']; @@ -1734,6 +2048,136 @@ function parseSubagentContent( ); } + // executor: qwen-code extension (not part of the mirrored CC schema). + // Strictly validated because it names an external process to run; see + // `parseAgentExecutor`. Availability of the named command is NOT checked + // here — that is the injected executor's job at spawn time. + // + // A malformed block is a hard error, NOT a lenient drop. Dropping it would + // leave `config.executor` undefined, so the definition would run + // in-process: the task completes under Qwen's model, billed to the Qwen + // provider, with nothing on stdout or stderr — precisely the silent + // substitution this feature exists to prevent, and the consumption-point + // re-validation can never catch it because the field is already gone. The + // warn-only path was invisible in practice too, since `debugLogger.warn` + // is a no-op unless QWEN_DEBUG_LOG_FILE is set. This deliberately diverges + // from the lenient posture used for `mcpServers` / `hooks`: losing those + // degrades a capability, whereas losing this one substitutes a different + // agent for the one the definition asked for. + // The shared parser strips nulls, including null arguments. Validate the + // original YAML node so sanitization cannot change the executable request. + // (`document`, `hasExecutor`, `executorClaimMatch` and `claimsExecutor` are + // computed earlier, right after the frontmatter is parsed, so the catch can + // route an executor-claiming file's failure to a named refusal.) + // parseDocument repairs a syntactically invalid document instead of + // throwing, and `document.errors` is the only signal that it did so. For an + // executor-bearing definition that is unsafe: the repaired node can dispatch + // a different command/args than the file declares, or drop the key and run + // in-process. Refuse it and surface the real YAML error (with line/column). + // + // Scope this to executor-bearing files only. Every OTHER definition must + // keep loading through the shared parser's lenient `parseSimple` fallback — + // a description containing a colon, a tab-indented field, or a key + // duplicated by a bad merge loads at the merge base, and refusing it here + // would silently delete an agent from `/agents` (and break + // `subagent_type:`) with nothing on stdout or stderr. Detect the claim from + // the raw text too, so a YAML error that corrupts the `executor` key out of + // the AST still cannot slip through to a silent in-process run. The probe is + // indentation-TOLERANT (`^[ \t]*`, optional quotes) on purpose: a TAB before + // a top-level `executor:` is invalid YAML indentation, so parseDocument + // errors and drops the key, and a column-0-anchored probe would miss it — the + // definition would then load executor-less and run in-process under a Qwen + // model with nothing on any channel, the exact silent substitution this + // feature exists to prevent. The accepted cost is that an `executor:` token + // nested under another key (e.g. `metadata:`) in an otherwise-malformed file + // is also treated as a claim and refused; a visible, user-fixable + // over-refusal beats an invisible substitution or an invented dispatch. The + // claim is NOT read from the lenient `parseSimple` value: that line-based + // heuristic hoists an `executor:` line inside a `description: |` block scalar + // into a top-level key, which would dispatch a command existing only as prose. + // Refuse only when an error can actually corrupt the executor node, not on + // every tolerated YAML error in the file. Two distinct dangers: (1) the real + // YAML parser (parseDocument) lost the executor node (!hasExecutor) although + // the file claims one; do NOT fall back to the lenient parseSimple value, a + // line-based heuristic that mangles block scalars and sequences (command: | + // plus an indented npx becomes command '|', spawning an executable the file + // never declared), and YAML errors are not line-local, so an earlier + // compact-mapping error can drop the whole remainder including executor; + // (2) the AST + // has the executor but an error sits at or after its line, so parseDocument + // may have repaired the node into a different command/args than the file + // declares. An error strictly BEFORE the executor line — a duplicate `name:` + // from a bad merge, say — cannot have reached the executor subtree that + // parseDocument kept (hasExecutor still true), which stays byte-faithful, so + // refusing there would delete a valid external-agent + // definition over an unrelated quirk (it still loads at the merge base via + // the lenient `parseSimple` fallback). When the claim cannot be localized to + // a line, fail closed and treat every error as reaching it. + const executorLine = + executorClaimMatch === null + ? undefined + : frontmatterYaml.slice(0, executorClaimMatch.index).split('\n').length; + if (claimsExecutor && document.errors.length > 0) { + const astLostExecutor = !hasExecutor; + const reachingError = document.errors.find( + (error) => + executorLine === undefined || + error.linePos === undefined || + error.linePos[0].line >= executorLine, + ); + if (astLostExecutor || reachingError !== undefined) { + throw new SubagentError( + `Agent file ${filePath} has an invalid executor block: its frontmatter ` + + `YAML does not parse (${(reachingError ?? document.errors[0]).message}). ` + + `Refusing to load the definition rather than dispatch a silently ` + + `repaired executor.`, + SubagentErrorCode.INVALID_CONFIG, + declaredName ?? name, + ); + } + } + // parseDocument tolerates an unresolved YAML alias (e.g. `command: *undef`) + // without recording a `document.errors` entry, but `toJS()` throws when it + // resolves the node — so the errors-based guard above cannot catch it. Refuse + // gracefully rather than let a raw YAML error escape parseSubagentContent. + let executorRaw: unknown; + if (hasExecutor) { + let documentJs: unknown; + try { + documentJs = document.toJS(); + } catch (error) { + throw new SubagentError( + `Agent file ${filePath} has an invalid executor block: its frontmatter ` + + `YAML does not resolve (${error instanceof Error ? error.message : String(error)}). ` + + `Refusing to load the definition rather than dispatch an executor ` + + `parsed from YAML that will not resolve to a document.`, + SubagentErrorCode.INVALID_CONFIG, + declaredName ?? name, + ); + } + executorRaw = (documentJs as Record)['executor']; + } else { + // Do NOT fall back to the lenient parseSimple value: when parseDocument + // (the real YAML parser) has no top-level executor node, any `executor:` + // token the text probe matched is nested or block-scalar prose, and + // parseSimple's reconstruction is untrustworthy (it invents `command: '|'`, + // hoists prose into a key). A claim the AST lost under a YAML error is + // already refused above; with no error there is genuinely no top-level + // executor, so load without one rather than dispatch an unconfirmed value. + executorRaw = undefined; + } + const executor = parseAgentExecutor(executorRaw); + if ((hasExecutor || executorRaw !== undefined) && executor === undefined) { + throw new SubagentError( + `Agent file ${filePath} has an invalid executor block (expected ` + + `{ kind: 'acp', command: string, args?: string[] }). Refusing to load ` + + `the definition: dropping the block would silently run it in-process ` + + `instead of in the external agent it asked for.`, + SubagentErrorCode.INVALID_CONFIG, + declaredName ?? name, + ); + } + const config: SubagentConfig = { name, description, @@ -1751,6 +2195,7 @@ function parseSubagentContent( ...(maxTurns !== undefined ? { maxTurns } : {}), ...(mcpServers !== undefined ? { mcpServers } : {}), ...(hooks !== undefined ? { hooks } : {}), + ...(executor !== undefined ? { executor } : {}), }; // Validate the parsed configuration @@ -1761,6 +2206,26 @@ function parseSubagentContent( return config; } catch (error) { + // Preserve a SubagentError as-is: an executor-block refusal already carries + // the declared agent name (subagentName), which loadSubagent needs to refuse + // a by-name dispatch instead of falling through (R10-2). Re-wrapping would + // strip the name and code. + if (error instanceof SubagentError) throw error; + // R11-1: a file that CLAIMS an executor but failed to load for ANY reason — + // a missing description, an invalid approvalMode, any validation before or + // after the executor block — must surface as a named executor refusal, so the + // dispatch refuses rather than falling through to an in-process substitute. + // (A file whose own name is unparseable stays undefined-keyed and falls + // through to the generic wrap; it cannot be matched by name anyway.) + if (claimsExecutor && declaredName) { + throw new SubagentError( + `Agent file ${filePath} has an invalid executor block: it declares an ` + + `executor but failed to load (${error instanceof Error ? error.message : 'Unknown error'}). ` + + `Refusing to fall through to a lower-precedence in-process substitute.`, + SubagentErrorCode.INVALID_CONFIG, + declaredName, + ); + } throw new SubagentError( `Failed to parse subagent file: ${error instanceof Error ? error.message : 'Unknown error'}`, SubagentErrorCode.INVALID_CONFIG, @@ -1776,5 +2241,13 @@ function parseSubagentContent( */ function warnInvalidSubagentFile(filePath: string, error: unknown): void { const message = error instanceof Error ? error.message : String(error); + if ( + error instanceof SubagentError && + message.includes('invalid executor block') + ) { + // eslint-disable-next-line no-console -- executor rejection must be visible without debug logging + console.warn(`Skipped invalid file ${filePath}: ${message}`); + return; + } debugLogger.debug(`Skipped invalid file ${filePath}: ${message}`); } diff --git a/packages/core/src/subagents/types.ts b/packages/core/src/subagents/types.ts index ea4c6bf9a30..6dd79e29b11 100644 --- a/packages/core/src/subagents/types.ts +++ b/packages/core/src/subagents/types.ts @@ -43,6 +43,26 @@ export type SubagentLevel = | 'extension' | 'builtin'; +/** + * Declares that a subagent's turn is executed by an external agent process + * speaking ACP, rather than by the in-process reasoning loop. + * + * `parseAgentExecutor` validates this strictly: an unrecognized `kind`, a + * missing/blank `command`, or a malformed `args` yields `undefined`. The + * frontmatter loader then **rejects the whole definition** rather than dropping + * the field — a dropped field would leave the definition running in-process, + * silently substituting a different agent for the one it asked for. This is a + * deliberate divergence from the lenient drop used for `mcpServers` and `hooks`. + */ +export interface SubagentExecutorSpec { + /** Executor kind. `acp` is the only supported value today. */ + kind: 'acp'; + /** Executable to run, resolved on PATH. */ + command: string; + /** Arguments passed to `command`. Omitted when the definition declares none. */ + args?: string[]; +} + /** * Core configuration for a subagent as stored in Markdown files. * This interface represents the file-based configuration that gets @@ -161,6 +181,22 @@ export interface SubagentConfig { */ hooks?: Record; + /** + * Optional external executor. When present, the subagent's turn is run by an + * external agent process instead of the in-process `AgentCore` reasoning + * loop; `SubagentManager.createAgentHeadless` dispatches to the executor + * injected via `Config.setExternalAgentExecutor`. + * + * Absent (the default) keeps the in-process executor. A definition that + * declares an executor while none is injected fails loudly rather than + * silently falling back to in-process. + * + * `command` is resolved on PATH. Project-level executors require a trusted + * workspace. Declared host tool restrictions, MCP servers, hooks and + * model/provider overrides are unsupported and rejected before spawning. + */ + executor?: SubagentExecutorSpec; + /** * Indicates whether this is a built-in agent. * Built-in agents cannot be modified or deleted. diff --git a/packages/core/src/tools/agent/agent.test.ts b/packages/core/src/tools/agent/agent.test.ts index f2b7646a915..c060cbd4449 100644 --- a/packages/core/src/tools/agent/agent.test.ts +++ b/packages/core/src/tools/agent/agent.test.ts @@ -6089,6 +6089,109 @@ describe('AgentTool', () => { }); }); + it.each([true, false])( + 'omits parent attribution and unknown usage for external agents (background=%s)', + async (background) => { + const writeMetaSpy = vi.spyOn(transcript, 'writeAgentMeta'); + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue({ + ...bgSubagent, + executor: { kind: 'acp', command: 'claude' }, + }); + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation({ + description: 'External task', + prompt: 'Do the task', + subagent_type: 'monitor', + run_in_background: background, + }); + const result = await invocation.execute(); + expect(partToString(result.llmContent)).toContain( + background ? 'Background agent launched' : 'Monitor done', + ); + const meta = writeMetaSpy.mock.calls.at(-1)?.[1]; + expect(meta?.executor).toBe('acp'); + expect(meta?.persistedCliFlags).toBeUndefined(); + expect(meta?.model).toBeUndefined(); + expect( + (result.returnDisplay as AgentResultDisplay).executionSummary, + ).toBeUndefined(); + if (background) { + await vi.waitFor(() => + expect(mockRegistry.complete).toHaveBeenCalled(), + ); + expect(mockRegistry.complete.mock.calls[0]?.[2]).toBeUndefined(); + expect(mockRegistry.complete.mock.calls[0]?.[1]).toContain( + 'token usage and cost are unavailable', + ); + expect(mockRegistry.tryReserveBackgroundSlot).toHaveBeenCalledWith( + undefined, + null, + ); + const resident = mockRegistry.registerResidentAgent.mock + .calls[0]?.[1] as { + continue: (message: string) => boolean; + }; + expect(resident.continue('Continue externally')).toBe(true); + await vi.waitFor(() => + expect(mockAgent.execute).toHaveBeenCalledTimes(2), + ); + expect(mockSubagentManager.createAgentHeadless).toHaveBeenCalledTimes( + 1, + ); + } + }, + ); + + it('publishes the real failure reason, not just the usage notice, for a background external agent that produced no text', async () => { + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue({ + ...bgSubagent, + executor: { kind: 'acp', command: 'claude' }, + }); + vi.mocked(mockAgent.getTerminateMode).mockReturnValue( + AgentTerminateMode.TIMEOUT, + ); + vi.mocked(mockAgent.getFinalText).mockReturnValue(''); + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation({ + description: 'External task', + prompt: 'Do the task', + subagent_type: 'monitor', + run_in_background: true, + }); + await invocation.execute(); + await vi.waitFor(() => expect(mockRegistry.fail).toHaveBeenCalled()); + const failureMessage = mockRegistry.fail.mock.calls[0]?.[1] as string; + // The notice must be appended AFTER the fallback, never in place of it. + expect(failureMessage).toContain('Agent terminated with mode: TIMEOUT'); + expect(failureMessage).toContain('token usage and cost are unavailable'); + }); + + it('surfaces that mid-turn input is unavailable for a background external agent (R3-6)', async () => { + vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue({ + ...bgSubagent, + executor: { kind: 'acp', command: 'claude' }, + }); + const invocation = ( + agentTool as AgentToolWithProtectedMethods + ).createInvocation({ + description: 'External task', + prompt: 'Do the task', + subagent_type: 'monitor', + run_in_background: true, + }); + await invocation.execute(); + await vi.waitFor(() => expect(mockRegistry.complete).toHaveBeenCalled()); + const completionText = mockRegistry.complete.mock.calls[0]?.[1] as string; + // ACP v1 has no mid-turn injection primitive, so a steer sent while a turn + // is running reaches the peer only at the next turn boundary. The result + // must say so, not present a queued steer as delivered mid-turn. Dropping + // EXTERNAL_MID_TURN_INPUT_NOTICE turns the second assertion red. + expect(completionText).toContain('token usage and cost are unavailable'); + expect(completionText).toContain('next turn boundary'); + }); + it('should run in background when agent definition has background: true', async () => { const writeMetaSpy = vi.spyOn(transcript, 'writeAgentMeta'); const attachSpy = vi.spyOn(transcript, 'attachJsonlTranscriptWriter'); diff --git a/packages/core/src/tools/agent/agent.ts b/packages/core/src/tools/agent/agent.ts index 2bc685bcf9a..6e2199922f8 100644 --- a/packages/core/src/tools/agent/agent.ts +++ b/packages/core/src/tools/agent/agent.ts @@ -34,6 +34,7 @@ import { AgentHeadless, ContextState, } from '../../agents/runtime/agent-headless.js'; +import type { SubagentExecutor } from '../../agents/runtime/subagent-executor.js'; import type { AgentExternalInput } from '../../agents/runtime/agent-types.js'; import type { Content } from '@google/genai'; import { @@ -133,6 +134,16 @@ import type { import { buildModelIdContext, resolveModelId } from '../../utils/modelId.js'; import type { AuthOverrides } from '../../models/content-generator-config.js'; +const EXTERNAL_USAGE_NOTICE = + '\n\n[External executor token usage and cost are unavailable.]'; + +// ACP v1 has no mid-turn injection primitive, so an external agent cannot be +// steered while a prompt is in flight — queued input is delivered at the next +// turn boundary. Surface that on the result so a caller does not read a queued +// steer as having been delivered mid-turn. (R3-6) +const EXTERNAL_MID_TURN_INPUT_NOTICE = + '\n\n[External agents receive queued input only between turns; a message sent while a turn is running is delivered at the next turn boundary, not mid-turn.]'; + function persistBackgroundCancellation( metaPath: string, persistedStatus: 'running' | 'cancelled', @@ -1687,7 +1698,7 @@ class AgentToolInvocation extends BaseToolInvocation { eventEmitter: AgentEventEmitter = this.eventEmitter, subagentId?: string, ): Promise<{ - subagent: AgentHeadless; + subagent: SubagentExecutor; initialMessages?: Content[]; taskPrompt: string; toolConfig: ToolConfig; @@ -1876,7 +1887,7 @@ class AgentToolInvocation extends BaseToolInvocation { // the reason back and re-executes until the configured cap prevents a // misconfigured hook from looping forever. private async runSubagentStopHookLoop( - subagent: AgentHeadless, + subagent: SubagentExecutor, opts: { agentId: string; agentType: string; @@ -2098,12 +2109,13 @@ class AgentToolInvocation extends BaseToolInvocation { * as execution progresses. */ private async runSubagentWithHooks( - subagent: AgentHeadless, + subagent: SubagentExecutor, contextState: ContextState, opts: { agentId: string; agentType: string; resolvedMode: PermissionMode; + externalExecutor?: boolean; signal?: AbortSignal; updateOutput?: (output: ToolResultDisplay) => void; /** @@ -2165,7 +2177,9 @@ class AgentToolInvocation extends BaseToolInvocation { stopHookWarning, ); const success = terminateMode === AgentTerminateMode.GOAL; - const executionSummary = subagent.getExecutionSummary(); + const executionSummary = opts.externalExecutor + ? undefined + : subagent.getExecutionSummary(); // Publish span outcome BEFORE side-effectful UI/registry calls — if // updateDisplay throws, the subagent's real terminal state must @@ -2726,31 +2740,33 @@ class AgentToolInvocation extends BaseToolInvocation { } if (shouldRunInBackground) { - // Resolve the concrete model the sub-agent (or fork) will run with so the - // registry can apply a per-model cap. `subagentConfig.model` is a - // selector (omitted/"inherit"/"fast"/modelId/authType:modelId); - // resolveModelId maps it to the actual model ID, falling back to the - // parent's current model when the sub-agent inherits (forks always - // inherit, since FORK_AGENT has no model selector). - const resolvedSubagentModel = resolveModelId( - subagentConfig.model, - buildModelIdContext(this.config), - ); - subagentModelId = resolvedSubagentModel?.modelId; - subagentModelId ??= this.config.getModel(); - const parentContentGeneratorConfig = - this.config.getContentGeneratorConfig(); - const authType = - resolvedSubagentModel?.authType ?? - parentContentGeneratorConfig.authType; - subagentRuntimeAuthOverrides = authType - ? { - authType, - ...(authType === parentContentGeneratorConfig.authType - ? { baseUrl: parentContentGeneratorConfig.baseUrl } - : {}), - } - : undefined; + if (subagentConfig.executor === undefined) { + // Resolve the concrete model the sub-agent (or fork) will run with so the + // registry can apply a per-model cap. `subagentConfig.model` is a + // selector (omitted/"inherit"/"fast"/modelId/authType:modelId); + // resolveModelId maps it to the actual model ID, falling back to the + // parent's current model when the sub-agent inherits (forks always + // inherit, since FORK_AGENT has no model selector). + const resolvedSubagentModel = resolveModelId( + subagentConfig.model, + buildModelIdContext(this.config), + ); + subagentModelId = resolvedSubagentModel?.modelId; + subagentModelId ??= this.config.getModel(); + const parentContentGeneratorConfig = + this.config.getContentGeneratorConfig(); + const authType = + resolvedSubagentModel?.authType ?? + parentContentGeneratorConfig.authType; + subagentRuntimeAuthOverrides = authType + ? { + authType, + ...(authType === parentContentGeneratorConfig.authType + ? { baseUrl: parentContentGeneratorConfig.baseUrl } + : {}), + } + : undefined; + } const registry = this.config.getBackgroundTaskRegistry(); backgroundSlotReservation = registry.tryReserveBackgroundSlot( subagentModelId, @@ -3003,6 +3019,7 @@ class AgentToolInvocation extends BaseToolInvocation { // rows, and the meta sidecar all read this field. agentType: subagentConfig.name, resolvedMode, + externalExecutor: subagentConfig.executor !== undefined, signal, updateOutput, }; @@ -3027,7 +3044,7 @@ class AgentToolInvocation extends BaseToolInvocation { // Create the subagent. Fork bypasses SubagentManager because its runtime // configs are synthesized from the parent's cache-safe params. - let subagent: AgentHeadless; + let subagent: SubagentExecutor; let taskPrompt: string; let initialMessages: Content[] | undefined; let toolConfig: ToolConfig | undefined; @@ -3302,13 +3319,17 @@ class AgentToolInvocation extends BaseToolInvocation { executionAllowedTools: [...bgToolConfig.executionAllowedTools], } : {}), - persistedCliFlags: capturePersistedCliFlags( - this.config, - resolvedApprovalMode, - bgSubagent.getCore().modelConfig.model, - bgSubagent.getCore().runtimeView?.contentGeneratorConfig ?? - subagentRuntimeAuthOverrides, - ), + executor: subagentConfig.executor?.kind, + persistedCliFlags: + subagentConfig.executor !== undefined + ? undefined + : capturePersistedCliFlags( + this.config, + resolvedApprovalMode, + bgSubagent.getCore().modelConfig.model, + bgSubagent.getCore().runtimeView?.contentGeneratorConfig ?? + subagentRuntimeAuthOverrides, + ), subagentName: subagentConfig.name, agentColor: subagentConfig.color, resumeCount: 0, @@ -3333,7 +3354,12 @@ class AgentToolInvocation extends BaseToolInvocation { let liveToolCallCount = 0; const refreshLiveStats = () => { const entry = registry.get(hookOpts.agentId); - if (!entry || entry.status !== 'running') return; + if ( + !entry || + entry.status !== 'running' || + subagentConfig.executor !== undefined + ) + return; const summary = bgSubagent.getExecutionSummary(); entry.stats = { totalTokens: summary.totalTokens, @@ -3386,6 +3412,9 @@ class AgentToolInvocation extends BaseToolInvocation { ); const getCompletionStats = () => { + // The shared summary requires known token counts. Keep external + // usage absent rather than describing an unmetered run as free. + if (subagentConfig.executor !== undefined) return undefined; const summary = bgSubagent.getExecutionSummary(); return { totalTokens: summary.totalTokens, @@ -3563,18 +3592,27 @@ class AgentToolInvocation extends BaseToolInvocation { const wtSuffix = formatWorktreeSuffix( hadWorktreeIsolation ? await cleanupWorktreeIsolation() : {}, ); + // The usage notice is a suffix, not part of the model-visible + // text: baking it into finalText would make the `finalText || + // ` fallbacks below see a non-empty string and publish + // the notice in place of the real failure reason for any + // non-GOAL external run that produced no text. The foreground + // path already appends it after its fallbacks; mirror that. + const externalSuffix = + subagentConfig.executor !== undefined + ? EXTERNAL_USAGE_NOTICE + EXTERNAL_MID_TURN_INPUT_NOTICE + : ''; const modelVisibleText = toModelVisibleSubagentResult( subagentRawText, terminateMode, ); - const finalText = - appendStopHookBlockingCapWarning( - terminateMode === AgentTerminateMode.GOAL - ? modelVisibleText || - '(subagent produced no model-visible output)' - : modelVisibleText, - stopHookWarning, - ) + wtSuffix; + const finalText = appendStopHookBlockingCapWarning( + terminateMode === AgentTerminateMode.GOAL + ? modelVisibleText || + '(subagent produced no model-visible output)' + : modelVisibleText, + stopHookWarning, + ); const completionStats = getCompletionStats(); if ( terminateMode === AgentTerminateMode.GOAL && @@ -3617,7 +3655,11 @@ class AgentToolInvocation extends BaseToolInvocation { ) : {}), }); - registry.complete(hookOpts.agentId, finalText, completionStats); + registry.complete( + hookOpts.agentId, + finalText + wtSuffix + externalSuffix, + completionStats, + ); } else if ( terminateMode === AgentTerminateMode.CANCELLED || terminateMode === AgentTerminateMode.SHUTDOWN @@ -3629,7 +3671,7 @@ class AgentToolInvocation extends BaseToolInvocation { // wenshao @ #4410. registry.finalizeCancelled( hookOpts.agentId, - finalText, + finalText + wtSuffix + externalSuffix, completionStats, ); persistBackgroundCancellation( @@ -3641,16 +3683,16 @@ class AgentToolInvocation extends BaseToolInvocation { registry.get(hookOpts.agentId)?.recentActivities, ); } else { - registry.fail( - hookOpts.agentId, - finalText || `Agent terminated with mode: ${terminateMode}`, - completionStats, - ); + const failureText = + (finalText || + `Agent terminated with mode: ${terminateMode}`) + + wtSuffix + + externalSuffix; + registry.fail(hookOpts.agentId, failureText, completionStats); patchAgentMeta(metaPath, { status: 'failed', lastUpdatedAt: new Date().toISOString(), - lastError: - finalText || `Agent terminated with mode: ${terminateMode}`, + lastError: failureText, ...(sessionWorkflowAgent ? getAgentMetaTerminalSummary( completionStats, @@ -4048,7 +4090,12 @@ class AgentToolInvocation extends BaseToolInvocation { let fgLiveToolCallCount = 0; const refreshFgLiveStats = () => { const entry = registry.get(hookOpts.agentId); - if (!entry || entry.status !== 'running') return; + if ( + !entry || + entry.status !== 'running' || + subagentConfig.executor !== undefined + ) + return; const summary = subagent.getExecutionSummary(); entry.stats = { totalTokens: summary.totalTokens, @@ -4151,11 +4198,15 @@ class AgentToolInvocation extends BaseToolInvocation { executionAllowedTools: [...toolConfig.executionAllowedTools], } : {}), - persistedCliFlags: capturePersistedCliFlags( - this.config, - resolvedApprovalMode, - subagent.getCore().modelConfig.model, - ), + executor: subagentConfig.executor?.kind, + persistedCliFlags: + subagentConfig.executor !== undefined + ? undefined + : capturePersistedCliFlags( + this.config, + resolvedApprovalMode, + subagent.getCore().modelConfig.model, + ), subagentName: subagentConfig.name, agentColor: subagentConfig.color, resumeCount: 0, @@ -4170,7 +4221,9 @@ class AgentToolInvocation extends BaseToolInvocation { toModelVisibleSubagentResult(subagent.getFinalText(), terminateMode), stopHookWarning, ); - const wtSuffix = formatWorktreeSuffix(await cleanupWorktreeIsolation()); + const wtSuffix = + formatWorktreeSuffix(await cleanupWorktreeIsolation()) + + (subagentConfig.executor !== undefined ? EXTERNAL_USAGE_NOTICE : ''); if (terminateMode === AgentTerminateMode.ERROR) { return { llmContent: (finalText || 'Subagent execution failed.') + wtSuffix, diff --git a/scripts/tests/cross-package-contracts.test.js b/scripts/tests/cross-package-contracts.test.js index ce3afc86f8a..a04114c61be 100644 --- a/scripts/tests/cross-package-contracts.test.js +++ b/scripts/tests/cross-package-contracts.test.js @@ -112,3 +112,34 @@ it('distinguishes locale resolution from prompt sanitization', () => { definitionFiles('^export function getSanitizedExtensionDisplayName[(]'), ).toEqual(['packages/cli/src/utils/extension-mention.ts']); }); + +it('publishes the external subagent runtime through its declared subpath', () => { + const pkg = JSON.parse( + readFileSync(join(root, 'packages/core/package.json'), 'utf8'), + ); + expect(pkg.exports['./subagentRuntime']).toEqual({ + types: './dist/src/subagent-runtime.d.ts', + import: './dist/src/subagent-runtime.js', + }); + const barrel = readFileSync( + join(root, 'packages/core/src/subagent-runtime.ts'), + 'utf8', + ); + expect(barrel).toContain("from './agents/runtime/subagent-executor.js'"); + expect(barrel).toContain('ExternalAgentExecutor'); + expect(barrel).toContain('AgentEventEmitter'); + const aliases = readFileSync( + join(root, 'packages/cli/vitest.config.ts'), + 'utf8', + ); + expect(aliases).toMatch( + /'@qwen-code\/qwen-code-core\/subagentRuntime'\s*:\s*path\.resolve\(\s*__dirname,\s*'\.\.\/core\/src\/subagent-runtime\.ts',?\s*\)/, + ); + const tsconfig = readFileSync( + join(root, 'packages/cli/tsconfig.json'), + 'utf8', + ); + expect(tsconfig).toMatch( + /"@qwen-code\/qwen-code-core\/subagentRuntime"\s*:\s*\[\s*"\.\.\/core\/src\/subagent-runtime\.ts"\s*\]/, + ); +});