Skip to content

Commit 95fe26a

Browse files
chubes4chubes4
committed
fix: harden cleanup apply resumability
1 parent c474155 commit 95fe26a

4 files changed

Lines changed: 203 additions & 20 deletions

File tree

‎inc/Cleanup/CleanupRemainingWorkSummary.php‎

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public static function from_items( array $items ): array {
5050
if ( 'artifact_cleanup' === $type && 'applied' !== $status ) {
5151
$summary['remaining_reclaimable_artifact_bytes'] += self::row_bytes($row, array( 'artifact_size_bytes', 'size_bytes' ));
5252
}
53-
if ( 'worktree_removal' === $type && in_array($status, array( 'pending', 'failed' ), true) ) {
53+
if ( 'worktree_removal' === $type && in_array($status, array( 'pending', 'failed', 'applying' ), true) ) {
5454
++$summary['remaining_safely_removable_worktrees'];
5555
}
5656
}
@@ -181,18 +181,22 @@ private static function recommended_commands( array $summary ): array {
181181
$commands = array();
182182
if ( (int) $summary['remaining_reclaimable_artifact_bytes'] > 0 ) {
183183
$commands[] = array(
184-
'bucket' => 'remaining_artifacts',
185-
'command' => 'studio wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --format=json',
186-
'apply' => 'studio wp datamachine-code workspace cleanup run --mode=artifacts',
187-
'destructive' => false,
184+
'bucket' => 'remaining_artifacts',
185+
'command' => 'studio wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --format=json',
186+
'apply' => 'studio wp datamachine-code workspace cleanup run --mode=artifacts',
187+
'destructive' => false,
188+
'apply_destructive' => true,
189+
'why' => 'Preview remaining artifact cleanup first; the apply command removes revalidated artifacts.',
188190
);
189191
}
190192
if ( (int) $summary['remaining_safely_removable_worktrees'] > 0 ) {
191193
$commands[] = array(
192-
'bucket' => 'remaining_worktrees',
193-
'command' => 'studio wp datamachine-code workspace worktree bounded-cleanup-eligible-apply --dry-run --limit=25',
194-
'apply' => 'studio wp datamachine-code workspace cleanup run --mode=retention',
195-
'destructive' => false,
194+
'bucket' => 'remaining_worktrees',
195+
'command' => 'studio wp datamachine-code workspace worktree bounded-cleanup-eligible-apply --dry-run --limit=25',
196+
'apply' => 'studio wp datamachine-code workspace cleanup run --mode=retention',
197+
'destructive' => false,
198+
'apply_destructive' => true,
199+
'why' => 'Preview cleanup-eligible worktrees first; the apply command removes revalidated worktrees.',
196200
);
197201
}
198202
foreach ( array_keys( (array) $summary['skipped_by_reason'] ) as $reason ) {
@@ -225,9 +229,24 @@ private static function command_for_reason( string $reason, string $bucket ): ar
225229
};
226230

227231
return array(
228-
'bucket' => $bucket . ':' . $reason,
229-
'command' => $command,
230-
'destructive' => false,
232+
'bucket' => $bucket . ':' . $reason,
233+
'command' => $command,
234+
'destructive' => false,
235+
'apply_destructive' => false,
236+
'why' => self::reason_remediation($reason),
231237
);
232238
}
239+
240+
private static function reason_remediation( string $reason ): string {
241+
return match ( $reason ) {
242+
'dirty_worktree' => 'Inspect dirty files before applying cleanup; artifact-only dirt may be removable through artifact cleanup, source dirt needs review.',
243+
'artifact_plan_mismatch', 'plan_mismatch' => 'Regenerate a fresh plan because the saved row no longer matches current filesystem or branch state.',
244+
'artifact_plan_not_current', 'artifact_already_removed' => 'Regenerate artifact cleanup evidence; the saved artifact row is no longer a current candidate.',
245+
'needs_metadata_reconcile' => 'Run metadata reconciliation so DMC can classify the worktree without a full cleanup scan.',
246+
'lifecycle_reconciliation_candidate' => 'Run lifecycle reconciliation to collect PR/merge signals before emitting removal rows.',
247+
'unpushed_commits' => 'Push, merge, or intentionally abandon commits before retrying cleanup.',
248+
'probe_timeout' => 'Retry the review path with a smaller bounded page or investigate the git probe timeout.',
249+
default => 'Run the review command to refresh evidence before applying cleanup.',
250+
};
251+
}
233252
}

