Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/core/src/tools/handlers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ mod search;
pub(crate) mod search_spec;
mod shell;
pub(crate) mod shell_spec;
mod smart_path;
mod smart_read;
pub(crate) mod smart_read_spec;
mod smart_search;
Expand Down
74 changes: 74 additions & 0 deletions codex-rs/core/src/tools/handlers/smart_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use std::path::Component;
use std::path::Path;
use std::path::PathBuf;

use crate::function_tool::FunctionCallError;

pub(super) fn canonicalize_path(path: &Path, label: &str) -> Result<PathBuf, FunctionCallError> {
path.canonicalize().map_err(|err| {
FunctionCallError::RespondToModel(format!(
"unable to resolve {label} `{}`: {err}",
path.display()
))
})
}

pub(super) fn resolve_existing_path(
cwd: &Path,
requested_path: &str,
label: &str,
) -> Result<PathBuf, FunctionCallError> {
let raw_path = Path::new(requested_path);
let primary = path_from_cwd(cwd, raw_path);

match primary.canonicalize() {
Ok(path) => Ok(path),
Err(primary_err) => {
if let Some(fallback) = without_redundant_cwd_prefix(cwd, raw_path)
&& let Ok(path) = fallback.canonicalize()
{
return Ok(path);
}

Err(FunctionCallError::RespondToModel(format!(
"unable to resolve {label} `{}`: {primary_err}",
primary.display()
)))
}
}
}

fn path_from_cwd(cwd: &Path, path: &Path) -> PathBuf {
if path.is_absolute() {
path.to_path_buf()
} else {
cwd.join(path)
}
}

fn without_redundant_cwd_prefix(cwd: &Path, path: &Path) -> Option<PathBuf> {
if path.is_absolute() {
return None;
}

let cwd_name = cwd.file_name()?;
let mut components = path.components();
let first = components.next()?;
let Component::Normal(first) = first else {
return None;
};
if first != cwd_name {
return None;
}

let stripped = components.as_path();
if stripped.as_os_str().is_empty() {
return None;
}

Some(cwd.join(stripped))
}

#[cfg(test)]
#[path = "smart_path_tests.rs"]
mod tests;
50 changes: 50 additions & 0 deletions codex-rs/core/src/tools/handlers/smart_path_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use super::*;
use pretty_assertions::assert_eq;

#[test]
fn resolves_path_with_redundant_cwd_basename_prefix() -> anyhow::Result<()> {
let temp = tempfile::tempdir()?;
let cwd = temp.path().join("codex-rs");
let file = cwd.join("sandboxing").join("mod.rs");
std::fs::create_dir_all(file.parent().expect("test file should have parent"))?;
std::fs::write(&file, "pub mod linux;\n")?;

let resolved = resolve_existing_path(&cwd, "codex-rs/sandboxing/mod.rs", "source file")?;

assert_eq!(resolved, file.canonicalize()?);
Ok(())
}

#[test]
fn resolves_absolute_paths_without_rewriting() -> anyhow::Result<()> {
let temp = tempfile::tempdir()?;
let cwd = temp.path().join("codex-rs");
let file = cwd.join("sandboxing").join("mod.rs");
std::fs::create_dir_all(file.parent().expect("test file should have parent"))?;
std::fs::write(&file, "pub mod linux;\n")?;

let resolved = resolve_existing_path(
&cwd,
file.to_str().expect("test path should be valid utf-8"),
"source file",
)?;

assert_eq!(resolved, file.canonicalize()?);
Ok(())
}

#[test]
fn reports_primary_path_when_resolution_fails() -> anyhow::Result<()> {
let temp = tempfile::tempdir()?;
let cwd = temp.path().join("codex-rs");
std::fs::create_dir_all(&cwd)?;

let error = resolve_existing_path(&cwd, "missing.rs", "source file").unwrap_err();
let FunctionCallError::RespondToModel(message) = error else {
panic!("expected model-facing error");
};

assert!(message.contains("unable to resolve source file"));
assert!(message.contains("codex-rs/missing.rs"));
Ok(())
}
16 changes: 4 additions & 12 deletions codex-rs/core/src/tools/handlers/smart_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::resolve_tool_environment;
use crate::tools::handlers::smart_path::canonicalize_path;
use crate::tools::handlers::smart_path::resolve_existing_path;
use crate::tools::handlers::smart_read_spec::SmartReadToolOptions;
use crate::tools::handlers::smart_read_spec::create_smart_read_tool;
use crate::tools::registry::CoreToolRuntime;
Expand Down Expand Up @@ -104,9 +106,8 @@ impl ToolExecutor<ToolInvocation> for SmartReadHandler {
));
}

let project_root = canonicalize(environment.cwd.as_path(), "project root")?;
let requested_path = environment.cwd.join(&args.path);
let path = canonicalize(requested_path.as_path(), "source file")?;
let project_root = canonicalize_path(environment.cwd.as_path(), "project root")?;
let path = resolve_existing_path(&project_root, &args.path, "source file")?;
if !path.starts_with(&project_root) {
return Err(FunctionCallError::RespondToModel(format!(
"smart_read path `{}` is outside project root `{}`",
Expand Down Expand Up @@ -137,15 +138,6 @@ impl ToolExecutor<ToolInvocation> for SmartReadHandler {

impl CoreToolRuntime for SmartReadHandler {}

fn canonicalize(path: &std::path::Path, label: &str) -> Result<PathBuf, FunctionCallError> {
path.canonicalize().map_err(|err| {
FunctionCallError::RespondToModel(format!(
"unable to resolve {label} `{}`: {err}",
path.display()
))
})
}

fn read_with_sdk(
project_root: PathBuf,
path: PathBuf,
Expand Down
16 changes: 4 additions & 12 deletions codex-rs/core/src/tools/handlers/smart_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::resolve_tool_environment;
use crate::tools::handlers::smart_path::canonicalize_path;
use crate::tools::handlers::smart_path::resolve_existing_path;
use crate::tools::handlers::smart_write_spec::SmartWriteToolOptions;
use crate::tools::handlers::smart_write_spec::create_smart_write_tool;
use crate::tools::registry::CoreToolRuntime;
Expand Down Expand Up @@ -88,9 +90,8 @@ impl ToolExecutor<ToolInvocation> for SmartWriteHandler {
));
}

let project_root = canonicalize(environment.cwd.as_path(), "project root")?;
let requested_path = environment.cwd.join(&args.path);
let path = canonicalize(requested_path.as_path(), "source file")?;
let project_root = canonicalize_path(environment.cwd.as_path(), "project root")?;
let path = resolve_existing_path(&project_root, &args.path, "source file")?;
validate_file_inside_root(&project_root, &path, "smart_write")?;

let output = tokio::task::spawn_blocking(move || write_with_sdk(project_root, path, args))
Expand All @@ -108,15 +109,6 @@ impl ToolExecutor<ToolInvocation> for SmartWriteHandler {

impl CoreToolRuntime for SmartWriteHandler {}

fn canonicalize(path: &Path, label: &str) -> Result<PathBuf, FunctionCallError> {
path.canonicalize().map_err(|err| {
FunctionCallError::RespondToModel(format!(
"unable to resolve {label} `{}`: {err}",
path.display()
))
})
}

fn validate_file_inside_root(
project_root: &Path,
path: &Path,
Expand Down
Loading