diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index f1d839213..22d53c3de 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -209,6 +209,8 @@ 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, } @@ -245,10 +247,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 +259,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 68bbdd8c6..ae02927f7 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -109,12 +109,17 @@ 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 + .as_ref() + .map(PathBuf::from) + .unwrap_or_else(|| allowed_path.clone()); + + if !paths_match(&allowed_path, &provided_path) { return Err(config_write_error( ConfigWriteErrorCode::ConfigLayerReadonly, "Only writes to the user config are allowed", @@ -190,9 +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, overridden_metadata: overridden, }) } @@ -587,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> { @@ -795,7 +806,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 +843,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 +863,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 +907,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 +992,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 343a13c3c..eb3ece64b 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, @@ -219,8 +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_eq!(write.file_path, expected_file_path); assert!(write.overridden_metadata.is_none()); let verify_id = mcp @@ -254,7 +262,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 +296,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 +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); + 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 {