a73x

tests/collab_test.rs

Ref:   Size: 57.8 KiB

mod common;

use tempfile::TempDir;

use git_collab::dag;
use git_collab::error::Error;
use git_collab::event::{Action, Author, Event, ReviewVerdict};
use git_collab::signing::{self, DetachedSignature, VerifyStatus};
use git_collab::state::{self, IssueState, IssueStatus, PatchState, PatchStatus};

use common::{
    add_comment, add_review, alice, bob, close_issue, create_patch, init_repo, now, open_issue,
    reopen_issue, setup_signing_key, test_signing_key, ScopedTestConfig,
};

// ---------------------------------------------------------------------------
// Basic DAG tests
// ---------------------------------------------------------------------------

#[test]
fn test_create_and_walk_issue_dag() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "First issue");
    add_comment(&repo, &ref_name, &alice(), "A comment");

    let events = dag::walk_events(&repo, &ref_name).unwrap();
    assert_eq!(events.len(), 2);
    assert!(matches!(events[0].1.action, Action::IssueOpen { .. }));
    assert!(matches!(events[1].1.action, Action::IssueComment { .. }));

    // State materializes correctly
    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.title, "First issue");
    assert_eq!(state.status, IssueStatus::Open);
    assert_eq!(state.comments.len(), 1);
}

#[test]
fn test_issue_lifecycle() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Bug report");
    add_comment(&repo, &ref_name, &bob(), "I can reproduce");
    close_issue(&repo, &ref_name, &alice());

    let s = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(s.status, IssueStatus::Closed);
    assert_eq!(s.comments.len(), 1);

    // Reopen
    reopen_issue(&repo, &ref_name, &bob());
    let s = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(s.status, IssueStatus::Open);
}

#[test]
fn test_list_issues_filters_by_status() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (_ref1, _) = open_issue(&repo, &alice(), "Open issue");
    let (ref2, _) = open_issue(&repo, &alice(), "Closed issue");
    close_issue(&repo, &ref2, &alice());

    let all = state::list_issues(&repo).unwrap();
    assert_eq!(all.len(), 2);

    let open: Vec<_> = all
        .iter()
        .filter(|i| i.status == IssueStatus::Open)
        .collect();
    assert_eq!(open.len(), 1);
    assert_eq!(open[0].title, "Open issue");
}

// ---------------------------------------------------------------------------
// Issue edit via DAG
// ---------------------------------------------------------------------------

#[test]
fn test_issue_edit_updates_title_and_body() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Original title");

    let sk = test_signing_key();
    let event = Event {
        timestamp: now(),
        author: alice(),
        action: Action::IssueEdit {
            title: Some("New title".to_string()),
            body: Some("New body".to_string()),
        },
        clock: 0,
    };
    dag::append_event(&repo, &ref_name, &event, &sk).unwrap();

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.title, "New title");
    assert_eq!(state.body, "New body");
}

#[test]
fn test_issue_edit_partial_update() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Keep this title");

    // Edit only body
    let sk = test_signing_key();
    let event = Event {
        timestamp: now(),
        author: alice(),
        action: Action::IssueEdit {
            title: None,
            body: Some("Added body".to_string()),
        },
        clock: 0,
    };
    dag::append_event(&repo, &ref_name, &event, &sk).unwrap();

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.title, "Keep this title");
    assert_eq!(state.body, "Added body");
}

// ---------------------------------------------------------------------------
// Multi-user collaboration: concurrent edits → forked DAG → reconcile
// ---------------------------------------------------------------------------

#[test]
fn test_concurrent_comments_create_fork_and_reconcile() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Concurrent test");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    add_comment(&repo, &ref_name, &alice(), "Alice's comment");
    let alice_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, root_oid, true, "simulate bob fork")
        .unwrap();
    add_comment(&repo, &ref_name, &bob(), "Bob's comment");
    let bob_tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, bob_tip, true, "remote tip")
        .unwrap();

    repo.reference(&ref_name, alice_tip, true, "restore alice tip")
        .unwrap();

    let (merge_oid, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
    assert_eq!(outcome, dag::ReconcileOutcome::Merge);

    let merge_commit = repo.find_commit(merge_oid).unwrap();
    assert_eq!(merge_commit.parent_count(), 2);

    let events = dag::walk_events(&repo, &ref_name).unwrap();
    let actions: Vec<_> = events.iter().map(|(_, e)| &e.action).collect();

    assert!(actions
        .iter()
        .any(|a| matches!(a, Action::IssueOpen { .. })));
    let comments: Vec<_> = actions
        .iter()
        .filter(|a| matches!(a, Action::IssueComment { .. }))
        .collect();
    assert_eq!(comments.len(), 2, "both concurrent comments should appear");
    assert!(actions.iter().any(|a| matches!(a, Action::Merge)));

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.comments.len(), 2);
    assert_eq!(state.status, IssueStatus::Open);
}

#[test]
fn test_concurrent_close_and_comment() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Close vs comment");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    close_issue(&repo, &ref_name, &alice());
    let alice_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, root_oid, true, "bob fork")
        .unwrap();
    add_comment(&repo, &ref_name, &bob(), "Wait, I have thoughts");
    let bob_tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, bob_tip, true, "remote")
        .unwrap();
    repo.reference(&ref_name, alice_tip, true, "restore")
        .unwrap();

    dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.comments.len(), 1);
    assert_eq!(state.status, IssueStatus::Closed);
}

