mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Prefer persisted cwd when reading local threads (#37198)
## Why The `cwd` recorded in a rollout can become stale after thread metadata is updated, causing thread reads and lists to disagree with the persisted state. ## What changed - Overlay a non-empty state database `cwd` when reading a thread by ID. - Apply the same overlay when reading by rollout path, but only when the metadata points to the requested rollout. - Recompute the permission profile against the selected `cwd` and continue to fall back to rollout metadata when the persisted `cwd` is empty or belongs to another rollout. ## Testing - Cover stale, empty, and mismatched persisted `cwd` metadata. - Verify that `thread/read`, state-only `thread/list`, and `thread/resume` report the persisted thread `cwd` while the resumed session uses its requested live `cwd`. GitOrigin-RevId: 50a3f187618ee3cadbad95196ee2976e2d5e9a5a
This commit is contained in:
@@ -66,6 +66,9 @@ fn normalized_summary_path(mut summary: ConversationSummary) -> Result<Conversat
|
||||
if !summary.path.as_os_str().is_empty() {
|
||||
summary.path = normalized_canonical_path(summary.path)?;
|
||||
}
|
||||
if !summary.cwd.as_os_str().is_empty() {
|
||||
summary.cwd = AbsolutePathBuf::from_absolute_path(summary.cwd)?.into_path_buf();
|
||||
}
|
||||
Ok(summary)
|
||||
}
|
||||
|
||||
|
||||
@@ -833,6 +833,9 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe
|
||||
-> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
let updated_workspace = TempDir::new()?;
|
||||
let persisted_cwd = normalized_existing_path(updated_workspace.path())?;
|
||||
let live_cwd = normalized_existing_path(codex_home.path())?;
|
||||
mock_responses_config(&server.uri()).write(codex_home.path())?;
|
||||
let config_path = codex_home.path().join("config.toml");
|
||||
let config_toml = std::fs::read_to_string(&config_path)?;
|
||||
@@ -887,6 +890,7 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe
|
||||
model: Some("gpt-5.2-codex".to_string()),
|
||||
effort: Some(ReasoningEffort::Ultra),
|
||||
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
|
||||
cwd: Some(persisted_cwd.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
@@ -898,6 +902,31 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe
|
||||
)
|
||||
.await??;
|
||||
|
||||
let read_id = mcp
|
||||
.send_thread_read_request(ThreadReadParams {
|
||||
thread_id: thread.id.clone(),
|
||||
include_turns: false,
|
||||
})
|
||||
.await?;
|
||||
let ThreadReadResponse { thread: read } =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(read_id)).await??;
|
||||
assert_eq!(read.cwd.as_path(), persisted_cwd);
|
||||
|
||||
let list_id = mcp
|
||||
.send_raw_request(
|
||||
"thread/list",
|
||||
Some(json!({ "cwd": persisted_cwd, "useStateDbOnly": true })),
|
||||
)
|
||||
.await?;
|
||||
let ThreadListResponse { data, .. } =
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(list_id)).await??;
|
||||
assert_eq!(
|
||||
data.iter()
|
||||
.find(|listed| listed.id == thread.id)
|
||||
.map(|listed| &listed.cwd),
|
||||
Some(&read.cwd)
|
||||
);
|
||||
|
||||
thread.id
|
||||
};
|
||||
|
||||
@@ -908,10 +937,13 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id: thread_id.clone(),
|
||||
cwd: Some(live_cwd.to_string_lossy().into_owned()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let ThreadResumeResponse {
|
||||
thread,
|
||||
cwd,
|
||||
model,
|
||||
reasoning_effort,
|
||||
approvals_reviewer,
|
||||
@@ -921,6 +953,8 @@ async fn thread_resume_preserves_acknowledged_model_effort_and_approvals_reviewe
|
||||
assert_eq!(model, "gpt-5.2-codex");
|
||||
assert_eq!(reasoning_effort, Some(ReasoningEffort::Ultra));
|
||||
assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview);
|
||||
assert_eq!(thread.cwd.as_path(), persisted_cwd);
|
||||
assert_eq!(cwd.as_path(), live_cwd);
|
||||
|
||||
let update_id = mcp
|
||||
.send_thread_settings_update_request(ThreadSettingsUpdateParams {
|
||||
|
||||
@@ -65,6 +65,9 @@ pub(super) async fn read_thread(
|
||||
rollout_thread.section = thread.section;
|
||||
rollout_thread.section_position = thread.section_position;
|
||||
rollout_thread.section_entered_at = thread.section_entered_at;
|
||||
if !thread.cwd.as_os_str().is_empty() {
|
||||
rollout_thread.cwd = thread.cwd;
|
||||
}
|
||||
if thread.name.is_some() {
|
||||
rollout_thread.name = thread.name;
|
||||
}
|
||||
@@ -137,6 +140,17 @@ pub(super) async fn read_thread_by_rollout_path(
|
||||
thread.section = metadata.section;
|
||||
thread.section_position = metadata.section_position;
|
||||
thread.section_entered_at = metadata.section_entered_at;
|
||||
if !metadata.cwd.as_os_str().is_empty()
|
||||
&& resolve_requested_rollout_path(store, metadata.rollout_path.clone())
|
||||
.await
|
||||
.is_ok_and(|metadata_rollout_path| metadata_rollout_path == path)
|
||||
{
|
||||
thread.cwd = metadata.cwd;
|
||||
thread.permission_profile = permission_profile_from_metadata_value(
|
||||
&metadata.sandbox_policy,
|
||||
thread.cwd.as_path(),
|
||||
);
|
||||
}
|
||||
let (fallback_sha, fallback_branch, fallback_origin_url) = match thread.git_info.take()
|
||||
{
|
||||
Some(info) => (
|
||||
@@ -1026,7 +1040,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_thread_preserves_rollout_cwd_when_sqlite_metadata_exists() {
|
||||
async fn read_thread_preserves_sqlite_cwd_when_rollout_metadata_is_stale() {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let config = test_config(home.path());
|
||||
let runtime = codex_state::StateRuntime::init(
|
||||
@@ -1076,7 +1090,8 @@ mod tests {
|
||||
SessionSource::Cli,
|
||||
);
|
||||
builder.model_provider = Some(config.default_model_provider_id.clone());
|
||||
builder.cwd = home.path().join("sqlite-workspace");
|
||||
let sqlite_cwd = home.path().join("sqlite-workspace");
|
||||
builder.cwd = sqlite_cwd.clone();
|
||||
let mut metadata = builder.build(config.default_model_provider_id.as_str());
|
||||
metadata.title = "Saved title".to_string();
|
||||
metadata.first_user_message = Some("Hello from sqlite".to_string());
|
||||
@@ -1096,11 +1111,11 @@ mod tests {
|
||||
.expect("read thread");
|
||||
|
||||
assert_eq!(thread.thread_id, thread_id);
|
||||
assert_eq!(thread.rollout_path, Some(rollout_path));
|
||||
assert_eq!(thread.rollout_path, Some(rollout_path.clone()));
|
||||
assert_eq!(thread.preview, "Hello from rollout");
|
||||
assert_eq!(thread.name, Some("Saved title".to_string()));
|
||||
assert_eq!(thread.model_provider, "rollout-provider");
|
||||
assert_eq!(thread.cwd, rollout_cwd);
|
||||
assert_eq!(thread.cwd, sqlite_cwd);
|
||||
let legacy_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
network_access: false,
|
||||
@@ -1111,9 +1126,70 @@ mod tests {
|
||||
thread.permission_profile,
|
||||
PermissionProfile::from_legacy_sandbox_policy_for_cwd(
|
||||
&legacy_policy,
|
||||
rollout_cwd.as_path()
|
||||
sqlite_cwd.as_path()
|
||||
)
|
||||
);
|
||||
|
||||
let thread_by_rollout_path = store
|
||||
.read_thread_by_rollout_path(
|
||||
rollout_path.clone(),
|
||||
/*include_archived*/ false,
|
||||
/*include_history*/ false,
|
||||
)
|
||||
.await
|
||||
.expect("read thread by rollout path");
|
||||
|
||||
assert_eq!(thread_by_rollout_path.cwd, sqlite_cwd);
|
||||
assert_eq!(
|
||||
thread_by_rollout_path.permission_profile,
|
||||
thread.permission_profile
|
||||
);
|
||||
|
||||
metadata.cwd = PathBuf::new();
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("update state db with empty cwd");
|
||||
|
||||
for thread in [
|
||||
store
|
||||
.read_thread(ReadThreadParams {
|
||||
thread_id,
|
||||
include_archived: false,
|
||||
include_history: false,
|
||||
})
|
||||
.await
|
||||
.expect("read thread with empty sqlite cwd"),
|
||||
store
|
||||
.read_thread_by_rollout_path(
|
||||
rollout_path.clone(),
|
||||
/*include_archived*/ false,
|
||||
/*include_history*/ false,
|
||||
)
|
||||
.await
|
||||
.expect("read rollout path with empty sqlite cwd"),
|
||||
] {
|
||||
assert_eq!(thread.cwd, rollout_cwd);
|
||||
}
|
||||
|
||||
metadata.cwd = sqlite_cwd;
|
||||
metadata.rollout_path =
|
||||
write_session_file(home.path(), "2025-01-04T12-00-00", uuid).expect("newer rollout");
|
||||
runtime
|
||||
.upsert_thread(&metadata)
|
||||
.await
|
||||
.expect("update state db with newer rollout");
|
||||
|
||||
let thread_from_requested_rollout = store
|
||||
.read_thread_by_rollout_path(
|
||||
rollout_path,
|
||||
/*include_archived*/ false,
|
||||
/*include_history*/ false,
|
||||
)
|
||||
.await
|
||||
.expect("read explicitly requested rollout");
|
||||
|
||||
assert_eq!(thread_from_requested_rollout.cwd, rollout_cwd);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user