From 542a01c40738d3211a45a2570e945c66019900be Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 30 Jul 2026 15:15:38 +0800 Subject: [PATCH] test(acp): cover the continuable drain ordering and its failure path Pins that the bridge releases the Activation forest before its own sessions, and that a failed drain is reported without stranding that teardown. Reads the one teardown method structurally so the bridge keeps no dependency on the subagent seam. --- packages/acp/acp/src/index.ts | 14 +++++++++- packages/acp/acp/tests/dispose.spec.ts | 38 ++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/acp/acp/src/index.ts b/packages/acp/acp/src/index.ts index d76a184f5e..d19a2e5753 100644 --- a/packages/acp/acp/src/index.ts +++ b/packages/acp/acp/src/index.ts @@ -43,6 +43,16 @@ export const name = 'acp' /** The bridge creates and owns agents; every other concern is carried by the agent composition. */ export const inject = ['agents'] +/** + * The single continuable-subagent teardown the bridge needs. Declared + * structurally so this package does not depend on the subagent seam for one + * shutdown hook; an absent service means nothing continuable was materialized. + */ +interface ContinuableDrain { + /** Close continuable admission, then dispose every live Activation child-first. */ + drainContinuable(): Promise +} + /** Preserve invalid-parameter detail in the SDK wire error message. */ function invalidParams(detail: string): RequestError { return RequestError.invalidParams(undefined, detail) @@ -331,7 +341,9 @@ export function apply(ctx: Context, config: AcpConfig): void { // Activations own descendant teardown. Drain that forest child-first // BEFORE disposing the top-level agents, so no descendant is left holding // a runtime its owner already released. - const subagents = ctx.get('subagents') + // Read the one teardown method structurally: the bridge needs no other + // part of the subagent seam, so it does not depend on that package. + const subagents = ctx.get('subagents') as ContinuableDrain | undefined if (subagents !== undefined) { try { await subagents.drainContinuable() diff --git a/packages/acp/acp/tests/dispose.spec.ts b/packages/acp/acp/tests/dispose.spec.ts index ee57d51baf..64bab702b3 100644 --- a/packages/acp/acp/tests/dispose.spec.ts +++ b/packages/acp/acp/tests/dispose.spec.ts @@ -25,6 +25,44 @@ describe('ACP connection ownership', () => { expect(harness.ctx.agents.get(SessionId(sessionId))).toBeUndefined() }) + it('drains continuable subagents before disposing its own sessions', async () => { + harness = await makeBridgeHarness() + const order: string[] = [] + // A continuable Activation outlives the turn that started it, so the bridge + // must release that forest before the agents whose runtime it depends on. + harness.ctx.provide('subagents', { + drainContinuable: () => { + order.push('drained') + return Promise.resolve() + }, + } as never, true) + await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + const { sessionId } = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [] }) + harness.ctx.on('agent/disposed', () => { order.push('agent disposed') }) + + await harness.acpFiber.dispose() + + expect(order).toEqual(['drained', 'agent disposed']) + expect(harness.ctx.agents.get(SessionId(sessionId))).toBeUndefined() + }) + + it('reports a failed continuable drain and still disposes its sessions', async () => { + harness = await makeBridgeHarness() + const warnings: string[] = [] + harness.ctx.logger.warn = (message: string) => { warnings.push(message) } + harness.ctx.provide('subagents', { + drainContinuable: () => Promise.reject(new Error('activation teardown failed')), + } as never, true) + await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} }) + const { sessionId } = await harness.client.newSession({ cwd: process.cwd(), mcpServers: [] }) + + await harness.acpFiber.dispose() + + // A stuck descendant must not strand the bridge's own teardown. + expect(warnings.some(warning => warning.includes('continuable subagent teardown failed'))).toBe(true) + expect(harness.ctx.agents.get(SessionId(sessionId))).toBeUndefined() + }) + it('an ACP-only reload rejects new sessions before creating an orphan', async () => { harness = await makeBridgeHarness() await harness.client.initialize({ protocolVersion: PROTOCOL_VERSION, clientCapabilities: {} })