Skip to content

Commit 841d3ac

Browse files
chubes4chubes4
authored
Merge pull request #683 from Extra-Chill/fix/issue-678-cleanup-snapshots
Fix cleanup pagination snapshots
2 parents 2cac59a + b8b1e8d commit 841d3ac

5 files changed

Lines changed: 227 additions & 122 deletions

File tree

‎inc/Abilities/WorkspaceAbilities.php‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3639,18 +3639,6 @@ public static function workspaceCleanupRun( array $input ): array|\WP_Error {
36393639
if ( isset($input['older_than']) && '' !== trim( (string) $input['older_than']) ) {
36403640
$params['worktree_older_than'] = trim( (string) $input['older_than']);
36413641
}
3642-
if ( 'artifacts' === $mode ) {
3643-
if ( isset($input['limit']) ) {
3644-
$params['limit'] = (int) $input['limit'];
3645-
}
3646-
if ( isset($input['offset']) ) {
3647-
$params['offset'] = (int) $input['offset'];
3648-
}
3649-
if ( ! empty($input['exhaustive']) ) {
3650-
$params['exhaustive'] = true;
3651-
}
3652-
}
3653-
36543642
$context = array();
36553643
if ( isset($input['user_id']) ) {
36563644
$context['user_id'] = (int) $input['user_id'];

‎inc/Cli/Commands/WorkspaceCommand.php‎

Lines changed: 66 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -654,9 +654,9 @@ public function adopt_repo( array $args, array $assoc_args ): void {
654654
* # Review artifact cleanup synchronously (bounded; default limit=100)
655655
* wp datamachine-code workspace cleanup run --mode=artifacts --dry-run
656656
*
657-
* # Walk a huge workspace in 100-worktree pages
658-
* wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --offset=0 --format=json
659-
* wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --offset=100 --format=json
657+
* # Persist a snapshot-safe artifact cleanup plan, then apply it by run ID
658+
* wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --format=json
659+
* wp datamachine-code workspace cleanup apply cleanup-run-20260504120000-abc123
660660
*
661661
* # Full audit (slow on huge workspaces)
662662
* wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --exhaustive --format=json
@@ -777,11 +777,11 @@ private function attach_cleanup_run_commands( array $result, string $mode ): arr
777777
}
778778

779779
$result['commands'] = array(
780-
'drain_parent' => sprintf('studio wp datamachine drain --job-id=%d', $job_id),
781-
'status' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
782-
'status_verbose' => sprintf('studio wp datamachine-code workspace cleanup status %s --verbose --format=json', $run_id),
783-
'one_command_drain' => sprintf('studio wp datamachine-code workspace cleanup run --mode=%s --drain --format=json', $mode),
784-
'bytes_verification' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
780+
'drain_parent' => sprintf('studio wp datamachine drain --job-id=%d', $job_id),
781+
'status' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
782+
'status_verbose' => sprintf('studio wp datamachine-code workspace cleanup status %s --verbose --format=json', $run_id),
783+
'one_command_drain' => sprintf('studio wp datamachine-code workspace cleanup run --mode=%s --drain --format=json', $mode),
784+
'bytes_verification' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
785785
);
786786

787787
return $result;
@@ -805,8 +805,8 @@ private function drain_cleanup_run_to_status( array $result, array $assoc_args )
805805
return $result;
806806
}
807807

808-
$commands = array();
809-
$errors = array();
808+
$commands = array();
809+
$errors = array();
810810
$max_passes = 10;
811811

812812
$parent_command = sprintf('datamachine drain --job-id=%d', $job_id);
@@ -823,7 +823,7 @@ private function drain_cleanup_run_to_status( array $result, array $assoc_args )
823823
break;
824824
}
825825

