896e4754

Add per-ref push tracking and resumable sync on partial failure

a73x   2026-03-21 13:41

Replace batch git push with per-ref pushes that track success/failure
individually. On partial failure, persist pending refs with error messages
to .git/collab/sync-state.json. Next sync detects the state file and
resumes by retrying only the failed refs.

- Per-ref Pushed/FAILED output lines
- SyncState persistence with (ref_name, last_error) tuples
- Resume mode: skip fetch/reconcile, retry pending, clean stale sync refs
- PartialSync error variant with exit code 1
- 12 new tests including hook-based failure simulation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---

diff --git a/src/error.rs b/src/error.rs
index ce300ea..8755825 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -28,4 +28,7 @@ pub enum Error {

    #[error("another sync is in progress (pid {pid}, started {since})")]
    SyncLocked { pid: u32, since: String },

    #[error("sync partially failed: {succeeded} of {total} refs pushed")]
    PartialSync { succeeded: usize, total: usize },
}
diff --git a/src/main.rs b/src/main.rs
index 89cecfb..464e96a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -15,7 +15,15 @@ fn main() {
    };

    if let Err(e) = git_collab::run(cli, &repo) {
        eprintln!("error: {}", e);
        std::process::exit(1);
        match &e {
            git_collab::error::Error::PartialSync { .. } => {
                // Summary already printed by sync; just exit with code 1
                std::process::exit(1);
            }
            _ => {
                eprintln!("error: {}", e);
                std::process::exit(1);
            }
        }
    }
}
diff --git a/src/sync.rs b/src/sync.rs
index f6d9b10..ec0d890 100644
--- a/src/sync.rs
+++ b/src/sync.rs
@@ -1,6 +1,9 @@
use std::fs;
use std::path::{Path, PathBuf};
use std::process::Command;

use git2::Repository;
use serde::{Deserialize, Serialize};

use crate::dag;
use crate::error::Error;
@@ -9,6 +12,225 @@ use crate::signing;
use crate::sync_lock::SyncLock;
use crate::trust;

// ---------------------------------------------------------------------------
// Per-ref push types (T002a)
// ---------------------------------------------------------------------------

/// Status of pushing a single ref.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub enum PushStatus {
    Pushed,
    Failed,
}

/// Result of pushing a single ref to the remote.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct RefPushResult {
    pub ref_name: String,
    pub status: PushStatus,
    pub error: Option<String>,
}

/// Aggregated outcome of the entire push phase.
#[derive(Debug, Clone)]
pub struct SyncResult {
    pub results: Vec<RefPushResult>,
    pub remote: String,
}

impl SyncResult {
    pub fn succeeded(&self) -> Vec<&RefPushResult> {
        self.results
            .iter()
            .filter(|r| r.status == PushStatus::Pushed)
            .collect()
    }

    pub fn failed(&self) -> Vec<&RefPushResult> {
        self.results
            .iter()
            .filter(|r| r.status == PushStatus::Failed)
            .collect()
    }

    pub fn is_complete(&self) -> bool {
        self.results.iter().all(|r| r.status == PushStatus::Pushed)
    }
}

// ---------------------------------------------------------------------------
// Persistent sync state (T002b)
// ---------------------------------------------------------------------------

/// Persistent record of a partially-completed sync.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct SyncState {
    pub remote: String,
    pub pending_refs: Vec<(String, String)>,
    pub timestamp: String,
}

impl SyncState {
    /// Path to the sync state file within a repo.
    fn path(repo: &Repository) -> PathBuf {
        let git_dir = repo.path();
        git_dir.join("collab").join("sync-state.json")
    }

    /// Load sync state from disk. Returns None if the file does not exist.
    /// If the file is corrupted, prints a warning, deletes it, and returns None.
    pub fn load(repo: &Repository) -> Option<SyncState> {
        let path = Self::path(repo);
        if !path.exists() {
            return None;
        }
        match fs::read_to_string(&path) {
            Ok(contents) => match serde_json::from_str(&contents) {
                Ok(state) => Some(state),
                Err(e) => {
                    eprintln!(
                        "warning: corrupted sync state file ({}); ignoring and proceeding with full sync",
                        e
                    );
                    let _ = fs::remove_file(&path);
                    None
                }
            },
            Err(_) => None,
        }
    }

