From 3cb70eef9d824d4213d7ff88d69521c0d86c578f Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Sun, 21 Jun 2026 10:05:13 -0700 Subject: [PATCH] Migrate worktree-tidy onto core run_pipeline scan seam worktree-tidy was the last tool still on the worktree-specific core::types::{RepoGroup, ScanResult} that PR B's generics were modeled on. Route run_scan_repos through git_tidy_core::scan::run_pipeline with ScanOptions { fetch: true }, handing the per-repo git fetch --prune to the seam and dropping the tool's manual parallel_fetch, parallel_classify, empty-group check, and counts/total_scanned fold. The closure now returns the repo's Vec (empty for skipped repos) and the seam wraps non-empty groups, folds counts by Classified::classification_label, and sums total_scanned. Discovery and the --match basename filter stay in the thin run_scan, applied to the parent-repo map before the pipeline; the closure still captures that map and looks worktrees up by parent repo. Delete the now-dead non-generic RepoGroup, ScanResult, and the FlatJsonItems impl from core::types, and add the per-item glue next to JsonWorktree: Classified and IntoJsonItem impls on WorktreeInfo (both must live in core because WorktreeInfo is a core type, the orphan-rule inverse of branch-tidy). A WorktreeScanResult = ScanResult alias keeps a stable type name for the tool and the audit runner, preserving repos/total_scanned/counts/warnings so audit JSON keys are unchanged. Public output is byte-identical: flat --json array, 9-field porcelain (worktree dir then parent repo), and the LANDED_SUMMARY human line. Item ordering is preserved by the closure's sort_by_key on classification priority. Add a regression-guard test confirming a repo present in the map with a worktree but with failing default-branch detection yields no group and surfaces the warning. Part of #117 --- crates/git-tidy-core/src/types.rs | 46 ++++------ crates/git-worktree-tidy/src/clean.rs | 17 ++-- crates/git-worktree-tidy/src/output.rs | 32 +++---- crates/git-worktree-tidy/src/scan.rs | 88 +++++++++---------- .../tests/integration_git.rs | 2 +- 5 files changed, 85 insertions(+), 100 deletions(-) diff --git a/crates/git-tidy-core/src/types.rs b/crates/git-tidy-core/src/types.rs index e062ebe..e60fc87 100644 --- a/crates/git-tidy-core/src/types.rs +++ b/crates/git-tidy-core/src/types.rs @@ -2,7 +2,8 @@ use std::path::PathBuf; use serde::Serialize; -use crate::counts::Counts; +use crate::output::IntoJsonItem; +use crate::scan::Classified; /// Shared interface for classification enums across all git-tidy tools. pub trait ClassificationLabel { @@ -187,15 +188,10 @@ pub struct WorktreeInfo { pub meaningful_dirty_files: Vec, } -/// A group of worktrees sharing the same parent repo. -#[derive(Debug, Clone, Serialize)] -pub struct RepoGroup { - /// Path to the parent repo. - pub repo_path: PathBuf, - /// Display name (directory basename). - pub name: String, - /// Worktrees belonging to this repo, sorted by classification priority. - pub worktrees: Vec, +impl Classified for WorktreeInfo { + fn classification_label(&self) -> &str { + self.classification.label() + } } /// Flat JSON representation of a worktree matching the spec. @@ -244,31 +240,21 @@ impl From<&WorktreeInfo> for JsonWorktree { } } -/// Result of a full scan operation. -#[derive(Debug, Clone, Serialize)] -pub struct ScanResult { - /// Worktrees grouped by parent repo. - pub repos: Vec, - /// Total worktrees scanned. - pub total_scanned: usize, - /// Summary counts by classification. - pub counts: Counts, - /// Repos that were skipped (e.g. no default branch). - pub warnings: Vec, -} - -impl crate::output::FlatJsonItems for ScanResult { +impl IntoJsonItem for WorktreeInfo { type JsonItem = JsonWorktree; - fn to_json_items(&self) -> Vec { - self.repos - .iter() - .flat_map(|g| g.worktrees.iter()) - .map(JsonWorktree::from) - .collect() + fn to_json_item(&self) -> JsonWorktree { + JsonWorktree::from(self) } } +/// Result of a full worktree scan. +/// +/// Worktrees are grouped per parent repo in `repos` as `RepoGroup` +/// (the generic core group type); each group's `items` are sorted by +/// classification priority. +pub type WorktreeScanResult = crate::scan::ScanResult; + #[cfg(test)] mod tests { use super::*; diff --git a/crates/git-worktree-tidy/src/clean.rs b/crates/git-worktree-tidy/src/clean.rs index 0b6b00a..cfceb1c 100644 --- a/crates/git-worktree-tidy/src/clean.rs +++ b/crates/git-worktree-tidy/src/clean.rs @@ -3,7 +3,9 @@ use std::path::PathBuf; use git_tidy_core::error::Error; use git_tidy_core::git::GitOps; -use git_tidy_core::types::{Classification, CleanResult, FailedItem, ScanResult, WorktreeInfo}; +use git_tidy_core::types::{ + Classification, CleanResult, FailedItem, WorktreeInfo, WorktreeScanResult, +}; /// Options controlling worktree cleanup behavior. pub struct CleanOptions { @@ -32,7 +34,7 @@ pub struct RemovedWorktree { /// Run the clean operation on a scan result. pub fn run_clean( git: &dyn GitOps, - scan_result: &ScanResult, + scan_result: &WorktreeScanResult, options: &CleanOptions, out: &mut dyn Write, ) -> Result, Error> { @@ -41,7 +43,7 @@ pub fn run_clean( let mut skipped = 0; for group in &scan_result.repos { - for wt in &group.worktrees { + for wt in &group.items { // Filter by classification if !should_clean(&wt.classification, options) { skipped += 1; @@ -230,8 +232,9 @@ mod tests { use std::path::PathBuf; use git_tidy_core::counts::Counts; + use git_tidy_core::scan::RepoGroup; use git_tidy_core::testutil::MockGitBuilder; - use git_tidy_core::types::{Annotations, ClassificationLabel, RepoGroup, WorktreeInfo}; + use git_tidy_core::types::{Annotations, ClassificationLabel, WorktreeInfo}; use super::*; @@ -255,17 +258,17 @@ mod tests { } } - fn make_scan(worktrees: Vec) -> ScanResult { + fn make_scan(worktrees: Vec) -> WorktreeScanResult { let mut counts = Counts::default(); for wt in &worktrees { counts.increment(wt.classification.label()); } let total = worktrees.len(); - ScanResult { + WorktreeScanResult { repos: vec![RepoGroup { repo_path: repo(), name: "test".to_string(), - worktrees, + items: worktrees, }], total_scanned: total, counts, diff --git a/crates/git-worktree-tidy/src/output.rs b/crates/git-worktree-tidy/src/output.rs index 8812068..a87aba3 100644 --- a/crates/git-worktree-tidy/src/output.rs +++ b/crates/git-worktree-tidy/src/output.rs @@ -1,23 +1,18 @@ use std::io::Write; use git_tidy_core::output as shared; -use git_tidy_core::types::ScanResult; +use git_tidy_core::types::WorktreeScanResult; /// Write human-readable scan output. /// /// Per-group: heading + `format_table` over the group's `WorktreeInfo`s, which /// reads `TidyItem for WorktreeInfo` defined in `git_tidy_core::output`. -pub fn write_human(out: &mut dyn Write, result: &ScanResult) -> std::io::Result<()> { +pub fn write_human(out: &mut dyn Write, result: &WorktreeScanResult) -> std::io::Result<()> { shared::write_warnings(out, &result.warnings)?; for group in &result.repos { - writeln!( - out, - "\n{} ({} worktrees)", - group.name, - group.worktrees.len() - )?; - shared::format_table(out, &group.worktrees)?; + writeln!(out, "\n{} ({} worktrees)", group.name, group.items.len())?; + shared::format_table(out, &group.items)?; } shared::write_summary_line( @@ -33,14 +28,14 @@ pub fn write_human(out: &mut dyn Write, result: &ScanResult) -> std::io::Result< } /// Write JSON scan output using the flat spec format. -pub fn write_json(out: &mut dyn Write, result: &ScanResult) -> std::io::Result<()> { +pub fn write_json(out: &mut dyn Write, result: &WorktreeScanResult) -> std::io::Result<()> { shared::write_json_flat(out, result) } /// Write porcelain (machine-readable, tab-delimited) scan output. -pub fn write_porcelain(out: &mut dyn Write, result: &ScanResult) -> std::io::Result<()> { +pub fn write_porcelain(out: &mut dyn Write, result: &WorktreeScanResult) -> std::io::Result<()> { for group in &result.repos { - shared::format_porcelain(out, &group.worktrees)?; + shared::format_porcelain(out, &group.items)?; } Ok(()) } @@ -51,14 +46,15 @@ mod tests { use super::*; use git_tidy_core::counts::Counts; + use git_tidy_core::scan::RepoGroup; use git_tidy_core::types::*; - fn make_scan_result() -> ScanResult { - ScanResult { + fn make_scan_result() -> WorktreeScanResult { + WorktreeScanResult { repos: vec![RepoGroup { repo_path: PathBuf::from("/repos/Backend"), name: "Backend".to_string(), - worktrees: vec![ + items: vec![ WorktreeInfo { path: PathBuf::from("/dev/Backend-parallel"), parent_repo: PathBuf::from("/repos/Backend"), @@ -113,11 +109,11 @@ mod tests { #[test] fn write_human_with_partial_includes_unmatched_extras() { - let result = ScanResult { + let result = WorktreeScanResult { repos: vec![RepoGroup { repo_path: PathBuf::from("/repos/App"), name: "App".to_string(), - worktrees: vec![WorktreeInfo { + items: vec![WorktreeInfo { path: PathBuf::from("/dev/App-theme"), parent_repo: PathBuf::from("/repos/App"), branch: Some("alternate-icons".to_string()), @@ -166,7 +162,7 @@ mod tests { #[test] fn write_human_with_warnings() { - let result = ScanResult { + let result = WorktreeScanResult { repos: vec![], total_scanned: 0, counts: Counts::default(), diff --git a/crates/git-worktree-tidy/src/scan.rs b/crates/git-worktree-tidy/src/scan.rs index 66a9ff3..2faa0f4 100644 --- a/crates/git-worktree-tidy/src/scan.rs +++ b/crates/git-worktree-tidy/src/scan.rs @@ -2,15 +2,14 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use git_tidy_core::classification; -use git_tidy_core::counts::Counts; use git_tidy_core::discovery; use git_tidy_core::error::Error; use git_tidy_core::filter::{NameFilter, filter_paths}; use git_tidy_core::git::GitOps; use git_tidy_core::output::repo_display_name; use git_tidy_core::progress::Progress; -use git_tidy_core::scan::parallel_classify; -use git_tidy_core::types::{ClassificationLabel, RepoGroup, ScanResult}; +use git_tidy_core::scan::{ScanOptions, run_pipeline}; +use git_tidy_core::types::{ClassificationLabel, WorktreeScanResult}; use crate::discovery::{self as wt_discovery, DiscoveredWorktree}; @@ -62,7 +61,7 @@ pub fn run_scan( entity_filter: &NameFilter, repo_filter: &NameFilter, progress: &Progress, -) -> Result { +) -> Result { let repo_paths = discovery::discover_repos(directory)?; let repo_paths = filter_paths(repo_paths, repo_filter); let groups = wt_discovery::discover_worktrees(git, &repo_paths); @@ -81,8 +80,11 @@ pub fn run_scan( /// Classify pre-discovered worktree groups. /// /// Accepts a `BTreeMap` of parent-repo to worktrees (as returned by -/// [`discovery::discover_worktrees`] and optional filtering), fetches each -/// repo, classifies every worktree, and returns a [`ScanResult`]. +/// [`discovery::discover_worktrees`] and optional filtering) and runs the shared +/// [`run_pipeline`] seam over the parent repos with fetch enabled. The per-repo +/// closure looks each repo's worktrees up from the captured map and classifies +/// them; the seam fetches every repo first and aggregates the +/// [`WorktreeScanResult`]. pub fn run_scan_repos( git: &dyn GitOps, groups: BTreeMap>, @@ -90,19 +92,21 @@ pub fn run_scan_repos( verbose: bool, noise_patterns: &[String], progress: &Progress, -) -> Result { +) -> Result { let repo_paths: Vec = groups.keys().cloned().collect(); - let fetch_paths: Vec<&Path> = repo_paths.iter().map(|p| p.as_path()).collect(); - let mut warnings = git_tidy_core::fetch::parallel_fetch(git, &fetch_paths, progress); - let (repos, scan_warnings) = parallel_classify( + let result = run_pipeline( + git, &repo_paths, + &ScanOptions { fetch: true }, + "Scanning worktrees", + progress, |repo_path| { let mut local_warnings = Vec::new(); let worktrees = match groups.get(repo_path) { Some(wts) => wts, - None => return (None, vec![]), + None => return (Vec::new(), vec![]), }; let default_branch = match classification::detect_default_branch(git, repo_path) { @@ -112,15 +116,14 @@ pub fn run_scan_repos( "could not determine default branch for {} -- skipping", repo_path.display() )); - return (None, local_warnings); + return (Vec::new(), local_warnings); } }; - let repo_name = repo_display_name(repo_path); - if verbose { eprintln!( - "{repo_name}: {} worktrees (default_branch={default_branch})", + "{}: {} worktrees (default_branch={default_branch})", + repo_display_name(repo_path), worktrees.len(), ); } @@ -159,38 +162,11 @@ pub fn run_scan_repos( classified.sort_by_key(|wt| wt.classification.priority()); - let group = if classified.is_empty() { - None - } else { - Some(RepoGroup { - repo_path: repo_path.to_path_buf(), - name: repo_name, - worktrees: classified, - }) - }; - - (group, local_warnings) + (classified, local_warnings) }, - "Scanning worktrees", - progress, ); - warnings.extend(scan_warnings); - - let mut counts = Counts::default(); - let mut total_scanned = 0; - for g in &repos { - for wt in &g.worktrees { - counts.increment(wt.classification.label()); - } - total_scanned += g.worktrees.len(); - } - Ok(ScanResult { - repos, - total_scanned, - counts, - warnings, - }) + Ok(result) } #[cfg(test)] @@ -334,4 +310,28 @@ mod tests { assert!(result.repos.is_empty()); assert!(result.warnings.is_empty()); } + + #[test] + fn run_scan_repos_drops_repo_when_default_branch_errors() { + // Confirms preserved behavior across the run_pipeline migration: a repo + // present in the map with a worktree but whose default-branch detection + // fails yields no group. The closure returns (Vec::new(), warning) and the + // seam drops the now-empty group, exactly as the old manual is_empty check did. + let groups = make_groups(&[("/dev/RepoA", vec![make_worktree("RepoA-feature", "RepoA")])]); + // No with_symbolic_ref / with_rev_parse_verify stubs, so detect_default_branch fails. + let git = MockGitBuilder::new().build(); + let progress = Progress::disabled(); + let result = run_scan_repos(&git, groups, 100, false, &[], &progress).unwrap(); + + assert!(result.repos.is_empty()); + assert_eq!(result.total_scanned, 0); + assert!( + result + .warnings + .iter() + .any(|w| w.contains("could not determine default branch")), + "expected default-branch warning, got: {:?}", + result.warnings, + ); + } } diff --git a/crates/git-worktree-tidy/tests/integration_git.rs b/crates/git-worktree-tidy/tests/integration_git.rs index 3f538a4..06eea5a 100644 --- a/crates/git-worktree-tidy/tests/integration_git.rs +++ b/crates/git-worktree-tidy/tests/integration_git.rs @@ -168,7 +168,7 @@ fn worktree_with_deleted_branch_classifies_as_landed_stale() { assert_eq!(result.total_scanned, 1); assert_eq!(result.repos.len(), 1); - let wt_info = &result.repos[0].worktrees[0]; + let wt_info = &result.repos[0].items[0]; assert_eq!( wt_info.classification, Classification::LandedStale,