github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Render turn diffs for foreign environment paths (#34654)

Browse Source 
## Why

Turn diffs need to render paths from remote environments whose path convention may differ from the Codex host.

## What changed

- Preserve `PathUri` values through applied patch deltas, display-root discovery, and turn-diff tracking.
- Add host-independent relative-path rendering and normalize Windows drive-letter case in `PathUri`.
- Render paths relative to each environment's repository root while keeping Git diff headers slash-separated.

## Testing

- Cover POSIX, Windows, UNC, encoded-separator, and opaque URI relative paths.
- Verify an applied patch in a remote Windows working directory emits a portable unified diff.

GitOrigin-RevId: 526f70ae36a40627de5496d375310d12295b69bb
This commit is contained in:
Adam Perry @ OpenAI
2026-07-22 02:19:45 +00:00
committed by copyberry
parent 9fce9e13fd
commit 1823c13771
11 changed files with 303 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/apply-patch/src/lib.rs
+9 -10
View File
@@ -223,7 +223,7 @@ impl Default for AppliedPatchDelta {
/// A committed file change, preserved in the order it was applied.
#[derive(Clone, Debug, PartialEq)]
pub struct AppliedPatchChange {
pub path: PathBuf,
pub path: PathUri,
pub change: AppliedPatchFileChange,
}
@@ -237,7 +237,7 @@ pub enum AppliedPatchFileChange {
content: String,
},
Update {
move_path: Option<PathBuf>,
move_path: Option<PathUri>,
old_content: String,
overwritten_move_content: Option<String>,
new_content: String,
@@ -387,7 +387,6 @@ async fn apply_hunks_to_files(
};
}
// TODO(anp): Carry PathUri through committed patch deltas and the turn diff tracker.
for hunk in hunks {
let affected_path = hunk.path().to_path_buf();
let path_uri = hunk.resolve_path(cwd)?;
@@ -406,7 +405,7 @@ async fn apply_hunks_to_files(
.await
);
delta.changes.push(AppliedPatchChange {
path: path_uri.to_path_buf(),
path: path_uri,
change: AppliedPatchFileChange::Add {
content: contents.clone(),
overwritten_content,
@@ -456,7 +455,7 @@ async fn apply_hunks_to_files(
}
if let Some(content) = deleted_content {
delta.changes.push(AppliedPatchChange {
path: path_uri.to_path_buf(),
path: path_uri,
change: AppliedPatchFileChange::Delete { content },
});
}
@@ -486,7 +485,7 @@ async fn apply_hunks_to_files(
);
let dest_write_change_index = delta.changes.len();
delta.changes.push(AppliedPatchChange {
path: dest_uri.to_path_buf(),
path: dest_uri.clone(),
change: AppliedPatchFileChange::Add {
content: new_contents.clone(),
overwritten_content: overwritten_move_content.clone(),
@@ -527,9 +526,9 @@ async fn apply_hunks_to_files(
return Err(error);
}
delta.changes[dest_write_change_index] = AppliedPatchChange {
path: path_uri.to_path_buf(),
path: path_uri,
change: AppliedPatchFileChange::Update {
move_path: Some(dest_uri.to_path_buf()),
move_path: Some(dest_uri),
old_content: original_contents,
overwritten_move_content,
new_content: new_contents,
@@ -546,7 +545,7 @@ async fn apply_hunks_to_files(
))
);
delta.changes.push(AppliedPatchChange {
path: path_uri.to_path_buf(),
path: path_uri,
change: AppliedPatchFileChange::Update {
move_path: None,
old_content: original_contents,
@@ -1156,7 +1155,7 @@ mod tests {
failure.delta(),
&AppliedPatchDelta::new(
vec![AppliedPatchChange {
path: dest.clone(),
path: PathUri::from_host_native_path(&dest).expect("absolute destination path"),
change: AppliedPatchFileChange::Add {
content: "line2\n".to_string(),
overwritten_content: None,
codex-rs/core/src/git_info_tests.rs
+6 -21
View File
@@ -12,7 +12,6 @@ use codex_exec_server::RemoveOptions;
use codex_git_utils::GitInfo;
use codex_git_utils::GitSha;
use codex_git_utils::collect_git_info;
use codex_git_utils::get_git_repo_root_with_fs;
use codex_git_utils::get_has_changes;
use codex_git_utils::git_diff_to_remote;
use codex_git_utils::recent_commits;
@@ -602,21 +601,7 @@ async fn resolve_root_git_project_for_trust_returns_none_outside_repo() {
}
#[tokio::test]
async fn get_git_repo_root_with_fs_detects_gitdir_pointer() {
let tmp = TempDir::new().expect("tempdir");
let proj = tmp.path().join("proj");
let nested = proj.join("nested");
std::fs::create_dir_all(&nested).unwrap();
std::fs::write(proj.join(".git"), "gitdir: /tmp/fake-worktree\n").unwrap();
assert_eq!(
get_git_repo_root_with_fs(LOCAL_FS.as_ref(), &nested.abs()).await,
Some(proj.abs())
);
}
#[tokio::test]
async fn get_git_repo_root_with_fs_starts_at_parent_for_file() {
async fn resolve_root_git_project_for_trust_starts_at_parent_for_file() {
let tmp = TempDir::new().expect("tempdir");
let proj = tmp.path().join("proj");
let nested = proj.join("nested");
@@ -626,13 +611,13 @@ async fn get_git_repo_root_with_fs_starts_at_parent_for_file() {
std::fs::write(&file, "contents").unwrap();
assert_eq!(
get_git_repo_root_with_fs(LOCAL_FS.as_ref(), &file.abs()).await,
resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &file.abs()).await,
Some(proj.abs())
);
}
#[tokio::test]
async fn get_git_repo_root_with_fs_ignores_metadata_errors() {
async fn resolve_root_git_project_for_trust_ignores_metadata_errors() {
let tmp = TempDir::new().expect("tempdir");
let proj = tmp.path().join("proj");
let nested = proj.join("nested");
@@ -643,14 +628,14 @@ async fn get_git_repo_root_with_fs_ignores_metadata_errors() {
};
assert_eq!(
get_git_repo_root_with_fs(&fs, &nested.abs()).await,
resolve_root_git_project_for_trust(&fs, &nested.abs()).await,
Some(proj.abs())
);
}
#[cfg(windows)]
#[tokio::test]
async fn get_git_repo_root_with_fs_supports_windows_namespace_paths() {
async fn resolve_root_git_project_for_trust_supports_windows_namespace_paths() {
let tmp = TempDir::new().expect("tempdir");
let repo = tmp.path().join("repo");
std::fs::create_dir_all(repo.join(".git")).unwrap();
@@ -660,7 +645,7 @@ async fn get_git_repo_root_with_fs_supports_windows_namespace_paths() {
let namespace_nested = namespace_repo.join("nested");
assert_eq!(
get_git_repo_root_with_fs(LOCAL_FS.as_ref(), &namespace_nested.abs()).await,
resolve_root_git_project_for_trust(LOCAL_FS.as_ref(), &namespace_nested.abs()).await,
Some(namespace_repo.abs())
);
}
codex-rs/core/src/session/turn.rs
+20 -15
View File
@@ -1,6 +1,5 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::Ordering;
@@ -79,7 +78,8 @@ use codex_core_skills::injection::InjectedHostSkillPrompts;
use codex_extension_api::TurnInputContext;
use codex_extension_api::TurnInputEnvironment;
use codex_features::Feature;
use codex_git_utils::get_git_repo_root_with_fs;
use codex_file_system::FindUpErrorPolicy;
use codex_file_system::find_nearest_ancestor_with_markers;
use codex_protocol::ResponseItemId;
use codex_protocol::config_types::AutoCompactTokenLimitScope;
use codex_protocol::config_types::ModeKind;
@@ -109,6 +109,7 @@ use codex_protocol::protocol::WarningEvent;
use codex_protocol::user_input::UserInput;
use codex_tools::ToolName;
use codex_tools::filter_request_plugin_install_discoverable_tools_for_client;
use codex_utils_path_uri::PathUri;
use codex_utils_stream_parser::AssistantTextChunk;
use codex_utils_stream_parser::AssistantTextStreamParser;
use codex_utils_stream_parser::ProposedPlanSegment;
@@ -171,7 +172,7 @@ pub(crate) async fn run_turn(
// Keep the exact model-visible state used by this turn and its inline compactions.
let (mut world_state, display_roots) = tokio::join!(
sess.record_context_updates_and_set_reference_context_item(first_step_context.as_ref()),
turn_diff_display_roots(turn_context.as_ref()),
turn_diff_display_roots(first_step_context.as_ref()),
);
let Some((injection_items, explicitly_enabled_connectors)) = build_skills_and_plugins(
@@ -468,19 +469,23 @@ pub(crate) async fn run_turn(
}
#[instrument(level = "trace", skip_all)]
async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, PathBuf)> {
async fn turn_diff_display_roots(step_context: &StepContext) -> Vec<(String, PathUri)> {
let mut display_roots = Vec::new();
for turn_environment in turn_context.environments.turn_environments() {
// TODO(anp): Migrate git-root discovery and diff display roots to PathUri so foreign
// environment roots can participate without host-native conversion.
let Ok(cwd) = turn_environment.cwd().to_abs_path() else {
continue;
};
let root =
get_git_repo_root_with_fs(turn_environment.environment.get_filesystem().as_ref(), &cwd)
.await
.unwrap_or(cwd)
.into_path_buf();
for turn_environment in step_context.environments.turn_environments() {
let cwd = turn_environment.cwd();
// A turn cwd is expected to be a directory. If it is a file, the failed `<cwd>/.git` probe
// is ignored and ancestor search continues from its parent.
let root = find_nearest_ancestor_with_markers(
turn_environment.environment.get_filesystem().as_ref(),
cwd,
vec![".git".to_string()],
FindUpErrorPolicy::Ignore,
/*sandbox*/ None,
)
.await
.ok()
.flatten()
.unwrap_or_else(|| cwd.clone());
display_roots.push((turn_environment.environment_id.clone(), root));
}
display_roots
codex-rs/core/src/turn_diff_tracker.rs
+13 -14
View File
@@ -1,7 +1,5 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
use sha1::digest::Output;
@@ -9,6 +7,7 @@ use sha1::digest::Output;
use codex_apply_patch::AppliedPatchChange;
use codex_apply_patch::AppliedPatchDelta;
use codex_apply_patch::AppliedPatchFileChange;
use codex_utils_path_uri::PathUri;
const ZERO_OID: &str = "0000000000000000000000000000000000000000";
const DEV_NULL: &str = "/dev/null";
@@ -25,14 +24,14 @@ struct TrackedContent {
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
struct TrackedPath {
environment_id: String,
path: PathBuf,
path: PathUri,
}
impl TrackedPath {
fn new(environment_id: &str, path: &Path) -> Self {
fn new(environment_id: &str, path: &PathUri) -> Self {
Self {
environment_id: environment_id.to_string(),
path: path.to_path_buf(),
path: path.clone(),
}
}
}
@@ -49,7 +48,7 @@ struct DiffCacheKey {
/// mutations, without rereading the workspace filesystem.
pub struct TurnDiffTracker {
valid: bool,
display_roots_by_environment: HashMap<String, PathBuf>,
display_roots_by_environment: HashMap<String, PathUri>,
baseline_by_path: HashMap<TrackedPath, TrackedContent>,
current_by_path: HashMap<TrackedPath, TrackedContent>,
origin_by_current_path: HashMap<TrackedPath, TrackedPath>,
@@ -83,7 +82,7 @@ impl TurnDiffTracker {
}
pub fn with_environment_display_roots(
display_roots: impl IntoIterator<Item = (String, PathBuf)>,
display_roots: impl IntoIterator<Item = (String, PathUri)>,
) -> Self {
let mut tracker = Self::new();
tracker.display_roots_by_environment = display_roots.into_iter().collect();
@@ -183,7 +182,7 @@ impl TurnDiffTracker {
}
fn apply_change(&mut self, environment_id: &str, change: &AppliedPatchChange) {
let source_path = TrackedPath::new(environment_id, change.path.as_path());
let source_path = TrackedPath::new(environment_id, &change.path);
match &change.change {
AppliedPatchFileChange::Add {
content,
@@ -197,7 +196,7 @@ impl TurnDiffTracker {
new_content,
} => {
let move_path = move_path
.as_deref()
.as_ref()
.map(|path| TrackedPath::new(environment_id, path));
self.apply_update(
source_path,
@@ -321,8 +320,9 @@ impl TurnDiffTracker {
self.rendered_diff_count
.set(self.rendered_diff_count.get() + 1);
let left_display = self.display_path(left_path);
let right_display = self.display_path(right_path);
// Git diff paths always use `/`, even when the displayed target path is Windows-native.
let left_display = self.display_path(left_path).replace('\\', "/");
let right_display = self.display_path(right_path).replace('\\', "/");
let left_oid = left_content.map_or_else(
|| ZERO_OID.to_string(),
|content| git_blob_oid(content.as_bytes()),
@@ -374,9 +374,8 @@ impl TurnDiffTracker {
let display = self
.display_roots_by_environment
.get(&path.environment_id)
.and_then(|root| path.path.strip_prefix(root).ok())
.unwrap_or(path.path.as_path());
let display = display.display().to_string().replace('\\', "/");
.and_then(|root| path.path.relative_path_from(root))
.unwrap_or_else(|| path.path.inferred_native_path_string());
if self.display_roots_by_environment.len() > 1 && !path.environment_id.is_empty() {
format!("{}/{display}", path.environment_id)
} else {
codex-rs/core/src/turn_diff_tracker_tests.rs
+29 -4
View File
@@ -47,7 +47,10 @@ async fn apply_verified_patch(root: &Path, patch: &str) -> AppliedPatchDelta {
}
fn tracker_with_root(root: &Path) -> TurnDiffTracker {
TurnDiffTracker::with_environment_display_roots([("".to_string(), root.to_path_buf())])
TurnDiffTracker::with_environment_display_roots([(
"".to_string(),
PathUri::from_host_native_path(root).expect("absolute display root"),
)])
}
#[tokio::test]
@@ -111,8 +114,14 @@ async fn tracks_same_absolute_path_across_multiple_environments() {
.await;
let mut tracker = TurnDiffTracker::with_environment_display_roots([
("local".to_string(), dir.path().to_path_buf()),
("remote".to_string(), dir.path().to_path_buf()),
(
"local".to_string(),
PathUri::from_host_native_path(dir.path()).expect("absolute local display root"),
),
(
"remote".to_string(),
PathUri::from_host_native_path(dir.path()).expect("absolute remote display root"),
),
]);
tracker.track_delta("remote", &add);
tracker.track_delta("local", &add);
@@ -138,6 +147,21 @@ index {ZERO_OID}..{right_oid}
assert_eq!(tracker.get_unified_diff(), Some(expected));
}
#[test]
fn displays_foreign_paths_relative_to_their_environment_root() {
let tracker = TurnDiffTracker::with_environment_display_roots([(
"windows".to_string(),
PathUri::parse("file:///C:/workspace/project").expect("valid Windows display root"),
)]);
let path = TrackedPath::new(
"windows",
&PathUri::parse("file:///C:/workspace/project/src/main.rs")
.expect("valid Windows file path"),
);
assert_eq!(tracker.display_path(&path), r"src\main.rs");
}
#[tokio::test]
async fn accumulates_delete() {
let dir = tempdir().expect("tempdir");
@@ -463,7 +487,8 @@ fn large_rewrite_returns_promptly_and_preserves_exact_content() {
.success()
);
let tracker = tracker_with_root(dir.path());
let tracked_path = TrackedPath::new("", &path);
let path_uri = PathUri::from_host_native_path(&path).expect("absolute tracked path");
let tracked_path = TrackedPath::new("", &path_uri);
let started = Instant::now();
let diff = tracker
codex-rs/core/tests/suite/apply_patch_cli.rs
+61 -2
View File
@@ -37,6 +37,7 @@ use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::PathBufExt;
use core_test_support::TestTargetOs;
use core_test_support::assert_regex_match;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
@@ -56,6 +57,7 @@ use core_test_support::test_codex::TestCodexHarness;
use core_test_support::test_codex::local;
use core_test_support::test_codex::test_codex;
use core_test_support::test_codex::turn_permission_fields;
use core_test_support::test_target_os;
use core_test_support::wait_for_event;
use core_test_support::wait_for_event_with_timeout;
use serde_json::json;
@@ -1582,6 +1584,63 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_turn_diff_emits_portable_paths_for_remote_cwd() -> Result<()> {
skip_if_no_network!(Ok(()));
skip_if_no_remote_env!(Ok(()));
let harness = apply_patch_harness().await?;
let test = harness.test();
let codex = test.codex.clone();
let call_id = "apply-foreign-windows-diff";
let file = "nested/foreign.txt";
let patch = format!("*** Begin Patch\n*** Add File: {file}\n+hello\n*** End Patch");
mount_apply_patch(&harness, call_id, &patch, "ok").await;
submit_without_wait(&harness, "emit diff for a foreign Windows cwd").await?;
let mut last_diff = None;
wait_for_event(&codex, |event| match event {
EventMsg::TurnDiff(ev) => {
last_diff = Some(ev.unified_diff.clone());
false
}
EventMsg::TurnComplete(_) => true,
_ => false,
})
.await;
let cwd = &test.executor_environment().selection().cwd;
let file_uri = cwd.join(file)?;
let expected_relative_path = match test_target_os() {
TestTargetOs::Linux | TestTargetOs::MacOs => "nested/foreign.txt",
TestTargetOs::Windows => r"nested\foreign.txt",
};
assert_eq!(
file_uri.relative_path_from(cwd).as_deref(),
Some(expected_relative_path)
);
assert_eq!(
test.fs()
.read_file_text(&file_uri, /*sandbox*/ None)
.await?,
"hello\n"
);
assert_eq!(
last_diff.expect("expected TurnDiff event"),
r#"diff --git a/nested/foreign.txt b/nested/foreign.txt
new file mode 100644
index 0000000000000000000000000000000000000000..ce013625030ba8dba906f756967f9e9ca394464a
--- /dev/null
+++ b/nested/foreign.txt
@@ -0,0 +1 @@
+hello
"#
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Result<()> {
// TODO(anp): Remove after shared-cwd helpers use target-native paths.
@@ -1789,8 +1848,8 @@ async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()>
.await;
let diff = last_diff.expect("expected TurnDiff after two patches");
assert!(diff.contains("agg/a.txt"), "diff missing a.txt");
assert!(diff.contains("agg/b.txt"), "diff missing b.txt");
assert!(diff.contains("agg/a.txt"), "diff missing agg/a.txt: {diff}");
assert!(diff.contains("agg/b.txt"), "diff missing agg/b.txt: {diff}");
// Final content reflects v2 for a.txt
assert!(diff.contains("+v2\n") || diff.contains("v2\n"));
Ok(())
codex-rs/git-utils/src/info.rs
+14 -25
View File
@@ -42,30 +42,6 @@ pub fn get_git_repo_root(base_dir: &Path) -> Option<PathBuf> {
find_ancestor_git_entry(base).map(|(repo_root, _)| repo_root)
}
/// Return the repository root for `cwd` using the provided filesystem.
///
/// This mirrors [`get_git_repo_root`] for local paths, but works when `cwd`
/// only exists inside a selected remote environment.
pub async fn get_git_repo_root_with_fs(
fs: &dyn ExecutorFileSystem,
cwd: &AbsolutePathBuf,
) -> Option<AbsolutePathBuf> {
let cwd_uri = PathUri::from_abs_path(cwd);
let base = match fs.get_metadata(&cwd_uri, /*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => cwd.clone(),
_ => cwd.parent()?,
};
find_nearest_native_ancestor_with_markers(
fs,
&base,
vec![".git".to_string()],
FindUpErrorPolicy::Ignore,
/*sandbox*/ None,
)
.await
.ok()?
}
/// Timeout for git commands to prevent freezing on large repositories
const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5);
const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
@@ -817,7 +793,20 @@ pub async fn resolve_root_git_project_for_trust(
fs: &dyn ExecutorFileSystem,
cwd: &AbsolutePathBuf,
) -> Option<AbsolutePathBuf> {
let repo_root = get_git_repo_root_with_fs(fs, cwd).await?;
let cwd_uri = PathUri::from_abs_path(cwd);
let base = match fs.get_metadata(&cwd_uri, /*sandbox*/ None).await {
Ok(metadata) if metadata.is_directory => cwd.clone(),
_ => cwd.parent()?,
};
let repo_root = find_nearest_native_ancestor_with_markers(
fs,
&base,
vec![".git".to_string()],
FindUpErrorPolicy::Ignore,
/*sandbox*/ None,
)
.await
.ok()??;
let dot_git = repo_root.join(".git");
let dot_git_uri = PathUri::from_abs_path(&dot_git);
if fs
codex-rs/git-utils/src/lib.rs
-1
View File
@@ -35,7 +35,6 @@ pub use info::default_branch_name;
pub use info::get_git_remote_urls;
pub use info::get_git_remote_urls_assume_git_repo;
pub use info::get_git_repo_root;
pub use info::get_git_repo_root_with_fs;
pub use info::get_has_changes;
pub use info::get_head_commit_hash;
pub use info::git_diff_to_remote;
codex-rs/utils/path-uri/src/api_path_string_tests.rs
+1 -1
View File
@@ -135,7 +135,7 @@ const RENDER_CASES: &[RenderCase] = &[
RenderCase::round_trips(
"file:///d:/snowman/%E2%98%83",
PathConvention::Windows,
r"d:\snowman\☃",
r"D:\snowman\☃",
),
RenderCase::round_trips("file:///C:/tmp/", PathConvention::Windows, "C:\\tmp\\"),
RenderCase::round_trips(
codex-rs/utils/path-uri/src/lib.rs
+67 -2
View File
@@ -38,8 +38,9 @@ const BAD_PATH_URI_PREFIX: &str = "file:///%00/bad/path/";
/// created by [`Self::from_abs_path`] are opaque to these lexical operations.
///
/// `file:` paths retain their URI spelling so they can be parsed independently
/// of the current host. A local POSIX `file:` URI can also retain
/// percent-encoded non-UTF-8 bytes for lossless native round trips.
/// of the current host, except that Windows drive letters are canonicalized to
/// uppercase. A local POSIX `file:` URI can also retain percent-encoded non-UTF-8
/// bytes for lossless native round trips.
///
/// Like [VS Code resources], path operations use `/` URI separators on every
/// host. Lexical path operations preserve a URL authority without interpreting
@@ -281,6 +282,43 @@ impl PathUri {
path_segments.starts_with(&base_segments)
}
/// Returns the decoded relative path from `base` to this URI.
///
/// The result uses the separators of the inferred path convention,
/// independently of the current host. Both URIs must use the same inferred
/// path convention and authority, and this URI must be lexically equal to
/// or below `base`. Percent-encoded native path separators fail closed.
/// Opaque fallback URIs created by [`Self::from_abs_path`] are only relative
/// to themselves.
pub fn relative_path_from(&self, base: &Self) -> Option<String> {
if self == base {
return Some(String::new());
}
if decode_bad_path_uri(&self.0).is_some()
|| decode_bad_path_uri(&base.0).is_some()
|| self.0.host_str() != base.0.host_str()
|| self.infer_path_convention() != base.infer_path_convention()
{
return None;
}
let convention = self.infer_path_convention()?;
let path_segments = containment_path_segments(&self.0, convention)?;
let base_segments = containment_path_segments(&base.0, convention)?;
let relative_segments = path_segments.strip_prefix(base_segments.as_slice())?;
let separator = match convention {
PathConvention::Posix => "/",
PathConvention::Windows => "\\",
};
Some(
relative_segments
.iter()
.map(|segment| decode_uri_path(segment))
.collect::<Vec<_>>()
.join(separator),
)
}
/// Lexically resolves native absolute or relative path text against this URI.
///
/// Path text is interpreted using the POSIX or Windows convention inferred
@@ -453,6 +491,7 @@ impl TryFrom<Url> for PathUri {
}
validate_file_url(&url)?;
let url = without_localhost_authority(url);
let url = with_normalized_windows_drive_letter(url);
Ok(Self(url))
}
}
@@ -524,6 +563,32 @@ fn without_localhost_authority(mut url: Url) -> Url {
url
}
fn with_normalized_windows_drive_letter(mut url: Url) -> Url {
if url.host_str().is_some() {
return url;
}
let path = url.path();
let Some(drive_start) = path.bytes().position(|byte| byte != b'/') else {
return url;
};
let Some(drive) = path[drive_start..].split('/').next() else {
return url;
};
if !is_windows_drive_uri_segment(drive) || drive.as_bytes()[0].is_ascii_uppercase() {
return url;
}
let drive_letter = char::from(drive.as_bytes()[0]).to_ascii_uppercase();
let normalized_path = format!(
"{}{drive_letter}{}",
&path[..drive_start],
&path[drive_start + 1..]
);
url.set_path(&normalized_path);
url
}
/// Percent-decodes a URI path when it is valid UTF-8.
///
/// `file:` URLs may contain encoded non-UTF-8 bytes. In that case the encoded
codex-rs/utils/path-uri/src/tests.rs
+83
View File
@@ -66,6 +66,20 @@ fn file_uri_parses_a_windows_path_on_any_host() {
);
}
#[test]
fn file_uri_normalizes_windows_drive_letter_case() {
let lowercase = PathUri::parse("file:///c:/Users/Alice%20Smith/src/main.rs")
.expect("Windows file URI should parse");
let uppercase = PathUri::parse("file:///C:/Users/Alice%20Smith/src/main.rs")
.expect("Windows file URI should parse");
assert_eq!(lowercase, uppercase);
assert_eq!(
lowercase.to_string(),
"file:///C:/Users/Alice%20Smith/src/main.rs"
);
}
#[test]
fn infers_path_conventions_from_uri_shape() {
for (uri, expected) in [
@@ -158,6 +172,75 @@ fn inferred_native_path_string_uses_the_inferred_convention() {
}
}
#[test]
fn relative_path_from_is_host_independent() {
// `file://abc/...` has an authority and is inferred as Windows UNC, while
// `file:///abc/...` is hostless and inferred as POSIX.
for (path, base, expected) in [
(
"file:///home/alice/project/src/a%20file.rs",
"file:///home/alice/project",
Some("src/a file.rs"),
),
(
"file:///c:/Users/Alice/project/src/main.rs",
"file:///C:/Users/Alice/project",
Some(r"src\main.rs"),
),
(
"file://server/share/project/src/main.rs",
"file://server/share/project",
Some(r"src\main.rs"),
),
(
"file:///home/alice/project",
"file:///home/alice/project/",
Some(""),
),
(
"file:///home/alice/project-two/main.rs",
"file:///home/alice/project",
None,
),
(
"file://other/share/project/main.rs",
"file://server/share/project",
None,
),
(
"file:///home/alice/project/src%2Fmain.rs",
"file:///home/alice/project",
None,
),
(
"file:///C:/project/src%5Cmain.rs",
"file:///C:/project",
None,
),
("file:///C:/project/main.rs", "file:///", None),
] {
let path = PathUri::parse(path).expect("valid path URI");
let base = PathUri::parse(base).expect("valid base URI");
assert_eq!(
path.relative_path_from(&base).as_deref(),
expected,
"finding {path} relative to {base}"
);
}
}
#[test]
fn relative_path_from_treats_fallback_uris_as_opaque() {
let path = PathUri::parse("file:///%00/bad/path/YQ").expect("valid fallback URI");
let other = PathUri::parse("file:///%00/bad/path/Yg").expect("valid fallback URI");
let root = PathUri::parse("file:///").expect("valid root URI");
assert_eq!(path.relative_path_from(&path), Some(String::new()));
assert_eq!(path.relative_path_from(&other), None);
assert_eq!(path.relative_path_from(&root), None);
}
#[cfg(windows)]
#[test]
fn file_uri_falls_back_for_windows_prefixes_without_a_uri_representation() {