diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 2798dcb92e..c3f8cd7dd5 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -213,6 +213,7 @@ use codex_core::exec::ExecExpiration; use codex_core::exec::ExecParams; use codex_core::exec_env::create_env; use codex_core::find_archived_thread_path_by_id_str; +use codex_core::find_or_unarchive_thread_path_by_id_str; use codex_core::find_thread_name_by_id; use codex_core::find_thread_names_by_ids; use codex_core::find_thread_path_by_id_str; @@ -3852,7 +3853,7 @@ impl CodexMessageProcessor { if path.exists() { path } else { - match find_thread_path_by_id_str( + match find_or_unarchive_thread_path_by_id_str( &self.config.codex_home, &existing_thread_id.to_string(), ) @@ -3878,7 +3879,7 @@ impl CodexMessageProcessor { } } } else { - match find_thread_path_by_id_str( + match find_or_unarchive_thread_path_by_id_str( &self.config.codex_home, &existing_thread_id.to_string(), ) @@ -4036,7 +4037,7 @@ impl CodexMessageProcessor { } }; - match find_thread_path_by_id_str( + match find_or_unarchive_thread_path_by_id_str( &self.config.codex_home, &existing_thread_id.to_string(), ) diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 564de50d3a..c49d005073 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -224,6 +224,62 @@ async fn thread_resume_returns_rollout_history() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_resume_unarchives_archived_rollout() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + + let filename_ts = "2025-01-05T12-00-00"; + let conversation_id = create_fake_rollout_with_text_elements( + codex_home.path(), + filename_ts, + "2025-01-05T12:00:00Z", + "Saved user message", + Vec::new(), + Some("mock_provider"), + None, + )?; + let active_rollout_path = rollout_path(codex_home.path(), filename_ts, &conversation_id); + let archived_rollout_path = codex_home.path().join("archived_sessions/2025/01/05").join( + active_rollout_path + .file_name() + .expect("active rollout file name"), + ); + std::fs::create_dir_all( + archived_rollout_path + .parent() + .expect("archived rollout parent directory"), + )?; + std::fs::rename(&active_rollout_path, &archived_rollout_path)?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id: conversation_id.clone(), + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { thread, .. } = to_response::(resume_resp)?; + + assert_eq!(thread.id, conversation_id); + assert!(active_rollout_path.exists()); + assert!(!archived_rollout_path.exists()); + assert_eq!( + std::fs::canonicalize(thread.path.as_ref().expect("thread path"))?, + std::fs::canonicalize(&active_rollout_path)? + ); + + Ok(()) +} + #[tokio::test] async fn thread_resume_prefers_persisted_git_metadata_for_local_threads() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index c0df326ebe..d4d86586e8 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -339,8 +339,8 @@ fn guardian_assessment_action_value_redacts_apply_patch_patch_text() { guardian_assessment_action_value(&action), serde_json::json!({ "tool": "apply_patch", - "cwd": cwd, - "files": [file], + "cwd": cwd.display().to_string(), + "files": [file.display().to_string()], "change_count": 1, }) ); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index f0390c805a..7616d8975c 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -168,6 +168,7 @@ pub use rollout::append_thread_name; pub use rollout::find_archived_thread_path_by_id_str; #[deprecated(note = "use find_thread_path_by_id_str")] pub use rollout::find_conversation_path_by_id_str; +pub use rollout::find_or_unarchive_thread_path_by_id_str; pub use rollout::find_thread_name_by_id; pub use rollout::find_thread_path_by_id_str; pub use rollout::find_thread_path_by_name_str; diff --git a/codex-rs/core/tests/suite/rollout_list_find.rs b/codex-rs/core/tests/suite/rollout_list_find.rs index 7445a4ba05..74dbc1fcf7 100644 --- a/codex-rs/core/tests/suite/rollout_list_find.rs +++ b/codex-rs/core/tests/suite/rollout_list_find.rs @@ -9,6 +9,7 @@ use codex_core::RolloutRecorder; use codex_core::RolloutRecorderParams; use codex_core::config::ConfigBuilder; use codex_core::find_archived_thread_path_by_id_str; +use codex_core::find_or_unarchive_thread_path_by_id_str; use codex_core::find_thread_path_by_id_str; use codex_core::find_thread_path_by_name_str; use codex_protocol::ThreadId; @@ -219,3 +220,74 @@ async fn find_archived_locates_rollout_file_by_id() { assert_eq!(found, Some(expected)); } + +#[tokio::test] +async fn find_thread_path_by_id_str_does_not_unarchive_archived_rollout() { + let home = TempDir::new().unwrap(); + let id = Uuid::new_v4(); + let archived = write_minimal_rollout_with_id_in_subdir(home.path(), "archived_sessions", id); + + let found = find_thread_path_by_id_str(home.path(), &id.to_string()) + .await + .unwrap(); + + assert_eq!(found, None); + assert!(archived.exists()); +} + +#[tokio::test] +async fn find_or_unarchive_restores_archived_rollout_file_by_id() { + let home = TempDir::new().unwrap(); + let id = Uuid::new_v4(); + let archived = write_minimal_rollout_with_id_in_subdir(home.path(), "archived_sessions", id); + let file_name = archived.file_name().unwrap().to_owned(); + let expected_restored = home.path().join("sessions/2024/01/01").join(file_name); + + let found = find_or_unarchive_thread_path_by_id_str(home.path(), &id.to_string()) + .await + .unwrap(); + + assert_eq!(found, Some(expected_restored.clone())); + assert!(expected_restored.exists()); + assert!(!archived.exists()); + + let archived_found = find_archived_thread_path_by_id_str(home.path(), &id.to_string()) + .await + .unwrap(); + assert_eq!(archived_found, None); +} + +#[tokio::test] +async fn find_does_not_move_unrelated_file_for_stale_archived_db_path() { + let home = TempDir::new().unwrap(); + let requested_id = Uuid::new_v4(); + let requested_thread_id = ThreadId::from_string(&requested_id.to_string()).unwrap(); + let unrelated_id = Uuid::new_v4(); + let unrelated_active_path = write_minimal_rollout_with_id(home.path(), unrelated_id); + + upsert_thread_metadata( + home.path(), + requested_thread_id, + unrelated_active_path.clone(), + ) + .await; + let runtime = StateRuntime::init(home.path().to_path_buf(), "test-provider".to_string()) + .await + .unwrap(); + runtime.mark_backfill_complete(None).await.unwrap(); + runtime + .mark_archived( + requested_thread_id, + unrelated_active_path.as_path(), + Utc::now(), + ) + .await + .unwrap(); + + let found = find_or_unarchive_thread_path_by_id_str(home.path(), &requested_id.to_string()) + .await + .unwrap(); + + assert_eq!(found, None); + assert!(unrelated_active_path.exists()); +} diff --git a/codex-rs/rollout/src/lib.rs b/codex-rs/rollout/src/lib.rs index 160792a390..4ba9796444 100644 --- a/codex-rs/rollout/src/lib.rs +++ b/codex-rs/rollout/src/lib.rs @@ -33,6 +33,7 @@ pub use codex_protocol::protocol::SessionMeta; pub use config::RolloutConfig; pub use config::RolloutConfigView; pub use list::find_archived_thread_path_by_id_str; +pub use list::find_or_unarchive_thread_path_by_id_str; pub use list::find_thread_path_by_id_str; #[deprecated(note = "use find_thread_path_by_id_str")] pub use list::find_thread_path_by_id_str as find_conversation_path_by_id_str; diff --git a/codex-rs/rollout/src/list.rs b/codex-rs/rollout/src/list.rs index e7d3dae5de..b910c5752e 100644 --- a/codex-rs/rollout/src/list.rs +++ b/codex-rs/rollout/src/list.rs @@ -1246,9 +1246,97 @@ async fn find_thread_path_by_id_str_in_subdir( Ok(found) } -/// Locate a recorded thread rollout file by its UUID string using the existing -/// paginated listing implementation. Returns `Ok(Some(path))` if found, `Ok(None)` if not present -/// or the id is invalid. +async fn try_unarchive_thread_path_by_id_str( + codex_home: &Path, + id_str: &str, +) -> io::Result> { + let Ok(requested_id) = Uuid::parse_str(id_str) else { + return Ok(None); + }; + let Some(archived_path) = + find_thread_path_by_id_str_in_subdir(codex_home, ARCHIVED_SESSIONS_SUBDIR, id_str).await? + else { + return Ok(None); + }; + let archived_root = codex_home.join(ARCHIVED_SESSIONS_SUBDIR); + if archived_path.strip_prefix(&archived_root).is_err() { + tracing::error!( + "archived rollout candidate for thread {id_str} is not under {}: {}", + archived_root.display(), + archived_path.display() + ); + return Ok(None); + } + + let Some(file_name) = archived_path.file_name().map(OsStr::to_owned) else { + tracing::error!( + "archived rollout path for thread {id_str} missing file name: {}", + archived_path.display() + ); + return Ok(None); + }; + let file_name_str = file_name.to_string_lossy(); + let Some((_created_at, file_id)) = parse_timestamp_uuid_from_filename(&file_name_str) else { + tracing::error!( + "archived rollout path for thread {id_str} has invalid rollout filename: {}", + archived_path.display() + ); + return Ok(None); + }; + if file_id != requested_id { + tracing::error!( + "archived rollout path for thread {id_str} has mismatched rollout filename: {}", + archived_path.display() + ); + return Ok(None); + } + let Some((year, month, day)) = rollout_date_parts(&file_name) else { + tracing::error!( + "archived rollout path for thread {id_str} missing filename timestamp: {}", + archived_path.display() + ); + return Ok(None); + }; + + let restored_dir = codex_home + .join(SESSIONS_SUBDIR) + .join(year) + .join(month) + .join(day); + tokio::fs::create_dir_all(&restored_dir).await?; + let restored_path = restored_dir.join(&file_name); + match tokio::fs::rename(&archived_path, &restored_path).await { + Ok(()) => {} + Err(err) => { + if tokio::fs::try_exists(&restored_path).await.unwrap_or(false) { + tracing::debug!( + "archived rollout for thread {id_str} already restored concurrently to {}", + restored_path.display() + ); + } else { + return Err(err); + } + } + } + + if let Some(state_db_ctx) = state_db::open_if_present(codex_home, "").await + && let Ok(thread_id) = ThreadId::from_string(id_str) + { + let _ = state_db_ctx + .mark_unarchived(thread_id, restored_path.as_path()) + .await; + } + + Ok(Some(restored_path)) +} + +/// Locate a recorded active thread rollout file by its UUID string using the existing paginated +/// listing implementation. +/// +/// This helper intentionally has no side effects. Callers that want "resume and restore from +/// archive if needed" semantics must opt in with [`find_or_unarchive_thread_path_by_id_str`] so +/// that other lookup paths (for example fork-reference resolution) do not silently move archived +/// rollouts back into `sessions/`. pub async fn find_thread_path_by_id_str( codex_home: &Path, id_str: &str, @@ -1256,6 +1344,18 @@ pub async fn find_thread_path_by_id_str( find_thread_path_by_id_str_in_subdir(codex_home, SESSIONS_SUBDIR, id_str).await } +/// Locate a thread rollout file by UUID string, restoring it from `archived_sessions/` when +/// needed for resume flows. +pub async fn find_or_unarchive_thread_path_by_id_str( + codex_home: &Path, + id_str: &str, +) -> io::Result> { + if let Some(active_path) = find_thread_path_by_id_str(codex_home, id_str).await? { + return Ok(Some(active_path)); + } + try_unarchive_thread_path_by_id_str(codex_home, id_str).await +} + /// Locate an archived thread rollout file by its UUID string. pub async fn find_archived_thread_path_by_id_str( codex_home: &Path, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index f54ff53745..38d26074fa 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -10714,13 +10714,15 @@ async fn permissions_selection_sends_approvals_reviewer_in_override_turn_context "expected permissions popup to open with the current preset selected: {popup}" ); + chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + #[cfg(target_os = "windows")] chat.handle_key_event(KeyEvent::from(KeyCode::Down)); let popup = render_bottom_popup(&chat, /*width*/ 120); assert!( popup .lines() .any(|line| line.contains("Guardian Approvals") && line.contains('›')), - "expected one Down from Default to select Guardian Approvals: {popup}" + "expected permissions popup to select Guardian Approvals: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter));