From eba0e32909a90a0c10d7078ae91592acd6db1b58 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 27 May 2025 10:06:41 -0700 Subject: [PATCH 1/3] fix: update install_native_deps.sh to pick up the latest release (#1136) --- codex-cli/scripts/install_native_deps.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-cli/scripts/install_native_deps.sh b/codex-cli/scripts/install_native_deps.sh index 5275627f6e..09c1553228 100755 --- a/codex-cli/scripts/install_native_deps.sh +++ b/codex-cli/scripts/install_native_deps.sh @@ -65,7 +65,7 @@ mkdir -p "$BIN_DIR" # Until we start publishing stable GitHub releases, we have to grab the binaries # from the GitHub Action that created them. Update the URL below to point to the # appropriate workflow run: -WORKFLOW_URL="https://github.com/openai/codex/actions/runs/15192425904" +WORKFLOW_URL="https://github.com/openai/codex/actions/runs/15280451034" WORKFLOW_ID="${WORKFLOW_URL##*/}" ARTIFACTS_DIR="$(mktemp -d)" From d60f350cf8b7dc361f9c0b3ab87daac86c79cd98 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 27 May 2025 23:11:44 -0700 Subject: [PATCH 2/3] feat: add support for -c/--config to override individual config items (#1137) This PR introduces support for `-c`/`--config` so users can override individual config values on the command line using `--config name=value`. Example: ``` codex --config model=o4-mini ``` Making it possible to set arbitrary config values on the command line results in a more flexible configuration scheme and makes it easier to provide single-line examples that can be copy-pasted from documentation. Effectively, it means there are four levels of configuration for some values: - Default value (e.g., `model` currently defaults to `o4-mini`) - Value in `config.toml` (e.g., user could override the default to be `model = "o3"` in their `config.toml`) - Specifying `-c` or `--config` to override `model` (e.g., user can include `-c model=o3` in their list of args to Codex) - If available, a config-specific flag can be used, which takes precedence over `-c` (e.g., user can specify `--model o3` in their list of args to Codex) Now that it is possible to specify anything that could be configured in `config.toml` on the command line using `-c`, we do not need to have a custom flag for every possible config option (which can clutter the output of `--help`). To that end, as part of this PR, we drop support for the `--disable-response-storage` flag, as users can now specify `-c disable_response_storage=true` to get the equivalent functionality. Under the hood, this works by loading the `config.toml` into a `toml::Value`. Then for each `key=value`, we create a small synthetic TOML file with `value` so that we can run the TOML parser to get the equivalent `toml::Value`. We then parse `key` to determine the point in the original `toml::Value` to do the insert/replace. Once all of the overrides from `-c` args have been applied, the `toml::Value` is deserialized into a `ConfigToml` and then the `ConfigOverrides` are applied, as before. --- codex-rs/Cargo.lock | 3 + codex-rs/cli/src/debug_sandbox.rs | 21 ++- codex-rs/cli/src/lib.rs | 7 + codex-rs/cli/src/main.rs | 35 +++- codex-rs/cli/src/proto.rs | 15 +- codex-rs/common/Cargo.toml | 4 +- codex-rs/common/src/config_override.rs | 170 +++++++++++++++++++ codex-rs/common/src/lib.rs | 6 + codex-rs/core/src/config.rs | 147 +++++++++++----- codex-rs/core/src/config_types.rs | 2 +- codex-rs/exec/src/cli.rs | 6 +- codex-rs/exec/src/lib.rs | 18 +- codex-rs/exec/src/main.rs | 21 ++- codex-rs/mcp-server/Cargo.toml | 1 + codex-rs/mcp-server/src/codex_tool_config.rs | 37 ++-- codex-rs/mcp-server/src/json_to_toml.rs | 84 +++++++++ codex-rs/mcp-server/src/lib.rs | 1 + codex-rs/tui/src/cli.rs | 6 +- codex-rs/tui/src/lib.rs | 17 +- codex-rs/tui/src/main.rs | 19 ++- 20 files changed, 522 insertions(+), 98 deletions(-) create mode 100644 codex-rs/common/src/config_override.rs create mode 100644 codex-rs/mcp-server/src/json_to_toml.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 309c671e74..8f1762cac6 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -506,6 +506,8 @@ version = "0.0.0" dependencies = [ "clap", "codex-core", + "serde", + "toml", ] [[package]] @@ -634,6 +636,7 @@ dependencies = [ "serde", "serde_json", "tokio", + "toml", "tracing", "tracing-subscriber", ] diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index c09cee020a..deacca5f28 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -1,5 +1,6 @@ use std::path::PathBuf; +use codex_common::CliConfigOverrides; use codex_common::SandboxPermissionOption; use codex_core::config::Config; use codex_core::config::ConfigOverrides; @@ -20,12 +21,14 @@ pub async fn run_command_under_seatbelt( let SeatbeltCommand { full_auto, sandbox, + config_overrides, command, } = command; run_command_under_sandbox( full_auto, sandbox, command, + config_overrides, codex_linux_sandbox_exe, SandboxType::Seatbelt, ) @@ -39,12 +42,14 @@ pub async fn run_command_under_landlock( let LandlockCommand { full_auto, sandbox, + config_overrides, command, } = command; run_command_under_sandbox( full_auto, sandbox, command, + config_overrides, codex_linux_sandbox_exe, SandboxType::Landlock, ) @@ -60,16 +65,22 @@ async fn run_command_under_sandbox( full_auto: bool, sandbox: SandboxPermissionOption, command: Vec, + config_overrides: CliConfigOverrides, codex_linux_sandbox_exe: Option, sandbox_type: SandboxType, ) -> anyhow::Result<()> { let sandbox_policy = create_sandbox_policy(full_auto, sandbox); let cwd = std::env::current_dir()?; - let config = Config::load_with_overrides(ConfigOverrides { - sandbox_policy: Some(sandbox_policy), - codex_linux_sandbox_exe, - ..Default::default() - })?; + let config = Config::load_with_cli_overrides( + config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?, + ConfigOverrides { + sandbox_policy: Some(sandbox_policy), + codex_linux_sandbox_exe, + ..Default::default() + }, + )?; let stdio_policy = StdioPolicy::Inherit; let env = create_env(&config.shell_environment_policy); diff --git a/codex-rs/cli/src/lib.rs b/codex-rs/cli/src/lib.rs index bf85c98c8e..0730a919d7 100644 --- a/codex-rs/cli/src/lib.rs +++ b/codex-rs/cli/src/lib.rs @@ -3,6 +3,7 @@ mod exit_status; pub mod proto; use clap::Parser; +use codex_common::CliConfigOverrides; use codex_common::SandboxPermissionOption; #[derive(Debug, Parser)] @@ -14,6 +15,9 @@ pub struct SeatbeltCommand { #[clap(flatten)] pub sandbox: SandboxPermissionOption, + #[clap(skip)] + pub config_overrides: CliConfigOverrides, + /// Full command args to run under seatbelt. #[arg(trailing_var_arg = true)] pub command: Vec, @@ -28,6 +32,9 @@ pub struct LandlockCommand { #[clap(flatten)] pub sandbox: SandboxPermissionOption, + #[clap(skip)] + pub config_overrides: CliConfigOverrides, + /// Full command args to run under landlock. #[arg(trailing_var_arg = true)] pub command: Vec, diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 8f44962e6d..1c362d2a48 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -2,6 +2,7 @@ use clap::Parser; use codex_cli::LandlockCommand; use codex_cli::SeatbeltCommand; use codex_cli::proto; +use codex_common::CliConfigOverrides; use codex_exec::Cli as ExecCli; use codex_tui::Cli as TuiCli; use std::path::PathBuf; @@ -19,6 +20,9 @@ use crate::proto::ProtoCli; subcommand_negates_reqs = true )] struct MultitoolCli { + #[clap(flatten)] + pub config_overrides: CliConfigOverrides, + #[clap(flatten)] interactive: TuiCli, @@ -73,28 +77,34 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() match cli.subcommand { None => { - codex_tui::run_main(cli.interactive, codex_linux_sandbox_exe)?; + let mut tui_cli = cli.interactive; + prepend_config_flags(&mut tui_cli.config_overrides, cli.config_overrides); + codex_tui::run_main(tui_cli, codex_linux_sandbox_exe)?; } - Some(Subcommand::Exec(exec_cli)) => { + Some(Subcommand::Exec(mut exec_cli)) => { + prepend_config_flags(&mut exec_cli.config_overrides, cli.config_overrides); codex_exec::run_main(exec_cli, codex_linux_sandbox_exe).await?; } Some(Subcommand::Mcp) => { codex_mcp_server::run_main(codex_linux_sandbox_exe).await?; } - Some(Subcommand::Proto(proto_cli)) => { + Some(Subcommand::Proto(mut proto_cli)) => { + prepend_config_flags(&mut proto_cli.config_overrides, cli.config_overrides); proto::run_main(proto_cli).await?; } Some(Subcommand::Debug(debug_args)) => match debug_args.cmd { - DebugCommand::Seatbelt(seatbelt_command) => { + DebugCommand::Seatbelt(mut seatbelt_cli) => { + prepend_config_flags(&mut seatbelt_cli.config_overrides, cli.config_overrides); codex_cli::debug_sandbox::run_command_under_seatbelt( - seatbelt_command, + seatbelt_cli, codex_linux_sandbox_exe, ) .await?; } - DebugCommand::Landlock(landlock_command) => { + DebugCommand::Landlock(mut landlock_cli) => { + prepend_config_flags(&mut landlock_cli.config_overrides, cli.config_overrides); codex_cli::debug_sandbox::run_command_under_landlock( - landlock_command, + landlock_cli, codex_linux_sandbox_exe, ) .await?; @@ -104,3 +114,14 @@ async fn cli_main(codex_linux_sandbox_exe: Option) -> anyhow::Result<() Ok(()) } + +/// Prepend root-level overrides so they have lower precedence than +/// CLI-specific ones specified after the subcommand (if any). +fn prepend_config_flags( + subcommand_config_overrides: &mut CliConfigOverrides, + cli_config_overrides: CliConfigOverrides, +) { + subcommand_config_overrides + .raw_overrides + .splice(0..0, cli_config_overrides.raw_overrides); +} diff --git a/codex-rs/cli/src/proto.rs b/codex-rs/cli/src/proto.rs index 6dbe049cc3..148699552a 100644 --- a/codex-rs/cli/src/proto.rs +++ b/codex-rs/cli/src/proto.rs @@ -2,6 +2,7 @@ use std::io::IsTerminal; use std::sync::Arc; use clap::Parser; +use codex_common::CliConfigOverrides; use codex_core::Codex; use codex_core::config::Config; use codex_core::config::ConfigOverrides; @@ -13,9 +14,12 @@ use tracing::error; use tracing::info; #[derive(Debug, Parser)] -pub struct ProtoCli {} +pub struct ProtoCli { + #[clap(skip)] + pub config_overrides: CliConfigOverrides, +} -pub async fn run_main(_opts: ProtoCli) -> anyhow::Result<()> { +pub async fn run_main(opts: ProtoCli) -> anyhow::Result<()> { if std::io::stdin().is_terminal() { anyhow::bail!("Protocol mode expects stdin to be a pipe, not a terminal"); } @@ -24,7 +28,12 @@ pub async fn run_main(_opts: ProtoCli) -> anyhow::Result<()> { .with_writer(std::io::stderr) .init(); - let config = Config::load_with_overrides(ConfigOverrides::default())?; + let ProtoCli { config_overrides } = opts; + let overrides_vec = config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?; + + let config = Config::load_with_cli_overrides(overrides_vec, ConfigOverrides::default())?; let ctrl_c = notify_on_sigint(); let (codex, _init_id) = Codex::spawn(config, ctrl_c.clone()).await?; let codex = Arc::new(codex); diff --git a/codex-rs/common/Cargo.toml b/codex-rs/common/Cargo.toml index 95e4a53182..b4b658dabf 100644 --- a/codex-rs/common/Cargo.toml +++ b/codex-rs/common/Cargo.toml @@ -9,8 +9,10 @@ workspace = true [dependencies] clap = { version = "4", features = ["derive", "wrap_help"], optional = true } codex-core = { path = "../core" } +toml = { version = "0.8", optional = true } +serde = { version = "1", optional = true } [features] # Separate feature so that `clap` is not a mandatory dependency. -cli = ["clap"] +cli = ["clap", "toml", "serde"] elapsed = [] diff --git a/codex-rs/common/src/config_override.rs b/codex-rs/common/src/config_override.rs new file mode 100644 index 0000000000..bd2c036940 --- /dev/null +++ b/codex-rs/common/src/config_override.rs @@ -0,0 +1,170 @@ +//! Support for `-c key=value` overrides shared across Codex CLI tools. +//! +//! This module provides a [`CliConfigOverrides`] struct that can be embedded +//! into a `clap`-derived CLI struct using `#[clap(flatten)]`. Each occurrence +//! of `-c key=value` (or `--config key=value`) will be collected as a raw +//! string. Helper methods are provided to convert the raw strings into +//! key/value pairs as well as to apply them onto a mutable +//! `serde_json::Value` representing the configuration tree. + +use clap::ArgAction; +use clap::Parser; +use serde::de::Error as SerdeError; +use toml::Value; + +/// CLI option that captures arbitrary configuration overrides specified as +/// `-c key=value`. It intentionally keeps both halves **unparsed** so that the +/// calling code can decide how to interpret the right-hand side. +#[derive(Parser, Debug, Default, Clone)] +pub struct CliConfigOverrides { + /// Override a configuration value that would otherwise be loaded from + /// `~/.codex/config.toml`. Use a dotted path (`foo.bar.baz`) to override + /// nested values. The `value` portion is parsed as JSON. If it fails to + /// parse as JSON, the raw string is used as a literal. + /// + /// Examples: + /// - `-c model="o4-mini"` + /// - `-c 'sandbox_permissions=["disk-full-read-access"]'` + /// - `-c shell_environment_policy.inherit=all` + #[arg( + short = 'c', + long = "config", + value_name = "key=value", + action = ArgAction::Append, + global = true, + )] + pub raw_overrides: Vec, +} + +impl CliConfigOverrides { + /// Parse the raw strings captured from the CLI into a list of `(path, + /// value)` tuples where `value` is a `serde_json::Value`. + pub fn parse_overrides(&self) -> Result, String> { + self.raw_overrides + .iter() + .map(|s| { + // Only split on the *first* '=' so values are free to contain + // the character. + let mut parts = s.splitn(2, '='); + let key = match parts.next() { + Some(k) => k.trim(), + None => return Err("Override missing key".to_string()), + }; + let value_str = parts + .next() + .ok_or_else(|| format!("Invalid override (missing '='): {s}"))? + .trim(); + + if key.is_empty() { + return Err(format!("Empty key in override: {s}")); + } + + // Attempt to parse as JSON. If that fails, treat it as a raw + // string. This allows convenient usage such as + // `-c model=o4-mini` without the quotes. + let value: Value = match parse_toml_value(value_str) { + Ok(v) => v, + Err(_) => Value::String(value_str.to_string()), + }; + + Ok((key.to_string(), value)) + }) + .collect() + } + + /// Apply all parsed overrides onto `target`. Intermediate objects will be + /// created as necessary. Values located at the destination path will be + /// replaced. + pub fn apply_on_value(&self, target: &mut Value) -> Result<(), String> { + let overrides = self.parse_overrides()?; + for (path, value) in overrides { + apply_single_override(target, &path, value); + } + Ok(()) + } +} + +/// Apply a single override onto `root`, creating intermediate objects as +/// necessary. +fn apply_single_override(root: &mut Value, path: &str, value: Value) { + use toml::value::Table; + + let parts: Vec<&str> = path.split('.').collect(); + let mut current = root; + + for (i, part) in parts.iter().enumerate() { + let is_last = i == parts.len() - 1; + + if is_last { + match current { + Value::Table(tbl) => { + tbl.insert((*part).to_string(), value); + } + _ => { + let mut tbl = Table::new(); + tbl.insert((*part).to_string(), value); + *current = Value::Table(tbl); + } + } + return; + } + + // Traverse or create intermediate table. + match current { + Value::Table(tbl) => { + current = tbl + .entry((*part).to_string()) + .or_insert_with(|| Value::Table(Table::new())); + } + _ => { + *current = Value::Table(Table::new()); + if let Value::Table(tbl) = current { + current = tbl + .entry((*part).to_string()) + .or_insert_with(|| Value::Table(Table::new())); + } + } + } + } +} + +fn parse_toml_value(raw: &str) -> Result { + let wrapped = format!("_x_ = {raw}"); + let table: toml::Table = toml::from_str(&wrapped)?; + table + .get("_x_") + .cloned() + .ok_or_else(|| SerdeError::custom("missing sentinel key")) +} + +#[cfg(all(test, feature = "cli"))] +#[allow(clippy::expect_used, clippy::unwrap_used)] +mod tests { + use super::*; + + #[test] + fn parses_basic_scalar() { + let v = parse_toml_value("42").expect("parse"); + assert_eq!(v.as_integer(), Some(42)); + } + + #[test] + fn fails_on_unquoted_string() { + assert!(parse_toml_value("hello").is_err()); + } + + #[test] + fn parses_array() { + let v = parse_toml_value("[1, 2, 3]").expect("parse"); + let arr = v.as_array().expect("array"); + assert_eq!(arr.len(), 3); + } + + #[test] + fn parses_inline_table() { + let v = parse_toml_value("{a = 1, b = 2}").expect("parse"); + let tbl = v.as_table().expect("table"); + assert_eq!(tbl.get("a").unwrap().as_integer(), Some(1)); + assert_eq!(tbl.get("b").unwrap().as_integer(), Some(2)); + } +} diff --git a/codex-rs/common/src/lib.rs b/codex-rs/common/src/lib.rs index 2533718883..c2283640cb 100644 --- a/codex-rs/common/src/lib.rs +++ b/codex-rs/common/src/lib.rs @@ -8,3 +8,9 @@ pub mod elapsed; pub use approval_mode_cli_arg::ApprovalModeCliArg; #[cfg(feature = "cli")] pub use approval_mode_cli_arg::SandboxPermissionOption; + +#[cfg(any(feature = "cli", test))] +mod config_override; + +#[cfg(feature = "cli")] +pub use config_override::CliConfigOverrides; diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index d643d00660..b6871da153 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -16,6 +16,7 @@ use serde::Deserialize; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; +use toml::Value as TomlValue; /// Maximum number of bytes of the documentation that will be embedded. Larger /// files are *silently truncated* to this size so we do not take up too much of @@ -108,6 +109,108 @@ pub struct Config { pub codex_linux_sandbox_exe: Option, } +impl Config { + /// Load configuration with *generic* CLI overrides (`-c key=value`) applied + /// **in between** the values parsed from `config.toml` and the + /// strongly-typed overrides specified via [`ConfigOverrides`]. + /// + /// The precedence order is therefore: `config.toml` < `-c` overrides < + /// `ConfigOverrides`. + pub fn load_with_cli_overrides( + cli_overrides: Vec<(String, TomlValue)>, + overrides: ConfigOverrides, + ) -> std::io::Result { + // Resolve the directory that stores Codex state (e.g. ~/.codex or the + // value of $CODEX_HOME) so we can embed it into the resulting + // `Config` instance. + let codex_home = find_codex_home()?; + + // Step 1: parse `config.toml` into a generic JSON value. + let mut root_value = load_config_as_toml(&codex_home)?; + + // Step 2: apply the `-c` overrides. + for (path, value) in cli_overrides.into_iter() { + apply_toml_override(&mut root_value, &path, value); + } + + // Step 3: deserialize into `ConfigToml` so that Serde can enforce the + // correct types. + let cfg: ConfigToml = root_value.try_into().map_err(|e| { + tracing::error!("Failed to deserialize overridden config: {e}"); + std::io::Error::new(std::io::ErrorKind::InvalidData, e) + })?; + + // Step 4: merge with the strongly-typed overrides. + Self::load_from_base_config_with_overrides(cfg, overrides, codex_home) + } +} + +/// Read `CODEX_HOME/config.toml` and return it as a generic TOML value. Returns +/// an empty TOML table when the file does not exist. +fn load_config_as_toml(codex_home: &Path) -> std::io::Result { + let config_path = codex_home.join("config.toml"); + match std::fs::read_to_string(&config_path) { + Ok(contents) => match toml::from_str::(&contents) { + Ok(val) => Ok(val), + Err(e) => { + tracing::error!("Failed to parse config.toml: {e}"); + Err(std::io::Error::new(std::io::ErrorKind::InvalidData, e)) + } + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + tracing::info!("config.toml not found, using defaults"); + Ok(TomlValue::Table(Default::default())) + } + Err(e) => { + tracing::error!("Failed to read config.toml: {e}"); + Err(e) + } + } +} + +/// Apply a single dotted-path override onto a TOML value. +fn apply_toml_override(root: &mut TomlValue, path: &str, value: TomlValue) { + use toml::value::Table; + + let segments: Vec<&str> = path.split('.').collect(); + let mut current = root; + + for (idx, segment) in segments.iter().enumerate() { + let is_last = idx == segments.len() - 1; + + if is_last { + match current { + TomlValue::Table(table) => { + table.insert(segment.to_string(), value); + } + _ => { + let mut table = Table::new(); + table.insert(segment.to_string(), value); + *current = TomlValue::Table(table); + } + } + return; + } + + // Traverse or create intermediate object. + match current { + TomlValue::Table(table) => { + current = table + .entry(segment.to_string()) + .or_insert_with(|| TomlValue::Table(Table::new())); + } + _ => { + *current = TomlValue::Table(Table::new()); + if let TomlValue::Table(tbl) = current { + current = tbl + .entry(segment.to_string()) + .or_insert_with(|| TomlValue::Table(Table::new())); + } + } + } + } +} + /// Base config deserialized from ~/.codex/config.toml. #[derive(Deserialize, Debug, Clone, Default)] pub struct ConfigToml { @@ -171,29 +274,6 @@ pub struct ConfigToml { pub tui: Option, } -impl ConfigToml { - /// Attempt to parse the file at `~/.codex/config.toml`. If it does not - /// exist, return a default config. Though if it exists and cannot be - /// parsed, report that to the user and force them to fix it. - fn load_from_toml(codex_home: &Path) -> std::io::Result { - let config_toml_path = codex_home.join("config.toml"); - match std::fs::read_to_string(&config_toml_path) { - Ok(contents) => toml::from_str::(&contents).map_err(|e| { - tracing::error!("Failed to parse config.toml: {e}"); - std::io::Error::new(std::io::ErrorKind::InvalidData, e) - }), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - tracing::info!("config.toml not found, using defaults"); - Ok(Self::default()) - } - Err(e) => { - tracing::error!("Failed to read config.toml: {e}"); - Err(e) - } - } - } -} - fn deserialize_sandbox_permissions<'de, D>( deserializer: D, ) -> Result>, D::Error> @@ -227,28 +307,12 @@ pub struct ConfigOverrides { pub cwd: Option, pub approval_policy: Option, pub sandbox_policy: Option, - pub disable_response_storage: Option, pub model_provider: Option, pub config_profile: Option, pub codex_linux_sandbox_exe: Option, } impl Config { - /// Load configuration, optionally applying overrides (CLI flags). Merges - /// ~/.codex/config.toml, ~/.codex/instructions.md, embedded defaults, and - /// any values provided in `overrides` (highest precedence). - pub fn load_with_overrides(overrides: ConfigOverrides) -> std::io::Result { - // Resolve the directory that stores Codex state (e.g. ~/.codex or the - // value of $CODEX_HOME) so we can embed it into the resulting - // `Config` instance. - let codex_home = find_codex_home()?; - - let cfg: ConfigToml = ConfigToml::load_from_toml(&codex_home)?; - tracing::warn!("Config parsed from config.toml: {cfg:?}"); - - Self::load_from_base_config_with_overrides(cfg, overrides, codex_home) - } - /// Meant to be used exclusively for tests: `load_with_overrides()` should /// be used in all other cases. pub fn load_from_base_config_with_overrides( @@ -264,7 +328,6 @@ impl Config { cwd, approval_policy, sandbox_policy, - disable_response_storage, model_provider, config_profile: config_profile_key, codex_linux_sandbox_exe, @@ -356,8 +419,8 @@ impl Config { .unwrap_or_else(AskForApproval::default), sandbox_policy, shell_environment_policy, - disable_response_storage: disable_response_storage - .or(config_profile.disable_response_storage) + disable_response_storage: config_profile + .disable_response_storage .or(cfg.disable_response_storage) .unwrap_or(false), notify: cfg.notify, diff --git a/codex-rs/core/src/config_types.rs b/codex-rs/core/src/config_types.rs index 6696f76f0b..d89b09f267 100644 --- a/codex-rs/core/src/config_types.rs +++ b/codex-rs/core/src/config_types.rs @@ -89,7 +89,7 @@ pub struct Tui { } #[derive(Deserialize, Debug, Clone, PartialEq, Default)] - +#[serde(rename_all = "kebab-case")] pub enum ShellEnvironmentPolicyInherit { /// "Core" environment variables for the platform. On UNIX, this would /// include HOME, LOGNAME, PATH, SHELL, and USER, among others. diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 4a3d493a89..1c2a9eb8aa 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,5 +1,6 @@ use clap::Parser; use clap::ValueEnum; +use codex_common::CliConfigOverrides; use codex_common::SandboxPermissionOption; use std::path::PathBuf; @@ -33,9 +34,8 @@ pub struct Cli { #[arg(long = "skip-git-repo-check", default_value_t = false)] pub skip_git_repo_check: bool, - /// Disable server‑side response storage (sends the full conversation context with every request) - #[arg(long = "disable-response-storage", default_value_t = false)] - pub disable_response_storage: bool, + #[clap(skip)] + pub config_overrides: CliConfigOverrides, /// Specifies color settings for use in the output. #[arg(long = "color", value_enum, default_value_t = Color::Auto)] diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index dbf01f025b..8c94fe5dc9 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -34,10 +34,10 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any sandbox, cwd, skip_git_repo_check, - disable_response_storage, color, last_message_file, prompt, + config_overrides, } = cli; let (stdout_with_ansi, stderr_with_ansi) = match color { @@ -63,16 +63,20 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> any // the user for approval. approval_policy: Some(AskForApproval::Never), sandbox_policy, - disable_response_storage: if disable_response_storage { - Some(true) - } else { - None - }, cwd: cwd.map(|p| p.canonicalize().unwrap_or(p)), model_provider: None, codex_linux_sandbox_exe, }; - let config = Config::load_with_overrides(overrides)?; + // Parse `-c` overrides. + let cli_kv_overrides = match config_overrides.parse_overrides() { + Ok(v) => v, + Err(e) => { + eprintln!("Error parsing -c overrides: {e}"); + std::process::exit(1); + } + }; + + let config = Config::load_with_cli_overrides(cli_kv_overrides, overrides)?; // Print the effective configuration so users can see what Codex is using. print_config_summary(&config, stdout_with_ansi); diff --git a/codex-rs/exec/src/main.rs b/codex-rs/exec/src/main.rs index 17aa5377d2..3a8e1f9411 100644 --- a/codex-rs/exec/src/main.rs +++ b/codex-rs/exec/src/main.rs @@ -10,13 +10,30 @@ //! This allows us to ship a completely separate set of functionality as part //! of the `codex-exec` binary. use clap::Parser; +use codex_common::CliConfigOverrides; use codex_exec::Cli; use codex_exec::run_main; +#[derive(Parser, Debug)] +struct TopCli { + #[clap(flatten)] + config_overrides: CliConfigOverrides, + + #[clap(flatten)] + inner: Cli, +} + fn main() -> anyhow::Result<()> { codex_linux_sandbox::run_with_sandbox(|codex_linux_sandbox_exe| async move { - let cli = Cli::parse(); - run_main(cli, codex_linux_sandbox_exe).await?; + let top_cli = TopCli::parse(); + // Merge root-level overrides into inner CLI struct so downstream logic remains unchanged. + let mut inner = top_cli.inner; + inner + .config_overrides + .raw_overrides + .splice(0..0, top_cli.config_overrides.raw_overrides); + + run_main(inner, codex_linux_sandbox_exe).await?; Ok(()) }) } diff --git a/codex-rs/mcp-server/Cargo.toml b/codex-rs/mcp-server/Cargo.toml index 968222c943..c3f1115819 100644 --- a/codex-rs/mcp-server/Cargo.toml +++ b/codex-rs/mcp-server/Cargo.toml @@ -22,6 +22,7 @@ mcp-types = { path = "../mcp-types" } schemars = "0.8.22" serde = { version = "1", features = ["derive"] } serde_json = "1" +toml = "0.8" tracing = { version = "0.1.41", features = ["log"] } tracing-subscriber = { version = "0.3", features = ["fmt", "env-filter"] } tokio = { version = "1", features = [ diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index d04a5c80bc..03e7234449 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -1,15 +1,16 @@ //! Configuration object accepted by the `codex` MCP tool-call. -use std::path::PathBuf; - +use codex_core::protocol::AskForApproval; +use codex_core::protocol::SandboxPolicy; use mcp_types::Tool; use mcp_types::ToolInputSchema; use schemars::JsonSchema; use schemars::r#gen::SchemaSettings; use serde::Deserialize; +use std::collections::HashMap; +use std::path::PathBuf; -use codex_core::protocol::AskForApproval; -use codex_core::protocol::SandboxPolicy; +use crate::json_to_toml::json_to_toml; /// Client-supplied configuration for a `codex` tool-call. #[derive(Debug, Clone, Deserialize, JsonSchema)] @@ -41,12 +42,10 @@ pub(crate) struct CodexToolCallParam { #[serde(default, skip_serializing_if = "Option::is_none")] pub sandbox_permissions: Option>, - /// Disable server-side response storage. + /// Individual config settings that will override what is in + /// CODEX_HOME/config.toml. #[serde(default, skip_serializing_if = "Option::is_none")] - pub disable_response_storage: Option, - // Custom system instructions. - // #[serde(default, skip_serializing_if = "Option::is_none")] - // pub instructions: Option, + pub config: Option>, } // Create custom enums for use with `CodexToolCallApprovalPolicy` where we @@ -155,7 +154,7 @@ impl CodexToolCallParam { cwd, approval_policy, sandbox_permissions, - disable_response_storage, + config: cli_overrides, } = self; let sandbox_policy = sandbox_permissions.map(|perms| { SandboxPolicy::from(perms.into_iter().map(Into::into).collect::>()) @@ -168,12 +167,17 @@ impl CodexToolCallParam { cwd: cwd.map(PathBuf::from), approval_policy: approval_policy.map(Into::into), sandbox_policy, - disable_response_storage, model_provider: None, codex_linux_sandbox_exe, }; - let cfg = codex_core::config::Config::load_with_overrides(overrides)?; + let cli_overrides = cli_overrides + .unwrap_or_default() + .into_iter() + .map(|(k, v)| (k, json_to_toml(v))) + .collect(); + + let cfg = codex_core::config::Config::load_with_cli_overrides(cli_overrides, overrides)?; Ok((prompt, cfg)) } @@ -216,14 +220,15 @@ mod tests { ], "type": "string" }, + "config": { + "description": "Individual config settings that will override what is in CODEX_HOME/config.toml.", + "additionalProperties": true, + "type": "object" + }, "cwd": { "description": "Working directory for the session. If relative, it is resolved against the server process's current working directory.", "type": "string" }, - "disable-response-storage": { - "description": "Disable server-side response storage.", - "type": "boolean" - }, "model": { "description": "Optional override for the model name (e.g. \"o3\", \"o4-mini\")", "type": "string" diff --git a/codex-rs/mcp-server/src/json_to_toml.rs b/codex-rs/mcp-server/src/json_to_toml.rs new file mode 100644 index 0000000000..ae33382a1d --- /dev/null +++ b/codex-rs/mcp-server/src/json_to_toml.rs @@ -0,0 +1,84 @@ +use serde_json::Value as JsonValue; +use toml::Value as TomlValue; + +/// Convert a `serde_json::Value` into a semantically equivalent `toml::Value`. +pub(crate) fn json_to_toml(v: JsonValue) -> TomlValue { + match v { + JsonValue::Null => TomlValue::String(String::new()), + JsonValue::Bool(b) => TomlValue::Boolean(b), + JsonValue::Number(n) => { + if let Some(i) = n.as_i64() { + TomlValue::Integer(i) + } else if let Some(f) = n.as_f64() { + TomlValue::Float(f) + } else { + TomlValue::String(n.to_string()) + } + } + JsonValue::String(s) => TomlValue::String(s), + JsonValue::Array(arr) => TomlValue::Array(arr.into_iter().map(json_to_toml).collect()), + JsonValue::Object(map) => { + let tbl = map + .into_iter() + .map(|(k, v)| (k, json_to_toml(v))) + .collect::(); + TomlValue::Table(tbl) + } + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use serde_json::json; + + #[test] + fn json_number_to_toml() { + let json_value = json!(123); + assert_eq!(TomlValue::Integer(123), json_to_toml(json_value)); + } + + #[test] + fn json_array_to_toml() { + let json_value = json!([true, 1]); + assert_eq!( + TomlValue::Array(vec![TomlValue::Boolean(true), TomlValue::Integer(1)]), + json_to_toml(json_value) + ); + } + + #[test] + fn json_bool_to_toml() { + let json_value = json!(false); + assert_eq!(TomlValue::Boolean(false), json_to_toml(json_value)); + } + + #[test] + fn json_float_to_toml() { + let json_value = json!(1.25); + assert_eq!(TomlValue::Float(1.25), json_to_toml(json_value)); + } + + #[test] + fn json_null_to_toml() { + let json_value = serde_json::Value::Null; + assert_eq!(TomlValue::String(String::new()), json_to_toml(json_value)); + } + + #[test] + fn json_object_nested() { + let json_value = json!({ "outer": { "inner": 2 } }); + let expected = { + let mut inner = toml::value::Table::new(); + inner.insert("inner".into(), TomlValue::Integer(2)); + + let mut outer = toml::value::Table::new(); + outer.insert("outer".into(), TomlValue::Table(inner)); + TomlValue::Table(outer) + }; + + assert_eq!(json_to_toml(json_value), expected); + } +} diff --git a/codex-rs/mcp-server/src/lib.rs b/codex-rs/mcp-server/src/lib.rs index 0f29eb7826..b2a7797fe6 100644 --- a/codex-rs/mcp-server/src/lib.rs +++ b/codex-rs/mcp-server/src/lib.rs @@ -16,6 +16,7 @@ use tracing::info; mod codex_tool_config; mod codex_tool_runner; +mod json_to_toml; mod message_processor; use crate::message_processor::MessageProcessor; diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index f077d26743..4abd684144 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -1,5 +1,6 @@ use clap::Parser; use codex_common::ApprovalModeCliArg; +use codex_common::CliConfigOverrides; use codex_common::SandboxPermissionOption; use std::path::PathBuf; @@ -40,7 +41,6 @@ pub struct Cli { #[arg(long = "skip-git-repo-check", default_value_t = false)] pub skip_git_repo_check: bool, - /// Disable server‑side response storage (sends the full conversation context with every request) - #[arg(long = "disable-response-storage", default_value_t = false)] - pub disable_response_storage: bool, + #[clap(skip)] + pub config_overrides: CliConfigOverrides, } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 4ab68724aa..1ddd79cf1a 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -54,18 +54,23 @@ pub fn run_main(cli: Cli, codex_linux_sandbox_exe: Option) -> std::io:: model: cli.model.clone(), approval_policy, sandbox_policy, - disable_response_storage: if cli.disable_response_storage { - Some(true) - } else { - None - }, cwd: cli.cwd.clone().map(|p| p.canonicalize().unwrap_or(p)), model_provider: None, config_profile: cli.config_profile.clone(), codex_linux_sandbox_exe, }; + // Parse `-c` overrides from the CLI. + let cli_kv_overrides = match cli.config_overrides.parse_overrides() { + Ok(v) => v, + #[allow(clippy::print_stderr)] + Err(e) => { + eprintln!("Error parsing -c overrides: {e}"); + std::process::exit(1); + } + }; + #[allow(clippy::print_stderr)] - match Config::load_with_overrides(overrides) { + match Config::load_with_cli_overrides(cli_kv_overrides, overrides) { Ok(config) => config, Err(err) => { eprintln!("Error loading configuration: {err}"); diff --git a/codex-rs/tui/src/main.rs b/codex-rs/tui/src/main.rs index 7e55f2af5d..7fcc944504 100644 --- a/codex-rs/tui/src/main.rs +++ b/codex-rs/tui/src/main.rs @@ -1,11 +1,26 @@ use clap::Parser; +use codex_common::CliConfigOverrides; use codex_tui::Cli; use codex_tui::run_main; +#[derive(Parser, Debug)] +struct TopCli { + #[clap(flatten)] + config_overrides: CliConfigOverrides, + + #[clap(flatten)] + inner: Cli, +} + fn main() -> anyhow::Result<()> { codex_linux_sandbox::run_with_sandbox(|codex_linux_sandbox_exe| async move { - let cli = Cli::parse(); - run_main(cli, codex_linux_sandbox_exe)?; + let top_cli = TopCli::parse(); + let mut inner = top_cli.inner; + inner + .config_overrides + .raw_overrides + .splice(0..0, top_cli.config_overrides.raw_overrides); + run_main(inner, codex_linux_sandbox_exe)?; Ok(()) }) } From 23d47c4d482a4df75ff5a7f5b15cf8899bb6f875 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 28 May 2025 13:26:39 -0700 Subject: [PATCH 3/3] feat: introduce CellWidget trait --- codex-rs/tui/src/cell_widget.rs | 20 +++ .../tui/src/conversation_history_widget.rs | 138 +++++++---------- codex-rs/tui/src/history_cell.rs | 140 ++++++++++++------ codex-rs/tui/src/lib.rs | 2 + codex-rs/tui/src/text_block.rs | 32 ++++ 5 files changed, 205 insertions(+), 127 deletions(-) create mode 100644 codex-rs/tui/src/cell_widget.rs create mode 100644 codex-rs/tui/src/text_block.rs diff --git a/codex-rs/tui/src/cell_widget.rs b/codex-rs/tui/src/cell_widget.rs new file mode 100644 index 0000000000..8acdc0553a --- /dev/null +++ b/codex-rs/tui/src/cell_widget.rs @@ -0,0 +1,20 @@ +use ratatui::prelude::*; + +/// Trait implemented by every type that can live inside the conversation +/// history list. It provides two primitives that the parent scroll-view +/// needs: how *tall* the widget is at a given width and how to render an +/// arbitrary contiguous *window* of that widget. +/// +/// The `first_visible_line` argument to [`render_window`] allows partial +/// rendering when the top of the widget is scrolled off-screen. The caller +/// guarantees that `first_visible_line + area.height as usize` never exceeds +/// the total height previously returned by [`height`]. +pub(crate) trait CellWidget { + /// Total height measured in wrapped terminal lines when drawn with the + /// given *content* width (no scrollbar column included). + fn height(&self, width: u16) -> usize; + + /// Render a *window* that starts `first_visible_line` lines below the top + /// of the widget. The window’s size is given by `area`. + fn render_window(&self, first_visible_line: usize, area: Rect, buf: &mut Buffer); +} diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index 83d5ebc496..d69f4db88e 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -1,3 +1,4 @@ +use crate::cell_widget::CellWidget; use crate::history_cell::CommandOutput; use crate::history_cell::HistoryCell; use crate::history_cell::PatchEventType; @@ -236,11 +237,7 @@ impl ConversationHistoryWidget { fn add_to_history(&mut self, cell: HistoryCell) { let width = self.cached_width.get(); - let count = if width > 0 { - wrapped_line_count_for_cell(&cell, width) - } else { - 0 - }; + let count = if width > 0 { cell.height(width) } else { 0 }; self.entries.push(Entry { cell, @@ -284,9 +281,7 @@ impl ConversationHistoryWidget { // Update cached line count. if width > 0 { - entry - .line_count - .set(wrapped_line_count_for_cell(cell, width)); + entry.line_count.set(cell.height(width)); } break; } @@ -328,9 +323,7 @@ impl ConversationHistoryWidget { entry.cell = completed; if width > 0 { - entry - .line_count - .set(wrapped_line_count_for_cell(&entry.cell, width)); + entry.line_count.set(entry.cell.height(width)); } break; @@ -378,7 +371,7 @@ impl WidgetRef for ConversationHistoryWidget { let mut num_lines: usize = 0; for entry in &self.entries { - let count = wrapped_line_count_for_cell(&entry.cell, effective_width); + let count = entry.cell.height(effective_width); num_lines += count; entry.line_count.set(count); } @@ -397,79 +390,69 @@ impl WidgetRef for ConversationHistoryWidget { self.scroll_position.min(max_scroll) }; - // ------------------------------------------------------------------ - // Build a *window* into the history so we only clone the `Line`s that - // may actually be visible in this frame. We still hand the slice off - // to a `Paragraph` with an additional scroll offset to avoid slicing - // inside a wrapped line (we don’t have per-subline granularity). - // ------------------------------------------------------------------ - - // Find the first entry that intersects the current scroll position. - let mut cumulative = 0usize; - let mut first_idx = 0usize; - for (idx, entry) in self.entries.iter().enumerate() { - let next = cumulative + entry.line_count.get(); - if next > scroll_pos { - first_idx = idx; - break; - } - cumulative = next; - } - - let offset_into_first = scroll_pos - cumulative; - - // Collect enough raw lines from `first_idx` onward to cover the - // viewport. We may fetch *slightly* more than necessary (whole cells) - // but never the entire history. - let mut collected_wrapped = 0usize; - let mut visible_lines: Vec> = Vec::new(); - - for entry in &self.entries[first_idx..] { - visible_lines.extend(entry.cell.lines().iter().cloned()); - collected_wrapped += entry.line_count.get(); - if collected_wrapped >= offset_into_first + viewport_height { - break; - } - } - - // Build the Paragraph with wrapping enabled so long lines are not - // clipped. Apply vertical scroll so that `offset_into_first` wrapped - // lines are hidden at the top. // ------------------------------------------------------------------ // Render order: - // 1. Clear the whole widget area so we do not leave behind any glyphs - // from the previous frame. + // 1. Clear full widget area (avoid artifacts from prior frame). // 2. Draw the surrounding Block (border and title). - // 3. Draw the Paragraph inside the Block, **leaving the right-most - // column free** for the scrollbar. - // 4. Finally draw the scrollbar (if needed). + // 3. Render *each* visible HistoryCell into its own sub-Rect while + // respecting partial visibility at the top and bottom. + // 4. Draw the scrollbar track / thumb in the reserved column. // ------------------------------------------------------------------ - // Clear the widget area to avoid visual artifacts from previous frames. + // Clear entire widget area first. Clear.render(area, buf); - // Draw the outer border and title first so the Paragraph does not - // overwrite it. + // Draw border + title. block.render(area, buf); - // Area available for text after accounting for the scrollbar. - let text_area = Rect { - x: inner.x, - y: inner.y, - width: effective_width, - height: inner.height, - }; + // ------------------------------------------------------------------ + // Calculate which cells are visible for the current scroll position + // and paint them one by one. + // ------------------------------------------------------------------ - let paragraph = Paragraph::new(visible_lines) - .wrap(wrap_cfg()) - .scroll((offset_into_first as u16, 0)); + let mut y_cursor = inner.y; // first line inside viewport + let mut remaining_height = inner.height as usize; + let mut lines_to_skip = scroll_pos; // number of wrapped lines to skip (above viewport) - paragraph.render(text_area, buf); + for entry in &self.entries { + let cell_height = entry.line_count.get(); - // Always render a scrollbar *track* so that the reserved column is - // visually filled, even when the content fits within the viewport. - // We only draw the *thumb* when the content actually overflows. + // Completely above viewport? Skip whole cell. + if lines_to_skip >= cell_height { + lines_to_skip -= cell_height; + continue; + } + // Determine how much of this cell is visible. + let visible_height = (cell_height - lines_to_skip).min(remaining_height); + + if visible_height == 0 { + break; // no space left + } + + let cell_rect = Rect { + x: inner.x, + y: y_cursor, + width: effective_width, + height: visible_height as u16, + }; + + entry.cell.render_window(lines_to_skip, cell_rect, buf); + + // Advance cursor inside viewport. + y_cursor += visible_height as u16; + remaining_height -= visible_height; + + // After the first (possibly partially skipped) cell, we no longer + // need to skip lines at the top. + lines_to_skip = 0; + + if remaining_height == 0 { + break; // viewport filled + } + } + + // Always render a scrollbar *track* so the reserved column is filled. let overflow = num_lines.saturating_sub(viewport_height); let mut scroll_state = ScrollbarState::default() @@ -521,15 +504,6 @@ impl WidgetRef for ConversationHistoryWidget { /// Common [`Wrap`] configuration used for both measurement and rendering so /// they stay in sync. #[inline] -const fn wrap_cfg() -> ratatui::widgets::Wrap { +pub(crate) const fn wrap_cfg() -> ratatui::widgets::Wrap { ratatui::widgets::Wrap { trim: false } } - -/// Returns the wrapped line count for `cell` at the given `width` using the -/// same wrapping rules that `ConversationHistoryWidget` uses during -/// rendering. -fn wrapped_line_count_for_cell(cell: &HistoryCell, width: u16) -> usize { - Paragraph::new(cell.lines().clone()) - .wrap(wrap_cfg()) - .line_count(width) -} diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index fab9432724..c2938f4b85 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -9,6 +9,9 @@ use ratatui::style::Modifier; use ratatui::style::Style; use ratatui::text::Line as RtLine; use ratatui::text::Span as RtSpan; + +use crate::cell_widget::CellWidget; +use crate::text_block::TextBlock; use std::collections::HashMap; use std::path::PathBuf; use std::time::Duration; @@ -34,16 +37,16 @@ pub(crate) enum PatchEventType { /// scrollable list. pub(crate) enum HistoryCell { /// Welcome message. - WelcomeMessage { lines: Vec> }, + WelcomeMessage { view: TextBlock }, /// Message from the user. - UserPrompt { lines: Vec> }, + UserPrompt { view: TextBlock }, /// Message from the agent. - AgentMessage { lines: Vec> }, + AgentMessage { view: TextBlock }, /// Reasoning event from the agent. - AgentReasoning { lines: Vec> }, + AgentReasoning { view: TextBlock }, /// An exec tool call that has not finished yet. ActiveExecCommand { @@ -51,11 +54,11 @@ pub(crate) enum HistoryCell { /// The shell command, escaped and formatted. command: String, start: Instant, - lines: Vec>, + view: TextBlock, }, /// Completed exec tool call. - CompletedExecCommand { lines: Vec> }, + CompletedExecCommand { view: TextBlock }, /// An MCP tool call that has not finished yet. ActiveMcpToolCall { @@ -67,29 +70,25 @@ pub(crate) enum HistoryCell { /// exact same text without re-formatting. invocation: String, start: Instant, - lines: Vec>, + view: TextBlock, }, /// Completed MCP tool call. - CompletedMcpToolCall { lines: Vec> }, + CompletedMcpToolCall { view: TextBlock }, - /// Background event - BackgroundEvent { lines: Vec> }, + /// Background event. + BackgroundEvent { view: TextBlock }, /// Error event from the backend. - ErrorEvent { lines: Vec> }, + ErrorEvent { view: TextBlock }, - /// Info describing the newly‑initialized session. - SessionInfo { lines: Vec> }, + /// Info describing the newly-initialized session. + SessionInfo { view: TextBlock }, /// A pending code patch that is awaiting user approval. Mirrors the /// behaviour of `ActiveExecCommand` so the user sees *what* patch the /// model wants to apply before being prompted to approve or deny it. - PendingPatch { - /// Identifier so that a future `PatchApplyEnd` can update the entry - /// with the final status (not yet implemented). - lines: Vec>, - }, + PendingPatch { view: TextBlock }, } const TOOL_CALL_MAX_LINES: usize = 5; @@ -132,9 +131,13 @@ impl HistoryCell { lines.push(Line::from(vec![format!("{key}: ").bold(), value.into()])); } lines.push(Line::from("")); - HistoryCell::WelcomeMessage { lines } + HistoryCell::WelcomeMessage { + view: TextBlock::new(lines), + } } else if config.model == model { - HistoryCell::SessionInfo { lines: vec![] } + HistoryCell::SessionInfo { + view: TextBlock::new(Vec::new()), + } } else { let lines = vec![ Line::from("model changed:".magenta().bold()), @@ -142,7 +145,9 @@ impl HistoryCell { Line::from(format!("used: {}", model)), Line::from(""), ]; - HistoryCell::SessionInfo { lines } + HistoryCell::SessionInfo { + view: TextBlock::new(lines), + } } } @@ -152,7 +157,9 @@ impl HistoryCell { lines.extend(message.lines().map(|l| Line::from(l.to_string()))); lines.push(Line::from("")); - HistoryCell::UserPrompt { lines } + HistoryCell::UserPrompt { + view: TextBlock::new(lines), + } } pub(crate) fn new_agent_message(config: &Config, message: String) -> Self { @@ -161,7 +168,9 @@ impl HistoryCell { append_markdown(&message, &mut lines, config); lines.push(Line::from("")); - HistoryCell::AgentMessage { lines } + HistoryCell::AgentMessage { + view: TextBlock::new(lines), + } } pub(crate) fn new_agent_reasoning(config: &Config, text: String) -> Self { @@ -170,7 +179,9 @@ impl HistoryCell { append_markdown(&text, &mut lines, config); lines.push(Line::from("")); - HistoryCell::AgentReasoning { lines } + HistoryCell::AgentReasoning { + view: TextBlock::new(lines), + } } pub(crate) fn new_active_exec_command(call_id: String, command: Vec) -> Self { @@ -187,7 +198,7 @@ impl HistoryCell { call_id, command: command_escaped, start, - lines, + view: TextBlock::new(lines), } } @@ -226,7 +237,9 @@ impl HistoryCell { } lines.push(Line::from("")); - HistoryCell::CompletedExecCommand { lines } + HistoryCell::CompletedExecCommand { + view: TextBlock::new(lines), + } } pub(crate) fn new_active_mcp_tool_call( @@ -267,7 +280,7 @@ impl HistoryCell { fq_tool_name, invocation, start, - lines, + view: TextBlock::new(lines), } } @@ -304,7 +317,9 @@ impl HistoryCell { lines.push(Line::from("")); - HistoryCell::CompletedMcpToolCall { lines } + HistoryCell::CompletedMcpToolCall { + view: TextBlock::new(lines), + } } pub(crate) fn new_background_event(message: String) -> Self { @@ -312,7 +327,9 @@ impl HistoryCell { lines.push(Line::from("event".dim())); lines.extend(message.lines().map(|l| Line::from(l.to_string()).dim())); lines.push(Line::from("")); - HistoryCell::BackgroundEvent { lines } + HistoryCell::BackgroundEvent { + view: TextBlock::new(lines), + } } pub(crate) fn new_error_event(message: String) -> Self { @@ -320,7 +337,9 @@ impl HistoryCell { vec!["ERROR: ".red().bold(), message.into()].into(), "".into(), ]; - HistoryCell::ErrorEvent { lines } + HistoryCell::ErrorEvent { + view: TextBlock::new(lines), + } } /// Create a new `PendingPatch` cell that lists the file‑level summary of @@ -339,7 +358,9 @@ impl HistoryCell { auto_approved: false, } => { let lines = vec![Line::from("patch applied".magenta().bold())]; - return Self::PendingPatch { lines }; + return Self::PendingPatch { + view: TextBlock::new(lines), + }; } }; @@ -380,23 +401,52 @@ impl HistoryCell { lines.push(Line::from("")); - HistoryCell::PendingPatch { lines } + HistoryCell::PendingPatch { + view: TextBlock::new(lines), + } + } +} + +// --------------------------------------------------------------------------- +// `CellWidget` implementation – most variants delegate to their internal +// `TextBlock`. Variants that need custom painting can add their own logic in +// the match arms. +// --------------------------------------------------------------------------- + +impl CellWidget for HistoryCell { + fn height(&self, width: u16) -> usize { + match self { + HistoryCell::WelcomeMessage { view } + | HistoryCell::UserPrompt { view } + | HistoryCell::AgentMessage { view } + | HistoryCell::AgentReasoning { view } + | HistoryCell::BackgroundEvent { view } + | HistoryCell::ErrorEvent { view } + | HistoryCell::SessionInfo { view } + | HistoryCell::CompletedExecCommand { view } + | HistoryCell::CompletedMcpToolCall { view } + | HistoryCell::PendingPatch { view } + | HistoryCell::ActiveExecCommand { view, .. } + | HistoryCell::ActiveMcpToolCall { view, .. } => view.height(width), + } } - pub(crate) fn lines(&self) -> &Vec> { + fn render_window(&self, first_visible_line: usize, area: Rect, buf: &mut Buffer) { match self { - HistoryCell::WelcomeMessage { lines, .. } - | HistoryCell::UserPrompt { lines, .. } - | HistoryCell::AgentMessage { lines, .. } - | HistoryCell::AgentReasoning { lines, .. } - | HistoryCell::BackgroundEvent { lines, .. } - | HistoryCell::ErrorEvent { lines, .. } - | HistoryCell::SessionInfo { lines, .. } - | HistoryCell::ActiveExecCommand { lines, .. } - | HistoryCell::CompletedExecCommand { lines, .. } - | HistoryCell::ActiveMcpToolCall { lines, .. } - | HistoryCell::CompletedMcpToolCall { lines, .. } - | HistoryCell::PendingPatch { lines, .. } => lines, + HistoryCell::WelcomeMessage { view } + | HistoryCell::UserPrompt { view } + | HistoryCell::AgentMessage { view } + | HistoryCell::AgentReasoning { view } + | HistoryCell::BackgroundEvent { view } + | HistoryCell::ErrorEvent { view } + | HistoryCell::SessionInfo { view } + | HistoryCell::CompletedExecCommand { view } + | HistoryCell::CompletedMcpToolCall { view } + | HistoryCell::PendingPatch { view } + | HistoryCell::ActiveExecCommand { view, .. } + | HistoryCell::ActiveMcpToolCall { view, .. } => { + view.render_window(first_visible_line, area, buf) + } } } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 1ddd79cf1a..df85673ef1 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -19,6 +19,7 @@ mod app; mod app_event; mod app_event_sender; mod bottom_pane; +mod cell_widget; mod chatwidget; mod citation_regex; mod cli; @@ -32,6 +33,7 @@ mod mouse_capture; mod scroll_event_helper; mod slash_command; mod status_indicator_widget; +mod text_block; mod tui; mod user_approval_widget; diff --git a/codex-rs/tui/src/text_block.rs b/codex-rs/tui/src/text_block.rs new file mode 100644 index 0000000000..2c68d90f11 --- /dev/null +++ b/codex-rs/tui/src/text_block.rs @@ -0,0 +1,32 @@ +use crate::cell_widget::CellWidget; +use ratatui::prelude::*; + +/// A simple widget that just displays a list of `Line`s via a `Paragraph`. +/// This is the default rendering backend for most `HistoryCell` variants. +#[derive(Clone)] +pub(crate) struct TextBlock { + pub(crate) lines: Vec>, +} + +impl TextBlock { + pub(crate) fn new(lines: Vec>) -> Self { + Self { lines } + } +} + +impl CellWidget for TextBlock { + fn height(&self, width: u16) -> usize { + // Use the same wrapping configuration as ConversationHistoryWidget so + // measurement stays in sync with rendering. + ratatui::widgets::Paragraph::new(self.lines.clone()) + .wrap(crate::conversation_history_widget::wrap_cfg()) + .line_count(width) + } + + fn render_window(&self, first_visible_line: usize, area: Rect, buf: &mut Buffer) { + ratatui::widgets::Paragraph::new(self.lines.clone()) + .wrap(crate::conversation_history_widget::wrap_cfg()) + .scroll((first_visible_line as u16, 0)) + .render(area, buf); + } +}