    /// Save sync state to disk.
    pub fn save(&self, repo: &Repository) -> Result<(), Error> {
        let path = Self::path(repo);
        if let Some(parent) = path.parent() {
            fs::create_dir_all(parent)?;
        }
        let json = serde_json::to_string_pretty(self)?;
        fs::write(&path, json)?;
        Ok(())
    }

    /// Delete the sync state file.
    pub fn clear(repo: &Repository) -> Result<(), Error> {
        let path = Self::path(repo);
        if path.exists() {
            fs::remove_file(&path)?;
        }
        Ok(())
    }
}

// ---------------------------------------------------------------------------
// Push helpers (T003, T004)
// ---------------------------------------------------------------------------

/// Push a single ref to the remote. Returns a RefPushResult.
fn push_ref(workdir: &Path, remote_name: &str, ref_name: &str) -> RefPushResult {
    let refspec = format!("+{}:{}", ref_name, ref_name);
    match Command::new("git")
        .args(["push", remote_name, &refspec])
        .current_dir(workdir)
        .output()
    {
        Ok(output) => {
            if output.status.success() {
                RefPushResult {
                    ref_name: ref_name.to_string(),
                    status: PushStatus::Pushed,
                    error: None,
                }
            } else {
                let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
                RefPushResult {
                    ref_name: ref_name.to_string(),
                    status: PushStatus::Failed,
                    error: Some(stderr),
                }
            }
        }
        Err(e) => RefPushResult {
            ref_name: ref_name.to_string(),
            status: PushStatus::Failed,
            error: Some(format!("failed to run git push: {}", e)),
        },
    }
}

/// Collect all collab ref names that should be pushed.
fn collect_push_refs(repo: &Repository) -> Result<Vec<String>, Error> {
    let mut refs = Vec::new();
    for pattern in &["refs/collab/issues/*", "refs/collab/patches/*"] {
        let iter = repo.references_glob(pattern)?;
        for r in iter {
            let r = r?;
            if let Some(name) = r.name() {
                refs.push(name.to_string());
            }
        }
    }
    Ok(refs)
}

// ---------------------------------------------------------------------------
// Summary printing (T012)
// ---------------------------------------------------------------------------

/// Print a failure summary to stderr.
fn print_sync_summary(result: &SyncResult) {
    let succeeded = result.succeeded().len();
    let total = result.results.len();

    if succeeded == 0 {
        eprintln!("\nSync failed: {} of {} refs pushed.", succeeded, total);
    } else {
        eprintln!(
            "\nSync partially failed: {} of {} refs pushed.",
            succeeded, total
        );
    }

    let failed = result.failed();
    eprintln!("Failed refs:");
    for f in &failed {
        eprintln!(
            "  {}: {}",
            f.ref_name,
            f.error.as_deref().unwrap_or("unknown error")
        );
    }

    eprintln!(
        "\nRun `collab sync {}` again to retry {} failed ref(s).",
        result.remote,
        failed.len()
    );
}

/// Clean up refs/collab/sync/* refs.
fn cleanup_sync_refs(repo: &Repository) -> Result<(), Error> {
    for prefix in &["refs/collab/sync/issues/", "refs/collab/sync/patches/"] {
        let refs: Vec<String> = repo
            .references_glob(&format!("{}*", prefix))?
            .filter_map(|r| r.ok()?.name().map(|n| n.to_string()))
            .collect();
        for ref_name in refs {
            let mut r = repo.find_reference(&ref_name)?;
            r.delete()?;
        }
    }
    // Also clean up any refs under refs/collab/sync/ with remote-name prefix
    let refs: Vec<String> = repo
        .references_glob("refs/collab/sync/*")?
        .filter_map(|r| r.ok()?.name().map(|n| n.to_string()))
        .collect();
    for ref_name in refs {
        let mut r = repo.find_reference(&ref_name)?;
        r.delete()?;
    }
    Ok(())
}

