a73x

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)
// ---------------------------------------------------------------------------