Skip to content

[codex] make PathUri::from_abs_path infallible #27976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anp-oai merged 3 commits into from
Jun 12, 2026
Merged

[codex] make PathUri::from_abs_path infallible #27976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5beb1d3
[codex] make PathUri::from_abs_path infallible
anp-oai Jun 12, 2026
4b5ca0f
codex: address PR review feedback (#27976)
anp-oai Jun 12, 2026
d4554c3
codex: fix CI failure on PR #27976
anp-oai Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 8 additions & 9 deletions codex-rs/app-server/src/request_processors/fs_processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl FsRequestProcessor {
&self,
params: FsReadFileParams,
) -> Result<FsReadFileResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = PathUri::from_abs_path(&params.path);
let bytes = self
.file_system()?
.read_file(&path, /*sandbox*/ None)
Expand All @@ -85,7 +85,7 @@ impl FsRequestProcessor {
"fs/writeFile requires valid base64 dataBase64: {err}"
))
})?;
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = PathUri::from_abs_path(&params.path);
self.file_system()?
.write_file(&path, bytes, /*sandbox*/ None)
.await
Expand All @@ -97,7 +97,7 @@ impl FsRequestProcessor {
&self,
params: FsCreateDirectoryParams,
) -> Result<FsCreateDirectoryResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = PathUri::from_abs_path(&params.path);
self.file_system()?
.create_directory(
&path,
Expand All @@ -115,7 +115,7 @@ impl FsRequestProcessor {
&self,
params: FsGetMetadataParams,
) -> Result<FsGetMetadataResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = PathUri::from_abs_path(&params.path);
let metadata = self
.file_system()?
.get_metadata(&path, /*sandbox*/ None)
Expand All @@ -134,7 +134,7 @@ impl FsRequestProcessor {
&self,
params: FsReadDirectoryParams,
) -> Result<FsReadDirectoryResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = PathUri::from_abs_path(&params.path);
let entries = self
.file_system()?
.read_directory(&path, /*sandbox*/ None)
Expand All @@ -156,7 +156,7 @@ impl FsRequestProcessor {
&self,
params: FsRemoveParams,
) -> Result<FsRemoveResponse, JSONRPCErrorError> {
let path = PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = PathUri::from_abs_path(&params.path);
self.file_system()?
.remove(
&path,
Expand All @@ -175,9 +175,8 @@ impl FsRequestProcessor {
&self,
params: FsCopyParams,
) -> Result<FsCopyResponse, JSONRPCErrorError> {
let source_path = PathUri::from_abs_path(&params.source_path).map_err(map_fs_error)?;
let destination_path =
PathUri::from_abs_path(&params.destination_path).map_err(map_fs_error)?;
let source_path = PathUri::from_abs_path(&params.source_path);
let destination_path = PathUri::from_abs_path(&params.destination_path);
self.file_system()?
.copy(
&source_path,
Expand Down
12 changes: 1 addition & 11 deletions codex-rs/apply-patch/src/invocation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,17 +186,7 @@ pub async fn verify_apply_patch_args(
);
}
Hunk::DeleteFile { .. } => {
let path_uri = match PathUri::from_abs_path(&path) {
Ok(path_uri) => path_uri,
Err(e) => {
return MaybeApplyPatchVerified::CorrectnessError(
ApplyPatchError::IoError(IoError {
context: format!("Failed to read {}", path.display()),
source: e,
}),
);
}
};
let path_uri = PathUri::from_abs_path(&path);
let content = match fs.read_file_text(&path_uri, sandbox).await {
Ok(content) => content,
Err(e) => {
Expand Down
27 changes: 8 additions & 19 deletions codex-rs/apply-patch/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ async fn apply_hunks_to_files(
for hunk in hunks {
let affected_path = hunk.path().to_path_buf();
let path_abs = hunk.resolve_path(cwd);
let path_uri = PathUri::from_abs_path(&path_abs)?;
let path_uri = PathUri::from_abs_path(&path_abs);
match hunk {
Hunk::AddFile { contents, .. } => {
let overwritten_content =
Expand Down Expand Up @@ -556,7 +556,7 @@ async fn ensure_not_directory(
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> io::Result<()> {
let path_uri = PathUri::from_abs_path(path)?;
let path_uri = PathUri::from_abs_path(path);
let metadata = fs.get_metadata(&path_uri, sandbox).await?;
if metadata.is_directory {
return Err(io::Error::new(
Expand All @@ -573,9 +573,7 @@ async fn remove_failure_was_side_effect_free(
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> bool {
let Ok(path_uri) = PathUri::from_abs_path(path) else {
return false;
};
let path_uri = PathUri::from_abs_path(path);
match expected_content {
Some(expected_content) => fs
.read_file_text(&path_uri, sandbox)
Expand All @@ -592,13 +590,7 @@ async fn read_optional_file_text_for_delta(
exact: &mut bool,
) -> Option<String> {
note_existing_path_delta_support(path, fs, sandbox, exact).await;
let path_uri = match PathUri::from_abs_path(path) {
Ok(path_uri) => path_uri,
Err(_) => {
*exact = false;
return None;
}
};
let path_uri = PathUri::from_abs_path(path);
match fs.read_file_text(&path_uri, sandbox).await {
Ok(content) => Some(content),
Err(source) if source.kind() == io::ErrorKind::NotFound => None,
Expand All @@ -615,10 +607,7 @@ async fn note_existing_path_delta_support(
sandbox: Option<&FileSystemSandboxContext>,
exact: &mut bool,
) {
let Ok(path_uri) = PathUri::from_abs_path(path) else {
*exact = false;
return;
};
let path_uri = PathUri::from_abs_path(path);
match fs.get_metadata(&path_uri, sandbox).await {
Ok(metadata) if metadata.is_file && !metadata.is_symlink => {}
Ok(_) => *exact = false,
Expand All @@ -633,12 +622,12 @@ async fn write_file_with_missing_parent_retry(
contents: Vec<u8>,
sandbox: Option<&FileSystemSandboxContext>,
) -> anyhow::Result<()> {
let path_uri = PathUri::from_abs_path(path_abs)?;
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() {
let parent_uri = PathUri::from_abs_path(&parent_abs)?;
let parent_uri = PathUri::from_abs_path(&parent_abs);
fs.create_directory(
&parent_uri,
CreateDirectoryOptions { recursive: true },
Expand Down Expand Up @@ -676,7 +665,7 @@ async fn derive_new_contents_from_chunks(
fs: &dyn ExecutorFileSystem,
sandbox: Option<&FileSystemSandboxContext>,
) -> std::result::Result<AppliedPatch, ApplyPatchError> {
let path_uri = PathUri::from_abs_path(path_abs)?;
let path_uri = PathUri::from_abs_path(path_abs);
let original_contents = fs.read_file_text(&path_uri, sandbox).await.map_err(|err| {
ApplyPatchError::IoError(IoError {
context: format!("Failed to read file to update {}", path_abs.display()),
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/config/src/loader/layer_io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ pub(super) async fn read_config_from_path(
log_missing_as_info: bool,
strict_config: bool,
) -> io::Result<Option<TomlValue>> {
let path_uri = PathUri::from_abs_path(path)?;
let path_uri = PathUri::from_abs_path(path);
match fs.read_file_text(&path_uri, /*sandbox*/ None).await {
Ok(contents) => match toml::from_str::<TomlValue>(&contents) {
Ok(value) => {
Expand Down
16 changes: 8 additions & 8 deletions codex-rs/config/src/loader/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ async fn load_config_toml_for_required_layer(
strict_config: bool,
create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry,
) -> io::Result<ConfigLayerEntry> {
let toml_file_uri = PathUri::from_abs_path(toml_file)?;
let toml_file_uri = PathUri::from_abs_path(toml_file);
let toml_value = match fs.read_file_text(&toml_file_uri, /*sandbox*/ None).await {
Ok(contents) => {
let config_parent = toml_file.as_path().parent().ok_or_else(|| {
Expand Down Expand Up @@ -569,7 +569,7 @@ pub async fn load_requirements_toml(
fs: &dyn ExecutorFileSystem,
requirements_toml_file: &AbsolutePathBuf,
) -> io::Result<Option<RequirementsLayerEntry>> {
let requirements_toml_file_uri = PathUri::from_abs_path(requirements_toml_file)?;
let requirements_toml_file_uri = PathUri::from_abs_path(requirements_toml_file);
match fs
.read_file_text(&requirements_toml_file_uri, /*sandbox*/ None)
.await
Expand Down Expand Up @@ -1139,7 +1139,7 @@ async fn find_project_root(
for ancestor in cwd.ancestors() {
for marker in project_root_markers {
let marker_path = ancestor.join(marker);
let marker_path_uri = PathUri::from_abs_path(&marker_path)?;
let marker_path_uri = PathUri::from_abs_path(&marker_path);
if fs
.get_metadata(&marker_path_uri, /*sandbox*/ None)
.await
Expand All @@ -1156,15 +1156,15 @@ async fn find_git_checkout_root(
fs: &dyn ExecutorFileSystem,
cwd: &AbsolutePathBuf,
) -> Option<AbsolutePathBuf> {
let cwd_uri = PathUri::from_abs_path(cwd).ok()?;
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()?,
};

for dir in base.ancestors() {
let dot_git = dir.join(".git");
let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?;
let dot_git_uri = PathUri::from_abs_path(&dot_git);
if fs
.get_metadata(&dot_git_uri, /*sandbox*/ None)
.await
Expand Down Expand Up @@ -1217,7 +1217,7 @@ async fn load_project_layers(
let mut startup_warnings = Vec::new();
for dir in dirs {
let dot_codex_abs = dir.join(".codex");
let dot_codex_uri = PathUri::from_abs_path(&dot_codex_abs)?;
let dot_codex_uri = PathUri::from_abs_path(&dot_codex_abs);
if !fs
.get_metadata(&dot_codex_uri, /*sandbox*/ None)
.await
Expand All @@ -1236,7 +1236,7 @@ async fn load_project_layers(
continue;
}
let config_file = dot_codex_abs.join(CONFIG_TOML_FILE);
let config_file_uri = PathUri::from_abs_path(&config_file)?;
let config_file_uri = PathUri::from_abs_path(&config_file);
match fs.read_file_text(&config_file_uri, /*sandbox*/ None).await {
Ok(contents) => {
let config: TomlValue = match toml::from_str(&contents) {
Expand Down Expand Up @@ -1340,7 +1340,7 @@ async fn merge_root_checkout_project_hooks(
return Ok(config);
};
let hooks_config_file = hooks_config_folder.join(CONFIG_TOML_FILE);
let hooks_config_file_uri = PathUri::from_abs_path(&hooks_config_file)?;
let hooks_config_file_uri = PathUri::from_abs_path(&hooks_config_file);
let root_config = match fs
.read_file_text(&hooks_config_file_uri, /*sandbox*/ None)
.await
Expand Down
2 changes: 1 addition & 1 deletion codex-rs/config/src/loader/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl ExecutorFileSystem for TestFileSystem {
Box::pin(async move {
let path = path.to_abs_path()?;
let canonicalized = path.canonicalize()?;
PathUri::from_abs_path(&canonicalized)
Ok(PathUri::from_abs_path(&canonicalized))
})
}

Expand Down
21 changes: 3 additions & 18 deletions codex-rs/core-plugins/src/provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,17 +137,8 @@ async fn resolve_plugin_root(
file_system: &dyn ExecutorFileSystem,
) -> Result<Option<ResolvedPlugin>, ExecutorPluginProviderError> {
let root_id = &selected_root.id;
let CapabilityRootLocation::Environment {
environment_id,
path,
} = &selected_root.location;
let root_uri = PathUri::from_abs_path(&plugin_root).map_err(|err| {
ExecutorPluginProviderError::InvalidRootPath {
root_id: root_id.clone(),
path: path.clone(),
message: err.to_string(),
}
})?;
let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location;
let root_uri = PathUri::from_abs_path(&plugin_root);
let root_metadata = file_system
.get_metadata(&root_uri, /*sandbox*/ None)
.await
Expand All @@ -166,13 +157,7 @@ async fn resolve_plugin_root(
let mut manifest_path = None;
for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS {
let candidate = plugin_root.join(relative_path);
let candidate_uri = PathUri::from_abs_path(&candidate).map_err(|err| {
ExecutorPluginProviderError::InvalidRootPath {
root_id: root_id.clone(),
path: candidate.as_path().to_string_lossy().into_owned(),
message: err.to_string(),
}
})?;
let candidate_uri = PathUri::from_abs_path(&candidate);
match file_system
.get_metadata(&candidate_uri, /*sandbox*/ None)
.await
Expand Down
15 changes: 1 addition & 14 deletions codex-rs/core-skills/src/injection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,20 +76,7 @@ pub async fn build_skill_injections(
let fs = loaded_skills
.and_then(|outcome| outcome.file_system_for_skill(skill))
.unwrap_or_else(|| Arc::clone(&LOCAL_FS));
// Skill metadata may point at a file that is absent, but a path the host
// cannot represent means the configured skill cannot be loaded correctly.
let path = match PathUri::from_abs_path(&skill.path_to_skills_md) {
Ok(path) => path,
Err(err) => {
emit_skill_injected_metric(otel, skill, "error");
result.warnings.push(format!(
"Failed to load skill {name} at {path}: {err:#}",
name = skill.name,
path = skill.path_to_skills_md.display()
));
continue;
}
};
let path = PathUri::from_abs_path(&skill.path_to_skills_md);
match fs.read_file_text(&path, /*sandbox*/ None).await {
Ok(contents) => {
emit_skill_injected_metric(otel, skill, "ok");
Expand Down
Loading
Loading