diff --git a/inc/Workspace/WorkspaceActiveNoSignalCleanup.php b/inc/Workspace/WorkspaceActiveNoSignalCleanup.php index 482e108..406f8e9 100644 --- a/inc/Workspace/WorkspaceActiveNoSignalCleanup.php +++ b/inc/Workspace/WorkspaceActiveNoSignalCleanup.php @@ -989,7 +989,7 @@ private function validate_current_cleanup_worktree( string $repo, string $path, } $current_branch = (string) $this->resolve_worktree_branch_from_head_file($real_path); - if ( null !== $expected_branch && $expected_branch !== $current_branch ) { + if ( null !== $expected_branch && ! $this->cleanup_branch_identity_matches($expected_branch, $current_branch) ) { return new \WP_Error('branch_identity_mismatch', 'worktree branch identity changed before apply'); } if ( null === $expected_branch && '' === $current_branch ) { @@ -1027,12 +1027,31 @@ private function validate_current_cleanup_worktree( string $repo, string $path, return array( 'real_path' => $real_path, 'primary_path' => $primary_path, - 'branch' => null !== $expected_branch ? $expected_branch : $current_branch, + 'branch' => '' !== $current_branch ? $current_branch : (string) $expected_branch, 'dirty' => (int) $dirty, 'unpushed' => (int) $unpushed, ); } + /** + * Compare a report branch identity against the live worktree branch. + * + * Active/no-signal rows may originate from stale metadata or handle slugs where + * `fix/foo` is represented as `fix-foo`. Treat that as the same branch while + * still rejecting detached/missing heads and unrelated branch changes. + * + * @param string $expected_branch Branch carried by the report row. + * @param string $current_branch Branch resolved from the live worktree HEAD. + * @return bool Whether the identities are compatible. + */ + private function cleanup_branch_identity_matches( string $expected_branch, string $current_branch ): bool { + if ( '' === $expected_branch || '' === $current_branch ) { + return false; + } + + return $expected_branch === $current_branch || $expected_branch === $this->slugify_branch($current_branch); + } + /** * Build a skip row for finalized active/no-signal apply. * diff --git a/inc/Workspace/WorkspaceCoreUtilities.php b/inc/Workspace/WorkspaceCoreUtilities.php index cd0e0bf..c4e550a 100644 --- a/inc/Workspace/WorkspaceCoreUtilities.php +++ b/inc/Workspace/WorkspaceCoreUtilities.php @@ -519,7 +519,8 @@ private function is_git_timeout_error( mixed $result ): bool { return false; } - return 'git_command_timeout' === $result->get_error_code(); + $code = method_exists($result, 'get_error_code') ? $result->get_error_code() : ( $result->code ?? '' ); + return 'git_command_timeout' === $code; } /** diff --git a/inc/Workspace/WorkspaceWorktreeCleanupEngine.php b/inc/Workspace/WorkspaceWorktreeCleanupEngine.php index 768158d..846cd02 100644 --- a/inc/Workspace/WorkspaceWorktreeCleanupEngine.php +++ b/inc/Workspace/WorkspaceWorktreeCleanupEngine.php @@ -25,14 +25,17 @@ trait WorkspaceWorktreeCleanupEngine { * - It is not the primary checkout * - Its branch is not main/master/trunk/develop/HEAD * - It has no uncommitted changes (unless $force) - * - At least one merge signal is present: + * - At least one cleanup signal is present: * a) `git for-each-ref` reports upstream status "gone" (branch * was deleted on the remote — typical after GitHub * "auto-delete head branches" fires on PR merge), OR * b) GitHub API reports a closed+merged PR whose head - * branch matches this worktree's branch. + * branch matches this worktree's branch, OR + * c) the clean local worktree has no unpushed commits and is backed + * by an existing remote branch, so removing the local checkout does + * not delete recoverable Git state. * - * Signal (a) is local and fast; signal (b) requires a PAT + network + * Signals (a) and (c) are local and fast; signal (b) requires a PAT + network * but catches the case where the remote branch still exists (e.g. * manual merge without branch deletion). * @@ -848,7 +851,7 @@ private function classify_worktree_git_probe_failure( string $handle, string $re * @return string */ private function get_wp_error_code( \WP_Error $error ): string { - return (string) $error->get_error_code(); + return method_exists($error, 'get_error_code') ? (string) $error->get_error_code() : (string) ( $error->code ?? '' ); } /** @@ -858,7 +861,7 @@ private function get_wp_error_code( \WP_Error $error ): string { * @return mixed */ private function get_wp_error_data( \WP_Error $error ): mixed { - return $error->get_error_data(); + return method_exists($error, 'get_error_data') ? $error->get_error_data() : ( $error->data ?? null ); } /** @@ -2500,7 +2503,12 @@ private function classify_dirty_obsolete_on_default_branch( * Signal priority: * 1. `upstream-gone` — local branch's upstream tracking ref is gone. * Typical after GitHub auto-deletes the head branch on PR merge. - * 2. `pr-merged` — GitHub API reports a closed+merged PR for this + * 2. `local-merged` — branch has no commits outside remote default. + * 3. `remote-tracking-clean` — branch still exists on origin, while the + * local worktree is clean and has no unpushed commits. Age gates are + * enforced by the caller before removal when retention cleanup passes + * `older_than`. + * 4. `pr-merged` — GitHub API reports a closed+merged PR for this * branch. Requires $skip_github = false and a configured PAT. * * @param string $primary_path Path to the primary git checkout. @@ -2537,6 +2545,11 @@ private function detect_merge_signal( string $primary_path, string $repo, string return $local_merged; } + $remote_tracking_clean = $this->detect_remote_tracking_clean_signal($primary_path, $branch); + if ( null !== $remote_tracking_clean ) { + return $remote_tracking_clean; + } + if ( $skip_github ) { return null; } @@ -2624,6 +2637,33 @@ private function detect_local_merged_signal( string $primary_path, string $branc ); } + /** + * Detect clean local-only checkouts whose work is preserved by a remote branch. + * + * This is intentionally less strict than a merge signal: the remote branch may + * still be open or unmerged, but the local checkout is only a disposable copy + * when it is clean, has no unpushed commits, and `origin/` exists. + * Removing the local worktree and local branch does not delete the remote + * branch or close any PR, so the work can be picked up from Git later. + * + * @param string $primary_path Path to the primary git checkout. + * @param string $branch Branch name. + * @return array{signal: string, reason: string, remote_ref: string}|null + */ + private function detect_remote_tracking_clean_signal( string $primary_path, string $branch ): ?array { + $remote_ref = 'refs/remotes/origin/' . $branch; + $result = $this->run_git($primary_path, sprintf('rev-parse --verify --quiet %s', escapeshellarg($remote_ref)), self::CLEANUP_GIT_PROBE_TIMEOUT); + if ( is_wp_error($result) ) { + return null; + } + + return array( + 'signal' => 'remote-tracking-clean', + 'reason' => 'clean local worktree has no unpushed commits and the branch exists on origin; removing the local checkout preserves remote Git state', + 'remote_ref' => $remote_ref, + ); + } + /** * Extract owner/repo slug from a primary checkout's origin remote. * diff --git a/tests/smoke-worktree-cleanup.php b/tests/smoke-worktree-cleanup.php index 1bff916..e1493a9 100644 --- a/tests/smoke-worktree-cleanup.php +++ b/tests/smoke-worktree-cleanup.php @@ -450,42 +450,35 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra $mixed_dirty_row = array_values($mixed_dirty_skips)[0] ?? array(); $assert('dirty_worktree', $mixed_dirty_row['reason_code'] ?? '', 'mixed source/artifact dirt stays protected as generic dirty worktree'); - // unmerged-feature should be skipped (no merge signal) - $unmerged = array_filter($plan['skipped'] ?? array(), fn( $s ) => ( $s['handle'] ?? '' ) === 'demo@unmerged-feature'); - $assert(1, count($unmerged), 'unmerged worktree skipped with exactly one entry'); - $unmerged_row = array_values($unmerged)[0] ?? array(); - $assert('no_merge_signal', $unmerged_row['reason_code'] ?? '', 'unmerged skip exposes stable reason code'); + $assert_contains($plan['candidates'] ?? array(), 'demo@unmerged-feature', 'clean remote-backed unmerged worktree flagged as local-only cleanup candidate'); + $unmerged_candidate = array_values(array_filter($plan['candidates'] ?? array(), fn( $s ) => ( $s['handle'] ?? '' ) === 'demo@unmerged-feature'))[0] ?? array(); + $assert('remote-tracking-clean', $unmerged_candidate['reason_code'] ?? '', 'unmerged remote-backed candidate exposes stable reason code'); + $assert('remote-tracking-clean', $unmerged_candidate['signal'] ?? '', 'unmerged remote-backed candidate records cleanup signal'); $detached = array_values(array_filter($plan['skipped'] ?? array(), fn( $s ) => ( $s['handle'] ?? '' ) === 'demo@feature-detached-stored'))[0] ?? array(); $assert('detached_worktree', $detached['reason_code'] ?? '', 'detached worktree with stored branch metadata is classified explicitly'); $assert('feature/detached-stored', $detached['branch'] ?? '', 'detached worktree recovers branch from stored metadata'); $assert(array( 'branch' ), $detached['hydrated_fields'] ?? array(), 'detached worktree reports hydrated branch field'); $protected = array_values(array_filter($plan['skipped'] ?? array(), fn( $s ) => ( $s['handle'] ?? '' ) === 'demo@develop'))[0] ?? array(); $assert('protected_base_branch_worktree', $protected['reason_code'] ?? '', 'protected branch worktree gets actionable protected-base diagnostic'); - $assert('remote_branch_still_exists', $unmerged_row['merge_signal_evidence']['classification'] ?? '', 'unmerged skip distinguishes existing remote branch'); - $assert('still_exists', $unmerged_row['merge_signal_evidence']['remote_branch'] ?? '', 'unmerged skip records remote branch evidence'); - $assert(true, str_contains($unmerged_row['active_review_command'] ?? '', 'active-no-signal-report'), 'unmerged skip includes active/no-signal review command'); - $assert(true, str_contains($unmerged_row['active_review_commands']['finalized_apply_dry_run'] ?? '', 'active-no-signal-finalized-apply --dry-run'), 'unmerged skip includes finalized PR dry-run apply command'); + $assert(true, str_contains($unmerged_candidate['reason'] ?? '', 'origin'), 'unmerged remote-backed candidate explains remote preservation'); $external_rows = array_values(array_filter($plan['skipped'] ?? array(), fn( $s ) => ( $s['reason_code'] ?? '' ) === 'external_worktree')); $external_row = $external_rows[0] ?? array(); $assert('external_worktree', $external_row['reason_code'] ?? '', 'external worktree exposes stable reason code'); $assert(true, str_contains($external_row['hint'] ?? '', 'outside the DMC workspace'), 'external worktree includes remediation hint'); - $assert(4, (int) ( $plan['summary']['would_remove'] ?? 0 ), 'summary counts cleanup candidates'); + $assert(6, (int) ( $plan['summary']['would_remove'] ?? 0 ), 'summary counts cleanup candidates'); $assert(2, (int) ( $plan['summary']['skipped_by_reason']['dirty_worktree'] ?? 0 ), 'summary counts dirty skips by reason'); $assert(1, (int) ( $plan['summary']['skipped_by_reason']['artifact_only_dirty_worktree'] ?? 0 ), 'summary counts artifact-only dirty skips separately'); $assert(1, (int) ( $plan['summary']['cleanup_buckets']['artifact_only_dirty_worktree'] ?? 0 ), 'cleanup buckets expose artifact-only dirty count'); $assert(true, in_array('artifact_only_dirty_worktree', array_column($plan['summary']['skipped_next_commands'] ?? array(), 'reason_code'), true), 'summary exposes artifact-only dirty next command'); - $assert(true, isset($plan['summary']['skipped_by_reason']['no_merge_signal']), 'summary includes no_merge_signal bucket'); + $assert(false, isset($plan['summary']['skipped_by_reason']['no_merge_signal']), 'summary has no no_merge_signal bucket when clean remote-backed worktrees are removable'); $assert(4096, (int) ( $plan['summary']['total_size_bytes'] ?? -1 ), 'summary counts artifact-only dirty worktree size bytes'); $assert(4096, (int) ( $plan['summary']['artifact_size_bytes'] ?? -1 ), 'summary counts artifact-only dirty artifact size bytes'); $top_by_size = (array) ( $plan['summary']['top_by_size'] ?? array() ); $assert('demo@artifact-only-dirty', $top_by_size[0]['handle'] ?? '', 'summary top-by-size includes artifact-only dirty worktree'); $next_commands = (array) ( $plan['summary']['skipped_next_commands'] ?? array() ); - $assert(true, in_array('no_merge_signal', array_column($next_commands, 'reason_code'), true), 'summary includes no_merge_signal next command'); - $no_merge_commands = array_values(array_filter($next_commands, fn( $row ) => ( $row['reason_code'] ?? '' ) === 'no_merge_signal'))[0] ?? array(); - $assert(true, str_contains($no_merge_commands['command'] ?? '', 'active-no-signal-report'), 'no_merge_signal next command routes to active/no-signal evidence report'); - $assert(true, str_contains($no_merge_commands['commands']['equivalent_clean_apply_dry_run'] ?? '', 'active-no-signal-equivalent-clean-apply --dry-run'), 'no_merge_signal next commands include equivalent-clean dry-run apply'); + $assert(false, in_array('no_merge_signal', array_column($next_commands, 'reason_code'), true), 'summary omits no_merge_signal next command when no rows need active/no-signal review'); $assert(true, array_key_exists('total_size_bytes', $plan['summary'] ?? array()), 'summary exposes total worktree size bytes field'); $assert(true, array_key_exists('artifact_size_bytes', $plan['summary'] ?? array()), 'summary exposes artifact size bytes field'); $assert(true, array_key_exists('top_by_size', $plan['summary'] ?? array()), 'summary exposes top worktrees by size field'); @@ -670,10 +663,11 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra ) ); $assert(true, ! is_wp_error($age_plan) && ( $age_plan['success'] ?? false ), 'age-filtered dry_run returns success'); - $assert(1, (int) ( $age_plan['summary']['would_remove'] ?? 0 ), 'older_than keeps only old cleanup candidate'); + $assert(2, (int) ( $age_plan['summary']['would_remove'] ?? 0 ), 'older_than keeps old merged and remote-backed cleanup candidates'); $assert(1, (int) ( $age_plan['summary']['age_filter']['excluded'] ?? 0 ), 'age filter summary counts newer candidate exclusion'); - $assert(2, (int) ( $age_plan['summary']['age_filter']['unknown_age'] ?? 0 ), 'age filter summary counts unknown-age candidate exclusions'); + $assert(3, (int) ( $age_plan['summary']['age_filter']['unknown_age'] ?? 0 ), 'age filter summary counts unknown-age candidate exclusions'); $assert_contains($age_plan['candidates'] ?? array(), 'demo@merged-autodelete', 'older_than keeps old merged worktree'); + $assert_contains($age_plan['candidates'] ?? array(), 'demo@unmerged-feature', 'older_than keeps old clean remote-backed worktree'); $recent_age_rows = array_values(array_filter($age_plan['skipped'] ?? array(), fn( $s ) => ( $s['handle'] ?? '' ) === 'demo@merged-recent')); $assert('age_filter', $recent_age_rows[0]['reason_code'] ?? '', 'newer merged worktree is skipped by age_filter'); $assert('excluded', $recent_age_rows[0]['age_filter']['decision'] ?? '', 'age-filter skip row exposes excluded decision'); @@ -820,8 +814,8 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra // Primary survives. $assert(true, is_dir($primary . '/.git'), 'primary .git survives cleanup'); - // Protected branches: unmerged/dirty worktrees survive. - $assert(true, is_dir($tmp . '/demo@unmerged-feature'), 'unmerged worktree survives cleanup'); + // Dirty/external worktrees survive; clean remote-backed worktrees are local-only cleanup candidates. + $assert(false, is_dir($tmp . '/demo@unmerged-feature'), 'clean remote-backed unmerged worktree is removed locally'); $assert(true, is_dir($tmp . '/demo@dirty-branch'), 'dirty worktree survives cleanup'); $assert(true, is_dir($tmp . '/demo@merged-stale-plan'), 'dirty stale-plan worktree survives cleanup'); $assert(true, is_dir($tmp . '-external/demo-external'), 'external worktree survives cleanup');