mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
codex-tools: introduce named tool definitions (#15953)
## Why This continues the `codex-tools` migration by moving one more piece of generic tool-definition bookkeeping out of `codex-core`. The earlier extraction steps moved shared schema parsing into `codex-tools`, but `core/src/tools/spec.rs` still had to supply tool names separately and perform ad hoc rewrites for deferred MCP aliases. That meant the crate boundary was still awkward: the parsed shape coming back from `codex-tools` was missing part of the definition that `codex-core` ultimately needs to assemble a `ResponsesApiTool`. This change introduces a named `ToolDefinition` in `codex-tools` so both MCP tools and dynamic tools cross the crate boundary in the same reusable model. `codex-core` still owns the final `ResponsesApiTool` assembly, but less of the generic tool-definition shaping logic stays behind in `core`. ## What changed - replaced `ParsedToolDefinition` with a named `ToolDefinition` in `codex-rs/tools/src/tool_definition.rs` - added `codex-rs/tools/src/tool_definition_tests.rs` for `renamed()` and `into_deferred()` - updated `parse_dynamic_tool()` and `parse_mcp_tool()` to return `ToolDefinition` - simplified `codex-rs/core/src/tools/spec.rs` so it adapts `ToolDefinition` into `ResponsesApiTool` instead of rewriting names and deferred fields inline - updated parser tests and `codex-rs/tools/README.md` to reflect the named tool-definition model ## Test plan - `cargo test -p codex-tools` - `cargo test -p codex-core --lib tools::spec::`
This commit is contained in:
@@ -46,7 +46,7 @@ use codex_protocol::openai_models::WebSearchToolType;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_tools::ParsedToolDefinition;
|
||||
use codex_tools::ToolDefinition;
|
||||
use codex_tools::parse_dynamic_tool;
|
||||
use codex_tools::parse_mcp_tool;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -2386,9 +2386,8 @@ pub(crate) fn mcp_tool_to_openai_tool(
|
||||
fully_qualified_name: String,
|
||||
tool: rmcp::model::Tool,
|
||||
) -> Result<ResponsesApiTool, serde_json::Error> {
|
||||
Ok(parsed_tool_to_openai_tool(
|
||||
fully_qualified_name,
|
||||
parse_mcp_tool(&tool)?,
|
||||
Ok(tool_definition_to_openai_tool(
|
||||
parse_mcp_tool(&tool)?.renamed(fully_qualified_name),
|
||||
))
|
||||
}
|
||||
|
||||
@@ -2396,35 +2395,25 @@ pub(crate) fn mcp_tool_to_deferred_openai_tool(
|
||||
name: String,
|
||||
tool: rmcp::model::Tool,
|
||||
) -> Result<ResponsesApiTool, serde_json::Error> {
|
||||
let parsed_tool = parse_mcp_tool(&tool)?;
|
||||
|
||||
Ok(parsed_tool_to_openai_tool(
|
||||
name,
|
||||
ParsedToolDefinition {
|
||||
output_schema: None,
|
||||
defer_loading: true,
|
||||
..parsed_tool
|
||||
},
|
||||
Ok(tool_definition_to_openai_tool(
|
||||
parse_mcp_tool(&tool)?.renamed(name).into_deferred(),
|
||||
))
|
||||
}
|
||||
|
||||
fn dynamic_tool_to_openai_tool(
|
||||
tool: &DynamicToolSpec,
|
||||
) -> Result<ResponsesApiTool, serde_json::Error> {
|
||||
Ok(parsed_tool_to_openai_tool(
|
||||
tool.name.clone(),
|
||||
parse_dynamic_tool(tool)?,
|
||||
))
|
||||
Ok(tool_definition_to_openai_tool(parse_dynamic_tool(tool)?))
|
||||
}
|
||||
|
||||
fn parsed_tool_to_openai_tool(name: String, parsed_tool: ParsedToolDefinition) -> ResponsesApiTool {
|
||||
fn tool_definition_to_openai_tool(tool_definition: ToolDefinition) -> ResponsesApiTool {
|
||||
ResponsesApiTool {
|
||||
name,
|
||||
description: parsed_tool.description,
|
||||
name: tool_definition.name,
|
||||
description: tool_definition.description,
|
||||
strict: false,
|
||||
defer_loading: parsed_tool.defer_loading.then_some(true),
|
||||
parameters: parsed_tool.input_schema,
|
||||
output_schema: parsed_tool.output_schema,
|
||||
defer_loading: tool_definition.defer_loading.then_some(true),
|
||||
parameters: tool_definition.input_schema,
|
||||
output_schema: tool_definition.output_schema,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ schema primitives that no longer need to live in `core/src/tools/spec.rs`:
|
||||
|
||||
- `JsonSchema`
|
||||
- `AdditionalProperties`
|
||||
- `ParsedToolDefinition`
|
||||
- `ToolDefinition`
|
||||
- `parse_tool_input_schema()`
|
||||
- `parse_dynamic_tool()`
|
||||
- `parse_mcp_tool()`
|
||||
|
||||
@@ -1,17 +1,16 @@
|
||||
use crate::ParsedToolDefinition;
|
||||
use crate::ToolDefinition;
|
||||
use crate::parse_tool_input_schema;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
|
||||
pub fn parse_dynamic_tool(
|
||||
tool: &DynamicToolSpec,
|
||||
) -> Result<ParsedToolDefinition, serde_json::Error> {
|
||||
pub fn parse_dynamic_tool(tool: &DynamicToolSpec) -> Result<ToolDefinition, serde_json::Error> {
|
||||
let DynamicToolSpec {
|
||||
name: _,
|
||||
name,
|
||||
description,
|
||||
input_schema,
|
||||
defer_loading,
|
||||
} = tool;
|
||||
Ok(ParsedToolDefinition {
|
||||
Ok(ToolDefinition {
|
||||
name: name.clone(),
|
||||
description: description.clone(),
|
||||
input_schema: parse_tool_input_schema(input_schema)?,
|
||||
output_schema: None,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::parse_dynamic_tool;
|
||||
use crate::JsonSchema;
|
||||
use crate::ParsedToolDefinition;
|
||||
use crate::ToolDefinition;
|
||||
use codex_protocol::dynamic_tools::DynamicToolSpec;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
@@ -22,7 +22,8 @@ fn parse_dynamic_tool_sanitizes_input_schema() {
|
||||
|
||||
assert_eq!(
|
||||
parse_dynamic_tool(&tool).expect("parse dynamic tool"),
|
||||
ParsedToolDefinition {
|
||||
ToolDefinition {
|
||||
name: "lookup_ticket".to_string(),
|
||||
description: "Fetch a ticket".to_string(),
|
||||
input_schema: JsonSchema::Object {
|
||||
properties: BTreeMap::from([(
|
||||
@@ -54,7 +55,8 @@ fn parse_dynamic_tool_preserves_defer_loading() {
|
||||
|
||||
assert_eq!(
|
||||
parse_dynamic_tool(&tool).expect("parse dynamic tool"),
|
||||
ParsedToolDefinition {
|
||||
ToolDefinition {
|
||||
name: "lookup_ticket".to_string(),
|
||||
description: "Fetch a ticket".to_string(),
|
||||
input_schema: JsonSchema::Object {
|
||||
properties: BTreeMap::new(),
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
mod dynamic_tool;
|
||||
mod json_schema;
|
||||
mod mcp_tool;
|
||||
mod parsed_tool_definition;
|
||||
mod tool_definition;
|
||||
|
||||
pub use dynamic_tool::parse_dynamic_tool;
|
||||
pub use json_schema::AdditionalProperties;
|
||||
@@ -11,4 +11,4 @@ pub use json_schema::JsonSchema;
|
||||
pub use json_schema::parse_tool_input_schema;
|
||||
pub use mcp_tool::mcp_call_tool_result_output_schema;
|
||||
pub use mcp_tool::parse_mcp_tool;
|
||||
pub use parsed_tool_definition::ParsedToolDefinition;
|
||||
pub use tool_definition::ToolDefinition;
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
use crate::ParsedToolDefinition;
|
||||
use crate::ToolDefinition;
|
||||
use crate::parse_tool_input_schema;
|
||||
use serde_json::Value as JsonValue;
|
||||
use serde_json::json;
|
||||
|
||||
pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ParsedToolDefinition, serde_json::Error> {
|
||||
pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ToolDefinition, serde_json::Error> {
|
||||
let mut serialized_input_schema = serde_json::Value::Object(tool.input_schema.as_ref().clone());
|
||||
|
||||
// OpenAI models mandate the "properties" field in the schema. Some MCP
|
||||
@@ -25,7 +25,8 @@ pub fn parse_mcp_tool(tool: &rmcp::model::Tool) -> Result<ParsedToolDefinition,
|
||||
.map(|output_schema| serde_json::Value::Object(output_schema.as_ref().clone()))
|
||||
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new()));
|
||||
|
||||
Ok(ParsedToolDefinition {
|
||||
Ok(ToolDefinition {
|
||||
name: tool.name.to_string(),
|
||||
description: tool.description.clone().map(Into::into).unwrap_or_default(),
|
||||
input_schema,
|
||||
output_schema: Some(mcp_call_tool_result_output_schema(
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use super::mcp_call_tool_result_output_schema;
|
||||
use super::parse_mcp_tool;
|
||||
use crate::JsonSchema;
|
||||
use crate::ParsedToolDefinition;
|
||||
use crate::ToolDefinition;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
@@ -31,7 +31,8 @@ fn parse_mcp_tool_inserts_empty_properties() {
|
||||
|
||||
assert_eq!(
|
||||
parse_mcp_tool(&tool).expect("parse MCP tool"),
|
||||
ParsedToolDefinition {
|
||||
ToolDefinition {
|
||||
name: "no_props".to_string(),
|
||||
description: "No properties".to_string(),
|
||||
input_schema: JsonSchema::Object {
|
||||
properties: BTreeMap::new(),
|
||||
@@ -68,7 +69,8 @@ fn parse_mcp_tool_preserves_top_level_output_schema() {
|
||||
|
||||
assert_eq!(
|
||||
parse_mcp_tool(&tool).expect("parse MCP tool"),
|
||||
ParsedToolDefinition {
|
||||
ToolDefinition {
|
||||
name: "with_output".to_string(),
|
||||
description: "Has output schema".to_string(),
|
||||
input_schema: JsonSchema::Object {
|
||||
properties: BTreeMap::new(),
|
||||
@@ -107,7 +109,8 @@ fn parse_mcp_tool_preserves_output_schema_without_inferred_type() {
|
||||
|
||||
assert_eq!(
|
||||
parse_mcp_tool(&tool).expect("parse MCP tool"),
|
||||
ParsedToolDefinition {
|
||||
ToolDefinition {
|
||||
name: "with_enum_output".to_string(),
|
||||
description: "Has enum output schema".to_string(),
|
||||
input_schema: JsonSchema::Object {
|
||||
properties: BTreeMap::new(),
|
||||
|
||||
@@ -1,12 +0,0 @@
|
||||
use crate::JsonSchema;
|
||||
use serde_json::Value as JsonValue;
|
||||
|
||||
/// Parsed tool metadata and schemas that downstream crates can adapt into
|
||||
/// higher-level tool specs.
|
||||
#[derive(Debug, PartialEq)]
|
||||
pub struct ParsedToolDefinition {
|
||||
pub description: String,
|
||||
pub input_schema: JsonSchema,
|
||||
pub output_schema: Option<JsonValue>,
|
||||
pub defer_loading: bool,
|
||||
}
|
||||
30
codex-rs/tools/src/tool_definition.rs
Normal file
30
codex-rs/tools/src/tool_definition.rs
Normal file
@@ -0,0 +1,30 @@
|
||||
use crate::JsonSchema;
|
||||
use serde_json::Value as JsonValue;
|
||||
|
||||
/// Tool metadata and schemas that downstream crates can adapt into higher-level
|
||||
/// tool specs.
|
||||
#[derive(Debug, PartialEq)]
|
||||
pub struct ToolDefinition {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
pub input_schema: JsonSchema,
|
||||
pub output_schema: Option<JsonValue>,
|
||||
pub defer_loading: bool,
|
||||
}
|
||||
|
||||
impl ToolDefinition {
|
||||
pub fn renamed(mut self, name: String) -> Self {
|
||||
self.name = name;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn into_deferred(mut self) -> Self {
|
||||
self.output_schema = None;
|
||||
self.defer_loading = true;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "tool_definition_tests.rs"]
|
||||
mod tests;
|
||||
43
codex-rs/tools/src/tool_definition_tests.rs
Normal file
43
codex-rs/tools/src/tool_definition_tests.rs
Normal file
@@ -0,0 +1,43 @@
|
||||
use super::ToolDefinition;
|
||||
use crate::JsonSchema;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
fn tool_definition() -> ToolDefinition {
|
||||
ToolDefinition {
|
||||
name: "lookup_order".to_string(),
|
||||
description: "Look up an order".to_string(),
|
||||
input_schema: JsonSchema::Object {
|
||||
properties: BTreeMap::new(),
|
||||
required: None,
|
||||
additional_properties: None,
|
||||
},
|
||||
output_schema: Some(serde_json::json!({
|
||||
"type": "object",
|
||||
})),
|
||||
defer_loading: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn renamed_overrides_name_only() {
|
||||
assert_eq!(
|
||||
tool_definition().renamed("mcp__orders__lookup_order".to_string()),
|
||||
ToolDefinition {
|
||||
name: "mcp__orders__lookup_order".to_string(),
|
||||
..tool_definition()
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn into_deferred_drops_output_schema_and_sets_defer_loading() {
|
||||
assert_eq!(
|
||||
tool_definition().into_deferred(),
|
||||
ToolDefinition {
|
||||
output_schema: None,
|
||||
defer_loading: true,
|
||||
..tool_definition()
|
||||
}
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user