diff --git a/src/agent/__tests__/wizard-ask-bridge.test.ts b/src/agent/__tests__/wizard-ask-bridge.test.ts index d8e923d16..ce736e55a 100644 --- a/src/agent/__tests__/wizard-ask-bridge.test.ts +++ b/src/agent/__tests__/wizard-ask-bridge.test.ts @@ -433,6 +433,79 @@ describe('createWizardAskBridge', () => { } }); + it('re-arms on each answer, so one clock is not shared by every question', async () => { + vi.useFakeTimers(); + try { + // The overlay walks a multi-question request one question at a time + // and resolves once, at the end — so without the re-arm a user still + // filling in a connection form is cut off mid-form and loses every + // field they already typed. + let noteAnswer: (() => void) | undefined; + let settled = false; + const bridge = createWizardAskBridge({ + getSource: () => 'postgres', + showQuestion: (_question, { onAnswer }) => { + noteAnswer = onAnswer; + return new Promise(() => undefined); + }, + timeoutMs: 1000, + }); + + const promise = bridge.request({ + questions: [ + { id: 'host', prompt: 'Host?', kind: 'text' }, + { id: 'port', prompt: 'Port?', kind: 'text' }, + ], + }); + void promise.then(() => { + settled = true; + }); + + await vi.advanceTimersByTimeAsync(800); + noteAnswer?.(); + // Past the request's age limit, but only 800ms of silence. + await vi.advanceTimersByTimeAsync(800); + expect(settled).toBe(false); + + await vi.advanceTimersByTimeAsync(200); + await expect(promise).resolves.toMatchObject({ timedOut: true }); + } finally { + vi.useRealTimers(); + } + }); + + it('records how many questions the user answered before it expired', async () => { + vi.useFakeTimers(); + try { + const bridge = createWizardAskBridge({ + getSource: () => 'postgres', + showQuestion: (_question, { onAnswer }) => { + onAnswer(); + return new Promise(() => undefined); + }, + timeoutMs: 1000, + }); + const promise = bridge.request({ + questions: [ + { id: 'host', prompt: 'Host?', kind: 'text' }, + { id: 'port', prompt: 'Port?', kind: 'text' }, + ], + }); + await vi.advanceTimersByTimeAsync(1000); + await promise; + + const cancelledCall = wizardCaptureMock.mock.calls.find( + ([name]) => name === 'wizard_ask cancelled', + ); + expect(cancelledCall?.[1]).toMatchObject({ + questions_answered: 1, + question_count: 2, + }); + } finally { + vi.useRealTimers(); + } + }); + it('aborts only the question whose timeout fired', async () => { vi.useFakeTimers(); try { diff --git a/src/agent/progress.ts b/src/agent/progress.ts index 26ddfcb90..a2dc168be 100644 --- a/src/agent/progress.ts +++ b/src/agent/progress.ts @@ -209,10 +209,15 @@ export interface AgentInteraction { /** * Open a question and resolve with the answers. The bridge that calls this * owns the timeout, the `__cancelled__` sentinel and the analytics. + * + * `onAnswer` reports that the user answered one question of a request that + * has more to come, so the bridge can re-arm its timeout against the user's + * silence rather than against the age of the whole request. A host that + * answers a request in one shot never calls it. */ ask?: ( question: PendingQuestion, - context: { signal: AbortSignal }, + context: { signal: AbortSignal; onAnswer?: () => void }, ) => Promise; /** Offer an optional step and resolve with whether to keep it. */ taskNotice?: ( diff --git a/src/agent/wizard-ask-bridge.ts b/src/agent/wizard-ask-bridge.ts index d9d865751..5b7f43060 100644 --- a/src/agent/wizard-ask-bridge.ts +++ b/src/agent/wizard-ask-bridge.ts @@ -74,12 +74,13 @@ export interface WizardAskBridgeOptions { */ showQuestion: ( question: PendingQuestion, - context: { signal: AbortSignal }, + context: { signal: AbortSignal; onAnswer: () => void }, ) => Promise; /** * Per-question timeout in milliseconds. When the user takes longer than - * this to answer, every unanswered field resolves with the - * {@link CANCELLED_SENTINEL} value. Defaults to {@link DEFAULT_ASK_TIMEOUT_MS}. + * this to answer one question, every unanswered field of the request + * resolves with the {@link CANCELLED_SENTINEL} value. Defaults to + * {@link DEFAULT_ASK_TIMEOUT_MS}. */ timeoutMs?: number; /** @@ -137,7 +138,16 @@ export function createWizardAskBridge( const controller = new AbortController(); let timer: ReturnType | undefined; let timedOut = false; + let settled = false; + let answeredCount = 0; let cancelForAbort: (() => void) | undefined; + // Re-armed by `onAnswer`, so the clock measures the user's silence and + // not the age of the request. One clock over the whole request is the + // same allowance for a one-line question and for a nine-field database + // form the overlay walks one question at a time — and when it expired on + // a user who was still filling that form in, every field they had + // already typed was discarded and the agent was told nobody was there. + let armTimeout = (): void => undefined; // Race the user against the timeout and the run. Whichever fires first // wins. When the timeout or the run wins we also abort this question's @@ -145,12 +155,17 @@ export function createWizardAskBridge( // would leave the host's pending-question state set, and the next // wizard_ask would be rejected as a duplicate request. const timeoutPromise = new Promise((resolve) => { - timer = setTimeout(() => { - timedOut = true; - // Settle first: a host that rejects once dismissed must not win. - resolve(buildCancelledAnswers(questions)); - controller.abort(); - }, timeoutMs); + armTimeout = () => { + if (settled) return; + if (timer) clearTimeout(timer); + timer = setTimeout(() => { + timedOut = true; + // Settle first: a host that rejects once dismissed must not win. + resolve(buildCancelledAnswers(questions)); + controller.abort(); + }, timeoutMs); + }; + armTimeout(); }); const aborted = new Promise((resolve) => { cancelForAbort = () => { @@ -162,7 +177,13 @@ export function createWizardAskBridge( try { const answers = await Promise.race([ - opts.showQuestion(pending, { signal: controller.signal }), + opts.showQuestion(pending, { + signal: controller.signal, + onAnswer: () => { + answeredCount += 1; + armTimeout(); + }, + }), timeoutPromise, aborted, ]); @@ -173,6 +194,11 @@ export function createWizardAskBridge( source: pending.source, subject, question_count: questions.length, + // How far the user got before the ask ended. A count, never a + // value: it separates a request nobody touched from one abandoned + // part-filled, which is the difference between an absent user and + // a form that asks for more than they were willing to hand over. + questions_answered: answeredCount, duration_ms: durationMs, timed_out: timedOut, }); @@ -187,6 +213,7 @@ export function createWizardAskBridge( return { answers, timedOut }; } finally { + settled = true; if (timer) clearTimeout(timer); if (cancelForAbort) opts.signal?.removeEventListener('abort', cancelForAbort); diff --git a/src/tui/__tests__/store-invariants.test.ts b/src/tui/__tests__/store-invariants.test.ts index 55d364c64..6ca74e00e 100644 --- a/src/tui/__tests__/store-invariants.test.ts +++ b/src/tui/__tests__/store-invariants.test.ts @@ -498,6 +498,8 @@ const MUTATIONS: MutationCase[] = [ /** Read-only or notification-plumbing methods, excluded by the task brief. */ const NON_MUTATING = [ 'subscribe', + // Forwards the ask bridge's timeout heartbeat; holds no session state. + 'noteAskProgress', 'getSnapshot', 'getVersion', 'runInitHooks', diff --git a/src/tui/__tests__/store.test.ts b/src/tui/__tests__/store.test.ts index f3f2b3443..e8d881160 100644 --- a/src/tui/__tests__/store.test.ts +++ b/src/tui/__tests__/store.test.ts @@ -784,6 +784,21 @@ describe('WizardStore', () => { expect(store.currentScreen).not.toBe(Overlay.WizardAsk); }); + it('noteAskProgress reports an answered question to the ask bridge', () => { + const store = createStore(); + const onAnswer = vi.fn(); + const promise = store.requestQuestion(pending, onAnswer); + + store.noteAskProgress(); + expect(onAnswer).toHaveBeenCalledTimes(1); + + // The request is over: a later call must not re-arm its timeout. + store.resolvePendingQuestion({ goal: 'Find export', audience: 'new' }); + store.noteAskProgress(); + expect(onAnswer).toHaveBeenCalledTimes(1); + return promise; + }); + it('throws when requestQuestion is called while another is pending', () => { const store = createStore(); void store.requestQuestion(pending); diff --git a/src/tui/screens/WizardAskScreen.tsx b/src/tui/screens/WizardAskScreen.tsx index bb20da8e4..5bc8c35a4 100644 --- a/src/tui/screens/WizardAskScreen.tsx +++ b/src/tui/screens/WizardAskScreen.tsx @@ -228,6 +228,10 @@ export const WizardAskScreen = ({ store }: WizardAskScreenProps) => { if (index + 1 < total) { setAnswers(next); setIndex(index + 1); + // The request's timeout is armed per question, and only this screen + // knows a question was answered — the request itself resolves once, at + // the end of the walk. + store.noteAskProgress(); return; } store.resolvePendingQuestion(next); diff --git a/src/tui/store.ts b/src/tui/store.ts index 65d3c173e..f1ff531cb 100644 --- a/src/tui/store.ts +++ b/src/tui/store.ts @@ -161,6 +161,7 @@ export class WizardStore { private _resolveManualAuthCode: ((code: string) => void) | null = null; /** Resolves the in-flight wizard_ask request. */ + private _noteAskProgress: (() => void) | null = null; private _resolvePendingQuestion: ((answers: AskAnswers) => void) | null = null; @@ -629,12 +630,16 @@ export class WizardStore { * Only one request is in flight at a time — calling this while a request * is already pending throws. */ - requestQuestion(question: PendingQuestion): Promise { + requestQuestion( + question: PendingQuestion, + onAnswer?: () => void, + ): Promise { if (this._resolvePendingQuestion) { throw new Error( 'requestQuestion called while another wizard_ask request is pending', ); } + this._noteAskProgress = onAnswer ?? null; this.$session.setKey('pendingQuestion', question); this.pushOverlay(Overlay.WizardAsk); analytics.wizardCapture('wizard_ask shown', { @@ -647,6 +652,14 @@ export class WizardStore { }); } + /** + * Report that the user answered one question of the in-flight request and + * another is coming — the ask bridge's timeout heartbeat. + */ + noteAskProgress(): void { + this._noteAskProgress?.(); + } + /** * Resolve the in-flight wizard_ask request with the user's answers and * dismiss the overlay. Answers flow back to the agent as the tool result. @@ -654,6 +667,7 @@ export class WizardStore { resolvePendingQuestion(answers: AskAnswers): void { const resolve = this._resolvePendingQuestion; this._resolvePendingQuestion = null; + this._noteAskProgress = null; this.$session.setKey('pendingQuestion', null); this.popOverlay(); resolve?.(answers); diff --git a/src/ui/__tests__/agent-progress.test.ts b/src/ui/__tests__/agent-progress.test.ts index 0c6ffa4ae..41e4cc596 100644 --- a/src/ui/__tests__/agent-progress.test.ts +++ b/src/ui/__tests__/agent-progress.test.ts @@ -134,8 +134,9 @@ it('forwards answers and notices, leaving the host alone once they settle', asyn const interaction = uiInteraction(ui); const asked = new AbortController(); const noticed = new AbortController(); + const onAnswer = vi.fn(); await expect( - interaction.ask?.(question, { signal: asked.signal }), + interaction.ask?.(question, { signal: asked.signal, onAnswer }), ).resolves.toEqual({ q: 'yes' }); await expect( interaction.taskNotice?.(notice, { signal: noticed.signal }), @@ -143,7 +144,8 @@ it('forwards answers and notices, leaving the host alone once they settle', asyn // A late abort must not dismiss whatever the host shows next. asked.abort(); noticed.abort(); - expect(ask).toHaveBeenCalledWith(question); + // The host gets the bridge's timeout heartbeat alongside the question. + expect(ask).toHaveBeenCalledWith(question, onAnswer); expect(show).toHaveBeenCalledWith(notice); expect(cancelAsk).not.toHaveBeenCalled(); expect(cancelNotice).not.toHaveBeenCalled(); diff --git a/src/ui/agent-progress.ts b/src/ui/agent-progress.ts index 20305c5a5..7c0a6af9a 100644 --- a/src/ui/agent-progress.ts +++ b/src/ui/agent-progress.ts @@ -70,8 +70,8 @@ export function createUiReducer(ui: WizardUI): (event: AgentProgress) => void { /** The agent's questions, answered wherever `getUI()` answers them today. */ export function uiInteraction(ui: WizardUI): AgentInteraction { return { - ask: (question, { signal }) => - dismissOnAbort(ui.requestQuestion(question), signal, () => + ask: (question, { signal, onAnswer }) => + dismissOnAbort(ui.requestQuestion(question, onAnswer), signal, () => ui.cancelPendingQuestion(), ), taskNotice: (notice, { signal }) => diff --git a/src/ui/tui/ink-ui.ts b/src/ui/tui/ink-ui.ts index ab5c4ad4a..fb6ada2f2 100644 --- a/src/ui/tui/ink-ui.ts +++ b/src/ui/tui/ink-ui.ts @@ -182,8 +182,11 @@ export class InkUI implements WizardUI { this.store.showSessionTimeout(); } - requestQuestion(question: PendingQuestion): Promise { - return this.store.requestQuestion(question); + requestQuestion( + question: PendingQuestion, + onAnswer?: () => void, + ): Promise { + return this.store.requestQuestion(question, onAnswer); } cancelPendingQuestion(): void { diff --git a/src/ui/wizard-ui.ts b/src/ui/wizard-ui.ts index ec5a2f5fe..4acd9eb40 100644 --- a/src/ui/wizard-ui.ts +++ b/src/ui/wizard-ui.ts @@ -152,8 +152,14 @@ export interface WizardUI { * Open the wizard_ask overlay and resolve with the user's answers. * Implementations that can't ask (CI/logging) reject so the bridge can * surface a clear "not available" error to the agent. + * + * `onAnswer` is the bridge's timeout heartbeat: an overlay that walks a + * multi-question request calls it as each answer goes in. */ - requestQuestion(question: PendingQuestion): Promise; + requestQuestion( + question: PendingQuestion, + onAnswer?: () => void, + ): Promise; /** * Dismiss the in-flight wizard_ask overlay, resolving its request with