Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 61 additions & 6 deletions inc/Workspace/WorkspaceWorktreeCleanupEngine.php
Original file line number Diff line number Diff line change
Expand Up @@ -657,10 +657,16 @@ function () use ( $cand, $force, $remove_timeout_seconds ) {
return $validated;
}

$recovery = $this->preserve_cleanup_recovery_ref($validated);
if ( is_wp_error($recovery) ) {
return $recovery;
}

$remove = $this->remove_worktree_by_path($validated['repo'], $validated['branch'], $validated['path'], $force, $remove_timeout_seconds);
if ( is_wp_error($remove) ) {
return $remove;
}
$remove = array_merge($remove, $recovery);

if ( ! empty($validated['preserve_local_branch']) ) {
$remove['local_branch_preserved'] = true;
Expand Down Expand Up @@ -692,6 +698,10 @@ function () use ( $cand, $force, $remove_timeout_seconds ) {
array(
'removed_path' => (string) ( $validated['path'] ?? '' ),
'path_exists_after' => is_dir( (string) ( $validated['path'] ?? '' ) ),
'recovery_ref' => $remove['remove']['recovery_ref'] ?? null,
'recovery_commit' => $remove['remove']['recovery_commit'] ?? null,
'recovery_command' => $remove['remove']['recovery_command'] ?? null,
'recovery_dispose_command' => $remove['remove']['recovery_dispose_command'] ?? null,
)
);
++$removed_count;
Expand Down Expand Up @@ -1433,7 +1443,7 @@ public function worktree_bounded_cleanup_eligible_apply( array $opts = array() )
*/
private function build_bounded_cleanup_processed_candidate( array $candidate, string $action, array $outcome ): array {
$row = $candidate;
foreach ( array( 'dirty', 'unpushed', 'path', 'size_bytes', 'removal_status', 'removal_error', 'local_branch_deleted', 'branch_delete_error', 'path_exists_after' ) as $field ) {
foreach ( array( 'dirty', 'unpushed', 'path', 'size_bytes', 'removal_status', 'removal_error', 'local_branch_deleted', 'branch_delete_error', 'path_exists_after', 'recovery_ref', 'recovery_commit', 'recovery_command', 'recovery_dispose_command' ) as $field ) {
if ( array_key_exists($field, $outcome) ) {
$row[ $field ] = $outcome[ $field ];
}
Expand Down Expand Up @@ -1499,6 +1509,10 @@ private function apply_worktree_cleanup_plan_candidates( array $candidates, bool
'removal_error' => $remove['removal_error'] ?? null,
'local_branch_deleted' => $remove['local_branch_deleted'] ?? null,
'branch_delete_error' => $remove['branch_delete_error'] ?? null,
'recovery_ref' => $remove['recovery_ref'] ?? null,
'recovery_commit' => $remove['recovery_commit'] ?? null,
'recovery_command' => $remove['recovery_command'] ?? null,
'recovery_dispose_command' => $remove['recovery_dispose_command'] ?? null,
)
);
if ( null === $size ) {
Expand Down Expand Up @@ -1572,6 +1586,11 @@ private function remove_revalidated_cleanup_candidate( array $candidate, bool $f
$size = null === $measured ? null : (int) $measured;
}

$recovery = ! empty($validated['broken_orphan']) ? array() : $this->preserve_cleanup_recovery_ref($validated);
if ( is_wp_error($recovery) ) {
return $recovery;
}

$result = $this->remove_worktree_by_path($repo, $branch, $wt_path, $force, $remove_timeout_seconds, ! empty($validated['broken_orphan']));
if ( is_wp_error($result) ) {
return $result;
Expand All @@ -1580,6 +1599,7 @@ private function remove_revalidated_cleanup_candidate( array $candidate, bool $f
return $this->normalize_bounded_cleanup_locked_result($result, $validated);
}

$result = array_merge($result, $recovery);
$primary_path = $this->get_primary_path($repo);
if ( '' !== $branch ) {
$delete = $this->run_git($primary_path, sprintf('branch -D %s', escapeshellarg($branch)), self::CLEANUP_GIT_PROBE_TIMEOUT);
Expand All @@ -1598,6 +1618,42 @@ private function remove_revalidated_cleanup_candidate( array $candidate, bool $f
return array( 'validated' => $validated, 'size' => $size, 'remove' => $result );
}

/** Preserve the exact candidate commit before crossing the removal boundary. */
private function preserve_cleanup_recovery_ref( array $candidate ): array|\WP_Error {
$repo = (string) ( $candidate['repo'] ?? '' );
$worktree = (string) ( $candidate['path'] ?? '' );
$primary_path = $this->get_primary_path($repo);
$head = $this->run_git($worktree, 'rev-parse --verify HEAD', self::CLEANUP_GIT_PROBE_TIMEOUT);
$commit = is_wp_error($head) ? '' : trim( (string) ( $head['output'] ?? '' ) );
if ( 1 !== preg_match('/^[0-9a-f]{40,64}$/', $commit) ) {
return new \WP_Error('cleanup_recovery_head_unverified', 'Cleanup refused removal because the candidate commit could not be resolved for durable recovery.', array( 'status' => 409 ));
}

$ref = 'refs/dmc/recovery/' . $commit;
$existing = $this->run_git($primary_path, sprintf('rev-parse --verify %s', escapeshellarg($ref)), self::CLEANUP_GIT_PROBE_TIMEOUT);
if ( ! is_wp_error($existing) && ! hash_equals($commit, trim( (string) ( $existing['output'] ?? '' ) )) ) {
return new \WP_Error('cleanup_recovery_ref_conflict', 'Cleanup refused removal because the deterministic recovery ref identifies another commit.', array( 'status' => 409, 'recovery_ref' => $ref ));
}
if ( is_wp_error($existing) ) {
$preserved = $this->run_git($primary_path, sprintf('update-ref %s %s %s', escapeshellarg($ref), escapeshellarg($commit), escapeshellarg(str_repeat('0', strlen($commit)))), self::CLEANUP_GIT_PROBE_TIMEOUT);
if ( is_wp_error($preserved) ) {
return new \WP_Error('cleanup_recovery_ref_failed', 'Cleanup refused removal because the durable recovery ref could not be written.', array( 'status' => 409, 'recovery_ref' => $ref ));
}
}

$verified = $this->run_git($primary_path, sprintf('rev-parse --verify %s', escapeshellarg($ref)), self::CLEANUP_GIT_PROBE_TIMEOUT);
if ( is_wp_error($verified) || ! hash_equals($commit, trim( (string) ( $verified['output'] ?? '' ) )) ) {
return new \WP_Error('cleanup_recovery_ref_unverified', 'Cleanup refused removal because the durable recovery ref could not be verified.', array( 'status' => 409, 'recovery_ref' => $ref ));
}

return array(
'recovery_ref' => $ref,
'recovery_commit' => $commit,
'recovery_command' => sprintf('git -C %s worktree add --detach %s %s', escapeshellarg($primary_path), escapeshellarg($worktree), escapeshellarg($ref)),
'recovery_dispose_command' => sprintf('git -C %s update-ref -d %s %s', escapeshellarg($primary_path), escapeshellarg($ref), escapeshellarg($commit)),
);
}

/** Normalize the repository-lock callback contract before callers inspect it. */
private function normalize_bounded_cleanup_locked_result( mixed $result, array $candidate ): array|\WP_Error {
if ( is_wp_error($result) ) {
Expand Down Expand Up @@ -2044,11 +2100,10 @@ private function worktree_cleanup_removable_lifecycle_states(): array {
* @return bool
*/
private function worktree_cleanup_has_removable_lifecycle( array $metadata ): bool {
$state = WorktreeContextInjector::project_lifecycle_state($metadata);
$finalized_state = isset($metadata['finalized_state']) ? WorktreeContextInjector::normalize_state( (string) $metadata['finalized_state']) : null;
$removable = $this->worktree_cleanup_removable_lifecycle_states();
$state = WorktreeContextInjector::project_lifecycle_state($metadata);
$removable = $this->worktree_cleanup_removable_lifecycle_states();

return in_array($state, $removable, true) || in_array($finalized_state, $removable, true);
return in_array($state, $removable, true);
}

/**
Expand Down Expand Up @@ -2102,7 +2157,7 @@ private function worktree_cleanup_recent_activity_protection( array $metadata ):
* @return bool
*/
private function worktree_cleanup_lifecycle_matches_reviewed_plan( array $reviewed_metadata, array $current_metadata ): bool {
foreach ( array( 'finalized_at', 'cleanup_eligible_at', 'created_at', 'lifecycle_state' ) as $field ) {
foreach ( array( 'finalized_at', 'cleanup_eligible_at', 'created_at', 'lifecycle_state', 'owner_run_ref', 'finalized_owner_run_ref', 'owner_terminal_at', 'owner_terminal_owner_run_ref' ) as $field ) {
if ( (string) ( $reviewed_metadata[ $field ] ?? '' ) !== (string) ( $current_metadata[ $field ] ?? '' ) ) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions inc/Workspace/WorkspaceWorktreeLifecycle.php
Original file line number Diff line number Diff line change
Expand Up @@ -3088,7 +3088,7 @@ private function claim_expired_worktree( string $handle, string $branch, ?string
'new_purpose' => $intent['purpose'],
'base_ref' => $base,
);
$metadata = array_merge($metadata, array(
$metadata = WorktreeContextInjector::reactivate_for_reuse($metadata, array(
'lifecycle_state' => WorktreeContextInjector::STATE_ACTIVE,
'last_seen_at' => gmdate('c'),
'observed_at' => gmdate('c'),
Expand All @@ -3098,7 +3098,7 @@ private function claim_expired_worktree( string $handle, string $branch, ?string
'ownership_lineage' => array_merge( (array) ( $metadata['ownership_lineage'] ?? array() ), array( $lineage )),
));
$metadata['reuse_contract'] = array_merge($contract, $intent);
$stored = WorktreeContextInjector::store_lifecycle_metadata($handle, $metadata);
$stored = WorktreeContextInjector::restore_lifecycle_metadata($handle, $metadata);
if ( is_wp_error($stored) ) {
return new \WP_Error('worktree_claim_metadata_persistence_failed', 'Claim ownership metadata could not be persisted.', array(
'status' => 500,
Expand Down
7 changes: 3 additions & 4 deletions inc/Workspace/WorktreeCleanupCandidateClassifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,16 +240,15 @@ private static function has_removable_lifecycle( mixed $metadata ): bool {
return false;
}

$state = isset($metadata['lifecycle_state']) ? WorktreeContextInjector::normalize_state( (string) $metadata['lifecycle_state']) : null;
$finalized_state = isset($metadata['finalized_state']) ? WorktreeContextInjector::normalize_state( (string) $metadata['finalized_state']) : null;
$removable = array(
$state = WorktreeContextInjector::project_lifecycle_state($metadata);
$removable = array(
WorktreeContextInjector::STATE_CLEANUP_ELIGIBLE,
WorktreeContextInjector::STATE_MERGED,
WorktreeContextInjector::STATE_CLOSED,
WorktreeContextInjector::STATE_ABANDONED,
);

return in_array($state, $removable, true) || in_array($finalized_state, $removable, true);
return in_array($state, $removable, true);
}

/**
Expand Down
69 changes: 67 additions & 2 deletions inc/Workspace/WorktreeContextInjector.php
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,33 @@ public static function has_owner_terminal_disposable_cleanup_signal( array $meta
return self::CLEANUP_POLICY_REMOVE_ON_SUCCESS === ( $metadata['cleanup_policy'] ?? null )
&& null !== self::normalize_scalar_metadata_value($metadata['purpose'] ?? null)
&& null !== self::normalize_scalar_metadata_value($metadata['owner_run_ref'] ?? null)
&& 'success' === ( $metadata['owner_terminal_outcome'] ?? null );
&& 'success' === ( $metadata['owner_terminal_outcome'] ?? null )
&& self::terminal_evidence_matches_current_ownership($metadata, 'owner_terminal_at', 'owner_terminal_owner_run_ref');
}

/** Remove terminal authority when a clean worktree starts a new lifecycle. */
public static function reactivate_for_reuse( array $metadata, array $active_metadata ): array {
foreach ( array(
'finalized_at',
'finalized_state',
'finalized_owner_run_ref',
'cleanup_eligible_at',
'owner_terminal_outcome',
'owner_terminal_at',
'owner_terminal_owner_run_ref',
'auto_finalized_by',
'auto_finalized_signal',
'auto_finalized_reason',
'cleanup_eligibility_evidence',
'pr_ref',
'pr_url',
'pr_number',
'pr_repo',
) as $field ) {
unset($metadata[ $field ]);
}

return array_merge($metadata, $active_metadata);
}

private static function optional_intent_value( mixed $value ): ?string {
Expand Down Expand Up @@ -946,6 +972,10 @@ public static function build_finalizer_metadata( string $state, ?string $pr = nu
'lifecycle_state' => $normalized,
'finalized_at' => gmdate('c'),
);
$finalized_owner_run_ref = self::normalize_scalar_metadata_value($existing['owner_run_ref'] ?? null);
if ( null !== $finalized_owner_run_ref ) {
$metadata['finalized_owner_run_ref'] = $finalized_owner_run_ref;
}

$pr_metadata = self::parse_pr_reference($pr);
if ( ! empty($pr_metadata) ) {
Expand All @@ -956,6 +986,9 @@ public static function build_finalizer_metadata( string $state, ?string $pr = nu
if ( '' !== $owner_terminal_outcome ) {
$metadata['owner_terminal_outcome'] = $owner_terminal_outcome;
$metadata['owner_terminal_at'] = $metadata['finalized_at'];
if ( null !== $finalized_owner_run_ref ) {
$metadata['owner_terminal_owner_run_ref'] = $finalized_owner_run_ref;
}
}

if ( self::should_mark_cleanup_eligible($normalized, $pr_metadata) || self::has_owner_terminal_disposable_cleanup_signal(array_merge($existing, $metadata)) ) {
Expand Down Expand Up @@ -1003,7 +1036,9 @@ public static function has_cleanup_signal( array $metadata ): bool {
}

$finalized_state = isset($metadata['finalized_state']) ? self::normalize_state( (string) $metadata['finalized_state']) : null;
return null !== $finalized_state && self::should_mark_cleanup_eligible($finalized_state, self::extract_pr_metadata($metadata));
return null !== $finalized_state
&& self::terminal_evidence_matches_current_ownership($metadata, 'finalized_at', 'finalized_owner_run_ref')
&& self::should_mark_cleanup_eligible($finalized_state, self::extract_pr_metadata($metadata));
}

/**
Expand Down Expand Up @@ -1182,6 +1217,9 @@ public static function has_explicit_cleanup_eligibility( array $metadata ): bool
if ( empty($metadata['cleanup_eligible_at']) || false === strtotime( (string) $metadata['cleanup_eligible_at'] ) || empty($metadata['finalized_at']) || false === strtotime( (string) $metadata['finalized_at'] ) ) {
return false;
}
if ( ! self::terminal_evidence_matches_current_ownership($metadata, 'finalized_at', 'finalized_owner_run_ref') ) {
return false;
}

$finalized_state = isset($metadata['finalized_state']) ? self::normalize_state( (string) $metadata['finalized_state'] ) : null;
if ( null !== $finalized_state && in_array($finalized_state, array( self::STATE_MERGED, self::STATE_CLOSED, self::STATE_ABANDONED, self::STATE_CLEANUP_ELIGIBLE ), true) ) {
Expand All @@ -1191,6 +1229,33 @@ public static function has_explicit_cleanup_eligibility( array $metadata ): bool
return array() !== self::extract_pr_metadata($metadata);
}

/** Reject terminal evidence recorded by an owner superseded by a later claim. */
private static function terminal_evidence_matches_current_ownership( array $metadata, string $timestamp_field, string $owner_field ): bool {
$terminal_owner = self::normalize_scalar_metadata_value($metadata[ $owner_field ] ?? null);
$current_owner = self::normalize_scalar_metadata_value($metadata['owner_run_ref'] ?? null);
if ( null !== $terminal_owner && $terminal_owner !== $current_owner ) {
return false;
}

$latest_claim = 0;
foreach ( (array) ( $metadata['ownership_lineage'] ?? array() ) as $transition ) {
if ( ! is_array($transition) || ! array_key_exists('claimed_at', $transition) ) {
continue;
}
$claimed_at = is_scalar($transition['claimed_at']) ? strtotime((string) $transition['claimed_at']) : false;
if ( false === $claimed_at ) {
return false;
}
$latest_claim = max($latest_claim, $claimed_at);
}
if ( 0 === $latest_claim ) {
return true;
}

$terminal_at = is_scalar($metadata[ $timestamp_field ] ?? null) ? strtotime((string) $metadata[ $timestamp_field ]) : false;
return false !== $terminal_at && $terminal_at >= $latest_claim;
}

/**
* Extract PR-like fields from a persisted metadata record.
*
Expand Down
19 changes: 19 additions & 0 deletions tests/bounded-cleanup-processed-candidates.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ public function processed( array $candidate, string $action, array $outcome ): a

final class BoundedCleanupRemovalCallbackHarness {
use WorkspaceWorktreeCleanupEngine;
private const RECOVERY_COMMIT = '0123456789abcdef0123456789abcdef01234567';

protected const CLEANUP_GIT_PROBE_TIMEOUT = 5;
protected const CLEANUP_GIT_REMOVE_TIMEOUT = 60;
Expand All @@ -54,6 +55,7 @@ final class BoundedCleanupRemovalCallbackHarness {
'success' => true,
'removal_status' => 'complete',
);
public bool $fail_recovery_write = false;

public function remove( array $candidate ): array|WP_Error {
return $this->remove_revalidated_cleanup_candidate($candidate, false, false, 60, false);
Expand All @@ -77,6 +79,15 @@ private function get_primary_path( string $repo ): string {
}

private function run_git( string $path, string $command, int $timeout = 0 ): array|WP_Error {
if ( str_starts_with($command, 'rev-parse --verify') ) {
if ( $this->fail_recovery_write && str_contains($command, 'refs/dmc/recovery/') ) {
return new WP_Error('missing_ref', 'recovery ref does not exist');
}
return array( 'output' => self::RECOVERY_COMMIT );
}
if ( str_starts_with($command, 'update-ref ') ) {
return $this->fail_recovery_write ? new WP_Error('cannot_write_ref', 'cannot write recovery ref') : array( 'output' => '' );
}
return new WP_Error('git_failed', 'cannot lock ref');
}
}
Expand Down Expand Up @@ -116,7 +127,12 @@ private function run_git( string $path, string $command, int $timeout = 0 ): arr
$callback_harness = new BoundedCleanupRemovalCallbackHarness();
$callback_path = sys_get_temp_dir() . '/dmc-bounded-cleanup-callback-' . getmypid();
$callback_candidate = array_merge($candidate, array( 'path' => $callback_path ));
$callback_harness->fail_recovery_write = true;
mkdir($callback_path);
$unpreserved = $callback_harness->remove($callback_candidate);
bounded_cleanup_processed_candidates_assert_same('cleanup_recovery_ref_failed', is_wp_error($unpreserved) ? $unpreserved->get_error_code() : null, 'cleanup fails closed when durable recovery cannot be written');
bounded_cleanup_processed_candidates_assert_same(true, is_dir($callback_path), 'failed recovery preservation crossed the worktree removal boundary');
$callback_harness->fail_recovery_write = false;
$locked = $callback_harness->remove($callback_candidate);
$branch_delete_error = array(
'code' => 'git_failed',
Expand All @@ -126,12 +142,15 @@ private function run_git( string $path, string $command, int $timeout = 0 ): arr
bounded_cleanup_processed_candidates_assert_same($callback_candidate, $locked['validated'] ?? null, 'branch deletion failure retains the normalized callback envelope');
bounded_cleanup_processed_candidates_assert_same(false, $locked['remove']['local_branch_deleted'] ?? null, 'normalized removal records retained local branch');
bounded_cleanup_processed_candidates_assert_same($branch_delete_error, $locked['remove']['branch_delete_error'] ?? null, 'normalized removal records branch deletion failure');
bounded_cleanup_processed_candidates_assert_same('refs/dmc/recovery/0123456789abcdef0123456789abcdef01234567', $locked['remove']['recovery_ref'] ?? null, 'removal records retain the durable recovery ref');
bounded_cleanup_processed_candidates_assert_same(true, str_contains((string) ($locked['remove']['recovery_command'] ?? ''), 'worktree add --detach'), 'removal records expose a reconstruction command');

$removed_outcome = array_merge($locked['remove'], array( 'path_exists_after' => false ));
$removed_processed = $harness->processed($locked['validated'], 'removed', $removed_outcome);
bounded_cleanup_processed_candidates_assert_same('removed', $removed_processed['final_action'], 'branch deletion failure does not discard successful worktree removal');
bounded_cleanup_processed_candidates_assert_same(false, $removed_processed['local_branch_deleted'], 'processed evidence records retained local branch');
bounded_cleanup_processed_candidates_assert_same($branch_delete_error, $removed_processed['branch_delete_error'], 'processed evidence records the branch deletion failure');
bounded_cleanup_processed_candidates_assert_same($locked['remove']['recovery_ref'], $removed_processed['recovery_ref'], 'processed evidence retains the durable recovery ref');

$callback_harness->remove_result = null;
mkdir($callback_path);
Expand Down
Loading
Loading