From 4b54940cae505c6bfd57bd018bbb640a8dcabbf6 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 15:17:47 -0800 Subject: [PATCH 1/8] Add text element metadata to protocol, app server, and core --- .../src/protocol/thread_history.rs | 11 +- .../app-server-protocol/src/protocol/v1.rs | 73 ++- .../app-server-protocol/src/protocol/v2.rs | 182 ++------ codex-rs/app-server/README.md | 1 - .../app-server/src/bespoke_event_handling.rs | 175 -------- .../app-server/src/codex_message_processor.rs | 27 +- .../suite/codex_message_processor_flow.rs | 5 + .../app-server/tests/suite/create_thread.rs | 1 + codex-rs/app-server/tests/suite/interrupt.rs | 1 + .../app-server/tests/suite/output_schema.rs | 3 + .../app-server/tests/suite/send_message.rs | 5 +- codex-rs/core/src/agent/control.rs | 8 +- codex-rs/core/src/agent/mod.rs | 2 - codex-rs/core/src/client.rs | 20 +- codex-rs/core/src/codex.rs | 56 ++- codex-rs/core/src/compact.rs | 4 +- codex-rs/core/src/config/edit.rs | 6 - codex-rs/core/src/config/mod.rs | 124 ++--- codex-rs/core/src/config/types.rs | 24 - .../src/config_loader/config_requirements.rs | 39 +- codex-rs/core/src/config_loader/mod.rs | 1 - codex-rs/core/src/context_manager/history.rs | 2 +- codex-rs/core/src/event_mapping.rs | 2 +- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/mcp_connection_manager.rs | 2 - codex-rs/core/src/rollout/recorder.rs | 63 --- codex-rs/core/src/skills/loader.rs | 424 ++---------------- codex-rs/core/src/skills/model.rs | 11 - codex-rs/core/src/tasks/review.rs | 2 +- codex-rs/core/src/tools/handlers/collab.rs | 57 +-- codex-rs/core/src/tools/spec.rs | 79 ++-- codex-rs/core/src/user_instructions.rs | 8 +- codex-rs/core/src/user_shell_command.rs | 2 +- codex-rs/core/tests/chat_completions_sse.rs | 2 +- codex-rs/core/tests/responses_headers.rs | 63 --- codex-rs/core/tests/suite/client.rs | 5 +- codex-rs/core/tests/suite/model_tools.rs | 2 +- codex-rs/core/tests/suite/prompt_caching.rs | 2 +- codex-rs/core/tests/suite/review.rs | 4 +- codex-rs/core/tests/suite/rmcp_client.rs | 6 - codex-rs/core/tests/suite/truncation.rs | 3 - .../core/tests/suite/web_search_cached.rs | 4 +- codex-rs/protocol/src/items.rs | 46 +- codex-rs/protocol/src/models.rs | 6 +- codex-rs/protocol/src/protocol.rs | 22 +- 45 files changed, 374 insertions(+), 1212 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index 6772d6f92a59..e6c679b41861 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -197,8 +197,12 @@ impl ThreadHistoryBuilder { if !payload.message.trim().is_empty() { content.push(UserInput::Text { text: payload.message.clone(), - // TODO: Thread text element ranges into thread history. Empty keeps old behavior. - text_elements: Vec::new(), + text_elements: payload + .text_elements + .iter() + .cloned() + .map(Into::into) + .collect(), }); } if let Some(images) = &payload.images { @@ -206,6 +210,9 @@ impl ThreadHistoryBuilder { content.push(UserInput::Image { url: image.clone() }); } } + for path in &payload.local_images { + content.push(UserInput::LocalImage { path: path.clone() }); + } content } } diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index ecc9d7c07de5..a5bc69be5dfd 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -16,6 +16,8 @@ use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::TurnAbortReason; +use codex_protocol::user_input::ByteRange as CoreByteRange; +use codex_protocol::user_input::TextElement as CoreTextElement; use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; use serde::Deserialize; @@ -444,9 +446,74 @@ pub struct RemoveConversationListenerParams { #[serde(rename_all = "camelCase")] #[serde(tag = "type", content = "data")] pub enum InputItem { - Text { text: String }, - Image { image_url: String }, - LocalImage { path: PathBuf }, + Text { + text: String, + /// UI-defined spans within `text` used to render or persist special elements. + #[serde(default)] + text_elements: Vec, + }, + Image { + image_url: String, + }, + LocalImage { + path: PathBuf, + }, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(rename = "ByteRange")] +pub struct V1ByteRange { + /// Start byte offset (inclusive) within the UTF-8 text buffer. + pub start: usize, + /// End byte offset (exclusive) within the UTF-8 text buffer. + pub end: usize, +} + +impl From for V1ByteRange { + fn from(value: CoreByteRange) -> Self { + Self { + start: value.start, + end: value.end, + } + } +} + +impl From for CoreByteRange { + fn from(value: V1ByteRange) -> Self { + Self { + start: value.start, + end: value.end, + } + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(rename = "TextElement")] +pub struct V1TextElement { + /// Byte range in the parent `text` buffer that this element occupies. + pub byte_range: V1ByteRange, + /// Optional human-readable placeholder for the element, displayed in the UI. + pub placeholder: Option, +} + +impl From for V1TextElement { + fn from(value: CoreTextElement) -> Self { + Self { + byte_range: value.byte_range.into(), + placeholder: value.placeholder, + } + } +} + +impl From for CoreTextElement { + fn from(value: V1TextElement) -> Self { + Self { + byte_range: value.byte_range.into(), + placeholder: value.placeholder, + } + } } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 4ff129154492..e3499951df13 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -16,7 +16,6 @@ use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; -use codex_protocol::protocol::AgentStatus as CoreAgentStatus; use codex_protocol::protocol::AskForApproval as CoreAskForApproval; use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo; use codex_protocol::protocol::CreditsSnapshot as CoreCreditsSnapshot; @@ -25,11 +24,12 @@ use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; -use codex_protocol::protocol::SkillInterface as CoreSkillInterface; use codex_protocol::protocol::SkillMetadata as CoreSkillMetadata; use codex_protocol::protocol::SkillScope as CoreSkillScope; use codex_protocol::protocol::TokenUsage as CoreTokenUsage; use codex_protocol::protocol::TokenUsageInfo as CoreTokenUsageInfo; +use codex_protocol::user_input::ByteRange as CoreByteRange; +use codex_protocol::user_input::TextElement as CoreTextElement; use codex_protocol::user_input::UserInput as CoreUserInput; use codex_utils_absolute_path::AbsolutePathBuf; use mcp_types::ContentBlock as McpContentBlock; @@ -1253,35 +1253,13 @@ pub enum SkillScope { pub struct SkillMetadata { pub name: String, pub description: String, - #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - /// Legacy short_description from SKILL.md. Prefer SKILL.toml interface.short_description. - pub short_description: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub interface: Option, + pub short_description: Option, pub path: PathBuf, pub scope: SkillScope, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub struct SkillInterface { - #[ts(optional)] - pub display_name: Option, - #[ts(optional)] - pub short_description: Option, - #[ts(optional)] - pub icon_small: Option, - #[ts(optional)] - pub icon_large: Option, - #[ts(optional)] - pub brand_color: Option, - #[ts(optional)] - pub default_prompt: Option, -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1305,26 +1283,12 @@ impl From for SkillMetadata { name: value.name, description: value.description, short_description: value.short_description, - interface: value.interface.map(SkillInterface::from), path: value.path, scope: value.scope.into(), } } } -impl From for SkillInterface { - fn from(value: CoreSkillInterface) -> Self { - Self { - display_name: value.display_name, - short_description: value.short_description, - brand_color: value.brand_color, - default_prompt: value.default_prompt, - icon_small: value.icon_small, - icon_large: value.icon_large, - } - } -} - impl From for SkillScope { fn from(value: CoreSkillScope) -> Self { match value { @@ -1589,6 +1553,23 @@ pub struct ByteRange { pub end: usize, } +impl From for ByteRange { + fn from(value: CoreByteRange) -> Self { + Self { + start: value.start, + end: value.end, + } + } +} + +impl From for CoreByteRange { + fn from(value: ByteRange) -> Self { + Self { + start: value.start, + end: value.end, + } + } +} #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1599,6 +1580,24 @@ pub struct TextElement { pub placeholder: Option, } +impl From for TextElement { + fn from(value: CoreTextElement) -> Self { + Self { + byte_range: value.byte_range.into(), + placeholder: value.placeholder, + } + } +} + +impl From for CoreTextElement { + fn from(value: TextElement) -> Self { + Self { + byte_range: value.byte_range.into(), + placeholder: value.placeholder, + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type")] @@ -1625,10 +1624,12 @@ pub enum UserInput { impl UserInput { pub fn into_core(self) -> CoreUserInput { match self { - UserInput::Text { text, .. } => CoreUserInput::Text { + UserInput::Text { + text, + text_elements, + } => CoreUserInput::Text { text, - // TODO: Thread text element ranges into v2 inputs. Empty keeps old behavior. - text_elements: Vec::new(), + text_elements: text_elements.into_iter().map(Into::into).collect(), }, UserInput::Image { url } => CoreUserInput::Image { image_url: url }, UserInput::LocalImage { path } => CoreUserInput::LocalImage { path }, @@ -1640,10 +1641,12 @@ impl UserInput { impl From for UserInput { fn from(value: CoreUserInput) -> Self { match value { - CoreUserInput::Text { text, .. } => UserInput::Text { + CoreUserInput::Text { text, - // TODO: Thread text element ranges from core into v2 inputs. - text_elements: Vec::new(), + text_elements, + } => UserInput::Text { + text, + text_elements: text_elements.into_iter().map(Into::into).collect(), }, CoreUserInput::Image { image_url } => UserInput::Image { url: image_url }, CoreUserInput::LocalImage { path } => UserInput::LocalImage { path }, @@ -1719,25 +1722,6 @@ pub enum ThreadItem { }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] - CollabAgentToolCall { - /// Unique identifier for this collab tool call. - id: String, - /// Name of the collab tool that was invoked. - tool: CollabAgentTool, - /// Current status of the collab tool call. - status: CollabAgentToolCallStatus, - /// Thread ID of the agent issuing the collab request. - sender_thread_id: String, - /// Thread ID of the receiving agent, when applicable. In case of spawn operation, - /// this correspond to the newly spawned agent. - receiver_thread_id: Option, - /// Prompt text sent as part of the collab tool call, when available. - prompt: Option, - /// Last known status of the target agent, when available. - agent_state: Option, - }, - #[serde(rename_all = "camelCase")] - #[ts(rename_all = "camelCase")] WebSearch { id: String, query: String }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] @@ -1790,16 +1774,6 @@ pub enum CommandExecutionStatus { Declined, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub enum CollabAgentTool { - SpawnAgent, - SendInput, - Wait, - CloseAgent, -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1838,66 +1812,6 @@ pub enum McpToolCallStatus { Failed, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub enum CollabAgentToolCallStatus { - InProgress, - Completed, - Failed, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub enum CollabAgentStatus { - PendingInit, - Running, - Completed, - Errored, - Shutdown, - NotFound, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] -#[serde(rename_all = "camelCase")] -#[ts(export_to = "v2/")] -pub struct CollabAgentState { - pub status: CollabAgentStatus, - pub message: Option, -} - -impl From for CollabAgentState { - fn from(value: CoreAgentStatus) -> Self { - match value { - CoreAgentStatus::PendingInit => Self { - status: CollabAgentStatus::PendingInit, - message: None, - }, - CoreAgentStatus::Running => Self { - status: CollabAgentStatus::Running, - message: None, - }, - CoreAgentStatus::Completed(message) => Self { - status: CollabAgentStatus::Completed, - message, - }, - CoreAgentStatus::Errored(message) => Self { - status: CollabAgentStatus::Errored, - message: Some(message), - }, - CoreAgentStatus::Shutdown => Self { - status: CollabAgentStatus::Shutdown, - message: None, - }, - CoreAgentStatus::NotFound => Self { - status: CollabAgentStatus::NotFound, - message: None, - }, - } - } -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 63f4dea965d2..26c2dee9cf86 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -375,7 +375,6 @@ Today both notifications carry an empty `items` array even when item events were - `commandExecution` — `{id, command, cwd, status, commandActions, aggregatedOutput?, exitCode?, durationMs?}` for sandboxed commands; `status` is `inProgress`, `completed`, `failed`, or `declined`. - `fileChange` — `{id, changes, status}` describing proposed edits; `changes` list `{path, kind, diff}` and `status` is `inProgress`, `completed`, `failed`, or `declined`. - `mcpToolCall` — `{id, server, tool, status, arguments, result?, error?}` describing MCP calls; `status` is `inProgress`, `completed`, or `failed`. -- `collabToolCall` — `{id, tool, status, senderThreadId, receiverThreadId?, newThreadId?, prompt?, agentStatus?}` describing collab tool calls (`spawn_agent`, `send_input`, `wait`, `close_agent`); `status` is `inProgress`, `completed`, or `failed`. - `webSearch` — `{id, query}` for a web search request issued by the agent. - `imageView` — `{id, path}` emitted when the agent invokes the image viewer tool. - `enteredReviewMode` — `{id, review}` sent when the reviewer starts; `review` is a short user-facing label such as `"current changes"` or the requested target description. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index f14dd2ddca48..0870191ec8f1 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -14,9 +14,6 @@ use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; use codex_app_server_protocol::CodexErrorInfo as V2CodexErrorInfo; -use codex_app_server_protocol::CollabAgentState as V2CollabAgentStatus; -use codex_app_server_protocol::CollabAgentTool; -use codex_app_server_protocol::CollabAgentToolCallStatus as V2CollabToolCallStatus; use codex_app_server_protocol::CommandAction as V2ParsedCommand; use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionOutputDeltaNotification; @@ -281,178 +278,6 @@ pub(crate) async fn apply_bespoke_event_handling( .send_server_notification(ServerNotification::ItemCompleted(notification)) .await; } - EventMsg::CollabAgentSpawnBegin(begin_event) => { - let item = ThreadItem::CollabAgentToolCall { - id: begin_event.call_id, - tool: CollabAgentTool::SpawnAgent, - status: V2CollabToolCallStatus::InProgress, - sender_thread_id: begin_event.sender_thread_id.to_string(), - receiver_thread_id: None, - prompt: Some(begin_event.prompt), - agent_state: None, - }; - let notification = ItemStartedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemStarted(notification)) - .await; - } - EventMsg::CollabAgentSpawnEnd(end_event) => { - let status = if end_event.new_thread_id.is_some() { - V2CollabToolCallStatus::Completed - } else { - V2CollabToolCallStatus::Failed - }; - let item = ThreadItem::CollabAgentToolCall { - id: end_event.call_id, - tool: CollabAgentTool::SpawnAgent, - status, - sender_thread_id: end_event.sender_thread_id.to_string(), - receiver_thread_id: end_event.new_thread_id.map(|id| id.to_string()), - prompt: Some(end_event.prompt), - agent_state: Some(V2CollabAgentStatus::from(end_event.status)), - }; - let notification = ItemCompletedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemCompleted(notification)) - .await; - } - EventMsg::CollabAgentInteractionBegin(begin_event) => { - let item = ThreadItem::CollabAgentToolCall { - id: begin_event.call_id, - tool: CollabAgentTool::SendInput, - status: V2CollabToolCallStatus::InProgress, - sender_thread_id: begin_event.sender_thread_id.to_string(), - receiver_thread_id: Some(begin_event.receiver_thread_id.to_string()), - prompt: Some(begin_event.prompt), - agent_state: None, - }; - let notification = ItemStartedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemStarted(notification)) - .await; - } - EventMsg::CollabAgentInteractionEnd(end_event) => { - let status = match end_event.status { - codex_protocol::protocol::AgentStatus::Errored(_) - | codex_protocol::protocol::AgentStatus::NotFound => V2CollabToolCallStatus::Failed, - _ => V2CollabToolCallStatus::Completed, - }; - let item = ThreadItem::CollabAgentToolCall { - id: end_event.call_id, - tool: CollabAgentTool::SendInput, - status, - sender_thread_id: end_event.sender_thread_id.to_string(), - receiver_thread_id: Some(end_event.receiver_thread_id.to_string()), - prompt: Some(end_event.prompt), - agent_state: Some(V2CollabAgentStatus::from(end_event.status)), - }; - let notification = ItemCompletedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemCompleted(notification)) - .await; - } - EventMsg::CollabWaitingBegin(begin_event) => { - let item = ThreadItem::CollabAgentToolCall { - id: begin_event.call_id, - tool: CollabAgentTool::Wait, - status: V2CollabToolCallStatus::InProgress, - sender_thread_id: begin_event.sender_thread_id.to_string(), - receiver_thread_id: Some(begin_event.receiver_thread_id.to_string()), - prompt: None, - agent_state: None, - }; - let notification = ItemStartedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemStarted(notification)) - .await; - } - EventMsg::CollabWaitingEnd(end_event) => { - let status = match end_event.status { - codex_protocol::protocol::AgentStatus::Errored(_) - | codex_protocol::protocol::AgentStatus::NotFound => V2CollabToolCallStatus::Failed, - _ => V2CollabToolCallStatus::Completed, - }; - let item = ThreadItem::CollabAgentToolCall { - id: end_event.call_id, - tool: CollabAgentTool::Wait, - status, - sender_thread_id: end_event.sender_thread_id.to_string(), - receiver_thread_id: Some(end_event.receiver_thread_id.to_string()), - prompt: None, - agent_state: Some(V2CollabAgentStatus::from(end_event.status)), - }; - let notification = ItemCompletedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemCompleted(notification)) - .await; - } - EventMsg::CollabCloseBegin(begin_event) => { - let item = ThreadItem::CollabAgentToolCall { - id: begin_event.call_id, - tool: CollabAgentTool::CloseAgent, - status: V2CollabToolCallStatus::InProgress, - sender_thread_id: begin_event.sender_thread_id.to_string(), - receiver_thread_id: Some(begin_event.receiver_thread_id.to_string()), - prompt: None, - agent_state: None, - }; - let notification = ItemStartedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemStarted(notification)) - .await; - } - EventMsg::CollabCloseEnd(end_event) => { - let status = match end_event.status { - codex_protocol::protocol::AgentStatus::Errored(_) - | codex_protocol::protocol::AgentStatus::NotFound => V2CollabToolCallStatus::Failed, - _ => V2CollabToolCallStatus::Completed, - }; - let item = ThreadItem::CollabAgentToolCall { - id: end_event.call_id, - tool: CollabAgentTool::CloseAgent, - status, - sender_thread_id: end_event.sender_thread_id.to_string(), - receiver_thread_id: Some(end_event.receiver_thread_id.to_string()), - prompt: None, - agent_state: Some(V2CollabAgentStatus::from(end_event.status)), - }; - let notification = ItemCompletedNotification { - thread_id: conversation_id.to_string(), - turn_id: event_turn_id.clone(), - item, - }; - outgoing - .send_server_notification(ServerNotification::ItemCompleted(notification)) - .await; - } EventMsg::AgentMessageContentDelta(event) => { let notification = AgentMessageDeltaNotification { thread_id: conversation_id.to_string(), diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 770e8de4e353..c5d2994f4d67 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -3125,10 +3125,12 @@ impl CodexMessageProcessor { let mapped_items: Vec = items .into_iter() .map(|item| match item { - WireInputItem::Text { text } => CoreInputItem::Text { + WireInputItem::Text { text, - // TODO: Thread text element ranges into v1 input handling. - text_elements: Vec::new(), + text_elements, + } => CoreInputItem::Text { + text, + text_elements: text_elements.into_iter().map(Into::into).collect(), }, WireInputItem::Image { image_url } => CoreInputItem::Image { image_url }, WireInputItem::LocalImage { path } => CoreInputItem::LocalImage { path }, @@ -3175,10 +3177,12 @@ impl CodexMessageProcessor { let mapped_items: Vec = items .into_iter() .map(|item| match item { - WireInputItem::Text { text } => CoreInputItem::Text { + WireInputItem::Text { text, - // TODO: Thread text element ranges into v1 input handling. - text_elements: Vec::new(), + text_elements, + } => CoreInputItem::Text { + text, + text_elements: text_elements.into_iter().map(Into::into).collect(), }, WireInputItem::Image { image_url } => CoreInputItem::Image { image_url }, WireInputItem::LocalImage { path } => CoreInputItem::LocalImage { path }, @@ -3341,6 +3345,7 @@ impl CodexMessageProcessor { id: turn_id.clone(), content: vec![V2UserInput::Text { text: display_text.to_string(), + // Review prompt display text is synthesized; no UI element ranges to preserve. text_elements: Vec::new(), }], }] @@ -3894,16 +3899,6 @@ fn skills_to_info( name: skill.name.clone(), description: skill.description.clone(), short_description: skill.short_description.clone(), - interface: skill.interface.clone().map(|interface| { - codex_app_server_protocol::SkillInterface { - display_name: interface.display_name, - short_description: interface.short_description, - icon_small: interface.icon_small, - icon_large: interface.icon_large, - brand_color: interface.brand_color, - default_prompt: interface.default_prompt, - } - }), path: skill.path.clone(), scope: skill.scope.into(), }) diff --git a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs index 456206af8968..b97acfc40c07 100644 --- a/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs +++ b/codex-rs/app-server/tests/suite/codex_message_processor_flow.rs @@ -114,6 +114,7 @@ async fn test_codex_jsonrpc_conversation_flow() -> Result<()> { conversation_id, items: vec![codex_app_server_protocol::InputItem::Text { text: "text".to_string(), + text_elements: Vec::new(), }], }) .await?; @@ -241,6 +242,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { conversation_id, items: vec![codex_app_server_protocol::InputItem::Text { text: "run python".to_string(), + text_elements: Vec::new(), }], }) .await?; @@ -296,6 +298,7 @@ async fn test_send_user_turn_changes_approval_policy_behavior() -> Result<()> { conversation_id, items: vec![codex_app_server_protocol::InputItem::Text { text: "run python again".to_string(), + text_elements: Vec::new(), }], cwd: working_directory.clone(), approval_policy: AskForApproval::Never, @@ -405,6 +408,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( conversation_id, items: vec![InputItem::Text { text: "first turn".to_string(), + text_elements: Vec::new(), }], cwd: first_cwd.clone(), approval_policy: AskForApproval::Never, @@ -437,6 +441,7 @@ async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() -> Result<( conversation_id, items: vec![InputItem::Text { text: "second turn".to_string(), + text_elements: Vec::new(), }], cwd: second_cwd.clone(), approval_policy: AskForApproval::Never, diff --git a/codex-rs/app-server/tests/suite/create_thread.rs b/codex-rs/app-server/tests/suite/create_thread.rs index 9709af03bf6f..8ad33425393b 100644 --- a/codex-rs/app-server/tests/suite/create_thread.rs +++ b/codex-rs/app-server/tests/suite/create_thread.rs @@ -77,6 +77,7 @@ async fn test_conversation_create_and_send_message_ok() -> Result<()> { conversation_id, items: vec![InputItem::Text { text: "Hello".to_string(), + text_elements: Vec::new(), }], }) .await?; diff --git a/codex-rs/app-server/tests/suite/interrupt.rs b/codex-rs/app-server/tests/suite/interrupt.rs index 6248581e28c7..f8dc2a7e8e1a 100644 --- a/codex-rs/app-server/tests/suite/interrupt.rs +++ b/codex-rs/app-server/tests/suite/interrupt.rs @@ -105,6 +105,7 @@ async fn shell_command_interruption() -> anyhow::Result<()> { conversation_id, items: vec![codex_app_server_protocol::InputItem::Text { text: "run first sleep command".to_string(), + text_elements: Vec::new(), }], }) .await?; diff --git a/codex-rs/app-server/tests/suite/output_schema.rs b/codex-rs/app-server/tests/suite/output_schema.rs index 4ec500a245c3..c120a7fe2da6 100644 --- a/codex-rs/app-server/tests/suite/output_schema.rs +++ b/codex-rs/app-server/tests/suite/output_schema.rs @@ -80,6 +80,7 @@ async fn send_user_turn_accepts_output_schema_v1() -> Result<()> { conversation_id, items: vec![InputItem::Text { text: "Hello".to_string(), + text_elements: Vec::new(), }], cwd: codex_home.path().to_path_buf(), approval_policy: AskForApproval::Never, @@ -181,6 +182,7 @@ async fn send_user_turn_output_schema_is_per_turn_v1() -> Result<()> { conversation_id, items: vec![InputItem::Text { text: "Hello".to_string(), + text_elements: Vec::new(), }], cwd: codex_home.path().to_path_buf(), approval_policy: AskForApproval::Never, @@ -228,6 +230,7 @@ async fn send_user_turn_output_schema_is_per_turn_v1() -> Result<()> { conversation_id, items: vec![InputItem::Text { text: "Hello again".to_string(), + text_elements: Vec::new(), }], cwd: codex_home.path().to_path_buf(), approval_policy: AskForApproval::Never, diff --git a/codex-rs/app-server/tests/suite/send_message.rs b/codex-rs/app-server/tests/suite/send_message.rs index 83e809f48eb1..0c713de87ceb 100644 --- a/codex-rs/app-server/tests/suite/send_message.rs +++ b/codex-rs/app-server/tests/suite/send_message.rs @@ -101,6 +101,7 @@ async fn send_message( conversation_id, items: vec![InputItem::Text { text: message.to_string(), + text_elements: Vec::new(), }], }) .await?; @@ -194,6 +195,7 @@ async fn test_send_message_raw_notifications_opt_in() -> Result<()> { conversation_id, items: vec![InputItem::Text { text: "Hello".to_string(), + text_elements: Vec::new(), }], }) .await?; @@ -245,6 +247,7 @@ async fn test_send_message_session_not_found() -> Result<()> { conversation_id: unknown, items: vec![InputItem::Text { text: "ping".to_string(), + text_elements: Vec::new(), }], }) .await?; @@ -425,7 +428,7 @@ fn content_texts(content: &[ContentItem]) -> Vec<&str> { content .iter() .filter_map(|item| match item { - ContentItem::InputText { text } | ContentItem::OutputText { text } => { + ContentItem::InputText { text, .. } | ContentItem::OutputText { text } => { Some(text.as_str()) } _ => None, diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index 7083081dc62a..bc648f4e50d8 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -58,7 +58,7 @@ impl AgentControl { Op::UserInput { items: vec![UserInput::Text { text: prompt, - // Plain text conversion has no UI element ranges. + // Agent control prompts are plain text with no UI text elements. text_elements: Vec::new(), }], final_output_json_schema: None, @@ -71,12 +71,6 @@ impl AgentControl { result } - /// Interrupt the current task for an existing agent thread. - pub(crate) async fn interrupt_agent(&self, agent_id: ThreadId) -> CodexResult { - let state = self.upgrade()?; - state.send_op(agent_id, Op::Interrupt).await - } - /// Submit a shutdown request to an existing agent thread. pub(crate) async fn shutdown_agent(&self, agent_id: ThreadId) -> CodexResult { let state = self.upgrade()?; diff --git a/codex-rs/core/src/agent/mod.rs b/codex-rs/core/src/agent/mod.rs index 0387eda494f6..d6348b38b3e0 100644 --- a/codex-rs/core/src/agent/mod.rs +++ b/codex-rs/core/src/agent/mod.rs @@ -1,8 +1,6 @@ pub(crate) mod control; -pub(crate) mod role; pub(crate) mod status; pub(crate) use codex_protocol::protocol::AgentStatus; pub(crate) use control::AgentControl; -pub(crate) use role::AgentRole; pub(crate) use status::agent_status_from_event; diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 326d81e0e912..438c7c5eeee8 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -31,7 +31,6 @@ use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; -use codex_protocol::config_types::WebSearchMode; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; @@ -65,8 +64,6 @@ use crate::model_provider_info::WireApi; use crate::tools::spec::create_tools_json_for_chat_completions_api; use crate::tools::spec::create_tools_json_for_responses_api; -pub const WEB_SEARCH_ELIGIBLE_HEADER: &str = "x-oai-web-search-eligible"; - #[derive(Debug)] struct ModelClientState { config: Arc, @@ -322,7 +319,7 @@ impl ModelClientSession { store_override: None, conversation_id: Some(conversation_id), session_source: Some(self.state.session_source.clone()), - extra_headers: build_responses_headers(&self.state.config), + extra_headers: beta_feature_headers(&self.state.config), compression, } } @@ -638,21 +635,6 @@ fn beta_feature_headers(config: &Config) -> ApiHeaderMap { headers } -fn build_responses_headers(config: &Config) -> ApiHeaderMap { - let mut headers = beta_feature_headers(config); - headers.insert( - WEB_SEARCH_ELIGIBLE_HEADER, - HeaderValue::from_static( - if matches!(config.web_search_mode, Some(WebSearchMode::Disabled)) { - "false" - } else { - "true" - }, - ), - ); - headers -} - fn map_response_stream(api_stream: S, otel_manager: OtelManager) -> ResponseStream where S: futures::Stream> diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 59f19dc9bec7..b60ae30662cf 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -36,6 +36,7 @@ use codex_protocol::ThreadId; use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::config_types::WebSearchMode; use codex_protocol::items::TurnItem; +use codex_protocol::items::UserMessageItem; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::HasLegacyEvent; @@ -120,7 +121,6 @@ use crate::protocol::ReviewDecision; use crate::protocol::SandboxPolicy; use crate::protocol::SessionConfiguredEvent; use crate::protocol::SkillErrorInfo; -use crate::protocol::SkillInterface as ProtocolSkillInterface; use crate::protocol::SkillMetadata as ProtocolSkillMetadata; use crate::protocol::StreamErrorEvent; use crate::protocol::Submission; @@ -542,12 +542,24 @@ impl Session { web_search_mode: per_turn_config.web_search_mode, }); + let base_instructions = if per_turn_config.features.enabled(Feature::Collab) { + const COLLAB_INSTRUCTIONS: &str = + include_str!("../templates/collab/experimental_prompt.md"); + let base = session_configuration + .base_instructions + .as_deref() + .unwrap_or(model_info.base_instructions.as_str()); + Some(format!("{base}\n\n{COLLAB_INSTRUCTIONS}")) + } else { + session_configuration.base_instructions.clone() + }; + TurnContext { sub_id, client, cwd: session_configuration.cwd.clone(), developer_instructions: session_configuration.developer_instructions.clone(), - base_instructions: session_configuration.base_instructions.clone(), + base_instructions, compact_prompt: session_configuration.compact_prompt.clone(), user_instructions: session_configuration.user_instructions.clone(), approval_policy: session_configuration.approval_policy.value(), @@ -1518,7 +1530,6 @@ impl Session { self.record_conversation_items(turn_context, std::slice::from_ref(&response_item)) .await; - // Derive a turn item and emit lifecycle events if applicable. if let Some(item) = parse_turn_item(&response_item) { self.emit_turn_item_started(turn_context, &item).await; self.emit_turn_item_completed(turn_context, item).await; @@ -2065,13 +2076,10 @@ mod handlers { codex_protocol::approvals::ElicitationAction::Decline => ElicitationAction::Decline, codex_protocol::approvals::ElicitationAction::Cancel => ElicitationAction::Cancel, }; - // When accepting, send an empty object as content to satisfy MCP servers - // that expect non-null content on Accept. For Decline/Cancel, content is None. - let content = match action { - ElicitationAction::Accept => Some(serde_json::json!({})), - ElicitationAction::Decline | ElicitationAction::Cancel => None, + let response = ElicitationResponse { + action, + content: None, }; - let response = ElicitationResponse { action, content }; if let Err(err) = sess .resolve_elicitation(server_name, request_id, response) .await @@ -2252,6 +2260,7 @@ mod handlers { Arc::clone(&turn_context), vec![UserInput::Text { text: turn_context.compact_prompt().to_string(), + // Compaction prompt is synthesized; no UI element ranges to preserve. text_elements: Vec::new(), }], CompactTask, @@ -2408,7 +2417,7 @@ async fn spawn_review_thread( let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &review_model_info, features: &review_features, - web_search_mode: Some(review_web_search_mode), + web_search_mode: review_web_search_mode, }); let base_instructions = REVIEW_PROMPT.to_string(); @@ -2421,7 +2430,7 @@ async fn spawn_review_thread( let mut per_turn_config = (*config).clone(); per_turn_config.model = Some(model.clone()); per_turn_config.features = review_features.clone(); - per_turn_config.web_search_mode = Some(review_web_search_mode); + per_turn_config.web_search_mode = review_web_search_mode; let otel_manager = parent_turn_context .client @@ -2463,6 +2472,7 @@ async fn spawn_review_thread( // Seed the child task with the review prompt as the initial user message. let input: Vec = vec![UserInput::Text { text: review_prompt, + // Review prompt is synthesized; no UI element ranges to preserve. text_elements: Vec::new(), }]; let tc = Arc::new(review_turn_context); @@ -2484,17 +2494,6 @@ fn skills_to_info(skills: &[SkillMetadata]) -> Vec { name: skill.name.clone(), description: skill.description.clone(), short_description: skill.short_description.clone(), - interface: skill - .interface - .clone() - .map(|interface| ProtocolSkillInterface { - display_name: interface.display_name, - short_description: interface.short_description, - icon_small: interface.icon_small, - icon_large: interface.icon_large, - brand_color: interface.brand_color, - default_prompt: interface.default_prompt, - }), path: skill.path.clone(), scope: skill.scope, }) @@ -2563,9 +2562,19 @@ pub(crate) async fn run_turn( .await; } + let user_message_item = UserMessageItem::new(&input); let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input); let response_item: ResponseItem = initial_input_for_turn.clone().into(); - sess.record_response_item_and_emit_turn_item(turn_context.as_ref(), response_item) + + // Persist the user message to history, but emit the turn item from `UserInput` so + // UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry + // those spans, and `record_response_item_and_emit_turn_item` would drop them. + sess.record_conversation_items(turn_context.as_ref(), &[response_item]) + .await; + let turn_item = TurnItem::UserMessage(user_message_item); + sess.emit_turn_item_started(turn_context.as_ref(), &turn_item) + .await; + sess.emit_turn_item_completed(turn_context.as_ref(), turn_item) .await; if !skill_items.is_empty() { @@ -2965,6 +2974,7 @@ async fn try_run_turn( should_emit_turn_diff = true; needs_follow_up |= sess.has_pending_input().await; + error!("needs_follow_up: {needs_follow_up}"); break Ok(TurnRunResult { needs_follow_up, diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 4dc56f10d25a..f13a5914d59c 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -46,7 +46,7 @@ pub(crate) async fn run_inline_auto_compact_task( let prompt = turn_context.compact_prompt().to_string(); let input = vec![UserInput::Text { text: prompt, - // Plain text conversion has no UI element ranges. + // Compaction prompt is synthesized; no UI element ranges to preserve. text_elements: Vec::new(), }]; @@ -197,7 +197,7 @@ pub fn content_items_to_text(content: &[ContentItem]) -> Option { let mut pieces = Vec::new(); for item in content { match item { - ContentItem::InputText { text } | ContentItem::OutputText { text } => { + ContentItem::InputText { text, .. } | ContentItem::OutputText { text } => { if !text.is_empty() { pieces.push(text.as_str()); } diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index fb6829bb9687..0def0440a80a 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -1124,7 +1124,6 @@ gpt-5 = "gpt-5.1" cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: Some(vec!["one".to_string(), "two".to_string()]), @@ -1146,7 +1145,6 @@ gpt-5 = "gpt-5.1" env_http_headers: None, }, enabled: false, - disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(5)), tool_timeout_sec: None, enabled_tools: None, @@ -1211,7 +1209,6 @@ foo = { command = "cmd" } cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1255,7 +1252,6 @@ foo = { command = "cmd" } # keep me cwd: None, }, enabled: false, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1298,7 +1294,6 @@ foo = { command = "cmd", args = ["--flag"] } # keep me cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1342,7 +1337,6 @@ foo = { command = "cmd" } cwd: None, }, enabled: false, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 6228bc3ac384..5e5d26c791ad 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2,7 +2,6 @@ use crate::auth::AuthCredentialsStoreMode; use crate::config::types::DEFAULT_OTEL_ENVIRONMENT; use crate::config::types::History; use crate::config::types::McpServerConfig; -use crate::config::types::McpServerDisabledReason; use crate::config::types::McpServerTransportConfig; use crate::config::types::Notice; use crate::config::types::Notifications; @@ -20,7 +19,6 @@ use crate::config_loader::ConfigRequirements; use crate::config_loader::LoaderOverrides; use crate::config_loader::McpServerIdentity; use crate::config_loader::McpServerRequirement; -use crate::config_loader::Sourced; use crate::config_loader::load_config_layers_state; use crate::features::Feature; use crate::features::FeatureOverrides; @@ -339,8 +337,7 @@ pub struct Config { /// model info's default preference. pub include_apply_patch_tool: bool, - /// Explicit or feature-derived web search mode. - pub web_search_mode: Option, + pub web_search_mode: WebSearchMode, /// If set to `true`, used only the experimental unified exec tool. pub use_experimental_unified_exec_tool: bool, @@ -542,32 +539,25 @@ fn deserialize_config_toml_with_base( fn filter_mcp_servers_by_requirements( mcp_servers: &mut HashMap, - mcp_requirements: Option<&Sourced>>, + mcp_requirements: Option<&BTreeMap>, ) { let Some(allowlist) = mcp_requirements else { return; }; - let source = allowlist.source.clone(); for (name, server) in mcp_servers.iter_mut() { let allowed = allowlist - .value .get(name) .is_some_and(|requirement| mcp_server_matches_requirement(requirement, server)); - if allowed { - server.disabled_reason = None; - } else { + if !allowed { server.enabled = false; - server.disabled_reason = Some(McpServerDisabledReason::Requirements { - source: source.clone(), - }); } } } fn constrain_mcp_servers( mcp_servers: HashMap, - mcp_requirements: Option<&Sourced>>, + mcp_requirements: Option<&BTreeMap>, ) -> ConstraintResult>> { if mcp_requirements.is_none() { return Ok(Constrained::allow_any(mcp_servers)); @@ -1192,22 +1182,24 @@ pub fn resolve_oss_provider( } } -/// Resolve the web search mode from explicit config and feature flags. +/// Resolve the web search mode from the config, profile, and features. fn resolve_web_search_mode( config_toml: &ConfigToml, config_profile: &ConfigProfile, features: &Features, -) -> Option { +) -> WebSearchMode { + // Enum gets precedence over features flags if let Some(mode) = config_profile.web_search.or(config_toml.web_search) { - return Some(mode); + return mode; } if features.enabled(Feature::WebSearchCached) { - return Some(WebSearchMode::Cached); + return WebSearchMode::Cached; } if features.enabled(Feature::WebSearchRequest) { - return Some(WebSearchMode::Live); + return WebSearchMode::Live; } - None + // Fall back to default + WebSearchMode::default() } impl Config { @@ -1715,7 +1707,6 @@ mod tests { use crate::config::types::HistoryPersistence; use crate::config::types::McpServerTransportConfig; use crate::config::types::Notifications; - use crate::config_loader::RequirementSource; use crate::features::Feature; use super::*; @@ -1737,7 +1728,6 @@ mod tests { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1754,7 +1744,6 @@ mod tests { env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1987,9 +1976,9 @@ trust_level = "trusted" (MATCHED_URL_SERVER.to_string(), http_mcp(GOOD_URL)), (DIFFERENT_NAME_SERVER.to_string(), stdio_mcp("same-cmd")), ]); - let source = RequirementSource::LegacyManagedConfigTomlFromMdm; - let requirements = Sourced::new( - BTreeMap::from([ + filter_mcp_servers_by_requirements( + &mut servers, + Some(&BTreeMap::from([ ( MISMATCHED_URL_SERVER.to_string(), McpServerRequirement { @@ -2022,29 +2011,20 @@ trust_level = "trusted" }, }, ), - ]), - source.clone(), + ])), ); - filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); - let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( servers .iter() - .map(|(name, server)| ( - name.clone(), - (server.enabled, server.disabled_reason.clone()) - )) - .collect::)>>(), + .map(|(name, server)| (name.clone(), server.enabled)) + .collect::>(), HashMap::from([ - (MISMATCHED_URL_SERVER.to_string(), (false, reason.clone())), - ( - MISMATCHED_COMMAND_SERVER.to_string(), - (false, reason.clone()), - ), - (MATCHED_URL_SERVER.to_string(), (true, None)), - (MATCHED_COMMAND_SERVER.to_string(), (true, None)), - (DIFFERENT_NAME_SERVER.to_string(), (false, reason)), + (MISMATCHED_URL_SERVER.to_string(), false), + (MISMATCHED_COMMAND_SERVER.to_string(), false), + (MATCHED_URL_SERVER.to_string(), true), + (MATCHED_COMMAND_SERVER.to_string(), true), + (DIFFERENT_NAME_SERVER.to_string(), false), ]) ); } @@ -2061,14 +2041,11 @@ trust_level = "trusted" assert_eq!( servers .iter() - .map(|(name, server)| ( - name.clone(), - (server.enabled, server.disabled_reason.clone()) - )) - .collect::)>>(), + .map(|(name, server)| (name.clone(), server.enabled)) + .collect::>(), HashMap::from([ - ("server-a".to_string(), (true, None)), - ("server-b".to_string(), (true, None)), + ("server-a".to_string(), true), + ("server-b".to_string(), true), ]) ); } @@ -2080,22 +2057,16 @@ trust_level = "trusted" ("server-b".to_string(), http_mcp("https://example.com/b")), ]); - let source = RequirementSource::LegacyManagedConfigTomlFromMdm; - let requirements = Sourced::new(BTreeMap::new(), source.clone()); - filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); + filter_mcp_servers_by_requirements(&mut servers, Some(&BTreeMap::new())); - let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( servers .iter() - .map(|(name, server)| ( - name.clone(), - (server.enabled, server.disabled_reason.clone()) - )) - .collect::)>>(), + .map(|(name, server)| (name.clone(), server.enabled)) + .collect::>(), HashMap::from([ - ("server-a".to_string(), (false, reason.clone())), - ("server-b".to_string(), (false, reason)), + ("server-a".to_string(), false), + ("server-b".to_string(), false), ]) ); } @@ -2231,12 +2202,15 @@ trust_level = "trusted" } #[test] - fn web_search_mode_uses_none_if_unset() { + fn web_search_mode_uses_default_if_unset() { let cfg = ConfigToml::default(); let profile = ConfigProfile::default(); let features = Features::with_defaults(); - assert_eq!(resolve_web_search_mode(&cfg, &profile, &features), None); + assert_eq!( + resolve_web_search_mode(&cfg, &profile, &features), + WebSearchMode::default() + ); } #[test] @@ -2251,7 +2225,7 @@ trust_level = "trusted" assert_eq!( resolve_web_search_mode(&cfg, &profile, &features), - Some(WebSearchMode::Live) + WebSearchMode::Live ); } @@ -2267,7 +2241,7 @@ trust_level = "trusted" assert_eq!( resolve_web_search_mode(&cfg, &profile, &features), - Some(WebSearchMode::Disabled) + WebSearchMode::Disabled ); } @@ -2517,7 +2491,6 @@ trust_level = "trusted" cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(3)), tool_timeout_sec: Some(Duration::from_secs(5)), enabled_tools: None, @@ -2671,7 +2644,6 @@ bearer_token = "secret" cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -2740,7 +2712,6 @@ ZIG_VAR = "3" cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -2789,7 +2760,6 @@ ZIG_VAR = "3" cwd: Some(cwd_path.clone()), }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -2836,7 +2806,6 @@ ZIG_VAR = "3" env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -2899,7 +2868,6 @@ startup_timeout_sec = 2.0 )])), }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -2974,7 +2942,6 @@ X-Auth = "DOCS_AUTH" )])), }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -3002,7 +2969,6 @@ X-Auth = "DOCS_AUTH" env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -3068,7 +3034,6 @@ url = "https://example.com/mcp" )])), }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -3086,7 +3051,6 @@ url = "https://example.com/mcp" cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -3167,7 +3131,6 @@ url = "https://example.com/mcp" cwd: None, }, enabled: false, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -3210,7 +3173,6 @@ url = "https://example.com/mcp" cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: Some(vec!["allowed".to_string()]), @@ -3619,7 +3581,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: None, + web_search_mode: WebSearchMode::default(), use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), @@ -3706,7 +3668,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: None, + web_search_mode: WebSearchMode::default(), use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), @@ -3808,7 +3770,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: None, + web_search_mode: WebSearchMode::default(), use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), @@ -3896,7 +3858,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: None, + web_search_mode: WebSearchMode::default(), use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 981f9b541aa8..13b201e84e9a 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -3,13 +3,11 @@ // Note this file should generally be restricted to simple struct/enum // definitions that do not contain business logic. -use crate::config_loader::RequirementSource; pub use codex_protocol::config_types::AltScreenMode; pub use codex_protocol::config_types::WebSearchMode; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::BTreeMap; use std::collections::HashMap; -use std::fmt; use std::path::PathBuf; use std::time::Duration; use wildmatch::WildMatchPattern; @@ -22,23 +20,6 @@ use serde::de::Error as SerdeError; pub const DEFAULT_OTEL_ENVIRONMENT: &str = "dev"; -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum McpServerDisabledReason { - Unknown, - Requirements { source: RequirementSource }, -} - -impl fmt::Display for McpServerDisabledReason { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - McpServerDisabledReason::Unknown => write!(f, "unknown"), - McpServerDisabledReason::Requirements { source } => { - write!(f, "requirements ({source})") - } - } - } -} - #[derive(Serialize, Debug, Clone, PartialEq)] pub struct McpServerConfig { #[serde(flatten)] @@ -48,10 +29,6 @@ pub struct McpServerConfig { #[serde(default = "default_enabled")] pub enabled: bool, - /// Reason this server was disabled after applying requirements. - #[serde(skip)] - pub disabled_reason: Option, - /// Startup timeout in seconds for initializing MCP server & initially listing tools. #[serde( default, @@ -183,7 +160,6 @@ impl<'de> Deserialize<'de> for McpServerConfig { startup_timeout_sec, tool_timeout_sec, enabled, - disabled_reason: None, enabled_tools, disabled_tools, }) diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index a83398c7116a..0411b0916b51 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -44,7 +44,7 @@ impl fmt::Display for RequirementSource { pub struct ConfigRequirements { pub approval_policy: Constrained, pub sandbox_policy: Constrained, - pub mcp_servers: Option>>, + pub mcp_servers: Option>, } impl Default for ConfigRequirements { @@ -273,7 +273,7 @@ impl TryFrom for ConfigRequirements { Ok(ConfigRequirements { approval_policy, sandbox_policy, - mcp_servers, + mcp_servers: mcp_servers.map(|sourced| sourced.value), }) } } @@ -571,27 +571,24 @@ mod tests { assert_eq!( requirements.mcp_servers, - Some(Sourced::new( - BTreeMap::from([ - ( - "docs".to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: "codex-mcp".to_string(), - }, + Some(BTreeMap::from([ + ( + "docs".to_string(), + McpServerRequirement { + identity: McpServerIdentity::Command { + command: "codex-mcp".to_string(), }, - ), - ( - "remote".to_string(), - McpServerRequirement { - identity: McpServerIdentity::Url { - url: "https://example.com/mcp".to_string(), - }, + }, + ), + ( + "remote".to_string(), + McpServerRequirement { + identity: McpServerIdentity::Url { + url: "https://example.com/mcp".to_string(), }, - ), - ]), - RequirementSource::Unknown, - )) + }, + ), + ])) ); Ok(()) } diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 9a8791eeef9a..7e6d4223ca8d 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -30,7 +30,6 @@ pub use config_requirements::McpServerIdentity; pub use config_requirements::McpServerRequirement; pub use config_requirements::RequirementSource; pub use config_requirements::SandboxModeRequirement; -pub use config_requirements::Sourced; pub use merge::merge_toml_values; pub(crate) use overrides::build_cli_overrides_layer; pub use state::ConfigLayerEntry; diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 4ee2e252c4d8..4234365f3ae8 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -340,7 +340,7 @@ pub(crate) fn is_user_turn_boundary(item: &ResponseItem) -> bool { for content_item in content { match content_item { - ContentItem::InputText { text } => { + ContentItem::InputText { text, .. } => { if is_session_prefix(text) || is_user_shell_command_text(text) { return false; } diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 6c19c726df31..8ccd034af99a 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -52,7 +52,7 @@ fn parse_user_message(message: &[ContentItem]) -> Option { } content.push(UserInput::Text { text: text.clone(), - // Plain text conversion has no UI element ranges. + // Model input content does not carry UI element ranges. text_elements: Vec::new(), }); } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 498c45748fdc..4c6183454d90 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -111,7 +111,6 @@ mod user_shell_command; pub mod util; pub use apply_patch::CODEX_APPLY_PATCH_ARG1; -pub use client::WEB_SEARCH_ELIGIBLE_HEADER; pub use command_safety::is_dangerous_command; pub use command_safety::is_safe_command; pub use exec_policy::ExecPolicyError; diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 0d638760f556..6574437bd030 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -1171,7 +1171,6 @@ mod tests { env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1216,7 +1215,6 @@ mod tests { env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 7151f95b95d5..dfafb2911588 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -28,7 +28,6 @@ use super::policy::is_persisted_response_item; use crate::config::Config; use crate::default_client::originator; use crate::git_info::collect_git_info; -use crate::path_utils; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; @@ -114,35 +113,6 @@ impl RolloutRecorder { .await } - /// Find the newest recorded thread path, optionally filtering to a matching cwd. - pub async fn find_latest_thread_path( - codex_home: &Path, - allowed_sources: &[SessionSource], - model_providers: Option<&[String]>, - default_provider: &str, - filter_cwd: Option<&Path>, - ) -> std::io::Result> { - let mut cursor: Option = None; - loop { - let page = Self::list_threads( - codex_home, - 25, - cursor.as_ref(), - allowed_sources, - model_providers, - default_provider, - ) - .await?; - if let Some(path) = select_resume_path(&page, filter_cwd) { - return Ok(Some(path)); - } - cursor = page.next_cursor; - if cursor.is_none() { - return Ok(None); - } - } - } - /// Attempt to create a new [`RolloutRecorder`]. If the sessions directory /// cannot be created or the rollout file cannot be opened we return the /// error so the caller can decide whether to disable persistence. @@ -461,36 +431,3 @@ impl JsonlWriter { Ok(()) } } - -fn select_resume_path(page: &ThreadsPage, filter_cwd: Option<&Path>) -> Option { - match filter_cwd { - Some(cwd) => page.items.iter().find_map(|item| { - if session_cwd_matches(&item.head, cwd) { - Some(item.path.clone()) - } else { - None - } - }), - None => page.items.first().map(|item| item.path.clone()), - } -} - -fn session_cwd_matches(head: &[serde_json::Value], cwd: &Path) -> bool { - let Some(session_cwd) = extract_session_cwd(head) else { - return false; - }; - if let (Ok(ca), Ok(cb)) = ( - path_utils::normalize_for_path_comparison(&session_cwd), - path_utils::normalize_for_path_comparison(cwd), - ) { - return ca == cb; - } - session_cwd == cwd -} - -fn extract_session_cwd(head: &[serde_json::Value]) -> Option { - head.iter().find_map(|value| { - let meta_line = serde_json::from_value::(value.clone()).ok()?; - Some(meta_line.meta.cwd) - }) -} diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index da51fea41eed..a7461dcf97fd 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -1,7 +1,6 @@ use crate::config::Config; use crate::config_loader::ConfigLayerStack; use crate::skills::model::SkillError; -use crate::skills::model::SkillInterface; use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; use crate::skills::system::system_cache_root_dir; @@ -14,7 +13,6 @@ use std::collections::VecDeque; use std::error::Error; use std::fmt; use std::fs; -use std::path::Component; use std::path::Path; use std::path::PathBuf; use tracing::error; @@ -33,29 +31,11 @@ struct SkillFrontmatterMetadata { short_description: Option, } -#[derive(Debug, Default, Deserialize)] -struct SkillToml { - #[serde(default)] - interface: Option, -} - -#[derive(Debug, Default, Deserialize)] -struct Interface { - display_name: Option, - short_description: Option, - icon_small: Option, - icon_large: Option, - brand_color: Option, - default_prompt: Option, -} - const SKILLS_FILENAME: &str = "SKILL.md"; -const SKILLS_TOML_FILENAME: &str = "SKILL.toml"; const SKILLS_DIR_NAME: &str = "skills"; const MAX_NAME_LEN: usize = 64; const MAX_DESCRIPTION_LEN: usize = 1024; const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN; -const MAX_DEFAULT_PROMPT_LEN: usize = MAX_DESCRIPTION_LEN; // Traversal depth from the skills root. const MAX_SCAN_DEPTH: usize = 6; const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000; @@ -215,7 +195,7 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil } } - // Follow symlinked directories for user, admin, and repo skills. System skills are written by Codex itself. + // Follow symlinks for user, admin, and repo skills. System skills are written by Codex itself. let follow_symlinks = matches!( scope, SkillScope::Repo | SkillScope::User | SkillScope::Admin @@ -282,6 +262,20 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil continue; } + if metadata.is_file() && file_name == SKILLS_FILENAME { + match parse_skill_file(&path, scope) { + Ok(skill) => outcome.skills.push(skill), + Err(err) => { + if scope != SkillScope::System { + outcome.errors.push(SkillError { + path, + message: err.to_string(), + }); + } + } + } + } + continue; } @@ -342,12 +336,11 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result Option { - // Fail open: optional SKILL.toml metadata should not block loading SKILL.md. - let skill_dir = skill_path.parent()?; - let interface_path = skill_dir.join(SKILLS_TOML_FILENAME); - if !interface_path.exists() { - return None; - } - - let contents = match fs::read_to_string(&interface_path) { - Ok(contents) => contents, - Err(error) => { - tracing::warn!( - "ignoring {path}: failed to read SKILL.toml: {error}", - path = interface_path.display() - ); - return None; - } - }; - let parsed: SkillToml = match toml::from_str(&contents) { - Ok(parsed) => parsed, - Err(error) => { - tracing::warn!( - "ignoring {path}: invalid TOML: {error}", - path = interface_path.display() - ); - return None; - } - }; - let interface = parsed.interface?; - - let interface = SkillInterface { - display_name: resolve_str( - interface.display_name, - MAX_NAME_LEN, - "interface.display_name", - ), - short_description: resolve_str( - interface.short_description, - MAX_SHORT_DESCRIPTION_LEN, - "interface.short_description", - ), - icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small), - icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large), - brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"), - default_prompt: resolve_str( - interface.default_prompt, - MAX_DEFAULT_PROMPT_LEN, - "interface.default_prompt", - ), - }; - let has_fields = interface.display_name.is_some() - || interface.short_description.is_some() - || interface.icon_small.is_some() - || interface.icon_large.is_some() - || interface.brand_color.is_some() - || interface.default_prompt.is_some(); - if has_fields { Some(interface) } else { None } -} - -fn resolve_asset_path( - skill_dir: &Path, - field: &'static str, - path: Option, -) -> Option { - // Icons must be relative paths under the skill's assets/ directory; otherwise return None. - let path = path?; - if path.as_os_str().is_empty() { - return None; - } - - let assets_dir = skill_dir.join("assets"); - if path.is_absolute() { - tracing::warn!( - "ignoring {field}: icon must be a relative assets path (not {})", - assets_dir.display() - ); - return None; - } - - let mut components = path.components().peekable(); - while matches!(components.peek(), Some(Component::CurDir)) { - components.next(); - } - match components.next() { - Some(Component::Normal(component)) if component == "assets" => {} - _ => { - tracing::warn!("ignoring {field}: icon path must be under assets/"); - return None; - } - } - if components.any(|component| matches!(component, Component::ParentDir)) { - tracing::warn!("ignoring {field}: icon path must not contain '..'"); - return None; - } - - Some(skill_dir.join(path)) -} - fn sanitize_single_line(raw: &str) -> String { raw.split_whitespace().collect::>().join(" ") } -fn validate_len( +fn validate_field( value: &str, max_len: usize, field_name: &'static str, @@ -485,36 +379,6 @@ fn validate_len( Ok(()) } -fn resolve_str(value: Option, max_len: usize, field: &'static str) -> Option { - let value = value?; - let value = sanitize_single_line(&value); - if value.is_empty() { - tracing::warn!("ignoring {field}: value is empty"); - return None; - } - if value.chars().count() > max_len { - tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters"); - return None; - } - Some(value) -} - -fn resolve_color_str(value: Option, field: &'static str) -> Option { - let value = value?; - let value = value.trim(); - if value.is_empty() { - tracing::warn!("ignoring {field}: value is empty"); - return None; - } - let mut chars = value.chars(); - if value.len() == 7 && chars.next() == Some('#') && chars.all(|c| c.is_ascii_hexdigit()) { - Some(value.to_string()) - } else { - tracing::warn!("ignoring {field}: expected #RRGGBB, got {value}"); - None - } -} - fn extract_frontmatter(contents: &str) -> Option { let mut lines = contents.lines(); if !matches!(lines.next(), Some(line) if line.trim() == "---") { @@ -664,224 +528,6 @@ mod tests { path } - fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf { - let path = skill_dir.join(SKILLS_TOML_FILENAME); - fs::write(&path, contents).unwrap(); - path - } - - #[tokio::test] - async fn loads_skill_interface_metadata_happy_path() { - let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); - let skill_dir = skill_path.parent().expect("skill dir"); - let normalized_skill_dir = normalized(skill_dir); - - write_skill_interface_at( - skill_dir, - r##" -[interface] -display_name = "UI Skill" -short_description = " short desc " -icon_small = "./assets/small-400px.png" -icon_large = "./assets/large-logo.svg" -brand_color = "#3B82F6" -default_prompt = " default prompt " -"##, - ); - - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); - - assert!( - outcome.errors.is_empty(), - "unexpected errors: {:?}", - outcome.errors - ); - assert_eq!( - outcome.skills, - vec![SkillMetadata { - name: "ui-skill".to_string(), - description: "from toml".to_string(), - short_description: None, - interface: Some(SkillInterface { - display_name: Some("UI Skill".to_string()), - short_description: Some("short desc".to_string()), - icon_small: Some(normalized_skill_dir.join("./assets/small-400px.png")), - icon_large: Some(normalized_skill_dir.join("./assets/large-logo.svg")), - brand_color: Some("#3B82F6".to_string()), - default_prompt: Some("default prompt".to_string()), - }), - path: normalized(&skill_path), - scope: SkillScope::User, - }] - ); - } - - #[tokio::test] - async fn accepts_icon_paths_under_assets_dir() { - let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); - let skill_dir = skill_path.parent().expect("skill dir"); - let normalized_skill_dir = normalized(skill_dir); - - write_skill_interface_at( - skill_dir, - r#" -[interface] -display_name = "UI Skill" -icon_small = "assets/icon.png" -icon_large = "./assets/logo.svg" -"#, - ); - - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); - - assert!( - outcome.errors.is_empty(), - "unexpected errors: {:?}", - outcome.errors - ); - assert_eq!( - outcome.skills, - vec![SkillMetadata { - name: "ui-skill".to_string(), - description: "from toml".to_string(), - short_description: None, - interface: Some(SkillInterface { - display_name: Some("UI Skill".to_string()), - short_description: None, - icon_small: Some(normalized_skill_dir.join("assets/icon.png")), - icon_large: Some(normalized_skill_dir.join("./assets/logo.svg")), - brand_color: None, - default_prompt: None, - }), - path: normalized(&skill_path), - scope: SkillScope::User, - }] - ); - } - - #[tokio::test] - async fn ignores_invalid_brand_color() { - let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); - let skill_dir = skill_path.parent().expect("skill dir"); - - write_skill_interface_at( - skill_dir, - r#" -[interface] -brand_color = "blue" -"#, - ); - - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); - - assert!( - outcome.errors.is_empty(), - "unexpected errors: {:?}", - outcome.errors - ); - assert_eq!( - outcome.skills, - vec![SkillMetadata { - name: "ui-skill".to_string(), - description: "from toml".to_string(), - short_description: None, - interface: None, - path: normalized(&skill_path), - scope: SkillScope::User, - }] - ); - } - - #[tokio::test] - async fn ignores_default_prompt_over_max_length() { - let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); - let skill_dir = skill_path.parent().expect("skill dir"); - let normalized_skill_dir = normalized(skill_dir); - let too_long = "x".repeat(MAX_DEFAULT_PROMPT_LEN + 1); - - write_skill_interface_at( - skill_dir, - &format!( - r##" -[interface] -display_name = "UI Skill" -icon_small = "./assets/small-400px.png" -default_prompt = "{too_long}" -"## - ), - ); - - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); - - assert!( - outcome.errors.is_empty(), - "unexpected errors: {:?}", - outcome.errors - ); - assert_eq!( - outcome.skills, - vec![SkillMetadata { - name: "ui-skill".to_string(), - description: "from toml".to_string(), - short_description: None, - interface: Some(SkillInterface { - display_name: Some("UI Skill".to_string()), - short_description: None, - icon_small: Some(normalized_skill_dir.join("./assets/small-400px.png")), - icon_large: None, - brand_color: None, - default_prompt: None, - }), - path: normalized(&skill_path), - scope: SkillScope::User, - }] - ); - } - - #[tokio::test] - async fn drops_interface_when_icons_are_invalid() { - let codex_home = tempfile::tempdir().expect("tempdir"); - let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); - let skill_dir = skill_path.parent().expect("skill dir"); - - write_skill_interface_at( - skill_dir, - r#" -[interface] -icon_small = "icon.png" -icon_large = "./assets/../logo.svg" -"#, - ); - - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); - - assert!( - outcome.errors.is_empty(), - "unexpected errors: {:?}", - outcome.errors - ); - assert_eq!( - outcome.skills, - vec![SkillMetadata { - name: "ui-skill".to_string(), - description: "from toml".to_string(), - short_description: None, - interface: None, - path: normalized(&skill_path), - scope: SkillScope::User, - }] - ); - } - #[cfg(unix)] fn symlink_dir(target: &Path, link: &Path) { std::os::unix::fs::symlink(target, link).unwrap(); @@ -917,7 +563,6 @@ icon_large = "./assets/../logo.svg" name: "linked-skill".to_string(), description: "from link".to_string(), short_description: None, - interface: None, path: normalized(&shared_skill_path), scope: SkillScope::User, }] @@ -926,7 +571,7 @@ icon_large = "./assets/../logo.svg" #[tokio::test] #[cfg(unix)] - async fn ignores_symlinked_skill_file_for_user_scope() { + async fn loads_skills_via_symlinked_skill_file_for_user_scope() { let codex_home = tempfile::tempdir().expect("tempdir"); let shared = tempfile::tempdir().expect("tempdir"); @@ -945,7 +590,16 @@ icon_large = "./assets/../logo.svg" "unexpected errors: {:?}", outcome.errors ); - assert_eq!(outcome.skills, Vec::new()); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "linked-file-skill".to_string(), + description: "from link".to_string(), + short_description: None, + path: normalized(&shared_skill_path), + scope: SkillScope::User, + }] + ); } #[tokio::test] @@ -975,7 +629,6 @@ icon_large = "./assets/../logo.svg" name: "cycle-skill".to_string(), description: "still loads".to_string(), short_description: None, - interface: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1009,7 +662,6 @@ icon_large = "./assets/../logo.svg" name: "admin-linked-skill".to_string(), description: "from link".to_string(), short_description: None, - interface: None, path: normalized(&shared_skill_path), scope: SkillScope::Admin, }] @@ -1047,7 +699,6 @@ icon_large = "./assets/../logo.svg" name: "repo-linked-skill".to_string(), description: "from link".to_string(), short_description: None, - interface: None, path: normalized(&linked_skill_path), scope: SkillScope::Repo, }] @@ -1108,7 +759,6 @@ icon_large = "./assets/../logo.svg" name: "within-depth-skill".to_string(), description: "loads".to_string(), short_description: None, - interface: None, path: normalized(&within_depth_path), scope: SkillScope::User, }] @@ -1133,7 +783,6 @@ icon_large = "./assets/../logo.svg" name: "demo-skill".to_string(), description: "does things carefully".to_string(), short_description: None, - interface: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1162,7 +811,6 @@ icon_large = "./assets/../logo.svg" name: "demo-skill".to_string(), description: "long description".to_string(), short_description: Some("short summary".to_string()), - interface: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1272,7 +920,6 @@ icon_large = "./assets/../logo.svg" name: "repo-skill".to_string(), description: "from repo".to_string(), short_description: None, - interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1323,7 +970,6 @@ icon_large = "./assets/../logo.svg" name: "nested-skill".to_string(), description: "from nested".to_string(), short_description: None, - interface: None, path: normalized(&nested_skill_path), scope: SkillScope::Repo, }, @@ -1331,7 +977,6 @@ icon_large = "./assets/../logo.svg" name: "root-skill".to_string(), description: "from root".to_string(), short_description: None, - interface: None, path: normalized(&root_skill_path), scope: SkillScope::Repo, }, @@ -1368,7 +1013,6 @@ icon_large = "./assets/../logo.svg" name: "local-skill".to_string(), description: "from cwd".to_string(), short_description: None, - interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1406,7 +1050,6 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from repo".to_string(), short_description: None, - interface: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, }] @@ -1434,7 +1077,6 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from user".to_string(), short_description: None, - interface: None, path: normalized(&user_skill_path), scope: SkillScope::User, }] @@ -1503,7 +1145,6 @@ icon_large = "./assets/../logo.svg" name: "repo-skill".to_string(), description: "from repo".to_string(), short_description: None, - interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1559,7 +1200,6 @@ icon_large = "./assets/../logo.svg" name: "system-skill".to_string(), description: "from system".to_string(), short_description: None, - interface: None, path: normalized(&skill_path), scope: SkillScope::System, }] @@ -1614,7 +1254,6 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from system".to_string(), short_description: None, - interface: None, path: normalized(&system_skill_path), scope: SkillScope::System, }] @@ -1644,7 +1283,6 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from user".to_string(), short_description: None, - interface: None, path: normalized(&user_skill_path), scope: SkillScope::User, }] @@ -1683,7 +1321,6 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from repo".to_string(), short_description: None, - interface: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, }] @@ -1734,7 +1371,6 @@ icon_large = "./assets/../logo.svg" name: "dupe-skill".to_string(), description: "from nested".to_string(), short_description: None, - interface: None, path: expected_path, scope: SkillScope::Repo, }], diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core/src/skills/model.rs index fba00d2711ee..9063d7a25039 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -7,21 +7,10 @@ pub struct SkillMetadata { pub name: String, pub description: String, pub short_description: Option, - pub interface: Option, pub path: PathBuf, pub scope: SkillScope, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SkillInterface { - pub display_name: Option, - pub short_description: Option, - pub icon_small: Option, - pub icon_large: Option, - pub brand_color: Option, - pub default_prompt: Option, -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillError { pub path: PathBuf, diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index 9157e922ecf3..353189a6be97 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -86,7 +86,7 @@ async fn start_review_conversation( let mut sub_agent_config = config.as_ref().clone(); // Carry over review-only feature restrictions so the delegate cannot // re-enable blocked tools (web search, view image). - sub_agent_config.web_search_mode = Some(WebSearchMode::Disabled); + sub_agent_config.web_search_mode = WebSearchMode::Disabled; // Set explicit review rubric for the sub-agent sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string()); diff --git a/codex-rs/core/src/tools/handlers/collab.rs b/codex-rs/core/src/tools/handlers/collab.rs index df3f2901dac9..805af76b3aea 100644 --- a/codex-rs/core/src/tools/handlers/collab.rs +++ b/codex-rs/core/src/tools/handlers/collab.rs @@ -76,13 +76,11 @@ impl ToolHandler for CollabHandler { mod spawn { use super::*; - use crate::agent::AgentRole; use std::sync::Arc; #[derive(Debug, Deserialize)] struct SpawnAgentArgs { message: String, - agent_type: Option, } #[derive(Debug, Serialize)] @@ -97,7 +95,6 @@ mod spawn { arguments: String, ) -> Result { let args: SpawnAgentArgs = parse_arguments(&arguments)?; - let agent_role = args.agent_type.unwrap_or(AgentRole::Default); let prompt = args.message; if prompt.trim().is_empty() { return Err(FunctionCallError::RespondToModel( @@ -115,10 +112,7 @@ mod spawn { .into(), ) .await; - let mut config = build_agent_spawn_config(turn.as_ref())?; - agent_role - .apply_to_config(&mut config) - .map_err(FunctionCallError::RespondToModel)?; + let config = build_agent_spawn_config(turn.as_ref())?; let result = session .services .agent_control @@ -170,8 +164,6 @@ mod send_input { struct SendInputArgs { id: String, message: String, - #[serde(default)] - interrupt: bool, } #[derive(Debug, Serialize)] @@ -193,14 +185,6 @@ mod send_input { "Empty message can't be sent to an agent".to_string(), )); } - if args.interrupt { - session - .services - .agent_control - .interrupt_agent(receiver_thread_id) - .await - .map_err(|err| collab_agent_error(receiver_thread_id, err))?; - } session .send_event( &turn, @@ -733,45 +717,6 @@ mod tests { ); } - #[tokio::test] - async fn send_input_interrupts_before_prompt() { - let (mut session, turn) = make_session_and_context().await; - let manager = thread_manager(); - session.services.agent_control = manager.agent_control(); - let config = turn.client.config().as_ref().clone(); - let thread = manager.start_thread(config).await.expect("start thread"); - let agent_id = thread.thread_id; - let invocation = invocation( - Arc::new(session), - Arc::new(turn), - "send_input", - function_payload(json!({ - "id": agent_id.to_string(), - "message": "hi", - "interrupt": true - })), - ); - CollabHandler - .handle(invocation) - .await - .expect("send_input should succeed"); - - let ops = manager.captured_ops(); - let ops_for_agent: Vec<&Op> = ops - .iter() - .filter_map(|(id, op)| (*id == agent_id).then_some(op)) - .collect(); - assert_eq!(ops_for_agent.len(), 2); - assert!(matches!(ops_for_agent[0], Op::Interrupt)); - assert!(matches!(ops_for_agent[1], Op::UserInput { .. })); - - let _ = thread - .thread - .submit(Op::Shutdown {}) - .await - .expect("shutdown should submit"); - } - #[tokio::test] async fn wait_rejects_non_positive_timeout() { let (session, turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 5a35f2b6481c..28fc3a170174 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1,4 +1,3 @@ -use crate::agent::AgentRole; use crate::client_common::tools::ResponsesApiTool; use crate::client_common::tools::ToolSpec; use crate::features::Feature; @@ -25,7 +24,7 @@ use std::collections::HashMap; pub(crate) struct ToolsConfig { pub shell_type: ConfigShellToolType, pub apply_patch_tool_type: Option, - pub web_search_mode: Option, + pub web_search_mode: WebSearchMode, pub collab_tools: bool, pub experimental_supported_tools: Vec, } @@ -33,7 +32,7 @@ pub(crate) struct ToolsConfig { pub(crate) struct ToolsConfigParams<'a> { pub(crate) model_info: &'a ModelInfo, pub(crate) features: &'a Features, - pub(crate) web_search_mode: Option, + pub(crate) web_search_mode: WebSearchMode, } impl ToolsConfig { @@ -442,15 +441,6 @@ fn create_spawn_agent_tool() -> ToolSpec { description: Some("Initial message to send to the new agent.".to_string()), }, ); - properties.insert( - "agent_type".to_string(), - JsonSchema::String { - description: Some(format!( - "Optional agent type to spawn ({}).", - AgentRole::enum_values().join(", ") - )), - }, - ); ToolSpec::Function(ResponsesApiTool { name: "spawn_agent".to_string(), @@ -478,15 +468,6 @@ fn create_send_input_tool() -> ToolSpec { description: Some("Message to send to the agent.".to_string()), }, ); - properties.insert( - "interrupt".to_string(), - JsonSchema::Boolean { - description: Some( - "When true, interrupt the agent's current task before sending the message. When false (default), the message will be processed when the agent is done on its current task." - .to_string(), - ), - }, - ); ToolSpec::Function(ResponsesApiTool { name: "send_input".to_string(), @@ -1244,17 +1225,17 @@ pub(crate) fn build_specs( } match config.web_search_mode { - Some(WebSearchMode::Cached) => { + WebSearchMode::Disabled => {} + WebSearchMode::Cached => { builder.push_spec(ToolSpec::WebSearch { external_web_access: Some(false), }); } - Some(WebSearchMode::Live) => { + WebSearchMode::Live => { builder.push_spec(ToolSpec::WebSearch { external_web_access: Some(true), }); } - Some(WebSearchMode::Disabled) | None => {} } builder.push_spec_with_parallel_support(create_view_image_tool(), true); @@ -1398,7 +1379,7 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Live), + web_search_mode: WebSearchMode::Live, }); let (tools, _) = build_specs(&config, None).build(); @@ -1460,7 +1441,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs(&tools_config, None).build(); assert_contains_tool_names( @@ -1472,7 +1453,7 @@ mod tests { fn assert_model_tools( model_slug: &str, features: &Features, - web_search_mode: Option, + web_search_mode: WebSearchMode, expected_tools: &[&str], ) { let config = test_config(); @@ -1496,7 +1477,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1518,7 +1499,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Live), + web_search_mode: WebSearchMode::Live, }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1536,7 +1517,7 @@ mod tests { assert_model_tools( "gpt-5-codex", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "shell_command", "list_mcp_resources", @@ -1555,7 +1536,7 @@ mod tests { assert_model_tools( "gpt-5.1-codex", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "shell_command", "list_mcp_resources", @@ -1574,7 +1555,7 @@ mod tests { assert_model_tools( "gpt-5-codex", Features::with_defaults().enable(Feature::UnifiedExec), - Some(WebSearchMode::Live), + WebSearchMode::Live, &[ "exec_command", "write_stdin", @@ -1594,7 +1575,7 @@ mod tests { assert_model_tools( "gpt-5.1-codex", Features::with_defaults().enable(Feature::UnifiedExec), - Some(WebSearchMode::Live), + WebSearchMode::Live, &[ "exec_command", "write_stdin", @@ -1614,7 +1595,7 @@ mod tests { assert_model_tools( "codex-mini-latest", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "local_shell", "list_mcp_resources", @@ -1632,7 +1613,7 @@ mod tests { assert_model_tools( "gpt-5.1-codex-mini", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "shell_command", "list_mcp_resources", @@ -1651,7 +1632,7 @@ mod tests { assert_model_tools( "gpt-5", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "shell", "list_mcp_resources", @@ -1669,7 +1650,7 @@ mod tests { assert_model_tools( "gpt-5.1", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "shell_command", "list_mcp_resources", @@ -1688,7 +1669,7 @@ mod tests { assert_model_tools( "exp-5.1", &Features::with_defaults(), - Some(WebSearchMode::Cached), + WebSearchMode::Cached, &[ "exec_command", "write_stdin", @@ -1708,7 +1689,7 @@ mod tests { assert_model_tools( "codex-mini-latest", Features::with_defaults().enable(Feature::UnifiedExec), - Some(WebSearchMode::Live), + WebSearchMode::Live, &[ "exec_command", "write_stdin", @@ -1731,7 +1712,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Live), + web_search_mode: WebSearchMode::Live, }); let (tools, _) = build_specs(&tools_config, Some(HashMap::new())).build(); @@ -1753,7 +1734,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1772,7 +1753,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1803,7 +1784,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Live), + web_search_mode: WebSearchMode::Live, }); let (tools, _) = build_specs( &tools_config, @@ -1898,7 +1879,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); // Intentionally construct a map with keys that would sort alphabetically. @@ -1975,7 +1956,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs( @@ -2032,7 +2013,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs( @@ -2086,7 +2067,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs( @@ -2142,7 +2123,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs( @@ -2254,7 +2235,7 @@ Examples of valid command strings: let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: Some(WebSearchMode::Cached), + web_search_mode: WebSearchMode::Cached, }); let (tools, _) = build_specs( &tools_config, diff --git a/codex-rs/core/src/user_instructions.rs b/codex-rs/core/src/user_instructions.rs index 9c563c29c5b0..eb7d520e997a 100644 --- a/codex-rs/core/src/user_instructions.rs +++ b/codex-rs/core/src/user_instructions.rs @@ -17,7 +17,7 @@ pub(crate) struct UserInstructions { impl UserInstructions { pub fn is_user_instructions(message: &[ContentItem]) -> bool { - if let [ContentItem::InputText { text }] = message { + if let [ContentItem::InputText { text, .. }] = message { text.starts_with(USER_INSTRUCTIONS_PREFIX) || text.starts_with(USER_INSTRUCTIONS_OPEN_TAG_LEGACY) } else { @@ -52,7 +52,7 @@ pub(crate) struct SkillInstructions { impl SkillInstructions { pub fn is_skill_instructions(message: &[ContentItem]) -> bool { - if let [ContentItem::InputText { text }] = message { + if let [ContentItem::InputText { text, .. }] = message { text.starts_with(SKILL_INSTRUCTIONS_PREFIX) } else { false @@ -94,7 +94,7 @@ mod tests { assert_eq!(role, "user"); - let [ContentItem::InputText { text }] = content.as_slice() else { + let [ContentItem::InputText { text, .. }] = content.as_slice() else { panic!("expected one InputText content item"); }; @@ -138,7 +138,7 @@ mod tests { assert_eq!(role, "user"); - let [ContentItem::InputText { text }] = content.as_slice() else { + let [ContentItem::InputText { text, .. }] = content.as_slice() else { panic!("expected one InputText content item"); }; diff --git a/codex-rs/core/src/user_shell_command.rs b/codex-rs/core/src/user_shell_command.rs index fb8efcc09ca5..e8f6ab7e9a7f 100644 --- a/codex-rs/core/src/user_shell_command.rs +++ b/codex-rs/core/src/user_shell_command.rs @@ -95,7 +95,7 @@ mod tests { let ResponseItem::Message { content, .. } = item else { panic!("expected message"); }; - let [ContentItem::InputText { text }] = content.as_slice() else { + let [ContentItem::InputText { text, .. }] = content.as_slice() else { panic!("expected input text"); }; assert_eq!( diff --git a/codex-rs/core/tests/chat_completions_sse.rs b/codex-rs/core/tests/chat_completions_sse.rs index f6d7eb24fed6..115acf889fd4 100644 --- a/codex-rs/core/tests/chat_completions_sse.rs +++ b/codex-rs/core/tests/chat_completions_sse.rs @@ -129,7 +129,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec { fn assert_message(item: &ResponseItem, expected: &str) { if let ResponseItem::Message { content, .. } = item { let text = content.iter().find_map(|part| match part { - ContentItem::OutputText { text } | ContentItem::InputText { text } => Some(text), + ContentItem::OutputText { text } | ContentItem::InputText { text, .. } => Some(text), _ => None, }); let Some(text) = text else { diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 8eba3eabe71d..8be6e3634051 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -9,18 +9,15 @@ use codex_core::ModelProviderInfo; use codex_core::Prompt; use codex_core::ResponseEvent; use codex_core::ResponseItem; -use codex_core::WEB_SEARCH_ELIGIBLE_HEADER; use codex_core::WireApi; use codex_core::models_manager::manager::ModelsManager; use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::config_types::ReasoningSummary; -use codex_protocol::config_types::WebSearchMode; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use core_test_support::load_default_config_for_test; use core_test_support::responses; -use core_test_support::test_codex::test_codex; use futures::StreamExt; use tempfile::TempDir; use wiremock::matchers::header; @@ -216,66 +213,6 @@ async fn responses_stream_includes_subagent_header_on_other() { ); } -#[tokio::test] -async fn responses_stream_includes_web_search_eligible_header_true_by_default() { - core_test_support::skip_if_no_network!(); - - let server = responses::start_mock_server().await; - let response_body = responses::sse(vec![ - responses::ev_response_created("resp-1"), - responses::ev_completed("resp-1"), - ]); - - let request_recorder = responses::mount_sse_once_match( - &server, - header(WEB_SEARCH_ELIGIBLE_HEADER, "true"), - response_body, - ) - .await; - - let test = test_codex().build(&server).await.expect("build test codex"); - test.submit_turn("hello").await.expect("submit test prompt"); - - let request = request_recorder.single_request(); - assert_eq!( - request.header(WEB_SEARCH_ELIGIBLE_HEADER).as_deref(), - Some("true") - ); -} - -#[tokio::test] -async fn responses_stream_includes_web_search_eligible_header_false_when_disabled() { - core_test_support::skip_if_no_network!(); - - let server = responses::start_mock_server().await; - let response_body = responses::sse(vec![ - responses::ev_response_created("resp-1"), - responses::ev_completed("resp-1"), - ]); - - let request_recorder = responses::mount_sse_once_match( - &server, - header(WEB_SEARCH_ELIGIBLE_HEADER, "false"), - response_body, - ) - .await; - - let test = test_codex() - .with_config(|config| { - config.web_search_mode = Some(WebSearchMode::Disabled); - }) - .build(&server) - .await - .expect("build test codex"); - test.submit_turn("hello").await.expect("submit test prompt"); - - let request = request_recorder.single_request(); - assert_eq!( - request.header(WEB_SEARCH_ELIGIBLE_HEADER).as_deref(), - Some("false") - ); -} - #[tokio::test] async fn responses_respects_model_info_overrides_from_config() { core_test_support::skip_if_no_network!(); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 6f659e1b6497..18d3caf3bfcb 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -14,7 +14,6 @@ use codex_core::ThreadManager; use codex_core::WireApi; use codex_core::auth::AuthCredentialsStoreMode; use codex_core::built_in_model_providers; -use codex_core::default_client::originator; use codex_core::error::CodexErr; use codex_core::models_manager::manager::ModelsManager; use codex_core::protocol::EventMsg; @@ -407,7 +406,7 @@ async fn includes_conversation_id_and_model_headers_in_request() { let request_originator = request.header("originator").expect("originator header"); assert_eq!(request_session_id, session_id.to_string()); - assert_eq!(request_originator, originator().value); + assert_eq!(request_originator, "codex_cli_rs"); assert_eq!(request_authorization, "Bearer Test API Key"); } @@ -523,7 +522,7 @@ async fn chatgpt_auth_sends_correct_request() { let session_id = request.header("session_id").expect("session_id header"); assert_eq!(session_id, thread_id.to_string()); - assert_eq!(request_originator, originator().value); + assert_eq!(request_originator, "codex_cli_rs"); assert_eq!(request_authorization, "Bearer Access Token"); assert_eq!(request_chatgpt_account_id, "account_id"); assert!(request_body["stream"].as_bool().unwrap()); diff --git a/codex-rs/core/tests/suite/model_tools.rs b/codex-rs/core/tests/suite/model_tools.rs index 8a4d0a77176c..4eaf3735a37c 100644 --- a/codex-rs/core/tests/suite/model_tools.rs +++ b/codex-rs/core/tests/suite/model_tools.rs @@ -36,7 +36,7 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec { let mut builder = test_codex() .with_model(model) // Keep tool expectations stable when the default web_search mode changes. - .with_config(|config| config.web_search_mode = Some(WebSearchMode::Cached)); + .with_config(|config| config.web_search_mode = WebSearchMode::Cached); let test = builder .build(&server) .await diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 6ea5619bfe59..b3ed78f69db4 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -88,7 +88,7 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> { config.user_instructions = Some("be consistent and helpful".to_string()); config.model = Some("gpt-5.1-codex-max".to_string()); // Keep tool expectations stable when the default web_search mode changes. - config.web_search_mode = Some(WebSearchMode::Cached); + config.web_search_mode = WebSearchMode::Cached; }) .build(&server) .await?; diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index d182af82558c..b0ed8db07feb 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -137,7 +137,7 @@ async fn review_op_emits_lifecycle_and_review_output() { if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item { if role == "user" { for c in content { - if let ContentItem::InputText { text } = c { + if let ContentItem::InputText { text, .. } = c { if text.contains("full review output from reviewer model") { saw_header = true; } @@ -639,7 +639,7 @@ async fn review_input_isolated_from_parent_history() { && role == "user" { for c in content { - if let ContentItem::InputText { text } = c + if let ContentItem::InputText { text, .. } = c && text.contains("User initiated a review task, but was interrupted.") { saw_interruption_message = true; diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 5abc39264406..5001eead3234 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -88,7 +88,6 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -226,7 +225,6 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -422,7 +420,6 @@ async fn stdio_image_completions_round_trip() -> anyhow::Result<()> { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -566,7 +563,6 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -721,7 +717,6 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -908,7 +903,6 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { env_http_headers: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index 4ae095517c1b..a86489bdd05b 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -426,7 +426,6 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -518,7 +517,6 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -779,7 +777,6 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { cwd: None, }, enabled: true, - disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/tests/suite/web_search_cached.rs b/codex-rs/core/tests/suite/web_search_cached.rs index 1a69d8b73707..261efaf942b7 100644 --- a/codex-rs/core/tests/suite/web_search_cached.rs +++ b/codex-rs/core/tests/suite/web_search_cached.rs @@ -35,7 +35,7 @@ async fn web_search_mode_cached_sets_external_web_access_false_in_request_body() let mut builder = test_codex() .with_model("gpt-5-codex") .with_config(|config| { - config.web_search_mode = Some(WebSearchMode::Cached); + config.web_search_mode = WebSearchMode::Cached; }); let test = builder .build(&server) @@ -67,7 +67,7 @@ async fn web_search_mode_takes_precedence_over_legacy_flags_in_request_body() { .with_model("gpt-5-codex") .with_config(|config| { config.features.enable(Feature::WebSearchRequest); - config.web_search_mode = Some(WebSearchMode::Cached); + config.web_search_mode = WebSearchMode::Cached; }); let test = builder .build(&server) diff --git a/codex-rs/protocol/src/items.rs b/codex-rs/protocol/src/items.rs index 9d6b484d920e..cb04d427b9d2 100644 --- a/codex-rs/protocol/src/items.rs +++ b/codex-rs/protocol/src/items.rs @@ -4,6 +4,8 @@ use crate::protocol::AgentReasoningRawContentEvent; use crate::protocol::EventMsg; use crate::protocol::UserMessageEvent; use crate::protocol::WebSearchEndEvent; +use crate::user_input::ByteRange; +use crate::user_input::TextElement; use crate::user_input::UserInput; use schemars::JsonSchema; use serde::Deserialize; @@ -62,13 +64,13 @@ impl UserMessageItem { } pub fn as_legacy_event(&self) -> EventMsg { + // Legacy user-message events flatten only text inputs into `message` and + // rebase text element ranges onto that concatenated text. EventMsg::UserMessage(UserMessageEvent { message: self.message(), images: Some(self.image_urls()), - // TODO: Thread text element ranges into legacy user message events. - text_elements: Vec::new(), - // TODO: Thread local image paths into legacy user message events. - local_images: Vec::new(), + local_images: self.local_image_paths(), + text_elements: self.text_elements(), }) } @@ -83,6 +85,32 @@ impl UserMessageItem { .join("") } + pub fn text_elements(&self) -> Vec { + let mut out = Vec::new(); + let mut offset = 0usize; + for input in &self.content { + if let UserInput::Text { + text, + text_elements, + } = input + { + // Text element ranges are relative to each text chunk; offset them so they align + // with the concatenated message returned by `message()`. + for elem in text_elements { + out.push(TextElement { + byte_range: ByteRange { + start: offset + elem.byte_range.start, + end: offset + elem.byte_range.end, + }, + placeholder: elem.placeholder.clone(), + }); + } + offset += text.len(); + } + } + out + } + pub fn image_urls(&self) -> Vec { self.content .iter() @@ -92,6 +120,16 @@ impl UserMessageItem { }) .collect() } + + pub fn local_image_paths(&self) -> Vec { + self.content + .iter() + .filter_map(|c| match c { + UserInput::LocalImage { path } => Some(path.clone()), + _ => None, + }) + .collect() + } } impl AgentMessageItem { diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index d2c9ddd564d4..01424bfc0c3d 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1103,7 +1103,7 @@ mod tests { ResponseInputItem::Message { content, .. } => { assert_eq!(content.len(), 1); match &content[0] { - ContentItem::InputText { text } => { + ContentItem::InputText { text, .. } => { let display_path = missing_path.display().to_string(); assert!( text.contains(&display_path), @@ -1137,7 +1137,7 @@ mod tests { ResponseInputItem::Message { content, .. } => { assert_eq!(content.len(), 1); match &content[0] { - ContentItem::InputText { text } => { + ContentItem::InputText { text, .. } => { assert!( text.contains("unsupported MIME type `application/json`"), "placeholder should mention unsupported MIME: {text}" @@ -1178,7 +1178,7 @@ mod tests { svg_path.display() ); match &content[0] { - ContentItem::InputText { text } => assert_eq!(text, &expected), + ContentItem::InputText { text, .. } => assert_eq!(text, &expected), other => panic!("expected placeholder text but found {other:?}"), } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index df1b523cbc45..6850c2ac1159 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1962,33 +1962,13 @@ pub enum SkillScope { pub struct SkillMetadata { pub name: String, pub description: String, - #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - /// Legacy short_description from SKILL.md. Prefer SKILL.toml interface.short_description. - pub short_description: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub interface: Option, + pub short_description: Option, pub path: PathBuf, pub scope: SkillScope, } -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)] -pub struct SkillInterface { - #[ts(optional)] - pub display_name: Option, - #[ts(optional)] - pub short_description: Option, - #[ts(optional)] - pub icon_small: Option, - #[ts(optional)] - pub icon_large: Option, - #[ts(optional)] - pub brand_color: Option, - #[ts(optional)] - pub default_prompt: Option, -} - #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct SkillErrorInfo { pub path: PathBuf, From e609bbdbb8630e9b2513ed2855aeab5109600160 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 15:54:17 -0800 Subject: [PATCH 2/8] Reduce diff --- .../app-server-protocol/src/protocol/v2.rs | 130 +++++- codex-rs/app-server/README.md | 1 + .../app-server/src/bespoke_event_handling.rs | 175 ++++++++ .../app-server/src/codex_message_processor.rs | 10 + codex-rs/core/src/agent/control.rs | 8 +- codex-rs/core/src/agent/mod.rs | 2 + codex-rs/core/src/client.rs | 20 +- codex-rs/core/src/codex.rs | 44 +- codex-rs/core/src/compact.rs | 4 +- codex-rs/core/src/config/edit.rs | 6 + codex-rs/core/src/config/mod.rs | 124 +++-- codex-rs/core/src/config/types.rs | 24 + .../src/config_loader/config_requirements.rs | 39 +- codex-rs/core/src/config_loader/mod.rs | 1 + codex-rs/core/src/context_manager/history.rs | 2 +- codex-rs/core/src/event_mapping.rs | 2 +- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/mcp_connection_manager.rs | 2 + codex-rs/core/src/rollout/recorder.rs | 63 +++ codex-rs/core/src/skills/loader.rs | 424 ++++++++++++++++-- codex-rs/core/src/skills/model.rs | 11 + codex-rs/core/src/tasks/review.rs | 2 +- codex-rs/core/src/tools/handlers/collab.rs | 57 ++- codex-rs/core/src/tools/spec.rs | 79 ++-- codex-rs/core/src/user_instructions.rs | 8 +- codex-rs/core/src/user_shell_command.rs | 2 +- codex-rs/core/tests/chat_completions_sse.rs | 2 +- codex-rs/core/tests/responses_headers.rs | 63 +++ codex-rs/core/tests/suite/client.rs | 5 +- codex-rs/core/tests/suite/model_tools.rs | 2 +- codex-rs/core/tests/suite/prompt_caching.rs | 2 +- codex-rs/core/tests/suite/review.rs | 4 +- codex-rs/core/tests/suite/rmcp_client.rs | 6 + codex-rs/core/tests/suite/truncation.rs | 3 + .../core/tests/suite/web_search_cached.rs | 4 +- codex-rs/protocol/src/models.rs | 6 +- codex-rs/protocol/src/protocol.rs | 22 +- 37 files changed, 1190 insertions(+), 170 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index e3499951df13..ac930e1f429d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -16,6 +16,7 @@ use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; use codex_protocol::plan_tool::PlanItemArg as CorePlanItemArg; use codex_protocol::plan_tool::StepStatus as CorePlanStepStatus; +use codex_protocol::protocol::AgentStatus as CoreAgentStatus; use codex_protocol::protocol::AskForApproval as CoreAskForApproval; use codex_protocol::protocol::CodexErrorInfo as CoreCodexErrorInfo; use codex_protocol::protocol::CreditsSnapshot as CoreCreditsSnapshot; @@ -24,6 +25,7 @@ use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; +use codex_protocol::protocol::SkillInterface as CoreSkillInterface; use codex_protocol::protocol::SkillMetadata as CoreSkillMetadata; use codex_protocol::protocol::SkillScope as CoreSkillScope; use codex_protocol::protocol::TokenUsage as CoreTokenUsage; @@ -1253,13 +1255,35 @@ pub enum SkillScope { pub struct SkillMetadata { pub name: String, pub description: String, - #[ts(optional)] #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + /// Legacy short_description from SKILL.md. Prefer SKILL.toml interface.short_description. pub short_description: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub interface: Option, pub path: PathBuf, pub scope: SkillScope, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct SkillInterface { + #[ts(optional)] + pub display_name: Option, + #[ts(optional)] + pub short_description: Option, + #[ts(optional)] + pub icon_small: Option, + #[ts(optional)] + pub icon_large: Option, + #[ts(optional)] + pub brand_color: Option, + #[ts(optional)] + pub default_prompt: Option, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1283,12 +1307,26 @@ impl From for SkillMetadata { name: value.name, description: value.description, short_description: value.short_description, + interface: value.interface.map(SkillInterface::from), path: value.path, scope: value.scope.into(), } } } +impl From for SkillInterface { + fn from(value: CoreSkillInterface) -> Self { + Self { + display_name: value.display_name, + short_description: value.short_description, + brand_color: value.brand_color, + default_prompt: value.default_prompt, + icon_small: value.icon_small, + icon_large: value.icon_large, + } + } +} + impl From for SkillScope { fn from(value: CoreSkillScope) -> Self { match value { @@ -1570,6 +1608,7 @@ impl From for CoreByteRange { } } } + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1722,6 +1761,25 @@ pub enum ThreadItem { }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] + CollabAgentToolCall { + /// Unique identifier for this collab tool call. + id: String, + /// Name of the collab tool that was invoked. + tool: CollabAgentTool, + /// Current status of the collab tool call. + status: CollabAgentToolCallStatus, + /// Thread ID of the agent issuing the collab request. + sender_thread_id: String, + /// Thread ID of the receiving agent, when applicable. In case of spawn operation, + /// this correspond to the newly spawned agent. + receiver_thread_id: Option, + /// Prompt text sent as part of the collab tool call, when available. + prompt: Option, + /// Last known status of the target agent, when available. + agent_state: Option, + }, + #[serde(rename_all = "camelCase")] + #[ts(rename_all = "camelCase")] WebSearch { id: String, query: String }, #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] @@ -1774,6 +1832,16 @@ pub enum CommandExecutionStatus { Declined, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub enum CollabAgentTool { + SpawnAgent, + SendInput, + Wait, + CloseAgent, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1812,6 +1880,66 @@ pub enum McpToolCallStatus { Failed, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub enum CollabAgentToolCallStatus { + InProgress, + Completed, + Failed, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub enum CollabAgentStatus { + PendingInit, + Running, + Completed, + Errored, + Shutdown, + NotFound, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct CollabAgentState { + pub status: CollabAgentStatus, + pub message: Option, +} + +impl From for CollabAgentState { + fn from(value: CoreAgentStatus) -> Self { + match value { + CoreAgentStatus::PendingInit => Self { + status: CollabAgentStatus::PendingInit, + message: None, + }, + CoreAgentStatus::Running => Self { + status: CollabAgentStatus::Running, + message: None, + }, + CoreAgentStatus::Completed(message) => Self { + status: CollabAgentStatus::Completed, + message, + }, + CoreAgentStatus::Errored(message) => Self { + status: CollabAgentStatus::Errored, + message: Some(message), + }, + CoreAgentStatus::Shutdown => Self { + status: CollabAgentStatus::Shutdown, + message: None, + }, + CoreAgentStatus::NotFound => Self { + status: CollabAgentStatus::NotFound, + message: None, + }, + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 26c2dee9cf86..63f4dea965d2 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -375,6 +375,7 @@ Today both notifications carry an empty `items` array even when item events were - `commandExecution` — `{id, command, cwd, status, commandActions, aggregatedOutput?, exitCode?, durationMs?}` for sandboxed commands; `status` is `inProgress`, `completed`, `failed`, or `declined`. - `fileChange` — `{id, changes, status}` describing proposed edits; `changes` list `{path, kind, diff}` and `status` is `inProgress`, `completed`, `failed`, or `declined`. - `mcpToolCall` — `{id, server, tool, status, arguments, result?, error?}` describing MCP calls; `status` is `inProgress`, `completed`, or `failed`. +- `collabToolCall` — `{id, tool, status, senderThreadId, receiverThreadId?, newThreadId?, prompt?, agentStatus?}` describing collab tool calls (`spawn_agent`, `send_input`, `wait`, `close_agent`); `status` is `inProgress`, `completed`, or `failed`. - `webSearch` — `{id, query}` for a web search request issued by the agent. - `imageView` — `{id, path}` emitted when the agent invokes the image viewer tool. - `enteredReviewMode` — `{id, review}` sent when the reviewer starts; `review` is a short user-facing label such as `"current changes"` or the requested target description. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 0870191ec8f1..f14dd2ddca48 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -14,6 +14,9 @@ use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; use codex_app_server_protocol::CodexErrorInfo as V2CodexErrorInfo; +use codex_app_server_protocol::CollabAgentState as V2CollabAgentStatus; +use codex_app_server_protocol::CollabAgentTool; +use codex_app_server_protocol::CollabAgentToolCallStatus as V2CollabToolCallStatus; use codex_app_server_protocol::CommandAction as V2ParsedCommand; use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionOutputDeltaNotification; @@ -278,6 +281,178 @@ pub(crate) async fn apply_bespoke_event_handling( .send_server_notification(ServerNotification::ItemCompleted(notification)) .await; } + EventMsg::CollabAgentSpawnBegin(begin_event) => { + let item = ThreadItem::CollabAgentToolCall { + id: begin_event.call_id, + tool: CollabAgentTool::SpawnAgent, + status: V2CollabToolCallStatus::InProgress, + sender_thread_id: begin_event.sender_thread_id.to_string(), + receiver_thread_id: None, + prompt: Some(begin_event.prompt), + agent_state: None, + }; + let notification = ItemStartedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(notification)) + .await; + } + EventMsg::CollabAgentSpawnEnd(end_event) => { + let status = if end_event.new_thread_id.is_some() { + V2CollabToolCallStatus::Completed + } else { + V2CollabToolCallStatus::Failed + }; + let item = ThreadItem::CollabAgentToolCall { + id: end_event.call_id, + tool: CollabAgentTool::SpawnAgent, + status, + sender_thread_id: end_event.sender_thread_id.to_string(), + receiver_thread_id: end_event.new_thread_id.map(|id| id.to_string()), + prompt: Some(end_event.prompt), + agent_state: Some(V2CollabAgentStatus::from(end_event.status)), + }; + let notification = ItemCompletedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemCompleted(notification)) + .await; + } + EventMsg::CollabAgentInteractionBegin(begin_event) => { + let item = ThreadItem::CollabAgentToolCall { + id: begin_event.call_id, + tool: CollabAgentTool::SendInput, + status: V2CollabToolCallStatus::InProgress, + sender_thread_id: begin_event.sender_thread_id.to_string(), + receiver_thread_id: Some(begin_event.receiver_thread_id.to_string()), + prompt: Some(begin_event.prompt), + agent_state: None, + }; + let notification = ItemStartedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(notification)) + .await; + } + EventMsg::CollabAgentInteractionEnd(end_event) => { + let status = match end_event.status { + codex_protocol::protocol::AgentStatus::Errored(_) + | codex_protocol::protocol::AgentStatus::NotFound => V2CollabToolCallStatus::Failed, + _ => V2CollabToolCallStatus::Completed, + }; + let item = ThreadItem::CollabAgentToolCall { + id: end_event.call_id, + tool: CollabAgentTool::SendInput, + status, + sender_thread_id: end_event.sender_thread_id.to_string(), + receiver_thread_id: Some(end_event.receiver_thread_id.to_string()), + prompt: Some(end_event.prompt), + agent_state: Some(V2CollabAgentStatus::from(end_event.status)), + }; + let notification = ItemCompletedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemCompleted(notification)) + .await; + } + EventMsg::CollabWaitingBegin(begin_event) => { + let item = ThreadItem::CollabAgentToolCall { + id: begin_event.call_id, + tool: CollabAgentTool::Wait, + status: V2CollabToolCallStatus::InProgress, + sender_thread_id: begin_event.sender_thread_id.to_string(), + receiver_thread_id: Some(begin_event.receiver_thread_id.to_string()), + prompt: None, + agent_state: None, + }; + let notification = ItemStartedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(notification)) + .await; + } + EventMsg::CollabWaitingEnd(end_event) => { + let status = match end_event.status { + codex_protocol::protocol::AgentStatus::Errored(_) + | codex_protocol::protocol::AgentStatus::NotFound => V2CollabToolCallStatus::Failed, + _ => V2CollabToolCallStatus::Completed, + }; + let item = ThreadItem::CollabAgentToolCall { + id: end_event.call_id, + tool: CollabAgentTool::Wait, + status, + sender_thread_id: end_event.sender_thread_id.to_string(), + receiver_thread_id: Some(end_event.receiver_thread_id.to_string()), + prompt: None, + agent_state: Some(V2CollabAgentStatus::from(end_event.status)), + }; + let notification = ItemCompletedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemCompleted(notification)) + .await; + } + EventMsg::CollabCloseBegin(begin_event) => { + let item = ThreadItem::CollabAgentToolCall { + id: begin_event.call_id, + tool: CollabAgentTool::CloseAgent, + status: V2CollabToolCallStatus::InProgress, + sender_thread_id: begin_event.sender_thread_id.to_string(), + receiver_thread_id: Some(begin_event.receiver_thread_id.to_string()), + prompt: None, + agent_state: None, + }; + let notification = ItemStartedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemStarted(notification)) + .await; + } + EventMsg::CollabCloseEnd(end_event) => { + let status = match end_event.status { + codex_protocol::protocol::AgentStatus::Errored(_) + | codex_protocol::protocol::AgentStatus::NotFound => V2CollabToolCallStatus::Failed, + _ => V2CollabToolCallStatus::Completed, + }; + let item = ThreadItem::CollabAgentToolCall { + id: end_event.call_id, + tool: CollabAgentTool::CloseAgent, + status, + sender_thread_id: end_event.sender_thread_id.to_string(), + receiver_thread_id: Some(end_event.receiver_thread_id.to_string()), + prompt: None, + agent_state: Some(V2CollabAgentStatus::from(end_event.status)), + }; + let notification = ItemCompletedNotification { + thread_id: conversation_id.to_string(), + turn_id: event_turn_id.clone(), + item, + }; + outgoing + .send_server_notification(ServerNotification::ItemCompleted(notification)) + .await; + } EventMsg::AgentMessageContentDelta(event) => { let notification = AgentMessageDeltaNotification { thread_id: conversation_id.to_string(), diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c5d2994f4d67..fc7115447dbf 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -3899,6 +3899,16 @@ fn skills_to_info( name: skill.name.clone(), description: skill.description.clone(), short_description: skill.short_description.clone(), + interface: skill.interface.clone().map(|interface| { + codex_app_server_protocol::SkillInterface { + display_name: interface.display_name, + short_description: interface.short_description, + icon_small: interface.icon_small, + icon_large: interface.icon_large, + brand_color: interface.brand_color, + default_prompt: interface.default_prompt, + } + }), path: skill.path.clone(), scope: skill.scope.into(), }) diff --git a/codex-rs/core/src/agent/control.rs b/codex-rs/core/src/agent/control.rs index bc648f4e50d8..7083081dc62a 100644 --- a/codex-rs/core/src/agent/control.rs +++ b/codex-rs/core/src/agent/control.rs @@ -58,7 +58,7 @@ impl AgentControl { Op::UserInput { items: vec![UserInput::Text { text: prompt, - // Agent control prompts are plain text with no UI text elements. + // Plain text conversion has no UI element ranges. text_elements: Vec::new(), }], final_output_json_schema: None, @@ -71,6 +71,12 @@ impl AgentControl { result } + /// Interrupt the current task for an existing agent thread. + pub(crate) async fn interrupt_agent(&self, agent_id: ThreadId) -> CodexResult { + let state = self.upgrade()?; + state.send_op(agent_id, Op::Interrupt).await + } + /// Submit a shutdown request to an existing agent thread. pub(crate) async fn shutdown_agent(&self, agent_id: ThreadId) -> CodexResult { let state = self.upgrade()?; diff --git a/codex-rs/core/src/agent/mod.rs b/codex-rs/core/src/agent/mod.rs index d6348b38b3e0..0387eda494f6 100644 --- a/codex-rs/core/src/agent/mod.rs +++ b/codex-rs/core/src/agent/mod.rs @@ -1,6 +1,8 @@ pub(crate) mod control; +pub(crate) mod role; pub(crate) mod status; pub(crate) use codex_protocol::protocol::AgentStatus; pub(crate) use control::AgentControl; +pub(crate) use role::AgentRole; pub(crate) use status::agent_status_from_event; diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 438c7c5eeee8..326d81e0e912 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -31,6 +31,7 @@ use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; +use codex_protocol::config_types::WebSearchMode; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; @@ -64,6 +65,8 @@ use crate::model_provider_info::WireApi; use crate::tools::spec::create_tools_json_for_chat_completions_api; use crate::tools::spec::create_tools_json_for_responses_api; +pub const WEB_SEARCH_ELIGIBLE_HEADER: &str = "x-oai-web-search-eligible"; + #[derive(Debug)] struct ModelClientState { config: Arc, @@ -319,7 +322,7 @@ impl ModelClientSession { store_override: None, conversation_id: Some(conversation_id), session_source: Some(self.state.session_source.clone()), - extra_headers: beta_feature_headers(&self.state.config), + extra_headers: build_responses_headers(&self.state.config), compression, } } @@ -635,6 +638,21 @@ fn beta_feature_headers(config: &Config) -> ApiHeaderMap { headers } +fn build_responses_headers(config: &Config) -> ApiHeaderMap { + let mut headers = beta_feature_headers(config); + headers.insert( + WEB_SEARCH_ELIGIBLE_HEADER, + HeaderValue::from_static( + if matches!(config.web_search_mode, Some(WebSearchMode::Disabled)) { + "false" + } else { + "true" + }, + ), + ); + headers +} + fn map_response_stream(api_stream: S, otel_manager: OtelManager) -> ResponseStream where S: futures::Stream> diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b60ae30662cf..c33e12f2a6d4 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -121,6 +121,7 @@ use crate::protocol::ReviewDecision; use crate::protocol::SandboxPolicy; use crate::protocol::SessionConfiguredEvent; use crate::protocol::SkillErrorInfo; +use crate::protocol::SkillInterface as ProtocolSkillInterface; use crate::protocol::SkillMetadata as ProtocolSkillMetadata; use crate::protocol::StreamErrorEvent; use crate::protocol::Submission; @@ -542,24 +543,12 @@ impl Session { web_search_mode: per_turn_config.web_search_mode, }); - let base_instructions = if per_turn_config.features.enabled(Feature::Collab) { - const COLLAB_INSTRUCTIONS: &str = - include_str!("../templates/collab/experimental_prompt.md"); - let base = session_configuration - .base_instructions - .as_deref() - .unwrap_or(model_info.base_instructions.as_str()); - Some(format!("{base}\n\n{COLLAB_INSTRUCTIONS}")) - } else { - session_configuration.base_instructions.clone() - }; - TurnContext { sub_id, client, cwd: session_configuration.cwd.clone(), developer_instructions: session_configuration.developer_instructions.clone(), - base_instructions, + base_instructions: session_configuration.base_instructions.clone(), compact_prompt: session_configuration.compact_prompt.clone(), user_instructions: session_configuration.user_instructions.clone(), approval_policy: session_configuration.approval_policy.value(), @@ -1530,6 +1519,7 @@ impl Session { self.record_conversation_items(turn_context, std::slice::from_ref(&response_item)) .await; + // Derive a turn item and emit lifecycle events if applicable. if let Some(item) = parse_turn_item(&response_item) { self.emit_turn_item_started(turn_context, &item).await; self.emit_turn_item_completed(turn_context, item).await; @@ -2076,10 +2066,13 @@ mod handlers { codex_protocol::approvals::ElicitationAction::Decline => ElicitationAction::Decline, codex_protocol::approvals::ElicitationAction::Cancel => ElicitationAction::Cancel, }; - let response = ElicitationResponse { - action, - content: None, + // When accepting, send an empty object as content to satisfy MCP servers + // that expect non-null content on Accept. For Decline/Cancel, content is None. + let content = match action { + ElicitationAction::Accept => Some(serde_json::json!({})), + ElicitationAction::Decline | ElicitationAction::Cancel => None, }; + let response = ElicitationResponse { action, content }; if let Err(err) = sess .resolve_elicitation(server_name, request_id, response) .await @@ -2260,7 +2253,6 @@ mod handlers { Arc::clone(&turn_context), vec![UserInput::Text { text: turn_context.compact_prompt().to_string(), - // Compaction prompt is synthesized; no UI element ranges to preserve. text_elements: Vec::new(), }], CompactTask, @@ -2417,7 +2409,7 @@ async fn spawn_review_thread( let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &review_model_info, features: &review_features, - web_search_mode: review_web_search_mode, + web_search_mode: Some(review_web_search_mode), }); let base_instructions = REVIEW_PROMPT.to_string(); @@ -2430,7 +2422,7 @@ async fn spawn_review_thread( let mut per_turn_config = (*config).clone(); per_turn_config.model = Some(model.clone()); per_turn_config.features = review_features.clone(); - per_turn_config.web_search_mode = review_web_search_mode; + per_turn_config.web_search_mode = Some(review_web_search_mode); let otel_manager = parent_turn_context .client @@ -2472,7 +2464,6 @@ async fn spawn_review_thread( // Seed the child task with the review prompt as the initial user message. let input: Vec = vec![UserInput::Text { text: review_prompt, - // Review prompt is synthesized; no UI element ranges to preserve. text_elements: Vec::new(), }]; let tc = Arc::new(review_turn_context); @@ -2494,6 +2485,17 @@ fn skills_to_info(skills: &[SkillMetadata]) -> Vec { name: skill.name.clone(), description: skill.description.clone(), short_description: skill.short_description.clone(), + interface: skill + .interface + .clone() + .map(|interface| ProtocolSkillInterface { + display_name: interface.display_name, + short_description: interface.short_description, + icon_small: interface.icon_small, + icon_large: interface.icon_large, + brand_color: interface.brand_color, + default_prompt: interface.default_prompt, + }), path: skill.path.clone(), scope: skill.scope, }) @@ -2565,7 +2567,6 @@ pub(crate) async fn run_turn( let user_message_item = UserMessageItem::new(&input); let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input); let response_item: ResponseItem = initial_input_for_turn.clone().into(); - // Persist the user message to history, but emit the turn item from `UserInput` so // UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry // those spans, and `record_response_item_and_emit_turn_item` would drop them. @@ -2974,7 +2975,6 @@ async fn try_run_turn( should_emit_turn_diff = true; needs_follow_up |= sess.has_pending_input().await; - error!("needs_follow_up: {needs_follow_up}"); break Ok(TurnRunResult { needs_follow_up, diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index f13a5914d59c..4dc56f10d25a 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -46,7 +46,7 @@ pub(crate) async fn run_inline_auto_compact_task( let prompt = turn_context.compact_prompt().to_string(); let input = vec![UserInput::Text { text: prompt, - // Compaction prompt is synthesized; no UI element ranges to preserve. + // Plain text conversion has no UI element ranges. text_elements: Vec::new(), }]; @@ -197,7 +197,7 @@ pub fn content_items_to_text(content: &[ContentItem]) -> Option { let mut pieces = Vec::new(); for item in content { match item { - ContentItem::InputText { text, .. } | ContentItem::OutputText { text } => { + ContentItem::InputText { text } | ContentItem::OutputText { text } => { if !text.is_empty() { pieces.push(text.as_str()); } diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 0def0440a80a..fb6829bb9687 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -1124,6 +1124,7 @@ gpt-5 = "gpt-5.1" cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: Some(vec!["one".to_string(), "two".to_string()]), @@ -1145,6 +1146,7 @@ gpt-5 = "gpt-5.1" env_http_headers: None, }, enabled: false, + disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(5)), tool_timeout_sec: None, enabled_tools: None, @@ -1209,6 +1211,7 @@ foo = { command = "cmd" } cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1252,6 +1255,7 @@ foo = { command = "cmd" } # keep me cwd: None, }, enabled: false, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1294,6 +1298,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1337,6 +1342,7 @@ foo = { command = "cmd" } cwd: None, }, enabled: false, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 5e5d26c791ad..6228bc3ac384 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2,6 +2,7 @@ use crate::auth::AuthCredentialsStoreMode; use crate::config::types::DEFAULT_OTEL_ENVIRONMENT; use crate::config::types::History; use crate::config::types::McpServerConfig; +use crate::config::types::McpServerDisabledReason; use crate::config::types::McpServerTransportConfig; use crate::config::types::Notice; use crate::config::types::Notifications; @@ -19,6 +20,7 @@ use crate::config_loader::ConfigRequirements; use crate::config_loader::LoaderOverrides; use crate::config_loader::McpServerIdentity; use crate::config_loader::McpServerRequirement; +use crate::config_loader::Sourced; use crate::config_loader::load_config_layers_state; use crate::features::Feature; use crate::features::FeatureOverrides; @@ -337,7 +339,8 @@ pub struct Config { /// model info's default preference. pub include_apply_patch_tool: bool, - pub web_search_mode: WebSearchMode, + /// Explicit or feature-derived web search mode. + pub web_search_mode: Option, /// If set to `true`, used only the experimental unified exec tool. pub use_experimental_unified_exec_tool: bool, @@ -539,25 +542,32 @@ fn deserialize_config_toml_with_base( fn filter_mcp_servers_by_requirements( mcp_servers: &mut HashMap, - mcp_requirements: Option<&BTreeMap>, + mcp_requirements: Option<&Sourced>>, ) { let Some(allowlist) = mcp_requirements else { return; }; + let source = allowlist.source.clone(); for (name, server) in mcp_servers.iter_mut() { let allowed = allowlist + .value .get(name) .is_some_and(|requirement| mcp_server_matches_requirement(requirement, server)); - if !allowed { + if allowed { + server.disabled_reason = None; + } else { server.enabled = false; + server.disabled_reason = Some(McpServerDisabledReason::Requirements { + source: source.clone(), + }); } } } fn constrain_mcp_servers( mcp_servers: HashMap, - mcp_requirements: Option<&BTreeMap>, + mcp_requirements: Option<&Sourced>>, ) -> ConstraintResult>> { if mcp_requirements.is_none() { return Ok(Constrained::allow_any(mcp_servers)); @@ -1182,24 +1192,22 @@ pub fn resolve_oss_provider( } } -/// Resolve the web search mode from the config, profile, and features. +/// Resolve the web search mode from explicit config and feature flags. fn resolve_web_search_mode( config_toml: &ConfigToml, config_profile: &ConfigProfile, features: &Features, -) -> WebSearchMode { - // Enum gets precedence over features flags +) -> Option { if let Some(mode) = config_profile.web_search.or(config_toml.web_search) { - return mode; + return Some(mode); } if features.enabled(Feature::WebSearchCached) { - return WebSearchMode::Cached; + return Some(WebSearchMode::Cached); } if features.enabled(Feature::WebSearchRequest) { - return WebSearchMode::Live; + return Some(WebSearchMode::Live); } - // Fall back to default - WebSearchMode::default() + None } impl Config { @@ -1707,6 +1715,7 @@ mod tests { use crate::config::types::HistoryPersistence; use crate::config::types::McpServerTransportConfig; use crate::config::types::Notifications; + use crate::config_loader::RequirementSource; use crate::features::Feature; use super::*; @@ -1728,6 +1737,7 @@ mod tests { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1744,6 +1754,7 @@ mod tests { env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1976,9 +1987,9 @@ trust_level = "trusted" (MATCHED_URL_SERVER.to_string(), http_mcp(GOOD_URL)), (DIFFERENT_NAME_SERVER.to_string(), stdio_mcp("same-cmd")), ]); - filter_mcp_servers_by_requirements( - &mut servers, - Some(&BTreeMap::from([ + let source = RequirementSource::LegacyManagedConfigTomlFromMdm; + let requirements = Sourced::new( + BTreeMap::from([ ( MISMATCHED_URL_SERVER.to_string(), McpServerRequirement { @@ -2011,20 +2022,29 @@ trust_level = "trusted" }, }, ), - ])), + ]), + source.clone(), ); + filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); + let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( servers .iter() - .map(|(name, server)| (name.clone(), server.enabled)) - .collect::>(), + .map(|(name, server)| ( + name.clone(), + (server.enabled, server.disabled_reason.clone()) + )) + .collect::)>>(), HashMap::from([ - (MISMATCHED_URL_SERVER.to_string(), false), - (MISMATCHED_COMMAND_SERVER.to_string(), false), - (MATCHED_URL_SERVER.to_string(), true), - (MATCHED_COMMAND_SERVER.to_string(), true), - (DIFFERENT_NAME_SERVER.to_string(), false), + (MISMATCHED_URL_SERVER.to_string(), (false, reason.clone())), + ( + MISMATCHED_COMMAND_SERVER.to_string(), + (false, reason.clone()), + ), + (MATCHED_URL_SERVER.to_string(), (true, None)), + (MATCHED_COMMAND_SERVER.to_string(), (true, None)), + (DIFFERENT_NAME_SERVER.to_string(), (false, reason)), ]) ); } @@ -2041,11 +2061,14 @@ trust_level = "trusted" assert_eq!( servers .iter() - .map(|(name, server)| (name.clone(), server.enabled)) - .collect::>(), + .map(|(name, server)| ( + name.clone(), + (server.enabled, server.disabled_reason.clone()) + )) + .collect::)>>(), HashMap::from([ - ("server-a".to_string(), true), - ("server-b".to_string(), true), + ("server-a".to_string(), (true, None)), + ("server-b".to_string(), (true, None)), ]) ); } @@ -2057,16 +2080,22 @@ trust_level = "trusted" ("server-b".to_string(), http_mcp("https://example.com/b")), ]); - filter_mcp_servers_by_requirements(&mut servers, Some(&BTreeMap::new())); + let source = RequirementSource::LegacyManagedConfigTomlFromMdm; + let requirements = Sourced::new(BTreeMap::new(), source.clone()); + filter_mcp_servers_by_requirements(&mut servers, Some(&requirements)); + let reason = Some(McpServerDisabledReason::Requirements { source }); assert_eq!( servers .iter() - .map(|(name, server)| (name.clone(), server.enabled)) - .collect::>(), + .map(|(name, server)| ( + name.clone(), + (server.enabled, server.disabled_reason.clone()) + )) + .collect::)>>(), HashMap::from([ - ("server-a".to_string(), false), - ("server-b".to_string(), false), + ("server-a".to_string(), (false, reason.clone())), + ("server-b".to_string(), (false, reason)), ]) ); } @@ -2202,15 +2231,12 @@ trust_level = "trusted" } #[test] - fn web_search_mode_uses_default_if_unset() { + fn web_search_mode_uses_none_if_unset() { let cfg = ConfigToml::default(); let profile = ConfigProfile::default(); let features = Features::with_defaults(); - assert_eq!( - resolve_web_search_mode(&cfg, &profile, &features), - WebSearchMode::default() - ); + assert_eq!(resolve_web_search_mode(&cfg, &profile, &features), None); } #[test] @@ -2225,7 +2251,7 @@ trust_level = "trusted" assert_eq!( resolve_web_search_mode(&cfg, &profile, &features), - WebSearchMode::Live + Some(WebSearchMode::Live) ); } @@ -2241,7 +2267,7 @@ trust_level = "trusted" assert_eq!( resolve_web_search_mode(&cfg, &profile, &features), - WebSearchMode::Disabled + Some(WebSearchMode::Disabled) ); } @@ -2491,6 +2517,7 @@ trust_level = "trusted" cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(3)), tool_timeout_sec: Some(Duration::from_secs(5)), enabled_tools: None, @@ -2644,6 +2671,7 @@ bearer_token = "secret" cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -2712,6 +2740,7 @@ ZIG_VAR = "3" cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -2760,6 +2789,7 @@ ZIG_VAR = "3" cwd: Some(cwd_path.clone()), }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -2806,6 +2836,7 @@ ZIG_VAR = "3" env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -2868,6 +2899,7 @@ startup_timeout_sec = 2.0 )])), }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -2942,6 +2974,7 @@ X-Auth = "DOCS_AUTH" )])), }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -2969,6 +3002,7 @@ X-Auth = "DOCS_AUTH" env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -3034,6 +3068,7 @@ url = "https://example.com/mcp" )])), }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(2)), tool_timeout_sec: None, enabled_tools: None, @@ -3051,6 +3086,7 @@ url = "https://example.com/mcp" cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -3131,6 +3167,7 @@ url = "https://example.com/mcp" cwd: None, }, enabled: false, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -3173,6 +3210,7 @@ url = "https://example.com/mcp" cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: Some(vec!["allowed".to_string()]), @@ -3581,7 +3619,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: WebSearchMode::default(), + web_search_mode: None, use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), @@ -3668,7 +3706,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: WebSearchMode::default(), + web_search_mode: None, use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), @@ -3770,7 +3808,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: WebSearchMode::default(), + web_search_mode: None, use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), @@ -3858,7 +3896,7 @@ model_verbosity = "high" forced_chatgpt_workspace_id: None, forced_login_method: None, include_apply_patch_tool: false, - web_search_mode: WebSearchMode::default(), + web_search_mode: None, use_experimental_unified_exec_tool: false, ghost_snapshot: GhostSnapshotConfig::default(), features: Features::with_defaults(), diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 13b201e84e9a..981f9b541aa8 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -3,11 +3,13 @@ // Note this file should generally be restricted to simple struct/enum // definitions that do not contain business logic. +use crate::config_loader::RequirementSource; pub use codex_protocol::config_types::AltScreenMode; pub use codex_protocol::config_types::WebSearchMode; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::BTreeMap; use std::collections::HashMap; +use std::fmt; use std::path::PathBuf; use std::time::Duration; use wildmatch::WildMatchPattern; @@ -20,6 +22,23 @@ use serde::de::Error as SerdeError; pub const DEFAULT_OTEL_ENVIRONMENT: &str = "dev"; +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum McpServerDisabledReason { + Unknown, + Requirements { source: RequirementSource }, +} + +impl fmt::Display for McpServerDisabledReason { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + McpServerDisabledReason::Unknown => write!(f, "unknown"), + McpServerDisabledReason::Requirements { source } => { + write!(f, "requirements ({source})") + } + } + } +} + #[derive(Serialize, Debug, Clone, PartialEq)] pub struct McpServerConfig { #[serde(flatten)] @@ -29,6 +48,10 @@ pub struct McpServerConfig { #[serde(default = "default_enabled")] pub enabled: bool, + /// Reason this server was disabled after applying requirements. + #[serde(skip)] + pub disabled_reason: Option, + /// Startup timeout in seconds for initializing MCP server & initially listing tools. #[serde( default, @@ -160,6 +183,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { startup_timeout_sec, tool_timeout_sec, enabled, + disabled_reason: None, enabled_tools, disabled_tools, }) diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index 0411b0916b51..a83398c7116a 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -44,7 +44,7 @@ impl fmt::Display for RequirementSource { pub struct ConfigRequirements { pub approval_policy: Constrained, pub sandbox_policy: Constrained, - pub mcp_servers: Option>, + pub mcp_servers: Option>>, } impl Default for ConfigRequirements { @@ -273,7 +273,7 @@ impl TryFrom for ConfigRequirements { Ok(ConfigRequirements { approval_policy, sandbox_policy, - mcp_servers: mcp_servers.map(|sourced| sourced.value), + mcp_servers, }) } } @@ -571,24 +571,27 @@ mod tests { assert_eq!( requirements.mcp_servers, - Some(BTreeMap::from([ - ( - "docs".to_string(), - McpServerRequirement { - identity: McpServerIdentity::Command { - command: "codex-mcp".to_string(), + Some(Sourced::new( + BTreeMap::from([ + ( + "docs".to_string(), + McpServerRequirement { + identity: McpServerIdentity::Command { + command: "codex-mcp".to_string(), + }, }, - }, - ), - ( - "remote".to_string(), - McpServerRequirement { - identity: McpServerIdentity::Url { - url: "https://example.com/mcp".to_string(), + ), + ( + "remote".to_string(), + McpServerRequirement { + identity: McpServerIdentity::Url { + url: "https://example.com/mcp".to_string(), + }, }, - }, - ), - ])) + ), + ]), + RequirementSource::Unknown, + )) ); Ok(()) } diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 7e6d4223ca8d..9a8791eeef9a 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -30,6 +30,7 @@ pub use config_requirements::McpServerIdentity; pub use config_requirements::McpServerRequirement; pub use config_requirements::RequirementSource; pub use config_requirements::SandboxModeRequirement; +pub use config_requirements::Sourced; pub use merge::merge_toml_values; pub(crate) use overrides::build_cli_overrides_layer; pub use state::ConfigLayerEntry; diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 4234365f3ae8..4ee2e252c4d8 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -340,7 +340,7 @@ pub(crate) fn is_user_turn_boundary(item: &ResponseItem) -> bool { for content_item in content { match content_item { - ContentItem::InputText { text, .. } => { + ContentItem::InputText { text } => { if is_session_prefix(text) || is_user_shell_command_text(text) { return false; } diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 8ccd034af99a..6c19c726df31 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -52,7 +52,7 @@ fn parse_user_message(message: &[ContentItem]) -> Option { } content.push(UserInput::Text { text: text.clone(), - // Model input content does not carry UI element ranges. + // Plain text conversion has no UI element ranges. text_elements: Vec::new(), }); } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 4c6183454d90..498c45748fdc 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -111,6 +111,7 @@ mod user_shell_command; pub mod util; pub use apply_patch::CODEX_APPLY_PATCH_ARG1; +pub use client::WEB_SEARCH_ELIGIBLE_HEADER; pub use command_safety::is_dangerous_command; pub use command_safety::is_safe_command; pub use exec_policy::ExecPolicyError; diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 6574437bd030..0d638760f556 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -1171,6 +1171,7 @@ mod tests { env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, @@ -1215,6 +1216,7 @@ mod tests { env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: None, tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index dfafb2911588..7151f95b95d5 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -28,6 +28,7 @@ use super::policy::is_persisted_response_item; use crate::config::Config; use crate::default_client::originator; use crate::git_info::collect_git_info; +use crate::path_utils; use codex_protocol::protocol::InitialHistory; use codex_protocol::protocol::ResumedHistory; use codex_protocol::protocol::RolloutItem; @@ -113,6 +114,35 @@ impl RolloutRecorder { .await } + /// Find the newest recorded thread path, optionally filtering to a matching cwd. + pub async fn find_latest_thread_path( + codex_home: &Path, + allowed_sources: &[SessionSource], + model_providers: Option<&[String]>, + default_provider: &str, + filter_cwd: Option<&Path>, + ) -> std::io::Result> { + let mut cursor: Option = None; + loop { + let page = Self::list_threads( + codex_home, + 25, + cursor.as_ref(), + allowed_sources, + model_providers, + default_provider, + ) + .await?; + if let Some(path) = select_resume_path(&page, filter_cwd) { + return Ok(Some(path)); + } + cursor = page.next_cursor; + if cursor.is_none() { + return Ok(None); + } + } + } + /// Attempt to create a new [`RolloutRecorder`]. If the sessions directory /// cannot be created or the rollout file cannot be opened we return the /// error so the caller can decide whether to disable persistence. @@ -431,3 +461,36 @@ impl JsonlWriter { Ok(()) } } + +fn select_resume_path(page: &ThreadsPage, filter_cwd: Option<&Path>) -> Option { + match filter_cwd { + Some(cwd) => page.items.iter().find_map(|item| { + if session_cwd_matches(&item.head, cwd) { + Some(item.path.clone()) + } else { + None + } + }), + None => page.items.first().map(|item| item.path.clone()), + } +} + +fn session_cwd_matches(head: &[serde_json::Value], cwd: &Path) -> bool { + let Some(session_cwd) = extract_session_cwd(head) else { + return false; + }; + if let (Ok(ca), Ok(cb)) = ( + path_utils::normalize_for_path_comparison(&session_cwd), + path_utils::normalize_for_path_comparison(cwd), + ) { + return ca == cb; + } + session_cwd == cwd +} + +fn extract_session_cwd(head: &[serde_json::Value]) -> Option { + head.iter().find_map(|value| { + let meta_line = serde_json::from_value::(value.clone()).ok()?; + Some(meta_line.meta.cwd) + }) +} diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index a7461dcf97fd..da51fea41eed 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -1,6 +1,7 @@ use crate::config::Config; use crate::config_loader::ConfigLayerStack; use crate::skills::model::SkillError; +use crate::skills::model::SkillInterface; use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; use crate::skills::system::system_cache_root_dir; @@ -13,6 +14,7 @@ use std::collections::VecDeque; use std::error::Error; use std::fmt; use std::fs; +use std::path::Component; use std::path::Path; use std::path::PathBuf; use tracing::error; @@ -31,11 +33,29 @@ struct SkillFrontmatterMetadata { short_description: Option, } +#[derive(Debug, Default, Deserialize)] +struct SkillToml { + #[serde(default)] + interface: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct Interface { + display_name: Option, + short_description: Option, + icon_small: Option, + icon_large: Option, + brand_color: Option, + default_prompt: Option, +} + const SKILLS_FILENAME: &str = "SKILL.md"; +const SKILLS_TOML_FILENAME: &str = "SKILL.toml"; const SKILLS_DIR_NAME: &str = "skills"; const MAX_NAME_LEN: usize = 64; const MAX_DESCRIPTION_LEN: usize = 1024; const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN; +const MAX_DEFAULT_PROMPT_LEN: usize = MAX_DESCRIPTION_LEN; // Traversal depth from the skills root. const MAX_SCAN_DEPTH: usize = 6; const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000; @@ -195,7 +215,7 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil } } - // Follow symlinks for user, admin, and repo skills. System skills are written by Codex itself. + // Follow symlinked directories for user, admin, and repo skills. System skills are written by Codex itself. let follow_symlinks = matches!( scope, SkillScope::Repo | SkillScope::User | SkillScope::Admin @@ -262,20 +282,6 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil continue; } - if metadata.is_file() && file_name == SKILLS_FILENAME { - match parse_skill_file(&path, scope) { - Ok(skill) => outcome.skills.push(skill), - Err(err) => { - if scope != SkillScope::System { - outcome.errors.push(SkillError { - path, - message: err.to_string(), - }); - } - } - } - } - continue; } @@ -336,11 +342,12 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result Option { + // Fail open: optional SKILL.toml metadata should not block loading SKILL.md. + let skill_dir = skill_path.parent()?; + let interface_path = skill_dir.join(SKILLS_TOML_FILENAME); + if !interface_path.exists() { + return None; + } + + let contents = match fs::read_to_string(&interface_path) { + Ok(contents) => contents, + Err(error) => { + tracing::warn!( + "ignoring {path}: failed to read SKILL.toml: {error}", + path = interface_path.display() + ); + return None; + } + }; + let parsed: SkillToml = match toml::from_str(&contents) { + Ok(parsed) => parsed, + Err(error) => { + tracing::warn!( + "ignoring {path}: invalid TOML: {error}", + path = interface_path.display() + ); + return None; + } + }; + let interface = parsed.interface?; + + let interface = SkillInterface { + display_name: resolve_str( + interface.display_name, + MAX_NAME_LEN, + "interface.display_name", + ), + short_description: resolve_str( + interface.short_description, + MAX_SHORT_DESCRIPTION_LEN, + "interface.short_description", + ), + icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small), + icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large), + brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"), + default_prompt: resolve_str( + interface.default_prompt, + MAX_DEFAULT_PROMPT_LEN, + "interface.default_prompt", + ), + }; + let has_fields = interface.display_name.is_some() + || interface.short_description.is_some() + || interface.icon_small.is_some() + || interface.icon_large.is_some() + || interface.brand_color.is_some() + || interface.default_prompt.is_some(); + if has_fields { Some(interface) } else { None } +} + +fn resolve_asset_path( + skill_dir: &Path, + field: &'static str, + path: Option, +) -> Option { + // Icons must be relative paths under the skill's assets/ directory; otherwise return None. + let path = path?; + if path.as_os_str().is_empty() { + return None; + } + + let assets_dir = skill_dir.join("assets"); + if path.is_absolute() { + tracing::warn!( + "ignoring {field}: icon must be a relative assets path (not {})", + assets_dir.display() + ); + return None; + } + + let mut components = path.components().peekable(); + while matches!(components.peek(), Some(Component::CurDir)) { + components.next(); + } + match components.next() { + Some(Component::Normal(component)) if component == "assets" => {} + _ => { + tracing::warn!("ignoring {field}: icon path must be under assets/"); + return None; + } + } + if components.any(|component| matches!(component, Component::ParentDir)) { + tracing::warn!("ignoring {field}: icon path must not contain '..'"); + return None; + } + + Some(skill_dir.join(path)) +} + fn sanitize_single_line(raw: &str) -> String { raw.split_whitespace().collect::>().join(" ") } -fn validate_field( +fn validate_len( value: &str, max_len: usize, field_name: &'static str, @@ -379,6 +485,36 @@ fn validate_field( Ok(()) } +fn resolve_str(value: Option, max_len: usize, field: &'static str) -> Option { + let value = value?; + let value = sanitize_single_line(&value); + if value.is_empty() { + tracing::warn!("ignoring {field}: value is empty"); + return None; + } + if value.chars().count() > max_len { + tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters"); + return None; + } + Some(value) +} + +fn resolve_color_str(value: Option, field: &'static str) -> Option { + let value = value?; + let value = value.trim(); + if value.is_empty() { + tracing::warn!("ignoring {field}: value is empty"); + return None; + } + let mut chars = value.chars(); + if value.len() == 7 && chars.next() == Some('#') && chars.all(|c| c.is_ascii_hexdigit()) { + Some(value.to_string()) + } else { + tracing::warn!("ignoring {field}: expected #RRGGBB, got {value}"); + None + } +} + fn extract_frontmatter(contents: &str) -> Option { let mut lines = contents.lines(); if !matches!(lines.next(), Some(line) if line.trim() == "---") { @@ -528,6 +664,224 @@ mod tests { path } + fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf { + let path = skill_dir.join(SKILLS_TOML_FILENAME); + fs::write(&path, contents).unwrap(); + path + } + + #[tokio::test] + async fn loads_skill_interface_metadata_happy_path() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); + let skill_dir = skill_path.parent().expect("skill dir"); + let normalized_skill_dir = normalized(skill_dir); + + write_skill_interface_at( + skill_dir, + r##" +[interface] +display_name = "UI Skill" +short_description = " short desc " +icon_small = "./assets/small-400px.png" +icon_large = "./assets/large-logo.svg" +brand_color = "#3B82F6" +default_prompt = " default prompt " +"##, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "ui-skill".to_string(), + description: "from toml".to_string(), + short_description: None, + interface: Some(SkillInterface { + display_name: Some("UI Skill".to_string()), + short_description: Some("short desc".to_string()), + icon_small: Some(normalized_skill_dir.join("./assets/small-400px.png")), + icon_large: Some(normalized_skill_dir.join("./assets/large-logo.svg")), + brand_color: Some("#3B82F6".to_string()), + default_prompt: Some("default prompt".to_string()), + }), + path: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + + #[tokio::test] + async fn accepts_icon_paths_under_assets_dir() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); + let skill_dir = skill_path.parent().expect("skill dir"); + let normalized_skill_dir = normalized(skill_dir); + + write_skill_interface_at( + skill_dir, + r#" +[interface] +display_name = "UI Skill" +icon_small = "assets/icon.png" +icon_large = "./assets/logo.svg" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "ui-skill".to_string(), + description: "from toml".to_string(), + short_description: None, + interface: Some(SkillInterface { + display_name: Some("UI Skill".to_string()), + short_description: None, + icon_small: Some(normalized_skill_dir.join("assets/icon.png")), + icon_large: Some(normalized_skill_dir.join("./assets/logo.svg")), + brand_color: None, + default_prompt: None, + }), + path: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + + #[tokio::test] + async fn ignores_invalid_brand_color() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_interface_at( + skill_dir, + r#" +[interface] +brand_color = "blue" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "ui-skill".to_string(), + description: "from toml".to_string(), + short_description: None, + interface: None, + path: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + + #[tokio::test] + async fn ignores_default_prompt_over_max_length() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); + let skill_dir = skill_path.parent().expect("skill dir"); + let normalized_skill_dir = normalized(skill_dir); + let too_long = "x".repeat(MAX_DEFAULT_PROMPT_LEN + 1); + + write_skill_interface_at( + skill_dir, + &format!( + r##" +[interface] +display_name = "UI Skill" +icon_small = "./assets/small-400px.png" +default_prompt = "{too_long}" +"## + ), + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "ui-skill".to_string(), + description: "from toml".to_string(), + short_description: None, + interface: Some(SkillInterface { + display_name: Some("UI Skill".to_string()), + short_description: None, + icon_small: Some(normalized_skill_dir.join("./assets/small-400px.png")), + icon_large: None, + brand_color: None, + default_prompt: None, + }), + path: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + + #[tokio::test] + async fn drops_interface_when_icons_are_invalid() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_interface_at( + skill_dir, + r#" +[interface] +icon_small = "icon.png" +icon_large = "./assets/../logo.svg" +"#, + ); + + let cfg = make_config(&codex_home).await; + let outcome = load_skills(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "ui-skill".to_string(), + description: "from toml".to_string(), + short_description: None, + interface: None, + path: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + #[cfg(unix)] fn symlink_dir(target: &Path, link: &Path) { std::os::unix::fs::symlink(target, link).unwrap(); @@ -563,6 +917,7 @@ mod tests { name: "linked-skill".to_string(), description: "from link".to_string(), short_description: None, + interface: None, path: normalized(&shared_skill_path), scope: SkillScope::User, }] @@ -571,7 +926,7 @@ mod tests { #[tokio::test] #[cfg(unix)] - async fn loads_skills_via_symlinked_skill_file_for_user_scope() { + async fn ignores_symlinked_skill_file_for_user_scope() { let codex_home = tempfile::tempdir().expect("tempdir"); let shared = tempfile::tempdir().expect("tempdir"); @@ -590,16 +945,7 @@ mod tests { "unexpected errors: {:?}", outcome.errors ); - assert_eq!( - outcome.skills, - vec![SkillMetadata { - name: "linked-file-skill".to_string(), - description: "from link".to_string(), - short_description: None, - path: normalized(&shared_skill_path), - scope: SkillScope::User, - }] - ); + assert_eq!(outcome.skills, Vec::new()); } #[tokio::test] @@ -629,6 +975,7 @@ mod tests { name: "cycle-skill".to_string(), description: "still loads".to_string(), short_description: None, + interface: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -662,6 +1009,7 @@ mod tests { name: "admin-linked-skill".to_string(), description: "from link".to_string(), short_description: None, + interface: None, path: normalized(&shared_skill_path), scope: SkillScope::Admin, }] @@ -699,6 +1047,7 @@ mod tests { name: "repo-linked-skill".to_string(), description: "from link".to_string(), short_description: None, + interface: None, path: normalized(&linked_skill_path), scope: SkillScope::Repo, }] @@ -759,6 +1108,7 @@ mod tests { name: "within-depth-skill".to_string(), description: "loads".to_string(), short_description: None, + interface: None, path: normalized(&within_depth_path), scope: SkillScope::User, }] @@ -783,6 +1133,7 @@ mod tests { name: "demo-skill".to_string(), description: "does things carefully".to_string(), short_description: None, + interface: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -811,6 +1162,7 @@ mod tests { name: "demo-skill".to_string(), description: "long description".to_string(), short_description: Some("short summary".to_string()), + interface: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -920,6 +1272,7 @@ mod tests { name: "repo-skill".to_string(), description: "from repo".to_string(), short_description: None, + interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -970,6 +1323,7 @@ mod tests { name: "nested-skill".to_string(), description: "from nested".to_string(), short_description: None, + interface: None, path: normalized(&nested_skill_path), scope: SkillScope::Repo, }, @@ -977,6 +1331,7 @@ mod tests { name: "root-skill".to_string(), description: "from root".to_string(), short_description: None, + interface: None, path: normalized(&root_skill_path), scope: SkillScope::Repo, }, @@ -1013,6 +1368,7 @@ mod tests { name: "local-skill".to_string(), description: "from cwd".to_string(), short_description: None, + interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1050,6 +1406,7 @@ mod tests { name: "dupe-skill".to_string(), description: "from repo".to_string(), short_description: None, + interface: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, }] @@ -1077,6 +1434,7 @@ mod tests { name: "dupe-skill".to_string(), description: "from user".to_string(), short_description: None, + interface: None, path: normalized(&user_skill_path), scope: SkillScope::User, }] @@ -1145,6 +1503,7 @@ mod tests { name: "repo-skill".to_string(), description: "from repo".to_string(), short_description: None, + interface: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1200,6 +1559,7 @@ mod tests { name: "system-skill".to_string(), description: "from system".to_string(), short_description: None, + interface: None, path: normalized(&skill_path), scope: SkillScope::System, }] @@ -1254,6 +1614,7 @@ mod tests { name: "dupe-skill".to_string(), description: "from system".to_string(), short_description: None, + interface: None, path: normalized(&system_skill_path), scope: SkillScope::System, }] @@ -1283,6 +1644,7 @@ mod tests { name: "dupe-skill".to_string(), description: "from user".to_string(), short_description: None, + interface: None, path: normalized(&user_skill_path), scope: SkillScope::User, }] @@ -1321,6 +1683,7 @@ mod tests { name: "dupe-skill".to_string(), description: "from repo".to_string(), short_description: None, + interface: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, }] @@ -1371,6 +1734,7 @@ mod tests { name: "dupe-skill".to_string(), description: "from nested".to_string(), short_description: None, + interface: None, path: expected_path, scope: SkillScope::Repo, }], diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core/src/skills/model.rs index 9063d7a25039..fba00d2711ee 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -7,10 +7,21 @@ pub struct SkillMetadata { pub name: String, pub description: String, pub short_description: Option, + pub interface: Option, pub path: PathBuf, pub scope: SkillScope, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillInterface { + pub display_name: Option, + pub short_description: Option, + pub icon_small: Option, + pub icon_large: Option, + pub brand_color: Option, + pub default_prompt: Option, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillError { pub path: PathBuf, diff --git a/codex-rs/core/src/tasks/review.rs b/codex-rs/core/src/tasks/review.rs index 353189a6be97..9157e922ecf3 100644 --- a/codex-rs/core/src/tasks/review.rs +++ b/codex-rs/core/src/tasks/review.rs @@ -86,7 +86,7 @@ async fn start_review_conversation( let mut sub_agent_config = config.as_ref().clone(); // Carry over review-only feature restrictions so the delegate cannot // re-enable blocked tools (web search, view image). - sub_agent_config.web_search_mode = WebSearchMode::Disabled; + sub_agent_config.web_search_mode = Some(WebSearchMode::Disabled); // Set explicit review rubric for the sub-agent sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string()); diff --git a/codex-rs/core/src/tools/handlers/collab.rs b/codex-rs/core/src/tools/handlers/collab.rs index 805af76b3aea..df3f2901dac9 100644 --- a/codex-rs/core/src/tools/handlers/collab.rs +++ b/codex-rs/core/src/tools/handlers/collab.rs @@ -76,11 +76,13 @@ impl ToolHandler for CollabHandler { mod spawn { use super::*; + use crate::agent::AgentRole; use std::sync::Arc; #[derive(Debug, Deserialize)] struct SpawnAgentArgs { message: String, + agent_type: Option, } #[derive(Debug, Serialize)] @@ -95,6 +97,7 @@ mod spawn { arguments: String, ) -> Result { let args: SpawnAgentArgs = parse_arguments(&arguments)?; + let agent_role = args.agent_type.unwrap_or(AgentRole::Default); let prompt = args.message; if prompt.trim().is_empty() { return Err(FunctionCallError::RespondToModel( @@ -112,7 +115,10 @@ mod spawn { .into(), ) .await; - let config = build_agent_spawn_config(turn.as_ref())?; + let mut config = build_agent_spawn_config(turn.as_ref())?; + agent_role + .apply_to_config(&mut config) + .map_err(FunctionCallError::RespondToModel)?; let result = session .services .agent_control @@ -164,6 +170,8 @@ mod send_input { struct SendInputArgs { id: String, message: String, + #[serde(default)] + interrupt: bool, } #[derive(Debug, Serialize)] @@ -185,6 +193,14 @@ mod send_input { "Empty message can't be sent to an agent".to_string(), )); } + if args.interrupt { + session + .services + .agent_control + .interrupt_agent(receiver_thread_id) + .await + .map_err(|err| collab_agent_error(receiver_thread_id, err))?; + } session .send_event( &turn, @@ -717,6 +733,45 @@ mod tests { ); } + #[tokio::test] + async fn send_input_interrupts_before_prompt() { + let (mut session, turn) = make_session_and_context().await; + let manager = thread_manager(); + session.services.agent_control = manager.agent_control(); + let config = turn.client.config().as_ref().clone(); + let thread = manager.start_thread(config).await.expect("start thread"); + let agent_id = thread.thread_id; + let invocation = invocation( + Arc::new(session), + Arc::new(turn), + "send_input", + function_payload(json!({ + "id": agent_id.to_string(), + "message": "hi", + "interrupt": true + })), + ); + CollabHandler + .handle(invocation) + .await + .expect("send_input should succeed"); + + let ops = manager.captured_ops(); + let ops_for_agent: Vec<&Op> = ops + .iter() + .filter_map(|(id, op)| (*id == agent_id).then_some(op)) + .collect(); + assert_eq!(ops_for_agent.len(), 2); + assert!(matches!(ops_for_agent[0], Op::Interrupt)); + assert!(matches!(ops_for_agent[1], Op::UserInput { .. })); + + let _ = thread + .thread + .submit(Op::Shutdown {}) + .await + .expect("shutdown should submit"); + } + #[tokio::test] async fn wait_rejects_non_positive_timeout() { let (session, turn) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 28fc3a170174..5a35f2b6481c 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -1,3 +1,4 @@ +use crate::agent::AgentRole; use crate::client_common::tools::ResponsesApiTool; use crate::client_common::tools::ToolSpec; use crate::features::Feature; @@ -24,7 +25,7 @@ use std::collections::HashMap; pub(crate) struct ToolsConfig { pub shell_type: ConfigShellToolType, pub apply_patch_tool_type: Option, - pub web_search_mode: WebSearchMode, + pub web_search_mode: Option, pub collab_tools: bool, pub experimental_supported_tools: Vec, } @@ -32,7 +33,7 @@ pub(crate) struct ToolsConfig { pub(crate) struct ToolsConfigParams<'a> { pub(crate) model_info: &'a ModelInfo, pub(crate) features: &'a Features, - pub(crate) web_search_mode: WebSearchMode, + pub(crate) web_search_mode: Option, } impl ToolsConfig { @@ -441,6 +442,15 @@ fn create_spawn_agent_tool() -> ToolSpec { description: Some("Initial message to send to the new agent.".to_string()), }, ); + properties.insert( + "agent_type".to_string(), + JsonSchema::String { + description: Some(format!( + "Optional agent type to spawn ({}).", + AgentRole::enum_values().join(", ") + )), + }, + ); ToolSpec::Function(ResponsesApiTool { name: "spawn_agent".to_string(), @@ -468,6 +478,15 @@ fn create_send_input_tool() -> ToolSpec { description: Some("Message to send to the agent.".to_string()), }, ); + properties.insert( + "interrupt".to_string(), + JsonSchema::Boolean { + description: Some( + "When true, interrupt the agent's current task before sending the message. When false (default), the message will be processed when the agent is done on its current task." + .to_string(), + ), + }, + ); ToolSpec::Function(ResponsesApiTool { name: "send_input".to_string(), @@ -1225,17 +1244,17 @@ pub(crate) fn build_specs( } match config.web_search_mode { - WebSearchMode::Disabled => {} - WebSearchMode::Cached => { + Some(WebSearchMode::Cached) => { builder.push_spec(ToolSpec::WebSearch { external_web_access: Some(false), }); } - WebSearchMode::Live => { + Some(WebSearchMode::Live) => { builder.push_spec(ToolSpec::WebSearch { external_web_access: Some(true), }); } + Some(WebSearchMode::Disabled) | None => {} } builder.push_spec_with_parallel_support(create_view_image_tool(), true); @@ -1379,7 +1398,7 @@ mod tests { let config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Live, + web_search_mode: Some(WebSearchMode::Live), }); let (tools, _) = build_specs(&config, None).build(); @@ -1441,7 +1460,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs(&tools_config, None).build(); assert_contains_tool_names( @@ -1453,7 +1472,7 @@ mod tests { fn assert_model_tools( model_slug: &str, features: &Features, - web_search_mode: WebSearchMode, + web_search_mode: Option, expected_tools: &[&str], ) { let config = test_config(); @@ -1477,7 +1496,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1499,7 +1518,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Live, + web_search_mode: Some(WebSearchMode::Live), }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1517,7 +1536,7 @@ mod tests { assert_model_tools( "gpt-5-codex", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "shell_command", "list_mcp_resources", @@ -1536,7 +1555,7 @@ mod tests { assert_model_tools( "gpt-5.1-codex", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "shell_command", "list_mcp_resources", @@ -1555,7 +1574,7 @@ mod tests { assert_model_tools( "gpt-5-codex", Features::with_defaults().enable(Feature::UnifiedExec), - WebSearchMode::Live, + Some(WebSearchMode::Live), &[ "exec_command", "write_stdin", @@ -1575,7 +1594,7 @@ mod tests { assert_model_tools( "gpt-5.1-codex", Features::with_defaults().enable(Feature::UnifiedExec), - WebSearchMode::Live, + Some(WebSearchMode::Live), &[ "exec_command", "write_stdin", @@ -1595,7 +1614,7 @@ mod tests { assert_model_tools( "codex-mini-latest", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "local_shell", "list_mcp_resources", @@ -1613,7 +1632,7 @@ mod tests { assert_model_tools( "gpt-5.1-codex-mini", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "shell_command", "list_mcp_resources", @@ -1632,7 +1651,7 @@ mod tests { assert_model_tools( "gpt-5", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "shell", "list_mcp_resources", @@ -1650,7 +1669,7 @@ mod tests { assert_model_tools( "gpt-5.1", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "shell_command", "list_mcp_resources", @@ -1669,7 +1688,7 @@ mod tests { assert_model_tools( "exp-5.1", &Features::with_defaults(), - WebSearchMode::Cached, + Some(WebSearchMode::Cached), &[ "exec_command", "write_stdin", @@ -1689,7 +1708,7 @@ mod tests { assert_model_tools( "codex-mini-latest", Features::with_defaults().enable(Feature::UnifiedExec), - WebSearchMode::Live, + Some(WebSearchMode::Live), &[ "exec_command", "write_stdin", @@ -1712,7 +1731,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Live, + web_search_mode: Some(WebSearchMode::Live), }); let (tools, _) = build_specs(&tools_config, Some(HashMap::new())).build(); @@ -1734,7 +1753,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1753,7 +1772,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs(&tools_config, None).build(); @@ -1784,7 +1803,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Live, + web_search_mode: Some(WebSearchMode::Live), }); let (tools, _) = build_specs( &tools_config, @@ -1879,7 +1898,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); // Intentionally construct a map with keys that would sort alphabetically. @@ -1956,7 +1975,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs( @@ -2013,7 +2032,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs( @@ -2067,7 +2086,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs( @@ -2123,7 +2142,7 @@ mod tests { let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs( @@ -2235,7 +2254,7 @@ Examples of valid command strings: let tools_config = ToolsConfig::new(&ToolsConfigParams { model_info: &model_info, features: &features, - web_search_mode: WebSearchMode::Cached, + web_search_mode: Some(WebSearchMode::Cached), }); let (tools, _) = build_specs( &tools_config, diff --git a/codex-rs/core/src/user_instructions.rs b/codex-rs/core/src/user_instructions.rs index eb7d520e997a..9c563c29c5b0 100644 --- a/codex-rs/core/src/user_instructions.rs +++ b/codex-rs/core/src/user_instructions.rs @@ -17,7 +17,7 @@ pub(crate) struct UserInstructions { impl UserInstructions { pub fn is_user_instructions(message: &[ContentItem]) -> bool { - if let [ContentItem::InputText { text, .. }] = message { + if let [ContentItem::InputText { text }] = message { text.starts_with(USER_INSTRUCTIONS_PREFIX) || text.starts_with(USER_INSTRUCTIONS_OPEN_TAG_LEGACY) } else { @@ -52,7 +52,7 @@ pub(crate) struct SkillInstructions { impl SkillInstructions { pub fn is_skill_instructions(message: &[ContentItem]) -> bool { - if let [ContentItem::InputText { text, .. }] = message { + if let [ContentItem::InputText { text }] = message { text.starts_with(SKILL_INSTRUCTIONS_PREFIX) } else { false @@ -94,7 +94,7 @@ mod tests { assert_eq!(role, "user"); - let [ContentItem::InputText { text, .. }] = content.as_slice() else { + let [ContentItem::InputText { text }] = content.as_slice() else { panic!("expected one InputText content item"); }; @@ -138,7 +138,7 @@ mod tests { assert_eq!(role, "user"); - let [ContentItem::InputText { text, .. }] = content.as_slice() else { + let [ContentItem::InputText { text }] = content.as_slice() else { panic!("expected one InputText content item"); }; diff --git a/codex-rs/core/src/user_shell_command.rs b/codex-rs/core/src/user_shell_command.rs index e8f6ab7e9a7f..fb8efcc09ca5 100644 --- a/codex-rs/core/src/user_shell_command.rs +++ b/codex-rs/core/src/user_shell_command.rs @@ -95,7 +95,7 @@ mod tests { let ResponseItem::Message { content, .. } = item else { panic!("expected message"); }; - let [ContentItem::InputText { text, .. }] = content.as_slice() else { + let [ContentItem::InputText { text }] = content.as_slice() else { panic!("expected input text"); }; assert_eq!( diff --git a/codex-rs/core/tests/chat_completions_sse.rs b/codex-rs/core/tests/chat_completions_sse.rs index 115acf889fd4..f6d7eb24fed6 100644 --- a/codex-rs/core/tests/chat_completions_sse.rs +++ b/codex-rs/core/tests/chat_completions_sse.rs @@ -129,7 +129,7 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec { fn assert_message(item: &ResponseItem, expected: &str) { if let ResponseItem::Message { content, .. } = item { let text = content.iter().find_map(|part| match part { - ContentItem::OutputText { text } | ContentItem::InputText { text, .. } => Some(text), + ContentItem::OutputText { text } | ContentItem::InputText { text } => Some(text), _ => None, }); let Some(text) = text else { diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 8be6e3634051..8eba3eabe71d 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -9,15 +9,18 @@ use codex_core::ModelProviderInfo; use codex_core::Prompt; use codex_core::ResponseEvent; use codex_core::ResponseItem; +use codex_core::WEB_SEARCH_ELIGIBLE_HEADER; use codex_core::WireApi; use codex_core::models_manager::manager::ModelsManager; use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::config_types::WebSearchMode; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; use core_test_support::load_default_config_for_test; use core_test_support::responses; +use core_test_support::test_codex::test_codex; use futures::StreamExt; use tempfile::TempDir; use wiremock::matchers::header; @@ -213,6 +216,66 @@ async fn responses_stream_includes_subagent_header_on_other() { ); } +#[tokio::test] +async fn responses_stream_includes_web_search_eligible_header_true_by_default() { + core_test_support::skip_if_no_network!(); + + let server = responses::start_mock_server().await; + let response_body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_completed("resp-1"), + ]); + + let request_recorder = responses::mount_sse_once_match( + &server, + header(WEB_SEARCH_ELIGIBLE_HEADER, "true"), + response_body, + ) + .await; + + let test = test_codex().build(&server).await.expect("build test codex"); + test.submit_turn("hello").await.expect("submit test prompt"); + + let request = request_recorder.single_request(); + assert_eq!( + request.header(WEB_SEARCH_ELIGIBLE_HEADER).as_deref(), + Some("true") + ); +} + +#[tokio::test] +async fn responses_stream_includes_web_search_eligible_header_false_when_disabled() { + core_test_support::skip_if_no_network!(); + + let server = responses::start_mock_server().await; + let response_body = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_completed("resp-1"), + ]); + + let request_recorder = responses::mount_sse_once_match( + &server, + header(WEB_SEARCH_ELIGIBLE_HEADER, "false"), + response_body, + ) + .await; + + let test = test_codex() + .with_config(|config| { + config.web_search_mode = Some(WebSearchMode::Disabled); + }) + .build(&server) + .await + .expect("build test codex"); + test.submit_turn("hello").await.expect("submit test prompt"); + + let request = request_recorder.single_request(); + assert_eq!( + request.header(WEB_SEARCH_ELIGIBLE_HEADER).as_deref(), + Some("false") + ); +} + #[tokio::test] async fn responses_respects_model_info_overrides_from_config() { core_test_support::skip_if_no_network!(); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 18d3caf3bfcb..6f659e1b6497 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -14,6 +14,7 @@ use codex_core::ThreadManager; use codex_core::WireApi; use codex_core::auth::AuthCredentialsStoreMode; use codex_core::built_in_model_providers; +use codex_core::default_client::originator; use codex_core::error::CodexErr; use codex_core::models_manager::manager::ModelsManager; use codex_core::protocol::EventMsg; @@ -406,7 +407,7 @@ async fn includes_conversation_id_and_model_headers_in_request() { let request_originator = request.header("originator").expect("originator header"); assert_eq!(request_session_id, session_id.to_string()); - assert_eq!(request_originator, "codex_cli_rs"); + assert_eq!(request_originator, originator().value); assert_eq!(request_authorization, "Bearer Test API Key"); } @@ -522,7 +523,7 @@ async fn chatgpt_auth_sends_correct_request() { let session_id = request.header("session_id").expect("session_id header"); assert_eq!(session_id, thread_id.to_string()); - assert_eq!(request_originator, "codex_cli_rs"); + assert_eq!(request_originator, originator().value); assert_eq!(request_authorization, "Bearer Access Token"); assert_eq!(request_chatgpt_account_id, "account_id"); assert!(request_body["stream"].as_bool().unwrap()); diff --git a/codex-rs/core/tests/suite/model_tools.rs b/codex-rs/core/tests/suite/model_tools.rs index 4eaf3735a37c..8a4d0a77176c 100644 --- a/codex-rs/core/tests/suite/model_tools.rs +++ b/codex-rs/core/tests/suite/model_tools.rs @@ -36,7 +36,7 @@ async fn collect_tool_identifiers_for_model(model: &str) -> Vec { let mut builder = test_codex() .with_model(model) // Keep tool expectations stable when the default web_search mode changes. - .with_config(|config| config.web_search_mode = WebSearchMode::Cached); + .with_config(|config| config.web_search_mode = Some(WebSearchMode::Cached)); let test = builder .build(&server) .await diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index b3ed78f69db4..6ea5619bfe59 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -88,7 +88,7 @@ async fn prompt_tools_are_consistent_across_requests() -> anyhow::Result<()> { config.user_instructions = Some("be consistent and helpful".to_string()); config.model = Some("gpt-5.1-codex-max".to_string()); // Keep tool expectations stable when the default web_search mode changes. - config.web_search_mode = WebSearchMode::Cached; + config.web_search_mode = Some(WebSearchMode::Cached); }) .build(&server) .await?; diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index b0ed8db07feb..d182af82558c 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -137,7 +137,7 @@ async fn review_op_emits_lifecycle_and_review_output() { if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rl.item { if role == "user" { for c in content { - if let ContentItem::InputText { text, .. } = c { + if let ContentItem::InputText { text } = c { if text.contains("full review output from reviewer model") { saw_header = true; } @@ -639,7 +639,7 @@ async fn review_input_isolated_from_parent_history() { && role == "user" { for c in content { - if let ContentItem::InputText { text, .. } = c + if let ContentItem::InputText { text } = c && text.contains("User initiated a review task, but was interrupted.") { saw_interruption_message = true; diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 5001eead3234..5abc39264406 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -88,6 +88,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -225,6 +226,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -420,6 +422,7 @@ async fn stdio_image_completions_round_trip() -> anyhow::Result<()> { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -563,6 +566,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -717,6 +721,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -903,6 +908,7 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { env_http_headers: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index a86489bdd05b..4ae095517c1b 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -426,6 +426,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -517,6 +518,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, @@ -777,6 +779,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { cwd: None, }, enabled: true, + disabled_reason: None, startup_timeout_sec: Some(std::time::Duration::from_secs(10)), tool_timeout_sec: None, enabled_tools: None, diff --git a/codex-rs/core/tests/suite/web_search_cached.rs b/codex-rs/core/tests/suite/web_search_cached.rs index 261efaf942b7..1a69d8b73707 100644 --- a/codex-rs/core/tests/suite/web_search_cached.rs +++ b/codex-rs/core/tests/suite/web_search_cached.rs @@ -35,7 +35,7 @@ async fn web_search_mode_cached_sets_external_web_access_false_in_request_body() let mut builder = test_codex() .with_model("gpt-5-codex") .with_config(|config| { - config.web_search_mode = WebSearchMode::Cached; + config.web_search_mode = Some(WebSearchMode::Cached); }); let test = builder .build(&server) @@ -67,7 +67,7 @@ async fn web_search_mode_takes_precedence_over_legacy_flags_in_request_body() { .with_model("gpt-5-codex") .with_config(|config| { config.features.enable(Feature::WebSearchRequest); - config.web_search_mode = WebSearchMode::Cached; + config.web_search_mode = Some(WebSearchMode::Cached); }); let test = builder .build(&server) diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 01424bfc0c3d..d2c9ddd564d4 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1103,7 +1103,7 @@ mod tests { ResponseInputItem::Message { content, .. } => { assert_eq!(content.len(), 1); match &content[0] { - ContentItem::InputText { text, .. } => { + ContentItem::InputText { text } => { let display_path = missing_path.display().to_string(); assert!( text.contains(&display_path), @@ -1137,7 +1137,7 @@ mod tests { ResponseInputItem::Message { content, .. } => { assert_eq!(content.len(), 1); match &content[0] { - ContentItem::InputText { text, .. } => { + ContentItem::InputText { text } => { assert!( text.contains("unsupported MIME type `application/json`"), "placeholder should mention unsupported MIME: {text}" @@ -1178,7 +1178,7 @@ mod tests { svg_path.display() ); match &content[0] { - ContentItem::InputText { text, .. } => assert_eq!(text, &expected), + ContentItem::InputText { text } => assert_eq!(text, &expected), other => panic!("expected placeholder text but found {other:?}"), } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 6850c2ac1159..df1b523cbc45 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1962,13 +1962,33 @@ pub enum SkillScope { pub struct SkillMetadata { pub name: String, pub description: String, - #[ts(optional)] #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + /// Legacy short_description from SKILL.md. Prefer SKILL.toml interface.short_description. pub short_description: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub interface: Option, pub path: PathBuf, pub scope: SkillScope, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS, PartialEq, Eq)] +pub struct SkillInterface { + #[ts(optional)] + pub display_name: Option, + #[ts(optional)] + pub short_description: Option, + #[ts(optional)] + pub icon_small: Option, + #[ts(optional)] + pub icon_large: Option, + #[ts(optional)] + pub brand_color: Option, + #[ts(optional)] + pub default_prompt: Option, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct SkillErrorInfo { pub path: PathBuf, From 6f1e875d3fa7e98e036eba473988ff7a0cc34246 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 15:57:34 -0800 Subject: [PATCH 3/8] Fix test --- codex-rs/app-server-test-client/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/app-server-test-client/src/main.rs b/codex-rs/app-server-test-client/src/main.rs index 809c2b5a3385..9df895a70a85 100644 --- a/codex-rs/app-server-test-client/src/main.rs +++ b/codex-rs/app-server-test-client/src/main.rs @@ -477,6 +477,7 @@ impl CodexClient { conversation_id: *conversation_id, items: vec![InputItem::Text { text: message.to_string(), + text_elements: Vec::new(), }], }, }; From 450582dfa81833cee76b4ccb0682609e0761282c Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 16:44:12 -0800 Subject: [PATCH 4/8] record_user_prompt_and_emit_turn_item helper function --- codex-rs/core/src/codex.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index c33e12f2a6d4..aef688652b2c 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1526,6 +1526,22 @@ impl Session { } } + pub(crate) async fn record_user_prompt_and_emit_turn_item( + &self, + turn_context: &TurnContext, + input: &[UserInput], + response_item: ResponseItem, + ) { + // Persist the user message to history, but emit the turn item from `UserInput` so + // UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry + // those spans, and `record_response_item_and_emit_turn_item` would drop them. + self.record_conversation_items(turn_context, std::slice::from_ref(&response_item)) + .await; + let turn_item = TurnItem::UserMessage(UserMessageItem::new(input)); + self.emit_turn_item_started(turn_context, &turn_item).await; + self.emit_turn_item_completed(turn_context, turn_item).await; + } + pub(crate) async fn notify_background_event( &self, turn_context: &TurnContext, @@ -2564,18 +2580,9 @@ pub(crate) async fn run_turn( .await; } - let user_message_item = UserMessageItem::new(&input); let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input); let response_item: ResponseItem = initial_input_for_turn.clone().into(); - // Persist the user message to history, but emit the turn item from `UserInput` so - // UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry - // those spans, and `record_response_item_and_emit_turn_item` would drop them. - sess.record_conversation_items(turn_context.as_ref(), &[response_item]) - .await; - let turn_item = TurnItem::UserMessage(user_message_item); - sess.emit_turn_item_started(turn_context.as_ref(), &turn_item) - .await; - sess.emit_turn_item_completed(turn_context.as_ref(), turn_item) + sess.record_user_prompt_and_emit_turn_item(turn_context.as_ref(), &input, response_item) .await; if !skill_items.is_empty() { From af486c19b66f936f2077d64f03ad6721c0ca5347 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 16:47:35 -0800 Subject: [PATCH 5/8] Add tests --- codex-rs/core/tests/suite/items.rs | 40 ++++++++++++++++------------- codex-rs/core/tests/suite/resume.rs | 11 +++++++- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/codex-rs/core/tests/suite/items.rs b/codex-rs/core/tests/suite/items.rs index ce9b0f13407a..20739ab0dbed 100644 --- a/codex-rs/core/tests/suite/items.rs +++ b/codex-rs/core/tests/suite/items.rs @@ -6,6 +6,8 @@ use codex_core::protocol::ItemCompletedEvent; use codex_core::protocol::ItemStartedEvent; use codex_core::protocol::Op; use codex_protocol::items::TurnItem; +use codex_protocol::user_input::ByteRange; +use codex_protocol::user_input::TextElement; use codex_protocol::user_input::UserInput; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -38,12 +40,18 @@ async fn user_message_item_is_emitted() -> anyhow::Result<()> { let first_response = sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]); mount_sse_once(&server, first_response).await; + let text_elements = vec![TextElement { + byte_range: ByteRange { start: 0, end: 6 }, + placeholder: Some("".into()), + }]; + let expected_input = UserInput::Text { + text: "please inspect sample.txt".into(), + text_elements: text_elements.clone(), + }; + codex .submit(Op::UserInput { - items: (vec![UserInput::Text { - text: "please inspect sample.txt".into(), - text_elements: Vec::new(), - }]), + items: vec![expected_input.clone()], final_output_json_schema: None, }) .await?; @@ -66,20 +74,16 @@ async fn user_message_item_is_emitted() -> anyhow::Result<()> { .await; assert_eq!(started_item.id, completed_item.id); - assert_eq!( - started_item.content, - vec![UserInput::Text { - text: "please inspect sample.txt".into(), - text_elements: Vec::new(), - }] - ); - assert_eq!( - completed_item.content, - vec![UserInput::Text { - text: "please inspect sample.txt".into(), - text_elements: Vec::new(), - }] - ); + assert_eq!(started_item.content, vec![expected_input.clone()]); + assert_eq!(completed_item.content, vec![expected_input]); + + let legacy_message = wait_for_event_match(&codex, |ev| match ev { + EventMsg::UserMessage(event) => Some(event.clone()), + _ => None, + }) + .await; + assert_eq!(legacy_message.message, "please inspect sample.txt"); + assert_eq!(legacy_message.text_elements, text_elements); Ok(()) } diff --git a/codex-rs/core/tests/suite/resume.rs b/codex-rs/core/tests/suite/resume.rs index 8424146d65b3..8912870e5eb0 100644 --- a/codex-rs/core/tests/suite/resume.rs +++ b/codex-rs/core/tests/suite/resume.rs @@ -1,6 +1,8 @@ use anyhow::Result; use codex_core::protocol::EventMsg; use codex_core::protocol::Op; +use codex_protocol::user_input::ByteRange; +use codex_protocol::user_input::TextElement; use codex_protocol::user_input::UserInput; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -12,6 +14,7 @@ use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; use std::sync::Arc; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -32,11 +35,16 @@ async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> { ]); mount_sse_once(&server, initial_sse).await; + let text_elements = vec![TextElement { + byte_range: ByteRange { start: 0, end: 6 }, + placeholder: Some("".into()), + }]; + codex .submit(Op::UserInput { items: vec![UserInput::Text { text: "Record some messages".into(), - text_elements: Vec::new(), + text_elements: text_elements.clone(), }], final_output_json_schema: None, }) @@ -57,6 +65,7 @@ async fn resume_includes_initial_messages_from_rollout_events() -> Result<()> { EventMsg::TokenCount(_), ] => { assert_eq!(first_user.message, "Record some messages"); + assert_eq!(first_user.text_elements, text_elements); assert_eq!(assistant_message.message, "Completed first turn"); } other => panic!("unexpected initial messages after resume: {other:#?}"), From 830536213604c1cc2e70fbb9435b25e7f6d298bb Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 16:53:04 -0800 Subject: [PATCH 6/8] Lint --- codex-rs/core/src/codex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index aef688652b2c..06000a345c71 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -2580,7 +2580,7 @@ pub(crate) async fn run_turn( .await; } - let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input); + let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input.clone()); let response_item: ResponseItem = initial_input_for_turn.clone().into(); sess.record_user_prompt_and_emit_turn_item(turn_context.as_ref(), &input, response_item) .await; From 3be63df9cfba99a7b741c45aec8e9ce03f1c256b Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 17:05:45 -0800 Subject: [PATCH 7/8] Add some tests to app-server --- codex-rs/app-server/tests/common/lib.rs | 1 + codex-rs/app-server/tests/common/rollout.rs | 72 ++++++++++++++++ .../tests/suite/v2/thread_resume.rs | 16 +++- .../app-server/tests/suite/v2/turn_start.rs | 83 +++++++++++++++++++ 4 files changed, 170 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server/tests/common/lib.rs b/codex-rs/app-server/tests/common/lib.rs index af4982b846bb..48577db110ca 100644 --- a/codex-rs/app-server/tests/common/lib.rs +++ b/codex-rs/app-server/tests/common/lib.rs @@ -29,6 +29,7 @@ pub use responses::create_exec_command_sse_response; pub use responses::create_final_assistant_message_sse_response; pub use responses::create_shell_command_sse_response; pub use rollout::create_fake_rollout; +pub use rollout::create_fake_rollout_with_text_elements; use serde::de::DeserializeOwned; pub fn to_response(response: JSONRPCResponse) -> anyhow::Result { diff --git a/codex-rs/app-server/tests/common/rollout.rs b/codex-rs/app-server/tests/common/rollout.rs index b5829716af60..784b79b98d71 100644 --- a/codex-rs/app-server/tests/common/rollout.rs +++ b/codex-rs/app-server/tests/common/rollout.rs @@ -87,3 +87,75 @@ pub fn create_fake_rollout( fs::write(file_path, lines.join("\n") + "\n")?; Ok(uuid_str) } + +pub fn create_fake_rollout_with_text_elements( + codex_home: &Path, + filename_ts: &str, + meta_rfc3339: &str, + preview: &str, + text_elements: Vec, + model_provider: Option<&str>, + git_info: Option, +) -> Result { + let uuid = Uuid::new_v4(); + let uuid_str = uuid.to_string(); + let conversation_id = ThreadId::from_string(&uuid_str)?; + + // sessions/YYYY/MM/DD derived from filename_ts (YYYY-MM-DDThh-mm-ss) + let year = &filename_ts[0..4]; + let month = &filename_ts[5..7]; + let day = &filename_ts[8..10]; + let dir = codex_home.join("sessions").join(year).join(month).join(day); + fs::create_dir_all(&dir)?; + + let file_path = dir.join(format!("rollout-{filename_ts}-{uuid}.jsonl")); + + // Build JSONL lines + let meta = SessionMeta { + id: conversation_id, + timestamp: meta_rfc3339.to_string(), + cwd: PathBuf::from("/"), + originator: "codex".to_string(), + cli_version: "0.0.0".to_string(), + instructions: None, + source: SessionSource::Cli, + model_provider: model_provider.map(str::to_string), + }; + let payload = serde_json::to_value(SessionMetaLine { + meta, + git: git_info, + })?; + + let lines = [ + json!( { + "timestamp": meta_rfc3339, + "type": "session_meta", + "payload": payload + }) + .to_string(), + json!( { + "timestamp": meta_rfc3339, + "type":"response_item", + "payload": { + "type":"message", + "role":"user", + "content":[{"type":"input_text","text": preview}] + } + }) + .to_string(), + json!( { + "timestamp": meta_rfc3339, + "type":"event_msg", + "payload": { + "type":"user_message", + "message": preview, + "text_elements": text_elements, + "local_images": [] + } + }) + .to_string(), + ]; + + fs::write(file_path, lines.join("\n") + "\n")?; + Ok(uuid_str) +} diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index cfcfcedf70fb..0cd9bbe773b2 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -1,6 +1,7 @@ use anyhow::Result; use app_test_support::McpProcess; use app_test_support::create_fake_rollout; +use app_test_support::create_fake_rollout_with_text_elements; use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::to_response; use codex_app_server_protocol::JSONRPCResponse; @@ -15,6 +16,9 @@ use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseItem; +use codex_protocol::user_input::ByteRange; +use codex_protocol::user_input::TextElement; +use pretty_assertions::assert_eq; use std::path::PathBuf; use tempfile::TempDir; use tokio::time::timeout; @@ -71,11 +75,19 @@ async fn thread_resume_returns_rollout_history() -> Result<()> { create_config_toml(codex_home.path(), &server.uri())?; let preview = "Saved user message"; - let conversation_id = create_fake_rollout( + let text_elements = vec![TextElement { + byte_range: ByteRange { start: 0, end: 5 }, + placeholder: Some("".into()), + }]; + let conversation_id = create_fake_rollout_with_text_elements( codex_home.path(), "2025-01-05T12-00-00", "2025-01-05T12:00:00Z", preview, + text_elements + .iter() + .map(|elem| serde_json::to_value(elem).expect("serialize text element")) + .collect(), Some("mock_provider"), None, )?; @@ -119,7 +131,7 @@ async fn thread_resume_returns_rollout_history() -> Result<()> { content, &vec![UserInput::Text { text: preview.to_string(), - text_elements: Vec::new(), + text_elements: text_elements.clone().into_iter().map(Into::into).collect(), }] ); } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 406f80328ba0..e3b758a4c646 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -8,6 +8,7 @@ use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::create_shell_command_sse_response; use app_test_support::format_with_current_shell_display; use app_test_support::to_response; +use codex_app_server_protocol::ByteRange; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::CommandExecutionApprovalDecision; use codex_app_server_protocol::CommandExecutionRequestApprovalResponse; @@ -23,6 +24,7 @@ use codex_app_server_protocol::PatchApplyStatus; use codex_app_server_protocol::PatchChangeKind; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::TextElement; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; @@ -113,6 +115,87 @@ async fn turn_start_sends_originator_header() -> Result<()> { Ok(()) } +#[tokio::test] +async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> { + let responses = vec![create_final_assistant_message_sse_response("Done")?]; + let server = create_mock_responses_server_sequence_unchecked(responses).await; + + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri(), "never")?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + ..Default::default() + }) + .await?; + let thread_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_req)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + + let text_elements = vec![TextElement { + byte_range: ByteRange { start: 0, end: 5 }, + placeholder: Some("".to_string()), + }]; + let turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements: text_elements.clone(), + }], + ..Default::default() + }) + .await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + + let user_message_item = timeout(DEFAULT_READ_TIMEOUT, async { + loop { + let notification = mcp + .read_stream_until_notification_message("item/started") + .await?; + let params = notification.params.expect("item/started params"); + let item_started: ItemStartedNotification = + serde_json::from_value(params).expect("deserialize item/started notification"); + if let ThreadItem::UserMessage { .. } = item_started.item { + return Ok::(item_started.item); + } + } + }) + .await??; + + match user_message_item { + ThreadItem::UserMessage { content, .. } => { + assert_eq!( + content, + vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements, + }] + ); + } + other => panic!("expected user message item, got {other:?}"), + } + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + Ok(()) +} + #[tokio::test] async fn turn_start_emits_notifications_and_accepts_model_override() -> Result<()> { // Provide a mock server and config so model wiring is valid. From ea5d263dcea1cb03d7dd4a00a5b57681dddfe1bd Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Thu, 15 Jan 2026 17:09:10 -0800 Subject: [PATCH 8/8] Lint --- codex-rs/app-server/tests/suite/v2/thread_resume.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 0cd9bbe773b2..be2971397dee 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -1,6 +1,5 @@ use anyhow::Result; use app_test_support::McpProcess; -use app_test_support::create_fake_rollout; use app_test_support::create_fake_rollout_with_text_elements; use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::to_response;