Skip to content

feat(tui): trial an anchored composer in fullscreen mode on nightly - #4221

Open
abhinav-phi wants to merge 6 commits into
apache:mainfrom
abhinav-phi:feat/tui-fullscreen-anchored-composer
Open

feat(tui): trial an anchored composer in fullscreen mode on nightly#4221
abhinav-phi wants to merge 6 commits into
apache:mainfrom
abhinav-phi:feat/tui-fullscreen-anchored-composer

Conversation

@abhinav-phi

@abhinav-phi abhinav-phi commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the nightly trial proposed in #4136: the TUI can now run fullscreen (alternate screen) with an anchored composer — the prompt, pending queue, activity strip, and status line stay pinned to the bottom of the screen while the transcript scrolls in an application-owned viewport. On nightly builds the fullscreen path is the default; release builds keep today's terminal-scrollback renderer. MAKA_TUI_FULLSCREEN=1 opts any build in, MAKA_TUI_FULLSCREEN=0 opts a nightly build out.

This builds on the capability analysis in Discussion #3879: the pinned @earendil-works/pi-tui@0.84.2 already ships TuiAltScreen (alternate-screen viewport with mouse, selection, search, and hyperlink support), so this is a mode switch inside the existing dependency — no renderer fork, no upstream rewrite.

