Skip to content

Commit bca4a85

Browse files
chubes4chubes4
authored
Merge pull request #681 from Extra-Chill/fix/issue-676-stale-artifact-markers
Fix stale marker artifact cleanup
2 parents 7f68ddd + 890a741 commit bca4a85

2 files changed

Lines changed: 83 additions & 34 deletions

File tree

‎inc/Workspace/WorkspaceArtifactCleanup.php‎

Lines changed: 60 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,11 @@ private function build_worktree_artifact_cleanup_plan( bool $force, array $opts
305305
$skipped = array();
306306

307307
foreach ( $slice as $wt ) {
308-
$handle = (string) ( $wt['handle'] ?? '?' );
309-
$repo = (string) ( $wt['repo'] ?? '' );
310-
$wt_path = (string) ( $wt['path'] ?? '' );
311-
$resolved_branch = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file($wt_path) : null;
308+
$handle = (string) ( $wt['handle'] ?? '?' );
309+
$repo = (string) ( $wt['repo'] ?? '' );
310+
$wt_path = (string) ( $wt['path'] ?? '' );
311+
$resolved_branch = '' !== $wt_path ? $this->resolve_worktree_branch_from_head_file($wt_path) : null;
312+
$stale_marker_recovery = null;
312313
if ( $safety_probes ) {
313314
$branch = (string) ( $resolved_branch ?? $wt['branch'] ?? $wt['branch_slug'] ?? '' );
314315
} else {
@@ -381,14 +382,20 @@ private function build_worktree_artifact_cleanup_plan( bool $force, array $opts
381382
$dirty_probe = $this->probe_worktree_dirty_count($wt_path, self::CLEANUP_GIT_PROBE_TIMEOUT);
382383
if ( is_wp_error($dirty_probe) ) {
383384
$diagnostic = $this->classify_worktree_git_probe_failure($handle, $repo, $wt_path, $dirty_probe, 'artifact cleanup dirty-state probe', 'leaving artifacts in place');
384-
$skipped[] = array_merge(
385-
$base_row,
386-
$diagnostic,
387-
array( 'artifacts' => $artifacts )
388-
);
389-
continue;
385+
if ( $force && $this->is_stale_worktree_marker_diagnostic($diagnostic) ) {
386+
$stale_marker_recovery = $diagnostic;
387+
$dirty_count = 0;
388+
} else {
389+
$skipped[] = array_merge(
390+
$base_row,
391+
$diagnostic,
392+
array( 'artifacts' => $artifacts )
393+
);
394+
continue;
395+
}
396+
} else {
397+
$dirty_count = (int) $dirty_probe;
390398
}
391-
$dirty_count = (int) $dirty_probe;
392399
}
393400
if ( $dirty_count > 0 && ! $force ) {
394401
$skipped[] = array_merge(
@@ -402,26 +409,32 @@ private function build_worktree_artifact_cleanup_plan( bool $force, array $opts
402409
continue;
403410
}
404411

405-
$unpushed = $this->count_unpushed_commits($wt_path, self::CLEANUP_GIT_PROBE_TIMEOUT);
406-
if ( is_wp_error($unpushed) ) {
407-
$diagnostic = $this->classify_worktree_git_probe_failure($handle, $repo, $wt_path, $unpushed, 'artifact cleanup safety probe', 'leaving artifacts in place');
408-
$skipped[] = array_merge(
409-
$base_row,
410-
$diagnostic,
411-
array( 'artifacts' => $artifacts )
412-
);
413-
continue;
414-
}
415-
if ( $unpushed > 0 && ! $force ) {
416-
$skipped[] = array_merge(
417-
$base_row, array(
418-
'reason_code' => 'unpushed_commits',
419-
'reason' => sprintf('%d unpushed commit(s) - pass force=true to override artifact cleanup only', $unpushed),
420-
'unpushed' => $unpushed,
421-
'artifacts' => $artifacts,
422-
)
423-
);
424-
continue;
412+
if ( null === $stale_marker_recovery ) {
413+
$unpushed = $this->count_unpushed_commits($wt_path, self::CLEANUP_GIT_PROBE_TIMEOUT);
414+
if ( is_wp_error($unpushed) ) {
415+
$diagnostic = $this->classify_worktree_git_probe_failure($handle, $repo, $wt_path, $unpushed, 'artifact cleanup safety probe', 'leaving artifacts in place');
416+
if ( $force && $this->is_stale_worktree_marker_diagnostic($diagnostic) ) {
417+
$stale_marker_recovery = $diagnostic;
418+
} else {
419+
$skipped[] = array_merge(
420+
$base_row,
421+
$diagnostic,
422+
array( 'artifacts' => $artifacts )
423+
);
424+
continue;
425+
}
426+
}
427+
if ( isset($unpushed) && ! is_wp_error($unpushed) && $unpushed > 0 && ! $force ) {
428+
$skipped[] = array_merge(
429+
$base_row, array(
430+
'reason_code' => 'unpushed_commits',
431+
'reason' => sprintf('%d unpushed commit(s) - pass force=true to override artifact cleanup only', $unpushed),
432+
'unpushed' => $unpushed,
433+
'artifacts' => $artifacts,
434+
)
435+
);
436+
continue;
437+
}
425438
}
426439
}
427440

@@ -441,6 +454,12 @@ private function build_worktree_artifact_cleanup_plan( bool $force, array $opts
441454
// destructible from a bounded plan alone.
442455
$candidate['safety_probes_deferred'] = true;
443456
}
457+
if ( null !== $stale_marker_recovery ) {
458+
$candidate['reason_code'] = 'profile_artifacts_stale_worktree_marker';
459+
$candidate['reason'] = 'profile-derived reconstructable artifacts can be removed; git worktree marker is stale, but explicit force allows artifact-only cleanup after path containment validation';
460+
$candidate['git_metadata_warning'] = $stale_marker_recovery['reason'] ?? 'git worktree metadata marker is stale or missing';
461+
$candidate['metadata_reconciliation_hint'] = 'Run studio wp datamachine-code workspace worktree reconcile-metadata --dry-run --limit=25 --offset=0 --until-budget=30s --format=json to repair stale worktree metadata after artifact cleanup.';
462+
}
444463
$candidates[] = $candidate;
445464
}
446465

@@ -518,6 +537,16 @@ private function build_worktree_artifact_cleanup_summary( array $candidates, arr
518537
);
519538
}
520539

540+
/**
541+
* Check whether a git probe diagnostic represents stale worktree metadata.
542+
*
543+
* @param array<string,mixed> $diagnostic Classified git probe diagnostic.
544+
* @return bool
545+
*/
546+
private function is_stale_worktree_marker_diagnostic( array $diagnostic ): bool {
547+
return 'stale_worktree_marker' === (string) ( $diagnostic['reason_code'] ?? '' );
548+
}
549+
521550
/**
522551
* Extract artifact cleanup candidates from a dry-run JSON report.
523552
*

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

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,11 @@ function apply_filters( string $hook_name, $value ) // phpcs:ignore Generic.Cod
193193
$run('git add local.txt && git commit -m local', $tmp . '/demo@unpushed');
194194
symlink($tmp . '/demo@active', $site_tmp . '/wp-content/plugins/demo-active');
195195

196+
mkdir($tmp . '/demo@broken-marker/target', 0755, true);
197+
file_put_contents($tmp . '/demo@broken-marker/.git', 'gitdir: ' . $primary . '/.git/worktrees/demo@broken-marker' . "\n");
198+
file_put_contents($tmp . '/demo@broken-marker/Cargo.toml', "[package]\nname = \"demo\"\nversion = \"0.1.0\"\n");
199+
file_put_contents($tmp . '/demo@broken-marker/target/artifact.bin', str_repeat('broken', 128));
200+
196201
$workspace = new \DataMachineCode\Workspace\Workspace();
197202

198203
echo "=== smoke-worktree-cleanup-artifacts ===\n";
@@ -218,6 +223,7 @@ function apply_filters( string $hook_name, $value ) // phpcs:ignore Generic.Cod
218223
$assert(true, in_array('demo@clean', $bounded_handles, true), 'bounded dry-run surfaces clean worktree');
219224
$assert(true, in_array('demo@dirty', $bounded_handles, true), 'bounded dry-run surfaces dirty worktree (deferred safety probes)');
220225
$assert(true, in_array('demo@unpushed', $bounded_handles, true), 'bounded dry-run surfaces unpushed worktree (deferred safety probes)');
226+
$assert(true, in_array('demo@broken-marker', $bounded_handles, true), 'bounded dry-run surfaces stale-marker worktree for artifact review');
221227
foreach ( $plan['candidates'] ?? array() as $candidate ) {
222228
$assert(true, (bool) ( $candidate['safety_probes_deferred'] ?? false ), 'bounded candidate ' . ( $candidate['handle'] ?? '?' ) . ' is flagged safety_probes_deferred');
223229
}
@@ -301,19 +307,33 @@ function apply_filters( string $hook_name, $value ) // phpcs:ignore Generic.Cod
301307
$assert(false, is_dir($tmp . '/demo@clean/target'), 'apply-plan removes clean artifact directory');
302308
$assert(true, is_dir($tmp . '/demo@dirty/target'), 'apply-plan revalidation skips dirty worktree even when bounded plan flagged it');
303309
$assert(true, is_dir($tmp . '/demo@unpushed/target'), 'apply-plan revalidation skips unpushed worktree even when bounded plan flagged it');
310+
$assert(true, is_dir($tmp . '/demo@broken-marker/target'), 'apply-plan revalidation skips stale-marker worktree without force');
304311
$assert(true, is_dir($tmp . '/demo@clean'), 'apply-plan leaves worktree directory in place');
305312

306313
$apply_skip_reasons = array_column($apply['skipped'] ?? array(), 'reason_code', 'handle');
307314
$assert('dirty_worktree', $apply_skip_reasons['demo@dirty'] ?? '', 'apply-plan revalidation skips dirty rows with explicit reason');
308315
$assert('unpushed_commits', $apply_skip_reasons['demo@unpushed'] ?? '', 'apply-plan revalidation skips unpushed rows with explicit reason');
316+
$assert('stale_worktree_marker', $apply_skip_reasons['demo@broken-marker'] ?? '', 'apply-plan revalidation gives stale marker recovery reason without force');
309317

310-
$force_plan = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'exhaustive' => true, 'force' => true ));
318+
$force_plan = $workspace->worktree_cleanup_artifacts(array( 'dry_run' => true, 'safety_probes' => true, 'force' => true ));
311319
$force_handles = array_column($force_plan['candidates'] ?? array(), 'handle');
312-
$assert(true, in_array('demo@dirty', $force_handles, true), 'force permits dirty artifact candidate (exhaustive)');
313-
$assert(true, in_array('demo@unpushed', $force_handles, true), 'force permits unpushed artifact candidate (exhaustive)');
320+
$assert(true, in_array('demo@dirty', $force_handles, true), 'force permits dirty artifact candidate with bounded safety probes');
321+
$assert(true, in_array('demo@unpushed', $force_handles, true), 'force permits unpushed artifact candidate with bounded safety probes');
322+
$assert(true, in_array('demo@broken-marker', $force_handles, true), 'force permits stale-marker artifact candidate when path safety is validated');
323+
$force_by_handle = array_column($force_plan['candidates'] ?? array(), null, 'handle');
314324
$force_skip_reasons = array_column($force_plan['skipped'] ?? array(), 'reason_code', 'handle');
315325
$assert('active_symlink_target', $force_skip_reasons['demo@active'] ?? '', 'force still protects active symlink target');
316326

327+
$broken_force_plan = array( 'candidates' => array( $force_by_handle['demo@broken-marker'] ?? array() ) );
328+
$broken_force_apply = $workspace->worktree_cleanup_artifacts(array( 'apply_plan' => $broken_force_plan, 'force' => true ));
329+
$assert(false, is_wp_error($broken_force_apply), 'force apply-plan for stale-marker artifact returns report');
330+
$assert(1, (int) ( $broken_force_apply['summary']['removed_artifacts'] ?? 0 ), 'force apply-plan removes stale-marker reconstructable artifact');
331+
$removed_by_handle = array_column($broken_force_apply['removed'] ?? array(), null, 'handle');
332+
$assert('profile_artifacts_stale_worktree_marker', $removed_by_handle['demo@broken-marker']['reason_code'] ?? '', 'force apply-plan records explicit stale-marker recovery reason');
333+
$assert(true, str_contains($removed_by_handle['demo@broken-marker']['metadata_reconciliation_hint'] ?? '', 'reconcile-metadata --dry-run'), 'force apply-plan points to metadata reconciliation after removal');
334+
$assert(false, is_dir($tmp . '/demo@broken-marker/target'), 'force apply-plan removes stale-marker artifact directory');
335+
$assert(true, is_dir($tmp . '/demo@broken-marker'), 'force apply-plan leaves stale-marker worktree directory in place');
336+
317337
if ($failures > 0 ) {
318338
echo "\nFAILURES: {$failures}/{$total}\n";
319339
exit(1);

0 commit comments

Comments
 (0)