Skip to content

Commit d954df3

Browse files
chubes4chubes4
authored
Fix workspace hygiene and cleanup resume (#642)
* Add workspace remote backend hygiene diagnostics * Apply cleanup rows without full scan * Fix workspace hygiene lint
1 parent 97f137d commit d954df3

4 files changed

Lines changed: 131 additions & 26 deletions

File tree

‎inc/Workspace/CleanupRunService.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,9 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
124124
$this->mark_batch_applying($worktree_batch, $run_id, $batch_type, $limit, $remaining_rows);
125125
$results['worktree_removal'] = $this->workspace->worktree_cleanup_merged(
126126
array(
127-
'apply_plan' => array( 'candidates' => array_map(fn( $item ) => $item['evidence'], $worktree_batch) ),
128-
'skip_github' => true,
127+
'apply_plan' => array( 'candidates' => array_map(fn( $item ) => $item['evidence'], $worktree_batch) ),
128+
'direct_apply_plan' => true,
129+
'skip_github' => true,
129130
)
130131
);
131132
$this->record_apply_result($worktree_batch, $results['worktree_removal'], 'removed');

‎inc/Workspace/WorkspaceHygieneReport.php‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ public function workspace_hygiene_report( array $opts = array() ): array|\WP_Err
5555
$worktree_status_mode = 'top_level_inventory';
5656
}
5757

58-
$size_report = $include_sizes ? $this->build_workspace_size_report($size_limit) : $this->empty_workspace_size_report($size_limit, false);
59-
$cleanup = null;
60-
$cleanup_error = null;
61-
$locks = WorkspaceMutationLock::status($this->workspace_path);
58+
$size_report = $include_sizes ? $this->build_workspace_size_report($size_limit) : $this->empty_workspace_size_report($size_limit, false);
59+
$cleanup = null;
60+
$cleanup_error = null;
61+
$locks = WorkspaceMutationLock::status($this->workspace_path);
6262
$remote_backend = $this->build_remote_workspace_backend_report();
6363

