mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
core: carry PathUri through patch diffs
This commit is contained in:
@@ -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.clone(),
|
||||
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.clone(),
|
||||
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.clone(),
|
||||
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.clone(),
|
||||
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"),
|
||||
change: AppliedPatchFileChange::Add {
|
||||
content: "line2\n".to_string(),
|
||||
overwritten_content: None,
|
||||
|
||||
@@ -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,7 @@ 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_git_utils::get_git_repo_root_uri_with_fs;
|
||||
use codex_protocol::config_types::AutoCompactTokenLimitScope;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
@@ -107,6 +106,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;
|
||||
@@ -460,19 +460,16 @@ 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(turn_context: &TurnContext) -> 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();
|
||||
let cwd = turn_environment.cwd();
|
||||
let root = get_git_repo_root_uri_with_fs(
|
||||
turn_environment.environment.get_filesystem().as_ref(),
|
||||
cwd,
|
||||
)
|
||||
.await
|
||||
.unwrap_or_else(|| cwd.clone());
|
||||
display_roots.push((turn_environment.environment_id.clone(), root));
|
||||
}
|
||||
display_roots
|
||||
|
||||
@@ -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,
|
||||
@@ -374,9 +373,9 @@ 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());
|
||||
let display = display.replace('\\', "/");
|
||||
if self.display_roots_by_environment.len() > 1 && !path.environment_id.is_empty() {
|
||||
format!("{}/{display}", path.environment_id)
|
||||
} else {
|
||||
|
||||
@@ -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 root"),
|
||||
)])
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -110,9 +113,10 @@ async fn tracks_same_absolute_path_across_multiple_environments() {
|
||||
)
|
||||
.await;
|
||||
|
||||
let root = PathUri::from_host_native_path(dir.path()).expect("absolute root");
|
||||
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(), root.clone()),
|
||||
("remote".to_string(), root),
|
||||
]);
|
||||
tracker.track_delta("remote", &add);
|
||||
tracker.track_delta("local", &add);
|
||||
@@ -463,7 +467,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 path");
|
||||
let tracked_path = TrackedPath::new("", &path_uri);
|
||||
|
||||
let started = Instant::now();
|
||||
let diff = tracker
|
||||
|
||||
@@ -6,6 +6,7 @@ use std::path::PathBuf;
|
||||
|
||||
use codex_file_system::ExecutorFileSystem;
|
||||
use codex_file_system::FindUpErrorPolicy;
|
||||
use codex_file_system::find_nearest_ancestor_with_markers;
|
||||
use codex_file_system::find_nearest_native_ancestor_with_markers;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
@@ -65,6 +66,36 @@ pub async fn get_git_repo_root_with_fs(
|
||||
.ok()?
|
||||
}
|
||||
|
||||
/// Return the repository root for a URI `cwd` using the provided filesystem.
|
||||
///
|
||||
/// Native paths retain the lossless traversal behavior of
|
||||
/// [`get_git_repo_root_with_fs`]. Foreign-platform paths stay in URI space so
|
||||
/// their filesystem can be inspected without host reinterpretation.
|
||||
pub async fn get_git_repo_root_uri_with_fs(
|
||||
fs: &dyn ExecutorFileSystem,
|
||||
cwd: &PathUri,
|
||||
) -> Option<PathUri> {
|
||||
if let Ok(native_cwd) = cwd.to_abs_path() {
|
||||
return get_git_repo_root_with_fs(fs, &native_cwd)
|
||||
.await
|
||||
.map(|root| PathUri::from_abs_path(&root));
|
||||
}
|
||||
|
||||
let base = match fs.get_metadata(cwd, /*sandbox*/ None).await {
|
||||
Ok(metadata) if metadata.is_directory => cwd.clone(),
|
||||
_ => cwd.parent()?,
|
||||
};
|
||||
find_nearest_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" };
|
||||
|
||||
@@ -35,6 +35,7 @@ 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_uri_with_fs;
|
||||
pub use info::get_git_repo_root_with_fs;
|
||||
pub use info::get_has_changes;
|
||||
pub use info::get_head_commit_hash;
|
||||
|
||||
Reference in New Issue
Block a user