mirror of
https://github.com/openai/codex.git
synced 2026-05-17 17:53:06 +00:00
Trim TUI legacy core helper usage (#22695)
## Why The TUI still had a few low-risk dependencies flowing through the transitional `legacy_core` namespace after the app-server migration. These helpers either already have clearer non-core owners or are presentation logic that does not belong in `codex-core`, so moving them out reduces the compatibility surface without changing product behavior. ## What changed This is a low-risk change, almost completely mechanical in nature. - Route TUI Codex-home lookup through `codex-utils-home-dir`, use `Config::log_dir` directly, and call `codex-sandboxing::system_bwrap_warning` without going through `legacy_core`. - Move shared `codex resume` hint formatting from `codex-core` into `codex-utils-cli`. - Update CLI and TUI call sites to use the shared CLI utility, and keep the resume-command behavior covered by tests in its new home. ## Verification - `cargo test -p codex-utils-cli` - `cargo test -p codex-utils-cli resume_command`
This commit is contained in:
3
codex-rs/Cargo.lock
generated
3
codex-rs/Cargo.lock
generated
@@ -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",
|
||||
|
||||
@@ -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<Stri
|
||||
lines.push(token_usage.to_string());
|
||||
}
|
||||
|
||||
if let Some(resume_cmd) =
|
||||
codex_core::util::resume_command(/*thread_name*/ None, conversation_id)
|
||||
{
|
||||
if let Some(resume_cmd) = resume_command(/*thread_name*/ None, conversation_id) {
|
||||
let command = if color_enabled {
|
||||
resume_cmd.cyan().to_string()
|
||||
} else {
|
||||
|
||||
@@ -2,12 +2,9 @@ use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_protocol::ThreadId;
|
||||
use rand::Rng;
|
||||
use tracing::error;
|
||||
|
||||
use codex_shell_command::parse_command::shlex_join;
|
||||
|
||||
const INITIAL_DELAY_MS: u64 = 200;
|
||||
const BACKOFF_FACTOR: f64 = 2.0;
|
||||
|
||||
@@ -118,22 +115,6 @@ pub fn normalize_thread_name(name: &str) -> Option<String> {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn resume_command(thread_name: Option<&str>, thread_id: Option<ThreadId>) -> Option<String> {
|
||||
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;
|
||||
|
||||
@@ -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()));
|
||||
}
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
};
|
||||
|
||||
|
||||
@@ -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<ThreadId>) -> 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(),
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<String> {
|
||||
@@ -22,9 +22,7 @@ fn format_exit_messages(exit_info: AppExitInfo, color_enabled: bool) -> Vec<Stri
|
||||
lines.push(token_usage.to_string());
|
||||
}
|
||||
|
||||
if let Some(resume_cmd) =
|
||||
legacy_core::util::resume_command(/*thread_name*/ None, thread_id)
|
||||
{
|
||||
if let Some(resume_cmd) = resume_command(/*thread_name*/ None, thread_id) {
|
||||
let command = if color_enabled {
|
||||
format!("\u{1b}[36m{resume_cmd}\u{1b}[39m")
|
||||
} else {
|
||||
|
||||
@@ -88,10 +88,7 @@ pub(crate) fn maybe_init(config: &Config) {
|
||||
let path = if let Ok(path) = std::env::var("CODEX_TUI_SESSION_LOG_PATH") {
|
||||
PathBuf::from(path)
|
||||
} else {
|
||||
let mut p = match crate::legacy_core::config::log_dir(config) {
|
||||
Ok(dir) => 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")
|
||||
|
||||
@@ -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 }
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
64
codex-rs/utils/cli/src/resume_command.rs
Normal file
64
codex-rs/utils/cli/src/resume_command.rs
Normal file
@@ -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<ThreadId>) -> Option<String> {
|
||||
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()));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user