fix(tui): allow transcript scroll + Esc during approval/clarify/confirm prompts (#26414)

When an approval / clarify / confirm overlay was active, the global input
handler in useInputHandlers returned for every key that wasn't Ctrl+C, which
silently disabled transcript scrolling. On long threads the context the
prompt was asking about often lived above the visible viewport, and being
unable to scroll while answering felt like the prompt had locked the UI.
ApprovalPrompt also had no Esc handler at all, so the one obvious 'abort'
key did nothing during a permission prompt and the user had to memorize
Ctrl+C or hunt for the deny number.

Fixes:

- Extract shouldFallThroughForScroll(key) (pure, exported) covering wheel
  scrolls, PageUp/PageDown, and Shift+ArrowUp/Down. When a prompt overlay
  is up and the pressed key is a scroll input, skip the early return so it
  reaches the existing wheel/PageUp/Shift+arrow handlers below. Plain
  arrows still drive in-prompt selection — they don't fall through.
- ApprovalPrompt now maps Esc to onChoice('deny'), parity with the global
  Ctrl+C cancellation path that already invokes cancelOverlayFromCtrlC()
  for approvals. The bottom-of-prompt hint now advertises 'Esc/Ctrl+C deny'.
- Extract approvalAction(ch, key, sel) — pure key-dispatch helper for the
  approval prompt, exported so the regression matrix (Esc, numbers, Enter,
  arrows, edge clamping, precedence) is testable without mounting Ink.

Tests:
- useInputHandlers.test.ts: 6 cases covering shouldFallThroughForScroll
  positives (wheel/PageUp/PageDown/Shift+arrows) and negatives (plain
  arrows, bare shift, no scroll key).
- approvalAction.test.ts: 8 cases covering Esc→deny, numeric mapping,
  Enter, ↑↓ within bounds, edge clamping, Esc-beats-others precedence,
  unrelated keystrokes.
This commit is contained in:
brooklyn! 2026-05-15 21:59:28 -05:00 committed by GitHub
parent 97a32afdc4
commit 44b63fc6de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 201 additions and 21 deletions

View file

@ -0,0 +1,50 @@
import { describe, expect, it } from 'vitest'
import { approvalAction } from '../components/prompts.js'
describe('approvalAction — pure key dispatch for ApprovalPrompt', () => {
it('maps Esc to deny — parity with global Ctrl+C cancellation', () => {
expect(approvalAction('', { escape: true }, 0)).toEqual({ kind: 'choose', choice: 'deny' })
expect(approvalAction('', { escape: true }, 2)).toEqual({ kind: 'choose', choice: 'deny' })
})
it('maps number keys 1..4 to once/session/always/deny in registration order', () => {
expect(approvalAction('1', {}, 0)).toEqual({ kind: 'choose', choice: 'once' })
expect(approvalAction('2', {}, 0)).toEqual({ kind: 'choose', choice: 'session' })
expect(approvalAction('3', {}, 0)).toEqual({ kind: 'choose', choice: 'always' })
expect(approvalAction('4', {}, 0)).toEqual({ kind: 'choose', choice: 'deny' })
})
it('ignores out-of-range numbers', () => {
expect(approvalAction('0', {}, 1)).toEqual({ kind: 'noop' })
expect(approvalAction('5', {}, 1)).toEqual({ kind: 'noop' })
expect(approvalAction('9', {}, 1)).toEqual({ kind: 'noop' })
})
it('confirms the current selection on Enter', () => {
expect(approvalAction('', { return: true }, 0)).toEqual({ kind: 'choose', choice: 'once' })
expect(approvalAction('', { return: true }, 3)).toEqual({ kind: 'choose', choice: 'deny' })
})
it('moves selection up/down within bounds', () => {
expect(approvalAction('', { upArrow: true }, 2)).toEqual({ kind: 'move', delta: -1 })
expect(approvalAction('', { downArrow: true }, 1)).toEqual({ kind: 'move', delta: 1 })
})
it('clamps selection movement at the edges', () => {
expect(approvalAction('', { upArrow: true }, 0)).toEqual({ kind: 'noop' })
expect(approvalAction('', { downArrow: true }, 3)).toEqual({ kind: 'noop' })
})
it('Esc beats numeric/return — denying is always the first interpretation', () => {
// If a terminal somehow delivers Esc + a digit in the same event, deny
// wins. Documents the precedence so a future refactor doesn't flip it.
expect(approvalAction('1', { escape: true }, 0)).toEqual({ kind: 'choose', choice: 'deny' })
expect(approvalAction('', { escape: true, return: true }, 1)).toEqual({ kind: 'choose', choice: 'deny' })
})
it('returns noop for unrelated keystrokes (printable letters etc.)', () => {
expect(approvalAction('a', {}, 0)).toEqual({ kind: 'noop' })
expect(approvalAction(' ', {}, 0)).toEqual({ kind: 'noop' })
})
})

View file

@ -1,6 +1,46 @@
import { describe, expect, it, vi } from 'vitest'
import { applyVoiceRecordResponse } from '../app/useInputHandlers.js'
import { applyVoiceRecordResponse, shouldFallThroughForScroll } from '../app/useInputHandlers.js'
const baseKey = {
downArrow: false,
pageDown: false,
pageUp: false,
shift: false,
upArrow: false,
wheelDown: false,
wheelUp: false
}
describe('shouldFallThroughForScroll — keep transcript scrolling alive during prompt overlays', () => {
it('falls through for wheel scrolls', () => {
expect(shouldFallThroughForScroll({ ...baseKey, wheelUp: true })).toBe(true)
expect(shouldFallThroughForScroll({ ...baseKey, wheelDown: true })).toBe(true)
})
it('falls through for PageUp / PageDown', () => {
expect(shouldFallThroughForScroll({ ...baseKey, pageUp: true })).toBe(true)
expect(shouldFallThroughForScroll({ ...baseKey, pageDown: true })).toBe(true)
})
it('falls through for Shift+ArrowUp / Shift+ArrowDown', () => {
expect(shouldFallThroughForScroll({ ...baseKey, shift: true, upArrow: true })).toBe(true)
expect(shouldFallThroughForScroll({ ...baseKey, shift: true, downArrow: true })).toBe(true)
})
it('does NOT fall through for plain arrows — those drive in-prompt selection', () => {
expect(shouldFallThroughForScroll({ ...baseKey, upArrow: true })).toBe(false)
expect(shouldFallThroughForScroll({ ...baseKey, downArrow: true })).toBe(false)
})
it('does NOT fall through for plain Shift — without an arrow it is a no-op', () => {
expect(shouldFallThroughForScroll({ ...baseKey, shift: true })).toBe(false)
})
it('does NOT fall through for unrelated state (no scroll keys held)', () => {
expect(shouldFallThroughForScroll(baseKey)).toBe(false)
})
})
describe('applyVoiceRecordResponse', () => {
it('reverts optimistic REC state when the gateway reports voice busy', () => {