/// Add collab refspecs to all remotes.
pub fn init(repo: &Repository) -> Result<(), Error> {
    let remotes = repo.remotes()?;
@@ -30,9 +252,18 @@ pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
    // Acquire advisory lock — held until _lock is dropped (RAII)
    let _lock = SyncLock::acquire(repo)?;

    let author = get_author(repo)?;
    let workdir = repo.path().parent().unwrap_or(repo.path()).to_path_buf();

    // Check for existing sync state (resume mode)
    if let Some(state) = SyncState::load(repo) {
        if state.remote == remote_name {
            return sync_resume(repo, remote_name, &workdir, &state);
        }
        // State is for a different remote — ignore it, run full sync
    }

    let author = get_author(repo)?;

    // Step 1: Fetch collab refs using system git (handles SSH agent, credentials, etc.)
    println!("Fetching from '{}'...", remote_name);
    let fetch_status = Command::new("git")
@@ -60,57 +291,146 @@ pub fn sync(repo: &Repository, remote_name: &str) -> Result<(), Error> {
    reconcile_refs(&repo, "issues", &author, &sk)?;
    reconcile_refs(&repo, "patches", &author, &sk)?;

    // Step 3: Push collab refs using system git
    // Step 3: Push collab refs individually
    println!("Pushing to '{}'...", remote_name);
    let mut push_args = vec!["push", remote_name];
    let mut refspecs: Vec<String> = Vec::new();
    let refs_to_push = collect_push_refs(&repo)?;

    for pattern in &["refs/collab/issues/*", "refs/collab/patches/*"] {
        let refs = repo.references_glob(pattern)?;
        for r in refs {
            let r = r?;
            if let Some(name) = r.name() {
                refspecs.push(format!("+{}:{}", name, name));
            }
        }
    }

    if refspecs.is_empty() {
    if refs_to_push.is_empty() {
        println!("Nothing to push.");
    } else {
        let refspec_strs: Vec<&str> = refspecs.iter().map(|s| s.as_str()).collect();
        push_args.extend(refspec_strs);

        let push_status = Command::new("git")
            .args(&push_args)
            .current_dir(&workdir)
            .status()
            .map_err(|e| Error::Cmd(format!("failed to run git push: {}", e)))?;

        if !push_status.success() {
            return Err(Error::Cmd(format!(
                "git push exited with status {}",
                push_status
            )));
        let sync_result = push_refs_individually(&workdir, remote_name, &refs_to_push);

        if !sync_result.is_complete() {
            // Save state for resume
            let now = chrono::Utc::now().to_rfc3339();
            let pending: Vec<(String, String)> = sync_result
                .failed()
                .iter()
                .map(|r| {
                    (
                        r.ref_name.clone(),
                        r.error.clone().unwrap_or_else(|| "unknown error".to_string()),
                    )
                })
                .collect();
            let state = SyncState {
                remote: remote_name.to_string(),
                pending_refs: pending,
                timestamp: now,
            };
            state.save(&repo)?;

            print_sync_summary(&sync_result);

            // Clean up sync refs before returning error
            cleanup_sync_refs(&repo)?;

            let succeeded = sync_result.succeeded().len();
            let total = sync_result.results.len();
            return Err(Error::PartialSync {
                succeeded,
                total,
            });
        }
    }

    // Step 4: Clean up sync refs
    for prefix in &["refs/collab/sync/issues/", "refs/collab/sync/patches/"] {
        let refs: Vec<String> = repo
            .references_glob(&format!("{}*", prefix))?
            .filter_map(|r| r.ok()?.name().map(|n| n.to_string()))
            .collect();
        for ref_name in refs {
            let mut r = repo.find_reference(&ref_name)?;
            r.delete()?;
        }
    }
    cleanup_sync_refs(&repo)?;

    println!("Sync complete.");
    Ok(())
}

/// Resume a partially-failed sync by retrying only the pending refs.
fn sync_resume(
    repo: &Repository,
    remote_name: &str,
    workdir: &Path,
    state: &SyncState,
) -> Result<(), Error> {
    println!(
        "Resuming sync to '{}' ({} refs pending from previous failure)...",
        remote_name,
        state.pending_refs.len()
    );

    // Print pending refs with their last error
    for (ref_name, last_error) in &state.pending_refs {
        println!("  Pending: {} (last error: {})", ref_name, last_error);
    }

    // Clean up stale sync refs (T018b)
    cleanup_sync_refs(repo)?;

    // Push the pending refs
    let ref_names: Vec<String> = state.pending_refs.iter().map(|(r, _)| r.clone()).collect();
    let sync_result = push_refs_individually(workdir, remote_name, &ref_names);

    if sync_result.is_complete() {
        // All pending refs pushed successfully
        SyncState::clear(repo)?;
        if sync_result.results.is_empty() {
            println!("All pending refs are already up to date. Clearing stale sync state.");
        } else {
            println!("\nSync complete. All previously-failed refs pushed.");
        }
        println!("Sync complete.");
        Ok(())
    } else {
        // Some refs still failing - update state
        let now = chrono::Utc::now().to_rfc3339();
        let pending: Vec<(String, String)> = sync_result
            .failed()
            .iter()
            .map(|r| {
                (
                    r.ref_name.clone(),
                    r.error.clone().unwrap_or_else(|| "unknown error".to_string()),
                )
            })
            .collect();
        let new_state = SyncState {
            remote: remote_name.to_string(),
            pending_refs: pending,
            timestamp: now,
        };
        new_state.save(repo)?;

        print_sync_summary(&sync_result);
        eprintln!("To force a full sync, delete .git/collab/sync-state.json");

        let succeeded = sync_result.succeeded().len();
        let total = sync_result.results.len();
        Err(Error::PartialSync {
            succeeded,
            total,
        })
    }
}

/// Push refs one at a time, printing per-ref status, and return aggregated results.
fn push_refs_individually(workdir: &Path, remote_name: &str, refs: &[String]) -> SyncResult {
    let mut results = Vec::new();
    for ref_name in refs {
        let result = push_ref(workdir, remote_name, ref_name);
        match &result.status {
            PushStatus::Pushed => println!("  Pushed {}", ref_name),
            PushStatus::Failed => {
                eprintln!(
                    "  FAILED {}: {}",
                    ref_name,
                    result.error.as_deref().unwrap_or("unknown error")
                );
            }
        }
        results.push(result);
    }
    SyncResult {
        results,
        remote: remote_name.to_string(),
    }
}

/// Reconcile all refs of a given kind (issues or patches) from sync refs.
fn reconcile_refs(
    repo: &Repository,
diff --git a/tests/sync_test.rs b/tests/sync_test.rs
index 48a7445..cbcd567 100644
--- a/tests/sync_test.rs
+++ b/tests/sync_test.rs
@@ -11,6 +11,7 @@ use tempfile::TempDir;

use git2::Repository;
use git_collab::dag;
use git_collab::error;
use git_collab::event::{Action, Event, ReviewVerdict};
use git_collab::signing;
use git_collab::state::{self, IssueState, IssueStatus, PatchState};
@@ -26,7 +27,7 @@ use common::{
// ---------------------------------------------------------------------------

struct TestCluster {
    _bare_dir: TempDir,
    bare_dir: TempDir,
    alice_dir: TempDir,
    bob_dir: TempDir,
    _key_setup: (), // signing key created in default config dir
@@ -80,7 +81,7 @@ impl TestCluster {
        }

        TestCluster {
            _bare_dir: bare_dir,
            bare_dir,
            alice_dir,
            bob_dir,
            _key_setup: (),
@@ -94,6 +95,11 @@ impl TestCluster {
    fn bob_repo(&self) -> Repository {
        Repository::open(self.bob_dir.path()).unwrap()
    }

    /// Return the path to the bare remote directory.
    fn bare_dir(&self) -> &std::path::Path {
        self.bare_dir.path()
    }
}

// ---------------------------------------------------------------------------
@@ -654,3 +660,412 @@ fn test_reconciliation_merge_commit_is_signed() {
        "Merge commit signature must be valid"
    );
}

// ---------------------------------------------------------------------------
// Sync Recovery Tests
// ---------------------------------------------------------------------------

/// Install a pre-receive hook in the bare repo that rejects refs matching a pattern.
fn install_reject_hook(bare_dir: &std::path::Path, reject_pattern: &str) {
    let hooks_dir = bare_dir.join("hooks");
    std::fs::create_dir_all(&hooks_dir).unwrap();
    let hook_path = hooks_dir.join("pre-receive");
    let script = format!(
        r#"#!/bin/sh
while read oldrev newrev refname; do
    case "$refname" in
        *{}*)
            echo "REJECT: $refname matches reject pattern" >&2
            exit 1
            ;;
    esac
done
exit 0
"#,
        reject_pattern
    );
    std::fs::write(&hook_path, script).unwrap();
    #[cfg(unix)]
    {
        use std::os::unix::fs::PermissionsExt;
        std::fs::set_permissions(&hook_path, std::fs::Permissions::from_mode(0o755)).unwrap();
    }
}

/// Remove the pre-receive hook from the bare repo.
fn remove_reject_hook(bare_dir: &std::path::Path) {
    let hook_path = bare_dir.join("hooks").join("pre-receive");
    let _ = std::fs::remove_file(&hook_path);
}

// T005 [US4]: Per-ref push isolates failures
#[test]
fn test_per_ref_push_isolates_failures() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    // Create two issues
    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Issue one");
    let (_ref2, id2) = open_issue(&alice_repo, &alice(), "Issue two");

    // Install hook that rejects one specific issue
    install_reject_hook(cluster.bare_dir(), &id1[..8]);

    // Sync should fail with PartialSync
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_err(), "sync should fail when a ref is rejected");
    let err = result.unwrap_err();
    match &err {
        git_collab::error::Error::PartialSync { succeeded, total } => {
            assert_eq!(*succeeded, 1, "one ref should succeed");
            assert_eq!(*total, 2, "two refs total");
        }
        other => panic!("expected PartialSync error, got: {:?}", other),
    }

    // Verify the non-rejected ref was pushed to the bare remote
    let bare_repo = Repository::open_bare(cluster.bare_dir()).unwrap();
    let pushed_ref = format!("refs/collab/issues/{}", id2);
    assert!(
        bare_repo.refname_to_id(&pushed_ref).is_ok(),
        "non-rejected ref should be on remote"
    );

    // The rejected ref should NOT be on the remote
    let rejected_ref = format!("refs/collab/issues/{}", id1);
    assert!(
        bare_repo.refname_to_id(&rejected_ref).is_err(),
        "rejected ref should NOT be on remote"
    );
}

// T006 [US4]: All refs push successfully — happy path unchanged
#[test]
fn test_all_refs_push_succeeds_unchanged_behavior() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, _id1) = open_issue(&alice_repo, &alice(), "Happy issue");

    // Sync should succeed
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_ok(), "sync should succeed: {:?}", result.err());
}

