Skip to content

Commit 960de7f

Browse files
fix: waive reviewed cleanup recency guard (#888) (#889)
Co-authored-by: homeboy-ci[bot] <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 449b749 commit 960de7f

2 files changed

Lines changed: 45 additions & 5 deletions

File tree

inc/Workspace/WorkspaceWorktreeCleanupEngine.php

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,7 +1368,8 @@ private function apply_worktree_cleanup_plan_candidates( array $candidates, bool
13681368

13691369
foreach ( $candidates as $candidate ) {
13701370
++$processed;
1371-
$revalidated = $this->revalidate_bounded_cleanup_eligible_candidate($candidate, $force, $stale_liveness_only, $discard_unpushed);
1371+
$reviewed_lifecycle_snapshot = is_array($candidate['metadata'] ?? null) ? $candidate['metadata'] : array();
1372+
$revalidated = $this->revalidate_bounded_cleanup_eligible_candidate($candidate, $force, $stale_liveness_only, $discard_unpushed, $reviewed_lifecycle_snapshot);
13721373
if ( isset($revalidated['skipped']) ) {
13731374
$skipped[] = $revalidated['skipped'];
13741375
continue;
@@ -1461,15 +1462,22 @@ function () use ( $repo, $branch, $wt_path, $force, $remove_timeout_seconds ) {
14611462
*
14621463
* @param array $candidate Inventory candidate row.
14631464
* @param bool $force Allow dirty worktrees.
1464-
* @return array<string,mixed>
1465-
*/
1466-
private function revalidate_bounded_cleanup_eligible_candidate( array $candidate, bool $force, bool $stale_liveness_only = false, bool $discard_unpushed = false ): array {
1465+
* @param bool $stale_liveness_only Limit removal to stale worktrees.
1466+
* @param bool $discard_unpushed Allow removal of reviewed unpushed commits.
1467+
* @param array<string,mixed>|null $reviewed_lifecycle_snapshot Lifecycle metadata captured in a reviewed plan.
1468+
* @return array<string,mixed>
1469+
*/
1470+
private function revalidate_bounded_cleanup_eligible_candidate( array $candidate, bool $force, bool $stale_liveness_only = false, bool $discard_unpushed = false, ?array $reviewed_lifecycle_snapshot = null ): array {
14671471
$handle = (string) ( $candidate['handle'] ?? '' );
14681472
$repo = (string) ( $candidate['repo'] ?? '' );
14691473
$branch = (string) ( $candidate['branch'] ?? '' );
14701474
$wt_path = (string) ( $candidate['path'] ?? '' );
14711475
$liveness = (string) ( $candidate['liveness'] ?? '' );
14721476
$metadata = is_array($candidate['metadata'] ?? null) ? $candidate['metadata'] : array();
1477+
if ( null !== $reviewed_lifecycle_snapshot && function_exists('get_option') ) {
1478+
$current_metadata = WorktreeContextInjector::get_metadata($handle);
1479+
$metadata = is_array($current_metadata) ? $current_metadata : array();
1480+
}
14731481

14741482
if ( $stale_liveness_only && 'stale' !== $liveness ) {
14751483
return array(
@@ -1513,7 +1521,7 @@ private function revalidate_bounded_cleanup_eligible_candidate( array $candidate
15131521
}
15141522

15151523
$recent_activity = $this->worktree_cleanup_recent_activity_protection($metadata);
1516-
if ( null !== $recent_activity ) {
1524+
if ( null !== $recent_activity && ( null === $reviewed_lifecycle_snapshot || ! $this->worktree_cleanup_lifecycle_matches_reviewed_plan($reviewed_lifecycle_snapshot, $metadata) ) ) {
15171525
return array(
15181526
'skipped' => array_merge(
15191527
array(
@@ -1834,6 +1842,23 @@ private function worktree_cleanup_recent_activity_protection( array $metadata ):
18341842
);
18351843
}
18361844

1845+
/**
1846+
* Determine whether a reviewed row still has the lifecycle state the operator saw.
1847+
*
1848+
* @param array<string,mixed> $reviewed_metadata Lifecycle metadata captured in the reviewed plan.
1849+
* @param array<string,mixed> $current_metadata Current lifecycle metadata.
1850+
* @return bool
1851+
*/
1852+
private function worktree_cleanup_lifecycle_matches_reviewed_plan( array $reviewed_metadata, array $current_metadata ): bool {
1853+
foreach ( array( 'finalized_at', 'cleanup_eligible_at', 'created_at', 'lifecycle_state' ) as $field ) {
1854+
if ( (string) ( $reviewed_metadata[ $field ] ?? '' ) !== (string) ( $current_metadata[ $field ] ?? '' ) ) {
1855+
return false;
1856+
}
1857+
}
1858+
1859+
return true;
1860+
}
1861+
18371862
/**
18381863
* Fail-safe open-PR protection for remote-tracking-clean cleanup rows.
18391864
*

tests/worktree-retention-apply-protections.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ public function revalidate( array $candidate ): array {
105105
return $method->invoke($this, $candidate, false, false, false);
106106
}
107107

108+
public function revalidate_reviewed( array $candidate, array $reviewed_lifecycle_snapshot ): array {
109+
$method = new ReflectionMethod($this, 'revalidate_bounded_cleanup_eligible_candidate');
110+
return $method->invoke($this, $candidate, false, false, false, $reviewed_lifecycle_snapshot);
111+
}
112+
108113
private function validate_containment( string $path, string $container ): array {
109114
$real_path = realpath($path);
110115
$real_root = realpath($container);
@@ -173,6 +178,16 @@ private function git_get_remote( string $path ): ?string {
173178
retention_apply_protections_assert('recent_activity' === ( $recent_lifecycle['skipped']['reason_code'] ?? null ), 'recent cleanup_eligible_at rows are protected from apply removal');
174179
retention_apply_protections_assert('cleanup_eligible_at' === ( $recent_lifecycle['skipped']['activity_field'] ?? null ), 'recent lifecycle protection identifies the lifecycle activity field');
175180

181+
$reviewed_lifecycle_candidate = $base_candidate;
182+
$reviewed_lifecycle_candidate['metadata']['cleanup_eligible_at'] = gmdate('c', time() - 60);
183+
$reviewed_lifecycle = $harness->revalidate_reviewed($reviewed_lifecycle_candidate, $reviewed_lifecycle_candidate['metadata']);
184+
retention_apply_protections_assert(! isset($reviewed_lifecycle['skipped']), 'reviewed rows with unchanged recent lifecycle metadata remain removable');
185+
186+
$changed_lifecycle_candidate = $reviewed_lifecycle_candidate;
187+
$changed_lifecycle_candidate['metadata']['finalized_at'] = gmdate('c', time() - 30);
188+
$changed_lifecycle = $harness->revalidate_reviewed($changed_lifecycle_candidate, $reviewed_lifecycle_candidate['metadata']);
189+
retention_apply_protections_assert('recent_activity' === ( $changed_lifecycle['skipped']['reason_code'] ?? null ), 'reviewed rows with changed lifecycle metadata remain protected');
190+
176191
GitHubAbilities::$mode = 'open';
177192
$open_pr = $harness->revalidate($base_candidate);
178193
retention_apply_protections_assert('open_pr' === ( $open_pr['skipped']['reason_code'] ?? null ), 'open PR heads are protected from apply removal');

0 commit comments

Comments
 (0)