This commit is contained in:
Liang-Ting Jiang
2026-04-15 20:17:30 -07:00
parent 6717d23c57
commit 27db0443d2
12 changed files with 228 additions and 1671 deletions
+1 -6
View File
@@ -7052,12 +7052,7 @@ async fn run_sampling_request(
let _code_mode_worker = sess
.services
.code_mode_service
.start_turn_worker(
&sess,
&turn_context,
Arc::clone(&router),
Arc::clone(&turn_diff_tracker),
)
.start_turn_worker(&sess, &turn_context, Arc::clone(&turn_diff_tracker))
.await;
let mut retries = 0;
let mut initial_input = Some(input);
-1
View File
@@ -44,7 +44,6 @@ mod installation_id;
pub(crate) mod instructions;
pub(crate) mod landlock;
pub use landlock::spawn_command_under_linux_sandbox;
mod library_mcp;
pub(crate) mod mcp;
mod mcp_skill_dependencies;
mod mcp_tool_approval_templates;
File diff suppressed because it is too large Load Diff
+108 -3
View File
@@ -13,6 +13,7 @@
use crate::codex::Session;
use crate::codex::TurnContext;
use codex_api::CoreAuthProvider;
use codex_api::OpenAiFileUploadOptions;
use codex_api::upload_local_file;
use codex_login::CodexAuth;
use serde_json::Value as JsonValue;
@@ -22,6 +23,7 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
turn_context: &TurnContext,
arguments_value: Option<JsonValue>,
openai_file_input_params: Option<&[String]>,
upload_options: Option<&OpenAiFileUploadOptions>,
) -> Result<Option<JsonValue>, String> {
let Some(openai_file_input_params) = openai_file_input_params else {
return Ok(arguments_value);
@@ -40,9 +42,14 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files(
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?
let Some(uploaded_value) = rewrite_argument_value_for_openai_files(
turn_context,
auth.as_ref(),
field_name,
value,
upload_options,
)
.await?
else {
continue;
};
@@ -61,6 +68,7 @@ async fn rewrite_argument_value_for_openai_files(
auth: Option<&CodexAuth>,
field_name: &str,
value: &JsonValue,
upload_options: Option<&OpenAiFileUploadOptions>,
) -> Result<Option<JsonValue>, String> {
match value {
JsonValue::String(path_or_file_ref) => {
@@ -70,6 +78,7 @@ async fn rewrite_argument_value_for_openai_files(
field_name,
/*index*/ None,
path_or_file_ref,
upload_options,
)
.await?;
Ok(Some(rewritten))
@@ -86,6 +95,7 @@ async fn rewrite_argument_value_for_openai_files(
field_name,
Some(index),
path_or_file_ref,
upload_options,
)
.await?;
rewritten_values.push(rewritten);
@@ -102,6 +112,7 @@ async fn build_uploaded_local_argument_value(
field_name: &str,
index: Option<usize>,
file_path: &str,
upload_options: Option<&OpenAiFileUploadOptions>,
) -> Result<JsonValue, String> {
let resolved_path = turn_context.resolve_path(Some(file_path.to_string()));
let Some(auth) = auth else {
@@ -116,10 +127,12 @@ async fn build_uploaded_local_argument_value(
token: Some(token_data.access_token),
account_id: token_data.account_id,
};
let default_upload_options = OpenAiFileUploadOptions::default();
let uploaded = upload_local_file(
turn_context.config.chatgpt_base_url.trim_end_matches('/'),
&upload_auth,
&resolved_path,
upload_options.unwrap_or(&default_upload_options),
)
.await
.map_err(|error| match index {
@@ -159,6 +172,7 @@ mod tests {
&Arc::new(turn_context),
arguments.clone(),
/*openai_file_input_params*/ None,
/*upload_options*/ None,
)
.await
.expect("rewrite should succeed");
@@ -230,6 +244,7 @@ mod tests {
"file",
/*index*/ None,
"file_report.csv",
/*upload_options*/ None,
)
.await
.expect("rewrite should upload the local file");
@@ -247,6 +262,93 @@ mod tests {
);
}
#[tokio::test]
async fn build_uploaded_local_argument_value_honors_upload_options() {
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": "library.txt",
"file_size": 7,
"use_case": "codex",
"store_in_library": true,
})))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"file_id": "file_library",
"upload_url": format!("{}/upload/file_library", server.uri()),
})))
.expect(1)
.mount(&server)
.await;
Mock::given(method("PUT"))
.and(path("/upload/file_library"))
.respond_with(ResponseTemplate::new(200))
.expect(1)
.mount(&server)
.await;
Mock::given(method("POST"))
.and(path("/backend-api/files/file_library/uploaded"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"status": "success",
"download_url": format!("{}/download/file_library", server.uri()),
"file_name": "library.txt",
"mime_type": "text/plain",
"file_size_bytes": 7,
})))
.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("library.txt");
tokio::fs::write(&local_path, b"library")
.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 upload_options = OpenAiFileUploadOptions {
store_in_library: true,
..OpenAiFileUploadOptions::default()
};
let rewritten = build_uploaded_local_argument_value(
&turn_context,
Some(&auth),
"file",
/*index*/ None,
"library.txt",
Some(&upload_options),
)
.await
.expect("rewrite should upload the local file");
assert_eq!(
rewritten,
serde_json::json!({
"download_url": format!("{}/download/file_library", server.uri()),
"file_id": "file_library",
"mime_type": "text/plain",
"file_name": "library.txt",
"uri": "sediment://file_library",
"file_size_bytes": 7,
})
);
}
#[tokio::test]
async fn rewrite_argument_value_for_openai_files_rewrites_scalar_path() {
use wiremock::Mock;
@@ -309,6 +411,7 @@ mod tests {
Some(&auth),
"file",
&serde_json::json!("file_report.csv"),
/*upload_options*/ None,
)
.await
.expect("rewrite should succeed");
@@ -423,6 +526,7 @@ mod tests {
Some(&auth),
"files",
&serde_json::json!(["one.csv", "two.csv"]),
/*upload_options*/ None,
)
.await
.expect("rewrite should succeed");
@@ -463,6 +567,7 @@ mod tests {
"file": "/definitely/missing/file.csv",
})),
Some(&["file".to_string()]),
/*upload_options*/ None,
)
.await
.expect_err("missing file should fail");
+38 -12
View File
@@ -27,19 +27,18 @@ use crate::guardian::guardian_timeout_message;
use crate::guardian::new_guardian_review_id;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::library_mcp::handle_library_mcp_tool_call;
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;
use codex_analytics::InvocationType;
use codex_analytics::build_track_events_context;
use codex_api::OpenAiFileUploadOptions;
use codex_config::types::AppToolApproval;
use codex_features::Feature;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_mcp::SandboxState;
use codex_mcp::declared_openai_file_input_param_names;
use codex_mcp::is_codex_apps_library_tool;
use codex_mcp::mcp_permission_prompt_is_auto_approved;
use codex_otel::sanitize_metric_tag_value;
use codex_protocol::mcp::CallToolResult;
@@ -462,21 +461,12 @@ async fn execute_mcp_tool_call(
metadata: Option<&McpToolApprovalMetadata>,
request_meta: Option<serde_json::Value>,
) -> Result<CallToolResult, String> {
if server == CODEX_APPS_MCP_SERVER_NAME && is_codex_apps_library_tool(tool_name) {
return sanitize_mcp_tool_result_for_model(
turn_context
.model_info
.input_modalities
.contains(&InputModality::Image),
handle_library_mcp_tool_call(sess, turn_context, tool_name, arguments_value).await,
);
}
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()),
metadata.and_then(|metadata| metadata.openai_file_upload_options.as_ref()),
)
.await?;
let request_meta =
@@ -656,9 +646,42 @@ pub(crate) struct McpToolApprovalMetadata {
tool_description: Option<String>,
codex_apps_meta: Option<serde_json::Map<String, serde_json::Value>>,
openai_file_input_params: Option<Vec<String>>,
openai_file_upload_options: Option<OpenAiFileUploadOptions>,
}
const MCP_TOOL_CODEX_APPS_META_KEY: &str = "_codex_apps";
const MCP_TOOL_OPENAI_FILE_UPLOAD_CONFIG_KEY: &str = "openai/fileUploadConfig";
#[derive(Debug, Clone, Deserialize)]
struct RawOpenAiFileUploadConfig {
#[serde(default)]
use_case: Option<String>,
#[serde(default)]
store_in_library: bool,
#[serde(default)]
upload_source: Option<String>,
}
fn parse_openai_file_upload_options(
meta: Option<&serde_json::Map<String, serde_json::Value>>,
) -> Option<OpenAiFileUploadOptions> {
let raw = meta?
.get(MCP_TOOL_OPENAI_FILE_UPLOAD_CONFIG_KEY)
.cloned()
.and_then(|value| serde_json::from_value::<RawOpenAiFileUploadConfig>(value).ok())?;
if raw.use_case.is_none() && !raw.store_in_library && raw.upload_source.is_none() {
return None;
}
let mut options = OpenAiFileUploadOptions::default();
if let Some(use_case) = raw.use_case {
options.use_case = use_case;
}
options.store_in_library = raw.store_in_library;
options.upload_source = raw.upload_source;
Some(options)
}
fn custom_mcp_tool_approval_mode(
turn_context: &TurnContext,
@@ -1123,6 +1146,9 @@ pub(crate) async fn lookup_mcp_tool_metadata(
tool_info.tool.meta.as_deref(),
))
.filter(|params| !params.is_empty()),
openai_file_upload_options: parse_openai_file_upload_options(
tool_info.tool.meta.as_deref(),
),
})
}
+32
View File
@@ -61,6 +61,7 @@ fn approval_metadata(
tool_description: tool_description.map(str::to_string),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
}
}
@@ -74,6 +75,26 @@ fn prompt_options(
}
}
#[test]
fn parse_openai_file_upload_options_reads_store_in_library_config() {
let meta = serde_json::json!({
"openai/fileUploadConfig": {
"store_in_library": true,
}
});
let parsed = parse_openai_file_upload_options(meta.as_object());
assert_eq!(
parsed,
Some(codex_api::OpenAiFileUploadOptions {
use_case: "codex".to_string(),
store_in_library: true,
upload_source: None,
})
);
}
#[test]
fn approval_required_when_read_only_false_and_destructive() {
let annotations = annotations(Some(false), Some(true), /*open_world*/ None);
@@ -600,6 +621,7 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
.expect("_codex_apps metadata should be an object"),
),
openai_file_input_params: None,
openai_file_upload_options: None,
};
assert_eq!(
@@ -748,6 +770,7 @@ fn guardian_mcp_review_request_includes_annotations_when_present() {
tool_description: None,
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let request = build_guardian_mcp_tool_review_request("call-1", &invocation, Some(&metadata));
@@ -1274,6 +1297,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
tool_description: None,
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
@@ -1342,6 +1366,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
tool_description: None,
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
@@ -1413,6 +1438,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
tool_description: Some("Reads calendar data.".to_string()),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
@@ -1464,6 +1490,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
tool_description: None,
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let mut approval_task = {
@@ -1541,6 +1568,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_for_model() {
tool_description: Some("Performs a risky action.".to_string()),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
@@ -1611,6 +1639,7 @@ async fn custom_approve_mode_blocks_when_arc_returns_interrupt_for_model() {
tool_description: Some("Performs a risky action.".to_string()),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
@@ -1681,6 +1710,7 @@ async fn approve_mode_blocks_when_arc_returns_interrupt_without_annotations() {
tool_description: Some("Performs a risky action.".to_string()),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
@@ -1759,6 +1789,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
tool_description: Some("Performs a risky action.".to_string()),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
for approval_mode in [
@@ -1861,6 +1892,7 @@ async fn approve_mode_routes_arc_ask_user_to_guardian_when_guardian_reviewer_is_
tool_description: Some("Performs a risky action.".to_string()),
codex_apps_meta: None,
openai_file_input_params: None,
openai_file_upload_options: None,
};
let decision = maybe_request_mcp_tool_approval(
+7 -3
View File
@@ -91,7 +91,6 @@ impl CodeModeService {
&self,
session: &Arc<Session>,
turn: &Arc<TurnContext>,
router: Arc<ToolRouter>,
tracker: SharedTurnDiffTracker,
) -> Option<codex_code_mode::CodeModeTurnWorker> {
if !turn.features.enabled(Feature::CodeMode) {
@@ -102,8 +101,13 @@ impl CodeModeService {
session: Arc::clone(session),
turn: Arc::clone(turn),
};
let tool_runtime =
ToolCallRuntime::new(router, Arc::clone(session), Arc::clone(turn), tracker);
let nested_router = Arc::new(build_nested_router(&exec).await);
let tool_runtime = ToolCallRuntime::new(
nested_router,
Arc::clone(session),
Arc::clone(turn),
tracker,
);
let host = Arc::new(CoreTurnHost { exec, tool_runtime });
Some(self.inner.start_turn_worker(host))
}