mirror of
https://github.com/openai/codex.git
synced 2026-11-03 16:23:51 +00:00
feat: add Codex Apps sediment file remapping (#15197)
## Summary - bridge Codex Apps tools that declare `_meta["openai/fileParams"]` through the OpenAI file upload flow - mask those file params in model-visible tool schemas so the model provides absolute local file paths instead of raw file payload objects - rewrite those local file path arguments client-side into `ProvidedFilePayload`-shaped objects before the normal MCP tool call ## Details - applies to scalar and array file params declared in `openai/fileParams` - Codex uploads local files directly to the backend and uses the uploaded file metadata to build the MCP tool arguments locally - this PR is input-only ## Verification - `just fmt` - `cargo test -p codex-core mcp_tool_call -- --nocapture` --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
1 parent
25a0f6784d
commit
244b15c95d
15 files changed
+1313
-35
No files matched your search
@@ -56,6 +56,7 @@ mod original_image_detail;
|
||||
pub use codex_mcp::MCP_SANDBOX_STATE_CAPABILITY;
|
||||
pub use codex_mcp::MCP_SANDBOX_STATE_METHOD;
|
||||
pub use codex_mcp::SandboxState;
|
||||
mod mcp_openai_file;
|
||||
mod mcp_tool_call;
|
||||
mod memories;
|
||||
pub(crate) mod mention_syntax;
|
||||
|
||||
@@ -0,0 +1,473 @@
|
||||
//! Bridges Apps SDK-style `openai/fileParams` metadata into Codex's MCP flow.
|
||||
//!
|
||||
//! Strategy:
|
||||
//! - Inspect `_meta["openai/fileParams"]` to discover which tool arguments are
|
||||
//! file inputs.
|
||||
//! - At tool execution time, upload those local files to OpenAI file storage
|
||||
//! and rewrite only the declared arguments into the provided-file payload
|
||||
//! shape expected by the downstream Apps tool.
|
||||
//!
|
||||
//! Model-visible schema masking is owned by `codex-mcp` alongside MCP tool
|
||||
//! inventory, so this module only handles the execution-time argument rewrite.
|
||||
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use codex_api::CoreAuthProvider;
|
||||
use codex_api::upload_local_file;
|
||||
use codex_login::CodexAuth;
|
||||
use serde_json::Value as JsonValue;
|
||||
|
||||
pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
arguments_value: Option<JsonValue>,
|
||||
openai_file_input_params: Option<&[String]>,
|
||||
) -> Result<Option<JsonValue>, String> {
|
||||
let Some(openai_file_input_params) = openai_file_input_params else {
|
||||
return Ok(arguments_value);
|
||||
};
|
||||
|
||||
let Some(arguments_value) = arguments_value else {
|
||||
return Ok(None);
|
||||
};
|
||||
let Some(arguments) = arguments_value.as_object() else {
|
||||
return Ok(Some(arguments_value));
|
||||
};
|
||||
let auth = sess.services.auth_manager.auth().await;
|
||||
let mut rewritten_arguments = arguments.clone();
|
||||
|
||||
for field_name in openai_file_input_params {
|
||||
let Some(value) = arguments.get(field_name) else {
|
||||
continue;
|
||||
};
|
||||
let Some(uploaded_value) =
|
||||
rewrite_argument_value_for_openai_files(turn_context, auth.as_ref(), field_name, value)
|
||||
.await?
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
rewritten_arguments.insert(field_name.clone(), uploaded_value);
|
||||
}
|
||||
|
||||
if rewritten_arguments == *arguments {
|
||||
return Ok(Some(arguments_value));
|
||||
}
|
||||
|
||||
Ok(Some(JsonValue::Object(rewritten_arguments)))
|
||||
}
|
||||
|
||||
async fn rewrite_argument_value_for_openai_files(
|
||||
turn_context: &TurnContext,
|
||||
auth: Option<&CodexAuth>,
|
||||
field_name: &str,
|
||||
value: &JsonValue,
|
||||
) -> Result<Option<JsonValue>, String> {
|
||||
match value {
|
||||
JsonValue::String(path_or_file_ref) => {
|
||||
let rewritten = build_uploaded_local_argument_value(
|
||||
turn_context,
|
||||
auth,
|
||||
field_name,
|
||||
/*index*/ None,
|
||||
path_or_file_ref,
|
||||
)
|
||||
.await?;
|
||||
Ok(Some(rewritten))
|
||||
}
|
||||
JsonValue::Array(values) => {
|
||||
let mut rewritten_values = Vec::with_capacity(values.len());
|
||||
for (index, item) in values.iter().enumerate() {
|
||||
let Some(path_or_file_ref) = item.as_str() else {
|
||||
return Ok(None);
|
||||
};
|
||||
let rewritten = build_uploaded_local_argument_value(
|
||||
turn_context,
|
||||
auth,
|
||||
field_name,
|
||||
Some(index),
|
||||
path_or_file_ref,
|
||||
)
|
||||
.await?;
|
||||
rewritten_values.push(rewritten);
|
||||
}
|
||||
Ok(Some(JsonValue::Array(rewritten_values)))
|
||||
}
|
||||
_ => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
async fn build_uploaded_local_argument_value(
|
||||
turn_context: &TurnContext,
|
||||
auth: Option<&CodexAuth>,
|
||||
field_name: &str,
|
||||
index: Option<usize>,
|
||||
file_path: &str,
|
||||
) -> Result<JsonValue, String> {
|
||||
let resolved_path = turn_context.resolve_path(Some(file_path.to_string()));
|
||||
let Some(auth) = auth else {
|
||||
return Err(
|
||||
"ChatGPT auth is required to upload local files for Codex Apps tools".to_string(),
|
||||
);
|
||||
};
|
||||
let token_data = auth
|
||||
.get_token_data()
|
||||
.map_err(|error| format!("failed to read ChatGPT auth for file upload: {error}"))?;
|
||||
let upload_auth = CoreAuthProvider {
|
||||
token: Some(token_data.access_token),
|
||||
account_id: token_data.account_id,
|
||||
};
|
||||
let uploaded = upload_local_file(
|
||||
turn_context.config.chatgpt_base_url.trim_end_matches('/'),
|
||||
&upload_auth,
|
||||
&resolved_path,
|
||||
)
|
||||
.await
|
||||
.map_err(|error| match index {
|
||||
Some(index) => {
|
||||
format!("failed to upload `{file_path}` for `{field_name}[{index}]`: {error}")
|
||||
}
|
||||
None => format!("failed to upload `{file_path}` for `{field_name}`: {error}"),
|
||||
})?;
|
||||
Ok(serde_json::json!({
|
||||
"download_url": uploaded.download_url,
|
||||
"file_id": uploaded.file_id,
|
||||
"mime_type": uploaded.mime_type,
|
||||
"file_name": uploaded.file_name,
|
||||
"uri": uploaded.uri,
|
||||
"file_size_bytes": uploaded.file_size_bytes,
|
||||
}))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::codex::make_session_and_context;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn openai_file_argument_rewrite_requires_declared_file_params() {
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let arguments = Some(serde_json::json!({
|
||||
"file": "/tmp/codex-smoke-file.txt"
|
||||
}));
|
||||
|
||||
let rewritten = rewrite_mcp_tool_arguments_for_openai_files(
|
||||
&session,
|
||||
&Arc::new(turn_context),
|
||||
arguments.clone(),
|
||||
/*openai_file_input_params*/ None,
|
||||
)
|
||||
.await
|
||||
.expect("rewrite should succeed");
|
||||
|
||||
assert_eq!(rewritten, arguments);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_uploaded_local_argument_value_uploads_local_file_path() {
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::body_json;
|
||||
use wiremock::matchers::header;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files"))
|
||||
.and(header("chatgpt-account-id", "account_id"))
|
||||
.and(body_json(serde_json::json!({
|
||||
"file_name": "file_report.csv",
|
||||
"file_size": 5,
|
||||
"use_case": "codex",
|
||||
})))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"file_id": "file_123",
|
||||
"upload_url": format!("{}/upload/file_123", server.uri()),
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("PUT"))
|
||||
.and(path("/upload/file_123"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files/file_123/uploaded"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"status": "success",
|
||||
"download_url": format!("{}/download/file_123", server.uri()),
|
||||
"file_name": "file_report.csv",
|
||||
"mime_type": "text/csv",
|
||||
"file_size_bytes": 5,
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let (_, mut turn_context) = make_session_and_context().await;
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
let dir = tempdir().expect("temp dir");
|
||||
let local_path = dir.path().join("file_report.csv");
|
||||
tokio::fs::write(&local_path, b"hello")
|
||||
.await
|
||||
.expect("write local file");
|
||||
turn_context.cwd = AbsolutePathBuf::try_from(dir.path()).expect("absolute path");
|
||||
|
||||
let mut config = (*turn_context.config).clone();
|
||||
config.chatgpt_base_url = format!("{}/backend-api", server.uri());
|
||||
turn_context.config = Arc::new(config);
|
||||
|
||||
let rewritten = build_uploaded_local_argument_value(
|
||||
&turn_context,
|
||||
Some(&auth),
|
||||
"file",
|
||||
/*index*/ None,
|
||||
"file_report.csv",
|
||||
)
|
||||
.await
|
||||
.expect("rewrite should upload the local file");
|
||||
|
||||
assert_eq!(
|
||||
rewritten,
|
||||
serde_json::json!({
|
||||
"download_url": format!("{}/download/file_123", server.uri()),
|
||||
"file_id": "file_123",
|
||||
"mime_type": "text/csv",
|
||||
"file_name": "file_report.csv",
|
||||
"uri": "sediment://file_123",
|
||||
"file_size_bytes": 5,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rewrite_argument_value_for_openai_files_rewrites_scalar_path() {
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::body_json;
|
||||
use wiremock::matchers::header;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files"))
|
||||
.and(header("chatgpt-account-id", "account_id"))
|
||||
.and(body_json(serde_json::json!({
|
||||
"file_name": "file_report.csv",
|
||||
"file_size": 5,
|
||||
"use_case": "codex",
|
||||
})))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"file_id": "file_123",
|
||||
"upload_url": format!("{}/upload/file_123", server.uri()),
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("PUT"))
|
||||
.and(path("/upload/file_123"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files/file_123/uploaded"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"status": "success",
|
||||
"download_url": format!("{}/download/file_123", server.uri()),
|
||||
"file_name": "file_report.csv",
|
||||
"mime_type": "text/csv",
|
||||
"file_size_bytes": 5,
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let (_, mut turn_context) = make_session_and_context().await;
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
let dir = tempdir().expect("temp dir");
|
||||
let local_path = dir.path().join("file_report.csv");
|
||||
tokio::fs::write(&local_path, b"hello")
|
||||
.await
|
||||
.expect("write local file");
|
||||
turn_context.cwd = AbsolutePathBuf::try_from(dir.path()).expect("absolute path");
|
||||
|
||||
let mut config = (*turn_context.config).clone();
|
||||
config.chatgpt_base_url = format!("{}/backend-api", server.uri());
|
||||
turn_context.config = Arc::new(config);
|
||||
let rewritten = rewrite_argument_value_for_openai_files(
|
||||
&turn_context,
|
||||
Some(&auth),
|
||||
"file",
|
||||
&serde_json::json!("file_report.csv"),
|
||||
)
|
||||
.await
|
||||
.expect("rewrite should succeed");
|
||||
|
||||
assert_eq!(
|
||||
rewritten,
|
||||
Some(serde_json::json!({
|
||||
"download_url": format!("{}/download/file_123", server.uri()),
|
||||
"file_id": "file_123",
|
||||
"mime_type": "text/csv",
|
||||
"file_name": "file_report.csv",
|
||||
"uri": "sediment://file_123",
|
||||
"file_size_bytes": 5,
|
||||
}))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rewrite_argument_value_for_openai_files_rewrites_array_paths() {
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::body_json;
|
||||
use wiremock::matchers::header;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files"))
|
||||
.and(header("chatgpt-account-id", "account_id"))
|
||||
.and(body_json(serde_json::json!({
|
||||
"file_name": "one.csv",
|
||||
"file_size": 3,
|
||||
"use_case": "codex",
|
||||
})))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"file_id": "file_1",
|
||||
"upload_url": format!("{}/upload/file_1", server.uri()),
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files"))
|
||||
.and(header("chatgpt-account-id", "account_id"))
|
||||
.and(body_json(serde_json::json!({
|
||||
"file_name": "two.csv",
|
||||
"file_size": 3,
|
||||
"use_case": "codex",
|
||||
})))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"file_id": "file_2",
|
||||
"upload_url": format!("{}/upload/file_2", server.uri()),
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("PUT"))
|
||||
.and(path("/upload/file_1"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("PUT"))
|
||||
.and(path("/upload/file_2"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files/file_1/uploaded"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"status": "success",
|
||||
"download_url": format!("{}/download/file_1", server.uri()),
|
||||
"file_name": "one.csv",
|
||||
"mime_type": "text/csv",
|
||||
"file_size_bytes": 3,
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/files/file_2/uploaded"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
|
||||
"status": "success",
|
||||
"download_url": format!("{}/download/file_2", server.uri()),
|
||||
"file_name": "two.csv",
|
||||
"mime_type": "text/csv",
|
||||
"file_size_bytes": 3,
|
||||
})))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let (_, mut turn_context) = make_session_and_context().await;
|
||||
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
|
||||
let dir = tempdir().expect("temp dir");
|
||||
tokio::fs::write(dir.path().join("one.csv"), b"one")
|
||||
.await
|
||||
.expect("write first local file");
|
||||
tokio::fs::write(dir.path().join("two.csv"), b"two")
|
||||
.await
|
||||
.expect("write second local file");
|
||||
turn_context.cwd = AbsolutePathBuf::try_from(dir.path()).expect("absolute path");
|
||||
|
||||
let mut config = (*turn_context.config).clone();
|
||||
config.chatgpt_base_url = format!("{}/backend-api", server.uri());
|
||||
turn_context.config = Arc::new(config);
|
||||
let rewritten = rewrite_argument_value_for_openai_files(
|
||||
&turn_context,
|
||||
Some(&auth),
|
||||
"files",
|
||||
&serde_json::json!(["one.csv", "two.csv"]),
|
||||
)
|
||||
.await
|
||||
.expect("rewrite should succeed");
|
||||
|
||||
assert_eq!(
|
||||
rewritten,
|
||||
Some(serde_json::json!([
|
||||
{
|
||||
"download_url": format!("{}/download/file_1", server.uri()),
|
||||
"file_id": "file_1",
|
||||
"mime_type": "text/csv",
|
||||
"file_name": "one.csv",
|
||||
"uri": "sediment://file_1",
|
||||
"file_size_bytes": 3,
|
||||
},
|
||||
{
|
||||
"download_url": format!("{}/download/file_2", server.uri()),
|
||||
"file_id": "file_2",
|
||||
"mime_type": "text/csv",
|
||||
"file_name": "two.csv",
|
||||
"uri": "sediment://file_2",
|
||||
"file_size_bytes": 3,
|
||||
}
|
||||
]))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rewrite_mcp_tool_arguments_for_openai_files_surfaces_upload_failures() {
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
session.services.auth_manager = crate::test_support::auth_manager_from_auth(
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
);
|
||||
let error = rewrite_mcp_tool_arguments_for_openai_files(
|
||||
&session,
|
||||
&turn_context,
|
||||
Some(serde_json::json!({
|
||||
"file": "/definitely/missing/file.csv",
|
||||
})),
|
||||
Some(&["file".to_string()]),
|
||||
)
|
||||
.await
|
||||
.expect_err("missing file should fail");
|
||||
|
||||
assert!(error.contains("failed to upload"));
|
||||
assert!(error.contains("file"));
|
||||
}
|
||||
}
|
||||
@@ -26,6 +26,7 @@ use crate::guardian::guardian_approval_request_to_json;
|
||||
use crate::guardian::guardian_rejection_message;
|
||||
use crate::guardian::review_approval_request;
|
||||
use crate::guardian::routes_approval_to_guardian;
|
||||
use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files;
|
||||
use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam;
|
||||
use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template;
|
||||
use codex_analytics::AppInvocation;
|
||||
@@ -34,6 +35,7 @@ use codex_analytics::build_track_events_context;
|
||||
use codex_config::types::AppToolApproval;
|
||||
use codex_features::Feature;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::declared_openai_file_input_param_names;
|
||||
use codex_mcp::mcp_permission_prompt_is_auto_approved;
|
||||
use codex_otel::sanitize_metric_tag_value;
|
||||
use codex_protocol::mcp::CallToolResult;
|
||||
@@ -178,14 +180,16 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
|
||||
let start = Instant::now();
|
||||
let result = async {
|
||||
sess.call_tool(
|
||||
execute_mcp_tool_call(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&server,
|
||||
&tool_name,
|
||||
arguments_value.clone(),
|
||||
metadata.as_ref(),
|
||||
request_meta.clone(),
|
||||
)
|
||||
.await
|
||||
.map_err(|e| format!("tool call error: {e:?}"))
|
||||
}
|
||||
.instrument(mcp_tool_call_span(
|
||||
sess.as_ref(),
|
||||
@@ -200,13 +204,6 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
},
|
||||
))
|
||||
.await;
|
||||
let result = sanitize_mcp_tool_result_for_model(
|
||||
turn_context
|
||||
.model_info
|
||||
.input_modalities
|
||||
.contains(&InputModality::Image),
|
||||
result,
|
||||
);
|
||||
if let Err(error) = &result {
|
||||
tracing::warn!("MCP tool call error: {error:?}");
|
||||
}
|
||||
@@ -294,11 +291,17 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
maybe_mark_thread_memory_mode_polluted(sess.as_ref(), turn_context.as_ref()).await;
|
||||
|
||||
let start = Instant::now();
|
||||
// Perform the tool call.
|
||||
let result = async {
|
||||
sess.call_tool(&server, &tool_name, arguments_value.clone(), request_meta)
|
||||
.await
|
||||
.map_err(|e| format!("tool call error: {e:?}"))
|
||||
execute_mcp_tool_call(
|
||||
sess.as_ref(),
|
||||
turn_context.as_ref(),
|
||||
&server,
|
||||
&tool_name,
|
||||
arguments_value.clone(),
|
||||
metadata.as_ref(),
|
||||
request_meta,
|
||||
)
|
||||
.await
|
||||
}
|
||||
.instrument(mcp_tool_call_span(
|
||||
sess.as_ref(),
|
||||
@@ -313,13 +316,6 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
},
|
||||
))
|
||||
.await;
|
||||
let result = sanitize_mcp_tool_result_for_model(
|
||||
turn_context
|
||||
.model_info
|
||||
.input_modalities
|
||||
.contains(&InputModality::Image),
|
||||
result,
|
||||
);
|
||||
if let Err(error) = &result {
|
||||
tracing::warn!("MCP tool call error: {error:?}");
|
||||
}
|
||||
@@ -453,6 +449,35 @@ fn record_server_fields(span: &Span, url: Option<&str>) {
|
||||
}
|
||||
}
|
||||
|
||||
async fn execute_mcp_tool_call(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
arguments_value: Option<serde_json::Value>,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
request_meta: Option<serde_json::Value>,
|
||||
) -> Result<CallToolResult, String> {
|
||||
let rewritten_arguments = rewrite_mcp_tool_arguments_for_openai_files(
|
||||
sess,
|
||||
turn_context,
|
||||
arguments_value,
|
||||
metadata.and_then(|metadata| metadata.openai_file_input_params.as_deref()),
|
||||
)
|
||||
.await?;
|
||||
let result = sess
|
||||
.call_tool(server, tool_name, rewritten_arguments, request_meta)
|
||||
.await
|
||||
.map_err(|e| format!("tool call error: {e:?}"))?;
|
||||
sanitize_mcp_tool_result_for_model(
|
||||
turn_context
|
||||
.model_info
|
||||
.input_modalities
|
||||
.contains(&InputModality::Image),
|
||||
Ok(result),
|
||||
)
|
||||
}
|
||||
|
||||
async fn maybe_mark_thread_memory_mode_polluted(sess: &Session, turn_context: &TurnContext) {
|
||||
if !turn_context
|
||||
.config
|
||||
@@ -566,6 +591,7 @@ pub(crate) struct McpToolApprovalMetadata {
|
||||
tool_title: Option<String>,
|
||||
tool_description: Option<String>,
|
||||
codex_apps_meta: Option<serde_json::Map<String, serde_json::Value>>,
|
||||
openai_file_input_params: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
const MCP_TOOL_CODEX_APPS_META_KEY: &str = "_codex_apps";
|
||||
@@ -983,7 +1009,6 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
.await
|
||||
.list_all_tools()
|
||||
.await;
|
||||
|
||||
let tool_info = tools
|
||||
.into_values()
|
||||
.find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
|
||||
@@ -1025,6 +1050,10 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
.and_then(|meta| meta.get(MCP_TOOL_CODEX_APPS_META_KEY))
|
||||
.and_then(serde_json::Value::as_object)
|
||||
.cloned(),
|
||||
openai_file_input_params: Some(declared_openai_file_input_param_names(
|
||||
tool_info.tool.meta.as_deref(),
|
||||
))
|
||||
.filter(|params| !params.is_empty()),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -59,6 +59,7 @@ fn approval_metadata(
|
||||
tool_title: tool_title.map(str::to_string),
|
||||
tool_description: tool_description.map(str::to_string),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -597,6 +598,7 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
|
||||
.cloned()
|
||||
.expect("_codex_apps metadata should be an object"),
|
||||
),
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
@@ -744,6 +746,7 @@ fn guardian_mcp_review_request_includes_annotations_when_present() {
|
||||
tool_title: None,
|
||||
tool_description: None,
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let request = build_guardian_mcp_tool_review_request("call-1", &invocation, Some(&metadata));
|
||||
@@ -1254,6 +1257,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
|
||||
tool_title: Some("Read Only Tool".to_string()),
|
||||
tool_description: None,
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
@@ -1321,6 +1325,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
|
||||
tool_title: Some("Read Only Tool".to_string()),
|
||||
tool_description: None,
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
@@ -1391,6 +1396,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Reads calendar data.".to_string()),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
@@ -1441,6 +1447,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
|
||||
tool_title: Some("Read Only Tool".to_string()),
|
||||
tool_description: None,
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let mut approval_task = {
|
||||
@@ -1517,6 +1524,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_for_model() {
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
@@ -1586,6 +1594,7 @@ async fn custom_approve_mode_blocks_when_arc_returns_interrupt_for_model() {
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
@@ -1655,6 +1664,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_without_annotations() {
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
@@ -1732,6 +1742,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
for approval_mode in [
|
||||
@@ -1833,6 +1844,7 @@ async fn approve_mode_routes_arc_ask_user_to_guardian_when_guardian_reviewer_is_
|
||||
tool_title: Some("Dangerous Tool".to_string()),
|
||||
tool_description: Some("Performs a risky action.".to_string()),
|
||||
codex_apps_meta: None,
|
||||
openai_file_input_params: None,
|
||||
};
|
||||
|
||||
let decision = maybe_request_mcp_tool_approval(
|
||||
|
||||
Reference in new issue
Block a user