ci: enforce conventional commits and semantic versioning - #80
Conversation
Add a dependency-free validator (scripts/validate_commits.py) and a 50-case regression suite (scripts/test_validate_commits.py) that enforce Conventional Commits 1.0.0 on proposed commits and PR titles, plus Semantic Versioning 2.0.0 version strings and a --suggest-bump release-gate helper. Document the commit/version contract, 0.y.z initial-development meaning, the public API/compatibility surface, and the release gate in docs/CONTRIBUTING.md. Wire enforcement into the CI workflow so every new PR commit and the PR title fail closed, while existing base-branch commits are never re-checked; the validator is shallow-clone safe. BREAKING CHANGE: none (policy-only addition).
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53d85913d1
ℹ️ 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".
- extract the validator from the trusted base SHA on pull_request so a PR cannot weaken its own commit/title checks (matches the secret- scanner pattern) - validate release tag names against SemVer 2.0.0 by running the workflow for tag pushes and invoking --version on the pushed tag - require descriptions to begin with a lowercase letter per docs/CONTRIBUTING.md, rejecting capitalized subjects and double spaces after the colon - verify merge exemptions by parent count instead of subject prefix, closing the single-parent 'Merge ...' bypass; keep unconditional exemption only for pure plumbing lines (Revert "...", chore(deps):) - recognize BREAKING-CHANGE as synonymous with BREAKING CHANGE and classify MAJOR from the grammar's ! marker only (not any '!' in the subject) - restrict SemVer numeric components to ASCII digits (\d matched Unicode decimals) and drop the walrus operator for Python 3.7 - pass an empty base as HEAD^..HEAD so workflow_dispatch validates only the head commit instead of all history - give every Dependabot ecosystem a conventional commit-message prefix - route PR titles through an env var instead of Actions interpolation - restore the full unittest regression suite covering all of the above
|
@codex review |
|
Review request is for head 16e1cfc (fix(ci): close Codex review findings in conventional-commit gates + test(ci): give the merge test a committer identity). All 11 prior threads are resolved with fixes pushed; CI is green on this exact head. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16e1cfcbe4
ℹ️ 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".
- always extract the validator to a trusted temp copy, documenting that this bootstrap PR's checkout copy is used only until the script exists on the base branch - drop the chore(deps): prefix bypass: Dependabot messages are configured with conventional prefixes and must satisfy the grammar like every other commit - state in docs/CONTRIBUTING.md that the 100-character limit applies to the complete subject, matching is_conventional_header() - reject two or more blank lines between subject and body (exactly one separator required)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d59aeca6ec
ℹ️ 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".
- require git's generated revert form (Revert "<sha>") before exempting revert-shaped subjects; authored reverts must use the revert: type - classify BREAKING CHANGE only inside a blank-line-separated footer block, so footer-shaped body prose no longer forces MAJOR - add the edited pull_request activity type so title changes revalidate - distinguish omitted from explicitly empty --version/--pr-title/--message values; empty inputs now fail closed instead of validating the range - stable (non-prerelease) tags must point at a commit reachable from origin/main via the new --tag-commit gate in the tag workflow step; prerelease/build tags may stay branch-local - use fullmatch for SemVer so a trailing newline is rejected Regression tests cover all six findings.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eac01c93e0
ℹ️ 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".
| # "Revert ..." shape (including `Revert "<subject>"` with arbitrary text) is | ||
| # ordinary authored content and must use the approved `revert:` type. | ||
| MERGE_RE = re.compile(r"^Merge ", re.IGNORECASE) | ||
| PLUMBING_RE = re.compile(r'^Revert "[0-9a-fA-F]{40}"$') |
There was a problem hiding this comment.
Fresh evidence is the newly added full-hex-only pattern: following the git revert instruction in docs/CONTRIBUTING.md and running git revert --no-edit produces a subject such as Revert "feat: add x" with This reverts commit <sha>. in the body, not a SHA inside the subject. Consequently genuine generated reverts are rejected by the mandatory commit check, while the nonstandard subject this regex exempts can still be authored manually; recognize the real subject/body structure or require users to rewrite reverts as revert: commits.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accepted and fixed in 0ac42cf: the exemption now requires git's actual generated form — subject Revert "<original subject>" plus a body line This reverts commit <40-hex sha>., exactly what git revert --no-edit produces (the flow docs/CONTRIBUTING.md instructs). The proof line alone, or the subject shape alone, no longer exempts anything; authored reverts still use revert:. Regressions: test_git_generated_revert_form_with_body_proof_is_exempt, test_revert_subject_without_body_proof_is_validated, test_revert_proof_line_without_generated_subject_is_validated.
|
|
||
| # A scope is optional and nested in parentheses. Keep the character class broad | ||
| # but disallow parentheses/newlines to avoid structural ambiguity. | ||
| SCOPE = r"[a-zA-Z0-9_ -]+" |
There was a problem hiding this comment.
The mandatory contributor contract says the scope uses lowercase ASCII, but this character class also permits uppercase letters and arbitrary spaces, so both feat(Runner): add x and even feat( ): add x pass commit and PR-title validation. Restrict the scope grammar to nonempty lowercase scope components so the new gate actually enforces the convention it documents.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Accepted and fixed in 0ac42cf: SCOPE is now lowercase-ASCII components only ([a-z0-9_-]+ with '/' nesting), so feat(Runner):, feat(a b): and feat( ): are all rejected in commits and PR titles; lowercase/hyphenated/nested scopes still pass. Regressions: test_uppercase_scope_is_rejected, test_spaces_in_scope_are_rejected, test_empty_scope_is_rejected, test_lowercase_scopes_still_pass.
- exempt only git's real generated revert form: Revert "<subject>" plus a This-reverts-commit proof line in the body (docs/CONTRIBUTING.md instructs git revert); authored Revert-shaped subjects still require the approved revert: type - recognize git's trailer grammar in footer blocks, so a BREAKING CHANGE trailer after Reviewed-by/Closes-style trailers still classifies MAJOR - restrict scopes to lowercase ASCII components per docs/CONTRIBUTING.md; uppercase letters and spaces no longer pass - derive a newly created tag's validation range from its merge base with origin/main instead of the all-zero push payload 'before', so every commit behind a branch-local prerelease tag is validated
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ac42cfb9f
ℹ️ 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".
- accept git's generated merge-revert proof line ('This reverts commit
<sha>, reversing') so genuine 'git revert -m 1' commits validate
- require the revert proof's referenced commit to exist during range
validation, closing the fabricated-reference exemption bypass
- reject prerelease tags pointing into the main line (prereleases are
branch-local per docs/CONTRIBUTING.md)
- enforce the Conventional Commits SemVer bump against the latest stable
release tag when --version is paired with a release range
- run tag validation from the trusted merge-base revision instead of the
tagged tree, so a tagged commit cannot weaken its own tag policy
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6909c58df7
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6909c58df7
ℹ️ 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".
| prior = latest_release_tag(workspace) | ||
| if prior is None: | ||
| return [] |
There was a problem hiding this comment.
When the repository has no stable release tag—as is currently the case—this early return accepts every valid SemVer without inspecting either the candidate or the release range. Consequently v0.0.0 passes even though the function's contract requires an initial version above 0.0.0, and it cannot represent the required PATCH/MINOR/MAJOR change from the commits being released; compare the first candidate against a 0.0.0 baseline rather than skipping the bump gate.
Useful? React with 👍 / 👎.
6909c58 to
60d1a3a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60d1a3aa85
ℹ️ 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".
| types: [opened, synchronize, reopened, edited] | ||
| push: | ||
| branches: [main] | ||
| tags: ['**'] |
There was a problem hiding this comment.
For an invalid or incorrectly placed version tag, this workflow starts only after the tag ref has already been created, and the workflow has read-only permissions with no step that removes a rejected tag. A malformed or mis-versioned tag therefore remains publicly fetchable even when this check fails, so the stated tag-based release gate does not prevent publication; restrict tag creation to a protected release path that runs these checks before pushing the ref.
Useful? React with 👍 / 👎.
| # The convention checks must run trusted code: a PR can edit its own | ||
| # copy of the validator, so extract it from the base revision when it |
There was a problem hiding this comment.
When a pull request modifies .github/workflows/validate.yml, the pull_request run uses the proposed workflow definition, so the contributor can replace these extraction and validation commands with a no-op while retaining the same job and required-check name. Extracting the Python validator from the base revision therefore does not make the convention gate trusted; enforce it through a protected workflow or ruleset whose definition cannot be changed by the pull request being checked.
AGENTS.md reference: AGENTS.md:L40-L40
Useful? React with 👍 / 👎.
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: cdbe90ddfa
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 641cc21e22
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92ced28ba0
ℹ️ 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".
| if level == "MAJOR": | ||
| return new_major > old_major and (new_minor, new_patch) == (0, 0) | ||
| if level == "MINOR": | ||
| # MINOR keeps major and increases minor (0.y.z included). | ||
| return new_major == old_major and new_minor > old_minor and new_patch == 0 |
There was a problem hiding this comment.
When a release requires a MAJOR or MINOR bump, these predicates accept any larger selected component rather than the next component value. For example, after 1.2.7, a breaking change can be tagged 9.0.0 and a feature can be tagged 1.9.0, even though neither is the SemVer bump classified for the range; compare against old_major + 1 or old_minor + 1 so the release gate cannot skip arbitrary versions.
Useful? React with 👍 / 👎.
| commits = [line for line in result.stdout.splitlines() if line.strip()] | ||
| if not commits: | ||
| # Shallow clone or range resolved to nothing: resolve the head alone. | ||
| resolved = subprocess.run( |
There was a problem hiding this comment.
When the prior release tag and candidate tag point at the same commit, git rev-list base..head succeeds with an empty result, but this fallback injects the head commit anyway. If that commit was originally a feat:, for example, tagging it first as v0.1.0 and later as v0.2.0 passes check_required_bump() despite there being no commits in the new release; distinguish a successful empty range from an unresolved range instead of treating both as a single-commit range.
Useful? React with 👍 / 👎.
| result = subprocess.run( | ||
| ["git", "-C", workspace, "tag", "--list", "--merged", main_ref], | ||
| stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, |
There was a problem hiding this comment.
When a new stable tag targets an older commit that is still reachable from origin/main, limiting prior-tag discovery to tags merged into that candidate commit hides releases made later on main. For example, after v1.1.0 is published at a later commit, an older intervening fix: commit can still be tagged v1.0.1: this lookup sees only v1.0.0, the ancestry check accepts the historical commit as on main, and the patch bump passes even though it regresses the published version sequence. Check the candidate against the highest stable tag on the full main ref in addition to deriving the candidate's commit range.
Useful? React with 👍 / 👎.
| if [[ "$EVENT_NAME" == push && ( -z "$BASE_SHA" || "$BASE_SHA" =~ ^0+$ ) ]]; then | ||
| BASE_SHA="$(git merge-base "$HEAD_SHA" origin/main)" | ||
| fi | ||
| if [[ -n "$BASE_SHA" ]] && git cat-file -e "$BASE_SHA:$scanner" 2>/dev/null; then | ||
| git show "$BASE_SHA:$scanner" >"$RUNNER_TEMP/trusted-secret-scanner.py" |
There was a problem hiding this comment.
When an existing branch-local tag is moved forward, github.event.before is nonzero, so this condition does not replace BASE_SHA with the merge base and the scanner is extracted from the old tagged commit rather than trusted main history. A tag commit can harmlessly introduce a scanner that ignores selected files, pass its initial run because the main-derived scanner is used then, and later move the same tag to a descendant containing a credential; the update run executes the weakened old scanner and can report green. Fresh current-head evidence beyond the prior new-tag fix is that trusted-base normalization is still limited to empty/all-zero before values; derive scanner trust from origin/main for every tag push.
AGENTS.md reference: AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
1 participant
Add a dependency-free validator (scripts/validate_commits.py) and a 50-case regression suite (scripts/test_validate_commits.py) that enforce Conventional Commits 1.0.0 on proposed commits and PR titles, plus Semantic Versioning 2.0.0 version strings and a --suggest-bump release-gate helper.
Document the commit/version contract, 0.y.z initial-development meaning, the public API/compatibility surface, and the release gate in docs/CONTRIBUTING.md.
Wire enforcement into the CI workflow so every new PR commit and the PR title fail closed, while existing base-branch commits are never re-checked; the validator is shallow-clone safe.
Validator tests pass locally: