diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ed3263..8574027 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,24 @@ concurrency: cancel-in-progress: true jobs: + contracts: + name: contracts + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # The exemption in `commitlint.config.mjs` decides whether a dependency + # update can merge, and an exemption written one notch too wide reads + # exactly like a correct one. These controls are the difference. + - name: The commitlint exemption is not wider than stated + run: node scripts/check_commitlint_config.mjs + rust: name: rust permissions: @@ -73,3 +91,10 @@ jobs: issues: write # the stale bot closes threads pull-requests: write # the labeler applies labels uses: NDDev-OpenNetwork/ci-workflows/.github/workflows/pr-hygiene.yml@1ab6708b62ec7bd17f2d8a519c6fcc39edb22243 + with: + # The stock configuration rejects any body line over 100 characters, and + # Dependabot writes a 144-character compare link into every git-ref bump. + # Five dependencies here are pinned by git ref, so the rule would make + # their updates unmergeable. `commitlint.config.mjs` keeps the limit and + # exempts only lines that no amount of wrapping could shorten. + commitlint_config: commitlint.config.mjs diff --git a/commitlint.config.mjs b/commitlint.config.mjs new file mode 100644 index 0000000..0954470 --- /dev/null +++ b/commitlint.config.mjs @@ -0,0 +1,78 @@ +// Conventional Commits, with one narrow exemption to the line-length rules. +// +// `body-max-line-length` and `footer-max-line-length` exist so that `git log` +// stays readable in a narrow terminal, and for prose that is the right rule. +// It is not achievable for a line that is long because of a URL. Dependabot +// writes several such lines into every dependency bump: +// +// - [Commits](https://github.com///compare/<40 hex>...<40 hex>) +// | [rustls-platform-verifier](https://github.com/rustls/rustls-platform-verifier) | `0.6.2` | `0.7.0` | +// +// 144 and 102 characters, and neither has a wrap point that would help: the +// first is one link, the second is a table row. With the stock rule every such +// pull request fails commitlint, is blocked from merging, and the dependency +// never lands - which is how a security update stops arriving. +// +// So the limit still applies, and it is measured over what the author could +// have wrapped. Each URL counts as one character; if the line is still too +// long after that, nobody wrapped it and the rule fires. A 119-character line +// of ordinary words fails, and so does a 130-character line of prose that +// happens to contain a short link. A line whose length is a link is exempt. +// +// A single non-URL token longer than the limit - a hash, a base64 blob, a very +// deep path - is exempt on the same reasoning. +// +// The controls are in `scripts/check_commitlint_config.mjs`, which CI runs. + +const LIMIT = 100; + +// Stops at whitespace and at a closing paren, so the `)` and `.` that end a +// markdown link are counted as the text they are. +const URL_PATTERN = /https?:\/\/[^\s)]+/g; + +const unwrappable = (line) => { + if (line.split(/\s+/).some((token) => token.length > LIMIT)) { + return true; + } + return line.replace(URL_PATTERN, '').length <= LIMIT; +}; + +const offendingLines = (text) => + (text ?? '') + .split('\n') + .filter((line) => line.length > LIMIT) + .filter((line) => !unwrappable(line)); + +const wrappableLineLength = (section) => (parsed) => { + const offenders = offendingLines(parsed[section]); + const detail = offenders + .map((line) => ` ${line.length} chars: ${line.slice(0, 60)}...`) + .join('\n'); + return [ + offenders.length === 0, + `${section} lines must not be longer than ${LIMIT} characters. A line is ` + + `exempt only when its length comes from a URL or from one very long ` + + `token:\n${detail}`, + ]; +}; + +export const testable = { LIMIT, unwrappable, offendingLines }; + +export default { + extends: ['@commitlint/config-conventional'], + plugins: [ + { + rules: { + 'body-max-line-length-wrappable': wrappableLineLength('body'), + 'footer-max-line-length-wrappable': wrappableLineLength('footer'), + }, + }, + ], + rules: { + // Replaced, not relaxed: the two rules below enforce the same limit. + 'body-max-line-length': [0, 'always', LIMIT], + 'footer-max-line-length': [0, 'always', LIMIT], + 'body-max-line-length-wrappable': [2, 'always'], + 'footer-max-line-length-wrappable': [2, 'always'], + }, +}; diff --git a/scripts/check_commitlint_config.mjs b/scripts/check_commitlint_config.mjs new file mode 100644 index 0000000..4ea3bf9 --- /dev/null +++ b/scripts/check_commitlint_config.mjs @@ -0,0 +1,78 @@ +// Controls for the line-length exemption in `commitlint.config.mjs`. +// +// The exemption exists so Dependabot's pull requests can merge. It is worth a +// test because it is the kind of rule that is easy to write too wide: an +// exemption that lets every long line through reads exactly like one that lets +// only the unwrappable ones through, and the difference only shows up months +// later as an unreadable `git log`. +// +// Run with `node scripts/check_commitlint_config.mjs`. No dependencies: this +// exercises the predicate directly, not commitlint, so it needs no install. + +import { testable } from '../commitlint.config.mjs'; + +const { LIMIT, unwrappable } = testable; + +const repeat = (word, times) => Array(times).fill(word).join(' '); + +const cases = [ + // Exempt: the line is long because of a link, and there is no useful wrap + // point inside one. All four are real lines from Dependabot commits. + [ + true, + 'a Dependabot compare link', + '- [Commits](https://github.com/example/kcp-sys/compare/32a6c09fc6223f54aea83981a6aa8995931d29be...d7427c22d764deb1860a7d37acc446ed5033464c)', + ], + [ + true, + 'a Dependabot table row', + '| [rustls-platform-verifier](https://github.com/rustls/rustls-platform-verifier) | `0.6.2` | `0.7.0` |', + ], + [ + true, + 'a Dependabot sentence carrying a link', + 'Bumps the cargo-major group with 1 update in the /libs/portable directory: [md5](https://github.com/stainless-steel/md5).', + ], + [ + true, + 'a single token longer than the limit', + `sha256:${'a'.repeat(120)}`, + ], + + // Not exempt: every one of these could have been wrapped, and the rule + // exists for exactly these. + [false, 'unwrapped prose', repeat('word', 24)], + [ + false, + 'prose that merely contains a short link', + `${repeat('word', 22)} https://example.com/x and more words here`, + ], + [ + false, + 'a long line with a link that does not account for its length', + `${repeat('sentence', 14)} https://example.com/`, + ], +]; + +let failed = 0; +for (const [expected, name, line] of cases) { + const actual = unwrappable(line); + const verdict = actual === expected ? 'ok ' : 'FAIL'; + if (actual !== expected) failed += 1; + console.log( + `${verdict} ${name} (${line.length} chars, exempt=${actual}, want=${expected})`, + ); +} + +// The predicate must never be reached for a line inside the limit; if it were, +// a short line could be reported. Guard the caller's contract too. +if (LIMIT !== 100) { + console.log(`FAIL the limit moved to ${LIMIT} without this test moving`); + failed += 1; +} + +if (failed > 0) { + console.error(`\n${failed} control(s) failed`); + process.exit(1); +} +console.log(`\nall ${cases.length} controls passed`);