Skip to content

Commit 7259546

Browse files
chubes4chubes4
authored
Merge pull request #454 from Extra-Chill/fix/gitsync-proposal-sibling-refs
fix: avoid nested gitsync proposal refs
2 parents b0540e6 + 3669295 commit 7259546

4 files changed

Lines changed: 66 additions & 11 deletions

File tree

‎inc/Abilities/GitSyncAbilities.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,15 @@ private function registerAbilities(): void {
218218
'datamachine/gitsync-submit',
219219
array(
220220
'label' => 'Submit GitSync Binding as Pull Request',
221-
'description' => 'Upload changed local files to the sticky proposal branch (gitsync/<slug>) by default, or to a keyed proposal branch (gitsync/<slug>/<proposal-slug>) when proposal is provided, and open or update a PR against the pinned branch.',
221+
'description' => 'Upload changed local files to the sticky proposal branch (gitsync/<slug>) by default, or to a keyed proposal branch (gitsync/<slug>-<proposal-slug>) when proposal is provided, and open or update a PR against the pinned branch.',
222222
'category' => 'datamachine-code-gitsync',
223223
'input_schema' => array(
224224
'type' => 'object',
225225
'required' => array( 'slug', 'message' ),
226226
'properties' => array(
227-
'slug' => array( 'type' => 'string' ),
228-
'message' => array( 'type' => 'string' ),
229-
'paths' => array(
227+
'slug' => array( 'type' => 'string' ),
228+
'message' => array( 'type' => 'string' ),
229+
'paths' => array(
230230
'type' => 'array',
231231
'items' => array( 'type' => 'string' ),
232232
'description' => 'Optional explicit list of relative paths. If omitted, every file with a SHA mismatch against upstream (filtered by allowed_paths) is submitted.',
@@ -235,7 +235,7 @@ private function registerAbilities(): void {
235235
'body' => array( 'type' => 'string' ),
236236
'proposal' => array(
237237
'type' => 'string',
238-
'description' => 'Optional proposal key. Omit to reuse gitsync/<slug>; pass a key to use gitsync/<slug>/<proposal-slug>.',
238+
'description' => 'Optional proposal key. Omit to reuse gitsync/<slug>; pass a key to use gitsync/<slug>-<proposal-slug>.',
239239
),
240240
),
241241
),

‎inc/Cli/Commands/GitSyncCommand.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ public function status( array $args, array $assoc_args ): void {
340340
*
341341
* [--proposal=<proposal>]
342342
* : Optional proposal key. Omit to reuse the sticky branch gitsync/<slug>;
343-
* pass a key to use an isolated branch gitsync/<slug>/<proposal-slug>.
343+
* pass a key to use an isolated branch gitsync/<slug>-<proposal-slug>.
344344
*
345345
* [--title=<title>]
346346
* : PR title. Defaults to --message.

‎inc/GitSync/GitSyncProposer.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function __construct( GitSyncRegistry $registry ) {
6060
* @type string $title PR title. Defaults to $message.
6161
* @type string $body PR body. Defaults to an auto-summary.
6262
* @type string $proposal Optional proposal key. When present,
63-
* submits to gitsync/<slug>/<proposal-slug>.
63+
* submits to gitsync/<slug>-<proposal-slug>.
6464
* }
6565
* @return array<string, mixed>|\WP_Error
6666
*/
@@ -93,13 +93,13 @@ public function submit( GitSyncBinding $binding, array $args ): array|\WP_Error
9393
);
9494
}
9595

96-
$proposal = $this->normalizeProposalKey( (string) ( $args['proposal'] ?? '' ));
96+
$proposal = $this->normalizeProposalKey( (string) ( $args['proposal'] ?? '' ));
9797
if ( is_wp_error($proposal) ) {
9898
return $proposal;
9999
}
100100
$feature_branch = self::BRANCH_PREFIX . $binding->slug;
101101
if ( null !== $proposal ) {
102-
$feature_branch .= '/' . $proposal;
102+
$feature_branch .= '-' . $proposal;
103103
}
104104

105105
// Ensure the feature branch exists and points at the current

‎tests/smoke-gitsync.php‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,26 @@ public static function apiRequest( string $method, string $url, array $body, str
458458
// =========================================================================
459459
echo "\nSubmit (keyed proposal branch)\n";
460460
$GLOBALS['__dmc_http_mock']['GET https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/git/ref/heads/gitsync%2Fwiki%2Fscript-modules'] = array(
461+
'response' => array( 'code' => 500 ),
462+
'body' => json_encode(array( 'message' => 'Nested proposal ref must not be used' )),
463+
);
464+
$GLOBALS['__dmc_http_mock']['GET https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/git/ref/heads/gitsync%2Fwiki-script-modules'] = array(
461465
'response' => array( 'code' => 404 ),
462466
'body' => json_encode(array( 'message' => 'Not Found' )),
463467
);
468+
$GLOBALS['__dmc_http_mock']['POST https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/git/refs'] = static function ( string $url, array $args ): array {
469+
$body = json_decode((string) ( $args['body'] ?? '' ), true);
470+
if ( ! is_array($body) || 'refs/heads/gitsync/wiki-script-modules' !== ( $body['ref'] ?? null ) ) {
471+
return array(
472+
'response' => array( 'code' => 500 ),
473+
'body' => json_encode(array( 'message' => 'Unexpected proposal ref' )),
474+
);
475+
}
476+
return array(
477+
'response' => array( 'code' => 201 ),
478+
'body' => json_encode(array( 'ref' => 'refs/heads/gitsync/wiki-script-modules', 'object' => array( 'sha' => 'base-sha-1' ) )),
479+
);
480+
};
464481
$GLOBALS['__dmc_http_mock']['GET https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/pulls'] = array(
465482
'response' => array( 'code' => 200 ),
466483
'body' => '[]',
@@ -474,7 +491,7 @@ public static function apiRequest( string $method, string $url, array $body, str
474491
$keyed = $gs->submit('wiki', array( 'message' => 'script modules update', 'proposal' => 'Script Modules' ));
475492
$assert(! is_wp_error($keyed), 'keyed submit succeeded — ' . ( is_wp_error($keyed) ? $keyed->get_error_message() : '' ));
476493
$assert('script-modules' === ( $keyed['proposal'] ?? null ), 'proposal key normalized to script-modules');
477-
$assert('gitsync/wiki/script-modules' === ( $keyed['branch'] ?? null ), 'keyed feature branch includes proposal slug');
494+
$assert('gitsync/wiki-script-modules' === ( $keyed['branch'] ?? null ), 'keyed feature branch uses sibling proposal slug');
478495
$assert(8 === ( $keyed['pr']['number'] ?? null ), 'keyed proposal opened separate PR');
479496
$keyed_requests = array_slice($GLOBALS['__dmc_http_capture'], $capture_before_keyed);
480497
$keyed_pr_body = null;
@@ -483,7 +500,45 @@ public static function apiRequest( string $method, string $url, array $body, str
483500
$keyed_pr_body = json_decode((string) $request['body'], true);
484501
}
485502
}
486-
$assert('gitsync/wiki/script-modules' === ( $keyed_pr_body['head'] ?? null ), 'keyed PR uses keyed branch head');
503+
$assert('gitsync/wiki-script-modules' === ( $keyed_pr_body['head'] ?? null ), 'keyed PR uses keyed branch head');
504+
$used_nested_keyed_ref = false;
505+
foreach ( $keyed_requests as $request ) {
506+
$used_nested_keyed_ref = $used_nested_keyed_ref || str_contains((string) $request['url'], 'gitsync%2Fwiki%2Fscript-modules');
507+
}
508+
$assert(! $used_nested_keyed_ref, 'keyed submit avoids nested proposal ref URLs');
509+
510+
$GLOBALS['__dmc_http_mock']['GET https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/git/ref/heads/gitsync%2Fwiki-script-modules'] = array(
511+
'response' => array( 'code' => 200 ),
512+
'body' => json_encode(array( 'object' => array( 'sha' => 'keyed-feature-old-sha' ) )),
513+
);
514+
$GLOBALS['__dmc_http_mock']['PATCH https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/git/refs/heads/gitsync%2Fwiki-script-modules'] = array(
515+
'response' => array( 'code' => 200 ),
516+
'body' => json_encode(array( 'object' => array( 'sha' => 'base-sha-1' ) )),
517+
);
518+
$GLOBALS['__dmc_http_mock']['GET https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/pulls'] = array(
519+
'response' => array( 'code' => 200 ),
520+
'body' => json_encode(
521+
array(
522+
array( 'number' => 8, 'html_url' => 'https://github.com/a/b/pull/8', 'state' => 'open' ),
523+
)
524+
),
525+
);
526+
$GLOBALS['__dmc_http_mock']['PATCH https://api.github.com/repos/Automattic/a8c-wiki-woocommerce/pulls/8'] = array(
527+
'response' => array( 'code' => 200 ),
528+
'body' => json_encode(array( 'number' => 8, 'html_url' => 'https://github.com/a/b/pull/8', 'state' => 'open' )),
529+
);
530+
file_put_contents(ABSPATH . 'content/wiki/articles/a.md', "article a script modules proposal v2\n");
531+
$capture_before_keyed_reuse = count($GLOBALS['__dmc_http_capture']);
532+
$keyed_reuse = $gs->submit('wiki', array( 'message' => 'script modules follow-up', 'proposal' => 'Script Modules' ));
533+
$assert(! is_wp_error($keyed_reuse), 'keyed submit reuses sibling branch — ' . ( is_wp_error($keyed_reuse) ? $keyed_reuse->get_error_message() : '' ));
534+
$assert('gitsync/wiki-script-modules' === ( $keyed_reuse['branch'] ?? null ), 'keyed branch reuse keeps sibling branch shape');
535+
$assert('updated' === ( $keyed_reuse['pr']['action'] ?? null ), 'keyed proposal PR reported as updated');
536+
$keyed_reuse_requests = array_slice($GLOBALS['__dmc_http_capture'], $capture_before_keyed_reuse);
537+
$used_nested_keyed_ref = false;
538+
foreach ( $keyed_reuse_requests as $request ) {
539+
$used_nested_keyed_ref = $used_nested_keyed_ref || str_contains((string) $request['url'], 'gitsync%2Fwiki%2Fscript-modules');
540+
}
541+
$assert(! $used_nested_keyed_ref, 'keyed branch reuse avoids nested proposal ref URLs');
487542

488543
// =========================================================================
489544
// 10. Submit with nothing changed → nothing_to_submit.

0 commit comments

Comments
 (0)