From 635ff02cf228e3f6e67d3c58d5aeb666f720037c Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 14 May 2026 14:02:14 -0700 Subject: [PATCH] tui: reduce legacy core helper usage --- codex-rs/Cargo.lock | 3 + codex-rs/cli/src/main.rs | 5 +- codex-rs/core/src/util.rs | 19 ------ codex-rs/core/src/util_tests.rs | 38 ----------- codex-rs/tui/Cargo.toml | 2 + codex-rs/tui/src/app.rs | 3 +- codex-rs/tui/src/app/startup_prompts.rs | 6 +- codex-rs/tui/src/chatwidget.rs | 5 +- .../tui/src/chatwidget/settings_popups.rs | 2 +- codex-rs/tui/src/lib.rs | 4 +- codex-rs/tui/src/main.rs | 6 +- codex-rs/tui/src/session_log.rs | 5 +- codex-rs/utils/cli/Cargo.toml | 1 + codex-rs/utils/cli/src/lib.rs | 2 + codex-rs/utils/cli/src/resume_command.rs | 64 +++++++++++++++++++ 15 files changed, 87 insertions(+), 78 deletions(-) create mode 100644 codex-rs/utils/cli/src/resume_command.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 20197567abed..4d1f66008fdf 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3785,6 +3785,7 @@ dependencies = [ "codex-protocol", "codex-realtime-webrtc", "codex-rollout", + "codex-sandboxing", "codex-shell-command", "codex-state", "codex-terminal-detection", @@ -3794,6 +3795,7 @@ dependencies = [ "codex-utils-cli", "codex-utils-elapsed", "codex-utils-fuzzy-match", + "codex-utils-home-dir", "codex-utils-oss", "codex-utils-path", "codex-utils-plugins", @@ -3913,6 +3915,7 @@ version = "0.0.0" dependencies = [ "clap", "codex-protocol", + "codex-shell-command", "pretty_assertions", "serde", "toml 0.9.11+spec-1.1.0", diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 6e1ccac8673f..de5f52f4e76c 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -38,6 +38,7 @@ use codex_tui::UpdateAction; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_cli::CliConfigOverrides; use codex_utils_cli::ProfileV2Name; +use codex_utils_cli::resume_command; use owo_colors::OwoColorize; use std::io::IsTerminal; use std::path::PathBuf; @@ -628,9 +629,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec Option { } } -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)] #[path = "util_tests.rs"] mod tests; diff --git a/codex-rs/core/src/util_tests.rs b/codex-rs/core/src/util_tests.rs index 48dd6fc9a0a0..fdc753515973 100644 --- a/codex-rs/core/src/util_tests.rs +++ b/codex-rs/core/src/util_tests.rs @@ -432,41 +432,3 @@ fn normalize_thread_name_trims_and_rejects_empty() { 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(/*thread_name*/ 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(/*thread_name*/ None, /*thread_id*/ None); - assert_eq!(command, None); -} - -#[test] -fn resume_command_quotes_thread_name_when_needed() { - let command = resume_command(Some("-starts-with-dash"), /*thread_id*/ None); - assert_eq!( - command, - Some("codex resume -- -starts-with-dash".to_string()) - ); - - let command = resume_command(Some("two words"), /*thread_id*/ None); - assert_eq!(command, Some("codex resume 'two words'".to_string())); - - let command = resume_command(Some("quote'case"), /*thread_id*/ None); - assert_eq!(command, Some("codex resume \"quote'case\"".to_string())); -} diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index abd1ccb5c695..c213e92bae41 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -52,6 +52,7 @@ codex-plugin = { workspace = true } codex-protocol = { workspace = true } codex-realtime-webrtc = { workspace = true } codex-rollout = { workspace = true } +codex-sandboxing = { workspace = true } codex-shell-command = { workspace = true } codex-state = { workspace = true } codex-terminal-detection = { workspace = true } @@ -60,6 +61,7 @@ codex-utils-absolute-path = { workspace = true } codex-utils-cli = { workspace = true } codex-utils-elapsed = { workspace = true } codex-utils-fuzzy-match = { workspace = true } +codex-utils-home-dir = { workspace = true } codex-utils-oss = { workspace = true } codex-utils-path = { workspace = true } codex-utils-plugins = { workspace = true } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7a74e0f95636..77154d3e5a35 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -397,8 +397,7 @@ fn session_summary( let usage_line = (!token_usage.is_zero()).then(|| token_usage.to_string()); let thread_id = resumable_thread(thread_id, thread_name, rollout_path).map(|thread| thread.thread_id); - let resume_command = - crate::legacy_core::util::resume_command(/*thread_name*/ None, thread_id); + let resume_command = codex_utils_cli::resume_command(/*thread_name*/ None, thread_id); if usage_line.is_none() && resume_command.is_none() { return None; diff --git a/codex-rs/tui/src/app/startup_prompts.rs b/codex-rs/tui/src/app/startup_prompts.rs index aa281d453325..c04c16fdc80e 100644 --- a/codex-rs/tui/src/app/startup_prompts.rs +++ b/codex-rs/tui/src/app/startup_prompts.rs @@ -66,9 +66,9 @@ pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config } pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) { - let Some(message) = crate::legacy_core::config::system_bwrap_warning( - config.permissions.permission_profile.get(), - ) else { + let Some(message) = + codex_sandboxing::system_bwrap_warning(config.permissions.permission_profile.get()) + else { return; }; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 57adb065b988..5c7d0194ef49 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -165,6 +165,7 @@ use codex_terminal_detection::TerminalInfo; use codex_terminal_detection::TerminalName; use codex_terminal_detection::terminal_info; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_cli::resume_command; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -1433,8 +1434,8 @@ impl ChatWidget { } fn rename_confirmation_cell(name: &str, thread_id: Option) -> PlainHistoryCell { - let resume_cmd = crate::legacy_core::util::resume_command(Some(name), thread_id) - .unwrap_or_else(|| format!("codex resume {name}")); + let resume_cmd = + resume_command(Some(name), thread_id).unwrap_or_else(|| format!("codex resume {name}")); let name = name.to_string(); let line = vec![ "• ".into(), diff --git a/codex-rs/tui/src/chatwidget/settings_popups.rs b/codex-rs/tui/src/chatwidget/settings_popups.rs index cdd38ad3a8a4..d718e70d790e 100644 --- a/codex-rs/tui/src/chatwidget/settings_popups.rs +++ b/codex-rs/tui/src/chatwidget/settings_popups.rs @@ -7,7 +7,7 @@ use super::*; impl ChatWidget { pub(super) fn open_theme_picker(&mut self) { - let codex_home = crate::legacy_core::config::find_codex_home().ok(); + let codex_home = codex_utils_home_dir::find_codex_home().ok(); let terminal_width = self .last_rendered_width .get() diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 32ca6617f74f..47ff11d3741b 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -7,7 +7,6 @@ use crate::legacy_core::check_execpolicy_for_warnings; use crate::legacy_core::config::Config; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; -use crate::legacy_core::config::find_codex_home; use crate::legacy_core::config::load_config_as_toml_with_cli_and_load_options; use crate::legacy_core::config::resolve_oss_provider; use crate::legacy_core::config::resolve_profile_v2_config_path; @@ -56,6 +55,7 @@ use codex_rollout::state_db; use codex_state::log_db; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_existing_preserving_symlinks; +use codex_utils_home_dir::find_codex_home; use codex_utils_oss::ensure_oss_provider_ready; use codex_utils_oss::get_default_model_for_oss_provider; use color_eyre::eyre::WrapErr; @@ -1073,7 +1073,7 @@ pub async fn run_main( } } - let log_dir = crate::legacy_core::config::log_dir(&config)?; + let log_dir = config.log_dir.clone(); std::fs::create_dir_all(&log_dir)?; // Open (or create) your log file, appending to it. let mut log_file_opts = OpenOptions::new(); diff --git a/codex-rs/tui/src/main.rs b/codex-rs/tui/src/main.rs index 8574b16f8a36..e41d5f5a7df9 100644 --- a/codex-rs/tui/src/main.rs +++ b/codex-rs/tui/src/main.rs @@ -1,5 +1,4 @@ use clap::Parser; -use codex_app_server_client::legacy_core; use codex_arg0::Arg0DispatchPaths; use codex_arg0::arg0_dispatch_or_else; use codex_config::LoaderOverrides; @@ -8,6 +7,7 @@ use codex_tui::Cli; use codex_tui::ExitReason; use codex_tui::run_main; use codex_utils_cli::CliConfigOverrides; +use codex_utils_cli::resume_command; use supports_color::Stream; fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec { @@ -22,9 +22,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec dir, - Err(_) => std::env::temp_dir(), - }; + let mut p = config.log_dir.clone(); let filename = format!( "session-{}.jsonl", chrono::Utc::now().format("%Y%m%dT%H%M%SZ") diff --git a/codex-rs/utils/cli/Cargo.toml b/codex-rs/utils/cli/Cargo.toml index 1ade005e3b86..9d330c5ce6ba 100644 --- a/codex-rs/utils/cli/Cargo.toml +++ b/codex-rs/utils/cli/Cargo.toml @@ -10,6 +10,7 @@ workspace = true [dependencies] clap = { workspace = true, features = ["derive", "wrap_help"] } codex-protocol = { workspace = true } +codex-shell-command = { workspace = true } serde = { workspace = true } toml = { workspace = true } diff --git a/codex-rs/utils/cli/src/lib.rs b/codex-rs/utils/cli/src/lib.rs index 98d2318d9b7a..32ef52e09526 100644 --- a/codex-rs/utils/cli/src/lib.rs +++ b/codex-rs/utils/cli/src/lib.rs @@ -1,6 +1,7 @@ mod approval_mode_cli_arg; mod config_override; pub(crate) mod format_env_display; +mod resume_command; mod sandbox_mode_cli_arg; mod shared_options; @@ -8,5 +9,6 @@ pub use approval_mode_cli_arg::ApprovalModeCliArg; pub use codex_protocol::config_types::ProfileV2Name; pub use config_override::CliConfigOverrides; pub use format_env_display::format_env_display; +pub use resume_command::resume_command; pub use sandbox_mode_cli_arg::SandboxModeCliArg; pub use shared_options::SharedCliOptions; diff --git a/codex-rs/utils/cli/src/resume_command.rs b/codex-rs/utils/cli/src/resume_command.rs new file mode 100644 index 000000000000..52b88e0e3355 --- /dev/null +++ b/codex-rs/utils/cli/src/resume_command.rs @@ -0,0 +1,64 @@ +//! Shared formatting for user-facing `codex resume` command hints. + +use codex_protocol::ThreadId; +use codex_shell_command::parse_command::shlex_join; + +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::*; + use pretty_assertions::assert_eq; + + #[test] + fn 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 formats_thread_id_when_name_is_missing() { + let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let command = resume_command(/*thread_name*/ None, Some(thread_id)); + assert_eq!( + command, + Some("codex resume 123e4567-e89b-12d3-a456-426614174000".to_string()) + ); + } + + #[test] + fn returns_none_without_a_resume_target() { + let command = resume_command(/*thread_name*/ None, /*thread_id*/ None); + assert_eq!(command, None); + } + + #[test] + fn quotes_thread_names_when_needed() { + let command = resume_command(Some("-starts-with-dash"), /*thread_id*/ None); + assert_eq!( + command, + Some("codex resume -- -starts-with-dash".to_string()) + ); + + let command = resume_command(Some("two words"), /*thread_id*/ None); + assert_eq!(command, Some("codex resume 'two words'".to_string())); + + let command = resume_command(Some("quote'case"), /*thread_id*/ None); + assert_eq!(command, Some("codex resume \"quote'case\"".to_string())); + } +}