diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 4a35fd9221..fe154ed8cd 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -15,8 +15,6 @@ use crate::user_notification::UserNotifier; use async_channel::Receiver; use async_channel::Sender; use codex_apply_patch::ApplyPatchAction; -use codex_apply_patch::MaybeApplyPatchVerified; -use codex_apply_patch::maybe_parse_apply_patch_verified; use codex_protocol::mcp_protocol::ConversationId; use codex_protocol::protocol::ConversationPathResponseEvent; use codex_protocol::protocol::ExitedReviewModeEvent; @@ -27,7 +25,6 @@ use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::TurnContextItem; use futures::prelude::*; use mcp_types::CallToolResult; -use serde::Serialize; use serde_json; use serde_json::Value; use tokio::sync::Mutex; @@ -39,9 +36,6 @@ use tracing::trace; use tracing::warn; use crate::ModelProviderInfo; -use crate::apply_patch; -use crate::apply_patch::ApplyPatchExec; -use crate::apply_patch::InternalApplyPatchInvocation; use crate::apply_patch::convert_apply_patch_to_protocol; use crate::client::ModelClient; use crate::client_common::Prompt; @@ -52,16 +46,12 @@ use crate::conversation_history::ConversationHistory; use crate::environment_context::EnvironmentContext; use crate::error::CodexErr; use crate::error::Result as CodexResult; -use crate::error::SandboxErr; -use crate::exec::ExecParams; use crate::exec::ExecToolCallOutput; -use crate::exec::StdoutStream; #[cfg(test)] use crate::exec::StreamOutput; use crate::exec_command::ExecCommandParams; use crate::exec_command::ExecSessionManager; use crate::exec_command::WriteStdinParams; -use crate::executor::ExecutionMode; use crate::executor::Executor; use crate::executor::ExecutorConfig; use crate::executor::normalize_exec_result; @@ -110,7 +100,8 @@ use crate::state::SessionServices; use crate::tasks::CompactTask; use crate::tasks::RegularTask; use crate::tasks::ReviewTask; -use crate::tools::{format_exec_output_str, Router}; +use crate::tools::Router; +use crate::tools::format_exec_output_str; use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::UnifiedExecSessionManager; use crate::user_instructions::UserInstructions; @@ -2389,6 +2380,10 @@ mod tests { use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; + use crate::tools::MODEL_FORMAT_HEAD_LINES; + use crate::tools::MODEL_FORMAT_MAX_BYTES; + use crate::tools::MODEL_FORMAT_MAX_LINES; + use crate::tools::MODEL_FORMAT_TAIL_LINES; use crate::tools::handle_container_exec_with_params; use codex_protocol::mcp_protocol::AuthMode; use codex_protocol::models::ContentItem; diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 72daff6485..02852edfb7 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -1,42 +1,15 @@ use std::collections::HashMap; -use crate::apply_patch; -use crate::apply_patch::ApplyPatchExec; -use crate::apply_patch::InternalApplyPatchInvocation; -use crate::apply_patch::convert_apply_patch_to_protocol; -use crate::codex::ApplyPatchCommandContext; -use crate::codex::ExecCommandContext; -use crate::codex::Session; -use crate::codex::TurnContext; -use crate::error::CodexErr; -use crate::error::SandboxErr; use crate::exec::ExecParams; -use crate::exec::ExecToolCallOutput; -use crate::exec::StdoutStream; -use crate::executor::ExecutionMode; -use crate::executor::errors::ExecError; -use crate::executor::linkers::PreparedExec; use crate::function_tool::FunctionCallError; -use crate::tools::{handle_container_exec_with_params, MODEL_FORMAT_HEAD_BYTES}; -use crate::tools::MODEL_FORMAT_HEAD_LINES; -use crate::tools::MODEL_FORMAT_MAX_BYTES; -use crate::tools::MODEL_FORMAT_MAX_LINES; -use crate::tools::MODEL_FORMAT_TAIL_LINES; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::handle_container_exec_with_params; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::tools::spec::ApplyPatchToolArgs; -use crate::turn_diff_tracker::TurnDiffTracker; use async_trait::async_trait; -use codex_apply_patch::MaybeApplyPatchVerified; -use codex_apply_patch::maybe_parse_apply_patch_verified; -use codex_protocol::protocol::AskForApproval; -use codex_utils_string::take_bytes_at_char_boundary; -use codex_utils_string::take_last_bytes_at_char_boundary; -use serde::Serialize; -use tracing::trace; pub struct ApplyPatchHandler; diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 80596807a6..c4ee89b1b6 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -2,6 +2,7 @@ pub mod apply_patch; mod exec_stream; mod mcp; mod plan; +mod read_file; mod shell; mod unified_exec; mod view_image; @@ -10,6 +11,7 @@ pub use apply_patch::ApplyPatchHandler; pub use exec_stream::ExecStreamHandler; pub use mcp::McpHandler; pub use plan::PlanHandler; +pub use read_file::ReadFileHandler; pub use shell::ShellHandler; pub use unified_exec::UnifiedExecHandler; pub use view_image::ViewImageHandler; diff --git a/codex-rs/core/src/tools/handlers/read_file.rs b/codex-rs/core/src/tools/handlers/read_file.rs new file mode 100644 index 0000000000..c14aed840a --- /dev/null +++ b/codex-rs/core/src/tools/handlers/read_file.rs @@ -0,0 +1,182 @@ +use std::path::Path; +use std::path::PathBuf; + +use async_trait::async_trait; +use codex_utils_string::take_bytes_at_char_boundary; +use serde::Deserialize; +use tokio::fs::File; +use tokio::io::AsyncBufReadExt; +use tokio::io::BufReader; + +use crate::function_tool::FunctionCallError; +use crate::tools::context::ToolInvocation; +use crate::tools::context::ToolOutput; +use crate::tools::context::ToolPayload; +use crate::tools::registry::ToolHandler; +use crate::tools::registry::ToolKind; + +pub struct ReadFileHandler; + +const MAX_LINE_LENGTH: usize = 2000; + +#[derive(Deserialize)] +struct ReadFileArgs { + file_path: String, + #[serde(default = "default_offset")] + offset: usize, + #[serde(default = "default_limit")] + limit: usize, +} + +fn default_offset() -> usize { + 1 +} + +fn default_limit() -> usize { + 2000 +} + +#[async_trait] +impl ToolHandler for ReadFileHandler { + fn kind(&self) -> ToolKind { + ToolKind::Function + } + + async fn handle( + &self, + invocation: ToolInvocation<'_>, + ) -> Result { + let ToolInvocation { payload, .. } = invocation; + + let arguments = match payload { + ToolPayload::Function { arguments } => arguments, + _ => { + return Err(FunctionCallError::RespondToModel( + "read_file handler received unsupported payload".to_string(), + )); + } + }; + + let args: ReadFileArgs = serde_json::from_str(&arguments).map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to parse function arguments: {err:?}" + )) + })?; + + let ReadFileArgs { + file_path, + offset, + limit, + } = args; + + if offset == 0 { + return Err(FunctionCallError::RespondToModel( + "offset must be a 1-indexed line number".to_string(), + )); + } + + if limit == 0 { + return Err(FunctionCallError::RespondToModel( + "limit must be greater than zero".to_string(), + )); + } + + let path = PathBuf::from(&file_path); + if !path.is_absolute() { + return Err(FunctionCallError::RespondToModel( + "file_path must be an absolute path".to_string(), + )); + } + + let collected = read_file_slice(&path, offset, limit).await?; + Ok(ToolOutput::Function { + content: collected.join("\n"), + success: true, + }) + } +} + +async fn read_file_slice( + path: &Path, + offset: usize, + limit: usize, +) -> Result, FunctionCallError> { + let file = File::open(path) + .await + .map_err(|err| FunctionCallError::RespondToModel(format!("failed to read file: {err}")))?; + + let mut reader = BufReader::new(file).lines(); + let mut collected = Vec::new(); + let mut seen = 0usize; + + while let Some(line) = reader + .next_line() + .await + .map_err(|err| FunctionCallError::RespondToModel(format!("failed to read file: {err}")))? + { + seen += 1; + + if seen < offset { + continue; + } + + if collected.len() == limit { + break; + } + + let formatted = if line.len() > MAX_LINE_LENGTH { + take_bytes_at_char_boundary(&line, MAX_LINE_LENGTH).to_string() + } else { + line + }; + + collected.push(format!("L{seen}: {formatted}")); + + if collected.len() == limit { + break; + } + } + + if seen < offset { + return Err(FunctionCallError::RespondToModel( + "offset exceeds file length".to_string(), + )); + } + + Ok(collected) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::NamedTempFile; + + #[tokio::test] + async fn reads_requested_range() { + let mut temp = NamedTempFile::new().expect("create temp file"); + use std::io::Write as _; + writeln!(temp, "alpha").unwrap(); + writeln!(temp, "beta").unwrap(); + writeln!(temp, "gamma").unwrap(); + + let lines = read_file_slice(temp.path(), 2, 2) + .await + .expect("read slice"); + assert_eq!(lines, vec!["L2: beta".to_string(), "L3: gamma".to_string()]); + } + + #[tokio::test] + async fn errors_when_offset_exceeds_length() { + let mut temp = NamedTempFile::new().expect("create temp file"); + use std::io::Write as _; + writeln!(temp, "only").unwrap(); + + let err = read_file_slice(temp.path(), 3, 1) + .await + .expect_err("offset exceeds length"); + assert_eq!( + err, + FunctionCallError::RespondToModel("offset exceeds file length".to_string()) + ); + } +} diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 23c5d459b8..9497633eb9 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -4,22 +4,32 @@ pub mod registry; pub mod router; pub mod spec; -use serde::Serialize; -use tracing::trace; -use codex_apply_patch::{maybe_parse_apply_patch_verified, MaybeApplyPatchVerified}; -use codex_protocol::protocol::AskForApproval; -use codex_utils_string::{take_bytes_at_char_boundary, take_last_bytes_at_char_boundary}; -pub use router::Router; use crate::apply_patch; -use crate::apply_patch::{convert_apply_patch_to_protocol, ApplyPatchExec, InternalApplyPatchInvocation}; -use crate::codex::{ApplyPatchCommandContext, ExecCommandContext, Session, TurnContext}; -use crate::error::{CodexErr, SandboxErr}; -use crate::exec::{ExecParams, ExecToolCallOutput, StdoutStream}; -use crate::executor::errors::ExecError; +use crate::apply_patch::ApplyPatchExec; +use crate::apply_patch::InternalApplyPatchInvocation; +use crate::apply_patch::convert_apply_patch_to_protocol; +use crate::codex::ApplyPatchCommandContext; +use crate::codex::ExecCommandContext; +use crate::codex::Session; +use crate::codex::TurnContext; +use crate::error::CodexErr; +use crate::error::SandboxErr; +use crate::exec::ExecParams; +use crate::exec::ExecToolCallOutput; +use crate::exec::StdoutStream; use crate::executor::ExecutionMode; +use crate::executor::errors::ExecError; use crate::executor::linkers::PreparedExec; use crate::function_tool::FunctionCallError; use crate::turn_diff_tracker::TurnDiffTracker; +use codex_apply_patch::MaybeApplyPatchVerified; +use codex_apply_patch::maybe_parse_apply_patch_verified; +use codex_protocol::protocol::AskForApproval; +use codex_utils_string::take_bytes_at_char_boundary; +use codex_utils_string::take_last_bytes_at_char_boundary; +pub use router::Router; +use serde::Serialize; +use tracing::trace; // Model-formatting limits: clients get full streams; oonly content sent to the model is truncated. pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB @@ -28,7 +38,6 @@ pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2; pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128 pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2; - pub(crate) async fn handle_container_exec_with_params( tool_name: &str, params: ExecParams, @@ -256,4 +265,4 @@ pub fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String { result.push_str(tail_part); result -} \ No newline at end of file +} diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 42814b1b57..38a0df7bd2 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -276,6 +276,43 @@ fn create_view_image_tool() -> ToolSpec { }, }) } + +fn create_read_file_tool() -> ToolSpec { + let mut properties = BTreeMap::new(); + properties.insert( + "file_path".to_string(), + JsonSchema::String { + description: Some("Absolute path to the file".to_string()), + }, + ); + properties.insert( + "offset".to_string(), + JsonSchema::Number { + description: Some( + "The line number to start reading from. Must be 1 or greater.".to_string(), + ), + }, + ); + properties.insert( + "limit".to_string(), + JsonSchema::Number { + description: Some("The maximum number of lines to return.".to_string()), + }, + ); + + ToolSpec::Function(ResponsesApiTool { + name: "read_file".to_string(), + description: + "Reads a local file with 1-indexed line numbers and returns up to the requested number of lines." + .to_string(), + strict: false, + parameters: JsonSchema::Object { + properties, + required: Some(vec!["file_path".to_string()]), + additional_properties: Some(false), + }, + }) +} /// TODO(dylan): deprecate once we get rid of json tool #[derive(Serialize, Deserialize)] pub(crate) struct ApplyPatchToolArgs { @@ -487,6 +524,7 @@ pub(crate) fn build_specs( use crate::tools::handlers::ExecStreamHandler; use crate::tools::handlers::McpHandler; use crate::tools::handlers::PlanHandler; + use crate::tools::handlers::ReadFileHandler; use crate::tools::handlers::ShellHandler; use crate::tools::handlers::UnifiedExecHandler; use crate::tools::handlers::ViewImageHandler; @@ -499,6 +537,7 @@ pub(crate) fn build_specs( let exec_stream_handler = Arc::new(ExecStreamHandler); let unified_exec_handler = Arc::new(UnifiedExecHandler); let plan_handler = Arc::new(PlanHandler); + let read_file_handler = Arc::new(ReadFileHandler); let apply_patch_handler = Arc::new(ApplyPatchHandler); let view_image_handler = Arc::new(ViewImageHandler); let mcp_handler = Arc::new(McpHandler); @@ -549,6 +588,9 @@ pub(crate) fn build_specs( builder.register_handler("apply_patch", apply_patch_handler); } + specs.push(create_read_file_tool()); + builder.register_handler("read_file", read_file_handler); + if config.web_search_request { specs.push(ToolSpec::WebSearch {}); } @@ -627,7 +669,13 @@ mod tests { assert_eq_tool_names( &tools, - &["unified_exec", "update_plan", "web_search", "view_image"], + &[ + "unified_exec", + "update_plan", + "read_file", + "web_search", + "view_image", + ], ); } @@ -647,7 +695,13 @@ mod tests { assert_eq_tool_names( &tools, - &["unified_exec", "update_plan", "web_search", "view_image"], + &[ + "unified_exec", + "update_plan", + "read_file", + "web_search", + "view_image", + ], ); } @@ -706,6 +760,7 @@ mod tests { &tools, &[ "unified_exec", + "read_file", "web_search", "view_image", "test_server/do_something_cool", @@ -713,7 +768,7 @@ mod tests { ); assert_eq!( - tools[3], + tools[4], ToolSpec::Function(ResponsesApiTool { name: "test_server/do_something_cool".to_string(), parameters: JsonSchema::Object { @@ -824,6 +879,7 @@ mod tests { &tools, &[ "unified_exec", + "read_file", "view_image", "test_server/cool", "test_server/do", @@ -871,11 +927,17 @@ mod tests { assert_eq_tool_names( &tools, - &["unified_exec", "web_search", "view_image", "dash/search"], + &[ + "unified_exec", + "read_file", + "web_search", + "view_image", + "dash/search", + ], ); assert_eq!( - tools[3], + tools[4], ToolSpec::Function(ResponsesApiTool { name: "dash/search".to_string(), parameters: JsonSchema::Object { @@ -931,10 +993,16 @@ mod tests { assert_eq_tool_names( &tools, - &["unified_exec", "web_search", "view_image", "dash/paginate"], + &[ + "unified_exec", + "read_file", + "web_search", + "view_image", + "dash/paginate", + ], ); assert_eq!( - tools[3], + tools[4], ToolSpec::Function(ResponsesApiTool { name: "dash/paginate".to_string(), parameters: JsonSchema::Object { @@ -988,10 +1056,16 @@ mod tests { assert_eq_tool_names( &tools, - &["unified_exec", "web_search", "view_image", "dash/tags"], + &[ + "unified_exec", + "read_file", + "web_search", + "view_image", + "dash/tags", + ], ); assert_eq!( - tools[3], + tools[4], ToolSpec::Function(ResponsesApiTool { name: "dash/tags".to_string(), parameters: JsonSchema::Object { @@ -1048,10 +1122,16 @@ mod tests { assert_eq_tool_names( &tools, - &["unified_exec", "web_search", "view_image", "dash/value"], + &[ + "unified_exec", + "read_file", + "web_search", + "view_image", + "dash/value", + ], ); assert_eq!( - tools[3], + tools[4], ToolSpec::Function(ResponsesApiTool { name: "dash/value".to_string(), parameters: JsonSchema::Object { diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index b3a90ff339..cd251c9f77 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -14,6 +14,7 @@ mod live_cli; mod model_overrides; mod otel; mod prompt_caching; +mod read_file; mod review; mod rmcp_client; mod rollout_list_find; diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 79be6083ab..bc66be18f6 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -219,7 +219,13 @@ async fn prompt_tools_are_consistent_across_requests() { // our internal implementation is responsible for keeping tools in sync // with the OpenAI schema, so we just verify the tool presence here - let expected_tools_names: &[&str] = &["shell", "update_plan", "apply_patch", "view_image"]; + let expected_tools_names: &[&str] = &[ + "shell", + "update_plan", + "apply_patch", + "read_file", + "view_image", + ]; let body0 = requests[0].body_json::().unwrap(); assert_eq!( body0["instructions"],