diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a68f99425c..8398d0b99c 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1900,6 +1900,7 @@ version = "0.0.0" dependencies = [ "anyhow", "clap", + "codex-utils-absolute-path", "multimap", "pretty_assertions", "serde", diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 5ab197c84b..03e42bed45 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1340,7 +1340,7 @@ "type": "string" }, "output": { - "type": "string" + "$ref": "#/definitions/FunctionCallOutputPayload" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 745979df4e..b3e4317065 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -4822,7 +4822,7 @@ "type": "string" }, "output": { - "type": "string" + "$ref": "#/definitions/FunctionCallOutputPayload" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 9fa85afdf2..a0863ad61d 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -10225,6 +10225,16 @@ }, "Model": { "properties": { + "availabilityNux": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ModelAvailabilityNux" + }, + { + "type": "null" + } + ] + }, "defaultReasoningEffort": { "$ref": "#/definitions/v2/ReasoningEffort" }, @@ -10271,6 +10281,16 @@ "string", "null" ] + }, + "upgradeInfo": { + "anyOf": [ + { + "$ref": "#/definitions/v2/ModelUpgradeInfo" + }, + { + "type": "null" + } + ] } }, "required": [ @@ -10285,6 +10305,17 @@ ], "type": "object" }, + "ModelAvailabilityNux": { + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ], + "type": "object" + }, "ModelListParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -10373,6 +10404,35 @@ "title": "ModelReroutedNotification", "type": "object" }, + "ModelUpgradeInfo": { + "properties": { + "migrationMarkdown": { + "type": [ + "string", + "null" + ] + }, + "model": { + "type": "string" + }, + "modelLink": { + "type": [ + "string", + "null" + ] + }, + "upgradeCopy": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "model" + ], + "type": "object" + }, "NetworkAccess": { "enum": [ "restricted", @@ -11382,7 +11442,7 @@ "type": "string" }, "output": { - "type": "string" + "$ref": "#/definitions/v2/FunctionCallOutputPayload" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json index 2afa018dc5..4023f76b51 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ModelListResponse.json @@ -22,6 +22,16 @@ }, "Model": { "properties": { + "availabilityNux": { + "anyOf": [ + { + "$ref": "#/definitions/ModelAvailabilityNux" + }, + { + "type": "null" + } + ] + }, "defaultReasoningEffort": { "$ref": "#/definitions/ReasoningEffort" }, @@ -68,6 +78,16 @@ "string", "null" ] + }, + "upgradeInfo": { + "anyOf": [ + { + "$ref": "#/definitions/ModelUpgradeInfo" + }, + { + "type": "null" + } + ] } }, "required": [ @@ -82,6 +102,46 @@ ], "type": "object" }, + "ModelAvailabilityNux": { + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ], + "type": "object" + }, + "ModelUpgradeInfo": { + "properties": { + "migrationMarkdown": { + "type": [ + "string", + "null" + ] + }, + "model": { + "type": "string" + }, + "modelLink": { + "type": [ + "string", + "null" + ] + }, + "upgradeCopy": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "model" + ], + "type": "object" + }, "ReasoningEffort": { "description": "See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning", "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json index 748eeaab46..4717ff266b 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/RawResponseItemCompletedNotification.json @@ -565,7 +565,7 @@ "type": "string" }, "output": { - "type": "string" + "$ref": "#/definitions/FunctionCallOutputPayload" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index 29d6fbc6d7..ef7607d3ca 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -615,7 +615,7 @@ "type": "string" }, "output": { - "type": "string" + "$ref": "#/definitions/FunctionCallOutputPayload" }, "type": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts index 611c7fb22d..dd7621f01d 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ResponseItem.ts @@ -15,4 +15,4 @@ export type ResponseItem = { "type": "message", role: string, content: Array, defaultReasoningEffort: ReasoningEffort, inputModalities: Array, supportsPersonality: boolean, isDefault: boolean, }; +export type Model = { id: string, model: string, upgrade: string | null, upgradeInfo: ModelUpgradeInfo | null, availabilityNux: ModelAvailabilityNux | null, displayName: string, description: string, hidden: boolean, supportedReasoningEfforts: Array, defaultReasoningEffort: ReasoningEffort, inputModalities: Array, supportsPersonality: boolean, isDefault: boolean, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ModelAvailabilityNux.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ModelAvailabilityNux.ts new file mode 100644 index 0000000000..7254aaec9b --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ModelAvailabilityNux.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type ModelAvailabilityNux = { message: string, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/ModelUpgradeInfo.ts b/codex-rs/app-server-protocol/schema/typescript/v2/ModelUpgradeInfo.ts new file mode 100644 index 0000000000..82d73e9d06 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/ModelUpgradeInfo.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type ModelUpgradeInfo = { model: string, upgradeCopy: string | null, modelLink: string | null, migrationMarkdown: string | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 4c638c7d30..4a881427d9 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -107,10 +107,12 @@ export type { McpToolCallResult } from "./McpToolCallResult"; export type { McpToolCallStatus } from "./McpToolCallStatus"; export type { MergeStrategy } from "./MergeStrategy"; export type { Model } from "./Model"; +export type { ModelAvailabilityNux } from "./ModelAvailabilityNux"; export type { ModelListParams } from "./ModelListParams"; export type { ModelListResponse } from "./ModelListResponse"; export type { ModelRerouteReason } from "./ModelRerouteReason"; export type { ModelReroutedNotification } from "./ModelReroutedNotification"; +export type { ModelUpgradeInfo } from "./ModelUpgradeInfo"; export type { NetworkAccess } from "./NetworkAccess"; export type { NetworkApprovalContext } from "./NetworkApprovalContext"; export type { NetworkApprovalProtocol } from "./NetworkApprovalProtocol"; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 0602dc0e60..8052eb080e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -31,6 +31,7 @@ use codex_protocol::models::MessagePhase; use codex_protocol::models::PermissionProfile as CorePermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::InputModality; +use codex_protocol::openai_models::ModelAvailabilityNux as CoreModelAvailabilityNux; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::default_input_modalities; use codex_protocol::parse_command::ParsedCommand as CoreParsedCommand; @@ -1389,6 +1390,21 @@ pub struct ModelListParams { pub include_hidden: Option, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ModelAvailabilityNux { + pub message: String, +} + +impl From for ModelAvailabilityNux { + fn from(value: CoreModelAvailabilityNux) -> Self { + Self { + message: value.message, + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1396,6 +1412,8 @@ pub struct Model { pub id: String, pub model: String, pub upgrade: Option, + pub upgrade_info: Option, + pub availability_nux: Option, pub display_name: String, pub description: String, pub hidden: bool, @@ -1409,6 +1427,16 @@ pub struct Model { pub is_default: bool, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ModelUpgradeInfo { + pub model: String, + pub upgrade_copy: Option, + pub model_link: Option, + pub migration_markdown: Option, +} + #[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 d1254b9f7a..1e5c37d286 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -142,7 +142,7 @@ Example with notification opt-out: - `thread/realtime/stop` — stop the active realtime session for the thread (experimental); returns `{}`. - `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review. - `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation). -- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with reasoning effort options and optional `upgrade` model ids. +- `model/list` — list available models (set `includeHidden: true` to include entries with `hidden: true`), with reasoning effort options, optional legacy `upgrade` model ids, optional `upgradeInfo` metadata (`model`, `upgradeCopy`, `modelLink`, `migrationMarkdown`), and optional `availabilityNux` metadata. - `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. For non-beta flags, `displayName`/`description`/`announcement` are `null`. - `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly. - `skills/list` — list skills for one or more `cwd` values (optional `forceReload`). diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 85dc08a5f9..1f6e1ac0f1 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -560,7 +560,12 @@ impl CodexMessageProcessor { Ok((review_request, hint)) } - pub async fn process_request(&mut self, connection_id: ConnectionId, request: ClientRequest) { + pub async fn process_request( + &mut self, + connection_id: ConnectionId, + request: ClientRequest, + app_server_client_name: Option, + ) { let to_connection_request_id = |request_id| ConnectionRequestId { connection_id, request_id, @@ -647,8 +652,12 @@ impl CodexMessageProcessor { .await; } ClientRequest::TurnStart { request_id, params } => { - self.turn_start(to_connection_request_id(request_id), params) - .await; + self.turn_start( + to_connection_request_id(request_id), + params, + app_server_client_name.clone(), + ) + .await; } ClientRequest::TurnSteer { request_id, params } => { self.turn_steer(to_connection_request_id(request_id), params) @@ -767,12 +776,20 @@ impl CodexMessageProcessor { .await; } ClientRequest::SendUserMessage { request_id, params } => { - self.send_user_message(to_connection_request_id(request_id), params) - .await; + self.send_user_message( + to_connection_request_id(request_id), + params, + app_server_client_name.clone(), + ) + .await; } ClientRequest::SendUserTurn { request_id, params } => { - self.send_user_turn(to_connection_request_id(request_id), params) - .await; + self.send_user_turn( + to_connection_request_id(request_id), + params, + app_server_client_name.clone(), + ) + .await; } ClientRequest::InterruptConversation { request_id, params } => { self.interrupt_conversation(to_connection_request_id(request_id), params) @@ -4152,6 +4169,7 @@ impl CodexMessageProcessor { http_headers, env_http_headers, scopes.as_deref().unwrap_or_default(), + server.oauth_resource.as_deref(), timeout_secs, config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), @@ -5062,6 +5080,7 @@ impl CodexMessageProcessor { &self, request_id: ConnectionRequestId, params: SendUserMessageParams, + app_server_client_name: Option, ) { let SendUserMessageParams { conversation_id, @@ -5080,6 +5099,12 @@ impl CodexMessageProcessor { self.outgoing.send_error(request_id, error).await; return; }; + if let Err(error) = + Self::set_app_server_client_name(conversation.as_ref(), app_server_client_name).await + { + self.outgoing.send_error(request_id, error).await; + return; + } let mapped_items: Vec = items .into_iter() @@ -5110,7 +5135,12 @@ impl CodexMessageProcessor { .await; } - async fn send_user_turn(&self, request_id: ConnectionRequestId, params: SendUserTurnParams) { + async fn send_user_turn( + &self, + request_id: ConnectionRequestId, + params: SendUserTurnParams, + app_server_client_name: Option, + ) { let SendUserTurnParams { conversation_id, items, @@ -5136,6 +5166,12 @@ impl CodexMessageProcessor { self.outgoing.send_error(request_id, error).await; return; }; + if let Err(error) = + Self::set_app_server_client_name(conversation.as_ref(), app_server_client_name).await + { + self.outgoing.send_error(request_id, error).await; + return; + } let mapped_items: Vec = items .into_iter() @@ -5249,6 +5285,7 @@ impl CodexMessageProcessor { connectors::list_cached_accessible_connectors_from_mcp_tools(&config), connectors::list_cached_all_connectors(&config) ); + let cached_all_connectors = all_connectors.clone(); let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); @@ -5275,6 +5312,19 @@ impl CodexMessageProcessor { let app_list_deadline = tokio::time::Instant::now() + APP_LIST_LOAD_TIMEOUT; let mut accessible_loaded = false; let mut all_loaded = false; + let mut last_notified_apps = None; + + if accessible_connectors.is_some() || all_connectors.is_some() { + let merged = connectors::with_app_enabled_state( + Self::merge_loaded_apps( + all_connectors.as_deref(), + accessible_connectors.as_deref(), + ), + &config, + ); + Self::send_app_list_updated_notification(&outgoing, merged.clone()).await; + last_notified_apps = Some(merged); + } loop { let result = match tokio::time::timeout_at(app_list_deadline, rx.recv()).await { @@ -5331,14 +5381,30 @@ impl CodexMessageProcessor { } } + let showing_interim_force_refetch = force_refetch && !(accessible_loaded && all_loaded); + let all_connectors_for_update = + if showing_interim_force_refetch && cached_all_connectors.is_some() { + cached_all_connectors.as_deref() + } else { + all_connectors.as_deref() + }; + let accessible_connectors_for_update = + if showing_interim_force_refetch && !accessible_loaded { + None + } else { + accessible_connectors.as_deref() + }; let merged = connectors::with_app_enabled_state( Self::merge_loaded_apps( - all_connectors.as_deref(), - accessible_connectors.as_deref(), + all_connectors_for_update, + accessible_connectors_for_update, ), &config, ); - Self::send_app_list_updated_notification(&outgoing, merged.clone()).await; + if last_notified_apps.as_ref() != Some(&merged) { + Self::send_app_list_updated_notification(&outgoing, merged.clone()).await; + last_notified_apps = Some(merged.clone()); + } if accessible_loaded && all_loaded { match Self::paginate_apps(merged.as_slice(), start, limit) { @@ -5607,7 +5673,12 @@ impl CodexMessageProcessor { let _ = conversation.submit(Op::Interrupt).await; } - async fn turn_start(&self, request_id: ConnectionRequestId, params: TurnStartParams) { + async fn turn_start( + &self, + request_id: ConnectionRequestId, + params: TurnStartParams, + app_server_client_name: Option, + ) { if let Err(error) = Self::validate_v2_input_limit(¶ms.input) { self.outgoing.send_error(request_id, error).await; return; @@ -5619,6 +5690,12 @@ impl CodexMessageProcessor { return; } }; + if let Err(error) = + Self::set_app_server_client_name(thread.as_ref(), app_server_client_name).await + { + self.outgoing.send_error(request_id, error).await; + return; + } let collaboration_modes_config = CollaborationModesConfig { default_mode_request_user_input: thread.enabled(Feature::DefaultModeRequestUserInput), @@ -5700,6 +5777,20 @@ impl CodexMessageProcessor { } } + async fn set_app_server_client_name( + thread: &CodexThread, + app_server_client_name: Option, + ) -> Result<(), JSONRPCErrorError> { + thread + .set_app_server_client_name(app_server_client_name) + .await + .map_err(|err| JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!("failed to set app server client name: {err}"), + data: None, + }) + } + async fn turn_steer(&self, request_id: ConnectionRequestId, params: TurnSteerParams) { let (_, thread) = match self.load_thread(¶ms.thread_id).await { Ok(v) => v, diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 9f1b182cfa..79f845ad4b 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -140,6 +140,7 @@ pub(crate) struct ConnectionSessionState { pub(crate) initialized: bool, pub(crate) experimental_api_enabled: bool, pub(crate) opted_out_notification_methods: HashSet, + pub(crate) app_server_client_name: Option, } pub(crate) struct MessageProcessorArgs { @@ -329,6 +330,7 @@ impl MessageProcessor { if let Ok(mut suffix) = USER_AGENT_SUFFIX.lock() { *suffix = Some(user_agent_suffix); } + session.app_server_client_name = Some(name.clone()); let user_agent = get_codex_user_agent(); let response = InitializeResponse { user_agent }; @@ -430,7 +432,7 @@ impl MessageProcessor { // inline the full `CodexMessageProcessor::process_request` future, which // can otherwise push worker-thread stack usage over the edge. self.codex_message_processor - .process_request(connection_id, other) + .process_request(connection_id, other, session.app_server_client_name.clone()) .boxed() .await; } diff --git a/codex-rs/app-server/src/models.rs b/codex-rs/app-server/src/models.rs index 1594c66229..6dbe77455f 100644 --- a/codex-rs/app-server/src/models.rs +++ b/codex-rs/app-server/src/models.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use codex_app_server_protocol::Model; +use codex_app_server_protocol::ModelUpgradeInfo; use codex_app_server_protocol::ReasoningEffortOption; use codex_core::ThreadManager; use codex_core::models_manager::manager::RefreshStrategy; @@ -24,7 +25,14 @@ fn model_from_preset(preset: ModelPreset) -> Model { Model { id: preset.id.to_string(), model: preset.model.to_string(), - upgrade: preset.upgrade.map(|upgrade| upgrade.id), + upgrade: preset.upgrade.as_ref().map(|upgrade| upgrade.id.clone()), + upgrade_info: preset.upgrade.as_ref().map(|upgrade| ModelUpgradeInfo { + model: upgrade.id.clone(), + upgrade_copy: upgrade.upgrade_copy.clone(), + model_link: upgrade.model_link.clone(), + migration_markdown: upgrade.migration_markdown.clone(), + }), + availability_nux: preset.availability_nux.map(Into::into), display_name: preset.display_name.to_string(), description: preset.description.to_string(), hidden: !preset.show_in_picker, diff --git a/codex-rs/app-server/tests/common/models_cache.rs b/codex-rs/app-server/tests/common/models_cache.rs index a38da2e07f..0de8fda5f2 100644 --- a/codex-rs/app-server/tests/common/models_cache.rs +++ b/codex-rs/app-server/tests/common/models_cache.rs @@ -34,6 +34,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo { default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index a9bc14e932..6a3243ad56 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -994,6 +994,41 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu let first_update = read_app_list_updated_notification(&mut mcp).await?; assert_eq!( first_update.data, + vec![ + AppInfo { + id: "beta".to_string(), + name: "Beta App".to_string(), + description: Some("Beta v1".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://chatgpt.com/apps/beta-app/beta".to_string()), + is_accessible: true, + is_enabled: true, + }, + AppInfo { + id: "alpha".to_string(), + name: "Alpha".to_string(), + description: Some("Alpha v1".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), + is_accessible: false, + is_enabled: true, + }, + ] + ); + + let second_update = read_app_list_updated_notification(&mut mcp).await?; + assert_eq!( + second_update.data, vec![ AppInfo { id: "alpha".to_string(), @@ -1040,8 +1075,8 @@ async fn list_apps_force_refetch_patches_updates_from_cached_snapshots() -> Resu is_accessible: false, is_enabled: true, }]; - let second_update = read_app_list_updated_notification(&mut mcp).await?; - assert_eq!(second_update.data, expected_final); + let third_update = read_app_list_updated_notification(&mut mcp).await?; + assert_eq!(third_update.data, expected_final); let refetch_response: JSONRPCResponse = timeout( DEFAULT_TIMEOUT, diff --git a/codex-rs/app-server/tests/suite/v2/initialize.rs b/codex-rs/app-server/tests/suite/v2/initialize.rs index 2edc83a7f4..de437c5bf5 100644 --- a/codex-rs/app-server/tests/suite/v2/initialize.rs +++ b/codex-rs/app-server/tests/suite/v2/initialize.rs @@ -1,16 +1,24 @@ use anyhow::Result; use app_test_support::McpProcess; +use app_test_support::create_final_assistant_message_sse_response; use app_test_support::create_mock_responses_server_sequence_unchecked; use app_test_support::to_response; use codex_app_server_protocol::ClientInfo; use codex_app_server_protocol::InitializeCapabilities; use codex_app_server_protocol::InitializeResponse; use codex_app_server_protocol::JSONRPCMessage; +use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::UserInput as V2UserInput; +use core_test_support::fs_wait; use pretty_assertions::assert_eq; +use serde_json::Value; use std::path::Path; +use std::time::Duration; use tempfile::TempDir; use tokio::time::timeout; @@ -178,11 +186,103 @@ async fn initialize_opt_out_notification_methods_filters_notifications() -> Resu Ok(()) } +#[tokio::test] +async fn turn_start_notify_payload_includes_initialize_client_name() -> 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()?; + let notify_script = codex_home.path().join("notify.py"); + std::fs::write( + ¬ify_script, + r#"from pathlib import Path +import sys + +payload_path = Path(__file__).with_name("notify.json") +tmp_path = payload_path.with_suffix(".json.tmp") +tmp_path.write_text(sys.argv[-1], encoding="utf-8") +tmp_path.replace(payload_path) +"#, + )?; + let notify_file = codex_home.path().join("notify.json"); + let notify_script = notify_script + .to_str() + .expect("notify script path should be valid UTF-8"); + create_config_toml_with_extra( + codex_home.path(), + &server.uri(), + "never", + &format!( + "notify = [\"python3\", {}]", + toml_basic_string(notify_script) + ), + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout( + DEFAULT_READ_TIMEOUT, + mcp.initialize_with_client_info(ClientInfo { + name: "xcode".to_string(), + title: Some("Xcode".to_string()), + version: "1.0.0".to_string(), + }), + ) + .await??; + + let thread_req = mcp + .send_thread_start_request(ThreadStartParams::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 turn_req = mcp + .send_turn_start_request(TurnStartParams { + thread_id: thread.id, + input: vec![V2UserInput::Text { + text: "Hello".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }) + .await?; + let turn_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(turn_req)), + ) + .await??; + let _: TurnStartResponse = to_response(turn_resp)?; + + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + + fs_wait::wait_for_path_exists(¬ify_file, Duration::from_secs(5)).await?; + let payload_raw = tokio::fs::read_to_string(¬ify_file).await?; + let payload: Value = serde_json::from_str(&payload_raw)?; + assert_eq!(payload["client"], "xcode"); + + Ok(()) +} + // Helper to create a config.toml pointing at the mock model server. fn create_config_toml( codex_home: &Path, server_uri: &str, approval_policy: &str, +) -> std::io::Result<()> { + create_config_toml_with_extra(codex_home, server_uri, approval_policy, "") +} + +fn create_config_toml_with_extra( + codex_home: &Path, + server_uri: &str, + approval_policy: &str, + extra: &str, ) -> std::io::Result<()> { let config_toml = codex_home.join("config.toml"); std::fs::write( @@ -195,6 +295,8 @@ sandbox_mode = "read-only" model_provider = "mock_provider" +{extra} + [model_providers.mock_provider] name = "Mock provider for test" base_url = "{server_uri}/v1" @@ -205,3 +307,7 @@ stream_max_retries = 0 ), ) } + +fn toml_basic_string(value: &str) -> String { + format!("\"{}\"", value.replace('\\', "\\\\").replace('"', "\\\"")) +} diff --git a/codex-rs/app-server/tests/suite/v2/model_list.rs b/codex-rs/app-server/tests/suite/v2/model_list.rs index a71a8a3377..eba8905a77 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list.rs @@ -9,6 +9,7 @@ use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::Model; use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::ModelListResponse; +use codex_app_server_protocol::ModelUpgradeInfo; use codex_app_server_protocol::ReasoningEffortOption; use codex_app_server_protocol::RequestId; use codex_protocol::openai_models::ModelPreset; @@ -24,6 +25,13 @@ fn model_from_preset(preset: &ModelPreset) -> Model { id: preset.id.clone(), model: preset.model.clone(), upgrade: preset.upgrade.as_ref().map(|upgrade| upgrade.id.clone()), + upgrade_info: preset.upgrade.as_ref().map(|upgrade| ModelUpgradeInfo { + model: upgrade.id.clone(), + upgrade_copy: upgrade.upgrade_copy.clone(), + model_link: upgrade.model_link.clone(), + migration_markdown: upgrade.migration_markdown.clone(), + }), + availability_nux: preset.availability_nux.clone().map(Into::into), display_name: preset.display_name.clone(), description: preset.description.clone(), hidden: !preset.show_in_picker, diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index a3470ff1b9..620a93872e 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -298,12 +298,7 @@ fn merge_directory_app(existing: &mut DirectoryApp, incoming: DirectoryApp) { .as_deref() .map(|value| !value.trim().is_empty()) .unwrap_or(false); - let existing_description_present = existing - .description - .as_deref() - .map(|value| !value.trim().is_empty()) - .unwrap_or(false); - if !existing_description_present && incoming_description_present { + if incoming_description_present { existing.description = description; } diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 396e077411..2da868cada 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -250,6 +250,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }; servers.insert(name.clone(), new_entry); @@ -272,6 +273,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re oauth_config.http_headers, oauth_config.env_http_headers, &Vec::new(), + None, config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), ) @@ -356,6 +358,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) http_headers, env_http_headers, &scopes, + server.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), ) diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index 6f6bf3b6de..d89ff53524 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -28,17 +28,21 @@ use serde::Serialize; use sha2::Sha256; use std::path::PathBuf; use std::sync::Arc; +use std::sync::Mutex; +use std::sync::OnceLock; use std::time::Duration; use std::time::Instant; use thiserror::Error; use tokio::fs; +use tokio::task::JoinHandle; use tokio::time::sleep; use tokio::time::timeout; const CLOUD_REQUIREMENTS_TIMEOUT: Duration = Duration::from_secs(15); const CLOUD_REQUIREMENTS_MAX_ATTEMPTS: usize = 5; const CLOUD_REQUIREMENTS_CACHE_FILENAME: &str = "cloud-requirements-cache.json"; -const CLOUD_REQUIREMENTS_CACHE_TTL: Duration = Duration::from_secs(60 * 60); +const CLOUD_REQUIREMENTS_CACHE_REFRESH_INTERVAL: Duration = Duration::from_secs(5 * 60); +const CLOUD_REQUIREMENTS_CACHE_TTL: Duration = Duration::from_secs(30 * 60); const CLOUD_REQUIREMENTS_CACHE_WRITE_HMAC_KEY: &[u8] = b"codex-cloud-requirements-cache-v3-064f8542-75b4-494c-a294-97d3ce597271"; const CLOUD_REQUIREMENTS_CACHE_READ_HMAC_KEYS: &[&[u8]] = @@ -46,6 +50,11 @@ const CLOUD_REQUIREMENTS_CACHE_READ_HMAC_KEYS: &[&[u8]] = type HmacSha256 = Hmac; +fn refresher_task_slot() -> &'static Mutex>> { + static REFRESHER_TASK: OnceLock>>> = OnceLock::new(); + REFRESHER_TASK.get_or_init(|| Mutex::new(None)) +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum FetchCloudRequirementsStatus { BackendClientInit, @@ -188,6 +197,7 @@ impl RequirementsFetcher for BackendRequirementsFetcher { } } +#[derive(Clone)] struct CloudRequirementsService { auth_manager: Arc, fetcher: Arc, @@ -325,6 +335,54 @@ impl CloudRequirementsService { None } + async fn refresh_cache_in_background(&self) { + loop { + sleep(CLOUD_REQUIREMENTS_CACHE_REFRESH_INTERVAL).await; + match timeout(self.timeout, self.refresh_cache()).await { + Ok(true) => {} + Ok(false) => break, + Err(_) => { + tracing::warn!( + "Timed out refreshing cloud requirements cache from remote; keeping existing cache" + ); + } + } + } + } + + async fn refresh_cache(&self) -> bool { + let Some(auth) = self.auth_manager.auth().await else { + return false; + }; + if !auth.is_chatgpt_auth() + || !matches!( + auth.account_plan_type(), + Some(PlanType::Business | PlanType::Enterprise) + ) + { + return false; + } + + let token_data = auth.get_token_data().ok(); + let chatgpt_user_id = token_data + .as_ref() + .and_then(|token_data| token_data.id_token.chatgpt_user_id.as_deref()); + let account_id = auth.get_account_id(); + let account_id = account_id.as_deref(); + + if self + .fetch_with_retries(&auth, chatgpt_user_id, account_id) + .await + .is_none() + { + tracing::warn!( + path = %self.cache_path.display(), + "Failed to refresh cloud requirements cache from remote" + ); + } + true + } + async fn load_cache( &self, chatgpt_user_id: Option<&str>, @@ -452,7 +510,17 @@ pub fn cloud_requirements_loader( codex_home, CLOUD_REQUIREMENTS_TIMEOUT, ); + let refresh_service = service.clone(); let task = tokio::spawn(async move { service.fetch_with_timeout().await }); + let refresh_task = + tokio::spawn(async move { refresh_service.refresh_cache_in_background().await }); + let mut refresher_guard = refresher_task_slot().lock().unwrap_or_else(|err| { + tracing::warn!("cloud requirements refresher task slot was poisoned"); + err.into_inner() + }); + if let Some(existing_task) = refresher_guard.replace(refresh_task) { + existing_task.abort(); + } CloudRequirementsLoader::new(async move { task.await .inspect_err(|err| tracing::warn!(error = %err, "Cloud requirements task failed")) @@ -1052,7 +1120,11 @@ mod tests { let cache_file: CloudRequirementsCacheFile = serde_json::from_str(&std::fs::read_to_string(path).expect("read cache")) .expect("parse cache"); - assert!(cache_file.signed_payload.expires_at > Utc::now()); + assert!( + cache_file.signed_payload.expires_at + <= cache_file.signed_payload.cached_at + ChronoDuration::minutes(30) + ); + assert!(cache_file.signed_payload.expires_at > cache_file.signed_payload.cached_at); assert!(cache_file.signed_payload.cached_at <= Utc::now()); assert_eq!( cache_file.signed_payload.chatgpt_user_id, @@ -1130,4 +1202,57 @@ mod tests { CLOUD_REQUIREMENTS_MAX_ATTEMPTS ); } + + #[tokio::test] + async fn refresh_from_remote_updates_cached_cloud_requirements() { + let codex_home = tempdir().expect("tempdir"); + let fetcher = Arc::new(SequenceFetcher::new(vec![ + Ok(Some("allowed_approval_policies = [\"never\"]".to_string())), + Ok(Some( + "allowed_approval_policies = [\"on-request\"]".to_string(), + )), + ])); + let service = CloudRequirementsService::new( + auth_manager_with_plan("business"), + fetcher, + codex_home.path().to_path_buf(), + CLOUD_REQUIREMENTS_TIMEOUT, + ); + + assert_eq!( + service.fetch().await, + Some(ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::Never]), + allowed_sandbox_modes: None, + allowed_web_search_modes: None, + mcp_servers: None, + rules: None, + enforce_residency: None, + network: None, + }) + ); + + service.refresh_cache().await; + + let path = codex_home.path().join(CLOUD_REQUIREMENTS_CACHE_FILENAME); + let cache_file: CloudRequirementsCacheFile = + serde_json::from_str(&std::fs::read_to_string(path).expect("read cache")) + .expect("parse cache"); + assert_eq!( + cache_file + .signed_payload + .contents + .as_deref() + .and_then(|contents| parse_cloud_requirements(contents).ok().flatten()), + Some(ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::OnRequest]), + allowed_sandbox_modes: None, + allowed_web_search_modes: None, + mcp_servers: None, + rules: None, + enforce_residency: None, + network: None, + }) + ); + } } diff --git a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs index 48cc1d7002..97d0b9e320 100644 --- a/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs +++ b/codex-rs/codex-api/src/endpoint/realtime_websocket/methods.rs @@ -25,6 +25,8 @@ use tokio_tungstenite::WebSocketStream; use tokio_tungstenite::tungstenite::Error as WsError; use tokio_tungstenite::tungstenite::Message; use tokio_tungstenite::tungstenite::client::IntoClientRequest; +use tracing::debug; +use tracing::error; use tracing::info; use tracing::trace; use tungstenite::protocol::WebSocketConfig; @@ -62,15 +64,23 @@ impl WsStream { }; match command { WsCommand::Send { message, tx_result } => { + debug!("realtime websocket sending message"); let result = inner.send(message).await; let should_break = result.is_err(); + if let Err(err) = &result { + error!("realtime websocket send failed: {err}"); + } let _ = tx_result.send(result); if should_break { break; } } WsCommand::Close { tx_result } => { + info!("realtime websocket sending close"); let result = inner.close(None).await; + if let Err(err) = &result { + error!("realtime websocket close failed: {err}"); + } let _ = tx_result.send(result); break; } @@ -82,7 +92,9 @@ impl WsStream { }; match message { Ok(Message::Ping(payload)) => { + trace!(payload_len = payload.len(), "realtime websocket received ping"); if let Err(err) = inner.send(Message::Pong(payload)).await { + error!("realtime websocket failed to send pong: {err}"); let _ = tx_message.send(Err(err)); break; } @@ -93,6 +105,24 @@ impl WsStream { | Message::Close(_) | Message::Frame(_))) => { let is_close = matches!(message, Message::Close(_)); + match &message { + Message::Text(_) => trace!("realtime websocket received text frame"), + Message::Binary(binary) => { + error!( + payload_len = binary.len(), + "realtime websocket received unexpected binary frame" + ); + } + Message::Close(frame) => info!( + "realtime websocket received close frame: code={:?} reason={:?}", + frame.as_ref().map(|frame| frame.code), + frame.as_ref().map(|frame| frame.reason.as_str()) + ), + Message::Frame(_) => { + trace!("realtime websocket received raw frame"); + } + Message::Ping(_) | Message::Pong(_) => {} + } if tx_message.send(Ok(message)).is_err() { break; } @@ -101,6 +131,7 @@ impl WsStream { } } Err(err) => { + error!("realtime websocket receive failed: {err}"); let _ = tx_message.send(Err(err)); break; } @@ -108,6 +139,7 @@ impl WsStream { } } } + info!("realtime websocket pump exiting"); }); ( @@ -298,7 +330,7 @@ impl RealtimeWebsocketWriter { async fn send_json(&self, message: RealtimeOutboundMessage) -> Result<(), ApiError> { let payload = serde_json::to_string(&message) .map_err(|err| ApiError::Stream(format!("failed to encode realtime request: {err}")))?; - trace!("realtime websocket request: {payload}"); + debug!(?message, "realtime websocket request"); if self.is_closed.load(Ordering::SeqCst) { return Err(ApiError::Stream( @@ -325,12 +357,14 @@ impl RealtimeWebsocketEvents { Some(Ok(msg)) => msg, Some(Err(err)) => { self.is_closed.store(true, Ordering::SeqCst); + error!("realtime websocket read failed: {err}"); return Err(ApiError::Stream(format!( "failed to read websocket message: {err}" ))); } None => { self.is_closed.store(true, Ordering::SeqCst); + info!("realtime websocket event stream ended"); return Ok(None); } }; @@ -338,11 +372,18 @@ impl RealtimeWebsocketEvents { match msg { Message::Text(text) => { if let Some(event) = parse_realtime_event(&text) { + debug!(?event, "realtime websocket parsed event"); return Ok(Some(event)); } + debug!("realtime websocket ignored unsupported text frame"); } - Message::Close(_) => { + Message::Close(frame) => { self.is_closed.store(true, Ordering::SeqCst); + info!( + "realtime websocket closed: code={:?} reason={:?}", + frame.as_ref().map(|frame| frame.code), + frame.as_ref().map(|frame| frame.reason.as_str()) + ); return Ok(None); } Message::Binary(_) => { @@ -383,15 +424,24 @@ impl RealtimeWebsocketClient { request.headers_mut().extend(headers); info!("connecting realtime websocket: {ws_url}"); - let (stream, _) = + let (stream, response) = tokio_tungstenite::connect_async_with_config(request, Some(websocket_config()), false) .await .map_err(|err| { ApiError::Stream(format!("failed to connect realtime websocket: {err}")) })?; + info!( + ws_url = %ws_url, + status = %response.status(), + "realtime websocket connected" + ); let (stream, rx_message) = WsStream::new(stream); let connection = RealtimeWebsocketConnection::new(stream, rx_message); + debug!( + conversation_id = config.session_id.as_deref().unwrap_or(""), + "realtime websocket sending session.create" + ); connection .send_session_create(config.prompt, config.session_id) .await?; diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index 08a0860594..2b61f0de60 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -82,6 +82,7 @@ async fn models_client_hits_models_endpoint() { default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/config_requirements.rs b/codex-rs/config/src/config_requirements.rs index db81b45e35..d895ef0736 100644 --- a/codex-rs/config/src/config_requirements.rs +++ b/codex-rs/config/src/config_requirements.rs @@ -1147,6 +1147,7 @@ mod tests { matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["rm"]), decision: Decision::Forbidden, + resolved_program: None, justification: None, }], } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 01ac97ef26..59140d9d16 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -613,6 +613,10 @@ "additionalProperties": false, "description": "Memories settings loaded from config.toml.", "properties": { + "generate_memories": { + "description": "When `false`, newly created threads are stored with `memory_mode = \"disabled\"` in the state DB.", + "type": "boolean" + }, "max_raw_memories_for_global": { "description": "Maximum number of recent raw memories retained for global consolidation.", "format": "uint", @@ -647,6 +651,10 @@ "phase_2_model": { "description": "Model used for memory consolidation.", "type": "string" + }, + "use_memories": { + "description": "When `false`, skip injecting memory usage instructions into developer prompts.", + "type": "boolean" } }, "type": "object" @@ -1146,6 +1154,10 @@ }, "type": "object" }, + "oauth_resource": { + "default": null, + "type": "string" + }, "required": { "default": null, "type": "boolean" diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 766497cda0..3b34d3a71e 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -187,23 +187,24 @@ Rules: config_file: None, } ), - ( - "awaiter".to_string(), - AgentRoleConfig { - description: Some(r#"Use an `awaiter` agent EVERY TIME you must run a command that will take some very long time. -This includes, but not only: -* testing -* monitoring of a long running process -* explicit ask to wait for something - -Rules: -- When an awaiter is running, you can work on something else. If you need to wait for its completion, use the largest possible timeout. -- Be patient with the `awaiter`. -- Do not use an awaiter for every compilation/test if it won't take time. Only use if for long running commands. -- Close the awaiter when you're done with it."#.to_string()), - config_file: Some("awaiter.toml".to_string().parse().unwrap_or_default()), - } - ) + // Awaiter is temp removed +// ( +// "awaiter".to_string(), +// AgentRoleConfig { +// description: Some(r#"Use an `awaiter` agent EVERY TIME you must run a command that will take some very long time. +// This includes, but not only: +// * testing +// * monitoring of a long running process +// * explicit ask to wait for something +// +// Rules: +// - When an awaiter is running, you can work on something else. If you need to wait for its completion, use the largest possible timeout. +// - Be patient with the `awaiter`. +// - Do not use an awaiter for every compilation/test if it won't take time. Only use if for long running commands. +// - Close the awaiter when you're done with it."#.to_string()), +// config_file: Some("awaiter.toml".to_string().parse().unwrap_or_default()), +// } +// ) ]) }); &CONFIG diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 33fc438727..3fea6eed56 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -84,19 +84,13 @@ fn reserialize_shell_outputs(items: &mut [ResponseItem]) { shell_call_ids.insert(call_id.clone()); } } - ResponseItem::CustomToolCallOutput { call_id, output } => { - if shell_call_ids.remove(call_id) - && let Some(structured) = parse_structured_shell_output(output) - { - *output = structured - } - } ResponseItem::FunctionCall { name, call_id, .. } if is_shell_tool_name(name) || name == "apply_patch" => { shell_call_ids.insert(call_id.clone()); } - ResponseItem::FunctionCallOutput { call_id, output } => { + ResponseItem::FunctionCallOutput { call_id, output } + | ResponseItem::CustomToolCallOutput { call_id, output } => { if shell_call_ids.remove(call_id) && let Some(structured) = output .text_content() @@ -240,6 +234,7 @@ mod tests { use codex_api::common::OpenAiVerbosity; use codex_api::common::TextControls; use codex_api::create_text_param_for_request; + use codex_protocol::models::FunctionCallOutputPayload; use pretty_assertions::assert_eq; use super::*; @@ -343,4 +338,62 @@ mod tests { let v = serde_json::to_value(&req).expect("json"); assert!(v.get("text").is_none()); } + + #[test] + fn reserializes_shell_outputs_for_function_and_custom_tool_calls() { + let raw_output = r#"{"output":"hello","metadata":{"exit_code":0,"duration_seconds":0.5}}"#; + let expected_output = "Exit code: 0\nWall time: 0.5 seconds\nOutput:\nhello"; + let mut items = vec![ + ResponseItem::FunctionCall { + id: None, + name: "shell".to_string(), + arguments: "{}".to_string(), + call_id: "call-1".to_string(), + }, + ResponseItem::FunctionCallOutput { + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload::from_text(raw_output.to_string()), + }, + ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "call-2".to_string(), + name: "apply_patch".to_string(), + input: "*** Begin Patch".to_string(), + }, + ResponseItem::CustomToolCallOutput { + call_id: "call-2".to_string(), + output: FunctionCallOutputPayload::from_text(raw_output.to_string()), + }, + ]; + + reserialize_shell_outputs(&mut items); + + assert_eq!( + items, + vec![ + ResponseItem::FunctionCall { + id: None, + name: "shell".to_string(), + arguments: "{}".to_string(), + call_id: "call-1".to_string(), + }, + ResponseItem::FunctionCallOutput { + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload::from_text(expected_output.to_string()), + }, + ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "call-2".to_string(), + name: "apply_patch".to_string(), + input: "*** Begin Patch".to_string(), + }, + ResponseItem::CustomToolCallOutput { + call_id: "call-2".to_string(), + output: FunctionCallOutputPayload::from_text(expected_output.to_string()), + }, + ] + ); + } } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6d583e46ed..91b24a0f25 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -443,6 +443,7 @@ impl Codex { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name, + app_server_client_name: None, session_source, dynamic_tools, persist_extended_history, @@ -530,6 +531,18 @@ impl Codex { self.session.steer_input(input, expected_turn_id).await } + pub(crate) async fn set_app_server_client_name( + &self, + app_server_client_name: Option, + ) -> ConstraintResult<()> { + self.session + .update_settings(SessionSettingsUpdate { + app_server_client_name, + ..Default::default() + }) + .await + } + pub(crate) async fn agent_status(&self) -> AgentStatus { self.agent_status.borrow().clone() } @@ -599,6 +612,7 @@ pub(crate) struct TurnContext { pub(crate) cwd: PathBuf, pub(crate) current_date: Option, pub(crate) timezone: Option, + pub(crate) app_server_client_name: Option, pub(crate) developer_instructions: Option, pub(crate) compact_prompt: Option, pub(crate) user_instructions: Option, @@ -685,6 +699,7 @@ impl TurnContext { cwd: self.cwd.clone(), current_date: self.current_date.clone(), timezone: self.timezone.clone(), + app_server_client_name: self.app_server_client_name.clone(), developer_instructions: self.developer_instructions.clone(), compact_prompt: self.compact_prompt.clone(), user_instructions: self.user_instructions.clone(), @@ -812,6 +827,7 @@ pub(crate) struct SessionConfiguration { original_config_do_not_use: Arc, /// Optional service name tag for session metrics. metrics_service_name: Option, + app_server_client_name: Option, /// Source of the session (cli, vscode, exec, mcp, ...) session_source: SessionSource, dynamic_tools: Vec, @@ -859,6 +875,9 @@ impl SessionConfiguration { if let Some(cwd) = updates.cwd.clone() { next_configuration.cwd = cwd; } + if let Some(app_server_client_name) = updates.app_server_client_name.clone() { + next_configuration.app_server_client_name = Some(app_server_client_name); + } Ok(next_configuration) } } @@ -873,6 +892,7 @@ pub(crate) struct SessionSettingsUpdate { pub(crate) reasoning_summary: Option, pub(crate) final_output_json_schema: Option>, pub(crate) personality: Option, + pub(crate) app_server_client_name: Option, } impl Session { @@ -1049,6 +1069,7 @@ impl Session { cwd, current_date: Some(current_date), timezone: Some(timezone), + app_server_client_name: session_configuration.app_server_client_name.clone(), developer_instructions: session_configuration.developer_instructions.clone(), compact_prompt: session_configuration.compact_prompt.clone(), user_instructions: session_configuration.user_instructions.clone(), @@ -3054,9 +3075,10 @@ impl Session { developer_sections.push(developer_instructions.to_string()); } // Add developer instructions for memories. - if let Some(memory_prompt) = - build_memory_tool_developer_instructions(&turn_context.config.codex_home).await - && turn_context.features.enabled(Feature::MemoryTool) + if turn_context.features.enabled(Feature::MemoryTool) + && turn_context.config.memories.use_memories + && let Some(memory_prompt) = + build_memory_tool_developer_instructions(&turn_context.config.codex_home).await { developer_sections.push(memory_prompt); } @@ -3948,6 +3970,7 @@ mod handlers { reasoning_summary: summary, final_output_json_schema: Some(final_output_json_schema), personality, + app_server_client_name: None, }, ) } @@ -4699,6 +4722,7 @@ async fn spawn_review_thread( ghost_snapshot: parent_turn_context.ghost_snapshot.clone(), current_date: parent_turn_context.current_date.clone(), timezone: parent_turn_context.timezone.clone(), + app_server_client_name: parent_turn_context.app_server_client_name.clone(), developer_instructions: None, user_instructions: None, compact_prompt: parent_turn_context.compact_prompt.clone(), @@ -5084,6 +5108,7 @@ pub(crate) async fn run_turn( .dispatch(HookPayload { session_id: sess.conversation_id, cwd: turn_context.cwd.clone(), + client: turn_context.app_server_client_name.clone(), triggered_at: chrono::Utc::now(), hook_event: HookEvent::AfterAgent { event: HookEventAfterAgent { @@ -7893,6 +7918,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, + app_server_client_name: None, session_source: SessionSource::Exec, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -7985,6 +8011,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, + app_server_client_name: None, session_source: SessionSource::Exec, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -8296,6 +8323,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, + app_server_client_name: None, session_source: SessionSource::Exec, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -8349,6 +8377,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, + app_server_client_name: None, session_source: SessionSource::Exec, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -8430,6 +8459,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, + app_server_client_name: None, session_source: SessionSource::Exec, dynamic_tools: Vec::new(), persist_extended_history: false, @@ -8589,6 +8619,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, + app_server_client_name: None, session_source: SessionSource::Exec, dynamic_tools, persist_extended_history: false, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index b493075d4a..19a8214ee0 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -1,6 +1,7 @@ use crate::agent::AgentStatus; use crate::codex::Codex; use crate::codex::SteerInputError; +use crate::config::ConstraintResult; use crate::error::Result as CodexResult; use crate::features::Feature; use crate::file_watcher::WatchRegistration; @@ -67,6 +68,15 @@ impl CodexThread { self.codex.steer_input(input, expected_turn_id).await } + pub async fn set_app_server_client_name( + &self, + app_server_client_name: Option, + ) -> ConstraintResult<()> { + self.codex + .set_app_server_client_name(app_server_client_name) + .await + } + /// Use sparingly: this is intended to be removed soon. pub async fn submit_with_id(&self, sub: Submission) -> CodexResult<()> { self.codex.submit_with_id(sub).await diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index ad404a9f83..592f50d907 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -195,6 +195,11 @@ mod document_helpers { { entry["scopes"] = array_from_iter(scopes.iter().cloned()); } + if let Some(resource) = &config.oauth_resource + && !resource.is_empty() + { + entry["oauth_resource"] = value(resource.clone()); + } entry } @@ -1465,6 +1470,7 @@ gpt-5 = "gpt-5.1" enabled_tools: Some(vec!["one".to_string(), "two".to_string()]), disabled_tools: None, scopes: None, + oauth_resource: None, }, ); @@ -1489,6 +1495,7 @@ gpt-5 = "gpt-5.1" enabled_tools: None, disabled_tools: Some(vec!["forbidden".to_string()]), scopes: None, + oauth_resource: Some("https://resource.example.com".to_string()), }, ); @@ -1507,6 +1514,7 @@ bearer_token_env_var = \"TOKEN\" enabled = false startup_timeout_sec = 5.0 disabled_tools = [\"forbidden\"] +oauth_resource = \"https://resource.example.com\" [mcp_servers.http.http_headers] Z-Header = \"z\" @@ -1556,6 +1564,7 @@ foo = { command = "cmd" } enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); @@ -1602,6 +1611,7 @@ foo = { command = "cmd" } # keep me enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); @@ -1647,6 +1657,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); @@ -1693,6 +1704,7 @@ foo = { command = "cmd" } enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e9da18b5c4..5eb1ad4c30 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2433,6 +2433,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, } } @@ -2452,6 +2453,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, } } @@ -2488,6 +2490,8 @@ persistence = "none" let memories = r#" [memories] +generate_memories = false +use_memories = false max_raw_memories_for_global = 512 max_unused_days = 21 max_rollout_age_days = 42 @@ -2500,6 +2504,8 @@ phase_2_model = "gpt-5" toml::from_str::(memories).expect("TOML deserialization should succeed"); assert_eq!( Some(MemoriesToml { + generate_memories: Some(false), + use_memories: Some(false), max_raw_memories_for_global: Some(512), max_unused_days: Some(21), max_rollout_age_days: Some(42), @@ -2520,6 +2526,8 @@ phase_2_model = "gpt-5" assert_eq!( config.memories, MemoriesConfig { + generate_memories: false, + use_memories: false, max_raw_memories_for_global: 512, max_unused_days: 21, max_rollout_age_days: 42, @@ -3489,6 +3497,7 @@ profile = "project" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); @@ -3646,6 +3655,7 @@ bearer_token = "secret" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -3717,6 +3727,7 @@ ZIG_VAR = "3" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -3768,6 +3779,7 @@ ZIG_VAR = "3" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -3817,6 +3829,7 @@ ZIG_VAR = "3" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -3882,6 +3895,7 @@ startup_timeout_sec = 2.0 enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); apply_blocking( @@ -3959,6 +3973,7 @@ X-Auth = "DOCS_AUTH" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -3989,6 +4004,7 @@ X-Auth = "DOCS_AUTH" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); apply_blocking( @@ -4057,6 +4073,7 @@ url = "https://example.com/mcp" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ), ( @@ -4077,6 +4094,7 @@ url = "https://example.com/mcp" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ), ]); @@ -4160,6 +4178,7 @@ url = "https://example.com/mcp" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -4205,6 +4224,7 @@ url = "https://example.com/mcp" enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -4250,6 +4270,7 @@ url = "https://example.com/mcp" enabled_tools: Some(vec!["allowed".to_string()]), disabled_tools: Some(vec!["blocked".to_string()]), scopes: None, + oauth_resource: None, }, )]); @@ -4278,6 +4299,51 @@ url = "https://example.com/mcp" Ok(()) } + #[tokio::test] + async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyhow::Result<()> { + let codex_home = TempDir::new()?; + + let servers = BTreeMap::from([( + "docs".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: "https://example.com/mcp".to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth_resource: Some("https://resource.example.com".to_string()), + }, + )]); + + apply_blocking( + codex_home.path(), + None, + &[ConfigEdit::ReplaceMcpServers(servers.clone())], + )?; + + let config_path = codex_home.path().join(CONFIG_TOML_FILE); + let serialized = std::fs::read_to_string(&config_path)?; + assert!(serialized.contains(r#"oauth_resource = "https://resource.example.com""#)); + + let loaded = load_global_mcp_servers(codex_home.path()).await?; + let docs = loaded.get("docs").expect("docs entry"); + assert_eq!( + docs.oauth_resource.as_deref(), + Some("https://resource.example.com") + ); + + Ok(()) + } + #[tokio::test] async fn set_model_updates_defaults() -> anyhow::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index d7026d6c22..3ee85226e1 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -99,6 +99,10 @@ pub struct McpServerConfig { /// Optional OAuth scopes to request during MCP login. #[serde(default, skip_serializing_if = "Option::is_none")] pub scopes: Option>, + + /// Optional OAuth resource parameter to include during MCP login (RFC 8707). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub oauth_resource: Option, } // Raw MCP config shape used for deserialization and JSON Schema generation. @@ -143,6 +147,8 @@ pub(crate) struct RawMcpServerConfig { pub disabled_tools: Option>, #[serde(default)] pub scopes: Option>, + #[serde(default)] + pub oauth_resource: Option, } impl<'de> Deserialize<'de> for McpServerConfig { @@ -166,6 +172,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { let enabled_tools = raw.enabled_tools.clone(); let disabled_tools = raw.disabled_tools.clone(); let scopes = raw.scopes.clone(); + let oauth_resource = raw.oauth_resource.clone(); fn throw_if_set(transport: &str, field: &str, value: Option<&T>) -> Result<(), E> where @@ -189,6 +196,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { throw_if_set("stdio", "bearer_token", raw.bearer_token.as_ref())?; throw_if_set("stdio", "http_headers", raw.http_headers.as_ref())?; throw_if_set("stdio", "env_http_headers", raw.env_http_headers.as_ref())?; + throw_if_set("stdio", "oauth_resource", raw.oauth_resource.as_ref())?; McpServerTransportConfig::Stdio { command, args: raw.args.clone().unwrap_or_default(), @@ -222,6 +230,7 @@ impl<'de> Deserialize<'de> for McpServerConfig { enabled_tools, disabled_tools, scopes, + oauth_resource, }) } } @@ -362,6 +371,10 @@ pub struct FeedbackConfigToml { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct MemoriesToml { + /// When `false`, newly created threads are stored with `memory_mode = "disabled"` in the state DB. + pub generate_memories: Option, + /// When `false`, skip injecting memory usage instructions into developer prompts. + pub use_memories: Option, /// Maximum number of recent raw memories retained for global consolidation. pub max_raw_memories_for_global: Option, /// Maximum number of days since a memory was last used before it becomes ineligible for phase 2 selection. @@ -381,6 +394,8 @@ pub struct MemoriesToml { /// Effective memories settings after defaults are applied. #[derive(Debug, Clone, PartialEq, Eq)] pub struct MemoriesConfig { + pub generate_memories: bool, + pub use_memories: bool, pub max_raw_memories_for_global: usize, pub max_unused_days: i64, pub max_rollout_age_days: i64, @@ -393,6 +408,8 @@ pub struct MemoriesConfig { impl Default for MemoriesConfig { fn default() -> Self { Self { + generate_memories: true, + use_memories: true, max_raw_memories_for_global: DEFAULT_MEMORIES_MAX_RAW_MEMORIES_FOR_GLOBAL, max_unused_days: DEFAULT_MEMORIES_MAX_UNUSED_DAYS, max_rollout_age_days: DEFAULT_MEMORIES_MAX_ROLLOUT_AGE_DAYS, @@ -408,6 +425,8 @@ impl From for MemoriesConfig { fn from(toml: MemoriesToml) -> Self { let defaults = Self::default(); Self { + generate_memories: toml.generate_memories.unwrap_or(defaults.generate_memories), + use_memories: toml.use_memories.unwrap_or(defaults.use_memories), max_raw_memories_for_global: toml .max_raw_memories_for_global .unwrap_or(defaults.max_raw_memories_for_global) @@ -1093,6 +1112,22 @@ mod tests { ); } + #[test] + fn deserialize_streamable_http_server_config_with_oauth_resource() { + let cfg: McpServerConfig = toml::from_str( + r#" + url = "https://example.com/mcp" + oauth_resource = "https://api.example.com" + "#, + ) + .expect("should deserialize http config with oauth_resource"); + + assert_eq!( + cfg.oauth_resource, + Some("https://api.example.com".to_string()) + ); + } + #[test] fn deserialize_server_config_with_tool_filters() { let cfg: McpServerConfig = toml::from_str( @@ -1147,6 +1182,20 @@ mod tests { "#, ) .expect_err("should reject env_http_headers for stdio transport"); + + let err = toml::from_str::( + r#" + command = "echo" + oauth_resource = "https://api.example.com" + "#, + ) + .expect_err("should reject oauth_resource for stdio transport"); + + assert!( + err.to_string() + .contains("oauth_resource is not supported for stdio"), + "unexpected error: {err}" + ); } #[test] diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 3eab05ba18..72c4b28730 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -1390,6 +1390,7 @@ prefix_rules = [ matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["rm"]), decision: Decision::Forbidden, + resolved_program: None, justification: None, }], } @@ -1415,6 +1416,7 @@ prefix_rules = [ matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git", "status"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }], } @@ -1426,6 +1428,7 @@ prefix_rules = [ matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["hg", "status"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }], } @@ -1509,6 +1512,7 @@ prefix_rules = [] matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["rm".to_string()], decision: Decision::Forbidden, + resolved_program: None, justification: None, }], } @@ -1547,6 +1551,7 @@ prefix_rules = [] matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["rm".to_string()], decision: Decision::Forbidden, + resolved_program: None, justification: None, }], } @@ -1561,6 +1566,7 @@ prefix_rules = [] matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["git".to_string(), "push".to_string()], decision: Decision::Prompt, + resolved_program: None, justification: None, }], } diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index 016642b331..e4b7755abf 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -344,32 +344,21 @@ impl ContextManager { let policy_with_serialization_budget = policy * 1.2; match item { ResponseItem::FunctionCallOutput { call_id, output } => { - let body = match &output.body { - FunctionCallOutputBody::Text(content) => FunctionCallOutputBody::Text( - truncate_text(content, policy_with_serialization_budget), - ), - FunctionCallOutputBody::ContentItems(items) => { - FunctionCallOutputBody::ContentItems( - truncate_function_output_items_with_policy( - items, - policy_with_serialization_budget, - ), - ) - } - }; ResponseItem::FunctionCallOutput { call_id: call_id.clone(), - output: FunctionCallOutputPayload { - body, - success: output.success, - }, + output: truncate_function_output_payload( + output, + policy_with_serialization_budget, + ), } } ResponseItem::CustomToolCallOutput { call_id, output } => { - let truncated = truncate_text(output, policy_with_serialization_budget); ResponseItem::CustomToolCallOutput { call_id: call_id.clone(), - output: truncated, + output: truncate_function_output_payload( + output, + policy_with_serialization_budget, + ), } } ResponseItem::Message { .. } @@ -385,6 +374,25 @@ impl ContextManager { } } +fn truncate_function_output_payload( + output: &FunctionCallOutputPayload, + policy: TruncationPolicy, +) -> FunctionCallOutputPayload { + let body = match &output.body { + FunctionCallOutputBody::Text(content) => { + FunctionCallOutputBody::Text(truncate_text(content, policy)) + } + FunctionCallOutputBody::ContentItems(items) => FunctionCallOutputBody::ContentItems( + truncate_function_output_items_with_policy(items, policy), + ), + }; + + FunctionCallOutputPayload { + body, + success: output.success, + } +} + /// API messages include every non-system item (user/assistant messages, reasoning, /// tool calls, tool outputs, shell calls, and web-search calls). fn is_api_message(message: &ResponseItem) -> bool { @@ -508,7 +516,8 @@ fn image_data_url_estimate_adjustment(item: &ResponseItem) -> (i64, i64) { } } } - ResponseItem::FunctionCallOutput { output, .. } => { + ResponseItem::FunctionCallOutput { output, .. } + | ResponseItem::CustomToolCallOutput { output, .. } => { if let FunctionCallOutputBody::ContentItems(items) = &output.body { for content_item in items { if let FunctionCallOutputContentItem::InputImage { image_url } = content_item { diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index 52fff81ed0..798abc7675 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -67,7 +67,7 @@ fn user_input_text_msg(text: &str) -> ResponseItem { fn custom_tool_call_output(call_id: &str, output: &str) -> ResponseItem { ResponseItem::CustomToolCallOutput { call_id: call_id.to_string(), - output: output.to_string(), + output: FunctionCallOutputPayload::from_text(output.to_string()), } } @@ -279,6 +279,24 @@ fn for_prompt_strips_images_when_model_does_not_support_images() { }, ]), }, + ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "tool-1".to_string(), + name: "js_repl".to_string(), + input: "view_image".to_string(), + }, + ResponseItem::CustomToolCallOutput { + call_id: "tool-1".to_string(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputText { + text: "js repl result".to_string(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "https://example.com/js-repl-result.png".to_string(), + }, + ]), + }, ]; let history = create_history_with_items(items); let text_only_modalities = vec![InputModality::Text]; @@ -321,6 +339,25 @@ fn for_prompt_strips_images_when_model_does_not_support_images() { }, ]), }, + ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "tool-1".to_string(), + name: "js_repl".to_string(), + input: "view_image".to_string(), + }, + ResponseItem::CustomToolCallOutput { + call_id: "tool-1".to_string(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputText { + text: "js repl result".to_string(), + }, + FunctionCallOutputContentItem::InputText { + text: "image content omitted because you do not support image input" + .to_string(), + }, + ]), + }, ]; assert_eq!(stripped, expected); @@ -671,7 +708,7 @@ fn remove_first_item_handles_custom_tool_pair() { }, ResponseItem::CustomToolCallOutput { call_id: "tool-1".to_string(), - output: "ok".to_string(), + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, ]; let mut h = create_history_with_items(items); @@ -750,7 +787,7 @@ fn record_items_truncates_custom_tool_call_output_content() { let long_output = line.repeat(2_500); let item = ResponseItem::CustomToolCallOutput { call_id: "tool-200".to_string(), - output: long_output.clone(), + output: FunctionCallOutputPayload::from_text(long_output.clone()), }; history.record_items([&item], policy); @@ -758,7 +795,8 @@ fn record_items_truncates_custom_tool_call_output_content() { assert_eq!(history.items.len(), 1); match &history.items[0] { ResponseItem::CustomToolCallOutput { output, .. } => { - assert_ne!(output, &long_output); + let output = output.text_content().unwrap_or_default(); + assert_ne!(output, long_output); assert!( output.contains("tokens truncated"), "expected token-based truncation marker, got {output}" @@ -949,7 +987,7 @@ fn normalize_adds_missing_output_for_custom_tool_call() { }, ResponseItem::CustomToolCallOutput { call_id: "tool-x".to_string(), - output: "aborted".to_string(), + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ] ); @@ -1016,7 +1054,7 @@ fn normalize_removes_orphan_function_call_output() { fn normalize_removes_orphan_custom_tool_call_output() { let items = vec![ResponseItem::CustomToolCallOutput { call_id: "orphan-2".to_string(), - output: "ok".to_string(), + output: FunctionCallOutputPayload::from_text("ok".to_string()), }]; let mut h = create_history_with_items(items); @@ -1089,7 +1127,7 @@ fn normalize_mixed_inserts_and_removals() { }, ResponseItem::CustomToolCallOutput { call_id: "t1".to_string(), - output: "aborted".to_string(), + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ResponseItem::LocalShellCall { id: None, @@ -1191,7 +1229,7 @@ fn normalize_removes_orphan_function_call_output_panics_in_debug() { fn normalize_removes_orphan_custom_tool_call_output_panics_in_debug() { let items = vec![ResponseItem::CustomToolCallOutput { call_id: "orphan-2".to_string(), - output: "ok".to_string(), + output: FunctionCallOutputPayload::from_text("ok".to_string()), }]; let mut h = create_history_with_items(items); h.normalize_history(&default_input_modalities()); @@ -1294,6 +1332,28 @@ fn image_data_url_payload_does_not_dominate_function_call_output_estimate() { assert!(estimated < raw_len); } +#[test] +fn image_data_url_payload_does_not_dominate_custom_tool_call_output_estimate() { + let payload = "C".repeat(50_000); + let image_url = format!("data:image/png;base64,{payload}"); + let item = ResponseItem::CustomToolCallOutput { + call_id: "call-js-repl".to_string(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputText { + text: "Screenshot captured".to_string(), + }, + FunctionCallOutputContentItem::InputImage { image_url }, + ]), + }; + + let raw_len = serde_json::to_string(&item).unwrap().len() as i64; + let estimated = estimate_response_item_model_visible_bytes(&item); + let expected = raw_len - payload.len() as i64 + IMAGE_BYTES_ESTIMATE; + + assert_eq!(estimated, expected); + assert!(estimated < raw_len); +} + #[test] fn non_base64_image_urls_are_unchanged() { let message_item = ResponseItem::Message { diff --git a/codex-rs/core/src/context_manager/normalize.rs b/codex-rs/core/src/context_manager/normalize.rs index a4fe9e64fd..572ac51fc8 100644 --- a/codex-rs/core/src/context_manager/normalize.rs +++ b/codex-rs/core/src/context_manager/normalize.rs @@ -1,7 +1,6 @@ use std::collections::HashSet; use codex_protocol::models::ContentItem; -use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; @@ -35,10 +34,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { idx, ResponseItem::FunctionCallOutput { call_id: call_id.clone(), - output: FunctionCallOutputPayload { - body: FunctionCallOutputBody::Text("aborted".to_string()), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, )); } @@ -59,7 +55,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { idx, ResponseItem::CustomToolCallOutput { call_id: call_id.clone(), - output: "aborted".to_string(), + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, )); } @@ -82,10 +78,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { idx, ResponseItem::FunctionCallOutput { call_id: call_id.clone(), - output: FunctionCallOutputPayload { - body: FunctionCallOutputBody::Text("aborted".to_string()), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, )); } @@ -245,7 +238,8 @@ pub(crate) fn strip_images_when_unsupported( } *content = normalized_content; } - ResponseItem::FunctionCallOutput { output, .. } => { + ResponseItem::FunctionCallOutput { output, .. } + | ResponseItem::CustomToolCallOutput { output, .. } => { if let Some(content_items) = output.content_items_mut() { let mut normalized_content_items = Vec::with_capacity(content_items.len()); for content_item in content_items.iter() { diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index e390996c4e..b781f0bc7d 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -472,17 +472,7 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result String { + let mut path = if cfg!(windows) { + PathBuf::from(r"C:\") + } else { + PathBuf::from("/") + }; + for segment in segments { + path.push(segment); + } + path.to_string_lossy().into_owned() + } + + fn starlark_string(value: &str) -> String { + value.replace('\\', "\\\\").replace('"', "\\\"") + } + #[tokio::test] async fn returns_empty_policy_when_no_policy_files_exist() { let temp_dir = tempdir().expect("create temp dir"); @@ -949,6 +956,7 @@ mod tests { matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["rm".to_string()], decision: Decision::Forbidden, + resolved_program: None, justification: None, }], }, @@ -991,6 +999,59 @@ mod tests { Ok(()) } + #[tokio::test] + async fn preserves_host_executables_when_requirements_overlay_is_present() -> anyhow::Result<()> + { + let temp_dir = tempdir()?; + let policy_dir = temp_dir.path().join(RULES_DIR_NAME); + fs::create_dir_all(&policy_dir)?; + let git_path = host_absolute_path(&["usr", "bin", "git"]); + let git_path_literal = starlark_string(&git_path); + fs::write( + policy_dir.join("host.rules"), + format!( + r#" +host_executable(name = "git", paths = ["{git_path_literal}"]) +"# + ), + )?; + + let mut requirements_exec_policy = Policy::empty(); + requirements_exec_policy.add_network_rule( + "blocked.example.com", + codex_execpolicy::NetworkRuleProtocol::Https, + Decision::Forbidden, + None, + )?; + + let requirements = ConfigRequirements { + exec_policy: Some(codex_config::Sourced::new( + codex_config::RequirementsExecPolicy::new(requirements_exec_policy), + codex_config::RequirementSource::Unknown, + )), + ..ConfigRequirements::default() + }; + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path())?; + let layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + TomlValue::Table(Default::default()), + ); + let config_stack = + ConfigLayerStack::new(vec![layer], requirements, ConfigRequirementsToml::default())?; + + let policy = load_exec_policy(&config_stack).await?; + + assert_eq!( + policy + .host_executables() + .get("git") + .expect("missing git host executable") + .as_ref(), + [AbsolutePathBuf::try_from(git_path)?] + ); + Ok(()) + } + #[tokio::test] async fn ignores_policies_outside_policy_dir() { let temp_dir = tempdir().expect("create temp dir"); @@ -1106,6 +1167,7 @@ mod tests { matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["rm".to_string()], decision: Decision::Forbidden, + resolved_program: None, justification: None, }], }, @@ -1117,6 +1179,7 @@ mod tests { matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["ls".to_string()], decision: Decision::Prompt, + resolved_program: None, justification: None, }], }, @@ -1983,6 +2046,7 @@ prefix_rule( let matched_rules_prompt = vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["cargo".to_string()], decision: Decision::Prompt, + resolved_program: None, justification: None, }]; assert_eq!( @@ -1996,6 +2060,7 @@ prefix_rule( let matched_rules_allow = vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["cargo".to_string()], decision: Decision::Allow, + resolved_program: None, justification: None, }]; assert_eq!( @@ -2009,6 +2074,7 @@ prefix_rule( let matched_rules_forbidden = vec![RuleMatch::PrefixRuleMatch { matched_prefix: vec!["cargo".to_string()], decision: Decision::Forbidden, + resolved_program: None, justification: None, }]; assert_eq!( diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 3abe4e9233..d744010469 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -139,6 +139,7 @@ fn codex_apps_mcp_server_config(config: &Config, auth: Option<&CodexAuth>) -> Mc enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, } } diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index 1969bc6322..8d5fc3c1a3 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -241,6 +241,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_config.http_headers, oauth_config.env_http_headers, &[], + server_config.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), ) @@ -387,6 +388,7 @@ fn mcp_dependency_to_server_config( enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }); } @@ -411,6 +413,7 @@ fn mcp_dependency_to_server_config( enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }); } @@ -468,6 +471,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); @@ -516,6 +520,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]); diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 12e3d325d3..2d22351d1f 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -2115,6 +2115,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, auth_status: McpAuthStatus::Unsupported, }; @@ -2162,6 +2163,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, auth_status: McpAuthStatus::Unsupported, }; diff --git a/codex-rs/core/src/models_manager/model_info.rs b/codex-rs/core/src/models_manager/model_info.rs index 79b8520ecd..4824e4cd11 100644 --- a/codex-rs/core/src/models_manager/model_info.rs +++ b/codex-rs/core/src/models_manager/model_info.rs @@ -69,6 +69,7 @@ pub(crate) fn model_info_from_slug(slug: &str) -> ModelInfo { visibility: ModelVisibility::None, supported_in_api: true, priority: 99, + availability_nux: None, upgrade: None, base_instructions: BASE_INSTRUCTIONS.to_string(), model_messages: local_personality_messages_for_slug(slug), diff --git a/codex-rs/core/src/realtime_conversation.rs b/codex-rs/core/src/realtime_conversation.rs index 1b858aedcf..d643d74682 100644 --- a/codex-rs/core/src/realtime_conversation.rs +++ b/codex-rs/core/src/realtime_conversation.rs @@ -31,6 +31,7 @@ use tokio::sync::Mutex; use tokio::task::JoinHandle; use tracing::debug; use tracing::error; +use tracing::info; use tracing::warn; const AUDIO_IN_QUEUE_CAPACITY: usize = 256; @@ -184,6 +185,7 @@ pub(crate) async fn handle_start( let requested_session_id = params .session_id .or_else(|| Some(sess.conversation_id.to_string())); + info!("starting realtime conversation"); let events_rx = match sess .conversation .start(api_provider, None, prompt, requested_session_id.clone()) @@ -191,11 +193,14 @@ pub(crate) async fn handle_start( { Ok(events_rx) => events_rx, Err(err) => { + error!("failed to start realtime conversation: {err}"); send_conversation_error(sess, sub_id, err.to_string(), CodexErrorInfo::Other).await; return Ok(()); } }; + info!("realtime conversation started"); + sess.send_event_raw(Event { id: sub_id.clone(), msg: EventMsg::RealtimeConversationStarted(RealtimeConversationStartedEvent { @@ -211,6 +216,7 @@ pub(crate) async fn handle_start( msg, }; while let Ok(event) = events_rx.recv().await { + debug!(conversation_id = %sess_clone.conversation_id, "received realtime conversation event"); let maybe_routed_text = match &event { RealtimeEvent::ConversationItemAdded(item) => { realtime_text_from_conversation_item(item) @@ -231,6 +237,7 @@ pub(crate) async fn handle_start( .await; } if let Some(()) = sess_clone.conversation.running_state().await { + info!("realtime conversation transport closed"); sess_clone .send_event_raw(ev(EventMsg::RealtimeConversationClosed( RealtimeConversationClosedEvent { @@ -250,6 +257,7 @@ pub(crate) async fn handle_audio( params: ConversationAudioParams, ) { if let Err(err) = sess.conversation.audio_in(params.frame).await { + error!("failed to append realtime audio: {err}"); send_conversation_error(sess, sub_id, err.to_string(), CodexErrorInfo::BadRequest).await; } } @@ -284,6 +292,7 @@ pub(crate) async fn handle_text( debug!(text = %params.text, "[realtime-text] appending realtime conversation text input"); if let Err(err) = sess.conversation.text_in(params.text).await { + error!("failed to append realtime text: {err}"); send_conversation_error(sess, sub_id, err.to_string(), CodexErrorInfo::BadRequest).await; } } diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index bc2421927b..d7404fd2c6 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -460,6 +460,7 @@ impl RolloutRecorder { state_db_ctx.clone(), state_builder, config.model_provider_id.clone(), + config.memories.generate_memories, )); Ok(Self { @@ -711,6 +712,7 @@ async fn rollout_writer( state_db_ctx: Option, mut state_builder: Option, default_provider: String, + generate_memories: bool, ) -> std::io::Result<()> { let mut writer = file.map(|file| JsonlWriter { file }); let mut buffered_items = Vec::::new(); @@ -731,6 +733,7 @@ async fn rollout_writer( state_db_ctx.as_deref(), &mut state_builder, default_provider.as_str(), + generate_memories, ) .await?; } @@ -784,6 +787,7 @@ async fn rollout_writer( state_db_ctx.as_deref(), &mut state_builder, default_provider.as_str(), + generate_memories, ) .await?; } @@ -831,6 +835,7 @@ async fn rollout_writer( Ok(()) } +#[allow(clippy::too_many_arguments)] async fn write_session_meta( mut writer: Option<&mut JsonlWriter>, session_meta: SessionMeta, @@ -839,6 +844,7 @@ async fn write_session_meta( state_db_ctx: Option<&StateRuntime>, state_builder: &mut Option, default_provider: &str, + generate_memories: bool, ) -> std::io::Result<()> { let git_info = collect_git_info(cwd).await; let session_meta_line = SessionMetaLine { @@ -860,6 +866,7 @@ async fn write_session_meta( state_builder.as_ref(), std::slice::from_ref(&rollout_item), None, + (!generate_memories).then_some("disabled"), ) .await; Ok(()) @@ -888,6 +895,7 @@ async fn write_and_reconcile_items( state_builder.as_ref(), items, "rollout_writer", + None, ) .await; Ok(()) diff --git a/codex-rs/core/src/state_db.rs b/codex-rs/core/src/state_db.rs index f14b932fa6..93c7e75f97 100644 --- a/codex-rs/core/src/state_db.rs +++ b/codex-rs/core/src/state_db.rs @@ -345,6 +345,7 @@ pub async fn reconcile_rollout( builder: Option<&ThreadMetadataBuilder>, items: &[RolloutItem], archived_only: Option, + new_thread_memory_mode: Option<&str>, ) { let Some(ctx) = context else { return; @@ -357,6 +358,7 @@ pub async fn reconcile_rollout( builder, items, "reconcile_rollout", + new_thread_memory_mode, ) .await; return; @@ -467,6 +469,7 @@ pub async fn read_repair_rollout_path( None, &[], archived_only, + None, ) .await; } @@ -479,6 +482,7 @@ pub async fn apply_rollout_items( builder: Option<&ThreadMetadataBuilder>, items: &[RolloutItem], stage: &str, + new_thread_memory_mode: Option<&str>, ) { let Some(ctx) = context else { return; @@ -499,7 +503,10 @@ pub async fn apply_rollout_items( }; builder.rollout_path = rollout_path.to_path_buf(); builder.cwd = normalize_cwd_for_state_db(&builder.cwd); - if let Err(err) = ctx.apply_rollout_items(&builder, items, None).await { + if let Err(err) = ctx + .apply_rollout_items(&builder, items, None, new_thread_memory_mode) + .await + { warn!( "state db apply_rollout_items failed during {stage} for {}: {err}", rollout_path.display() diff --git a/codex-rs/core/src/terminal.rs b/codex-rs/core/src/terminal.rs index 8437962f6c..b91aef106b 100644 --- a/codex-rs/core/src/terminal.rs +++ b/codex-rs/core/src/terminal.rs @@ -249,6 +249,7 @@ pub fn terminal_info() -> TerminalInfo { /// type is split on whitespace to extract a program name plus optional version (for example, /// `ghostty 1.2.3`), while the client term name becomes the `TERM` capability string. /// - Otherwise, `TERM_PROGRAM` (plus `TERM_PROGRAM_VERSION`) drives the detected terminal name. +/// This means `TERM_PROGRAM` can mask later probes (for example `WT_SESSION`). /// - Next, terminal-specific variables (WEZTERM, iTerm2, Apple Terminal, kitty, etc.) are checked. /// - Finally, `TERM` is used as the capability fallback with `TerminalName::Unknown`. /// diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 58925622a0..0700b4d013 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -95,15 +95,12 @@ impl ToolOutput { match self { ToolOutput::Function { body, success } => { // `custom_tool_call` is the Responses API item type for freeform - // tools (`ToolSpec::Freeform`, e.g. freeform `apply_patch`). - // Those payloads must round-trip as `custom_tool_call_output` - // with plain string output. + // tools (`ToolSpec::Freeform`, e.g. freeform `apply_patch` or + // `js_repl`). if matches!(payload, ToolPayload::Custom { .. }) { - // Freeform/custom tools (`custom_tool_call`) use the custom - // output wire shape and remain string-only. return ResponseInputItem::CustomToolCallOutput { call_id: call_id.to_string(), - output: body.to_text().unwrap_or_default(), + output: FunctionCallOutputPayload { body, success }, }; } @@ -183,7 +180,9 @@ mod tests { match response { ResponseInputItem::CustomToolCallOutput { call_id, output } => { assert_eq!(call_id, "call-42"); - assert_eq!(output, "patched"); + assert_eq!(output.text_content(), Some("patched")); + assert!(output.content_items().is_none()); + assert_eq!(output.success, Some(true)); } other => panic!("expected CustomToolCallOutput, got {other:?}"), } @@ -234,8 +233,21 @@ mod tests { match response { ResponseInputItem::CustomToolCallOutput { call_id, output } => { + let expected = vec![ + FunctionCallOutputContentItem::InputText { + text: "line 1".to_string(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,AAA".to_string(), + }, + FunctionCallOutputContentItem::InputText { + text: "line 2".to_string(), + }, + ]; assert_eq!(call_id, "call-99"); - assert_eq!(output, "line 1\nline 2"); + assert_eq!(output.content_items(), Some(expected.as_slice())); + assert_eq!(output.body.to_text().as_deref(), Some("line 1\nline 2")); + assert_eq!(output.success, Some(true)); } other => panic!("expected CustomToolCallOutput, got {other:?}"), } diff --git a/codex-rs/core/src/tools/handlers/js_repl.rs b/codex-rs/core/src/tools/handlers/js_repl.rs index 4488b4ea51..362d25b81e 100644 --- a/codex-rs/core/src/tools/handlers/js_repl.rs +++ b/codex-rs/core/src/tools/handlers/js_repl.rs @@ -155,9 +155,13 @@ impl ToolHandler for JsReplHandler { }; let content = result.output; - let items = vec![FunctionCallOutputContentItem::InputText { - text: content.clone(), - }]; + let mut items = Vec::with_capacity(result.content_items.len() + 1); + if !content.is_empty() { + items.push(FunctionCallOutputContentItem::InputText { + text: content.clone(), + }); + } + items.extend(result.content_items); emit_js_repl_exec_end( session.as_ref(), @@ -170,7 +174,11 @@ impl ToolHandler for JsReplHandler { .await; Ok(ToolOutput::Function { - body: FunctionCallOutputBody::ContentItems(items), + body: if items.is_empty() { + FunctionCallOutputBody::Text(content) + } else { + FunctionCallOutputBody::ContentItems(items) + }, success: Some(true), }) } diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index 16ff943ab6..076a496a49 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -1228,7 +1228,7 @@ mod tests { "spawn_agent", function_payload(json!({ "message": "await this command", - "agent_type": "awaiter" + "agent_type": "explorer" })), ); let output = MultiAgentHandler diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index d2defa9dbb..a234fd0eb8 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -104,6 +104,7 @@ pub struct JsReplArgs { #[derive(Clone, Debug)] pub struct JsExecResult { pub output: String, + pub content_items: Vec, } struct KernelState { @@ -125,6 +126,7 @@ struct ExecContext { #[derive(Default)] struct ExecToolCalls { in_flight: usize, + content_items: Vec, notify: Arc, cancel: CancellationToken, } @@ -136,6 +138,7 @@ enum JsReplToolCallPayloadKind { FunctionText, FunctionContentItems, CustomText, + CustomContentItems, McpResult, McpErrorResult, Error, @@ -369,6 +372,21 @@ impl JsReplManager { Some(state.cancel.clone()) } + async fn record_exec_tool_call_content_items( + exec_tool_calls: &Arc>>, + exec_id: &str, + content_items: Vec, + ) { + if content_items.is_empty() { + return; + } + + let mut calls = exec_tool_calls.lock().await; + if let Some(state) = calls.get_mut(exec_id) { + state.content_items.extend(content_items); + } + } + async fn finish_exec_tool_call( exec_tool_calls: &Arc>>, exec_id: &str, @@ -592,11 +610,18 @@ impl JsReplManager { output, ) } - ResponseInputItem::CustomToolCallOutput { output, .. } => Self::summarize_text_payload( - Some("custom_tool_call_output"), - JsReplToolCallPayloadKind::CustomText, - output, - ), + ResponseInputItem::CustomToolCallOutput { output, .. } => { + let payload_kind = if output.content_items().is_some() { + JsReplToolCallPayloadKind::CustomContentItems + } else { + JsReplToolCallPayloadKind::CustomText + }; + Self::summarize_function_output_payload( + "custom_tool_call_output", + payload_kind, + output, + ) + } ResponseInputItem::McpToolCallOutput { result, .. } => match result { Ok(result) => { let output = FunctionCallOutputPayload::from(result); @@ -769,7 +794,13 @@ impl JsReplManager { }; match response { - ExecResultMessage::Ok { output } => Ok(JsExecResult { output }), + ExecResultMessage::Ok { content_items } => { + let (output, content_items) = split_exec_result_content_items(content_items); + Ok(JsExecResult { + output, + content_items, + }) + } ExecResultMessage::Err { message } => Err(FunctionCallError::RespondToModel(message)), } } @@ -1073,10 +1104,22 @@ impl JsReplManager { error, } => { JsReplManager::wait_for_exec_tool_calls_map(&exec_tool_calls, &id).await; + let content_items = { + let calls = exec_tool_calls.lock().await; + calls + .get(&id) + .map(|state| state.content_items.clone()) + .unwrap_or_default() + }; let mut pending = pending_execs.lock().await; if let Some(tx) = pending.remove(&id) { let payload = if ok { - ExecResultMessage::Ok { output } + ExecResultMessage::Ok { + content_items: build_exec_result_content_items( + output, + content_items, + ), + } } else { ExecResultMessage::Err { message: error @@ -1133,7 +1176,11 @@ impl JsReplManager { response: None, error: Some("js_repl execution reset".to_string()), }, - result = JsReplManager::run_tool_request(ctx, req) => result, + result = JsReplManager::run_tool_request( + ctx, + req, + Arc::clone(&exec_tool_calls_for_task), + ) => result, } } None => RunToolResult { @@ -1227,7 +1274,11 @@ impl JsReplManager { } } - async fn run_tool_request(exec: ExecContext, req: RunToolRequest) -> RunToolResult { + async fn run_tool_request( + exec: ExecContext, + req: RunToolRequest, + exec_tool_calls: Arc>>, + ) -> RunToolResult { if is_js_repl_internal_tool(&req.tool_name) { let error = "js_repl cannot invoke itself".to_string(); let summary = Self::summarize_tool_call_error(&error); @@ -1300,39 +1351,13 @@ impl JsReplManager { .await { Ok(response) => { - if let ResponseInputItem::FunctionCallOutput { output, .. } = &response - && let Some(items) = output.content_items() - { - let mut has_image = false; - let mut content = Vec::with_capacity(items.len()); - for item in items { - match item { - FunctionCallOutputContentItem::InputText { text } => { - content.push(ContentItem::InputText { text: text.clone() }); - } - FunctionCallOutputContentItem::InputImage { image_url } => { - has_image = true; - content.push(ContentItem::InputImage { - image_url: image_url.clone(), - }); - } - } - } - - if has_image - && session - .inject_response_items(vec![ResponseInputItem::Message { - role: "user".to_string(), - content, - }]) - .await - .is_err() - { - warn!( - tool_name = %tool_name, - "js_repl tool call returned image content but there was no active turn to attach it to" - ); - } + if let Some(items) = response_content_items(&response) { + Self::record_exec_tool_call_content_items( + &exec_tool_calls, + &req.exec_id, + items, + ) + .await; } let summary = Self::summarize_tool_call_response(&response); @@ -1407,6 +1432,50 @@ impl JsReplManager { } } +fn response_content_items( + response: &ResponseInputItem, +) -> Option> { + match response { + ResponseInputItem::FunctionCallOutput { output, .. } + | ResponseInputItem::CustomToolCallOutput { output, .. } => output + .content_items() + .map(<[FunctionCallOutputContentItem]>::to_vec), + ResponseInputItem::McpToolCallOutput { result, .. } => match result { + Ok(result) => FunctionCallOutputPayload::from(result) + .content_items() + .map(<[FunctionCallOutputContentItem]>::to_vec), + Err(_) => None, + }, + ResponseInputItem::Message { .. } => None, + } +} + +fn build_exec_result_content_items( + output: String, + content_items: Vec, +) -> Vec { + let mut all_content_items = Vec::with_capacity(content_items.len() + 1); + all_content_items.push(FunctionCallOutputContentItem::InputText { text: output }); + all_content_items.extend(content_items); + all_content_items +} + +fn split_exec_result_content_items( + mut content_items: Vec, +) -> (String, Vec) { + match content_items.first() { + Some(FunctionCallOutputContentItem::InputText { .. }) => { + let FunctionCallOutputContentItem::InputText { text } = content_items.remove(0) else { + unreachable!("first content item should be input_text"); + }; + (text, content_items) + } + Some(FunctionCallOutputContentItem::InputImage { .. }) | None => { + (String::new(), content_items) + } + } +} + fn is_freeform_tool(specs: &[ToolSpec], name: &str) -> bool { specs .iter() @@ -1462,8 +1531,12 @@ struct RunToolResult { #[derive(Debug)] enum ExecResultMessage { - Ok { output: String }, - Err { message: String }, + Ok { + content_items: Vec, + }, + Err { + message: String, + }, } #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -1601,7 +1674,6 @@ mod tests { use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem; use codex_protocol::dynamic_tools::DynamicToolResponse; use codex_protocol::dynamic_tools::DynamicToolSpec; - use codex_protocol::models::ContentItem; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; @@ -1850,6 +1922,35 @@ mod tests { ); } + #[test] + fn summarize_tool_call_response_for_multimodal_custom_output() { + let response = ResponseInputItem::CustomToolCallOutput { + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,abcd".to_string(), + }, + ]), + }; + + let actual = JsReplManager::summarize_tool_call_response(&response); + + assert_eq!( + actual, + JsReplToolCallResponseSummary { + response_type: Some("custom_tool_call_output".to_string()), + payload_kind: Some(JsReplToolCallPayloadKind::CustomContentItems), + payload_text_preview: None, + payload_text_length: None, + payload_item_count: Some(1), + text_item_count: Some(0), + image_item_count: Some(1), + structured_content_present: None, + result_is_error: None, + } + ); + } + #[test] fn summarize_tool_call_error_marks_error_payload() { let actual = JsReplManager::summarize_tool_call_error("tool failed"); @@ -2310,20 +2411,22 @@ console.log(out.output?.body?.text ?? ""); ) .await?; assert!(result.output.contains("function_call_output")); - - let pending_input = session.get_pending_input().await; - let [ResponseInputItem::Message { role, content }] = pending_input.as_slice() else { - panic!( - "view_image should inject exactly one pending input message, got {pending_input:?}" - ); - }; - assert_eq!(role, "user"); - let [ContentItem::InputImage { image_url }] = content.as_slice() else { - panic!( - "view_image should inject exactly one input_image content item, got {content:?}" - ); + assert_eq!( + result.content_items.as_slice(), + [FunctionCallOutputContentItem::InputImage { + image_url: + "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==" + .to_string(), + }] + .as_slice() + ); + let [FunctionCallOutputContentItem::InputImage { image_url }] = + result.content_items.as_slice() + else { + panic!("view_image should return exactly one input_image content item"); }; assert!(image_url.starts_with("data:image/png;base64,")); + assert!(session.get_pending_input().await.is_empty()); Ok(()) } @@ -2404,22 +2507,18 @@ console.log(out.type); response_watcher_result?; let result = result?; assert!(result.output.contains("function_call_output")); - - let pending_input = session.get_pending_input().await; assert_eq!( - pending_input, - vec![ResponseInputItem::Message { - role: "user".to_string(), - content: vec![ - ContentItem::InputText { - text: "inline image note".to_string(), - }, - ContentItem::InputImage { - image_url: image_url.to_string(), - }, - ], - }] + result.content_items, + vec![ + FunctionCallOutputContentItem::InputText { + text: "inline image note".to_string(), + }, + FunctionCallOutputContentItem::InputImage { + image_url: image_url.to_string(), + }, + ] ); + assert!(session.get_pending_input().await.is_empty()); Ok(()) } diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index 04d505c584..5f2fc89e5f 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -117,7 +117,10 @@ impl ToolCallRuntime { match &call.payload { ToolPayload::Custom { .. } => ResponseInputItem::CustomToolCallOutput { call_id: call.call_id.clone(), - output: Self::abort_message(call, secs), + output: FunctionCallOutputPayload { + body: FunctionCallOutputBody::Text(Self::abort_message(call, secs)), + ..Default::default() + }, }, ToolPayload::Mcp { .. } => ResponseInputItem::McpToolCallOutput { call_id: call.call_id.clone(), diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 7a0be68df3..3c021cc582 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -379,6 +379,7 @@ async fn dispatch_after_tool_use_hook( .dispatch(HookPayload { session_id: session.conversation_id, cwd: turn.cwd.clone(), + client: turn.app_server_client_name.clone(), triggered_at: chrono::Utc::now(), hook_event: HookEvent::AfterToolUse { event: HookEventAfterToolUse { diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 4897b4ea6f..a55fb5fd5a 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -197,7 +197,10 @@ impl ToolRouter { if payload_outputs_custom { ResponseInputItem::CustomToolCallOutput { call_id, - output: message, + output: codex_protocol::models::FunctionCallOutputPayload { + body: FunctionCallOutputBody::Text(message), + success: Some(false), + }, } } else { ResponseInputItem::FunctionCallOutput { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index ae1df34ece..c6d82a7956 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -16,6 +16,8 @@ use crate::tools::sandboxing::SandboxablePreference; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use codex_execpolicy::Decision; +use codex_execpolicy::Evaluation; +use codex_execpolicy::MatchOptions; use codex_execpolicy::Policy; use codex_execpolicy::RuleMatch; use codex_protocol::config_types::WindowsSandboxLevel; @@ -493,28 +495,16 @@ impl EscalationPolicy for CoreShellActionProvider { .await; } - let command = join_program_and_argv(program, argv); - let (commands, used_complex_parsing) = - if let Some(commands) = parse_shell_lc_plain_commands(&command) { - (commands, false) - } else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) { - (vec![single_command], true) - } else { - (vec![command.clone()], false) - }; - - let fallback = |cmd: &[String]| { - crate::exec_policy::render_decision_for_unmatched_command( - self.approval_policy, - &self.sandbox_policy, - cmd, - self.sandbox_permissions, - used_complex_parsing, - ) - }; let evaluation = { let policy = self.policy.read().await; - policy.check_multiple(commands.iter(), &fallback) + evaluate_intercepted_exec_policy( + &policy, + program, + argv, + self.approval_policy, + &self.sandbox_policy, + self.sandbox_permissions, + ) }; // When true, means the Evaluation was due to *.rules, not the // fallback function. @@ -552,6 +542,43 @@ impl EscalationPolicy for CoreShellActionProvider { } } +fn evaluate_intercepted_exec_policy( + policy: &Policy, + program: &AbsolutePathBuf, + argv: &[String], + approval_policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + sandbox_permissions: SandboxPermissions, +) -> Evaluation { + let command = join_program_and_argv(program, argv); + let (commands, used_complex_parsing) = + if let Some(commands) = parse_shell_lc_plain_commands(&command) { + (commands, false) + } else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) { + (vec![single_command], true) + } else { + (vec![command], false) + }; + + let fallback = |cmd: &[String]| { + crate::exec_policy::render_decision_for_unmatched_command( + approval_policy, + sandbox_policy, + cmd, + sandbox_permissions, + used_complex_parsing, + ) + }; + + policy.check_multiple_with_options( + commands.iter(), + &fallback, + &MatchOptions { + resolve_host_executables: true, + }, + ) +} + struct CoreShellCommandExecutor { command: Vec, cwd: PathBuf, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 519608a43b..cfcc3c154d 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -2,6 +2,7 @@ use super::CoreShellActionProvider; #[cfg(target_os = "macos")] use super::CoreShellCommandExecutor; use super::ParsedShellCommand; +use super::evaluate_intercepted_exec_policy; use super::extract_shell_script; use super::join_program_and_argv; use super::map_exec_result; @@ -12,14 +13,16 @@ use crate::config::Permissions; #[cfg(target_os = "macos")] use crate::config::types::ShellEnvironmentPolicy; use crate::exec::SandboxType; -#[cfg(target_os = "macos")] use crate::protocol::AskForApproval; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; -#[cfg(target_os = "macos")] use crate::sandboxing::SandboxPermissions; #[cfg(target_os = "macos")] use crate::seatbelt::MACOS_PATH_TO_SEATBELT_EXECUTABLE; +use codex_execpolicy::Decision; +use codex_execpolicy::Evaluation; +use codex_execpolicy::PolicyParser; +use codex_execpolicy::RuleMatch; #[cfg(target_os = "macos")] use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::FileSystemPermissions; @@ -38,6 +41,22 @@ use pretty_assertions::assert_eq; use std::collections::HashMap; use std::time::Duration; +fn host_absolute_path(segments: &[&str]) -> String { + let mut path = if cfg!(windows) { + PathBuf::from(r"C:\") + } else { + PathBuf::from("/") + }; + for segment in segments { + path.push(segment); + } + path.to_string_lossy().into_owned() +} + +fn starlark_string(value: &str) -> String { + value.replace('\\', "\\\\").replace('"', "\\\"") +} + #[test] fn extract_shell_script_preserves_login_flag() { assert_eq!( @@ -203,6 +222,84 @@ fn shell_request_escalation_execution_is_explicit() { ); } +#[test] +fn intercepted_exec_policy_uses_host_executable_mappings() { + let git_path = host_absolute_path(&["usr", "bin", "git"]); + let git_path_literal = starlark_string(&git_path); + let policy_src = format!( + r#" +prefix_rule(pattern = ["git", "status"], decision = "prompt") +host_executable(name = "git", paths = ["{git_path_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src).unwrap(); + let policy = parser.build(); + let program = AbsolutePathBuf::try_from(git_path).unwrap(); + + let evaluation = evaluate_intercepted_exec_policy( + &policy, + &program, + &["git".to_string(), "status".to_string()], + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy(), + SandboxPermissions::UseDefault, + ); + + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec!["git".to_string(), "status".to_string()], + decision: Decision::Prompt, + resolved_program: Some(program), + justification: None, + }], + } + ); + assert!(CoreShellActionProvider::decision_driven_by_policy( + &evaluation.matched_rules, + evaluation.decision + )); +} + +#[test] +fn intercepted_exec_policy_rejects_disallowed_host_executable_mapping() { + let allowed_git = host_absolute_path(&["usr", "bin", "git"]); + let other_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]); + let allowed_git_literal = starlark_string(&allowed_git); + let policy_src = format!( + r#" +prefix_rule(pattern = ["git", "status"], decision = "prompt") +host_executable(name = "git", paths = ["{allowed_git_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src).unwrap(); + let policy = parser.build(); + let program = AbsolutePathBuf::try_from(other_git.clone()).unwrap(); + + let evaluation = evaluate_intercepted_exec_policy( + &policy, + &program, + &["git".to_string(), "status".to_string()], + AskForApproval::OnRequest, + &SandboxPolicy::new_read_only_policy(), + SandboxPermissions::UseDefault, + ); + + assert!(matches!( + evaluation.matched_rules.as_slice(), + [RuleMatch::HeuristicsRuleMatch { command, .. }] + if command == &vec![other_git, "status".to_string()] + )); + assert!(!CoreShellActionProvider::decision_driven_by_policy( + &evaluation.matched_rules, + evaluation.decision + )); +} + #[cfg(target_os = "macos")] #[tokio::test] async fn prepare_escalated_exec_turn_default_preserves_macos_seatbelt_extensions() { diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 6a073d9261..bb6de200a9 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -264,7 +264,7 @@ impl ResponsesRequest { .cloned() .unwrap_or(Value::Null); match output { - Value::String(text) => Some((Some(text), None)), + Value::String(_) | Value::Array(_) => Some((output_value_to_text(&output), None)), Value::Object(obj) => Some(( obj.get("content") .and_then(Value::as_str) @@ -296,6 +296,87 @@ impl ResponsesRequest { } } +pub(crate) fn output_value_to_text(value: &Value) -> Option { + match value { + Value::String(text) => Some(text.clone()), + Value::Array(items) => match items.as_slice() { + [item] if item.get("type").and_then(Value::as_str) == Some("input_text") => { + item.get("text").and_then(Value::as_str).map(str::to_string) + } + [_] | [] | [_, _, ..] => None, + }, + Value::Object(_) | Value::Number(_) | Value::Bool(_) | Value::Null => None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use wiremock::http::HeaderMap; + use wiremock::http::Method; + + fn request_with_input(input: Value) -> ResponsesRequest { + ResponsesRequest(wiremock::Request { + url: "http://localhost/v1/responses" + .parse() + .expect("valid request url"), + method: Method::POST, + headers: HeaderMap::new(), + body: serde_json::to_vec(&serde_json::json!({ "input": input })) + .expect("serialize request body"), + }) + } + + #[test] + fn call_output_content_and_success_returns_only_single_text_content_item() { + let single_text = request_with_input(serde_json::json!([ + { + "type": "function_call_output", + "call_id": "call-1", + "output": [{ "type": "input_text", "text": "hello" }] + }, + { + "type": "custom_tool_call_output", + "call_id": "call-2", + "output": [{ "type": "input_text", "text": "world" }] + } + ])); + assert_eq!( + single_text.function_call_output_content_and_success("call-1"), + Some((Some("hello".to_string()), None)) + ); + assert_eq!( + single_text.custom_tool_call_output_content_and_success("call-2"), + Some((Some("world".to_string()), None)) + ); + + let mixed_content = request_with_input(serde_json::json!([ + { + "type": "function_call_output", + "call_id": "call-3", + "output": [ + { "type": "input_text", "text": "hello" }, + { "type": "input_image", "image_url": "data:image/png;base64,abc" } + ] + }, + { + "type": "custom_tool_call_output", + "call_id": "call-4", + "output": [{ "type": "input_image", "image_url": "data:image/png;base64,abc" }] + } + ])); + assert_eq!( + mixed_content.function_call_output_content_and_success("call-3"), + Some((None, None)) + ); + assert_eq!( + mixed_content.custom_tool_call_output_content_and_success("call-4"), + Some((None, None)) + ); + } +} + #[derive(Debug, Clone)] pub struct WebSocketRequest { body: Value, diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 4253b4b686..bd15c6d7e4 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -23,6 +23,7 @@ use wiremock::MockServer; use crate::load_default_config_for_test; use crate::responses::WebSocketTestServer; +use crate::responses::output_value_to_text; use crate::responses::start_mock_server; use crate::streaming_sse::StreamingSseServer; use crate::wait_for_event; @@ -394,11 +395,7 @@ impl TestCodexHarness { pub async fn custom_tool_call_output(&self, call_id: &str) -> String { let bodies = self.request_bodies().await; - custom_tool_call_output(&bodies, call_id) - .get("output") - .and_then(Value::as_str) - .expect("output string") - .to_string() + custom_tool_call_output_text(&bodies, call_id) } pub async fn apply_patch_output( @@ -433,6 +430,14 @@ fn custom_tool_call_output<'a>(bodies: &'a [Value], call_id: &str) -> &'a Value panic!("custom_tool_call_output {call_id} not found"); } +fn custom_tool_call_output_text(bodies: &[Value], call_id: &str) -> String { + let output = custom_tool_call_output(bodies, call_id) + .get("output") + .unwrap_or_else(|| panic!("custom_tool_call_output {call_id} missing output")); + output_value_to_text(output) + .unwrap_or_else(|| panic!("custom_tool_call_output {call_id} missing text output")) +} + fn function_call_output<'a>(bodies: &'a [Value], call_id: &str) -> &'a Value { for body in bodies { if let Some(items) = body.get("input").and_then(Value::as_array) { @@ -456,3 +461,36 @@ pub fn test_codex() -> TestCodexBuilder { home: None, } } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serde_json::json; + + #[test] + fn custom_tool_call_output_text_returns_output_text() { + let bodies = vec![json!({ + "input": [{ + "type": "custom_tool_call_output", + "call_id": "call-1", + "output": "hello" + }] + })]; + + assert_eq!(custom_tool_call_output_text(&bodies, "call-1"), "hello"); + } + + #[test] + #[should_panic(expected = "custom_tool_call_output call-2 missing output")] + fn custom_tool_call_output_text_panics_when_output_is_missing() { + let bodies = vec![json!({ + "input": [{ + "type": "custom_tool_call_output", + "call_id": "call-2" + }] + })]; + + let _ = custom_tool_call_output_text(&bodies, "call-2"); + } +} diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 082b41a373..1de0522cb5 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -35,6 +35,10 @@ use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; +use codex_protocol::protocol::RolloutItem; +use codex_protocol::protocol::RolloutLine; +use codex_protocol::protocol::SessionMeta; +use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use core_test_support::apps_test_server::AppsTestServer; @@ -344,6 +348,144 @@ async fn resume_includes_initial_messages_and_sends_prior_items() { assert!(pos_environment < pos_new_user); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn resume_replays_legacy_js_repl_image_rollout_shapes() { + skip_if_no_network!(); + + // Early js_repl builds persisted image tool results as two separate rollout items: + // a string-valued custom_tool_call_output plus a standalone user input_image message. + // Current image tests cover today's shapes; this keeps resume compatibility for that + // legacy rollout representation. + let legacy_custom_tool_call = ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "legacy-js-call".to_string(), + name: "js_repl".to_string(), + input: "console.log('legacy image flow')".to_string(), + }; + let legacy_image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="; + let rollout = vec![ + RolloutLine { + timestamp: "2024-01-01T00:00:00.000Z".to_string(), + item: RolloutItem::SessionMeta(SessionMetaLine { + meta: SessionMeta { + id: ThreadId::default(), + timestamp: "2024-01-01T00:00:00Z".to_string(), + cwd: ".".into(), + originator: "test_originator".to_string(), + cli_version: "test_version".to_string(), + model_provider: Some("test-provider".to_string()), + ..Default::default() + }, + git: None, + }), + }, + RolloutLine { + timestamp: "2024-01-01T00:00:01.000Z".to_string(), + item: RolloutItem::ResponseItem(legacy_custom_tool_call), + }, + RolloutLine { + timestamp: "2024-01-01T00:00:02.000Z".to_string(), + item: RolloutItem::ResponseItem(ResponseItem::CustomToolCallOutput { + call_id: "legacy-js-call".to_string(), + output: FunctionCallOutputPayload::from_text("legacy js_repl stdout".to_string()), + }), + }, + RolloutLine { + timestamp: "2024-01-01T00:00:03.000Z".to_string(), + item: RolloutItem::ResponseItem(ResponseItem::Message { + id: None, + role: "user".to_string(), + content: vec![ContentItem::InputImage { + image_url: legacy_image_url.to_string(), + }], + end_turn: None, + phase: None, + }), + }, + ]; + + let tmpdir = TempDir::new().unwrap(); + let session_path = tmpdir + .path() + .join("resume-legacy-js-repl-image-rollout.jsonl"); + let mut f = std::fs::File::create(&session_path).unwrap(); + for line in rollout { + writeln!(f, "{}", serde_json::to_string(&line).unwrap()).unwrap(); + } + + let server = MockServer::start().await; + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new().unwrap()); + let mut builder = test_codex().with_model("gpt-5.1"); + let test = builder + .resume(&server, codex_home, session_path.clone()) + .await + .expect("resume conversation"); + test.submit_turn("after resume").await.unwrap(); + + let input = resp_mock.single_request().input(); + + let legacy_output_index = input + .iter() + .position(|item| { + item.get("type").and_then(|value| value.as_str()) == Some("custom_tool_call_output") + && item.get("call_id").and_then(|value| value.as_str()) == Some("legacy-js-call") + }) + .expect("legacy custom tool output should be replayed"); + assert_eq!( + input[legacy_output_index] + .get("output") + .and_then(|value| value.as_str()), + Some("legacy js_repl stdout") + ); + + let legacy_image_index = input + .iter() + .position(|item| { + item.get("type").and_then(|value| value.as_str()) == Some("message") + && item.get("role").and_then(|value| value.as_str()) == Some("user") + && item + .get("content") + .and_then(|value| value.as_array()) + .is_some_and(|content| { + content.iter().any(|entry| { + entry.get("type").and_then(|value| value.as_str()) + == Some("input_image") + && entry.get("image_url").and_then(|value| value.as_str()) + == Some(legacy_image_url) + }) + }) + }) + .expect("legacy injected image message should be replayed"); + + let new_user_index = input + .iter() + .position(|item| { + item.get("type").and_then(|value| value.as_str()) == Some("message") + && item.get("role").and_then(|value| value.as_str()) == Some("user") + && item + .get("content") + .and_then(|value| value.as_array()) + .is_some_and(|content| { + content.iter().any(|entry| { + entry.get("type").and_then(|value| value.as_str()) == Some("input_text") + && entry.get("text").and_then(|value| value.as_str()) + == Some("after resume") + }) + }) + }) + .expect("new user message should be present"); + + assert!(legacy_output_index < new_user_index); + assert!(legacy_image_index < new_user_index); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn includes_conversation_id_and_model_headers_in_request() { skip_if_no_network!(); @@ -1565,7 +1707,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { }); prompt.input.push(ResponseItem::CustomToolCallOutput { call_id: "custom-tool-call-id".into(), - output: "ok".into(), + output: FunctionCallOutputPayload::from_text("ok".into()), }); let mut stream = client_session diff --git a/codex-rs/core/tests/suite/model_switching.rs b/codex-rs/core/tests/suite/model_switching.rs index 159c1a5cab..62d0545462 100644 --- a/codex-rs/core/tests/suite/model_switching.rs +++ b/codex-rs/core/tests/suite/model_switching.rs @@ -237,6 +237,7 @@ async fn model_change_from_image_to_text_strips_prior_image_content() -> Result< default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, @@ -395,6 +396,7 @@ async fn model_switch_to_smaller_model_updates_token_context_window() -> Result< default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/models_cache_ttl.rs b/codex-rs/core/tests/suite/models_cache_ttl.rs index 2ecf157512..9948eb4e21 100644 --- a/codex-rs/core/tests/suite/models_cache_ttl.rs +++ b/codex-rs/core/tests/suite/models_cache_ttl.rs @@ -339,6 +339,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo { default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 09d0137995..97ce2360ba 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -602,6 +602,7 @@ async fn remote_model_friendly_personality_instructions_with_feature() -> anyhow default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, @@ -710,6 +711,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index c89d5341da..c304cc25fa 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -297,6 +297,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> { default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, @@ -534,6 +535,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> { default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, @@ -995,6 +997,7 @@ fn test_remote_model_with_policy( default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index cf56a1bb2d..dfbac85a09 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -104,6 +104,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -246,6 +247,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -401,6 +403,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, @@ -463,6 +466,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -581,6 +585,7 @@ async fn stdio_server_propagates_whitelisted_env_vars() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -740,6 +745,7 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -959,6 +965,7 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 1caab45b12..d852c9cfb3 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -133,6 +133,7 @@ fn rmcp_server_config(command: String) -> McpServerConfig { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, } } diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index eedfc02bbb..6ba6e53b1b 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -370,6 +370,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -464,6 +465,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -729,6 +731,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config diff --git a/codex-rs/core/tests/suite/user_notification.rs b/codex-rs/core/tests/suite/user_notification.rs index 5ef939776b..a2303c0ecf 100644 --- a/codex-rs/core/tests/suite/user_notification.rs +++ b/codex-rs/core/tests/suite/user_notification.rs @@ -23,10 +23,6 @@ use responses::start_mock_server; use std::time::Duration; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[ignore = "flaky on ubuntu-24.04-arm - aarch64-unknown-linux-gnu"] -// The notify script gets far enough to create (and therefore surface) the file, -// but hasn’t flushed the JSON yet. Reading an empty file produces EOF while parsing -// a value at line 1 column 0. May be caused by a slow runner. async fn summarize_context_three_requests_and_instructions() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); @@ -43,7 +39,10 @@ async fn summarize_context_three_requests_and_instructions() -> anyhow::Result<( ¬ify_script, r#"#!/bin/bash set -e -echo -n "${@: -1}" > $(dirname "${0}")/notify.txt"#, +payload_path="$(dirname "${0}")/notify.txt" +tmp_path="${payload_path}.tmp" +echo -n "${@: -1}" > "${tmp_path}" +mv "${tmp_path}" "${payload_path}""#, )?; std::fs::set_permissions(¬ify_script, std::fs::Permissions::from_mode(0o755))?; diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index c4f8ebec36..4521ed45f8 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -358,40 +358,26 @@ console.log(out.output?.body?.text ?? ""); .await; let req = mock.single_request(); - let (js_repl_output, js_repl_success) = req - .custom_tool_call_output_content_and_success(call_id) - .expect("custom tool output present"); - let js_repl_output = js_repl_output.expect("custom tool output text present"); - assert_ne!( - js_repl_success, - Some(false), - "js_repl call failed unexpectedly: {js_repl_output}" + let body = req.body_json(); + assert_eq!( + image_messages(&body).len(), + 0, + "js_repl view_image should not inject a pending input image message" ); - let body = req.body_json(); - let image_messages = image_messages(&body); - assert_eq!( - image_messages.len(), - 1, - "js_repl view_image should inject exactly one pending input image message" - ); - let image_message = image_messages - .into_iter() - .next() - .expect("pending input image message not included in request"); - let image_url = image_message - .get("content") + let custom_output = req.custom_tool_call_output(call_id); + let output_items = custom_output + .get("output") .and_then(Value::as_array) - .and_then(|content| { - content.iter().find_map(|span| { - if span.get("type").and_then(Value::as_str) == Some("input_image") { - span.get("image_url").and_then(Value::as_str) - } else { - None - } - }) + .expect("custom_tool_call_output should be a content item array"); + let image_url = output_items + .iter() + .find_map(|item| { + (item.get("type").and_then(Value::as_str) == Some("input_image")) + .then(|| item.get("image_url").and_then(Value::as_str)) + .flatten() }) - .expect("image_url present"); + .expect("image_url present in js_repl custom tool output"); assert!( image_url.starts_with("data:image/png;base64,"), "expected png data URL, got {image_url}" @@ -678,6 +664,7 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an default_reasoning_summary: ReasoningSummary::Auto, support_verbosity: false, default_verbosity: None, + availability_nux: None, apply_patch_tool_type: None, truncation_policy: TruncationPolicyConfig::bytes(10_000), supports_parallel_tool_calls: false, diff --git a/codex-rs/docs/codex_mcp_interface.md b/codex-rs/docs/codex_mcp_interface.md index 3a3abe9a9e..763f2602ad 100644 --- a/codex-rs/docs/codex_mcp_interface.md +++ b/codex-rs/docs/codex_mcp_interface.md @@ -90,20 +90,26 @@ directory (it returns the restored thread summary). Fetch the catalog of models available in the current Codex build with `model/list`. The request accepts optional pagination inputs: -- `pageSize` – number of models to return (defaults to a server-selected value) +- `limit` – number of models to return (defaults to a server-selected value) - `cursor` – opaque string from the previous response’s `nextCursor` Each response yields: -- `items` – ordered list of models. A model includes: +- `data` – ordered list of models. A model includes: - `id`, `model`, `displayName`, `description` - `supportedReasoningEfforts` – array of objects with: - - `reasoningEffort` – one of `minimal|low|medium|high` + - `reasoningEffort` – one of `none|minimal|low|medium|high|xhigh` - `description` – human-friendly label for the effort - `defaultReasoningEffort` – suggested effort for the UI + - `inputModalities` – accepted input types for the model - `supportsPersonality` – whether the model supports personality-specific instructions - `isDefault` – whether the model is recommended for most users - `upgrade` – optional recommended upgrade model id + - `upgradeInfo` – optional upgrade metadata object with: + - `model` – recommended upgrade model id + - `upgradeCopy` – optional display copy for the upgrade recommendation + - `modelLink` – optional link for the upgrade recommendation + - `migrationMarkdown` – optional markdown shown when presenting the upgrade - `nextCursor` – pass into the next request to continue paging (optional) ## Collaboration modes (experimental) diff --git a/codex-rs/execpolicy/Cargo.toml b/codex-rs/execpolicy/Cargo.toml index 890c23faa7..2105ce27d5 100644 --- a/codex-rs/execpolicy/Cargo.toml +++ b/codex-rs/execpolicy/Cargo.toml @@ -19,6 +19,7 @@ workspace = true [dependencies] anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } +codex-utils-absolute-path = { workspace = true } multimap = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/execpolicy/README.md b/codex-rs/execpolicy/README.md index 30fc57184b..a57276767a 100644 --- a/codex-rs/execpolicy/README.md +++ b/codex-rs/execpolicy/README.md @@ -2,8 +2,8 @@ ## Overview -- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, justification?, match?, not_match?)`. -- This release covers the prefix-rule subset of the execpolicy language; a richer language will follow. +- Policy engine and CLI built around `prefix_rule(pattern=[...], decision?, justification?, match?, not_match?)` plus `host_executable(name=..., paths=[...])`. +- This release covers the prefix-rule subset of the execpolicy language plus host executable metadata; a richer language will follow. - Tokens are matched in order; any `pattern` element may be a list to denote alternatives. `decision` defaults to `allow`; valid values: `allow`, `prompt`, `forbidden`. - `justification` is an optional human-readable rationale for why a rule exists. It can be provided for any `decision` and may be surfaced in different contexts (for example, in approval prompts or rejection messages). When `decision = "forbidden"` is used, include a recommended alternative in the `justification`, when appropriate (e.g., ``"Use `jj` instead of `git`."``). - `match` / `not_match` supply example invocations that are validated at load time (think of them as unit tests); examples can be token arrays or strings (strings are tokenized with `shlex`). @@ -24,6 +24,27 @@ prefix_rule( ) ``` +- Host executable metadata can optionally constrain which absolute paths may + resolve through basename rules: + +```starlark +host_executable( + name = "git", + paths = [ + "/Users/example/.openai/bin/git", + "/opt/homebrew/bin/git", + "/usr/bin/git", + ], +) +``` + +- Matching semantics: + - execpolicy always tries exact first-token matches first. + - With host-executable resolution disabled, `/usr/bin/git status` only matches a rule whose first token is `/usr/bin/git`. + - With host-executable resolution enabled, if no exact rule matches, execpolicy may fall back from `/usr/bin/git` to basename rules for `git`. + - If `host_executable(name="git", ...)` exists, basename fallback is only allowed for listed absolute paths. + - If no `host_executable()` entry exists for a basename, basename fallback is allowed. + ## CLI - From the Codex CLI, run `codex execpolicy check` subcommand with one or more policy files (for example `src/default.rules`) to check a command: @@ -32,6 +53,15 @@ prefix_rule( codex execpolicy check --rules path/to/policy.rules git status ``` +- To opt into basename fallback for absolute program paths, pass `--resolve-host-executables`: + +```bash +codex execpolicy check \ + --rules path/to/policy.rules \ + --resolve-host-executables \ + /usr/bin/git status +``` + - Pass multiple `--rules` flags to merge rules, evaluated in the order provided, and use `--pretty` for formatted JSON. - You can also run the standalone dev binary directly during development: @@ -52,6 +82,7 @@ cargo run -p codex-execpolicy -- check --rules path/to/policy.rules git status "prefixRuleMatch": { "matchedPrefix": ["", "..."], "decision": "allow|prompt|forbidden", + "resolvedProgram": "/absolute/path/to/program", "justification": "..." } } @@ -62,6 +93,7 @@ cargo run -p codex-execpolicy -- check --rules path/to/policy.rules git status - When no rules match, `matchedRules` is an empty array and `decision` is omitted. - `matchedRules` lists every rule whose prefix matched the command; `matchedPrefix` is the exact prefix that matched. +- `resolvedProgram` is omitted unless an absolute executable path matched via basename fallback. - The effective `decision` is the strictest severity across all matches (`forbidden` > `prompt` > `allow`). Note: `execpolicy` commands are still in preview. The API may have breaking changes in the future. diff --git a/codex-rs/execpolicy/src/execpolicycheck.rs b/codex-rs/execpolicy/src/execpolicycheck.rs index f191f4b12e..1ff8937a5d 100644 --- a/codex-rs/execpolicy/src/execpolicycheck.rs +++ b/codex-rs/execpolicy/src/execpolicycheck.rs @@ -7,6 +7,7 @@ use clap::Parser; use serde::Serialize; use crate::Decision; +use crate::MatchOptions; use crate::Policy; use crate::PolicyParser; use crate::RuleMatch; @@ -22,6 +23,11 @@ pub struct ExecPolicyCheckCommand { #[arg(long)] pub pretty: bool, + /// Resolve absolute program paths against basename rules, gated by any + /// `host_executable()` definitions in the loaded policy files. + #[arg(long)] + pub resolve_host_executables: bool, + /// Command tokens to check against the policy. #[arg( value_name = "COMMAND", @@ -36,7 +42,13 @@ impl ExecPolicyCheckCommand { /// Load the policies for this command, evaluate the command, and render JSON output. pub fn run(&self) -> Result<()> { let policy = load_policies(&self.rules)?; - let matched_rules = policy.matches_for_command(&self.command, None); + let matched_rules = policy.matches_for_command_with_options( + &self.command, + None, + &MatchOptions { + resolve_host_executables: self.resolve_host_executables, + }, + ); let json = format_matches_json(&matched_rules, self.pretty)?; println!("{json}"); diff --git a/codex-rs/execpolicy/src/lib.rs b/codex-rs/execpolicy/src/lib.rs index a9e9faa56d..13dc48f119 100644 --- a/codex-rs/execpolicy/src/lib.rs +++ b/codex-rs/execpolicy/src/lib.rs @@ -18,6 +18,7 @@ pub use error::TextRange; pub use execpolicycheck::ExecPolicyCheckCommand; pub use parser::PolicyParser; pub use policy::Evaluation; +pub use policy::MatchOptions; pub use policy::Policy; pub use rule::NetworkRuleProtocol; pub use rule::Rule; diff --git a/codex-rs/execpolicy/src/parser.rs b/codex-rs/execpolicy/src/parser.rs index 8490c4848c..e9efdb7824 100644 --- a/codex-rs/execpolicy/src/parser.rs +++ b/codex-rs/execpolicy/src/parser.rs @@ -1,3 +1,4 @@ +use codex_utils_absolute_path::AbsolutePathBuf; use multimap::MultiMap; use shlex; use starlark::any::ProvidesStaticType; @@ -13,6 +14,8 @@ use starlark::values::list::UnpackList; use starlark::values::none::NoneType; use std::cell::RefCell; use std::cell::RefMut; +use std::collections::HashMap; +use std::path::Path; use std::sync::Arc; use crate::decision::Decision; @@ -74,6 +77,7 @@ impl PolicyParser { struct PolicyBuilder { rules_by_program: MultiMap, network_rules: Vec, + host_executables_by_name: HashMap>, } impl PolicyBuilder { @@ -81,6 +85,7 @@ impl PolicyBuilder { Self { rules_by_program: MultiMap::new(), network_rules: Vec::new(), + host_executables_by_name: HashMap::new(), } } @@ -93,8 +98,16 @@ impl PolicyBuilder { self.network_rules.push(rule); } + fn add_host_executable(&mut self, name: String, paths: Vec) { + self.host_executables_by_name.insert(name, paths.into()); + } + fn build(self) -> crate::policy::Policy { - crate::policy::Policy::from_parts(self.rules_by_program, self.network_rules) + crate::policy::Policy::from_parts( + self.rules_by_program, + self.network_rules, + self.host_executables_by_name, + ) } } @@ -150,6 +163,36 @@ fn parse_examples<'v>(examples: UnpackList>) -> Result examples.items.into_iter().map(parse_example).collect() } +fn parse_literal_absolute_path(raw: &str) -> Result { + if !Path::new(raw).is_absolute() { + return Err(Error::InvalidRule(format!( + "host_executable paths must be absolute (got {raw})" + ))); + } + + AbsolutePathBuf::try_from(raw.to_string()) + .map_err(|error| Error::InvalidRule(format!("invalid absolute path `{raw}`: {error}"))) +} + +fn validate_host_executable_name(name: &str) -> Result<()> { + if name.is_empty() { + return Err(Error::InvalidRule( + "host_executable name cannot be empty".to_string(), + )); + } + + let path = Path::new(name); + if path.components().count() != 1 + || path.file_name().and_then(|value| value.to_str()) != Some(name) + { + return Err(Error::InvalidRule(format!( + "host_executable name must be a bare executable name (got {name})" + ))); + } + + Ok(()) +} + fn parse_network_rule_decision(raw: &str) -> Result { match raw { "deny" => Ok(Decision::Forbidden), @@ -308,4 +351,35 @@ fn policy_builtins(builder: &mut GlobalsBuilder) { }); Ok(NoneType) } + + fn host_executable<'v>( + name: &'v str, + paths: UnpackList>, + eval: &mut Evaluator<'v, '_, '_>, + ) -> anyhow::Result { + validate_host_executable_name(name)?; + + let mut parsed_paths = Vec::new(); + for value in paths.items { + let raw = value.unpack_str().ok_or_else(|| { + Error::InvalidRule(format!( + "host_executable paths must be strings (got {})", + value.get_type() + )) + })?; + let path = parse_literal_absolute_path(raw)?; + if path.as_path().file_name().and_then(|value| value.to_str()) != Some(name) { + return Err(Error::InvalidRule(format!( + "host_executable path `{raw}` must have basename `{name}`" + )) + .into()); + } + if !parsed_paths.iter().any(|existing| existing == &path) { + parsed_paths.push(path); + } + } + + policy_builder(eval).add_host_executable(name.to_string(), parsed_paths); + Ok(NoneType) + } } diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index faa74cc517..86a2cf54eb 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -9,31 +9,42 @@ use crate::rule::PrefixRule; use crate::rule::RuleMatch; use crate::rule::RuleRef; use crate::rule::normalize_network_rule_host; +use codex_utils_absolute_path::AbsolutePathBuf; use multimap::MultiMap; use serde::Deserialize; use serde::Serialize; +use std::collections::HashMap; +use std::path::Path; use std::sync::Arc; type HeuristicsFallback<'a> = Option<&'a dyn Fn(&[String]) -> Decision>; +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct MatchOptions { + pub resolve_host_executables: bool, +} + #[derive(Clone, Debug)] pub struct Policy { rules_by_program: MultiMap, network_rules: Vec, + host_executables_by_name: HashMap>, } impl Policy { pub fn new(rules_by_program: MultiMap) -> Self { - Self::from_parts(rules_by_program, Vec::new()) + Self::from_parts(rules_by_program, Vec::new(), HashMap::new()) } pub fn from_parts( rules_by_program: MultiMap, network_rules: Vec, + host_executables_by_name: HashMap>, ) -> Self { Self { rules_by_program, network_rules, + host_executables_by_name, } } @@ -49,6 +60,10 @@ impl Policy { &self.network_rules } + pub fn host_executables(&self) -> &HashMap> { + &self.host_executables_by_name + } + pub fn get_allowed_prefixes(&self) -> Vec> { let mut prefixes = Vec::new(); @@ -119,6 +134,36 @@ impl Policy { Ok(()) } + pub fn set_host_executable_paths(&mut self, name: String, paths: Vec) { + self.host_executables_by_name.insert(name, paths.into()); + } + + pub fn merge_overlay(&self, overlay: &Policy) -> Policy { + let mut combined_rules = self.rules_by_program.clone(); + for (program, rules) in overlay.rules_by_program.iter_all() { + for rule in rules { + combined_rules.insert(program.clone(), rule.clone()); + } + } + + let mut combined_network_rules = self.network_rules.clone(); + combined_network_rules.extend(overlay.network_rules.iter().cloned()); + + let mut host_executables_by_name = self.host_executables_by_name.clone(); + host_executables_by_name.extend( + overlay + .host_executables_by_name + .iter() + .map(|(name, paths)| (name.clone(), paths.clone())), + ); + + Policy::from_parts( + combined_rules, + combined_network_rules, + host_executables_by_name, + ) + } + pub fn compiled_network_domains(&self) -> (Vec, Vec) { let mut allowed = Vec::new(); let mut denied = Vec::new(); @@ -144,7 +189,25 @@ impl Policy { where F: Fn(&[String]) -> Decision, { - let matched_rules = self.matches_for_command(cmd, Some(heuristics_fallback)); + let matched_rules = self.matches_for_command_with_options( + cmd, + Some(heuristics_fallback), + &MatchOptions::default(), + ); + Evaluation::from_matches(matched_rules) + } + + pub fn check_with_options( + &self, + cmd: &[String], + heuristics_fallback: &F, + options: &MatchOptions, + ) -> Evaluation + where + F: Fn(&[String]) -> Decision, + { + let matched_rules = + self.matches_for_command_with_options(cmd, Some(heuristics_fallback), options); Evaluation::from_matches(matched_rules) } @@ -154,6 +217,20 @@ impl Policy { commands: Commands, heuristics_fallback: &F, ) -> Evaluation + where + Commands: IntoIterator, + Commands::Item: AsRef<[String]>, + F: Fn(&[String]) -> Decision, + { + self.check_multiple_with_options(commands, heuristics_fallback, &MatchOptions::default()) + } + + pub fn check_multiple_with_options( + &self, + commands: Commands, + heuristics_fallback: &F, + options: &MatchOptions, + ) -> Evaluation where Commands: IntoIterator, Commands::Item: AsRef<[String]>, @@ -162,7 +239,11 @@ impl Policy { let matched_rules: Vec = commands .into_iter() .flat_map(|command| { - self.matches_for_command(command.as_ref(), Some(heuristics_fallback)) + self.matches_for_command_with_options( + command.as_ref(), + Some(heuristics_fallback), + options, + ) }) .collect(); @@ -181,14 +262,25 @@ impl Policy { cmd: &[String], heuristics_fallback: HeuristicsFallback<'_>, ) -> Vec { - let matched_rules: Vec = match cmd.first() { - Some(first) => self - .rules_by_program - .get_vec(first) - .map(|rules| rules.iter().filter_map(|rule| rule.matches(cmd)).collect()) - .unwrap_or_default(), - None => Vec::new(), - }; + self.matches_for_command_with_options(cmd, heuristics_fallback, &MatchOptions::default()) + } + + pub fn matches_for_command_with_options( + &self, + cmd: &[String], + heuristics_fallback: HeuristicsFallback<'_>, + options: &MatchOptions, + ) -> Vec { + let matched_rules = self + .match_exact_rules(cmd) + .filter(|matched_rules| !matched_rules.is_empty()) + .or_else(|| { + options + .resolve_host_executables + .then(|| self.match_host_executable_rules(cmd)) + .filter(|matched_rules| !matched_rules.is_empty()) + }) + .unwrap_or_default(); if matched_rules.is_empty() && let Some(heuristics_fallback) = heuristics_fallback @@ -201,6 +293,49 @@ impl Policy { matched_rules } } + + fn match_exact_rules(&self, cmd: &[String]) -> Option> { + let first = cmd.first()?; + Some( + self.rules_by_program + .get_vec(first) + .map(|rules| rules.iter().filter_map(|rule| rule.matches(cmd)).collect()) + .unwrap_or_default(), + ) + } + + fn match_host_executable_rules(&self, cmd: &[String]) -> Vec { + let Some(first) = cmd.first() else { + return Vec::new(); + }; + if !Path::new(first).is_absolute() { + return Vec::new(); + } + + let Ok(program) = AbsolutePathBuf::try_from(first.clone()) else { + return Vec::new(); + }; + let Some(basename) = program.as_path().file_name().and_then(|name| name.to_str()) else { + return Vec::new(); + }; + let Some(rules) = self.rules_by_program.get_vec(basename) else { + return Vec::new(); + }; + if let Some(paths) = self.host_executables_by_name.get(basename) + && !paths.iter().any(|path| path == &program) + { + return Vec::new(); + } + + let basename_command = std::iter::once(basename.to_string()) + .chain(cmd.iter().skip(1).cloned()) + .collect::>(); + rules + .iter() + .filter_map(|rule| rule.matches(&basename_command)) + .map(|rule_match| rule_match.with_resolved_program(&program)) + .collect() + } } fn upsert_domain(entries: &mut Vec, host: &str) { diff --git a/codex-rs/execpolicy/src/rule.rs b/codex-rs/execpolicy/src/rule.rs index 0d8bfc243a..0c92b55070 100644 --- a/codex-rs/execpolicy/src/rule.rs +++ b/codex-rs/execpolicy/src/rule.rs @@ -1,6 +1,7 @@ use crate::decision::Decision; use crate::error::Error; use crate::error::Result; +use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde::Serialize; use shlex::try_join; @@ -63,6 +64,8 @@ pub enum RuleMatch { #[serde(rename = "matchedPrefix")] matched_prefix: Vec, decision: Decision, + #[serde(rename = "resolvedProgram", skip_serializing_if = "Option::is_none")] + resolved_program: Option, /// Optional rationale for why this rule exists. /// /// This can be supplied for any decision and may be surfaced in different contexts @@ -83,6 +86,23 @@ impl RuleMatch { Self::HeuristicsRuleMatch { decision, .. } => *decision, } } + + pub fn with_resolved_program(self, resolved_program: &AbsolutePathBuf) -> Self { + match self { + Self::PrefixRuleMatch { + matched_prefix, + decision, + justification, + .. + } => Self::PrefixRuleMatch { + matched_prefix, + decision, + resolved_program: Some(resolved_program.clone()), + justification, + }, + other => other, + } + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -210,6 +230,7 @@ impl Rule for PrefixRule { .map(|matched_prefix| RuleMatch::PrefixRuleMatch { matched_prefix, decision: self.decision, + resolved_program: None, justification: self.justification.clone(), }) } diff --git a/codex-rs/execpolicy/tests/basic.rs b/codex-rs/execpolicy/tests/basic.rs index eec7cec651..9e1aa4c38b 100644 --- a/codex-rs/execpolicy/tests/basic.rs +++ b/codex-rs/execpolicy/tests/basic.rs @@ -1,5 +1,6 @@ use std::any::Any; use std::fs; +use std::path::PathBuf; use std::sync::Arc; use anyhow::Context; @@ -7,6 +8,7 @@ use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Error; use codex_execpolicy::Evaluation; +use codex_execpolicy::MatchOptions; use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; @@ -16,6 +18,7 @@ use codex_execpolicy::blocking_append_allow_prefix_rule; use codex_execpolicy::rule::PatternToken; use codex_execpolicy::rule::PrefixPattern; use codex_execpolicy::rule::PrefixRule; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -31,6 +34,27 @@ fn prompt_all(_: &[String]) -> Decision { Decision::Prompt } +fn absolute_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::try_from(path.to_string()) + .unwrap_or_else(|error| panic!("expected absolute path `{path}`: {error}")) +} + +fn host_absolute_path(segments: &[&str]) -> String { + let mut path = if cfg!(windows) { + PathBuf::from(r"C:\") + } else { + PathBuf::from("/") + }; + for segment in segments { + path.push(segment); + } + path.to_string_lossy().into_owned() +} + +fn starlark_string(value: &str) -> String { + value.replace('\\', "\\\\").replace('"', "\\\"") +} + #[derive(Clone, Debug, Eq, PartialEq)] enum RuleSnapshot { Prefix(PrefixRule), @@ -125,6 +149,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git", "status"]), decision: Decision::Allow, + resolved_program: None, justification: None, }], }, @@ -156,6 +181,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["rm"]), decision: Decision::Forbidden, + resolved_program: None, justification: Some("destructive command".to_string()), }], }, @@ -184,6 +210,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["ls"]), decision: Decision::Allow, + resolved_program: None, justification: Some("safe and commonly used".to_string()), }], }, @@ -236,6 +263,7 @@ fn add_prefix_rule_extends_policy() -> Result<()> { matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["ls", "-l"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }], }, @@ -305,6 +333,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }], }, @@ -319,11 +348,13 @@ prefix_rule( RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }, RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git", "commit"]), decision: Decision::Forbidden, + resolved_program: None, justification: None, }, ], @@ -381,6 +412,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["bash", "-c"]), decision: Decision::Allow, + resolved_program: None, justification: None, }], }, @@ -394,6 +426,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["sh", "-l"]), decision: Decision::Allow, + resolved_program: None, justification: None, }], }, @@ -440,6 +473,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["npm", "i", "--legacy-peer-deps"]), decision: Decision::Allow, + resolved_program: None, justification: None, }], }, @@ -456,6 +490,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["npm", "install", "--no-save"]), decision: Decision::Allow, + resolved_program: None, justification: None, }], }, @@ -486,6 +521,7 @@ prefix_rule( matched_rules: vec![RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git", "status"]), decision: Decision::Allow, + resolved_program: None, justification: None, }], }, @@ -533,11 +569,13 @@ prefix_rule( RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }, RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git", "commit"]), decision: Decision::Forbidden, + resolved_program: None, justification: None, }, ], @@ -576,16 +614,19 @@ prefix_rule( RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }, RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git"]), decision: Decision::Prompt, + resolved_program: None, justification: None, }, RuleMatch::PrefixRuleMatch { matched_prefix: tokens(&["git", "commit"]), decision: Decision::Forbidden, + resolved_program: None, justification: None, }, ], @@ -612,3 +653,278 @@ fn heuristics_match_is_returned_when_no_policy_matches() { evaluation ); } + +#[test] +fn parses_host_executable_paths() -> Result<()> { + let homebrew_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]); + let usr_git = host_absolute_path(&["usr", "bin", "git"]); + let homebrew_git_literal = starlark_string(&homebrew_git); + let usr_git_literal = starlark_string(&usr_git); + let policy_src = format!( + r#" +host_executable( + name = "git", + paths = [ + "{homebrew_git_literal}", + "{usr_git_literal}", + "{usr_git_literal}", + ], +) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src)?; + let policy = parser.build(); + + assert_eq!( + policy + .host_executables() + .get("git") + .expect("missing git host executable") + .as_ref(), + [absolute_path(&homebrew_git), absolute_path(&usr_git)] + ); + Ok(()) +} + +#[test] +fn host_executable_rejects_non_absolute_path() { + let policy_src = r#" +host_executable(name = "git", paths = ["git"]) +"#; + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", policy_src) + .expect_err("expected parse error"); + assert!( + err.to_string() + .contains("host_executable paths must be absolute") + ); +} + +#[test] +fn host_executable_rejects_name_with_path_separator() { + let git_path = host_absolute_path(&["usr", "bin", "git"]); + let git_path_literal = starlark_string(&git_path); + let policy_src = + format!(r#"host_executable(name = "{git_path_literal}", paths = ["{git_path_literal}"])"#); + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", &policy_src) + .expect_err("expected parse error"); + assert!( + err.to_string() + .contains("host_executable name must be a bare executable name") + ); +} + +#[test] +fn host_executable_rejects_path_with_wrong_basename() { + let rg_path = host_absolute_path(&["usr", "bin", "rg"]); + let rg_path_literal = starlark_string(&rg_path); + let policy_src = format!(r#"host_executable(name = "git", paths = ["{rg_path_literal}"])"#); + let mut parser = PolicyParser::new(); + let err = parser + .parse("test.rules", &policy_src) + .expect_err("expected parse error"); + assert!(err.to_string().contains("must have basename `git`")); +} + +#[test] +fn host_executable_last_definition_wins() -> Result<()> { + let usr_git = host_absolute_path(&["usr", "bin", "git"]); + let homebrew_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]); + let usr_git_literal = starlark_string(&usr_git); + let homebrew_git_literal = starlark_string(&homebrew_git); + let mut parser = PolicyParser::new(); + parser.parse( + "shared.rules", + &format!(r#"host_executable(name = "git", paths = ["{usr_git_literal}"])"#), + )?; + parser.parse( + "user.rules", + &format!(r#"host_executable(name = "git", paths = ["{homebrew_git_literal}"])"#), + )?; + let policy = parser.build(); + + assert_eq!( + policy + .host_executables() + .get("git") + .expect("missing git host executable") + .as_ref(), + [absolute_path(&homebrew_git)] + ); + Ok(()) +} + +#[test] +fn host_executable_resolution_uses_basename_rule_when_allowed() -> Result<()> { + let git_path = host_absolute_path(&["usr", "bin", "git"]); + let git_path_literal = starlark_string(&git_path); + let policy_src = format!( + r#" +prefix_rule(pattern = ["git", "status"], decision = "prompt") +host_executable(name = "git", paths = ["{git_path_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src)?; + let policy = parser.build(); + + let evaluation = policy.check_with_options( + &[git_path.clone(), "status".to_string()], + &allow_all, + &MatchOptions { + resolve_host_executables: true, + }, + ); + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git", "status"]), + decision: Decision::Prompt, + resolved_program: Some(absolute_path(&git_path)), + justification: None, + }], + } + ); + Ok(()) +} + +#[test] +fn host_executable_resolution_respects_explicit_empty_allowlist() -> Result<()> { + let policy_src = r#" +prefix_rule(pattern = ["git"], decision = "prompt") +host_executable(name = "git", paths = []) +"#; + let mut parser = PolicyParser::new(); + parser.parse("test.rules", policy_src)?; + let policy = parser.build(); + let git_path = host_absolute_path(&["usr", "bin", "git"]); + + let evaluation = policy.check_with_options( + &[git_path.clone(), "status".to_string()], + &allow_all, + &MatchOptions { + resolve_host_executables: true, + }, + ); + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::HeuristicsRuleMatch { + command: vec![git_path, "status".to_string()], + decision: Decision::Allow, + }], + } + ); + Ok(()) +} + +#[test] +fn host_executable_resolution_ignores_path_not_in_allowlist() -> Result<()> { + let allowed_git = host_absolute_path(&["usr", "bin", "git"]); + let other_git = host_absolute_path(&["opt", "homebrew", "bin", "git"]); + let allowed_git_literal = starlark_string(&allowed_git); + let policy_src = format!( + r#" +prefix_rule(pattern = ["git"], decision = "prompt") +host_executable(name = "git", paths = ["{allowed_git_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src)?; + let policy = parser.build(); + + let evaluation = policy.check_with_options( + &[other_git.clone(), "status".to_string()], + &allow_all, + &MatchOptions { + resolve_host_executables: true, + }, + ); + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::HeuristicsRuleMatch { + command: vec![other_git, "status".to_string()], + decision: Decision::Allow, + }], + } + ); + Ok(()) +} + +#[test] +fn host_executable_resolution_falls_back_without_mapping() -> Result<()> { + let policy_src = r#" +prefix_rule(pattern = ["git"], decision = "prompt") +"#; + let mut parser = PolicyParser::new(); + parser.parse("test.rules", policy_src)?; + let policy = parser.build(); + let git_path = host_absolute_path(&["usr", "bin", "git"]); + + let evaluation = policy.check_with_options( + &[git_path.clone(), "status".to_string()], + &allow_all, + &MatchOptions { + resolve_host_executables: true, + }, + ); + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Prompt, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: tokens(&["git"]), + decision: Decision::Prompt, + resolved_program: Some(absolute_path(&git_path)), + justification: None, + }], + } + ); + Ok(()) +} + +#[test] +fn host_executable_resolution_does_not_override_exact_match() -> Result<()> { + let git_path = host_absolute_path(&["usr", "bin", "git"]); + let git_path_literal = starlark_string(&git_path); + let policy_src = format!( + r#" +prefix_rule(pattern = ["{git_path_literal}"], decision = "allow") +prefix_rule(pattern = ["git"], decision = "prompt") +host_executable(name = "git", paths = ["{git_path_literal}"]) +"# + ); + let mut parser = PolicyParser::new(); + parser.parse("test.rules", &policy_src)?; + let policy = parser.build(); + + let evaluation = policy.check_with_options( + &[git_path.clone(), "status".to_string()], + &allow_all, + &MatchOptions { + resolve_host_executables: true, + }, + ); + assert_eq!( + evaluation, + Evaluation { + decision: Decision::Allow, + matched_rules: vec![RuleMatch::PrefixRuleMatch { + matched_prefix: vec![git_path], + decision: Decision::Allow, + resolved_program: None, + justification: None, + }], + } + ); + Ok(()) +} diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 6568f5374f..1648fa0462 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -104,6 +104,7 @@ mod tests { HookPayload { session_id: ThreadId::new(), cwd: PathBuf::from(CWD), + client: None, triggered_at: Utc .with_ymd_and_hms(2025, 1, 1, 0, 0, 0) .single() @@ -172,6 +173,7 @@ mod tests { HookPayload { session_id: ThreadId::new(), cwd: PathBuf::from(CWD), + client: None, triggered_at: Utc .with_ymd_and_hms(2025, 1, 1, 0, 0, 0) .single() diff --git a/codex-rs/hooks/src/types.rs b/codex-rs/hooks/src/types.rs index 5131580128..b7ef3c2141 100644 --- a/codex-rs/hooks/src/types.rs +++ b/codex-rs/hooks/src/types.rs @@ -65,6 +65,8 @@ impl Hook { pub struct HookPayload { pub session_id: ThreadId, pub cwd: PathBuf, + #[serde(skip_serializing_if = "Option::is_none")] + pub client: Option, #[serde(serialize_with = "serialize_triggered_at")] pub triggered_at: DateTime, pub hook_event: HookEvent, @@ -181,6 +183,7 @@ mod tests { let payload = HookPayload { session_id, cwd: PathBuf::from("tmp"), + client: None, triggered_at: Utc .with_ymd_and_hms(2025, 1, 1, 0, 0, 0) .single() @@ -218,6 +221,7 @@ mod tests { let payload = HookPayload { session_id, cwd: PathBuf::from("tmp"), + client: None, triggered_at: Utc .with_ymd_and_hms(2025, 1, 1, 0, 0, 0) .single() diff --git a/codex-rs/hooks/src/user_notification.rs b/codex-rs/hooks/src/user_notification.rs index 50472225e4..caeca59b50 100644 --- a/codex-rs/hooks/src/user_notification.rs +++ b/codex-rs/hooks/src/user_notification.rs @@ -1,4 +1,3 @@ -use std::path::Path; use std::process::Stdio; use std::sync::Arc; @@ -19,6 +18,8 @@ enum UserNotification { thread_id: String, turn_id: String, cwd: String, + #[serde(skip_serializing_if = "Option::is_none")] + client: Option, /// Messages that the user sent to the agent to initiate the turn. input_messages: Vec, @@ -28,13 +29,14 @@ enum UserNotification { }, } -pub fn legacy_notify_json(hook_event: &HookEvent, cwd: &Path) -> Result { - match hook_event { +pub fn legacy_notify_json(payload: &HookPayload) -> Result { + match &payload.hook_event { HookEvent::AfterAgent { event } => { serde_json::to_string(&UserNotification::AgentTurnComplete { thread_id: event.thread_id.to_string(), turn_id: event.turn_id.clone(), - cwd: cwd.display().to_string(), + cwd: payload.cwd.display().to_string(), + client: payload.client.clone(), input_messages: event.input_messages.clone(), last_assistant_message: event.last_assistant_message.clone(), }) @@ -56,7 +58,7 @@ pub fn notify_hook(argv: Vec) -> Hook { Some(command) => command, None => return HookResult::Success, }; - if let Ok(notify_payload) = legacy_notify_json(&payload.hook_event, &payload.cwd) { + if let Ok(notify_payload) = legacy_notify_json(payload) { command.arg(notify_payload); } @@ -91,6 +93,7 @@ mod tests { "thread-id": "b5f6c1c2-1111-2222-3333-444455556666", "turn-id": "12345", "cwd": "/Users/example/project", + "client": "codex-tui", "input-messages": ["Rename `foo` to `bar` and update the callsites."], "last-assistant-message": "Rename complete and verified `cargo build` succeeds.", }) @@ -102,6 +105,7 @@ mod tests { thread_id: "b5f6c1c2-1111-2222-3333-444455556666".to_string(), turn_id: "12345".to_string(), cwd: "/Users/example/project".to_string(), + client: Some("codex-tui".to_string()), input_messages: vec!["Rename `foo` to `bar` and update the callsites.".to_string()], last_assistant_message: Some( "Rename complete and verified `cargo build` succeeds.".to_string(), @@ -115,19 +119,27 @@ mod tests { #[test] fn legacy_notify_json_matches_historical_wire_shape() -> Result<()> { - let hook_event = HookEvent::AfterAgent { - event: crate::HookEventAfterAgent { - thread_id: ThreadId::from_string("b5f6c1c2-1111-2222-3333-444455556666") - .expect("valid thread id"), - turn_id: "12345".to_string(), - input_messages: vec!["Rename `foo` to `bar` and update the callsites.".to_string()], - last_assistant_message: Some( - "Rename complete and verified `cargo build` succeeds.".to_string(), - ), + let payload = HookPayload { + session_id: ThreadId::new(), + cwd: std::path::Path::new("/Users/example/project").to_path_buf(), + client: Some("codex-tui".to_string()), + triggered_at: chrono::Utc::now(), + hook_event: HookEvent::AfterAgent { + event: crate::HookEventAfterAgent { + thread_id: ThreadId::from_string("b5f6c1c2-1111-2222-3333-444455556666") + .expect("valid thread id"), + turn_id: "12345".to_string(), + input_messages: vec![ + "Rename `foo` to `bar` and update the callsites.".to_string(), + ], + last_assistant_message: Some( + "Rename complete and verified `cargo build` succeeds.".to_string(), + ), + }, }, }; - let serialized = legacy_notify_json(&hook_event, Path::new("/Users/example/project"))?; + let serialized = legacy_notify_json(&payload)?; let actual: Value = serde_json::from_str(&serialized)?; assert_eq!(actual, expected_notification_json()); diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index ee58d46e68..cf06636ad8 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -161,7 +161,7 @@ pub enum ResponseInputItem { }, CustomToolCallOutput { call_id: String, - output: String, + output: FunctionCallOutputPayload, }, } @@ -261,9 +261,12 @@ pub enum ResponseItem { name: String, input: String, }, + // `custom_tool_call_output.output` uses the same wire encoding as + // `function_call_output.output` so freeform tools can return either plain + // text or structured content items. CustomToolCallOutput { call_id: String, - output: String, + output: FunctionCallOutputPayload, }, // Emitted by the Responses API when the agent triggers a web search. // Example payload (from SSE `response.output_item.done`): @@ -1538,6 +1541,26 @@ mod tests { Ok(()) } + #[test] + fn serializes_custom_tool_image_outputs_as_array() -> Result<()> { + let item = ResponseInputItem::CustomToolCallOutput { + call_id: "call1".into(), + output: FunctionCallOutputPayload::from_content_items(vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,BASE64".into(), + }, + ]), + }; + + let json = serde_json::to_string(&item)?; + let v: serde_json::Value = serde_json::from_str(&json)?; + + let output = v.get("output").expect("output field"); + assert!(output.is_array(), "expected array output"); + + Ok(()) + } + #[test] fn preserves_existing_image_data_urls() -> Result<()> { let call_tool_result = CallToolResult { diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 9b0f25e27c..b63f137442 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -99,6 +99,11 @@ pub struct ModelUpgrade { pub migration_markdown: Option, } +#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema, PartialEq, Eq)] +pub struct ModelAvailabilityNux { + pub message: String, +} + /// Metadata describing a Codex-supported model. #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema, PartialEq)] pub struct ModelPreset { @@ -123,6 +128,8 @@ pub struct ModelPreset { pub upgrade: Option, /// Whether this preset should appear in the picker UI. pub show_in_picker: bool, + /// Availability NUX shown when this preset becomes accessible to the user. + pub availability_nux: Option, /// whether this model is supported in the api pub supported_in_api: bool, /// Input modalities accepted when composing user turns for this preset. @@ -225,6 +232,7 @@ pub struct ModelInfo { pub visibility: ModelVisibility, pub supported_in_api: bool, pub priority: i32, + pub availability_nux: Option, pub upgrade: Option, pub base_instructions: String, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -410,6 +418,7 @@ impl From for ModelPreset { migration_markdown: Some(upgrade.migration_markdown.clone()), }), show_in_picker: info.visibility == ModelVisibility::List, + availability_nux: info.availability_nux, supported_in_api: info.supported_in_api, input_modalities: info.input_modalities, } @@ -495,6 +504,7 @@ mod tests { visibility: ModelVisibility::List, supported_in_api: true, priority: 1, + availability_nux: None, upgrade: None, base_instructions: "base".to_string(), model_messages: spec, @@ -668,4 +678,57 @@ mod tests { ); assert_eq!(personality_variables.get_personality_message(None), None); } + + #[test] + fn model_info_defaults_availability_nux_to_none_when_omitted() { + let model: ModelInfo = serde_json::from_value(serde_json::json!({ + "slug": "test-model", + "display_name": "Test Model", + "description": null, + "supported_reasoning_levels": [], + "shell_type": "shell_command", + "visibility": "list", + "supported_in_api": true, + "priority": 1, + "upgrade": null, + "base_instructions": "base", + "model_messages": null, + "supports_reasoning_summaries": false, + "default_reasoning_summary": "auto", + "support_verbosity": false, + "default_verbosity": null, + "apply_patch_tool_type": null, + "truncation_policy": { + "mode": "bytes", + "limit": 10000 + }, + "supports_parallel_tool_calls": false, + "context_window": null, + "auto_compact_token_limit": null, + "effective_context_window_percent": 95, + "experimental_supported_tools": [], + "input_modalities": ["text", "image"], + "prefer_websockets": false + })) + .expect("deserialize model info"); + + assert_eq!(model.availability_nux, None); + } + + #[test] + fn model_preset_preserves_availability_nux() { + let preset = ModelPreset::from(ModelInfo { + availability_nux: Some(ModelAvailabilityNux { + message: "Try Spark.".to_string(), + }), + ..test_model(None) + }); + + assert_eq!( + preset.availability_nux, + Some(ModelAvailabilityNux { + message: "Try Spark.".to_string(), + }) + ); + } } diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index 62a9c3b019..c71799c629 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -47,6 +47,7 @@ pub async fn perform_oauth_login( http_headers: Option>, env_http_headers: Option>, scopes: &[String], + oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, ) -> Result<()> { @@ -60,6 +61,7 @@ pub async fn perform_oauth_login( store_mode, headers, scopes, + oauth_resource, true, callback_port, callback_url, @@ -78,6 +80,7 @@ pub async fn perform_oauth_login_return_url( http_headers: Option>, env_http_headers: Option>, scopes: &[String], + oauth_resource: Option<&str>, timeout_secs: Option, callback_port: Option, callback_url: Option<&str>, @@ -92,6 +95,7 @@ pub async fn perform_oauth_login_return_url( store_mode, headers, scopes, + oauth_resource, false, callback_port, callback_url, @@ -303,6 +307,7 @@ impl OauthLoginFlow { store_mode: OAuthCredentialsStoreMode, headers: OauthHeaders, scopes: &[String], + oauth_resource: Option<&str>, launch_browser: bool, callback_port: Option, callback_url: Option<&str>, @@ -340,7 +345,11 @@ impl OauthLoginFlow { oauth_state .start_authorization(&scope_refs, &redirect_uri, Some("Codex")) .await?; - let auth_url = oauth_state.get_authorization_url().await?; + let auth_url = append_query_param( + &oauth_state.get_authorization_url().await?, + "resource", + oauth_resource, + ); let timeout_secs = timeout_secs.unwrap_or(DEFAULT_OAUTH_TIMEOUT_SECS).max(1); let timeout = Duration::from_secs(timeout_secs as u64); @@ -431,9 +440,29 @@ impl OauthLoginFlow { } } +fn append_query_param(url: &str, key: &str, value: Option<&str>) -> String { + let Some(value) = value else { + return url.to_string(); + }; + let value = value.trim(); + if value.is_empty() { + return url.to_string(); + } + if let Ok(mut parsed) = Url::parse(url) { + parsed.query_pairs_mut().append_pair(key, value); + return parsed.to_string(); + } + let encoded = urlencoding::encode(value); + let separator = if url.contains('?') { "&" } else { "?" }; + format!("{url}{separator}{key}={encoded}") +} + #[cfg(test)] mod tests { + use pretty_assertions::assert_eq; + use super::CallbackOutcome; + use super::append_query_param; use super::callback_path_from_redirect_uri; use super::parse_oauth_callback; @@ -461,4 +490,36 @@ mod tests { .expect("redirect URI should parse"); assert_eq!(path, "/oauth/callback"); } + + #[test] + fn append_query_param_adds_resource_to_absolute_url() { + let url = append_query_param( + "https://example.com/authorize?scope=read", + "resource", + Some("https://api.example.com"), + ); + + assert_eq!( + url, + "https://example.com/authorize?scope=read&resource=https%3A%2F%2Fapi.example.com" + ); + } + + #[test] + fn append_query_param_ignores_empty_values() { + let url = append_query_param( + "https://example.com/authorize?scope=read", + "resource", + Some(" "), + ); + + assert_eq!(url, "https://example.com/authorize?scope=read"); + } + + #[test] + fn append_query_param_handles_unparseable_url() { + let url = append_query_param("not a url", "resource", Some("api/resource")); + + assert_eq!(url, "not a url?resource=api%2Fresource"); + } } diff --git a/codex-rs/state/src/runtime/memories.rs b/codex-rs/state/src/runtime/memories.rs index 5ebb2be88f..908919d0aa 100644 --- a/codex-rs/state/src/runtime/memories.rs +++ b/codex-rs/state/src/runtime/memories.rs @@ -97,6 +97,7 @@ WHERE thread_id = ? /// Query behavior: /// - starts from `threads` filtered to active threads and allowed sources /// (`push_thread_filters`) + /// - excludes threads with `memory_mode != 'enabled'` /// - excludes the current thread id /// - keeps only threads in the age window: /// `updated_at >= now - max_age_days` and `updated_at <= now - min_rollout_idle_hours` @@ -174,6 +175,7 @@ LEFT JOIN jobs SortKey::UpdatedAt, None, ); + builder.push(" AND threads.memory_mode = 'enabled'"); builder .push(" AND id != ") .push_bind(current_thread_id.as_str()); @@ -1591,6 +1593,77 @@ mod tests { let _ = tokio::fs::remove_dir_all(codex_home).await; } + #[tokio::test] + async fn claim_stage1_jobs_skips_threads_with_disabled_memory_mode() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("initialize runtime"); + + let now = Utc::now(); + let eligible_at = now - Duration::hours(13); + + let current_thread_id = + ThreadId::from_string(&Uuid::new_v4().to_string()).expect("current thread id"); + let disabled_thread_id = + ThreadId::from_string(&Uuid::new_v4().to_string()).expect("disabled thread id"); + let enabled_thread_id = + ThreadId::from_string(&Uuid::new_v4().to_string()).expect("enabled thread id"); + + let mut current = + test_thread_metadata(&codex_home, current_thread_id, codex_home.join("current")); + current.created_at = now; + current.updated_at = now; + runtime + .upsert_thread(¤t) + .await + .expect("upsert current thread"); + + let mut disabled = + test_thread_metadata(&codex_home, disabled_thread_id, codex_home.join("disabled")); + disabled.created_at = eligible_at; + disabled.updated_at = eligible_at; + runtime + .upsert_thread(&disabled) + .await + .expect("upsert disabled thread"); + sqlx::query("UPDATE threads SET memory_mode = 'disabled' WHERE id = ?") + .bind(disabled_thread_id.to_string()) + .execute(runtime.pool.as_ref()) + .await + .expect("disable thread memory mode"); + + let mut enabled = + test_thread_metadata(&codex_home, enabled_thread_id, codex_home.join("enabled")); + enabled.created_at = eligible_at; + enabled.updated_at = eligible_at; + runtime + .upsert_thread(&enabled) + .await + .expect("upsert enabled thread"); + + let allowed_sources = vec!["cli".to_string()]; + let claims = runtime + .claim_stage1_jobs_for_startup( + current_thread_id, + Stage1StartupClaimParams { + scan_limit: 10, + max_claimed: 10, + max_age_days: 30, + min_rollout_idle_hours: 12, + allowed_sources: allowed_sources.as_slice(), + lease_seconds: 3600, + }, + ) + .await + .expect("claim stage1 startup jobs"); + + assert_eq!(claims.len(), 1); + assert_eq!(claims[0].thread.id, enabled_thread_id); + + let _ = tokio::fs::remove_dir_all(codex_home).await; + } + #[tokio::test] async fn claim_stage1_jobs_enforces_global_running_cap() { let codex_home = unique_temp_dir(); diff --git a/codex-rs/state/src/runtime/threads.rs b/codex-rs/state/src/runtime/threads.rs index 0eb64d6be5..e02cfb3e4a 100644 --- a/codex-rs/state/src/runtime/threads.rs +++ b/codex-rs/state/src/runtime/threads.rs @@ -195,6 +195,15 @@ FROM threads /// Insert or replace thread metadata directly. pub async fn upsert_thread(&self, metadata: &crate::ThreadMetadata) -> anyhow::Result<()> { + self.upsert_thread_with_creation_memory_mode(metadata, None) + .await + } + + async fn upsert_thread_with_creation_memory_mode( + &self, + metadata: &crate::ThreadMetadata, + creation_memory_mode: Option<&str>, + ) -> anyhow::Result<()> { sqlx::query( r#" INSERT INTO threads ( @@ -217,8 +226,9 @@ INSERT INTO threads ( archived_at, git_sha, git_branch, - git_origin_url -) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + git_origin_url, + memory_mode +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET rollout_path = excluded.rollout_path, created_at = excluded.created_at, @@ -261,6 +271,7 @@ ON CONFLICT(id) DO UPDATE SET .bind(metadata.git_sha.as_deref()) .bind(metadata.git_branch.as_deref()) .bind(metadata.git_origin_url.as_deref()) + .bind(creation_memory_mode.unwrap_or("enabled")) .execute(self.pool.as_ref()) .await?; Ok(()) @@ -316,13 +327,14 @@ ON CONFLICT(thread_id, position) DO NOTHING builder: &ThreadMetadataBuilder, items: &[RolloutItem], otel: Option<&OtelManager>, + new_thread_memory_mode: Option<&str>, ) -> anyhow::Result<()> { if items.is_empty() { return Ok(()); } - let mut metadata = self - .get_thread(builder.id) - .await? + let existing_metadata = self.get_thread(builder.id).await?; + let mut metadata = existing_metadata + .clone() .unwrap_or_else(|| builder.build(&self.default_provider)); metadata.rollout_path = builder.rollout_path.clone(); for item in items { @@ -333,7 +345,13 @@ ON CONFLICT(thread_id, position) DO NOTHING } // Keep the thread upsert before dynamic tools to satisfy the foreign key constraint: // thread_dynamic_tools.thread_id -> threads.id. - if let Err(err) = self.upsert_thread(&metadata).await { + let upsert_result = if existing_metadata.is_none() { + self.upsert_thread_with_creation_memory_mode(&metadata, new_thread_memory_mode) + .await + } else { + self.upsert_thread(&metadata).await + }; + if let Err(err) = upsert_result { if let Some(otel) = otel { otel.counter(DB_ERROR_METRIC, 1, &[("stage", "apply_rollout_items")]); } @@ -494,3 +512,49 @@ pub(super) fn push_thread_order_and_limit( builder.push(" LIMIT "); builder.push_bind(limit as i64); } + +#[cfg(test)] +mod tests { + use super::*; + use crate::runtime::test_support::test_thread_metadata; + use crate::runtime::test_support::unique_temp_dir; + use pretty_assertions::assert_eq; + + #[tokio::test] + async fn upsert_thread_keeps_creation_memory_mode_for_existing_rows() { + let codex_home = unique_temp_dir(); + let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string(), None) + .await + .expect("state db should initialize"); + let thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000123").expect("valid thread id"); + let mut metadata = test_thread_metadata(&codex_home, thread_id, codex_home.clone()); + + runtime + .upsert_thread_with_creation_memory_mode(&metadata, Some("disabled")) + .await + .expect("initial insert should succeed"); + + let memory_mode: String = + sqlx::query_scalar("SELECT memory_mode FROM threads WHERE id = ?") + .bind(thread_id.to_string()) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("memory mode should be readable"); + assert_eq!(memory_mode, "disabled"); + + metadata.title = "updated title".to_string(); + runtime + .upsert_thread(&metadata) + .await + .expect("upsert should succeed"); + + let memory_mode: String = + sqlx::query_scalar("SELECT memory_mode FROM threads WHERE id = ?") + .bind(thread_id.to_string()) + .fetch_one(runtime.pool.as_ref()) + .await + .expect("memory mode should remain readable"); + assert_eq!(memory_mode, "disabled"); + } +} diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 99684ddede..e32a76da0f 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1,6 +1,7 @@ use crate::app_backtrack::BacktrackState; use crate::app_event::AppEvent; use crate::app_event::ExitMode; +use crate::app_event::RealtimeAudioDeviceKind; #[cfg(target_os = "windows")] use crate::app_event::WindowsSandboxEnableMode; use crate::app_event_sender::AppEventSender; @@ -2020,6 +2021,9 @@ impl App { AppEvent::UpdatePersonality(personality) => { self.on_update_personality(personality); } + AppEvent::OpenRealtimeAudioDeviceSelection { kind } => { + self.chat_widget.open_realtime_audio_device_selection(kind); + } AppEvent::OpenReasoningPopup { model } => { self.chat_widget.open_reasoning_popup(model); } @@ -2445,6 +2449,56 @@ impl App { } } } + AppEvent::PersistRealtimeAudioDeviceSelection { kind, name } => { + let builder = match kind { + RealtimeAudioDeviceKind::Microphone => { + ConfigEditsBuilder::new(&self.config.codex_home) + .set_realtime_microphone(name.as_deref()) + } + RealtimeAudioDeviceKind::Speaker => { + ConfigEditsBuilder::new(&self.config.codex_home) + .set_realtime_speaker(name.as_deref()) + } + }; + + match builder.apply().await { + Ok(()) => { + match kind { + RealtimeAudioDeviceKind::Microphone => { + self.config.realtime_audio.microphone = name.clone(); + } + RealtimeAudioDeviceKind::Speaker => { + self.config.realtime_audio.speaker = name.clone(); + } + } + self.chat_widget + .set_realtime_audio_device(kind, name.clone()); + + if self.chat_widget.realtime_conversation_is_live() { + self.chat_widget.open_realtime_audio_restart_prompt(kind); + } else { + let selection = name.unwrap_or_else(|| "System default".to_string()); + self.chat_widget.add_info_message( + format!("Realtime {} set to {selection}", kind.noun()), + None, + ); + } + } + Err(err) => { + tracing::error!( + error = %err, + "failed to persist realtime audio selection" + ); + self.chat_widget.add_error_message(format!( + "Failed to save realtime {}: {err}", + kind.noun() + )); + } + } + } + AppEvent::RestartRealtimeAudioDevice { kind } => { + self.chat_widget.restart_realtime_audio_device(kind); + } AppEvent::UpdateAskForApprovalPolicy(policy) => { self.runtime_approval_policy_override = Some(policy); if let Err(err) = self.config.permissions.approval_policy.set(policy) { diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 0767696e28..104895ad9e 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -29,6 +29,28 @@ use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum RealtimeAudioDeviceKind { + Microphone, + Speaker, +} + +impl RealtimeAudioDeviceKind { + pub(crate) fn title(self) -> &'static str { + match self { + Self::Microphone => "Microphone", + Self::Speaker => "Speaker", + } + } + + pub(crate) fn noun(self) -> &'static str { + match self { + Self::Microphone => "microphone", + Self::Speaker => "speaker", + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) enum WindowsSandboxEnableMode { @@ -166,6 +188,26 @@ pub(crate) enum AppEvent { personality: Personality, }, + /// Open the device picker for a realtime microphone or speaker. + OpenRealtimeAudioDeviceSelection { + kind: RealtimeAudioDeviceKind, + }, + + /// Persist the selected realtime microphone or speaker to top-level config. + #[cfg_attr( + any(target_os = "linux", not(feature = "voice-input")), + allow(dead_code) + )] + PersistRealtimeAudioDeviceSelection { + kind: RealtimeAudioDeviceKind, + name: Option, + }, + + /// Restart the selected realtime microphone or speaker locally. + RestartRealtimeAudioDevice { + kind: RealtimeAudioDeviceKind, + }, + /// Open the reasoning selection popup after picking a model. OpenReasoningPopup { model: ModelPreset, diff --git a/codex-rs/tui/src/audio_device.rs b/codex-rs/tui/src/audio_device.rs index ec8a86ff29..6c3b22ccdd 100644 --- a/codex-rs/tui/src/audio_device.rs +++ b/codex-rs/tui/src/audio_device.rs @@ -3,53 +3,79 @@ use cpal::traits::DeviceTrait; use cpal::traits::HostTrait; use tracing::warn; -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum AudioDeviceKind { - Input, - Output, -} +use crate::app_event::RealtimeAudioDeviceKind; -impl AudioDeviceKind { - fn noun(self) -> &'static str { - match self { - Self::Input => "input", - Self::Output => "output", - } - } - - fn configured_name(self, config: &Config) -> Option<&str> { - match self { - Self::Input => config.realtime_audio.microphone.as_deref(), - Self::Output => config.realtime_audio.speaker.as_deref(), +const PREFERRED_INPUT_SAMPLE_RATE: u32 = 24_000; +const PREFERRED_INPUT_CHANNELS: u16 = 1; + +pub(crate) fn list_realtime_audio_device_names( + kind: RealtimeAudioDeviceKind, +) -> Result, String> { + let host = cpal::default_host(); + let mut device_names = Vec::new(); + for device in devices(&host, kind)? { + let Ok(name) = device.name() else { + continue; + }; + if !device_names.contains(&name) { + device_names.push(name); } } + Ok(device_names) } pub(crate) fn select_configured_input_device_and_config( config: &Config, ) -> Result<(cpal::Device, cpal::SupportedStreamConfig), String> { - select_device_and_config(AudioDeviceKind::Input, config) + select_device_and_config(RealtimeAudioDeviceKind::Microphone, config) } pub(crate) fn select_configured_output_device_and_config( config: &Config, ) -> Result<(cpal::Device, cpal::SupportedStreamConfig), String> { - select_device_and_config(AudioDeviceKind::Output, config) + select_device_and_config(RealtimeAudioDeviceKind::Speaker, config) +} + +pub(crate) fn preferred_input_config( + device: &cpal::Device, +) -> Result { + let supported_configs = device + .supported_input_configs() + .map_err(|err| format!("failed to enumerate input audio configs: {err}"))?; + + supported_configs + .filter_map(|range| { + let sample_format_rank = match range.sample_format() { + cpal::SampleFormat::I16 => 0u8, + cpal::SampleFormat::U16 => 1u8, + cpal::SampleFormat::F32 => 2u8, + _ => return None, + }; + let sample_rate = preferred_input_sample_rate(&range); + let sample_rate_penalty = sample_rate.0.abs_diff(PREFERRED_INPUT_SAMPLE_RATE); + let channel_penalty = range.channels().abs_diff(PREFERRED_INPUT_CHANNELS); + Some(( + (sample_rate_penalty, channel_penalty, sample_format_rank), + range.with_sample_rate(sample_rate), + )) + }) + .min_by_key(|(score, _)| *score) + .map(|(_, config)| config) + .or_else(|| device.default_input_config().ok()) + .ok_or_else(|| "failed to get default input config".to_string()) } fn select_device_and_config( - kind: AudioDeviceKind, + kind: RealtimeAudioDeviceKind, config: &Config, ) -> Result<(cpal::Device, cpal::SupportedStreamConfig), String> { let host = cpal::default_host(); - let configured_name = kind.configured_name(config); + let configured_name = configured_name(kind, config); let selected = configured_name .and_then(|name| find_device_by_name(&host, kind, name)) .or_else(|| { let default_device = default_device(&host, kind); - if let Some(name) = configured_name - && default_device.is_some() - { + if let Some(name) = configured_name && default_device.is_some() { warn!( "configured {} audio device `{name}` was unavailable; falling back to system default", kind.noun() @@ -59,13 +85,23 @@ fn select_device_and_config( }) .ok_or_else(|| missing_device_error(kind, configured_name))?; - let stream_config = default_config(&selected, kind)?; + let stream_config = match kind { + RealtimeAudioDeviceKind::Microphone => preferred_input_config(&selected)?, + RealtimeAudioDeviceKind::Speaker => default_config(&selected, kind)?, + }; Ok((selected, stream_config)) } +fn configured_name(kind: RealtimeAudioDeviceKind, config: &Config) -> Option<&str> { + match kind { + RealtimeAudioDeviceKind::Microphone => config.realtime_audio.microphone.as_deref(), + RealtimeAudioDeviceKind::Speaker => config.realtime_audio.speaker.as_deref(), + } +} + fn find_device_by_name( host: &cpal::Host, - kind: AudioDeviceKind, + kind: RealtimeAudioDeviceKind, name: &str, ) -> Option { let devices = devices(host, kind).ok()?; @@ -74,49 +110,67 @@ fn find_device_by_name( .find(|device| device.name().ok().as_deref() == Some(name)) } -fn devices(host: &cpal::Host, kind: AudioDeviceKind) -> Result, String> { +fn devices(host: &cpal::Host, kind: RealtimeAudioDeviceKind) -> Result, String> { match kind { - AudioDeviceKind::Input => host + RealtimeAudioDeviceKind::Microphone => host .input_devices() .map(|devices| devices.collect()) .map_err(|err| format!("failed to enumerate input audio devices: {err}")), - AudioDeviceKind::Output => host + RealtimeAudioDeviceKind::Speaker => host .output_devices() .map(|devices| devices.collect()) .map_err(|err| format!("failed to enumerate output audio devices: {err}")), } } -fn default_device(host: &cpal::Host, kind: AudioDeviceKind) -> Option { +fn default_device(host: &cpal::Host, kind: RealtimeAudioDeviceKind) -> Option { match kind { - AudioDeviceKind::Input => host.default_input_device(), - AudioDeviceKind::Output => host.default_output_device(), + RealtimeAudioDeviceKind::Microphone => host.default_input_device(), + RealtimeAudioDeviceKind::Speaker => host.default_output_device(), } } fn default_config( device: &cpal::Device, - kind: AudioDeviceKind, + kind: RealtimeAudioDeviceKind, ) -> Result { match kind { - AudioDeviceKind::Input => device + RealtimeAudioDeviceKind::Microphone => device .default_input_config() .map_err(|err| format!("failed to get default input config: {err}")), - AudioDeviceKind::Output => device + RealtimeAudioDeviceKind::Speaker => device .default_output_config() .map_err(|err| format!("failed to get default output config: {err}")), } } -fn missing_device_error(kind: AudioDeviceKind, configured_name: Option<&str>) -> String { - match (kind, configured_name) { - (AudioDeviceKind::Input, Some(name)) => format!( - "configured input audio device `{name}` was unavailable and no default input audio device was found" - ), - (AudioDeviceKind::Output, Some(name)) => format!( - "configured output audio device `{name}` was unavailable and no default output audio device was found" - ), - (AudioDeviceKind::Input, None) => "no input audio device available".to_string(), - (AudioDeviceKind::Output, None) => "no output audio device available".to_string(), +fn preferred_input_sample_rate(range: &cpal::SupportedStreamConfigRange) -> cpal::SampleRate { + let min = range.min_sample_rate().0; + let max = range.max_sample_rate().0; + if (min..=max).contains(&PREFERRED_INPUT_SAMPLE_RATE) { + cpal::SampleRate(PREFERRED_INPUT_SAMPLE_RATE) + } else if PREFERRED_INPUT_SAMPLE_RATE < min { + cpal::SampleRate(min) + } else { + cpal::SampleRate(max) + } +} + +fn missing_device_error(kind: RealtimeAudioDeviceKind, configured_name: Option<&str>) -> String { + match (kind, configured_name) { + (RealtimeAudioDeviceKind::Microphone, Some(name)) => { + format!( + "configured microphone `{name}` was unavailable and no default input audio device was found" + ) + } + (RealtimeAudioDeviceKind::Speaker, Some(name)) => { + format!( + "configured speaker `{name}` was unavailable and no default output audio device was found" + ) + } + (RealtimeAudioDeviceKind::Microphone, None) => { + "no input audio device available".to_string() + } + (RealtimeAudioDeviceKind::Speaker, None) => "no output audio device available".to_string(), } } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 9943a22531..fe40f4fa7b 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -395,6 +395,7 @@ pub(crate) struct ChatComposer { connectors_enabled: bool, personality_command_enabled: bool, realtime_conversation_enabled: bool, + audio_device_selection_enabled: bool, windows_degraded_sandbox_active: bool, status_line_value: Option>, status_line_enabled: bool, @@ -500,6 +501,7 @@ impl ChatComposer { connectors_enabled: false, personality_command_enabled: false, realtime_conversation_enabled: false, + audio_device_selection_enabled: false, windows_degraded_sandbox_active: false, status_line_value: None, status_line_enabled: false, @@ -577,10 +579,13 @@ impl ChatComposer { self.realtime_conversation_enabled = enabled; } + pub fn set_audio_device_selection_enabled(&mut self, enabled: bool) { + self.audio_device_selection_enabled = enabled; + } + /// Compatibility shim for tests that still toggle the removed steer mode flag. #[cfg(test)] pub fn set_steer_enabled(&mut self, _enabled: bool) {} - pub fn set_voice_transcription_enabled(&mut self, enabled: bool) { self.voice_state.transcription_enabled = enabled; if !enabled { @@ -2264,6 +2269,7 @@ impl ChatComposer { self.connectors_enabled, self.personality_command_enabled, self.realtime_conversation_enabled, + self.audio_device_selection_enabled, self.windows_degraded_sandbox_active, ) .is_some(); @@ -2480,6 +2486,7 @@ impl ChatComposer { self.connectors_enabled, self.personality_command_enabled, self.realtime_conversation_enabled, + self.audio_device_selection_enabled, self.windows_degraded_sandbox_active, ) { @@ -2515,6 +2522,7 @@ impl ChatComposer { self.connectors_enabled, self.personality_command_enabled, self.realtime_conversation_enabled, + self.audio_device_selection_enabled, self.windows_degraded_sandbox_active, )?; @@ -3334,6 +3342,7 @@ impl ChatComposer { self.connectors_enabled, self.personality_command_enabled, self.realtime_conversation_enabled, + self.audio_device_selection_enabled, self.windows_degraded_sandbox_active, ) .is_some(); @@ -3396,6 +3405,7 @@ impl ChatComposer { self.connectors_enabled, self.personality_command_enabled, self.realtime_conversation_enabled, + self.audio_device_selection_enabled, self.windows_degraded_sandbox_active, ) { return true; @@ -3450,6 +3460,7 @@ impl ChatComposer { let connectors_enabled = self.connectors_enabled; let personality_command_enabled = self.personality_command_enabled; let realtime_conversation_enabled = self.realtime_conversation_enabled; + let audio_device_selection_enabled = self.audio_device_selection_enabled; let mut command_popup = CommandPopup::new( self.custom_prompts.clone(), CommandPopupFlags { @@ -3457,6 +3468,7 @@ impl ChatComposer { connectors_enabled, personality_command_enabled, realtime_conversation_enabled, + audio_device_selection_enabled, windows_degraded_sandbox_active: self.windows_degraded_sandbox_active, }, ); diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index 83b523cc23..62765d9c2c 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -40,6 +40,7 @@ pub(crate) struct CommandPopupFlags { pub(crate) connectors_enabled: bool, pub(crate) personality_command_enabled: bool, pub(crate) realtime_conversation_enabled: bool, + pub(crate) audio_device_selection_enabled: bool, pub(crate) windows_degraded_sandbox_active: bool, } @@ -51,6 +52,7 @@ impl CommandPopup { flags.connectors_enabled, flags.personality_command_enabled, flags.realtime_conversation_enabled, + flags.audio_device_selection_enabled, flags.windows_degraded_sandbox_active, ) .into_iter() @@ -498,6 +500,7 @@ mod tests { connectors_enabled: false, personality_command_enabled: true, realtime_conversation_enabled: false, + audio_device_selection_enabled: false, windows_degraded_sandbox_active: false, }, ); @@ -518,6 +521,7 @@ mod tests { connectors_enabled: false, personality_command_enabled: true, realtime_conversation_enabled: false, + audio_device_selection_enabled: false, windows_degraded_sandbox_active: false, }, ); @@ -538,6 +542,7 @@ mod tests { connectors_enabled: false, personality_command_enabled: false, realtime_conversation_enabled: false, + audio_device_selection_enabled: false, windows_degraded_sandbox_active: false, }, ); @@ -566,6 +571,7 @@ mod tests { connectors_enabled: false, personality_command_enabled: true, realtime_conversation_enabled: false, + audio_device_selection_enabled: false, windows_degraded_sandbox_active: false, }, ); @@ -577,6 +583,36 @@ mod tests { } } + #[test] + fn settings_command_hidden_when_audio_device_selection_is_disabled() { + let mut popup = CommandPopup::new( + Vec::new(), + CommandPopupFlags { + collaboration_modes_enabled: false, + connectors_enabled: false, + personality_command_enabled: true, + realtime_conversation_enabled: true, + audio_device_selection_enabled: false, + windows_degraded_sandbox_active: false, + }, + ); + popup.on_composer_text_change("/aud".to_string()); + + let cmds: Vec<&str> = popup + .filtered_items() + .into_iter() + .filter_map(|item| match item { + CommandItem::Builtin(cmd) => Some(cmd.command()), + CommandItem::UserPrompt(_) => None, + }) + .collect(); + + assert!( + !cmds.contains(&"settings"), + "expected '/settings' to be hidden when audio device selection is disabled, got {cmds:?}" + ); + } + #[test] fn debug_commands_are_hidden_from_popup() { let popup = CommandPopup::new(Vec::new(), CommandPopupFlags::default()); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e774e9c420..dac0069887 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -298,6 +298,11 @@ impl BottomPane { self.request_redraw(); } + pub fn set_audio_device_selection_enabled(&mut self, enabled: bool) { + self.composer.set_audio_device_selection_enabled(enabled); + self.request_redraw(); + } + pub fn set_voice_transcription_enabled(&mut self, enabled: bool) { self.composer.set_voice_transcription_enabled(enabled); self.request_redraw(); diff --git a/codex-rs/tui/src/bottom_pane/slash_commands.rs b/codex-rs/tui/src/bottom_pane/slash_commands.rs index 86c131e6db..981c61c451 100644 --- a/codex-rs/tui/src/bottom_pane/slash_commands.rs +++ b/codex-rs/tui/src/bottom_pane/slash_commands.rs @@ -14,6 +14,7 @@ pub(crate) fn builtins_for_input( connectors_enabled: bool, personality_command_enabled: bool, realtime_conversation_enabled: bool, + audio_device_selection_enabled: bool, allow_elevate_sandbox: bool, ) -> Vec<(&'static str, SlashCommand)> { built_in_slash_commands() @@ -26,6 +27,7 @@ pub(crate) fn builtins_for_input( .filter(|(_, cmd)| connectors_enabled || *cmd != SlashCommand::Apps) .filter(|(_, cmd)| personality_command_enabled || *cmd != SlashCommand::Personality) .filter(|(_, cmd)| realtime_conversation_enabled || *cmd != SlashCommand::Realtime) + .filter(|(_, cmd)| audio_device_selection_enabled || *cmd != SlashCommand::Settings) .collect() } @@ -36,6 +38,7 @@ pub(crate) fn find_builtin_command( connectors_enabled: bool, personality_command_enabled: bool, realtime_conversation_enabled: bool, + audio_device_selection_enabled: bool, allow_elevate_sandbox: bool, ) -> Option { builtins_for_input( @@ -43,6 +46,7 @@ pub(crate) fn find_builtin_command( connectors_enabled, personality_command_enabled, realtime_conversation_enabled, + audio_device_selection_enabled, allow_elevate_sandbox, ) .into_iter() @@ -57,6 +61,7 @@ pub(crate) fn has_builtin_prefix( connectors_enabled: bool, personality_command_enabled: bool, realtime_conversation_enabled: bool, + audio_device_selection_enabled: bool, allow_elevate_sandbox: bool, ) -> bool { builtins_for_input( @@ -64,6 +69,7 @@ pub(crate) fn has_builtin_prefix( connectors_enabled, personality_command_enabled, realtime_conversation_enabled, + audio_device_selection_enabled, allow_elevate_sandbox, ) .into_iter() @@ -77,14 +83,14 @@ mod tests { #[test] fn debug_command_still_resolves_for_dispatch() { - let cmd = find_builtin_command("debug-config", true, true, true, false, false); + let cmd = find_builtin_command("debug-config", true, true, true, false, false, false); assert_eq!(cmd, Some(SlashCommand::DebugConfig)); } #[test] fn clear_command_resolves_for_dispatch() { assert_eq!( - find_builtin_command("clear", true, true, true, false, false), + find_builtin_command("clear", true, true, true, false, false, false), Some(SlashCommand::Clear) ); } @@ -92,7 +98,23 @@ mod tests { #[test] fn realtime_command_is_hidden_when_realtime_is_disabled() { assert_eq!( - find_builtin_command("realtime", true, true, true, false, false), + find_builtin_command("realtime", true, true, true, false, true, false), + None + ); + } + + #[test] + fn settings_command_is_hidden_when_realtime_is_disabled() { + assert_eq!( + find_builtin_command("settings", true, true, true, false, false, false), + None + ); + } + + #[test] + fn settings_command_is_hidden_when_audio_device_selection_is_disabled() { + assert_eq!( + find_builtin_command("settings", true, true, true, true, false, false), None ); } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9e5f1d8d80..4031cecaf6 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -37,6 +37,9 @@ use std::sync::atomic::Ordering; use std::time::Duration; use std::time::Instant; +use crate::app_event::RealtimeAudioDeviceKind; +#[cfg(all(not(target_os = "linux"), feature = "voice-input"))] +use crate::audio_device::list_realtime_audio_device_names; use crate::bottom_pane::StatusLineItem; use crate::bottom_pane::StatusLineSetupView; use crate::status::RateLimitWindowDisplay; @@ -855,6 +858,10 @@ impl ChatWidget { && cfg!(not(target_os = "linux")) } + fn realtime_audio_device_selection_enabled(&self) -> bool { + self.realtime_conversation_enabled() && cfg!(feature = "voice-input") + } + /// Synchronize the bottom-pane "task running" indicator with the current lifecycles. /// /// The bottom pane only has one running flag, but this module treats it as a derived state of @@ -2882,6 +2889,9 @@ impl ChatWidget { widget .bottom_pane .set_realtime_conversation_enabled(widget.realtime_conversation_enabled()); + widget + .bottom_pane + .set_audio_device_selection_enabled(widget.realtime_audio_device_selection_enabled()); widget .bottom_pane .set_status_line_enabled(!widget.configured_status_line_items().is_empty()); @@ -3056,6 +3066,9 @@ impl ChatWidget { widget .bottom_pane .set_realtime_conversation_enabled(widget.realtime_conversation_enabled()); + widget + .bottom_pane + .set_audio_device_selection_enabled(widget.realtime_audio_device_selection_enabled()); widget .bottom_pane .set_status_line_enabled(!widget.configured_status_line_items().is_empty()); @@ -3219,6 +3232,9 @@ impl ChatWidget { widget .bottom_pane .set_realtime_conversation_enabled(widget.realtime_conversation_enabled()); + widget + .bottom_pane + .set_audio_device_selection_enabled(widget.realtime_audio_device_selection_enabled()); widget .bottom_pane .set_status_line_enabled(!widget.configured_status_line_items().is_empty()); @@ -3531,6 +3547,12 @@ impl ChatWidget { self.start_realtime_conversation(); } } + SlashCommand::Settings => { + if !self.realtime_audio_device_selection_enabled() { + return; + } + self.open_realtime_audio_popup(); + } SlashCommand::Personality => { self.open_personality_popup(); } @@ -5270,6 +5292,161 @@ impl ChatWidget { }); } + pub(crate) fn open_realtime_audio_popup(&mut self) { + let items = [ + RealtimeAudioDeviceKind::Microphone, + RealtimeAudioDeviceKind::Speaker, + ] + .into_iter() + .map(|kind| { + let description = Some(format!( + "Current: {}", + self.current_realtime_audio_selection_label(kind) + )); + let actions: Vec = vec![Box::new(move |tx| { + tx.send(AppEvent::OpenRealtimeAudioDeviceSelection { kind }); + })]; + SelectionItem { + name: kind.title().to_string(), + description, + actions, + dismiss_on_select: true, + ..Default::default() + } + }) + .collect(); + + self.bottom_pane.show_selection_view(SelectionViewParams { + title: Some("Settings".to_string()), + subtitle: Some("Configure settings for Codex.".to_string()), + footer_hint: Some(standard_popup_hint_line()), + items, + ..Default::default() + }); + } + + #[cfg(all(not(target_os = "linux"), feature = "voice-input"))] + pub(crate) fn open_realtime_audio_device_selection(&mut self, kind: RealtimeAudioDeviceKind) { + match list_realtime_audio_device_names(kind) { + Ok(device_names) => { + self.open_realtime_audio_device_selection_with_names(kind, device_names); + } + Err(err) => { + self.add_error_message(format!( + "Failed to load realtime {} devices: {err}", + kind.noun() + )); + } + } + } + + #[cfg(any(target_os = "linux", not(feature = "voice-input")))] + pub(crate) fn open_realtime_audio_device_selection(&mut self, kind: RealtimeAudioDeviceKind) { + let _ = kind; + } + + #[cfg(all(not(target_os = "linux"), feature = "voice-input"))] + fn open_realtime_audio_device_selection_with_names( + &mut self, + kind: RealtimeAudioDeviceKind, + device_names: Vec, + ) { + let current_selection = self.current_realtime_audio_device_name(kind); + let current_available = current_selection + .as_deref() + .is_some_and(|name| device_names.iter().any(|device_name| device_name == name)); + let mut items = vec![SelectionItem { + name: "System default".to_string(), + description: Some("Use your operating system default device.".to_string()), + is_current: current_selection.is_none(), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::PersistRealtimeAudioDeviceSelection { kind, name: None }); + })], + dismiss_on_select: true, + ..Default::default() + }]; + + if let Some(selection) = current_selection.as_deref() + && !current_available + { + items.push(SelectionItem { + name: format!("Unavailable: {selection}"), + description: Some("Configured device is not currently available.".to_string()), + is_current: true, + is_disabled: true, + disabled_reason: Some("Reconnect the device or choose another one.".to_string()), + ..Default::default() + }); + } + + items.extend(device_names.into_iter().map(|device_name| { + let persisted_name = device_name.clone(); + let actions: Vec = vec![Box::new(move |tx| { + tx.send(AppEvent::PersistRealtimeAudioDeviceSelection { + kind, + name: Some(persisted_name.clone()), + }); + })]; + SelectionItem { + is_current: current_selection.as_deref() == Some(device_name.as_str()), + name: device_name, + actions, + dismiss_on_select: true, + ..Default::default() + } + })); + + let mut header = ColumnRenderable::new(); + header.push(Line::from(format!("Select {}", kind.title()).bold())); + header.push(Line::from( + "Saved devices apply to realtime voice only.".dim(), + )); + + self.bottom_pane.show_selection_view(SelectionViewParams { + header: Box::new(header), + footer_hint: Some(standard_popup_hint_line()), + items, + ..Default::default() + }); + } + + pub(crate) fn open_realtime_audio_restart_prompt(&mut self, kind: RealtimeAudioDeviceKind) { + let restart_actions: Vec = vec![Box::new(move |tx| { + tx.send(AppEvent::RestartRealtimeAudioDevice { kind }); + })]; + let items = vec![ + SelectionItem { + name: "Restart now".to_string(), + description: Some(format!("Restart local {} audio now.", kind.noun())), + actions: restart_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Apply later".to_string(), + description: Some(format!( + "Keep the current {} until local audio starts again.", + kind.noun() + )), + dismiss_on_select: true, + ..Default::default() + }, + ]; + + let mut header = ColumnRenderable::new(); + header.push(Line::from(format!("Restart {} now?", kind.title()).bold())); + header.push(Line::from( + "Configuration is saved. Restart local audio to use it immediately.".dim(), + )); + + self.bottom_pane.show_selection_view(SelectionViewParams { + header: Box::new(header), + footer_hint: Some(standard_popup_hint_line()), + items, + ..Default::default() + }); + } + fn model_menu_header(&self, title: &str, subtitle: &str) -> Box { let title = title.to_string(); let subtitle = subtitle.to_string(); @@ -6523,6 +6700,8 @@ impl ChatWidget { let realtime_conversation_enabled = self.realtime_conversation_enabled(); self.bottom_pane .set_realtime_conversation_enabled(realtime_conversation_enabled); + self.bottom_pane + .set_audio_device_selection_enabled(self.realtime_audio_device_selection_enabled()); if !realtime_conversation_enabled && self.realtime_conversation.is_live() { self.request_realtime_conversation_close(Some( "Realtime voice mode was closed because the feature was disabled.".to_string(), @@ -6612,6 +6791,17 @@ impl ChatWidget { self.config.personality = Some(personality); } + pub(crate) fn set_realtime_audio_device( + &mut self, + kind: RealtimeAudioDeviceKind, + name: Option, + ) { + match kind { + RealtimeAudioDeviceKind::Microphone => self.config.realtime_audio.microphone = name, + RealtimeAudioDeviceKind::Speaker => self.config.realtime_audio.speaker = name, + } + } + /// Set the syntax theme override in the widget's config copy. pub(crate) fn set_tui_theme(&mut self, theme: Option) { self.config.tui_theme = theme; @@ -6640,6 +6830,22 @@ impl ChatWidget { .unwrap_or_else(|| self.current_collaboration_mode.model()) } + pub(crate) fn realtime_conversation_is_live(&self) -> bool { + self.realtime_conversation.is_active() + } + + fn current_realtime_audio_device_name(&self, kind: RealtimeAudioDeviceKind) -> Option { + match kind { + RealtimeAudioDeviceKind::Microphone => self.config.realtime_audio.microphone.clone(), + RealtimeAudioDeviceKind::Speaker => self.config.realtime_audio.speaker.clone(), + } + } + + fn current_realtime_audio_selection_label(&self, kind: RealtimeAudioDeviceKind) -> String { + self.current_realtime_audio_device_name(kind) + .unwrap_or_else(|| "System default".to_string()) + } + fn sync_personality_command_enabled(&mut self) { self.bottom_pane .set_personality_command_enabled(self.config.features.enabled(Feature::Personality)); diff --git a/codex-rs/tui/src/chatwidget/agent.rs b/codex-rs/tui/src/chatwidget/agent.rs index 63d519a7a5..e14a9e3628 100644 --- a/codex-rs/tui/src/chatwidget/agent.rs +++ b/codex-rs/tui/src/chatwidget/agent.rs @@ -13,6 +13,17 @@ use tokio::sync::mpsc::unbounded_channel; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; +const TUI_NOTIFY_CLIENT: &str = "codex-tui"; + +async fn initialize_app_server_client_name(thread: &CodexThread) { + if let Err(err) = thread + .set_app_server_client_name(Some(TUI_NOTIFY_CLIENT.to_string())) + .await + { + tracing::error!("failed to set app server client name: {err}"); + } +} + /// Spawn the agent bootstrapper and op forwarding loop, returning the /// `UnboundedSender` used by the UI to submit operations. pub(crate) fn spawn_agent( @@ -42,6 +53,7 @@ pub(crate) fn spawn_agent( return; } }; + initialize_app_server_client_name(thread.as_ref()).await; // Forward the captured `SessionConfigured` event so it can be rendered in the UI. let ev = codex_protocol::protocol::Event { @@ -87,6 +99,8 @@ pub(crate) fn spawn_agent_from_existing( let app_event_tx_clone = app_event_tx; tokio::spawn(async move { + initialize_app_server_client_name(thread.as_ref()).await; + // Forward the captured `SessionConfigured` event so it can be rendered in the UI. let ev = codex_protocol::protocol::Event { id: "".to_string(), @@ -123,6 +137,7 @@ pub(crate) fn spawn_op_forwarder(thread: std::sync::Arc) -> Unbound let (codex_op_tx, mut codex_op_rx) = unbounded_channel::(); tokio::spawn(async move { + initialize_app_server_client_name(thread.as_ref()).await; while let Some(op) = codex_op_rx.recv().await { if let Err(e) = thread.submit(op).await { tracing::error!("failed to submit op: {e}"); diff --git a/codex-rs/tui/src/chatwidget/realtime.rs b/codex-rs/tui/src/chatwidget/realtime.rs index e4fd2b6314..98ac76a34f 100644 --- a/codex-rs/tui/src/chatwidget/realtime.rs +++ b/codex-rs/tui/src/chatwidget/realtime.rs @@ -41,6 +41,10 @@ impl RealtimeConversationUiState { | RealtimeConversationPhase::Stopping ) } + + pub(super) fn is_active(&self) -> bool { + matches!(self.phase, RealtimeConversationPhase::Active) + } } #[derive(Clone, Debug, PartialEq)] @@ -278,8 +282,50 @@ impl ChatWidget { #[cfg(target_os = "linux")] fn start_realtime_local_audio(&mut self) {} + #[cfg(all(not(target_os = "linux"), feature = "voice-input"))] + pub(crate) fn restart_realtime_audio_device(&mut self, kind: RealtimeAudioDeviceKind) { + if !self.realtime_conversation.is_active() { + return; + } + + match kind { + RealtimeAudioDeviceKind::Microphone => { + self.stop_realtime_microphone(); + self.start_realtime_local_audio(); + } + RealtimeAudioDeviceKind::Speaker => { + self.stop_realtime_speaker(); + match crate::voice::RealtimeAudioPlayer::start(&self.config) { + Ok(player) => { + self.realtime_conversation.audio_player = Some(player); + } + Err(err) => { + self.add_error_message(format!("Failed to start speaker output: {err}")); + } + } + } + } + self.request_redraw(); + } + + #[cfg(any(target_os = "linux", not(feature = "voice-input")))] + pub(crate) fn restart_realtime_audio_device(&mut self, kind: RealtimeAudioDeviceKind) { + let _ = kind; + } + #[cfg(not(target_os = "linux"))] fn stop_realtime_local_audio(&mut self) { + self.stop_realtime_microphone(); + self.stop_realtime_speaker(); + } + + #[cfg(target_os = "linux")] + fn stop_realtime_local_audio(&mut self) { + self.realtime_conversation.meter_placeholder_id = None; + } + + #[cfg(not(target_os = "linux"))] + fn stop_realtime_microphone(&mut self) { if let Some(flag) = self.realtime_conversation.capture_stop_flag.take() { flag.store(true, Ordering::Relaxed); } @@ -289,13 +335,12 @@ impl ChatWidget { if let Some(id) = self.realtime_conversation.meter_placeholder_id.take() { self.remove_transcription_placeholder(&id); } + } + + #[cfg(not(target_os = "linux"))] + fn stop_realtime_speaker(&mut self) { if let Some(player) = self.realtime_conversation.audio_player.take() { player.clear(); } } - - #[cfg(target_os = "linux")] - fn stop_realtime_local_audio(&mut self) { - self.realtime_conversation.meter_placeholder_id = None; - } } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_audio_selection_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_audio_selection_popup.snap new file mode 100644 index 0000000000..8c60f961f9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_audio_selection_popup.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: popup +--- + Settings + Configure settings for Codex. + +› 1. Microphone Current: System default + 2. Speaker Current: System default + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_audio_selection_popup_narrow.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_audio_selection_popup_narrow.snap new file mode 100644 index 0000000000..8c60f961f9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_audio_selection_popup_narrow.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: popup +--- + Settings + Configure settings for Codex. + +› 1. Microphone Current: System default + 2. Speaker Current: System default + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_microphone_picker_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_microphone_picker_popup.snap new file mode 100644 index 0000000000..3095e6da97 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__realtime_microphone_picker_popup.snap @@ -0,0 +1,18 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: popup +--- + Select Microphone + Saved devices apply to realtime voice only. + + 1. System default Use your operating system + default device. +› 2. Unavailable: Studio Mic (current) (disabled) Configured device is not + currently available. + (disabled: Reconnect the + device or choose another + one.) + 3. Built-in Mic + 4. USB Mic + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index cbbd40ec93..b0ba5200a2 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -7,6 +7,8 @@ use super::*; use crate::app_event::AppEvent; use crate::app_event::ExitMode; +#[cfg(all(not(target_os = "linux"), feature = "voice-input"))] +use crate::app_event::RealtimeAudioDeviceKind; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::FeedbackAudience; use crate::bottom_pane::LocalImageAttachment; @@ -6001,6 +6003,62 @@ async fn personality_selection_popup_snapshot() { assert_snapshot!("personality_selection_popup", popup); } +#[cfg(all(not(target_os = "linux"), feature = "voice-input"))] +#[tokio::test] +async fn realtime_audio_selection_popup_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + chat.open_realtime_audio_popup(); + + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("realtime_audio_selection_popup", popup); +} + +#[cfg(all(not(target_os = "linux"), feature = "voice-input"))] +#[tokio::test] +async fn realtime_audio_selection_popup_narrow_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + chat.open_realtime_audio_popup(); + + let popup = render_bottom_popup(&chat, 56); + assert_snapshot!("realtime_audio_selection_popup_narrow", popup); +} + +#[cfg(all(not(target_os = "linux"), feature = "voice-input"))] +#[tokio::test] +async fn realtime_microphone_picker_popup_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + chat.config.realtime_audio.microphone = Some("Studio Mic".to_string()); + chat.open_realtime_audio_device_selection_with_names( + RealtimeAudioDeviceKind::Microphone, + vec!["Built-in Mic".to_string(), "USB Mic".to_string()], + ); + + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("realtime_microphone_picker_popup", popup); +} + +#[cfg(all(not(target_os = "linux"), feature = "voice-input"))] +#[tokio::test] +async fn realtime_audio_picker_emits_persist_event() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.2-codex")).await; + chat.open_realtime_audio_device_selection_with_names( + RealtimeAudioDeviceKind::Speaker, + vec!["Desk Speakers".to_string(), "Headphones".to_string()], + ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + assert_matches!( + rx.try_recv(), + Ok(AppEvent::PersistRealtimeAudioDeviceSelection { + kind: RealtimeAudioDeviceKind::Speaker, + name: Some(name), + }) if name == "Headphones" + ); +} + #[tokio::test] async fn model_picker_hides_show_in_picker_false_models_from_cache() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("test-visible-model")).await; @@ -6019,6 +6077,7 @@ async fn model_picker_hides_show_in_picker_false_models_from_cache() { is_default: false, upgrade: None, show_in_picker, + availability_nux: None, supported_in_api: true, input_modalities: default_input_modalities(), }; @@ -6287,6 +6346,7 @@ async fn single_reasoning_option_skips_selection() { is_default: false, upgrade: None, show_in_picker: true, + availability_nux: None, supported_in_api: true, input_modalities: default_input_modalities(), }; diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index 32591945fa..d779541fcd 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -83,6 +83,8 @@ use crate::terminal_palette::indexed_color; use crate::terminal_palette::rgb_color; use crate::terminal_palette::stdout_color_level; use codex_core::git_info::get_git_repo_root; +use codex_core::terminal::TerminalName; +use codex_core::terminal::terminal_info; use codex_protocol::protocol::FileChange; /// Classifies a diff line for gutter sign rendering and style selection. @@ -110,6 +112,14 @@ enum DiffTheme { Light, } +/// Palette depth the diff renderer will target. +/// +/// This is the *renderer's own* notion of color depth, derived from — but not +/// identical to — the raw [`StdoutColorLevel`] reported by `supports-color`. +/// The indirection exists because some terminals (notably Windows Terminal) +/// advertise only ANSI-16 support while actually rendering truecolor sequences +/// correctly; [`diff_color_level_for_terminal`] promotes those cases so the +/// diff output uses the richer palette. #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum DiffColorLevel { TrueColor, @@ -117,6 +127,22 @@ enum DiffColorLevel { Ansi16, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RichDiffColorLevel { + TrueColor, + Ansi256, +} + +impl RichDiffColorLevel { + fn from_diff_color_level(level: DiffColorLevel) -> Option { + match level { + DiffColorLevel::TrueColor => Some(Self::TrueColor), + DiffColorLevel::Ansi256 => Some(Self::Ansi256), + DiffColorLevel::Ansi16 => None, + } + } +} + pub struct DiffSummary { changes: HashMap, cwd: PathBuf, @@ -876,11 +902,75 @@ fn diff_theme() -> DiffTheme { diff_theme_for_bg(default_bg()) } +/// Return the [`DiffColorLevel`] for the current terminal session. +/// +/// This is the environment-reading adapter: it samples runtime signals +/// (`supports-color` level, terminal name, `WT_SESSION`, and `FORCE_COLOR`) +/// and forwards them to [`diff_color_level_for_terminal`]. +/// +/// Keeping env reads in this thin wrapper lets +/// [`diff_color_level_for_terminal`] stay pure and easy to unit test. fn diff_color_level() -> DiffColorLevel { - match stdout_color_level() { + diff_color_level_for_terminal( + stdout_color_level(), + terminal_info().name, + std::env::var_os("WT_SESSION").is_some(), + has_force_color_override(), + ) +} + +/// Returns whether `FORCE_COLOR` is explicitly set. +fn has_force_color_override() -> bool { + std::env::var_os("FORCE_COLOR").is_some() +} + +/// Map a raw [`StdoutColorLevel`] to a [`DiffColorLevel`] using +/// Windows Terminal-specific truecolor promotion rules. +/// +/// This helper is intentionally pure (no env access) so tests can validate +/// the policy table by passing explicit inputs. +/// +/// Windows Terminal fully supports 24-bit color but the `supports-color` +/// crate often reports only ANSI-16 there because no `COLORTERM` variable +/// is set. We detect Windows Terminal two ways — via `terminal_name` +/// (parsed from `WT_SESSION` / `TERM_PROGRAM` by `terminal_info()`) and +/// via the raw `has_wt_session` flag. +/// +/// These signals are intentionally not equivalent: `terminal_name` is a +/// derived classification with `TERM_PROGRAM` precedence, so `WT_SESSION` +/// can be present while `terminal_name` is not `WindowsTerminal`. +/// +/// When `WT_SESSION` is present, we promote to truecolor unconditionally +/// unless `FORCE_COLOR` is set. This keeps Windows Terminal rendering rich +/// by default while preserving explicit `FORCE_COLOR` user intent. +/// +/// Outside `WT_SESSION`, only ANSI-16 is promoted for identified +/// `WindowsTerminal` sessions; `Unknown` stays conservative. +fn diff_color_level_for_terminal( + stdout_level: StdoutColorLevel, + terminal_name: TerminalName, + has_wt_session: bool, + has_force_color_override: bool, +) -> DiffColorLevel { + if has_wt_session && !has_force_color_override { + return DiffColorLevel::TrueColor; + } + + let base = match stdout_level { StdoutColorLevel::TrueColor => DiffColorLevel::TrueColor, StdoutColorLevel::Ansi256 => DiffColorLevel::Ansi256, StdoutColorLevel::Ansi16 | StdoutColorLevel::Unknown => DiffColorLevel::Ansi16, + }; + + // Outside `WT_SESSION`, keep the existing Windows Terminal promotion for + // ANSI-16 sessions that likely support truecolor. + if stdout_level == StdoutColorLevel::Ansi16 + && terminal_name == TerminalName::WindowsTerminal + && !has_force_color_override + { + DiffColorLevel::TrueColor + } else { + base } } @@ -908,10 +998,11 @@ fn diff_color_level() -> DiffColorLevel { /// Context lines intentionally leave the background unset so the terminal /// default shows through. fn style_line_bg_for(kind: DiffLineType, theme: DiffTheme, color_level: DiffColorLevel) -> Style { - match kind { - DiffLineType::Insert => Style::default().bg(add_line_bg(theme, color_level)), - DiffLineType::Delete => Style::default().bg(del_line_bg(theme, color_level)), - DiffLineType::Context => Style::default(), + match (kind, RichDiffColorLevel::from_diff_color_level(color_level)) { + (_, None) => Style::default(), + (DiffLineType::Insert, Some(level)) => Style::default().bg(add_line_bg(theme, level)), + (DiffLineType::Delete, Some(level)) => Style::default().bg(del_line_bg(theme, level)), + (DiffLineType::Context, _) => Style::default(), } } @@ -919,25 +1010,21 @@ fn style_context() -> Style { Style::default() } -fn add_line_bg(theme: DiffTheme, color_level: DiffColorLevel) -> Color { +fn add_line_bg(theme: DiffTheme, color_level: RichDiffColorLevel) -> Color { match (theme, color_level) { - (DiffTheme::Dark, DiffColorLevel::TrueColor) => rgb_color(DARK_TC_ADD_LINE_BG_RGB), - (DiffTheme::Dark, DiffColorLevel::Ansi256) => indexed_color(DARK_256_ADD_LINE_BG_IDX), - (DiffTheme::Dark, DiffColorLevel::Ansi16) => Color::Green, - (DiffTheme::Light, DiffColorLevel::TrueColor) => rgb_color(LIGHT_TC_ADD_LINE_BG_RGB), - (DiffTheme::Light, DiffColorLevel::Ansi256) => indexed_color(LIGHT_256_ADD_LINE_BG_IDX), - (DiffTheme::Light, DiffColorLevel::Ansi16) => Color::LightGreen, + (DiffTheme::Dark, RichDiffColorLevel::TrueColor) => rgb_color(DARK_TC_ADD_LINE_BG_RGB), + (DiffTheme::Dark, RichDiffColorLevel::Ansi256) => indexed_color(DARK_256_ADD_LINE_BG_IDX), + (DiffTheme::Light, RichDiffColorLevel::TrueColor) => rgb_color(LIGHT_TC_ADD_LINE_BG_RGB), + (DiffTheme::Light, RichDiffColorLevel::Ansi256) => indexed_color(LIGHT_256_ADD_LINE_BG_IDX), } } -fn del_line_bg(theme: DiffTheme, color_level: DiffColorLevel) -> Color { +fn del_line_bg(theme: DiffTheme, color_level: RichDiffColorLevel) -> Color { match (theme, color_level) { - (DiffTheme::Dark, DiffColorLevel::TrueColor) => rgb_color(DARK_TC_DEL_LINE_BG_RGB), - (DiffTheme::Dark, DiffColorLevel::Ansi256) => indexed_color(DARK_256_DEL_LINE_BG_IDX), - (DiffTheme::Dark, DiffColorLevel::Ansi16) => Color::Red, - (DiffTheme::Light, DiffColorLevel::TrueColor) => rgb_color(LIGHT_TC_DEL_LINE_BG_RGB), - (DiffTheme::Light, DiffColorLevel::Ansi256) => indexed_color(LIGHT_256_DEL_LINE_BG_IDX), - (DiffTheme::Light, DiffColorLevel::Ansi16) => Color::LightRed, + (DiffTheme::Dark, RichDiffColorLevel::TrueColor) => rgb_color(DARK_TC_DEL_LINE_BG_RGB), + (DiffTheme::Dark, RichDiffColorLevel::Ansi256) => indexed_color(DARK_256_DEL_LINE_BG_IDX), + (DiffTheme::Light, RichDiffColorLevel::TrueColor) => rgb_color(LIGHT_TC_DEL_LINE_BG_RGB), + (DiffTheme::Light, RichDiffColorLevel::Ansi256) => indexed_color(LIGHT_256_DEL_LINE_BG_IDX), } } @@ -949,19 +1036,17 @@ fn light_gutter_fg(color_level: DiffColorLevel) -> Color { } } -fn light_add_num_bg(color_level: DiffColorLevel) -> Color { +fn light_add_num_bg(color_level: RichDiffColorLevel) -> Color { match color_level { - DiffColorLevel::TrueColor => rgb_color(LIGHT_TC_ADD_NUM_BG_RGB), - DiffColorLevel::Ansi256 => indexed_color(LIGHT_256_ADD_NUM_BG_IDX), - DiffColorLevel::Ansi16 => Color::Green, + RichDiffColorLevel::TrueColor => rgb_color(LIGHT_TC_ADD_NUM_BG_RGB), + RichDiffColorLevel::Ansi256 => indexed_color(LIGHT_256_ADD_NUM_BG_IDX), } } -fn light_del_num_bg(color_level: DiffColorLevel) -> Color { +fn light_del_num_bg(color_level: RichDiffColorLevel) -> Color { match color_level { - DiffColorLevel::TrueColor => rgb_color(LIGHT_TC_DEL_NUM_BG_RGB), - DiffColorLevel::Ansi256 => indexed_color(LIGHT_256_DEL_NUM_BG_IDX), - DiffColorLevel::Ansi16 => Color::Red, + RichDiffColorLevel::TrueColor => rgb_color(LIGHT_TC_DEL_NUM_BG_RGB), + RichDiffColorLevel::Ansi256 => indexed_color(LIGHT_256_DEL_NUM_BG_IDX), } } @@ -969,13 +1054,23 @@ fn light_del_num_bg(color_level: DiffColorLevel) -> Color { /// tinted background so numbers contrast against the pastel line fill. On /// dark backgrounds a simple `DIM` modifier is sufficient. fn style_gutter_for(kind: DiffLineType, theme: DiffTheme, color_level: DiffColorLevel) -> Style { - match (theme, kind) { - (DiffTheme::Light, DiffLineType::Insert) => Style::default() + match ( + theme, + kind, + RichDiffColorLevel::from_diff_color_level(color_level), + ) { + (DiffTheme::Light, DiffLineType::Insert, None) => { + Style::default().fg(light_gutter_fg(color_level)) + } + (DiffTheme::Light, DiffLineType::Delete, None) => { + Style::default().fg(light_gutter_fg(color_level)) + } + (DiffTheme::Light, DiffLineType::Insert, Some(level)) => Style::default() .fg(light_gutter_fg(color_level)) - .bg(light_add_num_bg(color_level)), - (DiffTheme::Light, DiffLineType::Delete) => Style::default() + .bg(light_add_num_bg(level)), + (DiffTheme::Light, DiffLineType::Delete, Some(level)) => Style::default() .fg(light_gutter_fg(color_level)) - .bg(light_del_num_bg(color_level)), + .bg(light_del_num_bg(level)), _ => style_gutter_dim(), } } @@ -1001,26 +1096,38 @@ fn style_sign_del(theme: DiffTheme, color_level: DiffColorLevel) -> Style { /// Content style for insert lines (plain, non-syntax-highlighted text). fn style_add(theme: DiffTheme, color_level: DiffColorLevel) -> Style { match (theme, color_level) { - (DiffTheme::Dark, DiffColorLevel::Ansi16) => Style::default() - .fg(Color::Black) - .bg(add_line_bg(theme, color_level)), - (DiffTheme::Light, _) => Style::default().bg(add_line_bg(theme, color_level)), - (DiffTheme::Dark, _) => Style::default() + (_, DiffColorLevel::Ansi16) => Style::default().fg(Color::Green), + (DiffTheme::Light, DiffColorLevel::TrueColor) => { + Style::default().bg(add_line_bg(theme, RichDiffColorLevel::TrueColor)) + } + (DiffTheme::Light, DiffColorLevel::Ansi256) => { + Style::default().bg(add_line_bg(theme, RichDiffColorLevel::Ansi256)) + } + (DiffTheme::Dark, DiffColorLevel::TrueColor) => Style::default() .fg(Color::Green) - .bg(add_line_bg(theme, color_level)), + .bg(add_line_bg(theme, RichDiffColorLevel::TrueColor)), + (DiffTheme::Dark, DiffColorLevel::Ansi256) => Style::default() + .fg(Color::Green) + .bg(add_line_bg(theme, RichDiffColorLevel::Ansi256)), } } /// Content style for delete lines (plain, non-syntax-highlighted text). fn style_del(theme: DiffTheme, color_level: DiffColorLevel) -> Style { match (theme, color_level) { - (DiffTheme::Dark, DiffColorLevel::Ansi16) => Style::default() - .fg(Color::Black) - .bg(del_line_bg(theme, color_level)), - (DiffTheme::Light, _) => Style::default().bg(del_line_bg(theme, color_level)), - (DiffTheme::Dark, _) => Style::default() + (_, DiffColorLevel::Ansi16) => Style::default().fg(Color::Red), + (DiffTheme::Light, DiffColorLevel::TrueColor) => { + Style::default().bg(del_line_bg(theme, RichDiffColorLevel::TrueColor)) + } + (DiffTheme::Light, DiffColorLevel::Ansi256) => { + Style::default().bg(del_line_bg(theme, RichDiffColorLevel::Ansi256)) + } + (DiffTheme::Dark, DiffColorLevel::TrueColor) => Style::default() .fg(Color::Red) - .bg(del_line_bg(theme, color_level)), + .bg(del_line_bg(theme, RichDiffColorLevel::TrueColor)), + (DiffTheme::Dark, DiffColorLevel::Ansi256) => Style::default() + .fg(Color::Red) + .bg(del_line_bg(theme, RichDiffColorLevel::Ansi256)), } } @@ -1039,32 +1146,28 @@ mod tests { use ratatui::widgets::Paragraph; #[test] - fn dark_ansi16_add_style_has_contrast() { + fn ansi16_add_style_uses_foreground_only() { let style = style_add(DiffTheme::Dark, DiffColorLevel::Ansi16); - assert_eq!(style.fg, Some(Color::Black)); - assert_eq!(style.bg, Some(Color::Green)); - assert_ne!(style.fg, style.bg); + assert_eq!(style.fg, Some(Color::Green)); + assert_eq!(style.bg, None); } #[test] - fn dark_ansi16_del_style_has_contrast() { + fn ansi16_del_style_uses_foreground_only() { let style = style_del(DiffTheme::Dark, DiffColorLevel::Ansi16); - assert_eq!(style.fg, Some(Color::Black)); - assert_eq!(style.bg, Some(Color::Red)); - assert_ne!(style.fg, style.bg); + assert_eq!(style.fg, Some(Color::Red)); + assert_eq!(style.bg, None); } #[test] - fn dark_ansi16_sign_styles_have_contrast() { + fn ansi16_sign_styles_use_foreground_only() { let add_sign = style_sign_add(DiffTheme::Dark, DiffColorLevel::Ansi16); - assert_eq!(add_sign.fg, Some(Color::Black)); - assert_eq!(add_sign.bg, Some(Color::Green)); - assert_ne!(add_sign.fg, add_sign.bg); + assert_eq!(add_sign.fg, Some(Color::Green)); + assert_eq!(add_sign.bg, None); let del_sign = style_sign_del(DiffTheme::Dark, DiffColorLevel::Ansi16); - assert_eq!(del_sign.fg, Some(Color::Black)); - assert_eq!(del_sign.bg, Some(Color::Red)); - assert_ne!(del_sign.fg, del_sign.bg); + assert_eq!(del_sign.fg, Some(Color::Red)); + assert_eq!(del_sign.bg, None); } use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; @@ -1466,6 +1569,32 @@ mod tests { snapshot_diff_gallery("diff_gallery_120x40", 120, 40); } + #[test] + fn ui_snapshot_ansi16_insert_delete_no_background() { + let mut lines = push_wrapped_diff_line_inner_with_theme_and_color_level( + 1, + DiffLineType::Insert, + "added in ansi16 mode", + 80, + line_number_width(2), + None, + DiffTheme::Dark, + DiffColorLevel::Ansi16, + ); + lines.extend(push_wrapped_diff_line_inner_with_theme_and_color_level( + 2, + DiffLineType::Delete, + "deleted in ansi16 mode", + 80, + line_number_width(2), + None, + DiffTheme::Dark, + DiffColorLevel::Ansi16, + )); + + snapshot_lines("ansi16_insert_delete_no_background", lines, 40, 4); + } + #[test] fn truecolor_dark_theme_uses_configured_backgrounds() { assert_eq!( @@ -1535,6 +1664,42 @@ mod tests { ); } + #[test] + fn ansi16_disables_line_and_gutter_backgrounds() { + assert_eq!( + style_line_bg_for( + DiffLineType::Insert, + DiffTheme::Dark, + DiffColorLevel::Ansi16 + ), + Style::default() + ); + assert_eq!( + style_line_bg_for( + DiffLineType::Delete, + DiffTheme::Light, + DiffColorLevel::Ansi16 + ), + Style::default() + ); + assert_eq!( + style_gutter_for( + DiffLineType::Insert, + DiffTheme::Light, + DiffColorLevel::Ansi16 + ), + Style::default().fg(Color::Black) + ); + assert_eq!( + style_gutter_for( + DiffLineType::Delete, + DiffTheme::Light, + DiffColorLevel::Ansi16 + ), + Style::default().fg(Color::Black) + ); + } + #[test] fn light_truecolor_theme_uses_readable_gutter_and_line_backgrounds() { assert_eq!( @@ -1608,6 +1773,97 @@ mod tests { assert_eq!(lines[1].style.bg, Some(rgb_color(LIGHT_TC_ADD_LINE_BG_RGB))); } + #[test] + fn windows_terminal_promotes_ansi16_to_truecolor_for_diffs() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Ansi16, + TerminalName::WindowsTerminal, + false, + false, + ), + DiffColorLevel::TrueColor + ); + } + + #[test] + fn wt_session_promotes_ansi16_to_truecolor_for_diffs() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Ansi16, + TerminalName::Unknown, + true, + false, + ), + DiffColorLevel::TrueColor + ); + } + + #[test] + fn non_windows_terminal_keeps_ansi16_diff_palette() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Ansi16, + TerminalName::WezTerm, + false, + false, + ), + DiffColorLevel::Ansi16 + ); + } + + #[test] + fn wt_session_promotes_unknown_color_level_to_truecolor() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Unknown, + TerminalName::WindowsTerminal, + true, + false, + ), + DiffColorLevel::TrueColor + ); + } + + #[test] + fn non_wt_windows_terminal_keeps_unknown_color_level_conservative() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Unknown, + TerminalName::WindowsTerminal, + false, + false, + ), + DiffColorLevel::Ansi16 + ); + } + + #[test] + fn explicit_force_override_keeps_ansi16_on_windows_terminal() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Ansi16, + TerminalName::WindowsTerminal, + false, + true, + ), + DiffColorLevel::Ansi16 + ); + } + + #[test] + fn explicit_force_override_keeps_ansi256_on_windows_terminal() { + assert_eq!( + diff_color_level_for_terminal( + StdoutColorLevel::Ansi256, + TerminalName::WindowsTerminal, + true, + true, + ), + DiffColorLevel::Ansi256 + ); + } + #[test] fn add_diff_uses_path_extension_for_highlighting() { let mut changes: HashMap = HashMap::new(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index affbb7297d..624d47d915 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -2633,6 +2633,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }; let mut servers = config.mcp_servers.get().clone(); servers.insert("docs".to_string(), stdio_config); @@ -2656,6 +2657,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }; servers.insert("http".to_string(), http_config); config diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 2799c80b24..bbd0307da3 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -51,6 +51,7 @@ pub enum SlashCommand { Clear, Personality, Realtime, + Settings, TestApproval, // Debugging commands. #[strum(serialize = "debug-m-drop")] @@ -89,6 +90,7 @@ impl SlashCommand { SlashCommand::Model => "choose what model and reasoning effort to use", SlashCommand::Personality => "choose a communication style for Codex", SlashCommand::Realtime => "toggle realtime voice mode (experimental)", + SlashCommand::Settings => "configure realtime microphone/speaker", SlashCommand::Plan => "switch to Plan mode", SlashCommand::Collab => "change collaboration mode (experimental)", SlashCommand::Agent => "switch the active agent thread", @@ -163,6 +165,7 @@ impl SlashCommand { SlashCommand::Rollout => true, SlashCommand::TestApproval => true, SlashCommand::Realtime => true, + SlashCommand::Settings => true, SlashCommand::Collab => true, SlashCommand::Agent => true, SlashCommand::Statusline => false, diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__ansi16_insert_delete_no_background.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__ansi16_insert_delete_no_background.snap new file mode 100644 index 0000000000..0614873eae --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__ansi16_insert_delete_no_background.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"1 +added in ansi16 mode " +"2 -deleted in ansi16 mode " +" " +" " diff --git a/codex-rs/tui/src/voice.rs b/codex-rs/tui/src/voice.rs index 6c4236ac81..227d27c88f 100644 --- a/codex-rs/tui/src/voice.rs +++ b/codex-rs/tui/src/voice.rs @@ -27,6 +27,10 @@ use tracing::error; use tracing::info; use tracing::trace; +const AUDIO_MODEL: &str = "gpt-4o-mini-transcribe"; +const MODEL_AUDIO_SAMPLE_RATE: u32 = 24_000; +const MODEL_AUDIO_CHANNELS: u16 = 1; + struct TranscriptionAuthContext { mode: AuthMode, bearer_token: String, @@ -268,9 +272,7 @@ fn select_default_input_device_and_config() let device = host .default_input_device() .ok_or_else(|| "no input audio device available".to_string())?; - let config = device - .default_input_config() - .map_err(|e| format!("failed to get default input config: {e}"))?; + let config = crate::audio_device::preferred_input_config(&device)?; Ok((device, config)) } @@ -395,20 +397,35 @@ fn send_realtime_audio_chunk( return; } + let samples = if sample_rate == MODEL_AUDIO_SAMPLE_RATE && channels == MODEL_AUDIO_CHANNELS { + samples + } else { + convert_pcm16( + &samples, + sample_rate, + channels, + MODEL_AUDIO_SAMPLE_RATE, + MODEL_AUDIO_CHANNELS, + ) + }; + if samples.is_empty() { + return; + } + let mut bytes = Vec::with_capacity(samples.len() * 2); for sample in &samples { bytes.extend_from_slice(&sample.to_le_bytes()); } let encoded = base64::engine::general_purpose::STANDARD.encode(bytes); - let samples_per_channel = (samples.len() / usize::from(channels)) as u32; + let samples_per_channel = (samples.len() / usize::from(MODEL_AUDIO_CHANNELS)) as u32; tx.send(AppEvent::CodexOp(Op::RealtimeConversationAudio( ConversationAudioParams { frame: RealtimeAudioFrame { data: encoded, - sample_rate, - num_channels: channels, + sample_rate: MODEL_AUDIO_SAMPLE_RATE, + num_channels: MODEL_AUDIO_CHANNELS, samples_per_channel: Some(samples_per_channel), }, }, @@ -505,7 +522,7 @@ impl RealtimeAudioPlayer { for pair in raw_bytes.chunks_exact(2) { pcm.push(i16::from_le_bytes([pair[0], pair[1]])); } - let converted = convert_pcm16_for_output( + let converted = convert_pcm16( &pcm, frame.sample_rate, frame.num_channels, @@ -598,7 +615,7 @@ fn fill_output_u16(output: &mut [u16], queue: &Arc>>) { output.fill(32768); } -fn convert_pcm16_for_output( +fn convert_pcm16( input: &[i16], input_sample_rate: u32, input_channels: u16, @@ -672,10 +689,29 @@ fn clip_duration_seconds(audio: &RecordedAudio) -> f32 { } fn encode_wav_normalized(audio: &RecordedAudio) -> Result, String> { + let converted; + let (channels, sample_rate, segment) = + if audio.channels == MODEL_AUDIO_CHANNELS && audio.sample_rate == MODEL_AUDIO_SAMPLE_RATE { + (audio.channels, audio.sample_rate, audio.data.as_slice()) + } else { + converted = convert_pcm16( + &audio.data, + audio.sample_rate, + audio.channels, + MODEL_AUDIO_SAMPLE_RATE, + MODEL_AUDIO_CHANNELS, + ); + ( + MODEL_AUDIO_CHANNELS, + MODEL_AUDIO_SAMPLE_RATE, + converted.as_slice(), + ) + }; + let mut wav_bytes: Vec = Vec::new(); let spec = WavSpec { - channels: audio.channels, - sample_rate: audio.sample_rate, + channels, + sample_rate, bits_per_sample: 16, sample_format: SampleFormat::Int, }; @@ -684,7 +720,6 @@ fn encode_wav_normalized(audio: &RecordedAudio) -> Result, String> { WavWriter::new(&mut cursor, spec).map_err(|_| "failed to create wav writer".to_string())?; // Simple peak normalization with headroom to improve audibility on quiet inputs. - let segment = &audio.data[..]; let mut peak: i16 = 0; for &s in segment { let a = s.unsigned_abs(); @@ -782,7 +817,7 @@ async fn transcribe_bytes( .mime_str("audio/wav") .map_err(|e| format!("failed to set mime: {e}"))?; let mut form = reqwest::multipart::Form::new() - .text("model", "gpt-4o-transcribe") + .text("model", AUDIO_MODEL) .part("file", part); if let Some(context) = context { form = form.text("prompt", context); @@ -834,3 +869,40 @@ async fn transcribe_bytes( Ok(text) } } + +#[cfg(test)] +mod tests { + use super::RecordedAudio; + use super::convert_pcm16; + use super::encode_wav_normalized; + use pretty_assertions::assert_eq; + use std::io::Cursor; + + #[test] + fn convert_pcm16_downmixes_and_resamples_for_model_input() { + let input = vec![100, 300, 200, 400, 500, 700, 600, 800]; + let converted = convert_pcm16(&input, 48_000, 2, 24_000, 1); + assert_eq!(converted, vec![200, 700]); + } + + #[test] + fn encode_wav_normalized_outputs_24khz_mono_audio() { + let audio = RecordedAudio { + data: vec![100, 300, 200, 400, 500, 700, 600, 800], + sample_rate: 48_000, + channels: 2, + }; + + let wav = encode_wav_normalized(&audio).expect("wav should encode"); + let reader = hound::WavReader::new(Cursor::new(wav)).expect("wav should parse"); + let spec = reader.spec(); + let samples = reader + .into_samples::() + .collect::, _>>() + .expect("samples should decode"); + + assert_eq!(spec.channels, 1); + assert_eq!(spec.sample_rate, 24_000); + assert_eq!(samples, vec![8_426, 29_490]); + } +} diff --git a/docs/config.md b/docs/config.md index 30665bb11b..fc9d62b8e8 100644 --- a/docs/config.md +++ b/docs/config.md @@ -24,6 +24,8 @@ Codex can run a notification hook when the agent finishes a turn. See the config - https://developers.openai.com/codex/config-reference +When Codex knows which client started the turn, the legacy notify JSON payload also includes a top-level `client` field. The TUI reports `codex-tui`, and the app server reports the `clientInfo.name` value from `initialize`. + ## JSON Schema The generated JSON Schema for `config.toml` lives at `codex-rs/core/config.schema.json`.