fix: prevent ghost cursor on DECSCUSR style change#19
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a rendering edge case in the web client where DECSCUSR cursor style/blink changes (e.g., vim normal/insert toggles without cursor movement) can leave the old cursor shape painted on the canvas (“ghost cursor”). The PR also documents the root cause and workaround via a new ADR.
Changes:
- Track whether DECSCUSR updates actually change
cursorStyle/cursorBlink, and force a full renderer repaint when they do. - Add unit tests covering the repaint trigger behavior.
- Add an ADR documenting the rendering gap and the chosen mitigation (and remove an unrelated prior ADR 015 file).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/client/cursor.ts | Forces a full repaint when DECSCUSR changes cursor style/blink to clear stale cursor pixels. |
| src/client/cursor.test.ts | Adds tests ensuring the repaint is called (or not) under the right conditions. |
| docs/adrs/015.ordering-system.pricing-storage.md | Removes an unrelated ADR 015 document. |
| docs/adrs/015.client.cursor-ghost-repaint.md | Adds ADR documenting the ghost-cursor root cause and fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ghostty-web's render loop only clears the cursor row when the cursor moves or cursorBlink=true. Switching vim normal→insert (steady block→bar) satisfies neither: cursor stays put, both modes non-blinking. The old block cursor shape persisted on canvas; the new bar was drawn on top, leaving the cursor visually unchanged. Force a full repaint (forceAll=true) in applyDecscusr when the net cursor style/blink differs from the initial values at function entry. Capturing initial vs final (rather than a mid-loop changed flag) avoids a spurious repaint when multiple sequences in one chunk cancel each other out. ADR 013 manual verification passed because vim's full-screen redraw on open makes all rows dirty, clearing the cursor correctly. The gap only shows on subsequent in-place mode switches. Adds ADR 015 documenting the ghost cursor root cause and the fix. Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/adrs/015.ordering-system.pricing-storage.md(unrelated leftover, deleted in prior commit3fce33c)Root cause
ADR 013 (
cursor.ts) correctly intercepts DECSCUSR sequences and updatesterm.options.cursorStylevia ghostty-web's options Proxy →renderer.setCursorStyle(). But ghostty-web's render loop only clears the cursor row (to erase the old shape) under two conditions:When vim switches normal → insert (
\x1b[2 qsteady block →\x1b[6 qsteady bar), neither fires — cursor stays put, both modes non-blinking. Old block cursor background persists on canvas; bar is drawn on top of it, leaving the cursor visually unchanged ("ghost cursor").Why ADR 013 manual verification passed: vim's full-screen redraw on open makes all rows dirty, so the first mode switch works correctly. The gap only appears on subsequent in-place mode switches (pressing
i/Escwithout moving the cursor first).Fix
Capture initial style/blink at function entry, process all sequences, then compare final vs initial:
Comparing net initial→final (rather than a mid-loop flag) avoids a spurious repaint when multiple sequences in one chunk cancel each other out. Fires only on vim mode switches — no per-message overhead.
Changes
src/client/cursor.ts— compare initial vs final style/blink, force repaint on net changesrc/client/cursor.test.ts— tests covering repaint trigger and the cancel-out no-op casedocs/adrs/015.client.cursor-ghost-repaint.md— ADR documenting root cause, options considered, and relationship to ADR 013