ed0b2ce9
Refactor patches to reference branches instead of commit OIDs
a73x 2026-03-21 10:34
Patches now store a branch name and resolve the tip live via git, making the review DAG purely a review layer on top of normal git branches. Includes staleness detection (ahead/behind), three-dot diff, duplicate branch prevention, and full backward compatibility with old-format patches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/src/cli.rs b/src/cli.rs index a676fc7..a896e87 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -92,9 +92,12 @@ pub enum PatchCmd { /// Base branch ref #[arg(long, default_value = "main")] base: String, /// Head commit to review #[arg(long)] head: String, /// Source branch (defaults to current branch) #[arg(short = 'B', long)] branch: Option<String>, /// Deprecated: use --branch instead #[arg(long, hide = true)] head: Option<String>, }, /// List patches List { diff --git a/src/event.rs b/src/event.rs index c26d86b..c201a72 100644 --- a/src/event.rs +++ b/src/event.rs @@ -32,6 +32,8 @@ pub enum Action { body: String, base_ref: String, head_commit: String, #[serde(default, skip_serializing_if = "Option::is_none")] branch: Option<String>, }, PatchRevise { body: Option<String>, diff --git a/src/main.rs b/src/main.rs index 81f1568..493f417 100644 --- a/src/main.rs +++ b/src/main.rs @@ -46,9 +46,60 @@ fn run(cli: Cli) -> Result<(), error::Error> { title, body, base, branch, head, } => { let id = patch::create(&repo, &title, &body, &base, &head)?; // Resolve branch name: --branch takes priority, then --head (deprecated), then current branch let branch_name = if let Some(b) = branch { b } else if let Some(h) = head { // Deprecated --head: if it looks like a branch name, use it; // otherwise try to resolve as OID and create auto-branch if repo.refname_to_id(&format!("refs/heads/{}", h)).is_ok() { h } else if let Ok(obj) = repo.revparse_single(&h) { // OID provided — auto-create a branch let commit = obj.into_commit().map_err(|_| { error::Error::Cmd("--head value is not a commit".to_string()) })?; let short_oid = &commit.id().to_string()[..8]; let auto_branch = format!("collab/patch/{}", short_oid); repo.branch(&auto_branch, &commit, false)?; auto_branch } else { return Err(error::Error::Cmd(format!( "cannot resolve '{}' as a branch or commit", h ))); } } else { // Default to current branch let head_ref = repo.head().map_err(|_| { error::Error::Cmd("cannot determine current branch (detached HEAD?)".to_string()) })?; if head_ref.is_branch() { let name = head_ref.shorthand().ok_or_else(|| { error::Error::Cmd("cannot determine branch name".to_string()) })?; if name == base { return Err(error::Error::Cmd( "cannot create patch from base branch; switch to a feature branch first".to_string(), )); } name.to_string() } else { // Detached HEAD — auto-create branch let oid = head_ref.target().ok_or_else(|| { error::Error::Cmd("cannot determine HEAD OID".to_string()) })?; let commit = repo.find_commit(oid)?; let short_oid = &oid.to_string()[..8]; let auto_branch = format!("collab/patch/{}", short_oid); repo.branch(&auto_branch, &commit, false)?; auto_branch } }; let id = patch::create(&repo, &title, &body, &base, &branch_name)?; println!("Created patch {:.8}", id); Ok(()) } diff --git a/src/patch.rs b/src/patch.rs index fca1add..6f890a1 100644 --- a/src/patch.rs +++ b/src/patch.rs @@ -10,8 +10,36 @@ pub fn create( title: &str, body: &str, base_ref: &str, head_commit: &str, branch: &str, ) -> Result<String, crate::error::Error> { // Reject creating a patch from the base branch if branch == base_ref { return Err(crate::error::Error::Cmd( "cannot create patch from base branch".to_string(), )); } // Resolve branch to its tip OID let branch_ref = format!("refs/heads/{}", branch); let tip_oid = repo .refname_to_id(&branch_ref) .map_err(|e| crate::error::Error::Cmd(format!("branch '{}' not found: {}", branch, e)))?; // Check for duplicate: scan open patches for matching branch let patches = state::list_patches(repo)?; for p in &patches { if p.status == PatchStatus::Open { if let Some(ref existing_branch) = p.branch { if existing_branch == branch { return Err(crate::error::Error::Cmd(format!( "patch already exists for branch '{}'", branch ))); } } } } let author = get_author(repo)?; let event = Event { timestamp: chrono::Utc::now().to_rfc3339(), @@ -20,7 +48,8 @@ pub fn create( title: title.to_string(), body: body.to_string(), base_ref: base_ref.to_string(), head_commit: head_commit.to_string(), head_commit: tip_oid.to_string(), branch: Some(branch.to_string()), }, }; let oid = dag::create_root_event(repo, &event)?; @@ -68,7 +97,22 @@ pub fn show(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Erro println!("Patch {} [{}]", &p.id[..8], status); println!("Title: {}", p.title); println!("Author: {} <{}>", p.author.name, p.author.email); println!("Base: {} Head: {:.8}", p.base_ref, p.head_commit); if let Some(ref branch) = p.branch { match p.resolve_head(repo) { Ok(_) => { println!("Branch: {} -> {}", branch, p.base_ref); if let Ok((ahead, behind)) = p.staleness(repo) { let freshness = if behind == 0 { "up-to-date" } else { "outdated" }; println!("Commits: {} ahead, {} behind ({})", ahead, behind, freshness); } } Err(_) => { println!("Branch: {} (not found)", branch); } } } else { println!("Base: {} Head: {:.8}", p.base_ref, p.head_commit); } println!("Created: {}", p.created_at); if !p.body.is_empty() { println!("\n{}", p.body); @@ -191,13 +235,10 @@ pub fn merge(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Err .into()); } // Resolve the head commit (supports short OIDs via revparse) let head_obj = repo .revparse_single(&p.head_commit) // Resolve the head commit: use branch tip for branch-based patches, stored OID otherwise let head_oid = p.resolve_head(repo)?; let head_commit = repo.find_commit(head_oid) .map_err(|_| git2::Error::from_str("cannot resolve head commit in patch"))?; let head_commit = head_obj .into_commit() .map_err(|_| git2::Error::from_str("head ref is not a commit"))?; let base_ref = format!("refs/heads/{}", p.base_ref); let base_oid = repo.refname_to_id(&base_ref)?; diff --git a/src/state.rs b/src/state.rs index e2a02c0..45c4bfc 100644 --- a/src/state.rs +++ b/src/state.rs @@ -63,6 +63,7 @@ pub struct PatchState { pub status: PatchStatus, pub base_ref: String, pub head_commit: String, pub branch: Option<String>, pub comments: Vec<Comment>, pub inline_comments: Vec<InlineComment>, pub reviews: Vec<Review>, @@ -135,6 +136,46 @@ impl IssueState { } impl PatchState { /// Returns true if this patch tracks a git branch (new format). /// Returns false for old-format patches that only store a commit OID. pub fn is_branch_based(&self) -> bool { self.branch.is_some() } /// Resolve the current head commit OID for this patch. /// For branch-based patches, resolves `refs/heads/{branch}` live. /// For old-format patches, uses `revparse_single` on the stored `head_commit`. pub fn resolve_head(&self, repo: &Repository) -> Result<Oid, crate::error::Error> { if let Some(ref branch) = self.branch { let ref_name = format!("refs/heads/{}", branch); repo.refname_to_id(&ref_name).map_err(|e| { crate::error::Error::Cmd(format!( "branch '{}' not found: {}", branch, e )) }) } else { let obj = repo.revparse_single(&self.head_commit).map_err(|e| { crate::error::Error::Cmd(format!( "cannot resolve head commit '{}': {}", self.head_commit, e )) })?; Ok(obj.id()) } } /// Compute staleness: how many commits the branch is ahead of base, /// and how many commits the base is ahead of the branch. /// Returns (ahead, behind). pub fn staleness(&self, repo: &Repository) -> Result<(usize, usize), crate::error::Error> { let branch_tip = self.resolve_head(repo)?; let base_ref = format!("refs/heads/{}", self.base_ref); let base_tip = repo.refname_to_id(&base_ref)?; let (ahead, behind) = repo.graph_ahead_behind(branch_tip, base_tip)?; Ok((ahead, behind)) } pub fn from_ref( repo: &Repository, ref_name: &str, @@ -152,6 +193,7 @@ impl PatchState { body, base_ref, head_commit, branch, } => { state = Some(PatchState { id: id.to_string(), @@ -160,6 +202,7 @@ impl PatchState { status: PatchStatus::Open, base_ref, head_commit, branch, comments: Vec::new(), inline_comments: Vec::new(), reviews: Vec::new(), diff --git a/src/tui.rs b/src/tui.rs index 05d05dd..80a2185 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -30,12 +30,21 @@ enum ViewMode { Diff, } /// Cached staleness info for a patch. #[derive(Clone)] struct PatchBranchInfo { branch: Option<String>, staleness: Option<(usize, usize)>, branch_exists: bool, } struct App { tab: Tab, issues: Vec<IssueState>, patches: Vec<PatchState>, list_state: ListState, diff_cache: HashMap<String, String>, branch_info_cache: HashMap<String, PatchBranchInfo>, scroll: u16, pane: Pane, mode: ViewMode, @@ -54,6 +63,7 @@ impl App { patches, list_state, diff_cache: HashMap::new(), branch_info_cache: HashMap::new(), scroll: 0, pane: Pane::ItemList, mode: ViewMode::Details, @@ -153,18 +163,29 @@ impl App { fn generate_diff(repo: &Repository, patch: &PatchState) -> String { let result = (|| -> Result<String, Error> { let head_obj = repo .revparse_single(&patch.head_commit) let head_oid = patch.resolve_head(repo)?; let head_commit = repo.find_commit(head_oid) .map_err(|e| Error::Cmd(format!("bad head ref: {}", e)))?; let head_commit = head_obj .into_commit() .map_err(|_| Error::Cmd("head ref is not a commit".to_string()))?; let head_tree = head_commit.tree()?; let base_ref = format!("refs/heads/{}", patch.base_ref); // For branch-based patches, use three-dot diff (merge-base to branch tip) // For old-format patches, use two-dot diff (base tip to head) let base_tree = if let Ok(base_oid) = repo.refname_to_id(&base_ref) { let base_commit = repo.find_commit(base_oid)?; Some(base_commit.tree()?) if patch.is_branch_based() { // Three-dot: find merge-base, diff from there if let Ok(merge_base_oid) = repo.merge_base(base_oid, head_oid) { let merge_base_commit = repo.find_commit(merge_base_oid)?; Some(merge_base_commit.tree()?) } else { let base_commit = repo.find_commit(base_oid)?; Some(base_commit.tree()?) } } else { let base_commit = repo.find_commit(base_oid)?; Some(base_commit.tree()?) } } else { None }; @@ -237,15 +258,54 @@ fn run_loop( repo: &Repository, ) -> Result<(), Error> { loop { // Cache diff for selected patch if needed if app.tab == Tab::Patches && app.mode == ViewMode::Diff { // Cache diff and branch info for selected patch if needed if app.tab == Tab::Patches { if let Some(idx) = app.list_state.selected() { let visible = app.visible_patches(); if let Some(patch) = visible.get(idx) { if !app.diff_cache.contains_key(&patch.id) { // Collect info we need without holding the borrow let patch_data = { let visible = app.visible_patches(); visible.get(idx).map(|patch| { let id = patch.id.clone(); let diff = generate_diff(repo, patch); app.diff_cache.insert(id, diff); let needs_branch_info = !app.branch_info_cache.contains_key(&id); let needs_diff = app.mode == ViewMode::Diff && !app.diff_cache.contains_key(&id); let branch_info = if needs_branch_info { Some(if patch.is_branch_based() { let branch_exists = patch.resolve_head(repo).is_ok(); let staleness = if branch_exists { patch.staleness(repo).ok() } else { None }; PatchBranchInfo { branch: patch.branch.clone(), staleness, branch_exists, } } else { PatchBranchInfo { branch: None, staleness: None, branch_exists: false, } }) } else { None }; let diff = if needs_diff { Some(generate_diff(repo, patch)) } else { None }; (id, branch_info, diff) }) }; if let Some((id, branch_info, diff)) = patch_data { if let Some(info) = branch_info { app.branch_info_cache.insert(id.clone(), info); } if let Some(d) = diff { app.diff_cache.insert(id, d); } } } @@ -467,7 +527,10 @@ fn render_detail(frame: &mut Frame, app: &App, area: Rect) { let selected_idx = app.list_state.selected().unwrap_or(0); match visible.get(selected_idx) { Some(patch) => match app.mode { ViewMode::Details => build_patch_detail(patch), ViewMode::Details => { let branch_info = app.branch_info_cache.get(&patch.id).cloned(); build_patch_detail(patch, branch_info.as_ref()) } ViewMode::Diff => { let diff_text = app .diff_cache @@ -569,7 +632,7 @@ fn build_issue_detail(issue: &IssueState) -> Text<'static> { Text::from(lines) } fn build_patch_detail(patch: &PatchState) -> Text<'static> { fn build_patch_detail(patch: &PatchState, branch_info: Option<&PatchBranchInfo>) -> Text<'static> { let status = match patch.status { PatchStatus::Open => "open", PatchStatus::Closed => "closed", @@ -595,7 +658,40 @@ fn build_patch_detail(patch: &PatchState) -> Text<'static> { Span::styled("Author: ", Style::default().fg(Color::DarkGray)), Span::raw(format!("{} <{}>", patch.author.name, patch.author.email)), ]), Line::from(vec![ ]; // Show branch info for branch-based patches, head commit for old-format if let Some(info) = branch_info { if let Some(ref branch) = info.branch { if info.branch_exists { lines.push(Line::from(vec![ Span::styled("Branch: ", Style::default().fg(Color::DarkGray)), Span::styled( branch.clone(), Style::default().fg(Color::Cyan), ), Span::raw(format!(" -> {}", patch.base_ref)), ])); if let Some((ahead, behind)) = info.staleness { let freshness = if behind == 0 { "up-to-date" } else { "outdated" }; lines.push(Line::from(vec![ Span::styled("Commits: ", Style::default().fg(Color::DarkGray)), Span::raw(format!("{} ahead, {} behind ({})", ahead, behind, freshness)), ])); } } else { lines.push(Line::from(vec![ Span::styled("Branch: ", Style::default().fg(Color::DarkGray)), Span::styled( branch.clone(), Style::default().fg(Color::Red), ), Span::styled(" (not found)", Style::default().fg(Color::Red)), ])); } } } else { lines.push(Line::from(vec![ Span::styled("Base: ", Style::default().fg(Color::DarkGray)), Span::raw(patch.base_ref.clone()), Span::raw(" "), @@ -604,12 +700,13 @@ fn build_patch_detail(patch: &PatchState) -> Text<'static> { format!("{:.8}", patch.head_commit), Style::default().fg(Color::Cyan), ), ]), Line::from(vec![ Span::styled("Created: ", Style::default().fg(Color::DarkGray)), Span::raw(patch.created_at.clone()), ]), ]; ])); } lines.push(Line::from(vec![ Span::styled("Created: ", Style::default().fg(Color::DarkGray)), Span::raw(patch.created_at.clone()), ])); if !patch.body.is_empty() { lines.push(Line::raw("")); diff --git a/tests/collab_test.rs b/tests/collab_test.rs index f19b6a9..a1f77a1 100644 --- a/tests/collab_test.rs +++ b/tests/collab_test.rs @@ -390,6 +390,7 @@ fn create_patch(repo: &Repository, author: &Author, title: &str) -> (String, Str body: "".to_string(), base_ref: "main".to_string(), head_commit: "abc123".to_string(), branch: None, }, }; let oid = dag::create_root_event(repo, &event).unwrap(); @@ -599,3 +600,480 @@ fn test_resolve_prefix_match() { assert_eq!(resolved_id, id); assert_eq!(resolved_ref, format!("refs/collab/issues/{}", id)); } // --------------------------------------------------------------------------- // 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: &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: &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(); let oid = repo.commit(Some(&ref_name), &sig, &sig, &format!("add {}", filename), &tree, &[&parent]).unwrap(); oid } /// Create a branch-based patch using DAG primitives. fn create_branch_patch( repo: &Repository, author: &Author, title: &str, branch: &str, base_ref: &str, ) -> (String, String) { let ref_name = format!("refs/heads/{}", branch); let tip = repo.refname_to_id(&ref_name).unwrap(); let event = Event { timestamp: now(), author: author.clone(), action: Action::PatchCreate { title: title.to_string(), body: "".to_string(), base_ref: base_ref.to_string(), head_commit: tip.to_string(), branch: Some(branch.to_string()), }, }; let oid = dag::create_root_event(repo, &event).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_oid_based() { let tmp = TempDir::new().unwrap(); let repo = init_repo(tmp.path(), &alice()); make_initial_commit(&repo, "main"); let commit_oid = add_commit_on_branch(&repo, "main", "file.rs", b"content"); // Create old-format (OID-based) patch let (_patch_ref, _id) = create_patch(&repo, &alice(), "Old patch"); // The test helper uses "abc123" as head_commit which won't resolve. // Create a proper old-format patch with a real OID. let event = Event { timestamp: now(), author: alice(), action: Action::PatchCreate { title: "OID patch".to_string(), body: "".to_string(), base_ref: "main".to_string(), head_commit: commit_oid.to_string(), branch: None, }, }; let oid = dag::create_root_event(&repo, &event).unwrap(); let id = oid.to_string(); let patch_ref = format!("refs/collab/patches/{}", id); repo.reference(&patch_ref, oid, false, "test oid patch").unwrap(); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); let resolved = state.resolve_head(&repo).unwrap(); assert_eq!(resolved, commit_oid); } #[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_is_branch_based() { 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(); // Branch-based patch let (patch_ref, id) = create_branch_patch(&repo, &alice(), "Branch patch", "feat", "main"); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); assert!(state.is_branch_based()); // OID-based patch let (patch_ref2, id2) = create_patch(&repo, &alice(), "OID patch"); let state2 = PatchState::from_ref(&repo, &patch_ref2, &id2).unwrap(); assert!(!state2.is_branch_based()); } #[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 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").unwrap(); let patch_ref = format!("refs/collab/patches/{}", id); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); assert!(state.is_branch_based()); assert_eq!(state.branch, Some("feature/foo".to_string())); // head_commit should be set to the branch tip at creation time let branch_tip = repo.refname_to_id("refs/heads/feature/foo").unwrap(); assert_eq!(state.head_commit, branch_tip.to_string()); } #[test] fn test_create_duplicate_patch_for_same_branch_returns_error() { // T011: creating a duplicate patch for same branch returns 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("feature/dup", &repo.find_commit(tip).unwrap(), false).unwrap(); // First creation should succeed patch::create(&repo, "First", "", "main", "feature/dup").unwrap(); // Second creation for same branch should fail let result = patch::create(&repo, "Second", "", "main", "feature/dup"); 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"); 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); } // --------------------------------------------------------------------------- // Phase 5: US3 — Merge (T024-T025) // --------------------------------------------------------------------------- #[test] fn test_merge_branch_based_patch() { // T024: merging a branch-based patch performs git merge and updates DAG 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"feature code"); let id = patch::create(&repo, "Merge test", "", "main", "feat").unwrap(); // Merge the patch patch::merge(&repo, &id).unwrap(); // Check that the patch is now merged let patch_ref = format!("refs/collab/patches/{}", id); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); assert_eq!(state.status, PatchStatus::Merged); // Check that the base branch now has the feature commit let base_tip = repo.refname_to_id("refs/heads/main").unwrap(); let feat_tip = repo.refname_to_id("refs/heads/feat").unwrap(); assert!( repo.graph_descendant_of(base_tip, feat_tip).unwrap() || base_tip == feat_tip, "base should contain the feature branch" ); } #[test] fn test_merge_conflicting_patch_reports_error() { // T025: merging a conflicting patch reports 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", "conflict.rs", b"original"); repo.branch("feat", &repo.find_commit(tip).unwrap(), false).unwrap(); // Both branches modify the same file differently add_commit_on_branch(&repo, "feat", "conflict.rs", b"feature version"); add_commit_on_branch(&repo, "main", "conflict.rs", b"main version"); let id = patch::create(&repo, "Conflict test", "", "main", "feat").unwrap(); let result = patch::merge(&repo, &id); assert!(result.is_err(), "conflicting merge should fail"); let err_msg = result.unwrap_err().to_string(); assert!(err_msg.contains("conflict"), "error should mention conflicts"); } // --------------------------------------------------------------------------- // 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"); } // --------------------------------------------------------------------------- // Phase 7: US5 — Backward Compatibility (T029-T031) // --------------------------------------------------------------------------- #[test] fn test_old_format_patch_without_branch_deserializes() { // T029: old-format patch without branch field works let tmp = TempDir::new().unwrap(); let repo = init_repo(tmp.path(), &alice()); let (patch_ref, id) = create_patch(&repo, &alice(), "Old format patch"); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); assert!(!state.is_branch_based()); assert_eq!(state.branch, None); assert_eq!(state.head_commit, "abc123"); } #[test] fn test_old_format_patch_diff_uses_stored_head() { // T030: old-format patch diff uses stored head_commit let tmp = TempDir::new().unwrap(); let repo = init_repo(tmp.path(), &alice()); make_initial_commit(&repo, "main"); let commit_oid = add_commit_on_branch(&repo, "main", "file.rs", b"content"); // Create OID-based patch with real commit let event = Event { timestamp: now(), author: alice(), action: Action::PatchCreate { title: "OID diff test".to_string(), body: "".to_string(), base_ref: "main".to_string(), head_commit: commit_oid.to_string(), branch: None, }, }; let oid = dag::create_root_event(&repo, &event).unwrap(); let id = oid.to_string(); let patch_ref = format!("refs/collab/patches/{}", id); repo.reference(&patch_ref, oid, false, "test").unwrap(); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); let resolved = state.resolve_head(&repo).unwrap(); assert_eq!(resolved, commit_oid, "old-format should resolve to stored head_commit"); } #[test] fn test_old_format_patch_merge_uses_stored_head() { // T031: old-format patch merge uses stored head_commit let tmp = TempDir::new().unwrap(); let repo = init_repo(tmp.path(), &alice()); make_initial_commit(&repo, "main"); let commit_oid = add_commit_on_branch(&repo, "main", "file.rs", b"content"); // Create a branch for the head commit so we can merge it repo.branch("old-feat", &repo.find_commit(commit_oid).unwrap(), false).unwrap(); add_commit_on_branch(&repo, "old-feat", "old.rs", b"old feature"); let feat_tip = repo.refname_to_id("refs/heads/old-feat").unwrap(); // Create OID-based patch with the feature tip let event = Event { timestamp: now(), author: alice(), action: Action::PatchCreate { title: "Old merge test".to_string(), body: "".to_string(), base_ref: "main".to_string(), head_commit: feat_tip.to_string(), branch: None, }, }; let oid = dag::create_root_event(&repo, &event).unwrap(); let id = oid.to_string(); let patch_ref = format!("refs/collab/patches/{}", id); repo.reference(&patch_ref, oid, false, "test").unwrap(); // Merge should succeed using the stored OID patch::merge(&repo, &id[..8]).unwrap(); let state = PatchState::from_ref(&repo, &patch_ref, &id).unwrap(); assert_eq!(state.status, PatchStatus::Merged); } diff --git a/tests/sync_test.rs b/tests/sync_test.rs index 320613b..de04adf 100644 --- a/tests/sync_test.rs +++ b/tests/sync_test.rs @@ -371,6 +371,7 @@ fn test_patch_review_across_repos() { body: "Please review".to_string(), base_ref: "main".to_string(), head_commit: "abc123".to_string(), branch: None, }, }; let oid = dag::create_root_event(&alice_repo, &event).unwrap(); @@ -422,6 +423,7 @@ fn test_concurrent_review_and_revise() { body: "".to_string(), base_ref: "main".to_string(), head_commit: "v1".to_string(), branch: None, }, }; let oid = dag::create_root_event(&alice_repo, &event).unwrap();