Skip to content

feat(desktop): add terminal surface crate with libghostty-vt spike - #862

Open
roodboi wants to merge 17 commits into
nextfrom
feat/desktop-term-surface
Open

roodboi wants to merge 17 commits into
nextfrom
feat/desktop-term-surface

Conversation

@roodboi

@roodboi roodboi commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Unit B of the desktop app (HACK-2252, child of HACK-2214): the terminal surface, now a per-project bottom panel with tabs (owner decision, Nov 15 2026). Linear status is reported here because Linear may be unreachable.

Crate apps/desktop/term (hack-desktop-term):

  • pty, session, stream: an app-owned PTY whose child leads its own session; closing it ends the session and never blocks the caller. Read-only byte sources for logs.
  • launch: what a tab is (logs, container shell, host shell, host shell with env), its label and badge, and the command each kind runs.
  • ghostty_config: the user's Ghostty font, colours, cursor and padding, read from their config.
  • emulator, terminal, view, panel, actions (feature ghostty): libghostty-vt 1.0.1 state painted by a GPUI cell renderer, in a tabbed panel with keyboard actions.
  • sprite, glyphs: box drawing, blocks and powerline separators drawn as cell-exact geometry; other prompt symbols filled from a bundled symbols font, so they do not depend on the user's font or the platform's font fallback.
  • Examples: term_spike (all four tab kinds in one panel) and term_bench.
  • term/DECISION.md: research, comparison, why not option (a), build and CI, sessions, CLI requests, ownership, tests, performance, uses, security rules, licences.

CLI requests

Three small CLI changes would close gaps the app must not work around:

  1. hack env list --json: add overlays, the names of the overlays that exist. Today the payload has the selection (env_selection) and the scopes, but only the selected overlay's files, so the app cannot offer a list of overlays to open a shell with. Names only.
  2. hack host shell: when SHELL is unset or empty, use the user's passwd shell before falling back to /bin/sh. Run for real with SHELL unset, it started /bin/sh.
  3. hack exec: document and test that a command that cannot be started in the container exits 127 (or 126) on both backends. The bash-to-sh fallback depends on it, and today it is the engine's behaviour, not the CLI's contract.

Reviewer round at 330f84a8: one search pass per change

With find open, every render ran the libghostty search twice (once for the count, once inside frame()), and each run checks every match it holds. Fixed in e518f0bf.

  1. One pass per frame. frame() and search_refresh() share one pass, and Frame now carries the SearchState its highlights were built with.

  2. No pass when nothing changed. The emulator keeps a revision counter, bumped by output, a resize, a viewport move, a new needle and a new selected match. The search runs at most once per revision and the state is reused until the next one.

  3. Regression tests. Emulator::search_runs() counts runs. the_search_runs_once_per_change_and_not_at_all_for_an_unchanged_redraw (emulator) covers unchanged redraws, no-op calls, and exactly one pass after output, resize, scroll, follow and a step. redrawing_an_unchanged_view_with_find_open_does_not_run_the_search (view) draws three more frames and requires zero runs. Tried against three faults (cache never used; output not counted; scroll not counted): each fails a test.

  4. Timing, same dense case (dev build, 10,000-line scrollback, 390,000 matches, unchanged terminal, medians of 30):

    Before (330f84a8) After (e518f0bf)
    Count, then frame 19.4 to 20.5 ms 1.8 ms
    Frame alone 11.2 to 11.6 ms 1.8 ms

    A frame after one new line of output takes 12.4 ms: one run, where it was two. That run is libghostty's own cost for this many matches and is not made incremental here.

  5. The quiet-source count regression still passes, unchanged.

One thing fixed on the way, flagged: libghostty lists matches that share a page with the viewport but lie below it, and those were put in the frame as highlights on rows that are not on screen. They are now left out, so Rendered::highlights is the number on screen as documented.

Bot thread at a5d5868a: the find count lagged behind the highlights

Fixed in 330f84a8, replied on the thread and resolved it.

  • Bug: with find open, output that added matches updated the highlights but not the count beside the find field, because the view copied the search's state before the search had run on the new output. If the source then went quiet, the stale count stayed on screen.
  • Fix: the view runs the search in render, before reading the state the overlay shows (Emulator::search_refresh), so the count and the frame's highlights come from the same pass. If a resize during layout changes the count, the view requests another frame.
  • Regression: the_find_count_matches_the_highlights_when_output_arrives_and_then_stops (tests/view.rs). With the old copy put back it fails with a count of 1 beside 3 highlights. Rendered now reports the find state and the number of highlights a frame drew, which the test reads.
  • Not covered by a test: the extra frame after a resize that changes the count. I found no way to make a resize change the match count in a test.

