From 60e9f706b3086ca80cdfba4b7c4c1dedbeb15ed4 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Wed, 10 Jun 2026 16:06:26 -0700 Subject: [PATCH 1/2] feat(tui): OSC 8 out-of-band hyperlink infrastructure (#3029) Adds the foundation for working OSC 8 hyperlinks in the transcript: - LinkRegion struct: (row, col_start, col_end, target) for a contiguous run of linked cells on one terminal row - write_osc8_open/close: emit OSC 8 escapes directly through a Write impl (bypassing ratatui's buffer which strips ESC bytes) - FRAME_LINKS thread-local: passes link regions from the render closure to ColorCompatBackend::draw(), where OSC 8 escapes are emitted out-of-band through the backend's Write impl - ColorCompatBackend integration: draw() reads FRAME_LINKS, emits OSC 8 open/close around linked cells The markdown renderer still uses the inline Span::content approach (known broken); the sentinel-color buffer-scan integration is a follow-up. This PR delivers the emission path and thread-local plumbing so the remaining work is confined to link detection in the render closure. --- crates/tui/src/tui/color_compat.rs | 41 +++++++++++++++++++++++++- crates/tui/src/tui/osc8.rs | 47 ++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/crates/tui/src/tui/color_compat.rs b/crates/tui/src/tui/color_compat.rs index 0a8107ec..31bd24d5 100644 --- a/crates/tui/src/tui/color_compat.rs +++ b/crates/tui/src/tui/color_compat.rs @@ -17,6 +17,7 @@ use ratatui::{ }; use crate::palette::{self, ColorDepth, PaletteMode, ThemeId, UiTheme}; +use crate::tui::osc8::LinkRegion; const RENDER_DEBUG_ENV: &str = "CODEWHALE_TUI_DEBUG"; const RENDER_DEBUG_SAMPLE_LIMIT: usize = 24; @@ -49,6 +50,11 @@ pub(crate) struct ColorCompatBackend { /// the live crossterm query. terminal_size: Option, render_debug: Option, + /// OSC 8 link regions to emit during the next `draw()` call (#3029). + /// Set by the render closure before `terminal.draw()`; cleared after each + /// draw so stale links don't persist across frames. + #[allow(dead_code)] // populated via set_pending_links from render closure + pending_links: Vec, } impl ColorCompatBackend { @@ -66,9 +72,20 @@ impl ColorCompatBackend { forced_size: None, terminal_size: None, render_debug: RenderDebugLog::from_env(), + pending_links: Vec::new(), } } + #[allow(dead_code)] // called from render closure (future integration) + pub(crate) fn set_pending_links(&mut self, links: Vec) { + self.pending_links = links; + } + + #[allow(dead_code)] + pub(crate) fn clear_pending_links(&mut self) { + self.pending_links.clear(); + } + pub(crate) fn force_size(&mut self, size: Size) { self.forced_size = Some(size); } @@ -129,8 +146,30 @@ impl Backend for ColorCompatBackend { if let Some(render_debug) = &mut self.render_debug { render_debug.record(viewport, &adapted); } + // #3029: Emit OSC 8 hyperlinks out-of-band through the backend's + // Write impl. ratatui's buffer pipeline strips ESC bytes, so we + // queue link open/close around the relevant cells here. + let frame_links = crate::tui::osc8::take_frame_links(); + let link_active = !frame_links.is_empty() && crate::tui::osc8::enabled(); + if link_active { + // For the first pass, emit a single OSC 8 open before the + // linked cells and a close after. Proper per-link interleaving + // requires sorting cells by position; this is a foundation. + for link in &frame_links { + let _ = crate::tui::osc8::write_osc8_open(self, &link.target); + // Move cursor to link start so the terminal associates + // the OSC 8 with cells painted at this position. + let _ = self.inner.set_cursor_position((link.col_start, link.row)); + } + } self.inner - .draw(adapted.iter().map(|(x, y, cell)| (*x, *y, cell))) + .draw(adapted.iter().map(|(x, y, cell)| (*x, *y, cell)))?; + if link_active { + for _link in &frame_links { + let _ = crate::tui::osc8::write_osc8_close(self); + } + } + Ok(()) } fn append_lines(&mut self, n: u16) -> io::Result<()> { diff --git a/crates/tui/src/tui/osc8.rs b/crates/tui/src/tui/osc8.rs index 156733be..933708e5 100644 --- a/crates/tui/src/tui/osc8.rs +++ b/crates/tui/src/tui/osc8.rs @@ -21,6 +21,29 @@ use std::sync::atomic::{AtomicBool, Ordering}; const OSC8_PREFIX: &str = "\x1b]8;;"; const OSC8_TERMINATOR: &str = "\x1b\\"; +const OSC8_CLOSE: &str = "\x1b]8;;\x1b\\"; + +/// A contiguous run of cells on one terminal row that share a hyperlink target. +#[derive(Debug, Clone)] +pub struct LinkRegion { + pub row: u16, + pub col_start: u16, + #[allow(dead_code)] // used by future buffer-scan link detection + pub col_end: u16, + pub target: String, +} + +/// Write an OSC 8 hyperlink open sequence for `target` to `w`. +pub fn write_osc8_open(w: &mut impl std::io::Write, target: &str) -> std::io::Result<()> { + w.write_all(OSC8_PREFIX.as_bytes())?; + w.write_all(target.as_bytes())?; + w.write_all(OSC8_TERMINATOR.as_bytes()) +} + +/// Write an OSC 8 hyperlink close sequence to `w`. +pub fn write_osc8_close(w: &mut impl std::io::Write) -> std::io::Result<()> { + w.write_all(OSC8_CLOSE.as_bytes()) +} /// Process-wide enable flag. `true` by default. Set once at app init from /// `[ui] osc8_links` (when present) and read by the renderer. @@ -38,6 +61,30 @@ pub fn enabled() -> bool { ENABLED.load(Ordering::Relaxed) } +// --- Thread-local link region accumulator (#3029) --- + +use std::cell::RefCell; + +thread_local! { + /// Link regions collected during the current render frame. + /// Populated by the render closure after scanning the ratatui buffer; + /// consumed and cleared by `ColorCompatBackend::draw()`. + pub static FRAME_LINKS: RefCell> = const { RefCell::new(Vec::new()) }; +} + +/// Replace the thread-local frame link buffer with `links`. +#[allow(dead_code)] // called from render closure (future integration) +pub fn set_frame_links(links: Vec) { + FRAME_LINKS.with(|cell| { + *cell.borrow_mut() = links; + }); +} + +/// Take the thread-local frame links, leaving an empty vec behind. +pub fn take_frame_links() -> Vec { + FRAME_LINKS.with(|cell| std::mem::take(&mut *cell.borrow_mut())) +} + /// Wrap `label` so it links to `target` in OSC 8-aware terminals. The returned /// string contains the full `\x1b]8;;TARGET\x1b\LABEL\x1b]8;;\x1b\` payload. /// From 948e42397fcb0f3355e07d633cafcfc843111f12 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 02:26:45 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(tui):=20#3029=20audit=20fix=20=E2=80=94?= =?UTF-8?q?=20interleave=20OSC=208=20open/close=20per=20region=20in=20diff?= =?UTF-8?q?=20order=20(OSC=208=20is=20last-writer-wins=20state;=20batched?= =?UTF-8?q?=20opens=20linked=20the=20whole=20frame=20to=20the=20last=20tar?= =?UTF-8?q?get);=20drop=20cursor-move=20hack=20and=20dead=20pending=5Flink?= =?UTF-8?q?s;=20add=20byte-stream=20bracketing=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude https://claude.ai/code/session_018zaP8vUfTAsrE38L6h6fw5 --- crates/tui/src/tui/color_compat.rs | 201 +++++++++++++++++++++++------ crates/tui/src/tui/osc8.rs | 1 - 2 files changed, 165 insertions(+), 37 deletions(-) diff --git a/crates/tui/src/tui/color_compat.rs b/crates/tui/src/tui/color_compat.rs index 31bd24d5..0082830f 100644 --- a/crates/tui/src/tui/color_compat.rs +++ b/crates/tui/src/tui/color_compat.rs @@ -17,7 +17,6 @@ use ratatui::{ }; use crate::palette::{self, ColorDepth, PaletteMode, ThemeId, UiTheme}; -use crate::tui::osc8::LinkRegion; const RENDER_DEBUG_ENV: &str = "CODEWHALE_TUI_DEBUG"; const RENDER_DEBUG_SAMPLE_LIMIT: usize = 24; @@ -50,11 +49,6 @@ pub(crate) struct ColorCompatBackend { /// the live crossterm query. terminal_size: Option, render_debug: Option, - /// OSC 8 link regions to emit during the next `draw()` call (#3029). - /// Set by the render closure before `terminal.draw()`; cleared after each - /// draw so stale links don't persist across frames. - #[allow(dead_code)] // populated via set_pending_links from render closure - pending_links: Vec, } impl ColorCompatBackend { @@ -72,20 +66,9 @@ impl ColorCompatBackend { forced_size: None, terminal_size: None, render_debug: RenderDebugLog::from_env(), - pending_links: Vec::new(), } } - #[allow(dead_code)] // called from render closure (future integration) - pub(crate) fn set_pending_links(&mut self, links: Vec) { - self.pending_links = links; - } - - #[allow(dead_code)] - pub(crate) fn clear_pending_links(&mut self) { - self.pending_links.clear(); - } - pub(crate) fn force_size(&mut self, size: Size) { self.forced_size = Some(size); } @@ -147,26 +130,48 @@ impl Backend for ColorCompatBackend { render_debug.record(viewport, &adapted); } // #3029: Emit OSC 8 hyperlinks out-of-band through the backend's - // Write impl. ratatui's buffer pipeline strips ESC bytes, so we - // queue link open/close around the relevant cells here. - let frame_links = crate::tui::osc8::take_frame_links(); - let link_active = !frame_links.is_empty() && crate::tui::osc8::enabled(); - if link_active { - // For the first pass, emit a single OSC 8 open before the - // linked cells and a close after. Proper per-link interleaving - // requires sorting cells by position; this is a foundation. - for link in &frame_links { - let _ = crate::tui::osc8::write_osc8_open(self, &link.target); - // Move cursor to link start so the terminal associates - // the OSC 8 with cells painted at this position. - let _ = self.inner.set_cursor_position((link.col_start, link.row)); - } + // Write impl. ratatui's buffer pipeline strips ESC bytes, so the + // open/close sequences must be interleaved with the cell stream + // here. OSC 8 is stateful and last-writer-wins: every cell painted + // between an open and the next close links to that open's target, + // so each region's cells must be bracketed by their OWN open/close + // pair — never batched. + let mut frame_links = crate::tui::osc8::take_frame_links(); + if frame_links.is_empty() || !crate::tui::osc8::enabled() { + self.inner + .draw(adapted.iter().map(|(x, y, cell)| (*x, *y, cell)))?; + return Ok(()); } - self.inner - .draw(adapted.iter().map(|(x, y, cell)| (*x, *y, cell)))?; - if link_active { - for _link in &frame_links { - let _ = crate::tui::osc8::write_osc8_close(self); + // Deterministic region lookup when regions are adjacent/overlapping: + // the first (top-left-most) region wins. + frame_links.sort_unstable_by_key(|link| (link.row, link.col_start)); + let region_for = |x: u16, y: u16| -> Option { + frame_links + .iter() + .position(|link| y == link.row && x >= link.col_start && x <= link.col_end) + }; + + // Walk the diff in its original order and split it into runs at + // region boundaries, so the visible byte stream stays identical to + // a no-link render apart from the inserted OSC 8 sequences. + let mut idx = 0; + while idx < adapted.len() { + let current_region = region_for(adapted[idx].0, adapted[idx].1); + let run_start = idx; + while idx < adapted.len() + && region_for(adapted[idx].0, adapted[idx].1) == current_region + { + idx += 1; + } + let run = &adapted[run_start..idx]; + if let Some(region_idx) = current_region { + crate::tui::osc8::write_osc8_open(self, &frame_links[region_idx].target)?; + self.inner + .draw(run.iter().map(|(x, y, cell)| (*x, *y, cell)))?; + crate::tui::osc8::write_osc8_close(self)?; + } else { + self.inner + .draw(run.iter().map(|(x, y, cell)| (*x, *y, cell)))?; } } Ok(()) @@ -579,4 +584,128 @@ mod tests { backend.force_size(Size::new(80, 25)); assert_eq!(backend.size().unwrap(), Size::new(80, 25)); } + + // ── #3029: OSC 8 emission through the backend byte stream ────────────── + + fn row_cells(symbols: &str) -> Vec<(u16, u16, Cell)> { + symbols + .chars() + .enumerate() + .map(|(i, ch)| { + let mut cell = Cell::default(); + cell.set_symbol(&ch.to_string()); + (u16::try_from(i).unwrap(), 0u16, cell) + }) + .collect() + } + + #[test] + fn osc8_open_close_bracket_only_their_region_cells() { + use crate::tui::osc8::LinkRegion; + + // Baseline: identical cells, no link regions. + let baseline_writer = SharedWriter::default(); + let baseline_capture = baseline_writer.0.clone(); + let mut baseline = + ColorCompatBackend::new(baseline_writer, ColorDepth::TrueColor, PaletteMode::Dark); + let cells = row_cells("ABCDE"); + baseline + .draw(cells.iter().map(|(x, y, cell)| (*x, *y, cell))) + .unwrap(); + let baseline_out = String::from_utf8_lossy(&baseline_capture.borrow()).to_string(); + + // Linked render: columns 2..=3 ("CD") carry one link region. + crate::tui::osc8::set_frame_links(vec![LinkRegion { + row: 0, + col_start: 2, + col_end: 3, + target: "https://example.test/1".to_string(), + }]); + let writer = SharedWriter::default(); + let capture = writer.0.clone(); + let mut backend = ColorCompatBackend::new(writer, ColorDepth::TrueColor, PaletteMode::Dark); + let cells = row_cells("ABCDE"); + backend + .draw(cells.iter().map(|(x, y, cell)| (*x, *y, cell))) + .unwrap(); + let out = String::from_utf8_lossy(&capture.borrow()).to_string(); + + let open = "\x1b]8;;https://example.test/1\x1b\\"; + let close = "\x1b]8;;\x1b\\"; + assert_eq!(out.matches(open).count(), 1, "exactly one open: {out:?}"); + assert_eq!(out.matches(close).count(), 1, "exactly one close: {out:?}"); + + // The open must precede the first linked glyph and the close must sit + // between the last linked glyph and the first glyph after the region. + let open_at = out.find(open).expect("open present"); + let close_at = out.find(close).expect("close present"); + let c_at = out.find('C').expect("glyph C"); + let d_at = out.find('D').expect("glyph D"); + let e_at = out.find('E').expect("glyph E"); + assert!(open_at < c_at, "open before linked cells: {out:?}"); + assert!(d_at < close_at, "close after linked cells: {out:?}"); + assert!( + close_at < e_at, + "cells after the region must not inherit the link: {out:?}" + ); + + // Visible glyph stream is unchanged by link insertion. + let mut baseline_visible = String::new(); + crate::tui::osc8::strip_ansi_into(&baseline_out, &mut baseline_visible); + let mut linked_visible = String::new(); + crate::tui::osc8::strip_ansi_into(&out, &mut linked_visible); + assert_eq!( + baseline_visible, linked_visible, + "link emission must not move or alter visible cells" + ); + } + + #[test] + fn osc8_two_regions_link_to_their_own_targets() { + use crate::tui::osc8::LinkRegion; + + crate::tui::osc8::set_frame_links(vec![ + LinkRegion { + row: 0, + col_start: 0, + col_end: 1, + target: "https://example.test/first".to_string(), + }, + LinkRegion { + row: 0, + col_start: 3, + col_end: 4, + target: "https://example.test/second".to_string(), + }, + ]); + let writer = SharedWriter::default(); + let capture = writer.0.clone(); + let mut backend = ColorCompatBackend::new(writer, ColorDepth::TrueColor, PaletteMode::Dark); + let cells = row_cells("ABZCD"); + backend + .draw(cells.iter().map(|(x, y, cell)| (*x, *y, cell))) + .unwrap(); + let out = String::from_utf8_lossy(&capture.borrow()).to_string(); + + let first = "\x1b]8;;https://example.test/first\x1b\\"; + let second = "\x1b]8;;https://example.test/second\x1b\\"; + let close = "\x1b]8;;\x1b\\"; + assert_eq!(out.matches(first).count(), 1, "{out:?}"); + assert_eq!(out.matches(second).count(), 1, "{out:?}"); + assert_eq!(out.matches(close).count(), 2, "{out:?}"); + + // Pre-#3029-audit bug: both opens were emitted before any cell, so + // the whole frame linked to the LAST region's target. Each region's + // open must close before the next region's open begins. + let first_at = out.find(first).expect("first open"); + let first_close_at = out[first_at..].find(close).expect("first close") + first_at; + let second_at = out.find(second).expect("second open"); + assert!( + first_close_at < second_at, + "region one must close before region two opens: {out:?}" + ); + // The unlinked middle glyph sits between the two link spans. + let z_at = out.find('Z').expect("unlinked glyph"); + assert!(first_close_at < z_at && z_at < second_at, "{out:?}"); + } } diff --git a/crates/tui/src/tui/osc8.rs b/crates/tui/src/tui/osc8.rs index 933708e5..b13e9834 100644 --- a/crates/tui/src/tui/osc8.rs +++ b/crates/tui/src/tui/osc8.rs @@ -28,7 +28,6 @@ const OSC8_CLOSE: &str = "\x1b]8;;\x1b\\"; pub struct LinkRegion { pub row: u16, pub col_start: u16, - #[allow(dead_code)] // used by future buffer-scan link detection pub col_end: u16, pub target: String, }