Migrate ExecutorFileSystem remove to PathUri

This commit is contained in:
Codex
2026-06-10 22:16:25 +00:00
committed by Adam Perry
parent b76475bc76
commit cd02f0c115
16 changed files with 41 additions and 34 deletions
@@ -156,9 +156,10 @@ impl FsRequestProcessor {
&self,
params: FsRemoveParams,
) -> Result<FsRemoveResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
self.file_system()?
.remove(
&params.path,
&path,
RemoveOptions {
recursive: params.recursive.unwrap_or(true),
force: params.force.unwrap_or(true),
+2 -2
View File
@@ -426,7 +426,7 @@ async fn apply_hunks_to_files(
.with_context(|| format!("Failed to delete file {}", path_abs.display()))?;
if let Err(error) = fs
.remove(
&path_abs,
&path_uri,
RemoveOptions {
recursive: false,
force: false,
@@ -490,7 +490,7 @@ async fn apply_hunks_to_files(
})?;
if let Err(error) = fs
.remove(
&path_abs,
&path_uri,
RemoveOptions {
recursive: false,
force: false,
+1 -1
View File
@@ -70,7 +70,7 @@ impl ExecutorFileSystem for TestFileSystem {
async fn remove(
&self,
_path: &AbsolutePathBuf,
_path: &PathUri,
_remove_options: RemoveOptions,
_sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
+2 -1
View File
@@ -960,10 +960,11 @@ impl TestCodexHarness {
}
pub async fn remove_abs_path(&self, path: &AbsolutePathBuf) -> Result<()> {
let path_uri = PathUri::from_abs_path(path)?;
self.test
.fs()
.remove(
path,
&path_uri,
RemoveOptions {
recursive: false,
force: true,
+2 -2
View File
@@ -1586,7 +1586,7 @@ async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Re
let _ = fs::remove_dir_all(shared_cwd.as_path());
test.fs()
.remove(
&shared_cwd,
&shared_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
@@ -1714,7 +1714,7 @@ index 0000000000000000000000000000000000000000..9c998f7b995a7327177b38a90d138517
let _ = fs::remove_dir_all(shared_cwd.as_path());
test.fs()
.remove(
&shared_cwd,
&shared_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
+12 -12
View File
@@ -169,7 +169,7 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> {
file_system
.remove(
&file_path_abs,
&file_path_uri,
RemoveOptions {
recursive: false,
force: true,
@@ -342,7 +342,7 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> {
test.fs()
.remove(
&remote_cwd,
&remote_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
@@ -546,7 +546,7 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result
test.fs()
.remove(
&remote_cwd,
&PathUri::from_abs_path(&remote_cwd)?,
RemoveOptions {
recursive: true,
force: true,
@@ -632,7 +632,7 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result<
test.fs()
.remove(
&remote_cwd,
&remote_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
@@ -681,7 +681,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
let _ = fs::remove_file(&target_path);
test.fs()
.remove(
&target_path,
&target_path_uri,
RemoveOptions {
recursive: false,
force: true,
@@ -807,7 +807,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
let _ = fs::remove_file(&target_path);
test.fs()
.remove(
&target_path,
&target_path_uri,
RemoveOptions {
recursive: false,
force: true,
@@ -817,7 +817,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> {
.await?;
test.fs()
.remove(
&remote_cwd,
&remote_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
@@ -913,7 +913,7 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm
test.fs()
.remove(
&remote_cwd,
&remote_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
@@ -962,7 +962,7 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> {
file_system
.remove(
&absolute_path(allowed_dir),
&allowed_dir_uri,
RemoveOptions {
recursive: true,
force: true,
@@ -1044,7 +1044,7 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> {
let sandbox = workspace_write_sandbox(allowed_dir.clone());
file_system
.remove(
&absolute_path(symlink_path.clone()),
&PathUri::from_path(&symlink_path)?,
RemoveOptions {
recursive: false,
force: false,
@@ -1068,7 +1068,7 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> {
file_system
.remove(
&absolute_path(root),
&PathUri::from_path(&root)?,
RemoveOptions {
recursive: true,
force: true,
@@ -1141,7 +1141,7 @@ async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> {
file_system
.remove(
&absolute_path(root),
&PathUri::from_path(&root)?,
RemoveOptions {
recursive: true,
force: true,
+1 -1
View File
@@ -651,7 +651,7 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<()
test.fs()
.remove(
&remote_cwd,
&remote_cwd_uri,
RemoveOptions {
recursive: true,
force: true,
+3 -1
View File
@@ -276,9 +276,11 @@ pub(crate) async fn run_direct_request(
}))
}
FsHelperRequest::Remove(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
file_system
.remove(
&params.path,
&path,
RemoveOptions {
recursive: params.recursive.unwrap_or(true),
force: params.force.unwrap_or(true),
@@ -138,7 +138,7 @@ impl ExecutorFileSystem for LocalFileSystem {
async fn remove(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
@@ -226,7 +226,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
async fn remove(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
@@ -355,11 +355,12 @@ impl ExecutorFileSystem for DirectFileSystem {
async fn remove(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
reject_sandbox_context(sandbox)?;
let path = path.to_abs_path()?;
match tokio::fs::symlink_metadata(path.as_path()).await {
Ok(metadata) => {
let file_type = metadata.file_type();
@@ -174,15 +174,16 @@ impl ExecutorFileSystem for RemoteFileSystem {
async fn remove(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
trace!("remote fs remove");
let path = path.to_abs_path()?;
let client = self.client.get().await.map_err(map_remote_error)?;
client
.fs_remove(FsRemoveParams {
path: path.clone(),
path,
recursive: Some(options.recursive),
force: Some(options.force),
sandbox: remote_sandbox_context(sandbox),
@@ -196,7 +196,7 @@ impl ExecutorFileSystem for SandboxedFileSystem {
async fn remove(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
remove_options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
@@ -204,7 +204,7 @@ impl ExecutorFileSystem for SandboxedFileSystem {
self.run_sandboxed(
sandbox,
FsHelperRequest::Remove(FsRemoveParams {
path: path.clone(),
path: path.to_abs_path()?,
recursive: Some(remove_options.recursive),
force: Some(remove_options.force),
sandbox: None,
@@ -175,9 +175,10 @@ impl FileSystemHandler {
) -> Result<FsRemoveResponse, JSONRPCErrorError> {
let recursive = params.recursive.unwrap_or(true);
let force = params.force.unwrap_or(true);
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
self.file_system
.remove(
&params.path,
&path,
RemoveOptions { recursive, force },
params.sandbox.as_ref(),
)
@@ -311,7 +311,7 @@ async fn file_system_remove_removes_directory(
file_system
.remove(
&absolute_path(&directory_path),
&PathUri::from_path(&directory_path)?,
RemoveOptions {
recursive: true,
force: true,
@@ -583,7 +583,7 @@ async fn file_system_remove_removes_symlink_not_target(
let sandbox = workspace_write_sandbox(allowed_dir);
file_system
.remove(
&absolute_path(&symlink_path),
&PathUri::from_path(&symlink_path)?,
RemoveOptions {
recursive: false,
force: false,
@@ -660,7 +660,7 @@ async fn file_system_remove_rejects_symlink_escape(
let sandbox = workspace_write_sandbox(allowed_dir);
let error = match file_system
.remove(
&absolute_path(&requested_path),
&PathUri::from_path(&requested_path)?,
RemoveOptions {
recursive: false,
force: false,
@@ -134,7 +134,7 @@ impl ExecutorFileSystem for SyntheticFileSystem {
async fn remove(
&self,
_path: &AbsolutePathBuf,
_path: &PathUri,
_options: RemoveOptions,
_sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
+1 -1
View File
@@ -185,7 +185,7 @@ pub trait ExecutorFileSystem: Send + Sync {
async fn remove(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
remove_options: RemoveOptions,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()>;