From c9c65606852c0cda9d983b4917359a0826a4b7f0 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Thu, 8 Jan 2026 19:49:17 +0000 Subject: [PATCH] nit: parse_arguments (#8927) --- .../core/src/tools/handlers/apply_patch.rs | 7 +--- .../core/src/tools/handlers/grep_files.rs | 7 +--- codex-rs/core/src/tools/handlers/list_dir.rs | 7 +--- codex-rs/core/src/tools/handlers/mod.rs | 11 ++++++ codex-rs/core/src/tools/handlers/read_file.rs | 7 +--- codex-rs/core/src/tools/handlers/shell.rs | 12 ++---- codex-rs/core/src/tools/handlers/test_sync.rs | 7 +--- .../core/src/tools/handlers/unified_exec.rs | 37 ++++++++----------- .../core/src/tools/handlers/view_image.rs | 5 +-- codex-rs/protocol/src/models.rs | 1 - 10 files changed, 41 insertions(+), 60 deletions(-) diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index adf76e682..46723decf 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -17,6 +17,7 @@ use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; +use crate::tools::handlers::parse_arguments; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -87,11 +88,7 @@ impl ToolHandler for ApplyPatchHandler { let patch_input = match payload { ToolPayload::Function { arguments } => { - let args: ApplyPatchToolArgs = serde_json::from_str(&arguments).map_err(|e| { - FunctionCallError::RespondToModel(format!( - "failed to parse function arguments: {e:?}" - )) - })?; + let args: ApplyPatchToolArgs = parse_arguments(&arguments)?; args.input } ToolPayload::Custom { input } => input, diff --git a/codex-rs/core/src/tools/handlers/grep_files.rs b/codex-rs/core/src/tools/handlers/grep_files.rs index 5473f8693..a3e89af6a 100644 --- a/codex-rs/core/src/tools/handlers/grep_files.rs +++ b/codex-rs/core/src/tools/handlers/grep_files.rs @@ -10,6 +10,7 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -52,11 +53,7 @@ impl ToolHandler for GrepFilesHandler { } }; - let args: GrepFilesArgs = serde_json::from_str(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse function arguments: {err:?}" - )) - })?; + let args: GrepFilesArgs = parse_arguments(&arguments)?; let pattern = args.pattern.trim(); if pattern.is_empty() { diff --git a/codex-rs/core/src/tools/handlers/list_dir.rs b/codex-rs/core/src/tools/handlers/list_dir.rs index ffeed98f7..a06fca3d1 100644 --- a/codex-rs/core/src/tools/handlers/list_dir.rs +++ b/codex-rs/core/src/tools/handlers/list_dir.rs @@ -13,6 +13,7 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -62,11 +63,7 @@ impl ToolHandler for ListDirHandler { } }; - let args: ListDirArgs = serde_json::from_str(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse function arguments: {err:?}" - )) - })?; + let args: ListDirArgs = parse_arguments(&arguments)?; let ListDirArgs { dir_path, diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index dcf848e37..d9f6859c6 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -11,7 +11,9 @@ mod unified_exec; mod view_image; pub use plan::PLAN_TOOL; +use serde::Deserialize; +use crate::function_tool::FunctionCallError; pub use apply_patch::ApplyPatchHandler; pub use grep_files::GrepFilesHandler; pub use list_dir::ListDirHandler; @@ -24,3 +26,12 @@ pub use shell::ShellHandler; pub use test_sync::TestSyncHandler; pub use unified_exec::UnifiedExecHandler; pub use view_image::ViewImageHandler; + +fn parse_arguments(arguments: &str) -> Result +where + T: for<'de> Deserialize<'de>, +{ + serde_json::from_str(arguments).map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to parse function arguments: {err}")) + }) +} diff --git a/codex-rs/core/src/tools/handlers/read_file.rs b/codex-rs/core/src/tools/handlers/read_file.rs index 58b6ea688..4f187540a 100644 --- a/codex-rs/core/src/tools/handlers/read_file.rs +++ b/codex-rs/core/src/tools/handlers/read_file.rs @@ -9,6 +9,7 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -107,11 +108,7 @@ impl ToolHandler for ReadFileHandler { } }; - let args: ReadFileArgs = serde_json::from_str(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse function arguments: {err:?}" - )) - })?; + let args: ReadFileArgs = parse_arguments(&arguments)?; let ReadFileArgs { file_path, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 720361771..0e14da68f 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -16,6 +16,7 @@ use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::handlers::apply_patch::intercept_apply_patch; +use crate::tools::handlers::parse_arguments; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -104,12 +105,7 @@ impl ToolHandler for ShellHandler { match payload { ToolPayload::Function { arguments } => { - let params: ShellToolCallParams = - serde_json::from_str(&arguments).map_err(|e| { - FunctionCallError::RespondToModel(format!( - "failed to parse function arguments: {e:?}" - )) - })?; + let params: ShellToolCallParams = parse_arguments(&arguments)?; let exec_params = Self::to_exec_params(params, turn.as_ref()); Self::run_exec_like( tool_name.as_str(), @@ -182,9 +178,7 @@ impl ToolHandler for ShellCommandHandler { ))); }; - let params: ShellCommandToolCallParams = serde_json::from_str(&arguments).map_err(|e| { - FunctionCallError::RespondToModel(format!("failed to parse function arguments: {e:?}")) - })?; + let params: ShellCommandToolCallParams = parse_arguments(&arguments)?; let exec_params = Self::to_exec_params(params, session.as_ref(), turn.as_ref()); ShellHandler::run_exec_like( tool_name.as_str(), diff --git a/codex-rs/core/src/tools/handlers/test_sync.rs b/codex-rs/core/src/tools/handlers/test_sync.rs index d217c1e8a..643cb464f 100644 --- a/codex-rs/core/src/tools/handlers/test_sync.rs +++ b/codex-rs/core/src/tools/handlers/test_sync.rs @@ -13,6 +13,7 @@ use crate::function_tool::FunctionCallError; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; @@ -71,11 +72,7 @@ impl ToolHandler for TestSyncHandler { } }; - let args: TestSyncArgs = serde_json::from_str(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse function arguments: {err:?}" - )) - })?; + let args: TestSyncArgs = parse_arguments(&arguments)?; if let Some(delay) = args.sleep_before_ms && delay > 0 diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 8ae6b1a8d..7769f262a 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -9,6 +9,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::handlers::apply_patch::intercept_apply_patch; +use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use crate::unified_exec::ExecCommandRequest; @@ -117,11 +118,7 @@ impl ToolHandler for UnifiedExecHandler { let response = match tool_name.as_str() { "exec_command" => { - let args: ExecCommandArgs = serde_json::from_str(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse exec_command arguments: {err:?}" - )) - })?; + let args: ExecCommandArgs = parse_arguments(&arguments)?; let process_id = manager.allocate_process_id().await; let command = get_command(&args, session.user_shell()); @@ -187,11 +184,7 @@ impl ToolHandler for UnifiedExecHandler { })? } "write_stdin" => { - let args: WriteStdinArgs = serde_json::from_str(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse write_stdin arguments: {err:?}" - )) - })?; + let args: WriteStdinArgs = parse_arguments(&arguments)?; let response = manager .write_stdin(WriteStdinRequest { process_id: &args.session_id.to_string(), @@ -280,11 +273,10 @@ mod tests { use std::sync::Arc; #[test] - fn test_get_command_uses_default_shell_when_unspecified() { + fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello"}"#; - let args: ExecCommandArgs = - serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + let args: ExecCommandArgs = parse_arguments(json)?; assert!(args.shell.is_none()); @@ -292,14 +284,14 @@ mod tests { assert_eq!(command.len(), 3); assert_eq!(command[2], "echo hello"); + Ok(()) } #[test] - fn test_get_command_respects_explicit_bash_shell() { + fn test_get_command_respects_explicit_bash_shell() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#; - let args: ExecCommandArgs = - serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + let args: ExecCommandArgs = parse_arguments(json)?; assert_eq!(args.shell.as_deref(), Some("/bin/bash")); @@ -312,33 +304,34 @@ mod tests { { assert!(command.contains(&"-NoProfile".to_string())); } + Ok(()) } #[test] - fn test_get_command_respects_explicit_powershell_shell() { + fn test_get_command_respects_explicit_powershell_shell() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#; - let args: ExecCommandArgs = - serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + let args: ExecCommandArgs = parse_arguments(json)?; assert_eq!(args.shell.as_deref(), Some("powershell")); let command = get_command(&args, Arc::new(default_user_shell())); assert_eq!(command[2], "echo hello"); + Ok(()) } #[test] - fn test_get_command_respects_explicit_cmd_shell() { + fn test_get_command_respects_explicit_cmd_shell() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello", "shell": "cmd"}"#; - let args: ExecCommandArgs = - serde_json::from_str(json).expect("deserialize ExecCommandArgs"); + let args: ExecCommandArgs = parse_arguments(json)?; assert_eq!(args.shell.as_deref(), Some("cmd")); let command = get_command(&args, Arc::new(default_user_shell())); assert_eq!(command[2], "echo hello"); + Ok(()) } } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 6b308c094..3c0670e3e 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -8,6 +8,7 @@ use crate::protocol::ViewImageToolCallEvent; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use codex_protocol::user_input::UserInput; @@ -43,9 +44,7 @@ impl ToolHandler for ViewImageHandler { } }; - let args: ViewImageArgs = serde_json::from_str(&arguments).map_err(|e| { - FunctionCallError::RespondToModel(format!("failed to parse function arguments: {e:?}")) - })?; + let args: ViewImageArgs = parse_arguments(&arguments)?; let abs_path = turn.resolve_path(Some(args.path)); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 5113dadd0..69a682f2d 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -397,7 +397,6 @@ pub struct FunctionCallOutputPayload { pub content: String, #[serde(skip_serializing_if = "Option::is_none")] pub content_items: Option>, - // TODO(jif) drop this. pub success: Option, }