mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Require absolute cwd in thread settings (#26532)
## Why Thread settings cwd overrides are expected to be resolved before they enter core. Keeping this boundary as a plain `PathBuf` made it easy for core/session code to keep fallback normalization and relative-path resolution logic in places that should only receive an already-resolved cwd. This is intentionally the absolute-cwd-only slice: it does not change environment selection stickiness or cwd-to-default-environment fallback behavior. ## What changed - Changes `ThreadSettingsOverrides.cwd`, `CodexThreadSettingsOverrides.cwd`, and `SessionSettingsUpdate.cwd` to use `AbsolutePathBuf`. - Removes core-side cwd normalization/resolution from session settings updates. - Updates affected core/app-server test helpers and callsites to pass existing absolute cwd values or use `abs()` helpers. ## Validation Opening as draft so CI can start while local validation continues.
This commit is contained in:
@@ -127,7 +127,7 @@ impl ThreadConfigSnapshot {
|
||||
/// Thread settings overrides that app-server validates before starting a turn.
|
||||
#[derive(Clone, Default)]
|
||||
pub struct CodexThreadSettingsOverrides {
|
||||
pub cwd: Option<PathBuf>,
|
||||
pub cwd: Option<AbsolutePathBuf>,
|
||||
pub workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
pub profile_workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
pub approval_policy: Option<AskForApproval>,
|
||||
|
||||
@@ -733,7 +733,7 @@ async fn run_review_on_session(
|
||||
additional_context: Default::default(),
|
||||
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
#[allow(deprecated)]
|
||||
cwd: Some(params.parent_turn.cwd.to_path_buf()),
|
||||
cwd: Some(params.parent_turn.cwd.clone()),
|
||||
approval_policy: Some(AskForApproval::Never),
|
||||
sandbox_policy: None,
|
||||
permission_profile: Some(guardian_permission_profile),
|
||||
|
||||
@@ -32,7 +32,6 @@ use crate::default_skill_metadata_budget;
|
||||
use crate::environment_selection::ResolvedTurnEnvironments;
|
||||
use crate::exec_policy::ExecPolicyManager;
|
||||
use crate::parse_turn_item;
|
||||
use crate::path_utils::normalize_for_native_workdir;
|
||||
use crate::realtime_conversation::RealtimeConversationManager;
|
||||
use crate::session_prefix::format_subagent_notification_message;
|
||||
use crate::skills::SkillRenderSideEffects;
|
||||
|
||||
@@ -251,19 +251,7 @@ impl SessionConfiguration {
|
||||
next_configuration.windows_sandbox_level = windows_sandbox_level;
|
||||
}
|
||||
|
||||
let absolute_cwd = updates
|
||||
.cwd
|
||||
.as_ref()
|
||||
.map(|cwd| {
|
||||
AbsolutePathBuf::relative_to_current_dir(normalize_for_native_workdir(
|
||||
cwd.as_path(),
|
||||
))
|
||||
.unwrap_or_else(|e| {
|
||||
warn!("failed to normalize update cwd: {cwd:?}: {e}");
|
||||
self.cwd.clone()
|
||||
})
|
||||
})
|
||||
.unwrap_or_else(|| self.cwd.clone());
|
||||
let absolute_cwd = updates.cwd.clone().unwrap_or_else(|| self.cwd.clone());
|
||||
|
||||
let cwd_changed = absolute_cwd.as_path() != self.cwd.as_path();
|
||||
next_configuration.cwd = absolute_cwd;
|
||||
@@ -415,7 +403,7 @@ impl SessionConfiguration {
|
||||
|
||||
#[derive(Default, Clone)]
|
||||
pub(crate) struct SessionSettingsUpdate {
|
||||
pub(crate) cwd: Option<PathBuf>,
|
||||
pub(crate) cwd: Option<AbsolutePathBuf>,
|
||||
pub(crate) workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
pub(crate) profile_workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
pub(crate) approval_policy: Option<AskForApproval>,
|
||||
|
||||
@@ -3932,6 +3932,7 @@ async fn session_configuration_apply_preserves_profile_file_system_policy_on_cwd
|
||||
let original_cwd = project_root.join("subdir");
|
||||
let docs_dir = original_cwd.join("docs");
|
||||
std::fs::create_dir_all(&docs_dir).expect("create docs dir");
|
||||
let project_root = project_root.abs();
|
||||
let docs_dir = docs_dir.abs();
|
||||
|
||||
session_configuration.cwd = original_cwd.abs();
|
||||
@@ -4152,7 +4153,7 @@ async fn session_configuration_apply_retargets_implicit_workspace_root_on_cwd_up
|
||||
|
||||
let updated = session_configuration
|
||||
.apply(&SessionSettingsUpdate {
|
||||
cwd: Some(new_root.to_path_buf()),
|
||||
cwd: Some(new_root.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.expect("cwd-only update should succeed");
|
||||
@@ -4391,7 +4392,7 @@ async fn session_configuration_apply_retargets_legacy_workspace_root_on_cwd_upda
|
||||
|
||||
let updated = session_configuration
|
||||
.apply(&SessionSettingsUpdate {
|
||||
cwd: Some(project_root.to_path_buf()),
|
||||
cwd: Some(project_root.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.expect("cwd-only update should succeed");
|
||||
@@ -4420,6 +4421,7 @@ async fn session_configuration_apply_preserves_absolute_cwd_write_root_on_cwd_up
|
||||
std::fs::create_dir_all(&original_cwd).expect("create original cwd");
|
||||
std::fs::create_dir_all(&next_cwd).expect("create next cwd");
|
||||
let original_cwd = original_cwd.abs();
|
||||
let next_cwd = next_cwd.abs();
|
||||
|
||||
session_configuration.cwd = original_cwd.clone();
|
||||
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
|
||||
@@ -4480,7 +4482,7 @@ async fn session_update_settings_does_not_rewrite_sticky_environment_cwds() {
|
||||
|
||||
session
|
||||
.update_settings(SessionSettingsUpdate {
|
||||
cwd: Some(PathBuf::from("project")),
|
||||
cwd: Some(updated_cwd.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await
|
||||
@@ -4516,7 +4518,7 @@ async fn relative_cwd_update_without_environments_resolves_under_session_cwd() {
|
||||
|
||||
session
|
||||
.update_settings(SessionSettingsUpdate {
|
||||
cwd: Some(PathBuf::from("project")),
|
||||
cwd: Some(updated_cwd.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await
|
||||
@@ -4545,7 +4547,7 @@ async fn cwd_update_does_not_rewrite_sticky_environment_cwd() {
|
||||
|
||||
session
|
||||
.update_settings(SessionSettingsUpdate {
|
||||
cwd: Some(PathBuf::from("project")),
|
||||
cwd: Some(updated_cwd.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await
|
||||
@@ -4572,7 +4574,7 @@ async fn absolute_cwd_update_with_turn_environment_is_allowed() {
|
||||
.new_turn_with_sub_id(
|
||||
"sub-1".to_string(),
|
||||
SessionSettingsUpdate {
|
||||
cwd: Some(absolute_cwd.to_path_buf()),
|
||||
cwd: Some(absolute_cwd.clone()),
|
||||
environments: Some(vec![TurnEnvironmentSelection {
|
||||
environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(),
|
||||
cwd: absolute_cwd.clone(),
|
||||
@@ -5989,7 +5991,7 @@ async fn user_turn_updates_approvals_reviewer() {
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: Default::default(),
|
||||
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
cwd: Some(config.cwd.to_path_buf()),
|
||||
cwd: Some(config.cwd.clone()),
|
||||
approval_policy: Some(config.permissions.approval_policy.value()),
|
||||
approvals_reviewer: Some(codex_config::types::ApprovalsReviewer::AutoReview),
|
||||
sandbox_policy: Some(config.legacy_sandbox_policy()),
|
||||
|
||||
Reference in New Issue
Block a user