From 00f7da50b4bedaf9ef4224092b3c8601f156302b Mon Sep 17 00:00:00 2001 From: celia-oai Date: Wed, 3 Dec 2025 18:30:14 -0800 Subject: [PATCH 1/2] changes --- .../app-server-protocol/src/protocol/v2.rs | 7 +++- codex-rs/app-server/src/config_api.rs | 39 ++++++++++++++++--- .../app-server/tests/suite/v2/config_rpc.rs | 11 ++++-- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index f1d8392135..0599f704d5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -209,6 +209,7 @@ pub struct OverriddenMetadata { pub struct ConfigWriteResponse { pub status: WriteStatus, pub version: String, + pub file_path: String, pub overridden_metadata: Option, } @@ -245,10 +246,11 @@ pub struct ConfigReadResponse { #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct ConfigValueWriteParams { - pub file_path: String, pub key_path: String, pub value: JsonValue, pub merge_strategy: MergeStrategy, + /// Path to the config file to write; defaults to the user's `config.toml` when omitted. + pub file_path: Option, pub expected_version: Option, } @@ -256,8 +258,9 @@ pub struct ConfigValueWriteParams { #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct ConfigBatchWriteParams { - pub file_path: String, pub edits: Vec, + /// Path to the config file to write; defaults to the user's `config.toml` when omitted. + pub file_path: Option, pub expected_version: Option, } diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index 68bbdd8c66..970b996644 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -109,12 +109,14 @@ impl ConfigApi { async fn apply_edits( &self, - file_path: String, + file_path: Option, expected_version: Option, edits: Vec<(String, JsonValue, MergeStrategy)>, ) -> Result { let allowed_path = self.codex_home.join(CONFIG_FILE_NAME); - if !paths_match(&allowed_path, &file_path) { + let provided_path = file_path.unwrap_or_else(|| allowed_path.display().to_string()); + + if !paths_match(&allowed_path, &provided_path) { return Err(config_write_error( ConfigWriteErrorCode::ConfigLayerReadonly, "Only writes to the user config are allowed", @@ -193,6 +195,7 @@ impl ConfigApi { Ok(ConfigWriteResponse { status, version: updated_layers.user.version.clone(), + file_path: provided_path, overridden_metadata: overridden, }) } @@ -795,7 +798,7 @@ mod tests { let result = api .write_value(ConfigValueWriteParams { - file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(), + file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), key_path: "approval_policy".to_string(), value: json!("never"), merge_strategy: MergeStrategy::Replace, @@ -832,7 +835,7 @@ mod tests { let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]); let error = api .write_value(ConfigValueWriteParams { - file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(), + file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), key_path: "model".to_string(), value: json!("gpt-5"), merge_strategy: MergeStrategy::Replace, @@ -852,6 +855,30 @@ mod tests { ); } + #[tokio::test] + async fn write_value_defaults_to_user_config_path() { + let tmp = tempdir().expect("tempdir"); + std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "").unwrap(); + + let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]); + api.write_value(ConfigValueWriteParams { + file_path: None, + key_path: "model".to_string(), + value: json!("gpt-new"), + merge_strategy: MergeStrategy::Replace, + expected_version: None, + }) + .await + .expect("write succeeds"); + + let contents = + std::fs::read_to_string(tmp.path().join(CONFIG_FILE_NAME)).expect("read config"); + assert!( + contents.contains("model = \"gpt-new\""), + "config.toml should be updated even when file_path is omitted" + ); + } + #[tokio::test] async fn invalid_user_value_rejected_even_if_overridden_by_managed() { let tmp = tempdir().expect("tempdir"); @@ -872,7 +899,7 @@ mod tests { let error = api .write_value(ConfigValueWriteParams { - file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(), + file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), key_path: "approval_policy".to_string(), value: json!("bogus"), merge_strategy: MergeStrategy::Replace, @@ -957,7 +984,7 @@ mod tests { let result = api .write_value(ConfigValueWriteParams { - file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(), + file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()), key_path: "approval_policy".to_string(), value: json!("on-request"), merge_strategy: MergeStrategy::Replace, diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 343a13c3c4..4c7881a9e4 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -206,7 +206,7 @@ model = "gpt-old" let write_id = mcp .send_config_value_write_request(ConfigValueWriteParams { - file_path: codex_home.path().join("config.toml").display().to_string(), + file_path: None, key_path: "model".to_string(), value: json!("gpt-new"), merge_strategy: MergeStrategy::Replace, @@ -221,6 +221,7 @@ model = "gpt-old" let write: ConfigWriteResponse = to_response(write_resp)?; assert_eq!(write.status, WriteStatus::Ok); + assert!(write.file_path.is_empty()); assert!(write.overridden_metadata.is_none()); let verify_id = mcp @@ -254,7 +255,7 @@ model = "gpt-old" let write_id = mcp .send_config_value_write_request(ConfigValueWriteParams { - file_path: codex_home.path().join("config.toml").display().to_string(), + file_path: Some(codex_home.path().join("config.toml").display().to_string()), key_path: "model".to_string(), value: json!("gpt-new"), merge_strategy: MergeStrategy::Replace, @@ -288,7 +289,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { let batch_id = mcp .send_config_batch_write_request(ConfigBatchWriteParams { - file_path: codex_home.path().join("config.toml").display().to_string(), + file_path: Some(codex_home.path().join("config.toml").display().to_string()), edits: vec![ ConfigEdit { key_path: "sandbox_mode".to_string(), @@ -314,6 +315,10 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { .await??; let batch_write: ConfigWriteResponse = to_response(batch_resp)?; assert_eq!(batch_write.status, WriteStatus::Ok); + assert_eq!( + batch_write.file_path, + codex_home.path().join("config.toml").display().to_string() + ); let read_id = mcp .send_config_read_request(ConfigReadParams { From 292b515b9a13abd5b50d1ba4dd88c6f615a5ed87 Mon Sep 17 00:00:00 2001 From: celia-oai Date: Wed, 3 Dec 2025 18:39:05 -0800 Subject: [PATCH 2/2] changes --- .../app-server-protocol/src/protocol/v2.rs | 1 + codex-rs/app-server/src/config_api.rs | 20 ++++++++++++------ .../app-server/tests/suite/v2/config_rpc.rs | 21 ++++++++++++++----- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 0599f704d5..22d53c3dee 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -209,6 +209,7 @@ pub struct OverriddenMetadata { pub struct ConfigWriteResponse { pub status: WriteStatus, pub version: String, + /// Canonical path to the config file that was written. pub file_path: String, pub overridden_metadata: Option, } diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index 970b996644..ae02927f7a 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -114,7 +114,10 @@ impl ConfigApi { edits: Vec<(String, JsonValue, MergeStrategy)>, ) -> Result { let allowed_path = self.codex_home.join(CONFIG_FILE_NAME); - let provided_path = file_path.unwrap_or_else(|| allowed_path.display().to_string()); + let provided_path = file_path + .as_ref() + .map(PathBuf::from) + .unwrap_or_else(|| allowed_path.clone()); if !paths_match(&allowed_path, &provided_path) { return Err(config_write_error( @@ -192,10 +195,16 @@ impl ConfigApi { .map(|_| WriteStatus::OkOverridden) .unwrap_or(WriteStatus::Ok); + let file_path = provided_path + .canonicalize() + .unwrap_or(provided_path.clone()) + .display() + .to_string(); + Ok(ConfigWriteResponse { status, version: updated_layers.user.version.clone(), - file_path: provided_path, + file_path, overridden_metadata: overridden, }) } @@ -590,15 +599,14 @@ fn canonical_json(value: &JsonValue) -> JsonValue { } } -fn paths_match(expected: &Path, provided: &str) -> bool { - let provided_path = PathBuf::from(provided); +fn paths_match(expected: &Path, provided: &Path) -> bool { if let (Ok(expanded_expected), Ok(expanded_provided)) = - (expected.canonicalize(), provided_path.canonicalize()) + (expected.canonicalize(), provided.canonicalize()) { return expanded_expected == expanded_provided; } - expected == provided_path + expected == provided } fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a TomlValue> { diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 4c7881a9e4..eb3ece64b2 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -219,9 +219,16 @@ model = "gpt-old" ) .await??; let write: ConfigWriteResponse = to_response(write_resp)?; + let expected_file_path = codex_home + .path() + .join("config.toml") + .canonicalize() + .unwrap() + .display() + .to_string(); assert_eq!(write.status, WriteStatus::Ok); - assert!(write.file_path.is_empty()); + assert_eq!(write.file_path, expected_file_path); assert!(write.overridden_metadata.is_none()); let verify_id = mcp @@ -315,10 +322,14 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> { .await??; let batch_write: ConfigWriteResponse = to_response(batch_resp)?; assert_eq!(batch_write.status, WriteStatus::Ok); - assert_eq!( - batch_write.file_path, - codex_home.path().join("config.toml").display().to_string() - ); + let expected_file_path = codex_home + .path() + .join("config.toml") + .canonicalize() + .unwrap() + .display() + .to_string(); + assert_eq!(batch_write.file_path, expected_file_path); let read_id = mcp .send_config_read_request(ConfigReadParams {