mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Include root turn IDs in host-owned Apps tool calls (#51421)
## What changed Add the turn's `root_turn_id` to `_codex_apps` request metadata for host-owned Apps MCP calls. Remove any tool-supplied value before populating it from turn metadata, and omit the field for user-configured or extension-owned Apps servers. ## Testing Extend unit coverage to verify that host-owned Apps calls replace an untrusted `root_turn_id` and user-configured Apps calls omit it. Update integration assertions for default and explicitly supplied root turn IDs, including approval flows, and verify that extension-owned calls omit the field. GitOrigin-RevId: 754a642c2fff36a866b8cffa9fb3e161f232de08
This commit is contained in:
@@ -523,6 +523,7 @@ async fn handle_approved_mcp_tool_call(
|
||||
&server,
|
||||
call_id,
|
||||
Some(&metadata),
|
||||
prepared_call.is_host_owned_apps(),
|
||||
);
|
||||
let request_meta = with_mcp_tool_call_ids_meta(
|
||||
request_meta,
|
||||
@@ -1308,6 +1309,7 @@ fn build_mcp_tool_call_request_meta(
|
||||
server: &str,
|
||||
call_id: &str,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
is_host_owned_apps: bool,
|
||||
) -> Option<serde_json::Value> {
|
||||
let mut request_meta = serde_json::Map::new();
|
||||
request_meta.insert(
|
||||
@@ -1336,6 +1338,16 @@ fn build_mcp_tool_call_request_meta(
|
||||
"call_id".to_string(),
|
||||
serde_json::Value::String(call_id.to_string()),
|
||||
);
|
||||
// Only the captured host-owned Apps registration receives Core lineage.
|
||||
codex_apps_meta.remove("root_turn_id");
|
||||
if is_host_owned_apps
|
||||
&& let Some(root_turn_id) = step_context.turn.turn_metadata_state.root_turn_id()
|
||||
{
|
||||
codex_apps_meta.insert(
|
||||
"root_turn_id".to_string(),
|
||||
serde_json::Value::String(root_turn_id),
|
||||
);
|
||||
}
|
||||
request_meta.insert(
|
||||
MCP_TOOL_CODEX_APPS_META_KEY.to_string(),
|
||||
serde_json::Value::Object(codex_apps_meta),
|
||||
|
||||
@@ -1103,6 +1103,7 @@ async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() {
|
||||
"custom_server",
|
||||
"call-custom",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false,
|
||||
)
|
||||
.expect("custom servers should receive turn metadata");
|
||||
let turn_metadata = meta
|
||||
@@ -1158,8 +1159,13 @@ async fn mcp_tool_call_request_meta_uses_the_issuing_step(
|
||||
let (_, turn_context) = make_session_and_context().await;
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let step_a = StepContext::for_test(Arc::clone(&turn_context));
|
||||
let original_meta =
|
||||
build_mcp_tool_call_request_meta(&step_a, "node_repl", "call-a", /*metadata*/ None);
|
||||
let original_meta = build_mcp_tool_call_request_meta(
|
||||
&step_a,
|
||||
"node_repl",
|
||||
"call-a",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false,
|
||||
);
|
||||
let mut step_b = StepContext::for_test(Arc::clone(&turn_context));
|
||||
let settings = Arc::make_mut(&mut Arc::get_mut(&mut step_b).expect("unique step").settings);
|
||||
update_selected_settings_for_test(settings, |selected| {
|
||||
@@ -1184,7 +1190,13 @@ async fn mcp_tool_call_request_meta_uses_the_issuing_step(
|
||||
expected["node_repl_auto_review_required"] =
|
||||
serde_json::json!(step_b.settings.model_info.node_repl_auto_review_required);
|
||||
assert_eq!(
|
||||
build_mcp_tool_call_request_meta(&step_b, "node_repl", "call-b", /*metadata*/ None),
|
||||
build_mcp_tool_call_request_meta(
|
||||
&step_b,
|
||||
"node_repl",
|
||||
"call-b",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
"callId": "call-b",
|
||||
crate::X_CODEX_TURN_METADATA_HEADER: expected,
|
||||
@@ -1192,7 +1204,13 @@ async fn mcp_tool_call_request_meta_uses_the_issuing_step(
|
||||
})),
|
||||
);
|
||||
assert_eq!(
|
||||
build_mcp_tool_call_request_meta(&step_a, "node_repl", "call-a", /*metadata*/ None),
|
||||
build_mcp_tool_call_request_meta(
|
||||
&step_a,
|
||||
"node_repl",
|
||||
"call-a",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false
|
||||
),
|
||||
original_meta,
|
||||
);
|
||||
}
|
||||
@@ -1231,6 +1249,7 @@ async fn guardian_mcp_tool_call_request_meta_excludes_actor_confirmation_policy(
|
||||
server,
|
||||
"call-guardian",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false
|
||||
),
|
||||
expected,
|
||||
"{server}: {:?}",
|
||||
@@ -1254,6 +1273,7 @@ async fn mcp_tool_call_request_meta_includes_turn_started_at_unix_ms() {
|
||||
"custom_server",
|
||||
"call-custom",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false,
|
||||
)
|
||||
.expect("custom servers should receive turn metadata");
|
||||
let turn_metadata = meta
|
||||
@@ -1341,7 +1361,13 @@ async fn plugin_mcp_tool_call_request_meta_includes_plugin_id() {
|
||||
metadata.plugin_id = Some("sample@test".to_string());
|
||||
|
||||
assert_eq!(
|
||||
build_mcp_tool_call_request_meta(&step_context, "sample", "call-plugin", Some(&metadata),),
|
||||
build_mcp_tool_call_request_meta(
|
||||
&step_context,
|
||||
"sample",
|
||||
"call-plugin",
|
||||
Some(&metadata),
|
||||
/*is_host_owned_apps*/ false
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
"callId": "call-plugin",
|
||||
crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata,
|
||||
@@ -1453,9 +1479,16 @@ async fn mcp_tool_call_item_includes_app_identity() {
|
||||
assert_eq!(item.read_only_hint, Some(false));
|
||||
}
|
||||
|
||||
#[test_case::test_case(false; "user configured Apps name")]
|
||||
#[test_case::test_case(true; "host owned Apps")]
|
||||
#[tokio::test]
|
||||
async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps_meta() {
|
||||
async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps_meta(
|
||||
is_host_owned_apps: bool,
|
||||
) {
|
||||
let (_, turn_context) = make_session_and_context().await;
|
||||
turn_context
|
||||
.turn_metadata_state
|
||||
.set_root_turn_id("causal-root".to_string());
|
||||
let turn_context = Arc::new(turn_context);
|
||||
let step_context = StepContext::for_test(Arc::clone(&turn_context));
|
||||
let expected_turn_metadata = expected_mcp_turn_metadata(&turn_context);
|
||||
@@ -1476,6 +1509,7 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
|
||||
"resource_uri": "connector://calendar/tools/calendar_create_event",
|
||||
"contains_mcp_source": true,
|
||||
"connector_id": "calendar",
|
||||
"root_turn_id": "untrusted-tool-metadata",
|
||||
})
|
||||
.as_object()
|
||||
.cloned()
|
||||
@@ -1484,22 +1518,28 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
|
||||
openai_file_input_optional_fields: None,
|
||||
};
|
||||
|
||||
let mut expected_apps_meta = serde_json::json!({
|
||||
"call_id": "call_abc123xyz789",
|
||||
"resource_uri": "connector://calendar/tools/calendar_create_event",
|
||||
"contains_mcp_source": true,
|
||||
"connector_id": "calendar",
|
||||
});
|
||||
if is_host_owned_apps {
|
||||
expected_apps_meta["root_turn_id"] = serde_json::json!("causal-root");
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
build_mcp_tool_call_request_meta(
|
||||
&step_context,
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"call_abc123xyz789",
|
||||
Some(&metadata),
|
||||
is_host_owned_apps
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
"callId": "call_abc123xyz789",
|
||||
crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata,
|
||||
MCP_TOOL_CODEX_APPS_META_KEY: {
|
||||
"call_id": "call_abc123xyz789",
|
||||
"resource_uri": "connector://calendar/tools/calendar_create_event",
|
||||
"contains_mcp_source": true,
|
||||
"connector_id": "calendar",
|
||||
},
|
||||
MCP_TOOL_CODEX_APPS_META_KEY: expected_apps_meta,
|
||||
}))
|
||||
);
|
||||
}
|
||||
@@ -1517,6 +1557,7 @@ async fn codex_apps_tool_call_request_meta_includes_call_id_without_existing_cod
|
||||
CODEX_APPS_MCP_SERVER_NAME,
|
||||
"call_abc123xyz789",
|
||||
/*metadata*/ None,
|
||||
/*is_host_owned_apps*/ false
|
||||
),
|
||||
Some(serde_json::json!({
|
||||
"callId": "call_abc123xyz789",
|
||||
|
||||
@@ -5,6 +5,7 @@ use anyhow::Result;
|
||||
use codex_config::test_support::CloudConfigBundleFixture;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::TurnStartOptions;
|
||||
use codex_core::config::Config;
|
||||
use codex_features::Feature;
|
||||
use codex_history::RolloutItem;
|
||||
@@ -377,6 +378,7 @@ async fn auto_session_approval_is_scoped_to_tool_link_id() -> Result<()> {
|
||||
let mut expected_result = json!({
|
||||
"_codex_apps": {
|
||||
"call_id": call_id,
|
||||
"root_turn_id": request.body_json()["client_metadata"]["turn_id"],
|
||||
"connector_id": "calendar",
|
||||
"contains_mcp_source": true,
|
||||
"resource_uri": "connector://calendar/tools/calendar_create_event"
|
||||
@@ -654,15 +656,39 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request(
|
||||
);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
submit_user_turn(
|
||||
&test,
|
||||
"Use [$calendar](app://calendar) to create a calendar event.",
|
||||
AskForApproval::OnRequest,
|
||||
PermissionProfile::Disabled,
|
||||
/*collaboration_mode*/ None,
|
||||
)
|
||||
.await?;
|
||||
let root_turn_id = "originating-calendar-turn";
|
||||
let (sandbox_policy, permission_profile) =
|
||||
turn_permission_fields(PermissionProfile::Disabled, test.cwd.path());
|
||||
test.codex
|
||||
.start_or_steer_turn(
|
||||
TurnInputRequest::user_input(vec to create a calendar event.".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}])
|
||||
.on_start(TurnStartOptions {
|
||||
root_turn_id: Some(root_turn_id.to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.with_thread_settings(ThreadSettingsOverrides {
|
||||
environments: Some(TurnEnvironmentSelections::new(
|
||||
test.config.cwd.clone(),
|
||||
vec![test.executor_environment().selection().clone()],
|
||||
)),
|
||||
approval_policy: Some(AskForApproval::OnRequest),
|
||||
sandbox_policy: Some(sandbox_policy),
|
||||
permission_profile,
|
||||
collaboration_mode: Some(CollaborationMode {
|
||||
mode: ModeKind::Default,
|
||||
settings: Settings {
|
||||
model: test.session_configured.model.clone(),
|
||||
reasoning_effort: None,
|
||||
developer_instructions: None,
|
||||
},
|
||||
}),
|
||||
..Default::default()
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
if strict_auto_review {
|
||||
let event = wait_for_event(&test.codex, |event| {
|
||||
@@ -731,10 +757,7 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request(
|
||||
2 + 2 * usize::from(strict_auto_review)
|
||||
);
|
||||
let response_body = response_requests[0].body_json();
|
||||
let turn_id = response_body["client_metadata"]["turn_id"]
|
||||
.as_str()
|
||||
.expect("Responses request turn id");
|
||||
assert_root_turn(&response_body, Some(turn_id))?;
|
||||
assert_root_turn(&response_body, Some(root_turn_id))?;
|
||||
let apps_tool_call = recorded_apps_tool_call_by_call_id(&server, call_id).await;
|
||||
let mcp_turn_metadata = apps_tool_call
|
||||
.pointer("/params/_meta/x-codex-turn-metadata")
|
||||
@@ -751,6 +774,11 @@ async fn approved_mcp_tool_call_metadata_records_prior_user_input_request(
|
||||
(None, None)
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
apps_tool_call.pointer("/params/_meta/_codex_apps/root_turn_id"),
|
||||
Some(&json!(root_turn_id))
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
apps_tool_call.pointer("/params/_meta/callId"),
|
||||
Some(&json!(call_id))
|
||||
|
||||
@@ -275,6 +275,7 @@ async fn codex_apps_file_params_omit_fields_absent_from_tool_schema() -> Result<
|
||||
apps_tool_call.pointer("/params/_meta/_codex_apps"),
|
||||
Some(&json!({
|
||||
"call_id": "extract-call-1",
|
||||
"root_turn_id": requests[0].body_json()["client_metadata"]["turn_id"],
|
||||
"resource_uri": DOCUMENT_EXTRACT_TEXT_RESOURCE_URI,
|
||||
"contains_mcp_source": true,
|
||||
"connector_id": "calendar",
|
||||
|
||||
@@ -656,6 +656,7 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
|
||||
Some(json!({
|
||||
"_codex_apps": {
|
||||
"call_id": "calendar-call-1",
|
||||
"root_turn_id": mock.requests()[0].body_json()["client_metadata"]["turn_id"],
|
||||
"resource_uri": CALENDAR_CREATE_EVENT_RESOURCE_URI,
|
||||
"contains_mcp_source": true,
|
||||
"connector_id": "calendar",
|
||||
@@ -678,6 +679,7 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
|
||||
apps_tool_call.pointer("/params/_meta/_codex_apps"),
|
||||
Some(&json!({
|
||||
"call_id": "calendar-call-1",
|
||||
"root_turn_id": first_request_body["client_metadata"]["turn_id"],
|
||||
"resource_uri": CALENDAR_CREATE_EVENT_RESOURCE_URI,
|
||||
"contains_mcp_source": true,
|
||||
"connector_id": "calendar",
|
||||
|
||||
@@ -305,6 +305,12 @@ async fn tool_start_receives_executed_mcp_call_for_connector(
|
||||
.and_then(Value::as_str),
|
||||
Some("calendar_list_events")
|
||||
);
|
||||
if matches!(owner, AppsServerOwner::Extension) {
|
||||
assert_eq!(
|
||||
executed_call.pointer("/params/_meta/_codex_apps/root_turn_id"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user