#[test]
fn test_concurrent_close_and_reopen() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Status conflict");

    close_issue(&repo, &ref_name, &alice());
    let closed_oid = repo.refname_to_id(&ref_name).unwrap();

    add_comment(&repo, &ref_name, &alice(), "Staying closed");
    let alice_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, closed_oid, true, "bob fork")
        .unwrap();
    reopen_issue(&repo, &ref_name, &bob());
    let bob_tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, bob_tip, true, "remote")
        .unwrap();
    repo.reference(&ref_name, alice_tip, true, "restore")
        .unwrap();

    dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();

    let _state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    let events = dag::walk_events(&repo, &ref_name).unwrap();
    let has_close = events
        .iter()
        .any(|(_, e)| matches!(e.action, Action::IssueClose { .. }));
    let has_reopen = events
        .iter()
        .any(|(_, e)| matches!(e.action, Action::IssueReopen));
    assert!(has_close, "close event must be in DAG");
    assert!(has_reopen, "reopen event must be in DAG");
}

#[test]
fn test_fast_forward_reconcile() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "FF test");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    add_comment(&repo, &ref_name, &alice(), "Extra comment");
    let ahead_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, root_oid, true, "reset local")
        .unwrap();
    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, ahead_tip, true, "remote ahead")
        .unwrap();

    let (result, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
    assert_eq!(result, ahead_tip, "should fast-forward to remote tip");
    assert_eq!(outcome, dag::ReconcileOutcome::FastForward);

    let events = dag::walk_events(&repo, &ref_name).unwrap();
    assert_eq!(events.len(), 2);
    assert!(!events
        .iter()
        .any(|(_, e)| matches!(e.action, Action::Merge)));
}

#[test]
fn test_no_op_when_already_in_sync() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Already synced");
    let tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, tip, true, "same tip").unwrap();

    let (result, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
    assert_eq!(result, tip);
    assert_eq!(outcome, dag::ReconcileOutcome::AlreadyCurrent);
}

#[test]
fn test_local_ahead_no_merge() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Local ahead");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    add_comment(&repo, &ref_name, &alice(), "Local only comment");
    let local_tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, root_oid, true, "remote behind")
        .unwrap();

    let (result, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();
    assert_eq!(result, local_tip, "local should stay ahead");
    assert_eq!(outcome, dag::ReconcileOutcome::LocalAhead);
}

// ---------------------------------------------------------------------------
// Patch collaboration tests
// ---------------------------------------------------------------------------

#[test]
fn test_patch_review_workflow() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = create_patch(&repo, &alice(), "Add feature X");

    add_review(&repo, &ref_name, &bob(), ReviewVerdict::RequestChanges);

    let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.reviews.len(), 1);
    assert_eq!(state.reviews[0].verdict, ReviewVerdict::RequestChanges);

    let sk = test_signing_key();
    let event = Event {
        timestamp: now(),
        author: alice(),
        action: Action::PatchRevision {
            commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
            tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
            body: Some("Updated implementation".to_string()),
        },
        clock: 0,
    };
    dag::append_event(&repo, &ref_name, &event, &sk).unwrap();

    add_review(&repo, &ref_name, &bob(), ReviewVerdict::Approve);

    let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.reviews.len(), 2);
    // PatchRevision body is on the revision, not the patch body
    assert_eq!(state.revisions.len(), 2); // revision 1 from create + revision 2 from PatchRevision
    assert_eq!(state.revisions[1].body.as_deref(), Some("Updated implementation"));
}

#[test]
fn test_concurrent_reviews_on_patch() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = create_patch(&repo, &alice(), "Concurrent review");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    add_review(&repo, &ref_name, &alice(), ReviewVerdict::Approve);
    let alice_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, root_oid, true, "bob fork")
        .unwrap();
    add_review(&repo, &ref_name, &bob(), ReviewVerdict::RequestChanges);
    let bob_tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/patches/{}", id);
    repo.reference(&remote_ref, bob_tip, true, "remote")
        .unwrap();
    repo.reference(&ref_name, alice_tip, true, "restore")
        .unwrap();

    dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &test_signing_key()).unwrap();

    let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.reviews.len(), 2, "both reviews should be present");
    assert_eq!(state.status, PatchStatus::Open);
}

// ---------------------------------------------------------------------------
// Three-way concurrent edits
// ---------------------------------------------------------------------------

#[test]
fn test_three_way_fork_sequential_reconcile() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let charlie = Author {
        name: "Charlie".to_string(),
        email: "charlie@example.com".to_string(),
    };

    let (ref_name, id) = open_issue(&repo, &alice(), "Three-way");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    add_comment(&repo, &ref_name, &alice(), "Alice here");
    let alice_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, root_oid, true, "bob").unwrap();
    add_comment(&repo, &ref_name, &bob(), "Bob here");
    let bob_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, root_oid, true, "charlie")
        .unwrap();
    add_comment(&repo, &ref_name, &charlie, "Charlie here");
    let charlie_tip = repo.refname_to_id(&ref_name).unwrap();

    repo.reference(&ref_name, alice_tip, true, "alice").unwrap();
    let bob_ref = "refs/collab/sync/origin/issues/bob_temp";
    repo.reference(bob_ref, bob_tip, true, "bob remote")
        .unwrap();
    dag::reconcile(&repo, &ref_name, bob_ref, &alice(), &test_signing_key()).unwrap();

    let charlie_ref = "refs/collab/sync/origin/issues/charlie_temp";
    repo.reference(charlie_ref, charlie_tip, true, "charlie remote")
        .unwrap();
    dag::reconcile(&repo, &ref_name, charlie_ref, &alice(), &test_signing_key()).unwrap();

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.comments.len(), 3, "all three comments must survive");

    let bodies: Vec<&str> = state.comments.iter().map(|c| c.body.as_str()).collect();
    assert!(bodies.contains(&"Alice here"));
    assert!(bodies.contains(&"Bob here"));
    assert!(bodies.contains(&"Charlie here"));
}

// ---------------------------------------------------------------------------
// Multiple issues don't interfere
// ---------------------------------------------------------------------------

