Skip to content

ci(lint): forbid misleading type+scope combinations in commit messages#2799

Merged
waynesun09 merged 3 commits into
mainfrom
rbean/forbid-misleading-commit-scopes
Jul 1, 2026
Merged

ci(lint): forbid misleading type+scope combinations in commit messages#2799
waynesun09 merged 3 commits into
mainfrom
rbean/forbid-misleading-commit-scopes

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Adds a custom gitlint rule (UC1) that rejects feat(ci), fix(ci), feat(e2e), and fix(e2e) with actionable error messages suggesting the correct prefix
  • Documents the forbidden combinations in COMMITS.md
  • Includes 18 pytest cases covering rejections, suggestions, valid combos, and edge cases

These type+scope combos put infrastructure noise in user-facing release notes under Features or Bug Fixes — confusing for users deciding whether to upgrade.

Test plan

  • python3 -m pytest gitlint_rules_test.py -v — 18/18 pass
  • make script-test — all pass including the new test
  • E2E verified: echo "fix(ci): foo" | gitlint --config .gitlint --ignore B6 --msg-filename /dev/stdin exits non-zero with clear message
  • Valid commits like ci(e2e):, chore(ci):, fix(mint): still pass

🤖 Generated with Claude Code

@ralphbean ralphbean requested a review from a team as a code owner June 30, 2026 19:40
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Enforce gitlint UC1 to block misleading feat/fix scopes for ci/e2e

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Add gitlint rule UC1 rejecting feat/fix with ci/e2e scopes to protect release notes.
• Document forbidden type+scope pairs and recommended replacements in COMMITS.md.
• Add pytest coverage and run it in the existing script-test Makefile target.
Diagram

graph TD
  A[/"Commit title"/] --> D["gitlint"] --> C["UC1 rule"] --> E{"Forbidden type+scope?"}
  B[".gitlint config"] --> D
  E -->|"yes"| F["RuleViolation + suggestion"] --> I[/"Non-zero exit"/]
  E -->|"no"| J[/"No violations"/] --> K[/"Zero exit"/]
  G["pytest"] --> L["gitlint_rules_test.py"] --> C
  subgraph Legend
    direction LR
    _in[/"Input"/] ~~~ _svc["Tool/runner"] ~~~ _file["Config/test file"] ~~~ _dec{"Decision"} ~~~ _out[/"Outcome"/]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Filter release notes instead of linting commits
  • ➕ Avoids adding custom gitlint rules/maintenance overhead
  • ➕ Allows historical commits/PR titles to remain unchanged while keeping user-facing notes clean
  • ➖ Noise still exists in PR titles/commit history and can confuse reviewers
  • ➖ Requires extra tooling/config in GoReleaser (or downstream) and may be harder to enforce consistently
2. Use a configurable allow/deny list via .gitlint options
  • ➕ Makes policy changes (new forbidden pairs) possible without code changes
  • ➕ Can be reused by other rules/policies
  • ➖ More upfront complexity than a small hard-coded map
  • ➖ Still requires a custom rule implementation to read options and format actionable messages

Recommendation: Current approach (a small custom gitlint rule with actionable guidance) is the best fit because it prevents misleading release-note categorization at the source and provides immediate, developer-friendly feedback. Release-note filtering was considered but would not improve commit/PR hygiene and is easier to bypass.

Files changed (5) +151 / -0

Enhancement (1) +51 / -0
forbidden_type_scope.pyIntroduce UC1 rule forbidding misleading feat/fix scopes +51/-0

Introduce UC1 rule forbidding misleading feat/fix scopes

• Implements a custom gitlint CommitRule (UC1) that parses conventional commit titles and rejects (feat|fix) combined with scopes (ci|e2e). Produces a single, actionable RuleViolation suggesting the correct prefix.

gitlint_rules/forbidden_type_scope.py

Tests (1) +87 / -0
gitlint_rules_test.pyAdd pytest coverage for UC1 forbidden type+scope enforcement +87/-0

Add pytest coverage for UC1 forbidden type+scope enforcement

• Adds unit tests covering rejected combinations, suggestion text, allowed combinations, and non-conventional titles. Uses a minimal FakeCommit to exercise the rule without requiring full gitlint runtime context.

gitlint_rules_test.py

