From 2a7fdefb92dbf25897ef48a35db1a5588706e3d2 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 20 May 2026 08:41:09 -0400 Subject: [PATCH 1/2] fix: preserve local PR cleanup results --- inc/Abilities/GitHubAbilities.php | 25 +++++++++++++- inc/Cli/Commands/GitHubCommand.php | 4 +++ inc/Tools/GitHubTools.php | 6 +++- tests/smoke-github-merge-pull-request.php | 42 +++++++++++++++++++++++ 4 files changed, 75 insertions(+), 2 deletions(-) diff --git a/inc/Abilities/GitHubAbilities.php b/inc/Abilities/GitHubAbilities.php index 0ee253e..8e6a292 100644 --- a/inc/Abilities/GitHubAbilities.php +++ b/inc/Abilities/GitHubAbilities.php @@ -578,6 +578,10 @@ private function registerAbilities(): void { 'type' => 'boolean', 'description' => 'Preview the cleanup decision without deleting the branch.', ), + 'local_only' => array( + 'type' => 'boolean', + 'description' => 'Finalize and remove the matching local DMC worktree without deleting the remote branch.', + ), ), ), 'output_schema' => array( @@ -2376,7 +2380,7 @@ public static function mergePullRequest( array $input, ?callable $api_get = null * from a linked worktree where the base branch is already checked out in the * primary checkout. * - * @param array $input Required: repo, pull_number. Optional: dry_run. + * @param array $input Required: repo, pull_number. Optional: dry_run, local_only. * @param callable|null $api_get Optional test seam: fn(string $url, array $query, string $pat): array|WP_Error. * @param callable|null $api_request Optional test seam: fn(string $method, string $url, ?array $body, string $pat): array|WP_Error. * @return array|\WP_Error Success payload or error. @@ -2385,6 +2389,7 @@ public static function cleanupPullRequest( array $input, ?callable $api_get = nu $repo = sanitize_text_field( $input['repo'] ?? '' ); $pull_number = (int) ( $input['pull_number'] ?? 0 ); $dry_run = ! empty( $input['dry_run'] ); + $local_only = ! empty( $input['local_only'] ); if ( empty( $repo ) || $pull_number <= 0 ) { return new \WP_Error( 'missing_params', 'Repository and pull_number are required.', array( 'status' => 400 ) ); @@ -2434,6 +2439,8 @@ public static function cleanupPullRequest( array $input, ?callable $api_get = nu 'branch_deleted' => false, 'branch_already_deleted' => false, 'dry_run' => $dry_run, + 'local_only' => $local_only, + 'partial_success' => false, 'pull_request_url' => (string) ( $pull['html_url'] ?? '' ), ); @@ -2453,12 +2460,28 @@ public static function cleanupPullRequest( array $input, ?callable $api_get = nu $result['local_worktree_cleanup'] = $local_cleanup; } + if ( $local_only ) { + $result['message'] = sprintf( 'Cleaned up local DMC worktree for %s without deleting remote branch %s.', $repo, $head_branch ); + return $result; + } + $encoded_head_branch = implode( '/', array_map( 'rawurlencode', explode( '/', $head_branch ) ) ); $delete_url = sprintf( '%s/repos/%s/git/refs/heads/%s', self::API_BASE, $repo, $encoded_head_branch ); $deleted = $api_request( 'DELETE', $delete_url, null, $pat ); if ( is_wp_error( $deleted ) ) { $status = is_array( $deleted->get_error_data() ) ? (int) ( $deleted->get_error_data()['status'] ?? 0 ) : 0; if ( 404 !== $status ) { + if ( true === ( $result['local_worktree_cleanup']['success'] ?? false ) ) { + $result['partial_success'] = true; + $result['branch_delete_error'] = array( + 'code' => $deleted->get_error_code(), + 'message' => $deleted->get_error_message(), + 'status' => $status, + ); + $result['message'] = sprintf( 'Cleaned up local DMC worktree for %s, but could not delete remote branch %s: %s', $repo, $head_branch, $deleted->get_error_message() ); + return $result; + } + return $deleted; } diff --git a/inc/Cli/Commands/GitHubCommand.php b/inc/Cli/Commands/GitHubCommand.php index e4b5058..764e100 100644 --- a/inc/Cli/Commands/GitHubCommand.php +++ b/inc/Cli/Commands/GitHubCommand.php @@ -458,6 +458,9 @@ public function merge_pull( array $args, array $assoc_args ): void { * [--dry-run] * : Preview the cleanup decision without deleting the branch. * + * [--local-only] + * : Finalize and remove the matching local DMC worktree without deleting the remote branch. + * * [--format=] * : Output format. * --- @@ -487,6 +490,7 @@ public function cleanup_pull( array $args, array $assoc_args ): void { 'repo' => $assoc_args['repo'] ?? '', 'pull_number' => $pull_number, 'dry_run' => ! empty( $assoc_args['dry-run'] ), + 'local_only' => ! empty( $assoc_args['local-only'] ), ) ); $result = GitHubAbilities::cleanupPullRequest( $input ); diff --git a/inc/Tools/GitHubTools.php b/inc/Tools/GitHubTools.php index 8ae09c1..a27f42f 100644 --- a/inc/Tools/GitHubTools.php +++ b/inc/Tools/GitHubTools.php @@ -663,7 +663,7 @@ public function getCleanupPullRequestDefinition(): array { return array( 'class' => __CLASS__, 'method' => 'handleCleanupPullRequest', - 'description' => 'Delete a merged pull request head branch through the GitHub API without checking out or switching local branches. Supports dry_run previews.', + 'description' => 'Cleanup a merged pull request by finalizing the matching local DMC worktree before optional remote branch deletion. Supports dry_run previews and local_only cleanup.', 'parameters' => array( 'type' => 'object', 'properties' => array( @@ -679,6 +679,10 @@ public function getCleanupPullRequestDefinition(): array { 'type' => 'boolean', 'description' => 'Preview the cleanup decision without deleting the branch.', ), + 'local_only' => array( + 'type' => 'boolean', + 'description' => 'Finalize and remove the matching local DMC worktree without deleting the remote branch.', + ), ), 'required' => array( 'repo', 'pull_number' ), ), diff --git a/tests/smoke-github-merge-pull-request.php b/tests/smoke-github-merge-pull-request.php index c3f9762..242fa6a 100644 --- a/tests/smoke-github-merge-pull-request.php +++ b/tests/smoke-github-merge-pull-request.php @@ -253,6 +253,7 @@ function wp_get_ability( string $name ): ?DMC_Test_Ability { $cleanup_ability = $GLOBALS['dmc_registered_abilities']['datamachine/cleanup-github-pull-request'] ?? null; $assert( 'cleanup ability is registered', null !== $cleanup_ability ); $assert( 'cleanup ability uses cleanupPullRequest execute_callback', array( GitHubAbilities::class, 'cleanupPullRequest' ) === ( $cleanup_ability['execute_callback'] ?? null ) ); + $assert( 'cleanup ability exposes local_only', isset( $cleanup_ability['input_schema']['properties']['local_only'] ) ); $permission = $ability['permission_callback'] ?? null; PermissionHelper::$can_manage_result = false; @@ -377,6 +378,46 @@ static function ( string $method, string $url, ?array $body, string $pat ) use ( $assert( 'cleanupPullRequest removes matching DMC worktree before remote branch delete', array( 'repo' => 'owner/repo', 'branch' => 'feature/test', 'pr_url' => 'https://github.com/owner/repo/pull/42' ) === ( Workspace::$cleanup_calls[0] ?? array() ) ); $assert( 'cleanupPullRequest returns local worktree cleanup evidence', true === ( $result['local_worktree_cleanup']['found'] ?? false ) ); + $calls = array(); + Workspace::$cleanup_calls = array(); + $result = GitHubAbilities::cleanupPullRequest( + array( + 'repo' => 'owner/repo', + 'pull_number' => 42, + 'local_only' => true, + ), + static function ( string $url, array $query, string $pat ) use ( &$calls, $merged_pull ): array { + $calls[] = array( 'method' => 'GET', 'url' => $url, 'query' => $query, 'pat' => $pat ); + return $merged_pull(); + }, + static function ( string $method, string $url, ?array $body, string $pat ) use ( &$calls ): array { + $calls[] = compact( 'method', 'url', 'body', 'pat' ); + return array( 'success' => true, 'data' => null ); + } + ); + $assert( 'cleanupPullRequest local_only succeeds', is_array( $result ) && true === ( $result['local_only'] ?? false ) ); + $assert( 'cleanupPullRequest local_only skips remote branch delete', 1 === count( $calls ) ); + $assert( 'cleanupPullRequest local_only returns local cleanup evidence', true === ( $result['local_worktree_cleanup']['found'] ?? false ) ); + + $calls = array(); + Workspace::$cleanup_calls = array(); + $result = GitHubAbilities::cleanupPullRequest( + array( + 'repo' => 'owner/repo', + 'pull_number' => 42, + ), + static function ( string $url, array $query, string $pat ) use ( &$calls, $merged_pull ): array { + $calls[] = array( 'method' => 'GET', 'url' => $url, 'query' => $query, 'pat' => $pat ); + return $merged_pull(); + }, + static function ( string $method, string $url, ?array $body, string $pat ) use ( &$calls ): WP_Error { + $calls[] = compact( 'method', 'url', 'body', 'pat' ); + return new WP_Error( 'github_api_error', 'GitHub API error (403): Resource not accessible by integration', array( 'status' => 403 ) ); + } + ); + $assert( 'cleanupPullRequest preserves local cleanup on remote delete failure', is_array( $result ) && true === ( $result['partial_success'] ?? false ) ); + $assert( 'cleanupPullRequest reports remote delete error without losing local evidence', 403 === ( $result['branch_delete_error']['status'] ?? 0 ) && true === ( $result['local_worktree_cleanup']['found'] ?? false ) ); + $result = GitHubAbilities::cleanupPullRequest( array( 'repo' => 'owner/repo', @@ -418,6 +459,7 @@ static function ( string $method, string $url, ?array $body, string $pat ) use ( $tool_call = $GLOBALS['dmc_tool_ability_calls'][0] ?? array(); $assert( 'merge tool calls merge ability', 'datamachine/merge-github-pull-request' === ( $tool_call['name'] ?? '' ) ); $cleanup_definition = $tools->getCleanupPullRequestDefinition(); + $assert( 'cleanup tool exposes local_only', isset( $cleanup_definition['parameters']['properties']['local_only'] ) ); $cleanup_result = $tools->handle_tool_call( array( 'repo' => 'owner/repo', From e2d707bdfc2693493674532cfdb6e2719f8fc89a Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 20 May 2026 08:47:59 -0400 Subject: [PATCH 2/2] fix: satisfy github cleanup lint --- inc/Cli/Commands/GitHubCommand.php | 14 +++++++------- inc/Tools/GitHubTools.php | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/inc/Cli/Commands/GitHubCommand.php b/inc/Cli/Commands/GitHubCommand.php index 764e100..7df4628 100644 --- a/inc/Cli/Commands/GitHubCommand.php +++ b/inc/Cli/Commands/GitHubCommand.php @@ -83,7 +83,7 @@ public function issues( array $args, array $assoc_args ): void { $format = $assoc_args['format'] ?? 'table'; if ( 'json' === $format ) { - WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); + WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); return; } @@ -160,7 +160,7 @@ public function view( array $args, array $assoc_args ): void { $format = $assoc_args['format'] ?? 'table'; if ( 'json' === $format ) { - WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); + WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); return; } @@ -341,7 +341,7 @@ public function pulls( array $args, array $assoc_args ): void { $format = $assoc_args['format'] ?? 'table'; if ( 'json' === $format ) { - WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); + WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); return; } @@ -554,7 +554,7 @@ public function repos( array $args, array $assoc_args ): void { $format = $assoc_args['format'] ?? 'table'; if ( 'json' === $format ) { - WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); + WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) ); return; } @@ -812,13 +812,13 @@ public function review_flow( array $args, array $assoc_args ): void { return; } - WP_CLI::line( wp_json_encode( $installed, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) ); + WP_CLI::line( (string) wp_json_encode( $installed, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) ); return; } $definition = PrReviewFlowScaffold::build( $options ); - WP_CLI::line( wp_json_encode( $definition, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) ); + WP_CLI::line( (string) wp_json_encode( $definition, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) ); } // ------------------------------------------------------------------------- @@ -875,7 +875,7 @@ private function render_pull_mutation_result( array|\WP_Error $result, array $as } if ( 'json' === ( $assoc_args['format'] ?? 'table' ) ) { - WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) ); + WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) ); return; } diff --git a/inc/Tools/GitHubTools.php b/inc/Tools/GitHubTools.php index a27f42f..2e5d6c3 100644 --- a/inc/Tools/GitHubTools.php +++ b/inc/Tools/GitHubTools.php @@ -113,6 +113,21 @@ public function handle_tool_call( array $parameters, array $tool_def = array() ) return $this->buildErrorResponse( "Unknown method: {$method}", 'github_tools' ); } + /** + * Build a standard GitHub tool error response. + * + * @param string $message Error message. + * @param string $tool_name Tool name. + * @return array + */ + protected function buildErrorResponse( string $message, string $tool_name ): array { + return array( + 'success' => false, + 'error' => $message, + 'tool_name' => $tool_name, + ); + } + /** * Check if GitHub tools are properly configured. *