Reviewer round at 610cb216 (three items)

  1. Self-drawn glyphs stay inside their cells (933705dd).
    • A symbol from the bundled font is now placed by its ink box, not by the em and ascender alone: the scale is reduced until the ink fits its room, and the glyph is moved by as little as brings it inside (SymbolOutline::place). U+F0DC and U+F0DE, whose ink rises above the ascender, now sit wholly inside their room.
    • Everything self-drawn is clipped to its room: a symbol to its one or two cells; box drawing, blocks and separators to their one cell. A diagonal's stroke is cut off at the cell's edges.
    • Tests: a unit test places every glyph of the font (over 10,000) in eight room sizes and checks the ink is inside. The render test now has blank cells and rows around every glyph and fails on any pixel outside a glyph's room; it includes U+F0DC, U+F0DE, U+2571 and U+2573. It also checks each symbol's ink box against the placement rule to a pixel and a half.
  2. A double-width symbol uses both of its cells. The room is decided by symbol_cells: the emulator's two cells first, then the blank-cell rule for a narrow symbol. A double-width symbol is not widened into a third cell (flagged: it already has the two cells that rule exists to give). New render test a_double_width_symbol_is_drawn_in_both_of_its_cells for U+26A1, plus a unit test of the rule.
  3. Font Logos is under the Unlicense. I read the v1.3.0 LICENSE: it is the Unlicense, a public-domain dedication. DECISION.md records that with the link; the pending-verification claim and the owner decision are removed, here too.

One addition, flagged: TermFont gains symbols: BundledSymbols (WhenMissing by default, Always to draw every bundled symbol from the bundled font). The double-width test needs it, because many fonts have U+26A1 and the default rule would then use the terminal's font. Default behaviour is unchanged.

Each new check was tried against the fault it is for: without the sprite clip the diagonal paints 4 pixels outside its cell; with the old placement U+F0DC is cut off at the top of its room and the unit test fails; with a one-cell room the double-width symbol is half the size. Not separately detectable: removing only the symbol clip, because the placement already keeps the ink inside.

Three renders changed slightly (host dark, host light, host env): the folder icon is fitted by its ink. The other three are byte-identical.

First Mac result at 410b0590: prompt symbols drew as boxed question marks

On Apple silicon (macOS 26.5.1) provisioning, the build, the window and --screenshots worked, but powerline and Nerd Font symbols in the prompt drew as missing-glyph boxes. Fixed in b33c5421; the fix has not been run on a Mac yet.

  1. Glyph fallback, the way Ghostty does it.
    • libghostty-vt gives no help here: it exports no font, glyph or sprite API. It reports the code point in each cell and nothing about how to draw it.
    • Ghostty itself draws box drawing, blocks and the powerline separators as geometry and embeds a Nerd Fonts symbols font for the rest. Its sprite code is MIT, but it is Zig that rasterises into Ghostty's own canvas, so it is not reusable as code. This crate follows the same design with its own implementation.
    • sprite: U+2500 to U+259F (box drawing, block elements) and U+E0B0 to U+E0BF (powerline separators) are drawn as rectangles and paths on whole device pixels. Neighbouring cells join with no seam at any font size. Cell backgrounds, selection and the cursor now use the same pixel edges.
    • glyphs: every other symbol comes from Symbols Nerd Font Mono 3.5.1 when the terminal's font lacks it. A private-use code point always comes from the bundled font unless the terminal font is itself a Nerd Font and has the glyph.
  2. DECISION.md has a new section, "Glyphs: prompt symbols, powerline and box drawing": what libghostty-vt gives, what Ghostty does, what this crate does, sizing, limits, licences and size.
  3. Render test tests/render.rs (extended in the round above): draws the two solid arrows, branch U+E0A0, U+F418, U+E718, U+F07B, four box-drawing characters, two blocks and a known-missing code point offscreen. It fails if a cell is blank, matches the missing-glyph render, equals another icon, is a speck, did not come from the bundled font, does not have the bundled glyph's proportions, or (for the geometry) does not fit its cell to the pixel. Checked by breaking the code five ways; each break fails the test.
  4. The six PNGs are re-rendered and inspected: separators meet their neighbours with no seam, branch and folder icons are the right glyphs.

Departures from the suggested default, flagged:

  • The bundled font is not loaded with GPUI add_fonts. GPUI's Linux text system refuses any font with no glyph for "m", so a symbols-only font can never be selected there, and a system font on the development machine defined U+E0A0 as an unrelated glyph, which is what got drawn. Instead the crate reads the outlines with ttf-parser and fills them as GPUI vector paths. This is the same code on macOS and Linux and uses no platform font fallback, which is the part that failed on the Mac.
  • U+E0A0 to U+E0A3 (branch, line number, lock) come from the bundled font, not hand-drawn geometry. Ghostty does the same.
  • The font is provisioned, not committed. provision-ghostty.sh fetches the release archive and checks two sha256 sums (archive and font file); build.rs checks the font file again and embeds it. It adds 2.6 MB to a binary built with the ghostty feature. The lead must run provision-ghostty.sh again to get it.
  • A narrow symbol followed by a blank cell uses both cells, as Ghostty does, so prompt icons are not squeezed. Otherwise it is fitted to one cell. A double-width symbol has its two cells.
  • CI: the term job now installs Mesa's software Vulkan driver and sets HACK_TERM_REQUIRE_RENDERER=1, so the pixel test must run there. Without that variable the test reports "NOT RUN" and passes on a machine with no renderer.

Licence: the Nerd Fonts archive is MIT. The font combines icon sets under their own licences: Font Awesome and Codicons (CC BY 4.0), Material Design Icons (Apache-2.0), Pomicons and Weather Icons (SIL OFL 1.1), Font Logos (the Unlicense, public domain), Devicons, Octicons, Powerline symbols, Seti-UI and others (MIT). All allow embedding; the app's licence notices must list them for attribution.