#[test]
fn test_multiple_issues_independent() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref1, id1) = open_issue(&repo, &alice(), "Issue one");
    let (ref2, id2) = open_issue(&repo, &bob(), "Issue two");

    add_comment(&repo, &ref1, &bob(), "Comment on issue one");
    close_issue(&repo, &ref2, &bob());

    let s1 = IssueState::from_ref(&repo, &ref1, &id1).unwrap();
    let s2 = IssueState::from_ref(&repo, &ref2, &id2).unwrap();

    assert_eq!(s1.status, IssueStatus::Open);
    assert_eq!(s1.comments.len(), 1);
    assert_eq!(s2.status, IssueStatus::Closed);
    assert_eq!(s2.comments.len(), 0);
}

// ---------------------------------------------------------------------------
// Edge cases
// ---------------------------------------------------------------------------

#[test]
fn test_empty_issue_body() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "No body");
    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.body, "");
}

#[test]
fn test_many_comments_performance() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Many comments");
    for i in 0..50 {
        add_comment(&repo, &ref_name, &alice(), &format!("Comment {}", i));
    }

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.comments.len(), 50);
}

#[test]
fn test_resolve_prefix_match() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (_, id) = open_issue(&repo, &alice(), "Prefix test");
    let short = &id[..8];

    let (resolved_ref, resolved_id) = state::resolve_issue_ref(&repo, short).unwrap();
    assert_eq!(resolved_id, id);
    assert_eq!(resolved_ref, format!("refs/collab/issues/{}", id));
}

// ---------------------------------------------------------------------------
// T012: Signed event integration test
// ---------------------------------------------------------------------------

#[test]
fn test_signed_event_in_dag() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    // Set up a signing key on disk so issue::open() can find it
    let config_dir = tmp.path().join("test-config");
    setup_signing_key(&config_dir);
    let sk = signing::load_signing_key(&config_dir).unwrap();

    // Create an issue using dag primitives with the signing key
    let event = Event {
        timestamp: now(),
        author: alice(),
        action: Action::IssueOpen {
            title: "Signed issue".to_string(),
            body: "".to_string(),
            relates_to: None,
        },
        clock: 0,
    };
    let oid = dag::create_root_event(&repo, &event, &sk).unwrap();
    let id = oid.to_string();
    let ref_name = format!("refs/collab/issues/{}", id);
    repo.reference(&ref_name, oid, false, "test open").unwrap();

    // Read the raw blobs from the commit tree
    let tip = repo.refname_to_id(&ref_name).unwrap();
    let commit = repo.find_commit(tip).unwrap();
    let tree = commit.tree().unwrap();

    // event.json should be a plain Event (no signature fields)
    let event_entry = tree.get_name("event.json").unwrap();
    let event_blob = repo.find_blob(event_entry.id()).unwrap();
    let event: Event = serde_json::from_slice(event_blob.content()).unwrap();
    assert!(matches!(event.action, Action::IssueOpen { .. }));

    // signature and pubkey should be separate blobs
    let sig_entry = tree.get_name("signature").expect("signature blob should exist");
    let pk_entry = tree.get_name("pubkey").expect("pubkey blob should exist");
    let sig_blob = repo.find_blob(sig_entry.id()).unwrap();
    let pk_blob = repo.find_blob(pk_entry.id()).unwrap();
    let sig_str = std::str::from_utf8(sig_blob.content()).unwrap();
    let pk_str = std::str::from_utf8(pk_blob.content()).unwrap();
    assert!(!sig_str.is_empty(), "signature should be present");
    assert!(!pk_str.is_empty(), "pubkey should be present");

    // manifest.json should exist
    let manifest_entry = tree.get_name("manifest.json").expect("manifest.json should exist");
    let manifest_blob = repo.find_blob(manifest_entry.id()).unwrap();
    let manifest_str = std::str::from_utf8(manifest_blob.content()).unwrap();
    assert!(manifest_str.contains("\"version\":1"), "manifest should contain version");
    assert!(manifest_str.contains("\"format\":\"git-collab\""), "manifest should contain format");

    // Verify the detached signature
    let detached = DetachedSignature {
        signature: sig_str.to_string(),
        pubkey: pk_str.to_string(),
    };
    let status = signing::verify_detached(&event, &detached).unwrap();
    assert_eq!(status, VerifyStatus::Valid, "signature should verify as valid");

    // walk_events should still extract the Event correctly
    let events = dag::walk_events(&repo, &ref_name).unwrap();
    assert_eq!(events.len(), 1);
    assert!(matches!(events[0].1.action, Action::IssueOpen { .. }));
}

// ---------------------------------------------------------------------------
// T013: Missing signing key error test
// ---------------------------------------------------------------------------

#[test]
fn test_issue_open_without_signing_key_returns_key_not_found() {
    let tmp = TempDir::new().unwrap();
    let _repo = init_repo(tmp.path(), &alice());

    // Point to a nonexistent config dir so load_signing_key fails
    let bad_config = tmp.path().join("nonexistent-config");
    let result = signing::load_signing_key(&bad_config);

    assert!(result.is_err());
    match result.unwrap_err() {
        Error::KeyNotFound => {} // expected
        other => panic!("expected KeyNotFound error, got: {:?}", other),
    }
}

// ---------------------------------------------------------------------------
// Helpers for branch-based patch tests
// ---------------------------------------------------------------------------

/// Create an initial commit on the given branch so the repo is not empty.
fn make_initial_commit(repo: &git2::Repository, branch: &str) -> git2::Oid {
    let sig = git2::Signature::now("Test", "test@example.com").unwrap();
    let mut tb = repo.treebuilder(None).unwrap();
    let blob = repo.blob(b"init").unwrap();
    tb.insert("README.md", blob, 0o100644).unwrap();
    let tree_oid = tb.write().unwrap();
    let tree = repo.find_tree(tree_oid).unwrap();
    let oid = repo.commit(None, &sig, &sig, "initial commit", &tree, &[]).unwrap();
    let ref_name = format!("refs/heads/{}", branch);
    repo.reference(&ref_name, oid, true, "init branch").unwrap();
    // Set HEAD to this branch
    repo.set_head(&ref_name).unwrap();
    oid
}

