dgf1988/codewhale
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix(tui): #3029 audit fix — interleave OSC 8 open/close per region in diff order (OSC 8 is last-writer-wins state; batched opens linked the whole frame to the last target); drop cursor-move hack and dead pending_links; add byte-stream bracketing tests

Browse Source 
Co-Authored-By: Claude <noreply@anthropic.com>
https://claude.ai/code/session_018zaP8vUfTAsrE38L6h6fw5
This commit is contained in:
Claude
2026-06-11 02:26:45 +00:00
parent 60e9f706b3
commit 948e42397f
2 changed files with 165 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/tui/src/tui/color_compat.rs
+165 -36
View File
@@ -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<W: Write> {
/// the live crossterm query.
terminal_size: Option<Size>,
render_debug: Option<RenderDebugLog>,
/// 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<LinkRegion>,
}
impl<W: Write> ColorCompatBackend<W> {
@@ -72,20 +66,9 @@ impl<W: Write> ColorCompatBackend<W> {
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<LinkRegion>) {
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<W: Write> Backend for ColorCompatBackend<W> {
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<usize> {
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:?}");
}
}
crates/tui/src/tui/osc8.rs
-1
View File
@@ -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,
}