diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index c29996b9e171..fe78cffc0771 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -1,4 +1,5 @@ use super::*; +use codex_auto_review::ReviewCoordination; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_protocol::protocol::AdditionalContextEntry as CoreAdditionalContextEntry; use codex_protocol::protocol::AdditionalContextKind as CoreAdditionalContextKind; @@ -355,7 +356,7 @@ impl TurnRequestProcessor { Ok(environment_selections) } - async fn auto_review_target_for_thread(thread: &CodexThread) -> AutoReviewRunTarget { + async fn auto_review_target_for_thread(&self, thread: &CodexThread) -> AutoReviewRunTarget { let snapshot = thread.config_snapshot().await; let environments = thread.environment_selections().await; let cwd = match environments.as_slice() { @@ -366,6 +367,19 @@ impl TurnRequestProcessor { }; let git_info = collect_git_info(cwd.as_path()).await; let repo_root = get_git_repo_root(cwd.as_path()); + let worktree_path = repo_root.or_else(|| Some(cwd.as_path().to_path_buf())); + let snapshot_epoch = worktree_path.as_ref().and_then(|scope| { + match ReviewCoordination::for_scope(self.config.codex_home.as_ref(), scope) + .current_snapshot_epoch() + { + Ok(0) => None, + Ok(epoch) => Some(epoch), + Err(err) => { + tracing::warn!(error = %err, "failed to collect app-server auto review snapshot epoch"); + None + } + } + }); let worktree_diff_fingerprint = get_worktree_diff_fingerprint(cwd.as_path()).await; AutoReviewRunTarget { @@ -374,7 +388,14 @@ impl TurnRequestProcessor { .as_ref() .and_then(|git| git.commit_hash.as_ref().map(|sha| sha.0.clone())), base_sha: None, - worktree_path: repo_root.or_else(|| Some(cwd.as_path().to_path_buf())), + worktree_path, + snapshot_epoch, + snapshot_commit: git_info + .as_ref() + .and_then(|git| git.commit_hash.as_ref().map(|sha| sha.0.clone())), + head_at_launch: git_info + .as_ref() + .and_then(|git| git.commit_hash.as_ref().map(|sha| sha.0.clone())), worktree_diff_fingerprint, } } @@ -1359,7 +1380,7 @@ impl TurnRequestProcessor { let AutoReviewSummaryReadParams { thread_id } = params; let (_, thread) = self.load_thread(&thread_id).await?; let active_review_target = CoreReviewTarget::UncommittedChanges; - let active_target = Self::auto_review_target_for_thread(thread.as_ref()).await; + let active_target = self.auto_review_target_for_thread(thread.as_ref()).await; let store_scope = active_target .worktree_path .as_deref() @@ -1445,7 +1466,7 @@ impl TurnRequestProcessor { let (_, thread) = self.load_thread(&thread_id).await?; let active_review_target = CoreReviewTarget::UncommittedChanges; - let active_target = Self::auto_review_target_for_thread(thread.as_ref()).await; + let active_target = self.auto_review_target_for_thread(thread.as_ref()).await; let store_scope = active_target .worktree_path .as_deref() diff --git a/codex-rs/app-server/tests/suite/v2/review.rs b/codex-rs/app-server/tests/suite/v2/review.rs index 3e657ec79a00..964b7bfda354 100644 --- a/codex-rs/app-server/tests/suite/v2/review.rs +++ b/codex-rs/app-server/tests/suite/v2/review.rs @@ -941,6 +941,9 @@ fn sample_auto_review_run( head_sha: None, base_sha: None, worktree_path: Some(worktree_path.to_path_buf()), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: None, }, review_target: CoreReviewTarget::UncommittedChanges, diff --git a/codex-rs/auto-review/src/lib.rs b/codex-rs/auto-review/src/lib.rs index 1eb1392e9fa7..d14a33c786cb 100644 --- a/codex-rs/auto-review/src/lib.rs +++ b/codex-rs/auto-review/src/lib.rs @@ -152,7 +152,7 @@ impl AutoReviewStore { for run in self.load_index()?.runs { if run.run_id == superseded_by || run.target.worktree_diff_fingerprint.as_deref() != Some(fingerprint) - || !duplicate_target_is_reusable(&run, active_branch, active_head) + || !duplicate_target_matches_branch_head(&run, active_branch, active_head) { continue; } @@ -248,7 +248,54 @@ impl AutoReviewStore { | AutoReviewRunStatus::Superseded ) }) - .filter(|run| duplicate_target_is_reusable(run, active_branch, active_head)) + .filter(|run| duplicate_target_matches_branch_head(run, active_branch, active_head)) + .filter_map(|run| { + let duplicate = AutoReviewDuplicateMatch { + run_id: run.run_id.clone(), + status: run.status.clone(), + disposition: duplicate_disposition(&run), + finding_count: run.findings.len(), + model: run.model.clone(), + }; + is_eligible(&duplicate).then_some((run, duplicate)) + }) + .max_by(|left, right| { + duplicate_priority(&left.0) + .cmp(&duplicate_priority(&right.0)) + .then_with(|| { + auto_review_run_sort_key(&left.0).cmp(&auto_review_run_sort_key(&right.0)) + }) + }) + .map(|(_run, duplicate)| duplicate)) + } + + pub fn find_duplicate_by_fingerprint_with_target_proof_and_filter( + &self, + diff_fingerprint: &str, + active_target: Option<&AutoReviewRunTarget>, + is_eligible: F, + ) -> Result> + where + F: Fn(&AutoReviewDuplicateMatch) -> bool, + { + let fingerprint = diff_fingerprint.trim(); + if fingerprint.is_empty() { + return Ok(None); + } + Ok(self + .load_index_for_read()? + .runs + .into_iter() + .filter(|run| run.target.worktree_diff_fingerprint.as_deref() == Some(fingerprint)) + .filter(|run| { + !matches!( + run.status, + AutoReviewRunStatus::Lost + | AutoReviewRunStatus::Skipped + | AutoReviewRunStatus::Superseded + ) + }) + .filter(|run| duplicate_target_is_reusable(run, active_target)) .filter_map(|run| { let duplicate = AutoReviewDuplicateMatch { run_id: run.run_id.clone(), @@ -670,6 +717,12 @@ pub struct AutoReviewRunTarget { pub base_sha: Option, pub worktree_path: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub snapshot_epoch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub snapshot_commit: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub head_at_launch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub worktree_diff_fingerprint: Option, } @@ -684,6 +737,15 @@ impl AutoReviewRunTarget { if self.base_sha != active.base_sha { return AutoReviewFreshness::Stale; } + if active.snapshot_epoch.is_some() && self.snapshot_epoch != active.snapshot_epoch { + return AutoReviewFreshness::Stale; + } + if self.snapshot_commit.is_some() + && active.snapshot_commit.is_some() + && self.snapshot_commit != active.snapshot_commit + { + return AutoReviewFreshness::Stale; + } if self.worktree_diff_fingerprint != active.worktree_diff_fingerprint { return AutoReviewFreshness::Stale; } @@ -778,7 +840,7 @@ fn duplicate_disposition(run: &AutoReviewRun) -> AutoReviewDuplicateDisposition } } -fn duplicate_target_is_reusable( +fn duplicate_target_matches_branch_head( run: &AutoReviewRun, active_branch: Option<&str>, active_head: Option<&str>, @@ -794,6 +856,15 @@ fn duplicate_target_is_reusable( run.target.head_sha.as_deref() == active_head.and_then(non_empty_str) } +fn duplicate_target_is_reusable( + run: &AutoReviewRun, + active_target: Option<&AutoReviewRunTarget>, +) -> bool { + active_target.is_none_or(|active_target| { + run.target.freshness(active_target) == AutoReviewFreshness::Current + }) +} + fn non_empty_str(value: &str) -> Option<&str> { let value = value.trim(); (!value.is_empty()).then_some(value) diff --git a/codex-rs/auto-review/src/lib_tests.rs b/codex-rs/auto-review/src/lib_tests.rs index cc7a91af62eb..79dd37a3f521 100644 --- a/codex-rs/auto-review/src/lib_tests.rs +++ b/codex-rs/auto-review/src/lib_tests.rs @@ -615,6 +615,66 @@ fn freshness_classifies_current_stale_and_detached() { ); } +#[test] +fn freshness_treats_known_snapshot_epoch_mismatch_as_stale() { + let epoch_zero_active = AutoReviewRunTarget { + snapshot_epoch: None, + ..sample_target("main", "head-2", "/repo") + }; + let active = AutoReviewRunTarget { + snapshot_epoch: Some(2), + ..sample_target("main", "head-2", "/repo") + }; + let stale_epoch = AutoReviewRunTarget { + snapshot_epoch: Some(1), + ..sample_target("main", "head-2", "/repo") + }; + let legacy_without_epoch = sample_target("main", "head-2", "/repo"); + + assert_eq!( + legacy_without_epoch.freshness(&epoch_zero_active), + AutoReviewFreshness::Current + ); + assert_eq!(stale_epoch.freshness(&active), AutoReviewFreshness::Stale); + assert_eq!( + legacy_without_epoch.freshness(&active), + AutoReviewFreshness::Stale + ); +} + +#[test] +fn freshness_treats_known_snapshot_commit_mismatch_as_stale() { + let active = AutoReviewRunTarget { + snapshot_commit: Some("snapshot-new".to_string()), + ..sample_target("main", "head-2", "/repo") + }; + let stale_snapshot = AutoReviewRunTarget { + snapshot_commit: Some("snapshot-old".to_string()), + ..sample_target("main", "head-2", "/repo") + }; + + assert_eq!( + stale_snapshot.freshness(&active), + AutoReviewFreshness::Stale + ); +} + +#[test] +fn legacy_run_targets_without_snapshot_proof_still_deserialize() -> anyhow::Result<()> { + let target: AutoReviewRunTarget = serde_json::from_value(serde_json::json!({ + "branch": "main", + "head_sha": "head-2", + "base_sha": "base-1", + "worktree_path": "/repo" + }))?; + + assert_eq!(target.snapshot_epoch, None); + assert_eq!(target.snapshot_commit, None); + assert_eq!(target.head_at_launch, None); + assert_eq!(target.worktree_diff_fingerprint, None); + Ok(()) +} + #[test] fn summary_only_surfaces_current_findings() { let active = sample_target("main", "head-2", "/repo"); @@ -800,6 +860,35 @@ fn duplicate_lookup_can_filter_stale_target_matches() -> anyhow::Result<()> { Ok(()) } +#[test] +fn duplicate_lookup_with_target_proof_rejects_mismatched_snapshot_epoch() -> anyhow::Result<()> { + let codex_home = tempfile::tempdir()?; + let scope = tempfile::tempdir()?; + let store = AutoReviewStore::for_scope(codex_home.path(), scope.path()); + store.save_run(&AutoReviewRun { + target: AutoReviewRunTarget { + snapshot_epoch: Some(1), + worktree_diff_fingerprint: Some("sha256:abc".to_string()), + ..sample_target("main", "head-2", "/repo") + }, + ..sample_run("stale_duplicate", vec![sample_finding("f1", "Old")]) + })?; + let active_target = AutoReviewRunTarget { + snapshot_epoch: Some(2), + worktree_diff_fingerprint: Some("sha256:abc".to_string()), + ..sample_target("main", "head-2", "/repo") + }; + + let duplicate = store.find_duplicate_by_fingerprint_with_target_proof_and_filter( + "sha256:abc", + Some(&active_target), + |_| true, + )?; + + assert_eq!(duplicate, None); + Ok(()) +} + #[test] fn duplicate_lookup_reports_pending_but_ignores_lost_skipped_and_superseded() -> anyhow::Result<()> { @@ -1067,6 +1156,9 @@ fn commit_review_targets_ignore_checkout_metadata_when_reopened() { head_sha: Some("abc123".to_string()), base_sha: Some("base-old".to_string()), worktree_path: Some(PathBuf::from("/repo-old")), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: Some("sha256:old".to_string()), }, review_target: ReviewTarget::Commit { @@ -1080,6 +1172,9 @@ fn commit_review_targets_ignore_checkout_metadata_when_reopened() { head_sha: Some("abc123".to_string()), base_sha: None, worktree_path: None, + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: Some("sha256:new".to_string()), }; let reopened_renamed_branch = AutoReviewRunTarget { @@ -1087,6 +1182,9 @@ fn commit_review_targets_ignore_checkout_metadata_when_reopened() { head_sha: Some("abc123".to_string()), base_sha: Some("base-new".to_string()), worktree_path: Some(PathBuf::from("/repo-new")), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: Some("sha256:new".to_string()), }; @@ -1266,6 +1364,9 @@ fn sample_target(branch: &str, head_sha: &str, worktree_path: &str) -> AutoRevie head_sha: Some(head_sha.to_string()), base_sha: Some("base-1".to_string()), worktree_path: Some(PathBuf::from(worktree_path)), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: None, } } diff --git a/codex-rs/core/src/context/auto_review_awareness.rs b/codex-rs/core/src/context/auto_review_awareness.rs index 6ed29adece1e..c4e11f972675 100644 --- a/codex-rs/core/src/context/auto_review_awareness.rs +++ b/codex-rs/core/src/context/auto_review_awareness.rs @@ -84,7 +84,7 @@ async fn build_auto_review_awareness_inner( } let active_review_target = ReviewTarget::UncommittedChanges; - let active_target = collect_auto_review_target(cwd, &active_review_target).await; + let active_target = collect_auto_review_target(codex_home, cwd, &active_review_target).await; let store_scope = active_target.worktree_path.as_deref().unwrap_or(cwd); let runs = AutoReviewStore::for_scope(codex_home, store_scope).list_runs()?; if runs.is_empty() diff --git a/codex-rs/core/src/context/auto_review_awareness_tests.rs b/codex-rs/core/src/context/auto_review_awareness_tests.rs index f79d400bf2fe..7995d8b01101 100644 --- a/codex-rs/core/src/context/auto_review_awareness_tests.rs +++ b/codex-rs/core/src/context/auto_review_awareness_tests.rs @@ -233,6 +233,9 @@ fn sample_target(branch: &str, head_sha: &str, worktree_path: &str) -> AutoRevie head_sha: Some(head_sha.to_string()), base_sha: Some("base-1".to_string()), worktree_path: Some(PathBuf::from(worktree_path)), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: None, } } diff --git a/codex-rs/core/src/review_persistence.rs b/codex-rs/core/src/review_persistence.rs index d35a5919d099..215883c7bf99 100644 --- a/codex-rs/core/src/review_persistence.rs +++ b/codex-rs/core/src/review_persistence.rs @@ -10,6 +10,7 @@ use codex_auto_review::AutoReviewRunSource; use codex_auto_review::AutoReviewRunStatus; use codex_auto_review::AutoReviewRunTarget; use codex_auto_review::AutoReviewStore; +use codex_auto_review::ReviewCoordination; use codex_auto_review::SCHEMA_VERSION; use codex_git_utils::collect_git_info; use codex_git_utils::get_git_repo_root; @@ -42,10 +43,11 @@ impl ReviewPersistenceContext { run_id: String, mode: ReviewPersistence, review_target: ReviewTarget, + codex_home: &Path, cwd: &Path, model: Option, ) -> Self { - let target = collect_auto_review_target(cwd, &review_target).await; + let target = collect_auto_review_target(codex_home, cwd, &review_target).await; let store_scope = target .worktree_path .clone() @@ -84,6 +86,16 @@ impl ReviewPersistenceContext { &self.target } + pub(crate) async fn refresh_target(mut self, codex_home: &Path, cwd: &Path) -> Self { + let target = collect_auto_review_target(codex_home, cwd, &self.review_target).await; + self.store_scope = target + .worktree_path + .clone() + .unwrap_or_else(|| cwd.to_path_buf()); + self.target = target; + self + } + pub(crate) fn worktree_diff_fingerprint(&self) -> Option<&str> { self.target.worktree_diff_fingerprint.as_deref() } @@ -267,11 +279,22 @@ fn is_terminal_status(status: &AutoReviewRunStatus) -> bool { } pub(crate) async fn collect_auto_review_target( + codex_home: &Path, cwd: &Path, review_target: &ReviewTarget, ) -> AutoReviewRunTarget { let git_info = collect_git_info(cwd).await; let repo_root = get_git_repo_root(cwd); + let store_scope = repo_root.as_deref().unwrap_or(cwd); + let snapshot_epoch = + match ReviewCoordination::for_scope(codex_home, store_scope).current_snapshot_epoch() { + Ok(0) => None, + Ok(epoch) => Some(epoch), + Err(err) => { + tracing::warn!(error = %err, "failed to collect auto review snapshot epoch"); + None + } + }; let base_sha = match (repo_root.as_deref(), review_target) { (Some(repo_root), ReviewTarget::BaseBranch { branch }) => { match merge_base_with_head(repo_root, branch) { @@ -301,6 +324,13 @@ pub(crate) async fn collect_auto_review_target( .and_then(|git| git.commit_hash.as_ref().map(|sha| sha.0.clone())), base_sha, worktree_path: repo_root.or_else(|| Some(PathBuf::from(cwd))), + snapshot_epoch, + snapshot_commit: git_info + .as_ref() + .and_then(|git| git.commit_hash.as_ref().map(|sha| sha.0.clone())), + head_at_launch: git_info + .as_ref() + .and_then(|git| git.commit_hash.as_ref().map(|sha| sha.0.clone())), worktree_diff_fingerprint, } } @@ -336,6 +366,7 @@ mod tests { "late-running".to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.path(), cwd.path(), Some("test-model".to_string()), ) @@ -359,6 +390,7 @@ mod tests { "custom-cancelled".to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.path(), cwd.path(), Some("test-model".to_string()), ) @@ -388,6 +420,7 @@ mod tests { "pending-running".to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.path(), cwd.path(), Some("test-model".to_string()), ) @@ -412,6 +445,7 @@ mod tests { "skipped-running".to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.path(), cwd.path(), Some("test-model".to_string()), ) @@ -436,6 +470,7 @@ mod tests { "duplicate-candidate".to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.path(), cwd.path(), Some("test-model".to_string()), ) @@ -466,4 +501,31 @@ mod tests { Some("equivalent background auto review already exists: existing-run") ); } + + #[tokio::test] + async fn collect_target_records_current_snapshot_epoch() { + let codex_home = TempDir::new().expect("create temp codex home"); + let cwd = TempDir::new().expect("create temp cwd"); + + let target = collect_auto_review_target( + codex_home.path(), + cwd.path(), + &ReviewTarget::UncommittedChanges, + ) + .await; + assert_eq!(target.snapshot_epoch, None); + + ReviewCoordination::for_scope(codex_home.path(), cwd.path()) + .bump_snapshot_epoch() + .expect("bump snapshot epoch"); + + let target = collect_auto_review_target( + codex_home.path(), + cwd.path(), + &ReviewTarget::UncommittedChanges, + ) + .await; + + assert_eq!(target.snapshot_epoch, Some(1)); + } } diff --git a/codex-rs/core/src/session/background_auto_review.rs b/codex-rs/core/src/session/background_auto_review.rs index 4a9ab4dc5fef..8dd51e3c51eb 100644 --- a/codex-rs/core/src/session/background_auto_review.rs +++ b/codex-rs/core/src/session/background_auto_review.rs @@ -111,10 +111,12 @@ impl Session { .review_model .clone() .unwrap_or_else(|| turn_context.model_info.slug.clone()); + let codex_home = self.codex_home().await; let persistence = ReviewPersistenceContext::new( uuid::Uuid::new_v4().to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.as_ref(), cwd.as_ref(), Some(model), ) @@ -296,6 +298,80 @@ impl Session { } } + if self.input_queue.has_trigger_turn_mailbox_items().await + || self.active_turn.lock().await.is_some() + { + debug!( + "background auto review skipped after prepare: foreground work pending or active" + ); + let error_summary = + "foreground work became active before background auto review could start" + .to_string(); + self.record_skipped_background_auto_review( + &persistence, + generation, + &fingerprint, + error_summary, + ) + .await; + return; + } + let coordination = + ReviewCoordination::for_scope(self.codex_home().await, persistence.store_scope()); + let review_lock_guard = match acquire_background_auto_review_lock( + &coordination, + format!("background_auto_review:{}", persistence.run_id()), + ) + .await + { + Ok(Some(guard)) => guard, + Ok(None) => { + let error_summary = + "another background auto review is already running for this worktree" + .to_string(); + self.record_skipped_background_auto_review( + &persistence, + generation, + &fingerprint, + error_summary, + ) + .await; + debug!("background auto review skipped: review lock is held"); + return; + } + Err(err) => { + let error_summary = format!("failed to acquire background auto review lock: {err}"); + self.record_skipped_background_auto_review( + &persistence, + generation, + &fingerprint, + error_summary, + ) + .await; + warn!(error = %err, "failed to acquire background auto review lock"); + return; + } + }; + let persistence = match coordination.bump_snapshot_epoch() { + Ok(_snapshot_epoch) => { + persistence + .refresh_target(turn_context.config.codex_home.as_ref(), cwd.as_ref()) + .await + } + Err(err) => { + let error_summary = + format!("failed to bump background auto review snapshot epoch: {err}"); + self.record_skipped_background_auto_review( + &persistence, + generation, + &fingerprint, + error_summary, + ) + .await; + warn!(error = %err, "failed to bump background auto review snapshot epoch"); + return; + } + }; let prepared = prepare_review_thread( Arc::clone(self), Arc::clone(&turn_context.config), @@ -359,44 +435,6 @@ impl Session { ) .await; } - let coordination = - ReviewCoordination::for_scope(self.codex_home().await, persistence.store_scope()); - let review_lock_guard = match acquire_background_auto_review_lock( - &coordination, - format!("background_auto_review:{}", persistence.run_id()), - ) - .await - { - Ok(Some(guard)) => guard, - Ok(None) => { - let error_summary = - "another background auto review is already running for this worktree" - .to_string(); - self.clear_background_auto_review(generation).await; - self.record_skipped_background_auto_review( - &persistence, - generation, - &fingerprint, - error_summary, - ) - .await; - debug!("background auto review skipped: review lock is held"); - return; - } - Err(err) => { - let error_summary = format!("failed to acquire background auto review lock: {err}"); - self.clear_background_auto_review(generation).await; - self.record_skipped_background_auto_review( - &persistence, - generation, - &fingerprint, - error_summary, - ) - .await; - warn!(error = %err, "failed to acquire background auto review lock"); - return; - } - }; if started_review .running_review .cancellation_token @@ -549,10 +587,9 @@ impl Session { let state = self.state.lock().await; state.background_auto_review.active_snapshot() }; - let duplicate = match store.find_duplicate_by_fingerprint_with_target_and_filter( + let duplicate = match store.find_duplicate_by_fingerprint_with_target_proof_and_filter( fingerprint, - persistence.target().branch.as_deref(), - persistence.target().head_sha.as_deref(), + Some(persistence.target()), |duplicate| match duplicate.disposition { AutoReviewDuplicateDisposition::ReuseTerminal => true, AutoReviewDuplicateDisposition::Adopt => { diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index e87f0ad33229..157aa61654a6 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -258,6 +258,7 @@ pub(super) async fn prepare_review_thread( review_turn_id.clone(), mode, resolved.target.clone(), + tc.config.codex_home.as_ref(), target_cwd, Some(model), ) diff --git a/codex-rs/core/src/state/session_tests.rs b/codex-rs/core/src/state/session_tests.rs index 8d12d3a8dff0..23e6aaa4c338 100644 --- a/codex-rs/core/src/state/session_tests.rs +++ b/codex-rs/core/src/state/session_tests.rs @@ -279,6 +279,7 @@ fn background_auto_review_skips_duplicate_fingerprint() { fn test_background_review_persistence( run_id: &str, ) -> (ReviewPersistenceContext, tempfile::TempDir) { + let codex_home = tempfile::TempDir::new().expect("create temp codex home"); let cwd = tempfile::TempDir::new().expect("create temp cwd"); let runtime = tokio::runtime::Builder::new_current_thread() .enable_all() @@ -288,6 +289,7 @@ fn test_background_review_persistence( run_id.to_string(), ReviewPersistence::BackgroundAutoReview, ReviewTarget::UncommittedChanges, + codex_home.path(), cwd.path(), Some("test-model".to_string()), )); diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index a5225b7ed50f..f94606071742 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -828,6 +828,9 @@ async fn session_startup_marks_orphaned_background_review_lost() -> anyhow::Resu head_sha: current_git_head(cwd.path()), base_sha: None, worktree_path: Some(cwd.path().to_path_buf()), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: Some("orphaned-fingerprint".to_string()), }, review_target: ReviewTarget::UncommittedChanges, @@ -890,6 +893,9 @@ async fn auto_review_awareness_injected_for_current_findings() -> anyhow::Result head_sha: current_git_head(harness.cwd()), base_sha: None, worktree_path: Some(harness.cwd().to_path_buf()), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: Some(fingerprint), }, review_target: ReviewTarget::UncommittedChanges, @@ -1517,6 +1523,9 @@ async fn automatic_background_review_reuses_completed_durable_duplicate() -> any head_sha: current_git_head(test.cwd_path()), base_sha: None, worktree_path: Some(test.cwd_path().to_path_buf()), + snapshot_epoch: None, + snapshot_commit: None, + head_at_launch: None, worktree_diff_fingerprint: Some(fingerprint.clone()), }, review_target: ReviewTarget::UncommittedChanges,