diff --git a/.changeset/history-scroll-and-full-errors.md b/.changeset/history-scroll-and-full-errors.md new file mode 100644 index 00000000..1a7c9acc --- /dev/null +++ b/.changeset/history-scroll-and-full-errors.md @@ -0,0 +1,31 @@ +--- +"@noormdev/cli": minor +--- + +Make the execution history and file-execution screens scroll, and let a failed +file's error be read in full. + +Both screens drew a fixed window — `slice(0, 15)` of the history, `slice(0, 20)` +of the files — while their selection cursor ranged over every record. Past that +window the arrow keys still moved a selection that was not on screen, and the +detail box under the list described a record the reader could not see, so the +screens were not merely unscrollable: they were reporting on rows that had +scrolled out from under the cursor. Both now use `SelectList`, which owns the +cursor and the window together and sizes itself from the terminal, so what is +selected is always drawn and a tall terminal shows more rows rather than the same +fifteen. The history list also fetches 200 records instead of 50, now that +reaching past the fifteenth is possible. + +A failed file's error message was rendered as one line per `\n`, with no bound. +A stack trace therefore pushed the file list, the detail box and the hotkey hints +off the bottom together — and on the history screen the same message was cut at +80 characters instead, which drops the part that names the constraint or the +syntax error. Both screens now show a bounded single line in the detail box and +open the whole message, wrapped and scrollable, on a keypress: `e` on the history +screen, Enter on a file execution. + +`SelectList` gains an optional `renderItem`, which lets a screen draw its own row +body while the list keeps the cursor, the window, the scroll indicators, focus and +the mouse. Without it these two screens would have had to give up the per-status +colour a reader scans a history list for, which is why they had their own list in +the first place. diff --git a/.changeset/inspect-scroll-and-masked-secrets.md b/.changeset/inspect-scroll-and-masked-secrets.md new file mode 100644 index 00000000..557d5ee1 --- /dev/null +++ b/.changeset/inspect-scroll-and-masked-secrets.md @@ -0,0 +1,34 @@ +--- +"@noormdev/cli": minor +--- + +Make `run inspect` scrollable, and show secrets partially rather than as a count. + +The inspect screen rendered its context as a nested tree that grew with the +project, and Ink has no scroll offset — so on any real template the bottom of +the view sat below the fold with no key that could reach it, and the screen's +own footer was what got pushed off to make room. Every view it offers (summary, +expanded, rendered SQL, and render errors) is now a flat list of one element per +visual line behind a viewport, scrolled with the same `↑↓` / `^U` / `^D` keys the +explore and SQL screens already use. + +`$.secrets` and `$.globalSecrets` reported a key count, which cannot answer the +question the screen is opened to answer: a stale password and a fresh one are +both `Object (7 keys)`. Both tiers now show a partial reveal that narrows as the +value gets shorter — a four-character value shows nothing, a long one shows two +characters and a four-character suffix — with the length beside it as a number, +so a value that is set but empty is distinguishable from one that is set wrong. +`$.env` is listed and masked on the same terms, because it is the whole of +`process.env` and nothing in the screen can tell which of its keys are +credentials. + +The mouse wheel now scrolls every viewport, which it never did. Only `SelectList` +and `ResultTable` consumed wheel notches, so the explore detail view, the +full-text overlay and the row viewer ignored them — and because the TUI runs in +the alternate screen, which has no scrollback, and mouse tracking intercepts the +notches a terminal would otherwise translate into arrow keys, turning the mouse +on had actually removed the only wheel behaviour those panes had. + +Also fixes an error path that could never render: a template whose helper failed +to load set the screen's error phase with a file selected, which no branch +matched, so the most likely failure showed as "Unknown phase". diff --git a/.claude/project/followups/INDEX.md b/.claude/project/followups/INDEX.md index 866a85f2..750df647 100644 --- a/.claude/project/followups/INDEX.md +++ b/.claude/project/followups/INDEX.md @@ -2,26 +2,27 @@ Auto-generated by `atomic followups render`. Do not edit. -Open: 10 • Stale: 0 • Last rendered: 2026-07-14 +Open: 11 • Stale: 6 • Last rendered: 2026-09-08 ## 📋 plans (1) - [configurable-sql-function-policy](configurable-sql-function-policy.md) — Configurable per-config SQL function allow/deny list + TUI editor → src/core/policy/classify.ts:79 (DESTRUCTIVE_FUNCTIONS) -## 🟡 risks (6) +## 🟡 risks (7) -- [config-module-scope-env-snapshot](config-module-scope-env-snapshot.md) — Move makeNestedConfig to call-time in config module (43d) -- [db-transfer-no-fk-negation-collision](db-transfer-no-fk-negation-collision.md) — citty --no-fk/--no-identity never actually toggle (negation collision) (1d) -- [downgrade-unprotects-configs](downgrade-unprotects-configs.md) — Downgrade after schemaVersion-2 migration silently unprotects all configs (6d) -- [policy-denial-observability](policy-denial-observability.md) — Policy denials leave no server-side trace; MCP server never inits logger (6d) -- [state-enc-atomic-write-lock](state-enc-atomic-write-lock.md) — state.enc: atomic write + inter-process lock + pre-migration backup (6d) -- [v1-45-rewind-tiebreak-f1](v1-45-rewind-tiebreak-f1.md) — rewind tiebreak tests rely on wall-clock timing, not forced tie (0d) +- [config-module-scope-env-snapshot](config-module-scope-env-snapshot.md) — Move makeNestedConfig to call-time in config module (99d, **stale**) +- [db-transfer-no-fk-negation-collision](db-transfer-no-fk-negation-collision.md) — citty --no-fk/--no-identity never actually toggle (negation collision) (57d) +- [downgrade-unprotects-configs](downgrade-unprotects-configs.md) — Downgrade after schemaVersion-2 migration silently unprotects all configs (62d, **stale**) +- [policy-denial-observability](policy-denial-observability.md) — Policy denials leave no server-side trace; MCP server never inits logger (62d, **stale**) +- [state-enc-atomic-write-lock](state-enc-atomic-write-lock.md) — state.enc: atomic write + inter-process lock + pre-migration backup (62d, **stale**) +- [tui-unbounded-render-sweep](tui-unbounded-render-sweep.md) — ~40 TUI screens still render unbounded content past the fold (1d) +- [v1-45-rewind-tiebreak-f1](v1-45-rewind-tiebreak-f1.md) — rewind tiebreak tests rely on wall-clock timing, not forced tie (56d) ## 🔵 nits (2) -- [debug-process-test-no-assertions](debug-process-test-no-assertions.md) — debug-process.test.ts has no assertions (passes unconditionally) (43d) -- [legacy-protected-removal-trigger](legacy-protected-removal-trigger.md) — Legacy 'protected' input path has no removal trigger; export still mints it (6d) +- [debug-process-test-no-assertions](debug-process-test-no-assertions.md) — debug-process.test.ts has no assertions (passes unconditionally) (99d, **stale**) +- [legacy-protected-removal-trigger](legacy-protected-removal-trigger.md) — Legacy 'protected' input path has no removal trigger; export still mints it (62d, **stale**) ## ❓ questions (1) -- [v1-21-31-hygiene-f2](v1-21-31-hygiene-f2.md) — Add release-engine paragraph to ignatius CLAUDE.md (2d) +- [v1-21-31-hygiene-f2](v1-21-31-hygiene-f2.md) — Add release-engine paragraph to ignatius CLAUDE.md (58d) diff --git a/.claude/project/followups/tui-unbounded-render-sweep.md b/.claude/project/followups/tui-unbounded-render-sweep.md new file mode 100644 index 00000000..ae073426 --- /dev/null +++ b/.claude/project/followups/tui-unbounded-render-sweep.md @@ -0,0 +1,86 @@ +--- +id: tui-unbounded-render-sweep +title: ~40 TUI screens still render unbounded content past the fold +created: "2026-09-07" +origin: | + Full sweep of src/tui/screens/** and src/tui/components/** after the third + user report of a screen that "doesn't scroll" (inspect, then history). +kind: finding +severity: risk +review_by: "2026-11-07" +status: open +file: src/tui/screens/change/ChangeListScreen.tsx:318 (worst of ~40) +--- + +Ink has no scroll offset: content taller than the terminal is unreachable, and it +pushes the screen's own footer off the bottom rather than clipping. Three screens +have been fixed one at a time as they were reported — explore, run inspect, then +the two change-history screens. This is the list of every remaining site, so the +next one is picked off a list rather than off a user's screen. + +Four defect shapes, in descending severity: + +- **A — fixed slice.** `.slice(0, N)` with a literal N and usually an "…and N + more" line. The window never moves, so the rest is unreachable. +- **B — roaming cursor past a fixed window.** A `selectedIndex` clamped to + `array.length - 1` while the render draws a fixed slice, so past row N the + cursor selects rows that are not drawn and the detail box describes a record + that is not on screen. Always co-occurs with A, and is the reason A is worse + than it looks. +- **C — unbounded map.** `.map()` over state, or over `err.split('\n')`, with no + windowing at all. Multi-line SQL errors and stack traces are the usual source. +- **D — hardcoded height.** A row budget that is a literal instead of + `useViewportRows()`. Content stays reachable; the screen just mis-sizes. + +## Ranked + +1. `screens/change/ChangeListScreen.tsx:318` — C+B. The primary `noorm change` + screen. Fully unbounded, no cap to even hit. +2. `screens/identity/KnownUsersScreen.tsx:164,190` — C+B, nested. Grows with team + size, which is the screen's whole purpose. +3. `screens/vault/VaultScreen.tsx:447` — C. Lists every identity about to receive + the vault master key, directly above the type-to-confirm gate it can push off + screen. +4. `screens/db/DbTruncateScreen.tsx:276,290` — A. Preview before an irreversible + TRUNCATE, capped at 10/5. +5. `screens/db/DbTransferScreen.tsx:1053,1091,1375,1392` — C ×4. Import preview, + plan warnings, export summary, failures. +6. **The `split('\n').map()` family** — C, the same defect copy-pasted across + every execution screen: `ChangeRunScreen.tsx:356,369`, + `RunBuildScreen.tsx:481,502`, `RunDirScreen.tsx:463,763,771` (763 has no "more" + indicator at all), `RunExecScreen.tsx:231`, `RunFileScreen.tsx:593`. + `TextOverlay` was built for exactly this and is the direct fix. +7. `screens/db/DtModifyScreen.tsx:963,1096` — C. Column-schema dumps; 30-60 + columns is normal for a DT export. +8. `components/overlays/LogViewerOverlay.tsx:327` — C. The log-entry detail view + has no scroll keys at all, only Escape. +9. Confirm-preview family, bounded impact — `ChangeNextScreen.tsx:310`, + `ChangeFFScreen.tsx:255`, `ChangeRewindScreen.tsx:334,376` (334 has no + indicator), `ChangeRevertScreen.tsx:337`. +10. Pattern D, reachable but mis-sized — `SqlHistoryScreen.tsx:74` (reads + `terminalHeight` two lines away and still hardcodes 10), + `ResultTable.tsx:333` (default 15, not overridden by SqlHistoryScreen), + `SecretDefinitionList.tsx:319` (8), `SecretValueList.tsx:262` (10), + `DbTeardownScreen.tsx:264,459` (15), `LogViewerOverlay.tsx:44` (12). + +Low volume, real but bounded in practice: `home.tsx:528`, +`ConfigValidateScreen.tsx:163`, `SecretValueForm.tsx:204`, +`DtModifyScreen.tsx:1059,1547`, `SqlInput.tsx:261`. + +Checked and ruled out: `DebugDetailScreen.tsx:282` (columns come from noorm's own +six fixed tracking tables, and values truncate at 77 chars), `home.tsx`'s +`recentActivity` (capped at the query), `InitScreen`'s fixed wizard steps, +`Form.tsx`'s `SELECT_VISIBLE_OPTIONS` and `RowPeekOverlay`'s `MAX_SET_ROWS` (both +deliberate centered windows). + +## Reuse, do not reinvent + +- `SelectList` with `renderItem` — a list with a cursor. Owns cursor, window, + wheel, click and position memory together, which is what makes B impossible. + Target for ChangeListScreen, KnownUsersScreen, DtModifyScreen's column tables. +- `TextOverlay` — a long message at full length. Target for every + `split('\n').map()` above. +- `ScrollPane` + `rowWindow`/`scrollTarget` — a viewport over pre-laid-out lines + with no cursor. +- `useViewportRows(reserveRows)` — every hardcoded Pattern-D constant. +- `oneLine(text)` — any string entering a counted single row. diff --git a/src/core/shared/index.ts b/src/core/shared/index.ts index fafd72b1..85ae15c4 100644 --- a/src/core/shared/index.ts +++ b/src/core/shared/index.ts @@ -14,6 +14,9 @@ export { OperationAbortedError, throwIfAborted, raceAbort } from './abort.js'; // Files export { filterFilesByPaths, findUnmatchedIncludePatterns, findUnmatchedExcludePatterns } from './files.js'; +// Secret display +export { maskSecret } from './mask.js'; + // Dialect quoting export { createDialectQuoting, type DialectQuoting } from './dialect-quoting.js'; diff --git a/src/core/shared/mask.ts b/src/core/shared/mask.ts new file mode 100644 index 00000000..9eafe5cb --- /dev/null +++ b/src/core/shared/mask.ts @@ -0,0 +1,88 @@ +/** + * Partial masking for a secret that is being shown to the person who owns it. + * + * The inspect screen exists to answer "did this template get the values I + * think it got", and a row of `Object (7 keys)` cannot answer it. Neither can + * a full reveal, which turns a screen someone leaves open during a screen + * share into a credential leak. What answers it is enough of the value to + * recognise which secret it is and to catch the two mistakes that actually + * happen — a stale value, and a key resolved from the wrong tier. + * + * How much is safe to show depends on how much there is. A four-character + * value has no middle to hide, so revealing its ends reveals the value; a + * forty-character token gives away nothing in six characters. The bands below + * reveal less as the value gets shorter, and stop revealing at all once a + * value is short enough that any window is most of it. + * + * The mask core is a fixed width on purpose. Sizing it to the value would + * publish the exact length of every secret on screen, which is a real + * narrowing hint against a value someone is trying to guess. Callers that + * want the length — inspect does, because "set but empty" and "set to the + * 8-character staging password" are different bugs — ask for it separately + * and render it as a number, where it reads as the diagnostic it is rather + * than as part of the value. + * + * @example + * maskSecret('hunter2'); // 'h*****2' + * maskSecret('postgres://user:pw@host/db'); // 'po*****t/db' + */ + +/** + * What stands in for the hidden middle, at every length that has one. + * + * Fixed rather than proportional so the rendering never encodes how long the + * value is. See the module note. + */ +const MASK_CORE = '*****'; + +/** + * Longest value that is shown as nothing but mask. + * + * At four characters a first-and-last window is half the value, which is not + * a mask. + */ +const OPAQUE_MAX = 4; + +/** Longest value that reveals only one character at each end. */ +const NARROW_MAX = 8; + +/** Longest value that reveals a suffix but no prefix. */ +const SUFFIX_ONLY_MAX = 12; + +/** What an empty value renders as, so it is not mistaken for an unset one. */ +const EMPTY_LABEL = '(empty)'; + +/** + * A secret rendered for display, revealing less the shorter it is. + * + * @example + * maskSecret(''); // '(empty)' + * maskSecret('abcd'); // '*****' + * maskSecret('abcdefgh'); // 'a*****h' + * maskSecret('abcdefghijkl'); // '*****ijkl' + * maskSecret('abcdefghijklm'); // 'ab*****jklm' + */ +export function maskSecret(value: string): string { + + // Code points, not `.length`. A `String.prototype.slice` offset counts + // UTF-16 code units, so a value ending in an emoji or any other non-BMP + // character gets sliced through the middle of a surrogate pair and the + // reveal renders as a replacement glyph — the one part of the value a + // reader is meant to recognise, corrupted. + const characters = [...value]; + const { length } = characters; + + const head = (count: number) => characters.slice(0, count).join(''); + const tail = (count: number) => characters.slice(-count).join(''); + + if (length === 0) return EMPTY_LABEL; + + if (length <= OPAQUE_MAX) return MASK_CORE; + + if (length <= NARROW_MAX) return `${head(1)}${MASK_CORE}${tail(1)}`; + + if (length <= SUFFIX_ONLY_MAX) return `${MASK_CORE}${tail(4)}`; + + return `${head(2)}${MASK_CORE}${tail(4)}`; + +} diff --git a/src/tui/components/index.ts b/src/tui/components/index.ts index f95669d6..b53a8b86 100644 --- a/src/tui/components/index.ts +++ b/src/tui/components/index.ts @@ -18,6 +18,7 @@ export { SelectList, SearchableList, ActionList, StatusList } from './lists/inde export type { SelectListProps, SelectListItem, + SelectListRowState, SearchableListProps, SearchableListFilterState, ActionListProps, @@ -87,8 +88,8 @@ export type { } from './secrets/index.js'; // Overlays -export { LogViewerOverlay } from './overlays/index.js'; -export type { LogViewerOverlayProps } from './overlays/index.js'; +export { LogViewerOverlay, TextOverlay } from './overlays/index.js'; +export type { LogViewerOverlayProps, TextOverlayProps } from './overlays/index.js'; // Terminal (SQL REPL) export { SqlInput, ResultTable } from './terminal/index.js'; diff --git a/src/tui/components/lists/SelectList.tsx b/src/tui/components/lists/SelectList.tsx index 5a139a88..25dfc73b 100644 --- a/src/tui/components/lists/SelectList.tsx +++ b/src/tui/components/lists/SelectList.tsx @@ -16,7 +16,7 @@ import { useState, useEffect, useMemo, useRef } from 'react'; import { Box, Text, useInput } from 'ink'; -import type { ReactElement } from 'react'; +import type { ReactElement, ReactNode } from 'react'; import { useFocusScope } from '../../focus.js'; import { useRowMouse } from '../../mouse.js'; @@ -112,6 +112,35 @@ export interface SelectListProps { /** Callback when Escape is pressed (for navigation) */ onCancel?: () => void; + + /** + * Draw the body of a row in place of the default icon/label/description. + * + * The list keeps the cursor, the window, the scroll indicators, focus and + * the mouse; this replaces only what one row *says*. A row whose meaning + * lives in per-segment colour — a green `[OK]` against a red `[ERR]`, a + * type badge — cannot be expressed as one `label` string, and the + * alternative was every such screen hand-rolling its own list and losing + * the windowing with it. + * + * Must draw exactly one row. The window arithmetic counts items, so a body + * that wraps or embeds a newline puts the fold out by that much: truncate + * (`wrap="truncate"`) or pre-wrap before returning. + */ + renderItem?: (item: SelectListItem, state: SelectListRowState) => ReactNode; +} + +/** + * What a row needs to know to draw itself. + */ +export interface SelectListRowState { + + /** The cursor is on this row. */ + isHighlighted: boolean; + + /** The list itself holds focus — a highlight is inert without it. */ + isFocused: boolean; + } /** @@ -137,6 +166,7 @@ export function SelectList({ onToggle, onSubmit, onCancel, + renderItem, }: SelectListProps): ReactElement { // Unconditional so the hook count is stable whether or not the caller @@ -500,18 +530,33 @@ export function SelectList({ {numberNav && ( {numberIndicator} )} - - {isHighlighted ? '❯ ' : ' '} - {item.icon ? `${item.icon} ` : ''} - {item.label} - {/* Inline description (default behavior) */} - {!showDescriptionBelow && item.description && ( - {item.description} - )} - + {/* The cursor stays with the list even under a custom + body, so every list in the app marks its selection + the same way and a caller cannot forget to. */} + {renderItem ? ( + <> + + {isHighlighted ? '❯ ' : ' '} + + {renderItem(item, { isHighlighted, isFocused })} + + ) : ( + + {isHighlighted ? '❯ ' : ' '} + {item.icon ? `${item.icon} ` : ''} + {item.label} + {/* Inline description (default behavior) */} + {!showDescriptionBelow && item.description && ( + {item.description} + )} + + )} {/* Description below (when showDescriptionBelow is true) */} {showDescriptionBelow && item.description && ( diff --git a/src/tui/components/lists/index.ts b/src/tui/components/lists/index.ts index ba901519..b5631bc9 100644 --- a/src/tui/components/lists/index.ts +++ b/src/tui/components/lists/index.ts @@ -6,7 +6,7 @@ export { SearchableList } from './SearchableList.js'; export { ActionList } from './ActionList.js'; export { StatusList } from './StatusList.js'; -export type { SelectListProps, SelectListItem } from './SelectList.js'; +export type { SelectListProps, SelectListItem, SelectListRowState } from './SelectList.js'; export type { SearchableListProps, SearchableListFilterState } from './SearchableList.js'; export type { ActionListProps, ActionItem } from './ActionList.js'; export type { StatusListProps, StatusListItem, StatusType } from './StatusList.js'; diff --git a/src/tui/components/overlays/TextOverlay.tsx b/src/tui/components/overlays/TextOverlay.tsx new file mode 100644 index 00000000..46e550d2 --- /dev/null +++ b/src/tui/components/overlays/TextOverlay.tsx @@ -0,0 +1,85 @@ +/** + * TextOverlay - a scrollable view of one block of text, at full length. + * + * Screens that list operations have to summarise: a history row is one line, and + * the detail box under it has a handful. A stack trace fits neither, so both + * history screens truncated one — at 80 characters in the list, and at nothing + * at all in the detail box, where `split('\n').map()` drew a row per line and + * pushed the rest of the screen off the bottom. Truncating loses the part of an + * error that says what went wrong, and not truncating loses the screen; this is + * the third option, and the reason a screen may show a bounded preview without + * that preview being all a reader can ever get. + * + * The text is wrapped here rather than by Ink, because a viewport counts rows + * and a `` left to wrap itself occupies however many the terminal decides. + * + * Focus follows the overlay pattern `LogViewerOverlay` established: its own + * `useFocusScope`, and Escape as the only way out. + * + * @example + * {showError && } + */ +import { Box, Text, useWindowSize, useInput } from 'ink'; + +import type { ReactElement } from 'react'; + +import { useFocusScope } from '../../focus.js'; +import { ScrollPane, rowBudget, wrapText } from '../terminal/index.js'; + +/** + * Props for the text overlay. + */ +export interface TextOverlayProps { + + /** Names what is being shown, since the overlay covers the screen that has the context. */ + title: string; + + /** The text, at full length. Newlines are honoured; long lines are wrapped, not cut. */ + text: string; + + /** Called when the reader dismisses the overlay. */ + onClose: () => void; + +} + +/** The title line, its rule, and the hint line under the body. */ +const CHROME_ROWS = 3; + +/** Rows the app shell and this overlay's own border spend before any text is drawn. */ +const SHELL_ROWS = 8; + +/** + * TextOverlay component. + */ +export function TextOverlay({ title, text, onClose }: TextOverlayProps): ReactElement { + + const { isFocused } = useFocusScope('TextOverlay'); + const { columns, rows } = useWindowSize(); + + const width = rowBudget(columns); + const height = Math.max(rows - SHELL_ROWS - CHROME_ROWS, 3); + + const lines = wrapText(text, width).map((line, index) => ( + {line} + )); + + // Escape only. Every other key belongs to the ScrollPane below, which reads + // the same focus value and so is live exactly while this is. + useInput((_input, key) => { + + if (!isFocused) return; + + if (key.escape) onClose(); + + }); + + return ( + + {title} + {'─'.repeat(width)} + + [↑↓] Scroll [Ctrl+U/D] Page [Esc] Back + + ); + +} diff --git a/src/tui/components/overlays/index.ts b/src/tui/components/overlays/index.ts index 4c7a0835..d6a747b4 100644 --- a/src/tui/components/overlays/index.ts +++ b/src/tui/components/overlays/index.ts @@ -5,3 +5,6 @@ */ export { LogViewerOverlay } from './LogViewerOverlay.js'; export type { LogViewerOverlayProps } from './LogViewerOverlay.js'; + +export { TextOverlay } from './TextOverlay.js'; +export type { TextOverlayProps } from './TextOverlay.js'; diff --git a/src/tui/components/terminal/RowViewOverlay.tsx b/src/tui/components/terminal/RowViewOverlay.tsx index ece7bb95..fa881cc5 100644 --- a/src/tui/components/terminal/RowViewOverlay.tsx +++ b/src/tui/components/terminal/RowViewOverlay.tsx @@ -41,6 +41,7 @@ import type { ReactElement } from 'react'; import type { RowFormat } from './rowDocument.js'; import { useFocusScope } from '../../focus.js'; +import { useWheelScroll } from '../../mouse.js'; import { preferredRowFormat, rememberRowFormat, renderRowDocument } from './rowDocument.js'; import { rowBudget, rowWindow, scrollTarget, wrapText } from './viewport.js'; @@ -118,6 +119,13 @@ export function RowViewOverlay({ const view = rowWindow(lines.length, offset, budget); const maxOffset = lines.length - view.count; + const scrollTo = (next: number) => setOffset(Math.min(Math.max(next, 0), maxOffset)); + + // Scrolls the document, not the row cursor: ←/→ change rows, and a wheel + // that jumped between rows would lose the reader's place in a long one. + // Inert without a MouseProvider above it or with the setting off. + useWheelScroll({ isActive: isFocused, onWheel: (delta) => scrollTo(view.start + delta) }); + const move = (next: number) => { if (next < 0 || next > rows.length - 1 || next === index) return; @@ -169,7 +177,7 @@ export function RowViewOverlay({ const target = scrollTarget(input, key, view, maxOffset); - if (target !== null) setOffset(Math.min(Math.max(target, 0), maxOffset)); + if (target !== null) scrollTo(target); }); diff --git a/src/tui/components/terminal/ScrollPane.tsx b/src/tui/components/terminal/ScrollPane.tsx new file mode 100644 index 00000000..b6c99da9 --- /dev/null +++ b/src/tui/components/terminal/ScrollPane.tsx @@ -0,0 +1,93 @@ +/** + * ScrollPane - a vertical viewport over a flat list of pre-laid-out lines. + * + * Ink has no scroll offset. A screen that renders more rows than the terminal + * holds does not clip them, it pushes its own footer off the bottom, so the + * content past the fold is not merely unscrolled — it is unreachable. Every + * screen that can overflow therefore has to flatten itself to one element per + * visual line and draw a slice of that list, which is what this does. + * + * It is the plain form of the pattern: offset state, the shared window + * arithmetic, the shared scroll keys, and the two "more above / more below" + * indicators. `ExploreDetailScreen`'s `ScrollView` is the same viewport with a + * full-text overlay and a row peek switched in over the top of it, and it stays + * where it is — its overlays are built from explore's own layout module, so + * pulling it down here would drag a screen's vocabulary into a shared + * component for no gain. Anything that needs a viewport and not those overlays + * uses this. + * + * Takes `height` and `isFocused` as props rather than measuring or scoping for + * itself, so the screen stays the single place that accounts for chrome, and so + * a test can pin a viewport without a terminal to measure. + * + * @example + * + */ +import { useState } from 'react'; +import { Box, Text, useInput } from 'ink'; + +import type { ReactElement } from 'react'; + +import { useWheelScroll } from '../../mouse.js'; +import { rowWindow, scrollTarget } from './viewport.js'; + +/** + * Props for the scroll pane. + */ +export interface ScrollPaneProps { + + /** + * One element per visual line, each carrying its own `key`. + * + * One *visual* line: an element that wraps to two rows makes the window + * arithmetic wrong by one, so callers wrap their own text (`wrapText`) or + * truncate it (`wrap="truncate"`) before handing it over. + */ + lines: ReactElement[]; + + /** Rows the viewport may draw, indicators included. */ + height: number; + + /** Focus comes from the screen; this component opens no scope of its own. */ + isFocused: boolean; + +} + +/** + * ScrollPane component. + */ +export function ScrollPane({ lines, height, isFocused }: ScrollPaneProps): ReactElement { + + const [offset, setOffset] = useState(0); + + const view = rowWindow(lines.length, offset, height); + const maxOffset = lines.length - view.count; + + const scrollTo = (next: number) => setOffset(Math.min(Math.max(next, 0), maxOffset)); + + // Inert without a MouseProvider above it or with the setting off. + useWheelScroll({ isActive: isFocused, onWheel: (delta) => scrollTo(view.start + delta) }); + + useInput((input, key) => { + + if (!isFocused) return; + + // Rebases on `view.start` rather than on `offset`: the window clamps + // what it draws, so a stale offset left by a resize or by shorter + // content cannot send the next keypress somewhere the viewport never + // was. + const target = scrollTarget(input, key, view, maxOffset); + + if (target !== null) scrollTo(target); + + }); + + return ( + + {view.above > 0 && ↑ {view.above} more} + {lines.slice(view.start, view.start + view.count)} + {view.below > 0 && ↓ {view.below} more} + + ); + +} diff --git a/src/tui/components/terminal/index.ts b/src/tui/components/terminal/index.ts index 05cd92fe..16c33a06 100644 --- a/src/tui/components/terminal/index.ts +++ b/src/tui/components/terminal/index.ts @@ -17,6 +17,9 @@ export type { ResultBrowserProps } from './ResultBrowser.js'; export { RowViewOverlay } from './RowViewOverlay.js'; export type { RowViewOverlayProps } from './RowViewOverlay.js'; +export { ScrollPane } from './ScrollPane.js'; +export type { ScrollPaneProps } from './ScrollPane.js'; + export { fitGridColumns, fitPeekColumns, PEEK_COLUMN_CAP } from './columnFit.js'; export type { GridColumnFit, PeekColumnFit } from './columnFit.js'; diff --git a/src/tui/mouse.tsx b/src/tui/mouse.tsx index f88e7b12..94282eb9 100644 --- a/src/tui/mouse.tsx +++ b/src/tui/mouse.tsx @@ -435,6 +435,72 @@ export function useMouseTransport(): MouseTransport { } +/** + * Options for useWheelScroll. + */ +export interface WheelScrollOptions { + + /** + * Whether the component owning the viewport currently has input. + * + * The same guard its `useInput` handler uses: a wheel notch acts on + * whatever already has focus, so a pane sitting behind an overlay must not + * move under it. + */ + isActive: boolean; + + /** A wheel notch: -1 for up, 1 for down. */ + onWheel: (delta: -1 | 1) => void; + +} + +/** + * Wheel notches for a viewport that scrolls but has no rows to click. + * + * `useRowMouse` already carries wheel handling, but it is built around + * hit-testing a list of registered row refs, and a viewport has none — it draws + * a slice of a flat line list where nothing is selectable. Subscribing for the + * wheel alone is the whole of what those panes need. + * + * Without this the wheel is not merely unhandled, it is broken: the TUI runs in + * the alternate screen, which has no scrollback of its own, and mouse tracking + * takes the notches that a terminal would otherwise translate into arrow keys. + * So turning the mouse on removes the only wheel behaviour a viewport had. + * + * @example + * useWheelScroll({ isActive: isFocused, onWheel: (delta) => scrollTo(view.start + delta) }); + */ +export function useWheelScroll({ isActive, onWheel }: WheelScrollOptions): void { + + const { enabled, subscribe } = useMouseTransport(); + + // Registered once, reading the latest props through this ref. Listing them + // as dependencies would resubscribe on every render, since every caller + // passes an inline arrow. + const latest = useRef({ isActive, onWheel }); + + latest.current = { isActive, onWheel }; + + useEffect(() => { + + if (!enabled) return; + + return subscribe((event) => { + + const current = latest.current; + + if (!current.isActive || event.kind !== 'press') return; + + if (event.button === 'wheel-up') current.onWheel(-1); + + else if (event.button === 'wheel-down') current.onWheel(1); + + }); + + }, [enabled, subscribe]); + +} + /** * Options for useRowMouse. */ diff --git a/src/tui/screens/change/ChangeHistoryDetailScreen.tsx b/src/tui/screens/change/ChangeHistoryDetailScreen.tsx index c74d10a3..40d45dd0 100644 --- a/src/tui/screens/change/ChangeHistoryDetailScreen.tsx +++ b/src/tui/screens/change/ChangeHistoryDetailScreen.tsx @@ -4,26 +4,49 @@ * Shows all files executed as part of a build, run, or change * with their individual status, duration, and errors. * + * A build runs however many files the project has, so this is the screen most + * likely to hold more rows than the terminal. It drew `files.slice(0, 20)` with + * the cursor ranging over all of them, and rendered a failed file's error as one + * `` per line of the message — so a stack trace pushed the file list, the + * detail box and the hints off the bottom together. The list is now a + * `SelectList`, which windows around its own cursor, and the error is a bounded + * line here with the full text an Enter away in a `TextOverlay`. + * * @example * ```bash * # Navigate from ChangeHistoryScreen by pressing Enter * ``` */ -import { useState, useMemo } from 'react'; +import { useState } from 'react'; import { Box, Text, useInput } from 'ink'; import type { ReactElement } from 'react'; import type { ScreenProps } from '../../types.js'; +import type { SelectListItem, SelectListRowState } from '../../components/index.js'; import type { FileHistoryRecord } from '../../../core/change/types.js'; import { attempt } from '@logosdx/utils'; import { useRouter } from '../../router.js'; import { useFocusScope } from '../../focus.js'; import { useAppContext } from '../../app-context.js'; -import { Panel, Spinner } from '../../components/index.js'; +import { Panel, Spinner, SelectList, TextOverlay } from '../../components/index.js'; import { useConnection, useAsyncEffect } from '../../hooks/index.js'; import { ChangeHistory } from '../../../core/change/history.js'; -import { getErrorMessage } from '../../utils/index.js'; +import { getErrorMessage, oneLine } from '../../utils/index.js'; + +/** + * Rows this screen spends inside its Panel on everything that is not the list. + * + * The statistics line and the gap under it, the list's own top margin, the gap + * above the detail box, the box itself — two borders around three content lines + * plus its top margin — and the gap before the hotkey hints, which are + * themselves already counted by `SCREEN_CHROME_ROWS`. + * + * Three content lines whatever the record: the box used to grow a row per line + * of a failed file's error message, which is exactly how an unlucky stack trace + * took the whole screen with it. + */ +const CHROME_ROWS = 11; /** * Get status indicator for a file execution. @@ -58,6 +81,37 @@ function getFilename(filepath: string): string { } +/** + * One file row: status, filename, duration, skip reason. + * + * Drawn by the caller rather than handed over as a label because a reader scans + * this list for the red row among the green, and a `label` string carries one + * colour for the whole line. + */ +function fileRow(file: FileHistoryRecord, state: SelectListRowState): ReactElement { + + const statusIndicator = getStatusIndicator(file.status); + const duration = file.durationMs ? `(${(file.durationMs / 1000).toFixed(1)}s)` : ''; + + return ( + <> + {statusIndicator.icon} + + {getFilename(file.filepath)} + + + {' '}{duration} + {file.status === 'skipped' && file.skipReason ? ` - ${oneLine(file.skipReason)}` : ''} + + + ); + +} + /** * ChangeHistoryDetailScreen component. */ @@ -73,7 +127,8 @@ export function ChangeHistoryDetailScreen({ params }: ScreenProps): ReactElement const [files, setFiles] = useState([]); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); - const [selectedIndex, setSelectedIndex] = useState(0); + const [selectedFile, setSelectedFile] = useState(null); + const [showError, setShowError] = useState(false); // Shared connection const { db, dialect, loading: connLoading, error: connError } = useConnection(); @@ -117,40 +172,26 @@ export function ChangeHistoryDetailScreen({ params }: ScreenProps): ReactElement }, [db, activeConfigName, loadingStatus, operationId]); - // Get selected file - const selectedFile = useMemo(() => { - - return files[selectedIndex]; - - }, [files, selectedIndex]); + const failureText = selectedFile?.errorMessage ?? ''; - // Keyboard handling + // Arrows belong to the SelectList below. Enter opens the error because a + // file execution has nowhere further to drill into, so the only thing left + // to ask of a row is what went wrong with it. useInput((_input, key) => { if (!isFocused) return; - // Navigation - if (key.upArrow) { - - setSelectedIndex((prev) => Math.max(0, prev - 1)); - - return; - - } - - if (key.downArrow) { + if (key.escape) { - setSelectedIndex((prev) => Math.min(files.length - 1, prev + 1)); + back(); return; } - if (key.escape) { + if (key.return && failureText) { - back(); - - return; + setShowError(true); } @@ -203,11 +244,29 @@ export function ChangeHistoryDetailScreen({ params }: ScreenProps): ReactElement } + if (showError && failureText) { + + return ( + setShowError(false)} + /> + ); + + } + // Statistics const totalSuccess = files.filter((f) => f.status === 'success').length; const totalFailed = files.filter((f) => f.status === 'failed').length; const totalSkipped = files.filter((f) => f.status === 'skipped').length; + const items: SelectListItem[] = files.map((file) => ({ + key: String(file.id), + label: getFilename(file.filepath), + value: file, + })); + return ( @@ -232,52 +291,22 @@ export function ChangeHistoryDetailScreen({ params }: ScreenProps): ReactElement {/* File List */} - {files.length === 0 ? ( - - No file executions recorded. - - ) : ( - - {files.slice(0, 20).map((file, index) => { - - const isSelected = index === selectedIndex; - const statusIndicator = getStatusIndicator(file.status); - const duration = file.durationMs - ? `(${(file.durationMs / 1000).toFixed(1)}s)` - : ''; - - return ( - - - {isSelected ? '>' : ' '} - - - {' '} - {statusIndicator.icon}{' '} - - - {getFilename(file.filepath)} - - {duration} - {file.status === 'skipped' && file.skipReason && ( - - {file.skipReason} - )} - - ); - - })} - {files.length > 20 && ( - - ...and {files.length - 20} more - - )} - - )} + + fileRow(item.value, state)} + onHighlight={(item) => setSelectedFile(item.value)} + /> + - {/* Selected file details */} + {/* Selected file details, fixed at three lines - see CHROME_ROWS */} {selectedFile && ( - File Details - Path: {selectedFile.filepath} - + Path: {selectedFile.filepath} + Checksum: {selectedFile.checksum.slice(0, 16)}... - {selectedFile.status === 'skipped' && selectedFile.skipReason && ( - Skip Reason: {selectedFile.skipReason} - )} - {selectedFile.status === 'failed' && selectedFile.errorMessage && ( - - Error: - - {selectedFile.errorMessage.split('\n').map((line, i) => ( - - {line} - - ))} - - + {failureText ? ( + {oneLine(failureText)} + ) : selectedFile.status === 'skipped' && selectedFile.skipReason ? ( + + Skip Reason: {oneLine(selectedFile.skipReason)} + + ) : ( + No errors recorded for this file. )} )} {/* Keyboard hints */} + [↑↓] Navigate + {failureText && [Enter] Full Error} [Esc] Back diff --git a/src/tui/screens/change/ChangeHistoryScreen.tsx b/src/tui/screens/change/ChangeHistoryScreen.tsx index bb6a188c..aa22aa14 100644 --- a/src/tui/screens/change/ChangeHistoryScreen.tsx +++ b/src/tui/screens/change/ChangeHistoryScreen.tsx @@ -4,16 +4,24 @@ * Shows unified history of changes, builds, and runs with drill-down * to file execution details. * + * The list is a `SelectList` rather than a hand-rolled column of rows. The + * hand-rolled one drew `history.slice(0, 15)` while the cursor ranged over the + * whole result, so past the fifteenth record the selection moved somewhere the + * reader could not see, and the detail box below described a row that was not + * on screen. `SelectList` owns the cursor and the window together, which is the + * only arrangement where those two cannot disagree. + * * @example * ```bash * noorm change history # View execution history * ``` */ -import { useState, useMemo } from 'react'; +import { useState } from 'react'; import { Box, Text, useInput } from 'ink'; import type { ReactElement } from 'react'; import type { ScreenProps } from '../../types.js'; +import type { SelectListItem, SelectListRowState } from '../../components/index.js'; import type { UnifiedHistoryRecord } from '../../../core/change/types.js'; import { attempt } from '@logosdx/utils'; @@ -21,11 +29,35 @@ import { attempt } from '@logosdx/utils'; import { useRouter } from '../../router.js'; import { useFocusScope } from '../../focus.js'; import { useAppContext } from '../../app-context.js'; -import { Panel, Spinner } from '../../components/index.js'; +import { Panel, Spinner, SelectList, TextOverlay } from '../../components/index.js'; import { useConnection, useAsyncEffect } from '../../hooks/index.js'; import { ChangeHistory } from '../../../core/change/history.js'; import { relativeTimeAgo } from '../../utils/date.js'; -import { getErrorMessage } from '../../utils/index.js'; +import { getErrorMessage, oneLine } from '../../utils/index.js'; + +/** + * Records fetched for the list. + * + * Raised from 50 once the list could actually reach past its fifteenth row. + * The query is a `LIMIT` on an indexed history table returning small rows, so + * the ceiling is about what a reader can navigate, not about cost. + */ +const HISTORY_LIMIT = 200; + +/** + * Rows this screen spends inside its Panel on everything that is not the list. + * + * The statistics line and the gap under it, the list's own top margin, the gap + * above the detail box, the box itself — two borders around three content lines + * plus its top margin — and the gap before the hotkey hints. The hints are + * already counted by `SCREEN_CHROME_ROWS`. + * + * The detail box draws exactly three lines whatever the record, rather than + * growing one when there is an error to show, so this number stays true: a box + * that sometimes takes a fourth row pushes the last list row under the status + * bar, which is the failure this screen was rebuilt to remove. + */ +const CHROME_ROWS = 11; /** * Get type indicator for display. @@ -48,6 +80,37 @@ function getTypeIndicator(changeType: string): { label: string; color: string } } +/** + * One history row: status, type, name, age, duration. + * + * Drawn by the caller rather than handed to `SelectList` as a label because the + * status colour is what a reader scans the list for — a red `[ERR]` in a column + * of green is the whole point of the screen, and a single `label` string can + * only be one colour. + */ +function historyRow(record: UnifiedHistoryRecord, state: SelectListRowState): ReactElement { + + const typeIndicator = getTypeIndicator(record.changeType); + const statusOk = record.status === 'success'; + const duration = record.durationMs ? `(${(record.durationMs / 1000).toFixed(1)}s)` : ''; + + return ( + <> + {statusOk ? '[OK]' : '[ERR]'} + {typeIndicator.label} + + {oneLine(record.name)} + + {relativeTimeAgo(record.executedAt)} {duration} + + ); + +} + /** * ChangeHistoryScreen component. */ @@ -60,7 +123,8 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem const [history, setHistory] = useState([]); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); - const [selectedIndex, setSelectedIndex] = useState(0); + const [selectedRecord, setSelectedRecord] = useState(null); + const [showError, setShowError] = useState(false); // Shared connection const { db, dialect, loading: connLoading, error: connError } = useConnection(); @@ -82,7 +146,7 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem const [_, err] = await attempt(async () => { const changeHistory = new ChangeHistory(db, activeConfigName ?? '', dialect ?? 'postgres'); - const records = await changeHistory.getUnifiedHistory(undefined, 50); + const records = await changeHistory.getUnifiedHistory(undefined, HISTORY_LIMIT); if (isCancelled()) return; @@ -104,35 +168,15 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem }, [db, activeConfigName, loadingStatus]); - // Get selected record - const selectedRecord = useMemo(() => { - - return history[selectedIndex]; + const failureText = selectedRecord?.errorMessage ?? ''; - }, [history, selectedIndex]); - - // Keyboard handling + // Arrows and Enter belong to the SelectList below; this handles only what + // the screen itself owns. While the overlay is mounted its own focus scope + // sits on top, so `isFocused` is false here and both go quiet together. useInput((input, key) => { if (!isFocused) return; - // Navigation - if (key.upArrow) { - - setSelectedIndex((prev) => Math.max(0, prev - 1)); - - return; - - } - - if (key.downArrow) { - - setSelectedIndex((prev) => Math.min(history.length - 1, prev + 1)); - - return; - - } - if (key.escape) { back(); @@ -141,15 +185,9 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem } - // Enter - view file details - if (key.return && selectedRecord) { + if (input === 'e' && !key.ctrl && !key.meta && failureText) { - navigate('change/history/detail', { - operationId: selectedRecord.id, - name: selectedRecord.name, - }); - - return; + setShowError(true); } @@ -191,6 +229,18 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem } + if (showError && failureText) { + + return ( + setShowError(false)} + /> + ); + + } + // Statistics const totalChanges = history.filter((r) => r.changeType === 'change').length; const totalBuilds = history.filter((r) => r.changeType === 'build').length; @@ -198,6 +248,12 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem const totalSuccess = history.filter((r) => r.status === 'success').length; const totalFailed = history.filter((r) => r.status === 'failed').length; + const items: SelectListItem[] = history.map((record) => ({ + key: String(record.id), + label: record.name, + value: record, + })); + return ( @@ -226,53 +282,27 @@ export function ChangeHistoryScreen({ params: _params }: ScreenProps): ReactElem {/* History List */} - {history.length === 0 ? ( - - No execution history found. - - ) : ( - - {history.slice(0, 15).map((record, index) => { - - const isSelected = index === selectedIndex; - const typeIndicator = getTypeIndicator(record.changeType); - const statusOk = record.status === 'success'; - const statusColor = statusOk ? 'green' : 'red'; - const statusIcon = statusOk ? '[OK]' : '[ERR]'; - const duration = record.durationMs - ? `(${(record.durationMs / 1000).toFixed(1)}s)` - : ''; - - return ( - - - {isSelected ? '>' : ' '} - - {statusIcon} - {typeIndicator.label} - - {record.name} - - - {' '} - {relativeTimeAgo(record.executedAt)} {duration} - - - ); - + + historyRow(item.value, state)} + onHighlight={(item) => setSelectedRecord(item.value)} + onSelect={(item) => navigate('change/history/detail', { + operationId: item.value.id, + name: item.value.name, })} - {history.length > 15 && ( - - ...and {history.length - 15} more - - )} - - )} + /> + - {/* Selected record details */} + {/* Selected record details, fixed at three lines - see CHROME_ROWS */} {selectedRecord && ( - {selectedRecord.name} - - - By: {selectedRecord.executedBy} - - - Duration: {(selectedRecord.durationMs / 1000).toFixed(2)}s - - - {selectedRecord.status === 'failed' && selectedRecord.errorMessage && ( - - Error: {selectedRecord.errorMessage.slice(0, 80)} - {selectedRecord.errorMessage.length > 80 ? '...' : ''} + {oneLine(selectedRecord.name)} + + By: {selectedRecord.executedBy} Duration: {(selectedRecord.durationMs / 1000).toFixed(2)}s + + {failureText ? ( + + {oneLine(failureText)} + ) : ( + Press Enter to view file details )} - Press Enter to view file details )} {/* Keyboard hints */} + [↑↓] Navigate [Enter] View Files + {failureText && [e] Full Error} [Esc] Back diff --git a/src/tui/screens/db/explore/ExploreDetailScreen.tsx b/src/tui/screens/db/explore/ExploreDetailScreen.tsx index a387b49d..a7ef498f 100644 --- a/src/tui/screens/db/explore/ExploreDetailScreen.tsx +++ b/src/tui/screens/db/explore/ExploreDetailScreen.tsx @@ -34,6 +34,7 @@ import type { ScreenProps } from '../../../types.js'; import { useRouter } from '../../../router.js'; import { useFocusScope } from '../../../focus.js'; +import { useWheelScroll } from '../../../mouse.js'; import { useAppContext } from '../../../app-context.js'; import { Panel, Spinner } from '../../../components/index.js'; import { useConnection, useAsyncEffect } from '../../../hooks/index.js'; @@ -614,6 +615,13 @@ export function ScrollView({ // cannot send the next keypress somewhere the viewport never was. const scrollTo = (next: number) => setOffset(Math.min(Math.max(next, 0), maxOffset)); + // Only while this component owns the viewport: an overlay draws over it and + // scrolls itself, so the pane underneath must not move under the notch. + useWheelScroll({ + isActive: isFocused && overlay === 'none', + onWheel: (delta) => scrollTo(view.start + delta), + }); + const open = (next: DetailOverlay) => { setOverlay(next); diff --git a/src/tui/screens/db/explore/FullTextOverlay.tsx b/src/tui/screens/db/explore/FullTextOverlay.tsx index 65845f2a..42d4d19f 100644 --- a/src/tui/screens/db/explore/FullTextOverlay.tsx +++ b/src/tui/screens/db/explore/FullTextOverlay.tsx @@ -28,6 +28,7 @@ import { Box, Text, useInput, useWindowSize } from 'ink'; import type { ReactElement } from 'react'; import { useFocusScope } from '../../../focus.js'; +import { useWheelScroll } from '../../../mouse.js'; import { rowBudget, rowWindow, scrollTarget, wrapText } from './layout.js'; /** @@ -96,6 +97,11 @@ export function FullTextOverlay({ text, startRow, height, onClose }: FullTextOve const view = rowWindow(lines.length, offset, budget); const maxOffset = lines.length - view.count; + const scrollTo = (next: number) => setOffset(Math.min(Math.max(next, 0), maxOffset)); + + // Inert without a MouseProvider above it or with the setting off. + useWheelScroll({ isActive: isFocused, onWheel: (delta) => scrollTo(view.start + delta) }); + useInput((input, key) => { if (!isFocused) return; @@ -110,7 +116,7 @@ export function FullTextOverlay({ text, startRow, height, onClose }: FullTextOve const target = scrollTarget(input, key, view, maxOffset); - if (target !== null) setOffset(Math.min(Math.max(target, 0), maxOffset)); + if (target !== null) scrollTo(target); }); diff --git a/src/tui/screens/run/RunInspectScreen.tsx b/src/tui/screens/run/RunInspectScreen.tsx index 36eef206..8481b617 100644 --- a/src/tui/screens/run/RunInspectScreen.tsx +++ b/src/tui/screens/run/RunInspectScreen.tsx @@ -10,8 +10,8 @@ * noorm run inspect sql/users/001_create.sql.tmpl # With pre-filled path * ``` */ -import { useState, useCallback, useEffect } from 'react'; -import { Box, Text, useInput } from 'ink'; +import { useState, useCallback, useEffect, useMemo } from 'react'; +import { Box, Text, useInput, useWindowSize } from 'ink'; import { join, relative } from 'path'; import type { ReactElement } from 'react'; @@ -22,7 +22,10 @@ import { useRouter } from '../../router.js'; import { useFocusScope } from '../../focus.js'; import { useSettings, useAppContext } from '../../app-context.js'; import { Panel, Spinner, SearchableList } from '../../components/index.js'; -import { useAsyncEffect, useConnection } from '../../hooks/index.js'; +import { ScrollPane, rowBudget, wrapText } from '../../components/terminal/index.js'; +import { useAsyncEffect, useConnection, viewportRows } from '../../hooks/index.js'; +import { oneLine } from '../../utils/index.js'; +import { maskSecret } from '../../../core/shared/index.js'; import { discoverFiles } from '../../../core/runner/index.js'; import { buildContext } from '../../../core/template/context.js'; import { processFile } from '../../../core/template/engine.js'; @@ -43,7 +46,27 @@ type Phase = 'loading' | 'picker' | 'inspecting' | 'expanded' | 'preview' | 'err const BUILTIN_HELPERS = new Set(['quote', 'escape', 'uuid', 'now', 'json', 'include']); const STANDARD_KEYS = new Set(['config', 'secrets', 'globalSecrets', 'env']); -interface CategorizedContext { +/** + * Rows a scrolling phase spends inside its Panel before the pane starts. + * + * The `File:` line and the gap under it. The file name stays out of the pane on + * purpose: it is the one thing a reader needs at every scroll position, and a + * heading that scrolls away is a heading that is missing when it is wanted. + */ +const HEADER_ROWS = 2; + +/** + * A template context split into the groups the screen draws as sections. + * + * Exported because the line builders below are unit-tested directly, the way + * `ExploreDetailScreen`'s row builders are — rendering the whole screen to + * assert on a mask would mean standing up a project, a config and a connection + * for a pure function. + * + * @example + * const lines = contextLines(categorizeContext(ctx, helperKeys, []), projectRoot, 96); + */ +export interface CategorizedContext { dataFiles: Array<{ key: string; value: unknown }>; helpers: Array<{ key: string; value: unknown }>; helperErrors: HelperLoadError[]; @@ -276,42 +299,227 @@ async function resolveRenderSecrets( } +/** Widest the name column grows before it truncates. */ +const NAME_CAP = 30; + +/** Narrowest the name column shrinks to, however little the terminal offers. */ +const NAME_MIN = 12; + +/** Left inset every entry under a section heading shares. */ +const ENTRY_INDENT = 2; + /** - * Component that handles keyboard input for a specific focus scope. + * Width of the name column, derived from what this context actually holds. + * + * Same idiom as the explore rows and the Form label gutter: size once from the + * content, cap it, truncate past the cap. A context of short names does not pay + * for the one environment variable with a sixty-character name. */ -function KeyHandler({ - focusLabel, - onEscape, - onKey, -}: { - focusLabel: string; - onEscape?: () => void; - onKey?: (input: string, key: { escape: boolean }) => void; -}): null { - - const { isFocused } = useFocusScope(focusLabel); +function nameColumnWidth(names: string[], budget: number): number { - useInput((input, key) => { + let widest = 0; - if (!isFocused) return; + for (const name of names) { - if (key.escape && onEscape) { + if (name.length > widest) widest = name.length; - onEscape(); + } - return; + return Math.max(NAME_MIN, Math.min(widest, NAME_CAP, budget - ENTRY_INDENT - NAME_MIN)); - } +} - if (onKey) { +/** + * One `name detail` line, exactly one row tall. + * + * `flexShrink={0}` on the name cell because Ink's `width` is a flex basis and + * flex items shrink by default: without it a long detail squeezes the name on + * that row alone, and the column wanders down the page. Both cells truncate + * rather than wrap, which bounds their width; `oneLine` is what bounds their + * height. + */ +function entryRow(key: string, name: string, color: string, detail: string, width: number): ReactElement { - onKey(input, key); + return ( + + + {oneLine(name)} + + {oneLine(detail)} + + ); - } +} + +/** + * A heading, its entries, and the blank line under them. + * + * An empty section contributes nothing rather than a bare heading, so a project + * with no data files does not scroll past a promise of some. + */ +function sectionLines(key: string, title: string, rows: ReactElement[]): ReactElement[] { + + if (rows.length === 0) return []; + + return [ + {title}, + ...rows, + , + ]; + +} + +/** + * Secret keys, each with as much of its value as is safe to show. + * + * A count answers "is anything there". The question this screen is actually + * asked is "did this template get the value I think it got", and only the value + * answers that — a stale password and a fresh one are both `Object (7 keys)`. + * How much of it is safe to show is `maskSecret`'s decision, not this + * component's; see `core/shared/mask.ts`. The length rides alongside as a + * number rather than as mask width so that "set but empty" and "set to the + * wrong 8-character value" stay distinguishable without the asterisks + * themselves leaking anything. + */ +function secretRows(prefix: string, values: Record, color: string, width: number): ReactElement[] { + + return Object.keys(values).sort().map((key) => { + + const value = values[key] ?? ''; + + // Code points, matching how `maskSecret` counts. Reporting UTF-16 units + // beside a mask banded on characters would call the same value two + // different lengths. + const count = [...value].length; + + return entryRow(`${prefix}:${key}`, key, color, `${maskSecret(value)} (${count} chars)`, width); }); - return null; +} + +/** + * The summary view as one element per visual line. + * + * Flattened rather than nested because Ink has no scroll offset: the only way + * to reach content past the bottom of the terminal is to draw a slice of a flat + * list, and a tree cannot be sliced. + * + * `$.env` is listed and masked like the other two secret tiers. It is the whole + * of `process.env` (`core/template/context.ts`), which on a developer's machine + * routinely carries tokens that never went near noorm's vault, and nothing here + * can tell which of its keys those are. Masking every value is the answer that + * is wrong in the harmless direction. + * + * @example + * + */ +export function contextLines(context: CategorizedContext, projectRoot: string, budget: number): ReactElement[] { + + const envKeys = Object.keys(context.env); + const width = nameColumnWidth( + [ + ...context.dataFiles.map(({ key }) => `$.${key}`), + ...context.helpers.map(({ key }) => `$.${key}`), + ...context.builtins.map(({ key }) => `$.${key}`), + ...Object.keys(context.secrets), + ...Object.keys(context.globalSecrets), + ...envKeys, + '$.config', + ], + budget, + ); + + // A helper error gets the whole row rather than the two-column treatment. + // The name column is sized from the `$.name` entries beside it, which are + // short, and a path is the one thing this row exists to say — put it in + // that column and `sql/helpers/slug.js` renders as `sql/helpers…`, naming + // no file at all. + const helperEntries = [ + ...context.helpers.map(({ key, value }) => + entryRow(`helper:${key}`, `$.${key}`, 'magenta', describeType(value), width)), + ...context.helperErrors.map(({ filepath, error }) => ( + + {oneLine(`${' '.repeat(ENTRY_INDENT)}${relative(projectRoot, filepath)} — ${error.message}`)} + + )), + ]; + + return [ + ...sectionLines('data', 'Data Files', context.dataFiles.map(({ key, value }) => + entryRow(`data:${key}`, `$.${key}`, 'green', describeType(value), width))), + ...sectionLines('helpers', 'Helpers ($helpers)', helperEntries), + ...sectionLines('builtins', 'Built-ins', context.builtins.map(({ key }) => + entryRow(`builtin:${key}`, `$.${key}`, 'blue', 'Function', width))), + ...sectionLines('config', 'Config', [ + entryRow('config', '$.config', 'yellow', context.config ? describeType(context.config) : '(not set)', width), + ]), + ...sectionLines( + 'secrets', + `Secrets ($.secrets — ${Object.keys(context.secrets).length})`, + secretRows('secret', context.secrets, 'red', width), + ), + ...sectionLines( + 'globalSecrets', + `Global Secrets ($.globalSecrets — ${Object.keys(context.globalSecrets).length})`, + secretRows('globalSecret', context.globalSecrets, 'red', width), + ), + ...sectionLines( + 'env', + `Environment ($.env — ${envKeys.length})`, + secretRows('env', context.env, 'gray', width), + ), + ]; + +} + +/** + * Plain text as one element per visual line. + * + * Wrapped here rather than left to Ink because a `` that wraps itself + * occupies however many rows the terminal decides, and the viewport has to know + * the count before Ink lays it out. + */ +function textLines(key: string, text: string, budget: number, style: { color?: string; dim?: boolean } = {}): ReactElement[] { + + return wrapText(text, budget).map((line, index) => ( + {line} + )); + +} + +/** + * The expanded view as one element per visual line. + * + * Reports shapes rather than values, so what can overflow a row here is a long + * key or a wide shape summary, and both are wrapped to the budget rather than + * truncated — the expanded view exists to show what a summary cut. + * + * @example + * + */ +export function expandedLines(context: CategorizedContext, budget: number): ReactElement[] { + + const lines: ReactElement[] = []; + + for (const { key, value } of context.dataFiles) { + + lines.push({oneLine(`$.${key}`)}); + lines.push(...describeTypeExpanded(value, 1).flatMap((line, index) => + textLines(`exp:${key}:${index}`, line, budget, { dim: true }))); + lines.push( ); + + } + + if (context.config !== undefined && context.config !== null) { + + lines.push($.config); + lines.push(...describeTypeExpanded(context.config, 1).flatMap((line, index) => + textLines(`exp:config:${index}`, line, budget, { dim: true }))); + + } + + return lines; } @@ -325,6 +533,12 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { const { settings } = useSettings(); const { db, dialect } = useConnection(); + // useWindowSize, not useStdout: stdout.columns and .rows mutate on resize + // without telling React, so anything derived from them would freeze at + // mount size. Above the early returns, or the hook count changes once the + // load resolves. + const { columns: terminalColumns, rows: terminalRows } = useWindowSize(); + const [phase, setPhase] = useState('loading'); const [allFiles, setAllFiles] = useState([]); const [selectedFile, setSelectedFile] = useState(params.path ?? null); @@ -333,6 +547,18 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { const [renderDuration, setRenderDuration] = useState(null); const [error, setError] = useState(null); + // One scope for the whole screen rather than one per phase, because the + // scroll pane and the action keys have to agree on who is focused and two + // scopes cannot: React runs a child's effects before its parent's, so a + // screen-level push lands *above* its own child's and takes the keys the + // child was mounted to receive. `skip` is how a screen that sometimes hosts + // a focusable child stays out of the stack while that child is up — here, + // the file picker's `SearchableList`. + const { isFocused } = useFocusScope({ + label: 'RunInspect', + skip: phase === 'picker' && allFiles.length > 0, + }); + const projectRoot = process.cwd(); // Load template files on mount @@ -511,42 +737,79 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { }); - // Handlers for inspecting phase - const handleInspectKey = useCallback((input: string) => { + const handleInspectEscape = useCallback(() => { - if (input === 'e') { + setSelectedFile(null); + setContext(null); + setError(null); + setPhase('picker'); - setPhase('expanded'); + }, []); - } - else if (input === 'p') { + const displayPath = selectedFile ? relative(projectRoot, selectedFile) : ''; - handlePreview(); + const paneHeight = viewportRows(terminalRows, HEADER_ROWS); + const budget = rowBudget(terminalColumns); + + const summaryLines = useMemo( + () => (context ? contextLines(context, projectRoot, budget) : []), + [context, projectRoot, budget], + ); + + const detailLines = useMemo( + () => (context ? expandedLines(context, budget) : []), + [context, budget], + ); + + // The render error and the rendered SQL share the pane, because they are + // the same thing to a reader: what came back from asking for this template. + // A stack trace overflows a terminal as readily as a schema does. + const previewLines = useMemo( + () => (error !== null + ? textLines('previewError', error, budget, { color: 'red' }) + : textLines('preview', renderedSql ?? '', budget)), + [error, renderedSql, budget], + ); + + const errorLines = useMemo( + () => textLines('error', error ?? 'Unknown error', budget, { dim: true }), + [error, budget], + ); - } - else if (input === 'r') { + useInput((input, key) => { - handleRefresh(); + if (!isFocused) return; - } + if (key.escape) { - }, [handlePreview, handleRefresh]); + // An error raised against a chosen template goes back to the + // picker, like a successful inspection does. Only a failure to + // discover any templates at all leaves the screen, because there is + // no picker to go back to. + if (phase === 'inspecting' || (phase === 'error' && selectedFile)) handleInspectEscape(); + else if (phase === 'expanded' || phase === 'preview') setPhase('inspecting'); + else back(); - const handleInspectEscape = useCallback(() => { + return; - setSelectedFile(null); - setContext(null); - setPhase('picker'); + } - }, []); + if (phase !== 'inspecting') return; - const handleBackToInspect = useCallback(() => { + // Ink reports a Ctrl chord as the bare letter with `key.ctrl` set, so + // without this Ctrl+E would expand and Ctrl+R would re-render. Ctrl+D + // is safe either way — it arrives as `d`, which none of these match — + // but the pane below reads it, so the modifier check has to happen + // before any of them. + if (key.ctrl || key.meta) return; - setPhase('inspecting'); + if (input === 'e') setPhase('expanded'); - }, []); + if (input === 'p') handlePreview(); - const displayPath = selectedFile ? relative(projectRoot, selectedFile) : ''; + if (input === 'r') handleRefresh(); + + }); // Loading if (phase === 'loading') { @@ -561,19 +824,26 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { } - // Error (for context loading errors) - if (phase === 'error' && !selectedFile) { + // Error, from discovering the file list or from building the context. + // + // Both, deliberately: this used to require `!selectedFile`, which is true + // only of a discovery failure, so a template whose helper threw set + // `phase: 'error'` with a file selected and fell through every branch to + // "Unknown phase" — the one error a reader is most likely to hit was the + // one the screen would not show. + if (phase === 'error') { return ( - - Error - {error} + Error{displayPath ? `: ${displayPath}` : ''} + + [↑↓] Scroll + [^U/^D] Half [Esc] Back @@ -608,7 +878,6 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { ) : ( <> - No template files found in {sqlPath}/ @@ -639,111 +908,19 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { return ( - - - File: + + File: {displayPath} - - - {context.dataFiles.length > 0 && ( - - Data Files - {context.dataFiles.map(({ key, value }) => ( - - - $.{key} - - {describeType(value)} - - ))} - - )} - - {(context.helpers.length > 0 || context.helperErrors.length > 0) && ( - - Helpers ($helpers) - {context.helpers.map(({ key, value }) => ( - - - $.{key} - - {describeType(value)} - - ))} - {context.helperErrors.map(({ filepath, error: helperErr }) => ( - - Failed to load: {relative(projectRoot, filepath)} - {helperErr.message} - - ))} - - )} - - - Built-ins - {context.builtins.map(({ key }) => ( - - - $.{key} - - Function - - ))} - - - - Config - - - $.config - - - {context.config ? describeType(context.config) : '(not set)'} - - - - - - Secrets - - - $.secrets - - - Object ({Object.keys(context.secrets).length} keys) - - - - - $.globalSecrets - - - Object ({Object.keys(context.globalSecrets).length} keys) - - - - - - Environment - - - $.env - - - Object ({Object.keys(context.env).length} keys) - - - + + + [↑↓] Scroll + [^U/^D] Half [e] Expand [p] Preview SQL [r] Refresh @@ -759,38 +936,19 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { return ( - - - File: + + File: {displayPath} - - - {context.dataFiles.map(({ key, value }) => ( - - $.{key} - {describeTypeExpanded(value, 1).map((line, i) => ( - {line} - ))} - - ))} - - {context.config !== undefined && context.config !== null && ( - - $.config - {describeTypeExpanded(context.config, 1).map((line, i) => ( - {line} - ))} - - )} + + + [↑↓] Scroll + [^U/^D] Half [Esc] Back to summary @@ -802,13 +960,10 @@ export function RunInspectScreen({ params }: ScreenProps): ReactElement { if (phase === 'preview') { const hasError = error !== null; + const timing = renderDuration !== null ? ` · ${renderDuration.toFixed(1)}ms` : ''; return ( - - - File: + + File: {displayPath} - - - {!hasError && renderDuration !== null && ( - - Rendered in: - {renderDuration.toFixed(1)}ms - - )} - - - {hasError ? ( - {error} - ) : ( - {renderedSql} - )} - + {hasError ? '' : timing} + + + [↑↓] Scroll + [^U/^D] Half [Esc] Back to summary diff --git a/src/tui/utils/index.ts b/src/tui/utils/index.ts index 0dd2b9f7..278642c8 100644 --- a/src/tui/utils/index.ts +++ b/src/tui/utils/index.ts @@ -2,7 +2,7 @@ * CLI utilities barrel export. */ export { copyToClipboard, isClipboardAvailable } from './clipboard.js'; -export { toKebabCase } from './string.js'; +export { toKebabCase, oneLine } from './string.js'; export { resolveChangesDir, resolveSqlDir } from './paths.js'; export { resolveScreenIdentity } from './identity.js'; export { createChangeManager, type CreateChangeManagerOptions } from './change-context.js'; diff --git a/src/tui/utils/string.ts b/src/tui/utils/string.ts index 30b9e4d5..921899da 100644 --- a/src/tui/utils/string.ts +++ b/src/tui/utils/string.ts @@ -2,6 +2,24 @@ * String utilities for CLI. */ +/** + * Flatten text to a single line. + * + * `wrap="truncate"` bounds a ``'s width and not its height: Ink still + * breaks on an embedded newline, so one row of a windowed list silently becomes + * three and the fold is wrong by two for everything below it. Anything drawn + * into a counted row — a secret, an environment value, a database error, a + * stack trace — goes through here first. + * + * @example + * oneLine('Unexpected token\n at line 3'); // 'Unexpected token at line 3' + */ +export function oneLine(text: string): string { + + return text.replace(/[\r\n]+/g, ' '); + +} + /** * Converts a sentence or phrase to kebab-case. * diff --git a/tests/cli/app-context.test.tsx b/tests/cli/app-context.test.tsx index c8ae34d3..afc8479d 100644 --- a/tests/cli/app-context.test.tsx +++ b/tests/cli/app-context.test.tsx @@ -39,24 +39,28 @@ const createMockStateManager = () => ({ isLoaded: true, }); -const createMockSettingsManager = () => ({ - load: vi.fn().mockResolvedValue({ version: '0.1.0' }), - isLoaded: true, - settings: { version: '0.1.0' }, - getStages: vi.fn().mockReturnValue([]), -}); - -// Mock the state and settings managers +// Spread the real module rather than listing replacements. +// +// `mock.module` swaps the whole module, so naming only the handful of exports +// this file uses deleted the other ~40 the barrel carries — and because the +// registry is process-global and never restores, it deleted them for every file +// loaded afterwards too. +// +// `getSettingsManager` is deliberately *not* mocked. `app.test.tsx` is the next +// file in the suite, and it writes a real `.noorm/settings.yml` to assert the +// mouse honours `ui.mouse: false`; a faked manager handed it a settings object +// with no `ui` section, which `isMouseEnabled` reads as on. That was two CI +// failures on Linux, where the leak reproduces and macOS does not. The real +// manager on a project with no settings file returns defaults, which is what +// the assertions below want anyway. mock.module('../../src/core/index.js', () => ({ - observer, + ...actualCore, getStateManager: vi.fn(() => createMockStateManager()), - getSettingsManager: vi.fn(() => createMockSettingsManager()), - resetStateManager: vi.fn(), - resetSettingsManager: vi.fn(), })); // Mock identity loading to return null (no global identity) mock.module('../../src/core/identity/index.js', () => ({ + ...actualIdentity, loadExistingIdentity: vi.fn().mockResolvedValue(null), })); @@ -183,11 +187,15 @@ describe('cli: app-context', () => { }); - // Restore mocked modules to prevent pollution of subsequent test files + // No restore here, deliberately. Re-registering the real module does not + // undo `mock.module` — measured on Bun 1.3.11 and 1.4.0 alike, a re-register + // still resolves to the mock. The block that used to sit here claimed to + // prevent pollution and did nothing. What keeps this file honest is the + // mocks above spreading the real module and faking as little as possible, + // because whatever they fake is faked for the rest of the process. afterAll(() => { - mock.module('../../src/core/index.js', () => actualCore); - mock.module('../../src/core/identity/index.js', () => actualIdentity); + observer.clear(); }); diff --git a/tests/cli/components/scroll-pane.test.tsx b/tests/cli/components/scroll-pane.test.tsx new file mode 100644 index 00000000..38b5f698 --- /dev/null +++ b/tests/cli/components/scroll-pane.test.tsx @@ -0,0 +1,261 @@ +/** + * ScrollPane tests. + * + * Ink has no scroll offset, so content past the bottom of the terminal is not + * merely unscrolled — it is unreachable, and the screen's own footer is what + * gets pushed off to make room for it. The contract pinned here is that every + * line handed to the pane can be brought on screen by a key, and that the pane + * never draws more rows than the budget it was given, because the budget is + * what the screen subtracted its chrome from. + * + * The "not focused" case is load-bearing rather than incidental: the pane takes + * focus as a prop and registers its handler unconditionally (Ink's `useInput` + * never re-registers once skipped), so the guard inside the handler is the only + * thing stopping a background pane from consuming a focused screen's arrows. + */ +import { describe, it, expect } from 'bun:test'; +import { render } from 'ink-testing-library'; +import { Text } from 'ink'; +import React from 'react'; + +import { ScrollPane } from '../../../src/tui/components/terminal/ScrollPane.js'; +import { MouseProvider } from '../../../src/tui/mouse.js'; + +/** An SGR wheel-down press. 64 is the wheel bit; the low bit picks the direction. */ +const WHEEL_DOWN = '\x1B[<65;10;10M'; + +/** An SGR wheel-up press. */ +const WHEEL_UP = '\x1B[<64;10;10M'; + +/** Rows the pane may draw, indicators included. */ +const HEIGHT = 10; + +/** More lines than the height, so there is always something below the fold. */ +const TOTAL = 40; + +// eslint-disable-next-line no-control-regex -- matching the ANSI SGR escape is the point +const ANSI_PATTERN = /\[[0-9;]*m/g; + +function strip(frame: string | undefined): string { + + return (frame ?? '').replace(ANSI_PATTERN, ''); + +} + +/** + * Poll rather than sleep a guessed duration: a fixed wait is the suite's known + * flake class under load. + */ +async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promise { + + const deadline = Date.now() + timeoutMs; + + while (!predicate() && Date.now() < deadline) { + + await new Promise((resolve) => setTimeout(resolve, 10)); + + } + +} + +function lines(count = TOTAL) { + + return Array.from({ length: count }, (_, index) => ( + line-{index} + )); + +} + +/** + * One keypress, then wait for Ink to take it. + * + * Writes in a tight loop are coalesced: twenty synchronous `\x04` writes reach + * `useInput` as one twenty-character string, which is not twenty Ctrl+D events + * and scrolls exactly one half-page. Yielding between presses is what makes a + * press a press. + */ +async function press(stdin: { write: (data: string) => void }, sequence: string): Promise { + + stdin.write(sequence); + + await new Promise((resolve) => setTimeout(resolve, 10)); + +} + +describe('cli: ScrollPane', () => { + + it('should draw no more rows than the height it was given', async () => { + + const { stdin, lastFrame, unmount } = render( + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + // At rest and mid-scroll both, because the second is where the pane is + // tallest: `rowWindow` reserves the two indicator rows as a pair, so at + // the ends of the scroll one of them is unused and the pane is a row + // shorter than its budget rather than a row longer. + expect(strip(lastFrame()).split('\n').length).toBeLessThanOrEqual(HEIGHT); + + await press(stdin, '\x04'); + + expect(strip(lastFrame()).split('\n').length).toBeLessThanOrEqual(HEIGHT); + expect(strip(lastFrame())).toContain('↑'); + + unmount(); + + }); + + it('should hold back the lines past the fold, and say how many', async () => { + + const { lastFrame, unmount } = render( + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + const frame = strip(lastFrame()); + + expect(frame).toContain('line-0'); + expect(frame).not.toContain('line-39'); + expect(frame).toContain('more'); + + unmount(); + + }); + + it('should reach the last line by paging', async () => { + + const { stdin, lastFrame, unmount } = render( + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + // Ctrl+D, the advertised half-page key, enough times to pass the end. + for (let count = 0; count < 20; count += 1) { + + await press(stdin, '\x04'); + + if (strip(lastFrame()).includes('line-39')) break; + + } + + expect(strip(lastFrame())).toContain('line-39'); + + unmount(); + + }); + + it('should move one line at a time on an arrow', async () => { + + const { stdin, lastFrame, unmount } = render( + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + stdin.write('\x1B[B'); + + await waitFor(() => !strip(lastFrame()).includes('line-0')); + + expect(strip(lastFrame())).toContain('line-1'); + expect(strip(lastFrame())).not.toContain('line-0'); + + unmount(); + + }); + + it('should ignore every scroll key while it is not focused', async () => { + + const { stdin, lastFrame, unmount } = render( + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + const before = strip(lastFrame()); + + stdin.write('\x1B[B'); + stdin.write('\x04'); + + await new Promise((resolve) => setTimeout(resolve, 50)); + + expect(strip(lastFrame())).toBe(before); + + unmount(); + + }); + + it('should scroll on a wheel notch', async () => { + + // The reason this is not optional: the TUI runs in the alternate + // screen, which has no scrollback, and mouse tracking takes the wheel + // notches a terminal would otherwise turn into arrow keys. Unhandled + // here means the wheel does nothing at all. + const { stdin, lastFrame, unmount } = render( + + + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + await press(stdin, WHEEL_DOWN); + await press(stdin, WHEEL_DOWN); + + await waitFor(() => !strip(lastFrame()).includes('line-0')); + + expect(strip(lastFrame())).toContain('line-2'); + expect(strip(lastFrame())).not.toContain('line-0'); + + await press(stdin, WHEEL_UP); + + await waitFor(() => strip(lastFrame()).includes('line-1')); + + expect(strip(lastFrame())).toContain('line-1'); + + unmount(); + + }); + + it('should ignore a wheel notch aimed at a pane that is not focused', async () => { + + const { stdin, lastFrame, unmount } = render( + + + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + const before = strip(lastFrame()); + + await press(stdin, WHEEL_DOWN); + await press(stdin, WHEEL_DOWN); + + expect(strip(lastFrame())).toBe(before); + + unmount(); + + }); + + it('should draw content that fits without stealing a row for an indicator', async () => { + + const { lastFrame, unmount } = render( + , + ); + + await waitFor(() => strip(lastFrame()).includes('line-0')); + + const frame = strip(lastFrame()); + + expect(frame.split('\n')).toHaveLength(3); + expect(frame).not.toContain('more'); + + unmount(); + + }); + +}); diff --git a/tests/cli/components/select-list-window.test.tsx b/tests/cli/components/select-list-window.test.tsx new file mode 100644 index 00000000..70ffde57 --- /dev/null +++ b/tests/cli/components/select-list-window.test.tsx @@ -0,0 +1,268 @@ +/** + * SelectList windowing and custom-row tests. + * + * The defect these pin is not "the list does not scroll" but something worse + * that hid inside it. Both history screens drew a fixed `slice(0, N)` while + * their cursor ranged over the whole array, so past row N the arrow keys still + * moved a selection the reader could not see, and the detail box under the list + * described a record that was not on screen. A test that only asserted "more + * rows are reachable" would pass on that arrangement. + * + * So what is pinned here is the invariant that makes the two impossible to + * disagree: whatever the cursor is on is drawn, and it is drawn as selected. + * Every screen that hands its list to `SelectList` inherits it, which is the + * point of moving them onto it. + * + * `renderItem` is tested alongside because it is the reason those screens could + * move: without a custom row body they would have had to give up the per-status + * colour that a reader scans a history list for, and would have kept their own + * list to keep it. + */ +import { describe, it, expect } from 'bun:test'; +import { render } from 'ink-testing-library'; +import { Text } from 'ink'; +import React from 'react'; + +import { FocusProvider } from '../../../src/tui/focus.js'; +import { SelectList } from '../../../src/tui/components/lists/index.js'; + +/** Rows the list may draw, pinned so the assertions do not depend on a terminal. */ +const VISIBLE = 5; + +/** Far more items than fit, so there is always something past the fold. */ +const TOTAL = 40; + +const DOWN = '\x1B[B'; +const UP = '\x1B[A'; + +const ANSI_PATTERN = /\[[0-9;]*m/g; + +function strip(frame: string | undefined): string { + + return (frame ?? '').replace(ANSI_PATTERN, ''); + +} + +/** The line the cursor is on, marker included. */ +function selectedLine(frame: string | undefined): string { + + return strip(frame).split('\n').find((line) => line.includes('❯')) ?? ''; + +} + +/** + * Press a key until the list says what we are waiting for, or give up. + * + * Counting presses does not survive contact with Ink. The focus stack + * initialises in an effect, so a keypress written on the first tick is dropped + * before any handler exists, and writes in a tight loop coalesce into one input + * string and one event. Both make "press N times, expect index N" wrong in a + * way that looks like a component bug. Driving to a condition sidesteps both, + * and the condition is the thing worth asserting anyway. + */ +async function pressUntil( + stdin: { write: (data: string) => void }, + sequence: string, + predicate: () => boolean, + limit = 80, +): Promise { + + for (let count = 0; count < limit && !predicate(); count += 1) { + + stdin.write(sequence); + + await new Promise((resolve) => setTimeout(resolve, 10)); + + } + +} + +/** Wait for the first frame to settle, without pressing anything. */ +async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promise { + + const deadline = Date.now() + timeoutMs; + + while (!predicate() && Date.now() < deadline) { + + await new Promise((resolve) => setTimeout(resolve, 10)); + + } + +} + +function items(count = TOTAL) { + + return Array.from({ length: count }, (_, index) => ({ + key: `k${index}`, + label: `item-${index}`, + value: index, + })); + +} + +describe('cli: SelectList windowing', () => { + + it('should keep the cursor on a row it is actually drawing', async () => { + + const { stdin, lastFrame, unmount } = render( + + + , + ); + + // Well past the old fixed window of fifteen. + // Driven on the *selected* line, not the frame: a row enters the + // window several presses before the cursor reaches it, so watching the + // frame stops the loop early and asserts nothing. + await pressUntil(stdin, DOWN, () => selectedLine(lastFrame()).includes('item-25')); + + // The invariant: the row the cursor is on is a row that got drawn. The + // hand-rolled lists failed this — the marker sat on an index outside + // the slice, so no drawn line carried it and the reader was steering + // something invisible. + expect(selectedLine(lastFrame())).toContain('item-25'); + + unmount(); + + }); + + it('should draw no more rows than it was given room for', async () => { + + const { stdin, lastFrame, unmount } = render( + + + , + ); + + await pressUntil(stdin, DOWN, () => strip(lastFrame()).includes('item-10')); + + // The items, plus at most one indicator line at each end. A list that + // draws more than this is not clipping, it is pushing the screen's own + // footer off the bottom. + expect(strip(lastFrame()).split('\n').length).toBeLessThanOrEqual(VISIBLE + 2); + + unmount(); + + }); + + it('should leave the first rows reachable after scrolling away from them', async () => { + + const { stdin, lastFrame, unmount } = render( + + + , + ); + + await pressUntil(stdin, DOWN, () => selectedLine(lastFrame()).includes('item-30')); + + expect(strip(lastFrame())).not.toContain('item-0\n'); + + await pressUntil(stdin, UP, () => selectedLine(lastFrame()).includes('item-0')); + + expect(selectedLine(lastFrame())).toContain('item-0'); + + unmount(); + + }); + +}); + +describe('cli: SelectList renderItem', () => { + + it('should draw the custom body in place of the label', async () => { + + const { lastFrame, unmount } = render( + + [OK] custom-{item.value}} + /> + , + ); + + await waitFor(() => strip(lastFrame()).includes('custom-0')); + + const frame = strip(lastFrame()); + + expect(frame).toContain('[OK] custom-0'); + expect(frame).not.toContain('item-0'); + + unmount(); + + }); + + it('should keep the cursor marker even though the caller draws the row', async () => { + + // The list owns the marker so every list in the app marks its selection + // the same way, and so a caller supplying a body cannot forget one. + const { stdin, lastFrame, unmount } = render( + + custom-{item.value}} + /> + , + ); + + await pressUntil(stdin, DOWN, () => selectedLine(lastFrame()).includes('custom-2')); + + expect(selectedLine(lastFrame())).toContain('custom-2'); + + unmount(); + + }); + + it('should report the highlight that matches the drawn cursor', async () => { + + // The detail box under both history screens is built from this + // callback. When it disagreed with the window, the box described a + // record that was not on screen — the same defect seen from the other + // side, so it is asserted against the drawn row rather than a count. + const seen: number[] = []; + + const { stdin, lastFrame, unmount } = render( + + seen.push(item.value)} + renderItem={(item) => custom-{item.value}} + /> + , + ); + + await pressUntil(stdin, DOWN, () => selectedLine(lastFrame()).includes('custom-7')); + + expect(seen.at(-1)).toBe(7); + expect(selectedLine(lastFrame())).toContain(`custom-${seen.at(-1)}`); + + unmount(); + + }); + + it('should still window a custom-bodied list', async () => { + + const { lastFrame, unmount } = render( + + custom-{item.value}} + /> + , + ); + + await waitFor(() => strip(lastFrame()).includes('custom-0')); + + const frame = strip(lastFrame()); + + expect(frame).not.toContain('custom-39'); + expect(frame).toContain('more'); + + unmount(); + + }); + +}); diff --git a/tests/cli/components/terminal.test.tsx b/tests/cli/components/terminal.test.tsx index b696a64d..52540762 100644 --- a/tests/cli/components/terminal.test.tsx +++ b/tests/cli/components/terminal.test.tsx @@ -31,6 +31,27 @@ import { ResultBrowser, ResultTable } from '../../../src/tui/components/terminal /** Columns the ink-testing-library terminal reports. */ const TERMINAL_COLUMNS = 100; +// eslint-disable-next-line no-control-regex -- matching the ANSI SGR escape is the point +const SGR_PATTERN = /\x1B\[[0-9;]*m/g; + +/** + * A frame with its colour escapes removed. + * + * The header of this file used to say the suite runs at `FORCE_COLOR=0` and + * that nothing here may read an SGR escape. Only the second half was true: CI + * sets `FORCE_COLOR: 'true'`, so every line arrives prefixed with one. The two + * tests that located a row by `startsWith('─')` therefore never found it — + * `findIndex` returned -1, the assertion indexed past the start of the array, + * and both have failed since the day they were written. Reading the layout + * through this instead makes them agnostic to whether colour is on, which is + * what they meant in the first place. + */ +function plain(frame: string | undefined): string { + + return (frame ?? '').replace(SGR_PATTERN, ''); + +} + /** * A line only the grid prints. * @@ -229,10 +250,12 @@ describe('cli: components/terminal', () => { await waitFor(() => Boolean(lastFrame()?.includes('col_00'))); - const frame = lastFrame() ?? ''; + const frame = plain(lastFrame()); const lines = frame.split('\n'); const rule = lines.findIndex((line) => line.startsWith('─')); + expect(rule).toBeGreaterThan(0); + // Five whole columns at sixteen, not fifteen at four. Asserting on // the header line rather than on the frame is the point: squeezing // fifteen columns into the row also removes the string `col_05`, diff --git a/tests/cli/components/text-overlay.test.tsx b/tests/cli/components/text-overlay.test.tsx new file mode 100644 index 00000000..ee6c7f25 --- /dev/null +++ b/tests/cli/components/text-overlay.test.tsx @@ -0,0 +1,162 @@ +/** + * TextOverlay tests. + * + * The history screens have to summarise, and a database error does not + * summarise: the line that names the constraint or the syntax error can be + * anywhere in it. Before this, one screen cut the message at 80 characters and + * the other drew a `` per line with no bound, so a stack trace either lost + * its middle or took the screen's own footer down with it. This is the way to + * read the whole thing, so what is pinned is that the whole thing is reachable + * — the last line as well as the first — and that the overlay stays inside its + * row budget while doing it. + */ +import { describe, it, expect } from 'bun:test'; +import { render } from 'ink-testing-library'; +import React from 'react'; + +import { FocusProvider } from '../../../src/tui/focus.js'; +import { TextOverlay } from '../../../src/tui/components/overlays/TextOverlay.js'; + +/** Long enough that no terminal draws it in one screen. */ +const LINES = Array.from({ length: 120 }, (_, index) => `trace-line-${index}`); + +const LONG_ERROR = LINES.join('\n'); + +const ANSI_PATTERN = /\[[0-9;]*m/g; + +function strip(frame: string | undefined): string { + + return (frame ?? '').replace(ANSI_PATTERN, ''); + +} + +async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promise { + + const deadline = Date.now() + timeoutMs; + + while (!predicate() && Date.now() < deadline) { + + await new Promise((resolve) => setTimeout(resolve, 10)); + + } + +} + +/** + * Press until the frame says what we are waiting for. + * + * The focus stack initialises in an effect, so the first keypress after render + * is dropped, and writes in a tight loop coalesce into one event. Driving to a + * condition survives both. + */ +async function pressUntil( + stdin: { write: (data: string) => void }, + sequence: string, + predicate: () => boolean, + limit = 200, +): Promise { + + for (let count = 0; count < limit && !predicate(); count += 1) { + + stdin.write(sequence); + + await new Promise((resolve) => setTimeout(resolve, 5)); + + } + +} + +describe('cli: TextOverlay', () => { + + it('should open on the start of the text and hold the rest back', async () => { + + const { lastFrame, unmount } = render( + + {}} /> + , + ); + + await waitFor(() => strip(lastFrame()).includes('trace-line-0')); + + const frame = strip(lastFrame()); + + expect(frame).toContain('Error'); + expect(frame).toContain('trace-line-0'); + expect(frame).not.toContain('trace-line-119'); + expect(frame).toContain('more'); + + unmount(); + + }); + + it('should reach the last line of the message', async () => { + + // The reason the overlay exists. A message this long was previously + // either cut at 80 characters or drawn in full off the bottom of the + // screen; neither let a reader see this line. + const { stdin, lastFrame, unmount } = render( + + {}} /> + , + ); + + await waitFor(() => strip(lastFrame()).includes('trace-line-0')); + + await pressUntil(stdin, '\x04', () => strip(lastFrame()).includes('trace-line-119')); + + expect(strip(lastFrame())).toContain('trace-line-119'); + + unmount(); + + }); + + it('should close on Escape', async () => { + + let closed = false; + + const close = () => { + + closed = true; + + }; + + const { stdin, lastFrame, unmount } = render( + + + , + ); + + await waitFor(() => strip(lastFrame()).includes('trace-line-0')); + + await pressUntil(stdin, '\x1B', () => closed); + + expect(closed).toBe(true); + + unmount(); + + }); + + it('should wrap a long single line rather than cutting it', async () => { + + // A database error is often one very long line. Cutting it loses the + // end, which is where the detail usually is. + const tail = 'CONSTRAINT-VIOLATED-HERE'; + const oneLongLine = `${'padding '.repeat(80)}${tail}`; + + const { stdin, lastFrame, unmount } = render( + + {}} /> + , + ); + + await waitFor(() => strip(lastFrame()).includes('padding')); + + await pressUntil(stdin, '\x04', () => strip(lastFrame()).includes(tail)); + + expect(strip(lastFrame())).toContain(tail); + + unmount(); + + }); + +}); diff --git a/tests/cli/screens/db/explore-row-view.test.tsx b/tests/cli/screens/db/explore-row-view.test.tsx index 6870265c..a7c08b8c 100644 --- a/tests/cli/screens/db/explore-row-view.test.tsx +++ b/tests/cli/screens/db/explore-row-view.test.tsx @@ -133,6 +133,40 @@ async function settleRowView( } +/** + * Wait until the peek's grid is listening, before any key that has to land. + * + * Waiting for the header and the spinner's absence proves the query resolved, + * not that `ResultTable` registered its `useInput` — that happens in an effect, + * one tick after the frame those checks read. An arrow written in between is + * dropped silently, and the test then asserts against row one having asked for + * row two. On a loaded CI runner that gap is wide enough to lose the keystroke + * every time, which is what made five tests here fail there and nowhere else. + * + * `/` is the probe because filter mode announces itself in the footer, and + * Escape out of it restores the grid without moving the cursor. + */ +async function settleGrid( + stdin: { write: (data: string) => void }, + frame: () => string, +): Promise { + + const deadline = Date.now() + 2000; + + while (!frame().includes('[Tab] Column') && Date.now() < deadline) { + + stdin.write(KEY.filter); + + await new Promise((resolve) => setTimeout(resolve, 20)); + + } + + stdin.write(KEY.escape); + + await waitFor(() => !frame().includes('[Tab] Column')); + +} + function column(name: string, overrides: Partial = {}): ColumnDetail { return { @@ -920,6 +954,9 @@ describe('cli: screens/db/explore peek navigation', () => { // keystroke after it lands nowhere. await waitFor(() => frame().includes(PEEK_HEADER) && !frame().includes('Reading')); + // ...and then until the grid under it is actually listening. + await settleGrid(stdin, frame); + const press = async (sequence: string, settled: (current: string) => boolean) => { stdin.write(sequence); diff --git a/tests/cli/screens/db/sql-row-view.test.tsx b/tests/cli/screens/db/sql-row-view.test.tsx index b8299e06..fb38918f 100644 --- a/tests/cli/screens/db/sql-row-view.test.tsx +++ b/tests/cli/screens/db/sql-row-view.test.tsx @@ -243,7 +243,13 @@ describe('cli: screens/db sql result browsing', () => { , ); - const frame = () => view.lastFrame() ?? ''; + // Colour escapes stripped: CI runs at `FORCE_COLOR: 'true'`, so every + // line arrives prefixed with an SGR sequence and the chop test's + // `startsWith('│')` never matched — it has failed since it was written. + // Every assertion here is on text, so reading the frame without colour + // is both correct and agnostic to how the suite is invoked. + // eslint-disable-next-line no-control-regex -- matching the ANSI SGR escape is the point + const frame = () => (view.lastFrame() ?? '').replace(/\x1B\[[0-9;]*m/g, ''); await waitFor(() => frame().includes('select * from ai_usage')); diff --git a/tests/cli/screens/run/inspect-lines.test.tsx b/tests/cli/screens/run/inspect-lines.test.tsx new file mode 100644 index 00000000..06c26eef --- /dev/null +++ b/tests/cli/screens/run/inspect-lines.test.tsx @@ -0,0 +1,238 @@ +/** + * Inspect context line-builder tests. + * + * The inspect screen rendered a nested tree that grew with the context, so on + * any real project the bottom of it — secrets, global secrets, environment — + * sat below the fold with no key that could reach it. Ink has no scroll offset, + * so the fix is structural: the view is built as a flat list of one element per + * visual line, which is the only shape a viewport can slice. These tests pin + * that shape and what the lines are allowed to say. + * + * The secret cases are the load-bearing ones. `$.secrets` used to show a key + * count, which cannot answer the question the screen is opened to answer, and + * the fix is a partial reveal — so what is pinned is that the plaintext is + * absent and the masked form is present. Asserting only the latter would pass + * on a line that printed both. + */ +import { describe, it, expect } from 'bun:test'; +import { render } from 'ink-testing-library'; +import { Box } from 'ink'; +import React from 'react'; + +import type { CategorizedContext } from '../../../../src/tui/screens/run/RunInspectScreen.js'; + +import { contextLines, expandedLines } from '../../../../src/tui/screens/run/RunInspectScreen.js'; + +/** Row budget inside the inspect Panel on a 100-column terminal. */ +const WIDE = 96; + +// eslint-disable-next-line no-control-regex -- matching the ANSI SGR escape is the point +const ANSI_PATTERN = /\[[0-9;]*m/g; + +function draw(lines: React.ReactElement[]): string { + + const { lastFrame, unmount } = render({lines}); + const frame = (lastFrame() ?? '').replace(ANSI_PATTERN, ''); + + unmount(); + + return frame; + +} + +function makeContext(overrides: Partial = {}): CategorizedContext { + + return { + dataFiles: [{ key: 'users', value: [{ id: 1, name: 'ada' }] }], + helpers: [], + helperErrors: [], + builtins: [{ key: 'quote', value: () => '' }], + config: { host: 'localhost' }, + secrets: {}, + globalSecrets: {}, + env: {}, + ...overrides, + }; + +} + +describe('cli: inspect context lines', () => { + + it('should reveal part of a secret rather than only a key count', () => { + + const lines = contextLines( + makeContext({ secrets: { DB_PASSWORD: 'sup3rs3cr3tvalue' } }), + '/project', + WIDE, + ); + const frame = draw(lines); + + expect(frame).toContain('DB_PASSWORD'); + expect(frame).toContain('su*****alue'); + expect(frame).not.toContain('sup3rs3cr3tvalue'); + + }); + + it('should report the length as a number rather than as mask width', () => { + + const frame = draw(contextLines( + makeContext({ secrets: { TOKEN: 'sup3rs3cr3tvalue' } }), + '/project', + WIDE, + )); + + expect(frame).toContain('(16 chars)'); + + }); + + it('should tell a secret that is set but empty from one that is short', () => { + + const frame = draw(contextLines( + makeContext({ secrets: { BLANK: '', TINY: 'abc' } }), + '/project', + WIDE, + )); + + expect(frame).toContain('(empty)'); + expect(frame).toContain('(0 chars)'); + expect(frame).not.toContain('abc'); + + }); + + it('should mask global secrets on the same terms as config-scoped ones', () => { + + const frame = draw(contextLines( + makeContext({ globalSecrets: { LICENSE: 'aaaabbbbccccdddd' } }), + '/project', + WIDE, + )); + + expect(frame).toContain('aaaa'.slice(0, 2) + '*****' + 'dddd'); + expect(frame).not.toContain('aaaabbbbccccdddd'); + + }); + + it('should mask environment values, which carry secrets nothing here can identify', () => { + + const frame = draw(contextLines( + makeContext({ env: { AWS_SECRET_ACCESS_KEY: 'abcdefghijklmnopqrst' } }), + '/project', + WIDE, + )); + + expect(frame).toContain('AWS_SECRET_ACCESS_KEY'); + expect(frame).toContain('ab*****qrst'); + expect(frame).not.toContain('abcdefghijklmnopqrst'); + + }); + + it('should keep a multi-line secret to one row', () => { + + // `wrap="truncate"` bounds width, not height: Ink still breaks on an + // embedded newline, so one of these rows would draw three and put the + // viewport's count out by two for everything below it. + // + // The newline has to fall inside the revealed window to reach the + // screen at all. A whole PEM key does not test this: the mask keeps + // two leading and four trailing characters, which for + // `-----BEGIN…-----` are dashes, so the newlines never survive masking + // and the case passes whether or not the guard is there. + const lines = contextLines( + makeContext({ secrets: { TLS_KEY: '\nMIIEvQIBADANBgkq\n' } }), + '/project', + WIDE, + ); + + expect(draw(lines).split('\n')).toHaveLength(lines.length); + + }); + + it('should keep a multi-line environment value and helper error to one row', () => { + + const lines = contextLines( + makeContext({ + env: { SSH_KEY: 'line one\nline two\nline three' }, + helperErrors: [{ + filepath: '/project/sql/helpers/slug.js', + error: new Error('Unexpected token\n at line 3\n at line 4'), + }], + }), + '/project', + WIDE, + ); + + expect(draw(lines).split('\n')).toHaveLength(lines.length); + + }); + + it('should keep a data-file string preview with a newline to one row', () => { + + // `describeType` truncates a string preview by character count and was + // safe under the old nested tree, which counted no rows. + const lines = contextLines( + makeContext({ dataFiles: [{ key: 'banner', value: 'first line\nsecond line' }] }), + '/project', + WIDE, + ); + + expect(draw(lines).split('\n')).toHaveLength(lines.length); + + }); + + it('should draw one row per line, so a viewport can count them', () => { + + const lines = contextLines( + makeContext({ + secrets: Object.fromEntries( + Array.from({ length: 12 }, (_, index) => [`SECRET_${index}`, 'x'.repeat(20)]), + ), + }), + '/project', + WIDE, + ); + + expect(draw(lines).split('\n')).toHaveLength(lines.length); + + }); + + it('should omit a section that has nothing in it', () => { + + const frame = draw(contextLines(makeContext({ dataFiles: [] }), '/project', WIDE)); + + expect(frame).not.toContain('Data Files'); + expect(frame).toContain('Built-ins'); + + }); + + it('should name the helper that failed to load, and why', () => { + + const frame = draw(contextLines( + makeContext({ + helperErrors: [{ + filepath: '/project/sql/helpers/slug.js', + error: new Error('Unexpected token'), + }], + }), + '/project', + WIDE, + )); + + expect(frame).toContain('sql/helpers/slug.js'); + expect(frame).toContain('Unexpected token'); + + }); + + it('should wrap the expanded view so a narrow terminal still gets one row per line', () => { + + // A long key, not a long value: the expanded view reports shapes and + // never prints a value, so the key is what can overflow a row. + const value = { ['deeply_nested_'.repeat(30)]: 1 }; + const wide = expandedLines(makeContext({ dataFiles: [{ key: 'doc', value }] }), WIDE); + const narrow = expandedLines(makeContext({ dataFiles: [{ key: 'doc', value }] }), 20); + + expect(narrow.length).toBeGreaterThan(wide.length); + expect(draw(narrow).split('\n')).toHaveLength(narrow.length); + + }); + +}); diff --git a/tests/core/shared/mask.test.ts b/tests/core/shared/mask.test.ts new file mode 100644 index 00000000..51b72a08 --- /dev/null +++ b/tests/core/shared/mask.test.ts @@ -0,0 +1,119 @@ +/** + * maskSecret band tests. + * + * The bands exist so that the shorter a value is, the less of it a reader is + * shown: a four-character secret has no middle to hide, so revealing its ends + * would reveal the value. What is pinned here is how much of the input survives + * each band, not how the asterisks look — widening a band fails these tests + * even when the output still reads as masked, which is the point. A test that + * only checked for the presence of a `*` would pass on a full reveal with one + * asterisk appended. + */ +import { describe, it, expect } from 'bun:test'; + +import { maskSecret } from '../../../src/core/shared/mask.js'; + +/** Everything of the value that survived the mask, in order. */ +function revealed(value: string): string { + + return maskSecret(value).replaceAll('*', ''); + +} + +describe('core: maskSecret', () => { + + it('should distinguish a value that is set but empty from one that is masked', () => { + + expect(maskSecret('')).toBe('(empty)'); + + }); + + it('should reveal nothing of a value with no middle to hide', () => { + + for (const value of ['a', 'ab', 'abc', 'abcd']) { + + expect(maskSecret(value)).toBe('*****'); + expect(revealed(value)).toBe(''); + + } + + }); + + it('should reveal one character at each end from five through eight', () => { + + expect(maskSecret('abcde')).toBe('a*****e'); + expect(maskSecret('hunter2!')).toBe('h*****!'); + expect(revealed('hunter2!')).toHaveLength(2); + + }); + + it('should reveal only a suffix from nine through twelve', () => { + + expect(maskSecret('abcdefghi')).toBe('*****fghi'); + expect(maskSecret('abcdefghijkl')).toBe('*****ijkl'); + expect(revealed('abcdefghijkl')).toHaveLength(4); + + }); + + it('should reveal a short prefix and a suffix past twelve', () => { + + expect(maskSecret('abcdefghijklm')).toBe('ab*****jklm'); + expect(maskSecret('postgres://user:pw@host/db')).toBe('po*****t/db'); + + }); + + it('should reveal no more of a long value than of a barely-long one', () => { + + expect(revealed('x'.repeat(4096))).toHaveLength(6); + expect(revealed('abcdefghijklm')).toHaveLength(6); + + }); + + it('should leak no interior character, however recognisable', () => { + + const value = 'PREFIXsecretmiddleSUFFIX'; + + expect(maskSecret(value)).toBe('PR*****FFIX'); + expect(maskSecret(value)).not.toContain('secretmiddle'); + + }); + + it('should never split a surrogate pair', () => { + + // `.slice` counts UTF-16 code units, so slicing a non-BMP character in + // half yields a lone surrogate and the reveal renders as a replacement + // glyph — corrupting the one part a reader is meant to recognise. + const masked = maskSecret('🔥alpha-beta-gamma-omega🎉'); + + expect(masked).toBe('🔥a*****ega🎉'); + + // `for...of` walks code points, so a paired emoji arrives whole and a + // broken one arrives as a bare surrogate in D800-DFFF. + for (const character of masked) { + + const codePoint = character.codePointAt(0) ?? 0; + + expect(codePoint < 0xD800 || codePoint > 0xDFFF).toBe(true); + + } + + }); + + it('should count bands in characters, not code units', () => { + + // Four emoji are eight code units. Counting units would put this in the + // reveal-both-ends band, showing half of a four-character value. + expect(maskSecret('🔥🎉🚀🌟')).toBe('*****'); + + }); + + it('should never encode the length in the width of the mask', () => { + + const short = maskSecret('abcdefghijklm'); + const long = maskSecret('abcdefghijklm'.repeat(20)); + + expect(short.length).toBe(long.length); + + }); + +});