Skip to content

ci: drop branch-commit conventional check, keep PR-title gate - #1110

Merged
LeTuR merged 1 commit into
mainfrom
fix/ci-squash-coherence
Sep 12, 2026
Merged

LeTuR merged 1 commit into
mainfrom
fix/ci-squash-coherence

Conversation

@LeTuR

@LeTuR LeTuR commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Intent

The operator said: 'conventional commit check is still here, but it makes no sense since we are using squash and merge with PR title. Remove this check. Double check CI there might be some other similar coherence issues.'

The repository squash-merges only (verified: allow_squash_merge=true, merge/rebase false, squash_merge_commit_title=PR_TITLE), so the commit that lands on main is built from the pull request title, not from any branch commit. There were three enforcement points and only one of them is coherent with that:

  1. .github/workflows/pr-title.yml validates the PR title. KEEP - deliberately not removed. It is the only thing validating what cog bump --auto reads for the release decision and what the changelog quotes (verified in cd.yml: check-release runs cog bump --auto --dry-run on push to main, and generate-changelog runs cog changelog). Removing it would break versioning. It is configured as a required status check in the repo ruleset alongside 'All Checks' (verified).
  2. ci.yml's 'Check conventional commits' step running scripts/ci/check-conventional-commits.sh validated INDIVIDUAL BRANCH COMMITS, which the squash discards. THIS IS WHAT WAS REMOVED, along with the script, its check-conventional-commits.bats suite, and the .pre-commit-config.yaml 'cocogitto-check' pre-push hook that ran the same script (it had to go: the script it invoked no longer exists). With that hook gone the pre-push stage is empty, so pre-push was dropped from default_install_hook_types too.
  3. .pre-commit-config.yaml's 'cocogitto-verify' commit-msg hook (local cog verify) was deliberately LEFT IN PLACE. That is the operator's judgement call, not mine; it is being put to them in the task's result.md rather than decided silently. It keeps local history legible and costs nothing in CI.

The 'conventional-commits' CI job was not deleted outright because it also ran the PR-title checker's bats suite, which lives in ci.yml on purpose (pr-title.yml re-runs on every edit of the title box and stays lean). The job was renamed to 'pr-title-checker' / 'PR Title Checker', reduced to that one suite, and gated on a new 'pr_title' paths filter instead of running unconditionally on every PR - matching how every other job in that file is gated by the 'changes' job. all-checks' needs list was updated accordingly. 'Conventional Commits' was not itself a required status check, so the rename is safe.

