From 24af5f0fcaeb273a656de1f6633aa6c9fb44cad6 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Sat, 6 Jun 2026 16:09:05 -0400 Subject: [PATCH 1/2] refactor: centralize worktree cleanup classification --- inc/Support/GitRunner.php | 2 +- inc/Workspace/Workspace.php | 40 +---- inc/Workspace/WorkspaceCleanupPlan.php | 12 +- inc/Workspace/WorktreeCleanupClassifier.php | 153 ++++++++++++++++++++ tests/smoke-worktree-cleanup-classifier.php | 85 +++++++++++ 5 files changed, 247 insertions(+), 45 deletions(-) create mode 100644 inc/Workspace/WorktreeCleanupClassifier.php create mode 100644 tests/smoke-worktree-cleanup-classifier.php diff --git a/inc/Support/GitRunner.php b/inc/Support/GitRunner.php index 4ba4ec7f..3f2ef624 100644 --- a/inc/Support/GitRunner.php +++ b/inc/Support/GitRunner.php @@ -121,7 +121,7 @@ public static function run( string $path, string $args, int $timeout_seconds = 0 ); if ( is_wp_error($result) ) { - $data = $result->get_error_data(); + $data = method_exists($result, 'get_error_data') ? $result->get_error_data() : array(); $data = is_array($data) ? $data : array(); if ( $timeout_seconds > 0 && isset($data['timeout']) ) { return new \WP_Error('git_command_timeout', $result->get_error_message(), $data); diff --git a/inc/Workspace/Workspace.php b/inc/Workspace/Workspace.php index d6efc49e..42ea2743 100644 --- a/inc/Workspace/Workspace.php +++ b/inc/Workspace/Workspace.php @@ -29,6 +29,7 @@ require_once __DIR__ . '/WorkspaceWorktreeLifecycle.php'; require_once __DIR__ . '/WorkspaceWorktreeInventoryCleanup.php'; require_once __DIR__ . '/WorkspaceWorktreeEmergencyCleanup.php'; +require_once __DIR__ . '/WorktreeCleanupClassifier.php'; class Workspace { @@ -3444,44 +3445,7 @@ private function worktree_declares_submodules( string $path ): bool { * @return array */ private function worktree_cleanup_buckets( int $candidate_count, array $candidates_by_signal, array $skipped_by_reason ): array { - $needs_reconciliation = (int) ( $skipped_by_reason['needs_metadata_reconcile'] ?? 0 ) - + (int) ( $skipped_by_reason['requires_full_scan'] ?? 0 ) - + (int) ( $skipped_by_reason['missing_metadata'] ?? 0 ) - + (int) ( $skipped_by_reason['lifecycle_reconciliation_candidate'] ?? 0 ); - $needs_full_review = (int) ( $skipped_by_reason['active_no_signal'] ?? 0 ) - + (int) ( $skipped_by_reason['no_inventory_cleanup_signal'] ?? 0 ) - + (int) ( $skipped_by_reason['no_merge_signal'] ?? 0 ) - + (int) ( $skipped_by_reason['github_unknown'] ?? 0 ) - + (int) ( $skipped_by_reason['external_worktree'] ?? 0 ) - + (int) ( $skipped_by_reason['protected_branch'] ?? 0 ) - + (int) ( $skipped_by_reason['protected_base_branch_worktree'] ?? 0 ) - + (int) ( $skipped_by_reason['detached_worktree'] ?? 0 ) - + (int) ( $skipped_by_reason['detached_protected_branch'] ?? 0 ) - + (int) ( $skipped_by_reason['submodule_worktree'] ?? 0 ) - + (int) ( $skipped_by_reason['probe_timeout'] ?? 0 ) - + (int) ( $skipped_by_reason['unknown_age'] ?? 0 ); - $blocked_by_dirty_or_unpushed = (int) ( $skipped_by_reason['dirty_worktree'] ?? 0 ) - + (int) ( $skipped_by_reason['merged_pr_with_only_obsolete_dirty_changes'] ?? 0 ) - + (int) ( $skipped_by_reason['unpushed_commits'] ?? 0 ); - - $buckets = array( - 'artifact_only_dirty_worktree' => (int) ( $skipped_by_reason['artifact_only_dirty_worktree'] ?? 0 ), - 'blocked_by_dirty_or_unpushed' => $blocked_by_dirty_or_unpushed, - 'needs_full_review' => $needs_full_review, - 'needs_reconciliation' => $needs_reconciliation, - 'safe_to_remove_now' => $candidate_count, - - // Legacy aliases retained for existing automation while callers migrate - // to the explicit safety buckets above. - 'explicit_cleanup_candidates' => (int) ( $candidates_by_signal['cleanup_eligible'] ?? 0 ), - 'lifecycle_reconciliation_candidates' => (int) ( $skipped_by_reason['lifecycle_reconciliation_candidate'] ?? 0 ), - 'metadata_reconciliation_candidates' => (int) ( $skipped_by_reason['needs_metadata_reconcile'] ?? 0 ) + (int) ( $skipped_by_reason['requires_full_scan'] ?? 0 ) + (int) ( $skipped_by_reason['missing_metadata'] ?? 0 ), - 'dirty_unpushed' => $blocked_by_dirty_or_unpushed, - 'active_no_signal' => (int) ( $skipped_by_reason['active_no_signal'] ?? 0 ) + (int) ( $skipped_by_reason['no_inventory_cleanup_signal'] ?? 0 ), - ); - - ksort($buckets); - return $buckets; + return WorktreeCleanupClassifier::buckets($candidate_count, $candidates_by_signal, $skipped_by_reason); } /** diff --git a/inc/Workspace/WorkspaceCleanupPlan.php b/inc/Workspace/WorkspaceCleanupPlan.php index b6769a73..d983a459 100644 --- a/inc/Workspace/WorkspaceCleanupPlan.php +++ b/inc/Workspace/WorkspaceCleanupPlan.php @@ -9,6 +9,10 @@ defined('ABSPATH') || exit; +if ( ! class_exists(WorktreeCleanupClassifier::class) ) { + require_once __DIR__ . '/WorktreeCleanupClassifier.php'; +} + trait WorkspaceCleanupPlan { @@ -220,15 +224,11 @@ private function build_cleanup_plan_resolver_rows( array $skipped ): array { continue; } $reason = (string) ( $row['reason_code'] ?? '' ); - if ( ! in_array($reason, array( 'needs_metadata_reconcile', 'requires_full_scan', 'lifecycle_reconciliation_candidate', 'active_no_signal', 'no_inventory_cleanup_signal' ), true) ) { + if ( ! WorktreeCleanupClassifier::is_resolver_reason($reason) ) { continue; } - $resolver_type = match ( $reason ) { - 'needs_metadata_reconcile', 'requires_full_scan' => 'metadata_reconciliation', - 'lifecycle_reconciliation_candidate' => 'lifecycle_reconciliation', - default => 'merge_signal', - }; + $resolver_type = WorktreeCleanupClassifier::resolver_type($reason); $next_action = match ( $resolver_type ) { 'metadata_reconciliation' => 'workspace worktree reconcile-metadata --dry-run --format=json', 'lifecycle_reconciliation' => 'workspace worktree cleanup --dry-run --format=json', diff --git a/inc/Workspace/WorktreeCleanupClassifier.php b/inc/Workspace/WorktreeCleanupClassifier.php new file mode 100644 index 00000000..97a71960 --- /dev/null +++ b/inc/Workspace/WorktreeCleanupClassifier.php @@ -0,0 +1,153 @@ + $candidates_by_signal Candidate signal counts. + * @param array $skipped_by_reason Skipped reason counts. + * @return array + */ + public static function buckets( int $candidate_count, array $candidates_by_signal, array $skipped_by_reason ): array { + $buckets = array( + self::BUCKET_ARTIFACT_ONLY_DIRTY => 0, + self::BUCKET_BLOCKED_BY_DIRTY_OR_UNPUSHED => 0, + self::BUCKET_NEEDS_FULL_REVIEW => 0, + self::BUCKET_NEEDS_RECONCILIATION => 0, + self::BUCKET_SAFE_TO_REMOVE_NOW => $candidate_count, + ); + + foreach ( $skipped_by_reason as $reason_code => $count ) { + $bucket = self::bucket_for_reason((string) $reason_code); + $buckets[ $bucket ] = ( $buckets[ $bucket ] ?? 0 ) + (int) $count; + } + + $buckets['explicit_cleanup_candidates'] = (int) ( $candidates_by_signal['cleanup_eligible'] ?? 0 ); + $buckets['lifecycle_reconciliation_candidates'] = (int) ( $skipped_by_reason['lifecycle_reconciliation_candidate'] ?? 0 ); + $buckets['metadata_reconciliation_candidates'] = (int) ( $skipped_by_reason['needs_metadata_reconcile'] ?? 0 ) + (int) ( $skipped_by_reason['requires_full_scan'] ?? 0 ) + (int) ( $skipped_by_reason['missing_metadata'] ?? 0 ); + $buckets['dirty_unpushed'] = $buckets[self::BUCKET_BLOCKED_BY_DIRTY_OR_UNPUSHED]; + $buckets['active_no_signal'] = (int) ( $skipped_by_reason['active_no_signal'] ?? 0 ) + (int) ( $skipped_by_reason['no_inventory_cleanup_signal'] ?? 0 ); + + ksort($buckets); + return $buckets; + } + + /** + * Whether a skip row reason can produce a read-only resolver plan row. + */ + public static function is_resolver_reason( string $reason_code ): bool { + return in_array($reason_code, self::RESOLVER_REASONS, true); + } + + /** + * Return resolver type for a skip reason. + */ + public static function resolver_type( string $reason_code ): string { + return match ( $reason_code ) { + 'needs_metadata_reconcile', 'requires_full_scan' => 'metadata_reconciliation', + 'lifecycle_reconciliation_candidate' => 'lifecycle_reconciliation', + default => 'merge_signal', + }; + } +} diff --git a/tests/smoke-worktree-cleanup-classifier.php b/tests/smoke-worktree-cleanup-classifier.php new file mode 100644 index 00000000..5e5b5c2b --- /dev/null +++ b/tests/smoke-worktree-cleanup-classifier.php @@ -0,0 +1,85 @@ + 1, + ), + array( + 'needs_metadata_reconcile' => 2, + 'requires_full_scan' => 1, + 'lifecycle_reconciliation_candidate' => 1, + 'active_no_signal' => 3, + 'github_unknown' => 1, + 'dirty_worktree' => 1, + 'merged_pr_with_only_obsolete_dirty_changes' => 1, + 'unpushed_commits' => 1, + 'artifact_only_dirty_worktree' => 1, + ) +); + +$assert('bucket counts safe candidates', 2 === (int) ( $buckets['safe_to_remove_now'] ?? -1 )); +$assert('bucket counts reconciliation reasons', 4 === (int) ( $buckets['needs_reconciliation'] ?? -1 )); +$assert('bucket counts full review reasons', 4 === (int) ( $buckets['needs_full_review'] ?? -1 )); +$assert('bucket counts dirty/unpushed blockers', 3 === (int) ( $buckets['blocked_by_dirty_or_unpushed'] ?? -1 )); +$assert('bucket counts artifact-only dirty separately', 1 === (int) ( $buckets['artifact_only_dirty_worktree'] ?? -1 )); +$assert('legacy explicit cleanup alias remains', 1 === (int) ( $buckets['explicit_cleanup_candidates'] ?? -1 )); +$assert('legacy metadata alias remains', 3 === (int) ( $buckets['metadata_reconciliation_candidates'] ?? -1 )); +$assert('legacy lifecycle alias remains', 1 === (int) ( $buckets['lifecycle_reconciliation_candidates'] ?? -1 )); +$assert('legacy active alias remains', 3 === (int) ( $buckets['active_no_signal'] ?? -1 )); +$assert('legacy dirty alias remains', 3 === (int) ( $buckets['dirty_unpushed'] ?? -1 )); + +$assert('metadata rows can produce resolver rows', WorktreeCleanupClassifier::is_resolver_reason('needs_metadata_reconcile')); +$assert('dirty rows do not produce resolver rows', ! WorktreeCleanupClassifier::is_resolver_reason('dirty_worktree')); +$assert('metadata resolver type is stable', 'metadata_reconciliation' === WorktreeCleanupClassifier::resolver_type('requires_full_scan')); +$assert('lifecycle resolver type is stable', 'lifecycle_reconciliation' === WorktreeCleanupClassifier::resolver_type('lifecycle_reconciliation_candidate')); +$assert('active resolver type is merge signal', 'merge_signal' === WorktreeCleanupClassifier::resolver_type('active_no_signal')); + +if ( ! empty($failures) ) { + echo "\nFAIL: " . count($failures) . " assertion(s) failed out of {$total}\n"; + foreach ( $failures as $failure ) { + echo " - {$failure}\n"; + } + exit(1); +} + +echo "\nOK ({$total} assertions)\n"; +exit(0); From e77e91fc2a05a969efa8ed3f87067e61967c46fd Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Sat, 6 Jun 2026 16:16:10 -0400 Subject: [PATCH 2/2] fix: align cleanup classifier formatting --- inc/Workspace/WorkspaceCleanupPlan.php | 2 +- inc/Workspace/WorktreeCleanupClassifier.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/inc/Workspace/WorkspaceCleanupPlan.php b/inc/Workspace/WorkspaceCleanupPlan.php index d983a459..baafca57 100644 --- a/inc/Workspace/WorkspaceCleanupPlan.php +++ b/inc/Workspace/WorkspaceCleanupPlan.php @@ -229,7 +229,7 @@ private function build_cleanup_plan_resolver_rows( array $skipped ): array { } $resolver_type = WorktreeCleanupClassifier::resolver_type($reason); - $next_action = match ( $resolver_type ) { + $next_action = match ( $resolver_type ) { 'metadata_reconciliation' => 'workspace worktree reconcile-metadata --dry-run --format=json', 'lifecycle_reconciliation' => 'workspace worktree cleanup --dry-run --format=json', default => 'workspace worktree cleanup --dry-run --skip-github --format=json', diff --git a/inc/Workspace/WorktreeCleanupClassifier.php b/inc/Workspace/WorktreeCleanupClassifier.php index 97a71960..bd3ec921 100644 --- a/inc/Workspace/WorktreeCleanupClassifier.php +++ b/inc/Workspace/WorktreeCleanupClassifier.php @@ -16,10 +16,10 @@ final class WorktreeCleanupClassifier { public const BUCKET_SAFE_TO_REMOVE_NOW = 'safe_to_remove_now'; - public const BUCKET_NEEDS_RECONCILIATION = 'needs_reconciliation'; - public const BUCKET_NEEDS_FULL_REVIEW = 'needs_full_review'; + public const BUCKET_NEEDS_RECONCILIATION = 'needs_reconciliation'; + public const BUCKET_NEEDS_FULL_REVIEW = 'needs_full_review'; public const BUCKET_BLOCKED_BY_DIRTY_OR_UNPUSHED = 'blocked_by_dirty_or_unpushed'; - public const BUCKET_ARTIFACT_ONLY_DIRTY = 'artifact_only_dirty_worktree'; + public const BUCKET_ARTIFACT_ONLY_DIRTY = 'artifact_only_dirty_worktree'; /** * Reason codes that indicate metadata/lifecycle reconciliation should run @@ -119,14 +119,14 @@ public static function buckets( int $candidate_count, array $candidates_by_signa ); foreach ( $skipped_by_reason as $reason_code => $count ) { - $bucket = self::bucket_for_reason((string) $reason_code); + $bucket = self::bucket_for_reason( (string) $reason_code ); $buckets[ $bucket ] = ( $buckets[ $bucket ] ?? 0 ) + (int) $count; } $buckets['explicit_cleanup_candidates'] = (int) ( $candidates_by_signal['cleanup_eligible'] ?? 0 ); $buckets['lifecycle_reconciliation_candidates'] = (int) ( $skipped_by_reason['lifecycle_reconciliation_candidate'] ?? 0 ); $buckets['metadata_reconciliation_candidates'] = (int) ( $skipped_by_reason['needs_metadata_reconcile'] ?? 0 ) + (int) ( $skipped_by_reason['requires_full_scan'] ?? 0 ) + (int) ( $skipped_by_reason['missing_metadata'] ?? 0 ); - $buckets['dirty_unpushed'] = $buckets[self::BUCKET_BLOCKED_BY_DIRTY_OR_UNPUSHED]; + $buckets['dirty_unpushed'] = $buckets[ self::BUCKET_BLOCKED_BY_DIRTY_OR_UNPUSHED ]; $buckets['active_no_signal'] = (int) ( $skipped_by_reason['active_no_signal'] ?? 0 ) + (int) ( $skipped_by_reason['no_inventory_cleanup_signal'] ?? 0 ); ksort($buckets);