826-
$children = (array) ( $status['evidence']['children'] ?? array() );
826+
$children = (array) ( $status['evidence']['children'] ?? array() );
827827
$active_child_ids = array_values(
828828
array_unique(
829829
array_filter(
@@ -850,17 +850,17 @@ private function drain_cleanup_run_to_status( array $result, array $assoc_args )
850850
}
851851
}
852852

853-
$final = $this->cleanup_run_evidence_store()->read($run_id, false, ! empty($assoc_args['verbose']));
854-
$output = $final instanceof \WP_Error ? $result : $final;
853+
$final = $this->cleanup_run_evidence_store()->read($run_id, false, ! empty($assoc_args['verbose']));
854+
$output = $final instanceof \WP_Error ? $result : $final;
855855
$output['initial_run'] = $result;
856856
$output['drain'] = array(
857-
'success' => array() === $errors,
858-
'commands' => $commands,
859-
'errors' => $errors,
860-
'verify_command' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
861-
'bytes_reclaimed' => (int) ( $output['cleanup_items']['bytes_reclaimed'] ?? 0 ),
862-
'freed_human' => (string) ( $output['cleanup_items']['freed_human'] ?? $this->format_bytes(0) ),
863-
'completion_state' => (string) ( $output['state'] ?? 'unknown' ),
857+
'success' => array() === $errors,
858+
'commands' => $commands,
859+
'errors' => $errors,
860+
'verify_command' => sprintf('studio wp datamachine-code workspace cleanup status %s --format=json', $run_id),
861+
'bytes_reclaimed' => (int) ( $output['cleanup_items']['bytes_reclaimed'] ?? 0 ),
862+
'freed_human' => (string) ( $output['cleanup_items']['freed_human'] ?? $this->format_bytes(0) ),
863+
'completion_state' => (string) ( $output['state'] ?? 'unknown' ),
864864
);
865865

866866
return $output;
@@ -873,10 +873,6 @@ private function drain_cleanup_run_to_status( array $result, array $assoc_args )
873873
* @return string Empty string on success.
874874
*/
875875
private function run_wp_cli_command( string $command ): string {
876-
if ( ! method_exists('WP_CLI', 'runcommand') ) {
877-
return 'WP_CLI::runcommand is unavailable; run the reported drain commands manually.';
878-
}
879-
880876
try {
881877
WP_CLI::runcommand(
882878
$command,
@@ -903,17 +899,6 @@ private function cleanup_run_input( string $mode, array $assoc_args ): array {
903899
if ( isset($assoc_args['older-than']) && '' !== trim( (string) $assoc_args['older-than']) ) {
904900
$input['older_than'] = trim( (string) $assoc_args['older-than']);
905901
}
906-
if ( 'artifacts' === $mode ) {
907-
if ( isset($assoc_args['limit']) ) {
908-
$input['limit'] = (int) $assoc_args['limit'];
909-
}
910-
if ( isset($assoc_args['offset']) ) {
911-
$input['offset'] = (int) $assoc_args['offset'];
912-
}
913-
if ( ! empty($assoc_args['exhaustive']) ) {
914-
$input['exhaustive'] = true;
915-
}
916-
}
917902

918903
return $input;
919904
}
@@ -931,6 +916,29 @@ private function run_cleanup_plan( array $assoc_args ): void {
931916
return;
932917
}
933918

919+
$input = $this->cleanup_plan_input($mode, $assoc_args);
920+
if ( 'json' !== (string) ( $assoc_args['format'] ?? 'table' ) ) {
921+
$profile = ! empty($input['include_artifacts']) ? 'includes artifact scan' : 'local worktree merge signals';
922+
WP_CLI::log(sprintf('Planning cleanup (%s; %s)...', $mode, $profile));
923+
}
924+
925+
$result = $ability->execute($input);
926+
if ( is_wp_error($result) ) {
927+
WP_CLI::error($result->get_error_message());
928+
return;
929+
}
930+
931+
$this->render_cleanup_plan_result($result, $assoc_args);
932+
}
933+
934+
/**
935+
* Normalize cleanup plan input shared by `cleanup plan` and dry-run `cleanup run`.
936+
*
937+
* @param string $mode Cleanup mode.
938+
* @param array $assoc_args CLI associative args.
939+
* @return array<string,mixed>
940+
*/
941+
private function cleanup_plan_input( string $mode, array $assoc_args ): array {
934942
$include_artifacts = 'artifacts' === $mode || ! empty($assoc_args['include-artifacts']);
935943
$include_worktrees = 'artifacts' !== $mode;
936944
$input = array(
@@ -945,18 +953,8 @@ private function run_cleanup_plan( array $assoc_args ): void {
945953
if ( isset($assoc_args['force']) ) {
946954
$input['force_artifact_cleanup'] = (bool) $assoc_args['force'];
947955
}
948-
if ( 'json' !== (string) ( $assoc_args['format'] ?? 'table' ) ) {
949-
$profile = $include_artifacts ? 'includes artifact scan' : 'local worktree merge signals';
950-
WP_CLI::log(sprintf('Planning cleanup (%s; %s)...', $mode, $profile));
951-
}
952956

953-
$result = $ability->execute($input);
954-
if ( is_wp_error($result) ) {
955-
WP_CLI::error($result->get_error_message());
956-
return;
957-
}
958-
959-
$this->render_cleanup_plan_result($result, $assoc_args);
957+
return $input;
960958
}
961959

962960
private function run_cleanup_control_ability( string $operation, string $run_id, array $assoc_args ): void {
@@ -1008,28 +1006,13 @@ private function run_cleanup_review( array $assoc_args ): void {
10081006
return;
10091007

10101008
case 'artifacts':
1011-
$ability = wp_get_ability('datamachine-code/workspace-worktree-cleanup-artifacts');
1012-
$artifact_input = array(
1013-
'dry_run' => true,
1014-
'force' => ! empty($assoc_args['force']),
1015-
);
1016-
if ( isset($assoc_args['limit']) ) {
1017-
$artifact_input['limit'] = (int) $assoc_args['limit'];
1018-
}
1019-
if ( isset($assoc_args['offset']) ) {
1020-
$artifact_input['offset'] = (int) $assoc_args['offset'];
1021-
}
1022-
if ( ! empty($assoc_args['exhaustive']) ) {
1023-
$artifact_input['exhaustive'] = true;
1024-
}
1025-
if ( ! empty($assoc_args['safety-probes']) ) {
1026-
$artifact_input['safety_probes'] = true;
1027-
}
1028-
if ( isset($assoc_args['sort']) && '' !== trim( (string) $assoc_args['sort']) ) {
1029-
$artifact_input['sort'] = trim( (string) $assoc_args['sort']);
1009+
$ability = wp_get_ability('datamachine-code/workspace-cleanup-plan');
1010+
$result = $ability ? $ability->execute($this->cleanup_plan_input($mode, $assoc_args)) : new \WP_Error('cleanup_plan_ability_missing', 'Workspace cleanup plan ability not registered.');
1011+
if ( is_wp_error($result) ) {
1012+
WP_CLI::error($result->get_error_message());
1013+
return;
10301014
}
1031-
$result = $ability ? $ability->execute($artifact_input) : new \WP_Error('artifact_cleanup_ability_missing', 'Artifact cleanup ability not registered.');
1032-
$this->render_worktree_artifact_cleanup_result_from_ability($result, $assoc_args);
1015+
$this->render_cleanup_plan_result($result, $assoc_args);
10331016
return;
10341017

10351018
case 'emergency':
@@ -1076,14 +1059,6 @@ private function render_worktree_cleanup_result_from_ability( array|\WP_Error $r
10761059
$this->render_worktree_cleanup_result($result, $assoc_args);
10771060
}
10781061

1079-
private function render_worktree_artifact_cleanup_result_from_ability( array|\WP_Error $result, array $assoc_args ): void {
1080-
if ( is_wp_error($result) ) {
1081-
WP_CLI::error($result->get_error_message());
1082-
return;
1083-
}
1084-
$this->render_worktree_artifact_cleanup_result($result, $assoc_args);
1085-
}
1086-
10871062
private function render_worktree_emergency_cleanup_result_from_ability( array|\WP_Error $result, array $assoc_args ): void {
10881063
if ( is_wp_error($result) ) {
10891064
WP_CLI::error($result->get_error_message());
@@ -1310,10 +1285,22 @@ private function render_cleanup_drain_summary( array $drain ): void {
13101285
WP_CLI::log('Drain summary:');
13111286
$this->format_items(
13121287
array(
1313-
array( 'metric' => 'success', 'value' => ! empty($drain['success']) ? 'yes' : 'no' ),
1314-
array( 'metric' => 'completion_state', 'value' => (string) ( $drain['completion_state'] ?? 'unknown' ) ),
1315-
array( 'metric' => 'bytes_reclaimed', 'value' => $this->format_bytes($drain['bytes_reclaimed'] ?? 0) ),
1316-
array( 'metric' => 'verify_command', 'value' => (string) ( $drain['verify_command'] ?? '' ) ),
1288+
array(
1289+
'metric' => 'success',
1290+
'value' => ! empty($drain['success']) ? 'yes' : 'no',
1291+
),
1292+
array(
1293+
'metric' => 'completion_state',
1294+
'value' => (string) ( $drain['completion_state'] ?? 'unknown' ),
1295+
),
1296+
array(
1297+
'metric' => 'bytes_reclaimed',
1298+
'value' => $this->format_bytes($drain['bytes_reclaimed'] ?? 0),
1299+
),
1300+
array(
1301+
'metric' => 'verify_command',
1302+
'value' => (string) ( $drain['verify_command'] ?? '' ),
1303+
),
13171304
),
13181305
array( 'metric', 'value' ),
13191306
array( 'format' => 'table' ),

‎inc/Tasks/WorkspaceRetentionCleanupTask.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -255,14 +255,11 @@ private function build_cleanup_chunk_rows( Workspace $workspace, array $opts, ar
255255
);
256256

257257
if ( ! empty($opts['artifact_cleanup']) ) {
258-
$artifact_limit = isset($params['limit']) ? max(0, (int) $params['limit']) : 100;
259-
$artifact_page = $workspace->worktree_cleanup_artifacts(
258+
$artifact_page = $workspace->worktree_cleanup_artifacts(
260259
array(
261260
'dry_run' => true,
262261
'force' => ! empty($opts['force']),
263-
'limit' => $artifact_limit,
264-
'offset' => isset($params['offset']) ? max(0, (int) $params['offset']) : 0,
265-
'exhaustive' => ! empty($params['exhaustive']),
262+
'exhaustive' => true,
266263
'safety_probes' => true,
267264
)
268265
);

‎inc/Workspace/WorkspaceArtifactCleanup.php‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,10 @@ trait WorkspaceArtifactCleanup {
2121
* plan-only so every destructive run revalidates the exact worktree and
2222
* profile-derived artifact paths from a reviewed dry-run.
2323
*
24-
* Dry-run is bounded by default to keep huge workspaces (~hundreds of
25-
* worktrees) responsive: only the cheap top-level inventory is scanned for
26-
* artifact directories, per-worktree git status / unpushed-commit probes are
27-
* deferred unless `safety_probes` is requested, and the result is paginated
28-
* via `limit` + `offset`. Pass `exhaustive=true` to restore the full scan
29-
* (full git status + unpushed checks for every worktree).
24+
* Direct low-level dry-run is bounded by default to keep huge workspaces
25+
* (~hundreds of worktrees) responsive. Operators should apply through the
26+
* high-level cleanup plan/apply commands, which persist reviewed rows by run
27+
* ID instead of replaying a mutable inventory offset.
3028
*
3129
* Apply paths revalidate the planned subset only — they pass `only_handles`
3230
* derived from the plan into the builder so safety probes run against the
@@ -55,7 +53,7 @@ public function worktree_cleanup_artifacts( array $opts = array() ): array|\WP_E
5553
if ( $exhaustive ) {
5654
$limit = 0;
5755
}
58-
$apply_command = $this->build_artifact_cleanup_apply_command($limit, $offset, $exhaustive);
56+
$apply_command = $this->build_artifact_cleanup_apply_command();
5957
// Apply paths default to safety probing (small subset). Dry-run defaults
6058
// to skipping the per-worktree git probes unless explicitly requested or
6159
// the caller asked for exhaustive mode.
@@ -204,27 +202,11 @@ public function worktree_cleanup_artifacts( array $opts = array() ): array|\WP_E
204202
}
205203

206204
/**
207-
* Build the high-level command that applies the same artifact cleanup page.
208-
*
209-
* @param int $limit Effective bounded scan limit.
210-
* @param int $offset Bounded inventory offset.
211-
* @param bool $exhaustive Whether the dry-run used exhaustive mode.
205+
* Build the high-level command that persists a snapshot-safe artifact plan.
212206
* @return string
213207
*/
214-
private function build_artifact_cleanup_apply_command( int $limit, int $offset, bool $exhaustive ): string {
215-
$parts = array(
216-
'studio wp datamachine-code workspace cleanup run',
217-
'--mode=artifacts',
218-
);
219-
if ( $exhaustive ) {
220-
$parts[] = '--exhaustive';
221-
} else {
222-
$parts[] = sprintf('--limit=%d', $limit);
223-
$parts[] = sprintf('--offset=%d', $offset);
224-
}
225-
$parts[] = '--format=json';
226-
227-
return implode(' ', $parts);
208+
private function build_artifact_cleanup_apply_command(): string {
209+
return 'studio wp datamachine-code workspace cleanup run --mode=artifacts --dry-run --format=json';
228210
}
229211

230212
/**

0 commit comments

Comments
 (0)