From 98129fb9c5a1c60118523534aba6f3de89fd0585 Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Fri, 15 May 2026 14:19:06 -0700 Subject: [PATCH 1/6] Disable DMG staging for signed macOS promotion (#22900) ## Why `promote_signed` is now used to finish a release from an externally signed macOS handoff, but this release path (temporarily) no longer distributes DMGs. Keeping DMG staging enabled made the handoff unnecessarily require DMG assets and notarization/stapling validation even though the promoted release only needs the signed macOS binaries. ## What changed - Set every `stage-signed-macos` matrix entry to `build_dmg: "false"`, including the primary macOS bundles. - Kept the existing DMG staging branch in place behind `matrix.build_dmg` so it can be re-enabled deliberately later. - Updated the workflow header comment so the signed handoff contract asks for signed binaries, not signed DMGs. The regular signed build path that creates, signs, notarizes, and stages DMGs is unchanged; this only affects the `promote_signed` handoff path. --- .github/workflows/rust-release.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index 0e13aa366c..b6c293d6cd 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -10,7 +10,7 @@ # archive as a GitHub Release asset, then manually dispatch # `release_mode=promote_signed` with `unsigned_run_id` and `signed_macos_asset`. # The signed handoff archive should contain target or artifact directories such -# as `aarch64-apple-darwin/` with signed binaries and signed DMGs. +# as `aarch64-apple-darwin/` with signed binaries. name: rust-release on: @@ -644,7 +644,7 @@ jobs: bundle: primary artifact_name: aarch64-apple-darwin binaries: "codex codex-responses-api-proxy" - build_dmg: "true" + build_dmg: "false" - target: aarch64-apple-darwin bundle: app-server artifact_name: aarch64-apple-darwin-app-server @@ -654,7 +654,7 @@ jobs: bundle: primary artifact_name: x86_64-apple-darwin binaries: "codex codex-responses-api-proxy" - build_dmg: "true" + build_dmg: "false" - target: x86_64-apple-darwin bundle: app-server artifact_name: x86_64-apple-darwin-app-server @@ -770,6 +770,9 @@ jobs: codesign --verify --strict --verbose=2 "$release_path" done + # DMG staging is disabled for signed promotion because we no longer + # distribute DMGs from this release path. Keep the branch here so the + # handoff can opt back in by flipping matrix.build_dmg if needed. if [[ "${{ matrix.build_dmg }}" == "true" ]]; then dmg_name="codex-${target}.dmg" dmg_source="${source_dir}/${dmg_name}" From 6a331a66eb1d526f9b647b07fadf0c2e2833a3d2 Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Fri, 15 May 2026 14:33:24 -0700 Subject: [PATCH 2/6] feat(app-server): update remote control APIs for better UX (#22877) ## Why To help improve `codex remote-control` CLI UX which I plan to do in a followup, this PR adds `server-name` to the various remote control APIs: - `remoteControl/enable` - `remoteControl/disable` - `remoteControl/status/changed` Also, add a `remoteControl/status/read` API. This will be helpful in the Codex App. --- .../schema/json/ServerNotification.json | 4 + .../codex_app_server_protocol.schemas.json | 4 + .../codex_app_server_protocol.v2.schemas.json | 4 + ...emoteControlStatusChangedNotification.json | 4 + .../RemoteControlStatusChangedNotification.ts | 2 +- .../src/protocol/common.rs | 6 + .../src/protocol/v2/remote_control.rs | 17 ++ .../src/transport/remote_control/enroll.rs | 8 +- .../src/transport/remote_control/mod.rs | 31 ++-- .../src/transport/remote_control/tests.rs | 11 ++ .../src/transport/remote_control/websocket.rs | 38 +++-- codex-rs/app-server/README.md | 3 +- codex-rs/app-server/src/lib.rs | 10 +- codex-rs/app-server/src/message_processor.rs | 4 + .../remote_control_processor.rs | 11 ++ .../app-server/tests/common/mcp_process.rs | 6 + .../tests/suite/v2/remote_control.rs | 146 ++++++++++++++++++ 17 files changed, 276 insertions(+), 33 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index 6af19f89e2..a5fce14a4f 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2748,12 +2748,16 @@ "installationId": { "type": "string" }, + "serverName": { + "type": "string" + }, "status": { "$ref": "#/definitions/RemoteControlConnectionStatus" } }, "required": [ "installationId", + "serverName", "status" ], "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 935ec98e88..d1b318889b 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -13446,12 +13446,16 @@ "installationId": { "type": "string" }, + "serverName": { + "type": "string" + }, "status": { "$ref": "#/definitions/v2/RemoteControlConnectionStatus" } }, "required": [ "installationId", + "serverName", "status" ], "title": "RemoteControlStatusChangedNotification", diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index ccf68b28d8..bb926075ee 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -9995,12 +9995,16 @@ "installationId": { "type": "string" }, + "serverName": { + "type": "string" + }, "status": { "$ref": "#/definitions/RemoteControlConnectionStatus" } }, "required": [ "installationId", + "serverName", "status" ], "title": "RemoteControlStatusChangedNotification", diff --git a/codex-rs/app-server-protocol/schema/json/v2/RemoteControlStatusChangedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RemoteControlStatusChangedNotification.json index 85be3316d7..2f305c7d78 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RemoteControlStatusChangedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RemoteControlStatusChangedNotification.json @@ -22,12 +22,16 @@ "installationId": { "type": "string" }, + "serverName": { + "type": "string" + }, "status": { "$ref": "#/definitions/RemoteControlConnectionStatus" } }, "required": [ "installationId", + "serverName", "status" ], "title": "RemoteControlStatusChangedNotification", diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/RemoteControlStatusChangedNotification.ts b/codex-rs/app-server-protocol/schema/typescript/v2/RemoteControlStatusChangedNotification.ts index 8c63ab9029..403b0e6457 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/RemoteControlStatusChangedNotification.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/RemoteControlStatusChangedNotification.ts @@ -6,4 +6,4 @@ import type { RemoteControlConnectionStatus } from "./RemoteControlConnectionSta /** * Current remote-control connection status and remote identity exposed to clients. */ -export type RemoteControlStatusChangedNotification = { status: RemoteControlConnectionStatus, installationId: string, environmentId: string | null, }; +export type RemoteControlStatusChangedNotification = { status: RemoteControlConnectionStatus, serverName: string, installationId: string, environmentId: string | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index f7e04b3dc5..9681f70c34 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -812,6 +812,12 @@ client_request_definitions! { serialization: global("remote-control"), response: v2::RemoteControlDisableResponse, }, + #[experimental("remoteControl/status/read")] + RemoteControlStatusRead => "remoteControl/status/read" { + params: #[ts(type = "undefined")] #[serde(skip_serializing_if = "Option::is_none")] Option<()>, + serialization: global_shared_read("remote-control"), + response: v2::RemoteControlStatusReadResponse, + }, #[experimental("collaborationMode/list")] /// Lists collaboration mode presets. CollaborationModeList => "collaborationMode/list" { diff --git a/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs b/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs index e89a9d19b8..c8ad617e7d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs @@ -9,6 +9,7 @@ use ts_rs::TS; #[ts(export_to = "v2/")] pub struct RemoteControlStatusChangedNotification { pub status: RemoteControlConnectionStatus, + pub server_name: String, pub installation_id: String, pub environment_id: Option, } @@ -18,6 +19,7 @@ pub struct RemoteControlStatusChangedNotification { #[ts(export_to = "v2/")] pub struct RemoteControlEnableResponse { pub status: RemoteControlConnectionStatus, + pub server_name: String, pub installation_id: String, pub environment_id: Option, } @@ -27,6 +29,17 @@ pub struct RemoteControlEnableResponse { #[ts(export_to = "v2/")] pub struct RemoteControlDisableResponse { pub status: RemoteControlConnectionStatus, + pub server_name: String, + pub installation_id: String, + pub environment_id: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct RemoteControlStatusReadResponse { + pub status: RemoteControlConnectionStatus, + pub server_name: String, pub installation_id: String, pub environment_id: Option, } @@ -45,11 +58,13 @@ impl From for RemoteControlEnableRespons fn from(notification: RemoteControlStatusChangedNotification) -> Self { let RemoteControlStatusChangedNotification { status, + server_name, installation_id, environment_id, } = notification; Self { status, + server_name, installation_id, environment_id, } @@ -60,11 +75,13 @@ impl From for RemoteControlDisableRespon fn from(notification: RemoteControlStatusChangedNotification) -> Self { let RemoteControlStatusChangedNotification { status, + server_name, installation_id, environment_id, } = notification; Self { status, + server_name, installation_id, environment_id, } diff --git a/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs b/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs index 60dfc3d845..9ec6e4d004 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs @@ -6,7 +6,6 @@ use codex_api::SharedAuthProvider; use codex_login::default_client::build_reqwest_client; use codex_state::RemoteControlEnrollmentRecord; use codex_state::StateRuntime; -use gethostname::gethostname; use std::io; use std::io::ErrorKind; use tracing::info; @@ -195,11 +194,11 @@ pub(super) async fn enroll_remote_control_server( remote_control_target: &RemoteControlTarget, auth: &RemoteControlConnectionAuth, installation_id: &str, + server_name: &str, ) -> io::Result { let enroll_url = &remote_control_target.enroll_url; - let server_name = gethostname().to_string_lossy().trim().to_string(); let request = EnrollRemoteServerRequest { - name: server_name.clone(), + name: server_name.to_string(), os: std::env::consts::OS, arch: std::env::consts::ARCH, app_server_version: env!("CARGO_PKG_VERSION"), @@ -255,7 +254,7 @@ pub(super) async fn enroll_remote_control_server( account_id: auth.account_id.clone(), environment_id: enrollment.environment_id, server_id: enrollment.server_id, - server_name, + server_name: server_name.to_string(), }) } @@ -464,6 +463,7 @@ mod tests { account_id: "account_id".to_string(), }, "11111111-1111-4111-8111-111111111111", + "test-server", ) .await .expect_err("invalid response should fail to parse"); diff --git a/codex-rs/app-server-transport/src/transport/remote_control/mod.rs b/codex-rs/app-server-transport/src/transport/remote_control/mod.rs index 8722f4e9ee..ae6d79462c 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/mod.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/mod.rs @@ -19,6 +19,7 @@ use codex_app_server_protocol::RemoteControlConnectionStatus; use codex_app_server_protocol::RemoteControlStatusChangedNotification; use codex_login::AuthManager; use codex_state::StateRuntime; +use gethostname::gethostname; use std::error::Error; use std::fmt; use std::io; @@ -112,15 +113,8 @@ impl RemoteControlHandle { connection_status: RemoteControlConnectionStatus, ) -> RemoteControlStatusChangedNotification { self.status_tx.send_if_modified(|status| { - let next_status = RemoteControlStatusChangedNotification { - status: connection_status, - installation_id: status.installation_id.clone(), - environment_id: if connection_status == RemoteControlConnectionStatus::Disabled { - None - } else { - status.environment_id.clone() - }, - }; + let next_status = + remote_control_status_with_connection_status(status, connection_status); if *status == next_status { return false; } @@ -132,6 +126,22 @@ impl RemoteControlHandle { } } +fn remote_control_status_with_connection_status( + status: &RemoteControlStatusChangedNotification, + connection_status: RemoteControlConnectionStatus, +) -> RemoteControlStatusChangedNotification { + RemoteControlStatusChangedNotification { + status: connection_status, + server_name: status.server_name.clone(), + installation_id: status.installation_id.clone(), + environment_id: if connection_status == RemoteControlConnectionStatus::Disabled { + None + } else { + status.environment_id.clone() + }, + } +} + pub async fn start_remote_control( config: RemoteControlStartConfig, state_db: Option>, @@ -154,12 +164,14 @@ pub async fn start_remote_control( }; let (enabled_tx, enabled_rx) = watch::channel(initial_enabled); + let server_name = gethostname().to_string_lossy().trim().to_string(); let initial_status = RemoteControlStatusChangedNotification { status: if initial_enabled { RemoteControlConnectionStatus::Connecting } else { RemoteControlConnectionStatus::Disabled }, + server_name: server_name.clone(), installation_id: config.installation_id.clone(), environment_id: None, }; @@ -171,6 +183,7 @@ pub async fn start_remote_control( remote_control_url: config.remote_control_url, installation_id: config.installation_id, remote_control_target, + server_name, }, state_db, auth_manager, diff --git a/codex-rs/app-server-transport/src/transport/remote_control/tests.rs b/codex-rs/app-server-transport/src/transport/remote_control/tests.rs index 3dcfb81ce5..fb1512fedb 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/tests.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/tests.rs @@ -121,6 +121,10 @@ fn remote_control_url_for_listener(listener: &TcpListener) -> String { format!("http://{addr}/backend-api/") } +fn test_server_name() -> String { + gethostname().to_string_lossy().trim().to_string() +} + async fn expect_remote_control_status( status_rx: &mut watch::Receiver, expected_status: Option, @@ -134,6 +138,7 @@ async fn expect_remote_control_status( if let Some(expected_status) = expected_status { assert_eq!(status.status, expected_status); } + assert_eq!(status.server_name, test_server_name()); assert_eq!(status.installation_id, TEST_INSTALLATION_ID); assert_eq!(status.environment_id.as_deref(), expected_environment_id); } @@ -630,6 +635,7 @@ async fn remote_control_start_reports_missing_state_db_as_disabled_when_enabled( status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Disabled, + server_name: test_server_name(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, } @@ -698,6 +704,7 @@ async fn remote_control_handle_enable_disable_stops_and_restarts_connections() { &mut status_rx, RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connected, + server_name: test_server_name(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: Some("env_test".to_string()), }, @@ -708,6 +715,7 @@ async fn remote_control_handle_enable_disable_stops_and_restarts_connections() { remote_handle.disable(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Disabled, + server_name: test_server_name(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, } @@ -716,6 +724,7 @@ async fn remote_control_handle_enable_disable_stops_and_restarts_connections() { &mut status_rx, RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Disabled, + server_name: test_server_name(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, }, @@ -732,6 +741,7 @@ async fn remote_control_handle_enable_disable_stops_and_restarts_connections() { remote_handle.enable().expect("enable should succeed"), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: test_server_name(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, } @@ -740,6 +750,7 @@ async fn remote_control_handle_enable_disable_stops_and_restarts_connections() { &mut status_rx, RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: test_server_name(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, }, diff --git a/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs b/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs index 472639bc68..f117aec3b4 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs @@ -15,6 +15,7 @@ use super::protocol::ClientId; use super::protocol::RemoteControlTarget; use super::protocol::ServerEnvelope; use super::protocol::StreamId; +use super::remote_control_status_with_connection_status; use super::segment::ClientSegmentObservation; use super::segment::ClientSegmentReassembler; use super::segment::REMOTE_CONTROL_SEGMENT_MAX_BYTES; @@ -216,6 +217,7 @@ impl WebsocketState { pub(crate) struct RemoteControlWebsocket { remote_control_url: String, installation_id: String, + server_name: String, remote_control_target: Option, state_db: Option>, auth_manager: Arc, @@ -235,6 +237,7 @@ pub(crate) struct RemoteControlWebsocketConfig { pub(crate) remote_control_url: String, pub(crate) installation_id: String, pub(crate) remote_control_target: Option, + pub(crate) server_name: String, } enum ConnectOutcome { @@ -260,15 +263,8 @@ impl RemoteControlStatusPublisher { fn publish_status(&self, connection_status: RemoteControlConnectionStatus) { self.tx.send_if_modified(|status| { - let next_status = RemoteControlStatusChangedNotification { - status: connection_status, - installation_id: status.installation_id.clone(), - environment_id: if connection_status == RemoteControlConnectionStatus::Disabled { - None - } else { - status.environment_id.clone() - }, - }; + let next_status = + remote_control_status_with_connection_status(status, connection_status); if *status == next_status { return false; } @@ -285,6 +281,7 @@ impl RemoteControlStatusPublisher { } let next_status = RemoteControlStatusChangedNotification { status: status.status, + server_name: status.server_name.clone(), installation_id: status.installation_id.clone(), environment_id, }; @@ -301,6 +298,7 @@ impl RemoteControlStatusPublisher { #[derive(Clone, Copy)] pub(super) struct RemoteControlConnectOptions<'a> { installation_id: &'a str, + server_name: &'a str, subscribe_cursor: Option<&'a str>, app_server_client_name: Option<&'a str>, } @@ -327,6 +325,7 @@ impl RemoteControlWebsocket { Self { remote_control_url: config.remote_control_url, installation_id: config.installation_id, + server_name: config.server_name, remote_control_target: config.remote_control_target, state_db, auth_manager, @@ -454,6 +453,7 @@ impl RemoteControlWebsocket { let subscribe_cursor = self.state.lock().await.subscribe_cursor.clone(); let connect_options = RemoteControlConnectOptions { installation_id: &self.installation_id, + server_name: &self.server_name, subscribe_cursor: subscribe_cursor.as_deref(), app_server_client_name, }; @@ -1076,7 +1076,10 @@ pub(super) async fn connect_remote_control_websocket( if let Some(loaded_enrollment) = loaded_enrollment.as_ref() { status_publisher.publish_environment_id(Some(loaded_enrollment.environment_id.clone())); } - *enrollment = loaded_enrollment; + *enrollment = loaded_enrollment.map(|mut enrollment| { + enrollment.server_name = connect_options.server_name.to_string(); + enrollment + }); } if enrollment.is_none() { @@ -1088,6 +1091,7 @@ pub(super) async fn connect_remote_control_websocket( remote_control_target, &auth, connect_options.installation_id, + connect_options.server_name, ) .await { @@ -1279,6 +1283,7 @@ mod tests { ) { let (status_tx, status_rx) = watch::channel(RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, }); @@ -1386,6 +1391,7 @@ mod tests { &mut enrollment, RemoteControlConnectOptions { installation_id: TEST_INSTALLATION_ID, + server_name: "test-server", subscribe_cursor: None, app_server_client_name: None, }, @@ -1403,6 +1409,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: Some("env_test".to_string()), } @@ -1464,6 +1471,7 @@ mod tests { &mut enrollment, RemoteControlConnectOptions { installation_id: TEST_INSTALLATION_ID, + server_name: "test-server", subscribe_cursor: None, app_server_client_name: None, }, @@ -1477,6 +1485,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: Some("env_test".to_string()), } @@ -1546,6 +1555,7 @@ mod tests { &mut enrollment, RemoteControlConnectOptions { installation_id: TEST_INSTALLATION_ID, + server_name: "test-server", subscribe_cursor: None, app_server_client_name: None, }, @@ -1599,6 +1609,7 @@ mod tests { &mut enrollment, RemoteControlConnectOptions { installation_id: TEST_INSTALLATION_ID, + server_name: "test-server", subscribe_cursor: None, app_server_client_name: None, }, @@ -1647,6 +1658,7 @@ mod tests { &mut enrollment, RemoteControlConnectOptions { installation_id: TEST_INSTALLATION_ID, + server_name: "test-server", subscribe_cursor: None, app_server_client_name: None, }, @@ -1665,6 +1677,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, } @@ -1694,6 +1707,7 @@ mod tests { remote_control_url, installation_id: TEST_INSTALLATION_ID.to_string(), remote_control_target: Some(remote_control_target), + server_name: "test-server".to_string(), }, /*state_db*/ None, remote_control_auth_manager(), @@ -1738,6 +1752,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connecting, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: Some("env_first".to_string()), } @@ -1759,6 +1774,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connected, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: Some("env_first".to_string()), } @@ -1773,6 +1789,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Connected, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, } @@ -1788,6 +1805,7 @@ mod tests { status_rx.borrow().clone(), RemoteControlStatusChangedNotification { status: RemoteControlConnectionStatus::Disabled, + server_name: "test-server".to_string(), installation_id: TEST_INSTALLATION_ID.to_string(), environment_id: None, } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 4d21eb2846..8e3a23f0f7 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -204,7 +204,8 @@ Example with notification opt-out: - `app/list` — list available apps. - `remoteControl/enable` — experimental; enable remote control for the current app-server process and return the current remote-control status snapshot. The caller is responsible for persisting the desired setting outside app-server. - `remoteControl/disable` — experimental; disable remote control for the current app-server process and return the current remote-control status snapshot. This does not revoke already enrolled controller devices. -- `remoteControl/status/changed` — notification emitted when the remote-control status or client-visible environment id changes. `status` is one of `disabled`, `connecting`, `connected`, or `errored`; `environmentId` is a string when the app-server has a current enrollment and `null` when that enrollment is cleared, invalidated, or remote control is disabled. Newly initialized app-server clients always receive the current status snapshot. +- `remoteControl/status/read` — experimental; read the current remote-control status snapshot. `status` is one of `disabled`, `connecting`, `connected`, or `errored`; `serverName` is the local machine name used by this app-server process; `environmentId` is a string when the app-server has a current enrollment and `null` when that enrollment is cleared, invalidated, or remote control is disabled. +- `remoteControl/status/changed` — notification emitted when the remote-control status or client-visible environment id changes. `status` is one of `disabled`, `connecting`, `connected`, or `errored`; `serverName` is the local machine name used by this app-server process; `environmentId` is a string when the app-server has a current enrollment and `null` when that enrollment is cleared, invalidated, or remote control is disabled. Newly initialized app-server clients always receive the current status snapshot. - `skills/config/write` — write user-level skill config by name or absolute path. - `plugin/install` — install a plugin from a discovered marketplace entry, rejecting marketplace entries marked unavailable for install, install MCPs if any, and return the effective plugin auth policy plus any apps that still need auth (**under development; do not call from production clients yet**). - `plugin/uninstall` — uninstall a local plugin by `pluginId` in `@` form by removing its cached files and clearing its user-level config entry, or uninstall a remote ChatGPT plugin by backend `pluginId` by forwarding the uninstall to the ChatGPT plugin backend and removing any downloaded remote-plugin cache (**under development; do not call from production clients yet**). diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 673dec6341..4482d326f6 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -41,7 +41,6 @@ use codex_analytics::AppServerRpcTransport; use codex_app_server_protocol::ConfigLayerSource; use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::JSONRPCMessage; -use codex_app_server_protocol::RemoteControlStatusChangedNotification; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::TextPosition as AppTextPosition; use codex_app_server_protocol::TextRange as AppTextRange; @@ -1003,14 +1002,9 @@ pub async fn run_main_with_transport_options( continue; } remote_control_status = status.clone(); + let notification = ServerNotification::RemoteControlStatusChanged(status); initialize_notification_sender - .send_server_notification(ServerNotification::RemoteControlStatusChanged( - RemoteControlStatusChangedNotification { - status: status.status, - installation_id: status.installation_id, - environment_id: status.environment_id, - }, - )) + .send_server_notification(notification) .await; } created = thread_created_rx.recv(), if listen_for_threads => { diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 0f291abfcc..79c077dec7 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -897,6 +897,10 @@ impl MessageProcessor { .remote_control_processor .disable() .map(|response| Some(response.into())), + ClientRequest::RemoteControlStatusRead { .. } => self + .remote_control_processor + .status_read() + .map(|response| Some(response.into())), ClientRequest::ConfigRequirementsRead { params: _, .. } => self .config_processor .config_requirements_read() diff --git a/codex-rs/app-server/src/request_processors/remote_control_processor.rs b/codex-rs/app-server/src/request_processors/remote_control_processor.rs index 41a124e94b..2fceeb5030 100644 --- a/codex-rs/app-server/src/request_processors/remote_control_processor.rs +++ b/codex-rs/app-server/src/request_processors/remote_control_processor.rs @@ -5,6 +5,7 @@ use crate::transport::RemoteControlUnavailable; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::RemoteControlDisableResponse; use codex_app_server_protocol::RemoteControlEnableResponse; +use codex_app_server_protocol::RemoteControlStatusReadResponse; #[derive(Clone)] pub(crate) struct RemoteControlRequestProcessor { @@ -31,6 +32,16 @@ impl RemoteControlRequestProcessor { Ok(RemoteControlDisableResponse::from(handle.disable())) } + pub(crate) fn status_read(&self) -> Result { + let status = self.handle()?.status(); + Ok(RemoteControlStatusReadResponse { + status: status.status, + server_name: status.server_name, + installation_id: status.installation_id, + environment_id: status.environment_id, + }) + } + fn handle(&self) -> Result<&RemoteControlHandle, JSONRPCErrorError> { self.remote_control_handle .as_ref() diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index 19f3b5b85e..981c22f625 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -582,6 +582,12 @@ impl McpProcess { .await } + /// Send a `remoteControl/status/read` JSON-RPC request. + pub async fn send_remote_control_status_read_request(&mut self) -> anyhow::Result { + self.send_request("remoteControl/status/read", /*params*/ None) + .await + } + /// Send an `app/list` JSON-RPC request. pub async fn send_apps_list_request(&mut self, params: AppsListParams) -> anyhow::Result { let params = Some(serde_json::to_value(params)?); diff --git a/codex-rs/app-server/tests/suite/v2/remote_control.rs b/codex-rs/app-server/tests/suite/v2/remote_control.rs index ae6bc9013f..d71f05bf58 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_control.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_control.rs @@ -1,14 +1,27 @@ use std::time::Duration; +use anyhow::Context; use anyhow::Result; +use app_test_support::ChatGptAuthFixture; use app_test_support::McpProcess; use app_test_support::to_response; +use app_test_support::write_chatgpt_auth; +use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RemoteControlConnectionStatus; use codex_app_server_protocol::RemoteControlDisableResponse; use codex_app_server_protocol::RemoteControlEnableResponse; +use codex_app_server_protocol::RemoteControlStatusReadResponse; use codex_app_server_protocol::RequestId; +use codex_config::types::AuthCredentialsStoreMode; +use pretty_assertions::assert_eq; use tempfile::TempDir; +use tokio::io::AsyncBufReadExt; +use tokio::io::BufReader; +use tokio::net::TcpListener; +use tokio::net::TcpStream; +use tokio::sync::oneshot; +use tokio::task::JoinHandle; use tokio::time::timeout; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); @@ -28,6 +41,28 @@ async fn remote_control_disable_returns_disabled_status() -> Result<()> { let received: RemoteControlDisableResponse = to_response(response)?; assert_eq!(received.status, RemoteControlConnectionStatus::Disabled); + assert!(!received.server_name.is_empty()); + assert_eq!(received.environment_id, None); + assert!(!received.installation_id.is_empty()); + Ok(()) +} + +#[tokio::test] +async fn remote_control_status_read_returns_disabled_status() -> Result<()> { + let codex_home = TempDir::new()?; + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp.send_remote_control_status_read_request().await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let received: RemoteControlStatusReadResponse = to_response(response)?; + + assert_eq!(received.status, RemoteControlConnectionStatus::Disabled); + assert!(!received.server_name.is_empty()); assert_eq!(received.environment_id, None); assert!(!received.installation_id.is_empty()); Ok(()) @@ -36,6 +71,7 @@ async fn remote_control_disable_returns_disabled_status() -> Result<()> { #[tokio::test] async fn remote_control_enable_returns_connecting_status() -> Result<()> { let codex_home = TempDir::new()?; + let _backend = BlockingRemoteControlBackend::start(codex_home.path()).await?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; @@ -48,7 +84,117 @@ async fn remote_control_enable_returns_connecting_status() -> Result<()> { let received: RemoteControlEnableResponse = to_response(response)?; assert_eq!(received.status, RemoteControlConnectionStatus::Connecting); + assert!(!received.server_name.is_empty()); assert_eq!(received.environment_id, None); assert!(!received.installation_id.is_empty()); Ok(()) } + +#[tokio::test] +async fn remote_control_status_read_returns_connecting_status_after_enable() -> Result<()> { + let codex_home = TempDir::new()?; + let mut backend = BlockingRemoteControlBackend::start(codex_home.path()).await?; + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp.send_remote_control_enable_request().await?; + let _: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + + let enroll_request = timeout(DEFAULT_TIMEOUT, backend.wait_for_enroll_request()).await??; + assert_eq!( + enroll_request, + "POST /backend-api/wham/remote/control/server/enroll HTTP/1.1" + ); + + let request_id = mcp.send_remote_control_status_read_request().await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let received: RemoteControlStatusReadResponse = to_response(response)?; + + assert_eq!(received.status, RemoteControlConnectionStatus::Connecting); + assert!(!received.server_name.is_empty()); + assert_eq!(received.environment_id, None); + assert!(!received.installation_id.is_empty()); + Ok(()) +} + +struct BlockingRemoteControlBackend { + enroll_request_rx: Option>>, + server_task: JoinHandle<()>, +} + +impl BlockingRemoteControlBackend { + async fn start(codex_home: &std::path::Path) -> Result { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let remote_control_url = format!("http://{}/backend-api/", listener.local_addr()?); + write_mock_responses_config_toml_with_chatgpt_base_url( + codex_home, + &remote_control_url, + &remote_control_url, + )?; + write_chatgpt_auth( + codex_home, + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account_id") + .chatgpt_account_id("account_id"), + AuthCredentialsStoreMode::File, + )?; + + let (enroll_request_tx, enroll_request_rx) = oneshot::channel(); + let server_task = tokio::spawn(async move { + match read_enroll_request(listener).await { + Ok((request_line, _reader)) => { + let _ = enroll_request_tx.send(Ok(request_line)); + std::future::pending::<()>().await; + } + Err(err) => { + let _ = enroll_request_tx.send(Err(err)); + } + } + }); + + Ok(Self { + enroll_request_rx: Some(enroll_request_rx), + server_task, + }) + } + + async fn wait_for_enroll_request(&mut self) -> Result { + let rx = self + .enroll_request_rx + .take() + .context("enroll request should only be awaited once")?; + rx.await? + } +} + +impl Drop for BlockingRemoteControlBackend { + fn drop(&mut self) { + self.server_task.abort(); + } +} + +async fn read_enroll_request(listener: TcpListener) -> Result<(String, BufReader)> { + let (stream, _) = listener.accept().await?; + let mut reader = BufReader::new(stream); + + let mut request_line = String::new(); + reader.read_line(&mut request_line).await?; + + loop { + let mut line = String::new(); + reader.read_line(&mut line).await?; + if line == "\r\n" { + break; + } + } + + Ok((request_line.trim_end().to_string(), reader)) +} From 249d50aafc552d2fdcd3836a50b03ce366d9744f Mon Sep 17 00:00:00 2001 From: Tom Date: Fri, 15 May 2026 14:37:27 -0700 Subject: [PATCH 3/6] [codex] Soften SQLite metadata sync failures (#22899) ## Summary - keep transcript-derived local thread metadata SQLite failures best-effort - preserve hard failures for explicit git-only metadata updates that still require SQLite state - add regression coverage for the soft-vs-hard metadata update policy ## Root cause The live thread metadata sync introduced after v0.131.0-alpha.8 moved append-derived metadata writes above the rollout writer. Those SQLite writes now propagated through the live thread flush path, so a corrupted optional state DB could surface as a transcript persistence warning even when JSONL writes still succeeded. The hard failures were introduced in #22236 --- .../src/local/update_thread_metadata.rs | 80 ++++++++++++++++++- 1 file changed, 76 insertions(+), 4 deletions(-) diff --git a/codex-rs/thread-store/src/local/update_thread_metadata.rs b/codex-rs/thread-store/src/local/update_thread_metadata.rs index c09ca80f97..16f9f79e06 100644 --- a/codex-rs/thread-store/src/local/update_thread_metadata.rs +++ b/codex-rs/thread-store/src/local/update_thread_metadata.rs @@ -14,6 +14,7 @@ use codex_rollout::find_archived_thread_path_by_id_str; use codex_rollout::find_thread_path_by_id_str; use codex_rollout::read_session_meta_line; use codex_state::ThreadMetadataBuilder; +use tracing::warn; use super::LocalThreadStore; use super::helpers::git_info_from_parts; @@ -51,8 +52,15 @@ pub(super) async fn update_thread_metadata( } let needs_rollout_compat = needs_rollout_compatibility_update(&patch); - let updated = - apply_metadata_update(store, thread_id, patch.clone(), params.include_archived).await?; + let require_sqlite_write = sqlite_write_failure_should_block(&patch); + let updated = apply_metadata_update( + store, + thread_id, + patch.clone(), + params.include_archived, + require_sqlite_write, + ) + .await?; if !needs_rollout_compat { return Ok(updated); } @@ -169,6 +177,7 @@ async fn apply_metadata_update( thread_id: ThreadId, patch: ThreadMetadataPatch, include_archived: bool, + require_sqlite_write: bool, ) -> ThreadStoreResult { let live_rollout_path = live_writer::rollout_path(store, thread_id).await.ok(); let mut rollout_path = patch.rollout_path.clone().or(live_rollout_path); @@ -316,9 +325,19 @@ async fn apply_metadata_update( }; match (state_db.is_some(), sqlite_write_result) { (true, Ok(())) => {} - (true, Err(err)) => return Err(err), + (true, Err(err)) if require_sqlite_write || !sqlite_write_error_is_best_effort(&err) => { + return Err(err); + } + (true, Err(err)) => { + warn!("state db update_thread_metadata failed for {thread_id}: {err}"); + } (false, Ok(())) => {} - (false, Err(err)) => return Err(err), + (false, Err(err)) if require_sqlite_write || !sqlite_write_error_is_best_effort(&err) => { + return Err(err); + } + (false, Err(err)) => { + warn!("state db update_thread_metadata failed for {thread_id}: {err}"); + } } read_thread::read_thread( @@ -342,6 +361,19 @@ fn needs_rollout_compatibility_update(patch: &ThreadMetadataPatch) -> bool { !has_observed_metadata_facts(patch) } +fn sqlite_write_failure_should_block(patch: &ThreadMetadataPatch) -> bool { + // Before live metadata sync moved above the rollout writer, SQLite sync failures for + // transcript-derived metadata, thread names, and memory-mode indexing were log-only. Keep that + // failure isolation so a corrupted optional state DB does not make JSONL transcript durability + // look broken. Explicit git-only updates still require SQLite because partial git patches need + // the existing SQLite value to preserve unspecified fields. + patch.git_info.is_some() && !has_observed_metadata_facts(patch) +} + +fn sqlite_write_error_is_best_effort(err: &ThreadStoreError) -> bool { + matches!(err, ThreadStoreError::Internal { .. }) +} + fn has_observed_metadata_facts(patch: &ThreadMetadataPatch) -> bool { patch.rollout_path.is_some() || patch.preview.is_some() @@ -1136,6 +1168,46 @@ mod tests { assert_eq!(memory_mode.as_deref(), Some("disabled")); } + #[test] + fn sqlite_failures_are_best_effort_for_legacy_rollout_compat_updates() { + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + name: Some(Some("User chosen name".to_string())), + ..Default::default() + })); + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + memory_mode: Some(ThreadMemoryMode::Disabled), + ..Default::default() + })); + } + + #[test] + fn sqlite_failures_are_best_effort_for_observed_metadata_updates() { + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + updated_at: Some(Utc::now()), + ..Default::default() + })); + assert!(!sqlite_write_failure_should_block(&ThreadMetadataPatch { + preview: Some("Observed preview".to_string()), + git_info: Some(GitInfoPatch { + branch: Some(Some("main".to_string())), + ..Default::default() + }), + memory_mode: Some(ThreadMemoryMode::Enabled), + ..Default::default() + })); + } + + #[test] + fn sqlite_failures_still_block_for_explicit_git_only_updates() { + assert!(sqlite_write_failure_should_block(&ThreadMetadataPatch { + git_info: Some(GitInfoPatch { + branch: Some(Some("main".to_string())), + ..Default::default() + }), + ..Default::default() + })); + } + #[tokio::test] async fn metadata_patch_applies_title_over_existing_name() { let home = TempDir::new().expect("temp dir"); From 8543e39885956b5d38b416acd1868b5f643cc0ed Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Fri, 15 May 2026 15:04:04 -0700 Subject: [PATCH 4/6] Preserve image detail in app-server inputs (#20693) ## Summary - Add optional image detail to user image inputs across core, app-server v2, thread history/event mapping, and the generated app-server schemas/types. - Preserve requested detail when serializing Responses image inputs: omitted detail stays on the existing `high` default, while explicit `original` keeps local images on the original-resolution path. - Support `high`/`original` consistently for tool image outputs, including MCP `codex/imageDetail`, code-mode image helpers, and `view_image`. --- .../analytics/src/analytics_client_tests.rs | 2 + .../schema/json/ClientRequest.json | 24 ++- .../schema/json/ServerNotification.json | 29 +++ .../codex_app_server_protocol.schemas.json | 24 ++- .../codex_app_server_protocol.v2.schemas.json | 24 ++- .../json/v2/ItemCompletedNotification.json | 29 +++ .../json/v2/ItemStartedNotification.json | 29 +++ .../RawResponseItemCompletedNotification.json | 2 - .../schema/json/v2/ReviewStartResponse.json | 29 +++ .../schema/json/v2/ThreadForkResponse.json | 29 +++ .../schema/json/v2/ThreadListResponse.json | 29 +++ .../json/v2/ThreadMetadataUpdateResponse.json | 29 +++ .../schema/json/v2/ThreadReadResponse.json | 29 +++ .../schema/json/v2/ThreadResumeParams.json | 2 - .../schema/json/v2/ThreadResumeResponse.json | 29 +++ .../json/v2/ThreadRollbackResponse.json | 29 +++ .../schema/json/v2/ThreadStartResponse.json | 29 +++ .../json/v2/ThreadStartedNotification.json | 29 +++ .../json/v2/ThreadUnarchiveResponse.json | 29 +++ .../json/v2/TurnCompletedNotification.json | 29 +++ .../schema/json/v2/TurnStartParams.json | 29 +++ .../schema/json/v2/TurnStartResponse.json | 29 +++ .../json/v2/TurnStartedNotification.json | 29 +++ .../schema/json/v2/TurnSteerParams.json | 29 +++ .../schema/typescript/ImageDetail.ts | 2 +- .../schema/typescript/v2/UserInput.ts | 3 +- .../src/protocol/thread_history.rs | 91 ++++++++- .../src/protocol/v2/tests.rs | 32 ++++ .../src/protocol/v2/turn.rs | 21 +- .../app-server/src/bespoke_event_handling.rs | 2 + .../thread_processor_tests.rs | 1 + .../request_processors/token_usage_replay.rs | 2 + .../app-server/tests/suite/v2/thread_read.rs | 1 + .../app-server/tests/suite/v2/turn_start.rs | 180 ++++++++++++------ codex-rs/cli/src/main.rs | 2 +- codex-rs/code-mode/src/description.rs | 2 +- codex-rs/code-mode/src/response.rs | 2 - codex-rs/code-mode/src/runtime/value.rs | 8 +- codex-rs/code-mode/src/service.rs | 36 +++- codex-rs/core/src/agent/control.rs | 4 +- codex-rs/core/src/event_mapping.rs | 3 +- codex-rs/core/src/event_mapping_tests.rs | 20 +- .../core/src/personality_migration_tests.rs | 1 + .../session/rollout_reconstruction_tests.rs | 23 +++ codex-rs/core/src/session/tests.rs | 10 + codex-rs/core/src/thread_manager_tests.rs | 2 + .../src/tools/code_mode/response_adapter.rs | 2 - .../core/src/tools/handlers/view_image.rs | 84 +++++++- .../src/tools/handlers/view_image_spec.rs | 14 +- codex-rs/core/src/tools/spec_plan_tests.rs | 17 +- codex-rs/core/tests/suite/code_mode.rs | 2 +- codex-rs/core/tests/suite/compact.rs | 1 + codex-rs/core/tests/suite/compact_remote.rs | 5 +- .../core/tests/suite/compact_remote_parity.rs | 1 + codex-rs/core/tests/suite/image_rollout.rs | 2 + codex-rs/core/tests/suite/model_switching.rs | 1 + .../core/tests/suite/personality_migration.rs | 1 + codex-rs/core/tests/suite/resume_warning.rs | 1 + codex-rs/core/tests/suite/sqlite_state.rs | 1 + codex-rs/core/tests/suite/view_image.rs | 4 +- codex-rs/exec/src/lib.rs | 4 +- .../external-agent-sessions/src/export.rs | 1 + .../tests/suite/otel_export_routing_policy.rs | 2 + codex-rs/protocol/src/items.rs | 40 +++- codex-rs/protocol/src/models.rs | 100 ++++++++-- codex-rs/protocol/src/protocol.rs | 68 ++++++- codex-rs/protocol/src/user_input.rs | 16 +- codex-rs/rollout/src/recorder_tests.rs | 2 + codex-rs/rollout/src/tests.rs | 1 + codex-rs/state/src/extract.rs | 4 + codex-rs/thread-store/src/local/mod.rs | 1 + .../thread-store/src/thread_metadata_sync.rs | 1 + codex-rs/tools/src/image_detail.rs | 2 +- codex-rs/tools/src/image_detail_tests.rs | 8 +- codex-rs/tui/src/app/tests.rs | 2 +- .../tui/src/chatwidget/input_submission.rs | 2 + .../chatwidget/tests/composer_submission.rs | 9 +- .../src/chatwidget/tests/history_replay.rs | 4 + .../tui/src/chatwidget/tests/review_mode.rs | 1 + codex-rs/tui/src/chatwidget/user_messages.rs | 4 +- codex-rs/tui/src/ide_context/prompt.rs | 2 + 81 files changed, 1302 insertions(+), 156 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index ed626699d3..f3c56bfb3d 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -279,6 +279,7 @@ fn sample_turn_start_request(thread_id: &str, request_id: i64) -> ClientRequest }, UserInput::Image { url: "https://example.com/a.png".to_string(), + detail: None, }, ], ..Default::default() @@ -399,6 +400,7 @@ fn sample_turn_steer_request( }, UserInput::LocalImage { path: "/tmp/a.png".into(), + detail: None, }, ], responsesapi_client_metadata: None, diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index fffc610128..5fc44d4730 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1235,8 +1235,6 @@ }, "ImageDetail": { "enum": [ - "auto", - "low", "high", "original" ], @@ -4250,6 +4248,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -4270,6 +4279,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index a5fce14a4f..6e8b5d8a04 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -1932,6 +1932,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "ItemCompletedNotification": { "properties": { "completedAtMs": { @@ -4593,6 +4600,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -4613,6 +4631,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index d1b318889b..5813afcdb2 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -9920,8 +9920,6 @@ }, "ImageDetail": { "enum": [ - "auto", - "low", "high", "original" ], @@ -18134,6 +18132,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -18154,6 +18163,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index bb926075ee..d14acc7a1b 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6420,8 +6420,6 @@ }, "ImageDetail": { "enum": [ - "auto", - "low", "high", "original" ], @@ -15958,6 +15956,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -15978,6 +15987,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json index 6909415c2a..7aed9d9722 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemCompletedNotification.json @@ -285,6 +285,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1179,6 +1186,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1199,6 +1217,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json index 758ceba32d..80a3cd2425 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ItemStartedNotification.json @@ -285,6 +285,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1179,6 +1186,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1199,6 +1217,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index bd88872fb6..74420ea57e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -145,8 +145,6 @@ }, "ImageDetail": { "enum": [ - "auto", - "low", "high", "original" ], diff --git a/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json index 9afd1ae514..882fa1f598 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ReviewStartResponse.json @@ -422,6 +422,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1452,6 +1459,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1472,6 +1490,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index 1608b2f48c..3d66152a60 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -527,6 +527,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -2012,6 +2019,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -2032,6 +2050,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json index f78fbaf27e..0693b19028 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadListResponse.json @@ -448,6 +448,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1827,6 +1834,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1847,6 +1865,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json index 4268ad203a..d2e4ca6728 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadMetadataUpdateResponse.json @@ -448,6 +448,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1827,6 +1834,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1847,6 +1865,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json index fb0d80a047..431ca759de 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadReadResponse.json @@ -448,6 +448,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1827,6 +1834,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1847,6 +1865,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index e8a437f003..253ebf4572 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -204,8 +204,6 @@ }, "ImageDetail": { "enum": [ - "auto", - "low", "high", "original" ], diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index 302c2e1069..f379455b59 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -527,6 +527,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -2012,6 +2019,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -2032,6 +2050,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json index 204828c732..8d807d76cb 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadRollbackResponse.json @@ -448,6 +448,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1827,6 +1834,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1847,6 +1865,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index 9dc08614cf..7d31c52f1c 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -527,6 +527,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -2012,6 +2019,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -2032,6 +2050,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json index 759b5990be..3ec843e817 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartedNotification.json @@ -448,6 +448,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1827,6 +1834,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1847,6 +1865,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json index f64400129a..ea48e34180 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadUnarchiveResponse.json @@ -448,6 +448,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1827,6 +1834,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1847,6 +1865,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json index e5e2558e9c..40b81e5a34 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnCompletedNotification.json @@ -422,6 +422,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1452,6 +1459,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1472,6 +1490,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index 086d3c6ed1..ecea8f1997 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -99,6 +99,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -351,6 +358,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -371,6 +389,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json index a2eff7fdd8..ffe0a28d15 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartResponse.json @@ -422,6 +422,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1452,6 +1459,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1472,6 +1490,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json index 0952db2aca..cbde541e52 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartedNotification.json @@ -422,6 +422,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "McpToolCallError": { "properties": { "message": { @@ -1452,6 +1459,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -1472,6 +1490,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnSteerParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnSteerParams.json index a064d9e7e3..1b7cfbf400 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnSteerParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnSteerParams.json @@ -20,6 +20,13 @@ ], "type": "object" }, + "ImageDetail": { + "enum": [ + "high", + "original" + ], + "type": "string" + }, "TextElement": { "properties": { "byteRange": { @@ -75,6 +82,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "type": { "enum": [ "image" @@ -95,6 +113,17 @@ }, { "properties": { + "detail": { + "anyOf": [ + { + "$ref": "#/definitions/ImageDetail" + }, + { + "type": "null" + } + ], + "default": null + }, "path": { "type": "string" }, diff --git a/codex-rs/app-server-protocol/schema/typescript/ImageDetail.ts b/codex-rs/app-server-protocol/schema/typescript/ImageDetail.ts index a48f07c088..5a62cc32f1 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ImageDetail.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ImageDetail.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type ImageDetail = "auto" | "low" | "high" | "original"; +export type ImageDetail = "high" | "original"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/UserInput.ts b/codex-rs/app-server-protocol/schema/typescript/v2/UserInput.ts index 38abc2338b..2ac37c5228 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/UserInput.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/UserInput.ts @@ -1,10 +1,11 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { ImageDetail } from "../ImageDetail"; import type { TextElement } from "./TextElement"; export type UserInput = { "type": "text", text: string, /** * UI-defined spans within `text` used to render or persist special elements. */ -text_elements: Array, } | { "type": "image", url: string, } | { "type": "localImage", path: string, } | { "type": "skill", name: string, path: string, } | { "type": "mention", name: string, path: string, }; +text_elements: Array, } | { "type": "image", detail?: ImageDetail, url: string, } | { "type": "localImage", detail?: ImageDetail, path: string, } | { "type": "skill", name: string, path: string, } | { "type": "mention", name: string, path: string, }; diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index 1121d3a35b..3918e4a9bd 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -1077,12 +1077,18 @@ impl ThreadHistoryBuilder { }); } if let Some(images) = &payload.images { - for image in images { - content.push(UserInput::Image { url: image.clone() }); + for (idx, image) in images.iter().enumerate() { + content.push(UserInput::Image { + url: image.clone(), + detail: payload.image_details.get(idx).copied().flatten(), + }); } } - for path in &payload.local_images { - content.push(UserInput::LocalImage { path: path.clone() }); + for (idx, path) in payload.local_images.iter().enumerate() { + content.push(UserInput::LocalImage { + path: path.clone(), + detail: payload.local_image_details.get(idx).copied().flatten(), + }); } content } @@ -1203,6 +1209,7 @@ mod tests { use codex_protocol::items::UserMessageItem as CoreUserMessageItem; use codex_protocol::items::build_hook_prompt_message; use codex_protocol::mcp::CallToolResult; + use codex_protocol::models::ImageDetail; use codex_protocol::models::MessagePhase as CoreMessagePhase; use codex_protocol::models::WebSearchAction as CoreWebSearchAction; use codex_protocol::parse_command::ParsedCommand; @@ -1241,6 +1248,7 @@ mod tests { images: Some(vec!["https://example.com/one.png".into()]), text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "Hi there".into(), @@ -1258,6 +1266,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "Reply two".into(), @@ -1288,6 +1297,7 @@ mod tests { }, UserInput::Image { url: "https://example.com/one.png".into(), + detail: None, } ], } @@ -1335,6 +1345,45 @@ mod tests { ); } + #[test] + fn rebuilds_user_message_image_details_from_legacy_events() { + let local_path = PathBuf::from("/tmp/local.png"); + let events = vec![RolloutItem::EventMsg(EventMsg::UserMessage( + UserMessageEvent { + message: "inspect these".into(), + images: Some(vec!["https://example.com/image.png".into()]), + image_details: vec![Some(ImageDetail::Original)], + local_images: vec![local_path.clone()], + local_image_details: vec![Some(ImageDetail::Original)], + text_elements: Vec::new(), + }, + ))]; + + let turns = build_turns_from_rollout_items(&events); + + assert_eq!(turns.len(), 1); + assert_eq!( + turns[0].items[0], + ThreadItem::UserMessage { + id: "item-1".into(), + content: vec![ + UserInput::Text { + text: "inspect these".into(), + text_elements: Vec::new(), + }, + UserInput::Image { + url: "https://example.com/image.png".into(), + detail: Some(ImageDetail::Original), + }, + UserInput::LocalImage { + path: local_path, + detail: Some(ImageDetail::Original), + }, + ], + } + ); + } + #[test] fn ignores_non_plan_item_lifecycle_events() { let turn_id = "turn-1"; @@ -1351,6 +1400,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::ItemStarted(ItemStartedEvent { thread_id, @@ -1428,6 +1478,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() })), RolloutItem::EventMsg(EventMsg::ImageGenerationEnd(ImageGenerationEndEvent { call_id: "ig_123".into(), @@ -1485,6 +1536,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentReasoning(AgentReasoningEvent { text: "first summary".into(), @@ -1537,6 +1589,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "Working...".into(), @@ -1554,6 +1607,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "Second attempt complete.".into(), @@ -1624,6 +1678,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "A1".into(), @@ -1635,6 +1690,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "A2".into(), @@ -1647,6 +1703,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "A3".into(), @@ -1712,6 +1769,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "A1".into(), @@ -1723,6 +1781,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "A2".into(), @@ -1754,12 +1813,14 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::UserMessage(UserMessageEvent { message: "Steer".into(), images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -1812,6 +1873,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::WebSearchEnd(WebSearchEndEvent { call_id: "search-1".into(), @@ -1984,6 +2046,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::DynamicToolCallRequest( codex_protocol::dynamic_tools::DynamicToolCallRequest { @@ -2049,6 +2112,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "exec-declined".into(), @@ -2138,6 +2202,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::GuardianAssessment(GuardianAssessmentEvent { id: "review-guardian-exec".into(), @@ -2221,6 +2286,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::GuardianAssessment(GuardianAssessmentEvent { id: "review-guardian-execve".into(), @@ -2284,6 +2350,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -2303,6 +2370,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "exec-late".into(), @@ -2376,6 +2444,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -2395,6 +2464,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id: "exec-unknown-turn".into(), @@ -2463,6 +2533,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::PatchApplyBegin(PatchApplyBeginEvent { call_id: "patch-call".into(), @@ -2527,6 +2598,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent { call_id: "patch-call".into(), @@ -2591,6 +2663,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -2610,6 +2683,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -2657,6 +2731,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -2676,6 +2751,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnAborted(TurnAbortedEvent { turn_id: Some("turn-a".into()), @@ -2748,6 +2824,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::CollabResumeEnd(codex_protocol::protocol::CollabResumeEndEvent { call_id: "resume-1".into(), @@ -2805,6 +2882,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::CollabAgentSpawnEnd(codex_protocol::protocol::CollabAgentSpawnEndEvent { call_id: "spawn-1".into(), @@ -2866,6 +2944,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::CollabAgentInteractionBegin( codex_protocol::protocol::CollabAgentInteractionBeginEvent { @@ -2929,6 +3008,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::AgentMessage(AgentMessageEvent { message: "done".into(), @@ -2965,6 +3045,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::TurnComplete(TurnCompleteEvent { turn_id: "turn-a".into(), @@ -3020,6 +3101,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() }), EventMsg::Error(ErrorEvent { message: "stream failure".into(), @@ -3077,6 +3159,7 @@ mod tests { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() })), RolloutItem::ResponseItem(hook_prompt), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index d17a3dd988..526cc22131 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -15,6 +15,7 @@ use codex_protocol::memory_citation::MemoryCitation as CoreMemoryCitation; use codex_protocol::memory_citation::MemoryCitationEntry as CoreMemoryCitationEntry; use codex_protocol::models::AdditionalPermissionProfile as CoreAdditionalPermissionProfile; use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions; +use codex_protocol::models::ImageDetail; use codex_protocol::models::ManagedFileSystemPermissions as CoreManagedFileSystemPermissions; use codex_protocol::models::MessagePhase; use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions; @@ -2319,9 +2320,11 @@ fn core_turn_item_into_thread_item_converts_supported_variants() { }, CoreUserInput::Image { image_url: "https://example.com/image.png".to_string(), + detail: Some(ImageDetail::Original), }, CoreUserInput::LocalImage { path: PathBuf::from("local/image.png"), + detail: Some(ImageDetail::Original), }, CoreUserInput::Skill { name: "skill-creator".to_string(), @@ -2345,9 +2348,11 @@ fn core_turn_item_into_thread_item_converts_supported_variants() { }, UserInput::Image { url: "https://example.com/image.png".to_string(), + detail: Some(ImageDetail::Original), }, UserInput::LocalImage { path: PathBuf::from("local/image.png"), + detail: Some(ImageDetail::Original), }, UserInput::Skill { name: "skill-creator".to_string(), @@ -2562,6 +2567,33 @@ fn core_turn_item_into_thread_item_converts_supported_variants() { ); } +#[test] +fn user_input_into_core_preserves_image_detail() { + assert_eq!( + UserInput::Image { + url: "https://example.com/image.png".to_string(), + detail: Some(ImageDetail::Original), + } + .into_core(), + CoreUserInput::Image { + image_url: "https://example.com/image.png".to_string(), + detail: Some(ImageDetail::Original), + } + ); + + assert_eq!( + UserInput::LocalImage { + path: PathBuf::from("local/image.png"), + detail: Some(ImageDetail::Original), + } + .into_core(), + CoreUserInput::LocalImage { + path: PathBuf::from("local/image.png"), + detail: Some(ImageDetail::Original), + } + ); +} + #[test] fn skills_list_params_serialization_uses_force_reload() { assert_eq!( diff --git a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs index 8f4cd04e2d..8781dd2bd5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs @@ -7,6 +7,7 @@ use codex_experimental_api_macros::ExperimentalApi; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::models::ImageDetail; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; @@ -249,9 +250,15 @@ pub enum UserInput { text_elements: Vec, }, Image { + #[serde(default)] + #[ts(optional)] + detail: Option, url: String, }, LocalImage { + #[serde(default)] + #[ts(optional)] + detail: Option, path: PathBuf, }, Skill { @@ -274,8 +281,11 @@ impl UserInput { text, text_elements: text_elements.into_iter().map(Into::into).collect(), }, - UserInput::Image { url } => CoreUserInput::Image { image_url: url }, - UserInput::LocalImage { path } => CoreUserInput::LocalImage { path }, + UserInput::Image { url, detail } => CoreUserInput::Image { + image_url: url, + detail, + }, + UserInput::LocalImage { path, detail } => CoreUserInput::LocalImage { path, detail }, UserInput::Skill { name, path } => CoreUserInput::Skill { name, path }, UserInput::Mention { name, path } => CoreUserInput::Mention { name, path }, } @@ -292,8 +302,11 @@ impl From for UserInput { text, text_elements: text_elements.into_iter().map(Into::into).collect(), }, - CoreUserInput::Image { image_url } => UserInput::Image { url: image_url }, - CoreUserInput::LocalImage { path } => UserInput::LocalImage { path }, + CoreUserInput::Image { image_url, detail } => UserInput::Image { + url: image_url, + detail, + }, + CoreUserInput::LocalImage { path, detail } => UserInput::LocalImage { path, detail }, CoreUserInput::Skill { name, path } => UserInput::Skill { name, path }, CoreUserInput::Mention { name, path } => UserInput::Mention { name, path }, _ => unreachable!("unsupported user input variant"), diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index ddd7bf1a38..5b137c1091 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -2137,6 +2137,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::EventMsg(EventMsg::AgentMessage(AgentMessageEvent { message: "after rollback".to_string(), @@ -3210,6 +3211,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }), ); } diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index c2e6b9a55e..f9841421ca 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -214,6 +214,7 @@ mod thread_processor_behavior_tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, ))]; let active_turn = Turn { diff --git a/codex-rs/app-server/src/request_processors/token_usage_replay.rs b/codex-rs/app-server/src/request_processors/token_usage_replay.rs index b19c4a61a0..b8c65645fc 100644 --- a/codex-rs/app-server/src/request_processors/token_usage_replay.rs +++ b/codex-rs/app-server/src/request_processors/token_usage_replay.rs @@ -152,6 +152,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::EventMsg(EventMsg::AgentMessage(AgentMessageEvent { message: "first answer".to_string(), @@ -167,6 +168,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), ] } diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index fa143254a5..c8e3e179fb 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -1323,6 +1323,7 @@ fn store_history_items() -> Vec { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, ))] } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index afc37d73c4..844d9ab1bb 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -1,3 +1,4 @@ +use anyhow::Context; use anyhow::Result; use app_test_support::DEFAULT_CLIENT_NAME; use app_test_support::McpProcess; @@ -62,11 +63,13 @@ use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::Settings; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; +use codex_protocol::models::ImageDetail; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; +use serde_json::Value; use serde_json::json; use std::collections::BTreeMap; use std::collections::HashMap; @@ -84,6 +87,11 @@ const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs const TEST_ORIGINATOR: &str = "codex_vscode"; const LOCAL_PRAGMATIC_TEMPLATE: &str = "You are a deeply pragmatic, effective software engineer."; const INVALID_REQUEST_ERROR_CODE: i64 = -32600; +const TINY_PNG_BYTES: &[u8] = &[ + 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0, + 0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, 0, 0, 5, 0, 1, + 122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130, +]; fn body_contains(req: &wiremock::Request, text: &str) -> bool { String::from_utf8(req.body.clone()) @@ -91,6 +99,107 @@ fn body_contains(req: &wiremock::Request, text: &str) -> bool { .is_some_and(|body| body.contains(text)) } +async fn run_local_image_turn(detail: Option) -> Result> { + // Two Codex turns hit the mock model (session start + turn/start). + let responses = vec![ + create_final_assistant_message_sse_response("Done")?, + create_final_assistant_message_sse_response("Done")?, + ]; + // Use the unchecked variant because the strict matcher does not currently + // cover image-bearing request payloads. + let server = create_mock_responses_server_sequence_unchecked(responses).await; + + let codex_home = TempDir::new()?; + create_config_toml( + codex_home.path(), + &server.uri(), + "never", + &BTreeMap::default(), + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let image_path = codex_home.path().join("image.png"); + std::fs::write(&image_path, TINY_PNG_BYTES)?; + + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::LocalImage { + path: image_path, + detail, + }], + ..Default::default() + }) + .await?; + let turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + let TurnStartResponse { turn } = to_response::(turn_resp)?; + assert!(!turn.id.is_empty()); + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + received_response_input_images(&server).await +} + +async fn received_response_input_images(server: &wiremock::MockServer) -> Result> { + let requests = server + .received_requests() + .await + .context("failed to fetch received requests")?; + let mut input_images = Vec::new(); + + for request in requests { + if !request.url.path().ends_with("/responses") { + continue; + } + let body = request + .body_json::() + .context("request body should be JSON")?; + let Some(input) = body.get("input").and_then(Value::as_array) else { + continue; + }; + + for item in input { + if item.get("type").and_then(Value::as_str) != Some("message") { + continue; + } + let Some(content) = item.get("content").and_then(Value::as_array) else { + continue; + }; + input_images.extend( + content + .iter() + .filter(|span| span.get("type").and_then(Value::as_str) == Some("input_image")) + .cloned(), + ); + } + } + + Ok(input_images) +} + #[tokio::test] async fn turn_start_sends_originator_header() -> Result<()> { let responses = vec![create_final_assistant_message_sse_response("Done")?]; @@ -554,6 +663,7 @@ async fn turn_start_tracks_turn_event_analytics() -> Result<()> { thread_id: thread.id.clone(), input: vec![V2UserInput::Image { url: "https://example.com/a.png".to_string(), + detail: None, }], ..Default::default() }) @@ -1465,63 +1575,27 @@ async fn turn_start_uses_migrated_pragmatic_personality_without_override_v2() -> } #[tokio::test] -async fn turn_start_accepts_local_image_input() -> Result<()> { - // Two Codex turns hit the mock model (session start + turn/start). - let responses = vec![ - create_final_assistant_message_sse_response("Done")?, - create_final_assistant_message_sse_response("Done")?, - ]; - // Use the unchecked variant because the request payload includes a LocalImage - // which the strict matcher does not currently cover. - let server = create_mock_responses_server_sequence_unchecked(responses).await; +async fn turn_start_defaults_local_image_detail_to_high() -> Result<()> { + let input_images = run_local_image_turn(/*detail*/ None).await?; - let codex_home = TempDir::new()?; - create_config_toml( - codex_home.path(), - &server.uri(), - "never", - &BTreeMap::default(), - )?; + assert_eq!(input_images.len(), 1); + assert_eq!( + input_images[0].get("detail").and_then(Value::as_str), + Some("high") + ); - let mut mcp = McpProcess::new(codex_home.path()).await?; - timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + Ok(()) +} - let thread_req = mcp - .send_thread_start_request(ThreadStartParams { - model: Some("mock-model".to_string()), - ..Default::default() - }) - .await?; - let thread_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), - ) - .await??; - let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; +#[tokio::test] +async fn turn_start_forwards_custom_local_image_detail() -> Result<()> { + let input_images = run_local_image_turn(Some(ImageDetail::Original)).await?; - let image_path = codex_home.path().join("image.png"); - // No need to actually write the file; we just exercise the input path. - - let turn_req = mcp - .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), - input: vec![V2UserInput::LocalImage { path: image_path }], - ..Default::default() - }) - .await?; - let turn_resp: JSONRPCResponse = timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), - ) - .await??; - let TurnStartResponse { turn } = to_response::(turn_resp)?; - assert!(!turn.id.is_empty()); - - timeout( - DEFAULT_READ_TIMEOUT, - mcp.read_stream_until_notification_message("turn/completed"), - ) - .await??; + assert_eq!(input_images.len(), 1); + assert_eq!( + input_images[0].get("detail").and_then(Value::as_str), + Some("original") + ); Ok(()) } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 87ac5d6676..7ffdff0011 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1646,7 +1646,7 @@ async fn run_debug_prompt_input_command( .images .into_iter() .chain(cmd.images) - .map(|path| UserInput::LocalImage { path }) + .map(|path| UserInput::LocalImage { path, detail: None }) .collect::>(); if let Some(prompt) = cmd.prompt.or(interactive.prompt) { input.push(UserInput::Text { diff --git a/codex-rs/code-mode/src/description.rs b/codex-rs/code-mode/src/description.rs index 0c2813e51a..18d3c4555c 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 } | 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 detail with `_meta: { "codex/imageDetail": "original" }`. When provided, the second `detail` argument overrides any detail embedded in the first argument. +- `image(imageUrlOrItem: string | { image_url: string; detail?: "high" | "original" | null } | ImageContent, detail?: "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 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/response.rs b/codex-rs/code-mode/src/response.rs index 0ac3a03770..ae92639cc0 100644 --- a/codex-rs/code-mode/src/response.rs +++ b/codex-rs/code-mode/src/response.rs @@ -4,8 +4,6 @@ use serde::Serialize; #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "lowercase")] pub enum ImageDetail { - Auto, - Low, High, Original, } diff --git a/codex-rs/code-mode/src/runtime/value.rs b/codex-rs/code-mode/src/runtime/value.rs index 8d76a832d3..865b5a569d 100644 --- a/codex-rs/code-mode/src/runtime/value.rs +++ b/codex-rs/code-mode/src/runtime/value.rs @@ -71,14 +71,10 @@ pub(super) fn normalize_output_image( Some(detail) => { let normalized = detail.to_ascii_lowercase(); Some(match normalized.as_str() { - "auto" => ImageDetail::Auto, - "low" => ImageDetail::Low, "high" => ImageDetail::High, "original" => ImageDetail::Original, _ => { - return Err( - "image detail must be one of: auto, low, high, original".to_string() - ); + return Err("image detail must be one of: high, original".to_string()); } }) } @@ -160,7 +156,7 @@ fn parse_mcp_output_image( .and_then(JsonValue::as_object) .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) .and_then(JsonValue::as_str) - .filter(|detail| matches!(*detail, "auto" | "low" | "high" | "original")) + .filter(|detail| matches!(*detail, "high" | "original")) .map(str::to_string); Ok((image_url, detail)) } diff --git a/codex-rs/code-mode/src/service.rs b/codex-rs/code-mode/src/service.rs index 44e4be4939..1544f84f36 100644 --- a/codex-rs/code-mode/src/service.rs +++ b/codex-rs/code-mode/src/service.rs @@ -1308,7 +1308,7 @@ image({ image( { image_url: "https://example.com/image.jpg", - detail: "low", + detail: "high", }, "original", ); @@ -1348,7 +1348,7 @@ image( mimeType: "image/png", _meta: { "codex/imageDetail": "original" }, }, - "low", + "high", ); "# .to_string(), @@ -1364,7 +1364,7 @@ image( 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), + detail: Some(crate::ImageDetail::High), }], stored_values: HashMap::new(), error_text: None, @@ -1372,6 +1372,36 @@ image( ); } + #[tokio::test] + async fn image_helper_rejects_unsupported_detail() { + let service = CodeModeService::new(); + + let response = service + .execute(ExecuteRequest { + source: r#" +image({ + image_url: "https://example.com/image.jpg", + detail: "low", +}); +"# + .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 detail must be one of: high, original".to_string()), + } + ); + } + #[tokio::test] async fn image_helper_rejects_raw_mcp_result_container() { let service = CodeModeService::new(); diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 329c6048b0..71bc026a13 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -1235,7 +1235,9 @@ pub(crate) fn render_input_preview(initial_operation: &Op) -> String { .map(|item| match item { UserInput::Text { text, .. } => text.clone(), UserInput::Image { .. } => "[image]".to_string(), - UserInput::LocalImage { path } => format!("[local_image:{}]", path.display()), + UserInput::LocalImage { path, .. } => { + format!("[local_image:{}]", path.display()) + } UserInput::Skill { name, path } => format!("[skill:${name}]({})", path.display()), UserInput::Mention { name, path } => format!("[mention:${name}]({path})"), _ => "[input]".to_string(), diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index e7c79e6dd2..d3cae7feed 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -90,9 +90,10 @@ fn parse_user_message(message: &[ContentItem]) -> Option { text_elements: Vec::new(), }); } - ContentItem::InputImage { image_url, .. } => { + ContentItem::InputImage { image_url, detail } => { content.push(UserInput::Image { image_url: image_url.clone(), + detail: *detail, }); } ContentItem::OutputText { text } => { diff --git a/codex-rs/core/src/event_mapping_tests.rs b/codex-rs/core/src/event_mapping_tests.rs index a70b2a69b0..c8311440eb 100644 --- a/codex-rs/core/src/event_mapping_tests.rs +++ b/codex-rs/core/src/event_mapping_tests.rs @@ -48,8 +48,14 @@ fn parses_user_message_with_text_and_two_images() { text: "Hello world".to_string(), text_elements: Vec::new(), }, - UserInput::Image { image_url: img1 }, - UserInput::Image { image_url: img2 }, + UserInput::Image { + image_url: img1, + detail: Some(DEFAULT_IMAGE_DETAIL), + }, + UserInput::Image { + image_url: img2, + detail: Some(DEFAULT_IMAGE_DETAIL), + }, ]; assert_eq!(user.content, expected_content); } @@ -87,7 +93,10 @@ fn skips_local_image_label_text() { match turn_item { TurnItem::UserMessage(user) => { let expected_content = vec![ - UserInput::Image { image_url }, + UserInput::Image { + image_url, + detail: Some(DEFAULT_IMAGE_DETAIL), + }, UserInput::Text { text: user_text, text_elements: Vec::new(), @@ -165,7 +174,10 @@ fn skips_unnamed_image_label_text() { match turn_item { TurnItem::UserMessage(user) => { let expected_content = vec![ - UserInput::Image { image_url }, + UserInput::Image { + image_url, + detail: Some(DEFAULT_IMAGE_DETAIL), + }, UserInput::Text { text: user_text, text_elements: Vec::new(), diff --git a/codex-rs/core/src/personality_migration_tests.rs b/codex-rs/core/src/personality_migration_tests.rs index 699e06fe67..b84d6dd3eb 100644 --- a/codex-rs/core/src/personality_migration_tests.rs +++ b/codex-rs/core/src/personality_migration_tests.rs @@ -72,6 +72,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), }; diff --git a/codex-rs/core/src/session/rollout_reconstruction_tests.rs b/codex-rs/core/src/session/rollout_reconstruction_tests.rs index 143b23d3a3..aa403316e0 100644 --- a/codex-rs/core/src/session/rollout_reconstruction_tests.rs +++ b/codex-rs/core/src/session/rollout_reconstruction_tests.rs @@ -143,6 +143,7 @@ async fn record_initial_history_resumed_hydrates_previous_turn_settings_from_lif images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item), @@ -209,6 +210,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(first_context_item.clone()), @@ -237,6 +239,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_com images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(rolled_back_context_item), @@ -307,6 +310,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(first_context_item.clone()), @@ -335,6 +339,7 @@ async fn reconstruct_history_rollback_keeps_history_and_metadata_in_sync_for_inc images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::ResponseItem(turn_two_user), @@ -397,6 +402,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(first_context_item.clone()), @@ -425,6 +431,7 @@ async fn reconstruct_history_rollback_skips_non_user_turns_for_history_and_metad images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::ResponseItem(turn_two_user), @@ -515,6 +522,7 @@ async fn reconstruct_history_rollback_counts_inter_agent_assistant_turns() { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(first_context_item.clone()), @@ -603,6 +611,7 @@ async fn reconstruct_history_rollback_clears_history_and_metadata_when_exceeding images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(only_context_item), @@ -655,6 +664,7 @@ async fn record_initial_history_resumed_rollback_skips_only_user_turns() { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item), @@ -727,6 +737,7 @@ async fn record_initial_history_resumed_rollback_drops_incomplete_user_turn_comp images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item.clone()), @@ -753,6 +764,7 @@ async fn record_initial_history_resumed_rollback_drops_incomplete_user_turn_comp images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::Compacted(CompactedItem { @@ -884,6 +896,7 @@ async fn reconstruct_history_legacy_compaction_without_replacement_history_clear images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(current_context_item), @@ -952,6 +965,7 @@ async fn record_initial_history_resumed_turn_context_after_compaction_reestablis images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), // Compaction clears baseline until a later TurnContextItem re-establishes it. @@ -1065,6 +1079,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item), @@ -1091,6 +1106,7 @@ async fn record_initial_history_resumed_aborted_turn_without_id_clears_active_tu images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::EventMsg(EventMsg::TurnAborted( @@ -1176,6 +1192,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item), @@ -1202,6 +1219,7 @@ async fn record_initial_history_resumed_unmatched_abort_preserves_active_turn_fo images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::EventMsg(EventMsg::TurnAborted( @@ -1296,6 +1314,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item), @@ -1322,6 +1341,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_compaction_clea images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::Compacted(CompactedItem { @@ -1372,6 +1392,7 @@ async fn record_initial_history_resumed_trailing_incomplete_turn_preserves_turn_ images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(current_context_item.clone()), @@ -1450,6 +1471,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item), @@ -1476,6 +1498,7 @@ async fn record_initial_history_resumed_replaced_incomplete_compacted_turn_clear images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::Compacted(CompactedItem { diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index aa89efe1d4..cad702bca7 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -2364,6 +2364,7 @@ async fn record_initial_history_forked_hydrates_previous_turn_settings() { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(previous_context_item.clone()), @@ -2558,6 +2559,7 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(first_context_item.clone()), @@ -2584,6 +2586,7 @@ async fn thread_rollback_recomputes_previous_turn_settings_and_reference_context images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, )), RolloutItem::TurnContext(rolled_back_context_item), @@ -2666,6 +2669,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::TurnContext(first_context_item.clone()), RolloutItem::ResponseItem(user_message("turn 1 user")), @@ -2709,6 +2713,7 @@ async fn thread_rollback_restores_cleared_reference_context_item_after_compactio images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::TurnContext(TurnContextItem { turn_id: Some(rolled_back_turn_id.clone()), @@ -2763,6 +2768,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::TurnContext(turn_context_item.clone()), RolloutItem::ResponseItem(user_message("turn 1 user")), @@ -2787,6 +2793,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::TurnContext(turn_context_item.clone()), RolloutItem::ResponseItem(user_message("turn 2 user")), @@ -2811,6 +2818,7 @@ async fn thread_rollback_persists_marker_and_replays_cumulatively() { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), RolloutItem::TurnContext(turn_context_item), RolloutItem::ResponseItem(user_message("turn 3 user")), @@ -7521,6 +7529,7 @@ async fn record_context_updates_and_set_reference_context_item_persists_full_rei images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, ))]) .await; @@ -7946,6 +7955,7 @@ async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input() images, text_elements, local_images, + .. }) if message == "late pending input" && images == Some(Vec::new()) && text_elements.is_empty() diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 9fedff9470..956486f182 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1127,6 +1127,7 @@ fn completed_legacy_event_history_is_not_mid_turn() { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() })), RolloutItem::EventMsg(EventMsg::AgentMessage(AgentMessageEvent { message: "done".to_string(), @@ -1154,6 +1155,7 @@ fn mixed_response_and_legacy_user_event_history_is_mid_turn() { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() })), ]); diff --git a/codex-rs/core/src/tools/code_mode/response_adapter.rs b/codex-rs/core/src/tools/code_mode/response_adapter.rs index e20cf6a071..133d6e2fc1 100644 --- a/codex-rs/core/src/tools/code_mode/response_adapter.rs +++ b/codex-rs/core/src/tools/code_mode/response_adapter.rs @@ -17,8 +17,6 @@ impl IntoProtocol for CodeModeImageDetail { fn into_protocol(self) -> ImageDetail { let value = self; match value { - CodeModeImageDetail::Auto => ImageDetail::Auto, - CodeModeImageDetail::Low => ImageDetail::Low, CodeModeImageDetail::High => ImageDetail::High, CodeModeImageDetail::Original => ImageDetail::Original, } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 7ad6a79829..c3e6792c06 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -60,6 +60,7 @@ struct ViewImageArgs { #[derive(Clone, Copy, Eq, PartialEq)] enum ViewImageDetail { + High, Original, } @@ -114,16 +115,15 @@ impl ToolExecutor for ViewImageHandler { environment_id, detail, } = parse_arguments(&arguments)?; - // `view_image` accepts only its documented detail values: omit - // `detail` for the default path or set it to `original`. - // Other string values remain invalid rather than being silently - // reinterpreted. + // `high` is the explicit spelling of the default resized path. + // Other string values remain invalid rather than being silently reinterpreted. let detail = match detail.as_deref() { None => None, + Some("high") => Some(ViewImageDetail::High), Some("original") => Some(ViewImageDetail::Original), Some(detail) => { return Err(FunctionCallError::RespondToModel(format!( - "view_image.detail only supports `original`; omit `detail` for default resized behavior, got `{detail}`" + "view_image.detail only supports `high` or `original`; omit `detail` for default high resized behavior, got `{detail}`" ))); } }; @@ -175,11 +175,11 @@ impl ToolExecutor for ViewImageHandler { } else { PromptImageMode::ResizeToFit }; - let image_detail = Some(if use_original_detail { + let image_detail = if use_original_detail { ImageDetail::Original } else { DEFAULT_IMAGE_DETAIL - }); + }; let image = load_for_prompt_bytes(abs_path.as_path(), file_bytes, image_mode).map_err(|error| { @@ -208,7 +208,7 @@ impl CoreToolRuntime for ViewImageHandler {} pub struct ViewImageOutput { image_url: String, - image_detail: Option, + image_detail: ImageDetail, } impl ToolOutput for ViewImageOutput { @@ -224,7 +224,7 @@ impl ToolOutput for ViewImageOutput { let body = FunctionCallOutputBody::ContentItems(vec![FunctionCallOutputContentItem::InputImage { image_url: self.image_url.clone(), - detail: self.image_detail, + detail: Some(self.image_detail), }]); let output = FunctionCallOutputPayload { body, @@ -263,7 +263,7 @@ mod tests { fn code_mode_result_returns_image_url_object() { let output = ViewImageOutput { image_url: "data:image/png;base64,AAA".to_string(), - image_detail: Some(DEFAULT_IMAGE_DETAIL), + image_detail: DEFAULT_IMAGE_DETAIL, }; let result = output.code_mode_result(&ToolPayload::Function { @@ -317,4 +317,68 @@ mod tests { "{message}" ); } + + #[tokio::test] + async fn handle_rejects_unsupported_detail() { + let (session, turn) = make_session_and_context().await; + + let result = ViewImageHandler::default() + .handle(ToolInvocation { + session: Arc::new(session), + turn: Arc::new(turn), + cancellation_token: tokio_util::sync::CancellationToken::new(), + tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), + call_id: "call-view-image".to_string(), + tool_name: codex_tools::ToolName::plain("view_image"), + source: ToolCallSource::Direct, + payload: ToolPayload::Function { + arguments: json!({ "path": "image.png", "detail": "low" }).to_string(), + }, + }) + .await; + + let Err(FunctionCallError::RespondToModel(message)) = result else { + panic!("expected unsupported detail error"); + }; + assert_eq!( + message, + "view_image.detail only supports `high` or `original`; omit `detail` for default high resized behavior, got `low`" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn handle_accepts_explicit_high_detail() { + let (session, mut turn) = make_session_and_context().await; + let image_dir = tempfile::tempdir().expect("create image temp dir"); + let image_cwd = image_dir.abs(); + + turn.environments + .turn_environments + .first_mut() + .expect("default local turn environment") + .cwd = image_cwd.clone(); + let image_path = image_cwd.join("image.png"); + std::fs::write(image_path.as_path(), b"not a real image").expect("write test image"); + turn.permission_profile = PermissionProfile::Disabled; + + let result = ViewImageHandler::default() + .handle(ToolInvocation { + session: Arc::new(session), + turn: Arc::new(turn), + cancellation_token: tokio_util::sync::CancellationToken::new(), + tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), + call_id: "call-view-image".to_string(), + tool_name: codex_tools::ToolName::plain("view_image"), + source: ToolCallSource::Direct, + payload: ToolPayload::Function { + arguments: json!({ "path": "image.png", "detail": "high" }).to_string(), + }, + }) + .await; + + let Err(FunctionCallError::RespondToModel(message)) = result else { + panic!("expected image processing error"); + }; + assert!(message.contains("unable to process image"), "{message}"); + } } diff --git a/codex-rs/core/src/tools/handlers/view_image_spec.rs b/codex-rs/core/src/tools/handlers/view_image_spec.rs index 7d1422a037..662cd639a6 100644 --- a/codex-rs/core/src/tools/handlers/view_image_spec.rs +++ b/codex-rs/core/src/tools/handlers/view_image_spec.rs @@ -20,9 +20,12 @@ pub fn create_view_image_tool(options: ViewImageToolOptions) -> ToolSpec { if options.can_request_original_image_detail { properties.insert( "detail".to_string(), - JsonSchema::string(Some( - "Optional detail override. The only supported value is `original`; omit this field for default resized behavior. Use `original` to preserve the file's original resolution instead of resizing to fit. This is important when high-fidelity image perception or precise localization is needed, especially for CUA agents.".to_string(), - )), + JsonSchema::string_enum( + vec![json!("high"), json!("original")], + Some( + "Optional detail override. Supported values are `high` and `original`; omit this field for default high resized behavior. Use `original` to preserve the file's original resolution instead of resizing to fit. This is important when high-fidelity image perception or precise localization is needed, especially for CUA agents.".to_string(), + ), + ), ); } if options.include_environment_id { @@ -55,8 +58,9 @@ fn view_image_output_schema() -> Value { "description": "Data URL for the loaded image." }, "detail": { - "type": ["string", "null"], - "description": "Image detail hint returned by view_image. Returns `original` when original resolution is preserved, otherwise `null`." + "type": "string", + "enum": ["high", "original"], + "description": "Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved." } }, "required": ["image_url", "detail"], diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 063d75e6e8..333efe6f06 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -711,13 +711,14 @@ fn view_image_tool_includes_detail_with_original_detail_support() { }; let (properties, _) = expect_object_schema(parameters); assert!(properties.contains_key("detail")); - let description = expect_string_description( - properties - .get("detail") - .expect("view_image detail should include a description"), - ); - assert!(description.contains("only supported value is `original`")); - assert!(description.contains("omit this field for default resized behavior")); + let detail_schema = properties + .get("detail") + .expect("view_image detail should include a description"); + let description = expect_string_description(detail_schema); + let expected = vec![json!("high"), json!("original")]; + assert_eq!(detail_schema.enum_values.as_ref(), Some(&expected)); + assert!(description.contains("Supported values are `high` and `original`")); + assert!(description.contains("omit this field for default high resized behavior")); } #[test] @@ -2239,7 +2240,7 @@ fn code_mode_augments_builtin_tool_descriptions_with_typed_sample() { assert_eq!( description, - "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within tags).\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `original` when original resolution is preserved, otherwise `null`.\n detail: string | null;\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```" + "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within tags).\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved.\n detail: \"high\" | \"original\";\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```" ); } diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index ff7640f05e..f117631c4a 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -2466,7 +2466,7 @@ text(JSON.stringify(tool)); parsed, serde_json::json!({ "name": "view_image", - "description": "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within tags).\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `original` when original resolution is preserved, otherwise `null`.\n detail: string | null;\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```", + "description": "View a local image from the filesystem (only use if given a full filepath by the user, and the image isn't already attached to the thread context within tags).\n\nexec tool declaration:\n```ts\ndeclare const tools: { view_image(args: {\n // Local filesystem path to an image file\n path: string;\n}): Promise<{\n // Image detail hint returned by view_image. Returns `high` for default resized behavior or `original` when original resolution is preserved.\n detail: \"high\" | \"original\";\n // Data URL for the loaded image.\n image_url: string;\n}>; };\n```", }) ); diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index 68ddf1691b..be3423a85e 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -3272,6 +3272,7 @@ async fn snapshot_request_shape_pre_turn_compaction_including_incoming_user_mess items: vec![ UserInput::Image { image_url: image_url.clone(), + detail: None, }, UserInput::Text { text: "USER_THREE".to_string(), diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 720ae10599..15197042fa 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -556,7 +556,10 @@ async fn assert_remote_manual_compact_request_parity( .submit(Op::UserInput { environments: None, items: vec![ - UserInput::Image { image_url }, + UserInput::Image { + image_url, + detail: None, + }, UserInput::Text { text: "TURN_FOUR_IMAGE_USER".to_string(), text_elements: Vec::new(), diff --git a/codex-rs/core/tests/suite/compact_remote_parity.rs b/codex-rs/core/tests/suite/compact_remote_parity.rs index 96616fa9cb..32c13b0b27 100644 --- a/codex-rs/core/tests/suite/compact_remote_parity.rs +++ b/codex-rs/core/tests/suite/compact_remote_parity.rs @@ -621,6 +621,7 @@ fn user_input_for_step(scenario_name: &str, idx: usize, step: Step) -> Vec anyhow::Resu items: vec![ UserInput::LocalImage { path: abs_path.clone(), + detail: None, }, UserInput::Text { text: "pasted image".to_string(), @@ -208,6 +209,7 @@ async fn drag_drop_image_persists_rollout_request_shape() -> anyhow::Result<()> items: vec![ UserInput::Image { image_url: image_url.clone(), + detail: None, }, UserInput::Text { text: "dropped image".to_string(), diff --git a/codex-rs/core/tests/suite/model_switching.rs b/codex-rs/core/tests/suite/model_switching.rs index 21ab43f10d..adce6e36eb 100644 --- a/codex-rs/core/tests/suite/model_switching.rs +++ b/codex-rs/core/tests/suite/model_switching.rs @@ -439,6 +439,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< vec![ UserInput::Image { image_url: image_url.clone(), + detail: None, }, UserInput::Text { text: "first turn".to_string(), diff --git a/codex-rs/core/tests/suite/personality_migration.rs b/codex-rs/core/tests/suite/personality_migration.rs index 0b89a9cfba..5151d4e55d 100644 --- a/codex-rs/core/tests/suite/personality_migration.rs +++ b/codex-rs/core/tests/suite/personality_migration.rs @@ -88,6 +88,7 @@ async fn write_rollout_with_user_event(dir: &Path, thread_id: ThreadId) -> io::R images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), }; diff --git a/codex-rs/core/tests/suite/resume_warning.rs b/codex-rs/core/tests/suite/resume_warning.rs index cb545df351..ac916ba66d 100644 --- a/codex-rs/core/tests/suite/resume_warning.rs +++ b/codex-rs/core/tests/suite/resume_warning.rs @@ -64,6 +64,7 @@ fn resume_history( images: None, local_images: vec![], text_elements: vec![], + ..Default::default() })), RolloutItem::TurnContext(turn_ctx), RolloutItem::EventMsg(EventMsg::TurnComplete(TurnCompleteEvent { diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index 0cc7d07b16..ab0b4c27ea 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -168,6 +168,7 @@ async fn backfill_scans_existing_rollouts() -> Result<()> { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), }, ]; diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 5d5673b011..d78e8761fd 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -201,6 +201,7 @@ async fn assert_user_turn_local_image_resizes_to( &test, vec![UserInput::LocalImage { path: abs_path.clone(), + detail: None, }], session_model, )) @@ -814,7 +815,7 @@ async fn view_image_tool_errors_clearly_for_unsupported_detail_values() -> anyho .expect("output text present"); assert_eq!( output_text, - "view_image.detail only supports `original`; omit `detail` for default resized behavior, got `low`" + "view_image.detail only supports `high` or `original`; omit `detail` for default high resized behavior, got `low`" ); assert!( @@ -1487,6 +1488,7 @@ async fn replaces_invalid_local_image_after_bad_request() -> anyhow::Result<()> &test, vec![UserInput::LocalImage { path: abs_path.clone(), + detail: None, }], session_model, )) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 1004124ba5..3e2cf21051 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -637,7 +637,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { let mut items: Vec = imgs .into_iter() .chain(args.images.iter().cloned()) - .map(|path| UserInput::LocalImage { path }) + .map(|path| UserInput::LocalImage { path, detail: None }) .collect(); items.push(UserInput::Text { text: prompt_text.clone(), @@ -657,7 +657,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { let prompt_text = resolve_root_prompt(root_prompt); let mut items: Vec = imgs .into_iter() - .map(|path| UserInput::LocalImage { path }) + .map(|path| UserInput::LocalImage { path, detail: None }) .collect(); items.push(UserInput::Text { text: prompt_text.clone(), diff --git a/codex-rs/external-agent-sessions/src/export.rs b/codex-rs/external-agent-sessions/src/export.rs index 8f34f6ba5a..3682a4f7f8 100644 --- a/codex-rs/external-agent-sessions/src/export.rs +++ b/codex-rs/external-agent-sessions/src/export.rs @@ -81,6 +81,7 @@ fn rollout_items_from_messages(messages: &[ConversationMessage]) -> Vec Some(image_url.clone()), + UserInput::Image { image_url, .. } => Some(image_url.clone()), _ => None, }) .collect() } + pub fn image_details(&self) -> Vec> { + trim_trailing_default_image_details( + self.content + .iter() + .filter_map(|c| match c { + UserInput::Image { detail, .. } => Some(*detail), + _ => None, + }) + .collect(), + ) + } + pub fn local_image_paths(&self) -> Vec { self.content .iter() .filter_map(|c| match c { - UserInput::LocalImage { path } => Some(path.clone()), + UserInput::LocalImage { path, .. } => Some(path.clone()), _ => None, }) .collect() } + + pub fn local_image_details(&self) -> Vec> { + trim_trailing_default_image_details( + self.content + .iter() + .filter_map(|c| match c { + UserInput::LocalImage { detail, .. } => Some(*detail), + _ => None, + }) + .collect(), + ) + } +} + +fn trim_trailing_default_image_details( + mut details: Vec>, +) -> Vec> { + while matches!(details.last(), Some(None)) { + details.pop(); + } + details } impl HookPromptItem { diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 2706a47c82..48a18a5cd9 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -721,8 +721,6 @@ pub enum ContentItem { #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "lowercase")] pub enum ImageDetail { - Auto, - Low, High, Original, } @@ -1064,8 +1062,13 @@ pub fn local_image_content_items_with_label_number( path: &std::path::Path, file_bytes: Vec, label_number: Option, - mode: PromptImageMode, + detail: ImageDetail, ) -> Vec { + let mode = match detail { + ImageDetail::Original => PromptImageMode::Original, + ImageDetail::High => PromptImageMode::ResizeToFit, + }; + match load_for_prompt_bytes(path, file_bytes, mode) { Ok(image) => { let mut items = Vec::with_capacity(3); @@ -1076,7 +1079,7 @@ pub fn local_image_content_items_with_label_number( } items.push(ContentItem::InputImage { image_url: image.into_data_url(), - detail: Some(DEFAULT_IMAGE_DETAIL), + detail: Some(detail), }); if label_number.is_some() { items.push(ContentItem::InputText { @@ -1221,29 +1224,31 @@ impl From> for ResponseInputItem { .into_iter() .flat_map(|c| match c { UserInput::Text { text, .. } => vec![ContentItem::InputText { text }], - UserInput::Image { image_url } => { + UserInput::Image { image_url, detail } => { image_index += 1; + let detail = detail.unwrap_or(DEFAULT_IMAGE_DETAIL); vec![ ContentItem::InputText { text: image_open_tag_text(), }, ContentItem::InputImage { image_url, - detail: Some(DEFAULT_IMAGE_DETAIL), + detail: Some(detail), }, ContentItem::InputText { text: image_close_tag_text(), }, ] } - UserInput::LocalImage { path } => { + UserInput::LocalImage { path, detail } => { image_index += 1; + let detail = detail.unwrap_or(DEFAULT_IMAGE_DETAIL); match std::fs::read(&path) { Ok(file_bytes) => local_image_content_items_with_label_number( &path, file_bytes, Some(image_index), - PromptImageMode::ResizeToFit, + detail, ), Err(err) => vec![local_image_error_placeholder(&path, err)], } @@ -1587,8 +1592,6 @@ fn convert_mcp_content_to_items( .and_then(|meta| meta.get(CODEX_IMAGE_DETAIL_META_KEY)) .and_then(serde_json::Value::as_str) .and_then(|detail| match detail { - "auto" => Some(ImageDetail::Auto), - "low" => Some(ImageDetail::Low), "high" => Some(ImageDetail::High), "original" => Some(ImageDetail::Original), _ => None, @@ -1633,6 +1636,14 @@ mod tests { use std::path::PathBuf; use tempfile::tempdir; + // A tiny valid PNG (1x1) so image conversion tests don't depend on cross-crate + // file paths, which break under Bazel sandboxing. + const TINY_PNG_BYTES: &[u8] = &[ + 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, + 0, 0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, 0, 0, 5, 0, + 1, 122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130, + ]; + #[test] fn response_input_message_conversion_preserves_phase() { let item = ResponseItem::from(ResponseInputItem::Message { @@ -2545,6 +2556,7 @@ mod tests { let item = ResponseInputItem::from(vec![UserInput::Image { image_url: image_url.clone(), + detail: None, }]); match item { @@ -2569,6 +2581,31 @@ mod tests { Ok(()) } + #[test] + fn image_user_input_preserves_requested_detail() -> Result<()> { + let image_url = "data:image/png;base64,abc".to_string(); + + let item = ResponseInputItem::from(vec![UserInput::Image { + image_url: image_url.clone(), + detail: Some(ImageDetail::Original), + }]); + + match item { + ResponseInputItem::Message { content, .. } => { + assert_eq!( + content.get(1), + Some(&ContentItem::InputImage { + image_url, + detail: Some(ImageDetail::Original), + }) + ); + } + other => panic!("expected message response but got {other:?}"), + } + + Ok(()) + } + #[test] fn tool_search_call_roundtrips() -> Result<()> { let parsed: ResponseItem = serde_json::from_str( @@ -2737,20 +2774,17 @@ mod tests { let image_url = "data:image/png;base64,abc".to_string(); let dir = tempdir()?; let local_path = dir.path().join("local.png"); - // A tiny valid PNG (1x1) so this test doesn't depend on cross-crate file paths, which - // break under Bazel sandboxing. - const TINY_PNG_BYTES: &[u8] = &[ - 137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, - 8, 6, 0, 0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, - 0, 0, 5, 0, 1, 122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130, - ]; std::fs::write(&local_path, TINY_PNG_BYTES)?; let item = ResponseInputItem::from(vec![ UserInput::Image { image_url: image_url.clone(), + detail: None, + }, + UserInput::LocalImage { + path: local_path, + detail: None, }, - UserInput::LocalImage { path: local_path }, ]); match item { @@ -2797,6 +2831,33 @@ mod tests { Ok(()) } + #[test] + fn local_image_user_input_preserves_requested_detail() -> Result<()> { + let dir = tempdir()?; + let local_path = dir.path().join("local.png"); + std::fs::write(&local_path, TINY_PNG_BYTES)?; + + let item = ResponseInputItem::from(vec![UserInput::LocalImage { + path: local_path, + detail: Some(ImageDetail::Original), + }]); + + match item { + ResponseInputItem::Message { content, .. } => { + assert!(matches!( + content.get(1), + Some(ContentItem::InputImage { + detail: Some(ImageDetail::Original), + .. + }) + )); + } + other => panic!("expected message response but got {other:?}"), + } + + Ok(()) + } + #[test] fn local_image_read_error_adds_placeholder() -> Result<()> { let dir = tempdir()?; @@ -2804,6 +2865,7 @@ mod tests { let item = ResponseInputItem::from(vec![UserInput::LocalImage { path: missing_path.clone(), + detail: None, }]); match item { @@ -2838,6 +2900,7 @@ mod tests { let item = ResponseInputItem::from(vec![UserInput::LocalImage { path: json_path.clone(), + detail: None, }]); match item { @@ -2875,6 +2938,7 @@ mod tests { let item = ResponseInputItem::from(vec![UserInput::LocalImage { path: svg_path.clone(), + detail: None, }]); match item { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index fac2616d04..94753780cb 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -34,6 +34,7 @@ use crate::memory_citation::MemoryCitation; use crate::models::ActivePermissionProfile; use crate::models::BaseInstructions; use crate::models::ContentItem; +use crate::models::ImageDetail; use crate::models::MessagePhase; use crate::models::PermissionProfile; use crate::models::ResponseInputItem; @@ -2225,7 +2226,7 @@ pub struct AgentMessageEvent { pub memory_citation: Option, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +#[derive(Debug, Clone, Default, Deserialize, Serialize, JsonSchema, TS)] pub struct UserMessageEvent { pub message: String, /// Image URLs sourced from `UserInput::Image`. These are safe @@ -2233,11 +2234,19 @@ pub struct UserMessageEvent { /// the model. #[serde(skip_serializing_if = "Option::is_none")] pub images: Option>, + /// Detail hints for `images`, indexed in parallel. Missing entries imply + /// default image detail behavior. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub image_details: Vec>, /// Local file paths sourced from `UserInput::LocalImage`. These are kept so /// the UI can reattach images when editing history, and should not be sent /// to the model or treated as API-ready URLs. #[serde(default)] pub local_images: Vec, + /// Detail hints for `local_images`, indexed in parallel. Missing entries + /// imply default image detail behavior. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub local_image_details: Vec>, /// UI-defined spans within `message` used to render or persist special elements. #[serde(default)] pub text_elements: Vec, @@ -5133,6 +5142,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }; let json_event = serde_json::to_value(event)?; @@ -5148,6 +5158,62 @@ mod tests { Ok(()) } + #[test] + fn user_message_event_deserializes_without_image_detail_fields() -> Result<()> { + let event: UserMessageEvent = serde_json::from_value(json!({ + "message": "hello", + "images": ["https://example.com/image.png"], + "local_images": ["/tmp/local.png"], + "text_elements": [], + }))?; + + assert_eq!(event.message, "hello"); + assert_eq!( + event.images, + Some(vec!["https://example.com/image.png".to_string()]) + ); + assert_eq!(event.image_details, Vec::>::new()); + assert_eq!(event.local_images, vec![PathBuf::from("/tmp/local.png")]); + assert_eq!(event.local_image_details, Vec::>::new()); + assert_eq!(event.text_elements, Vec::new()); + + Ok(()) + } + + #[test] + fn user_message_item_legacy_event_preserves_image_details() { + let local_path = PathBuf::from("/tmp/local.png"); + let item = UserMessageItem::new(&[ + crate::user_input::UserInput::Image { + image_url: "https://example.com/first.png".to_string(), + detail: Some(ImageDetail::Original), + }, + crate::user_input::UserInput::Image { + image_url: "https://example.com/second.png".to_string(), + detail: None, + }, + crate::user_input::UserInput::LocalImage { + path: local_path.clone(), + detail: Some(ImageDetail::Original), + }, + ]); + + let EventMsg::UserMessage(event) = item.as_legacy_event() else { + panic!("expected user message event"); + }; + + assert_eq!( + event.images, + Some(vec![ + "https://example.com/first.png".to_string(), + "https://example.com/second.png".to_string(), + ]) + ); + assert_eq!(event.image_details, vec![Some(ImageDetail::Original)]); + assert_eq!(event.local_images, vec![local_path]); + assert_eq!(event.local_image_details, vec![Some(ImageDetail::Original)]); + } + #[test] fn turn_aborted_event_deserializes_without_turn_id() -> Result<()> { let event: EventMsg = serde_json::from_value(json!({ diff --git a/codex-rs/protocol/src/user_input.rs b/codex-rs/protocol/src/user_input.rs index 4ed112df8d..ce4cf99eba 100644 --- a/codex-rs/protocol/src/user_input.rs +++ b/codex-rs/protocol/src/user_input.rs @@ -3,6 +3,8 @@ use serde::Deserialize; use serde::Serialize; use ts_rs::TS; +use crate::models::ImageDetail; + /// Conservative cap so one user message cannot monopolize a large context window. pub const MAX_USER_INPUT_TEXT_CHARS: usize = 1 << 20; @@ -21,11 +23,21 @@ pub enum UserInput { text_elements: Vec, }, /// Pre‑encoded data: URI image. - Image { image_url: String }, + Image { + image_url: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + detail: Option, + }, /// Local image path provided by the user. This will be converted to an /// `Image` variant (base64 data URL) during request serialization. - LocalImage { path: std::path::PathBuf }, + LocalImage { + path: std::path::PathBuf, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + detail: Option, + }, /// Skill selected by the user (name + path to SKILL.md). Skill { diff --git a/codex-rs/rollout/src/recorder_tests.rs b/codex-rs/rollout/src/recorder_tests.rs index ede7f720a9..8199f290d8 100644 --- a/codex-rs/rollout/src/recorder_tests.rs +++ b/codex-rs/rollout/src/recorder_tests.rs @@ -114,6 +114,7 @@ async fn state_db_init_backfills_before_returning() -> anyhow::Result<()> { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })), }, ]; @@ -404,6 +405,7 @@ async fn recorder_materializes_on_flush_with_pending_items() -> std::io::Result< images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() }, ))]) .await?; diff --git a/codex-rs/rollout/src/tests.rs b/codex-rs/rollout/src/tests.rs index 22f9a73a4e..bcd395d820 100644 --- a/codex-rs/rollout/src/tests.rs +++ b/codex-rs/rollout/src/tests.rs @@ -1404,6 +1404,7 @@ async fn test_updated_at_uses_file_mtime() -> Result<()> { images: None, text_elements: Vec::new(), local_images: Vec::new(), + ..Default::default() })), }; writeln!(file, "{}", serde_json::to_string(&user_event_line)?)?; diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index d815d444ce..886d267e4f 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -205,6 +205,7 @@ mod tests { images: Some(vec![]), local_images: vec![], text_elements: vec![], + ..Default::default() })); apply_rollout_item(&mut metadata, &item, "test-provider"); @@ -225,6 +226,7 @@ mod tests { images: Some(vec!["https://example.com/image.png".to_string()]), local_images: vec![], text_elements: vec![], + ..Default::default() })); apply_rollout_item(&mut metadata, &item, "test-provider"); @@ -248,6 +250,7 @@ mod tests { images: Some(vec![]), local_images: vec![], text_elements: vec![], + ..Default::default() })); apply_rollout_item(&mut metadata, &item, "test-provider"); @@ -287,6 +290,7 @@ mod tests { images: Some(vec![]), local_images: vec![], text_elements: vec![], + ..Default::default() })); apply_rollout_item(&mut metadata, &user_item, "test-provider"); diff --git a/codex-rs/thread-store/src/local/mod.rs b/codex-rs/thread-store/src/local/mod.rs index ab53e12bbe..f6a48a400b 100644 --- a/codex-rs/thread-store/src/local/mod.rs +++ b/codex-rs/thread-store/src/local/mod.rs @@ -1036,6 +1036,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() })) } diff --git a/codex-rs/thread-store/src/thread_metadata_sync.rs b/codex-rs/thread-store/src/thread_metadata_sync.rs index 37afda8bc2..edc78a36c2 100644 --- a/codex-rs/thread-store/src/thread_metadata_sync.rs +++ b/codex-rs/thread-store/src/thread_metadata_sync.rs @@ -546,6 +546,7 @@ mod tests { images: None, local_images: Vec::new(), text_elements: Vec::new(), + ..Default::default() } } diff --git a/codex-rs/tools/src/image_detail.rs b/codex-rs/tools/src/image_detail.rs index 37086f691d..145cda663c 100644 --- a/codex-rs/tools/src/image_detail.rs +++ b/codex-rs/tools/src/image_detail.rs @@ -16,7 +16,7 @@ pub fn normalize_output_image_detail( Some(ImageDetail::Original) } Some(ImageDetail::Original) | None => None, - Some(ImageDetail::Auto | ImageDetail::Low | ImageDetail::High) => detail, + Some(ImageDetail::High) => Some(ImageDetail::High), } } diff --git a/codex-rs/tools/src/image_detail_tests.rs b/codex-rs/tools/src/image_detail_tests.rs index 393a962ac4..919537acf7 100644 --- a/codex-rs/tools/src/image_detail_tests.rs +++ b/codex-rs/tools/src/image_detail_tests.rs @@ -70,10 +70,6 @@ fn explicit_original_is_dropped_without_model_support() { fn explicit_non_original_detail_is_preserved() { let model_info = model_info(); - assert_eq!( - normalize_output_image_detail(&model_info, Some(ImageDetail::Low)), - Some(ImageDetail::Low) - ); assert_eq!( normalize_output_image_detail(&model_info, Some(ImageDetail::High)), Some(ImageDetail::High) @@ -92,7 +88,7 @@ fn sanitize_original_falls_back_to_high_without_support() { }, FunctionCallOutputContentItem::InputImage { image_url: "data:image/png;base64,BBB".to_string(), - detail: Some(ImageDetail::Low), + detail: Some(ImageDetail::High), }, ]; @@ -110,7 +106,7 @@ fn sanitize_original_falls_back_to_high_without_support() { }, FunctionCallOutputContentItem::InputImage { image_url: "data:image/png;base64,BBB".to_string(), - detail: Some(ImageDetail::Low), + detail: Some(ImageDetail::High), }, ] ); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index bbc1802d17..c107bc777f 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -4727,7 +4727,7 @@ async fn backtrack_resubmit_preserves_data_image_urls_in_user_turn() { assert!(items.iter().any(|item| { matches!( item, - UserInput::Image { url } if url == &data_image_url + UserInput::Image { url, .. } if url == &data_image_url ) })); } diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index 6e9860bb77..2ead61f3dc 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -164,12 +164,14 @@ impl ChatWidget { for image_url in &remote_image_urls { items.push(UserInput::Image { url: image_url.clone(), + detail: None, }); } for image in &local_images { items.push(UserInput::LocalImage { path: image.path.clone(), + detail: None, }); } diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 96477c8d38..3e8635c46a 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -58,7 +58,8 @@ async fn submission_preserves_text_elements_and_local_images() { assert_eq!( items[0], UserInput::LocalImage { - path: local_images[0].clone() + path: local_images[0].clone(), + detail: None, } ); assert_eq!( @@ -259,12 +260,14 @@ async fn submission_with_remote_and_local_images_keeps_local_placeholder_numberi items[0], UserInput::Image { url: remote_url.clone(), + detail: None, } ); assert_eq!( items[1], UserInput::LocalImage { path: local_images[0].clone(), + detail: None, } ); assert_eq!( @@ -342,6 +345,7 @@ async fn enter_with_only_remote_images_submits_user_turn() { items, vec![UserInput::Image { url: remote_url.clone(), + detail: None, }] ); assert_eq!(summary, None); @@ -1157,9 +1161,11 @@ fn user_message_display_from_inputs_matches_flattened_user_message_shape() { }, UserInput::Image { url: "https://example.com/remote.png".to_string(), + detail: None, }, UserInput::LocalImage { path: local_image.clone(), + detail: None, }, UserInput::Skill { name: "demo".to_string(), @@ -1232,6 +1238,7 @@ async fn committed_user_message_with_hidden_prompt_context_renders_local_images( }, UserInput::LocalImage { path: local_image.clone(), + detail: None, }, ], ); diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index 8282430306..1d1b19fdf5 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -119,6 +119,7 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() { }, AppServerUserInput::LocalImage { path: local_images[0].clone(), + detail: None, }, ], ReplayKind::ResumeInitialMessages, @@ -188,6 +189,7 @@ async fn replayed_user_message_preserves_remote_image_urls() { }, AppServerUserInput::Image { url: remote_image_urls[0].clone(), + detail: None, }, ], ReplayKind::ResumeInitialMessages, @@ -441,6 +443,7 @@ async fn replayed_user_message_with_only_remote_images_renders_history_cell() { "user-1", vec![AppServerUserInput::Image { url: remote_image_urls[0].clone(), + detail: None, }], ReplayKind::ResumeInitialMessages, ); @@ -496,6 +499,7 @@ async fn replayed_user_message_with_only_local_images_renders_history_cell() { "user-1", vec![AppServerUserInput::LocalImage { path: local_images[0].clone(), + detail: None, }], ReplayKind::ResumeInitialMessages, ); diff --git a/codex-rs/tui/src/chatwidget/tests/review_mode.rs b/codex-rs/tui/src/chatwidget/tests/review_mode.rs index 8c16e0b929..48bc341f6b 100644 --- a/codex-rs/tui/src/chatwidget/tests/review_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/review_mode.rs @@ -597,6 +597,7 @@ async fn item_completed_pops_pending_steer_with_local_image_and_text_elements() vec![ UserInput::Image { url: "data:image/png;base64,placeholder".to_string(), + detail: None, }, UserInput::Text { text, diff --git a/codex-rs/tui/src/chatwidget/user_messages.rs b/codex-rs/tui/src/chatwidget/user_messages.rs index bf3f9e8fe6..cdcf650cb1 100644 --- a/codex-rs/tui/src/chatwidget/user_messages.rs +++ b/codex-rs/tui/src/chatwidget/user_messages.rs @@ -593,8 +593,8 @@ impl ChatWidget { ) }), ), - UserInput::Image { url } => remote_image_urls.push(url.clone()), - UserInput::LocalImage { path } => local_images.push(path.clone()), + UserInput::Image { url, .. } => remote_image_urls.push(url.clone()), + UserInput::LocalImage { path, .. } => local_images.push(path.clone()), UserInput::Skill { .. } | UserInput::Mention { .. } => {} } } diff --git a/codex-rs/tui/src/ide_context/prompt.rs b/codex-rs/tui/src/ide_context/prompt.rs index ec7e165ba8..884ebfb646 100644 --- a/codex-rs/tui/src/ide_context/prompt.rs +++ b/codex-rs/tui/src/ide_context/prompt.rs @@ -269,6 +269,7 @@ mod tests { let mut items = vec![ UserInput::LocalImage { path: PathBuf::from("/tmp/screenshot.png"), + detail: None, }, UserInput::Text { text, @@ -288,6 +289,7 @@ mod tests { vec![ UserInput::LocalImage { path: PathBuf::from("/tmp/screenshot.png"), + detail: None, }, UserInput::Text { text: format!("{expected_prefix}Ask $figma"), From bbb5c2811dc6a38a6da6c6992bc57034da516d2c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 15 May 2026 15:42:35 -0700 Subject: [PATCH 5/6] tui: pass active permission profiles through app commands (#22891) ## Why This continues the permissions migration by keeping the TUI command boundary aligned with the app-server protocol direction from #22795: callers should select a permission profile by id instead of passing a concrete `PermissionProfile` value around as the turn configuration. `AppCommand` is internal to the TUI, but it is the path that eventually becomes `thread/turn/start`, so carrying concrete profile details there made it too easy for UI code to keep relying on the old whole-profile replacement model. ## What changed - `AppCommand::UserTurn` and `AppCommand::OverrideTurnContext` now carry `Option` instead of `PermissionProfile`. - Composer submissions copy the active permission profile id from the current session snapshot; legacy snapshots intentionally submit no active profile id. - Permission preset UI events now carry only the active built-in profile id. The app derives the concrete built-in `PermissionProfile` internally only when updating its local config/status snapshot. - Permission presets expose their built-in active profile id, and preset selection preserves that id in both the immediate turn override and the local TUI config snapshot. - Turn routing sends `TurnPermissionsOverride::ActiveProfile` when an active id is present, and only falls back to the legacy sandbox projection for the remaining runtime override path. ## How to review Start with `codex-rs/tui/src/app_command.rs` to verify the command shape no longer exposes `PermissionProfile`. Then read `codex-rs/tui/src/app/thread_routing.rs` to verify the app-server turn-start conversion: active ids go through as ids, while the legacy sandbox fallback is still constrained to the existing runtime override case. Finally, check `codex-rs/tui/src/chatwidget/permission_popups.rs`, `codex-rs/tui/src/app/event_dispatch.rs`, `codex-rs/tui/src/app/config_persistence.rs`, and `codex-rs/utils/approval-presets/src/lib.rs` to see how preset selections stay id-only across TUI events while the local display/config mirror still gets a concrete built-in profile. ## Verification Latest local verification after the id-only `AppEvent` cleanup: - `cargo check -p codex-tui --tests` - `cargo test -p codex-tui permissions_selection_sends_approvals_reviewer_in_override_turn_context` - `cargo test -p codex-tui update_feature_flags_enabling_guardian` - `cargo test -p codex-utils-approval-presets` - `just fmt` - `just fix -p codex-tui -p codex-utils-approval-presets` Earlier in the same PR, before the final event-shape cleanup: - `cargo test -p codex-tui turn_permissions_` - `cargo test -p codex-tui submission_` - `cargo test -p codex-tui session_configured_syncs_widget_config_permissions_and_cwd` - `RUST_MIN_STACK=16777216 cargo test -p codex-tui` --- codex-rs/tui/src/app.rs | 17 +++++- codex-rs/tui/src/app/config_persistence.rs | 59 +++++++++++++------ codex-rs/tui/src/app/event_dispatch.rs | 38 +++++++----- codex-rs/tui/src/app/tests.rs | 39 ++++++++---- codex-rs/tui/src/app/thread_routing.rs | 28 ++++----- codex-rs/tui/src/app/thread_session_state.rs | 9 ++- codex-rs/tui/src/app_command.rs | 14 ++--- codex-rs/tui/src/app_event.rs | 6 +- codex-rs/tui/src/chatwidget.rs | 1 + .../tui/src/chatwidget/input_submission.rs | 4 +- .../tui/src/chatwidget/permission_popups.rs | 22 +++---- codex-rs/tui/src/chatwidget/rate_limits.rs | 2 +- codex-rs/tui/src/chatwidget/service_tiers.rs | 2 +- codex-rs/tui/src/chatwidget/settings.rs | 8 ++- .../tui/src/chatwidget/settings_popups.rs | 2 +- codex-rs/tui/src/chatwidget/tests.rs | 2 + .../chatwidget/tests/composer_submission.rs | 32 ++++++---- .../src/chatwidget/tests/history_replay.rs | 7 ++- .../tui/src/chatwidget/tests/permissions.rs | 18 +++++- .../src/chatwidget/windows_sandbox_prompts.rs | 16 +++-- codex-rs/utils/approval-presets/src/lib.rs | 31 ++++++++++ 21 files changed, 243 insertions(+), 114 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index fcaf4d2290..00e45f979b 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -140,6 +140,8 @@ use codex_protocol::ThreadId; use codex_protocol::config_types::Personality; #[cfg(target_os = "windows")] use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::models::ActivePermissionProfile; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ModelAvailabilityNux; use codex_protocol::openai_models::ModelPreset; @@ -150,6 +152,7 @@ use codex_protocol::permissions::FileSystemSandboxKind; use codex_rollout::StateDbHandle; use codex_terminal_detection::user_agent; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_approval_presets::builtin_permission_profile_for_active_permission_profile; use color_eyre::eyre::Result; use color_eyre::eyre::WrapErr; use crossterm::event::KeyCode; @@ -327,7 +330,7 @@ fn default_exec_approval_decisions( struct AutoReviewMode { approval_policy: AskForApproval, approvals_reviewer: ApprovalsReviewer, - permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, } /// Enabling the Auto-review experiment in the TUI should also switch the @@ -338,7 +341,17 @@ fn auto_review_mode() -> AutoReviewMode { AutoReviewMode { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::AutoReview, - permission_profile: PermissionProfile::workspace_write(), + active_permission_profile: ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_WORKSPACE, + ), + } +} + +#[cfg(test)] +impl AutoReviewMode { + fn permission_profile(&self) -> PermissionProfile { + builtin_permission_profile_for_active_permission_profile(&self.active_permission_profile) + .expect("auto-review mode should use a built-in permission profile") } } diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 8d02e6f1e4..4a410f8130 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -105,24 +105,41 @@ impl App { true } - pub(super) fn try_set_permission_profile_on_config( + pub(super) fn try_set_builtin_active_permission_profile_on_config( &mut self, config: &mut Config, - permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, user_message_prefix: &str, log_message: &str, - ) -> bool { + ) -> Option { + let Some(permission_profile) = + builtin_permission_profile_for_active_permission_profile(&active_permission_profile) + else { + tracing::warn!( + id = %active_permission_profile.id, + "{log_message}: unsupported active permission profile" + ); + self.chat_widget.add_error_message(format!( + "{user_message_prefix}: unsupported active permission profile `{}`", + active_permission_profile.id + )); + return None; + }; + if let Err(err) = config .permissions - .set_permission_profile(permission_profile) + .set_permission_profile_from_session_snapshot( + permission_profile.clone(), + Some(active_permission_profile), + ) { tracing::warn!(error = %err, "{log_message}"); self.chat_widget .add_error_message(format!("{user_message_prefix}: {err}")); - return false; + return None; } - true + Some(permission_profile) } pub(super) async fn update_feature_flags(&mut self, updates: Vec<(Feature, bool)>) { @@ -149,6 +166,7 @@ impl App { let mut approval_policy_override = None; let mut approvals_reviewer_override = None; let mut permission_profile_override = None; + let mut active_permission_profile_override = None; let mut feature_updates_to_apply = Vec::with_capacity(updates.len()); // Auto-Review owns `approvals_reviewer`, but disabling the feature // from inside a profile should not silently clear a value configured at @@ -240,14 +258,16 @@ impl App { ) { continue; } - if !self.try_set_permission_profile_on_config( - &mut feature_config, - auto_review_preset.permission_profile.clone(), - "Failed to enable Auto-review", - "failed to set auto-review permission profile on staged config", - ) { + let Some(permission_profile) = self + .try_set_builtin_active_permission_profile_on_config( + &mut feature_config, + auto_review_preset.active_permission_profile.clone(), + "Failed to enable Auto-review", + "failed to set auto-review permission profile on staged config", + ) + else { continue; - } + }; feature_edits.extend([ ConfigEdit::SetPath { segments: scoped_segments("approval_policy"), @@ -259,7 +279,9 @@ impl App { }, ]); approval_policy_override = Some(auto_review_preset.approval_policy); - permission_profile_override = Some(auto_review_preset.permission_profile.clone()); + permission_profile_override = Some(permission_profile); + active_permission_profile_override = + Some(auto_review_preset.active_permission_profile.clone()); } next_config = feature_config; feature_updates_to_apply.push((feature, effective_enabled)); @@ -305,7 +327,10 @@ impl App { if let Some(permission_profile) = permission_profile_override_value.as_ref() && let Err(err) = self .chat_widget - .set_permission_profile(permission_profile.clone()) + .set_permission_profile_from_session_snapshot( + permission_profile.clone(), + active_permission_profile_override.clone(), + ) { tracing::error!( error = %err, @@ -333,7 +358,7 @@ impl App { /*cwd*/ None, approval_policy_override, approvals_reviewer_override, - permission_profile_override, + active_permission_profile_override, /*windows_sandbox_level*/ None, /*model*/ None, /*effort*/ None, @@ -360,7 +385,7 @@ impl App { /*cwd*/ None, /*approval_policy*/ None, /*approvals_reviewer*/ None, - /*permission_profile*/ None, + /*active_permission_profile*/ None, #[cfg(target_os = "windows")] Some(windows_sandbox_level), /*model*/ None, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 811c24cc4d..16e25e4d16 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1111,7 +1111,7 @@ impl App { /*cwd*/ None, /*approval_policy*/ None, /*approvals_reviewer*/ None, - /*permission_profile*/ None, + /*active_permission_profile*/ None, #[cfg(target_os = "windows")] Some(windows_sandbox_level), /*model*/ None, @@ -1136,7 +1136,7 @@ impl App { /*cwd*/ None, Some(AskForApproval::from(preset.approval)), Some(self.config.approvals_reviewer), - Some(preset.permission_profile.clone()), + Some(preset.active_permission_profile.clone()), #[cfg(target_os = "windows")] Some(windows_sandbox_level), /*model*/ None, @@ -1150,9 +1150,10 @@ impl App { self.app_event_tx.send(AppEvent::UpdateAskForApprovalPolicy( AskForApproval::from(preset.approval), )); - self.app_event_tx.send(AppEvent::UpdatePermissionProfile( - preset.permission_profile.clone(), - )); + self.app_event_tx + .send(AppEvent::UpdateActivePermissionProfile( + preset.active_permission_profile.clone(), + )); let _ = mode; self.chat_widget.add_plain_history_lines(vec![ Line::from(vec!["• ".dim(), "Sandbox ready".into()]), @@ -1400,25 +1401,30 @@ impl App { self.sync_active_thread_permission_settings_to_cached_session() .await; } - AppEvent::UpdatePermissionProfile(permission_profile) => { + AppEvent::UpdateActivePermissionProfile(active_permission_profile) => { + let mut config = self.config.clone(); + let Some(permission_profile) = self + .try_set_builtin_active_permission_profile_on_config( + &mut config, + active_permission_profile.clone(), + "Failed to set permission profile", + "failed to set active permission profile on app config", + ) + else { + return Ok(AppRunControl::Continue); + }; #[cfg(target_os = "windows")] let permission_profile_is_managed_restricted = managed_filesystem_sandbox_is_restricted(&permission_profile); let permission_profile_for_chat = permission_profile.clone(); - let mut config = self.config.clone(); - if !self.try_set_permission_profile_on_config( - &mut config, - permission_profile, - "Failed to set permission profile", - "failed to set permission profile on app config", - ) { - return Ok(AppRunControl::Continue); - } self.config = config; if let Err(err) = self .chat_widget - .set_permission_profile(permission_profile_for_chat) + .set_permission_profile_from_session_snapshot( + permission_profile_for_chat, + Some(active_permission_profile), + ) { tracing::warn!(%err, "failed to set permission profile on chat config"); self.chat_widget diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index c107bc777f..a925b4d488 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1640,7 +1640,18 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result< .config_ref() .permissions .permission_profile(), - &auto_review.permission_profile + &auto_review.permission_profile() + ); + assert_eq!( + app.config.permissions.active_permission_profile(), + Some(auto_review.active_permission_profile.clone()) + ); + assert_eq!( + app.chat_widget + .config_ref() + .permissions + .active_permission_profile(), + Some(auto_review.active_permission_profile.clone()) ); assert_eq!( app.chat_widget.config_ref().approvals_reviewer, @@ -1649,7 +1660,7 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result< assert_eq!(app.runtime_approval_policy_override, None); assert_eq!( app.runtime_permission_profile_override, - Some(auto_review.permission_profile.clone()) + Some(auto_review.permission_profile()) ); assert_eq!( op_rx.try_recv(), @@ -1657,7 +1668,7 @@ async fn update_feature_flags_enabling_guardian_selects_auto_review() -> Result< cwd: None, approval_policy: Some(auto_review.approval_policy), approvals_reviewer: Some(auto_review.approvals_reviewer), - permission_profile: Some(auto_review.permission_profile.clone()), + active_permission_profile: Some(auto_review.active_permission_profile.clone()), windows_sandbox_level: None, model: None, effort: None, @@ -1719,7 +1730,10 @@ async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restor app.chat_widget .set_approval_policy(AskForApproval::OnRequest); app.chat_widget - .set_permission_profile(PermissionProfile::workspace_write())?; + .set_permission_profile_from_session_snapshot( + PermissionProfile::workspace_write(), + /*active_profile*/ None, + )?; app.update_feature_flags(vec![(Feature::GuardianApproval, false)]) .await; @@ -1747,7 +1761,7 @@ async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restor cwd: None, approval_policy: None, approvals_reviewer: Some(ApprovalsReviewer::User), - permission_profile: None, + active_permission_profile: None, windows_sandbox_level: None, model: None, effort: None, @@ -1817,7 +1831,7 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review .config_ref() .permissions .permission_profile(), - &auto_review.permission_profile + &auto_review.permission_profile() ); assert_eq!( op_rx.try_recv(), @@ -1825,7 +1839,7 @@ async fn update_feature_flags_enabling_guardian_overrides_explicit_manual_review cwd: None, approval_policy: Some(auto_review.approval_policy), approvals_reviewer: Some(auto_review.approvals_reviewer), - permission_profile: Some(auto_review.permission_profile.clone()), + active_permission_profile: Some(auto_review.active_permission_profile.clone()), windows_sandbox_level: None, model: None, effort: None, @@ -1882,7 +1896,7 @@ async fn update_feature_flags_disabling_guardian_clears_manual_review_policy_wit cwd: None, approval_policy: None, approvals_reviewer: Some(ApprovalsReviewer::User), - permission_profile: None, + active_permission_profile: None, windows_sandbox_level: None, model: None, effort: None, @@ -1941,7 +1955,7 @@ async fn update_feature_flags_enabling_guardian_in_profile_sets_profile_auto_rev cwd: None, approval_policy: Some(auto_review.approval_policy), approvals_reviewer: Some(auto_review.approvals_reviewer), - permission_profile: Some(auto_review.permission_profile.clone()), + active_permission_profile: Some(auto_review.active_permission_profile.clone()), windows_sandbox_level: None, model: None, effort: None, @@ -2028,7 +2042,7 @@ guardian_approval = true cwd: None, approval_policy: None, approvals_reviewer: Some(ApprovalsReviewer::User), - permission_profile: None, + active_permission_profile: None, windows_sandbox_level: None, model: None, effort: None, @@ -3139,7 +3153,10 @@ async fn side_fork_config_inherits_parent_thread_runtime_settings() { app.chat_widget .set_approval_policy(AskForApproval::OnRequest); app.chat_widget - .set_permission_profile(parent_permission_profile.clone()) + .set_permission_profile_from_session_snapshot( + parent_permission_profile.clone(), + /*active_profile*/ None, + ) .expect("test permission profile should be accepted"); app.chat_widget .set_approvals_reviewer(ApprovalsReviewer::AutoReview); diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 4f858f10e6..d270bb53ce 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -510,7 +510,7 @@ impl App { cwd, approval_policy, approvals_reviewer, - permission_profile, + active_permission_profile, model, effort, summary, @@ -590,7 +590,7 @@ impl App { approvals_reviewer.unwrap_or(config.approvals_reviewer); let permissions_override = Self::turn_permissions_override_from_config( config, - permission_profile, + active_permission_profile.as_ref(), self.runtime_permission_profile_override.as_ref(), ); app_server @@ -698,16 +698,14 @@ impl App { fn turn_permissions_override_from_config( config: &Config, - permission_profile: &PermissionProfile, + active_permission_profile: Option<&ActivePermissionProfile>, runtime_permission_profile_override: Option<&PermissionProfile>, ) -> TurnPermissionsOverride { - let effective_permission_profile = config.permissions.effective_permission_profile(); - if &effective_permission_profile == permission_profile - && let Some(active_permission_profile) = config.permissions.active_permission_profile() - { - return TurnPermissionsOverride::ActiveProfile(active_permission_profile); + if let Some(active_permission_profile) = active_permission_profile { + return TurnPermissionsOverride::ActiveProfile(active_permission_profile.clone()); } + let effective_permission_profile = config.permissions.effective_permission_profile(); let runtime_permission_profile_override = runtime_permission_profile_override.map(|profile| { profile @@ -718,9 +716,9 @@ impl App { }); if runtime_permission_profile_override .as_ref() - .is_some_and(|profile| profile == permission_profile) + .is_some_and(|profile| profile == &effective_permission_profile) { - return TurnPermissionsOverride::LegacySandbox(permission_profile.clone()); + return TurnPermissionsOverride::LegacySandbox(effective_permission_profile); } TurnPermissionsOverride::Preserve @@ -1506,12 +1504,12 @@ mod tests { #[tokio::test] async fn turn_permissions_use_active_profile_when_available() { let config = config_with_workspace_profile().await; - let permission_profile = config.permissions.effective_permission_profile(); + let active_permission_profile = config.permissions.active_permission_profile(); assert_eq!( App::turn_permissions_override_from_config( &config, - &permission_profile, + active_permission_profile.as_ref(), /*runtime_permission_profile_override*/ None, ), TurnPermissionsOverride::ActiveProfile(ActivePermissionProfile::new( @@ -1527,12 +1525,10 @@ mod tests { .permissions .set_permission_profile(PermissionProfile::read_only()) .expect("read-only profile should be allowed"); - let permission_profile = config.permissions.effective_permission_profile(); assert_eq!( App::turn_permissions_override_from_config( - &config, - &permission_profile, + &config, /*active_permission_profile*/ None, /*runtime_permission_profile_override*/ None, ), TurnPermissionsOverride::Preserve @@ -1552,7 +1548,7 @@ mod tests { assert_eq!( App::turn_permissions_override_from_config( &config, - &effective_permission_profile, + /*active_permission_profile*/ None, Some(&permission_profile), ), TurnPermissionsOverride::LegacySandbox(effective_permission_profile) diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index a8d421563b..eb56325663 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -131,6 +131,7 @@ mod tests { use codex_app_server_protocol::PermissionProfileFileSystemPermissions; use codex_app_server_protocol::PermissionProfileNetworkPermissions; use codex_config::types::ApprovalsReviewer; + use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::PermissionProfile; use pretty_assertions::assert_eq; use std::path::PathBuf; @@ -197,9 +198,14 @@ mod tests { codex_config::Constrained::allow_any(AskForApproval::OnRequest.to_core()); app.config.approvals_reviewer = ApprovalsReviewer::AutoReview; let expected_permission_profile = PermissionProfile::workspace_write(); + let expected_active_permission_profile = + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE); app.chat_widget.handle_thread_session(main_session.clone()); app.chat_widget - .set_permission_profile(expected_permission_profile.clone()) + .set_permission_profile_from_session_snapshot( + expected_permission_profile.clone(), + Some(expected_active_permission_profile.clone()), + ) .expect("set widget permission profile"); app.config .permissions @@ -213,6 +219,7 @@ mod tests { approval_policy: AskForApproval::OnRequest, approvals_reviewer: ApprovalsReviewer::AutoReview, permission_profile: expected_permission_profile, + active_permission_profile: Some(expected_active_permission_profile), ..main_session }; assert_eq!( diff --git a/codex-rs/tui/src/app_command.rs b/codex-rs/tui/src/app_command.rs index 89fd2600f8..0b6484d015 100644 --- a/codex-rs/tui/src/app_command.rs +++ b/codex-rs/tui/src/app_command.rs @@ -16,7 +16,7 @@ use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::Personality; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::models::PermissionProfile; +use codex_protocol::models::ActivePermissionProfile; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::request_permissions::RequestPermissionsResponse; use serde::Serialize; @@ -41,7 +41,7 @@ pub(crate) enum AppCommand { cwd: PathBuf, approval_policy: AskForApproval, approvals_reviewer: Option, - permission_profile: PermissionProfile, + active_permission_profile: Option, model: String, effort: Option, summary: Option, @@ -54,7 +54,7 @@ pub(crate) enum AppCommand { cwd: Option, approval_policy: Option, approvals_reviewer: Option, - permission_profile: Option, + active_permission_profile: Option, windows_sandbox_level: Option, model: Option, effort: Option>, @@ -142,7 +142,7 @@ impl AppCommand { items: Vec, cwd: PathBuf, approval_policy: AskForApproval, - permission_profile: PermissionProfile, + active_permission_profile: Option, model: String, effort: Option, summary: Option, @@ -156,7 +156,7 @@ impl AppCommand { cwd, approval_policy, approvals_reviewer: None, - permission_profile, + active_permission_profile, model, effort, summary, @@ -172,7 +172,7 @@ impl AppCommand { cwd: Option, approval_policy: Option, approvals_reviewer: Option, - permission_profile: Option, + active_permission_profile: Option, windows_sandbox_level: Option, model: Option, effort: Option>, @@ -185,7 +185,7 @@ impl AppCommand { cwd, approval_policy, approvals_reviewer, - permission_profile, + active_permission_profile, windows_sandbox_level, model, effort, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index ab9055197b..57ec371749 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -43,7 +43,7 @@ use codex_features::Feature; use codex_plugin::PluginCapabilitySummary; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::Personality; -use codex_protocol::models::PermissionProfile; +use codex_protocol::models::ActivePermissionProfile; use codex_protocol::openai_models::ReasoningEffort; use codex_realtime_webrtc::RealtimeWebrtcEvent; use codex_realtime_webrtc::RealtimeWebrtcSessionHandle; @@ -742,8 +742,8 @@ pub(crate) enum AppEvent { /// Update the current approval policy in the running app and widget. UpdateAskForApprovalPolicy(AskForApproval), - /// Update the current permission profile in the running app and widget. - UpdatePermissionProfile(PermissionProfile), + /// Update the current built-in active permission profile in the running app and widget. + UpdateActivePermissionProfile(ActivePermissionProfile), /// Update the current approvals reviewer in the running app and widget. UpdateApprovalsReviewer(ApprovalsReviewer), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 5c7d0194ef..207c547562 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -442,6 +442,7 @@ use crate::workspace_command::WorkspaceCommandRunner; use chrono::Local; use codex_app_server_protocol::AskForApproval; use codex_file_search::FileMatch; +use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ModelPreset; diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index 2ead61f3dc..2fd376f139 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -336,12 +336,12 @@ impl ChatWidget { None if self.config.notices.fast_default_opt_out == Some(true) => Some(None), None => None, }; - let permission_profile = self.config.permissions.effective_permission_profile(); + let active_permission_profile = self.config.permissions.active_permission_profile(); let op = AppCommand::user_turn( items, self.config.cwd.to_path_buf(), AskForApproval::from(self.config.permissions.approval_policy.value()), - permission_profile, + active_permission_profile, effective_mode.model().to_string(), effective_mode.reasoning_effort(), /*summary*/ None, diff --git a/codex-rs/tui/src/chatwidget/permission_popups.rs b/codex-rs/tui/src/chatwidget/permission_popups.rs index 6f2e1c4dbd..a82f3a6cdb 100644 --- a/codex-rs/tui/src/chatwidget/permission_popups.rs +++ b/codex-rs/tui/src/chatwidget/permission_popups.rs @@ -124,7 +124,7 @@ impl ChatWidget { } else { Self::approval_preset_actions( preset_approval, - preset.permission_profile.clone(), + preset.active_permission_profile.clone(), base_name.clone(), ApprovalsReviewer::User, ) @@ -134,7 +134,7 @@ impl ChatWidget { { Self::approval_preset_actions( preset_approval, - preset.permission_profile.clone(), + preset.active_permission_profile.clone(), base_name.clone(), ApprovalsReviewer::User, ) @@ -142,7 +142,7 @@ impl ChatWidget { } else { Self::approval_preset_actions( preset_approval, - preset.permission_profile.clone(), + preset.active_permission_profile.clone(), base_name.clone(), ApprovalsReviewer::User, ) @@ -180,7 +180,7 @@ impl ChatWidget { ), actions: Self::approval_preset_actions( preset_approval, - preset.permission_profile.clone(), + preset.active_permission_profile.clone(), "Auto-review".to_string(), ApprovalsReviewer::AutoReview, ), @@ -308,17 +308,16 @@ impl ChatWidget { pub(super) fn approval_preset_actions( approval: AskForApproval, - permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, label: String, approvals_reviewer: ApprovalsReviewer, ) -> Vec { vec![Box::new(move |tx| { - let permission_profile_clone = permission_profile.clone(); tx.send(AppEvent::CodexOp(AppCommand::override_turn_context( /*cwd*/ None, Some(approval), Some(approvals_reviewer), - Some(permission_profile_clone.clone()), + Some(active_permission_profile.clone()), /*windows_sandbox_level*/ None, /*model*/ None, /*effort*/ None, @@ -328,7 +327,9 @@ impl ChatWidget { /*personality*/ None, ))); tx.send(AppEvent::UpdateAskForApprovalPolicy(approval)); - tx.send(AppEvent::UpdatePermissionProfile(permission_profile_clone)); + tx.send(AppEvent::UpdateActivePermissionProfile( + active_permission_profile.clone(), + )); tx.send(AppEvent::UpdateApprovalsReviewer(approvals_reviewer)); tx.send(AppEvent::InsertHistoryCell(Box::new( history_cell::new_info_event( @@ -385,7 +386,6 @@ impl ChatWidget { ) { let selected_name = preset.label.to_string(); let approval = AskForApproval::from(preset.approval); - let permission_profile = preset.permission_profile; let mut header_children: Vec> = Vec::new(); let title_line = Line::from("Enable full access?").bold(); let info_line = Line::from(vec![ @@ -402,7 +402,7 @@ impl ChatWidget { let mut accept_actions = Self::approval_preset_actions( approval, - permission_profile.clone(), + preset.active_permission_profile.clone(), selected_name.clone(), ApprovalsReviewer::User, ); @@ -412,7 +412,7 @@ impl ChatWidget { let mut accept_and_remember_actions = Self::approval_preset_actions( approval, - permission_profile, + preset.active_permission_profile, selected_name, ApprovalsReviewer::User, ); diff --git a/codex-rs/tui/src/chatwidget/rate_limits.rs b/codex-rs/tui/src/chatwidget/rate_limits.rs index 6b5414536d..7eaca908c6 100644 --- a/codex-rs/tui/src/chatwidget/rate_limits.rs +++ b/codex-rs/tui/src/chatwidget/rate_limits.rs @@ -285,7 +285,7 @@ impl ChatWidget { /*cwd*/ None, /*approval_policy*/ None, /*approvals_reviewer*/ None, - /*permission_profile*/ None, + /*active_permission_profile*/ None, /*windows_sandbox_level*/ None, Some(switch_model_for_events.clone()), Some(Some(default_effort)), diff --git a/codex-rs/tui/src/chatwidget/service_tiers.rs b/codex-rs/tui/src/chatwidget/service_tiers.rs index 1cdb52f224..a60af0a69e 100644 --- a/codex-rs/tui/src/chatwidget/service_tiers.rs +++ b/codex-rs/tui/src/chatwidget/service_tiers.rs @@ -107,7 +107,7 @@ impl ChatWidget { /*cwd*/ None, /*approval_policy*/ None, /*approvals_reviewer*/ None, - /*permission_profile*/ None, + /*active_permission_profile*/ None, /*windows_sandbox_level*/ None, /*model*/ None, /*effort*/ None, diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index 419070ef9e..a2125c3f47 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -17,13 +17,15 @@ impl ChatWidget { } } - /// Set the permission profile in the widget's config copy. #[cfg_attr(not(target_os = "windows"), allow(dead_code))] - pub(crate) fn set_permission_profile( + pub(crate) fn set_permission_profile_from_session_snapshot( &mut self, profile: PermissionProfile, + active_profile: Option, ) -> ConstraintResult<()> { - self.config.permissions.set_permission_profile(profile)?; + self.config + .permissions + .set_permission_profile_from_session_snapshot(profile, active_profile)?; self.refresh_status_surfaces(); Ok(()) } diff --git a/codex-rs/tui/src/chatwidget/settings_popups.rs b/codex-rs/tui/src/chatwidget/settings_popups.rs index d718e70d79..2dcfefb73a 100644 --- a/codex-rs/tui/src/chatwidget/settings_popups.rs +++ b/codex-rs/tui/src/chatwidget/settings_popups.rs @@ -53,7 +53,7 @@ impl ChatWidget { /*cwd*/ None, /*approval_policy*/ None, /*approvals_reviewer*/ None, - /*permission_profile*/ None, + /*active_permission_profile*/ None, /*windows_sandbox_level*/ None, /*model*/ None, /*effort*/ None, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index f86b896e5b..e0dda9eb5c 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -149,6 +149,8 @@ pub(super) use codex_protocol::config_types::CollaborationMode; pub(super) use codex_protocol::config_types::ModeKind; pub(super) use codex_protocol::config_types::Personality; pub(super) use codex_protocol::config_types::ServiceTier; +pub(super) use codex_protocol::models::ActivePermissionProfile; +pub(super) use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; pub(super) use codex_protocol::models::FileSystemPermissions; pub(super) use codex_protocol::models::MessagePhase; pub(super) use codex_protocol::models::NetworkPermissions; diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 3e8635c46a..381c5430a9 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -94,7 +94,7 @@ async fn submission_preserves_text_elements_and_local_images() { } #[tokio::test] -async fn submission_includes_configured_permission_profile() { +async fn submission_includes_configured_active_permission_profile() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let thread_id = ThreadId::new(); @@ -120,6 +120,7 @@ async fn submission_includes_configured_permission_profile() { }, } .into(); + let expected_active_permission_profile = ActivePermissionProfile::new("custom"); let configured = crate::session_state::ThreadSessionState { thread_id, forked_from_id: None, @@ -130,8 +131,8 @@ async fn submission_includes_configured_permission_profile() { service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, - permission_profile: expected_permission_profile.clone(), - active_permission_profile: None, + permission_profile: expected_permission_profile, + active_permission_profile: Some(expected_active_permission_profile.clone()), cwd: test_path_buf("/home/user/project").abs(), runtime_workspace_roots: Vec::new(), instruction_source_paths: Vec::new(), @@ -150,17 +151,21 @@ async fn submission_includes_configured_permission_profile() { ); chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - let permission_profile = match next_submit_op(&mut op_rx) { + let active_permission_profile = match next_submit_op(&mut op_rx) { Op::UserTurn { - permission_profile, .. - } => permission_profile, + active_permission_profile, + .. + } => active_permission_profile, other => panic!("expected Op::UserTurn, got {other:?}"), }; - assert_eq!(permission_profile, expected_permission_profile); + assert_eq!( + active_permission_profile, + Some(expected_active_permission_profile) + ); } #[tokio::test] -async fn submission_keeps_profile_when_legacy_projection_is_external() { +async fn submission_omits_active_permission_profile_for_legacy_snapshot() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let thread_id = ThreadId::new(); @@ -180,7 +185,7 @@ async fn submission_keeps_profile_when_legacy_projection_is_external() { service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, - permission_profile: expected_permission_profile.clone(), + permission_profile: expected_permission_profile, active_permission_profile: None, cwd: test_path_buf("/home/user/project").abs(), runtime_workspace_roots: Vec::new(), @@ -197,13 +202,14 @@ async fn submission_keeps_profile_when_legacy_projection_is_external() { .set_composer_text("submit".to_string(), Vec::new(), Vec::new()); chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - let permission_profile = match next_submit_op(&mut op_rx) { + let active_permission_profile = match next_submit_op(&mut op_rx) { Op::UserTurn { - permission_profile, .. - } => permission_profile, + active_permission_profile, + .. + } => active_permission_profile, other => panic!("expected Op::UserTurn, got {other:?}"), }; - assert_eq!(permission_profile, expected_permission_profile); + assert_eq!(active_permission_profile, None); } #[tokio::test] diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index 1d1b19fdf5..7dca128448 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -296,8 +296,11 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { assert_eq!(&chat.config_ref().cwd, &expected_cwd); let updated_profile = PermissionProfile::workspace_write(); - chat.set_permission_profile(updated_profile.clone()) - .expect("set permission profile"); + chat.set_permission_profile_from_session_snapshot( + updated_profile.clone(), + /*active_profile*/ None, + ) + .expect("set permission profile"); assert_eq!( chat.config_ref().permissions.permission_profile(), &updated_profile, diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index 4c66041271..9928676e4f 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -742,7 +742,9 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context cwd: None, approval_policy: Some(AskForApproval::OnRequest), approvals_reviewer: Some(ApprovalsReviewer::AutoReview), - permission_profile: Some(PermissionProfile::workspace_write()), + active_permission_profile: Some(ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_WORKSPACE, + )), windows_sandbox_level: None, model: None, effort: None, @@ -752,6 +754,20 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context personality: None, } ); + + let active_permission_profile_update = std::iter::from_fn(|| rx.try_recv().ok()) + .find_map(|event| match event { + AppEvent::UpdateActivePermissionProfile(active_permission_profile) => { + Some(active_permission_profile) + } + _ => None, + }) + .expect("expected UpdateActivePermissionProfile event"); + + assert_eq!( + active_permission_profile_update, + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE) + ); } #[tokio::test] diff --git a/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs b/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs index 401e2a787d..d8ff59da48 100644 --- a/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs +++ b/codex-rs/tui/src/chatwidget/windows_sandbox_prompts.rs @@ -42,10 +42,10 @@ impl ChatWidget { extra_count: usize, failed_scan: bool, ) { - let (approval, permission_profile) = match &preset { + let (approval, active_permission_profile) = match &preset { Some(p) => ( Some(AskForApproval::from(p.approval)), - Some(p.permission_profile.clone()), + Some(p.active_permission_profile.clone()), ), None => (None, None), }; @@ -110,10 +110,12 @@ impl ChatWidget { tx.send(AppEvent::SkipNextWorldWritableScan); })); } - if let (Some(approval), Some(permission_profile)) = (approval, permission_profile.clone()) { + if let (Some(approval), Some(active_permission_profile)) = + (approval, active_permission_profile.clone()) + { accept_actions.extend(Self::approval_preset_actions( approval, - permission_profile, + active_permission_profile, mode_label.to_string(), ApprovalsReviewer::User, )); @@ -124,10 +126,12 @@ impl ChatWidget { tx.send(AppEvent::UpdateWorldWritableWarningAcknowledged(true)); tx.send(AppEvent::PersistWorldWritableWarningAcknowledged); })); - if let (Some(approval), Some(permission_profile)) = (approval, permission_profile) { + if let (Some(approval), Some(active_permission_profile)) = + (approval, active_permission_profile) + { accept_and_remember_actions.extend(Self::approval_preset_actions( approval, - permission_profile, + active_permission_profile, mode_label.to_string(), ApprovalsReviewer::User, )); diff --git a/codex-rs/utils/approval-presets/src/lib.rs b/codex-rs/utils/approval-presets/src/lib.rs index b254950500..c7ce9e578b 100644 --- a/codex-rs/utils/approval-presets/src/lib.rs +++ b/codex-rs/utils/approval-presets/src/lib.rs @@ -1,3 +1,7 @@ +use codex_protocol::models::ActivePermissionProfile; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY; +use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -12,6 +16,8 @@ pub struct ApprovalPreset { pub description: &'static str, /// Approval policy to apply. pub approval: AskForApproval, + /// Built-in permission profile selected by this preset. + pub active_permission_profile: ActivePermissionProfile, /// Permission profile to apply. pub permission_profile: PermissionProfile, } @@ -26,6 +32,9 @@ pub fn builtin_approval_presets() -> Vec { label: "Read Only", description: "Codex can read files in the current workspace. Approval is required to edit files or access the internet.", approval: AskForApproval::OnRequest, + active_permission_profile: ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_READ_ONLY, + ), permission_profile: PermissionProfile::read_only(), }, ApprovalPreset { @@ -33,6 +42,9 @@ pub fn builtin_approval_presets() -> Vec { label: "Default", description: "Codex can read and edit files in the current workspace, and run commands. Approval is required to access the internet or edit other files. (Identical to Agent mode)", approval: AskForApproval::OnRequest, + active_permission_profile: ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_WORKSPACE, + ), permission_profile: PermissionProfile::workspace_write(), }, ApprovalPreset { @@ -40,7 +52,26 @@ pub fn builtin_approval_presets() -> Vec { label: "Full Access", description: "Codex can edit files outside this workspace and access the internet without asking for approval. Exercise caution when using.", approval: AskForApproval::Never, + active_permission_profile: ActivePermissionProfile::new( + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS, + ), permission_profile: PermissionProfile::Disabled, }, ] } + +/// Return the concrete profile for one of the built-in active profile ids. +pub fn builtin_permission_profile_for_active_permission_profile( + active_permission_profile: &ActivePermissionProfile, +) -> Option { + if active_permission_profile.extends.is_some() { + return None; + } + + match active_permission_profile.id.as_str() { + BUILT_IN_PERMISSION_PROFILE_READ_ONLY => Some(PermissionProfile::read_only()), + BUILT_IN_PERMISSION_PROFILE_WORKSPACE => Some(PermissionProfile::workspace_write()), + BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS => Some(PermissionProfile::Disabled), + _ => None, + } +} From b2d0a5a7853d8c5943c4c05389f5c72105278865 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 15 May 2026 15:58:13 -0700 Subject: [PATCH 6/6] core: set permission profiles from snapshots --- codex-rs/core/src/config/config_tests.rs | 24 +++++ codex-rs/core/src/config/mod.rs | 46 ++------- .../src/config/resolved_permission_profile.rs | 80 ++++++++++++++-- codex-rs/core/src/session/mod.rs | 1 + codex-rs/core/src/session/session.rs | 16 +++- codex-rs/tui/src/app.rs | 1 + codex-rs/tui/src/app/config_persistence.rs | 12 ++- codex-rs/tui/src/app/event_dispatch.rs | 6 +- codex-rs/tui/src/app/tests.rs | 11 +-- codex-rs/tui/src/app/thread_session_state.rs | 7 +- codex-rs/tui/src/chatwidget.rs | 1 + codex-rs/tui/src/chatwidget/session_flow.rs | 11 ++- codex-rs/tui/src/chatwidget/settings.rs | 5 +- .../src/chatwidget/tests/history_replay.rs | 5 +- codex-rs/tui/src/status/tests.rs | 93 ++++++++----------- 15 files changed, 187 insertions(+), 132 deletions(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 7a4a3a5c7b..b8bd7d2398 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1453,6 +1453,30 @@ async fn permission_profile_override_populates_runtime_permissions() -> std::io: Ok(()) } +#[test] +fn permission_snapshot_setter_preserves_permission_constraints() { + let initial_profile = PermissionProfile::read_only(); + let mut permissions = Permissions::from_approval_and_profile( + Constrained::allow_any(AskForApproval::Never), + Constrained::allow_only(initial_profile.clone()), + ) + .expect("initial permissions should satisfy constraints"); + + let err = permissions + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( + PermissionProfile::workspace_write(), + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE), + )) + .expect_err("workspace profile should violate read-only constraint"); + + assert_eq!(permissions.permission_profile(), &initial_profile); + assert_eq!(permissions.active_permission_profile(), None); + assert!( + matches!(err, ConstraintError::InvalidValue { .. }), + "expected invalid value constraint error, got {err:?}" + ); +} + #[tokio::test] async fn permission_profile_override_preserves_managed_unrestricted_filesystem() -> std::io::Result<()> { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index bad23da221..5bd017e320 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -149,6 +149,7 @@ pub use managed_features::ManagedFeatures; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; pub(crate) use permissions::resolve_permission_profile; +pub use resolved_permission_profile::PermissionProfileSnapshot; pub(crate) use resolved_permission_profile::PermissionProfileState; const DEFAULT_IGNORE_LARGE_UNTRACKED_DIRS: i64 = 200; @@ -311,30 +312,13 @@ impl Permissions { /// /// This is a trusted-state bridge for consumers of `SessionConfigured`. /// Config loading and app-server selection should resolve named profiles - /// through config instead of constructing this pair directly. + /// through config instead of constructing a snapshot directly. pub fn set_permission_profile_from_session_snapshot( &mut self, - permission_profile: PermissionProfile, - active_permission_profile: Option, + snapshot: PermissionProfileSnapshot, ) -> ConstraintResult<()> { - self.set_permission_profile_from_session_snapshot_with_profile_workspace_roots( - permission_profile, - active_permission_profile, - Vec::new(), - ) - } - - pub fn set_permission_profile_from_session_snapshot_with_profile_workspace_roots( - &mut self, - permission_profile: PermissionProfile, - active_permission_profile: Option, - profile_workspace_roots: Vec, - ) -> ConstraintResult<()> { - self.permission_profile_state.set_active_permission_profile( - permission_profile, - active_permission_profile, - profile_workspace_roots, - ) + self.permission_profile_state + .set_permission_profile_snapshot(snapshot) } /// Replace the current permission constraints with a trusted session @@ -343,25 +327,11 @@ impl Permissions { pub fn replace_permission_profile_from_session_snapshot( &mut self, permission_profile: Constrained, - active_permission_profile: Option, + snapshot: PermissionProfileSnapshot, ) -> ConstraintResult<()> { - self.replace_permission_profile_from_session_snapshot_with_profile_workspace_roots( + self.permission_profile_state = PermissionProfileState::from_constrained_resolved( permission_profile, - active_permission_profile, - Vec::new(), - ) - } - - pub fn replace_permission_profile_from_session_snapshot_with_profile_workspace_roots( - &mut self, - permission_profile: Constrained, - active_permission_profile: Option, - profile_workspace_roots: Vec, - ) -> ConstraintResult<()> { - self.permission_profile_state = PermissionProfileState::from_constrained_active_profile( - permission_profile, - active_permission_profile, - profile_workspace_roots, + snapshot.into_resolved_permission_profile(), )?; Ok(()) } diff --git a/codex-rs/core/src/config/resolved_permission_profile.rs b/codex-rs/core/src/config/resolved_permission_profile.rs index 911d9bf2e8..1db0f0e48d 100644 --- a/codex-rs/core/src/config/resolved_permission_profile.rs +++ b/codex-rs/core/src/config/resolved_permission_profile.rs @@ -40,6 +40,11 @@ pub(crate) enum ResolvedPermissionProfile { Named(NamedPermissionProfile), } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PermissionProfileSnapshot { + resolved_permission_profile: ResolvedPermissionProfile, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct LegacyPermissionProfile { permission_profile: PermissionProfile, @@ -124,6 +129,67 @@ impl ResolvedPermissionProfile { } } +impl PermissionProfileSnapshot { + pub fn legacy(permission_profile: PermissionProfile) -> Self { + Self { + resolved_permission_profile: ResolvedPermissionProfile::legacy(permission_profile), + } + } + + pub fn active( + permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, + ) -> Self { + Self::active_with_profile_workspace_roots( + permission_profile, + active_permission_profile, + Vec::new(), + ) + } + + pub fn active_with_profile_workspace_roots( + permission_profile: PermissionProfile, + active_permission_profile: ActivePermissionProfile, + profile_workspace_roots: Vec, + ) -> Self { + Self { + resolved_permission_profile: ResolvedPermissionProfile::from_active_profile( + permission_profile, + Some(active_permission_profile), + profile_workspace_roots, + ), + } + } + + pub fn from_session_snapshot( + permission_profile: PermissionProfile, + active_permission_profile: Option, + ) -> Self { + match active_permission_profile { + Some(active_permission_profile) => { + Self::active(permission_profile, active_permission_profile) + } + None => Self::legacy(permission_profile), + } + } + + pub fn permission_profile(&self) -> &PermissionProfile { + self.resolved_permission_profile.permission_profile() + } + + pub fn active_permission_profile(&self) -> Option { + self.resolved_permission_profile.active_permission_profile() + } + + pub fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + self.resolved_permission_profile.profile_workspace_roots() + } + + pub(crate) fn into_resolved_permission_profile(self) -> ResolvedPermissionProfile { + self.resolved_permission_profile + } +} + #[derive(Debug, Clone, PartialEq)] pub(crate) struct PermissionProfileState { resolved_permission_profile: Constrained, @@ -199,17 +265,11 @@ impl PermissionProfileState { .set(ResolvedPermissionProfile::legacy(permission_profile)) } - pub(crate) fn set_active_permission_profile( + pub(crate) fn set_permission_profile_snapshot( &mut self, - permission_profile: PermissionProfile, - active_permission_profile: Option, - profile_workspace_roots: Vec, + snapshot: PermissionProfileSnapshot, ) -> ConstraintResult<()> { - let candidate = ResolvedPermissionProfile::from_active_profile( - permission_profile, - active_permission_profile, - profile_workspace_roots, - ); - self.resolved_permission_profile.set(candidate) + self.resolved_permission_profile + .set(snapshot.into_resolved_permission_profile()) } } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 2dd97710e8..d67e79f794 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -173,6 +173,7 @@ use crate::compact::collect_user_messages; use crate::config::Config; use crate::config::Constrained; use crate::config::ConstraintResult; +use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileState; use crate::config::StartedNetworkProxy; use crate::config::resolve_web_search_mode_for_turn; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 5b57e11895..c04f2d5828 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -357,11 +357,17 @@ impl SessionConfiguration { &file_system_sandbox_policy, network_sandbox_policy, ); - self.permission_profile_state.set_active_permission_profile( - effective_permission_profile, - active_permission_profile, - profile_workspace_roots, - ) + self.permission_profile_state + .set_permission_profile_snapshot(match active_permission_profile { + Some(active_permission_profile) => { + PermissionProfileSnapshot::active_with_profile_workspace_roots( + effective_permission_profile, + active_permission_profile, + profile_workspace_roots, + ) + } + None => PermissionProfileSnapshot::legacy(effective_permission_profile), + }) } } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 00e45f979b..46a7be00dd 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -47,6 +47,7 @@ use crate::keymap::RuntimeKeymap; use crate::legacy_core::config::Config; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; +use crate::legacy_core::config::PermissionProfileSnapshot; use crate::legacy_core::config::edit::ConfigEdit; use crate::legacy_core::config::edit::ConfigEditsBuilder; #[cfg(target_os = "windows")] diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 4a410f8130..6f4fda608e 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -128,10 +128,10 @@ impl App { if let Err(err) = config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( permission_profile.clone(), - Some(active_permission_profile), - ) + active_permission_profile, + )) { tracing::warn!(error = %err, "{log_message}"); self.chat_widget @@ -328,8 +328,10 @@ impl App { && let Err(err) = self .chat_widget .set_permission_profile_from_session_snapshot( - permission_profile.clone(), - active_permission_profile_override.clone(), + PermissionProfileSnapshot::from_session_snapshot( + permission_profile.clone(), + active_permission_profile_override.clone(), + ), ) { tracing::error!( diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 16e25e4d16..15e4f9b6cd 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -1422,8 +1422,10 @@ impl App { if let Err(err) = self .chat_widget .set_permission_profile_from_session_snapshot( - permission_profile_for_chat, - Some(active_permission_profile), + PermissionProfileSnapshot::active( + permission_profile_for_chat, + active_permission_profile, + ), ) { tracing::warn!(%err, "failed to set permission profile on chat config"); diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index a925b4d488..c42f42c257 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -28,6 +28,7 @@ use crate::app_command::AppCommand as Op; use crate::diff_model::FileChange; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; +use crate::legacy_core::config::PermissionProfileSnapshot; use crate::legacy_core::config::TerminalResizeReflowMaxRows; use codex_app_server_protocol::AdditionalFileSystemPermissions; use codex_app_server_protocol::AdditionalNetworkPermissions; @@ -1730,10 +1731,9 @@ async fn update_feature_flags_disabling_guardian_clears_review_policy_and_restor app.chat_widget .set_approval_policy(AskForApproval::OnRequest); app.chat_widget - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::legacy( PermissionProfile::workspace_write(), - /*active_profile*/ None, - )?; + ))?; app.update_feature_flags(vec![(Feature::GuardianApproval, false)]) .await; @@ -3153,10 +3153,9 @@ async fn side_fork_config_inherits_parent_thread_runtime_settings() { app.chat_widget .set_approval_policy(AskForApproval::OnRequest); app.chat_widget - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::legacy( parent_permission_profile.clone(), - /*active_profile*/ None, - ) + )) .expect("test permission profile should be accepted"); app.chat_widget .set_approvals_reviewer(ApprovalsReviewer::AutoReview); diff --git a/codex-rs/tui/src/app/thread_session_state.rs b/codex-rs/tui/src/app/thread_session_state.rs index eb56325663..0734db5280 100644 --- a/codex-rs/tui/src/app/thread_session_state.rs +++ b/codex-rs/tui/src/app/thread_session_state.rs @@ -120,6 +120,7 @@ mod tests { use crate::app::side::SideThreadState; use crate::app::test_support::make_test_app; use crate::app::thread_events::ThreadEventChannel; + use crate::legacy_core::config::PermissionProfileSnapshot; use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; use codex_app_server_protocol::AskForApproval; @@ -202,10 +203,10 @@ mod tests { ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE); app.chat_widget.handle_thread_session(main_session.clone()); app.chat_widget - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( expected_permission_profile.clone(), - Some(expected_active_permission_profile.clone()), - ) + expected_active_permission_profile.clone(), + )) .expect("set widget permission profile"); app.config .permissions diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 207c547562..a37371bf73 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -61,6 +61,7 @@ use crate::legacy_core::DEFAULT_AGENTS_MD_FILENAME; use crate::legacy_core::config::Config; use crate::legacy_core::config::Constrained; use crate::legacy_core::config::ConstraintResult; +use crate::legacy_core::config::PermissionProfileSnapshot; #[cfg(target_os = "windows")] use crate::legacy_core::windows_sandbox::WindowsSandboxLevelExt; use crate::mention_codec::LinkedMention; diff --git a/codex-rs/tui/src/chatwidget/session_flow.rs b/codex-rs/tui/src/chatwidget/session_flow.rs index 5dfb2090f7..324a76eb50 100644 --- a/codex-rs/tui/src/chatwidget/session_flow.rs +++ b/codex-rs/tui/src/chatwidget/session_flow.rs @@ -49,13 +49,14 @@ impl ChatWidget { self.config.permissions.approval_policy = Constrained::allow_only(session.approval_policy.to_core()); } + let permission_snapshot = PermissionProfileSnapshot::from_session_snapshot( + session.permission_profile.clone(), + session.active_permission_profile.clone(), + ); let permission_sync = self .config .permissions - .set_permission_profile_from_session_snapshot( - session.permission_profile.clone(), - session.active_permission_profile.clone(), - ); + .set_permission_profile_from_session_snapshot(permission_snapshot.clone()); if let Err(err) = permission_sync { tracing::warn!(%err, "failed to sync permissions from SessionConfigured"); if let Err(replace_err) = self @@ -63,7 +64,7 @@ impl ChatWidget { .permissions .replace_permission_profile_from_session_snapshot( Constrained::allow_only(session.permission_profile.clone()), - session.active_permission_profile.clone(), + permission_snapshot, ) { tracing::error!( diff --git a/codex-rs/tui/src/chatwidget/settings.rs b/codex-rs/tui/src/chatwidget/settings.rs index a2125c3f47..91abc728db 100644 --- a/codex-rs/tui/src/chatwidget/settings.rs +++ b/codex-rs/tui/src/chatwidget/settings.rs @@ -20,12 +20,11 @@ impl ChatWidget { #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) fn set_permission_profile_from_session_snapshot( &mut self, - profile: PermissionProfile, - active_profile: Option, + snapshot: PermissionProfileSnapshot, ) -> ConstraintResult<()> { self.config .permissions - .set_permission_profile_from_session_snapshot(profile, active_profile)?; + .set_permission_profile_from_session_snapshot(snapshot)?; self.refresh_status_surfaces(); Ok(()) } diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index 7dca128448..9eeb8c432d 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -296,10 +296,9 @@ async fn session_configured_syncs_widget_config_permissions_and_cwd() { assert_eq!(&chat.config_ref().cwd, &expected_cwd); let updated_profile = PermissionProfile::workspace_write(); - chat.set_permission_profile_from_session_snapshot( + chat.set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::legacy( updated_profile.clone(), - /*active_profile*/ None, - ) + )) .expect("set permission profile"); assert_eq!( chat.config_ref().permissions.permission_profile(), diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index 9380718196..73455f52ea 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -5,6 +5,7 @@ use super::rate_limit_snapshot_display; use crate::history_cell::HistoryCell; use crate::legacy_core::config::Config; use crate::legacy_core::config::ConfigBuilder; +use crate::legacy_core::config::PermissionProfileSnapshot; use crate::status::StatusAccountDisplay; use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; @@ -306,12 +307,10 @@ async fn status_permissions_named_read_only_profile_shows_builtin_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::read_only(), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_READ_ONLY, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_READ_ONLY), + )) .expect("set permission profile"); assert_eq!( @@ -335,15 +334,13 @@ async fn status_permissions_read_only_profile_shows_additional_writable_roots() .with_additional_writable_roots(config.cwd.as_path(), std::slice::from_ref(&extra_root)); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::from_runtime_permissions( &file_system_policy, NetworkSandboxPolicy::Restricted, ), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_READ_ONLY, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_READ_ONLY), + )) .expect("set permission profile"); assert_eq!( @@ -363,12 +360,10 @@ async fn status_permissions_named_workspace_profile_shows_builtin_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::workspace_write(), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_WORKSPACE, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE), + )) .expect("set permission profile"); assert_eq!( @@ -389,12 +384,10 @@ async fn status_permissions_workspace_auto_review_shows_reviewer_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::workspace_write(), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_WORKSPACE, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE), + )) .expect("set permission profile"); assert_eq!( @@ -415,17 +408,15 @@ async fn status_permissions_named_profile_shows_additional_writable_roots() { let extra_root = test_path_buf("/workspace/extra").abs(); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::workspace_write_with( std::slice::from_ref(&extra_root), NetworkSandboxPolicy::Restricted, /*exclude_tmpdir_env_var*/ false, /*exclude_slash_tmp*/ false, ), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_WORKSPACE, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE), + )) .expect("set permission profile"); assert_eq!( @@ -451,10 +442,10 @@ async fn status_permissions_workspace_roots_show_additional_directories() { .set_workspace_roots(config.workspace_roots.clone()); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::workspace_write(), - Some(ActivePermissionProfile::new(":workspace")), - ) + ActivePermissionProfile::new(":workspace"), + )) .expect("set permission profile"); assert_eq!( @@ -476,15 +467,17 @@ async fn status_permissions_workspace_roots_include_profile_defined_directories( let profile_root = test_path_buf("/workspace/shared").abs(); config .permissions - .set_permission_profile_from_session_snapshot_with_profile_workspace_roots( - PermissionProfile::workspace_write_with( - std::slice::from_ref(&profile_root), - NetworkSandboxPolicy::Restricted, - /*exclude_tmpdir_env_var*/ false, - /*exclude_slash_tmp*/ false, + .set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::active_with_profile_workspace_roots( + PermissionProfile::workspace_write_with( + std::slice::from_ref(&profile_root), + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ), + ActivePermissionProfile::new(":workspace"), + vec![profile_root.clone()], ), - Some(ActivePermissionProfile::new(":workspace")), - vec![profile_root.clone()], ) .expect("set permission profile"); @@ -508,17 +501,15 @@ async fn status_permissions_broadened_workspace_profile_shows_builtin_label() { .expect("set approval policy"); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::workspace_write_with( &[], NetworkSandboxPolicy::Enabled, /*exclude_tmpdir_env_var*/ false, /*exclude_slash_tmp*/ false, ), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_WORKSPACE, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE), + )) .expect("set permission profile"); assert_eq!( @@ -533,10 +524,10 @@ async fn status_permissions_user_defined_profile_shows_name() { let mut config = test_config(&temp_home).await; config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::read_only(), - Some(ActivePermissionProfile::new("locked")), - ) + ActivePermissionProfile::new("locked"), + )) .expect("set permission profile"); assert_eq!( @@ -553,10 +544,10 @@ async fn status_snapshot_shows_active_user_defined_profile() { set_workspace_cwd(&mut config, test_path_buf("/workspace/tests").abs()); config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::read_only(), - Some(ActivePermissionProfile::new("locked")), - ) + ActivePermissionProfile::new("locked"), + )) .expect("set permission profile"); let usage = TokenUsage::default(); @@ -651,12 +642,10 @@ async fn status_snapshot_shows_auto_review_permissions() { config.approvals_reviewer = ApprovalsReviewer::AutoReview; config .permissions - .set_permission_profile_from_session_snapshot( + .set_permission_profile_from_session_snapshot(PermissionProfileSnapshot::active( PermissionProfile::workspace_write(), - Some(ActivePermissionProfile::new( - BUILT_IN_PERMISSION_PROFILE_WORKSPACE, - )), - ) + ActivePermissionProfile::new(BUILT_IN_PERMISSION_PROFILE_WORKSPACE), + )) .expect("set permission profile"); let usage = TokenUsage::default();