Skip to content

ci: run pre-commit on pull requests, and repair the hooks it runs - #22

Merged
slin1237 merged 3 commits into
mainfrom
chore/pre-commit-ci
Aug 26, 2026
Merged

ci: run pre-commit on pull requests, and repair the hooks it runs#22
slin1237 merged 3 commits into
mainfrom
chore/pre-commit-ci

Conversation

@hello-alexmcc

@hello-alexmcc hello-alexmcc commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #21. While checking that PR's CI I found that only PR Title Check ran — because it is the only workflow in the repo. Digging in, the pre-commit hook set has never worked at all.

Two bugs in .pre-commit-config.yaml

1. The config has been invalid YAML since the day it was added (017ebcc, 2026-03-02 — its only commit).

branch-name-check's unquoted entry: contains ": " inside its own error message, which YAML parses as a nested mapping:

$ pre-commit run --all-files
An error has occurred: InvalidConfigError:
==> File .pre-commit-config.yaml
=====> mapping values are not allowed in this context
  in "<unicode string>", line 28, column 177

So pre-commit install and pre-commit run have always aborted, and none of the eight hooks has ever executed for anyone — not trailing-whitespace, end-of-file-fixer, check-yaml, check-merge-conflict, check-added-large-files, codespell, dco-check, nor branch-name-check. Rewritten as a folded block scalar.

2. dco-check validated the wrong commit. It ran git log -1, which at commit-msg stage is the parent commit — the one being written doesn't exist yet. It passed whenever the previous commit happened to carry a sign-off, regardless of the current one. Demonstrated before the fix:

$ pre-commit run dco-check --hook-stage commit-msg --commit-msg-filename msg_without_signoff
DCO sign-off check.......................................................Passed   # wrong

It now greps the commit-msg file pre-commit passes in ("$1") and prints an actionable hint. After the fix, that same message fails and a signed one passes.

3. codespell gains -L wit,WIT. With the config repaired, codespell fires for the first time and flags 14 occurrences of WIT/wit — the WebAssembly Interface Types format name, used throughout wasm-plugin.md, map/SKILL.md and README.md. smg's own codespell args already carry wit,WIT for exactly this reason, so this mirrors the sibling repo rather than inventing an exception.

New workflow

.github/workflows/pr-checks.yml, three jobs. pr-title-check.yml is untouched.

Job What it does
pre-commit pre-commit run --all-files --show-diff-on-failure, skipping the two local git-stage hooks that cannot run over a checkout. Mirrors smg's pre-commit job.
commit-messages DCO sign-off on every commit in the PR range, plus the forbidden AI co-author/sign-off check — ported from smg's pr-naming-check.yml.
branch-name Enforces the same prefixes branch-name-check allows. Internal PRs only; forks exempt, as in smg.

Verification

Run locally with pre-commit 4.6.2 against a repaired config:

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for merge conflicts................................................Passed
check for added large files..............................................Passed
codespell................................................................Passed

