diff --git a/examples/acp-agent/cordis.snapshot.yml b/examples/acp-agent/cordis.snapshot.yml index 5bee10f2a7..5f36a11efa 100644 --- a/examples/acp-agent/cordis.snapshot.yml +++ b/examples/acp-agent/cordis.snapshot.yml @@ -42,8 +42,10 @@ a fresh child agent (it works in its own context and returns only its final result) — give it a complete, standalone instruction. -# The subagent seam + both in-process backends + the model-facing `subagent` -# tool — identical to cordis.yml's wiring (only the LLM backend differs above). +# The subagent seam + both in-process backends + two model-facing tools — +# identical to cordis.yml's wiring (only the LLM backend differs above): spawn +# and fork are each reachable via a dsh-tool-subagent bound to it with a distinct +# toolName (subagent → spawn, subagent_fork → fork). - id: subagent name: '@deepseek-ai/dsh-subagent' @@ -61,3 +63,10 @@ name: '@deepseek-ai/dsh-tool-subagent' config: provider: spawn + toolName: subagent + +- id: tool-subagent-fork + name: '@deepseek-ai/dsh-tool-subagent' + config: + provider: fork + toolName: subagent_fork diff --git a/examples/acp-agent/cordis.yml b/examples/acp-agent/cordis.yml index e00e868dce..a00d0e6036 100644 --- a/examples/acp-agent/cordis.yml +++ b/examples/acp-agent/cordis.yml @@ -51,10 +51,12 @@ a fresh child agent (it works in its own context and returns only its final result) — give it a complete, standalone instruction. -# The subagent seam + both in-process backends + the model-facing `subagent` -# tool, as leaf entries after the app (which provides ctx.agents/ctx.tools). The -# tool is bound to the `spawn` backend (a fresh child); the `fork` backend is -# loaded too so a multi-child scenario can exercise both transports. +# The subagent seam + both in-process backends + two model-facing tools, as leaf +# entries after the app (which provides ctx.agents/ctx.tools). spawn (a fresh +# child) and fork (a child seeded with the parent's completed-turn prefix) are +# both reachable by the model: dsh-tool-subagent is loaded once per backend with +# a distinct toolName (subagent → spawn, subagent_fork → fork), so a multi-child +# scenario can exercise both transports. - id: subagent name: '@deepseek-ai/dsh-subagent' @@ -72,3 +74,10 @@ name: '@deepseek-ai/dsh-tool-subagent' config: provider: spawn + toolName: subagent + +- id: tool-subagent-fork + name: '@deepseek-ai/dsh-tool-subagent' + config: + provider: fork + toolName: subagent_fork diff --git a/packages/subagent/tool-subagent/src/index.ts b/packages/subagent/tool-subagent/src/index.ts index a19e09db98..05490127ea 100644 --- a/packages/subagent/tool-subagent/src/index.ts +++ b/packages/subagent/tool-subagent/src/index.ts @@ -136,6 +136,11 @@ export function apply(ctx: Context, config: Config): void { // aborted while the child is in flight, cancel the child too. const onAbort = (): void => { run.cancel('parent step aborted') } exec.signal?.addEventListener('abort', onAbort, { once: true }) + // `addEventListener` does NOT fire for a signal already aborted before this + // line, so a step cancelled before the tool ran would never reach the + // child. Cancel explicitly in that case — the bridge must honor an + // already-aborted signal, not lean on each provider re-checking it. + if (exec.signal?.aborted) run.cancel('parent step aborted') try { const result = await run.result diff --git a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts index 521fdfba50..dda4e7c3d0 100644 --- a/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts +++ b/packages/subagent/tool-subagent/tests/tool-subagent.spec.ts @@ -282,6 +282,43 @@ describe('dsh-tool-subagent', () => { expect(result.isError).toBe(true) }) + it('cancels the run when the tool signal is ALREADY aborted before execute (no missed abort)', async () => { + // `addEventListener('abort')` does not fire for a signal already aborted + // before the listener is added, so a step cancelled before the tool ran + // would never reach the child unless the bridge re-checks `signal.aborted`. + // A provider that leans only on the abort EVENT (this spy never inspects + // request.signal) proves the bridge itself must cancel. + const cancelled = vi.fn() + const ctx = new Context() + await ctx.plugin(SystemPrompt) + await ctx.plugin(ToolRegistry) + await ctx.plugin(SubagentService) + ctx.subagents.registerProvider({ + name: 'spy', + capabilities: { outputSchema: false, depthLimit: false, toolFilter: false }, + start: () => { + let resolveResult: (r: { output: never[]; stopReason: 'aborted' }) => void + const result = new Promise<{ output: never[]; stopReason: 'aborted' }>((res) => { resolveResult = res }) + return { + id: AgentId('spy-child'), + result, + cancel: () => { + cancelled() + resolveResult({ output: [], stopReason: 'aborted' }) + }, + dispose: async () => {}, + } + }, + }) + await ctx.plugin(tool, { provider: 'spy' }) + + const controller = new AbortController() + controller.abort() // already aborted BEFORE the tool runs + const result = await callSubagent(ctx, { description: 'd', prompt: 'p' }, { signal: controller.signal }) + expect(cancelled).toHaveBeenCalledTimes(1) + expect(result.isError).toBe(true) + }) + it('tools depend on the service: no `subagent` tool without ctx.subagents', async () => { const ctx = new Context() await ctx.plugin(SystemPrompt)