Separately and in the same pass, the task asked for a fix to a live contributor problem: two open fork PRs (#1107 fix(program), #1108 fix(clipboard)) both fail the PR Title check with 'Commit scope X not allowed', and cog verify names only the offending token, never the allowed set, so a first-time contributor cannot correct the title without finding and reading cog.toml. scripts/ci/check-pr-title.sh now quotes cog.toml's declared commit types and scopes back in the failure message. Deliberately NOT widening cog.toml's scope allowlist - the task says to propose that to the operator rather than decide it, and the better-for-everyone fix is the self-describing error message. Tests were written first (two new bats cases asserting the message names the allowed types and scopes), confirmed failing, then the script changed.

Docs invalidated by the removal were updated in the same commit: CONTRIBUTING.md, CLAUDE.md (hook count 19 -> 18, pre-push stage note, Conventional Commits section rewritten around the squash reality), docs/CONSTITUTION.md (principle 6 and the enforcement map), docs/DEVELOPMENT.md (just test-scripts row), justfile (test-scripts recipe), and the stale comment in .no-mistakes.yaml that referenced cog check in pre-push and CI.

Constraint held: this removes checks that guard nothing, it does not lower the bar on the commit that ships. Nothing that validates main was weakened.

What Changed

  • Removed ci.yml's "Check conventional commits" step along with scripts/ci/check-conventional-commits.sh/.bats, since they validated individual branch commits that squash-merge discards; also dropped the now-dead cocogitto-check pre-push hook from .pre-commit-config.yaml and pruned pre-push from default_install_hook_types (the stage is now empty), leaving cocogitto-verify (commit-msg) in place.
  • Renamed the CI job from "Conventional Commits" to "PR Title Checker", narrowed it to running check-pr-title.bats, gated it on a new pr_title paths filter consistent with the other changes-gated jobs, and updated all-checks' needs list accordingly.
  • Made check-pr-title.sh quote cog.toml's declared commit types and scopes back in its failure message (with new bats coverage) so a rejected PR title names the allowed values directly instead of requiring a contributor to read cog.toml.
  • Updated CONTRIBUTING.md, CLAUDE.md, docs/CONSTITUTION.md, docs/DEVELOPMENT.md, justfile, and the stale comment in .no-mistakes.yaml to match the removal (hook count, pre-push stage, Conventional Commits section/enforcement map, test-scripts recipe/row).

Risk Assessment

✅ Low: The change removes a check that validated branch commits the squash discards, leaves the actually-load-bearing PR-title check and the local commit-msg hook untouched, and verified GitHub repo settings (squash-only, PR_TITLE, required checks are exactly "All Checks" and "PR Title") confirm the rename of the CI job is safe and no required gate weakened.

Testing

Baseline cargo nextest run --all had already passed. For this targeted pass I ran the actual executable bats suite scripts/ci/check-pr-title.bats (the real consumer of the PR-title checker) against the target commit's script — all 13 cases pass, including the two new cases asserting the failure message names cog.toml's allowed commit types and scopes. To prove these two are genuine regression tests and not vacuously true, I re-ran the same new bats file against the base commit's (pre-fix) check-pr-title.sh in an isolated /tmp copy: both new cases failed there as expected, confirming the fix is real and observable through the tool's actual CLI behavior/output, not just source inspection. I also diffed ci.yml, pre-commit-config.yaml, .no-mistakes.yaml, and pr-title.yml between base and target to confirm every enforcement-point change (job rename/gating, all-checks needs list, hook removal, pr-title.yml left untouched) matches the stated intent, and grepped the repo to confirm no dangling references to the removed check-conventional-commits script remain. No source or test changes were made during this test phase; temporary files created in /tmp during verification were removed, and the worktree is clean.

Evidence: bats scripts/ci/check-pr-title.bats (target commit, all 13 pass including 2 new error-message cases)
1..13
ok 1 accepts a conventional title
ok 2 validates the title in the form squash merge lands, suffix included
ok 3 rejects a title that is not a conventional commit
ok 4 rejects a commit type cog.toml does not declare
ok 5 rejects a scope outside the allowlist
ok 6 a rejected scope's message names the scopes cog.toml allows
ok 7 a rejected type's message names the types cog.toml allows
ok 8 accepts a breaking change declared in the title
ok 9 rejects a title that already ends in its own (#N)
ok 10 accepts a title citing another pull request mid-sentence
ok 11 rejects an empty title
ok 12 reports a usage error when the pr number is missing
ok 13 works in a checkout that has configured no git identity
Evidence: Same new bats suite run against the OLD (base commit) checker script — regression proof: the two new cases fail pre-fix
not ok 6 a rejected scope's message names the scopes cog.toml allows
# `[[ "$output" == *"allowed scopes"* ]]' failed
not ok 7 a rejected type's message names the types cog.toml allows
# `[[ "$output" == *"allowed types"* ]]' failed

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • cargo nextest run --all
  • bats scripts/ci/check-pr-title.bats (target commit) — 13/13 pass
  • bats /tmp/old-checker-test/check-pr-title.bats against base-commit check-pr-title.sh — confirms the 2 new cases fail pre-fix (regression proof)
  • git diff 0c588f3..74e3e86 review of .github/workflows/ci.yml, .pre-commit-config.yaml, .no-mistakes.yaml, justfile, .github/workflows/pr-title.yml
  • grep -rn check-conventional-commits across yml/yaml/md/justfile — no stale references
  • ls scripts/ci/ — confirmed check-conventional-commits.sh and .bats are deleted
  • grep cocogitto-verify .pre-commit-config.yaml — confirmed commit-msg hook intentionally retained
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…less

Under squash merge the commit that lands on main is built from the pull
request title, not from any commit on the branch. CI's "Conventional
Commits" job and the `pre-push` hook both walked the branch commits the
squash discards, so they cost a contributor a rejected push for a message
nobody will ever read.

Remove `scripts/ci/check-conventional-commits.sh`, its bats suite, the CI
step and the `cocogitto-check` hook. What replaces them was already there:
`pr-title.yml`'s required `PR Title` check validates the string that
actually lands, which is what `cog bump --auto` reads for the release
decision and what the changelog quotes.

The title checker's bats suite stays in `ci.yml` (pr-title.yml re-runs on
every edit of the title box and stays lean), now in a `PR Title Checker`
job gated on a `pr_title` paths filter rather than running unconditionally.

The checker also now quotes cog.toml's declared types and scopes back when
it rejects a title: `cog verify` names only the token it disliked, so a
first-time contributor had to find and read cog.toml to correct it.

Claude-Session: https://claude.ai/code/session_01VphvgTExT9vE7nd2DxTgyi
@sonarqubecloud

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Sep 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR removes branch-commit validation that does not affect squash-merged history while retaining the PR-title gate that protects release metadata.

  • Replaces the branch-commit CI job with a path-filtered test job for the PR-title checker.
  • Removes the obsolete pre-push hook, checker script, and associated tests.
  • Improves rejected-title diagnostics by printing the configured type and scope allowlists.
  • Updates contributor, development, and governance documentation to reflect the squash-merge workflow.

Confidence Score: 5/5

The PR appears safe to merge; the release-relevant PR-title validation remains intact and the removed branch-history checks have no effect on squash-merged commits.

No actionable failures remain: checker inputs are fully covered by the new CI filter, title enforcement remains independently active, the diagnostic parser matches the repository’s actual configuration, and deleted checker references were consistently removed.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Replaces branch-history validation with a correctly filtered PR-title checker test job and updates the aggregate dependency.
scripts/ci/check-pr-title.sh Adds failure diagnostics that extract and display the repository’s configured commit types and scopes.
scripts/ci/check-pr-title.bats Adds focused coverage confirming rejected titles report the allowed types and scopes.
.pre-commit-config.yaml Removes the obsolete pre-push history checker while retaining local commit-message validation.
CONTRIBUTING.md Documents that the squash commit derives from the PR title and that branch-message validation is local-only.
docs/CONSTITUTION.md Updates the conventional-commit principle and enforcement map to identify the required PR-title check.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Pull request] --> B[Required PR Title workflow]
    B --> C[Validate title plus GitHub suffix]
    C --> D[Squash merge]
    D --> E[Commit on main]
    E --> F[cog bump --auto]
    E --> G[cog changelog]

    H[Checker or cog.toml change] --> I[CI paths filter]
    I --> J[PR Title Checker bats suite]
    J --> K[All Checks]
Loading

Reviews (1): Last reviewed commit: "ci: drop the per-commit conventional che..." | Re-trigger Greptile

@LeTuR
LeTuR merged commit 6b397c7 into main Sep 12, 2026
23 checks passed
@LeTuR
LeTuR deleted the fix/ci-squash-coherence branch September 12, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant