From 30b9d0464a0b66cb1a1190f62043db6a5eb519dd Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Mon, 27 Jul 2026 10:22:36 +0800 Subject: [PATCH] fix: address codex review round 3 Recheck the listing cancellation signal after each child inspection settles: a per-child read failing with a diagnostic-mapped code during an abort previously bypassed the inspection's own checkpoints, letting a cancelled single-child scan return a successful result. --- packages/subagent/subagent/src/index.ts | 5 +++++ .../subagent/tests/list-children.spec.ts | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/packages/subagent/subagent/src/index.ts b/packages/subagent/subagent/src/index.ts index 361baa00fe..33b2883521 100644 --- a/packages/subagent/subagent/src/index.ts +++ b/packages/subagent/subagent/src/index.ts @@ -275,6 +275,11 @@ export class SubagentService extends Service { const entries: SubagentListEntry[] = [] for (const node of trace.descendants) { const entry = await this.inspectChild(query, parentSessionId, node.session, signal) + // Recheck after the inspection settles, not only inside it: a mapped + // per-child failure during an abort becomes a diagnostic and skips the + // inspection's own checkpoints, and a cancelled scan must not return a + // successful result or start another candidate read. + assertListingNotCancelled(signal) if (entry !== undefined) entries.push(entry) } return entries diff --git a/packages/subagent/subagent/tests/list-children.spec.ts b/packages/subagent/subagent/tests/list-children.spec.ts index f7d9d5cb8f..2e5dfc1498 100644 --- a/packages/subagent/subagent/tests/list-children.spec.ts +++ b/packages/subagent/subagent/tests/list-children.spec.ts @@ -429,6 +429,23 @@ describe('SubagentService.listChildren', () => { expect(exactReads).toBe(1) }) + it('a mapped per-child failure during an abort cannot become a successful result', async () => { + const { ctx, parent } = await setup([textResponse('done')]) + await startChild(ctx, parent, 'aborted behind a diagnostic') + const controller = new AbortController() + const query = ctx.get('sessionQuery')! + query.listEvents = () => { + // The read fails with a diagnostic-mapped code while the caller aborts: + // the loop's post-inspection checkpoint must fail the scan rather than + // return a one-diagnostic success. + controller.abort() + return Promise.reject(new SessionQueryError('backend read failed', 'SESSION_QUERY_PERSISTENCE_FAILED')) + } + await expect(ctx.subagents.listChildren(parent.id, controller.signal)).rejects.toThrow( + expect.objectContaining({ code: 'CANCELLED' }) as Error, + ) + }) + it('a pre-aborted signal stops before any candidate read', async () => { const { ctx, parent } = await setup([textResponse('done')]) await startChild(ctx, parent, 'never read')