fix(tty): wait for the app to finish drawing before a shot

.changeset/tty-shot-settle.md

@@ -0,0 +1,5 @@
+---
+"@noctcore/showcase-kit": patch
+---
+
+Terminal apps: a shot no longer catches a frame the app is still drawing. A terminal can hand one write over in pieces (a macOS pty passes 1024 bytes at a time), so the `waitFor` text could be on screen before the rest of its frame, and the shot came out cut off, with different bytes from run to run. Before every shot the kit now waits until the screen has not changed for 100 ms, which adds about 100 ms to each shot. An app that redraws the same frame on a timer settles at once. An app whose screen never stops changing (a clock, a spinner) is shot after at most 1 s, or sooner when the shot's `timeouts.shotMs` runs out, with a warning; give it a frozen mode for captures, as the terminal determinism guide describes. Trailing separators in a CDP url, `outputs.portfolio` and shim arguments are now trimmed in linear time, with the same results as before.

.github/workflows/ci.yml

@@ -14,8 +14,9 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        # Linux runs the POSIX process group kill path, Windows the taskkill path.
-        os: [ubuntu-24.04, windows-latest]
+        # Linux runs the POSIX process group kill path, Windows the taskkill path. macOS ptys hand an app's output
+        # over in smaller reads than Linux, which is where a shot of a half drawn frame showed up (#3).
+        os: [ubuntu-24.04, windows-latest, macos-latest]
     runs-on: ${{ matrix.os }}
     steps:
       - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0

CONTRIBUTING.md

