clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-29 00:55:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Support original-detail metadata on MCP image outputs (#17714)

Browse Source 
## Summary
- honor `_meta["codex/imageDetail"] == "original"` on MCP image content
and map it to `detail: "original"` where supported
- strip that detail back out when the active model does not support
original-detail image inputs
- update code-mode `image(...)` to accept individual MCP image blocks
- teach `js_repl` / `codex.emitImage(...)` to preserve the same hint
from raw MCP image outputs
- document the new `_meta` contract and add generic RMCP-backed coverage
across protocol, core, code-mode, and js_repl paths
This commit is contained in:
Curtis 'Fjord' Hawthorne
2026-04-15 14:43:33 -07:00
committed by GitHub
parent 17d94bd1e3
commit 9e2fc31854
20 changed files with 905 additions and 368 deletions
Download Patch File Download Diff File Expand all files Collapse all files

635
codex-rs/core/tests/suite/rmcp_client.rs
View File

@@ -11,6 +11,8 @@ use std::time::UNIX_EPOCH;
use codex_config::types::McpServerConfig;
use codex_config::types::McpServerTransportConfig;
use codex_core::config::Config;
use codex_features::Feature;
use codex_login::CodexAuth;
use codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY;
use codex_models_manager::manager::RefreshStrategy;
@@ -33,10 +35,12 @@ use codex_protocol::user_input::UserInput;
use codex_utils_cargo_bin::cargo_bin;
use core_test_support::assert_regex_match;
use core_test_support::responses;
use core_test_support::responses::ev_custom_tool_call;
use core_test_support::responses::mount_models_once;
use core_test_support::responses::mount_sse_once;
use core_test_support::skip_if_no_network;
use core_test_support::stdio_server_bin;
use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use core_test_support::wait_for_event_with_timeout;
@@ -82,6 +86,82 @@ enum McpCallEvent {
End(String),
}
async fn wait_for_mcp_tool(fixture: &TestCodex, tool_name: &str) -> anyhow::Result<()> {
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
if tool_list.tools.contains_key(tool_name) {
return Ok(());
}
let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
if Instant::now() >= tools_ready_deadline {
panic!(
"timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}"
);
}
sleep(Duration::from_millis(200)).await;
}
}
#[derive(Default)]
struct TestMcpServerOptions {
supports_parallel_tool_calls: bool,
tool_timeout_sec: Option<Duration>,
}
fn stdio_transport(
command: String,
env: Option<HashMap<String, String>>,
env_vars: Vec<String>,
) -> McpServerTransportConfig {
McpServerTransportConfig::Stdio {
command,
args: Vec::new(),
env,
env_vars,
cwd: None,
}
}
fn insert_mcp_server(
config: &mut Config,
server_name: &str,
transport: McpServerTransportConfig,
options: TestMcpServerOptions,
) {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport,
enabled: true,
required: false,
supports_parallel_tool_calls: options.supports_parallel_tool_calls,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: options.tool_timeout_sec,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
);
if let Err(err) = config.mcp_servers.set(servers) {
panic!("test mcp servers should accept any configuration: {err}");
}
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial(mcp_test_value)]
async fn stdio_server_round_trip() -> anyhow::Result<()> {
@@ -121,37 +201,19 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: Some(HashMap::from([(
"MCP_TEST_VALUE".to_string(),
expected_env_value.to_string(),
)])),
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
insert_mcp_server(
config,
server_name,
stdio_transport(
rmcp_test_server_bin,
Some(HashMap::from([(
"MCP_TEST_VALUE".to_string(),
expected_env_value.to_string(),
)])),
Vec::new(),
),
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -282,34 +344,12 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
let rmcp_test_server_bin = stdio_server_bin()?;
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
insert_mcp_server(
config,
server_name,
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -417,34 +457,15 @@ async fn stdio_mcp_parallel_tool_calls_default_false_runs_serially() -> anyhow::
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
insert_mcp_server(
config,
server_name,
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
TestMcpServerOptions {
tool_timeout_sec: Some(Duration::from_secs(2)),
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
..Default::default()
},
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -567,34 +588,15 @@ async fn stdio_mcp_parallel_tool_calls_opt_in_runs_concurrently() -> anyhow::Res
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
insert_mcp_server(
config,
server_name,
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
TestMcpServerOptions {
supports_parallel_tool_calls: true,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: Some(Duration::from_secs(2)),
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -676,66 +678,25 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: Some(HashMap::from([(
"MCP_TEST_IMAGE_DATA_URL".to_string(),
OPENAI_PNG.to_string(),
)])),
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
insert_mcp_server(
config,
server_name,
stdio_transport(
rmcp_test_server_bin,
Some(HashMap::from([(
"MCP_TEST_IMAGE_DATA_URL".to_string(),
OPENAI_PNG.to_string(),
)])),
Vec::new(),
),
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
let session_model = fixture.session_configured.model.clone();
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
if tool_list.tools.contains_key(&tool_name) {
break;
}
let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
if Instant::now() >= tools_ready_deadline {
panic!(
"timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}"
);
}
sleep(Duration::from_millis(200)).await;
}
wait_for_mcp_tool(&fixture, &tool_name).await?;
fixture
.codex
@@ -830,6 +791,189 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial(mcp_test_value)]
async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let call_id = "img-original-detail-1";
let server_name = "rmcp";
let tool_name = format!("mcp__{server_name}__image_scenario");
let namespace = format!("mcp__{server_name}__");
mount_sse_once(
&server,
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call_with_namespace(
call_id,
&namespace,
"image_scenario",
r#"{"scenario":"image_only_original_detail"}"#,
),
responses::ev_completed("resp-1"),
]),
)
.await;
let final_mock = mount_sse_once(
&server,
responses::sse(vec![
responses::ev_assistant_message("msg-1", "rmcp original-detail image completed."),
responses::ev_completed("resp-2"),
]),
)
.await;
let rmcp_test_server_bin = stdio_server_bin()?;
let fixture = test_codex()
.with_model("gpt-5.3-codex")
.with_config(move |config| {
insert_mcp_server(
config,
server_name,
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
TestMcpServerOptions::default(),
);
})
.build(&server)
.await?;
let session_model = fixture.session_configured.model.clone();
wait_for_mcp_tool(&fixture, &tool_name).await?;
fixture
.codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "call the rmcp image_scenario tool".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: fixture.cwd.path().to_path_buf(),
approval_policy: AskForApproval::Never,
approvals_reviewer: None,
sandbox_policy: SandboxPolicy::new_read_only_policy(),
model: session_model,
effort: None,
summary: None,
service_tier: None,
collaboration_mode: None,
personality: None,
})
.await?;
wait_for_event(&fixture.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let output_item = final_mock.single_request().function_call_output(call_id);
let output = output_item["output"]
.as_array()
.expect("image MCP output should be content items");
assert_eq!(output.len(), 2);
assert_wall_time_header(
output[0]["text"]
.as_str()
.expect("first MCP image output item should be wall-time text"),
);
assert_eq!(
output[1],
json!({
"type": "input_image",
"image_url": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
"detail": "original",
})
);
server.verify().await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial(mcp_test_value)]
async fn js_repl_emit_image_preserves_original_detail_for_mcp_images() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let call_id = "js-repl-rmcp-image";
let rmcp_test_server_bin = stdio_server_bin()?;
let fixture = test_codex()
.with_model("gpt-5.3-codex")
.with_config(move |config| {
config
.features
.enable(Feature::JsRepl)
.expect("test config should allow feature update");
insert_mcp_server(
config,
"rmcp",
stdio_transport(rmcp_test_server_bin, /*env*/ None, Vec::new()),
TestMcpServerOptions::default(),
);
})
.build(&server)
.await?;
wait_for_mcp_tool(&fixture, "mcp__rmcp__image_scenario").await?;
mount_sse_once(
&server,
responses::sse(vec![
responses::ev_response_created("resp-1"),
ev_custom_tool_call(
call_id,
"js_repl",
r#"
const out = await codex.tool("mcp__rmcp__image_scenario", {
scenario: "image_only_original_detail",
});
const imageItem = out.output.find((item) => item.type === "input_image");
await codex.emitImage(imageItem);
"#,
),
responses::ev_completed("resp-1"),
]),
)
.await;
let final_mock = mount_sse_once(
&server,
responses::sse(vec![
responses::ev_assistant_message("msg-1", "done"),
responses::ev_completed("resp-2"),
]),
)
.await;
fixture
.submit_turn("use js_repl to emit the rmcp image scenario output")
.await?;
let output = final_mock.single_request().custom_tool_call_output(call_id);
let output_items = output["output"]
.as_array()
.expect("js_repl output should be content items");
let image_item = output_items
.iter()
.find(|item| item.get("type").and_then(Value::as_str) == Some("input_image"))
.expect("js_repl should emit an input_image item");
assert_eq!(
image_item.get("detail").and_then(Value::as_str),
Some("original")
);
assert!(
image_item
.get("image_url")
.and_then(Value::as_str)
.is_some_and(|image_url| image_url.starts_with("data:image/png;base64,")),
"js_repl should emit a png data URL"
);
server.verify().await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
#[serial(mcp_test_value)]
async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Result<()> {
@@ -909,37 +1053,19 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
let fixture = test_codex()
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: Some(HashMap::from([(
"MCP_TEST_IMAGE_DATA_URL".to_string(),
OPENAI_PNG.to_string(),
)])),
env_vars: Vec::new(),
cwd: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
insert_mcp_server(
config,
server_name,
stdio_transport(
rmcp_test_server_bin,
Some(HashMap::from([(
"MCP_TEST_IMAGE_DATA_URL".to_string(),
OPENAI_PNG.to_string(),
)])),
Vec::new(),
),
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -1041,34 +1167,16 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: None,
env_vars: vec!["MCP_TEST_VALUE".to_string()],
cwd: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
},
insert_mcp_server(
config,
server_name,
stdio_transport(
rmcp_test_server_bin,
/*env*/ None,
vec!["MCP_TEST_VALUE".to_string()],
),
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -1211,33 +1319,17 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> {
let fixture = test_codex()
.with_config(move |config| {
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: server_url,
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
insert_mcp_server(
config,
server_name,
McpServerTransportConfig::StreamableHttp {
url: server_url,
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
@@ -1441,62 +1533,23 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
// runs the full core suite in one process.
config.mcp_oauth_credentials_store_mode = serde_json::from_value(json!("file"))
.expect("`file` should deserialize as OAuthCredentialsStoreMode");
let mut servers = config.mcp_servers.get().clone();
servers.insert(
server_name.to_string(),
McpServerConfig {
transport: McpServerTransportConfig::StreamableHttp {
url: server_url,
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth_resource: None,
tools: HashMap::new(),
insert_mcp_server(
config,
server_name,
McpServerTransportConfig::StreamableHttp {
url: server_url,
bearer_token_env_var: None,
http_headers: None,
env_http_headers: None,
},
TestMcpServerOptions::default(),
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
let session_model = fixture.session_configured.model.clone();
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
if tool_list.tools.contains_key(&tool_name) {
break;
}
let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
if Instant::now() >= tools_ready_deadline {
panic!(
"timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}"
);
}
sleep(Duration::from_millis(200)).await;
}
wait_for_mcp_tool(&fixture, &tool_name).await?;
fixture
.codex