Skip to content

Commit a1a07bd

Browse files
chubes4chubes4
authored
Preserve local PR cleanup results (#440)
* fix: preserve local PR cleanup results * fix: satisfy github cleanup lint
1 parent 9aa81c3 commit a1a07bd

4 files changed

Lines changed: 97 additions & 9 deletions

File tree

‎inc/Abilities/GitHubAbilities.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,10 @@ private function registerAbilities(): void {
578578
'type' => 'boolean',
579579
'description' => 'Preview the cleanup decision without deleting the branch.',
580580
),
581+
'local_only' => array(
582+
'type' => 'boolean',
583+
'description' => 'Finalize and remove the matching local DMC worktree without deleting the remote branch.',
584+
),
581585
),
582586
),
583587
'output_schema' => array(
@@ -2376,7 +2380,7 @@ public static function mergePullRequest( array $input, ?callable $api_get = null
23762380
* from a linked worktree where the base branch is already checked out in the
23772381
* primary checkout.
23782382
*
2379-
* @param array $input Required: repo, pull_number. Optional: dry_run.
2383+
* @param array $input Required: repo, pull_number. Optional: dry_run, local_only.
23802384
* @param callable|null $api_get Optional test seam: fn(string $url, array $query, string $pat): array|WP_Error.
23812385
* @param callable|null $api_request Optional test seam: fn(string $method, string $url, ?array $body, string $pat): array|WP_Error.
23822386
* @return array|\WP_Error Success payload or error.
@@ -2385,6 +2389,7 @@ public static function cleanupPullRequest( array $input, ?callable $api_get = nu
23852389
$repo = sanitize_text_field( $input['repo'] ?? '' );
23862390
$pull_number = (int) ( $input['pull_number'] ?? 0 );
23872391
$dry_run = ! empty( $input['dry_run'] );
2392+
$local_only = ! empty( $input['local_only'] );
23882393

23892394
if ( empty( $repo ) || $pull_number <= 0 ) {
23902395
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
24342439
'branch_deleted' => false,
24352440
'branch_already_deleted' => false,
24362441
'dry_run' => $dry_run,
2442+
'local_only' => $local_only,
2443+
'partial_success' => false,
24372444
'pull_request_url' => (string) ( $pull['html_url'] ?? '' ),
24382445
);
24392446

@@ -2453,12 +2460,28 @@ public static function cleanupPullRequest( array $input, ?callable $api_get = nu
24532460
$result['local_worktree_cleanup'] = $local_cleanup;
24542461
}
24552462

2463+
if ( $local_only ) {
2464+
$result['message'] = sprintf( 'Cleaned up local DMC worktree for %s without deleting remote branch %s.', $repo, $head_branch );
2465+
return $result;
2466+
}
2467+
24562468
$encoded_head_branch = implode( '/', array_map( 'rawurlencode', explode( '/', $head_branch ) ) );
24572469
$delete_url = sprintf( '%s/repos/%s/git/refs/heads/%s', self::API_BASE, $repo, $encoded_head_branch );
24582470
$deleted = $api_request( 'DELETE', $delete_url, null, $pat );
24592471
if ( is_wp_error( $deleted ) ) {
24602472
$status = is_array( $deleted->get_error_data() ) ? (int) ( $deleted->get_error_data()['status'] ?? 0 ) : 0;
24612473
if ( 404 !== $status ) {
2474+
if ( true === ( $result['local_worktree_cleanup']['success'] ?? false ) ) {
2475+
$result['partial_success'] = true;
2476+
$result['branch_delete_error'] = array(
2477+
'code' => $deleted->get_error_code(),
2478+
'message' => $deleted->get_error_message(),
2479+
'status' => $status,
2480+
);
2481+
$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() );
2482+
return $result;
2483+
}
2484+
24622485
return $deleted;
24632486
}
24642487

‎inc/Cli/Commands/GitHubCommand.php‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function issues( array $args, array $assoc_args ): void {
8383
$format = $assoc_args['format'] ?? 'table';
8484

8585
if ( 'json' === $format ) {
86-
WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
86+
WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
8787
return;
8888
}
8989

@@ -160,7 +160,7 @@ public function view( array $args, array $assoc_args ): void {
160160
$format = $assoc_args['format'] ?? 'table';
161161

162162
if ( 'json' === $format ) {
163-
WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
163+
WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
164164
return;
165165
}
166166

@@ -341,7 +341,7 @@ public function pulls( array $args, array $assoc_args ): void {
341341
$format = $assoc_args['format'] ?? 'table';
342342

343343
if ( 'json' === $format ) {
344-
WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
344+
WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
345345
return;
346346
}
347347

@@ -458,6 +458,9 @@ public function merge_pull( array $args, array $assoc_args ): void {
458458
* [--dry-run]
459459
* : Preview the cleanup decision without deleting the branch.
460460
*
461+
* [--local-only]
462+
* : Finalize and remove the matching local DMC worktree without deleting the remote branch.
463+
*
461464
* [--format=<format>]
462465
* : Output format.
463466
* ---
@@ -487,6 +490,7 @@ public function cleanup_pull( array $args, array $assoc_args ): void {
487490
'repo' => $assoc_args['repo'] ?? '',
488491
'pull_number' => $pull_number,
489492
'dry_run' => ! empty( $assoc_args['dry-run'] ),
493+
'local_only' => ! empty( $assoc_args['local-only'] ),
490494
) );
491495

492496
$result = GitHubAbilities::cleanupPullRequest( $input );
@@ -550,7 +554,7 @@ public function repos( array $args, array $assoc_args ): void {
550554
$format = $assoc_args['format'] ?? 'table';
551555

552556
if ( 'json' === $format ) {
553-
WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
557+
WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT ) );
554558
return;
555559
}
556560

@@ -808,13 +812,13 @@ public function review_flow( array $args, array $assoc_args ): void {
808812
return;
809813
}
810814

811-
WP_CLI::line( wp_json_encode( $installed, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
815+
WP_CLI::line( (string) wp_json_encode( $installed, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
812816
return;
813817
}
814818

815819
$definition = PrReviewFlowScaffold::build( $options );
816820

817-
WP_CLI::line( wp_json_encode( $definition, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
821+
WP_CLI::line( (string) wp_json_encode( $definition, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
818822
}
819823

820824
// -------------------------------------------------------------------------
@@ -871,7 +875,7 @@ private function render_pull_mutation_result( array|\WP_Error $result, array $as
871875
}
872876

873877
if ( 'json' === ( $assoc_args['format'] ?? 'table' ) ) {
874-
WP_CLI::line( \wp_json_encode( $result, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
878+
WP_CLI::line( (string) \wp_json_encode( $result, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) );
875879
return;
876880
}
877881

‎inc/Tools/GitHubTools.php‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ public function handle_tool_call( array $parameters, array $tool_def = array() )
113113
return $this->buildErrorResponse( "Unknown method: {$method}", 'github_tools' );
114114
}
115115

116+
/**
117+
* Build a standard GitHub tool error response.
118+
*
119+
* @param string $message Error message.
120+
* @param string $tool_name Tool name.
121+
* @return array<string,mixed>
122+
*/
123+
protected function buildErrorResponse( string $message, string $tool_name ): array {
124+
return array(
125+
'success' => false,
126+
'error' => $message,
127+
'tool_name' => $tool_name,
128+
);
129+
}
130+
116131
/**
117132
* Check if GitHub tools are properly configured.
118133
*
@@ -663,7 +678,7 @@ public function getCleanupPullRequestDefinition(): array {
663678
return array(
664679
'class' => __CLASS__,
665680
'method' => 'handleCleanupPullRequest',
666-
'description' => 'Delete a merged pull request head branch through the GitHub API without checking out or switching local branches. Supports dry_run previews.',
681+
'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.',
667682
'parameters' => array(
668683
'type' => 'object',
669684
'properties' => array(
@@ -679,6 +694,10 @@ public function getCleanupPullRequestDefinition(): array {
679694
'type' => 'boolean',
680695
'description' => 'Preview the cleanup decision without deleting the branch.',
681696
),
697+
'local_only' => array(
698+
'type' => 'boolean',
699+
'description' => 'Finalize and remove the matching local DMC worktree without deleting the remote branch.',
700+
),
682701
),
683702
'required' => array( 'repo', 'pull_number' ),
684703
),

‎tests/smoke-github-merge-pull-request.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ function wp_get_ability( string $name ): ?DMC_Test_Ability {
253253
$cleanup_ability = $GLOBALS['dmc_registered_abilities']['datamachine/cleanup-github-pull-request'] ?? null;
254254
$assert( 'cleanup ability is registered', null !== $cleanup_ability );
255255
$assert( 'cleanup ability uses cleanupPullRequest execute_callback', array( GitHubAbilities::class, 'cleanupPullRequest' ) === ( $cleanup_ability['execute_callback'] ?? null ) );
256+
$assert( 'cleanup ability exposes local_only', isset( $cleanup_ability['input_schema']['properties']['local_only'] ) );
256257

257258
$permission = $ability['permission_callback'] ?? null;
258259
PermissionHelper::$can_manage_result = false;
@@ -377,6 +378,46 @@ static function ( string $method, string $url, ?array $body, string $pat ) use (
377378
$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() ) );
378379
$assert( 'cleanupPullRequest returns local worktree cleanup evidence', true === ( $result['local_worktree_cleanup']['found'] ?? false ) );
379380

381+
$calls = array();
382+
Workspace::$cleanup_calls = array();
383+
$result = GitHubAbilities::cleanupPullRequest(
384+
array(
385+
'repo' => 'owner/repo',
386+
'pull_number' => 42,
387+
'local_only' => true,
388+
),
389+
static function ( string $url, array $query, string $pat ) use ( &$calls, $merged_pull ): array {
390+
$calls[] = array( 'method' => 'GET', 'url' => $url, 'query' => $query, 'pat' => $pat );
391+
return $merged_pull();
392+
},
393+
static function ( string $method, string $url, ?array $body, string $pat ) use ( &$calls ): array {
394+
$calls[] = compact( 'method', 'url', 'body', 'pat' );
395+
return array( 'success' => true, 'data' => null );
396+
}
397+
);
398+
$assert( 'cleanupPullRequest local_only succeeds', is_array( $result ) && true === ( $result['local_only'] ?? false ) );
399+
$assert( 'cleanupPullRequest local_only skips remote branch delete', 1 === count( $calls ) );
400+
$assert( 'cleanupPullRequest local_only returns local cleanup evidence', true === ( $result['local_worktree_cleanup']['found'] ?? false ) );
401+
402+
$calls = array();
403+
Workspace::$cleanup_calls = array();
404+
$result = GitHubAbilities::cleanupPullRequest(
405+
array(
406+
'repo' => 'owner/repo',
407+
'pull_number' => 42,
408+
),
409+
static function ( string $url, array $query, string $pat ) use ( &$calls, $merged_pull ): array {
410+
$calls[] = array( 'method' => 'GET', 'url' => $url, 'query' => $query, 'pat' => $pat );
411+
return $merged_pull();
412+
},
413+
static function ( string $method, string $url, ?array $body, string $pat ) use ( &$calls ): WP_Error {
414+
$calls[] = compact( 'method', 'url', 'body', 'pat' );
415+
return new WP_Error( 'github_api_error', 'GitHub API error (403): Resource not accessible by integration', array( 'status' => 403 ) );
416+
}
417+
);
418+
$assert( 'cleanupPullRequest preserves local cleanup on remote delete failure', is_array( $result ) && true === ( $result['partial_success'] ?? false ) );
419+
$assert( 'cleanupPullRequest reports remote delete error without losing local evidence', 403 === ( $result['branch_delete_error']['status'] ?? 0 ) && true === ( $result['local_worktree_cleanup']['found'] ?? false ) );
420+
380421
$result = GitHubAbilities::cleanupPullRequest(
381422
array(
382423
'repo' => 'owner/repo',
@@ -418,6 +459,7 @@ static function ( string $method, string $url, ?array $body, string $pat ) use (
418459
$tool_call = $GLOBALS['dmc_tool_ability_calls'][0] ?? array();
419460
$assert( 'merge tool calls merge ability', 'datamachine/merge-github-pull-request' === ( $tool_call['name'] ?? '' ) );
420461
$cleanup_definition = $tools->getCleanupPullRequestDefinition();
462+
$assert( 'cleanup tool exposes local_only', isset( $cleanup_definition['parameters']['properties']['local_only'] ) );
421463
$cleanup_result = $tools->handle_tool_call(
422464
array(
423465
'repo' => 'owner/repo',

0 commit comments

Comments
 (0)