From ded58f9dc5538e56d53e9b7e922df9a0419d3564 Mon Sep 17 00:00:00 2001 From: Nyakku Shigure Date: Fri, 2 Oct 2026 12:38:31 +0800 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat:=20merge=20PRs=20and=20native?= =?UTF-8?q?=20stacks=20via=20async=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Codex --- README.md | 26 ++- skills/github-conversation/SKILL.md | 15 +- src/gh_llm/commands/pr.py | 2 + src/gh_llm/commands/pr_merge.py | 158 ++++++++++++++++++ src/gh_llm/github_api.py | 86 ++++++++++ src/gh_llm/merge_render.py | 38 +++++ src/gh_llm/models.py | 13 ++ src/gh_llm/render.py | 36 +--- src/gh_llm/stack_render.py | 7 +- tests/test_cli.py | 15 +- tests/test_pr_merge.py | 248 ++++++++++++++++++++++++++++ tests/test_stacks.py | 6 +- 12 files changed, 604 insertions(+), 46 deletions(-) create mode 100644 src/gh_llm/commands/pr_merge.py create mode 100644 src/gh_llm/merge_render.py create mode 100644 tests/test_pr_merge.py diff --git a/README.md b/README.md index c0acc37..fed2d52 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,31 @@ Native stacks appear in the default PR overview with bottom-to-top ordering, the Checks are paginated and tied to the PR head. Workflow names and run links distinguish same-named jobs; the display keeps the latest reported run per workflow and event. For stacks, required checks come from the stack target's classic branch protection and rulesets, with missing contexts shown as `EXPECTED`. Required checks pinned to an app must match that app; optional failures alone do not block a stack merge. If branch rules are inaccessible, the output reports that required-check coverage is incomplete. `pr checks --all` also shows historical head checks for closed and merged PRs. -Stack mergeability describes the unmerged layers from the bottom through the selected PR and their blockers, then links to GitHub's native stack merge controls. GitHub makes the final readiness decision. Diff and `review-start` remain scoped to the selected PR and its direct base; cumulative stack diffs are not included. Base branch changes appear in the timeline. Historical stack join/leave events are not reconstructed: the API does not reliably expose their historical stack IDs. +Stack mergeability describes the unmerged layers from the bottom through the selected PR and their blockers, then offers `gh-llm pr merge` commands pinned to the inspected head. GitHub makes the final readiness decision. Diff and `review-start` remain scoped to the selected PR and its direct base; cumulative stack diffs are not included. Base branch changes appear in the timeline. Historical stack join/leave events are not reconstructed: the API does not reliably expose their historical stack IDs. + +### Merging PRs and Native Stacks + +`pr merge` uses GitHub's [asynchronous merge API](https://docs.github.com/en/rest/pulls/pulls?apiVersion=2026-03-10#merge-a-pull-request-asynchronously) for both ordinary and stacked PRs. It requires neither a browser nor the `gh-stack` extension. For a stack, selecting a PR also merges its open downstack PRs into the stack target; upper layers are excluded. The command prints this scope before submitting one request to GitHub. + +```bash +# Use the head SHA from `pr view` to guard against changes since inspection +gh-llm pr merge --repo yutto-dev/yutto --squash --head + +# Customize a direct merge's commit message +gh-llm pr merge --repo yutto-dev/yutto --merge \ + --subject 'Feature title' --body-file merge-message.md + +# Request the merge queue explicitly; its configuration determines the merge method +gh-llm pr merge --repo yutto-dev/yutto --merge-action merge_queue + +# Return immediately and retain the request UUID for a later status check +gh-llm pr merge --repo yutto-dev/yutto --squash --timeout 0 +gh-llm pr merge-status --pr --repo yutto-dev/yutto --timeout 60 +``` + +By default, `pr merge` follows the target branch's merge queue configuration and polls for up to 60 seconds. `--merge-action direct_merge` requests a direct merge; rules are enforced unless you explicitly pass `--bypass-rules` and have the necessary permission. `--merge`, `--squash`, and `--rebase` select the direct merge method. Omitted commit titles, messages, and methods use GitHub's defaults. `--body-file -` reads a message from standard input. Without `--head`, the freshly fetched PR head is sent as the expected SHA. + +Output distinguishes `pending`, `merged`, `enqueued`, and `failed`. Exit codes are **0** for merged or enqueued, **1** for failure, and **2** for a request still pending after the polling timeout. `enqueued` means only that the PR entered the merge queue; inspect the PR to confirm its eventual merge. The request UUID and a `merge-status` command are printed before polling, so interrupted waits can be resumed without another write. An existing pending request is followed with its original options, which are shown in the output. Merge writes are never automatically retried after a network error. GitHub retains request results for 24 hours after their last update; if a result has expired, inspect the PR's current state. ### PR Body Scaffold diff --git a/skills/github-conversation/SKILL.md b/skills/github-conversation/SKILL.md index 0d183b1..1ae5511 100644 --- a/skills/github-conversation/SKILL.md +++ b/skills/github-conversation/SKILL.md @@ -18,8 +18,8 @@ metadata: ## Tool split -1. Use `gh-llm` for reading context (timeline, collapsed items, review threads, checks) and structured review actions (reply, submit, resolve). -2. Use `gh` for simple write actions (comment, reactions, labels, assignees, reviewers, close/reopen, merge). +1. Use `gh-llm` for reading context (timeline, collapsed items, review threads, checks), structured review actions (reply, submit, resolve), and asynchronous PR or stack merges. +2. Use `gh` for simple write actions (comment, reactions, labels, assignees, reviewers, close/reopen). 3. If context is incomplete, do not reply yet; expand first. 4. Apply the current task's publication mode and authorized scope to every GitHub write, including reactions and resolving threads. In read-only or dry-run mode, keep proposed updates local. `viewerCanReact` indicates account capability, not operator authorization. @@ -133,6 +133,17 @@ gh-llm pr body-template \ Use this before `gh pr create` when you need to load a repo PR template, append required sections, and produce a ready-to-edit body file. +### Merge a PR or native stack + +After checking the current PR and receiving authorization to merge, use the inspected head SHA: + +```bash +gh-llm pr merge --repo --squash --head +gh-llm pr merge-status --pr --repo --timeout 60 +``` + +For a native stack, the merge includes the selected PR and its open downstack PRs. Verify that this entire scope is authorized. The async API honors the target branch's merge queue by default. `enqueued` is not merged; check the PR again for the eventual outcome. A pending request returns exit code 2 after the timeout and prints its UUID and resume command. Resume with `merge-status` instead of resubmitting. When an existing request is returned, its original options remain in effect. Do not retry a merge write blindly after a network error; inspect the PR first. + ### Read an issue ```bash diff --git a/src/gh_llm/commands/pr.py b/src/gh_llm/commands/pr.py index dc67894..d101a76 100644 --- a/src/gh_llm/commands/pr.py +++ b/src/gh_llm/commands/pr.py @@ -18,6 +18,7 @@ resolve_file_or_inline_text, resolve_subject, ) +from gh_llm.commands.pr_merge import register_merge_parsers from gh_llm.github_api import GitHubClient from gh_llm.invocation import display_command_with from gh_llm.models import CheckResults, PullRequestDiffPage @@ -74,6 +75,7 @@ class _ShowOptions: def register_pr_parser(subparsers: Any) -> None: pr_parser = subparsers.add_parser("pr", help="pull request reading, review, and actions") pr_subparsers = pr_parser.add_subparsers(dest="pr_command") + register_merge_parsers(pr_subparsers) view_parser = pr_subparsers.add_parser( "view", diff --git a/src/gh_llm/commands/pr_merge.py b/src/gh_llm/commands/pr_merge.py new file mode 100644 index 0000000..42732bd --- /dev/null +++ b/src/gh_llm/commands/pr_merge.py @@ -0,0 +1,158 @@ +import time +from typing import TYPE_CHECKING, Any +from uuid import UUID + +from gh_llm.commands.options import add_body_input_arguments, resolve_file_or_inline_text, resolve_subject +from gh_llm.github_api import GitHubClient +from gh_llm.invocation import display_command_with + +if TYPE_CHECKING: + from gh_llm.models import AsyncMergeResult, PullRequestRef + + +def register_merge_parsers(subparsers: Any) -> None: + parser = subparsers.add_parser("merge", help="merge a PR and its open downstack PRs through the async merge API") + parser.add_argument("pr", nargs="?", help="PR number/url/branch") + parser.add_argument("--repo", help="repository in OWNER/REPO format") + methods = parser.add_mutually_exclusive_group() + for method in ("merge", "squash", "rebase"): + methods.add_argument( + f"--{method}", + dest="merge_method", + action="store_const", + const=method, + help=f"use {method} for a direct merge", + ) + parser.add_argument("--head", help="expected PR head SHA; defaults to the freshly fetched head") + parser.add_argument("--subject", help="commit title for a direct merge; defaults to GitHub's title") + add_body_input_arguments( + parser, + required=False, + body_help="commit message for a direct merge", + file_help="read commit message from file (use `-` for standard input)", + ) + parser.add_argument( + "--merge-action", + choices=("default", "direct_merge", "merge_queue"), + default="default", + help="default honors the target branch's merge queue configuration", + ) + parser.add_argument( + "--bypass-rules", action="store_true", help="explicitly request bypass of rules that your account may bypass" + ) + parser.add_argument( + "--timeout", type=int, default=60, help="seconds to poll for a result (default: 60; 0 returns immediately)" + ) + parser.set_defaults(handler=cmd_pr_merge) + + status = subparsers.add_parser( + "merge-status", help="inspect or wait for an existing async merge request without resubmitting" + ) + status.add_argument("request_id", help="merge request UUID") + status.add_argument("--pr", help="PR number/url/branch") + status.add_argument("--repo", help="repository in OWNER/REPO format") + status.add_argument("--timeout", type=int, default=0, help="seconds to poll while pending (default: 0)") + status.set_defaults(handler=cmd_pr_merge_status) + + +def cmd_pr_merge(args: Any) -> int: + _validate_timeout(args.timeout) + body = None + if args.body is not None or args.body_file is not None: + body = resolve_file_or_inline_text(args, text_attr="body", file_attr="body_file") + if args.merge_action == "merge_queue" and any( + value is not None for value in (args.merge_method, args.subject, body) + ): + raise RuntimeError("merge method, subject, and body are only supported for direct merges") + client = GitHubClient() + meta = resolve_subject(selector=args.pr, repo=args.repo, selector_flag="PR", resolver=client.resolve_pull_request) + if meta.is_draft or meta.state not in {"OPEN", "MERGED"}: + raise RuntimeError("cannot merge a draft, closed, or unknown-state pull request") + if not meta.stack_supported: + raise RuntimeError("native stack metadata is unavailable; cannot determine the merge scope") + head = meta.head_ref_oid + if args.head is not None and args.head != head: + raise RuntimeError("PR head changed; refresh the PR before merging") + if meta.state == "OPEN" and not head: + raise RuntimeError("PR head SHA is unavailable; cannot submit a merge request") + meta = client.fetch_pull_request_stack(meta) + if meta.stack: + if not meta.stack.complete or not any( + entry.number == meta.ref.number and entry.position == meta.stack.position for entry in meta.stack.entries + ): + raise RuntimeError("stack entries are incomplete; refresh the PR before merging") + scope = [ + entry for entry in meta.stack.entries if entry.position <= meta.stack.position and entry.state != "MERGED" + ] + if any(entry.is_draft or entry.state != "OPEN" for entry in scope): + raise RuntimeError("merge scope includes a draft or closed downstack PR") + print( + f"Stack merge scope into `{meta.stack.base_ref_name}`: " + + " → ".join(f"#{entry.number}" for entry in scope), + flush=True, + ) + result = client.request_pull_request_merge( + meta.ref, + sha=head, + merge_method=args.merge_method, + merge_action=args.merge_action, + subject=args.subject, + body=body, + bypass_rules=args.bypass_rules, + ) + return _wait_for_merge(client, meta.ref, result, timeout=args.timeout) + + +def cmd_pr_merge_status(args: Any) -> int: + _validate_timeout(args.timeout) + request_id = str(UUID(args.request_id)) + client = GitHubClient() + meta = resolve_subject(selector=args.pr, repo=args.repo, selector_flag="--pr", resolver=client.resolve_pull_request) + result = client.fetch_pull_request_merge(meta.ref, request_id) + return _wait_for_merge(client, meta.ref, result, timeout=args.timeout) + + +def _validate_timeout(timeout: int) -> None: + if timeout < 0: + raise RuntimeError("--timeout must be zero or greater") + + +def _wait_for_merge(client: GitHubClient, ref: PullRequestRef, result: AsyncMergeResult, *, timeout: int) -> int: + print("## Merge") + print(f"repo: {ref.owner}/{ref.name}") + print(f"pr: #{ref.number}") + if result.existing_request: + print("existing_request: true (following the existing request; new merge options were not applied)") + if result.request_id: + print(f"request_id: {result.request_id}") + for key in ("merge_method", "merge_action", "expected_head_sha", "bypass_rules"): + value = getattr(result, key) + if value is not None: + print(f"{key}: {str(value).lower() if isinstance(value, bool) else value}") + if result.status == "pending": + assert result.request_id is not None + request_id = result.request_id + print("status: pending") + resume = display_command_with( + f"pr merge-status {result.request_id} --pr {ref.number} --repo {ref.owner}/{ref.name} --timeout 60" + ) + print(f"⏎ resume: `{resume}`", flush=True) + deadline = time.monotonic() + timeout + while result.status == "pending": + remaining = deadline - time.monotonic() + if remaining <= 0: + print("Merge request is still pending; use the request ID above to check again.") + return 2 + time.sleep(min(2, remaining)) + result = client.fetch_pull_request_merge(ref, request_id) + print(f"status: {result.status}") + if result.message: + print(f"message: {result.message}") + if result.sha: + print(f"merge_commit: {result.sha}") + if result.status == "enqueued": + print("Added to the merge queue; the PR has not necessarily merged.") + print( + f"⏎ check PR: `{display_command_with(f'pr view {ref.number} --repo {ref.owner}/{ref.name} --show meta,mergeability')}`" + ) + return 1 if result.status == "failed" else 0 diff --git a/src/gh_llm/github_api.py b/src/gh_llm/github_api.py index 18541be..bd203b5 100644 --- a/src/gh_llm/github_api.py +++ b/src/gh_llm/github_api.py @@ -10,10 +10,12 @@ from fnmatch import fnmatchcase from typing import TYPE_CHECKING, cast from urllib.parse import quote, urlparse +from uuid import UUID from gh_llm.diagnostics import GhCommandError, looks_like_transport_error from gh_llm.invocation import display_command, display_command_with from gh_llm.models import ( + AsyncMergeResult, CheckItem, CheckResults, PageInfo, @@ -1715,6 +1717,53 @@ def _get_viewer_login(self) -> str: login = _as_optional_str(payload.get("login")) return login or "" + def request_pull_request_merge( + self, + ref: PullRequestRef, + *, + sha: str | None, + merge_method: str | None = None, + merge_action: str = "default", + subject: str | None = None, + body: str | None = None, + bypass_rules: bool = False, + ) -> AsyncMergeResult: + cmd = _async_merge_command(ref) + cmd.extend( + ["--method", "PUT", "-f", f"merge_action={merge_action}", "-F", f"bypass_rules={str(bypass_rules).lower()}"] + ) + for key, value in ( + ("sha", sha), + ("merge_method", merge_method), + ("commit_title", subject), + ("commit_message", body), + ): + if value is not None: + cmd.extend(["-f", f"{key}={value}"]) + try: + # A transport failure may occur after acceptance. Never retry a merge write automatically. + payload = _run_command_json(cmd) + except GhCommandError as error: + if "HTTP 409" not in error.stderr: + raise + try: + payload = _as_dict(json.loads(error.stdout), context="existing merge request") + result = _parse_async_merge_result(payload) + except ValueError, RuntimeError: + raise error from None + if result.status != "pending": + raise + return replace(result, existing_request=True) + return _parse_async_merge_result(payload) + + def fetch_pull_request_merge(self, ref: PullRequestRef, request_id: str) -> AsyncMergeResult: + request_id = str(UUID(request_id)) + payload = _run_command_json(_async_merge_command(ref, request_id)) + result = _parse_async_merge_result(payload) + if result.request_id and result.request_id != request_id: + raise RuntimeError("merge response does not match the requested UUID") + return replace(result, request_id=request_id) + def fetch_checks(self, ref: PullRequestRef, *, meta: PullRequestMeta | None = None) -> CheckResults: nodes: list[dict[str, object]] = [] head_oid = meta.head_ref_oid if meta else None @@ -2444,6 +2493,43 @@ def _graphql_query_max_attempts(query: str) -> int: return GRAPHQL_MAX_ATTEMPTS +def _async_merge_command(ref: PullRequestRef, request_id: str | None = None) -> list[str]: + endpoint = f"repos/{ref.owner}/{ref.name}/pulls/{ref.number}/merge-async" + if request_id: + endpoint += f"/{request_id}" + return [ + "gh", + "api", + endpoint, + "-H", + "Accept: application/vnd.github+json", + "-H", + "X-GitHub-Api-Version: 2026-03-10", + ] + + +def _parse_async_merge_result(payload: dict[str, object]) -> AsyncMergeResult: + status = _as_optional_str(payload.get("status")) + if status not in {"pending", "merged", "enqueued", "failed"}: + raise RuntimeError(f"unexpected asynchronous merge status: {status!r}") + details = _as_dict(payload.get("details"), context="merge result details") + request_id = _as_optional_str(details.get("uuid")) + if request_id: + request_id = str(UUID(request_id)) + if status == "pending" and not request_id: + raise RuntimeError("pending merge response is missing its request UUID") + return AsyncMergeResult( + status=status, + message=_as_optional_str(details.get("message")) or "", + request_id=request_id, + sha=_as_optional_str(details.get("sha")), + merge_method=_as_optional_str(details.get("merge_method")), + merge_action=_as_optional_str(details.get("merge_action")), + expected_head_sha=_as_optional_str(details.get("expected_head_sha")), + bypass_rules=_as_optional_bool(details.get("bypass_rules")), + ) + + def _run_command_json( cmd: list[str], *, diff --git a/src/gh_llm/merge_render.py b/src/gh_llm/merge_render.py new file mode 100644 index 0000000..905a82c --- /dev/null +++ b/src/gh_llm/merge_render.py @@ -0,0 +1,38 @@ +from typing import TYPE_CHECKING + +from gh_llm.invocation import display_command_with + +if TYPE_CHECKING: + from gh_llm.models import TimelineContext + + +def render_merge_actions(context: TimelineContext) -> list[str]: + if not context.head_ref_oid: + return ["Merge actions unavailable: refresh the PR to load its head SHA."] + available = [ + ("merge", context.merge_commit_allowed), + ("squash", context.squash_merge_allowed), + ("rebase", context.rebase_merge_allowed), + ] + enabled = [method for method, allowed in available if allowed is True] + disabled = [method for method, allowed in available if allowed is False] + lines: list[str] = [] + if enabled: + lines.append("Merge actions (async; honors the target branch's merge queue):") + subject = f"{context.title} (#{context.number})" + if not context.stack: + lines.append(f"⌨ merge_subject: '{subject}'") + if context.co_author_trailers: + lines.extend(["⌨ merge_body (default):", " ", ""]) + lines.extend(f" {trailer}" for trailer in context.co_author_trailers) + else: + lines.append("⌨ merge_body: ''") + for method in enabled: + args = f"pr merge {context.number} --repo {context.owner}/{context.name} --{method} --head {context.head_ref_oid}" + if method != "rebase" and not context.stack: + quoted_subject = "'" + subject.replace("'", "'\"'\"'") + "'" + args += f" --subject {quoted_subject} --body ''" + lines.append(f"⏎ {method}: `{display_command_with(args)}`") + if disabled: + lines.append(f"Disabled by repository settings: {', '.join(disabled)}") + return lines diff --git a/src/gh_llm/models.py b/src/gh_llm/models.py index 4189f68..85511d4 100644 --- a/src/gh_llm/models.py +++ b/src/gh_llm/models.py @@ -12,6 +12,19 @@ class PullRequestRef: number: int +@dataclass(frozen=True) +class AsyncMergeResult: + status: str + message: str = "" + request_id: str | None = None + sha: str | None = None + merge_method: str | None = None + merge_action: str | None = None + expected_head_sha: str | None = None + bypass_rules: bool | None = None + existing_request: bool = False + + @dataclass(frozen=True) class StackEntry: position: int diff --git a/src/gh_llm/render.py b/src/gh_llm/render.py index bbb44e9..d3dd27d 100644 --- a/src/gh_llm/render.py +++ b/src/gh_llm/render.py @@ -5,6 +5,7 @@ from typing import TYPE_CHECKING, cast from gh_llm.invocation import display_command, display_command_with +from gh_llm.merge_render import render_merge_actions from gh_llm.stack_render import render_stack_mergeability if TYPE_CHECKING: @@ -301,7 +302,7 @@ def render_mergeability_section( return [ "## Mergeability", "Status: Unknown; native stack metadata is unavailable.", - f"⏎ confirm on GitHub: `gh pr view {context.number} --repo {context.owner}/{context.name} --web`", + "Merge scope cannot be determined; refresh the PR once native stack metadata is available.", "", ] lines = ["## Mergeability"] @@ -384,34 +385,7 @@ def render_mergeability_section( lines.append("Status: Merging is allowed") if context.state == "OPEN" and not blockers: - merge_subject = f"{context.title} (#{context.number})" - merge_subject_quoted = _shell_single_quote(merge_subject) - available_methods = [ - ("merge", context.merge_commit_allowed), - ("squash", context.squash_merge_allowed), - ("rebase", context.rebase_merge_allowed), - ] - enabled_methods = [method for method, enabled in available_methods if enabled is True] - disabled_methods = [method for method, enabled in available_methods if enabled is False] - if enabled_methods: - lines.append("Merge actions:") - lines.append(f"⌨ merge_subject: '{merge_subject}'") - if context.co_author_trailers: - lines.append("⌨ merge_body (default):") - lines.append(" ") - lines.append("") - lines.extend(f" {trailer}" for trailer in context.co_author_trailers) - else: - lines.append("⌨ merge_body: ''") - for method in enabled_methods: - if method == "rebase": - lines.append(f"⏎ rebase via gh: `gh pr merge {context.number} --repo {repo} --rebase`") - continue - lines.append( - f"⏎ {method} via gh: `gh pr merge {context.number} --repo {repo} --{method} --subject {merge_subject_quoted} --body ''`" - ) - if disabled_methods: - lines.append(f"Disabled by repository settings: {', '.join(disabled_methods)}") + lines.extend(render_merge_actions(context)) details: list[str] = [] if merge_state: @@ -429,10 +403,6 @@ def render_mergeability_section( return lines -def _shell_single_quote(value: str) -> str: - return "'" + value.replace("'", "'\"'\"'") + "'" - - def render_hidden_gap(context: TimelineContext, hidden_pages: list[int]) -> list[str]: if not hidden_pages: return [] diff --git a/src/gh_llm/stack_render.py b/src/gh_llm/stack_render.py index d1d9d8d..c75d9fa 100644 --- a/src/gh_llm/stack_render.py +++ b/src/gh_llm/stack_render.py @@ -1,6 +1,7 @@ from typing import TYPE_CHECKING from gh_llm.invocation import display_command_with +from gh_llm.merge_render import render_merge_actions if TYPE_CHECKING: from collections.abc import Sequence @@ -46,7 +47,6 @@ def render_stack_mergeability( ) -> list[str]: stack = context.stack assert stack is not None - repo = f"{context.owner}/{context.name}" lines = ["## Mergeability"] if context.is_merged or context.state == "MERGED": return [*lines, "Status: Already merged", ""] @@ -87,7 +87,7 @@ def render_stack_mergeability( elif unknown: lines.append("Status: Stack merge readiness is unknown") else: - lines.append("Status: No blockers reported for this merge scope; confirm readiness on GitHub.") + lines.append("Status: No blockers reported for this merge scope; GitHub validates requirements when merging.") if not stack.complete: lines.append("Some stack entries are unavailable.") if requirements_known is False: @@ -97,6 +97,7 @@ def render_stack_mergeability( lines.append( "GitHub evaluates stack rules against the target branch. Optional CI failures alone are not merge blockers." ) - lines.append(f"⏎ open native stack merge controls: `gh pr view {context.number} --repo {repo} --web`") + if not blockers and not unknown: + lines.extend(render_merge_actions(context)) lines.append("") return lines diff --git a/tests/test_cli.py b/tests/test_cli.py index e29f121..606374b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -4664,14 +4664,14 @@ def test_pr_mergeability_show_merge_actions_with_repo_method_filter( assert "Co-authored-by: Alice Example " in out assert "Co-authored-by: Bob Example " in out assert ( - "⏎ merge via gh: `gh pr merge 77972 --repo PaddlePaddle/Paddle --merge --subject 'Timeline test (#77972)' --body ''`" + "⏎ merge: `gh-llm pr merge 77972 --repo PaddlePaddle/Paddle --merge --head 3333333333333333333333333333333333333333 --subject 'Timeline test (#77972)' --body ''`" in out ) assert ( - "⏎ squash via gh: `gh pr merge 77972 --repo PaddlePaddle/Paddle --squash --subject 'Timeline test (#77972)' --body ''`" + "⏎ squash: `gh-llm pr merge 77972 --repo PaddlePaddle/Paddle --squash --head 3333333333333333333333333333333333333333 --subject 'Timeline test (#77972)' --body ''`" in out ) - assert "⏎ rebase via gh: `gh pr merge 77972 --repo PaddlePaddle/Paddle --rebase`" not in out + assert "⏎ rebase:" not in out assert "Disabled by repository settings: rebase" in out @@ -4683,9 +4683,12 @@ def test_pr_mergeability_rebase_action_has_no_subject_or_body( code = cli.run(["pr", "view", "77928", "--repo", "PaddlePaddle/Paddle", "--show", "mergeability"]) assert code == 0 out = capsys.readouterr().out - assert "⏎ rebase via gh: `gh pr merge 77928 --repo PaddlePaddle/Paddle --rebase`" in out - assert "--rebase --subject" not in out - assert "--rebase --body" not in out + rebase_action = next(line for line in out.splitlines() if "⏎ rebase:" in line) + assert ( + "gh-llm pr merge 77928 --repo PaddlePaddle/Paddle --rebase --head 3333333333333333333333333333333333333333" + in rebase_action + ) + assert "--subject" not in rebase_action and "--body" not in rebase_action def test_pr_view_invalid_expand_error_lists_valid_values(capsys: pytest.CaptureFixture[str]) -> None: diff --git a/tests/test_pr_merge.py b/tests/test_pr_merge.py new file mode 100644 index 0000000..6e91523 --- /dev/null +++ b/tests/test_pr_merge.py @@ -0,0 +1,248 @@ +import json +import subprocess +from dataclasses import replace +from typing import TYPE_CHECKING + +import pytest + +from gh_llm import cli, github_api as api +from gh_llm.commands import pr_merge +from gh_llm.models import PullRequestMeta, PullRequestRef, PullRequestStack, StackEntry + +if TYPE_CHECKING: + from pathlib import Path + +REF = PullRequestRef("owner", "repo", 20) +HEAD = "a" * 40 +UUID = "630b9d5e-3f2a-4f7e-8b0c-2d5f9a8c1e42" +ENDPOINT = "repos/owner/repo/pulls/20/merge-async" +PENDING: dict[str, object] = { + "status": "pending", + "details": {"uuid": UUID, "merge_method": "squash", "merge_action": "default", "expected_head_sha": HEAD}, +} +MERGED: dict[str, object] = {"status": "merged", "details": {"message": "Pull request was merged.", "sha": "b" * 40}} + + +@pytest.fixture +def meta(monkeypatch: pytest.MonkeyPatch) -> PullRequestMeta: + value = PullRequestMeta( + REF, "A change", "https://github.com/owner/repo/pull/20", "alice", "OPEN", False, "", "", head_ref_oid=HEAD + ) + monkeypatch.setattr(api.GitHubClient, "resolve_pull_request", lambda *_args, **_kwargs: value) + monkeypatch.setattr(api.GitHubClient, "fetch_pull_request_stack", lambda _self, meta: meta) + monkeypatch.setattr(pr_merge.time, "sleep", lambda _seconds: None) + return value + + +def respond( + monkeypatch: pytest.MonkeyPatch, responses: list[tuple[dict[str, object] | str, int, str]] +) -> list[list[str]]: + calls: list[list[str]] = [] + + def run(cmd: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: + calls.append(cmd) + payload, code, stderr = responses.pop(0) + return subprocess.CompletedProcess( + cmd, code, stdout=payload if isinstance(payload, str) else json.dumps(payload), stderr=stderr + ) + + monkeypatch.setattr(api.subprocess, "run", run) + return calls + + +def test_merge_waits_and_preserves_explicit_message( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], tmp_path: Path +) -> None: + body = "Details: `code` and $literal\n\nCo-authored-by: Codex \n" + body_file = tmp_path / "body.md" + body_file.write_text(body) + calls = respond(monkeypatch, [(PENDING, 0, ""), (MERGED, 0, "")]) + assert ( + cli.run( + [ + "pr", + "merge", + "20", + "--repo", + "owner/repo", + "--squash", + "--head", + HEAD, + "--subject", + "A 'quoted' title", + "--body-file", + str(body_file), + ] + ) + == 0 + ) + put, get = calls + assert put[:3] == ["gh", "api", ENDPOINT] + assert "PUT" in put and f"sha={HEAD}" in put + assert "commit_title=A 'quoted' title" in put and f"commit_message={body}" in put + assert "merge_method=squash" in put and "merge_action=default" in put and "bypass_rules=false" in put + assert "X-GitHub-Api-Version: 2026-03-10" in put + assert get[:3] == ["gh", "api", f"{ENDPOINT}/{UUID}"] and "PUT" not in get + out = capsys.readouterr().out + assert f"request_id: {UUID}" in out and "status: pending" in out and "status: merged" in out + assert f"merge_commit: {'b' * 40}" in out and f"pr merge-status {UUID}" in out + + +@pytest.mark.parametrize(("status", "exit_code"), [("merged", 0), ("enqueued", 0), ("failed", 1)]) +def test_immediate_results( + meta: PullRequestMeta, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + status: str, + exit_code: int, +) -> None: + calls = respond(monkeypatch, [({"status": status, "details": {"message": "API result"}}, 0, "")]) + assert cli.run(["pr", "merge", "20", "--repo", "owner/repo"]) == exit_code + assert len(calls) == 1 + out = capsys.readouterr().out + assert f"status: {status}" in out + if status == "enqueued": + assert "has not necessarily merged" in out and "status: merged" not in out + + +def test_existing_request_is_followed_without_applying_new_options( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + calls = respond(monkeypatch, [(PENDING, 1, "gh: request exists (HTTP 409)"), (MERGED, 0, "")]) + assert cli.run(["pr", "merge", "20", "--rebase"]) == 0 + out = capsys.readouterr().out + assert "existing_request: true" in out and "new merge options were not applied" in out + assert "merge_method: squash" in out + assert sum("PUT" in call for call in calls) == 1 + + +@pytest.mark.parametrize("timeout", [0, 3]) +def test_pending_timeout_retains_resume_command( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], timeout: int +) -> None: + times = iter([0, 0, 3]) + monkeypatch.setattr(pr_merge.time, "monotonic", lambda: next(times)) + calls = respond(monkeypatch, [(PENDING, 0, ""), (PENDING, 0, "")]) + assert cli.run(["pr", "merge", "20", "--timeout", str(timeout)]) == 2 + assert len(calls) == (1 if timeout == 0 else 2) + out = capsys.readouterr().out + assert f"pr merge-status {UUID} --pr 20 --repo owner/repo --timeout 60" in out + assert "still pending" in out and "status: merged" not in out + + +def test_merge_status_only_reads_existing_request( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + calls = respond(monkeypatch, [(PENDING, 0, ""), (MERGED, 0, "")]) + assert cli.run(["pr", "merge-status", UUID, "--pr", "20", "--repo", "owner/repo", "--timeout", "60"]) == 0 + assert len(calls) == 2 and all(call[2] == f"{ENDPOINT}/{UUID}" and "PUT" not in call for call in calls) + assert "status: merged" in capsys.readouterr().out + + +@pytest.mark.parametrize("during_poll", [False, True]) +def test_network_failure_never_replays_merge( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], during_poll: bool +) -> None: + responses: list[tuple[dict[str, object] | str, int, str]] = [(PENDING, 0, "")] if during_poll else [] + responses.append(("", 1, "connection reset by peer")) + calls = respond(monkeypatch, responses) + assert cli.run(["pr", "merge", "20"]) == 1 + assert sum("PUT" in call for call in calls) == 1 + out = capsys.readouterr().out + assert "status: merged" not in out + if during_poll: + assert f"pr merge-status {UUID}" in out + + +@pytest.mark.parametrize( + "changes", + [ + {"head_ref_oid": "c" * 40}, + {"head_ref_oid": None}, + {"is_draft": True}, + {"state": "CLOSED"}, + {"state": "UNKNOWN"}, + {"stack_supported": False}, + ], +) +def test_unsafe_snapshot_does_not_submit( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, changes: dict[str, object] +) -> None: + value = replace(meta, **changes) # type: ignore[invalid-argument-type] + monkeypatch.setattr(api.GitHubClient, "resolve_pull_request", lambda *_args, **_kwargs: value) + calls = respond(monkeypatch, []) + assert cli.run(["pr", "merge", "20", "--head", HEAD]) == 1 + assert calls == [] + + +def test_stack_merge_submits_selected_pr_once( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + entries = ( + StackEntry(1, 10, "Lower", "OPEN"), + StackEntry(2, 20, "Selected", "OPEN"), + StackEntry(3, 30, "Upper", "OPEN", is_draft=True), + ) + value = replace(meta, stack=PullRequestStack(50, 2, 3, "main", entries)) + monkeypatch.setattr(api.GitHubClient, "resolve_pull_request", lambda *_args, **_kwargs: value) + calls = respond(monkeypatch, [(MERGED, 0, "")]) + assert cli.run(["pr", "merge", "20", "--squash"]) == 0 + out = capsys.readouterr().out + assert "Stack merge scope into `main`: #10 → #20" in out and "#30" not in out + assert len(calls) == 1 and calls[0][2] == ENDPOINT + + +@pytest.mark.parametrize( + "entries", + [ + (StackEntry(2, 20, "Selected", "OPEN"),), + (StackEntry(1, 10, "Lower", "OPEN", is_draft=True), StackEntry(2, 20, "Selected", "OPEN")), + (StackEntry(1, 10, "Lower", "OPEN"), StackEntry(2, 30, "Other", "OPEN")), + ], +) +def test_incomplete_or_blocked_stack_does_not_submit( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, entries: tuple[StackEntry, ...] +) -> None: + value = replace(meta, stack=PullRequestStack(50, 2, 2, "main", entries)) + monkeypatch.setattr(api.GitHubClient, "resolve_pull_request", lambda *_args, **_kwargs: value) + calls = respond(monkeypatch, []) + assert cli.run(["pr", "merge", "20"]) == 1 + assert calls == [] + + +def test_queue_and_explicit_bypass_are_sent(meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch) -> None: + calls = respond(monkeypatch, [({"status": "enqueued", "details": {}}, 0, "")]) + assert cli.run(["pr", "merge", "20", "--merge-action", "merge_queue", "--bypass-rules"]) == 0 + assert "merge_action=merge_queue" in calls[0] and "bypass_rules=true" in calls[0] + assert not any(field.startswith("merge_method=") for field in calls[0]) + + +@pytest.mark.parametrize( + "args", + [ + ["merge", "20", "--timeout", "-1"], + ["merge", "20", "--merge-action", "merge_queue", "--squash"], + ["merge-status", "../invalid", "--pr", "20"], + ], +) +def test_invalid_arguments_do_not_call_api( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, args: list[str] +) -> None: + calls = respond(monkeypatch, []) + assert cli.run(["pr", *args]) == 1 + assert calls == [] + + +@pytest.mark.parametrize( + "payload", + [ + {"status": "pending", "details": {}}, + {"status": "unknown", "details": {}}, + {"status": "pending", "details": {"uuid": "not-a-uuid"}}, + ], +) +def test_malformed_result_is_not_success( + meta: PullRequestMeta, monkeypatch: pytest.MonkeyPatch, payload: dict[str, object] +) -> None: + respond(monkeypatch, [(payload, 0, "")]) + assert cli.run(["pr", "merge", "20"]) == 1 diff --git a/tests/test_stacks.py b/tests/test_stacks.py index ad56d02..f287c59 100644 --- a/tests/test_stacks.py +++ b/tests/test_stacks.py @@ -35,6 +35,7 @@ def meta(*, position: int = 2, size: int = 3) -> PullRequestMeta: head_ref_oid=HEAD, head_ref_name="layer-2", base_ref_name="layer-1", + squash_merge_allowed=True, stack=PullRequestStack(number=50, position=position, size=size, base_ref_name="main"), ) @@ -82,7 +83,8 @@ def test_merge_scope_skips_merged_layers_and_excludes_upper_blockers() -> None: assert "scope into `main`: #20\n" in rendered assert "No blockers reported" in rendered assert "#30:" not in rendered and "gh pr merge" not in rendered - assert "--web" in rendered + assert f"gh-llm pr merge 20 --repo owner/repo --squash --head {HEAD}" in rendered + assert "--web" not in rendered and "--subject" not in rendered def test_downstack_draft_review_and_required_checks_block_merge() -> None: @@ -92,6 +94,7 @@ def test_downstack_draft_review_and_required_checks_block_merge() -> None: assert "Stack merge is blocked" in rendered assert "#10: draft, review: REVIEW_REQUIRED" in rendered assert "#20: required checks not passed: test" in rendered + assert "gh-llm pr merge" not in rendered and "--web" not in rendered @pytest.mark.parametrize( @@ -104,6 +107,7 @@ def test_downstack_draft_review_and_required_checks_block_merge() -> None: def test_incomplete_or_unknown_stack_cannot_claim_ready(entries: tuple[StackEntry, ...]) -> None: rendered = "\n".join(render_mergeability_section(context=context(entries), checks=[])) assert "readiness is unknown" in rendered and "Merging is allowed" not in rendered + assert "gh-llm pr merge" not in rendered and "--web" not in rendered def test_optional_check_failure_does_not_block_stack() -> None: