fix(pty): respond to DSR cursor position queries for inline TUI compatibility#9
fix(pty): respond to DSR cursor position queries for inline TUI compatibility#9cavanaug wants to merge 2 commits intomsmps:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds PTY-side support for answering Device Status Report cursor-position queries (ESC[6n) so inline/overlay TUIs can successfully query the cursor position when running under Pilotty.
Changes:
- Introduces a
CursorTracker(vt100-backed) to track cursor position and detectESC[6nacross chunk boundaries. - On detecting DSR queries in the PTY output stream, enqueues synthetic cursor-position replies (
ESC[{row};{col}R) back to child stdin via the existing write channel. - Adds unit tests for DSR detection and 1-indexed coordinates, plus an async end-to-end test.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
crates/pilotty-cli/src/daemon/pty.rs
Outdated
| fn process_output(&mut self, bytes: &[u8]) -> usize { | ||
| self.parser.process(bytes); | ||
| self.count_dsr_queries(bytes) | ||
| } |
There was a problem hiding this comment.
Updated CursorTracker::process_output to process output incrementally and capture (row, col) at each detected ESC[6n boundary. It now returns Vec<(u16, u16)>, so DSR replies use the cursor position at query time even when trailing bytes exist in the same chunk.
crates/pilotty-cli/src/daemon/pty.rs
Outdated
| let dsr_queries = match cursor_tracker.lock() { | ||
| Ok(mut tracker) => tracker.process_output(&buf[..n]), | ||
| Err(_) => { | ||
| warn!("Cursor tracker mutex poisoned; skipping DSR detection"); | ||
| 0 | ||
| } | ||
| }; | ||
|
|
||
| if dsr_queries > 0 { | ||
| let (row, col) = match cursor_tracker.lock() { | ||
| Ok(tracker) => tracker.cursor_position_one_indexed(), | ||
| Err(_) => { | ||
| warn!("Cursor tracker mutex poisoned; using fallback cursor response"); | ||
| (1, 1) | ||
| } | ||
| }; |
There was a problem hiding this comment.
Refactored to a single lock per read chunk. reader_loop now calls process_output once, receives all DSR boundary positions, and emits responses from that vector. This removes the previous double-lock and keeps parse/position capture consistent.
crates/pilotty-cli/src/daemon/pty.rs
Outdated
| for _ in 0..dsr_queries { | ||
| if write_tx.blocking_send(response.clone()).is_err() { | ||
| debug!("PTY write channel closed while sending DSR response"); | ||
| break; |
There was a problem hiding this comment.
Changed synthetic DSR enqueue from blocking_send to try_send. On full channel, we log and drop the synthetic response rather than blocking the reader thread; on closed channel, we stop sending. This keeps PTY output draining even under backpressure.
| let output = String::from_utf8_lossy(&bytes); | ||
| assert!( | ||
| output.contains("ok:"), | ||
| "Expected shell to receive DSR response, got: {:?}", | ||
| output | ||
| ); | ||
|
|
There was a problem hiding this comment.
Strengthened the async E2E test to validate the actual cursor report. The test now writes a known sequence (abc + ESC[6n + xyz), parses ok:\x1b[{row};{col}, and asserts (row, col) == (1, 4).
Summary
ESC[6n) so apps running in Pilotty can receive a valid cursor-position response (ESC[{row};{col}R).Problem
Apps using inline terminal viewports (e.g.
ratatuiViewport::Inline, similar to Atuin-style overlays) depend on cursor-position queries succeeding during startup/rendering.In Pilotty sessions, these apps could fail with errors equivalent to:
The cursor position could not be read within a normal durationRoot cause: PTY output was parsed for display/snapshot state but DSR cursor queries (
ESC[6n) were not answered, so querying apps timed out waiting for terminal response.Solution Approach
ESC[6nreliably, including split/chunked sequences.ESC[{row};{col}R(1-indexed terminal coordinates) back to child stdin via the existing write channel.Validation
cargo test -p pilotty-cli daemon::pty::testscargo test -p pilotty-cliBoth pass locally.