/// Add a commit on the given branch, returns the new OID.
fn add_commit_on_branch(repo: &git2::Repository, branch: &str, filename: &str, content: &[u8]) -> git2::Oid {
    let ref_name = format!("refs/heads/{}", branch);
    let parent_oid = repo.refname_to_id(&ref_name).unwrap();
    let parent = repo.find_commit(parent_oid).unwrap();
    let sig = git2::Signature::now("Test", "test@example.com").unwrap();

    // Build a tree with the new file added to parent's tree
    let parent_tree = parent.tree().unwrap();
    let mut tb = repo.treebuilder(Some(&parent_tree)).unwrap();
    let blob = repo.blob(content).unwrap();
    tb.insert(filename, blob, 0o100644).unwrap();
    let tree_oid = tb.write().unwrap();
    let tree = repo.find_tree(tree_oid).unwrap();

    repo.commit(Some(&ref_name), &sig, &sig, &format!("add {}", filename), &tree, &[&parent]).unwrap()
}

/// Create a branch-based patch using DAG primitives.
fn create_branch_patch(
    repo: &git2::Repository,
    author: &Author,
    title: &str,
    branch: &str,
    base_ref: &str,
) -> (String, String) {
    let sk = test_signing_key();
    let event = Event {
        timestamp: now(),
        author: author.clone(),
        action: Action::PatchCreate {
            title: title.to_string(),
            body: "".to_string(),
            base_ref: base_ref.to_string(),
            branch: branch.to_string(),
            fixes: None,
            commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
            tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
            base_commit: None,
        },
        clock: 0,
    };
    let oid = dag::create_root_event(repo, &event, &sk).unwrap();
    let id = oid.to_string();
    let patch_ref = format!("refs/collab/patches/{}", id);
    repo.reference(&patch_ref, oid, false, "test branch patch").unwrap();
    (patch_ref, id)
}

// ---------------------------------------------------------------------------
// Phase 2: Branch resolution infrastructure tests (T007, T008)
// ---------------------------------------------------------------------------

#[test]
fn test_resolve_head_branch_based() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    // Create main branch and feature branch
    make_initial_commit(&repo, "main");
    let feature_tip = add_commit_on_branch(&repo, "main", "feature.rs", b"fn feature() {}");
    // Create the feature branch at the current tip
    repo.branch("feature/test", &repo.find_commit(feature_tip).unwrap(), false).unwrap();

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Test patch", "feature/test", "main");
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();

    // resolve_head should return the branch tip
    let resolved = state.resolve_head(&repo).unwrap();
    assert_eq!(resolved, feature_tip);

    // Now add a commit to the branch — resolve_head should return the new tip
    let new_tip = add_commit_on_branch(&repo, "feature/test", "more.rs", b"more code");
    let resolved2 = state.resolve_head(&repo).unwrap();
    assert_eq!(resolved2, new_tip);
}

#[test]
fn test_resolve_head_deleted_branch_error() {
    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", "f.rs", b"x");
    repo.branch("ephemeral", &repo.find_commit(tip).unwrap(), false).unwrap();

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Ephemeral patch", "ephemeral", "main");

    // Delete the branch
    let mut branch = repo.find_branch("ephemeral", git2::BranchType::Local).unwrap();
    branch.delete().unwrap();

    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
    let result = state.resolve_head(&repo);
    assert!(result.is_err(), "resolve_head should error when branch is deleted");
}

#[test]
fn test_staleness_up_to_date() {
    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", "f.rs", b"x");
    repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
    // Add one commit on the feature branch
    add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Up-to-date", "feat", "main");
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();

    let (ahead, behind) = state.staleness(&repo).unwrap();
    assert_eq!(ahead, 1, "feature branch has 1 commit ahead of main");
    assert_eq!(behind, 0, "main has not moved, so 0 behind");
}

#[test]
fn test_staleness_outdated() {
    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", "f.rs", b"x");
    repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
    // Add commit on feature branch
    add_commit_on_branch(&repo, "feat", "feat.rs", b"feature code");
    // Advance main by 2 commits
    add_commit_on_branch(&repo, "main", "main1.rs", b"main work 1");
    add_commit_on_branch(&repo, "main", "main2.rs", b"main work 2");

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Outdated", "feat", "main");
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();

    let (ahead, behind) = state.staleness(&repo).unwrap();
    assert_eq!(ahead, 1, "feature has 1 commit ahead");
    assert_eq!(behind, 2, "main has moved 2 commits ahead");
}

// ---------------------------------------------------------------------------
// Phase 3: US1 — Create Patch from Branch (T009-T012)
// ---------------------------------------------------------------------------

use git_collab::patch;

#[test]
fn test_create_patch_from_branch_populates_branch_field() {
    // T009: creating a patch from current branch populates `branch` field
    let cfg = ScopedTestConfig::new();
    cfg.ensure_signing_key();
    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", "f.rs", b"x");
    repo.branch("feature/foo", &repo.find_commit(tip).unwrap(), false).unwrap();
    add_commit_on_branch(&repo, "feature/foo", "feat.rs", b"feat");

    let id = patch::create(&repo, "My patch", "desc", "main", "feature/foo", None).unwrap();
    let patch_ref = format!("refs/collab/patches/{}", id);
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();

    assert_eq!(state.branch, "feature/foo");
    // resolve_head should return the branch tip
    let branch_tip = repo.refname_to_id("refs/heads/feature/foo").unwrap();
    assert_eq!(state.resolve_head(&repo).unwrap(), branch_tip);
}

