Skip to content

Commit 2ce1f34

Browse files
chubes4homeboy-ci[bot]
chubes4
and
homeboy-ci[bot]
authored
Fail closed on cleanup state persistence errors (#769)
* fix: fail closed on cleanup state persistence errors * fix: satisfy cleanup state lint --------- Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent d22bd75 commit 2ce1f34

1 file changed

Lines changed: 104 additions & 23 deletions

File tree

‎inc/Workspace/CleanupRunService.php‎

Lines changed: 104 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ public function plan( array $opts = array() ): array|\WP_Error {
5454
return $run_id;
5555
}
5656
$plan['summary'] = $this->materialize_plan_recommended_commands( (array) ( $plan['summary'] ?? array() ), $run_id );
57-
$this->repository->update_run($run_id, array( 'summary' => $plan['summary'] ));
57+
$updated = $this->update_run_or_error($run_id, array( 'summary' => $plan['summary'] ), 'planned');
58+
if ( $updated instanceof \WP_Error ) {
59+
return $updated;
60+
}
5861

5962
$inserted = $this->repository->add_items($run_id, $items);
6063
if ( $inserted instanceof \WP_Error ) {
@@ -107,12 +110,16 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
107110

108111
$limit = $this->apply_limit($opts);
109112

110-
$this->repository->update_run(
113+
$updated = $this->update_run_or_error(
111114
$run_id, array(
112115
'status' => 'applying',
113116
'started_at' => gmdate('Y-m-d H:i:s'),
114-
)
117+
),
118+
'applying'
115119
);
120+
if ( $updated instanceof \WP_Error ) {
121+
return $updated;
122+
}
116123

117124
$items = $this->repository->get_items($run_id);
118125
$artifact_rows = $this->pending_rows_of_type($items, 'artifact_cleanup');
@@ -129,7 +136,10 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
129136
$artifact_batch = array_slice($artifact_rows, 0, $limit);
130137
$processed_rows += count($artifact_batch);
131138
$batch_type = 'artifact_cleanup';
132-
$this->mark_batch_applying($artifact_batch, $run_id, $batch_type, $limit, $remaining_rows);
139+
$marked = $this->mark_batch_applying($artifact_batch, $run_id, $batch_type, $limit, $remaining_rows);
140+
if ( $marked instanceof \WP_Error ) {
141+
return $marked;
142+
}
133143
$results['artifact_cleanup'] = $this->workspace->worktree_cleanup_artifacts(
134144
array(
135145
'apply_plan' => array( 'candidates' => array_map(fn( $item ) => $item['evidence'], $artifact_batch) ),
@@ -140,7 +150,10 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
140150
if ( $results['artifact_cleanup'] instanceof \WP_Error ) {
141151
return $results['artifact_cleanup'];
142152
}
143-
$this->record_apply_result($artifact_batch, $results['artifact_cleanup'], 'removed');
153+
$recorded = $this->record_apply_result($artifact_batch, $results['artifact_cleanup'], 'removed');
154+
if ( $recorded instanceof \WP_Error ) {
155+
return $recorded;
156+
}
144157
$applied_rows += count( (array) ( $results['artifact_cleanup']['removed'] ?? array() ) );
145158
$skipped_rows += count( (array) ( $results['artifact_cleanup']['skipped'] ?? array() ) );
146159
}
@@ -150,7 +163,10 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
150163
$worktree_batch = array_slice($worktree_rows, 0, $remaining_capacity);
151164
$processed_rows += count($worktree_batch);
152165
$batch_type = '' === $batch_type ? 'worktree_removal' : 'mixed';
153-
$this->mark_batch_applying($worktree_batch, $run_id, $batch_type, $limit, $remaining_rows);
166+
$marked = $this->mark_batch_applying($worktree_batch, $run_id, $batch_type, $limit, $remaining_rows);
167+
if ( $marked instanceof \WP_Error ) {
168+
return $marked;
169+
}
154170
$results['worktree_removal'] = $this->workspace->worktree_cleanup_merged(
155171
array(
156172
'apply_plan' => array( 'candidates' => array_map(fn( $item ) => $item['evidence'], $worktree_batch) ),
@@ -162,7 +178,10 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
162178
if ( $results['worktree_removal'] instanceof \WP_Error ) {
163179
return $results['worktree_removal'];
164180
}
165-
$this->record_apply_result($worktree_batch, $results['worktree_removal'], 'removed');
181+
$recorded = $this->record_apply_result($worktree_batch, $results['worktree_removal'], 'removed');
182+
if ( $recorded instanceof \WP_Error ) {
183+
return $recorded;
184+
}
166185
$applied_rows += count( (array) ( $results['worktree_removal']['removed'] ?? array() ) );
167186
$skipped_rows += count( (array) ( $results['worktree_removal']['skipped'] ?? array() ) );
168187
}
@@ -173,13 +192,17 @@ public function apply( string $run_id, array $opts = array() ): array|\WP_Error
173192
$next_status = $pending_or_fail > 0 ? 'needs_resume' : 'completed';
174193
$completed_at = 'completed' === $next_status ? gmdate('Y-m-d H:i:s') : null;
175194

176-
$this->repository->update_run(
195+
$updated = $this->update_run_or_error(
177196
$run_id, array(
178197
'status' => $next_status,
179198
'completed_at' => $completed_at,
180199
'summary' => $summary,
181-
)
200+
),
201+
$next_status
182202
);
203+
if ( $updated instanceof \WP_Error ) {
204+
return $updated;
205+
}
183206

184207
$status = $this->status($run_id);
185208
if ( $status instanceof \WP_Error ) {
@@ -502,10 +525,10 @@ private function pending_rows_of_type( array $items, string $type ): array {
502525
* @param int $limit Requested apply limit.
503526
* @param int $remaining_rows Rows remaining before this batch.
504527
*/
505-
private function mark_batch_applying( array $items, string $run_id, string $batch_type, int $limit, int $remaining_rows ): void {
528+
private function mark_batch_applying( array $items, string $run_id, string $batch_type, int $limit, int $remaining_rows ): ?\WP_Error {
506529
$started_at = gmdate('Y-m-d H:i:s');
507530
foreach ( $items as $item ) {
508-
$this->repository->update_item(
531+
$updated = $this->update_item_or_error(
509532
(int) $item['id'],
510533
array(
511534
'status' => 'applying',
@@ -516,11 +539,15 @@ private function mark_batch_applying( array $items, string $run_id, string $batc
516539
'applying_batch_type' => $batch_type,
517540
)
518541
),
519-
)
542+
),
543+
'applying'
520544
);
545+
if ( $updated instanceof \WP_Error ) {
546+
return $updated;
547+
}
521548
}
522549

523-
$this->repository->update_run(
550+
$updated = $this->update_run_or_error(
524551
$run_id,
525552
array(
526553
'summary' => array(
@@ -532,8 +559,10 @@ private function mark_batch_applying( array $items, string $run_id, string $batc
532559
'started_at' => $started_at,
533560
),
534561
),
535-
)
562+
),
563+
'applying'
536564
);
565+
return $updated instanceof \WP_Error ? $updated : null;
537566
}
538567

539568
/**
@@ -599,18 +628,22 @@ private function apply_limit( array $opts ): int {
599628
return max(1, min(self::MAX_APPLY_LIMIT, $limit));
600629
}
601630

602-
private function record_apply_result( array $items, mixed $result, string $applied_key ): void {
631+
private function record_apply_result( array $items, mixed $result, string $applied_key ): ?\WP_Error {
603632
if ( $result instanceof \WP_Error ) {
604633
foreach ( $items as $item ) {
605-
$this->repository->update_item(
634+
$updated = $this->update_item_or_error(
606635
(int) $item['id'], array(
607636
'status' => 'failed',
608637
'reason_code' => $result->get_error_code(),
609638
'reason' => $result->get_error_message(),
610-
)
639+
),
640+
'failed'
611641
);
642+
if ( $updated instanceof \WP_Error ) {
643+
return $updated;
644+
}
612645
}
613-
return;
646+
return null;
614647
}
615648

616649
$applied_by_handle = array();
@@ -626,28 +659,76 @@ private function record_apply_result( array $items, mixed $result, string $appli
626659
$handle = (string) ( $item['handle'] ?? '' );
627660
if ( isset($applied_by_handle[ $handle ]) ) {
628661
$applied = $applied_by_handle[ $handle ];
629-
$this->repository->update_item(
662+
$updated = $this->update_item_or_error(
630663
(int) $item['id'],
631664
array(
632665
'status' => 'applied',
633666
'applied_at' => gmdate('Y-m-d H:i:s'),
634667
'bytes_reclaimed' => max(0, (int) ( $applied['artifact_size_bytes'] ?? $applied['size_bytes'] ?? $item['evidence']['artifact_size_bytes'] ?? $item['evidence']['size_bytes'] ?? 0 )),
635668
'evidence' => array_merge( (array) $item['evidence'], array( 'applied' => $applied )),
636-
)
669+
),
670+
'applied'
637671
);
672+
if ( $updated instanceof \WP_Error ) {
673+
return $updated;
674+
}
638675
continue;
639676
}
640-
$skip = $skipped_by_handle[ $handle ] ?? null;
641-
$this->repository->update_item(
677+
$skip = $skipped_by_handle[ $handle ] ?? null;
678+
$updated = $this->update_item_or_error(
642679
(int) $item['id'],
643680
array(
644681
'status' => 'skipped',
645682
'reason_code' => is_array($skip) ? (string) ( $skip['reason_code'] ?? 'apply_skipped' ) : 'apply_skipped',
646683
'reason' => is_array($skip) ? (string) ( $skip['reason'] ?? '' ) : 'row was not applied',
647684
'evidence' => is_array($skip) ? $skip : $item['evidence'],
648-
)
685+
),
686+
'skipped'
649687
);
688+
if ( $updated instanceof \WP_Error ) {
689+
return $updated;
690+
}
691+
}
692+
693+
return null;
694+
}
695+
696+
/**
697+
* @param array<string,mixed> $fields Run fields.
698+
*/
699+
private function update_run_or_error( string $run_id, array $fields, string $state ): ?\WP_Error {
700+
if ( $this->repository->update_run($run_id, $fields) ) {
701+
return null;
650702
}
703+
704+
return new \WP_Error(
705+
'cleanup_run_update_failed',
706+
sprintf('Failed to persist cleanup run %s state for run %s.', $state, $run_id),
707+
array(
708+
'status' => 500,
709+
'run_id' => $run_id,
710+
'state' => $state,
711+
)
712+
);
713+
}
714+
715+
/**
716+
* @param array<string,mixed> $fields Item fields.
717+
*/
718+
private function update_item_or_error( int $item_id, array $fields, string $state ): ?\WP_Error {
719+
if ( $this->repository->update_item($item_id, $fields) ) {
720+
return null;
721+
}
722+
723+
return new \WP_Error(
724+
'cleanup_item_update_failed',
725+
sprintf('Failed to persist cleanup item %s state for item %d.', $state, $item_id),
726+
array(
727+
'status' => 500,
728+
'item_id' => $item_id,
729+
'state' => $state,
730+
)
731+
);
651732
}
652733

653734
private function planned_action_for_type( string $type ): string {

0 commit comments

Comments
 (0)