From b83017d646143a17dca158b634f4fda9c1247714 Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" Date: Thu, 16 Jul 2026 04:08:45 +0000 Subject: [PATCH] [INITIAL] Add native GitHub Stacks support (--github-stacks) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub Stacks (private preview) is a first-class REST resource: POST /repos/{owner}/{repo}/stacks links a bottom-to-top list of PR numbers into a native stack, /stacks/{n}/add appends, and /stacks/{n}/unstack dissolves. This adds an opt-in flag that links the PRs ghstack submits into such a stack, enabled per-invocation with `--github-stacks` or persistently via `ghstack config github_stacks true`. The feature requires --direct mode: the server requires each PR's base ref to be the head ref of the PR below it, which is exactly what --direct submits produce and the classic base-branch layout never satisfies. Without --direct, a warning is logged and no stack is linked. After push_updates finishes updating PRs, _sync_github_stack reconciles the server-side stack against the full local chain (all_diffs, not just the submitted prefix, so prefix-only submits don't truncate an existing stack): an in-sync stack is a no-op, new PRs on top are appended via /add, and any other restructure dissolves and recreates the stack since the API has no reorder endpoint. All stack operations are non-fatal — a 404 (Stacks not enabled on the repository) or any other error logs a warning and the PRs remain submitted normally. Live testing surfaced an undocumented server rule that shaped the design: once a PR is in a native stack, any PATCH /pulls/{n} that includes the base field is rejected with 422, even if the value is unchanged. Direct mode previously sent base on every update, so resubmitting an unchanged stack would crash. To handle this, no-op base updates are no longer sent at all, and when a resubmit genuinely retargets a PR (e.g. inserting a commit mid-stack) the affected stack is dissolved first via _unstack_prs and recreated afterwards. Supporting changes: github_real's REST layer now returns None on 204 No Content (which unstack returns), and the fake GitHub endpoint models the stacks resource — including the base-chaining rule on create/add and the no-base-PATCH rule — so tests exercise the same server behavior observed live. A new test covers stack creation, idempotent resubmit, append-on-top, prefix-only submits, and mid-stack insert (dissolve + recreate) in both direct and non-direct modes. The README documents the feature and its --direct requirement. [ghstack-poisoned] --- README.md | 19 +++++ src/ghstack/cli.py | 23 ++++++ src/ghstack/config.py | 9 +++ src/ghstack/github_fake.py | 118 +++++++++++++++++++++++++++++ src/ghstack/github_real.py | 2 + src/ghstack/submit.py | 120 +++++++++++++++++++++++++++++- src/ghstack/test_prelude.py | 8 ++ test/submit/github_stacks.py.test | 57 ++++++++++++++ 8 files changed, 355 insertions(+), 1 deletion(-) create mode 100644 test/submit/github_stacks.py.test diff --git a/README.md b/README.md index 3e59e5b..b7a3200 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,25 @@ patch update itself is included directly in the prompt. For existing PRs the patch update is an interdiff against the previously submitted PR patch, not the whole PR patch. Explicit `ghstack -m MESSAGE` still overrides this. +## Native GitHub Stacks + +If your repository has [GitHub Stacks](https://github.github.com/gh-stack/) +enabled (currently a private preview), ghstack can link the PRs it submits +into a native stack, which gets you the stack navigation UI, stack-aware +branch protection, and partial-stack merges. Enable it with: + +``` +ghstack config github_stacks true +``` + +or per-invocation with `ghstack --direct --github-stacks`. This requires +`--direct` mode, because GitHub requires each PR in a stack to target the +head branch of the PR below it, which the classic base-branch layout does +not satisfy. ghstack keeps the native stack in sync on resubmit: new +commits on top are appended, and restructures (inserts, reorders) dissolve +and recreate the stack, since the API has no reorder operation. Stack +linking failures are never fatal; the PRs themselves are always submitted. + ## How to use Make sure you have write permission to the repo you're opening PR with. diff --git a/src/ghstack/cli.py b/src/ghstack/cli.py index e2c196a..e3e98b1 100644 --- a/src/ghstack/cli.py +++ b/src/ghstack/cli.py @@ -110,6 +110,13 @@ def cli_context( default=None, help="Create stack that directly merges into main", ) +@click.option( + "--github-stacks/--no-github-stacks", + "github_stacks_opt", + is_flag=True, + default=None, + help="Link submitted PRs into a native GitHub stack (requires --direct)", +) @click.option( "--base", "-B", @@ -141,6 +148,7 @@ def main( short: bool, force: bool, direct_opt: Optional[bool], + github_stacks_opt: Optional[bool], no_skip: bool, draft: bool, base: Optional[str], @@ -166,6 +174,7 @@ def main( base=base, stack=stack, direct_opt=direct_opt, + github_stacks_opt=github_stacks_opt, reviewer=reviewer, label=label, ) @@ -505,6 +514,14 @@ def status(pull_request: str) -> None: is_flag=True, help="Create stack that directly merges into main", ) +@click.option( + "--github-stacks/--no-github-stacks", + "github_stacks_opt", + default=None, + is_flag=True, + help="Link submitted PRs into a native GitHub stack (requires --direct and " + "a repository with GitHub Stacks enabled; overrides .ghstackrc setting)", +) @click.option( "--no-fetch", is_flag=True, @@ -523,6 +540,7 @@ def submit( no_skip: bool, draft: bool, direct_opt: Optional[bool], + github_stacks_opt: Optional[bool], base: Optional[str], revs: Tuple[str, ...], stack: bool, @@ -553,6 +571,11 @@ def submit( revs=revs, stack=stack, direct_opt=direct_opt, + github_stacks=( + github_stacks_opt + if github_stacks_opt is not None + else config.github_stacks + ), reviewer=reviewer if reviewer is not None else config.reviewer, label=label if label is not None else config.label, no_fetch=no_fetch, diff --git a/src/ghstack/config.py b/src/ghstack/config.py index 9e031f1..2ded515 100644 --- a/src/ghstack/config.py +++ b/src/ghstack/config.py @@ -113,6 +113,9 @@ def get_gh_cli_credentials( ("label", Optional[str]), # Command to generate a per-PR update description from diff contents ("automsg", Optional[str]), + # Link submitted PRs into a native GitHub stack (requires direct mode + # and a repository with GitHub Stacks enabled) + ("github_stacks", bool), ], ) @@ -334,6 +337,11 @@ def read_config( else: automsg = None + if config.has_option("ghstack", "github_stacks"): + github_stacks = config.getboolean("ghstack", "github_stacks") + else: + github_stacks = False + if write_back: with open(config_path, "w") as f: config.write(f) @@ -352,6 +360,7 @@ def read_config( reviewer=reviewer, label=label, automsg=automsg, + github_stacks=github_stacks, ) logging.debug(f"conf = {conf}") return conf diff --git a/src/ghstack/github_fake.py b/src/ghstack/github_fake.py index 4871312..9eea89d 100644 --- a/src/ghstack/github_fake.py +++ b/src/ghstack/github_fake.py @@ -71,10 +71,20 @@ ) +# A native GitHub stack (the REST resource behind `gh stack`). Stack +# numbers are allocated from the same sequence as PR numbers. +@dataclass +class PullRequestStack: + number: GitHubNumber + _repository: GraphQLId + pull_request_numbers: List[GitHubNumber] + + # The "database" for our mock instance class GitHubState: repositories: Dict[GraphQLId, "Repository"] pull_requests: Dict[GraphQLId, "PullRequest"] + stacks: Dict[GitHubNumber, "PullRequestStack"] # This is very inefficient but whatever issue_comments: Dict[GraphQLId, "IssueComment"] _next_id: int @@ -142,6 +152,7 @@ def notify_merged(self, pr_resolved: ghstack.diff.PullRequestResolved) -> None: def __init__(self, upstream_sh: Optional[ghstack.shell.Shell]) -> None: self.repositories = {} self.pull_requests = {} + self.stacks = {} self.issue_comments = {} self._next_id = 5000 self._next_pull_request_number = {} @@ -417,6 +428,14 @@ async def _update_pull_async( if "title" in input and input["title"] is not None: pr.title = input["title"] if "base" in input and input["base"] is not None: + # Real GitHub rejects any PATCH with a base field on a PR that + # is part of a native stack, even if the value is unchanged + for stack in state.stacks.values(): + if number in stack.pull_request_numbers: + raise RuntimeError( + "Cannot change the base branch because the pull " + "request is part of a stack (HTTP 422)" + ) pr.baseRefName = input["base"] pr.baseRef = await repo._make_ref_async(state, pr.baseRefName) if "body" in input and input["body"] is not None: @@ -451,6 +470,79 @@ async def _update_issue_comment_async( if (r := input.get("body")) is not None: comment.body = r + def _dump_stack(self, owner: str, name: str, stack: PullRequestStack) -> Any: + state = self.state + repo = state.repository(owner, name) + prs = [state.pull_request(repo, n) for n in stack.pull_request_numbers] + return { + "number": stack.number, + "base": {"ref": prs[0].baseRefName}, + "open": True, + "pull_requests": [ + { + "number": pr.number, + "state": "closed" if pr.closed else "open", + "head": {"ref": pr.headRefName}, + "base": {"ref": pr.baseRefName}, + } + for pr in prs + ], + } + + # Mimics the base-chaining rule the real server enforces on both + # create and add: each PR's base ref must be the previous PR's head ref + def _validate_stack_chain( + self, repo: Repository, numbers: List[GitHubNumber], prev_head: Optional[str] + ) -> None: + state = self.state + for number in numbers: + for stack in state.stacks.values(): + if number in stack.pull_request_numbers: + raise RuntimeError( + f"Pull request #{number} is already in a stack (HTTP 422)" + ) + pr = state.pull_request(repo, number) + if prev_head is not None and pr.baseRefName != prev_head: + raise RuntimeError( + "Pull requests must form a stack, where each PR's base ref " + "is the previous PR's head ref (HTTP 422)" + ) + prev_head = pr.headRefName + + async def _create_stack_async( + self, owner: str, name: str, pull_requests: List[int] + ) -> Any: + state = self.state + repo = state.repository(owner, name) + numbers = [GitHubNumber(n) for n in pull_requests] + if len(numbers) < 2: + raise RuntimeError("A stack must contain at least two PRs (HTTP 422)") + self._validate_stack_chain(repo, numbers, None) + number = state.next_pull_request_number(repo.id) + stack = PullRequestStack( + number=number, + _repository=repo.id, + pull_request_numbers=numbers, + ) + state.stacks[number] = stack + return self._dump_stack(owner, name, stack) + + async def _add_to_stack_async( + self, + owner: str, + name: str, + stack_number: GitHubNumber, + pull_requests: List[int], + ) -> Any: + state = self.state + repo = state.repository(owner, name) + stack = state.stacks[stack_number] + numbers = [GitHubNumber(n) for n in pull_requests] + top = state.pull_request(repo, stack.pull_request_numbers[-1]) + self._validate_stack_chain(repo, numbers, top.headRefName) + stack.pull_request_numbers.extend(numbers) + return self._dump_stack(owner, name, stack) + # NB: This may have a payload, but we don't # use it so I didn't bother constructing it. async def _set_default_branch_async( @@ -495,6 +587,18 @@ async def _arest_impl(self, method: str, path: str, **kwargs: Any) -> Any: "id": comment.fullDatabaseId, "body": comment.body, } + if m := re.match( + r"^repos/([^/]+)/([^/]+)/stacks\?pull_request=(\d+)$", path + ): + pr_number = int(m.group(3)) + return [ + self._dump_stack(m.group(1), m.group(2), stack) + for stack in self.state.stacks.values() + if pr_number in stack.pull_request_numbers + ] + if m := re.match(r"^repos/([^/]+)/([^/]+)/stacks/(\d+)$", path): + stack = self.state.stacks[GitHubNumber(int(m.group(3)))] + return self._dump_stack(m.group(1), m.group(2), stack) elif method == "post": if m := re.match(r"^repos/([^/]+)/([^/]+)/pulls$", path): @@ -518,6 +622,20 @@ async def _arest_impl(self, method: str, path: str, **kwargs: Any) -> Any: reviewers = kwargs.get("reviewers", []) pr.reviewers.extend(reviewers) return {} + if m := re.match(r"^repos/([^/]+)/([^/]+)/stacks$", path): + return await self._create_stack_async( + m.group(1), m.group(2), kwargs["pull_requests"] + ) + if m := re.match(r"^repos/([^/]+)/([^/]+)/stacks/(\d+)/add$", path): + return await self._add_to_stack_async( + m.group(1), + m.group(2), + GitHubNumber(int(m.group(3))), + kwargs["pull_requests"], + ) + if m := re.match(r"^repos/([^/]+)/([^/]+)/stacks/(\d+)/unstack$", path): + del self.state.stacks[GitHubNumber(int(m.group(3)))] + return None if m := re.match(r"^repos/([^/]+)/([^/]+)/issues/([^/]+)/labels", path): # Handle adding labels state = self.state diff --git a/src/ghstack/github_real.py b/src/ghstack/github_real.py index 042d38a..43f9f62 100644 --- a/src/ghstack/github_real.py +++ b/src/ghstack/github_real.py @@ -224,6 +224,8 @@ async def arest(self, method: str, path: str, **kwargs: Any) -> Any: async with getattr(session, method)(url, **request_kwargs) as resp: logging.debug("%s response status: %s", log_prefix, resp.status) + if resp.status == 204: + return None try: r = await resp.json() except (aiohttp.ContentTypeError, ValueError): diff --git a/src/ghstack/submit.py b/src/ghstack/submit.py index a649f03..56597c9 100644 --- a/src/ghstack/submit.py +++ b/src/ghstack/submit.py @@ -445,6 +445,12 @@ class Submitter: # Command to generate a per-PR update description from diff contents. automsg: Optional[str] = None + # Link submitted PRs into a native GitHub stack. Only has an effect + # with --direct, because GitHub requires each PR's base ref to be the + # previous PR's head ref, which non-direct ghstack branches don't + # satisfy. Requires the repository to have GitHub Stacks enabled. + github_stacks: bool = False + # ~~~~~~~~~~~~~~~~~~~~~~~~ # Computed in post init @@ -2061,7 +2067,10 @@ def _update_pr_args(s: DiffMeta) -> Tuple[str, Dict[str, Any], Optional[str]]: ) base_kwargs = {} if self.direct: - base_kwargs["base"] = s.base + # Don't send no-op base updates: GitHub rejects any PATCH + # containing the base field on a PR in a native stack + if s.base != s.elab_diff.base_ref: + base_kwargs["base"] = s.base else: assert s.base == s.elab_diff.base_ref stack_desc = self._format_stack( @@ -2100,8 +2109,28 @@ async def _update_pr_async(s: DiffMeta) -> None: ), ) + # GitHub rejects base changes on PRs in a native stack, so any PR + # we are about to retarget must be unstacked first; + # _sync_github_stack will recreate the stack afterwards + if self.github_stacks and self.direct: + retarget_numbers = [ + s.number for s in diffs_to_submit if s.base != s.elab_diff.base_ref + ] + if retarget_numbers: + await self._unstack_prs(retarget_numbers) + await _gather_ordered(_update_pr_async(s) for s in reversed(diffs_to_submit)) + if self.github_stacks and diffs_to_submit: + if self.direct: + await self._sync_github_stack(all_diffs or diffs_to_submit) + else: + logging.warning( + "Not linking a GitHub stack: github_stacks requires --direct, " + "because GitHub requires each PR to target the head branch of " + "the PR below it" + ) + # Report what happened def format_url(s: DiffMeta) -> str: return "https://{github_url}/{owner}/{repo}/pull/{number}".format( @@ -2163,6 +2192,95 @@ def format_url(s: DiffMeta) -> str: "I did NOT close or update PRs previously associated with these commits." ) + # Dissolve any native GitHub stacks containing the given PRs. Errors + # are non-fatal here: if a PR is genuinely stacked and unstacking + # failed, the subsequent base-changing PATCH will fail loudly anyway. + async def _unstack_prs(self, numbers: List[GitHubNumber]) -> None: + prefix = f"repos/{self.repo_owner}/{self.repo_name}/stacks" + try: + memberships = await _gather_ordered( + self.github.aget(f"{prefix}?pull_request={n}") for n in numbers + ) + for stack_number in {s["number"] for r in memberships for s in r}: + await self.github.apost(f"{prefix}/{stack_number}/unstack") + logging.info( + "Dissolved GitHub stack #%s to allow retargeting; it will " + "be recreated after the update", + stack_number, + ) + except Exception: + logging.warning("Failed to unstack PRs", exc_info=True) + + # Sync the current chain of PRs to a native GitHub stack (the + # first-class REST resource behind `gh stack`). The server requires + # each PR's base ref to be the head ref of the PR below it, which is + # exactly what --direct submits produce. Stack linking failures are + # never fatal: the PRs themselves are already submitted correctly. + async def _sync_github_stack(self, all_diffs: List[DiffMeta]) -> None: + # all_diffs is in topo order (top of stack first); the stacks API + # wants bottom-to-top + chain = list(reversed(all_diffs)) + expected_base = self.base + for s in chain: + if s.base != expected_base: + logging.warning( + "Not linking a GitHub stack: PR #%s targets %s but the PR " + "below it ends at %s. This usually means the stack is " + "mid-restructure; resubmit the whole stack and try again.", + s.number, + s.base, + expected_base, + ) + return + expected_base = branch_head(s.username, s.ghnum) + numbers = [s.number for s in chain] + prefix = f"repos/{self.repo_owner}/{self.repo_name}/stacks" + try: + memberships = await _gather_ordered( + self.github.aget(f"{prefix}?pull_request={n}") for n in numbers + ) + existing = {s["number"]: s for r in memberships for s in r} + if len(existing) == 1: + (stack,) = existing.values() + open_numbers = [ + pr["number"] + for pr in stack["pull_requests"] + if pr["state"] == "open" + ] + if open_numbers[: len(numbers)] == numbers: + return + if numbers[: len(open_numbers)] == open_numbers: + added = numbers[len(open_numbers) :] + await self.github.apost( + f"{prefix}/{stack['number']}/add", pull_requests=added + ) + logging.info( + "Added %s to GitHub stack #%s", + ", ".join(f"#{n}" for n in added), + stack["number"], + ) + return + # The server has no reorder/remove endpoint, so any other + # restructure is unstack + recreate + for stack_number in existing: + await self.github.apost(f"{prefix}/{stack_number}/unstack") + if len(numbers) < 2: + return + r = await self.github.apost(prefix, pull_requests=numbers) + logging.info( + "Linked GitHub stack #%s: %s", + r["number"], + " <- ".join(f"#{n}" for n in numbers), + ) + except ghstack.github.NotFoundError: + logging.warning( + "Not linking a GitHub stack: the stacks API returned 404, which " + "usually means GitHub Stacks is not enabled on this repository " + "(it is in private preview; see https://github.github.com/gh-stack/)" + ) + except Exception: + logging.warning("Failed to link GitHub stack", exc_info=True) + async def check_invariants_for_diff( self, # the user diff is what the user actual sent us diff --git a/src/ghstack/test_prelude.py b/src/ghstack/test_prelude.py index 69ea201..20f7bc6 100644 --- a/src/ghstack/test_prelude.py +++ b/src/ghstack/test_prelude.py @@ -69,6 +69,7 @@ "get_sh", "get_upstream_sh", "get_github", + "get_github_stacks", "get_pr_reviewers", "get_pr_labels", "tick", @@ -224,6 +225,7 @@ async def gh_submit( reviewer: Optional[str] = None, label: Optional[str] = None, automsg: Optional[str] = None, + github_stacks: bool = False, ) -> List[ghstack.submit.DiffMeta]: self = CTX r = await ghstack.submit.main( @@ -247,6 +249,7 @@ async def gh_submit( reviewer=reviewer, label=label, automsg=automsg, + github_stacks=github_stacks, ) await self.check_global_github_invariants(self.direct) return r @@ -469,6 +472,11 @@ def get_pr_reviewers(pr_number: int) -> List[str]: return pr.reviewers +def get_github_stacks() -> List[List[int]]: + github = get_github() + return [list(stack.pull_request_numbers) for stack in github.state.stacks.values()] + + def get_pr_labels(pr_number: int) -> List[str]: """Get the labels for a PR number.""" github = get_github() diff --git a/test/submit/github_stacks.py.test b/test/submit/github_stacks.py.test new file mode 100644 index 0000000..1abd397 --- /dev/null +++ b/test/submit/github_stacks.py.test @@ -0,0 +1,57 @@ +from ghstack.test_prelude import * + +await init_test() + +await commit("A") +await commit("B") +A, B = await gh_submit("Initial", github_stacks=True) + +# Native GitHub stacks require direct mode (each PR must target the head +# branch of the PR below it) +if is_direct(): + assert_eq(get_github_stacks(), [[500, 501]]) +else: + assert_eq(get_github_stacks(), []) + +# Resubmitting an unchanged stack is idempotent +await amend("B") +await gh_submit("Update B", github_stacks=True) + +if is_direct(): + assert_eq(get_github_stacks(), [[500, 501]]) +else: + assert_eq(get_github_stacks(), []) + +# A new commit on top is appended to the existing stack +# (in direct mode, PR number 502 was consumed by the stack itself) +await commit("C") +await gh_submit("Add C", github_stacks=True) + +if is_direct(): + assert_eq(get_github_stacks(), [[500, 501, 503]]) +else: + assert_eq(get_github_stacks(), []) + +# Submitting only a prefix of the stack leaves the existing stack alone +await gh_submit("Update A only", github_stacks=True, revs=["HEAD~~"]) + +if is_direct(): + assert_eq(get_github_stacks(), [[500, 501, 503]]) +else: + assert_eq(get_github_stacks(), []) + +# Inserting a commit mid-stack retargets the PR above it, which requires +# dissolving and recreating the native stack +top = GitCommitHash(await git("rev-parse", "HEAD")) +await checkout(GitCommitHash(await git("rev-parse", "HEAD~"))) +await commit("M") +await cherry_pick(top) +await gh_submit("Insert M", github_stacks=True) + +if is_direct(): + # 504 is M's PR (505 was consumed by the recreated stack) + assert_eq(get_github_stacks(), [[500, 501, 504, 503]]) +else: + assert_eq(get_github_stacks(), []) + +ok()