mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Use AbsolutePathBuf for cwd state (#15710)
Migrate `cwd` and related session/config state to `AbsolutePathBuf` so downstream consumers consistently see absolute working directories. Add test-only `.abs()` helpers for `Path`, `PathBuf`, and `TempDir`, and update branch-local tests to use them instead of `AbsolutePathBuf::try_from(...)`. For the remaining TUI/app-server snapshot coverage that renders absolute cwd values, keep the snapshots unchanged and skip the Windows-only cases where the platform-specific absolute path layout differs.
This commit is contained in:
committed by
GitHub
Unverified
parent
178c3b15b4
commit
504aeb0e09
@@ -27,6 +27,9 @@ use serde::Deserialize;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use super::*;
|
||||
use core_test_support::PathBufExt;
|
||||
use core_test_support::PathExt;
|
||||
use core_test_support::TempDirExt;
|
||||
use core_test_support::test_absolute_path;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
@@ -77,6 +80,23 @@ fn http_mcp(url: &str) -> McpServerConfig {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn load_config_normalizes_relative_cwd_override() -> std::io::Result<()> {
|
||||
let expected_cwd = AbsolutePathBuf::relative_to_current_dir("nested")?;
|
||||
let codex_home = tempdir()?;
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides {
|
||||
cwd: Some(PathBuf::from("nested")),
|
||||
..Default::default()
|
||||
},
|
||||
codex_home.abs().into_path_buf(),
|
||||
)?;
|
||||
|
||||
assert_eq!(config.cwd, expected_cwd);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_toml_parsing() {
|
||||
let history_with_persistence = r#"
|
||||
@@ -460,7 +480,7 @@ fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::io::Re
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
let memories_root = AbsolutePathBuf::try_from(codex_home.path().join("memories")).unwrap();
|
||||
let memories_root = codex_home.path().join("memories").abs();
|
||||
assert_eq!(
|
||||
config.permissions.file_system_sandbox_policy,
|
||||
FileSystemSandboxPolicy::restricted(vec![
|
||||
@@ -496,9 +516,7 @@ fn default_permissions_profile_populates_runtime_sandbox_policy() -> std::io::Re
|
||||
writable_roots: vec![memories_root],
|
||||
read_only_access: ReadOnlyAccess::Restricted {
|
||||
include_platform_defaults: true,
|
||||
readable_roots: vec![
|
||||
AbsolutePathBuf::try_from(cwd.path().join("docs")).expect("absolute docs path"),
|
||||
],
|
||||
readable_roots: vec![cwd.path().join("docs").abs(),],
|
||||
},
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: true,
|
||||
@@ -1277,7 +1295,7 @@ fn add_dir_override_extends_workspace_writable_roots() -> std::io::Result<()> {
|
||||
temp_dir.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
let expected_backend = AbsolutePathBuf::try_from(backend).unwrap();
|
||||
let expected_backend = backend.abs();
|
||||
if cfg!(target_os = "windows") {
|
||||
match config.permissions.sandbox_policy.get() {
|
||||
SandboxPolicy::ReadOnly { .. } => {}
|
||||
@@ -1327,7 +1345,7 @@ fn workspace_write_always_includes_memories_root_once() -> std::io::Result<()> {
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml {
|
||||
sandbox_workspace_write: Some(SandboxWorkspaceWrite {
|
||||
writable_roots: vec![AbsolutePathBuf::from_absolute_path(&memories_root)?],
|
||||
writable_roots: vec![memories_root.abs()],
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
@@ -1350,7 +1368,7 @@ fn workspace_write_always_includes_memories_root_once() -> std::io::Result<()> {
|
||||
"expected memories root directory to exist at {}",
|
||||
memories_root.display()
|
||||
);
|
||||
let expected_memories_root = AbsolutePathBuf::from_absolute_path(&memories_root)?;
|
||||
let expected_memories_root = memories_root.abs();
|
||||
match config.permissions.sandbox_policy.get() {
|
||||
SandboxPolicy::WorkspaceWrite { writable_roots, .. } => {
|
||||
assert_eq!(
|
||||
@@ -1768,7 +1786,7 @@ async fn managed_config_overrides_oauth_store_mode() -> anyhow::Result<()> {
|
||||
macos_managed_config_requirements_base64: None,
|
||||
};
|
||||
|
||||
let cwd = AbsolutePathBuf::try_from(codex_home.path())?;
|
||||
let cwd = codex_home.path().abs();
|
||||
let config_layer_stack = load_config_layers_state(
|
||||
codex_home.path(),
|
||||
Some(cwd),
|
||||
@@ -1897,7 +1915,7 @@ async fn managed_config_wins_over_cli_overrides() -> anyhow::Result<()> {
|
||||
macos_managed_config_requirements_base64: None,
|
||||
};
|
||||
|
||||
let cwd = AbsolutePathBuf::try_from(codex_home.path())?;
|
||||
let cwd = codex_home.path().abs();
|
||||
let config_layer_stack = load_config_layers_state(
|
||||
codex_home.path(),
|
||||
Some(cwd),
|
||||
@@ -2933,7 +2951,11 @@ struct PrecedenceTestFixture {
|
||||
}
|
||||
|
||||
impl PrecedenceTestFixture {
|
||||
fn cwd(&self) -> PathBuf {
|
||||
fn cwd(&self) -> AbsolutePathBuf {
|
||||
self.cwd.abs()
|
||||
}
|
||||
|
||||
fn cwd_path(&self) -> PathBuf {
|
||||
self.cwd.path().to_path_buf()
|
||||
}
|
||||
|
||||
@@ -2974,7 +2996,7 @@ fn loads_compact_prompt_from_file() -> std::io::Result<()> {
|
||||
std::fs::write(&prompt_path, " summarize differently ")?;
|
||||
|
||||
let cfg = ConfigToml {
|
||||
experimental_compact_prompt_file: Some(AbsolutePathBuf::from_absolute_path(prompt_path)?),
|
||||
experimental_compact_prompt_file: Some(prompt_path.abs()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -3071,7 +3093,7 @@ fn load_config_rejects_missing_agent_role_config_file() -> std::io::Result<()> {
|
||||
"researcher".to_string(),
|
||||
AgentRoleToml {
|
||||
description: Some("Research role".to_string()),
|
||||
config_file: Some(AbsolutePathBuf::from_absolute_path(missing_path)?),
|
||||
config_file: Some(missing_path.abs()),
|
||||
nickname_candidates: None,
|
||||
},
|
||||
)]),
|
||||
@@ -4082,7 +4104,7 @@ fn model_catalog_json_loads_from_path() -> std::io::Result<()> {
|
||||
)?;
|
||||
|
||||
let cfg = ConfigToml {
|
||||
model_catalog_json: Some(AbsolutePathBuf::from_absolute_path(catalog_path)?),
|
||||
model_catalog_json: Some(catalog_path.abs()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -4103,7 +4125,7 @@ fn model_catalog_json_rejects_empty_catalog() -> std::io::Result<()> {
|
||||
std::fs::write(&catalog_path, r#"{"models":[]}"#)?;
|
||||
|
||||
let cfg = ConfigToml {
|
||||
model_catalog_json: Some(AbsolutePathBuf::from_absolute_path(catalog_path)?),
|
||||
model_catalog_json: Some(catalog_path.abs()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -4240,7 +4262,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
|
||||
|
||||
let o3_profile_overrides = ConfigOverrides {
|
||||
config_profile: Some("o3".to_string()),
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
};
|
||||
let o3_profile_config: Config = Config::load_from_base_config_with_overrides(
|
||||
@@ -4368,7 +4390,7 @@ fn metrics_exporter_defaults_to_statsig_when_missing() -> std::io::Result<()> {
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
fixture.cfg.clone(),
|
||||
ConfigOverrides {
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
},
|
||||
fixture.codex_home(),
|
||||
@@ -4384,7 +4406,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
|
||||
let gpt3_profile_overrides = ConfigOverrides {
|
||||
config_profile: Some("gpt3".to_string()),
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
};
|
||||
let gpt3_profile_config = Config::load_from_base_config_with_overrides(
|
||||
@@ -4505,7 +4527,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
|
||||
// Verify that loading without specifying a profile in ConfigOverrides
|
||||
// uses the default profile from the config file (which is "gpt3").
|
||||
let default_profile_overrides = ConfigOverrides {
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
@@ -4525,7 +4547,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
|
||||
|
||||
let zdr_profile_overrides = ConfigOverrides {
|
||||
config_profile: Some("zdr".to_string()),
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
};
|
||||
let zdr_profile_config = Config::load_from_base_config_with_overrides(
|
||||
@@ -4652,7 +4674,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
|
||||
|
||||
let gpt5_profile_overrides = ConfigOverrides {
|
||||
config_profile: Some("gpt5".to_string()),
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
};
|
||||
let gpt5_profile_config = Config::load_from_base_config_with_overrides(
|
||||
@@ -4820,7 +4842,7 @@ fn test_requirements_web_search_mode_allowlist_does_not_warn_when_unset() -> any
|
||||
let config = Config::load_config_with_layer_stack(
|
||||
fixture.cfg.clone(),
|
||||
ConfigOverrides {
|
||||
cwd: Some(fixture.cwd()),
|
||||
cwd: Some(fixture.cwd_path()),
|
||||
..Default::default()
|
||||
},
|
||||
fixture.codex_home(),
|
||||
|
||||
@@ -395,10 +395,10 @@ pub struct Config {
|
||||
/// Syntax highlighting theme override (kebab-case name).
|
||||
pub tui_theme: Option<String>,
|
||||
|
||||
/// The directory that should be treated as the current working directory
|
||||
/// for the session. All relative paths inside the business-logic layer are
|
||||
/// resolved against this path.
|
||||
pub cwd: PathBuf,
|
||||
/// The absolute directory that should be treated as the current working
|
||||
/// directory for the session. All relative paths inside the business-logic
|
||||
/// layer are resolved against this path.
|
||||
pub cwd: AbsolutePathBuf,
|
||||
|
||||
/// Preferred store for CLI auth credentials.
|
||||
/// file (default): Use a file in the Codex home directory.
|
||||
@@ -683,7 +683,7 @@ impl ConfigBuilder {
|
||||
let loader_overrides = loader_overrides.unwrap_or_default();
|
||||
let cwd_override = harness_overrides.cwd.as_deref().or(fallback_cwd.as_deref());
|
||||
let cwd = match cwd_override {
|
||||
Some(path) => AbsolutePathBuf::try_from(path)?,
|
||||
Some(path) => AbsolutePathBuf::relative_to_current_dir(path)?,
|
||||
None => AbsolutePathBuf::current_dir()?,
|
||||
};
|
||||
harness_overrides.cwd = Some(cwd.to_path_buf());
|
||||
@@ -2104,7 +2104,7 @@ impl Config {
|
||||
let windows_sandbox_mode = resolve_windows_sandbox_mode(&cfg, &config_profile);
|
||||
let windows_sandbox_private_desktop =
|
||||
resolve_windows_sandbox_private_desktop(&cfg, &config_profile);
|
||||
let resolved_cwd = normalize_for_native_workdir({
|
||||
let resolved_cwd = AbsolutePathBuf::try_from(normalize_for_native_workdir({
|
||||
use std::env;
|
||||
|
||||
match cwd {
|
||||
@@ -2121,13 +2121,13 @@ impl Config {
|
||||
current
|
||||
}
|
||||
}
|
||||
});
|
||||
}))?;
|
||||
let mut additional_writable_roots: Vec<AbsolutePathBuf> = additional_writable_roots
|
||||
.into_iter()
|
||||
.map(|path| AbsolutePathBuf::resolve_path_against_base(path, &resolved_cwd))
|
||||
.map(|path| AbsolutePathBuf::resolve_path_against_base(path, resolved_cwd.as_path()))
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
let active_project = cfg
|
||||
.get_active_project(&resolved_cwd)
|
||||
.get_active_project(resolved_cwd.as_path())
|
||||
.unwrap_or(ProjectConfig { trust_level: None });
|
||||
let permission_config_syntax = resolve_permission_config_syntax(
|
||||
&config_layer_stack,
|
||||
@@ -2200,12 +2200,15 @@ impl Config {
|
||||
&mut startup_warnings,
|
||||
)?;
|
||||
let mut sandbox_policy = file_system_sandbox_policy
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?;
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, resolved_cwd.as_path())?;
|
||||
if matches!(sandbox_policy, SandboxPolicy::WorkspaceWrite { .. }) {
|
||||
file_system_sandbox_policy = file_system_sandbox_policy
|
||||
.with_additional_writable_roots(&resolved_cwd, &additional_writable_roots);
|
||||
.with_additional_writable_roots(
|
||||
resolved_cwd.as_path(),
|
||||
&additional_writable_roots,
|
||||
);
|
||||
sandbox_policy = file_system_sandbox_policy
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, &resolved_cwd)?;
|
||||
.to_legacy_sandbox_policy(network_sandbox_policy, resolved_cwd.as_path())?;
|
||||
}
|
||||
(
|
||||
configured_network_proxy_config,
|
||||
@@ -2219,7 +2222,7 @@ impl Config {
|
||||
sandbox_mode,
|
||||
config_profile.sandbox_mode,
|
||||
windows_sandbox_level,
|
||||
&resolved_cwd,
|
||||
resolved_cwd.as_path(),
|
||||
Some(&constrained_sandbox_policy),
|
||||
);
|
||||
if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy {
|
||||
@@ -2229,8 +2232,10 @@ impl Config {
|
||||
}
|
||||
}
|
||||
}
|
||||
let file_system_sandbox_policy =
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, &resolved_cwd);
|
||||
let file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy(
|
||||
&sandbox_policy,
|
||||
resolved_cwd.as_path(),
|
||||
);
|
||||
let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy);
|
||||
(
|
||||
configured_network_proxy_config,
|
||||
@@ -2566,11 +2571,11 @@ impl Config {
|
||||
} else {
|
||||
FileSystemSandboxPolicy::from_legacy_sandbox_policy(
|
||||
&effective_sandbox_policy,
|
||||
&resolved_cwd,
|
||||
resolved_cwd.as_path(),
|
||||
)
|
||||
};
|
||||
let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy
|
||||
.with_additional_readable_roots(&resolved_cwd, &helper_readable_roots);
|
||||
.with_additional_readable_roots(resolved_cwd.as_path(), &helper_readable_roots);
|
||||
let effective_network_sandbox_policy =
|
||||
if effective_sandbox_policy == original_sandbox_policy {
|
||||
network_sandbox_policy
|
||||
|
||||
Reference in New Issue
Block a user