#[test]
fn test_create_duplicate_patch_for_same_branch_returns_error() {
    // T011: creating a duplicate patch for same branch returns error
    let cfg = ScopedTestConfig::new();
    cfg.ensure_signing_key();
    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", "f.rs", b"x");
    repo.branch("feature/dup", &repo.find_commit(tip).unwrap(), false).unwrap();

    // First creation should succeed
    patch::create(&repo, "First", "", "main", "feature/dup", None).unwrap();

    // Second creation for same branch should fail
    let result = patch::create(&repo, "Second", "", "main", "feature/dup", None);
    assert!(result.is_err(), "duplicate branch patch should fail");
    let err_msg = result.unwrap_err().to_string();
    assert!(err_msg.contains("feature/dup"), "error should mention the branch name");
}

#[test]
fn test_create_patch_from_base_branch_returns_error() {
    // T012: creating a patch when on base branch returns error
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());
    make_initial_commit(&repo, "main");

    let result = patch::create(&repo, "Bad patch", "", "main", "main", None);
    assert!(result.is_err(), "creating patch from base branch should fail");
    let err_msg = result.unwrap_err().to_string();
    assert!(err_msg.contains("base branch"), "error should mention base branch");
}

// ---------------------------------------------------------------------------
// Phase 4: US2 — Staleness and Diff (T019-T021)
// ---------------------------------------------------------------------------

#[test]
fn test_patch_show_up_to_date_staleness() {
    // T019: patch show on up-to-date patch shows "0 behind"
    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", "f.rs", b"x");
    repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
    add_commit_on_branch(&repo, "feat", "feat.rs", b"code");

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Up to date", "feat", "main");
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
    let (_, behind) = state.staleness(&repo).unwrap();
    assert_eq!(behind, 0);
}

#[test]
fn test_patch_show_outdated_staleness() {
    // T020: patch show on outdated patch shows correct behind count
    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", "f.rs", b"x");
    repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
    add_commit_on_branch(&repo, "feat", "feat.rs", b"code");
    // Advance main
    add_commit_on_branch(&repo, "main", "m1.rs", b"m1");
    add_commit_on_branch(&repo, "main", "m2.rs", b"m2");
    add_commit_on_branch(&repo, "main", "m3.rs", b"m3");

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Outdated", "feat", "main");
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
    let (ahead, behind) = state.staleness(&repo).unwrap();
    assert_eq!(ahead, 1);
    assert_eq!(behind, 3);
}

// ---------------------------------------------------------------------------
// Backward compat: head_commit alias for branch field
// ---------------------------------------------------------------------------

#[test]
fn test_patch_create_with_head_commit_field_deserializes() {
    // Events created with "head_commit" instead of "branch" should still work
    let json = r#"{
        "timestamp": "2026-03-21T00:00:00+00:00",
        "author": {"name": "agent", "email": "agent@test"},
        "action": {
            "type": "patch.create",
            "title": "Agent patch",
            "body": "from worktree",
            "base_ref": "main",
            "head_commit": "abc123def456",
            "commit": "abc123",
            "tree": "def456"
        }
    }"#;
    let event: Event = serde_json::from_str(json).unwrap();
    match event.action {
        Action::PatchCreate { branch, .. } => {
            assert_eq!(branch, "abc123def456");
        }
        _ => panic!("expected PatchCreate"),
    }
}

#[test]
fn test_patch_create_with_branch_field_still_works() {
    let json = r#"{
        "timestamp": "2026-03-21T00:00:00+00:00",
        "author": {"name": "dev", "email": "dev@test"},
        "action": {
            "type": "patch.create",
            "title": "Normal patch",
            "body": "",
            "base_ref": "main",
            "branch": "feature-branch",
            "commit": "abc123",
            "tree": "def456"
        }
    }"#;
    let event: Event = serde_json::from_str(json).unwrap();
    match event.action {
        Action::PatchCreate { branch, .. } => {
            assert_eq!(branch, "feature-branch");
        }
        _ => panic!("expected PatchCreate"),
    }
}

#[test]
fn test_resolve_head_with_oid_string() {
    // If branch field contains a hex OID, resolve_head should try to parse it as an OID
    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", "f.rs", b"content");

    // Create a patch where "branch" is actually a commit OID string
    let sk = test_signing_key();
    let event = Event {
        timestamp: now(),
        author: alice(),
        action: Action::PatchCreate {
            title: "OID-based patch".to_string(),
            body: "".to_string(),
            base_ref: "main".to_string(),
            branch: tip.to_string(),
            fixes: None,
            commit: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(),
            tree: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(),
            base_commit: None,
        },
        clock: 0,
    };
    let oid = dag::create_root_event(&repo, &event, &sk).unwrap();
    let id = oid.to_string();
    let ref_name = format!("refs/collab/patches/{}", id);
    repo.reference(&ref_name, oid, false, "test").unwrap();

    let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
    let resolved = state.resolve_head(&repo).unwrap();
    assert_eq!(resolved, tip);
}

// ---------------------------------------------------------------------------
// Phase 5: US3 — Merge auto-detection
// ---------------------------------------------------------------------------

#[test]
fn test_auto_detect_merged_patch_via_git_merge() {
    // When a user merges the patch branch into the base branch manually
    // (using git merge), PatchState should auto-detect that the patch
    // is merged without needing `patch merge`.
    let cfg = ScopedTestConfig::new();
    cfg.ensure_signing_key();
    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
    let id = patch::create(&repo, "Auto-detect test", "", "main", "feat", None).unwrap();

    // Verify it's open
    let ref_name = format!("refs/collab/patches/{}", id);
    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();

    // Now PatchState should auto-detect that it's merged
    let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
    assert_eq!(state.status, PatchStatus::Merged, "should auto-detect merge");
}

