diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index ca247f2d55..f3983e31f4 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -2683,6 +2683,12 @@ "name": { "type": "string" }, + "namespace": { + "type": [ + "string", + "null" + ] + }, "status": { "type": [ "string", @@ -3631,6 +3637,13 @@ "ephemeral": { "type": "boolean" }, + "lastTurnId": { + "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", + "type": [ + "string", + "null" + ] + }, "model": { "description": "Configuration overrides for the forked thread, if any.", "type": [ @@ -3673,13 +3686,6 @@ } ], "description": "Optional client-supplied analytics source classification for this forked thread." - }, - "lastTurnId": { - "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", - "type": [ - "string", - "null" - ] } }, "required": [ @@ -6873,4 +6879,4 @@ } ], "title": "ClientRequest" -} +} \ No newline at end of file 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 341c325b1d..b77b512b36 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 @@ -15510,6 +15510,12 @@ "name": { "type": "string" }, + "namespace": { + "type": [ + "string", + "null" + ] + }, "status": { "type": [ "string", @@ -17279,6 +17285,13 @@ "ephemeral": { "type": "boolean" }, + "lastTurnId": { + "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", + "type": [ + "string", + "null" + ] + }, "model": { "description": "Configuration overrides for the forked thread, if any.", "type": [ @@ -17321,13 +17334,6 @@ } ], "description": "Optional client-supplied analytics source classification for this forked thread." - }, - "lastTurnId": { - "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", - "type": [ - "string", - "null" - ] } }, "required": [ @@ -20874,4 +20880,4 @@ }, "title": "CodexAppServerProtocol", "type": "object" -} +} \ No newline at end of file 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 488bd6016e..bdd61d6b94 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 @@ -11914,6 +11914,12 @@ "name": { "type": "string" }, + "namespace": { + "type": [ + "string", + "null" + ] + }, "status": { "type": [ "string", @@ -15058,6 +15064,13 @@ "ephemeral": { "type": "boolean" }, + "lastTurnId": { + "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", + "type": [ + "string", + "null" + ] + }, "model": { "description": "Configuration overrides for the forked thread, if any.", "type": [ @@ -15100,13 +15113,6 @@ } ], "description": "Optional client-supplied analytics source classification for this forked thread." - }, - "lastTurnId": { - "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", - "type": [ - "string", - "null" - ] } }, "required": [ @@ -18652,4 +18658,4 @@ }, "title": "CodexAppServerProtocolV2", "type": "object" -} +} \ No newline at end of file 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 01ac63c268..dad669b139 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -751,6 +751,12 @@ "name": { "type": "string" }, + "namespace": { + "type": [ + "string", + "null" + ] + }, "status": { "type": [ "string", diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json index 7a00e3803a..76278b106b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json @@ -126,6 +126,13 @@ "ephemeral": { "type": "boolean" }, + "lastTurnId": { + "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", + "type": [ + "string", + "null" + ] + }, "model": { "description": "Configuration overrides for the forked thread, if any.", "type": [ @@ -168,13 +175,6 @@ } ], "description": "Optional client-supplied analytics source classification for this forked thread." - }, - "lastTurnId": { - "description": "Optional last turn id to fork through, inclusive.\n\nWhen specified, turns after `last_turn_id` are omitted from the fork. The referenced turn cannot be in progress.", - "type": [ - "string", - "null" - ] } }, "required": [ @@ -182,4 +182,4 @@ ], "title": "ThreadForkParams", "type": "object" -} +} \ No newline at end of file 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 af078af68e..0cd7d9ae7b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -821,6 +821,12 @@ "name": { "type": "string" }, + "namespace": { + "type": [ + "string", + "null" + ] + }, "status": { "type": [ "string", diff --git a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts index 62547d3e40..769b1b08fc 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts @@ -20,4 +20,4 @@ id?: string, /** * Set when using the Responses API. */ -call_id: string | null, status: LocalShellStatus, action: LocalShellAction, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "function_call", id?: string, name: string, namespace?: string, arguments: string, call_id: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "tool_search_call", id?: string, call_id: string | null, status?: string, execution: string, arguments: unknown, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "function_call_output", id?: string, call_id: string, output: FunctionCallOutputBody, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "custom_tool_call", id?: string, status?: string, call_id: string, name: string, input: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "custom_tool_call_output", id?: string, call_id: string, name?: string, output: FunctionCallOutputBody, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "tool_search_output", id?: string, call_id: string | null, status: string, execution: string, tools: unknown[], internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "web_search_call", id?: string, status?: string, action?: WebSearchAction, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "image_generation_call", id?: string, status: string, revised_prompt?: string, result: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "compaction", id?: string, encrypted_content: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "compaction_trigger", } | { "type": "context_compaction", id?: string, encrypted_content?: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "other" }; +call_id: string | null, status: LocalShellStatus, action: LocalShellAction, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "function_call", id?: string, name: string, namespace?: string, arguments: string, call_id: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "tool_search_call", id?: string, call_id: string | null, status?: string, execution: string, arguments: unknown, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "function_call_output", id?: string, call_id: string, output: FunctionCallOutputBody, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "custom_tool_call", id?: string, status?: string, call_id: string, name: string, namespace?: string, input: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "custom_tool_call_output", id?: string, call_id: string, name?: string, output: FunctionCallOutputBody, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "tool_search_output", id?: string, call_id: string | null, status: string, execution: string, tools: unknown[], internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "web_search_call", id?: string, status?: string, action?: WebSearchAction, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "image_generation_call", id?: string, status: string, revised_prompt?: string, result: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "compaction", id?: string, encrypted_content: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "compaction_trigger", } | { "type": "context_compaction", id?: string, encrypted_content?: string, internal_chat_message_metadata_passthrough?: InternalChatMessageMetadataPassthrough, } | { "type": "other" }; diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index 91eca690f1..9f27dff7fa 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -43,6 +43,8 @@ pub(crate) const GENERATED_TS_HEADER: &str = "// GENERATED CODE! DO NOT MODIFY B const IGNORED_DEFINITIONS: &[&str] = &["Option<()>"]; const JSON_V1_ALLOWLIST: &[&str] = &["InitializeParams", "InitializeResponse"]; const EXPERIMENTAL_CLIENT_METHOD_DEPENDENCY_TYPES: &[&str] = &[ + "EnvironmentShellInfo", + "PathUri", "RemoteControlClient", "RemoteControlClientsListOrder", "ThreadBackgroundTerminal", diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 7b15035575..d8db99d4ff 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -943,6 +943,13 @@ client_request_definitions! { serialization: global("environment"), response: v2::EnvironmentAddResponse, }, + #[experimental("environment/info")] + /// Reads information from a configured execution environment. + EnvironmentInfo => "environment/info" { + params: v2::EnvironmentInfoParams, + serialization: global_shared_read("environment"), + response: v2::EnvironmentInfoResponse, + }, McpServerOauthLogin => "mcpServer/oauth/login" { params: v2::McpServerOauthLoginParams, diff --git a/codex-rs/app-server-protocol/src/protocol/v2/environment.rs b/codex-rs/app-server-protocol/src/protocol/v2/environment.rs index ccffd5813b..320e2af894 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/environment.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/environment.rs @@ -1,3 +1,4 @@ +use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -19,3 +20,29 @@ pub struct EnvironmentAddParams { #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct EnvironmentAddResponse {} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct EnvironmentInfoParams { + pub environment_id: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct EnvironmentInfoResponse { + pub shell: EnvironmentShellInfo, + /// Default working directory reported by the environment, as a canonical file URI. + pub cwd: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct EnvironmentShellInfo { + /// Stable shell name, for example `zsh`, `bash`, `powershell`, `sh`, or `cmd`. + pub name: String, + /// Target-native shell executable path or command name. + pub path: String, +} diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 6f34cae541..c9ac2f5fb2 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -205,6 +205,7 @@ Example with notification opt-out: - `permissionProfile/list` — beta; list available permission profile ids with optional display `description` text and an `allowed` flag reflecting effective requirements, using cursor pagination. Pass `cwd` when the caller needs project-local `[permissions.]` entries to be included in the current catalog view. - `experimentalFeature/enablement/set` — patch the in-memory process-wide runtime feature enablement for currently supported feature keys. For each feature, precedence is: cloud requirements > --enable > config.toml > experimentalFeature/enablement/set (new) > code default. Invalid keys will be ignored. - `environment/add` — experimental; add or replace a named remote environment by `environmentId` and `execServerUrl` for later selection by `thread/start` or `turn/start`; optional `connectTimeoutMs` overrides the WebSocket connection timeout; returns `{}` and does not change the default environment. +- `environment/info` — experimental; connect to a configured environment by `environmentId` and return its detected `shell` plus its default `cwd` as a canonical environment-native `file:` URI. Connection failures are returned as request errors. - `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). Built-in presets do not select a model; the Plan preset selects medium reasoning effort. This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly. - `skills/list` — list skills for one or more `cwd` values (optional `forceReload`). - `skills/extraRoots/set` — replace the app-server process runtime extra standalone skill roots. The roots are not persisted; missing directories are accepted and simply load no skills. diff --git a/codex-rs/app-server/src/current_time.rs b/codex-rs/app-server/src/current_time.rs index ae873bfc63..57be618af3 100644 --- a/codex-rs/app-server/src/current_time.rs +++ b/codex-rs/app-server/src/current_time.rs @@ -22,7 +22,7 @@ use crate::outgoing_message::ConnectionId; use crate::outgoing_message::OutgoingMessageSender; use crate::thread_state::ThreadStateManager; -const CURRENT_TIME_REQUEST_TIMEOUT: Duration = Duration::from_secs(5); +const CURRENT_TIME_REQUEST_TIMEOUT: Duration = Duration::from_secs(10); const CURRENT_TIME_POLL_INTERVAL: Duration = Duration::from_secs(1); pub(crate) fn app_server_time_provider( diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 3322f24d8b..375307e804 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -1037,6 +1037,9 @@ impl MessageProcessor { ClientRequest::EnvironmentAdd { params, .. } => { self.environment_processor.environment_add(params).await } + ClientRequest::EnvironmentInfo { params, .. } => { + self.environment_processor.environment_info(params).await + } ClientRequest::FsReadFile { params, .. } => self .fs_processor .read_file(params) diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index cb0f276d85..587325c246 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -62,6 +62,9 @@ use codex_app_server_protocol::DynamicToolNamespaceTool; use codex_app_server_protocol::DynamicToolSpec; use codex_app_server_protocol::EnvironmentAddParams; use codex_app_server_protocol::EnvironmentAddResponse; +use codex_app_server_protocol::EnvironmentInfoParams; +use codex_app_server_protocol::EnvironmentInfoResponse; +use codex_app_server_protocol::EnvironmentShellInfo; use codex_app_server_protocol::ExperimentalFeature as ApiExperimentalFeature; use codex_app_server_protocol::ExperimentalFeatureListParams; use codex_app_server_protocol::ExperimentalFeatureListResponse; diff --git a/codex-rs/app-server/src/request_processors/environment_processor.rs b/codex-rs/app-server/src/request_processors/environment_processor.rs index 36533aa231..635e1732ec 100644 --- a/codex-rs/app-server/src/request_processors/environment_processor.rs +++ b/codex-rs/app-server/src/request_processors/environment_processor.rs @@ -26,4 +26,30 @@ impl EnvironmentRequestProcessor { .map_err(|err| invalid_request(err.to_string()))?; Ok(Some(EnvironmentAddResponse {}.into())) } + + pub(crate) async fn environment_info( + &self, + params: EnvironmentInfoParams, + ) -> Result, JSONRPCErrorError> { + let environment_id = params.environment_id; + let environment = self + .environment_manager + .get_environment(&environment_id) + .ok_or_else(|| invalid_request(format!("unknown environment id `{environment_id}`")))?; + let info = environment.info().await.map_err(|err| { + internal_error(format!( + "failed to get info for environment `{environment_id}`: {err}" + )) + })?; + Ok(Some( + EnvironmentInfoResponse { + shell: EnvironmentShellInfo { + name: info.shell.name, + path: info.shell.path, + }, + cwd: info.cwd, + } + .into(), + )) + } } diff --git a/codex-rs/app-server/tests/suite/v2/environment_info.rs b/codex-rs/app-server/tests/suite/v2/environment_info.rs new file mode 100644 index 0000000000..8b0e82518f --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/environment_info.rs @@ -0,0 +1,208 @@ +use std::time::Duration; + +use anyhow::Result; +use app_test_support::TestAppServer; +use app_test_support::to_response; +use codex_app_server_protocol::EnvironmentAddResponse; +use codex_app_server_protocol::EnvironmentInfoResponse; +use codex_app_server_protocol::EnvironmentShellInfo; +use codex_app_server_protocol::JSONRPCError; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; +use serde_json::json; +use tempfile::TempDir; +use tokio::net::TcpListener; +use tokio::time::timeout; + +use super::exec_server_test_support::accept_exec_server_environment; + +const RPC_TIMEOUT: Duration = Duration::from_secs(10); +const INVALID_REQUEST_ERROR_CODE: i64 = -32600; +const INTERNAL_ERROR_CODE: i64 = -32603; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn environment_info_returns_remote_environment_info() -> Result<()> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let exec_server_url = format!("ws://{}", listener.local_addr()?); + let exec_server = tokio::spawn(async move { + accept_exec_server_environment( + listener, + json!({ + "shell": {"name": "zsh", "path": "/bin/zsh"}, + "cwd": "file:///workspace", + }), + ) + .await?; + Ok::<_, anyhow::Error>(()) + }); + + let codex_home = TempDir::new()?; + let mut app_server = TestAppServer::new(codex_home.path()).await?; + timeout(RPC_TIMEOUT, app_server.initialize()).await??; + add_environment( + &mut app_server, + &exec_server_url, + /*connect_timeout_ms*/ None, + ) + .await?; + + let request_id = app_server + .send_raw_request( + "environment/info", + Some(json!({"environmentId": "remote-a"})), + ) + .await?; + let response: JSONRPCResponse = timeout( + RPC_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + to_response::(response)?, + EnvironmentInfoResponse { + shell: EnvironmentShellInfo { + name: "zsh".to_string(), + path: "/bin/zsh".to_string(), + }, + cwd: Some(PathUri::parse("file:///workspace")?), + } + ); + timeout(RPC_TIMEOUT, exec_server).await???; + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn environment_info_accepts_missing_cwd() -> Result<()> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let exec_server_url = format!("ws://{}", listener.local_addr()?); + let exec_server = tokio::spawn(async move { + accept_exec_server_environment( + listener, + json!({"shell": {"name": "zsh", "path": "/bin/zsh"}}), + ) + .await?; + Ok::<_, anyhow::Error>(()) + }); + + let codex_home = TempDir::new()?; + let mut app_server = TestAppServer::new(codex_home.path()).await?; + timeout(RPC_TIMEOUT, app_server.initialize()).await??; + add_environment( + &mut app_server, + &exec_server_url, + /*connect_timeout_ms*/ None, + ) + .await?; + + let request_id = app_server + .send_raw_request( + "environment/info", + Some(json!({"environmentId": "remote-a"})), + ) + .await?; + let response: JSONRPCResponse = timeout( + RPC_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + to_response::(response)?, + EnvironmentInfoResponse { + shell: EnvironmentShellInfo { + name: "zsh".to_string(), + path: "/bin/zsh".to_string(), + }, + cwd: None, + } + ); + timeout(RPC_TIMEOUT, exec_server).await???; + Ok(()) +} + +#[tokio::test] +async fn environment_info_rejects_unknown_environment() -> Result<()> { + let codex_home = TempDir::new()?; + let mut app_server = TestAppServer::new(codex_home.path()).await?; + timeout(RPC_TIMEOUT, app_server.initialize()).await??; + + let request_id = app_server + .send_raw_request( + "environment/info", + Some(json!({"environmentId": "missing"})), + ) + .await?; + let error = timeout( + RPC_TIMEOUT, + app_server.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!( + error, + JSONRPCError { + id: RequestId::Integer(request_id), + error: JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: "unknown environment id `missing`".to_string(), + data: None, + }, + } + ); + Ok(()) +} + +#[tokio::test] +async fn environment_info_reports_connection_failure() -> Result<()> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let exec_server_url = format!("ws://{}", listener.local_addr()?); + let codex_home = TempDir::new()?; + let mut app_server = TestAppServer::new(codex_home.path()).await?; + timeout(RPC_TIMEOUT, app_server.initialize()).await??; + add_environment(&mut app_server, &exec_server_url, Some(50)).await?; + + let request_id = app_server + .send_raw_request( + "environment/info", + Some(json!({"environmentId": "remote-a"})), + ) + .await?; + let error = timeout( + RPC_TIMEOUT, + app_server.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + assert_eq!(error.error.code, INTERNAL_ERROR_CODE); + assert!( + error + .error + .message + .contains("failed to get info for environment `remote-a`") + ); + Ok(()) +} + +async fn add_environment( + app_server: &mut TestAppServer, + exec_server_url: &str, + connect_timeout_ms: Option, +) -> Result<()> { + let request_id = app_server + .send_raw_request( + "environment/add", + Some(json!({ + "environmentId": "remote-a", + "execServerUrl": exec_server_url, + "connectTimeoutMs": connect_timeout_ms, + })), + ) + .await?; + let response: JSONRPCResponse = timeout( + RPC_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let _: EnvironmentAddResponse = to_response(response)?; + Ok(()) +} diff --git a/codex-rs/app-server/tests/suite/v2/exec_server_test_support.rs b/codex-rs/app-server/tests/suite/v2/exec_server_test_support.rs new file mode 100644 index 0000000000..f28e06e9fa --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/exec_server_test_support.rs @@ -0,0 +1,73 @@ +use anyhow::Result; +use futures::SinkExt; +use futures::StreamExt; +use serde_json::Value; +use serde_json::json; +use tokio::net::TcpListener; +use tokio::net::TcpStream; +use tokio_tungstenite::WebSocketStream; +use tokio_tungstenite::accept_async; +use tokio_tungstenite::tungstenite::Message; + +pub(crate) async fn accept_exec_server_environment( + listener: TcpListener, + environment_info: Value, +) -> Result> { + let mut websocket = accept_initialized_exec_server(listener).await?; + + let request = read_exec_server_json(&mut websocket).await?; + assert_eq!(request["method"], "environment/info"); + websocket + .send(Message::Text( + json!({ + "id": request["id"], + "result": environment_info, + }) + .to_string() + .into(), + )) + .await?; + + Ok(websocket) +} + +pub(crate) async fn accept_initialized_exec_server( + listener: TcpListener, +) -> Result> { + let (stream, _) = listener.accept().await?; + let mut websocket = accept_async(stream).await?; + + let initialize = read_exec_server_json(&mut websocket).await?; + assert_eq!(initialize["method"], "initialize"); + websocket + .send(Message::Text( + json!({ + "id": initialize["id"], + "result": {"sessionId": "test-session"}, + }) + .to_string() + .into(), + )) + .await?; + let initialized = read_exec_server_json(&mut websocket).await?; + assert_eq!(initialized["method"], "initialized"); + + Ok(websocket) +} + +pub(crate) async fn read_exec_server_json( + websocket: &mut WebSocketStream, +) -> Result { + loop { + match websocket + .next() + .await + .ok_or_else(|| anyhow::anyhow!("exec-server websocket closed"))?? + { + Message::Text(text) => return Ok(serde_json::from_str(text.as_ref())?), + Message::Binary(bytes) => return Ok(serde_json::from_slice(bytes.as_ref())?), + Message::Ping(_) | Message::Pong(_) => {} + message => anyhow::bail!("expected JSON-RPC message, got {message:?}"), + } + } +} diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index f3a3872450..1f3289e439 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -38,7 +38,6 @@ use codex_utils_path_uri::PathUri; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use core_test_support::responses; use futures::SinkExt; -use futures::StreamExt; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; use rmcp::model::BooleanSchema; @@ -64,14 +63,14 @@ use rmcp::transport::streamable_http_server::session::local::LocalSessionManager use serde_json::json; use tempfile::TempDir; use tokio::net::TcpListener; -use tokio::net::TcpStream; use tokio::sync::oneshot; use tokio::task::JoinHandle; use tokio::time::timeout; -use tokio_tungstenite::WebSocketStream; -use tokio_tungstenite::accept_async; use tokio_tungstenite::tungstenite::Message; +use super::exec_server_test_support::accept_exec_server_environment; +use super::exec_server_test_support::read_exec_server_json; + const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); const TEST_SERVER_NAME: &str = "tool_server"; const TEST_TOOL_NAME: &str = "echo_tool"; @@ -841,35 +840,11 @@ async fn serve_environment_until_shutdown( filesystem_request_tx: oneshot::Sender<()>, mut shutdown_rx: oneshot::Receiver<()>, ) -> Result<()> { - let (stream, _) = listener.accept().await?; - let mut websocket = accept_async(stream).await?; - - let initialize = read_exec_server_json(&mut websocket).await?; - assert_eq!(initialize["method"], "initialize"); - websocket - .send(Message::Text( - json!({ - "id": initialize["id"], - "result": {"sessionId": "test-session"}, - }) - .to_string() - .into(), - )) - .await?; - let initialized = read_exec_server_json(&mut websocket).await?; - assert_eq!(initialized["method"], "initialized"); - let environment_info = read_exec_server_json(&mut websocket).await?; - assert_eq!(environment_info["method"], "environment/info"); - websocket - .send(Message::Text( - json!({ - "id": environment_info["id"], - "result": {"shell": {"name": "zsh", "path": "/bin/zsh"}}, - }) - .to_string() - .into(), - )) - .await?; + let mut websocket = accept_exec_server_environment( + listener, + json!({"shell": {"name": "zsh", "path": "/bin/zsh"}}), + ) + .await?; let mut filesystem_request_tx = Some(filesystem_request_tx); loop { @@ -899,23 +874,6 @@ async fn serve_environment_until_shutdown( } } -async fn read_exec_server_json( - websocket: &mut WebSocketStream, -) -> Result { - loop { - match websocket - .next() - .await - .ok_or_else(|| anyhow::anyhow!("exec-server websocket closed"))?? - { - Message::Text(text) => return Ok(serde_json::from_str(text.as_ref())?), - Message::Binary(bytes) => return Ok(serde_json::from_slice(bytes.as_ref())?), - Message::Ping(_) | Message::Pong(_) => {} - message => anyhow::bail!("expected JSON-RPC message, got {message:?}"), - } - } -} - async fn wait_for_mcp_tool_call_completed( mcp: &mut TestAppServer, call_id: &str, diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 9963d37be5..bdb03271d0 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -15,6 +15,8 @@ mod connection_handling_websocket_unix; mod current_time; mod dynamic_tools; mod environment_add; +mod environment_info; +mod exec_server_test_support; #[cfg(not(target_os = "windows"))] mod executor_mcp; mod executor_skills; diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index e4e2b63378..16aeb092e9 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -341,7 +341,7 @@ fn configured_marketplace_sources_by_root( codex_home: &Path, plugins_input: &PluginsConfigInput, ) -> HashMap { - let marketplace_sources = configured_marketplace_sources(plugins_input); + let marketplace_sources = configured_marketplace_sources(plugins_input, codex_home); let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else { return HashMap::new(); }; diff --git a/codex-rs/cli/src/plugin_cmd.rs b/codex-rs/cli/src/plugin_cmd.rs index 6d17719974..056c256b39 100644 --- a/codex-rs/cli/src/plugin_cmd.rs +++ b/codex-rs/cli/src/plugin_cmd.rs @@ -10,6 +10,7 @@ use codex_core_plugins::PluginInstallOutcome; use codex_core_plugins::PluginInstallRequest; use codex_core_plugins::PluginsConfigInput; use codex_core_plugins::PluginsManager; +use codex_core_plugins::allowed_configured_marketplace_names; use codex_core_plugins::installed_marketplaces::marketplace_install_root; use codex_core_plugins::installed_marketplaces::resolve_configured_marketplace_root; use codex_core_plugins::marketplace::MarketplaceListError; @@ -228,7 +229,7 @@ pub async fn run_plugin_list( .is_none_or(|name| marketplace.name == *name) }) .collect::>(); - let marketplace_sources = configured_marketplace_sources(&plugins_input); + let marketplace_sources = configured_marketplace_sources(&plugins_input, codex_home.as_path()); if args.json { let output = JsonPluginListOutput::from_marketplaces( @@ -480,6 +481,7 @@ pub(crate) struct JsonMarketplaceSource { pub(crate) fn configured_marketplace_sources( plugins_input: &PluginsConfigInput, + codex_home: &Path, ) -> HashMap { let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else { return HashMap::new(); @@ -490,9 +492,12 @@ pub(crate) fn configured_marketplace_sources( else { return HashMap::new(); }; + let allowed_marketplace_names = + allowed_configured_marketplace_names(&plugins_input.config_layer_stack, codex_home); marketplaces .iter() + .filter(|(marketplace_name, _)| allowed_marketplace_names.contains(*marketplace_name)) .filter_map(|(marketplace_name, marketplace)| { let source_type = marketplace .get("source_type") @@ -746,11 +751,16 @@ pub(crate) fn configured_marketplace_snapshot_issues( else { return Vec::new(); }; + let allowed_marketplace_names = + allowed_configured_marketplace_names(&plugins_input.config_layer_stack, codex_home); let default_install_root = marketplace_install_root(codex_home); let mut manifest_paths = Vec::new(); let mut issues = Vec::new(); for (configured_name, marketplace) in configured_marketplaces { + if !allowed_marketplace_names.contains(configured_name) { + continue; + } if marketplace_name.is_some_and(|name| configured_name != name) { continue; } diff --git a/codex-rs/cli/tests/plugin_cli.rs b/codex-rs/cli/tests/plugin_cli.rs index c8bd7b5429..f9d73c1ce8 100644 --- a/codex-rs/cli/tests/plugin_cli.rs +++ b/codex-rs/cli/tests/plugin_cli.rs @@ -205,6 +205,7 @@ fn setup_local_marketplace_with_implicit_system_roots() -> Result<(TempDir, Temp let cache_home = TempDir::new()?; let runtime_root = cache_home .path() + .join(".cache") .join("codex-runtimes") .join("codex-primary-runtime") .join("plugins") @@ -844,7 +845,8 @@ async fn plugin_list_ignores_implicit_system_marketplace_roots_without_manifests let (codex_home, source, cache_home) = setup_local_marketplace_with_implicit_system_roots()?; codex_command(codex_home.path())? - .env("XDG_CACHE_HOME", cache_home.path()) + .env("HOME", cache_home.path()) + .env("USERPROFILE", cache_home.path()) .args(["plugin", "list"]) .assert() .success() diff --git a/codex-rs/core-plugins/src/installed_marketplaces.rs b/codex-rs/core-plugins/src/installed_marketplaces.rs index 0377fac9ed..b5bf3b7699 100644 --- a/codex-rs/core-plugins/src/installed_marketplaces.rs +++ b/codex-rs/core-plugins/src/installed_marketplaces.rs @@ -7,6 +7,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use tracing::warn; use crate::marketplace::find_marketplace_manifest_path; +use crate::marketplace_policy::project_effective_user_config; pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; @@ -18,7 +19,7 @@ pub fn installed_marketplace_roots_from_layer_stack( config_layer_stack: &ConfigLayerStack, codex_home: &Path, ) -> Vec { - let Some(user_config) = config_layer_stack.effective_user_config() else { + let Some(user_config) = project_effective_user_config(config_layer_stack, codex_home) else { return Vec::new(); }; let Some(marketplaces_value) = user_config.get("marketplaces") else { diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index d2047afa88..df30305c38 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -55,6 +55,7 @@ pub use manager::PluginUninstallError; pub use manager::PluginsConfigInput; pub use manager::PluginsManager; pub use manager::RecommendedPluginCandidatesInput; +pub use marketplace_policy::allowed_configured_marketplace_names; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome; pub use provider::ExecutorPluginProvider; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 2f406439ae..2b5c4782e8 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -8,8 +8,9 @@ use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; use crate::marketplace::MarketplacePluginSource; use crate::marketplace::find_marketplace_plugin; -use crate::marketplace::list_marketplaces; +use crate::marketplace::list_marketplaces_with_home; use crate::marketplace::load_marketplace; +use crate::marketplace_policy::configured_plugins_from_stack; use crate::npm_source::materialize_npm_plugin_source; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RemoteInstalledPlugin; @@ -84,6 +85,18 @@ enum NonCuratedCacheRefreshMode { ForceReinstall, } +#[derive(Debug)] +pub(crate) struct NonCuratedCacheRefreshOutcome { + pub(crate) cache_refreshed: bool, + pub(crate) errors: Vec, +} + +#[derive(Debug)] +pub(crate) struct NonCuratedCacheRefreshError { + pub(crate) marketplace_name: String, + pub(crate) message: String, +} + pub(crate) fn log_plugin_load_errors(plugins: &[LoadedPlugin]) { for plugin in plugins.iter().filter(|plugin| plugin.error.is_some()) { if let Some(error) = plugin.error.as_deref() { @@ -129,7 +142,7 @@ async fn load_plugins_from_layer_stack_with_scope( scope: PluginLoadScope<'_>, ) -> Vec> { let configured_plugins = merge_configured_plugins_with_remote_installed( - configured_plugins_from_stack(config_layer_stack), + configured_plugins_from_stack(config_layer_stack, store.codex_home().as_path()), extra_plugins, store, remote_global_catalog_active, @@ -410,24 +423,54 @@ pub fn curated_plugin_cache_version(plugin_version: &str) -> String { } } -pub fn refresh_non_curated_plugin_cache( +#[cfg(test)] +pub(crate) fn refresh_non_curated_plugin_cache( codex_home: &Path, additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], ) -> Result { + collapse_non_curated_cache_refresh(refresh_non_curated_plugin_cache_detailed( + codex_home, + additional_roots, + configured_plugin_keys, + )) +} + +pub(crate) fn refresh_non_curated_plugin_cache_detailed( + codex_home: &Path, + additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], +) -> Result { refresh_non_curated_plugin_cache_with_mode( codex_home, additional_roots, + configured_plugin_keys, NonCuratedCacheRefreshMode::IfVersionChanged, ) } -pub fn refresh_non_curated_plugin_cache_force_reinstall( +#[cfg(test)] +pub(crate) fn refresh_non_curated_plugin_cache_force_reinstall( codex_home: &Path, additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], ) -> Result { + collapse_non_curated_cache_refresh(refresh_non_curated_plugin_cache_force_reinstall_detailed( + codex_home, + additional_roots, + configured_plugin_keys, + )) +} + +pub(crate) fn refresh_non_curated_plugin_cache_force_reinstall_detailed( + codex_home: &Path, + additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], +) -> Result { refresh_non_curated_plugin_cache_with_mode( codex_home, additional_roots, + configured_plugin_keys, NonCuratedCacheRefreshMode::ForceReinstall, ) } @@ -435,16 +478,32 @@ pub fn refresh_non_curated_plugin_cache_force_reinstall( fn refresh_non_curated_plugin_cache_with_mode( codex_home: &Path, additional_roots: &[AbsolutePathBuf], + configured_plugin_keys: &[String], mode: NonCuratedCacheRefreshMode, -) -> Result { - let configured_non_curated_plugin_ids = - non_curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( - codex_home, - "failed to read user config while refreshing non-curated plugin cache", - "failed to parse user config while refreshing non-curated plugin cache", - )); +) -> Result { + let mut configured_non_curated_plugin_ids = configured_plugin_keys + .iter() + .filter_map(|plugin_key| match PluginId::parse(plugin_key) { + Ok(plugin_id) if !is_openai_curated_marketplace_name(&plugin_id.marketplace_name) => { + Some(plugin_id) + } + Ok(_) => None, + Err(err) => { + warn!( + plugin_key, + error = %err, + "ignoring invalid plugin key during non-curated cache refresh setup" + ); + None + } + }) + .collect::>(); + configured_non_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); if configured_non_curated_plugin_ids.is_empty() { - return Ok(false); + return Ok(NonCuratedCacheRefreshOutcome { + cache_refreshed: false, + errors: Vec::new(), + }); } let configured_non_curated_plugin_keys = configured_non_curated_plugin_ids .iter() @@ -452,7 +511,7 @@ fn refresh_non_curated_plugin_cache_with_mode( .collect::>(); let store = PluginStore::try_new(codex_home.to_path_buf()).map_err(|err| err.to_string())?; - let marketplace_outcome = list_marketplaces(additional_roots) + let marketplace_outcome = list_marketplaces_with_home(additional_roots, /*home_dir*/ None) .map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?; let mut plugin_sources = HashMap::)>::new(); @@ -462,14 +521,18 @@ fn refresh_non_curated_plugin_cache_with_mode( } for plugin in marketplace.plugins { - let plugin_id = - PluginId::new(plugin.name.clone(), marketplace.name.clone()).map_err(|err| { - match err { - PluginIdError::Invalid(message) => { - format!("failed to prepare non-curated plugin cache refresh: {message}") - } - } - })?; + let plugin_id = match PluginId::new(plugin.name.clone(), marketplace.name.clone()) { + Ok(plugin_id) => plugin_id, + Err(PluginIdError::Invalid(message)) => { + warn!( + plugin = plugin.name, + marketplace = marketplace.name, + error = %message, + "ignoring invalid plugin entry during cache refresh" + ); + continue; + } + }; let plugin_key = plugin_id.as_key(); if !configured_non_curated_plugin_keys.contains(&plugin_key) { continue; @@ -504,6 +567,7 @@ fn refresh_non_curated_plugin_cache_with_mode( } let mut cache_refreshed = false; + let mut refresh_errors = Vec::new(); for plugin_id in configured_non_curated_plugin_ids { let plugin_key = plugin_id.as_key(); let Some((source, manifest_fallback_contents)) = plugin_sources.get(&plugin_key).cloned() @@ -515,49 +579,70 @@ fn refresh_non_curated_plugin_cache_with_mode( ); continue; }; - let materialized = - materialize_marketplace_plugin_source(codex_home, &source).map_err(|err| { - format!("failed to materialize plugin source for {plugin_key}: {err}") - })?; - let source_path = materialized.path.clone(); - let plugin_version = match manifest_fallback_contents.as_deref() { - Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest( - source_path.as_path(), - manifest_contents, - ), - None => plugin_version_for_source(source_path.as_path()), - } - .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; + let refresh_result = (|| -> Result { + let materialized = + materialize_marketplace_plugin_source(codex_home, &source).map_err(|err| { + format!("failed to materialize plugin source for {plugin_key}: {err}") + })?; + let source_path = materialized.path; + let plugin_version = match manifest_fallback_contents.as_deref() { + Some(manifest_contents) => plugin_version_for_source_with_fallback_manifest( + source_path.as_path(), + manifest_contents, + ), + None => plugin_version_for_source(source_path.as_path()), + } + .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; - if mode == NonCuratedCacheRefreshMode::IfVersionChanged - && store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str()) - { - continue; - } + if mode == NonCuratedCacheRefreshMode::IfVersionChanged + && store.active_plugin_version(&plugin_id).as_deref() + == Some(plugin_version.as_str()) + { + return Ok(false); + } - match manifest_fallback_contents.as_deref() { - Some(manifest_contents) => store.install_with_version_and_fallback_manifest( - source_path, - plugin_id.clone(), - plugin_version, - manifest_contents, - ), - None => store.install_with_version(source_path, plugin_id.clone(), plugin_version), + match manifest_fallback_contents.as_deref() { + Some(manifest_contents) => store.install_with_version_and_fallback_manifest( + source_path, + plugin_id.clone(), + plugin_version, + manifest_contents, + ), + None => store.install_with_version(source_path, plugin_id.clone(), plugin_version), + } + .map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?; + Ok(true) + })(); + match refresh_result { + Ok(refreshed) => cache_refreshed |= refreshed, + Err(message) => refresh_errors.push(NonCuratedCacheRefreshError { + marketplace_name: plugin_id.marketplace_name, + message, + }), } - .map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?; - cache_refreshed = true; } - Ok(cache_refreshed) + Ok(NonCuratedCacheRefreshOutcome { + cache_refreshed, + errors: refresh_errors, + }) } -fn configured_plugins_from_stack( - config_layer_stack: &ConfigLayerStack, -) -> HashMap { - let Some(user_config) = config_layer_stack.effective_user_config() else { - return HashMap::new(); - }; - configured_plugins_from_user_config_value(&user_config) +#[cfg(test)] +fn collapse_non_curated_cache_refresh( + outcome: Result, +) -> Result { + let outcome = outcome?; + if outcome.errors.is_empty() { + Ok(outcome.cache_refreshed) + } else { + Err(outcome + .errors + .into_iter() + .map(|error| error.message) + .collect::>() + .join("; ")) + } } fn is_full_git_sha(value: &str) -> bool { @@ -647,20 +732,6 @@ fn curated_plugin_ids_from_config_keys( configured_curated_plugin_ids } -fn non_curated_plugin_ids_from_config_keys( - configured_plugins: HashMap, -) -> Vec { - let mut configured_non_curated_plugin_ids = configured_plugin_ids( - configured_plugins, - "ignoring invalid plugin key during non-curated cache refresh setup", - ) - .into_iter() - .filter(|plugin_id| !is_openai_curated_marketplace_name(&plugin_id.marketplace_name)) - .collect::>(); - configured_non_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); - configured_non_curated_plugin_ids -} - pub fn configured_curated_plugin_ids_from_codex_home(codex_home: &Path) -> Vec { curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( codex_home, diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index d4faaa8e7a..8e5a76b4bf 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -43,7 +43,7 @@ fn configured_plugins_from_stack_merges_user_layers() { ) .expect("valid config layer stack"); - let plugins = configured_plugins_from_stack(&stack); + let plugins = configured_plugins_from_stack(&stack, temp_dir.path()); assert_eq!( plugins, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 39e5152abd..4d215b61d4 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -16,8 +16,8 @@ use crate::loader::log_plugin_load_errors; use crate::loader::materialize_marketplace_plugin_source; use crate::loader::plugin_capability_summary_from_root; use crate::loader::refresh_curated_plugin_cache; -use crate::loader::refresh_non_curated_plugin_cache; -use crate::loader::refresh_non_curated_plugin_cache_force_reinstall; +use crate::loader::refresh_non_curated_plugin_cache_detailed; +use crate::loader::refresh_non_curated_plugin_cache_force_reinstall_detailed; use crate::loader::remote_installed_plugins_to_config; use crate::manifest::PluginManifestInterface; use crate::manifest::load_plugin_manifest; @@ -32,9 +32,12 @@ use crate::marketplace::MarketplacePluginSource; use crate::marketplace::ResolvedMarketplacePlugin; use crate::marketplace::find_installable_marketplace_plugin; use crate::marketplace::find_marketplace_plugin; -use crate::marketplace::list_marketplaces; +use crate::marketplace::home_dir; +use crate::marketplace::list_marketplaces_with_home; use crate::marketplace::plugin_interface_with_marketplace_category; use crate::marketplace_policy::MarketplacePolicy; +use crate::marketplace_policy::allowed_configured_marketplace_names; +use crate::marketplace_policy::configured_plugins_from_stack; use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError; use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; use crate::marketplace_upgrade::upgrade_configured_git_marketplaces; @@ -84,6 +87,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::PluginSkillRoot; use std::collections::HashMap; use std::collections::HashSet; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::RwLock; @@ -195,6 +199,7 @@ pub struct PluginListBackgroundTaskOptions { #[derive(Clone, PartialEq, Eq)] struct NonCuratedCacheRefreshRequest { roots: Vec, + configured_plugin_keys: Vec, mode: NonCuratedCacheRefreshMode, } @@ -388,9 +393,16 @@ struct PluginLoadCacheKey { } impl PluginLoadCacheKey { - fn from_config(config: &PluginsConfigInput, remote_global_catalog_active: bool) -> Self { + fn from_config( + config: &PluginsConfigInput, + codex_home: &Path, + remote_global_catalog_active: bool, + ) -> Self { Self { - configured_plugins: configured_plugins_from_stack(&config.config_layer_stack), + configured_plugins: configured_plugins_from_stack( + &config.config_layer_stack, + codex_home, + ), skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack), remote_global_catalog_active, } @@ -494,8 +506,11 @@ impl PluginsManager { if !config.plugins_enabled { return None; } - let key = - PluginLoadCacheKey::from_config(config, self.remote_global_catalog_active(config)); + let key = PluginLoadCacheKey::from_config( + config, + self.codex_home.as_path(), + self.remote_global_catalog_active(config), + ); self.loaded_plugins_cache .read() .unwrap_or_else(std::sync::PoisonError::into_inner) @@ -525,7 +540,11 @@ impl PluginsManager { } let remote_global_catalog_active = self.remote_global_catalog_active(config); - let cache_key = PluginLoadCacheKey::from_config(config, remote_global_catalog_active); + let cache_key = PluginLoadCacheKey::from_config( + config, + self.codex_home.as_path(), + remote_global_catalog_active, + ); if !force_reload && let Some(plugins) = self.cached_loaded_plugins(&cache_key) { return self.resolve_loaded_plugins_for_auth(plugins); } @@ -1007,7 +1026,7 @@ impl PluginsManager { options: PluginListBackgroundTaskOptions, on_effective_plugins_changed: Option>, ) { - self.maybe_start_non_curated_plugin_cache_refresh(roots); + self.maybe_start_non_curated_plugin_cache_refresh(config, roots); if options.refresh_global_remote_catalog_cache { self.maybe_start_global_remote_catalog_cache_refresh(config, auth.clone()); } @@ -1547,7 +1566,7 @@ impl PluginsManager { let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); let marketplace_roots = self.marketplace_roots(config, additional_roots, include_openai_curated); - let marketplace_outcome = list_marketplaces(&marketplace_roots)?; + let marketplace_outcome = self.list_marketplaces_with_policy(config, &marketplace_roots)?; let mut seen_plugin_keys = HashSet::new(); let marketplaces = marketplace_outcome .marketplaces @@ -1636,11 +1655,12 @@ impl PluginsManager { return Ok(MarketplaceListOutcome::default()); } - list_marketplaces(&self.marketplace_roots( + let marketplace_roots = self.marketplace_roots( config, additional_roots, /*include_openai_curated*/ true, - )) + ); + self.list_marketplaces_with_policy(config, &marketplace_roots) } pub(crate) async fn tool_suggest_metadata_for_marketplace_plugin( @@ -1666,6 +1686,17 @@ impl PluginsManager { } let plugin = find_marketplace_plugin(&request.marketplace_path, &request.plugin_name)?; + MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()) + .validate_install( + &config.config_layer_stack, + self.codex_home.as_path(), + &request.marketplace_path, + &plugin.plugin_id.marketplace_name, + ) + .map_err(|message| MarketplaceError::InvalidMarketplaceFile { + path: request.marketplace_path.to_path_buf(), + message, + })?; if !self.restriction_product_matches(plugin.policy.products.as_deref()) { return Err(MarketplaceError::PluginNotFound { plugin_name: plugin.plugin_id.plugin_name, @@ -2023,12 +2054,30 @@ impl PluginsManager { )); } if !outcome.upgraded_roots.is_empty() { - match refresh_non_curated_plugin_cache_force_reinstall( + let mut configured_plugin_keys = configured_plugins_from_stack( + &config.config_layer_stack, + self.codex_home.as_path(), + ) + .into_keys() + .collect::>(); + configured_plugin_keys.sort_unstable(); + match refresh_non_curated_plugin_cache_force_reinstall_detailed( self.codex_home.as_path(), &outcome.upgraded_roots, + &configured_plugin_keys, ) { - Ok(cache_refreshed) => { - self.clear_caches_after_marketplace_source_refresh(cache_refreshed); + Ok(refresh_outcome) => { + self.clear_caches_after_marketplace_source_refresh( + refresh_outcome.cache_refreshed, + ); + outcome + .errors + .extend(refresh_outcome.errors.into_iter().map(|error| { + ConfiguredMarketplaceUpgradeError { + marketplace_name: error.marketplace_name, + message: error.message, + } + })); } Err(err) => { self.clear_cache(); @@ -2048,9 +2097,11 @@ impl PluginsManager { pub fn maybe_start_non_curated_plugin_cache_refresh( self: &Arc, + config: &PluginsConfigInput, roots: &[AbsolutePathBuf], ) { self.schedule_non_curated_plugin_cache_refresh( + config, roots, NonCuratedCacheRefreshMode::IfVersionChanged, ); @@ -2127,38 +2178,81 @@ impl PluginsManager { fn schedule_non_curated_plugin_cache_refresh( self: &Arc, + config: &PluginsConfigInput, roots: &[AbsolutePathBuf], mode: NonCuratedCacheRefreshMode, ) { - let mut roots = roots.to_vec(); + let marketplace_roots = + self.marketplace_roots(config, roots, /*include_openai_curated*/ false); + let outcome = match self.list_marketplaces_with_policy(config, &marketplace_roots) { + Ok(outcome) => outcome, + Err(err) => { + warn!("failed to prepare non-curated plugin cache refresh: {err}"); + return; + } + }; + let policy = MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()); + let mut roots = outcome + .marketplaces + .into_iter() + .filter(|marketplace| !is_openai_curated_marketplace_name(&marketplace.name)) + .filter_map(|marketplace| { + match policy.validate_install( + &config.config_layer_stack, + self.codex_home.as_path(), + &marketplace.path, + &marketplace.name, + ) { + Ok(()) => Some(marketplace.path), + Err(err) => { + warn!( + marketplace = marketplace.name, + path = %marketplace.path.display(), + error = %err, + "skipping marketplace source during plugin cache refresh" + ); + None + } + } + }) + .collect::>(); roots.sort_unstable(); roots.dedup(); - if roots.is_empty() { + let mut configured_plugin_keys = + configured_plugins_from_stack(&config.config_layer_stack, self.codex_home.as_path()) + .into_keys() + .collect::>(); + configured_plugin_keys.sort_unstable(); + if roots.is_empty() || configured_plugin_keys.is_empty() { return; } - let request = NonCuratedCacheRefreshRequest { roots, mode }; + let mut request = NonCuratedCacheRefreshRequest { + roots, + configured_plugin_keys, + mode, + }; let should_spawn = { let mut state = match self.non_curated_cache_refresh_state.write() { Ok(state) => state, Err(err) => err.into_inner(), }; - // Collapse repeated plugin/list requests onto one worker and only queue another pass - // when the requested roots set actually changes. Forced reinstall requests are not - // deduped against the last completed pass because the same marketplace root path can - // point at newly activated files after an auto-upgrade. - if state.requested.as_ref() == Some(&request) - || (mode == NonCuratedCacheRefreshMode::IfVersionChanged - && !state.in_flight - && state.last_refreshed.as_ref() == Some(&request)) - { - return; - } - if mode == NonCuratedCacheRefreshMode::IfVersionChanged + if request.mode == NonCuratedCacheRefreshMode::IfVersionChanged && state.requested.as_ref().is_some_and(|requested| { requested.mode == NonCuratedCacheRefreshMode::ForceReinstall && requested.roots == request.roots }) + { + request.mode = NonCuratedCacheRefreshMode::ForceReinstall; + } + // Collapse repeated plugin/list requests onto one worker and only queue another pass + // when the roots or configured plugin set changes. Forced reinstall requests are not + // deduped against the last completed pass because the same marketplace root path can + // point at newly activated files after an auto-upgrade. + if state.requested.as_ref() == Some(&request) + || (request.mode == NonCuratedCacheRefreshMode::IfVersionChanged + && !state.in_flight + && state.last_refreshed.as_ref() == Some(&request)) { return; } @@ -2349,21 +2443,33 @@ impl PluginsManager { let refresh_result = match request.mode { NonCuratedCacheRefreshMode::IfVersionChanged => { - refresh_non_curated_plugin_cache(self.codex_home.as_path(), &request.roots) - } - NonCuratedCacheRefreshMode::ForceReinstall => { - refresh_non_curated_plugin_cache_force_reinstall( + refresh_non_curated_plugin_cache_detailed( self.codex_home.as_path(), &request.roots, + &request.configured_plugin_keys, + ) + } + NonCuratedCacheRefreshMode::ForceReinstall => { + refresh_non_curated_plugin_cache_force_reinstall_detailed( + self.codex_home.as_path(), + &request.roots, + &request.configured_plugin_keys, ) } }; let refreshed = match refresh_result { - Ok(cache_refreshed) => { - if cache_refreshed { + Ok(refresh_outcome) => { + if refresh_outcome.cache_refreshed { self.clear_cache(); } - true + for error in &refresh_outcome.errors { + warn!( + marketplace = error.marketplace_name, + error = %error.message, + "failed to refresh configured plugin cache" + ); + } + refresh_outcome.errors.is_empty() } Err(err) => { self.clear_cache(); @@ -2391,7 +2497,8 @@ impl PluginsManager { &self, config: &PluginsConfigInput, ) -> (HashSet, HashSet) { - let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); + let configured_plugins = + configured_plugins_from_stack(&config.config_layer_stack, self.codex_home.as_path()); let installed_plugins = configured_plugins .keys() .filter(|plugin_key| { @@ -2448,6 +2555,27 @@ impl PluginsManager { roots.dedup(); roots } + + fn list_marketplaces_with_policy( + &self, + config: &PluginsConfigInput, + roots: &[AbsolutePathBuf], + ) -> Result { + let mut outcome = list_marketplaces_with_home(roots, home_dir().as_deref())?; + let policy = MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()); + if !policy.is_restricted() { + return Ok(outcome); + } + let allowed_marketplace_names = allowed_configured_marketplace_names( + &config.config_layer_stack, + self.codex_home.as_path(), + ); + outcome.marketplaces.retain(|marketplace| { + is_openai_curated_marketplace_name(&marketplace.name) + || allowed_marketplace_names.contains(&marketplace.name) + }); + Ok(outcome) + } } pub(crate) fn remote_plugin_install_required_description( @@ -2612,31 +2740,6 @@ impl PluginUninstallError { } } -pub(crate) fn configured_plugins_from_stack( - config_layer_stack: &ConfigLayerStack, -) -> HashMap { - // Plugin entries remain persisted user config only. - let Some(user_config) = config_layer_stack.effective_user_config() else { - return HashMap::new(); - }; - configured_plugins_from_user_config_value(&user_config) -} - -fn configured_plugins_from_user_config_value( - user_config: &toml::Value, -) -> HashMap { - let Some(plugins_value) = user_config.get("plugins") else { - return HashMap::new(); - }; - match plugins_value.clone().try_into() { - Ok(plugins) => plugins, - Err(err) => { - warn!("invalid plugins config: {err}"); - HashMap::new() - } - } -} - #[cfg(test)] #[path = "manager_tests.rs"] mod tests; diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index cefca287ad..06ebf89a76 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -100,6 +100,19 @@ fn config_layer_stack_with_requirements( .expect("build config layer stack") } +fn plugins_config_input_with_requirements( + codex_home: &Path, + user_config: &str, + requirements: &str, +) -> PluginsConfigInput { + PluginsConfigInput::new( + config_layer_stack_with_requirements(codex_home, user_config, requirements), + /*plugins_enabled*/ true, + /*remote_plugin_enabled*/ false, + String::new(), + ) +} + #[test] fn plugins_manager_tracks_auth_mode() { let tmp = TempDir::new().unwrap(); @@ -122,6 +135,184 @@ fn plugins_manager_tracks_auth_mode() { assert_eq!(manager_with_auth.auth_mode(), Some(AuthMode::Chatgpt)); } +#[tokio::test] +async fn marketplace_policy_projection_disables_installed_plugin_and_invalidates_cache() { + let codex_home = TempDir::new().expect("create Codex home"); + write_plugin( + &codex_home.path().join("plugins/cache/company"), + "sample/local", + "sample", + ); + let user_config = r#" +[marketplaces.company] +source_type = "git" +source = "https://github.com/example/company.git" + +[plugins."sample@company"] +enabled = true +"#; + let allowed = plugins_config_input_with_requirements( + codex_home.path(), + user_config, + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.company] +source = "git" +url = "https://github.com/example/company.git" +"#, + ); + let blocked = plugins_config_input_with_requirements( + codex_home.path(), + user_config, + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.other] +source = "git" +url = "https://github.com/example/other.git" +"#, + ); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + + let allowed_outcome = manager.plugins_for_config(&allowed).await; + assert_eq!(allowed_outcome.plugins().len(), 1); + assert_eq!(allowed_outcome.plugins()[0].config_name, "sample@company"); + + let blocked_outcome = manager.plugins_for_config(&blocked).await; + assert_eq!(blocked_outcome, PluginLoadOutcome::default()); +} + +#[tokio::test] +async fn plugin_read_rejects_marketplace_blocked_by_requirements() { + let codex_home = TempDir::new().expect("create Codex home"); + let marketplace_root = codex_home.path().join("marketplace"); + write_plugin(&marketplace_root, "sample", "sample"); + write_file( + &marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "company", + "plugins": [ + { + "name": "sample", + "source": {"source": "local", "path": "./sample"} + } + ] +}"#, + ); + let config = plugins_config_input_with_requirements( + codex_home.path(), + "", + r#" +[marketplaces] +restrict_to_allowed_sources = true +"#, + ); + let marketplace_path = + AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json")) + .expect("absolute marketplace path"); + + let err = PluginsManager::new(codex_home.path().to_path_buf()) + .read_plugin_for_config( + &config, + &PluginReadRequest { + plugin_name: "sample".to_string(), + marketplace_path, + }, + ) + .await + .expect_err("blocked marketplace should not be readable"); + assert!(matches!( + err, + MarketplaceError::InvalidMarketplaceFile { .. } + )); +} + +#[test] +fn marketplace_policy_filters_discovered_marketplaces_by_configured_name() { + let codex_home = TempDir::new().expect("create Codex home"); + let repo_root = codex_home.path().join("repo"); + let subdirectory = repo_root.join("worktree/subdirectory"); + fs::create_dir_all(&subdirectory).expect("create input subdirectory"); + write_plugin(&repo_root, "sample", "sample"); + write_file( + &repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "company", + "plugins": [ + { + "name": "sample", + "source": {"source": "local", "path": "./sample"} + } + ] +}"#, + ); + init_git_repo(&repo_root); + let repo_root = AbsolutePathBuf::try_from(repo_root).expect("absolute repository root"); + let subdirectory = + AbsolutePathBuf::try_from(subdirectory).expect("absolute input subdirectory"); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let user_config = format!( + r#" +[marketplaces.company] +source_type = "local" +source = {:?} +"#, + repo_root.as_path() + ); + let allowed = plugins_config_input_with_requirements( + codex_home.path(), + &user_config, + &format!( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.company] +source = "local" +path = {:?} +"#, + repo_root.as_path() + ), + ); + let blocked = plugins_config_input_with_requirements( + codex_home.path(), + &user_config, + &format!( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.subdirectory] +source = "local" +path = {:?} +"#, + subdirectory.as_path() + ), + ); + + let allowed_outcome = manager + .list_marketplaces_for_config( + &allowed, + std::slice::from_ref(&subdirectory), + /*include_openai_curated*/ false, + ) + .expect("list allowed marketplace"); + assert_eq!(allowed_outcome.marketplaces.len(), 1); + assert_eq!(allowed_outcome.marketplaces[0].name, "company"); + + let blocked_outcome = manager + .list_marketplaces_for_config( + &blocked, + std::slice::from_ref(&subdirectory), + /*include_openai_curated*/ false, + ) + .expect("list blocked marketplace"); + assert_eq!(blocked_outcome.marketplaces, Vec::new()); +} + fn write_auth_projection_plugin(codex_home: &Path, name: &str, include_app: bool) { let plugin_root = codex_home .join("plugins/cache") @@ -3348,7 +3539,11 @@ plugins = true .unwrap() .marketplaces .into_iter() - .find(|marketplace| marketplace.name == "debug") + .find(|marketplace| { + marketplace.path + == AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")) + .unwrap() + }) .unwrap() .plugins .into_iter() @@ -3785,7 +3980,7 @@ enabled = true let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) .list_marketplaces_for_config( &config, - &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()], /*include_openai_curated*/ true, ) .unwrap() @@ -3793,7 +3988,11 @@ enabled = true let marketplace = marketplaces .into_iter() - .find(|marketplace| marketplace.name == "debug") + .find(|marketplace| { + marketplace.path + == AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")) + .unwrap() + }) .expect("debug marketplace should be listed"); let mut plugins = marketplace.plugins; @@ -5256,6 +5455,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should succeed") ); @@ -5308,6 +5508,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should reinstall missing configured plugin") ); @@ -5367,6 +5568,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should materialize configured Git plugin") ); @@ -5420,6 +5622,7 @@ enabled = true !refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should be a no-op when configured plugins are current") ); @@ -5473,6 +5676,7 @@ enabled = true refresh_non_curated_plugin_cache_force_reinstall( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should reinstall unchanged local version") ); @@ -5531,6 +5735,7 @@ enabled = true refresh_non_curated_plugin_cache( tmp.path(), &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["sample-plugin@debug".to_string()], ) .expect("cache refresh should ignore unrelated invalid plugin manifests") ); @@ -5542,6 +5747,47 @@ enabled = true ); } +#[test] +fn refresh_non_curated_plugin_cache_continues_after_plugin_error() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + write_plugin_with_version(&repo_root, "z-good", "z-good", Some("1.2.3")); + write_file( + &repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "a-broken", + "source": { + "source": "local", + "path": "./missing" + } + }, + { + "name": "z-good", + "source": { + "source": "local", + "path": "./z-good" + } + } + ] +}"#, + ); + + let err = refresh_non_curated_plugin_cache( + tmp.path(), + &[AbsolutePathBuf::try_from(repo_root).unwrap()], + &["a-broken@debug".to_string(), "z-good@debug".to_string()], + ) + .expect_err("broken plugin should be reported after refreshing the remaining plugins"); + + assert!(err.contains("a-broken@debug")); + assert!(tmp.path().join("plugins/cache/debug/z-good/1.2.3").is_dir()); +} + #[tokio::test] async fn load_plugins_ignores_project_config_files() { let codex_home = TempDir::new().unwrap(); diff --git a/codex-rs/core-plugins/src/marketplace_policy.rs b/codex-rs/core-plugins/src/marketplace_policy.rs index 8e41fb893e..a4c99a81cf 100644 --- a/codex-rs/core-plugins/src/marketplace_policy.rs +++ b/codex-rs/core-plugins/src/marketplace_policy.rs @@ -18,9 +18,13 @@ use codex_config::MarketplaceAllowedSourceToml; use codex_config::RequirementSource; use codex_config::types::MarketplaceConfig; use codex_config::types::MarketplaceSourceType; +use codex_config::types::PluginConfig; +use codex_plugin::PluginId; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path::paths_match_after_normalization; use regex::Regex; +use std::collections::HashMap; +use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; use url::Url; @@ -70,7 +74,7 @@ impl MarketplacePolicy { } } - fn is_restricted(&self) -> bool { + pub(crate) fn is_restricted(&self) -> bool { self.restricted.is_some() } @@ -198,6 +202,104 @@ impl AllowedMarketplaceSource { } } +pub(crate) fn project_effective_user_config( + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> Option { + let mut user_config = config_layer_stack.effective_user_config()?; + let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements()); + if !policy.is_restricted() { + return Some(user_config); + } + let allowed_marketplace_names = + allowed_configured_marketplace_names_with_policy(&user_config, &policy, codex_home); + let configured_marketplace_names = user_config + .get("marketplaces") + .and_then(toml::Value::as_table) + .map(|marketplaces| marketplaces.keys().cloned().collect::>()) + .unwrap_or_default(); + + if let Some(marketplaces) = user_config + .get_mut("marketplaces") + .and_then(toml::Value::as_table_mut) + { + marketplaces + .retain(|marketplace_name, _| allowed_marketplace_names.contains(marketplace_name)); + } + if let Some(plugins) = user_config + .get_mut("plugins") + .and_then(toml::Value::as_table_mut) + { + plugins.retain(|plugin_key, _| { + let Ok(plugin_id) = PluginId::parse(plugin_key) else { + return false; + }; + (is_openai_curated_marketplace_name(&plugin_id.marketplace_name) + && !configured_marketplace_names.contains(&plugin_id.marketplace_name)) + || allowed_marketplace_names.contains(&plugin_id.marketplace_name) + }); + } + Some(user_config) +} + +pub fn allowed_configured_marketplace_names( + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> HashSet { + let Some(user_config) = config_layer_stack.effective_user_config() else { + return HashSet::new(); + }; + let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements()); + allowed_configured_marketplace_names_with_policy(&user_config, &policy, codex_home) +} + +fn allowed_configured_marketplace_names_with_policy( + user_config: &toml::Value, + policy: &MarketplacePolicy, + codex_home: &Path, +) -> HashSet { + let Some(marketplaces) = user_config + .get("marketplaces") + .and_then(toml::Value::as_table) + else { + return HashSet::new(); + }; + if !policy.is_restricted() { + return marketplaces.keys().cloned().collect(); + } + marketplaces + .iter() + .filter_map(|(marketplace_name, marketplace)| { + let allowed = match managed_marketplace_config_name(codex_home, marketplace) { + Some(expected_name) => expected_name == marketplace_name, + None => policy + .validate_configured_marketplace(marketplace_name, marketplace) + .is_ok(), + }; + allowed.then(|| marketplace_name.clone()) + }) + .collect() +} + +pub(crate) fn configured_plugins_from_stack( + config_layer_stack: &ConfigLayerStack, + codex_home: &Path, +) -> HashMap { + let Some(user_config) = project_effective_user_config(config_layer_stack, codex_home) else { + return HashMap::new(); + }; + let Some(plugins_value) = user_config.get("plugins") else { + return HashMap::new(); + }; + match plugins_value.clone().try_into() { + Ok(plugins) => plugins, + Err(err) => { + tracing::warn!("invalid plugins config: {err}"); + HashMap::new() + } + } +} + pub(crate) fn validate_marketplace_source_for_add( codex_home: &Path, requirements: &ConfigRequirements, @@ -358,6 +460,21 @@ fn managed_marketplace_name( managed_local_marketplace_name(codex_home, root.as_path()) } +fn managed_marketplace_config_name( + codex_home: &Path, + marketplace: &toml::Value, +) -> Option<&'static str> { + if marketplace.get("source_type").and_then(toml::Value::as_str) != Some("local") { + return None; + } + let path = marketplace + .get("source") + .and_then(toml::Value::as_str) + .map(Path::new) + .filter(|path| path.is_absolute())?; + managed_local_marketplace_name(codex_home, path) +} + fn managed_local_marketplace_name(codex_home: &Path, root: &Path) -> Option<&'static str> { for marketplace_name in [ OPENAI_BUNDLED_MARKETPLACE_NAME, diff --git a/codex-rs/core-plugins/src/marketplace_policy_tests.rs b/codex-rs/core-plugins/src/marketplace_policy_tests.rs index 63518dfb3d..75c0a2f17c 100644 --- a/codex-rs/core-plugins/src/marketplace_policy_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_policy_tests.rs @@ -383,6 +383,197 @@ restrict_to_allowed_sources = true assert!(validate_marketplace_name_for_add(expected_name, "other").is_err()); } +#[test] +fn projected_user_config_removes_blocked_marketplaces_and_plugins() { + let codex_home = TempDir::new().expect("create Codex home"); + let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml")) + .expect("absolute config path"); + let stack = config_layer_stack_with_user_config( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.company] +source = "git" +url = "https://github.com/example/allowed.git" +"#, + Some(( + r#" +[marketplaces.allowed] +source_type = "git" +source = "https://github.com/example/allowed.git" + +[marketplaces.blocked] +source_type = "git" +source = "https://github.com/example/blocked.git" + +[plugins."sample@allowed"] +enabled = true + +[plugins."sample@blocked"] +enabled = true +"#, + config_file, + )), + ); + + let projected = + project_effective_user_config(&stack, codex_home.path()).expect("project user config"); + assert_eq!( + projected["marketplaces"] + .as_table() + .expect("projected marketplaces") + .keys() + .cloned() + .collect::>(), + vec!["allowed".to_string()] + ); + assert_eq!( + configured_plugins_from_stack(&stack, codex_home.path()) + .into_keys() + .collect::>(), + vec!["sample@allowed".to_string()] + ); + + let raw = stack.effective_user_config().expect("raw user config"); + assert!(raw["marketplaces"]["blocked"].is_table()); + assert!(raw["plugins"]["sample@blocked"].is_table()); +} + +#[test] +fn managed_bundled_config_is_retained_only_at_its_owned_path() { + let codex_home = TempDir::new().expect("create Codex home"); + let bundled_root = codex_home + .path() + .join(".tmp/bundled-marketplaces") + .join(crate::OPENAI_BUNDLED_MARKETPLACE_NAME); + let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml")) + .expect("absolute config path"); + let stack = config_layer_stack_with_user_config( + r#" +[marketplaces] +restrict_to_allowed_sources = true +"#, + Some(( + &format!( + r#" +[marketplaces.openai-bundled] +source_type = "local" +source = {bundled_root:?} + +[marketplaces.openai-bundled-alpha] +source_type = "local" +source = "/tmp/not-managed" + +[marketplaces.evil] +source_type = "local" +source = {bundled_root:?} + +[plugins."sample@openai-bundled"] +enabled = true + +[plugins."sample@openai-bundled-alpha"] +enabled = true + +[plugins."sample@evil"] +enabled = true +"# + ), + config_file, + )), + ); + + let projected = + project_effective_user_config(&stack, codex_home.path()).expect("project user config"); + + assert_eq!( + projected["marketplaces"] + .as_table() + .expect("projected marketplaces") + .keys() + .cloned() + .collect::>(), + vec![crate::OPENAI_BUNDLED_MARKETPLACE_NAME.to_string()] + ); + assert_eq!( + projected["plugins"] + .as_table() + .expect("projected plugins") + .keys() + .cloned() + .collect::>(), + vec![format!("sample@{}", crate::OPENAI_BUNDLED_MARKETPLACE_NAME)] + ); +} + +#[test] +fn allowlisted_config_names_are_not_globally_reserved() { + let codex_home = TempDir::new().expect("create Codex home"); + let source_root = TempDir::new().expect("create marketplace root"); + let source_root = source_root + .path() + .canonicalize() + .expect("canonical marketplace root"); + let config_file = AbsolutePathBuf::try_from(codex_home.path().join("config.toml")) + .expect("absolute config path"); + let stack = config_layer_stack_with_user_config( + &format!( + r#" +[marketplaces] +restrict_to_allowed_sources = true + +[marketplaces.allowed_sources.local] +source = "local" +path = {source_root:?} +"# + ), + Some(( + &format!( + r#" +[marketplaces.openai-bundled] +source_type = "local" +source = {source_root:?} + +[marketplaces.openai-curated] +source_type = "local" +source = {source_root:?} + +[plugins."sample@openai-bundled"] +enabled = true + +[plugins."sample@openai-curated"] +enabled = true +"# + ), + config_file, + )), + ); + + let projected = + project_effective_user_config(&stack, codex_home.path()).expect("project user config"); + assert_eq!( + projected["marketplaces"] + .as_table() + .expect("projected marketplaces") + .keys() + .cloned() + .collect::>(), + vec!["openai-bundled".to_string(), "openai-curated".to_string()] + ); + assert_eq!( + projected["plugins"] + .as_table() + .expect("projected plugins") + .keys() + .cloned() + .collect::>(), + vec![ + "sample@openai-bundled".to_string(), + "sample@openai-curated".to_string() + ] + ); +} + #[test] fn blocked_upgrade_is_rejected_before_marketplace_installation() { let codex_home = TempDir::new().expect("create Codex home"); diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 5d78fefa90..bb4929dedb 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -37,6 +37,7 @@ pub struct PluginInstallResult { #[derive(Debug, Clone)] pub struct PluginStore { + codex_home: AbsolutePathBuf, root: AbsolutePathBuf, data_root: AbsolutePathBuf, } @@ -59,14 +60,24 @@ impl PluginStore { let data_root = AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_DATA_DIR)) .map_err(|err| PluginStoreError::io("failed to resolve plugin data root", err))?; + let codex_home = AbsolutePathBuf::from_absolute_path_checked(codex_home) + .map_err(|err| PluginStoreError::io("failed to resolve Codex home", err))?; - Ok(Self { root, data_root }) + Ok(Self { + codex_home, + root, + data_root, + }) } pub fn root(&self) -> &AbsolutePathBuf { &self.root } + pub(crate) fn codex_home(&self) -> &AbsolutePathBuf { + &self.codex_home + } + pub fn plugin_base_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf { self.root .join(&plugin_id.marketplace_name) diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 2791c887e6..9513dcc56c 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -80,6 +80,7 @@ async fn forward_events_cancelled_while_send_blocked_shuts_down_delegate() { status: None, call_id: "call-1".to_string(), name: "tool".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }, diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index 5c320b798d..cbe0209e21 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -513,6 +513,7 @@ fn for_prompt_strips_images_when_model_does_not_support_images() { status: None, call_id: "tool-1".to_string(), name: "js_repl".to_string(), + namespace: None, input: "view_image".to_string(), internal_chat_message_metadata_passthrough: None, }, @@ -582,6 +583,7 @@ fn for_prompt_strips_images_when_model_does_not_support_images() { status: None, call_id: "tool-1".to_string(), name: "js_repl".to_string(), + namespace: None, input: "view_image".to_string(), internal_chat_message_metadata_passthrough: None, }, @@ -1090,6 +1092,7 @@ fn remove_first_item_handles_custom_tool_pair() { status: None, call_id: "tool-1".to_string(), name: "my_tool".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }, @@ -1378,6 +1381,7 @@ fn normalize_adds_missing_output_for_custom_tool_call() { status: None, call_id: "tool-x".to_string(), name: "custom".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }]; @@ -1393,6 +1397,7 @@ fn normalize_adds_missing_output_for_custom_tool_call() { status: None, call_id: "tool-x".to_string(), name: "custom".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }, @@ -1512,6 +1517,7 @@ fn normalize_mixed_inserts_and_removals() { status: None, call_id: "t1".to_string(), name: "tool".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }, @@ -1556,6 +1562,7 @@ fn normalize_mixed_inserts_and_removals() { status: None, call_id: "t1".to_string(), name: "tool".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }, @@ -1622,6 +1629,49 @@ fn normalize_adds_missing_output_for_function_call_inserts_output() { ); } +#[test] +fn for_prompt_assigns_stable_id_to_synthetic_output_without_reordering_history() { + let items = vec![ + ResponseItem::FunctionCall { + id: Some("fc_existing".to_string()), + name: "do_it".to_string(), + namespace: None, + arguments: "{}".to_string(), + call_id: "call-x".to_string(), + internal_chat_message_metadata_passthrough: None, + }, + ResponseItem::Message { + id: Some("msg_later".to_string()), + role: "user".to_string(), + content: vec![ContentItem::InputText { + text: "later turn".to_string(), + }], + phase: None, + internal_chat_message_metadata_passthrough: None, + }, + ]; + + let first = create_history_with_items(items.clone()).for_prompt(&default_input_modalities()); + let second = create_history_with_items(items).for_prompt(&default_input_modalities()); + + assert_eq!( + first, second, + "repeated prompt projections should assign the same ID to the synthetic output" + ); + let [ + ResponseItem::FunctionCall { .. }, + ResponseItem::FunctionCallOutput { id: Some(id), .. }, + ResponseItem::Message { .. }, + ] = first.as_slice() + else { + panic!("expected the synthetic output between its call and the later message"); + }; + assert!( + id.starts_with("fco_"), + "the synthetic function call output should use the Responses API output ID prefix" + ); +} + #[test] fn normalize_adds_missing_output_for_tool_search_call() { let items = vec![ResponseItem::ToolSearchCall { @@ -1668,6 +1718,7 @@ fn normalize_adds_missing_output_for_custom_tool_call_panics_in_debug() { status: None, call_id: "tool-x".to_string(), name: "custom".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }]; @@ -1810,6 +1861,7 @@ fn normalize_mixed_inserts_and_removals_panics_in_debug() { status: None, call_id: "t1".to_string(), name: "tool".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }, diff --git a/codex-rs/core/src/context_manager/normalize.rs b/codex-rs/core/src/context_manager/normalize.rs index d11cffe4e9..3fd1a59a8f 100644 --- a/codex-rs/core/src/context_manager/normalize.rs +++ b/codex-rs/core/src/context_manager/normalize.rs @@ -4,12 +4,15 @@ use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::InputModality; use std::collections::HashSet; +use uuid::Uuid; use crate::util::error_or_panic; use tracing::info; const IMAGE_CONTENT_OMITTED_PLACEHOLDER: &str = "image content omitted because you do not support image input"; +// Changing this value would change model-visible IDs and invalidate prompt caches. +const SYNTHETIC_OUTPUT_ID_NAMESPACE: Uuid = Uuid::from_u128(0x90d38d3e_6a5b_4d52_bfe2_2f1e634bfac4); pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { let mut function_output_ids = HashSet::new(); @@ -40,14 +43,14 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { for (idx, item) in items.iter().enumerate() { match item { - ResponseItem::FunctionCall { call_id, .. } + ResponseItem::FunctionCall { id, call_id, .. } if !function_output_ids.contains(call_id.as_str()) => { info!("Function call output is missing for call id: {call_id}"); missing_outputs_to_insert.push(( idx, ResponseItem::FunctionCallOutput { - id: None, + id: synthetic_output_id("fco", id.as_deref()), call_id: call_id.clone(), output: FunctionCallOutputPayload::from_text("aborted".to_string()), internal_chat_message_metadata_passthrough: None, @@ -55,6 +58,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { )); } ResponseItem::ToolSearchCall { + id, call_id: Some(call_id), .. } if !tool_search_output_ids.contains(call_id.as_str()) => { @@ -62,7 +66,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { missing_outputs_to_insert.push(( idx, ResponseItem::ToolSearchOutput { - id: None, + id: synthetic_output_id("tso", id.as_deref()), call_id: Some(call_id.clone()), status: "completed".to_string(), execution: "client".to_string(), @@ -71,7 +75,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { }, )); } - ResponseItem::CustomToolCall { call_id, .. } + ResponseItem::CustomToolCall { id, call_id, .. } if !custom_tool_output_ids.contains(call_id.as_str()) => { error_or_panic(format!( @@ -80,7 +84,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { missing_outputs_to_insert.push(( idx, ResponseItem::CustomToolCallOutput { - id: None, + id: synthetic_output_id("ctco", id.as_deref()), call_id: call_id.clone(), name: None, output: FunctionCallOutputPayload::from_text("aborted".to_string()), @@ -90,6 +94,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { } // LocalShellCall is represented in upstream streams by a FunctionCallOutput ResponseItem::LocalShellCall { + id, call_id: Some(call_id), .. } if !function_output_ids.contains(call_id.as_str()) => { @@ -99,7 +104,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { missing_outputs_to_insert.push(( idx, ResponseItem::FunctionCallOutput { - id: None, + id: synthetic_output_id("fco", id.as_deref()), call_id: call_id.clone(), output: FunctionCallOutputPayload::from_text("aborted".to_string()), internal_chat_message_metadata_passthrough: None, @@ -121,6 +126,21 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { } } +/// Derives a stable ID for a prompt-only output from its source call's item ID. +/// +/// Prompt normalization can run repeatedly without persisting its synthetic +/// outputs, so the namespace and name format must remain stable across retries +/// and resumes to preserve prompt-cache reuse. Returning `None` when the source +/// call has no ID preserves the legacy behavior for older history items. +fn synthetic_output_id(prefix: &str, item_id: Option<&str>) -> Option { + let source_id = item_id.filter(|id| !id.is_empty())?; + let name = format!("{prefix}:{source_id}"); + Some(format!( + "{prefix}_{}", + Uuid::new_v5(&SYNTHETIC_OUTPUT_ID_NAMESPACE, name.as_bytes()) + )) +} + pub(crate) fn remove_orphan_outputs(items: &mut Vec) { let function_call_ids: HashSet = items .iter() diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index d6336acd57..ece9d446b5 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -10482,6 +10482,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { status: None, call_id: "call-1".to_string(), name: "shell_command".to_string(), + namespace: None, input: "{}".to_string(), internal_chat_message_metadata_passthrough: None, }; diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 2e948b9c1e..f16b525002 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -2080,8 +2080,14 @@ async fn try_run_sampling_request( } } ResponseEvent::OutputItemAdded(item) => { - if let ResponseItem::CustomToolCall { call_id, name, .. } = &item { - let tool_name = ToolName::plain(name.as_str()); + if let ResponseItem::CustomToolCall { + call_id, + name, + namespace, + .. + } = &item + { + let tool_name = ToolName::new(namespace.clone(), name.as_str()); active_tool_argument_diff_consumer = tool_runtime .create_diff_consumer(&tool_name) .map(|consumer| (call_id.clone(), consumer)); diff --git a/codex-rs/core/src/stream_events_utils_tests.rs b/codex-rs/core/src/stream_events_utils_tests.rs index 78a219add6..b3312ad49d 100644 --- a/codex-rs/core/src/stream_events_utils_tests.rs +++ b/codex-rs/core/src/stream_events_utils_tests.rs @@ -116,6 +116,7 @@ fn external_context_pollution_items_exclude_local_tool_calls() { status: None, call_id: "custom-1".to_string(), name: "apply_patch".to_string(), + namespace: None, input: "*** Begin Patch\n*** End Patch\n".to_string(), internal_chat_message_metadata_passthrough: None, }, diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 4bdd121f6e..aca86b124b 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -25,7 +25,7 @@ use tracing::instrument; pub use crate::tools::context::ToolCallSource; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct ToolCall { pub tool_name: ToolName, pub call_id: String, @@ -147,11 +147,12 @@ impl ToolRouter { ResponseItem::ToolSearchCall { .. } => Ok(None), ResponseItem::CustomToolCall { name, + namespace, input, call_id, .. } => Ok(Some(ToolCall { - tool_name: ToolName::plain(name), + tool_name: ToolName::new(namespace, name), call_id, payload: ToolPayload::Custom { input }, })), diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 6f399fd5fc..62391395f8 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -179,6 +179,35 @@ async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<() Ok(()) } +#[tokio::test] +async fn build_custom_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<()> { + let tool_name = "exec".to_string(); + + let call = ToolRouter::build_tool_call(ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "call-namespace".to_string(), + name: tool_name.clone(), + namespace: Some("mcp__python".to_string()), + input: "print('hello')".to_string(), + internal_chat_message_metadata_passthrough: None, + })? + .expect("custom_tool_call should produce a tool call"); + + assert_eq!( + call, + ToolCall { + tool_name: ToolName::namespaced("mcp__python", tool_name), + call_id: "call-namespace".to_string(), + payload: ToolPayload::Custom { + input: "print('hello')".to_string(), + }, + } + ); + + Ok(()) +} + #[tokio::test] async fn mcp_parallel_support_uses_handler_data() -> anyhow::Result<()> { let (_, turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/turn_timing_tests.rs b/codex-rs/core/src/turn_timing_tests.rs index 956468da23..fcb34b4742 100644 --- a/codex-rs/core/src/turn_timing_tests.rs +++ b/codex-rs/core/src/turn_timing_tests.rs @@ -121,6 +121,7 @@ fn response_item_records_turn_ttft_for_first_output_signals() { status: None, call_id: "call-2".to_string(), name: "custom".to_string(), + namespace: None, input: "echo hi".to_string(), internal_chat_message_metadata_passthrough: None, } diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 6a605a80b3..9569df1c90 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -895,6 +895,24 @@ pub fn ev_custom_tool_call(call_id: &str, name: &str, input: &str) -> Value { }) } +pub fn ev_custom_tool_call_with_namespace( + call_id: &str, + namespace: &str, + name: &str, + input: &str, +) -> Value { + serde_json::json!({ + "type": "response.output_item.done", + "item": { + "type": "custom_tool_call", + "call_id": call_id, + "namespace": namespace, + "name": name, + "input": input + } + }) +} + pub fn ev_local_shell_call(call_id: &str, status: &str, command: Vec<&str>) -> Value { serde_json::json!({ "type": "response.output_item.done", diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 4df2bca1c1..e109e51e66 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -363,6 +363,107 @@ async fn response_item_ids_persist_across_resume_and_preserve_server_ids() -> an Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn synthetic_call_output_id_is_stable_across_resumes() -> anyhow::Result<()> { + let function_call_id = "missing-output-call"; + let thread_id = ThreadId::default(); + let rollout = vec![ + RolloutLine { + timestamp: "2024-01-01T00:00:00.000Z".to_string(), + item: RolloutItem::SessionMeta(SessionMetaLine { + meta: SessionMeta { + session_id: thread_id.into(), + id: thread_id, + parent_thread_id: None, + timestamp: "2024-01-01T00:00:00Z".to_string(), + cwd: ".".into(), + originator: "test_originator".to_string(), + cli_version: "test_version".to_string(), + model_provider: Some("test-provider".to_string()), + ..Default::default() + }, + git: None, + }), + }, + RolloutLine { + timestamp: "2024-01-01T00:00:01.000Z".to_string(), + item: RolloutItem::ResponseItem(ResponseItem::FunctionCall { + id: Some("fc_existing".to_string()), + name: "do_it".to_string(), + namespace: None, + arguments: "{}".to_string(), + call_id: function_call_id.to_string(), + internal_chat_message_metadata_passthrough: None, + }), + }, + ]; + let tmpdir = TempDir::new()?; + let session_path = tmpdir.path().join("normalized-call-output-item-id.jsonl"); + let mut file = std::fs::File::create(&session_path)?; + for line in rollout { + writeln!(file, "{}", serde_json::to_string(&line)?)?; + } + + let server = MockServer::start().await; + let response_mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]), + ], + ) + .await; + let codex_home = Arc::new(TempDir::new()?); + let mut builder = test_codex().with_config(|config| { + let _ = config.features.enable(Feature::ItemIds); + }); + let first = builder + .resume(&server, Arc::clone(&codex_home), session_path.clone()) + .await?; + + first.submit_turn("first resume").await?; + first.codex.submit(Op::Shutdown).await?; + wait_for_event(&first.codex, |event| { + matches!(event, EventMsg::ShutdownComplete) + }) + .await; + assert!( + !std::fs::read_to_string(&session_path)?.contains("\"type\":\"function_call_output\""), + "prompt-only repair should not be persisted to the rollout" + ); + + builder = builder.with_config(|config| { + let _ = config.features.enable(Feature::ItemIds); + }); + let second = builder.resume(&server, codex_home, session_path).await?; + second.submit_turn("second resume").await?; + + let requests = response_mock.requests(); + assert_eq!(requests.len(), 2); + let first_output = requests[0].function_call_output(function_call_id); + let first_output_id = first_output + .get("id") + .and_then(serde_json::Value::as_str) + .expect("reconstructed output should have an item ID") + .to_string(); + let first_output_uuid = first_output_id + .strip_prefix("fco_") + .expect("synthetic output should use the Responses API prefix"); + assert_eq!( + Uuid::parse_str(first_output_uuid)?.get_version(), + Some(uuid::Version::Sha1) + ); + assert_eq!( + requests[1] + .function_call_output(function_call_id) + .get("id") + .and_then(serde_json::Value::as_str), + Some(first_output_id.as_str()) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn response_item_ids_are_sent_for_all_remote_v2_compaction_requests() -> anyhow::Result<()> { let server = MockServer::start().await; @@ -785,6 +886,7 @@ async fn resume_replays_legacy_js_repl_image_rollout_shapes() { status: None, call_id: "legacy-js-call".to_string(), name: "js_repl".to_string(), + namespace: None, input: "console.log('legacy image flow')".to_string(), internal_chat_message_metadata_passthrough: None, }; @@ -976,6 +1078,7 @@ async fn resume_replays_image_tool_outputs_with_detail() { status: Some("completed".to_string()), call_id: custom_call_id.to_string(), name: "js_repl".to_string(), + namespace: None, input: "console.log('image flow')".to_string(), internal_chat_message_metadata_passthrough: None, }), @@ -2912,6 +3015,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { status: Some("completed".into()), call_id: "custom-tool-call-id".into(), name: "custom_tool".into(), + namespace: None, input: "{}".into(), internal_chat_message_metadata_passthrough: None, }); diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 83584b8ec0..5e47eae52e 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -20,6 +20,7 @@ use core_test_support::assert_regex_match; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_custom_tool_call; +use core_test_support::responses::ev_custom_tool_call_with_namespace; use core_test_support::responses::ev_function_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; @@ -177,6 +178,80 @@ async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn namespaced_custom_tool_call_preserves_namespace_through_dispatch_and_replay() -> Result<()> +{ + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let mut builder = test_codex(); + let test = builder.build(&server).await?; + + let call_id = "custom-namespaced"; + let namespace = "test_namespace::"; + let tool_name = "unsupported_tool"; + let input = "\"payload\""; + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_custom_tool_call_with_namespace(call_id, namespace, tool_name, input), + ev_completed("resp-1"), + ]), + ) + .await; + let mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + test.submit_turn_with_approval_and_permission_profile( + "invoke namespaced custom tool", + AskForApproval::Never, + PermissionProfile::Disabled, + ) + .await?; + + let request = mock.single_request(); + let custom_tool_calls = request.inputs_of_type("custom_tool_call"); + let turn_id = custom_tool_calls + .first() + .and_then(|item| item.pointer("/internal_chat_message_metadata_passthrough/turn_id")) + .and_then(Value::as_str) + .map(str::to_string) + .expect("custom tool call should include turn metadata"); + assert_eq!( + (custom_tool_calls, request.custom_tool_call_output(call_id),), + ( + vec![json!({ + "type": "custom_tool_call", + "call_id": call_id, + "namespace": namespace, + "name": tool_name, + "input": input, + "internal_chat_message_metadata_passthrough": { + "turn_id": turn_id, + }, + })], + json!({ + "type": "custom_tool_call_output", + "call_id": call_id, + "output": format!("unsupported custom tool call: {namespace}{tool_name}"), + "internal_chat_message_metadata_passthrough": { + "turn_id": turn_id, + }, + }), + ) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shell_command_escalated_permissions_rejected_then_ok() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index e5f813cc4e..82412a58e6 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -110,6 +110,7 @@ mod recovery; const CONNECT_TIMEOUT: Duration = Duration::from_secs(10); const INITIALIZE_TIMEOUT: Duration = Duration::from_secs(10); +const ENVIRONMENT_INFO_TIMEOUT: Duration = Duration::from_secs(30); const PROCESS_EVENT_CHANNEL_CAPACITY: usize = 256; const PROCESS_EVENT_RETAINED_BYTES: usize = 1024 * 1024; @@ -510,7 +511,12 @@ impl ExecServerClient { } pub async fn environment_info(&self) -> Result { - self.call(ENVIRONMENT_INFO_METHOD, &()).await + let rpc_client = self.inner.rpc_client().await?; + map_rpc_call_result( + rpc_client + .call_with_timeout(ENVIRONMENT_INFO_METHOD, &(), ENVIRONMENT_INFO_TIMEOUT) + .await, + ) } pub async fn read(&self, params: ReadParams) -> Result { @@ -780,22 +786,21 @@ impl ExecServerClient { P: serde::Serialize, T: serde::de::DeserializeOwned, { - match rpc_client.call(method, params).await { - Ok(response) => Ok(response), - Err(error) => { - let error = ExecServerError::from(error); - if is_transport_closed_error(&error) { - Err(ExecServerError::Disconnected(disconnected_message( - /*reason*/ None, - ))) - } else { - Err(error) - } - } - } + map_rpc_call_result(rpc_client.call(method, params).await) } } +fn map_rpc_call_result(result: Result) -> Result { + result.map_err(|error| { + let error = ExecServerError::from(error); + if is_transport_closed_error(&error) { + ExecServerError::Disconnected(disconnected_message(/*reason*/ None)) + } else { + error + } + }) +} + async fn cleanup_process_start( client: &ExecServerClient, process_id: &ProcessId, @@ -822,6 +827,9 @@ impl From for ExecServerError { code: error.code, message: error.message, }, + RpcCallError::TimedOut { method, timeout } => Self::Protocol(format!( + "timed out waiting for exec-server `{method}` response after {timeout:?}" + )), } } } diff --git a/codex-rs/exec-server/src/rpc.rs b/codex-rs/exec-server/src/rpc.rs index a941dce41f..46a0bd14e8 100644 --- a/codex-rs/exec-server/src/rpc.rs +++ b/codex-rs/exec-server/src/rpc.rs @@ -5,6 +5,7 @@ use std::sync::Arc; use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicI64; use std::sync::atomic::Ordering; +use std::time::Duration; use codex_exec_server_protocol::JSONRPCError; use codex_exec_server_protocol::JSONRPCErrorError; @@ -21,6 +22,7 @@ use tokio::sync::mpsc; use tokio::sync::oneshot; use tokio::sync::watch; use tokio::task::JoinHandle; +use tokio::time::timeout; use crate::connection::JsonRpcConnection; use crate::connection::JsonRpcConnectionEvent; @@ -36,6 +38,8 @@ pub(crate) enum RpcCallError { Json(serde_json::Error), /// The executor returned a JSON-RPC error response for this call. Server(JSONRPCErrorError), + /// The executor did not return a response before the caller's deadline. + TimedOut { method: String, timeout: Duration }, } type PendingRequest = oneshot::Sender>; @@ -46,6 +50,11 @@ type RequestRoute = Box< type NotificationRoute = Box, JSONRPCNotification) -> BoxFuture> + Send + Sync>; +enum RpcCallTimeout { + None, + After(Duration), +} + #[derive(Debug)] pub(crate) enum RpcClientEvent { Notification(JSONRPCNotification), @@ -334,6 +343,33 @@ impl RpcClient { } pub(crate) async fn call(&self, method: &str, params: &P) -> Result + where + P: Serialize, + T: DeserializeOwned, + { + self.call_inner(method, params, RpcCallTimeout::None).await + } + + pub(crate) async fn call_with_timeout( + &self, + method: &str, + params: &P, + call_timeout: Duration, + ) -> Result + where + P: Serialize, + T: DeserializeOwned, + { + self.call_inner(method, params, RpcCallTimeout::After(call_timeout)) + .await + } + + async fn call_inner( + &self, + method: &str, + params: &P, + call_timeout: RpcCallTimeout, + ) -> Result where P: Serialize, T: DeserializeOwned, @@ -379,8 +415,20 @@ impl RpcClient { // still-pending requests. Awaiting this receiver preserves that order: // responses already read before EOF still win, and truly pending calls // are failed once the reader observes the disconnect. - let result: Result = - response_rx.await.map_err(|_| RpcCallError::Closed)?; + let response = match call_timeout { + RpcCallTimeout::None => response_rx.await, + RpcCallTimeout::After(call_timeout) => match timeout(call_timeout, response_rx).await { + Ok(response) => response, + Err(_) => { + self.pending.lock().await.remove(&request_id); + return Err(RpcCallError::TimedOut { + method: method.to_string(), + timeout: call_timeout, + }); + } + }, + }; + let result: Result = response.map_err(|_| RpcCallError::Closed)?; let response = match result { Ok(response) => response, Err(error) => return Err(error), @@ -673,6 +721,40 @@ mod tests { } } + #[tokio::test] + async fn rpc_client_timeout_removes_pending_request() { + let (client_stdin, server_reader) = tokio::io::duplex(4096); + let (server_writer, client_stdout) = tokio::io::duplex(4096); + let (release_server_tx, release_server_rx) = tokio::sync::oneshot::channel(); + let connection = + JsonRpcConnection::from_stdio(client_stdout, client_stdin, "test-rpc".to_string()); + let (client, _events_rx) = RpcClient::new(connection); + + let server = tokio::spawn(async move { + let mut lines = BufReader::new(server_reader).lines(); + let request = read_jsonrpc_line(&mut lines).await; + assert!(matches!(request, JSONRPCMessage::Request(_))); + let _server_writer = server_writer; + let _ = release_server_rx.await; + }); + + let call_timeout = Duration::from_millis(10); + let result = client + .call_with_timeout::<_, serde_json::Value>("slow", &serde_json::json!({}), call_timeout) + .await; + assert!(matches!( + result, + Err(super::RpcCallError::TimedOut { method, timeout }) + if method == "slow" && timeout == call_timeout + )); + assert_eq!(client.pending_request_count().await, 0); + + let _ = release_server_tx.send(()); + if let Err(err) = server.await { + panic!("server task failed: {err}"); + } + } + #[tokio::test(flavor = "current_thread")] async fn rpc_client_propagates_current_trace_context() { let span_exporter = InMemorySpanExporter::default(); diff --git a/codex-rs/ext/image-generation/src/tests.rs b/codex-rs/ext/image-generation/src/tests.rs index fe915746f9..1ec9371214 100644 --- a/codex-rs/ext/image-generation/src/tests.rs +++ b/codex-rs/ext/image-generation/src/tests.rs @@ -97,6 +97,7 @@ async fn recent_image_fallback_selects_newest_images_in_chronological_order() { status: Some("completed".to_string()), call_id: "code-mode-call".to_string(), name: "exec".to_string(), + namespace: None, input: String::new(), internal_chat_message_metadata_passthrough: None, }, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 2faad87349..46aecb61b9 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1052,6 +1052,9 @@ pub enum ResponseItem { call_id: String, name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + namespace: Option, input: String, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] diff --git a/codex-rs/tools/src/tool_payload.rs b/codex-rs/tools/src/tool_payload.rs index 71af457743..101d95a278 100644 --- a/codex-rs/tools/src/tool_payload.rs +++ b/codex-rs/tools/src/tool_payload.rs @@ -3,7 +3,7 @@ use std::borrow::Cow; use codex_protocol::models::SearchToolCallParams; /// Canonical payload shapes accepted by model-visible tool runtimes. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub enum ToolPayload { Function { arguments: String }, ToolSearch { arguments: SearchToolCallParams }, diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 2e6d7f8f7f..3c7d9bbf50 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -1151,8 +1151,8 @@ async fn live_app_server_cyber_policy_error_renders_dedicated_notice() { let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 1); let rendered = lines_to_single_string(&cells[0]); - assert!(rendered.contains("This chat was flagged for possible cybersecurity risk")); - assert!(rendered.contains("Trusted Access for Cyber")); + assert!(rendered.contains("This content can't be shown")); + assert!(rendered.contains("extra caution with cybersecurity requests")); assert!(!rendered.contains("server fallback message")); assert!(!chat.bottom_pane.is_task_running()); } @@ -1171,6 +1171,7 @@ async fn app_server_safety_access_errors_render_dedicated_notice() { assert_eq!(cells.len(), 1); let rendered = lines_to_single_string(&cells[0]); assert!(rendered.contains("This content can't be shown")); + assert!(rendered.contains("biological research")); assert!(!rendered.contains("Invalid prompt:")); } } diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index 329d249340..156348d42c 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -54,8 +54,8 @@ async fn app_server_cyber_policy_error_renders_dedicated_notice() { let cells = drain_insert_history(&mut rx); assert_eq!(cells.len(), 1); let rendered = lines_to_single_string(&cells[0]); - assert!(rendered.contains("This chat was flagged for possible cybersecurity risk")); - assert!(rendered.contains("Trusted Access for Cyber")); + assert!(rendered.contains("This content can't be shown")); + assert!(rendered.contains("extra caution with cybersecurity requests")); assert!(!rendered.contains("server fallback message")); } diff --git a/codex-rs/tui/src/history_cell/notices.rs b/codex-rs/tui/src/history_cell/notices.rs index c4cc79cf70..c2fa6c921b 100644 --- a/codex-rs/tui/src/history_cell/notices.rs +++ b/codex-rs/tui/src/history_cell/notices.rs @@ -86,91 +86,76 @@ pub(crate) fn new_warning_event(message: String) -> PrefixedWrappedHistoryCell { } #[derive(Debug)] -pub(crate) struct SafetyAccessBlockCell; +pub(crate) struct SafetyAccessBlockCell { + body: &'static str, + trusted_access_url: &'static str, +} const SAFETY_ACCESS_BLOCK_TITLE: &str = "This content can't be shown"; -const SAFETY_ACCESS_BLOCK_BODY: &str = "We've limited access to this content for safety reasons. This type of information may be used to benefit or to harm people."; +const SAFETY_ACCESS_BLOCK_LEARN_MORE_URL: &str = "https://help.openai.com/en/articles/20001326"; pub(crate) fn new_safety_access_block_event() -> SafetyAccessBlockCell { - SafetyAccessBlockCell + SafetyAccessBlockCell { + body: "We take extra caution with requests involving biological research and applications that could pose safety risks. If you’re a researcher at an approved organization, you may be able to apply for Trusted Access.", + trusted_access_url: "https://openai.com/form/trusted-access-for-life-sciences", + } +} + +pub(crate) fn new_cyber_policy_error_event() -> SafetyAccessBlockCell { + SafetyAccessBlockCell { + body: "We take extra caution with cybersecurity requests. If you’re a security professional, you may be able to apply for Trusted Access.", + trusted_access_url: "https://openai.com/form/enterprise-trusted-access-for-cyber/", + } } impl HistoryCell for SafetyAccessBlockCell { fn display_lines(&self, width: u16) -> Vec> { - let mut lines = vec![vec!["ⓘ ".cyan(), SAFETY_ACCESS_BLOCK_TITLE.bold()].into()]; - let body = Line::from(vec![" ".into(), SAFETY_ACCESS_BLOCK_BODY.dim()]); - let wrap_width = width.saturating_sub(2).max(1) as usize; - let wrapped = adaptive_wrap_line( - &body, - RtOptions::new(wrap_width).subsequent_indent(" ".into()), - ); - push_owned_lines(&wrapped, &mut lines); - lines - } - - fn raw_lines(&self) -> Vec> { - vec![ - Line::from(SAFETY_ACCESS_BLOCK_TITLE), - Line::from(SAFETY_ACCESS_BLOCK_BODY), - ] - } -} - -const TRUSTED_ACCESS_FOR_CYBER_URL: &str = "https://chatgpt.com/cyber"; - -#[derive(Debug)] -pub(crate) struct CyberPolicyNoticeCell; - -pub(crate) fn new_cyber_policy_error_event() -> CyberPolicyNoticeCell { - CyberPolicyNoticeCell -} - -impl HistoryCell for CyberPolicyNoticeCell { - fn display_lines(&self, width: u16) -> Vec> { - let mut lines: Vec> = Vec::new(); - lines.push( - vec![ - "ⓘ ".cyan(), - "This chat was flagged for possible cybersecurity risk".bold(), - ] - .into(), - ); - - let wrap_width = width.saturating_sub(2).max(1) as usize; - let body = Line::from(vec![ - " If this seems wrong, try rephrasing your request. To get authorized for security work, join the " - .dim(), - "Trusted Access for Cyber".cyan().underlined(), - " program.".dim(), - ]); - let wrapped = adaptive_wrap_line( - &body, - RtOptions::new(wrap_width).subsequent_indent(" ".into()), - ); - push_owned_lines(&wrapped, &mut lines); - lines.push( - vec![ - " ".into(), - TRUSTED_ACCESS_FOR_CYBER_URL.cyan().underlined(), - ] - .into(), - ); - - lines - } - - fn raw_lines(&self) -> Vec> { - vec![ - Line::from("This chat was flagged for possible cybersecurity risk"), - Line::from( - "If this seems wrong, try rephrasing your request. To get authorized for security work, join the Trusted Access for Cyber program.", - ), - Line::from(TRUSTED_ACCESS_FOR_CYBER_URL), - ] + visible_lines(self.display_hyperlink_lines(width)) } fn display_hyperlink_lines(&self, width: u16) -> Vec { - crate::terminal_hyperlinks::annotate_web_urls(self.display_lines(width)) + let mut lines = vec![HyperlinkLine::new( + vec!["ⓘ ".cyan(), SAFETY_ACCESS_BLOCK_TITLE.bold()].into(), + )]; + let body = Line::from(vec![" ".into(), self.body.dim()]); + let wrap_width = width.saturating_sub(2).max(1) as usize; + let wrapped = adaptive_wrap_line( + &body, + RtOptions::new(wrap_width).subsequent_indent(" ".into()), + ); + let mut wrapped_body = Vec::new(); + push_owned_lines(&wrapped, &mut wrapped_body); + lines.extend(plain_hyperlink_lines(wrapped_body)); + + for (label, url) in [ + ("Trusted Access", self.trusted_access_url), + ("Learn more", SAFETY_ACCESS_BLOCK_LEARN_MORE_URL), + ] { + let source = crate::terminal_hyperlinks::annotate_web_urls_in_line( + vec![format!(" {label}: ").dim(), url.cyan().underlined()].into(), + ); + let wrapped = crate::wrapping::word_wrap_line( + &source.line, + RtOptions::new(wrap_width).subsequent_indent(" ".into()), + ); + let mut wrapped_links = Vec::new(); + push_owned_lines(&wrapped, &mut wrapped_links); + lines.extend(crate::terminal_hyperlinks::remap_wrapped_line( + &source, + wrapped_links, + )); + } + lines + } + + fn raw_lines(&self) -> Vec> { + let trusted_access_url = self.trusted_access_url; + vec![ + Line::from(SAFETY_ACCESS_BLOCK_TITLE), + Line::from(self.body), + Line::from(format!("Trusted Access: {trusted_access_url}")), + Line::from(format!("Learn more: {SAFETY_ACCESS_BLOCK_LEARN_MORE_URL}")), + ] } fn transcript_hyperlink_lines(&self, width: u16) -> Vec { diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_narrow_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_narrow_snapshot.snap index 9039b196ee..17d6bf7ce6 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_narrow_snapshot.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_narrow_snapshot.snap @@ -1,12 +1,16 @@ --- -source: tui/src/history_cell.rs -assertion_line: 3312 +source: tui/src/history_cell/tests.rs expression: rendered --- -ⓘ This chat was flagged for possible cybersecurity risk - If this seems wrong, try - rephrasing your request. To get - authorized for security work, - join the Trusted Access for - Cyber program. - https://chatgpt.com/cyber +ⓘ This content can't be shown + We take extra caution with + cybersecurity requests. If + you’re a security professional, + you may be able to apply for + Trusted Access. + Trusted Access: https:// + openai.com/form/enterprise- + trusted-access-for-cyber/ + Learn more: https:// + help.openai.com/en/ + articles/20001326 diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_snapshot.snap index 888a10c37e..ca800e50de 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_snapshot.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__cyber_policy_error_event_snapshot.snap @@ -3,7 +3,8 @@ source: tui/src/history_cell.rs assertion_line: 3305 expression: rendered --- -ⓘ This chat was flagged for possible cybersecurity risk - If this seems wrong, try rephrasing your request. To get authorized for - security work, join the Trusted Access for Cyber program. - https://chatgpt.com/cyber +ⓘ This content can't be shown + We take extra caution with cybersecurity requests. If you’re a security + professional, you may be able to apply for Trusted Access. + Trusted Access: https://openai.com/form/enterprise-trusted-access-for-cyber/ + Learn more: https://help.openai.com/en/articles/20001326 diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__safety_access_block_event_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__safety_access_block_event_snapshot.snap index f512697888..fe027f0d9d 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__safety_access_block_event_snapshot.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__safety_access_block_event_snapshot.snap @@ -3,5 +3,8 @@ source: tui/src/history_cell/tests.rs expression: rendered --- ⓘ This content can't be shown - We've limited access to this content for safety reasons. This type of - information may be used to benefit or to harm people. + We take extra caution with requests involving biological research and + applications that could pose safety risks. If you’re a researcher at an + approved organization, you may be able to apply for Trusted Access. + Trusted Access: https://openai.com/form/trusted-access-for-life-sciences + Learn more: https://help.openai.com/en/articles/20001326