From ac88b6e4c79df1a8e109009a02b63a7955bbf469 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 2 Aug 2026 20:14:46 +0800 Subject: [PATCH] fix(acp): retain nested teardown diagnostics ACP waits for every owned Agent disposal and throws one AggregateError when any Session teardown fails. The aggregate message embedded each rejected value with String(failure) because the connection-close logger itself renders only the outer error message. String preserves only an Error name and message, so causes and AggregateError members disappeared from the operational warning. Render each per-session rejection with the existing errorChain diagnostic helper before joining it into the outer message. The original rejected values remain in AggregateError.errors for programmatic inspection, while the message now carries cause chains and nested aggregate members through the String-based logger. Exercise a disposal failure containing both AggregateError members and a nested cause, while retaining the existing barrier that proves the second Session finishes disposal before any warning is emitted. All 10 ACP disposal tests pass and the ACP TypeScript project builds cleanly. --- packages/acp/acp/src/index.ts | 11 +++++------ packages/acp/acp/tests/dispose.spec.ts | 12 +++++++++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/acp/acp/src/index.ts b/packages/acp/acp/src/index.ts index 7af26b594a..b88322012c 100644 --- a/packages/acp/acp/src/index.ts +++ b/packages/acp/acp/src/index.ts @@ -14,7 +14,7 @@ import { randomUUID } from 'node:crypto' import { isAbsolute } from 'node:path' import { Readable, Writable } from 'node:stream' import Schema from 'schemastery' -import { createUserMessage } from '@deepseek-ai/dsh-llm' +import { createUserMessage, errorChain } from '@deepseek-ai/dsh-llm' import { AgentSideConnection, ndJsonStream, @@ -368,11 +368,10 @@ export function apply(ctx: Context, config: AcpConfig): void { if (result.status === 'rejected') failures.push(result.reason as unknown) } if (failures.length > 0) { - // The only production consumer logs this error through `String`, which - // renders the message alone — without the joined reasons, per-session - // disposal failures would vanish from operational logs. Join them like - // the subagent seam's own aggregate disposal messages. - const detail = failures.map(failure => String(failure)).join('; ') + // The production consumer logs this AggregateError through `String`, + // which renders only its message. Embed every per-session diagnostic, + // including nested causes and aggregate members, in that message. + const detail = failures.map(failure => errorChain(failure)).join('; ') throw new AggregateError( failures, `ACP agent teardown failed for ${failures.length} session(s): ${detail}`, diff --git a/packages/acp/acp/tests/dispose.spec.ts b/packages/acp/acp/tests/dispose.spec.ts index 0eea014eeb..b4b1eaeef2 100644 --- a/packages/acp/acp/tests/dispose.spec.ts +++ b/packages/acp/acp/tests/dispose.spec.ts @@ -96,7 +96,7 @@ describe('ACP connection ownership', () => { expect(harness.ctx.agents.get(SessionId(sessionId))).toBeUndefined() }) - it('awaits every owned session disposal before reporting one failure', async () => { + it('awaits every owned session disposal and reports nested failure reasons', async () => { harness = await makeBridgeHarness() const create = harness.ctx.agents.create.bind(harness.ctx.agents) const releaseSecond = Promise.withResolvers() @@ -110,7 +110,10 @@ describe('ACP connection ownership', () => { if (created++ === 0) { handle.dispose = async () => { await originalDispose() - throw new Error('first session cleanup failed') + throw new AggregateError([ + new Error('scope cleanup failed', { cause: new Error('sqlite busy') }), + new Error('hook cleanup failed'), + ], 'first session cleanup failed') } } else { handle.dispose = async () => { @@ -132,7 +135,10 @@ describe('ACP connection ownership', () => { releaseSecond.resolve(undefined) await vi.waitFor(() => { expect(warnings.some(warning => - warning.includes('ACP agent teardown failed for 1 session(s): Error: first session cleanup failed'))).toBe(true) + warning.includes( + 'ACP agent teardown failed for 1 session(s): ' + + 'first session cleanup failed [scope cleanup failed: sqlite busy; hook cleanup failed]', + ))).toBe(true) expect(harness!.ctx.agents.get(SessionId(first.sessionId))).toBeUndefined() expect(harness!.ctx.agents.get(SessionId(second.sessionId))).toBeUndefined() })