Skip to content

Commit 091c846

Browse files
authored
fix: keep artifact cleanup branch identity stable (#428)
1 parent 488ac1e commit 091c846

2 files changed

Lines changed: 14 additions & 14 deletions

File tree

inc/Workspace/WorkspaceArtifactCleanup.php

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -245,23 +245,19 @@ private function build_worktree_artifact_cleanup_plan( bool $force, array $opts
245245
$skipped = array();
246246

247247
foreach ( $slice as $wt ) {
248-
$handle = (string) ( $wt['handle'] ?? '?' );
249-
$repo = (string) ( $wt['repo'] ?? '' );
250-
$wt_path = (string) ( $wt['path'] ?? '' );
248+
$handle = (string) ( $wt['handle'] ?? '?' );
249+
$repo = (string) ( $wt['repo'] ?? '' );
250+
$wt_path = (string) ( $wt['path'] ?? '' );
251+
$resolved_branch = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file( $wt_path ) : null;
251252
if ( $safety_probes ) {
252-
$branch = (string) ( $wt['branch'] ?? '' );
253-
if ( '' === $branch ) {
254-
$resolved = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file( $wt_path ) : null;
255-
$branch = (string) ( $resolved ?? $wt['branch_slug'] ?? '' );
256-
}
253+
$branch = (string) ( $resolved_branch ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' );
257254
} else {
258255
// Inventory rows only carry `branch_slug` (the directory slug,
259256
// e.g. `fix-foo`). The plan apply path revalidates against the
260257
// real git branch from `worktree_list()` (e.g. `fix/foo`), so
261258
// resolve it cheaply here from the per-worktree `.git/HEAD`
262259
// pointer file. This is two file reads vs a `git` invocation.
263-
$resolved = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file( $wt_path ) : null;
264-
$branch = (string) ( $resolved ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' );
260+
$branch = (string) ( $resolved_branch ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' );
265261
}
266262

267263
// Inventory rows don't include detected artifacts; detect them on

tests/smoke-worktree-cleanup-artifacts-bounded.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,15 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra
192192
$run( 'git branch -M main', $primary );
193193
$run( 'git push -u origin main', $primary );
194194

195-
// One real worktree with an artifact directory.
196-
$run( 'git checkout -b feature-real', $primary );
195+
// One real worktree with an artifact directory. Use a slashed branch so
196+
// bounded inventory's directory slug (`feature-real`) differs from HEAD's
197+
// branch identity (`feature/real`).
198+
$run( 'git checkout -b feature/real', $primary );
197199
file_put_contents( $primary . '/feature-real.txt', 'real' );
198200
$run( 'git add . && git commit -m feature', $primary );
199-
$run( 'git push -u origin feature-real', $primary );
201+
$run( 'git push -u origin feature/real', $primary );
200202
$run( 'git checkout main', $primary );
201-
$run( sprintf( 'git worktree add %s feature-real', escapeshellarg( $tmp . '/demo@feature-real' ) ), $primary );
203+
$run( sprintf( 'git worktree add %s feature/real', escapeshellarg( $tmp . '/demo@feature-real' ) ), $primary );
202204
mkdir( $tmp . '/demo@feature-real/target', 0755, true );
203205
file_put_contents( $tmp . '/demo@feature-real/target/artifact.bin', str_repeat( 'x', 1024 ) );
204206

@@ -249,6 +251,7 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra
249251
fn( $row ) => 'demo@feature-real' === ( $row['handle'] ?? '' )
250252
) ) );
251253
$assert( 1, count( $apply_plan['candidates'] ), 'extracted exactly one planned candidate' );
254+
$assert( 'feature/real', $apply_plan['candidates'][0]['branch'] ?? '', 'bounded dry-run records HEAD branch instead of directory slug' );
252255

253256
$workspace->full_listing_calls = 0;
254257
$start = microtime( true );
@@ -257,6 +260,7 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra
257260
$assert( false, is_wp_error( $apply ), 'apply_plan revalidation succeeds on huge workspace' );
258261
$assert( 0, $workspace->full_listing_calls, 'apply_plan revalidation does not call full worktree_list' );
259262
$assert( true, (bool) ( $apply['pagination']['safety_probes'] ?? false ), 'apply_plan revalidation still runs safety probes' );
263+
$assert( array(), (array) ( $apply['summary']['skipped_by_reason'] ?? array() ), 'apply_plan does not reject slashed branches as branch mismatches' );
260264
$assert( 1, (int) ( $apply['summary']['removed_artifacts'] ?? 0 ), 'apply_plan removes the planned artifact' );
261265
$assert( false, is_dir( $tmp . '/demo@feature-real/target' ), 'apply_plan revalidation removes the target directory' );
262266
$assert_lt( 5.0, $elapsed, 'apply_plan revalidation stays fast because only_handles narrows the scan (' . number_format( $elapsed, 3 ) . 's)' );

0 commit comments

Comments
 (0)