Repository navigation
ci: refuse pull request titles without a Conventional Commits type - #916
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cffe3e8a7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ragnorc
left a comment
There was a problem hiding this comment.
Recommendation: approve the code. I found no blocking defect in the reviewed head cffe3e8a7a284a0b728cdcbefff38d6356408cb2. Deployment still needs the rollout steps in the PR body.
This PR makes change history easier to classify. It requires a typed PR title, such as fix(server): reject an invalid token. A small Python checker validates the title. A base-branch workflow runs it after PR events, including title edits. The merge-queue path checks the captured commit subject. The branch-protection configuration adds the required context. Contributor documentation explains the same rule.
The general problem is admission of untyped history, rather than one badly named PR. The change addresses it at the shared admission boundary. It does not add exceptions for individual authors or past PR numbers. The validator owns the grammar. The workflow supplies event data through quoted environment variables. It does not interpolate the title into shell code. The PR path checks out the base version and never fetches the PR head. I checked the default ref/SHA selection in the exact pinned checkout implementation, not only its current README.
Tradeoffs and liability: a stdlib checker avoids an additional package manager, service, or title action. Its fixtures make the local rule easy to inspect. The cost is another required job and a repository-specific grammar to maintain. The documentation repeats parts of that grammar, so future rule changes must update those descriptions together. Five similar policy changes should retain small local owners and explicit event boundaries, rather than introduce a general policy framework. This addition reduces history-classification liability, while adding modest contributor and CI maintenance. Shape checks cannot prove that a selected type describes the change. They also do not establish breaking-change compatibility.
The operating boundary needs care. The live repository API currently reports squash_merge_commit_title=COMMIT_OR_PR_TITLE, not PR_TITLE. The PR body already identifies this rollout obligation. Set the intended title setting before enabling the required context if the contract is specifically that PR titles become squash subjects. The queue check still validates the actual captured subject rather than trusting that setting. A recent merge-group run exposed the typed subject of #914, and that SHA is now main. This is one observation, not proof for all queue configurations or title-edit races. I made no repository-setting changes.
Validation on the exact head:
python3 scripts/check-pr-title.py --self-test: passed, with 18 accepted titles, 54 refused titles and six squash subjects.- The PR's actual title passed
--title. The captured #914 subject passed--squash-subject.Update Readme (#881)failed with exit 1 for the expected missing type prefix. python3 scripts/check-merge-group-triggers.py --self-test: passed and found all 13 configured required contexts reporting on the queue.- Workflow action-pin checks passed for 77 external uses. CI-cell checks passed for 48 required test names. AGENTS link checks and
git diff --check HEAD^ HEADpassed. scripts/check-docs.pypassed for 228 Markdown files using a temporary virtual environment with the pinned documentation requirements.
This is a new contributor-policy gate, not a repair to query behavior. GQT cannot express GitHub metadata events, token permissions or branch protection. The checker fixtures are the appropriate cheap test owner. Parent af83de9d2456c3f91469132ea3b296a2e9dec4fe has no checker. I do not count that missing program as a behavioral before/after regression proof. No Rust or Lance behavior changes, and no Lance implementation assumption is needed for this scope.
I checked the GitHub event contract, the merge-group payload schema and example, Conventional Commits, and the pinned checkout ref-selection source. Local tests do not execute a hosted title-edit event or qualify the new required context. The new workflow is absent from the base branch, so its lack of a check on this PR is expected. I do not claim full exact-head CI success or a hosted run of this new workflow. No optional code requests remain. The isolated checkout is clean. I rechecked open state, head, reviews and inline comments before posting.
cffe3e8 to
8e7c45c
Compare
What & why
This PR adds
PR Title, a required check that holds every pull request title totype(scope)!: description. Every merge intomainis a squash whose subject is the title, so the title is the one linegit logand the release tooling keep; 22 of the 91 PRs between v0.11.0 and v0.12.0 carry no type, among them #861, #849, #829, #713 and #827, so a release summary built from subjects would drop exactly those.typeis one offeat fix perf refactor docs test ci build revert rfc release(nochore);(scope)is an optional lowercase comma list;!marks a breaking change; the description starts lowercase (a backtick or digit also passes), has no trailing period, and the title stays within 100 characters with no whitespace but the plain space.docs(rfc):is refused in favour ofrfc:, and GitHub'sRevert "…"default in favour ofrevert: <what is undone>; the failure message names the accepted form.scripts/check-pr-title.pyis the check: stdlib only, runs locally with--title, carries its fixtures in--self-test, and on failure prints one line with the reason, the shape and two examples, plus a GitHub error annotation..github/workflows/pr-title.ymlruns it onpull_request_target(opened, synchronize, reopened, edited) withcontents: read, the same shape asFix Regression Gate: the checker comes frommainand the PR head is never fetched, so a PR cannot edit the rule it is checked by. A title edit re-runs the check.merge_groupthe same checker runs onmerge_group.head_commit.message(first line, minus the(#N)suffix), the squash subject the queue is about to land.PR Titlejoins.github/branch-protection.jsonand the two mirrored lists; the rule lives inCONTRIBUTING.md, Pull Requests, with a pointer from the PR template andAGENTS.md.--self-testis the convention the other metadata gates follow.Backing issue / RFC
mainmaking a title-based release summary drop user-facing changesChecklist
scripts/check-pr-title.py --self-test: 18 accepted, 54 refused, 6 squash subjects; the refused fixtures include 25 subjects that reachedmainbefore the check, and the workflow runs the self-test before every title check)CONTRIBUTING.md,docs/dev/ci.md,docs/dev/branch-protection.md,.github/PULL_REQUEST_TEMPLATE.md,AGENTS.md; no user-facing surface)Local verification
python3 scripts/check-pr-title.py --self-test: ok (18 accepted, 54 refused, 6 squash subjects); not visible on this PR's checks becausepull_request_targettakes the workflow frommain, where it does not exist yetpython3 scripts/check-pr-title.py --title "ci: refuse pull request titles without a Conventional Commits type": accepted (this PR's own title)python3 scripts/check-pr-title.py --squash-subject "Update Readme (#881)": refused, exit 1, one failure line with the shape and two examples.github/workflows/pr-title.yml:onismerge_grouppluspull_request_targetwithopened, synchronize, reopened, edited; jobpr_titleis namedPR Titleactionlint,shellcheck: not run: not installed herehead_commit.messageis inferred from the payload; one queue run after rollout confirms itNotes for reviewers
pull_request_targetexecutesmain's copy of the workflow. Its first run is the first PR event after merge. After merge, apply the rule (scripts/apply-branch-protection.sh) soPR Titlebinds, and retitle the open PRs it refuses before they can queue.mainreads<title> (#N), which is consistent with it but does not prove it. Under GitHub's "Default message", a one-commit PR's squash would use the commit subject instead. Verify on the rule page before applying.rfc: <subject>for RFCs,revert: <what is undone>for reverts,release: …ordocs(release): …where chore(release): prepare notes for v0.13.1 #889 usedchore(release).choreis deliberately absent: a type should say what kind of change landed, and 0 of 205 squash-era PRs before chore(release): prepare notes for v0.13.1 #889 used it.fix:on a feature) or an empty description (fix: stuff) passes; the check is shape only, review still decides whether the type describes the change. A breaking change without!passes; the cross-check againstchangelog.d/*.breaking.mdis the follow-up PR.head_commit.message, which the edit does not change, so the subject that lands was linted; the edit itself is not detected on the queue. Theeditedevent re-runs the PR-side check, and whether a red result there removes the queued entry is undocumented by GitHub, the same gapdocs/dev/branch-protection.mdrecords forFix Regression Gate.pull_request_targetrun on the PR.