Skip to content

Commit 739d4b5

Browse files
authored
fix: do not apply turn cwd to metadata (#12887)
Details here: https://openai.slack.com/archives/C09NZ54M4KY/p1772056758227339
1 parent c528f32 commit 739d4b5

File tree

2 files changed

+210
-21
lines changed

2 files changed

+210
-21
lines changed

codex-rs/core/src/rollout/recorder.rs

Lines changed: 113 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,9 @@ impl RolloutRecorder {
326326
else {
327327
break;
328328
};
329-
if let Some(path) = select_resume_path_from_db_page(&db_page, filter_cwd) {
329+
if let Some(path) =
330+
select_resume_path_from_db_page(&db_page, filter_cwd, default_provider).await
331+
{
330332
return Ok(Some(path));
331333
}
332334
db_cursor = db_page.next_anchor.map(Into::into);
@@ -348,7 +350,7 @@ impl RolloutRecorder {
348350
default_provider,
349351
)
350352
.await?;
351-
if let Some(path) = select_resume_path(&page, filter_cwd) {
353+
if let Some(path) = select_resume_path(&page, filter_cwd, default_provider).await {
352354
return Ok(Some(path));
353355
}
354356
cursor = page.next_cursor;
@@ -961,35 +963,79 @@ impl From<codex_state::ThreadsPage> for ThreadsPage {
961963
}
962964
}
963965

964-
fn select_resume_path(page: &ThreadsPage, filter_cwd: Option<&Path>) -> Option<PathBuf> {
966+
async fn select_resume_path(
967+
page: &ThreadsPage,
968+
filter_cwd: Option<&Path>,
969+
default_provider: &str,
970+
) -> Option<PathBuf> {
965971
match filter_cwd {
966-
Some(cwd) => page.items.iter().find_map(|item| {
967-
if item
968-
.cwd
969-
.as_ref()
970-
.is_some_and(|session_cwd| cwd_matches(session_cwd, cwd))
971-
{
972-
Some(item.path.clone())
973-
} else {
974-
None
972+
Some(cwd) => {
973+
for item in &page.items {
974+
if resume_candidate_matches_cwd(
975+
item.path.as_path(),
976+
item.cwd.as_deref(),
977+
cwd,
978+
default_provider,
979+
)
980+
.await
981+
{
982+
return Some(item.path.clone());
983+
}
975984
}
976-
}),
985+
None
986+
}
977987
None => page.items.first().map(|item| item.path.clone()),
978988
}
979989
}
980990

981-
fn select_resume_path_from_db_page(
991+
async fn resume_candidate_matches_cwd(
992+
rollout_path: &Path,
993+
cached_cwd: Option<&Path>,
994+
cwd: &Path,
995+
default_provider: &str,
996+
) -> bool {
997+
if cached_cwd.is_some_and(|session_cwd| cwd_matches(session_cwd, cwd)) {
998+
return true;
999+
}
1000+
1001+
if let Ok((items, _, _)) = RolloutRecorder::load_rollout_items(rollout_path).await
1002+
&& let Some(latest_turn_context_cwd) = items.iter().rev().find_map(|item| match item {
1003+
RolloutItem::TurnContext(turn_context) => Some(turn_context.cwd.as_path()),
1004+
RolloutItem::SessionMeta(_)
1005+
| RolloutItem::ResponseItem(_)
1006+
| RolloutItem::Compacted(_)
1007+
| RolloutItem::EventMsg(_) => None,
1008+
})
1009+
{
1010+
return cwd_matches(latest_turn_context_cwd, cwd);
1011+
}
1012+
1013+
metadata::extract_metadata_from_rollout(rollout_path, default_provider, None)
1014+
.await
1015+
.is_ok_and(|outcome| cwd_matches(outcome.metadata.cwd.as_path(), cwd))
1016+
}
1017+
1018+
async fn select_resume_path_from_db_page(
9821019
page: &codex_state::ThreadsPage,
9831020
filter_cwd: Option<&Path>,
1021+
default_provider: &str,
9841022
) -> Option<PathBuf> {
9851023
match filter_cwd {
986-
Some(cwd) => page.items.iter().find_map(|item| {
987-
if cwd_matches(item.cwd.as_path(), cwd) {
988-
Some(item.rollout_path.clone())
989-
} else {
990-
None
1024+
Some(cwd) => {
1025+
for item in &page.items {
1026+
if resume_candidate_matches_cwd(
1027+
item.rollout_path.as_path(),
1028+
Some(item.cwd.as_path()),
1029+
cwd,
1030+
default_provider,
1031+
)
1032+
.await
1033+
{
1034+
return Some(item.rollout_path.clone());
1035+
}
9911036
}
992-
}),
1037+
None
1038+
}
9931039
None => page.items.first().map(|item| item.rollout_path.clone()),
9941040
}
9951041
}
@@ -1010,8 +1056,12 @@ mod tests {
10101056
use crate::config::ConfigBuilder;
10111057
use crate::features::Feature;
10121058
use chrono::TimeZone;
1059+
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
10131060
use codex_protocol::protocol::AgentMessageEvent;
1061+
use codex_protocol::protocol::AskForApproval;
10141062
use codex_protocol::protocol::EventMsg;
1063+
use codex_protocol::protocol::SandboxPolicy;
1064+
use codex_protocol::protocol::TurnContextItem;
10151065
use codex_protocol::protocol::UserMessageEvent;
10161066
use pretty_assertions::assert_eq;
10171067
use std::fs::File;
@@ -1320,4 +1370,47 @@ mod tests {
13201370
assert_eq!(repaired_path, Some(real_path));
13211371
Ok(())
13221372
}
1373+
1374+
#[tokio::test]
1375+
async fn resume_candidate_matches_cwd_reads_latest_turn_context() -> std::io::Result<()> {
1376+
let home = TempDir::new().expect("temp dir");
1377+
let stale_cwd = home.path().join("stale");
1378+
let latest_cwd = home.path().join("latest");
1379+
fs::create_dir_all(&stale_cwd)?;
1380+
fs::create_dir_all(&latest_cwd)?;
1381+
1382+
let path = write_session_file(home.path(), "2025-01-03T13-00-00", Uuid::from_u128(9012))?;
1383+
let mut file = std::fs::OpenOptions::new().append(true).open(&path)?;
1384+
let turn_context = RolloutLine {
1385+
timestamp: "2025-01-03T13:00:01Z".to_string(),
1386+
item: RolloutItem::TurnContext(TurnContextItem {
1387+
turn_id: Some("turn-1".to_string()),
1388+
cwd: latest_cwd.clone(),
1389+
approval_policy: AskForApproval::Never,
1390+
sandbox_policy: SandboxPolicy::new_read_only_policy(),
1391+
network: None,
1392+
model: "test-model".to_string(),
1393+
personality: None,
1394+
collaboration_mode: None,
1395+
effort: None,
1396+
summary: ReasoningSummaryConfig::Auto,
1397+
user_instructions: None,
1398+
developer_instructions: None,
1399+
final_output_json_schema: None,
1400+
truncation_policy: None,
1401+
}),
1402+
};
1403+
writeln!(file, "{}", serde_json::to_string(&turn_context)?)?;
1404+
1405+
assert!(
1406+
resume_candidate_matches_cwd(
1407+
path.as_path(),
1408+
Some(stale_cwd.as_path()),
1409+
latest_cwd.as_path(),
1410+
"test-provider",
1411+
)
1412+
.await
1413+
);
1414+
Ok(())
1415+
}
13231416
}

codex-rs/state/src/extract.rs

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &Sessi
5656
}
5757

5858
fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) {
59-
metadata.cwd = turn_ctx.cwd.clone();
59+
if metadata.cwd.as_os_str().is_empty() {
60+
metadata.cwd = turn_ctx.cwd.clone();
61+
}
6062
metadata.sandbox_policy = enum_to_string(&turn_ctx.sandbox_policy);
6163
metadata.approval_mode = enum_to_string(&turn_ctx.approval_policy);
6264
}
@@ -125,10 +127,17 @@ mod tests {
125127
use chrono::DateTime;
126128
use chrono::Utc;
127129
use codex_protocol::ThreadId;
130+
use codex_protocol::config_types::ReasoningSummary;
128131
use codex_protocol::models::ContentItem;
129132
use codex_protocol::models::ResponseItem;
133+
use codex_protocol::protocol::AskForApproval;
130134
use codex_protocol::protocol::EventMsg;
131135
use codex_protocol::protocol::RolloutItem;
136+
use codex_protocol::protocol::SandboxPolicy;
137+
use codex_protocol::protocol::SessionMeta;
138+
use codex_protocol::protocol::SessionMetaLine;
139+
use codex_protocol::protocol::SessionSource;
140+
use codex_protocol::protocol::TurnContextItem;
132141
use codex_protocol::protocol::USER_MESSAGE_BEGIN;
133142
use codex_protocol::protocol::UserMessageEvent;
134143

@@ -209,6 +218,93 @@ mod tests {
209218
assert_eq!(metadata.title, "");
210219
}
211220

221+
#[test]
222+
fn turn_context_does_not_override_session_cwd() {
223+
let mut metadata = metadata_for_test();
224+
metadata.cwd = PathBuf::new();
225+
let thread_id = metadata.id;
226+
227+
apply_rollout_item(
228+
&mut metadata,
229+
&RolloutItem::SessionMeta(SessionMetaLine {
230+
meta: SessionMeta {
231+
id: thread_id,
232+
forked_from_id: Some(
233+
ThreadId::from_string(&Uuid::now_v7().to_string()).expect("thread id"),
234+
),
235+
timestamp: "2026-02-26T00:00:00.000Z".to_string(),
236+
cwd: PathBuf::from("/child/worktree"),
237+
originator: "codex_cli_rs".to_string(),
238+
cli_version: "0.0.0".to_string(),
239+
source: SessionSource::Cli,
240+
agent_nickname: None,
241+
agent_role: None,
242+
model_provider: Some("openai".to_string()),
243+
base_instructions: None,
244+
dynamic_tools: None,
245+
},
246+
git: None,
247+
}),
248+
"test-provider",
249+
);
250+
apply_rollout_item(
251+
&mut metadata,
252+
&RolloutItem::TurnContext(TurnContextItem {
253+
turn_id: Some("turn-1".to_string()),
254+
cwd: PathBuf::from("/parent/workspace"),
255+
approval_policy: AskForApproval::Never,
256+
sandbox_policy: SandboxPolicy::DangerFullAccess,
257+
network: None,
258+
model: "gpt-5".to_string(),
259+
personality: None,
260+
collaboration_mode: None,
261+
effort: None,
262+
summary: ReasoningSummary::Auto,
263+
user_instructions: None,
264+
developer_instructions: None,
265+
final_output_json_schema: None,
266+
truncation_policy: None,
267+
}),
268+
"test-provider",
269+
);
270+
271+
assert_eq!(metadata.cwd, PathBuf::from("/child/worktree"));
272+
assert_eq!(
273+
metadata.sandbox_policy,
274+
super::enum_to_string(&SandboxPolicy::DangerFullAccess)
275+
);
276+
assert_eq!(metadata.approval_mode, "never");
277+
}
278+
279+
#[test]
280+
fn turn_context_sets_cwd_when_session_cwd_missing() {
281+
let mut metadata = metadata_for_test();
282+
metadata.cwd = PathBuf::new();
283+
284+
apply_rollout_item(
285+
&mut metadata,
286+
&RolloutItem::TurnContext(TurnContextItem {
287+
turn_id: Some("turn-1".to_string()),
288+
cwd: PathBuf::from("/fallback/workspace"),
289+
approval_policy: AskForApproval::OnRequest,
290+
sandbox_policy: SandboxPolicy::new_read_only_policy(),
291+
network: None,
292+
model: "gpt-5".to_string(),
293+
personality: None,
294+
collaboration_mode: None,
295+
effort: None,
296+
summary: ReasoningSummary::Auto,
297+
user_instructions: None,
298+
developer_instructions: None,
299+
final_output_json_schema: None,
300+
truncation_policy: None,
301+
}),
302+
"test-provider",
303+
);
304+
305+
assert_eq!(metadata.cwd, PathBuf::from("/fallback/workspace"));
306+
}
307+
212308
fn metadata_for_test() -> ThreadMetadata {
213309
let id = ThreadId::from_string(&Uuid::from_u128(42).to_string()).expect("thread id");
214310
let created_at = DateTime::<Utc>::from_timestamp(1_735_689_600, 0).expect("timestamp");

0 commit comments

Comments
 (0)