Migrate ExecutorFileSystem write_file to PathUri

This commit is contained in:
Codex
2026-06-10 22:15:02 +00:00
committed by Adam Perry
parent df0f70f7b3
commit 3f075fccbb
19 changed files with 94 additions and 53 deletions
@@ -85,8 +85,9 @@ impl FsRequestProcessor {
"fs/writeFile requires valid base64 dataBase64: {err}"
))
})?;
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
self.file_system()?
.write_file(&params.path, bytes, /*sandbox*/ None)
.write_file(&path, bytes, /*sandbox*/ None)
.await
.map_err(map_fs_error)?;
Ok(FsWriteFileResponse {})
+4 -3
View File
@@ -523,7 +523,7 @@ async fn apply_hunks_to_files(
modified.push(affected_path);
} else {
try_write!(
fs.write_file(&path_abs, new_contents.clone().into_bytes(), sandbox)
fs.write_file(&path_uri, new_contents.clone().into_bytes(), sandbox)
.await
.with_context(|| format!(
"Failed to write file {}",
@@ -628,7 +628,8 @@ async fn write_file_with_missing_parent_retry(
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> anyhow::Result<()> {
match fs.write_file(path_abs, contents.clone(), sandbox).await {
let path_uri = PathUri::from_abs_path(path_abs)?;
match fs.write_file(&path_uri, contents.clone(), sandbox).await {
Ok(()) => Ok(()),
Err(err) if err.kind() == io::ErrorKind::NotFound => {
if let Some(parent_abs) = path_abs.parent() {
@@ -645,7 +646,7 @@ async fn write_file_with_missing_parent_retry(
)
})?;
}
fs.write_file(path_abs, contents, sandbox)
fs.write_file(&path_uri, contents, sandbox)
.await
.with_context(|| format!("Failed to write file {}", path_abs.display()))?;
Ok(())
+1 -1
View File
@@ -36,7 +36,7 @@ impl ExecutorFileSystem for TestFileSystem {
async fn write_file(
&self,
_path: &AbsolutePathBuf,
_path: &PathUri,
_contents: Vec<u8>,
_sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
+6 -1
View File
@@ -917,9 +917,14 @@ impl TestCodexHarness {
)
.await?;
}
let abs_path_uri = PathUri::from_path(&abs_path)?;
self.test
.fs()
.write_file(&abs_path, contents.as_ref().to_vec(), /*sandbox*/ None)
.write_file(
&abs_path_uri,
contents.as_ref().to_vec(),
/*sandbox*/ None,
)
.await?;
Ok(())
}
+33 -12
View File
@@ -7,6 +7,7 @@ use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::PathBufExt;
use core_test_support::create_directory_symlink;
use core_test_support::load_default_config_for_test;
@@ -112,10 +113,12 @@ async fn agents_override_is_preferred_over_agents_md() -> Result<()> {
agents_instructions(test_codex().with_workspace_setup(|cwd, fs| async move {
let agents_md = cwd.join("AGENTS.md");
let override_md = cwd.join("AGENTS.override.md");
fs.write_file(&agents_md, b"base doc".to_vec(), /*sandbox*/ None)
let agents_md_uri = PathUri::from_path(&agents_md)?;
let override_md_uri = PathUri::from_path(&override_md)?;
fs.write_file(&agents_md_uri, b"base doc".to_vec(), /*sandbox*/ None)
.await?;
fs.write_file(
&override_md,
&override_md_uri,
b"override doc".to_vec(),
/*sandbox*/ None,
)
@@ -146,14 +149,19 @@ async fn configured_fallback_is_used_when_agents_candidate_is_directory() -> Res
.with_workspace_setup(|cwd, fs| async move {
let agents_dir = cwd.join("AGENTS.md");
let fallback = cwd.join("WORKFLOW.md");
let fallback_uri = PathUri::from_path(&fallback)?;
fs.create_directory(
&agents_dir,
CreateDirectoryOptions { recursive: true },
/*sandbox*/ None,
)
.await?;
fs.write_file(&fallback, b"fallback doc".to_vec(), /*sandbox*/ None)
.await?;
fs.write_file(
&fallback_uri,
b"fallback doc".to_vec(),
/*sandbox*/ None,
)
.await?;
Ok::<(), anyhow::Error>(())
}),
)
@@ -183,6 +191,9 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> {
let root_agents = root.join("AGENTS.md");
let git_marker = root.join(".git");
let nested_agents = nested.join("AGENTS.md");
let root_agents_uri = PathUri::from_path(&root_agents)?;
let git_marker_uri = PathUri::from_path(&git_marker)?;
let nested_agents_uri = PathUri::from_path(&nested_agents)?;
fs.create_directory(
&nested,
@@ -190,16 +201,24 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> {
/*sandbox*/ None,
)
.await?;
fs.write_file(&root_agents, b"root doc".to_vec(), /*sandbox*/ None)
.await?;
fs.write_file(
&git_marker,
&root_agents_uri,
b"root doc".to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(
&git_marker_uri,
b"gitdir: /tmp/mock-git-dir\n".to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(&nested_agents, b"child doc".to_vec(), /*sandbox*/ None)
.await?;
fs.write_file(
&nested_agents_uri,
b"child doc".to_vec(),
/*sandbox*/ None,
)
.await?;
Ok::<(), anyhow::Error>(())
}),
)
@@ -314,8 +333,9 @@ async fn selected_environment_sources_match_model_visible_instructions() -> Resu
let mut builder = test_codex()
.with_home(home)
.with_workspace_setup(|cwd, fs| async move {
let agents_md_uri = PathUri::from_path(cwd.join("AGENTS.md"))?;
fs.write_file(
&cwd.join("AGENTS.md"),
&agents_md_uri,
b"project doc".to_vec(),
/*sandbox*/ None,
)
@@ -367,8 +387,9 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re
let mut builder = test_codex()
.with_home(Arc::clone(&home))
.with_workspace_setup(|cwd, fs| async move {
let agents_md_uri = PathUri::from_path(cwd.join("AGENTS.md"))?;
fs.write_file(
&cwd.join("AGENTS.md"),
&agents_md_uri,
PROJECT_INSTRUCTIONS.as_bytes().to_vec(),
/*sandbox*/ None,
)
@@ -392,7 +413,7 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re
)?;
test.fs()
.write_file(
&project_source,
&PathUri::from_path(&project_source)?,
NEW_PROJECT_INSTRUCTIONS.as_bytes().to_vec(),
/*sandbox*/ None,
)
+7 -8
View File
@@ -1337,18 +1337,16 @@ async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nest
)
.await?;
let repo_root = cwd.parent().expect("nested cwd should have parent");
let git_uri = PathUri::from_path(repo_root.join(".git"))?;
let repo_file_uri = PathUri::from_path(repo_root.join("repo.txt"))?;
fs.write_file(
&repo_root.join(".git"),
&git_uri,
b"gitdir: /tmp/fake-worktree\n".to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(
&repo_root.join("repo.txt"),
b"before\n".to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(&repo_file_uri, b"before\n".to_vec(), /*sandbox*/ None)
.await?;
Ok(())
})
})
@@ -1855,8 +1853,9 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()>
let harness = apply_patch_harness_with(|builder| {
builder.with_workspace_setup(|cwd, fs| async move {
let binary_path_uri = PathUri::from_path(cwd.join("binary.dat"))?;
fs.write_file(
&cwd.join("binary.dat"),
&binary_path_uri,
vec![0xff, 0xfe, 0xfd],
/*sandbox*/ None,
)
@@ -1,4 +1,5 @@
use codex_features::Feature;
use codex_utils_path_uri::PathUri;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
@@ -27,7 +28,8 @@ async fn hierarchical_agents_appends_to_project_doc_in_user_instructions() {
})
.with_workspace_setup(|cwd, fs| async move {
let agents_md = cwd.join("AGENTS.md");
fs.write_file(&agents_md, b"be nice".to_vec(), /*sandbox*/ None)
let agents_md_uri = PathUri::from_path(&agents_md)?;
fs.write_file(&agents_md_uri, b"be nice".to_vec(), /*sandbox*/ None)
.await?;
Ok::<(), anyhow::Error>(())
});
+6 -4
View File
@@ -160,7 +160,7 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> {
let payload = b"remote-test-env-ok".to_vec();
file_system
.write_file(&file_path_abs, payload.clone(), /*sandbox*/ None)
.write_file(&file_path_uri, payload.clone(), /*sandbox*/ None)
.await?;
let actual = file_system
.read_file(&file_path_uri, /*sandbox*/ None)
@@ -297,6 +297,7 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> {
))
.abs();
let remote_marker_name = "marker.txt";
let remote_marker_uri = PathUri::from_path(remote_cwd.join(remote_marker_name))?;
test.fs()
.create_directory(
&remote_cwd,
@@ -306,7 +307,7 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> {
.await?;
test.fs()
.write_file(
&remote_cwd.join(remote_marker_name),
&remote_marker_uri,
b"remote-routing".to_vec(),
/*sandbox*/ None,
)
@@ -931,6 +932,7 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> {
let allowed_dir = PathBuf::from(format!("/tmp/codex-remote-readable-{}", std::process::id()));
let file_path = allowed_dir.join("note.txt");
let file_path_uri = PathUri::from_path(&file_path)?;
file_system
.create_directory(
&absolute_path(allowed_dir.clone()),
@@ -940,7 +942,7 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> {
.await?;
file_system
.write_file(
&absolute_path(file_path.clone()),
&file_path_uri,
b"sandboxed hello".to_vec(),
/*sandbox*/ None,
)
@@ -948,7 +950,7 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> {
let sandbox = read_only_sandbox(allowed_dir.clone());
let contents = file_system
.read_file(&PathUri::from_path(&file_path)?, Some(&sandbox))
.read_file(&file_path_uri, Some(&sandbox))
.await?;
assert_eq!(contents, b"sandboxed hello");
+3 -1
View File
@@ -9,6 +9,7 @@ use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Op;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
@@ -37,7 +38,8 @@ async fn write_repo_skill(
.await?;
let contents = format!("---\nname: {name}\ndescription: {description}\n---\n\n{body}\n");
let path = skill_dir.join("SKILL.md");
fs.write_file(&path, contents.into_bytes(), /*sandbox*/ None)
let path_uri = PathUri::from_path(&path)?;
fs.write_file(&path_uri, contents.into_bytes(), /*sandbox*/ None)
.await?;
Ok(())
}
+5 -2
View File
@@ -28,6 +28,7 @@ use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::user_input::UserInput;
use codex_utils_path_uri::PathUri;
use core_test_support::PathBufExt;
use core_test_support::PathExt;
use core_test_support::get_remote_test_env;
@@ -158,8 +159,9 @@ async fn write_workspace_file(
)
.await?;
}
let abs_path_uri = PathUri::from_path(&abs_path)?;
test.fs()
.write_file(&abs_path, contents, /*sandbox*/ None)
.write_file(&abs_path_uri, contents, /*sandbox*/ None)
.await?;
Ok(abs_path.into_path_buf())
}
@@ -585,8 +587,9 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<()
)
.await?;
let png = png_bytes(/*width*/ 1, /*height*/ 1, [0, 255, 0, 255])?;
let image_path_uri = PathUri::from_path(&image_path)?;
test.fs()
.write_file(&image_path, png, /*sandbox*/ None)
.write_file(&image_path_uri, png, /*sandbox*/ None)
.await?;
let remote_selection = TurnEnvironmentSelection {
environment_id: REMOTE_ENVIRONMENT_ID.to_string(),
+3 -1
View File
@@ -200,13 +200,15 @@ pub(crate) async fn run_direct_request(
}))
}
FsHelperRequest::WriteFile(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let bytes = STANDARD.decode(params.data_base64).map_err(|err| {
invalid_request(format!(
"{FS_WRITE_FILE_METHOD} requires valid base64 dataBase64: {err}"
))
})?;
file_system
.write_file(&params.path, bytes, /*sandbox*/ None)
.write_file(&path, bytes, /*sandbox*/ None)
.await
.map_err(map_fs_error)?;
Ok(FsHelperPayload::WriteFile(FsWriteFileResponse {}))
@@ -100,7 +100,7 @@ impl ExecutorFileSystem for LocalFileSystem {
async fn write_file(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
@@ -182,7 +182,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
async fn write_file(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
@@ -288,11 +288,12 @@ impl ExecutorFileSystem for DirectFileSystem {
async fn write_file(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
reject_sandbox_context(sandbox)?;
let path = path.to_abs_path()?;
tokio::fs::write(path.as_path(), contents).await
}
@@ -84,15 +84,16 @@ impl ExecutorFileSystem for RemoteFileSystem {
async fn write_file(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
trace!("remote fs write_file");
let path = path.to_abs_path()?;
let client = self.client.get().await.map_err(map_remote_error)?;
client
.fs_write_file(FsWriteFileParams {
path: path.clone(),
path,
data_base64: STANDARD.encode(contents),
sandbox: remote_sandbox_context(sandbox),
})
@@ -100,7 +100,7 @@ impl ExecutorFileSystem for SandboxedFileSystem {
async fn write_file(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
@@ -108,7 +108,7 @@ impl ExecutorFileSystem for SandboxedFileSystem {
self.run_sandboxed(
sandbox,
FsHelperRequest::WriteFile(FsWriteFileParams {
path: path.clone(),
path: path.to_abs_path()?,
data_base64: STANDARD.encode(contents),
sandbox: None,
}),
@@ -68,13 +68,14 @@ impl FileSystemHandler {
&self,
params: FsWriteFileParams,
) -> Result<FsWriteFileResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let bytes = STANDARD.decode(params.data_base64).map_err(|err| {
invalid_request(format!(
"{FS_WRITE_FILE_METHOD} requires valid base64 dataBase64: {err}"
))
})?;
self.file_system
.write_file(&params.path, bytes, params.sandbox.as_ref())
.write_file(&path, bytes, params.sandbox.as_ref())
.await
.map_err(map_fs_error)?;
Ok(FsWriteFileResponse {})
@@ -120,7 +120,7 @@ async fn file_system_write_file_writes_bytes(
let file_path = tmp.path().join("note.txt");
file_system
.write_file(
&absolute_path(&file_path),
&PathUri::from_path(&file_path)?,
b"hello from trait".to_vec(),
/*sandbox*/ None,
)
@@ -339,7 +339,7 @@ async fn file_system_write_file_reports_missing_parent(
let error = match file_system
.write_file(
&absolute_path(&missing_parent_path),
&PathUri::from_path(&missing_parent_path)?,
b"hello from trait".to_vec(),
/*sandbox*/ None,
)
@@ -517,7 +517,7 @@ async fn file_system_sandboxed_write_allows_additional_write_root(
file_system
.write_file(
&absolute_path(&file_path),
&PathUri::from_path(&file_path)?,
b"created".to_vec(),
Some(&sandbox),
)
@@ -186,7 +186,7 @@ async fn sandboxed_file_system_helper_finds_bwrap_on_preserved_path() -> Result<
file_system
.write_file(
&absolute_path(&file_path),
&PathUri::from_path(&file_path)?,
b"written through fs helper".to_vec(),
Some(&sandbox),
)
@@ -258,7 +258,7 @@ async fn file_system_sandboxed_write_rejects_unwritable_path(
let sandbox = read_only_sandbox(tmp.path().to_path_buf());
let error = match file_system
.write_file(
&absolute_path(&blocked_path),
&PathUri::from_path(&blocked_path)?,
b"nope".to_vec(),
Some(&sandbox),
)
@@ -294,7 +294,7 @@ async fn file_system_sandboxed_write_allows_explicit_alias_roots(
file_system
.write_file(
&absolute_path(&file_path),
&PathUri::from_path(&file_path)?,
b"created".to_vec(),
Some(&sandbox),
)
@@ -391,7 +391,7 @@ async fn file_system_sandboxed_write_rejects_symlink_escape(
let sandbox = workspace_write_sandbox(allowed_dir);
let error = match file_system
.write_file(
&absolute_path(&requested_path),
&PathUri::from_path(&requested_path)?,
b"nope".to_vec(),
Some(&sandbox),
)
@@ -429,7 +429,7 @@ async fn file_system_sandboxed_write_preserves_existing_hard_link(
let sandbox = workspace_write_sandbox(allowed_dir);
file_system
.write_file(
&absolute_path(&hard_link),
&PathUri::from_path(&hard_link)?,
b"updated through existing hard link\n".to_vec(),
Some(&sandbox),
)
@@ -85,7 +85,7 @@ impl ExecutorFileSystem for SyntheticFileSystem {
async fn write_file(
&self,
_path: &AbsolutePathBuf,
_path: &PathUri,
_contents: Vec<u8>,
_sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()> {
+1 -1
View File
@@ -159,7 +159,7 @@ pub trait ExecutorFileSystem: Send + Sync {
async fn write_file(
&self,
path: &AbsolutePathBuf,
path: &PathUri,
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> FileSystemResult<()>;