feat: load defaults into Config and introduce ConfigOverrides (#677)
This changes how instantiating `Config` works and also adds `approval_policy` and `sandbox_policy` as fields. The idea is: * All fields of `Config` have appropriate default values. * `Config` is initially loaded from `~/.codex/config.toml`, so values in `config.toml` will override those defaults. * Clients must instantiate `Config` via `Config::load_with_overrides(ConfigOverrides)` where `ConfigOverrides` has optional overrides that are expected to be settable based on CLI flags. The `Config` should be defined early in the program and then passed down. Now functions like `init_codex()` take fewer individual parameters because they can just take a `Config`. Also, `Config::load()` used to fail silently if `~/.codex/config.toml` had a parse error and fell back to the default config. This seemed really bad because it wasn't clear why the values in my `config.toml` weren't getting picked up. I changed things so that `load_with_overrides()` returns `Result<Config>` and verified that the various CLIs print a reasonable error if `config.toml` is malformed. Finally, I also updated the TUI to show which **sandbox** value is being used, as we do for other key values like **model** and **approval**. This was also a reminder that the various values of `--sandbox` are honored on Linux but not macOS today, so I added some TODOs about fixing that.
This commit is contained in:
@@ -4,10 +4,9 @@ use crate::git_warning_screen::GitWarningOutcome;
|
||||
use crate::git_warning_screen::GitWarningScreen;
|
||||
use crate::scroll_event_helper::ScrollEventHelper;
|
||||
use crate::tui;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use color_eyre::eyre::Result;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
@@ -34,12 +33,10 @@ pub(crate) struct App<'a> {
|
||||
|
||||
impl App<'_> {
|
||||
pub(crate) fn new(
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
config: Config,
|
||||
initial_prompt: Option<String>,
|
||||
show_git_warning: bool,
|
||||
initial_images: Vec<std::path::PathBuf>,
|
||||
model: Option<String>,
|
||||
disable_response_storage: bool,
|
||||
) -> Self {
|
||||
let (app_event_tx, app_event_rx) = channel();
|
||||
@@ -80,12 +77,10 @@ impl App<'_> {
|
||||
}
|
||||
|
||||
let chat_widget = ChatWidget::new(
|
||||
approval_policy,
|
||||
sandbox_policy,
|
||||
config,
|
||||
app_event_tx.clone(),
|
||||
initial_prompt.clone(),
|
||||
initial_images,
|
||||
model,
|
||||
disable_response_storage,
|
||||
);
|
||||
|
||||
|
||||
@@ -3,12 +3,11 @@ use std::sync::mpsc::Sender;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core::codex_wrapper::init_codex;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::InputItem;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use crossterm::event::KeyEvent;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Constraint;
|
||||
@@ -34,7 +33,7 @@ pub(crate) struct ChatWidget<'a> {
|
||||
conversation_history: ConversationHistoryWidget,
|
||||
bottom_pane: BottomPane<'a>,
|
||||
input_focus: InputFocus,
|
||||
approval_policy: AskForApproval,
|
||||
config: Config,
|
||||
cwd: std::path::PathBuf,
|
||||
}
|
||||
|
||||
@@ -46,12 +45,10 @@ enum InputFocus {
|
||||
|
||||
impl ChatWidget<'_> {
|
||||
pub(crate) fn new(
|
||||
approval_policy: AskForApproval,
|
||||
sandbox_policy: SandboxPolicy,
|
||||
config: Config,
|
||||
app_event_tx: Sender<AppEvent>,
|
||||
initial_prompt: Option<String>,
|
||||
initial_images: Vec<std::path::PathBuf>,
|
||||
model: Option<String>,
|
||||
disable_response_storage: bool,
|
||||
) -> Self {
|
||||
let (codex_op_tx, mut codex_op_rx) = unbounded_channel::<Op>();
|
||||
@@ -63,23 +60,17 @@ impl ChatWidget<'_> {
|
||||
|
||||
let app_event_tx_clone = app_event_tx.clone();
|
||||
// Create the Codex asynchronously so the UI loads as quickly as possible.
|
||||
let config_for_agent_loop = config.clone();
|
||||
tokio::spawn(async move {
|
||||
// Initialize session; storage enabled by default
|
||||
let (codex, session_event, _ctrl_c) = match init_codex(
|
||||
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.
|
||||
tracing::error!("failed to initialize codex: {e}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
let (codex, session_event, _ctrl_c) =
|
||||
match init_codex(config_for_agent_loop, disable_response_storage).await {
|
||||
Ok(vals) => vals,
|
||||
Err(e) => {
|
||||
// TODO: surface this error to the user.
|
||||
tracing::error!("failed to initialize codex: {e}");
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
// Forward the captured `SessionInitialized` event that was consumed
|
||||
// inside `init_codex()` so it can be rendered in the UI.
|
||||
@@ -115,7 +106,7 @@ impl ChatWidget<'_> {
|
||||
has_input_focus: true,
|
||||
}),
|
||||
input_focus: InputFocus::BottomPane,
|
||||
approval_policy,
|
||||
config,
|
||||
cwd: cwd.clone(),
|
||||
};
|
||||
|
||||
@@ -243,11 +234,8 @@ impl ChatWidget<'_> {
|
||||
match msg {
|
||||
EventMsg::SessionConfigured { model } => {
|
||||
// Record session information at the top of the conversation.
|
||||
self.conversation_history.add_session_info(
|
||||
model,
|
||||
self.cwd.clone(),
|
||||
self.approval_policy,
|
||||
);
|
||||
self.conversation_history
|
||||
.add_session_info(&self.config, model, self.cwd.clone());
|
||||
self.request_redraw()?;
|
||||
}
|
||||
EventMsg::AgentMessage { message } => {
|
||||
|
||||
@@ -18,14 +18,14 @@ pub struct Cli {
|
||||
pub model: Option<String>,
|
||||
|
||||
/// 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,
|
||||
#[arg(long = "ask-for-approval", short = 'a')]
|
||||
pub approval_policy: Option<ApprovalModeCliArg>,
|
||||
|
||||
/// Configure the process restrictions when a command is executed.
|
||||
///
|
||||
/// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux.
|
||||
#[arg(long = "sandbox", short = 's', value_enum, default_value_t = SandboxModeCliArg::NetworkAndFileWriteRestricted)]
|
||||
pub sandbox_policy: SandboxModeCliArg,
|
||||
#[arg(long = "sandbox", short = 's')]
|
||||
pub sandbox_policy: Option<SandboxModeCliArg>,
|
||||
|
||||
/// Allow running Codex outside a Git repository.
|
||||
#[arg(long = "skip-git-repo-check", default_value_t = false)]
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use crate::history_cell::CommandOutput;
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::PatchEventType;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::protocol::FileChange;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
@@ -181,13 +182,10 @@ impl ConversationHistoryWidget {
|
||||
self.add_to_history(HistoryCell::new_patch_event(event_type, changes));
|
||||
}
|
||||
|
||||
pub fn add_session_info(
|
||||
&mut self,
|
||||
model: String,
|
||||
cwd: std::path::PathBuf,
|
||||
approval_policy: codex_core::protocol::AskForApproval,
|
||||
) {
|
||||
self.add_to_history(HistoryCell::new_session_info(model, cwd, approval_policy));
|
||||
/// Note `model` could differ from `config.model` if the agent decided to
|
||||
/// use a different model than the one requested by the user.
|
||||
pub fn add_session_info(&mut self, config: &Config, model: String, cwd: PathBuf) {
|
||||
self.add_to_history(HistoryCell::new_session_info(config, model, cwd));
|
||||
}
|
||||
|
||||
pub fn add_active_exec_command(&mut self, call_id: String, command: Vec<String>) {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use codex_ansi_escape::ansi_escape_line;
|
||||
use codex_core::config::Config;
|
||||
use codex_core::protocol::FileChange;
|
||||
use ratatui::prelude::*;
|
||||
use ratatui::style::Color;
|
||||
@@ -144,9 +145,9 @@ impl HistoryCell {
|
||||
}
|
||||
|
||||
pub(crate) fn new_session_info(
|
||||
config: &Config,
|
||||
model: String,
|
||||
cwd: std::path::PathBuf,
|
||||
approval_policy: codex_core::protocol::AskForApproval,
|
||||
) -> Self {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
|
||||
@@ -158,7 +159,11 @@ impl HistoryCell {
|
||||
]));
|
||||
lines.push(Line::from(vec![
|
||||
"↳ approval: ".bold(),
|
||||
format!("{:?}", approval_policy).into(),
|
||||
format!("{:?}", config.approval_policy).into(),
|
||||
]));
|
||||
lines.push(Line::from(vec![
|
||||
"↳ sandbox: ".bold(),
|
||||
format!("{:?}", config.sandbox_policy).into(),
|
||||
]));
|
||||
lines.push(Line::from(""));
|
||||
|
||||
|
||||
@@ -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;
|
||||
@@ -31,6 +33,23 @@ pub use cli::Cli;
|
||||
pub fn run_main(cli: Cli) -> std::io::Result<()> {
|
||||
assert_env_var_set();
|
||||
|
||||
let config = {
|
||||
// Load configuration and support CLI overrides.
|
||||
let overrides = ConfigOverrides {
|
||||
model: cli.model.clone(),
|
||||
approval_policy: cli.approval_policy.map(Into::into),
|
||||
sandbox_policy: cli.sandbox_policy.map(Into::into),
|
||||
};
|
||||
#[allow(clippy::print_stderr)]
|
||||
match Config::load_with_overrides(overrides) {
|
||||
Ok(config) => config,
|
||||
Err(err) => {
|
||||
eprintln!("Error loading configuration: {err}");
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let log_dir = codex_core::config::log_dir()?;
|
||||
std::fs::create_dir_all(&log_dir)?;
|
||||
// Open (or create) your log file, appending to it.
|
||||
@@ -79,7 +98,7 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> {
|
||||
// `--allow-no-git-exec` flag.
|
||||
let show_git_warning = !cli.skip_git_repo_check && !is_inside_git_repo();
|
||||
|
||||
try_run_ratatui_app(cli, show_git_warning, log_rx);
|
||||
try_run_ratatui_app(cli, config, show_git_warning, log_rx);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -89,16 +108,18 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> {
|
||||
)]
|
||||
fn try_run_ratatui_app(
|
||||
cli: Cli,
|
||||
config: Config,
|
||||
show_git_warning: bool,
|
||||
log_rx: tokio::sync::mpsc::UnboundedReceiver<String>,
|
||||
) {
|
||||
if let Err(report) = run_ratatui_app(cli, show_git_warning, log_rx) {
|
||||
if let Err(report) = run_ratatui_app(cli, config, show_git_warning, log_rx) {
|
||||
eprintln!("Error: {report:?}");
|
||||
}
|
||||
}
|
||||
|
||||
fn run_ratatui_app(
|
||||
cli: Cli,
|
||||
config: Config,
|
||||
show_git_warning: bool,
|
||||
mut log_rx: tokio::sync::mpsc::UnboundedReceiver<String>,
|
||||
) -> color_eyre::Result<()> {
|
||||
@@ -116,23 +137,14 @@ fn run_ratatui_app(
|
||||
let Cli {
|
||||
prompt,
|
||||
images,
|
||||
approval_policy,
|
||||
sandbox_policy: sandbox,
|
||||
model,
|
||||
disable_response_storage,
|
||||
..
|
||||
} = cli;
|
||||
|
||||
let approval_policy = approval_policy.into();
|
||||
let sandbox_policy = sandbox.into();
|
||||
|
||||
let mut app = App::new(
|
||||
approval_policy,
|
||||
sandbox_policy,
|
||||
config,
|
||||
prompt,
|
||||
show_git_warning,
|
||||
images,
|
||||
model,
|
||||
disable_response_storage,
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user