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.
This commit is contained in:
@@ -275,6 +275,11 @@ export class SubagentService extends Service {
|
|||||||
const entries: SubagentListEntry[] = []
|
const entries: SubagentListEntry[] = []
|
||||||
for (const node of trace.descendants) {
|
for (const node of trace.descendants) {
|
||||||
const entry = await this.inspectChild(query, parentSessionId, node.session, signal)
|
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)
|
if (entry !== undefined) entries.push(entry)
|
||||||
}
|
}
|
||||||
return entries
|
return entries
|
||||||
|
|||||||
@@ -429,6 +429,23 @@ describe('SubagentService.listChildren', () => {
|
|||||||
expect(exactReads).toBe(1)
|
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 () => {
|
it('a pre-aborted signal stops before any candidate read', async () => {
|
||||||
const { ctx, parent } = await setup([textResponse('done')])
|
const { ctx, parent } = await setup([textResponse('done')])
|
||||||
await startChild(ctx, parent, 'never read')
|
await startChild(ctx, parent, 'never read')
|
||||||
|
|||||||
Reference in New Issue
Block a user