Skip to content

Commit 20259e7

Browse files
chubes4chubes4
committed
Fix cleanup pagination snapshots
1 parent 7f68ddd commit 20259e7

5 files changed

Lines changed: 193 additions & 88 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: 33 additions & 46 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
@@ -903,17 +903,6 @@ private function cleanup_run_input( string $mode, array $assoc_args ): array {
903903
if ( isset($assoc_args['older-than']) && '' !== trim( (string) $assoc_args['older-than']) ) {
904904
$input['older_than'] = trim( (string) $assoc_args['older-than']);
905905
}
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-
}
917906

918907
return $input;
919908
}
@@ -931,6 +920,29 @@ private function run_cleanup_plan( array $assoc_args ): void {
931920
return;
932921
}
933922

923+
$input = $this->cleanup_plan_input($mode, $assoc_args);
924+
if ( 'json' !== (string) ( $assoc_args['format'] ?? 'table' ) ) {
925+
$profile = ! empty($input['include_artifacts']) ? 'includes artifact scan' : 'local worktree merge signals';
926+
WP_CLI::log(sprintf('Planning cleanup (%s; %s)...', $mode, $profile));
927+
}
928+
929+
$result = $ability->execute($input);
930+
if ( is_wp_error($result) ) {
931+
WP_CLI::error($result->get_error_message());
932+
return;
933+
}
934+
935+
$this->render_cleanup_plan_result($result, $assoc_args);
936+
}
937+
938+
/**
939+
* Normalize cleanup plan input shared by `cleanup plan` and dry-run `cleanup run`.
940+
*
941+
* @param string $mode Cleanup mode.
942+
* @param array $assoc_args CLI associative args.
943+
* @return array<string,mixed>
944+
*/
945+
private function cleanup_plan_input( string $mode, array $assoc_args ): array {
934946
$include_artifacts = 'artifacts' === $mode || ! empty($assoc_args['include-artifacts']);
935947
$include_worktrees = 'artifacts' !== $mode;
936948
$input = array(
@@ -945,18 +957,8 @@ private function run_cleanup_plan( array $assoc_args ): void {
945957
if ( isset($assoc_args['force']) ) {
946958
$input['force_artifact_cleanup'] = (bool) $assoc_args['force'];
947959
}
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-
}
952-
953-
$result = $ability->execute($input);
954-
if ( is_wp_error($result) ) {
955-
WP_CLI::error($result->get_error_message());
956-
return;
957-
}
958960

959-
$this->render_cleanup_plan_result($result, $assoc_args);
961+
return $input;
960962
}
961963

962964
private function run_cleanup_control_ability( string $operation, string $run_id, array $assoc_args ): void {
@@ -1008,28 +1010,13 @@ private function run_cleanup_review( array $assoc_args ): void {
10081010
return;
10091011

10101012
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']);
1013+
$ability = wp_get_ability('datamachine-code/workspace-cleanup-plan');
1014+
$result = $ability ? $ability->execute($this->cleanup_plan_input($mode, $assoc_args)) : new \WP_Error('cleanup_plan_ability_missing', 'Workspace cleanup plan ability not registered.');
1015+
if ( is_wp_error($result) ) {
1016+
WP_CLI::error($result->get_error_message());
1017+
return;
10301018
}
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);
1019+
$this->render_cleanup_plan_result($result, $assoc_args);
10331020
return;
10341021

10351022
case 'emergency':

‎inc/Tasks/WorkspaceRetentionCleanupTask.php‎

Lines changed: 1 addition & 4 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;
259258
$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
/**

‎tests/smoke-workspace-cleanup-snapshot-pagination.php‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
<?php
2+
/**
3+
* Smoke test for snapshot-safe cleanup pagination CLI routing.
4+
*
5+
* php tests/smoke-workspace-cleanup-snapshot-pagination.php
6+
*
7+
* @package DataMachineCode\Tests
8+
*/
9+
10+
declare( strict_types=1 );
11+
12+
namespace {
13+
if ( ! defined('ABSPATH') ) {
14+
define('ABSPATH', __DIR__);
15+
}
16+
17+
class WP_Error {
18+
public function __construct( private string $code = '', private string $message = '' ) {}
19+
20+
public function get_error_code(): string {
21+
return $this->code;
22+
}
23+
24+
public function get_error_message(): string {
25+
return $this->message;
26+
}
27+
}
28+
29+
class WP_CLI {
30+
public static array $logs = array();
31+
32+
public static function error( string $message ): void {
33+
throw new RuntimeException($message);
34+
}
35+
36+
public static function success( string $message ): void {
37+
self::$logs[] = $message;
38+
}
39+
40+
public static function log( string $message ): void {
41+
self::$logs[] = $message;
42+
}
43+
}
44+
45+
function is_wp_error( mixed $value ): bool {
46+
return $value instanceof WP_Error;
47+
}
48+
49+
function wp_json_encode( mixed $data, int $flags = 0 ): string|false {
50+
return json_encode($data, $flags);
51+
}
52+
53+
function wp_get_ability( string $name ): ?object {
54+
return $GLOBALS['datamachine_code_snapshot_abilities'][ $name ] ?? null;
55+
}
56+
57+
class DataMachineCodeSnapshotFakeAbility {
58+
public array $calls = array();
59+
60+
public function __construct( private mixed $result ) {}
61+
62+
public function execute( array $input ): mixed {
63+
$this->calls[] = $input;
64+
return $this->result;
65+
}
66+
}
67+
}
68+
69+
namespace DataMachine\Cli {
70+
class BaseCommand {
71+
protected function format_items( array $items, array $fields, array $assoc_args, string $default_sort = '' ): void { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed
72+
\WP_CLI::log('table:' . count($items));
73+
}
74+
}
75+
}
76+
77+
namespace {
78+
include_once dirname(__DIR__) . '/inc/Cleanup/CleanupRunEvidenceStoreInterface.php';
79+
include_once dirname(__DIR__) . '/inc/Cleanup/CleanupRemainingWorkSummary.php';
80+
include_once dirname(__DIR__) . '/inc/Cleanup/DataMachineJobCleanupRunEvidenceStore.php';
81+
include_once dirname(__DIR__) . '/inc/Cli/Commands/WorkspaceCommand.php';
82+
83+
function datamachine_code_snapshot_assert( bool $condition, string $message ): void {
84+
if ( ! $condition ) {
85+
fwrite(STDERR, "FAIL: {$message}\n");
86+
exit(1);
87+
}
88+
echo "ok - {$message}\n";
89+
}
90+
91+
$plan_ability = new DataMachineCodeSnapshotFakeAbility(
92+
array(
93+
'success' => true,
94+
'run_id' => 'cleanup-run-snapshot-page',
95+
'plan_id' => 'cleanup-plan-stable',
96+
'inputs' => array( 'include_artifacts' => true, 'include_worktrees' => false ),
97+
'summary' => array( 'total_rows' => 4, 'total_size_bytes' => 185204736 ),
98+
'cleanup_storage' => array( 'type' => 'database', 'item_count' => 4 ),
99+
)
100+
);
101+
$run_ability = new DataMachineCodeSnapshotFakeAbility(
102+
array(
103+
'success' => true,
104+
'state' => 'jobs_queued',
105+
'run_id' => 'cleanup-run-123',
106+
'job_id' => 123,
107+
)
108+
);
109+
110+
$GLOBALS['datamachine_code_snapshot_abilities'] = array(
111+
'datamachine-code/workspace-cleanup-plan' => $plan_ability,
112+
'datamachine-code/workspace-cleanup-run' => $run_ability,
113+
);
114+
115+
$command = new \DataMachineCode\Cli\Commands\WorkspaceCommand();
116+
$command->cleanup(
117+
array( 'run' ),
118+
array(
119+
'mode' => 'artifacts',
120+
'dry-run' => true,
121+
'limit' => 100,
122+
'offset' => 400,
123+
'format' => 'json',
124+
)
125+
);
126+
127+
datamachine_code_snapshot_assert(1 === count($plan_ability->calls), 'artifact dry-run uses DB-backed cleanup plan ability');
128+
datamachine_code_snapshot_assert(true === (bool) ( $plan_ability->calls[0]['include_artifacts'] ?? false ), 'artifact dry-run includes artifacts in persisted plan');
129+
datamachine_code_snapshot_assert(false === (bool) ( $plan_ability->calls[0]['include_worktrees'] ?? true ), 'artifact dry-run excludes worktree deletion rows');
130+
datamachine_code_snapshot_assert(! array_key_exists('offset', $plan_ability->calls[0]), 'artifact dry-run does not pass mutable offset into plan');
131+
datamachine_code_snapshot_assert(str_contains(implode("\n", WP_CLI::$logs), 'cleanup-run-snapshot-page'), 'artifact dry-run output exposes run_id for stable apply');
132+
133+
WP_CLI::$logs = array();
134+
$command->cleanup(
135+
array( 'run' ),
136+
array(
137+
'mode' => 'artifacts',
138+
'force' => true,
139+
'limit' => 100,
140+
'offset' => 400,
141+
'format' => 'json',
142+
)
143+
);
144+
145+
datamachine_code_snapshot_assert(1 === count($run_ability->calls), 'artifact apply schedules background cleanup once');
146+
datamachine_code_snapshot_assert(! array_key_exists('offset', $run_ability->calls[0]), 'artifact apply does not forward mutable offset to background cleanup');
147+
datamachine_code_snapshot_assert(! array_key_exists('limit', $run_ability->calls[0]), 'artifact apply does not forward page limit to background cleanup');
148+
datamachine_code_snapshot_assert(true === (bool) ( $run_ability->calls[0]['force'] ?? false ), 'artifact apply preserves force flag');
149+
150+
echo "Workspace cleanup snapshot pagination smoke passed.\n";
151+
}

0 commit comments

Comments
 (0)