mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
chore: remove grep_files handler (#15775)
# External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. Include a link to a bug report or enhancement request. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -1,176 +0,0 @@
|
||||
use std::path::Path;
|
||||
use std::time::Duration;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use serde::Deserialize;
|
||||
use tokio::process::Command;
|
||||
use tokio::time::timeout;
|
||||
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::tools::context::FunctionToolOutput;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::handlers::parse_arguments;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
|
||||
pub struct GrepFilesHandler;
|
||||
|
||||
const DEFAULT_LIMIT: usize = 100;
|
||||
const MAX_LIMIT: usize = 2000;
|
||||
const COMMAND_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
fn default_limit() -> usize {
|
||||
DEFAULT_LIMIT
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct GrepFilesArgs {
|
||||
pattern: String,
|
||||
#[serde(default)]
|
||||
include: Option<String>,
|
||||
#[serde(default)]
|
||||
path: Option<String>,
|
||||
#[serde(default = "default_limit")]
|
||||
limit: usize,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ToolHandler for GrepFilesHandler {
|
||||
type Output = FunctionToolOutput;
|
||||
|
||||
fn kind(&self) -> ToolKind {
|
||||
ToolKind::Function
|
||||
}
|
||||
|
||||
async fn handle(&self, invocation: ToolInvocation) -> Result<Self::Output, FunctionCallError> {
|
||||
let ToolInvocation { payload, turn, .. } = invocation;
|
||||
|
||||
let arguments = match payload {
|
||||
ToolPayload::Function { arguments } => arguments,
|
||||
_ => {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"grep_files handler received unsupported payload".to_string(),
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
let args: GrepFilesArgs = parse_arguments(&arguments)?;
|
||||
|
||||
let pattern = args.pattern.trim();
|
||||
if pattern.is_empty() {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"pattern must not be empty".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
if args.limit == 0 {
|
||||
return Err(FunctionCallError::RespondToModel(
|
||||
"limit must be greater than zero".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let limit = args.limit.min(MAX_LIMIT);
|
||||
let search_path = turn.resolve_path(args.path.clone());
|
||||
|
||||
verify_path_exists(&search_path).await?;
|
||||
|
||||
let include = args.include.as_deref().map(str::trim).and_then(|val| {
|
||||
if val.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(val.to_string())
|
||||
}
|
||||
});
|
||||
|
||||
let search_results =
|
||||
run_rg_search(pattern, include.as_deref(), &search_path, limit, &turn.cwd).await?;
|
||||
|
||||
if search_results.is_empty() {
|
||||
Ok(FunctionToolOutput::from_text(
|
||||
"No matches found.".to_string(),
|
||||
Some(false),
|
||||
))
|
||||
} else {
|
||||
Ok(FunctionToolOutput::from_text(
|
||||
search_results.join("\n"),
|
||||
Some(true),
|
||||
))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn verify_path_exists(path: &Path) -> Result<(), FunctionCallError> {
|
||||
tokio::fs::metadata(path).await.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!("unable to access `{}`: {err}", path.display()))
|
||||
})?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn run_rg_search(
|
||||
pattern: &str,
|
||||
include: Option<&str>,
|
||||
search_path: &Path,
|
||||
limit: usize,
|
||||
cwd: &Path,
|
||||
) -> Result<Vec<String>, FunctionCallError> {
|
||||
let mut command = Command::new("rg");
|
||||
command
|
||||
.current_dir(cwd)
|
||||
.arg("--files-with-matches")
|
||||
.arg("--sortr=modified")
|
||||
.arg("--regexp")
|
||||
.arg(pattern)
|
||||
.arg("--no-messages");
|
||||
|
||||
if let Some(glob) = include {
|
||||
command.arg("--glob").arg(glob);
|
||||
}
|
||||
|
||||
command.arg("--").arg(search_path);
|
||||
|
||||
let output = timeout(COMMAND_TIMEOUT, command.output())
|
||||
.await
|
||||
.map_err(|_| {
|
||||
FunctionCallError::RespondToModel("rg timed out after 30 seconds".to_string())
|
||||
})?
|
||||
.map_err(|err| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
"failed to launch rg: {err}. Ensure ripgrep is installed and on PATH."
|
||||
))
|
||||
})?;
|
||||
|
||||
match output.status.code() {
|
||||
Some(0) => Ok(parse_results(&output.stdout, limit)),
|
||||
Some(1) => Ok(Vec::new()),
|
||||
_ => {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
Err(FunctionCallError::RespondToModel(format!(
|
||||
"rg failed: {stderr}"
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_results(stdout: &[u8], limit: usize) -> Vec<String> {
|
||||
let mut results = Vec::new();
|
||||
for line in stdout.split(|byte| *byte == b'\n') {
|
||||
if line.is_empty() {
|
||||
continue;
|
||||
}
|
||||
if let Ok(text) = std::str::from_utf8(line) {
|
||||
if text.is_empty() {
|
||||
continue;
|
||||
}
|
||||
results.push(text.to_string());
|
||||
if results.len() == limit {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
results
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "grep_files_tests.rs"]
|
||||
mod tests;
|
||||
@@ -2,7 +2,6 @@ pub(crate) mod agent_jobs;
|
||||
pub mod apply_patch;
|
||||
mod artifacts;
|
||||
mod dynamic;
|
||||
mod grep_files;
|
||||
mod js_repl;
|
||||
mod list_dir;
|
||||
mod mcp;
|
||||
@@ -41,7 +40,6 @@ pub use artifacts::ArtifactsHandler;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
pub use dynamic::DynamicToolHandler;
|
||||
pub use grep_files::GrepFilesHandler;
|
||||
pub use js_repl::JsReplHandler;
|
||||
pub use js_repl::JsReplResetHandler;
|
||||
pub use list_dir::ListDirHandler;
|
||||
|
||||
@@ -1839,59 +1839,6 @@ fn create_test_sync_tool() -> ToolSpec {
|
||||
})
|
||||
}
|
||||
|
||||
fn create_grep_files_tool() -> ToolSpec {
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
"pattern".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some("Regular expression pattern to search for.".to_string()),
|
||||
},
|
||||
),
|
||||
(
|
||||
"include".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
"Optional glob that limits which files are searched (e.g. \"*.rs\" or \
|
||||
\"*.{ts,tsx}\")."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
(
|
||||
"path".to_string(),
|
||||
JsonSchema::String {
|
||||
description: Some(
|
||||
"Directory or file path to search. Defaults to the session's working directory."
|
||||
.to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
(
|
||||
"limit".to_string(),
|
||||
JsonSchema::Number {
|
||||
description: Some(
|
||||
"Maximum number of file paths to return (defaults to 100).".to_string(),
|
||||
),
|
||||
},
|
||||
),
|
||||
]);
|
||||
|
||||
ToolSpec::Function(ResponsesApiTool {
|
||||
name: "grep_files".to_string(),
|
||||
description: "Finds files whose contents match the pattern and lists them by modification \
|
||||
time."
|
||||
.to_string(),
|
||||
strict: false,
|
||||
defer_loading: None,
|
||||
parameters: JsonSchema::Object {
|
||||
properties,
|
||||
required: Some(vec!["pattern".to_string()]),
|
||||
additional_properties: Some(false.into()),
|
||||
},
|
||||
output_schema: None,
|
||||
})
|
||||
}
|
||||
|
||||
fn create_tool_search_tool(app_tools: &HashMap<String, ToolInfo>) -> ToolSpec {
|
||||
let properties = BTreeMap::from([
|
||||
(
|
||||
@@ -2751,7 +2698,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
use crate::tools::handlers::CodeModeExecuteHandler;
|
||||
use crate::tools::handlers::CodeModeWaitHandler;
|
||||
use crate::tools::handlers::DynamicToolHandler;
|
||||
use crate::tools::handlers::GrepFilesHandler;
|
||||
use crate::tools::handlers::JsReplHandler;
|
||||
use crate::tools::handlers::JsReplResetHandler;
|
||||
use crate::tools::handlers::ListDirHandler;
|
||||
@@ -3029,20 +2975,6 @@ pub(crate) fn build_specs_with_discoverable_tools(
|
||||
builder.register_handler("apply_patch", apply_patch_handler);
|
||||
}
|
||||
|
||||
if config
|
||||
.experimental_supported_tools
|
||||
.contains(&"grep_files".to_string())
|
||||
{
|
||||
let grep_files_handler = Arc::new(GrepFilesHandler);
|
||||
push_tool_spec(
|
||||
&mut builder,
|
||||
create_grep_files_tool(),
|
||||
/*supports_parallel_tool_calls*/ true,
|
||||
config.code_mode_enabled,
|
||||
);
|
||||
builder.register_handler("grep_files", grep_files_handler);
|
||||
}
|
||||
|
||||
if config
|
||||
.experimental_supported_tools
|
||||
.contains(&"read_file".to_string())
|
||||
|
||||
@@ -1665,21 +1665,13 @@ fn test_parallel_support_flags() {
|
||||
|
||||
assert!(find_tool(&tools, "exec_command").supports_parallel_tool_calls);
|
||||
assert!(!find_tool(&tools, "write_stdin").supports_parallel_tool_calls);
|
||||
assert!(find_tool(&tools, "grep_files").supports_parallel_tool_calls);
|
||||
assert!(find_tool(&tools, "list_dir").supports_parallel_tool_calls);
|
||||
assert!(find_tool(&tools, "read_file").supports_parallel_tool_calls);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_test_model_info_includes_sync_tool() {
|
||||
let _config = test_config();
|
||||
let mut model_info = model_info_from_models_json("gpt-5-codex");
|
||||
model_info.experimental_supported_tools = vec![
|
||||
"test_sync_tool".to_string(),
|
||||
"read_file".to_string(),
|
||||
"grep_files".to_string(),
|
||||
"list_dir".to_string(),
|
||||
];
|
||||
model_info.experimental_supported_tools = vec!["test_sync_tool".to_string()];
|
||||
let features = Features::with_defaults();
|
||||
let available_models = Vec::new();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
@@ -1698,17 +1690,6 @@ fn test_test_model_info_includes_sync_tool() {
|
||||
.iter()
|
||||
.any(|tool| tool_name(&tool.spec) == "test_sync_tool")
|
||||
);
|
||||
assert!(
|
||||
tools
|
||||
.iter()
|
||||
.any(|tool| tool_name(&tool.spec) == "read_file")
|
||||
);
|
||||
assert!(
|
||||
tools
|
||||
.iter()
|
||||
.any(|tool| tool_name(&tool.spec) == "grep_files")
|
||||
);
|
||||
assert!(tools.iter().any(|tool| tool_name(&tool.spec) == "list_dir"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user