diff --git a/ui-tui/src/__tests__/themeBoot.test.ts b/ui-tui/src/__tests__/themeBoot.test.ts index 92bb500f0a4f..29f406520656 100644 --- a/ui-tui/src/__tests__/themeBoot.test.ts +++ b/ui-tui/src/__tests__/themeBoot.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { invalidateBootBackground, seedBootEnvironment, type BootTheme } from '../lib/themeBoot.js' +import { type BootTheme, invalidateBootBackground, seedBootEnvironment } from '../lib/themeBoot.js' import { defaultTheme } from '../theme.js' // Review on #20379 (finding 2): the boot cache seeds the previous session's diff --git a/ui-tui/src/__tests__/useConfigSync.test.ts b/ui-tui/src/__tests__/useConfigSync.test.ts index 4338541c6235..13bd5f4924f6 100644 --- a/ui-tui/src/__tests__/useConfigSync.test.ts +++ b/ui-tui/src/__tests__/useConfigSync.test.ts @@ -4,12 +4,12 @@ import { $uiState, resetUiState } from '../app/uiStore.js' import { applyDisplay, hydrateFullConfig, + type McpRevState, normalizeBusyInputMode, normalizeIndicatorStyle, normalizeMouseTracking, normalizeStatusBar, - syncMcpReload, - type McpRevState + syncMcpReload } from '../app/useConfigSync.js' describe('applyDisplay', () => { @@ -449,11 +449,13 @@ describe('syncMcpReload (revision-aware ack)', () => { it('does not stack requests while one is in flight', async () => { let resolveFirst!: (v: unknown) => void + const gw = { request: vi.fn(() => new Promise(res => (resolveFirst = res))), on: vi.fn(), off: vi.fn() } as any + const state = freshState() const first = syncMcpReload(gw, 's1', 'rev-b', state) diff --git a/ui-tui/src/__tests__/useInputHandlers.test.ts b/ui-tui/src/__tests__/useInputHandlers.test.ts index ef9e676f73e1..a9c690cf0230 100644 --- a/ui-tui/src/__tests__/useInputHandlers.test.ts +++ b/ui-tui/src/__tests__/useInputHandlers.test.ts @@ -1,10 +1,12 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { GridTestState } from '../app/interfaces.js' import { getOverlayState, patchOverlayState, resetOverlayState } from '../app/overlayStore.js' import { applyVoiceRecordResponse, dismissSensitivePrompt, handleIdleHotkeyExit, + handleStackedModalInput, shouldAllowIdleHotkeyExit, shouldFallThroughForScroll } from '../app/useInputHandlers.js' @@ -144,3 +146,96 @@ describe('dismissSensitivePrompt', () => { await pending }) }) + +// Review on #20379 (finding 3): a dialog stacked over /grid-test was +// visually modal but did not receive input — the grid branch ran first, so +// every advertised close key (Esc/q/Enter) mutated the hidden grid instead +// of closing the visible dialog. Input routing must follow visual stacking. +describe('handleStackedModalInput — dialog over grid-test', () => { + const baseModalKey = { + ctrl: false, + downArrow: false, + escape: false, + leftArrow: false, + return: false, + rightArrow: false, + upArrow: false + } + + const grid: GridTestState = { + activeCol: 1, + activeRow: 1, + areas: false, + cols: 4, + gap: null, + nested: false, + paddingX: null, + rows: 3, + streamFocus: 0, + streamMain: 0, + streams: false, + zoomed: false + } + + beforeEach(() => { + resetOverlayState() + patchOverlayState({ gridTest: { ...grid } }) + }) + + const openDialogViaD = () => { + expect(handleStackedModalInput(getOverlayState(), baseModalKey, 'd')).toBe(true) + expect(getOverlayState().dialog).not.toBeNull() + expect(getOverlayState().gridTest).not.toBeNull() + } + + it.each([ + ['Esc', { ...baseModalKey, escape: true }, ''], + ['q', baseModalKey, 'q'], + ['Enter', { ...baseModalKey, return: true }, ''], + ['Ctrl+C', { ...baseModalKey, ctrl: true }, 'c'] + ])('%s closes only the dialog, leaving the grid untouched', (_label, key, ch) => { + openDialogViaD() + + const gridBefore = getOverlayState().gridTest + + expect(handleStackedModalInput(getOverlayState(), key, ch)).toBe(true) + expect(getOverlayState().dialog).toBeNull() + // The grid must be byte-identical: not closed, not zoomed, not reset. + expect(getOverlayState().gridTest).toBe(gridBefore) + }) + + it('after the dialog closes, the same keys route to the grid again', () => { + openDialogViaD() + handleStackedModalInput(getOverlayState(), { ...baseModalKey, escape: true }, '') + expect(getOverlayState().dialog).toBeNull() + + // Esc now closes the grid — the dialog no longer shields it. + expect(handleStackedModalInput(getOverlayState(), { ...baseModalKey, escape: true }, '')).toBe(true) + expect(getOverlayState().gridTest).toBeNull() + }) + + it('the dialog swallows grid keys entirely while open (no leak-through)', () => { + openDialogViaD() + + const gridBefore = getOverlayState().gridTest + + // 'a' toggles areas mode when the grid has focus — it must not now. + expect(handleStackedModalInput(getOverlayState(), baseModalKey, 'a')).toBe(true) + expect(getOverlayState().gridTest).toBe(gridBefore) + expect(getOverlayState().dialog).not.toBeNull() + }) + + it('stacking works from streams mode too', () => { + patchOverlayState({ gridTest: { ...grid, streams: true } }) + openDialogViaD() + + expect(handleStackedModalInput(getOverlayState(), { ...baseModalKey, return: true }, '')).toBe(true) + expect(getOverlayState().dialog).toBeNull() + expect(getOverlayState().gridTest?.streams).toBe(true) + }) + + it('reports unconsumed when neither modal is up', () => { + resetOverlayState() + expect(handleStackedModalInput(getOverlayState(), baseModalKey, 'x')).toBe(false) + }) +}) diff --git a/ui-tui/src/app/useInputHandlers.ts b/ui-tui/src/app/useInputHandlers.ts index 8bd5f2944a01..11f23fad1117 100644 --- a/ui-tui/src/app/useInputHandlers.ts +++ b/ui-tui/src/app/useInputHandlers.ts @@ -146,6 +146,145 @@ const keepGridCursorInBounds = (grid: GridTestState): GridTestState => ({ activeRow: clamp(grid.activeRow, 0, grid.rows - 1) }) +interface StackedModalKey { + ctrl: boolean + downArrow: boolean + escape: boolean + leftArrow: boolean + return: boolean + rightArrow: boolean + upArrow: boolean +} + +/** + * Input routing for the stacked demo modals (dialog over grid-test). + * Exported so tests can drive the real dispatch against the overlay store. + * + * ORDER IS THE CONTRACT: input routing follows visual stacking. /grid-test's + * `d` opens a dialog ON TOP of the grid without clearing it — the dialog + * branch must run first, or Esc/q/Enter mutate the hidden grid instead of + * closing the visible dialog, contradicting its "Esc/q/Enter close" hint + * (review on #20379, finding 3). + * + * Returns true when a modal consumed the key (callers stop routing). + */ +export function handleStackedModalInput( + overlay: Pick, + key: StackedModalKey, + ch: string +): boolean { + if (overlay.dialog) { + if (key.escape || isCtrl(key, ch, 'c') || ch === 'q' || key.return) { + patchOverlayState({ dialog: null }) + } + + return true + } + + if (!overlay.gridTest) { + return false + } + + const updateGrid = (fn: (grid: GridTestState) => GridTestState) => + patchOverlayState(prev => (prev.gridTest ? { ...prev, gridTest: keepGridCursorInBounds(fn(prev.gridTest)) } : prev)) + + const openDemoDialog = () => + patchOverlayState({ + dialog: { + body: ['Dialog overlaid on top of /grid-test.', '', 'Backdrop dims the grid behind.'].join('\n'), + hint: 'Esc/q/Enter close', + title: 'Overlay primitive', + zone: 'center' + } + }) + + const resetGrid = () => + updateGrid(grid => ({ + ...grid, + activeCol: 0, + activeRow: 0, + areas: false, + cols: 4, + gap: null, + nested: false, + paddingX: null, + rows: 3, + streamFocus: 0, + streamMain: 0, + streams: false, + zoomed: false + })) + + if (isCtrl(key, ch, 'c')) { + patchOverlayState({ gridTest: null }) + + return true + } + + // Streams mode swallows the grid-shape keys: focus cycles across the + // live panels and Enter promotes the focused one to the 2x2 slot. + if (overlay.gridTest.streams) { + if (key.escape || ch === 'q' || ch === 's') { + updateGrid(grid => ({ ...grid, streams: false })) + } else if (key.return) { + updateGrid(grid => ({ ...grid, streamMain: grid.streamFocus })) + } else if (ch === 'd') { + openDemoDialog() + } else if (ch === 'r') { + resetGrid() + } else if (key.leftArrow || key.upArrow || ch === 'h' || ch === 'k') { + updateGrid(grid => ({ + ...grid, + streamFocus: (grid.streamFocus + GRID_STREAM_COUNT - 1) % GRID_STREAM_COUNT + })) + } else if (key.rightArrow || key.downArrow || ch === 'l' || ch === 'j') { + updateGrid(grid => ({ ...grid, streamFocus: (grid.streamFocus + 1) % GRID_STREAM_COUNT })) + } + + return true + } + + if (overlay.gridTest.zoomed && (key.escape || ch === 'q')) { + updateGrid(grid => ({ ...grid, zoomed: false })) + } else if (key.escape || ch === 'q') { + patchOverlayState({ gridTest: null }) + } else if (key.return) { + updateGrid(grid => ({ ...grid, nested: true, zoomed: true })) + } else if (ch === 'n') { + updateGrid(grid => ({ ...grid, nested: !grid.nested })) + } else if (ch === 'a') { + updateGrid(grid => ({ ...grid, areas: !grid.areas, streams: false })) + } else if (ch === 's') { + updateGrid(grid => ({ ...grid, areas: false, streams: true })) + } else if (ch === 'g') { + updateGrid(grid => ({ ...grid, gap: cycleAutoNumber(grid.gap, 3) })) + } else if (ch === 'p') { + updateGrid(grid => ({ ...grid, paddingX: cycleAutoNumber(grid.paddingX, 2) })) + } else if (ch === 'd') { + openDemoDialog() + } else if (ch === 'r') { + resetGrid() + } else if (ch === '+' || ch === '=') { + updateGrid(grid => ({ ...grid, cols: clamp(grid.cols + 1, 1, GRID_TEST_MAX_SIZE) })) + } else if (ch === '-' || ch === '_') { + updateGrid(grid => ({ ...grid, cols: clamp(grid.cols - 1, 1, GRID_TEST_MAX_SIZE) })) + } else if (ch === ']') { + updateGrid(grid => ({ ...grid, rows: clamp(grid.rows + 1, 1, GRID_TEST_MAX_SIZE) })) + } else if (ch === '[') { + updateGrid(grid => ({ ...grid, rows: clamp(grid.rows - 1, 1, GRID_TEST_MAX_SIZE) })) + } else if (key.leftArrow || ch === 'h') { + updateGrid(grid => ({ ...grid, activeCol: clamp(grid.activeCol - 1, 0, grid.cols - 1) })) + } else if (key.rightArrow || ch === 'l') { + updateGrid(grid => ({ ...grid, activeCol: clamp(grid.activeCol + 1, 0, grid.cols - 1) })) + } else if (key.upArrow || ch === 'k') { + updateGrid(grid => ({ ...grid, activeRow: clamp(grid.activeRow - 1, 0, grid.rows - 1) })) + } else if (key.downArrow || ch === 'j') { + updateGrid(grid => ({ ...grid, activeRow: clamp(grid.activeRow + 1, 0, grid.rows - 1) })) + } + + return true +} + export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { const { actions, composer, gateway, terminal, voice, wheelStep } = ctx const { actions: cActions, refs: cRefs, state: cState } = composer @@ -428,156 +567,9 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult { return } - if (overlay.gridTest) { - const updateGrid = (fn: (grid: GridTestState) => GridTestState) => - patchOverlayState(prev => - prev.gridTest ? { ...prev, gridTest: keepGridCursorInBounds(fn(prev.gridTest)) } : prev - ) - - const openDemoDialog = () => - patchOverlayState({ - dialog: { - body: ['Dialog overlaid on top of /grid-test.', '', 'Backdrop dims the grid behind.'].join('\n'), - hint: 'Esc/q/Enter close', - title: 'Overlay primitive', - zone: 'center' - } - }) - - const resetGrid = () => - updateGrid(grid => ({ - ...grid, - activeCol: 0, - activeRow: 0, - areas: false, - cols: 4, - gap: null, - nested: false, - paddingX: null, - rows: 3, - streamFocus: 0, - streamMain: 0, - streams: false, - zoomed: false - })) - - if (isCtrl(key, ch, 'c')) { - return patchOverlayState({ gridTest: null }) - } - - // Streams mode swallows the grid-shape keys: focus cycles across the - // live panels and Enter promotes the focused one to the 2x2 slot. - if (overlay.gridTest.streams) { - if (key.escape || ch === 'q' || ch === 's') { - return updateGrid(grid => ({ ...grid, streams: false })) - } - - if (key.return) { - return updateGrid(grid => ({ ...grid, streamMain: grid.streamFocus })) - } - - if (ch === 'd') { - return openDemoDialog() - } - - if (ch === 'r') { - return resetGrid() - } - - if (key.leftArrow || key.upArrow || ch === 'h' || ch === 'k') { - return updateGrid(grid => ({ - ...grid, - streamFocus: (grid.streamFocus + GRID_STREAM_COUNT - 1) % GRID_STREAM_COUNT - })) - } - - if (key.rightArrow || key.downArrow || ch === 'l' || ch === 'j') { - return updateGrid(grid => ({ ...grid, streamFocus: (grid.streamFocus + 1) % GRID_STREAM_COUNT })) - } - - return - } - - if (overlay.gridTest.zoomed && (key.escape || ch === 'q')) { - return updateGrid(grid => ({ ...grid, zoomed: false })) - } - - if (key.escape || ch === 'q') { - return patchOverlayState({ gridTest: null }) - } - - if (key.return) { - return updateGrid(grid => ({ ...grid, nested: true, zoomed: true })) - } - - if (ch === 'n') { - return updateGrid(grid => ({ ...grid, nested: !grid.nested })) - } - - if (ch === 'a') { - return updateGrid(grid => ({ ...grid, areas: !grid.areas, streams: false })) - } - - if (ch === 's') { - return updateGrid(grid => ({ ...grid, areas: false, streams: true })) - } - - if (ch === 'g') { - return updateGrid(grid => ({ ...grid, gap: cycleAutoNumber(grid.gap, 3) })) - } - - if (ch === 'p') { - return updateGrid(grid => ({ ...grid, paddingX: cycleAutoNumber(grid.paddingX, 2) })) - } - - if (ch === 'd') { - return openDemoDialog() - } - - if (ch === 'r') { - return resetGrid() - } - - if (ch === '+' || ch === '=') { - return updateGrid(grid => ({ ...grid, cols: clamp(grid.cols + 1, 1, GRID_TEST_MAX_SIZE) })) - } - - if (ch === '-' || ch === '_') { - return updateGrid(grid => ({ ...grid, cols: clamp(grid.cols - 1, 1, GRID_TEST_MAX_SIZE) })) - } - - if (ch === ']') { - return updateGrid(grid => ({ ...grid, rows: clamp(grid.rows + 1, 1, GRID_TEST_MAX_SIZE) })) - } - - if (ch === '[') { - return updateGrid(grid => ({ ...grid, rows: clamp(grid.rows - 1, 1, GRID_TEST_MAX_SIZE) })) - } - - if (key.leftArrow || ch === 'h') { - return updateGrid(grid => ({ ...grid, activeCol: clamp(grid.activeCol - 1, 0, grid.cols - 1) })) - } - - if (key.rightArrow || ch === 'l') { - return updateGrid(grid => ({ ...grid, activeCol: clamp(grid.activeCol + 1, 0, grid.cols - 1) })) - } - - if (key.upArrow || ch === 'k') { - return updateGrid(grid => ({ ...grid, activeRow: clamp(grid.activeRow - 1, 0, grid.rows - 1) })) - } - - if (key.downArrow || ch === 'j') { - return updateGrid(grid => ({ ...grid, activeRow: clamp(grid.activeRow + 1, 0, grid.rows - 1) })) - } - - return - } - - if (overlay.dialog) { - if (key.escape || isCtrl(key, ch, 'c') || ch === 'q' || key.return) { - return patchOverlayState({ dialog: null }) - } - + // Stacked demo modals (dialog over grid-test): shared routing where + // the topmost visual layer consumes input first. + if (handleStackedModalInput(overlay, key, ch)) { return }