d14b85c4
Remove defunct code: head_commit, patch import, backward compat
a73x 2026-03-21 11:27
No release yet so no backward compatibility needed. Patches are branches now — simplify everything: - Remove head_commit from PatchCreate/PatchRevise events and PatchState - Make branch a required String (not Option) - Remove patch import command and all import infrastructure - Remove --head flag from create and revise - Remove is_branch_based() — always branch-based - Always use three-dot (merge-base) diff - Simplify revise to just record notes - Remove MalformedPatch/PatchApplyFailed error variants - Delete tests/patch_import_test.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/src/cli.rs b/src/cli.rs index 5acc21e..202a6fd 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -168,9 +168,6 @@ pub enum PatchCmd { /// Source branch (defaults to current branch) #[arg(short = 'B', long)] branch: Option<String>, /// Deprecated: use --branch instead #[arg(long, hide = true)] head: Option<String>, /// Issue ID this patch fixes (auto-closes on merge) #[arg(long)] fixes: Option<String>, @@ -216,13 +213,10 @@ pub enum PatchCmd { #[arg(short, long)] body: String, }, /// Revise a patch with a new head commit /// Revise a patch (record a revision note) Revise { /// Patch ID (prefix match) id: String, /// New head commit (defaults to HEAD) #[arg(long)] head: Option<String>, /// Updated description #[arg(short, long)] body: Option<String>, @@ -240,9 +234,4 @@ pub enum PatchCmd { #[arg(short, long)] reason: Option<String>, }, /// Import patches from format-patch files Import { /// One or more .patch files to import files: Vec<std::path::PathBuf>, }, } diff --git a/src/error.rs b/src/error.rs index f0ccc73..2295ef2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -25,10 +25,4 @@ pub enum Error { #[error("untrusted key: {0}")] UntrustedKey(String), #[error("malformed patch: {0}")] MalformedPatch(String), #[error("patch apply failed: {0}")] PatchApplyFailed(String), } diff --git a/src/event.rs b/src/event.rs index 609f5b2..c320717 100644 --- a/src/event.rs +++ b/src/event.rs @@ -47,15 +47,12 @@ pub enum Action { title: String, body: String, base_ref: String, head_commit: String, branch: String, #[serde(default, skip_serializing_if = "Option::is_none")] fixes: Option<String>, #[serde(default, skip_serializing_if = "Option::is_none")] branch: Option<String>, }, PatchRevise { body: Option<String>, head_commit: String, }, PatchReview { verdict: ReviewVerdict, diff --git a/src/lib.rs b/src/lib.rs index 59269cf..56dd0d2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -132,32 +132,11 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> { body, base, branch, head, fixes, } => { // Resolve branch name: --branch takes priority, then --head (deprecated), then current branch // Resolve branch name: --branch takes priority, 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(|_| { @@ -218,21 +197,17 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> { println!("Patch {} [{}]", &p.id[..8], status); println!("Title: {}", p.title); println!("Author: {} <{}>", p.author.name, p.author.email); 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); match p.resolve_head(repo) { Ok(_) => { println!("Branch: {} -> {}", p.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); } } } else { println!("Base: {} Head: {:.8}", p.base_ref, p.head_commit); Err(_) => { println!("Branch: {} (not found)", p.branch); } } println!("Created: {}", p.created_at); if let Some(ref fixes) = p.fixes { @@ -302,12 +277,8 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> { println!("Review submitted."); Ok(()) } PatchCmd::Revise { id, head, body } => { let head = match head { Some(h) => h, None => repo.head()?.peel_to_commit()?.id().to_string(), }; patch::revise(repo, &id, &head, body.as_deref())?; PatchCmd::Revise { id, body } => { patch::revise(repo, &id, body.as_deref())?; println!("Patch revised."); Ok(()) } @@ -321,13 +292,6 @@ pub fn run(cli: cli::Cli, repo: &Repository) -> Result<(), error::Error> { println!("Patch closed."); Ok(()) } PatchCmd::Import { files } => { let ids = patch::import_series(repo, &files)?; for id in &ids { println!("Imported patch {:.8}", id); } Ok(()) } }, Commands::Dashboard => tui::run(repo), Commands::Sync { remote } => sync::sync(repo, &remote), diff --git a/src/patch.rs b/src/patch.rs index d3ee8e8..cfae1b3 100644 --- a/src/patch.rs +++ b/src/patch.rs @@ -1,6 +1,4 @@ use std::path::Path; use git2::{Diff, DiffFormat, Repository}; use git2::{DiffFormat, Repository}; use crate::dag; use crate::error::Error; @@ -24,24 +22,19 @@ pub fn create( )); } // Resolve branch to its tip OID // Verify branch exists let branch_ref = format!("refs/heads/{}", branch); let tip_oid = repo .refname_to_id(&branch_ref) 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 ))); } } if p.status == PatchStatus::Open && p.branch == branch { return Err(crate::error::Error::Cmd(format!( "patch already exists for branch '{}'", branch ))); } } @@ -54,8 +47,7 @@ pub fn create( title: title.to_string(), body: body.to_string(), base_ref: base_ref.to_string(), head_commit: tip_oid.to_string(), branch: Some(branch.to_string()), branch: branch.to_string(), fixes: fixes.map(|s| s.to_string()), }, }; @@ -141,7 +133,6 @@ pub fn review( pub fn revise( repo: &Repository, id_prefix: &str, head_commit: &str, body: Option<&str>, ) -> Result<(), crate::error::Error> { let sk = signing::load_signing_key(&signing::signing_key_dir()?)?; @@ -152,7 +143,6 @@ pub fn revise( author, action: Action::PatchRevise { body: body.map(|s| s.to_string()), head_commit: head_commit.to_string(), }, }; dag::append_event(repo, &ref_name, &event, &sk)?; @@ -244,20 +234,22 @@ pub fn diff(repo: &Repository, id_prefix: &str) -> Result<String, Error> { generate_diff(repo, &p) } /// Generate a diff string from a patch's base and head. pub fn generate_diff(repo: &Repository, patch: &PatchState) -> Result<String, Error> { let head_obj = repo .revparse_single(&patch.head_commit) /// Generate a diff string from a patch's base and head using three-dot (merge-base) diff. pub fn generate_diff(repo: &Repository, patch: &state::PatchState) -> Result<String, Error> { 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); 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 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 { None }; @@ -310,191 +302,3 @@ pub fn close( Ok(()) } // --------------------------------------------------------------------------- // Patch import from format-patch files // --------------------------------------------------------------------------- /// Parsed metadata from a git format-patch mbox header. struct PatchHeader { subject: String, body: String, } /// Parse a format-patch file into its mbox header metadata and the raw diff portion. fn parse_format_patch(content: &str) -> Result<(PatchHeader, String), Error> { // Find the "---" separator that divides the commit message from the diffstat/diff. // The diff starts at the first line matching "diff --git". let diff_start = content .find("\ndiff --git ") .map(|i| i + 1) // skip the leading newline .ok_or_else(|| Error::MalformedPatch("no 'diff --git' found in patch file".to_string()))?; let header_section = &content[..diff_start]; let diff_section = &content[diff_start..]; // Extract Subject line let subject_line = header_section .lines() .find(|l| l.starts_with("Subject:")) .ok_or_else(|| Error::MalformedPatch("no Subject header found".to_string()))?; // Strip "Subject: " prefix and optional "[PATCH] " or "[PATCH n/m] " prefix let subject = subject_line .strip_prefix("Subject:") .unwrap() .trim(); let subject = if let Some(rest) = subject.strip_prefix("[PATCH") { // Skip to the "] " closing bracket if let Some(idx) = rest.find("] ") { rest[idx + 2..].to_string() } else { subject.to_string() } } else { subject.to_string() }; // Extract body: everything between the blank line after headers and the "---" separator let body = extract_body(header_section); // Trim trailing "-- \n2.xx.x\n" signature from diff let diff_clean = trim_patch_signature(diff_section); Ok((PatchHeader { subject, body }, diff_clean)) } /// Extract the commit message body from the header section. /// The body is between the first blank line after headers and the "---" line. fn extract_body(header_section: &str) -> String { let lines: Vec<&str> = header_section.lines().collect(); let mut body_start = None; let mut body_end = None; // Find first blank line (end of mail headers) for (i, line) in lines.iter().enumerate() { if line.is_empty() && body_start.is_none() { body_start = Some(i + 1); } } // Find the "---" separator line (start of diffstat) for (i, line) in lines.iter().enumerate().rev() { if *line == "---" { body_end = Some(i); break; } } match (body_start, body_end) { (Some(start), Some(end)) if start < end => { lines[start..end].join("\n").trim().to_string() } (Some(start), None) => { // No "---" separator, take everything after headers lines[start..].join("\n").trim().to_string() } _ => String::new(), } } /// Remove trailing git patch signature ("-- \n2.xx.x\n") from diff content. fn trim_patch_signature(diff: &str) -> String { if let Some(idx) = diff.rfind("\n-- \n") { diff[..idx + 1].to_string() // keep the trailing newline } else { diff.to_string() } } /// Import a single format-patch file, creating a commit and DAG entry. /// Returns the patch ID. pub fn import(repo: &Repository, patch_path: &Path) -> Result<String, Error> { let content = std::fs::read_to_string(patch_path)?; let (header, diff_text) = parse_format_patch(&content)?; // Parse the diff with git2 let diff = Diff::from_buffer(diff_text.as_bytes()) .map_err(|e| Error::MalformedPatch(format!("invalid diff: {}", e)))?; // Get the base (HEAD) commit and its tree let head_ref = repo .head() .map_err(|e| Error::PatchApplyFailed(format!("cannot resolve HEAD: {}", e)))?; let head_commit = head_ref .peel_to_commit() .map_err(|e| Error::PatchApplyFailed(format!("HEAD is not a commit: {}", e)))?; let base_tree = head_commit.tree()?; // Apply the diff to the base tree in-memory let new_index = repo .apply_to_tree(&base_tree, &diff, None) .map_err(|e| Error::PatchApplyFailed(format!("apply failed: {}", e)))?; // Write the index to a tree let tree_oid = { let mut idx = new_index; idx.write_tree_to(repo)? }; let new_tree = repo.find_tree(tree_oid)?; // Determine the base branch name from HEAD let base_ref = repo .head()? .shorthand() .unwrap_or("main") .to_string(); // Create a commit on a new branch for the import let author = get_author(repo)?; let sig = crate::identity::author_signature(&author)?; let commit_msg = format!("imported: {}", header.subject); let short_id = &header.subject.replace(|c: char| !c.is_alphanumeric(), "-"); let import_branch = format!("collab/import/{}", &short_id[..short_id.len().min(30)]); let branch_ref = format!("refs/heads/{}", import_branch); let commit_oid = repo.commit( Some(&branch_ref), &sig, &sig, &commit_msg, &new_tree, &[&head_commit], )?; // Create a DAG entry using the existing patch create infrastructure let id = create( repo, &header.subject, &header.body, &base_ref, &import_branch, None, )?; // Suppress unused variable warning let _ = commit_oid; Ok(id) } /// Import a series of format-patch files. If any fails, rolls back all /// previously imported patches from this series. pub fn import_series(repo: &Repository, files: &[impl AsRef<Path>]) -> Result<Vec<String>, Error> { let mut imported_ids: Vec<String> = Vec::new(); for file in files { match import(repo, file.as_ref()) { Ok(id) => imported_ids.push(id), Err(e) => { // Rollback: delete all refs created in this series for id in &imported_ids { let ref_name = format!("refs/collab/patches/{}", id); if let Ok(mut reference) = repo.find_reference(&ref_name) { let _ = reference.delete(); } } return Err(e); } } } Ok(imported_ids) } diff --git a/src/state.rs b/src/state.rs index 161fdc1..261d8b6 100644 --- a/src/state.rs +++ b/src/state.rs @@ -65,9 +65,8 @@ pub struct PatchState { pub body: String, pub status: PatchStatus, pub base_ref: String, pub head_commit: String, pub fixes: Option<String>, pub branch: Option<String>, pub branch: String, pub comments: Vec<Comment>, pub inline_comments: Vec<InlineComment>, pub reviews: Vec<Review>, @@ -180,33 +179,15 @@ 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`. /// Resolve the current head commit OID for this patch by looking up `refs/heads/{branch}`. 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()) } let ref_name = format!("refs/heads/{}", self.branch); repo.refname_to_id(&ref_name).map_err(|e| { crate::error::Error::Cmd(format!( "branch '{}' not found: {}", self.branch, e )) }) } /// Compute staleness: how many commits the branch is ahead of base, @@ -236,9 +217,8 @@ impl PatchState { title, body, base_ref, head_commit, fixes, branch, fixes, } => { state = Some(PatchState { id: id.to_string(), @@ -246,7 +226,6 @@ impl PatchState { body, status: PatchStatus::Open, base_ref, head_commit, fixes, branch, comments: Vec::new(), @@ -256,9 +235,8 @@ impl PatchState { author: event.author.clone(), }); } Action::PatchRevise { body, head_commit } => { Action::PatchRevise { body } => { if let Some(ref mut s) = state { s.head_commit = head_commit; if let Some(b) = body { s.body = b; } diff --git a/src/tui.rs b/src/tui.rs index b55c2fd..930b3da 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -78,7 +78,6 @@ enum InputMode { /// Cached staleness info for a patch. #[derive(Clone)] struct PatchBranchInfo { branch: Option<String>, staleness: Option<(usize, usize)>, branch_exists: bool, } @@ -500,18 +499,10 @@ fn generate_diff(repo: &Repository, patch: &PatchState) -> String { 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) { 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()?) } 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()?) @@ -593,18 +584,17 @@ fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> String { title, body, base_ref, head_commit, branch, .. } => { detail.push_str(&format!("\nTitle: {}\n", title)); detail.push_str(&format!("Base: {}\n", base_ref)); detail.push_str(&format!("Head: {}\n", head_commit)); detail.push_str(&format!("\nTitle: {}\n", title)); detail.push_str(&format!("Base: {}\n", base_ref)); detail.push_str(&format!("Branch: {}\n", branch)); if !body.is_empty() { detail.push_str(&format!("\n{}\n", body)); } } Action::PatchRevise { body, head_commit } => { detail.push_str(&format!("\nHead: {}\n", head_commit)); Action::PatchRevise { body } => { if let Some(b) = body { if !b.is_empty() { detail.push_str(&format!("\n{}\n", b)); @@ -686,7 +676,7 @@ fn run_loop( 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() { Some({ let branch_exists = patch.resolve_head(repo).is_ok(); let staleness = if branch_exists { patch.staleness(repo).ok() @@ -694,16 +684,9 @@ fn run_loop( None }; PatchBranchInfo { branch: patch.branch.clone(), staleness, branch_exists, } } else { PatchBranchInfo { branch: None, staleness: None, branch_exists: false, } }) } else { None @@ -871,10 +854,10 @@ fn run_loop( app.list_state .selected() .and_then(|idx| visible.get(idx)) .map(|p| p.head_commit.clone()) .map(|p| p.branch.clone()) } Tab::Issues => { // Find linked patch's head commit, or fall back to closing commit // Find linked patch's branch, or fall back to closing commit let visible = app.visible_issues(); app.list_state .selected() @@ -884,7 +867,7 @@ fn run_loop( app.patches .iter() .find(|p| p.fixes.as_deref() == Some(&issue.id)) .map(|p| p.head_commit.clone()) .map(|p| p.branch.clone()) // Fall back to closing commit .or_else(|| { issue.closed_by.map(|oid| oid.to_string()) @@ -1348,46 +1331,38 @@ fn build_patch_detail(patch: &PatchState, branch_info: Option<&PatchBranchInfo>) ]), ]; // 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 { if info.branch_exists { lines.push(Line::from(vec![ Span::styled("Branch: ", Style::default().fg(Color::DarkGray)), Span::styled( patch.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("Branch: ", Style::default().fg(Color::DarkGray)), Span::styled( branch.clone(), Style::default().fg(Color::Red), ), Span::styled(" (not found)", Style::default().fg(Color::Red)), 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( patch.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(" "), Span::styled("Head: ", Style::default().fg(Color::DarkGray)), Span::styled( format!("{:.8}", patch.head_commit), Style::default().fg(Color::Cyan), ), Span::styled("Branch: ", Style::default().fg(Color::DarkGray)), Span::raw(patch.branch.clone()), Span::raw(format!(" -> {}", patch.base_ref)), ])); } @@ -1738,9 +1713,8 @@ mod tests { body: String::new(), status, base_ref: "main".into(), head_commit: "abc123".into(), fixes: None, branch: None, branch: format!("feature/{}", id), comments: vec![], inline_comments: vec![], reviews: vec![], @@ -1956,9 +1930,8 @@ mod tests { PatchStatus::Closed }, base_ref: "main".to_string(), head_commit: format!("h{:07x}", i), fixes: None, branch: None, branch: format!("feature/p{:07x}", i), comments: Vec::new(), inline_comments: Vec::new(), reviews: Vec::new(), @@ -2082,9 +2055,8 @@ mod tests { title: "t".to_string(), body: "b".to_string(), base_ref: "main".to_string(), head_commit: "abc".to_string(), branch: "feature/test".to_string(), fixes: None, branch: None, }; assert_eq!(action_type_label(&action), "Patch Create"); } diff --git a/tests/cli_test.rs b/tests/cli_test.rs index c9c1af1..497ed3f 100644 --- a/tests/cli_test.rs +++ b/tests/cli_test.rs @@ -433,15 +433,13 @@ fn test_patch_revise() { // Switch to the patch's branch and add a new commit repo.git(&["checkout", "test/wip-feature"]); let new_head = repo.commit_file("v2.txt", "v2", "version 2"); repo.commit_file("v2.txt", "v2", "version 2"); repo.git(&["checkout", "main"]); let out = repo.run_ok(&[ "patch", "revise", &id, "--head", &new_head, "-b", "Updated implementation", ]); @@ -464,10 +462,9 @@ fn test_patch_diff() { "fn main() {\n println!(\"hello\");\n}\n", "add hello", ); let head = repo.git(&["rev-parse", "HEAD"]).trim().to_string(); repo.git(&["checkout", "main"]); let out = repo.run_ok(&["patch", "create", "-t", "Add hello", "--head", &head]); let out = repo.run_ok(&["patch", "create", "-t", "Add hello", "-B", "feature"]); let id = out.trim().strip_prefix("Created patch ").unwrap(); let out = repo.run_ok(&["patch", "diff", id]); @@ -508,11 +505,11 @@ fn test_patch_merge_fast_forward() { // Create a feature branch ahead of main repo.git(&["checkout", "-b", "feature"]); let head = repo.commit_file("feature.txt", "new feature", "add feature"); repo.commit_file("feature.txt", "new feature", "add feature"); repo.git(&["checkout", "main"]); // Create patch pointing at the feature commit let out = repo.run_ok(&["patch", "create", "-t", "Add feature", "--head", &head]); // Create patch pointing at the feature branch let out = repo.run_ok(&["patch", "create", "-t", "Add feature", "-B", "feature"]); let id = out.trim().strip_prefix("Created patch ").unwrap(); let out = repo.run_ok(&["patch", "merge", id]); @@ -520,7 +517,8 @@ fn test_patch_merge_fast_forward() { // Main should now point at the feature commit let main_head = repo.git(&["rev-parse", "main"]).trim().to_string(); assert_eq!(main_head, head); let feature_head = repo.git(&["rev-parse", "feature"]).trim().to_string(); assert_eq!(main_head, feature_head); let out = repo.run_ok(&["patch", "show", id]); assert!(out.contains("[merged]")); @@ -531,10 +529,10 @@ fn test_patch_cannot_merge_closed() { let repo = TestRepo::new("Alice", "alice@example.com"); repo.git(&["checkout", "-b", "feature"]); let head = repo.commit_file("f.txt", "f", "feature commit"); repo.commit_file("f.txt", "f", "feature commit"); repo.git(&["checkout", "main"]); let out = repo.run_ok(&["patch", "create", "-t", "Will close", "--head", &head]); let out = repo.run_ok(&["patch", "create", "-t", "Will close", "-B", "feature"]); let id = out.trim().strip_prefix("Created patch ").unwrap(); repo.run_ok(&["patch", "close", id]); @@ -562,13 +560,13 @@ fn test_patch_create_with_fixes() { let issue_id = repo.issue_open("Login bug"); repo.git(&["checkout", "-b", "fix"]); let head = repo.commit_file("fix.rs", "fixed", "fix login"); repo.commit_file("fix.rs", "fixed", "fix login"); repo.git(&["checkout", "main"]); let out = repo.run_ok(&[ "patch", "create", "-t", "Fix login bug", "--head", &head, "-B", "fix", "--fixes", &issue_id, ]); let patch_id = out.trim().strip_prefix("Created patch ").unwrap(); @@ -586,13 +584,13 @@ fn test_patch_merge_auto_closes_linked_issue() { let issue_id = repo.issue_open("Crash on startup"); repo.git(&["checkout", "-b", "fix"]); let head = repo.commit_file("fix.rs", "fixed", "fix crash"); repo.commit_file("fix.rs", "fixed", "fix crash"); repo.git(&["checkout", "main"]); let out = repo.run_ok(&[ "patch", "create", "-t", "Fix crash", "--head", &head, "-B", "fix", "--fixes", &issue_id, ]); let patch_id = out.trim().strip_prefix("Created patch ").unwrap(); @@ -717,10 +715,10 @@ fn test_full_patch_review_cycle() { // Create feature branch with v1 repo.git(&["checkout", "-b", "feature"]); let v1 = repo.commit_file("feature.rs", "fn hello() {}", "v1 of feature"); repo.commit_file("feature.rs", "fn hello() {}", "v1 of feature"); repo.git(&["checkout", "main"]); let out = repo.run_ok(&["patch", "create", "-t", "Add hello function", "--head", &v1]); let out = repo.run_ok(&["patch", "create", "-t", "Add hello function", "-B", "feature"]); let id = out .trim() .strip_prefix("Created patch ") @@ -756,7 +754,7 @@ fn test_full_patch_review_cycle() { // Revise with updated code repo.git(&["checkout", "feature"]); let v2 = repo.commit_file( repo.commit_file( "feature.rs", "/// Says hello\nfn hello() {}", "v2: add docs", @@ -767,8 +765,6 @@ fn test_full_patch_review_cycle() { "patch", "revise", &id, "--head", &v2, "-b", "Added documentation", ]); diff --git a/tests/collab_test.rs b/tests/collab_test.rs index 0e51acd..48a1868 100644 --- a/tests/collab_test.rs +++ b/tests/collab_test.rs @@ -328,7 +328,6 @@ fn test_patch_review_workflow() { author: alice(), action: Action::PatchRevise { body: Some("Updated implementation".to_string()), head_commit: "def456".to_string(), }, }; dag::append_event(&repo, &ref_name, &event, &sk).unwrap(); @@ -337,7 +336,6 @@ fn test_patch_review_workflow() { let state = PatchState::from_ref(&repo, &ref_name, &id).unwrap(); assert_eq!(state.reviews.len(), 2); assert_eq!(state.head_commit, "def456"); assert_eq!(state.body, "Updated implementation"); } @@ -601,8 +599,6 @@ fn create_branch_patch( base_ref: &str, ) -> (String, String) { let sk = test_signing_key(); let ref_name = format!("refs/heads/{}", branch); let tip = repo.refname_to_id(&ref_name).unwrap(); let event = Event { timestamp: now(), author: author.clone(), @@ -610,8 +606,7 @@ fn create_branch_patch( title: title.to_string(), body: "".to_string(), base_ref: base_ref.to_string(), head_commit: tip.to_string(), branch: Some(branch.to_string()), branch: branch.to_string(), fixes: None, }, }; @@ -651,41 +646,6 @@ fn test_resolve_head_branch_based() { } #[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 sk = test_signing_key(); 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, fixes: None, }, }; 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 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()); @@ -706,26 +666,6 @@ fn test_resolve_head_deleted_branch_error() { } #[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()); @@ -786,11 +726,10 @@ fn test_create_patch_from_branch_populates_branch_field() { 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 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.head_commit, branch_tip.to_string()); assert_eq!(state.resolve_head(&repo).unwrap(), branch_tip); } #[test] @@ -949,91 +888,3 @@ fn test_branch_push_auto_reflects_in_patch() { 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 sk = test_signing_key(); 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, fixes: None, }, }; 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").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 sk = test_signing_key(); 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, fixes: None, }, }; 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").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/common/mod.rs b/tests/common/mod.rs index b827a36..dc34a98 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -119,9 +119,8 @@ pub fn create_patch(repo: &Repository, author: &Author, title: &str) -> (String, title: title.to_string(), body: "".to_string(), base_ref: "main".to_string(), head_commit: "abc123".to_string(), branch: "test-branch".to_string(), fixes: None, branch: None, }, }; let oid = dag::create_root_event(repo, &event, &sk).unwrap(); diff --git a/tests/patch_import_test.rs b/tests/patch_import_test.rs deleted file mode 100644 index 0dafb6d..0000000 --- a/tests/patch_import_test.rs +++ /dev/null @@ -1,298 +0,0 @@ use git2::Repository; use std::path::{Path, PathBuf}; use tempfile::TempDir; use git_collab::event::Author; use git_collab::patch; use git_collab::state::{self, PatchStatus}; // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- fn alice() -> Author { Author { name: "Alice".to_string(), email: "alice@example.com".to_string(), } } /// Create a repo with an initial commit so we have a valid HEAD and tree. fn init_repo_with_commit(dir: &Path, author: &Author) -> Repository { let repo = Repository::init(dir).expect("init repo"); { let mut config = repo.config().unwrap(); config.set_str("user.name", &author.name).unwrap(); config.set_str("user.email", &author.email).unwrap(); } // Create an initial commit with a file so we have a valid tree/HEAD let sig = git2::Signature::now(&author.name, &author.email).unwrap(); let tree_oid = { let blob_oid = repo.blob(b"initial content\n").unwrap(); let mut tb = repo.treebuilder(None).unwrap(); tb.insert("README", blob_oid, 0o100644).unwrap(); tb.write().unwrap() }; { let tree = repo.find_tree(tree_oid).unwrap(); repo.commit(Some("refs/heads/main"), &sig, &sig, "Initial commit", &tree, &[]) .unwrap(); } // Set HEAD to point to main repo.set_head("refs/heads/main").unwrap(); repo } /// Generate a valid git format-patch style .patch file content. /// This creates a patch that adds a new file called `filename` with `content`. fn make_format_patch( from_name: &str, from_email: &str, subject: &str, body: &str, filename: &str, content: &str, ) -> String { let date = "Thu, 19 Mar 2026 10:30:00 +0000"; // Build the diff portion let lines: Vec<&str> = content.lines().collect(); let mut diff_lines = String::new(); for line in &lines { diff_lines.push_str(&format!("+{}\n", line)); } let line_count = lines.len(); format!( "From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001\n\ From: {} <{}>\n\ Date: {}\n\ Subject: [PATCH] {}\n\ \n\ {}\n\ ---\n\ {filename} | {line_count} +\n\ 1 file changed, {line_count} insertions(+)\n\ create mode 100644 {filename}\n\ \n\ diff --git a/{filename} b/{filename}\n\ new file mode 100644\n\ index 0000000..1234567\n\ --- /dev/null\n\ +++ b/{filename}\n\ @@ -0,0 +1,{line_count} @@\n\ {diff_lines}\ -- \n\ 2.40.0\n", from_name, from_email, date, subject, body, filename = filename, line_count = line_count, diff_lines = diff_lines, ) } /// Write patch content to a file and return its path. fn write_patch_file(dir: &Path, name: &str, content: &str) -> PathBuf { let path = dir.join(name); std::fs::write(&path, content).unwrap(); path } // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- #[test] fn test_import_single_patch_success() { let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); let patch_content = make_format_patch( "Bob", "bob@example.com", "Add hello.txt", "This patch adds a hello file.", "hello.txt", "Hello, world!\n", ); let patch_dir = TempDir::new().unwrap(); let patch_file = write_patch_file(patch_dir.path(), "0001-add-hello.patch", &patch_content); let id = patch::import(&repo, &patch_file).unwrap(); // Verify the patch was created in the DAG let ref_name = format!("refs/collab/patches/{}", id); let patch_state = state::PatchState::from_ref(&repo, &ref_name, &id).unwrap(); assert_eq!(patch_state.title, "Add hello.txt"); assert_eq!(patch_state.status, PatchStatus::Open); assert_eq!(patch_state.author.name, "Alice"); // importer is the author in the DAG assert!(!patch_state.head_commit.is_empty()); assert_eq!(patch_state.base_ref, "main"); } #[test] fn test_import_file_not_found() { let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); let nonexistent = PathBuf::from("/tmp/does-not-exist-12345.patch"); let result = patch::import(&repo, &nonexistent); assert!(result.is_err()); } #[test] fn test_import_malformed_patch() { let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); let patch_dir = TempDir::new().unwrap(); let patch_file = write_patch_file( patch_dir.path(), "bad.patch", "This is not a valid patch file at all.\nJust random text.\n", ); let result = patch::import(&repo, &patch_file); assert!(result.is_err()); } #[test] fn test_import_creates_dag_entry_readable_by_show() { let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); let patch_content = make_format_patch( "Charlie", "charlie@example.com", "Fix bug in parser", "Fixes an off-by-one error in the parser module.", "parser.txt", "fixed parser code\n", ); let patch_dir = TempDir::new().unwrap(); let patch_file = write_patch_file(patch_dir.path(), "0001-fix-bug.patch", &patch_content); let id = patch::import(&repo, &patch_file).unwrap(); // Verify it can be resolved and read back through state infrastructure let (ref_name, resolved_id) = state::resolve_patch_ref(&repo, &id[..8]).unwrap(); assert_eq!(resolved_id, id); let patch_state = state::PatchState::from_ref(&repo, &ref_name, &resolved_id).unwrap(); assert_eq!(patch_state.title, "Fix bug in parser"); assert!( patch_state.body.contains("off-by-one"), "body should contain the patch description" ); } #[test] fn test_import_series_multiple_files() { let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); let patch1 = make_format_patch( "Bob", "bob@example.com", "Add file one", "First patch in series.", "one.txt", "one\n", ); let patch2 = make_format_patch( "Bob", "bob@example.com", "Add file two", "Second patch in series.", "two.txt", "two\n", ); let patch_dir = TempDir::new().unwrap(); let f1 = write_patch_file(patch_dir.path(), "0001-add-one.patch", &patch1); let f2 = write_patch_file(patch_dir.path(), "0002-add-two.patch", &patch2); let ids = patch::import_series(&repo, &[f1, f2]).unwrap(); assert_eq!(ids.len(), 2); // Both should be valid patches in the DAG for id in &ids { let (ref_name, _) = state::resolve_patch_ref(&repo, &id[..8]).unwrap(); let ps = state::PatchState::from_ref(&repo, &ref_name, id).unwrap(); assert_eq!(ps.status, PatchStatus::Open); } } #[test] fn test_import_series_rollback_on_failure() { let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); let good_patch = make_format_patch( "Bob", "bob@example.com", "Add good file", "A good patch.", "good.txt", "good\n", ); let patch_dir = TempDir::new().unwrap(); let f1 = write_patch_file(patch_dir.path(), "0001-good.patch", &good_patch); let f2 = PathBuf::from("/tmp/nonexistent-bad-patch-12345.patch"); let result = patch::import_series(&repo, &[f1, f2]); assert!(result.is_err()); // After rollback, no patches should exist let patches = state::list_patches(&repo).unwrap(); assert_eq!(patches.len(), 0, "rollback should remove all imported patches"); } #[test] fn test_import_patch_with_modification() { // Test importing a patch that modifies an existing file (not just new files) let tmp = TempDir::new().unwrap(); let repo = init_repo_with_commit(tmp.path(), &alice()); // Create a patch that modifies README (which exists in our initial commit) let date = "Thu, 19 Mar 2026 10:30:00 +0000"; let patch_content = format!( "From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001\n\ From: Bob <bob@example.com>\n\ Date: {}\n\ Subject: [PATCH] Update README\n\ \n\ Updated the README with more info.\n\ ---\n\ README | 2 +-\n\ 1 file changed, 1 insertion(+), 1 deletion(-)\n\ \n\ diff --git a/README b/README\n\ index 1234567..abcdef0 100644\n\ --- a/README\n\ +++ b/README\n\ @@ -1 +1 @@\n\ -initial content\n\ +updated content\n\ -- \n\ 2.40.0\n", date, ); let patch_dir = TempDir::new().unwrap(); let patch_file = write_patch_file(patch_dir.path(), "0001-update-readme.patch", &patch_content); let id = patch::import(&repo, &patch_file).unwrap(); let ref_name = format!("refs/collab/patches/{}", id); let ps = state::PatchState::from_ref(&repo, &ref_name, &id).unwrap(); assert_eq!(ps.title, "Update README"); assert_eq!(ps.status, PatchStatus::Open); } diff --git a/tests/signing_test.rs b/tests/signing_test.rs index abde304..04db8ce 100644 --- a/tests/signing_test.rs +++ b/tests/signing_test.rs @@ -211,9 +211,8 @@ fn signed_event_flatten_round_trip_with_tagged_enum() { title: "Fix bug".to_string(), body: "Fixes #42".to_string(), base_ref: "main".to_string(), head_commit: "abc123".to_string(), branch: "feature/fix-bug".to_string(), fixes: Some("deadbeef".to_string()), branch: None, }, }; diff --git a/tests/sync_test.rs b/tests/sync_test.rs index c668533..34e6eb0 100644 --- a/tests/sync_test.rs +++ b/tests/sync_test.rs @@ -283,9 +283,8 @@ fn test_patch_review_across_repos() { title: "Add feature X".to_string(), body: "Please review".to_string(), base_ref: "main".to_string(), head_commit: "abc123".to_string(), branch: "feature/x".to_string(), fixes: None, branch: None, }, }; let sk = test_signing_key(); @@ -331,9 +330,8 @@ fn test_concurrent_review_and_revise() { title: "WIP feature".to_string(), body: "".to_string(), base_ref: "main".to_string(), head_commit: "v1".to_string(), branch: "feature/wip".to_string(), fixes: None, branch: None, }, }; let sk = test_signing_key(); @@ -351,7 +349,6 @@ fn test_concurrent_review_and_revise() { author: alice(), action: Action::PatchRevise { body: Some("Updated description".to_string()), head_commit: "v2".to_string(), }, }; dag::append_event(&alice_repo, &alice_ref, &revise_event, &sk).unwrap(); @@ -376,7 +373,7 @@ fn test_concurrent_review_and_revise() { assert_eq!(alice_state.reviews.len(), 1); assert_eq!(bob_state.reviews.len(), 1); assert_eq!(alice_state.head_commit, bob_state.head_commit); assert_eq!(alice_state.body, bob_state.body); } #[test]