Add read file

This commit is contained in:
jimmyfraiture
2025-09-30 19:07:22 +01:00
parent 150765dbe3
commit 9e04b908e1
8 changed files with 312 additions and 64 deletions
+6 -11
View File
@@ -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;
@@ -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;
+2
View File
@@ -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;
@@ -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<ToolOutput, FunctionCallError> {
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<Vec<String>, 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())
);
}
}
+22 -13
View File
@@ -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
}
}
+91 -11
View File
@@ -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 {
+1
View File
@@ -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;
+7 -1
View File
@@ -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::<serde_json::Value>().unwrap();
assert_eq!(
body0["instructions"],