fix review findings: reject queued ask aborts
This commit is contained in:
@@ -223,13 +223,10 @@ export function createStdioChat(ctx: Context, config: Config, runtime: StdioRunt
|
|||||||
if (activeQuestion !== undefined) return
|
if (activeQuestion !== undefined) return
|
||||||
const pending = questionQueue.shift()
|
const pending = questionQueue.shift()
|
||||||
if (pending === undefined) return
|
if (pending === undefined) return
|
||||||
if (pending.request.signal?.aborted) {
|
// The queue never contains an aborted pending ask: the seam rejects an
|
||||||
pending.reject(new UserInteractionError('ask_user_question was aborted before the user answered', 'ASK_ABORTED'))
|
// already-aborted request synchronously, and queued asks attach their
|
||||||
startNextQuestion()
|
// abort listener before enqueueing.
|
||||||
return
|
|
||||||
}
|
|
||||||
activeQuestion = pending
|
activeQuestion = pending
|
||||||
pending.request.signal?.addEventListener('abort', pending.onAbort, { once: true })
|
|
||||||
renderQuestion(pending)
|
renderQuestion(pending)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -325,11 +322,19 @@ export function createStdioChat(ctx: Context, config: Config, runtime: StdioRunt
|
|||||||
resolve,
|
resolve,
|
||||||
reject,
|
reject,
|
||||||
onAbort: () => {
|
onAbort: () => {
|
||||||
activeQuestion = undefined
|
if (activeQuestion === pending) {
|
||||||
|
activeQuestion = undefined
|
||||||
|
disposeQuestion(pending)
|
||||||
|
startNextQuestion()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
// If it is not active, this listener can only fire while the ask
|
||||||
|
// remains queued; settled asks remove the listener first.
|
||||||
|
questionQueue.splice(questionQueue.indexOf(pending), 1)
|
||||||
disposeQuestion(pending)
|
disposeQuestion(pending)
|
||||||
startNextQuestion()
|
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
request.signal?.addEventListener('abort', pending.onAbort, { once: true })
|
||||||
questionQueue.push(pending)
|
questionQueue.push(pending)
|
||||||
startNextQuestion()
|
startNextQuestion()
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -550,15 +550,44 @@ describe('createStdioChat input', () => {
|
|||||||
const controller = new AbortController()
|
const controller = new AbortController()
|
||||||
const first = ctx.userInteraction.ask({ questions: [{ id: 'first', question: 'First?' }] })
|
const first = ctx.userInteraction.ask({ questions: [{ id: 'first', question: 'First?' }] })
|
||||||
const second = ctx.userInteraction.ask({ questions: [{ id: 'second', question: 'Second?' }], signal: controller.signal })
|
const second = ctx.userInteraction.ask({ questions: [{ id: 'second', question: 'Second?' }], signal: controller.signal })
|
||||||
const secondRejected = expect(second).rejects.toMatchObject({ code: 'ASK_ABORTED' })
|
|
||||||
await new Promise(r => setImmediate(r))
|
await new Promise(r => setImmediate(r))
|
||||||
|
|
||||||
controller.abort()
|
controller.abort()
|
||||||
|
|
||||||
|
await expect(Promise.race([
|
||||||
|
second.then(
|
||||||
|
() => 'resolved',
|
||||||
|
(error: unknown) => (error as { code?: string }).code,
|
||||||
|
),
|
||||||
|
new Promise<string>((resolve) => { setImmediate(() => { resolve('pending') }) }),
|
||||||
|
])).resolves.toBe('ASK_ABORTED')
|
||||||
|
expect(out.text()).not.toContain('\nSecond?\n')
|
||||||
input.feed('first answer')
|
input.feed('first answer')
|
||||||
|
await expect(first).resolves.toEqual({ answers: [{ id: 'first', selected: [], custom: 'first answer' }] })
|
||||||
|
})
|
||||||
|
|
||||||
|
it('removes an aborted queued question without promoting later queued work early', async () => {
|
||||||
|
const { ctx, input, out } = await setup()
|
||||||
|
const controller = new AbortController()
|
||||||
|
const first = ctx.userInteraction.ask({ questions: [{ id: 'first', question: 'First?' }] })
|
||||||
|
const second = ctx.userInteraction.ask({ questions: [{ id: 'second', question: 'Second?' }], signal: controller.signal })
|
||||||
|
const third = ctx.userInteraction.ask({ questions: [{ id: 'third', question: 'Third?' }] })
|
||||||
|
await new Promise(r => setImmediate(r))
|
||||||
|
|
||||||
|
controller.abort()
|
||||||
|
|
||||||
|
await expect(second).rejects.toMatchObject({ code: 'ASK_ABORTED' })
|
||||||
|
expect(out.text()).toContain('\nFirst?\n')
|
||||||
|
expect(out.text()).not.toContain('\nSecond?\n')
|
||||||
|
expect(out.text()).not.toContain('\nThird?\n')
|
||||||
|
input.feed('first answer')
|
||||||
|
await new Promise(r => setImmediate(r))
|
||||||
|
|
||||||
|
expect(out.text()).toContain('\nThird?\n')
|
||||||
|
input.feed('third answer')
|
||||||
|
|
||||||
await expect(first).resolves.toEqual({ answers: [{ id: 'first', selected: [], custom: 'first answer' }] })
|
await expect(first).resolves.toEqual({ answers: [{ id: 'first', selected: [], custom: 'first answer' }] })
|
||||||
await secondRejected
|
await expect(third).resolves.toEqual({ answers: [{ id: 'third', selected: [], custom: 'third answer' }] })
|
||||||
expect(out.text()).not.toContain('\nSecond?\n')
|
|
||||||
})
|
})
|
||||||
|
|
||||||
it('rejects active and queued questions when the UI is disposed', async () => {
|
it('rejects active and queued questions when the UI is disposed', async () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user