ci: enforce conventional commits on PRs#2
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a GitHub Actions workflow to enforce Conventional Commits formatting for all commits included in a pull request, aligning this repo’s CI checks with the org-standard commit type list.
Changes:
- Introduces a new PR-triggered workflow to lint commit subjects against a Conventional Commits regex.
- Checks all commits in the PR range and reports invalid messages via GitHub Actions annotations.
- Intends to allow merge commits while enforcing a fixed set of commit types.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three valid review comments on PR #2: 1. Merge-commit allowlist regex `^Merge\ ` had two spaces (the `\ ` escape plus a trailing literal space), so it never matched real `Merge branch …` / `Merge pull request …` subjects with a single space. Merge commits would have failed the check despite the comment claiming they're allowed. Switched to a literal glob `Merge\ *` with one space, which matches what git actually writes. 2. `echo "$msg" | grep` mishandled subjects starting with `-n`/`-e` (echo's flag parsing). Switched to `printf '%s\n' "$msg" | grep` which is safe for any input. 3. `::error::` workflow commands containing user-controlled commit subjects could inject further workflow commands via `%`, `\r`, `\n` (log spoofing or command injection). Added an `escape_workflow_msg` helper that URL-encodes those three characters per the GitHub Actions docs, applied to the subject before printing. 4. `pull-requests: read` permission was unused — workflow only reads the event payload + local git log, no API calls. Dropped per least-privilege. All four addressed.
3 tasks
Pinning to the v5 release SHA (93cb6efe18208431cddfb8368fd83d5badbf9bfd) matches the discipline used by anytown's vendored agent workflows and removes the supply-chain risk of a moving tag. The bare `@v5` tag form is still in use across the main happyvertical workflows but the vendored-agent pattern (SHA + `# v5` comment) is the better practice and easier to update via dependabot/renovate. Comment in the file documents how to refresh the SHA when bumping.
willgriffin
added a commit
that referenced
this pull request
May 22, 2026
Pattern catch-rate audit on the 17 Copilot findings from have-config #2, pr-review #2, pr-review #3, and have-config #4 showed that the post-#3 checklist catches ~9 of 17 strongly and ~3 borderline. Four additional patterns surfaced repeatedly without dedicated bullets; adding them brings the catch rate to ~14 of 17 (~82%), leaving 3 genuinely uncatchable findings (one Copilot false positive + two meta-writing-quality issues). New bullets: Theme 7 (Config hazards): - Engine/version constraints looser than what the lockfile needs. Catches the recurring `engines.node: ">=20"` vs lockfile-requires- 20.19 pattern plus its sibling on `actions/setup-node node-version`, packageManager, .nvmrc, Docker tags, CI tool pins. Theme 8 (Infra/deploy hazards): - Third-party GitHub Actions pinned to moving tags instead of commit SHAs. Real supply-chain risk; vendored anytown agents already SHA-pin. Includes the readable `# v5` comment pattern and link to GitHub's hardening docs. - Extended the existing "Interpolated shell variables into psql/sed/ perl substitutions without escaping" bullet with a sibling bullet covering shell-escape / regex visual-vs-parsed ambiguity (the `Merge\ ` two-char issue, `'\n'` vs `$'\n'`, BRE vs ERE vs PCRE flavors). Theme 11 (Mechanical): - Shebang interpreter doesn't match the file's syntax. Catches the `#!/usr/bin/env node` on TypeScript file pattern from have-config #4 and generalizes to python version mismatch and `#!/bin/sh` bashisms. Out of scope: - Library-version-specific quirks like the disableTypeChecked array-vs-object spread (Copilot was wrong about that; doesn't generalize). - Meta writing-quality concerns (don't ship pseudo-code as examples) — fits Theme 6 in spirit but adding a separate bullet would dilute the theme. Skip until it shows up again.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Add a CI workflow that validates all PR commit messages follow Conventional Commits format. Matches the pattern in
smrtandsdk..github/workflows/commitlint.ymlNo local commit-msg hook added (this repo is bash-only, no package.json — adding a JS toolchain just for commitlint is overhead). CI catches messages that slip through.
Test plan