chore: reconcile next into master - #86
Conversation
The Ink 7 upgrade is what made the rest possible: useWindowSize replaced the hardcoded row counts, alternateScreen let the shell own the window, and measureElement gained the coordinates a click needs. TextInput is ours now because @inkjs/ui does not publish the hooks to rebuild its handler, and a mouse report reaches every useInput handler — clicking mid-edit typed the escape sequence into the field. Cancelling is honest per dialect: postgres and mysql ask the server to stop, mssql and sqlite can only stop waiting, and the wording differs.
Ink has no scroll offset, so a viewport can only draw a slice of a flat list and a tree cannot be sliced. Inspect built its context as a nested tree, so everything past the fold was unreachable and the screen's own footer was what got pushed off to make room. All four views — summary, expanded shapes, rendered SQL, render error — are now one element per visual line behind `ScrollPane`. One row must stay one row or the window arithmetic is wrong by the difference. `wrap="truncate"` bounds a cell's width but not its height, so `oneLine` flattens line breaks where text enters a one-row cell: nothing here controls what it displays, and a secret can be a PEM key, an environment variable can hold anything, and a helper's error message can be a multi-line diagnostic. The screen opens one focus scope with `skip` rather than one per phase. React runs a child's effects before its parent's, so a screen-level push lands above its own `SearchableList` and takes the keys the picker was mounted to receive. `$.secrets` reported a key count, which cannot answer what the screen is opened to ask: a stale password and a fresh one are both `Object (7 keys)`. It shows a partial reveal instead, narrowing as the value shortens, with the length beside it as a number rather than encoded in the mask width. Banding counts code points, so a non-BMP character is never sliced through a surrogate pair. `$.env` gets the same treatment — it is the whole of `process.env` and nothing here can tell which keys are credentials. The mouse wheel now scrolls every viewport. It reached only `SelectList` and `ResultTable`, the two `useRowMouse` callers, so the explore detail view, the full-text overlay and the row viewer ignored it. That is worse than unhandled: the TUI runs in the alternate screen, which has no scrollback, and mouse tracking takes the notches a terminal would otherwise translate into arrow keys — turning the mouse on is what removed the only wheel behaviour those panes had. `useWheelScroll` is the wheel-only half of `useRowMouse`, which needs row refs a viewport has none of. Also fixes an error branch that could never render: a helper that failed to load set the error phase with a file selected, which required `!selectedFile`, so the likeliest failure showed as "Unknown phase". Claude-Session: https://claude.ai/code/session_01JGZ8ocdNed7TFFEbccQsW2
next and master both carried the ink7 work as separate commits with different SHAs, so the files that commit *added* — mouse.tsx, RowViewOverlay, and the two explore overlays — did not exist at the merge base and came back as add/add conflicts on every attempt. Each was resolved to next's side after checking the pair: next's version is master's plus the wheel-scroll additions, with nothing master-only in any of them. Drop tui-ink7-and-row-inspection and connection-timeouts from .changeset/. master's Version Packages commit already consumed both into 1.2.0 — the ink7 notes are in packages/cli/CHANGELOG.md and the connect-timeout note is verbatim in packages/sdk/CHANGELOG.md — while next never saw that release and still carried the files. Merging them back would bump 1.2.0 a second time for changes that already shipped. inspect-scroll-and-masked-secrets stays: it has only ever been on next and is still unreleased. Package versions stay at master's 1.2.0.
Both history screens drew a fixed slice while their cursor ranged over every record, so past the window the selection moved somewhere invisible and the detail box described a row that was not on screen. Move them onto SelectList, which owns the cursor and the window together, and give it a renderItem prop so they keep the per-status colour that made them hand-roll a list to begin with. A failed file's error was drawn one row per newline with no bound, which took the list and the footer down with it; the history screen cut the same message at 80 characters instead. Add TextOverlay for the whole message, wrapped and scrollable, behind one key. Record the remaining ~40 unbounded-render sites as a follow-up rather than waiting to be told about them one at a time.
CI is red on this PR — and identically red on
|
| pass | fail | |
|---|---|---|
master @ 634597f8 (run 32555067359) |
919 | 9 |
| this PR (run 34183934896) | 939 | 9 |
Same failures, +20 passing. This merge introduces none of them.
What the 9 actually are
1. mock.module contamination, surfacing only on Linux (2 tests) — cli: app > should tell the help screen reader how to turn the mouse back on and provider hierarchy > should leave the mouse off when settings.yml writes ui.mouse: false. The test writes ui.mouse: false and asserts the help footer says Mouse off.; CI renders Mouse on.
This is the failure mode the root CLAUDE.md already documents for cli-logger-settings.test.ts: two init-screen tests replace the SettingsManager class, Bun's mock registry is process-global and never restores, so whichever file loads first decides what every later file sees. Load order is root-files-first on macOS and the reverse on Linux — so it passes locally and fails only on CI. These two need the same treatment cli-logger-settings.test.ts got: their own serial group.
2. explore peek navigation (5 tests) — three assertion failures plus two 5s timeouts. Passes in isolation locally, so contamination and/or the fixed-sleep flake class .claude/rules/tui-development.md calls out.
3. Terminal-width assumption (2 tests) — ResultTable column fit and sql result browsing chop both assume a 100-column terminal. Fails on CI and locally alike.
None of these are terminal-only cosmetics — #1 is a genuine "settings.yml is not honored" signal under a poisoned module registry, which is worth fixing rather than re-grouping around, if the mock can be dropped.
Happy to take these on in a separate PR; they predate this branch and I did not want to widen a merge-reconciliation commit into a test-infrastructure change.
Correction: I named the wrong file above, and now have the proven causeMy earlier comment blamed the
The leak. const createMockSettingsManager = () => ({ settings: { version: '0.1.0' }, ... });
Its Proof it is this object and not something else. Adding Why Linux only. The same pair passes on macOS under both Bun versions, so this is a platform difference in Bun's mock/module resolution, not a version difference. Why it looked sudden. It is not. Fix options, cheapest first:
The remaining 7 failures are unrelated: 5 |
Nine tests have failed on CI since bab7dfc. Nobody saw it: the Release workflow is what gates publishing and it stays green, and CI only triggers on master while recent work went to next. Three unrelated causes. app-context.test.tsx replaced the whole src/core/index.js barrel, so the mock both faked getSettingsManager and deleted the other ~40 exports — for every file loaded after it, since bun's registry is process-global and re-registering does not restore (measured on 1.3.11 and 1.4.0). app.test.tsx is next in the suite and writes a real settings.yml to assert ui.mouse: false; it read the fake, which has no ui section, and isMouseEnabled treats that as on. Spread the real module and stop faking settings. Reproduces on Linux only, which is why it passed locally for everyone. terminal.test.tsx and sql-row-view.test.tsx located a row with startsWith('─'). CI sets FORCE_COLOR=true, so every line begins with an SGR escape, findIndex returned -1, and the assertion indexed past the array. Both have failed since the day they were written. Read the layout with the escapes stripped. explore-row-view.test.tsx wrote arrow keys as soon as the query resolved, but ResultTable registers useInput in an effect a tick later, so the keystroke was dropped and the test asserted row two against row one. Settle the grid first, the way sql-row-view already does.
1 participant
Reconciles
nextintomaster. Merge commit only — no new source changes.Why they conflicted
nextandmastereach carried the ink7 work as a separate commit with a different SHA (06ad1d22vsbab7dfcd), from being merged into the two branches independently. Their patch-ids are identical, but that commit added several files, so at the merge base those files do not exist and git reports add/add on every one:src/tui/mouse.tsxsrc/tui/components/terminal/RowViewOverlay.tsxsrc/tui/components/terminal/index.tssrc/tui/screens/db/explore/ExploreDetailScreen.tsxsrc/tui/screens/db/explore/FullTextOverlay.tsxEach was resolved to
next's side, after diffing the pair per file:next's version ismaster's plus the wheel-scroll additions from37abd22c, with nothing master-only in any of them.git diff origin/next -- src testson the result is empty.The part that wasn't mechanical
master'sVersion Packagescommit released 1.2.0 and consumed the changesets.nextnever saw that release, so it still carried the files — and merging brought them back. Two of the three had already shipped:tui-ink7-and-row-inspection.mdpackages/cli1.2.0connection-timeouts.mdpackages/sdk1.2.0 (text is verbatim in the changelog)inspect-scroll-and-masked-secrets.mdnextLeft alone, the next release would have bumped 1.2.0 a second time and reprinted release notes for changes already published.
Package versions stay at
master's 1.2.0. No CHANGELOG entries were altered.Verification
master)git diff origin/next -- src testsempty, so no source fromnextwas dropped in the resolutionOrdering
This does not include PR #85 (history screens), which targets
nextand is now conflict-free there. Either order works: merge this first and #85 flows to master on the next reconcile, or merge #85 intonextfirst and rerun this merge.Worth deciding separately: the duplicate-SHA divergence will keep producing this conflict every cycle as long as the same PR is merged into both
nextandmasterindependently.