From a9409dc175917a68b7f9c8c9e0a0f03bd59a1d3a Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 27 May 2025 10:33:47 -0700 Subject: [PATCH] feat: add support for -c/--config to override individual config items --- codex-rs/Cargo.lock | 1 + codex-rs/common/Cargo.toml | 3 +- codex-rs/common/src/config_override.rs | 130 +++++++++++++++++++ codex-rs/common/src/lib.rs | 6 + codex-rs/core/src/config.rs | 127 +++++++++++++++++- codex-rs/core/src/config_types.rs | 2 +- codex-rs/exec/src/cli.rs | 6 +- codex-rs/exec/src/lib.rs | 18 ++- codex-rs/mcp-server/src/codex_tool_config.rs | 9 -- codex-rs/tui/src/cli.rs | 6 +- codex-rs/tui/src/lib.rs | 17 ++- 11 files changed, 291 insertions(+), 34 deletions(-) create mode 100644 codex-rs/common/src/config_override.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 309c671e74..8674b8d499 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -506,6 +506,7 @@ version = "0.0.0" dependencies = [ "clap", "codex-core", + "serde_json", ] [[package]] diff --git a/codex-rs/common/Cargo.toml b/codex-rs/common/Cargo.toml index 95e4a53182..08974583f2 100644 --- a/codex-rs/common/Cargo.toml +++ b/codex-rs/common/Cargo.toml @@ -9,8 +9,9 @@ workspace = true [dependencies] clap = { version = "4", features = ["derive", "wrap_help"], optional = true } codex-core = { path = "../core" } +serde_json = { version = "1", optional = true } [features] # Separate feature so that `clap` is not a mandatory dependency. -cli = ["clap"] +cli = ["clap", "serde_json"] 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..5763dbe1aa --- /dev/null +++ b/codex-rs/common/src/config_override.rs @@ -0,0 +1,130 @@ +//! 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_json::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, + )] + 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 serde_json::from_str(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 serde_json::Map; + + 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 { + // Replace value at leaf. + if let Value::Object(obj) = current { + obj.insert(part.to_string(), value); + } else { + // Replace non-object with object containing the leaf. + *current = Value::Object({ + let mut m = Map::new(); + m.insert(part.to_string(), value); + m + }); + } + return; + } + + // Traverse or create intermediate object. + match current { + Value::Object(obj) => { + current = obj + .entry(part.to_string()) + .or_insert_with(|| Value::Object(Map::new())); + } + _ => { + // Non-object encountered, replace with object so we can + // continue traversal. + *current = Value::Object(Map::new()); + if let Value::Object(obj) = current { + current = obj + .entry((*part).to_string()) + .or_insert_with(|| Value::Object(Map::new())); + } + } + } + } +} diff --git a/codex-rs/common/src/lib.rs b/codex-rs/common/src/lib.rs index 2533718883..6027e5e44f 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(feature = "cli")] +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..ec9176ecc1 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -13,6 +13,7 @@ use crate::protocol::SandboxPermission; use crate::protocol::SandboxPolicy; use dirs::home_dir; use serde::Deserialize; +use serde_json::Value as JsonValue; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -108,6 +109,126 @@ pub struct Config { pub codex_linux_sandbox_exe: Option, } +//////////////////////////////////////////////////////////////////////////////////////////////// +// CLI-level JSON overrides ("-c key=value") support // +//////////////////////////////////////////////////////////////////////////////////////////////// + +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`. + /// + /// Most CLI binaries should call this method – `load_with_overrides()` is still available + /// for non-CLI callers (tests, servers) which do not accept the `-c` flag. + pub fn load_with_cli_overrides( + cli_overrides: Vec<(String, JsonValue)>, + overrides: ConfigOverrides, + ) -> std::io::Result { + // Resolve CODEX_HOME first; needed by sandbox deserializer later. + let codex_home = find_codex_home()?; + + // Step 1: parse `config.toml` into a generic JSON value. + let mut root_value = load_config_as_json(&codex_home)?; + + // Step 2: apply the `-c` overrides. + for (path, value) in cli_overrides.into_iter() { + apply_json_override(&mut root_value, &path, value); + } + + // Step 3: deserialize into `ConfigToml` so that Serde can enforce the + // correct types. + let cfg: ConfigToml = serde_json::from_value(root_value).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/config.toml` (or the resolved CODEX_HOME location) and +/// return it as a generic JSON value. Returns an empty JSON object when the +/// file does not exist. +fn load_config_as_json(codex_home: &Path) -> std::io::Result { + let config_path = codex_home.join("config.toml"); + match std::fs::read_to_string(&config_path) { + Ok(contents) => { + // Parse as TOML first, then convert to JSON for easier mutation. + match toml::from_str::(&contents) { + Ok(toml_val) => { + let json_val = serde_json::to_value(toml_val).map_err(|e| { + tracing::error!("Failed to convert TOML config to JSON value: {e}"); + std::io::Error::new(std::io::ErrorKind::InvalidData, e) + })?; + Ok(json_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(JsonValue::Object(Default::default())) + } + Err(e) => { + tracing::error!("Failed to read config.toml: {e}"); + Err(e) + } + } +} + +/// Apply a single dotted-path override onto a JSON value. +fn apply_json_override(root: &mut JsonValue, path: &str, value: JsonValue) { + use serde_json::Map; + + 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 { + JsonValue::Object(map) => { + map.insert(segment.to_string(), value); + } + _ => { + *current = JsonValue::Object({ + let mut m = Map::new(); + m.insert(segment.to_string(), value); + m + }); + } + } + return; + } + + // Traverse or create intermediate object. + match current { + JsonValue::Object(map) => { + current = map + .entry(segment.to_string()) + .or_insert_with(|| JsonValue::Object(Map::new())); + } + _ => { + *current = JsonValue::Object(Map::new()); + if let JsonValue::Object(map) = current { + // Safe unwrap: we just replaced current with an empty + // object and immediately inserted the key. + current = map + .entry(segment.to_string()) + .or_insert_with(|| JsonValue::Object(Map::new())); + } + } + } + } +} + /// Base config deserialized from ~/.codex/config.toml. #[derive(Deserialize, Debug, Clone, Default)] pub struct ConfigToml { @@ -227,7 +348,6 @@ 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, @@ -264,7 +384,6 @@ impl Config { cwd, approval_policy, sandbox_policy, - disable_response_storage, model_provider, config_profile: config_profile_key, codex_linux_sandbox_exe, @@ -356,8 +475,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..f92b075403 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(flatten)] + 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/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index d04a5c80bc..72a97638db 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -40,13 +40,6 @@ pub(crate) struct CodexToolCallParam { /// (e.g. "disk-write-cwd", "network-full-access"). #[serde(default, skip_serializing_if = "Option::is_none")] pub sandbox_permissions: Option>, - - /// Disable server-side response storage. - #[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, } // Create custom enums for use with `CodexToolCallApprovalPolicy` where we @@ -155,7 +148,6 @@ impl CodexToolCallParam { cwd, approval_policy, sandbox_permissions, - disable_response_storage, } = self; let sandbox_policy = sandbox_permissions.map(|perms| { SandboxPolicy::from(perms.into_iter().map(Into::into).collect::>()) @@ -168,7 +160,6 @@ 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, }; diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index f077d26743..a5e2acaa91 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(flatten)] + 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}");