Skip to content

Fix kitty reply leak, inline-image cleanup, cursor handling and kitty image IDs - #15

Merged
planadecu merged 3 commits into
mainfrom
fix/inline-image-cleanup
Sep 29, 2026
Merged

planadecu merged 3 commits into
mainfrom
fix/inline-image-cleanup

Conversation

@planadecu

@planadecu planadecu commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Problems (found in glassnode-terminal)

  1. Kitty: stray \\ characters typed into inputs. Kitty graphics commands didn't set q=2, so kitty replied to every image transmit and delete with an OK message (ESC _G…;OK ESC \\) on the app's input. Ink parsed the reply as keypresses, and the trailing \\ landed in the host app's search box. If the host redraws the chart after input, this becomes an endless reply → redraw → reply loop.

  2. Inline images (iTerm2 protocol, used for VS Code): black box after switching views. Inline images live in the terminal's text cells. On unmount the image was cleared in a passive effect, which runs after the host's next Ink frame is written, and it painted the cells black. That covered the replacement view (for example a table), and with Ink's incremental rendering nothing ever repainted those lines.

  3. Inline images: leftovers after a size change. When a new, smaller image replaced a larger one, the parts of the old image outside the new one stayed on screen (for example a duplicated row of x-axis labels).

  4. Graphics writes moved Ink's cursor (found in a full-library review). Ink writes each frame relative to where it left the cursor, but every graphics write jumped to an absolute row that assumes the frame starts at terminal row 1, and the image then moved the cursor further. In a frame that doesn't start at row 1, the image lands in the wrong place and Ink's next frame is written from the wrong line. This may explain a missing header row seen in VS Code.

  5. Kitty image IDs were shared. Every chart used IDs 1 and 2, so two charts overwrote each other and unmounting one deleted the other's image. Deletes used d=i, which removes the image from the screen but keeps its data in terminal memory.

Fix

  • format.ts: q=2 added to kittyDelete and injected by kittyTagImage, so every kitty command is quiet.
  • InkUPlot.tsx: inline images are cleared in a layout-effect cleanup, which runs during React's commit and before Ink writes the host's next frame, so that frame lands on top. The cleanup uses default attributes (no black) and also runs when the chart changes size, not only on unmount. The kitty unmount delete also moved to a layout effect. I corrected the comment claiming VS Code keeps inline images through later text; it doesn't.
  • Cursor: every out-of-band write (draw, redraw, blank, kitty delete) is wrapped in save/restore cursor (ESC 7 … ESC 8) and positioned relative to Ink's cursor: on the frame's last line when the frame fills the terminal, otherwise on the line after it. The terminal is also asked not to move the cursor (iTerm2 doNotMoveCursor=1, which xterm.js safely ignores; kitty C=1).
  • Kitty IDs: each chart gets its own pair of IDs from a counter offset by process ID, and deletes use d=I, which also frees the image data.

Dependencies and security (added before release so this ships as one version):

  • Bumped every dependency to the newest version that's at least 7 days old:
    • runtime: path2d 0.3.4
    • development: ink 7.1.1, react 19.3.0, @types/node 26.6.2, @types/react 19.3.0, tsx 4.23.15, vitest 5.0.1 (major version) and vite 8.3.0
  • vite is now listed directly. vitest 5 takes it as a peer dependency, and the lockfile had kept a vulnerable 7.3.1.
  • An override forces tsup's esbuild to 0.28.2. tsup pins ^0.27, and no 0.27 release is patched.
  • pnpm audit: 15 findings → 0. All 15 were development-only (vitest, vite, postcss, esbuild, picomatch); none of the runtime dependencies were affected.
  • Added .github/dependabot.yml: weekly grouped minor and patch updates with a 7-day cooldown, and monthly grouped GitHub Actions updates. Supersedes chore(deps-dev): bump vitest from 4.1.10 to 4.1.11 #14.

