diff --git a/Cargo.lock b/Cargo.lock index f31c68a5e..f93270b3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -126,9 +126,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.101" +version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f0e0fee31ef5ed1ba1316088939cea399010ed7731dba877ed44aeb407a75ea" +checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" [[package]] name = "arrayvec" @@ -682,9 +682,9 @@ dependencies = [ [[package]] name = "crossbeam-epoch" -version = "0.9.18" +version = "0.9.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" +checksum = "2d6914041f254d6e9176c01941b21115dcfb7089e55135a35411081bd106ef3f" dependencies = [ "crossbeam-utils", ] @@ -1983,9 +1983,9 @@ checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" [[package]] name = "memmap2" -version = "0.9.9" +version = "0.9.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "744133e4a0e0a658e1374cf3bf8e415c4052a15a111acd372764c55b4177d490" +checksum = "d1219ed1b7f229ee7104d281dd01d6802fe28bb6e95d292942c4daacdeb798c0" dependencies = [ "libc", ] @@ -2863,6 +2863,7 @@ dependencies = [ "pty_terminal", "pty_terminal_test_client", "subprocess_test", + "vte", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 0b9187367..c40f4aa7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ future_not_send = "allow" [workspace.dependencies] anstream = "1.0.0" -anyhow = "1.0.98" +anyhow = "1.0.103" assert2 = "0.4.0" assertables = "10.0.0" async-trait = "0.1.89" @@ -84,7 +84,7 @@ globset = "0.4.18" jsonc-parser = { version = "0.32.0", features = ["serde"] } libc = "0.2.185" libtest-mimic = "0.8.2" -memmap2 = "0.9.7" +memmap2 = "0.9.11" monostate = "1.0.2" napi = "3" napi-build = "2" @@ -161,6 +161,7 @@ vite_task_plan = { path = "crates/vite_task_plan" } vite_task_server = { path = "crates/vite_task_server" } vite_workspace = { path = "crates/vite_workspace" } vt100 = "0.16.2" +vte = "0.15.0" wax = "0.7.0" which = "8.0.0" widestring = "1.2.0" diff --git a/crates/pty_terminal_test/Cargo.toml b/crates/pty_terminal_test/Cargo.toml index e47ca1ffa..0f624bef0 100644 --- a/crates/pty_terminal_test/Cargo.toml +++ b/crates/pty_terminal_test/Cargo.toml @@ -12,6 +12,7 @@ anyhow = { workspace = true } portable-pty = { workspace = true } pty_terminal = { workspace = true } pty_terminal_test_client = { workspace = true } +vte = { workspace = true } [dev-dependencies] crossterm = { workspace = true } @@ -24,7 +25,6 @@ subprocess_test = { workspace = true, features = ["portable-pty"] } workspace = true [lib] -test = false doctest = false [package.metadata.cargo-shear] diff --git a/crates/pty_terminal_test/README.md b/crates/pty_terminal_test/README.md index ab97238cd..9e2b4e776 100644 --- a/crates/pty_terminal_test/README.md +++ b/crates/pty_terminal_test/README.md @@ -53,10 +53,10 @@ Milestones are encoded as an OSC 8 hyperlink: `Reader::expect_milestone` works like this: -1. Drain parsed unhandled OSC sequences from `PtyReader`. -2. Decode OSC 8 URI payload back into milestone name. -3. If no match yet, continue reading from PTY and repeat. -4. On match, return current `screen_contents()`. +1. Decode OSC 8 URI payloads from the PTY stream back into milestone names. +2. Pair each marker with its following zero-width rendered anchor. +3. Wait until both the requested marker and its anchor have arrived. +4. Return the current `screen_contents()`. The helper strips the protocol's zero-width space from returned screen text. @@ -65,6 +65,8 @@ The helper strips the protocol's zero-width space from returned screen text. The OSC 8 + zero-width anchor approach is used because it works across Unix and Windows ConPTY in this project. In particular, zero-length hyperlink opens can be lost on some Windows output paths, so the zero-width anchor is intentional. +Waiting for the anchor also prevents ConPTY's control-sequence path from +delivering a milestone before earlier rendered text. ## Typical test pattern diff --git a/crates/pty_terminal_test/src/lib.rs b/crates/pty_terminal_test/src/lib.rs index af724599e..2cf5d0479 100644 --- a/crates/pty_terminal_test/src/lib.rs +++ b/crates/pty_terminal_test/src/lib.rs @@ -1,4 +1,4 @@ -use std::io::{BufReader, Read}; +use std::{collections::VecDeque, io::Read}; pub use portable_pty::CommandBuilder; use pty_terminal::terminal::{PtyReader, Terminal}; @@ -10,6 +10,58 @@ pub use pty_terminal::{ const MILESTONE_HYPERTEXT: char = '\u{200b}'; +/// Tracks the two independently delivered parts of each milestone. +/// +/// A milestone starts with an OSC 8 hyperlink carrying its name and contains a +/// zero-width printable character. On `ConPTY`, the OSC control sequence can be +/// forwarded before earlier screen updates, while the printable character +/// follows those updates through the asynchronous rendering path. A milestone +/// is therefore complete only after both parts have arrived. +/// +/// Several OSC markers can overtake their anchors. The two queues preserve the +/// protocol order so an earlier marker's delayed anchor cannot complete a later +/// marker by mistake. +#[derive(Default)] +struct MilestoneTracker { + /// Marker names whose rendered zero-width anchors have not arrived yet. + awaiting_fence: VecDeque, + /// Marker names whose matching rendered anchors have arrived. + completed: VecDeque, +} + +impl MilestoneTracker { + fn take_completed(&mut self, name: &str) -> bool { + // Keep unrelated completed milestones available for later calls. A PTY + // read can contain more than the milestone currently being requested. + self.completed + .iter() + .position(|completed| completed == name) + .and_then(|index| self.completed.remove(index)) + .is_some() + } +} + +impl vte::Perform for MilestoneTracker { + fn print(&mut self, character: char) { + // `print` is called only for rendered characters, not for bytes inside + // OSC metadata. ConPTY preserves the order of these rendered anchors, + // so each anchor completes the oldest marker still awaiting one. + if character == MILESTONE_HYPERTEXT + && let Some(name) = self.awaiting_fence.pop_front() + { + self.completed.push_back(name); + } + } + + fn osc_dispatch(&mut self, params: &[&[u8]], _bell_terminated: bool) { + // The decoder accepts only milestone hyperlink opens. Ordinary OSC + // sequences and the empty OSC 8 close sequence are ignored. + if let Some(name) = pty_terminal_test_client::decode_milestone_from_osc8_params(params) { + self.awaiting_fence.push_back(name); + } + } +} + /// A test-oriented terminal that provides milestone-based synchronization. /// /// Wraps a PTY terminal, splitting it into a [`PtyWriter`] for sending input @@ -23,7 +75,17 @@ pub struct TestTerminal { /// The read half of a test terminal, wrapping [`PtyReader`] with milestone support. pub struct Reader { - pty: BufReader, + /// Reads bytes and updates the terminal's primary `vt100` screen parser. + /// + /// This is deliberately not wrapped in `BufReader`: its read-ahead would + /// let the primary parser consume bytes the milestone parser has not seen. + pty: PtyReader, + /// Observes the same byte stream to distinguish OSC markers from printable + /// anchors. `vt100::Callbacks` exposes unhandled OSC sequences but has no + /// callback for ordinary rendered characters, hence this small second parser. + milestone_parser: vte::Parser, + /// Persists protocol state across reads and `expect_milestone` calls. + milestone_tracker: MilestoneTracker, child_handle: ChildHandle, } @@ -37,17 +99,38 @@ impl TestTerminal { let Terminal { pty_reader, pty_writer, child_handle, .. } = Terminal::spawn(size, cmd)?; Ok(Self { writer: pty_writer, - reader: Reader { pty: BufReader::new(pty_reader), child_handle: child_handle.clone() }, + reader: Reader { + pty: pty_reader, + milestone_parser: vte::Parser::new(), + milestone_tracker: MilestoneTracker::default(), + child_handle: child_handle.clone(), + }, child_handle, }) } } impl Reader { + /// Reads once while keeping the screen parser and milestone parser in lockstep. + /// + /// All PTY draining, including shutdown, must go through this method. Reading + /// directly from `pty` would update the screen while silently skipping those + /// bytes in the milestone protocol state. + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + let n = self.pty.read(buf)?; + self.milestone_parser.advance(&mut self.milestone_tracker, &buf[..n]); + + // `PtyReader`'s primary parser also records the OSC sequences. The + // dedicated tracker above owns milestone handling, so discard this + // duplicate copy rather than letting it grow for the lifetime of a test. + drop(self.pty.take_unhandled_osc_sequences()); + Ok(n) + } + /// Returns terminal screen contents with milestone hyperlink text removed. #[must_use] pub fn screen_contents(&self) -> String { - let mut contents = self.pty.get_ref().screen_contents(); + let mut contents = self.pty.screen_contents(); contents.retain(|ch| ch != MILESTONE_HYPERTEXT); contents } @@ -56,7 +139,7 @@ impl Reader { /// Useful for snapshot tests that need to assert colour or style attributes. #[must_use] pub fn screen_contents_formatted(&self) -> Vec { - self.pty.get_ref().screen_contents_formatted() + self.pty.screen_contents_formatted() } /// Reads from the PTY until a milestone with the given name is encountered. @@ -64,10 +147,12 @@ impl Reader { /// Returns the terminal screen contents at the moment the milestone is detected. /// /// Milestones use a uniform protocol across platforms: the milestone name - /// is encoded in an OSC 8 hyperlink URI. We parse unhandled OSC sequences - /// from the VT parser state (instead of raw byte matching), then decode the - /// milestone URI payload. The zero-width milestone hyperlink anchor is - /// stripped from returned screen contents. + /// is encoded in an OSC 8 hyperlink URI. A zero-width hyperlink anchor follows + /// each marker through the rendered output path. The reader waits for both the + /// marker and its corresponding anchor before returning, then strips the anchor + /// from the returned screen contents. Marker and anchor parsing is incremental, + /// so either sequence may be split across PTY reads or share a read with other + /// milestones. /// /// # Panics /// @@ -78,20 +163,11 @@ impl Reader { let mut buf = [0u8; 4096]; loop { - let found = self - .pty - .get_ref() - .take_unhandled_osc_sequences() - .into_iter() - .filter_map(|params| { - pty_terminal_test_client::decode_milestone_from_osc8_params(¶ms) - }) - .any(|decoded| decoded == name); - if found { + if self.milestone_tracker.take_completed(name) { return self.screen_contents(); } - let n = self.pty.read(&mut buf).expect("PTY read failed"); + let n = self.read(&mut buf).expect("PTY read failed"); assert!(n > 0, "EOF reached before milestone '{name}'"); } } @@ -106,8 +182,75 @@ impl Reader { /// /// Panics if reading from the PTY fails. pub fn wait_for_exit(&mut self) -> anyhow::Result { - let mut discard = Vec::new(); - self.pty.read_to_end(&mut discard).expect("PTY read_to_end failed"); + let mut buf = [0u8; 4096]; + while self.read(&mut buf).expect("PTY read failed") > 0 {} self.child_handle.wait() } } + +#[cfg(test)] +mod tests { + use super::*; + + fn marker_without_fence(name: &str) -> Vec { + // Model ConPTY's fast control path by delivering the complete OSC marker + // before its printable anchor reaches the output pipe. + let mut marker = pty_terminal_test_client::encoded_milestone(name); + let index = marker + .windows(pty_terminal_test_client::MILESTONE_RENDER_FENCE.len()) + .position(|window| window == pty_terminal_test_client::MILESTONE_RENDER_FENCE) + .unwrap(); + marker.drain(index..index + pty_terminal_test_client::MILESTONE_RENDER_FENCE.len()); + marker + } + + fn advance(parser: &mut vte::Parser, tracker: &mut MilestoneTracker, bytes: &[u8]) { + parser.advance(tracker, bytes); + } + + #[test] + fn milestone_waits_for_rendered_fence() { + let mut parser = vte::Parser::new(); + let mut tracker = MilestoneTracker::default(); + + // Receiving the marker and subsequent printable output is insufficient: + // only the protocol's rendered anchor establishes the screen barrier. + advance(&mut parser, &mut tracker, &marker_without_fence("target")); + advance(&mut parser, &mut tracker, b"rendered output"); + assert!(!tracker.take_completed("target")); + + advance(&mut parser, &mut tracker, pty_terminal_test_client::MILESTONE_RENDER_FENCE); + assert!(tracker.take_completed("target")); + } + + #[test] + fn milestone_parses_across_every_chunk_boundary() { + let marker = pty_terminal_test_client::encoded_milestone("target"); + + for split in 0..=marker.len() { + let mut parser = vte::Parser::new(); + let mut tracker = MilestoneTracker::default(); + advance(&mut parser, &mut tracker, &marker[..split]); + advance(&mut parser, &mut tracker, &marker[split..]); + assert!(tracker.take_completed("target"), "failed at split {split}"); + } + } + + #[test] + fn rendered_fences_complete_overtaken_markers_in_order() { + let mut parser = vte::Parser::new(); + let mut tracker = MilestoneTracker::default(); + let mut markers = marker_without_fence("first"); + markers.extend(marker_without_fence("second")); + + // Both controls overtake rendering. The first anchor must still complete + // `first`, never whichever marker the caller happens to be waiting for. + advance(&mut parser, &mut tracker, &markers); + advance(&mut parser, &mut tracker, pty_terminal_test_client::MILESTONE_RENDER_FENCE); + assert!(tracker.take_completed("first")); + assert!(!tracker.take_completed("second")); + + advance(&mut parser, &mut tracker, pty_terminal_test_client::MILESTONE_RENDER_FENCE); + assert!(tracker.take_completed("second")); + } +} diff --git a/crates/pty_terminal_test_client/src/lib.rs b/crates/pty_terminal_test_client/src/lib.rs index 921f37ab8..d662e5f25 100644 --- a/crates/pty_terminal_test_client/src/lib.rs +++ b/crates/pty_terminal_test_client/src/lib.rs @@ -5,7 +5,16 @@ const OSC_ST: &str = "\x1b\\"; /// Invisible hyperlink text anchor. const MILESTONE_HYPERTEXT: &str = "\u{200b}"; /// OSC 8 close sequence. +/// +/// This terminates hyperlink metadata only. It is a control sequence and does +/// not guarantee that `ConPTY` has emitted preceding rendered text. pub const MILESTONE_FENCE: &[u8] = b"\x1b]8;;\x1b\\"; +/// Zero-width printable fence that follows each milestone marker. +/// +/// Unlike the OSC control sequences, `ConPTY` emits this character through its +/// rendering path. Observing it therefore confirms that earlier rendered text +/// has reached the reader. +pub const MILESTONE_RENDER_FENCE: &[u8] = MILESTONE_HYPERTEXT.as_bytes(); /// Builds an OSC 8 marker with milestone name encoded in the hyperlink URI. /// @@ -45,12 +54,12 @@ const fn decode_hex_nibble(byte: u8) -> Option { /// Returns `Some(name)` only when the URI uses the milestone prefix and the /// suffix is valid hex-encoded UTF-8. #[must_use] -pub fn decode_milestone_from_osc8_params(params: &[Vec]) -> Option { - if params.first().is_none_or(|p| p.as_slice() != b"8") { +pub fn decode_milestone_from_osc8_params>(params: &[T]) -> Option { + if params.first().is_none_or(|p| p.as_ref() != b"8") { return None; } - let uri = params.get(2)?.as_slice(); + let uri = params.get(2)?.as_ref(); let encoded = uri.strip_prefix(MILESTONE_URI_PREFIX.as_bytes())?; if encoded.is_empty() || encoded.len() % 2 != 0 { return None; @@ -80,7 +89,8 @@ pub fn decode_milestone_from_osc8_params(params: &[Vec]) -> Option { /// /// Milestones include a zero-width hyperlink anchor (`U+200B`) before closing. /// This keeps the hyperlink metadata observable in `ConPTY` output paths that can -/// drop zero-length hyperlinks. +/// drop zero-length hyperlinks. The test harness also waits for this rendered +/// character so preceding screen output cannot be overtaken by the OSC marker. /// /// When the `testing` feature is disabled, this is a no-op. /// @@ -93,7 +103,8 @@ pub fn mark_milestone(name: &str) { let milestone = encoded_milestone(name); let mut stdout = stdout(); - // Flush prior output, then emit milestone sequence. + // Flush prior output before emitting the marker. On ConPTY this flush alone + // is not a rendering barrier; the reader waits for MILESTONE_RENDER_FENCE. stdout.flush().unwrap(); stdout.write_all(&milestone).unwrap();