diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06dc1f404..04dc31bf8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -237,7 +237,7 @@ jobs: - shard: windows-ignored partition: '' mode: ignored - runs-on: windows-latest + runs-on: namespace-profile-windows-4c-8g env: PARTITION: ${{ matrix.partition }} steps: diff --git a/Cargo.lock b/Cargo.lock index 6a8fd0e9f..0ba95ea17 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2868,6 +2868,11 @@ dependencies = [ [[package]] name = "pty_terminal_test_client" version = "0.0.0" +dependencies = [ + "base64", + "getrandom 0.4.2", + "winapi", +] [[package]] name = "quote" diff --git a/Cargo.toml b/Cargo.toml index 93d6882ec..c734a89db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -81,6 +81,7 @@ fspy_shared_unix = { path = "crates/fspy_shared_unix" } futures = "0.3.31" futures-util = "0.3.31" globset = "0.4.18" +getrandom = "0.4.2" jsonc-parser = { version = "0.32.0", features = ["serde"] } libc = "0.2.185" libtest-mimic = "0.8.2" diff --git a/crates/pty_terminal/src/terminal.rs b/crates/pty_terminal/src/terminal.rs index 9ecf5a1e0..3382cfff4 100644 --- a/crates/pty_terminal/src/terminal.rs +++ b/crates/pty_terminal/src/terminal.rs @@ -55,13 +55,12 @@ pub struct Terminal { struct Vt100Callbacks { writer: Arc>>>, - unhandled_osc_sequences: VecDeque>>, + window_titles: VecDeque>, } impl vt100::Callbacks for Vt100Callbacks { - fn unhandled_osc(&mut self, _screen: &mut vt100::Screen, params: &[&[u8]]) { - let owned: Vec> = params.iter().map(|p| p.to_vec()).collect(); - self.unhandled_osc_sequences.push_back(owned); + fn set_window_title(&mut self, _screen: &mut vt100::Screen, title: &[u8]) { + self.window_titles.push_back(title.to_vec()); } fn unhandled_csi( @@ -175,17 +174,14 @@ impl PtyReader { out } - /// Drains and returns all unhandled OSC sequences received since the last call. - /// - /// Each entry is a list of byte-vector parameters from a single OSC sequence - /// (`ESC ] param1 ; param2 ; ... ST`). + /// Takes the next window title received while parsing PTY output. /// /// # Panics /// /// Panics if the parser lock is poisoned. #[must_use] - pub fn take_unhandled_osc_sequences(&self) -> VecDeque>> { - std::mem::take(&mut self.parser.lock().unwrap().callbacks_mut().unhandled_osc_sequences) + pub fn take_window_title(&self) -> Option> { + self.parser.lock().unwrap().callbacks_mut().window_titles.pop_front() } /// Returns the current cursor position as `(row, col)`, both 0-indexed. @@ -371,10 +367,7 @@ impl Terminal { size.rows, size.cols, 0, - Vt100Callbacks { - writer: Arc::clone(&writer), - unhandled_osc_sequences: VecDeque::new(), - }, + Vt100Callbacks { writer: Arc::clone(&writer), window_titles: VecDeque::new() }, ))); Ok(Self { diff --git a/crates/pty_terminal_test/README.md b/crates/pty_terminal_test/README.md index ab97238cd..27110c3b1 100644 --- a/crates/pty_terminal_test/README.md +++ b/crates/pty_terminal_test/README.md @@ -45,26 +45,25 @@ assert!(status.success()); ## Milestone protocol -Milestones are encoded as an OSC 8 hyperlink: +Milestones are encoded as unique window titles: -- open: `ESC ] 8 ; ; https://milestone.invalid/ ESC \` -- hypertext: zero-width space (`U+200B`) -- close: `ESC ] 8 ; ; ESC \` +```text +pty-terminal-test:<32-hex-random-id>: +``` `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()`. - -The helper strips the protocol's zero-width space from returned screen text. +1. Drain title events captured by `PtyReader`. +2. Ignore ordinary titles and completed non-target milestones. +3. If no match exists, continue reading from the PTY and repeat. +4. Return the current screen once the requested title is observed. ## Cross-platform behavior -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. +On Windows the client calls `SetConsoleTitleW`; ConPTY emits the resulting title +through its asynchronous renderer after preceding text and cursor state. On Unix +the client emits an OSC 2 title update, which follows normal PTY byte ordering. +The same token decoder and test API are used on every platform. ## Typical test pattern diff --git a/crates/pty_terminal_test/src/lib.rs b/crates/pty_terminal_test/src/lib.rs index af724599e..c3b69d8cf 100644 --- a/crates/pty_terminal_test/src/lib.rs +++ b/crates/pty_terminal_test/src/lib.rs @@ -8,8 +8,6 @@ pub use pty_terminal::{ terminal::{ChildHandle, PtyWriter}, }; -const MILESTONE_HYPERTEXT: char = '\u{200b}'; - /// A test-oriented terminal that provides milestone-based synchronization. /// /// Wraps a PTY terminal, splitting it into a [`PtyWriter`] for sending input @@ -44,12 +42,10 @@ impl TestTerminal { } impl Reader { - /// Returns terminal screen contents with milestone hyperlink text removed. + /// Returns the current terminal screen contents. #[must_use] pub fn screen_contents(&self) -> String { - let mut contents = self.pty.get_ref().screen_contents(); - contents.retain(|ch| ch != MILESTONE_HYPERTEXT); - contents + self.pty.get_ref().screen_contents() } /// Returns the screen contents with inline ANSI SGR escape codes preserved. @@ -63,11 +59,7 @@ 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. + /// Milestones use a uniform title token across platforms. /// /// # Panics /// @@ -78,17 +70,12 @@ 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 { - return self.screen_contents(); + while let Some(title) = self.pty.get_ref().take_window_title() { + if pty_terminal_test_client::decode_milestone_title(&title) + .is_some_and(|milestone| milestone == name) + { + return self.screen_contents(); + } } let n = self.pty.read(&mut buf).expect("PTY read failed"); diff --git a/crates/pty_terminal_test/tests/milestone.rs b/crates/pty_terminal_test/tests/milestone.rs index e878e0690..0b7bba03e 100644 --- a/crates/pty_terminal_test/tests/milestone.rs +++ b/crates/pty_terminal_test/tests/milestone.rs @@ -71,7 +71,7 @@ fn milestone_raw_mode_keystrokes() { assert!(status.success()); } -/// Verifies that the non-visual milestone fence in `mark_milestone` does not +/// Verifies that the non-visual milestone title in `mark_milestone` does not /// pollute `screen_contents()`. The subprocess appends characters without /// clearing the screen, so any leftover space would appear between them. #[test] @@ -125,3 +125,30 @@ fn milestone_does_not_pollute_screen() { let status = reader.wait_for_exit().unwrap(); assert!(status.success()); } + +#[test] +#[timeout(5000)] +#[expect(clippy::redundant_clone, reason = "command_for_fn evaluates its argument twice")] +fn descendant_process_can_mark_milestone() { + let nested = command_for_fn!((), |(): ()| { + pty_terminal_test_client::mark_milestone("nested"); + }); + let nested_program = nested.program.to_string_lossy().into_owned(); + let nested_args = + nested.args.iter().map(|arg| arg.to_string_lossy().into_owned()).collect::>(); + + let cmd = CommandBuilder::from(command_for_fn!( + (nested_program.clone(), nested_args.clone()), + |(nested_program, nested_args): (String, Vec)| { + pty_terminal_test_client::mark_milestone("root"); + let status = + std::process::Command::new(nested_program).args(nested_args).status().unwrap(); + assert!(status.success()); + } + )); + + let TestTerminal { writer: _, mut reader, child_handle: _ } = + TestTerminal::spawn(ScreenSize { rows: 80, cols: 80 }, cmd).unwrap(); + let _ = reader.expect_milestone("nested"); + assert!(reader.wait_for_exit().unwrap().success()); +} diff --git a/crates/pty_terminal_test_client/Cargo.toml b/crates/pty_terminal_test_client/Cargo.toml index 3ac4e8ff5..8af6077af 100644 --- a/crates/pty_terminal_test_client/Cargo.toml +++ b/crates/pty_terminal_test_client/Cargo.toml @@ -9,11 +9,20 @@ rust-version.workspace = true [features] default = [] -testing = [] +testing = ["dep:getrandom", "dep:winapi"] + +[dependencies] +base64 = { workspace = true } +getrandom = { workspace = true, optional = true } + +[target.'cfg(windows)'.dependencies] +winapi = { workspace = true, features = ["wincon"], optional = true } + +[dev-dependencies] +getrandom = { workspace = true } [lints] workspace = true [lib] -test = false doctest = false diff --git a/crates/pty_terminal_test_client/src/lib.rs b/crates/pty_terminal_test_client/src/lib.rs index 921f37ab8..c6afd8f82 100644 --- a/crates/pty_terminal_test_client/src/lib.rs +++ b/crates/pty_terminal_test_client/src/lib.rs @@ -1,107 +1,108 @@ -/// Prefix for hyperlink URI payload that carries milestone data. -const MILESTONE_URI_PREFIX: &str = "https://milestone.invalid/"; -/// Terminator for OSC sequences using ST (`ESC \`). -const OSC_ST: &str = "\x1b\\"; -/// Invisible hyperlink text anchor. -const MILESTONE_HYPERTEXT: &str = "\u{200b}"; -/// OSC 8 close sequence. -pub const MILESTONE_FENCE: &[u8] = b"\x1b]8;;\x1b\\"; +use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; -/// Builds an OSC 8 marker with milestone name encoded in the hyperlink URI. -/// -/// Format: -/// `OSC 8 ; ; https://milestone.invalid/ ST OSC 8 ; ; ST`. -#[must_use] -pub fn encoded_milestone(name: &str) -> Vec { - use std::fmt::Write as _; - - let mut hex = String::with_capacity(name.len() * 2); - for &byte in name.as_bytes() { - write!(&mut hex, "{byte:02x}").unwrap(); - } +const MILESTONE_TITLE_MARKER: &str = "pty-terminal-test:"; - let mut seq = String::new(); - write!(&mut seq, "\x1b]8;;{MILESTONE_URI_PREFIX}{hex}{OSC_ST}").unwrap(); - seq.push_str(MILESTONE_HYPERTEXT); - write!(&mut seq, "\x1b]8;;{OSC_ST}").unwrap(); - seq.into_bytes() -} - -const fn decode_hex_nibble(byte: u8) -> Option { - match byte { - b'0'..=b'9' => Some(byte - b'0'), - b'a'..=b'f' => Some(byte - b'a' + 10), - b'A'..=b'F' => Some(byte - b'A' + 10), - _ => None, - } +#[cfg(any(feature = "testing", test))] +fn encode_milestone_title(name: &str) -> String { + let mut random = [0u8; 16]; + getrandom::fill(&mut random).expect("failed to generate milestone identity"); + let id = u128::from_be_bytes(random); + let encoded_name = URL_SAFE_NO_PAD.encode(name.as_bytes()); + format!("{MILESTONE_TITLE_MARKER}{id:032x}:{encoded_name}") } -/// Decodes a milestone name from OSC 8 parameters if present. -/// -/// Expects VT parser parameters for OSC 8 in the form: -/// - open: `["8", "", ""]` -/// - close: `["8", "", ""]` -/// -/// Returns `Some(name)` only when the URI uses the milestone prefix and the -/// suffix is valid hex-encoded UTF-8. +/// Decodes a milestone title, ignoring ordinary application title updates. #[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_title(title: &[u8]) -> Option { + let encoded = title.strip_prefix(MILESTONE_TITLE_MARKER.as_bytes())?; + let (encoded_id, encoded_name) = encoded.split_at_checked(32)?; + let (&b':', encoded_name) = encoded_name.split_first()? else { return None; - } - - 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 { + }; + if !encoded_id.iter().all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(byte)) { return None; } - let mut bytes = Vec::with_capacity(encoded.len() / 2); - for pair in encoded.chunks_exact(2) { - let high = decode_hex_nibble(pair[0])?; - let low = decode_hex_nibble(pair[1])?; - bytes.push((high << 4) | low); + let name_bytes = URL_SAFE_NO_PAD.decode(encoded_name).ok()?; + if URL_SAFE_NO_PAD.encode(&name_bytes).as_bytes() != encoded_name { + return None; } - - String::from_utf8(bytes).ok() + String::from_utf8(name_bytes).ok() } -/// Emits a milestone marker as OSC 8 hyperlink metadata. +/// Emits a milestone marker as a unique window-title update. /// /// The child process calls this to signal it has reached a named synchronization /// point. The test harness (via `pty_terminal_test::Reader::expect_milestone`) /// detects this marker and returns the screen contents at that point. /// -/// On Windows, `ConPTY` passes control sequences directly to the -/// output pipe (synchronous, inline with input processing), while rendered -/// character output is generated asynchronously by a separate output thread -/// that polls the console buffer. This means the marker can arrive at the -/// reader before preceding character output has been emitted. -/// -/// 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. +/// Windows uses `SetConsoleTitleW`, which `ConPTY` emits through its renderer after +/// preceding text and cursor state. Other platforms emit the equivalent OSC 2 +/// title update through the ordered PTY byte stream. /// /// When the `testing` feature is disabled, this is a no-op. /// /// # Panics /// -/// Panics if writing to stdout fails. +/// Panics if secure randomness is unavailable or emitting the title fails. #[cfg(feature = "testing")] pub fn mark_milestone(name: &str) { - use std::io::{Write, stdout}; + emit_title(&encode_milestone_title(name)).expect("failed to emit milestone title"); +} - let milestone = encoded_milestone(name); - let mut stdout = stdout(); - // Flush prior output, then emit milestone sequence. - stdout.flush().unwrap(); - stdout.write_all(&milestone).unwrap(); +#[cfg(all(feature = "testing", windows))] +fn emit_title(title: &str) -> std::io::Result<()> { + use std::io::Write as _; - stdout.flush().unwrap(); + std::io::stdout().flush()?; + let mut wide = title.encode_utf16().collect::>(); + wide.push(0); + // SAFETY: `wide` is a valid NUL-terminated UTF-16 title. + if unsafe { winapi::um::wincon::SetConsoleTitleW(wide.as_ptr()) } == 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(()) + } } -/// Emits a milestone marker as a private OSC escape sequence. +#[cfg(all(feature = "testing", not(windows)))] +fn emit_title(title: &str) -> std::io::Result<()> { + use std::io::Write as _; + + let mut stdout = std::io::stdout().lock(); + stdout.flush()?; + write!(stdout, "\x1b]2;{title}\x1b\\")?; + stdout.flush() +} + +/// Does nothing when milestone instrumentation is disabled. /// /// When the `testing` feature is disabled, this is a no-op. #[cfg(not(feature = "testing"))] pub const fn mark_milestone(_name: &str) {} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn title_round_trip() { + let title = encode_milestone_title("task-select:lib#:0"); + assert_eq!(decode_milestone_title(title.as_bytes()).as_deref(), Some("task-select:lib#:0")); + } + + #[test] + fn repeated_names_get_unique_titles() { + assert_ne!(encode_milestone_title("ready"), encode_milestone_title("ready")); + } + + #[test] + fn ignores_normal_and_malformed_titles() { + assert!(decode_milestone_title(b"normal title").is_none()); + assert!(decode_milestone_title(b"pty-terminal-test:not-hex:cmVhZHk").is_none()); + assert!( + decode_milestone_title(b"pty-terminal-test:00000000000000000000000000000000:*") + .is_none() + ); + } +} diff --git a/crates/vite_task/src/session/mod.rs b/crates/vite_task/src/session/mod.rs index 5a5fb160c..b7768f68f 100644 --- a/crates/vite_task/src/session/mod.rs +++ b/crates/vite_task/src/session/mod.rs @@ -535,13 +535,9 @@ impl<'a> Session<'a> { }; let select_result = vite_select::select_list(&mut stdout, ¶ms, mode, |state| { - use std::io::Write; let milestone_name = vite_str::format!("task-select:{}:{}", state.query, state.selected_index); - let milestone_bytes = pty_terminal_test_client::encoded_milestone(&milestone_name); - let mut out = std::io::stdout(); - let _ = out.write_all(&milestone_bytes); - let _ = out.flush(); + pty_terminal_test_client::mark_milestone(&milestone_name); })?; if matches!(select_result, vite_select::SelectResult::Cancelled) { diff --git a/crates/vite_task_bin/Cargo.toml b/crates/vite_task_bin/Cargo.toml index b8a8b8d8c..a0d8b8ba3 100644 --- a/crates/vite_task_bin/Cargo.toml +++ b/crates/vite_task_bin/Cargo.toml @@ -20,7 +20,7 @@ anyhow = { workspace = true } ctrlc = { workspace = true } libc = { workspace = true } notify = { workspace = true } -pty_terminal_test_client = { workspace = true, features = ["testing"] } +pty_terminal_test_client = { workspace = true } async-trait = { workspace = true } clap = { workspace = true, features = ["derive"] } jsonc-parser = { workspace = true } @@ -38,6 +38,7 @@ libtest-mimic = { workspace = true } snapshot_test = { workspace = true } pty_terminal = { workspace = true } pty_terminal_test = { workspace = true } +pty_terminal_test_client = { workspace = true, features = ["testing"] } regex = { workspace = true } serde = { workspace = true, features = ["derive", "rc"] } shell-escape = { workspace = true } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/main.rs b/crates/vite_task_bin/tests/e2e_snapshots/main.rs index 0fc54c43f..ce5f81176 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/main.rs +++ b/crates/vite_task_bin/tests/e2e_snapshots/main.rs @@ -438,7 +438,7 @@ fn run_case( cmd.env("PATH", &e2e_env_path); // Use `xterm-256color` and report color support so the runner does // NOT wrap its output writers with [`anstream::StripStream`]. The - // strip layer would otherwise eat OSC8 milestone sequences that + // strip layer would otherwise eat OSC title sequences that // the test harness relies on to synchronise with the child. ANSI // escapes emitted by the runner still get flattened to plain text // by vt100's `Screen::contents()` when the snapshot is rendered,