@@ -55,9 +55,9 @@ bun run typecheck   # tsc --noEmit over src/, test/ and the two config files
 bun run test        # bun run build, then vitest run: every test file, one at a time
 ```
 
-CI (`.github/workflows/ci.yml`, on `ubuntu-24.04` and `windows-latest`) runs the same steps:
-`bun install --frozen-lockfile`, `bunx playwright install --with-deps chromium`, `bun run typecheck`
-and `bun run test`, which does the build.
+CI (`.github/workflows/ci.yml`, on `ubuntu-24.04`, `windows-latest` and `macos-latest`) runs the
+same steps: `bun install --frozen-lockfile`, `bunx playwright install --with-deps chromium`,
+`bun run typecheck` and `bun run test`, which does the build.
 
 The docs site under `site/` is its own package with its own `bun.lock`, not a workspace (a root
 `workspaces` field would make changesets stop versioning the published package). It reads the built
@@ -178,9 +178,9 @@ hold themselves to the same rule, on Linux, Windows and macOS:
   directory from `tempDir()`, under the OS temp directory.
 - **Fixtures have no clock and no randomness.** The fixture app and TUI draw fixed content; the TUI
   changes behaviour only through environment variables (`TUI_GRANDCHILD`, `TUI_IGNORE_QUIT`,
-  `TUI_APP_CURSOR`). A new fixture follows the same rule. The kit itself gives terminal apps
-  `TZ=UTC`, a fixed `TERM`, `COLORTERM`, `FORCE_COLOR`, `LANG` and `LC_ALL`, and strips `CI`,
-  `NO_COLOR` and other terminal hints (`ttyEnv` in `src/tty/pty.ts`).
+  `TUI_APP_CURSOR`, `TUI_SPLIT_MS`, `TUI_REDRAW_MS`). A new fixture follows the same rule. The kit
+  itself gives terminal apps `TZ=UTC`, a fixed `TERM`, `COLORTERM`, `FORCE_COLOR`, `LANG` and
+  `LC_ALL`, and strips `CI`, `NO_COLOR` and other terminal hints (`ttyEnv` in `src/tty/pty.ts`).
 - **No golden images.** Fonts rasterize differently on macOS than on Windows and Linux, so tests
   check image sizes computed from the layout (the comment next to each expected size shows the
   arithmetic), the colour of chosen pixels with a tolerance, file lists and screen text. They never
@@ -201,8 +201,7 @@ hold themselves to the same rule, on Linux, Windows and macOS:
 
 ## Platform notes
 
-CI runs on Linux and Windows. macOS is not in CI, so if your change touches the PTY, fonts, paths or
-process handling and you have a Mac, run the suite there too.
+CI runs on Linux, Windows and macOS.
 
 - **Paths.** Build paths with `node:path` (`join`, `resolve`), never with `/` in a string, and turn
   a path into an import URL with `pathToFileURL`. On Windows `\tools` is absolute but relative to the
@@ -270,8 +269,8 @@ for dependencies and packaging. For example `fix(tty): pass shim arguments with
 like 8.3 short paths`. One logical change per commit.
 
 The pull request template asks for a changeset, the gates you actually ran, docs and JSDoc for a
-new or changed config key, and deterministic tests. CI runs `typecheck` and `test` on Linux and
-Windows on every pull request.
+new or changed config key, and deterministic tests. CI runs `typecheck` and `test` on Linux,
+Windows and macOS on every pull request.
 
 ## Reporting
 

site/src/content/docs/guides/terminal-apps.mdx

@@ -215,15 +215,21 @@ last one left off. For each language the kit:
 1. starts the app and waits for `ready` (without `ready`, for any text at all), up to
    `target.readyTimeoutMs`;
 2. waits `inputDelayMs`, then runs `setup` once, with `{ tty, lang, mode: 'tty', config }`;
-3. per shot: presses the `keys` (or runs `nav`), waits for `waitFor`, waits `delayMs`, and renders
-   the screen to the raw PNG;
+3. per shot: presses the `keys` (or runs `nav`), waits for `waitFor`, waits `delayMs`, waits for
+   the screen to stop changing, and renders the screen to the raw PNG;
 4. closes the app: sends `quitKey`, waits up to 1.5 s for it to quit, then kills its whole process
    tree by PID.
 
 Without `waitFor`, the kit gives the app up to one second to redraw after the keys (it moves on as
 soon as the screen changes); set `waitFor` for anything slower. `timeouts.shotMs` (default 15000)
 bounds each `waitFor`.
 
+Before it renders, the kit waits until the screen has not changed for 100 ms, so a shot never shows
+a frame the app is still drawing (a terminal can hand one write over in pieces). That adds about
+100 ms to a shot. For an app whose screen never stops changing, the wait gives up after 1 s (sooner
+if the shot's `timeouts.shotMs` runs out), takes the screen as it is and warns: freeze the app for
+captures, see [Terminal determinism](/showcase-kit/guides/terminal-determinism/).
+
 Without `ready`, the kit only waits for the app to draw anything, then the `inputDelayMs` grace,
 which may catch an app halfway through its first screen. Set `ready` to text the finished screen
 shows.

site/src/content/docs/guides/terminal-determinism.mdx

@@ -22,9 +22,11 @@ images must not churn.
    `FORCE_COLOR=3`), `TZ=UTC`, `LANG` and `LC_ALL` set to `en_US.UTF-8`, and no CI or terminal
    program hints (`CI`, `NO_COLOR`, `TERM_PROGRAM`, `WT_SESSION` and the like are removed). The full
    list is in [Terminal apps](/showcase-kit/guides/terminal-apps/).
-3. **It settles on screen content, not on output silence.** It waits for the `ready` and `waitFor`
-   text, then a fixed grace. Waiting for the app to go quiet would never end for an app with a clock
-   or a spinner.
+3. **It waits for a finished screen.** It waits for the `ready` and `waitFor` text, then, before
+   every shot, for the screen to stay unchanged for 100 ms, so a frame the terminal hands over in
+   pieces is never shot half drawn. It compares screens, not output, so an app that redraws the same
+   frame on a timer settles at once. A screen that never stops changing (a clock, a spinner) is shot
+   after at most 1 s, with a warning.
 4. **A grace period after `ready`** (`target.inputDelayMs`, default 300 ms) before the first key, so
    keys are not lost while the app is still switching its terminal to raw mode.
 5. **A fixed look.** A bundled font, a whole-pixel cell width, a fixed line height, a fixed theme and

src/capture.ts

@@ -9,6 +9,7 @@ import { ShowcaseError } from './errors.js';
 import { log } from './log.js';
 import { navUrl, outputPath, select } from './paths.js';
 import { answers, startCommand, waitForUrl, type StartedProcess } from './process.js';
+import { trimTrailing } from './text.js';
 
 export interface CaptureOptions {
   /** Shot ids to capture. Default: all. */
@@ -106,7 +107,7 @@ async function startTarget(config: ResolvedWebConfig): Promise<StartedProcess |
 /** The HTTP endpoint that tells whether a CDP server is up, or undefined for a bare ws:// URL. */
 function cdpVersionUrl(target: CdpTarget): string | undefined {
   const url = target.cdpUrl ?? '';
-  return /^https?:/.test(url) ? `${url.replace(/\/+$/, '')}/json/version` : undefined;
+  return /^https?:/.test(url) ? `${trimTrailing(url, '/')}/json/version` : undefined;
 }
 
 async function captureUrl(

src/config/resolve.ts

@@ -1,5 +1,6 @@
 import { isAbsolute, resolve } from 'node:path';
 import { ConfigError } from '../errors.js';
+import { trimTrailing } from '../text.js';
 import { resolveTerminalOptions } from '../tty/theme.js';
 import { DEFAULT_CLIPS, resolveClips, WEB_CLIPS_MESSAGE } from './clips.js';
 import {
@@ -235,7 +236,7 @@ function resolvePortfolio(
     required: [],
   });
 
-  const defaultGallery = `${dir.replace(/[\\/]+$/, '')}/${GALLERY_FILE}`;
+  const defaultGallery = `${trimTrailing(dir, '\\/')}/${GALLERY_FILE}`;
   const gallery =
     value.gallery === false
       ? false
@@ -252,7 +253,7 @@ function resolvePortfolio(
     quality: num(issues, `${path}.quality`, value.quality, 90, { min: 1, max: 100, integer: true }),
     thumbnail,
     lang,
-    publicPath: publicPath.replace(/\/+$/, ''),
+    publicPath: trimTrailing(publicPath, '/'),
     padding: num(issues, `${path}.padding`, value.padding, 96, { min: 0, integer: true }),
     gallery,
   };

src/record.ts

@@ -10,6 +10,7 @@ import { ShowcaseError } from './errors.js';
 import { composeInHole, renderFrameHole, type FrameHole } from './frame/render.js';
 import { log } from './log.js';
 import { fillTemplate } from './template.js';
+import { trimTrailing } from './text.js';
 import type { TtyEngine } from './tty/capture.js';
 import type { TtyScreen, TtySession } from './tty/types.js';
 
@@ -182,7 +183,7 @@ function matches(text: string, pattern: string | RegExp): boolean {
 }
 
 function screenNote(text: string): string {
-  const trimmed = text.replace(/\n+$/, '');
+  const trimmed = trimTrailing(text, '\n');
   return trimmed ? `Last screen:\n${trimmed.replace(/^/gm, '  | ')}` : 'The screen was empty.';
 }
 

src/text.ts

@@ -0,0 +1,16 @@
+/**
+ * Where the run of `chars` at the end of `text` starts, or `text.length` when it does not end in one of them.
+ *
+ * A loop from the end, not a regex like `/\/+$/`: a regex engine tries the run again from every position in it, so
+ * a long run followed by another character takes quadratic time.
+ */
+export function trailingRunStart(text: string, chars: string): number {
+  let start = text.length;
+  while (start > 0 && chars.includes(text.charAt(start - 1))) start--;
+  return start;
+}
+
+/** `text` without the run of `chars` at its end, like `text.replace(/[chars]+$/, '')` in linear time. */
+export function trimTrailing(text: string, chars: string): string {
+  return text.slice(0, trailingRunStart(text, chars));
+}

src/tty/capture.ts

@@ -9,8 +9,9 @@ import { ShowcaseError } from '../errors.js';
 import { log } from '../log.js';
 import { outputPath } from '../paths.js';
 import { killTreeSync } from '../process.js';
+import { trimTrailing } from '../text.js';
 import { assertNodeRuntime, openTtySession, renderTtyScreen } from './index.js';
-import type { OpenTtySession, RenderTtyScreen, TtySession } from './types.js';
+import type { OpenTtySession, RenderTtyScreen, TtyScreen, TtySession } from './types.js';
 
 /** The engine calls capture needs. A parameter so tests can drive the flow without a PTY. */
 export interface TtyEngine {
@@ -26,6 +27,21 @@ export interface TtyCaptureResult {
 
 /** Without `waitFor`, how long to wait for keys or `nav` to change the screen before taking it anyway. */
 const CHANGE_WAIT_MS = 1_000;
+/**
+ * How long the screen must stay unchanged before a shot is taken. One write of a frame can reach the kit in more
+ * than one read (a macOS pty hands it over 1024 bytes at a time), so the text a shot waits for can be on screen
+ * before the rest of its frame is. The reads of one write arrive well under a millisecond apart, so 100 ms bridges
+ * them even on a loaded machine, and it is about all a shot of a finished screen costs.
+ */
+const SETTLE_MS = 100;
+/** How often to look at the screen while it settles: a quarter of `SETTLE_MS`, so a change is seen promptly. */
+const SETTLE_POLL_MS = 25;
+/**
+ * The longest a shot waits for the screen to settle. A spinner or a clock never stops changing, and every shot of
+ * such an app costs this much, so it is short; a frozen mode in the app (see the terminal determinism guide) avoids it.
+ */
+const SETTLE_CAP_MS = 1_000;
+const DETERMINISM_GUIDE = 'https://noctcore.github.io/showcase-kit/guides/terminal-determinism/';
 /** How long a signal waits for open sessions to quit before exiting anyway. */
 const SIGNAL_CLOSE_MS = 5_000;
 
@@ -104,7 +120,7 @@ export function describePattern(pattern: string | RegExp): string {
 }
 
 function lastScreen(session: TtySession): string {
-  const text = session.screenText().replace(/\n+$/, '');
+  const text = trimTrailing(session.screenText(), '\n');
   return text ? `Last screen:\n${text.replace(/^/gm, '  | ')}` : 'The screen was empty.';
 }
 
@@ -151,6 +167,34 @@ async function waitForChange(session: TtySession, before: string, timeoutMs: num
   while (Date.now() < deadline && session.screen().key === before) await session.sleep(25);
 }
 
+const sleep = (ms: number): Promise<void> => new Promise(resolve => setTimeout(resolve, Math.max(0, ms)));
+
+/** The screen with everything the app printed so far parsed into it: a wait with no time left flushes, then checks. */
+async function flushedScreen(session: TtySession): Promise<TtyScreen> {
+  await session.waitForText('', { timeoutMs: 0 }).catch(() => {});
+  return session.screen();
+}
+
+/**
+ * Wait until the screen has not changed for `SETTLE_MS`, so a shot never shows a frame the app is still drawing.
+ * It compares screens, not output, so an app that redraws the same frame on a timer settles at once. Gives up at
+ * `timeoutMs` and returns the screen as it is then, with `settled: false`.
+ */
+async function waitForSettle(session: TtySession, timeoutMs: number): Promise<{ screen: TtyScreen; settled: boolean }> {
+  const deadline = Date.now() + timeoutMs;
+  let screen = await flushedScreen(session);
+  let since = Date.now();
+  for (;;) {
+    const now = Date.now();
+    if (now - since >= SETTLE_MS) return { screen, settled: true };
+    if (now >= deadline) return { screen, settled: false };
+    await sleep(Math.min(SETTLE_POLL_MS, deadline - now));
+    const next = await flushedScreen(session);
+    if (next.key !== screen.key) since = Date.now();
+    screen = next;
+  }
+}
+
 export async function startSession(
   config: ResolvedTtyConfig,
   lang: string,
@@ -201,6 +245,7 @@ async function shoot(
   lang: string,
   engine: TtyEngine,
 ): Promise<CapturedFile> {
+  const started = Date.now();
   const before = session.screen().key;
   if (shot.keys !== undefined) await session.press(shot.keys);
   else if (shot.nav) await shot.nav(session);
@@ -215,8 +260,18 @@ async function shoot(
     await waitForChange(session, before, CHANGE_WAIT_MS);
   }
   if (shot.delayMs > 0) await session.sleep(shot.delayMs);
+  // Every shot, including the first one after `ready` and one after a restart, waits for the app to finish drawing.
+  const budget = Math.min(SETTLE_CAP_MS, started + config.timeouts.shotMs - Date.now());
+  const { screen, settled } = await waitForSettle(session, budget);
+  if (!settled) {
+    log.warn(
+      `  ${lang}/${shot.id}: the screen did not stay still for ${String(SETTLE_MS)}ms within ` +
+        `${String(Math.max(0, budget))}ms, so the shot may show it mid-change. Freeze spinners and clocks in the app ` +
+        `for captures: ${DETERMINISM_GUIDE}`,
+    );
+  }
 
-  const png = await engine.renderTtyScreen(page, session.screen(), config.terminal, config.deviceScaleFactor);
+  const png = await engine.renderTtyScreen(page, screen, config.terminal, config.deviceScaleFactor);
   const path = outputPath(config, config.outputs.raw, lang, shot.id);
   await mkdir(dirname(path), { recursive: true });
   await writeFile(path, png);

src/tty/pty.ts

@@ -1,6 +1,7 @@
 import { existsSync, statSync } from 'node:fs';
 import { delimiter, extname, isAbsolute, join } from 'node:path';
 import { ShowcaseError } from '../errors.js';
+import { trailingRunStart } from '../text.js';
 
 /** The part of the node-pty API the engine uses. `@lydell/node-pty` and `node-pty` both provide it. */
 export interface PtyProcess {
@@ -149,7 +150,7 @@ function cmdQuote(arg: string): string {
         'nor a line break. Run the program the shim starts directly, or use a command string and quote it yourself.',
     );
   }
-  return /^[\w\-./\\:@+~]+$/.test(arg) ? arg : `"${arg.replace(/(\\+)$/, '$1$1')}"`;
+  return /^[\w\-./\\:@+~]+$/.test(arg) ? arg : `"${arg}${arg.slice(trailingRunStart(arg, '\\'))}"`;
 }
 
 /**

src/tty/render.ts

@@ -1,5 +1,6 @@
 import type { Page } from 'playwright';
 import { ShowcaseError } from '../errors.js';
+import { trimTrailing } from '../text.js';
 import { Attr, DEFAULT_COLOR, TRUECOLOR, type Grid, type GridCell, type GridColor } from './session.js';
 import { BUNDLED_ADVANCE, checkTheme, FALLBACK_FAMILY, FONT_FAMILY, fontFaceCss, fontFaces } from './theme.js';
 import type { RenderTtyScreen, ResolvedTerminalOptions, TerminalTheme, TtyScreen } from './types.js';
@@ -93,7 +94,7 @@ export function gridHtml(grid: Grid, look: ResolvedTerminalOptions, metrics: Cel
       if (!run) return;
       // Trailing blanks without a background or a line draw nothing: leave them out.
       if (run.text.length === run.width && !/background|text-decoration/.test(run.style)) {
-        const text = run.text.replace(/ +$/, '');
+        const text = trimTrailing(run.text, ' ');
         run.width -= run.text.length - text.length;
         run.text = text;
       }

src/tty/session.ts

@@ -3,6 +3,7 @@ import type { IBufferCell, IBufferLine, Terminal } from '@xterm/headless';
 import { ShowcaseError } from '../errors.js';
 import { log } from '../log.js';
 import { killTreeSync } from '../process.js';
+import { trimTrailing } from '../text.js';
 import { parseKeys } from './keys.js';
 import { loadPty, spawnPty, type PtyProcess } from './pty.js';
 import type { OpenTtySession, TtyScreen, TtySession } from './types.js';
@@ -63,7 +64,7 @@ function attrs(cell: IBufferCell): number {
 
 /** A row as plain text. xterm only trims cells never written to; spaces the app printed are trimmed too. */
 function rowText(line: IBufferLine | undefined): string {
-  return (line?.translateToString(true) ?? '').replace(/ +$/, '');
+  return trimTrailing(line?.translateToString(true) ?? '', ' ');
 }
 
 /** Copy the visible screen of `term` into an immutable `TtyScreen`. */