Skip to content

Commit ed2487c

Browse files
authored
feat(stack): preserve approvals by default in stack push (#1293)
`mergify stack push` now auto-skips the stack rebase when any PR in the stack has an APPROVED review, preventing GitHub's "dismiss stale approvals" branch protection from wiping reviews on a routine push. Use `--force-rebase` (mutually exclusive with `--skip-rebase`) to rebase anyway and accept the approval loss. If the bottom PR has a merge conflict (mergeable_state == "dirty"), the rebase runs regardless — the stack cannot merge without it, so preserving approvals is moot. The rebase decision lives in a new `mergify_cli/stack/approvals.py` module: `fetch_approved_pull_numbers` collapses review timelines per-reviewer (ignoring COMMENTED/PENDING, with DISMISSED and CHANGES_REQUESTED overriding earlier APPROVED); `bottom_pull_has_conflict` checks the bottom PR with one retry for GitHub's lazy mergeable computation; `decide_rebase` combines both into a typed RebaseDecision. `stack_push` consults it once per invocation, and the dry-run preview shows which PRs would keep their approvals.
1 parent c8c4189 commit ed2487c

6 files changed

Lines changed: 1330 additions & 60 deletions

File tree

mergify_cli/github_types.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class PullRequest(typing.TypedDict):
2121
merged_at: str | None
2222
merge_commit_sha: str | None
2323
mergeable: bool | None
24+
mergeable_state: typing.NotRequired[str | None]
2425

2526

2627
class Comment(typing.TypedDict):

mergify_cli/stack/approvals.py

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
#
2+
# Copyright © 2026 Mergify SAS
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License"); you may
5+
# not use this file except in compliance with the License. You may obtain
6+
# a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
# License for the specific language governing permissions and limitations
14+
# under the License.
15+
from __future__ import annotations
16+
17+
import asyncio
18+
import dataclasses
19+
import enum
20+
import typing
21+
22+
import httpx
23+
24+
25+
if typing.TYPE_CHECKING:
26+
from mergify_cli import github_types
27+
from mergify_cli.stack import changes
28+
29+
30+
MAX_CONCURRENT_API_CALLS = 5
31+
32+
33+
async def _pull_is_approved(
34+
client: httpx.AsyncClient,
35+
user: str,
36+
repo: str,
37+
pull_number: int,
38+
sem: asyncio.Semaphore,
39+
) -> bool:
40+
async with sem:
41+
r = await client.get(f"/repos/{user}/{repo}/pulls/{pull_number}/reviews")
42+
r.raise_for_status()
43+
reviews = r.json()
44+
45+
latest_by_reviewer: dict[str, str] = {}
46+
for review in reviews:
47+
state = review["state"]
48+
if state in {"COMMENTED", "PENDING"}:
49+
continue
50+
login = (review.get("user") or {}).get("login")
51+
if not login:
52+
continue
53+
latest_by_reviewer[login] = state
54+
55+
return any(state == "APPROVED" for state in latest_by_reviewer.values())
56+
57+
58+
async def fetch_approved_pull_numbers(
59+
client: httpx.AsyncClient,
60+
user: str,
61+
repo: str,
62+
pulls: list[github_types.PullRequest],
63+
) -> set[int]:
64+
if not pulls:
65+
return set()
66+
67+
sem = asyncio.Semaphore(MAX_CONCURRENT_API_CALLS)
68+
numbers = [int(pull["number"]) for pull in pulls]
69+
results = await asyncio.gather(
70+
*(_pull_is_approved(client, user, repo, n, sem) for n in numbers),
71+
)
72+
return {n for n, approved in zip(numbers, results, strict=True) if approved}
73+
74+
75+
CONFLICT_STATE = "dirty"
76+
_MERGEABLE_RETRY_DELAY_SECONDS = 1.0
77+
78+
79+
async def bottom_pull_has_conflict(
80+
client: httpx.AsyncClient,
81+
user: str,
82+
repo: str,
83+
bottom_pull: github_types.PullRequest | None,
84+
) -> bool:
85+
if bottom_pull is None:
86+
return False
87+
88+
pull_number = int(bottom_pull["number"])
89+
url = f"/repos/{user}/{repo}/pulls/{pull_number}"
90+
91+
try:
92+
r = await client.get(url)
93+
r.raise_for_status()
94+
data = r.json()
95+
96+
if data.get("mergeable") is None:
97+
await asyncio.sleep(_MERGEABLE_RETRY_DELAY_SECONDS)
98+
r = await client.get(url)
99+
r.raise_for_status()
100+
data = r.json()
101+
except httpx.HTTPError:
102+
return False
103+
104+
return bool(data.get("mergeable_state") == CONFLICT_STATE)
105+
106+
107+
class RebaseReason(enum.Enum):
108+
EXPLICIT_SKIP = "explicit_skip"
109+
FORCED = "forced"
110+
CONFLICT_OVERRIDE = "conflict_override"
111+
SKIPPED_FOR_APPROVALS = "skipped_for_approvals"
112+
NO_APPROVALS = "no_approvals"
113+
114+
115+
@dataclasses.dataclass
116+
class RebaseDecision:
117+
should_rebase: bool
118+
reason: RebaseReason
119+
approved_pulls: list[github_types.PullRequest]
120+
121+
122+
async def decide_rebase(
123+
client: httpx.AsyncClient,
124+
user: str,
125+
repo: str,
126+
*,
127+
planned_changes: changes.Changes,
128+
skip_rebase: bool,
129+
force_rebase: bool,
130+
) -> RebaseDecision:
131+
if skip_rebase:
132+
return RebaseDecision(
133+
should_rebase=False,
134+
reason=RebaseReason.EXPLICIT_SKIP,
135+
approved_pulls=[],
136+
)
137+
if force_rebase:
138+
return RebaseDecision(
139+
should_rebase=True,
140+
reason=RebaseReason.FORCED,
141+
approved_pulls=[],
142+
)
143+
144+
# All live PRs in the stack (excluding already-merged ones). A rebase
145+
# force-pushes every branch in the stack, not just the ones whose local
146+
# commit currently differs from the remote head — `skip-up-to-date` PRs
147+
# become `update` once the stack gets rebased, and their approvals would
148+
# be dismissed too.
149+
stack_pulls = [
150+
change.pull
151+
for change in planned_changes.locals
152+
if change.action != "skip-merged" and change.pull is not None
153+
]
154+
155+
approved_numbers = await fetch_approved_pull_numbers(
156+
client,
157+
user,
158+
repo,
159+
stack_pulls,
160+
)
161+
approved_pulls = [p for p in stack_pulls if int(p["number"]) in approved_numbers]
162+
163+
# Bottom PR = first live (non-merged) change in the stack. If it's a new
164+
# `create`, there's no existing PR to check. Otherwise its pull is what
165+
# can force a rebase via a dirty mergeable state.
166+
bottom_pull: github_types.PullRequest | None = None
167+
for change in planned_changes.locals:
168+
if change.action == "skip-merged":
169+
continue
170+
bottom_pull = change.pull
171+
break
172+
has_conflict = await bottom_pull_has_conflict(client, user, repo, bottom_pull)
173+
174+
if approved_pulls and has_conflict:
175+
return RebaseDecision(
176+
should_rebase=True,
177+
reason=RebaseReason.CONFLICT_OVERRIDE,
178+
approved_pulls=approved_pulls,
179+
)
180+
if approved_pulls:
181+
return RebaseDecision(
182+
should_rebase=False,
183+
reason=RebaseReason.SKIPPED_FOR_APPROVALS,
184+
approved_pulls=approved_pulls,
185+
)
186+
return RebaseDecision(
187+
should_rebase=True,
188+
reason=RebaseReason.NO_APPROVALS,
189+
approved_pulls=[],
190+
)

mergify_cli/stack/cli.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ async def get_default_token() -> str:
7171

7272

7373
def token_to_context(ctx: click.Context, _param: click.Parameter, value: str) -> None:
74+
if ctx.obj is None:
75+
ctx.obj = {}
7476
ctx.obj["token"] = value
7577

7678

@@ -79,6 +81,8 @@ def github_server_to_context(
7981
_param: click.Parameter,
8082
value: str,
8183
) -> None:
84+
if ctx.obj is None:
85+
ctx.obj = {}
8286
ctx.obj["github_server"] = value
8387

8488

@@ -315,7 +319,13 @@ async def new(
315319
)
316320

317321

318-
@stack.command(help="Push/sync the pull requests stack")
322+
@stack.command(
323+
help=(
324+
"Push/sync the pull requests stack. "
325+
"By default, `stack push` skips the rebase when any PR in the stack "
326+
"has approvals; use --force-rebase to rebase anyway."
327+
),
328+
)
319329
@click.pass_context
320330
@click.option(
321331
"--setup",
@@ -340,6 +350,12 @@ async def new(
340350
is_flag=True,
341351
help="Skip stack rebase",
342352
)
353+
@click.option(
354+
"--force-rebase",
355+
is_flag=True,
356+
help="Rebase the stack even if PRs have approvals "
357+
"(mutually exclusive with --skip-rebase)",
358+
)
343359
@click.option(
344360
"--draft",
345361
"-d",
@@ -408,6 +424,7 @@ async def push(
408424
dry_run: bool,
409425
next_only: bool,
410426
skip_rebase: bool,
427+
force_rebase: bool,
411428
draft: bool,
412429
keep_pull_request_title_and_body: bool,
413430
author: str,
@@ -417,6 +434,10 @@ async def push(
417434
no_revision_history: bool,
418435
no_verify: bool,
419436
) -> None:
437+
if skip_rebase and force_rebase:
438+
msg = "--skip-rebase and --force-rebase are mutually exclusive"
439+
raise click.UsageError(msg)
440+
420441
if setup:
421442
# backward compat
422443
await stack_setup_mod.stack_setup()
@@ -430,6 +451,7 @@ async def push(
430451
github_server=ctx.obj["github_server"],
431452
token=ctx.obj["token"],
432453
skip_rebase=skip_rebase,
454+
force_rebase=force_rebase,
433455
next_only=next_only,
434456
branch_prefix=branch_prefix,
435457
dry_run=dry_run,

0 commit comments

Comments
 (0)