Coder/deepseek-harness
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(acp): retain nested teardown diagnostics

Browse Source 
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.
This commit is contained in:
Tianyi Cui
2026-08-02 20:14:46 +08:00
parent 7b40fd5419
commit ac88b6e4c7
2 changed files with 14 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
packages/acp/acp/src/index.ts
View File

@@ -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}`,

12
packages/acp/acp/tests/dispose.spec.ts
View File

@@ -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<undefined>()
@@ -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()
})