Merge pull request #405 from Extra-Chill/fix/worktree-base-branch-warnings

Warn on base branches checked out in worktrees

Author: chubes4

inc/Cli/Commands/WorkspaceCommand.php

@@ -2515,12 +2515,19 @@ function ( $wt ) {
 				}
 				$this->format_items( $items, $fields, $assoc_args, 'handle' );
 				$duplicates = (array) ( $result['duplicates'] ?? array() );
+				$base_branch_worktrees = (array) ( $result['base_branch_worktrees'] ?? array() );
 				if ( ! empty( $duplicates ) && ! in_array( (string) ( $assoc_args['format'] ?? '' ), array( 'json', 'yaml' ), true ) ) {
 					WP_CLI::log( sprintf( 'Duplicate task ownership groups: %d', count( $duplicates ) ) );
 					foreach ( $duplicates as $group ) {
 						WP_CLI::log( sprintf( '  - [%s=%s] %s', (string) ( $group['kind'] ?? '' ), (string) ( $group['key'] ?? '' ), implode( ', ', (array) ( $group['handles'] ?? array() ) ) ) );
 					}
 				}
+				if ( ! empty( $base_branch_worktrees ) && ! in_array( (string) ( $assoc_args['format'] ?? '' ), array( 'json', 'yaml' ), true ) ) {
+					WP_CLI::warning( sprintf( 'Base branch checked out in %d non-primary worktree%s; gh pr merge --delete-branch can merge remotely but fail local cleanup.', count( $base_branch_worktrees ), 1 === count( $base_branch_worktrees ) ? '' : 's' ) );
+					foreach ( $base_branch_worktrees as $warning ) {
+						WP_CLI::log( sprintf( '  - %s (%s) at %s', (string) ( $warning['handle'] ?? '' ), (string) ( $warning['branch'] ?? '' ), (string) ( $warning['path'] ?? '' ) ) );
+					}
+				}
 				return;
 
 			case 'prune':

inc/Workspace/WorkspaceHygieneReport.php

@@ -75,6 +75,8 @@ public function workspace_hygiene_report( array $opts = array() ): array|\WP_Err
 				$cleanup       = null;
 			}
 		}
+		$worktree_summary = $this->summarize_workspace_worktrees( $worktrees, $cleanup );
+
 		return array(
 			'success'                   => true,
 			'generated_at'              => gmdate( 'c' ),
@@ -86,7 +88,7 @@ public function workspace_hygiene_report( array $opts = array() ): array|\WP_Err
 				'freshness' => $this->worktree_inventory()->freshness(),
 				'refresh'   => $inventory_refresh,
 			),
-			'worktrees'                 => $this->summarize_workspace_worktrees( $worktrees, $cleanup ),
+			'worktrees'                 => $worktree_summary,
 			'worktree_status_mode'      => $worktree_status_mode,
 			'top_repos_by_worktrees'    => $this->top_repos_by_worktree_count( $worktrees, 10 ),
 			'top_repos_by_size'         => $this->top_repos_by_size( (array) ( $size_report['entries'] ?? array() ), 10 ),
@@ -97,6 +99,7 @@ public function workspace_hygiene_report( array $opts = array() ): array|\WP_Err
 				$include_sizes ? (string) ( $size_report['mode_note'] ?? '' ) : 'Size scan disabled by request.',
 				$include_worktree_status ? 'Full worktree status enabled; this may run git status across every worktree.' : 'Worktree status uses cheap top-level inventory; pass --include-worktree-status for full git status.',
 				$include_cleanup ? 'Cleanup summary uses inventory-only dry-run detection (--inventory-only --skip-github); no per-worktree git probes or GitHub API lookups are required.' : 'Cleanup dry-run disabled by request.',
+				! empty( $worktree_summary['base_branch_worktree_count'] ) ? 'One or more non-primary worktrees have a base branch checked out; gh pr merge --delete-branch can merge remotely but fail local cleanup.' : '',
 			) ) ),
 		);
 	}
