chore: validate Dependabot commits instead of skipping them - #22
Conversation
|
|
There was a problem hiding this comment.
Pins Dependabot's commit prefix and removes the commitlint exemption for its commits — the right end-state, but the precondition the PR relies on (a commitlint version whose ensure() exempts URL lines from body-max-line-length) is not actually wired into this repo's CI. .github/workflows/ci.yaml still runs wagoid/commitlint-github-action@v6.2.1, which pins @commitlint/ensure ^19.0.3 — no URL exemption. The PR's own comparison target, yo61/agent-team-topologies, gets its green Dependabot PR by running yo61/commitlint-github-action@v6.3.0 (ensure ^21.2.0) instead; that action is not adopted here. As written, this will make Dependabot's own bump PRs fail commitlint the next time a changelog/compare-link line in the body exceeds 100 chars — which is routine (e.g. this repo's own recent #18/#19 zizmor-pre-commit bumps).
| // Dependabot auto-generates commit bodies with long markdown URLs that exceed | ||
| // body-max-line-length. It won't wrap them, so skip linting its commits while | ||
| // keeping the full ruleset enforced for human authors. | ||
| ignores: [(message) => message.includes("Signed-off-by: dependabot[bot]")], |
There was a problem hiding this comment.
[Critical] Exemption removed without switching CI to a commitlint that has the URL exemption
This repo's .github/workflows/ci.yaml still runs wagoid/commitlint-github-action@b948419 (v6.2.1), whose package.json pins @commitlint/ensure ^19.0.3 — that's the version the PR says predates the URL-line exemption. @commitlint/config-conventional also enables body-max-line-length at 100 by default (the earlier 200-char override was already dropped in f1bddcd), so it's even stricter than the pre-#16 baseline. Removing the ignores here without also pointing CI at yo61/commitlint-github-action (as yo61/agent-team-topologies does, cited in the PR body as the live proof) means the next Dependabot PR whose body has a long changelog/compare URL — a routine occurrence, as seen in this repo's own #18/#19 — will fail commitlint in CI. The verification in the PR description was run against a different action/repo than the one this diff actually ships to. The fix belongs in .github/workflows/ci.yaml — swap the wagoid/commitlint-github-action step for yo61/commitlint-github-action@v6.3.0 (or otherwise get ensure>=20 into this repo's CI) in the same commit that drops this exemption.
commitlint.config.mjs ignored any commit containing "Signed-off-by: dependabot[bot]", so Dependabot's commit subjects were never checked at all. The exemption existed because Dependabot writes long unwrapped markdown URLs in commit bodies, which tripped body-max-line-length. @commitlint/ensure v20 added an exemption for lines containing a URL, and CI now runs v21 via yo61/commitlint-github-action, so those bodies pass on their own merits. Verified against a real Dependabot commit carrying a 314-character body line: it passes, and a malformed commit is still rejected. Pin Dependabot's commit prefix explicitly rather than relying on its auto-detection, so subjects are conventional by construction now that they are actually checked.
The previously pinned fork commit resolved its runtime image through a mutable registry tag, so pinning the action by SHA still left the image that actually runs swappable. The fork now refers to the image by digest, and proves the reference resolves by running the action from the published image in its own CI.
5938661 to
33ab896
Compare
There was a problem hiding this comment.
Re-review. Prior finding: the commitlint exemption was removed without CI actually running a commitlint that exempts URL lines from body-max-line-length — fixed. origin/main now already carries yo61/commitlint-github-action (merged separately in #20/#21 before this branch rebased), whose package.json pins @commitlint/ensure ^21.2.0; I fetched that fork's package.json and confirmed the version. This PR's own commit re-pins that action to ghcr.io/yo61/commitlint-github-action@sha256:78d4769..., which I verified against the fork's action.yml at that exact ref — it matches, and closes the tag/SHA-swap gap the commit message describes. So the precondition my last review blocked on is now genuinely in place, and the exemption removal in commitlint.config.mjs is safe. One new gap: the Dependabot commit-message prefix is pinned only for the pre-commit ecosystem, leaving uv and github-actions still dependent on auto-detection for the same guarantee this PR is built around.
| # its commits satisfy commitlint now that they are no longer ignored. | ||
| commit-message: | ||
| prefix: "chore" | ||
| include: "scope" |
There was a problem hiding this comment.
[Important] Prefix pin covers only the pre-commit ecosystem
The commit-message.prefix override is added only to the pre-commit block. uv (line ~19) and github-actions (line ~30) still have no commit-message config, so their first bump PRs still rely on Dependabot's auto-detected prefix — the exact thing this PR's own comment says not to rely on now that commitlint is enforced on every Dependabot commit. Both ecosystems are new (added in #21) with no bump commits yet, so auto-detection has no ecosystem-specific precedent to key off; if it falls back to an unprefixed subject ("Bump foo from 1 to 2"), that fails commitlint's type-enum/subject-empty rules and the first uv/actions Dependabot PR breaks CI. Add the same commit-message: {prefix: ..., include: scope} to the uv and github-actions blocks (e.g. chore/ci respectively) rather than leaving two of three ecosystems on auto-detection.
|
@LastLight review |
Only the pre-commit ecosystem had commit-message.prefix pinned. The uv and github-actions blocks were added later and kept Dependabot's auto-detection, so with the blanket commitlint exemption gone their first bump could land a subject that fails CI. Pin the prefix on every ecosystem so the guarantee holds for all of them, not just the one that happened to exist when the exemption was removed.
There was a problem hiding this comment.
Re-review. Prior findings ledger: (1) exemption removed without CI running a commitlint that exempts URL lines — fixed in an earlier push (33ab896), verified again here: .github/workflows/ci.yaml pins yo61/commitlint-github-action@501ff7b… (v6.3.0, digest-pinned), which carries @commitlint/ensure ^21.2.0. (2) prefix pin covered only the pre-commit ecosystem, leaving uv and github-actions on auto-detection — fixed by this commit: both blocks now carry commit-message: {prefix: chore, include: scope}, matching the pre-commit block and this repo's own historical Dependabot subject style (chore(deps): bump …). All three Dependabot ecosystems now produce a pinned, commitlint-conformant subject, and the exemption removal in commitlint.config.mjs no longer relies on auto-detection anywhere. No new findings.
What
Stops exempting Dependabot from commitlint, and pins Dependabot's commit prefix so its subjects are conventional by construction.
Why
commitlint.config.mjsignored any commit containingSigned-off-by: dependabot[bot], which every Dependabot commit carries. Its subjects were therefore never validated — including thechore(deps): …prefix that release tooling reads.The exemption was there for a real reason: Dependabot writes long unwrapped markdown URLs in commit bodies, which tripped
body-max-line-length. That reason no longer holds.@commitlint/ensurev20 added an exemption for lines containing a URL:CI now runs commitlint v21 via
yo61/commitlint-github-action, so those bodies pass on their own merits.Order of operations
The prefix is pinned in the same commit that removes the exemption, so there is no window where Dependabot commits are validated while their subject format still depends on auto-detection.
Verification
Replayed the action's own load+lint path against this repo's
commitlint.config.mjswith the exemption removed, using a real Dependabot commit whose body has a 314-character line:Live proof:
yo61/agent-team-topologieshas never had this exemption, and its Dependabot PR #7 is green under the same action.