mirror of
https://github.com/openai/codex.git
synced 2026-11-13 22:47:20 +00:00
Pair thread environment settings (#26687)
## Why Thread cwd and environment selections are a single logical setting in core: updating one without the other can silently desynchronize the next-turn execution context. This change makes that relationship explicit in the internal thread settings flow while preserving the existing app-server public API shape. ## What changed - Moved the cwd/environment pair through internal `ThreadSettingsOverrides.environment_settings` instead of a top-level internal `cwd` field. - Kept `thread/settings/update` public params unchanged, with app-server translating top-level `cwd` into the paired internal settings shape. - Moved `Op::UserInput` environment overrides into thread settings so user turns and settings updates use the same core path. - Updated core, app-server, MCP, memories, sample, and test callsites to construct the paired settings shape. ## Verification - `git diff --check` - Local test run starting after PR creation.
This commit is contained in:
1 parent
f9a680b907
commit
f3c1283411
90 files changed
+521
-820
No files matched your search
@@ -53,7 +53,7 @@ impl AppsRequestProcessor {
|
||||
None
|
||||
};
|
||||
let fallback_cwd = match thread.as_ref() {
|
||||
Some(thread) => Some(thread.config_snapshot().await.cwd.to_path_buf()),
|
||||
Some(thread) => Some(thread.config_snapshot().await.cwd().to_path_buf()),
|
||||
None => None,
|
||||
};
|
||||
let mut config = self.load_latest_config(fallback_cwd).await?;
|
||||
|
||||
@@ -627,6 +627,8 @@ pub(super) async fn handle_pending_thread_resume_request(
|
||||
}
|
||||
}
|
||||
|
||||
let config_snapshot = pending.config_snapshot;
|
||||
let cwd = config_snapshot.cwd().clone();
|
||||
let ThreadConfigSnapshot {
|
||||
model,
|
||||
model_provider_id,
|
||||
@@ -635,11 +637,10 @@ pub(super) async fn handle_pending_thread_resume_request(
|
||||
approvals_reviewer,
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
reasoning_effort,
|
||||
..
|
||||
} = pending.config_snapshot;
|
||||
} = config_snapshot;
|
||||
let instruction_sources = pending.instruction_sources;
|
||||
let sandbox = thread_response_sandbox_policy(&permission_profile, cwd.as_path());
|
||||
let active_permission_profile =
|
||||
|
||||
@@ -47,11 +47,11 @@ fn collect_resume_override_mismatches(
|
||||
}
|
||||
if let Some(requested_cwd) = request.cwd.as_deref() {
|
||||
let requested_cwd_path = std::path::PathBuf::from(requested_cwd);
|
||||
if requested_cwd_path != config_snapshot.cwd.as_path() {
|
||||
if requested_cwd_path != config_snapshot.cwd().as_path() {
|
||||
mismatch_details.push(format!(
|
||||
"cwd requested={} active={}",
|
||||
requested_cwd_path.display(),
|
||||
config_snapshot.cwd.display()
|
||||
config_snapshot.cwd().display()
|
||||
));
|
||||
}
|
||||
}
|
||||
@@ -1154,8 +1154,9 @@ impl ThreadRequestProcessor {
|
||||
|
||||
let sandbox = thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
config_snapshot.cwd.as_path(),
|
||||
config_snapshot.cwd().as_path(),
|
||||
);
|
||||
let cwd = config_snapshot.cwd().clone();
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(config_snapshot.active_permission_profile);
|
||||
|
||||
@@ -1164,7 +1165,7 @@ impl ThreadRequestProcessor {
|
||||
model: config_snapshot.model,
|
||||
model_provider: config_snapshot.model_provider_id,
|
||||
service_tier: config_snapshot.service_tier,
|
||||
cwd: config_snapshot.cwd,
|
||||
cwd,
|
||||
runtime_workspace_roots: config_snapshot.workspace_roots,
|
||||
instruction_sources,
|
||||
approval_policy: config_snapshot.approval_policy.into(),
|
||||
@@ -2601,7 +2602,7 @@ impl ThreadRequestProcessor {
|
||||
let config_snapshot = codex_thread.config_snapshot().await;
|
||||
let sandbox = thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
config_snapshot.cwd.as_path(),
|
||||
config_snapshot.cwd().as_path(),
|
||||
);
|
||||
let active_permission_profile = thread_response_active_permission_profile(
|
||||
config_snapshot.active_permission_profile,
|
||||
@@ -3335,7 +3336,7 @@ impl ThreadRequestProcessor {
|
||||
let config_snapshot = forked_thread.config_snapshot().await;
|
||||
let sandbox = thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
config_snapshot.cwd.as_path(),
|
||||
config_snapshot.cwd().as_path(),
|
||||
);
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(config_snapshot.active_permission_profile);
|
||||
@@ -4205,7 +4206,7 @@ fn build_thread_from_snapshot(
|
||||
updated_at: now,
|
||||
status: ThreadStatus::NotLoaded,
|
||||
path,
|
||||
cwd: config_snapshot.cwd.clone(),
|
||||
cwd: config_snapshot.cwd().clone(),
|
||||
cli_version: env!("CARGO_PKG_VERSION").to_string(),
|
||||
agent_nickname: config_snapshot.session_source.get_nickname(),
|
||||
agent_role: config_snapshot.session_source.get_agent_role(),
|
||||
|
||||
@@ -77,6 +77,7 @@ mod thread_processor_behavior_tests {
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::SubAgentSource;
|
||||
use codex_protocol::protocol::TurnEnvironmentSelections;
|
||||
use codex_state::ThreadMetadataBuilder;
|
||||
use codex_thread_store::StoredThread;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
@@ -685,7 +686,7 @@ mod thread_processor_behavior_tests {
|
||||
approvals_reviewer: codex_protocol::config_types::ApprovalsReviewer::User,
|
||||
permission_profile: codex_protocol::models::PermissionProfile::Disabled,
|
||||
active_permission_profile: None,
|
||||
cwd,
|
||||
environments: TurnEnvironmentSelections::new(cwd, Vec::new()),
|
||||
workspace_roots: Vec::new(),
|
||||
profile_workspace_roots: Vec::new(),
|
||||
ephemeral: false,
|
||||
|
||||
@@ -190,12 +190,12 @@ pub(crate) fn thread_settings_from_config_snapshot(
|
||||
config_snapshot: &ThreadConfigSnapshot,
|
||||
) -> ThreadSettings {
|
||||
ThreadSettings {
|
||||
cwd: config_snapshot.cwd.clone(),
|
||||
cwd: config_snapshot.cwd().clone(),
|
||||
approval_policy: config_snapshot.approval_policy.into(),
|
||||
approvals_reviewer: config_snapshot.approvals_reviewer.into(),
|
||||
sandbox_policy: thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
config_snapshot.cwd.as_path(),
|
||||
config_snapshot.cwd().as_path(),
|
||||
),
|
||||
active_permission_profile: thread_response_active_permission_profile(
|
||||
config_snapshot.active_permission_profile.clone(),
|
||||
@@ -213,24 +213,36 @@ pub(crate) fn thread_settings_from_config_snapshot(
|
||||
pub(crate) fn thread_settings_from_core_snapshot(
|
||||
snapshot: codex_protocol::protocol::ThreadSettingsSnapshot,
|
||||
) -> ThreadSettings {
|
||||
let codex_protocol::protocol::ThreadSettingsSnapshot {
|
||||
model,
|
||||
model_provider_id,
|
||||
service_tier,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
cwd,
|
||||
reasoning_effort,
|
||||
reasoning_summary,
|
||||
personality,
|
||||
collaboration_mode,
|
||||
} = snapshot;
|
||||
let sandbox_policy = thread_response_sandbox_policy(&permission_profile, cwd.as_path());
|
||||
ThreadSettings {
|
||||
sandbox_policy: thread_response_sandbox_policy(
|
||||
&snapshot.permission_profile,
|
||||
snapshot.cwd.as_path(),
|
||||
),
|
||||
cwd: snapshot.cwd,
|
||||
approval_policy: snapshot.approval_policy.into(),
|
||||
approvals_reviewer: snapshot.approvals_reviewer.into(),
|
||||
sandbox_policy,
|
||||
cwd,
|
||||
approval_policy: approval_policy.into(),
|
||||
approvals_reviewer: approvals_reviewer.into(),
|
||||
active_permission_profile: thread_response_active_permission_profile(
|
||||
snapshot.active_permission_profile,
|
||||
active_permission_profile,
|
||||
),
|
||||
model: snapshot.model,
|
||||
model_provider: snapshot.model_provider_id,
|
||||
service_tier: snapshot.service_tier,
|
||||
effort: snapshot.reasoning_effort,
|
||||
summary: snapshot.reasoning_summary,
|
||||
collaboration_mode: snapshot.collaboration_mode,
|
||||
personality: snapshot.personality,
|
||||
model,
|
||||
model_provider: model_provider_id,
|
||||
service_tier,
|
||||
effort: reasoning_effort,
|
||||
summary: reasoning_summary,
|
||||
collaboration_mode,
|
||||
personality,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -43,7 +43,7 @@ fn map_additional_context(
|
||||
|
||||
struct ThreadSettingsBuildParams {
|
||||
method: &'static str,
|
||||
cwd: Option<AbsolutePathBuf>,
|
||||
environments: Option<TurnEnvironmentSelections>,
|
||||
runtime_workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
||||
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
||||
@@ -406,12 +406,14 @@ impl TurnRequestProcessor {
|
||||
let additional_context = map_additional_context(params.additional_context);
|
||||
let turn_has_input = !mapped_items.is_empty();
|
||||
let cwd = resolve_request_cwd(params.cwd)?;
|
||||
let environments =
|
||||
Self::build_environment_override(thread.as_ref(), cwd, environment_selections).await;
|
||||
let thread_settings = self
|
||||
.build_thread_settings_overrides(
|
||||
thread.as_ref(),
|
||||
ThreadSettingsBuildParams {
|
||||
method: "turn/start",
|
||||
cwd,
|
||||
environments,
|
||||
runtime_workspace_roots: params.runtime_workspace_roots,
|
||||
approval_policy: params.approval_policy,
|
||||
approvals_reviewer: params.approvals_reviewer,
|
||||
@@ -430,7 +432,6 @@ impl TurnRequestProcessor {
|
||||
// Start the turn by submitting the user input. Return its submission id as turn_id.
|
||||
let turn_op = Op::UserInput {
|
||||
items: mapped_items,
|
||||
environments: environment_selections,
|
||||
final_output_json_schema: params.output_schema,
|
||||
responsesapi_client_metadata: params.responsesapi_client_metadata,
|
||||
additional_context,
|
||||
@@ -478,6 +479,31 @@ impl TurnRequestProcessor {
|
||||
Ok(TurnStartResponse { turn })
|
||||
}
|
||||
|
||||
async fn build_environment_override(
|
||||
thread: &CodexThread,
|
||||
cwd: Option<AbsolutePathBuf>,
|
||||
environment_selections: Option<Vec<TurnEnvironmentSelection>>,
|
||||
) -> Option<TurnEnvironmentSelections> {
|
||||
if cwd.is_none() && environment_selections.is_none() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let snapshot = thread.config_snapshot().await;
|
||||
let environment_selections =
|
||||
environment_selections.unwrap_or_else(|| snapshot.environment_selections().to_vec());
|
||||
let legacy_fallback_cwd = cwd.unwrap_or_else(|| {
|
||||
environment_selections
|
||||
.iter()
|
||||
.find(|selection| selection.environment_id == LOCAL_ENVIRONMENT_ID)
|
||||
.map(|selection| selection.cwd.clone())
|
||||
.unwrap_or_else(|| snapshot.cwd().clone())
|
||||
});
|
||||
Some(TurnEnvironmentSelections::new(
|
||||
legacy_fallback_cwd,
|
||||
environment_selections,
|
||||
))
|
||||
}
|
||||
|
||||
async fn build_thread_settings_overrides(
|
||||
&self,
|
||||
thread: &CodexThread,
|
||||
@@ -485,7 +511,7 @@ impl TurnRequestProcessor {
|
||||
) -> Result<codex_protocol::protocol::ThreadSettingsOverrides, JSONRPCErrorError> {
|
||||
let ThreadSettingsBuildParams {
|
||||
method,
|
||||
cwd,
|
||||
environments,
|
||||
runtime_workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
@@ -508,6 +534,7 @@ impl TurnRequestProcessor {
|
||||
let collaboration_mode =
|
||||
collaboration_mode.map(|mode| self.normalize_collaboration_mode(mode));
|
||||
let runtime_workspace_roots_request = runtime_workspace_roots;
|
||||
let has_environment_override = environments.is_some();
|
||||
// `thread/settings/update` only acknowledges that the update was queued.
|
||||
// Clients that send dependent partial updates should wait for
|
||||
// `thread/settings/updated` or combine the fields in one request.
|
||||
@@ -517,7 +544,7 @@ impl TurnRequestProcessor {
|
||||
None
|
||||
};
|
||||
|
||||
let has_any_overrides = cwd.is_some()
|
||||
let has_any_overrides = has_environment_override
|
||||
|| runtime_workspace_roots_request.is_some()
|
||||
|| approval_policy.is_some()
|
||||
|| approvals_reviewer.is_some()
|
||||
@@ -545,7 +572,9 @@ impl TurnRequestProcessor {
|
||||
)));
|
||||
};
|
||||
let overrides = ConfigOverrides {
|
||||
cwd: cwd.as_ref().map(AbsolutePathBuf::to_path_buf),
|
||||
cwd: environments
|
||||
.as_ref()
|
||||
.map(|environments| environments.legacy_fallback_cwd.to_path_buf()),
|
||||
workspace_roots: Some(
|
||||
runtime_workspace_roots
|
||||
.clone()
|
||||
@@ -561,7 +590,7 @@ impl TurnRequestProcessor {
|
||||
.load_for_cwd(
|
||||
/*request_overrides*/ None,
|
||||
overrides,
|
||||
Some(snapshot.cwd.to_path_buf()),
|
||||
Some(snapshot.cwd().to_path_buf()),
|
||||
)
|
||||
.await
|
||||
.map_err(|err| config_load_error(&err))?;
|
||||
@@ -588,7 +617,7 @@ impl TurnRequestProcessor {
|
||||
if has_any_overrides {
|
||||
thread
|
||||
.preview_thread_settings_overrides(CodexThreadSettingsOverrides {
|
||||
cwd: cwd.clone(),
|
||||
environments: environments.clone(),
|
||||
workspace_roots: runtime_workspace_roots.clone(),
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
@@ -611,7 +640,7 @@ impl TurnRequestProcessor {
|
||||
}
|
||||
|
||||
Ok(codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
cwd,
|
||||
environments,
|
||||
workspace_roots: runtime_workspace_roots,
|
||||
profile_workspace_roots,
|
||||
approval_policy,
|
||||
@@ -636,12 +665,18 @@ impl TurnRequestProcessor {
|
||||
) -> Result<ThreadSettingsUpdateResponse, JSONRPCErrorError> {
|
||||
let (_, thread) = self.load_thread(¶ms.thread_id).await?;
|
||||
let cwd = resolve_request_cwd(params.cwd)?;
|
||||
let environments = Self::build_environment_override(
|
||||
thread.as_ref(),
|
||||
cwd,
|
||||
/*environment_selections*/ None,
|
||||
)
|
||||
.await;
|
||||
let thread_settings = self
|
||||
.build_thread_settings_overrides(
|
||||
thread.as_ref(),
|
||||
ThreadSettingsBuildParams {
|
||||
method: "thread/settings/update",
|
||||
cwd,
|
||||
environments,
|
||||
runtime_workspace_roots: None,
|
||||
approval_policy: params.approval_policy,
|
||||
approvals_reviewer: params.approvals_reviewer,
|
||||
|
||||
Reference in new issue
Block a user