Skip to content

Commit 7001ddd

Browse files
authored
Fix cleanup resume limit batching (#651)
1 parent 0c1deab commit 7001ddd

2 files changed

Lines changed: 38 additions & 8 deletions

File tree

inc/Workspace/CleanupRunService.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ 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;
2019

2120

2221

@@ -98,6 +97,8 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
9897
$worktree_rows = $this->pending_rows_of_type($items, 'worktree_removal');
9998
$batch_type = '';
10099
$processed_rows = 0;
100+
$applied_rows = 0;
101+
$skipped_rows = 0;
101102
$remaining_rows = max(0, count($artifact_rows) + count($worktree_rows));
102103
$results = array();
103104

@@ -114,11 +115,13 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
114115
)
115116
);
116117
$this->record_apply_result($artifact_batch, $results['artifact_cleanup'], 'removed');
118+
$applied_rows += count((array) ( $results['artifact_cleanup']['removed'] ?? array() ));
119+
$skipped_rows += count((array) ( $results['artifact_cleanup']['skipped'] ?? array() ));
117120
}
118121

119122
$remaining_capacity = max(0, $limit - $processed_rows);
120123
if ( $remaining_capacity > 0 && array() !== $worktree_rows ) {
121-
$worktree_batch = array_slice($worktree_rows, 0, min($remaining_capacity, self::WORKTREE_APPLY_BATCH_LIMIT));
124+
$worktree_batch = array_slice($worktree_rows, 0, $remaining_capacity);
122125
$processed_rows += count($worktree_batch);
123126
$batch_type = '' === $batch_type ? 'worktree_removal' : 'mixed';
124127
$this->mark_batch_applying($worktree_batch, $run_id, $batch_type, $limit, $remaining_rows);
@@ -130,6 +133,8 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
130133
)
131134
);
132135
$this->record_apply_result($worktree_batch, $results['worktree_removal'], 'removed');
136+
$applied_rows += count((array) ( $results['worktree_removal']['removed'] ?? array() ));
137+
$skipped_rows += count((array) ( $results['worktree_removal']['skipped'] ?? array() ));
133138
}
134139

135140
$status = $this->status($run_id);
@@ -151,11 +156,17 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
151156
return $status;
152157
}
153158

