From 825629424d765da6a86b01ad9352ba2f98f51b61 Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 4 Jun 2026 11:00:31 +1000 Subject: [PATCH] fix(tui): persist timed-out/cancelled clarify prompts in transcript When a clarify prompt times out (backend _block returns an empty answer after the configured timeout) or is dismissed with Esc/Ctrl+C, the live ClarifyPrompt overlay was torn down by turnController.idle() -> resetFlowOverlays() with no persistent transcript record. The question and options vanished from the screen while the agent's follow-up still referred to "the options above". The answered path already persists the question + answer; only the unanswered exits left no trace. This asymmetry is the bug. Fix (TUI layer only, no Python/protocol change): - formatAbandonedClarify() in lib/text.ts renders the question + the same 1-based numbered option list shown by ClarifyPrompt, plus a reason ('timed out' / 'cancelled'). - Timeout: createGatewayEventHandler flushes a still-live clarify into the transcript as a plain system line when the clarify tool's own tool.complete fires. A live capture of the event stream confirmed this is the only point where the overlay is still set after a timeout: the sequence is clarify.request -> (timeout) -> tool.complete -> message.complete, with NO intervening message.start/tool.start. On a real answer, answerClarify() clears the overlay before tool.complete arrives, so the hook no-ops there (no double-write); a per-requestId guard set is belt-and-braces. - Explicit cancel: answerClarify('') persists the prompt as a system line instead of a transient 'prompt cancelled' flash. System lines always render (unlike trail lines, which /details can hide), so the record reliably survives on screen as standard output. Verified live in the TUI: an Esc-cancelled clarify now leaves the question + options + '(cancelled - no selection)' in the transcript after the turn ends. Tests: formatAbandonedClarify unit cases + gateway-handler behavioral cases (persist on clarify tool.complete, no flush on a non-clarify tool.complete, no double-persist on repeat tool.complete, no-op when the overlay was already cleared by an answer). --- .../createGatewayEventHandler.test.ts | 64 +++++++++++++++++++ ui-tui/src/app/createGatewayEventHandler.ts | 39 ++++++++++- ui-tui/src/app/useMainApp.ts | 12 +++- ui-tui/src/lib/text.test.ts | 33 +++++++++- ui-tui/src/lib/text.ts | 16 +++++ 5 files changed, 159 insertions(+), 5 deletions(-) diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index afebc4d10..1b433220b 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -1113,4 +1113,68 @@ describe('createGatewayEventHandler', () => { vi.useRealTimers() } }) + + it('persists an abandoned (timed-out) clarify into the transcript when the clarify tool completes', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + + // Backend clarify timed out: the overlay is still live (Python returned an + // empty answer), and the clarify tool's own tool.complete then fires. + patchOverlayState({ + clarify: { choices: ['Scope A', 'Scope B'], question: 'How do you want to scope?', requestId: 'req-1' } + }) + + onEvent({ payload: { duration_s: 300, name: 'clarify', tool_id: 'clar-1' }, type: 'tool.complete' } as any) + + const record = appended.find(msg => msg.role === 'system' && msg.text.startsWith('ask How do you want to scope?')) + expect(record).toBeDefined() + expect(record?.text).toContain('1. Scope A') + expect(record?.text).toContain('2. Scope B') + expect(record?.text).toContain('timed out — no selection') + // The live overlay is cleared so it doesn't double-render with the record. + expect(getOverlayState().clarify).toBeNull() + }) + + it('only persists an abandoned clarify once even if tool.complete fires twice', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + + patchOverlayState({ + clarify: { choices: ['A'], question: 'Pick?', requestId: 'req-3' } + }) + + onEvent({ payload: { name: 'clarify', tool_id: 'clar-1' }, type: 'tool.complete' } as any) + // A duplicate clarify tool.complete must not re-persist the same prompt. + onEvent({ payload: { name: 'clarify', tool_id: 'clar-1' }, type: 'tool.complete' } as any) + + const records = appended.filter(msg => msg.role === 'system' && msg.text.startsWith('ask Pick?')) + expect(records).toHaveLength(1) + }) + + it('does not flush the clarify overlay when a non-clarify tool completes', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + + // A clarify is live, but it's a *different* tool that just completed — the + // clarify itself is still pending, so we must not persist or clear it. + patchOverlayState({ + clarify: { choices: ['A', 'B'], question: 'Pick?', requestId: 'req-4' } + }) + + onEvent({ payload: { name: 'search', tool_id: 'tool-1' }, type: 'tool.complete' } as any) + + expect(appended.some(msg => msg.role === 'system' && msg.text.startsWith('ask '))).toBe(false) + expect(getOverlayState().clarify).not.toBeNull() + }) + + it('does not persist when an answered clarify already cleared the overlay before tool.complete', () => { + const appended: Msg[] = [] + const onEvent = createGatewayEventHandler(buildCtx(appended)) + + // Answered path (answerClarify) clears the overlay before the agent's + // tool.complete arrives, so there's nothing live to persist. + onEvent({ payload: { duration_s: 4.2, name: 'clarify', tool_id: 'clar-1' }, type: 'tool.complete' } as any) + + expect(appended.some(msg => msg.role === 'system' && msg.text.startsWith('ask '))).toBe(false) + }) }) diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 987518a44..6e40e1c75 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -11,7 +11,7 @@ import type { } from '../gatewayTypes.js' import { rpcErrorMessage } from '../lib/rpc.js' import { topLevelSubagents } from '../lib/subagentTree.js' -import { formatToolCall, stripAnsi } from '../lib/text.js' +import { formatAbandonedClarify, formatToolCall, stripAnsi } from '../lib/text.js' import { fromSkin } from '../theme.js' import type { Msg, SubagentProgress, SubagentStatus } from '../types.js' @@ -87,6 +87,35 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: let thinkingStatusTimer: null | ReturnType = null let startupPromptSubmitted = false + // Request IDs of clarify prompts we've already flushed to the transcript as + // an abandoned-prompt record, so the tool.complete and message.complete + // paths can't both persist the same prompt twice. + const persistedAbandonedClarify = new Set() + + // When a clarify prompt is dismissed without an answer (the backend _block + // timed out and returned an empty string), the live ClarifyPrompt overlay is + // left set until the next turn's idle() silently nulls it — so the question + // and options vanish from the screen while the agent's follow-up still refers + // to them. The reliable signal is the clarify tool's own tool.complete (and, + // as a backstop, message.complete): at those points the overlay is provably + // still set on a timeout, but already cleared by answerClarify() on a real + // answer (so this no-ops there). Flush the question + options into the + // transcript as a persistent system line, then clear the overlay. + const flushAbandonedClarify = () => { + const { clarify } = getOverlayState() + + if (!clarify || persistedAbandonedClarify.has(clarify.requestId)) { + return + } + + persistedAbandonedClarify.add(clarify.requestId) + appendMessage({ + role: 'system', + text: formatAbandonedClarify(clarify.question, clarify.choices, 'timed out') + }) + patchOverlayState({ clarify: null }) + } + // Inject the disk-save callback into turnController so recordMessageComplete // can fire-and-forget a persist without having to plumb a gateway ref around. turnController.persistSpawnTree = async (subagents, sessionId) => { @@ -624,6 +653,14 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: return case 'tool.complete': { + // The clarify tool finishing with its overlay still live means it was + // abandoned (backend _block timed out, empty answer). A real answer + // clears the overlay in answerClarify() before this fires, so this + // no-ops there. Persist the question + options so they don't vanish. + if (ev.payload.name === 'clarify') { + flushAbandonedClarify() + } + const inlineDiffText = ev.payload.inline_diff && getUiState().inlineDiffs ? stripAnsi(String(ev.payload.inline_diff)).trim() : '' diff --git a/ui-tui/src/app/useMainApp.ts b/ui-tui/src/app/useMainApp.ts index 6c48c56f9..509e87758 100644 --- a/ui-tui/src/app/useMainApp.ts +++ b/ui-tui/src/app/useMainApp.ts @@ -25,7 +25,7 @@ import { appendTranscriptMessage } from '../lib/messages.js' import { DEFAULT_VOICE_RECORD_KEY, isMac, type ParsedVoiceRecordKey } from '../lib/platform.js' import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js' import { terminalParityHints } from '../lib/terminalParity.js' -import { buildToolTrailLine, sameToolTrailGroup, toolTrailLabel } from '../lib/text.js' +import { buildToolTrailLine, formatAbandonedClarify, sameToolTrailGroup, toolTrailLabel } from '../lib/text.js' import { estimatedMsgHeight, messageHeightKey } from '../lib/virtualHeights.js' import type { Msg, PanelSection, SlashCatalog } from '../types.js' @@ -608,13 +608,19 @@ export function useMainApp(gw: GatewayClient) { appendMessage({ role: 'user', text: answer }) patchUiState({ status: 'running…' }) } else { - sys('prompt cancelled') + // Esc / Ctrl+C cancel: persist the question + options as a system + // line (not a transient "prompt cancelled" flash) so the prompt + // survives on screen as standard output, matching the timeout path. + appendMessage({ + role: 'system', + text: formatAbandonedClarify(clarify.question, clarify.choices, 'cancelled') + }) } patchOverlayState({ clarify: null }) }) }, - [appendMessage, overlay.clarify, rpc, sys] + [appendMessage, overlay.clarify, rpc] ) const paste = useCallback( diff --git a/ui-tui/src/lib/text.test.ts b/ui-tui/src/lib/text.test.ts index 1a3800ec7..ebea2f5b5 100644 --- a/ui-tui/src/lib/text.test.ts +++ b/ui-tui/src/lib/text.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { stripTrailingPasteNewlines } from './text.js' +import { formatAbandonedClarify, stripTrailingPasteNewlines } from './text.js' describe('stripTrailingPasteNewlines', () => { it('removes trailing newline runs from pasted text', () => { @@ -16,3 +16,34 @@ describe('stripTrailingPasteNewlines', () => { expect(stripTrailingPasteNewlines('\n\n')).toBe('\n\n') }) }) + +describe('formatAbandonedClarify', () => { + it('renders the question, numbered options, and reason', () => { + const out = formatAbandonedClarify('How do you want to scope?', ['Option A', 'Option B', 'Option C'], 'timed out') + + expect(out).toBe( + ['ask How do you want to scope?', ' 1. Option A', ' 2. Option B', ' 3. Option C', ' (timed out — no selection)'].join( + '\n' + ) + ) + }) + + it('handles a prompt with no choices (free-text clarify)', () => { + const out = formatAbandonedClarify('What is the target branch?', null, 'cancelled') + + expect(out).toBe(['ask What is the target branch?', ' (cancelled — no selection)'].join('\n')) + }) + + it('trims surrounding whitespace on the question', () => { + const out = formatAbandonedClarify(' trailing space ', [], 'timed out') + + expect(out.split('\n')[0]).toBe('ask trailing space') + }) + + it('numbers options 1-based to match the live ClarifyPrompt', () => { + const out = formatAbandonedClarify('q', ['first'], 'timed out') + + expect(out).toContain(' 1. first') + expect(out).not.toContain(' 0.') + }) +}) diff --git a/ui-tui/src/lib/text.ts b/ui-tui/src/lib/text.ts index feb3547a3..b1e86e367 100644 --- a/ui-tui/src/lib/text.ts +++ b/ui-tui/src/lib/text.ts @@ -338,6 +338,22 @@ export const estimateRows = (text: string, w: number, compact = false) => { return Math.max(1, rows) } +/** + * Render an unanswered clarify prompt (timed out, or cancelled with Esc/Ctrl+C) + * as a persistent transcript block. The live `ClarifyPrompt` overlay is torn + * down the moment the turn settles, so without this the question + options + * vanish from the screen while the agent's follow-up still refers to "the + * options above". Mirrors the option formatting in ClarifyPrompt (the same + * 1-based numbered list) so the persisted record reads identically to what was + * on screen. `reason` states why the prompt ended ("timed out", "cancelled"). + */ +export const formatAbandonedClarify = (question: string, choices: string[] | null, reason: string) => { + const head = `ask ${question.trim()}` + const opts = (choices ?? []).map((c, i) => ` ${i + 1}. ${c}`) + + return [head, ...opts, ` (${reason} — no selection)`].join('\n') +} + export const flat = (r: Record) => Object.values(r).flat() const COMPACT_NUMBER = new Intl.NumberFormat('en-US', { maximumFractionDigits: 1, notation: 'compact' })