From 17d94bd1e32f8f75676a0927ca297031f4b7d7ae Mon Sep 17 00:00:00 2001 From: evawong-oai Date: Wed, 15 Apr 2026 14:43:30 -0700 Subject: [PATCH 1/4] [docs] Revert extra changes from PR 17848 (#18003) ## Summary 1. Revert https://github.com/openai/codex/pull/17848 so the Bazel and `BUILD` file changes leave `main`. 2. Prepare for a narrower follow up that restores only `SECURITY.md`. ## Validation 1. Reviewed the revert diff against `main`. 2. Ran a clean diff check before push. --- .github/workflows/bazel.yml | 2 +- SECURITY.md | 4 --- codex-rs/app-server/BUILD.bazel | 3 -- codex-rs/core/BUILD.bazel | 55 +-------------------------------- codex-rs/tui/BUILD.bazel | 21 +------------ defs.bzl | 54 +++----------------------------- 6 files changed, 8 insertions(+), 131 deletions(-) diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index c5a4c0c1c7..5d30916f9f 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -94,7 +94,7 @@ jobs: "${bazel_wrapper_args[@]}" \ -- \ test \ - --test_tag_filters=-argument-comment-lint,-flaky \ + --test_tag_filters=-argument-comment-lint \ --test_verbose_timeout_warnings \ --build_metadata=COMMIT_SHA=${GITHUB_SHA} \ -- \ diff --git a/SECURITY.md b/SECURITY.md index 29a9b903df..d6dd568910 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -11,7 +11,3 @@ Our security program is managed through Bugcrowd, and we ask that any validated ## Vulnerability Disclosure Program Our Vulnerability Program Guidelines are defined on our [Bugcrowd program page](https://bugcrowd.com/engagements/openai). - -## How to operate CODEX safely - -For details on Codex security boundaries, including sandboxing, approvals, and network controls, see [Agent approvals & security](https://developers.openai.com/codex/agent-approvals-security). diff --git a/codex-rs/app-server/BUILD.bazel b/codex-rs/app-server/BUILD.bazel index f32f811627..872f533be1 100644 --- a/codex-rs/app-server/BUILD.bazel +++ b/codex-rs/app-server/BUILD.bazel @@ -3,9 +3,6 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "app-server", crate_name = "codex_app_server", - integration_test_tags_extra_by_stem = { - "all": ["flaky"], - }, integration_test_timeout = "long", test_tags = ["no-sandbox"], ) diff --git a/codex-rs/core/BUILD.bazel b/codex-rs/core/BUILD.bazel index be07ef7999..434dc1f6a4 100644 --- a/codex-rs/core/BUILD.bazel +++ b/codex-rs/core/BUILD.bazel @@ -1,4 +1,4 @@ -load("//:defs.bzl", "codex_rust_crate", "workspace_root_test") +load("//:defs.bzl", "codex_rust_crate") filegroup( name = "model_availability_nux_fixtures", @@ -31,15 +31,6 @@ codex_rust_crate( "//codex-rs/apply-patch:apply_patch_tool_instructions.md", "templates/realtime/backend_prompt.md", ], - integration_test_flaky_by_stem = { - "all": [ - "suite::abort_tasks::interrupt_tool_records_history_entries", - "suite::apply_patch_cli::apply_patch_cli_rejects_invalid_hunk_header::applypatchmodeloutput_function_expects", - ], - "responses_headers": [ - "responses_stream_includes_turn_metadata_header_for_git_workspace_e2e", - ], - }, integration_test_timeout = "long", test_data_extra = [ "config.schema.json", @@ -57,12 +48,6 @@ codex_rust_crate( "//:AGENTS.md", ], test_tags = ["no-sandbox"], - unit_test_args = [ - "--skip", - "guardian::tests::guardian_parallel_reviews_fork_from_last_committed_trunk_history", - "--skip", - "agent::control::tests::completion_watcher_notifies_parent_when_child_is_missing", - ], unit_test_timeout = "long", extra_binaries = [ "//codex-rs/linux-sandbox:codex-linux-sandbox", @@ -72,41 +57,3 @@ codex_rust_crate( "//codex-rs/cli:codex", ], ) - -workspace_root_test( - name = "core-guardian-parallel-reviews-flaky-test", - env = { - "INSTA_WORKSPACE_ROOT": ".", - "INSTA_SNAPSHOT_PATH": "src", - }, - test_bin = ":core-unit-tests-bin", - workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", - args = [ - "--exact", - "guardian::tests::guardian_parallel_reviews_fork_from_last_committed_trunk_history", - ], - tags = [ - "flaky", - "no-sandbox", - ], - timeout = "long", -) - -workspace_root_test( - name = "core-completion-watcher-missing-child-flaky-test", - env = { - "INSTA_WORKSPACE_ROOT": ".", - "INSTA_SNAPSHOT_PATH": "src", - }, - test_bin = ":core-unit-tests-bin", - workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", - args = [ - "--exact", - "agent::control::tests::completion_watcher_notifies_parent_when_child_is_missing", - ], - tags = [ - "flaky", - "no-sandbox", - ], - timeout = "long", -) diff --git a/codex-rs/tui/BUILD.bazel b/codex-rs/tui/BUILD.bazel index a8cf752f96..33cc133431 100644 --- a/codex-rs/tui/BUILD.bazel +++ b/codex-rs/tui/BUILD.bazel @@ -1,4 +1,4 @@ -load("//:defs.bzl", "MACOS_WEBRTC_RUSTC_LINK_FLAGS", "codex_rust_crate", "workspace_root_test") +load("//:defs.bzl", "MACOS_WEBRTC_RUSTC_LINK_FLAGS", "codex_rust_crate") codex_rust_crate( name = "tui", @@ -20,27 +20,8 @@ codex_rust_crate( "src/**/snapshots/**", ]) + ["//codex-rs/core:model_availability_nux_fixtures"], integration_compile_data_extra = ["src/test_backend.rs"], - unit_test_args = [ - "--skip", - "app::tests::update_memory_settings_updates_current_thread_memory_mode", - ], extra_binaries = [ "//codex-rs/cli:codex", ], rustc_flags_extra = MACOS_WEBRTC_RUSTC_LINK_FLAGS, ) - -workspace_root_test( - name = "tui-update-memory-settings-flaky-test", - env = { - "INSTA_WORKSPACE_ROOT": ".", - "INSTA_SNAPSHOT_PATH": "src", - }, - test_bin = ":tui-unit-tests-bin", - workspace_root_marker = "//codex-rs/utils/cargo-bin:repo_root.marker", - args = [ - "--exact", - "app::tests::update_memory_settings_updates_current_thread_memory_mode", - ], - tags = ["flaky"], -) diff --git a/defs.bzl b/defs.bzl index 7cd89ce63e..6972b65a65 100644 --- a/defs.bzl +++ b/defs.bzl @@ -137,13 +137,10 @@ def codex_rust_crate( rustc_env = {}, deps_extra = [], integration_compile_data_extra = [], - integration_test_tags_extra_by_stem = {}, - integration_test_flaky_by_stem = {}, integration_test_args = [], integration_test_timeout = None, test_data_extra = [], test_tags = [], - unit_test_args = [], unit_test_timeout = None, extra_binaries = []): """Defines a Rust crate with library, binaries, and tests wired for Bazel + Cargo parity. @@ -173,9 +170,6 @@ def codex_rust_crate( deps_extra: Extra normal deps beyond @crates resolution. Typically only needed when features add additional deps. integration_compile_data_extra: Extra compile_data for integration tests. - integration_test_flaky_by_stem: Mapping from `tests/*.rs` stem to exact - Rust test names that should be split into dedicated flaky targets - and skipped from the main integration test target. integration_test_args: Optional args for integration test binaries. integration_test_timeout: Optional Bazel timeout for integration test targets generated from `tests/*.rs`. @@ -276,8 +270,6 @@ def codex_rust_crate( unit_test_kwargs = {} if unit_test_timeout: unit_test_kwargs["timeout"] = unit_test_timeout - if unit_test_args: - unit_test_kwargs["args"] = unit_test_args workspace_root_test( name = name + "-unit-tests", @@ -313,9 +305,11 @@ def codex_rust_crate( binary = Label(binary_label).name cargo_env["CARGO_BIN_EXE_" + binary] = "$(rlocationpath %s)" % binary_label - integration_test_base_kwargs = {} + integration_test_kwargs = {} + if integration_test_args: + integration_test_kwargs["args"] = integration_test_args if integration_test_timeout: - integration_test_base_kwargs["timeout"] = integration_test_timeout + integration_test_kwargs["timeout"] = integration_test_timeout for test in native.glob(["tests/*.rs"], allow_empty = True): test_file_stem = test.removeprefix("tests/").removesuffix(".rs") @@ -323,18 +317,6 @@ def codex_rust_crate( test_name = name + "-" + test_file_stem.replace("/", "-") if not test_name.endswith("-test"): test_name += "-test" - extra_test_tags = integration_test_tags_extra_by_stem.get(test_file_stem, []) - flaky_tests = integration_test_flaky_by_stem.get(test_file_stem, []) - stem_integration_test_args = [] + integration_test_args - for flaky_test in flaky_tests: - stem_integration_test_args += [ - "--skip", - flaky_test, - ] - - integration_test_kwargs = dict(integration_test_base_kwargs) - if stem_integration_test_args: - integration_test_kwargs["args"] = stem_integration_test_args rust_test( name = test_name, @@ -356,32 +338,6 @@ def codex_rust_crate( # `INSTA_WORKSPACE_ROOT="codex-rs"` is tuned for unit tests that # execute from the repo root and can misplace integration snapshots. env = cargo_env, - tags = test_tags + extra_test_tags, + tags = test_tags, **integration_test_kwargs ) - - for flaky_test in flaky_tests: - flaky_test_suffix = flaky_test.replace("::", "-").replace("/", "-").replace("_", "-") - flaky_test_kwargs = dict(integration_test_base_kwargs) - flaky_test_kwargs["args"] = [ - "--exact", - flaky_test, - ] - - rust_test( - name = test_name.removesuffix("-test") + "-flaky-" + flaky_test_suffix + "-test", - crate_name = test_crate_name + "_flaky_" + flaky_test.replace("::", "_").replace("/", "_").replace("-", "_"), - crate_root = test, - srcs = [test], - data = native.glob(["tests/**"], allow_empty = True) + sanitized_binaries + test_data_extra, - compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra, - deps = all_crate_deps(normal = True, normal_dev = True) + maybe_deps + deps_extra, - rustc_flags = rustc_flags_extra + WINDOWS_RUSTC_LINK_FLAGS + [ - "--remap-path-prefix=../codex-rs=", - "--remap-path-prefix=codex-rs=", - ], - rustc_env = rustc_env, - env = cargo_env, - tags = test_tags + extra_test_tags + ["flaky"], - **flaky_test_kwargs - ) From 9e2fc318549f7546c87b9bd63db98a7c57f54cb4 Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Wed, 15 Apr 2026 14:43:33 -0700 Subject: [PATCH 2/4] Support original-detail metadata on MCP image outputs (#17714) ## 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 --- codex-rs/code-mode/src/description.rs | 2 +- codex-rs/code-mode/src/runtime/callbacks.rs | 15 +- codex-rs/code-mode/src/runtime/value.rs | 124 +++- codex-rs/code-mode/src/service.rs | 148 ++++ codex-rs/core/src/original_image_detail.rs | 1 + codex-rs/core/src/project_doc.rs | 1 + codex-rs/core/src/project_doc_tests.rs | 4 +- codex-rs/core/src/tools/code_mode/mod.rs | 9 + codex-rs/core/src/tools/context.rs | 6 + codex-rs/core/src/tools/context_tests.rs | 3 + codex-rs/core/src/tools/handlers/mcp.rs | 2 + codex-rs/core/src/tools/js_repl/kernel.js | 8 + codex-rs/core/tests/suite/code_mode.rs | 156 +++-- codex-rs/core/tests/suite/rmcp_client.rs | 635 ++++++++++-------- codex-rs/protocol/src/models.rs | 84 ++- .../rmcp-client/src/bin/test_stdio_server.rs | 18 + codex-rs/tools/src/image_detail.rs | 18 + codex-rs/tools/src/image_detail_tests.rs | 37 + codex-rs/tools/src/lib.rs | 1 + docs/js_repl.md | 1 + 20 files changed, 905 insertions(+), 368 deletions(-) diff --git a/codex-rs/code-mode/src/description.rs b/codex-rs/code-mode/src/description.rs index 1aee1f7a3b..81e45add00 100644 --- a/codex-rs/code-mode/src/description.rs +++ b/codex-rs/code-mode/src/description.rs @@ -24,7 +24,7 @@ const EXEC_DESCRIPTION_TEMPLATE: &str = r#"Run JavaScript code to orchestrate/co - Global helpers: - `exit()`: Immediately ends the current script successfully (like an early return from the top level). - `text(value: string | number | boolean | undefined | null)`: Appends a text item. Non-string values are stringified with `JSON.stringify(...)` when possible. -- `image(imageUrlOrItem: string | { image_url: string; detail?: "auto" | "low" | "high" | "original" | null })`: Appends an image item. `image_url` can be an HTTPS URL or a base64-encoded `data:` URL. +- `image(imageUrlOrItem: string | { image_url: string; detail?: "auto" | "low" | "high" | "original" | null } | ImageContent, detail?: "auto" | "low" | "high" | "original" | null)`: Appends an image item. `image_url` can be an HTTPS URL or a base64-encoded `data:` URL. To forward an MCP tool image, pass an individual `ImageContent` block from `result.content`, for example `image(result.content[0])`. MCP image blocks may request original detail with `_meta: { "codex/imageDetail": "original" }`. When provided, the second `detail` argument overrides any detail embedded in the first argument. - `store(key: string, value: any)`: stores a serializable value under a string key for later `exec` calls in the same session. - `load(key: string)`: returns the stored value for a string key, or `undefined` if it is missing. - `notify(value: string | number | boolean | undefined | null)`: immediately injects an extra `custom_tool_call_output` for the current `exec` call. Values are stringified like `text(...)`. diff --git a/codex-rs/code-mode/src/runtime/callbacks.rs b/codex-rs/code-mode/src/runtime/callbacks.rs index ace6f61144..a9755f6eb0 100644 --- a/codex-rs/code-mode/src/runtime/callbacks.rs +++ b/codex-rs/code-mode/src/runtime/callbacks.rs @@ -109,7 +109,20 @@ pub(super) fn image_callback( } else { args.get(0) }; - let image_item = match normalize_output_image(scope, value) { + let detail_override = if args.length() < 2 { + None + } else { + let detail = args.get(1); + if detail.is_string() { + Some(detail.to_rust_string_lossy(scope)) + } else if detail.is_null() || detail.is_undefined() { + None + } else { + throw_type_error(scope, "image detail must be a string when provided"); + return; + } + }; + let image_item = match normalize_output_image(scope, value, detail_override) { Ok(image_item) => image_item, Err(()) => return, }; diff --git a/codex-rs/code-mode/src/runtime/value.rs b/codex-rs/code-mode/src/runtime/value.rs index eb0280142c..5c63434f4f 100644 --- a/codex-rs/code-mode/src/runtime/value.rs +++ b/codex-rs/code-mode/src/runtime/value.rs @@ -3,6 +3,9 @@ use serde_json::Value as JsonValue; use crate::response::FunctionCallOutputContentItem; use crate::response::ImageDetail; +const IMAGE_HELPER_EXPECTS_MESSAGE: &str = "image expects a non-empty image URL string, an object with image_url and optional detail, or a raw MCP image block"; +const CODEX_IMAGE_DETAIL_META_KEY: &str = "codex/imageDetail"; + pub(super) fn serialize_output_text( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, @@ -34,45 +37,25 @@ pub(super) fn serialize_output_text( pub(super) fn normalize_output_image( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, + detail_override: Option, ) -> Result { let result = (|| -> Result { let (image_url, detail) = if value.is_string() { (value.to_rust_string_lossy(scope), None) } else if value.is_object() && !value.is_array() { - let object = v8::Local::::try_from(value).map_err(|_| { - "image expects a non-empty image URL string or an object with image_url and optional detail".to_string() - })?; - let image_url_key = v8::String::new(scope, "image_url") - .ok_or_else(|| "failed to allocate image helper keys".to_string())?; - let detail_key = v8::String::new(scope, "detail") - .ok_or_else(|| "failed to allocate image helper keys".to_string())?; - let image_url = object - .get(scope, image_url_key.into()) - .filter(|value| value.is_string()) - .map(|value| value.to_rust_string_lossy(scope)) - .ok_or_else(|| { - "image expects a non-empty image URL string or an object with image_url and optional detail" - .to_string() - })?; - let detail = match object.get(scope, detail_key.into()) { - Some(value) if value.is_string() => Some(value.to_rust_string_lossy(scope)), - Some(value) if value.is_null() || value.is_undefined() => None, - Some(_) => return Err("image detail must be a string when provided".to_string()), - None => None, - }; - (image_url, detail) + let object = v8::Local::::try_from(value) + .map_err(|_| IMAGE_HELPER_EXPECTS_MESSAGE.to_string())?; + if let Some(image) = parse_non_mcp_output_image(scope, object)? { + image + } else { + parse_mcp_output_image(scope, value)? + } } else { - return Err( - "image expects a non-empty image URL string or an object with image_url and optional detail" - .to_string(), - ); + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); }; if image_url.is_empty() { - return Err( - "image expects a non-empty image URL string or an object with image_url and optional detail" - .to_string(), - ); + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); } let lower = image_url.to_ascii_lowercase(); if !(lower.starts_with("http://") @@ -82,6 +65,7 @@ pub(super) fn normalize_output_image( return Err("image expects an http(s) or data URL".to_string()); } + let detail = detail_override.or(detail); let detail = match detail { Some(detail) => { let normalized = detail.to_ascii_lowercase(); @@ -112,6 +96,86 @@ pub(super) fn normalize_output_image( } } +fn parse_non_mcp_output_image( + scope: &mut v8::PinScope<'_, '_>, + object: v8::Local<'_, v8::Object>, +) -> Result)>, String> { + let image_url_key = v8::String::new(scope, "image_url") + .ok_or_else(|| "failed to allocate image helper keys".to_string())?; + let Some(image_url) = object.get(scope, image_url_key.into()) else { + return Ok(None); + }; + if image_url.is_undefined() { + return Ok(None); + } + if !image_url.is_string() { + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + } + let detail_key = v8::String::new(scope, "detail") + .ok_or_else(|| "failed to allocate image helper keys".to_string())?; + let detail = parse_image_detail_value(scope, object.get(scope, detail_key.into()))?; + Ok(Some((image_url.to_rust_string_lossy(scope), detail))) +} + +fn parse_mcp_output_image( + scope: &mut v8::PinScope<'_, '_>, + value: v8::Local<'_, v8::Value>, +) -> Result<(String, Option), String> { + let Some(result) = v8_value_to_json(scope, value)? else { + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + }; + let JsonValue::Object(result) = result else { + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + }; + let Some(item_type) = result.get("type").and_then(JsonValue::as_str) else { + return Err(IMAGE_HELPER_EXPECTS_MESSAGE.to_string()); + }; + if item_type != "image" { + return Err(format!( + "image only accepts MCP image blocks, got \"{item_type}\"" + )); + } + let data = result + .get("data") + .and_then(JsonValue::as_str) + .ok_or_else(|| "image expected MCP image data".to_string())?; + if data.is_empty() { + return Err("image expected MCP image data".to_string()); + } + + let image_url = if data.to_ascii_lowercase().starts_with("data:") { + data.to_string() + } else { + let mime_type = result + .get("mimeType") + .or_else(|| result.get("mime_type")) + .and_then(JsonValue::as_str) + .filter(|mime_type| !mime_type.is_empty()) + .unwrap_or("application/octet-stream"); + format!("data:{mime_type};base64,{data}") + }; + let detail = result + .get("_meta") + .and_then(JsonValue::as_object) + .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) + .and_then(JsonValue::as_str) + .filter(|detail| *detail == "original") + .map(str::to_string); + Ok((image_url, detail)) +} + +fn parse_image_detail_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + value: Option>, +) -> Result, String> { + match value { + Some(value) if value.is_string() => Ok(Some(value.to_rust_string_lossy(scope))), + Some(value) if value.is_null() || value.is_undefined() => Ok(None), + Some(_) => Err("image detail must be a string when provided".to_string()), + None => Ok(None), + } +} + pub(super) fn v8_value_to_json( scope: &mut v8::PinScope<'_, '_>, value: v8::Local<'_, v8::Value>, diff --git a/codex-rs/code-mode/src/service.rs b/codex-rs/code-mode/src/service.rs index 8a37880131..23ca7a7460 100644 --- a/codex-rs/code-mode/src/service.rs +++ b/codex-rs/code-mode/src/service.rs @@ -684,6 +684,154 @@ text(JSON.stringify(returnsUndefined)); ); } + #[tokio::test] + async fn image_helper_accepts_raw_mcp_image_block_with_original_detail() { + let service = CodeModeService::new(); + + let response = service + .execute(ExecuteRequest { + source: r#" +image({ + type: "image", + data: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==", + mimeType: "image/png", + _meta: { "codex/imageDetail": "original" }, +}); +"# + .to_string(), + yield_time_ms: None, + ..execute_request("") + }) + .await + .unwrap(); + + assert_eq!( + response, + RuntimeResponse::Result { + cell_id: "1".to_string(), + content_items: vec![FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==".to_string(), + detail: Some(crate::ImageDetail::Original), + }], + stored_values: HashMap::new(), + error_text: None, + } + ); + } + + #[tokio::test] + async fn image_helper_second_arg_overrides_explicit_object_detail() { + let service = CodeModeService::new(); + + let response = service + .execute(ExecuteRequest { + source: r#" +image( + { + image_url: "https://example.com/image.jpg", + detail: "low", + }, + "original", +); +"# + .to_string(), + yield_time_ms: None, + ..execute_request("") + }) + .await + .unwrap(); + + assert_eq!( + response, + RuntimeResponse::Result { + cell_id: "1".to_string(), + content_items: vec![FunctionCallOutputContentItem::InputImage { + image_url: "https://example.com/image.jpg".to_string(), + detail: Some(crate::ImageDetail::Original), + }], + stored_values: HashMap::new(), + error_text: None, + } + ); + } + + #[tokio::test] + async fn image_helper_second_arg_overrides_raw_mcp_image_detail() { + let service = CodeModeService::new(); + + let response = service + .execute(ExecuteRequest { + source: r#" +image( + { + type: "image", + data: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==", + mimeType: "image/png", + _meta: { "codex/imageDetail": "original" }, + }, + "low", +); +"# + .to_string(), + yield_time_ms: None, + ..execute_request("") + }) + .await + .unwrap(); + + assert_eq!( + response, + RuntimeResponse::Result { + cell_id: "1".to_string(), + content_items: vec![FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==".to_string(), + detail: Some(crate::ImageDetail::Low), + }], + stored_values: HashMap::new(), + error_text: None, + } + ); + } + + #[tokio::test] + async fn image_helper_rejects_raw_mcp_result_container() { + let service = CodeModeService::new(); + + let response = service + .execute(ExecuteRequest { + source: r#" +image({ + content: [ + { + type: "image", + data: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==", + mimeType: "image/png", + _meta: { "codex/imageDetail": "original" }, + }, + ], + isError: false, +}); +"# + .to_string(), + yield_time_ms: None, + ..execute_request("") + }) + .await + .unwrap(); + + assert_eq!( + response, + RuntimeResponse::Result { + cell_id: "1".to_string(), + content_items: Vec::new(), + stored_values: HashMap::new(), + error_text: Some( + "image expects a non-empty image URL string, an object with image_url and optional detail, or a raw MCP image block".to_string(), + ), + } + ); + } + #[tokio::test] async fn terminate_waits_for_runtime_shutdown_before_responding() { let inner = test_inner(); diff --git a/codex-rs/core/src/original_image_detail.rs b/codex-rs/core/src/original_image_detail.rs index c3e5c3d9a5..adfed321b8 100644 --- a/codex-rs/core/src/original_image_detail.rs +++ b/codex-rs/core/src/original_image_detail.rs @@ -1,2 +1,3 @@ pub(crate) use codex_tools::can_request_original_image_detail; pub(crate) use codex_tools::normalize_output_image_detail; +pub(crate) use codex_tools::sanitize_original_image_detail; diff --git a/codex-rs/core/src/project_doc.rs b/codex-rs/core/src/project_doc.rs index 6659dfea7f..c4f70a6f55 100644 --- a/codex-rs/core/src/project_doc.rs +++ b/codex-rs/core/src/project_doc.rs @@ -59,6 +59,7 @@ fn render_js_repl_instructions(config: &Config) -> Option { section.push_str("- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n"); section.push_str("- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n"); section.push_str("- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n"); + section.push_str("- Raw MCP image blocks can request the same behavior by returning `_meta: { \"codex/imageDetail\": \"original\" }` on the image content item.\n"); section.push_str("- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n"); section.push_str("- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n"); section.push_str("- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n"); diff --git a/codex-rs/core/src/project_doc_tests.rs b/codex-rs/core/src/project_doc_tests.rs index 705801317e..31f73805f7 100644 --- a/codex-rs/core/src/project_doc_tests.rs +++ b/codex-rs/core/src/project_doc_tests.rs @@ -205,7 +205,7 @@ async fn js_repl_instructions_are_appended_when_enabled() { let res = get_user_instructions(&cfg) .await .expect("js_repl instructions expected"); - let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`."; + let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Raw MCP image blocks can request the same behavior by returning `_meta: { \"codex/imageDetail\": \"original\" }` on the image content item.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`."; assert_eq!(res, expected); } @@ -224,7 +224,7 @@ async fn js_repl_tools_only_instructions_are_feature_gated() { let res = get_user_instructions(&cfg) .await .expect("js_repl instructions expected"); - let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Do not call tools directly; use `js_repl` + `codex.tool(...)` for all tool calls, including shell commands.\n- MCP tools (if any) can also be called by name via `codex.tool(...)`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`."; + let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.cwd`, `codex.homeDir`, `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds one image to the outer `js_repl` function output each time you call it, so you can call it multiple times to emit multiple images. It accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- `codex.tool(...)` and `codex.emitImage(...)` keep stable helper identities across cells. Saved references and persisted objects can reuse them in later cells, but async callbacks that fire after a cell finishes still fail because no exec is active.\n- Request full-resolution image processing with `detail: \"original\"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: \"original\"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents.\n- Raw MCP image blocks can request the same behavior by returning `_meta: { \"codex/imageDetail\": \"original\" }` on the image content item.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\", detail: \"original\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\", detail: \"original\" }))`.\n- When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits.\n- Top-level bindings persist across cells. If a cell throws, prior bindings remain available and bindings that finished initializing before the throw often remain usable in later cells. For code you plan to reuse across cells, prefer declaring or assigning it in direct top-level statements before operations that might throw. If you hit `SyntaxError: Identifier 'x' has already been declared`, first reuse the existing binding, reassign a previously declared `let`, or pick a new descriptive name. Use `{ ... }` only for a short temporary block when you specifically need local scratch names; do not wrap an entire cell in block scope if you want those names reusable later. Reset the kernel with `js_repl_reset` only when you need a clean state.\n- Top-level static import declarations (for example `import x from \"./file.js\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")`, `await import(\"./file.js\")`, or `await import(\"/abs/path/file.mjs\")` instead. Imported local files must be ESM `.js`/`.mjs` files and run in the same REPL VM context. Bare package imports always resolve from REPL-global search roots (`CODEX_JS_REPL_NODE_MODULE_DIRS`, then cwd), not relative to the imported file location. Local files may statically import only other local relative/absolute/`file://` `.js`/`.mjs` files; package and builtin imports from local files must stay dynamic. `import.meta.resolve()` returns importable strings such as `file://...`, bare package names, and `node:...` specifiers. Local file modules reload between execs, while top-level bindings persist until `js_repl_reset`.\n- Do not call tools directly; use `js_repl` + `codex.tool(...)` for all tool calls, including shell commands.\n- MCP tools (if any) can also be called by name via `codex.tool(...)`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`."; assert_eq!(res, expected); } diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index 0004ec6f51..8445fdf645 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -18,6 +18,8 @@ use tokio_util::sync::CancellationToken; use crate::codex::Session; use crate::codex::TurnContext; use crate::function_tool::FunctionCallError; +use crate::original_image_detail::can_request_original_image_detail; +use crate::original_image_detail::sanitize_original_image_detail as sanitize_image_detail_items; use crate::tools::ToolRouter; use crate::tools::context::FunctionToolOutput; use crate::tools::context::SharedTurnDiffTracker; @@ -160,12 +162,14 @@ pub(super) async fn handle_runtime_response( match response { RuntimeResponse::Yielded { content_items, .. } => { let mut content_items = into_function_call_output_content_items(content_items); + sanitize_runtime_image_detail(exec.turn.as_ref(), &mut content_items); content_items = truncate_code_mode_result(content_items, max_output_tokens); prepend_script_status(&mut content_items, &script_status, started_at.elapsed()); Ok(FunctionToolOutput::from_content(content_items, Some(true))) } RuntimeResponse::Terminated { content_items, .. } => { let mut content_items = into_function_call_output_content_items(content_items); + sanitize_runtime_image_detail(exec.turn.as_ref(), &mut content_items); content_items = truncate_code_mode_result(content_items, max_output_tokens); prepend_script_status(&mut content_items, &script_status, started_at.elapsed()); Ok(FunctionToolOutput::from_content(content_items, Some(true))) @@ -177,6 +181,7 @@ pub(super) async fn handle_runtime_response( .. } => { let mut content_items = into_function_call_output_content_items(content_items); + sanitize_runtime_image_detail(exec.turn.as_ref(), &mut content_items); exec.session .services .code_mode_service @@ -198,6 +203,10 @@ pub(super) async fn handle_runtime_response( } } +fn sanitize_runtime_image_detail(turn: &TurnContext, items: &mut [FunctionCallOutputContentItem]) { + sanitize_image_detail_items(can_request_original_image_detail(&turn.model_info), items); +} + fn format_script_status(response: &RuntimeResponse) -> String { match response { RuntimeResponse::Yielded { cell_id, .. } => { diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index dff1e444da..60bcbcda23 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -1,5 +1,6 @@ use crate::codex::Session; use crate::codex::TurnContext; +use crate::original_image_detail::sanitize_original_image_detail; use crate::tools::TELEMETRY_PREVIEW_MAX_BYTES; use crate::tools::TELEMETRY_PREVIEW_MAX_LINES; use crate::tools::TELEMETRY_PREVIEW_TRUNCATION_NOTICE; @@ -122,6 +123,7 @@ impl ToolOutput for CallToolResult { pub struct McpToolOutput { pub result: CallToolResult, pub wall_time: Duration, + pub original_image_detail_supported: bool, } impl ToolOutput for McpToolOutput { @@ -155,6 +157,10 @@ impl ToolOutput for McpToolOutput { impl McpToolOutput { fn response_payload(&self) -> FunctionCallOutputPayload { let mut payload = self.result.as_function_call_output_payload(); + if let Some(items) = payload.content_items_mut() { + sanitize_original_image_detail(self.original_image_detail_supported, items); + } + let wall_time_seconds = self.wall_time.as_secs_f64(); let header = format!("Wall time: {wall_time_seconds:.4} seconds\nOutput:"); diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index d45f351833..8df9159edd 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -98,6 +98,7 @@ fn mcp_tool_output_response_item_includes_wall_time() { meta: None, }, wall_time: std::time::Duration::from_millis(1250), + original_image_detail_supported: false, }; let response = output.to_response_item( @@ -149,6 +150,7 @@ fn mcp_tool_output_response_item_preserves_content_items() { meta: None, }, wall_time: std::time::Duration::from_millis(500), + original_image_detail_supported: false, }; let response = output.to_response_item( @@ -201,6 +203,7 @@ fn mcp_tool_output_code_mode_result_stays_raw_call_tool_result() { meta: None, }, wall_time: std::time::Duration::from_millis(1250), + original_image_detail_supported: false, }; let result = output.code_mode_result(&ToolPayload::Mcp { diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index cfe219f443..b24db734b9 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -3,6 +3,7 @@ use std::time::Instant; use crate::function_tool::FunctionCallError; use crate::mcp_tool_call::handle_mcp_tool_call; +use crate::original_image_detail::can_request_original_image_detail; use crate::tools::context::McpToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -56,6 +57,7 @@ impl ToolHandler for McpHandler { Ok(McpToolOutput { result, wall_time: started.elapsed(), + original_image_detail_supported: can_request_original_image_detail(&turn.model_info), }) } } diff --git a/codex-rs/core/src/tools/js_repl/kernel.js b/codex-rs/core/src/tools/js_repl/kernel.js index 5d31818521..3e5cf855f7 100644 --- a/codex-rs/core/src/tools/js_repl/kernel.js +++ b/codex-rs/core/src/tools/js_repl/kernel.js @@ -1330,6 +1330,13 @@ function normalizeMcpImageData(data, mimeType) { return `data:${normalizedMimeType};base64,${data}`; } +function parseMcpImageDetail(meta) { + if (!isPlainObject(meta) || meta["codex/imageDetail"] !== "original") { + return undefined; + } + return "original"; +} + function parseMcpToolResult(result) { if (typeof result === "string") { return { images: [], textCount: result.length > 0 ? 1 : 0 }; @@ -1362,6 +1369,7 @@ function parseMcpToolResult(result) { if (item.type === "image") { images.push({ image_url: normalizeMcpImageData(item.data, item.mimeType ?? item.mime_type), + detail: parseMcpImageDetail(item._meta), }); continue; } diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index f010c9e587..d514e801e9 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -178,7 +178,16 @@ async fn run_code_mode_turn_with_rmcp( prompt: &str, code: &str, ) -> Result<(TestCodex, ResponseMock)> { - run_code_mode_turn_with_rmcp_mode(server, prompt, code, /*code_mode_only*/ false).await + run_code_mode_turn_with_rmcp_model(server, prompt, code, "test-gpt-5.1-codex").await +} + +async fn run_code_mode_turn_with_rmcp_model( + server: &MockServer, + prompt: &str, + code: &str, + model: &'static str, +) -> Result<(TestCodex, ResponseMock)> { + run_code_mode_turn_with_rmcp_config(server, prompt, code, model, /*code_mode_only*/ false).await } async fn run_code_mode_turn_with_rmcp_mode( @@ -187,48 +196,57 @@ async fn run_code_mode_turn_with_rmcp_mode( code: &str, code_mode_only: bool, ) -> Result<(TestCodex, ResponseMock)> { - let rmcp_test_server_bin = stdio_server_bin()?; - let mut builder = test_codex() - .with_model("test-gpt-5.1-codex") - .with_config(move |config| { - let _ = if code_mode_only { - config.features.enable(Feature::CodeModeOnly) - } else { - config.features.enable(Feature::CodeMode) - }; + run_code_mode_turn_with_rmcp_config(server, prompt, code, "test-gpt-5.1-codex", code_mode_only) + .await +} - let mut servers = config.mcp_servers.get().clone(); - servers.insert( - "rmcp".to_string(), - McpServerConfig { - transport: McpServerTransportConfig::Stdio { - command: rmcp_test_server_bin, - args: Vec::new(), - env: Some(HashMap::from([( - "MCP_TEST_VALUE".to_string(), - "propagated-env".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(), +async fn run_code_mode_turn_with_rmcp_config( + server: &MockServer, + prompt: &str, + code: &str, + model: &'static str, + code_mode_only: bool, +) -> Result<(TestCodex, ResponseMock)> { + let rmcp_test_server_bin = stdio_server_bin()?; + let mut builder = test_codex().with_model(model).with_config(move |config| { + let _ = if code_mode_only { + config.features.enable(Feature::CodeModeOnly) + } else { + config.features.enable(Feature::CodeMode) + }; + + let mut servers = config.mcp_servers.get().clone(); + servers.insert( + "rmcp".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: rmcp_test_server_bin, + args: Vec::new(), + env: Some(HashMap::from([( + "MCP_TEST_VALUE".to_string(), + "propagated-env".to_string(), + )])), + env_vars: Vec::new(), + cwd: None, }, - ); - config - .mcp_servers - .set(servers) - .expect("test mcp servers should accept any configuration"); - }); + 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(), + }, + ); + config + .mcp_servers + .set(servers) + .expect("test mcp servers should accept any configuration"); + }); let test = builder.build(server).await?; responses::mount_sse_once( @@ -1919,6 +1937,62 @@ image(out); Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_can_use_mcp_image_result_with_image_helper() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let code = r#" +const out = await tools.mcp__rmcp__image_scenario({ + scenario: "image_only_original_detail", +}); +const imageItem = out.content.find((item) => item.type === "image"); +image(imageItem); +"#; + + let (_test, second_mock) = run_code_mode_turn_with_rmcp_model( + &server, + "use exec to call the rmcp image scenario tool and emit its image output", + code, + "gpt-5.3-codex", + ) + .await?; + + let req = second_mock.single_request(); + let items = custom_tool_output_items(&req, "call-1"); + let (_, success) = custom_tool_output_body_and_success(&req, "call-1"); + assert_ne!( + success, + Some(false), + "code_mode mcp image scenario call failed unexpectedly" + ); + assert_eq!(items.len(), 2); + assert_regex_match( + concat!( + r"(?s)\A", + r"Script completed\nWall time \d+\.\d seconds\nOutput:\n\z" + ), + text_item(&items, /*index*/ 0), + ); + + assert_eq!( + items[1].get("type").and_then(Value::as_str), + Some("input_image") + ); + + let emitted_image_url = items[1] + .get("image_url") + .and_then(Value::as_str) + .expect("image helper should emit an input_image item with image_url"); + assert!(emitted_image_url.starts_with("data:image/png;base64,")); + assert_eq!( + items[1].get("detail").and_then(Value::as_str), + Some("original") + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_can_apply_patch_via_nested_tool() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index b86094f910..6d19e6ac52 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -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, +} + +fn stdio_transport( + command: String, + env: Option>, + env_vars: Vec, +) -> 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 diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 0f18b68393..2200456527 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1362,6 +1362,8 @@ impl CallToolResult { fn convert_mcp_content_to_items( contents: &[serde_json::Value], ) -> Option> { + const CODEX_IMAGE_DETAIL_META_KEY: &str = "codex/imageDetail"; + #[derive(serde::Deserialize)] #[serde(tag = "type")] enum McpContent { @@ -1372,6 +1374,8 @@ fn convert_mcp_content_to_items( data: String, #[serde(rename = "mimeType", alias = "mime_type")] mime_type: Option, + #[serde(rename = "_meta", default)] + meta: Option, }, #[serde(other)] Unknown, @@ -1383,7 +1387,11 @@ fn convert_mcp_content_to_items( for content in contents { let item = match serde_json::from_value::(content.clone()) { Ok(McpContent::Text { text }) => FunctionCallOutputContentItem::InputText { text }, - Ok(McpContent::Image { data, mime_type }) => { + Ok(McpContent::Image { + data, + mime_type, + meta, + }) => { saw_image = true; let image_url = if data.starts_with("data:") { data @@ -1393,7 +1401,15 @@ fn convert_mcp_content_to_items( }; FunctionCallOutputContentItem::InputImage { image_url, - detail: None, + detail: meta + .as_ref() + .and_then(serde_json::Value::as_object) + .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) + .and_then(serde_json::Value::as_str) + .and_then(|detail| match detail { + "original" => Some(ImageDetail::Original), + _ => None, + }), } } Ok(McpContent::Unknown) | Err(_) => FunctionCallOutputContentItem::InputText { @@ -2264,6 +2280,70 @@ mod tests { Ok(()) } + #[test] + fn preserves_original_detail_metadata_on_mcp_images() -> Result<()> { + let call_tool_result = CallToolResult { + content: vec![serde_json::json!({ + "type": "image", + "data": "BASE64", + "mimeType": "image/png", + "_meta": { + "codex/imageDetail": "original", + }, + })], + structured_content: None, + is_error: Some(false), + meta: None, + }; + + let payload = call_tool_result.into_function_call_output_payload(); + let Some(items) = payload.content_items() else { + panic!("expected content items"); + }; + let items = items.to_vec(); + assert_eq!( + items, + vec![FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,BASE64".into(), + detail: Some(ImageDetail::Original), + }] + ); + + Ok(()) + } + + #[test] + fn ignores_unknown_mcp_image_detail_metadata() -> Result<()> { + let call_tool_result = CallToolResult { + content: vec![serde_json::json!({ + "type": "image", + "data": "BASE64", + "mimeType": "image/png", + "_meta": { + "codex/imageDetail": "high", + }, + })], + structured_content: None, + is_error: Some(false), + meta: None, + }; + + let payload = call_tool_result.into_function_call_output_payload(); + let Some(items) = payload.content_items() else { + panic!("expected content items"); + }; + let items = items.to_vec(); + assert_eq!( + items, + vec![FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,BASE64".into(), + detail: None, + }] + ); + + Ok(()) + } + #[test] fn deserializes_array_payload_into_items() -> Result<()> { let json = r#"[ diff --git a/codex-rs/rmcp-client/src/bin/test_stdio_server.rs b/codex-rs/rmcp-client/src/bin/test_stdio_server.rs index 326fd94279..82bc387d99 100644 --- a/codex-rs/rmcp-client/src/bin/test_stdio_server.rs +++ b/codex-rs/rmcp-client/src/bin/test_stdio_server.rs @@ -208,6 +208,7 @@ impl TestToolServer { /// - `codex mcp add mcpimg -- /abs/path/to/test_stdio_server` /// - Then in Codex TUI, ask it to call: /// - `mcpimg.image_scenario({"scenario":"image_only"})` + /// - `mcpimg.image_scenario({"scenario":"image_only_original_detail"})` /// - `mcpimg.image_scenario({"scenario":"text_then_image","caption":"Here is the image:"})` /// - `mcpimg.image_scenario({"scenario":"invalid_base64_then_image"})` /// - `mcpimg.image_scenario({"scenario":"invalid_image_bytes_then_image"})` @@ -224,6 +225,7 @@ impl TestToolServer { "type": "string", "enum": [ "image_only", + "image_only_original_detail", "text_then_image", "invalid_base64_then_image", "invalid_image_bytes_then_image", @@ -339,6 +341,7 @@ fn sync_barrier_map() -> &'static tokio::sync::Mutex { content.push(rmcp::model::Content::image(valid_data_b64, mime_type)); } + ImageScenario::ImageOnlyOriginalDetail => { + let mut meta = rmcp::model::Meta::new(); + meta.insert( + "codex/imageDetail".to_string(), + serde_json::json!("original"), + ); + content.push(rmcp::model::Annotated::new( + rmcp::model::RawContent::Image(rmcp::model::RawImageContent { + data: valid_data_b64, + mime_type, + meta: Some(meta), + }), + None, + )); + } ImageScenario::TextThenImage => { content.push(rmcp::model::Content::text(caption)); content.push(rmcp::model::Content::image(valid_data_b64, mime_type)); diff --git a/codex-rs/tools/src/image_detail.rs b/codex-rs/tools/src/image_detail.rs index 94639e61f9..56987e4831 100644 --- a/codex-rs/tools/src/image_detail.rs +++ b/codex-rs/tools/src/image_detail.rs @@ -1,3 +1,4 @@ +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::ImageDetail; use codex_protocol::openai_models::ModelInfo; @@ -17,6 +18,23 @@ pub fn normalize_output_image_detail( } } +pub fn sanitize_original_image_detail( + can_request_original_image_detail: bool, + items: &mut [FunctionCallOutputContentItem], +) { + if can_request_original_image_detail { + return; + } + + for item in items { + if let FunctionCallOutputContentItem::InputImage { detail, .. } = item + && matches!(detail, Some(ImageDetail::Original)) + { + *detail = None; + } + } +} + #[cfg(test)] #[path = "image_detail_tests.rs"] mod tests; diff --git a/codex-rs/tools/src/image_detail_tests.rs b/codex-rs/tools/src/image_detail_tests.rs index 35d666149f..c1a0f8ca1a 100644 --- a/codex-rs/tools/src/image_detail_tests.rs +++ b/codex-rs/tools/src/image_detail_tests.rs @@ -1,4 +1,5 @@ use super::*; +use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::ImageDetail; use codex_protocol::openai_models::ModelInfo; use pretty_assertions::assert_eq; @@ -73,3 +74,39 @@ fn unsupported_non_original_detail_is_dropped() { None ); } + +#[test] +fn sanitize_original_drops_original_without_support() { + let mut items = vec![ + FunctionCallOutputContentItem::InputText { + text: "header".to_string(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,AAA".to_string(), + detail: Some(ImageDetail::Original), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,BBB".to_string(), + detail: Some(ImageDetail::Low), + }, + ]; + + sanitize_original_image_detail(/*can_request_original_image_detail*/ false, &mut items); + + assert_eq!( + items, + vec![ + FunctionCallOutputContentItem::InputText { + text: "header".to_string(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,AAA".to_string(), + detail: None, + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,BBB".to_string(), + detail: Some(ImageDetail::Low), + }, + ] + ); +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 4a3ccc01ae..b7c6a57d2b 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -53,6 +53,7 @@ pub use codex_protocol::ToolName; pub use dynamic_tool::parse_dynamic_tool; pub use image_detail::can_request_original_image_detail; pub use image_detail::normalize_output_image_detail; +pub use image_detail::sanitize_original_image_detail; pub use js_repl_tool::create_js_repl_reset_tool; pub use js_repl_tool::create_js_repl_tool; pub use json_schema::AdditionalProperties; diff --git a/docs/js_repl.md b/docs/js_repl.md index d5edc71b46..aee635b77a 100644 --- a/docs/js_repl.md +++ b/docs/js_repl.md @@ -86,6 +86,7 @@ imported local file. They are not resolved relative to the imported file's locat - `codex.emitImage(...)` accepts a data URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object that contains exactly one image and no text. Call it multiple times if you want to emit multiple images. - `codex.emitImage(...)` rejects mixed text-and-image content. - Request full-resolution image processing with `detail: "original"` only when the `view_image` tool schema includes a `detail` argument. The same availability applies to `codex.emitImage(...)`: if `view_image.detail` is present, you may also pass `detail: "original"` there. Use this when high-fidelity image perception or precise localization is needed, especially for CUA agents. +- Raw MCP image blocks can request the same behavior by returning `_meta: { "codex/imageDetail": "original" }` on the image content item. - Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: "jpeg", quality: 85 }), mimeType: "image/jpeg", detail: "original" })`. - Example of sharing a local image tool result: `await codex.emitImage(codex.tool("view_image", { path: "/absolute/path", detail: "original" }))`. - When encoding an image to send with `codex.emitImage(...)` or `view_image`, prefer JPEG at about 85 quality when lossy compression is acceptable; use PNG when transparency or lossless detail matters. Smaller uploads are faster and less likely to hit size limits. From 9d1bf002c6f3d25b355a0104994bc4b410011a14 Mon Sep 17 00:00:00 2001 From: efrazer-oai Date: Wed, 15 Apr 2026 14:44:01 -0700 Subject: [PATCH 3/4] Significantly improve standalone installer (#17022) ## Summary This PR significantly improves the standalone installer experience. The main changes are: 1. We now install the codex binary and other dependencies in a subdirectory under CODEX_HOME. (`CODEX_HOME/packages/standalone/releases/...`) 2. We replace the `codex.js` launcher that npm/bun rely on with logic in the Rust binary that automatically resolves its dependencies (like ripgrep) ## Motivation A few design constraints pushed this work. 1. Currently, the entrypoint to codex is through `codex.js`, which forces a node dependency to kick off our rust app. We want to move away from this so that the entrypoint to codex does not rely on node or external package managers. 2. Right now, the native script adds codex and its dependencies directly to user PATH. Given that codex is likely to add more binary dependencies than ripgrep, we want a solution which does not add arbitrary binaries to user PATH -- the only one we want to add is the `codex` command itself. 3. We want upgrades to be atomic. We do not want scenarios where interrupting an upgrade command can move codex into undefined state (for example, having a new codex binary but an old ripgrep binary). This was ~possible with the old script. 4. Currently, the Rust binary uses heuristics to determine which installer created it. These heuristics are flaky and are tied to the `codex.js` launcher. We need a more stable/deterministic way to determine how the binary was installed for standalone. 5. We do not want conflicting codex installations on PATH. For example, the user installing via npm, then installing via brew, then installing via standalone would make it unclear which version of codex is being launched and make it tough for us to determine the right upgrade command. ## Design ### Standalone package layout Standalone installs now live under `CODEX_HOME/packages/standalone`: ```text $CODEX_HOME/ packages/ standalone/ current -> releases/0.111.0-x86_64-unknown-linux-musl releases/ 0.111.0-x86_64-unknown-linux-musl/ codex codex-resources/ rg ``` where `standalone/current` is a symlink to a release directory. On Windows, the release directory has the same shape, with `.exe` names and Windows helpers in `codex-resources`: ```text %CODEX_HOME%\ packages\ standalone\ current -> releases\0.111.0-x86_64-pc-windows-msvc releases\ 0.111.0-x86_64-pc-windows-msvc\ codex.exe codex-resources\ rg.exe codex-command-runner.exe codex-windows-sandbox-setup.exe ``` This gives us: - atomic upgrades because we can fully stage a release before switching `standalone/current` - a stable way for the binary to recognize a standalone install from its canonical `current_exe()` path under CODEX_HOME - a clean place for binary dependencies like `rg`, Windows sandbox helpers, and, in the future, our custom `zsh` etc ### Command location On Unix, we add a symlink at `~/.local/bin/codex` which points directly to the `$CODEX_HOME/packages/standalone/current/codex` binary. This becomes the main entrypoint for the CLI. On Windows, we store the link at `%LOCALAPPDATA%\Programs\OpenAI\Codex\bin`. ### PATH persistence This is a tricky part of the PR, as there's no ~super reliable way to ensure that we end up on PATH without significant tradeoffs. Most Unix variants will have `~/.local/bin` on PATH already, which means we *should* be fine simply registering the command there in most cases. However, there are cases where this is not the case. In these cases, we directly edit the profile depending on the shell we're in. - macOS zsh: `~/.zprofile` - macOS bash: `~/.bash_profile` - Linux zsh: `~/.zshrc` - Linux bash: `~/.bashrc` - fallback: `~/.profile` On Windows, we update the User `Path` environment variable directly and we don't need to worry about shell profiles. ### Standalone runtime detection This PR adds a new shared crate, `codex-install-context`, which computes install ownership once per process and caches it in a `OnceLock`. That context includes: - install manager (`Standalone`, `Npm`, `Bun`, `Brew`, `Other`) - the managed standalone release directory, when applicable - the managed standalone `codex-resources` directory, when present - the resolved `rg_command` The standalone path is detected by canonicalizing `current_exe()`, canonicalizing CODEX_HOME via `find_codex_home()`, and checking whether the binary is running from under `$CODEX_HOME/packages/standalone/releases`. We intentionally do not use a release metadata file. The binary path is the source of truth. ### Dependency resolution For standalone installs, `grep_files` now resolves bundled `rg` from `codex-resources` next to the Codex binary. For npm/bun/brew/other installs, `grep_files` falls back to resolving `rg` from PATH. For Windows standalone installs, Windows sandbox helpers are still found as direct siblings when present. If they are not direct siblings, the lookup also checks the sibling `codex-resources` directory. ### TUI update path The TUI now has `UpdateAction::StandaloneUnix` and `UpdateAction::StandaloneWindows`, which rerun the standalone install commands. Unix update command: ```sh sh -c "curl -fsSL https://chatgpt.com/codex/install.sh | sh" ``` Windows update command: ```powershell powershell -c "irm https://chatgpt.com/codex/install.ps1|iex" ``` The Windows updater runs PowerShell directly. We do this because `cmd /C` would parse the `|iex` as a cmd pipeline instead of passing it to PowerShell. ## Additional installer behavior - standalone installs now warn about conflicting npm/bun/brew-managed `codex` installs and offer to uninstall them - same-version reruns do not redownload the release if it is already staged locally ## Testing Installer smoke tests run: - macOS: fresh install into isolated `HOME` and `CODEX_HOME` with `scripts/install/install.sh --release latest` - macOS: reran the installer against the same isolated install to verify the same-version/update path and PATH block idempotence - macOS: verified the installed `codex --version` and bundled `codex-resources/rg --version` - Windows: parsed `scripts/install/install.ps1` with PowerShell via `[scriptblock]::Create(...)` - Windows: verified the standalone update action builds a direct PowerShell command and does not route the `irm ...|iex` command through `cmd /C` --------- Co-authored-by: Codex --- codex-rs/Cargo.lock | 10 + codex-rs/Cargo.toml | 2 + codex-rs/README.md | 2 +- codex-rs/cli/src/main.rs | 17 +- codex-rs/install-context/BUILD.bazel | 6 + codex-rs/install-context/Cargo.toml | 19 + codex-rs/install-context/src/lib.rs | 258 +++++++ codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/update_action.rs | 135 ++-- .../src/helper_materialization.rs | 78 +- .../src/setup_orchestrator.rs | 10 + scripts/install/install.ps1 | 668 ++++++++++++++++-- scripts/install/install.sh | 660 +++++++++++++++-- 13 files changed, 1656 insertions(+), 210 deletions(-) create mode 100644 codex-rs/install-context/BUILD.bazel create mode 100644 codex-rs/install-context/Cargo.toml create mode 100644 codex-rs/install-context/src/lib.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 68e0ec092f..2d39c42df4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2255,6 +2255,15 @@ dependencies = [ "tokio", ] +[[package]] +name = "codex-install-context" +version = "0.0.0" +dependencies = [ + "codex-utils-home-dir", + "pretty_assertions", + "tempfile", +] + [[package]] name = "codex-instructions" version = "0.0.0" @@ -2896,6 +2905,7 @@ dependencies = [ "codex-feedback", "codex-file-search", "codex-git-utils", + "codex-install-context", "codex-login", "codex-mcp", "codex-model-provider-info", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 4bb8837211..47c0d348bf 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -13,6 +13,7 @@ members = [ "arg0", "feedback", "features", + "install-context", "codex-backend-openapi-models", "code-mode", "cloud-requirements", @@ -134,6 +135,7 @@ codex-execpolicy = { path = "execpolicy" } codex-experimental-api-macros = { path = "codex-experimental-api-macros" } codex-features = { path = "features" } codex-feedback = { path = "feedback" } +codex-install-context = { path = "install-context" } codex-file-search = { path = "file-search" } codex-git-utils = { path = "git-utils" } codex-hooks = { path = "hooks" } diff --git a/codex-rs/README.md b/codex-rs/README.md index 6307668f39..2ad7158f98 100644 --- a/codex-rs/README.md +++ b/codex-rs/README.md @@ -1,6 +1,6 @@ # Codex CLI (Rust Implementation) -We provide Codex CLI as a standalone, native executable to ensure a zero-dependency install. +We provide Codex CLI as a standalone executable to ensure a zero-dependency install. ## Installing Codex diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 4e962cf091..6accb2cb0c 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -516,10 +516,19 @@ fn run_update_action(action: UpdateAction) -> anyhow::Result<()> { let status = { #[cfg(windows)] { - // On Windows, run via cmd.exe so .CMD/.BAT are correctly resolved (PATHEXT semantics). - std::process::Command::new("cmd") - .args(["/C", &cmd_str]) - .status()? + if action == UpdateAction::StandaloneWindows { + let (cmd, args) = action.command_args(); + // Run the standalone PowerShell installer with PowerShell + // itself. Routing this through `cmd.exe /C` would parse + // PowerShell metacharacters like `|` before PowerShell sees + // the installer command. + std::process::Command::new(cmd).args(args).status()? + } else { + // On Windows, run via cmd.exe so .CMD/.BAT are correctly resolved (PATHEXT semantics). + std::process::Command::new("cmd") + .args(["/C", &cmd_str]) + .status()? + } } #[cfg(not(windows))] { diff --git a/codex-rs/install-context/BUILD.bazel b/codex-rs/install-context/BUILD.bazel new file mode 100644 index 0000000000..68254d10cc --- /dev/null +++ b/codex-rs/install-context/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "install-context", + crate_name = "codex_install_context", +) diff --git a/codex-rs/install-context/Cargo.toml b/codex-rs/install-context/Cargo.toml new file mode 100644 index 0000000000..ce4eeefe77 --- /dev/null +++ b/codex-rs/install-context/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "codex-install-context" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +name = "codex_install_context" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-utils-home-dir = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/install-context/src/lib.rs b/codex-rs/install-context/src/lib.rs new file mode 100644 index 0000000000..980fc3f546 --- /dev/null +++ b/codex-rs/install-context/src/lib.rs @@ -0,0 +1,258 @@ +use std::path::Path; +use std::path::PathBuf; +use std::sync::OnceLock; + +const RELEASES_DIRNAME: &str = "releases"; +const RESOURCES_DIRNAME: &str = "codex-resources"; +const STANDALONE_PACKAGES_DIRNAME: &str = "standalone"; +static INSTALL_CONTEXT: OnceLock = OnceLock::new(); + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum StandalonePlatform { + Unix, + Windows, +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum InstallContext { + Standalone { + /// The managed standalone release directory, for example + /// `~/.codex/packages/standalone/releases/0.111.0-x86_64-unknown-linux-musl`. + release_dir: PathBuf, + /// The bundled resource directory that sits next to the executable when + /// this install ships managed dependencies. + resources_dir: Option, + /// The platform of the standalone release, either `Unix` or `Windows`. + platform: StandalonePlatform, + }, + /// A Codex binary launched through the npm-managed `codex.js` shim. + Npm, + /// A Codex binary launched through the bun-managed `codex.js` shim. + Bun, + /// A Codex binary that appears to come from a Homebrew install prefix. + Brew, + /// Any other execution environment. + /// + /// This commonly covers `cargo run`, app-bundled Codex binaries, custom + /// internal launchers, and tests that execute Codex from an arbitrary path. + Other, +} + +impl InstallContext { + pub fn from_exe( + is_macos: bool, + current_exe: Option<&Path>, + managed_by_npm: bool, + managed_by_bun: bool, + ) -> Self { + let codex_home = codex_utils_home_dir::find_codex_home().ok(); + Self::from_exe_with_codex_home( + is_macos, + current_exe, + managed_by_npm, + managed_by_bun, + codex_home.as_deref(), + ) + } + + fn from_exe_with_codex_home( + is_macos: bool, + current_exe: Option<&Path>, + managed_by_npm: bool, + managed_by_bun: bool, + codex_home: Option<&Path>, + ) -> Self { + if managed_by_npm { + return Self::Npm; + } + + if managed_by_bun { + return Self::Bun; + } + + if let Some(exe_path) = current_exe + && let Some(standalone_context) = standalone_install_context(exe_path, codex_home) + { + return standalone_context; + } + + if is_macos + && let Some(exe_path) = current_exe + && (exe_path.starts_with("/opt/homebrew") || exe_path.starts_with("/usr/local")) + { + return Self::Brew; + } + + Self::Other + } + + pub fn current() -> &'static Self { + INSTALL_CONTEXT.get_or_init(|| { + let current_exe = std::env::current_exe().ok(); + let managed_by_npm = std::env::var_os("CODEX_MANAGED_BY_NPM").is_some(); + let managed_by_bun = std::env::var_os("CODEX_MANAGED_BY_BUN").is_some(); + Self::from_exe( + cfg!(target_os = "macos"), + current_exe.as_deref(), + managed_by_npm, + managed_by_bun, + ) + }) + } + + pub fn rg_command(&self) -> PathBuf { + match self { + Self::Standalone { + resources_dir: Some(resources_dir), + .. + } => { + let bundled_rg = resources_dir.join(default_rg_command()); + if bundled_rg.exists() { + bundled_rg + } else { + default_rg_command() + } + } + Self::Standalone { + resources_dir: None, + .. + } + | Self::Npm + | Self::Bun + | Self::Brew + | Self::Other => default_rg_command(), + } + } +} + +fn standalone_install_context( + exe_path: &Path, + codex_home: Option<&Path>, +) -> Option { + let canonical_exe = std::fs::canonicalize(exe_path).ok()?; + let canonical_codex_home = std::fs::canonicalize(codex_home?).ok()?; + let release_dir = canonical_exe.parent()?.to_path_buf(); + let releases_root = canonical_codex_home + .join("packages") + .join(STANDALONE_PACKAGES_DIRNAME) + .join(RELEASES_DIRNAME); + if !release_dir.starts_with(releases_root) { + return None; + } + + let resources_dir = release_dir.join(RESOURCES_DIRNAME); + Some(InstallContext::Standalone { + release_dir, + resources_dir: resources_dir.is_dir().then_some(resources_dir), + platform: standalone_platform(), + }) +} + +fn standalone_platform() -> StandalonePlatform { + if cfg!(windows) { + StandalonePlatform::Windows + } else { + StandalonePlatform::Unix + } +} + +fn default_rg_command() -> PathBuf { + if cfg!(windows) { + PathBuf::from("rg.exe") + } else { + PathBuf::from("rg") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use std::fs; + + #[test] + fn detects_standalone_install_from_release_layout() -> std::io::Result<()> { + let codex_home = tempfile::tempdir()?; + let release_dir = codex_home + .path() + .join("packages/standalone/releases/1.2.3-x86_64-unknown-linux-musl"); + let resources_dir = release_dir.join(RESOURCES_DIRNAME); + fs::create_dir_all(&resources_dir)?; + let exe_path = release_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" }); + fs::write(&exe_path, "")?; + fs::write(resources_dir.join(default_rg_command()), "")?; + let canonical_release_dir = release_dir.canonicalize()?; + let canonical_resources_dir = resources_dir.canonicalize()?; + + let context = InstallContext::from_exe_with_codex_home( + /*is_macos*/ false, + /*current_exe*/ Some(&exe_path), + /*managed_by_npm*/ false, + /*managed_by_bun*/ false, + /*codex_home*/ Some(codex_home.path()), + ); + assert_eq!( + context, + InstallContext::Standalone { + release_dir: canonical_release_dir, + resources_dir: Some(canonical_resources_dir), + platform: standalone_platform(), + } + ); + Ok(()) + } + + #[test] + fn standalone_rg_falls_back_when_resources_are_missing() -> std::io::Result<()> { + let codex_home = tempfile::tempdir()?; + let release_dir = codex_home + .path() + .join("packages/standalone/releases/1.2.3-x86_64-unknown-linux-musl"); + fs::create_dir_all(&release_dir)?; + let exe_path = release_dir.join(if cfg!(windows) { "codex.exe" } else { "codex" }); + fs::write(&exe_path, "")?; + + let context = InstallContext::from_exe_with_codex_home( + /*is_macos*/ false, + /*current_exe*/ Some(&exe_path), + /*managed_by_npm*/ false, + /*managed_by_bun*/ false, + /*codex_home*/ Some(codex_home.path()), + ); + assert_eq!(context.rg_command(), default_rg_command()); + Ok(()) + } + + #[test] + fn npm_and_bun_take_precedence() { + let npm_context = InstallContext::from_exe_with_codex_home( + /*is_macos*/ false, + /*current_exe*/ Some(Path::new("/tmp/codex")), + /*managed_by_npm*/ true, + /*managed_by_bun*/ false, + /*codex_home*/ None, + ); + assert_eq!(npm_context, InstallContext::Npm); + + let bun_context = InstallContext::from_exe_with_codex_home( + /*is_macos*/ false, + /*current_exe*/ Some(Path::new("/tmp/codex")), + /*managed_by_npm*/ false, + /*managed_by_bun*/ true, + /*codex_home*/ None, + ); + assert_eq!(bun_context, InstallContext::Bun); + } + + #[test] + fn brew_is_detected_on_macos_prefixes() { + let context = InstallContext::from_exe_with_codex_home( + /*is_macos*/ true, + /*current_exe*/ Some(Path::new("/opt/homebrew/bin/codex")), + /*managed_by_npm*/ false, + /*managed_by_bun*/ false, + /*codex_home*/ None, + ); + assert_eq!(context, InstallContext::Brew); + } +} diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 0dec8ceed1..0f29fdeae6 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -29,6 +29,7 @@ codex-ansi-escape = { workspace = true } codex-app-server-client = { workspace = true } codex-app-server-protocol = { workspace = true } codex-arg0 = { workspace = true } +codex-install-context = { workspace = true } codex-chatgpt = { workspace = true } codex-cloud-requirements = { workspace = true } codex-config = { workspace = true } diff --git a/codex-rs/tui/src/update_action.rs b/codex-rs/tui/src/update_action.rs index baee1662fd..1ac2ff675f 100644 --- a/codex-rs/tui/src/update_action.rs +++ b/codex-rs/tui/src/update_action.rs @@ -1,3 +1,8 @@ +#[cfg(any(not(debug_assertions), test))] +use codex_install_context::InstallContext; +#[cfg(any(not(debug_assertions), test))] +use codex_install_context::StandalonePlatform; + /// Update action the CLI should perform after the TUI exits. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum UpdateAction { @@ -7,15 +12,41 @@ pub enum UpdateAction { BunGlobalLatest, /// Update via `brew upgrade codex`. BrewUpgrade, + /// Update via `curl -fsSL https://chatgpt.com/codex/install.sh | sh`. + StandaloneUnix, + /// Update via `irm https://chatgpt.com/codex/install.ps1|iex`. + StandaloneWindows, } impl UpdateAction { + #[cfg(any(not(debug_assertions), test))] + pub(crate) fn from_install_context(context: &InstallContext) -> Option { + match context { + InstallContext::Npm => Some(UpdateAction::NpmGlobalLatest), + InstallContext::Bun => Some(UpdateAction::BunGlobalLatest), + InstallContext::Brew => Some(UpdateAction::BrewUpgrade), + InstallContext::Standalone { platform, .. } => Some(match platform { + StandalonePlatform::Unix => UpdateAction::StandaloneUnix, + StandalonePlatform::Windows => UpdateAction::StandaloneWindows, + }), + InstallContext::Other => None, + } + } + /// Returns the list of command-line arguments for invoking the update. pub fn command_args(self) -> (&'static str, &'static [&'static str]) { match self { UpdateAction::NpmGlobalLatest => ("npm", &["install", "-g", "@openai/codex"]), UpdateAction::BunGlobalLatest => ("bun", &["install", "-g", "@openai/codex"]), UpdateAction::BrewUpgrade => ("brew", &["upgrade", "--cask", "codex"]), + UpdateAction::StandaloneUnix => ( + "sh", + &["-c", "curl -fsSL https://chatgpt.com/codex/install.sh | sh"], + ), + UpdateAction::StandaloneWindows => ( + "powershell", + &["-c", "irm https://chatgpt.com/codex/install.ps1|iex"], + ), } } @@ -29,88 +60,68 @@ impl UpdateAction { #[cfg(not(debug_assertions))] pub(crate) fn get_update_action() -> Option { - let exe = std::env::current_exe().unwrap_or_default(); - let managed_by_npm = std::env::var_os("CODEX_MANAGED_BY_NPM").is_some(); - let managed_by_bun = std::env::var_os("CODEX_MANAGED_BY_BUN").is_some(); - - detect_update_action( - cfg!(target_os = "macos"), - &exe, - managed_by_npm, - managed_by_bun, - ) -} - -#[cfg(any(not(debug_assertions), test))] -fn detect_update_action( - is_macos: bool, - current_exe: &std::path::Path, - managed_by_npm: bool, - managed_by_bun: bool, -) -> Option { - if managed_by_npm { - Some(UpdateAction::NpmGlobalLatest) - } else if managed_by_bun { - Some(UpdateAction::BunGlobalLatest) - } else if is_macos - && (current_exe.starts_with("/opt/homebrew") || current_exe.starts_with("/usr/local")) - { - Some(UpdateAction::BrewUpgrade) - } else { - None - } + UpdateAction::from_install_context(InstallContext::current()) } #[cfg(test)] mod tests { use super::*; + use pretty_assertions::assert_eq; + use std::path::PathBuf; #[test] - fn detects_update_action_without_env_mutation() { + fn maps_install_context_to_update_action() { + let native_release_dir = PathBuf::from("/tmp/native-release"); + assert_eq!( - detect_update_action( - /*is_macos*/ false, - std::path::Path::new("/any/path"), - /*managed_by_npm*/ false, - /*managed_by_bun*/ false - ), + UpdateAction::from_install_context(&InstallContext::Other), None ); assert_eq!( - detect_update_action( - /*is_macos*/ false, - std::path::Path::new("/any/path"), - /*managed_by_npm*/ true, - /*managed_by_bun*/ false - ), + UpdateAction::from_install_context(&InstallContext::Npm), Some(UpdateAction::NpmGlobalLatest) ); assert_eq!( - detect_update_action( - /*is_macos*/ false, - std::path::Path::new("/any/path"), - /*managed_by_npm*/ false, - /*managed_by_bun*/ true - ), + UpdateAction::from_install_context(&InstallContext::Bun), Some(UpdateAction::BunGlobalLatest) ); assert_eq!( - detect_update_action( - /*is_macos*/ true, - std::path::Path::new("/opt/homebrew/bin/codex"), - /*managed_by_npm*/ false, - /*managed_by_bun*/ false - ), + UpdateAction::from_install_context(&InstallContext::Brew), Some(UpdateAction::BrewUpgrade) ); assert_eq!( - detect_update_action( - /*is_macos*/ true, - std::path::Path::new("/usr/local/bin/codex"), - /*managed_by_npm*/ false, - /*managed_by_bun*/ false - ), - Some(UpdateAction::BrewUpgrade) + UpdateAction::from_install_context(&InstallContext::Standalone { + platform: StandalonePlatform::Unix, + release_dir: native_release_dir.clone(), + resources_dir: Some(native_release_dir.join("codex-resources")), + }), + Some(UpdateAction::StandaloneUnix) + ); + assert_eq!( + UpdateAction::from_install_context(&InstallContext::Standalone { + platform: StandalonePlatform::Windows, + release_dir: native_release_dir.clone(), + resources_dir: Some(native_release_dir.join("codex-resources")), + }), + Some(UpdateAction::StandaloneWindows) + ); + } + + #[test] + fn standalone_update_commands_rerun_latest_installer() { + assert_eq!( + UpdateAction::StandaloneUnix.command_args(), + ( + "sh", + &["-c", "curl -fsSL https://chatgpt.com/codex/install.sh | sh"][..], + ) + ); + assert_eq!( + UpdateAction::StandaloneWindows.command_args(), + ( + "powershell", + &["-c", "irm https://chatgpt.com/codex/install.ps1|iex"][..], + ) ); } } diff --git a/codex-rs/windows-sandbox-rs/src/helper_materialization.rs b/codex-rs/windows-sandbox-rs/src/helper_materialization.rs index 375199f5cc..ad5cc4e2b2 100644 --- a/codex-rs/windows-sandbox-rs/src/helper_materialization.rs +++ b/codex-rs/windows-sandbox-rs/src/helper_materialization.rs @@ -13,6 +13,8 @@ use tempfile::NamedTempFile; use crate::logging::log_note; use crate::sandbox_bin_dir; +const RESOURCES_DIRNAME: &str = "codex-resources"; + #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub(crate) enum HelperExecutable { CommandRunner, @@ -46,12 +48,9 @@ pub(crate) fn helper_bin_dir(codex_home: &Path) -> PathBuf { pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf { if let Ok(exe) = std::env::current_exe() - && let Some(dir) = exe.parent() + && let Some(candidate) = source_path_for_exe(&exe, kind.file_name()) { - let candidate = dir.join(kind.file_name()); - if candidate.exists() { - return candidate; - } + return candidate; } PathBuf::from(kind.file_name()) } @@ -179,18 +178,23 @@ fn store_helper_path(cache_key: String, path: PathBuf) { fn sibling_source_path(kind: HelperExecutable) -> Result { let exe = std::env::current_exe().context("resolve current executable for helper lookup")?; - let dir = exe - .parent() - .ok_or_else(|| anyhow!("current executable has no parent directory"))?; - let candidate = dir.join(kind.file_name()); - if candidate.exists() { - Ok(candidate) - } else { - Err(anyhow!( - "helper not found next to current executable: {}", - candidate.display() - )) + source_path_for_exe(&exe, kind.file_name()).ok_or_else(|| { + anyhow!( + "helper not found next to current executable or under {RESOURCES_DIRNAME}: {}", + exe.display() + ) + }) +} + +fn source_path_for_exe(exe: &Path, file_name: &str) -> Option { + let dir = exe.parent()?; + let direct_candidate = dir.join(file_name); + if direct_candidate.exists() { + return Some(direct_candidate); } + + let resource_candidate = dir.join(RESOURCES_DIRNAME).join(file_name); + resource_candidate.exists().then_some(resource_candidate) } fn copy_from_source_if_needed(source: &Path, destination: &Path) -> Result { @@ -292,10 +296,12 @@ fn destination_is_fresh(source: &Path, destination: &Path) -> Result { #[cfg(test)] mod tests { - use super::destination_is_fresh; - use super::helper_bin_dir; use super::copy_from_source_if_needed; use super::CopyOutcome; + use super::destination_is_fresh; + use super::helper_bin_dir; + use super::RESOURCES_DIRNAME; + use super::source_path_for_exe; use pretty_assertions::assert_eq; use std::fs; use std::path::Path; @@ -376,4 +382,40 @@ mod tests { fs::read(&runner_destination).expect("read runner") ); } + + #[test] + fn helper_source_lookup_checks_resource_dir() { + let tmp = TempDir::new().expect("tempdir"); + let release_dir = tmp.path().join("release"); + let resources_dir = release_dir.join(RESOURCES_DIRNAME); + fs::create_dir_all(&resources_dir).expect("create resources dir"); + let exe = release_dir.join("codex.exe"); + let helper = resources_dir.join("codex-command-runner.exe"); + fs::write(&exe, b"codex").expect("write exe"); + fs::write(&helper, b"runner").expect("write helper"); + + let resolved = + source_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe").expect("helper path"); + + assert_eq!(resolved, helper); + } + + #[test] + fn helper_source_lookup_prefers_direct_sibling_over_resource_dir() { + let tmp = TempDir::new().expect("tempdir"); + let release_dir = tmp.path().join("release"); + let resources_dir = release_dir.join(RESOURCES_DIRNAME); + fs::create_dir_all(&resources_dir).expect("create resources dir"); + let exe = release_dir.join("codex.exe"); + let sibling_helper = release_dir.join("codex-command-runner.exe"); + let resource_helper = resources_dir.join("codex-command-runner.exe"); + fs::write(&exe, b"codex").expect("write exe"); + fs::write(&sibling_helper, b"sibling runner").expect("write sibling helper"); + fs::write(&resource_helper, b"resource runner").expect("write resource helper"); + + let resolved = + source_path_for_exe(&exe, /*file_name*/ "codex-command-runner.exe").expect("helper path"); + + assert_eq!(resolved, sibling_helper); + } } diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index e0a5a063f9..4effc0b70d 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -589,6 +589,16 @@ fn find_setup_exe() -> PathBuf { if candidate.exists() { return candidate; } + + // Standalone installs keep Windows helper binaries under + // `codex-resources/` next to `codex.exe`, so elevation needs to probe + // that sibling folder before falling back to PATH. + let resource_candidate = dir + .join("codex-resources") + .join("codex-windows-sandbox-setup.exe"); + if resource_candidate.exists() { + return resource_candidate; + } } PathBuf::from("codex-windows-sandbox-setup.exe") } diff --git a/scripts/install/install.ps1 b/scripts/install/install.ps1 index 40328db700..ed4a3e1a32 100644 --- a/scripts/install/install.ps1 +++ b/scripts/install/install.ps1 @@ -1,6 +1,5 @@ param( - [Parameter(Position=0)] - [string]$Version = "latest" + [string]$Release = "latest" ) Set-StrictMode -Version Latest @@ -15,6 +14,27 @@ function Write-Step { Write-Host "==> $Message" } +function Write-WarningStep { + param( + [string]$Message + ) + + Write-Warning $Message +} + +function Prompt-YesNo { + param( + [string]$Prompt + ) + + if ([Console]::IsInputRedirected -or [Console]::IsOutputRedirected) { + return $false + } + + $choice = Read-Host "$Prompt [y/N]" + return $choice -match "^(?i:y(?:es)?)$" +} + function Normalize-Version { param( [string]$RawVersion @@ -35,13 +55,39 @@ function Normalize-Version { return $RawVersion } -function Get-ReleaseUrl { +function Get-ReleaseAssetMetadata { param( [string]$AssetName, [string]$ResolvedVersion ) - return "https://github.com/openai/codex/releases/download/rust-v$ResolvedVersion/$AssetName" + $release = Invoke-RestMethod -Uri "https://api.github.com/repos/openai/codex/releases/tags/rust-v$ResolvedVersion" + $asset = $release.assets | Where-Object { $_.name -eq $AssetName } | Select-Object -First 1 + if ($null -eq $asset) { + throw "Could not find release asset $AssetName for Codex $ResolvedVersion." + } + + $digestMatch = [regex]::Match([string]$asset.digest, "^sha256:([0-9a-fA-F]{64})$") + if (-not $digestMatch.Success) { + throw "Could not find SHA-256 digest for release asset $AssetName." + } + + return [PSCustomObject]@{ + Url = $asset.browser_download_url + Sha256 = $digestMatch.Groups[1].Value.ToLowerInvariant() + } +} + +function Test-ArchiveDigest { + param( + [string]$ArchivePath, + [string]$ExpectedDigest + ) + + $actualDigest = (Get-FileHash -LiteralPath $ArchivePath -Algorithm SHA256).Hash.ToLowerInvariant() + if ($actualDigest -ne $ExpectedDigest) { + throw "Downloaded Codex archive checksum did not match release metadata. Expected $ExpectedDigest but got $actualDigest." + } } function Path-Contains { @@ -64,8 +110,46 @@ function Path-Contains { return $false } +function Invoke-WithInstallLock { + param( + [string]$LockPath, + [scriptblock]$Script + ) + + New-Item -ItemType Directory -Force -Path (Split-Path -Parent $LockPath) | Out-Null + $lock = $null + while ($null -eq $lock) { + try { + $lock = [System.IO.File]::Open( + $LockPath, + [System.IO.FileMode]::OpenOrCreate, + [System.IO.FileAccess]::ReadWrite, + [System.IO.FileShare]::None + ) + } catch [System.IO.IOException] { + Start-Sleep -Milliseconds 250 + } + } + try { + & $Script + } finally { + $lock.Dispose() + } +} + +function Remove-StaleInstallArtifacts { + param( + [string]$ReleasesDir + ) + + if (Test-Path -LiteralPath $ReleasesDir -PathType Container) { + Get-ChildItem -LiteralPath $ReleasesDir -Force -Directory -Filter ".staging.*" -ErrorAction SilentlyContinue | + Remove-Item -Recurse -Force -ErrorAction SilentlyContinue + } +} + function Resolve-Version { - $normalizedVersion = Normalize-Version -RawVersion $Version + $normalizedVersion = Normalize-Version -RawVersion $Release if ($normalizedVersion -ne "latest") { return $normalizedVersion } @@ -79,6 +163,414 @@ function Resolve-Version { return (Normalize-Version -RawVersion $release.tag_name) } +function Get-VersionFromBinary { + param( + [string]$CodexPath + ) + + if (-not (Test-Path -LiteralPath $CodexPath -PathType Leaf)) { + return $null + } + + try { + $versionOutput = & $CodexPath --version 2>$null + } catch { + return $null + } + + if ($versionOutput -match '([0-9][0-9A-Za-z.+-]*)$') { + return $matches[1] + } + + return $null +} + +function Get-CurrentInstalledVersion { + param( + [string]$StandaloneCurrentDir + ) + + $standaloneVersion = Get-VersionFromBinary -CodexPath (Join-Path $StandaloneCurrentDir "codex.exe") + if (-not [string]::IsNullOrWhiteSpace($standaloneVersion)) { + return $standaloneVersion + } + + return $null +} + +function Test-OldStandaloneBinLayout { + param( + [string]$VisibleBinDir, + [string]$DefaultVisibleBinDir + ) + + if (-not $VisibleBinDir.Equals($DefaultVisibleBinDir, [System.StringComparison]::OrdinalIgnoreCase)) { + return $false + } + if (-not (Test-Path -LiteralPath $VisibleBinDir -PathType Container)) { + return $false + } + + $item = Get-Item -LiteralPath $VisibleBinDir -Force + if ($item.Attributes -band [IO.FileAttributes]::ReparsePoint) { + return $false + } + + $requiredFiles = @("codex.exe", "rg.exe") + foreach ($fileName in $requiredFiles) { + if (-not (Test-Path -LiteralPath (Join-Path $VisibleBinDir $fileName) -PathType Leaf)) { + return $false + } + } + + $knownFiles = @( + "codex.exe", + "rg.exe", + "codex-command-runner.exe", + "codex-windows-sandbox.exe", + "codex-windows-sandbox-setup.exe" + ) + foreach ($child in Get-ChildItem -LiteralPath $VisibleBinDir -Force) { + if ($child.PSIsContainer) { + return $false + } + if ($knownFiles -notcontains $child.Name) { + return $false + } + } + + return $true +} + +function Move-OldStandaloneBinIfApproved { + param( + [string]$VisibleBinDir, + [string]$DefaultVisibleBinDir + ) + + if (-not (Test-OldStandaloneBinLayout -VisibleBinDir $VisibleBinDir -DefaultVisibleBinDir $DefaultVisibleBinDir)) { + return $null + } + + Write-Step "We found an older Codex install at $VisibleBinDir" + Write-WarningStep "To continue, Codex needs to update the install at this path." + if (-not (Prompt-YesNo "Replace it with the current Codex setup now?")) { + throw "Cannot replace older standalone install without confirmation: $VisibleBinDir" + } + + $backupDir = "$VisibleBinDir.backup.$([DateTimeOffset]::UtcNow.ToUnixTimeSeconds()).$PID" + Write-Step "Moving older standalone install to $backupDir" + Move-Item -LiteralPath $VisibleBinDir -Destination $backupDir + return $backupDir +} + +function Add-JunctionSupportType { + if (([System.Management.Automation.PSTypeName]'CodexInstaller.Junction').Type) { + return + } + + Add-Type -TypeDefinition @" +using System; +using System.ComponentModel; +using System.IO; +using System.Runtime.InteropServices; +using System.Text; +using Microsoft.Win32.SafeHandles; + +namespace CodexInstaller +{ + public static class Junction + { + private const uint GENERIC_WRITE = 0x40000000; + private const uint FILE_SHARE_READ = 0x00000001; + private const uint FILE_SHARE_WRITE = 0x00000002; + private const uint FILE_SHARE_DELETE = 0x00000004; + private const uint OPEN_EXISTING = 3; + private const uint FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; + private const uint FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000; + private const uint FSCTL_SET_REPARSE_POINT = 0x000900A4; + private const uint IO_REPARSE_TAG_MOUNT_POINT = 0xA0000003; + private const int HeaderLength = 20; + + [DllImport("kernel32.dll", CharSet = CharSet.Unicode, SetLastError = true)] + private static extern SafeFileHandle CreateFileW( + string lpFileName, + uint dwDesiredAccess, + uint dwShareMode, + IntPtr lpSecurityAttributes, + uint dwCreationDisposition, + uint dwFlagsAndAttributes, + IntPtr hTemplateFile); + + [DllImport("kernel32.dll", SetLastError = true)] + private static extern bool DeviceIoControl( + SafeFileHandle hDevice, + uint dwIoControlCode, + byte[] lpInBuffer, + int nInBufferSize, + IntPtr lpOutBuffer, + int nOutBufferSize, + out int lpBytesReturned, + IntPtr lpOverlapped); + + public static void SetTarget(string linkPath, string targetPath) + { + string substituteName = "\\??\\" + Path.GetFullPath(targetPath); + byte[] substituteNameBytes = Encoding.Unicode.GetBytes(substituteName); + if (substituteNameBytes.Length > ushort.MaxValue - HeaderLength) { + throw new ArgumentException("Junction target path is too long.", "targetPath"); + } + + byte[] reparseBuffer = new byte[substituteNameBytes.Length + HeaderLength]; + WriteUInt32(reparseBuffer, 0, IO_REPARSE_TAG_MOUNT_POINT); + WriteUInt16(reparseBuffer, 4, checked((ushort)(substituteNameBytes.Length + 12))); + WriteUInt16(reparseBuffer, 8, 0); + WriteUInt16(reparseBuffer, 10, checked((ushort)substituteNameBytes.Length)); + WriteUInt16(reparseBuffer, 12, checked((ushort)(substituteNameBytes.Length + 2))); + WriteUInt16(reparseBuffer, 14, 0); + Buffer.BlockCopy(substituteNameBytes, 0, reparseBuffer, 16, substituteNameBytes.Length); + + using (SafeFileHandle handle = CreateFileW( + linkPath, + GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + IntPtr.Zero, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + IntPtr.Zero)) + { + if (handle.IsInvalid) { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + int bytesReturned; + if (!DeviceIoControl( + handle, + FSCTL_SET_REPARSE_POINT, + reparseBuffer, + reparseBuffer.Length, + IntPtr.Zero, + 0, + out bytesReturned, + IntPtr.Zero)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + } + + private static void WriteUInt16(byte[] buffer, int offset, ushort value) + { + buffer[offset] = (byte)value; + buffer[offset + 1] = (byte)(value >> 8); + } + + private static void WriteUInt32(byte[] buffer, int offset, uint value) + { + buffer[offset] = (byte)value; + buffer[offset + 1] = (byte)(value >> 8); + buffer[offset + 2] = (byte)(value >> 16); + buffer[offset + 3] = (byte)(value >> 24); + } + } +} +"@ +} + +function Set-JunctionTarget { + param( + [string]$LinkPath, + [string]$TargetPath + ) + + Add-JunctionSupportType + [CodexInstaller.Junction]::SetTarget($LinkPath, $TargetPath) +} + +function Test-IsJunction { + param( + [string]$Path + ) + + if (-not (Test-Path -LiteralPath $Path)) { + return $false + } + + $item = Get-Item -LiteralPath $Path -Force + return ($item.Attributes -band [IO.FileAttributes]::ReparsePoint) -and $item.LinkType -eq "Junction" +} + +function Ensure-Junction { + param( + [string]$LinkPath, + [string]$TargetPath, + [string]$InstallerOwnedTargetPrefix + ) + + if (-not (Test-Path -LiteralPath $LinkPath)) { + New-Item -ItemType Junction -Path $LinkPath -Target $TargetPath | Out-Null + return + } + + $item = Get-Item -LiteralPath $LinkPath -Force + if (Test-IsJunction -Path $LinkPath) { + $existingTarget = [string]$item.Target + if (-not [string]::IsNullOrWhiteSpace($InstallerOwnedTargetPrefix)) { + $ownedTargetPrefix = $InstallerOwnedTargetPrefix.TrimEnd("\\") + if (-not $existingTarget.StartsWith($ownedTargetPrefix, [System.StringComparison]::OrdinalIgnoreCase)) { + throw "Refusing to retarget junction at $LinkPath because it is not managed by this installer." + } + } + if ($existingTarget.Equals($TargetPath, [System.StringComparison]::OrdinalIgnoreCase)) { + return + } + + # Keep the path itself in place and only retarget the junction. That + # avoids a gap where current or the visible bin path disappears during + # an update. + Set-JunctionTarget -LinkPath $LinkPath -TargetPath $TargetPath + return + } + + if ($item.Attributes -band [IO.FileAttributes]::ReparsePoint) { + throw "Refusing to replace non-junction reparse point at $LinkPath." + } + + if ($item.PSIsContainer) { + if ((Get-ChildItem -LiteralPath $LinkPath -Force | Select-Object -First 1) -ne $null) { + throw "Refusing to replace non-empty directory at $LinkPath with a junction." + } + + Remove-Item -LiteralPath $LinkPath -Force + New-Item -ItemType Junction -Path $LinkPath -Target $TargetPath | Out-Null + return + } + + throw "Refusing to replace file at $LinkPath with a junction." +} + +function Test-ReleaseIsComplete { + param( + [string]$ReleaseDir, + [string]$ExpectedVersion, + [string]$ExpectedTarget + ) + + if (-not (Test-Path -LiteralPath $ReleaseDir -PathType Container)) { + return $false + } + + $expectedFiles = @( + "codex.exe", + "codex-resources\codex-command-runner.exe", + "codex-resources\codex-windows-sandbox-setup.exe", + "codex-resources\rg.exe" + ) + foreach ($name in $expectedFiles) { + if (-not (Test-Path -LiteralPath (Join-Path $ReleaseDir $name) -PathType Leaf)) { + return $false + } + } + + return (Split-Path -Leaf $ReleaseDir) -eq "$ExpectedVersion-$ExpectedTarget" +} + +function Get-ExistingCodexCommand { + $existing = Get-Command codex -ErrorAction SilentlyContinue + if ($null -eq $existing) { + return $null + } + + return $existing.Source +} + +function Get-ExistingCodexManager { + param( + [string]$ExistingPath, + [string]$VisibleBinDir + ) + + if ([string]::IsNullOrWhiteSpace($ExistingPath)) { + return $null + } + + if ($ExistingPath.StartsWith($VisibleBinDir, [System.StringComparison]::OrdinalIgnoreCase)) { + return $null + } + + if ($ExistingPath -match "\\.bun\\") { + return "bun" + } + + if ($ExistingPath -match "node_modules" -or $ExistingPath -match "\\npm\\") { + return "npm" + } + + return $null +} + +function Get-ConflictingInstall { + param( + [string]$VisibleBinDir + ) + + $existingPath = Get-ExistingCodexCommand + $manager = Get-ExistingCodexManager -ExistingPath $existingPath -VisibleBinDir $VisibleBinDir + if ($null -eq $manager) { + return $null + } + + Write-Step "Detected existing $manager-managed Codex at $existingPath" + Write-WarningStep "Multiple managed Codex installs can be ambiguous because PATH order decides which one runs." + + return [PSCustomObject]@{ + Manager = $manager + Path = $existingPath + } +} + +function Maybe-HandleConflictingInstall { + param( + [object]$Conflict + ) + + if ($null -eq $Conflict) { + return + } + + $manager = $Conflict.Manager + + $uninstallArgs = if ($manager -eq "bun") { + @("remove", "-g", "@openai/codex") + } else { + @("uninstall", "-g", "@openai/codex") + } + $uninstallCommand = if ($manager -eq "bun") { "bun" } else { "npm" } + + if (Prompt-YesNo "Uninstall the existing $manager-managed Codex now?") { + Write-Step "Running: $uninstallCommand $($uninstallArgs -join ' ')" + try { + & $uninstallCommand @uninstallArgs + } catch { + Write-WarningStep "Failed to uninstall the existing $manager-managed Codex. Continuing with the standalone install." + } + } else { + Write-WarningStep "Leaving the existing $manager-managed Codex installed. PATH order will determine which codex runs." + } +} + +function Test-VisibleCodexCommand { + param( + [string]$VisibleBinDir + ) + + $codexCommand = Join-Path $VisibleBinDir "codex.exe" + & $codexCommand --version *> $null + if ($LASTEXITCODE -ne 0) { + throw "Installed Codex command failed verification: $codexCommand --version" + } +} + if ($env:OS -ne "Windows_NT") { Write-Error "install.ps1 supports Windows only. Use install.sh on macOS or Linux." exit 1 @@ -110,87 +602,149 @@ switch ($architecture) { } } -if ([string]::IsNullOrWhiteSpace($env:CODEX_INSTALL_DIR)) { - $installDir = Join-Path $env:LOCALAPPDATA "Programs\OpenAI\Codex\bin" +$codexHome = if ([string]::IsNullOrWhiteSpace($env:CODEX_HOME)) { + Join-Path $env:USERPROFILE ".codex" } else { - $installDir = $env:CODEX_INSTALL_DIR + $env:CODEX_HOME +} +$standaloneRoot = Join-Path $codexHome "packages\standalone" +$releasesDir = Join-Path $standaloneRoot "releases" +$currentDir = Join-Path $standaloneRoot "current" +$lockPath = Join-Path $standaloneRoot "install.lock" + +$defaultVisibleBinDir = Join-Path $env:LOCALAPPDATA "Programs\OpenAI\Codex\bin" +if ([string]::IsNullOrWhiteSpace($env:CODEX_INSTALL_DIR)) { + $visibleBinDir = $defaultVisibleBinDir +} else { + $visibleBinDir = $env:CODEX_INSTALL_DIR } -$codexPath = Join-Path $installDir "codex.exe" -$installMode = if (Test-Path $codexPath) { "Updating" } else { "Installing" } - -Write-Step "$installMode Codex CLI" -Write-Step "Detected platform: $platformLabel" - -New-Item -ItemType Directory -Force -Path $installDir | Out-Null - +$currentVersion = Get-CurrentInstalledVersion -StandaloneCurrentDir $currentDir $resolvedVersion = Resolve-Version -Write-Step "Resolved version: $resolvedVersion" -$packageAsset = "codex-npm-$npmTag-$resolvedVersion.tgz" +$releaseName = "$resolvedVersion-$target" +$releaseDir = Join-Path $releasesDir $releaseName +if (-not [string]::IsNullOrWhiteSpace($currentVersion) -and $currentVersion -ne $resolvedVersion) { + Write-Step "Updating Codex CLI from $currentVersion to $resolvedVersion" +} elseif (-not [string]::IsNullOrWhiteSpace($currentVersion)) { + Write-Step "Updating Codex CLI" +} else { + Write-Step "Installing Codex CLI" +} +Write-Step "Detected platform: $platformLabel" +Write-Step "Resolved version: $resolvedVersion" + +$conflictingInstall = Get-ConflictingInstall -VisibleBinDir $visibleBinDir +$oldStandaloneBackup = $null + +$packageAsset = "codex-npm-$npmTag-$resolvedVersion.tgz" $tempDir = Join-Path ([System.IO.Path]::GetTempPath()) ("codex-install-" + [System.Guid]::NewGuid().ToString("N")) New-Item -ItemType Directory -Force -Path $tempDir | Out-Null try { - $archivePath = Join-Path $tempDir $packageAsset - $extractDir = Join-Path $tempDir "extract" - $url = Get-ReleaseUrl -AssetName $packageAsset -ResolvedVersion $resolvedVersion + Invoke-WithInstallLock -LockPath $lockPath -Script { + Remove-StaleInstallArtifacts -ReleasesDir $releasesDir - Write-Step "Downloading Codex CLI" - Invoke-WebRequest -Uri $url -OutFile $archivePath + if (-not (Test-ReleaseIsComplete -ReleaseDir $releaseDir -ExpectedVersion $resolvedVersion -ExpectedTarget $target)) { + if (Test-Path -LiteralPath $releaseDir) { + Write-WarningStep "Found incomplete existing release at $releaseDir. Reinstalling." + } - New-Item -ItemType Directory -Force -Path $extractDir | Out-Null - tar -xzf $archivePath -C $extractDir + $archivePath = Join-Path $tempDir $packageAsset + $extractDir = Join-Path $tempDir "extract" + $stagingDir = Join-Path $releasesDir ".staging.$releaseName.$PID" + $assetMetadata = Get-ReleaseAssetMetadata -AssetName $packageAsset -ResolvedVersion $resolvedVersion - $vendorRoot = Join-Path $extractDir "package/vendor/$target" - Write-Step "Installing to $installDir" - $copyMap = @{ - "codex/codex.exe" = "codex.exe" - "codex/codex-command-runner.exe" = "codex-command-runner.exe" - "codex/codex-windows-sandbox-setup.exe" = "codex-windows-sandbox-setup.exe" - "path/rg.exe" = "rg.exe" - } + Write-Step "Downloading Codex CLI" + Invoke-WebRequest -Uri $assetMetadata.Url -OutFile $archivePath + Test-ArchiveDigest -ArchivePath $archivePath -ExpectedDigest $assetMetadata.Sha256 - foreach ($relativeSource in $copyMap.Keys) { - $sourcePath = Join-Path $vendorRoot $relativeSource - $destinationPath = Join-Path $installDir $copyMap[$relativeSource] - Move-Item -Force $sourcePath $destinationPath + New-Item -ItemType Directory -Force -Path $extractDir | Out-Null + New-Item -ItemType Directory -Force -Path $releasesDir | Out-Null + if (Test-Path -LiteralPath $stagingDir) { + Remove-Item -LiteralPath $stagingDir -Recurse -Force + } + New-Item -ItemType Directory -Force -Path $stagingDir | Out-Null + tar -xzf $archivePath -C $extractDir + + $vendorRoot = Join-Path $extractDir "package/vendor/$target" + $resourcesDir = Join-Path $stagingDir "codex-resources" + New-Item -ItemType Directory -Force -Path $resourcesDir | Out-Null + $copyMap = @{ + "codex/codex.exe" = "codex.exe" + "codex/codex-command-runner.exe" = "codex-resources\codex-command-runner.exe" + "codex/codex-windows-sandbox-setup.exe" = "codex-resources\codex-windows-sandbox-setup.exe" + "path/rg.exe" = "codex-resources\rg.exe" + } + + foreach ($relativeSource in $copyMap.Keys) { + Copy-Item -LiteralPath (Join-Path $vendorRoot $relativeSource) -Destination (Join-Path $stagingDir $copyMap[$relativeSource]) + } + + if (Test-Path -LiteralPath $releaseDir) { + Remove-Item -LiteralPath $releaseDir -Recurse -Force + } + Move-Item -LiteralPath $stagingDir -Destination $releaseDir + } + + New-Item -ItemType Directory -Force -Path $standaloneRoot | Out-Null + Ensure-Junction -LinkPath $currentDir -TargetPath $releaseDir -InstallerOwnedTargetPrefix $releasesDir + + $visibleParent = Split-Path -Parent $visibleBinDir + New-Item -ItemType Directory -Force -Path $visibleParent | Out-Null + $oldStandaloneBackup = Move-OldStandaloneBinIfApproved -VisibleBinDir $visibleBinDir -DefaultVisibleBinDir $defaultVisibleBinDir + try { + Ensure-Junction -LinkPath $visibleBinDir -TargetPath $currentDir -InstallerOwnedTargetPrefix $standaloneRoot + Test-VisibleCodexCommand -VisibleBinDir $visibleBinDir + } catch { + if ($null -ne $oldStandaloneBackup -and (Test-Path -LiteralPath $oldStandaloneBackup)) { + if (Test-Path -LiteralPath $visibleBinDir) { + Remove-Item -LiteralPath $visibleBinDir -Recurse -Force + } + Move-Item -LiteralPath $oldStandaloneBackup -Destination $visibleBinDir + } + throw + } + if ($null -ne $oldStandaloneBackup) { + Remove-Item -LiteralPath $oldStandaloneBackup -Recurse -Force + } } } finally { Remove-Item -Recurse -Force $tempDir -ErrorAction SilentlyContinue } +Maybe-HandleConflictingInstall -Conflict $conflictingInstall + $userPath = [Environment]::GetEnvironmentVariable("Path", "User") -$pathNeedsNewShell = $false -if (-not (Path-Contains -PathValue $userPath -Entry $installDir)) { +if (-not (Path-Contains -PathValue $userPath -Entry $visibleBinDir)) { if ([string]::IsNullOrWhiteSpace($userPath)) { - $newUserPath = $installDir + $newUserPath = $visibleBinDir } else { - $newUserPath = "$installDir;$userPath" + $newUserPath = "$visibleBinDir;$userPath" } [Environment]::SetEnvironmentVariable("Path", $newUserPath, "User") - if (-not (Path-Contains -PathValue $env:Path -Entry $installDir)) { - if ([string]::IsNullOrWhiteSpace($env:Path)) { - $env:Path = $installDir - } else { - $env:Path = "$installDir;$env:Path" - } - } Write-Step "PATH updated for future PowerShell sessions." - $pathNeedsNewShell = $true -} elseif (Path-Contains -PathValue $env:Path -Entry $installDir) { - Write-Step "$installDir is already on PATH." +} elseif (Path-Contains -PathValue $env:Path -Entry $visibleBinDir) { + Write-Step "$visibleBinDir is already on PATH." } else { Write-Step "PATH is already configured for future PowerShell sessions." - $pathNeedsNewShell = $true } -if ($pathNeedsNewShell) { - Write-Step ('Run now: $env:Path = "{0};$env:Path"; codex' -f $installDir) - Write-Step "Or open a new PowerShell window and run: codex" -} else { - Write-Step "Run: codex" +if (-not (Path-Contains -PathValue $env:Path -Entry $visibleBinDir)) { + if ([string]::IsNullOrWhiteSpace($env:Path)) { + $env:Path = $visibleBinDir + } else { + $env:Path = "$visibleBinDir;$env:Path" + } } +Write-Step "Current PowerShell session: codex" +Write-Step "Future PowerShell windows: open a new PowerShell window and run: codex" Write-Host "Codex CLI $resolvedVersion installed successfully." + +$codexCommand = Join-Path $visibleBinDir "codex.exe" +if (Prompt-YesNo "Start Codex now?") { + Write-Step "Launching Codex" + & $codexCommand +} diff --git a/scripts/install/install.sh b/scripts/install/install.sh index 86e2940d55..8c225e4d3b 100755 --- a/scripts/install/install.sh +++ b/scripts/install/install.sh @@ -2,15 +2,33 @@ set -eu -VERSION="${1:-latest}" -INSTALL_DIR="${CODEX_INSTALL_DIR:-$HOME/.local/bin}" +RELEASE="latest" + +BIN_DIR="${CODEX_INSTALL_DIR:-$HOME/.local/bin}" +BIN_PATH="$BIN_DIR/codex" +CODEX_HOME_DIR="${CODEX_HOME:-$HOME/.codex}" +STANDALONE_ROOT="$CODEX_HOME_DIR/packages/standalone" +RELEASES_DIR="$STANDALONE_ROOT/releases" +CURRENT_LINK="$STANDALONE_ROOT/current" +LOCK_FILE="$STANDALONE_ROOT/install.lock" +LOCK_DIR="$STANDALONE_ROOT/install.lock.d" +LOCK_STALE_AFTER_SECS=600 + path_action="already" path_profile="" +conflict_manager="" +conflict_path="" +lock_kind="" +tmp_dir="" step() { printf '==> %s\n' "$1" } +warn() { + printf 'WARNING: %s\n' "$1" >&2 +} + normalize_version() { case "$1" in "" | latest) @@ -28,6 +46,32 @@ normalize_version() { esac } +parse_args() { + while [ "$#" -gt 0 ]; do + case "$1" in + --release) + if [ "$#" -lt 2 ]; then + echo "--release requires a value." >&2 + exit 1 + fi + RELEASE="$2" + shift + ;; + --help | -h) + cat <&2 + exit 1 + ;; + esac + shift + done +} + download_file() { url="$1" output="$2" @@ -63,40 +107,6 @@ download_text() { exit 1 } -add_to_path() { - path_action="already" - path_profile="" - - case ":$PATH:" in - *":$INSTALL_DIR:"*) - return - ;; - esac - - profile="$HOME/.profile" - case "${SHELL:-}" in - */zsh) - profile="$HOME/.zshrc" - ;; - */bash) - profile="$HOME/.bashrc" - ;; - esac - - path_profile="$profile" - path_line="export PATH=\"$INSTALL_DIR:\$PATH\"" - if [ -f "$profile" ] && grep -F "$path_line" "$profile" >/dev/null 2>&1; then - path_action="configured" - return - fi - - { - printf '\n# Added by Codex installer\n' - printf '%s\n' "$path_line" - } >>"$profile" - path_action="added" -} - release_url_for_asset() { asset="$1" resolved_version="$2" @@ -104,6 +114,92 @@ release_url_for_asset() { printf 'https://github.com/openai/codex/releases/download/rust-v%s/%s\n' "$resolved_version" "$asset" } +release_metadata_url() { + resolved_version="$1" + + printf 'https://api.github.com/repos/openai/codex/releases/tags/rust-v%s\n' "$resolved_version" +} + +release_asset_digest() { + asset="$1" + resolved_version="$2" + release_json="$(download_text "$(release_metadata_url "$resolved_version")")" + + digest="$(printf '%s\n' "$release_json" | awk -v asset="$asset" ' + { + if ($0 ~ "\"name\":[[:space:]]*\"" asset "\"") { + in_asset = 1 + asset_depth = depth + } + + if (in_asset && /"digest":[[:space:]]*"[^"]+"/) { + sub(/^.*"digest":[[:space:]]*"/, "") + sub(/".*$/, "") + digest = $0 + } + + line = $0 + opens = gsub(/\{/, "{", line) + closes = gsub(/\}/, "}", line) + depth += opens - closes + + if (in_asset && depth < asset_depth) { + in_asset = 0 + } + } + END { + if (digest != "") { + print digest + } + } + ')" + + case "$digest" in + sha256:????????????????????????????????????????????????????????????????) + printf '%s\n' "${digest#sha256:}" + ;; + *) + echo "Could not find SHA-256 digest for release asset $asset." >&2 + exit 1 + ;; + esac +} + +file_sha256() { + path="$1" + + if command -v sha256sum >/dev/null 2>&1; then + sha256sum "$path" | awk '{print $1}' + return + fi + + if command -v shasum >/dev/null 2>&1; then + shasum -a 256 "$path" | awk '{print $1}' + return + fi + + if command -v openssl >/dev/null 2>&1; then + openssl dgst -sha256 "$path" | sed 's/^.*= //' + return + fi + + echo "sha256sum, shasum, or openssl is required to verify the Codex download." >&2 + exit 1 +} + +verify_archive_digest() { + archive_path="$1" + expected_digest="$2" + actual_digest="$(file_sha256 "$archive_path")" + + if [ "$actual_digest" != "$expected_digest" ]; then + echo "Downloaded Codex archive checksum did not match release metadata." >&2 + echo "expected: $expected_digest" >&2 + echo "actual: $actual_digest" >&2 + exit 1 + fi +} + require_command() { if ! command -v "$1" >/dev/null 2>&1; then echo "$1 is required to install Codex." >&2 @@ -111,11 +207,8 @@ require_command() { fi } -require_command mktemp -require_command tar - resolve_version() { - normalized_version="$(normalize_version "$VERSION")" + normalized_version="$(normalize_version "$RELEASE")" if [ "$normalized_version" != "latest" ]; then printf '%s\n' "$normalized_version" @@ -133,6 +226,417 @@ resolve_version() { printf '%s\n' "$resolved" } +pick_profile() { + # Use the same shell-specific split Homebrew documents because there is no + # universal startup file across macOS/Linux login and interactive shells. + case "$os:${SHELL:-}" in + darwin:*/zsh) + printf '%s\n' "$HOME/.zprofile" + ;; + darwin:*/bash) + printf '%s\n' "$HOME/.bash_profile" + ;; + linux:*/zsh) + printf '%s\n' "$HOME/.zshrc" + ;; + linux:*/bash) + printf '%s\n' "$HOME/.bashrc" + ;; + *) + printf '%s\n' "$HOME/.profile" + ;; + esac +} + +add_to_path() { + path_action="already" + path_profile="" + + case ":$PATH:" in + *":$BIN_DIR:"*) + return + ;; + esac + + profile="$(pick_profile)" + path_profile="$profile" + begin_marker="# >>> Codex installer >>>" + end_marker="# <<< Codex installer <<<" + path_line="export PATH=\"$BIN_DIR:\$PATH\"" + + if [ -f "$profile" ] && grep -F "$begin_marker" "$profile" >/dev/null 2>&1; then + if grep -F "$path_line" "$profile" >/dev/null 2>&1; then + path_action="configured" + return + fi + + if grep -F "$end_marker" "$profile" >/dev/null 2>&1; then + rewrite_path_block "$profile" "$begin_marker" "$end_marker" "$path_line" + path_action="updated" + return + fi + fi + + append_path_block "$profile" "$begin_marker" "$end_marker" "$path_line" + path_action="added" +} + +append_path_block() { + profile="$1" + begin_marker="$2" + end_marker="$3" + path_line="$4" + + { + printf '\n%s\n' "$begin_marker" + printf '%s\n' "$path_line" + printf '%s\n' "$end_marker" + } >>"$profile" +} + +rewrite_path_block() { + profile="$1" + begin_marker="$2" + end_marker="$3" + path_line="$4" + tmp_profile="$tmp_dir/profile.$$.tmp" + + awk -v begin="$begin_marker" -v end="$end_marker" -v line="$path_line" ' + BEGIN { + in_block = 0 + replaced = 0 + } + $0 == begin { + if (!replaced) { + print begin + print line + print end + replaced = 1 + } + in_block = 1 + next + } + in_block { + if ($0 == end) { + in_block = 0 + } + next + } + { + print + } + END { + if (in_block != 0) { + exit 1 + } + } + ' "$profile" >"$tmp_profile" + mv "$tmp_profile" "$profile" +} + +mkdir_lock_is_stale() { + [ -d "$LOCK_DIR" ] || return 1 + + pid="$(cat "$LOCK_DIR/pid" 2>/dev/null || true)" + started_at="$(cat "$LOCK_DIR/started_at" 2>/dev/null || true)" + now="$(date +%s 2>/dev/null || printf '0')" + + case "$started_at" in + ''|*[!0-9]*) + started_at=0 + ;; + esac + + if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null; then + return 1 + fi + + if [ "$started_at" -eq 0 ] || [ "$now" -eq 0 ]; then + return 0 + fi + + [ $((now - started_at)) -ge "$LOCK_STALE_AFTER_SECS" ] +} + +acquire_install_lock() { + mkdir -p "$STANDALONE_ROOT" + + if [ "$os" = "darwin" ] && command -v lockf >/dev/null 2>&1; then + : >>"$LOCK_FILE" + exec 9<>"$LOCK_FILE" + lockf 9 + lock_kind="lockf" + return + fi + + if command -v flock >/dev/null 2>&1; then + exec 9>"$LOCK_FILE" + flock 9 + lock_kind="flock" + return + fi + + while ! mkdir "$LOCK_DIR" 2>/dev/null; do + if mkdir_lock_is_stale; then + warn "Removing stale installer lock at $LOCK_DIR" + rm -rf "$LOCK_DIR" + continue + fi + sleep 1 + done + + printf '%s\n' "$$" >"$LOCK_DIR/pid" + date +%s >"$LOCK_DIR/started_at" 2>/dev/null || true + lock_kind="mkdir" +} + +release_install_lock() { + if [ "$lock_kind" = "mkdir" ]; then + rm -rf "$LOCK_DIR" 2>/dev/null || true + elif [ "$lock_kind" = "flock" ] || [ "$lock_kind" = "lockf" ]; then + exec 9>&- 2>/dev/null || true + fi + lock_kind="" +} + +cleanup_stale_install_artifacts() { + mkdir -p "$RELEASES_DIR" "$STANDALONE_ROOT" + + find "$RELEASES_DIR" -mindepth 1 -maxdepth 1 -name '.staging.*' -exec rm -rf {} + + find "$STANDALONE_ROOT" -mindepth 1 -maxdepth 1 -name '.current.*' -exec rm -f {} + + + if [ -d "$BIN_DIR" ]; then + find "$BIN_DIR" -mindepth 1 -maxdepth 1 -name '.codex.*' -exec rm -f {} + + fi +} + +replace_path_with_symlink() { + link_path="$1" + link_target="$2" + tmp_link="$3" + + rm -f "$tmp_link" + ln -s "$link_target" "$tmp_link" + + if mv -Tf "$tmp_link" "$link_path" 2>/dev/null; then + return + fi + + if mv -hf "$tmp_link" "$link_path" 2>/dev/null; then + return + fi + + rm -f "$link_path" + mv -f "$tmp_link" "$link_path" +} + +version_from_binary() { + codex_path="$1" + + if [ ! -x "$codex_path" ]; then + return 1 + fi + + "$codex_path" --version 2>/dev/null | sed -n 's/.* \([0-9][0-9A-Za-z.+-]*\)$/\1/p' | head -n 1 +} + +current_installed_version() { + version="$(version_from_binary "$CURRENT_LINK/codex" || true)" + if [ -n "$version" ]; then + printf '%s\n' "$version" + return 0 + fi + + return 0 +} + +resolve_existing_codex() { + command -v codex 2>/dev/null || true +} + +classify_existing_codex() { + existing_path="$1" + + if [ -z "$existing_path" ] || [ "$existing_path" = "$BIN_PATH" ]; then + return 1 + fi + + case "$existing_path" in + /opt/homebrew/* | /usr/local/*) + if [ "$os" = "darwin" ]; then + printf 'brew\n' + return 0 + fi + ;; + esac + + if [ -f "$existing_path" ] && grep -F "#!/usr/bin/env node" "$existing_path" >/dev/null 2>&1; then + case "$existing_path" in + *".bun"*) + printf 'bun\n' + ;; + *) + printf 'npm\n' + ;; + esac + return 0 + fi + + return 1 +} + +prompt_yes_no() { + prompt="$1" + + if ( : /dev/null; then + printf '%s [y/N] ' "$prompt" >/dev/tty + if ! IFS= read -r answer /dev/null +} + +parse_args "$@" + +require_command mktemp +require_command tar + case "$(uname -s)" in Darwin) os="darwin" @@ -187,58 +691,78 @@ else fi fi -if [ -x "$INSTALL_DIR/codex" ]; then - install_mode="Updating" -else - install_mode="Installing" -fi - -step "$install_mode Codex CLI" -step "Detected platform: $platform_label" - resolved_version="$(resolve_version)" asset="codex-npm-$npm_tag-$resolved_version.tgz" download_url="$(release_url_for_asset "$asset" "$resolved_version")" +release_name="$resolved_version-$vendor_target" +release_dir="$RELEASES_DIR/$release_name" +current_version="$(current_installed_version)" +if [ -n "$current_version" ] && [ "$current_version" != "$resolved_version" ]; then + step "Updating Codex CLI from $current_version to $resolved_version" +elif [ -n "$current_version" ]; then + step "Updating Codex CLI" +else + step "Installing Codex CLI" +fi +step "Detected platform: $platform_label" step "Resolved version: $resolved_version" +detect_conflicting_install + tmp_dir="$(mktemp -d)" cleanup() { - rm -rf "$tmp_dir" + release_install_lock + if [ -n "$tmp_dir" ]; then + rm -rf "$tmp_dir" + fi } trap cleanup EXIT INT TERM -archive_path="$tmp_dir/$asset" +acquire_install_lock +cleanup_stale_install_artifacts -step "Downloading Codex CLI" -download_file "$download_url" "$archive_path" +if ! release_dir_is_complete "$release_dir" "$resolved_version" "$vendor_target"; then + if [ -e "$release_dir" ] || [ -L "$release_dir" ]; then + warn "Found incomplete existing release at $release_dir; reinstalling." + fi -tar -xzf "$archive_path" -C "$tmp_dir" + archive_path="$tmp_dir/$asset" + extract_dir="$tmp_dir/extract" -step "Installing to $INSTALL_DIR" -mkdir -p "$INSTALL_DIR" -cp "$tmp_dir/package/vendor/$vendor_target/codex/codex" "$INSTALL_DIR/codex" -cp "$tmp_dir/package/vendor/$vendor_target/path/rg" "$INSTALL_DIR/rg" -chmod 0755 "$INSTALL_DIR/codex" -chmod 0755 "$INSTALL_DIR/rg" + step "Downloading Codex CLI" + expected_digest="$(release_asset_digest "$asset" "$resolved_version")" + download_file "$download_url" "$archive_path" + verify_archive_digest "$archive_path" "$expected_digest" + mkdir -p "$extract_dir" + tar -xzf "$archive_path" -C "$extract_dir" + + step "Installing standalone package to $release_dir" + install_release "$release_dir" "$extract_dir/package/vendor/$vendor_target" +fi +update_current_link "$release_dir" +update_visible_command add_to_path +verify_visible_command +release_install_lock +handle_conflicting_install case "$path_action" in added) - step "PATH updated for future shells in $path_profile" - step "Run now: export PATH=\"$INSTALL_DIR:\$PATH\" && codex" - step "Or open a new terminal and run: codex" + print_launch_instructions + ;; + updated) + print_launch_instructions ;; configured) - step "PATH is already configured for future shells in $path_profile" - step "Run now: export PATH=\"$INSTALL_DIR:\$PATH\" && codex" - step "Or open a new terminal and run: codex" + print_launch_instructions ;; *) - step "$INSTALL_DIR is already on PATH" - step "Run: codex" + step "$BIN_DIR is already on PATH" + print_launch_instructions ;; esac printf 'Codex CLI %s installed successfully.\n' "$resolved_version" +maybe_launch_codex_now From 28b76d13fe1a47b25b49354ee05741ddadb3ac84 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Wed, 15 Apr 2026 14:48:05 -0700 Subject: [PATCH 4/4] [mcp] Add dummy tools for previously called but currently missing tools. (#17853) - [x] Add dummy tools for previously called but currently missing tools. Currently supporting MCP tools only. --- codex-rs/core/config.schema.json | 6 + codex-rs/core/src/codex.rs | 23 ++- codex-rs/core/src/codex_tests.rs | 2 + codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/tools/code_mode/mod.rs | 1 + codex-rs/core/src/tools/handlers/mod.rs | 3 + .../src/tools/handlers/unavailable_tool.rs | 44 +++++ codex-rs/core/src/tools/js_repl/mod.rs | 1 + codex-rs/core/src/tools/router.rs | 3 + codex-rs/core/src/tools/router_tests.rs | 5 + codex-rs/core/src/tools/spec.rs | 43 +++++ codex-rs/core/src/tools/spec_tests.rs | 69 ++++++++ codex-rs/core/src/unavailable_tool.rs | 92 ++++++++++ codex-rs/core/tests/suite/tools.rs | 166 ++++++++++++++++++ codex-rs/features/src/lib.rs | 8 + codex-rs/features/src/tests.rs | 9 + 16 files changed, 473 insertions(+), 3 deletions(-) create mode 100644 codex-rs/core/src/tools/handlers/unavailable_tool.rs create mode 100644 codex-rs/core/src/unavailable_tool.rs diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 9280895b5e..c61daa8b56 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -500,6 +500,9 @@ "tui_app_server": { "type": "boolean" }, + "unavailable_dummy_tools": { + "type": "boolean" + }, "undo": { "type": "boolean" }, @@ -2360,6 +2363,9 @@ "tui_app_server": { "type": "boolean" }, + "unavailable_dummy_tools": { + "type": "boolean" + }, "undo": { "type": "boolean" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cc314a66e9..a3f65b2355 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -44,6 +44,7 @@ use crate::stream_events_utils::last_assistant_message_from_item; use crate::stream_events_utils::raw_assistant_output_text_from_item; use crate::stream_events_utils::record_completed_response_item; use crate::turn_metadata::TurnMetadataState; +use crate::unavailable_tool::collect_unavailable_called_tools; use crate::util::error_or_panic; use async_channel::Receiver; use async_channel::Sender; @@ -7229,7 +7230,22 @@ pub(crate) async fn built_tools( &turn_context.config, &turn_context.tools_config, ); - let direct_mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.direct_tools); + let mcp_tools = has_mcp_servers.then_some(mcp_tool_exposure.direct_tools); + let deferred_mcp_tools = mcp_tool_exposure.deferred_tools; + let unavailable_called_tools = if turn_context + .config + .features + .enabled(Feature::UnavailableDummyTools) + { + let exposed_tool_names = mcp_tools + .iter() + .chain(deferred_mcp_tools.iter()) + .flat_map(|tools| tools.keys().map(String::as_str)) + .collect::>(); + collect_unavailable_called_tools(input, &exposed_tool_names) + } else { + Vec::new() + }; let parallel_mcp_server_names = turn_context .config @@ -7246,8 +7262,9 @@ pub(crate) async fn built_tools( Ok(Arc::new(ToolRouter::from_config( &turn_context.tools_config, ToolRouterParams { - mcp_tools: direct_mcp_tools, - deferred_mcp_tools: mcp_tool_exposure.deferred_tools, + mcp_tools, + deferred_mcp_tools, + unavailable_called_tools, parallel_mcp_server_names, discoverable_tools, dynamic_tools: turn_context.dynamic_tools.as_slice(), diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index eaa2099098..19c0917f67 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -313,6 +313,7 @@ fn test_tool_runtime(session: Arc, turn_context: Arc) -> T crate::tools::router::ToolRouterParams { mcp_tools: None, deferred_mcp_tools: None, + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, dynamic_tools: turn_context.dynamic_tools.as_slice(), @@ -5451,6 +5452,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { crate::tools::router::ToolRouterParams { deferred_mcp_tools, mcp_tools: Some(tools), + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, dynamic_tools: turn_context.dynamic_tools.as_slice(), diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 8d463f3b51..abfe91a0e1 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -150,6 +150,7 @@ mod tools; pub(crate) mod turn_diff_tracker; mod turn_metadata; mod turn_timing; +mod unavailable_tool; pub use rollout::ARCHIVED_SESSIONS_SUBDIR; pub use rollout::Cursor; pub use rollout::EventPersistenceMode; diff --git a/codex-rs/core/src/tools/code_mode/mod.rs b/codex-rs/core/src/tools/code_mode/mod.rs index 8445fdf645..7d66d56c39 100644 --- a/codex-rs/core/src/tools/code_mode/mod.rs +++ b/codex-rs/core/src/tools/code_mode/mod.rs @@ -287,6 +287,7 @@ async fn build_nested_router(exec: &ExecContext) -> ToolRouter { ToolRouterParams { deferred_mcp_tools: None, mcp_tools: Some(listed_mcp_tools), + unavailable_called_tools: Vec::new(), parallel_mcp_server_names, discoverable_tools: None, dynamic_tools: exec.turn.dynamic_tools.as_slice(), diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 022e02c39e..6b48680b8a 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -15,6 +15,7 @@ mod shell; mod test_sync; mod tool_search; mod tool_suggest; +mod unavailable_tool; pub(crate) mod unified_exec; mod view_image; @@ -49,6 +50,8 @@ pub use shell::ShellHandler; pub use test_sync::TestSyncHandler; pub use tool_search::ToolSearchHandler; pub use tool_suggest::ToolSuggestHandler; +pub use unavailable_tool::UnavailableToolHandler; +pub(crate) use unavailable_tool::unavailable_tool_message; pub use unified_exec::UnifiedExecHandler; pub use view_image::ViewImageHandler; diff --git a/codex-rs/core/src/tools/handlers/unavailable_tool.rs b/codex-rs/core/src/tools/handlers/unavailable_tool.rs new file mode 100644 index 0000000000..eb00cf8ff1 --- /dev/null +++ b/codex-rs/core/src/tools/handlers/unavailable_tool.rs @@ -0,0 +1,44 @@ +use crate::function_tool::FunctionCallError; +use crate::tools::context::FunctionToolOutput; +use crate::tools::context::ToolInvocation; +use crate::tools::context::ToolPayload; +use crate::tools::registry::ToolHandler; +use crate::tools::registry::ToolKind; + +pub struct UnavailableToolHandler; + +pub(crate) fn unavailable_tool_message( + tool_name: impl std::fmt::Display, + next_step: &str, +) -> String { + format!( + "Tool `{tool_name}` is not currently available. It appeared in earlier tool calls in this conversation, but its implementation is not available in the current request. {next_step}" + ) +} + +impl ToolHandler for UnavailableToolHandler { + type Output = FunctionToolOutput; + + fn kind(&self) -> ToolKind { + ToolKind::Function + } + + async fn handle(&self, invocation: ToolInvocation) -> Result { + let ToolInvocation { + tool_name, payload, .. + } = invocation; + + match payload { + ToolPayload::Function { .. } => Ok(FunctionToolOutput::from_text( + unavailable_tool_message( + tool_name.display(), + "Retry after the tool becomes available or ask the user to re-enable it.", + ), + Some(false), + )), + _ => Err(FunctionCallError::RespondToModel( + "unavailable tool handler received unsupported payload".to_string(), + )), + } + } +} diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 92bd4c610f..5c0884d9ee 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1568,6 +1568,7 @@ impl JsReplManager { crate::tools::router::ToolRouterParams { deferred_mcp_tools: None, mcp_tools: Some(mcp_tools), + unavailable_called_tools: Vec::new(), // JS REPL dispatches nested tool calls directly, not through // `ToolCallRuntime`'s parallel scheduling lock. parallel_mcp_server_names: std::collections::HashSet::new(), diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 183b9fb416..ef7ae32d4b 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -44,6 +44,7 @@ pub struct ToolRouter { pub(crate) struct ToolRouterParams<'a> { pub(crate) mcp_tools: Option>, pub(crate) deferred_mcp_tools: Option>, + pub(crate) unavailable_called_tools: Vec, pub(crate) parallel_mcp_server_names: HashSet, pub(crate) discoverable_tools: Option>, pub(crate) dynamic_tools: &'a [DynamicToolSpec], @@ -54,6 +55,7 @@ impl ToolRouter { let ToolRouterParams { mcp_tools, deferred_mcp_tools, + unavailable_called_tools, parallel_mcp_server_names, discoverable_tools, dynamic_tools, @@ -62,6 +64,7 @@ impl ToolRouter { config, mcp_tools, deferred_mcp_tools, + unavailable_called_tools, discoverable_tools, dynamic_tools, ); diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index f4c0fb7c5a..d93796ab64 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -33,6 +33,7 @@ async fn js_repl_tools_only_blocks_direct_tool_calls() -> anyhow::Result<()> { ToolRouterParams { deferred_mcp_tools, mcp_tools: Some(mcp_tools), + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), @@ -86,6 +87,7 @@ async fn js_repl_tools_only_allows_js_repl_source_calls() -> anyhow::Result<()> ToolRouterParams { deferred_mcp_tools, mcp_tools: Some(mcp_tools), + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), @@ -132,6 +134,7 @@ async fn js_repl_tools_only_blocks_namespaced_js_repl_tool() -> anyhow::Result<( ToolRouterParams { deferred_mcp_tools: None, mcp_tools: None, + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), @@ -182,6 +185,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow ToolRouterParams { deferred_mcp_tools: None, mcp_tools: Some(mcp_tools), + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::new(), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), @@ -254,6 +258,7 @@ async fn mcp_parallel_support_uses_exact_payload_server() -> anyhow::Result<()> ToolRouterParams { deferred_mcp_tools: None, mcp_tools: None, + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: HashSet::from(["echo".to_string()]), discoverable_tools: None, dynamic_tools: turn.dynamic_tools.as_slice(), diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 9c05e6cc35..1a1a9f2e5f 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -7,8 +7,12 @@ use crate::tools::handlers::multi_agents_common::MIN_WAIT_TIMEOUT_MS; use crate::tools::registry::ToolRegistryBuilder; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; +use codex_tools::AdditionalProperties; use codex_tools::DiscoverableTool; +use codex_tools::JsonSchema; +use codex_tools::ResponsesApiTool; use codex_tools::ToolHandlerKind; +use codex_tools::ToolName; use codex_tools::ToolNamespace; use codex_tools::ToolRegistryPlanDeferredTool; use codex_tools::ToolRegistryPlanMcpTool; @@ -16,8 +20,10 @@ use codex_tools::ToolRegistryPlanParams; use codex_tools::ToolUserShellType; use codex_tools::ToolsConfig; use codex_tools::WaitAgentTimeoutOptions; +use codex_tools::augment_tool_spec_for_code_mode; use codex_tools::build_tool_registry_plan; use std::collections::HashMap; +use std::collections::HashSet; use std::sync::Arc; pub(crate) fn tool_user_shell_type(user_shell: &Shell) -> ToolUserShellType { @@ -66,6 +72,7 @@ pub(crate) fn build_specs_with_discoverable_tools( config: &ToolsConfig, mcp_tools: Option>, deferred_mcp_tools: Option>, + unavailable_called_tools: Vec, discoverable_tools: Option>, dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { @@ -86,6 +93,7 @@ pub(crate) fn build_specs_with_discoverable_tools( use crate::tools::handlers::TestSyncHandler; use crate::tools::handlers::ToolSearchHandler; use crate::tools::handlers::ToolSuggestHandler; + use crate::tools::handlers::UnavailableToolHandler; use crate::tools::handlers::UnifiedExecHandler; use crate::tools::handlers::ViewImageHandler; use crate::tools::handlers::multi_agents::CloseAgentHandler; @@ -99,6 +107,7 @@ pub(crate) fn build_specs_with_discoverable_tools( use crate::tools::handlers::multi_agents_v2::SendMessageHandler as SendMessageHandlerV2; use crate::tools::handlers::multi_agents_v2::SpawnAgentHandler as SpawnAgentHandlerV2; use crate::tools::handlers::multi_agents_v2::WaitAgentHandler as WaitAgentHandlerV2; + use crate::tools::handlers::unavailable_tool_message; let mut builder = ToolRegistryBuilder::new(); let mcp_tool_plan_inputs = mcp_tools.as_ref().map(map_mcp_tools_for_plan); @@ -154,6 +163,12 @@ pub(crate) fn build_specs_with_discoverable_tools( let code_mode_wait_handler = Arc::new(CodeModeWaitHandler); let js_repl_handler = Arc::new(JsReplHandler); let js_repl_reset_handler = Arc::new(JsReplResetHandler); + let unavailable_tool_handler = Arc::new(UnavailableToolHandler); + let mut existing_spec_names = plan + .specs + .iter() + .map(|configured_tool| configured_tool.name().to_string()) + .collect::>(); for spec in plan.specs { if spec.supports_parallel_tool_calls { @@ -278,6 +293,34 @@ pub(crate) fn build_specs_with_discoverable_tools( builder.register_handler(name.clone(), mcp_handler.clone()); } } + + for unavailable_tool in unavailable_called_tools { + let tool_name = unavailable_tool.display(); + if existing_spec_names.insert(tool_name.clone()) { + let spec = codex_tools::ToolSpec::Function(ResponsesApiTool { + name: tool_name.clone(), + description: unavailable_tool_message( + &tool_name, + "Calling this placeholder returns an error explaining that the tool is unavailable.", + ), + strict: false, + parameters: JsonSchema::object( + Default::default(), + /*required*/ None, + Some(AdditionalProperties::Boolean(false)), + ), + output_schema: None, + defer_loading: None, + }); + let spec = if config.code_mode_enabled { + augment_tool_spec_for_code_mode(spec) + } else { + spec + }; + builder.push_spec(spec); + } + builder.register_handler(unavailable_tool, unavailable_tool_handler.clone()); + } builder } diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index ee993b0d53..cab38bc21c 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -16,6 +16,7 @@ use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; +use codex_tools::AdditionalProperties; use codex_tools::ConfiguredToolSpec; use codex_tools::DiscoverableTool; use codex_tools::JsonSchema; @@ -267,11 +268,28 @@ fn build_specs( mcp_tools: Option>, deferred_mcp_tools: Option>, dynamic_tools: &[DynamicToolSpec], +) -> ToolRegistryBuilder { + build_specs_with_unavailable_tools( + config, + mcp_tools, + deferred_mcp_tools, + Vec::new(), + dynamic_tools, + ) +} + +fn build_specs_with_unavailable_tools( + config: &ToolsConfig, + mcp_tools: Option>, + deferred_mcp_tools: Option>, + unavailable_called_tools: Vec, + dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { build_specs_with_discoverable_tools( config, mcp_tools, deferred_mcp_tools, + unavailable_called_tools, /*discoverable_tools*/ None, dynamic_tools, ) @@ -356,6 +374,7 @@ fn assert_model_tools( ToolRouterParams { mcp_tools: None, deferred_mcp_tools: None, + unavailable_called_tools: Vec::new(), parallel_mcp_server_names: std::collections::HashSet::new(), discoverable_tools: None, dynamic_tools: &[], @@ -770,6 +789,7 @@ fn tool_suggest_requires_apps_and_plugins_features() { &tools_config, /*mcp_tools*/ None, /*deferred_mcp_tools*/ None, + Vec::new(), discoverable_tools.clone(), &[], ) @@ -991,6 +1011,55 @@ fn direct_mcp_tools_register_namespaced_handlers() { assert!(!registry.has_handler(&ToolName::plain("mcp__test_server__echo"))); } +#[test] +fn unavailable_mcp_tools_are_exposed_as_dummy_function_tools() { + let config = test_config(); + let model_info = construct_model_info_offline("gpt-5-codex", &config); + let mut features = Features::with_defaults(); + features.enable(Feature::UnifiedExec); + let available_models = Vec::new(); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &available_models, + features: &features, + image_generation_tool_auth_allowed: true, + web_search_mode: Some(WebSearchMode::Cached), + session_source: SessionSource::Cli, + sandbox_policy: &SandboxPolicy::DangerFullAccess, + windows_sandbox_level: WindowsSandboxLevel::Disabled, + }); + + let unavailable_tool = ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"); + let (tools, registry) = build_specs_with_unavailable_tools( + &tools_config, + /*mcp_tools*/ None, + /*deferred_mcp_tools*/ None, + vec![unavailable_tool], + &[], + ) + .build(); + + let tool = find_tool(&tools, "mcp__codex_apps__calendar_create_event"); + let ToolSpec::Function(ResponsesApiTool { + description, + parameters, + .. + }) = &tool.spec + else { + panic!("unavailable MCP tool should be exposed as a function tool"); + }; + assert!(description.contains("not currently available")); + assert_eq!( + parameters.additional_properties, + Some(AdditionalProperties::Boolean(false)) + ); + assert!(registry.has_handler(&ToolName::namespaced( + "mcp__codex_apps__calendar", + "_create_event" + ))); + assert!(!registry.has_handler(&ToolName::plain("mcp__codex_apps__calendar_create_event"))); +} + #[test] fn test_mcp_tool_property_missing_type_defaults_to_string() { let config = test_config(); diff --git a/codex-rs/core/src/unavailable_tool.rs b/codex-rs/core/src/unavailable_tool.rs new file mode 100644 index 0000000000..39ba21f9b8 --- /dev/null +++ b/codex-rs/core/src/unavailable_tool.rs @@ -0,0 +1,92 @@ +use std::collections::BTreeMap; +use std::collections::HashSet; + +use codex_protocol::models::ResponseItem; +use codex_tools::ToolName; + +pub(crate) fn collect_unavailable_called_tools( + input: &[ResponseItem], + exposed_tool_names: &HashSet<&str>, +) -> Vec { + let mut unavailable_tools = BTreeMap::new(); + + for item in input { + let ResponseItem::FunctionCall { + name, namespace, .. + } = item + else { + continue; + }; + if !should_collect_unavailable_tool(name, namespace.as_deref()) { + continue; + } + + let tool_name = match namespace { + Some(namespace) => ToolName::namespaced(namespace.clone(), name.clone()), + None => ToolName::plain(name.clone()), + }; + let display_name = tool_name.display(); + if exposed_tool_names.contains(display_name.as_str()) { + continue; + } + + unavailable_tools + .entry(display_name) + .or_insert_with(|| tool_name); + } + + unavailable_tools.into_values().collect() +} + +fn should_collect_unavailable_tool(name: &str, namespace: Option<&str>) -> bool { + namespace.is_some_and(|namespace| namespace.starts_with("mcp__")) || name.starts_with("mcp__") +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + fn function_call(name: &str, namespace: Option<&str>) -> ResponseItem { + ResponseItem::FunctionCall { + id: None, + name: name.to_string(), + namespace: namespace.map(str::to_string), + arguments: "{}".to_string(), + call_id: format!("call-{name}"), + } + } + + #[test] + fn collect_unavailable_called_tools_detects_mcp_function_calls() { + let input = vec![ + function_call("shell", /*namespace*/ None), + function_call("mcp__server__lookup", /*namespace*/ None), + function_call("_create_event", Some("mcp__codex_apps__calendar")), + ]; + + let tools = collect_unavailable_called_tools(&input, &HashSet::new()); + + assert_eq!( + tools, + vec![ + ToolName::namespaced("mcp__codex_apps__calendar", "_create_event"), + ToolName::plain("mcp__server__lookup"), + ] + ); + } + + #[test] + fn collect_unavailable_called_tools_skips_currently_available_tools() { + let exposed_tool_names = HashSet::from(["mcp__server__lookup", "mcp__server__search"]); + let input = vec![ + function_call("mcp__server__lookup", /*namespace*/ None), + function_call("mcp__server__search", /*namespace*/ None), + function_call("mcp__server__missing", /*namespace*/ None), + ]; + + let tools = collect_unavailable_called_tools(&input, &exposed_tool_names); + + assert_eq!(tools, vec![ToolName::plain("mcp__server__missing")]); + } +} diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 8dc91cc087..3ae2888ec7 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -1,12 +1,16 @@ #![cfg(not(target_os = "windows"))] #![allow(clippy::unwrap_used, clippy::expect_used)] +use std::collections::HashMap; use std::fs; +use std::sync::Arc; use std::time::Duration; use std::time::Instant; use anyhow::Context; use anyhow::Result; +use codex_config::types::McpServerConfig; +use codex_config::types::McpServerTransportConfig; use codex_core::sandboxing::SandboxPermissions; use codex_features::Feature; use codex_protocol::protocol::AskForApproval; @@ -22,10 +26,13 @@ use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; +use core_test_support::stdio_server_bin; use core_test_support::test_codex::test_codex; use regex_lite::Regex; use serde_json::Value; use serde_json::json; +use serial_test::serial; +use tempfile::TempDir; fn tool_names(body: &Value) -> Vec { body.get("tools") @@ -44,6 +51,24 @@ fn tool_names(body: &Value) -> Vec { .unwrap_or_default() } +fn ev_namespaced_function_call( + call_id: &str, + namespace: &str, + name: &str, + arguments: &str, +) -> Value { + json!({ + "type": "response.output_item.done", + "item": { + "type": "function_call", + "call_id": call_id, + "namespace": namespace, + "name": name, + "arguments": arguments, + } + }) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> { skip_if_no_network!(Ok(())); @@ -91,6 +116,147 @@ async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[serial(mcp_test_value)] +async fn historical_unavailable_mcp_call_is_exposed_as_placeholder_tool() -> Result<()> { + skip_if_no_network!(Ok(())); + + let historical_call_id = "historical-mcp-call"; + let retry_call_id = "retry-mcp-call"; + let server_name = "rmcp"; + let unavailable_tool_namespace = "mcp__rmcp__"; + let unavailable_tool_name = "echo"; + let unavailable_tool_display_name = "mcp__rmcp__echo"; + let server = start_mock_server().await; + let rmcp_test_server_bin = match stdio_server_bin() { + Ok(bin) => bin, + Err(err) => { + eprintln!("test_stdio_server binary not available, skipping test: {err}"); + return Ok(()); + } + }; + let codex_home = Arc::new(TempDir::new()?); + let mut builder = test_codex() + .with_model("gpt-5.1") + .with_home(Arc::clone(&codex_home)) + .with_config(move |config| { + config + .features + .enable(Feature::UnavailableDummyTools) + .expect("unavailable dummy tools should be enabled for this test"); + 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::new()), + 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(), + }, + ); + config + .mcp_servers + .set(servers) + .expect("test mcp servers should accept any configuration"); + }); + let test = builder.build(&server).await?; + + let first_turn_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_namespaced_function_call( + historical_call_id, + unavailable_tool_namespace, + unavailable_tool_name, + r#"{"message":"ping"}"#, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "rmcp echo tool completed"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + test.submit_turn("call the rmcp echo tool").await?; + let rollout_path = test.codex.rollout_path().context("rollout path")?; + assert_eq!(first_turn_mock.requests().len(), 2); + drop(test); + + let retry_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-3"), + ev_namespaced_function_call( + retry_call_id, + unavailable_tool_namespace, + unavailable_tool_name, + r#"{"message":"ping again"}"#, + ), + ev_completed("resp-3"), + ]), + sse(vec![ + ev_response_created("resp-4"), + ev_assistant_message("msg-2", "done"), + ev_completed("resp-4"), + ]), + ], + ) + .await; + + let mut resume_builder = test_codex().with_model("gpt-5.1").with_config(|config| { + config + .features + .enable(Feature::UnavailableDummyTools) + .expect("unavailable dummy tools should be enabled for this test"); + }); + let test = resume_builder + .resume(&server, codex_home, rollout_path) + .await?; + + test.submit_turn("retry the rmcp echo tool").await?; + + let requests = retry_mock.requests(); + assert_eq!(requests.len(), 2); + let first_request_tools = tool_names(&requests[0].body_json()); + assert!( + first_request_tools + .iter() + .any(|name| name == unavailable_tool_display_name), + "historical unavailable MCP call should add a placeholder tool; got {first_request_tools:?}" + ); + let output_text = requests[1] + .function_call_output_text(retry_call_id) + .context("placeholder tool output present")?; + assert!(output_text.contains("not currently available")); + assert!( + !output_text.contains("unsupported call"), + "placeholder handler should answer instead of falling back to unsupported call: {output_text}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 4d7a83b628..ce7a196362 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -146,6 +146,8 @@ pub enum Feature { Apps, /// Enable the tool_search tool for apps. ToolSearch, + /// Expose placeholder tools for unavailable historical tool calls. + UnavailableDummyTools, /// Enable discoverable tool suggestions for apps. ToolSuggest, /// Enable plugins. @@ -789,6 +791,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::UnavailableDummyTools, + key: "unavailable_dummy_tools", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::ToolSuggest, key: "tool_suggest", diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index f55578dbd6..85cb01be23 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -133,6 +133,15 @@ fn tool_search_is_under_development_and_disabled_by_default() { assert_eq!(Feature::ToolSearch.default_enabled(), false); } +#[test] +fn unavailable_dummy_tools_is_under_development_and_disabled_by_default() { + assert_eq!( + Feature::UnavailableDummyTools.stage(), + Stage::UnderDevelopment + ); + assert_eq!(Feature::UnavailableDummyTools.default_enabled(), false); +} + #[test] fn general_analytics_is_stable_and_enabled_by_default() { assert_eq!(Feature::GeneralAnalytics.stage(), Stage::Stable);