diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 8e7d022ce0..66b50fa745 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1293,6 +1293,25 @@ dependencies = [ "zstd", ] +[[package]] +name = "codex-cloud-requirements" +version = "0.0.0" +dependencies = [ + "async-trait", + "base64", + "codex-app-server-protocol", + "codex-backend-client", + "codex-core", + "codex-otel", + "codex-protocol", + "pretty_assertions", + "serde_json", + "tempfile", + "tokio", + "toml 0.9.5", + "tracing", +] + [[package]] name = "codex-cloud-tasks" version = "0.0.0" @@ -1896,6 +1915,7 @@ dependencies = [ "codex-backend-client", "codex-chatgpt", "codex-cli", + "codex-cloud-requirements", "codex-common", "codex-core", "codex-feedback", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 5196213991..327e6beeb0 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -11,6 +11,7 @@ members = [ "arg0", "feedback", "codex-backend-openapi-models", + "cloud-requirements", "cloud-tasks", "cloud-tasks-client", "cli", @@ -71,6 +72,7 @@ codex-apply-patch = { path = "apply-patch" } codex-arg0 = { path = "arg0" } codex-async-utils = { path = "async-utils" } codex-backend-client = { path = "backend-client" } +codex-cloud-requirements = { path = "cloud-requirements" } codex-chatgpt = { path = "chatgpt" } codex-cli = { path = "cli"} codex-client = { path = "codex-client" } diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 164c2aab6e..3b8c4ea63d 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -128,6 +128,10 @@ client_request_definitions! { params: v2::ThreadArchiveParams, response: v2::ThreadArchiveResponse, }, + ThreadSetName => "thread/name/set" { + params: v2::ThreadSetNameParams, + response: v2::ThreadSetNameResponse, + }, ThreadUnarchive => "thread/unarchive" { params: v2::ThreadUnarchiveParams, response: v2::ThreadUnarchiveResponse, @@ -594,6 +598,7 @@ server_notification_definitions! { /// NEW NOTIFICATIONS Error => "error" (v2::ErrorNotification), ThreadStarted => "thread/started" (v2::ThreadStartedNotification), + ThreadNameUpdated => "thread/name/updated" (v2::ThreadNameUpdatedNotification), ThreadTokenUsageUpdated => "thread/tokenUsage/updated" (v2::ThreadTokenUsageUpdatedNotification), TurnStarted => "turn/started" (v2::TurnStartedNotification), TurnCompleted => "turn/completed" (v2::TurnCompletedNotification), diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index b777282d47..ff8029cdb0 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1312,6 +1312,14 @@ pub struct ThreadArchiveParams { #[ts(export_to = "v2/")] pub struct ThreadArchiveResponse {} +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadSetNameParams { + pub thread_id: String, + pub name: String, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -1319,6 +1327,11 @@ pub struct ThreadUnarchiveParams { pub thread_id: String, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadSetNameResponse {} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -2302,6 +2315,16 @@ pub struct ThreadStartedNotification { pub thread: Thread, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct ThreadNameUpdatedNotification { + pub thread_id: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub thread_name: Option, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -2610,7 +2633,7 @@ pub struct ToolRequestUserInputOption { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] -/// EXPERIMENTAL. Represents one request_user_input question and its optional options. +/// EXPERIMENTAL. Represents one request_user_input question and its required options. pub struct ToolRequestUserInputQuestion { pub id: String, pub header: String, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index e9ebcd9661..3798d76027 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -82,6 +82,7 @@ Example (from OpenAI's official VSCode extension): - `thread/loaded/list` — list the thread ids currently loaded in memory. - `thread/read` — read a stored thread by id without resuming it; optionally include turns via `includeTurns`. - `thread/archive` — move a thread’s rollout file into the archived directory; returns `{}` on success. +- `thread/name/set` — set or update a thread’s user-facing name; returns `{}` on success. Thread names are not required to be unique; name lookups resolve to the most recently updated thread. - `thread/unarchive` — move an archived rollout file back into the sessions directory; returns the restored `thread` on success. - `thread/rollback` — drop the last N turns from the agent’s in-memory context and persist a rollback marker in the rollout so future resumes see the pruned history; returns the updated `thread` (with `turns` populated) on success. - `turn/start` — add user input to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications. diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 1a9fcaa705..dd2628deb7 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -52,6 +52,7 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; use codex_app_server_protocol::TerminalInteractionNotification; use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadNameUpdatedNotification; use codex_app_server_protocol::ThreadRollbackResponse; use codex_app_server_protocol::ThreadTokenUsage; use codex_app_server_protocol::ThreadTokenUsageUpdatedNotification; @@ -1097,6 +1098,17 @@ pub(crate) async fn apply_bespoke_event_handling( outgoing.send_response(request_id, response).await; } } + EventMsg::ThreadNameUpdated(thread_name_event) => { + if let ApiVersion::V2 = api_version { + let notification = ThreadNameUpdatedNotification { + thread_id: thread_name_event.thread_id.to_string(), + thread_name: thread_name_event.thread_name, + }; + outgoing + .send_server_notification(ServerNotification::ThreadNameUpdated(notification)) + .await; + } + } EventMsg::TurnDiff(turn_diff_event) => { handle_turn_diff( conversation_id, diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 8ee926126f..e09bbfdbd6 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -110,6 +110,8 @@ use codex_app_server_protocol::ThreadReadResponse; use codex_app_server_protocol::ThreadResumeParams; use codex_app_server_protocol::ThreadResumeResponse; use codex_app_server_protocol::ThreadRollbackParams; +use codex_app_server_protocol::ThreadSetNameParams; +use codex_app_server_protocol::ThreadSetNameResponse; use codex_app_server_protocol::ThreadSortKey; use codex_app_server_protocol::ThreadSourceKind; use codex_app_server_protocol::ThreadStartParams; @@ -418,6 +420,9 @@ impl CodexMessageProcessor { ClientRequest::ThreadArchive { request_id, params } => { self.thread_archive(request_id, params).await; } + ClientRequest::ThreadSetName { request_id, params } => { + self.thread_set_name(request_id, params).await; + } ClientRequest::ThreadUnarchive { request_id, params } => { self.thread_unarchive(request_id, params).await; } @@ -1877,6 +1882,36 @@ impl CodexMessageProcessor { } } + async fn thread_set_name(&self, request_id: RequestId, params: ThreadSetNameParams) { + let ThreadSetNameParams { thread_id, name } = params; + let Some(name) = codex_core::util::normalize_thread_name(&name) else { + self.send_invalid_request_error( + request_id, + "thread name must not be empty".to_string(), + ) + .await; + return; + }; + + let (_, thread) = match self.load_thread(&thread_id).await { + Ok(v) => v, + Err(error) => { + self.outgoing.send_error(request_id, error).await; + return; + } + }; + + if let Err(err) = thread.submit(Op::SetThreadName { name }).await { + self.send_internal_error(request_id, format!("failed to set thread name: {err}")) + .await; + return; + } + + self.outgoing + .send_response(request_id, ThreadSetNameResponse {}) + .await; + } + async fn thread_unarchive(&mut self, request_id: RequestId, params: ThreadUnarchiveParams) { // TODO(jif) mostly rewrite this using sqlite after phase 1 let thread_id = match ThreadId::from_string(¶ms.thread_id) { diff --git a/codex-rs/app-server/tests/common/models_cache.rs b/codex-rs/app-server/tests/common/models_cache.rs index 7a093dc7f0..59fddc2427 100644 --- a/codex-rs/app-server/tests/common/models_cache.rs +++ b/codex-rs/app-server/tests/common/models_cache.rs @@ -27,7 +27,7 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo { priority, upgrade: preset.upgrade.as_ref().map(|u| u.into()), base_instructions: "base instructions".to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, diff --git a/codex-rs/app-server/tests/common/responses.rs b/codex-rs/app-server/tests/common/responses.rs index 2a5b5d2159..e15319e02f 100644 --- a/codex-rs/app-server/tests/common/responses.rs +++ b/codex-rs/app-server/tests/common/responses.rs @@ -67,7 +67,6 @@ pub fn create_request_user_input_sse_response(call_id: &str) -> anyhow::Result Result<()> { @@ -368,7 +368,7 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> { } #[tokio::test] -async fn thread_resume_accepts_personality_override_v2() -> Result<()> { +async fn thread_resume_accepts_personality_override() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -438,14 +438,14 @@ async fn thread_resume_accepts_personality_override_v2() -> Result<()> { let request = response_mock.single_request(); let developer_texts = request.message_input_texts("developer"); assert!( - !developer_texts + developer_texts .iter() .any(|text| text.contains("")), - "did not expect a personality update message in developer input, got {developer_texts:?}" + "expected a personality update message in developer input, got {developer_texts:?}" ); let instructions_text = request.instructions_text(); assert!( - instructions_text.contains(DEFAULT_BASE_INSTRUCTIONS), + instructions_text.contains(CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT), "expected default base instructions from history, got {instructions_text:?}" ); @@ -459,7 +459,7 @@ fn create_config_toml(codex_home: &std::path::Path, server_uri: &str) -> std::io config_toml, format!( r#" -model = "mock-model" +model = "gpt-5.2-codex" approval_policy = "never" sandbox_mode = "read-only" @@ -467,6 +467,7 @@ model_provider = "mock_provider" [features] remote_models = false +personality = true [model_providers.mock_provider] name = "Mock provider for test" diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 3abe73e2e9..99eff9ecb2 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -63,7 +63,7 @@ async fn turn_start_sends_originator_header() -> Result<()> { codex_home.path(), &server.uri(), "never", - &BTreeMap::default(), + &BTreeMap::from([(Feature::Personality, true)]), )?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -138,7 +138,7 @@ async fn turn_start_emits_user_message_item_with_text_elements() -> Result<()> { codex_home.path(), &server.uri(), "never", - &BTreeMap::default(), + &BTreeMap::from([(Feature::Personality, true)]), )?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -230,7 +230,7 @@ async fn turn_start_emits_notifications_and_accepts_model_override() -> Result<( codex_home.path(), &server.uri(), "never", - &BTreeMap::default(), + &BTreeMap::from([(Feature::Personality, true)]), )?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -425,7 +425,7 @@ async fn turn_start_accepts_personality_override_v2() -> Result<()> { codex_home.path(), &server.uri(), "never", - &BTreeMap::default(), + &BTreeMap::from([(Feature::Personality, true)]), )?; let mut mcp = McpProcess::new(codex_home.path()).await?; @@ -473,6 +473,7 @@ async fn turn_start_accepts_personality_override_v2() -> Result<()> { if developer_texts.is_empty() { eprintln!("request body: {}", request.body_json()); } + assert!( developer_texts .iter() diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index c1a8f65359..83b7271583 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -221,7 +221,11 @@ fn normalize_connector_value(value: Option<&str>) -> Option { } const ALLOWED_APPS_SDK_APPS: &[&str] = &["asdk_app_69781557cc1481919cf5e9824fa2e792"]; -const DISALLOWED_CONNECTOR_IDS: &[&str] = &["asdk_app_6938a94a61d881918ef32cb999ff937c"]; +const DISALLOWED_CONNECTOR_IDS: &[&str] = &[ + "asdk_app_6938a94a61d881918ef32cb999ff937c", + "connector_2b0a9009c9c64bf9933a3dae3f2b1254", + "connector_68de829bf7648191acd70a907364c67c", +]; const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; fn filter_disallowed_connectors(connectors: Vec) -> Vec { diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index df8936e752..49343e33c8 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -144,7 +144,7 @@ struct CompletionCommand { #[derive(Debug, Parser)] struct ResumeCommand { - /// Conversation/session id (UUID). When provided, resumes this session. + /// Conversation/session id (UUID) or thread name. UUIDs take precedence if it parses. /// If omitted, use --last to pick the most recent recorded session. #[arg(value_name = "SESSION_ID")] session_id: Option, @@ -327,6 +327,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec Vec) -> AppExitInfo { + fn sample_exit_info(conversation_id: Option<&str>, thread_name: Option<&str>) -> AppExitInfo { let token_usage = TokenUsage { output_tokens: 2, total_tokens: 2, @@ -1041,7 +1043,10 @@ mod tests { }; AppExitInfo { token_usage, - thread_id: conversation.map(ThreadId::from_string).map(Result::unwrap), + thread_id: conversation_id + .map(ThreadId::from_string) + .map(Result::unwrap), + thread_name: thread_name.map(str::to_string), update_action: None, exit_reason: ExitReason::UserRequested, } @@ -1052,6 +1057,7 @@ mod tests { let exit_info = AppExitInfo { token_usage: TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::UserRequested, }; @@ -1061,7 +1067,7 @@ mod tests { #[test] fn format_exit_messages_includes_resume_hint_without_color() { - let exit_info = sample_exit_info(Some("123e4567-e89b-12d3-a456-426614174000")); + let exit_info = sample_exit_info(Some("123e4567-e89b-12d3-a456-426614174000"), None); let lines = format_exit_messages(exit_info, false); assert_eq!( lines, @@ -1075,12 +1081,28 @@ mod tests { #[test] fn format_exit_messages_applies_color_when_enabled() { - let exit_info = sample_exit_info(Some("123e4567-e89b-12d3-a456-426614174000")); + let exit_info = sample_exit_info(Some("123e4567-e89b-12d3-a456-426614174000"), None); let lines = format_exit_messages(exit_info, true); assert_eq!(lines.len(), 2); assert!(lines[1].contains("\u{1b}[36m")); } + #[test] + fn format_exit_messages_prefers_thread_name() { + let exit_info = sample_exit_info( + Some("123e4567-e89b-12d3-a456-426614174000"), + Some("my-thread"), + ); + let lines = format_exit_messages(exit_info, false); + assert_eq!( + lines, + vec![ + "Token usage: total=2 input=0 output=2".to_string(), + "To continue this session, run codex resume my-thread".to_string(), + ] + ); + } + #[test] fn resume_model_flag_applies_when_no_root_flags() { let interactive = diff --git a/codex-rs/cloud-requirements/BUILD.bazel b/codex-rs/cloud-requirements/BUILD.bazel new file mode 100644 index 0000000000..88243aff90 --- /dev/null +++ b/codex-rs/cloud-requirements/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "cloud-requirements", + crate_name = "codex_cloud_requirements", +) diff --git a/codex-rs/cloud-requirements/Cargo.toml b/codex-rs/cloud-requirements/Cargo.toml new file mode 100644 index 0000000000..2bfde660bf --- /dev/null +++ b/codex-rs/cloud-requirements/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "codex-cloud-requirements" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lints] +workspace = true + +[dependencies] +async-trait = { workspace = true } +codex-app-server-protocol = { workspace = true } +codex-backend-client = { workspace = true } +codex-core = { workspace = true } +codex-otel = { workspace = true } +codex-protocol = { workspace = true } +tokio = { workspace = true, features = ["sync", "time"] } +toml = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +base64 = { workspace = true } +pretty_assertions = { workspace = true } +serde_json = { workspace = true } +tempfile = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt", "test-util", "time"] } diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs new file mode 100644 index 0000000000..800d5313f5 --- /dev/null +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -0,0 +1,335 @@ +//! Cloud-hosted config requirements for Codex. +//! +//! This crate fetches `requirements.toml` data from the backend as an alternative to loading it +//! from the local filesystem. It only applies to Enterprise ChatGPT customers. +//! +//! Today, fetching is best-effort: on error or timeout, Codex continues without cloud requirements. +//! We expect to tighten this so that Enterprise ChatGPT customers must successfully fetch these +//! requirements before Codex will run. + +use async_trait::async_trait; +use codex_backend_client::Client as BackendClient; +use codex_core::AuthManager; +use codex_core::auth::CodexAuth; +use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::ConfigRequirementsToml; +use codex_protocol::account::PlanType; +use std::sync::Arc; +use std::time::Duration; +use std::time::Instant; +use tokio::time::timeout; + +/// This blocks codecs startup, so must be short. +const CLOUD_REQUIREMENTS_TIMEOUT: Duration = Duration::from_secs(5); + +#[async_trait] +trait RequirementsFetcher: Send + Sync { + /// Returns requirements as a TOML string. + /// + /// TODO(gt): For now, returns an Option. But when we want to make this fail-closed, return a + /// Result. + async fn fetch_requirements(&self, auth: &CodexAuth) -> Option; +} + +struct BackendRequirementsFetcher { + base_url: String, +} + +impl BackendRequirementsFetcher { + fn new(base_url: String) -> Self { + Self { base_url } + } +} + +#[async_trait] +impl RequirementsFetcher for BackendRequirementsFetcher { + async fn fetch_requirements(&self, auth: &CodexAuth) -> Option { + let client = BackendClient::from_auth(self.base_url.clone(), auth) + .inspect_err(|err| { + tracing::warn!( + error = %err, + "Failed to construct backend client for cloud requirements" + ); + }) + .ok()?; + + let response = client + .get_config_requirements_file() + .await + .inspect_err(|err| tracing::warn!(error = %err, "Failed to fetch cloud requirements")) + .ok()?; + + let Some(contents) = response.contents else { + tracing::warn!("Cloud requirements response missing contents"); + return None; + }; + + Some(contents) + } +} + +struct CloudRequirementsService { + auth_manager: Arc, + fetcher: Arc, + timeout: Duration, +} + +impl CloudRequirementsService { + fn new( + auth_manager: Arc, + fetcher: Arc, + timeout: Duration, + ) -> Self { + Self { + auth_manager, + fetcher, + timeout, + } + } + + async fn fetch_with_timeout(&self) -> Option { + let _timer = + codex_otel::start_global_timer("codex.cloud_requirements.fetch.duration_ms", &[]); + let started_at = Instant::now(); + let result = timeout(self.timeout, self.fetch()) + .await + .inspect_err(|_| { + tracing::warn!("Timed out waiting for cloud requirements; continuing without them"); + }) + .ok()?; + + match result.as_ref() { + Some(requirements) => { + tracing::info!( + elapsed_ms = started_at.elapsed().as_millis(), + requirements = ?requirements, + "Cloud requirements load completed" + ); + } + None => { + tracing::info!( + elapsed_ms = started_at.elapsed().as_millis(), + "Cloud requirements load completed (none)" + ); + } + } + + result + } + + async fn fetch(&self) -> Option { + let auth = self.auth_manager.auth().await?; + if !(auth.is_chatgpt_auth() && auth.account_plan_type() == Some(PlanType::Enterprise)) { + return None; + } + + let contents = self.fetcher.fetch_requirements(&auth).await?; + parse_cloud_requirements(&contents) + .inspect_err(|err| tracing::warn!(error = %err, "Failed to parse cloud requirements")) + .ok() + .flatten() + } +} + +pub fn cloud_requirements_loader( + auth_manager: Arc, + chatgpt_base_url: String, +) -> CloudRequirementsLoader { + let service = CloudRequirementsService::new( + auth_manager, + Arc::new(BackendRequirementsFetcher::new(chatgpt_base_url)), + CLOUD_REQUIREMENTS_TIMEOUT, + ); + let task = tokio::spawn(async move { service.fetch_with_timeout().await }); + CloudRequirementsLoader::new(async move { + task.await + .inspect_err(|err| tracing::warn!(error = %err, "Cloud requirements task failed")) + .ok() + .flatten() + }) +} + +fn parse_cloud_requirements( + contents: &str, +) -> Result, toml::de::Error> { + if contents.trim().is_empty() { + return Ok(None); + } + + let requirements: ConfigRequirementsToml = toml::from_str(contents)?; + if requirements.is_empty() { + Ok(None) + } else { + Ok(Some(requirements)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use base64::Engine; + use base64::engine::general_purpose::URL_SAFE_NO_PAD; + use codex_core::auth::AuthCredentialsStoreMode; + use codex_protocol::protocol::AskForApproval; + use pretty_assertions::assert_eq; + use serde_json::json; + use std::future::pending; + use std::path::Path; + use tempfile::tempdir; + + fn write_auth_json(codex_home: &Path, value: serde_json::Value) -> std::io::Result<()> { + std::fs::write(codex_home.join("auth.json"), serde_json::to_string(&value)?)?; + Ok(()) + } + + fn auth_manager_with_api_key() -> Arc { + let tmp = tempdir().expect("tempdir"); + let auth_json = json!({ + "OPENAI_API_KEY": "sk-test-key", + "tokens": null, + "last_refresh": null, + }); + write_auth_json(tmp.path(), auth_json).expect("write auth"); + Arc::new(AuthManager::new( + tmp.path().to_path_buf(), + false, + AuthCredentialsStoreMode::File, + )) + } + + fn auth_manager_with_plan(plan_type: &str) -> Arc { + let tmp = tempdir().expect("tempdir"); + let header = json!({ "alg": "none", "typ": "JWT" }); + let auth_payload = json!({ + "chatgpt_plan_type": plan_type, + "chatgpt_user_id": "user-12345", + "user_id": "user-12345", + }); + let payload = json!({ + "email": "user@example.com", + "https://api.openai.com/auth": auth_payload, + }); + let header_b64 = URL_SAFE_NO_PAD.encode(serde_json::to_vec(&header).expect("header")); + let payload_b64 = URL_SAFE_NO_PAD.encode(serde_json::to_vec(&payload).expect("payload")); + let signature_b64 = URL_SAFE_NO_PAD.encode(b"sig"); + let fake_jwt = format!("{header_b64}.{payload_b64}.{signature_b64}"); + + let auth_json = json!({ + "OPENAI_API_KEY": null, + "tokens": { + "id_token": fake_jwt, + "access_token": "test-access-token", + "refresh_token": "test-refresh-token", + }, + "last_refresh": null, + }); + write_auth_json(tmp.path(), auth_json).expect("write auth"); + Arc::new(AuthManager::new( + tmp.path().to_path_buf(), + false, + AuthCredentialsStoreMode::File, + )) + } + + fn parse_for_fetch(contents: Option<&str>) -> Option { + contents.and_then(|contents| parse_cloud_requirements(contents).ok().flatten()) + } + + struct StaticFetcher { + contents: Option, + } + + #[async_trait::async_trait] + impl RequirementsFetcher for StaticFetcher { + async fn fetch_requirements(&self, _auth: &CodexAuth) -> Option { + self.contents.clone() + } + } + + struct PendingFetcher; + + #[async_trait::async_trait] + impl RequirementsFetcher for PendingFetcher { + async fn fetch_requirements(&self, _auth: &CodexAuth) -> Option { + pending::<()>().await; + None + } + } + + #[tokio::test] + async fn fetch_cloud_requirements_skips_non_chatgpt_auth() { + let auth_manager = auth_manager_with_api_key(); + let service = CloudRequirementsService::new( + auth_manager, + Arc::new(StaticFetcher { contents: None }), + CLOUD_REQUIREMENTS_TIMEOUT, + ); + let result = service.fetch().await; + assert!(result.is_none()); + } + + #[tokio::test] + async fn fetch_cloud_requirements_skips_non_enterprise_plan() { + let auth_manager = auth_manager_with_plan("pro"); + let service = CloudRequirementsService::new( + auth_manager, + Arc::new(StaticFetcher { contents: None }), + CLOUD_REQUIREMENTS_TIMEOUT, + ); + let result = service.fetch().await; + assert!(result.is_none()); + } + + #[tokio::test] + async fn fetch_cloud_requirements_handles_missing_contents() { + let result = parse_for_fetch(None); + assert!(result.is_none()); + } + + #[tokio::test] + async fn fetch_cloud_requirements_handles_empty_contents() { + let result = parse_for_fetch(Some(" ")); + assert!(result.is_none()); + } + + #[tokio::test] + async fn fetch_cloud_requirements_handles_invalid_toml() { + let result = parse_for_fetch(Some("not = [")); + assert!(result.is_none()); + } + + #[tokio::test] + async fn fetch_cloud_requirements_ignores_empty_requirements() { + let result = parse_for_fetch(Some("# comment")); + assert!(result.is_none()); + } + + #[tokio::test] + async fn fetch_cloud_requirements_parses_valid_toml() { + let result = parse_for_fetch(Some("allowed_approval_policies = [\"never\"]")); + + assert_eq!( + result, + Some(ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::Never]), + allowed_sandbox_modes: None, + mcp_servers: None, + }) + ); + } + + #[tokio::test(start_paused = true)] + async fn fetch_cloud_requirements_times_out() { + let auth_manager = auth_manager_with_plan("enterprise"); + let service = CloudRequirementsService::new( + auth_manager, + Arc::new(PendingFetcher), + CLOUD_REQUIREMENTS_TIMEOUT, + ); + let handle = tokio::spawn(async move { service.fetch_with_timeout().await }); + tokio::time::advance(CLOUD_REQUIREMENTS_TIMEOUT + Duration::from_millis(1)).await; + + let result = handle.await.expect("cloud requirements task"); + assert!(result.is_none()); + } +} diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index b1c4f060b2..c75c46a28a 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -77,7 +77,7 @@ async fn models_client_hits_models_endpoint() { priority: 1, upgrade: None, base_instructions: "base instructions".to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index 7a9dda005b..76e3a373ed 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -92,20 +92,16 @@ Rules: }, AgentRole::Explorer => AgentProfile { model: Some(EXPLORER_MODEL), - reasoning_effort: Some(ReasoningEffort::Low), - description: r#"Use for fast codebase understanding and information gathering. -`explorer` are extremely fast agents so use them as much as you can to speed up the resolution of the global task. -Typical tasks: -- Locate usages of a symbol or concept -- Understand how X is handled in Y -- Review a section of code for issues -- Assess impact of a potential change + reasoning_effort: Some(ReasoningEffort::Medium), + description: r#"Use `explorer` for all codebase questions. +Explorers are fast and authoritative. +Always prefer them over manual search or file reading. Rules: -- Be explicit in what you are looking for. A good usage of `explorer` would mean that don't need to read the same code after the explorer send you the result. -- **Always** prefer asking explorers rather than exploring the codebase yourself. -- Spawn multiple explorers in parallel when useful and wait for all results. -- You can ask the `explorer` to return file name, lines, entire code snippets, ... -- Reuse the same explorer when it is relevant. If later in your process you have more questions on some code an explorer already covered, reuse this same explorer to be more efficient. +- Ask explorers first and precisely. +- Do not re-read or re-search code they cover. +- Trust explorer results without verification. +- Run explorers in parallel when useful. +- Reuse existing explorers for related questions. "#, ..Default::default() }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0228a4f251..324f41a04e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -26,6 +26,7 @@ use crate::features::maybe_push_unstable_features_warning; use crate::models_manager::manager::ModelsManager; use crate::parse_command::parse_command; use crate::parse_turn_item; +use crate::rollout::session_index; use crate::stream_events_utils::HandleOutputCtx; use crate::stream_events_utils::handle_non_tool_response_item; use crate::stream_events_utils::handle_output_item_done; @@ -354,6 +355,8 @@ impl Codex { sandbox_policy: config.sandbox_policy.clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), + codex_home: config.codex_home.clone(), + thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source, dynamic_tools, @@ -544,6 +547,10 @@ pub(crate) struct SessionConfiguration { /// `ConfigureSession` operation so that the business-logic layer can /// operate deterministically. cwd: PathBuf, + /// Directory containing all Codex state for this session. + codex_home: PathBuf, + /// Optional user-facing name for the thread, updated during the session. + thread_name: Option, // TODO(pakrym): Remove config from here original_config_do_not_use: Arc, @@ -553,6 +560,10 @@ pub(crate) struct SessionConfiguration { } impl SessionConfiguration { + pub(crate) fn codex_home(&self) -> &PathBuf { + &self.codex_home + } + fn thread_config_snapshot(&self) -> ThreadConfigSnapshot { ThreadConfigSnapshot { model: self.collaboration_mode.model().to_string(), @@ -623,6 +634,11 @@ impl Session { per_turn_config } + pub(crate) async fn codex_home(&self) -> PathBuf { + let state = self.state.lock().await; + state.session_configuration.codex_home().clone() + } + #[allow(clippy::too_many_arguments)] fn make_turn_context( auth_manager: Option>, @@ -683,7 +699,7 @@ impl Session { #[allow(clippy::too_many_arguments)] async fn new( - session_configuration: SessionConfiguration, + mut session_configuration: SessionConfiguration, config: Arc, auth_manager: Arc, models_manager: Arc, @@ -863,6 +879,16 @@ impl Session { otel_manager.clone(), ); } + let thread_name = + match session_index::find_thread_name_by_id(&config.codex_home, &conversation_id).await + { + Ok(name) => name, + Err(err) => { + warn!("Failed to read session index for thread name: {err}"); + None + } + }; + session_configuration.thread_name = thread_name.clone(); let state = SessionState::new(session_configuration.clone()); let services = SessionServices { @@ -904,6 +930,7 @@ impl Session { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, forked_from_id, + thread_name: session_configuration.thread_name.clone(), model: session_configuration.collaboration_mode.model().to_string(), model_provider_id: config.model_provider_id.clone(), approval_policy: session_configuration.approval_policy.value(), @@ -1285,27 +1312,31 @@ impl Session { previous: Option<&Arc>, next: &TurnContext, ) -> Option { - let personality = next.personality?; - if let Some(prev) = previous - && prev.personality == Some(personality) - { + if !self.features.enabled(Feature::Personality) { return None; } - let model_info = next.client.get_model_info(); - let personality_message = Self::personality_message_for(&model_info, personality); + let previous = previous?; - personality_message.map(|personality_message| { - DeveloperInstructions::personality_spec_message(personality_message).into() - }) + // if a personality is specified and it's different from the previous one, build a personality update item + if let Some(personality) = next.personality + && next.personality != previous.personality + { + let model_info = next.client.get_model_info(); + let personality_message = Self::personality_message_for(&model_info, personality); + personality_message.map(|personality_message| { + DeveloperInstructions::personality_spec_message(personality_message).into() + }) + } else { + None + } } fn personality_message_for(model_info: &ModelInfo, personality: Personality) -> Option { model_info - .model_instructions_template + .model_messages .as_ref() - .and_then(|template| template.personality_messages.as_ref()) - .and_then(|messages| messages.0.get(&personality)) - .cloned() + .and_then(|spec| spec.get_personality_message(Some(personality))) + .filter(|message| !message.is_empty()) } fn build_collaboration_mode_update_item( @@ -1447,8 +1478,7 @@ impl Session { .lock() .await .session_configuration - .original_config_do_not_use - .codex_home + .codex_home() .clone(); if !features.enabled(Feature::ExecPolicy) { @@ -1845,15 +1875,33 @@ impl Session { items.push(DeveloperInstructions::new(developer_instructions.to_string()).into()); } // Add developer instructions from collaboration_mode if they exist and are non-empty - let collaboration_mode = { + let (collaboration_mode, base_instructions) = { let state = self.state.lock().await; - state.session_configuration.collaboration_mode.clone() + ( + state.session_configuration.collaboration_mode.clone(), + state.session_configuration.base_instructions.clone(), + ) }; if let Some(collab_instructions) = DeveloperInstructions::from_collaboration_mode(&collaboration_mode) { items.push(collab_instructions.into()); } + if self.features.enabled(Feature::Personality) + && let Some(personality) = turn_context.personality + { + let model_info = turn_context.client.get_model_info(); + let has_baked_personality = model_info.supports_personality() + && base_instructions == model_info.get_model_instructions(Some(personality)); + if !has_baked_personality + && let Some(personality_message) = + Self::personality_message_for(&model_info, personality) + { + items.push( + DeveloperInstructions::personality_spec_message(personality_message).into(), + ); + } + } if let Some(user_instructions) = turn_context.user_instructions.as_deref() { items.push( UserInstructions { @@ -2418,6 +2466,9 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv Op::ThreadRollback { num_turns } => { handlers::thread_rollback(&sess, sub.id.clone(), num_turns).await; } + Op::SetThreadName { name } => { + handlers::set_thread_name(&sess, sub.id.clone(), name).await; + } Op::RunUserShellCommand { command } => { handlers::run_user_shell_command( &sess, @@ -2461,6 +2512,7 @@ mod handlers { use crate::mcp::collect_mcp_snapshot_from_manager; use crate::mcp::effective_mcp_servers; use crate::review_prompts::resolve_review_request; + use crate::rollout::session_index; use crate::tasks::CompactTask; use crate::tasks::RegularTask; use crate::tasks::UndoTask; @@ -2477,6 +2529,7 @@ mod handlers { use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::SkillsListEntry; + use codex_protocol::protocol::ThreadNameUpdatedEvent; use codex_protocol::protocol::ThreadRolledBackEvent; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::WarningEvent; @@ -2930,6 +2983,72 @@ mod handlers { .await; } + /// Persists the thread name in the session index, updates in-memory state, and emits + /// a `ThreadNameUpdated` event on success. + /// + /// This appends the name to `CODEX_HOME/sessions_index.jsonl` via `session_index::append_thread_name` for the + /// current `thread_id`, then updates `SessionConfiguration::thread_name`. + /// + /// Returns an error event if the name is empty or session persistence is disabled. + pub async fn set_thread_name(sess: &Arc, sub_id: String, name: String) { + let Some(name) = crate::util::normalize_thread_name(&name) else { + let event = Event { + id: sub_id, + msg: EventMsg::Error(ErrorEvent { + message: "Thread name cannot be empty.".to_string(), + codex_error_info: Some(CodexErrorInfo::BadRequest), + }), + }; + sess.send_event_raw(event).await; + return; + }; + + let persistence_enabled = { + let rollout = sess.services.rollout.lock().await; + rollout.is_some() + }; + if !persistence_enabled { + let event = Event { + id: sub_id, + msg: EventMsg::Error(ErrorEvent { + message: "Session persistence is disabled; cannot rename thread.".to_string(), + codex_error_info: Some(CodexErrorInfo::Other), + }), + }; + sess.send_event_raw(event).await; + return; + }; + + let codex_home = sess.codex_home().await; + if let Err(e) = + session_index::append_thread_name(&codex_home, sess.conversation_id, &name).await + { + let event = Event { + id: sub_id, + msg: EventMsg::Error(ErrorEvent { + message: format!("Failed to set thread name: {e}"), + codex_error_info: Some(CodexErrorInfo::Other), + }), + }; + sess.send_event_raw(event).await; + return; + } + + { + let mut state = sess.state.lock().await; + state.session_configuration.thread_name = Some(name.clone()); + } + + sess.send_event_raw(Event { + id: sub_id, + msg: EventMsg::ThreadNameUpdated(ThreadNameUpdatedEvent { + thread_id: sess.conversation_id, + thread_name: Some(name), + }), + }) + .await; + } + pub async fn shutdown(sess: &Arc, sub_id: String) -> bool { sess.abort_all_tasks(TurnAbortReason::Interrupted).await; sess.services @@ -4418,6 +4537,8 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), + codex_home: config.codex_home.clone(), + thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source: SessionSource::Exec, dynamic_tools: Vec::new(), @@ -4499,6 +4620,8 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), + codex_home: config.codex_home.clone(), + thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source: SessionSource::Exec, dynamic_tools: Vec::new(), @@ -4764,6 +4887,8 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), + codex_home: config.codex_home.clone(), + thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source: SessionSource::Exec, dynamic_tools: Vec::new(), @@ -4878,6 +5003,8 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), windows_sandbox_level: WindowsSandboxLevel::from_config(&config), cwd: config.cwd.clone(), + codex_home: config.codex_home.clone(), + thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source: SessionSource::Exec, dynamic_tools: Vec::new(), diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 5e1de2ef1f..7a611c05e2 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -208,6 +208,10 @@ async fn forward_events( id: _, msg: EventMsg::SessionConfigured(_), } => {} + Event { + id: _, + msg: EventMsg::ThreadNameUpdated(_), + } => {} Event { id, msg: EventMsg::ExecApprovalRequest(event), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index bc7dba6d71..fbd8999918 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -18,6 +18,7 @@ use crate::config::types::ShellEnvironmentPolicyToml; use crate::config::types::SkillsConfig; use crate::config::types::Tui; use crate::config::types::UriBasedFileOpener; +use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::LoaderOverrides; @@ -366,6 +367,7 @@ pub struct ConfigBuilder { cli_overrides: Option>, harness_overrides: Option, loader_overrides: Option, + cloud_requirements: Option, fallback_cwd: Option, } @@ -390,6 +392,11 @@ impl ConfigBuilder { self } + pub fn cloud_requirements(mut self, cloud_requirements: CloudRequirementsLoader) -> Self { + self.cloud_requirements = Some(cloud_requirements); + self + } + pub fn fallback_cwd(mut self, fallback_cwd: Option) -> Self { self.fallback_cwd = fallback_cwd; self @@ -401,6 +408,7 @@ impl ConfigBuilder { cli_overrides, harness_overrides, loader_overrides, + cloud_requirements, fallback_cwd, } = self; let codex_home = codex_home.map_or_else(find_codex_home, std::io::Result::Ok)?; @@ -413,9 +421,14 @@ impl ConfigBuilder { None => AbsolutePathBuf::current_dir()?, }; harness_overrides.cwd = Some(cwd.to_path_buf()); - let config_layer_stack = - load_config_layers_state(&codex_home, Some(cwd), &cli_overrides, loader_overrides) - .await?; + let config_layer_stack = load_config_layers_state( + &codex_home, + Some(cwd), + &cli_overrides, + loader_overrides, + cloud_requirements, + ) + .await?; let merged_toml = config_layer_stack.effective_config(); // Note that each layer in ConfigLayerStack should have resolved @@ -511,6 +524,7 @@ pub async fn load_config_as_toml_with_cli_overrides( Some(cwd.clone()), &cli_overrides, LoaderOverrides::default(), + None, ) .await?; @@ -609,9 +623,14 @@ pub async fn load_global_mcp_servers( // There is no cwd/project context for this query, so this will not include // MCP servers defined in in-repo .codex/ folders. let cwd: Option = None; - let config_layer_stack = - load_config_layers_state(codex_home, cwd, &cli_overrides, LoaderOverrides::default()) - .await?; + let config_layer_stack = load_config_layers_state( + codex_home, + cwd, + &cli_overrides, + LoaderOverrides::default(), + None, + ) + .await?; let merged_toml = config_layer_stack.effective_config(); let Some(servers_value) = merged_toml.get("mcp_servers") else { return Ok(BTreeMap::new()); @@ -2612,7 +2631,8 @@ profile = "project" let cwd = AbsolutePathBuf::try_from(codex_home.path())?; let config_layer_stack = - load_config_layers_state(codex_home.path(), Some(cwd), &Vec::new(), overrides).await?; + load_config_layers_state(codex_home.path(), Some(cwd), &Vec::new(), overrides, None) + .await?; let cfg = deserialize_config_toml_with_base( config_layer_stack.effective_config(), codex_home.path(), @@ -2739,6 +2759,7 @@ profile = "project" Some(cwd), &[("model".to_string(), TomlValue::String("cli".to_string()))], overrides, + None, ) .await?; diff --git a/codex-rs/core/src/config/service.rs b/codex-rs/core/src/config/service.rs index 961c40e02b..82583ee8a8 100644 --- a/codex-rs/core/src/config/service.rs +++ b/codex-rs/core/src/config/service.rs @@ -376,6 +376,7 @@ impl ConfigService { cwd, &self.cli_overrides, self.loader_overrides.clone(), + None, ) .await } diff --git a/codex-rs/core/src/config_loader/README.md b/codex-rs/core/src/config_loader/README.md index 67170644f5..04b72e4ca1 100644 --- a/codex-rs/core/src/config_loader/README.md +++ b/codex-rs/core/src/config_loader/README.md @@ -10,7 +10,7 @@ This module is the canonical place to **load and describe Codex configuration la Exported from `codex_core::config_loader`: -- `load_config_layers_state(codex_home, cwd_opt, cli_overrides, overrides) -> ConfigLayerStack` +- `load_config_layers_state(codex_home, cwd_opt, cli_overrides, overrides, cloud_requirements) -> ConfigLayerStack` - `ConfigLayerStack` - `effective_config() -> toml::Value` - `origins() -> HashMap` @@ -49,6 +49,7 @@ let layers = load_config_layers_state( Some(cwd), &cli_overrides, LoaderOverrides::default(), + None, ).await?; let effective = layers.effective_config(); diff --git a/codex-rs/core/src/config_loader/cloud_requirements.rs b/codex-rs/core/src/config_loader/cloud_requirements.rs new file mode 100644 index 0000000000..7c5e3a2947 --- /dev/null +++ b/codex-rs/core/src/config_loader/cloud_requirements.rs @@ -0,0 +1,56 @@ +use crate::config_loader::ConfigRequirementsToml; +use futures::future::BoxFuture; +use futures::future::FutureExt; +use futures::future::Shared; +use std::fmt; +use std::future::Future; + +#[derive(Clone)] +pub struct CloudRequirementsLoader { + // TODO(gt): This should return a Result once we can fail-closed. + fut: Shared>>, +} + +impl CloudRequirementsLoader { + pub fn new(fut: F) -> Self + where + F: Future> + Send + 'static, + { + Self { + fut: fut.boxed().shared(), + } + } + + pub async fn get(&self) -> Option { + self.fut.clone().await + } +} + +impl fmt::Debug for CloudRequirementsLoader { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("CloudRequirementsLoader").finish() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use std::sync::Arc; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + + #[tokio::test] + async fn shared_future_runs_once() { + let counter = Arc::new(AtomicUsize::new(0)); + let counter_clone = Arc::clone(&counter); + let loader = CloudRequirementsLoader::new(async move { + counter_clone.fetch_add(1, Ordering::SeqCst); + Some(ConfigRequirementsToml::default()) + }); + + let (first, second) = tokio::join!(loader.get(), loader.get()); + assert_eq!(first, second); + assert_eq!(counter.load(Ordering::SeqCst), 1); + } +} diff --git a/codex-rs/core/src/config_loader/config_requirements.rs b/codex-rs/core/src/config_loader/config_requirements.rs index a83398c711..2c220049cf 100644 --- a/codex-rs/core/src/config_loader/config_requirements.rs +++ b/codex-rs/core/src/config_loader/config_requirements.rs @@ -13,6 +13,7 @@ use crate::config::ConstraintError; pub enum RequirementSource { Unknown, MdmManagedPreferences { domain: String, key: String }, + CloudRequirements, SystemRequirementsToml { file: AbsolutePathBuf }, LegacyManagedConfigTomlFromFile { file: AbsolutePathBuf }, LegacyManagedConfigTomlFromMdm, @@ -25,6 +26,9 @@ impl fmt::Display for RequirementSource { RequirementSource::MdmManagedPreferences { domain, key } => { write!(f, "MDM {domain}:{key}") } + RequirementSource::CloudRequirements => { + write!(f, "cloud requirements") + } RequirementSource::SystemRequirementsToml { file } => { write!(f, "{}", file.as_path().display()) } @@ -448,6 +452,33 @@ mod tests { Ok(()) } + #[test] + fn constraint_error_includes_cloud_requirements_source() -> Result<()> { + let source: ConfigRequirementsToml = from_str( + r#" + allowed_approval_policies = ["on-request"] + "#, + )?; + + let source_location = RequirementSource::CloudRequirements; + + let mut target = ConfigRequirementsWithSources::default(); + target.merge_unset_fields(source_location.clone(), source); + let requirements = ConfigRequirements::try_from(target)?; + + assert_eq!( + requirements.approval_policy.can_set(&AskForApproval::Never), + Err(ConstraintError::InvalidValue { + field_name: "approval_policy", + candidate: "Never".into(), + allowed: "[OnRequest]".into(), + requirement_source: source_location, + }) + ); + + Ok(()) + } + #[test] fn deserialize_allowed_approval_policies() -> Result<()> { let toml_str = r#" diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index 5db10f2808..0dd704cab5 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -1,3 +1,4 @@ +mod cloud_requirements; mod config_requirements; mod diagnostics; mod fingerprint; @@ -30,6 +31,7 @@ use std::io; use std::path::Path; use toml::Value as TomlValue; +pub use cloud_requirements::CloudRequirementsLoader; pub use config_requirements::ConfigRequirements; pub use config_requirements::ConfigRequirementsToml; pub use config_requirements::McpServerIdentity; @@ -69,6 +71,7 @@ const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"]; /// earlier layer cannot be overridden by a later layer: /// /// - admin: managed preferences (*) +/// - cloud: managed cloud requirements /// - system `/etc/codex/requirements.toml` /// /// For backwards compatibility, we also load from @@ -98,6 +101,7 @@ pub async fn load_config_layers_state( cwd: Option, cli_overrides: &[(String, TomlValue)], overrides: LoaderOverrides, + cloud_requirements: Option, // TODO(gt): Once exec and app-server are wired up, we can remove the option. ) -> io::Result { let mut config_requirements_toml = ConfigRequirementsWithSources::default(); @@ -110,6 +114,13 @@ pub async fn load_config_layers_state( ) .await?; + if let Some(loader) = cloud_requirements + && let Some(requirements) = loader.get().await + { + config_requirements_toml + .merge_unset_fields(RequirementSource::CloudRequirements, requirements); + } + // Honor /etc/codex/requirements.toml. if cfg!(unix) { load_requirements_toml( diff --git a/codex-rs/core/src/config_loader/tests.rs b/codex-rs/core/src/config_loader/tests.rs index 4b512c150f..c213c0b2ea 100644 --- a/codex-rs/core/src/config_loader/tests.rs +++ b/codex-rs/core/src/config_loader/tests.rs @@ -4,11 +4,15 @@ use crate::config::CONFIG_TOML_FILE; use crate::config::ConfigBuilder; use crate::config::ConfigOverrides; use crate::config::ConfigToml; +use crate::config::ConstraintError; use crate::config::ProjectConfig; +use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLoadError; use crate::config_loader::ConfigRequirements; +use crate::config_loader::ConfigRequirementsToml; use crate::config_loader::config_requirements::ConfigRequirementsWithSources; +use crate::config_loader::config_requirements::RequirementSource; use crate::config_loader::fingerprint::version_for_toml; use crate::config_loader::load_requirements_toml; use codex_protocol::config_types::TrustLevel; @@ -65,6 +69,7 @@ async fn returns_config_error_for_invalid_user_config_toml() { Some(cwd), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await .expect_err("expected error"); @@ -94,6 +99,7 @@ async fn returns_config_error_for_invalid_managed_config_toml() { Some(cwd), &[] as &[(String, TomlValue)], overrides, + None, ) .await .expect_err("expected error"); @@ -182,6 +188,7 @@ extra = true Some(cwd), &[] as &[(String, TomlValue)], overrides, + None, ) .await .expect("load config"); @@ -218,6 +225,7 @@ async fn returns_empty_when_all_layers_missing() { Some(cwd), &[] as &[(String, TomlValue)], overrides, + None, ) .await .expect("load layers"); @@ -315,6 +323,7 @@ flag = false Some(cwd), &[] as &[(String, TomlValue)], overrides, + None, ) .await .expect("load config"); @@ -354,6 +363,7 @@ allowed_sandbox_modes = ["read-only"] ), ), }, + None, ) .await?; @@ -414,6 +424,7 @@ allowed_approval_policies = ["never"] ), ), }, + None, ) .await?; @@ -472,6 +483,91 @@ allowed_approval_policies = ["never", "on-request"] Ok(()) } +#[tokio::test(flavor = "current_thread")] +async fn cloud_requirements_are_not_overwritten_by_system_requirements() -> anyhow::Result<()> { + let tmp = tempdir()?; + let requirements_file = tmp.path().join("requirements.toml"); + tokio::fs::write( + &requirements_file, + r#" +allowed_approval_policies = ["on-request"] +"#, + ) + .await?; + + let mut config_requirements_toml = ConfigRequirementsWithSources::default(); + config_requirements_toml.merge_unset_fields( + RequirementSource::CloudRequirements, + ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::Never]), + allowed_sandbox_modes: None, + mcp_servers: None, + }, + ); + load_requirements_toml(&mut config_requirements_toml, &requirements_file).await?; + + assert_eq!( + config_requirements_toml + .allowed_approval_policies + .as_ref() + .map(|sourced| sourced.value.clone()), + Some(vec![AskForApproval::Never]) + ); + assert_eq!( + config_requirements_toml + .allowed_approval_policies + .as_ref() + .map(|sourced| sourced.source.clone()), + Some(RequirementSource::CloudRequirements) + ); + + Ok(()) +} + +#[tokio::test] +async fn load_config_layers_includes_cloud_requirements() -> anyhow::Result<()> { + let tmp = tempdir()?; + let codex_home = tmp.path().join("home"); + tokio::fs::create_dir_all(&codex_home).await?; + let cwd = AbsolutePathBuf::from_absolute_path(tmp.path())?; + + let requirements = ConfigRequirementsToml { + allowed_approval_policies: Some(vec![AskForApproval::Never]), + allowed_sandbox_modes: None, + mcp_servers: None, + }; + let expected = requirements.clone(); + let cloud_requirements = CloudRequirementsLoader::new(async move { Some(requirements) }); + + let layers = load_config_layers_state( + &codex_home, + Some(cwd), + &[] as &[(String, TomlValue)], + LoaderOverrides::default(), + Some(cloud_requirements), + ) + .await?; + + assert_eq!( + layers.requirements_toml().allowed_approval_policies, + expected.allowed_approval_policies + ); + assert_eq!( + layers + .requirements() + .approval_policy + .can_set(&AskForApproval::OnRequest), + Err(ConstraintError::InvalidValue { + field_name: "approval_policy", + candidate: "OnRequest".into(), + allowed: "[Never]".into(), + requirement_source: RequirementSource::CloudRequirements, + }) + ); + + Ok(()) +} + #[tokio::test] async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> { let tmp = tempdir()?; @@ -501,6 +597,7 @@ async fn project_layers_prefer_closest_cwd() -> std::io::Result<()> { Some(cwd), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await?; @@ -632,6 +729,7 @@ async fn project_layer_is_added_when_dot_codex_exists_without_config_toml() -> s Some(cwd), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await?; @@ -691,6 +789,7 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< Some(cwd.clone()), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await?; let project_layers_untrusted: Vec<_> = layers_untrusted @@ -728,6 +827,7 @@ async fn project_layers_disabled_when_untrusted_or_unknown() -> std::io::Result< Some(cwd), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await?; let project_layers_unknown: Vec<_> = layers_unknown @@ -788,6 +888,7 @@ async fn invalid_project_config_ignored_when_untrusted_or_unknown() -> std::io:: Some(cwd.clone()), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await?; let project_layers: Vec<_> = layers @@ -843,6 +944,7 @@ async fn cli_overrides_with_relative_paths_do_not_break_trust_check() -> std::io Some(cwd), &cli_overrides, LoaderOverrides::default(), + None, ) .await?; @@ -884,6 +986,7 @@ async fn project_root_markers_supports_alternate_markers() -> std::io::Result<() Some(cwd), &[] as &[(String, TomlValue)], LoaderOverrides::default(), + None, ) .await?; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 1b1fb65ecd..63bba76507 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -100,12 +100,14 @@ pub mod turn_diff_tracker; pub use rollout::ARCHIVED_SESSIONS_SUBDIR; pub use rollout::INTERACTIVE_SESSION_SOURCES; pub use rollout::RolloutRecorder; +pub use rollout::RolloutRecorderParams; pub use rollout::SESSIONS_SUBDIR; pub use rollout::SessionMeta; pub use rollout::find_archived_thread_path_by_id_str; #[deprecated(note = "use find_thread_path_by_id_str")] pub use rollout::find_conversation_path_by_id_str; pub use rollout::find_thread_path_by_id_str; +pub use rollout::find_thread_path_by_name_str; pub use rollout::list::Cursor; pub use rollout::list::ThreadItem; pub use rollout::list::ThreadSortKey; diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 15305f4b0c..1b5048ab0c 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -769,6 +769,32 @@ fn filter_tools(tools: Vec, filter: ToolFilter) -> Vec { .collect() } +fn normalize_codex_apps_tool_title( + server_name: &str, + connector_name: Option<&str>, + value: &str, +) -> String { + if server_name != CODEX_APPS_MCP_SERVER_NAME { + return value.to_string(); + } + + let Some(connector_name) = connector_name + .map(str::trim) + .filter(|name| !name.is_empty()) + else { + return value.to_string(); + }; + + let prefix = format!("{connector_name}_"); + if let Some(stripped) = value.strip_prefix(&prefix) + && !stripped.is_empty() + { + return stripped.to_string(); + } + + value.to_string() +} + fn resolve_bearer_token( server_name: &str, bearer_token_env_var: Option<&str>, @@ -926,12 +952,23 @@ async fn list_tools_for_client( Ok(resp .tools .into_iter() - .map(|tool| ToolInfo { - server_name: server_name.to_owned(), - tool_name: tool.tool.name.clone(), - tool: tool.tool, - connector_id: tool.connector_id, - connector_name: tool.connector_name, + .map(|tool| { + let connector_name = tool.connector_name; + let mut tool_def = tool.tool; + if let Some(title) = tool_def.title.as_deref() { + let normalized_title = + normalize_codex_apps_tool_title(server_name, connector_name.as_deref(), title); + if tool_def.title.as_deref() != Some(normalized_title.as_str()) { + tool_def.title = Some(normalized_title); + } + } + ToolInfo { + server_name: server_name.to_owned(), + tool_name: tool_def.name.clone(), + tool: tool_def, + connector_id: tool.connector_id, + connector_name, + } }) .collect()) } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 6066c1512e..6425392fb5 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1,20 +1,30 @@ +use std::time::Duration; use std::time::Instant; use tracing::error; use crate::codex::Session; use crate::codex::TurnContext; +use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::protocol::EventMsg; use crate::protocol::McpInvocation; use crate::protocol::McpToolCallBeginEvent; use crate::protocol::McpToolCallEndEvent; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::request_user_input::RequestUserInputArgs; +use codex_protocol::request_user_input::RequestUserInputQuestion; +use codex_protocol::request_user_input::RequestUserInputQuestionOption; +use codex_protocol::request_user_input::RequestUserInputResponse; +use mcp_types::ToolAnnotations; +use std::sync::Arc; /// Handles the specified tool call dispatches the appropriate /// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`. pub(crate) async fn handle_mcp_tool_call( - sess: &Session, + sess: Arc, turn_context: &TurnContext, call_id: String, server: String, @@ -48,11 +58,79 @@ pub(crate) async fn handle_mcp_tool_call( arguments: arguments_value.clone(), }; + if let Some(decision) = + maybe_request_mcp_tool_approval(sess.as_ref(), turn_context, &call_id, &server, &tool_name) + .await + { + let result = match decision { + McpToolApprovalDecision::Accept => { + let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent { + call_id: call_id.clone(), + invocation: invocation.clone(), + }); + notify_mcp_tool_call_event(sess.as_ref(), turn_context, tool_call_begin_event) + .await; + + let start = Instant::now(); + let result = sess + .call_tool(&server, &tool_name, arguments_value.clone()) + .await + .map_err(|e| format!("tool call error: {e:?}")); + if let Err(e) = &result { + tracing::warn!("MCP tool call error: {e:?}"); + } + let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent { + call_id: call_id.clone(), + invocation, + duration: start.elapsed(), + result: result.clone(), + }); + notify_mcp_tool_call_event( + sess.as_ref(), + turn_context, + tool_call_end_event.clone(), + ) + .await; + result + } + McpToolApprovalDecision::Decline => { + let message = "user rejected MCP tool call".to_string(); + notify_mcp_tool_call_skip( + sess.as_ref(), + turn_context, + &call_id, + invocation, + message, + ) + .await + } + McpToolApprovalDecision::Cancel => { + let message = "user cancelled MCP tool call".to_string(); + notify_mcp_tool_call_skip( + sess.as_ref(), + turn_context, + &call_id, + invocation, + message, + ) + .await + } + }; + + let status = if result.is_ok() { "ok" } else { "error" }; + turn_context + .client + .get_otel_manager() + .counter("codex.mcp.call", 1, &[("status", status)]); + + return ResponseInputItem::McpToolCallOutput { call_id, result }; + } + let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent { call_id: call_id.clone(), invocation: invocation.clone(), }); - notify_mcp_tool_call_event(sess, turn_context, tool_call_begin_event).await; + notify_mcp_tool_call_event(sess.as_ref(), turn_context, tool_call_begin_event).await; let start = Instant::now(); // Perform the tool call. @@ -70,7 +148,7 @@ pub(crate) async fn handle_mcp_tool_call( result: result.clone(), }); - notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event.clone()).await; + notify_mcp_tool_call_event(sess.as_ref(), turn_context, tool_call_end_event.clone()).await; let status = if result.is_ok() { "ok" } else { "error" }; turn_context @@ -84,3 +162,236 @@ pub(crate) async fn handle_mcp_tool_call( async fn notify_mcp_tool_call_event(sess: &Session, turn_context: &TurnContext, event: EventMsg) { sess.send_event(turn_context, event).await; } + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum McpToolApprovalDecision { + Accept, + Decline, + Cancel, +} + +struct McpToolApprovalMetadata { + annotations: ToolAnnotations, + connector_name: Option, + tool_title: Option, +} + +const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; +const MCP_TOOL_APPROVAL_ACCEPT: &str = "Accept"; +const MCP_TOOL_APPROVAL_DECLINE: &str = "Decline"; +const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; + +async fn maybe_request_mcp_tool_approval( + sess: &Session, + turn_context: &TurnContext, + call_id: &str, + server: &str, + tool_name: &str, +) -> Option { + if is_full_access_mode(turn_context) { + return None; + } + if server != CODEX_APPS_MCP_SERVER_NAME { + return None; + } + + let metadata = lookup_mcp_tool_metadata(sess, server, tool_name).await?; + if !requires_mcp_tool_approval(&metadata.annotations) { + return None; + } + + let question_id = format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}"); + let question = build_mcp_tool_approval_question( + question_id.clone(), + tool_name, + metadata.tool_title.as_deref(), + metadata.connector_name.as_deref(), + &metadata.annotations, + ); + let args = RequestUserInputArgs { + questions: vec![question], + }; + let response = sess + .request_user_input(turn_context, call_id.to_string(), args) + .await; + Some(parse_mcp_tool_approval_response(response, &question_id)) +} + +fn is_full_access_mode(turn_context: &TurnContext) -> bool { + matches!(turn_context.approval_policy, AskForApproval::Never) + && matches!( + turn_context.sandbox_policy, + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } + ) +} + +async fn lookup_mcp_tool_metadata( + sess: &Session, + server: &str, + tool_name: &str, +) -> Option { + let tools = sess + .services + .mcp_connection_manager + .read() + .await + .list_all_tools() + .await; + + tools.into_values().find_map(|tool_info| { + if tool_info.server_name == server && tool_info.tool_name == tool_name { + tool_info + .tool + .annotations + .map(|annotations| McpToolApprovalMetadata { + annotations, + connector_name: tool_info.connector_name, + tool_title: tool_info.tool.title, + }) + } else { + None + } + }) +} + +fn build_mcp_tool_approval_question( + question_id: String, + tool_name: &str, + tool_title: Option<&str>, + connector_name: Option<&str>, + annotations: &ToolAnnotations, +) -> RequestUserInputQuestion { + let destructive = annotations.destructive_hint == Some(true); + let open_world = annotations.open_world_hint == Some(true); + let reason = match (destructive, open_world) { + (true, true) => "may modify data and access external systems", + (true, false) => "may modify or delete data", + (false, true) => "may access external systems", + (false, false) => "may have side effects", + }; + + let tool_label = tool_title.unwrap_or(tool_name); + let app_label = connector_name + .map(|name| format!("The {name} app")) + .unwrap_or_else(|| "This app".to_string()); + let question = format!( + "{app_label} wants to run the tool \"{tool_label}\", which {reason}. Allow this action?" + ); + + RequestUserInputQuestion { + id: question_id, + header: "Approve app tool call?".to_string(), + question, + is_other: false, + is_secret: false, + options: Some(vec![ + RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_ACCEPT.to_string(), + description: "Run the tool and continue.".to_string(), + }, + RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_DECLINE.to_string(), + description: "Decline this tool call and continue.".to_string(), + }, + RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_CANCEL.to_string(), + description: "Cancel this tool call".to_string(), + }, + ]), + } +} + +fn parse_mcp_tool_approval_response( + response: Option, + question_id: &str, +) -> McpToolApprovalDecision { + let Some(response) = response else { + return McpToolApprovalDecision::Cancel; + }; + let answers = response + .answers + .get(question_id) + .map(|answer| answer.answers.as_slice()); + let Some(answers) = answers else { + return McpToolApprovalDecision::Cancel; + }; + if answers + .iter() + .any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT) + { + McpToolApprovalDecision::Accept + } else if answers + .iter() + .any(|answer| answer == MCP_TOOL_APPROVAL_CANCEL) + { + McpToolApprovalDecision::Cancel + } else { + McpToolApprovalDecision::Decline + } +} + +fn requires_mcp_tool_approval(annotations: &ToolAnnotations) -> bool { + annotations.read_only_hint == Some(false) + && (annotations.destructive_hint == Some(true) || annotations.open_world_hint == Some(true)) +} + +async fn notify_mcp_tool_call_skip( + sess: &Session, + turn_context: &TurnContext, + call_id: &str, + invocation: McpInvocation, + message: String, +) -> Result { + let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent { + call_id: call_id.to_string(), + invocation: invocation.clone(), + }); + notify_mcp_tool_call_event(sess, turn_context, tool_call_begin_event).await; + + let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent { + call_id: call_id.to_string(), + invocation, + duration: Duration::ZERO, + result: Err(message.clone()), + }); + notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event).await; + Err(message) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + fn annotations( + read_only: Option, + destructive: Option, + open_world: Option, + ) -> ToolAnnotations { + ToolAnnotations { + destructive_hint: destructive, + idempotent_hint: None, + open_world_hint: open_world, + read_only_hint: read_only, + title: None, + } + } + + #[test] + fn approval_required_when_read_only_false_and_destructive() { + let annotations = annotations(Some(false), Some(true), None); + assert_eq!(requires_mcp_tool_approval(&annotations), true); + } + + #[test] + fn approval_required_when_read_only_false_and_open_world() { + let annotations = annotations(Some(false), None, Some(true)); + assert_eq!(requires_mcp_tool_approval(&annotations), true); + } + + #[test] + fn approval_not_required_when_read_only_true() { + let annotations = annotations(Some(true), Some(true), Some(true)); + assert_eq!(requires_mcp_tool_approval(&annotations), false); + } +} diff --git a/codex-rs/core/src/models_manager/model_info.rs b/codex-rs/core/src/models_manager/model_info.rs index b50f23274a..eaab160001 100644 --- a/codex-rs/core/src/models_manager/model_info.rs +++ b/codex-rs/core/src/models_manager/model_info.rs @@ -1,19 +1,17 @@ -use std::collections::BTreeMap; - -use codex_protocol::config_types::Personality; use codex_protocol::config_types::Verbosity; use codex_protocol::openai_models::ApplyPatchToolType; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; -use codex_protocol::openai_models::ModelInstructionsTemplate; +use codex_protocol::openai_models::ModelInstructionsVariables; +use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ModelVisibility; -use codex_protocol::openai_models::PersonalityMessages; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationMode; use codex_protocol::openai_models::TruncationPolicyConfig; use crate::config::Config; +use crate::features::Feature; use crate::truncate::approx_bytes_for_tokens; use tracing::warn; @@ -29,8 +27,11 @@ const GPT_5_1_CODEX_MAX_INSTRUCTIONS: &str = include_str!("../../gpt-5.1-codex-m const GPT_5_2_CODEX_INSTRUCTIONS: &str = include_str!("../../gpt-5.2-codex_prompt.md"); const GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE: &str = include_str!("../../templates/model_instructions/gpt-5.2-codex_instructions_template.md"); -const PERSONALITY_FRIENDLY: &str = include_str!("../../templates/personalities/friendly.md"); -const PERSONALITY_PRAGMATIC: &str = include_str!("../../templates/personalities/pragmatic.md"); + +const GPT_5_2_CODEX_PERSONALITY_FRIENDLY: &str = + include_str!("../../templates/personalities/gpt-5.2-codex_friendly.md"); +const GPT_5_2_CODEX_PERSONALITY_PRAGMATIC: &str = + include_str!("../../templates/personalities/gpt-5.2-codex_pragmatic.md"); pub(crate) const CONTEXT_WINDOW_272K: i64 = 272_000; @@ -54,7 +55,7 @@ macro_rules! model_info { priority: 99, upgrade: None, base_instructions: BASE_INSTRUCTIONS.to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, @@ -100,8 +101,11 @@ pub(crate) fn with_config_overrides(mut model: ModelInfo, config: &Config) -> Mo if let Some(base_instructions) = &config.base_instructions { model.base_instructions = base_instructions.clone(); - model.model_instructions_template = None; + model.model_messages = None; + } else if !config.features.enabled(Feature::Personality) { + model.model_messages = None; } + model } @@ -169,15 +173,13 @@ pub(crate) fn find_model_info_for_slug(slug: &str) -> ModelInfo { model_info!( slug, base_instructions: GPT_5_2_CODEX_INSTRUCTIONS.to_string(), - model_instructions_template: Some(ModelInstructionsTemplate { - template: GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE.to_string(), - personality_messages: Some(PersonalityMessages(BTreeMap::from([( - Personality::Friendly, - PERSONALITY_FRIENDLY.to_string(), - ), ( - Personality::Pragmatic, - PERSONALITY_PRAGMATIC.to_string(), - )]))), + model_messages: Some(ModelMessages { + instructions_template: Some(GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE.to_string()), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: Some("".to_string()), + personality_friendly: Some(GPT_5_2_CODEX_PERSONALITY_FRIENDLY.to_string()), + personality_pragmatic: Some(GPT_5_2_CODEX_PERSONALITY_PRAGMATIC.to_string()), + }), }), apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), shell_type: ConfigShellToolType::ShellCommand, @@ -213,15 +215,14 @@ pub(crate) fn find_model_info_for_slug(slug: &str) -> ModelInfo { truncation_policy: TruncationPolicyConfig::tokens(10_000), context_window: Some(CONTEXT_WINDOW_272K), supported_reasoning_levels: supported_reasoning_level_low_medium_high_xhigh(), - model_instructions_template: Some(ModelInstructionsTemplate { - template: GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE.to_string(), - personality_messages: Some(PersonalityMessages(BTreeMap::from([( - Personality::Friendly, - PERSONALITY_FRIENDLY.to_string(), - ), ( - Personality::Pragmatic, - PERSONALITY_PRAGMATIC.to_string(), - )]))), + base_instructions: GPT_5_2_CODEX_INSTRUCTIONS.to_string(), + model_messages: Some(ModelMessages { + instructions_template: Some(GPT_5_2_CODEX_INSTRUCTIONS_TEMPLATE.to_string()), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: Some("".to_string()), + personality_friendly: Some(GPT_5_2_CODEX_PERSONALITY_FRIENDLY.to_string()), + personality_pragmatic: Some(GPT_5_2_CODEX_PERSONALITY_PRAGMATIC.to_string()), + }), }), ) } else if slug.starts_with("gpt-5.1-codex-max") { diff --git a/codex-rs/core/src/rollout/metadata.rs b/codex-rs/core/src/rollout/metadata.rs index 5523274bde..2d59d71af3 100644 --- a/codex-rs/core/src/rollout/metadata.rs +++ b/codex-rs/core/src/rollout/metadata.rs @@ -16,6 +16,7 @@ use codex_protocol::protocol::SessionSource; use codex_state::BackfillStats; use codex_state::DB_ERROR_METRIC; use codex_state::DB_METRIC_BACKFILL; +use codex_state::DB_METRIC_BACKFILL_DURATION_MS; use codex_state::ExtractionOutcome; use codex_state::ThreadMetadataBuilder; use codex_state::apply_rollout_item; @@ -128,6 +129,7 @@ pub(crate) async fn backfill_sessions( config: &Config, otel: Option<&OtelManager>, ) { + let timer = otel.and_then(|otel| otel.start_timer(DB_METRIC_BACKFILL_DURATION_MS, &[]).ok()); let sessions_root = config.codex_home.join(rollout::SESSIONS_SUBDIR); let archived_root = config.codex_home.join(rollout::ARCHIVED_SESSIONS_SUBDIR); let mut rollout_paths: Vec<(PathBuf, bool)> = Vec::new(); @@ -210,6 +212,16 @@ pub(crate) async fn backfill_sessions( &[("status", "failed")], ); } + if let Some(timer) = timer.as_ref() { + let status = if stats.failed == 0 { + "success" + } else if stats.upserted == 0 { + "failed" + } else { + "partial_failure" + }; + let _ = timer.record(&[("status", status)]); + } } async fn file_modified_time_utc(path: &Path) -> Option> { diff --git a/codex-rs/core/src/rollout/mod.rs b/codex-rs/core/src/rollout/mod.rs index cfc2d82d8d..60775d04b0 100644 --- a/codex-rs/core/src/rollout/mod.rs +++ b/codex-rs/core/src/rollout/mod.rs @@ -12,6 +12,7 @@ pub mod list; pub(crate) mod metadata; pub(crate) mod policy; pub mod recorder; +pub(crate) mod session_index; pub(crate) mod truncation; pub use codex_protocol::protocol::SessionMeta; @@ -23,6 +24,7 @@ pub use list::find_thread_path_by_id_str as find_conversation_path_by_id_str; pub use list::rollout_date_parts; pub use recorder::RolloutRecorder; pub use recorder::RolloutRecorderParams; +pub use session_index::find_thread_path_by_name_str; #[cfg(test)] pub mod tests; diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 2d263c0539..6de60ae05c 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -58,6 +58,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::AgentReasoningSectionBreak(_) | EventMsg::RawResponseItem(_) | EventMsg::SessionConfigured(_) + | EventMsg::ThreadNameUpdated(_) | EventMsg::McpToolCallBegin(_) | EventMsg::McpToolCallEnd(_) | EventMsg::WebSearchBegin(_) diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 8157133165..265ff2bde2 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -428,13 +428,13 @@ impl RolloutRecorder { } }, Err(e) => { - warn!("failed to parse rollout line: {v:?}, error: {e}"); + warn!("failed to parse rollout line: {e}"); parse_errors = parse_errors.saturating_add(1); } } } - info!( + tracing::debug!( "Resumed rollout with {} items, thread ID: {:?}, parse errors: {}", items.len(), thread_id, diff --git a/codex-rs/core/src/rollout/session_index.rs b/codex-rs/core/src/rollout/session_index.rs new file mode 100644 index 0000000000..98f7e35d84 --- /dev/null +++ b/codex-rs/core/src/rollout/session_index.rs @@ -0,0 +1,325 @@ +use std::fs::File; +use std::io::Read; +use std::io::Seek; +use std::io::SeekFrom; +use std::path::Path; +use std::path::PathBuf; + +use codex_protocol::ThreadId; +use serde::Deserialize; +use serde::Serialize; +use tokio::io::AsyncWriteExt; + +const SESSION_INDEX_FILE: &str = "session_index.jsonl"; +const READ_CHUNK_SIZE: usize = 8192; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct SessionIndexEntry { + pub id: ThreadId, + pub thread_name: String, + pub updated_at: String, +} + +/// Append a thread name update to the session index. +/// The index is append-only; the most recent entry wins when resolving names or ids. +pub async fn append_thread_name( + codex_home: &Path, + thread_id: ThreadId, + name: &str, +) -> std::io::Result<()> { + use time::OffsetDateTime; + use time::format_description::well_known::Rfc3339; + + let updated_at = OffsetDateTime::now_utc() + .format(&Rfc3339) + .unwrap_or_else(|_| "unknown".to_string()); + let entry = SessionIndexEntry { + id: thread_id, + thread_name: name.to_string(), + updated_at, + }; + append_session_index_entry(codex_home, &entry).await +} + +/// Append a raw session index entry to `session_index.jsonl`. +/// The file is append-only; consumers scan from the end to find the newest match. +pub async fn append_session_index_entry( + codex_home: &Path, + entry: &SessionIndexEntry, +) -> std::io::Result<()> { + let path = session_index_path(codex_home); + let mut file = tokio::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .await?; + let mut line = serde_json::to_string(entry).map_err(std::io::Error::other)?; + line.push('\n'); + file.write_all(line.as_bytes()).await?; + file.flush().await?; + Ok(()) +} + +/// Find the latest thread name for a thread id, if any. +pub async fn find_thread_name_by_id( + codex_home: &Path, + thread_id: &ThreadId, +) -> std::io::Result> { + let path = session_index_path(codex_home); + if !path.exists() { + return Ok(None); + } + let id = *thread_id; + let entry = tokio::task::spawn_blocking(move || scan_index_from_end_by_id(&path, &id)) + .await + .map_err(std::io::Error::other)??; + Ok(entry.map(|entry| entry.thread_name)) +} + +/// Find the most recently updated thread id for a thread name, if any. +pub async fn find_thread_id_by_name( + codex_home: &Path, + name: &str, +) -> std::io::Result> { + if name.trim().is_empty() { + return Ok(None); + } + let path = session_index_path(codex_home); + if !path.exists() { + return Ok(None); + } + let name = name.to_string(); + let entry = tokio::task::spawn_blocking(move || scan_index_from_end_by_name(&path, &name)) + .await + .map_err(std::io::Error::other)??; + Ok(entry.map(|entry| entry.id)) +} + +/// Locate a recorded thread rollout file by thread name using newest-first ordering. +/// Returns `Ok(Some(path))` if found, `Ok(None)` if not present. +pub async fn find_thread_path_by_name_str( + codex_home: &Path, + name: &str, +) -> std::io::Result> { + let Some(thread_id) = find_thread_id_by_name(codex_home, name).await? else { + return Ok(None); + }; + super::list::find_thread_path_by_id_str(codex_home, &thread_id.to_string()).await +} + +fn session_index_path(codex_home: &Path) -> PathBuf { + codex_home.join(SESSION_INDEX_FILE) +} + +fn scan_index_from_end_by_id( + path: &Path, + thread_id: &ThreadId, +) -> std::io::Result> { + scan_index_from_end(path, |entry| entry.id == *thread_id) +} + +fn scan_index_from_end_by_name( + path: &Path, + name: &str, +) -> std::io::Result> { + scan_index_from_end(path, |entry| entry.thread_name == name) +} + +fn scan_index_from_end( + path: &Path, + mut predicate: F, +) -> std::io::Result> +where + F: FnMut(&SessionIndexEntry) -> bool, +{ + let mut file = File::open(path)?; + let mut remaining = file.metadata()?.len(); + let mut line_rev: Vec = Vec::new(); + let mut buf = vec![0u8; READ_CHUNK_SIZE]; + + while remaining > 0 { + let read_size = usize::try_from(remaining.min(READ_CHUNK_SIZE as u64)) + .map_err(std::io::Error::other)?; + remaining -= read_size as u64; + file.seek(SeekFrom::Start(remaining))?; + file.read_exact(&mut buf[..read_size])?; + + for &byte in buf[..read_size].iter().rev() { + if byte == b'\n' { + if let Some(entry) = parse_line_from_rev(&mut line_rev, &mut predicate)? { + return Ok(Some(entry)); + } + continue; + } + line_rev.push(byte); + } + } + + if let Some(entry) = parse_line_from_rev(&mut line_rev, &mut predicate)? { + return Ok(Some(entry)); + } + + Ok(None) +} + +fn parse_line_from_rev( + line_rev: &mut Vec, + predicate: &mut F, +) -> std::io::Result> +where + F: FnMut(&SessionIndexEntry) -> bool, +{ + if line_rev.is_empty() { + return Ok(None); + } + line_rev.reverse(); + let line = std::mem::take(line_rev); + let Ok(mut line) = String::from_utf8(line) else { + return Ok(None); + }; + if line.ends_with('\r') { + line.pop(); + } + let trimmed = line.trim(); + if trimmed.is_empty() { + return Ok(None); + } + let Ok(entry) = serde_json::from_str::(trimmed) else { + return Ok(None); + }; + if predicate(&entry) { + return Ok(Some(entry)); + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + fn write_index(path: &Path, lines: &[SessionIndexEntry]) -> std::io::Result<()> { + let mut out = String::new(); + for entry in lines { + out.push_str(&serde_json::to_string(entry).unwrap()); + out.push('\n'); + } + std::fs::write(path, out) + } + + #[test] + fn find_thread_id_by_name_prefers_latest_entry() -> std::io::Result<()> { + let temp = TempDir::new()?; + let path = session_index_path(temp.path()); + let id1 = ThreadId::new(); + let id2 = ThreadId::new(); + let lines = vec![ + SessionIndexEntry { + id: id1, + thread_name: "same".to_string(), + updated_at: "2024-01-01T00:00:00Z".to_string(), + }, + SessionIndexEntry { + id: id2, + thread_name: "same".to_string(), + updated_at: "2024-01-02T00:00:00Z".to_string(), + }, + ]; + write_index(&path, &lines)?; + + let found = scan_index_from_end_by_name(&path, "same")?; + assert_eq!(found.map(|entry| entry.id), Some(id2)); + Ok(()) + } + + #[test] + fn find_thread_name_by_id_prefers_latest_entry() -> std::io::Result<()> { + let temp = TempDir::new()?; + let path = session_index_path(temp.path()); + let id = ThreadId::new(); + let lines = vec![ + SessionIndexEntry { + id, + thread_name: "first".to_string(), + updated_at: "2024-01-01T00:00:00Z".to_string(), + }, + SessionIndexEntry { + id, + thread_name: "second".to_string(), + updated_at: "2024-01-02T00:00:00Z".to_string(), + }, + ]; + write_index(&path, &lines)?; + + let found = scan_index_from_end_by_id(&path, &id)?; + assert_eq!( + found.map(|entry| entry.thread_name), + Some("second".to_string()) + ); + Ok(()) + } + + #[test] + fn scan_index_returns_none_when_entry_missing() -> std::io::Result<()> { + let temp = TempDir::new()?; + let path = session_index_path(temp.path()); + let id = ThreadId::new(); + let lines = vec![SessionIndexEntry { + id, + thread_name: "present".to_string(), + updated_at: "2024-01-01T00:00:00Z".to_string(), + }]; + write_index(&path, &lines)?; + + let missing_name = scan_index_from_end_by_name(&path, "missing")?; + assert_eq!(missing_name, None); + + let missing_id = scan_index_from_end_by_id(&path, &ThreadId::new())?; + assert_eq!(missing_id, None); + Ok(()) + } + + #[test] + fn scan_index_finds_latest_match_among_mixed_entries() -> std::io::Result<()> { + let temp = TempDir::new()?; + let path = session_index_path(temp.path()); + let id_target = ThreadId::new(); + let id_other = ThreadId::new(); + let expected = SessionIndexEntry { + id: id_target, + thread_name: "target".to_string(), + updated_at: "2024-01-03T00:00:00Z".to_string(), + }; + let expected_other = SessionIndexEntry { + id: id_other, + thread_name: "target".to_string(), + updated_at: "2024-01-02T00:00:00Z".to_string(), + }; + // Resolution is based on append order (scan from end), not updated_at. + let lines = vec![ + SessionIndexEntry { + id: id_target, + thread_name: "target".to_string(), + updated_at: "2024-01-01T00:00:00Z".to_string(), + }, + expected_other.clone(), + expected.clone(), + SessionIndexEntry { + id: ThreadId::new(), + thread_name: "another".to_string(), + updated_at: "2024-01-04T00:00:00Z".to_string(), + }, + ]; + write_index(&path, &lines)?; + + let found_by_name = scan_index_from_end_by_name(&path, "target")?; + assert_eq!(found_by_name, Some(expected.clone())); + + let found_by_id = scan_index_from_end_by_id(&path, &id_target)?; + assert_eq!(found_by_id, Some(expected)); + + let found_other_by_id = scan_index_from_end_by_id(&path, &id_other)?; + assert_eq!(found_other_by_id, Some(expected_other)); + Ok(()) + } +} diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index c866afd3bd..6cde28d2f1 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -137,6 +137,7 @@ fn get_shell_path( let default_shell_path = get_user_shell_path(); if let Some(default_shell_path) = default_shell_path && detect_shell_type(&default_shell_path) == Some(shell_type) + && file_exists(&default_shell_path).is_some() { return Some(default_shell_path); } diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index 8aa6986882..f71ecfffa2 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -88,6 +88,7 @@ impl SkillsManager { Some(cwd_abs), &cli_overrides, LoaderOverrides::default(), + None, ) .await { diff --git a/codex-rs/core/src/state_db.rs b/codex-rs/core/src/state_db.rs index 7990a017ef..3e5beaa87e 100644 --- a/codex-rs/core/src/state_db.rs +++ b/codex-rs/core/src/state_db.rs @@ -11,6 +11,7 @@ use codex_otel::OtelManager; use codex_protocol::ThreadId; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource; +use codex_state::DB_METRIC_COMPARE_ERROR; pub use codex_state::LogEntry; use codex_state::STATE_DB_FILENAME; use codex_state::ThreadMetadataBuilder; @@ -32,12 +33,6 @@ pub(crate) async fn init_if_enabled( ) -> Option { let state_path = config.codex_home.join(STATE_DB_FILENAME); if !config.features.enabled(Feature::Sqlite) { - // We delete the file on best effort basis to maintain retro-compatibility in the future. - let wal_path = state_path.with_extension("sqlite-wal"); - let shm_path = state_path.with_extension("sqlite-shm"); - for path in [state_path.as_path(), wal_path.as_path(), shm_path.as_path()] { - tokio::fs::remove_file(path).await.ok(); - } return None; } let existed = tokio::fs::try_exists(&state_path).await.unwrap_or(false); @@ -282,9 +277,10 @@ pub async fn apply_rollout_items( pub fn record_discrepancy(stage: &str, reason: &str) { // We access the global metric because the call sites might not have access to the broader // OtelManager. + tracing::warn!("state db record_discrepancy: {stage}{reason}"); if let Some(metric) = codex_otel::metrics::global() { let _ = metric.counter( - "codex.db.discrepancy", + DB_METRIC_COMPARE_ERROR, 1, &[("stage", stage), ("reason", reason)], ); diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 9798fb8241..5138b3cd0d 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -1,4 +1,5 @@ use async_trait::async_trait; +use std::sync::Arc; use crate::function_tool::FunctionCallError; use crate::mcp_tool_call::handle_mcp_tool_call; @@ -42,7 +43,7 @@ impl ToolHandler for McpHandler { let arguments_str = raw_arguments; let response = handle_mcp_tool_call( - session.as_ref(), + Arc::clone(&session), turn.as_ref(), call_id.clone(), server, diff --git a/codex-rs/core/src/tools/handlers/request_user_input.rs b/codex-rs/core/src/tools/handlers/request_user_input.rs index 112dcae21e..b78bb51181 100644 --- a/codex-rs/core/src/tools/handlers/request_user_input.rs +++ b/codex-rs/core/src/tools/handlers/request_user_input.rs @@ -49,7 +49,19 @@ impl ToolHandler for RequestUserInputHandler { ))); } - let args: RequestUserInputArgs = parse_arguments(&arguments)?; + let mut args: RequestUserInputArgs = parse_arguments(&arguments)?; + let missing_options = args + .questions + .iter() + .any(|question| question.options.as_ref().is_none_or(Vec::is_empty)); + if missing_options { + return Err(FunctionCallError::RespondToModel( + "request_user_input requires non-empty options for every question".to_string(), + )); + } + for question in &mut args.questions { + question.is_other = true; + } let response = session .request_user_input(turn.as_ref(), call_id, args) .await diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index df65a94d8a..bf3b7b4b25 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -571,7 +571,7 @@ fn create_request_user_input_tool() -> ToolSpec { let options_schema = JsonSchema::Array { description: Some( - "Optional 2-3 mutually exclusive choices. Put the recommended option first and suffix its label with \"(Recommended)\". Do not include an \"Other\" option in this list; use isOther on the question to request a free form choice. If the question is free form in nature, please do not have any option." + "Provide 2-3 mutually exclusive choices. Put the recommended option first and suffix its label with \"(Recommended)\". Do not include an \"Other\" option in this list; the client will add a free-form \"Other\" option automatically." .to_string(), ), items: Box::new(JsonSchema::Object { @@ -602,15 +602,6 @@ fn create_request_user_input_tool() -> ToolSpec { description: Some("Single-sentence prompt shown to the user.".to_string()), }, ); - question_props.insert( - "isOther".to_string(), - JsonSchema::Boolean { - description: Some( - "True when this question should include a free-form \"Other\" option. Otherwise false." - .to_string(), - ), - }, - ); question_props.insert("options".to_string(), options_schema); let questions_schema = JsonSchema::Array { @@ -621,7 +612,7 @@ fn create_request_user_input_tool() -> ToolSpec { "id".to_string(), "header".to_string(), "question".to_string(), - "isOther".to_string(), + "options".to_string(), ]), additional_properties: Some(false.into()), }), diff --git a/codex-rs/core/src/util.rs b/codex-rs/core/src/util.rs index a100f28443..1a538da558 100644 --- a/codex-rs/core/src/util.rs +++ b/codex-rs/core/src/util.rs @@ -2,10 +2,13 @@ use std::path::Path; use std::path::PathBuf; use std::time::Duration; +use codex_protocol::ThreadId; use rand::Rng; use tracing::debug; use tracing::error; +use crate::parse_command::shlex_join; + const INITIAL_DELAY_MS: u64 = 200; const BACKOFF_FACTOR: f64 = 2.0; @@ -72,6 +75,32 @@ pub fn resolve_path(base: &Path, path: &PathBuf) -> PathBuf { } } +/// Trim a thread name and return `None` if it is empty after trimming. +pub fn normalize_thread_name(name: &str) -> Option { + let trimmed = name.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } +} + +pub fn resume_command(thread_name: Option<&str>, thread_id: Option) -> Option { + let resume_target = thread_name + .filter(|name| !name.is_empty()) + .map(str::to_string) + .or_else(|| thread_id.map(|thread_id| thread_id.to_string())); + resume_target.map(|target| { + let needs_double_dash = target.starts_with('-'); + let escaped = shlex_join(&[target]); + if needs_double_dash { + format!("codex resume -- {escaped}") + } else { + format!("codex resume {escaped}") + } + }) +} + #[cfg(test)] mod tests { use super::*; @@ -107,4 +136,51 @@ mod tests { feedback_tags!(model = "gpt-5", cached = true, debug_only = OnlyDebug); } + + #[test] + fn normalize_thread_name_trims_and_rejects_empty() { + assert_eq!(normalize_thread_name(" "), None); + assert_eq!( + normalize_thread_name(" my thread "), + Some("my thread".to_string()) + ); + } + + #[test] + fn resume_command_prefers_name_over_id() { + let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let command = resume_command(Some("my-thread"), Some(thread_id)); + assert_eq!(command, Some("codex resume my-thread".to_string())); + } + + #[test] + fn resume_command_with_only_id() { + let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let command = resume_command(None, Some(thread_id)); + assert_eq!( + command, + Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) + ); + } + + #[test] + fn resume_command_with_no_name_or_id() { + let command = resume_command(None, None); + assert_eq!(command, None); + } + + #[test] + fn resume_command_quotes_thread_name_when_needed() { + let command = resume_command(Some("-starts-with-dash"), None); + assert_eq!( + command, + Some("codex resume -- -starts-with-dash".to_string()) + ); + + let command = resume_command(Some("two words"), None); + assert_eq!(command, Some("codex resume 'two words'".to_string())); + + let command = resume_command(Some("quote'case"), None); + assert_eq!(command, Some("codex resume \"quote'case\"".to_string())); + } } diff --git a/codex-rs/core/templates/model_instructions/gpt-5.2-codex_instructions_template.md b/codex-rs/core/templates/model_instructions/gpt-5.2-codex_instructions_template.md index 6e9e53e361..530e825d5e 100644 --- a/codex-rs/core/templates/model_instructions/gpt-5.2-codex_instructions_template.md +++ b/codex-rs/core/templates/model_instructions/gpt-5.2-codex_instructions_template.md @@ -1,8 +1,6 @@ You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals. -# Personality - -{{ personality_message }} +{{ personality }} ## Tone and style - Anything you say outside of tool use is shown to the user. Do not narrate abstractly; explain what you are doing and why, using plain language. diff --git a/codex-rs/core/templates/personalities/friendly.md b/codex-rs/core/templates/personalities/gpt-5.2-codex_friendly.md similarity index 100% rename from codex-rs/core/templates/personalities/friendly.md rename to codex-rs/core/templates/personalities/gpt-5.2-codex_friendly.md diff --git a/codex-rs/core/templates/personalities/pragmatic.md b/codex-rs/core/templates/personalities/gpt-5.2-codex_pragmatic.md similarity index 99% rename from codex-rs/core/templates/personalities/pragmatic.md rename to codex-rs/core/templates/personalities/gpt-5.2-codex_pragmatic.md index 0fd76898a6..ca1738e42a 100644 --- a/codex-rs/core/templates/personalities/pragmatic.md +++ b/codex-rs/core/templates/personalities/gpt-5.2-codex_pragmatic.md @@ -1,4 +1,5 @@ # Personality + You are a deeply pragmatic, effective software engineer. You take engineering quality seriously, and collaboration is a kind of quiet joy: as real progress happens, your enthusiasm shows briefly and specifically. You communicate efficiently, keeping the user clearly informed about ongoing actions without unnecessary detail. ## Values diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 0744624a92..defc704353 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -73,6 +73,7 @@ mod tool_harness; mod tool_parallelism; mod tools; mod truncation; +mod turn_state; mod undo; mod unified_exec; mod unstable_features_warning; diff --git a/codex-rs/core/tests/suite/models_cache_ttl.rs b/codex-rs/core/tests/suite/models_cache_ttl.rs index 69506cffc5..5756f46a69 100644 --- a/codex-rs/core/tests/suite/models_cache_ttl.rs +++ b/codex-rs/core/tests/suite/models_cache_ttl.rs @@ -175,7 +175,7 @@ fn test_remote_model(slug: &str, priority: i32) -> ModelInfo { priority, upgrade: None, base_instructions: "base instructions".to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, diff --git a/codex-rs/core/tests/suite/personality.rs b/codex-rs/core/tests/suite/personality.rs index 7a27866569..79e0d36c68 100644 --- a/codex-rs/core/tests/suite/personality.rs +++ b/codex-rs/core/tests/suite/personality.rs @@ -9,10 +9,10 @@ use codex_core::protocol::SandboxPolicy; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; -use codex_protocol::openai_models::ModelInstructionsTemplate; +use codex_protocol::openai_models::ModelInstructionsVariables; +use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ModelVisibility; use codex_protocol::openai_models::ModelsResponse; -use codex_protocol::openai_models::PersonalityMessages; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::openai_models::ReasoningEffortPreset; use codex_protocol::openai_models::TruncationPolicyConfig; @@ -29,7 +29,6 @@ use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; -use std::collections::BTreeMap; use std::sync::Arc; use tempfile::TempDir; use tokio::time::Duration; @@ -49,6 +48,7 @@ fn sse_completed(id: &str) -> String { async fn model_personality_does_not_mutate_base_instructions_without_template() { let codex_home = TempDir::new().expect("create temp dir"); let mut config = load_default_config_for_test(&codex_home).await; + config.features.enable(Feature::Personality); config.model_personality = Some(Personality::Friendly); let model_info = ModelsManager::construct_model_info_offline("gpt-5.1", &config); @@ -62,6 +62,7 @@ async fn model_personality_does_not_mutate_base_instructions_without_template() async fn base_instructions_override_disables_personality_template() { let codex_home = TempDir::new().expect("create temp dir"); let mut config = load_default_config_for_test(&codex_home).await; + config.features.enable(Feature::Personality); config.model_personality = Some(Personality::Friendly); config.base_instructions = Some("override instructions".to_string()); @@ -80,7 +81,12 @@ async fn user_turn_personality_none_does_not_add_update_message() -> anyhow::Res let server = start_mock_server().await; let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await; - let mut builder = test_codex().with_model("gpt-5.2-codex"); + let mut builder = test_codex() + .with_model("gpt-5.2-codex") + .with_config(|config| { + config.features.disable(Feature::RemoteModels); + config.features.enable(Feature::Personality); + }); let test = builder.build(&server).await?; test.codex @@ -122,10 +128,11 @@ async fn config_personality_some_sets_instructions_template() -> anyhow::Result< let server = start_mock_server().await; let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await; let mut builder = test_codex() - .with_model("exp-codex-personality") + .with_model("gpt-5.2-codex") .with_config(|config| { - config.model_personality = Some(Personality::Friendly); config.features.disable(Feature::RemoteModels); + config.features.enable(Feature::Personality); + config.model_personality = Some(Personality::Friendly); }); let test = builder.build(&server).await?; @@ -182,6 +189,7 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> .with_model("exp-codex-personality") .with_config(|config| { config.features.disable(Feature::RemoteModels); + config.features.enable(Feature::Personality); }); let test = builder.build(&server).await?; @@ -263,6 +271,330 @@ async fn user_turn_personality_some_adds_update_message() -> anyhow::Result<()> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn instructions_uses_base_if_feature_disabled() -> anyhow::Result<()> { + let codex_home = TempDir::new().expect("create temp dir"); + let mut config = load_default_config_for_test(&codex_home).await; + config.features.disable(Feature::Personality); + config.model_personality = Some(Personality::Friendly); + + let model_info = ModelsManager::construct_model_info_offline("gpt-5.2-codex", &config); + assert_eq!( + model_info.get_model_instructions(config.model_personality), + model_info.base_instructions + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn user_turn_personality_skips_if_feature_disabled() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let resp_mock = mount_sse_sequence( + &server, + vec![sse_completed("resp-1"), sse_completed("resp-2")], + ) + .await; + let mut builder = test_codex() + .with_model("exp-codex-personality") + .with_config(|config| { + config.features.disable(Feature::RemoteModels); + config.features.disable(Feature::Personality); + }); + let test = builder.build(&server).await?; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: test.config.approval_policy.value(), + sandbox_policy: SandboxPolicy::ReadOnly, + model: test.session_configured.model.clone(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + test.codex + .submit(Op::OverrideTurnContext { + cwd: None, + approval_policy: None, + sandbox_policy: None, + windows_sandbox_level: None, + model: None, + effort: None, + summary: None, + collaboration_mode: None, + personality: Some(Personality::Friendly), + }) + .await?; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: test.config.approval_policy.value(), + sandbox_policy: SandboxPolicy::ReadOnly, + model: test.session_configured.model.clone(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let requests = resp_mock.requests(); + assert_eq!(requests.len(), 2, "expected two requests"); + let request = requests + .last() + .expect("expected personality update request"); + + let developer_texts = request.message_input_texts("developer"); + let personality_text = developer_texts + .iter() + .find(|text| text.contains("")); + assert!( + personality_text.is_none(), + "expected no personality preamble, got {personality_text:?}" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn ignores_remote_model_personality_if_remote_models_disabled() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = MockServer::builder() + .body_print_limit(BodyPrintLimit::Limited(80_000)) + .start() + .await; + + let remote_slug = "gpt-5.2-codex"; + let remote_personality_message = "Friendly from remote template"; + let remote_model = ModelInfo { + slug: remote_slug.to_string(), + display_name: "Remote personality test".to_string(), + description: Some("Remote model with personality template".to_string()), + default_reasoning_level: Some(ReasoningEffort::Medium), + supported_reasoning_levels: vec![ReasoningEffortPreset { + effort: ReasoningEffort::Medium, + description: ReasoningEffort::Medium.to_string(), + }], + shell_type: ConfigShellToolType::UnifiedExec, + visibility: ModelVisibility::List, + supported_in_api: true, + priority: 1, + upgrade: None, + base_instructions: "base instructions".to_string(), + model_messages: Some(ModelMessages { + instructions_template: Some( + "Base instructions\n{{ personality_message }}\n".to_string(), + ), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: None, + personality_friendly: Some(remote_personality_message.to_string()), + personality_pragmatic: None, + }), + }), + supports_reasoning_summaries: false, + support_verbosity: false, + default_verbosity: None, + apply_patch_tool_type: None, + truncation_policy: TruncationPolicyConfig::bytes(10_000), + supports_parallel_tool_calls: false, + context_window: Some(128_000), + auto_compact_token_limit: None, + effective_context_window_percent: 95, + experimental_supported_tools: Vec::new(), + }; + + let _models_mock = mount_models_once( + &server, + ModelsResponse { + models: vec![remote_model], + }, + ) + .await; + + let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await; + + let mut builder = test_codex() + .with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + config.features.disable(Feature::RemoteModels); + config.features.enable(Feature::Personality); + config.model = Some(remote_slug.to_string()); + config.model_personality = Some(Personality::Friendly); + }); + let test = builder.build(&server).await?; + + wait_for_model_available( + &test.thread_manager.get_models_manager(), + remote_slug, + &test.config, + ) + .await; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + model: remote_slug.to_string(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + let instructions_text = request.instructions_text(); + + assert!( + instructions_text.contains("You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals."), + "expected instructions to use the template instructions, got: {instructions_text:?}" + ); + assert!( + instructions_text.contains( + "You optimize for team morale and being a supportive teammate as much as code quality." + ), + "expected instructions to include the local friendly personality template, got: {instructions_text:?}" + ); + assert!( + !instructions_text.contains("{{ personality_message }}"), + "expected legacy personality placeholder to be replaced, got: {instructions_text:?}" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn remote_model_default_personality_instructions_with_feature() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + + let server = MockServer::builder() + .body_print_limit(BodyPrintLimit::Limited(80_000)) + .start() + .await; + + let remote_slug = "codex-remote-default-personality"; + let default_personality_message = "Default from remote template"; + let remote_model = ModelInfo { + slug: remote_slug.to_string(), + display_name: "Remote default personality test".to_string(), + description: Some("Remote model with default personality template".to_string()), + default_reasoning_level: Some(ReasoningEffort::Medium), + supported_reasoning_levels: vec![ReasoningEffortPreset { + effort: ReasoningEffort::Medium, + description: ReasoningEffort::Medium.to_string(), + }], + shell_type: ConfigShellToolType::UnifiedExec, + visibility: ModelVisibility::List, + supported_in_api: true, + priority: 1, + upgrade: None, + base_instructions: "base instructions".to_string(), + model_messages: Some(ModelMessages { + instructions_template: Some("Base instructions\n{{ personality }}\n".to_string()), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: Some(default_personality_message.to_string()), + personality_friendly: Some("Friendly variant".to_string()), + personality_pragmatic: Some("Pragmatic variant".to_string()), + }), + }), + supports_reasoning_summaries: false, + support_verbosity: false, + default_verbosity: None, + apply_patch_tool_type: None, + truncation_policy: TruncationPolicyConfig::bytes(10_000), + supports_parallel_tool_calls: false, + context_window: Some(128_000), + auto_compact_token_limit: None, + effective_context_window_percent: 95, + experimental_supported_tools: Vec::new(), + }; + + let _models_mock = mount_models_once( + &server, + ModelsResponse { + models: vec![remote_model], + }, + ) + .await; + + let resp_mock = mount_sse_once(&server, sse_completed("resp-1")).await; + + let mut builder = test_codex() + .with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(|config| { + config.features.enable(Feature::RemoteModels); + config.features.enable(Feature::Personality); + config.model = Some(remote_slug.to_string()); + }); + let test = builder.build(&server).await?; + + wait_for_model_available( + &test.thread_manager.get_models_manager(), + remote_slug, + &test.config, + ) + .await; + + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::ReadOnly, + model: remote_slug.to_string(), + effort: test.config.model_reasoning_effort, + summary: ReasoningSummary::Auto, + collaboration_mode: None, + personality: None, + }) + .await?; + + wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + let instructions_text = request.instructions_text(); + + assert!( + instructions_text.contains(default_personality_message), + "expected instructions to include the remote default personality template, got: {instructions_text:?}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn user_turn_personality_remote_model_template_includes_update_message() -> anyhow::Result<()> { @@ -290,12 +622,15 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - priority: 1, upgrade: None, base_instructions: "base instructions".to_string(), - model_instructions_template: Some(ModelInstructionsTemplate { - template: "Base instructions\n{{ personality_message }}\n".to_string(), - personality_messages: Some(PersonalityMessages(BTreeMap::from([( - Personality::Friendly, - remote_personality_message.to_string(), - )]))), + model_messages: Some(ModelMessages { + instructions_template: Some( + "Base instructions\n{{ personality_message }}\n".to_string(), + ), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: None, + personality_friendly: Some(remote_personality_message.to_string()), + personality_pragmatic: None, + }), }), supports_reasoning_summaries: false, support_verbosity: false, @@ -327,6 +662,7 @@ async fn user_turn_personality_remote_model_template_includes_update_message() - .with_auth(codex_core::CodexAuth::create_dummy_chatgpt_auth_for_testing()) .with_config(|config| { config.features.enable(Feature::RemoteModels); + config.features.enable(Feature::Personality); config.model = Some("gpt-5.2-codex".to_string()); }); let test = builder.build(&server).await?; diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index 71374cb84f..f59f73fd14 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -79,7 +79,7 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> { priority: 1, upgrade: None, base_instructions: "base instructions".to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, @@ -316,7 +316,7 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> { priority: 1, upgrade: None, base_instructions: remote_base.to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, @@ -790,7 +790,7 @@ fn test_remote_model_with_policy( priority, upgrade: None, base_instructions: "base instructions".to_string(), - model_instructions_template: None, + model_messages: None, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, diff --git a/codex-rs/core/tests/suite/request_user_input.rs b/codex-rs/core/tests/suite/request_user_input.rs index 3872969610..89a5ed3557 100644 --- a/codex-rs/core/tests/suite/request_user_input.rs +++ b/codex-rs/core/tests/suite/request_user_input.rs @@ -94,7 +94,6 @@ async fn request_user_input_round_trip_resolves_pending() -> anyhow::Result<()> "id": "confirm_path", "header": "Confirm", "question": "Proceed with the plan?", - "isOther": false, "options": [{ "label": "Yes (Recommended)", "description": "Continue the current plan." @@ -153,6 +152,7 @@ async fn request_user_input_round_trip_resolves_pending() -> anyhow::Result<()> .await; assert_eq!(request.call_id, call_id); assert_eq!(request.questions.len(), 1); + assert_eq!(request.questions[0].is_other, true); let mut answers = HashMap::new(); answers.insert( @@ -214,7 +214,6 @@ where "id": "confirm_path", "header": "Confirm", "question": "Proceed with the plan?", - "isOther": false, "options": [{ "label": "Yes (Recommended)", "description": "Continue the current plan." diff --git a/codex-rs/core/tests/suite/rollout_list_find.rs b/codex-rs/core/tests/suite/rollout_list_find.rs index 2af6192dbe..fc3ab80fc3 100644 --- a/codex-rs/core/tests/suite/rollout_list_find.rs +++ b/codex-rs/core/tests/suite/rollout_list_find.rs @@ -3,8 +3,16 @@ use std::io::Write; use std::path::Path; use std::path::PathBuf; +use codex_core::RolloutRecorder; +use codex_core::RolloutRecorderParams; +use codex_core::config::ConfigBuilder; use codex_core::find_archived_thread_path_by_id_str; use codex_core::find_thread_path_by_id_str; +use codex_core::find_thread_path_by_name_str; +use codex_core::protocol::SessionSource; +use codex_protocol::ThreadId; +use codex_protocol::models::BaseInstructions; +use pretty_assertions::assert_eq; use tempfile::TempDir; use uuid::Uuid; @@ -87,6 +95,54 @@ async fn find_ignores_granular_gitignore_rules() { assert_eq!(found, Some(expected)); } +#[tokio::test] +async fn find_locates_rollout_file_written_by_recorder() -> std::io::Result<()> { + // Ensures the name-based finder locates a rollout produced by the real recorder. + let home = TempDir::new().unwrap(); + let config = ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .build() + .await?; + let thread_id = ThreadId::new(); + let thread_name = "named thread"; + let recorder = RolloutRecorder::new( + &config, + RolloutRecorderParams::new( + thread_id, + None, + SessionSource::Exec, + BaseInstructions::default(), + Vec::new(), + ), + None, + None, + ) + .await?; + recorder.flush().await?; + + let index_path = home.path().join("session_index.jsonl"); + std::fs::write( + &index_path, + format!( + "{}\n", + serde_json::json!({ + "id": thread_id, + "thread_name": thread_name, + "updated_at": "2024-01-01T00:00:00Z" + }) + ), + )?; + + let found = find_thread_path_by_name_str(home.path(), thread_name).await?; + + let path = found.expect("expected rollout path to be found"); + assert!(path.exists()); + let contents = std::fs::read_to_string(&path)?; + assert!(contents.contains(&thread_id.to_string())); + recorder.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn find_archived_locates_rollout_file_by_id() { let home = TempDir::new().unwrap(); diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 8c79190383..b72f66fce8 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -64,7 +64,9 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { if let Some(parent) = abs_path.parent() { std::fs::create_dir_all(parent)?; } - let image = ImageBuffer::from_pixel(4096, 1024, Rgba([20u8, 40, 60, 255])); + let original_width = 2304; + let original_height = 864; + let image = ImageBuffer::from_pixel(original_width, original_height, Rgba([20u8, 40, 60, 255])); image.save(&abs_path)?; let response = sse(vec![ @@ -93,7 +95,13 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { }) .await?; - wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + &codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + // Empirically, image attachment can be slow under Bazel/RBE. + Duration::from_secs(10), + ) + .await; let body = mock.single_request().body_json(); let image_message = @@ -124,8 +132,8 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { let (width, height) = resized.dimensions(); assert!(width <= 2048); assert!(height <= 768); - assert!(width < 4096); - assert!(height < 1024); + assert!(width < original_width); + assert!(height < original_height); Ok(()) } @@ -148,7 +156,9 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { if let Some(parent) = abs_path.parent() { std::fs::create_dir_all(parent)?; } - let image = ImageBuffer::from_pixel(4096, 1024, Rgba([255u8, 0, 0, 255])); + let original_width = 2304; + let original_height = 864; + let image = ImageBuffer::from_pixel(original_width, original_height, Rgba([255u8, 0, 0, 255])); image.save(&abs_path)?; let call_id = "view-image-call"; @@ -261,8 +271,8 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { let (resized_width, resized_height) = resized.dimensions(); assert!(resized_width <= 2048); assert!(resized_height <= 768); - assert!(resized_width < 4096); - assert!(resized_height < 1024); + assert!(resized_width < original_width); + assert!(resized_height < original_height); Ok(()) } diff --git a/codex-rs/docs/protocol_v1.md b/codex-rs/docs/protocol_v1.md index 8cdc5c9575..74b5546012 100644 --- a/codex-rs/docs/protocol_v1.md +++ b/codex-rs/docs/protocol_v1.md @@ -75,7 +75,7 @@ For complete documentation of the `Op` and `EventMsg` variants, refer to [protoc - `EventMsg` - `EventMsg::AgentMessage` – Messages from the `Model` - `EventMsg::ExecApprovalRequest` – Request approval from user to execute a command - - `EventMsg::RequestUserInput` – Request user input for a tool call (questions can include options plus `isOther` to add a free-form choice) + - `EventMsg::RequestUserInput` – Request user input for a tool call (questions must include options; the client always adds a free-form choice) - `EventMsg::TurnComplete` – A turn completed successfully - `EventMsg::Error` – A turn stopped with an error - `EventMsg::Warning` – A non-fatal warning that the client should surface to the user diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index 12d0055cdd..3ec623330c 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -241,6 +241,7 @@ async fn load_exec_policy() -> anyhow::Result { cwd, &cli_overrides, overrides, + None, ) .await?; diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index 9156c22ea3..d4a64772e1 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -47,6 +47,7 @@ ts-rs = { workspace = true, features = [ "serde-json-impl", "no-serde-warnings", ] } +uuid = { workspace = true } [dev-dependencies] diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 4c10bb0d72..2980370de7 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -114,7 +114,7 @@ pub enum Command { struct ResumeArgsRaw { // Note: This is the direct clap shape. We reinterpret the positional when --last is set // so "codex resume --last " treats the positional as a prompt, not a session id. - /// Conversation/session id (UUID). When provided, resumes this session. + /// Conversation/session id (UUID) or thread name. UUIDs take precedence if it parses. /// If omitted, use --last to pick the most recent recorded session. #[arg(value_name = "SESSION_ID")] session_id: Option, @@ -144,7 +144,7 @@ struct ResumeArgsRaw { #[derive(Debug)] pub struct ResumeArgs { - /// Conversation/session id (UUID). When provided, resumes this session. + /// Conversation/session id (UUID) or thread name. UUIDs take precedence if it parses. /// If omitted, use --last to pick the most recent recorded session. pub session_id: Option, diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index fb7aa54ab4..e11e1fcadb 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -750,7 +750,8 @@ impl EventProcessor for EventProcessorWithHumanOutput { ); } EventMsg::ShutdownComplete => return CodexStatus::Shutdown, - EventMsg::ExecApprovalRequest(_) + EventMsg::ThreadNameUpdated(_) + | EventMsg::ExecApprovalRequest(_) | EventMsg::ApplyPatchApprovalRequest(_) | EventMsg::TerminalInteraction(_) | EventMsg::ExecCommandOutputDelta(_) diff --git a/codex-rs/exec/src/event_processor_with_jsonl_output.rs b/codex-rs/exec/src/event_processor_with_jsonl_output.rs index d43ce4387c..687df574d6 100644 --- a/codex-rs/exec/src/event_processor_with_jsonl_output.rs +++ b/codex-rs/exec/src/event_processor_with_jsonl_output.rs @@ -117,6 +117,7 @@ impl EventProcessorWithJsonOutput { pub fn collect_thread_events(&mut self, event: &protocol::Event) -> Vec { match &event.msg { protocol::EventMsg::SessionConfigured(ev) => self.handle_session_configured(ev), + protocol::EventMsg::ThreadNameUpdated(_) => Vec::new(), protocol::EventMsg::AgentMessage(ev) => self.handle_agent_message(ev), protocol::EventMsg::AgentReasoning(ev) => self.handle_reasoning_event(ev), protocol::EventMsg::ExecCommandBegin(ev) => self.handle_exec_command_begin(ev), diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index f2c212d895..f7176c13b0 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -59,12 +59,14 @@ use tracing::info; use tracing::warn; use tracing_subscriber::EnvFilter; use tracing_subscriber::prelude::*; +use uuid::Uuid; use crate::cli::Command as ExecCommand; use crate::event_processor::CodexStatus; use crate::event_processor::EventProcessor; use codex_core::default_client::set_default_originator; use codex_core::find_thread_path_by_id_str; +use codex_core::find_thread_path_by_name_str; enum InitialOperation { UserTurn { @@ -619,8 +621,13 @@ async fn resolve_resume_path( } } } else if let Some(id_str) = args.session_id.as_deref() { - let path = find_thread_path_by_id_str(&config.codex_home, id_str).await?; - Ok(path) + if Uuid::parse_str(id_str).is_ok() { + let path = find_thread_path_by_id_str(&config.codex_home, id_str).await?; + Ok(path) + } else { + let path = find_thread_path_by_name_str(&config.codex_home, id_str).await?; + Ok(path) + } } else { Ok(None) } diff --git a/codex-rs/exec/tests/event_processor_with_json_output.rs b/codex-rs/exec/tests/event_processor_with_json_output.rs index 105824c60d..dc5f220fd3 100644 --- a/codex-rs/exec/tests/event_processor_with_json_output.rs +++ b/codex-rs/exec/tests/event_processor_with_json_output.rs @@ -88,6 +88,7 @@ fn session_configured_produces_thread_started_event() { EventMsg::SessionConfigured(SessionConfiguredEvent { session_id, forked_from_id: None, + thread_name: None, model: "codex-mini-latest".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index dcaf8a89e6..c96bc60706 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -308,6 +308,9 @@ async fn run_codex_tool_session_inner( EventMsg::SessionConfigured(_) => { tracing::error!("unexpected SessionConfigured event"); } + EventMsg::ThreadNameUpdated(_) => { + // Ignore session metadata updates in MCP tool runner. + } EventMsg::AgentMessageDelta(_) => { // TODO: think how we want to support this in the MCP } diff --git a/codex-rs/mcp-server/src/outgoing_message.rs b/codex-rs/mcp-server/src/outgoing_message.rs index 2069db69d0..954e50d9de 100644 --- a/codex-rs/mcp-server/src/outgoing_message.rs +++ b/codex-rs/mcp-server/src/outgoing_message.rs @@ -258,6 +258,7 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: thread_id, forked_from_id: None, + thread_name: None, model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -298,6 +299,7 @@ mod tests { let session_configured_event = SessionConfiguredEvent { session_id: conversation_id, forked_from_id: None, + thread_name: None, model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -362,6 +364,7 @@ mod tests { let session_configured_event = SessionConfiguredEvent { session_id: thread_id, forked_from_id: None, + thread_name: None, model: "gpt-4o".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, diff --git a/codex-rs/network-proxy/src/state.rs b/codex-rs/network-proxy/src/state.rs index 552455f510..b0c8fdfbcb 100644 --- a/codex-rs/network-proxy/src/state.rs +++ b/codex-rs/network-proxy/src/state.rs @@ -31,9 +31,10 @@ pub(crate) async fn build_config_state() -> Result { let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; let cli_overrides = Vec::new(); let overrides = LoaderOverrides::default(); - let config_layer_stack = load_config_layers_state(&codex_home, None, &cli_overrides, overrides) - .await - .context("failed to load Codex config")?; + let config_layer_stack = + load_config_layers_state(&codex_home, None, &cli_overrides, overrides, None) + .await + .context("failed to load Codex config")?; let cfg_path = codex_home.join(CONFIG_TOML_FILE); diff --git a/codex-rs/otel/src/metrics/client.rs b/codex-rs/otel/src/metrics/client.rs index 9b1b01a3ed..62a2df2e8a 100644 --- a/codex-rs/otel/src/metrics/client.rs +++ b/codex-rs/otel/src/metrics/client.rs @@ -34,6 +34,8 @@ use tracing::debug; const ENV_ATTRIBUTE: &str = "env"; const METER_NAME: &str = "codex"; +const DURATION_UNIT: &str = "ms"; +const DURATION_DESCRIPTION: &str = "Duration in milliseconds."; #[derive(Debug)] struct MetricsClientInner { @@ -41,6 +43,7 @@ struct MetricsClientInner { meter: Meter, counters: Mutex>>, histograms: Mutex>>, + duration_histograms: Mutex>>, default_tags: BTreeMap, } @@ -81,6 +84,25 @@ impl MetricsClientInner { Ok(()) } + fn duration_histogram(&self, name: &str, value: i64, tags: &[(&str, &str)]) -> Result<()> { + validate_metric_name(name)?; + let attributes = self.attributes(tags)?; + + let mut histograms = self + .duration_histograms + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let histogram = histograms.entry(name.to_string()).or_insert_with(|| { + self.meter + .f64_histogram(name.to_string()) + .with_unit(DURATION_UNIT) + .with_description(DURATION_DESCRIPTION) + .build() + }); + histogram.record(value as f64, &attributes); + Ok(()) + } + fn attributes(&self, tags: &[(&str, &str)]) -> Result> { if tags.is_empty() { return Ok(self @@ -150,6 +172,7 @@ impl MetricsClient { meter, counters: Mutex::new(HashMap::new()), histograms: Mutex::new(HashMap::new()), + duration_histograms: Mutex::new(HashMap::new()), default_tags: config.default_tags, }))) } @@ -171,7 +194,7 @@ impl MetricsClient { duration: Duration, tags: &[(&str, &str)], ) -> Result<()> { - self.histogram( + self.0.duration_histogram( name, duration.as_millis().min(i64::MAX as u128) as i64, tags, diff --git a/codex-rs/otel/tests/suite/timing.rs b/codex-rs/otel/tests/suite/timing.rs index ce4f2f982e..8398e1a0d7 100644 --- a/codex-rs/otel/tests/suite/timing.rs +++ b/codex-rs/otel/tests/suite/timing.rs @@ -18,12 +18,17 @@ fn record_duration_records_histogram() -> Result<()> { )?; metrics.shutdown()?; + let resource_metrics = latest_metrics(&exporter); let (bounds, bucket_counts, sum, count) = - histogram_data(&latest_metrics(&exporter), "codex.request_latency"); + histogram_data(&resource_metrics, "codex.request_latency"); assert!(!bounds.is_empty()); assert_eq!(bucket_counts.iter().sum::(), 1); assert_eq!(sum, 15.0); assert_eq!(count, 1); + let metric = crate::harness::find_metric(&resource_metrics, "codex.request_latency") + .unwrap_or_else(|| panic!("metric codex.request_latency missing")); + assert_eq!(metric.unit(), "ms"); + assert_eq!(metric.description(), "Duration in milliseconds."); Ok(()) } @@ -46,6 +51,10 @@ fn timer_result_records_success() -> Result<()> { assert!(!bounds.is_empty()); assert_eq!(count, 1); assert_eq!(bucket_counts.iter().sum::(), 1); + let metric = crate::harness::find_metric(&resource_metrics, "codex.request_latency") + .unwrap_or_else(|| panic!("metric codex.request_latency missing")); + assert_eq!(metric.unit(), "ms"); + assert_eq!(metric.description(), "Duration in milliseconds."); let attrs = attributes_to_map( match crate::harness::find_metric(&resource_metrics, "codex.request_latency").and_then( |metric| match metric.data() { diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 3f29484ffd..9a940539d5 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -1,4 +1,3 @@ -use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; @@ -14,7 +13,7 @@ use ts_rs::TS; use crate::config_types::Personality; use crate::config_types::Verbosity; -const PERSONALITY_PLACEHOLDER: &str = "{{ personality_message }}"; +const PERSONALITY_PLACEHOLDER: &str = "{{ personality }}"; /// See https://platform.openai.com/docs/guides/reasoning?api-mode=responses#get-started-with-reasoning #[derive( @@ -189,7 +188,7 @@ pub struct ModelInfo { pub upgrade: Option, pub base_instructions: String, #[serde(default, skip_serializing_if = "Option::is_none")] - pub model_instructions_template: Option, + pub model_messages: Option, pub supports_reasoning_summaries: bool, pub support_verbosity: bool, pub default_verbosity: Option, @@ -218,26 +217,25 @@ impl ModelInfo { } pub fn supports_personality(&self) -> bool { - self.model_instructions_template + self.model_messages .as_ref() - .is_some_and(ModelInstructionsTemplate::supports_personality) + .is_some_and(ModelMessages::supports_personality) } pub fn get_model_instructions(&self, personality: Option) -> String { - if let Some(personality) = personality - && let Some(template) = &self.model_instructions_template - && template.has_personality_placeholder() - && let Some(personality_messages) = &template.personality_messages - && let Some(personality_message) = personality_messages.0.get(&personality) + if let Some(model_messages) = &self.model_messages + && let Some(template) = &model_messages.instructions_template { - template - .template - .replace(PERSONALITY_PLACEHOLDER, personality_message.as_str()) + // if we have a template, always use it + let personality_message = model_messages + .get_personality_message(personality) + .unwrap_or_default(); + template.replace(PERSONALITY_PLACEHOLDER, personality_message.as_str()) } else if let Some(personality) = personality { warn!( model = %self.slug, %personality, - "Model personality requested but model_instructions_template is invalid, falling back to base instructions." + "Model personality requested but model_messages is missing, falling back to base instructions." ); self.base_instructions.clone() } else { @@ -246,31 +244,62 @@ impl ModelInfo { } } -/// A strongly-typed template for assembling model instructions. If populated and valid, will override -/// base_instructions. +/// A strongly-typed template for assembling model instructions and developer messages. If +/// instructions_* is populated and valid, it will override base_instructions. #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] -pub struct ModelInstructionsTemplate { - pub template: String, - pub personality_messages: Option, +pub struct ModelMessages { + pub instructions_template: Option, + pub instructions_variables: Option, } -impl ModelInstructionsTemplate { +impl ModelMessages { fn has_personality_placeholder(&self) -> bool { - self.template.contains(PERSONALITY_PLACEHOLDER) + self.instructions_template + .as_ref() + .map(|spec| spec.contains(PERSONALITY_PLACEHOLDER)) + .unwrap_or(false) } fn supports_personality(&self) -> bool { self.has_personality_placeholder() - && self.personality_messages.as_ref().is_some_and(|messages| { - Personality::iter().all(|personality| messages.0.contains_key(&personality)) - }) + && self + .instructions_variables + .as_ref() + .is_some_and(ModelInstructionsVariables::is_complete) + } + + pub fn get_personality_message(&self, personality: Option) -> Option { + self.instructions_variables + .as_ref() + .and_then(|variables| variables.get_personality_message(personality)) } } -// serializes as a dictionary from personality to message -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, TS, JsonSchema)] -#[serde(transparent)] -pub struct PersonalityMessages(pub BTreeMap); +#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] +pub struct ModelInstructionsVariables { + pub personality_default: Option, + pub personality_friendly: Option, + pub personality_pragmatic: Option, +} + +impl ModelInstructionsVariables { + pub fn is_complete(&self) -> bool { + self.personality_default.is_some() + && self.personality_friendly.is_some() + && self.personality_pragmatic.is_some() + } + + pub fn get_personality_message(&self, personality: Option) -> Option { + if let Some(personality) = personality { + match personality { + Personality::Friendly => self.personality_friendly.clone(), + Personality::Pragmatic => self.personality_pragmatic.clone(), + } + } else { + self.personality_default.clone() + } + } +} #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema)] pub struct ModelInfoUpgrade { @@ -407,7 +436,7 @@ mod tests { use super::*; use pretty_assertions::assert_eq; - fn test_model(template: Option) -> ModelInfo { + fn test_model(spec: Option) -> ModelInfo { ModelInfo { slug: "test-model".to_string(), display_name: "Test Model".to_string(), @@ -420,7 +449,7 @@ mod tests { priority: 1, upgrade: None, base_instructions: "base".to_string(), - model_instructions_template: template, + model_messages: spec, supports_reasoning_summaries: false, support_verbosity: false, default_verbosity: None, @@ -434,18 +463,19 @@ mod tests { } } - fn personality_messages() -> PersonalityMessages { - PersonalityMessages(BTreeMap::from([( - Personality::Friendly, - "friendly".to_string(), - )])) + fn personality_variables() -> ModelInstructionsVariables { + ModelInstructionsVariables { + personality_default: Some("default".to_string()), + personality_friendly: Some("friendly".to_string()), + personality_pragmatic: Some("pragmatic".to_string()), + } } #[test] fn get_model_instructions_uses_template_when_placeholder_present() { - let model = test_model(Some(ModelInstructionsTemplate { - template: "Hello {{ personality_message }}".to_string(), - personality_messages: Some(personality_messages()), + let model = test_model(Some(ModelMessages { + instructions_template: Some("Hello {{ personality }}".to_string()), + instructions_variables: Some(personality_variables()), })); let instructions = model.get_model_instructions(Some(Personality::Friendly)); @@ -454,14 +484,116 @@ mod tests { } #[test] - fn get_model_instructions_falls_back_when_placeholder_missing() { - let model = test_model(Some(ModelInstructionsTemplate { - template: "Hello there".to_string(), - personality_messages: Some(personality_messages()), + fn get_model_instructions_always_strips_placeholder() { + let model = test_model(Some(ModelMessages { + instructions_template: Some("Hello\n{{ personality }}".to_string()), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: None, + personality_friendly: Some("friendly".to_string()), + personality_pragmatic: None, + }), + })); + assert_eq!( + model.get_model_instructions(Some(Personality::Friendly)), + "Hello\nfriendly" + ); + assert_eq!( + model.get_model_instructions(Some(Personality::Pragmatic)), + "Hello\n" + ); + assert_eq!(model.get_model_instructions(None), "Hello\n"); + + let model_no_personality = test_model(Some(ModelMessages { + instructions_template: Some("Hello\n{{ personality }}".to_string()), + instructions_variables: Some(ModelInstructionsVariables { + personality_default: None, + personality_friendly: None, + personality_pragmatic: None, + }), + })); + assert_eq!( + model_no_personality.get_model_instructions(Some(Personality::Friendly)), + "Hello\n" + ); + assert_eq!( + model_no_personality.get_model_instructions(Some(Personality::Pragmatic)), + "Hello\n" + ); + assert_eq!(model_no_personality.get_model_instructions(None), "Hello\n"); + } + + #[test] + fn get_model_instructions_falls_back_when_template_is_missing() { + let model = test_model(Some(ModelMessages { + instructions_template: None, + instructions_variables: Some(ModelInstructionsVariables { + personality_default: None, + personality_friendly: None, + personality_pragmatic: None, + }), })); let instructions = model.get_model_instructions(Some(Personality::Friendly)); assert_eq!(instructions, "base"); } + + #[test] + fn get_personality_message_returns_default_when_personality_is_none() { + let personality_template = personality_variables(); + assert_eq!( + personality_template.get_personality_message(None), + Some("default".to_string()) + ); + } + + #[test] + fn get_personality_message() { + let personality_variables = personality_variables(); + assert_eq!( + personality_variables.get_personality_message(Some(Personality::Friendly)), + Some("friendly".to_string()) + ); + assert_eq!( + personality_variables.get_personality_message(Some(Personality::Pragmatic)), + Some("pragmatic".to_string()) + ); + assert_eq!( + personality_variables.get_personality_message(None), + Some("default".to_string()) + ); + + let personality_variables = ModelInstructionsVariables { + personality_default: Some("default".to_string()), + personality_friendly: None, + personality_pragmatic: None, + }; + assert_eq!( + personality_variables.get_personality_message(Some(Personality::Friendly)), + None + ); + assert_eq!( + personality_variables.get_personality_message(Some(Personality::Pragmatic)), + None + ); + assert_eq!( + personality_variables.get_personality_message(None), + Some("default".to_string()) + ); + + let personality_variables = ModelInstructionsVariables { + personality_default: None, + personality_friendly: Some("friendly".to_string()), + personality_pragmatic: Some("pragmatic".to_string()), + }; + assert_eq!( + personality_variables.get_personality_message(Some(Personality::Friendly)), + Some("friendly".to_string()) + ); + assert_eq!( + personality_variables.get_personality_message(Some(Personality::Pragmatic)), + Some("pragmatic".to_string()) + ); + assert_eq!(personality_variables.get_personality_message(None), None); + } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 7ba01fa4bd..fbaf4f4457 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -273,6 +273,11 @@ pub enum Op { /// to generate a summary which will be returned as an AgentMessage event. Compact, + /// Set a user-facing thread name in the persisted rollout metadata. + /// This is a local-only operation handled by codex-core; it does not + /// involve the model. + SetThreadName { name: String }, + /// Request Codex to undo a turn (turn are stacked so it is the same effect as CMD + Z). Undo, @@ -735,6 +740,9 @@ pub enum EventMsg { /// Ack the client's configure message. SessionConfigured(SessionConfiguredEvent), + /// Updated session metadata (e.g., thread name changes). + ThreadNameUpdated(ThreadNameUpdatedEvent), + /// Incremental MCP startup progress updates. McpStartupUpdate(McpStartupUpdateEvent), @@ -2186,11 +2194,15 @@ pub struct SkillsListEntry { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct SessionConfiguredEvent { - /// Name left as session_id instead of thread_id for backwards compatibility. pub session_id: ThreadId, #[serde(skip_serializing_if = "Option::is_none")] pub forked_from_id: Option, + /// Optional user-facing thread name (may be unset). + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub thread_name: Option, + /// Tell the client what model is being queried. pub model: String, @@ -2226,6 +2238,14 @@ pub struct SessionConfiguredEvent { pub rollout_path: Option, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct ThreadNameUpdatedEvent { + pub thread_id: ThreadId, + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub thread_name: Option, +} + /// User's decision in response to an ExecApprovalRequest. #[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, Eq, Display, JsonSchema, TS)] #[serde(rename_all = "snake_case")] @@ -2569,6 +2589,7 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: conversation_id, forked_from_id: None, + thread_name: None, model: "codex-mini-latest".to_string(), model_provider_id: "openai".to_string(), approval_policy: AskForApproval::Never, diff --git a/codex-rs/state/src/bin/logs_client.rs b/codex-rs/state/src/bin/logs_client.rs index 4389c29bc6..d1329a1a89 100644 --- a/codex-rs/state/src/bin/logs_client.rs +++ b/codex-rs/state/src/bin/logs_client.rs @@ -3,8 +3,6 @@ use std::time::Duration; use anyhow::Context; use chrono::DateTime; -use chrono::SecondsFormat; -use chrono::Utc; use clap::Parser; use codex_state::LogQuery; use codex_state::LogRow; @@ -37,17 +35,21 @@ struct Args { #[arg(long, value_name = "RFC3339|UNIX")] to: Option, - /// Substring match on module_path. - #[arg(long)] - module: Option, + /// Substring match on module_path. Repeat to include multiple substrings. + #[arg(long = "module")] + module: Vec, - /// Substring match on file path. - #[arg(long)] - file: Option, + /// Substring match on file path. Repeat to include multiple substrings. + #[arg(long = "file")] + file: Vec, - /// Match a specific thread id. + /// Match one or more thread ids. Repeat to include multiple threads. + #[arg(long = "thread-id")] + thread_id: Vec, + + /// Include logs that do not have a thread id. #[arg(long)] - thread_id: Option, + threadless: bool, /// Number of matching rows to show before tailing. #[arg(long, default_value_t = 200)] @@ -63,9 +65,10 @@ struct LogFilter { level_upper: Option, from_ts: Option, to_ts: Option, - module_like: Option, - file_like: Option, - thread_id: Option, + module_like: Vec, + file_like: Vec, + thread_ids: Vec, + include_threadless: bool, } #[tokio::main] @@ -126,14 +129,33 @@ fn build_filter(args: &Args) -> anyhow::Result { .context("failed to parse --to")?; let level_upper = args.level.as_ref().map(|level| level.to_ascii_uppercase()); + let module_like = args + .module + .iter() + .filter(|module| !module.is_empty()) + .cloned() + .collect::>(); + let file_like = args + .file + .iter() + .filter(|file| !file.is_empty()) + .cloned() + .collect::>(); + let thread_ids = args + .thread_id + .iter() + .filter(|thread_id| !thread_id.is_empty()) + .cloned() + .collect::>(); Ok(LogFilter { level_upper, from_ts, to_ts, - module_like: args.module.clone(), - file_like: args.file.clone(), - thread_id: args.thread_id.clone(), + module_like, + file_like, + thread_ids, + include_threadless: args.threadless, }) } @@ -211,7 +233,8 @@ fn to_log_query( to_ts: filter.to_ts, module_like: filter.module_like.clone(), file_like: filter.file_like.clone(), - thread_id: filter.thread_id.clone(), + thread_ids: filter.thread_ids.clone(), + include_threadless: filter.include_threadless, after_id, limit, descending, @@ -219,45 +242,82 @@ fn to_log_query( } fn format_row(row: &LogRow) -> String { - let timestamp = format_timestamp(row.ts, row.ts_nanos); + let timestamp = formatter::ts(row.ts, row.ts_nanos); let level = row.level.as_str(); let target = row.target.as_str(); let message = row.message.as_deref().unwrap_or(""); - let level_colored = color_level(level); + let level_colored = formatter::level(level); let timestamp_colored = timestamp.dimmed().to_string(); let thread_id = row.thread_id.as_deref().unwrap_or("-"); let thread_id_colored = thread_id.blue().dimmed().to_string(); let target_colored = target.dimmed().to_string(); - let message_colored = message.bold().to_string(); + let message_colored = heuristic_formatting(message); format!( "{timestamp_colored} {level_colored} [{thread_id_colored}] {target_colored} - {message_colored}" ) } -fn color_level(level: &str) -> String { - let padded = format!("{level:<5}"); - if level.eq_ignore_ascii_case("error") { - return padded.red().bold().to_string(); +fn heuristic_formatting(message: &str) -> String { + if matcher::apply_patch(message) { + formatter::apply_patch(message) + } else { + message.bold().to_string() } - if level.eq_ignore_ascii_case("warn") { - return padded.yellow().bold().to_string(); - } - if level.eq_ignore_ascii_case("info") { - return padded.green().bold().to_string(); - } - if level.eq_ignore_ascii_case("debug") { - return padded.blue().bold().to_string(); - } - if level.eq_ignore_ascii_case("trace") { - return padded.magenta().bold().to_string(); - } - padded.bold().to_string() } -fn format_timestamp(ts: i64, ts_nanos: i64) -> String { - let nanos = u32::try_from(ts_nanos).unwrap_or(0); - match DateTime::::from_timestamp(ts, nanos) { - Some(dt) => dt.to_rfc3339_opts(SecondsFormat::Millis, true), - None => format!("{ts}.{ts_nanos:09}Z"), +mod matcher { + pub(super) fn apply_patch(message: &str) -> bool { + message.starts_with("ToolCall: apply_patch") + } +} + +mod formatter { + use chrono::DateTime; + use chrono::SecondsFormat; + use chrono::Utc; + use owo_colors::OwoColorize; + + pub(super) fn apply_patch(message: &str) -> String { + message + .lines() + .map(|line| { + if line.starts_with('+') { + line.green().bold().to_string() + } else if line.starts_with('-') { + line.red().bold().to_string() + } else { + line.bold().to_string() + } + }) + .collect::>() + .join("\n") + } + + pub(super) fn ts(ts: i64, ts_nanos: i64) -> String { + let nanos = u32::try_from(ts_nanos).unwrap_or(0); + match DateTime::::from_timestamp(ts, nanos) { + Some(dt) => dt.to_rfc3339_opts(SecondsFormat::Millis, true), + None => format!("{ts}.{ts_nanos:09}Z"), + } + } + + pub(super) fn level(level: &str) -> String { + let padded = format!("{level:<5}"); + if level.eq_ignore_ascii_case("error") { + return padded.red().bold().to_string(); + } + if level.eq_ignore_ascii_case("warn") { + return padded.yellow().bold().to_string(); + } + if level.eq_ignore_ascii_case("info") { + return padded.green().bold().to_string(); + } + if level.eq_ignore_ascii_case("debug") { + return padded.blue().bold().to_string(); + } + if level.eq_ignore_ascii_case("trace") { + return padded.magenta().bold().to_string(); + } + padded.bold().to_string() } } diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 9c8ce21556..8d327a4d1a 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -30,6 +30,11 @@ pub fn apply_rollout_item( } fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &SessionMetaLine) { + if metadata.id != meta_line.meta.id { + // Ignore session_meta lines that don't match the canonical thread ID, + // e.g., forked rollouts that embed the source session metadata. + return; + } metadata.id = meta_line.meta.id; metadata.source = enum_to_string(&meta_line.meta.source); if let Some(provider) = meta_line.meta.model_provider.as_deref() { diff --git a/codex-rs/state/src/lib.rs b/codex-rs/state/src/lib.rs index 685552015e..c08c76a1cf 100644 --- a/codex-rs/state/src/lib.rs +++ b/codex-rs/state/src/lib.rs @@ -34,5 +34,7 @@ pub use runtime::STATE_DB_FILENAME; pub const DB_ERROR_METRIC: &str = "codex.db.error"; /// Metrics on backfill process during first init of the db. Tags: [status] pub const DB_METRIC_BACKFILL: &str = "codex.db.backfill"; +/// Metrics on backfill duration during first init of the db. Tags: [status] +pub const DB_METRIC_BACKFILL_DURATION_MS: &str = "codex.db.backfill.duration_ms"; /// Metrics on errors during comparison between DB and rollout file. Tags: [stage] pub const DB_METRIC_COMPARE_ERROR: &str = "codex.db.compare_error"; diff --git a/codex-rs/state/src/model/log.rs b/codex-rs/state/src/model/log.rs index 5d774249dd..819abb5d22 100644 --- a/codex-rs/state/src/model/log.rs +++ b/codex-rs/state/src/model/log.rs @@ -32,9 +32,10 @@ pub struct LogQuery { pub level_upper: Option, pub from_ts: Option, pub to_ts: Option, - pub module_like: Option, - pub file_like: Option, - pub thread_id: Option, + pub module_like: Vec, + pub file_like: Vec, + pub thread_ids: Vec, + pub include_threadless: bool, pub after_id: Option, pub limit: Option, pub descending: bool, diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index e0805e5b65..39c79c32b9 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -457,28 +457,54 @@ fn push_log_filters<'a>(builder: &mut QueryBuilder<'a, Sqlite>, query: &'a LogQu if let Some(to_ts) = query.to_ts { builder.push(" AND ts <= ").push_bind(to_ts); } - if let Some(module_like) = query.module_like.as_ref() { - builder - .push(" AND module_path LIKE '%' || ") - .push_bind(module_like.as_str()) - .push(" || '%'"); - } - if let Some(file_like) = query.file_like.as_ref() { - builder - .push(" AND file LIKE '%' || ") - .push_bind(file_like.as_str()) - .push(" || '%'"); - } - if let Some(thread_id) = query.thread_id.as_ref() { - builder - .push(" AND thread_id = ") - .push_bind(thread_id.as_str()); + push_like_filters(builder, "module_path", &query.module_like); + push_like_filters(builder, "file", &query.file_like); + let has_thread_filter = !query.thread_ids.is_empty() || query.include_threadless; + if has_thread_filter { + builder.push(" AND ("); + let mut needs_or = false; + for thread_id in &query.thread_ids { + if needs_or { + builder.push(" OR "); + } + builder.push("thread_id = ").push_bind(thread_id.as_str()); + needs_or = true; + } + if query.include_threadless { + if needs_or { + builder.push(" OR "); + } + builder.push("thread_id IS NULL"); + } + builder.push(")"); } if let Some(after_id) = query.after_id { builder.push(" AND id > ").push_bind(after_id); } } +fn push_like_filters<'a>( + builder: &mut QueryBuilder<'a, Sqlite>, + column: &str, + filters: &'a [String], +) { + if filters.is_empty() { + return; + } + builder.push(" AND ("); + for (idx, filter) in filters.iter().enumerate() { + if idx > 0 { + builder.push(" OR "); + } + builder + .push(column) + .push(" LIKE '%' || ") + .push_bind(filter.as_str()) + .push(" || '%'"); + } + builder.push(")"); +} + async fn open_sqlite(path: &Path) -> anyhow::Result { let options = SqliteConnectOptions::new() .filename(path) diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 6664e7f9c5..d859aa945e 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -31,6 +31,7 @@ codex-app-server-protocol = { workspace = true } codex-arg0 = { workspace = true } codex-backend-client = { workspace = true } codex-chatgpt = { workspace = true } +codex-cloud-requirements = { workspace = true } codex-common = { workspace = true, features = [ "cli", "elapsed", @@ -94,6 +95,7 @@ tree-sitter-highlight = { workspace = true } unicode-segmentation = { workspace = true } unicode-width = { workspace = true } url = { workspace = true } +uuid = { workspace = true } codex-windows-sandbox = { workspace = true } tokio-util = { workspace = true, features = ["time"] } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 04dd27c121..eae5af1b33 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -107,6 +107,7 @@ const THREAD_EVENT_CHANNEL_CAPACITY: usize = 32768; pub struct AppExitInfo { pub token_usage: TokenUsage, pub thread_id: Option, + pub thread_name: Option, pub update_action: Option, pub exit_reason: ExitReason, } @@ -116,6 +117,7 @@ impl AppExitInfo { Self { token_usage: TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::Fatal(message.into()), } @@ -134,13 +136,17 @@ pub enum ExitReason { Fatal(String), } -fn session_summary(token_usage: TokenUsage, thread_id: Option) -> Option { +fn session_summary( + token_usage: TokenUsage, + thread_id: Option, + thread_name: Option, +) -> Option { if token_usage.is_zero() { return None; } let usage_line = FinalOutput::from(token_usage).to_string(); - let resume_command = thread_id.map(|thread_id| format!("codex resume {thread_id}")); + let resume_command = codex_core::util::resume_command(thread_name.as_deref(), thread_id); Some(SessionSummary { usage_line, resume_command, @@ -495,6 +501,7 @@ async fn handle_model_migration_prompt_if_needed( return Some(AppExitInfo { token_usage: TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::UserRequested, }); @@ -1190,6 +1197,7 @@ impl App { Ok(AppExitInfo { token_usage: app.token_usage(), thread_id: app.chat_widget.thread_id(), + thread_name: app.chat_widget.thread_name(), update_action: app.pending_update_action, exit_reason, }) @@ -1251,8 +1259,11 @@ impl App { match event { AppEvent::NewSession => { let model = self.chat_widget.current_model().to_string(); - let summary = - session_summary(self.chat_widget.token_usage(), self.chat_widget.thread_id()); + let summary = session_summary( + self.chat_widget.token_usage(), + self.chat_widget.thread_id(), + self.chat_widget.thread_name(), + ); self.shutdown_current_thread().await; if let Err(err) = self.server.remove_and_close_all_threads().await { tracing::warn!(error = %err, "failed to close all threads"); @@ -1324,6 +1335,7 @@ impl App { let summary = session_summary( self.chat_widget.token_usage(), self.chat_widget.thread_id(), + self.chat_widget.thread_name(), ); match self .server @@ -1382,8 +1394,11 @@ impl App { tui.frame_requester().schedule_frame(); } AppEvent::ForkCurrentSession => { - let summary = - session_summary(self.chat_widget.token_usage(), self.chat_widget.thread_id()); + let summary = session_summary( + self.chat_widget.token_usage(), + self.chat_widget.thread_id(), + self.chat_widget.thread_name(), + ); if let Some(path) = self.chat_widget.rollout_path() { match self .server @@ -2222,6 +2237,7 @@ impl App { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: thread_id, forked_from_id: None, + thread_name: None, model: config_snapshot.model, model_provider_id: config_snapshot.model_provider_id, approval_policy: config_snapshot.approval_policy, @@ -2887,6 +2903,7 @@ mod tests { let event = SessionConfiguredEvent { session_id: ThreadId::new(), forked_from_id: None, + thread_name: None, model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -2939,6 +2956,7 @@ mod tests { msg: EventMsg::SessionConfigured(SessionConfiguredEvent { session_id: base_id, forked_from_id: None, + thread_name: None, model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -2984,6 +3002,7 @@ mod tests { let event = SessionConfiguredEvent { session_id: thread_id, forked_from_id: None, + thread_name: None, model: "gpt-test".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -3015,7 +3034,7 @@ mod tests { #[tokio::test] async fn session_summary_skip_zero_usage() { - assert!(session_summary(TokenUsage::default(), None).is_none()); + assert!(session_summary(TokenUsage::default(), None, None).is_none()); } #[tokio::test] @@ -3028,7 +3047,7 @@ mod tests { }; let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); - let summary = session_summary(usage, Some(conversation)).expect("summary"); + let summary = session_summary(usage, Some(conversation), None).expect("summary"); assert_eq!( summary.usage_line, "Token usage: total=12 input=10 output=2" @@ -3038,4 +3057,22 @@ mod tests { Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) ); } + + #[tokio::test] + async fn session_summary_prefers_name_over_id() { + let usage = TokenUsage { + input_tokens: 10, + output_tokens: 2, + total_tokens: 12, + ..Default::default() + }; + let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + + let summary = session_summary(usage, Some(conversation), Some("my-session".to_string())) + .expect("summary"); + assert_eq!( + summary.resume_command, + Some("codex resume my-session".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 90cae4b4a0..7c8b7585ac 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -2054,7 +2054,7 @@ impl ChatComposer { self.personality_command_enabled, self.windows_degraded_sandbox_active, ) - && cmd == SlashCommand::Review + && matches!(cmd, SlashCommand::Review | SlashCommand::Rename) { self.textarea.set_text_clearing_elements(""); return Some(InputResult::CommandWithArgs(cmd, rest.to_string())); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 72a3521014..0469814122 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -23,6 +23,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -195,7 +196,6 @@ use self::skills::collect_tool_mentions; use self::skills::find_app_mentions; use self::skills::find_skill_mentions_with_tool_mentions; use crate::streaming::controller::StreamController; -use std::path::Path; use chrono::Local; use codex_common::approval_presets::ApprovalPreset; @@ -223,7 +223,9 @@ struct RunningCommand { struct UnifiedExecProcessSummary { key: String, + call_id: String, command_display: String, + recent_chunks: Vec, } struct UnifiedExecWaitState { @@ -512,6 +514,7 @@ pub(crate) struct ChatWidget { // Previous status header to restore after a transient stream retry. retry_status_header: Option, thread_id: Option, + thread_name: Option, forked_from: Option, frame_requester: FrameRequester, // Whether to include the initial welcome banner on session configured @@ -782,6 +785,7 @@ impl ChatWidget { self.set_skills(None); self.bottom_pane.set_connectors_snapshot(None); self.thread_id = Some(event.session_id); + self.thread_name = event.thread_name.clone(); self.forked_from = event.forked_from_id; self.current_rollout_path = event.rollout_path.clone(); let initial_messages = event.initial_messages.clone(); @@ -822,6 +826,13 @@ impl ChatWidget { } } + fn on_thread_name_updated(&mut self, event: codex_core::protocol::ThreadNameUpdatedEvent) { + if self.thread_id == Some(event.thread_id) { + self.thread_name = event.thread_name; + self.request_redraw(); + } + } + fn set_skills(&mut self, skills: Option>) { self.bottom_pane.set_skills(skills); } @@ -1421,6 +1432,8 @@ impl ChatWidget { } fn on_exec_command_output_delta(&mut self, ev: ExecCommandOutputDeltaEvent) { + self.track_unified_exec_output_chunk(&ev.call_id, &ev.chunk); + let Some(cell) = self .active_cell .as_mut() @@ -1540,11 +1553,15 @@ impl ChatWidget { .iter_mut() .find(|process| process.key == key) { + existing.call_id = ev.call_id.clone(); existing.command_display = command_display; + existing.recent_chunks.clear(); } else { self.unified_exec_processes.push(UnifiedExecProcessSummary { key, + call_id: ev.call_id.clone(), command_display, + recent_chunks: Vec::new(), }); } self.sync_unified_exec_footer(); @@ -1569,6 +1586,32 @@ impl ChatWidget { self.bottom_pane.set_unified_exec_processes(processes); } + /// Record recent stdout/stderr lines for the unified exec footer. + fn track_unified_exec_output_chunk(&mut self, call_id: &str, chunk: &[u8]) { + let Some(process) = self + .unified_exec_processes + .iter_mut() + .find(|process| process.call_id == call_id) + else { + return; + }; + + let text = String::from_utf8_lossy(chunk); + for line in text + .lines() + .map(str::trim_end) + .filter(|line| !line.is_empty()) + { + process.recent_chunks.push(line.to_string()); + } + + const MAX_RECENT_CHUNKS: usize = 3; + if process.recent_chunks.len() > MAX_RECENT_CHUNKS { + let drop_count = process.recent_chunks.len() - MAX_RECENT_CHUNKS; + process.recent_chunks.drain(0..drop_count); + } + } + fn clear_unified_exec_processes(&mut self) { if self.unified_exec_processes.is_empty() { return; @@ -2127,6 +2170,7 @@ impl ChatWidget { current_status_header: String::from("Working"), retry_status_header: None, thread_id: None, + thread_name: None, forked_from: None, queued_user_messages: VecDeque::new(), show_welcome_banner: is_first_run, @@ -2265,6 +2309,7 @@ impl ChatWidget { current_status_header: String::from("Working"), retry_status_header: None, thread_id: None, + thread_name: None, forked_from: None, saw_plan_update_this_turn: false, queued_user_messages: VecDeque::new(), @@ -2392,6 +2437,7 @@ impl ChatWidget { current_status_header: String::from("Working"), retry_status_header: None, thread_id: None, + thread_name: None, forked_from: None, queued_user_messages: VecDeque::new(), show_welcome_banner: false, @@ -2661,6 +2707,9 @@ impl ChatWidget { SlashCommand::Review => { self.open_review_popup(); } + SlashCommand::Rename => { + self.show_rename_prompt(); + } SlashCommand::Model => { self.open_model_popup(); } @@ -2862,6 +2911,17 @@ impl ChatWidget { let trimmed = args.trim(); match cmd { + SlashCommand::Rename if !trimmed.is_empty() => { + let Some(name) = codex_core::util::normalize_thread_name(trimmed) else { + self.add_error_message("Thread name cannot be empty.".to_string()); + return; + }; + let cell = Self::rename_confirmation_cell(&name, self.thread_id); + self.add_boxed_history(Box::new(cell)); + self.request_redraw(); + self.app_event_tx + .send(AppEvent::CodexOp(Op::SetThreadName { name })); + } SlashCommand::Collab | SlashCommand::Plan => { let _ = trimmed; self.dispatch_command(cmd); @@ -2880,6 +2940,38 @@ impl ChatWidget { } } + fn show_rename_prompt(&mut self) { + let tx = self.app_event_tx.clone(); + let has_name = self + .thread_name + .as_ref() + .is_some_and(|name| !name.is_empty()); + let title = if has_name { + "Rename thread" + } else { + "Name thread" + }; + let thread_id = self.thread_id; + let view = CustomPromptView::new( + title.to_string(), + "Type a name and press Enter".to_string(), + None, + Box::new(move |name: String| { + let Some(name) = codex_core::util::normalize_thread_name(&name) else { + tx.send(AppEvent::InsertHistoryCell(Box::new( + history_cell::new_error_event("Thread name cannot be empty.".to_string()), + ))); + return; + }; + let cell = Self::rename_confirmation_cell(&name, thread_id); + tx.send(AppEvent::InsertHistoryCell(Box::new(cell))); + tx.send(AppEvent::CodexOp(Op::SetThreadName { name })); + }), + ); + + self.bottom_pane.show_view(Box::new(view)); + } + pub(crate) fn handle_paste(&mut self, text: String) { self.bottom_pane.handle_paste(text); } @@ -3080,7 +3172,10 @@ impl ChatWidget { /// distinguish replayed events from live ones. fn replay_initial_messages(&mut self, events: Vec) { for msg in events { - if matches!(msg, EventMsg::SessionConfigured(_)) { + if matches!( + msg, + EventMsg::SessionConfigured(_) | EventMsg::ThreadNameUpdated(_) + ) { continue; } // `id: None` indicates a synthetic/fake id coming from replay. @@ -3124,6 +3219,7 @@ impl ChatWidget { match msg { EventMsg::SessionConfigured(e) => self.on_session_configured(e), + EventMsg::ThreadNameUpdated(e) => self.on_thread_name_updated(e), EventMsg::AgentMessage(AgentMessageEvent { message }) => self.on_agent_message(message), EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { delta }) => { self.on_agent_message_delta(delta) @@ -3423,6 +3519,7 @@ impl ChatWidget { token_info, total_usage, &self.thread_id, + self.thread_name.clone(), self.forked_from, self.rate_limit_snapshot.as_ref(), self.plan_type, @@ -3437,7 +3534,10 @@ impl ChatWidget { let processes = self .unified_exec_processes .iter() - .map(|process| process.command_display.clone()) + .map(|process| history_cell::UnifiedExecProcessDetails { + command_display: process.command_display.clone(), + recent_chunks: process.recent_chunks.clone(), + }) .collect(); self.add_to_history(history_cell::new_unified_exec_processes_output(processes)); } @@ -5229,6 +5329,20 @@ impl ChatWidget { self.request_redraw(); } + fn rename_confirmation_cell(name: &str, thread_id: Option) -> PlainHistoryCell { + let resume_cmd = codex_core::util::resume_command(Some(name), thread_id) + .unwrap_or_else(|| format!("codex resume {name}")); + let name = name.to_string(); + let line = vec![ + "• ".into(), + "Thread renamed to ".into(), + name.cyan(), + ", to resume this thread run ".into(), + resume_cmd.cyan(), + ]; + PlainHistoryCell::new(vec![line.into()]) + } + pub(crate) fn add_mcp_output(&mut self) { if self.config.mcp_servers.is_empty() { self.add_to_history(history_cell::empty_mcp_output()); @@ -5750,6 +5864,9 @@ impl ChatWidget { self.thread_id } + pub(crate) fn thread_name(&self) -> Option { + self.thread_name.clone() + } pub(crate) fn rollout_path(&self) -> Option { self.current_rollout_path.clone() } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index d8f22dc8e3..fef941660a 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -133,6 +133,7 @@ async fn resumed_initial_messages_render_history() { let configured = codex_core::protocol::SessionConfiguredEvent { session_id: conversation_id, forked_from_id: None, + thread_name: None, model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -199,6 +200,7 @@ async fn replayed_user_message_preserves_text_elements_and_local_images() { let configured = codex_core::protocol::SessionConfiguredEvent { session_id: conversation_id, forked_from_id: None, + thread_name: None, model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -251,6 +253,7 @@ async fn submission_preserves_text_elements_and_local_images() { let configured = codex_core::protocol::SessionConfiguredEvent { session_id: conversation_id, forked_from_id: None, + thread_name: None, model: "test-model".to_string(), model_provider_id: "test-provider".to_string(), approval_policy: AskForApproval::Never, @@ -821,6 +824,7 @@ async fn make_chatwidget_manual( current_status_header: String::from("Working"), retry_status_header: None, thread_id: None, + thread_name: None, forked_from: None, frame_requester: FrameRequester::test_dummy(), show_welcome_banner: true, @@ -2022,7 +2026,9 @@ async fn unified_exec_wait_status_header_updates_on_late_command_display() { chat.on_task_started(); chat.unified_exec_processes.push(UnifiedExecProcessSummary { key: "proc-1".to_string(), + call_id: "call-1".to_string(), command_display: "sleep 5".to_string(), + recent_chunks: Vec::new(), }); chat.on_terminal_interaction(TerminalInteractionEvent { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 687d18fa6a..1f19c929a2 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -558,15 +558,21 @@ pub(crate) fn new_unified_exec_interaction( #[derive(Debug)] struct UnifiedExecProcessesCell { - processes: Vec, + processes: Vec, } impl UnifiedExecProcessesCell { - fn new(processes: Vec) -> Self { + fn new(processes: Vec) -> Self { Self { processes } } } +#[derive(Debug, Clone)] +pub(crate) struct UnifiedExecProcessDetails { + pub(crate) command_display: String, + pub(crate) recent_chunks: Vec, +} + impl HistoryCell for UnifiedExecProcessesCell { fn display_lines(&self, width: u16) -> Vec> { if width == 0 { @@ -589,10 +595,11 @@ impl HistoryCell for UnifiedExecProcessesCell { let truncation_suffix = " [...]"; let truncation_suffix_width = UnicodeWidthStr::width(truncation_suffix); let mut shown = 0usize; - for command in &self.processes { + for process in &self.processes { if shown >= max_processes { break; } + let command = &process.command_display; let (snippet, snippet_truncated) = { let (first_line, has_more_lines) = match command.split_once('\n') { Some((first, _)) => (first, true), @@ -627,6 +634,32 @@ impl HistoryCell for UnifiedExecProcessesCell { let (truncated, _, _) = take_prefix_by_width(&snippet, budget); out.push(vec![prefix.dim(), truncated.cyan()].into()); } + + let chunk_prefix_first = " ↳ "; + let chunk_prefix_next = " "; + for (idx, chunk) in process.recent_chunks.iter().enumerate() { + let chunk_prefix = if idx == 0 { + chunk_prefix_first + } else { + chunk_prefix_next + }; + let chunk_prefix_width = UnicodeWidthStr::width(chunk_prefix); + if wrap_width <= chunk_prefix_width { + out.push(Line::from(chunk_prefix.dim())); + continue; + } + let budget = wrap_width.saturating_sub(chunk_prefix_width); + let (truncated, remainder, _) = take_prefix_by_width(chunk, budget); + if !remainder.is_empty() && budget > truncation_suffix_width { + let available = budget.saturating_sub(truncation_suffix_width); + let (shorter, _, _) = take_prefix_by_width(chunk, available); + out.push( + vec![chunk_prefix.dim(), shorter.dim(), truncation_suffix.dim()].into(), + ); + } else { + out.push(vec![chunk_prefix.dim(), truncated.dim()].into()); + } + } shown += 1; } @@ -650,7 +683,9 @@ impl HistoryCell for UnifiedExecProcessesCell { } } -pub(crate) fn new_unified_exec_processes_output(processes: Vec) -> CompositeHistoryCell { +pub(crate) fn new_unified_exec_processes_output( + processes: Vec, +) -> CompositeHistoryCell { let command = PlainHistoryCell::new(vec!["/ps".magenta().into()]); let summary = UnifiedExecProcessesCell::new(processes); CompositeHistoryCell::new(vec![Box::new(command), Box::new(summary)]) @@ -2022,8 +2057,14 @@ mod tests { #[test] fn ps_output_multiline_snapshot() { let cell = new_unified_exec_processes_output(vec![ - "echo hello\nand then some extra text".to_string(), - "rg \"foo\" src".to_string(), + UnifiedExecProcessDetails { + command_display: "echo hello\nand then some extra text".to_string(), + recent_chunks: vec!["hello".to_string(), "done".to_string()], + }, + UnifiedExecProcessDetails { + command_display: "rg \"foo\" src".to_string(), + recent_chunks: vec!["src/main.rs:12:foo".to_string()], + }, ]); let rendered = render_lines(&cell.display_lines(40)).join("\n"); insta::assert_snapshot!(rendered); @@ -2031,9 +2072,12 @@ mod tests { #[test] fn ps_output_long_command_snapshot() { - let cell = new_unified_exec_processes_output(vec![String::from( - "rg \"foo\" src --glob '**/*.rs' --max-count 1000 --no-ignore --hidden --follow --glob '!target/**'", - )]); + let cell = new_unified_exec_processes_output(vec![UnifiedExecProcessDetails { + command_display: String::from( + "rg \"foo\" src --glob '**/*.rs' --max-count 1000 --no-ignore --hidden --follow --glob '!target/**'", + ), + recent_chunks: vec!["searching...".to_string()], + }]); let rendered = render_lines(&cell.display_lines(36)).join("\n"); insta::assert_snapshot!(rendered); } @@ -2041,12 +2085,30 @@ mod tests { #[test] fn ps_output_many_sessions_snapshot() { let cell = new_unified_exec_processes_output( - (0..20).map(|idx| format!("command {idx}")).collect(), + (0..20) + .map(|idx| UnifiedExecProcessDetails { + command_display: format!("command {idx}"), + recent_chunks: Vec::new(), + }) + .collect(), ); let rendered = render_lines(&cell.display_lines(32)).join("\n"); insta::assert_snapshot!(rendered); } + #[test] + fn ps_output_chunk_leading_whitespace_snapshot() { + let cell = new_unified_exec_processes_output(vec![UnifiedExecProcessDetails { + command_display: "just fix".to_string(), + recent_chunks: vec![ + " indented first".to_string(), + " more indented".to_string(), + ], + }]); + let rendered = render_lines(&cell.display_lines(60)).join("\n"); + insta::assert_snapshot!(rendered); + } + #[tokio::test] async fn mcp_tools_output_masks_sensitive_values() { let mut config = test_config().await; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 22c2d6db54..1f34ed6049 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -8,6 +8,7 @@ use app::App; pub use app::AppExitInfo; pub use app::ExitReason; use codex_app_server_protocol::AuthMode; +use codex_cloud_requirements::cloud_requirements_loader; use codex_common::oss::ensure_oss_provider_ready; use codex_common::oss::get_default_model_for_oss_provider; use codex_common::oss::ollama_chat_deprecation_notice; @@ -23,9 +24,11 @@ use codex_core::config::ConfigOverrides; use codex_core::config::find_codex_home; use codex_core::config::load_config_as_toml_with_cli_overrides; use codex_core::config::resolve_oss_provider; +use codex_core::config_loader::CloudRequirementsLoader; use codex_core::config_loader::ConfigLoadError; use codex_core::config_loader::format_config_error_with_source; use codex_core::find_thread_path_by_id_str; +use codex_core::find_thread_path_by_name_str; use codex_core::path_utils; use codex_core::protocol::AskForApproval; use codex_core::read_session_meta_line; @@ -47,6 +50,7 @@ use tracing::error; use tracing_appender::non_blocking; use tracing_subscriber::EnvFilter; use tracing_subscriber::prelude::*; +use uuid::Uuid; mod additional_dirs; mod app; @@ -204,6 +208,17 @@ pub async fn run_main( } }; + let cloud_auth_manager = AuthManager::shared( + codex_home.to_path_buf(), + false, + config_toml.cli_auth_credentials_store.unwrap_or_default(), + ); + let chatgpt_base_url = config_toml + .chatgpt_base_url + .clone() + .unwrap_or_else(|| "https://chatgpt.com/backend-api/".to_string()); + let cloud_requirements = cloud_requirements_loader(cloud_auth_manager, chatgpt_base_url); + let model_provider_override = if cli.oss { let resolved = resolve_oss_provider( cli.oss_provider.as_deref(), @@ -255,7 +270,12 @@ pub async fn run_main( ..Default::default() }; - let config = load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await; + let config = load_config_or_exit( + cli_kv_overrides.clone(), + overrides.clone(), + cloud_requirements.clone(), + ) + .await; if let Some(warning) = add_dir_warning_message(&cli.add_dir, config.sandbox_policy.get()) { #[allow(clippy::print_stderr)] @@ -368,9 +388,16 @@ pub async fn run_main( .with(otel_tracing_layer) .try_init(); - run_ratatui_app(cli, config, overrides, cli_kv_overrides, feedback) - .await - .map_err(|err| std::io::Error::other(err.to_string())) + run_ratatui_app( + cli, + config, + overrides, + cli_kv_overrides, + cloud_requirements, + feedback, + ) + .await + .map_err(|err| std::io::Error::other(err.to_string())) } async fn run_ratatui_app( @@ -378,6 +405,7 @@ async fn run_ratatui_app( initial_config: Config, overrides: ConfigOverrides, cli_kv_overrides: Vec<(String, toml::Value)>, + cloud_requirements: CloudRequirementsLoader, feedback: codex_feedback::CodexFeedback, ) -> color_eyre::Result { color_eyre::install()?; @@ -411,6 +439,7 @@ async fn run_ratatui_app( return Ok(AppExitInfo { token_usage: codex_core::protocol::TokenUsage::default(), thread_id: None, + thread_name: None, update_action: Some(action), exit_reason: ExitReason::UserRequested, }); @@ -451,6 +480,7 @@ async fn run_ratatui_app( return Ok(AppExitInfo { token_usage: codex_core::protocol::TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::UserRequested, }); @@ -461,7 +491,12 @@ async fn run_ratatui_app( .map(|d| d == TrustDirectorySelection::Trust) .unwrap_or(false) { - load_config_or_exit(cli_kv_overrides.clone(), overrides.clone()).await + load_config_or_exit( + cli_kv_overrides.clone(), + overrides.clone(), + cloud_requirements.clone(), + ) + .await } else { initial_config } @@ -484,6 +519,7 @@ async fn run_ratatui_app( Ok(AppExitInfo { token_usage: codex_core::protocol::TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::Fatal(format!( "No saved session found with ID {id_str}. Run `codex {action}` without an ID to choose from existing sessions." @@ -494,7 +530,13 @@ async fn run_ratatui_app( let use_fork = cli.fork_picker || cli.fork_last || cli.fork_session_id.is_some(); let session_selection = if use_fork { if let Some(id_str) = cli.fork_session_id.as_deref() { - match find_thread_path_by_id_str(&config.codex_home, id_str).await? { + let is_uuid = Uuid::parse_str(id_str).is_ok(); + let path = if is_uuid { + find_thread_path_by_id_str(&config.codex_home, id_str).await? + } else { + find_thread_path_by_name_str(&config.codex_home, id_str).await? + }; + match path { Some(path) => resume_picker::SessionSelection::Fork(path), None => return missing_session_exit(id_str, "fork"), } @@ -533,6 +575,7 @@ async fn run_ratatui_app( return Ok(AppExitInfo { token_usage: codex_core::protocol::TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::UserRequested, }); @@ -543,7 +586,13 @@ async fn run_ratatui_app( resume_picker::SessionSelection::StartFresh } } else if let Some(id_str) = cli.resume_session_id.as_deref() { - match find_thread_path_by_id_str(&config.codex_home, id_str).await? { + let is_uuid = Uuid::parse_str(id_str).is_ok(); + let path = if is_uuid { + find_thread_path_by_id_str(&config.codex_home, id_str).await? + } else { + find_thread_path_by_name_str(&config.codex_home, id_str).await? + }; + match path { Some(path) => resume_picker::SessionSelection::Resume(path), None => return missing_session_exit(id_str, "resume"), } @@ -584,6 +633,7 @@ async fn run_ratatui_app( return Ok(AppExitInfo { token_usage: codex_core::protocol::TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::UserRequested, }); @@ -614,6 +664,7 @@ async fn run_ratatui_app( load_config_or_exit_with_fallback_cwd( cli_kv_overrides.clone(), overrides.clone(), + cloud_requirements.clone(), fallback_cwd, ) .await @@ -797,19 +848,23 @@ fn get_login_status(config: &Config) -> LoginStatus { async fn load_config_or_exit( cli_kv_overrides: Vec<(String, toml::Value)>, overrides: ConfigOverrides, + cloud_requirements: CloudRequirementsLoader, ) -> Config { - load_config_or_exit_with_fallback_cwd(cli_kv_overrides, overrides, None).await + load_config_or_exit_with_fallback_cwd(cli_kv_overrides, overrides, cloud_requirements, None) + .await } async fn load_config_or_exit_with_fallback_cwd( cli_kv_overrides: Vec<(String, toml::Value)>, overrides: ConfigOverrides, + cloud_requirements: CloudRequirementsLoader, fallback_cwd: Option, ) -> Config { #[allow(clippy::print_stderr)] match ConfigBuilder::default() .cli_overrides(cli_kv_overrides) .harness_overrides(overrides) + .cloud_requirements(cloud_requirements) .fallback_cwd(fallback_cwd) .build() .await diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 9488577c1b..4b68645aef 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -20,6 +20,7 @@ pub enum SlashCommand { Experimental, Skills, Review, + Rename, New, Resume, Fork, @@ -53,6 +54,7 @@ impl SlashCommand { SlashCommand::Init => "create an AGENTS.md file with instructions for Codex", SlashCommand::Compact => "summarize conversation to prevent hitting the context limit", SlashCommand::Review => "review my current changes and find issues", + SlashCommand::Rename => "rename the current thread", SlashCommand::Resume => "resume a saved chat", SlashCommand::Fork => "fork the current chat", // SlashCommand::Undo => "ask Codex to undo a turn", @@ -103,6 +105,7 @@ impl SlashCommand { | SlashCommand::Review | SlashCommand::Logout => false, SlashCommand::Diff + | SlashCommand::Rename | SlashCommand::Mention | SlashCommand::Skills | SlashCommand::Status diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_chunk_leading_whitespace_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_chunk_leading_whitespace_snapshot.snap new file mode 100644 index 0000000000..e13dcf0a1e --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_chunk_leading_whitespace_snapshot.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +/ps + +Background terminals + + • just fix + ↳ indented first + more indented diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_long_command_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_long_command_snapshot.snap index b9302295d3..2bd4bf9fb9 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_long_command_snapshot.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_long_command_snapshot.snap @@ -7,3 +7,4 @@ expression: rendered Background terminals • rg "foo" src --glob '**/*. [...] + ↳ searching... diff --git a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_multiline_snapshot.snap b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_multiline_snapshot.snap index c073349e8f..0e31eddfa0 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_multiline_snapshot.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_multiline_snapshot.snap @@ -7,4 +7,7 @@ expression: rendered Background terminals • echo hello [...] + ↳ hello + done • rg "foo" src + ↳ src/main.rs:12:foo diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index 5e2fe57b8a..23005274db 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -68,6 +68,7 @@ struct StatusHistoryCell { collaboration_mode: Option, model_provider: Option, account: Option, + thread_name: Option, session_id: Option, forked_from: Option, token_usage: StatusTokenUsageData, @@ -81,6 +82,7 @@ pub(crate) fn new_status_output( token_info: Option<&TokenUsageInfo>, total_usage: &TokenUsage, session_id: &Option, + thread_name: Option, forked_from: Option, rate_limits: Option<&RateLimitSnapshotDisplay>, plan_type: Option, @@ -96,6 +98,7 @@ pub(crate) fn new_status_output( token_info, total_usage, session_id, + thread_name, forked_from, rate_limits, plan_type, @@ -116,6 +119,7 @@ impl StatusHistoryCell { token_info: Option<&TokenUsageInfo>, total_usage: &TokenUsage, session_id: &Option, + thread_name: Option, forked_from: Option, rate_limits: Option<&RateLimitSnapshotDisplay>, plan_type: Option, @@ -197,6 +201,7 @@ impl StatusHistoryCell { collaboration_mode: collaboration_mode.map(ToString::to_string), model_provider, account, + thread_name, session_id, forked_from, token_usage, @@ -377,6 +382,7 @@ impl HistoryCell for StatusHistoryCell { .map(str::to_string) .collect(); let mut seen: BTreeSet = labels.iter().cloned().collect(); + let thread_name = self.thread_name.as_deref().filter(|name| !name.is_empty()); if self.model_provider.is_some() { push_label(&mut labels, &mut seen, "Model provider"); @@ -384,6 +390,9 @@ impl HistoryCell for StatusHistoryCell { if account_value.is_some() { push_label(&mut labels, &mut seen, "Account"); } + if thread_name.is_some() { + push_label(&mut labels, &mut seen, "Thread name"); + } if self.session_id.is_some() { push_label(&mut labels, &mut seen, "Session"); } @@ -442,10 +451,12 @@ impl HistoryCell for StatusHistoryCell { lines.push(formatter.line("Account", vec![Span::from(account_value)])); } + if let Some(thread_name) = thread_name { + lines.push(formatter.line("Thread name", vec![Span::from(thread_name.to_string())])); + } if let Some(collab_mode) = self.collaboration_mode.as_ref() { lines.push(formatter.line("Collaboration mode", vec![Span::from(collab_mode.clone())])); } - if let Some(session) = self.session_id.as_ref() { lines.push(formatter.line("Session", vec![Span::from(session.clone())])); } diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index 92ed340817..c8844904ab 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -148,6 +148,7 @@ async fn status_snapshot_includes_reasoning_details() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -200,6 +201,7 @@ async fn status_snapshot_includes_forked_from() { Some(&token_info), &usage, &Some(session_id), + None, Some(forked_from), None, None, @@ -260,6 +262,7 @@ async fn status_snapshot_includes_monthly_limit() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -307,6 +310,7 @@ async fn status_snapshot_shows_unlimited_credits() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -353,6 +357,7 @@ async fn status_snapshot_shows_positive_credits() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -399,6 +404,7 @@ async fn status_snapshot_hides_zero_credits() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -443,6 +449,7 @@ async fn status_snapshot_hides_when_has_no_credits_flag() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -489,6 +496,7 @@ async fn status_card_token_usage_excludes_cached_tokens() { None, None, None, + None, now, &model_slug, None, @@ -546,6 +554,7 @@ async fn status_snapshot_truncates_in_narrow_terminal() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -596,6 +605,7 @@ async fn status_snapshot_shows_missing_limits_message() { None, None, None, + None, now, &model_slug, None, @@ -660,6 +670,7 @@ async fn status_snapshot_includes_credits_and_limits() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -714,6 +725,7 @@ async fn status_snapshot_shows_empty_limits_message() { &usage, &None, None, + None, Some(&rate_display), None, captured_at, @@ -777,6 +789,7 @@ async fn status_snapshot_shows_stale_limits_message() { &usage, &None, None, + None, Some(&rate_display), None, now, @@ -844,6 +857,7 @@ async fn status_snapshot_cached_limits_hide_credits_without_flag() { &usage, &None, None, + None, Some(&rate_display), None, now, @@ -903,6 +917,7 @@ async fn status_context_window_uses_last_usage() { None, None, None, + None, now, &model_slug, None, diff --git a/codex-rs/tui/tooltips.txt b/codex-rs/tui/tooltips.txt index 455ae5cd03..a8521f359f 100644 --- a/codex-rs/tui/tooltips.txt +++ b/codex-rs/tui/tooltips.txt @@ -9,7 +9,10 @@ Use /status to see the current model, approvals, and token usage. Use /fork to branch the current chat into a new thread. Use /init to create an AGENTS.md with project-specific guidance. Use /mcp to list configured MCP tools. +Use /rename to rename your threads for easier thread resuming. Use the OpenAI docs MCP for API questions; enable it with `codex mcp add openaiDeveloperDocs --url https://developers.openai.com/mcp`. +Join the OpenAI community Discord: http://discord.gg/openai +Visit the Codex community forum: https://community.openai.com/c/codex/37 You can run any shell command from Codex using `!` (e.g. `!ls`) Type / to open the command popup; Tab autocompletes slash commands. When the composer is empty, press Esc to step back and edit your last message; Enter confirms.