Verification

  • New test/inline-cleanup.test.tsx: on unmount the old cells are blanked with default attributes before the replacement frame; on a size change the whole old area (8 rows) is blanked before the new image is drawn. Both tests fail on the previous code. The kitty tests in format.test.ts now require q=2.
  • pnpm vitest run: 81 tests pass. tsup build and tsc --noEmit pass.
  • New test/cursor-and-ids.test.tsx, for both iTerm2 and kitty: every write is wrapped in cursor save/restore and uses no absolute positioning, and a chart below a header line is placed 4 lines up at column 1. A second test runs two kitty charts: they get different IDs, and unmounting one doesn't delete the other's image. These tests fail on the previous commit. 85 tests pass.
  • In glassnode-terminal, loading this build in a pseudo-terminal:
    • VS Code mode: after switching to table, the blanking is written before the table text, and nothing is painted black.
    • Kitty mode: all 10 kitty image commands carry q=2. Every transmit has C=1, the chart uses its own pair of IDs, deletes use d=I, and every graphics write is wrapped in ESC 7 … ESC 8.
    • Placement, with each image replaced by a visible marker in the emulator: the image lands at row 7, column 82 (inside the data pane) both fullscreen and when started below a shell prompt, and the header row stays visible.
  • After the dependency bump: pnpm audit reports no known vulnerabilities, the build (now on esbuild 0.28) and tsc --noEmit pass, and 85 tests pass. The new build in glassnode-terminal passes its 32 tests and still places the chart correctly in the emulator.

planadecu and others added 2 commits September 29, 2026 16:26
…xt frame

Kitty: every graphics command now carries q=2. Without it kitty answers each
transmit/delete with an OK reply on stdin, which Ink parses as keypresses
(stray `\` characters in the host's inputs; with a host that redraws on input,
an endless reply -> redraw -> reply loop).

Inline images (iterm2/sixels, e.g. VS Code): they live in the text cells, so
the old image must be blanked when the chart unmounts or changes size. That
ran as a passive effect, i.e. after the host's next Ink frame had been written,
and painted black: it covered the replacement view (e.g. a table) and, with
incremental rendering, nothing repainted it. It now runs as a layout-effect
cleanup (during commit, before Ink writes the frame), uses default attributes,
and also fires on size changes (stale parts of a larger old image stayed).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PzJHgXssByMwBXH1PGbXCL
…e ids

Cursor: Ink writes each frame relative to where it left the cursor, but every
graphics write moved it (absolute CUP to the image, then the image advanced it)
and positioned at an absolute row that assumes the frame starts at terminal
row 1. Now every out-of-band write (stamp, re-stamp, blank, kitty delete) is
wrapped in DECSC/DECRC, positions relative to Ink's cursor (fullscreen: on the
frame's last line; otherwise the line after), and asks the terminal not to move
the cursor (iTerm2 doNotMoveCursor=1, kitty C=1).

Kitty ids: every chart used ids 1/2, so two charts overwrote each other and
unmounting one deleted the other's image. Each chart now gets its own pair from
a pid-offset counter, and deletes use d=I so the image data is freed, not just
its placement.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PzJHgXssByMwBXH1PGbXCL
@planadecu planadecu changed the title Fix stray input from kitty replies; clear inline images before the next frame Fix kitty reply leak, inline-image cleanup, cursor handling and kitty image IDs Sep 29, 2026
- Bump to the newest versions >= 7 days old: path2d 0.3.4; dev: ink 7.1.1,
  react 19.3.0, @types/node 26.6.2, @types/react 19.3.0, tsx 4.23.15,
  vitest 5.0.1 (major), vite 8.3.0 (now explicit; vitest 5 takes it as a peer,
  the lockfile kept a vulnerable 7.3.1).
- Override tsup's esbuild to ^0.28.2 (tsup pins ^0.27; no patched 0.27).
- pnpm audit: 15 -> 0 (all were dev-only: vitest/vite/postcss/esbuild/picomatch).
- Dependabot: weekly grouped minor/patch npm updates with a 7-day cooldown,
  monthly grouped GitHub Actions updates. Supersedes #14.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PzJHgXssByMwBXH1PGbXCL
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