Skip to content

Commit 3b88731

Browse files
authored
Merge branch 'openai:main' into main
2 parents 3516d5b + edd98dd commit 3b88731

File tree

24 files changed

+179
-140
lines changed

24 files changed

+179
-140
lines changed

codex-rs/app-server-protocol/src/protocol/v2.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ pub struct OverriddenMetadata {
209209
pub struct ConfigWriteResponse {
210210
pub status: WriteStatus,
211211
pub version: String,
212+
/// Canonical path to the config file that was written.
213+
pub file_path: String,
212214
pub overridden_metadata: Option<OverriddenMetadata>,
213215
}
214216

@@ -245,19 +247,21 @@ pub struct ConfigReadResponse {
245247
#[serde(rename_all = "camelCase")]
246248
#[ts(export_to = "v2/")]
247249
pub struct ConfigValueWriteParams {
248-
pub file_path: String,
249250
pub key_path: String,
250251
pub value: JsonValue,
251252
pub merge_strategy: MergeStrategy,
253+
/// Path to the config file to write; defaults to the user's `config.toml` when omitted.
254+
pub file_path: Option<String>,
252255
pub expected_version: Option<String>,
253256
}
254257

255258
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
256259
#[serde(rename_all = "camelCase")]
257260
#[ts(export_to = "v2/")]
258261
pub struct ConfigBatchWriteParams {
259-
pub file_path: String,
260262
pub edits: Vec<ConfigEdit>,
263+
/// Path to the config file to write; defaults to the user's `config.toml` when omitted.
264+
pub file_path: Option<String>,
261265
pub expected_version: Option<String>,
262266
}
263267

codex-rs/app-server/src/config_api.rs

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,17 @@ impl ConfigApi {
109109

110110
async fn apply_edits(
111111
&self,
112-
file_path: String,
112+
file_path: Option<String>,
113113
expected_version: Option<String>,
114114
edits: Vec<(String, JsonValue, MergeStrategy)>,
115115
) -> Result<ConfigWriteResponse, JSONRPCErrorError> {
116116
let allowed_path = self.codex_home.join(CONFIG_FILE_NAME);
117-
if !paths_match(&allowed_path, &file_path) {
117+
let provided_path = file_path
118+
.as_ref()
119+
.map(PathBuf::from)
120+
.unwrap_or_else(|| allowed_path.clone());
121+
122+
if !paths_match(&allowed_path, &provided_path) {
118123
return Err(config_write_error(
119124
ConfigWriteErrorCode::ConfigLayerReadonly,
120125
"Only writes to the user config are allowed",
@@ -190,9 +195,16 @@ impl ConfigApi {
190195
.map(|_| WriteStatus::OkOverridden)
191196
.unwrap_or(WriteStatus::Ok);
192197

198+
let file_path = provided_path
199+
.canonicalize()
200+
.unwrap_or(provided_path.clone())
201+
.display()
202+
.to_string();
203+
193204
Ok(ConfigWriteResponse {
194205
status,
195206
version: updated_layers.user.version.clone(),
207+
file_path,
196208
overridden_metadata: overridden,
197209
})
198210
}
@@ -587,15 +599,14 @@ fn canonical_json(value: &JsonValue) -> JsonValue {
587599
}
588600
}
589601

590-
fn paths_match(expected: &Path, provided: &str) -> bool {
591-
let provided_path = PathBuf::from(provided);
602+
fn paths_match(expected: &Path, provided: &Path) -> bool {
592603
if let (Ok(expanded_expected), Ok(expanded_provided)) =
593-
(expected.canonicalize(), provided_path.canonicalize())
604+
(expected.canonicalize(), provided.canonicalize())
594605
{
595606
return expanded_expected == expanded_provided;
596607
}
597608

598-
expected == provided_path
609+
expected == provided
599610
}
600611

601612
fn value_at_path<'a>(root: &'a TomlValue, segments: &[String]) -> Option<&'a TomlValue> {
@@ -795,7 +806,7 @@ mod tests {
795806

796807
let result = api
797808
.write_value(ConfigValueWriteParams {
798-
file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(),
809+
file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()),
799810
key_path: "approval_policy".to_string(),
800811
value: json!("never"),
801812
merge_strategy: MergeStrategy::Replace,
@@ -832,7 +843,7 @@ mod tests {
832843
let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]);
833844
let error = api
834845
.write_value(ConfigValueWriteParams {
835-
file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(),
846+
file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()),
836847
key_path: "model".to_string(),
837848
value: json!("gpt-5"),
838849
merge_strategy: MergeStrategy::Replace,
@@ -852,6 +863,30 @@ mod tests {
852863
);
853864
}
854865

866+
#[tokio::test]
867+
async fn write_value_defaults_to_user_config_path() {
868+
let tmp = tempdir().expect("tempdir");
869+
std::fs::write(tmp.path().join(CONFIG_FILE_NAME), "").unwrap();
870+
871+
let api = ConfigApi::new(tmp.path().to_path_buf(), vec![]);
872+
api.write_value(ConfigValueWriteParams {
873+
file_path: None,
874+
key_path: "model".to_string(),
875+
value: json!("gpt-new"),
876+
merge_strategy: MergeStrategy::Replace,
877+
expected_version: None,
878+
})
879+
.await
880+
.expect("write succeeds");
881+
882+
let contents =
883+
std::fs::read_to_string(tmp.path().join(CONFIG_FILE_NAME)).expect("read config");
884+
assert!(
885+
contents.contains("model = \"gpt-new\""),
886+
"config.toml should be updated even when file_path is omitted"
887+
);
888+
}
889+
855890
#[tokio::test]
856891
async fn invalid_user_value_rejected_even_if_overridden_by_managed() {
857892
let tmp = tempdir().expect("tempdir");
@@ -872,7 +907,7 @@ mod tests {
872907

873908
let error = api
874909
.write_value(ConfigValueWriteParams {
875-
file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(),
910+
file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()),
876911
key_path: "approval_policy".to_string(),
877912
value: json!("bogus"),
878913
merge_strategy: MergeStrategy::Replace,
@@ -957,7 +992,7 @@ mod tests {
957992

958993
let result = api
959994
.write_value(ConfigValueWriteParams {
960-
file_path: tmp.path().join(CONFIG_FILE_NAME).display().to_string(),
995+
file_path: Some(tmp.path().join(CONFIG_FILE_NAME).display().to_string()),
961996
key_path: "approval_policy".to_string(),
962997
value: json!("on-request"),
963998
merge_strategy: MergeStrategy::Replace,

codex-rs/app-server/tests/suite/v2/config_rpc.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ model = "gpt-old"
206206

207207
let write_id = mcp
208208
.send_config_value_write_request(ConfigValueWriteParams {
209-
file_path: codex_home.path().join("config.toml").display().to_string(),
209+
file_path: None,
210210
key_path: "model".to_string(),
211211
value: json!("gpt-new"),
212212
merge_strategy: MergeStrategy::Replace,
@@ -219,8 +219,16 @@ model = "gpt-old"
219219
)
220220
.await??;
221221
let write: ConfigWriteResponse = to_response(write_resp)?;
222+
let expected_file_path = codex_home
223+
.path()
224+
.join("config.toml")
225+
.canonicalize()
226+
.unwrap()
227+
.display()
228+
.to_string();
222229

223230
assert_eq!(write.status, WriteStatus::Ok);
231+
assert_eq!(write.file_path, expected_file_path);
224232
assert!(write.overridden_metadata.is_none());
225233

226234
let verify_id = mcp
@@ -254,7 +262,7 @@ model = "gpt-old"
254262

255263
let write_id = mcp
256264
.send_config_value_write_request(ConfigValueWriteParams {
257-
file_path: codex_home.path().join("config.toml").display().to_string(),
265+
file_path: Some(codex_home.path().join("config.toml").display().to_string()),
258266
key_path: "model".to_string(),
259267
value: json!("gpt-new"),
260268
merge_strategy: MergeStrategy::Replace,
@@ -288,7 +296,7 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
288296

289297
let batch_id = mcp
290298
.send_config_batch_write_request(ConfigBatchWriteParams {
291-
file_path: codex_home.path().join("config.toml").display().to_string(),
299+
file_path: Some(codex_home.path().join("config.toml").display().to_string()),
292300
edits: vec![
293301
ConfigEdit {
294302
key_path: "sandbox_mode".to_string(),
@@ -314,6 +322,14 @@ async fn config_batch_write_applies_multiple_edits() -> Result<()> {
314322
.await??;
315323
let batch_write: ConfigWriteResponse = to_response(batch_resp)?;
316324
assert_eq!(batch_write.status, WriteStatus::Ok);
325+
let expected_file_path = codex_home
326+
.path()
327+
.join("config.toml")
328+
.canonicalize()
329+
.unwrap()
330+
.display()
331+
.to_string();
332+
assert_eq!(batch_write.file_path, expected_file_path);
317333

318334
let read_id = mcp
319335
.send_config_read_request(ConfigReadParams {

codex-rs/core/src/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ use crate::default_client::build_reqwest_client;
4646
use crate::error::CodexErr;
4747
use crate::error::Result;
4848
use crate::flags::CODEX_RS_SSE_FIXTURE;
49-
use crate::model_family::ModelFamily;
5049
use crate::model_provider_info::ModelProviderInfo;
5150
use crate::model_provider_info::WireApi;
5251
use crate::openai_model_info::get_model_info;
52+
use crate::openai_models::model_family::ModelFamily;
5353
use crate::tools::spec::create_tools_json_for_chat_completions_api;
5454
use crate::tools::spec::create_tools_json_for_responses_api;
5555

codex-rs/core/src/client_common.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::client_common::tools::ToolSpec;
22
use crate::error::Result;
3-
use crate::model_family::ModelFamily;
3+
use crate::openai_models::model_family::ModelFamily;
44
pub use codex_api::common::ResponseEvent;
55
use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS;
66
use codex_protocol::models::ResponseItem;
@@ -252,7 +252,7 @@ impl Stream for ResponseStream {
252252

253253
#[cfg(test)]
254254
mod tests {
255-
use crate::model_family::find_family_for_model;
255+
use crate::openai_models::model_family::find_family_for_model;
256256
use codex_api::ResponsesApiRequest;
257257
use codex_api::common::OpenAiVerbosity;
258258
use codex_api::common::TextControls;
@@ -309,7 +309,7 @@ mod tests {
309309
},
310310
];
311311
for test_case in test_cases {
312-
let model_family = find_family_for_model(test_case.slug).expect("known model slug");
312+
let model_family = find_family_for_model(test_case.slug);
313313
let expected = if test_case.expects_apply_patch_instructions {
314314
format!(
315315
"{}\n{}",

0 commit comments

Comments
 (0)