From 9900781db19e4ea97abec971cdaa827850e9917d Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 15:06:29 -0400 Subject: [PATCH 1/8] feat(terminal): answer OSC color queries and track dynamic colors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Programs ask the terminal what color it is before deciding whether to draw for a light or a dark background. Nothing answered, so every one of them blocked until it timed out and guessed. The emulator answers now, through `take_pending_writes`, which already exists for exactly this: the replies a terminal owes to device queries. alacritty parses the sequence, tracks what a program set, and hands back a formatter with the query's own prefix and terminator already captured, so the only missing piece was the color itself. That comes from the session profile, which the emulator is now constructed with. The alternative was parsing the sequences off the PTY stream, the way shell integration is tracked. That would have meant reimplementing color parsing, the runtime table, reply formatting, and terminator tracking, all of which the emulator already does — and getting the terminator wrong, since it is only visible to whoever parsed the sequence. Reading what the emulator already knows is both less code and more faithful. Colors resolve in three layers: what a program set, else the session profile, else the table the specification defines. A reset clears only the first, so the profile is unreachable from the byte stream and there is always something to restore. That is what the specification asks for, describing a reset as restoring "the color specified by the corresponding X resource". `Emulator` gains `color(slot)`, which every backend answers from its own state, plus a `palette()` snapshot of all 259 slots. The screenshot renderer and `expect --fg/--bg` take that snapshot rather than the emulator, so neither holds the session lock while it renders, and neither knows which backend produced the colors. Five conformance cases cover queries, terminator echo, set-then-reset, unconfigured indices, and that a cell follows whatever its slot now holds. They run against every backend, so a future one cannot answer differently. An end-to-end test drives a real program through the whole path: it reads the configured background, sets its own, resets, and gets the configured one back. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- SKILL.md | 6 + crates/shell-use-cli/src/monitor.rs | 2 +- .../shell-use-cli/tests/session_lifecycle.rs | 74 +++++++++ crates/shell-use/src/assert/color.rs | 35 +++- crates/shell-use/src/engine.rs | 12 +- crates/shell-use/src/profile.rs | 46 ++++++ crates/shell-use/src/render/svg.rs | 36 ++-- crates/shell-use/src/session.rs | 2 +- crates/shell-use/src/terminal/alacritty.rs | 95 ++++++++++- crates/shell-use/src/terminal/conformance.rs | 156 +++++++++++++++++- crates/shell-use/src/terminal/emu.rs | 37 +++++ 11 files changed, 459 insertions(+), 42 deletions(-) diff --git a/SKILL.md b/SKILL.md index fc185db..c82e0cc 100644 --- a/SKILL.md +++ b/SKILL.md @@ -321,6 +321,12 @@ stable across profiles. The palette is what a screenshot paints **and** what `expect --fg/--bg` matches a `#rrggbb` against, so the two always agree. +Programs can also set and query colours at runtime with `OSC 4/10/11/12` and +reset them with `OSC 104/110/111/112`. A query is answered with the colour +currently showing; a reset restores the profile's colour, which no escape +sequence can change. Note that a program setting a colour also changes what a +screenshot of that session looks like. + ## Supported shells & integration `open --shell S` accepts: `bash`, `zsh`, `fish`, `powershell`, `pwsh`, `cmd`, diff --git a/crates/shell-use-cli/src/monitor.rs b/crates/shell-use-cli/src/monitor.rs index 4fb14c8..e08b985 100644 --- a/crates/shell-use-cli/src/monitor.rs +++ b/crates/shell-use-cli/src/monitor.rs @@ -395,7 +395,7 @@ mod tests { ]; for want in styles { - let mut emu = AlacrittyEmu::new(10, 2, 0); + let mut emu = AlacrittyEmu::new(10, 2, &shell_use::profile::Profile::default()); emu.process(want.sgr().as_bytes()); emu.process(b"x"); let got = Style::from(&emu.viewable_rows()[0][0]); diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index 67e95e7..8ff72fb 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -343,6 +343,80 @@ fn an_unknown_profile_is_rejected() { ); } +/// A program that asks the terminal what color it is gets an answer. +/// +/// This is how tools decide whether they are on a light or a dark background. +/// A terminal that stays silent leaves them blocked until they time out and +/// guess, so this drives the whole path: daemon, emulator, and the reply on +/// its way back up the PTY. +#[test] +fn a_color_query_is_answered_over_the_pty() { + let sandbox = Sandbox::new("osc-query"); + let probe = sandbox.home.join("probe.py"); + std::fs::write( + &probe, + r#" +import os, sys, termios, tty, select + +# Unbuffered reads: a buffered reader would take bytes off the fd that +# select() then cannot see, and the reply would look truncated. +def ask(fd, query): + os.write(1, query) + buf = b"" + while select.select([fd], [], [], 2.0)[0]: + buf += os.read(fd, 64) + if buf.endswith(b"\x07"): + break + return buf.decode("utf8", "replace") + +fd = sys.stdin.fileno() +old = termios.tcgetattr(fd) +try: + tty.setraw(fd) + configured = ask(fd, b"\x1b]11;?\x07") + os.write(1, b"\x1b]11;#654321\x07") + overridden = ask(fd, b"\x1b]11;?\x07") + os.write(1, b"\x1b]111\x07") + restored = ask(fd, b"\x1b]11;?\x07") +finally: + termios.tcsetattr(fd, termios.TCSADRAIN, old) + +strip = lambda s: s.replace("\x1b", "").replace("\x07", "") +print("\r\nRESULT %s %s %s\r" % (strip(configured), strip(overridden), strip(restored))) +"#, + ) + .expect("write probe"); + + sandbox.ok(&["run", "--cols", "80", "--", "bash", "--norc"]); + sandbox.ok(&[ + "submit", + &format!("python3 {}", probe.to_str().expect("utf-8 path")), + ]); + sandbox.ok(&["wait", "command"]); + let text = sandbox.ok(&["text", "--full"]); + + let line = text + .lines() + .find(|l| l.contains("RESULT")) + .unwrap_or_else(|| panic!("the probe never reported: {text}")); + + // The default profile's background is black, so the terminal reports it, + // then the color the program set, then the configured one again. + assert!( + line.contains("]11;rgb:0000/0000/0000"), + "the configured background should be reported: {line}" + ); + assert!( + line.contains("]11;rgb:6565/4343/2121"), + "a set color should be reported back: {line}" + ); + assert_eq!( + line.matches("]11;rgb:0000/0000/0000").count(), + 2, + "a reset should restore the configured background: {line}" + ); +} + #[test] fn state_reports_effective_timeouts() { let sandbox = Sandbox::new("state-timeouts"); diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 56105ef..040788c 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -1,7 +1,7 @@ //! Color parsing and comparison for `expect --fg/--bg`. use super::super::terminal::cell::Color; -use crate::profile::Colors; +use crate::profile::Palette; /// The spelling of [`Expected::Default`], on the command line and in messages. pub const DEFAULT: &str = "default"; @@ -74,7 +74,7 @@ fn parse_hex(hex: &str) -> anyhow::Result<(u8, u8, u8)> { /// the screenshot renderer draws with. These used to be two separate hardcoded /// tables that disagreed on every ANSI slot, so `expect --fg "#800000"` passed /// on a cell a screenshot painted `#e88388`. -pub fn matches(cell: Option, expected: &Expected, colors: &Colors) -> bool { +pub fn matches(cell: Option, expected: &Expected, colors: &Palette) -> bool { let Some(cell) = cell else { return matches!(expected, Expected::Default); }; @@ -89,7 +89,7 @@ pub fn matches(cell: Option, expected: &Expected, colors: &Colors) -> boo } /// Render a cell's color in the same space as the expected value, for messages. -pub fn describe_cell(cell: Option, expected: &Expected, colors: &Colors) -> String { +pub fn describe_cell(cell: Option, expected: &Expected, colors: &Palette) -> String { let Some(cell) = cell else { return DEFAULT.to_string(); }; @@ -124,7 +124,24 @@ pub fn rgb_to_ansi256(r: u8, g: u8, b: u8) -> u8 { #[cfg(test)] mod tests { use super::*; + use crate::profile::{Colors, Profile}; + use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::cell::Color; + use crate::terminal::emu::Emulator; + + /// Snapshotted from a real emulator, so these exercise the same path a + /// session uses rather than a stand-in that could drift from it. + fn emu_with(colors: Colors) -> Palette { + AlacrittyEmu::new( + 10, + 2, + &Profile { + colors, + ..Default::default() + }, + ) + .palette() + } #[test] fn parse_forms() { @@ -144,7 +161,7 @@ mod tests { #[test] fn matches_palette_and_default() { - let c = Colors::default(); + let c = emu_with(Colors::default()); let idx = |i| Some(Color::from_index(i)); assert!(matches(idx(9), &Expected::Ansi256(9), &c)); assert!(!matches(idx(2), &Expected::Ansi256(9), &c)); @@ -161,7 +178,7 @@ mod tests { /// `default` keyword, which is the way to assert on it. #[test] fn default_color_matches_only_default() { - let c = Colors::default(); + let c = emu_with(Colors::default()); assert!(!matches(None, &Expected::Ansi256(0), &c)); assert!(!matches(None, &Expected::Hex(0, 0, 0), &c)); assert!(matches(None, &Expected::Default, &c)); @@ -170,7 +187,7 @@ mod tests { #[test] fn a_colored_cell_is_not_default() { - let c = Colors::default(); + let c = emu_with(Colors::default()); let red = Some(Color::from_index(1)); assert!(!matches(red, &Expected::Default, &c)); assert!(matches(red, &Expected::Ansi256(1), &c)); @@ -190,7 +207,7 @@ mod tests { /// value for every slot, because both come from the profile. #[test] fn an_assertion_matches_the_color_a_screenshot_paints() { - let colors = Colors::default(); + let colors = emu_with(Colors::default()); for index in 0u8..=255 { let cell = Some(Color::from_index(index)); let painted = colors.resolve(cell, true); @@ -210,10 +227,10 @@ mod tests { /// profiles genuinely disagree rather than sharing one hardcoded table. #[test] fn a_recolored_profile_moves_what_an_assertion_matches() { - let colors = Colors { + let colors = emu_with(Colors { red: crate::profile::Rgb::new(1, 2, 3), ..Default::default() - }; + }); let red = Some(Color::from_index(1)); assert!(matches(red, &Expected::Hex(1, 2, 3), &colors)); assert!(!matches(red, &Expected::Hex(128, 0, 0), &colors)); diff --git a/crates/shell-use/src/engine.rs b/crates/shell-use/src/engine.rs index 34d55ff..403da33 100644 --- a/crates/shell-use/src/engine.rs +++ b/crates/shell-use/src/engine.rs @@ -284,6 +284,12 @@ fn await_ready(s: &Session, timeout_ms: u64) -> bool { } } +/// Snapshot the colors the session is currently showing. Taken before +/// rendering or asserting so neither holds the session lock while it works. +fn palette(s: &Session) -> crate::profile::Palette { + s.state.lock().unwrap().emu.palette() +} + fn viewable(s: &Session) -> Vec> { s.state.lock().unwrap().emu.viewable_rows() } @@ -727,7 +733,7 @@ fn expect_text( let ok = poll_until( || match locator::find(&grid(s, full), &pattern, strict) { Ok(Some(cells)) if !cells.is_empty() => { - if let Some(err) = check_colors(&cells, &fg, &bg, not, &s.profile.colors) { + if let Some(err) = check_colors(&cells, &fg, &bg, not, &palette(s)) { last_err = Some(err); false } else { @@ -757,7 +763,7 @@ fn check_colors( fg: &Option, bg: &Option, not: bool, - colors: &crate::profile::Colors, + colors: &crate::profile::Palette, ) -> Option { let want = !not; if let Some(spec) = fg { @@ -870,7 +876,7 @@ fn screenshot(s: &Session, full: bool, path: Option) -> Response { let rows = grid(s, full); match path { Some(path) => { - let svg = crate::render::svg::render_svg(&rows, s.cols, &s.profile.colors); + let svg = crate::render::svg::render_svg(&rows, s.cols, &palette(s)); match std::fs::write(&path, svg) { Ok(()) => Response::with(json!({ "path": path })), Err(e) => Response::internal(e.to_string()), diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs index ec98799..a2c9e00 100644 --- a/crates/shell-use/src/profile.rs +++ b/crates/shell-use/src/profile.rs @@ -236,6 +236,52 @@ impl Colors { } } +/// The colors a session is showing right now. +/// +/// A snapshot of every slot, taken from the emulator, so the screenshot +/// renderer and `expect --fg/--bg` can resolve a cell without holding the +/// session lock or knowing which backend produced it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Palette { + slots: [Rgb; 259], +} + +impl Palette { + pub fn new(slots: [Rgb; 259]) -> Self { + Palette { slots } + } + + pub fn color(&self, slot: usize) -> Rgb { + self.slots.get(slot).copied().unwrap_or(self.slots[256]) + } + + /// Resolve a cell's color, where `None` is the terminal default. The grid + /// records the slot a cell chose, never a color, so this is where a cell + /// becomes something to paint or compare. + pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { + match color { + None => self.color(if is_fg { 256 } else { 257 }), + Some(Color::Named(n)) => self.color(n.index() as usize), + Some(Color::Idx(i)) => self.color(i as usize), + Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), + } + } +} + +impl Default for Palette { + fn default() -> Self { + let config = Colors::default(); + let mut slots = [config.foreground; 259]; + for (i, slot) in slots.iter_mut().enumerate().take(256) { + *slot = config.rgb(i as u8); + } + slots[256] = config.foreground; + slots[257] = config.background; + slots[258] = config.cursor; + Palette { slots } + } +} + /// The settings a session runs with. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(default, deny_unknown_fields)] diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index ddce6a1..e88710c 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -11,7 +11,7 @@ use std::fmt::Write; use super::nerd_font::NerdFont; -use crate::profile::{Colors, Rgb}; +use crate::profile::{Palette, Rgb}; use crate::terminal::cell::{Attrs, EmuCell}; const CELL_W: f32 = 10.0; @@ -41,7 +41,7 @@ fn cell_at(row: &[EmuCell], x: usize) -> &EmuCell { } /// Resolved background color for a cell (honoring inverse). -fn bg_of(cell: &EmuCell, colors: &Colors) -> Rgb { +fn bg_of(cell: &EmuCell, colors: &Palette) -> Rgb { let bg = colors.resolve(cell.bg, false); let fg = colors.resolve(cell.fg, true); if cell.has(Attrs::INVERSE) { @@ -61,7 +61,7 @@ struct Style { invisible: bool, } -fn style_of(cell: &EmuCell, colors: &Colors) -> Style { +fn style_of(cell: &EmuCell, colors: &Palette) -> Style { let mut fg = colors.resolve(cell.fg, true); let bg = colors.resolve(cell.bg, false); if cell.has(Attrs::INVERSE) { @@ -102,7 +102,7 @@ fn run_text(row: &[EmuCell], start: usize, end: usize) -> String { } /// Render a grid to a standalone SVG document. -pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { +pub fn render_svg(rows: &[Vec], cols: u16, colors: &Palette) -> String { let nerd_font = NerdFont::new(rows, FONT_SIZE); let cols = cols as usize; let x0 = MARGIN_X; @@ -119,7 +119,7 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { let _ = write!( out, r#""#, - hex(colors.background) + hex(colors.resolve(None, false)) ); for (i, dot) in ["#ff5f56", "#ffbd2e", "#27c93f"].iter().enumerate() { let cx = MARGIN_X + 5.0 + i as f32 * 20.0; @@ -138,7 +138,7 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &Colors) -> String { while x + run < cols && bg_of(cell_at(row, x + run), colors) == bg { run += 1; } - if bg != colors.background { + if bg != colors.resolve(None, false) { let rx = x0 + x as f32 * CELL_W; let ry = y0 + y as f32 * CELL_H; let rw = run as f32 * CELL_W; @@ -217,6 +217,10 @@ mod tests { use super::*; use crate::terminal::cell::Color; + fn colors() -> Palette { + Palette::default() + } + fn cell(ch: &str, fg: Option, bg: Option) -> EmuCell { EmuCell { ch: ch.into(), @@ -232,12 +236,12 @@ mod tests { cell("h", Some(Color::from_index(1)), None), cell("i", Some(Color::from_index(1)), None), ]]; - let svg = render_svg(&rows, 2, &Colors::default()); + let svg = render_svg(&rows, 2, &colors()); assert!(svg.starts_with("")); assert!(svg.contains("textLength")); assert!( - svg.contains(&hex(Colors::default().rgb(1))), + svg.contains(&hex(colors().color(1))), "slot 1 is painted with the profile color" ); assert!(svg.contains(">hi")); @@ -247,7 +251,7 @@ mod tests { #[test] fn emits_window_chrome() { - let svg = render_svg(&[vec![cell(" ", None, None)]], 1, &Colors::default()); + let svg = render_svg(&[vec![cell(" ", None, None)]], 1, &colors()); assert!(svg.contains("<")); } @@ -272,9 +276,9 @@ mod tests { #[test] fn background_run_emitted_for_non_default_bg() { let rows = vec![vec![cell(" ", None, Some(Color::from_index(4)))]]; - let svg = render_svg(&rows, 1, &Colors::default()); + let svg = render_svg(&rows, 1, &colors()); assert!( - svg.contains(&hex(Colors::default().rgb(4))), + svg.contains(&hex(colors().color(4))), "slot 4 is painted with the profile color" ); } @@ -287,7 +291,7 @@ mod tests { cell(glyph, None, None), cell("b", None, None), ]]; - let svg = render_svg(&rows, 3, &Colors::default()); + let svg = render_svg(&rows, 3, &colors()); assert!(svg.contains(r#"")); diff --git a/crates/shell-use/src/session.rs b/crates/shell-use/src/session.rs index ea5c05a..c3379a1 100644 --- a/crates/shell-use/src/session.rs +++ b/crates/shell-use/src/session.rs @@ -80,7 +80,7 @@ impl Session { }; let state = Arc::new(Mutex::new(TermState { - emu: Box::new(AlacrittyEmu::new(cols, rows, profile.scrollback)), + emu: Box::new(AlacrittyEmu::new(cols, rows, &profile)), tracker: CommandTracker::new(), last_change: Instant::now(), awaiting_start: None, diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 47c41a6..6ac8d05 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -12,11 +12,13 @@ use alacritty_terminal::term::cell::Flags as AlacFlags; use alacritty_terminal::term::test::TermSize; use alacritty_terminal::term::{Config as AlacConfig, Term}; use alacritty_terminal::vte::ansi; +use alacritty_terminal::vte::ansi::Rgb as AlacRgb; use compact_str::{CompactString, ToCompactString}; +use crate::profile::{Colors, Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; -use crate::terminal::emu::Emulator; +use crate::terminal::emu::{self, Emulator}; /// Alacritty's palette colors arrive either as a `Named` variant or an index; /// both funnel through [`Color::from_index`] so a given slot always yields the @@ -112,17 +114,37 @@ fn cell_from_alac(c: &alacritty_terminal::term::cell::Cell) -> EmuCell { } } +/// Formats a color query's reply once the color is known. alacritty builds +/// this closure with the query's own prefix and terminator already captured, +/// so the answer echoes the form the program asked in. +type ReplyFormat = Arc String + Send + Sync>; + +/// Queues what the terminal wants to say back to the PTY. +/// +/// Color queries cannot be answered here: the color lives in the terminal's +/// own palette, and this listener is constructed before the terminal it +/// listens to. They are parked instead, and [`AlacrittyEmu::answer_queries`] +/// resolves them once `process` returns, where the terminal is in scope. #[derive(Default, Clone)] struct CaptureProxy { pending: Arc>>, + queries: Arc>>, } impl EventListener for CaptureProxy { fn send_event(&self, ev: Event) { - if let Event::PtyWrite(bytes) = ev { - if let Ok(mut buf) = self.pending.lock() { - buf.extend_from_slice(bytes.as_bytes()); + match ev { + Event::PtyWrite(bytes) => { + if let Ok(mut buf) = self.pending.lock() { + buf.extend_from_slice(bytes.as_bytes()); + } + } + Event::ColorRequest(slot, format) => { + if let Ok(mut queries) = self.queries.lock() { + queries.push((slot, format)); + } } + _ => {} } } } @@ -133,25 +155,62 @@ pub struct AlacrittyEmu { cols: u16, rows: u16, pending: Arc>>, + queries: Arc>>, + /// The colors this session was configured with. A program can shadow them + /// at runtime but never reach them, so a reset always has a value to + /// restore. + config: Colors, } impl AlacrittyEmu { - pub fn new(cols: u16, rows: u16, scrollback: usize) -> Self { + pub fn new(cols: u16, rows: u16, profile: &Profile) -> Self { let size = TermSize::new(cols as usize, rows as usize); - let config = AlacConfig { - scrolling_history: scrollback, + let alac_config = AlacConfig { + scrolling_history: profile.scrollback, ..Default::default() }; let pending: Arc>> = Arc::default(); + let queries: Arc>> = Arc::default(); let proxy = CaptureProxy { pending: pending.clone(), + queries: queries.clone(), }; AlacrittyEmu { - term: Term::new(config, &size, proxy), + term: Term::new(alac_config, &size, proxy), processor: ansi::Processor::new(), cols, rows, pending, + queries, + config: profile.colors, + } + } + + /// Answer any color queries parked while the last chunk was parsed. + /// + /// alacritty stores a color a program set, and leaves the slot empty + /// otherwise, so an empty slot is answered from the session profile. + fn answer_queries(&mut self) { + let parked: Vec<(usize, ReplyFormat)> = match self.queries.lock() { + Ok(mut queries) => queries.drain(..).collect(), + Err(_) => return, + }; + if parked.is_empty() { + return; + } + let replies: String = parked + .into_iter() + .map(|(slot, format)| { + let c = self.color(slot); + format(AlacRgb { + r: c.r, + g: c.g, + b: c.b, + }) + }) + .collect(); + if let Ok(mut buf) = self.pending.lock() { + buf.extend_from_slice(replies.as_bytes()); } } @@ -173,6 +232,9 @@ impl AlacrittyEmu { impl Emulator for AlacrittyEmu { fn process(&mut self, bytes: &[u8]) { self.processor.advance(&mut self.term, bytes); + // Queries are answered here rather than in the listener because the + // terminal holding the palette is only in scope once parsing is done. + self.answer_queries(); } fn take_pending_writes(&mut self) -> Vec { @@ -204,6 +266,21 @@ impl Emulator for AlacrittyEmu { self.rows_in_range(0, self.rows as i32) } + fn color(&self, slot: usize) -> Rgb { + // `Colors` stores only what a program set; an empty slot means the + // session's configured color still shows through. + if let Some(set) = self.term.colors()[slot] { + return Rgb::new(set.r, set.g, set.b); + } + match slot { + emu::FOREGROUND => self.config.foreground, + emu::BACKGROUND => self.config.background, + emu::CURSOR => self.config.cursor, + i if i < emu::FOREGROUND => self.config.rgb(i as u8), + _ => self.config.foreground, + } + } + fn full_rows(&self) -> Vec> { let grid = self.term.grid(); let total = grid.total_lines() as i32; @@ -217,5 +294,5 @@ impl Emulator for AlacrittyEmu { mod tests { use super::*; - crate::emulator_conformance_tests!(|c, r, s| Box::new(AlacrittyEmu::new(c, r, s))); + crate::emulator_conformance_tests!(|c, r, p| Box::new(AlacrittyEmu::new(c, r, p))); } diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index bee931a..6b5fc85 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -16,7 +16,7 @@ //! the test pins only the part that is universal and says why it stops short. /// Generates the conformance tests for one backend. `$make` builds a boxed -/// emulator from `(cols, rows, scrollback)`. +/// emulator from `(cols, rows, &Profile)`. /// /// The body is fully path-qualified because it expands into the caller's /// module; it must not collide with whatever that module already imports. @@ -28,8 +28,27 @@ macro_rules! emulator_conformance_tests { rows: u16, scrollback: usize, ) -> Box { - let make: fn(u16, u16, usize) -> Box = $make; - make(cols, rows, scrollback) + conformance_emu_with( + cols, + rows, + $crate::profile::Profile { + scrollback, + ..Default::default() + }, + ) + } + + fn conformance_emu_with( + cols: u16, + rows: u16, + profile: $crate::profile::Profile, + ) -> Box { + let make: fn( + u16, + u16, + &$crate::profile::Profile, + ) -> Box = $make; + make(cols, rows, &profile) } /// Row text with trailing blanks removed, for readable assertions. @@ -542,6 +561,137 @@ macro_rules! emulator_conformance_tests { ); } + /// A color query is answered with the session's configured color. + /// + /// Programs query the background to decide whether they are on a light + /// or a dark terminal. A backend that stays silent leaves them blocked + /// until they time out and guess. + #[test] + fn conformance_color_queries_are_answered() { + use $crate::profile::{Colors, Profile, Rgb}; + let profile = Profile { + colors: Colors { + background: Rgb::new(0x12, 0x34, 0x56), + red: Rgb::new(0xab, 0xcd, 0xef), + ..Default::default() + }, + ..Default::default() + }; + let mut e = conformance_emu_with(10, 4, profile); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]11;?\x07"); + assert_eq!( + String::from_utf8_lossy(&e.take_pending_writes()), + "\x1b]11;rgb:1212/3434/5656\x07", + "OSC 11 must report the configured background" + ); + + e.process(b"\x1b]4;1;?\x07"); + assert_eq!( + String::from_utf8_lossy(&e.take_pending_writes()), + "\x1b]4;1;rgb:abab/cdcd/efef\x07", + "OSC 4 must report the configured palette entry" + ); + } + + /// A reply uses the terminator the query used. A program that reads + /// until the terminator it sent would otherwise wait for one that + /// never comes. + #[test] + fn conformance_a_color_reply_echoes_the_terminator() { + let mut e = conformance_emu(10, 4, 100); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]11;?\x07"); + let bel = e.take_pending_writes(); + assert!( + bel.ends_with(b"\x07"), + "a BEL query is answered with BEL: {:?}", + String::from_utf8_lossy(&bel) + ); + + e.process(b"\x1b]11;?\x1b\\"); + let st = e.take_pending_writes(); + assert!( + st.ends_with(b"\x1b\\"), + "an ST query is answered with ST: {:?}", + String::from_utf8_lossy(&st) + ); + } + + /// A program can shadow a color, and a reset puts the configured one + /// back. The configured color is never reachable, so a reset always + /// has something to restore. + #[test] + fn conformance_a_color_set_is_undone_by_a_reset() { + use $crate::profile::{Colors, Profile, Rgb}; + let configured = Rgb::new(0x11, 0x22, 0x33); + let profile = Profile { + colors: Colors { + background: configured, + ..Default::default() + }, + ..Default::default() + }; + let mut e = conformance_emu_with(10, 4, profile); + let background = $crate::terminal::emu::BACKGROUND; + assert_eq!(e.color(background), configured); + + e.process(b"\x1b]11;#654321\x07"); + assert_eq!( + e.color(background), + Rgb::new(0x65, 0x43, 0x21), + "a set shadows the configured color" + ); + + e.process(b"\x1b]111\x07"); + assert_eq!( + e.color(background), + configured, + "OSC 111 restores the configured color" + ); + + // The same for a palette entry, which resets with OSC 104. + e.process(b"\x1b]4;2;#010203\x07"); + assert_eq!(e.color(2), Rgb::new(1, 2, 3)); + e.process(b"\x1b]104;2\x07"); + assert_eq!(e.color(2), Colors::default().green); + } + + /// An unconfigured palette entry still answers, from the table the + /// specification defines for it. + #[test] + fn conformance_an_unconfigured_index_resolves_from_the_spec_table() { + use $crate::profile::Rgb; + let e = conformance_emu(10, 4, 100); + assert_eq!( + e.color(196), + Rgb::new(255, 0, 0), + "index 196 is pure red in the xterm color cube" + ); + assert_eq!(e.color(232), Rgb::new(8, 8, 8), "the gray ramp starts at 8"); + } + + /// A cell records which slot it chose, never a color, so what it + /// paints follows whatever that slot currently holds. + #[test] + fn conformance_a_cell_follows_its_slot() { + use $crate::profile::Rgb; + let mut e = conformance_emu(10, 4, 100); + e.process(b"\x1b[31mR"); + let cell = e.viewable_rows()[0][0].clone(); + + let before = e.palette().resolve(cell.fg, true); + e.process(b"\x1b]4;1;#0a0b0c\x07"); + assert_eq!( + e.palette().resolve(cell.fg, true), + Rgb::new(0x0a, 0x0b, 0x0c), + "recoloring the slot recolors the cell that chose it" + ); + assert_ne!(before, e.palette().resolve(cell.fg, true)); + } + /// The alternate screen hides primary content and restores it on exit. #[test] fn conformance_alt_screen_round_trip() { diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index 83c0ba7..9da0c00 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -10,8 +10,18 @@ //! identical shell-integration behavior by construction rather than by //! reimplementation. +use crate::profile::{Palette, Rgb}; use crate::terminal::cell::EmuCell; +/// Runtime color slots: the 256-color palette, then the three dynamic colors. +/// +/// The numbering is not ours — both emulators already address their special +/// colors this way, so a backend can hand its own table straight through. +pub const FOREGROUND: usize = 256; +pub const BACKGROUND: usize = 257; +pub const CURSOR: usize = 258; +pub const COLOR_SLOTS: usize = 259; + /// A headless terminal emulator: bytes in, cell grid out. /// /// Implementations must be `Send`; the daemon shares the emulator across its @@ -40,4 +50,31 @@ pub trait Emulator: Send { /// Scrollback history followed by the visible screen. fn full_rows(&self) -> Vec>; + + /// The color a slot currently shows. + /// + /// Programs move these with `OSC 4` (palette) and `OSC 10/11/12` (default + /// foreground, background, cursor), and put them back with `OSC 104` and + /// `OSC 110/111/112`. A reset restores the color the session was configured + /// with; nothing a program sends can change that configured value, so + /// there is always something to fall back to. + /// + /// Backends answer color *queries* themselves, through + /// [`Emulator::take_pending_writes`], because each one already parses the + /// sequence and knows which terminator the query used. This method is how + /// the screenshot renderer and `expect --fg/--bg` see the same answer. + /// + /// `slot` is a palette index, or one of [`FOREGROUND`], [`BACKGROUND`], + /// [`CURSOR`]. + fn color(&self, slot: usize) -> Rgb; + + /// Every slot at once, so a consumer can resolve colors without holding + /// the session lock or knowing which backend produced them. + fn palette(&self) -> Palette { + let mut slots = [Rgb::new(0, 0, 0); COLOR_SLOTS]; + for (slot, out) in slots.iter_mut().enumerate() { + *out = self.color(slot); + } + Palette::new(slots) + } } From 915bc8b59ac1040d1d6fe9866d6eaf71b8cfade3 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 15:22:49 -0400 Subject: [PATCH 2/8] refactor(terminal): resolve colors in the emulator against its profile Follow-up to the previous commit, which routed color resolution through a `Palette` snapshot the session passed around. That put the fallback in the wrong place: the emulator reported only what a program had set, and every consumer had to know how to fill in the rest. The emulator now takes the session profile at construction and answers `color(slot)` for any slot, mapping its own table onto the profile when nothing has overridden it. Consumers ask the emulator and get a color, with no second layer to consult. `Palette` is gone. The 256-color table above the sixteen configurable slots is a static built at compile time rather than arithmetic run per lookup. It is the same in every terminal, so computing it repeatedly only invited the two implementations of it to drift. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/assert/color.rs | 13 +- crates/shell-use/src/engine.rs | 15 +- crates/shell-use/src/profile.rs | 211 ++++++++++++------- crates/shell-use/src/render/svg.rs | 17 +- crates/shell-use/src/terminal/alacritty.rs | 40 ++-- crates/shell-use/src/terminal/conformance.rs | 8 +- crates/shell-use/src/terminal/emu.rs | 52 ++--- 7 files changed, 200 insertions(+), 156 deletions(-) diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 040788c..5cf89ea 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -1,7 +1,7 @@ //! Color parsing and comparison for `expect --fg/--bg`. use super::super::terminal::cell::Color; -use crate::profile::Palette; +use crate::terminal::emu::Emulator; /// The spelling of [`Expected::Default`], on the command line and in messages. pub const DEFAULT: &str = "default"; @@ -74,7 +74,7 @@ fn parse_hex(hex: &str) -> anyhow::Result<(u8, u8, u8)> { /// the screenshot renderer draws with. These used to be two separate hardcoded /// tables that disagreed on every ANSI slot, so `expect --fg "#800000"` passed /// on a cell a screenshot painted `#e88388`. -pub fn matches(cell: Option, expected: &Expected, colors: &Palette) -> bool { +pub fn matches(cell: Option, expected: &Expected, colors: &dyn Emulator) -> bool { let Some(cell) = cell else { return matches!(expected, Expected::Default); }; @@ -89,7 +89,7 @@ pub fn matches(cell: Option, expected: &Expected, colors: &Palette) -> bo } /// Render a cell's color in the same space as the expected value, for messages. -pub fn describe_cell(cell: Option, expected: &Expected, colors: &Palette) -> String { +pub fn describe_cell(cell: Option, expected: &Expected, colors: &dyn Emulator) -> String { let Some(cell) = cell else { return DEFAULT.to_string(); }; @@ -129,9 +129,9 @@ mod tests { use crate::terminal::cell::Color; use crate::terminal::emu::Emulator; - /// Snapshotted from a real emulator, so these exercise the same path a - /// session uses rather than a stand-in that could drift from it. - fn emu_with(colors: Colors) -> Palette { + /// A real emulator, so these exercise the same resolution path a session + /// uses rather than a stand-in that could drift from it. + fn emu_with(colors: Colors) -> AlacrittyEmu { AlacrittyEmu::new( 10, 2, @@ -140,7 +140,6 @@ mod tests { ..Default::default() }, ) - .palette() } #[test] diff --git a/crates/shell-use/src/engine.rs b/crates/shell-use/src/engine.rs index 403da33..584b780 100644 --- a/crates/shell-use/src/engine.rs +++ b/crates/shell-use/src/engine.rs @@ -284,12 +284,6 @@ fn await_ready(s: &Session, timeout_ms: u64) -> bool { } } -/// Snapshot the colors the session is currently showing. Taken before -/// rendering or asserting so neither holds the session lock while it works. -fn palette(s: &Session) -> crate::profile::Palette { - s.state.lock().unwrap().emu.palette() -} - fn viewable(s: &Session) -> Vec> { s.state.lock().unwrap().emu.viewable_rows() } @@ -733,7 +727,9 @@ fn expect_text( let ok = poll_until( || match locator::find(&grid(s, full), &pattern, strict) { Ok(Some(cells)) if !cells.is_empty() => { - if let Some(err) = check_colors(&cells, &fg, &bg, not, &palette(s)) { + if let Some(err) = + check_colors(&cells, &fg, &bg, not, s.state.lock().unwrap().emu.as_ref()) + { last_err = Some(err); false } else { @@ -763,7 +759,7 @@ fn check_colors( fg: &Option, bg: &Option, not: bool, - colors: &crate::profile::Palette, + colors: &dyn crate::terminal::emu::Emulator, ) -> Option { let want = !not; if let Some(spec) = fg { @@ -876,7 +872,8 @@ fn screenshot(s: &Session, full: bool, path: Option) -> Response { let rows = grid(s, full); match path { Some(path) => { - let svg = crate::render::svg::render_svg(&rows, s.cols, &palette(s)); + let svg = + crate::render::svg::render_svg(&rows, s.cols, s.state.lock().unwrap().emu.as_ref()); match std::fs::write(&path, svg) { Ok(()) => Response::with(json!({ "path": path })), Err(e) => Response::internal(e.to_string()), diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs index a2c9e00..64b630e 100644 --- a/crates/shell-use/src/profile.rs +++ b/crates/shell-use/src/profile.rs @@ -22,7 +22,7 @@ use std::path::{Path, PathBuf}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use crate::terminal::cell::{Color, NamedColor}; +use crate::terminal::cell::NamedColor; /// Rows of scrollback a profile retains when it does not say otherwise. /// @@ -193,93 +193,99 @@ impl Colors { /// Resolve any 256-color index. /// - /// Slots 0-15 come from the profile. The color cube (16-231) and gray ramp - /// (232-255) are fixed by the xterm spec and identical under every profile. + /// Slots 0-15 come from the profile; everything above comes from the + /// xterm table, which no profile can move. pub fn rgb(&self, index: u8) -> Rgb { match index { 0..=15 => self.ansi()[index as usize], - 16..=231 => { - let i = index as u16 - 16; - let level = |c: u16| -> u8 { - if c == 0 { - 0 - } else { - (c * 40 + 55) as u8 - } - }; - Rgb::new(level((i / 36) % 6), level((i / 6) % 6), level(i % 6)) - } - 232..=255 => { - let v = ((index as u16 - 232) * 10 + 8) as u8; - Rgb::new(v, v, v) - } + _ => xterm_color(index), } } - /// Resolve a cell's color, where `None` is the terminal default. + /// The color a slot shows when no program has overridden it. /// - /// This is the one function both the screenshot renderer and `expect - /// --fg/--bg` call, which is what keeps them agreeing. - pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { - match color { - None => { - if is_fg { - self.foreground - } else { - self.background - } - } - Some(Color::Named(n)) => self.rgb(n.index()), - Some(Color::Idx(i)) => self.rgb(i), - Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), + /// This is what a backend falls back to: `slot` is a palette index, or one + /// of [`FOREGROUND`], [`BACKGROUND`], [`CURSOR`], matching how `OSC 4` and + /// `OSC 10/11/12` address them. + pub fn color(&self, slot: usize) -> Rgb { + match slot { + FOREGROUND => self.foreground, + BACKGROUND => self.background, + CURSOR => self.cursor, + index if index < FOREGROUND => self.rgb(index as u8), + // Above the addressable range there is nothing sensible to report; + // the foreground is the least surprising answer. + _ => self.foreground, } } } -/// The colors a session is showing right now. +/// Where the three dynamic colors sit when a color slot is addressed by +/// number. /// -/// A snapshot of every slot, taken from the emulator, so the screenshot -/// renderer and `expect --fg/--bg` can resolve a cell without holding the -/// session lock or knowing which backend produced it. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct Palette { - slots: [Rgb; 259], -} - -impl Palette { - pub fn new(slots: [Rgb; 259]) -> Self { - Palette { slots } - } - - pub fn color(&self, slot: usize) -> Rgb { - self.slots.get(slot).copied().unwrap_or(self.slots[256]) - } - - /// Resolve a cell's color, where `None` is the terminal default. The grid - /// records the slot a cell chose, never a color, so this is where a cell - /// becomes something to paint or compare. - pub fn resolve(&self, color: Option, is_fg: bool) -> Rgb { - match color { - None => self.color(if is_fg { 256 } else { 257 }), - Some(Color::Named(n)) => self.color(n.index() as usize), - Some(Color::Idx(i)) => self.color(i as usize), - Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), - } - } +/// The numbering is not ours: `OSC 4` addresses the 256-color palette, and +/// `OSC 10/11/12` address the three colors after it. Both emulators already +/// index their tables this way, so a backend reads its own state directly. +pub const FOREGROUND: usize = 256; +pub const BACKGROUND: usize = 257; +pub const CURSOR: usize = 258; + +/// The xterm 256-color table, which is the same in every terminal. +/// +/// Slots 0-15 here are the classic VGA colors, and a profile overrides them. +/// The rest is the 6x6x6 color cube and the 24-step gray ramp, which the +/// specification fixes and no profile can move: `--fg 196` has to mean the +/// same thing in every session. +static XTERM_256: [Rgb; 256] = build_xterm_256(); + +const fn build_xterm_256() -> [Rgb; 256] { + let mut table = [Rgb::new(0, 0, 0); 256]; + + // 0-15: VGA. + let vga = [ + (0, 0, 0), + (128, 0, 0), + (0, 128, 0), + (128, 128, 0), + (0, 0, 128), + (128, 0, 128), + (0, 128, 128), + (192, 192, 192), + (128, 128, 128), + (255, 0, 0), + (0, 255, 0), + (255, 255, 0), + (0, 0, 255), + (255, 0, 255), + (0, 255, 255), + (255, 255, 255), + ]; + let mut i = 0; + while i < 16 { + table[i] = Rgb::new(vga[i].0, vga[i].1, vga[i].2); + i += 1; + } + + // 16-231: a 6x6x6 cube whose levels step 0, 95, 135, 175, 215, 255. + let levels = [0u8, 95, 135, 175, 215, 255]; + while i < 232 { + let n = i - 16; + table[i] = Rgb::new(levels[(n / 36) % 6], levels[(n / 6) % 6], levels[n % 6]); + i += 1; + } + + // 232-255: a gray ramp from 8 to 238 in steps of 10. + while i < 256 { + let v = (i - 232) as u8 * 10 + 8; + table[i] = Rgb::new(v, v, v); + i += 1; + } + table } -impl Default for Palette { - fn default() -> Self { - let config = Colors::default(); - let mut slots = [config.foreground; 259]; - for (i, slot) in slots.iter_mut().enumerate().take(256) { - *slot = config.rgb(i as u8); - } - slots[256] = config.foreground; - slots[257] = config.background; - slots[258] = config.cursor; - Palette { slots } - } +/// The color a slot has when nothing has overridden it. +pub fn xterm_color(index: u8) -> Rgb { + XTERM_256[index as usize] } /// The settings a session runs with. @@ -494,15 +500,60 @@ mod tests { assert_eq!(Colors::slot_name(16), None, "only 0-15 are configurable"); } + /// Every slot a program can address resolves, so a backend always has a + /// color to fall back to and a query always has an answer. #[test] - fn a_cell_that_set_no_color_takes_the_profile_default() { + fn every_addressable_slot_resolves() { let c = Colors::default(); - assert_eq!(c.resolve(None, true), c.foreground); - assert_eq!(c.resolve(None, false), c.background); + for index in 0u8..=255 { + assert_eq!(c.color(index as usize), c.rgb(index), "slot {index}"); + } + assert_eq!(c.color(FOREGROUND), c.foreground); + assert_eq!(c.color(BACKGROUND), c.background); + assert_eq!(c.color(CURSOR), c.cursor); + } + + /// The 16 configurable slots come from the profile; the rest come from the + /// xterm table, which is the same in every terminal. + #[test] + fn only_the_ansi_slots_follow_the_profile() { + let recolored = Colors { + red: Rgb::new(1, 2, 3), + ..Default::default() + }; + assert_eq!(recolored.rgb(1), Rgb::new(1, 2, 3), "slot 1 follows it"); + for index in 16u8..=255 { + assert_eq!( + recolored.rgb(index), + xterm_color(index), + "slot {index} is fixed by the specification" + ); + } + } + + /// Spot-check the static table against the values the specification + /// defines, so a typo in 256 entries cannot pass unnoticed. + #[test] + fn the_xterm_table_matches_the_specification() { + assert_eq!(xterm_color(0), Rgb::new(0, 0, 0), "VGA black"); + assert_eq!(xterm_color(1), Rgb::new(128, 0, 0), "VGA red"); + assert_eq!(xterm_color(15), Rgb::new(255, 255, 255), "VGA bright white"); + assert_eq!( + xterm_color(16), + Rgb::new(0, 0, 0), + "the cube starts at black" + ); + assert_eq!(xterm_color(196), Rgb::new(255, 0, 0), "cube red"); + assert_eq!( + xterm_color(231), + Rgb::new(255, 255, 255), + "the cube ends white" + ); + assert_eq!(xterm_color(232), Rgb::new(8, 8, 8), "the ramp starts at 8"); assert_eq!( - c.resolve(Some(Color::Rgb(1, 2, 3)), true), - Rgb::new(1, 2, 3), - "a true-color cell is itself whatever the profile says" + xterm_color(255), + Rgb::new(238, 238, 238), + "the ramp ends at 238" ); } diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index e88710c..3a3f0b1 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -11,8 +11,9 @@ use std::fmt::Write; use super::nerd_font::NerdFont; -use crate::profile::{Palette, Rgb}; +use crate::profile::Rgb; use crate::terminal::cell::{Attrs, EmuCell}; +use crate::terminal::emu::Emulator; const CELL_W: f32 = 10.0; const CELL_H: f32 = 21.0; @@ -41,7 +42,7 @@ fn cell_at(row: &[EmuCell], x: usize) -> &EmuCell { } /// Resolved background color for a cell (honoring inverse). -fn bg_of(cell: &EmuCell, colors: &Palette) -> Rgb { +fn bg_of(cell: &EmuCell, colors: &dyn Emulator) -> Rgb { let bg = colors.resolve(cell.bg, false); let fg = colors.resolve(cell.fg, true); if cell.has(Attrs::INVERSE) { @@ -61,7 +62,7 @@ struct Style { invisible: bool, } -fn style_of(cell: &EmuCell, colors: &Palette) -> Style { +fn style_of(cell: &EmuCell, colors: &dyn Emulator) -> Style { let mut fg = colors.resolve(cell.fg, true); let bg = colors.resolve(cell.bg, false); if cell.has(Attrs::INVERSE) { @@ -102,7 +103,7 @@ fn run_text(row: &[EmuCell], start: usize, end: usize) -> String { } /// Render a grid to a standalone SVG document. -pub fn render_svg(rows: &[Vec], cols: u16, colors: &Palette) -> String { +pub fn render_svg(rows: &[Vec], cols: u16, colors: &dyn Emulator) -> String { let nerd_font = NerdFont::new(rows, FONT_SIZE); let cols = cols as usize; let x0 = MARGIN_X; @@ -215,10 +216,14 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &Palette) -> String #[cfg(test)] mod tests { use super::*; + use crate::profile::Profile; + use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::cell::Color; - fn colors() -> Palette { - Palette::default() + /// A real emulator: the renderer resolves through the same path a session + /// uses, so a stand-in could not drift from it. + fn colors() -> AlacrittyEmu { + AlacrittyEmu::new(10, 2, &Profile::default()) } fn cell(ch: &str, fg: Option, bg: Option) -> EmuCell { diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 6ac8d05..24bc20e 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -16,9 +16,9 @@ use alacritty_terminal::vte::ansi::Rgb as AlacRgb; use compact_str::{CompactString, ToCompactString}; -use crate::profile::{Colors, Profile, Rgb}; +use crate::profile::{Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; -use crate::terminal::emu::{self, Emulator}; +use crate::terminal::emu::Emulator; /// Alacritty's palette colors arrive either as a `Named` variant or an index; /// both funnel through [`Color::from_index`] so a given slot always yields the @@ -156,10 +156,10 @@ pub struct AlacrittyEmu { rows: u16, pending: Arc>>, queries: Arc>>, - /// The colors this session was configured with. A program can shadow them - /// at runtime but never reach them, so a reset always has a value to - /// restore. - config: Colors, + /// The settings this session was opened with. A program can shadow the + /// colors at runtime but never reach them, so a reset always has a value + /// to restore. + profile: Profile, } impl AlacrittyEmu { @@ -182,14 +182,12 @@ impl AlacrittyEmu { rows, pending, queries, - config: profile.colors, + profile: *profile, } } /// Answer any color queries parked while the last chunk was parsed. /// - /// alacritty stores a color a program set, and leaves the slot empty - /// otherwise, so an empty slot is answered from the session profile. fn answer_queries(&mut self) { let parked: Vec<(usize, ReplyFormat)> = match self.queries.lock() { Ok(mut queries) => queries.drain(..).collect(), @@ -255,6 +253,15 @@ impl Emulator for AlacrittyEmu { (self.cols, self.rows) } + /// alacritty stores only what a program set, leaving every other slot + /// empty, so an empty slot means the profile's color still shows through. + fn color(&self, slot: usize) -> Rgb { + match self.term.colors()[slot] { + Some(set) => Rgb::new(set.r, set.g, set.b), + None => self.profile.colors.color(slot), + } + } + fn cursor(&self) -> (u16, u16) { let p = self.term.grid().cursor.point; let y = p.line.0.max(0).min(self.rows as i32 - 1) as u16; @@ -266,21 +273,6 @@ impl Emulator for AlacrittyEmu { self.rows_in_range(0, self.rows as i32) } - fn color(&self, slot: usize) -> Rgb { - // `Colors` stores only what a program set; an empty slot means the - // session's configured color still shows through. - if let Some(set) = self.term.colors()[slot] { - return Rgb::new(set.r, set.g, set.b); - } - match slot { - emu::FOREGROUND => self.config.foreground, - emu::BACKGROUND => self.config.background, - emu::CURSOR => self.config.cursor, - i if i < emu::FOREGROUND => self.config.rgb(i as u8), - _ => self.config.foreground, - } - } - fn full_rows(&self) -> Vec> { let grid = self.term.grid(); let total = grid.total_lines() as i32; diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index 6b5fc85..2672a59 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -635,7 +635,7 @@ macro_rules! emulator_conformance_tests { ..Default::default() }; let mut e = conformance_emu_with(10, 4, profile); - let background = $crate::terminal::emu::BACKGROUND; + let background = $crate::profile::BACKGROUND; assert_eq!(e.color(background), configured); e.process(b"\x1b]11;#654321\x07"); @@ -682,14 +682,14 @@ macro_rules! emulator_conformance_tests { e.process(b"\x1b[31mR"); let cell = e.viewable_rows()[0][0].clone(); - let before = e.palette().resolve(cell.fg, true); + let before = e.resolve(cell.fg, true); e.process(b"\x1b]4;1;#0a0b0c\x07"); assert_eq!( - e.palette().resolve(cell.fg, true), + e.resolve(cell.fg, true), Rgb::new(0x0a, 0x0b, 0x0c), "recoloring the slot recolors the cell that chose it" ); - assert_ne!(before, e.palette().resolve(cell.fg, true)); + assert_ne!(before, e.resolve(cell.fg, true)); } /// The alternate screen hides primary content and restores it on exit. diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index 9da0c00..de43d4d 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -10,17 +10,8 @@ //! identical shell-integration behavior by construction rather than by //! reimplementation. -use crate::profile::{Palette, Rgb}; -use crate::terminal::cell::EmuCell; - -/// Runtime color slots: the 256-color palette, then the three dynamic colors. -/// -/// The numbering is not ours — both emulators already address their special -/// colors this way, so a backend can hand its own table straight through. -pub const FOREGROUND: usize = 256; -pub const BACKGROUND: usize = 257; -pub const CURSOR: usize = 258; -pub const COLOR_SLOTS: usize = 259; +use crate::profile::Rgb; +use crate::terminal::cell::{Color, EmuCell}; /// A headless terminal emulator: bytes in, cell grid out. /// @@ -51,30 +42,39 @@ pub trait Emulator: Send { /// Scrollback history followed by the visible screen. fn full_rows(&self) -> Vec>; - /// The color a slot currently shows. + /// The color a slot is currently showing. /// /// Programs move these with `OSC 4` (palette) and `OSC 10/11/12` (default /// foreground, background, cursor), and put them back with `OSC 104` and - /// `OSC 110/111/112`. A reset restores the color the session was configured - /// with; nothing a program sends can change that configured value, so - /// there is always something to fall back to. + /// `OSC 110/111/112`. A slot nothing has overridden shows the color the + /// session's profile gives it, so a reset always has something to restore + /// and this always has an answer. /// /// Backends answer color *queries* themselves, through /// [`Emulator::take_pending_writes`], because each one already parses the - /// sequence and knows which terminator the query used. This method is how - /// the screenshot renderer and `expect --fg/--bg` see the same answer. + /// sequence and knows which terminator the query used. This reports the + /// same colors, so a screenshot and `expect --fg/--bg` agree with what a + /// program was told. /// - /// `slot` is a palette index, or one of [`FOREGROUND`], [`BACKGROUND`], - /// [`CURSOR`]. + /// `slot` is a palette index, or one of [`crate::profile::FOREGROUND`], + /// [`crate::profile::BACKGROUND`], [`crate::profile::CURSOR`]. fn color(&self, slot: usize) -> Rgb; - /// Every slot at once, so a consumer can resolve colors without holding - /// the session lock or knowing which backend produced them. - fn palette(&self) -> Palette { - let mut slots = [Rgb::new(0, 0, 0); COLOR_SLOTS]; - for (slot, out) in slots.iter_mut().enumerate() { - *out = self.color(slot); + /// Resolve a cell's color, where `None` is the terminal default. + /// + /// The grid records which slot a cell chose, never a color, so this is + /// where a cell becomes something to paint or compare. Provided rather + /// than required so every backend resolves a cell identically. + fn resolve(&self, color: Option, is_fg: bool) -> Rgb { + match color { + None => self.color(if is_fg { + crate::profile::FOREGROUND + } else { + crate::profile::BACKGROUND + }), + Some(Color::Named(n)) => self.color(n.index() as usize), + Some(Color::Idx(i)) => self.color(i as usize), + Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), } - Palette::new(slots) } } From 9de66758deb857c8d6f11b96d57165985b3aba58 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 15:28:57 -0400 Subject: [PATCH 3/8] refactor(profile): name color slots instead of numbering them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The profile addressed colors by index, using 256, 257, 258 for the foreground, background, and cursor. That is alacritty's layout — it stores the dynamic colors after the palette — and it had spread into the profile, the emulator trait, and the conformance suite, none of which have any reason to know it. A backend that numbered its own table differently would have had to pretend otherwise. Slots are a `ColorSlot` enum now: `Indexed(u8)`, `Foreground`, `Background`, `Cursor`. The alacritty backend translates that to its own indices inside the one match that reads its table, and resolves an unset slot to the profile, falling back to the xterm table for an index the profile does not name. The tests for that resolution moved to the backend that performs it, where they exercise the real path rather than a helper that mirrored it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/profile.rs | 49 +++-------- crates/shell-use/src/render/svg.rs | 6 +- crates/shell-use/src/terminal/alacritty.rs | 88 ++++++++++++++++++-- crates/shell-use/src/terminal/conformance.rs | 20 +++-- crates/shell-use/src/terminal/emu.rs | 14 ++-- 5 files changed, 117 insertions(+), 60 deletions(-) diff --git a/crates/shell-use/src/profile.rs b/crates/shell-use/src/profile.rs index 64b630e..698afd7 100644 --- a/crates/shell-use/src/profile.rs +++ b/crates/shell-use/src/profile.rs @@ -201,34 +201,20 @@ impl Colors { _ => xterm_color(index), } } - - /// The color a slot shows when no program has overridden it. - /// - /// This is what a backend falls back to: `slot` is a palette index, or one - /// of [`FOREGROUND`], [`BACKGROUND`], [`CURSOR`], matching how `OSC 4` and - /// `OSC 10/11/12` address them. - pub fn color(&self, slot: usize) -> Rgb { - match slot { - FOREGROUND => self.foreground, - BACKGROUND => self.background, - CURSOR => self.cursor, - index if index < FOREGROUND => self.rgb(index as u8), - // Above the addressable range there is nothing sensible to report; - // the foreground is the least surprising answer. - _ => self.foreground, - } - } } -/// Where the three dynamic colors sit when a color slot is addressed by -/// number. +/// A color a program can address. /// -/// The numbering is not ours: `OSC 4` addresses the 256-color palette, and -/// `OSC 10/11/12` address the three colors after it. Both emulators already -/// index their tables this way, so a backend reads its own state directly. -pub const FOREGROUND: usize = 256; -pub const BACKGROUND: usize = 257; -pub const CURSOR: usize = 258; +/// `OSC 4` names a palette entry and `OSC 10/11/12` name the three defaults. +/// Emulators number these however they like internally, so each backend +/// translates its own layout and that numbering never reaches here. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ColorSlot { + Indexed(u8), + Foreground, + Background, + Cursor, +} /// The xterm 256-color table, which is the same in every terminal. /// @@ -500,19 +486,6 @@ mod tests { assert_eq!(Colors::slot_name(16), None, "only 0-15 are configurable"); } - /// Every slot a program can address resolves, so a backend always has a - /// color to fall back to and a query always has an answer. - #[test] - fn every_addressable_slot_resolves() { - let c = Colors::default(); - for index in 0u8..=255 { - assert_eq!(c.color(index as usize), c.rgb(index), "slot {index}"); - } - assert_eq!(c.color(FOREGROUND), c.foreground); - assert_eq!(c.color(BACKGROUND), c.background); - assert_eq!(c.color(CURSOR), c.cursor); - } - /// The 16 configurable slots come from the profile; the rest come from the /// xterm table, which is the same in every terminal. #[test] diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index 3a3f0b1..c2f1666 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -216,7 +216,7 @@ pub fn render_svg(rows: &[Vec], cols: u16, colors: &dyn Emulator) -> St #[cfg(test)] mod tests { use super::*; - use crate::profile::Profile; + use crate::profile::{ColorSlot, Profile}; use crate::terminal::alacritty::AlacrittyEmu; use crate::terminal::cell::Color; @@ -246,7 +246,7 @@ mod tests { assert!(svg.ends_with("")); assert!(svg.contains("textLength")); assert!( - svg.contains(&hex(colors().color(1))), + svg.contains(&hex(colors().color(ColorSlot::Indexed(1)))), "slot 1 is painted with the profile color" ); assert!(svg.contains(">hi")); @@ -283,7 +283,7 @@ mod tests { let rows = vec![vec![cell(" ", None, Some(Color::from_index(4)))]]; let svg = render_svg(&rows, 1, &colors()); assert!( - svg.contains(&hex(colors().color(4))), + svg.contains(&hex(colors().color(ColorSlot::Indexed(4)))), "slot 4 is painted with the profile color" ); } diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 24bc20e..5fdb9c3 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -12,11 +12,12 @@ use alacritty_terminal::term::cell::Flags as AlacFlags; use alacritty_terminal::term::test::TermSize; use alacritty_terminal::term::{Config as AlacConfig, Term}; use alacritty_terminal::vte::ansi; +use alacritty_terminal::vte::ansi::NamedColor; use alacritty_terminal::vte::ansi::Rgb as AlacRgb; use compact_str::{CompactString, ToCompactString}; -use crate::profile::{Profile, Rgb}; +use crate::profile::{xterm_color, ColorSlot, Profile, Rgb}; use crate::terminal::cell::{Attrs, Color, EmuCell, UnderlineStyle, CONTINUATION}; use crate::terminal::emu::Emulator; @@ -198,7 +199,13 @@ impl AlacrittyEmu { } let replies: String = parked .into_iter() - .map(|(slot, format)| { + .map(|(index, format)| { + let slot = match index { + i if i == NamedColor::Foreground as usize => ColorSlot::Foreground, + i if i == NamedColor::Background as usize => ColorSlot::Background, + i if i == NamedColor::Cursor as usize => ColorSlot::Cursor, + i => ColorSlot::Indexed(i as u8), + }; let c = self.color(slot); format(AlacRgb { r: c.r, @@ -254,11 +261,30 @@ impl Emulator for AlacrittyEmu { } /// alacritty stores only what a program set, leaving every other slot - /// empty, so an empty slot means the profile's color still shows through. - fn color(&self, slot: usize) -> Rgb { - match self.term.colors()[slot] { + /// empty, so an empty slot falls through to the profile, and then to the + /// xterm table for an index the profile does not name. + /// + /// The indices are alacritty's own: it lays its table out as the + /// 256-color palette followed by the dynamic colors, which is why the + /// three are 256, 257, 258. That layout stops here. + fn color(&self, slot: ColorSlot) -> Rgb { + let colors = &self.profile.colors; + let (index, configured) = match slot { + ColorSlot::Indexed(index) => ( + index as usize, + colors + .ansi() + .get(index as usize) + .copied() + .unwrap_or_else(|| xterm_color(index)), + ), + ColorSlot::Foreground => (NamedColor::Foreground as usize, colors.foreground), + ColorSlot::Background => (NamedColor::Background as usize, colors.background), + ColorSlot::Cursor => (NamedColor::Cursor as usize, colors.cursor), + }; + match self.term.colors()[index] { Some(set) => Rgb::new(set.r, set.g, set.b), - None => self.profile.colors.color(slot), + None => configured, } } @@ -287,4 +313,54 @@ mod tests { use super::*; crate::emulator_conformance_tests!(|c, r, p| Box::new(AlacrittyEmu::new(c, r, p))); + + /// Every slot a program can address resolves, so a query always has an + /// answer and a reset always has something to restore. The profile names + /// sixteen; everything above falls through to the xterm table. + #[test] + fn every_slot_resolves_through_the_profile_then_xterm() { + use crate::profile::{Colors, Rgb}; + let profile = Profile { + colors: Colors { + red: Rgb::new(1, 2, 3), + background: Rgb::new(4, 5, 6), + ..Default::default() + }, + ..Default::default() + }; + let emu = AlacrittyEmu::new(10, 2, &profile); + + assert_eq!( + emu.color(ColorSlot::Indexed(1)), + Rgb::new(1, 2, 3), + "the profile names slot 1" + ); + assert_eq!(emu.color(ColorSlot::Background), Rgb::new(4, 5, 6)); + assert_eq!( + emu.color(ColorSlot::Foreground), + Colors::default().foreground, + "an unset profile color keeps its default" + ); + for index in 16u8..=255 { + assert_eq!( + emu.color(ColorSlot::Indexed(index)), + xterm_color(index), + "slot {index} is not the profile's to name" + ); + } + } + + /// A program's color outranks the profile until it is reset, at which + /// point the profile shows through again. + #[test] + fn a_program_color_outranks_the_profile_until_reset() { + let mut emu = AlacrittyEmu::new(10, 2, &Profile::default()); + let configured = emu.color(ColorSlot::Background); + + emu.process(b"\x1b]11;#123456\x07"); + assert_eq!(emu.color(ColorSlot::Background), Rgb::new(0x12, 0x34, 0x56)); + + emu.process(b"\x1b]111\x07"); + assert_eq!(emu.color(ColorSlot::Background), configured); + } } diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index 2672a59..df2c708 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -635,7 +635,7 @@ macro_rules! emulator_conformance_tests { ..Default::default() }; let mut e = conformance_emu_with(10, 4, profile); - let background = $crate::profile::BACKGROUND; + let background = $crate::profile::ColorSlot::Background; assert_eq!(e.color(background), configured); e.process(b"\x1b]11;#654321\x07"); @@ -654,9 +654,15 @@ macro_rules! emulator_conformance_tests { // The same for a palette entry, which resets with OSC 104. e.process(b"\x1b]4;2;#010203\x07"); - assert_eq!(e.color(2), Rgb::new(1, 2, 3)); + assert_eq!( + e.color($crate::profile::ColorSlot::Indexed(2)), + Rgb::new(1, 2, 3) + ); e.process(b"\x1b]104;2\x07"); - assert_eq!(e.color(2), Colors::default().green); + assert_eq!( + e.color($crate::profile::ColorSlot::Indexed(2)), + Colors::default().green + ); } /// An unconfigured palette entry still answers, from the table the @@ -666,11 +672,15 @@ macro_rules! emulator_conformance_tests { use $crate::profile::Rgb; let e = conformance_emu(10, 4, 100); assert_eq!( - e.color(196), + e.color($crate::profile::ColorSlot::Indexed(196)), Rgb::new(255, 0, 0), "index 196 is pure red in the xterm color cube" ); - assert_eq!(e.color(232), Rgb::new(8, 8, 8), "the gray ramp starts at 8"); + assert_eq!( + e.color($crate::profile::ColorSlot::Indexed(232)), + Rgb::new(8, 8, 8), + "the gray ramp starts at 8" + ); } /// A cell records which slot it chose, never a color, so what it diff --git a/crates/shell-use/src/terminal/emu.rs b/crates/shell-use/src/terminal/emu.rs index de43d4d..edbcb41 100644 --- a/crates/shell-use/src/terminal/emu.rs +++ b/crates/shell-use/src/terminal/emu.rs @@ -10,7 +10,7 @@ //! identical shell-integration behavior by construction rather than by //! reimplementation. -use crate::profile::Rgb; +use crate::profile::{ColorSlot, Rgb}; use crate::terminal::cell::{Color, EmuCell}; /// A headless terminal emulator: bytes in, cell grid out. @@ -56,9 +56,7 @@ pub trait Emulator: Send { /// same colors, so a screenshot and `expect --fg/--bg` agree with what a /// program was told. /// - /// `slot` is a palette index, or one of [`crate::profile::FOREGROUND`], - /// [`crate::profile::BACKGROUND`], [`crate::profile::CURSOR`]. - fn color(&self, slot: usize) -> Rgb; + fn color(&self, slot: ColorSlot) -> Rgb; /// Resolve a cell's color, where `None` is the terminal default. /// @@ -68,12 +66,12 @@ pub trait Emulator: Send { fn resolve(&self, color: Option, is_fg: bool) -> Rgb { match color { None => self.color(if is_fg { - crate::profile::FOREGROUND + ColorSlot::Foreground } else { - crate::profile::BACKGROUND + ColorSlot::Background }), - Some(Color::Named(n)) => self.color(n.index() as usize), - Some(Color::Idx(i)) => self.color(i as usize), + Some(Color::Named(n)) => self.color(ColorSlot::Indexed(n.index())), + Some(Color::Idx(i)) => self.color(ColorSlot::Indexed(i)), Some(Color::Rgb(r, g, b)) => Rgb::new(r, g, b), } } From 68cb9f6777eb692858f99f5c79b147d291f922a2 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 16:23:16 -0400 Subject: [PATCH 4/8] test(snapshot): pin that a snapshot records slots, not colors A snapshot stores the palette slot a cell chose rather than the color that slot resolves to, which is what lets a saved baseline outlive a profile change: recoloring a terminal would otherwise invalidate every snapshot in a suite at once. That was already true and nothing checked it, so a change to how colors are serialized could have quietly made snapshots profile-dependent. The companion case pins the exception: a true-color cell names its own color, so that one is recorded literally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/assert/snapshot.rs | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/shell-use/src/assert/snapshot.rs b/crates/shell-use/src/assert/snapshot.rs index ca0f00a..186b686 100644 --- a/crates/shell-use/src/assert/snapshot.rs +++ b/crates/shell-use/src/assert/snapshot.rs @@ -149,6 +149,43 @@ mod tests { use super::*; use crate::terminal::cell::CONTINUATION; + /// A snapshot records the palette *slot* a cell chose, never the color + /// that slot resolves to. + /// + /// This is what lets a saved baseline outlive a profile change: the same + /// screen recorded under two profiles that disagree about what red looks + /// like still produces the same snapshot, so recoloring a terminal does + /// not invalidate every snapshot in a suite. + #[test] + fn a_snapshot_records_the_slot_rather_than_the_color() { + let colored = EmuCell { + ch: "x".into(), + fg: Some(Color::from_index(1)), + ..EmuCell::blank() + }; + let out = serialize(&[vec![colored]], 1, true); + assert!( + out.contains("\"fg\": 1"), + "the slot is recorded, not an rgb value: {out}" + ); + assert!( + !out.contains('#'), + "a palette color must not be resolved into the snapshot: {out}" + ); + } + + /// A true-color cell names its own color, so that one *is* recorded + /// literally: no profile can change what `38;2;r;g;b` means. + #[test] + fn a_true_color_cell_records_its_own_value() { + let rgb = EmuCell { + ch: "x".into(), + fg: Some(Color::Rgb(0x11, 0x22, 0x33)), + ..EmuCell::blank() + }; + assert!(serialize(&[vec![rgb]], 1, true).contains("#112233")); + } + fn cell(s: &str) -> EmuCell { EmuCell { ch: s.into(), From 1fe6420a64dbfa15e42abca033180dad74c94f3c Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 16:29:32 -0400 Subject: [PATCH 5/8] test(color): cover setting the foreground and cursor, and rendering after The color tests leaned almost entirely on `OSC 11`. That is the sequence programs actually reach for, but it meant the foreground and cursor were never set, never queried, and never reset, so wiring any of them to the wrong slot would have gone unnoticed. Three conformance cases now set all three to distinct colors and check that each reset frees only its own, that each answers its own query, and that a bare `OSC 104` resets the palette without touching them. Nothing covered the path from an escape sequence to a rendered pixel either. Both halves are pinned now: a screenshot paints the background a program set and recolors a cell whose slot it moved, and an assertion matches that same color while still comparing the index unchanged. Both return to the profile after a reset. They read the same state, so this is the earlier "a screenshot and an assertion agree" guarantee held at every point in a session rather than only at the start. The end-to-end test drives all three dynamic colors over a real PTY. It needs a wide terminal: its report is one line, and `text` returns the grid, so a narrower one wrapped the reply out of the assertion's reach. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- .../shell-use-cli/tests/session_lifecycle.rs | 23 ++++- crates/shell-use/src/assert/color.rs | 48 ++++++++++ crates/shell-use/src/render/svg.rs | 49 ++++++++++ crates/shell-use/src/terminal/conformance.rs | 92 +++++++++++++++++++ 4 files changed, 208 insertions(+), 4 deletions(-) diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index 8ff72fb..aabc5f3 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -374,20 +374,27 @@ old = termios.tcgetattr(fd) try: tty.setraw(fd) configured = ask(fd, b"\x1b]11;?\x07") - os.write(1, b"\x1b]11;#654321\x07") + # Every dynamic colour, not just the background: a program that sets the + # foreground and cursor has to be answered about those too. + os.write(1, b"\x1b]10;#abcdef\x07\x1b]11;#654321\x07\x1b]12;#fedcba\x07") + fg = ask(fd, b"\x1b]10;?\x07") overridden = ask(fd, b"\x1b]11;?\x07") + cursor = ask(fd, b"\x1b]12;?\x07") os.write(1, b"\x1b]111\x07") restored = ask(fd, b"\x1b]11;?\x07") finally: termios.tcsetattr(fd, termios.TCSADRAIN, old) strip = lambda s: s.replace("\x1b", "").replace("\x07", "") -print("\r\nRESULT %s %s %s\r" % (strip(configured), strip(overridden), strip(restored))) +print("\r\nRESULT %s %s %s %s %s\r" % ( + strip(configured), strip(fg), strip(overridden), strip(cursor), strip(restored))) "#, ) .expect("write probe"); - sandbox.ok(&["run", "--cols", "80", "--", "bash", "--norc"]); + // Wide enough that the report is one unwrapped line: `text` returns the + // grid, so a wrapped reply would be split across rows. + sandbox.ok(&["run", "--cols", "200", "--", "bash", "--norc"]); sandbox.ok(&[ "submit", &format!("python3 {}", probe.to_str().expect("utf-8 path")), @@ -408,7 +415,15 @@ print("\r\nRESULT %s %s %s\r" % (strip(configured), strip(overridden), strip(res ); assert!( line.contains("]11;rgb:6565/4343/2121"), - "a set color should be reported back: {line}" + "a set background should be reported back: {line}" + ); + assert!( + line.contains("]10;rgb:abab/cdcd/efef"), + "a set foreground should be reported back: {line}" + ); + assert!( + line.contains("]12;rgb:fefe/dcdc/baba"), + "a set cursor color should be reported back: {line}" ); assert_eq!( line.matches("]11;rgb:0000/0000/0000").count(), diff --git a/crates/shell-use/src/assert/color.rs b/crates/shell-use/src/assert/color.rs index 5cf89ea..ed6b245 100644 --- a/crates/shell-use/src/assert/color.rs +++ b/crates/shell-use/src/assert/color.rs @@ -222,6 +222,54 @@ mod tests { } } + /// An assertion compares against what the terminal is *currently* + /// showing, so a program that recolors a slot changes what matches. + /// + /// This is the other half of the screenshot test: both read the same + /// state, so a colour a screenshot paints is a colour an assertion + /// matches, at every point in a session rather than only at the start. + #[test] + fn an_assertion_follows_a_color_a_program_set() { + use crate::terminal::emu::Emulator; + let mut emu = emu_with(Colors::default()); + let red = Some(Color::from_index(1)); + let configured = Colors::default().red; + + assert!(matches( + red, + &Expected::Hex(configured.r, configured.g, configured.b), + &emu + )); + + emu.process(b"\x1b]4;1;#22c55e\x07"); + assert!( + matches(red, &Expected::Hex(0x22, 0xc5, 0x5e), &emu), + "the assertion follows the colour the program set" + ); + assert!( + !matches( + red, + &Expected::Hex(configured.r, configured.g, configured.b), + &emu + ), + "the configured colour is no longer what slot 1 shows" + ); + assert!( + matches(red, &Expected::Ansi256(1), &emu), + "the index is unaffected: it names a slot, not a colour" + ); + + emu.process(b"\x1b]104;1\x07"); + assert!( + matches( + red, + &Expected::Hex(configured.r, configured.g, configured.b), + &emu + ), + "a reset restores the configured colour" + ); + } + /// A profile's palette is what an assertion compares against, so two /// profiles genuinely disagree rather than sharing one hardcoded table. #[test] diff --git a/crates/shell-use/src/render/svg.rs b/crates/shell-use/src/render/svg.rs index c2f1666..ff15d99 100644 --- a/crates/shell-use/src/render/svg.rs +++ b/crates/shell-use/src/render/svg.rs @@ -235,6 +235,55 @@ mod tests { } } + /// A program that repaints the terminal repaints the screenshot. + /// + /// The renderer draws what the terminal is currently showing, not what it + /// was configured with, so a background set with `OSC 11` is the one that + /// gets painted. Nothing else covers the path from an escape sequence to + /// a rendered pixel. + #[test] + fn a_screenshot_follows_colors_a_program_set() { + use crate::terminal::emu::Emulator; + let mut emu = colors(); + let rows = vec![vec![cell("x", Some(Color::from_index(1)), None)]]; + + let before = render_svg(&rows, 1, &emu); + assert!(before.contains(&hex(Profile::default().colors.red))); + assert!(before.contains(&hex(Profile::default().colors.background))); + + // The program picks its own background and recolors palette slot 1. + emu.process(b"\x1b]11;#3b0764\x07\x1b]4;1;#22c55e\x07"); + + let after = render_svg(&rows, 1, &emu); + assert!( + after.contains("#3b0764"), + "the window is painted with the background the program set" + ); + assert!( + after.contains("#22c55e"), + "a cell follows the slot the program recolored" + ); + assert!( + !after.contains(&hex(Profile::default().colors.red)), + "the configured red is no longer what slot 1 shows" + ); + } + + /// And a reset puts the configured colors back on screen. + #[test] + fn a_screenshot_returns_to_the_profile_after_a_reset() { + use crate::terminal::emu::Emulator; + let mut emu = colors(); + let rows = vec![vec![cell("x", Some(Color::from_index(1)), None)]]; + + emu.process(b"\x1b]11;#3b0764\x07\x1b]4;1;#22c55e\x07"); + emu.process(b"\x1b]111\x07\x1b]104;1\x07"); + + let after = render_svg(&rows, 1, &emu); + assert!(after.contains(&hex(Profile::default().colors.background))); + assert!(after.contains(&hex(Profile::default().colors.red))); + } + #[test] fn emits_valid_svg_with_text_and_color() { let rows = vec![vec![ diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index df2c708..713629c 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -665,6 +665,98 @@ macro_rules! emulator_conformance_tests { ); } + /// Each dynamic color is addressed by its own sequence, and each is + /// reset by its own. + /// + /// `OSC 11` is the one programs reach for, so it is easy to wire that + /// up and leave the foreground or the cursor answering the wrong slot. + #[test] + fn conformance_each_dynamic_color_is_separately_addressable() { + use $crate::profile::ColorSlot; + let mut e = conformance_emu(10, 4, 100); + let before = [ + e.color(ColorSlot::Foreground), + e.color(ColorSlot::Background), + e.color(ColorSlot::Cursor), + ]; + + // Set all three to distinct colors, then check none bled into + // another. + e.process(b"\x1b]10;#111111\x07\x1b]11;#222222\x07\x1b]12;#333333\x07"); + assert_eq!(e.color(ColorSlot::Foreground), Rgb::new(0x11, 0x11, 0x11)); + assert_eq!(e.color(ColorSlot::Background), Rgb::new(0x22, 0x22, 0x22)); + assert_eq!(e.color(ColorSlot::Cursor), Rgb::new(0x33, 0x33, 0x33)); + + // And each reset frees only its own slot. + e.process(b"\x1b]110\x07"); + assert_eq!(e.color(ColorSlot::Foreground), before[0], "110 resets fg"); + assert_eq!( + e.color(ColorSlot::Background), + Rgb::new(0x22, 0x22, 0x22), + "110 must leave the background alone" + ); + + e.process(b"\x1b]112\x07"); + assert_eq!( + e.color(ColorSlot::Cursor), + before[2], + "112 resets the cursor" + ); + assert_eq!( + e.color(ColorSlot::Background), + Rgb::new(0x22, 0x22, 0x22), + "112 must leave the background alone" + ); + + e.process(b"\x1b]111\x07"); + assert_eq!(e.color(ColorSlot::Background), before[1], "111 resets bg"); + } + + /// Every dynamic color answers a query, not just the background. + #[test] + fn conformance_every_dynamic_color_answers_a_query() { + let mut e = conformance_emu(10, 4, 100); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]10;#010203\x07\x1b]11;#040506\x07\x1b]12;#070809\x07"); + let _ = e.take_pending_writes(); + + for (query, expected) in [ + (&b"\x1b]10;?\x07"[..], "\x1b]10;rgb:0101/0202/0303\x07"), + (b"\x1b]11;?\x07", "\x1b]11;rgb:0404/0505/0606\x07"), + (b"\x1b]12;?\x07", "\x1b]12;rgb:0707/0808/0909\x07"), + ] { + e.process(query); + assert_eq!( + String::from_utf8_lossy(&e.take_pending_writes()), + expected, + "querying {:?}", + String::from_utf8_lossy(query) + ); + } + } + + /// `OSC 104` with no index resets the whole palette, and leaves the + /// three dynamic colors alone: they have their own resets. + #[test] + fn conformance_a_bare_palette_reset_spares_the_dynamic_colors() { + use $crate::profile::ColorSlot; + let mut e = conformance_emu(10, 4, 100); + let configured_red = e.color(ColorSlot::Indexed(1)); + + e.process(b"\x1b]4;1;#111111;200;#222222\x07\x1b]11;#333333\x07"); + assert_eq!(e.color(ColorSlot::Indexed(1)), Rgb::new(0x11, 0x11, 0x11)); + assert_eq!(e.color(ColorSlot::Indexed(200)), Rgb::new(0x22, 0x22, 0x22)); + + e.process(b"\x1b]104\x07"); + assert_eq!(e.color(ColorSlot::Indexed(1)), configured_red); + assert_eq!( + e.color(ColorSlot::Background), + Rgb::new(0x33, 0x33, 0x33), + "a palette reset is not a background reset" + ); + } + /// An unconfigured palette entry still answers, from the table the /// specification defines for it. #[test] From f293b49c2c560c3cd8397cfbe4a5327ac79ec253 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 4 Aug 2026 17:13:05 -0400 Subject: [PATCH 6/8] test(color): run the color query probe on unix only The probe puts its own terminal in raw mode so it can read a reply that arrives without a newline and must not be echoed. That needs `termios`, which Windows CPython does not ship, so the test could only ever fail there, and a fail-fast matrix let it cancel the other two platforms. Nothing about the reply is platform specific. Its format is covered by conformance cases that run against every backend, and the write that carries it to the child is the same `pty.write` that every `type` and `submit` already exercises on Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use-cli/tests/session_lifecycle.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index aabc5f3..27a630f 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -349,6 +349,14 @@ fn an_unknown_profile_is_rejected() { /// A terminal that stays silent leaves them blocked until they time out and /// guess, so this drives the whole path: daemon, emulator, and the reply on /// its way back up the PTY. +/// +/// Unix only, because the probe has to put its own terminal in raw mode to +/// read a reply that arrives without a newline and must not be echoed, and +/// `termios` does not exist on Windows CPython. The reply itself is not +/// platform specific: how it is formatted is covered by conformance cases +/// that run against every backend, and the write that carries it to the child +/// is the same `pty.write` every `type` and `submit` on Windows already uses. +#[cfg(unix)] #[test] fn a_color_query_is_answered_over_the_pty() { let sandbox = Sandbox::new("osc-query"); From a75487787e8a92724dd94dd838f3a7c7fd34dc60 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Wed, 5 Aug 2026 16:12:49 -0400 Subject: [PATCH 7/8] fix(terminal): answer queries in the order they were asked Color answers were parked in their own buffer and appended after everything else the terminal had to say, so a chunk holding a color query followed by a device attributes request was answered attributes first. That order is load-bearing. A program pipelines a batch of queries and ends it with a device attributes request, whose reply every terminal sends, then reads until that reply and treats it as the end of the batch. An answer arriving after it looks like the query went unanswered, and is then read as though the user had typed it. Replies now share one ordered queue and a query is resolved where it sits, so answers leave in the order they were asked for. Covered by a conformance case, so any backend added later inherits it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use/src/terminal/alacritty.rs | 129 +++++++++++-------- crates/shell-use/src/terminal/conformance.rs | 45 +++++++ 2 files changed, 123 insertions(+), 51 deletions(-) diff --git a/crates/shell-use/src/terminal/alacritty.rs b/crates/shell-use/src/terminal/alacritty.rs index 5fdb9c3..8c35234 100644 --- a/crates/shell-use/src/terminal/alacritty.rs +++ b/crates/shell-use/src/terminal/alacritty.rs @@ -120,32 +120,40 @@ fn cell_from_alac(c: &alacritty_terminal::term::cell::Cell) -> EmuCell { /// so the answer echoes the form the program asked in. type ReplyFormat = Arc String + Send + Sync>; -/// Queues what the terminal wants to say back to the PTY. +/// One thing the terminal wants to say back to the PTY. /// -/// Color queries cannot be answered here: the color lives in the terminal's -/// own palette, and this listener is constructed before the terminal it -/// listens to. They are parked instead, and [`AlacrittyEmu::answer_queries`] -/// resolves them once `process` returns, where the terminal is in scope. +/// A color query cannot be answered where it arrives: the color lives in the +/// terminal's own palette, and this listener is constructed before the +/// terminal it listens to. It is parked as a [`Reply::Color`] and resolved by +/// [`AlacrittyEmu::answer_queries`] once `process` returns. +enum Reply { + Bytes(Vec), + Color(usize, ReplyFormat), +} + +/// Queues what the terminal wants to say back to the PTY, in the order it +/// decided to say it. +/// +/// Answers and other replies share one queue because a program may pipeline +/// several requests in a single write and match the answers up by position. +/// The common idiom ends a batch of queries with a device attributes request, +/// whose reply every terminal sends, and treats that reply as the end of the +/// batch: an answer that arrived after it would look like the query went +/// unanswered, and would then be read as though the user had typed it. #[derive(Default, Clone)] struct CaptureProxy { - pending: Arc>>, - queries: Arc>>, + pending: Arc>>, } impl EventListener for CaptureProxy { fn send_event(&self, ev: Event) { - match ev { - Event::PtyWrite(bytes) => { - if let Ok(mut buf) = self.pending.lock() { - buf.extend_from_slice(bytes.as_bytes()); - } - } - Event::ColorRequest(slot, format) => { - if let Ok(mut queries) = self.queries.lock() { - queries.push((slot, format)); - } - } - _ => {} + let reply = match ev { + Event::PtyWrite(bytes) => Reply::Bytes(bytes.as_bytes().to_vec()), + Event::ColorRequest(slot, format) => Reply::Color(slot, format), + _ => return, + }; + if let Ok(mut queue) = self.pending.lock() { + queue.push(reply); } } } @@ -155,8 +163,7 @@ pub struct AlacrittyEmu { processor: ansi::Processor, cols: u16, rows: u16, - pending: Arc>>, - queries: Arc>>, + pending: Arc>>, /// The settings this session was opened with. A program can shadow the /// colors at runtime but never reach them, so a reset always has a value /// to restore. @@ -170,11 +177,9 @@ impl AlacrittyEmu { scrolling_history: profile.scrollback, ..Default::default() }; - let pending: Arc>> = Arc::default(); - let queries: Arc>> = Arc::default(); + let pending: Arc>> = Arc::default(); let proxy = CaptureProxy { pending: pending.clone(), - queries: queries.clone(), }; AlacrittyEmu { term: Term::new(alac_config, &size, proxy), @@ -182,40 +187,53 @@ impl AlacrittyEmu { cols, rows, pending, - queries, profile: *profile, } } - /// Answer any color queries parked while the last chunk was parsed. + /// Resolve any color queries parked while the last chunk was parsed. /// + /// Each answer replaces the query where it sits in the queue, so replies + /// leave in the order the program asked for them. Resolving them at the + /// end of the chunk rather than where they arrived is what makes this + /// necessary: the palette lives in the terminal, which the listener that + /// received the query cannot reach. fn answer_queries(&mut self) { - let parked: Vec<(usize, ReplyFormat)> = match self.queries.lock() { - Ok(mut queries) => queries.drain(..).collect(), + let parked: Vec = match self.pending.lock() { + Ok(mut queue) => { + if !queue.iter().any(|r| matches!(r, Reply::Color(..))) { + return; + } + std::mem::take(&mut queue) + } Err(_) => return, }; - if parked.is_empty() { - return; - } - let replies: String = parked + let resolved: Vec = parked .into_iter() - .map(|(index, format)| { - let slot = match index { - i if i == NamedColor::Foreground as usize => ColorSlot::Foreground, - i if i == NamedColor::Background as usize => ColorSlot::Background, - i if i == NamedColor::Cursor as usize => ColorSlot::Cursor, - i => ColorSlot::Indexed(i as u8), - }; - let c = self.color(slot); - format(AlacRgb { - r: c.r, - g: c.g, - b: c.b, - }) + .map(|reply| match reply { + Reply::Bytes(bytes) => Reply::Bytes(bytes), + Reply::Color(index, format) => { + let slot = match index { + i if i == NamedColor::Foreground as usize => ColorSlot::Foreground, + i if i == NamedColor::Background as usize => ColorSlot::Background, + i if i == NamedColor::Cursor as usize => ColorSlot::Cursor, + i => ColorSlot::Indexed(i as u8), + }; + let c = self.color(slot); + Reply::Bytes( + format(AlacRgb { + r: c.r, + g: c.g, + b: c.b, + }) + .into_bytes(), + ) + } }) .collect(); - if let Ok(mut buf) = self.pending.lock() { - buf.extend_from_slice(replies.as_bytes()); + if let Ok(mut queue) = self.pending.lock() { + // Anything queued meanwhile was asked for later, so it goes after. + queue.splice(0..0, resolved); } } @@ -243,10 +261,19 @@ impl Emulator for AlacrittyEmu { } fn take_pending_writes(&mut self) -> Vec { - match self.pending.lock() { - Ok(mut buf) => std::mem::take(&mut *buf), - Err(_) => Vec::new(), - } + let queued = match self.pending.lock() { + Ok(mut queue) => std::mem::take(&mut *queue), + Err(_) => return Vec::new(), + }; + queued + .into_iter() + .flat_map(|reply| match reply { + Reply::Bytes(bytes) => bytes, + // `answer_queries` runs at the end of every chunk, so a query + // is always resolved before anything can drain it. + Reply::Color(..) => Vec::new(), + }) + .collect() } fn resize(&mut self, cols: u16, rows: u16) { diff --git a/crates/shell-use/src/terminal/conformance.rs b/crates/shell-use/src/terminal/conformance.rs index 713629c..6dec835 100644 --- a/crates/shell-use/src/terminal/conformance.rs +++ b/crates/shell-use/src/terminal/conformance.rs @@ -620,6 +620,51 @@ macro_rules! emulator_conformance_tests { ); } + /// Replies leave in the order the program asked for them. + /// + /// A batch of queries is commonly ended with a device attributes + /// request, whose reply every terminal sends, and the reply to it is + /// read as the end of the batch. An answer that arrived after it would + /// look like the query went unanswered, and would then be read as + /// though the user had typed it. + #[test] + fn conformance_replies_keep_the_order_they_were_asked_in() { + let mut e = conformance_emu(10, 4, 100); + let _ = e.take_pending_writes(); + + e.process(b"\x1b]11;?\x07\x1b[c"); + let asked_color_first = e.take_pending_writes(); + let color = asked_color_first + .windows(2) + .position(|w| w == b"]1") + .expect("a color reply"); + let attributes = asked_color_first + .windows(2) + .position(|w| w == b"[?") + .expect("a device attributes reply"); + assert!( + color < attributes, + "the color was asked for first, so it is answered first: {:?}", + String::from_utf8_lossy(&asked_color_first) + ); + + e.process(b"\x1b[c\x1b]11;?\x07"); + let asked_color_second = e.take_pending_writes(); + let color = asked_color_second + .windows(2) + .position(|w| w == b"]1") + .expect("a color reply"); + let attributes = asked_color_second + .windows(2) + .position(|w| w == b"[?") + .expect("a device attributes reply"); + assert!( + attributes < color, + "and asked for second, it is answered second: {:?}", + String::from_utf8_lossy(&asked_color_second) + ); + } + /// A program can shadow a color, and a reset puts the configured one /// back. The configured color is never reachable, so a reset always /// has something to restore. From bfbe054ae7cbbd45fa3ac747a42f2187dcee4874 Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Wed, 5 Aug 2026 16:47:36 -0400 Subject: [PATCH 8/8] test(color): wait for the report, not for the command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe prints nothing until it is finished: its queries go to the terminal, which answers them rather than echoing them, so the screen stays unchanged for as long as python takes to start. The session runs `bash --norc` and so has no shell integration, which leaves `wait command` falling back to "the prompt came back and the screen is idle" — and an idle screen arrives immediately, long before the report does. On a loaded machine that fallback won the race and the test read the screen before the probe had written to it. Waiting for the line the test actually reads makes it wait for the right thing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Ayman Bagabas --- crates/shell-use-cli/tests/session_lifecycle.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/shell-use-cli/tests/session_lifecycle.rs b/crates/shell-use-cli/tests/session_lifecycle.rs index 27a630f..2320c1c 100644 --- a/crates/shell-use-cli/tests/session_lifecycle.rs +++ b/crates/shell-use-cli/tests/session_lifecycle.rs @@ -407,7 +407,15 @@ print("\r\nRESULT %s %s %s %s %s\r" % ( "submit", &format!("python3 {}", probe.to_str().expect("utf-8 path")), ]); - sandbox.ok(&["wait", "command"]); + // Wait for the line this test reads, not for the command. + // + // The probe prints nothing until it is done: its queries go to the + // terminal, which answers them rather than echoing them, so the screen + // stays unchanged for as long as python takes to start. `bash --norc` has + // no shell integration, so `wait command` falls back to "the prompt came + // back and the screen is idle", and on a loaded machine an idle screen + // arrives long before the report does. + sandbox.ok(&["wait", "text", "RESULT", "--timeout", "30000"]); let text = sandbox.ok(&["text", "--full"]); let line = text