mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Add opaque desktop config namespace (#22584)
## Summary - reserve an explicit opaque `desktop` namespace in `ConfigToml` - expose `desktop` directly in the app-server v2 `config/read` response - keep `config/value/write` and `config/batchWrite` as the only mutation seam for paths like `desktop.someKey` - regenerate the config/app-server schema outputs and document the new contract ## Why The desktop settings work wants one durable, user-editable home for app-owned preferences in `~/.codex/config.toml`, without forcing Rust to model every individual desktop setting key. This PR is only the enabling Rust/app-server layer. It gives the Electron app a first-class config namespace it can read and write through the existing config APIs, while leaving the actual desktop migration to the app PR. ## Behavior and design notes - **Opaque but explicit:** `desktop` is first-class at the typed config root, while its children remain app-owned and open-ended. - **Strict validation still works:** arbitrary nested `desktop.*` keys are accepted instead of being rejected as unknown config. - **Existing config APIs stay the seam:** `config/read` returns the bag, and dotted writes such as `desktop.someKey` continue to flow through `config/value/write` / `config/batchWrite` rather than a bespoke RPC. - **No new consumer behavior:** Core/TUI do not start depending on desktop preferences. This only preserves and exposes the namespace for callers that intentionally use it. - **Same persistence machinery:** hand-edited `config.toml` keeps using the existing TOML edit/write path; this PR does not introduce a second serializer or side channel. - **TOML-friendly values:** the namespace is intended for ordinary JSON-shaped setting values that map cleanly into TOML: strings, numbers, booleans, arrays, and nested object/table values. This PR does not add special handling for TOML-only edge cases such as datetimes. ## Layering semantics Reads keep using the ordinary effective config pipeline, so `desktop` participates in the same layered `config/read` behavior as the rest of `ConfigToml`. Writes still target user config through the existing config service. ## Why this is the shape The alternative would be teaching Rust about each desktop setting as it is added. That would make ordinary app preferences into a cross-repo change, which is exactly the coupling we want to avoid. This keeps the contract small: 1. Rust owns one opaque `desktop` namespace in `config.toml`. 2. The desktop app owns the schema and meaning of individual keys inside it. 3. The existing config APIs remain the transport and mutation surface. That is the piece the desktop settings PR needs in order to move forward cleanly. ## Verification - `cargo test -p codex-config strict_config_accepts_opaque_desktop_keys` - `cargo test -p codex-core desktop_toml_round_trips_opaque_nested_values` - `cargo test -p codex-core config_schema_matches_fixture` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-app-server --test all desktop_settings`
This commit is contained in:
@@ -411,6 +411,52 @@ default_tools_approval_mode = "prompt"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_read_includes_desktop_settings() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
write_config(
|
||||
&codex_home,
|
||||
r#"
|
||||
[desktop]
|
||||
appearanceTheme = "dark"
|
||||
selected-avatar-id = "codex"
|
||||
|
||||
[desktop.workspace]
|
||||
collapsed = true
|
||||
width = 320
|
||||
"#,
|
||||
)?;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let ConfigReadResponse { config, .. } = to_response(resp)?;
|
||||
|
||||
let desktop = config.desktop.expect("desktop settings present");
|
||||
assert_eq!(desktop.get("appearanceTheme"), Some(&json!("dark")));
|
||||
assert_eq!(desktop.get("selected-avatar-id"), Some(&json!("codex")));
|
||||
assert_eq!(
|
||||
desktop.get("workspace"),
|
||||
Some(&json!({
|
||||
"collapsed": true,
|
||||
"width": 320,
|
||||
}))
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_read_includes_project_layers_for_cwd() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -649,6 +695,50 @@ model = "gpt-old"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_value_write_updates_desktop_settings() -> Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
let codex_home = temp_dir.path().canonicalize()?;
|
||||
write_config(&temp_dir, "")?;
|
||||
|
||||
let mut mcp = McpProcess::new(&codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let write_id = mcp
|
||||
.send_config_value_write_request(ConfigValueWriteParams {
|
||||
file_path: None,
|
||||
key_path: "desktop.appearanceTheme".to_string(),
|
||||
value: json!("dark"),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
expected_version: None,
|
||||
})
|
||||
.await?;
|
||||
let write_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(write_id)),
|
||||
)
|
||||
.await??;
|
||||
let write: ConfigWriteResponse = to_response(write_resp)?;
|
||||
assert_eq!(write.status, WriteStatus::Ok);
|
||||
|
||||
let read_id = mcp
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let read_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(read_id)),
|
||||
)
|
||||
.await??;
|
||||
let read: ConfigReadResponse = to_response(read_resp)?;
|
||||
let desktop = read.config.desktop.expect("desktop settings present");
|
||||
assert_eq!(desktop.get("appearanceTheme"), Some(&json!("dark")));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_read_after_pipelined_write_sees_written_value() -> Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
@@ -803,6 +893,70 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn config_batch_write_updates_multiple_desktop_settings() -> Result<()> {
|
||||
let tmp_dir = TempDir::new()?;
|
||||
let codex_home = tmp_dir.path().canonicalize()?;
|
||||
write_config(&tmp_dir, "")?;
|
||||
|
||||
let mut mcp = McpProcess::new(&codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let batch_id = mcp
|
||||
.send_config_batch_write_request(ConfigBatchWriteParams {
|
||||
file_path: Some(codex_home.join("config.toml").display().to_string()),
|
||||
edits: vec![
|
||||
ConfigEdit {
|
||||
key_path: "desktop.selected-avatar-id".to_string(),
|
||||
value: json!("codex"),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
},
|
||||
ConfigEdit {
|
||||
key_path: "desktop.workspace".to_string(),
|
||||
value: json!({
|
||||
"collapsed": true,
|
||||
"width": 320,
|
||||
}),
|
||||
merge_strategy: MergeStrategy::Replace,
|
||||
},
|
||||
],
|
||||
expected_version: None,
|
||||
reload_user_config: false,
|
||||
})
|
||||
.await?;
|
||||
let batch_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(batch_id)),
|
||||
)
|
||||
.await??;
|
||||
let batch_write: ConfigWriteResponse = to_response(batch_resp)?;
|
||||
assert_eq!(batch_write.status, WriteStatus::Ok);
|
||||
|
||||
let read_id = mcp
|
||||
.send_config_read_request(ConfigReadParams {
|
||||
include_layers: false,
|
||||
cwd: None,
|
||||
})
|
||||
.await?;
|
||||
let read_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(read_id)),
|
||||
)
|
||||
.await??;
|
||||
let read: ConfigReadResponse = to_response(read_resp)?;
|
||||
let desktop = read.config.desktop.expect("desktop settings present");
|
||||
assert_eq!(desktop.get("selected-avatar-id"), Some(&json!("codex")));
|
||||
assert_eq!(
|
||||
desktop.get("workspace"),
|
||||
Some(&json!({
|
||||
"collapsed": true,
|
||||
"width": 320,
|
||||
}))
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn assert_layers_user_then_optional_system(
|
||||
layers: &[codex_app_server_protocol::ConfigLayer],
|
||||
user_file: AbsolutePathBuf,
|
||||
|
||||
Reference in New Issue
Block a user