merge queue: checking main (65ee09a) and #1762 together - #1763
Closed
mergify[bot] wants to merge 2 commits into
Closed
merge queue: checking main (65ee09a) and #1762 together#1763mergify[bot] wants to merge 2 commits into
mergify[bot] wants to merge 2 commits into
Conversation
`mergify stack push --github-native` additionally registers the pushed
stack with GitHub's native Stacks API, so GitHub renders it as a stack.
Off by default (`git config mergify-cli.stack-github-native true` to
opt in per repo); with the flag off, not a single stacks request is
issued and the flow is what it was.
Native membership is additive — Change-Ids, branch layout, stack
comments and revision history are untouched.
## What a live registration actually blocks
Registration is not inert, but it locks exactly one thing. Measured
against the live API on a sandbox repo:
- `PATCH /pulls/{n}` 422s ("Cannot change the base branch because the
pull request is part of a stack") whenever the body carries `base` —
including when it is set to the value the PR already has. That is the
whole lock. The same PATCH *without* the key succeeds while stacked
(new title, new body, even a `head` that does not exist — `head` is
not an updatable field and GitHub ignores it), and the stack survives
the force-push of its members' branches.
- The 422 is **not atomic**: a body carrying `base` *and* `title`
applies the title and rejects the base. Firing one blind and reading
the error as "nothing happened" is not an option.
- Orphan teardown deletes a dropped change's head branch, and GitHub
closes every PR still based on it. Push survives that only because
step 9 retargets the survivors *before* step 11 deletes the branch —
the very PATCH the lock blocks. A PR closed this way cannot be
reopened (base branch gone) nor retargeted (it is closed). The dropped
PR itself also stays in the stack, closed and unmerged.
- `POST /stacks` over PRs already in an open stack 422s, so *re-forming*
a stack does require an unstack first. But `POST /stacks/{n}/add`
appends onto an existing one, keeping its number, its webhooks and its
members' registration. It needs the first appended PR's base ref to be
the current top's head ref; for anything else — insert in the middle,
reorder, remove a single member — there is no endpoint at all.
## So the dance is per-shape, not per-push
| push | stacks requests |
|---|---|
| refresh commits (amend, reword, force-push) | **none** |
| append a change on top | one `POST /stacks/{n}/add` |
| retarget a pull request, or tear down an orphan | `unstack` up front, `POST /stacks` at the end |
`create_or_update_pr` now sends `base` only when the pull request is
really being retargeted — the same technique the function already used
for `title` under `--keep-pull-request-title-and-body`: to leave a field
alone, omit the key. That is what makes the common case free. It also
fixes a smaller thing on its own: we no longer ask GitHub to set a base
to the value it already has on every single update.
This matters beyond API economy. Every dissolve is a window where the
stack is unregistered, and a failed unstack is fatal by design — fewer
windows, fewer ways to leave a user half-registered. A re-registration
also mints a new stack number and re-emits `pull_request.stacked` for
every member, which watchers see.
The fence, where a push still needs one, remains the whole mutation
stage rather than one call site: the retarget can come from
`neutralize_stale_bases` or from the upsert itself, and the orphan
teardown at the end depends on it having landed.
Failure policy is deliberately asymmetric. Registering and appending
never fail a push — an old GHES, a repo without the feature, a chain
with a hole, or a stack below GitHub's 2-PR floor all just leave the PRs
unregistered and report `not registered on GitHub`. A failed *unstack
before the mutations* is fatal, because carrying on is what produces the
unrecoverable state above. A failed *append* is repaired by rebuilding
the stack, which is safe there because the mutations are already done.
## Tested
Unit + wiremock coverage of the module, plus end-to-end tests that
assert on the request *sequence* (`stack_push_github_native.rs`) — that
is where the contract lives: a routine push sends no `/stacks` request
and no `base` key, a push with a new change on top sends exactly one
`add`, an append GitHub refuses is rebuilt, and a push that retargets a
PR unstacks before the first mutation and registers after the last.
Verified the fence test fails when the fence is removed.
Byte-identical default: an end-to-end test asserts a flag-off push
issues zero `/stacks` requests, and the only CLI-schema drift is the one
new flag.
Also exercised live against a real repo with the built binary: create a
2-PR stack (registered as #33); amend and re-push (`GitHub stack #33
unchanged`, zero stacks requests — the case that 422d before `base`
became conditional); add a change on top (`added to GitHub stack #33`,
same stack number and `created_at`, three members); drop the middle
change (fence: dropped PR closed, both survivors open, retargeted and
re-registered); push again with nothing to do (`unchanged`).
MRGFY-8452
Change-Id: I13d5f27ebf27217473c2588aeab90f0c28e1a392
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
Branch main (65ee09a) and #1762 are queued together for merge.
This pull request has been created by Mergify to check the mergeability of #1762.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:github-review-approved[🛡 GitHub branch protection]github-review-approved[🛡 GitHub repository ruleset ruleRequire pull request for default branch]Enforce conventional commit]:title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:👀 Review Requirements]:#approved-reviews-by>=2author = dependabot[bot]author = mergify-ci-botauthor = renovate[bot]📕 PR description]:body ~= (?ms:.{48,})🔎 Reviews]:#changes-requested-reviews-by = 0#review-requested = 0#review-threads-unresolved = 0🤖 Continuous Integration]:check-success=ci-gateRequired conditions to stay in the queue:
base=maingithub-review-approved[🛡 GitHub branch protection]github-review-approved[🛡 GitHub repository ruleset ruleRequire pull request for default branch]label!=manual mergeEnforce conventional commit]:title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:👀 Review Requirements]:#approved-reviews-by>=2author = dependabot[bot]author = mergify-ci-botauthor = renovate[bot]📕 PR description]:body ~= (?ms:.{48,})🔎 Reviews]:#changes-requested-reviews-by = 0#review-requested = 0#review-threads-unresolved = 0🤖 Continuous Integration]:check-success=ci-gate