‎inc/Cli/Commands/WorkspaceCommand.php‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,9 @@ private function render_cleanup_control_result( array $result, array $assoc_args
767767
WP_CLI::log(sprintf('%s: %s', ucfirst(str_replace('_', ' ', $key)), (string) $result[ $key ]));
768768
}
769769
}
770+
if ( ! empty($result['progress']) && is_array($result['progress']) ) {
771+
$this->render_cleanup_progress_summary( (array) $result['progress']);
772+
}
770773
if ( ! empty($result['remaining_work_summary']) && is_array($result['remaining_work_summary']) ) {
771774
$this->render_cleanup_remaining_work_summary( (array) $result['remaining_work_summary']);
772775
}
@@ -810,12 +813,41 @@ private function render_cleanup_remaining_work_summary( array $summary ): void {
810813
WP_CLI::log('Recommended next commands:');
811814
$rows = array_map(
812815
fn( $row ) => array(
813-
'bucket' => is_array($row) ? (string) ( $row['bucket'] ?? '' ) : '',
814-
'command' => is_array($row) ? (string) ( $row['command'] ?? '' ) : '',
816+
'bucket' => is_array($row) ? (string) ( $row['bucket'] ?? '' ) : '',
817+
'review_command' => is_array($row) ? (string) ( $row['command'] ?? '' ) : '',
818+
'apply_command' => is_array($row) ? (string) ( $row['apply'] ?? '' ) : '',
819+
'apply_destructive' => is_array($row) && ! empty($row['apply_destructive']) ? 'yes' : 'no',
815820
),
816821
array_slice($commands, 0, 10)
817822
);
818-
$this->format_items($rows, array( 'bucket', 'command' ), array( 'format' => 'table' ), 'bucket');
823+
$this->format_items($rows, array( 'bucket', 'review_command', 'apply_command', 'apply_destructive' ), array( 'format' => 'table' ), 'bucket');
824+
}
825+
}
826+
827+
private function render_cleanup_progress_summary( array $progress ): void {
828+
WP_CLI::log('');
829+
WP_CLI::log('Progress:');
830+
$this->format_items(
831+
array(
832+
array(
833+
'metric' => 'applying_rows',
834+
'value' => (int) ( $progress['applying_rows'] ?? 0 ),
835+
),
836+
array(
837+
'metric' => 'pending_or_failed',
838+
'value' => (int) ( $progress['pending_or_failed'] ?? 0 ),
839+
),
840+
array(
841+
'metric' => 'resumable',
842+
'value' => ! empty($progress['resumable']) ? 'yes' : 'no',
843+
),
844+
),
845+
array( 'metric', 'value' ),
846+
array( 'format' => 'table' ),
847+
'metric'
848+
);
849+
if ( ! empty($progress['note']) ) {
850+
WP_CLI::log((string) $progress['note']);
819851
}
820852
}
821853

‎inc/Workspace/CleanupRunService.php‎