159+
$next_command = $pending_or_fail > 0 ? sprintf('studio wp datamachine-code workspace cleanup resume %s --limit=%d', $run_id, $limit) : null;
160+
154161
return array(
155162
'success' => true,
156163
'state' => $next_status,
157164
'run_id' => $run_id,
158165
'status' => $next_status,
166+
'processed' => $processed_rows,
167+
'applied' => $applied_rows,
168+
'skipped' => $skipped_rows,
169+
'next_command' => $next_command,
159170
'batch' => array(
160171
'type' => $batch_type,
161172
'limit' => $limit,
@@ -167,7 +178,7 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
167178
'summary' => $status['summary'] ?? array(),
168179
'remaining_work_summary' => $status['remaining_work_summary'] ?? array(),
169180
'next' => $pending_or_fail > 0 ? array(
170-
'resume_command' => sprintf('studio wp datamachine-code workspace cleanup resume %s --limit=%d', $run_id, $limit),
181+
'resume_command' => $next_command,
171182
'pending_rows' => $pending_or_fail,
172183
) : null,
173184
);

tests/smoke-cleanup-run-storage.php

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,15 @@ function datamachine_code_cleanup_run_assert( bool $condition, string $message )
230230

231231
$apply = $service->apply($plan['run_id']);
232232
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');
233+
datamachine_code_cleanup_run_assert('completed' === (string) ( $apply['status'] ?? '' ), 'apply drains worktree rows up to the default limit');
234+
datamachine_code_cleanup_run_assert(4 === (int) ( $apply['processed'] ?? 0 ), 'apply reports processed row count');
235+
datamachine_code_cleanup_run_assert(2 === (int) ( $apply['applied'] ?? 0 ), 'apply reports applied row count');
236+
datamachine_code_cleanup_run_assert(2 === (int) ( $apply['skipped'] ?? 0 ), 'apply reports skipped row count');
237+
datamachine_code_cleanup_run_assert(null === ( $apply['next_command'] ?? null ), 'completed apply omits next command');
234238

235239
$evidence = $service->evidence($plan['run_id']);
236240
datamachine_code_cleanup_run_assert(2 === (int) ( $evidence['summary']['items_by_status']['applied'] ?? 0 ), 'evidence reflects applied rows');
237-
datamachine_code_cleanup_run_assert(1 === (int) ( $evidence['summary']['items_by_status']['skipped'] ?? 0 ), 'evidence reflects skipped rows from first bounded batch');
241+
datamachine_code_cleanup_run_assert(2 === (int) ( $evidence['summary']['items_by_status']['skipped'] ?? 0 ), 'evidence reflects skipped rows from bounded batch');
238242
datamachine_code_cleanup_run_assert(35 === (int) ( $evidence['summary']['bytes_reclaimed'] ?? 0 ), 'evidence aggregates reclaimed bytes');
239243
datamachine_code_cleanup_run_assert(1 === (int) ( $evidence['remaining_work_summary']['applied_by_type']['artifact_cleanup']['count'] ?? 0 ), 'evidence groups applied artifact rows');
240244
datamachine_code_cleanup_run_assert(15 === (int) ( $evidence['remaining_work_summary']['applied_by_type']['artifact_cleanup']['bytes_reclaimed'] ?? 0 ), 'evidence reports artifact bytes reclaimed');
@@ -244,9 +248,6 @@ function datamachine_code_cleanup_run_assert( bool $condition, string $message )
244248
datamachine_code_cleanup_run_assert(str_contains((string) wp_json_encode($evidence['remaining_work_summary']['recommended_commands']), 'workspace worktree reconcile-metadata --dry-run --limit=25 --offset=0 --until-budget=30s --format=json'), 'summary recommends bounded next DMC commands');
245249
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');
246250

247-
$done = $service->resume($plan['run_id']);
248-
datamachine_code_cleanup_run_assert('completed' === (string) ( $done['status'] ?? '' ), 'resume completes final bounded worktree-removal batch');
249-
250251
$bounded_workspace = new DataMachineCodeCleanupRunFakeWorkspace();
251252
$bounded_service = new \DataMachineCode\Workspace\CleanupRunService($repo, $bounded_workspace);
252253
$bounded_plan = $bounded_service->plan(array( 'mode' => 'retention' ));
@@ -260,6 +261,24 @@ function datamachine_code_cleanup_run_assert( bool $condition, string $message )
260261
datamachine_code_cleanup_run_assert(1 === count($bounded_workspace->artifact_calls), 'bounded apply only runs artifact cleanup first');
261262
datamachine_code_cleanup_run_assert(0 === count($bounded_workspace->worktree_calls), 'bounded apply defers worktree cleanup until artifacts drain');
262263

264+
$multi_workspace = new DataMachineCodeCleanupRunFakeWorkspace();
265+
$multi_service = new \DataMachineCode\Workspace\CleanupRunService($repo, $multi_workspace);
266+
$multi_plan = $multi_service->plan(array( 'mode' => 'retention' ));
267+
datamachine_code_cleanup_run_assert(! is_wp_error($multi_plan), 'multi-row resume plan succeeds');
268+
$multi_apply = $multi_service->apply($multi_plan['run_id'], array( 'limit' => 2 ));
269+
datamachine_code_cleanup_run_assert('needs_resume' === (string) ( $multi_apply['status'] ?? '' ), 'multi-row apply pauses after artifact rows when worktrees remain');
270+
datamachine_code_cleanup_run_assert(2 === (int) ( $multi_apply['processed'] ?? 0 ), 'multi-row apply reports processed artifact rows');
271+
datamachine_code_cleanup_run_assert(str_contains((string) ( $multi_apply['next_command'] ?? '' ), 'workspace cleanup resume'), 'multi-row apply exposes top-level next command');
272+
$multi_resume = $multi_service->resume($multi_plan['run_id'], array( 'limit' => 2 ));
273+
datamachine_code_cleanup_run_assert('completed' === (string) ( $multi_resume['status'] ?? '' ), 'resume drains multiple worktree rows up to the requested limit');
274+
datamachine_code_cleanup_run_assert('worktree_removal' === (string) ( $multi_resume['batch']['type'] ?? '' ), 'multi-row resume reports worktree batch type');
275+
datamachine_code_cleanup_run_assert(2 === (int) ( $multi_resume['processed'] ?? 0 ), 'multi-row resume reports processed worktree rows');
276+
datamachine_code_cleanup_run_assert(1 === (int) ( $multi_resume['applied'] ?? 0 ), 'multi-row resume reports applied worktree rows');
277+
datamachine_code_cleanup_run_assert(1 === (int) ( $multi_resume['skipped'] ?? 0 ), 'multi-row resume reports skipped worktree rows');
278+
datamachine_code_cleanup_run_assert(null === ( $multi_resume['next_command'] ?? null ), 'multi-row resume omits next command when no rows remain');
279+
datamachine_code_cleanup_run_assert(1 === count($multi_workspace->worktree_calls), 'multi-row resume applies worktree rows in one safety-revalidated call');
280+
datamachine_code_cleanup_run_assert(2 === count($multi_workspace->worktree_calls[0]['apply_plan']['candidates'] ?? array()), 'multi-row resume sends both eligible worktree candidates');
281+
263282
$applying_repo = new \DataMachineCode\Storage\CleanupRunRepository();
264283
$applying_plan = ( new \DataMachineCode\Workspace\CleanupRunService($applying_repo, new DataMachineCodeCleanupRunFakeWorkspace()) )->plan(array( 'mode' => 'retention' ));
265284
datamachine_code_cleanup_run_assert(! is_wp_error($applying_plan), 'applying status plan succeeds');

0 commit comments

Comments
 (0)