mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
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
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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!({
|
||||
|
||||
@@ -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!({
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -386,6 +386,7 @@ fn submit_nested_tool(
|
||||
call_id,
|
||||
payload,
|
||||
encrypted_function_args: None,
|
||||
native_call: None,
|
||||
};
|
||||
session
|
||||
.services
|
||||
|
||||
@@ -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<Arc<ResponseItem>>,
|
||||
/// Captured before PreToolUse and retained across input rewrites.
|
||||
pub(crate) environments: ToolEnvironments,
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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!({
|
||||
|
||||
@@ -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!(
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<ToolInvocation> 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<Session>) -> Option<BoxFuture<'a, ()>> {
|
||||
self.handler.wait_until_ready(session)
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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(),
|
||||
)
|
||||
|
||||
@@ -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<ToolInvocation> {
|
||||
/// 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<dyn CoreToolRuntime>,
|
||||
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<Arc<ToolSpec>>,
|
||||
}
|
||||
@@ -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<dyn CoreToolRuntime>,
|
||||
exposure: ToolExposure,
|
||||
) {
|
||||
self.register_trusted_with_native_review(runtime, exposure, NativeReview::Disabled);
|
||||
}
|
||||
|
||||
pub(crate) fn register_trusted_with_native_review(
|
||||
&mut self,
|
||||
runtime: Arc<dyn CoreToolRuntime>,
|
||||
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);
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<Vec<String>>,
|
||||
/// Original function call, retained only for tools that opt into native review.
|
||||
pub native_call: Option<Arc<ResponseItem>>,
|
||||
}
|
||||
|
||||
// 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<Option<ToolCall>, FunctionCallError> {
|
||||
pub fn build_tool_call(
|
||||
&self,
|
||||
mut item: ResponseItem,
|
||||
) -> Result<Option<ToolCall>, 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(),
|
||||
};
|
||||
|
||||
|
||||
@@ -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<dyn CoreToolRuntime>;
|
||||
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),
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
};
|
||||
|
||||
@@ -367,6 +367,7 @@ fn test_invocation_with_payload(
|
||||
tool_name,
|
||||
source,
|
||||
payload,
|
||||
native_call: None,
|
||||
environments: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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::<Vec<_>>();
|
||||
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()
|
||||
|
||||
+1
-1
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user