#[test]
fn test_auto_detect_merged_patch_deleted_branch() {
    // If the patch branch was deleted after a manual merge,
    // auto-detection should not crash — patch stays Open.
    let cfg = ScopedTestConfig::new();
    cfg.ensure_signing_key();
    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");

    let id = patch::create(&repo, "Deleted branch test", "", "main", "feat", None).unwrap();

    // Delete the feature branch (simulating cleanup after merge)
    repo.find_reference("refs/heads/feat").unwrap().delete().unwrap();

    // Should not crash, patch stays Open (can't verify merge without the branch)
    let ref_name = format!("refs/collab/patches/{}", id);
    let state = PatchState::from_ref_uncached(&repo, &ref_name, &id).unwrap();
    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 cfg = ScopedTestConfig::new();
    cfg.ensure_signing_key();
    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)
// ---------------------------------------------------------------------------

#[test]
fn test_branch_push_auto_reflects_in_patch() {
    // T027: after pushing a commit to the branch, patch show reflects the new commit
    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", "f.rs", b"x");
    repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap();
    add_commit_on_branch(&repo, "feat", "v1.rs", b"version 1");

    let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Auto revise", "feat", "main");
    let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
    let head1 = state.resolve_head(&repo).unwrap();

    // Add another commit to the branch (simulating a push)
    let new_tip = add_commit_on_branch(&repo, "feat", "v2.rs", b"version 2");

    // Re-read the state and resolve head — should see the new commit
    let state2 = PatchState::from_ref(&repo, &patch_ref, &id).unwrap();
    let head2 = state2.resolve_head(&repo).unwrap();

    assert_ne!(head1, head2, "head should change after branch update");
    assert_eq!(head2, new_tip, "head should be the new tip");
}

// ---------------------------------------------------------------------------
// Log command tests
// ---------------------------------------------------------------------------

use git_collab::issue;
use git_collab::log;

#[test]
fn test_log_collects_events_from_all_refs() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    // Create an issue and a patch
    let (ref1, _) = open_issue(&repo, &alice(), "Log test issue");
    add_comment(&repo, &ref1, &bob(), "A comment");

    let (ref2, _) = create_patch(&repo, &alice(), "Log test patch");
    add_review(&repo, &ref2, &bob(), ReviewVerdict::Approve);

    let entries = log::collect_events(&repo, None).unwrap();
    // 2 issue events + 2 patch events = 4
    assert_eq!(entries.len(), 4);
}

#[test]
fn test_log_entries_sorted_by_timestamp() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref1, _) = open_issue(&repo, &alice(), "First");
    add_comment(&repo, &ref1, &bob(), "Second");

    let entries = log::collect_events(&repo, None).unwrap();
    assert!(entries.len() >= 2);
    // Timestamps should be non-decreasing
    for w in entries.windows(2) {
        assert!(w[0].timestamp <= w[1].timestamp);
    }
}

#[test]
fn test_log_limit() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref1, _) = open_issue(&repo, &alice(), "Limit test");
    add_comment(&repo, &ref1, &alice(), "c1");
    add_comment(&repo, &ref1, &alice(), "c2");
    add_comment(&repo, &ref1, &alice(), "c3");

    let entries = log::collect_events(&repo, Some(2)).unwrap();
    assert_eq!(entries.len(), 2);
}

#[test]
fn test_log_entry_fields() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (_, _) = open_issue(&repo, &alice(), "Field test");

    let entries = log::collect_events(&repo, None).unwrap();
    assert_eq!(entries.len(), 1);

    let entry = &entries[0];
    assert_eq!(entry.entity_kind, "issue");
    assert_eq!(entry.event_type, "IssueOpen");
    assert_eq!(entry.author.name, "Alice");
    assert_eq!(entry.summary, "open \"Field test\"");
    assert!(!entry.entity_id.is_empty());
    assert!(!entry.timestamp.is_empty());
}

#[test]
fn test_log_empty_repo() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let entries = log::collect_events(&repo, None).unwrap();
    assert!(entries.is_empty());
}

#[test]
fn test_log_format_output() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (_, _) = open_issue(&repo, &alice(), "Format test");

    let entries = log::collect_events(&repo, None).unwrap();
    let output = log::format_log(&entries);
    assert!(output.contains("issue"));
    assert!(output.contains("Alice"));
    assert!(output.contains("IssueOpen"));
}

// ---------------------------------------------------------------------------
// Pagination tests
// ---------------------------------------------------------------------------

/// Helper: capture output from issue::list_to_writer
fn capture_issue_list(
    repo: &git2::Repository,
    show_closed: bool,
    limit: Option<usize>,
    offset: Option<usize>,
) -> String {
    let mut buf = Vec::new();
    issue::list_to_writer(repo, show_closed, false, limit, offset, git_collab::cli::SortMode::Recent, &mut buf).unwrap();
    String::from_utf8(buf).unwrap()
}

/// Helper: capture output from patch::list_to_writer
fn capture_patch_list(
    repo: &git2::Repository,
    show_closed: bool,
    limit: Option<usize>,
    offset: Option<usize>,
) -> String {
    let mut buf = Vec::new();
    patch::list_to_writer(repo, show_closed, false, limit, offset, git_collab::cli::SortMode::Recent, &mut buf).unwrap();
    String::from_utf8(buf).unwrap()
}

#[test]
fn test_issue_list_limit() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    open_issue(&repo, &alice(), "Issue A");
    open_issue(&repo, &alice(), "Issue B");
    open_issue(&repo, &alice(), "Issue C");

    let all = state::list_issues(&repo).unwrap();
    assert_eq!(all.len(), 3);

    let output = capture_issue_list(&repo, false, Some(2), None);
    let lines: Vec<_> = output.lines().filter(|l| !l.is_empty()).collect();
    assert_eq!(lines.len(), 2, "limit=2 should show exactly 2 issues");
}

