fix(core): stop ratatui cursor queries from racing the TUI event stream - #36318
Merged
FrozenPandaz merged 2 commits intoJul 17, 2026
Conversation
ratatui-core 0.1.2 added a cursor-position snapshot to Terminal::clear, which insert_before calls on every inline scrollback insert. The query writes ESC[6n and reads the reply from terminal input, racing the live crossterm EventStream and intermittently timing out, surfacing as 'insert_before failed' when running in inline mode. Wrap the crossterm backend so get_cursor_position answers from a cached position (last set through the backend) instead of touching the terminal. The TUI keeps the cursor hidden and positions it absolutely, and the inline viewport is full-height, so ratatui's viewport math is unaffected by the cached value.
The previous message asserted insert_before 'may not exist on this terminal type', which is not a real failure mode and discarded the underlying io::Error that would have pointed at the root cause.
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit f163af1
☁️ Nx Cloud last updated this comment at |
leosvelperez
approved these changes
Jul 17, 2026
FrozenPandaz
approved these changes
Jul 17, 2026
FrozenPandaz
deleted the
feature/nxc-4597-error-insert_before-failed-when-swapping-to-inline-mode
branch
July 17, 2026 22:13
FrozenPandaz
pushed a commit
that referenced
this pull request
Jul 20, 2026
…am (#36318) ## Current Behavior Since the ratatui 0.30 bump, running tasks in the inline TUI intermittently logs `ERROR insert_before failed - method may not exist on this terminal type`, typically noticed when swapping to inline mode. Root cause: ratatui-core **0.1.2** (a semver-compatible patch that arrived via a later `Cargo.lock` refresh, not the 0.30 bump commit itself) added a cursor-position snapshot to `Terminal::clear()`: ```rust pub fn clear(&mut self) -> Result<(), B::Error> { let original_cursor = self.backend.get_cursor_position()?; // new in 0.1.2 self.clear_viewport()?; self.backend.set_cursor_position(original_cursor)?; ... ``` `insert_before` (the inline scrollback path) calls `clear()` on every insert, so every scrollback flush now writes `ESC[6n` and reads the reply from terminal input — while crossterm's `EventStream` owns terminal input. When the query loses that race it times out (~2s) and `insert_before` returns `Err`. This is the same query/event-stream conflict the TUI already works around with `draw_without_autoresize` and by stopping the event stream around mode switches; ratatui-core 0.1.2 re-introduced it from inside the render path where we can't stop the stream. (Enabling ratatui's `scrolling-regions` feature was considered and rejected: with our full-height inline viewport it pushes lines to scrollback via `CSI S` in a 1-row DECSTBM region, which xterm.js/VSCode drops instead of saving to scrollback.) ## Expected Behavior No cursor-position query can ever run on the TUI render path. The crossterm backend is wrapped in `CursorCachingBackend`, whose `get_cursor_position` answers from the last position set through the backend instead of touching the terminal. This is sound because the TUI keeps the cursor hidden and positions it absolutely, and the inline viewport is full-height, so ratatui's inline viewport math yields the same result regardless of the reported position. This also structurally covers other ratatui internals that query the cursor (e.g. fullscreen `autoresize` → `resize` → `clear()` on terminal resize). The error log for a failed scrollback insert now includes the actual `io::Error` instead of the speculative "method may not exist on this terminal type" message. Validation: `cargo test -p nx --lib` passes (477 tests, includes a new unit test for the cached-cursor behavior); `cargo check`/`clippy` introduce no new warnings. ## Related Issue(s) Fixes NXC-4597 (cherry picked from commit af78b8d)
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
3 participants
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.
Current Behavior
Since the ratatui 0.30 bump, running tasks in the inline TUI intermittently logs
ERROR insert_before failed - method may not exist on this terminal type, typically noticed when swapping to inline mode.Root cause: ratatui-core 0.1.2 (a semver-compatible patch that arrived via a later
Cargo.lockrefresh, not the 0.30 bump commit itself) added a cursor-position snapshot toTerminal::clear():insert_before(the inline scrollback path) callsclear()on every insert, so every scrollback flush now writesESC[6nand reads the reply from terminal input — while crossterm'sEventStreamowns terminal input. When the query loses that race it times out (~2s) andinsert_beforereturnsErr. This is the same query/event-stream conflict the TUI already works around withdraw_without_autoresizeand by stopping the event stream around mode switches; ratatui-core 0.1.2 re-introduced it from inside the render path where we can't stop the stream.(Enabling ratatui's
scrolling-regionsfeature was considered and rejected: with our full-height inline viewport it pushes lines to scrollback viaCSI Sin a 1-row DECSTBM region, which xterm.js/VSCode drops instead of saving to scrollback.)Expected Behavior
No cursor-position query can ever run on the TUI render path. The crossterm backend is wrapped in
CursorCachingBackend, whoseget_cursor_positionanswers from the last position set through the backend instead of touching the terminal. This is sound because the TUI keeps the cursor hidden and positions it absolutely, and the inline viewport is full-height, so ratatui's inline viewport math yields the same result regardless of the reported position. This also structurally covers other ratatui internals that query the cursor (e.g. fullscreenautoresize→resize→clear()on terminal resize).The error log for a failed scrollback insert now includes the actual
io::Errorinstead of the speculative "method may not exist on this terminal type" message.Validation:
cargo test -p nx --libpasses (477 tests, includes a new unit test for the cached-cursor behavior);cargo check/clippyintroduce no new warnings.Related Issue(s)
Fixes NXC-4597