render local file links from target paths (#13857)

Co-authored-by: Josh McKinney <joshka@openai.com>

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<usize>) -> 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<usize>,
+    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<usize>)
 struct LinkState {
     destination: String,
     show_destination: bool,
-    hidden_location_suffix: Option<String>,
-    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<String>,
 }
 
 fn should_render_link_destination(dest_url: &str) -> bool {
@@ -116,20 +144,6 @@ static HASH_LOCATION_SUFFIX_RE: LazyLock<Regex> =
         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<Item = Event<'a>>,
@@ -148,6 +162,9 @@ where
     code_block_lang: Option<String>,
     code_block_buffer: String,
     wrap_width: Option<usize>,
+    cwd: Option<PathBuf>,
+    line_ends_with_local_link_target: bool,
+    pending_local_link_soft_break: bool,
     current_line_content: Option<Line<'static>>,
     current_initial_indent: Vec<Span<'static>>,
     current_subsequent_indent: Vec<Span<'static>>,
@@ -159,7 +176,7 @@ impl<'a, I> Writer<'a, I>
 where
     I: Iterator<Item = Event<'a>>,
 {
-    fn new(iter: I, wrap_width: Option<usize>) -> Self {
+    fn new(iter: I, wrap_width: Option<usize>, 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::<String>()
-                        })
-                    })
-                    .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<String> {
+    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<String>)> {
+    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<String> {
+    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<String> {
+    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<String> {
+    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");