mirror of
https://github.com/openai/codex.git
synced 2026-04-28 02:11:08 +03:00
fix: do not apply turn cwd to metadata (#12887)
Details here: https://openai.slack.com/archives/C09NZ54M4KY/p1772056758227339
This commit is contained in:
@@ -326,7 +326,9 @@ impl RolloutRecorder {
|
||||
else {
|
||||
break;
|
||||
};
|
||||
if let Some(path) = select_resume_path_from_db_page(&db_page, filter_cwd) {
|
||||
if let Some(path) =
|
||||
select_resume_path_from_db_page(&db_page, filter_cwd, default_provider).await
|
||||
{
|
||||
return Ok(Some(path));
|
||||
}
|
||||
db_cursor = db_page.next_anchor.map(Into::into);
|
||||
@@ -348,7 +350,7 @@ impl RolloutRecorder {
|
||||
default_provider,
|
||||
)
|
||||
.await?;
|
||||
if let Some(path) = select_resume_path(&page, filter_cwd) {
|
||||
if let Some(path) = select_resume_path(&page, filter_cwd, default_provider).await {
|
||||
return Ok(Some(path));
|
||||
}
|
||||
cursor = page.next_cursor;
|
||||
@@ -961,35 +963,79 @@ impl From<codex_state::ThreadsPage> for ThreadsPage {
|
||||
}
|
||||
}
|
||||
|
||||
fn select_resume_path(page: &ThreadsPage, filter_cwd: Option<&Path>) -> Option<PathBuf> {
|
||||
async fn select_resume_path(
|
||||
page: &ThreadsPage,
|
||||
filter_cwd: Option<&Path>,
|
||||
default_provider: &str,
|
||||
) -> Option<PathBuf> {
|
||||
match filter_cwd {
|
||||
Some(cwd) => page.items.iter().find_map(|item| {
|
||||
if item
|
||||
.cwd
|
||||
.as_ref()
|
||||
.is_some_and(|session_cwd| cwd_matches(session_cwd, cwd))
|
||||
{
|
||||
Some(item.path.clone())
|
||||
} else {
|
||||
None
|
||||
Some(cwd) => {
|
||||
for item in &page.items {
|
||||
if resume_candidate_matches_cwd(
|
||||
item.path.as_path(),
|
||||
item.cwd.as_deref(),
|
||||
cwd,
|
||||
default_provider,
|
||||
)
|
||||
.await
|
||||
{
|
||||
return Some(item.path.clone());
|
||||
}
|
||||
}
|
||||
}),
|
||||
None
|
||||
}
|
||||
None => page.items.first().map(|item| item.path.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
fn select_resume_path_from_db_page(
|
||||
async fn resume_candidate_matches_cwd(
|
||||
rollout_path: &Path,
|
||||
cached_cwd: Option<&Path>,
|
||||
cwd: &Path,
|
||||
default_provider: &str,
|
||||
) -> bool {
|
||||
if cached_cwd.is_some_and(|session_cwd| cwd_matches(session_cwd, cwd)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if let Ok((items, _, _)) = RolloutRecorder::load_rollout_items(rollout_path).await
|
||||
&& let Some(latest_turn_context_cwd) = items.iter().rev().find_map(|item| match item {
|
||||
RolloutItem::TurnContext(turn_context) => Some(turn_context.cwd.as_path()),
|
||||
RolloutItem::SessionMeta(_)
|
||||
| RolloutItem::ResponseItem(_)
|
||||
| RolloutItem::Compacted(_)
|
||||
| RolloutItem::EventMsg(_) => None,
|
||||
})
|
||||
{
|
||||
return cwd_matches(latest_turn_context_cwd, cwd);
|
||||
}
|
||||
|
||||
metadata::extract_metadata_from_rollout(rollout_path, default_provider, None)
|
||||
.await
|
||||
.is_ok_and(|outcome| cwd_matches(outcome.metadata.cwd.as_path(), cwd))
|
||||
}
|
||||
|
||||
async fn select_resume_path_from_db_page(
|
||||
page: &codex_state::ThreadsPage,
|
||||
filter_cwd: Option<&Path>,
|
||||
default_provider: &str,
|
||||
) -> Option<PathBuf> {
|
||||
match filter_cwd {
|
||||
Some(cwd) => page.items.iter().find_map(|item| {
|
||||
if cwd_matches(item.cwd.as_path(), cwd) {
|
||||
Some(item.rollout_path.clone())
|
||||
} else {
|
||||
None
|
||||
Some(cwd) => {
|
||||
for item in &page.items {
|
||||
if resume_candidate_matches_cwd(
|
||||
item.rollout_path.as_path(),
|
||||
Some(item.cwd.as_path()),
|
||||
cwd,
|
||||
default_provider,
|
||||
)
|
||||
.await
|
||||
{
|
||||
return Some(item.rollout_path.clone());
|
||||
}
|
||||
}
|
||||
}),
|
||||
None
|
||||
}
|
||||
None => page.items.first().map(|item| item.rollout_path.clone()),
|
||||
}
|
||||
}
|
||||
@@ -1010,8 +1056,12 @@ mod tests {
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::features::Feature;
|
||||
use chrono::TimeZone;
|
||||
use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig;
|
||||
use codex_protocol::protocol::AgentMessageEvent;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs::File;
|
||||
@@ -1320,4 +1370,47 @@ mod tests {
|
||||
assert_eq!(repaired_path, Some(real_path));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resume_candidate_matches_cwd_reads_latest_turn_context() -> std::io::Result<()> {
|
||||
let home = TempDir::new().expect("temp dir");
|
||||
let stale_cwd = home.path().join("stale");
|
||||
let latest_cwd = home.path().join("latest");
|
||||
fs::create_dir_all(&stale_cwd)?;
|
||||
fs::create_dir_all(&latest_cwd)?;
|
||||
|
||||
let path = write_session_file(home.path(), "2025-01-03T13-00-00", Uuid::from_u128(9012))?;
|
||||
let mut file = std::fs::OpenOptions::new().append(true).open(&path)?;
|
||||
let turn_context = RolloutLine {
|
||||
timestamp: "2025-01-03T13:00:01Z".to_string(),
|
||||
item: RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some("turn-1".to_string()),
|
||||
cwd: latest_cwd.clone(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
network: None,
|
||||
model: "test-model".to_string(),
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
effort: None,
|
||||
summary: ReasoningSummaryConfig::Auto,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: None,
|
||||
}),
|
||||
};
|
||||
writeln!(file, "{}", serde_json::to_string(&turn_context)?)?;
|
||||
|
||||
assert!(
|
||||
resume_candidate_matches_cwd(
|
||||
path.as_path(),
|
||||
Some(stale_cwd.as_path()),
|
||||
latest_cwd.as_path(),
|
||||
"test-provider",
|
||||
)
|
||||
.await
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,7 +56,9 @@ fn apply_session_meta_from_item(metadata: &mut ThreadMetadata, meta_line: &Sessi
|
||||
}
|
||||
|
||||
fn apply_turn_context(metadata: &mut ThreadMetadata, turn_ctx: &TurnContextItem) {
|
||||
metadata.cwd = turn_ctx.cwd.clone();
|
||||
if metadata.cwd.as_os_str().is_empty() {
|
||||
metadata.cwd = turn_ctx.cwd.clone();
|
||||
}
|
||||
metadata.sandbox_policy = enum_to_string(&turn_ctx.sandbox_policy);
|
||||
metadata.approval_mode = enum_to_string(&turn_ctx.approval_policy);
|
||||
}
|
||||
@@ -125,10 +127,17 @@ mod tests {
|
||||
use chrono::DateTime;
|
||||
use chrono::Utc;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::config_types::ReasoningSummary;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::RolloutItem;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::protocol::SessionMeta;
|
||||
use codex_protocol::protocol::SessionMetaLine;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use codex_protocol::protocol::USER_MESSAGE_BEGIN;
|
||||
use codex_protocol::protocol::UserMessageEvent;
|
||||
|
||||
@@ -209,6 +218,93 @@ mod tests {
|
||||
assert_eq!(metadata.title, "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_does_not_override_session_cwd() {
|
||||
let mut metadata = metadata_for_test();
|
||||
metadata.cwd = PathBuf::new();
|
||||
let thread_id = metadata.id;
|
||||
|
||||
apply_rollout_item(
|
||||
&mut metadata,
|
||||
&RolloutItem::SessionMeta(SessionMetaLine {
|
||||
meta: SessionMeta {
|
||||
id: thread_id,
|
||||
forked_from_id: Some(
|
||||
ThreadId::from_string(&Uuid::now_v7().to_string()).expect("thread id"),
|
||||
),
|
||||
timestamp: "2026-02-26T00:00:00.000Z".to_string(),
|
||||
cwd: PathBuf::from("/child/worktree"),
|
||||
originator: "codex_cli_rs".to_string(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
agent_nickname: None,
|
||||
agent_role: None,
|
||||
model_provider: Some("openai".to_string()),
|
||||
base_instructions: None,
|
||||
dynamic_tools: None,
|
||||
},
|
||||
git: None,
|
||||
}),
|
||||
"test-provider",
|
||||
);
|
||||
apply_rollout_item(
|
||||
&mut metadata,
|
||||
&RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some("turn-1".to_string()),
|
||||
cwd: PathBuf::from("/parent/workspace"),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
network: None,
|
||||
model: "gpt-5".to_string(),
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: None,
|
||||
}),
|
||||
"test-provider",
|
||||
);
|
||||
|
||||
assert_eq!(metadata.cwd, PathBuf::from("/child/worktree"));
|
||||
assert_eq!(
|
||||
metadata.sandbox_policy,
|
||||
super::enum_to_string(&SandboxPolicy::DangerFullAccess)
|
||||
);
|
||||
assert_eq!(metadata.approval_mode, "never");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_context_sets_cwd_when_session_cwd_missing() {
|
||||
let mut metadata = metadata_for_test();
|
||||
metadata.cwd = PathBuf::new();
|
||||
|
||||
apply_rollout_item(
|
||||
&mut metadata,
|
||||
&RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some("turn-1".to_string()),
|
||||
cwd: PathBuf::from("/fallback/workspace"),
|
||||
approval_policy: AskForApproval::OnRequest,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
network: None,
|
||||
model: "gpt-5".to_string(),
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
user_instructions: None,
|
||||
developer_instructions: None,
|
||||
final_output_json_schema: None,
|
||||
truncation_policy: None,
|
||||
}),
|
||||
"test-provider",
|
||||
);
|
||||
|
||||
assert_eq!(metadata.cwd, PathBuf::from("/fallback/workspace"));
|
||||
}
|
||||
|
||||
fn metadata_for_test() -> ThreadMetadata {
|
||||
let id = ThreadId::from_string(&Uuid::from_u128(42).to_string()).expect("thread id");
|
||||
let created_at = DateTime::<Utc>::from_timestamp(1_735_689_600, 0).expect("timestamp");
|
||||
|
||||
Reference in New Issue
Block a user