From 707beb21e815120b6a9fa74c91f573370ed10763 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 25 Apr 2025 18:03:11 -0700 Subject: [PATCH] feat: load defaults into Config and introduce ConfigOverrides --- codex-rs/core/src/approval_mode_cli_arg.rs | 4 +- codex-rs/core/src/codex.rs | 2 - codex-rs/core/src/codex_wrapper.rs | 9 ++-- codex-rs/core/src/config.rs | 51 +++++++++++++++------ codex-rs/core/src/protocol.rs | 5 +- codex-rs/core/tests/live_agent.rs | 8 ++-- codex-rs/core/tests/previous_response_id.rs | 8 ++-- codex-rs/core/tests/stream_no_completed.rs | 8 ++-- codex-rs/exec/src/lib.rs | 15 ++++-- codex-rs/repl/src/cli.rs | 5 +- codex-rs/repl/src/lib.rs | 19 +++++--- codex-rs/tui/src/chatwidget.rs | 9 ++-- codex-rs/tui/src/cli.rs | 5 +- codex-rs/tui/src/lib.rs | 15 ++++-- 14 files changed, 107 insertions(+), 56 deletions(-) diff --git a/codex-rs/core/src/approval_mode_cli_arg.rs b/codex-rs/core/src/approval_mode_cli_arg.rs index eb90b24d87..0da6a89efc 100644 --- a/codex-rs/core/src/approval_mode_cli_arg.rs +++ b/codex-rs/core/src/approval_mode_cli_arg.rs @@ -6,7 +6,7 @@ use clap::ValueEnum; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; -#[derive(Clone, Debug, ValueEnum)] +#[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] pub enum ApprovalModeCliArg { /// Run all commands without asking for user approval. @@ -24,7 +24,7 @@ pub enum ApprovalModeCliArg { Never, } -#[derive(Clone, Debug, ValueEnum)] +#[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] pub enum SandboxModeCliArg { /// Network syscalls will be blocked diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cfeb7e401f..2f80e505c0 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -36,7 +36,6 @@ use crate::exec::process_exec_tool_call; use crate::exec::ExecParams; use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; -use crate::flags::OPENAI_DEFAULT_MODEL; use crate::flags::OPENAI_STREAM_MAX_RETRIES; use crate::models::ContentItem; use crate::models::FunctionCallOutputPayload; @@ -486,7 +485,6 @@ async fn submission_loop( sandbox_policy, disable_response_storage, } => { - let model = model.unwrap_or_else(|| OPENAI_DEFAULT_MODEL.to_string()); info!(model, "Configuring session"); let client = ModelClient::new(model.clone()); diff --git a/codex-rs/core/src/codex_wrapper.rs b/codex-rs/core/src/codex_wrapper.rs index 8d19683ffa..991c134fbb 100644 --- a/codex-rs/core/src/codex_wrapper.rs +++ b/codex-rs/core/src/codex_wrapper.rs @@ -11,7 +11,6 @@ use crate::protocol::Submission; use crate::util::notify_on_sigint; use crate::Codex; use tokio::sync::Notify; -use tracing::debug; /// Spawn a new [`Codex`] and initialise the session. /// @@ -19,19 +18,17 @@ use tracing::debug; /// is received as a response to the initial `ConfigureSession` submission so /// that callers can surface the information to the UI. pub async fn init_codex( + config: Config, approval_policy: AskForApproval, sandbox_policy: SandboxPolicy, disable_response_storage: bool, - model_override: Option, ) -> anyhow::Result<(CodexWrapper, Event, Arc)> { let ctrl_c = notify_on_sigint(); - let config = Config::load().unwrap_or_default(); - debug!("loaded config: {config:?}"); let codex = CodexWrapper::new(Codex::spawn(ctrl_c.clone())?); let init_id = codex .submit(Op::ConfigureSession { - model: model_override.or_else(|| config.model.clone()), - instructions: config.instructions, + model: config.model.clone(), + instructions: config.instructions.clone(), approval_policy, sandbox_policy, disable_response_storage, diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index b5574ceade..27cd900f8a 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,32 +1,53 @@ -use std::path::PathBuf; - +use crate::flags::OPENAI_DEFAULT_MODEL; +use crate::protocol::AskForApproval; use dirs::home_dir; use serde::Deserialize; +use std::path::PathBuf; -/// Embedded fallback instructions that mirror the TypeScript CLI’s default system prompt. These -/// are compiled into the binary so a clean install behaves correctly even if the user has not -/// created `~/.codex/instructions.md`. +/// Embedded fallback instructions that mirror the TypeScript CLI’s default +/// system prompt. These are compiled into the binary so a clean install behaves +/// correctly even if the user has not created `~/.codex/instructions.md`. const EMBEDDED_INSTRUCTIONS: &str = include_str!("../prompt.md"); +/// Application configuration loaded from disk and merged with overrides. #[derive(Default, Deserialize, Debug, Clone)] pub struct Config { - pub model: Option, + /// Optional override of model selection. + #[serde(default = "default_model")] + pub model: String, + /// Default approval policy for executing commands. + #[serde(default)] + pub approval_policy: AskForApproval, + /// System instructions. pub instructions: Option, } +/// Optional overrides for user configuration (e.g., from CLI flags). +#[derive(Default, Debug, Clone)] +pub struct ConfigOverrides { + pub model: Option, + pub approval_policy: Option, +} + impl Config { - /// Load ~/.codex/config.toml and ~/.codex/instructions.md (if present). - /// Returns `None` if neither file exists. - pub fn load() -> Option { + /// 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) -> Self { let mut cfg: Config = Self::load_from_toml().unwrap_or_default(); - // Highest precedence → user‑provided ~/.codex/instructions.md (if present) - // Fallback → embedded default instructions baked into the binary - + // Instructions: user-provided instructions.md > embedded default. cfg.instructions = Self::load_instructions().or_else(|| Some(EMBEDDED_INSTRUCTIONS.to_string())); - Some(cfg) + // Apply overrides. + if let Some(model) = overrides.model { + cfg.model = model; + } + if let Some(policy) = overrides.approval_policy { + cfg.approval_policy = policy; + } + cfg } fn load_from_toml() -> Option { @@ -43,6 +64,10 @@ impl Config { } } +fn default_model() -> String { + OPENAI_DEFAULT_MODEL.to_string() +} + /// Returns the path to the Codex configuration directory, which is `~/.codex`. /// Does not verify that the directory exists. pub fn codex_dir() -> std::io::Result { diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 96c4ea4832..8d019d784c 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -26,7 +26,7 @@ pub enum Op { /// Configure the model session. ConfigureSession { /// If not specified, server will use its default model. - model: Option, + model: String, /// Model instructions instructions: Option, /// When to escalate for approval for execution @@ -66,11 +66,12 @@ pub enum Op { } /// Determines how liberally commands are auto‑approved by the system. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] pub enum AskForApproval { /// Under this policy, only “known safe” commands—as determined by /// `is_safe_command()`—that **only read files** are auto‑approved. /// Everything else will ask the user to approve. + #[default] UnlessAllowListed, /// In addition to everything allowed by **`Suggest`**, commands that diff --git a/codex-rs/core/tests/live_agent.rs b/codex-rs/core/tests/live_agent.rs index 823cd73a01..c74d3e17dc 100644 --- a/codex-rs/core/tests/live_agent.rs +++ b/codex-rs/core/tests/live_agent.rs @@ -17,7 +17,8 @@ use std::time::Duration; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; @@ -47,13 +48,14 @@ async fn spawn_codex() -> Codex { let agent = Codex::spawn(std::sync::Arc::new(Notify::new())).unwrap(); + let config = Config::load_with_overrides(ConfigOverrides::default()); agent .submit(Submission { id: "init".into(), op: Op::ConfigureSession { - model: None, + model: config.model, instructions: None, - approval_policy: AskForApproval::OnFailure, + approval_policy: config.approval_policy, sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted, disable_response_storage: false, }, diff --git a/codex-rs/core/tests/previous_response_id.rs b/codex-rs/core/tests/previous_response_id.rs index de1309e856..bb04d54d1e 100644 --- a/codex-rs/core/tests/previous_response_id.rs +++ b/codex-rs/core/tests/previous_response_id.rs @@ -1,6 +1,7 @@ use std::time::Duration; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -87,13 +88,14 @@ async fn keeps_previous_response_id_between_tasks() { let codex = Codex::spawn(std::sync::Arc::new(tokio::sync::Notify::new())).unwrap(); // Init session + let config = Config::load_with_overrides(ConfigOverrides::default()); codex .submit(Submission { id: "init".into(), op: Op::ConfigureSession { - model: None, + model: config.model, instructions: None, - approval_policy: AskForApproval::OnFailure, + approval_policy: config.approval_policy, sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted, disable_response_storage: false, }, diff --git a/codex-rs/core/tests/stream_no_completed.rs b/codex-rs/core/tests/stream_no_completed.rs index c732a5fdbb..c33bfef753 100644 --- a/codex-rs/core/tests/stream_no_completed.rs +++ b/codex-rs/core/tests/stream_no_completed.rs @@ -3,7 +3,8 @@ use std::time::Duration; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -70,13 +71,14 @@ async fn retries_on_early_close() { let codex = Codex::spawn(std::sync::Arc::new(tokio::sync::Notify::new())).unwrap(); + let config = Config::load_with_overrides(ConfigOverrides::default()); codex .submit(Submission { id: "init".into(), op: Op::ConfigureSession { - model: None, + model: config.model, instructions: None, - approval_policy: AskForApproval::OnFailure, + approval_policy: config.approval_policy, sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted, disable_response_storage: false, }, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index ab7d735e0f..49ca07b623 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -3,7 +3,8 @@ use std::sync::Arc; pub use cli::Cli; use codex_core::codex_wrapper; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::FileChange; @@ -47,15 +48,19 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { std::process::exit(1); } - // TODO(mbolin): We are reworking the CLI args right now, so this will - // likely come from a new --execution-policy arg. - let approval_policy = AskForApproval::Never; + // Load configuration and determine approval policy + let overrides = ConfigOverrides { + model: model.clone(), + ..Default::default() + }; + let config = Config::load_with_overrides(overrides); + let approval_policy = config.approval_policy; let sandbox_policy = SandboxPolicy::NetworkAndFileWriteRestricted; let (codex_wrapper, event, ctrl_c) = codex_wrapper::init_codex( + config, approval_policy, sandbox_policy, disable_response_storage, - model, ) .await?; let codex = Arc::new(codex_wrapper); diff --git a/codex-rs/repl/src/cli.rs b/codex-rs/repl/src/cli.rs index ec6c652519..af1f1b9c17 100644 --- a/codex-rs/repl/src/cli.rs +++ b/codex-rs/repl/src/cli.rs @@ -34,8 +34,9 @@ pub struct Cli { pub no_ansi: bool, /// Configure when the model requires human approval before executing a command. - #[arg(long = "ask-for-approval", short = 'a', value_enum, default_value_t = ApprovalModeCliArg::OnFailure)] - pub approval_policy: ApprovalModeCliArg, + /// Overrides the value in ~/.codex/config.toml if provided. + #[arg(long = "ask-for-approval", short = 'a', value_enum)] + pub approval_policy: Option, /// Configure the process restrictions when a command is executed. /// diff --git a/codex-rs/repl/src/lib.rs b/codex-rs/repl/src/lib.rs index 0f9c47e49b..a027efe702 100644 --- a/codex-rs/repl/src/lib.rs +++ b/codex-rs/repl/src/lib.rs @@ -4,6 +4,7 @@ use std::io::Write; use std::sync::Arc; use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol; use codex_core::protocol::FileChange; use codex_core::util::is_inside_git_repo; @@ -75,12 +76,17 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { // Initialize logging before any other work so early errors are captured. init_logger(cli.verbose, !cli.no_ansi); - let config = Config::load().unwrap_or_default(); + // Load config file and apply CLI overrides (model & approval policy) + let overrides = ConfigOverrides { + model: cli.model.clone(), + approval_policy: cli.approval_policy.map(Into::into), + }; + let config = Config::load_with_overrides(overrides); codex_main(cli, config, ctrl_c).await } -async fn codex_main(mut cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::Result<()> { +async fn codex_main(cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::Result<()> { let mut builder = Codex::builder(); if let Some(path) = cli.record_submissions { builder = builder.record_submissions(path); @@ -90,12 +96,13 @@ async fn codex_main(mut cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::R } let codex = builder.spawn(Arc::clone(&ctrl_c))?; let init_id = random_id(); + // Determine effective approval policy: CLI flag > config file > default let init = protocol::Submission { id: init_id.clone(), op: protocol::Op::ConfigureSession { - model: cli.model.or(cfg.model), + model: cfg.model, instructions: cfg.instructions, - approval_policy: cli.approval_policy.into(), + approval_policy: cfg.approval_policy, sandbox_policy: cli.sandbox_policy.into(), disable_response_storage: cli.disable_response_storage, }, @@ -133,8 +140,8 @@ async fn codex_main(mut cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::R // run loop let mut reader = InputReader::new(ctrl_c.clone()); loop { - let text = match cli.prompt.take() { - Some(input) => input, + let text = match &cli.prompt { + Some(input) => input.clone(), None => match reader.request_input().await? { Some(input) => input, None => { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b852638cc2..46f83fdd8d 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3,6 +3,7 @@ use std::sync::mpsc::Sender; use std::sync::Arc; use codex_core::codex_wrapper::init_codex; +use codex_core::config::{Config, ConfigOverrides}; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; @@ -64,18 +65,20 @@ impl ChatWidget<'_> { let app_event_tx_clone = app_event_tx.clone(); // Create the Codex asynchronously so the UI loads as quickly as possible. tokio::spawn(async move { - // Initialize session; storage enabled by default + // Load config and initialize session + let overrides = ConfigOverrides { model: model.clone(), ..Default::default() }; + let config = Config::load_with_overrides(overrides); let (codex, session_event, _ctrl_c) = match init_codex( + config, approval_policy, sandbox_policy, disable_response_storage, - model, ) .await { Ok(vals) => vals, Err(e) => { - // TODO(mbolin): This error needs to be surfaced to the user. + // TODO: surface this error to the user. tracing::error!("failed to initialize codex: {e}"); return; } diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index db25ad2b3c..bdbb9fe7dc 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -18,8 +18,9 @@ pub struct Cli { pub model: Option, /// Configure when the model requires human approval before executing a command. - #[arg(long = "ask-for-approval", short = 'a', value_enum, default_value_t = ApprovalModeCliArg::OnFailure)] - pub approval_policy: ApprovalModeCliArg, + /// Overrides the value in ~/.codex/config.toml if provided. + #[arg(long = "ask-for-approval", short = 'a', value_enum)] + pub approval_policy: Option, /// Configure the process restrictions when a command is executed. /// diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index d0f5f664a6..c9f903ba15 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -4,6 +4,8 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] use app::App; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::util::is_inside_git_repo; use log_layer::TuiLogLayer; use std::fs::OpenOptions; @@ -113,21 +115,26 @@ fn run_ratatui_app( let mut terminal = tui::init()?; terminal.clear()?; + // Load configuration and destructure CLI flags let Cli { prompt, images, - approval_policy, + approval_policy: cli_approval, sandbox_policy: sandbox, model, disable_response_storage, .. } = cli; + // Apply CLI overrides and load merged configuration + let overrides = ConfigOverrides { + model: model.clone(), + approval_policy: cli_approval.map(Into::into), + }; + let cfg = Config::load_with_overrides(overrides); - let approval_policy = approval_policy.into(); let sandbox_policy = sandbox.into(); - let mut app = App::new( - approval_policy, + cfg.approval_policy, sandbox_policy, prompt, show_git_warning,