From 3f5b28e3444509bb1c50e4968462ae443c896a5f Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Mon, 11 May 2026 09:15:30 -0400 Subject: [PATCH] fix: persist cleanup eligibility signals --- inc/Workspace/Workspace.php | 75 ++++++++++++++++----- tests/smoke-worktree-metadata-reconcile.php | 43 ++++++++++-- 2 files changed, 96 insertions(+), 22 deletions(-) diff --git a/inc/Workspace/Workspace.php b/inc/Workspace/Workspace.php index 62ec73a..e58ea90 100644 --- a/inc/Workspace/Workspace.php +++ b/inc/Workspace/Workspace.php @@ -3955,7 +3955,7 @@ private function build_worktree_metadata_reconciliation_row( array $wt, array &$ ); } - if ( null !== $finalizer_signal && ! WorktreeContextInjector::has_cleanup_signal( $metadata ) ) { + if ( null !== $finalizer_signal && ! $this->has_explicit_cleanup_eligible_state( $metadata ) ) { if ( $dirty > 0 || $unpushed > 0 ) { return array( 'skip' => array_merge( @@ -3971,10 +3971,23 @@ private function build_worktree_metadata_reconciliation_row( array $wt, array &$ ); } + $finalized_state = (string) ( $finalizer_signal['finalized_state'] ?? WorktreeContextInjector::STATE_MERGED ); $finalizer_metadata = WorktreeContextInjector::build_finalizer_metadata( - WorktreeContextInjector::STATE_MERGED, + $finalized_state, isset( $finalizer_signal['pr_url'] ) ? (string) $finalizer_signal['pr_url'] : null ); + $evidence = array_filter( + array( + 'signal' => $finalizer_signal['signal'], + 'finalized_state' => $finalized_state, + 'reason' => $finalizer_signal['reason'], + 'detected_at' => gmdate( 'c' ), + 'dirty' => $dirty, + 'unpushed' => $unpushed, + 'pr_url' => $finalizer_signal['pr_url'] ?? null, + ), + fn( $value ) => null !== $value && '' !== $value + ); $proposed = array_merge( $metadata, array( @@ -3987,9 +4000,10 @@ private function build_worktree_metadata_reconciliation_row( array $wt, array &$ ), $finalizer_metadata, array( - 'auto_finalized_by' => 'worktree_reconcile_metadata', - 'auto_finalized_signal' => $finalizer_signal['signal'], - 'auto_finalized_reason' => $finalizer_signal['reason'], + 'auto_finalized_by' => 'worktree_reconcile_metadata', + 'auto_finalized_signal' => $finalizer_signal['signal'], + 'auto_finalized_reason' => $finalizer_signal['reason'], + 'cleanup_eligibility_evidence' => $evidence, ) ); @@ -4012,13 +4026,15 @@ private function build_worktree_metadata_reconciliation_row( array $wt, array &$ 'pr_url' => $finalizer_signal['pr_url'] ?? null, 'proposed_metadata' => $proposed, 'source_map' => array( - 'handle' => 'filesystem', - 'repo' => 'filesystem', - 'branch' => 'git', - 'path' => 'git', - 'created_at' => empty( $metadata['created_at'] ) ? 'filesystem' : 'metadata', - 'observed_at' => 'reconcile_run', - 'lifecycle_state' => 'merge_signal', + 'handle' => 'filesystem', + 'repo' => 'filesystem', + 'branch' => 'git', + 'path' => 'git', + 'created_at' => empty( $metadata['created_at'] ) ? 'filesystem' : 'metadata', + 'observed_at' => 'reconcile_run', + 'lifecycle_state' => 'merge_signal', + 'finalized_state' => 'merge_signal', + 'cleanup_eligibility_evidence' => 'merge_signal', ), ), ), @@ -4121,6 +4137,21 @@ private function build_worktree_metadata_reconciliation_row( array $wt, array &$ ); } + /** + * Check whether metadata already stores the current explicit cleanup state. + * + * Legacy finalized records are still cleanup signals, but reconciliation should + * promote them to explicit cleanup_eligible metadata so later inventory-only + * cleanup has durable evidence to inspect. + * + * @param array $metadata Worktree metadata. + * @return bool + */ + private function has_explicit_cleanup_eligible_state( array $metadata ): bool { + $state = isset( $metadata['lifecycle_state'] ) ? WorktreeContextInjector::normalize_state( (string) $metadata['lifecycle_state'] ) : null; + return WorktreeContextInjector::STATE_CLEANUP_ELIGIBLE === $state; + } + /** * Detect an unambiguous merge signal for lifecycle reconciliation. * @@ -4128,7 +4159,7 @@ private function build_worktree_metadata_reconciliation_row( array $wt, array &$ * @param array $metadata Persisted lifecycle metadata. * @param array $github_cache Run-local GitHub lookup cache. * @param array $fetched Run-local fetched repo cache. - * @return array{signal:string,reason:string,pr_url?:string}|null + * @return array{signal:string,reason:string,finalized_state?:string,pr_url?:string}|null */ private function detect_worktree_lifecycle_finalizer_signal( array $wt, array $metadata, array &$github_cache, array &$fetched ): ?array { $repo = (string) ( $wt['repo'] ?? '' ); @@ -4171,7 +4202,7 @@ private function detect_worktree_lifecycle_finalizer_signal( array $wt, array $m * * @param array $metadata Persisted lifecycle metadata. * @param array $github_cache Run-local GitHub lookup cache. - * @return array{signal:string,reason:string,pr_url?:string}|null + * @return array{signal:string,reason:string,finalized_state?:string,pr_url?:string}|null */ private function detect_stored_pr_merged_signal( array $metadata, array &$github_cache ): ?array { $pr_repo = isset( $metadata['pr_repo'] ) ? (string) $metadata['pr_repo'] : ''; @@ -4195,14 +4226,22 @@ private function detect_stored_pr_merged_signal( array $metadata, array &$github $github_cache[ $cache_key ] = $pr; } - if ( ! is_array( $pr ) || empty( $pr['merged_at'] ) ) { + if ( ! is_array( $pr ) ) { return null; } + $state = (string) ( $pr['state'] ?? '' ); + if ( empty( $pr['merged_at'] ) && 'closed' !== $state ) { + return null; + } + + $merged = ! empty( $pr['merged_at'] ); + return array( - 'signal' => 'pr-merged', - 'reason' => sprintf( 'stored PR #%d merged (%s)', $pr_number, (string) ( $pr['state'] ?? 'closed' ) ), - 'pr_url' => (string) ( $pr['html_url'] ?? $pr_url ), + 'signal' => $merged ? 'pr-merged' : 'pr-closed', + 'reason' => $merged ? sprintf( 'stored PR #%d merged (%s)', $pr_number, $state ) : sprintf( 'stored PR #%d closed without merge', $pr_number ), + 'finalized_state' => $merged ? WorktreeContextInjector::STATE_MERGED : WorktreeContextInjector::STATE_CLOSED, + 'pr_url' => (string) ( $pr['html_url'] ?? $pr_url ), ); } diff --git a/tests/smoke-worktree-metadata-reconcile.php b/tests/smoke-worktree-metadata-reconcile.php index 576ecd0..d5683e3 100644 --- a/tests/smoke-worktree-metadata-reconcile.php +++ b/tests/smoke-worktree-metadata-reconcile.php @@ -34,6 +34,16 @@ public static function apiGet( string $url, array $params, string $pat ) { // ph ), ); } + if ( str_ends_with( $url, '/pulls/102' ) ) { + return array( + 'data' => array( + 'number' => 102, + 'state' => 'closed', + 'merged_at' => '', + 'html_url' => 'https://github.com/acme/demo/pull/102', + ), + ); + } return array( 'data' => array() ); } } @@ -192,6 +202,7 @@ function size_format( $bytes ): string { $make_branch( 'unmanaged-invalid' ); $make_branch( 'already-current' ); $make_branch( 'pr-merged' ); + $make_branch( 'pr-closed' ); $make_branch( 'upstream-gone' ); $make_branch( 'dirty-merged' ); $make_branch( 'unpushed-merged' ); @@ -203,6 +214,7 @@ function size_format( $bytes ): string { $run( sprintf( 'git worktree add %s unmanaged-invalid', escapeshellarg( $tmp . '/demo@unmanaged-invalid' ) ), $primary ); $run( sprintf( 'git worktree add %s already-current', escapeshellarg( $tmp . '/demo@already-current' ) ), $primary ); $run( sprintf( 'git worktree add %s pr-merged', escapeshellarg( $tmp . '/demo@pr-merged' ) ), $primary ); + $run( sprintf( 'git worktree add %s pr-closed', escapeshellarg( $tmp . '/demo@pr-closed' ) ), $primary ); $run( sprintf( 'git worktree add %s upstream-gone', escapeshellarg( $tmp . '/demo@upstream-gone' ) ), $primary ); $run( sprintf( 'git worktree add %s dirty-merged', escapeshellarg( $tmp . '/demo@dirty-merged' ) ), $primary ); $run( sprintf( 'git worktree add %s unpushed-merged', escapeshellarg( $tmp . '/demo@unpushed-merged' ) ), $primary ); @@ -278,6 +290,21 @@ function size_format( $bytes ): string { 'pr_repo' => 'acme/demo', ) ); + \DataMachineCode\Workspace\WorktreeContextInjector::store_lifecycle_metadata( + 'demo@pr-closed', + array( + 'handle' => 'demo@pr-closed', + 'repo' => 'demo', + 'branch' => 'pr-closed', + 'path' => $tmp . '/demo@pr-closed', + 'created_at' => '2026-04-01T00:00:00+00:00', + 'observed_at' => '2026-04-01T00:00:00+00:00', + 'lifecycle_state' => \DataMachineCode\Workspace\WorktreeContextInjector::STATE_ACTIVE, + 'pr_url' => 'https://github.com/acme/demo/pull/102', + 'pr_number' => 102, + 'pr_repo' => 'acme/demo', + ) + ); $ws = new \DataMachineCode\Workspace\Workspace(); @@ -285,7 +312,7 @@ function size_format( $bytes ): string { $plan = $ws->worktree_reconcile_metadata( array( 'dry_run' => true ) ); $assert( true, ! is_wp_error( $plan ) && ( $plan['success'] ?? false ), 'dry-run succeeds' ); $assert( true, $plan['dry_run'] ?? false, 'dry-run flag is true' ); - $assert( 6, (int) ( $plan['summary']['proposed'] ?? 0 ), 'dry-run proposes unmanaged rows and safe merged lifecycle finalizers' ); + $assert( 7, (int) ( $plan['summary']['proposed'] ?? 0 ), 'dry-run proposes unmanaged rows and safe merged lifecycle finalizers' ); $assert( 0, (int) ( $plan['summary']['written'] ?? 0 ), 'dry-run writes nothing' ); $assert( 1, (int) ( $plan['summary']['skipped_by_reason']['external_worktree'] ?? 0 ), 'dry-run distinguishes external worktrees' ); $assert( 2, (int) ( $plan['summary']['skipped_by_reason']['unsafe_cleanup_eligible_state'] ?? 0 ), 'dry-run keeps dirty and unpushed merged worktrees out of auto-finalize proposals' ); @@ -309,6 +336,10 @@ function size_format( $bytes ): string { $assert( 'pr-merged', $by_handle['demo@pr-merged']['signal'] ?? '', 'stored PR proposal records pr-merged signal' ); $assert( 'cleanup_eligible', $by_handle['demo@pr-merged']['proposed_metadata']['lifecycle_state'] ?? '', 'stored PR proposal becomes cleanup_eligible metadata' ); $assert( 'merged', $by_handle['demo@pr-merged']['proposed_metadata']['finalized_state'] ?? '', 'stored PR proposal preserves merged finalized state' ); + $assert( 'auto_finalize_merged', $by_handle['demo@pr-closed']['reason_code'] ?? '', 'closed stored PR is proposed for auto-finalization' ); + $assert( 'pr-closed', $by_handle['demo@pr-closed']['signal'] ?? '', 'closed stored PR proposal records pr-closed signal' ); + $assert( 'closed', $by_handle['demo@pr-closed']['proposed_metadata']['finalized_state'] ?? '', 'closed stored PR preserves closed finalized state' ); + $assert( 'pr-closed', $by_handle['demo@pr-closed']['proposed_metadata']['cleanup_eligibility_evidence']['signal'] ?? '', 'closed stored PR records cleanup eligibility evidence' ); $assert( 'auto_finalize_merged', $by_handle['demo@upstream-gone']['reason_code'] ?? '', 'upstream-gone branch is proposed for auto-finalization' ); $assert( 'upstream-gone', $by_handle['demo@upstream-gone']['signal'] ?? '', 'upstream-gone proposal records local merge signal' ); $unsafe_by_handle = array(); @@ -326,10 +357,10 @@ function size_format( $bytes ): string { echo "\nApply reviewed plan\n"; $apply = $ws->worktree_reconcile_metadata( array( 'apply_plan' => $plan ) ); $assert( true, ! is_wp_error( $apply ) && ( $apply['success'] ?? false ), 'apply succeeds' ); - $assert( 6, (int) ( $apply['summary']['written'] ?? 0 ), 'apply writes exact current matches' ); - $assert( 6, (int) ( $apply['summary']['written'] ?? 0 ), 'apply reports written metadata rows' ); + $assert( 7, (int) ( $apply['summary']['written'] ?? 0 ), 'apply writes exact current matches' ); + $assert( 7, (int) ( $apply['summary']['written'] ?? 0 ), 'apply reports written metadata rows' ); $assert( 0, (int) ( $apply['summary']['skipped'] ?? 0 ), 'apply skips nothing for current plan' ); - $assert( 6, count( $apply['written'] ?? array() ), 'apply exposes written rows distinctly' ); + $assert( 7, count( $apply['written'] ?? array() ), 'apply exposes written rows distinctly' ); $stored = \DataMachineCode\Workspace\WorktreeContextInjector::get_metadata( 'demo@unmanaged-missing' ); $assert( 'demo@unmanaged-missing', $stored['handle'] ?? '', 'stored metadata includes handle' ); $assert( true, ! empty( $stored['observed_at'] ), 'stored metadata includes observed_at' ); @@ -340,6 +371,10 @@ function size_format( $bytes ): string { $assert( 'merged', $stored_pr['finalized_state'] ?? '', 'apply stores merged finalizer state for merged PR worktree' ); $stored_gone = \DataMachineCode\Workspace\WorktreeContextInjector::get_metadata( 'demo@upstream-gone' ); $assert( 'cleanup_eligible', $stored_gone['lifecycle_state'] ?? '', 'apply stores cleanup_eligible for upstream-gone worktree' ); + $assert( 'upstream-gone', $stored_gone['cleanup_eligibility_evidence']['signal'] ?? '', 'apply stores upstream-gone cleanup eligibility evidence' ); + $stored_closed = \DataMachineCode\Workspace\WorktreeContextInjector::get_metadata( 'demo@pr-closed' ); + $assert( 'cleanup_eligible', $stored_closed['lifecycle_state'] ?? '', 'apply stores cleanup_eligible for closed PR worktree' ); + $assert( 'closed', $stored_closed['cleanup_eligibility_evidence']['finalized_state'] ?? '', 'apply stores closed PR cleanup eligibility evidence' ); $auto_apply = $ws->worktree_reconcile_metadata( array( 'apply' => true ) ); $assert( true, ! is_wp_error( $auto_apply ) && ( $auto_apply['success'] ?? false ), 'DMC-owned reconciliation apply path runs without a manual plan' );