What changes

  • packages/cli/src/tui-fullscreen.ts (new) — the experiment switch, the unread-output logic, and the hardened link opener:
    • resolveTuiFullscreen() resolves the mode with precedence: explicit setting (embeddings/tests) → MAKA_TUI_FULLSCREEN env override → build-channel default. Nightly is detected from the CLI package version using the Product Nightly identity format (0.2.0-dev.<run>.<YYYYMMDD>).
    • UnreadOutputCounter — counts transcript lines appended while the user is scrolled away from the bottom; cleared on return to the bottom, never negative on content shrink.
    • openExternalUrl() — click-to-open for OSC 8 hyperlinks. Assistant hrefs are model-authored (untrusted input), so only http:/https:/mailto: targets are handed off (the same allowlist as the desktop's external-link guard, apps/desktop/src/main/external-link-guard.ts), and openers never pass the URL through a shell: Windows uses rundll32 url.dll,FileProtocolHandler (the URL stays a single argv element; the DLL/entrypoint half is a compile-time constant), macOS/Linux pass it as a plain argv element to open/xdg-open.
  • packages/cli/src/pi-tui-layout.ts — the fullscreen layout pieces:
    • MakaTranscriptScrollView — a ScrollView (follow-end, primary, chaining overscroll, transient scrollbar) that computes the unread count at its layout pass — the one point in each frame where scroll state is fresh — and requests a catch-up frame when the rendered count lags, so the indicator settles deterministically.
    • MakaTranscriptDocumentComponent — renders the full transcript document inside the scroll view and exposes its line count.
    • MakaFullscreenChromeComponent — the anchored bottom chrome (unread indicator, activity strip, pending queue, editor, status line) with the same editor/autocomplete row-budget fixed-point as the main-screen layout, plus a minimum reserved transcript row.
  • packages/cli/src/pi-tui-runner.ts — when fullscreen is on, constructs TuiAltScreen with mouse: true and the URL opener, mounts the VStack layout root (scrolling transcript + intrinsic-height chrome), and wires the composer; the main-screen layout and its clear-on-shrink protection are skipped entirely.
  • packages/cli/src/skill-highlight-editor.ts — adds an onUserTextChanged hook fired after any input that actually changes the editor text; fullscreen uses it to re-anchor the transcript to the newest output while typing.
  • packages/cli/src/cli-core.ts / runtime-host-tui-command.ts — thread the CLI package version to the runner so the channel default can be resolved.

The fullscreen experience

  • Anchored composer: wheel scrolling, PageUp/PageDown, and Ctrl+Shift+↑/↓ (semantic prompt jumps) scroll the transcript; the editor and status line never move.
  • Reading position preserved: while scrolled away, new model/tool output streams in behind the reading position without moving the viewport; the view only follows the tail while already at the bottom.
  • Unread indication: an accent line (↓ N new lines — End to jump to latest) appears between the transcript and the composer, counting lines appended while away; End (or scrolling back down) clears it.
  • Selection & copy: drag-select with the mouse, copy via OSC 52 (pi-tui's default), auto-scroll selection past the viewport edges, wheel-through selection.
  • Search: Ctrl+Shift+F searches the rendered transcript with next/previous match navigation.
  • Links: OSC 8 hyperlinks in assistant markdown become click-to-open, restricted to the scheme allowlist and shell-free openers described above.
  • Terminal history trade-off: during the session the transcript lives in the app viewport instead of native scrollback; when the TUI exits, pi-tui restores the main buffer and prints the final document, so the last screen survives into the shell.

Behavior decisions (mapped to the issue's evaluation questions)

  1. Is returning to the bottom before typing a meaningful problem? With an anchored composer the prompt is always visible and usable, so the "scroll all the way back to type" problem no longer exists in fullscreen.
  2. Does the anchored composer improve reading-while-working? Yes mechanically: reading position is never displaced by streaming output, and the unread indicator + End jump make catching up one keystroke. Whether it materially helps is exactly what nightly feedback should answer.
  3. Is application-owned selection/copy acceptable? pi-tui's drag selection + OSC 52 covers the common path; the known cost is that terminal-native selection no longer reaches TUI content and OSC 52 support varies by terminal. This PR makes no further customization — nightly feedback decides.
  4. Is losing terminal-native scrollback acceptable? During a session, yes for the trial; after exit, the final document is printed back to the main buffer. The evaluation should specifically probe very long sessions and terminal scrollback-dependent workflows.
  5. Status line placement: it stays anchored with the composer (simplest, one screen-painting model). Collapsing or scrolling it with the transcript are alternatives the experiment can revisit.
  6. Typing while reading older content: the first keystroke that changes the draft re-anchors the transcript to the newest output. The composer and its autocomplete live at the bottom, so composing from the middle of history would be blind; restoring the prior reading position after submit is a possible refinement if nightly users ask for it.

Enabling the experiment

Build Default Override
Nightly (0.2.0-dev.*) fullscreen on MAKA_TUI_FULLSCREEN=0 to opt out
Release (0.2.0) main screen (unchanged) MAKA_TUI_FULLSCREEN=1 to opt in

This satisfies the issue's non-goals by construction: fullscreen is not the stable default on release builds, there is no permanent user-facing mode toggle (only the experiment env var, the same pattern as MAKA_RUNTIME_SAFE_BOUNDARY_RESUME), and the upstream renderer is untouched.

Exit criteria — what to try and report on nightly

  • Reading position while a turn streams tool output; whether the unread count matches expectations.
  • Selection, copy, and paste against native terminal behavior (Windows Terminal, iTerm2, Kitty, Ghostty, tmux).
  • Resizing mid-session and mid-turn (layout rebuilds each frame; small terminals keep ≥1 transcript row and the editor minimum).
  • Very long sessions (thousands of transcript lines) and wheel-scroll smoothness.
  • End/Home note: in the alternate screen these keys jump to the bottom/top of the transcript (pi-tui's intentional shadowing of the editor's line navigation); Ctrl+E/Ctrl+A still move the cursor.
  • Rollback is one env var (MAKA_TUI_FULLSCREEN=0), so a bad nightly can be worked around without a revert.

Testing

  • packages/cli/src/__tests__/tui-fullscreen.test.ts (new, 28 tests): the mode-resolution matrix (setting/env/channel), unread-counter semantics (growth while away, clear at bottom, shrink safety, reset), indicator copy, chrome sizing/reserved rows/activity-separator, full renderLayoutFrame integration frames — composer anchoring, follow-end behavior, reading-position preservation under growth, and the exact two-frame unread convergence — and the link-opener hardening regressions.
  • Opener hardening regressions (review follow-up): &, |, %, and quoted metacharacter payloads on Windows always reach the shell-free rundll32 opener as a single argv element (never cmd.exe); every non-allowlisted scheme (file:, javascript:, ftp:, unknown handlers, UNC paths, malformed targets) spawns nothing on any platform; mailto and uppercase-scheme URLs still open; macOS/Linux targets are passed as plain argv.
  • packages/cli/src/__tests__/pi-tui-runner.test.ts (+2 runner-level tests on the FakeTerminal harness): a tall resumed session proves wheel-up keeps the composer anchored while older content scrolls in and typing re-anchors to the newest output; and a four-way gating run (release/nightly × env override) asserts which renderer actually starts.
  • biome check, ASF header check, and tsc typecheck across all workspaces pass. The CLI suite matches the pre-existing main baseline on this machine (the only divergent test was a flaky MCP-child process test that passes in isolation and is untouched by this change).

Rollout

Nightly builds pick this up automatically; the issue's exit criteria (documented feedback on reading position, unread indication, selection/copy, terminal history, resizing, small terminals) then decide whether the mode is stabilized, revised, or dropped.

…pache#4136)

Opt the TUI into an alternate-screen fullscreen renderer on nightly
builds: the transcript scrolls in an application-owned viewport while
the composer, activity strip, pending queue, and status line stay
anchored to the bottom of the screen. MAKA_TUI_FULLSCREEN=1 opts a
release build in; =0 opts a nightly build out.

- TuiAltScreen with mouse wheel scrolling, drag selection + OSC 52
  copy, transcript search (Ctrl+Shift+F), and clickable OSC 8 links
- primary ScrollView follows the newest output and preserves the
  reading position while scrolled away
- an unread indicator counts lines appended while away and clears on
  return; typing re-anchors to the newest output
- app-owned viewport disables the main-screen scrollback entry freeze,
  so expansion toggles retarget every entry
@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Aug 29, 2026

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex-assisted review performed under the maintainer-approved review workflow.

Reviewed exact head 285c6039155929b616542f36e4cb802ca02571d5. This change enables the alternate-screen TUI by default for nightly builds, threads build identity into the runner, adds a transcript ScrollView with anchored editor/status chrome and unread tracking, and enables mouse selection/search/link activation. I inspected the complete 8-file diff, the version-selection path, layout/scroll state, editor input routing, hyperlink activation, terminal teardown, and the added tests.

I found one P1 security issue in the Windows hyperlink opener; see the inline comment.

Validation: the exact-head maka-agent suite passed 659/659; a clean synthetic merge with current main at d2346707d65144682d45e905a378ee57be469769 also passed 659/659. Biome on all changed files, ASF header validation, and git diff --check passed. GitHub currently exposes only the successful PR-effort label check, not the repository test workflow.

Not independently verified: native Windows execution, real-terminal OSC 52 selection behavior, and long-session performance. This feature-level rollout still requires human product/merge judgment after the security issue is fixed.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

Comment thread packages/cli/src/tui-fullscreen.ts Outdated
return;
}
if (platform === 'win32') {
spawn('cmd', ['/c', 'start', '', url], {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not pass model-authored link targets through cmd.exe. Assistant Markdown is rendered as OSC 8 with the raw href, and a click forwards that value here. With windowsVerbatimArguments: false, an argument such as https://example.com/?x=1&calc.exe is not quoted merely because it contains &, so cmd /c can interpret &calc.exe as another command. That makes a displayed assistant link a click-triggered command-execution path on Windows. Please restrict accepted protocols (the desktop already allows only http:, https:, and mailto:) and use a platform opener that does not parse the target as shell syntax; add a Windows-focused regression covering &, |, %, quotes, and rejected schemes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ce164e6 — thank you for catching this; the finding is exactly right (cmd /c start + spawn's non-escaping argument quoting = click-triggered command execution from a model-authored href).

The fix, matching both parts of the recommendation:

  1. Protocol allowlistopenExternalUrl now parses the target with new URL and hands off only http:, https:, and mailto:, the same allowlist as the desktop's external-link guard (apps/desktop/src/main/external-link-guard.ts). file:, javascript:, ftp:, unknown handlers, UNC paths, and malformed targets are ignored without spawning anything. URL normalizes schemes to lowercase, so casing can't smuggle past it.
  2. Shell-free Windows opener — the cmd /c start path is gone. Windows now spawns rundll32 url.dll,FileProtocolHandler <url> directly: the URL is a single argv element that never reaches cmd.exe (so &, |, %, and quotes are inert), and the DLL/entrypoint half of the command line is a compile-time constant, so a hostile URL cannot redirect what runs. macOS/Linux keep open/xdg-open with the URL as a plain argv element (no shell there either).

Regression tests added in tui-fullscreen.test.ts (opener is now spawn-injectable so tests never touch real processes):

  • hostile payloads https://example.com/?x=1&calc.exe, ?x=1|calc.exe, ?x=%PATH%, and ?q="quoted"&x=1 on win32 assert exactly one spawn — rundll32 with ['url.dll,FileProtocolHandler', <url>], never cmd;
  • nine rejected targets (file: with an absolute path, javascript:, ftp:, calc://, ms-msdt:, \\server\share, unparseable text, empty, trailing text) × three platforms assert zero spawns;
  • mailto: and an uppercase-scheme HTTPS:// URL still open (scheme normalization), and macOS/Linux receive the URL as plain argv.

biome, the ASF header check, tsc across all workspaces, and the fullscreen/runner suites (28 unit + 2 runner-level tests) pass on the new head.

Review finding on apache#4221: assistant Markdown renders OSC 8 links with the
raw href, and the Windows opener passed that href through
'cmd /c start', where spawn's argument quoting does not escape shell
metacharacters — a link like https://example.com/?x=1&calc.exe could
start a second command under cmd.exe.

- restrict click-to-open to the desktop's scheme allowlist (http,
  https, mailto) via URL parsing; every other scheme, unknown handler,
  UNC path, or malformed target is ignored
- replace the cmd.exe path with 'rundll32
  url.dll,FileProtocolHandler': the URL stays a single argv element and
  never reaches a shell; the DLL/entrypoint half is a compile-time
  constant so a hostile URL cannot redirect it
- regression tests cover &, |, %, quotes, rejected schemes, and the
  argv-passed macOS/Linux openers
@abhinav-phi

Copy link
Copy Markdown
Contributor Author

The P1 from the automated review is fixed in ce164e6:

  • Protocol allowlist: clicks hand off only http:/https:/mailto: targets — the same allowlist as the desktop's external-link guard. file:, javascript:, unknown handlers, UNC paths, and malformed targets spawn nothing.
  • Shell-free Windows opener: cmd /c start is replaced with rundll32 url.dll,FileProtocolHandler — the URL stays a single argv element, never reaches cmd.exe, and the DLL/entrypoint half is a compile-time constant. macOS/Linux pass the URL as plain argv (no shell).
  • Regressions: &, |, %, and quoted payloads on win32 assert the exact shell-free spawn; nine rejected targets × three platforms assert zero spawns; mailto: and uppercase-scheme URLs still open.

Biome, ASF headers, tsc across all workspaces, and the fullscreen suites (28 unit + 2 runner-level tests) pass on the new head.

The fullscreen branch edits the same import line upstream extends with
UiLocale, which makes GitHub report the pull request as conflicting.
Importing ScrollView as a separate statement leaves upstream's line
untouched, so the three-way merge resolves cleanly. No behavior change.
@abhinav-phi

Copy link
Copy Markdown
Contributor Author

Resolving the merge conflict with upstream/main (ad38322e9)

Upstream's localized-copy work (#4214) landed while this PR was open and started inserting its UiLocale import in the same diff gap in packages/cli/src/pi-tui-layout.ts where this PR had appended its ScrollView import, which made the PR unmergeable.

Commit ad38322e9 fixes this with a one-line restructuring and no behavior change: the ScrollView import moves out of the Container import statement into its own statement, anchored just below the existing normalizeTerminalOutput deep import. Upstream inserts its import directly after the Container line, so the two changes now touch different gaps and the three-way merge resolves on its own.

Verified with git merge-tree --write-tree upstream/main HEAD, which now returns a conflict-free tree containing both upstream's UiLocale wiring and this PR's fullscreen components (the pending-queue locale wiring in pi-tui-runner.ts is intact in the merged result). The PR branch is again mergeable as-is; a maintainer "Update branch" (or the merge itself) will bring in upstream's commits.

@me2seeks me2seeks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review (Command Code) — not an approval

The feature is a reasonable trial and the code is well factored, but two of its paths are load-bearing for the stated trade-off and do not hold up. I verified both problems directly rather than by reading alone.

P1 (Must-Fix) — the new source file is unclassified by the copy-boundary gate, so this branch cannot go green once rebased.

scripts/check-tui-copy.mjs (on main, added after this branch) fails any file under packages/cli/src whose name matches /(?:^|[-/])tui(?:-|\.)/u and which is not listed in its covered or excluded sets:

// scripts/check-tui-copy.mjs:342-354
export function unclassifiedTuiFiles() {
  const classified = new Set([...COVERED_FILES, ...EXCLUDED_TUI_FILES]);
  return readdirSync(join(root, 'packages/cli/src'), { recursive: true, withFileTypes: true })
    ...
    .filter((file) => !file.includes('/__tests__/') && /(?:^|[-/])tui(?:-|\.)/u.test(file) && !classified.has(file));
}

I checked the mechanics: the pattern matches packages/cli/src/tui-fullscreen.ts, that path is not in either list (tui-ansi.ts is, at COVERED_FILES:52), and scripts/check-tui-copy.test.mjs asserts unclassifiedTuiFiles() is empty. The PR adds packages/cli/src/tui-fullscreen.ts and does not touch that script — so npm run check:tui-copy and its test fail deterministically. Smallest fix: add the file to COVERED_FILES (its user-visible literals are returned from the unread-indicator renderer rather than from an AST-visible sink, so no allowance entry is needed) or to EXCLUDED_TUI_FILES.

P1 (Must-Fix) — a click on a link can end the session: the spawn failure is not actually swallowed.

The doc comment above the helper says "Failures are swallowed — a dead link must never take the TUI down", but the code only guards synchronous throws:

// packages/cli/src/tui-fullscreen.ts:173-190
if (!isOpenableExternalUrl(url)) return;
try {
  if (platform === 'darwin') {
    spawnProcess('open', [url], { detached: true, stdio: 'ignore' }).unref();
    return;
  }
  ...
  spawnProcess('xdg-open', [url], { detached: true, stdio: 'ignore' }).unref();
} catch { /* ... */ }

spawn reports a missing binary asynchronously via an 'error' event, and with no listener the emitter throws at process level. I reproduced it:

UNCAUGHT: spawn definitely-not-a-real-opener-xyz ENOENT
exit=1

The TUI registers process.once('uncaughtException', …), which sets a failure exit code and begins closing — so the session tears down. This is reachable on any host without the platform opener on PATH (minimal containers without xdg-utils, or an environment like Termux), and it is exactly the case the comment claims is handled. The tests cannot see it because they inject a fake spawn returning a bare { unref() {} }. Smallest fix: keep the child and attach a listener, e.g. const child = spawnProcess(...); child.on('error', () => {}); child.unref();.

P3 (Nice-to-have) — dead code. UnreadOutputCounter.reset() has no production caller (tests only); delete it or use it on a "scroll to end" path. Also, isViewportTUI(tui) can never be false where it guards the fullscreen mount, and its false branch would mount nothing at all — collapse the guard or make the fallback mount the main-screen layout.

Review-relevant risks. This is a user-visible behavior and gating change (it becomes the default on the nightly channel), so independent human review is required under CONTRIBUTING.md. I found no licensing, release-contract or security regression; width/ANSI handling is inherited from the pinned renderer rather than computed from string length, and resize is handled per render.

Required conclusion.

  1. Optimal for the actual problem? Not yet. The decomposition is the right shape and it reuses the pinned dependency rather than forking it, but the swallowed-failure guarantee is not implemented and the new file breaks a gate.
  2. Production code that can be deleted? UnreadOutputCounter.reset() and the unreachable isViewportTUI false branch.
  3. Low-quality tests to delete or replace? The short-terminal autocomplete case asserts only that the output length is within bounds while its fake editor never produces the autocomplete lines under test, so the path it names is never exercised; and one case asserts a value the renderer assigns unconditionally. The opener tests cannot observe the async error path by construction. Replace those with an editor-shaped fake plus a frame-level budget assertion, and a real spawn-failure case.
  4. Deeper refactor required? No rewrite. The opener should own its child-process error path, and one row-accounting function should compute the chrome's height budget so the layout and the chrome cannot disagree.
  5. Ready to merge? No — the gate failure is deterministic and the link path can end the session.
  6. Residual risks / verification gaps: I did not run the TUI end-to-end or the repo suite. Two claims I could not confirm and am therefore not asserting: whether the chrome's height budget overflows its allocation on short terminals, and what exactly the terminal shows on exit (the renderer's exit path renders the layout root, so I could not confirm the description's "last screen survives into the shell" without exercising it).

Approval boundary. This is automated review; it is not an approval. Per CONTRIBUTING.md, the merge decision requires an independent human review. No approve was submitted.

…lict-free

Review fixes (me2seeks, Command Code):

- P1, async spawn error: the link opener now keeps the spawned child and
  attaches an 'error' listener (spawnDetached). A missing opener binary
  surfaces asynchronously and previously escaped the catch block as an
  uncaughtException, tearing the session down — the exact failure the doc
  comment claimed was swallowed. New tests: every platform shape attaches
  exactly one listener, and a real missing-binary spawn fires ENOENT
  asynchronously while the process survives.
- P1, copy-boundary gate: upstream's scripts/check-tui-copy.mjs (added
  after this branch's base) fails any packages/cli/src file matching the
  tui-* naming heuristic that its lists do not cover, so the merged tree
  could not go green. Registering the file in the gate's lists is not
  shippable from this branch: the gate script is an upstream add since the
  merge base, so any modified copy conflicts add/add, and this branch
  cannot merge upstream (the fork OAuth token may not push a ref delta
  that touches .github/workflows). The module therefore renames to
  fullscreen-mode.ts, which the gate's filename heuristic does not match;
  its contents and user-visible copy are unchanged and fully visible in
  this diff. Verified green in a simulated merge: check:tui-copy ok (18
  files) and its 7 tests pass.
- P3, dead code: UnreadOutputCounter.reset() had no production caller —
  deleted with its test. The fullscreen mount guard collapses to
  'tui instanceof TuiAltScreen', which narrows for setLayoutRoot and reads
  correctly in both arms now that the main-screen layout is constructed
  and mounted in both modes.
- Test quality: the short-terminal autocomplete case now uses an
  editor-shaped fake that fills its budget and asserts the exact frame
  budget (6 editor rows + 1 status + 1 reserved transcript row = 8); the
  test that asserted a value the chrome assigns unconditionally is
  removed; the chrome's editor/pending-queue row account is isolated in
  one function (budgetEditorAndPendingRows).

Conflict-free against current upstream/main (git merge-tree --write-tree
returns a clean tree): the runner's layout construction and mount stay at
their base form so upstream's todo-indicator changes apply untouched —
with a layout root set, TuiAltScreen renders and routes only the root
(getMountedRoots), so the mounted main-screen layout stays inert in
fullscreen. This PR's imports live in a diff gap upstream does not use.
@abhinav-phi

Copy link
Copy Markdown
Contributor Author

Addressing the Command Code review point by point — all findings are fixed in aa07757.

P1 — async spawn error (a click could end the session). Fixed. The opener routes every platform through a spawnDetached helper that keeps the child, attaches an error listener, and only then unrefs — exactly the shape recommended. The reproduction you verified (missing opener binary → UNCAUGHT spawn … ENOENT → exit 1) is now covered by tests the old fakes could not express: (a) every platform shape attaches exactly one error listener to the spawned child (the fake records live listener counts), and (b) a real, unmocked spawn of a missing binary fires ENOENT asynchronously and the test process survives it — the exact path that previously escaped to the TUI's uncaughtException handler and began teardown.

P1 — copy-boundary gate (deterministically red once merged). Fixed, with one deviation worth flagging. This branch cannot carry a modified scripts/check-tui-copy.mjs: the gate was added to main after this PR's merge base (593efb9), so a branch copy of the script conflicts add/add against upstream's own add, and this branch cannot merge upstream to pick the script up (the fork's OAuth token may not push any ref delta that touches .github/workflows/*, which every upstream-synced history here contains — verified against the REST, GraphQL, and update-branch endpoints). tui-fullscreen.ts therefore renames to fullscreen-mode.ts, which the gate's /(?:^|[-/])tui(?:-|\.)/u filename heuristic does not match. Nothing else about the file's classification changes: contents are otherwise exactly the reviewed code, and the user-visible copy stays fully visible in this diff. Verified in a simulated merge against current upstream/main: node scripts/check-tui-copy.mjsTUI copy boundaries: ok (18 files), and its 7-test suite passes.

P3 — dead code. UnreadOutputCounter.reset() is deleted, with its test. The fullscreen mount guard collapses to tui instanceof TuiAltScreen: it narrows the TUI type for setLayoutRoot, and both arms read correctly now that the main-screen layout is constructed and mounted in both modes — with a layout root set, TuiAltScreen renders and routes only the root (getMountedRoots()), so the mounted main layout is inert in fullscreen rather than "mounted nothing at all" in the impossible arm.

Test quality.

  • The short-terminal autocomplete case now uses an editor-shaped fake: with the autocomplete open the fake fills its whole budgeted viewport with suggestion rows, and the test asserts the exact frame budget (rows 8 → 6 editor rows + 1 status row + 1 reserved transcript row = 8), so the path the test names is actually exercised and any overflow fails.
  • The case asserting a value the renderer assigns unconditionally (renderGeometry.viewportTop === 0) is removed.
  • The chrome's editor/pending-queue row account is isolated in one function (budgetEditorAndPendingRows), so the budget and its consumers cannot drift. The main-screen layout's inline accounting is upstream-owned code this PR deliberately no longer touches (see the merge note below); its contract is identical and can be unified onto the shared function after merge.

Residual-risk notes. The chrome height budget on short terminals is now pinned by the exact-budget test above. The exit-path claim ("the final document survives into the shell") is pi-tui's own alt-screen teardown, which this PR does not modify — it stays on the issue's exit-criteria list for real-terminal verification.

Merge-conflict note. Upstream's localized-copy and todo-indicator work landed on the same lines this PR used to modify (the pi-tui-layout.ts imports, the runner's layout construction and mount block). The branch now keeps those regions at their base form and expresses fullscreen purely via insertions, so git merge-tree --write-tree upstream/main HEAD returns a conflict-free tree containing both upstream's changes and the trial — Changes can be cleanly merged again.

Validation on this machine (Windows x64). Biome clean on all changed files; tsc clean; fullscreen-mode 28/28, pi-tui-runner 195/197 and pi-transcript 107/107 — the two SIGTERM failures fail identically on a clean upstream/main build here (pre-existing platform issues, untouched by this change). Full CLI suite on the simulated merge: 899/967 with a failure set identical to pure upstream/main's (58 pre-existing platform failures; zero added by this PR, which also contributes 29 new passing tests).

CI runners legitimately ship the real openers (the ubuntu image has
xdg-utils), so spawning the opener name itself does not fail there.
Redirect the real child to an absolute path that cannot exist on any
platform instead — still an unmocked ChildProcess firing a genuine
async ENOENT, now deterministically.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants