From e3ab0bd9739106b039ebffe040914d4aac0c6de6 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Fri, 30 Jan 2026 00:10:12 -0700 Subject: [PATCH 01/18] chore(personality) new schema with fallbacks (#10147) ## Summary Let's dial in this api contract in a bit more with more robust fallback behavior when model_instructions_template is false. Switches to a more explicit template / variables structure, with more fallbacks. ## Testing - [x] Adding unit tests - [x] Tested locally --- .../app-server/tests/common/models_cache.rs | 2 +- .../tests/suite/v2/thread_resume.rs | 13 +- .../app-server/tests/suite/v2/turn_start.rs | 9 +- .../codex-api/tests/models_integration.rs | 2 +- codex-rs/core/src/codex.rs | 52 ++- .../core/src/models_manager/model_info.rs | 55 +-- .../gpt-5.2-codex_instructions_template.md | 4 +- ...{friendly.md => gpt-5.2-codex_friendly.md} | 0 ...ragmatic.md => gpt-5.2-codex_pragmatic.md} | 1 + codex-rs/core/tests/suite/models_cache_ttl.rs | 2 +- codex-rs/core/tests/suite/personality.rs | 360 +++++++++++++++++- codex-rs/core/tests/suite/remote_models.rs | 6 +- codex-rs/protocol/src/openai_models.rs | 216 +++++++++-- 13 files changed, 607 insertions(+), 115 deletions(-) rename codex-rs/core/templates/personalities/{friendly.md => gpt-5.2-codex_friendly.md} (100%) rename codex-rs/core/templates/personalities/{pragmatic.md => gpt-5.2-codex_pragmatic.md} (99%) 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/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index af746b37b7..6ad0a14b30 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -31,7 +31,7 @@ use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); -const DEFAULT_BASE_INSTRUCTIONS: &str = "You are Codex, based on GPT-5. You are running as a coding agent in the Codex CLI on a user's computer."; +const CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT: &str = "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."; #[tokio::test] async fn thread_resume_returns_original_thread() -> 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/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/codex.rs b/codex-rs/core/src/codex.rs index 1fb5572183..250ea1ff49 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -1285,27 +1285,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( @@ -1845,15 +1849,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 { 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/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/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/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); + } } From 34f89b12d038d330f1e8306dbe9e1727b32f09c0 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Thu, 29 Jan 2026 23:40:32 -0800 Subject: [PATCH 02/18] MCP tool call approval (simplified version) (#10200) Add elicitation approval request for MCP tool call requests. --- codex-rs/chatgpt/src/connectors.rs | 6 +- codex-rs/core/src/mcp_connection_manager.rs | 49 ++- codex-rs/core/src/mcp_tool_call.rs | 317 +++++++++++++++++++- codex-rs/core/src/tools/handlers/mcp.rs | 3 +- 4 files changed, 364 insertions(+), 11 deletions(-) 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/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/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, From a270a28a063ec3989c7421d1db1d200ac3e5faa5 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 09:00:44 +0100 Subject: [PATCH 03/18] feat: add output to `/ps` (#10154) Screenshot 2026-01-29 at 13 24 57 --- codex-rs/tui/src/chatwidget.rs | 39 ++++++++- codex-rs/tui/src/chatwidget/tests.rs | 2 + codex-rs/tui/src/history_cell.rs | 82 ++++++++++++++++--- ...put_chunk_leading_whitespace_snapshot.snap | 11 +++ ...ests__ps_output_long_command_snapshot.snap | 1 + ...__tests__ps_output_multiline_snapshot.snap | 3 + 6 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__history_cell__tests__ps_output_chunk_leading_whitespace_snapshot.snap diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 2833d4863e..9857956ed9 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -225,7 +225,9 @@ struct RunningCommand { struct UnifiedExecProcessSummary { key: String, + call_id: String, command_display: String, + recent_chunks: Vec, } struct UnifiedExecWaitState { @@ -1426,6 +1428,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() @@ -1545,11 +1549,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(); @@ -1574,6 +1582,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; @@ -3448,7 +3482,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)); } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a0e8269b58..144fc199c5 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2025,7 +2025,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/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 From f8056e62d4ee6063af909dc956f67ec509e0c3cd Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 10:02:46 +0100 Subject: [PATCH 04/18] nit: actually run tests (#10217) --- codex-rs/core/tests/suite/mod.rs | 1 + 1 file changed, 1 insertion(+) 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; From c0cad80668d8bc94085d8a59da31255d3550c16c Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Fri, 30 Jan 2026 01:14:15 -0800 Subject: [PATCH 05/18] Add community links to startup tooltips (#10177) ## Summary - add startup tooltip for OpenAI community Discord - add startup tooltip for Codex community forum ## Testing - not run (text-only tooltip change) --- codex-rs/tui/tooltips.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/codex-rs/tui/tooltips.txt b/codex-rs/tui/tooltips.txt index 455ae5cd03..84a96f54d0 100644 --- a/codex-rs/tui/tooltips.txt +++ b/codex-rs/tui/tooltips.txt @@ -10,6 +10,8 @@ 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 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. From a0ccef9d5c44b7517243163ceb64be8223d5d8d5 Mon Sep 17 00:00:00 2001 From: Shijie Rao Date: Fri, 30 Jan 2026 01:19:24 -0800 Subject: [PATCH 06/18] Chore: plan mode do not include free form question and always include isOther (#10210) We should never ask a freeform question when planning and we should always include isOther as an escape hatch. --- codex-rs/app-server-protocol/src/protocol/v2.rs | 2 +- codex-rs/app-server/tests/common/responses.rs | 1 - .../core/src/tools/handlers/request_user_input.rs | 14 +++++++++++++- codex-rs/core/src/tools/spec.rs | 13 ++----------- codex-rs/core/tests/suite/request_user_input.rs | 3 +-- codex-rs/docs/protocol_v1.md | 2 +- 6 files changed, 18 insertions(+), 17 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index ac3df37bd0..1e28b1b086 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -2593,7 +2593,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/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 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/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/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 From 129787493f47af804ce58716bac5f64e145096c5 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 10:19:41 +0100 Subject: [PATCH 07/18] feat: backfill timing metric (#10218) 1. Add a metric to measure the backfill time 2. Add a unit to the timing histogram --- codex-rs/core/src/rollout/metadata.rs | 12 ++++++++++++ codex-rs/otel/src/metrics/client.rs | 25 ++++++++++++++++++++++++- codex-rs/otel/tests/suite/timing.rs | 11 ++++++++++- codex-rs/state/src/lib.rs | 2 ++ 4 files changed, 48 insertions(+), 2 deletions(-) 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/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/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"; From 25ad414680df9b7b53c8bf968c660e5a7e208123 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 11:13:43 +0100 Subject: [PATCH 08/18] chore: unify metric (#10220) --- codex-rs/core/src/state_db.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/state_db.rs b/codex-rs/core/src/state_db.rs index 7990a017ef..0804273bba 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; @@ -282,9 +283,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)], ); From 1ef5455eb6d3f887f6676c451397294594390087 Mon Sep 17 00:00:00 2001 From: pap-openai Date: Fri, 30 Jan 2026 10:40:09 +0000 Subject: [PATCH 09/18] Conversation naming (#8991) Session renaming: - `/rename my_session` - `/rename` without arg and passing an argument in `customViewPrompt` - AppExitInfo shows resume hint using the session name if set instead of uuid, defaults to uuid if not set - Names are stored in `CODEX_HOME/sessions.jsonl` Session resuming: - codex resume lookup for `CODEX_HOME/sessions.jsonl` first entry matching the name and resumes the session --------- Co-authored-by: jif-oai --- .../src/protocol/common.rs | 5 + .../app-server-protocol/src/protocol/v2.rs | 23 ++ codex-rs/app-server/README.md | 1 + .../app-server/src/bespoke_event_handling.rs | 12 + .../app-server/src/codex_message_processor.rs | 35 ++ codex-rs/cli/src/main.rs | 36 +- codex-rs/core/src/codex.rs | 111 +++++- codex-rs/core/src/codex_delegate.rs | 4 + codex-rs/core/src/lib.rs | 2 + codex-rs/core/src/rollout/mod.rs | 2 + codex-rs/core/src/rollout/policy.rs | 1 + codex-rs/core/src/rollout/session_index.rs | 325 ++++++++++++++++++ codex-rs/core/src/util.rs | 76 ++++ .../core/tests/suite/rollout_list_find.rs | 56 +++ codex-rs/exec/Cargo.toml | 1 + codex-rs/exec/src/cli.rs | 4 +- .../src/event_processor_with_human_output.rs | 3 +- .../src/event_processor_with_jsonl_output.rs | 1 + codex-rs/exec/src/lib.rs | 11 +- .../tests/event_processor_with_json_output.rs | 1 + codex-rs/mcp-server/src/codex_tool_runner.rs | 3 + codex-rs/mcp-server/src/outgoing_message.rs | 3 + codex-rs/protocol/src/protocol.rs | 23 +- codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/app.rs | 53 ++- codex-rs/tui/src/bottom_pane/chat_composer.rs | 2 +- codex-rs/tui/src/chatwidget.rs | 84 ++++- codex-rs/tui/src/chatwidget/tests.rs | 4 + codex-rs/tui/src/lib.rs | 23 +- codex-rs/tui/src/slash_command.rs | 3 + codex-rs/tui/src/status/card.rs | 13 +- codex-rs/tui/src/status/tests.rs | 15 + codex-rs/tui/tooltips.txt | 1 + 33 files changed, 908 insertions(+), 30 deletions(-) create mode 100644 codex-rs/core/src/rollout/session_index.rs diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index b9c58a5883..2e8778bdc5 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -132,6 +132,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, @@ -598,6 +602,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 1e28b1b086..8bea094487 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -1295,6 +1295,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/")] @@ -1302,6 +1310,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/")] @@ -2285,6 +2298,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/")] 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 660b4b87d0..3817bce3a1 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; @@ -416,6 +418,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; } @@ -1780,6 +1785,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/cli/src/main.rs b/codex-rs/cli/src/main.rs index dd20f04128..7ab8bc937a 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, @@ -323,6 +323,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, @@ -1036,7 +1038,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, } @@ -1047,6 +1052,7 @@ mod tests { let exit_info = AppExitInfo { token_usage: TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::UserRequested, }; @@ -1056,7 +1062,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, @@ -1070,12 +1076,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/core/src/codex.rs b/codex-rs/core/src/codex.rs index 250ea1ff49..7eefd04ec4 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(), @@ -1451,8 +1478,7 @@ impl Session { .lock() .await .session_configuration - .original_config_do_not_use - .codex_home + .codex_home() .clone(); if !features.enabled(Feature::ExecPolicy) { @@ -2440,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, @@ -2483,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; @@ -2499,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; @@ -2952,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 @@ -4440,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(), @@ -4521,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(), @@ -4786,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(), @@ -4900,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/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/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/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/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/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/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/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/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 6664e7f9c5..7f7fe51c5d 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -94,6 +94,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 fe33df8378..107e9f6ed6 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -108,6 +108,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, } @@ -117,6 +118,7 @@ impl AppExitInfo { Self { token_usage: TokenUsage::default(), thread_id: None, + thread_name: None, update_action: None, exit_reason: ExitReason::Fatal(message.into()), } @@ -135,13 +137,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, @@ -496,6 +502,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, }); @@ -1208,6 +1215,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, }) @@ -1269,8 +1277,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"); @@ -1343,6 +1354,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 @@ -1401,8 +1413,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 @@ -2241,6 +2256,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, @@ -2908,6 +2924,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, @@ -2960,6 +2977,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, @@ -3005,6 +3023,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, @@ -3036,7 +3055,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] @@ -3049,7 +3068,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" @@ -3059,4 +3078,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 9857956ed9..8223c40ce0 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; @@ -197,7 +198,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; @@ -517,6 +517,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 @@ -788,6 +789,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(); @@ -828,6 +830,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); } @@ -2167,6 +2176,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, @@ -2307,6 +2317,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(), @@ -2436,6 +2447,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, @@ -2706,6 +2718,9 @@ impl ChatWidget { SlashCommand::Review => { self.open_review_popup(); } + SlashCommand::Rename => { + self.show_rename_prompt(); + } SlashCommand::Model => { self.open_model_popup(); } @@ -2907,6 +2922,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); @@ -2925,6 +2951,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); } @@ -3125,7 +3183,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. @@ -3169,6 +3230,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) @@ -3468,6 +3530,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, @@ -5275,6 +5338,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()); @@ -5796,6 +5873,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 144fc199c5..331afdf90a 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -134,6 +134,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, @@ -200,6 +201,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, @@ -252,6 +254,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, @@ -823,6 +826,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, diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 010f8c83e5..c9f307dbe4 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -26,6 +26,7 @@ use codex_core::config::resolve_oss_provider; 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 +48,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; @@ -411,6 +413,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 +454,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, }); @@ -484,6 +488,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 +499,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 +544,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 +555,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 +602,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, }); 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/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 84a96f54d0..a8521f359f 100644 --- a/codex-rs/tui/tooltips.txt +++ b/codex-rs/tui/tooltips.txt @@ -9,6 +9,7 @@ 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 From e85d019daaa4c42fc9404678f5dec52e424c9f6a Mon Sep 17 00:00:00 2001 From: gt-oai Date: Fri, 30 Jan 2026 12:03:29 +0000 Subject: [PATCH 10/18] Fetch Requirements from cloud (#10167) Load requirements from Codex Backend. It only does this for enterprise customers signed in with ChatGPT. Todo in follow-up PRs: * Add to app-server and exec too * Switch from fail-open to fail-closed on failure --- codex-rs/Cargo.lock | 20 ++ codex-rs/Cargo.toml | 2 + codex-rs/cloud-requirements/BUILD.bazel | 6 + codex-rs/cloud-requirements/Cargo.toml | 26 ++ codex-rs/cloud-requirements/src/lib.rs | 338 ++++++++++++++++++ codex-rs/core/src/config/mod.rs | 35 +- codex-rs/core/src/config/service.rs | 1 + codex-rs/core/src/config_loader/README.md | 3 +- .../src/config_loader/cloud_requirements.rs | 56 +++ .../src/config_loader/config_requirements.rs | 31 ++ codex-rs/core/src/config_loader/mod.rs | 11 + codex-rs/core/src/config_loader/tests.rs | 103 ++++++ codex-rs/core/src/skills/manager.rs | 1 + codex-rs/exec-server/src/posix.rs | 1 + codex-rs/network-proxy/src/state.rs | 7 +- codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/lib.rs | 48 ++- 17 files changed, 673 insertions(+), 17 deletions(-) create mode 100644 codex-rs/cloud-requirements/BUILD.bazel create mode 100644 codex-rs/cloud-requirements/Cargo.toml create mode 100644 codex-rs/cloud-requirements/src/lib.rs create mode 100644 codex-rs/core/src/config_loader/cloud_requirements.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 35d3129106..44f35f78ae 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1292,6 +1292,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" @@ -1895,6 +1914,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/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..ccfa600319 --- /dev/null +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -0,0 +1,338 @@ +//! 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_app_server_protocol::AuthMode; +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.mode == AuthMode::ChatGPT + && 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/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/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/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/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/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 7f7fe51c5d..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", diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index c9f307dbe4..a8c6e2b88b 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,6 +24,7 @@ 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; @@ -206,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(), @@ -257,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)] @@ -370,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( @@ -380,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()?; @@ -465,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 } @@ -633,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 @@ -816,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 From 6cee538380eb9735c1dbb91ec33c7cf9d43cac00 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 13:50:33 +0100 Subject: [PATCH 11/18] explorer prompt (#10225) --- codex-rs/core/src/agent/role.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) 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() }, From 09d25e91e96ac87e88171764fcd8e9da950ba7e4 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 14:18:31 +0100 Subject: [PATCH 12/18] fix: make sure the shell exists (#10222) --- codex-rs/core/src/shell.rs | 1 + codex-rs/core/tests/suite/view_image.rs | 24 +++++++++++++++++------- 2 files changed, 18 insertions(+), 7 deletions(-) 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/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(()) } From 887bec0dee624bbe5f3e3f064a72fb86e563a5af Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 18:23:00 +0100 Subject: [PATCH 13/18] chore: do not clean the DB anymore (#10232) --- codex-rs/core/src/rollout/recorder.rs | 2 +- codex-rs/core/src/state_db.rs | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 8157133165..501b46271f 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -428,7 +428,7 @@ 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); } } diff --git a/codex-rs/core/src/state_db.rs b/codex-rs/core/src/state_db.rs index 0804273bba..3e5beaa87e 100644 --- a/codex-rs/core/src/state_db.rs +++ b/codex-rs/core/src/state_db.rs @@ -33,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); From eff11f792b6bfae2dffdfa33c904b5732194e560 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 18:23:18 +0100 Subject: [PATCH 14/18] feat: improve logs client (#10229) --- codex-rs/state/src/bin/logs_client.rs | 144 ++++++++++++++++++-------- codex-rs/state/src/model/log.rs | 7 +- codex-rs/state/src/runtime.rs | 58 ++++++++--- 3 files changed, 148 insertions(+), 61 deletions(-) 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/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) From 079f4952e0bdb147769849f2f7dcc06d4100bdf0 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 18:26:49 +0100 Subject: [PATCH 15/18] feat: heuristic coloring of logs (#10228) From 0212f4010e2b66eea296202da90778ce9d44babb Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 30 Jan 2026 18:32:10 +0100 Subject: [PATCH 16/18] nit: fix db with multiple metadata lines (#10237) --- codex-rs/core/src/rollout/recorder.rs | 2 +- codex-rs/state/src/extract.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 501b46271f..265ff2bde2 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -434,7 +434,7 @@ impl RolloutRecorder { } } - info!( + tracing::debug!( "Resumed rollout with {} items, thread ID: {:?}, parse errors: {}", items.len(), thread_id, 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() { From 377ab0c77cd8609637f2fa6d7c0230fa69ce3acc Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 30 Jan 2026 09:33:23 -0800 Subject: [PATCH 17/18] feat: refactor CodexAuth so invalid state cannot be represented (#10208) Previously, `CodexAuth` was defined as follows: https://github.com/openai/codex/blob/d550fbf41afc09d7d7b5ac813aea38de07b2a73f/codex-rs/core/src/auth.rs#L39-L46 But if you looked at its constructors, we had creation for `AuthMode::ApiKey` where `storage` was built using a nonsensical path (`PathBuf::new()`) and `auth_dot_json` was `None`: https://github.com/openai/codex/blob/d550fbf41afc09d7d7b5ac813aea38de07b2a73f/codex-rs/core/src/auth.rs#L212-L220 By comparison, when `AuthMode::ChatGPT` was used, `api_key` was always `None`: https://github.com/openai/codex/blob/d550fbf41afc09d7d7b5ac813aea38de07b2a73f/codex-rs/core/src/auth.rs#L665-L671 https://github.com/openai/codex/pull/10012 took things further because it introduced a new `ChatgptAuthTokens` variant to `AuthMode`, which is important in when invoking `account/login/start` via the app server, but most logic _internal_ to the app server should just reason about two `AuthMode` variants: `ApiKey` and `ChatGPT`. This PR tries to clean things up as follows: - `LoginAccountParams` and `AuthMode` in `codex-rs/app-server-protocol/` both continue to have the `ChatgptAuthTokens` variant, though it is used exclusively for the on-the-wire messaging. - `codex-rs/core/src/auth.rs` now has its own `AuthMode` enum, which only has two variants: `ApiKey` and `ChatGPT`. - `CodexAuth` has been changed from a struct to an enum. It is a disjoint union where each variant (`ApiKey`, `ChatGpt`, and `ChatGptAuthTokens`) have only the associated fields that make sense for that variant. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/10208). * #10224 * __->__ #10208 --- .../src/protocol/common.rs | 4 - .../app-server/src/codex_message_processor.rs | 39 +- codex-rs/cli/src/login.rs | 2 +- codex-rs/core/src/auth.rs | 340 ++++++++++++------ codex-rs/core/src/client.rs | 14 +- codex-rs/core/src/codex.rs | 2 +- codex-rs/core/src/model_provider_info.rs | 9 +- codex-rs/core/src/models_manager/manager.rs | 14 +- .../core/src/models_manager/model_presets.rs | 2 +- codex-rs/tui/src/app.rs | 2 +- codex-rs/tui/src/chatwidget.rs | 13 +- codex-rs/tui/src/lib.rs | 2 +- codex-rs/tui/src/status/helpers.rs | 8 +- 13 files changed, 286 insertions(+), 165 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 2e8778bdc5..67736374ec 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -24,10 +24,6 @@ impl GitSha { } /// Authentication mode for OpenAI-backed providers. -/// -/// This is used internally to determine the base URL for generating responses, and to gate -/// ChatGPT-only behaviors like rate limits and available models (as opposed to API key-based -/// auth). #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Display, JsonSchema, TS)] #[serde(rename_all = "lowercase")] pub enum AuthMode { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 3817bce3a1..c02f3c7368 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -133,6 +133,7 @@ use codex_app_server_protocol::build_turns_from_event_msgs; use codex_backend_client::Client as BackendClient; use codex_chatgpt::connectors; use codex_core::AuthManager; +use codex_core::CodexAuth; use codex_core::CodexThread; use codex_core::Cursor as RolloutCursor; use codex_core::InitialHistory; @@ -686,7 +687,11 @@ impl CodexMessageProcessor { .await; let payload = AuthStatusChangeNotification { - auth_method: self.auth_manager.auth_cached().map(|auth| auth.mode), + auth_method: self + .auth_manager + .auth_cached() + .as_ref() + .map(CodexAuth::api_auth_mode), }; self.outgoing .send_server_notification(ServerNotification::AuthStatusChange(payload)) @@ -716,7 +721,11 @@ impl CodexMessageProcessor { .await; let payload_v2 = AccountUpdatedNotification { - auth_mode: self.auth_manager.auth_cached().map(|auth| auth.mode), + auth_mode: self + .auth_manager + .auth_cached() + .as_ref() + .map(CodexAuth::api_auth_mode), }; self.outgoing .send_server_notification(ServerNotification::AccountUpdated(payload_v2)) @@ -812,7 +821,10 @@ impl CodexMessageProcessor { auth_manager.reload(); // Notify clients with the actual current auth mode. - let current_auth_method = auth_manager.auth_cached().map(|a| a.mode); + let current_auth_method = auth_manager + .auth_cached() + .as_ref() + .map(CodexAuth::api_auth_mode); let payload = AuthStatusChangeNotification { auth_method: current_auth_method, }; @@ -902,7 +914,10 @@ impl CodexMessageProcessor { auth_manager.reload(); // Notify clients with the actual current auth mode. - let current_auth_method = auth_manager.auth_cached().map(|a| a.mode); + let current_auth_method = auth_manager + .auth_cached() + .as_ref() + .map(CodexAuth::api_auth_mode); let payload_v2 = AccountUpdatedNotification { auth_mode: current_auth_method, }; @@ -1106,7 +1121,11 @@ impl CodexMessageProcessor { } // Reflect the current auth method after logout (likely None). - Ok(self.auth_manager.auth_cached().map(|auth| auth.mode)) + Ok(self + .auth_manager + .auth_cached() + .as_ref() + .map(CodexAuth::api_auth_mode)) } async fn logout_v1(&mut self, request_id: RequestId) { @@ -1178,7 +1197,7 @@ impl CodexMessageProcessor { } else { match self.auth_manager.auth().await { Some(auth) => { - let auth_mode = auth.mode; + let auth_mode = auth.api_auth_mode(); let (reported_auth_method, token_opt) = match auth.get_token() { Ok(token) if !token.is_empty() => { let tok = if include_token { Some(token) } else { None }; @@ -1225,9 +1244,9 @@ impl CodexMessageProcessor { } let account = match self.auth_manager.auth_cached() { - Some(auth) => Some(match auth.mode { - AuthMode::ApiKey => Account::ApiKey {}, - AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens => { + Some(auth) => Some(match auth { + CodexAuth::ApiKey(_) => Account::ApiKey {}, + CodexAuth::ChatGpt(_) | CodexAuth::ChatGptAuthTokens(_) => { let email = auth.get_account_email(); let plan_type = auth.account_plan_type(); @@ -1286,7 +1305,7 @@ impl CodexMessageProcessor { }); }; - if !matches!(auth.mode, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) { + if !auth.is_chatgpt_auth() { return Err(JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, message: "chatgpt authentication required to read rate limits".to_string(), diff --git a/codex-rs/cli/src/login.rs b/codex-rs/cli/src/login.rs index 0f4e4eb39f..cee002f339 100644 --- a/codex-rs/cli/src/login.rs +++ b/codex-rs/cli/src/login.rs @@ -225,7 +225,7 @@ pub async fn run_login_status(cli_config_overrides: CliConfigOverrides) -> ! { let config = load_config_or_exit(cli_config_overrides).await; match CodexAuth::from_auth_storage(&config.codex_home, config.cli_auth_credentials_store_mode) { - Ok(Some(auth)) => match auth.mode { + Ok(Some(auth)) => match auth.api_auth_mode() { AuthMode::ApiKey => match auth.get_token() { Ok(api_key) => { eprintln!("Logged in using an API key - {}", safe_format_key(&api_key)); diff --git a/codex-rs/core/src/auth.rs b/codex-rs/core/src/auth.rs index d4520dc855..4bddbe3031 100644 --- a/codex-rs/core/src/auth.rs +++ b/codex-rs/core/src/auth.rs @@ -13,8 +13,9 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; +use std::sync::RwLock; -use codex_app_server_protocol::AuthMode; +use codex_app_server_protocol::AuthMode as ApiAuthMode; use codex_protocol::config_types::ForcedLoginMethod; pub use crate::auth::storage::AuthCredentialsStoreMode; @@ -35,19 +36,50 @@ use codex_protocol::account::PlanType as AccountPlanType; use serde_json::Value; use thiserror::Error; -#[derive(Debug, Clone)] -pub struct CodexAuth { - pub mode: AuthMode, +/// Account type for the current user. +/// +/// This is used internally to determine the base URL for generating responses, +/// and to gate ChatGPT-only behaviors like rate limits and available models (as +/// opposed to API key-based auth). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum AuthMode { + ApiKey, + ChatGPT, +} - pub(crate) api_key: Option, - pub(crate) auth_dot_json: Arc>>, +/// Authentication mechanism used by the current user. +#[derive(Debug, Clone)] +pub enum CodexAuth { + ApiKey(ApiKeyAuth), + ChatGpt(ChatGptAuth), + ChatGptAuthTokens(ChatGptAuthTokens), +} + +#[derive(Debug, Clone)] +pub struct ApiKeyAuth { + api_key: String, +} + +#[derive(Debug, Clone)] +pub struct ChatGptAuth { + state: ChatGptAuthState, storage: Arc, - pub(crate) client: CodexHttpClient, +} + +#[derive(Debug, Clone)] +pub struct ChatGptAuthTokens { + state: ChatGptAuthState, +} + +#[derive(Debug, Clone)] +struct ChatGptAuthState { + auth_dot_json: Arc>>, + client: CodexHttpClient, } impl PartialEq for CodexAuth { fn eq(&self, other: &Self) -> bool { - self.mode == other.mode + self.api_auth_mode() == other.api_auth_mode() } } @@ -114,14 +146,78 @@ impl From for std::io::Error { } impl CodexAuth { + fn from_auth_dot_json( + codex_home: &Path, + auth_dot_json: AuthDotJson, + auth_credentials_store_mode: AuthCredentialsStoreMode, + client: CodexHttpClient, + ) -> std::io::Result { + let auth_mode = auth_dot_json.resolved_mode(); + if auth_mode == ApiAuthMode::ApiKey { + let Some(api_key) = auth_dot_json.openai_api_key.as_deref() else { + return Err(std::io::Error::other("API key auth is missing a key.")); + }; + return Ok(CodexAuth::from_api_key_with_client(api_key, client)); + } + + let storage_mode = auth_dot_json.storage_mode(auth_credentials_store_mode); + let state = ChatGptAuthState { + auth_dot_json: Arc::new(Mutex::new(Some(auth_dot_json))), + client, + }; + + match auth_mode { + ApiAuthMode::ChatGPT => { + let storage = create_auth_storage(codex_home.to_path_buf(), storage_mode); + Ok(Self::ChatGpt(ChatGptAuth { state, storage })) + } + ApiAuthMode::ChatgptAuthTokens => { + Ok(Self::ChatGptAuthTokens(ChatGptAuthTokens { state })) + } + ApiAuthMode::ApiKey => unreachable!("api key mode is handled above"), + } + } + /// Loads the available auth information from auth storage. pub fn from_auth_storage( codex_home: &Path, auth_credentials_store_mode: AuthCredentialsStoreMode, - ) -> std::io::Result> { + ) -> std::io::Result> { load_auth(codex_home, false, auth_credentials_store_mode) } + pub fn internal_auth_mode(&self) -> AuthMode { + match self { + Self::ApiKey(_) => AuthMode::ApiKey, + Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) => AuthMode::ChatGPT, + } + } + + pub fn api_auth_mode(&self) -> ApiAuthMode { + match self { + Self::ApiKey(_) => ApiAuthMode::ApiKey, + Self::ChatGpt(_) => ApiAuthMode::ChatGPT, + Self::ChatGptAuthTokens(_) => ApiAuthMode::ChatgptAuthTokens, + } + } + + pub fn is_chatgpt_auth(&self) -> bool { + self.internal_auth_mode() == AuthMode::ChatGPT + } + + pub fn is_external_chatgpt_tokens(&self) -> bool { + matches!(self, Self::ChatGptAuthTokens(_)) + } + + /// Returns `None` is `is_internal_auth_mode() != AuthMode::ApiKey`. + pub fn api_key(&self) -> Option<&str> { + match self { + Self::ApiKey(auth) => Some(auth.api_key.as_str()), + Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) => None, + } + } + + /// Returns `Err` if `is_chatgpt_auth()` is false. pub fn get_token_data(&self) -> Result { let auth_dot_json: Option = self.get_current_auth_json(); match auth_dot_json { @@ -134,20 +230,23 @@ impl CodexAuth { } } + /// Returns the token string used for bearer authentication. pub fn get_token(&self) -> Result { - match self.mode { - AuthMode::ApiKey => Ok(self.api_key.clone().unwrap_or_default()), - AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens => { + match self { + Self::ApiKey(auth) => Ok(auth.api_key.clone()), + Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) => { let access_token = self.get_token_data()?.access_token; Ok(access_token) } } } + /// Returns `None` if `is_chatgpt_auth()` is false. pub fn get_account_id(&self) -> Option { self.get_current_token_data().and_then(|t| t.account_id) } + /// Returns `None` if `is_chatgpt_auth()` is false. pub fn get_account_email(&self) -> Option { self.get_current_token_data().and_then(|t| t.id_token.email) } @@ -176,11 +275,18 @@ impl CodexAuth { }) } + /// Returns `None` if `is_chatgpt_auth()` is false. fn get_current_auth_json(&self) -> Option { + let state = match self { + Self::ChatGpt(auth) => &auth.state, + Self::ChatGptAuthTokens(auth) => &auth.state, + Self::ApiKey(_) => return None, + }; #[expect(clippy::unwrap_used)] - self.auth_dot_json.lock().unwrap().clone() + state.auth_dot_json.lock().unwrap().clone() } + /// Returns `None` if `is_chatgpt_auth()` is false. fn get_current_token_data(&self) -> Option { self.get_current_auth_json().and_then(|t| t.tokens) } @@ -188,7 +294,7 @@ impl CodexAuth { /// Consider this private to integration tests. pub fn create_dummy_chatgpt_auth_for_testing() -> Self { let auth_dot_json = AuthDotJson { - auth_mode: Some(AuthMode::ChatGPT), + auth_mode: Some(ApiAuthMode::ChatGPT), openai_api_key: None, tokens: Some(TokenData { id_token: Default::default(), @@ -199,24 +305,19 @@ impl CodexAuth { last_refresh: Some(Utc::now()), }; - let auth_dot_json = Arc::new(Mutex::new(Some(auth_dot_json))); - Self { - api_key: None, - mode: AuthMode::ChatGPT, - storage: create_auth_storage(PathBuf::new(), AuthCredentialsStoreMode::File), - auth_dot_json, - client: crate::default_client::create_client(), - } + let client = crate::default_client::create_client(); + let state = ChatGptAuthState { + auth_dot_json: Arc::new(Mutex::new(Some(auth_dot_json))), + client, + }; + let storage = create_auth_storage(PathBuf::new(), AuthCredentialsStoreMode::File); + Self::ChatGpt(ChatGptAuth { state, storage }) } - fn from_api_key_with_client(api_key: &str, client: CodexHttpClient) -> Self { - Self { - api_key: Some(api_key.to_owned()), - mode: AuthMode::ApiKey, - storage: create_auth_storage(PathBuf::new(), AuthCredentialsStoreMode::File), - auth_dot_json: Arc::new(Mutex::new(None)), - client, - } + fn from_api_key_with_client(api_key: &str, _client: CodexHttpClient) -> Self { + Self::ApiKey(ApiKeyAuth { + api_key: api_key.to_owned(), + }) } pub fn from_api_key(api_key: &str) -> Self { @@ -224,6 +325,25 @@ impl CodexAuth { } } +impl ChatGptAuth { + fn current_auth_json(&self) -> Option { + #[expect(clippy::unwrap_used)] + self.state.auth_dot_json.lock().unwrap().clone() + } + + fn current_token_data(&self) -> Option { + self.current_auth_json().and_then(|auth| auth.tokens) + } + + fn storage(&self) -> &Arc { + &self.storage + } + + fn client(&self) -> &CodexHttpClient { + &self.state.client + } +} + pub const OPENAI_API_KEY_ENV_VAR: &str = "OPENAI_API_KEY"; pub const CODEX_API_KEY_ENV_VAR: &str = "CODEX_API_KEY"; @@ -258,7 +378,7 @@ pub fn login_with_api_key( auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> std::io::Result<()> { let auth_dot_json = AuthDotJson { - auth_mode: Some(AuthMode::ApiKey), + auth_mode: Some(ApiAuthMode::ApiKey), openai_api_key: Some(api_key.to_string()), tokens: None, last_refresh: None, @@ -314,10 +434,10 @@ pub fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> { }; if let Some(required_method) = config.forced_login_method { - let method_violation = match (required_method, auth.mode) { + let method_violation = match (required_method, auth.internal_auth_mode()) { (ForcedLoginMethod::Api, AuthMode::ApiKey) => None, - (ForcedLoginMethod::Chatgpt, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) => None, - (ForcedLoginMethod::Api, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) => Some( + (ForcedLoginMethod::Chatgpt, AuthMode::ChatGPT) => None, + (ForcedLoginMethod::Api, AuthMode::ChatGPT) => Some( "API key login is required, but ChatGPT is currently being used. Logging out." .to_string(), ), @@ -337,7 +457,7 @@ pub fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> { } if let Some(expected_account_id) = config.forced_chatgpt_workspace_id.as_deref() { - if !matches!(auth.mode, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) { + if !auth.is_chatgpt_auth() { return Ok(()); } @@ -607,7 +727,7 @@ impl AuthDotJson { }; Self { - auth_mode: Some(AuthMode::ChatgptAuthTokens), + auth_mode: Some(ApiAuthMode::ChatgptAuthTokens), openai_api_key: None, tokens: Some(tokens), last_refresh: Some(Utc::now()), @@ -623,21 +743,21 @@ impl AuthDotJson { Ok(Self::from_external_tokens(&external, id_token_info)) } - fn resolved_mode(&self) -> AuthMode { + fn resolved_mode(&self) -> ApiAuthMode { if let Some(mode) = self.auth_mode { return mode; } if self.openai_api_key.is_some() { - return AuthMode::ApiKey; + return ApiAuthMode::ApiKey; } - AuthMode::ChatGPT + ApiAuthMode::ChatGPT } fn storage_mode( &self, auth_credentials_store_mode: AuthCredentialsStoreMode, ) -> AuthCredentialsStoreMode { - if self.resolved_mode() == AuthMode::ChatgptAuthTokens { + if self.resolved_mode() == ApiAuthMode::ChatgptAuthTokens { AuthCredentialsStoreMode::Ephemeral } else { auth_credentials_store_mode @@ -645,35 +765,6 @@ impl AuthDotJson { } } -impl CodexAuth { - fn from_auth_dot_json( - codex_home: &Path, - auth_dot_json: AuthDotJson, - auth_credentials_store_mode: AuthCredentialsStoreMode, - client: CodexHttpClient, - ) -> std::io::Result { - let auth_mode = auth_dot_json.resolved_mode(); - if auth_mode == AuthMode::ApiKey { - let Some(api_key) = auth_dot_json.openai_api_key.as_deref() else { - return Err(std::io::Error::other("API key auth is missing a key.")); - }; - return Ok(CodexAuth::from_api_key_with_client(api_key, client)); - } - - let storage_mode = auth_dot_json.storage_mode(auth_credentials_store_mode); - let storage = create_auth_storage(codex_home.to_path_buf(), storage_mode); - Ok(Self { - api_key: None, - mode: auth_mode, - storage, - auth_dot_json: Arc::new(Mutex::new(Some(auth_dot_json))), - client, - }) - } -} - -use std::sync::RwLock; - /// Internal cached auth state. #[derive(Clone)] struct CachedAuth { @@ -685,7 +776,10 @@ struct CachedAuth { impl Debug for CachedAuth { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("CachedAuth") - .field("auth_mode", &self.auth.as_ref().map(|auth| auth.mode)) + .field( + "auth_mode", + &self.auth.as_ref().map(CodexAuth::api_auth_mode), + ) .field( "external_refresher", &self.external_refresher.as_ref().map(|_| "present"), @@ -736,11 +830,13 @@ impl UnauthorizedRecovery { fn new(manager: Arc) -> Self { let cached_auth = manager.auth_cached(); let expected_account_id = cached_auth.as_ref().and_then(CodexAuth::get_account_id); - let mode = match cached_auth { - Some(auth) if auth.mode == AuthMode::ChatgptAuthTokens => { - UnauthorizedRecoveryMode::External - } - _ => UnauthorizedRecoveryMode::Managed, + let mode = if cached_auth + .as_ref() + .is_some_and(CodexAuth::is_external_chatgpt_tokens) + { + UnauthorizedRecoveryMode::External + } else { + UnauthorizedRecoveryMode::Managed }; let step = match mode { UnauthorizedRecoveryMode::Managed => UnauthorizedRecoveryStep::Reload, @@ -755,9 +851,12 @@ impl UnauthorizedRecovery { } pub fn has_next(&self) -> bool { - if !self.manager.auth_cached().is_some_and(|auth| { - matches!(auth.mode, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) - }) { + if !self + .manager + .auth_cached() + .as_ref() + .is_some_and(CodexAuth::is_chatgpt_auth) + { return false; } @@ -996,7 +1095,8 @@ impl AuthManager { pub fn is_external_auth_active(&self) -> bool { self.auth_cached() - .is_some_and(|auth| auth.mode == AuthMode::ChatgptAuthTokens) + .as_ref() + .is_some_and(CodexAuth::is_external_chatgpt_tokens) } /// Convenience constructor returning an `Arc` wrapper. @@ -1026,18 +1126,25 @@ impl AuthManager { Some(auth) => auth, None => return Ok(()), }; - if auth.mode == AuthMode::ChatgptAuthTokens { - return self - .refresh_external_auth(ExternalAuthRefreshReason::Unauthorized) - .await; + match auth { + CodexAuth::ChatGptAuthTokens(_) => { + self.refresh_external_auth(ExternalAuthRefreshReason::Unauthorized) + .await + } + CodexAuth::ChatGpt(chatgpt_auth) => { + let token_data = chatgpt_auth.current_token_data().ok_or_else(|| { + RefreshTokenError::Transient(std::io::Error::other( + "Token data is not available.", + )) + })?; + self.refresh_tokens(&chatgpt_auth, token_data.refresh_token) + .await?; + // Reload to pick up persisted changes. + self.reload(); + Ok(()) + } + CodexAuth::ApiKey(_) => Ok(()), } - let token_data = auth.get_current_token_data().ok_or_else(|| { - RefreshTokenError::Transient(std::io::Error::other("Token data is not available.")) - })?; - self.refresh_tokens(&auth, token_data.refresh_token).await?; - // Reload to pick up persisted changes. - self.reload(); - Ok(()) } /// Log out by deleting the on‑disk auth.json (if present). Returns Ok(true) @@ -1051,16 +1158,23 @@ impl AuthManager { Ok(removed) } - pub fn get_auth_mode(&self) -> Option { - self.auth_cached().map(|a| a.mode) + pub fn get_auth_mode(&self) -> Option { + self.auth_cached().as_ref().map(CodexAuth::api_auth_mode) + } + + pub fn get_internal_auth_mode(&self) -> Option { + self.auth_cached() + .as_ref() + .map(CodexAuth::internal_auth_mode) } async fn refresh_if_stale(&self, auth: &CodexAuth) -> Result { - if auth.mode != AuthMode::ChatGPT { - return Ok(false); - } + let chatgpt_auth = match auth { + CodexAuth::ChatGpt(chatgpt_auth) => chatgpt_auth, + _ => return Ok(false), + }; - let auth_dot_json = match auth.get_current_auth_json() { + let auth_dot_json = match chatgpt_auth.current_auth_json() { Some(auth_dot_json) => auth_dot_json, None => return Ok(false), }; @@ -1075,7 +1189,8 @@ impl AuthManager { if last_refresh >= Utc::now() - chrono::Duration::days(TOKEN_REFRESH_INTERVAL) { return Ok(false); } - self.refresh_tokens(auth, tokens.refresh_token).await?; + self.refresh_tokens(chatgpt_auth, tokens.refresh_token) + .await?; self.reload(); Ok(true) } @@ -1135,13 +1250,13 @@ impl AuthManager { async fn refresh_tokens( &self, - auth: &CodexAuth, + auth: &ChatGptAuth, refresh_token: String, ) -> Result<(), RefreshTokenError> { - let refresh_response = try_refresh_token(refresh_token, &auth.client).await?; + let refresh_response = try_refresh_token(refresh_token, auth.client()).await?; update_tokens( - &auth.storage, + auth.storage(), refresh_response.id_token, refresh_response.access_token, refresh_response.refresh_token, @@ -1254,26 +1369,21 @@ mod tests { ) .expect("failed to write auth file"); - let CodexAuth { - api_key, - mode, - auth_dot_json, - storage: _, - .. - } = super::load_auth(codex_home.path(), false, AuthCredentialsStoreMode::File) + let auth = super::load_auth(codex_home.path(), false, AuthCredentialsStoreMode::File) .unwrap() .unwrap(); - assert_eq!(None, api_key); - assert_eq!(AuthMode::ChatGPT, mode); + assert_eq!(None, auth.api_key()); + assert_eq!(AuthMode::ChatGPT, auth.internal_auth_mode()); - let guard = auth_dot_json.lock().unwrap(); - let auth_dot_json = guard.as_ref().expect("AuthDotJson should exist"); + let auth_dot_json = auth + .get_current_auth_json() + .expect("AuthDotJson should exist"); let last_refresh = auth_dot_json .last_refresh .expect("last_refresh should be recorded"); assert_eq!( - &AuthDotJson { + AuthDotJson { auth_mode: None, openai_api_key: None, tokens: Some(TokenData { @@ -1308,8 +1418,8 @@ mod tests { let auth = super::load_auth(dir.path(), false, AuthCredentialsStoreMode::File) .unwrap() .unwrap(); - assert_eq!(auth.mode, AuthMode::ApiKey); - assert_eq!(auth.api_key, Some("sk-test-key".to_string())); + assert_eq!(auth.internal_auth_mode(), AuthMode::ApiKey); + assert_eq!(auth.api_key(), Some("sk-test-key")); assert!(auth.get_token_data().is_err()); } @@ -1318,7 +1428,7 @@ mod tests { fn logout_removes_auth_file() -> Result<(), std::io::Error> { let dir = tempdir()?; let auth_dot_json = AuthDotJson { - auth_mode: Some(AuthMode::ApiKey), + auth_mode: Some(ApiAuthMode::ApiKey), openai_api_key: Some("sk-test-key".to_string()), tokens: None, last_refresh: None, diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 0f05f58c47..f01c145e56 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -27,7 +27,6 @@ use codex_api::common::ResponsesWsRequest; use codex_api::create_text_param_for_request; use codex_api::error::ApiError; use codex_api::requests::responses::Compression; -use codex_app_server_protocol::AuthMode; use codex_otel::OtelManager; use codex_protocol::ThreadId; @@ -50,6 +49,7 @@ use tokio::sync::mpsc; use tracing::warn; use crate::AuthManager; +use crate::auth::CodexAuth; use crate::auth::RefreshTokenError; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; @@ -220,7 +220,7 @@ impl ModelClient { let api_provider = self .state .provider - .to_api_provider(auth.as_ref().map(|a| a.mode))?; + .to_api_provider(auth.as_ref().map(CodexAuth::internal_auth_mode))?; let api_auth = auth_provider_from_auth(auth.clone(), &self.state.provider)?; let transport = ReqwestTransport::new(build_reqwest_client()); let request_telemetry = self.build_request_telemetry(); @@ -469,9 +469,7 @@ impl ModelClientSession { .config .features .enabled(Feature::EnableRequestCompression) - && auth.is_some_and(|auth| { - matches!(auth.mode, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) - }) + && auth.is_some_and(CodexAuth::is_chatgpt_auth) && self.state.provider.is_openai() { Compression::Zstd @@ -509,7 +507,7 @@ impl ModelClientSession { let api_provider = self .state .provider - .to_api_provider(auth.as_ref().map(|a| a.mode))?; + .to_api_provider(auth.as_ref().map(CodexAuth::internal_auth_mode))?; let api_auth = auth_provider_from_auth(auth.clone(), &self.state.provider)?; let transport = ReqwestTransport::new(build_reqwest_client()); let (request_telemetry, sse_telemetry) = self.build_streaming_telemetry(); @@ -565,7 +563,7 @@ impl ModelClientSession { let api_provider = self .state .provider - .to_api_provider(auth.as_ref().map(|a| a.mode))?; + .to_api_provider(auth.as_ref().map(CodexAuth::internal_auth_mode))?; let api_auth = auth_provider_from_auth(auth.clone(), &self.state.provider)?; let transport = ReqwestTransport::new(build_reqwest_client()); let (request_telemetry, sse_telemetry) = self.build_streaming_telemetry(); @@ -611,7 +609,7 @@ impl ModelClientSession { let api_provider = self .state .provider - .to_api_provider(auth.as_ref().map(|a| a.mode))?; + .to_api_provider(auth.as_ref().map(CodexAuth::internal_auth_mode))?; let api_auth = auth_provider_from_auth(auth.clone(), &self.state.provider)?; let compression = self.responses_request_compression(auth.as_ref()); diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7eefd04ec4..324f41a04e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -838,7 +838,7 @@ impl Session { session_configuration.collaboration_mode.model(), auth.and_then(CodexAuth::get_account_id), auth.and_then(CodexAuth::get_account_email), - auth.map(|a| a.mode), + auth.map(CodexAuth::api_auth_mode), config.otel.log_user_prompt, terminal::user_agent(), session_configuration.session_source.clone(), diff --git a/codex-rs/core/src/model_provider_info.rs b/codex-rs/core/src/model_provider_info.rs index 0f3551480c..0f5a8643a7 100644 --- a/codex-rs/core/src/model_provider_info.rs +++ b/codex-rs/core/src/model_provider_info.rs @@ -5,10 +5,11 @@ //! 2. User-defined entries inside `~/.codex/config.toml` under the `model_providers` //! key. These override or extend the defaults at runtime. +use crate::auth::AuthMode; +use crate::error::EnvVarError; use codex_api::Provider as ApiProvider; use codex_api::WireApi as ApiWireApi; use codex_api::provider::RetryConfig as ApiRetryConfig; -use codex_app_server_protocol::AuthMode; use http::HeaderMap; use http::header::HeaderName; use http::header::HeaderValue; @@ -19,7 +20,6 @@ use std::collections::HashMap; use std::env::VarError; use std::time::Duration; -use crate::error::EnvVarError; const DEFAULT_STREAM_IDLE_TIMEOUT_MS: u64 = 300_000; const DEFAULT_STREAM_MAX_RETRIES: u64 = 5; const DEFAULT_REQUEST_MAX_RETRIES: u64 = 4; @@ -137,10 +137,7 @@ impl ModelProviderInfo { &self, auth_mode: Option, ) -> crate::error::Result { - let default_base_url = if matches!( - auth_mode, - Some(AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) - ) { + let default_base_url = if matches!(auth_mode, Some(AuthMode::ChatGPT)) { "https://chatgpt.com/backend-api/codex" } else { "https://api.openai.com/v1" diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index c993032ea9..40f9c8ccdc 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -2,6 +2,7 @@ use super::cache::ModelsCacheManager; use crate::api_bridge::auth_provider_from_auth; use crate::api_bridge::map_api_error; use crate::auth::AuthManager; +use crate::auth::AuthMode; use crate::config::Config; use crate::default_client::build_reqwest_client; use crate::error::CodexErr; @@ -13,7 +14,6 @@ use crate::models_manager::model_info; use crate::models_manager::model_presets::builtin_model_presets; use codex_api::ModelsClient; use codex_api::ReqwestTransport; -use codex_app_server_protocol::AuthMode; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelPreset; @@ -61,7 +61,7 @@ impl ModelsManager { let cache_path = codex_home.join(MODEL_CACHE_FILE); let cache_manager = ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL); Self { - local_models: builtin_model_presets(auth_manager.get_auth_mode()), + local_models: builtin_model_presets(auth_manager.get_internal_auth_mode()), remote_models: RwLock::new(Self::load_remote_models_from_file().unwrap_or_default()), auth_manager, etag: RwLock::new(None), @@ -175,7 +175,7 @@ impl ModelsManager { refresh_strategy: RefreshStrategy, ) -> CoreResult<()> { if !config.features.enabled(Feature::RemoteModels) - || self.auth_manager.get_auth_mode() == Some(AuthMode::ApiKey) + || self.auth_manager.get_internal_auth_mode() == Some(AuthMode::ApiKey) { return Ok(()); } @@ -204,7 +204,7 @@ impl ModelsManager { let _timer = codex_otel::start_global_timer("codex.remote_models.fetch_update.duration_ms", &[]); let auth = self.auth_manager.auth().await; - let auth_mode = self.auth_manager.get_auth_mode(); + let auth_mode = self.auth_manager.get_internal_auth_mode(); let api_provider = self.provider.to_api_provider(auth_mode)?; let api_auth = auth_provider_from_auth(auth.clone(), &self.provider)?; let transport = ReqwestTransport::new(build_reqwest_client()); @@ -273,8 +273,8 @@ impl ModelsManager { let existing_presets = self.local_models.clone(); let mut merged_presets = ModelPreset::merge(remote_presets, existing_presets); let chatgpt_mode = matches!( - self.auth_manager.get_auth_mode(), - Some(AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) + self.auth_manager.get_internal_auth_mode(), + Some(AuthMode::ChatGPT) ); merged_presets = ModelPreset::filter_by_auth(merged_presets, chatgpt_mode); @@ -319,7 +319,7 @@ impl ModelsManager { let cache_path = codex_home.join(MODEL_CACHE_FILE); let cache_manager = ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL); Self { - local_models: builtin_model_presets(auth_manager.get_auth_mode()), + local_models: builtin_model_presets(auth_manager.get_internal_auth_mode()), remote_models: RwLock::new(Self::load_remote_models_from_file().unwrap_or_default()), auth_manager, etag: RwLock::new(None), diff --git a/codex-rs/core/src/models_manager/model_presets.rs b/codex-rs/core/src/models_manager/model_presets.rs index a8f4931f8d..f9105c6448 100644 --- a/codex-rs/core/src/models_manager/model_presets.rs +++ b/codex-rs/core/src/models_manager/model_presets.rs @@ -1,4 +1,4 @@ -use codex_app_server_protocol::AuthMode; +use crate::auth::AuthMode; use codex_protocol::openai_models::ModelPreset; use codex_protocol::openai_models::ModelUpgrade; use codex_protocol::openai_models::ReasoningEffort; diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 107e9f6ed6..c65ce4498d 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -984,7 +984,7 @@ impl App { model.as_str(), auth_ref.and_then(CodexAuth::get_account_id), auth_ref.and_then(CodexAuth::get_account_email), - auth_ref.map(|auth| auth.mode), + auth_ref.map(CodexAuth::api_auth_mode), config.otel.log_user_prompt, codex_core::terminal::user_agent(), SessionSource::Cli, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 8223c40ce0..491ea3add3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -30,7 +30,6 @@ use std::time::Duration; use std::time::Instant; use crate::version::CODEX_CLI_VERSION; -use codex_app_server_protocol::AuthMode; use codex_backend_client::Client as BackendClient; use codex_chatgpt::connectors; use codex_core::config::Config; @@ -3584,10 +3583,12 @@ impl ChatWidget { fn prefetch_rate_limits(&mut self) { self.stop_rate_limit_poller(); - if !matches!( - self.auth_manager.auth_cached().map(|auth| auth.mode), - Some(AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) - ) { + if !self + .auth_manager + .auth_cached() + .as_ref() + .is_some_and(CodexAuth::is_chatgpt_auth) + { return; } @@ -3600,7 +3601,7 @@ impl ChatWidget { loop { if let Some(auth) = auth_manager.auth().await - && matches!(auth.mode, AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens) + && auth.is_chatgpt_auth() && let Some(snapshot) = fetch_rate_limits(base_url.clone(), auth).await { app_event_tx.send(AppEvent::RateLimitSnapshotFetched(snapshot)); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index a8c6e2b88b..1f34ed6049 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -833,7 +833,7 @@ fn get_login_status(config: &Config) -> LoginStatus { // to refresh the token. Block on it. let codex_home = config.codex_home.clone(); match CodexAuth::from_auth_storage(&codex_home, config.cli_auth_credentials_store_mode) { - Ok(Some(auth)) => LoginStatus::AuthMode(auth.mode), + Ok(Some(auth)) => LoginStatus::AuthMode(auth.api_auth_mode()), Ok(None) => LoginStatus::NotAuthenticated, Err(err) => { error!("Failed to read auth.json: {err}"); diff --git a/codex-rs/tui/src/status/helpers.rs b/codex-rs/tui/src/status/helpers.rs index 91f958a9cf..0a801227f5 100644 --- a/codex-rs/tui/src/status/helpers.rs +++ b/codex-rs/tui/src/status/helpers.rs @@ -2,8 +2,8 @@ use crate::exec_command::relativize_to_home; use crate::text_formatting; use chrono::DateTime; use chrono::Local; -use codex_app_server_protocol::AuthMode; use codex_core::AuthManager; +use codex_core::CodexAuth; use codex_core::config::Config; use codex_core::project_doc::discover_project_doc_paths; use codex_protocol::account::PlanType; @@ -90,15 +90,15 @@ pub(crate) fn compose_account_display( ) -> Option { let auth = auth_manager.auth_cached()?; - match auth.mode { - AuthMode::ChatGPT | AuthMode::ChatgptAuthTokens => { + match auth { + CodexAuth::ChatGpt(_) | CodexAuth::ChatGptAuthTokens(_) => { let email = auth.get_account_email(); let plan = plan .map(|plan_type| title_case(format!("{plan_type:?}").as_str())) .or_else(|| Some("Unknown".to_string())); Some(StatusAccountDisplay::ChatGpt { email, plan }) } - AuthMode::ApiKey => Some(StatusAccountDisplay::ApiKey), + CodexAuth::ApiKey(_) => Some(StatusAccountDisplay::ApiKey), } } From 1286b29a65157b41758dc06935e4fdab62e5bba9 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 30 Jan 2026 09:48:33 -0800 Subject: [PATCH 18/18] feat: codex app-server --default-chatgpt-proxy-auth --- codex-rs/Cargo.lock | 1 + .../src/protocol/common.rs | 30 +++- .../app-server-protocol/src/protocol/v2.rs | 17 ++ codex-rs/app-server/Cargo.toml | 1 + .../app-server/src/codex_message_processor.rs | 148 +++++++++++++----- codex-rs/app-server/src/lib.rs | 2 + codex-rs/app-server/src/main.rs | 16 +- codex-rs/app-server/src/message_processor.rs | 22 +++ .../app-server/tests/common/auth_fixtures.rs | 1 + codex-rs/backend-client/src/client.rs | 8 +- codex-rs/cli/src/main.rs | 18 +++ codex-rs/cloud-requirements/src/lib.rs | 5 +- codex-rs/core/src/api_bridge.rs | 4 +- codex-rs/core/src/auth.rs | 113 +++++++++---- codex-rs/core/src/auth/storage.rs | 25 +++ codex-rs/core/src/mcp/mod.rs | 2 +- codex-rs/core/tests/suite/auth_refresh.rs | 10 ++ codex-rs/login/src/server.rs | 1 + codex-rs/tui/src/status/helpers.rs | 2 +- 19 files changed, 348 insertions(+), 78 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 44f35f78ae..66b50fa745 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1084,6 +1084,7 @@ dependencies = [ "axum", "base64", "chrono", + "clap", "codex-app-server-protocol", "codex-arg0", "codex-backend-client", diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 67736374ec..3b8c4ea63d 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -910,10 +910,36 @@ mod tests { Ok(()) } + #[test] + fn serialize_account_login_chatgpt_proxy() -> Result<()> { + let request = ClientRequest::LoginAccount { + request_id: RequestId::Integer(6), + params: v2::LoginAccountParams::ChatgptProxy { + account_id: Some("acc-123".to_string()), + email: Some("user@example.com".to_string()), + plan_type: Some(PlanType::Pro), + }, + }; + assert_eq!( + json!({ + "method": "account/login/start", + "id": 6, + "params": { + "type": "chatgptProxy", + "accountId": "acc-123", + "email": "user@example.com", + "planType": "pro" + } + }), + serde_json::to_value(&request)?, + ); + Ok(()) + } + #[test] fn serialize_get_account() -> Result<()> { let request = ClientRequest::GetAccount { - request_id: RequestId::Integer(6), + request_id: RequestId::Integer(7), params: v2::GetAccountParams { refresh_token: false, }, @@ -921,7 +947,7 @@ mod tests { assert_eq!( json!({ "method": "account/read", - "id": 6, + "id": 7, "params": { "refreshToken": false } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 8bea094487..ff8029cdb0 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -835,6 +835,20 @@ pub enum LoginAccountParams { #[serde(rename = "chatgpt")] #[ts(rename = "chatgpt")] Chatgpt, + /// Tokenless ChatGPT auth via a trusted proxy. + /// + /// The proxy is expected to inject the Authorization header, so Codex + /// should not set a bearer token on outgoing requests. + #[serde(rename = "chatgptProxy", rename_all = "camelCase")] + #[ts(rename = "chatgptProxy", rename_all = "camelCase")] + ChatgptProxy { + /// ChatGPT workspace/account identifier. + account_id: Option, + /// Account email address. + email: Option, + /// Account plan type. + plan_type: Option, + }, /// [UNSTABLE] FOR OPENAI INTERNAL USE ONLY - DO NOT USE. /// The access token must contain the same scopes that Codex-managed ChatGPT auth tokens have. #[serde(rename = "chatgptAuthTokens")] @@ -872,6 +886,9 @@ pub enum LoginAccountResponse { /// URL the client should open in a browser to initiate the OAuth flow. auth_url: String, }, + #[serde(rename = "chatgptProxy", rename_all = "camelCase")] + #[ts(rename = "chatgptProxy", rename_all = "camelCase")] + ChatgptProxy {}, #[serde(rename = "chatgptAuthTokens", rename_all = "camelCase")] #[ts(rename = "chatgptAuthTokens", rename_all = "camelCase")] ChatgptAuthTokens {}, diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 820edf59c2..417d40ba08 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -32,6 +32,7 @@ codex-rmcp-client = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-json-to-toml = { workspace = true } chrono = { workspace = true } +clap = { workspace = true, features = ["derive"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } mcp-types = { workspace = true } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index c02f3c7368..e09bbfdbd6 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -146,6 +146,7 @@ use codex_core::ThreadSortKey as CoreThreadSortKey; use codex_core::auth::CLIENT_ID; use codex_core::auth::login_with_api_key; use codex_core::auth::login_with_chatgpt_auth_tokens; +use codex_core::auth::login_with_chatgpt_proxy; use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::ConfigService; @@ -616,6 +617,14 @@ impl CodexMessageProcessor { LoginAccountParams::Chatgpt => { self.login_chatgpt_v2(request_id).await; } + LoginAccountParams::ChatgptProxy { + account_id, + email, + plan_type, + } => { + self.login_chatgpt_proxy(request_id, account_id, email, plan_type) + .await; + } LoginAccountParams::ChatgptAuthTokens { id_token, access_token, @@ -629,7 +638,7 @@ impl CodexMessageProcessor { fn external_auth_active_error(&self) -> JSONRPCErrorError { JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, - message: "External auth is active. Use account/login/start (chatgptAuthTokens) to update it or account/logout to clear it." + message: "External auth is active. Use account/login/start (chatgptAuthTokens or chatgptProxy) to update it or account/logout to clear it." .to_string(), data: None, } @@ -1011,6 +1020,89 @@ impl CodexMessageProcessor { } } + async fn login_chatgpt_proxy( + &mut self, + request_id: RequestId, + account_id: Option, + email: Option, + plan_type: Option, + ) { + if matches!( + self.config.forced_login_method, + Some(ForcedLoginMethod::Api) + ) { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: "ChatGPT proxy auth is disabled. Use API key login instead.".to_string(), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + + // Cancel any active login attempt to avoid persisting managed auth state. + { + let mut guard = self.active_login.lock().await; + if let Some(active) = guard.take() { + drop(active); + } + } + + if let Some(expected_workspace) = self.config.forced_chatgpt_workspace_id.as_deref() + && account_id.as_deref() != Some(expected_workspace) + { + let actual_workspace = account_id.as_deref().unwrap_or(""); + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: format!( + "ChatGPT proxy auth must use workspace {expected_workspace}, but received {actual_workspace}." + ), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + + if let Err(err) = login_with_chatgpt_proxy( + &self.config.codex_home, + account_id.as_deref(), + email.as_deref(), + plan_type, + self.config.cli_auth_credentials_store_mode, + ) { + let error = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!("failed to set ChatGPT proxy auth: {err}"), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + self.auth_manager.reload(); + + self.outgoing + .send_response(request_id, LoginAccountResponse::ChatgptProxy {}) + .await; + + let payload_login_completed = AccountLoginCompletedNotification { + login_id: None, + success: true, + error: None, + }; + self.outgoing + .send_server_notification(ServerNotification::AccountLoginCompleted( + payload_login_completed, + )) + .await; + + let payload_v2 = AccountUpdatedNotification { + auth_mode: self.auth_manager.get_auth_mode(), + }; + self.outgoing + .send_server_notification(ServerNotification::AccountUpdated(payload_v2)) + .await; + } + async fn login_chatgpt_auth_tokens( &mut self, request_id: RequestId, @@ -1198,19 +1290,16 @@ impl CodexMessageProcessor { match self.auth_manager.auth().await { Some(auth) => { let auth_mode = auth.api_auth_mode(); - let (reported_auth_method, token_opt) = match auth.get_token() { - Ok(token) if !token.is_empty() => { - let tok = if include_token { Some(token) } else { None }; - (Some(auth_mode), tok) - } - Ok(_) => (None, None), + let token_opt = match auth.bearer_token() { + Ok(Some(token)) if include_token && !token.is_empty() => Some(token), + Ok(_) => None, Err(err) => { - tracing::warn!("failed to get token for auth status: {err}"); - (None, None) + tracing::warn!("failed to get bearer token for auth status: {err}"); + None } }; GetAuthStatusResponse { - auth_method: reported_auth_method, + auth_method: Some(auth_mode), auth_token: token_opt, requires_openai_auth: Some(true), } @@ -1243,31 +1332,20 @@ impl CodexMessageProcessor { return; } - let account = match self.auth_manager.auth_cached() { - Some(auth) => Some(match auth { - CodexAuth::ApiKey(_) => Account::ApiKey {}, - CodexAuth::ChatGpt(_) | CodexAuth::ChatGptAuthTokens(_) => { - let email = auth.get_account_email(); - let plan_type = auth.account_plan_type(); - - match (email, plan_type) { - (Some(email), Some(plan_type)) => Account::Chatgpt { email, plan_type }, - _ => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: - "email and plan type are required for chatgpt authentication" - .to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } - } - } - }), - None => None, - }; + let account = self.auth_manager.auth_cached().map(|auth| match auth { + CodexAuth::ApiKey(_) => Account::ApiKey {}, + CodexAuth::ChatGpt(_) + | CodexAuth::ChatGptAuthTokens(_) + | CodexAuth::ChatGptProxy(_) => { + let email = auth + .get_account_email() + .unwrap_or_else(|| "unknown".to_string()); + let plan_type = auth + .account_plan_type() + .unwrap_or(codex_protocol::account::PlanType::Unknown); + Account::Chatgpt { email, plan_type } + } + }); let response = GetAccountResponse { account, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 5b3d39704c..221d185108 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -168,6 +168,7 @@ pub async fn run_main( cli_config_overrides: CliConfigOverrides, loader_overrides: LoaderOverrides, default_analytics_enabled: bool, + default_chatgpt_proxy_auth: bool, ) -> IoResult<()> { // Set up channels. let (incoming_tx, mut incoming_rx) = mpsc::channel::(CHANNEL_CAPACITY); @@ -296,6 +297,7 @@ pub async fn run_main( std::sync::Arc::new(config), cli_overrides, loader_overrides, + default_chatgpt_proxy_auth, feedback.clone(), config_warnings, ); diff --git a/codex-rs/app-server/src/main.rs b/codex-rs/app-server/src/main.rs index 71d6dc338c..ec7d3e808a 100644 --- a/codex-rs/app-server/src/main.rs +++ b/codex-rs/app-server/src/main.rs @@ -1,3 +1,4 @@ +use clap::Parser; use codex_app_server::run_main; use codex_arg0::arg0_dispatch_or_else; use codex_common::CliConfigOverrides; @@ -8,8 +9,20 @@ use std::path::PathBuf; // managed config file without writing to /etc. const MANAGED_CONFIG_PATH_ENV_VAR: &str = "CODEX_APP_SERVER_MANAGED_CONFIG_PATH"; +#[derive(Debug, Parser, Default, Clone)] +#[command(bin_name = "codex-app-server")] +struct AppServerCli { + #[clap(flatten)] + config_overrides: CliConfigOverrides, + + /// Seed ChatGPT proxy auth (tokenless) on startup when no auth is present. + #[arg(long = "default-chatgpt-proxy-auth")] + default_chatgpt_proxy_auth: bool, +} + fn main() -> anyhow::Result<()> { arg0_dispatch_or_else(|codex_linux_sandbox_exe| async move { + let cli = AppServerCli::parse(); let managed_config_path = managed_config_path_from_debug_env(); let loader_overrides = LoaderOverrides { managed_config_path, @@ -18,9 +31,10 @@ fn main() -> anyhow::Result<()> { run_main( codex_linux_sandbox_exe, - CliConfigOverrides::default(), + cli.config_overrides, loader_overrides, false, + cli.default_chatgpt_proxy_auth, ) .await?; Ok(()) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index ced93c3bdd..c0c38df9e0 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -26,10 +26,12 @@ use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ServerRequestPayload; use codex_core::AuthManager; use codex_core::ThreadManager; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::auth::ExternalAuthRefreshContext; use codex_core::auth::ExternalAuthRefreshReason; use codex_core::auth::ExternalAuthRefresher; use codex_core::auth::ExternalAuthTokens; +use codex_core::auth::login_with_chatgpt_proxy; use codex_core::config::Config; use codex_core::config_loader::LoaderOverrides; use codex_core::default_client::SetOriginatorError; @@ -38,11 +40,13 @@ use codex_core::default_client::get_codex_user_agent; use codex_core::default_client::set_default_originator; use codex_feedback::CodexFeedback; use codex_protocol::ThreadId; +use codex_protocol::config_types::ForcedLoginMethod; use codex_protocol::protocol::SessionSource; use tokio::sync::broadcast; use tokio::time::Duration; use tokio::time::timeout; use toml::Value as TomlValue; +use tracing::warn; const EXTERNAL_AUTH_REFRESH_TIMEOUT: Duration = Duration::from_secs(10); @@ -115,6 +119,7 @@ impl MessageProcessor { config: Arc, cli_overrides: Vec<(String, TomlValue)>, loader_overrides: LoaderOverrides, + default_chatgpt_proxy_auth: bool, feedback: CodexFeedback, config_warnings: Vec, ) -> Self { @@ -124,6 +129,23 @@ impl MessageProcessor { false, config.cli_auth_credentials_store_mode, ); + if default_chatgpt_proxy_auth + && auth_manager.auth_cached().is_none() + && !matches!(config.forced_login_method, Some(ForcedLoginMethod::Api)) + { + let account_id = config.forced_chatgpt_workspace_id.as_deref(); + if let Err(err) = login_with_chatgpt_proxy( + &config.codex_home, + account_id, + None, + None, + AuthCredentialsStoreMode::Ephemeral, + ) { + warn!("failed to seed default ChatGPT proxy auth: {err}"); + } else { + auth_manager.reload(); + } + } auth_manager.set_forced_chatgpt_workspace_id(config.forced_chatgpt_workspace_id.clone()); auth_manager.set_external_auth_refresher(Arc::new(ExternalAuthRefreshBridge { outgoing: outgoing.clone(), diff --git a/codex-rs/app-server/tests/common/auth_fixtures.rs b/codex-rs/app-server/tests/common/auth_fixtures.rs index b78d5b105d..a1751a5020 100644 --- a/codex-rs/app-server/tests/common/auth_fixtures.rs +++ b/codex-rs/app-server/tests/common/auth_fixtures.rs @@ -163,6 +163,7 @@ pub fn write_chatgpt_auth( openai_api_key: None, tokens: Some(tokens), last_refresh, + chatgpt_proxy: None, }; save_auth(codex_home, &auth, cli_auth_credentials_store_mode).context("write auth.json") diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index 6fa36d1ffd..a21857da61 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -75,10 +75,10 @@ impl Client { } pub fn from_auth(base_url: impl Into, auth: &CodexAuth) -> Result { - let token = auth.get_token().map_err(anyhow::Error::from)?; - let mut client = Self::new(base_url)? - .with_user_agent(get_codex_user_agent()) - .with_bearer_token(token); + let mut client = Self::new(base_url)?.with_user_agent(get_codex_user_agent()); + if let Some(token) = auth.bearer_token().map_err(anyhow::Error::from)? { + client = client.with_bearer_token(token); + } if let Some(account_id) = auth.get_account_id() { client = client.with_chatgpt_account_id(account_id); } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 7ab8bc937a..49343e33c8 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -283,6 +283,10 @@ struct AppServerCommand { /// See https://developers.openai.com/codex/config-advanced/#metrics for more details. #[arg(long = "analytics-default-enabled")] analytics_default_enabled: bool, + + /// Seed ChatGPT proxy auth (tokenless) on startup when no auth is present. + #[arg(long = "default-chatgpt-proxy-auth")] + default_chatgpt_proxy_auth: bool, } #[derive(Debug, clap::Subcommand)] @@ -535,6 +539,7 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() root_config_overrides, codex_core::config_loader::LoaderOverrides::default(), app_server_cli.analytics_default_enabled, + app_server_cli.default_chatgpt_proxy_auth, ) .await?; } @@ -1263,6 +1268,19 @@ mod tests { assert!(app_server.analytics_default_enabled); } + #[test] + fn app_server_default_chatgpt_proxy_auth_disabled_without_flag() { + let app_server = app_server_from_args(["codex", "app-server"].as_ref()); + assert!(!app_server.default_chatgpt_proxy_auth); + } + + #[test] + fn app_server_default_chatgpt_proxy_auth_enabled_with_flag() { + let app_server = + app_server_from_args(["codex", "app-server", "--default-chatgpt-proxy-auth"].as_ref()); + assert!(app_server.default_chatgpt_proxy_auth); + } + #[test] fn features_enable_parses_feature_name() { let cli = MultitoolCli::try_parse_from(["codex", "features", "enable", "unified_exec"]) diff --git a/codex-rs/cloud-requirements/src/lib.rs b/codex-rs/cloud-requirements/src/lib.rs index ccfa600319..800d5313f5 100644 --- a/codex-rs/cloud-requirements/src/lib.rs +++ b/codex-rs/cloud-requirements/src/lib.rs @@ -8,7 +8,6 @@ //! requirements before Codex will run. use async_trait::async_trait; -use codex_app_server_protocol::AuthMode; use codex_backend_client::Client as BackendClient; use codex_core::AuthManager; use codex_core::auth::CodexAuth; @@ -120,9 +119,7 @@ impl CloudRequirementsService { async fn fetch(&self) -> Option { let auth = self.auth_manager.auth().await?; - if !(auth.mode == AuthMode::ChatGPT - && auth.account_plan_type() == Some(PlanType::Enterprise)) - { + if !(auth.is_chatgpt_auth() && auth.account_plan_type() == Some(PlanType::Enterprise)) { return None; } diff --git a/codex-rs/core/src/api_bridge.rs b/codex-rs/core/src/api_bridge.rs index ec21f1ec85..b1f0c49c67 100644 --- a/codex-rs/core/src/api_bridge.rs +++ b/codex-rs/core/src/api_bridge.rs @@ -177,9 +177,9 @@ pub(crate) fn auth_provider_from_auth( } if let Some(auth) = auth { - let token = auth.get_token()?; + let token = auth.bearer_token()?; Ok(CoreAuthProvider { - token: Some(token), + token, account_id: auth.get_account_id(), }) } else { diff --git a/codex-rs/core/src/auth.rs b/codex-rs/core/src/auth.rs index 4bddbe3031..6923fb2e53 100644 --- a/codex-rs/core/src/auth.rs +++ b/codex-rs/core/src/auth.rs @@ -21,6 +21,7 @@ use codex_protocol::config_types::ForcedLoginMethod; pub use crate::auth::storage::AuthCredentialsStoreMode; pub use crate::auth::storage::AuthDotJson; use crate::auth::storage::AuthStorageBackend; +use crate::auth::storage::ChatGptProxyAccount; use crate::auth::storage::create_auth_storage; use crate::config::Config; use crate::error::RefreshTokenFailedError; @@ -53,6 +54,7 @@ pub enum CodexAuth { ApiKey(ApiKeyAuth), ChatGpt(ChatGptAuth), ChatGptAuthTokens(ChatGptAuthTokens), + ChatGptProxy(ChatGptProxy), } #[derive(Debug, Clone)] @@ -71,6 +73,11 @@ pub struct ChatGptAuthTokens { state: ChatGptAuthState, } +#[derive(Debug, Clone)] +pub struct ChatGptProxy { + account: ChatGptProxyAccount, +} + #[derive(Debug, Clone)] struct ChatGptAuthState { auth_dot_json: Arc>>, @@ -160,6 +167,12 @@ impl CodexAuth { return Ok(CodexAuth::from_api_key_with_client(api_key, client)); } + if let Some(proxy_account) = auth_dot_json.chatgpt_proxy.clone() { + return Ok(Self::ChatGptProxy(ChatGptProxy { + account: proxy_account, + })); + } + let storage_mode = auth_dot_json.storage_mode(auth_credentials_store_mode); let state = ChatGptAuthState { auth_dot_json: Arc::new(Mutex::new(Some(auth_dot_json))), @@ -189,14 +202,16 @@ impl CodexAuth { pub fn internal_auth_mode(&self) -> AuthMode { match self { Self::ApiKey(_) => AuthMode::ApiKey, - Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) => AuthMode::ChatGPT, + Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) | Self::ChatGptProxy(_) => { + AuthMode::ChatGPT + } } } pub fn api_auth_mode(&self) -> ApiAuthMode { match self { Self::ApiKey(_) => ApiAuthMode::ApiKey, - Self::ChatGpt(_) => ApiAuthMode::ChatGPT, + Self::ChatGpt(_) | Self::ChatGptProxy(_) => ApiAuthMode::ChatGPT, Self::ChatGptAuthTokens(_) => ApiAuthMode::ChatgptAuthTokens, } } @@ -213,7 +228,7 @@ impl CodexAuth { pub fn api_key(&self) -> Option<&str> { match self { Self::ApiKey(auth) => Some(auth.api_key.as_str()), - Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) => None, + Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) | Self::ChatGptProxy(_) => None, } } @@ -230,25 +245,40 @@ impl CodexAuth { } } - /// Returns the token string used for bearer authentication. - pub fn get_token(&self) -> Result { + /// Returns the token string used for bearer authentication, if available. + pub fn bearer_token(&self) -> Result, std::io::Error> { match self { - Self::ApiKey(auth) => Ok(auth.api_key.clone()), + Self::ApiKey(auth) => Ok(Some(auth.api_key.clone())), Self::ChatGpt(_) | Self::ChatGptAuthTokens(_) => { let access_token = self.get_token_data()?.access_token; - Ok(access_token) + Ok(Some(access_token)) } + Self::ChatGptProxy(_) => Ok(None), } } + /// Returns the token string used for bearer authentication. + pub fn get_token(&self) -> Result { + let Some(token) = self.bearer_token()? else { + return Err(std::io::Error::other("Bearer token is not available.")); + }; + Ok(token) + } + /// Returns `None` if `is_chatgpt_auth()` is false. pub fn get_account_id(&self) -> Option { - self.get_current_token_data().and_then(|t| t.account_id) + match self { + Self::ChatGptProxy(proxy) => proxy.account.account_id.clone(), + _ => self.get_current_token_data().and_then(|t| t.account_id), + } } /// Returns `None` if `is_chatgpt_auth()` is false. pub fn get_account_email(&self) -> Option { - self.get_current_token_data().and_then(|t| t.id_token.email) + match self { + Self::ChatGptProxy(proxy) => proxy.account.email.clone(), + _ => self.get_current_token_data().and_then(|t| t.id_token.email), + } } /// Account-facing plan classification derived from the current token. @@ -256,6 +286,9 @@ impl CodexAuth { /// mapped from the ID token's internal plan value. Prefer this when you /// need to make UI or product decisions based on the user's subscription. pub fn account_plan_type(&self) -> Option { + if let Self::ChatGptProxy(proxy) = self { + return proxy.account.plan_type; + } let map_known = |kp: &InternalKnownPlan| match kp { InternalKnownPlan::Free => AccountPlanType::Free, InternalKnownPlan::Go => AccountPlanType::Go, @@ -275,12 +308,22 @@ impl CodexAuth { }) } + /// Returns the ChatGPT workspace/account identifier when available. + pub fn chatgpt_workspace_id(&self) -> Option { + match self { + Self::ChatGptProxy(proxy) => proxy.account.account_id.clone(), + _ => self + .get_current_token_data() + .and_then(|t| t.id_token.chatgpt_account_id.or(t.account_id)), + } + } + /// Returns `None` if `is_chatgpt_auth()` is false. fn get_current_auth_json(&self) -> Option { let state = match self { Self::ChatGpt(auth) => &auth.state, Self::ChatGptAuthTokens(auth) => &auth.state, - Self::ApiKey(_) => return None, + Self::ApiKey(_) | Self::ChatGptProxy(_) => return None, }; #[expect(clippy::unwrap_used)] state.auth_dot_json.lock().unwrap().clone() @@ -303,6 +346,7 @@ impl CodexAuth { account_id: Some("account_id".to_string()), }), last_refresh: Some(Utc::now()), + chatgpt_proxy: None, }; let client = crate::default_client::create_client(); @@ -382,6 +426,7 @@ pub fn login_with_api_key( openai_api_key: Some(api_key.to_string()), tokens: None, last_refresh: None, + chatgpt_proxy: None, }; save_auth(codex_home, &auth_dot_json, auth_credentials_store_mode) } @@ -400,6 +445,28 @@ pub fn login_with_chatgpt_auth_tokens( ) } +/// Writes a tokenless ChatGPT proxy auth payload. +pub fn login_with_chatgpt_proxy( + codex_home: &Path, + account_id: Option<&str>, + email: Option<&str>, + plan_type: Option, + auth_credentials_store_mode: AuthCredentialsStoreMode, +) -> std::io::Result<()> { + let auth_dot_json = AuthDotJson { + auth_mode: Some(ApiAuthMode::ChatGPT), + openai_api_key: None, + tokens: None, + last_refresh: None, + chatgpt_proxy: Some(ChatGptProxyAccount { + account_id: account_id.map(str::to_string), + email: email.map(str::to_string), + plan_type, + }), + }; + save_auth(codex_home, &auth_dot_json, auth_credentials_store_mode) +} + /// Persist the provided auth payload using the specified backend. pub fn save_auth( codex_home: &Path, @@ -461,23 +528,10 @@ pub fn enforce_login_restrictions(config: &Config) -> std::io::Result<()> { return Ok(()); } - let token_data = match auth.get_token_data() { - Ok(data) => data, - Err(err) => { - return logout_with_message( - &config.codex_home, - format!( - "Failed to load ChatGPT credentials while enforcing workspace restrictions: {err}. Logging out." - ), - config.cli_auth_credentials_store_mode, - ); - } - }; - - // workspace is the external identifier for account id. - let chatgpt_account_id = token_data.id_token.chatgpt_account_id.as_deref(); - if chatgpt_account_id != Some(expected_account_id) { - let message = match chatgpt_account_id { + // Workspace is the external identifier for account id. + let chatgpt_account_id = auth.chatgpt_workspace_id(); + if chatgpt_account_id.as_deref() != Some(expected_account_id) { + let message = match chatgpt_account_id.as_deref() { Some(actual) => format!( "Login is restricted to workspace {expected_account_id}, but current credentials belong to {actual}. Logging out." ), @@ -731,6 +785,7 @@ impl AuthDotJson { openai_api_key: None, tokens: Some(tokens), last_refresh: Some(Utc::now()), + chatgpt_proxy: None, } } @@ -1143,7 +1198,7 @@ impl AuthManager { self.reload(); Ok(()) } - CodexAuth::ApiKey(_) => Ok(()), + CodexAuth::ApiKey(_) | CodexAuth::ChatGptProxy(_) => Ok(()), } } @@ -1399,6 +1454,7 @@ mod tests { account_id: None, }), last_refresh: Some(last_refresh), + chatgpt_proxy: None, }, auth_dot_json ); @@ -1432,6 +1488,7 @@ mod tests { openai_api_key: Some("sk-test-key".to_string()), tokens: None, last_refresh: None, + chatgpt_proxy: None, }; super::save_auth(dir.path(), &auth_dot_json, AuthCredentialsStoreMode::File)?; let auth_file = get_auth_file(dir.path()); diff --git a/codex-rs/core/src/auth/storage.rs b/codex-rs/core/src/auth/storage.rs index b4f4bb8e7e..4ed00ddb5e 100644 --- a/codex-rs/core/src/auth/storage.rs +++ b/codex-rs/core/src/auth/storage.rs @@ -23,6 +23,7 @@ use crate::token_data::TokenData; use codex_app_server_protocol::AuthMode; use codex_keyring_store::DefaultKeyringStore; use codex_keyring_store::KeyringStore; +use codex_protocol::account::PlanType as AccountPlanType; use once_cell::sync::Lazy; /// Determine where Codex should store CLI auth credentials. @@ -54,6 +55,23 @@ pub struct AuthDotJson { #[serde(default, skip_serializing_if = "Option::is_none")] pub last_refresh: Option>, + + /// ChatGPT account metadata supplied by a trusted proxy. + #[serde( + default, + skip_serializing_if = "Option::is_none", + rename = "chatgptProxy" + )] + pub chatgpt_proxy: Option, +} + +/// Account metadata for the tokenless ChatGPT proxy auth mode. +#[derive(Deserialize, Serialize, Clone, Debug, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct ChatGptProxyAccount { + pub account_id: Option, + pub email: Option, + pub plan_type: Option, } pub(super) fn get_auth_file(codex_home: &Path) -> PathBuf { @@ -353,6 +371,7 @@ mod tests { openai_api_key: Some("test-key".to_string()), tokens: None, last_refresh: Some(Utc::now()), + chatgpt_proxy: None, }; storage @@ -373,6 +392,7 @@ mod tests { openai_api_key: Some("test-key".to_string()), tokens: None, last_refresh: Some(Utc::now()), + chatgpt_proxy: None, }; let file = get_auth_file(codex_home.path()); @@ -395,6 +415,7 @@ mod tests { openai_api_key: Some("sk-test-key".to_string()), tokens: None, last_refresh: None, + chatgpt_proxy: None, }; let storage = create_auth_storage(dir.path().to_path_buf(), AuthCredentialsStoreMode::File); storage.save(&auth_dot_json)?; @@ -418,6 +439,7 @@ mod tests { openai_api_key: Some("sk-ephemeral".to_string()), tokens: None, last_refresh: Some(Utc::now()), + chatgpt_proxy: None, }; storage.save(&auth_dot_json)?; @@ -516,6 +538,7 @@ mod tests { account_id: Some(format!("{prefix}-account-id")), }), last_refresh: None, + chatgpt_proxy: None, } } @@ -532,6 +555,7 @@ mod tests { openai_api_key: Some("sk-test".to_string()), tokens: None, last_refresh: None, + chatgpt_proxy: None, }; seed_keyring_with_auth( &mock_keyring, @@ -574,6 +598,7 @@ mod tests { account_id: Some("account".to_string()), }), last_refresh: Some(Utc::now()), + chatgpt_proxy: None, }; storage.save(&auth)?; diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 12c61ddaf1..613e0b1e92 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -39,7 +39,7 @@ fn codex_apps_mcp_bearer_token_env_var() -> Option { } fn codex_apps_mcp_bearer_token(auth: Option<&CodexAuth>) -> Option { - let token = auth.and_then(|auth| auth.get_token().ok())?; + let token = auth.and_then(|auth| auth.bearer_token().ok()).flatten()?; let token = token.trim(); if token.is_empty() { None diff --git a/codex-rs/core/tests/suite/auth_refresh.rs b/codex-rs/core/tests/suite/auth_refresh.rs index 4ef3b82eec..e768fff10d 100644 --- a/codex-rs/core/tests/suite/auth_refresh.rs +++ b/codex-rs/core/tests/suite/auth_refresh.rs @@ -55,6 +55,7 @@ async fn refresh_token_succeeds_updates_storage() -> Result<()> { openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -117,6 +118,7 @@ async fn returns_fresh_tokens_as_is() -> Result<()> { openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -163,6 +165,7 @@ async fn refreshes_token_when_last_refresh_is_stale() -> Result<()> { openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(stale_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -222,6 +225,7 @@ async fn refresh_token_returns_permanent_error_for_expired_refresh_token() -> Re openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -272,6 +276,7 @@ async fn refresh_token_returns_transient_error_on_server_failure() -> Result<()> openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -324,6 +329,7 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> { openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -333,6 +339,7 @@ async fn unauthorized_recovery_reloads_then_refreshes_tokens() -> Result<()> { openai_api_key: None, tokens: Some(disk_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; save_auth( ctx.codex_home.path(), @@ -416,6 +423,7 @@ async fn unauthorized_recovery_skips_reload_on_account_mismatch() -> Result<()> openai_api_key: None, tokens: Some(initial_tokens.clone()), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; ctx.write_auth(&initial_auth)?; @@ -431,6 +439,7 @@ async fn unauthorized_recovery_skips_reload_on_account_mismatch() -> Result<()> openai_api_key: None, tokens: Some(disk_tokens), last_refresh: Some(initial_last_refresh), + chatgpt_proxy: None, }; save_auth( ctx.codex_home.path(), @@ -495,6 +504,7 @@ async fn unauthorized_recovery_requires_chatgpt_auth() -> Result<()> { openai_api_key: Some("sk-test".to_string()), tokens: None, last_refresh: None, + chatgpt_proxy: None, }; ctx.write_auth(&auth)?; diff --git a/codex-rs/login/src/server.rs b/codex-rs/login/src/server.rs index 5272c84eaa..746850e222 100644 --- a/codex-rs/login/src/server.rs +++ b/codex-rs/login/src/server.rs @@ -564,6 +564,7 @@ pub(crate) async fn persist_tokens_async( openai_api_key: api_key, tokens: Some(tokens), last_refresh: Some(Utc::now()), + chatgpt_proxy: None, }; save_auth(&codex_home, &auth, auth_credentials_store_mode) }) diff --git a/codex-rs/tui/src/status/helpers.rs b/codex-rs/tui/src/status/helpers.rs index 0a801227f5..9c7e8b6b13 100644 --- a/codex-rs/tui/src/status/helpers.rs +++ b/codex-rs/tui/src/status/helpers.rs @@ -91,7 +91,7 @@ pub(crate) fn compose_account_display( let auth = auth_manager.auth_cached()?; match auth { - CodexAuth::ChatGpt(_) | CodexAuth::ChatGptAuthTokens(_) => { + CodexAuth::ChatGpt(_) | CodexAuth::ChatGptAuthTokens(_) | CodexAuth::ChatGptProxy(_) => { let email = auth.get_account_email(); let plan = plan .map(|plan_type| title_case(format!("{plan_type:?}").as_str()))