mirror of
https://github.com/openai/codex.git
synced 2026-10-31 19:28:49 +00:00
Make viewed image paths clickable in the TUI (#58446)
## What changed - Add terminal file hyperlinks to viewed image filenames and transcript paths, including older history loaded through pagination. - Enable links only for local workspaces outside SSH sessions. Require a local regular file with a recognized image extension on both the original and resolved paths, and reject Windows reparse points. - Preserve links when summaries are truncated or transcript paths wrap. ## Testing Add coverage for local versus remote workspace links and for hyperlink destinations and column ranges in compact and expanded image rows. GitOrigin-RevId: a7dff255c280ad4e609644e6f5ef5e130d9a55fb
This commit is contained in:
@@ -244,6 +244,9 @@ impl App {
|
||||
cells.push(Arc::new(completion));
|
||||
}
|
||||
}
|
||||
for cell in cells.iter_mut().filter_map(Arc::get_mut) {
|
||||
self.link_viewed_image(cell);
|
||||
}
|
||||
cells
|
||||
}
|
||||
|
||||
|
||||
@@ -22,7 +22,24 @@ pub(super) struct ThreadUsageStatusHistory {
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub(super) fn insert_history_cell(&mut self, tui: &mut tui::Tui, cell: Box<dyn HistoryCell>) {
|
||||
pub(super) fn link_viewed_image(&self, cell: &mut dyn HistoryCell) {
|
||||
if let Some(image) = cell
|
||||
.as_any_mut()
|
||||
.downcast_mut::<history_cell::ViewImageHistoryCell>()
|
||||
&& !self.app_server_target.uses_remote_workspace()
|
||||
&& crate::projectless::has_only_local_environments(&self.environment_manager)
|
||||
&& !crate::clipboard_copy::is_ssh_session()
|
||||
{
|
||||
image.link_local_image();
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn insert_history_cell(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
mut cell: Box<dyn HistoryCell>,
|
||||
) {
|
||||
self.link_viewed_image(cell.as_mut());
|
||||
// Global deprecations can be delivered again by hidden threads. Scope deduplication to
|
||||
// retained history so clearing or rebuilding a transcript can show the notice again.
|
||||
if let Some(notice) = cell
|
||||
|
||||
@@ -33,6 +33,42 @@ fn render_cell(cell: &impl HistoryCell) -> String {
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn viewed_image_links_follow_workspace_locality() -> Result<()> {
|
||||
let dir = tempfile::tempdir()?;
|
||||
let path = dir.path().join("frame.png");
|
||||
image::RgbaImage::new(1, 1).save(&path)?;
|
||||
let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500")?;
|
||||
let (mut app, _, _) = make_test_app_with_channels().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let local_links = !crate::clipboard_copy::is_ssh_session();
|
||||
for (target, linked) in [
|
||||
(AppServerTarget::Embedded, local_links),
|
||||
(
|
||||
AppServerTarget::LocalDaemon {
|
||||
endpoint: endpoint.clone(),
|
||||
allow_embedded_fallback: false,
|
||||
},
|
||||
local_links,
|
||||
),
|
||||
(AppServerTarget::Remote { endpoint }, false),
|
||||
] {
|
||||
app.app_server_target = target;
|
||||
let cell = history_cell::new_view_image_tool_call(
|
||||
"view".into(),
|
||||
codex_utils_path_uri::LegacyAppPathString::from_path(&path),
|
||||
);
|
||||
app.insert_history_cell(&mut tui, Box::new(cell));
|
||||
let rows = app
|
||||
.transcript_cells
|
||||
.last()
|
||||
.unwrap()
|
||||
.compact_hyperlink_lines(/*width*/ 80);
|
||||
assert_eq!(!rows[0].hyperlinks.is_empty(), linked);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn owned_clear_resets_navigation_and_retains_one_fresh_header() -> Result<()> {
|
||||
let (mut app, _rx, _op_rx) = make_test_app_with_channels().await;
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
use super::*;
|
||||
use crate::line_truncation::truncate_line_with_ellipsis_if_overflow;
|
||||
use crate::terminal_hyperlinks::TerminalHyperlink;
|
||||
use crate::terminal_hyperlinks::adaptive_wrap_hyperlink_lines;
|
||||
use codex_ansi_escape::ansi_escape;
|
||||
use codex_utils_path_uri::LegacyAppPathString;
|
||||
|
||||
@@ -162,6 +164,17 @@ pub(crate) struct ViewImageHistoryCell {
|
||||
activity_id: String,
|
||||
filename: String,
|
||||
path_label: String,
|
||||
link: Option<TerminalHyperlink>,
|
||||
}
|
||||
|
||||
impl ViewImageHistoryCell {
|
||||
pub(crate) fn link_local_image(&mut self) {
|
||||
let start = "Viewed image ".len();
|
||||
let end = start + crate::width::display_width(&self.path_label);
|
||||
self.link = self
|
||||
.image_path()
|
||||
.and_then(|path| TerminalHyperlink::viewed_image(start..end, path.as_path()));
|
||||
}
|
||||
}
|
||||
|
||||
impl HistoryCell for ViewImageHistoryCell {
|
||||
@@ -186,13 +199,36 @@ impl HistoryCell for ViewImageHistoryCell {
|
||||
)]
|
||||
}
|
||||
|
||||
fn display_hyperlink_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
let mut lines = plain_hyperlink_lines(self.display_lines(width));
|
||||
if let ([line], Some(link)) = (lines.as_mut_slice(), &self.link) {
|
||||
let start = crate::width::display_width("• Viewed image ");
|
||||
if start < line.width() {
|
||||
line.hyperlinks.push(link.with_columns(start..line.width()));
|
||||
}
|
||||
}
|
||||
lines
|
||||
}
|
||||
|
||||
fn transcript_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
PrefixedWrappedHistoryCell::new(
|
||||
Line::from(vec!["Viewed image ".bold(), self.path_label.clone().dim()]),
|
||||
vec!["• ".dim()],
|
||||
" ",
|
||||
visible_lines(self.transcript_hyperlink_lines(width))
|
||||
}
|
||||
|
||||
fn transcript_hyperlink_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
if width == 0 {
|
||||
return Vec::new();
|
||||
}
|
||||
let mut line = HyperlinkLine::new(Line::from(vec![
|
||||
"Viewed image ".bold(),
|
||||
self.path_label.clone().dim(),
|
||||
]));
|
||||
line.hyperlinks.extend(self.link.iter().cloned());
|
||||
adaptive_wrap_hyperlink_lines(
|
||||
&[line],
|
||||
RtOptions::new(usize::from(width))
|
||||
.initial_indent("• ".dim().into())
|
||||
.subsequent_indent(" ".into()),
|
||||
)
|
||||
.display_lines(width)
|
||||
}
|
||||
|
||||
fn raw_lines(&self) -> Vec<Line<'static>> {
|
||||
@@ -212,6 +248,7 @@ pub(crate) fn new_view_image_tool_call(
|
||||
activity_id: format!("image-view:{id}"),
|
||||
filename,
|
||||
path_label: path.into_string(),
|
||||
link: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -71,11 +71,24 @@ fn viewed_image_retains_original_path_in_details() {
|
||||
|
||||
#[test]
|
||||
fn viewed_image_narrow_summary() {
|
||||
let cell = new_view_image_tool_call(
|
||||
"view".to_string(),
|
||||
LegacyAppPathString::from_string("/workspace/very-long-screenshot-name.png"),
|
||||
);
|
||||
insta::assert_snapshot!(cell.display_lines(/*width*/ 32)[0].to_string());
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("very-long-screenshot #5.png");
|
||||
image::RgbaImage::new(1, 1).save(&path).unwrap();
|
||||
let url = Url::from_file_path(path.canonicalize().unwrap()).unwrap();
|
||||
let mut cell = new_view_image_tool_call("view".into(), LegacyAppPathString::from_path(&path));
|
||||
cell.link_local_image();
|
||||
let compact = cell.compact_hyperlink_lines(/*width*/ 32);
|
||||
let expanded = cell.expanded_hyperlink_lines(u16::MAX);
|
||||
for row in [&compact[0], &expanded[0]] {
|
||||
let [link] = row.hyperlinks.as_slice() else {
|
||||
panic!("expected one image link")
|
||||
};
|
||||
assert_eq!(
|
||||
(link.columns.clone(), link.terminal_destination()),
|
||||
(15..row.width(), Some(url.to_string()))
|
||||
);
|
||||
}
|
||||
insta::assert_snapshot!(compact[0].line.to_string());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -50,7 +50,7 @@ pub(crate) struct TerminalHyperlink {
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
enum DestinationKind {
|
||||
Web,
|
||||
/// A generated visualization or a capability-validated spoken workspace artifact.
|
||||
/// A generated visualization, validated workspace artifact, or locally viewed image.
|
||||
TrustedFile,
|
||||
}
|
||||
|
||||
@@ -113,6 +113,39 @@ impl TerminalHyperlink {
|
||||
self.destination_kind = DestinationKind::TrustedFile;
|
||||
}
|
||||
|
||||
/// A viewed image can live outside the workspace. Only link a local regular file whose
|
||||
/// resolved extension opens as an image (a symlink named screenshot.png may target a script).
|
||||
pub(crate) fn viewed_image(columns: Range<usize>, path: &Path) -> Option<Self> {
|
||||
let url = Url::from_file_path(path).ok()?;
|
||||
if url.host().is_some() || image::ImageFormat::from_path(path).is_err() {
|
||||
return None;
|
||||
}
|
||||
// A Windows junction can lead to a UNC share. Check parents before following any of them.
|
||||
#[cfg(windows)]
|
||||
for ancestor in path.ancestors().collect::<Vec<_>>().into_iter().rev() {
|
||||
use std::os::windows::fs::MetadataExt;
|
||||
use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT;
|
||||
if ancestor.symlink_metadata().ok()?.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT
|
||||
!= 0
|
||||
{
|
||||
return None;
|
||||
}
|
||||
}
|
||||
let target = path.canonicalize().ok()?;
|
||||
let url = Url::from_file_path(&target).ok()?;
|
||||
if url.host().is_some()
|
||||
|| image::ImageFormat::from_path(&target).is_err()
|
||||
|| !target.is_file()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(Self {
|
||||
columns,
|
||||
destination: url.into(),
|
||||
destination_kind: DestinationKind::TrustedFile,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn trusted_workspace_file(
|
||||
columns: Range<usize>,
|
||||
file: TrustedWorkspaceFile,
|
||||
|
||||
Reference in New Issue
Block a user