@@ -342,14 +345,15 @@ private function build_workspace_inventory_rows(): array {
 			$session_view = WorktreeContextInjector::summarize_session( is_array( $metadata ) ? $metadata : null );
 			$task_view    = is_array( $metadata ) && is_array( $metadata['origin_task'] ?? null ) ? $metadata['origin_task'] : null;
 
-			$rows[] = array(
+			$row = array(
 				'handle'           => $parsed['dir_name'],
 				'repo'             => $parsed['repo'],
 				'kind'             => $kind,
 				'is_worktree'      => $is_worktree,
 				'is_primary'       => 'primary' === $kind,
 				'external'         => false,
 				'branch_slug'      => $parsed['branch_slug'],
+				'branch'           => is_array( $metadata ) && ! empty( $metadata['branch'] ) ? (string) $metadata['branch'] : (string) ( $parsed['branch_slug'] ?? '' ),
 				'path'             => $path,
 				'dirty'            => 0,
 				'created_at'       => is_array( $metadata ) ? ( $metadata['created_at'] ?? null ) : null,
@@ -366,6 +370,13 @@ private function build_workspace_inventory_rows(): array {
 				'missing_metadata' => $is_worktree && ! is_array( $metadata ),
 				'metadata'         => $metadata,
 			);
+
+			$base_branch_warning = $this->base_branch_worktree_warning( $row );
+			if ( null !== $base_branch_warning ) {
+				$row['base_branch_warning'] = $base_branch_warning;
+			}
+
+			$rows[] = $row;
 		}
 
 		return $rows;
@@ -523,6 +534,8 @@ private function summarize_workspace_worktrees( array $worktrees, ?array $cleanu
 			'protected_dirty'    => 0,
 			'protected_unpushed' => 0,
 			'missing_metadata'   => 0,
+			'base_branch_worktree_count' => 0,
+			'base_branch_worktrees' => array(),
 			'by_liveness'        => array(
 				WorktreeContextInjector::LIVENESS_LIVE    => 0,
 				WorktreeContextInjector::LIVENESS_STOPPED => 0,
@@ -553,6 +566,11 @@ private function summarize_workspace_worktrees( array $worktrees, ?array $cleanu
 				++$summary['missing_metadata'];
 			}
 
+			if ( ! empty( $row['base_branch_warning'] ) && is_array( $row['base_branch_warning'] ) ) {
+				++$summary['base_branch_worktree_count'];
+				$summary['base_branch_worktrees'][] = $row['base_branch_warning'];
+			}
+
 			if ( ! empty( $row['is_worktree'] ) ) {
 				$liveness = (string) ( $row['liveness'] ?? WorktreeContextInjector::LIVENESS_UNKNOWN );
 				if ( ! isset( $summary['by_liveness'][ $liveness ] ) ) {

inc/Workspace/WorkspaceWorktreeLifecycle.php

@@ -807,6 +807,11 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra
 					$disk
 				);
 
+				$base_branch_warning = $this->base_branch_worktree_warning( $row );
+				if ( null !== $base_branch_warning ) {
+					$row['base_branch_warning'] = $base_branch_warning;
+				}
+
 				if ( ! empty( $skipped_groups ) ) {
 					$row['fields_skipped'] = $skipped_groups;
 				}
@@ -815,16 +820,60 @@ public function worktree_list( ?string $repo = null, ?string $state = null, arra
 			}
 		}
 
-		$duplicates = WorktreeContextInjector::find_duplicate_task_ownership( $worktrees );
+		$duplicates             = WorktreeContextInjector::find_duplicate_task_ownership( $worktrees );
+		$base_branch_worktrees  = array_values( array_filter( array_map(
+			fn( $row ) => $row['base_branch_warning'] ?? null,
+			$worktrees
+		) ) );
 
 		return array(
-			'success'        => true,
-			'worktrees'      => $worktrees,
-			'duplicates'     => $duplicates,
-			'fields_skipped' => $skipped_groups,
+			'success'                   => true,
+			'worktrees'                 => $worktrees,
+			'duplicates'                => $duplicates,
+			'base_branch_worktrees'     => $base_branch_worktrees,
+			'fields_skipped'            => $skipped_groups,
+		);
+	}
+
+	/**
+	 * Return warning metadata when a non-primary worktree holds a base branch.
+	 *
+	 * GitHub CLI merge flows may try to check out or delete the PR base branch
+	 * during local cleanup. If another worktree has that branch checked out, the
+	 * remote merge can succeed while local cleanup reports a fatal git error.
+	 *
+	 * @param array<string,mixed> $row Worktree listing row.
+	 * @return array<string,string>|null
+	 */
+	private function base_branch_worktree_warning( array $row ): ?array {
+		if ( empty( $row['is_worktree'] ) || ! empty( $row['is_primary'] ) || ! empty( $row['external'] ) ) {
+			return null;
+		}
+
+		$branch = (string) ( $row['branch'] ?? '' );
+		if ( '' === $branch || ! in_array( $branch, $this->protected_base_branch_names(), true ) ) {
+			return null;
+		}
+
+		return array(
+			'handle'       => (string) ( $row['handle'] ?? '' ),
+			'repo'         => (string) ( $row['repo'] ?? '' ),
+			'branch'       => $branch,
+			'path'         => (string) ( $row['path'] ?? '' ),
+			'reason_code'  => 'base_branch_checked_out_in_worktree',
+			'message'      => sprintf( 'Worktree %s has base branch %s checked out; gh pr merge --delete-branch may merge remotely but fail local cleanup.', (string) ( $row['handle'] ?? '' ), $branch ),
 		);
 	}
 
+	/**
+	 * Branch names that should normally be held by primaries, not feature worktrees.
+	 *
+	 * @return array<int,string>
+	 */
+	private function protected_base_branch_names(): array {
+		return array( 'main', 'master', 'trunk', 'develop' );
+	}
+
 	/**
 	 * Refresh the DB-backed worktree inventory from the current filesystem/git view.
 	 *

tests/smoke-worktree-list-scaling.php

@@ -188,6 +188,8 @@ function size_format( $bytes ): string {
 	$run( sprintf( 'git worktree add %s feat-one', escapeshellarg( $tmp . '/demo@feat-one' ) ), $primary );
 	$run( sprintf( 'git worktree add %s feat-two', escapeshellarg( $tmp . '/demo@feat-two' ) ), $primary );
 	$run( sprintf( 'git worktree add %s feat-three', escapeshellarg( $tmp . '/demo@feat-three' ) ), $primary );
+	$run( 'git checkout -b local-maintenance', $primary );
+	$run( sprintf( 'git worktree add %s main', escapeshellarg( $tmp . '/demo@main' ) ), $primary );
 
 	// Dirty one of them so the status probe has something to count.
 	file_put_contents( $tmp . '/demo@feat-two/scratch.txt', 'dirty' );
@@ -232,7 +234,7 @@ function size_format( $bytes ): string {
 	$assert( true, ! is_wp_error( $cheap_res ) && ( $cheap_res['success'] ?? false ), 'cheap listing returns success' );
 	$assert( array( 'status', 'disk' ), $cheap_res['fields_skipped'] ?? null, 'cheap listing reports both probe groups skipped' );
 	$cheap_rows = $cheap_res['worktrees'] ?? array();
-	$assert( 4, count( $cheap_rows ), 'cheap listing still enumerates every worktree' );
+	$assert( 5, count( $cheap_rows ), 'cheap listing still enumerates every worktree' );
 	$cheap_two  = array_values( array_filter( $cheap_rows, fn( $r ) => ( $r['handle'] ?? '' ) === 'demo@feat-two' ) )[0] ?? array();
 	// dirty=null is the structural signal that the per-row git status probe was skipped.
 	$assert( true, array_key_exists( 'dirty', $cheap_two ) && null === $cheap_two['dirty'], 'cheap listing leaves dirty as null (status probe skipped)' );
@@ -245,6 +247,14 @@ function size_format( $bytes ): string {
 	$assert( 'demo', $cheap_two['repo'] ?? '', 'cheap row carries repo' );
 	$assert( 'feat-two', $cheap_two['branch'] ?? '', 'cheap row carries branch' );
 	$assert( true, ! empty( $cheap_two['head'] ), 'cheap row carries head' );
+	$cheap_main = array_values( array_filter( $cheap_rows, fn( $r ) => ( $r['handle'] ?? '' ) === 'demo@main' ) )[0] ?? array();
+	$assert( 'base_branch_checked_out_in_worktree', $cheap_main['base_branch_warning']['reason_code'] ?? '', 'cheap listing flags non-primary worktree checked out on main' );
+	$assert( 1, count( $cheap_res['base_branch_worktrees'] ?? array() ), 'cheap listing exposes base branch worktree warnings at top level' );
+	$hygiene_res = $ws->workspace_hygiene_report( array(
+		'include_cleanup' => false,
+		'include_sizes'   => false,
+	) );
+	$assert( 1, (int) ( $hygiene_res['worktrees']['base_branch_worktree_count'] ?? 0 ), 'hygiene report summarizes base branch worktree warnings' );
 
 	// Lifecycle / metadata-driven stale_reason still works without probes.
 	// feat-two has NO lifecycle metadata → missing_metadata still fires.