ci: enforce Conventional Commits with commitizen by awsmadi · Pull Request #732 · aws-cloudformation/cloudformation-guard

awsmadi · 2026-08-26T14:48:55Z

Conventional Commits are already the practice in this repository — 81 of the last 120 commits on main satisfy cz check — but nothing enforces them and nothing helps an author write one. This wires up commitizen for both.

What this adds

file	change
`pyproject.toml`	`[tool.commitizen]`, configured to match how this repository actually releases
`.pre-commit-config.yaml`	the upstream commitizen hook at the `commit-msg` stage
`.github/workflows/commit-message.yml`	checks the pull request title
`requirements-dev.txt`	`commitizen`
`CONTRIBUTING.md`	a `Commit messages` section

Three decisions, each with a plausible alternative

cz bump is deliberately not wired in. release.yml already owns versioning: it takes a version as a workflow input, rewrites the [package] version in each crate's Cargo.toml, then commits, tags and publishes. Adding cz bump would give the version two owners. So commitizen is pointed at the git tags (version_provider = "scm") rather than at a version stored in config, and no fourth place has to be kept in step with the three Cargo.toml files. cz bump --dry-run still reports the version the commits since the last tag imply, which is a reasonable way to choose the input to give release.yml.

CI checks the pull request title, not the individual commits. Pull requests are squash-merged here — 59 of the last 60 commits on main end in (#NNN) and none is a merge commit — so the title is what becomes the commit message. Requiring every commit on a branch to be well-formed would reject work in progress without protecting main. If the merge strategy ever changes to merge commits or rebase, the commitizen-branch hook from the same upstream repository is the thing to add, and there is a comment in the workflow saying so.

It is its own workflow rather than a job in pr.yml. pr.yml carries paths-ignore: guard/ts-lib/**, so a pull request touching only that directory does not run it — and the title of such a pull request still lands on main.

Two things that would otherwise fail silently

default_install_hook_types is set. A commit-msg hook is not installed by a bare pre-commit install; only the pre-commit stage is. Without that line the hook would be configured, reported by pre-commit run as skipped, and never check anything. Contributors who installed hooks before this change need to run pre-commit install again — CONTRIBUTING.md says so.

The pull request title reaches the check through the environment, not ${{ }} interpolation. A title is author-controlled text, and interpolation is substituted before the shell parses the line, so a title containing shell syntax would otherwise run as part of the step.

Why the changelog is not generated here

changelog_start_rev is set to the current release. Of the 120 commits before this change, 39 do not satisfy cz check — older bracket-style subjects such as [BUG] Update Toolchain, and dependabot's Bump <dep> from x to y. A changelog generated across that boundary would omit the earlier half rather than describe it, so generation starts from 3.2.1.

Verification

A non-conventional message is rejected by the hook at exit code 14 (commitizen check.....Failed), and a conventional one commits. Confirmed it is the commitizen hook rejecting it, not the existing cfn-guard hook, which passes in both cases.
pre-commit install with no flags lands both .git/hooks/pre-commit and .git/hooks/commit-msg.
cz check works in a shallow clone with zero tags, which is what CI checks out — the one real risk in choosing version_provider = "scm".
The twelve types named in CONTRIBUTING.md are exactly the twelve the configured pattern accepts, each confirmed individually against cz check.
pre-commit run and pre-commit run --all-files output is unchanged: commitizen is a commit-msg-stage hook and appears in neither. pre-commit.yml greps that output for exit code: 19 and exit code: 7, so this matters.
This pull request's own commit message and title both pass cz check.

black --check was not run locally — the module is not installed in this environment. No .py files are changed, so it is unaffected.

Conventional Commits are already the practice here -- 81 of the last 120 commits on main satisfy `cz check` -- but nothing enforces them and nothing helps an author write one. This wires up commitizen for both. What it adds: - `[tool.commitizen]` in pyproject.toml, configured to match how this repository actually releases: tags are bare semver without a `v` prefix, and the version lives in those tags because release.yml is what writes them. - The upstream commitizen hook at the `commit-msg` stage, so `git commit` checks the message locally. - A `Commit Message` workflow that checks the pull request title. - `commitizen` in requirements-dev.txt, and a `Commit messages` section in CONTRIBUTING.md. Three decisions worth stating, because each had a plausible alternative. `cz bump` is not wired in. release.yml already owns versioning: it takes a version as a workflow input, rewrites the `[package]` version in each crate's Cargo.toml, then commits, tags and publishes. Adding `cz bump` would give the version two owners, so commitizen is pointed at the tags rather than at a version stored in config, and no fourth place has to be kept in step. `cz bump --dry-run` still reports the version the commits imply, which is a reasonable way to choose release.yml's input. CI checks the pull request title rather than the individual commits, because pull requests are squash-merged here -- 59 of the last 60 commits on main end in `(#NNN)` and none is a merge commit -- so the title is what becomes the commit message. Requiring every commit on a branch to be well-formed would reject work in progress without protecting main. If the merge strategy changes, the `commitizen-branch` hook from the same upstream is what to add. The check is its own workflow rather than a job in pr.yml because pr.yml carries `paths-ignore: guard/ts-lib/**`, and the title of a pull request touching only that directory still lands on main. Two things that would otherwise fail silently: `default_install_hook_types` is set. A `commit-msg` hook is not installed by a bare `pre-commit install`, so without that line the hook would be configured, reported as skipped, and never check anything. Contributors who installed hooks before this change need to run `pre-commit install` again, which CONTRIBUTING.md says. The pull request title reaches the check through the environment rather than `${{ }}` interpolation. A title is author-controlled text and interpolation is substituted before the shell parses the line. `changelog_start_rev` is set to the current release. Of the 120 commits before this change 39 do not satisfy `cz check` -- older bracket-style subjects such as `[BUG] Update Toolchain`, and dependabot's `Bump <dep> from x to y` -- so a changelog generated across that boundary would omit the earlier half rather than describe it. Verified: a non-conventional message is rejected by the commitizen hook at exit code 14 and a conventional one commits; `pre-commit install` with no flags lands both hook types; `cz check` works in a shallow clone with no tags, which is what CI checks out; the types named in CONTRIBUTING.md are exactly the twelve the configured pattern accepts, each confirmed against `cz check`; and `pre-commit run` output is unchanged, which pre-commit.yml greps for specific exit codes.

…ailures `Pre-Commit Hook CI / run-unit-tests-and-lint` was failing on `ruff check ./pre_commit_hooks_tests/ ./pre_commit_hooks` with 6 errors. None of them came from this branch's change: requirements-dev.txt pinned `ruff` only by name, so CI installed whichever release was current, and a newer default rule set started reporting code that had been in the tree untouched. The cap alone does not fix it. `ruff>=0.16.4,<0.17` still resolves to 0.16.5, the current release, so the cap only stops the next minor from moving the goalposts again; the reported code still has to change. Two of the six wanted syntax this hook cannot use. setup.cfg declares `python_requires = >=3.8`, and ruff infers its floor from `project.requires-python`, which this repository does not have. Assuming a modern interpreter, it asked for `Sequence[str] | None` (UP007) and for `Sequence` from `collections.abc` (UP035). Applying either as-is would have broken the hook at its own declared minimum: PEP 604 unions raise `TypeError: unsupported operand type(s) for |` when evaluated before 3.10, and `collections.abc.Sequence` cannot be subscripted before 3.9. `--fix` would have made exactly that change, and CI would have passed anyway -- it runs `python-version: '3.x'`, which resolves to an interpreter where both forms are valid. The break would only have appeared for users on 3.8 or 3.9, at import time. So `[tool.ruff] target-version = "py38"` now states the floor ruff could not infer. That is a fix for the class rather than these two instances: version-gated rules stop proposing syntax the hook cannot run, and they resume on their own if the floor moves. It is load-bearing -- removing it makes UP035 fire again. With the floor stated, ruff asked for `from __future__ import annotations` (FA100) instead, which is the change that actually makes PEP 604 safe here by keeping annotations unevaluated. Added, and the union rewrite applied on top of it. The remaining four were version-independent: import sorting in both modules (I001, applied by ruff), and two nested `with` statements (SIM117). The latter use the comma form rather than parenthesised context managers, which are 3.9+. Verified on Python 3.9.25, below the 3.10 boundary the UP007 rewrite would have crossed, with the versions requirements-dev.txt now resolves to: ruff 0.16.5 clean, mypy 1.19.1 clean, black 25.11.0 unchanged, pytest 8.4.2 4 passed. Separately confirmed the future import is what carries the annotation, by evaluating `Sequence[str] | None` with and without it on 3.9 -- TypeError without, fine with. Not addressed here, since it is a behaviour change rather than a lint fix: `tar.extractfile(member)` is typed `IO[bytes] | None` and returns None for members that are neither regular files nor links, while the loop above it only skips directories.

satyakigh

The new workflow does not fully enforce Conventional Commit titles. Commitizen automatically accepts titles beginning with Merge, Revert, Pull request, fixup!, squash!, or amend! without validating their format. The --allow-abort option also causes a title such as # invalid title to pass.

Since the PR title becomes the squash commit on main, these bypasses allow non-conventional commits to be merged. Think cz check --commit-msg-file "$RUNNER_TEMP/pr-title.txt" --allowed-prefixes should work instead

Also need to pin commitizen==4.10.1 in requirements-dev.txt. The pre-commit hook uses 4.10.1, but CI currently installs the latest version, so local and CI validation behavior may differ

`cz check` skipped the Conventional Commits regex entirely for a title beginning with one of its default allowed prefixes (Merge, Revert, Pull request, fixup!, squash!, amend!), and `--allow-abort` accepted an empty message. So both `Merge pull request aws-cloudformation#1 from nonsense` and `# invalid title` exited 0. The second leaks through the abort path rather than the prefix list: git comment lines are stripped before the regex runs, which leaves an empty message for `--allow-abort` to permit. Since the pull request title becomes the squash commit on main, either one lets a non-conventional commit land there. Passing `--allowed-prefixes` with no values empties the exempt set, and dropping `--allow-abort` closes the empty-message path. All seven default-prefix and comment-line titles now exit 14 (`cz check` uses 14, not 1), while valid conventional titles still exit 0. Pin commitizen to 4.10.1 so that `pip install -r requirements-dev.txt` in CI installs the same version as `rev: v4.10.1` in `.pre-commit-config.yaml`. CI took whatever was newest, so a behavior change between releases could have made the pre-commit hook and the workflow disagree about the same title.

awsmadi · 2026-09-02T15:35:25Z

Both correct, and both fixed in b1e4da3. Thank you — the first one is a real hole and I had not tested the bypass prefixes at all.

The bypasses. Measured on commitizen 4.10.1, exit codes captured directly rather than through a pipe. Every one of the documented prefixes passed under the old invocation:

PR title	before	after
`Merge pull request #1 from nonsense`	0	14
`Revert "fix: something"`	0	14
`fixup! whatever`	0	14
`squash! fix: x`	0	14
`amend! fix: x`	0	14
`Pull request #2: whatever`	0	14
`# invalid title`	0	14
`this is not conventional at all`	14	14
`fix: a real conventional title`	0	0
`feat(guard): add a thing`	0	0
`fix!: a breaking change`	0	0

--allowed-prefixes with no values passes an empty list, which is what removes the default allowlist, so your suggested form is what the workflow now runs:

cz check --commit-msg-file "$RUNNER_TEMP/pr-title.txt" --allowed-prefixes

Your # invalid title case is subtler than it looks and worth recording: comment lines get stripped from a commit-message file, which leaves an empty message, and --allow-abort exists precisely to permit an empty one. So dropping --allow-abort is not just tidying — it is the second half of the same hole. An empty PR title cannot be created through the web UI anyway, so nothing legitimate needs that flag here.

The last four rows are there because a check that rejects everything would also have passed the first seven. The valid titles, including this PR's own, still exit 0.

The pin. commitizen==4.10.1 now, matching rev: v4.10.1 in .pre-commit-config.yaml. Exact rather than a range, on the reasoning that the whole point is that the hook and CI cannot drift apart. pip install --dry-run -r requirements-dev.txt resolves with no conflict, and 4.10.1 declares >=3.9,<4.0 with a py3-none-any wheel, so it stays satisfiable under python-version: '3.x'.

Rebased onto the branch's current head first, so the ruff>=0.16.4,<0.17 pin from your ruff commit is preserved — requirements-dev.txt now carries both.

One thing I did not do, and it is your call. requirements-dev.txt pins two of five now; pytest, mypy, and black are still bare, so the same local-versus-CI drift you describe applies to them. I left them alone rather than widening a commitizen PR into a dependency-policy change. Happy to pin all five here, in a separate PR, or not at all.

I also left CONTRIBUTING.md's cz check --rev-range origin/main..HEAD unchanged. That one checks branch commits, which this workflow deliberately does not, and adding --allowed-prefixes there would reject a real merge commit made while syncing with main.

…stop reverting each other PRs aws-cloudformation#732 and aws-cloudformation#733 are siblings off main, not a stack: e320695 is not an ancestor of b1e4da3. Both fix the same ruff failure in pre_commit_hooks/cfn_guard.py, and they disagreed on the import spelling, so whichever merged second would have silently reverted the other. aws-cloudformation#732 from typing import Sequence aws-cloudformation#733 from collections.abc import Sequence Take aws-cloudformation#733's spelling. aws-cloudformation#732's rationale for `typing` was that [tool.ruff] target-version = "py38" gates UP035, which is true, but it does not buy anything at the declared floor. Measured on cpython-3.8.20 rather than reasoned about: both spellings import cleanly, and both fail typing.get_type_hints() on main(). The failure reasons differ -- `typing` dies on the PEP 604 `|` ("unsupported operand type(s) for |"), `collections.abc` dies one step earlier on the PEP 585 subscript ("'ABCMeta' object is not subscriptable") -- but neither annotation is evaluable before 3.10 either way, and nothing in pre_commit_hooks/ or pre_commit_hooks_tests/ calls get_type_hints. Under `from __future__ import annotations` the annotation stays a string and main() runs fine on 3.8.20 in both variants. So collections.abc is correct at py38 and at py39+, while `typing` is correct only while the target-version pin holds. pyproject.toml is left exactly as aws-cloudformation#732 has it. target-version = "py38" is now redundant for this file: ruff exits 0 on the collections.abc spelling with the setting present and with it absent. It is still correct, and it still gates the version-sensitive pyupgrade rules for future code, so removing it is a separate call for the maintainer. Measured cells, with a positive control to show the check can fail: collections.abc + target-version = py38 ruff rc=0 collections.abc + no target-version ruff rc=0 typing + no target-version ruff rc=1, UP035 <- control typing + target-version = py38 ruff rc=0 (aws-cloudformation#732 as it stands) pre_commit_hooks/cfn_guard.py is now byte-identical to e320695's copy, blob af1737a on both sides, so the file no longer conflicts between the branches at all. That also picks up aws-cloudformation#733's comment placement in install_cfn_guard(), where the two branches had independently reflowed the same collapsed `with` block.

…ersion This PR is what introduces commitizen, and CONTRIBUTING.md recommended `cz bump --dry-run` as a convenient way to see the version the commits since the last release imply. Measured: it exits 0 and reports `3.1.12 -> 3.2.0`, and 3.2.0 had already been released. A contributor following the file would have taken a version number that is not merely stale but already taken. Cause. `version_provider = "scm"` makes commitizen read the current version from the newest tag that is an ancestor of HEAD. The release tags are not ancestors: `release.yml` tags the commit on the PR branch, and squash-merge lands a different object on main. Tag 3.2.1 is e531ef5, `chore: bump version to 3.2.1` with parent fcc079f; main carries 3e265bb, `chore: bump version to 3.2.1 (aws-cloudformation#730)`, with parent ed7c703. Different SHA, different tree, different parent. The newest version tag reachable from main is 3.1.12. Both halves of the output are wrong, not just the base version. Over the range commitizen actually walked, 3.1.12..HEAD, there are three `feat:` commits, all of which shipped in 3.2.0 or 3.2.1, so it reported a minor increment. Over the range it should have walked, 3.2.1..HEAD, there are no `feat:` commits and the highest type is `fix:`, so the correct answer was a patch: 3.2.2, not 3.2.0. Qualified rather than removed. The command is the obvious thing to try once commitizen is installed, so deleting the line leaves a contributor to find the wrong number with no warning attached. The replacement states the mechanism, so a reader can tell why the number is wrong and recognise when it stops being wrong, which "may be inaccurate" would not give them. The replacement recommends reading the tag directly. It filters to the bare `N.N.N` shape on purpose: this repository also carries `action-v*`, `pre-commit-v*` and older `v`-prefixed tags, and an unfiltered `git tag --sort=-v:refname` puts `v2.1.0-pre-rc1` first. The unfiltered form was in a draft of this paragraph and would have replaced one wrong instruction with another. changelog_start_rev = "3.2.1" is left alone deliberately. It names one of the off-history tags, but it is neither inert nor broken: `cz changelog` renders the correct Unreleased section. It survives because commitizen's range excludes the tagged commit's ancestry, and e531ef5's parent fcc079f *is* on main, so the boundary still falls in the right place. Measured both ways: with the setting, `git log 3.2.1..HEAD` is 6 commits and the changelog shows only the 2 `fix:` entries; without it the Unreleased section would span 60 commits back to 3.1.12. The imprecision is that the squashed release commit 3e265bb falls inside the range rather than bounding it, and that commit is `chore:`, which is not rendered. Worth knowing that this depends on the tagged commit's parent being on main, but there is nothing to fix here today.

awsmadi added 2 commits August 26, 2026 14:48

satyakigh requested changes Aug 28, 2026

View reviewed changes

awsmadi added 2 commits September 2, 2026 19:13

Navigation Menu

Uh oh!

ci: enforce Conventional Commits with commitizen - #732

ci: enforce Conventional Commits with commitizen#732
awsmadi wants to merge 5 commits into
aws-cloudformation:mainfrom
awsmadi:pr/commitizen

awsmadi commented Aug 26, 2026

Uh oh!

satyakigh left a comment

Uh oh!

awsmadi commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

awsmadi commented Aug 26, 2026

What this adds

Three decisions, each with a plausible alternative

Two things that would otherwise fail silently

Why the changelog is not generated here

Verification

Uh oh!

satyakigh left a comment

Choose a reason for hiding this comment

Uh oh!

awsmadi commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants