From 1e9babe1781e63bb5b3ce14c6e34435756f6c422 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 17 Dec 2025 12:08:18 -0800 Subject: [PATCH] fix: PathBuf -> AbsolutePathBuf in ConfigToml struct (#8205) We should not have any `PathBuf` fields in `ConfigToml` or any of the transitive structs we include, as we should use `AbsolutePathBuf` instead so that we do not have to keep track of the file from which `ConfigToml` was loaded such that we need it to resolve relative paths later when the values of `ConfigToml` are used. I only found two instances of this: `experimental_instructions_file` and `experimental_compact_prompt_file`. Incidentally, when these were specified as relative paths, they were resolved against `cwd` rather than `config.toml`'s parent, which seems wrong to me. I changed the behavior so they are resolved against the parent folder of the `config.toml` being parsed, which we get "for free" due to the introduction of `AbsolutePathBufGuard ` in https://github.com/openai/codex/pull/7796. While it is not great to change the behavior of a released feature, these fields are prefixed with `experimental_`, which I interpret to mean we have the liberty to change the contract. For reference: - `experimental_instructions_file` was introduced in https://github.com/openai/codex/pull/1803 - `experimental_compact_prompt_file` was introduced in https://github.com/openai/codex/pull/5959 --- codex-rs/core/src/config/mod.rs | 35 ++++++++++++++--------------- codex-rs/core/src/config/profile.rs | 6 ++--- docs/example-config.md | 2 +- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index bcda301b7c..9a1ad16e94 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -26,7 +26,6 @@ use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; -use crate::util::resolve_path; use codex_app_server_protocol::Tools; use codex_app_server_protocol::UserSavedConfig; use codex_protocol::config_types::ForcedLoginMethod; @@ -692,8 +691,8 @@ pub struct ConfigToml { pub notice: Option, /// Legacy, now use features - pub experimental_instructions_file: Option, - pub experimental_compact_prompt_file: Option, + pub experimental_instructions_file: Option, + pub experimental_compact_prompt_file: Option, pub experimental_use_unified_exec_tool: Option, pub experimental_use_rmcp_client: Option, pub experimental_use_freeform_apply_patch: Option, @@ -1132,9 +1131,8 @@ impl Config { .experimental_instructions_file .as_ref() .or(cfg.experimental_instructions_file.as_ref()); - let file_base_instructions = Self::load_override_from_file( + let file_base_instructions = Self::try_read_non_empty_file( experimental_instructions_path, - &resolved_cwd, "experimental instructions file", )?; let base_instructions = base_instructions.or(file_base_instructions); @@ -1144,9 +1142,8 @@ impl Config { .experimental_compact_prompt_file .as_ref() .or(cfg.experimental_compact_prompt_file.as_ref()); - let file_compact_prompt = Self::load_override_from_file( + let file_compact_prompt = Self::try_read_non_empty_file( experimental_compact_prompt_path, - &resolved_cwd, "experimental compact prompt file", )?; let compact_prompt = compact_prompt.or(file_compact_prompt); @@ -1278,21 +1275,21 @@ impl Config { None } - fn load_override_from_file( - path: Option<&PathBuf>, - cwd: &Path, - description: &str, + /// If `path` is `Some`, attempts to read the file at the given path and + /// returns its contents as a trimmed `String`. If the file is empty, or + /// is `Some` but cannot be read, returns an `Err`. + fn try_read_non_empty_file( + path: Option<&AbsolutePathBuf>, + context: &str, ) -> std::io::Result> { - let Some(p) = path else { + let Some(path) = path else { return Ok(None); }; - let full_path = resolve_path(cwd, p); - - let contents = std::fs::read_to_string(&full_path).map_err(|e| { + let contents = std::fs::read_to_string(path).map_err(|e| { std::io::Error::new( e.kind(), - format!("failed to read {description} {}: {e}", full_path.display()), + format!("failed to read {context} {}: {e}", path.display()), ) })?; @@ -1300,7 +1297,7 @@ impl Config { if s.is_empty() { Err(std::io::Error::new( std::io::ErrorKind::InvalidData, - format!("{description} is empty: {}", full_path.display()), + format!("{context} is empty: {}", path.display()), )) } else { Ok(Some(s)) @@ -2804,7 +2801,9 @@ model = "gpt-5.1-codex" std::fs::write(&prompt_path, " summarize differently ")?; let cfg = ConfigToml { - experimental_compact_prompt_file: Some(PathBuf::from("compact_prompt.txt")), + experimental_compact_prompt_file: Some(AbsolutePathBuf::from_absolute_path( + prompt_path, + )?), ..Default::default() }; diff --git a/codex-rs/core/src/config/profile.rs b/codex-rs/core/src/config/profile.rs index 978e1fcb63..b74b70887d 100644 --- a/codex-rs/core/src/config/profile.rs +++ b/codex-rs/core/src/config/profile.rs @@ -1,5 +1,5 @@ +use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; -use std::path::PathBuf; use crate::protocol::AskForApproval; use codex_protocol::config_types::ReasoningSummary; @@ -21,8 +21,8 @@ pub struct ConfigProfile { pub model_reasoning_summary: Option, pub model_verbosity: Option, pub chatgpt_base_url: Option, - pub experimental_instructions_file: Option, - pub experimental_compact_prompt_file: Option, + pub experimental_instructions_file: Option, + pub experimental_compact_prompt_file: Option, pub include_apply_patch_tool: Option, pub experimental_use_unified_exec_tool: Option, pub experimental_use_rmcp_client: Option, diff --git a/docs/example-config.md b/docs/example-config.md index 0d09a61869..fd69faddde 100644 --- a/docs/example-config.md +++ b/docs/example-config.md @@ -311,7 +311,7 @@ experimental_use_freeform_apply_patch = false # model_reasoning_summary = "auto" # model_verbosity = "medium" # chatgpt_base_url = "https://chatgpt.com/backend-api/" -# experimental_compact_prompt_file = "compact_prompt.txt" +# experimental_compact_prompt_file = "./compact_prompt.txt" # include_apply_patch_tool = false # experimental_use_freeform_apply_patch = false # tools_web_search = false