diff --git a/ui-tui/src/__tests__/approvalAction.test.ts b/ui-tui/src/__tests__/approvalAction.test.ts new file mode 100644 index 00000000000..851b5093448 --- /dev/null +++ b/ui-tui/src/__tests__/approvalAction.test.ts @@ -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' }) + }) +}) diff --git a/ui-tui/src/__tests__/useInputHandlers.test.ts b/ui-tui/src/__tests__/useInputHandlers.test.ts index 066292abfa5..0d3fd69c1ed 100644 --- a/ui-tui/src/__tests__/useInputHandlers.test.ts +++ b/ui-tui/src/__tests__/useInputHandlers.test.ts @@ -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', () => { diff --git a/ui-tui/src/app/useInputHandlers.ts b/ui-tui/src/app/useInputHandlers.ts index ce25af70edd..59de48a310d 100644 --- a/ui-tui/src/app/useInputHandlers.ts +++ b/ui-tui/src/app/useInputHandlers.ts @@ -23,6 +23,42 @@ import { getUiState } from './uiStore.js' const isCtrl = (key: { ctrl: boolean }, ch: string, target: string) => key.ctrl && ch.toLowerCase() === target +/** + * Approval / clarify / confirm overlays mount their own `useInput` handlers + * for the in-prompt keys (arrows, numbers, Enter, sometimes Esc). The global + * input handler used to early-return for any other key while one of those + * overlays was up, which silently disabled transcript scrolling — the user + * couldn't read context above the prompt that the prompt itself was asking + * about. Returns true when the key is a transcript-scroll input that should + * fall through to the global scroll handlers even while a prompt is active. + * + * Modifier-held wheel (precision mode) is included — a user who wants to + * scroll a single line at a time during a prompt expects it to work. + */ +export function shouldFallThroughForScroll(key: { + downArrow: boolean + pageDown: boolean + pageUp: boolean + shift: boolean + upArrow: boolean + wheelDown: boolean + wheelUp: boolean +}): boolean { + if (key.wheelUp || key.wheelDown) { + return true + } + + if (key.pageUp || key.pageDown) { + return true + } + + if (key.shift && (key.upArrow || key.downArrow)) { + return true + } + + return false +} + export function applyVoiceRecordResponse( response: null | VoiceRecordResponse, starting: boolean, @@ -224,7 +260,18 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { // handlers must receive keystrokes (arrow keys, numbers, Enter). Only // intercept Ctrl+C here so the user can deny/dismiss — all other keys // fall through to the component-level handlers. - if (overlay.approval || overlay.clarify || overlay.confirm) { + // + // Scroll inputs (wheel / PageUp / PageDown / Shift+↑↓) are special: + // they must reach the transcript scroll handlers below even with a + // prompt up. Long-thread context the prompt is asking about often + // lives above the visible viewport, and being unable to read it while + // answering felt like the prompt had locked the entire UI. Explicitly + // skip the prompt-overlay early-return for scroll keys so they fall + // through to the wheel / PageUp / Shift+arrow handlers below. + const promptOverlay = overlay.approval || overlay.clarify || overlay.confirm + const fallThroughForScroll = promptOverlay && shouldFallThroughForScroll(key) + + if (promptOverlay && !fallThroughForScroll) { if (isCtrl(key, ch, 'c')) { cancelOverlayFromCtrlC() } @@ -298,7 +345,13 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { patchOverlayState({ picker: false }) } - return + // When a prompt overlay is up and the user pressed a scroll key, fall + // through to the global scroll handlers below instead of returning. + // Otherwise nothing above this comment matched, and there's nothing + // useful to do for an arbitrary key while blocked. + if (!fallThroughForScroll) { + return + } } if (cState.completions.length && cState.input && cState.historyIdx === null && (key.upArrow || key.downArrow)) { diff --git a/ui-tui/src/components/prompts.tsx b/ui-tui/src/components/prompts.tsx index e9d42485d9b..3dfd31be869 100644 --- a/ui-tui/src/components/prompts.tsx +++ b/ui-tui/src/components/prompts.tsx @@ -11,28 +11,65 @@ const OPTS = ['once', 'session', 'always', 'deny'] as const const LABELS = { always: 'Always allow', deny: 'Deny', once: 'Allow once', session: 'Allow this session' } as const const CMD_PREVIEW_LINES = 10 +type ApprovalKey = { + downArrow?: boolean + escape?: boolean + return?: boolean + upArrow?: boolean +} + +type ApprovalAction = + | { kind: 'choose'; choice: (typeof OPTS)[number] } + | { kind: 'move'; delta: -1 | 1 } + | { kind: 'noop' } + +/** + * Pure key-dispatch for the approval prompt — exported so the regression + * matrix (Esc, Ctrl+C-equivalent, number keys, Enter, ↑↓) is testable + * without mounting React + Ink + a fake stdin. The component just maps the + * action onto its own state setters. + * + * Esc and number keys both terminate the prompt; Esc maps to deny (parity + * with the global Ctrl+C handler that already calls cancelOverlayFromCtrlC + * for approvals). Numbers 1..OPTS.length pick the labelled choice. Enter + * confirms the current selection. ↑/↓ moves the selection within bounds. + */ +export function approvalAction(ch: string, key: ApprovalKey, sel: number): ApprovalAction { + if (key.escape) { + return { kind: 'choose', choice: 'deny' } + } + + const n = parseInt(ch, 10) + + if (n >= 1 && n <= OPTS.length) { + return { kind: 'choose', choice: OPTS[n - 1]! } + } + + if (key.return) { + return { kind: 'choose', choice: OPTS[sel]! } + } + + if (key.upArrow && sel > 0) { + return { kind: 'move', delta: -1 } + } + + if (key.downArrow && sel < OPTS.length - 1) { + return { kind: 'move', delta: 1 } + } + + return { kind: 'noop' } +} + export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) { const [sel, setSel] = useState(0) useInput((ch, key) => { - if (key.upArrow && sel > 0) { - setSel(s => s - 1) - } + const action = approvalAction(ch, key, sel) - if (key.downArrow && sel < OPTS.length - 1) { - setSel(s => s + 1) - } - - const n = parseInt(ch, 10) - - if (n >= 1 && n <= OPTS.length) { - onChoice(OPTS[n - 1]!) - - return - } - - if (key.return) { - onChoice(OPTS[sel]!) + if (action.kind === 'choose') { + onChoice(action.choice) + } else if (action.kind === 'move') { + setSel(s => s + action.delta) } }) @@ -71,7 +108,7 @@ export function ApprovalPrompt({ onChoice, req, t }: ApprovalPromptProps) { ))} - ↑/↓ select · Enter confirm · 1-4 quick pick · Ctrl+C deny + ↑/↓ select · Enter confirm · 1-4 quick pick · Esc/Ctrl+C deny ) }