// T010 [US1]: Partial failure reports failed refs
#[test]
fn test_partial_failure_reports_failed_refs() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Will fail");
    let (_ref2, _id2) = open_issue(&alice_repo, &alice(), "Will succeed");

    install_reject_hook(cluster.bare_dir(), &id1[..8]);

    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_err());

    // Verify sync state was saved with the failed ref
    let state = sync::SyncState::load(&alice_repo);
    assert!(state.is_some(), "sync state should be saved");
    let state = state.unwrap();
    assert_eq!(state.remote, "origin");
    assert_eq!(state.pending_refs.len(), 1, "one ref should be pending");
    assert!(
        state.pending_refs[0].0.contains(&id1),
        "pending ref should be the rejected one"
    );
    assert!(
        !state.pending_refs[0].1.is_empty(),
        "error message should be recorded"
    );
}

// T011 [US1]: Total failure reports all failed
#[test]
fn test_total_failure_reports_all_failed() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, _id1) = open_issue(&alice_repo, &alice(), "Fail one");
    let (_ref2, _id2) = open_issue(&alice_repo, &alice(), "Fail two");

    // Reject ALL collab refs
    install_reject_hook(cluster.bare_dir(), "refs/collab/");

    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_err());
    match result.unwrap_err() {
        git_collab::error::Error::PartialSync { succeeded, total } => {
            assert_eq!(succeeded, 0, "no refs should succeed");
            assert_eq!(total, 2, "two refs total");
        }
        other => panic!("expected PartialSync, got: {:?}", other),
    }

    let state = sync::SyncState::load(&alice_repo).unwrap();
    assert_eq!(state.pending_refs.len(), 2, "all refs should be pending");
}