Documentation (1) +11 / -0
COMMITS.mdDocument forbidden type+scope pairs enforced by UC1 +11/-0

Document forbidden type+scope pairs enforced by UC1

• Adds a dedicated section listing disallowed combinations (feat/fix with ci/e2e). Includes rationale and concrete replacement guidance to keep release notes user-focused.

COMMITS.md

Other (2) +2 / -0
.gitlintLoad custom gitlint rules from gitlint_rules/ +1/-0

Load custom gitlint rules from gitlint_rules/

• Adds an extra-path entry so gitlint can discover repository-local custom rules. This wires in the new UC1 rule without changing existing contrib rule configuration.

.gitlint

MakefileRun gitlint rule tests as part of script-test +1/-0

Run gitlint rule tests as part of script-test

• Extends the script-test target to execute the new pytest suite for the custom gitlint rule. Ensures the commit-lint policy is continuously validated in CI.

Makefile

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

Site preview

Preview: https://1646afc9-site.fullsend-ai.workers.dev

Commit: 72aa637156bff48aa7a3ca882428a7c4dfb07e11

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 30, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:43 PM UTC · Completed 8:06 PM UTC
Commit: 0266479 · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Jun 30, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 51 rules

Grey Divider


Action required

1. Missing pytest in CI ✓ Resolved 🐞 Bug ☼ Reliability
Description
make script-test now runs python3 -m pytest gitlint_rules_test.py -v, but the CI workflow that
runs make script-test doesn’t install pytest (and the test imports gitlint), so the job is
likely to fail with missing-module errors.
Code

Makefile[132]

+	$(call run-timed,python3 -m pytest gitlint_rules_test.py -v)
Relevance

⭐⭐⭐ High

Team installs needed tooling in CI when required (e.g., pinact added to lint.yml in PR #2509).

PR-#2509

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The Makefile change makes pytest part of script-test, while the workflow that runs `make
script-test does not install pytest; additionally, the new tests import gitlint.rules`, which
requires gitlint/gitlint-core to be installed in that environment.

Makefile[120-133]
.github/workflows/lint.yml[23-33]
.github/workflows/lint.yml[45-55]
gitlint_rules_test.py[1-7]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`make script-test` invokes `python3 -m pytest gitlint_rules_test.py -v`, but the `lint.yml` workflow installs only `pre-commit` and `jsonschema` before running `make script-test`. This will likely fail because `pytest` is not installed, and `gitlint_rules_test.py` also imports `gitlint.rules`, which won’t be present unless `gitlint-core` is installed.

### Issue Context
The repo’s CI uses `uv pip install --system ...` for Python tooling in `.github/workflows/lint.yml`, and separately uses `uvx --from gitlint-core gitlint ...` in the `commit-lint` job. The new pytest-based unit test runs under the `test` job, not under `uvx`, so required packages must be installed in that job.

### Fix Focus Areas
- Makefile[120-133]
- .github/workflows/lint.yml[23-33]
- .github/workflows/lint.yml[45-55]
- gitlint_rules_test.py[1-7]

### Suggested fix
Either:
1) Add dependencies in `.github/workflows/lint.yml` (recommended):
  - Extend the install step to include `pytest` and `gitlint-core`:
    - `uv pip install --system pre-commit jsonschema pytest gitlint-core`

OR
2) Change the Makefile to run pytest via `uvx` so it self-provisions dependencies:
  - e.g. `uvx --from pytest --with gitlint-core pytest gitlint_rules_test.py -v`

Ensure the chosen approach works both locally and in CI.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Regex misses colon delimiter 🐞 Bug ≡ Correctness
Description
ForbiddenTypeScope’s regex matches type(scope) without requiring the documented : delimiter,
so UC1 can emit its “pollutes release notes” message on malformed/non-conventional titles that
happen to start with fix(ci)/feat(ci) etc.
Code

gitlint_rules/forbidden_type_scope.py[R13-15]

+# Pattern: type(scope): description
+_CONVENTIONAL = re.compile(r"^(?P<type>\w+)\((?P<scope>[^)]+)\)")
+
Relevance

⭐⭐ Medium

No repo history for new gitlint_rules path; regex-hardening is sometimes accepted (e.g., PR #2451,
#736).

PR-#2451
PR-#736

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rule’s own comment and the repo’s commit message format require type(scope): ..., but the
implemented regex stops at ) and does not enforce the : delimiter.

