fix: prevent ghost cursor on DECSCUSR style change

docs/adrs/015.client.cursor-ghost-repaint.md

@@ -0,0 +1,116 @@
+# ADR 015: Client — Force repaint on DECSCUSR to prevent ghost cursor
+
+**SPEC:** [client](../specs/client.md)
+**Status:** Accepted
+**Date:** 2026-03-26
+**Supersedes:** Partially extends [ADR 013](013.client.cursor-style.md)
+
+---
+
+## Context
+
+ADR 013 introduced `cursor.ts` to intercept DECSCUSR sequences and update
+`term.options.cursorStyle` / `term.options.cursorBlink` via ghostty-web's
+options Proxy. The approach was sound, but it addressed only the "what to set"
+layer. A second rendering gap remained, causing cursor shape changes to be
+visually invisible in practice.
+
+### The ghost cursor problem
+
+ghostty-web's `CanvasRenderer.render()` only **clears the cursor row** (erases
+the old cursor shape from canvas) under two conditions:
+
+```js
+// ghostty-web/dist/ghostty-web.js — CanvasRenderer.render()
+const s = cursor.x !== lastPos.x || cursor.y !== lastPos.y;
+if (s || this.cursorBlink) {
+  renderLine(cursor_row);   // ← erases old cursor shape
+}
+// always runs after:
+renderCursor(cursor.x, cursor.y);  // ← draws new cursor shape
+```
+
+When vim switches normal → insert mode (steady block `\x1b[2 q` → steady bar
+`\x1b[6 q`), neither condition is true:
+
+- **Cursor did not move** — pressing `i` leaves the cursor in place (`s = false`)
+- **`cursorBlink = false`** — both normal and insert use steady cursors
+
+Result: the old block cursor background stays on canvas and the new bar cursor
+is drawn on top of it, leaving the cursor visually unchanged.
+
+### Why ADR 013 manual verification passed
+
+When vim first opens it performs a full-screen redraw, writing to every row
+including the cursor row. The dirty-row paint path also calls
+`renderLine(cursor_row)`, which clears and redraws the cursor correctly via
+`renderCursor()`. The first mode switch on vim open therefore works fine.
+
+The failure only surfaces on **subsequent in-place mode switches** (pressing `i`
+or `Esc` without moving the cursor), where only the status line is updated and
+the cursor row stays clean. This scenario was not covered by the original manual
+verification.
+
+---
+
+## Decision
+
+After detecting a cursor style or blink change, call
+`term.renderer.render(term.wasmTerm, true, term.viewportY)` synchronously.
+
+`forceAll = true` bypasses the dirty-state and blink checks, repainting every
+row. This clears the stale cursor shape from the cursor row before
+`renderCursor()` draws the new shape at the end of the same render pass.
+
+`term.renderer` and `term.wasmTerm` are public properties on `Terminal`
+(declared without `private` in ghostty-web's `.d.ts`). `GhosttyTerminal`
+satisfies both `IRenderable` and `IScrollbackProvider` structurally, so no
+casting is needed.
+
+The force repaint is guarded by a `changed` flag and only fires when style or
+blink actually differs from the current value — i.e., on vim mode switches,
+which are infrequent. There is no per-message overhead.
+
+---
+
+## Considered Options
+
+**Option A: `requestAnimationFrame` toggle on `cursorBlink`**
+
+Temporarily set `cursorBlink = true` for one RAF frame so the render loop's
+`if (cursorBlink)` branch clears the cursor row, then restore the correct value
+via RAF callback. Correct but adds async complexity and a one-frame blink pulse.
+
+**Option B: Ignore `cursorBlink` updates entirely**
+
+Only update `cursorStyle` from DECSCUSR; leave `cursorBlink` controlled by
+config. With the default `cursorStyleBlink: true`, blinking ensures the cursor
+row is always cleared. Fragile: silently breaks for users who disable blinking
+in config.
+
+**Option C: Force repaint via `term.renderer.render(forceAll=true)` (chosen)**
+
+Synchronous, no async, no side effects on blink state, works regardless of
+config. The extra full-canvas render fires only on mode switches and costs ~1 ms
+on modern hardware.
+
+---
+
+## Consequences
+
+- vim, neovim, fish — cursor shape changes are immediately visible on all mode
+  switches regardless of blink config or cursor position.
+- One additional full canvas render per DECSCUSR style change. No per-message
+  cost; normal typing is unaffected.
+- If ghostty-web adds a public `refresh()` or `invalidateCursorRow()` API, the
+  `forceAll` call can be replaced with the narrower method and this ADR updated.
+- When ghostty-web implements DECSCUSR natively (reading `cursor_visual_style`
+  from the WASM render state — see ADR 013's upstream fix checklist), the entire
+  `cursor.ts` module and this workaround are removed together.
+
+## Related Decisions
+
+- [ADR 013 — DECSCUSR cursor style via PTY intercept](013.client.cursor-style.md):
+  introduced `cursor.ts`; this ADR closes the rendering gap it left open.
+- [ADR 010 — Client UX polish](010.client.ux-polish.md): established the
+  WebSocket message handling that the intercept hooks into.

src/client/cursor.test.ts

@@ -1,10 +1,24 @@
-import { describe, expect, test } from 'bun:test';
+import { describe, expect, mock, test } from 'bun:test';
 import { applyDecscusr } from './cursor';
 
 function makeTerm(): { options: { cursorStyle: string; cursorBlink: boolean } } {
   return { options: { cursorStyle: 'block', cursorBlink: false } };
 }
 
+function makeTermWithRenderer(): {
+  options: { cursorStyle: string; cursorBlink: boolean };
+  renderer: { render: ReturnType<typeof mock> };
+  wasmTerm: object;
+  viewportY: number;
+} {
+  return {
+    options: { cursorStyle: 'block', cursorBlink: false },
+    renderer: { render: mock(() => {}) },
+    wasmTerm: {},
+    viewportY: 0,
+  };
+}
+
 describe('applyDecscusr', () => {
   test('Ps 0 — default reset — sets block blinking', () => {
     const term = makeTerm();
@@ -92,3 +106,45 @@ describe('applyDecscusr', () => {
     expect(term.options.cursorBlink).toBe(false);
   });
 });
+
+describe('applyDecscusr — force repaint', () => {
+  test('calls renderer.render with forceAll=true when style changes', () => {
+    const term = makeTermWithRenderer();
+    applyDecscusr(term as never, '\x1b[6 q');
+    expect(term.renderer.render).toHaveBeenCalledTimes(1);
+    expect(term.renderer.render).toHaveBeenCalledWith(term.wasmTerm, true, 0);
+  });
+
+  test('calls renderer.render when only blink changes', () => {
+    const term = makeTermWithRenderer();
+    term.options.cursorBlink = true;
+    applyDecscusr(term as never, '\x1b[2 q');
+    expect(term.renderer.render).toHaveBeenCalledTimes(1);
+  });
+
+  test('does not call renderer.render when style is unchanged', () => {
+    const term = makeTermWithRenderer();
+    term.options.cursorStyle = 'block';
+    term.options.cursorBlink = false;
+    applyDecscusr(term as never, '\x1b[2 q');
+    expect(term.renderer.render).not.toHaveBeenCalled();
+  });
+
+  test('does not call renderer.render when no DECSCUSR sequence', () => {
+    const term = makeTermWithRenderer();
+    applyDecscusr(term as never, 'hello world');
+    expect(term.renderer.render).not.toHaveBeenCalled();
+  });
+
+  test('calls renderer.render once even with multiple changing sequences', () => {
+    const term = makeTermWithRenderer();
+    applyDecscusr(term as never, '\x1b[2 q\x1b[6 q');
+    expect(term.renderer.render).toHaveBeenCalledTimes(1);
+  });
+
+  test('does not call renderer.render when sequences cancel out to original values', () => {
+    const term = makeTermWithRenderer();
+    applyDecscusr(term as never, '\x1b[6 q\x1b[2 q');
+    expect(term.renderer.render).not.toHaveBeenCalled();
+  });
+});

src/client/cursor.ts

@@ -21,6 +21,8 @@ const ESC = '\x1b';
 const DECSCUSR = new RegExp(`${ESC}\\[(\\d*) q`, 'g');
 
 export function applyDecscusr(term: Terminal, data: string): void {
+  const initialStyle = term.options.cursorStyle;
+  const initialBlink = term.options.cursorBlink;
   DECSCUSR.lastIndex = 0;
   let match = DECSCUSR.exec(data);
   while (match !== null) {
@@ -32,4 +34,19 @@ export function applyDecscusr(term: Terminal, data: string): void {
     }
     match = DECSCUSR.exec(data);
   }
+  // ghostty-web's render loop only clears the cursor row when the cursor moves or
+  // cursorBlink is true. When switching to a non-blinking style (e.g. block→bar in
+  // vim normal→insert), the old cursor shape stays painted on canvas and the new
+  // shape is drawn on top — leaving a ghost of the previous cursor.
+  //
+  // Force a full repaint so the cursor row is cleared before the new shape is drawn.
+  // term.renderer and term.wasmTerm are public on Terminal; forceAll=true repaints
+  // every row, which clears the stale cursor, and renderCursor() draws the new shape.
+  if (
+    (term.options.cursorStyle !== initialStyle || term.options.cursorBlink !== initialBlink) &&
+    term.renderer &&
+    term.wasmTerm
+  ) {
+    term.renderer.render(term.wasmTerm, true, term.viewportY);
+  }
 }