Skip to content

Repository navigation

chore: detect vnext release merge by ancestry, not branch name #714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
John McCall (lowlydba) wants to merge 3 commits into from
Closed

chore: detect vnext release merge by ancestry, not branch name #714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69cb339
chore: detect vnext release merge by ancestry, not branch name
lowlydba Sep 2, 2026
bd0c138
chore: surface skip reasons as GH Actions notice annotations
lowlydba Sep 2, 2026
6f32fdc
fix: run detect-release unconditionally to avoid needs-skip propagation
lowlydba Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions .github/workflows/rebase-vnext.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ name: Rebase vnext onto main
# Triggered on every push to main. Force-rebases vnext onto the new main HEAD
# using the overture-pull-requester GitHub App (which has branch-protection bypass).
#
# Skipped for vnext→main release merges — vnext is already equal to main at that
# point so a rebase would be a no-op, and the GitHub API check catches this.
# Skipped for a vnext release merge — detected by ancestry (origin/vnext is
# already an ancestor of the new main HEAD), not by branch name, so a staging
# branch built on top of vnext's tip (e.g. `2_0_0`) is skipped the same as a
# PR literally named `vnext`. vnext is already equal to main at that point so
# a rebase would be a no-op.
#
# If the rebase fails a GitHub issue is opened and assigned to the PR author
# so the conflict can be resolved manually.
Expand Down Expand Up @@ -41,38 +44,41 @@ jobs:
permission-pull-requests: read # read PR on merge commit to detect vnext→main release
permission-workflows: write # vnext commits may touch .github/workflows/**

# Detect whether this push was a vnext→main release merge.
# The GitHub API returns the PR(s) associated with the merge commit.
- name: Detect vnext→main release merge
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
fetch-depth: 0
# Use the app token so the subsequent force-push is authenticated.
token: ${{ steps.app-token.outputs.token }}
persist-credentials: true

# Detect whether this push was a vnext release merge, by ancestry rather
# than branch name: if origin/vnext is already an ancestor of the new
# main HEAD, all of vnext's commits are in main and rebasing is a no-op.
# The GitHub API additionally returns the PR associated with the merge
# commit, so a failure issue can still be filed against its author.
- name: Detect vnext release merge
id: skip
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
MAIN_SHA: ${{ github.sha }}
run: |
git fetch origin vnext

PR_JSON=$(gh api "repos/${GITHUB_REPOSITORY}/commits/${MAIN_SHA}/pulls" \
--jq '.[0] // empty' 2>/dev/null || true)

HEAD_REF=$(echo "$PR_JSON" | jq -r '.head.ref // empty' 2>/dev/null || true)
PR_NUMBER=$(echo "$PR_JSON" | jq -r '.number // empty' 2>/dev/null || true)
PR_AUTHOR=$(echo "$PR_JSON" | jq -r '.user.login // empty' 2>/dev/null || true)

if [ "$HEAD_REF" = "vnext" ]; then
echo "Skipping: this is a vnext→main release merge."
if git merge-base --is-ancestor origin/vnext "$MAIN_SHA"; then

@vcschapp Victor Schappert (vcschapp) Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[1/2] LLM overlord is reporting that origin/vnext may not be populated after git fetch origin vnext due to Git quirks. Apparently an alternative that is certain to work/can't break unexpectedly is to refer to FETCH_HEAD instead of origin/vnext.

echo "::notice::Skipping rebase: origin/vnext is already an ancestor of main, this is a release merge."
Comment on lines +73 to +74

@vcschapp Victor Schappert (vcschapp) Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we drop the skip step because if vnext is an ancestor of main, the rebase should always be a clean fast-forward. Skipping doesn't really buy us anything and adds complexity.

I think it's probably wrong also, in that after this change, the rebase won't run on every merge to main, it'll only run when vnext contains at least one commit that's not on main. This will mean that if a sequence of PRs target main then main will drift further and further ahead of vnext.

Basically on the old system if you pushed a commit B on main after a release (r), vnext would be rebased up to A:

Starting point after release `r` (old system):

   A ─── r        main
    \
     v            vnext  (behind r)

Push a new commit `B` to `main` and `vnext` is fast-forwarded up to `B`:

   A ─── r ─── B     main
              ╱
     v ──────╯      vnext

In the new system if this happens, then vnext is an ancestor of B so there's no fast-forward...

   A ─── r ─── B     main
    \
     v               vnext  (behind r)

And this would continue if you keep tacking on new commits C, D, etc.

   A ─── r ─── B ─── C ─── D  main
    \
     v                        vnext (drifting further behind, one push at a time...)

If we just get rid of the skip and always run the fast-forward I think we get rid of this problem, yes?

(Note: This is a Claude-detected bug but it makes sense to me when I play it through.)

echo "skip=true" >> "$GITHUB_OUTPUT"
else
echo "skip=false" >> "$GITHUB_OUTPUT"
echo "pr_number=${PR_NUMBER}" >> "$GITHUB_OUTPUT"
echo "pr_author=${PR_AUTHOR}" >> "$GITHUB_OUTPUT"
fi

- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
if: steps.skip.outputs.skip != 'true'
with:
fetch-depth: 0
# Use the app token so the subsequent force-push is authenticated.
token: ${{ steps.app-token.outputs.token }}
persist-credentials: true

- name: Rebase vnext onto new main HEAD
if: steps.skip.outputs.skip != 'true'
id: rebase
Expand Down
52 changes: 46 additions & 6 deletions .github/workflows/vnext-compat.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ name: vnext compatibility check
# exact commands to resolve it.
#
# Only PRs targeting main do that real work — compat-check is a no-op for
# anything else (including vnext→main release PRs, head_ref == 'vnext'). The
# noop job covers that inverse case, and both paths feed vnext-status so
# there's always a single, consistently-named check to require.
# anything else, including a vnext release merge. A release merge is detected
# by ancestry (origin/vnext is an ancestor of the PR head), not by branch name,
# so a staging branch built on top of vnext's tip (e.g. `2_0_0`) is recognized
# the same as a PR literally named `vnext`. The noop job covers that inverse
# case, and both paths feed vnext-status so there's always a single,
# consistently-named check to require.

on:
pull_request:
Expand All @@ -23,9 +26,45 @@ concurrency:
cancel-in-progress: true

jobs:
detect-release:
name: Detect vnext release merge
runs-on: ubuntu-slim
permissions:
contents: read
outputs:
is_release: ${{ steps.check.outputs.is_release }}
steps:
# Runs unconditionally so noop/compat-check (which `need` this job) never
# get skipped by dependency propagation for non-main bases; skip the
# actual check with a fast is_release=false instead.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
if: github.event.pull_request.base.ref == 'main'
with:
fetch-depth: 0
persist-credentials: false

- name: Check whether origin/vnext is an ancestor of this PR's head
id: check
env:
PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
PR_BASE_REF: ${{ github.event.pull_request.base.ref }}
run: |
if [ "$PR_BASE_REF" != "main" ]; then
echo "is_release=false" >> "$GITHUB_OUTPUT"
exit 0
fi

git fetch origin vnext
if git merge-base --is-ancestor origin/vnext "$PR_HEAD_SHA"; then

@vcschapp Victor Schappert (vcschapp) Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[2/2] LLM overlord is reporting that origin/vnext may not be populated after git fetch origin vnext due to Git quirks. Apparently an alternative that is certain to work/can't break unexpectedly is to refer to FETCH_HEAD instead of origin/vnext.

echo "is_release=true" >> "$GITHUB_OUTPUT"

@vcschapp Victor Schappert (vcschapp) Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same root cause as the comment on the skip issue, the --is-ancestor check will result in is_release=true during any period when vnext hasn't diverged from main, even though most of those occurrences wouldn't be considered releases.

My understanding of what we're trying to do here is that is_release is the workflow's way of answering the question, is this PR the one that ships accumulated breaking changes into the main line, rather than an ordinary change? And we want to know this because in the case where we were merging in a bunch of breaking changes from vnext, the compatibility check really would fail because it'd be replaying vnext's commits on top of itself.

Current check is saying "if vnext is an ancestor of main it's a release" which includes the case where main is just organically ahead of vnext because there are no new breaking changes, so it'll be true too often. If we exclude that case then I think it works:

git fetch --no-tags origin \
  '+refs/heads/main:refs/remotes/origin/main' \ # This +refs business is meant to ensure
  '+refs/heads/vnext:refs/remotes/origin/vnext' # you can validly refer to `origin/vnext and origin/main`

if   git merge-base --is-ancestor origin/vnext "$PR_HEAD_SHA" \
&& ! git merge-base --is-ancestor origin/vnext origin/main; then
  echo "is_release=true"  >> "$GITHUB_OUTPUT"
else
  echo "is_release=false" >> "$GITHUB_OUTPUT"
fi

else
echo "is_release=false" >> "$GITHUB_OUTPUT"
fi

compat-check:
name: Check vnext compatibility
if: github.event.pull_request.base.ref == 'main' && github.head_ref != 'vnext'
needs: detect-release
if: github.event.pull_request.base.ref == 'main' && needs.detect-release.outputs.is_release != 'true'
runs-on: ubuntu-slim
permissions:
contents: read
Expand Down Expand Up @@ -160,10 +199,11 @@ jobs:

noop:
name: Skip vnext compatibility check
if: github.event.pull_request.base.ref != 'main' || github.head_ref == 'vnext'
needs: detect-release
if: github.event.pull_request.base.ref != 'main' || needs.detect-release.outputs.is_release == 'true'
runs-on: ubuntu-slim
steps:
- run: echo "Base isn't main, or this is the vnext release PR — nothing to check here."
- run: echo "::notice::Base isn't main, or this is the vnext release merge — nothing to check here."

vnext-status:
name: vnext compatibility status

@vcschapp Victor Schappert (vcschapp) Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am informed by my LLM overlords that this will fail open if detect-release fails resulting in an otherwise non-mergeable change being allowed to merge.

The mechanism is apparently this dependency graph

vnext-status ──▶ are-we-good ──▶ compat-check ──▶ detect-release
                      │                                 ▲
                      └────────▶ noop ──────────────────┘

The claim is that GitHub action dependency propagation causes compat-check to be skipped if detect-release fails; and that are-we-good treats a skip (compat-check in this case) as if it was a pass.

Can you double-check this?

Expand Down
9 changes: 9 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ merge to `main`, CI cuts a published GitHub Release tagged
`<package>-v<version>` with those notes. See
[docs/versioning.md](docs/versioning.md).

The PR that carries the release merge doesn't have to be `vnext` itself: you
can stage it on a branch built on top of `vnext`'s tip (for example `2_0_0`)
instead, useful when `vnext` needs to stay open for more breaking work while
the release stabilizes. Either way it still has to land on `main` as a regular
merge commit, not a squash. [vnext-compat.yaml](.github/workflows/vnext-compat.yaml)
and [rebase-vnext.yaml](.github/workflows/rebase-vnext.yaml) detect the
release merge by ancestry (`git merge-base --is-ancestor origin/vnext <head>`)
rather than by branch name, so either path is recognized the same way.

## Opening a PR

Two CI checks may comment on your PR:
Expand Down
Loading