#[test]
fn test_issue_list_offset() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    open_issue(&repo, &alice(), "Issue A");
    open_issue(&repo, &alice(), "Issue B");
    open_issue(&repo, &alice(), "Issue C");

    let output_all = capture_issue_list(&repo, false, None, None);
    let all_lines: Vec<_> = output_all.lines().filter(|l| !l.is_empty()).collect();

    let output_offset = capture_issue_list(&repo, false, None, Some(1));
    let offset_lines: Vec<_> = output_offset.lines().filter(|l| !l.is_empty()).collect();

    assert_eq!(
        offset_lines.len(),
        all_lines.len() - 1,
        "offset=1 should skip one issue"
    );
    assert_eq!(offset_lines, &all_lines[1..]);
}

#[test]
fn test_issue_list_limit_and_offset() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    open_issue(&repo, &alice(), "Issue A");
    open_issue(&repo, &alice(), "Issue B");
    open_issue(&repo, &alice(), "Issue C");
    open_issue(&repo, &alice(), "Issue D");

    let output_all = capture_issue_list(&repo, false, None, None);
    let all_lines: Vec<_> = output_all.lines().filter(|l| !l.is_empty()).collect();
    assert_eq!(all_lines.len(), 4);

    let output = capture_issue_list(&repo, false, Some(2), Some(1));
    let lines: Vec<_> = output.lines().filter(|l| !l.is_empty()).collect();
    assert_eq!(
        lines.len(),
        2,
        "limit=2,offset=1 should show exactly 2 issues"
    );
    assert_eq!(lines, &all_lines[1..3]);
}

#[test]
fn test_issue_list_offset_beyond_end() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    open_issue(&repo, &alice(), "Issue A");

    let output = capture_issue_list(&repo, false, None, Some(100));
    assert!(
        output.contains("No issues found."),
        "offset beyond end should show 'No issues found.'"
    );
}

#[test]
fn test_patch_list_limit() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    create_patch(&repo, &alice(), "Patch A");
    create_patch(&repo, &alice(), "Patch B");
    create_patch(&repo, &alice(), "Patch C");

    let output = capture_patch_list(&repo, false, Some(2), None);
    let lines: Vec<_> = output.lines().filter(|l| !l.is_empty()).collect();
    assert_eq!(lines.len(), 2, "limit=2 should show exactly 2 patches");
}

#[test]
fn test_patch_list_offset() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    create_patch(&repo, &alice(), "Patch A");
    create_patch(&repo, &alice(), "Patch B");
    create_patch(&repo, &alice(), "Patch C");

    let output_all = capture_patch_list(&repo, false, None, None);
    let all_lines: Vec<_> = output_all.lines().filter(|l| !l.is_empty()).collect();

    let output_offset = capture_patch_list(&repo, false, None, Some(1));
    let offset_lines: Vec<_> = output_offset.lines().filter(|l| !l.is_empty()).collect();

    assert_eq!(offset_lines.len(), all_lines.len() - 1);
    assert_eq!(offset_lines, &all_lines[1..]);
}

#[test]
fn test_patch_list_limit_and_offset() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    create_patch(&repo, &alice(), "Patch A");
    create_patch(&repo, &alice(), "Patch B");
    create_patch(&repo, &alice(), "Patch C");
    create_patch(&repo, &alice(), "Patch D");

    let output_all = capture_patch_list(&repo, false, None, None);
    let all_lines: Vec<_> = output_all.lines().filter(|l| !l.is_empty()).collect();

    let output = capture_patch_list(&repo, false, Some(2), Some(1));
    let lines: Vec<_> = output.lines().filter(|l| !l.is_empty()).collect();
    assert_eq!(lines.len(), 2);
    assert_eq!(lines, &all_lines[1..3]);
}

#[test]
fn test_patch_list_offset_beyond_end() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    create_patch(&repo, &alice(), "Patch A");

    let output = capture_patch_list(&repo, false, None, Some(100));
    assert!(
        output.contains("No patches found."),
        "offset beyond end should show 'No patches found.'"
    );
}

// ---------------------------------------------------------------------------
// JSON serialization tests
// ---------------------------------------------------------------------------

#[test]
fn test_issue_state_serializes_to_json() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "JSON test issue");
    add_comment(&repo, &ref_name, &bob(), "A comment for JSON");

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    let json = serde_json::to_value(&state).unwrap();

    assert_eq!(json["title"], "JSON test issue");
    assert_eq!(json["status"], "open");
    assert_eq!(json["author"]["name"], "Alice");
    assert_eq!(json["comments"].as_array().unwrap().len(), 1);
    assert_eq!(json["comments"][0]["author"]["name"], "Bob");
    assert_eq!(json["comments"][0]["body"], "A comment for JSON");
    // commit_id should be serialized as a hex string
    assert!(json["comments"][0]["commit_id"].is_string());
}

#[test]
fn test_issue_state_closed_serializes() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Closed JSON");
    close_issue(&repo, &ref_name, &alice());

    let state = IssueState::from_ref(&repo, &ref_name, &id).unwrap();
    let json = serde_json::to_value(&state).unwrap();
    assert_eq!(json["status"], "closed");
}

#[test]
fn test_patch_state_serializes_to_json() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = create_patch(&repo, &alice(), "JSON patch test");
    add_review(&repo, &ref_name, &bob(), ReviewVerdict::Approve);

    let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap();
    let json = serde_json::to_value(&state).unwrap();

    assert_eq!(json["title"], "JSON patch test");
    assert_eq!(json["status"], "open");
    assert_eq!(json["base_ref"], "main");
    assert_eq!(json["branch"], "test-branch");
    assert_eq!(json["reviews"].as_array().unwrap().len(), 1);
    assert_eq!(json["reviews"][0]["verdict"], "approve");
}

