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).
This commit is contained in:
@ -1113,4 +1113,68 @@ describe('createGatewayEventHandler', () => {
|
|||||||
vi.useRealTimers()
|
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)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@ -11,7 +11,7 @@ import type {
|
|||||||
} from '../gatewayTypes.js'
|
} from '../gatewayTypes.js'
|
||||||
import { rpcErrorMessage } from '../lib/rpc.js'
|
import { rpcErrorMessage } from '../lib/rpc.js'
|
||||||
import { topLevelSubagents } from '../lib/subagentTree.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 { fromSkin } from '../theme.js'
|
||||||
import type { Msg, SubagentProgress, SubagentStatus } from '../types.js'
|
import type { Msg, SubagentProgress, SubagentStatus } from '../types.js'
|
||||||
|
|
||||||
@ -87,6 +87,35 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
|||||||
let thinkingStatusTimer: null | ReturnType<typeof setTimeout> = null
|
let thinkingStatusTimer: null | ReturnType<typeof setTimeout> = null
|
||||||
let startupPromptSubmitted = false
|
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<string>()
|
||||||
|
|
||||||
|
// 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
|
// Inject the disk-save callback into turnController so recordMessageComplete
|
||||||
// can fire-and-forget a persist without having to plumb a gateway ref around.
|
// can fire-and-forget a persist without having to plumb a gateway ref around.
|
||||||
turnController.persistSpawnTree = async (subagents, sessionId) => {
|
turnController.persistSpawnTree = async (subagents, sessionId) => {
|
||||||
@ -624,6 +653,14 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
|||||||
|
|
||||||
return
|
return
|
||||||
case 'tool.complete': {
|
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 =
|
const inlineDiffText =
|
||||||
ev.payload.inline_diff && getUiState().inlineDiffs ? stripAnsi(String(ev.payload.inline_diff)).trim() : ''
|
ev.payload.inline_diff && getUiState().inlineDiffs ? stripAnsi(String(ev.payload.inline_diff)).trim() : ''
|
||||||
|
|
||||||
|
|||||||
@ -25,7 +25,7 @@ import { appendTranscriptMessage } from '../lib/messages.js'
|
|||||||
import { DEFAULT_VOICE_RECORD_KEY, isMac, type ParsedVoiceRecordKey } from '../lib/platform.js'
|
import { DEFAULT_VOICE_RECORD_KEY, isMac, type ParsedVoiceRecordKey } from '../lib/platform.js'
|
||||||
import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js'
|
import { asRpcResult, rpcErrorMessage } from '../lib/rpc.js'
|
||||||
import { terminalParityHints } from '../lib/terminalParity.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 { estimatedMsgHeight, messageHeightKey } from '../lib/virtualHeights.js'
|
||||||
import type { Msg, PanelSection, SlashCatalog } from '../types.js'
|
import type { Msg, PanelSection, SlashCatalog } from '../types.js'
|
||||||
|
|
||||||
@ -608,13 +608,19 @@ export function useMainApp(gw: GatewayClient) {
|
|||||||
appendMessage({ role: 'user', text: answer })
|
appendMessage({ role: 'user', text: answer })
|
||||||
patchUiState({ status: 'running…' })
|
patchUiState({ status: 'running…' })
|
||||||
} else {
|
} 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 })
|
patchOverlayState({ clarify: null })
|
||||||
})
|
})
|
||||||
},
|
},
|
||||||
[appendMessage, overlay.clarify, rpc, sys]
|
[appendMessage, overlay.clarify, rpc]
|
||||||
)
|
)
|
||||||
|
|
||||||
const paste = useCallback(
|
const paste = useCallback(
|
||||||
|
|||||||
@ -1,6 +1,6 @@
|
|||||||
import { describe, expect, it } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
|
|
||||||
import { stripTrailingPasteNewlines } from './text.js'
|
import { formatAbandonedClarify, stripTrailingPasteNewlines } from './text.js'
|
||||||
|
|
||||||
describe('stripTrailingPasteNewlines', () => {
|
describe('stripTrailingPasteNewlines', () => {
|
||||||
it('removes trailing newline runs from pasted text', () => {
|
it('removes trailing newline runs from pasted text', () => {
|
||||||
@ -16,3 +16,34 @@ describe('stripTrailingPasteNewlines', () => {
|
|||||||
expect(stripTrailingPasteNewlines('\n\n')).toBe('\n\n')
|
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.')
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
@ -338,6 +338,22 @@ export const estimateRows = (text: string, w: number, compact = false) => {
|
|||||||
return Math.max(1, rows)
|
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<string, string[]>) => Object.values(r).flat()
|
export const flat = (r: Record<string, string[]>) => Object.values(r).flat()
|
||||||
|
|
||||||
const COMPACT_NUMBER = new Intl.NumberFormat('en-US', { maximumFractionDigits: 1, notation: 'compact' })
|
const COMPACT_NUMBER = new Intl.NumberFormat('en-US', { maximumFractionDigits: 1, notation: 'compact' })
|
||||||
|
|||||||
Reference in New Issue
Block a user