6464
if ( $include_cleanup ) {
@@ -570,27 +570,27 @@ private function build_workspace_disk_report(): array {
570570
*/
571571
private function summarize_workspace_worktrees( array $worktrees, ?array $cleanup ): array {
572572
$summary = array(
573-
'total' => count($worktrees),
574-
'primaries' => 0,
575-
'worktrees' => 0,
576-
'artifacts' => 0,
577-
'external' => 0,
578-
'dirty' => 0,
579-
'protected_dirty' => 0,
580-
'protected_unpushed' => 0,
581-
'missing_metadata' => 0,
582-
'stale_primaries' => 0,
573+
'total' => count($worktrees),
574+
'primaries' => 0,
575+
'worktrees' => 0,
576+
'artifacts' => 0,
577+
'external' => 0,
578+
'dirty' => 0,
579+
'protected_dirty' => 0,
580+
'protected_unpushed' => 0,
581+
'missing_metadata' => 0,
582+
'stale_primaries' => 0,
583583
'primary_freshness_by_status' => array(),
584584
'primary_freshness_attention' => array(),
585-
'base_branch_worktree_count' => 0,
586-
'base_branch_worktrees' => array(),
587-
'by_liveness' => array(
585+
'base_branch_worktree_count' => 0,
586+
'base_branch_worktrees' => array(),
587+
'by_liveness' => array(
588588
WorktreeContextInjector::LIVENESS_LIVE => 0,
589589
WorktreeContextInjector::LIVENESS_STOPPED => 0,
590590
WorktreeContextInjector::LIVENESS_STALE => 0,
591591
WorktreeContextInjector::LIVENESS_UNKNOWN => 0,
592592
),
593-
'duplicate_task_groups' => 0,
593+
'duplicate_task_groups' => 0,
594594
);
595595

596596
foreach ( $worktrees as $row ) {

‎inc/Workspace/WorkspaceWorktreeCleanupEngine.php‎

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public function worktree_cleanup_merged( array $opts = array() ): array|\WP_Erro
5151
$dry_run = ! empty($opts['dry_run']);
5252
$force = ! empty($opts['force']);
5353
$skip_github = ! empty($opts['skip_github']);
54+
$direct_apply_plan = ! empty($opts['direct_apply_plan']);
5455
$inventory_only = ! empty($opts['inventory_only']);
5556
$include_repaired_metadata = ! empty($opts['include_repaired_metadata']);
5657
$apply_plan = isset($opts['apply_plan']) && is_array($opts['apply_plan']) ? $opts['apply_plan'] : null;
@@ -104,6 +105,10 @@ public function worktree_cleanup_merged( array $opts = array() ): array|\WP_Erro
104105
// Applying a stale plan must never use the dirty override. The current
105106
// workspace state is re-evaluated below and dirty rows stay skipped.
106107
$force = false;
108+
109+
if ( $direct_apply_plan && ! $dry_run ) {
110+
return $this->apply_worktree_cleanup_plan_candidates($planned_candidates, $force, $started_at);
111+
}
107112
}
108113

109114
$age_filter = null;
@@ -631,7 +636,7 @@ function () use ( $cand, $force ) {
631636
// Delete the now-detached local branch while the repo lock still covers
632637
// shared git metadata.
633638
$primary_path = $this->get_primary_path($cand['repo']);
634-
$branch = $this->run_git($primary_path, sprintf('branch -D %s', escapeshellarg($cand['branch'])));
639+
$branch = $this->run_git($primary_path, sprintf('branch -D %s', escapeshellarg($cand['branch'])), self::CLEANUP_GIT_PROBE_TIMEOUT);
635640
return is_wp_error($branch) ? $branch : $remove;
636641
}
637642
);
@@ -1006,7 +1011,7 @@ function () use ( $repo, $branch, $wt_path, $force ) {
10061011

10071012
$primary_path = $this->get_primary_path($repo);
10081013
if ( '' !== $branch ) {
1009-
$delete = $this->run_git($primary_path, sprintf('branch -D %s', escapeshellarg($branch)));
1014+
$delete = $this->run_git($primary_path, sprintf('branch -D %s', escapeshellarg($branch)), self::CLEANUP_GIT_PROBE_TIMEOUT);
10101015
if ( is_wp_error($delete) ) {
10111016
// Branch deletion failure is non-fatal: the worktree is
10121017
// gone, the local branch may still be useful or may have
@@ -1076,6 +1081,95 @@ function () use ( $repo, $branch, $wt_path, $force ) {
10761081
);
10771082
}
10781083

1084+
/**
1085+
* Apply DB-backed cleanup rows without rebuilding a full workspace scan.
1086+
*
1087+
* @param array<int,array<string,mixed>> $candidates Reviewed cleanup rows.
1088+
* @param bool $force Whether dirty worktrees may be removed.
1089+
* @param float $started_at Start timestamp.
1090+
* @return array<string,mixed>
1091+
*/
1092+
private function apply_worktree_cleanup_plan_candidates( array $candidates, bool $force, float $started_at ): array {
1093+
$processed = 0;
1094+
$removed = array();
1095+
$skipped = array();
1096+
$bytes_reclaimed = 0;
1097+
1098+
foreach ( $candidates as $candidate ) {
1099+
++$processed;
1100+
$revalidated = $this->revalidate_bounded_cleanup_eligible_candidate($candidate, $force);
1101+
if ( isset($revalidated['skipped']) ) {
1102+
$skipped[] = $revalidated['skipped'];
1103+
continue;
1104+
}
1105+
1106+
$validated = $revalidated;
1107+
$repo = (string) ( $validated['repo'] ?? '' );
1108+
$branch = (string) ( $validated['branch'] ?? '' );
1109+
$wt_path = (string) ( $validated['path'] ?? '' );
1110+
$size = (int) ( $validated['size_bytes'] ?? 0 );
1111+
if ( $size <= 0 ) {
1112+
$measured = $this->estimate_path_size_bytes($wt_path);
1113+
$size = null === $measured ? 0 : (int) $measured;
1114+
}
1115+
1116+
$remove = WorkspaceMutationLock::with_repo(
1117+
$this->workspace_path,
1118+
$repo,
1119+
function () use ( $repo, $branch, $wt_path, $force ) {
1120+
$result = $this->remove_worktree_by_path($repo, $branch, $wt_path, $force);
1121+
if ( is_wp_error($result) ) {
1122+
return $result;
1123+
}
1124+
1125+
$primary_path = $this->get_primary_path($repo);
1126+
if ( '' !== $branch ) {
1127+
$delete = $this->run_git($primary_path, sprintf('branch -D %s', escapeshellarg($branch)), self::CLEANUP_GIT_PROBE_TIMEOUT);
1128+
if ( is_wp_error($delete) ) {
1129+
return $result;
1130+
}
1131+
}
1132+
1133+
return $result;
1134+
}
1135+
);
1136+
1137+
if ( is_wp_error($remove) ) {
1138+
$skipped[] = array(
1139+
'handle' => (string) ( $candidate['handle'] ?? '' ),
1140+
'repo' => $repo,
1141+
'branch' => $branch,
1142+
'path' => $wt_path,
1143+
'reason_code' => 'remove_failed',
1144+
'reason' => 'remove failed: ' . $remove->get_error_message(),
1145+
'size_bytes' => $size,
1146+
);
1147+
continue;
1148+
}
1149+
1150+
$removed[] = array_merge($validated, array( 'size_bytes' => $size ));
1151+
$bytes_reclaimed += max(0, $size);
1152+
}
1153+
1154+
return array(
1155+
'success' => true,
1156+
'dry_run' => false,
1157+
'candidates' => $candidates,
1158+
'removed' => $removed,
1159+
'skipped' => $skipped,
1160+
'summary' => array(
1161+
'processed' => $processed,
1162+
'removed' => count($removed),
1163+
'skipped' => count($skipped),
1164+
'bytes_reclaimed' => $bytes_reclaimed,
1165+
),
1166+
'evidence' => array(
1167+
'elapsed_ms' => (int) round(( microtime(true) - $started_at ) * 1000),
1168+
'direct_apply_plan' => true,
1169+
),
1170+
);
1171+
}
1172+
10791173
/**
10801174
* Re-run the bounded cleanup-eligible apply safety gates against the current state.
10811175
*

‎tests/smoke-worktree-cleanup-remove-guard.php‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77

88
declare( strict_types=1 );
99

10-
$source_path = __DIR__ . '/../inc/Workspace/WorkspaceWorktreeCleanupEngine.php';
11-
$source = file_get_contents($source_path);
10+
$source_path = __DIR__ . '/../inc/Workspace/WorkspaceWorktreeCleanupEngine.php';
11+
$service_path = __DIR__ . '/../inc/Workspace/CleanupRunService.php';
12+
$source = file_get_contents($source_path);
13+
$service = file_get_contents($service_path);
1214

13-
if ( false === $source ) {
14-
fwrite(STDERR, "Could not read cleanup engine source.\n");
15+
if ( false === $source || false === $service ) {
16+
fwrite(STDERR, "Could not read cleanup source.\n");
1517
exit(1);
1618
}
1719

@@ -35,6 +37,14 @@
3537
$failures[] = 'surviving worktree directory must return a row-level failure';
3638
}
3739

40+
if ( ! str_contains($source, 'apply_worktree_cleanup_plan_candidates') || ! str_contains($source, '$direct_apply_plan && ! $dry_run') ) {
41+
$failures[] = 'DB-backed cleanup apply must have a direct plan path';
42+
}
43+
44+
if ( ! str_contains($service, "'direct_apply_plan' => true") ) {
45+
$failures[] = 'CleanupRunService must request direct plan apply for worktree rows';
46+
}
47+
3848
if ( $failures ) {
3949
echo "Worktree cleanup remove guard failed:\n";
4050
foreach ( $failures as $failure ) {

0 commit comments

Comments
 (0)