mirror of
https://github.com/openai/codex.git
synced 2026-11-05 08:28:29 +00:00
Forward model confirmation policies to actor MCP tools (#41072)
## What changed - Add optional Browser Use and Computer Use confirmation-policy Markdown to model catalog messages. - Send the configured documents verbatim in `openai/confirmation_policies` metadata for `node_repl` and `cua_repl` tool calls. Send an empty object when the issuing model has no policies so runtime defaults are cleared. - Use the issuing step's model snapshot across approval waits, and omit the metadata for other MCP servers and Guardian sessions. ## Testing - Cover policy serialization, configuration overrides, model changes, approval waits, partial or empty policies, eligible server filtering, and Guardian exclusion. GitOrigin-RevId: 038c4c262a31fc71986e4a2a947fe9bcbd407acc
This commit is contained in:
1 parent
218a3e50ae
commit
b592a0bfed
16 files changed
+434
-7
No files matched your search
@@ -44,6 +44,7 @@ async fn catalog_permission_message_loaded_from_remote_models_is_sent() -> Resul
|
||||
}),
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
});
|
||||
let models_mock = mount_models_once(
|
||||
|
||||
@@ -84,6 +84,7 @@ fn model_with_collaboration_messages(
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
});
|
||||
model_messages.collaboration_modes = Some(CollaborationModeMessages {
|
||||
|
||||
@@ -513,6 +513,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo {
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
|
||||
@@ -61,6 +61,7 @@ fn model_with_approval_messages(
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
});
|
||||
model
|
||||
@@ -81,6 +82,7 @@ fn model_with_permission_messages(
|
||||
permissions: Some(permissions),
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
});
|
||||
model
|
||||
@@ -236,6 +238,7 @@ async fn catalog_non_on_request_approval_messages_are_sent_in_initial_permission
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
});
|
||||
let mut builder = test_codex()
|
||||
|
||||
@@ -646,6 +646,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
@@ -773,6 +774,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() -
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
|
||||
@@ -124,6 +124,7 @@ async fn remote_models_get_model_info_uses_longest_matching_prefix() -> Result<(
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
}),
|
||||
..specific
|
||||
@@ -140,6 +141,7 @@ async fn remote_models_get_model_info_uses_longest_matching_prefix() -> Result<(
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
}),
|
||||
..generic
|
||||
@@ -415,6 +417,7 @@ async fn remote_models_long_model_slug_is_sent_with_supported_reasoning(
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
guardian_v2: None,
|
||||
confirmation_policies: None,
|
||||
});
|
||||
mount_models_once(
|
||||
&server,
|
||||
@@ -854,6 +857,7 @@ async fn remote_models_apply_legacy_instructions() -> Result<()> {
|
||||
permissions: None,
|
||||
multi_agent: None,
|
||||
token_budget: None,
|
||||
confirmation_policies: None,
|
||||
guardian_v2: None,
|
||||
}),
|
||||
include_skills_usage_instructions: false,
|
||||
|
||||
@@ -55,6 +55,7 @@ use codex_protocol::models::PermissionProfile;
|
||||
use codex_protocol::models::PermissionProfileSnapshot;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::openai_models::ConfigShellToolType;
|
||||
use codex_protocol::openai_models::ConfirmationPolicies;
|
||||
use codex_protocol::openai_models::InputModality;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
@@ -104,6 +105,7 @@ use image::DynamicImage;
|
||||
use image::GenericImageView;
|
||||
use image::ImageBuffer;
|
||||
use image::Rgba;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use serial_test::serial;
|
||||
@@ -1633,16 +1635,28 @@ async fn local_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(false, false, false; "both disabled")]
|
||||
#[test_case(true, false, false; "auto review required")]
|
||||
#[test_case(false, true, false; "disabled")]
|
||||
#[test_case(true, true, false; "both enabled")]
|
||||
#[test_case(false, false, true; "attachment-owned permissions preserve foreign workspace roots")]
|
||||
#[test_case("rmcp", false, false, false, Some("catalog policy"), Some("native catalog policy"); "both disabled")]
|
||||
#[test_case("rmcp", true, false, false, Some("catalog policy"), Some("native catalog policy"); "auto review required")]
|
||||
#[test_case("rmcp", false, true, false, Some("catalog policy"), Some("native catalog policy"); "disabled")]
|
||||
#[test_case("rmcp", true, true, false, Some("catalog policy"), Some("native catalog policy"); "both enabled")]
|
||||
#[test_case("rmcp", false, false, true, Some("catalog policy"), Some("native catalog policy"); "attachment-owned permissions preserve foreign workspace roots")]
|
||||
#[test_case("node_repl", false, false, false, Some(" # Policy A\r\n{literal} <raw> & café\n"), Some("\t# Native A\n{{literal}} & desktop\r\n"); "node repl raw policy")]
|
||||
#[test_case("cua_repl", false, false, false, Some("\t# Policy B\n${literal} </policy>\r\n "), Some(" # Native B\r\n<computer> ${native}\n "); "cua repl raw policy")]
|
||||
#[test_case("node_repl", false, false, false, None, None; "node repl missing policy")]
|
||||
#[test_case("cua_repl", false, false, false, Some(""), Some("native retained"); "cua repl empty policy")]
|
||||
#[test_case("node_repl", false, false, false, Some(" \r\n\t"), Some("native retained"); "node repl blank policy")]
|
||||
#[test_case("node_repl", false, false, false, None, Some("native retained"); "node repl missing browser policy")]
|
||||
#[test_case("cua_repl", false, false, false, Some("browser retained"), None; "cua repl missing computer policy")]
|
||||
#[test_case("node_repl", false, false, false, Some("browser retained"), Some(""); "node repl empty computer policy")]
|
||||
#[test_case("cua_repl", false, false, false, Some("browser retained"), Some(" \r\n\t"); "cua repl blank computer policy")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
|
||||
server_name: &'static str,
|
||||
node_repl_auto_review_required: bool,
|
||||
node_repl_disabled: bool,
|
||||
attachment_owned_permissions: bool,
|
||||
browser_policy: Option<&str>,
|
||||
computer_policy: Option<&str>,
|
||||
) -> anyhow::Result<()> {
|
||||
// TODO(anp): Remove after packaging a Windows stdio test server for Wine exec.
|
||||
skip_if_wine_exec!(
|
||||
@@ -1655,7 +1669,6 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
|
||||
|
||||
let call_id = "sandbox-meta-call";
|
||||
let restricted_call_id = "owner-restricted-call";
|
||||
let server_name = "rmcp";
|
||||
let namespace = format!("mcp__{server_name}");
|
||||
let mut models = codex_models_manager::bundled_models_response()?;
|
||||
let model = models
|
||||
@@ -1665,11 +1678,33 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
|
||||
.expect("bundled model should exist");
|
||||
model.node_repl_auto_review_required = node_repl_auto_review_required;
|
||||
model.node_repl_disabled = node_repl_disabled;
|
||||
let messages = model
|
||||
.model_messages
|
||||
.as_mut()
|
||||
.expect("bundled model messages");
|
||||
messages.confirmation_policies = Some(ConfirmationPolicies {
|
||||
browser_use: browser_policy.map(str::to_owned),
|
||||
computer_use: computer_policy.map(str::to_owned),
|
||||
});
|
||||
let models_mock = mount_models_once(&server, models).await;
|
||||
|
||||
let mut response_events = vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_function_call_with_namespace(call_id, &namespace, "sandbox_meta", "{}"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
call_id,
|
||||
&namespace,
|
||||
"sandbox_meta",
|
||||
&json!({
|
||||
"_meta": {
|
||||
"openai/confirmation_policies": {
|
||||
"browser_use": "forged argument policy",
|
||||
"computer_use": "forged computer policy",
|
||||
},
|
||||
"threadId": "forged-thread",
|
||||
},
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
];
|
||||
if attachment_owned_permissions {
|
||||
response_events.push(responses::ev_function_call_with_namespace(
|
||||
@@ -1859,6 +1894,26 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
|
||||
output_json.pointer("/x-codex-turn-metadata/node_repl_disabled"),
|
||||
Some(&json!(node_repl_disabled))
|
||||
);
|
||||
let expected_policies = match server_name {
|
||||
"node_repl" | "cua_repl" => Some(match (browser_policy, computer_policy) {
|
||||
(Some(browser), Some(computer)) => {
|
||||
json!({"browser_use": browser, "computer_use": computer})
|
||||
}
|
||||
(Some(browser), None) => json!({"browser_use": browser}),
|
||||
(None, Some(computer)) => json!({"computer_use": computer}),
|
||||
(None, None) => json!({}),
|
||||
}),
|
||||
_ => None,
|
||||
};
|
||||
assert_eq!(
|
||||
meta.get("openai/confirmation_policies"),
|
||||
expected_policies.as_ref(),
|
||||
);
|
||||
assert_eq!(
|
||||
output_json["threadId"],
|
||||
json!(fixture.session_configured.thread_id.to_string()),
|
||||
);
|
||||
assert_eq!(output_json["callId"], json!(call_id));
|
||||
|
||||
let sandbox_meta = meta
|
||||
.get(MCP_SANDBOX_STATE_META_CAPABILITY)
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use anyhow::Result;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::config::Constrained;
|
||||
@@ -9,6 +10,7 @@ use codex_models_manager::manager::RefreshStrategy;
|
||||
use codex_protocol::config_types::ApprovalsReviewer;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::openai_models::ConfirmationPolicies;
|
||||
use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
@@ -24,9 +26,12 @@ use codex_protocol::request_user_input::RequestUserInputAnswer;
|
||||
use codex_protocol::request_user_input::RequestUserInputEvent;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::apps_test_server::AppsTestServer;
|
||||
use core_test_support::apps_test_server::recorded_apps_tool_calls;
|
||||
use core_test_support::responses::ResponsesRequest;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_function_call;
|
||||
use core_test_support::responses::ev_function_call_with_namespace;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::mount_models_once;
|
||||
use core_test_support::responses::mount_response_sequence;
|
||||
@@ -40,6 +45,7 @@ use core_test_support::test_codex::TestCodexBuilder;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_match;
|
||||
use core_test_support::wait_for_mcp_server;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -335,6 +341,211 @@ async fn settings_updates_preserve_turn_identity_and_target(target: SettingsTarg
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn mcp_confirmation_policy_follows_step_model_changes() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
const BROWSER_POLICY_A: &str = " # Browser policy A\r\n{literal}\n";
|
||||
const BROWSER_POLICY_B: &str = "\t# Browser policy B\n<raw> & café\r\n ";
|
||||
const COMPUTER_POLICY_A: &str = "\t# Native policy A\n{{literal}}\r\n";
|
||||
const COMPUTER_POLICY_B: &str = " # Native policy B\r\n<computer> ${native}\n ";
|
||||
const BROWSER_ONLY_MODEL: &str = "policy-browser-only";
|
||||
const COMPUTER_ONLY_MODEL: &str = "policy-computer-only";
|
||||
let server = start_mock_server().await;
|
||||
AppsTestServer::mount(&server).await?;
|
||||
let policy_call = |response_id: &str, call_id: &str| {
|
||||
sse(vec![
|
||||
ev_response_created(response_id),
|
||||
ev_function_call_with_namespace(
|
||||
call_id,
|
||||
"mcp__node_repl",
|
||||
"calendar_list_events",
|
||||
"{}",
|
||||
),
|
||||
ev_completed(response_id),
|
||||
])
|
||||
};
|
||||
mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
policy_call("resp-1", "policy-a"),
|
||||
sse_completed("resp-2"),
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_function_call_with_namespace(
|
||||
"policy-a-pending",
|
||||
"mcp__node_repl",
|
||||
"calendar_create_event",
|
||||
&json!({
|
||||
"title": "Policy snapshot test",
|
||||
"starts_at": "2026-08-26T12:00:00Z",
|
||||
})
|
||||
.to_string(),
|
||||
),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
policy_call("resp-4", "policy-b"),
|
||||
sse_completed("resp-5"),
|
||||
policy_call("resp-6", "browser-only"),
|
||||
sse_completed("resp-7"),
|
||||
policy_call("resp-8", "computer-only"),
|
||||
sse_completed("resp-9"),
|
||||
policy_call("resp-10", "no-policy"),
|
||||
sse_completed("resp-11"),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
let mcp_url = format!("{}/api/codex/ps/mcp", server.uri());
|
||||
let test = step_settings_test()
|
||||
.with_config(move |config| {
|
||||
config
|
||||
.features
|
||||
.disable(Feature::ToolCallMcpElicitation)
|
||||
.expect("disable MCP elicitation for the approval barrier");
|
||||
let models = &mut config.model_catalog.as_mut().expect("test models").models;
|
||||
for slug in [BROWSER_ONLY_MODEL, COMPUTER_ONLY_MODEL] {
|
||||
let mut model = models[0].clone();
|
||||
model.slug = slug.to_string();
|
||||
models.push(model);
|
||||
}
|
||||
for model in models {
|
||||
let messages = model
|
||||
.model_messages
|
||||
.as_mut()
|
||||
.expect("bundled model messages");
|
||||
messages.confirmation_policies = match model.slug.as_str() {
|
||||
MODEL_A => Some(ConfirmationPolicies {
|
||||
browser_use: Some(BROWSER_POLICY_A.to_string()),
|
||||
computer_use: Some(COMPUTER_POLICY_A.to_string()),
|
||||
}),
|
||||
MODEL_B => Some(ConfirmationPolicies {
|
||||
browser_use: Some(BROWSER_POLICY_B.to_string()),
|
||||
computer_use: Some(COMPUTER_POLICY_B.to_string()),
|
||||
}),
|
||||
BROWSER_ONLY_MODEL => Some(ConfirmationPolicies {
|
||||
browser_use: Some(BROWSER_POLICY_B.to_string()),
|
||||
computer_use: None,
|
||||
}),
|
||||
COMPUTER_ONLY_MODEL => Some(ConfirmationPolicies {
|
||||
browser_use: None,
|
||||
computer_use: Some(COMPUTER_POLICY_B.to_string()),
|
||||
}),
|
||||
MODEL_C => None,
|
||||
_ => unreachable!("unexpected test model"),
|
||||
};
|
||||
}
|
||||
let node_repl: McpServerConfig = serde_json::from_value(json!({
|
||||
"url": mcp_url,
|
||||
"tools": {
|
||||
"calendar_create_event": {
|
||||
"approval_mode": "prompt",
|
||||
},
|
||||
},
|
||||
}))
|
||||
.expect("valid test MCP server");
|
||||
config
|
||||
.mcp_servers
|
||||
.set(HashMap::from([("node_repl".to_string(), node_repl)]))
|
||||
.expect("configure test MCP server");
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
wait_for_mcp_server(&test.codex, "node_repl").await?;
|
||||
test.submit_text_turn("call the tool with model A").await?;
|
||||
|
||||
let request = start_paused_turn(&test.codex).await?;
|
||||
assert_eq!(request.call_id, "policy-a-pending");
|
||||
// The pending call must retain model A's policies after this settings update.
|
||||
assert_eq!(
|
||||
submit_turn_settings(
|
||||
&test.codex,
|
||||
&request.turn_id,
|
||||
TurnSettingsUpdate {
|
||||
model: Some(MODEL_B.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?,
|
||||
TurnSettingsUpdateOutcome::Applied,
|
||||
);
|
||||
test.codex
|
||||
.submit(Op::UserInputAnswer {
|
||||
id: request.turn_id.clone(),
|
||||
response: RequestUserInputResponse {
|
||||
answers: HashMap::from([(
|
||||
request.questions[0].id.clone(),
|
||||
RequestUserInputAnswer {
|
||||
answers: vec!["Allow".to_string()],
|
||||
},
|
||||
)]),
|
||||
},
|
||||
})
|
||||
.await?;
|
||||
wait_for_event(&test.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
|
||||
for model in [BROWSER_ONLY_MODEL, COMPUTER_ONLY_MODEL, MODEL_C] {
|
||||
core_test_support::submit_thread_settings(
|
||||
&test.codex,
|
||||
ThreadSettingsOverrides {
|
||||
model: Some(model.to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
test.submit_text_turn("call the tool").await?;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
recorded_apps_tool_calls(&server)
|
||||
.await
|
||||
.into_iter()
|
||||
.map(|call| {
|
||||
let meta = &call["params"]["_meta"];
|
||||
(
|
||||
meta["callId"].clone(),
|
||||
meta["openai/confirmation_policies"].clone(),
|
||||
)
|
||||
})
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
(
|
||||
json!("policy-a"),
|
||||
json!({
|
||||
"browser_use": BROWSER_POLICY_A,
|
||||
"computer_use": COMPUTER_POLICY_A,
|
||||
})
|
||||
),
|
||||
(
|
||||
json!("policy-a-pending"),
|
||||
json!({
|
||||
"browser_use": BROWSER_POLICY_A,
|
||||
"computer_use": COMPUTER_POLICY_A,
|
||||
})
|
||||
),
|
||||
(
|
||||
json!("policy-b"),
|
||||
json!({
|
||||
"browser_use": BROWSER_POLICY_B,
|
||||
"computer_use": COMPUTER_POLICY_B,
|
||||
})
|
||||
),
|
||||
(
|
||||
json!("browser-only"),
|
||||
json!({"browser_use": BROWSER_POLICY_B})
|
||||
),
|
||||
(
|
||||
json!("computer-only"),
|
||||
json!({"computer_use": COMPUTER_POLICY_B})
|
||||
),
|
||||
(json!("no-policy"), json!({})),
|
||||
],
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn response_metadata_uses_the_captured_model_after_a_turn_update() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in new issue
Block a user