ci: a PR that changes what we ship must say so in CHANGELOG.md - #252
Merged
Conversation
0.7.0 is why. Ninety commits landed between v0.6.9 and v0.7.0 with an EMPTY [Unreleased] section, so a minor release — two new skills, four new library modules, a new CI exit contract — reached the cut with no notes at all. Nothing told a user that /agami-eval or /agami-save-golden existed. Reconstructing that from nine merged PRs was slow, and worse, it was wrong in a way writing-at-the-time would not have been: the notes shipped a --out flag that run_golden_eval.py does not have, because a summary written weeks later describes what someone remembers building rather than what shipped. Review caught it; the author would never have written it. The gate: a PR touching plugins/ or packages/ must also touch CHANGELOG.md. Tests, docs, dev/ and workflows are NOT gated — none reach a user's machine, and a gate that fires on a test-only change gets waived by habit within a week. The escape hatch is a `no-changelog` LABEL rather than a magic string, so waiving is a visible act that survives a force-push and shows in the timeline. The decision lives in dev/changelog_gate.py as a pure function, so it is unit tested (13 tests, both directions) rather than asserted in YAML; the job only supplies the changed paths. Verified against the real backlog: the v0.6.9..fc02cfd diff trips it and names 22 files. Two adjacent fixes this made necessary: dev/ is added to the ruff targets, since it now holds gate logic CI executes rather than only local helpers, and CI's hardcoded ruff target list is aligned with dev.py's TARGETS — they had drifted, so dev.py itself was linted locally and not in CI.
There was a problem hiding this comment.
🟢 Approval recommended
The gate logic is simple, stdlib-only, covered by focused tests, and the CI wiring/docs are consistent with the stated process.
Pull request overview
Introduces a CI “changelog entry” gate to ensure any PR that changes shipped surfaces (plugins/ or packages/) also updates CHANGELOG.md (or is explicitly waived via the no-changelog label), addressing the release-note gap encountered around v0.7.0.
Changes:
- Added a stdlib-only gate script (
dev/changelog_gate.py) that flags shipped-path changes whenCHANGELOG.mdisn’t touched. - Added unit tests covering both “must fail” and “must pass” cases plus CLI behavior (
tests/test_changelog_gate.py). - Wired the gate into CI and aligned ruff targets between CI and
dev.py; documented the rule inCONTRIBUTING.md.
File summaries
| File | Description |
|---|---|
| tests/test_changelog_gate.py | Adds unit tests and CLI tests for the changelog gate behavior. |
| dev/changelog_gate.py | Implements the pure-function gate logic and CLI output/exit contract. |
| dev.py | Includes dev/ in ruff targets to lint gate logic executed by CI. |
| CONTRIBUTING.md | Documents the new changelog gate rule and the no-changelog waiver label. |
| .github/workflows/ci.yml | Adds the PR-only changelog gate job and aligns ruff target paths with dev.py. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two review findings, both in the YAML rather than the gate logic.
The `no-changelog` waiver could never clear a failed check. `on: pull_request`
defaults to [opened, synchronize, reopened], so applying the label queued no run
and the red check stood; re-running from the UI replays the original payload,
where the label is still absent. The escape hatch documented in CONTRIBUTING and
named in the gate's own failure message had no way to work short of an empty
commit.
The gate also failed open. A bare `run:` is `bash -e {0}` with no pipefail, so
`git diff | python3` reported only python's status — a failed diff would feed
empty stdin, print "nothing shipped changed" and exit 0, the job going green
having checked nothing while its message claimed otherwise.
The gate moves to its own workflow to fix the first without paying for it
everywhere: label events must trigger a re-run, but subscribing ci.yml to them
would re-run the full matrix and both safety-corpus legs on every label change.
This job takes seconds and can afford the extra triggers.
Both properties are now pinned by tests that parse the workflow, and both were
mutation-checked — dropping the label triggers or the explicit shell fails them.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes the process gap 0.7.0 exposed.
Why
Ninety commits landed between
v0.6.9andv0.7.0with an empty[Unreleased]section. A minor release — two new skills, four new library modules, a new CI exit contract — reached the cut with no notes at all; nothing told a user that/agami-evalor/agami-save-goldenexisted.Reconstructing them from nine merged PRs was slow, and worse, it was wrong in a way writing-at-the-time wouldn't have been: the notes shipped a
--outflag thatrun_golden_eval.pydoes not have. Review caught it. The author of that script never would have written it — a summary composed weeks later describes what someone remembers building rather than what shipped.The rule
A PR touching
plugins/orpackages/must also touchCHANGELOG.md.Deliberately not gated:
tests/, a package's owntests/,docs/,dev/, workflows, root markdown. None reach a user's machine on an install. This narrowness is the design: a gate that fires on a test-only change gets waived by habit inside a week, and then the release it exists to protect ships with no notes anyway.Escape hatch: the
no-changeloglabel (created on the repo). A label rather than a magic string in the title or body, because it survives a force-push and shows in the PR timeline — waiving is auditable rather than invisible. The failure message names it, since a gate that doesn't explain its own exit gets cleared by deleting the job.Shape
The decision is a pure function in
dev/changelog_gate.py, so it's unit-tested rather than asserted in YAML; the CI job only supplies the changed paths (git diff --name-only origin/<base>...HEAD, three dots so filesmainchanged under us aren't attributed to the PR).13 tests, both directions — what it catches and what it lets through. The second set matters more: they're what stops a future widening from quietly turning this into noise.
Verified against the real backlog rather than only synthetic input:
This PR touches only
dev/,tests/,.github/andCONTRIBUTING.md, so it does not trip its own gate — which is the intended behaviour, not an oversight.Two adjacent fixes it made necessary
dev/added to the ruff targets. It now holds gate logic CI executes, not just local helpers. It was already clean, so this costs nothing.dev.py'sTARGETS. They had drifted — CI lintedplugins packages testswhiledev.pyalso linteddev.py. Sodev.pywas checked locally and not in the gate, which is backwards from the stated principle that CI and local run the same rules.Verification
uv run dev.py check— 5422 passed (up 13), 12 skipped; ruff check + format clean; gitleaks clean.