#[test]
fn test_issue_list_json_output() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    open_issue(&repo, &alice(), "Issue one");
    open_issue(&repo, &bob(), "Issue two");

    let json_str = git_collab::issue::list_json(&repo, false, false, git_collab::cli::SortMode::Recent).unwrap();
    let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
    let arr = value.as_array().unwrap();
    assert_eq!(arr.len(), 2);

    let titles: Vec<&str> = arr.iter().map(|v| v["title"].as_str().unwrap()).collect();
    assert!(titles.contains(&"Issue one"));
    assert!(titles.contains(&"Issue two"));
}

#[test]
fn test_issue_list_json_filters_closed() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    open_issue(&repo, &alice(), "Open issue");
    let (ref2, _) = open_issue(&repo, &alice(), "Closed issue");
    close_issue(&repo, &ref2, &alice());

    // Without --all, only open issues
    let json_str = git_collab::issue::list_json(&repo, false, false, git_collab::cli::SortMode::Recent).unwrap();
    let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
    assert_eq!(value.as_array().unwrap().len(), 1);

    // With --all, both
    let json_str = git_collab::issue::list_json(&repo, true, false, git_collab::cli::SortMode::Recent).unwrap();
    let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
    assert_eq!(value.as_array().unwrap().len(), 2);
}

#[test]
fn test_issue_show_json_output() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = open_issue(&repo, &alice(), "Show JSON test");
    add_comment(&repo, &ref_name, &bob(), "test comment");

    let json_str = git_collab::issue::show_json(&repo, &id[..8]).unwrap();
    let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
    assert_eq!(value["title"], "Show JSON test");
    assert_eq!(value["comments"].as_array().unwrap().len(), 1);
}

#[test]
fn test_patch_list_json_output() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    create_patch(&repo, &alice(), "Patch one");
    create_patch(&repo, &bob(), "Patch two");

    let json_str = git_collab::patch::list_json(&repo, false, false, git_collab::cli::SortMode::Recent).unwrap();
    let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
    let arr = value.as_array().unwrap();
    assert_eq!(arr.len(), 2);
}

#[test]
fn test_patch_show_json_output() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());

    let (ref_name, id) = create_patch(&repo, &alice(), "Show patch JSON");
    add_review(&repo, &ref_name, &bob(), ReviewVerdict::Approve);

    let json_str = git_collab::patch::show_json(&repo, &id[..8]).unwrap();
    let value: serde_json::Value = serde_json::from_str(&json_str).unwrap();
    assert_eq!(value["title"], "Show patch JSON");
    assert_eq!(value["reviews"].as_array().unwrap().len(), 1);
}

// ---------------------------------------------------------------------------
// ReconcileOutcome tests
// ---------------------------------------------------------------------------

#[test]
fn test_reconcile_outcome_merge_on_divergent_refs() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());
    let sk = test_signing_key();

    let (ref_name, id) = open_issue(&repo, &alice(), "Divergent");

    // Save root OID, then add a local comment
    let root_oid = repo.refname_to_id(&ref_name).unwrap();
    add_comment(&repo, &ref_name, &alice(), "Local comment");
    let local_tip = repo.refname_to_id(&ref_name).unwrap();

    // Create a remote branch from root with a different comment
    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, root_oid, true, "remote branch point")
        .unwrap();
    add_comment(&repo, &remote_ref, &bob(), "Remote comment");

    // Restore local tip
    repo.reference(&ref_name, local_tip, true, "restore local tip")
        .unwrap();

    let (_oid, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap();
    assert_eq!(outcome, dag::ReconcileOutcome::Merge);
}

#[test]
fn test_reconcile_outcome_fast_forward() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());
    let sk = test_signing_key();

    let (ref_name, id) = open_issue(&repo, &alice(), "FF test");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();

    // Remote is ahead: add a comment on the remote branch
    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, root_oid, true, "remote start")
        .unwrap();
    add_comment(&repo, &remote_ref, &bob(), "Remote only");
    let remote_tip = repo.refname_to_id(&remote_ref).unwrap();

    // Local stays at root
    repo.reference(&ref_name, root_oid, true, "reset local")
        .unwrap();

    let (oid, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap();
    assert_eq!(outcome, dag::ReconcileOutcome::FastForward);
    assert_eq!(oid, remote_tip);
}

#[test]
fn test_reconcile_outcome_already_current() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());
    let sk = test_signing_key();

    let (ref_name, id) = open_issue(&repo, &alice(), "Same tip");
    let tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, tip, true, "same").unwrap();

    let (oid, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap();
    assert_eq!(outcome, dag::ReconcileOutcome::AlreadyCurrent);
    assert_eq!(oid, tip);
}

#[test]
fn test_reconcile_outcome_local_ahead() {
    let tmp = TempDir::new().unwrap();
    let repo = init_repo(tmp.path(), &alice());
    let sk = test_signing_key();

    let (ref_name, id) = open_issue(&repo, &alice(), "Local ahead");
    let root_oid = repo.refname_to_id(&ref_name).unwrap();
    add_comment(&repo, &ref_name, &alice(), "Local only");
    let local_tip = repo.refname_to_id(&ref_name).unwrap();

    let remote_ref = format!("refs/collab/sync/origin/issues/{}", id);
    repo.reference(&remote_ref, root_oid, true, "remote behind")
        .unwrap();

    let (oid, outcome) = dag::reconcile(&repo, &ref_name, &remote_ref, &alice(), &sk).unwrap();
    assert_eq!(outcome, dag::ReconcileOutcome::LocalAhead);
    assert_eq!(oid, local_tip);
}