From 67c9501f7a8c80cb7e9bfc715a0270ed2cadb067 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Sat, 16 May 2026 12:45:41 -0400 Subject: [PATCH] fix: reuse existing pull requests --- inc/Abilities/GitHubAbilities.php | 93 +++++++++++++++++++++++++ tests/smoke-github-create-abilities.php | 44 ++++++++++-- 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/inc/Abilities/GitHubAbilities.php b/inc/Abilities/GitHubAbilities.php index bce7b4d..27b1f2f 100644 --- a/inc/Abilities/GitHubAbilities.php +++ b/inc/Abilities/GitHubAbilities.php @@ -1566,6 +1566,55 @@ public static function createPullRequest( array $input ): array|\WP_Error { } $body['base'] = $base; + $existing_pull = self::findExistingOpenPullRequest( $repo, $head, $base, $pat ); + if ( is_wp_error( $existing_pull ) ) { + return $existing_pull; + } + + if ( null !== $existing_pull ) { + $pull = self::normalizePull( $existing_pull ); + $labels = self::mergeProvenanceLabels( isset( $input['labels'] ) && is_array( $input['labels'] ) ? $input['labels'] : array() ); + $labeling = null; + + if ( ! empty( $labels ) && ! empty( $pull['number'] ) ) { + $label_response = self::applyLabelsToNumber( $repo, (int) $pull['number'], $labels, $pat ); + if ( is_wp_error( $label_response ) ) { + $labeling = array( + 'success' => false, + 'labels' => $labels, + 'error_code' => $label_response->get_error_code(), + 'error' => $label_response->get_error_message(), + 'status' => is_array( $label_response->get_error_data() ) ? ( $label_response->get_error_data()['status'] ?? null ) : null, + ); + } else { + $labeling = array( + 'success' => true, + 'labels' => $labels, + 'applied_labels' => $label_response['applied_labels'] ?? array(), + ); + } + } + + $result = array( + 'success' => true, + 'kind' => 'pull_request', + 'repo' => $repo, + 'number' => $pull['number'] ?? 0, + 'pull_request' => $pull, + 'pull_number' => $pull['number'] ?? 0, + 'url' => $pull['html_url'] ?? '', + 'html_url' => $pull['html_url'] ?? '', + 'reused' => true, + 'message' => sprintf( 'Pull request #%d already exists in %s.', $pull['number'] ?? 0, $repo ), + ); + + if ( null !== $labeling ) { + $result['labeling'] = $labeling; + } + + return $result; + } + $body_text = isset( $input['body'] ) ? (string) $input['body'] : ''; $artifacts = self::preparePullRequestRunArtifacts( $input, $repo, $head, $body_text ); if ( is_wp_error( $artifacts ) ) { @@ -1637,6 +1686,50 @@ public static function createPullRequest( array $input ): array|\WP_Error { return $result; } + /** + * Find an open pull request for the exact head/base pair before creating one. + * + * @param string $repo Repository owner/name. + * @param string $head Pull request head branch or owner:branch. + * @param string $base Pull request base branch. + * @param string $pat GitHub token. + * @return array|null|\WP_Error + */ + private static function findExistingOpenPullRequest( string $repo, string $head, string $base, string $pat ): array|null|\WP_Error { + $repo_owner = strtok( $repo, '/' ); + $head_query = str_contains( $head, ':' ) ? $head : sprintf( '%s:%s', $repo_owner, $head ); + $head_ref = str_contains( $head, ':' ) ? substr( $head, (int) strpos( $head, ':' ) + 1 ) : $head; + + $url = sprintf( '%s/repos/%s/pulls', self::API_BASE, $repo ); + $response = self::apiGet( + $url, + array( + 'state' => 'open', + 'head' => $head_query, + 'base' => $base, + 'per_page' => 10, + ), + $pat + ); + + if ( is_wp_error( $response ) ) { + return $response; + } + + foreach ( $response['data'] ?? array() as $pull ) { + if ( ! is_array( $pull ) ) { + continue; + } + + $normalized = self::normalizePull( $pull ); + if ( $head_ref === $normalized['head_ref'] && $base === $normalized['base_ref'] ) { + return $pull; + } + } + + return null; + } + /** * Persist and render Data Machine run artifacts for direct PR creation. * diff --git a/tests/smoke-github-create-abilities.php b/tests/smoke-github-create-abilities.php index c4debf8..baeb209 100644 --- a/tests/smoke-github-create-abilities.php +++ b/tests/smoke-github-create-abilities.php @@ -338,6 +338,7 @@ function wp_remote_retrieve_body( $response ): string { // ---- createPullRequest: success path with explicit base, draft default false $reset_http(); + $queue_response( 200, array() ); $queue_response( 201, array( 'number' => 88, 'title' => 'Open PR', @@ -362,17 +363,41 @@ function wp_remote_retrieve_body( $response ): string { $assert( 'createPullRequest exposes pull_request key', is_array( $result ) && isset( $result['pull_request']['number'] ) && 88 === $result['pull_request']['number'] ); $assert( 'createPullRequest normalized head ref', is_array( $result ) && 'feature/x' === ( $result['pull_request']['head'] ?? '' ) ); - $call = $GLOBALS['dmc_http_calls'][0] ?? array(); + $call = $GLOBALS['dmc_http_calls'][1] ?? array(); $body = is_string( $call['args']['body'] ?? null ) ? json_decode( $call['args']['body'], true ) : null; + $preflight_call = $GLOBALS['dmc_http_calls'][0] ?? array(); + $assert( 'createPullRequest preflights open PRs for same head/base', is_string( $preflight_call['url'] ?? null ) && str_contains( $preflight_call['url'], '/repos/owner/repo/pulls' ) && str_contains( $preflight_call['url'], 'head=owner%3Afeature%2Fx' ) && str_contains( $preflight_call['url'], 'base=main' ) ); $assert( 'createPullRequest posts to /repos/owner/repo/pulls', is_string( $call['url'] ?? null ) && str_ends_with( $call['url'], '/repos/owner/repo/pulls' ) ); $assert( 'createPullRequest forwards head and base', is_array( $body ) && 'feature/x' === ( $body['head'] ?? '' ) && 'main' === ( $body['base'] ?? '' ) ); $assert( 'createPullRequest defaults maintainer_can_modify=true', is_array( $body ) && true === ( $body['maintainer_can_modify'] ?? null ) ); $assert( 'createPullRequest does not set draft when omitted', is_array( $body ) && ! array_key_exists( 'draft', $body ) ); - $assert( 'createPullRequest does not call labels endpoint without labels or agent context', 1 === count( $GLOBALS['dmc_http_calls'] ) ); + $assert( 'createPullRequest does not call labels endpoint without labels or agent context', 2 === count( $GLOBALS['dmc_http_calls'] ) ); + + // ---- createPullRequest: existing open PR is reused instead of POSTing a duplicate + $reset_http(); + $queue_response( 200, array( + array( + 'number' => 188, + 'title' => 'Existing PR', + 'state' => 'open', + 'html_url' => 'https://github.com/owner/repo/pull/188', + 'head' => array( 'ref' => 'feature/x', 'sha' => 'ccc' ), + 'base' => array( 'ref' => 'main', 'sha' => 'ddd' ), + ), + ) ); + $result = GitHubAbilities::createPullRequest( array( + 'repo' => 'owner/repo', + 'title' => 'Duplicate PR', + 'head' => 'feature/x', + 'base' => 'main', + ) ); + $assert( 'createPullRequest reuses existing open PR', is_array( $result ) && true === ( $result['reused'] ?? false ) && 188 === ( $result['number'] ?? 0 ) ); + $assert( 'createPullRequest skips POST when existing PR is reused', 1 === count( $GLOBALS['dmc_http_calls'] ) && 'GET' === ( $GLOBALS['dmc_http_calls'][0]['method'] ?? '' ) ); // ---- createPullRequest: agent context applies caller and provenance labels after creation $reset_http(); PermissionHelper::$acting_agent_slug = 'code-reviewer'; + $queue_response( 200, array() ); $queue_response( 201, array( 'number' => 91, 'html_url' => 'https://github.com/owner/repo/pull/91', @@ -391,7 +416,7 @@ function wp_remote_retrieve_body( $response ): string { 'base' => 'main', 'labels' => array( 'needs-review' ), ) ); - $label_call = $GLOBALS['dmc_http_calls'][1] ?? array(); + $label_call = $GLOBALS['dmc_http_calls'][2] ?? array(); $label_body = is_string( $label_call['args']['body'] ?? null ) ? json_decode( $label_call['args']['body'], true ) : null; $assert( 'createPullRequest labels PR through issues labels endpoint', is_string( $label_call['url'] ?? null ) && str_ends_with( $label_call['url'], '/repos/owner/repo/issues/91/labels' ) ); $assert( 'createPullRequest preserves caller label during post-create labeling', is_array( $label_body ) && in_array( 'needs-review', $label_body['labels'] ?? array(), true ) ); @@ -402,6 +427,7 @@ function wp_remote_retrieve_body( $response ): string { // ---- createPullRequest: run artifacts are committed and rendered on direct ability calls $reset_http(); + $queue_response( 200, array() ); $queue_response( 200, array( 'ref' => 'refs/heads/world-day/memory' ) ); $queue_response( 404, array( 'message' => 'Not Found' ) ); $queue_response( 201, array( @@ -441,9 +467,9 @@ function wp_remote_retrieve_body( $response ): string { 'daily_memory' => array( 'egress' => array( 'bundle-file', 'pr-body' ) ), ), ) ); - $artifact_put_call = $GLOBALS['dmc_http_calls'][2] ?? array(); + $artifact_put_call = $GLOBALS['dmc_http_calls'][3] ?? array(); $artifact_put_body = is_string( $artifact_put_call['args']['body'] ?? null ) ? json_decode( $artifact_put_call['args']['body'], true ) : null; - $pr_call = $GLOBALS['dmc_http_calls'][3] ?? array(); + $pr_call = $GLOBALS['dmc_http_calls'][4] ?? array(); $pr_body = is_string( $pr_call['args']['body'] ?? null ) ? json_decode( $pr_call['args']['body'], true ) : null; $assert( 'createPullRequest direct ability commits bundle-file artifact before PR creation', 'PUT' === ( $artifact_put_call['method'] ?? '' ) && str_contains( (string) ( $artifact_put_call['url'] ?? '' ), '/contents/bundles/world-creator/memory/agent/daily/2026/05/09.md' ) ); $assert( 'createPullRequest direct ability writes artifact to head branch', is_array( $artifact_put_body ) && 'world-day/memory' === ( $artifact_put_body['branch'] ?? '' ) ); @@ -454,6 +480,7 @@ function wp_remote_retrieve_body( $response ): string { // ---- createPullRequest: labeling failure does not mask PR creation success $reset_http(); PermissionHelper::$acting_agent_slug = 'code-reviewer'; + $queue_response( 200, array() ); $queue_response( 201, array( 'number' => 92, 'html_url' => 'https://github.com/owner/repo/pull/92', @@ -516,6 +543,7 @@ function wp_remote_retrieve_body( $response ): string { // ---- createPullRequest: explicit draft and maintainer_can_modify=false $reset_http(); + $queue_response( 200, array() ); $queue_response( 201, array( 'number' => 89, 'head' => array( 'ref' => 'feat/y' ), @@ -529,7 +557,7 @@ function wp_remote_retrieve_body( $response ): string { 'draft' => true, 'maintainer_can_modify' => false, ) ); - $call = $GLOBALS['dmc_http_calls'][0] ?? array(); + $call = $GLOBALS['dmc_http_calls'][1] ?? array(); $body = is_string( $call['args']['body'] ?? null ) ? json_decode( $call['args']['body'], true ) : null; $assert( 'createPullRequest forwards draft=true', is_array( $body ) && true === ( $body['draft'] ?? null ) ); $assert( 'createPullRequest forwards maintainer_can_modify=false', is_array( $body ) && false === ( $body['maintainer_can_modify'] ?? null ) ); @@ -537,6 +565,7 @@ function wp_remote_retrieve_body( $response ): string { // ---- createPullRequest: missing base falls back to default branch via GET /repos $reset_http(); $queue_response( 200, array( 'default_branch' => 'trunk' ) ); + $queue_response( 200, array() ); $queue_response( 201, array( 'number' => 90, 'head' => array( 'ref' => 'feat/z' ), @@ -549,12 +578,13 @@ function wp_remote_retrieve_body( $response ): string { ) ); $assert( 'createPullRequest fallback resolves default branch', is_array( $result ) && true === ( $result['success'] ?? false ) ); $assert( 'createPullRequest fallback issued GET /repos/owner/repo first', is_string( $GLOBALS['dmc_http_calls'][0]['url'] ?? null ) && str_contains( $GLOBALS['dmc_http_calls'][0]['url'], '/repos/owner/repo' ) && 'GET' === ( $GLOBALS['dmc_http_calls'][0]['method'] ?? '' ) ); - $pr_call = $GLOBALS['dmc_http_calls'][1] ?? array(); + $pr_call = $GLOBALS['dmc_http_calls'][2] ?? array(); $pr_body = is_string( $pr_call['args']['body'] ?? null ) ? json_decode( $pr_call['args']['body'], true ) : null; $assert( 'createPullRequest sends fallback default base', is_array( $pr_body ) && 'trunk' === ( $pr_body['base'] ?? '' ) ); // ---- createPullRequest: GitHub validation error surfaces as WP_Error $reset_http(); + $queue_response( 200, array() ); $queue_response( 422, array( 'message' => 'A pull request already exists for owner:feat/x.' ) ); $result = GitHubAbilities::createPullRequest( array( 'repo' => 'owner/repo',