From 7e317045ee04f1190736b7d93db8611ed96b57e8 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 31 Jul 2026 19:51:48 -0700 Subject: [PATCH 1/4] Scroll approval prompts when their body overflows the terminal The permission modal's request body (segment list, expanded payloads, notice) and the operator modal's question and option list now page against the real terminal height instead of painting past it. The focused choice and full action list stay fixed and reachable below the body; PageUp/PageDown scroll a long body or question, and a long option list windows to keep the highlighted option in view as the operator navigates. Short prompts render exactly as before, with no scroll indicator or forced affordance. --- src/tui/app.tsx | 1 + src/tui/components/modal-stack.tsx | 6 + src/tui/components/operator-modal.tsx | 114 +++++++++++++-- src/tui/components/permission-modal.tsx | 179 +++++++++++++++++------ tests/unit/tui/operator-modal.test.tsx | 52 +++++++ tests/unit/tui/permission-modal.test.tsx | 55 ++++++- 6 files changed, 354 insertions(+), 53 deletions(-) diff --git a/src/tui/app.tsx b/src/tui/app.tsx index ebcb933b1..8c0823b22 100644 --- a/src/tui/app.tsx +++ b/src/tui/app.tsx @@ -1130,6 +1130,7 @@ export function App({ onResolvePermission={gates.resolvePermission} width={columns} + {...(rows !== undefined ? { terminalRows: rows } : {})} /> void; + /** Terminal height, so approval bodies taller than it scroll instead of + * pushing the choices off screen. */ + terminalRows?: number; width?: number; @@ -120,6 +123,7 @@ export function ModalStack({ queuedApprovals, onResolvePermission, width, + terminalRows, }: ModalStackProps): ReactNode { return ( @@ -167,6 +171,7 @@ export function ModalStack({ options={activeApproval.options} onSelect={(result) => onSelectOperator(activeApproval.id, result)} {...(width !== undefined ? { width } : {})} + {...(terminalRows !== undefined ? { terminalRows } : {})} /> )} {activeApproval?.kind === "permission" && ( @@ -175,6 +180,7 @@ export function ModalStack({ request={activeApproval.request} {...(permissionQueueDepth !== undefined ? { permissionQueueDepth } : {})} {...(queuedApprovals !== undefined ? { queuedApprovals } : {})} + {...(terminalRows !== undefined ? { terminalRows } : {})} {...(activeApproval.timeoutMs !== null ? { goalTimeoutMs: activeApproval.timeoutMs } : {})} onResolve={(outcome) => onResolvePermission(activeApproval.id, outcome)} {...(width !== undefined ? { width } : {})} diff --git a/src/tui/components/operator-modal.tsx b/src/tui/components/operator-modal.tsx index cd9fb1972..a8b25c1b0 100644 --- a/src/tui/components/operator-modal.tsx +++ b/src/tui/components/operator-modal.tsx @@ -12,8 +12,19 @@ export type OperatorModalProps = { options: string[]; onSelect: (result: OperatorResult) => void; width?: number; + /** Terminal height, so a long question or option list scrolls/pages instead + * of pushing the selection out of view. Defaults to a conservative fallback. */ + terminalRows?: number; }; +const FALLBACK_TERMINAL_ROWS = 24; +const MIN_QUESTION_ROWS = 2; +const MIN_OPTION_ROWS = 3; + +function maxRowOffset(rowCount: number, visibleRows: number): number { + return Math.max(0, rowCount - visibleRows); +} + function segmentProps(seg: StyledSegment): Record { return inkPropsForSegment(seg); } @@ -23,8 +34,7 @@ function segmentProps(seg: StyledSegment): Record { // re-render is pure waste. A small bounded cache turns that into a cache hit. const memoizedParseMarkdown = createMemoizedParseMarkdown(); -function MarkdownText({ text, width }: { text: string; width: number }): ReactNode { - const lines = memoizedParseMarkdown(text, width); +function renderMarkdownLines(lines: readonly StyledSegment[][]): ReactNode { return ( {lines.map((line, li) => ( @@ -38,6 +48,7 @@ function MarkdownText({ text, width }: { text: string; width: number }): ReactNo ); } + // Two-column layout when all options are short enough to fit side by side. // Each column gets half the inner width minus a small gap for the number prefix. function renderOptionsGrid(options: string[], selected: number, innerWidth: number): ReactNode { @@ -80,15 +91,19 @@ function renderOptionsGrid(options: string[], selected: number, innerWidth: numb return {rows}; } -function renderOptionsList(options: string[], selected: number): ReactNode { +// `startIndex` lets a windowed slice of `options` keep its true 1-based +// number and highlight against the real `selected` index, not its position +// within the slice. +function renderOptionsList(options: string[], selected: number, startIndex = 0): ReactNode { return ( {options.map((opt, i) => { - const active = i === selected; + const realIndex = startIndex + i; + const active = realIndex === selected; return ( - + {active ? "› " : " "} - {`${i + 1}. `} + {`${realIndex + 1}. `} {opt} ); @@ -97,7 +112,13 @@ function renderOptionsList(options: string[], selected: number): ReactNode { ); } -export function OperatorModal({ question, options, onSelect, width = 80 }: OperatorModalProps): ReactNode { +export function OperatorModal({ + question, + options, + onSelect, + width = 80, + terminalRows = FALLBACK_TERMINAL_ROWS, +}: OperatorModalProps): ReactNode { const [selected, setSelected] = useState(0); const [draft, setDraft] = useState(""); const typing = draft.length > 0; @@ -109,6 +130,56 @@ export function OperatorModal({ question, options, onSelect, width = 80 }: Opera const maxOptLen = options.reduce((n, o) => Math.max(n, o.length), 0); const useGrid = options.length >= 2 && options.length <= 4 && maxOptLen <= colWidth - 5; + const questionLines = memoizedParseMarkdown(question, innerWidth); + const optionsRowsNeeded = useGrid ? Math.ceil(options.length / 2) : options.length; + // border(2) + paddingY(2) + marginBottom after the question(1) + marginTop + // before the footer(1) + footer line(1). + const reservedChrome = 7; + const available = Math.max( + MIN_QUESTION_ROWS + MIN_OPTION_ROWS, + terminalRows - reservedChrome, + ); + + let questionRows: number; + let optionsRows: number; + if (questionLines.length + optionsRowsNeeded <= available) { + questionRows = questionLines.length; + optionsRows = optionsRowsNeeded; + } else { + // The selection must stay reachable, so the option list gets priority; + // whatever's left goes to the question. + optionsRows = Math.min(optionsRowsNeeded, Math.max(MIN_OPTION_ROWS, available - MIN_QUESTION_ROWS)); + questionRows = Math.max(MIN_QUESTION_ROWS, available - optionsRows); + } + + const questionScrollable = questionLines.length > questionRows; + const questionMaxOffset = maxRowOffset(questionLines.length, questionRows); + const [questionScrollOffset, setQuestionScrollOffset] = useState(0); + const clampedQuestionOffset = Math.min(questionScrollOffset, questionMaxOffset); + const visibleQuestionLines = questionScrollable + ? questionLines.slice(clampedQuestionOffset, clampedQuestionOffset + questionRows) + : questionLines; + const questionLinesAbove = clampedQuestionOffset; + const questionLinesBelow = Math.max(0, questionLines.length - clampedQuestionOffset - questionRows); + + // Only the plain list windows around the selection — grid mode is capped at + // 4 options (2 rows), which always fits. + const optionsScrollable = !useGrid && options.length > optionsRows; + let optionsWindowStart = 0; + if (optionsScrollable) { + optionsWindowStart = Math.max( + 0, + Math.min(selected - Math.floor(optionsRows / 2), options.length - optionsRows), + ); + } + const visibleOptions = optionsScrollable + ? options.slice(optionsWindowStart, optionsWindowStart + optionsRows) + : options; + const optionsAbove = optionsWindowStart; + const optionsBelow = optionsScrollable + ? options.length - optionsWindowStart - visibleOptions.length + : 0; + useInput((input, key) => { if (typing) { if (key.escape) { @@ -134,6 +205,14 @@ export function OperatorModal({ question, options, onSelect, width = 80 }: Opera onSelect({ kind: "cancel" }); return; } + if (questionScrollable && key.pageUp) { + setQuestionScrollOffset((o) => Math.max(0, o - questionRows)); + return; + } + if (questionScrollable && key.pageDown) { + setQuestionScrollOffset((o) => Math.min(questionMaxOffset, o + questionRows)); + return; + } if (key.ctrl && (key.upArrow || key.downArrow)) return; if (key.upArrow || input === "\x1B[A" || input === "[A") { setSelected((s) => (s > 0 ? s - 1 : options.length - 1)); @@ -170,8 +249,16 @@ export function OperatorModal({ question, options, onSelect, width = 80 }: Opera marginX={1} width={Math.max(24, width - 2)} > - - + + {renderMarkdownLines(visibleQuestionLines)} + {questionScrollable && ( + + {questionLinesAbove > 0 ? `↑ ${questionLinesAbove} more above` : ""} + {questionLinesAbove > 0 && questionLinesBelow > 0 ? " · " : ""} + {questionLinesBelow > 0 ? `↓ ${questionLinesBelow} more below` : ""} + {" · PageUp/PageDown to scroll"} + + )} {typing ? ( @@ -188,7 +275,14 @@ export function OperatorModal({ question, options, onSelect, width = 80 }: Opera {useGrid ? renderOptionsGrid(options, selected, innerWidth) - : renderOptionsList(options, selected)} + : renderOptionsList(visibleOptions, selected, optionsWindowStart)} + {optionsScrollable && (optionsAbove > 0 || optionsBelow > 0) && ( + + {optionsAbove > 0 ? `↑ ${optionsAbove} more above` : ""} + {optionsAbove > 0 && optionsBelow > 0 ? " · " : ""} + {optionsBelow > 0 ? `↓ ${optionsBelow} more below` : ""} + + )} {`1-${options.length} select · ↑↓ navigate · Enter choose · type to respond · Esc dismiss`} diff --git a/src/tui/components/permission-modal.tsx b/src/tui/components/permission-modal.tsx index 5678046bf..5df71d919 100644 --- a/src/tui/components/permission-modal.tsx +++ b/src/tui/components/permission-modal.tsx @@ -1,6 +1,6 @@ import { Box, Text, useInput } from "ink"; import type { ReactNode } from "react"; -import { useState } from "react"; +import { Fragment, useState } from "react"; import type { ApprovalOutcome, ApprovalScope, GrantScope, PermissionRequest } from "../../permission/types.js"; import { color } from "../theme.js"; import { describeToolCall } from "../tool-formatter.js"; @@ -61,6 +61,16 @@ function agentTagColor(label: string): string { const MAX_RENDERED_QUEUE_ENTRIES = 5; +// A body window shorter than this reads as broken (no room to show anything +// meaningful plus its scroll indicators), so it is the floor regardless of +// how little the terminal reports. +const MIN_BODY_ROWS = 3; +const FALLBACK_TERMINAL_ROWS = 24; + +function maxRowOffset(rowCount: number, visibleRows: number): number { + return Math.max(0, rowCount - visibleRows); +} + export type PermissionModalProps = { request: PermissionRequest; /** Permission gates still queued, including this modal. */ @@ -74,6 +84,9 @@ export type PermissionModalProps = { goalTimeoutMs?: number | null; onResolve: (outcome: ApprovalOutcome) => void; width?: number; + /** Terminal height, so a request body taller than it scrolls instead of + * pushing the choices off screen. Defaults to a conservative fallback. */ + terminalRows?: number; }; @@ -211,6 +224,7 @@ export function PermissionModal({ goalTimeoutMs = null, onResolve, width = 80, + terminalRows = FALLBACK_TERMINAL_ROWS, }: PermissionModalProps): ReactNode { const queuedBehind = Math.max(0, permissionQueueDepth - 1); // Everything behind the currently visible entry, distinguished by agent. @@ -251,6 +265,105 @@ export function PermissionModal({ ? Math.max(1, Math.round(goalTimeoutMs / 1000)) : null; + // The request body (segment list / summary / notice) is the one part of + // this modal that can grow arbitrarily — a long shell chain, an expanded + // heredoc payload, a dense plan-review notice. Everything else (header, + // agent line, queued-behind list, choices, footer) is bounded, so only the + // body scrolls: the focused choice and the full action list stay reachable + // below it no matter how tall the body gets. + const bodyRows: ReactNode[] = []; + if (collapsedSegments.length > 0) { + collapsedSegments.forEach((segment, i) => { + bodyRows.push( + + {collapsedSegments.length > 1 ? `${i + 1}. ${segment.display}` : segment.display} + , + ); + if (expanded) { + segment.payloads.forEach((payload, pi) => { + const lines = payload.lines.slice(0, MAX_RENDERED_LINES); + const hiddenLines = payload.lines.length - lines.length; + lines.forEach((line, li) => { + bodyRows.push( + + {" "} + {clampForDisplay(sanitizeForPrompt(line))} + , + ); + }); + if (hiddenLines > 0) { + bodyRows.push( + + {` … ${hiddenLines} more lines`} + , + ); + } + }); + } + }); + if (hiddenSegmentCount > 0) { + bodyRows.push( + {`… ${hiddenSegmentCount} more segments`}, + ); + } + if (collapsedSegments.length > 1) { + bodyRows.push( + + One decision covers every segment — rejecting any blocks the whole command. + , + ); + } + } else if (summary.length > 0) { + bodyRows.push( + {summary}, + ); + } + if (request.notice !== undefined) { + bodyRows.push( + {sanitizeForPrompt(request.notice)}, + ); + } + + // Rows fixed above and below the scrollable body — border, padding, header + // lines, the queued-behind list (kept short and always visible), choices, + // and the footer. What's left is the body's viewport. + const fixedRowsAboveBody = + 2 /* border */ + + 2 /* paddingY */ + + 1 /* "Approval needed" */ + + (request.agentLabel !== undefined ? 1 : 0) + + (goalTimeoutSecs !== null ? 1 : 0) + + 1 /* marginTop before the info box */ + + 1 /* action line */ + + shownQueued.length + + (hiddenQueuedCount > 0 ? 1 : 0) + + 1 /* marginTop before the body */; + const fixedRowsBelowBody = + 1 /* marginTop before choices */ + + choices.length + + 1 /* marginTop before footer */ + + 1 /* footer line */; + const availableBodyRows = Math.max( + MIN_BODY_ROWS, + terminalRows - fixedRowsAboveBody - fixedRowsBelowBody, + ); + const bodyScrollable = bodyRows.length > availableBodyRows; + // Reserve one row for the scroll indicator only when actually scrolling, so + // a short prompt still fits on one screen with no forced scroll affordance. + const bodyViewportRows = bodyScrollable + ? Math.max(1, availableBodyRows - 1) + : availableBodyRows; + const bodyMaxOffset = maxRowOffset(bodyRows.length, bodyViewportRows); + // Local, top-pinned scroll state — unlike the transcript's useScroll (which + // pins to the newest/bottom line), an approval body should open showing its + // start, with PageDown revealing the rest. + const [bodyScrollOffset, setBodyScrollOffset] = useState(0); + const clampedBodyOffset = Math.min(bodyScrollOffset, bodyMaxOffset); + const visibleBodyRows = bodyScrollable + ? bodyRows.slice(clampedBodyOffset, clampedBodyOffset + bodyViewportRows) + : bodyRows; + const linesAbove = clampedBodyOffset; + const linesBelow = Math.max(0, bodyRows.length - clampedBodyOffset - bodyViewportRows); useInput((input, key) => { if (key.escape) { @@ -274,6 +387,15 @@ export function PermissionModal({ return; } + if (bodyScrollable && key.pageUp) { + setBodyScrollOffset((o) => Math.max(0, o - bodyViewportRows)); + return; + } + if (bodyScrollable && key.pageDown) { + setBodyScrollOffset((o) => Math.min(bodyMaxOffset, o + bodyViewportRows)); + return; + } + if (key.ctrl && (key.upArrow || key.downArrow)) return; if (key.upArrow && message.length === 0) { setSelected((s) => (s > 0 ? s - 1 : choices.length - 1)); @@ -362,55 +484,30 @@ export function PermissionModal({ )} )} - {collapsedSegments.length > 0 ? ( + {visibleBodyRows.length > 0 && ( // The command renders exactly once, as this segment list — no // separate raw dump. Heredoc/quoted payloads are already collapsed // to a placeholder; Ctrl+O reveals their full text below each one. + // When the body is taller than the terminal, only a window of it + // renders here — PageUp/PageDown scroll it — while the choices and + // footer below stay fixed and always visible. - {collapsedSegments.map((segment, i) => ( - - - {collapsedSegments.length > 1 ? `${i + 1}. ${segment.display}` : segment.display} - - {expanded && - segment.payloads.map((payload, pi) => { - const lines = payload.lines.slice(0, MAX_RENDERED_LINES); - const hiddenLines = payload.lines.length - lines.length; - return ( - - {lines.map((line, li) => ( - - {clampForDisplay(sanitizeForPrompt(line))} - - ))} - {hiddenLines > 0 && ( - {`… ${hiddenLines} more lines`} - )} - - ); - })} - + {visibleBodyRows.map((row, i) => ( + {row} ))} - {hiddenSegmentCount > 0 && ( - {`… ${hiddenSegmentCount} more segments`} - )} - {collapsedSegments.length > 1 && ( - - One decision covers every segment — rejecting any blocks the whole command. - - )} - ) : summary.length > 0 ? ( + )} + {bodyScrollable && ( - {summary} + + {linesAbove > 0 ? `↑ ${linesAbove} more above` : ""} + {linesAbove > 0 && linesBelow > 0 ? " · " : ""} + {linesBelow > 0 ? `↓ ${linesBelow} more below` : ""} + {" · PageUp/PageDown to scroll"} + - ) : null} + )} - {request.notice !== undefined && ( - - {sanitizeForPrompt(request.notice)} - - )} {choices.map((choice, i) => { const isReject = choice.outcome.allow === false; diff --git a/tests/unit/tui/operator-modal.test.tsx b/tests/unit/tui/operator-modal.test.tsx index 92c84974e..1f7e3124b 100644 --- a/tests/unit/tui/operator-modal.test.tsx +++ b/tests/unit/tui/operator-modal.test.tsx @@ -128,3 +128,55 @@ test("renders markdown formatting in the question", () => { // Raw markers should not appear expect(lastFrame()).not.toContain("**Bold**"); }); + +test("a long option list windows around the selection and pages via arrow keys", async () => { + const manyOptions = Array.from({ length: 30 }, (_, i) => `Option ${i + 1}`); + const { lastFrame, stdin } = render( + {}} terminalRows={16} />, + ); + await tick(); + const initial = lastFrame() ?? ""; + expect(initial).toContain("1. Option 1"); + expect(initial).not.toContain("Option 30"); + expect(initial).toMatch(/↓ \d+ more below/); + + // Move the selection down past the visible window; the window should + // follow the focused option so it stays reachable and visible. + for (let i = 0; i < 20; i++) { + stdin.write("\x1B[B"); + await tick(); + } + const afterMoves = lastFrame() ?? ""; + expect(afterMoves).toContain("21. Option 21"); + expect(afterMoves).toMatch(/↑ \d+ more above/); +}); + +test("a long question scrolls with PageUp/PageDown while options stay visible", async () => { + const longQuestion = Array.from({ length: 30 }, (_, i) => `Line ${i} of the question body.`).join("\n\n"); + const { lastFrame, stdin } = render( + {}} terminalRows={16} />, + ); + await tick(); + const initial = lastFrame() ?? ""; + expect(initial).toContain("Line 0 of the question body."); + expect(initial).not.toContain("Line 29 of the question body."); + expect(initial).toContain("Option A"); + expect(initial).toContain("Option C"); + expect(initial).toMatch(/↓ \d+ more below/); + + stdin.write("\x1B[6~"); // PageDown + await tick(); + const afterPageDown = lastFrame() ?? ""; + expect(afterPageDown).not.toContain("Line 0 of the question body."); + expect(afterPageDown).toContain("Option A"); +}); + +test("a short question and short option list have no scroll indicator", () => { + const { lastFrame } = render( + {}} terminalRows={24} />, + ); + const frame = lastFrame() ?? ""; + expect(frame).not.toContain("PageUp/PageDown to scroll"); + expect(frame).not.toMatch(/more below/); + expect(frame).not.toMatch(/more above/); +}); diff --git a/tests/unit/tui/permission-modal.test.tsx b/tests/unit/tui/permission-modal.test.tsx index ed6b03c48..cea0b11ea 100644 --- a/tests/unit/tui/permission-modal.test.tsx +++ b/tests/unit/tui/permission-modal.test.tsx @@ -235,7 +235,11 @@ test("a huge chain caps the segment list and keeps the decision buttons visible" const frame = lastFrame() ?? ""; const segmentLines = (frame.split("\n") as string[]).filter((line) => /\d+\. echo /.test(line)); expect(segmentLines.length).toBeLessThanOrEqual(20); - expect(frame).toMatch(/… \d+ more segments/); + // The terminal-height-aware body scroll windows this down to a handful of + // visible segments with a scroll indicator, well before the old absolute + // 12-segment display cap would even apply. + expect(frame).toMatch(/↓ \d+ more below/); + expect(frame).toContain("PageUp/PageDown to scroll"); expect(frame).toContain("Reject"); expect(frame).toContain("Accept once"); }); @@ -262,7 +266,7 @@ test("a many-line command caps the segment list and keeps the choice chrome in f const lines = frame.split("\n") as string[]; expect(lines.length).toBeLessThan(50); expect(frame).toContain("rm -rf / #hidden"); - expect(frame).toMatch(/… \d+ more segments/); + expect(frame).toMatch(/↓ \d+ more below/); expect(frame).toContain("Reject"); expect(frame).toContain("Accept once"); }); @@ -481,3 +485,50 @@ test("a multi-line quoted commit message collapses to ", () => expect(frame).toContain("git commit -m "); expect(frame).not.toContain("line two"); }); + +test("a body taller than the terminal keeps the choices visible and pages with PageUp/PageDown", async () => { + const chain = Array.from({ length: 40 }, (_, i) => `echo line${i}`).join(" && "); + const req: PermissionRequest = { + tool: "run_shell", + action: "Run shell command", + subject: chain, + scopes: [{ id: "exact", label: "x", pattern: chain }], + }; + const { lastFrame, stdin } = render( + {}} terminalRows={20} />, + ); + await tick(); + const initial = lastFrame() ?? ""; + // The selection and every choice stay visible even though the body is + // nowhere near tall enough to fit all 40 segments. + expect(initial).toContain("Reject"); + expect(initial).toContain("Accept once"); + expect(initial).toContain("Allow these 40 echo commands — all projects"); + expect(initial).toContain("1. echo line0"); + expect(initial).not.toContain("echo line39"); + expect(initial).toMatch(/↓ \d+ more below/); + + stdin.write("\x1B[6~"); // PageDown + await tick(); + const afterPageDown = lastFrame() ?? ""; + expect(afterPageDown).not.toContain("1. echo line0"); + expect(afterPageDown).toMatch(/↑ \d+ more above/); + // Choices remain reachable after paging the body. + expect(afterPageDown).toContain("Reject"); + expect(afterPageDown).toContain("Allow these 40 echo commands — all projects"); + + stdin.write("\x1B[5~"); // PageUp back to the top + await tick(); + const afterPageUp = lastFrame() ?? ""; + expect(afterPageUp).toContain("1. echo line0"); +}); + +test("a short prompt has no scroll indicator or forced scroll affordance", () => { + const { lastFrame } = render( + {}} terminalRows={24} />, + ); + const frame = lastFrame() ?? ""; + expect(frame).not.toContain("PageUp/PageDown to scroll"); + expect(frame).not.toMatch(/more below/); + expect(frame).not.toMatch(/more above/); +}); From c2feeb4f74f3fd83dc0b1d47e272b0cf1d1275fa Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 1 Aug 2026 10:42:37 -0700 Subject: [PATCH 2/4] Lock in the approval modal's scroll indicator behavior The overflow indicator already shows "more below" while content remains past the visible window and swaps to "more above" once scrolled to the end, right next to the Accept/Reject choices. No gap found, so this only covers the existing behavior with a test. --- src/tui/components/permission-modal.test.ts | 66 +++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 src/tui/components/permission-modal.test.ts diff --git a/src/tui/components/permission-modal.test.ts b/src/tui/components/permission-modal.test.ts new file mode 100644 index 000000000..84913036c --- /dev/null +++ b/src/tui/components/permission-modal.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "bun:test"; +import { createElement } from "react"; +import { render } from "ink-testing-library"; +import type { PermissionRequest } from "../../permission/types.js"; +import { PermissionModal } from "./permission-modal.js"; + +function longChainRequest(segmentCount: number): PermissionRequest { + const subject = Array.from({ length: segmentCount }, (_, i) => `echo line-${i}`).join(" && "); + return { tool: "run_shell", action: "Run", subject, scopes: [] }; +} + +describe("PermissionModal scroll indicator", () => { + test("shows a 'more below' indicator when the body overflows and is scrolled to the top", () => { + const { lastFrame, unmount } = render( + createElement(PermissionModal, { + request: longChainRequest(30), + onResolve: () => {}, + terminalRows: 15, + }), + ); + + const frame = lastFrame() ?? ""; + expect(frame).toContain("more below"); + expect(frame).not.toContain("more above"); + + unmount(); + }); + + test("swaps to a 'more above' indicator, with no 'more below', once scrolled to the end", async () => { + const { lastFrame, stdin, unmount } = render( + createElement(PermissionModal, { + request: longChainRequest(30), + onResolve: () => {}, + terminalRows: 15, + }), + ); + + // Page down repeatedly past the end of the body; offset clamps at max. + for (let i = 0; i < 10; i++) { + stdin.write("\x1b[6~"); + await new Promise((r) => setTimeout(r, 0)); + } + + const frame = lastFrame() ?? ""; + expect(frame).toContain("more above"); + expect(frame).not.toContain("more below"); + + unmount(); + }); + + test("shows no scroll indicator when the body fits without overflow", () => { + const { lastFrame, unmount } = render( + createElement(PermissionModal, { + request: longChainRequest(2), + onResolve: () => {}, + terminalRows: 40, + }), + ); + + const frame = lastFrame() ?? ""; + expect(frame).not.toContain("more below"); + expect(frame).not.toContain("more above"); + + unmount(); + }); +}); From 018502f15e81d90516cacdbb142915753af17a33 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 1 Aug 2026 21:28:07 -0700 Subject: [PATCH 3/4] Share the modal scroll-window math in one hook operator-modal and permission-modal each carried their own copy of FALLBACK_TERMINAL_ROWS, maxRowOffset, and the clamp-and-slice PageUp/PageDown logic. Extract it into useScrollWindow so the offset math is owned in one place. --- src/tui/components/operator-modal.tsx | 19 ++++----- src/tui/components/permission-modal.tsx | 23 +++++------ src/tui/hooks/use-scroll-window.test.tsx | 51 ++++++++++++++++++++++++ src/tui/hooks/use-scroll-window.ts | 39 ++++++++++++++++++ 4 files changed, 106 insertions(+), 26 deletions(-) create mode 100644 src/tui/hooks/use-scroll-window.test.tsx create mode 100644 src/tui/hooks/use-scroll-window.ts diff --git a/src/tui/components/operator-modal.tsx b/src/tui/components/operator-modal.tsx index a8b25c1b0..57dac9c23 100644 --- a/src/tui/components/operator-modal.tsx +++ b/src/tui/components/operator-modal.tsx @@ -6,6 +6,7 @@ import { createMemoizedParseMarkdown } from "../markdown-parser.js"; import type { StyledSegment } from "../markdown-parser.js"; import { color } from "../theme.js"; import { inkPropsForSegment } from "../styled-segment-props.js"; +import { FALLBACK_TERMINAL_ROWS, useScrollWindow } from "../hooks/use-scroll-window.js"; export type OperatorModalProps = { question: string; @@ -17,14 +18,9 @@ export type OperatorModalProps = { terminalRows?: number; }; -const FALLBACK_TERMINAL_ROWS = 24; const MIN_QUESTION_ROWS = 2; const MIN_OPTION_ROWS = 3; -function maxRowOffset(rowCount: number, visibleRows: number): number { - return Math.max(0, rowCount - visibleRows); -} - function segmentProps(seg: StyledSegment): Record { return inkPropsForSegment(seg); } @@ -153,14 +149,13 @@ export function OperatorModal({ } const questionScrollable = questionLines.length > questionRows; - const questionMaxOffset = maxRowOffset(questionLines.length, questionRows); - const [questionScrollOffset, setQuestionScrollOffset] = useState(0); - const clampedQuestionOffset = Math.min(questionScrollOffset, questionMaxOffset); + const questionScroll = useScrollWindow(questionLines.length, questionRows); + const clampedQuestionOffset = questionScroll.offset; const visibleQuestionLines = questionScrollable ? questionLines.slice(clampedQuestionOffset, clampedQuestionOffset + questionRows) : questionLines; - const questionLinesAbove = clampedQuestionOffset; - const questionLinesBelow = Math.max(0, questionLines.length - clampedQuestionOffset - questionRows); + const questionLinesAbove = questionScroll.above; + const questionLinesBelow = questionScroll.below; // Only the plain list windows around the selection — grid mode is capped at // 4 options (2 rows), which always fits. @@ -206,11 +201,11 @@ export function OperatorModal({ return; } if (questionScrollable && key.pageUp) { - setQuestionScrollOffset((o) => Math.max(0, o - questionRows)); + questionScroll.pageUp(); return; } if (questionScrollable && key.pageDown) { - setQuestionScrollOffset((o) => Math.min(questionMaxOffset, o + questionRows)); + questionScroll.pageDown(); return; } if (key.ctrl && (key.upArrow || key.downArrow)) return; diff --git a/src/tui/components/permission-modal.tsx b/src/tui/components/permission-modal.tsx index 5df71d919..66aede546 100644 --- a/src/tui/components/permission-modal.tsx +++ b/src/tui/components/permission-modal.tsx @@ -8,6 +8,7 @@ import { stripTerminalControlSequences } from "../../util/control-char-strip.js" import { isShellCommentOnly } from "../../permission/command.js"; import { collapseSegmentPayloads, groupChainSegmentsForDisplay, middleEllipsis } from "../command-display.js"; import type { QueuedApprovalSummary } from "../hooks/use-gates.js"; +import { FALLBACK_TERMINAL_ROWS, useScrollWindow } from "../hooks/use-scroll-window.js"; // Bidi controls (RLO, embeddings, isolates) visually reorder the rendered // command — Trojan Source — and zero-width characters hide payload boundaries, @@ -65,11 +66,6 @@ const MAX_RENDERED_QUEUE_ENTRIES = 5; // meaningful plus its scroll indicators), so it is the floor regardless of // how little the terminal reports. const MIN_BODY_ROWS = 3; -const FALLBACK_TERMINAL_ROWS = 24; - -function maxRowOffset(rowCount: number, visibleRows: number): number { - return Math.max(0, rowCount - visibleRows); -} export type PermissionModalProps = { request: PermissionRequest; @@ -353,17 +349,16 @@ export function PermissionModal({ const bodyViewportRows = bodyScrollable ? Math.max(1, availableBodyRows - 1) : availableBodyRows; - const bodyMaxOffset = maxRowOffset(bodyRows.length, bodyViewportRows); - // Local, top-pinned scroll state — unlike the transcript's useScroll (which - // pins to the newest/bottom line), an approval body should open showing its + // Top-pinned scroll state — unlike the transcript's useScroll (which pins + // to the newest/bottom line), an approval body should open showing its // start, with PageDown revealing the rest. - const [bodyScrollOffset, setBodyScrollOffset] = useState(0); - const clampedBodyOffset = Math.min(bodyScrollOffset, bodyMaxOffset); + const bodyScroll = useScrollWindow(bodyRows.length, bodyViewportRows); + const clampedBodyOffset = bodyScroll.offset; const visibleBodyRows = bodyScrollable ? bodyRows.slice(clampedBodyOffset, clampedBodyOffset + bodyViewportRows) : bodyRows; - const linesAbove = clampedBodyOffset; - const linesBelow = Math.max(0, bodyRows.length - clampedBodyOffset - bodyViewportRows); + const linesAbove = bodyScroll.above; + const linesBelow = bodyScroll.below; useInput((input, key) => { if (key.escape) { @@ -388,11 +383,11 @@ export function PermissionModal({ } if (bodyScrollable && key.pageUp) { - setBodyScrollOffset((o) => Math.max(0, o - bodyViewportRows)); + bodyScroll.pageUp(); return; } if (bodyScrollable && key.pageDown) { - setBodyScrollOffset((o) => Math.min(bodyMaxOffset, o + bodyViewportRows)); + bodyScroll.pageDown(); return; } diff --git a/src/tui/hooks/use-scroll-window.test.tsx b/src/tui/hooks/use-scroll-window.test.tsx new file mode 100644 index 000000000..7f407e7b9 --- /dev/null +++ b/src/tui/hooks/use-scroll-window.test.tsx @@ -0,0 +1,51 @@ +import { test, expect } from "bun:test"; +import { render } from "ink-testing-library"; +import type { ReactElement } from "react"; +import { Text, useInput } from "ink"; +import { useScrollWindow } from "./use-scroll-window.js"; + +const tick = () => new Promise((resolve) => setTimeout(resolve, 20)); + +// A minimal harness component so the hook's state (useState-backed) is +// exercised the same way the modals use it: PageUp/PageDown drive offset, +// and above/below report what's scrolled out of view. +function Harness({ rowCount, visibleRows }: { rowCount: number; visibleRows: number }): ReactElement { + const window = useScrollWindow(rowCount, visibleRows); + useInput((_input, key) => { + if (key.pageDown) window.pageDown(); + if (key.pageUp) window.pageUp(); + }); + return {`offset=${window.offset} above=${window.above} below=${window.below} max=${window.maxOffset}`}; +} + +test("useScrollWindow starts at the top with everything below in view", () => { + const { lastFrame } = render(); + expect(lastFrame()).toBe("offset=0 above=0 below=6 max=6"); +}); + +test("useScrollWindow pages down and clamps at maxOffset", async () => { + const { stdin, lastFrame } = render(); + stdin.write("\x1B[6~"); // PageDown + await tick(); + expect(lastFrame()).toBe("offset=4 above=4 below=2 max=6"); + stdin.write("\x1B[6~"); // PageDown again — clamps instead of overshooting + await tick(); + expect(lastFrame()).toBe("offset=6 above=6 below=0 max=6"); +}); + +test("useScrollWindow pages back up and clamps at 0", async () => { + const { stdin, lastFrame } = render(); + stdin.write("\x1B[6~"); // PageDown + await tick(); + stdin.write("\x1B[5~"); // PageUp back to the top + await tick(); + expect(lastFrame()).toBe("offset=0 above=0 below=6 max=6"); + stdin.write("\x1B[5~"); // PageUp again — clamps instead of going negative + await tick(); + expect(lastFrame()).toBe("offset=0 above=0 below=6 max=6"); +}); + +test("useScrollWindow reports maxOffset 0 when content already fits", () => { + const { lastFrame } = render(); + expect(lastFrame()).toBe("offset=0 above=0 below=0 max=0"); +}); diff --git a/src/tui/hooks/use-scroll-window.ts b/src/tui/hooks/use-scroll-window.ts new file mode 100644 index 000000000..2dd512d0d --- /dev/null +++ b/src/tui/hooks/use-scroll-window.ts @@ -0,0 +1,39 @@ +import { useState } from "react"; + +// Terminals that don't report a size (or report one before the first resize +// event lands) fall back to a conservative row count so a modal still lays +// out something scrollable rather than assuming infinite height. +export const FALLBACK_TERMINAL_ROWS = 24; + +function maxRowOffset(rowCount: number, visibleRows: number): number { + return Math.max(0, rowCount - visibleRows); +} + +export type ScrollWindow = { + // Current top-of-viewport row, clamped to [0, maxOffset]. + offset: number; + maxOffset: number; + // Row counts above/below the visible window, for a "N more above/below" + // scroll indicator. + above: number; + below: number; + pageUp: () => void; + pageDown: () => void; +}; + +// Top-pinned scroll state shared by the approval modals: a body opens showing +// its start, with PageUp/PageDown clamped to [0, rowCount - visibleRows]. +// Both operator-modal and permission-modal had their own copy of this offset +// math and paging logic; this is the single place it's owned now. +export function useScrollWindow(rowCount: number, visibleRows: number): ScrollWindow { + const maxOffset = maxRowOffset(rowCount, visibleRows); + const [rawOffset, setOffset] = useState(0); + const offset = Math.min(rawOffset, maxOffset); + const above = offset; + const below = Math.max(0, rowCount - offset - visibleRows); + + const pageUp = (): void => setOffset((o) => Math.max(0, o - visibleRows)); + const pageDown = (): void => setOffset((o) => Math.min(maxOffset, o + visibleRows)); + + return { offset, maxOffset, above, below, pageUp, pageDown }; +} From 278b8bc2017b91960f19094efc9abeb169b7fcf3 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 3 Aug 2026 17:27:31 -0700 Subject: [PATCH 4/4] Harden approval modal scroll, expand, and agent-label display Clamp scroll offset before paging when content shrinks, reserve rows for scroll indicators, keep PageUp/PageDown while typing a custom answer, sanitize and clamp agent labels/cwds, expand non-collapsible multi-line bodies, and hide Ctrl+O when nothing is expandable. --- src/tui/components/operator-modal.tsx | 39 ++++++++--- src/tui/components/permission-modal.tsx | 82 ++++++++++++++++++++---- src/tui/hooks/use-scroll-window.test.tsx | 14 ++++ src/tui/hooks/use-scroll-window.ts | 15 ++++- 4 files changed, 127 insertions(+), 23 deletions(-) diff --git a/src/tui/components/operator-modal.tsx b/src/tui/components/operator-modal.tsx index 57dac9c23..67000e839 100644 --- a/src/tui/components/operator-modal.tsx +++ b/src/tui/components/operator-modal.tsx @@ -131,21 +131,32 @@ export function OperatorModal({ // border(2) + paddingY(2) + marginBottom after the question(1) + marginTop // before the footer(1) + footer line(1). const reservedChrome = 7; - const available = Math.max( + const baseAvailable = Math.max( MIN_QUESTION_ROWS + MIN_OPTION_ROWS, terminalRows - reservedChrome, ); - let questionRows: number; - let optionsRows: number; - if (questionLines.length + optionsRowsNeeded <= available) { - questionRows = questionLines.length; - optionsRows = optionsRowsNeeded; - } else { + const layout = (available: number): { questionRows: number; optionsRows: number } => { + if (questionLines.length + optionsRowsNeeded <= available) { + return { questionRows: questionLines.length, optionsRows: optionsRowsNeeded }; + } // The selection must stay reachable, so the option list gets priority; // whatever's left goes to the question. - optionsRows = Math.min(optionsRowsNeeded, Math.max(MIN_OPTION_ROWS, available - MIN_QUESTION_ROWS)); - questionRows = Math.max(MIN_QUESTION_ROWS, available - optionsRows); + const optionsRows = Math.min( + optionsRowsNeeded, + Math.max(MIN_OPTION_ROWS, available - MIN_QUESTION_ROWS), + ); + const questionRows = Math.max(MIN_QUESTION_ROWS, available - optionsRows); + return { questionRows, optionsRows }; + }; + + let { questionRows, optionsRows } = layout(baseAvailable); + const questionScrollableTentative = questionLines.length > questionRows; + const optionsScrollableTentative = !useGrid && options.length > optionsRows; + const indicatorRows = + (questionScrollableTentative ? 1 : 0) + (optionsScrollableTentative ? 1 : 0); + if (indicatorRows > 0) { + ({ questionRows, optionsRows } = layout(baseAvailable - indicatorRows)); } const questionScrollable = questionLines.length > questionRows; @@ -190,6 +201,16 @@ export function OperatorModal({ setDraft((d) => d.slice(0, -1)); return; } + // Page keys still scroll a long question while the operator is typing a + // custom answer — the draft is unrelated to the question viewport. + if (questionScrollable && key.pageUp) { + questionScroll.pageUp(); + return; + } + if (questionScrollable && key.pageDown) { + questionScroll.pageDown(); + return; + } if (!key.ctrl && !key.meta && input.length > 0 && /^[\x20-\x7E -￿]+$/.test(input)) { setDraft((d) => d + input); } diff --git a/src/tui/components/permission-modal.tsx b/src/tui/components/permission-modal.tsx index 66aede546..78283e3bc 100644 --- a/src/tui/components/permission-modal.tsx +++ b/src/tui/components/permission-modal.tsx @@ -60,6 +60,26 @@ function agentTagColor(label: string): string { return color(AGENT_TAG_ROLES[hash % AGENT_TAG_ROLES.length]!); } +// Dispatch labels and worktree paths are model- or environment-authored and +// sit next to the approval chrome — strip the same bidi/control sequences the +// command subject gets, then clamp so a long label cannot shove choices off +// screen. +const MAX_AGENT_LABEL_LENGTH = 48; +const MAX_AGENT_CWD_LENGTH = 64; + +function displayAgentLabel(label: string): string { + const cleaned = sanitizeForPrompt(label); + return cleaned.length > MAX_AGENT_LABEL_LENGTH + ? `${cleaned.slice(0, MAX_AGENT_LABEL_LENGTH - 1)}…` + : cleaned; +} + +function displayAgentCwd(cwd: string): string { + const cleaned = sanitizeForPrompt(cwd); + if (cleaned.length <= MAX_AGENT_CWD_LENGTH) return cleaned; + return `…${cleaned.slice(-(MAX_AGENT_CWD_LENGTH - 1))}`; +} + const MAX_RENDERED_QUEUE_ENTRIES = 5; // A body window shorter than this reads as broken (no room to show anything @@ -246,13 +266,28 @@ export function PermissionModal({ // newline is recognized as a payload boundary, not just turned into a ↵ // marker — this is what lets the command render once, as one line per // segment, with no separate raw dump underneath. + // + // Segments that refuse to collapse (interpreters, eval, …) can still be + // multi-line or longer than the display clamp. Ctrl+O expands those via + // expandLines so the operator can read the full body they are approving. const collapsedSegments = cappedSegments.map((segment) => { const collapsed = collapseSegmentPayloads(segment); + const sanitizedDisplay = sanitizeForPrompt(collapsed.display); + const display = clampForDisplay(sanitizedDisplay); + const expandLines = + collapsed.payloads.length === 0 && + (segment.includes("\n") || sanitizedDisplay.length > MAX_DISPLAY_LINE_LENGTH) + ? segment.split("\n") + : []; return { - display: clampForDisplay(sanitizeForPrompt(collapsed.display)), + display, payloads: collapsed.payloads, + expandLines, }; }); + const canExpandBody = collapsedSegments.some( + (segment) => segment.payloads.length > 0 || segment.expandLines.length > 0, + ); const activeChoice = choices[selected]; const messageMode = message.length > 0 || false; @@ -276,12 +311,32 @@ export function PermissionModal({ , ); if (expanded) { - segment.payloads.forEach((payload, pi) => { - const lines = payload.lines.slice(0, MAX_RENDERED_LINES); - const hiddenLines = payload.lines.length - lines.length; + if (segment.payloads.length > 0) { + segment.payloads.forEach((payload, pi) => { + const lines = payload.lines.slice(0, MAX_RENDERED_LINES); + const hiddenLines = payload.lines.length - lines.length; + lines.forEach((line, li) => { + bodyRows.push( + + {" "} + {clampForDisplay(sanitizeForPrompt(line))} + , + ); + }); + if (hiddenLines > 0) { + bodyRows.push( + + {` … ${hiddenLines} more lines`} + , + ); + } + }); + } else if (segment.expandLines.length > 0) { + const lines = segment.expandLines.slice(0, MAX_RENDERED_LINES); + const hiddenLines = segment.expandLines.length - lines.length; lines.forEach((line, li) => { bodyRows.push( - + {" "} {clampForDisplay(sanitizeForPrompt(line))} , @@ -289,12 +344,12 @@ export function PermissionModal({ }); if (hiddenLines > 0) { bodyRows.push( - + {` … ${hiddenLines} more lines`} , ); } - }); + } } }); if (hiddenSegmentCount > 0) { @@ -378,7 +433,7 @@ export function PermissionModal({ } if (key.ctrl && input === "o") { - setExpanded((e) => !e); + if (canExpandBody) setExpanded((e) => !e); return; } @@ -436,9 +491,9 @@ export function PermissionModal({ Approval needed {request.agentLabel !== undefined && ( - {`⏺ ${request.agentLabel}`} + {`⏺ ${displayAgentLabel(request.agentLabel)}`} {request.cwd !== undefined && ( - {` ${request.cwd}`} + {` ${displayAgentCwd(request.cwd)}`} )} )} @@ -467,7 +522,7 @@ export function PermissionModal({ {"· "} {entry.agentLabel !== undefined ? ( - {entry.agentLabel} + {displayAgentLabel(entry.agentLabel)} ) : ( session )} @@ -560,7 +615,10 @@ export function PermissionModal({ {messageMode ? "Enter confirm · Esc clear · ↑↓ navigate" - : `1-${choices.length} select · ↑↓ navigate · Enter choose · Ctrl+O ${expanded ? "collapse" : "expand"} · Esc reject`} + : canExpandBody + ? `1-${choices.length} select · ↑↓ navigate · Enter choose · Ctrl+O ${expanded ? "collapse" : "expand"} · Esc reject` + : `1-${choices.length} select · ↑↓ navigate · Enter choose · Esc reject`} + diff --git a/src/tui/hooks/use-scroll-window.test.tsx b/src/tui/hooks/use-scroll-window.test.tsx index 7f407e7b9..ef9412dac 100644 --- a/src/tui/hooks/use-scroll-window.test.tsx +++ b/src/tui/hooks/use-scroll-window.test.tsx @@ -49,3 +49,17 @@ test("useScrollWindow reports maxOffset 0 when content already fits", () => { const { lastFrame } = render(); expect(lastFrame()).toBe("offset=0 above=0 below=0 max=0"); }); + +test("useScrollWindow clamps a stale raw offset when content shrinks", async () => { + // Start tall so paging can leave rawOffset past the later maxOffset. + const { stdin, lastFrame, rerender } = render(); + for (let i = 0; i < 6; i++) { + stdin.write("\x1B[6~"); // PageDown + await tick(); + } + expect(lastFrame()).toBe("offset=25 above=25 below=0 max=25"); + // Content shrinks — displayed offset must clamp immediately without PageUp. + rerender(); + await tick(); + expect(lastFrame()).toBe("offset=3 above=3 below=0 max=3"); +}); diff --git a/src/tui/hooks/use-scroll-window.ts b/src/tui/hooks/use-scroll-window.ts index 2dd512d0d..c7c6e18f3 100644 --- a/src/tui/hooks/use-scroll-window.ts +++ b/src/tui/hooks/use-scroll-window.ts @@ -32,8 +32,19 @@ export function useScrollWindow(rowCount: number, visibleRows: number): ScrollWi const above = offset; const below = Math.max(0, rowCount - offset - visibleRows); - const pageUp = (): void => setOffset((o) => Math.max(0, o - visibleRows)); - const pageDown = (): void => setOffset((o) => Math.min(maxOffset, o + visibleRows)); + // Clamp the stored offset to the current max before paging so a shrink of + // the content (or of the viewport) does not force the operator to press + // PageUp multiple times just to reach the true top of the window. + const pageUp = (): void => + setOffset((o) => { + const current = Math.min(o, maxOffset); + return Math.max(0, current - visibleRows); + }); + const pageDown = (): void => + setOffset((o) => { + const current = Math.min(o, maxOffset); + return Math.min(maxOffset, current + visibleRows); + }); return { offset, maxOffset, above, below, pageUp, pageDown }; }