mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
Add persistence support for status line provider selections (#61212)
## What changed - Add a TUI helper that saves `tui.status_line_provider` to the active local user configuration, supporting `"default"` and MCP server/tool selections. - Update the app and chat widget selections only after the write succeeds. - Return an error when `ConfigEdit::SetPath` cannot insert a value at the requested path. ## Testing Add tests covering active local profile writes during remote sessions, preservation of unrelated settings and configuration files, CLI override reloading, and failed writes that leave the file and live selection unchanged. GitOrigin-RevId: 08008f58d7a467bb0c9129b7c198b4d224262ff1
This commit is contained in:
@@ -11,6 +11,7 @@ use codex_features::FEATURES;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
use codex_protocol::config_types::TrustLevel;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use std::borrow::Cow;
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
@@ -330,18 +331,22 @@ impl ConfigDocument {
|
||||
Ok(self.write_value(&segments, value(*enabled)))
|
||||
}
|
||||
ConfigEdit::SetPath { segments, value } => {
|
||||
let mut path = Cow::Borrowed(segments.as_slice());
|
||||
if is_structured_feature_path(segments) && value.as_bool().is_some() {
|
||||
let mut existing = Some(self.doc.as_item());
|
||||
for segment in segments {
|
||||
existing = existing.and_then(|item| item.as_table_like()?.get(segment));
|
||||
}
|
||||
if existing.and_then(TomlItem::as_table_like).is_some() {
|
||||
let mut enabled_segments = segments.clone();
|
||||
enabled_segments.push("enabled".to_string());
|
||||
return Ok(self.insert(&enabled_segments, value.clone()));
|
||||
path.to_mut().push("enabled".to_string());
|
||||
}
|
||||
}
|
||||
Ok(self.insert(segments, value.clone()))
|
||||
anyhow::ensure!(
|
||||
self.insert(&path, value.clone()),
|
||||
"could not set config path `{}`",
|
||||
segments.join(".")
|
||||
);
|
||||
Ok(true)
|
||||
}
|
||||
ConfigEdit::ClearPath { segments } => {
|
||||
let preserves_broker_settings = is_structured_feature_path(segments)
|
||||
|
||||
@@ -270,6 +270,7 @@ mod resize_reflow;
|
||||
mod resume_config;
|
||||
mod resume_preparation;
|
||||
mod right_click_paste;
|
||||
mod status_line_settings;
|
||||
pub(crate) use right_click_paste::PasteEnvironment;
|
||||
mod pane_motion;
|
||||
mod safety_buffering;
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
//! Save the status line provider to the active local user configuration.
|
||||
|
||||
use super::App;
|
||||
use crate::legacy_core::config::edit::ConfigEdit;
|
||||
use crate::legacy_core::config::edit::ConfigEditsBuilder;
|
||||
use anyhow::Context;
|
||||
use codex_config::types::BuiltinStatusLine;
|
||||
use codex_config::types::StatusLineSelection;
|
||||
|
||||
impl App {
|
||||
#[cfg_attr(
|
||||
not(test),
|
||||
expect(dead_code, reason = "called by the upcoming provider picker")
|
||||
)]
|
||||
pub(super) async fn save_status_line_provider(
|
||||
&mut self,
|
||||
provider: StatusLineSelection,
|
||||
) -> anyhow::Result<()> {
|
||||
let value = match &provider {
|
||||
StatusLineSelection::Builtin(BuiltinStatusLine::Default) => toml_edit::value("default"),
|
||||
StatusLineSelection::Mcp(id) => {
|
||||
let table = toml_edit::InlineTable::from_iter([
|
||||
("server", id.server.as_str()),
|
||||
("tool", id.tool.as_str()),
|
||||
]);
|
||||
toml_edit::value(table)
|
||||
}
|
||||
};
|
||||
ConfigEditsBuilder::for_config_path(self.local_settings.user_config_path.as_path())
|
||||
.with_edits([ConfigEdit::SetPath {
|
||||
segments: vec!["tui".into(), "status_line_provider".into()],
|
||||
value,
|
||||
}])
|
||||
.apply()
|
||||
.await
|
||||
.context("Failed to save status line provider")?;
|
||||
|
||||
self.local_settings.tui.status_line_provider = Some(Box::new(provider.clone()));
|
||||
self.chat_widget.local_settings.tui.status_line_provider = Some(Box::new(provider));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "status_line_settings_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,123 @@
|
||||
#![cfg(test)]
|
||||
|
||||
use crate::AppServerTarget;
|
||||
use crate::app::test_support::make_test_app;
|
||||
use codex_config::types::BuiltinStatusLine;
|
||||
use codex_config::types::StatusLineProviderId;
|
||||
use codex_config::types::StatusLineSelection;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_session_saves_to_the_active_local_profile() -> anyhow::Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let base = app.local_settings.user_config_path.clone();
|
||||
let base_text = "[tui]\nstatus_line_provider = { server = 'personal', tool = 'usage' }\n";
|
||||
std::fs::write(&base, base_text)?;
|
||||
let profile = app.local_settings.codex_home.join("work.config.toml");
|
||||
std::fs::write(&profile, "[tui]\ntheme = 'nord'\n")?;
|
||||
app.loader_overrides.user_config_path = Some(profile.clone());
|
||||
app.loader_overrides.user_config_profile = Some("work".parse()?);
|
||||
|
||||
app.cli_kv_overrides = vec![("tui.status_line_provider".into(), "default".into())];
|
||||
app.refresh_in_memory_config_from_disk()
|
||||
.await
|
||||
.expect("load the initial provider from -c");
|
||||
assert_eq!(
|
||||
app.local_settings.tui.status_line_provider.as_deref(),
|
||||
Some(&StatusLineSelection::Builtin(BuiltinStatusLine::Default)),
|
||||
);
|
||||
|
||||
// The running thread's config and cwd belong to the remote app server.
|
||||
let remote = tempfile::tempdir()?;
|
||||
let remote_file = remote.path().join("config.toml");
|
||||
std::fs::write(&remote_file, "[tui]\ntheme = 'remote'\n")?;
|
||||
app.config.codex_home = AbsolutePathBuf::from_absolute_path(remote.path())?;
|
||||
app.config.cwd = app.config.codex_home.clone();
|
||||
app.app_server_target = AppServerTarget::Remote {
|
||||
endpoint: crate::resolve_remote_addr("ws://127.0.0.1:8765").expect("test endpoint"),
|
||||
};
|
||||
for (selection, expected) in [
|
||||
(
|
||||
StatusLineSelection::Builtin(BuiltinStatusLine::Default),
|
||||
toml::toml! {
|
||||
[tui]
|
||||
theme = "nord"
|
||||
status_line_provider = "default"
|
||||
},
|
||||
),
|
||||
(
|
||||
StatusLineSelection::Mcp(StatusLineProviderId {
|
||||
server: "work".into(),
|
||||
tool: "usage".into(),
|
||||
}),
|
||||
toml::toml! {
|
||||
[tui]
|
||||
theme = "nord"
|
||||
status_line_provider = { server = "work", tool = "usage" }
|
||||
},
|
||||
),
|
||||
] {
|
||||
app.save_status_line_provider(selection.clone()).await?;
|
||||
let saved: toml::Table = toml::from_str(&std::fs::read_to_string(&profile)?)?;
|
||||
assert_eq!(saved, expected);
|
||||
assert_eq!(
|
||||
(
|
||||
app.local_settings.tui.status_line_provider.as_deref(),
|
||||
app.chat_widget
|
||||
.local_settings
|
||||
.tui
|
||||
.status_line_provider
|
||||
.as_deref()
|
||||
),
|
||||
(Some(&selection), Some(&selection)),
|
||||
);
|
||||
}
|
||||
app.refresh_in_memory_config_from_disk()
|
||||
.await
|
||||
.expect("reload the CLI override");
|
||||
let default = StatusLineSelection::Builtin(BuiltinStatusLine::Default);
|
||||
assert_eq!(
|
||||
(
|
||||
app.local_settings.tui.status_line_provider.as_deref(),
|
||||
app.chat_widget
|
||||
.local_settings
|
||||
.tui
|
||||
.status_line_provider
|
||||
.as_deref(),
|
||||
),
|
||||
(Some(&default), Some(&default)),
|
||||
);
|
||||
assert_eq!(std::fs::read_to_string(base)?, base_text);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(remote_file)?,
|
||||
"[tui]\ntheme = 'remote'\n"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn failed_write_preserves_the_file_and_the_live_selection() -> anyhow::Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
let path = app.local_settings.user_config_path.clone();
|
||||
let active = StatusLineSelection::Mcp(StatusLineProviderId {
|
||||
server: "work".into(),
|
||||
tool: "usage".into(),
|
||||
});
|
||||
app.save_status_line_provider(active).await?;
|
||||
let before = (
|
||||
app.local_settings.clone(),
|
||||
app.chat_widget.local_settings.clone(),
|
||||
);
|
||||
for contents in ["[malformed", "[[tui]]\ntheme = 'nord'\n"] {
|
||||
std::fs::write(&path, contents)?;
|
||||
let selection = StatusLineSelection::Builtin(BuiltinStatusLine::Default);
|
||||
assert!(app.save_status_line_provider(selection).await.is_err());
|
||||
assert_eq!(std::fs::read_to_string(&path)?, contents);
|
||||
assert_eq!(
|
||||
(&app.local_settings, &app.chat_widget.local_settings),
|
||||
(&before.0, &before.1)
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user