gitlint_rules/forbidden_type_scope.py[13-15]
COMMITS.md[5-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`_CONVENTIONAL` is documented as matching `type(scope): description`, but the actual regex only matches `type(scope)` and does not require `:` (or whitespace after it). This can cause UC1 to run (and emit a misleading message) even when the title does not follow the documented Conventional Commits format.

### Issue Context
The repo’s commit format documentation explicitly requires `:<space>` after the scope.

### Fix Focus Areas
- gitlint_rules/forbidden_type_scope.py[13-15]
- COMMITS.md[5-13]
- gitlint_rules_test.py[26-40]

### Suggested fix
Update the regex to require the delimiter, and optionally support breaking markers:
- e.g. `re.compile(r"^(?P<type>\w+)\((?P<scope>[^)]+)\)!?:\s")`

Add a test case demonstrating that malformed titles (e.g. `fix(ci) update workflow` without `:`) do not trigger UC1.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread Makefile
Comment on lines +13 to +15
# Pattern: type(scope): description
_CONVENTIONAL = re.compile(r"^(?P<type>\w+)\((?P<scope>[^)]+)\)")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

2. Regex misses colon delimiter 🐞 Bug ≡ Correctness

ForbiddenTypeScope’s regex matches type(scope) without requiring the documented : delimiter,
so UC1 can emit its “pollutes release notes” message on malformed/non-conventional titles that
happen to start with fix(ci)/feat(ci) etc.
Agent Prompt
### Issue description
`_CONVENTIONAL` is documented as matching `type(scope): description`, but the actual regex only matches `type(scope)` and does not require `:` (or whitespace after it). This can cause UC1 to run (and emit a misleading message) even when the title does not follow the documented Conventional Commits format.

### Issue Context
The repo’s commit format documentation explicitly requires `:<space>` after the scope.

### Fix Focus Areas
- gitlint_rules/forbidden_type_scope.py[13-15]
- COMMITS.md[5-13]
- gitlint_rules_test.py[26-40]

### Suggested fix
Update the regex to require the delimiter, and optionally support breaking markers:
- e.g. `re.compile(r"^(?P<type>\w+)\((?P<scope>[^)]+)\)!?:\s")`

Add a test case demonstrating that malformed titles (e.g. `fix(ci) update workflow` without `:`) do not trigger UC1.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] .github/workflows/lint.yml — This PR modifies .github/workflows/lint.yml, which is under the .github/ protected path. The PR has no linked issue providing authorization for modifying governance/infrastructure files. Human approval is always required for protected-path changes.
    Remediation: File an issue documenting the need to add pytest and gitlint-core to the CI lint workflow, and link it to this PR.

Labels: PR adds gitlint commit-message linting rules and modifies the CI lint workflow.

Previous run

Review

Findings

Medium

  • [test-framework-consistency] gitlint_rules_test.py:3 — This is the first test file in the repo to explicitly import and use pytest (@pytest.mark.parametrize) and the first Makefile entry to invoke python3 -m pytest. All other Python test files in the script-test target use unittest with if __name__ == "__main__": unittest.main() and are invoked directly as python3 <file>. This introduces a new test framework dependency and a second invocation pattern.

  • [file-organization] gitlint_rules_test.py:1 — Test file is at the repo root while the source module is gitlint_rules/forbidden_type_scope.py. All other test files are co-located with their source (e.g., skills/topissues/scripts/topissues_test.py, internal/security/hooks/canary_posttool_test.py). The filename gitlint_rules_test.py also does not follow the <module_name>_test.py convention — it should be forbidden_type_scope_test.py co-located in gitlint_rules/.

  • [missing-authorization] — This PR adds a new enforcement policy (custom gitlint rule directory, 4 forbidden type+scope pairs, test suite, COMMITS.md documentation) with no linked issue. While the change is self-contained and well-explained in the PR description, non-trivial policy additions benefit from an issue to document the problem being solved and gather feedback before implementation.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 30, 2026
@ralphbean ralphbean force-pushed the rbean/forbid-misleading-commit-scopes branch from dbc2516 to f45f60e Compare July 1, 2026 18:40
ralphbean added 2 commits July 1, 2026 14:41
Add a custom gitlint rule (UC1) that rejects feat(ci), fix(ci),
feat(e2e), and fix(e2e). These combinations pollute user-facing release
notes with infrastructure changes that mean nothing to end users.