Reviewer round at f6e8aced (four items)

# Finding Fix Test
1 A test helper still checked a pid and then killed it assert_no_trace only reports the pid; it signals nothing. A test that may leave a process running holds a pidfd handle for that session, which cleans up when the test unwinds. The spawn-failure tests use it; no helper signals a number any more.
2 The Ghostty config reader was not bounded end to end A file is identified by its canonical path before it is opened and is never read twice. It is opened once (non-blocking, regular files only, checked on the handle) and read through a 1 MiB limit on that handle. One load reads at most 4 MiB and 64 files across config files, includes and the theme. A fanned-out include tree (three includes of the next level per level, plus back-references and alternate spellings) reads each file once and no byte twice. A reader that never ends is cut off at the limit plus one byte. A FIFO in place of the config is not read and does not block. Byte and file budgets. Each fails when its fix is removed.
3 Shortcuts were hard-coded in the crate The view binds no keys. Copy, Paste, Find, FindOlder, FindNewer, CloseFind and the scroll and follow actions are actions with public methods. The view keeps only key forwarding to the PTY and text entry for its find field. The example binds the design's keys per platform (table below). Headless GPUI: with nothing bound, Ctrl-Shift-F and ⌘F do not open find; the Find, CloseFind, Paste and Copy actions work; after the app binds Ctrl-Shift-F to Find, the key opens it.
4 An empty palette = should reset the palette The reset is explicit in the parsed settings and applied when the theme is merged: it drops every entry set so far, the theme's included, and later entries apply. Reset after earlier entries, after a theme's entries, with entries after it, and inside a theme. Fails without the fix.

Bindings, in the example and for the app's keymap (from the app's design):

