Merge 7d63c2ea5f into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-05-22 18:27:13 -07:00
committed by GitHub
10 changed files with 498 additions and 232 deletions
@@ -2,9 +2,12 @@ use serde::Deserialize;
use crate::function_tool::FunctionCallError;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
@@ -110,4 +113,22 @@ impl ToolExecutor<ToolInvocation> for CodeModeWaitHandler {
}
}
impl CoreToolRuntime for CodeModeWaitHandler {}
impl CoreToolRuntime for CodeModeWaitHandler {
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
// Code-mode `wait` is runtime control for an existing code cell, not a
// standalone user action. Tool calls made from code mode still flow
// through normal dispatch, but hooks should not block or rewrite the
// wait loop itself.
None
}
fn post_tool_use_payload(
&self,
_invocation: &ToolInvocation,
_result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
// The wait result feeds code-mode control flow, so do not let
// PostToolUse replace it with model-facing hook feedback.
None
}
}
@@ -4,17 +4,12 @@ use codex_tools::ConversationHistory;
use codex_tools::ToolCall as ExtensionToolCall;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use serde_json::Value;
use crate::function_tool::FunctionCallError;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::flat_tool_name;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
pub(crate) struct ExtensionToolAdapter(Arc<dyn codex_tools::ToolExecutor<ExtensionToolCall>>);
@@ -23,13 +18,6 @@ impl ExtensionToolAdapter {
pub(crate) fn new(executor: Arc<dyn codex_tools::ToolExecutor<ExtensionToolCall>>) -> Self {
Self(executor)
}
fn arguments_from_payload<'a>(&self, payload: &'a ToolPayload) -> Option<&'a str> {
let ToolPayload::Function { arguments } = payload else {
return None;
};
Some(arguments)
}
}
#[async_trait::async_trait]
@@ -60,30 +48,7 @@ impl ToolExecutor<ToolInvocation> for ExtensionToolAdapter {
impl CoreToolRuntime for ExtensionToolAdapter {
fn matches_kind(&self, payload: &ToolPayload) -> bool {
self.arguments_from_payload(payload).is_some()
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let arguments = self.arguments_from_payload(&invocation.payload)?;
Some(PreToolUsePayload {
tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()),
tool_input: extension_tool_hook_input(arguments),
})
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
let arguments = self.arguments_from_payload(&invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::new(flat_tool_name(&self.tool_name()).into_owned()),
tool_use_id: invocation.call_id.clone(),
tool_input: extension_tool_hook_input(arguments),
tool_response: result
.post_tool_use_response(&invocation.call_id, &invocation.payload)?,
})
matches!(payload, ToolPayload::Function { .. })
}
}
@@ -100,14 +65,6 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
}
}
fn extension_tool_hook_input(arguments: &str) -> Value {
if arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
}
serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string()))
}
#[cfg(test)]
mod tests {
use std::sync::Arc;
+3 -70
View File
@@ -9,10 +9,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::flat_tool_name;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
use crate::tools::registry::ToolTelemetryTags;
use crate::tools::tool_search_entry::ToolSearchInfo;
@@ -23,8 +20,6 @@ use codex_tools::ToolName;
use codex_tools::ToolSearchSourceInfo;
use codex_tools::ToolSpec;
use codex_tools::mcp_tool_to_responses_api_tool;
use serde_json::Map;
use serde_json::Value;
pub struct McpHandler {
tool_info: ToolInfo,
@@ -143,58 +138,6 @@ impl CoreToolRuntime for McpHandler {
tags
})
}
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Some(PreToolUsePayload {
tool_name: HookToolName::new(self.tool_name().to_string()),
tool_input: mcp_hook_tool_input(arguments),
})
}
fn with_updated_hook_input(
&self,
mut invocation: ToolInvocation,
updated_input: Value,
) -> Result<ToolInvocation, FunctionCallError> {
invocation.payload = match invocation.payload {
ToolPayload::Function { .. } => ToolPayload::Function {
arguments: serde_json::to_string(&updated_input).map_err(|err| {
FunctionCallError::RespondToModel(format!(
"failed to serialize rewritten MCP arguments: {err}"
))
})?,
},
payload => {
return Err(FunctionCallError::RespondToModel(format!(
"tool {} does not support hook input rewriting for payload {payload:?}",
self.tool_name()
)));
}
};
Ok(invocation)
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &dyn crate::tools::context::ToolOutput,
) -> Option<PostToolUsePayload> {
let ToolPayload::Function { .. } = &invocation.payload else {
return None;
};
let tool_response =
result.post_tool_use_response(&invocation.call_id, &invocation.payload)?;
Some(PostToolUsePayload {
tool_name: HookToolName::new(self.tool_name().to_string()),
tool_use_id: invocation.call_id.clone(),
tool_input: result.post_tool_use_input(&invocation.payload)?,
tool_response,
})
}
}
fn create_tool_spec(tool_info: &ToolInfo) -> Result<ToolSpec, serde_json::Error> {
@@ -223,14 +166,6 @@ fn create_tool_spec(tool_info: &ToolInfo) -> Result<ToolSpec, serde_json::Error>
}))
}
fn mcp_hook_tool_input(raw_arguments: &str) -> Value {
if raw_arguments.trim().is_empty() {
return Value::Object(Map::new());
}
serde_json::from_str(raw_arguments).unwrap_or_else(|_| Value::String(raw_arguments.to_string()))
}
fn build_mcp_search_text(info: &ToolInfo) -> String {
let tool_name = info.canonical_tool_name();
let mut schema_properties = info
@@ -288,6 +223,9 @@ mod tests {
use super::*;
use crate::session::tests::make_session_and_context;
use crate::tools::context::ToolCallSource;
use crate::tools::hook_names::HookToolName;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::turn_diff_tracker::TurnDiffTracker;
use pretty_assertions::assert_eq;
use serde_json::json;
@@ -447,11 +385,6 @@ mod tests {
);
}
#[test]
fn mcp_hook_tool_input_defaults_empty_args_to_object() {
assert_eq!(mcp_hook_tool_input(" "), json!({}));
}
#[test]
fn mcp_read_only_hint_supports_parallel_calls_without_server_opt_in() {
let mut read_only_info = tool_info("foo", "mcp__foo__", "read");
@@ -5,6 +5,7 @@ use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::PostToolUsePayload;
use crate::tools::registry::PreToolUsePayload;
use crate::tools::registry::ToolExecutor;
use crate::unified_exec::WriteStdinRequest;
use codex_protocol::protocol::EventMsg;
@@ -101,11 +102,20 @@ impl CoreToolRuntime for WriteStdinHandler {
matches!(payload, ToolPayload::Function { .. })
}
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
// `write_stdin` is transport for an existing exec session. Empty writes
// are background polls, and non-empty writes continue a command that
// already ran PreToolUse as Bash, so do not emit a second pre hook here.
None
}
fn post_tool_use_payload(
&self,
invocation: &ToolInvocation,
result: &dyn crate::tools::context::ToolOutput,
) -> Option<PostToolUsePayload> {
// A `write_stdin` poll can observe final completion for the original
// `exec_command`; emit that command's matching Bash PostToolUse.
post_unified_exec_tool_use_payload(invocation, result)
}
}
+12
View File
@@ -38,6 +38,18 @@ impl HookToolName {
}
}
/// Returns the hook identity for spawning sub-agents.
///
/// The serialized name remains `spawn_agent`, while `Agent` is accepted as
/// a matcher alias for compatibility with hook configurations that describe
/// sub-agent creation using Claude Code-style names.
pub(crate) fn spawn_agent() -> Self {
Self {
name: "spawn_agent".to_string(),
matcher_aliases: vec!["Agent".to_string()],
}
}
/// Returns the hook identity historically used for shell-like tools.
pub(crate) fn bash() -> Self {
Self::new("Bash")
+111 -15
View File
@@ -19,6 +19,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
use crate::tools::flat_tool_name;
use crate::tools::handlers::multi_agents_spec::MULTI_AGENT_V1_NAMESPACE;
use crate::tools::hook_names::HookToolName;
use crate::tools::lifecycle::notify_tool_finish;
use crate::tools::lifecycle::notify_tool_start;
@@ -26,6 +27,7 @@ use crate::tools::tool_dispatch_trace::ToolDispatchTrace;
use crate::tools::tool_search_entry::ToolSearchInfo;
use crate::util::error_or_panic;
use codex_extension_api::ToolCallOutcome;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::protocol::EventMsg;
use codex_tools::ToolName;
@@ -64,14 +66,47 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
fn post_tool_use_payload(
&self,
_invocation: &ToolInvocation,
_result: &dyn ToolOutput,
invocation: &ToolInvocation,
result: &dyn ToolOutput,
) -> Option<PostToolUsePayload> {
None
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Some(PostToolUsePayload {
tool_name: function_hook_tool_name(invocation),
tool_use_id: result.post_tool_use_id(&invocation.call_id),
tool_input: result
.post_tool_use_input(&invocation.payload)
.unwrap_or_else(|| function_hook_tool_input(arguments)),
tool_response: result
.post_tool_use_response(&invocation.call_id, &invocation.payload)
.or_else(|| {
// Most function tools can expose their model-facing output
// as the hook response. Outputs with a more stable hook
// contract should override post_tool_use_response above.
let ResponseInputItem::FunctionCallOutput {
output: FunctionCallOutputPayload { body, .. },
..
} = result.to_response_item(&invocation.call_id, &invocation.payload)
else {
return None;
};
serde_json::to_value(body).ok()
})?,
})
}
fn pre_tool_use_payload(&self, _invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
None
fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option<PreToolUsePayload> {
let ToolPayload::Function { arguments } = &invocation.payload else {
return None;
};
Some(PreToolUsePayload {
tool_name: function_hook_tool_name(invocation),
tool_input: function_hook_tool_input(arguments),
})
}
/// Rebuilds a tool invocation from hook-facing `tool_input`.
@@ -80,12 +115,25 @@ pub(crate) trait CoreToolRuntime: ToolExecutor<ToolInvocation> {
/// hook contract they expose from `pre_tool_use_payload`.
fn with_updated_hook_input(
&self,
_invocation: ToolInvocation,
_updated_input: Value,
invocation: ToolInvocation,
updated_input: Value,
) -> Result<ToolInvocation, FunctionCallError> {
Err(FunctionCallError::RespondToModel(
"tool does not support hook input rewriting".to_string(),
))
let ToolPayload::Function { .. } = &invocation.payload else {
return Err(FunctionCallError::RespondToModel(
"hook input rewrite received unsupported function tool payload".to_string(),
));
};
let arguments = serde_json::to_string(&updated_input).map_err(|err| {
FunctionCallError::RespondToModel(format!(
"failed to serialize rewritten {} arguments: {err}",
flat_tool_name(&invocation.tool_name)
))
})?;
Ok(ToolInvocation {
payload: ToolPayload::Function { arguments },
..invocation
})
}
/// Creates an optional consumer for streamed tool argument diffs.
@@ -133,6 +181,29 @@ impl AnyToolResult {
}
}
struct PostToolUseFeedbackOutput {
original: Box<dyn ToolOutput>,
model_visible: FunctionToolOutput,
}
impl ToolOutput for PostToolUseFeedbackOutput {
fn log_preview(&self) -> String {
self.original.log_preview()
}
fn success_for_logging(&self) -> bool {
self.original.success_for_logging()
}
fn to_response_item(&self, call_id: &str, payload: &ToolPayload) -> ResponseInputItem {
self.model_visible.to_response_item(call_id, payload)
}
fn code_mode_result(&self, payload: &ToolPayload) -> Value {
self.original.code_mode_result(payload)
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct PreToolUsePayload {
/// Hook-facing tool name model.
@@ -538,11 +609,15 @@ impl ToolRegistry {
};
if let Some(replacement_text) = replacement_text {
let mut guard = response_cell.lock().await;
if let Some(result) = guard.as_mut() {
result.result = Box::new(FunctionToolOutput::from_text(
replacement_text,
/*success*/ None,
));
if let Some(mut result) = guard.take() {
result.result = Box::new(PostToolUseFeedbackOutput {
original: result.result,
model_visible: FunctionToolOutput::from_text(
replacement_text,
/*success*/ None,
),
});
*guard = Some(result);
}
}
}
@@ -618,6 +693,27 @@ async fn handle_any_tool(
})
}
fn function_hook_tool_name(invocation: &ToolInvocation) -> HookToolName {
if invocation.tool_name.name == "spawn_agent"
&& matches!(
invocation.tool_name.namespace.as_deref(),
None | Some(MULTI_AGENT_V1_NAMESPACE)
)
{
return HookToolName::spawn_agent();
}
HookToolName::new(flat_tool_name(&invocation.tool_name).into_owned())
}
fn function_hook_tool_input(arguments: &str) -> Value {
if arguments.trim().is_empty() {
return Value::Object(serde_json::Map::new());
}
serde_json::from_str(arguments).unwrap_or_else(|_| Value::String(arguments.to_string()))
}
fn unsupported_tool_call_message(payload: &ToolPayload, tool_name: &ToolName) -> String {
match payload {
ToolPayload::Custom { .. } => format!("unsupported custom tool call: {tool_name}"),
+192
View File
@@ -172,6 +172,198 @@ fn handler_looks_up_namespaced_aliases_explicitly() {
);
}
#[tokio::test]
async fn function_tools_expose_default_hook_payloads_and_rewrites() -> anyhow::Result<()> {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let tool_name = codex_tools::ToolName::namespaced("functions.", "echo");
let handler = TestHandler {
tool_name: tool_name.clone(),
};
let invocation = ToolInvocation {
payload: ToolPayload::Function {
arguments: serde_json::json!({ "message": "hello" }).to_string(),
},
..test_invocation(Arc::new(session), Arc::new(turn), "call-1", tool_name)
};
let output =
crate::tools::context::FunctionToolOutput::from_text("echoed".to_string(), Some(true));
assert_eq!(
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: HookToolName::new("functions.echo"),
tool_input: serde_json::json!({ "message": "hello" }),
})
);
assert_eq!(
handler.post_tool_use_payload(&invocation, &output),
Some(PostToolUsePayload {
tool_name: HookToolName::new("functions.echo"),
tool_use_id: "call-1".to_string(),
tool_input: serde_json::json!({ "message": "hello" }),
tool_response: serde_json::json!("echoed"),
})
);
let invocation = handler
.with_updated_hook_input(invocation, serde_json::json!({ "message": "rewritten" }))?;
let ToolPayload::Function { arguments } = invocation.payload else {
panic!("generic rewritten function payload should remain function-shaped");
};
assert_eq!(
serde_json::from_str::<serde_json::Value>(&arguments)?,
serde_json::json!({ "message": "rewritten" })
);
Ok(())
}
#[tokio::test]
async fn function_hook_input_defaults_empty_arguments_to_object() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let tool_name = codex_tools::ToolName::plain("echo");
let handler = TestHandler {
tool_name: tool_name.clone(),
};
let invocation = ToolInvocation {
payload: ToolPayload::Function {
arguments: " ".to_string(),
},
..test_invocation(Arc::new(session), Arc::new(turn), "call-1", tool_name)
};
assert_eq!(
handler.pre_tool_use_payload(&invocation),
Some(PreToolUsePayload {
tool_name: HookToolName::new("echo"),
tool_input: serde_json::json!({}),
})
);
}
#[tokio::test]
async fn spawn_agent_function_tools_use_agent_matcher_alias() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let session = Arc::new(session);
let turn = Arc::new(turn);
let hook_payloads = [
codex_tools::ToolName::plain("spawn_agent"),
codex_tools::ToolName::namespaced(MULTI_AGENT_V1_NAMESPACE, "spawn_agent"),
]
.into_iter()
.map(|tool_name| {
let handler = TestHandler {
tool_name: tool_name.clone(),
};
let invocation = ToolInvocation {
payload: ToolPayload::Function {
arguments: serde_json::json!({ "message": "inspect this repo" }).to_string(),
},
..test_invocation(Arc::clone(&session), Arc::clone(&turn), "call-1", tool_name)
};
handler.pre_tool_use_payload(&invocation)
})
.collect::<Vec<_>>();
assert_eq!(
hook_payloads,
vec![
Some(PreToolUsePayload {
tool_name: HookToolName::spawn_agent(),
tool_input: serde_json::json!({ "message": "inspect this repo" }),
}),
Some(PreToolUsePayload {
tool_name: HookToolName::spawn_agent(),
tool_input: serde_json::json!({ "message": "inspect this repo" }),
}),
]
);
}
#[tokio::test]
async fn code_mode_wait_does_not_expose_default_hook_payloads() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let output = crate::tools::context::FunctionToolOutput::from_text("ok".to_string(), Some(true));
let wait = crate::tools::handlers::CodeModeWaitHandler;
let wait_invocation = test_invocation(
Arc::new(session),
Arc::new(turn),
"wait-call",
wait.tool_name(),
);
assert_eq!(wait.pre_tool_use_payload(&wait_invocation), None);
assert_eq!(wait.post_tool_use_payload(&wait_invocation, &output), None);
}
#[tokio::test]
async fn write_stdin_does_not_expose_default_pre_tool_use_payload() {
let (session, turn) = crate::session::tests::make_session_and_context().await;
let write_stdin = crate::tools::handlers::WriteStdinHandler;
let invocation = test_invocation(
Arc::new(session),
Arc::new(turn),
"write-stdin-call",
write_stdin.tool_name(),
);
assert_eq!(write_stdin.pre_tool_use_payload(&invocation), None);
}
#[test]
fn post_tool_use_feedback_output_keeps_code_mode_result_typed() {
let result = AnyToolResult {
call_id: "call-1".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
result: Box::new(PostToolUseFeedbackOutput {
original: Box::new(codex_tools::JsonToolOutput::new(
serde_json::json!({ "typed": true }),
)),
model_visible: crate::tools::context::FunctionToolOutput::from_text(
"hook feedback".to_string(),
/*success*/ None,
),
}),
post_tool_use_payload: None,
};
assert_eq!(
result.into_response(),
ResponseInputItem::FunctionCallOutput {
call_id: "call-1".to_string(),
output: codex_protocol::models::FunctionCallOutputPayload::from_text(
"hook feedback".to_string()
),
}
);
let result = AnyToolResult {
call_id: "call-1".to_string(),
payload: ToolPayload::Function {
arguments: "{}".to_string(),
},
result: Box::new(PostToolUseFeedbackOutput {
original: Box::new(codex_tools::JsonToolOutput::new(
serde_json::json!({ "typed": true }),
)),
model_visible: crate::tools::context::FunctionToolOutput::from_text(
"hook feedback".to_string(),
/*success*/ None,
),
}),
post_tool_use_payload: None,
};
assert_eq!(
result.code_mode_result(),
serde_json::json!({ "typed": true })
);
}
#[tokio::test]
async fn dispatch_notifies_tool_lifecycle_contributors() -> anyhow::Result<()> {
let (mut session, turn) = crate::session::tests::make_session_and_context().await;