mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
Preserve runtime workspace roots across thread resume (#43848)
## Why Resuming a thread should retain its selected workspace folders, including additional roots and explicit empty selections. Resume overrides also need to survive a subsequent resume when no turn has run. ## What changed - Persist `runtime_workspace_roots` in startup metadata and thread settings snapshots, separately from explicit environment selections and permission-profile roots. - Restore roots from the latest snapshot owned by the resumed thread, falling back to owned startup metadata only when no snapshot exists. Honor explicit `runtimeWorkspaceRoots` overrides, retarget the old `cwd` root when `cwd` changes, deduplicate roots, and validate restored paths for the current host. - Checkpoint effective settings on resume and restored settings after revert. Reload resume configuration if saved workspace roots change during loading. - Normalize Windows rollout path spellings when matching thread search results, preserving selection of the correct rollout after revert, including compressed rollouts. ## Testing Add regression coverage for workspace restoration, empty and explicit overrides, foreign paths, compaction and revert, resume checkpoints without recency changes, concurrent settings persistence, and rollout search path matching. GitOrigin-RevId: d98d9d34dd63934d441120916c61c12b69e7f062
This commit is contained in:
@@ -482,6 +482,7 @@ impl ExternalAgentSessionImporter {
|
||||
},
|
||||
dynamic_tools: Vec::new(),
|
||||
selected_capability_roots: Vec::new(),
|
||||
runtime_workspace_roots: None,
|
||||
multi_agent_version: Some(MultiAgentVersion::V1),
|
||||
history_mode: ThreadHistoryMode::Legacy,
|
||||
history_base: None,
|
||||
|
||||
@@ -38,6 +38,7 @@ fn settings_item(
|
||||
permission_profile: PermissionProfile::read_only(),
|
||||
active_permission_profile,
|
||||
cwd: cwd(),
|
||||
runtime_workspace_roots: None,
|
||||
reasoning_effort: None,
|
||||
reasoning_summary: None,
|
||||
personality: None,
|
||||
|
||||
@@ -84,6 +84,7 @@ struct ThreadListFilters {
|
||||
#[derive(PartialEq)]
|
||||
struct ResumeConfigState {
|
||||
history_cwd: Option<PathBuf>,
|
||||
workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
persisted_metadata: Option<ThreadMetadata>,
|
||||
persisted_settings: Option<PersistedResumeSettings>,
|
||||
}
|
||||
@@ -2269,6 +2270,15 @@ impl ThreadRequestProcessor {
|
||||
"failed to restore thread settings after revert: {err}"
|
||||
))
|
||||
})?;
|
||||
// Replace the resume-time checkpoint written from the original config.
|
||||
codex_thread
|
||||
.checkpoint_thread_settings()
|
||||
.await
|
||||
.map_err(|err| {
|
||||
internal_error(format!(
|
||||
"failed to persist restored thread settings after revert: {err}"
|
||||
))
|
||||
})?;
|
||||
Self::set_app_server_client_info(
|
||||
codex_thread.as_ref(),
|
||||
app_server_client_name,
|
||||
@@ -3799,21 +3809,72 @@ impl ThreadRequestProcessor {
|
||||
}
|
||||
|
||||
// Copied or referenced history can contain another thread's settings. Only snapshots
|
||||
// explicitly owned by this thread can override its startup cwd.
|
||||
let history_cwd = if let InitialHistory::Resumed(resumed) = &thread_history {
|
||||
// explicitly owned by this thread can override its startup cwd and workspace folders.
|
||||
let history_settings = if let InitialHistory::Resumed(resumed) = &thread_history {
|
||||
resumed.history.iter().rev().find_map(|item| match item {
|
||||
RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event))
|
||||
if event.thread_id == Some(resumed.conversation_id) =>
|
||||
{
|
||||
Some(event.thread_settings.cwd.to_path_buf())
|
||||
Some(&event.thread_settings)
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let history_cwd = history_cwd.or_else(|| thread_history.session_cwd());
|
||||
let runtime_workspace_roots = runtime_workspace_roots.map(resolve_runtime_workspace_roots);
|
||||
let history_cwd = history_settings
|
||||
.map(|settings| settings.cwd.to_path_buf())
|
||||
.or_else(|| thread_history.session_cwd());
|
||||
let mut runtime_workspace_roots =
|
||||
runtime_workspace_roots.map(resolve_runtime_workspace_roots);
|
||||
if runtime_workspace_roots.is_none() {
|
||||
// A retained owned snapshot is authoritative. Only use startup metadata when
|
||||
// no such snapshot exists; missing roots do not resurrect an older selection.
|
||||
let saved_workspaces: Option<(&Path, Vec<&Path>)> = match history_settings {
|
||||
Some(settings) => settings.runtime_workspace_roots.as_ref().map(|roots| {
|
||||
(
|
||||
settings.cwd.as_path(),
|
||||
roots.iter().map(AbsolutePathBuf::as_path).collect(),
|
||||
)
|
||||
}),
|
||||
None => {
|
||||
if let InitialHistory::Resumed(resumed) = &thread_history
|
||||
&& let Some(RolloutItem::SessionMeta(meta)) = resumed.history.first()
|
||||
&& meta.meta.id == resumed.conversation_id
|
||||
&& let Some(roots) = &meta.meta.runtime_workspace_roots
|
||||
{
|
||||
Some((
|
||||
meta.meta.cwd.as_path(),
|
||||
roots.iter().map(PathBuf::as_path).collect(),
|
||||
))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
};
|
||||
if let Some((saved_cwd, saved_roots)) = saved_workspaces {
|
||||
let resume_cwd = resolve_request_cwd(cwd.as_ref().map(PathBuf::from))?;
|
||||
// Retarget before validation so an explicit cwd can replace a
|
||||
// startup cwd recorded on another operating system.
|
||||
let restored_roots = path_utils::replace_path_and_deduplicate(
|
||||
saved_roots,
|
||||
saved_cwd,
|
||||
resume_cwd.as_ref().map_or(saved_cwd, AbsolutePathBuf::as_path),
|
||||
)
|
||||
.into_iter()
|
||||
.map(|root| {
|
||||
AbsolutePathBuf::from_absolute_path_checked(root).map_err(|err| {
|
||||
invalid_params(format!(
|
||||
"cannot restore workspace root `{}` on this host: {err}. Pass `runtimeWorkspaceRoots` with valid local paths or an empty list.",
|
||||
root.display()
|
||||
))
|
||||
})
|
||||
})
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
// Validation can normalize distinct saved paths to the same root.
|
||||
runtime_workspace_roots = Some(resolve_runtime_workspace_roots(restored_roots));
|
||||
}
|
||||
}
|
||||
let mut typesafe_overrides = self.build_thread_config_overrides(
|
||||
model,
|
||||
model_provider,
|
||||
@@ -3865,6 +3926,7 @@ impl ThreadRequestProcessor {
|
||||
.is_some_and(|metadata| metadata.reasoning_effort.is_none());
|
||||
let config_state = ResumeConfigState {
|
||||
history_cwd: history_cwd.clone(),
|
||||
workspace_roots: typesafe_overrides.workspace_roots.clone(),
|
||||
persisted_metadata,
|
||||
persisted_settings: match &thread_history {
|
||||
InitialHistory::Resumed(resumed) => {
|
||||
|
||||
@@ -113,9 +113,16 @@ fn map_additional_context(
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct ThreadEnvironmentOverride {
|
||||
environments: Option<TurnEnvironmentSelections>,
|
||||
// Only default-environment updates replace the task's separately persisted root selection.
|
||||
runtime_workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
}
|
||||
|
||||
struct ThreadSettingsBuildParams {
|
||||
method: &'static str,
|
||||
environments: Option<TurnEnvironmentSelections>,
|
||||
environment_override: ThreadEnvironmentOverride,
|
||||
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
||||
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
||||
sandbox_policy: Option<codex_app_server_protocol::SandboxPolicy>,
|
||||
@@ -607,7 +614,7 @@ impl TurnRequestProcessor {
|
||||
}
|
||||
};
|
||||
let cwd = resolve_request_cwd(params.cwd)?;
|
||||
let environments = self
|
||||
let environment_override = self
|
||||
.build_environment_override(
|
||||
thread.as_ref(),
|
||||
cwd,
|
||||
@@ -620,7 +627,7 @@ impl TurnRequestProcessor {
|
||||
thread.as_ref(),
|
||||
ThreadSettingsBuildParams {
|
||||
method: "turn/start",
|
||||
environments,
|
||||
environment_override,
|
||||
approval_policy: params.approval_policy,
|
||||
approvals_reviewer: params.approvals_reviewer,
|
||||
sandbox_policy: params.sandbox_policy,
|
||||
@@ -704,9 +711,9 @@ impl TurnRequestProcessor {
|
||||
cwd: Option<AbsolutePathBuf>,
|
||||
workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
environment_selections: Option<Vec<TurnEnvironmentSelection>>,
|
||||
) -> Option<TurnEnvironmentSelections> {
|
||||
) -> ThreadEnvironmentOverride {
|
||||
if cwd.is_none() && workspace_roots.is_none() && environment_selections.is_none() {
|
||||
return None;
|
||||
return ThreadEnvironmentOverride::default();
|
||||
}
|
||||
|
||||
// Explicit environment selections own their roots and pass through unchanged. Top-level
|
||||
@@ -723,42 +730,37 @@ impl TurnRequestProcessor {
|
||||
None => thread.config_snapshot().await.cwd().clone(),
|
||||
},
|
||||
};
|
||||
return Some(TurnEnvironmentSelections::new(
|
||||
legacy_fallback_cwd,
|
||||
environment_selections,
|
||||
));
|
||||
return ThreadEnvironmentOverride {
|
||||
environments: Some(TurnEnvironmentSelections::new(
|
||||
legacy_fallback_cwd,
|
||||
environment_selections,
|
||||
)),
|
||||
..Default::default()
|
||||
};
|
||||
}
|
||||
|
||||
let snapshot = thread.config_snapshot().await;
|
||||
let current_cwd = snapshot.cwd().clone();
|
||||
// Default-environment updates retain the task's fallback roots, not its active roots.
|
||||
let snapshot = thread.thread_settings_snapshot().await;
|
||||
let current_cwd = snapshot.cwd;
|
||||
let legacy_fallback_cwd = cwd.unwrap_or_else(|| current_cwd.clone());
|
||||
let workspace_roots = match workspace_roots {
|
||||
Some(workspace_roots) => workspace_roots,
|
||||
None => {
|
||||
// Match the pre-environment partial-update behavior: a cwd-only update retargets
|
||||
// the old cwd root while preserving any additional roots. Deduplicate because the
|
||||
// new cwd may already be present as an additional root.
|
||||
let mut retargeted_workspace_roots = Vec::new();
|
||||
for root in snapshot.workspace_roots {
|
||||
let root = if root == current_cwd {
|
||||
legacy_fallback_cwd.clone()
|
||||
} else {
|
||||
root
|
||||
};
|
||||
if !retargeted_workspace_roots.contains(&root) {
|
||||
retargeted_workspace_roots.push(root);
|
||||
}
|
||||
}
|
||||
retargeted_workspace_roots
|
||||
}
|
||||
None => path_utils::replace_path_and_deduplicate(
|
||||
snapshot.runtime_workspace_roots.unwrap_or_default(),
|
||||
current_cwd.as_path(),
|
||||
legacy_fallback_cwd.clone(),
|
||||
),
|
||||
};
|
||||
let environment_selections = self
|
||||
.thread_manager
|
||||
.default_environment_selections(&legacy_fallback_cwd, &workspace_roots);
|
||||
Some(TurnEnvironmentSelections::new(
|
||||
legacy_fallback_cwd,
|
||||
environment_selections,
|
||||
))
|
||||
ThreadEnvironmentOverride {
|
||||
environments: Some(TurnEnvironmentSelections::new(
|
||||
legacy_fallback_cwd,
|
||||
environment_selections,
|
||||
)),
|
||||
runtime_workspace_roots: Some(workspace_roots),
|
||||
}
|
||||
}
|
||||
|
||||
async fn build_thread_settings_overrides(
|
||||
@@ -768,7 +770,11 @@ impl TurnRequestProcessor {
|
||||
) -> Result<codex_protocol::protocol::ThreadSettingsOverrides, JSONRPCErrorError> {
|
||||
let ThreadSettingsBuildParams {
|
||||
method,
|
||||
environments,
|
||||
environment_override:
|
||||
ThreadEnvironmentOverride {
|
||||
environments,
|
||||
runtime_workspace_roots,
|
||||
},
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
@@ -865,6 +871,7 @@ impl TurnRequestProcessor {
|
||||
thread
|
||||
.preview_thread_settings_overrides(CodexThreadSettingsOverrides {
|
||||
environments: environments.clone(),
|
||||
runtime_workspace_roots: runtime_workspace_roots.clone(),
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy: sandbox_policy.clone(),
|
||||
@@ -887,6 +894,7 @@ impl TurnRequestProcessor {
|
||||
|
||||
Ok(codex_protocol::protocol::ThreadSettingsOverrides {
|
||||
environments,
|
||||
runtime_workspace_roots,
|
||||
profile_workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
@@ -912,7 +920,7 @@ impl TurnRequestProcessor {
|
||||
self.ensure_direct_input_allowed(request_id, thread.as_ref())
|
||||
.await?;
|
||||
let cwd = resolve_request_cwd(params.cwd)?;
|
||||
let environments = self
|
||||
let environment_override = self
|
||||
.build_environment_override(
|
||||
thread.as_ref(),
|
||||
cwd,
|
||||
@@ -925,7 +933,7 @@ impl TurnRequestProcessor {
|
||||
thread.as_ref(),
|
||||
ThreadSettingsBuildParams {
|
||||
method: "thread/settings/update",
|
||||
environments,
|
||||
environment_override,
|
||||
approval_policy: params.approval_policy,
|
||||
approvals_reviewer: params.approvals_reviewer,
|
||||
sandbox_policy: params.sandbox_policy,
|
||||
|
||||
Reference in New Issue
Block a user