mirror of
https://github.com/openai/codex.git
synced 2026-05-02 12:21:26 +03:00
Enforce single tool output type in codex handlers (#14157)
We'll need to associate output schema with each tool. Each tool can only have on output type.
This commit is contained in:
@@ -10,8 +10,8 @@ use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use codex_protocol::models::function_call_output_content_items_to_text;
|
||||
use codex_utils_string::take_bytes_at_char_boundary;
|
||||
use std::any::Any;
|
||||
use std::borrow::Cow;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Mutex;
|
||||
@@ -64,16 +64,14 @@ impl ToolPayload {
|
||||
}
|
||||
}
|
||||
|
||||
pub trait ToolOutput: Any + Send {
|
||||
pub trait ToolOutput: Send {
|
||||
fn log_preview(&self) -> String;
|
||||
|
||||
fn success_for_logging(&self) -> bool;
|
||||
|
||||
fn into_response(self: Box<Self>, call_id: &str, payload: &ToolPayload) -> ResponseInputItem;
|
||||
fn into_response(self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem;
|
||||
}
|
||||
|
||||
pub type ToolOutputBox = Box<dyn ToolOutput>;
|
||||
|
||||
pub struct McpToolOutput {
|
||||
pub result: Result<CallToolResult, String>,
|
||||
}
|
||||
@@ -87,8 +85,8 @@ impl ToolOutput for McpToolOutput {
|
||||
self.result.is_ok()
|
||||
}
|
||||
|
||||
fn into_response(self: Box<Self>, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem {
|
||||
let Self { result } = *self;
|
||||
fn into_response(self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem {
|
||||
let Self { result } = self;
|
||||
ResponseInputItem::McpToolCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
result,
|
||||
@@ -96,42 +94,34 @@ impl ToolOutput for McpToolOutput {
|
||||
}
|
||||
}
|
||||
|
||||
pub struct TextToolOutput {
|
||||
pub text: String,
|
||||
pub struct FunctionToolOutput {
|
||||
pub body: Vec<FunctionCallOutputContentItem>,
|
||||
pub success: Option<bool>,
|
||||
}
|
||||
|
||||
impl ToolOutput for TextToolOutput {
|
||||
fn log_preview(&self) -> String {
|
||||
telemetry_preview(&self.text)
|
||||
}
|
||||
|
||||
fn success_for_logging(&self) -> bool {
|
||||
self.success.unwrap_or(true)
|
||||
}
|
||||
|
||||
fn into_response(self: Box<Self>, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
|
||||
let Self { text, success } = *self;
|
||||
function_tool_response(
|
||||
call_id,
|
||||
payload,
|
||||
FunctionCallOutputBody::Text(text),
|
||||
impl FunctionToolOutput {
|
||||
pub fn from_text(text: String, success: Option<bool>) -> Self {
|
||||
Self {
|
||||
body: vec![FunctionCallOutputContentItem::InputText { text }],
|
||||
success,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
pub fn from_content(
|
||||
content: Vec<FunctionCallOutputContentItem>,
|
||||
success: Option<bool>,
|
||||
) -> Self {
|
||||
Self {
|
||||
body: content,
|
||||
success,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub struct ContentToolOutput {
|
||||
pub content: Vec<FunctionCallOutputContentItem>,
|
||||
pub success: Option<bool>,
|
||||
}
|
||||
|
||||
impl ToolOutput for ContentToolOutput {
|
||||
impl ToolOutput for FunctionToolOutput {
|
||||
fn log_preview(&self) -> String {
|
||||
telemetry_preview(
|
||||
&FunctionCallOutputBody::ContentItems(self.content.clone())
|
||||
.to_text()
|
||||
.unwrap_or_default(),
|
||||
&function_call_output_content_items_to_text(&self.body).unwrap_or_default(),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -139,23 +129,25 @@ impl ToolOutput for ContentToolOutput {
|
||||
self.success.unwrap_or(true)
|
||||
}
|
||||
|
||||
fn into_response(self: Box<Self>, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
|
||||
let Self { content, success } = *self;
|
||||
function_tool_response(
|
||||
call_id,
|
||||
payload,
|
||||
FunctionCallOutputBody::ContentItems(content),
|
||||
success,
|
||||
)
|
||||
fn into_response(self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
|
||||
let Self { body, success } = self;
|
||||
function_tool_response(call_id, payload, body, success)
|
||||
}
|
||||
}
|
||||
|
||||
fn function_tool_response(
|
||||
call_id: &str,
|
||||
payload: &ToolPayload,
|
||||
body: FunctionCallOutputBody,
|
||||
body: Vec<FunctionCallOutputContentItem>,
|
||||
success: Option<bool>,
|
||||
) -> ResponseInputItem {
|
||||
let body = match body.as_slice() {
|
||||
[FunctionCallOutputContentItem::InputText { text }] => {
|
||||
FunctionCallOutputBody::Text(text.clone())
|
||||
}
|
||||
_ => FunctionCallOutputBody::ContentItems(body),
|
||||
};
|
||||
|
||||
if matches!(payload, ToolPayload::Custom { .. }) {
|
||||
return ResponseInputItem::CustomToolCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
@@ -219,17 +211,14 @@ mod tests {
|
||||
let payload = ToolPayload::Custom {
|
||||
input: "patch".to_string(),
|
||||
};
|
||||
let response = Box::new(TextToolOutput {
|
||||
text: "patched".to_string(),
|
||||
success: Some(true),
|
||||
})
|
||||
.into_response("call-42", &payload);
|
||||
let response = FunctionToolOutput::from_text("patched".to_string(), Some(true))
|
||||
.into_response("call-42", &payload);
|
||||
|
||||
match response {
|
||||
ResponseInputItem::CustomToolCallOutput { call_id, output } => {
|
||||
assert_eq!(call_id, "call-42");
|
||||
assert_eq!(output.text_content(), Some("patched"));
|
||||
assert!(output.content_items().is_none());
|
||||
assert_eq!(output.content_items(), None);
|
||||
assert_eq!(output.body.to_text().as_deref(), Some("patched"));
|
||||
assert_eq!(output.success, Some(true));
|
||||
}
|
||||
other => panic!("expected CustomToolCallOutput, got {other:?}"),
|
||||
@@ -241,17 +230,14 @@ mod tests {
|
||||
let payload = ToolPayload::Function {
|
||||
arguments: "{}".to_string(),
|
||||
};
|
||||
let response = Box::new(TextToolOutput {
|
||||
text: "ok".to_string(),
|
||||
success: Some(true),
|
||||
})
|
||||
.into_response("fn-1", &payload);
|
||||
let response = FunctionToolOutput::from_text("ok".to_string(), Some(true))
|
||||
.into_response("fn-1", &payload);
|
||||
|
||||
match response {
|
||||
ResponseInputItem::FunctionCallOutput { call_id, output } => {
|
||||
assert_eq!(call_id, "fn-1");
|
||||
assert_eq!(output.text_content(), Some("ok"));
|
||||
assert!(output.content_items().is_none());
|
||||
assert_eq!(output.content_items(), None);
|
||||
assert_eq!(output.body.to_text().as_deref(), Some("ok"));
|
||||
assert_eq!(output.success, Some(true));
|
||||
}
|
||||
other => panic!("expected FunctionCallOutput, got {other:?}"),
|
||||
@@ -263,8 +249,8 @@ mod tests {
|
||||
let payload = ToolPayload::Custom {
|
||||
input: "patch".to_string(),
|
||||
};
|
||||
let response = Box::new(ContentToolOutput {
|
||||
content: vec![
|
||||
let response = FunctionToolOutput::from_content(
|
||||
vec![
|
||||
FunctionCallOutputContentItem::InputText {
|
||||
text: "line 1".to_string(),
|
||||
},
|
||||
@@ -276,8 +262,8 @@ mod tests {
|
||||
text: "line 2".to_string(),
|
||||
},
|
||||
],
|
||||
success: Some(true),
|
||||
})
|
||||
Some(true),
|
||||
)
|
||||
.into_response("call-99", &payload);
|
||||
|
||||
match response {
|
||||
@@ -305,14 +291,18 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn log_preview_uses_content_items_when_plain_text_is_missing() {
|
||||
let output = ContentToolOutput {
|
||||
content: vec![FunctionCallOutputContentItem::InputText {
|
||||
let output = FunctionToolOutput::from_content(
|
||||
vec![FunctionCallOutputContentItem::InputText {
|
||||
text: "preview".to_string(),
|
||||
}],
|
||||
success: Some(true),
|
||||
};
|
||||
Some(true),
|
||||
);
|
||||
|
||||
assert_eq!(output.log_preview(), "preview");
|
||||
assert_eq!(
|
||||
function_call_output_content_items_to_text(&output.body),
|
||||
Some("preview".to_string())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user