Skip to content

Commit c0099aa

Browse files
chubes4homeboy-ci[bot]
chubes4
and
homeboy-ci[bot]
authored
Fix empty safe cleanup status finalization (#853)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 93ee129 commit c0099aa

2 files changed

Lines changed: 200 additions & 2 deletions

File tree

‎inc/Workspace/CleanupRunService.php‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,28 @@ public function status( string $run_id ): array|\WP_Error {
270270
ksort($summary['items_by_status']);
271271
ksort($summary['items_by_type']);
272272

273+
$terminal_safe_state = $this->terminal_empty_safe_cleanup_state($run, $summary);
274+
if ( null !== $terminal_safe_state ) {
275+
$completed_at = (string) ( $run['completed_at'] ?? '' );
276+
if ( '' === $completed_at ) {
277+
$completed_at = gmdate('Y-m-d H:i:s');
278+
}
279+
$updated = $this->update_run_or_error(
280+
$run_id,
281+
array(
282+
'status' => $terminal_safe_state,
283+
'completed_at' => $completed_at,
284+
'summary' => (array) ( $run['summary'] ?? array() ),
285+
),
286+
$terminal_safe_state
287+
);
288+
if ( $updated instanceof \WP_Error ) {
289+
return $updated;
290+
}
291+
$run['status'] = $terminal_safe_state;
292+
$run['completed_at'] = $completed_at;
293+
}
294+
273295
$progress = $this->run_progress($run, $items, $summary);
274296

275297
return array(
@@ -629,11 +651,34 @@ private function run_progress( array $run, array $items, array $summary ): array
629651
'safe_cleanup' => $run['summary']['safe_cleanup_progress'] ?? null,
630652
'started_at' => $started_at,
631653
'age_seconds' => $age,
632-
'resumable' => $resumable || ( 'safe_workspace_cleanup' === (string) ( $run['mode'] ?? '' ) && 'applying' === $run_status ),
654+
'resumable' => $resumable,
633655
'note' => count($applying) > 0 ? 'Rows marked applying are safe to retry with workspace cleanup resume if the previous apply process was interrupted.' : '',
634656
);
635657
}
636658

659+
private function terminal_empty_safe_cleanup_state( array $run, array $summary ): ?string {
660+
if ( 'safe_workspace_cleanup' !== (string) ( $run['mode'] ?? '' ) || 'applying' !== (string) ( $run['status'] ?? '' ) ) {
661+
return null;
662+
}
663+
if ( (int) ( $summary['total_items'] ?? 0 ) > 0 || (int) ( $summary['pending_or_failed'] ?? 0 ) > 0 ) {
664+
return null;
665+
}
666+
667+
$safe_cleanup = (array) ( $run['summary']['safe_cleanup_progress'] ?? array() );
668+
if ( array() === $safe_cleanup ) {
669+
return null;
670+
}
671+
672+
$state = (string) ( $safe_cleanup['state'] ?? '' );
673+
if ( in_array($state, array( 'complete', 'complete_with_blockers' ), true) ) {
674+
return $state;
675+
}
676+
677+
$safe_summary = (array) ( $safe_cleanup['summary'] ?? array() );
678+
$blocker_count = (int) ( $safe_summary['blocker_count'] ?? 0 );
679+
return $blocker_count > 0 ? 'complete_with_blockers' : 'complete';
680+
}
681+
637682
/**
638683
* Build remaining-work summary and prepend the current run resume command.
639684
*
@@ -646,7 +691,7 @@ private function remaining_work_summary( string $run_id, array $items, array $pr
646691
$summary = CleanupRemainingWorkSummary::from_items($items);
647692
$safe_cleanup = is_array($progress['safe_cleanup'] ?? null) ? (array) $progress['safe_cleanup'] : array();
648693
$safe_commands = is_array($safe_cleanup['commands'] ?? null) ? (array) $safe_cleanup['commands'] : array();
649-
if ( isset($safe_commands['status'], $safe_commands['resume']) ) {
694+
if ( ! empty($progress['resumable']) && isset($safe_commands['status'], $safe_commands['resume']) ) {
650695
$resume_command = array(
651696
'bucket' => 'safe_cleanup_continuation',
652697
'command' => (string) $safe_commands['status'],

‎tests/cleanup-safe-status-terminal.php‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
<?php
2+
/**
3+
* Standalone coverage for safe cleanup status terminalization.
4+
*/
5+
6+
declare(strict_types=1);
7+
8+
namespace DataMachineCode\Workspace {
9+
if ( ! class_exists(Workspace::class) ) {
10+
class Workspace {}
11+
}
12+
}
13+
14+
namespace {
15+
if ( ! defined('ABSPATH') ) {
16+
define('ABSPATH', __DIR__ . '/fixtures/');
17+
}
18+
19+
if ( ! class_exists('WP_Error') ) {
20+
class WP_Error {
21+
public string $code;
22+
public string $message;
23+
public array $data;
24+
25+
public function __construct( string $code = '', string $message = '', array $data = array() ) {
26+
$this->code = $code;
27+
$this->message = $message;
28+
$this->data = $data;
29+
}
30+
31+
public function get_error_message(): string {
32+
return $this->message;
33+
}
34+
}
35+
}
36+
37+
if ( ! function_exists('is_wp_error') ) {
38+
function is_wp_error( mixed $thing ): bool {
39+
return $thing instanceof WP_Error;
40+
}
41+
}
42+
43+
require_once dirname(__DIR__) . '/inc/Storage/CleanupRunRepositoryInterface.php';
44+
require_once dirname(__DIR__) . '/inc/Storage/CleanupRunRepository.php';
45+
require_once dirname(__DIR__) . '/inc/Cleanup/CleanupRemainingWorkSummary.php';
46+
require_once dirname(__DIR__) . '/inc/Workspace/CleanupRunService.php';
47+
48+
final class SafeCleanupStatusRepository extends DataMachineCode\Storage\CleanupRunRepository {
49+
/** @var array<string,array<string,mixed>> */
50+
public array $runs = array();
51+
52+
/** @var array<string,array<int,array<string,mixed>>> */
53+
public array $items = array();
54+
55+
/** @var array<int,array<string,mixed>> */
56+
public array $updates = array();
57+
58+
public function get_run( string $run_id ): ?array {
59+
return $this->runs[ $run_id ] ?? null;
60+
}
61+
62+
public function get_items( string $run_id ): array {
63+
return $this->items[ $run_id ] ?? array();
64+
}
65+
66+
public function update_run( string $run_id, array $fields ): bool {
67+
$this->updates[] = array( 'run_id' => $run_id, 'fields' => $fields );
68+
$this->runs[ $run_id ] = array_merge($this->runs[ $run_id ] ?? array(), $fields);
69+
return true;
70+
}
71+
}
72+
73+
function safe_status_assert_same( mixed $expected, mixed $actual, string $message ): void {
74+
if ( $expected !== $actual ) {
75+
throw new RuntimeException(sprintf("%s\nExpected: %s\nActual: %s", $message, var_export($expected, true), var_export($actual, true)));
76+
}
77+
}
78+
79+
function safe_status_assert_false_contains( array $values, string $needle, string $message ): void {
80+
foreach ( $values as $value ) {
81+
if ( is_string($value) && str_contains($value, $needle) ) {
82+
throw new RuntimeException($message . "\nUnexpected value: " . $value);
83+
}
84+
}
85+
}
86+
87+
$repo = new SafeCleanupStatusRepository();
88+
$repo->runs['cleanup-run-empty-safe'] = array(
89+
'run_id' => 'cleanup-run-empty-safe',
90+
'mode' => 'safe_workspace_cleanup',
91+
'status' => 'applying',
92+
'started_at' => gmdate('Y-m-d H:i:s', time() - 600),
93+
'completed_at' => null,
94+
'summary' => array(
95+
'safe_cleanup_progress' => array(
96+
'state' => 'applying',
97+
'summary' => array( 'blocker_count' => 0 ),
98+
'commands' => array(
99+
'status' => 'studio wp datamachine-code workspace cleanup status cleanup-run-empty-safe --format=json',
100+
'resume' => 'studio wp datamachine-code workspace cleanup safe --format=json',
101+
),
102+
),
103+
),
104+
);
105+
106+
$service = new DataMachineCode\Workspace\CleanupRunService($repo);
107+
$status = $service->status('cleanup-run-empty-safe');
108+
safe_status_assert_same(false, is_wp_error($status), 'Empty safe cleanup status should succeed.');
109+
safe_status_assert_same('complete', $status['state'] ?? null, 'Empty safe cleanup should finalize to complete.');
110+
safe_status_assert_same('complete', $repo->runs['cleanup-run-empty-safe']['status'] ?? null, 'Empty safe cleanup terminal status should be persisted.');
111+
safe_status_assert_same(false, $status['progress']['resumable'] ?? null, 'Empty safe cleanup should not be resumable.');
112+
safe_status_assert_false_contains((array) ( $status['remaining_work_summary']['next_commands'] ?? array() ), 'workspace cleanup safe', 'Empty safe cleanup should not recommend safe resume.');
113+
safe_status_assert_false_contains((array) ( $status['remaining_work_summary']['next_commands'] ?? array() ), 'workspace cleanup resume', 'Empty safe cleanup should not recommend DB resume.');
114+
115+
$evidence = $service->evidence('cleanup-run-empty-safe');
116+
safe_status_assert_same(false, is_wp_error($evidence), 'Empty safe cleanup evidence should succeed.');
117+
safe_status_assert_same('complete', $evidence['state'] ?? null, 'Evidence should report terminal state.');
118+
safe_status_assert_same(array(), $evidence['items'] ?? null, 'Evidence should keep empty item list explicit.');
119+
120+
$repo->runs['cleanup-run-blocked-safe'] = array(
121+
'run_id' => 'cleanup-run-blocked-safe',
122+
'mode' => 'safe_workspace_cleanup',
123+
'status' => 'applying',
124+
'summary' => array(
125+
'safe_cleanup_progress' => array(
126+
'summary' => array( 'blocker_count' => 2 ),
127+
),
128+
),
129+
);
130+
$blocked = $service->status('cleanup-run-blocked-safe');
131+
safe_status_assert_same('complete_with_blockers', $blocked['state'] ?? null, 'Empty safe cleanup with saved blockers should finalize distinctly.');
132+
133+
$repo->runs['cleanup-run-pending'] = array(
134+
'run_id' => 'cleanup-run-pending',
135+
'mode' => 'cleanup_plan',
136+
'status' => 'applying',
137+
'summary' => array(),
138+
);
139+
$repo->items['cleanup-run-pending'] = array(
140+
array(
141+
'id' => 1,
142+
'handle' => 'repo@branch',
143+
'item_type' => 'worktree_removal',
144+
'status' => 'pending',
145+
'evidence' => array(),
146+
),
147+
);
148+
$pending = $service->status('cleanup-run-pending');
149+
safe_status_assert_same('applying', $pending['state'] ?? null, 'Applying run with pending work should stay applying.');
150+
safe_status_assert_same(true, $pending['progress']['resumable'] ?? null, 'Applying run with pending work should remain resumable.');
151+
152+
echo "cleanup safe status terminal test passed.\n";
153+
}

0 commit comments

Comments
 (0)