From 2923d3dff9f68417189fdce1cb495be93cc51123 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Mon, 18 May 2026 13:43:16 -0400 Subject: [PATCH] fix: keep artifact cleanup branch identity stable --- inc/Workspace/WorkspaceArtifactCleanup.php | 16 ++++++---------- .../smoke-worktree-cleanup-artifacts-bounded.php | 12 ++++++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/inc/Workspace/WorkspaceArtifactCleanup.php b/inc/Workspace/WorkspaceArtifactCleanup.php index c2d62fe..9489543 100644 --- a/inc/Workspace/WorkspaceArtifactCleanup.php +++ b/inc/Workspace/WorkspaceArtifactCleanup.php @@ -245,23 +245,19 @@ private function build_worktree_artifact_cleanup_plan( bool $force, array $opts $skipped = array(); foreach ( $slice as $wt ) { - $handle = (string) ( $wt['handle'] ?? '?' ); - $repo = (string) ( $wt['repo'] ?? '' ); - $wt_path = (string) ( $wt['path'] ?? '' ); + $handle = (string) ( $wt['handle'] ?? '?' ); + $repo = (string) ( $wt['repo'] ?? '' ); + $wt_path = (string) ( $wt['path'] ?? '' ); + $resolved_branch = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file( $wt_path ) : null; if ( $safety_probes ) { - $branch = (string) ( $wt['branch'] ?? '' ); - if ( '' === $branch ) { - $resolved = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file( $wt_path ) : null; - $branch = (string) ( $resolved ?? $wt['branch_slug'] ?? '' ); - } + $branch = (string) ( $resolved_branch ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' ); } else { // Inventory rows only carry `branch_slug` (the directory slug, // e.g. `fix-foo`). The plan apply path revalidates against the // real git branch from `worktree_list()` (e.g. `fix/foo`), so // resolve it cheaply here from the per-worktree `.git/HEAD` // pointer file. This is two file reads vs a `git` invocation. - $resolved = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file( $wt_path ) : null; - $branch = (string) ( $resolved ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' ); + $branch = (string) ( $resolved_branch ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' ); } // Inventory rows don't include detected artifacts; detect them on diff --git a/tests/smoke-worktree-cleanup-artifacts-bounded.php b/tests/smoke-worktree-cleanup-artifacts-bounded.php index f47cd08..92b327b 100644 --- a/tests/smoke-worktree-cleanup-artifacts-bounded.php +++ b/tests/smoke-worktree-cleanup-artifacts-bounded.php @@ -192,13 +192,15 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra $run( 'git branch -M main', $primary ); $run( 'git push -u origin main', $primary ); - // One real worktree with an artifact directory. - $run( 'git checkout -b feature-real', $primary ); + // One real worktree with an artifact directory. Use a slashed branch so + // bounded inventory's directory slug (`feature-real`) differs from HEAD's + // branch identity (`feature/real`). + $run( 'git checkout -b feature/real', $primary ); file_put_contents( $primary . '/feature-real.txt', 'real' ); $run( 'git add . && git commit -m feature', $primary ); - $run( 'git push -u origin feature-real', $primary ); + $run( 'git push -u origin feature/real', $primary ); $run( 'git checkout main', $primary ); - $run( sprintf( 'git worktree add %s feature-real', escapeshellarg( $tmp . '/demo@feature-real' ) ), $primary ); + $run( sprintf( 'git worktree add %s feature/real', escapeshellarg( $tmp . '/demo@feature-real' ) ), $primary ); mkdir( $tmp . '/demo@feature-real/target', 0755, true ); file_put_contents( $tmp . '/demo@feature-real/target/artifact.bin', str_repeat( 'x', 1024 ) ); @@ -249,6 +251,7 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra fn( $row ) => 'demo@feature-real' === ( $row['handle'] ?? '' ) ) ) ); $assert( 1, count( $apply_plan['candidates'] ), 'extracted exactly one planned candidate' ); + $assert( 'feature/real', $apply_plan['candidates'][0]['branch'] ?? '', 'bounded dry-run records HEAD branch instead of directory slug' ); $workspace->full_listing_calls = 0; $start = microtime( true ); @@ -257,6 +260,7 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra $assert( false, is_wp_error( $apply ), 'apply_plan revalidation succeeds on huge workspace' ); $assert( 0, $workspace->full_listing_calls, 'apply_plan revalidation does not call full worktree_list' ); $assert( true, (bool) ( $apply['pagination']['safety_probes'] ?? false ), 'apply_plan revalidation still runs safety probes' ); + $assert( array(), (array) ( $apply['summary']['skipped_by_reason'] ?? array() ), 'apply_plan does not reject slashed branches as branch mismatches' ); $assert( 1, (int) ( $apply['summary']['removed_artifacts'] ?? 0 ), 'apply_plan removes the planned artifact' ); $assert( false, is_dir( $tmp . '/demo@feature-real/target' ), 'apply_plan revalidation removes the target directory' ); $assert_lt( 5.0, $elapsed, 'apply_plan revalidation stays fast because only_handles narrows the scan (' . number_format( $elapsed, 3 ) . 's)' );