revert: remove rendered milestone fences (#528)

## What changed

Revert #527, removing the rendered zero-width milestone fence implementation and its dependency changes.

## Motivation

The anonymous FIFO fence protocol does not handle ConPTY coalescing or multiple milestone-producing processes reliably. Reverting it restores the prior marker behavior while a cross-platform window-title-based protocol with process-unique nonces is designed.

Author: wan9chi

Cargo.lock

@@ -161,7 +161,6 @@ 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"

Cargo.toml

@@ -12,7 +12,6 @@ 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 }
@@ -25,6 +24,7 @@ subprocess_test = { workspace = true, features = ["portable-pty"] }
 workspace = true
 
 [lib]
+test = false
 doctest = false
 
 [package.metadata.cargo-shear]

crates/pty_terminal_test/Cargo.toml

@@ -53,10 +53,10 @@ Milestones are encoded as an OSC 8 hyperlink:
 
 `Reader::expect_milestone` works like this:
 
-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()`.
+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()`.
 
 The helper strips the protocol's zero-width space from returned screen text.
 
@@ -65,8 +65,6 @@ 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
 

crates/pty_terminal_test/README.md

@@ -1,4 +1,4 @@
-use std::{collections::VecDeque, io::Read};
+use std::io::{BufReader, Read};
 
 pub use portable_pty::CommandBuilder;
 use pty_terminal::terminal::{PtyReader, Terminal};
@@ -10,58 +10,6 @@ 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<String>,
-    /// Marker names whose matching rendered anchors have arrived.
-    completed: VecDeque<String>,
-}
-
-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
@@ -75,17 +23,7 @@ pub struct TestTerminal {
 
 /// The read half of a test terminal, wrapping [`PtyReader`] with milestone support.
 pub struct Reader {
-    /// 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,
+    pty: BufReader<PtyReader>,
     child_handle: ChildHandle,
 }
 
@@ -99,38 +37,17 @@ impl TestTerminal {
         let Terminal { pty_reader, pty_writer, child_handle, .. } = Terminal::spawn(size, cmd)?;
         Ok(Self {
             writer: pty_writer,
-            reader: Reader {
-                pty: pty_reader,
-                milestone_parser: vte::Parser::new(),
-                milestone_tracker: MilestoneTracker::default(),
-                child_handle: child_handle.clone(),
-            },
+            reader: Reader { pty: BufReader::new(pty_reader), 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<usize> {
-        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.screen_contents();
+        let mut contents = self.pty.get_ref().screen_contents();
         contents.retain(|ch| ch != MILESTONE_HYPERTEXT);
         contents
     }
@@ -139,20 +56,18 @@ impl Reader {
     /// Useful for snapshot tests that need to assert colour or style attributes.
     #[must_use]
     pub fn screen_contents_formatted(&self) -> Vec<u8> {
-        self.pty.screen_contents_formatted()
+        self.pty.get_ref().screen_contents_formatted()
     }
 
     /// Reads from the PTY until a milestone with the given name is encountered.
     ///
     /// 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. 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.
+    /// 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.
     ///
     /// # Panics
     ///
@@ -163,11 +78,20 @@ impl Reader {
         let mut buf = [0u8; 4096];
 
         loop {
-            if self.milestone_tracker.take_completed(name) {
+            let found = self
+                .pty
+                .get_ref()
+                .take_unhandled_osc_sequences()
+                .into_iter()
+                .filter_map(|params| {
+                    pty_terminal_test_client::decode_milestone_from_osc8_params(&params)
+                })
+                .any(|decoded| decoded == name);
+            if found {
                 return self.screen_contents();
             }
 
-            let n = self.read(&mut buf).expect("PTY read failed");
+            let n = self.pty.read(&mut buf).expect("PTY read failed");
             assert!(n > 0, "EOF reached before milestone '{name}'");
         }
     }
@@ -182,75 +106,8 @@ impl Reader {
     ///
     /// Panics if reading from the PTY fails.
     pub fn wait_for_exit(&mut self) -> anyhow::Result<ExitStatus> {
-        let mut buf = [0u8; 4096];
-        while self.read(&mut buf).expect("PTY read failed") > 0 {}
+        let mut discard = Vec::new();
+        self.pty.read_to_end(&mut discard).expect("PTY read_to_end failed");
         self.child_handle.wait()
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    fn marker_without_fence(name: &str) -> Vec<u8> {
-        // 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"));
-    }
-}

crates/pty_terminal_test/src/lib.rs

@@ -5,16 +5,7 @@ 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.
 ///
@@ -54,12 +45,12 @@ const fn decode_hex_nibble(byte: u8) -> Option<u8> {
 /// 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<T: AsRef<[u8]>>(params: &[T]) -> Option<String> {
-    if params.first().is_none_or(|p| p.as_ref() != b"8") {
+pub fn decode_milestone_from_osc8_params(params: &[Vec<u8>]) -> Option<String> {
+    if params.first().is_none_or(|p| p.as_slice() != b"8") {
         return None;
     }
 
-    let uri = params.get(2)?.as_ref();
+    let uri = params.get(2)?.as_slice();
     let encoded = uri.strip_prefix(MILESTONE_URI_PREFIX.as_bytes())?;
     if encoded.is_empty() || encoded.len() % 2 != 0 {
         return None;
@@ -89,8 +80,7 @@ pub fn decode_milestone_from_osc8_params<T: AsRef<[u8]>>(params: &[T]) -> 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. The test harness also waits for this rendered
-/// character so preceding screen output cannot be overtaken by the OSC marker.
+/// drop zero-length hyperlinks.
 ///
 /// When the `testing` feature is disabled, this is a no-op.
 ///
@@ -103,8 +93,7 @@ pub fn mark_milestone(name: &str) {
 
     let milestone = encoded_milestone(name);
     let mut stdout = stdout();
-    // Flush prior output before emitting the marker. On ConPTY this flush alone
-    // is not a rendering barrier; the reader waits for MILESTONE_RENDER_FENCE.
+    // Flush prior output, then emit milestone sequence.
     stdout.flush().unwrap();
     stdout.write_all(&milestone).unwrap();