Repository navigation
A change entry can name another pull request and every gate passes #980
Description
A change entry can name another pull request and every gate goes green. The workflow step that demands the entry is already holding both facts needed to catch it, and compares neither.
Replication
Open any pull request. Add docs/changes/unreleased/<some-other-pr>-a-slug.md with pr: <some-other-pr> in the front matter. check, light gate and the changelog step all pass.
Measured tonight on three of my own, all merged or open with the wrong number until a person read them:
| entry written | actual pull request | belongs to |
|---|---|---|
pr: 914 |
#954 | #914, closed |
pr: 962 |
#963 | unrelated |
pr: 975 |
#976 | #975, "A task name is a label, not the front of the namer's plan" — another lane's merged work |
The third was caught in review by a person. CI agreed with all three.
Why the gates miss it
.github/workflows/ci.yml:107-142 ("The change is written down") declares the number in its own env: block:
env:
BASE: ${{ github.event.pull_request.base.sha }}
PRNUM: ${{ github.event.pull_request.number }}and spends it on exactly one thing — printing make changelog-new PR=${PRNUM} … inside the failure message (line 134). The step then asks only that
go run ./cmd/aforge-changes checkpasses, and- some entry was added against
$BASE.
Neither reads the added file's pr:. So the job holds the real number and the added filename in the same shell and never puts them beside each other.
cmd/aforge-changes/entry.go:130-144 looks like the missing check and is not — it validates that the filename agrees with the front matter:
if n, _ := strconv.Atoi(m[1]); n != e.PR {
bad("the file is named for pull request %s and the frontmatter says %d", m[1], e.PR)
}That is internal consistency, and it is worse than nothing here: renaming the file and editing the field together keeps it green, so a wrong number passes a check whose name implies it was verified.
And make changelog-check structurally cannot catch it — a laptop has no pull request number. That is why all three went green locally and on CI alike.
Acceptance
A pull request whose added entry carries a pr: other than its own number fails "The change is written down", naming both numbers. The comparison is between two values the step already has:
- read
pr:from the file(s) reported by the existinggit diff --diff-filter=Acall - fail unless it equals
$PRNUM
It belongs in the workflow and not in aforge-changes check, for the reason above. The failure message should keep the existing tone and print the rename, since the fix is always a rename plus a one-line edit.
Why it is worth the ten lines
The number is guessed because the entry is usually written before the pull request exists — make changelog-new PR=<n> needs a number the author does not have yet. Renumbering later is easy to forget, and GitHub creates the situation by itself: a pull request auto-closed when its base branch is deleted gets reopened under a new number (#914 → #954, and lane R had #955 → #972 the same evening).
A wrong number is invisible forever after the fact. The changelog is the one file read long after anybody remembers which pull request was open that evening — CLAUDE.md sends a reader to docs/changes/unreleased/ precisely when their memory of the repository is stale, and an entry pointing at somebody else's change is worse than a missing one, because it will be believed.
Reported by lane A (three instances above), with the workflow reading and the PRNUM observation from lane R.
Drafted with CodeAF · reviewed and owned by the author
Activity