// T014 [US2]: Resume retries only failed refs
#[test]
fn test_resume_retries_only_failed_refs() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Will fail first time");
    let (_ref2, _id2) = open_issue(&alice_repo, &alice(), "Will succeed first time");

    // First sync: reject id1
    install_reject_hook(cluster.bare_dir(), &id1[..8]);
    let _ = sync::sync(&alice_repo, "origin");

    // Verify state was saved
    let state = sync::SyncState::load(&alice_repo);
    assert!(state.is_some());
    assert_eq!(state.unwrap().pending_refs.len(), 1);

    // Remove the hook so all refs can push
    remove_reject_hook(cluster.bare_dir());

    // Resume sync should succeed
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_ok(), "resume sync should succeed: {:?}", result.err());

    // State should be cleared
    assert!(
        sync::SyncState::load(&alice_repo).is_none(),
        "sync state should be cleared after successful resume"
    );
}

// T015 [US2]: Resume clears state on full success
#[test]
fn test_resume_clears_state_on_full_success() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Issue for resume");

    // Manually create sync state
    let state = sync::SyncState {
        remote: "origin".to_string(),
        pending_refs: vec![(
            format!("refs/collab/issues/{}", id1),
            "previous error".to_string(),
        )],
        timestamp: chrono::Utc::now().to_rfc3339(),
    };
    state.save(&alice_repo).unwrap();

    // Sync in resume mode
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_ok(), "resume should succeed: {:?}", result.err());

    // Verify state file is gone
    let state_path = alice_repo.path().join("collab").join("sync-state.json");
    assert!(
        !state_path.exists(),
        "sync-state.json should be deleted after successful resume"
    );
}

// T016 [US2]: Resume updates state on continued failure
#[test]
fn test_resume_updates_state_on_continued_failure() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Will keep failing");
    let (_ref2, id2) = open_issue(&alice_repo, &alice(), "Will succeed on retry");

    // Manually create sync state with both refs pending
    let state = sync::SyncState {
        remote: "origin".to_string(),
        pending_refs: vec![
            (
                format!("refs/collab/issues/{}", id1),
                "previous error".to_string(),
            ),
            (
                format!("refs/collab/issues/{}", id2),
                "previous error".to_string(),
            ),
        ],
        timestamp: chrono::Utc::now().to_rfc3339(),
    };
    state.save(&alice_repo).unwrap();

    // Install hook that only rejects id1
    install_reject_hook(cluster.bare_dir(), &id1[..8]);

    // Resume should partially fail
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_err());

    // State should be updated with only the still-failing ref
    let new_state = sync::SyncState::load(&alice_repo).unwrap();
    assert_eq!(
        new_state.pending_refs.len(),
        1,
        "only the still-failing ref should remain"
    );
    assert!(
        new_state.pending_refs[0].0.contains(&id1),
        "the still-failing ref should be id1"
    );
}

// T017 [US2]: No resume without state file
#[test]
fn test_no_resume_without_state_file() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, _id1) = open_issue(&alice_repo, &alice(), "Normal sync");

    // No sync state file — should run full flow
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_ok(), "normal sync should succeed: {:?}", result.err());
}

