ci: use a maintained commitlint action fork - #20
Conversation
There was a problem hiding this comment.
Replaces the wagoid commitlint action with a directly pinned, version-controlled npx invocation. The mechanics (SHA-pinned setup-node, exact commitlint package versions, using env vars for BASE_SHA/HEAD_SHA to avoid template-injection in the run script, and keeping subject validation for Dependabot) are all sound, and CI is green. However, testing the stated root cause against the repo's own history contradicts it, which matters because that rationale is now committed as a comment in the workflow file.
| # Run commitlint directly rather than via wagoid/commitlint-github-action: | ||
| # that action pins @commitlint/ensure ^19, which predates the long-line | ||
| # exemption for URLs added in v20. Dependabot commit bodies are markdown | ||
| # link lists well over 100 chars, so v19 fails them on body-max-line-length |
There was a problem hiding this comment.
[Important] Stated root cause doesn't reproduce against the commit that actually failed CI
Ran @commitlint/cli@19.8.1 + @commitlint/config-conventional@19.8.1 directly (no ensure-v20 URL exemption) against ed4e2552 — the exact commit whose CI run (26260333092) failed on body-max-line-length via wagoid/commitlint-github-action@v6.2.1 — and it passes cleanly (found 0 problems, 0 warnings), because commitlint.config.mjs's ignores clause (added in #16) already matches on Signed-off-by: dependabot[bot] and skips the message. So a plain v19 CLI run doesn't reproduce the failure the PR attributes to @commitlint/ensure v19 lacking the URL exemption — the exemption isn't what makes these commits pass locally, the ignores clause is, and it already works under v19. The actual difference is something specific to how wagoid/commitlint-github-action invokes/loads the config (or passes the commit message) versus a direct CLI run — not the @commitlint/ensure version. The fix (running commitlint directly) likely still resolves the practical CI failure, but the comment block above bakes an unverified diagnosis into the codebase; a future maintainer chasing a similar failure would be pointed at bumping @commitlint/ensure when that isn't the mechanism.
|
|
There was a problem hiding this comment.
Re-review. Prior findings ledger: the [Important] root-cause finding (the workflow comment attributes the CI/pre-commit divergence to @commitlint/ensure v19 vs v20's URL exemption, but the actual divergence is the ignores clause added in #16 matching under a plain v19 CLI run) is still open — the comment at ci.yaml:81-87 is unchanged in the latest commit. New: the latest commit (542f1e7, "ci: pin commitlint in a tracked manifest") is failing CI on the commitlint job itself, and the failure reproduces locally — module resolution breaks because npm ci installs into .github/commitlint/node_modules but the lint step invokes the binary from repo-root cwd, where @commitlint/config-conventional isn't resolvable.
| set -euo pipefail | ||
| ./.github/commitlint/node_modules/.bin/commitlint \ | ||
| --config commitlint.config.mjs \ | ||
| --from "$BASE_SHA" --to "$HEAD_SHA" --verbose |
There was a problem hiding this comment.
[Critical] commitlint job fails: config-conventional isn't resolvable from the run step's cwd
npm ci (line 88-90) installs into .github/commitlint/node_modules, but the "Lint commit messages" step runs with cwd at the repo root and only prefixes the binary path (./.github/commitlint/node_modules/.bin/commitlint) — Node's module resolution for --config commitlint.config.mjs's extends: ["@commitlint/config-conventional"] still walks up from the repo root, which has no node_modules, and throws Cannot find module "@commitlint/config-conventional". Reproduced locally with the exact same install + invocation and it fails identically. This is exactly the failure on the current head SHA (542f1e7, CI run 31493104934, job 93784106014, conclusion failure) — the PR's fix is currently broken for every PR, including this one.
| --from "$BASE_SHA" --to "$HEAD_SHA" --verbose | |
| - name: Lint commit messages | |
| working-directory: .github/commitlint | |
| env: | |
| BASE_SHA: ${{ github.event.pull_request.base.sha }} | |
| HEAD_SHA: ${{ github.event.pull_request.head.sha }} | |
| run: | | |
| set -euo pipefail | |
| ./node_modules/.bin/commitlint \ | |
| --config ../../commitlint.config.mjs \ | |
| --from "$BASE_SHA" --to "$HEAD_SHA" --verbose |
| node-version: 22 | ||
| cache: npm | ||
| cache-dependency-path: .github/commitlint/package-lock.json | ||
| # Commitlint is pinned in .github/commitlint rather than run via |
There was a problem hiding this comment.
[Important] Comment still bakes in the unverified root-cause diagnosis
Still open from the prior review: running @commitlint/cli@19.8.1 + @commitlint/config-conventional@19.8.1 directly against ed4e255 (the commit whose CI run actually failed under wagoid v6.2.1) passes cleanly, because the config's ignores clause (added in #16) matches Signed-off-by: dependabot[bot] and skips the message entirely — a plain v19 CLI run never reaches body-max-line-length. The @commitlint/ensure v19-vs-v20 URL exemption isn't what makes local/pre-commit runs pass; the ignores clause is, and that already works under v19. This comment (lines 81-87) commits that unverified claim to the codebase; a future maintainer chasing a similar failure will look at bumping @commitlint/ensure when that isn't the mechanism at play.
542f1e7 to
aa07bb7
Compare
aa07bb7 to
595e08e
Compare
There was a problem hiding this comment.
Re-review. Prior findings ledger: [Critical] commitlint job failing because npm-installed deps weren't resolvable from the run step's cwd — fixed, the fix was withdrawn entirely: this commit replaces the whole custom npm/.github/commitlint invocation with a direct switch to a maintained action fork (yo61/commitlint-github-action@v6.3.0), so the module-resolution mechanism no longer exists. [Important] the workflow comment bakes in an unverified root-cause claim (attributing the CI/pre-commit divergence to @commitlint/ensure v19 lacking a URL exemption added in v20) — still open: the new comment at ci.yaml:76-80 restates the same claim verbatim, and it's still contradicted by the repo's own history. The action switch itself is a reasonable, low-risk fix (SHA-pinned, CI is green across all 8 checks) but the comment gets the mechanism wrong.
| # @commitlint/ensure ^19, which predates the exemption for long lines | ||
| # containing a URL added in v20. Dependabot bodies are markdown link lists | ||
| # well over 100 chars, so ^19 fails them on body-max-line-length while the | ||
| # pre-commit hook (v21) passes them. |
There was a problem hiding this comment.
[Important] Comment still attributes the fix to the wrong root cause
This comment (lines 76-80) claims Dependabot bodies fail body-max-line-length under @commitlint/ensure v19 because v19 lacks v20's URL-length exemption. That doesn't hold against the repo's own history: commitlint.config.mjs's ignores clause (added in #16) matches any commit containing Signed-off-by: dependabot[bot] — which every Dependabot commit has (e.g. 4955c03, ed4e255) — and skips linting it entirely, regardless of @commitlint/ensure version. Running @commitlint/cli@19.8.1 directly against ed4e2552 (the commit whose CI run actually failed under wagoid/commitlint-github-action@v6.2.1) passes cleanly for exactly this reason, never reaching body-max-line-length. The actual divergence is something specific to how wagoid/commitlint-github-action loads/applies the .mjs config (or the message it hands to ignores) versus a direct CLI/pre-commit run — not the @commitlint/ensure version. Switching to the fork is a reasonable fix regardless, but this comment commits the wrong diagnosis to the codebase; a future maintainer chasing a similar failure will look at bumping @commitlint/ensure when that isn't the mechanism.
Stale: this reviewed 542f1e7, the tracked-manifest approach that was abandoned. The branch was rebuilt on main and now contains only the one-line action swap.
595e08e to
91c7ec7
Compare
There was a problem hiding this comment.
Same net change reviewed three times before under an earlier SHA (595e08e) with an empty diff between that commit and the current head (91c7ec7) — this is a re-review of identical content, not new work. Prior findings ledger: [Critical] commitlint job failing because npm-installed deps weren't resolvable from the run step's cwd — fixed/withdrawn, that whole custom npm invocation is gone, replaced by this action-fork switch, so the module-resolution mechanism no longer exists. [Important] the workflow comment (lines 76-80) bakes in an unverified root-cause claim attributing the CI/pre-commit divergence to @commitlint/ensure v19 lacking v20's URL exemption — still open, re-verified directly against the repo's own commitlint.config.mjs: its ignores clause (added in #16) matches any commit containing Signed-off-by: dependabot[bot], which every Dependabot commit carries (e.g. 4955c03), so those commits are skipped entirely regardless of @commitlint/ensure version — a plain v19 run never reaches body-max-line-length for them. The actual divergence is something about how wagoid/commitlint-github-action loads/applies the .mjs config or the message text it hands to ignores, not the @commitlint/ensure version. Switching to the maintained fork is still a reasonable, low-risk fix (SHA-pinned, CI green across all 8 checks) — the objection is only to the diagnosis text now permanently committed to the workflow file.
| # Fork of wagoid/commitlint-github-action: upstream is unmaintained and pins | ||
| # @commitlint/ensure ^19, which predates the exemption for long lines | ||
| # containing a URL added in v20. Dependabot bodies are markdown link lists | ||
| # well over 100 chars, so ^19 fails them on body-max-line-length while the | ||
| # pre-commit hook (v21) passes them. |
There was a problem hiding this comment.
[Important] Comment commits an unverified root-cause diagnosis
This comment claims Dependabot bodies fail body-max-line-length under @commitlint/ensure v19 because v19 lacks v20's URL-length exemption. That doesn't hold against the repo's own config: commitlint.config.mjs's ignores clause (added in #16) matches any commit containing Signed-off-by: dependabot[bot] — which every Dependabot commit has — and skips linting it entirely, regardless of @commitlint/ensure version. Running @commitlint/cli@19.8.1 directly against a Dependabot commit that actually failed under wagoid/commitlint-github-action@v6.2.1 passes cleanly for exactly this reason, never reaching body-max-line-length. The real divergence is something specific to how the wagoid action loads/applies the .mjs config (or the message text it passes to ignores) versus a direct CLI/pre-commit run — not the @commitlint/ensure version. A future maintainer chasing a similar failure will be pointed at bumping @commitlint/ensure when that isn't the mechanism at play.
91c7ec7 to
1fdac9c
Compare
There was a problem hiding this comment.
Re-review of the same net change (only the workflow comment text changed since the last review at 91c7ec7). Prior findings ledger: [Critical] commitlint job failing because npm-installed deps weren't resolvable from the run step's cwd — fixed/withdrawn, still gone (the job is a single action step, no local npm/install). [Important] the comment attributing the CI/pre-commit divergence to @commitlint/ensure v19 lacking v20's URL exemption — fixed, that specific claim was removed from the comment. However the rewritten comment (lines 76-79) introduces a variant of the same problem: it now claims the fork's commitlint v21 is "matching the version the pre-commit hook resolves", which is still inaccurate — the local pre-commit hook has never been commitlint; it's compilerla/conventional-pre-commit (added in 41ad693, unchanged since), a separate tool with its own default ruleset that has never read commitlint.config.mjs. So the underlying root-cause narrative ("pre-commit and CI run the same commitlint config at different versions") is still not accurate, just restated with a different specific error. The action switch itself remains sound (SHA-pinned, no functional change in this diff, CI green across 8 checks).
| # Fork of wagoid/commitlint-github-action. Upstream is unmaintained: no | ||
| # commit to master since 2025-01-14, and it pins @commitlint/ensure ^19, | ||
| # two majors behind. This fork runs commitlint v21 on node 24, matching | ||
| # the version the pre-commit hook resolves. |
There was a problem hiding this comment.
[Important] Comment still misattributes the CI/pre-commit divergence
This claims the fork's commitlint v21 is "matching the version the pre-commit hook resolves", implying the local pre-commit hook runs commitlint at some version. It doesn't: the commit-msg hook is compilerla/conventional-pre-commit (.pre-commit-config.yaml, added in 41ad693 and unchanged since) — a different tool with its own default rules, not commitlint, and it has never read commitlint.config.mjs. There is no "version the pre-commit hook resolves" for commitlint to match. A future maintainer debugging a similar CI-vs-local divergence will look for a commitlint version skew between the two, when the actual difference is that only CI runs commitlint at all.
wagoid/commitlint-github-action pins @commitlint/ensure ^19. The exemption for long lines containing a URL landed in v20, so CI enforced a stricter rule than the config intended and failed Dependabot commits on body-max-line-length, while the pre-commit hook resolving v21 passed the same commit. Upstream is unmaintained: master has had no commit since 2025-01-14 and the v20 bump has sat open since July. Point at yo61/commitlint-github-action instead, which runs commitlint v21 on node 24 and is published to ghcr.io.
1fdac9c to
0fa603c
Compare
There was a problem hiding this comment.
Re-review at the latest head (0fa603c), which is unchanged in mechanism from the prior review but rewrites the workflow comment. Prior findings ledger: [Critical] commitlint job failing due to unresolvable module path — fixed/withdrawn, that whole custom npm invocation is gone. [Important] the workflow comment misattributed the CI/pre-commit divergence to @commitlint/ensure v19 vs v20's URL exemption, then to a pre-commit-hook version match that doesn't exist — fixed: the comment at lines 76-78 has been rewritten to drop both unverified claims and now states only that upstream is unmaintained and pins @commitlint/ensure two majors behind, which holds up against the repo's history. No new findings. The action switch is sound (SHA-pinned to yo61/commitlint-github-action@31e5e04 / v6.3.0, a real fork of the same account), CI is green across all checks at this head.
What
Points the commitlint job at
yo61/commitlint-github-action— a maintained fork running commitlint v21 on Node 24.Why
Dependabot PRs fail the commitlint job on
body-max-line-length, while the pre-commitlintjob passes the same commit. Same commit, samecommitlint.config.mjs, different verdict.@commitlint/ensurev20 added an exemption for long lines containing a URL:wagoid/commitlint-github-action@v6.2.1is the latest release and pins@commitlint/ensure: ^19.0.3, so CI enforced a stricter rule than the config intends. Dependabot bodies are markdown link lists — one real example is 314 characters on a single line.Upstream is unmaintained: no commit to
mastersince 2025-01-14, and the v20 bump (PR #837) has been open and mergeable since July with no response.The fork
yo61/commitlint-github-action@v6.3.0— all@commitlint/*at^21, Node 24.19.0 LTS (v21 needsnode >= 22.12),commitlint-plugin-function-rulesat^5(v4's peer range caps at<20and breaksnpm ci), published toghcr.iofrom a pushed tag. Actions SHA-pinned, least-privilege permissions,actionlintandzizmorclean.Verified against the action's own load+lint path with a real
commitlint.config.mjs:subject-case: [0].mjsoverride is honoredCommit subjects stay validated for Dependabot as well as humans, rather than exempting bot commits.