Skip to content

Commit f7265f4

Browse files
homeboy-ci[bot]homeboy-ci[bot]
committed
Fix worktree cleanup merged classifier
1 parent 4a564c6 commit f7265f4

4 files changed

Lines changed: 139 additions & 5 deletions

File tree

‎inc/Cli/Commands/WorkspaceCommand.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4081,6 +4081,9 @@ public function worktree( array $args, array $assoc_args ): void {
40814081
break;
40824082

40834083
case 'cleanup':
4084+
if ( ! empty($args[1]) ) {
4085+
$input['repo'] = (string) $args[1];
4086+
}
40844087
$input['dry_run'] = ! empty($assoc_args['dry-run']);
40854088
$input['force'] = ! empty($assoc_args['force']);
40864089
$input['skip_github'] = ! empty($assoc_args['skip-github']);

‎inc/Workspace/WorkspaceActiveNoSignalCleanup.php‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,14 @@ public function worktree_active_no_signal_merged_apply( array $opts = array() ):
266266
return array( 'report_action_counts' => $report['summary']['by_suggested_action'] ?? array() );
267267
},
268268
'prepare_row' => function ( array $row ): array|\WP_Error {
269-
if ( 'merged_to_default' !== (string) ( $row['suggested_action'] ?? '' ) ) {
269+
$is_merged_to_default = 'merged_to_default' === (string) ( $row['suggested_action'] ?? '' )
270+
|| (
271+
0 === (int) ( $row['dirty'] ?? -1 )
272+
&& 0 === (int) ( $row['unpushed'] ?? -1 )
273+
&& 0 === (int) ( $row['commits_outside_default'] ?? -1 )
274+
);
275+
276+
if ( ! $is_merged_to_default ) {
270277
return new \WP_Error('not_merged_to_default', 'row is not a clean merged-to-default candidate');
271278
}
272279

@@ -1699,6 +1706,10 @@ private function is_generated_or_artifact_path( string $path ): bool {
16991706
* @return string
17001707
*/
17011708
private function suggest_active_no_signal_action( array $row ): string {
1709+
if ( 0 === (int) ( $row['dirty'] ?? -1 ) && 0 === (int) ( $row['unpushed'] ?? -1 ) && 0 === (int) ( $row['commits_outside_default'] ?? -1 ) ) {
1710+
return 'merged_to_default';
1711+
}
1712+
17021713
$effective_status = (string) ( $row['upstream_equivalence']['effective_status'] ?? '' );
17031714
if ( 'equivalent_clean' === $effective_status ) {
17041715
return 'patch_equivalent_default';
@@ -1719,10 +1730,6 @@ private function suggest_active_no_signal_action( array $row ): string {
17191730
return 'active_open_pr';
17201731
}
17211732

1722-
if ( 0 === (int) ( $row['commits_outside_default'] ?? -1 ) ) {
1723-
return 'merged_to_default';
1724-
}
1725-
17261733
if ( true === ( $row['remote_tracking'] ?? null ) ) {
17271734
return 'remote_tracking_clean';
17281735
}

‎tests/worktree-cleanup-cli-scope.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
$source = file_get_contents(dirname(__DIR__) . '/inc/Cli/Commands/WorkspaceCommand.php');
6+
if ( false === $source ) {
7+
throw new RuntimeException('Unable to read WorkspaceCommand.php');
8+
}
9+
10+
$cleanup_case_start = strpos($source, "case 'cleanup':");
11+
if ( false === $cleanup_case_start ) {
12+
throw new RuntimeException('cleanup CLI case was not found');
13+
}
14+
15+
$cleanup_case_end = strpos($source, "case 'reconcile-metadata':", $cleanup_case_start);
16+
if ( false === $cleanup_case_end ) {
17+
throw new RuntimeException('cleanup CLI case end was not found');
18+
}
19+
20+
$cleanup_case = substr($source, $cleanup_case_start, $cleanup_case_end - $cleanup_case_start);
21+
if ( ! str_contains($cleanup_case, '$input[\'repo\'] = (string) $args[1];') ) {
22+
throw new RuntimeException('workspace worktree cleanup must forward its positional repo/worktree scope into ability input');
23+
}
24+
25+
echo "worktree-cleanup-cli-scope: ok\n";

‎tests/worktree-cleanup-patch-equivalence.php‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@ function is_wp_error( mixed $thing ): bool {
4040

4141
require_once dirname(__DIR__) . '/inc/Support/CommandSpec.php';
4242
require_once dirname(__DIR__) . '/inc/Support/RuntimeCapabilities.php';
43+
require_once dirname(__DIR__) . '/inc/Support/PathSecurity.php';
4344
require_once dirname(__DIR__) . '/inc/Support/ProcessRunner.php';
4445
require_once dirname(__DIR__) . '/inc/Support/GitRunner.php';
4546
require_once dirname(__DIR__) . '/inc/Workspace/WorkspaceHandle.php';
47+
require_once dirname(__DIR__) . '/inc/Workspace/WorktreeContextInjector.php';
4648
require_once dirname(__DIR__) . '/inc/Workspace/WorkspaceCoreUtilities.php';
4749
require_once dirname(__DIR__) . '/inc/Workspace/WorkspaceActiveNoSignalCleanup.php';
4850
require_once dirname(__DIR__) . '/inc/Workspace/WorkspaceWorktreeCleanupEngine.php';
@@ -137,4 +139,101 @@ protected function resolve_remote_default_ref( string $primary_path, int $timeou
137139
worktree_cleanup_patch_equivalence_assert_same(0, $evidence['git_cherry']['unmatched'], 'no unmatched commits are promoted');
138140
worktree_cleanup_patch_equivalence_assert_same('preserve_local_branch', $evidence['local_branch_handling'], 'cleanup preserves non-contained local branch');
139141

142+
$suggest_action = new ReflectionMethod($cleanup, 'suggest_active_no_signal_action');
143+
$merged_action = $suggest_action->invoke(
144+
$cleanup,
145+
array(
146+
'dirty' => 0,
147+
'unpushed' => 0,
148+
'commits_outside_default' => 0,
149+
'remote_tracking' => true,
150+
'upstream_equivalence' => array(
151+
'effective_status' => 'equivalent_clean',
152+
),
153+
)
154+
);
155+
worktree_cleanup_patch_equivalence_assert_same('merged_to_default', $merged_action, 'exact clean containment in the remote default branch should win over patch-equivalence and remote-tracking labels');
156+
157+
$merged = $root . '/merged-worktree';
158+
worktree_cleanup_patch_equivalence_run($primary, 'git worktree add -b merged-feature ../merged-worktree origin/main');
159+
worktree_cleanup_patch_equivalence_run($merged, 'git config user.email test@example.com');
160+
worktree_cleanup_patch_equivalence_run($merged, 'git config user.name Test');
161+
file_put_contents($merged . '/merged.txt', "merged\n");
162+
worktree_cleanup_patch_equivalence_run($merged, 'git add merged.txt');
163+
worktree_cleanup_patch_equivalence_run($merged, 'git commit -m merged-feature');
164+
worktree_cleanup_patch_equivalence_run($primary, 'git merge --ff-only merged-feature');
165+
worktree_cleanup_patch_equivalence_run($primary, 'git push origin main');
166+
167+
$merged_cleanup = new class($root) {
168+
use WorkspaceCoreUtilities;
169+
use WorkspaceActiveNoSignalCleanup;
170+
use WorkspaceWorktreeCleanupEngine;
171+
172+
protected const CLEANUP_GIT_PROBE_TIMEOUT = 5;
173+
174+
protected string $workspace_path;
175+
176+
public function __construct( string $workspace_path ) {
177+
$this->workspace_path = $workspace_path;
178+
}
179+
180+
public function get_primary_path( string $repo ): string {
181+
return $this->workspace_path . '/primary';
182+
}
183+
184+
protected function resolve_remote_default_ref( string $primary_path, int $timeout_seconds = 0 ): string|WP_Error|null {
185+
return 'refs/remotes/origin/main';
186+
}
187+
188+
/** @return array<int,string> */
189+
protected function protected_base_branch_names(): array {
190+
return array( 'main', 'master', 'trunk', 'develop' );
191+
}
192+
193+
protected function resolve_worktree_branch_from_head_file( string $wt_path ): ?string {
194+
$result = $this->run_git($wt_path, 'branch --show-current', self::CLEANUP_GIT_PROBE_TIMEOUT);
195+
if ( is_wp_error($result) ) {
196+
return null;
197+
}
198+
199+
$branch = trim((string) ( $result['output'] ?? '' ));
200+
return '' === $branch ? null : $branch;
201+
}
202+
203+
protected function probe_worktree_dirty_count( string $path, int $timeout_seconds = 0 ): int|WP_Error {
204+
$result = $this->run_git($path, 'status --porcelain', $timeout_seconds);
205+
if ( is_wp_error($result) ) {
206+
return $result;
207+
}
208+
209+
$lines = array_filter(array_map('trim', explode("\n", (string) ( $result['output'] ?? '' ))));
210+
return count($lines);
211+
}
212+
213+
/** @return array<string,mixed> */
214+
public function worktree_active_no_signal_report( array $opts = array() ): array {
215+
return array(
216+
'success' => true,
217+
'rows' => array(
218+
array(
219+
'handle' => 'repo@merged-feature',
220+
'repo' => 'repo',
221+
'branch' => 'merged-feature',
222+
'path' => $this->workspace_path . '/merged-worktree',
223+
'dirty' => 0,
224+
'unpushed' => 0,
225+
'commits_outside_default' => 0,
226+
'suggested_action' => 'remote_tracking_clean',
227+
),
228+
),
229+
'summary' => array( 'inspected' => 1 ),
230+
);
231+
}
232+
};
233+
234+
$merged_apply = $merged_cleanup->worktree_active_no_signal_merged_apply(array( 'dry_run' => true ));
235+
worktree_cleanup_patch_equivalence_assert_same(true, is_array($merged_apply), 'merged apply should return a dry-run result');
236+
worktree_cleanup_patch_equivalence_assert_same(1, $merged_apply['summary']['planned'], 'merged apply should plan clean rows with zero commits outside default even when an older report label was remote_tracking_clean');
237+
worktree_cleanup_patch_equivalence_assert_same('repo@merged-feature', $merged_apply['planned'][0]['handle'], 'merged apply should preserve the planned handle');
238+
140239
echo "worktree-cleanup-patch-equivalence: ok\n";

0 commit comments

Comments
 (0)