From 058e99a3aa5f325c3d2b2b3ad1872bfca5a86be8 Mon Sep 17 00:00:00 2001 From: ioanbudea-OAI Date: Thu, 29 Oct 2026 11:06:56 +0000 Subject: [PATCH] Preserve native tool-call evidence through Guardian review (#58040) ## Why Guardian review needs the original function-call metadata even when the call is no longer in live conversation history. Looking it up in history can lose the evidence needed to review encrypted multi-agent messages. ## What changed Capture native-review policy in the tool registry and retain the original call through dispatch for multi-agent tools that require review. Use that retained call for Guardian review, applying hook-rewritten arguments while preserving encrypted-argument metadata and clearing stale replay content. Limit `ToolCall` debug output to the tool name and call ID. ## Testing Add coverage for selective call retention across review settings and namespaces. Extend Guardian tests to cover calls absent from live history and hook rewrites that preserve call identity and encrypted-argument metadata across incremental reviews. GitOrigin-RevId: 0e49833741c09c29228440bddd2e12695ae99583 --- codex-rs/core/src/client_tests.rs | 1 + codex-rs/core/src/compact_tests.rs | 1 + codex-rs/core/src/guardian/tests.rs | 10 +- codex-rs/core/src/session/tests.rs | 6 +- .../core/src/session/tests/guardian_tests.rs | 5 + codex-rs/core/src/stream_events_utils.rs | 3 +- codex-rs/core/src/tools/code_mode/mod.rs | 1 + codex-rs/core/src/tools/context.rs | 2 + .../request_metadata_tests.rs | 6 + .../tools/executed_tool_calls_direct_tests.rs | 2 + .../src/tools/handlers/apply_patch_tests.rs | 1 + .../src/tools/handlers/extension_tools.rs | 2 + codex-rs/core/src/tools/handlers/mcp.rs | 5 + .../src/tools/handlers/multi_agents_tests.rs | 1 + .../handlers/request_plugin_install_tests.rs | 1 + .../handlers/request_user_input_tests.rs | 3 + .../src/tools/handlers/unified_exec_tests.rs | 7 ++ .../core/src/tools/handlers/view_image.rs | 4 + codex-rs/core/src/tools/multi_agent_tool.rs | 11 -- codex-rs/core/src/tools/native_tool_review.rs | 13 +- codex-rs/core/src/tools/parallel.rs | 4 + .../src/tools/parallel_observation_tests.rs | 1 + .../core/src/tools/parallel_trace_tests.rs | 1 + codex-rs/core/src/tools/registry.rs | 34 ++++-- codex-rs/core/src/tools/registry_tests.rs | 14 ++- codex-rs/core/src/tools/router.rs | 49 ++++++-- codex-rs/core/src/tools/router_tests.rs | 114 +++++++++++++++--- codex-rs/core/src/tools/spec_plan.rs | 22 +++- codex-rs/core/src/tools/spec_plan_tests.rs | 4 + .../src/tools/tool_dispatch_trace_tests.rs | 1 + codex-rs/core/tests/suite/hooks.rs | 2 +- ...scenarios_guardian_agent_messages_tests.rs | 58 ++++++++- ...survives_incremental_guardian_reviews.snap | 2 +- 33 files changed, 309 insertions(+), 82 deletions(-) diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index 4f60892b53..9d4257dee2 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -554,6 +554,7 @@ async fn responses_request_includes_internal_metadata_for_provider_grant_or_firs arguments: json!({"query": id}).to_string(), }, encrypted_function_args: None, + native_call: None, }; let (mut recorded, permit) = recorder .prepare_direct_call(&call, &ToolCallSource::Direct, &step) diff --git a/codex-rs/core/src/compact_tests.rs b/codex-rs/core/src/compact_tests.rs index 790f0d7362..a57d2f0240 100644 --- a/codex-rs/core/src/compact_tests.rs +++ b/codex-rs/core/src/compact_tests.rs @@ -103,6 +103,7 @@ async fn local_compaction_retries_preserve_instructions_and_tool_metadata( arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }, &ToolCallSource::CodeMode { cell_id: cell.as_str().to_string(), diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index b09c5431aa..14c95bcb1d 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -3267,13 +3267,8 @@ async fn guardian_review_retries_two_parse_failures_then_approves() -> anyhow::R "arguments": r#"{"target":"worker","message":"opaque-message"}"#, "encrypted_content": "opaque-replay", "encrypted_function_args": ["message"], }); - session - .record_conversation_items( - &turn, - turn.model_info(), - &[serde_json::from_value(original.clone())?], - ) - .await; + // Dispatch owns the original call even when it is no longer in live history. + let native_call = Arc::new(serde_json::from_value(original.clone())?); let mut expected = original; expected .as_object_mut() @@ -3291,6 +3286,7 @@ async fn guardian_review_retries_two_parse_failures_then_approves() -> anyhow::R call_id: "action".to_owned(), tool_name: codex_tools::ToolName::namespaced("team", "send_message"), source: crate::tools::context::ToolCallSource::Direct, + native_call: Some(native_call), environments: Default::default(), payload: crate::tools::context::ToolPayload::Function { arguments: expected["arguments"].as_str().unwrap().to_owned(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index d875ed01fe..e73d4bf2dc 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -7986,6 +7986,7 @@ async fn request_permissions_tool_resolves_legacy_paths_against_selected_environ call_id, tool_name: codex_tools::ToolName::plain("request_permissions"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments }, }; @@ -8071,6 +8072,7 @@ async fn request_permissions_tool_rejects_invalid_requests( call_id: "call-1".to_string(), tool_name: codex_tools::ToolName::plain("request_permissions"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ @@ -13432,7 +13434,8 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { internal_chat_message_metadata_passthrough: None, }; - let call = ToolRouter::build_tool_call(item.clone()) + let call = router + .build_tool_call(item.clone()) .expect("build tool call") .expect("tool call present"); let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); @@ -13593,6 +13596,7 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() call_id: "exec-call".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 971f4badc6..df0c4a3c57 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -615,6 +615,7 @@ async fn guardian_allows_exec_command_additional_permissions_requests_past_polic call_id: "test-call".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ @@ -740,6 +741,7 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_exec_command_policy_s call_id: "strict-shell-command-call".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ @@ -893,6 +895,7 @@ async fn delayed_exec_command_uses_its_captured_authority_after_next_turn_starts call_id: call_id.to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ @@ -1071,6 +1074,7 @@ async fn guardian_allows_unified_exec_additional_permissions_requests_past_polic call_id: "exec-call".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ @@ -1196,6 +1200,7 @@ async fn exec_command_allows_sticky_turn_permissions_without_inline_request_perm call_id: "sticky-turn-grant".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index 964302c4f1..e5481956fe 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -16,7 +16,6 @@ use crate::session::step_context::StepContext; use crate::session::turn_context::TurnContext; use crate::tools::call_trace; use crate::tools::parallel::ToolCallRuntime; -use crate::tools::router::ToolRouter; use codex_memories_read::citations::parse_memory_citation; use codex_memories_read::citations::thread_ids_from_memory_citation; use codex_protocol::error::CodexErr; @@ -322,7 +321,7 @@ pub(crate) async fn handle_output_item_done( let mut output = OutputItemResult::default(); let plan_mode = ctx.step_context.turn.mode() == ModeKind::Plan; - match ToolRouter::build_tool_call(item.clone()) { + match ctx.step_context.tool_router.build_tool_call(item.clone()) { // The model emitted a tool call; log it, persist the item immediately, and queue the tool execution. Ok(Some(call)) => { call_trace::received( diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index 2d7dac04e1..936d1bc266 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -386,6 +386,7 @@ fn submit_nested_tool( call_id, payload, encrypted_function_args: None, + native_call: None, }; session .services diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index fe7de2c90b..252ca66b74 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -118,6 +118,8 @@ pub struct ToolInvocation { pub tool_name: ToolName, pub source: ToolCallSource, pub payload: ToolPayload, + /// Original function-call evidence owned by this invocation, independent of live history. + pub native_call: Option>, /// Captured before PreToolUse and retained across input rewrites. pub(crate) environments: ToolEnvironments, } diff --git a/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs b/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs index db927b9c82..b068d9026e 100644 --- a/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs +++ b/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs @@ -498,6 +498,7 @@ async fn recorder_refreshes_without_changing_execution_features_or_claiming_miss arguments: r#"{"value":7}"#.to_string(), }, encrypted_function_args: None, + native_call: None, }; let prepared = calls.prepare_direct_call(&call, &ToolCallSource::Direct, &step); let expected = ExecutedToolCall::new("test_tool".to_string(), json!({"value": 7})); @@ -591,6 +592,7 @@ fn executed_tool_call_recorder_bounds_pending_calls_and_preserves_overflow() { input: "".to_string(), }, encrypted_function_args: None, + native_call: None, }, &ToolCallSource::Direct, ToolMode::CodeMode, @@ -864,6 +866,7 @@ fn tool_call_completeness_survives_waits_without_changing_deltas() { arguments: json!({"cell_id": cell_id.as_str()}).to_string(), }, encrypted_function_args: None, + native_call: None, }, &ToolCallSource::Direct, ToolMode::CodeModeOnly, @@ -1131,6 +1134,7 @@ fn completeness_rejects_direct_id_collision_and_late_calls() { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }, &ToolCallSource::Direct, ToolMode::Direct, @@ -1171,6 +1175,7 @@ fn large_wrapped_arguments_preserve_later_wait_completeness() { arguments: arguments.to_string(), }, encrypted_function_args: None, + native_call: None, }); recorder.record_nested_tool_call(cell.clone(), "nested".to_string(), call, original_bytes); let mut initial = [exec_input("exec"), exec_output("exec")]; @@ -1621,6 +1626,7 @@ fn late_truncated_metadata_with_wrapped_arguments_rejects_ambiguous_outputs() { call_id: "other".to_string(), payload: ToolPayload::Function { arguments }, encrypted_function_args: None, + native_call: None, }); recorder.record_nested_tool_call( cell.clone(), diff --git a/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs b/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs index 304375bbb8..bcf53892de 100644 --- a/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs +++ b/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs @@ -188,6 +188,7 @@ async fn direct_pending_limit_releases_on_completion_or_dropped_future() { arguments: json!({ "argument": "kept" }).to_string(), }, encrypted_function_args: None, + native_call: None, }; let prepare = |recorder: &ExecutedToolCalls| { recorder.prepare_direct_call(&call, &ToolCallSource::Direct, &step) @@ -261,6 +262,7 @@ async fn direct_recording_preserves_encoded_arguments_in_history() { arguments: invalid_json.clone(), }, encrypted_function_args: None, + native_call: None, }; let mut item = output("direct"); let ordinary_output = serde_json::to_value(&item).unwrap()["output"].clone(); diff --git a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs index 64d118a58a..e30ea8c49f 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch_tests.rs @@ -55,6 +55,7 @@ async fn invocation_for_payload(payload: ToolPayload) -> ToolInvocation { tool_name: codex_tools::ToolName::plain("apply_patch"), source: crate::tools::context::ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), } } diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index fd2c47f821..c7f72f7b32 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -432,6 +432,7 @@ mod tests { call_id: "call-extension".to_string(), tool_name: codex_tools::ToolName::plain("extension_echo"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ "message": "hello" }).to_string(), @@ -533,6 +534,7 @@ mod tests { cell_id: "cell-1".to_string(), runtime_tool_call_id: "nested-call-1".to_string(), }, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ "message": "hello" }).to_string(), diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 8ac6845dea..cc850ba8c4 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -692,6 +692,7 @@ mod tests { tool_name: codex_tools::ToolName::namespaced("memory", "create_entities"), source: ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), }), Some(PreToolUsePayload { @@ -727,6 +728,7 @@ mod tests { tool_name: codex_tools::ToolName::namespaced("mcp__foo", "exec_command"), source: ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), }), Some(PreToolUsePayload { @@ -758,6 +760,7 @@ mod tests { tool_name: codex_tools::ToolName::namespaced("mcp__foo", "exec_command"), source: ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), }, json!({ "message": "rewritten" }), @@ -820,6 +823,7 @@ mod tests { tool_name: codex_tools::ToolName::namespaced("filesystem", "read_file"), source: ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), }; assert_eq!( @@ -860,6 +864,7 @@ mod tests { call_id: invocation.call_id.clone(), payload: invocation.payload.clone(), encrypted_function_args: None, + native_call: None, }, &invocation.source, &invocation.step_context, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index d5a8dcc9eb..0a9d78cd5c 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -117,6 +117,7 @@ fn invocation( tool_name: codex_tools::ToolName::plain(tool_name), source: crate::tools::context::ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), } } diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs index b4cff1e0b4..f6af475f90 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs @@ -66,6 +66,7 @@ async fn request_plugin_install_rejects_subagent_threads(presentation: ToolSugge call_id: "call-1".to_string(), tool_name: codex_tools::ToolName::plain(REQUEST_PLUGIN_INSTALL_TOOL_NAME), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: arguments.to_string(), diff --git a/codex-rs/core/src/tools/handlers/request_user_input_tests.rs b/codex-rs/core/src/tools/handlers/request_user_input_tests.rs index cb470abada..ecb5ed51bd 100644 --- a/codex-rs/core/src/tools/handlers/request_user_input_tests.rs +++ b/codex-rs/core/src/tools/handlers/request_user_input_tests.rs @@ -48,6 +48,7 @@ async fn multi_agent_v2_request_user_input_rejects_subagent_threads() { call_id: "call-1".to_string(), tool_name: codex_tools::ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ @@ -127,6 +128,7 @@ async fn request_user_input_sets_non_blocking_outside_plan_mode( call_id: "call-1".to_string(), tool_name: codex_tools::ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ @@ -249,6 +251,7 @@ async fn request_user_input_sets_blocking_from_turn_mode() { call_id: "call-1".to_string(), tool_name: codex_tools::ToolName::plain(REQUEST_USER_INPUT_TOOL_NAME), source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index e67f1ba4ad..14dedbc272 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -42,6 +42,7 @@ async fn invocation_for_payload( tool_name: codex_tools::ToolName::plain(tool_name), source: ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), } } @@ -353,6 +354,7 @@ async fn exec_command_rejects_login_when_selected_environment_disallows_it() { call_id: "login-disallowed".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ "cmd": "echo hello", "login": true }).to_string(), @@ -438,6 +440,7 @@ async fn exec_command_reuses_foreign_windows_grant() { call_id: "foreign-windows-grant".to_string(), tool_name: codex_tools::ToolName::plain("exec_command"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: serde_json::json!({ @@ -480,6 +483,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() { tool_name: codex_tools::ToolName::plain("exec_command"), source: crate::tools::context::ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), }), Some(crate::tools::registry::PreToolUsePayload { @@ -512,6 +516,7 @@ async fn exec_command_pre_tool_use_cwd_resolves_selected_environment() { payload: ToolPayload::Function { arguments: String::new(), }, + native_call: None, environments: Default::default(), }; for (workdir, expected) in [ @@ -582,6 +587,7 @@ async fn exec_command_pre_tool_use_payload_skips_write_stdin() { tool_name: codex_tools::ToolName::plain("write_stdin"), source: crate::tools::context::ToolCallSource::Direct, payload, + native_call: None, environments: Default::default(), }), None @@ -827,6 +833,7 @@ async fn exec_command_telemetry_records_configured_snapshot_mode( payload: ToolPayload::Function { arguments: r#"{"cmd":"true","login":false}"#.to_string(), }, + native_call: None, environments: Default::default(), }; assert_eq!( diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 1d4e4e6c4b..8c62cf2394 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -397,6 +397,7 @@ mod tests { call_id: "call-view-image".to_string(), tool_name: codex_tools::ToolName::plain("view_image"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ "path": "image.png" }).to_string(), @@ -428,6 +429,7 @@ mod tests { call_id: "call-view-image".to_string(), tool_name: codex_tools::ToolName::plain("view_image"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ "path": "image.png", "detail": "low" }).to_string(), @@ -472,6 +474,7 @@ mod tests { call_id: "call-view-image".to_string(), tool_name: codex_tools::ToolName::plain("view_image"), source: ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ "path": "image.png", "detail": "high" }).to_string(), @@ -514,6 +517,7 @@ mod tests { cell_id: "cell-1".to_string(), runtime_tool_call_id: "tool-1".to_string(), }, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: json!({ "path": "not-an-image.txt" }).to_string(), diff --git a/codex-rs/core/src/tools/multi_agent_tool.rs b/codex-rs/core/src/tools/multi_agent_tool.rs index 3e8e77410c..2b271af752 100644 --- a/codex-rs/core/src/tools/multi_agent_tool.rs +++ b/codex-rs/core/src/tools/multi_agent_tool.rs @@ -4,7 +4,6 @@ use crate::session::session::Session; use crate::tools::context::ToolInvocation; use crate::tools::registry::CoreToolRuntime; -use codex_protocol::openai_models::GuardianScope; use codex_tools::JsonSchema; use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; @@ -118,16 +117,6 @@ impl ToolExecutor for MultiAgentV2ToolOverrides { } impl CoreToolRuntime for MultiAgentV2ToolOverrides { - fn requires_guardian_review(&self, invocation: &ToolInvocation) -> bool { - invocation - .turn - .config - .features - .enabled(codex_features::Feature::GuardianNativeMultiAgentHistory) - && GuardianScope::for_multi_agent_tool(&self.tool_name(), self.namespace.as_deref()) - .is_some() - } - fn wait_until_ready<'a>(&'a self, session: &'a Arc) -> Option> { self.handler.wait_until_ready(session) } diff --git a/codex-rs/core/src/tools/native_tool_review.rs b/codex-rs/core/src/tools/native_tool_review.rs index bb65bedffb..0e251b4abf 100644 --- a/codex-rs/core/src/tools/native_tool_review.rs +++ b/codex-rs/core/src/tools/native_tool_review.rs @@ -21,17 +21,8 @@ pub(crate) async fn review(invocation: &ToolInvocation) -> Result<(), FunctionCa )); }; let mut item = invocation - .session - .clone_history() - .await - .raw_items() - .rev() - .find(|item| { - matches!( - item, - ResponseItem::FunctionCall { call_id, .. } if call_id == &invocation.call_id - ) - }) + .native_call + .as_deref() .cloned() .unwrap_or_else(|| ResponseItem::FunctionCall { id: None, diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index ffb17eb8a7..5ea5fce365 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -682,6 +682,7 @@ mod tests { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; let direct_guard = ToolCallTimingGuard::capture( Instant::now(), @@ -734,6 +735,7 @@ mod tests { input: "text('ready')".to_string(), }, encrypted_function_args: None, + native_call: None, }; let started_at = Instant::now() - Duration::from_secs(/*secs*/ 5); let mut timing = ToolCallTimingGuard::capture( @@ -847,6 +849,7 @@ mod tests { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; let response_task = tokio::spawn(runtime.handle_tool_call(call, cancellation_token.clone())); @@ -1020,6 +1023,7 @@ mod tests { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; let response_task = diff --git a/codex-rs/core/src/tools/parallel_observation_tests.rs b/codex-rs/core/src/tools/parallel_observation_tests.rs index a2f8711ece..132294fb2e 100644 --- a/codex-rs/core/src/tools/parallel_observation_tests.rs +++ b/codex-rs/core/src/tools/parallel_observation_tests.rs @@ -26,6 +26,7 @@ fn cancelled_dispatch_reports_zero_handler_work_even_without_tracing() { arguments: "{}".into(), }, encrypted_function_args: None, + native_call: None, }; let mut extensions = codex_extension_api::ExtensionRegistryBuilder::new(); extensions.tool_lifecycle_contributor(observer.clone()); diff --git a/codex-rs/core/src/tools/parallel_trace_tests.rs b/codex-rs/core/src/tools/parallel_trace_tests.rs index 9d03df0243..1c4f6b11b3 100644 --- a/codex-rs/core/src/tools/parallel_trace_tests.rs +++ b/codex-rs/core/src/tools/parallel_trace_tests.rs @@ -71,6 +71,7 @@ async fn spawned_dispatch_preserves_caller_context(level: LevelFilter) -> anyhow arguments: "{}".into(), }, encrypted_function_args: None, + native_call: None, }, CancellationToken::new(), ) diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 3dccc2eece..7a4e17f1ed 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -62,11 +62,6 @@ pub use codex_tools::ToolExposure; /// Implementers provide the shared `ToolExecutor` behavior plus optional /// core-owned metadata for hooks, telemetry, tool search, and argument diffs. pub(crate) trait CoreToolRuntime: ToolExecutor { - /// Opts this invocation into synchronous native-call review before the handler runs. - fn requires_guardian_review(&self, _invocation: &ToolInvocation) -> bool { - false - } - /// The handler observes its cancellation token and must finish before dispatch is aborted. fn finishes_on_cancellation(&self) -> bool { false @@ -395,10 +390,19 @@ pub(crate) struct PostToolUsePayload { pub(crate) tool_response: Value, } -/// A tool runtime together with its effective exposure for the current step. +/// Captured with the tool plan; review runs after hooks using the rewritten arguments. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub(crate) enum NativeReview { + #[default] + Disabled, + Required, +} + +/// A tool runtime together with its effective exposure and review policy for the current step. pub(crate) struct RegisteredTool { pub(crate) runtime: Arc, pub(crate) exposure: ToolExposure, + pub(crate) native_review: NativeReview, /// Per-step description override; never written back to a shared runtime. pub(crate) model_spec: Option>, } @@ -472,6 +476,7 @@ impl ToolRegistry { RegisteredTool { runtime, exposure, + native_review: NativeReview::Disabled, model_spec, } } @@ -517,12 +522,22 @@ impl ToolRegistry { &mut self, runtime: Arc, exposure: ToolExposure, + ) { + self.register_trusted_with_native_review(runtime, exposure, NativeReview::Disabled); + } + + pub(crate) fn register_trusted_with_native_review( + &mut self, + runtime: Arc, + exposure: ToolExposure, + native_review: NativeReview, ) { let tool_name = runtime.tool_name().with_default_namespace(); if !self.tool_policy.allows(&tool_name) { return; } - let tool = self.prepare_tool(runtime, exposure); + let mut tool = self.prepare_tool(runtime, exposure); + tool.native_review = native_review; match self.tools.entry(tool_name) { Entry::Vacant(entry) => { entry.insert(tool); @@ -750,7 +765,7 @@ impl ToolRegistry { } let dispatch_trace = ToolDispatchTrace::start(&invocation); - let tool = match self.tool(&tool_name) { + let registered_tool = match self.registered_tool(&tool_name) { Some(tool) => tool, None => { let message = unsupported_tool_call_message(&invocation.payload, &tool_name); @@ -772,6 +787,7 @@ impl ToolRegistry { return Err(err); } }; + let tool = Arc::clone(®istered_tool.runtime); let telemetry_tags = tool.telemetry_tags(&invocation); let mut tool_result_tags = Vec::with_capacity(2 + telemetry_tags.len() + 1); let mut extra_trace_fields = Vec::new(); @@ -882,7 +898,7 @@ impl ToolRegistry { } } - if tool.requires_guardian_review(&invocation) + if registered_tool.native_review == NativeReview::Required && let Err(err) = super::review_native_tool_call(&invocation).await { return Err(fail_before_execution(&invocation, err).await); diff --git a/codex-rs/core/src/tools/registry_tests.rs b/codex-rs/core/src/tools/registry_tests.rs index 01f18aa364..10f445297f 100644 --- a/codex-rs/core/src/tools/registry_tests.rs +++ b/codex-rs/core/src/tools/registry_tests.rs @@ -120,11 +120,7 @@ impl LifecycleTestHandler { } } -impl CoreToolRuntime for LifecycleTestHandler { - fn requires_guardian_review(&self, _invocation: &ToolInvocation) -> bool { - self.tool_name.name == "reviewed_tool" - } -} +impl CoreToolRuntime for LifecycleTestHandler {} impl codex_extension_api::ApprovalReviewContributor for LifecycleTestHandler { fn decide<'a>( @@ -778,7 +774,12 @@ async fn dispatch_uses_canonical_tool_names_for_lifecycle_contributors() -> anyh }); builder.approval_review_contributor(reviewed_handler.clone()); session.services.extensions = Arc::new(builder.build()); - let registry = ToolRegistry::from_tools([ok_handler, failing_handler, reviewed_handler]); + let mut registry = ToolRegistry::from_tools([ok_handler, failing_handler]); + registry.register_trusted_with_native_review( + reviewed_handler, + super::ToolExposure::Direct, + super::NativeReview::Required, + ); let session = Arc::new(session); let turn = Arc::new(turn); @@ -879,6 +880,7 @@ fn test_invocation( call_id: call_id.to_string(), tool_name, source: crate::tools::context::ToolCallSource::Direct, + native_call: None, environments: Default::default(), payload: ToolPayload::Function { arguments: "{}".to_string(), diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 36fbfe4427..d21d1508e3 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -11,6 +11,7 @@ use crate::tools::context::ToolPayload; #[cfg(test)] use crate::tools::handlers::ToolSearchHandlerCache; use crate::tools::registry::AnyToolResult; +use crate::tools::registry::NativeReview; use crate::tools::registry::RegisteredTool; use crate::tools::registry::ToolArgumentDiffConsumer; use crate::tools::registry::ToolExposure; @@ -44,12 +45,25 @@ const EXPOSURE_DEFERRED_MODEL_ONLY: &str = "deferred_model_only"; const EXPOSURE_CODE_MODE_ONLY: &str = "code_mode_only"; const EXPOSURE_HIDDEN: &str = "hidden"; -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, PartialEq)] pub struct ToolCall { pub tool_name: ToolName, pub call_id: String, pub payload: ToolPayload, pub encrypted_function_args: Option>, + /// Original function call, retained only for tools that opt into native review. + pub native_call: Option>, +} + +// The original call can contain credentials, plaintext messages, or replay ciphertext. +impl std::fmt::Debug for ToolCall { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("ToolCall") + .field("tool_name", &self.tool_name) + .field("call_id", &self.call_id) + .finish_non_exhaustive() + } } impl ToolCall { @@ -254,22 +268,37 @@ impl ToolRouter { } #[instrument(level = "trace", skip_all, err)] - pub fn build_tool_call(item: ResponseItem) -> Result, FunctionCallError> { + pub fn build_tool_call( + &self, + mut item: ResponseItem, + ) -> Result, FunctionCallError> { match item { ResponseItem::FunctionCall { - name, - namespace, - arguments, - encrypted_function_args, - call_id, + ref name, + ref namespace, + ref mut arguments, + ref call_id, + ref mut encrypted_function_args, .. } => { - let tool_name = ToolName::new(namespace, name).with_default_namespace(); + let tool_name = + ToolName::new(namespace.clone(), name.clone()).with_default_namespace(); + let retain_native_call = self + .registry + .registered_tool(&tool_name) + .is_some_and(|tool| tool.native_review == NativeReview::Required); + let (arguments, encrypted_function_args) = if retain_native_call { + (arguments.clone(), encrypted_function_args.clone()) + } else { + (std::mem::take(arguments), encrypted_function_args.take()) + }; + let call_id = call_id.clone(); Ok(Some(ToolCall { tool_name, call_id, payload: ToolPayload::Function { arguments }, encrypted_function_args, + native_call: retain_native_call.then(|| Arc::new(item)), })) } ResponseItem::ToolSearchCall { @@ -289,6 +318,7 @@ impl ToolRouter { call_id, payload: ToolPayload::ToolSearch { arguments }, encrypted_function_args: None, + native_call: None, })) } ResponseItem::ToolSearchCall { .. } => Ok(None), @@ -303,6 +333,7 @@ impl ToolRouter { call_id, payload: ToolPayload::Custom { input }, encrypted_function_args: None, + native_call: None, })), _ => Ok(None), } @@ -370,6 +401,7 @@ impl ToolRouter { tool_name, call_id, payload, + native_call, .. } = call; @@ -385,6 +417,7 @@ impl ToolRouter { tool_name, source, payload, + native_call, environments: Default::default(), }; diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 498fe97057..1544c07799 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -44,6 +44,57 @@ use super::tool_log_payload; struct ExtensionEchoContributor; +#[tokio::test] +async fn build_tool_call_retains_native_evidence_only_for_reviewed_mav2_tools() { + for review_enabled in [false, true] { + for namespace in [None, Some("team")] { + let (_, mut turn) = make_session_and_context().await; + turn.multi_agent_version = codex_protocol::protocol::MultiAgentVersion::V2; + let config = Arc::make_mut(&mut turn.config); + config.multi_agent_v2.tool_namespace = namespace.map(str::to_owned); + if review_enabled { + config + .features + .enable(codex_features::Feature::GuardianNativeMultiAgentHistory) + .unwrap(); + } else { + config + .features + .disable(codex_features::Feature::GuardianNativeMultiAgentHistory) + .unwrap(); + } + let step = StepContext::for_test(Arc::new(turn)); + let router = test_tool_router(&step, Vec::new(), Vec::new(), &[]); + for (name, mutating) in [ + ("spawn_agent", true), + ("send_message", true), + ("followup_task", true), + ("interrupt_agent", true), + ("list_agents", false), + ("wait_agent", false), + ] { + let item: ResponseItem = serde_json::from_value(json!({ + "type": "function_call", + "id": "fc_review", + "call_id": "call-review", + "namespace": namespace, + "name": name, + "arguments": r#"{"message":"opaque-message"}"#, + "encrypted_content": "opaque-replay", + "encrypted_function_args": ["message"], + })) + .unwrap(); + let call = router.build_tool_call(item.clone()).unwrap().unwrap(); + assert_eq!( + call.native_call.as_deref(), + (review_enabled && mutating).then_some(&item), + "{name}, namespace={namespace:?}, review_enabled={review_enabled}", + ); + } + } + } +} + #[test] fn tool_log_payload_redacts_plaintext_multi_agent_messages() { let payload = ToolPayload::Function { @@ -184,6 +235,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }) }) .expect("test session should expose a parallel shell-like tool"); @@ -202,6 +254,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, })); Ok(()) @@ -209,9 +262,11 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow #[tokio::test] async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<()> { + let (_, turn) = make_session_and_context().await; + let step = StepContext::for_test(Arc::new(turn)); let tool_name = "create_event".to_string(); - let call = ToolRouter::build_tool_call(ResponseItem::FunctionCall { + let call_item = ResponseItem::FunctionCall { status: None, encrypted_content: None, id: None, @@ -221,8 +276,11 @@ async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<() encrypted_function_args: Some(Vec::new()), call_id: "call-namespace".to_string(), internal_chat_message_metadata_passthrough: None, - })? - .expect("function_call should produce a tool call"); + }; + let call = step + .tool_router + .build_tool_call(call_item)? + .expect("function_call should produce a tool call"); assert_eq!( call.tool_name, @@ -243,9 +301,11 @@ async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<() #[tokio::test] async fn build_custom_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<()> { + let (_, turn) = make_session_and_context().await; + let step = StepContext::for_test(Arc::new(turn)); let tool_name = "exec".to_string(); - let call = ToolRouter::build_tool_call(ResponseItem::CustomToolCall { + let call_item = ResponseItem::CustomToolCall { encrypted_content: None, id: None, status: None, @@ -254,8 +314,11 @@ async fn build_custom_tool_call_uses_namespace_for_registry_name() -> anyhow::Re namespace: Some("mcp__python".to_string()), input: "print('hello')".to_string(), internal_chat_message_metadata_passthrough: None, - })? - .expect("custom_tool_call should produce a tool call"); + }; + let call = step + .tool_router + .build_tool_call(call_item)? + .expect("custom_tool_call should produce a tool call"); assert_eq!( call, @@ -266,16 +329,19 @@ async fn build_custom_tool_call_uses_namespace_for_registry_name() -> anyhow::Re input: "print('hello')".to_string(), }, encrypted_function_args: None, + native_call: None, } ); Ok(()) } -#[test] -fn build_tool_call_normalizes_default_function_and_custom_namespaces() -> anyhow::Result<()> { +#[tokio::test] +async fn build_tool_call_normalizes_default_function_and_custom_namespaces() -> anyhow::Result<()> { + let (_, turn) = make_session_and_context().await; + let step = StepContext::for_test(Arc::new(turn)); for namespace in [None, Some(""), Some(DEFAULT_FUNCTION_NAMESPACE)] { - let function_call = ToolRouter::build_tool_call(ResponseItem::FunctionCall { + let function_call_item = ResponseItem::FunctionCall { status: None, encrypted_content: None, id: None, @@ -285,9 +351,12 @@ fn build_tool_call_normalizes_default_function_and_custom_namespaces() -> anyhow encrypted_function_args: None, call_id: "call-function".to_string(), internal_chat_message_metadata_passthrough: None, - })? - .expect("function_call should produce a tool call"); - let custom_call = ToolRouter::build_tool_call(ResponseItem::CustomToolCall { + }; + let function_call = step + .tool_router + .build_tool_call(function_call_item)? + .expect("function_call should produce a tool call"); + let custom_call_item = ResponseItem::CustomToolCall { encrypted_content: None, id: None, status: None, @@ -296,8 +365,11 @@ fn build_tool_call_normalizes_default_function_and_custom_namespaces() -> anyhow namespace: namespace.map(str::to_string), input: "patch".to_string(), internal_chat_message_metadata_passthrough: None, - })? - .expect("custom_tool_call should produce a tool call"); + }; + let custom_call = step + .tool_router + .build_tool_call(custom_call_item)? + .expect("custom_tool_call should produce a tool call"); assert_eq!( [function_call.tool_name, custom_call.tool_name], @@ -364,6 +436,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; assert!(router.tool_supports_parallel(&call)); assert_eq!( @@ -380,6 +453,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; assert!(!router.tool_supports_parallel(&different_server_call)); assert_eq!( @@ -396,6 +470,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; assert!(!router.tool_supports_parallel(&hidden_call)); assert!(router.registered_tool(&hidden_call.tool_name).is_some()); @@ -407,6 +482,7 @@ async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, }; assert!(router.tool_supports_parallel(&nested_only_call)); @@ -432,6 +508,7 @@ async fn tools_without_handlers_do_not_support_parallel() -> anyhow::Result<()> arguments: "{}".to_string(), }, encrypted_function_args: None, + native_call: None, })); Ok(()) @@ -529,6 +606,7 @@ fn mcp_runtime(tool_info: codex_mcp::ToolInfo) -> RegisteredTool { as Arc; RegisteredTool { exposure: runtime.exposure(), + native_review: Default::default(), runtime, model_spec: None, } @@ -588,7 +666,7 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow "expected extension-provided tool to be visible to the model" ); - let call = ToolRouter::build_tool_call(ResponseItem::FunctionCall { + let call_item = ResponseItem::FunctionCall { status: None, encrypted_content: None, id: None, @@ -598,8 +676,10 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow call_id: "call-extension".to_string(), encrypted_function_args: None, internal_chat_message_metadata_passthrough: None, - })? - .expect("function_call should produce a tool call"); + }; + let call = router + .build_tool_call(call_item)? + .expect("function_call should produce a tool call"); let result = router .dispatch_tool_call_with_code_mode_result( Arc::new(session), diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 2602fcb9bb..80ffe7b138 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -54,6 +54,7 @@ use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::hosted_spec::WebSearchToolOptions; use crate::tools::hosted_spec::create_web_search_tool; use crate::tools::multi_agent_tool::multi_agent_v2_handler; +use crate::tools::registry::NativeReview; use crate::tools::registry::RegisteredTool; use crate::tools::registry::ToolExposure; use crate::tools::registry::ToolRegistry; @@ -1466,11 +1467,20 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, registry: &mut Too ToolExposure::Direct }; let tool_namespace = turn_context.config.multi_agent_v2.tool_namespace.as_deref(); + let native_review = if turn_context + .config + .features + .enabled(Feature::GuardianNativeMultiAgentHistory) + { + NativeReview::Required + } else { + NativeReview::Disabled + }; let agent_type_description = agent_type_description(turn_context, context.default_agent_type_description); let hide_spawn_agent_metadata = turn_context.config.multi_agent_v2.hide_spawn_agent_metadata; - registry.register_trusted_with_exposure( + registry.register_trusted_with_native_review( multi_agent_v2_handler( SpawnAgentHandlerV2::new( SpawnAgentToolOptions { @@ -1502,9 +1512,10 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, registry: &mut Too model_messages.multi_agent_tool_parameters_override("spawn_agent"), ), exposure, + native_review, ); if !turn_context.config.multi_agent_v2.disable_direct_message { - registry.register_trusted_with_exposure( + registry.register_trusted_with_native_review( multi_agent_v2_handler( SendMessageHandlerV2, tool_namespace, @@ -1512,8 +1523,9 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, registry: &mut Too model_messages.multi_agent_tool_parameters_override("send_message"), ), exposure, + native_review, ); - registry.register_trusted_with_exposure( + registry.register_trusted_with_native_review( multi_agent_v2_handler( FollowupTaskHandlerV2, tool_namespace, @@ -1521,6 +1533,7 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, registry: &mut Too model_messages.multi_agent_tool_parameters_override("followup_task"), ), exposure, + native_review, ); } if turn_context.config.multi_agent_v2.wait_agent_enabled { @@ -1534,7 +1547,7 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, registry: &mut Too exposure, ); } - registry.register_trusted_with_exposure( + registry.register_trusted_with_native_review( multi_agent_v2_handler( InterruptAgentHandler, tool_namespace, @@ -1542,6 +1555,7 @@ fn add_collaboration_tools(context: &CoreToolPlanContext<'_>, registry: &mut Too model_messages.multi_agent_tool_parameters_override("interrupt_agent"), ), exposure, + native_review, ); registry.register_trusted_with_exposure( multi_agent_v2_handler( diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 58351c7cac..19b17318af 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -430,6 +430,7 @@ fn mcp_runtime( RegisteredTool { runtime: handler, exposure, + native_review: Default::default(), model_spec: None, } } @@ -1735,6 +1736,7 @@ async fn strict_namespace_ownership_requires_tool_namespace_inventory_opt_in() { RegisteredTool { runtime: Arc::new(McpHandler::new(tool).expect("MCP tool spec should build")), exposure, + native_review: Default::default(), model_spec: None, } }) @@ -2031,6 +2033,7 @@ async fn strict_tool_collisions_allow_multiple_tools_in_one_namespace() { McpHandler::new(undocumented_tool).expect("MCP tool spec should build"), ), exposure: ToolExposure::Direct, + native_review: Default::default(), model_spec: None, }, mcp_runtime("shared", "shared", "lookup", ToolExposure::Direct), @@ -2073,6 +2076,7 @@ async fn relaxed_tool_collisions_preserve_first_nonempty_namespace_description() RegisteredTool { runtime: Arc::new(McpHandler::new(tool).expect("MCP tool spec should build")), exposure: ToolExposure::Direct, + native_review: Default::default(), model_spec: None, } }; diff --git a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs index e5cb4a2a1a..194c5d6bca 100644 --- a/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs +++ b/codex-rs/core/src/tools/tool_dispatch_trace_tests.rs @@ -367,6 +367,7 @@ fn test_invocation_with_payload( tool_name, source, payload, + native_call: None, environments: Default::default(), } } diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 175b68983c..06d51e3b00 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -531,7 +531,7 @@ elif mode == "exit_2": Ok(()) } -fn write_updating_pre_tool_use_hook( +pub(super) fn write_updating_pre_tool_use_hook( home: &Path, matcher: &str, updated_input: &Value, diff --git a/codex-rs/core/tests/suite/scenarios_guardian_agent_messages_tests.rs b/codex-rs/core/tests/suite/scenarios_guardian_agent_messages_tests.rs index b7c10eee63..61f2ef6d24 100644 --- a/codex-rs/core/tests/suite/scenarios_guardian_agent_messages_tests.rs +++ b/codex-rs/core/tests/suite/scenarios_guardian_agent_messages_tests.rs @@ -12,6 +12,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::ThreadSettingsOverrides; use core_test_support::context_snapshot; use core_test_support::context_snapshot::ContextSnapshotOptions; +use core_test_support::hooks::trust_discovered_hooks; use core_test_support::responses; use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; @@ -20,7 +21,7 @@ use pretty_assertions::assert_eq; use serde_json::json; use test_case::test_case; -#[test_case(None; "agent_reply")] +#[cfg_attr(not(target_os = "windows"), test_case(None; "agent_reply"))] #[test_case(Some("functions"); "native_default_namespace")] #[test_case(Some("team"); "native_custom_namespace")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -35,8 +36,9 @@ async fn encrypted_parent_reply_survives_incremental_guardian_reviews( id, "team", "send_message", - r#"{"target":"/root","message":"opaque-message"}"#, + r#"{"target":"/root/original","message":"opaque-message"}"#, ); + call["item"]["id"] = json!("fc_native_hook_rewrite"); call["item"]["encrypted_function_args"] = serde_json::json!(["message"]); call["item"]["encrypted_content"] = serde_json::json!("opaque-replay"); return responses::sse(vec![call, responses::ev_completed(id)]); @@ -124,9 +126,30 @@ async fn encrypted_parent_reply_survives_incremental_guardian_reviews( .permissions .set_permission_profile(PermissionProfile::read_only()) .expect("set read-only permission profile"); + if native_call_namespace.is_none() { + trust_discovered_hooks(config); + } + }); + #[cfg(not(target_os = "windows"))] + let test = if native_call_namespace.is_none() { + test.with_pre_build_hook(|home| { + super::super::hooks::write_updating_pre_tool_use_hook( + home, + "^teamsend_message$", + &json!({"target": "/root", "message": "opaque-message"}), + ) + .expect("write MAv2 hook rewrite"); }) - .build_with_auto_env(&server) - .await?; + } else { + test + }; + let mut test_builder = test; + let test = if native_call_namespace.is_none() { + // Command hooks run on the host and require a local working directory. + test_builder.build(&server).await? + } else { + test_builder.build_with_auto_env(&server).await? + }; let prompt = "Check the change after the parent confirms the exact action."; if historical_call.is_some() { // Full Access records the historical call before any reviewer sees it. @@ -174,6 +197,33 @@ async fn encrypted_parent_reply_survives_incremental_guardian_reviews( reviews[1].body_json()["client_metadata"]["thread_id"] ); assert!(reviews[1].input().starts_with(&reviews[0].input())); + if native_call_namespace.is_none() { + let reviewed_calls = reviews[1] + .inputs_of_type("function_call") + .into_iter() + .map(|mut item| { + item.as_object_mut() + .expect("function call should be an object") + .remove("internal_chat_message_metadata_passthrough"); + item + }) + .collect::>(); + assert_eq!( + reviewed_calls, + vec![json!({ + "type": "function_call", + "id": "fc_native_hook_rewrite", + "call_id": "second-check", + "namespace": "team", + "name": "send_message", + "arguments": serde_json::to_string(&json!({ + "target": "/root", + "message": "opaque-message", + }))?, + "encrypted_function_args": ["message"], + })], + ); + } for request in reviews.iter().copied().chain(requests.last()) { let messages = request .input() diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_agent_messages__encrypted_parent_reply_survives_incremental_guardian_reviews.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_agent_messages__encrypted_parent_reply_survives_incremental_guardian_reviews.snap index a348116fb2..1cc6862bb5 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_agent_messages__encrypted_parent_reply_survives_incremental_guardian_reviews.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__guardian_agent_messages__encrypted_parent_reply_survives_incremental_guardian_reviews.snap @@ -282,7 +282,7 @@ Scenario: An encrypted parent reply is delivered as a native agent message. Both 07:agent_message 08:function_call/exec_command:{"cmd":"exit 0","justification":"Check the requested change.","sandbox_permissions":"require_escalated"} 09:function_call_output:exec_command failed: CreateProcess { message: "Rejected(\\"This action was rejected due to unacceptable r...e risk. Otherwise, stop and request user input.\\")" } [hash=ae05158f838e6b05] -10:function_call/team.send_message:{"message":"opaque-message","target":"/root"} +10:function_call/team.send_message:{"message":"opaque-message","target":"/root/original"} 11:function_call_output:This action was rejected due to unacceptable risk. Reason: Mock assessment; no command is executed. The agent must not attempt to achieve the same outcome via workaround, indirect execution, or policy cir... of the risk. Otherwise, stop and request user input. [hash=316a6640caee3870]