3185bac6
Fix cache defeating auto-detect merge for externally merged patches
a73x 2026-03-22 10:10
When PatchState::from_ref() got a cache hit, it returned the cached state directly, skipping the auto-detect merge logic that only ran in from_ref_uncached(). This meant patches merged via git (not git-collab) would keep showing status: open until the DAG changed and invalidated the cache. Now from_ref() re-runs the auto-detect merge check on cache hit when the cached status is Open, and updates the cache if merged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/src/state.rs b/src/state.rs index 9b0132e..14580cb 100644 --- a/src/state.rs +++ b/src/state.rs @@ -374,7 +374,29 @@ impl PatchState { id: &str, ) -> Result<Self, crate::error::Error> { // Check cache first if let Some(cached) = cache::get_cached_state::<PatchState>(repo, ref_name) { if let Some(mut cached) = cache::get_cached_state::<PatchState>(repo, ref_name) { // The cache may return a stale Open status if the patch was merged // outside of git-collab (e.g. git merge) since the DAG tip hasn't // changed. Re-run the auto-detect merge check for Open patches. if cached.status == PatchStatus::Open { if let Ok(patch_head) = cached.resolve_head(repo) { let base_ref_name = format!("refs/heads/{}", cached.base_ref); if let Ok(base_tip) = repo.refname_to_id(&base_ref_name) { let base_moved = cached.base_commit.as_ref() .map(|bc| bc != &base_tip.to_string()) .unwrap_or(false); let reachable = base_tip == patch_head || repo.graph_descendant_of(base_tip, patch_head).unwrap_or(false); if base_moved && reachable { cached.status = PatchStatus::Merged; // Update the cache with the corrected status if let Ok(tip) = repo.refname_to_id(ref_name) { cache::set_cached_state(repo, ref_name, tip, &cached); } } } } } return Ok(cached); } diff --git a/tests/collab_test.rs b/tests/collab_test.rs index 0f30678..81401da 100644 --- a/tests/collab_test.rs +++ b/tests/collab_test.rs @@ -987,6 +987,36 @@ fn test_auto_detect_merged_patch_deleted_branch() { assert_eq!(state.status, PatchStatus::Open); } #[test] fn test_cache_does_not_defeat_auto_detect_merge() { // Regression: from_ref() returned cached Open status even after the // patch branch was merged into main, because the cache hit bypassed // the auto-detect merge logic that only ran in from_ref_uncached(). let tmp = TempDir::new().unwrap(); let repo = init_repo(tmp.path(), &alice()); make_initial_commit(&repo, "main"); let tip = add_commit_on_branch(&repo, "main", "base.rs", b"base"); repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code"); // Create the patch via the high-level API (records base_commit) let id = patch::create(&repo, "Cache merge test", "", "main", "feat", None).unwrap(); let ref_name = format!("refs/collab/patches/{}", id); // First call: populates cache, status should be Open let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap(); assert_eq!(state.status, PatchStatus::Open); // Manually fast-forward main to feat (simulating `git merge feat`) let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap(); repo.reference("refs/heads/main", feat_tip, true, "manual merge").unwrap(); // Second call: cache hit (DAG tip unchanged), but should still detect merge let state2 = PatchState::from_ref(&repo, &ref_name, &id).unwrap(); assert_eq!(state2.status, PatchStatus::Merged, "cached from_ref should detect merge"); } // --------------------------------------------------------------------------- // Phase 6: US4 — Implicit Revision (T027) // ---------------------------------------------------------------------------