Lines changed: 112 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class CleanupRunService {
1616

1717
private const DEFAULT_APPLY_LIMIT = 25;
1818
private const MAX_APPLY_LIMIT = 100;
19+
private const WORKTREE_APPLY_BATCH_LIMIT = 1;
1920

2021

2122

@@ -104,6 +105,7 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
104105
$artifact_batch = array_slice($artifact_rows, 0, $limit);
105106
$processed_rows += count($artifact_batch);
106107
$batch_type = 'artifact_cleanup';
108+
$this->mark_batch_applying($artifact_batch, $run_id, $batch_type, $limit, $remaining_rows);
107109
$results['artifact_cleanup'] = $this->workspace->worktree_cleanup_artifacts(
108110
array(
109111
'apply_plan' => array( 'candidates' => array_map(fn( $item ) => $item['evidence'], $artifact_batch) ),
@@ -116,9 +118,10 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
116118

117119
$remaining_capacity = max(0, $limit - $processed_rows);
118120
if ( $remaining_capacity > 0 && array() !== $worktree_rows ) {
119-
$worktree_batch = array_slice($worktree_rows, 0, $remaining_capacity);
121+
$worktree_batch = array_slice($worktree_rows, 0, min($remaining_capacity, self::WORKTREE_APPLY_BATCH_LIMIT));
120122
$processed_rows += count($worktree_batch);
121123
$batch_type = '' === $batch_type ? 'worktree_removal' : 'mixed';
124+
$this->mark_batch_applying($worktree_batch, $run_id, $batch_type, $limit, $remaining_rows);
122125
$results['worktree_removal'] = $this->workspace->worktree_cleanup_merged(
123126
array(
124127
'apply_plan' => array( 'candidates' => array_map(fn( $item ) => $item['evidence'], $worktree_batch) ),
@@ -195,13 +198,15 @@ public function status( string $run_id ): array|\WP_Error {
195198
$summary['items_by_status'][ $status ] = ( $summary['items_by_status'][ $status ] ?? 0 ) + 1;
196199
$summary['items_by_type'][ $type ] = ( $summary['items_by_type'][ $type ] ?? 0 ) + 1;
197200
$summary['bytes_reclaimed'] += max(0, (int) ( $item['bytes_reclaimed'] ?? 0 ));
198-
if ( in_array($status, array( 'pending', 'failed' ), true) ) {
201+
if ( in_array($status, array( 'pending', 'failed', 'applying' ), true) ) {
199202
++$summary['pending_or_failed'];
200203
}
201204
}
202205
ksort($summary['items_by_status']);
203206
ksort($summary['items_by_type']);
204207

208+
$progress = $this->run_progress($run, $items, $summary);
209+
205210
return array(
206211
'success' => true,
207212
'state' => (string) ( $run['status'] ?? 'unknown' ),
@@ -210,7 +215,8 @@ public function status( string $run_id ): array|\WP_Error {
210215
'mode' => $run['mode'] ?? '',
211216
'run' => $run,
212217
'summary' => $summary,
213-
'remaining_work_summary' => CleanupRemainingWorkSummary::from_items($items),
218+
'progress' => $progress,
219+
'remaining_work_summary' => $this->remaining_work_summary($run_id, $items, $progress),
214220
);
215221
}
216222

@@ -285,7 +291,109 @@ private function plan_items( array $plan ): array {
285291
}
286292

287293
private function pending_rows_of_type( array $items, string $type ): array {
288-
return array_values(array_filter($items, fn( $item ) => (string) ( $item['item_type'] ?? '' ) === $type && in_array( (string) ( $item['status'] ?? '' ), array( 'pending', 'failed' ), true)));
294+
return array_values(array_filter($items, fn( $item ) => (string) ( $item['item_type'] ?? '' ) === $type && in_array( (string) ( $item['status'] ?? '' ), array( 'pending', 'failed', 'applying' ), true)));
295+
}
296+
297+
/**
298+
* Mark rows as in-progress before invoking destructive cleanup so interrupted
299+
* operator runs leave a visible, resumable checkpoint instead of silent state.
300+
*
301+
* @param array<int,array<string,mixed>> $items Batch rows.
302+
* @param string $run_id Run ID.
303+
* @param string $batch_type Batch type label.
304+
* @param int $limit Requested apply limit.
305+
* @param int $remaining_rows Rows remaining before this batch.
306+
*/
307+
private function mark_batch_applying( array $items, string $run_id, string $batch_type, int $limit, int $remaining_rows ): void {
308+
$started_at = gmdate('Y-m-d H:i:s');
309+
foreach ( $items as $item ) {
310+
$this->repository->update_item(
311+
(int) $item['id'],
312+
array(
313+
'status' => 'applying',
314+
'evidence' => array_merge(
315+
(array) ( $item['evidence'] ?? array() ),
316+
array(
317+
'applying_started_at' => $started_at,
318+
'applying_batch_type' => $batch_type,
319+
)
320+
),
321+
)
322+
);
323+
}
324+
325+
$this->repository->update_run(
326+
$run_id,
327+
array(
328+
'summary' => array(
329+
'applying_batch' => array(
330+
'type' => $batch_type,
331+
'limit' => $limit,
332+
'row_count' => count($items),
333+
'remaining_before' => $remaining_rows,
334+
'started_at' => $started_at,
335+
),
336+
),
337+
)
338+
);
339+
}
340+
341+
/**
342+
* Build operator progress metadata for status/evidence output.
343+
*
344+
* @param array<string,mixed> $run Run row.
345+
* @param array<int,array<string,mixed>> $items Item rows.
346+
* @param array<string,mixed> $summary Aggregate summary.
347+
* @return array<string,mixed>
348+
*/
349+
private function run_progress( array $run, array $items, array $summary ): array {
350+
$applying = array_values(array_filter($items, fn( $item ) => 'applying' === (string) ( $item['status'] ?? '' )));
351+
$examples = array_slice(array_map(fn( $item ) => array(
352+
'handle' => (string) ( $item['handle'] ?? '' ),
353+
'type' => (string) ( $item['item_type'] ?? '' ),
354+
), $applying), 0, 3);
355+
356+
$started_at = (string) ( $run['started_at'] ?? '' );
357+
$age = '' !== $started_at ? max(0, time() - strtotime($started_at)) : 0;
358+
$run_status = (string) ( $run['status'] ?? '' );
359+
$resumable = (int) ( $summary['pending_or_failed'] ?? 0 ) > 0 && in_array($run_status, array( 'applying', 'needs_resume' ), true);
360+
361+
return array(
362+
'applying_rows' => count($applying),
363+
'applying_examples' => $examples,
364+
'pending_or_failed' => (int) ( $summary['pending_or_failed'] ?? 0 ),
365+
'started_at' => $started_at,
366+
'age_seconds' => $age,
367+
'resumable' => $resumable,
368+
'note' => count($applying) > 0 ? 'Rows marked applying are safe to retry with workspace cleanup resume if the previous apply process was interrupted.' : '',
369+
);
370+
}
371+
372+
/**
373+
* Build remaining-work summary and prepend the current run resume command.
374+
*
375+
* @param string $run_id Run ID.
376+
* @param array<int,array<string,mixed>> $items Item rows.
377+
* @param array<string,mixed> $progress Progress metadata.
378+
* @return array<string,mixed>
379+
*/
380+
private function remaining_work_summary( string $run_id, array $items, array $progress ): array {
381+
$summary = CleanupRemainingWorkSummary::from_items($items);
382+
if ( ! empty($progress['resumable']) ) {
383+
array_unshift(
384+
$summary['recommended_commands'],
385+
array(
386+
'bucket' => 'current_run_resume',
387+
'command' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
388+
'apply' => sprintf('studio wp datamachine-code workspace cleanup resume %s --limit=%d', $run_id, self::DEFAULT_APPLY_LIMIT),
389+
'destructive' => false,
390+
'apply_destructive' => true,
391+
'why' => 'Resume the reviewed DB-backed cleanup run from persisted pending/failed/applying rows.',
392+
)
393+
);
394+
}
395+
396+
return $summary;
289397
}
290398

291399
private function apply_limit( array $opts ): int {

‎tests/smoke-cleanup-run-storage.php‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,20 +226,26 @@ function datamachine_code_cleanup_run_assert( bool $condition, string $message )
226226
datamachine_code_cleanup_run_assert(3 === count($status['remaining_work_summary']['blocked_resolvers_by_reason']['needs_metadata_reconcile']['examples'] ?? array()), 'blocked resolver examples are truncated');
227227
datamachine_code_cleanup_run_assert(24 === (int) ( $status['remaining_work_summary']['remaining_reclaimable_artifact_bytes'] ?? 0 ), 'status reports remaining reclaimable artifact bytes');
228228
datamachine_code_cleanup_run_assert(2 === (int) ( $status['remaining_work_summary']['remaining_safely_removable_worktrees'] ?? 0 ), 'status reports remaining safely removable worktrees');
229+
datamachine_code_cleanup_run_assert(false === (bool) ( $status['progress']['resumable'] ?? true ), 'planned status is not resumable before apply starts');
229230

230231
$apply = $service->apply($plan['run_id']);
231232
datamachine_code_cleanup_run_assert(! is_wp_error($apply), 'apply succeeds');
233+
datamachine_code_cleanup_run_assert('needs_resume' === (string) ( $apply['status'] ?? '' ), 'apply pauses after bounded worktree-removal batch');
232234

233235
$evidence = $service->evidence($plan['run_id']);
234236
datamachine_code_cleanup_run_assert(2 === (int) ( $evidence['summary']['items_by_status']['applied'] ?? 0 ), 'evidence reflects applied rows');
235-
datamachine_code_cleanup_run_assert(2 === (int) ( $evidence['summary']['items_by_status']['skipped'] ?? 0 ), 'evidence reflects skipped rows');
237+
datamachine_code_cleanup_run_assert(1 === (int) ( $evidence['summary']['items_by_status']['skipped'] ?? 0 ), 'evidence reflects skipped rows from first bounded batch');
236238
datamachine_code_cleanup_run_assert(35 === (int) ( $evidence['summary']['bytes_reclaimed'] ?? 0 ), 'evidence aggregates reclaimed bytes');
237239
datamachine_code_cleanup_run_assert(1 === (int) ( $evidence['remaining_work_summary']['applied_by_type']['artifact_cleanup']['count'] ?? 0 ), 'evidence groups applied artifact rows');
238240
datamachine_code_cleanup_run_assert(15 === (int) ( $evidence['remaining_work_summary']['applied_by_type']['artifact_cleanup']['bytes_reclaimed'] ?? 0 ), 'evidence reports artifact bytes reclaimed');
239241
datamachine_code_cleanup_run_assert(1 === (int) ( $evidence['remaining_work_summary']['skipped_by_reason']['plan_mismatch']['count'] ?? 0 ), 'evidence groups skipped plan mismatches');
240242
datamachine_code_cleanup_run_assert('demo@stale-artifact' === (string) ( $evidence['remaining_work_summary']['skipped_by_reason']['plan_mismatch']['examples'][0]['handle'] ?? '' ), 'skipped examples include representative handle');
241243
datamachine_code_cleanup_run_assert(4 === (int) ( $evidence['remaining_work_summary']['blocked_resolvers_by_reason']['needs_metadata_reconcile']['count'] ?? 0 ), 'evidence keeps blocked resolver bucket');
242244
datamachine_code_cleanup_run_assert(str_contains((string) wp_json_encode($evidence['remaining_work_summary']['recommended_commands']), 'workspace worktree reconcile-metadata'), 'summary recommends next DMC commands');
245+
datamachine_code_cleanup_run_assert(str_contains((string) wp_json_encode($evidence['remaining_work_summary']['recommended_commands']), 'apply_destructive'), 'summary labels destructive apply commands separately from review commands');
246+
247+
$done = $service->resume($plan['run_id']);
248+
datamachine_code_cleanup_run_assert('completed' === (string) ( $done['status'] ?? '' ), 'resume completes final bounded worktree-removal batch');
243249

244250
$bounded_workspace = new DataMachineCodeCleanupRunFakeWorkspace();
245251
$bounded_service = new \DataMachineCode\Workspace\CleanupRunService($repo, $bounded_workspace);
@@ -250,9 +256,27 @@ function datamachine_code_cleanup_run_assert( bool $condition, string $message )
250256
datamachine_code_cleanup_run_assert('needs_resume' === (string) ( $bounded_apply['status'] ?? '' ), 'bounded apply pauses when rows remain');
251257
datamachine_code_cleanup_run_assert(1 === (int) ( $bounded_apply['batch']['processed_rows'] ?? 0 ), 'bounded apply processes one row');
252258
datamachine_code_cleanup_run_assert(str_contains((string) ( $bounded_apply['next']['resume_command'] ?? '' ), 'workspace cleanup resume'), 'bounded apply returns resume command');
259+
datamachine_code_cleanup_run_assert(str_contains((string) wp_json_encode($bounded_apply['remaining_work_summary']['recommended_commands'] ?? array()), 'current_run_resume'), 'bounded apply recommends resuming the current reviewed run');
253260
datamachine_code_cleanup_run_assert(1 === count($bounded_workspace->artifact_calls), 'bounded apply only runs artifact cleanup first');
254261
datamachine_code_cleanup_run_assert(0 === count($bounded_workspace->worktree_calls), 'bounded apply defers worktree cleanup until artifacts drain');
255262

263+
$applying_repo = new \DataMachineCode\Storage\CleanupRunRepository();
264+
$applying_plan = ( new \DataMachineCode\Workspace\CleanupRunService($applying_repo, new DataMachineCodeCleanupRunFakeWorkspace()) )->plan(array( 'mode' => 'retention' ));
265+
datamachine_code_cleanup_run_assert(! is_wp_error($applying_plan), 'applying status plan succeeds');
266+
$GLOBALS['wpdb']->runs[ $applying_plan['run_id'] ]['status'] = 'applying';
267+
$GLOBALS['wpdb']->runs[ $applying_plan['run_id'] ]['started_at'] = gmdate('Y-m-d H:i:s', time() - 120);
268+
foreach ( $GLOBALS['wpdb']->items as &$item ) {
269+
if ((string) ( $item['run_id'] ?? '' ) === (string) $applying_plan['run_id'] && 'worktree_removal' === (string) ( $item['item_type'] ?? '' ) ) {
270+
$item['status'] = 'applying';
271+
break;
272+
}
273+
}
274+
unset($item);
275+
$applying_status = ( new \DataMachineCode\Workspace\CleanupRunService($applying_repo, new DataMachineCodeCleanupRunFakeWorkspace()) )->status((string) $applying_plan['run_id']);
276+
datamachine_code_cleanup_run_assert(1 === (int) ( $applying_status['summary']['items_by_status']['applying'] ?? 0 ), 'status reports interrupted applying rows');
277+
datamachine_code_cleanup_run_assert(true === (bool) ( $applying_status['progress']['resumable'] ?? false ), 'applying rows are resumable');
278+
datamachine_code_cleanup_run_assert(2 === (int) ( $applying_status['remaining_work_summary']['remaining_safely_removable_worktrees'] ?? 0 ), 'applying worktree rows still count as remaining removable worktrees');
279+
256280
$bounded_service->resume($bounded_plan['run_id'], array( 'limit' => 1 ));
257281
$bounded_service->resume($bounded_plan['run_id'], array( 'limit' => 1 ));
258282
$bounded_done = $bounded_service->resume($bounded_plan['run_id'], array( 'limit' => 1 ));

0 commit comments

Comments
 (0)