Action macOS Linux
Toggle panel ⌘J Ctrl-Shift-J
New tab ⌘T Ctrl-Shift-T
Close tab ⌘W Ctrl-Shift-W
Next / previous tab ⌘⇧] / ⌘⇧[ Ctrl-Shift-PageDown / Ctrl-Shift-PageUp
Maximize ⌘⇧↩ Ctrl-Shift-Enter
Find ⌘F Ctrl-Shift-F
Copy / paste ⌘C / ⌘V Ctrl-Shift-C / Ctrl-Shift-V

The example also binds keys the design does not list yet, as its own choice (flagged): Enter, Shift-Enter and Escape in the find field; Shift-PageUp and Shift-PageDown for scrollback; PageUp, PageDown, Home, End and f in a log view. Without those bindings an app has no key for them.

Reviewer round at 82578360 (three items, accepted)

# Finding Fix Test
1 Verify-then-kill(pid) is still a reuse race (macOS, and the Linux fallback without pidfd) Fail closed. A signal goes only through a handle whose identity cannot be recycled: killpg on the anchored leader group on every platform, and a pidfd for other session members on Linux. Where there is no such handle (macOS, Linux before 5.3), members outside the leader group are never signalled by number; they are found by a session scan and returned in Teardown::survivors. No kill(pid) after a check remains anywhere. Test cleanup uses pidfds on Linux and signals nothing on macOS. Fake process table: a pid replaced between verification and signalling receives nothing, and without handles no per-process signal is attempted. Real processes: a member killed and reaped at that boundary is not signalled by number (Linux). With handles disabled, a job in another group survives, is reported, is not signalled, and teardown returns in under three seconds.
2 The paste-rejection notice might never draw Every input and every notice change notifies the view. Headless GPUI: a quiet child, a paste larger than the queue. Exactly one new frame is drawn, it contains the notice, and no other event occurred. Fails without the fix.
3 Process-backed streams had no awaited quit teardown Quit teardown is registered for both backend kinds and awaited by GPUI. Headless GPUI: App::shutdown with a stream process that ignores SIGTERM and has a descendant that does too. Both are gone and reaped when shutdown returns. Fails without the fix. Same for a PTY session that ignores SIGHUP.

The macOS limit this leaves (documented in session.rs and DECISION.md; now confirmed on hosted macOS, see below): a process in another process group of the session that ignores or never receives SIGHUP can outlive a closed tab, as it would outlive any terminal app. It is reported, not ended. The panel shows how many processes outlived closed tabs.

Owner requirements A to E

A. Sessions. The app owns the PTY and runs the CLI in it. It sets only TERM=xterm-256color, COLORTERM=truecolor and TERM_PROGRAM=hack, and passes only subcommands, validated names and a shell name. It never builds an environment and never sees a value.

Tab What runs, in the project root
Shell in a service hack exec <service> -- bash, then -- sh if bash cannot start (exit 126 or 127 straight away, before any input)
Host shell The user's login shell ($SHELL, else passwd, else /bin/sh) as a login shell by the argv[0] convention (-zsh), so profile and rc files load
Host shell with env hack host shell --env <overlay> --scope <scope>; the CLI injects the values and starts $SHELL -l

Checked against the CLI source on next, and run for real in a throwaway project with a temporary home:

  • hack host shell --env qa --scope api on a pseudo-terminal: the profile loaded, its prompt showed, the shell was interactive, global and service-scoped values were present, TERM_PROGRAM and COLORTERM came through, and the working directory was the project root. So it can start the user's interactive login shell with rc files when SHELL is set.
  • hack env list --json: returns env_selection (requested, default, effective), scopes, selected_scope, files, and vars with names and metadata and no value field.

B. Tab identity and badge. SessionKind gives logs, api · sh, host · zsh, and the badge env staging · api with a warning tone when the overlay is not the default. Names only.

C. Look. The user's Ghostty config is read for font family (with fallbacks), font size, background, foreground, palette, cursor style and padding. config-file includes and theme (name, absolute path, light:A,dark:B) are followed. Only those keys are parsed, nothing is logged, and anything unset falls back to the app's tokens.

D. Verified in the spike. Renders below; each item also has a test (table in DECISION.md).

E. Keyboard actions. Panel: TogglePanel, NewTab, CloseTab, NextTab, PrevTab, ToggleMaximize. Terminal: Copy, Paste, Find, FindOlder, FindNewer, CloseFind, and scroll and follow actions. All in the hack_term namespace. The crate binds no keys.

Renders (offscreen GPUI capture on Linux)

Host login shell with a themed prompt, truecolour, bold and italic, wide glyphs, prompt symbols from the bundled font, drawn separators and an OSC 8 link:

host

The same tab with a fixture Ghostty config applied (Nerd Font with real italics, theme colours, bar cursor, padding):

ghostty look

Host shell with env, and its badge (a stand-in script in place of hack host shell):

host env

Container shell after falling back from bash to sh (a stand-in in place of hack exec), and the read-only log stream with find:

container
logs

Light theme: panel-host-light.png.

Recommendation (unchanged)

libghostty-vt for emulation, cells drawn with GPUI (option b). The comparison table, "Why not (a), even though it matches Ghostty's pixels", what (b) still has to prove, and the optional spike of (a) are in DECISION.md.

flowchart LR
  subgraph Tabs
    L[Logs: read-only stream]
    C[Container shell: hack exec service -- bash or sh]
    H[Host shell: login shell]
    E[Host shell with env: hack host shell --env --scope]
  end
  L -->|bounded byte queue| EM[Emulator: libghostty-vt 1.0.1]
  C -->|app-owned PTY| EM
  H -->|app-owned PTY| EM
  E -->|app-owned PTY| EM
  K[Keys and paste] -->|bounded input queue| C
  K --> H
  K --> E
  EM -->|Frame| R[GPUI cells in the tabbed panel]
  G[User's Ghostty config] -->|font, colours, cursor, padding| R
Loading

First performance numbers (term_bench, release, one Linux desktop; a sanity check): about 220 MiB/s of emulation, about 10,000 full 200×60 repaints per second, and about 4 ms of CPU per full-screen change (0.9 ms frame build + 3 ms paint). Not GPU time, not key-to-screen latency, not a Mac.

Defaults I chose (flagged)

  1. TERM is xterm-256color everywhere. xterm-ghostty is available as an opt-in for host shells and is used only when its terminfo entry is installed. Reason: containers and ssh targets rarely have it.
  2. bash-to-sh fallback by exit status, not by probing first. A second attempt happens only on exit 126 or 127 within 10 s and before any input.
  3. Login shell by argv[0] (-zsh), not -l -i: every common shell honours it, and flags differ between shells.
  4. A host shell with env is labelled by the CLI's own rule ($SHELL, else sh) until request 2 lands.
  5. NewTab is an event, not a default: the app chooses the kind.
  6. macOS survivors are reported, not ended (reviewer's decision, implemented as specified).
  7. Unchanged from before: the ghostty feature is off by default; inputs are provisioned and checksummed, not vendored; apps/desktop/.cargo/config.toml makes the dependency fail instead of download; queues are 1 MiB; SIGKILL grace is 300 ms on close and 100 ms on quit.

Disclosure (unchanged): during the first spike, one pkill -f "^sleep 300$" was run on the development host. No pattern kills are used any more. Tests signal only through pidfds, or a child of the test process that is still unreaped (its pid is anchored).

A side effect on the development host during this round: one cargo run with an overridden home made rustup install toolchain 1.98.1 into the default rustup home. Nothing in the repository depends on it.

Licences

This round adds no package to Cargo.lock: ttf-parser 0.25.1 (MIT OR Apache-2.0) and sha2 0.10.9 (MIT OR Apache-2.0, build script only) were already there through GPUI and are now direct, exactly pinned dependencies of the ghostty feature. It adds one embedded asset, Symbols Nerd Font Mono 3.5.1 (see the licence note above). The table of every package this branch adds to Cargo.lock is in DECISION.md ("Licences of new dependencies"): ghostty-vt and ghostty-vt-sys (Apache-2.0, linking libghostty-vt, MIT), their build-script dependencies (MIT, Apache-2.0, CDLA-Permissive-2.0, CC0), and nothing copyleft. Zig 0.17.1 (MIT) is a build tool and is not shipped.

How to run

cd apps/desktop
eval "$(bash term/scripts/provision-ghostty.sh)"     # the only network step: libghostty inputs and the symbols font, checksummed
cargo run -p hack-desktop-term --features ghostty --example term_spike
cargo run -p hack-desktop-term --features screenshots --example term_spike -- --screenshots /tmp/shots
cargo run --release -p hack-desktop-term --features screenshots --example term_bench
bash scripts/test.sh                                         # what the existing CI jobs run
bash scripts/test.sh --features hack-desktop-term/screenshots
cargo fetch --locked && bash term/scripts/no-network.sh bash term/scripts/check-ghostty.sh   # Linux

Verified on Linux (x86_64), at e518f0bf4cf395f912edbd48c5496a22475fcaba

  • cargo fmt --all --check; cargo clippy --locked --all-targets -- -D warnings with no features, hack-desktop/screenshots, hack-desktop-term/ghostty and hack-desktop-term/screenshots; cargo build --locked -p hack-desktop.
  • bash scripts/test.sh: 169/169 default, 201/201 with hack-desktop-term/ghostty, 209/209 with hack-desktop-term/screenshots (both pixel tests required and passing).
  • no-network.sh check-ghostty.sh with no display variables set and the renderer required: build, both clippy runs and 97/97 crate tests, both pixel tests among them, in a namespace with only a down loopback.
  • next 081a5263 is merged in (cbc9a800); the two Bun contract tests passed with the pinned Bun at 610cb216 (89 pass). This round changed no TypeScript.
  • macOS type check: cargo clippy --target aarch64-apple-darwin -p hack-desktop-term --all-targets -- -D warnings passes (default features only; the ghostty feature cannot be cross-built here).
  • The six renders were re-captured, compared and inspected at a5d5868a; this round changed no painting code.

Verified on hosted CI (earlier heads)

All three desktop jobs passed on f6e8aced and again on 410b0590. Neither includes the glyph work. On 330f84a8 the Linux term job passed: it built offline, ran the software Vulkan driver and the required pixel tests, so the glyph tests and the find-count regression have run on a hosted Linux runner. The macOS job for that head was cancelled by the next push. The macOS job (macos-15) ran 136/136, including the tests whose macOS behaviour had never run before:

  • a_job_in_another_process_group_is_ended_or_reported: on macOS the job is left alone and named in survivors.
  • the_leader_stays_unreaped_until_signalling_is_done and the fake-table fail-closed tests.
  • zsh_loads_its_login_and_interactive_files_when_it_is_installed: real zsh as a login shell, profile and rc loaded, alias works.
  • a_host_shell_is_an_interactive_login_shell_that_loads_the_users_profile against macOS's sh.
  • The PTY, stream, queue, spawn-failure and final-output tests.

The offline Linux job ran 73/73 inside its network namespace.

Not verified

  • The glyph fix on macOS. It was written and tested on Linux only. It does not use the platform's font fallback, which is what failed, but nobody has looked at it on a Mac.
  • This head on hosted CI. The jobs for e518f0bf had only just started when this was written.
  • The ghostty feature has no macOS CI job. On a Mac it has been built and run only by the lead, at 410b0590, before any of the glyph work. ⌘ bindings were never pressed.
  • Real hack exec in a container: not run (no Docker in this work). The container tab was tested and rendered with a stand-in script.
  • The live window was not driven by hand; only offscreen capture, headless GPUI tests and the benchmark ran. X11 was not run.
  • The user's real Ghostty config: only fixture configs were read.
  • GPU time and key-to-screen latency.
  • Not implemented: IME, mouse reporting to programs, plain-URL detection, right-to-left text, braille and legacy-computing symbols as geometry, Ghostty's per-glyph size and alignment table for symbols, cursor-color and selection-* from the Ghostty config.

macOS commands for the lead

git fetch origin && git switch --detach e518f0bf4cf395f912edbd48c5496a22475fcaba
cd apps/desktop
bash ../../scripts/install-cargo-nextest.sh "$TMPDIR/nextest" && export PATH="$TMPDIR/nextest:$PATH"
bash scripts/test.sh                                   # default features

eval "$(bash term/scripts/provision-ghostty.sh)"       # run it again: it now also fetches the symbols font. Prebuilt XCFramework, checksummed, no Zig
cargo fetch --locked
HACK_TERM_REQUIRE_RENDERER=1 bash term/scripts/check-ghostty.sh   # cargo --offline: build, clippy x2, tests incl. headless view tests and the pixel test
cargo run -p hack-desktop-term --features screenshots --example term_spike -- --screenshots "$TMPDIR/shots"
cargo run --release -p hack-desktop-term --features screenshots --example term_bench

# The real thing, in a Hack project with a running service:
cargo run -p hack-desktop-term --features ghostty --example term_spike -- \
    --project /path/to/project --hack "$(command -v hack)" --service SERVICE --env OVERLAY --scope SERVICE

What to capture:

  1. Every summary line, and the name of any failing test. The default-feature tests have passed on hosted macOS; check-ghostty.sh (the emulator, terminal and headless view tests) has never run on a Mac.
  2. The six PNGs and the five term_bench lines.
  3. In the live window with the real CLI:
    • Keys: ⌘J, ⌘T, ⌘W, ⌘⇧] and ⌘⇧[, ⌘⇧↩, ⌘F, ⌘C, ⌘V. Does each do what the table says, and does ⌘C in vim still copy rather than reach the program?
    • Host tab: is it your own prompt, theme and aliases? Does the font match your Ghostty?
    • Container tab: does it land in bash, or fall back to sh on an image without bash?
    • Env tab: does printenv SOME_KEY show the overlay value, and is the badge right?
    • Paste a 2 MB file's contents into sleep 100: the window must stay responsive and show "input not sent".
    • Start sleep 300 & and note its pid, close the tab, then ps -p PID: it should be gone. Then try (set -m; nohup sleep 301 &): on macOS this one may survive, and the panel should say one process is still running.
    • Prompt symbols (run --screenshots too and compare with the PNGs in this PR): are the powerline separators, the branch symbol and the folder and language icons real glyphs now, with no boxed question marks? Do separators meet the coloured segments with no hairline gap? Are icons a sensible size next to the text?
    • Anything blurry, misaligned or laggy.

🤖 Generated with Claude Code

Add apps/desktop/term (hack-desktop-term): an app-owned PTY whose child
leads its own session (setsid + controlling tty) and whose Drop ends the
whole session (process group and job groups, SIGHUP then SIGKILL), a
read-only byte stream (file or process group), and, behind the `ghostty`
feature, libghostty-vt 1.0.1 emulation with a GPUI-drawn cell renderer,
scrollback cap, follow, search, selection, copy, paste and theme
following. Includes the term_spike example and DECISION.md (research,
comparison, build/CI plan, uses, security rules, licences).

The feature is off by default so desktop CI, which has no Zig 0.17,
stays green; DECISION.md lists the workflow change needed to turn it on.

Refs HACK-2252

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@vercel

vercel Bot commented Nov 15, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hack-docs Ready Ready Preview Nov 15, 2026 3:55pm UTC

Request Review

@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 15, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-15T15:58:54.086170Z e518f0b Manual request
🔒 Security Review ✅ Completed 2026-11-15T16:01:07.348493Z e518f0b Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09b514f9d7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/term/src/view.rs Resolved
Comment thread apps/desktop/term/src/stream.rs Resolved
Comment thread apps/desktop/term/src/view.rs Resolved
@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 09b514f9d7

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

roodboi and others added 5 commits November 15, 2026 04:33
Review fixes for the terminal surface crate (HACK-2252).

- Hold the session leader unreaped until the last signal is sent, so its
  pid, group id and session id cannot be reused. Observe exit with
  waitid(WNOWAIT). Signal members outside the leader's group through a
  pidfd after re-verifying their session (Linux), or after an immediate
  re-check (macOS, residual race documented).
- Move PTY writes to a bounded, cancellable writer thread with
  non-blocking writes; input that does not fit is refused whole and shown
  as a notice. Run quit teardown on the background executor.
- Replace unbounded channels with queues bounded in pending bytes, with
  backpressure and cancellation.
- Decide the end of a PTY by end of output, not child exit, with a
  bounded wait when a descendant holds the terminal.
- Keep termination and reaping when a worker or reaper thread cannot be
  started; thread spawning is injectable for tests.
- Run stream processes in their own session with the same teardown.
- Make the process tests hermetic (emptied environment, fixed PATH,
  temporary HOME) and signal only recorded pids.
- Add term_bench, a rough throughput check.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…the build

- Add term/scripts/provision-ghostty.sh: fetch the pinned Zig 0.17.1 and
  libghostty-vt 1.0.1 archives into a cache directory, verify each
  against a committed sha256, and print the environment to export.
- Point the dependency's download base at an unfetchable URL in the
  desktop workspace's cargo config, and add a build guard to the crate,
  so the ghostty feature fails instead of downloading when the inputs
  are not provisioned.
- Add a Linux CI job that provisions, then builds, lints and tests the
  ghostty feature inside a network namespace with no usable interface.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
libghostty applies its byte limit and line limit together and its 10 MB
default byte limit kept only about 5,500 lines on a 200-column grid. Set
an explicit 256 MiB byte ceiling so the line bound is the one that
applies, and cover it with a test.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…s proven

Update DECISION.md for the review fixes: the offline provisioning flow and
CI job, session ownership and bounded I/O, the hermetic tests, first
throughput numbers, a section on why hosting Ghostty's renderer was not
chosen, what the chosen option still has to prove, and an optional spike
that would test the alternative.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82578360a0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/term/src/view.rs Resolved
@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 82578360a0

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

roodboi and others added 2 commits November 15, 2026 05:41
…sed on signals

Review round on the terminal surface (HACK-2252), and the owner's panel
requirements.

Review fixes:

- Signals fail closed. A signal goes only through a handle whose identity
  cannot be recycled: killpg on the anchored leader group everywhere, a
  pidfd for other session members on Linux. Where there is no such
  handle (macOS, Linux before 5.3) members outside the leader group are
  never signalled by number; they are found by a session scan and
  returned in Teardown::survivors. No kill(pid) after a check remains,
  test cleanup included. The process table is injectable so tests can
  replace a pid's identity between verification and signalling.
- Any input or notice change notifies the view, so a rejected paste to a
  quiet child draws its notice in the next frame.
- Quit teardown is registered and awaited for process-backed streams as
  well as PTY sessions.

Panel requirements:

- launch: session kinds (logs, container shell, host shell, host shell
  with env) with labels and badges, and the command each runs. The app
  sets only TERM, COLORTERM and TERM_PROGRAM and passes validated names;
  the CLI injects env values inside the PTY. A container shell tries
  bash, then sh when bash cannot start.
- ghostty_config: read font, colours, palette, cursor style and padding
  from the user's Ghostty config (includes and themes followed), falling
  back to app tokens. Only those keys are parsed and nothing is logged.
- panel and actions: a tabbed panel with toggle, new tab, close tab,
  next and previous tab, maximize and find. No keys are bound here.
- term_spike shows all four tab kinds in one panel and renders each.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…ed teardown

Record how each tab kind is started and why the CLI stays the only thing
that touches env values, why this is preferred to attaching to tmux, three
numbered CLI requests, tab identity and badges, the Ghostty config rules,
the keyboard actions, what the new renders and tests show, and the macOS
limit that fail-closed signalling leaves (reported as survivors). Replace
the two spike renders with one per tab kind.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: f6e8aced6f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: f6e8aced6f

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

roodboi and others added 2 commits November 15, 2026 06:03
…ctions

Review round on the terminal surface (HACK-2252).

- Test helpers never signal a pid by number. The "no trace" assertion
  only reports; a test that may leave a process running holds a pidfd
  handle for that session.
- The Ghostty config reader is bounded end to end. A file is identified
  by its canonical path before it is opened and is never read twice. It
  is opened once, without blocking and only if the handle is a regular
  file, and read through a 1 MiB limit on that handle. One load reads at
  most 4 MiB and 64 files across config files, includes and the theme.
- An empty `palette =` resets the palette, the theme's entries included.
- The view binds no keys. Copy, paste, find, find navigation, scrolling
  and follow are actions with public methods; the view keeps only key
  forwarding to the PTY and text entry for its find field. The example
  binds the keys from the app's design, per platform.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…onfig reads

Document every action with the bindings from the app's design, the key
contexts for binding predicates, the reader's limits and the palette
reset, and the tests added for them. Re-render the panel screenshots with
the example's new key hints.

Refs HACK-2252

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 410b059063

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@roodboi
roodboi marked this pull request as ready for review November 15, 2026 11:09
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

Ready for lead: 410b059

What it is: a new apps/desktop/term crate. It uses libghostty-vt 1.0.1 for emulation and draws the cells with GPUI. DECISION.md covers:

  • the research and comparison, and why not host Ghostty's own renderer;
  • what (b) still has to prove, and an optional (a) spike;
  • why not tmux;
  • a licence table.

What the crate does:

  • Sessions the app owns: the logs stream, a container shell via hack exec <service> -- bash (falls back to sh), a host login shell, and a host shell with env via hack host shell --env <overlay> --scope <scope>. The app passes names only and sets only TERM / COLORTERM / TERM_PROGRAM.
  • Tabs: tab kinds and the env badge.
  • Look: a bounded Ghostty config reader.
  • Actions: panel, copy, paste and find are actions; the crate binds no keys.
  • Spike example: the panel with four tabs.

The ghostty feature is off by default. It builds offline from checksummed inputs, and a new CI job builds and tests it with no network.

Cleanup when a tab or the app closes: fail-closed.

  • Signals go out only through killpg on the session leader's process group, which can't be reused because the leader is kept unreaped, or through pidfds on Linux.
  • Processes that can only be reached by raw pid are never signalled. On macOS, processes in other groups that ignore the hangup are reported as survivors.
  • Quitting the app awaits teardown for both PTYs and streams.

Ran (Linux, worker and reviewer):

  • fmt, and clippy for all four feature sets plus a type check for the macOS target.
  • scripts/test.sh: 139/139 by default, 167/167 with ghostty.
  • The offline check: 79/79 with no network and no display.
  • bun test tests/machine-output-contract.test.ts tests/sync-spawn-bound.test.ts: 89 pass.
  • The tests for the review fixes were mutation-checked: each fails when its fix is removed.
  • The next merge (34b5053) had no conflicts.
  • Speed on one Linux desktop, CPU only: about 220 MiB/s of log flood, and about 4 ms per full-screen change.

Hosted CI:

  • At f6e8aced: all three desktop jobs passed. The macOS job ran 136/136, including the survivor report, a real zsh login shell and the /bin/sh login shell.
  • At this head: Linux and the offline terminal job pass; macOS was still queued, and I did not wait for it.
  • The failing test and docker-e2e entries come from the draft gate run. I marked the PR ready now.

Review: one persistent Codex reviewer over 4 rounds, the same reviewer as #861.

  • Round 1, at 09b514f9: 6 findings, including the pid-reuse blocker.
  • Round 2, at 82578360: 3 findings.
  • Round 3, at f6e8aced: 4 findings.
  • All fixed. Round 4 is CLEAN at this head.
  • @codex review: no major issues at this head. Its 4 older threads are answered with the fix commits and resolved. computer pr review-status: CLEAR at 410b059.

CLI requests: numbered in the PR body.

  1. hack env list --json should list overlay names.
  2. hack host shell should use the passwd shell when SHELL is unset.
  3. hack exec should document and test that a command that can't start exits 126/127.

Not verified:

  • The ghostty feature on macOS: the view, panel, ⌘ bindings and Metal. The Mac commands and what to capture are in the PR body.
  • A real hack exec into a container (no Docker was used).
  • A real user Ghostty config (only fixtures were read).
  • The live window driven by hand, and X11.
  • GPU time and key-to-screen latency.

Defaulted decisions:

  • ghostty is off by default; its inputs are provisioned, not vendored (about 67 MB).
  • apps/desktop/.cargo/config.toml blocks downloads at build time.
  • TERM is xterm-256color; xterm-ghostty is used only when its terminfo is installed.
  • There is no macOS CI job for ghostty, because it would double macOS runner time.
  • There are extra find and scroll actions that the DESIGN-V1 key table does not list yet.
  • Wiring the panel into the real window is PR 3, after feat(desktop): add the Logs tab (HACK-2214) #808 and feat(desktop): add the Doctor view (HACK-2214) #806.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 410b059063

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex couldn't complete this request. Try again later.

roodboi and others added 2 commits November 15, 2026 09:09
…rminal itself

On macOS the prompt's powerline separators and Nerd Font symbols drew as
missing-glyph boxes when the terminal font had no glyph for them.

Box drawing, block elements and the powerline separators are now drawn
as cell-exact geometry on whole device pixels, so neighbouring cells
join without seams. Other prompt symbols come from Symbols Nerd Font
Mono 3.5.1: the provisioning script fetches and checksums it, the build
embeds it, and the view fills its outlines as vector paths. Outlines
are read with ttf-parser because GPUI's Linux text system refuses a
font with no glyph for "m", and a system font may define the same
private-use code points as something else.

A pixel test renders a row of these code points offscreen and fails on a
blank or missing-glyph cell, on an icon that did not come from the
bundled font, and on geometry that does not fit its cell. The term CI
job installs a software Vulkan driver and requires the renderer.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…new renders

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 610cb2167d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 610cb2167d

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

roodboi and others added 2 commits November 15, 2026 10:15
Symbols from the bundled font were sized by the em and the ascender, but
some glyphs have ink outside those metrics (U+F0DC and U+F0DE rise above
the ascender), so they could paint into the row above. Diagonal box
drawing strokes ran past the cell's corners.

A symbol is now placed by its ink box: the scale is reduced until the
ink fits its room and the glyph is moved by as little as brings it
inside. Symbols and the geometry glyphs are also clipped to their room.

A double-width symbol such as U+26A1 now draws in the two cells the
emulator gave it; it was fitted to one because its spacer cell did not
count as blank.

TermFont gains `symbols` (BundledSymbols::WhenMissing by default,
Always to draw every bundled symbol from the bundled font), which the
double-width render test needs on machines whose fonts have U+26A1.

The render tests now check every pixel around each glyph's room and the
placement of each symbol's ink, and a unit test places every glyph of
the font in eight room sizes.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…er the Unlicense

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5d5868aa5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/desktop/term/src/view.rs Resolved
@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: a5d5868aa5

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

With find open, new output that added matches updated the highlights
but not the count beside the find field: the view copied the search's
state before the search had run on the new output. If the source then
went quiet, the stale count ("no matches" next to highlighted matches,
for one) stayed on screen.

The view now runs the search in render, before reading the state the
overlay shows, so the count and the frame's highlights come from the
same pass. If a resize during layout changes the count, another frame
is requested.

Rendered reports the find state and the number of highlights a frame
drew, for the regression test: find open, output adds matches, the
source goes quiet, and the count shown equals the highlights with no
further frame.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 330f84a841

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: 330f84a841

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

…er frame

With find open every render ran the libghostty search twice, once for
the count and once for the frame's highlights, and each run checks
every match it holds. With 390,000 matches in a 10,000-line scrollback
an unchanged redraw took about 20 ms in a dev build.

The emulator now counts the changes a search depends on (output, a
resize, a viewport move, the needle, the selected match) and runs the
search at most once per change. The count and the frame's highlights
come from that one pass, the frame carries the state it was built with,
and redrawing an unchanged terminal runs nothing: 1.8 ms in the same
case.

Highlights for rows below the viewport, which libghostty lists when
they share a page with it, are no longer put in the frame.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@roodboi

roodboi commented Nov 15, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e518f0bf4c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +267 to +269
if let Some(shutdown) = view.update(cx, |v, _| v.take_shutdown()) {
self.closing.push(shutdown.terminate_in_background());
cx.spawn(async move |this, cx| {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include pending tab teardowns in application shutdown

If a tab containing a process that ignores SIGHUP/SIGTERM is closed and the application quits immediately afterward, that process can survive the application. terminate_in_background waits the 300 ms TERMINATE_GRACE before escalating, but the detached polling task here is not awaited during quit. The view's on_app_quit handler cannot help because take_shutdown has already removed its backend, and application exit can terminate the reaper before SIGKILL is sent. Retain shutdown ownership for closing tabs and include their pending teardowns in the awaited quit cleanup, with escalation that fits the quit deadline.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review · Automatically triggered

Security review completed. No security issues were found in this pull request.

Reviewed commit: e518f0bf4c

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant