Skip to content

release.yml ignores every release label on merge commits: PR #51 shipped v1.6.2 despite release:skip #53

Description

@evandhoffman

.github/workflows/release.yml reads the PR number out of the commit subject:

number="$(git log -1 --pretty=%s | sed -n 's/.*(#\([0-9][0-9]*\))$/\1/p')"

That pattern requires the subject to end in (#N), which is what a squash
merge writes. A merge commit writes Merge pull request #N from owner/branch
and does not match, so number is empty, the gh pr view lookup is skipped
entirely, and labels stays empty. Every release label is then silently
ignored.

Verified on this repo

99d351c  Merge pull request #51 from evanwtf/docs/agents-and-readme
a500b78  Version 1.6.2 [skip ci]

PR #51's labels: Docs: add/update repository documentation, release:skip.

It shipped v1.6.2 anyway. The label was never read.

Why this is worse than the one visible symptom

release:skip failing is the case you notice, because a release appears that
should not have. The expensive cases are silent:

  • release:minor and release:major are equally dead. A merge commit
    carrying either gets the patch default. Nobody learns until a version number
    is already published and wrong, and by then the tag, the release and any
    downstream pin all disagree with intent.
  • It is not the "direct push to main" path the code deliberately allows. The
    comment says a push with no number in its subject "is simply no labels — that
    path still gets the patch default, on purpose." A labelled PR merged as a
    merge commit is a different thing wearing the same result.

Note on the existing design

The workflow's own comments show this area has already cost something: the
GitHub association query "answered correctly for two merges and returned
nothing for the next two, all four carrying the label," which is why it reads
the subject instead. That reasoning is sound. The gap is that it assumes every
PR arrives as a squash merge, and this repo produces merge commits — the last
three merges on main are all Merge pull request #N from ....

Suggested fix

Match both shapes, e.g.:

subject="$(git log -1 --pretty=%s)"
number="$(printf '%s\n' "$subject" | sed -n -e 's/.*(#\([0-9][0-9]*\))$/\1/p' \
                                          -e 's/^Merge pull request #\([0-9][0-9]*\) .*/\1/p' | head -1)"

Given the existing "a lookup that cannot answer should stop the release"
principle, it is worth considering whether a merge commit whose subject parses
to no number should also stop rather than default to patch — the current
behavior cannot distinguish "direct push, no PR" from "PR merge we failed to
parse".

Found while reviewing this repo's release history from an adjacent session;
verified against origin/main and the GitHub API, not from memory.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions