From 63f2753e03069971b9150a781c08b10b962d7fa8 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 5 Aug 2026 13:24:27 -0400 Subject: [PATCH] fix(cleanup): scope active artifact process probes Retry incomplete host-wide process evidence against each candidate's roots so sibling builds do not block safe artifact reclamation. Preserve fail-closed behavior when scoped evidence remains ambiguous. AI assistance: OpenAI openai/gpt-5.6-sol via OpenCode implemented the scoped probe, evidence fields, and focused tests. Chris Huber reviewed and owns the change. --- inc/Support/ProcessPathProbe.php | 41 ++++++++++ inc/Workspace/WorkspaceArtifactCleanup.php | 49 ++++++++++-- .../artifact-cleanup-live-process-guards.php | 74 +++++++++++++++++++ 3 files changed, 156 insertions(+), 8 deletions(-) diff --git a/inc/Support/ProcessPathProbe.php b/inc/Support/ProcessPathProbe.php index 9a6d999c..a9dbbeb5 100644 --- a/inc/Support/ProcessPathProbe.php +++ b/inc/Support/ProcessPathProbe.php @@ -21,6 +21,9 @@ interface ProcessPathProbeInterface { /** @return array{status:string,records:array>,diagnostics:array} */ public function snapshot(): array; + + /** @param array $paths Absolute candidate roots to scope the inspection to. */ + public function snapshot_for_paths( array $paths ): array; } final class ProcfsProcessPathProbe implements ProcessPathProbeInterface { @@ -31,6 +34,12 @@ public function __construct(private $scanner) {} public function snapshot(): array { return ( $this->scanner )(); } + + public function snapshot_for_paths( array $paths ): array { + $result = $this->snapshot(); + $result['diagnostics']['scoped_paths'] = array_values(array_filter($paths, fn( $path ) => is_string($path) && str_starts_with($path, '/'))); + return $result; + } } final class UnsupportedProcessPathProbe implements ProcessPathProbeInterface { @@ -48,6 +57,12 @@ public function snapshot(): array { ), ); } + + public function snapshot_for_paths( array $paths ): array { + $result = $this->snapshot(); + $result['diagnostics']['scoped_paths'] = array_values(array_filter($paths, fn( $path ) => is_string($path) && str_starts_with($path, '/'))); + return $result; + } } final class MacOSLsofProcessPathProbe implements ProcessPathProbeInterface { @@ -56,7 +71,21 @@ final class MacOSLsofProcessPathProbe implements ProcessPathProbeInterface { public function __construct(private $runner = null) {} public function snapshot(): array { + return $this->run_snapshot(array()); + } + + public function snapshot_for_paths( array $paths ): array { + $paths = array_values(array_unique(array_filter($paths, fn( $path ) => is_string($path) && str_starts_with($path, '/')))); + return $this->run_snapshot($paths); + } + + /** @param array $paths */ + private function run_snapshot( array $paths ): array { $argv = array( 'lsof', '-n', '-P', '-Fpcfn0' ); + if ( array() !== $paths ) { + $argv[] = '--'; + $argv = array_merge($argv, $paths); + } if ( is_callable($this->runner) ) { $result = ( $this->runner )($argv); } else { @@ -67,6 +96,17 @@ public function snapshot(): array { 'error_as_result' => true, )); } + if ( is_array($result) && empty($result['success']) && 1 === (int) ( $result['exit_code'] ?? 0 ) && '' === trim( (string) ( $result['output'] ?? '' ) ) ) { + return array( + 'status' => 'available', + 'records' => array(), + 'diagnostics' => array( + 'provider' => 'lsof', + 'path_records' => 0, + 'scoped_paths' => $paths, + ), + ); + } if ( $result instanceof \WP_Error || ! is_array($result) || empty($result['success']) ) { $data = $result instanceof \WP_Error ? (array) $result->get_error_data() : (array) $result; return array( @@ -147,6 +187,7 @@ public function snapshot(): array { 'diagnostics' => array( 'provider' => 'lsof', 'path_records' => count($records), + 'scoped_paths' => $paths, ), ); } diff --git a/inc/Workspace/WorkspaceArtifactCleanup.php b/inc/Workspace/WorkspaceArtifactCleanup.php index f31527a5..a16068fa 100644 --- a/inc/Workspace/WorkspaceArtifactCleanup.php +++ b/inc/Workspace/WorkspaceArtifactCleanup.php @@ -725,7 +725,7 @@ private function active_artifact_process_protection( string $worktree_path, arra 'protecting_reason' => 'active_build', 'reason' => 'active process cwd or open file intersects the worktree artifacts; leaving reconstructable artifacts in place', 'process_probe' => $probe, - 'process_evidence' => $evidence, + 'process_evidence' => $this->process_evidence_for_candidate($evidence, $worktree_path), ); } @@ -829,13 +829,37 @@ protected function detect_active_artifact_processes( string $worktree_path, arra } $snapshot = $this->artifact_process_path_records($fresh); - $records = (array) ( $snapshot['records'] ?? array() ); - $matches = array(); + $matches = $this->match_artifact_process_records((array) ( $snapshot['records'] ?? array() ), $roots); + + // A truncated host-wide lsof result cannot clear a candidate. On providers + // that support path-scoped inspection, retry only this candidate so unrelated + // sibling builds cannot poison its evidence. + if ( array() === $matches && 'uncertain' === (string) ( $snapshot['status'] ?? '' ) ) { + $scoped = $this->artifact_process_path_probe()->snapshot_for_paths($roots); + $matches = $this->match_artifact_process_records((array) ( $scoped['records'] ?? array() ), $roots); + if ( 'available' === (string) ( $scoped['status'] ?? '' ) || array() !== $matches ) { + $snapshot = $scoped; + } + } + + return array( + 'status' => (string) ( $snapshot['status'] ?? 'unavailable' ), + 'evidence' => $matches, + 'diagnostics' => (array) ( $snapshot['diagnostics'] ?? array() ), + ); + } + + /** @param array> $records @param array $roots @return array> */ + private function match_artifact_process_records( array $records, array $roots ): array { + $matches = array(); foreach ( $records as $record ) { $path = rtrim( (string) ( $record['path'] ?? '' ), '/'); + $real = realpath($path); + $path = false !== $real ? rtrim($real, '/') : $path; foreach ( $roots as $root ) { if ( $path === $root || str_starts_with($path, $root . '/') ) { - $matches[] = $record; + $record['matched_root'] = $root; + $matches[] = $record; break; } } @@ -843,11 +867,20 @@ protected function detect_active_artifact_processes( string $worktree_path, arra break; } } + return $matches; + } - return array( - 'status' => (string) ( $snapshot['status'] ?? 'unavailable' ), - 'evidence' => $matches, - 'diagnostics' => (array) ( $snapshot['diagnostics'] ?? array() ), + /** @param array> $evidence @return array> */ + private function process_evidence_for_candidate( array $evidence, string $worktree_path ): array { + return array_map( + function ( array $record ) use ( $worktree_path ): array { + return array_merge($record, array( + 'candidate_path' => $worktree_path, + 'match_method' => (string) ( $record['match_type'] ?? 'path_ancestry' ), + 'confidence' => 'high', + )); + }, + $evidence ); } diff --git a/tests/artifact-cleanup-live-process-guards.php b/tests/artifact-cleanup-live-process-guards.php index d13776ac..bd78328a 100644 --- a/tests/artifact-cleanup-live-process-guards.php +++ b/tests/artifact-cleanup-live-process-guards.php @@ -194,14 +194,26 @@ protected function detect_active_artifact_processes( string $worktree_path, arra ); } } + + final class ScopedArtifactCleanupGuardHarness extends ArtifactCleanupGuardHarness { + public function __construct( string $workspace_path, private \DataMachineCode\Support\ProcessPathProbeInterface $probe ) { + parent::__construct($workspace_path); + } + + protected function artifact_process_path_probe(): \DataMachineCode\Support\ProcessPathProbeInterface { + return $this->probe; + } + } } namespace { use DataMachineCode\Workspace\ArtifactCleanupGuardHarness; use DataMachineCode\Workspace\ControlledArtifactCleanupGuardHarness; use DataMachineCode\Workspace\MacOSArtifactCleanupGuardHarness; + use DataMachineCode\Workspace\ScopedArtifactCleanupGuardHarness; use DataMachineCode\Workspace\WorktreeContextInjector; use DataMachineCode\Support\MacOSLsofProcessPathProbe; + use DataMachineCode\Support\ProcessPathProbeInterface; use DataMachineCode\Cli\WorkspaceCompactOutput; use DataMachineCode\Abilities\WorkspaceAbilities; @@ -240,6 +252,66 @@ function artifact_guard_create_artifacts( string $path, bool $multiple = false ) } } + final class ScopedProcessProbe implements ProcessPathProbeInterface { + public function __construct(private array $global, private array $scoped) {} + public function snapshot(): array { return $this->global; } + public function snapshot_for_paths(array $paths): array { + foreach ($paths as $path) { + if (str_contains($path, '/repo@active')) { + return $this->scoped['active']; + } + if (str_contains($path, '/repo@inactive')) { + return $this->scoped['inactive']; + } + } + return array('status' => 'uncertain', 'records' => array(), 'diagnostics' => array('reason' => 'process_path_probe_incomplete')); + } + } + + $scoped_root = sys_get_temp_dir() . '/dmc-artifact-scoped-' . getmypid(); + $active_target = $scoped_root . '/repo@active/target'; + $inactive_target = $scoped_root . '/repo@inactive/target'; + foreach (array($active_target, $inactive_target) as $target) { + mkdir($target, 0777, true); + file_put_contents(dirname($target) . '/Cargo.toml', '[package]'); + file_put_contents($target . '/generated.bin', str_repeat('x', 1024)); + } + $scoped_probe = new ScopedProcessProbe( + array( + 'status' => 'uncertain', + 'records' => array(array('pid' => 4242, 'command' => 'cargo', 'match_type' => 'open_file', 'path' => $active_target)), + 'diagnostics' => array('provider' => 'lsof', 'reason' => 'process_path_probe_incomplete'), + ), + array( + 'active' => array('status' => 'available', 'records' => array(array('pid' => 4242, 'command' => 'cargo', 'match_type' => 'open_file', 'path' => $active_target)), 'diagnostics' => array('provider' => 'lsof', 'path_records' => 1)), + 'inactive' => array('status' => 'available', 'records' => array(), 'diagnostics' => array('provider' => 'lsof', 'path_records' => 0)), + ) + ); + $scoped_harness = new ScopedArtifactCleanupGuardHarness($scoped_root, $scoped_probe); + $scoped_harness->rows = array( + array('handle' => 'repo@active', 'repo' => 'repo', 'branch' => 'test/active', 'path' => dirname($active_target), 'is_worktree' => true, 'is_primary' => false, 'liveness' => WorktreeContextInjector::LIVENESS_STALE), + array('handle' => 'repo@inactive', 'repo' => 'repo', 'branch' => 'test/inactive', 'path' => dirname($inactive_target), 'is_worktree' => true, 'is_primary' => false, 'liveness' => WorktreeContextInjector::LIVENESS_STALE), + ); + $scoped_active_probe = $scoped_harness->probe_processes(dirname($active_target), array(array('path' => 'target'))); + artifact_guard_assert_same(4242, $scoped_active_probe['evidence'][0]['pid'] ?? null, 'candidate-scoped probe must retain active target evidence'); + $scoped_preview = $scoped_harness->worktree_cleanup_artifacts(array('dry_run' => true, 'safety_probes' => true)); + artifact_guard_assert_same(array('repo@inactive'), array_column($scoped_preview['candidates'], 'handle'), 'a sibling Cargo process must not make an inactive target probe uncertain'); + artifact_guard_assert_same('active_build', $scoped_preview['skipped'][0]['reason_code'] ?? null, 'the active Cargo target must remain protected'); + artifact_guard_assert_same(4242, $scoped_preview['skipped'][0]['process_evidence'][0]['pid'] ?? null, 'active-process skips must report the PID'); + artifact_guard_assert_same(dirname($active_target), $scoped_preview['skipped'][0]['process_evidence'][0]['candidate_path'] ?? null, 'active-process skips must report the candidate path'); + artifact_guard_assert_same('open_file', $scoped_preview['skipped'][0]['process_evidence'][0]['match_method'] ?? null, 'active-process skips must report the match method'); + artifact_guard_assert_same('high', $scoped_preview['skipped'][0]['process_evidence'][0]['confidence'] ?? null, 'active-process skips must report match confidence'); + $mixed_apply = $scoped_harness->worktree_cleanup_artifacts(array('apply_plan' => array('candidates' => array( + array('handle' => 'repo@active', 'repo' => 'repo', 'branch' => 'test/guard', 'path' => dirname($active_target), 'artifacts' => array(array('path' => 'target'))), + array('handle' => 'repo@inactive', 'repo' => 'repo', 'branch' => 'test/guard', 'path' => dirname($inactive_target), 'artifacts' => array(array('path' => 'target'))), + )))); + artifact_guard_assert_same(1, count($mixed_apply['removed']), 'mixed apply must reclaim the independent inactive target'); + artifact_guard_assert_same(1, count($mixed_apply['skipped']), 'mixed apply must retain the active target'); + artifact_guard_assert_same(true, (int) ($mixed_apply['summary']['removed_size_bytes'] ?? 0) > 0, 'mixed apply must report nonzero reclaimed bytes'); + artifact_guard_assert_same(true, is_dir($active_target), 'mixed apply must retain the active Cargo target'); + artifact_guard_assert_same(false, is_dir($inactive_target), 'mixed apply must remove the inactive Cargo target'); + artifact_guard_remove_tree($scoped_root); + $root = sys_get_temp_dir() . '/dmc-artifact-guards-' . getmypid(); $path = $root . '/repo@guard'; mkdir($path, 0777, true); @@ -303,6 +375,8 @@ function artifact_guard_create_artifacts( string $path, bool $multiple = false ) $mac_no_match = new MacOSLsofProcessPathProbe(fn( array $argv ) => array( 'success' => true, 'output' => "p42\0cnode\0f3\0n/tmp/unrelated\0" )); artifact_guard_assert_same('available', $mac_no_match->snapshot()['status'], 'macOS lsof no-match snapshot must be available'); + $mac_scoped_no_match = new MacOSLsofProcessPathProbe(fn( array $argv ) => array( 'success' => false, 'exit_code' => 1, 'output' => '' )); + artifact_guard_assert_same('available', $mac_scoped_no_match->snapshot_for_paths(array($path))['status'], 'macOS scoped lsof exit 1 without output must be available no-match evidence'); $mac_no_process_harness = new MacOSArtifactCleanupGuardHarness($root, new MacOSLsofProcessPathProbe(fn( array $argv ) => array( 'success' => true, 'output' => '' ))); $mac_no_process_harness->rows = array($base); $mac_no_process = $mac_no_process_harness->worktree_cleanup_artifacts(array( 'dry_run' => true, 'safety_probes' => true ));