// T018b [US2]: Resume cleans up stale sync refs
#[test]
fn test_resume_cleans_up_stale_sync_refs() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Issue for stale ref test");

    // Push the issue first (normal sync)
    sync::sync(&alice_repo, "origin").unwrap();

    // Now manually create a stale sync ref
    let tip = alice_repo
        .refname_to_id(&format!("refs/collab/issues/{}", id1))
        .unwrap();
    alice_repo
        .reference(
            "refs/collab/sync/origin/issues/stale_ref",
            tip,
            true,
            "create stale sync ref",
        )
        .unwrap();

    // Create sync state to trigger resume mode
    let state = sync::SyncState {
        remote: "origin".to_string(),
        pending_refs: vec![(
            format!("refs/collab/issues/{}", id1),
            "previous error".to_string(),
        )],
        timestamp: chrono::Utc::now().to_rfc3339(),
    };
    state.save(&alice_repo).unwrap();

    // Resume sync
    let result = sync::sync(&alice_repo, "origin");
    assert!(result.is_ok(), "resume should succeed: {:?}", result.err());

    // Verify stale sync ref was cleaned up
    let stale_ref = alice_repo.refname_to_id("refs/collab/sync/origin/issues/stale_ref");
    assert!(
        stale_ref.is_err(),
        "stale sync ref should be cleaned up during resume"
    );
}

// T021 [US3]: Stale state detected and cleared when refs are already up to date
#[test]
fn test_stale_state_detected_and_cleared() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Already pushed issue");

    // Push the issue normally first
    sync::sync(&alice_repo, "origin").unwrap();

    // Now create sync state as if the push had failed — but the refs are actually pushed
    let state = sync::SyncState {
        remote: "origin".to_string(),
        pending_refs: vec![(
            format!("refs/collab/issues/{}", id1),
            "connection timed out".to_string(),
        )],
        timestamp: chrono::Utc::now().to_rfc3339(),
    };
    state.save(&alice_repo).unwrap();

    // Resume sync — refs are already up to date
    let result = sync::sync(&alice_repo, "origin");
    assert!(
        result.is_ok(),
        "sync should succeed when refs already up to date: {:?}",
        result.err()
    );

    // State should be cleared
    assert!(
        sync::SyncState::load(&alice_repo).is_none(),
        "stale sync state should be cleared"
    );
}

// T022 [US3]: State for different remote is ignored
#[test]
fn test_state_for_different_remote_ignored() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, id1) = open_issue(&alice_repo, &alice(), "Issue for remote test");

    // Create sync state for a different remote
    let state = sync::SyncState {
        remote: "upstream".to_string(),
        pending_refs: vec![(
            format!("refs/collab/issues/{}", id1),
            "some error".to_string(),
        )],
        timestamp: chrono::Utc::now().to_rfc3339(),
    };
    state.save(&alice_repo).unwrap();

    // Sync against origin — should run full flow, ignoring the upstream state
    let result = sync::sync(&alice_repo, "origin");
    assert!(
        result.is_ok(),
        "sync to origin should succeed ignoring upstream state: {:?}",
        result.err()
    );
}

// T026: Corrupted state file handled gracefully
#[test]
fn test_corrupted_state_file_handled_gracefully() {
    let cluster = TestCluster::new();
    let alice_repo = cluster.alice_repo();

    let (_ref1, _id1) = open_issue(&alice_repo, &alice(), "Issue for corruption test");

    // Write invalid JSON to sync-state.json
    let collab_dir = alice_repo.path().join("collab");
    std::fs::create_dir_all(&collab_dir).unwrap();
    std::fs::write(collab_dir.join("sync-state.json"), "not valid json{{{").unwrap();

    // Sync should proceed normally (treating as no state)
    let result = sync::sync(&alice_repo, "origin");
    assert!(
        result.is_ok(),
        "sync should succeed despite corrupted state: {:?}",
        result.err()
    );

    // State file should be cleaned up
    assert!(
        !collab_dir.join("sync-state.json").exists(),
        "corrupted state file should be deleted"
    );
}