From 564d374311a372a213b412ae3932303f378a0066 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 15:51:49 -0700 Subject: [PATCH 1/8] feat(emulator): capture OSC 52 clipboard stores for forwarding --- src/terminal/emulator.rs | 220 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 210 insertions(+), 10 deletions(-) diff --git a/src/terminal/emulator.rs b/src/terminal/emulator.rs index c280627..61eb65d 100644 --- a/src/terminal/emulator.rs +++ b/src/terminal/emulator.rs @@ -12,7 +12,7 @@ use alacritty_terminal::{ grid::{Dimensions, Scroll}, index::{Column, Line}, term::{ - Config, TermMode, + ClipboardType, Config, TermMode, cell::{Cell, Flags}, }, vte::ansi::{self as vt, Handler, Processor}, @@ -35,21 +35,74 @@ pub enum MouseProtocolEncoding { Sgr, } -/// Buffers backend-generated PTY responses while the parser advances. Other -/// backend events are discarded; [`ObservedTerm`] captures titles directly -/// from parser events. +/// Buffered-store cap in bytes for one OSC 52 clipboard payload. The cap +/// bounds forwarding-frame size and per-task memory, not what a host +/// clipboard could hold; an over-cap store is dropped and its length +/// recorded so the caller can surface a notice. +const CLIPBOARD_STORE_MAX_BYTES: usize = 1024 * 1024; + +// A maximum-size store expands to this base64 bound when re-encoded for +// forwarding. Reserve 64 KiB for the command envelope and keep the result +// within one frame. +const _: () = assert!( + CLIPBOARD_STORE_MAX_BYTES.div_ceil(3) * 4 + 64 * 1024 <= crate::frame::MAX_FRAME as usize, + "CLIPBOARD_STORE_MAX_BYTES must base64-encode to under frame::MAX_FRAME" +); + +/// OSC 52 clipboard stores captured since the last drain. A clipboard is +/// last-writer-wins by nature, so coalescing to one store per +/// [`ClipboardType`] between drains loses nothing, and the two-slot bound +/// means a never-drained background task cannot accumulate memory. +#[derive(Debug, Default)] +pub struct ClipboardStores { + /// At most one store per [`ClipboardType`], ordered by the surviving + /// store's arrival. + pub stores: Vec<(ClipboardType, String)>, + /// Byte length of the most recent store dropped for exceeding + /// [`CLIPBOARD_STORE_MAX_BYTES`], kept so the caller can surface a + /// notice instead of silently losing the copy. + pub oversized_len: Option, +} + +/// Buffers backend-generated PTY responses and OSC 52 clipboard stores while +/// the parser advances. Other backend events are discarded; [`ObservedTerm`] +/// captures titles directly from parser events. pub struct ProbeSink { responses: Arc>>, + clipboard: Arc>, } impl EventListener for ProbeSink { fn send_event(&self, event: Event) { - if let Event::PtyWrite(text) = event { - let mut buf = self - .responses - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - buf.push(text); + match event { + Event::PtyWrite(text) => { + let mut buf = self + .responses + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + buf.push(text); + } + // The backend has already base64-decoded the payload, validated + // UTF-8, mapped the kind byte, and denied OSC 52 loads under its + // default `Osc52::OnlyCopy`: only decoded stores arrive here. + Event::ClipboardStore(kind, text) => { + let mut buf = self + .clipboard + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + // Last writer wins per kind even when the last writer is + // over-cap: forwarding a superseded store would misrepresent + // the child's final clipboard state, so the drop clears its + // kind's slot and the recorded length feeds the caller's + // notice. + buf.stores.retain(|(k, _)| *k != kind); + if text.len() > CLIPBOARD_STORE_MAX_BYTES { + buf.oversized_len = Some(text.len()); + return; + } + buf.stores.push((kind, text)); + } + _ => {} } } } @@ -177,6 +230,11 @@ pub struct Emulator { term: Term, parser: Processor, responses: Arc>>, + // Read only by `drain_clipboard`, which has no production caller until + // the supervisor's forwarding tick lands; the allow is scoped to the + // non-test build because tests do read it. + #[cfg_attr(not(test), allow(dead_code))] + clipboard: Arc>, /// Alt-screen and title facts, advanced at parser-event granularity by /// [`ObservedTerm`] during the parse itself. alt: AltScreen, @@ -201,6 +259,23 @@ impl Emulator { .collect() } + /// Drain the OSC 52 clipboard stores captured since the last drain, + /// plus the oversized-drop record. The caller owns forwarding; a drain + /// whose result is dropped discards the stores. An empty capture costs + /// one lock and no allocation, so calling every tick for every task is + /// fine. + // No production caller until the supervisor's forwarding tick lands; + // tests drain meanwhile, so the allow is scoped to the non-test build + // (`expect` would be unfulfilled in the test target). + #[cfg_attr(not(test), allow(dead_code))] + pub fn drain_clipboard(&mut self) -> ClipboardStores { + let mut buf = self + .clipboard + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + std::mem::take(&mut *buf) + } + /// Truncate zero-width characters in each active-grid cell to /// `MAX_ZEROWIDTH`. The scan covers the viewport and all scrollback rows. /// @@ -242,6 +317,7 @@ impl Emulator { /// A fresh `rows`×`cols` grid retaining `scrollback` rows of history. pub fn new(rows: u16, cols: u16, scrollback: usize) -> Self { let responses = Arc::new(Mutex::new(Vec::new())); + let clipboard = Arc::new(Mutex::new(ClipboardStores::default())); let config = Config { // Use fleetcom's per-task history limit instead of the backend // default. @@ -256,12 +332,14 @@ impl Emulator { }, ProbeSink { responses: Arc::clone(&responses), + clipboard: Arc::clone(&clipboard), }, ); Self { term, parser: Processor::new(), responses, + clipboard, alt: AltScreen::default(), revision: 0, bytes_since_sweep: 0, @@ -1014,6 +1092,128 @@ mod tests { assert!(emu.process(b"\x1b[?u").is_empty()); } + /// End to end through `process`: an OSC 52 store is captured, produces + /// no probe reply, and drains exactly once. + #[test] + fn osc52_store_is_captured_and_drains_once() { + let mut emu = Emulator::new(4, 20, 0); + assert!( + emu.process(b"\x1b]52;c;aGVsbG8=\x07").is_empty(), + "a store is not a probe reply" + ); + let drained = emu.drain_clipboard(); + assert_eq!( + drained.stores, + vec![(ClipboardType::Clipboard, "hello".to_string())] + ); + assert_eq!(drained.oversized_len, None); + let again = emu.drain_clipboard(); + assert!(again.stores.is_empty(), "a drain empties the buffer"); + assert_eq!(again.oversized_len, None); + } + + /// The `p` and `s` kind bytes both map to the selection clipboard + /// (the backend's mapping; there is no separate primary kind). + #[test] + fn osc52_p_and_s_kinds_map_to_selection() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;p;YQ==\x07"); + assert_eq!( + emu.drain_clipboard().stores, + vec![(ClipboardType::Selection, "a".to_string())] + ); + emu.process(b"\x1b]52;s;Yg==\x07"); + assert_eq!( + emu.drain_clipboard().stores, + vec![(ClipboardType::Selection, "b".to_string())] + ); + } + + /// Two stores to the same kind before a drain coalesce: only the second + /// survives. + #[test] + fn osc52_last_store_wins_per_kind() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;c;Zmlyc3Q=\x07\x1b]52;c;c2Vjb25k\x07"); + assert_eq!( + emu.drain_clipboard().stores, + vec![(ClipboardType::Clipboard, "second".to_string())] + ); + } + + /// Stores to different kinds buffer independently and drain in arrival + /// order. + #[test] + fn osc52_kinds_buffer_independently() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;s;c2Vs\x07\x1b]52;c;Y2xpcA==\x07"); + assert_eq!( + emu.drain_clipboard().stores, + vec![ + (ClipboardType::Selection, "sel".to_string()), + (ClipboardType::Clipboard, "clip".to_string()), + ] + ); + } + + /// Invalid base64 and the `!` clear form die inside the backend's decode + /// before the capture point: nothing buffers. + #[test] + fn osc52_invalid_base64_and_clear_buffer_nothing() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;c;%%%\x07"); + emu.process(b"\x1b]52;c;!\x07"); + let drained = emu.drain_clipboard(); + assert!(drained.stores.is_empty()); + assert_eq!(drained.oversized_len, None); + } + + /// The query form is an OSC 52 load, which the backend's default + /// `Osc52::OnlyCopy` denies: nothing buffers and no reply is generated + /// for the child. + #[test] + fn osc52_query_is_denied_without_a_reply() { + let mut emu = Emulator::new(4, 20, 0); + assert!(emu.process(b"\x1b]52;c;?\x07").is_empty()); + let drained = emu.drain_clipboard(); + assert!(drained.stores.is_empty()); + assert_eq!(drained.oversized_len, None); + } + + /// vte accepts ST as the OSC terminator alongside BEL. + #[test] + fn osc52_st_terminated_store_is_captured() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;c;aGVsbG8=\x1b\\"); + assert_eq!( + emu.drain_clipboard().stores, + vec![(ClipboardType::Clipboard, "hello".to_string())] + ); + } + + /// An over-cap store is not buffered, but it still supersedes its + /// kind's slot: forwarding the older store would misrepresent the + /// child's final clipboard state. The recorded length feeds the + /// caller's notice; other kinds are untouched. + #[test] + fn osc52_oversized_store_supersedes_its_kind_and_records_length() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;c;aGVsbG8=\x07"); + emu.process(b"\x1b]52;s;c2Vs\x07"); + // "YWFh" decodes to "aaa"; this repeat count decodes to two bytes + // over the cap. + let reps = CLIPBOARD_STORE_MAX_BYTES / 3 + 1; + let payload = "YWFh".repeat(reps); + emu.process(format!("\x1b]52;c;{payload}\x07").as_bytes()); + let drained = emu.drain_clipboard(); + assert_eq!( + drained.stores, + vec![(ClipboardType::Selection, "sel".to_string())], + "the drop clears its own kind's slot and no other" + ); + assert_eq!(drained.oversized_len, Some(reps * 3)); + } + /// The stall the flush hook exists for: BSU plus a partial frame, then /// silence. The buffered frame must stay invisible while the timeout is /// pending (the hook must not tear an in-flight frame) and flush once the From 3da3807febc582e77ea8cf29212e26382bf48dbe Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 16:07:03 -0700 Subject: [PATCH 2/8] feat(supervisor): plumb OSC 52 clipboard stores to the client --- src/app.rs | 3 + src/protocol.rs | 106 +++++++++++++++++++++++++++++ src/supervisor.rs | 43 +++++++++++- src/supervisor_tests.rs | 143 +++++++++++++++++++++++++++++++++++++++ src/task.rs | 10 ++- src/terminal/emulator.rs | 10 +-- 6 files changed, 304 insertions(+), 11 deletions(-) diff --git a/src/app.rs b/src/app.rs index 0a8d0bc..c985c57 100644 --- a/src/app.rs +++ b/src/app.rs @@ -617,6 +617,9 @@ impl App { self.focused_screen = Some(s); } Event::Status(s) => self.status = Some(s), + // Dropped here until the client-side clipboard write lands + // in a later phase. + Event::ClipboardCopy { .. } => {} Event::Sessions { names, recovery } => { // Clamp both page selections to the refreshed lists. self.session_sel = self.session_sel.min(names.len().saturating_sub(1)); diff --git a/src/protocol.rs b/src/protocol.rs index e2e01c9..f8b559a 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -196,6 +196,21 @@ pub enum Event { names: Vec, recovery: Vec, }, + /// One OSC 52 clipboard store from the watched task, already base64-decoded + /// by the terminal backend. Store-only: no load or query path exists, so + /// this event never solicits a reply. + ClipboardCopy { kind: ClipboardKind, text: String }, +} + +/// Which clipboard an OSC 52 store targets. A wire-layer mirror of the +/// emulator's `ClipboardType`, kept here so `protocol` stays free of +/// `alacritty_terminal` types; the supervisor maps at its boundary. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ClipboardKind { + /// The system clipboard (OSC 52 kind byte `c`). + Clipboard, + /// The primary selection (OSC 52 kind byte `s`). + Selection, } /// Recovery-snapshot metadata sent to the session picker. @@ -884,6 +899,23 @@ pub fn encode_event(ev: &Event) -> (u8, Vec) { let _ = o.insert("recovery", rec); (KIND_CONTROL, o.dump().into_bytes()) } + // Base64 like `Command::Input`: clipboard text is arbitrary child + // output and must cross the wire unsanitized. The emulator's 1 MiB + // store cap keeps the encoded frame far under `frame::MAX_FRAME` + // (see `CLIPBOARD_STORE_MAX_BYTES`'s const assertion). + Event::ClipboardCopy { kind, text } => { + let mut o = jzon::JsonValue::new_object(); + let _ = o.insert("t", "clip"); + let _ = o.insert( + "k", + match kind { + ClipboardKind::Clipboard => "c", + ClipboardKind::Selection => "s", + }, + ); + let _ = o.insert("text", B64.encode(text.as_bytes())); + (KIND_CONTROL, o.dump().into_bytes()) + } Event::Screen(sv) => { let mut header = jzon::JsonValue::new_object(); let _ = header.insert("id", sv.id); @@ -968,6 +1000,17 @@ pub fn decode_event(kind: u8, payload: &[u8]) -> Option { names: str_vec(&v["names"])?, recovery: recovery_vec(&v["recovery"]), }), + "clip" => { + let kind = match v["k"].as_str()? { + "c" => ClipboardKind::Clipboard, + "s" => ClipboardKind::Selection, + _ => return None, + }; + // The store was UTF-8 when the emulator captured it; a + // frame that decodes to anything else is malformed. + let text = String::from_utf8(B64.decode(v["text"].as_str()?).ok()?).ok()?; + Some(Event::ClipboardCopy { kind, text }) + } _ => None, } } @@ -1721,4 +1764,67 @@ mod tests { assert_eq!(k, KIND_SCREEN); assert_eq!(decode_event(k, &p), Some(screen)); } + + /// `ClipboardCopy` round-trips both kinds and carries clipboard text + /// unsanitized: unicode, control characters, and embedded markers survive + /// the JSON+base64 path byte-for-byte. + #[test] + fn clipboard_copy_round_trips() { + for ev in [ + Event::ClipboardCopy { + kind: ClipboardKind::Clipboard, + text: "hello".into(), + }, + Event::ClipboardCopy { + kind: ClipboardKind::Selection, + text: "sélection λ 🦀".into(), + }, + Event::ClipboardCopy { + kind: ClipboardKind::Clipboard, + // Clipboard content is arbitrary: newlines, tabs, NUL, ESC, + // and paste-marker-shaped text must not be sanitized in + // transit. + text: "line1\nline2\tcol\u{0}\u{1b}[201~end".into(), + }, + ] { + let (k, p) = encode_event(&ev); + assert_eq!(k, KIND_CONTROL); + assert_eq!(decode_event(k, &p), Some(ev)); + } + } + + /// `ClipboardCopy` pins its wire shape: `t` discriminator, the OSC 52 + /// kind byte under `k`, and base64 text. + #[test] + fn clipboard_copy_wire_form() { + let (k, p) = encode_event(&Event::ClipboardCopy { + kind: ClipboardKind::Selection, + text: "hi".into(), + }); + assert_eq!(k, KIND_CONTROL); + assert_eq!( + std::str::from_utf8(&p).unwrap(), + r#"{"t":"clip","k":"s","text":"aGk="}"# + ); + } + + /// A malformed `ClipboardCopy` frame is rejected whole: missing fields, + /// an unknown kind string, invalid base64, and non-UTF-8 decoded bytes. + #[test] + fn malformed_clipboard_copy_is_rejected() { + for json in [ + r#"{"t":"clip","text":"aGk="}"#, // missing kind + r#"{"t":"clip","k":"c"}"#, // missing text + r#"{"t":"clip","k":"p","text":"aGk="}"#, // unknown kind string + r#"{"t":"clip","k":"c","text":"!!!"}"#, // invalid base64 + r#"{"t":"clip","k":"c","text":"/w=="}"#, // 0xFF: not UTF-8 + r#"{"t":"clip","k":"c","text":["aGk="]}"#, // text must be a string + ] { + assert_eq!( + decode_event(KIND_CONTROL, json.as_bytes()), + None, + "should reject {json}" + ); + } + } } diff --git a/src/supervisor.rs b/src/supervisor.rs index a5e2126..e921a6f 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -13,11 +13,15 @@ use std::{ time::{Duration, Instant}, }; +use alacritty_terminal::term::ClipboardType; + use crate::{ core::{Wake, Waker}, harness::{self, assets}, path, - protocol::{Command, Event, LaunchContext, ScreenView, ScrollAction, TaskView, env_get}, + protocol::{ + ClipboardKind, Command, Event, LaunchContext, ScreenView, ScrollAction, TaskView, env_get, + }, session::{self, SessionConfig, SessionEntry}, task::Task, }; @@ -119,6 +123,15 @@ fn normalize_group(name: Option) -> Option { normalize_label(name).filter(|g| g != "Unassigned") } +/// Map the emulator's clipboard kind to its wire mirror. The boundary where +/// `alacritty_terminal` types stop: `protocol` deliberately imports none. +fn clipboard_kind(kind: ClipboardType) -> ClipboardKind { + match kind { + ClipboardType::Clipboard => ClipboardKind::Clipboard, + ClipboardType::Selection => ClipboardKind::Selection, + } +} + /// Return the 64-bit FNV-1a hash used to separate fallback capture roots. The /// fixed-width hexadecimal result is a single path-safe component. fn fnv1a_hex(bytes: &[u8]) -> String { @@ -519,11 +532,23 @@ impl Supervisor { // least every 200 ms), so an expired sync flushes here, before the // preview resolution reads the grid, letting the same tick ship it. // Resolution mutates per-task hold state; all tasks use one timestamp. + let watched = self.watched; + let mut clipboard = None; let views = self .tasks .iter_mut() .map(|t| { t.flush_expired_sync(); + // Drain every task's clipboard every tick and forward only the + // watched task's. Dropping the others here is the staleness + // guarantee: a store captured while backgrounded must never + // fire when the task is later watched — a wrong clipboard is + // silently harmful, an empty one visibly inert. The two-slot + // capture bound makes the constant drain cheap. + let stores = t.drain_clipboard(); + if watched == Some(t.id) { + clipboard = Some(stores); + } TaskView { id: t.id, command: t.command.clone(), @@ -542,6 +567,22 @@ impl Supervisor { .collect(); self.events.push(Event::Tasks(views)); + if let Some(stores) = clipboard { + for (kind, text) in stores.stores { + self.events.push(Event::ClipboardCopy { + kind: clipboard_kind(kind), + text, + }); + } + if let Some(len) = stores.oversized_len { + self.status(format!( + "clipboard copy dropped: {} exceeds the {} limit", + crate::format::bytes(len), + crate::format::bytes(crate::emulator::CLIPBOARD_STORE_MAX_BYTES) + )); + } + } + if let Some(id) = self.watched && let Some(t) = self.tasks.iter().find(|t| t.id == id) { diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index 71a720f..d298a02 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -213,6 +213,149 @@ fn tick_flushes_a_stalled_sync_update() { ); } +/// OSC 52 stores from the watched task reach `drain` as decoded +/// `ClipboardCopy` events, one per store in arrival order. The emission is +/// flag-gated so `Watch` is installed before any store can arrive. +#[test] +fn watched_task_clipboard_stores_are_forwarded() { + let dir = scratch("clip_fwd"); + let ready = dir.join("ready"); + let flag = dir.join("flag"); + let mut s = sup(24, 80); + // "aGVsbG8=" is "hello" (clipboard), "d29ybGQ=" is "world" (selection). + let cmd = format!( + "touch {r}; until [ -e {f} ]; do sleep 0.05; done; \ + printf '\\033]52;c;aGVsbG8=\\007\\033]52;s;d29ybGQ=\\007'; sleep 30", + r = ready.display(), + f = flag.display() + ); + let id = spawn_ready(&mut s, cmd, here(), &ready); + s.apply(Command::Watch { id: Some(id) }); + std::fs::write(&flag, b"").unwrap(); + + let mut copies = Vec::new(); + let ok = wait_until(Duration::from_secs(5), || { + s.tick(); + copies.extend(s.drain().into_iter().filter_map(|e| match e { + Event::ClipboardCopy { kind, text } => Some((kind, text)), + _ => None, + })); + copies.len() >= 2 + }); + assert!(ok, "the clipboard stores never arrived; got {copies:?}"); + assert_eq!( + copies, + vec![ + (ClipboardKind::Clipboard, "hello".to_string()), + (ClipboardKind::Selection, "world".to_string()), + ] + ); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A store captured while the task is not watched is discarded by the +/// per-tick drain, never deferred: watching the task afterwards forwards +/// nothing. Staleness is worse than loss — a wrong clipboard is silently +/// harmful, an empty one visibly inert. +#[test] +fn backgrounded_clipboard_store_is_discarded_not_deferred() { + let mut s = sup(24, 80); + // "c3RhbGU=" is "stale". The trailing marker proves the store's bytes + // were parsed: it follows them in the output stream. + spawn( + &mut s, + "printf '\\033]52;c;c3RhbGU=\\007COPIED'; sleep 30", + here(), + ); + let mut id = 0; + let parsed = wait_until(Duration::from_secs(5), || { + s.tick(); + let mut seen = false; + for e in s.drain() { + match e { + Event::Tasks(v) => { + if let Some(t) = v.first() { + id = t.id; + seen = t.preview.text.contains("COPIED"); + } + } + Event::ClipboardCopy { .. } => { + panic!("an unwatched task's store must not be forwarded") + } + _ => {} + } + } + seen + }); + assert!(parsed, "the marker never reached the grid"); + // The marker only proves the store was parsed by that tick's preview + // resolution, which runs after the drain; one more tick guarantees a + // drain after capture. + s.tick(); + let _ = s.drain(); + + s.apply(Command::Watch { id: Some(id) }); + let mut saw_screen = false; + for _ in 0..3 { + s.tick(); + for e in s.drain() { + match e { + Event::ClipboardCopy { .. } => { + panic!("a store buffered while backgrounded must never fire on watch") + } + Event::Screen(_) => saw_screen = true, + _ => {} + } + } + } + assert!(saw_screen, "watching the task should stream its screen"); +} + +/// An over-cap store on the watched task yields the drop notice and no +/// `ClipboardCopy`: the copy is lost loudly, not truncated or forwarded. +#[test] +fn oversized_watched_store_yields_notice_and_no_copy() { + let dir = scratch("clip_oversize"); + let ready = dir.join("ready"); + let flag = dir.join("flag"); + let mut s = sup(24, 80); + // 3 MiB decoded exceeds the 1 MiB cap. The child generates the base64 + // itself because `MAX_COMMAND_LEN` cannot carry the payload inline; + // `tr` strips GNU base64's line wrapping (macOS emits none). + let cmd = format!( + "touch {r}; until [ -e {f} ]; do sleep 0.05; done; \ + printf '\\033]52;c;'; \ + dd if=/dev/zero bs=1024 count=3072 2>/dev/null | base64 | tr -d '\\n'; \ + printf '\\007'; sleep 30", + r = ready.display(), + f = flag.display() + ); + let id = spawn_ready(&mut s, cmd, here(), &ready); + s.apply(Command::Watch { id: Some(id) }); + std::fs::write(&flag, b"").unwrap(); + + let mut notice = None; + let ok = wait_until(Duration::from_secs(10), || { + s.tick(); + for e in s.drain() { + match e { + Event::ClipboardCopy { .. } => { + panic!("an over-cap store must be dropped, not forwarded") + } + Event::Status(msg) => notice = Some(msg), + _ => {} + } + } + notice.is_some() + }); + assert!(ok, "the oversized-store notice never arrived"); + assert_eq!( + notice.as_deref(), + Some("clipboard copy dropped: 3 MiB exceeds the 1 MiB limit") + ); + let _ = std::fs::remove_dir_all(&dir); +} + /// Scratch dir for tests that sync through marker files. fn scratch(tag: &str) -> PathBuf { crate::testutil::temp(&format!("sup_{tag}")) diff --git a/src/task.rs b/src/task.rs index 677f4df..fe02e5d 100644 --- a/src/task.rs +++ b/src/task.rs @@ -23,7 +23,7 @@ use rustix::process::{WaitId, WaitIdOptions, waitid}; use crate::{ core::{Wake, Waker}, - emulator::Emulator, + emulator::{ClipboardStores, Emulator}, input, preview::PreviewState, protocol::{Key, Lifecycle, Mods, MouseKind, Preview, ScrollAction, env_get}, @@ -485,6 +485,14 @@ impl Task { } } + /// Take the OSC 52 clipboard stores captured since the last drain (see + /// [`Emulator::drain_clipboard`]). The caller owns forwarding: a drain + /// whose result is dropped discards the stores, which is how a + /// non-watched task's copies are kept from ever firing later. + pub fn drain_clipboard(&self) -> ClipboardStores { + grid(&self.parser).drain_clipboard() + } + /// The dashboard preview, resolved through the provenance cascade under /// the grid lock (see [`crate::preview`]). `now` is the caller's tick /// instant so every task in one snapshot resolves against the same clock. diff --git a/src/terminal/emulator.rs b/src/terminal/emulator.rs index 61eb65d..213d254 100644 --- a/src/terminal/emulator.rs +++ b/src/terminal/emulator.rs @@ -39,7 +39,7 @@ pub enum MouseProtocolEncoding { /// bounds forwarding-frame size and per-task memory, not what a host /// clipboard could hold; an over-cap store is dropped and its length /// recorded so the caller can surface a notice. -const CLIPBOARD_STORE_MAX_BYTES: usize = 1024 * 1024; +pub(crate) const CLIPBOARD_STORE_MAX_BYTES: usize = 1024 * 1024; // A maximum-size store expands to this base64 bound when re-encoded for // forwarding. Reserve 64 KiB for the command envelope and keep the result @@ -230,10 +230,6 @@ pub struct Emulator { term: Term, parser: Processor, responses: Arc>>, - // Read only by `drain_clipboard`, which has no production caller until - // the supervisor's forwarding tick lands; the allow is scoped to the - // non-test build because tests do read it. - #[cfg_attr(not(test), allow(dead_code))] clipboard: Arc>, /// Alt-screen and title facts, advanced at parser-event granularity by /// [`ObservedTerm`] during the parse itself. @@ -264,10 +260,6 @@ impl Emulator { /// whose result is dropped discards the stores. An empty capture costs /// one lock and no allocation, so calling every tick for every task is /// fine. - // No production caller until the supervisor's forwarding tick lands; - // tests drain meanwhile, so the allow is scoped to the non-test build - // (`expect` would be unfulfilled in the test target). - #[cfg_attr(not(test), allow(dead_code))] pub fn drain_clipboard(&mut self) -> ClipboardStores { let mut buf = self .clipboard From 59e12508a270254adb67b3177a068ef6798cf01a Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 16:15:10 -0700 Subject: [PATCH 3/8] feat(client): re-emit forwarded OSC 52 stores to the host terminal --- src/app.rs | 59 +++++++++++++++++++++++--- src/app_tests.rs | 106 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 6 deletions(-) diff --git a/src/app.rs b/src/app.rs index c985c57..fff12ec 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2,7 +2,7 @@ //! here through `Command`s and `Event` snapshots over a `Transport`. use std::{ - io::{self, Stdout}, + io::{self, Stdout, Write}, path::{Path, PathBuf}, sync::{ Arc, @@ -13,6 +13,7 @@ use std::{ time::Duration, }; +use base64::{Engine as _, engine::general_purpose::STANDARD as B64}; use crossterm::{ cursor::MoveTo, event::{ @@ -27,8 +28,8 @@ use crate::{ editbuf::EditBuffer, path, protocol::{ - Command, Event, Key, Lifecycle, Mods, MouseBtn, MouseKind, RecoveryEntry, ScreenView, - ScrollAction, TaskView, + ClipboardKind, Command, Event, Key, Lifecycle, Mods, MouseBtn, MouseKind, RecoveryEntry, + ScreenView, ScrollAction, TaskView, }, transport::{ExitIntent, SocketTransport, ThreadTransport, Transport}, ui, @@ -194,6 +195,9 @@ pub struct App { pub recovery_sel: usize, /// Transient one-line notice (save/load result), dismissed on the next key. pub status: Option, + /// OSC 52 stores accepted while attached, awaiting re-emission to the host + /// terminal by `flush_clipboard` later in the same run-loop iteration. + pending_clipboard: Vec<(ClipboardKind, String)>, /// Parsed terminal events from the stdin reader thread. crossterm owns the /// tty, so a dedicated thread blocks on `event::read()` and forwards here; the /// run loop drains this instead of polling stdin itself. @@ -371,6 +375,7 @@ impl App { session_page: SessionPage::Saved, recovery_sel: 0, status: None, + pending_clipboard: Vec::new(), input_rx, input_tx: Some(input_tx), wait_rx, @@ -617,9 +622,7 @@ impl App { self.focused_screen = Some(s); } Event::Status(s) => self.status = Some(s), - // Dropped here until the client-side clipboard write lands - // in a later phase. - Event::ClipboardCopy { .. } => {} + Event::ClipboardCopy { kind, text } => self.on_clipboard_copy(kind, text), Event::Sessions { names, recovery } => { // Clamp both page selections to the refreshed lists. self.session_sel = self.session_sel.min(names.len().saturating_sub(1)); @@ -634,6 +637,39 @@ impl App { } } + /// Accept or drop one forwarded OSC 52 store. A clipboard write is an + /// outward-facing side effect; only an attached user plausibly caused it, + /// so every other mode drops the store silently — peek-mode stores and + /// copies still in flight when the user detaches both land here. + fn on_clipboard_copy(&mut self, kind: ClipboardKind, text: String) { + if self.mode == Mode::Attached { + self.pending_clipboard.push((kind, text)); + } + } + + /// Re-emit buffered clipboard stores to the host terminal, oldest first: + /// with multiple entries the host's clipboard ends at the last one, + /// matching last-writer-wins clipboard semantics. The payload is data, + /// the envelope is protocol: nothing from the stored text reaches `out` + /// except through base64, because raw payload bytes on the terminal are + /// an escape-sequence injection vector. + fn flush_clipboard(&mut self, out: &mut impl Write) -> io::Result<()> { + if self.pending_clipboard.is_empty() { + return Ok(()); + } + let mut last = 0; + for (_kind, text) in self.pending_clipboard.drain(..) { + // Selection also emits kind byte `c`: the host-terminal chain is + // verified working for `c` and unverified for `s`, and a selection + // copy that lands in the system clipboard beats one that vanishes. + write!(out, "\x1b]52;c;{}\x07", B64.encode(&text))?; + last = copied_chars(&text); + } + out.flush()?; + self.status = Some(format!("copied {last} chars")); + Ok(()) + } + pub fn run(&mut self, out: &mut Stdout) -> io::Result<()> { // Read terminal events on a dedicated thread and wake the UI loop. if let Some(input_tx) = self.input_tx.take() { @@ -683,6 +719,11 @@ impl App { self.focused_id = None; } + // After the `should_quit` break, so a final partial iteration's + // pending stores drop with the session instead of landing on a + // torn-down terminal; before `ui::render`, whose sequential turn + // keeps these bytes from interleaving with a frame. + self.flush_clipboard(out)?; self.sync_input_modes(out)?; ui::render(out, self)?; @@ -1495,6 +1536,12 @@ fn on_key_edit(buf: &mut EditBuffer, k: KeyEvent) -> Option { } } +/// Size of an emitted clipboard payload as the "copied {n} chars" notice +/// reports it: chars, not bytes, matching what the user sees when pasting. +fn copied_chars(text: &str) -> usize { + text.chars().count() +} + /// Insert pasted text at the caret after removing control characters. fn paste_into(buf: &mut EditBuffer, s: &str) { for c in s.chars().filter(|c| !c.is_control()) { diff --git a/src/app_tests.rs b/src/app_tests.rs index f239252..488f256 100644 --- a/src/app_tests.rs +++ b/src/app_tests.rs @@ -1882,3 +1882,109 @@ fn state_and_dir_mode_spawns_stay_unassigned() { let v = app.views.iter().find(|v| v.id == 2).unwrap(); assert_eq!(v.group, None); } + +// --- OSC 52 clipboard emission ------------------------------------------ + +/// An attached clipboard store re-emits as exactly one OSC 52 envelope: +/// kind byte `c`, padded standard base64, BEL-terminated. +#[test] +fn attached_clipboard_store_emits_the_osc52_envelope() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.on_clipboard_copy(ClipboardKind::Clipboard, "hello".to_string()); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + assert_eq!(out, b"\x1b]52;c;aGVsbG8=\x07"); + assert_eq!(app.status.as_deref(), Some("copied 5 chars")); +} + +/// A `Selection` store collapses to kind byte `c`: the host-terminal chain +/// is verified for `c` and unverified for `s`. +#[test] +fn selection_store_collapses_to_the_clipboard_kind() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.on_clipboard_copy(ClipboardKind::Selection, "hello".to_string()); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + assert_eq!(out, b"\x1b]52;c;aGVsbG8=\x07"); +} + +/// Nothing from the payload reaches the terminal raw: ESC/CSI sequences and +/// newlines cross only as base64 between the envelope prefix and the BEL. +#[test] +fn clipboard_payload_bytes_never_reach_the_terminal_raw() { + let payload = "line1\nline2\x1b[31mred\x1b]52;c;evil\x07"; + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.on_clipboard_copy(ClipboardKind::Clipboard, payload.to_string()); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + + assert!(out.starts_with(b"\x1b]52;c;")); + assert!(out.ends_with(b"\x07")); + let body = &out[b"\x1b]52;c;".len()..out.len() - 1]; + assert!( + body.iter() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'+' | b'/' | b'=')), + "only base64 may sit between the prefix and the BEL" + ); + assert_eq!(B64.decode(body).unwrap(), payload.as_bytes()); + assert!( + !out.windows(payload.len()).any(|w| w == payload.as_bytes()), + "the raw payload must not appear in the output" + ); +} + +/// Stores arriving outside attached mode buffer nothing and emit nothing: +/// only an attached user plausibly caused the copy. +#[test] +fn clipboard_stores_outside_attached_mode_are_dropped() { + for mode in [Mode::Peek, Mode::Dashboard] { + let mut app = App::new_local(30, 100); + app.mode = mode; + app.on_clipboard_copy(ClipboardKind::Clipboard, "hello".to_string()); + assert!(app.pending_clipboard.is_empty(), "nothing may buffer"); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + assert!(out.is_empty(), "nothing may emit"); + assert!(app.status.is_none(), "no notice without an emission"); + } +} + +/// Multiple pending stores emit in receipt order (the host clipboard ends +/// at the last: last-writer-wins), and the notice counts the last entry's +/// chars, not its bytes. +#[test] +fn pending_stores_emit_in_order_and_status_counts_last_entry_chars() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.on_clipboard_copy(ClipboardKind::Clipboard, "first".to_string()); + app.on_clipboard_copy(ClipboardKind::Clipboard, "héllo日".to_string()); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + + let expected = format!( + "\x1b]52;c;{}\x07\x1b]52;c;{}\x07", + B64.encode("first"), + B64.encode("héllo日") + ); + assert_eq!(out, expected.as_bytes()); + // "héllo日" is 6 chars but 9 bytes: the notice must report chars. + assert_eq!(app.status.as_deref(), Some("copied 6 chars")); + assert!( + app.pending_clipboard.is_empty(), + "the flush drains the buffer" + ); +} + +/// The common per-iteration case, an empty buffer, writes zero bytes. +#[test] +fn empty_clipboard_flush_writes_nothing() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + assert!(out.is_empty()); + assert!(app.status.is_none()); +} From 104de8b0f5bdd95fdc8ce43ecfee177f8f8180a2 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 16:26:31 -0700 Subject: [PATCH 4/8] feat(client): surface copy notices in the attached bar --- src/app.rs | 44 +++++++++++++++++++++++++++--- src/app_tests.rs | 57 +++++++++++++++++++++++++++++++++++---- src/ui.rs | 70 ++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 152 insertions(+), 19 deletions(-) diff --git a/src/app.rs b/src/app.rs index fff12ec..72dd7ab 100644 --- a/src/app.rs +++ b/src/app.rs @@ -10,7 +10,7 @@ use std::{ mpsc::{Receiver, Sender, channel}, }, thread, - time::Duration, + time::{Duration, Instant}, }; use base64::{Engine as _, engine::general_purpose::STANDARD as B64}; @@ -45,6 +45,11 @@ const _: () = assert!( "MAX_PASTE must base64-encode to under frame::MAX_FRAME" ); +/// How long an ephemeral notice stays visible. Expiry is lazy — `notice()` +/// answers `None` past this age — and the run loop's 100ms receive timeout +/// bounds how far past the deadline a stale one can stay painted. +const NOTICE_TTL: Duration = Duration::from_secs(5); + #[derive(Clone, Copy, PartialEq, Eq)] pub enum Mode { Dashboard, @@ -195,6 +200,11 @@ pub struct App { pub recovery_sel: usize, /// Transient one-line notice (save/load result), dismissed on the next key. pub status: Option, + /// Ephemeral notice and the instant it was set: clipboard-copy + /// confirmations, plus attached-mode mirrors of `Event::Status`. A + /// separate channel from `status` — that one persists until replaced or + /// cleared; this one dies of age through `notice()`. + notice: Option<(String, Instant)>, /// OSC 52 stores accepted while attached, awaiting re-emission to the host /// terminal by `flush_clipboard` later in the same run-loop iteration. pending_clipboard: Vec<(ClipboardKind, String)>, @@ -375,6 +385,7 @@ impl App { session_page: SessionPage::Saved, recovery_sel: 0, status: None, + notice: None, pending_clipboard: Vec::new(), input_rx, input_tx: Some(input_tx), @@ -621,7 +632,16 @@ impl App { } self.focused_screen = Some(s); } - Event::Status(s) => self.status = Some(s), + Event::Status(s) => { + // While attached, the dashboard row that displays `status` + // is off screen: mirror the message into the notice so + // supervisor warnings (e.g. the clipboard oversize drop) + // are seen when they happen. `status` is set as always. + if self.mode == Mode::Attached { + self.set_notice(s.clone()); + } + self.status = Some(s); + } Event::ClipboardCopy { kind, text } => self.on_clipboard_copy(kind, text), Event::Sessions { names, recovery } => { // Clamp both page selections to the refreshed lists. @@ -647,6 +667,21 @@ impl App { } } + /// Stage the ephemeral notice, replacing any predecessor: the newest + /// message wins, and replacement caps the state at one string. + fn set_notice(&mut self, msg: String) { + self.notice = Some((msg, Instant::now())); + } + + /// The staged notice while it is younger than `NOTICE_TTL`. Expiry is + /// lazy — nothing ever clears the field — because every render pass asks + /// here and the run loop renders at least every ~100ms, which bounds how + /// long an expired notice can stay painted. + pub fn notice(&self) -> Option<&str> { + let (msg, set_at) = self.notice.as_ref()?; + (set_at.elapsed() < NOTICE_TTL).then_some(msg.as_str()) + } + /// Re-emit buffered clipboard stores to the host terminal, oldest first: /// with multiple entries the host's clipboard ends at the last one, /// matching last-writer-wins clipboard semantics. The payload is data, @@ -666,7 +701,10 @@ impl App { last = copied_chars(&text); } out.flush()?; - self.status = Some(format!("copied {last} chars")); + // The status row is off screen while attached — exactly when copies + // happen — so the confirmation goes to the notice, which the attached + // bar renders. + self.set_notice(format!("copied {last} chars")); Ok(()) } diff --git a/src/app_tests.rs b/src/app_tests.rs index 488f256..5fd3ccc 100644 --- a/src/app_tests.rs +++ b/src/app_tests.rs @@ -1895,7 +1895,11 @@ fn attached_clipboard_store_emits_the_osc52_envelope() { let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); assert_eq!(out, b"\x1b]52;c;aGVsbG8=\x07"); - assert_eq!(app.status.as_deref(), Some("copied 5 chars")); + assert_eq!(app.notice(), Some("copied 5 chars")); + assert!( + app.status.is_none(), + "the copy confirmation is ephemeral; it must not occupy the status" + ); } /// A `Selection` store collapses to kind byte `c`: the host-terminal chain @@ -1948,7 +1952,7 @@ fn clipboard_stores_outside_attached_mode_are_dropped() { let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); assert!(out.is_empty(), "nothing may emit"); - assert!(app.status.is_none(), "no notice without an emission"); + assert!(app.notice().is_none(), "no notice without an emission"); } } @@ -1956,7 +1960,7 @@ fn clipboard_stores_outside_attached_mode_are_dropped() { /// at the last: last-writer-wins), and the notice counts the last entry's /// chars, not its bytes. #[test] -fn pending_stores_emit_in_order_and_status_counts_last_entry_chars() { +fn pending_stores_emit_in_order_and_notice_counts_last_entry_chars() { let mut app = App::new_local(30, 100); app.mode = Mode::Attached; app.on_clipboard_copy(ClipboardKind::Clipboard, "first".to_string()); @@ -1971,7 +1975,7 @@ fn pending_stores_emit_in_order_and_status_counts_last_entry_chars() { ); assert_eq!(out, expected.as_bytes()); // "héllo日" is 6 chars but 9 bytes: the notice must report chars. - assert_eq!(app.status.as_deref(), Some("copied 6 chars")); + assert_eq!(app.notice(), Some("copied 6 chars")); assert!( app.pending_clipboard.is_empty(), "the flush drains the buffer" @@ -1986,5 +1990,48 @@ fn empty_clipboard_flush_writes_nothing() { let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); assert!(out.is_empty()); - assert!(app.status.is_none()); + assert!(app.notice().is_none()); +} + +/// The notice dies of age: the accessor answers `None` once `NOTICE_TTL` has +/// passed. No clearing pass exists — expiry is the accessor's answer. +#[test] +fn notice_expires_lazily_after_the_ttl() { + let mut app = App::new_local(30, 100); + app.set_notice("copied 5 chars".to_string()); + assert_eq!(app.notice(), Some("copied 5 chars")); + + let past = Instant::now() + .checked_sub(NOTICE_TTL) + .expect("system uptime exceeds NOTICE_TTL"); + app.notice = Some(("copied 5 chars".to_string(), past)); + assert_eq!(app.notice(), None, "an aged-out notice must not render"); +} + +/// A status event arriving while attached mirrors into the notice — the +/// dashboard row that displays `status` is off screen there — and still sets +/// the persistent status verbatim. +#[test] +fn attached_status_event_mirrors_into_the_notice() { + let dir = session_scratch("status_mirror", &[]); + let mut app = app_with_config_dir(&dir); + app.mode = Mode::Attached; + app.save_session("mirror"); + app.pump(); + assert_eq!(app.status.as_deref(), Some("saved 'mirror': 0 command(s)")); + assert_eq!(app.notice(), Some("saved 'mirror': 0 command(s)")); + let _ = std::fs::remove_dir_all(&dir); +} + +/// A status event on the dashboard stays out of the notice: the command row +/// already displays `status` there, and the attached bar is off screen. +#[test] +fn dashboard_status_event_sets_only_the_status() { + let dir = session_scratch("status_dash", &[]); + let mut app = app_with_config_dir(&dir); + app.save_session("dash"); + app.pump(); + assert_eq!(app.status.as_deref(), Some("saved 'dash': 0 command(s)")); + assert!(app.notice().is_none(), "no mirror outside attached mode"); + let _ = std::fs::remove_dir_all(&dir); } diff --git a/src/ui.rs b/src/ui.rs index 2b72b0e..e2de4fd 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -181,13 +181,13 @@ fn render_dashboard(out: &mut impl Write, app: &App) -> io::Result<()> { y += 1; } - // Command line: input modes show a prompt (with cursor); otherwise a - // transient save/load notice, else the key hint. + // Command line: input modes show a prompt (with cursor); otherwise the + // ephemeral notice or persistent status, else the key hint. let cmd_y = rows.saturating_sub(2); match cmdline(app) { Some((line, _)) => put(out, cmd_y, &line, cols)?, - None => match &app.status { - Some(s) => put(out, cmd_y, &format!(" {s}"), cols)?, + None => match transient_line(app.notice(), app.status.as_deref()) { + Some(line) => put(out, cmd_y, &line, cols)?, None => dim( out, cmd_y, @@ -225,6 +225,12 @@ fn render_dashboard(out: &mut impl Write, app: &App) -> io::Result<()> { Ok(()) } +/// The command row's transient text: an active ephemeral notice beats the +/// persistent status, so a copy landing just before a detach is still seen. +fn transient_line(notice: Option<&str>, status: Option<&str>) -> Option { + notice.or(status).map(|s| format!(" {s}")) +} + /// The editable bottom line for the text-input modes (the rendered line and /// the caret's display column), or `None` when the command line should show a /// hint/status instead. @@ -704,14 +710,8 @@ fn render_attached(out: &mut impl Write, app: &App) -> io::Result<()> { } let cols = app.cols as usize; - // Display the scrollback offset when viewing history. let title = attached_title(v); - let bar = match screen.map_or(0, |s| s.scrollback) { - 0 => format!(" [attached] {title} Ctrl-\\ background"), - n => { - format!(" [scroll ↑{n}] {title} Esc live · PgUp/PgDn move · Ctrl-\\ background") - } - }; + let bar = attached_bar(&title, screen.map_or(0, |s| s.scrollback), app.notice()); rev(out, app.rows.saturating_sub(1), &bar, cols)?; // Place the real cursor where the child's is, so typing feels native. @@ -722,6 +722,21 @@ fn render_attached(out: &mut impl Write, app: &App) -> io::Result<()> { Ok(()) } +/// The attached bottom bar, titled by the live/scrollback state. In the live +/// view an active notice replaces the background hint; the bar is rebuilt +/// every frame, so the hint returns the moment the notice expires. The +/// scrollback bar never yields its hints: those keys are how the user gets +/// back out. +fn attached_bar(title: &str, scrollback: usize, notice: Option<&str>) -> String { + match (scrollback, notice) { + (0, Some(n)) => format!(" [attached] {title} {n}"), + (0, None) => format!(" [attached] {title} Ctrl-\\ background"), + (n, _) => { + format!(" [scroll ↑{n}] {title} Esc live · PgUp/PgDn move · Ctrl-\\ background") + } + } +} + /// Visible `(start, count)` window that includes the selected list item. pub fn scroll_window(sel: usize, total: usize, max: usize) -> (usize, usize) { if total == 0 || max == 0 { @@ -972,6 +987,39 @@ mod tests { ); } + /// The live bar swaps its background hint for an active notice; the + /// scrollback bar keeps its navigation hints regardless. + #[test] + fn attached_bar_swaps_the_hint_for_an_active_notice() { + assert_eq!( + attached_bar("cargo test", 0, None), + " [attached] cargo test Ctrl-\\ background" + ); + assert_eq!( + attached_bar("cargo test", 0, Some("copied 5 chars")), + " [attached] cargo test copied 5 chars" + ); + assert_eq!( + attached_bar("cargo test", 3, Some("copied 5 chars")), + " [scroll ↑3] cargo test Esc live · PgUp/PgDn move · Ctrl-\\ background" + ); + } + + /// The dashboard command row prefers an active notice over the + /// persistent status. + #[test] + fn transient_line_prefers_the_notice_over_the_status() { + assert_eq!( + transient_line(Some("copied 5 chars"), Some("saved 'x': 1 command(s)")), + Some(" copied 5 chars".to_string()) + ); + assert_eq!( + transient_line(None, Some("saved 'x': 1 command(s)")), + Some(" saved 'x': 1 command(s)".to_string()) + ); + assert_eq!(transient_line(None, None), None); + } + /// The strip's plain text is fixed; exactly the active mode's label is /// bold, every other strip run (labels and separators) is dim, and the /// prefix/suffix keep the header's bold. From 9bb75f38171b1506290f51f506c9235783c4208f Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 17:41:22 -0700 Subject: [PATCH 5/8] fix(clipboard): close the stale-forward race, emit kinds verbatim, tier the notice --- src/app.rs | 78 +++++++++++++++++++-------- src/app_tests.rs | 116 ++++++++++++++++++++++++++++++++++------ src/protocol.rs | 47 ++++++++++------ src/supervisor.rs | 22 ++++++-- src/supervisor_tests.rs | 50 +++++++++++++++-- 5 files changed, 254 insertions(+), 59 deletions(-) diff --git a/src/app.rs b/src/app.rs index 72dd7ab..91ce22a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -50,6 +50,17 @@ const _: () = assert!( /// bounds how far past the deadline a stale one can stay painted. const NOTICE_TTL: Duration = Duration::from_secs(5); +/// Notice severity, two tiers only — the notice is decoration, never +/// load-bearing. `Warning` covers attached-mode `Event::Status` mirrors +/// (spawn errors, recovery failures, clipboard drops); `Info` covers +/// confirmations (the copied-chars line). The sole ranking rule lives in +/// `set_notice`: `Info` never replaces an unexpired `Warning`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NoticeLevel { + Warning, + Info, +} + #[derive(Clone, Copy, PartialEq, Eq)] pub enum Mode { Dashboard, @@ -200,11 +211,11 @@ pub struct App { pub recovery_sel: usize, /// Transient one-line notice (save/load result), dismissed on the next key. pub status: Option, - /// Ephemeral notice and the instant it was set: clipboard-copy - /// confirmations, plus attached-mode mirrors of `Event::Status`. A - /// separate channel from `status` — that one persists until replaced or - /// cleared; this one dies of age through `notice()`. - notice: Option<(String, Instant)>, + /// Ephemeral notice, its severity, and the instant it was set: + /// clipboard-copy confirmations, plus attached-mode mirrors of + /// `Event::Status`. A separate channel from `status` — that one persists + /// until replaced or cleared; this one dies of age through `notice()`. + notice: Option<(String, NoticeLevel, Instant)>, /// OSC 52 stores accepted while attached, awaiting re-emission to the host /// terminal by `flush_clipboard` later in the same run-loop iteration. pending_clipboard: Vec<(ClipboardKind, String)>, @@ -637,12 +648,15 @@ impl App { // is off screen: mirror the message into the notice so // supervisor warnings (e.g. the clipboard oversize drop) // are seen when they happen. `status` is set as always. + // `Warning` because everything the supervisor says while + // attached is operational: spawn errors, recovery + // failures, clipboard drops. if self.mode == Mode::Attached { - self.set_notice(s.clone()); + self.set_notice(s.clone(), NoticeLevel::Warning); } self.status = Some(s); } - Event::ClipboardCopy { kind, text } => self.on_clipboard_copy(kind, text), + Event::ClipboardCopy { id, kind, text } => self.on_clipboard_copy(id, kind, text), Event::Sessions { names, recovery } => { // Clamp both page selections to the refreshed lists. self.session_sel = self.session_sel.min(names.len().saturating_sub(1)); @@ -660,17 +674,29 @@ impl App { /// Accept or drop one forwarded OSC 52 store. A clipboard write is an /// outward-facing side effect; only an attached user plausibly caused it, /// so every other mode drops the store silently — peek-mode stores and - /// copies still in flight when the user detaches both land here. - fn on_clipboard_copy(&mut self, kind: ClipboardKind, text: String) { - if self.mode == Mode::Attached { + /// copies still in flight when the user detaches both land here. The id + /// gate drops in-flight copies from a previously watched task: the wire + /// preserves ordering per direction, not across a Watch/forward cross, + /// so a copy from task A can arrive after attachment moved to task B. + fn on_clipboard_copy(&mut self, id: u64, kind: ClipboardKind, text: String) { + if self.mode == Mode::Attached && self.focused_id == Some(id) { self.pending_clipboard.push((kind, text)); } } - /// Stage the ephemeral notice, replacing any predecessor: the newest - /// message wins, and replacement caps the state at one string. - fn set_notice(&mut self, msg: String) { - self.notice = Some((msg, Instant::now())); + /// Stage the ephemeral notice. The newest message wins with one + /// exception: an `Info` set yields to an unexpired `Warning`, because the + /// attached bar is the only place a warning shows and a copy + /// confirmation landing in the same batch would clobber it. An expired + /// `Warning` loses — staleness must not pin warnings forever. + fn set_notice(&mut self, msg: String, level: NoticeLevel) { + if level == NoticeLevel::Info + && let Some((_, NoticeLevel::Warning, set_at)) = self.notice.as_ref() + && set_at.elapsed() < NOTICE_TTL + { + return; + } + self.notice = Some((msg, level, Instant::now())); } /// The staged notice while it is younger than `NOTICE_TTL`. Expiry is @@ -678,7 +704,7 @@ impl App { /// here and the run loop renders at least every ~100ms, which bounds how /// long an expired notice can stay painted. pub fn notice(&self) -> Option<&str> { - let (msg, set_at) = self.notice.as_ref()?; + let (msg, _, set_at) = self.notice.as_ref()?; (set_at.elapsed() < NOTICE_TTL).then_some(msg.as_str()) } @@ -693,18 +719,26 @@ impl App { return Ok(()); } let mut last = 0; - for (_kind, text) in self.pending_clipboard.drain(..) { - // Selection also emits kind byte `c`: the host-terminal chain is - // verified working for `c` and unverified for `s`, and a selection - // copy that lands in the system clipboard beats one that vanishes. - write!(out, "\x1b]52;c;{}\x07", B64.encode(&text))?; + for (kind, text) in self.pending_clipboard.drain(..) { + // Emit the kind verbatim. Collapsing `s` to `c` (the original + // design) creates a wrong-content collision: one batch can hold + // one store per kind, and the later-emitted selection payload + // would overwrite the clipboard payload. A host without `s` + // support ignores the sequence — unsupported means inert, not + // aimed at a different clipboard. + let k = match kind { + ClipboardKind::Clipboard => 'c', + ClipboardKind::Selection => 's', + }; + write!(out, "\x1b]52;{k};{}\x07", B64.encode(&text))?; last = copied_chars(&text); } out.flush()?; // The status row is off screen while attached — exactly when copies // happen — so the confirmation goes to the notice, which the attached - // bar renders. - self.set_notice(format!("copied {last} chars")); + // bar renders. `Info`: a confirmation must not clobber an unexpired + // warning (the oversize-drop mirror lands in the same iteration). + self.set_notice(format!("copied {last} chars"), NoticeLevel::Info); Ok(()) } diff --git a/src/app_tests.rs b/src/app_tests.rs index 5fd3ccc..8ef2b78 100644 --- a/src/app_tests.rs +++ b/src/app_tests.rs @@ -1891,7 +1891,8 @@ fn state_and_dir_mode_spawns_stay_unassigned() { fn attached_clipboard_store_emits_the_osc52_envelope() { let mut app = App::new_local(30, 100); app.mode = Mode::Attached; - app.on_clipboard_copy(ClipboardKind::Clipboard, "hello".to_string()); + app.focused_id = Some(1); + app.on_clipboard_copy(1, ClipboardKind::Clipboard, "hello".to_string()); let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); assert_eq!(out, b"\x1b]52;c;aGVsbG8=\x07"); @@ -1902,16 +1903,18 @@ fn attached_clipboard_store_emits_the_osc52_envelope() { ); } -/// A `Selection` store collapses to kind byte `c`: the host-terminal chain -/// is verified for `c` and unverified for `s`. +/// A `Selection` store emits its own kind byte `s`, never collapsed to `c`: +/// collapsing would let a same-batch selection payload overwrite the +/// clipboard payload. A host without `s` support ignores the sequence. #[test] -fn selection_store_collapses_to_the_clipboard_kind() { +fn selection_store_emits_its_own_kind_byte() { let mut app = App::new_local(30, 100); app.mode = Mode::Attached; - app.on_clipboard_copy(ClipboardKind::Selection, "hello".to_string()); + app.focused_id = Some(1); + app.on_clipboard_copy(1, ClipboardKind::Selection, "hello".to_string()); let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); - assert_eq!(out, b"\x1b]52;c;aGVsbG8=\x07"); + assert_eq!(out, b"\x1b]52;s;aGVsbG8=\x07"); } /// Nothing from the payload reaches the terminal raw: ESC/CSI sequences and @@ -1921,7 +1924,8 @@ fn clipboard_payload_bytes_never_reach_the_terminal_raw() { let payload = "line1\nline2\x1b[31mred\x1b]52;c;evil\x07"; let mut app = App::new_local(30, 100); app.mode = Mode::Attached; - app.on_clipboard_copy(ClipboardKind::Clipboard, payload.to_string()); + app.focused_id = Some(1); + app.on_clipboard_copy(1, ClipboardKind::Clipboard, payload.to_string()); let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); @@ -1941,13 +1945,15 @@ fn clipboard_payload_bytes_never_reach_the_terminal_raw() { } /// Stores arriving outside attached mode buffer nothing and emit nothing: -/// only an attached user plausibly caused the copy. +/// only an attached user plausibly caused the copy. The id matches +/// `focused_id` so the mode gate alone is what drops the store. #[test] fn clipboard_stores_outside_attached_mode_are_dropped() { for mode in [Mode::Peek, Mode::Dashboard] { let mut app = App::new_local(30, 100); app.mode = mode; - app.on_clipboard_copy(ClipboardKind::Clipboard, "hello".to_string()); + app.focused_id = Some(1); + app.on_clipboard_copy(1, ClipboardKind::Clipboard, "hello".to_string()); assert!(app.pending_clipboard.is_empty(), "nothing may buffer"); let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); @@ -1956,20 +1962,42 @@ fn clipboard_stores_outside_attached_mode_are_dropped() { } } +/// A store whose id is not the attached task's drops at receipt: the wire +/// preserves ordering per direction, not across a Watch/forward cross, so a +/// copy from the previously watched task can arrive after attachment moved. +/// The matching id buffers as before. +#[test] +fn mismatched_id_clipboard_store_drops_at_receipt() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.focused_id = Some(7); + app.on_clipboard_copy(3, ClipboardKind::Clipboard, "stale".to_string()); + assert!( + app.pending_clipboard.is_empty(), + "an in-flight copy from another task must not buffer" + ); + app.on_clipboard_copy(7, ClipboardKind::Clipboard, "fresh".to_string()); + assert_eq!( + app.pending_clipboard, + vec![(ClipboardKind::Clipboard, "fresh".to_string())] + ); +} + /// Multiple pending stores emit in receipt order (the host clipboard ends -/// at the last: last-writer-wins), and the notice counts the last entry's -/// chars, not its bytes. +/// at the last: last-writer-wins), each under its own kind byte, and the +/// notice counts the last entry's chars, not its bytes. #[test] fn pending_stores_emit_in_order_and_notice_counts_last_entry_chars() { let mut app = App::new_local(30, 100); app.mode = Mode::Attached; - app.on_clipboard_copy(ClipboardKind::Clipboard, "first".to_string()); - app.on_clipboard_copy(ClipboardKind::Clipboard, "héllo日".to_string()); + app.focused_id = Some(1); + app.on_clipboard_copy(1, ClipboardKind::Clipboard, "first".to_string()); + app.on_clipboard_copy(1, ClipboardKind::Selection, "héllo日".to_string()); let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); let expected = format!( - "\x1b]52;c;{}\x07\x1b]52;c;{}\x07", + "\x1b]52;c;{}\x07\x1b]52;s;{}\x07", B64.encode("first"), B64.encode("héllo日") ); @@ -1998,16 +2026,72 @@ fn empty_clipboard_flush_writes_nothing() { #[test] fn notice_expires_lazily_after_the_ttl() { let mut app = App::new_local(30, 100); - app.set_notice("copied 5 chars".to_string()); + app.set_notice("copied 5 chars".to_string(), NoticeLevel::Info); assert_eq!(app.notice(), Some("copied 5 chars")); let past = Instant::now() .checked_sub(NOTICE_TTL) .expect("system uptime exceeds NOTICE_TTL"); - app.notice = Some(("copied 5 chars".to_string(), past)); + app.notice = Some(("copied 5 chars".to_string(), NoticeLevel::Info, past)); assert_eq!(app.notice(), None, "an aged-out notice must not render"); } +/// A live `Warning` survives an `Info` set: the copy confirmation emitted by +/// `flush_clipboard` must not clobber the oversize-drop mirror that landed +/// in the same iteration — the attached bar is the only place that warning +/// shows. The copy itself still emits; only the notice yields. +#[test] +fn warning_notice_survives_the_copy_confirmation() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.focused_id = Some(1); + app.set_notice("clipboard copy dropped".to_string(), NoticeLevel::Warning); + app.on_clipboard_copy(1, ClipboardKind::Clipboard, "hello".to_string()); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + assert_eq!(out, b"\x1b]52;c;aGVsbG8=\x07", "the copy must still emit"); + assert_eq!(app.notice(), Some("clipboard copy dropped")); +} + +/// `Info` replaces `Info`: a second copy updates the count. +#[test] +fn info_notice_replaces_info() { + let mut app = App::new_local(30, 100); + app.set_notice("copied 5 chars".to_string(), NoticeLevel::Info); + app.set_notice("copied 2 chars".to_string(), NoticeLevel::Info); + assert_eq!(app.notice(), Some("copied 2 chars")); +} + +/// `Warning` replaces everything, `Info` included: a fresh operational +/// message always shows. +#[test] +fn warning_notice_replaces_info() { + let mut app = App::new_local(30, 100); + app.set_notice("copied 5 chars".to_string(), NoticeLevel::Info); + app.set_notice("spawn failed".to_string(), NoticeLevel::Warning); + assert_eq!(app.notice(), Some("spawn failed")); + + app.set_notice("recovery failed".to_string(), NoticeLevel::Warning); + assert_eq!( + app.notice(), + Some("recovery failed"), + "warning over warning" + ); +} + +/// An expired `Warning` loses to `Info`: staleness must not pin warnings +/// forever — the yield rule reads the same TTL clock as `notice()`. +#[test] +fn expired_warning_yields_to_info() { + let mut app = App::new_local(30, 100); + let past = Instant::now() + .checked_sub(NOTICE_TTL) + .expect("system uptime exceeds NOTICE_TTL"); + app.notice = Some(("old warning".to_string(), NoticeLevel::Warning, past)); + app.set_notice("copied 5 chars".to_string(), NoticeLevel::Info); + assert_eq!(app.notice(), Some("copied 5 chars")); +} + /// A status event arriving while attached mirrors into the notice — the /// dashboard row that displays `status` is off screen there — and still sets /// the persistent status verbatim. diff --git a/src/protocol.rs b/src/protocol.rs index f8b559a..35a8619 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -198,8 +198,15 @@ pub enum Event { }, /// One OSC 52 clipboard store from the watched task, already base64-decoded /// by the terminal backend. Store-only: no load or query path exists, so - /// this event never solicits a reply. - ClipboardCopy { kind: ClipboardKind, text: String }, + /// this event never solicits a reply. `id` names the source task so the + /// client can drop a copy still in flight when it switches attachment: + /// the wire preserves ordering per direction, not across a Watch/forward + /// cross. + ClipboardCopy { + id: u64, + kind: ClipboardKind, + text: String, + }, } /// Which clipboard an OSC 52 store targets. A wire-layer mirror of the @@ -903,9 +910,10 @@ pub fn encode_event(ev: &Event) -> (u8, Vec) { // output and must cross the wire unsanitized. The emulator's 1 MiB // store cap keeps the encoded frame far under `frame::MAX_FRAME` // (see `CLIPBOARD_STORE_MAX_BYTES`'s const assertion). - Event::ClipboardCopy { kind, text } => { + Event::ClipboardCopy { id, kind, text } => { let mut o = jzon::JsonValue::new_object(); let _ = o.insert("t", "clip"); + let _ = o.insert("id", *id); let _ = o.insert( "k", match kind { @@ -1001,6 +1009,7 @@ pub fn decode_event(kind: u8, payload: &[u8]) -> Option { recovery: recovery_vec(&v["recovery"]), }), "clip" => { + let id = v["id"].as_u64()?; let kind = match v["k"].as_str()? { "c" => ClipboardKind::Clipboard, "s" => ClipboardKind::Selection, @@ -1009,7 +1018,7 @@ pub fn decode_event(kind: u8, payload: &[u8]) -> Option { // The store was UTF-8 when the emulator captured it; a // frame that decodes to anything else is malformed. let text = String::from_utf8(B64.decode(v["text"].as_str()?).ok()?).ok()?; - Some(Event::ClipboardCopy { kind, text }) + Some(Event::ClipboardCopy { id, kind, text }) } _ => None, } @@ -1767,19 +1776,23 @@ mod tests { /// `ClipboardCopy` round-trips both kinds and carries clipboard text /// unsanitized: unicode, control characters, and embedded markers survive - /// the JSON+base64 path byte-for-byte. + /// the JSON+base64 path byte-for-byte. The source id survives too, + /// including values past `u32`. #[test] fn clipboard_copy_round_trips() { for ev in [ Event::ClipboardCopy { + id: 1, kind: ClipboardKind::Clipboard, text: "hello".into(), }, Event::ClipboardCopy { + id: 1 << 40, kind: ClipboardKind::Selection, text: "sélection λ 🦀".into(), }, Event::ClipboardCopy { + id: 7, kind: ClipboardKind::Clipboard, // Clipboard content is arbitrary: newlines, tabs, NUL, ESC, // and paste-marker-shaped text must not be sanitized in @@ -1793,32 +1806,36 @@ mod tests { } } - /// `ClipboardCopy` pins its wire shape: `t` discriminator, the OSC 52 - /// kind byte under `k`, and base64 text. + /// `ClipboardCopy` pins its wire shape: `t` discriminator, the source + /// task under `id`, the OSC 52 kind byte under `k`, and base64 text. #[test] fn clipboard_copy_wire_form() { let (k, p) = encode_event(&Event::ClipboardCopy { + id: 5, kind: ClipboardKind::Selection, text: "hi".into(), }); assert_eq!(k, KIND_CONTROL); assert_eq!( std::str::from_utf8(&p).unwrap(), - r#"{"t":"clip","k":"s","text":"aGk="}"# + r#"{"t":"clip","id":5,"k":"s","text":"aGk="}"# ); } /// A malformed `ClipboardCopy` frame is rejected whole: missing fields, - /// an unknown kind string, invalid base64, and non-UTF-8 decoded bytes. + /// an unknown kind string, a non-numeric id, invalid base64, and + /// non-UTF-8 decoded bytes. #[test] fn malformed_clipboard_copy_is_rejected() { for json in [ - r#"{"t":"clip","text":"aGk="}"#, // missing kind - r#"{"t":"clip","k":"c"}"#, // missing text - r#"{"t":"clip","k":"p","text":"aGk="}"#, // unknown kind string - r#"{"t":"clip","k":"c","text":"!!!"}"#, // invalid base64 - r#"{"t":"clip","k":"c","text":"/w=="}"#, // 0xFF: not UTF-8 - r#"{"t":"clip","k":"c","text":["aGk="]}"#, // text must be a string + r#"{"t":"clip","id":1,"text":"aGk="}"#, // missing kind + r#"{"t":"clip","id":1,"k":"c"}"#, // missing text + r#"{"t":"clip","k":"c","text":"aGk="}"#, // missing id + r#"{"t":"clip","id":"1","k":"c","text":"aGk="}"#, // id must be a number + r#"{"t":"clip","id":1,"k":"p","text":"aGk="}"#, // unknown kind string + r#"{"t":"clip","id":1,"k":"c","text":"!!!"}"#, // invalid base64 + r#"{"t":"clip","id":1,"k":"c","text":"/w=="}"#, // 0xFF: not UTF-8 + r#"{"t":"clip","id":1,"k":"c","text":["aGk="]}"#, // text must be a string ] { assert_eq!( decode_event(KIND_CONTROL, json.as_bytes()), diff --git a/src/supervisor.rs b/src/supervisor.rs index e921a6f..b06d44d 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -408,6 +408,18 @@ impl Supervisor { { t.scroll_view(ScrollAction::Live); } + // Purge the new target's buffered stores before the watch + // takes effect. The wake loop applies a whole burst before + // ticking, so without this a store captured while + // backgrounded survives into a tick that already sees the + // task as watched — and fires. The residual window (bytes + // emitted pre-attach but parsed post-purge) is + // irreducible: a transparent terminal has it too. + if let Some(new) = id + && let Some(t) = self.by_id_mut(new) + { + let _ = t.drain_clipboard(); + } self.last_screen = None; } self.watched = id; @@ -544,10 +556,13 @@ impl Supervisor { // guarantee: a store captured while backgrounded must never // fire when the task is later watched — a wrong clipboard is // silently harmful, an empty one visibly inert. The two-slot - // capture bound makes the constant drain cheap. + // capture bound makes the constant drain cheap. This drain + // alone cannot close the wake-coalescing race (a `Watch` in + // the same burst lands before the tick); `apply`'s `Watch` + // arm purges the new target for that case. let stores = t.drain_clipboard(); if watched == Some(t.id) { - clipboard = Some(stores); + clipboard = Some((t.id, stores)); } TaskView { id: t.id, @@ -567,9 +582,10 @@ impl Supervisor { .collect(); self.events.push(Event::Tasks(views)); - if let Some(stores) = clipboard { + if let Some((id, stores)) = clipboard { for (kind, text) in stores.stores { self.events.push(Event::ClipboardCopy { + id, kind: clipboard_kind(kind), text, }); diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index d298a02..5c3417e 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -237,7 +237,7 @@ fn watched_task_clipboard_stores_are_forwarded() { let ok = wait_until(Duration::from_secs(5), || { s.tick(); copies.extend(s.drain().into_iter().filter_map(|e| match e { - Event::ClipboardCopy { kind, text } => Some((kind, text)), + Event::ClipboardCopy { id, kind, text } => Some((id, kind, text)), _ => None, })); copies.len() >= 2 @@ -246,13 +246,57 @@ fn watched_task_clipboard_stores_are_forwarded() { assert_eq!( copies, vec![ - (ClipboardKind::Clipboard, "hello".to_string()), - (ClipboardKind::Selection, "world".to_string()), + (id, ClipboardKind::Clipboard, "hello".to_string()), + (id, ClipboardKind::Selection, "world".to_string()), ] ); let _ = std::fs::remove_dir_all(&dir); } +/// A store captured before its task is watched must not fire once `Watch` +/// lands. The per-tick drain cannot cover this: the wake loop applies a +/// whole command burst before ticking, so a `Watch` in the burst makes the +/// next drain see the task as already watched. The test reproduces that +/// interleaving exactly — capture, then `Watch`, with no tick between — +/// which only `apply`'s watch-time purge can close. The marker is observed +/// through a non-draining grid read; a tick here would drain (and discard) +/// the buffer and mask the race. +#[test] +fn watch_purges_stores_captured_before_the_watch() { + let mut s = sup(24, 80); + // "c3RhbGU=" is "stale". The trailing marker proves the store's bytes + // were parsed: it follows them in the output stream. + spawn( + &mut s, + "printf '\\033]52;c;c3RhbGU=\\007MARKER'; sleep 30", + here(), + ); + let parsed = wait_until(Duration::from_secs(5), || { + s.tasks.first().is_some_and(|t| { + let (formatted, _, _) = t.formatted(); + String::from_utf8_lossy(&formatted).contains("MARKER") + }) + }); + assert!(parsed, "the marker never reached the grid"); + + let id = s.tasks[0].id; + s.apply(Command::Watch { id: Some(id) }); + let mut saw_screen = false; + for _ in 0..3 { + s.tick(); + for e in s.drain() { + match e { + Event::ClipboardCopy { .. } => { + panic!("a store captured before the watch must not fire after it") + } + Event::Screen(_) => saw_screen = true, + _ => {} + } + } + } + assert!(saw_screen, "watching the task should stream its screen"); +} + /// A store captured while the task is not watched is discarded by the /// per-tick drain, never deferred: watching the task afterwards forwards /// nothing. Staleness is worse than loss — a wrong clipboard is silently From cad80b38ae88699badf6d41ff6266cc2de70c5c6 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 18:21:26 -0700 Subject: [PATCH 6/8] fix(clipboard): gate OSC 52 forwarding on attach, not watch --- src/app.rs | 26 +++++-- src/app_tests.rs | 46 ++++++++++- src/core.rs | 5 +- src/protocol.rs | 67 ++++++++++++++-- src/supervisor.rs | 65 ++++++++++------ src/supervisor_tests.rs | 164 +++++++++++++++++++++++++++++++++++++--- src/transport.rs | 5 +- tests/common/mod.rs | 2 +- 8 files changed, 330 insertions(+), 50 deletions(-) diff --git a/src/app.rs b/src/app.rs index 91ce22a..e970b0d 100644 --- a/src/app.rs +++ b/src/app.rs @@ -157,8 +157,11 @@ pub struct App { pub views: Vec, /// The watched task's screen (attach/peek), from `Event::Screen`. focused_screen: Option, - /// Last `Watch` target sent to the core, so we don't resend it every tick. - watched: Option, + /// Last `Watch` (target, attached) pair sent to the core, so we don't + /// resend it every tick. The pair, not the id: peek→attach on the same + /// task must send a fresh `Watch`, because the core's watch-time purge + /// and its clipboard-forwarding gate both key on the attach flag. + watched: Option<(u64, bool)>, /// Whether this client talks to a daemon (vs. an in-process `--foreground` /// core). Only a daemon client can meaningfully reconnect after a drop. pub daemon_backed: bool, @@ -613,9 +616,13 @@ impl App { self.selected_id = Some(self.views[sections[prev].1[0]].id); } - /// Tell the core which task's screen we need (attach/peek), sending `Watch` - /// only when the target actually changes. - fn set_watch(&mut self, want: Option) { + /// Tell the core which task's screen we need and whether we are attached + /// (`(id, attached)`), sending `Watch` only when that pair changes. + /// Deduplicating on the pair is load-bearing: an id-only dedup would + /// swallow the peek→attach transition on the same task, and the core's + /// watch-time clipboard purge would never run for it. Cost: one extra + /// command frame per attach. + fn set_watch(&mut self, want: Option<(u64, bool)>) { if want != self.watched { self.watched = want; // Drop the now-irrelevant screen so a stale one can't flash before @@ -623,7 +630,10 @@ impl App { if want.is_none() { self.focused_screen = None; } - self.transport.send(Command::Watch { id: want }); + self.transport.send(Command::Watch { + id: want.map(|(id, _)| id), + attached: want.is_some_and(|(_, attached)| attached), + }); } } @@ -761,8 +771,8 @@ impl App { // Synchronize before checking for exit so teardown still runs if the // terminal has gone away. let watch = match self.mode { - Mode::Peek => self.selected_id, - Mode::Attached => self.focused_id, + Mode::Peek => self.selected_id.map(|id| (id, false)), + Mode::Attached => self.focused_id.map(|id| (id, true)), _ => None, }; self.set_watch(watch); diff --git a/src/app_tests.rs b/src/app_tests.rs index 8ef2b78..c734634 100644 --- a/src/app_tests.rs +++ b/src/app_tests.rs @@ -1242,7 +1242,7 @@ fn attached_wheel_honors_the_childs_1007_veto() { app.resolve_selection(); app.attach(); let id = app.focused_id.expect("attached"); - app.set_watch(Some(id)); + app.set_watch(Some((id, true))); // Wait for the child's terminal modes to reach the client. assert!( wait_until(Duration::from_secs(5), || { @@ -1983,6 +1983,50 @@ fn mismatched_id_clipboard_store_drops_at_receipt() { ); } +/// `set_watch` deduplicates on the (id, attached) pair, not the id: the +/// peek→attach transition on the same task must send a fresh `Watch`, or the +/// core keeps treating the watch as a peek and never forwards. There is no +/// command-observation seam on the in-process transport (it applies commands +/// straight to the supervisor), so the resend is asserted through core +/// behavior: a store emitted after the transition forwards, which cannot +/// happen unless the attach-kind `Watch` actually left the client. +#[test] +fn set_watch_resends_on_kind_change_with_the_same_id() { + let dir = temp("app_watch_kind"); + let flag = dir.join("flag"); + let mut app = App::new_local(30, 100); + let cwd = app.invocation_dir.clone(); + // "cG9zdA==" is "post". + let cmd = format!( + "until [ -e {f} ]; do sleep 0.05; done; printf '\\033]52;c;cG9zdA==\\007'; sleep 30", + f = flag.display() + ); + app.spawn_in(&cmd, cwd); + app.pump(); + let id = app.views[0].id; + + // Peek, then attach the same task: the id is unchanged, the pair is not. + app.set_watch(Some((id, false))); + app.set_watch(Some((id, true))); + app.mode = Mode::Attached; + app.focused_id = Some(id); + + std::fs::write(&flag, b"").unwrap(); + let ok = wait_until(Duration::from_secs(5), || { + app.pump(); + !app.pending_clipboard.is_empty() + }); + assert!( + ok, + "the post-attach store never forwarded: the kind change never reached the core" + ); + assert_eq!( + app.pending_clipboard, + vec![(ClipboardKind::Clipboard, "post".to_string())] + ); + let _ = std::fs::remove_dir_all(&dir); +} + /// Multiple pending stores emit in receipt order (the host clipboard ends /// at the last: last-writer-wins), each under its own kind byte, and the /// notice counts the last entry's chars, not its bytes. diff --git a/src/core.rs b/src/core.rs index 8a11db6..81f989a 100644 --- a/src/core.rs +++ b/src/core.rs @@ -224,7 +224,10 @@ mod tests { })) .unwrap(); wake_tx - .send(Wake::Cmd(Command::Watch { id: Some(1) })) + .send(Wake::Cmd(Command::Watch { + id: Some(1), + attached: true, + })) .unwrap(); // Wait for the task to come up and emit its first (blank) screen. diff --git a/src/protocol.rs b/src/protocol.rs index 35a8619..90b814e 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -12,7 +12,7 @@ use base64::{Engine as _, engine::general_purpose::STANDARD as B64}; use crate::frame::{KIND_CONTROL, KIND_HELLO, KIND_SCREEN}; /// Wire-protocol version; the handshake rejects mismatched peers. -pub const PROTOCOL_VERSION: u32 = 9; +pub const PROTOCOL_VERSION: u32 = 10; /// Environment and working directory supplied by the launching client. #[derive(Debug, Clone, PartialEq)] @@ -71,7 +71,12 @@ pub enum Command { /// client has already subtracted the row it reserves for its status bar. Resize { rows: u16, cols: u16 }, /// Stream this task's screen (attach or peek), or `None` to stop. - Watch { id: Option }, + /// `attached` carries the attach/peek distinction to where clipboard + /// forwarding is enforced: attachment is the consent proxy (input flows + /// to the child only then), so the supervisor forwards OSC 52 stores only + /// for an attach-watch. Peek forwards no input, so nothing captured + /// during peek can be user-caused. + Watch { id: Option, attached: bool }, /// Forward raw keystroke bytes to a task's PTY. Input { id: u64, bytes: Vec }, /// Clipboard paste for a task. Kept distinct from `Input` because the @@ -579,7 +584,7 @@ pub fn encode_command(cmd: &Command) -> (u8, Vec) { let _ = o.insert("rows", *rows as u64); let _ = o.insert("cols", *cols as u64); } - Command::Watch { id } => { + Command::Watch { id, attached } => { let _ = o.insert("t", "watch"); match id { Some(n) => { @@ -589,6 +594,7 @@ pub fn encode_command(cmd: &Command) -> (u8, Vec) { let _ = o.insert("id", jzon::JsonValue::Null); } } + let _ = o.insert("attached", *attached); } // Encode both byte-carrying commands as base64. The paste-size bound in // `app` accounts for base64 expansion and the frame limit. @@ -744,6 +750,10 @@ pub fn decode_command(kind: u8, payload: &[u8]) -> Option { } else { Some(v["id"].as_u64()?) }, + // Required, not defaulted: the flag gates clipboard forwarding, + // and a frame without it (an older client) must reject rather + // than have the daemon guess the user's mode. + attached: v["attached"].as_bool()?, }, "input" => Command::Input { id: v["id"].as_u64()?, @@ -1090,8 +1100,18 @@ mod tests { rows: 30, cols: 100, }, - Command::Watch { id: Some(5) }, - Command::Watch { id: None }, + Command::Watch { + id: Some(5), + attached: true, + }, + Command::Watch { + id: Some(5), + attached: false, + }, + Command::Watch { + id: None, + attached: false, + }, Command::Input { id: 1, bytes: vec![0, 27, 91, 255], @@ -1729,6 +1749,43 @@ mod tests { ); } + /// `Watch` pins its wire shape and requires the `attached` flag: a frame + /// without it (an older client) or with a non-boolean value is rejected + /// whole. The flag gates clipboard forwarding, so the daemon never + /// defaults it. + #[test] + fn watch_wire_form_requires_the_attached_flag() { + let (k, p) = encode_command(&Command::Watch { + id: Some(5), + attached: true, + }); + assert_eq!(k, KIND_CONTROL); + assert_eq!( + std::str::from_utf8(&p).unwrap(), + r#"{"t":"watch","id":5,"attached":true}"# + ); + let (_, p) = encode_command(&Command::Watch { + id: None, + attached: false, + }); + assert_eq!( + std::str::from_utf8(&p).unwrap(), + r#"{"t":"watch","id":null,"attached":false}"# + ); + for json in [ + r#"{"t":"watch","id":5}"#, // missing flag + r#"{"t":"watch","id":null}"#, // missing flag on unwatch + r#"{"t":"watch","id":5,"attached":null}"#, // null is not a kind + r#"{"t":"watch","id":5,"attached":1}"#, // flag must be a boolean + ] { + assert_eq!( + decode_command(KIND_CONTROL, json.as_bytes()), + None, + "should reject {json}" + ); + } + } + /// `LoadRecovery` requires a string stem in its wire representation. #[test] fn load_recovery_wire_form() { diff --git a/src/supervisor.rs b/src/supervisor.rs index b06d44d..c51db29 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -236,6 +236,12 @@ pub struct Supervisor { scrollback: usize, /// The task whose screen the client is watching (attach/peek), or `None`. watched: Option, + /// Whether the current watch is an attach rather than a peek. Attachment + /// is the consent proxy for clipboard forwarding — input flows to the + /// child only then — so `tick` forwards OSC 52 stores only while this is + /// set. Screen streaming ignores it: peek needs screens. Meaningful only + /// while `watched` is `Some`. + watch_attached: bool, /// The last emitted screen fingerprint. `lines` stays empty because only /// emitted copies carry them. Cleared when `watched` changes to force a /// fresh screen after attachment. @@ -269,6 +275,7 @@ impl Supervisor { cols, scrollback, watched: None, + watch_attached: false, last_screen: None, launch: None, events: Vec::new(), @@ -324,6 +331,7 @@ impl Supervisor { t.scroll_view(ScrollAction::Live); } self.watched = None; + self.watch_attached = false; self.last_screen = None; } @@ -400,21 +408,27 @@ impl Supervisor { let _ = t.resize(self.rows, self.cols); } } - Command::Watch { id } => { - // Reset the previous task's viewport before changing targets. - if id != self.watched { - if let Some(old) = self.watched - && let Some(t) = self.by_id_mut(old) - { - t.scroll_view(ScrollAction::Live); - } + Command::Watch { id, attached } => { + // Reset the previous task's viewport only when the target + // itself changes: a peek→attach on the same task must keep + // the user's scrollback position. + if id != self.watched + && let Some(old) = self.watched + && let Some(t) = self.by_id_mut(old) + { + t.scroll_view(ScrollAction::Live); + } + if id != self.watched || attached != self.watch_attached { // Purge the new target's buffered stores before the watch // takes effect. The wake loop applies a whole burst before // ticking, so without this a store captured while - // backgrounded survives into a tick that already sees the - // task as watched — and fires. The residual window (bytes - // emitted pre-attach but parsed post-purge) is - // irreducible: a transparent terminal has it too. + // backgrounded — or during a peek of this same task, the + // peek→attach case — survives into a tick that already + // sees an attach-watch, and fires. Purging on the + // attach→peek edge too is harmless: peek forwards + // nothing. The residual window (bytes emitted pre-attach + // but parsed post-purge) is irreducible: a transparent + // terminal has it too. if let Some(new) = id && let Some(t) = self.by_id_mut(new) { @@ -423,6 +437,7 @@ impl Supervisor { self.last_screen = None; } self.watched = id; + self.watch_attached = attached; } Command::Input { id, bytes } => { let refused = self.by_id_mut(id).and_then(|t| t.send_input(&bytes).err()); @@ -544,7 +559,13 @@ impl Supervisor { // least every 200 ms), so an expired sync flushes here, before the // preview resolution reads the grid, letting the same tick ship it. // Resolution mutates per-task hold state; all tasks use one timestamp. - let watched = self.watched; + // Forward stores only for an attach-watch: peek forwards no input to + // the child, so nothing captured during peek can be user-caused. + let forwarding = if self.watch_attached { + self.watched + } else { + None + }; let mut clipboard = None; let views = self .tasks @@ -552,16 +573,16 @@ impl Supervisor { .map(|t| { t.flush_expired_sync(); // Drain every task's clipboard every tick and forward only the - // watched task's. Dropping the others here is the staleness - // guarantee: a store captured while backgrounded must never - // fire when the task is later watched — a wrong clipboard is - // silently harmful, an empty one visibly inert. The two-slot - // capture bound makes the constant drain cheap. This drain - // alone cannot close the wake-coalescing race (a `Watch` in - // the same burst lands before the tick); `apply`'s `Watch` - // arm purges the new target for that case. + // attach-watched task's. Dropping the others here is the + // staleness guarantee: a store captured while backgrounded or + // peeked must never fire when the task is later attached — a + // wrong clipboard is silently harmful, an empty one visibly + // inert. The two-slot capture bound makes the constant drain + // cheap. This drain alone cannot close the wake-coalescing + // race (a `Watch` in the same burst lands before the tick); + // `apply`'s `Watch` arm purges the new target for that case. let stores = t.drain_clipboard(); - if watched == Some(t.id) { + if forwarding == Some(t.id) { clipboard = Some((t.id, stores)); } TaskView { diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index 5c3417e..d72ac0e 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -100,7 +100,10 @@ fn tick_emits_snapshot_and_watched_screen() { _ => panic!("expected a Tasks snapshot"), }; - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); s.tick(); let evs = s.drain(); assert!(evs.iter().any(|e| matches!(e, Event::Tasks(_)))); @@ -133,7 +136,10 @@ fn watched_screen_not_resent_when_unchanged() { } assert!(id != 0, "task never appeared"); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); s.tick(); assert!( s.drain().iter().any(|e| matches!(e, Event::Screen(_))), @@ -163,7 +169,10 @@ fn decset_1007_flip_resends_watched_screen() { f = flag.display() ); let id = spawn_ready(&mut s, cmd, here(), &ready); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); // Wait for the initial alternate-scroll state. let open = wait_until(Duration::from_secs(5), || { @@ -230,7 +239,10 @@ fn watched_task_clipboard_stores_are_forwarded() { f = flag.display() ); let id = spawn_ready(&mut s, cmd, here(), &ready); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); std::fs::write(&flag, b"").unwrap(); let mut copies = Vec::new(); @@ -280,7 +292,10 @@ fn watch_purges_stores_captured_before_the_watch() { assert!(parsed, "the marker never reached the grid"); let id = s.tasks[0].id; - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); let mut saw_screen = false; for _ in 0..3 { s.tick(); @@ -338,7 +353,10 @@ fn backgrounded_clipboard_store_is_discarded_not_deferred() { s.tick(); let _ = s.drain(); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); let mut saw_screen = false; for _ in 0..3 { s.tick(); @@ -355,6 +373,115 @@ fn backgrounded_clipboard_store_is_discarded_not_deferred() { assert!(saw_screen, "watching the task should stream its screen"); } +/// Peek is clipboard-inert end to end, and consent is not retroactive. Three +/// phases against one task: a store drained while peek-watched never +/// forwards (though the peek's screen keeps streaming); a store captured +/// during peek and still buffered when the attach-watch lands — the +/// peek→attach straddle, with no tick between, exactly how the wake loop +/// applies a burst — dies in the watch-time purge instead of firing under +/// the new attach; a store emitted under the attach-watch forwards. Markers +/// are observed through non-draining grid reads; a tick while waiting would +/// drain the buffer and mask both races. +#[test] +fn peeked_stores_never_forward_and_die_at_the_attach_transition() { + let dir = scratch("clip_peek"); + let ready = dir.join("ready"); + let flag1 = dir.join("flag1"); + let flag2 = dir.join("flag2"); + let flag3 = dir.join("flag3"); + let mut s = sup(24, 80); + // "cGVlazE=" is "peek1", "cGVlazI=" is "peek2", "cG9zdA==" is "post". + // Each marker follows its store in the output stream, proving the + // store's bytes were parsed by the time the marker is visible. + let cmd = format!( + "touch {r}; until [ -e {f1} ]; do sleep 0.05; done; \ + printf '\\033]52;c;cGVlazE=\\007M1'; \ + until [ -e {f2} ]; do sleep 0.05; done; \ + printf '\\033]52;c;cGVlazI=\\007M2'; \ + until [ -e {f3} ]; do sleep 0.05; done; \ + printf '\\033]52;c;cG9zdA==\\007M3'; sleep 30", + r = ready.display(), + f1 = flag1.display(), + f2 = flag2.display(), + f3 = flag3.display() + ); + let id = spawn_ready(&mut s, cmd, here(), &ready); + s.apply(Command::Watch { + id: Some(id), + attached: false, + }); + + // Phase 1: drained while peeked, never forwarded. + std::fs::write(&flag1, b"").unwrap(); + let parsed = wait_until(Duration::from_secs(5), || { + s.tasks.first().is_some_and(|t| { + let (formatted, _, _) = t.formatted(); + String::from_utf8_lossy(&formatted).contains("M1") + }) + }); + assert!(parsed, "the first marker never reached the grid"); + let mut saw_screen = false; + for _ in 0..3 { + s.tick(); + for e in s.drain() { + match e { + Event::ClipboardCopy { .. } => { + panic!("a peeked task's store must never forward") + } + Event::Screen(_) => saw_screen = true, + _ => {} + } + } + } + assert!( + saw_screen, + "peeking the task should still stream its screen" + ); + + // Phase 2: the straddle. The store sits in the buffer across the + // peek→attach on the same id; only the purge on the kind change stops + // the next (attached) tick from forwarding it. + std::fs::write(&flag2, b"").unwrap(); + let parsed = wait_until(Duration::from_secs(5), || { + s.tasks.first().is_some_and(|t| { + let (formatted, _, _) = t.formatted(); + String::from_utf8_lossy(&formatted).contains("M2") + }) + }); + assert!(parsed, "the second marker never reached the grid"); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); + for _ in 0..3 { + s.tick(); + for e in s.drain() { + if let Event::ClipboardCopy { .. } = e { + panic!("a store captured during peek must not fire after attach") + } + } + } + + // Phase 3: a store emitted under the attach-watch forwards, and it is + // the only one that ever does. + std::fs::write(&flag3, b"").unwrap(); + let mut copies = Vec::new(); + let ok = wait_until(Duration::from_secs(5), || { + s.tick(); + copies.extend(s.drain().into_iter().filter_map(|e| match e { + Event::ClipboardCopy { id, kind, text } => Some((id, kind, text)), + _ => None, + })); + !copies.is_empty() + }); + assert!(ok, "the post-attach store never arrived"); + assert_eq!( + copies, + vec![(id, ClipboardKind::Clipboard, "post".to_string())] + ); + let _ = std::fs::remove_dir_all(&dir); +} + /// An over-cap store on the watched task yields the drop notice and no /// `ClipboardCopy`: the copy is lost loudly, not truncated or forwarded. #[test] @@ -375,7 +502,10 @@ fn oversized_watched_store_yields_notice_and_no_copy() { f = flag.display() ); let id = spawn_ready(&mut s, cmd, here(), &ready); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); std::fs::write(&flag, b"").unwrap(); let mut notice = None; @@ -615,7 +745,10 @@ fn clear_watch_stops_screen_stream_and_resets_dedup() { spawn(&mut s, "sleep 30", here()); let id = first_id(&mut s); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); s.tick(); assert!( s.drain().iter().any(|e| matches!(e, Event::Screen(_))), @@ -632,7 +765,10 @@ fn clear_watch_stops_screen_stream_and_resets_dedup() { // A new client watching the same task gets a full screen at once, even // though the screen bytes haven't changed since the last send. - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); s.tick(); assert!( s.drain().iter().any(|e| matches!(e, Event::Screen(_))), @@ -647,7 +783,10 @@ fn clear_watch_snaps_the_watched_task_live() { let mut s = sup(6, 80); spawn(&mut s, "seq 1 200; sleep 30", here()); let id = first_id(&mut s); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); // Retry until output has produced retained history. let scrolled = wait_until(Duration::from_secs(5), || { @@ -925,7 +1064,10 @@ fn rerun_watched_task_resends_screen() { let id = first_id(&mut s); wait_for_lifecycle(&mut s, id, |l| l == Lifecycle::Ok); - s.apply(Command::Watch { id: Some(id) }); + s.apply(Command::Watch { + id: Some(id), + attached: true, + }); s.tick(); assert!( s.drain().iter().any(|e| matches!(e, Event::Screen(_))), diff --git a/src/transport.rs b/src/transport.rs index 7e1c058..5833acd 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -290,7 +290,10 @@ mod tests { assert!(t.connected()); drop(theirs); // the daemon is gone - t.send(Command::Watch { id: None }); + t.send(Command::Watch { + id: None, + attached: false, + }); assert!(!t.connected(), "a failed send must mark the transport dead"); // The stream was shut down with it, so the reader thread saw EOF and // exited: joining it cannot hang. diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 9eb31b9..da75524 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -17,7 +17,7 @@ use std::{ /// The protocol version this test suite speaks; must track /// `protocol::PROTOCOL_VERSION` (drift fails the handshake, loudly). -pub const PROTOCOL_VERSION: u32 = 9; +pub const PROTOCOL_VERSION: u32 = 10; /// One frame of the given kind: `[u32 len][kind][payload]`. pub fn frame(kind: u8, payload: &[u8]) -> Vec { From 9f0d6deeb386245932daf4305511d79b27304b20 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 18:36:16 -0700 Subject: [PATCH 7/8] fix(clipboard): keep the raw OSC 52 selector; p and s are distinct targets --- src/app.rs | 14 +-- src/app_tests.rs | 18 +++- src/protocol.rs | 64 ++++++++---- src/supervisor.rs | 18 ++-- src/supervisor_tests.rs | 10 +- src/terminal/emulator.rs | 213 ++++++++++++++++++++++++++------------- 6 files changed, 230 insertions(+), 107 deletions(-) diff --git a/src/app.rs b/src/app.rs index e970b0d..7e544b3 100644 --- a/src/app.rs +++ b/src/app.rs @@ -730,14 +730,16 @@ impl App { } let mut last = 0; for (kind, text) in self.pending_clipboard.drain(..) { - // Emit the kind verbatim. Collapsing `s` to `c` (the original - // design) creates a wrong-content collision: one batch can hold - // one store per kind, and the later-emitted selection payload - // would overwrite the clipboard payload. A host without `s` - // support ignores the sequence — unsupported means inert, not - // aimed at a different clipboard. + // Emit the kind verbatim. Collapsing any pair (the original + // design folded `s` into `c`) creates a wrong-content collision: + // one batch can hold one store per kind, and a later-emitted + // payload would overwrite an earlier one aimed at the same + // collapsed target. A host without `p` or `s` support ignores + // the sequence — unsupported means inert, not aimed at a + // different clipboard. let k = match kind { ClipboardKind::Clipboard => 'c', + ClipboardKind::Primary => 'p', ClipboardKind::Selection => 's', }; write!(out, "\x1b]52;{k};{}\x07", B64.encode(&text))?; diff --git a/src/app_tests.rs b/src/app_tests.rs index c734634..7adf7ce 100644 --- a/src/app_tests.rs +++ b/src/app_tests.rs @@ -1917,6 +1917,20 @@ fn selection_store_emits_its_own_kind_byte() { assert_eq!(out, b"\x1b]52;s;aGVsbG8=\x07"); } +/// A `Primary` store emits its own kind byte `p`, never collapsed to `s` +/// or `c`: the child aimed at the primary selection, and on hosts where +/// the targets differ the byte is the aim. +#[test] +fn primary_store_emits_its_own_kind_byte() { + let mut app = App::new_local(30, 100); + app.mode = Mode::Attached; + app.focused_id = Some(1); + app.on_clipboard_copy(1, ClipboardKind::Primary, "hello".to_string()); + let mut out = Vec::new(); + app.flush_clipboard(&mut out).unwrap(); + assert_eq!(out, b"\x1b]52;p;aGVsbG8=\x07"); +} + /// Nothing from the payload reaches the terminal raw: ESC/CSI sequences and /// newlines cross only as base64 between the envelope prefix and the BEL. #[test] @@ -2036,13 +2050,15 @@ fn pending_stores_emit_in_order_and_notice_counts_last_entry_chars() { app.mode = Mode::Attached; app.focused_id = Some(1); app.on_clipboard_copy(1, ClipboardKind::Clipboard, "first".to_string()); + app.on_clipboard_copy(1, ClipboardKind::Primary, "second".to_string()); app.on_clipboard_copy(1, ClipboardKind::Selection, "héllo日".to_string()); let mut out = Vec::new(); app.flush_clipboard(&mut out).unwrap(); let expected = format!( - "\x1b]52;c;{}\x07\x1b]52;s;{}\x07", + "\x1b]52;c;{}\x07\x1b]52;p;{}\x07\x1b]52;s;{}\x07", B64.encode("first"), + B64.encode("second"), B64.encode("héllo日") ); assert_eq!(out, expected.as_bytes()); diff --git a/src/protocol.rs b/src/protocol.rs index 90b814e..5bd0bd3 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -202,8 +202,9 @@ pub enum Event { recovery: Vec, }, /// One OSC 52 clipboard store from the watched task, already base64-decoded - /// by the terminal backend. Store-only: no load or query path exists, so - /// this event never solicits a reply. `id` names the source task so the + /// by the emulator's capture pipeline. Store-only: no load or query path + /// exists, so this event never solicits a reply. `id` names the source + /// task so the /// client can drop a copy still in flight when it switches attachment: /// the wire preserves ordering per direction, not across a Watch/forward /// cross. @@ -215,13 +216,17 @@ pub enum Event { } /// Which clipboard an OSC 52 store targets. A wire-layer mirror of the -/// emulator's `ClipboardType`, kept here so `protocol` stays free of -/// `alacritty_terminal` types; the supervisor maps at its boundary. +/// emulator's `ClipboardSelector`, kept here so `protocol` stays free of +/// terminal-module types; the supervisor maps at its boundary. One variant +/// per raw selector byte: `p` and `s` are distinct xterm targets and must +/// not fold. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ClipboardKind { /// The system clipboard (OSC 52 kind byte `c`). Clipboard, - /// The primary selection (OSC 52 kind byte `s`). + /// The primary selection (OSC 52 kind byte `p`). + Primary, + /// The select buffer (OSC 52 kind byte `s`). Selection, } @@ -928,6 +933,7 @@ pub fn encode_event(ev: &Event) -> (u8, Vec) { "k", match kind { ClipboardKind::Clipboard => "c", + ClipboardKind::Primary => "p", ClipboardKind::Selection => "s", }, ); @@ -1022,6 +1028,7 @@ pub fn decode_event(kind: u8, payload: &[u8]) -> Option { let id = v["id"].as_u64()?; let kind = match v["k"].as_str()? { "c" => ClipboardKind::Clipboard, + "p" => ClipboardKind::Primary, "s" => ClipboardKind::Selection, _ => return None, }; @@ -1831,7 +1838,7 @@ mod tests { assert_eq!(decode_event(k, &p), Some(screen)); } - /// `ClipboardCopy` round-trips both kinds and carries clipboard text + /// `ClipboardCopy` round-trips all three kinds and carries clipboard text /// unsanitized: unicode, control characters, and embedded markers survive /// the JSON+base64 path byte-for-byte. The source id survives too, /// including values past `u32`. @@ -1848,6 +1855,11 @@ mod tests { kind: ClipboardKind::Selection, text: "sélection λ 🦀".into(), }, + Event::ClipboardCopy { + id: 2, + kind: ClipboardKind::Primary, + text: "primary".into(), + }, Event::ClipboardCopy { id: 7, kind: ClipboardKind::Clipboard, @@ -1865,18 +1877,32 @@ mod tests { /// `ClipboardCopy` pins its wire shape: `t` discriminator, the source /// task under `id`, the OSC 52 kind byte under `k`, and base64 text. + /// All three kind strings are pinned — `"p"` in particular, so the + /// primary selection can never silently re-fold into `"s"`. #[test] fn clipboard_copy_wire_form() { - let (k, p) = encode_event(&Event::ClipboardCopy { - id: 5, - kind: ClipboardKind::Selection, - text: "hi".into(), - }); - assert_eq!(k, KIND_CONTROL); - assert_eq!( - std::str::from_utf8(&p).unwrap(), - r#"{"t":"clip","id":5,"k":"s","text":"aGk="}"# - ); + for (kind, wire) in [ + ( + ClipboardKind::Clipboard, + r#"{"t":"clip","id":5,"k":"c","text":"aGk="}"#, + ), + ( + ClipboardKind::Primary, + r#"{"t":"clip","id":5,"k":"p","text":"aGk="}"#, + ), + ( + ClipboardKind::Selection, + r#"{"t":"clip","id":5,"k":"s","text":"aGk="}"#, + ), + ] { + let (k, p) = encode_event(&Event::ClipboardCopy { + id: 5, + kind, + text: "hi".into(), + }); + assert_eq!(k, KIND_CONTROL); + assert_eq!(std::str::from_utf8(&p).unwrap(), wire); + } } /// A malformed `ClipboardCopy` frame is rejected whole: missing fields, @@ -1889,8 +1915,10 @@ mod tests { r#"{"t":"clip","id":1,"k":"c"}"#, // missing text r#"{"t":"clip","k":"c","text":"aGk="}"#, // missing id r#"{"t":"clip","id":"1","k":"c","text":"aGk="}"#, // id must be a number - r#"{"t":"clip","id":1,"k":"p","text":"aGk="}"#, // unknown kind string - r#"{"t":"clip","id":1,"k":"c","text":"!!!"}"#, // invalid base64 + // "x" here, not "p": "p" joined the accept set when the primary + // selection got its own kind. + r#"{"t":"clip","id":1,"k":"x","text":"aGk="}"#, // unknown kind string + r#"{"t":"clip","id":1,"k":"c","text":"!!!"}"#, // invalid base64 r#"{"t":"clip","id":1,"k":"c","text":"/w=="}"#, // 0xFF: not UTF-8 r#"{"t":"clip","id":1,"k":"c","text":["aGk="]}"#, // text must be a string ] { diff --git a/src/supervisor.rs b/src/supervisor.rs index c51db29..df2ec90 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -13,10 +13,9 @@ use std::{ time::{Duration, Instant}, }; -use alacritty_terminal::term::ClipboardType; - use crate::{ core::{Wake, Waker}, + emulator::ClipboardSelector, harness::{self, assets}, path, protocol::{ @@ -123,12 +122,15 @@ fn normalize_group(name: Option) -> Option { normalize_label(name).filter(|g| g != "Unassigned") } -/// Map the emulator's clipboard kind to its wire mirror. The boundary where -/// `alacritty_terminal` types stop: `protocol` deliberately imports none. -fn clipboard_kind(kind: ClipboardType) -> ClipboardKind { +/// Map the emulator's clipboard selector to its wire mirror. The boundary +/// where terminal-module types stop: `protocol` deliberately imports none. +/// Three arms, verbatim — folding any pair here would recreate the fidelity +/// loss this mapping exists to prevent. +fn clipboard_kind(kind: ClipboardSelector) -> ClipboardKind { match kind { - ClipboardType::Clipboard => ClipboardKind::Clipboard, - ClipboardType::Selection => ClipboardKind::Selection, + ClipboardSelector::Clipboard => ClipboardKind::Clipboard, + ClipboardSelector::Primary => ClipboardKind::Primary, + ClipboardSelector::Select => ClipboardKind::Selection, } } @@ -577,7 +579,7 @@ impl Supervisor { // staleness guarantee: a store captured while backgrounded or // peeked must never fire when the task is later attached — a // wrong clipboard is silently harmful, an empty one visibly - // inert. The two-slot capture bound makes the constant drain + // inert. The three-slot capture bound makes the constant drain // cheap. This drain alone cannot close the wake-coalescing // race (a `Watch` in the same burst lands before the tick); // `apply`'s `Watch` arm purges the new target for that case. diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index d72ac0e..c38501d 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -231,10 +231,11 @@ fn watched_task_clipboard_stores_are_forwarded() { let ready = dir.join("ready"); let flag = dir.join("flag"); let mut s = sup(24, 80); - // "aGVsbG8=" is "hello" (clipboard), "d29ybGQ=" is "world" (selection). + // "aGVsbG8=" is "hello" (clipboard), "cHJp" is "pri" (primary), + // "d29ybGQ=" is "world" (select). let cmd = format!( "touch {r}; until [ -e {f} ]; do sleep 0.05; done; \ - printf '\\033]52;c;aGVsbG8=\\007\\033]52;s;d29ybGQ=\\007'; sleep 30", + printf '\\033]52;c;aGVsbG8=\\007\\033]52;p;cHJp\\007\\033]52;s;d29ybGQ=\\007'; sleep 30", r = ready.display(), f = flag.display() ); @@ -252,13 +253,16 @@ fn watched_task_clipboard_stores_are_forwarded() { Event::ClipboardCopy { id, kind, text } => Some((id, kind, text)), _ => None, })); - copies.len() >= 2 + copies.len() >= 3 }); assert!(ok, "the clipboard stores never arrived; got {copies:?}"); + // Three kinds in one burst, each under its own raw selector: a `p` + // store forwards as `Primary`, never re-folded into `Selection`. assert_eq!( copies, vec![ (id, ClipboardKind::Clipboard, "hello".to_string()), + (id, ClipboardKind::Primary, "pri".to_string()), (id, ClipboardKind::Selection, "world".to_string()), ] ); diff --git a/src/terminal/emulator.rs b/src/terminal/emulator.rs index 213d254..7554863 100644 --- a/src/terminal/emulator.rs +++ b/src/terminal/emulator.rs @@ -12,11 +12,12 @@ use alacritty_terminal::{ grid::{Dimensions, Scroll}, index::{Column, Line}, term::{ - ClipboardType, Config, TermMode, + Config, TermMode, cell::{Cell, Flags}, }, vte::ansi::{self as vt, Handler, Processor}, }; +use base64::{Engine as _, engine::general_purpose::STANDARD as B64}; /// Mouse event classes requested by the child through DECSET 1000/1002/1003. #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -49,60 +50,52 @@ const _: () = assert!( "CLIPBOARD_STORE_MAX_BYTES must base64-encode to under frame::MAX_FRAME" ); +/// Which OSC 52 target a store addressed, preserving the raw selector byte. +/// The backend's `ClipboardType` folds `p` and `s` into one variant; per +/// xterm ctlseqs they are distinct targets (the primary selection and the +/// select buffer), so [`ObservedTerm::clipboard_store`] captures the byte +/// before that fold can happen. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ClipboardSelector { + /// The system clipboard (selector byte `c`). + Clipboard, + /// The primary selection (selector byte `p`). + Primary, + /// The select buffer (selector byte `s`). + Select, +} + /// OSC 52 clipboard stores captured since the last drain. A clipboard is /// last-writer-wins by nature, so coalescing to one store per -/// [`ClipboardType`] between drains loses nothing, and the two-slot bound -/// means a never-drained background task cannot accumulate memory. +/// [`ClipboardSelector`] between drains loses nothing, and the three-slot +/// bound means a never-drained background task cannot accumulate memory. #[derive(Debug, Default)] pub struct ClipboardStores { - /// At most one store per [`ClipboardType`], ordered by the surviving + /// At most one store per [`ClipboardSelector`], ordered by the surviving /// store's arrival. - pub stores: Vec<(ClipboardType, String)>, + pub stores: Vec<(ClipboardSelector, String)>, /// Byte length of the most recent store dropped for exceeding /// [`CLIPBOARD_STORE_MAX_BYTES`], kept so the caller can surface a /// notice instead of silently losing the copy. pub oversized_len: Option, } -/// Buffers backend-generated PTY responses and OSC 52 clipboard stores while -/// the parser advances. Other backend events are discarded; [`ObservedTerm`] -/// captures titles directly from parser events. +/// Buffers backend-generated PTY responses while the parser advances. Other +/// backend events are discarded; [`ObservedTerm`] captures titles directly +/// from parser events and OSC 52 stores never reach the backend (see +/// [`ObservedTerm`]'s `clipboard_store`). pub struct ProbeSink { responses: Arc>>, - clipboard: Arc>, } impl EventListener for ProbeSink { fn send_event(&self, event: Event) { - match event { - Event::PtyWrite(text) => { - let mut buf = self - .responses - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - buf.push(text); - } - // The backend has already base64-decoded the payload, validated - // UTF-8, mapped the kind byte, and denied OSC 52 loads under its - // default `Osc52::OnlyCopy`: only decoded stores arrive here. - Event::ClipboardStore(kind, text) => { - let mut buf = self - .clipboard - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - // Last writer wins per kind even when the last writer is - // over-cap: forwarding a superseded store would misrepresent - // the child's final clipboard state, so the drop clears its - // kind's slot and the recorded length feeds the caller's - // notice. - buf.stores.retain(|(k, _)| *k != kind); - if text.len() > CLIPBOARD_STORE_MAX_BYTES { - buf.oversized_len = Some(text.len()); - return; - } - buf.stores.push((kind, text)); - } - _ => {} + if let Event::PtyWrite(text) = event { + let mut buf = self + .responses + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + buf.push(text); } } } @@ -230,7 +223,10 @@ pub struct Emulator { term: Term, parser: Processor, responses: Arc>>, - clipboard: Arc>, + /// OSC 52 stores captured since the last drain. A plain field, not an + /// `Arc>`: stores are written by [`ObservedTerm`] during the + /// parse (like `alt`), never through the backend's event listener. + clipboard: ClipboardStores, /// Alt-screen and title facts, advanced at parser-event granularity by /// [`ObservedTerm`] during the parse itself. alt: AltScreen, @@ -258,14 +254,9 @@ impl Emulator { /// Drain the OSC 52 clipboard stores captured since the last drain, /// plus the oversized-drop record. The caller owns forwarding; a drain /// whose result is dropped discards the stores. An empty capture costs - /// one lock and no allocation, so calling every tick for every task is - /// fine. + /// no allocation, so calling every tick for every task is fine. pub fn drain_clipboard(&mut self) -> ClipboardStores { - let mut buf = self - .clipboard - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - std::mem::take(&mut *buf) + std::mem::take(&mut self.clipboard) } /// Truncate zero-width characters in each active-grid cell to @@ -309,7 +300,6 @@ impl Emulator { /// A fresh `rows`×`cols` grid retaining `scrollback` rows of history. pub fn new(rows: u16, cols: u16, scrollback: usize) -> Self { let responses = Arc::new(Mutex::new(Vec::new())); - let clipboard = Arc::new(Mutex::new(ClipboardStores::default())); let config = Config { // Use fleetcom's per-task history limit instead of the backend // default. @@ -324,14 +314,13 @@ impl Emulator { }, ProbeSink { responses: Arc::clone(&responses), - clipboard: Arc::clone(&clipboard), }, ); Self { term, parser: Processor::new(), responses, - clipboard, + clipboard: ClipboardStores::default(), alt: AltScreen::default(), revision: 0, bytes_since_sweep: 0, @@ -350,6 +339,7 @@ impl Emulator { let mut observed = ObservedTerm { term: &mut self.term, alt: &mut self.alt, + clipboard: &mut self.clipboard, }; self.parser.advance(&mut observed, bytes); self.observe_advance(); @@ -380,6 +370,7 @@ impl Emulator { let mut observed = ObservedTerm { term: &mut self.term, alt: &mut self.alt, + clipboard: &mut self.clipboard, }; self.parser.stop_sync(&mut observed); self.observe_advance(); @@ -399,6 +390,7 @@ impl Emulator { let mut observed = ObservedTerm { term: &mut self.term, alt: &mut self.alt, + clipboard: &mut self.clipboard, }; self.parser.stop_sync(&mut observed); self.observe_advance(); @@ -698,6 +690,9 @@ const TITLE_STACK_SHADOW_MAX: usize = 4096; /// `Handler` methods default to no-ops, so every method must delegate to /// `Term`. `golden::emulator_wrapper_matches_the_raw_backend_on_every_fixture` /// compares wrapper and raw-backend replays to detect missing delegation. +/// One deliberate exception: `clipboard_store` never delegates — the wrapper +/// owns that pipeline outright, and the backend's version touches no grid +/// state the golden comparison could see. /// /// # Synchronized updates /// @@ -709,6 +704,7 @@ const TITLE_STACK_SHADOW_MAX: usize = 4096; struct ObservedTerm<'a> { term: &'a mut Term, alt: &'a mut AltScreen, + clipboard: &'a mut ClipboardStores, } impl ObservedTerm<'_> { @@ -944,10 +940,50 @@ impl Handler for ObservedTerm<'_> { fn reset_color(&mut self, a0: usize) { self.term.reset_color(a0); } + /// The one non-delegating handler: the backend's `clipboard_store` folds + /// both `p` and `s` into `ClipboardType::Selection` before its event + /// fires, so delegating loses the raw selector — a `p` store would + /// re-emit as `s`, and a `p` and an `s` store in one batch would collide + /// in one slot. Owning the pipeline here keeps the byte. + /// + /// Scope bound: vte's OSC 52 dispatch passes only the FIRST selector + /// byte to this handler and defaults an empty selector to `c` + /// (vte 0.15.0 `ansi.rs`, `params[1].first().unwrap_or(&b'c')`), so a + /// multi-target selector like `pc` is first-byte-truncated before + /// anything below can see it. fn clipboard_store(&mut self, a0: u8, a1: &[u8]) { - self.term.clipboard_store(a0, a1); + // Selector policy is ours now, not the backend's: `c`, `p`, and `s` + // buffer under their own slots; anything else (`q`, the numbered cut + // buffers `0`-`7`) drops silently, matching the backend behavior + // this replaces. + let selector = match a0 { + b'c' => ClipboardSelector::Clipboard, + b'p' => ClipboardSelector::Primary, + b's' => ClipboardSelector::Select, + _ => return, + }; + // Same decode contract as the backend: strict padded standard + // base64, then UTF-8. The `!` clear form and the `?` query never + // reach here as text — `?` dispatches to `clipboard_load`, and `!` + // fails the decode. + let Ok(bytes) = B64.decode(a1) else { return }; + let Ok(text) = String::from_utf8(bytes) else { + return; + }; + // Last writer wins per selector even when the last writer is + // over-cap: forwarding a superseded store would misrepresent the + // child's final clipboard state, so the drop clears its selector's + // slot and the recorded length feeds the caller's notice. + self.clipboard.stores.retain(|(k, _)| *k != selector); + if text.len() > CLIPBOARD_STORE_MAX_BYTES { + self.clipboard.oversized_len = Some(text.len()); + return; + } + self.clipboard.stores.push((selector, text)); } fn clipboard_load(&mut self, a0: u8, a1: &str) { + // Still delegated: the backend's default `Osc52::OnlyCopy` denies + // loads, which is exactly the policy fleetcom wants. self.term.clipboard_load(a0, a1); } fn decaln(&mut self) { @@ -1096,7 +1132,7 @@ mod tests { let drained = emu.drain_clipboard(); assert_eq!( drained.stores, - vec![(ClipboardType::Clipboard, "hello".to_string())] + vec![(ClipboardSelector::Clipboard, "hello".to_string())] ); assert_eq!(drained.oversized_len, None); let again = emu.drain_clipboard(); @@ -1104,23 +1140,49 @@ mod tests { assert_eq!(again.oversized_len, None); } - /// The `p` and `s` kind bytes both map to the selection clipboard - /// (the backend's mapping; there is no separate primary kind). + /// The `p` and `s` selectors stay distinct: per xterm ctlseqs they are + /// different targets, and folding them (the backend's mapping this + /// pipeline replaced) would both mislabel a lone `p` store and let a + /// same-batch pair collide in one slot. #[test] - fn osc52_p_and_s_kinds_map_to_selection() { + fn osc52_p_and_s_selectors_stay_distinct() { let mut emu = Emulator::new(4, 20, 0); - emu.process(b"\x1b]52;p;YQ==\x07"); + emu.process(b"\x1b]52;p;YQ==\x07\x1b]52;s;Yg==\x07"); assert_eq!( emu.drain_clipboard().stores, - vec![(ClipboardType::Selection, "a".to_string())] + vec![ + (ClipboardSelector::Primary, "a".to_string()), + (ClipboardSelector::Select, "b".to_string()), + ] ); - emu.process(b"\x1b]52;s;Yg==\x07"); + } + + /// An empty selector arrives as `c`: vte's dispatch defaults it + /// (`params[1].first().unwrap_or(&b'c')`, vte 0.15.0) before the + /// handler runs, so this pins upstream behavior the pipeline depends + /// on, not a choice made here. + #[test] + fn osc52_empty_selector_defaults_to_clipboard() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;;aGk=\x07"); assert_eq!( emu.drain_clipboard().stores, - vec![(ClipboardType::Selection, "b".to_string())] + vec![(ClipboardSelector::Clipboard, "hi".to_string())] ); } + /// Selectors outside `c`/`p`/`s` drop silently: `q` and the numbered + /// cut buffers have no forwarding target. + #[test] + fn osc52_unknown_selectors_drop() { + let mut emu = Emulator::new(4, 20, 0); + emu.process(b"\x1b]52;q;aGk=\x07"); + emu.process(b"\x1b]52;0;aGk=\x07"); + let drained = emu.drain_clipboard(); + assert!(drained.stores.is_empty()); + assert_eq!(drained.oversized_len, None); + } + /// Two stores to the same kind before a drain coalesce: only the second /// survives. #[test] @@ -1129,32 +1191,37 @@ mod tests { emu.process(b"\x1b]52;c;Zmlyc3Q=\x07\x1b]52;c;c2Vjb25k\x07"); assert_eq!( emu.drain_clipboard().stores, - vec![(ClipboardType::Clipboard, "second".to_string())] + vec![(ClipboardSelector::Clipboard, "second".to_string())] ); } - /// Stores to different kinds buffer independently and drain in arrival - /// order. + /// Stores to different selectors buffer independently and drain in + /// arrival order; all three selectors in one batch survive. #[test] fn osc52_kinds_buffer_independently() { let mut emu = Emulator::new(4, 20, 0); - emu.process(b"\x1b]52;s;c2Vs\x07\x1b]52;c;Y2xpcA==\x07"); + emu.process(b"\x1b]52;s;c2Vs\x07\x1b]52;p;cHJp\x07\x1b]52;c;Y2xpcA==\x07"); assert_eq!( emu.drain_clipboard().stores, vec![ - (ClipboardType::Selection, "sel".to_string()), - (ClipboardType::Clipboard, "clip".to_string()), + (ClipboardSelector::Select, "sel".to_string()), + (ClipboardSelector::Primary, "pri".to_string()), + (ClipboardSelector::Clipboard, "clip".to_string()), ] ); } - /// Invalid base64 and the `!` clear form die inside the backend's decode - /// before the capture point: nothing buffers. + /// Invalid base64 (including the `!` clear form) and non-UTF-8 payloads + /// die inside this pipeline's own decode — the backend no longer stands + /// in front of it — so these rejections are load-bearing here: nothing + /// buffers. #[test] fn osc52_invalid_base64_and_clear_buffer_nothing() { let mut emu = Emulator::new(4, 20, 0); emu.process(b"\x1b]52;c;%%%\x07"); emu.process(b"\x1b]52;c;!\x07"); + // "/w==" decodes to 0xFF: valid base64, invalid UTF-8. + emu.process(b"\x1b]52;c;/w==\x07"); let drained = emu.drain_clipboard(); assert!(drained.stores.is_empty()); assert_eq!(drained.oversized_len, None); @@ -1179,19 +1246,20 @@ mod tests { emu.process(b"\x1b]52;c;aGVsbG8=\x1b\\"); assert_eq!( emu.drain_clipboard().stores, - vec![(ClipboardType::Clipboard, "hello".to_string())] + vec![(ClipboardSelector::Clipboard, "hello".to_string())] ); } /// An over-cap store is not buffered, but it still supersedes its - /// kind's slot: forwarding the older store would misrepresent the + /// selector's slot: forwarding the older store would misrepresent the /// child's final clipboard state. The recorded length feeds the - /// caller's notice; other kinds are untouched. + /// caller's notice; other selectors are untouched. #[test] fn osc52_oversized_store_supersedes_its_kind_and_records_length() { let mut emu = Emulator::new(4, 20, 0); emu.process(b"\x1b]52;c;aGVsbG8=\x07"); emu.process(b"\x1b]52;s;c2Vs\x07"); + emu.process(b"\x1b]52;p;cHJp\x07"); // "YWFh" decodes to "aaa"; this repeat count decodes to two bytes // over the cap. let reps = CLIPBOARD_STORE_MAX_BYTES / 3 + 1; @@ -1200,8 +1268,11 @@ mod tests { let drained = emu.drain_clipboard(); assert_eq!( drained.stores, - vec![(ClipboardType::Selection, "sel".to_string())], - "the drop clears its own kind's slot and no other" + vec![ + (ClipboardSelector::Select, "sel".to_string()), + (ClipboardSelector::Primary, "pri".to_string()), + ], + "the drop clears its own selector's slot and no other" ); assert_eq!(drained.oversized_len, Some(reps * 3)); } From cca0e59dad15674247e39e7014ab9a1a0476c868 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Tue, 21 Jul 2026 19:15:12 -0700 Subject: [PATCH 8/8] Refactor clipboard handling documentation --- src/app.rs | 84 +++++------------------------ src/app_tests.rs | 65 +++++++---------------- src/protocol.rs | 59 +++++---------------- src/supervisor.rs | 39 +++----------- src/supervisor_tests.rs | 63 +++++----------------- src/task.rs | 5 +- src/terminal/emulator.rs | 111 +++++++++------------------------------ src/ui.rs | 18 ++----- 8 files changed, 95 insertions(+), 349 deletions(-) diff --git a/src/app.rs b/src/app.rs index 7e544b3..d95c4b4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -45,16 +45,10 @@ const _: () = assert!( "MAX_PASTE must base64-encode to under frame::MAX_FRAME" ); -/// How long an ephemeral notice stays visible. Expiry is lazy — `notice()` -/// answers `None` past this age — and the run loop's 100ms receive timeout -/// bounds how far past the deadline a stale one can stay painted. +/// How long an ephemeral notice remains visible. const NOTICE_TTL: Duration = Duration::from_secs(5); -/// Notice severity, two tiers only — the notice is decoration, never -/// load-bearing. `Warning` covers attached-mode `Event::Status` mirrors -/// (spawn errors, recovery failures, clipboard drops); `Info` covers -/// confirmations (the copied-chars line). The sole ranking rule lives in -/// `set_notice`: `Info` never replaces an unexpired `Warning`. +/// Priority of an ephemeral notice. Active warnings take precedence over info. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum NoticeLevel { Warning, @@ -157,10 +151,7 @@ pub struct App { pub views: Vec, /// The watched task's screen (attach/peek), from `Event::Screen`. focused_screen: Option, - /// Last `Watch` (target, attached) pair sent to the core, so we don't - /// resend it every tick. The pair, not the id: peek→attach on the same - /// task must send a fresh `Watch`, because the core's watch-time purge - /// and its clipboard-forwarding gate both key on the attach flag. + /// Last `(target, attached)` watch state sent to the core. watched: Option<(u64, bool)>, /// Whether this client talks to a daemon (vs. an in-process `--foreground` /// core). Only a daemon client can meaningfully reconnect after a drop. @@ -214,13 +205,9 @@ pub struct App { pub recovery_sel: usize, /// Transient one-line notice (save/load result), dismissed on the next key. pub status: Option, - /// Ephemeral notice, its severity, and the instant it was set: - /// clipboard-copy confirmations, plus attached-mode mirrors of - /// `Event::Status`. A separate channel from `status` — that one persists - /// until replaced or cleared; this one dies of age through `notice()`. + /// Ephemeral notice text, priority, and creation time. notice: Option<(String, NoticeLevel, Instant)>, - /// OSC 52 stores accepted while attached, awaiting re-emission to the host - /// terminal by `flush_clipboard` later in the same run-loop iteration. + /// Clipboard stores awaiting emission to the host terminal. pending_clipboard: Vec<(ClipboardKind, String)>, /// Parsed terminal events from the stdin reader thread. crossterm owns the /// tty, so a dedicated thread blocks on `event::read()` and forwards here; the @@ -616,12 +603,7 @@ impl App { self.selected_id = Some(self.views[sections[prev].1[0]].id); } - /// Tell the core which task's screen we need and whether we are attached - /// (`(id, attached)`), sending `Watch` only when that pair changes. - /// Deduplicating on the pair is load-bearing: an id-only dedup would - /// swallow the peek→attach transition on the same task, and the core's - /// watch-time clipboard purge would never run for it. Cost: one extra - /// command frame per attach. + /// Send the desired watch state when its target or attachment mode changes. fn set_watch(&mut self, want: Option<(u64, bool)>) { if want != self.watched { self.watched = want; @@ -654,13 +636,7 @@ impl App { self.focused_screen = Some(s); } Event::Status(s) => { - // While attached, the dashboard row that displays `status` - // is off screen: mirror the message into the notice so - // supervisor warnings (e.g. the clipboard oversize drop) - // are seen when they happen. `status` is set as always. - // `Warning` because everything the supervisor says while - // attached is operational: spawn errors, recovery - // failures, clipboard drops. + // Mirror attached-mode status messages into the visible notice bar. if self.mode == Mode::Attached { self.set_notice(s.clone(), NoticeLevel::Warning); } @@ -681,24 +657,14 @@ impl App { } } - /// Accept or drop one forwarded OSC 52 store. A clipboard write is an - /// outward-facing side effect; only an attached user plausibly caused it, - /// so every other mode drops the store silently — peek-mode stores and - /// copies still in flight when the user detaches both land here. The id - /// gate drops in-flight copies from a previously watched task: the wire - /// preserves ordering per direction, not across a Watch/forward cross, - /// so a copy from task A can arrive after attachment moved to task B. + /// Queue a clipboard store only when it comes from the attached task. fn on_clipboard_copy(&mut self, id: u64, kind: ClipboardKind, text: String) { if self.mode == Mode::Attached && self.focused_id == Some(id) { self.pending_clipboard.push((kind, text)); } } - /// Stage the ephemeral notice. The newest message wins with one - /// exception: an `Info` set yields to an unexpired `Warning`, because the - /// attached bar is the only place a warning shows and a copy - /// confirmation landing in the same batch would clobber it. An expired - /// `Warning` loses — staleness must not pin warnings forever. + /// Set an ephemeral notice without replacing an active warning with info. fn set_notice(&mut self, msg: String, level: NoticeLevel) { if level == NoticeLevel::Info && let Some((_, NoticeLevel::Warning, set_at)) = self.notice.as_ref() @@ -709,34 +675,19 @@ impl App { self.notice = Some((msg, level, Instant::now())); } - /// The staged notice while it is younger than `NOTICE_TTL`. Expiry is - /// lazy — nothing ever clears the field — because every render pass asks - /// here and the run loop renders at least every ~100ms, which bounds how - /// long an expired notice can stay painted. + /// Return the staged notice while it is younger than `NOTICE_TTL`. pub fn notice(&self) -> Option<&str> { let (msg, _, set_at) = self.notice.as_ref()?; (set_at.elapsed() < NOTICE_TTL).then_some(msg.as_str()) } - /// Re-emit buffered clipboard stores to the host terminal, oldest first: - /// with multiple entries the host's clipboard ends at the last one, - /// matching last-writer-wins clipboard semantics. The payload is data, - /// the envelope is protocol: nothing from the stored text reaches `out` - /// except through base64, because raw payload bytes on the terminal are - /// an escape-sequence injection vector. + /// Emit queued clipboard stores as base64-encoded OSC 52 sequences in order. fn flush_clipboard(&mut self, out: &mut impl Write) -> io::Result<()> { if self.pending_clipboard.is_empty() { return Ok(()); } let mut last = 0; for (kind, text) in self.pending_clipboard.drain(..) { - // Emit the kind verbatim. Collapsing any pair (the original - // design folded `s` into `c`) creates a wrong-content collision: - // one batch can hold one store per kind, and a later-emitted - // payload would overwrite an earlier one aimed at the same - // collapsed target. A host without `p` or `s` support ignores - // the sequence — unsupported means inert, not aimed at a - // different clipboard. let k = match kind { ClipboardKind::Clipboard => 'c', ClipboardKind::Primary => 'p', @@ -746,10 +697,7 @@ impl App { last = copied_chars(&text); } out.flush()?; - // The status row is off screen while attached — exactly when copies - // happen — so the confirmation goes to the notice, which the attached - // bar renders. `Info`: a confirmation must not clobber an unexpired - // warning (the oversize-drop mirror lands in the same iteration). + // Show the confirmation in the attached-mode notice bar. self.set_notice(format!("copied {last} chars"), NoticeLevel::Info); Ok(()) } @@ -803,10 +751,7 @@ impl App { self.focused_id = None; } - // After the `should_quit` break, so a final partial iteration's - // pending stores drop with the session instead of landing on a - // torn-down terminal; before `ui::render`, whose sequential turn - // keeps these bytes from interleaving with a frame. + // Emit accepted clipboard stores before painting the next frame. self.flush_clipboard(out)?; self.sync_input_modes(out)?; ui::render(out, self)?; @@ -1620,8 +1565,7 @@ fn on_key_edit(buf: &mut EditBuffer, k: KeyEvent) -> Option { } } -/// Size of an emitted clipboard payload as the "copied {n} chars" notice -/// reports it: chars, not bytes, matching what the user sees when pasting. +/// Count the characters reported by the clipboard-copy notice. fn copied_chars(text: &str) -> usize { text.chars().count() } diff --git a/src/app_tests.rs b/src/app_tests.rs index 7adf7ce..0bf15f1 100644 --- a/src/app_tests.rs +++ b/src/app_tests.rs @@ -1885,8 +1885,7 @@ fn state_and_dir_mode_spawns_stay_unassigned() { // --- OSC 52 clipboard emission ------------------------------------------ -/// An attached clipboard store re-emits as exactly one OSC 52 envelope: -/// kind byte `c`, padded standard base64, BEL-terminated. +/// An attached clipboard store emits one BEL-terminated OSC 52 sequence. #[test] fn attached_clipboard_store_emits_the_osc52_envelope() { let mut app = App::new_local(30, 100); @@ -1903,9 +1902,7 @@ fn attached_clipboard_store_emits_the_osc52_envelope() { ); } -/// A `Selection` store emits its own kind byte `s`, never collapsed to `c`: -/// collapsing would let a same-batch selection payload overwrite the -/// clipboard payload. A host without `s` support ignores the sequence. +/// A selection store emits the `s` selector. #[test] fn selection_store_emits_its_own_kind_byte() { let mut app = App::new_local(30, 100); @@ -1917,9 +1914,7 @@ fn selection_store_emits_its_own_kind_byte() { assert_eq!(out, b"\x1b]52;s;aGVsbG8=\x07"); } -/// A `Primary` store emits its own kind byte `p`, never collapsed to `s` -/// or `c`: the child aimed at the primary selection, and on hosts where -/// the targets differ the byte is the aim. +/// A primary-selection store emits the `p` selector. #[test] fn primary_store_emits_its_own_kind_byte() { let mut app = App::new_local(30, 100); @@ -1931,8 +1926,7 @@ fn primary_store_emits_its_own_kind_byte() { assert_eq!(out, b"\x1b]52;p;aGVsbG8=\x07"); } -/// Nothing from the payload reaches the terminal raw: ESC/CSI sequences and -/// newlines cross only as base64 between the envelope prefix and the BEL. +/// Clipboard payloads are base64-encoded before reaching the host terminal. #[test] fn clipboard_payload_bytes_never_reach_the_terminal_raw() { let payload = "line1\nline2\x1b[31mred\x1b]52;c;evil\x07"; @@ -1958,9 +1952,7 @@ fn clipboard_payload_bytes_never_reach_the_terminal_raw() { ); } -/// Stores arriving outside attached mode buffer nothing and emit nothing: -/// only an attached user plausibly caused the copy. The id matches -/// `focused_id` so the mode gate alone is what drops the store. +/// Clipboard stores are ignored outside attached mode. #[test] fn clipboard_stores_outside_attached_mode_are_dropped() { for mode in [Mode::Peek, Mode::Dashboard] { @@ -1976,10 +1968,7 @@ fn clipboard_stores_outside_attached_mode_are_dropped() { } } -/// A store whose id is not the attached task's drops at receipt: the wire -/// preserves ordering per direction, not across a Watch/forward cross, so a -/// copy from the previously watched task can arrive after attachment moved. -/// The matching id buffers as before. +/// Stores from tasks other than the attached task are ignored. #[test] fn mismatched_id_clipboard_store_drops_at_receipt() { let mut app = App::new_local(30, 100); @@ -1997,20 +1986,13 @@ fn mismatched_id_clipboard_store_drops_at_receipt() { ); } -/// `set_watch` deduplicates on the (id, attached) pair, not the id: the -/// peek→attach transition on the same task must send a fresh `Watch`, or the -/// core keeps treating the watch as a peek and never forwards. There is no -/// command-observation seam on the in-process transport (it applies commands -/// straight to the supervisor), so the resend is asserted through core -/// behavior: a store emitted after the transition forwards, which cannot -/// happen unless the attach-kind `Watch` actually left the client. +/// Changing from peek to attach sends a new watch for the same task. #[test] fn set_watch_resends_on_kind_change_with_the_same_id() { let dir = temp("app_watch_kind"); let flag = dir.join("flag"); let mut app = App::new_local(30, 100); let cwd = app.invocation_dir.clone(); - // "cG9zdA==" is "post". let cmd = format!( "until [ -e {f} ]; do sleep 0.05; done; printf '\\033]52;c;cG9zdA==\\007'; sleep 30", f = flag.display() @@ -2019,7 +2001,7 @@ fn set_watch_resends_on_kind_change_with_the_same_id() { app.pump(); let id = app.views[0].id; - // Peek, then attach the same task: the id is unchanged, the pair is not. + // Change only the attachment mode. app.set_watch(Some((id, false))); app.set_watch(Some((id, true))); app.mode = Mode::Attached; @@ -2041,9 +2023,7 @@ fn set_watch_resends_on_kind_change_with_the_same_id() { let _ = std::fs::remove_dir_all(&dir); } -/// Multiple pending stores emit in receipt order (the host clipboard ends -/// at the last: last-writer-wins), each under its own kind byte, and the -/// notice counts the last entry's chars, not its bytes. +/// Pending stores emit in order, and the notice counts the last store's characters. #[test] fn pending_stores_emit_in_order_and_notice_counts_last_entry_chars() { let mut app = App::new_local(30, 100); @@ -2062,7 +2042,7 @@ fn pending_stores_emit_in_order_and_notice_counts_last_entry_chars() { B64.encode("héllo日") ); assert_eq!(out, expected.as_bytes()); - // "héllo日" is 6 chars but 9 bytes: the notice must report chars. + // The final payload contains six characters and nine bytes. assert_eq!(app.notice(), Some("copied 6 chars")); assert!( app.pending_clipboard.is_empty(), @@ -2070,7 +2050,7 @@ fn pending_stores_emit_in_order_and_notice_counts_last_entry_chars() { ); } -/// The common per-iteration case, an empty buffer, writes zero bytes. +/// Flushing an empty clipboard buffer writes nothing. #[test] fn empty_clipboard_flush_writes_nothing() { let mut app = App::new_local(30, 100); @@ -2081,8 +2061,7 @@ fn empty_clipboard_flush_writes_nothing() { assert!(app.notice().is_none()); } -/// The notice dies of age: the accessor answers `None` once `NOTICE_TTL` has -/// passed. No clearing pass exists — expiry is the accessor's answer. +/// Notices are hidden after `NOTICE_TTL`. #[test] fn notice_expires_lazily_after_the_ttl() { let mut app = App::new_local(30, 100); @@ -2096,10 +2075,7 @@ fn notice_expires_lazily_after_the_ttl() { assert_eq!(app.notice(), None, "an aged-out notice must not render"); } -/// A live `Warning` survives an `Info` set: the copy confirmation emitted by -/// `flush_clipboard` must not clobber the oversize-drop mirror that landed -/// in the same iteration — the attached bar is the only place that warning -/// shows. The copy itself still emits; only the notice yields. +/// A copy confirmation does not replace an active warning. #[test] fn warning_notice_survives_the_copy_confirmation() { let mut app = App::new_local(30, 100); @@ -2113,7 +2089,7 @@ fn warning_notice_survives_the_copy_confirmation() { assert_eq!(app.notice(), Some("clipboard copy dropped")); } -/// `Info` replaces `Info`: a second copy updates the count. +/// A new info notice replaces the current info notice. #[test] fn info_notice_replaces_info() { let mut app = App::new_local(30, 100); @@ -2122,8 +2098,7 @@ fn info_notice_replaces_info() { assert_eq!(app.notice(), Some("copied 2 chars")); } -/// `Warning` replaces everything, `Info` included: a fresh operational -/// message always shows. +/// A warning replaces any current notice. #[test] fn warning_notice_replaces_info() { let mut app = App::new_local(30, 100); @@ -2139,8 +2114,7 @@ fn warning_notice_replaces_info() { ); } -/// An expired `Warning` loses to `Info`: staleness must not pin warnings -/// forever — the yield rule reads the same TTL clock as `notice()`. +/// An info notice replaces an expired warning. #[test] fn expired_warning_yields_to_info() { let mut app = App::new_local(30, 100); @@ -2152,9 +2126,7 @@ fn expired_warning_yields_to_info() { assert_eq!(app.notice(), Some("copied 5 chars")); } -/// A status event arriving while attached mirrors into the notice — the -/// dashboard row that displays `status` is off screen there — and still sets -/// the persistent status verbatim. +/// Attached-mode status events update both the notice and persistent status. #[test] fn attached_status_event_mirrors_into_the_notice() { let dir = session_scratch("status_mirror", &[]); @@ -2167,8 +2139,7 @@ fn attached_status_event_mirrors_into_the_notice() { let _ = std::fs::remove_dir_all(&dir); } -/// A status event on the dashboard stays out of the notice: the command row -/// already displays `status` there, and the attached bar is off screen. +/// Dashboard status events do not create an ephemeral notice. #[test] fn dashboard_status_event_sets_only_the_status() { let dir = session_scratch("status_dash", &[]); diff --git a/src/protocol.rs b/src/protocol.rs index 5bd0bd3..5ee19d0 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -71,11 +71,7 @@ pub enum Command { /// client has already subtracted the row it reserves for its status bar. Resize { rows: u16, cols: u16 }, /// Stream this task's screen (attach or peek), or `None` to stop. - /// `attached` carries the attach/peek distinction to where clipboard - /// forwarding is enforced: attachment is the consent proxy (input flows - /// to the child only then), so the supervisor forwards OSC 52 stores only - /// for an attach-watch. Peek forwards no input, so nothing captured - /// during peek can be user-caused. + /// `attached` permits clipboard forwarding from the watched task. Watch { id: Option, attached: bool }, /// Forward raw keystroke bytes to a task's PTY. Input { id: u64, bytes: Vec }, @@ -201,13 +197,7 @@ pub enum Event { names: Vec, recovery: Vec, }, - /// One OSC 52 clipboard store from the watched task, already base64-decoded - /// by the emulator's capture pipeline. Store-only: no load or query path - /// exists, so this event never solicits a reply. `id` names the source - /// task so the - /// client can drop a copy still in flight when it switches attachment: - /// the wire preserves ordering per direction, not across a Watch/forward - /// cross. + /// A decoded OSC 52 clipboard store from the attached task. ClipboardCopy { id: u64, kind: ClipboardKind, @@ -215,11 +205,7 @@ pub enum Event { }, } -/// Which clipboard an OSC 52 store targets. A wire-layer mirror of the -/// emulator's `ClipboardSelector`, kept here so `protocol` stays free of -/// terminal-module types; the supervisor maps at its boundary. One variant -/// per raw selector byte: `p` and `s` are distinct xterm targets and must -/// not fold. +/// Clipboard target carried by an OSC 52 event. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ClipboardKind { /// The system clipboard (OSC 52 kind byte `c`). @@ -755,9 +741,7 @@ pub fn decode_command(kind: u8, payload: &[u8]) -> Option { } else { Some(v["id"].as_u64()?) }, - // Required, not defaulted: the flag gates clipboard forwarding, - // and a frame without it (an older client) must reject rather - // than have the daemon guess the user's mode. + // Reject watch frames that do not specify an attachment mode. attached: v["attached"].as_bool()?, }, "input" => Command::Input { @@ -921,10 +905,7 @@ pub fn encode_event(ev: &Event) -> (u8, Vec) { let _ = o.insert("recovery", rec); (KIND_CONTROL, o.dump().into_bytes()) } - // Base64 like `Command::Input`: clipboard text is arbitrary child - // output and must cross the wire unsanitized. The emulator's 1 MiB - // store cap keeps the encoded frame far under `frame::MAX_FRAME` - // (see `CLIPBOARD_STORE_MAX_BYTES`'s const assertion). + // Base64 preserves arbitrary clipboard text in the JSON frame. Event::ClipboardCopy { id, kind, text } => { let mut o = jzon::JsonValue::new_object(); let _ = o.insert("t", "clip"); @@ -1032,8 +1013,7 @@ pub fn decode_event(kind: u8, payload: &[u8]) -> Option { "s" => ClipboardKind::Selection, _ => return None, }; - // The store was UTF-8 when the emulator captured it; a - // frame that decodes to anything else is malformed. + // Reject clipboard payloads that are not valid UTF-8. let text = String::from_utf8(B64.decode(v["text"].as_str()?).ok()?).ok()?; Some(Event::ClipboardCopy { id, kind, text }) } @@ -1756,10 +1736,7 @@ mod tests { ); } - /// `Watch` pins its wire shape and requires the `attached` flag: a frame - /// without it (an older client) or with a non-boolean value is rejected - /// whole. The flag gates clipboard forwarding, so the daemon never - /// defaults it. + /// Watch frames require a Boolean `attached` field. #[test] fn watch_wire_form_requires_the_attached_flag() { let (k, p) = encode_command(&Command::Watch { @@ -1838,10 +1815,7 @@ mod tests { assert_eq!(decode_event(k, &p), Some(screen)); } - /// `ClipboardCopy` round-trips all three kinds and carries clipboard text - /// unsanitized: unicode, control characters, and embedded markers survive - /// the JSON+base64 path byte-for-byte. The source id survives too, - /// including values past `u32`. + /// Clipboard events preserve their source, target, and text. #[test] fn clipboard_copy_round_trips() { for ev in [ @@ -1863,9 +1837,7 @@ mod tests { Event::ClipboardCopy { id: 7, kind: ClipboardKind::Clipboard, - // Clipboard content is arbitrary: newlines, tabs, NUL, ESC, - // and paste-marker-shaped text must not be sanitized in - // transit. + // Exercise control characters and paste-marker-shaped text. text: "line1\nline2\tcol\u{0}\u{1b}[201~end".into(), }, ] { @@ -1875,10 +1847,7 @@ mod tests { } } - /// `ClipboardCopy` pins its wire shape: `t` discriminator, the source - /// task under `id`, the OSC 52 kind byte under `k`, and base64 text. - /// All three kind strings are pinned — `"p"` in particular, so the - /// primary selection can never silently re-fold into `"s"`. + /// Clipboard events encode every target with its OSC 52 selector. #[test] fn clipboard_copy_wire_form() { for (kind, wire) in [ @@ -1905,9 +1874,7 @@ mod tests { } } - /// A malformed `ClipboardCopy` frame is rejected whole: missing fields, - /// an unknown kind string, a non-numeric id, invalid base64, and - /// non-UTF-8 decoded bytes. + /// Malformed clipboard event frames are rejected. #[test] fn malformed_clipboard_copy_is_rejected() { for json in [ @@ -1915,10 +1882,8 @@ mod tests { r#"{"t":"clip","id":1,"k":"c"}"#, // missing text r#"{"t":"clip","k":"c","text":"aGk="}"#, // missing id r#"{"t":"clip","id":"1","k":"c","text":"aGk="}"#, // id must be a number - // "x" here, not "p": "p" joined the accept set when the primary - // selection got its own kind. r#"{"t":"clip","id":1,"k":"x","text":"aGk="}"#, // unknown kind string - r#"{"t":"clip","id":1,"k":"c","text":"!!!"}"#, // invalid base64 + r#"{"t":"clip","id":1,"k":"c","text":"!!!"}"#, // invalid base64 r#"{"t":"clip","id":1,"k":"c","text":"/w=="}"#, // 0xFF: not UTF-8 r#"{"t":"clip","id":1,"k":"c","text":["aGk="]}"#, // text must be a string ] { diff --git a/src/supervisor.rs b/src/supervisor.rs index df2ec90..7938f24 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -122,10 +122,7 @@ fn normalize_group(name: Option) -> Option { normalize_label(name).filter(|g| g != "Unassigned") } -/// Map the emulator's clipboard selector to its wire mirror. The boundary -/// where terminal-module types stop: `protocol` deliberately imports none. -/// Three arms, verbatim — folding any pair here would recreate the fidelity -/// loss this mapping exists to prevent. +/// Map an emulator clipboard selector to its protocol representation. fn clipboard_kind(kind: ClipboardSelector) -> ClipboardKind { match kind { ClipboardSelector::Clipboard => ClipboardKind::Clipboard, @@ -238,11 +235,7 @@ pub struct Supervisor { scrollback: usize, /// The task whose screen the client is watching (attach/peek), or `None`. watched: Option, - /// Whether the current watch is an attach rather than a peek. Attachment - /// is the consent proxy for clipboard forwarding — input flows to the - /// child only then — so `tick` forwards OSC 52 stores only while this is - /// set. Screen streaming ignores it: peek needs screens. Meaningful only - /// while `watched` is `Some`. + /// Whether the current watch permits clipboard forwarding. watch_attached: bool, /// The last emitted screen fingerprint. `lines` stays empty because only /// emitted copies carry them. Cleared when `watched` changes to force a @@ -411,9 +404,7 @@ impl Supervisor { } } Command::Watch { id, attached } => { - // Reset the previous task's viewport only when the target - // itself changes: a peek→attach on the same task must keep - // the user's scrollback position. + // Preserve the viewport when only the attachment mode changes. if id != self.watched && let Some(old) = self.watched && let Some(t) = self.by_id_mut(old) @@ -421,16 +412,7 @@ impl Supervisor { t.scroll_view(ScrollAction::Live); } if id != self.watched || attached != self.watch_attached { - // Purge the new target's buffered stores before the watch - // takes effect. The wake loop applies a whole burst before - // ticking, so without this a store captured while - // backgrounded — or during a peek of this same task, the - // peek→attach case — survives into a tick that already - // sees an attach-watch, and fires. Purging on the - // attach→peek edge too is harmless: peek forwards - // nothing. The residual window (bytes emitted pre-attach - // but parsed post-purge) is irreducible: a transparent - // terminal has it too. + // Discard stores captured before this watch state took effect. if let Some(new) = id && let Some(t) = self.by_id_mut(new) { @@ -561,8 +543,7 @@ impl Supervisor { // least every 200 ms), so an expired sync flushes here, before the // preview resolution reads the grid, letting the same tick ship it. // Resolution mutates per-task hold state; all tasks use one timestamp. - // Forward stores only for an attach-watch: peek forwards no input to - // the child, so nothing captured during peek can be user-caused. + // Only the attached watch may forward clipboard stores. let forwarding = if self.watch_attached { self.watched } else { @@ -574,15 +555,7 @@ impl Supervisor { .iter_mut() .map(|t| { t.flush_expired_sync(); - // Drain every task's clipboard every tick and forward only the - // attach-watched task's. Dropping the others here is the - // staleness guarantee: a store captured while backgrounded or - // peeked must never fire when the task is later attached — a - // wrong clipboard is silently harmful, an empty one visibly - // inert. The three-slot capture bound makes the constant drain - // cheap. This drain alone cannot close the wake-coalescing - // race (a `Watch` in the same burst lands before the tick); - // `apply`'s `Watch` arm purges the new target for that case. + // Drain all tasks so stores from inactive tasks cannot be forwarded later. let stores = t.drain_clipboard(); if forwarding == Some(t.id) { clipboard = Some((t.id, stores)); diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index c38501d..111d9eb 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -222,17 +222,13 @@ fn tick_flushes_a_stalled_sync_update() { ); } -/// OSC 52 stores from the watched task reach `drain` as decoded -/// `ClipboardCopy` events, one per store in arrival order. The emission is -/// flag-gated so `Watch` is installed before any store can arrive. +/// Stores from the attached task are forwarded in arrival order. #[test] fn watched_task_clipboard_stores_are_forwarded() { let dir = scratch("clip_fwd"); let ready = dir.join("ready"); let flag = dir.join("flag"); let mut s = sup(24, 80); - // "aGVsbG8=" is "hello" (clipboard), "cHJp" is "pri" (primary), - // "d29ybGQ=" is "world" (select). let cmd = format!( "touch {r}; until [ -e {f} ]; do sleep 0.05; done; \ printf '\\033]52;c;aGVsbG8=\\007\\033]52;p;cHJp\\007\\033]52;s;d29ybGQ=\\007'; sleep 30", @@ -256,8 +252,7 @@ fn watched_task_clipboard_stores_are_forwarded() { copies.len() >= 3 }); assert!(ok, "the clipboard stores never arrived; got {copies:?}"); - // Three kinds in one burst, each under its own raw selector: a `p` - // store forwards as `Primary`, never re-folded into `Selection`. + // Each selector is forwarded as its matching protocol kind. assert_eq!( copies, vec![ @@ -269,19 +264,11 @@ fn watched_task_clipboard_stores_are_forwarded() { let _ = std::fs::remove_dir_all(&dir); } -/// A store captured before its task is watched must not fire once `Watch` -/// lands. The per-tick drain cannot cover this: the wake loop applies a -/// whole command burst before ticking, so a `Watch` in the burst makes the -/// next drain see the task as already watched. The test reproduces that -/// interleaving exactly — capture, then `Watch`, with no tick between — -/// which only `apply`'s watch-time purge can close. The marker is observed -/// through a non-draining grid read; a tick here would drain (and discard) -/// the buffer and mask the race. +/// Starting a watch discards stores captured before the watch. #[test] fn watch_purges_stores_captured_before_the_watch() { let mut s = sup(24, 80); - // "c3RhbGU=" is "stale". The trailing marker proves the store's bytes - // were parsed: it follows them in the output stream. + // The marker follows the store and confirms that both were parsed. spawn( &mut s, "printf '\\033]52;c;c3RhbGU=\\007MARKER'; sleep 30", @@ -316,15 +303,11 @@ fn watch_purges_stores_captured_before_the_watch() { assert!(saw_screen, "watching the task should stream its screen"); } -/// A store captured while the task is not watched is discarded by the -/// per-tick drain, never deferred: watching the task afterwards forwards -/// nothing. Staleness is worse than loss — a wrong clipboard is silently -/// harmful, an empty one visibly inert. +/// Stores from an unwatched task are discarded instead of deferred. #[test] fn backgrounded_clipboard_store_is_discarded_not_deferred() { let mut s = sup(24, 80); - // "c3RhbGU=" is "stale". The trailing marker proves the store's bytes - // were parsed: it follows them in the output stream. + // The marker follows the store and confirms that both were parsed. spawn( &mut s, "printf '\\033]52;c;c3RhbGU=\\007COPIED'; sleep 30", @@ -351,9 +334,7 @@ fn backgrounded_clipboard_store_is_discarded_not_deferred() { seen }); assert!(parsed, "the marker never reached the grid"); - // The marker only proves the store was parsed by that tick's preview - // resolution, which runs after the drain; one more tick guarantees a - // drain after capture. + // Run one more tick to drain a store parsed after the preceding drain. s.tick(); let _ = s.drain(); @@ -377,15 +358,7 @@ fn backgrounded_clipboard_store_is_discarded_not_deferred() { assert!(saw_screen, "watching the task should stream its screen"); } -/// Peek is clipboard-inert end to end, and consent is not retroactive. Three -/// phases against one task: a store drained while peek-watched never -/// forwards (though the peek's screen keeps streaming); a store captured -/// during peek and still buffered when the attach-watch lands — the -/// peek→attach straddle, with no tick between, exactly how the wake loop -/// applies a burst — dies in the watch-time purge instead of firing under -/// the new attach; a store emitted under the attach-watch forwards. Markers -/// are observed through non-draining grid reads; a tick while waiting would -/// drain the buffer and mask both races. +/// Peeked stores are discarded, while stores captured after attachment forward. #[test] fn peeked_stores_never_forward_and_die_at_the_attach_transition() { let dir = scratch("clip_peek"); @@ -394,9 +367,7 @@ fn peeked_stores_never_forward_and_die_at_the_attach_transition() { let flag2 = dir.join("flag2"); let flag3 = dir.join("flag3"); let mut s = sup(24, 80); - // "cGVlazE=" is "peek1", "cGVlazI=" is "peek2", "cG9zdA==" is "post". - // Each marker follows its store in the output stream, proving the - // store's bytes were parsed by the time the marker is visible. + // Each marker follows its store and confirms that both were parsed. let cmd = format!( "touch {r}; until [ -e {f1} ]; do sleep 0.05; done; \ printf '\\033]52;c;cGVlazE=\\007M1'; \ @@ -415,7 +386,7 @@ fn peeked_stores_never_forward_and_die_at_the_attach_transition() { attached: false, }); - // Phase 1: drained while peeked, never forwarded. + // A store drained during peek is not forwarded. std::fs::write(&flag1, b"").unwrap(); let parsed = wait_until(Duration::from_secs(5), || { s.tasks.first().is_some_and(|t| { @@ -442,9 +413,7 @@ fn peeked_stores_never_forward_and_die_at_the_attach_transition() { "peeking the task should still stream its screen" ); - // Phase 2: the straddle. The store sits in the buffer across the - // peek→attach on the same id; only the purge on the kind change stops - // the next (attached) tick from forwarding it. + // A buffered peek store is discarded when the same task becomes attached. std::fs::write(&flag2, b"").unwrap(); let parsed = wait_until(Duration::from_secs(5), || { s.tasks.first().is_some_and(|t| { @@ -466,8 +435,7 @@ fn peeked_stores_never_forward_and_die_at_the_attach_transition() { } } - // Phase 3: a store emitted under the attach-watch forwards, and it is - // the only one that ever does. + // A store captured after attachment is forwarded. std::fs::write(&flag3, b"").unwrap(); let mut copies = Vec::new(); let ok = wait_until(Duration::from_secs(5), || { @@ -486,17 +454,14 @@ fn peeked_stores_never_forward_and_die_at_the_attach_transition() { let _ = std::fs::remove_dir_all(&dir); } -/// An over-cap store on the watched task yields the drop notice and no -/// `ClipboardCopy`: the copy is lost loudly, not truncated or forwarded. +/// An oversized store produces a status notice instead of a clipboard event. #[test] fn oversized_watched_store_yields_notice_and_no_copy() { let dir = scratch("clip_oversize"); let ready = dir.join("ready"); let flag = dir.join("flag"); let mut s = sup(24, 80); - // 3 MiB decoded exceeds the 1 MiB cap. The child generates the base64 - // itself because `MAX_COMMAND_LEN` cannot carry the payload inline; - // `tr` strips GNU base64's line wrapping (macOS emits none). + // Generate a 3 MiB decoded payload without placing it in the command string. let cmd = format!( "touch {r}; until [ -e {f} ]; do sleep 0.05; done; \ printf '\\033]52;c;'; \ diff --git a/src/task.rs b/src/task.rs index fe02e5d..adb3f39 100644 --- a/src/task.rs +++ b/src/task.rs @@ -485,10 +485,7 @@ impl Task { } } - /// Take the OSC 52 clipboard stores captured since the last drain (see - /// [`Emulator::drain_clipboard`]). The caller owns forwarding: a drain - /// whose result is dropped discards the stores, which is how a - /// non-watched task's copies are kept from ever firing later. + /// Drain OSC 52 clipboard stores captured by this task's emulator. pub fn drain_clipboard(&self) -> ClipboardStores { grid(&self.parser).drain_clipboard() } diff --git a/src/terminal/emulator.rs b/src/terminal/emulator.rs index 7554863..07cd07d 100644 --- a/src/terminal/emulator.rs +++ b/src/terminal/emulator.rs @@ -36,10 +36,7 @@ pub enum MouseProtocolEncoding { Sgr, } -/// Buffered-store cap in bytes for one OSC 52 clipboard payload. The cap -/// bounds forwarding-frame size and per-task memory, not what a host -/// clipboard could hold; an over-cap store is dropped and its length -/// recorded so the caller can surface a notice. +/// Maximum decoded size of one buffered OSC 52 clipboard payload. pub(crate) const CLIPBOARD_STORE_MAX_BYTES: usize = 1024 * 1024; // A maximum-size store expands to this base64 bound when re-encoded for @@ -50,11 +47,7 @@ const _: () = assert!( "CLIPBOARD_STORE_MAX_BYTES must base64-encode to under frame::MAX_FRAME" ); -/// Which OSC 52 target a store addressed, preserving the raw selector byte. -/// The backend's `ClipboardType` folds `p` and `s` into one variant; per -/// xterm ctlseqs they are distinct targets (the primary selection and the -/// select buffer), so [`ObservedTerm::clipboard_store`] captures the byte -/// before that fold can happen. +/// Supported OSC 52 clipboard targets. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ClipboardSelector { /// The system clipboard (selector byte `c`). @@ -65,25 +58,17 @@ pub enum ClipboardSelector { Select, } -/// OSC 52 clipboard stores captured since the last drain. A clipboard is -/// last-writer-wins by nature, so coalescing to one store per -/// [`ClipboardSelector`] between drains loses nothing, and the three-slot -/// bound means a never-drained background task cannot accumulate memory. +/// OSC 52 clipboard stores captured since the last drain. #[derive(Debug, Default)] pub struct ClipboardStores { - /// At most one store per [`ClipboardSelector`], ordered by the surviving - /// store's arrival. + /// Latest store per selector, ordered by arrival. pub stores: Vec<(ClipboardSelector, String)>, - /// Byte length of the most recent store dropped for exceeding - /// [`CLIPBOARD_STORE_MAX_BYTES`], kept so the caller can surface a - /// notice instead of silently losing the copy. + /// Byte length of the most recent oversized store. pub oversized_len: Option, } /// Buffers backend-generated PTY responses while the parser advances. Other -/// backend events are discarded; [`ObservedTerm`] captures titles directly -/// from parser events and OSC 52 stores never reach the backend (see -/// [`ObservedTerm`]'s `clipboard_store`). +/// events are discarded; [`ObservedTerm`] captures titles and clipboard stores. pub struct ProbeSink { responses: Arc>>, } @@ -223,9 +208,7 @@ pub struct Emulator { term: Term, parser: Processor, responses: Arc>>, - /// OSC 52 stores captured since the last drain. A plain field, not an - /// `Arc>`: stores are written by [`ObservedTerm`] during the - /// parse (like `alt`), never through the backend's event listener. + /// OSC 52 stores captured since the last drain. clipboard: ClipboardStores, /// Alt-screen and title facts, advanced at parser-event granularity by /// [`ObservedTerm`] during the parse itself. @@ -251,10 +234,7 @@ impl Emulator { .collect() } - /// Drain the OSC 52 clipboard stores captured since the last drain, - /// plus the oversized-drop record. The caller owns forwarding; a drain - /// whose result is dropped discards the stores. An empty capture costs - /// no allocation, so calling every tick for every task is fine. + /// Drain captured OSC 52 stores and the latest oversized-store length. pub fn drain_clipboard(&mut self) -> ClipboardStores { std::mem::take(&mut self.clipboard) } @@ -690,9 +670,7 @@ const TITLE_STACK_SHADOW_MAX: usize = 4096; /// `Handler` methods default to no-ops, so every method must delegate to /// `Term`. `golden::emulator_wrapper_matches_the_raw_backend_on_every_fixture` /// compares wrapper and raw-backend replays to detect missing delegation. -/// One deliberate exception: `clipboard_store` never delegates — the wrapper -/// owns that pipeline outright, and the backend's version touches no grid -/// state the golden comparison could see. +/// `clipboard_store` is captured by this wrapper instead of delegated. /// /// # Synchronized updates /// @@ -940,40 +918,21 @@ impl Handler for ObservedTerm<'_> { fn reset_color(&mut self, a0: usize) { self.term.reset_color(a0); } - /// The one non-delegating handler: the backend's `clipboard_store` folds - /// both `p` and `s` into `ClipboardType::Selection` before its event - /// fires, so delegating loses the raw selector — a `p` store would - /// re-emit as `s`, and a `p` and an `s` store in one batch would collide - /// in one slot. Owning the pipeline here keeps the byte. - /// - /// Scope bound: vte's OSC 52 dispatch passes only the FIRST selector - /// byte to this handler and defaults an empty selector to `c` - /// (vte 0.15.0 `ansi.rs`, `params[1].first().unwrap_or(&b'c')`), so a - /// multi-target selector like `pc` is first-byte-truncated before - /// anything below can see it. + /// Capture supported OSC 52 stores while preserving their selector. fn clipboard_store(&mut self, a0: u8, a1: &[u8]) { - // Selector policy is ours now, not the backend's: `c`, `p`, and `s` - // buffer under their own slots; anything else (`q`, the numbered cut - // buffers `0`-`7`) drops silently, matching the backend behavior - // this replaces. + // Ignore selectors without a forwarding target. let selector = match a0 { b'c' => ClipboardSelector::Clipboard, b'p' => ClipboardSelector::Primary, b's' => ClipboardSelector::Select, _ => return, }; - // Same decode contract as the backend: strict padded standard - // base64, then UTF-8. The `!` clear form and the `?` query never - // reach here as text — `?` dispatches to `clipboard_load`, and `!` - // fails the decode. + // Accept padded standard base64 containing UTF-8 text. let Ok(bytes) = B64.decode(a1) else { return }; let Ok(text) = String::from_utf8(bytes) else { return; }; - // Last writer wins per selector even when the last writer is - // over-cap: forwarding a superseded store would misrepresent the - // child's final clipboard state, so the drop clears its selector's - // slot and the recorded length feeds the caller's notice. + // Retain only the most recent value for this selector, including on overflow. self.clipboard.stores.retain(|(k, _)| *k != selector); if text.len() > CLIPBOARD_STORE_MAX_BYTES { self.clipboard.oversized_len = Some(text.len()); @@ -982,8 +941,7 @@ impl Handler for ObservedTerm<'_> { self.clipboard.stores.push((selector, text)); } fn clipboard_load(&mut self, a0: u8, a1: &str) { - // Still delegated: the backend's default `Osc52::OnlyCopy` denies - // loads, which is exactly the policy fleetcom wants. + // The configured terminal policy denies clipboard loads. self.term.clipboard_load(a0, a1); } fn decaln(&mut self) { @@ -1120,8 +1078,7 @@ mod tests { assert!(emu.process(b"\x1b[?u").is_empty()); } - /// End to end through `process`: an OSC 52 store is captured, produces - /// no probe reply, and drains exactly once. + /// An OSC 52 store is captured without a probe reply and drains once. #[test] fn osc52_store_is_captured_and_drains_once() { let mut emu = Emulator::new(4, 20, 0); @@ -1140,10 +1097,7 @@ mod tests { assert_eq!(again.oversized_len, None); } - /// The `p` and `s` selectors stay distinct: per xterm ctlseqs they are - /// different targets, and folding them (the backend's mapping this - /// pipeline replaced) would both mislabel a lone `p` store and let a - /// same-batch pair collide in one slot. + /// Primary and selection stores remain distinct. #[test] fn osc52_p_and_s_selectors_stay_distinct() { let mut emu = Emulator::new(4, 20, 0); @@ -1157,10 +1111,7 @@ mod tests { ); } - /// An empty selector arrives as `c`: vte's dispatch defaults it - /// (`params[1].first().unwrap_or(&b'c')`, vte 0.15.0) before the - /// handler runs, so this pins upstream behavior the pipeline depends - /// on, not a choice made here. + /// An empty OSC 52 selector targets the system clipboard. #[test] fn osc52_empty_selector_defaults_to_clipboard() { let mut emu = Emulator::new(4, 20, 0); @@ -1171,8 +1122,7 @@ mod tests { ); } - /// Selectors outside `c`/`p`/`s` drop silently: `q` and the numbered - /// cut buffers have no forwarding target. + /// Unsupported OSC 52 selectors are ignored. #[test] fn osc52_unknown_selectors_drop() { let mut emu = Emulator::new(4, 20, 0); @@ -1183,8 +1133,7 @@ mod tests { assert_eq!(drained.oversized_len, None); } - /// Two stores to the same kind before a drain coalesce: only the second - /// survives. + /// Only the latest store for each selector is retained. #[test] fn osc52_last_store_wins_per_kind() { let mut emu = Emulator::new(4, 20, 0); @@ -1195,8 +1144,7 @@ mod tests { ); } - /// Stores to different selectors buffer independently and drain in - /// arrival order; all three selectors in one batch survive. + /// Different selectors buffer independently and drain in arrival order. #[test] fn osc52_kinds_buffer_independently() { let mut emu = Emulator::new(4, 20, 0); @@ -1211,10 +1159,7 @@ mod tests { ); } - /// Invalid base64 (including the `!` clear form) and non-UTF-8 payloads - /// die inside this pipeline's own decode — the backend no longer stands - /// in front of it — so these rejections are load-bearing here: nothing - /// buffers. + /// Invalid base64, clear requests, and non-UTF-8 payloads are ignored. #[test] fn osc52_invalid_base64_and_clear_buffer_nothing() { let mut emu = Emulator::new(4, 20, 0); @@ -1227,9 +1172,7 @@ mod tests { assert_eq!(drained.oversized_len, None); } - /// The query form is an OSC 52 load, which the backend's default - /// `Osc52::OnlyCopy` denies: nothing buffers and no reply is generated - /// for the child. + /// OSC 52 clipboard queries are denied without a reply. #[test] fn osc52_query_is_denied_without_a_reply() { let mut emu = Emulator::new(4, 20, 0); @@ -1239,7 +1182,7 @@ mod tests { assert_eq!(drained.oversized_len, None); } - /// vte accepts ST as the OSC terminator alongside BEL. + /// ST-terminated OSC 52 stores are captured. #[test] fn osc52_st_terminated_store_is_captured() { let mut emu = Emulator::new(4, 20, 0); @@ -1250,18 +1193,14 @@ mod tests { ); } - /// An over-cap store is not buffered, but it still supersedes its - /// selector's slot: forwarding the older store would misrepresent the - /// child's final clipboard state. The recorded length feeds the - /// caller's notice; other selectors are untouched. + /// An oversized store clears its selector and records its length. #[test] fn osc52_oversized_store_supersedes_its_kind_and_records_length() { let mut emu = Emulator::new(4, 20, 0); emu.process(b"\x1b]52;c;aGVsbG8=\x07"); emu.process(b"\x1b]52;s;c2Vs\x07"); emu.process(b"\x1b]52;p;cHJp\x07"); - // "YWFh" decodes to "aaa"; this repeat count decodes to two bytes - // over the cap. + // This payload decodes to two bytes over the cap. let reps = CLIPBOARD_STORE_MAX_BYTES / 3 + 1; let payload = "YWFh".repeat(reps); emu.process(format!("\x1b]52;c;{payload}\x07").as_bytes()); diff --git a/src/ui.rs b/src/ui.rs index e2de4fd..0cb78b0 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -181,8 +181,7 @@ fn render_dashboard(out: &mut impl Write, app: &App) -> io::Result<()> { y += 1; } - // Command line: input modes show a prompt (with cursor); otherwise the - // ephemeral notice or persistent status, else the key hint. + // Input modes show a prompt; otherwise show a notice, status, or key hint. let cmd_y = rows.saturating_sub(2); match cmdline(app) { Some((line, _)) => put(out, cmd_y, &line, cols)?, @@ -225,8 +224,7 @@ fn render_dashboard(out: &mut impl Write, app: &App) -> io::Result<()> { Ok(()) } -/// The command row's transient text: an active ephemeral notice beats the -/// persistent status, so a copy landing just before a detach is still seen. +/// Prefer an active notice over persistent status text. fn transient_line(notice: Option<&str>, status: Option<&str>) -> Option { notice.or(status).map(|s| format!(" {s}")) } @@ -722,11 +720,7 @@ fn render_attached(out: &mut impl Write, app: &App) -> io::Result<()> { Ok(()) } -/// The attached bottom bar, titled by the live/scrollback state. In the live -/// view an active notice replaces the background hint; the bar is rebuilt -/// every frame, so the hint returns the moment the notice expires. The -/// scrollback bar never yields its hints: those keys are how the user gets -/// back out. +/// Build the attached or scrollback bar, showing notices only in live view. fn attached_bar(title: &str, scrollback: usize, notice: Option<&str>) -> String { match (scrollback, notice) { (0, Some(n)) => format!(" [attached] {title} {n}"), @@ -987,8 +981,7 @@ mod tests { ); } - /// The live bar swaps its background hint for an active notice; the - /// scrollback bar keeps its navigation hints regardless. + /// The live bar shows notices while the scrollback bar keeps its key hints. #[test] fn attached_bar_swaps_the_hint_for_an_active_notice() { assert_eq!( @@ -1005,8 +998,7 @@ mod tests { ); } - /// The dashboard command row prefers an active notice over the - /// persistent status. + /// The dashboard command row prefers an active notice over status text. #[test] fn transient_line_prefers_the_notice_over_the_status() { assert_eq!(