Both repaired hooks were tested in both directions: branch-name-check passes on chore/* and fails with the correct message on a bad name; dco-check passes a signed message and fails an unsigned one. The workflow's DCO loop and attribution grep were run against real commit ranges, including a deliberately unsigned commit, which the loop correctly rejected.

One thing for you to decide

There is a pre-existing mismatch this PR mirrors rather than resolves: pr-title-check.yml accepts the title type ci:, but the branch-prefix list (setup|feat|fix|docs|chore|refactor) has no ci/*. This PR is therefore on chore/pre-commit-ci with a ci: title. If you would rather allow ci/* branches, that is a one-line addition to both .pre-commit-config.yaml and the new branch-name job — say the word and I will add it.


Follow-up commit (9eca55d): the first CI run of this very workflow failed its own DCO job — the only "unsigned commit" it found was e6d3561 Merge e357bb5 into e119304, GitHub's generated merge commit, which actions/checkout checks out on pull_request events and which carries no sign-off. Both commit-range walks now pass --no-merges, matching the usual DCO convention of exempting merges. All four checks green after the fix.


Why PRs were showing no CI at all (a31c4a8)

The deeper cause of "CI never runs": the main ruleset ("PRs & conventional commits", active, no bypass actors) requires three status checks that nothing in this repo produced, so every PR sat BLOCKED waiting on contexts that would never report:

Required context Reported by Reality before this PR
finish GitHub Actions no such job existed
Conventional Commit Title GitHub Actions the job reported as check-title
DCO DCO app (integration 1861) app not enabled on this repo

All three were copied from smg, where they are real (finish at pr-test-rust.yml:1192, PR Title & Commit Messages at pr-naming-check.yml:59, plus the DCO app). The guide's copy hand-edited one name and never created matching jobs. Nobody hit it because the last merge was #20 on 2026-07-15 and the ruleset was tightened on 2026-07-31 — #21 and #22 are the first PRs since.

Fixed on the runner side rather than by weakening the ruleset:

  • pr-title-check.yml — job now carries name: Conventional Commit Title, so its check run reports under the required context.
  • pr-checks.yml — new aggregate finish job (needs all three PR jobs, if: always()) that fails if any failed or was cancelled. skipped is allowed, since commit-messages and branch-name do not run on push and branch-name is skipped for fork PRs.

finish and Conventional Commit Title now report and pass. DCO still needs an org owner to add this repo (1170922059) to the existing smg-project dco installation (150406607) — it cannot be satisfied from Actions, because the ruleset pins that context to integration 1861.

Nothing in CI ran the pre-commit hook set, so the eight hooks in
.pre-commit-config.yaml only fired for contributors who had run
`pre-commit install` locally. Two of them could never have fired for
anyone.

Fixes to .pre-commit-config.yaml:

- The config has been invalid YAML since it was added (017ebcc,
  2026-03-02, its only commit): branch-name-check's unquoted `entry:`
  contains ": " inside its error message, which YAML reads as a nested
  mapping. `pre-commit run` aborts with InvalidConfigError, so every
  hook in the file — trailing-whitespace, end-of-file-fixer,
  check-yaml, check-merge-conflict, check-added-large-files, codespell,
  dco-check, branch-name-check — has never executed. Rewritten as a
  folded block scalar.

- dco-check inspected `git log -1`, which at commit-msg stage is the
  PARENT commit, not the message being written. It therefore passed
  whenever the previous commit happened to be signed off, regardless of
  the current one. It now greps the commit-msg file pre-commit passes
  in ("$1") and prints a usable hint on failure.

- codespell gains `-L wit,WIT`. With the config repaired, codespell
  flags 14 occurrences of WIT/wit, the WebAssembly Interface Types
  format named throughout wasm-plugin.md, map/SKILL.md and README.md.
  smg's own codespell args already carry `wit,WIT` for this reason.

New workflow .github/workflows/pr-checks.yml, three jobs:

- pre-commit — installs pre-commit and runs `--all-files
  --show-diff-on-failure`, skipping the two local git-stage hooks that
  cannot run over a checkout. Mirrors smg's pre-commit job.
- commit-messages — DCO sign-off on every commit in the PR range, and
  the forbidden AI co-author/sign-off check, ported from smg's
  pr-naming-check.yml.
- branch-name — enforces the same prefixes branch-name-check allows,
  on internal PRs only (forks exempt).

Existing pr-title-check.yml is untouched.

Verified locally with pre-commit 4.6.2: all six checkout-stage hooks
pass; branch-name-check passes on chore/* and fails with the right
message on a bad name; dco-check now passes a signed message and fails
an unsigned one; the DCO loop and attribution grep were run against
real commit ranges in both directions.

Signed-off-by: Alex McC <319643551+hello-alexmcc@users.noreply.github.com>
On pull_request events actions/checkout checks out GitHub's generated
merge commit (refs/pull/N/merge), which carries no Signed-off-by and
never will. The DCO walk flagged it and failed every PR. Both commit
range walks now pass --no-merges, which also matches the usual DCO
convention of exempting merge commits.

Signed-off-by: Alex McC <319643551+hello-alexmcc@users.noreply.github.com>
@slin1237 slin1237 closed this Aug 26, 2026
@slin1237 slin1237 reopened this Aug 26, 2026
The main branch ruleset requires three status checks that nothing in
this repo produced, so every PR sat BLOCKED with all its checks green,
waiting on contexts that would never report:

  finish                     (GitHub Actions)  -- no such job
  Conventional Commit Title  (GitHub Actions)  -- job reported as check-title
  DCO                        (DCO app, 1861)   -- app not enabled on this repo

The contexts were copied from smg, where all three are real. This fixes
the two an Actions workflow can satisfy, rather than editing the
ruleset:

- pr-title-check.yml: the job now carries name: Conventional Commit
  Title, so its check run reports under the required context.
- pr-checks.yml: new aggregate finish job (needs all three PR jobs,
  if: always()) that fails when any of them failed or was cancelled.
  skipped is allowed, since commit-messages and branch-name do not run
  on push and branch-name is skipped for fork PRs.

The DCO context needs an org owner to add this repo to the existing
smg-project dco app installation (150406607); it cannot be satisfied
from Actions because the ruleset pins it to integration 1861.

Signed-off-by: Alex McC <319643551+hello-alexmcc@users.noreply.github.com>
@slin1237
slin1237 merged commit 08f1fb3 into main Aug 26, 2026
6 checks passed
@slin1237
slin1237 deleted the chore/pre-commit-ci branch August 26, 2026 14:29
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.

2 participants