The rule provides actionable error messages pointing to the correct
prefix, e.g. "Use ci(<subsystem>)" or "Use ci(e2e)".

Signed-off-by: rbean <rbean@redhat.com>
Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean ralphbean force-pushed the rbean/forbid-misleading-commit-scopes branch from f45f60e to 4ac4b12 Compare July 1, 2026 18:41
@ralphbean ralphbean enabled auto-merge July 1, 2026 18:43
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 6:44 PM UTC · Ended 6:51 PM UTC
Commit: 0123b0b · View workflow run →

The gitlint_rules_test.py added in the previous commit imports pytest
and gitlint.rules, but the CI workflow only installed pre-commit and
jsonschema. Add both packages to the uv pip install step.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean ralphbean force-pushed the rbean/forbid-misleading-commit-scopes branch from d4c4954 to 72aa637 Compare July 1, 2026 18:54
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 6:58 PM UTC · Ended 7:10 PM UTC
Commit: 0123b0b · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the review comment for full details.


- name: Install pre-commit and test dependencies
run: uv pip install --system pre-commit jsonschema
run: uv pip install --system pre-commit jsonschema pytest gitlint-core

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[high] protected-path

This PR modifies .github/workflows/lint.yml, which is under the .github/ protected path. The PR has no linked issue providing authorization for modifying governance/infrastructure files. Human approval is always required for protected-path changes.

Suggested fix: File an issue documenting the need to add pytest and gitlint-core to the CI lint workflow, and link it to this PR.

@fullsend-ai-review fullsend-ai-review Bot added component/ci CI pipelines and checks and removed requires-manual-review Review requires human judgment labels Jul 1, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:58 PM UTC · Completed 7:10 PM UTC
Commit: 72aa637 · View workflow run →

@ralphbean ralphbean added this pull request to the merge queue Jul 1, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 1, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Simple, well-tested rule that prevents misleading release note entries. CI is green. Bot findings are all non-blocking (protected-path is procedural with existing human approval, regex colon nit is mitigated by CT1 running first).

@waynesun09 waynesun09 added this pull request to the merge queue Jul 1, 2026
Merged via the queue into main with commit ad09b33 Jul 1, 2026
22 of 23 checks passed
@waynesun09 waynesun09 deleted the rbean/forbid-misleading-commit-scopes branch July 1, 2026 20:46
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 1, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:51 PM UTC · Completed 8:58 PM UTC
Commit: 72aa637 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2799ci(lint): forbid misleading type+scope combinations in commit messages

Timeline

Human author rbean opened PR #2799 adding a custom gitlint rule (ForbiddenTypeScope) to reject misleading conventional commit type+scope pairs (e.g., fix(ci) should be ci(...)). The PR touched 5 files with +151/-0 lines.

  1. qodo-code-review[bot] reviewed first, catching a real CI gap: the Makefile invoked pytest but CI didn't install pytest or gitlint-core. This was fixed in a follow-up commit.
  2. fullsend-ai-review[bot] ran twice:
    • Run 1 surfaced three medium-severity findings: test-framework-consistency (pytest vs repo's unittest convention), file-organization (test at repo root vs co-located), and missing-authorization (no linked issue). All were substantive, codebase-aware observations.
    • Run 2 produced a single high-severity protected-path finding for modifying .github/workflows/lint.yml, resulting in a CHANGES_REQUESTED verdict.
  3. Two humans approved: rh-hemartin (silent approval) and waynesun09 (explicit LGTM noting all bot findings are non-blocking). PR merged.

Assessment

The workflow went well overall. The review bot's findings were substantively correct — its observations about test framework consistency and file organization were genuinely useful codebase-convention checks. The main friction point was the CHANGES_REQUESTED verdict driven by the protected-path finding on a human-authored PR, which required human override.

Existing coverage

All improvement opportunities identified are already tracked by open issues:

  • Protected-path severity for human PRs#1551
  • Missing-authorization severity cap for human PRs with descriptive bodies#2200
  • CHANGES_REQUESTED for governance-only findings the fix agent cannot resolve#1068

No new proposals are warranted. The existing issue backlog comprehensively covers the improvement areas surfaced by this workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants