From da74da6684026d68bbbfc5019411508cac707030 Mon Sep 17 00:00:00 2001 From: pash-openai Date: Tue, 10 Mar 2026 18:00:48 -0700 Subject: [PATCH] render local file links from target paths (#13857) Co-authored-by: Josh McKinney --- codex-rs/tui/src/chatwidget.rs | 17 +- codex-rs/tui/src/history_cell.rs | 76 +++- codex-rs/tui/src/markdown.rs | 23 +- codex-rs/tui/src/markdown_render.rs | 396 ++++++++++++++---- codex-rs/tui/src/markdown_render_tests.rs | 168 ++++++-- codex-rs/tui/src/markdown_stream.rs | 48 ++- ...s__markdown_render_file_link_snapshot.snap | 2 +- codex-rs/tui/src/streaming/controller.rs | 30 +- codex-rs/tui/src/streaming/mod.rs | 19 +- 9 files changed, 619 insertions(+), 160 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 70b524466..cd926590a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1468,6 +1468,7 @@ impl ChatWidget { if self.plan_stream_controller.is_none() { self.plan_stream_controller = Some(PlanStreamController::new( self.last_rendered_width.get().map(|w| w.saturating_sub(4)), + &self.config.cwd, )); } if let Some(controller) = self.plan_stream_controller.as_mut() @@ -1506,7 +1507,7 @@ impl ChatWidget { // TODO: Replace streamed output with the final plan item text if plan streaming is // removed or if we need to reconcile mismatches between streamed and final content. } else if !plan_text.is_empty() { - self.add_to_history(history_cell::new_proposed_plan(plan_text)); + self.add_to_history(history_cell::new_proposed_plan(plan_text, &self.config.cwd)); } if should_restore_after_stream { self.pending_status_indicator_restore = true; @@ -1539,8 +1540,10 @@ impl ChatWidget { // At the end of a reasoning block, record transcript-only content. self.full_reasoning_buffer.push_str(&self.reasoning_buffer); if !self.full_reasoning_buffer.is_empty() { - let cell = - history_cell::new_reasoning_summary_block(self.full_reasoning_buffer.clone()); + let cell = history_cell::new_reasoning_summary_block( + self.full_reasoning_buffer.clone(), + &self.config.cwd, + ); self.add_boxed_history(cell); } self.reasoning_buffer.clear(); @@ -2780,6 +2783,7 @@ impl ChatWidget { } self.stream_controller = Some(StreamController::new( self.last_rendered_width.get().map(|w| w.saturating_sub(2)), + &self.config.cwd, )); } if let Some(controller) = self.stream_controller.as_mut() @@ -5156,7 +5160,12 @@ impl ChatWidget { } else { // Show explanation when there are no structured findings. let mut rendered: Vec> = vec!["".into()]; - append_markdown(&explanation, None, &mut rendered); + append_markdown( + &explanation, + None, + Some(self.config.cwd.as_path()), + &mut rendered, + ); let body_cell = AgentMessageCell::new(rendered, false); self.app_event_tx .send(AppEvent::InsertHistoryCell(Box::new(body_cell))); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index e6feef2cf..3ae4213b8 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -375,14 +375,19 @@ impl HistoryCell for UserHistoryCell { pub(crate) struct ReasoningSummaryCell { _header: String, content: String, + /// Session cwd used to render local file links inside the reasoning body. + cwd: PathBuf, transcript_only: bool, } impl ReasoningSummaryCell { - pub(crate) fn new(header: String, content: String, transcript_only: bool) -> Self { + /// Create a reasoning summary cell that will render local file links relative to the session + /// cwd active when the summary was recorded. + pub(crate) fn new(header: String, content: String, cwd: &Path, transcript_only: bool) -> Self { Self { _header: header, content, + cwd: cwd.to_path_buf(), transcript_only, } } @@ -392,6 +397,7 @@ impl ReasoningSummaryCell { append_markdown( &self.content, Some((width as usize).saturating_sub(2)), + Some(self.cwd.as_path()), &mut lines, ); let summary_style = Style::default().dim().italic(); @@ -997,11 +1003,15 @@ pub(crate) fn padded_emoji(emoji: &str) -> String { #[derive(Debug)] struct TooltipHistoryCell { tip: String, + cwd: PathBuf, } impl TooltipHistoryCell { - fn new(tip: String) -> Self { - Self { tip } + fn new(tip: String, cwd: &Path) -> Self { + Self { + tip, + cwd: cwd.to_path_buf(), + } } } @@ -1016,6 +1026,7 @@ impl HistoryCell for TooltipHistoryCell { append_markdown( &format!("**Tip:** {}", self.tip), Some(wrap_width), + Some(self.cwd.as_path()), &mut lines, ); @@ -1108,7 +1119,7 @@ pub(crate) fn new_session_info( matches!(config.service_tier, Some(ServiceTier::Fast)), ) }) - .map(TooltipHistoryCell::new) + .map(|tip| TooltipHistoryCell::new(tip, &config.cwd)) { parts.push(Box::new(tooltips)); } @@ -2046,8 +2057,12 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlanUpdateCell { PlanUpdateCell { explanation, plan } } -pub(crate) fn new_proposed_plan(plan_markdown: String) -> ProposedPlanCell { - ProposedPlanCell { plan_markdown } +/// Create a proposed-plan cell that snapshots the session cwd for later markdown rendering. +pub(crate) fn new_proposed_plan(plan_markdown: String, cwd: &Path) -> ProposedPlanCell { + ProposedPlanCell { + plan_markdown, + cwd: cwd.to_path_buf(), + } } pub(crate) fn new_proposed_plan_stream( @@ -2063,6 +2078,8 @@ pub(crate) fn new_proposed_plan_stream( #[derive(Debug)] pub(crate) struct ProposedPlanCell { plan_markdown: String, + /// Session cwd used to keep local file-link display aligned with live streamed plan rendering. + cwd: PathBuf, } #[derive(Debug)] @@ -2081,7 +2098,12 @@ impl HistoryCell for ProposedPlanCell { let plan_style = proposed_plan_style(); let wrap_width = width.saturating_sub(4).max(1) as usize; let mut body: Vec> = Vec::new(); - append_markdown(&self.plan_markdown, Some(wrap_width), &mut body); + append_markdown( + &self.plan_markdown, + Some(wrap_width), + Some(self.cwd.as_path()), + &mut body, + ); if body.is_empty() { body.push(Line::from("(empty)".dim().italic())); } @@ -2231,7 +2253,15 @@ pub(crate) fn new_image_generation_call( PlainHistoryCell { lines } } -pub(crate) fn new_reasoning_summary_block(full_reasoning_buffer: String) -> Box { +/// Create the reasoning history cell emitted at the end of a reasoning block. +/// +/// The helper snapshots `cwd` into the returned cell so local file links render the same way they +/// did while the turn was live, even if rendering happens after other app state has advanced. +pub(crate) fn new_reasoning_summary_block( + full_reasoning_buffer: String, + cwd: &Path, +) -> Box { + let cwd = cwd.to_path_buf(); let full_reasoning_buffer = full_reasoning_buffer.trim(); if let Some(open) = full_reasoning_buffer.find("**") { let after_open = &full_reasoning_buffer[(open + 2)..]; @@ -2242,9 +2272,12 @@ pub(crate) fn new_reasoning_summary_block(full_reasoning_buffer: String) -> Box< if after_close_idx < full_reasoning_buffer.len() { let header_buffer = full_reasoning_buffer[..after_close_idx].to_string(); let summary_buffer = full_reasoning_buffer[after_close_idx..].to_string(); + // Preserve the session cwd so local file links render the same way in the + // collapsed reasoning block as they did while streaming live content. return Box::new(ReasoningSummaryCell::new( header_buffer, summary_buffer, + &cwd, false, )); } @@ -2253,6 +2286,7 @@ pub(crate) fn new_reasoning_summary_block(full_reasoning_buffer: String) -> Box< Box::new(ReasoningSummaryCell::new( "".to_string(), full_reasoning_buffer.to_string(), + &cwd, true, )) } @@ -2468,6 +2502,12 @@ mod tests { .expect("config") } + fn test_cwd() -> PathBuf { + // These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or + // Windows-specific root semantics into the fixtures. + std::env::temp_dir() + } + fn render_lines(lines: &[Line<'static>]) -> Vec { lines .iter() @@ -3999,6 +4039,7 @@ mod tests { fn reasoning_summary_block() { let cell = new_reasoning_summary_block( "**High level reasoning**\n\nDetailed reasoning goes here.".to_string(), + &test_cwd(), ); let rendered_display = render_lines(&cell.display_lines(80)); @@ -4014,6 +4055,7 @@ mod tests { let cell: Box = Box::new(ReasoningSummaryCell::new( "High level reasoning".to_string(), summary.to_string(), + &test_cwd(), false, )); let width: u16 = 24; @@ -4054,7 +4096,8 @@ mod tests { #[test] fn reasoning_summary_block_returns_reasoning_cell_when_feature_disabled() { - let cell = new_reasoning_summary_block("Detailed reasoning goes here.".to_string()); + let cell = + new_reasoning_summary_block("Detailed reasoning goes here.".to_string(), &test_cwd()); let rendered = render_transcript(cell.as_ref()); assert_eq!(rendered, vec!["• Detailed reasoning goes here."]); @@ -4067,6 +4110,7 @@ mod tests { config.model_supports_reasoning_summaries = Some(true); let cell = new_reasoning_summary_block( "**High level reasoning**\n\nDetailed reasoning goes here.".to_string(), + &test_cwd(), ); let rendered_display = render_lines(&cell.display_lines(80)); @@ -4075,8 +4119,10 @@ mod tests { #[test] fn reasoning_summary_block_falls_back_when_header_is_missing() { - let cell = - new_reasoning_summary_block("**High level reasoning without closing".to_string()); + let cell = new_reasoning_summary_block( + "**High level reasoning without closing".to_string(), + &test_cwd(), + ); let rendered = render_transcript(cell.as_ref()); assert_eq!(rendered, vec!["• **High level reasoning without closing"]); @@ -4084,14 +4130,17 @@ mod tests { #[test] fn reasoning_summary_block_falls_back_when_summary_is_missing() { - let cell = - new_reasoning_summary_block("**High level reasoning without closing**".to_string()); + let cell = new_reasoning_summary_block( + "**High level reasoning without closing**".to_string(), + &test_cwd(), + ); let rendered = render_transcript(cell.as_ref()); assert_eq!(rendered, vec!["• High level reasoning without closing"]); let cell = new_reasoning_summary_block( "**High level reasoning without closing**\n\n ".to_string(), + &test_cwd(), ); let rendered = render_transcript(cell.as_ref()); @@ -4102,6 +4151,7 @@ mod tests { fn reasoning_summary_block_splits_header_and_summary_when_present() { let cell = new_reasoning_summary_block( "**High level plan**\n\nWe should fix the bug next.".to_string(), + &test_cwd(), ); let rendered_display = render_lines(&cell.display_lines(80)); diff --git a/codex-rs/tui/src/markdown.rs b/codex-rs/tui/src/markdown.rs index 2ea307066..228febb85 100644 --- a/codex-rs/tui/src/markdown.rs +++ b/codex-rs/tui/src/markdown.rs @@ -1,10 +1,21 @@ use ratatui::text::Line; +use std::path::Path; + +/// Render markdown into `lines` while resolving local file-link display relative to `cwd`. +/// +/// Callers that already know the session working directory should pass it here so streamed and +/// non-streamed rendering show the same relative path text even if the process cwd differs. pub(crate) fn append_markdown( markdown_source: &str, width: Option, + cwd: Option<&Path>, lines: &mut Vec>, ) { - let rendered = crate::markdown_render::render_markdown_text_with_width(markdown_source, width); + let rendered = crate::markdown_render::render_markdown_text_with_width_and_cwd( + markdown_source, + width, + cwd, + ); crate::render::line_utils::push_owned_lines(&rendered.lines, lines); } @@ -30,7 +41,7 @@ mod tests { fn citations_render_as_plain_text() { let src = "Before 【F:/x.rs†L1】\nAfter 【F:/x.rs†L3】\n"; let mut out = Vec::new(); - append_markdown(src, None, &mut out); + append_markdown(src, None, None, &mut out); let rendered = lines_to_strings(&out); assert_eq!( rendered, @@ -46,7 +57,7 @@ mod tests { // Basic sanity: indented code with surrounding blank lines should produce the indented line. let src = "Before\n\n code 1\n\nAfter\n"; let mut out = Vec::new(); - append_markdown(src, None, &mut out); + append_markdown(src, None, None, &mut out); let lines = lines_to_strings(&out); assert_eq!(lines, vec!["Before", "", " code 1", "", "After"]); } @@ -55,7 +66,7 @@ mod tests { fn append_markdown_preserves_full_text_line() { let src = "Hi! How can I help with codex-rs today? Want me to explore the repo, run tests, or work on a specific change?\n"; let mut out = Vec::new(); - append_markdown(src, None, &mut out); + append_markdown(src, None, None, &mut out); assert_eq!( out.len(), 1, @@ -76,7 +87,7 @@ mod tests { #[test] fn append_markdown_matches_tui_markdown_for_ordered_item() { let mut out = Vec::new(); - append_markdown("1. Tight item\n", None, &mut out); + append_markdown("1. Tight item\n", None, None, &mut out); let lines = lines_to_strings(&out); assert_eq!(lines, vec!["1. Tight item".to_string()]); } @@ -85,7 +96,7 @@ mod tests { fn append_markdown_keeps_ordered_list_line_unsplit_in_context() { let src = "Loose vs. tight list items:\n1. Tight item\n"; let mut out = Vec::new(); - append_markdown(src, None, &mut out); + append_markdown(src, None, None, &mut out); let lines = lines_to_strings(&out); diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 2bbe19b6f..1b09b84c3 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -1,8 +1,16 @@ +//! Markdown rendering for the TUI transcript. +//! +//! This renderer intentionally treats local file links differently from normal web links. For +//! local paths, the displayed text comes from the destination, not the markdown label, so +//! transcripts show the real file target (including normalized location suffixes) and can shorten +//! absolute paths relative to a known working directory. + use crate::render::highlight::highlight_code_to_lines; use crate::render::line_utils::line_to_static; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; use codex_utils_string::normalize_markdown_hash_location_suffix; +use dirs::home_dir; use pulldown_cmark::CodeBlockKind; use pulldown_cmark::CowStr; use pulldown_cmark::Event; @@ -16,7 +24,10 @@ use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; use regex_lite::Regex; +use std::path::Path; +use std::path::PathBuf; use std::sync::LazyLock; +use url::Url; struct MarkdownStyles { h1: Style, @@ -79,11 +90,26 @@ pub fn render_markdown_text(input: &str) -> Text<'static> { render_markdown_text_with_width(input, None) } +/// Render markdown using the current process working directory for local file-link display. pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) -> Text<'static> { + let cwd = std::env::current_dir().ok(); + render_markdown_text_with_width_and_cwd(input, width, cwd.as_deref()) +} + +/// Render markdown with an explicit working directory for local file links. +/// +/// The `cwd` parameter controls how absolute local targets are shortened before display. Passing +/// the session cwd keeps full renders, history cells, and streamed deltas visually aligned even +/// when rendering happens away from the process cwd. +pub(crate) fn render_markdown_text_with_width_and_cwd( + input: &str, + width: Option, + cwd: Option<&Path>, +) -> Text<'static> { let mut options = Options::empty(); options.insert(Options::ENABLE_STRIKETHROUGH); let parser = Parser::new_ext(input, options); - let mut w = Writer::new(parser, width); + let mut w = Writer::new(parser, width, cwd); w.run(); w.text } @@ -92,9 +118,11 @@ pub(crate) fn render_markdown_text_with_width(input: &str, width: Option) struct LinkState { destination: String, show_destination: bool, - hidden_location_suffix: Option, - label_start_span_idx: usize, - label_styled: bool, + /// Pre-rendered display text for local file links. + /// + /// When this is present, the markdown label is intentionally suppressed so the rendered + /// transcript always reflects the real target path. + local_target_display: Option, } fn should_render_link_destination(dest_url: &str) -> bool { @@ -116,20 +144,6 @@ static HASH_LOCATION_SUFFIX_RE: LazyLock = Err(error) => panic!("invalid hash location regex: {error}"), }); -fn is_local_path_like_link(dest_url: &str) -> bool { - dest_url.starts_with("file://") - || dest_url.starts_with('/') - || dest_url.starts_with("~/") - || dest_url.starts_with("./") - || dest_url.starts_with("../") - || dest_url.starts_with("\\\\") - || matches!( - dest_url.as_bytes(), - [drive, b':', separator, ..] - if drive.is_ascii_alphabetic() && matches!(separator, b'/' | b'\\') - ) -} - struct Writer<'a, I> where I: Iterator>, @@ -148,6 +162,9 @@ where code_block_lang: Option, code_block_buffer: String, wrap_width: Option, + cwd: Option, + line_ends_with_local_link_target: bool, + pending_local_link_soft_break: bool, current_line_content: Option>, current_initial_indent: Vec>, current_subsequent_indent: Vec>, @@ -159,7 +176,7 @@ impl<'a, I> Writer<'a, I> where I: Iterator>, { - fn new(iter: I, wrap_width: Option) -> Self { + fn new(iter: I, wrap_width: Option, cwd: Option<&Path>) -> Self { Self { iter, text: Text::default(), @@ -175,6 +192,9 @@ where code_block_lang: None, code_block_buffer: String::new(), wrap_width, + cwd: cwd.map(Path::to_path_buf), + line_ends_with_local_link_target: false, + pending_local_link_soft_break: false, current_line_content: None, current_initial_indent: Vec::new(), current_subsequent_indent: Vec::new(), @@ -191,6 +211,7 @@ where } fn handle_event(&mut self, event: Event<'a>) { + self.prepare_for_event(&event); match event { Event::Start(tag) => self.start_tag(tag), Event::End(tag) => self.end_tag(tag), @@ -213,6 +234,23 @@ where } } + fn prepare_for_event(&mut self, event: &Event<'a>) { + if !self.pending_local_link_soft_break { + return; + } + + // Local file links render from the destination at `TagEnd::Link`, so a Markdown soft break + // immediately before a descriptive `: ...` should stay inline instead of splitting the + // list item across two lines. + if matches!(event, Event::Text(text) if text.trim_start().starts_with(':')) { + self.pending_local_link_soft_break = false; + return; + } + + self.pending_local_link_soft_break = false; + self.push_line(Line::default()); + } + fn start_tag(&mut self, tag: Tag<'a>) { match tag { Tag::Paragraph => self.start_paragraph(), @@ -324,6 +362,10 @@ where } fn text(&mut self, text: CowStr<'a>) { + if self.suppressing_local_link_label() { + return; + } + self.line_ends_with_local_link_target = false; if self.pending_marker_line { self.push_line(Line::default()); } @@ -373,6 +415,10 @@ where } fn code(&mut self, code: CowStr<'a>) { + if self.suppressing_local_link_label() { + return; + } + self.line_ends_with_local_link_target = false; if self.pending_marker_line { self.push_line(Line::default()); self.pending_marker_line = false; @@ -382,6 +428,10 @@ where } fn html(&mut self, html: CowStr<'a>, inline: bool) { + if self.suppressing_local_link_label() { + return; + } + self.line_ends_with_local_link_target = false; self.pending_marker_line = false; for (i, line) in html.lines().enumerate() { if self.needs_newline { @@ -398,10 +448,23 @@ where } fn hard_break(&mut self) { + if self.suppressing_local_link_label() { + return; + } + self.line_ends_with_local_link_target = false; self.push_line(Line::default()); } fn soft_break(&mut self) { + if self.suppressing_local_link_label() { + return; + } + if self.line_ends_with_local_link_target { + self.pending_local_link_soft_break = true; + self.line_ends_with_local_link_target = false; + return; + } + self.line_ends_with_local_link_target = false; self.push_line(Line::default()); } @@ -513,36 +576,13 @@ where fn push_link(&mut self, dest_url: String) { let show_destination = should_render_link_destination(&dest_url); - let label_styled = !show_destination; - let label_start_span_idx = self - .current_line_content - .as_ref() - .map(|line| line.spans.len()) - .unwrap_or(0); - if label_styled { - self.push_inline_style(self.styles.code); - } self.link = Some(LinkState { show_destination, - hidden_location_suffix: if is_local_path_like_link(&dest_url) { - dest_url - .rsplit_once('#') - .and_then(|(_, fragment)| { - HASH_LOCATION_SUFFIX_RE - .is_match(fragment) - .then(|| format!("#{fragment}")) - }) - .and_then(|suffix| normalize_markdown_hash_location_suffix(&suffix)) - .or_else(|| { - COLON_LOCATION_SUFFIX_RE - .find(&dest_url) - .map(|m| m.as_str().to_string()) - }) + local_target_display: if is_local_path_like_link(&dest_url) { + render_local_link_target(&dest_url, self.cwd.as_deref()) } else { None }, - label_start_span_idx, - label_styled, destination: dest_url, }); } @@ -550,43 +590,34 @@ where fn pop_link(&mut self) { if let Some(link) = self.link.take() { if link.show_destination { - if link.label_styled { - self.pop_inline_style(); - } self.push_span(" (".into()); self.push_span(Span::styled(link.destination, self.styles.link)); self.push_span(")".into()); - } else if let Some(location_suffix) = link.hidden_location_suffix.as_deref() { - let label_text = self - .current_line_content - .as_ref() - .and_then(|line| { - line.spans.get(link.label_start_span_idx..).map(|spans| { - spans - .iter() - .map(|span| span.content.as_ref()) - .collect::() - }) - }) - .unwrap_or_default(); - if label_text - .rsplit_once('#') - .is_some_and(|(_, fragment)| HASH_LOCATION_SUFFIX_RE.is_match(fragment)) - || COLON_LOCATION_SUFFIX_RE.find(&label_text).is_some() - { - // The label already carries a location suffix; don't duplicate it. - } else { - self.push_span(Span::styled(location_suffix.to_string(), self.styles.code)); + } else if let Some(local_target_display) = link.local_target_display { + if self.pending_marker_line { + self.push_line(Line::default()); } - if link.label_styled { - self.pop_inline_style(); - } - } else if link.label_styled { - self.pop_inline_style(); + // Local file links are rendered as code-like path text so the transcript shows the + // resolved target instead of arbitrary caller-provided label text. + let style = self + .inline_styles + .last() + .copied() + .unwrap_or_default() + .patch(self.styles.code); + self.push_span(Span::styled(local_target_display, style)); + self.line_ends_with_local_link_target = true; } } } + fn suppressing_local_link_label(&self) -> bool { + self.link + .as_ref() + .and_then(|link| link.local_target_display.as_ref()) + .is_some() + } + fn flush_current_line(&mut self) { if let Some(line) = self.current_line_content.take() { let style = self.current_line_style; @@ -610,6 +641,7 @@ where self.current_initial_indent.clear(); self.current_subsequent_indent.clear(); self.current_line_in_code_block = false; + self.line_ends_with_local_link_target = false; } } @@ -631,6 +663,7 @@ where self.current_line_style = style; self.current_line_content = Some(line); self.current_line_in_code_block = self.in_code_block; + self.line_ends_with_local_link_target = false; self.pending_marker_line = false; } @@ -687,6 +720,223 @@ where } } +fn is_local_path_like_link(dest_url: &str) -> bool { + dest_url.starts_with("file://") + || dest_url.starts_with('/') + || dest_url.starts_with("~/") + || dest_url.starts_with("./") + || dest_url.starts_with("../") + || dest_url.starts_with("\\\\") + || matches!( + dest_url.as_bytes(), + [drive, b':', separator, ..] + if drive.is_ascii_alphabetic() && matches!(separator, b'/' | b'\\') + ) +} + +/// Parse a local link target into normalized path text plus an optional location suffix. +/// +/// This accepts the path shapes Codex emits today: `file://` URLs, absolute and relative paths, +/// `~/...`, Windows paths, and `#L..C..` or `:line:col` suffixes. +fn render_local_link_target(dest_url: &str, cwd: Option<&Path>) -> Option { + let (path_text, location_suffix) = parse_local_link_target(dest_url)?; + let mut rendered = display_local_link_path(&path_text, cwd); + if let Some(location_suffix) = location_suffix { + rendered.push_str(&location_suffix); + } + Some(rendered) +} + +/// Split a local-link destination into `(normalized_path_text, location_suffix)`. +/// +/// The returned path text never includes a trailing `#L..` or `:line[:col]` suffix. Path +/// normalization expands `~/...` when possible and rewrites path separators into display-stable +/// forward slashes. The suffix, when present, is returned separately in normalized markdown form. +/// +/// Returns `None` only when the destination looks like a `file://` URL but cannot be parsed into a +/// local path. Plain path-like inputs always return `Some(...)` even if they are relative. +fn parse_local_link_target(dest_url: &str) -> Option<(String, Option)> { + if dest_url.starts_with("file://") { + let url = Url::parse(dest_url).ok()?; + let path_text = file_url_to_local_path_text(&url)?; + let location_suffix = url + .fragment() + .and_then(normalize_hash_location_suffix_fragment); + return Some((path_text, location_suffix)); + } + + let mut path_text = dest_url; + let mut location_suffix = None; + // Prefer `#L..` style fragments when both forms are present so URLs like `path#L10` do not + // get misparsed as a plain path ending in `:10`. + if let Some((candidate_path, fragment)) = dest_url.rsplit_once('#') + && let Some(normalized) = normalize_hash_location_suffix_fragment(fragment) + { + path_text = candidate_path; + location_suffix = Some(normalized); + } + if location_suffix.is_none() + && let Some(suffix) = extract_colon_location_suffix(path_text) + { + let path_len = path_text.len().saturating_sub(suffix.len()); + path_text = &path_text[..path_len]; + location_suffix = Some(suffix); + } + + Some((expand_local_link_path(path_text), location_suffix)) +} + +/// Normalize a hash fragment like `L12` or `L12C3-L14C9` into the display suffix we render. +/// +/// Returns `None` for fragments that are not location references. This deliberately ignores other +/// `#...` fragments so non-location hashes stay part of the path text. +fn normalize_hash_location_suffix_fragment(fragment: &str) -> Option { + HASH_LOCATION_SUFFIX_RE + .is_match(fragment) + .then(|| format!("#{fragment}")) + .and_then(|suffix| normalize_markdown_hash_location_suffix(&suffix)) +} + +/// Extract a trailing `:line`, `:line:col`, or range suffix from a plain path-like string. +/// +/// The suffix must occur at the end of the input; embedded colons elsewhere in the path are left +/// alone. This is what keeps Windows drive letters like `C:/...` from being misread as locations. +fn extract_colon_location_suffix(path_text: &str) -> Option { + COLON_LOCATION_SUFFIX_RE + .find(path_text) + .filter(|matched| matched.end() == path_text.len()) + .map(|matched| matched.as_str().to_string()) +} + +/// Expand home-relative paths and normalize separators for display. +/// +/// If `~/...` cannot be expanded because the home directory is unavailable, the original text still +/// goes through separator normalization and is returned as-is otherwise. +fn expand_local_link_path(path_text: &str) -> String { + // Expand `~/...` eagerly so home-relative links can participate in the same normalization and + // cwd-relative shortening path as absolute links. + if let Some(rest) = path_text.strip_prefix("~/") + && let Some(home) = home_dir() + { + return normalize_local_link_path_text(&home.join(rest).to_string_lossy()); + } + + normalize_local_link_path_text(path_text) +} + +/// Convert a `file://` URL into the normalized local-path text used for transcript rendering. +/// +/// This prefers `Url::to_file_path()` for standard file URLs. When that rejects Windows-oriented +/// encodings, we reconstruct a display path from the host/path parts so UNC paths and drive-letter +/// URLs still render sensibly. +fn file_url_to_local_path_text(url: &Url) -> Option { + if let Ok(path) = url.to_file_path() { + return Some(normalize_local_link_path_text(&path.to_string_lossy())); + } + + // Fall back to string reconstruction for cases `to_file_path()` rejects, especially UNC-style + // hosts and Windows drive paths encoded in URL form. + let mut path_text = url.path().to_string(); + if let Some(host) = url.host_str() + && !host.is_empty() + && host != "localhost" + { + path_text = format!("//{host}{path_text}"); + } else if matches!( + path_text.as_bytes(), + [b'/', drive, b':', b'/', ..] if drive.is_ascii_alphabetic() + ) { + path_text.remove(0); + } + + Some(normalize_local_link_path_text(&path_text)) +} + +/// Normalize local-path text into the transcript display form. +/// +/// Display normalization is intentionally lexical: it does not touch the filesystem, resolve +/// symlinks, or collapse `.` / `..`. It only converts separators to forward slashes and rewrites +/// UNC-style `\\\\server\\share` inputs into `//server/share` so later prefix checks operate on a +/// stable representation. +fn normalize_local_link_path_text(path_text: &str) -> String { + // Render all local link paths with forward slashes so display and prefix stripping are stable + // across mixed Windows and Unix-style inputs. + if let Some(rest) = path_text.strip_prefix("\\\\") { + format!("//{}", rest.replace('\\', "/").trim_start_matches('/')) + } else { + path_text.replace('\\', "/") + } +} + +fn is_absolute_local_link_path(path_text: &str) -> bool { + path_text.starts_with('/') + || path_text.starts_with("//") + || matches!( + path_text.as_bytes(), + [drive, b':', b'/', ..] if drive.is_ascii_alphabetic() + ) +} + +/// Remove trailing separators from a local path without destroying root semantics. +/// +/// Roots like `/`, `//`, and `C:/` stay intact so callers can still distinguish "the root itself" +/// from "a path under the root". +fn trim_trailing_local_path_separator(path_text: &str) -> &str { + if path_text == "/" || path_text == "//" { + return path_text; + } + if matches!(path_text.as_bytes(), [drive, b':', b'/'] if drive.is_ascii_alphabetic()) { + return path_text; + } + path_text.trim_end_matches('/') +} + +/// Strip `cwd_text` from the start of `path_text` when `path_text` is strictly underneath it. +/// +/// Returns the relative remainder without a leading slash. If the path equals the cwd exactly, this +/// returns `None` so callers can keep rendering the full path instead of collapsing it to an empty +/// string. +fn strip_local_path_prefix<'a>(path_text: &'a str, cwd_text: &str) -> Option<&'a str> { + let path_text = trim_trailing_local_path_separator(path_text); + let cwd_text = trim_trailing_local_path_separator(cwd_text); + if path_text == cwd_text { + return None; + } + + // Treat filesystem roots specially so `/tmp/x` under `/` becomes `tmp/x` instead of being + // left unchanged by the generic prefix-stripping branch. + if cwd_text == "/" || cwd_text == "//" { + return path_text.strip_prefix('/'); + } + + path_text + .strip_prefix(cwd_text) + .and_then(|rest| rest.strip_prefix('/')) +} + +/// Choose the visible path text for a local link after normalization. +/// +/// Relative paths stay relative. Absolute paths are shortened against `cwd` only when they are +/// lexically underneath it; otherwise the absolute path is preserved. This is display logic only, +/// not filesystem canonicalization. +fn display_local_link_path(path_text: &str, cwd: Option<&Path>) -> String { + let path_text = normalize_local_link_path_text(path_text); + if !is_absolute_local_link_path(&path_text) { + return path_text; + } + + if let Some(cwd) = cwd { + // Only shorten absolute paths that are under the provided session cwd; otherwise preserve + // the original absolute target for clarity. + let cwd_text = normalize_local_link_path_text(&cwd.to_string_lossy()); + if let Some(stripped) = strip_local_path_prefix(&path_text, &cwd_text) { + return stripped.to_string(); + } + } + + path_text +} + #[cfg(test)] mod markdown_render_tests { include!("markdown_render_tests.rs"); diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index 998124609..376b80f61 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -3,12 +3,18 @@ use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; use ratatui::text::Text; +use std::path::Path; use crate::markdown_render::COLON_LOCATION_SUFFIX_RE; use crate::markdown_render::HASH_LOCATION_SUFFIX_RE; use crate::markdown_render::render_markdown_text; +use crate::markdown_render::render_markdown_text_with_width_and_cwd; use insta::assert_snapshot; +fn render_markdown_text_for_cwd(input: &str, cwd: &Path) -> Text<'static> { + render_markdown_text_with_width_and_cwd(input, None, Some(cwd)) +} + #[test] fn empty() { assert_eq!(render_markdown_text(""), Text::default()); @@ -661,8 +667,9 @@ fn load_location_suffix_regexes() { #[test] fn file_link_hides_destination() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "[codex-rs/tui/src/markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs)", + Path::new("/Users/example/code/codex"), ); let expected = Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs".cyan()])); assert_eq!(text, expected); @@ -670,97 +677,101 @@ fn file_link_hides_destination() { #[test] fn file_link_appends_line_number_when_label_lacks_it() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter([ - "markdown_render.rs".cyan(), - ":74".cyan(), - ])); + let expected = Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74".cyan()])); assert_eq!(text, expected); } #[test] -fn file_link_uses_label_for_line_number() { - let text = render_markdown_text( - "[markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)", +fn file_link_keeps_absolute_paths_outside_cwd() { + let text = render_markdown_text_for_cwd( + "[README.md:74](/Users/example/code/codex/README.md:74)", + Path::new("/Users/example/code/codex/codex-rs/tui"), ); - let expected = Text::from(Line::from_iter(["markdown_render.rs:74".cyan()])); + let expected = Text::from(Line::from_iter(["/Users/example/code/codex/README.md:74".cyan()])); assert_eq!(text, expected); } #[test] fn file_link_appends_hash_anchor_when_label_lacks_it() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter([ - "markdown_render.rs".cyan(), - ":74:3".cyan(), - ])); + let expected = + Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3".cyan()])); assert_eq!(text, expected); } #[test] -fn file_link_uses_label_for_hash_anchor() { - let text = render_markdown_text( +fn file_link_uses_target_path_for_hash_anchor() { + let text = render_markdown_text_for_cwd( "[markdown_render.rs#L74C3](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter(["markdown_render.rs#L74C3".cyan()])); + let expected = + Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3".cyan()])); assert_eq!(text, expected); } #[test] fn file_link_appends_range_when_label_lacks_it() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter([ - "markdown_render.rs".cyan(), - ":74:3-76:9".cyan(), - ])); + let expected = + Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()])); assert_eq!(text, expected); } #[test] -fn file_link_uses_label_for_range() { - let text = render_markdown_text( +fn file_link_uses_target_path_for_range() { + let text = render_markdown_text_for_cwd( "[markdown_render.rs:74:3-76:9](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter(["markdown_render.rs:74:3-76:9".cyan()])); + let expected = + Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()])); assert_eq!(text, expected); } #[test] fn file_link_appends_hash_range_when_label_lacks_it() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter([ - "markdown_render.rs".cyan(), - ":74:3-76:9".cyan(), - ])); + let expected = + Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()])); assert_eq!(text, expected); } #[test] fn multiline_file_link_label_after_styled_prefix_does_not_panic() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "**bold** plain [foo\nbar](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from_iter([ - Line::from_iter(["bold".bold(), " plain ".into(), "foo".cyan()]), - Line::from_iter(["bar".cyan(), ":74:3".cyan()]), - ]); + let expected = Text::from(Line::from_iter([ + "bold".bold(), + " plain ".into(), + "codex-rs/tui/src/markdown_render.rs:74:3".cyan(), + ])); assert_eq!(text, expected); } #[test] -fn file_link_uses_label_for_hash_range() { - let text = render_markdown_text( +fn file_link_uses_target_path_for_hash_range() { + let text = render_markdown_text_for_cwd( "[markdown_render.rs#L74C3-L76C9](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)", + Path::new("/Users/example/code/codex"), ); - let expected = Text::from(Line::from_iter(["markdown_render.rs#L74C3-L76C9".cyan()])); + let expected = + Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()])); assert_eq!(text, expected); } @@ -778,8 +789,9 @@ fn url_link_shows_destination() { #[test] fn markdown_render_file_link_snapshot() { - let text = render_markdown_text( + let text = render_markdown_text_for_cwd( "See [markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74).", + Path::new("/Users/example/code/codex"), ); let rendered = text .lines @@ -796,6 +808,82 @@ fn markdown_render_file_link_snapshot() { assert_snapshot!(rendered); } +#[test] +fn unordered_list_local_file_link_stays_inline_with_following_text() { + let text = render_markdown_text_with_width_and_cwd( + "- [binary](/Users/example/code/codex/codex-rs/README.md:93): core is the agent/business logic, tui is the terminal UI, exec is the headless automation surface, and cli is the top-level multitool binary.", + Some(72), + Some(Path::new("/Users/example/code/codex")), + ); + let rendered = text + .lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>(); + assert_eq!( + rendered, + vec![ + "- codex-rs/README.md:93: core is the agent/business logic, tui is the", + " terminal UI, exec is the headless automation surface, and cli is the", + " top-level multitool binary.", + ] + ); +} + +#[test] +fn unordered_list_local_file_link_soft_break_before_colon_stays_inline() { + let text = render_markdown_text_with_width_and_cwd( + "- [binary](/Users/example/code/codex/codex-rs/README.md:93)\n : core is the agent/business logic.", + Some(72), + Some(Path::new("/Users/example/code/codex")), + ); + let rendered = text + .lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>(); + assert_eq!( + rendered, + vec!["- codex-rs/README.md:93: core is the agent/business logic.",] + ); +} + +#[test] +fn consecutive_unordered_list_local_file_links_do_not_detach_paths() { + let text = render_markdown_text_with_width_and_cwd( + "- [binary](/Users/example/code/codex/codex-rs/README.md:93)\n : cli is the top-level multitool binary.\n- [expectations](/Users/example/code/codex/codex-rs/core/README.md:1)\n : codex-core owns the real runtime behavior.", + Some(72), + Some(Path::new("/Users/example/code/codex")), + ); + let rendered = text + .lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>(); + assert_eq!( + rendered, + vec![ + "- codex-rs/README.md:93: cli is the top-level multitool binary.", + "- codex-rs/core/README.md:1: codex-core owns the real runtime behavior.", + ] + ); +} + #[test] fn code_block_known_lang_has_syntax_colors() { let text = render_markdown_text("```rust\nfn main() {}\n```\n"); diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 6ac457eee..a18457d6b 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -1,4 +1,6 @@ use ratatui::text::Line; +use std::path::Path; +use std::path::PathBuf; use crate::markdown; @@ -8,14 +10,22 @@ pub(crate) struct MarkdownStreamCollector { buffer: String, committed_line_count: usize, width: Option, + cwd: PathBuf, } impl MarkdownStreamCollector { - pub fn new(width: Option) -> Self { + /// Create a collector that renders markdown using `cwd` for local file-link display. + /// + /// The collector snapshots `cwd` into owned state because stream commits can happen long after + /// construction. The same `cwd` should be reused for the entire stream lifecycle; mixing + /// different working directories within one stream would make the same link render with + /// different path prefixes across incremental commits. + pub fn new(width: Option, cwd: &Path) -> Self { Self { buffer: String::new(), committed_line_count: 0, width, + cwd: cwd.to_path_buf(), } } @@ -41,7 +51,7 @@ impl MarkdownStreamCollector { return Vec::new(); }; let mut rendered: Vec> = Vec::new(); - markdown::append_markdown(&source, self.width, &mut rendered); + markdown::append_markdown(&source, self.width, Some(self.cwd.as_path()), &mut rendered); let mut complete_line_count = rendered.len(); if complete_line_count > 0 && crate::render::line_utils::is_blank_line_spaces_only( @@ -82,7 +92,7 @@ impl MarkdownStreamCollector { tracing::trace!("markdown finalize (raw source):\n---\n{source}\n---"); let mut rendered: Vec> = Vec::new(); - markdown::append_markdown(&source, self.width, &mut rendered); + markdown::append_markdown(&source, self.width, Some(self.cwd.as_path()), &mut rendered); let out = if self.committed_line_count >= rendered.len() { Vec::new() @@ -96,12 +106,19 @@ impl MarkdownStreamCollector { } } +#[cfg(test)] +fn test_cwd() -> PathBuf { + // These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or + // Windows-specific root semantics into the fixtures. + std::env::temp_dir() +} + #[cfg(test)] pub(crate) fn simulate_stream_markdown_for_tests( deltas: &[&str], finalize: bool, ) -> Vec> { - let mut collector = MarkdownStreamCollector::new(None); + let mut collector = MarkdownStreamCollector::new(None, &test_cwd()); let mut out = Vec::new(); for d in deltas { collector.push_delta(d); @@ -122,7 +139,7 @@ mod tests { #[tokio::test] async fn no_commit_until_newline() { - let mut c = super::MarkdownStreamCollector::new(None); + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); c.push_delta("Hello, world"); let out = c.commit_complete_lines(); assert!(out.is_empty(), "should not commit without newline"); @@ -133,7 +150,7 @@ mod tests { #[tokio::test] async fn finalize_commits_partial_line() { - let mut c = super::MarkdownStreamCollector::new(None); + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); c.push_delta("Line without newline"); let out = c.finalize_and_drain(); assert_eq!(out.len(), 1); @@ -253,7 +270,7 @@ mod tests { async fn heading_starts_on_new_line_when_following_paragraph() { // Stream a paragraph line, then a heading on the next line. // Expect two distinct rendered lines: "Hello." and "Heading". - let mut c = super::MarkdownStreamCollector::new(None); + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); c.push_delta("Hello.\n"); let out1 = c.commit_complete_lines(); let s1: Vec = out1 @@ -309,7 +326,7 @@ mod tests { // Paragraph without trailing newline, then a chunk that starts with the newline // and the heading text, then a final newline. The collector should first commit // only the paragraph line, and later commit the heading as its own line. - let mut c = super::MarkdownStreamCollector::new(None); + let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd()); c.push_delta("Sounds good!"); // No commit yet assert!(c.commit_complete_lines().is_empty()); @@ -354,7 +371,8 @@ mod tests { // Sanity check raw markdown rendering for a simple line does not produce spurious extras. let mut rendered: Vec> = Vec::new(); - crate::markdown::append_markdown("Hello.\n", None, &mut rendered); + let test_cwd = super::test_cwd(); + crate::markdown::append_markdown("Hello.\n", None, Some(test_cwd.as_path()), &mut rendered); let rendered_strings: Vec = rendered .iter() .map(|l| { @@ -414,7 +432,8 @@ mod tests { let streamed_str = lines_to_plain_strings(&streamed); let mut rendered_all: Vec> = Vec::new(); - crate::markdown::append_markdown(input, None, &mut rendered_all); + let test_cwd = super::test_cwd(); + crate::markdown::append_markdown(input, None, Some(test_cwd.as_path()), &mut rendered_all); let rendered_all_str = lines_to_plain_strings(&rendered_all); assert_eq!( @@ -520,7 +539,8 @@ mod tests { let full: String = deltas.iter().copied().collect(); let mut rendered_all: Vec> = Vec::new(); - crate::markdown::append_markdown(&full, None, &mut rendered_all); + let test_cwd = super::test_cwd(); + crate::markdown::append_markdown(&full, None, Some(test_cwd.as_path()), &mut rendered_all); let rendered_all_strs = lines_to_plain_strings(&rendered_all); assert_eq!( @@ -608,7 +628,8 @@ mod tests { // Compute a full render for diagnostics only. let full: String = deltas.iter().copied().collect(); let mut rendered_all: Vec> = Vec::new(); - crate::markdown::append_markdown(&full, None, &mut rendered_all); + let test_cwd = super::test_cwd(); + crate::markdown::append_markdown(&full, None, Some(test_cwd.as_path()), &mut rendered_all); // Also assert exact expected plain strings for clarity. let expected = vec![ @@ -635,7 +656,8 @@ mod tests { let streamed_strs = lines_to_plain_strings(&streamed); let full: String = deltas.iter().copied().collect(); let mut rendered: Vec> = Vec::new(); - crate::markdown::append_markdown(&full, None, &mut rendered); + let test_cwd = super::test_cwd(); + crate::markdown::append_markdown(&full, None, Some(test_cwd.as_path()), &mut rendered); let rendered_strs = lines_to_plain_strings(&rendered); assert_eq!(streamed_strs, rendered_strs, "full:\n---\n{full}\n---"); } diff --git a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap index 1b1f1210f..63c42564d 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__markdown_render__markdown_render_tests__markdown_render_file_link_snapshot.snap @@ -3,4 +3,4 @@ source: tui/src/markdown_render_tests.rs assertion_line: 714 expression: rendered --- -See markdown_render.rs:74. +See codex-rs/tui/src/markdown_render.rs:74. diff --git a/codex-rs/tui/src/streaming/controller.rs b/codex-rs/tui/src/streaming/controller.rs index 6117485ad..7f7346265 100644 --- a/codex-rs/tui/src/streaming/controller.rs +++ b/codex-rs/tui/src/streaming/controller.rs @@ -4,6 +4,7 @@ use crate::render::line_utils::prefix_lines; use crate::style::proposed_plan_style; use ratatui::prelude::Stylize; use ratatui::text::Line; +use std::path::Path; use std::time::Duration; use std::time::Instant; @@ -18,9 +19,13 @@ pub(crate) struct StreamController { } impl StreamController { - pub(crate) fn new(width: Option) -> Self { + /// Create a controller whose markdown renderer shortens local file links relative to `cwd`. + /// + /// The controller snapshots the path into stream state so later commit ticks and finalization + /// render against the same session cwd that was active when streaming started. + pub(crate) fn new(width: Option, cwd: &Path) -> Self { Self { - state: StreamState::new(width), + state: StreamState::new(width, cwd), finishing_after_drain: false, header_emitted: false, } @@ -115,9 +120,14 @@ pub(crate) struct PlanStreamController { } impl PlanStreamController { - pub(crate) fn new(width: Option) -> Self { + /// Create a plan-stream controller whose markdown renderer shortens local file links relative + /// to `cwd`. + /// + /// The controller snapshots the path into stream state so later commit ticks and finalization + /// render against the same session cwd that was active when streaming started. + pub(crate) fn new(width: Option, cwd: &Path) -> Self { Self { - state: StreamState::new(width), + state: StreamState::new(width, cwd), header_emitted: false, top_padding_emitted: false, } @@ -232,6 +242,13 @@ impl PlanStreamController { #[cfg(test)] mod tests { use super::*; + use std::path::PathBuf; + + fn test_cwd() -> PathBuf { + // These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or + // Windows-specific root semantics into the fixtures. + std::env::temp_dir() + } fn lines_to_plain_strings(lines: &[ratatui::text::Line<'_>]) -> Vec { lines @@ -248,7 +265,7 @@ mod tests { #[tokio::test] async fn controller_loose_vs_tight_with_commit_ticks_matches_full() { - let mut ctrl = StreamController::new(None); + let mut ctrl = StreamController::new(None, &test_cwd()); let mut lines = Vec::new(); // Exact deltas from the session log (section: Loose vs. tight list items) @@ -346,7 +363,8 @@ mod tests { // Full render of the same source let source: String = deltas.iter().copied().collect(); let mut rendered: Vec> = Vec::new(); - crate::markdown::append_markdown(&source, None, &mut rendered); + let test_cwd = test_cwd(); + crate::markdown::append_markdown(&source, None, Some(test_cwd.as_path()), &mut rendered); let rendered_strs = lines_to_plain_strings(&rendered); assert_eq!(streamed, rendered_strs); diff --git a/codex-rs/tui/src/streaming/mod.rs b/codex-rs/tui/src/streaming/mod.rs index c783f27ae..e39b00e09 100644 --- a/codex-rs/tui/src/streaming/mod.rs +++ b/codex-rs/tui/src/streaming/mod.rs @@ -10,6 +10,7 @@ //! arrival timestamp so policy code can reason about oldest queued age without peeking into text. use std::collections::VecDeque; +use std::path::Path; use std::time::Duration; use std::time::Instant; @@ -33,10 +34,13 @@ pub(crate) struct StreamState { } impl StreamState { - /// Creates an empty stream state with an optional target wrap width. - pub(crate) fn new(width: Option) -> Self { + /// Create stream state whose markdown collector renders local file links relative to `cwd`. + /// + /// Controllers are expected to pass the session cwd here once and keep it stable for the + /// lifetime of the active stream. + pub(crate) fn new(width: Option, cwd: &Path) -> Self { Self { - collector: MarkdownStreamCollector::new(width), + collector: MarkdownStreamCollector::new(width, cwd), queued_lines: VecDeque::new(), has_seen_delta: false, } @@ -102,10 +106,17 @@ impl StreamState { mod tests { use super::*; use pretty_assertions::assert_eq; + use std::path::PathBuf; + + fn test_cwd() -> PathBuf { + // These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or + // Windows-specific root semantics into the fixtures. + std::env::temp_dir() + } #[test] fn drain_n_clamps_to_available_lines() { - let mut state = StreamState::new(None); + let mut state = StreamState::new(None, &test_cwd()); state.enqueue(vec![Line::from("one")]); let drained = state.drain_n(8);