From f26dbcce5dc7638ba2418149ada737be0ba20203 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 13 Aug 2026 13:12:46 -0600 Subject: [PATCH 01/19] Scope Dependabot to our beyond-template packages Template-owned dependencies move only when we merge template changes, so the allow list covers just the packages this extension adds. Security updates stay inert until the repo's dependency graph is enabled. --- .github/dependabot.yml | 56 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 .github/dependabot.yml diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..12934185 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,56 @@ +# Dependabot is scoped to the packages this extension adds on top of +# paranext-extension-template. Everything else in package.json comes from the +# template and is updated only when we merge template changes, so letting +# Dependabot raise PRs for those would put our lockfile ahead of the template's +# and create conflicts at the next merge. An allow list filters security updates +# as well as version updates, so nothing template-owned is touched either way. +# +# Keep the allow list in sync with package.json: an entry belongs here if, and +# only if, it is absent from the template's package.json. +version: 2 +updates: + - package-ecosystem: npm + directory: / + schedule: + interval: weekly + # The three `file:` dependencies resolve against a sibling paranext-core + # checkout, which exists on developer machines and in CI but not inside + # Dependabot's container. Without these entries Dependabot aborts the whole + # job during file fetching and never reaches the allow list below. + ignore: + - dependency-name: platform-bible-utils + - dependency-name: papi-dts + - dependency-name: platform-bible-react + allow: + # dependencies + - dependency-name: '@reduxjs/toolkit' + - dependency-name: fast-xml-parser + - dependency-name: react-redux + # devDependencies + - dependency-name: '@playwright/test' + - dependency-name: '@testing-library/jest-dom' + - dependency-name: '@testing-library/react' + - dependency-name: '@testing-library/user-event' + - dependency-name: '@types/jest' + - dependency-name: '@types/ws' + - dependency-name: eslint-plugin-jest + - dependency-name: jest + - dependency-name: jest-environment-jsdom + - dependency-name: ts-jest + - dependency-name: ws + groups: + # Test tooling moves together, so one PR per week rather than one per + # package. Major bumps stay ungrouped: those are worth reading on their own. + test-tooling: + applies-to: version-updates + update-types: ['minor', 'patch'] + patterns: + - '@playwright/test' + - '@testing-library/*' + - '@types/jest' + - '@types/ws' + - eslint-plugin-jest + - jest + - jest-environment-jsdom + - ts-jest + - ws From 6884f291af0ba7b19872bed5c212fc72df19aa9d Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 14 Aug 2026 11:23:14 -0600 Subject: [PATCH 02/19] Slow Dependabot to monthly; fix the ignore rationale npm's file fetcher consults `ignore` and never `allow`, so the comment claiming the job "never reaches the allow list" named the wrong key. --- .github/dependabot.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 12934185..68709560 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -12,11 +12,14 @@ updates: - package-ecosystem: npm directory: / schedule: - interval: weekly + interval: monthly # The three `file:` dependencies resolve against a sibling paranext-core # checkout, which exists on developer machines and in CI but not inside - # Dependabot's container. Without these entries Dependabot aborts the whole - # job during file fetching and never reaches the allow list below. + # Dependabot's container. npm's file fetcher resolves path dependencies + # before any update is considered, and it consults `ignore` — never `allow` — + # when deciding to skip one. Omitting them from the allow list below is + # therefore not enough on its own: without these entries they become + # unfetchable and the job aborts during file fetching. ignore: - dependency-name: platform-bible-utils - dependency-name: papi-dts @@ -39,7 +42,7 @@ updates: - dependency-name: ts-jest - dependency-name: ws groups: - # Test tooling moves together, so one PR per week rather than one per + # Test tooling moves together, so one PR per cycle rather than one per # package. Major bumps stay ungrouped: those are worth reading on their own. test-tooling: applies-to: version-updates From b3650a46b3144c1e9cbf1ce2e9f59e3bf81dfb97 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 12:59:24 -0600 Subject: [PATCH 03/19] Cover GitHub Actions; check the scope against the template SHA-pinned actions receive no updates without an ecosystem entry, and nothing enforced the allow list's "absent from the template" rule. --- .github/dependabot.yml | 19 ++- .github/workflows/lint.yml | 9 ++ README.md | 2 + package-lock.json | 1 + package.json | 4 +- scripts/check-dependency-scope.cjs | 190 +++++++++++++++++++++++++++++ 6 files changed, 223 insertions(+), 2 deletions(-) create mode 100644 scripts/check-dependency-scope.cjs diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 68709560..9015369a 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -6,7 +6,8 @@ # as well as version updates, so nothing template-owned is touched either way. # # Keep the allow list in sync with package.json: an entry belongs here if, and -# only if, it is absent from the template's package.json. +# only if, it is absent from the template's package.json. `npm run +# lint:dependencies` checks that. version: 2 updates: - package-ecosystem: npm @@ -39,6 +40,7 @@ updates: - dependency-name: eslint-plugin-jest - dependency-name: jest - dependency-name: jest-environment-jsdom + - dependency-name: js-yaml - dependency-name: ts-jest - dependency-name: ws groups: @@ -57,3 +59,18 @@ updates: - jest-environment-jsdom - ts-jest - ws + + # No allow list here: we pin every action to a SHA while the template pins by + # tag, so no line Dependabot rewrites is a line the template also owns. + # Dependabot updates the SHA and the `# v1.2.3` comment beside it together. + - package-ecosystem: github-actions + directory: / + schedule: + interval: monthly + groups: + # Grouped for the same reason as the npm group above. + actions: + applies-to: version-updates + update-types: ['minor', 'patch'] + patterns: + - '*' diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e4825504..253be2a5 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -29,6 +29,15 @@ jobs: repository: paranext/paranext-core persist-credentials: false + # `npm run lint:dependencies` compares package.json against the template's. Without this + # checkout it falls back to the `template` remote, which a fresh CI clone does not have. + - name: Checkout paranext-extension-template repo to compare dependencies against + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + path: paranext-extension-template + repository: paranext/paranext-extension-template + persist-credentials: false + # See the matching step in test.yml: the core revision floats, so record which one this run # used. Relevant here because this job typechecks against core's type declarations, so an # upstream change can turn lint red with no change in this repo. diff --git a/README.md b/README.md index b4aeb428..0a968be1 100644 --- a/README.md +++ b/README.md @@ -327,6 +327,8 @@ npm run core:reinstall **Note:** The merge/squash commits created when updating this repo from the template are important; Git uses them to compare the files for future updates. If you edit this repo's Git history, please preserve these commits (do not squash them, for example) to avoid duplicated merge conflicts in the future. +Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml). A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The check reads the template's `package.json` from the `template` remote added above, or from a sibling `paranext-extension-template` checkout, which is how CI supplies it; with neither available it skips itself locally and fails in CI. + ## Special features in this project This project has special features and specific configuration to make building an extension for Platform.Bible easier. Rather than duplicating the full explanation here, please refer to the [`Special Features in this project` section of the multi-extension template README](https://github.com/paranext/paranext-multi-extension-template?tab=readme-ov-file#special-features-in-this-project) for details on these features. diff --git a/package-lock.json b/package-lock.json index 485045bb..1cb7ce28 100644 --- a/package-lock.json +++ b/package-lock.json @@ -56,6 +56,7 @@ "glob": "^10.5.0", "jest": "^30.2.0", "jest-environment-jsdom": "^30.2.0", + "js-yaml": "^4.3.0", "lucide-react": "^1.8.0", "papi-dts": "file:../paranext-core/lib/papi-dts", "platform-bible-react": "file:../paranext-core/lib/platform-bible-react", diff --git a/package.json b/package.json index 17b6d092..ac93ef9a 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "start": "cross-env MAIN_ARGS=\"--extensions $INIT_CWD/dist\" concurrently \"npm:watch\" \"npm:core:start\"", "start:cdp": "cross-env MAIN_ARGS=\"--extensions $INIT_CWD/dist --remote-debugging-port=9223\" concurrently \"npm:watch\" \"npm:core:start\"", "start:production": "cross-env MAIN_ARGS=\"--extensions $INIT_CWD/dist\" concurrently \"npm:watch:production\" \"npm:core:start\"", - "lint": "npm run lint:scripts && npm run lint:styles && npm run lint:typecheck", + "lint": "npm run lint:scripts && npm run lint:styles && npm run lint:typecheck && npm run lint:dependencies", + "lint:dependencies": "node ./scripts/check-dependency-scope.cjs", "lint:scripts": "cross-env NODE_ENV=development eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .", "lint:styles": "stylelint **/*.{css,scss} --allow-empty-input", "lint:typecheck": "tsc --noEmit", @@ -95,6 +96,7 @@ "glob": "^10.5.0", "jest": "^30.2.0", "jest-environment-jsdom": "^30.2.0", + "js-yaml": "^4.3.0", "lucide-react": "^1.8.0", "papi-dts": "file:../paranext-core/lib/papi-dts", "platform-bible-react": "file:../paranext-core/lib/platform-bible-react", diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs new file mode 100644 index 00000000..7ccbfc2c --- /dev/null +++ b/scripts/check-dependency-scope.cjs @@ -0,0 +1,190 @@ +const { execFileSync } = require('child_process'); +const fs = require('fs'); +const path = require('path'); +const yaml = require('js-yaml'); + +/** + * Cross-checks `package.json` against paranext-extension-template's and against + * `.github/dependabot.yml`, enforcing the rule the Dependabot config states in its header: a + * package belongs on the allow list if, and only if, it is absent from the template's + * `package.json`. Exits non-zero on any violation. + * + * When the template's `package.json` cannot be read, the check skips itself locally and fails in CI + * — a missing template remote should not block a lint run, but it must not quietly disable the + * check either. + */ + +const REPO_ROOT = path.join(__dirname, '..'); +const OUR_MANIFEST_PATH = path.join(REPO_ROOT, 'package.json'); +const DEPENDABOT_CONFIG_PATH = path.join(REPO_ROOT, '.github', 'dependabot.yml'); +const TEMPLATE_MANIFEST_PATH = path.join( + REPO_ROOT, + '..', + 'paranext-extension-template', + 'package.json', +); +const TEMPLATE_GIT_REF = 'template/main'; +const ADD_TEMPLATE_REMOTE_HINT = + 'git remote add template https://github.com/paranext/paranext-extension-template && git fetch template'; + +/** + * Version ranges this extension deliberately holds apart from the template's. Each entry records + * both sides, so it covers that one divergence and no other: change either range and the entry + * stops matching, which puts the pair back in front of a human. + */ +const RECORDED_RANGE_DIVERGENCES = [ + { + name: '@tailwindcss/postcss', + template: '^4.0.0', + ours: '^4.3.0', + reason: + 'Narrowed in 791ffd6 alongside the React 19 / Tailwind 4 upgrade. Every version it admits also satisfies the template range.', + }, + { + name: 'tailwindcss', + template: '^4.0.0', + ours: '^4.3.0', + reason: + 'Narrowed in 791ffd6 alongside the React 19 / Tailwind 4 upgrade. Every version it admits also satisfies the template range.', + }, +]; + +/** Whether a version range resolves against a sibling checkout rather than the registry. */ +function isFileDependency(range) { + return range !== undefined && range.startsWith('file:'); +} + +/** Runtime and development dependencies merged, since Dependabot scopes both as one npm ecosystem. */ +function collectDependencies(manifest) { + return { ...manifest.dependencies, ...manifest.devDependencies }; +} + +/** + * @returns {{ source: string; dependencies: Record } | undefined} `undefined` when + * neither the sibling checkout nor the template remote-tracking branch is available. + */ +function readTemplateDependencies() { + if (fs.existsSync(TEMPLATE_MANIFEST_PATH)) { + return { + source: TEMPLATE_MANIFEST_PATH, + dependencies: collectDependencies( + JSON.parse(fs.readFileSync(TEMPLATE_MANIFEST_PATH, 'utf8')), + ), + }; + } + + try { + const manifest = execFileSync('git', ['show', `${TEMPLATE_GIT_REF}:package.json`], { + cwd: REPO_ROOT, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'ignore'], + }); + return { + source: `\`git show ${TEMPLATE_GIT_REF}:package.json\``, + dependencies: collectDependencies(JSON.parse(manifest)), + }; + } catch { + return undefined; + } +} + +/** + * The dependency names the npm ecosystem entry allows and ignores. + * + * @throws When the config declares no npm ecosystem, which would otherwise read as an empty scope + * that passes every check. + */ +function readNpmScope() { + const config = yaml.load(fs.readFileSync(DEPENDABOT_CONFIG_PATH, 'utf8')); + const npmEntry = config.updates.find((entry) => entry['package-ecosystem'] === 'npm'); + if (!npmEntry) throw new Error(`No npm ecosystem entry in ${DEPENDABOT_CONFIG_PATH}`); + + const dependencyNames = (entries) => (entries ?? []).map((entry) => entry['dependency-name']); + return { allow: dependencyNames(npmEntry.allow), ignore: dependencyNames(npmEntry.ignore) }; +} + +/** @returns {string[]} One line per violation; empty when the scoping rule holds. */ +function findViolations(ours, template, scope) { + const violations = []; + const recordedByName = new Map(RECORDED_RANGE_DIVERGENCES.map((entry) => [entry.name, entry])); + + RECORDED_RANGE_DIVERGENCES.forEach((recorded) => { + if (ours[recorded.name] === recorded.ours && template[recorded.name] === recorded.template) + return; + violations.push( + `${recorded.name}: recorded divergence is stale — it records template ${recorded.template} against ours ${recorded.ours} ("${recorded.reason}"), but the manifests now read template ${template[recorded.name] ?? '(absent)'} against ours ${ours[recorded.name] ?? '(absent)'}`, + ); + }); + + Object.entries(ours).forEach(([name, range]) => { + if (recordedByName.has(name) || !(name in template) || template[name] === range) return; + violations.push( + `${name}: ${range} diverges from the template's ${template[name]} — sync it, or record the divergence in ${path.basename(__filename)}`, + ); + }); + + Object.entries(ours).forEach(([name, range]) => { + if (name in template || isFileDependency(range) || scope.allow.includes(name)) return; + violations.push( + `${name}: absent from the template's package.json, so it needs a Dependabot allow entry to receive updates`, + ); + }); + + scope.allow.forEach((name) => { + if (!(name in ours)) + violations.push(`${name}: on Dependabot's allow list but no longer in package.json`); + else if (name in template) + violations.push( + `${name}: on Dependabot's allow list but the template owns it, so template merges will fight its updates`, + ); + }); + + Object.entries(ours).forEach(([name, range]) => { + if (!isFileDependency(range) || scope.ignore.includes(name)) return; + violations.push( + `${name}: a file: dependency missing from Dependabot's ignore list, which aborts its file fetcher`, + ); + }); + + scope.ignore.forEach((name) => { + if (isFileDependency(ours[name])) return; + violations.push( + `${name}: on Dependabot's ignore list, which exists for file: dependencies — an ignore entry with another purpose needs this check updated`, + ); + }); + + return violations; +} + +const template = readTemplateDependencies(); + +if (!template) { + const message = `Cannot read the template's package.json — looked in ${TEMPLATE_MANIFEST_PATH} and \`${TEMPLATE_GIT_REF}\`.`; + if (process.env.CI) { + console.error(`✗ ${message}`); + process.exit(1); + } + console.log(`⊘ ${message}`); + console.log(` To run this check locally: ${ADD_TEMPLATE_REMOTE_HINT}`); + process.exit(0); +} + +const ours = collectDependencies(JSON.parse(fs.readFileSync(OUR_MANIFEST_PATH, 'utf8'))); +const violations = findViolations(ours, template.dependencies, readNpmScope()); + +console.log(`Comparing package.json against ${template.source}`); + +const templateOnly = Object.keys(template.dependencies).filter((name) => !(name in ours)); +if (templateOnly.length > 0) { + console.log( + `⊘ In the template but not here, which is expected between template merges: ${templateOnly.join(', ')}`, + ); +} + +if (violations.length > 0) { + violations.forEach((violation) => console.error(`✗ ${violation}`)); + process.exit(1); +} + +console.log('✓ Dependabot scope matches the packages this extension adds to the template'); +process.exit(0); From a0bd2608c2b997cb664fecd14b7c27a239ad86e3 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 13:41:01 -0600 Subject: [PATCH 04/19] Pin the dependency check to the merged template commit Comparing against the template's moving head turned lint red on every PR whenever the template bumped a shared range; reading the baseline out of this repo's own history also lets CI drop the template checkout. Failures name a stale baseline as a possible cause, since it inverts their advice. --- .github/workflows/lint.yml | 12 +--- README.md | 8 ++- scripts/check-dependency-scope.cjs | 103 ++++++++++++++++++----------- 3 files changed, 75 insertions(+), 48 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 253be2a5..eb23edc8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -21,6 +21,9 @@ jobs: with: path: extension-repo persist-credentials: false + # Full history because `npm run lint:dependencies` reads the template's package.json out + # of the template commit this repo merged, which the default shallow clone cannot reach. + fetch-depth: 0 - name: Checkout paranext-core repo to use its sub-packages uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -29,15 +32,6 @@ jobs: repository: paranext/paranext-core persist-credentials: false - # `npm run lint:dependencies` compares package.json against the template's. Without this - # checkout it falls back to the `template` remote, which a fresh CI clone does not have. - - name: Checkout paranext-extension-template repo to compare dependencies against - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - path: paranext-extension-template - repository: paranext/paranext-extension-template - persist-credentials: false - # See the matching step in test.yml: the core revision floats, so record which one this run # used. Relevant here because this job typechecks against core's type declarations, so an # upstream change can turn lint red with no change in this repo. diff --git a/README.md b/README.md index 0a968be1..4a11c87e 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,12 @@ git fetch template git merge template/main --allow-unrelated-histories ``` +Merging is also what moves the baseline `npm run lint:dependencies` compares dependency versions against, so in the same commit set `MERGED_TEMPLATE_COMMIT` at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs) to the template commit you just merged, which this prints: + +```bash +git rev-parse template/main +``` + For more information, read [the instructions on the wiki](https://github.com/paranext/paranext-extension-template/wiki/Merging-Template-Changes-into-Your-Extension). After updating this extension from the template, clear all temp/cache files and regenerate the extension's `package-lock.json` with this command: @@ -327,7 +333,7 @@ npm run core:reinstall **Note:** The merge/squash commits created when updating this repo from the template are important; Git uses them to compare the files for future updates. If you edit this repo's Git history, please preserve these commits (do not squash them, for example) to avoid duplicated merge conflicts in the future. -Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml). A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The check reads the template's `package.json` from the `template` remote added above, or from a sibling `paranext-extension-template` checkout, which is how CI supplies it; with neither available it skips itself locally and fails in CI. +Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml). A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is the merged commit recorded in that same file, read straight from this repo's history, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. A clone too shallow to reach that commit skips the check locally and fails it in CI. ## Special features in this project diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 7ccbfc2c..4aadf583 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -9,23 +9,35 @@ const yaml = require('js-yaml'); * package belongs on the allow list if, and only if, it is absent from the template's * `package.json`. Exits non-zero on any violation. * - * When the template's `package.json` cannot be read, the check skips itself locally and fails in CI - * — a missing template remote should not block a lint run, but it must not quietly disable the - * check either. + * When this clone's history does not reach the merged template commit, the check skips itself + * locally and fails in CI — a shallow clone should not block a lint run, but it must not quietly + * disable the check either. */ const REPO_ROOT = path.join(__dirname, '..'); const OUR_MANIFEST_PATH = path.join(REPO_ROOT, 'package.json'); const DEPENDABOT_CONFIG_PATH = path.join(REPO_ROOT, '.github', 'dependabot.yml'); -const TEMPLATE_MANIFEST_PATH = path.join( - REPO_ROOT, - '..', - 'paranext-extension-template', - 'package.json', -); -const TEMPLATE_GIT_REF = 'template/main'; -const ADD_TEMPLATE_REMOTE_HINT = - 'git remote add template https://github.com/paranext/paranext-extension-template && git fetch template'; + +/** + * The template commit this repo has merged. README's update instructions move it in the same commit + * that merges the template, which is also what puts it in this repo's history. + * + * Holding the comparison to a fixed commit rather than the template's moving head is what lets a + * range difference mean "this extension moved the range": the template bumps its own ranges between + * merges, and those bumps are for the next merge to adopt rather than a lint failure in the + * meantime. + */ +const MERGED_TEMPLATE_COMMIT = '5f44a9d8e18908374962a482f74d2cc76270f91c'; + +const SHORT_COMMIT = MERGED_TEMPLATE_COMMIT.slice(0, 7); +const DEEPEN_HISTORY_HINT = 'git fetch --unshallow'; + +/** + * A baseline left behind by a template merge reads that merge's own bumps as this extension's, so + * every violation comes out inverted and advises undoing the merge. Nothing in the manifests tells + * that case from real drift, so every failure carries the possibility. + */ +const STALE_BASELINE_HINT = `If these came in with a template merge, move MERGED_TEMPLATE_COMMIT in ${path.basename(__filename)} to the template commit that merge brought in, rather than acting on the lines above.`; /** * Version ranges this extension deliberately holds apart from the template's. Each entry records @@ -60,34 +72,39 @@ function collectDependencies(manifest) { } /** - * @returns {{ source: string; dependencies: Record } | undefined} `undefined` when - * neither the sibling checkout nor the template remote-tracking branch is available. + * @returns {Record | undefined} `undefined` when this clone's history does not + * reach {@link MERGED_TEMPLATE_COMMIT}, a shallow clone being the ordinary reason for that. */ -function readTemplateDependencies() { - if (fs.existsSync(TEMPLATE_MANIFEST_PATH)) { - return { - source: TEMPLATE_MANIFEST_PATH, - dependencies: collectDependencies( - JSON.parse(fs.readFileSync(TEMPLATE_MANIFEST_PATH, 'utf8')), - ), - }; - } - +function readMergedTemplateDependencies() { try { - const manifest = execFileSync('git', ['show', `${TEMPLATE_GIT_REF}:package.json`], { + const manifest = execFileSync('git', ['show', `${MERGED_TEMPLATE_COMMIT}:package.json`], { cwd: REPO_ROOT, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], }); - return { - source: `\`git show ${TEMPLATE_GIT_REF}:package.json\``, - dependencies: collectDependencies(JSON.parse(manifest)), - }; + return collectDependencies(JSON.parse(manifest)); } catch { return undefined; } } +/** + * Whether {@link MERGED_TEMPLATE_COMMIT} is one this repo has merged rather than one it is still + * behind, which is what keeps the baseline honest: moving it forward without merging would silently + * excuse the very drift this check exists to report. + */ +function isMergedIntoHead() { + try { + execFileSync('git', ['merge-base', '--is-ancestor', MERGED_TEMPLATE_COMMIT, 'HEAD'], { + cwd: REPO_ROOT, + stdio: 'ignore', + }); + return true; + } catch { + return false; + } +} + /** * The dependency names the npm ecosystem entry allows and ignores. * @@ -119,7 +136,7 @@ function findViolations(ours, template, scope) { Object.entries(ours).forEach(([name, range]) => { if (recordedByName.has(name) || !(name in template) || template[name] === range) return; violations.push( - `${name}: ${range} diverges from the template's ${template[name]} — sync it, or record the divergence in ${path.basename(__filename)}`, + `${name}: ${range} moved off the template's ${template[name]} — sync it back, or record the divergence in ${path.basename(__filename)}`, ); }); @@ -156,33 +173,43 @@ function findViolations(ours, template, scope) { return violations; } -const template = readTemplateDependencies(); +const templateDependencies = readMergedTemplateDependencies(); -if (!template) { - const message = `Cannot read the template's package.json — looked in ${TEMPLATE_MANIFEST_PATH} and \`${TEMPLATE_GIT_REF}\`.`; +if (!templateDependencies) { + const message = `Cannot read the template's package.json at ${SHORT_COMMIT} — this clone's history does not reach it.`; if (process.env.CI) { console.error(`✗ ${message}`); process.exit(1); } console.log(`⊘ ${message}`); - console.log(` To run this check locally: ${ADD_TEMPLATE_REMOTE_HINT}`); + console.log(` To run this check locally: ${DEEPEN_HISTORY_HINT}`); process.exit(0); } +if (!isMergedIntoHead()) { + console.error( + `✗ ${SHORT_COMMIT} is recorded as the template commit this repo has merged, but it is not in this history. Move that commit only in the merge that adopts it — or run \`${DEEPEN_HISTORY_HINT}\` if this clone is simply too shallow to see the merge.`, + ); + process.exit(1); +} + const ours = collectDependencies(JSON.parse(fs.readFileSync(OUR_MANIFEST_PATH, 'utf8'))); -const violations = findViolations(ours, template.dependencies, readNpmScope()); +const violations = findViolations(ours, templateDependencies, readNpmScope()); -console.log(`Comparing package.json against ${template.source}`); +console.log( + `Comparing package.json against the template at ${SHORT_COMMIT}, the commit this repo has merged`, +); -const templateOnly = Object.keys(template.dependencies).filter((name) => !(name in ours)); +const templateOnly = Object.keys(templateDependencies).filter((name) => !(name in ours)); if (templateOnly.length > 0) { console.log( - `⊘ In the template but not here, which is expected between template merges: ${templateOnly.join(', ')}`, + `⊘ In that template commit but not here, so this extension has dropped them: ${templateOnly.join(', ')}`, ); } if (violations.length > 0) { violations.forEach((violation) => console.error(`✗ ${violation}`)); + console.error(`ℹ ${STALE_BASELINE_HINT}`); process.exit(1); } From 13de9fa884aa6bb01e1f99d02ee881ccd824a8a5 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 14:49:59 -0600 Subject: [PATCH 05/19] Read the dependency baseline from a checked-in copy A squashed template update leaves the pinned commit unreachable, which would fail every build; the copy also drops lint's git dependency and CI-only skip. Add npm run template:baseline and document when the refresh is needed. --- .github/workflows/lint.yml | 3 - .prettierignore | 4 + AGENTS.md | 4 +- README.md | 7 +- package.json | 3 +- scripts/check-dependency-scope.cjs | 87 ++++++---------------- scripts/merged-template-package.json | 105 +++++++++++++++++++++++++++ 7 files changed, 141 insertions(+), 72 deletions(-) create mode 100644 scripts/merged-template-package.json diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index eb23edc8..e4825504 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -21,9 +21,6 @@ jobs: with: path: extension-repo persist-credentials: false - # Full history because `npm run lint:dependencies` reads the template's package.json out - # of the template commit this repo merged, which the default shallow clone cannot reach. - fetch-depth: 0 - name: Checkout paranext-core repo to use its sub-packages uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 diff --git a/.prettierignore b/.prettierignore index 01f359ac..a418d8f5 100644 --- a/.prettierignore +++ b/.prettierignore @@ -41,3 +41,7 @@ package-lock.json # Playwright test output e2e-tests/playwright-report e2e-tests/test-results + +# A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain +# `git show` redirect rather than a copy plus whatever reformatting our config would impose +scripts/merged-template-package.json diff --git a/AGENTS.md b/AGENTS.md index 08a50558..8a50af20 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -14,7 +14,7 @@ npm run build:web-view # Build React WebView only npm run watch # Continuous rebuild on changes # Lint & Format -npm run lint # Run ESLint + stylelint + tsc --noEmit +npm run lint # Run ESLint + stylelint + tsc --noEmit + dependency scope npm run lint-fix # Auto-fix linting issues npm run format # Format with Prettier @@ -25,6 +25,8 @@ npm test -- path/to/file.test.ts # Run a single test file npm test -- --testNamePattern="pattern" # Run tests matching name ``` +Only a template merge moves the dependency baseline `npm run lint:dependencies` checks against; refresh it in that same commit with `npm run template:baseline` and point `MERGED_TEMPLATE_COMMIT` in [scripts/check-dependency-scope.cjs](scripts/check-dependency-scope.cjs) at the merged commit. [README.md](README.md) has the full procedure. + ## Architecture This is a **Platform.Bible extension** for interlinear Bible text alignment. Platform.Bible (PAPI) is an Electron-based application; extensions run in a sandboxed context and communicate with the host via `papi.*` APIs. diff --git a/README.md b/README.md index 4a11c87e..c9962941 100644 --- a/README.md +++ b/README.md @@ -317,12 +317,15 @@ git fetch template git merge template/main --allow-unrelated-histories ``` -Merging is also what moves the baseline `npm run lint:dependencies` compares dependency versions against, so in the same commit set `MERGED_TEMPLATE_COMMIT` at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs) to the template commit you just merged, which this prints: +Merging is also what moves the baseline `npm run lint:dependencies` compares dependency versions against, so in the same commit refresh that baseline from the template you just merged and record the commit it came from: ```bash +npm run template:baseline git rev-parse template/main ``` +Run these before fetching the template again, so the baseline records the state you actually merged. Set `MERGED_TEMPLATE_COMMIT` at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs) to the commit the second command prints. Nothing resolves that id — it is there so the check's output names a template state you can go and look at — so move it and the copy together. + For more information, read [the instructions on the wiki](https://github.com/paranext/paranext-extension-template/wiki/Merging-Template-Changes-into-Your-Extension). After updating this extension from the template, clear all temp/cache files and regenerate the extension's `package-lock.json` with this command: @@ -333,7 +336,7 @@ npm run core:reinstall **Note:** The merge/squash commits created when updating this repo from the template are important; Git uses them to compare the files for future updates. If you edit this repo's Git history, please preserve these commits (do not squash them, for example) to avoid duplicated merge conflicts in the future. -Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml). A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is the merged commit recorded in that same file, read straight from this repo's history, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. A clone too shallow to reach that commit skips the check locally and fails it in CI. +Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml). A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. ## Special features in this project diff --git a/package.json b/package.json index ac93ef9a..878fa6ec 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,8 @@ "core:reinstall": "npm run core:stop && node ./scripts/delete-temp-files.cjs --all && npm run core:update && npm i", "core:install": "npm --prefix ../paranext-core install", "core:pull": "git -C ../paranext-core pull --ff-only", - "core:update": "npm run core:pull && npm run core:install" + "core:update": "npm run core:pull && npm run core:install", + "template:baseline": "git show template/main:package.json > scripts/merged-template-package.json" }, "browserslist": [], "peerDependencies": { diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 4aadf583..16cd6f36 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -1,4 +1,3 @@ -const { execFileSync } = require('child_process'); const fs = require('fs'); const path = require('path'); const yaml = require('js-yaml'); @@ -8,10 +7,6 @@ const yaml = require('js-yaml'); * `.github/dependabot.yml`, enforcing the rule the Dependabot config states in its header: a * package belongs on the allow list if, and only if, it is absent from the template's * `package.json`. Exits non-zero on any violation. - * - * When this clone's history does not reach the merged template commit, the check skips itself - * locally and fails in CI — a shallow clone should not block a lint run, but it must not quietly - * disable the check either. */ const REPO_ROOT = path.join(__dirname, '..'); @@ -19,25 +14,36 @@ const OUR_MANIFEST_PATH = path.join(REPO_ROOT, 'package.json'); const DEPENDABOT_CONFIG_PATH = path.join(REPO_ROOT, '.github', 'dependabot.yml'); /** - * The template commit this repo has merged. README's update instructions move it in the same commit - * that merges the template, which is also what puts it in this repo's history. + * The template's `package.json` as of the commit this repo has merged, copied in verbatim. README's + * update instructions refresh it in the same commit that merges the template. * - * Holding the comparison to a fixed commit rather than the template's moving head is what lets a - * range difference mean "this extension moved the range": the template bumps its own ranges between - * merges, and those bumps are for the next merge to adopt rather than a lint failure in the + * Holding the comparison to a fixed template state rather than the template's moving head is what + * lets a range difference mean "this extension moved the range": the template bumps its own ranges + * between merges, and those bumps are for the next merge to adopt rather than a lint failure in the * meantime. + * + * The baseline is a copy because it cannot be a git reference. A template update reaches `main` + * squashed as readily as merged — #204 did — and a squash leaves the template's own commits + * unreachable from this repo, so resolving a commit id would fail on every build once an update + * lands that way. A copy also puts each move of the baseline in a reviewable diff. + */ +const MERGED_TEMPLATE_MANIFEST_PATH = path.join(__dirname, 'merged-template-package.json'); + +/** + * The commit {@link MERGED_TEMPLATE_MANIFEST_PATH} was copied from, recorded so that output names a + * template state a reader can go and look at. Nothing resolves it — the copy beside it is what this + * check reads — so the two move together or not at all. */ const MERGED_TEMPLATE_COMMIT = '5f44a9d8e18908374962a482f74d2cc76270f91c'; const SHORT_COMMIT = MERGED_TEMPLATE_COMMIT.slice(0, 7); -const DEEPEN_HISTORY_HINT = 'git fetch --unshallow'; /** * A baseline left behind by a template merge reads that merge's own bumps as this extension's, so * every violation comes out inverted and advises undoing the merge. Nothing in the manifests tells * that case from real drift, so every failure carries the possibility. */ -const STALE_BASELINE_HINT = `If these came in with a template merge, move MERGED_TEMPLATE_COMMIT in ${path.basename(__filename)} to the template commit that merge brought in, rather than acting on the lines above.`; +const STALE_BASELINE_HINT = `If these came in with a template merge, refresh ${path.basename(MERGED_TEMPLATE_MANIFEST_PATH)} from the template commit that merge brought in, rather than acting on the lines above.`; /** * Version ranges this extension deliberately holds apart from the template's. Each entry records @@ -71,38 +77,8 @@ function collectDependencies(manifest) { return { ...manifest.dependencies, ...manifest.devDependencies }; } -/** - * @returns {Record | undefined} `undefined` when this clone's history does not - * reach {@link MERGED_TEMPLATE_COMMIT}, a shallow clone being the ordinary reason for that. - */ -function readMergedTemplateDependencies() { - try { - const manifest = execFileSync('git', ['show', `${MERGED_TEMPLATE_COMMIT}:package.json`], { - cwd: REPO_ROOT, - encoding: 'utf8', - stdio: ['ignore', 'pipe', 'ignore'], - }); - return collectDependencies(JSON.parse(manifest)); - } catch { - return undefined; - } -} - -/** - * Whether {@link MERGED_TEMPLATE_COMMIT} is one this repo has merged rather than one it is still - * behind, which is what keeps the baseline honest: moving it forward without merging would silently - * excuse the very drift this check exists to report. - */ -function isMergedIntoHead() { - try { - execFileSync('git', ['merge-base', '--is-ancestor', MERGED_TEMPLATE_COMMIT, 'HEAD'], { - cwd: REPO_ROOT, - stdio: 'ignore', - }); - return true; - } catch { - return false; - } +function readDependencies(manifestPath) { + return collectDependencies(JSON.parse(fs.readFileSync(manifestPath, 'utf8'))); } /** @@ -173,27 +149,8 @@ function findViolations(ours, template, scope) { return violations; } -const templateDependencies = readMergedTemplateDependencies(); - -if (!templateDependencies) { - const message = `Cannot read the template's package.json at ${SHORT_COMMIT} — this clone's history does not reach it.`; - if (process.env.CI) { - console.error(`✗ ${message}`); - process.exit(1); - } - console.log(`⊘ ${message}`); - console.log(` To run this check locally: ${DEEPEN_HISTORY_HINT}`); - process.exit(0); -} - -if (!isMergedIntoHead()) { - console.error( - `✗ ${SHORT_COMMIT} is recorded as the template commit this repo has merged, but it is not in this history. Move that commit only in the merge that adopts it — or run \`${DEEPEN_HISTORY_HINT}\` if this clone is simply too shallow to see the merge.`, - ); - process.exit(1); -} - -const ours = collectDependencies(JSON.parse(fs.readFileSync(OUR_MANIFEST_PATH, 'utf8'))); +const templateDependencies = readDependencies(MERGED_TEMPLATE_MANIFEST_PATH); +const ours = readDependencies(OUR_MANIFEST_PATH); const violations = findViolations(ours, templateDependencies, readNpmScope()); console.log( diff --git a/scripts/merged-template-package.json b/scripts/merged-template-package.json new file mode 100644 index 00000000..09b731fa --- /dev/null +++ b/scripts/merged-template-package.json @@ -0,0 +1,105 @@ +{ + "name": "paranext-extension-template", + "private": true, + "version": "0.0.1", + "main": "src/main.js", + "types": "src/types/paranext-extension-template.d.ts", + "author": "Paranext", + "license": "MIT", + "scripts": { + "build:web-view": "webpack --config ./webpack/webpack.config.web-view.ts", + "build:main": "webpack --config ./webpack/webpack.config.main.ts", + "build": "webpack", + "watch": "npm run build -- --watch", + "build:production": "cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=false webpack", + "watch:production": "npm run build:production -- --watch", + "format": "prettier --write .", + "format:check": "prettier --check .", + "zip": "zip-build dist release --template '%NAME%_%VERSION%.%EXT%' --override", + "package": "npm run build:production && npm run zip", + "package:debug": "cross-env DEBUG_PROD=true npm run package", + "start:core": "cd ../paranext-core && npm run start", + "start": "cross-env MAIN_ARGS=\"--extensions $INIT_CWD/dist\" concurrently \"npm:watch\" \"npm:start:core\"", + "start:production": "cross-env MAIN_ARGS=\"--extensions $INIT_CWD/dist\" concurrently \"npm:watch:production\" \"npm:start:core\"", + "lint": "npm run lint:scripts && npm run lint:styles", + "lint:scripts": "cross-env NODE_ENV=development eslint --ext .cjs,.js,.jsx,.ts,.tsx --cache .", + "lint:styles": "stylelint **/*.{css,scss} --allow-empty-input", + "lint-fix": "npm run lint-fix:scripts && npm run lint:styles -- --fix", + "lint-fix:scripts": "npm run format && npm run lint:scripts", + "bump-versions": "ts-node ./lib/bump-versions.ts" + }, + "browserslist": [], + "peerDependencies": { + "react": ">=19.0.0", + "react-dom": ">=19.0.0" + }, + "dependencies": { + "@sillsdev/scripture": "^2.0.5", + "platform-bible-utils": "file:../paranext-core/lib/platform-bible-utils" + }, + "devDependencies": { + "@dreamsicle.io/stylelint-config-tailwindcss": "^1.2.2", + "@fontsource-variable/ibm-plex-sans": "^5.2.8", + "@stylistic/eslint-plugin-ts": "^2.13.0", + "@swc/core": "1.13.3", + "@tailwindcss/postcss": "^4.0.0", + "@tailwindcss/typography": "^0.5.16", + "@types/node": "^22.19.13", + "@types/react": "^19.0.0", + "@types/react-dom": "^19.0.0", + "@types/webpack": "^5.28.5", + "@typescript-eslint/eslint-plugin": "^8.0.0", + "@typescript-eslint/parser": "^8.0.0", + "concurrently": "^9.1.2", + "copy-webpack-plugin": "^14.0.0", + "cross-env": "^7.0.3", + "css-loader": "^6.11.0", + "escape-string-regexp": "^5.0.0", + "eslint": "^8.57.1", + "eslint-config-airbnb": "^19.0.4", + "eslint-config-prettier": "^9.0.0", + "eslint-import-resolver-typescript": "^3.8.3", + "eslint-plugin-compat": "^4.2.0", + "eslint-plugin-import": "^2.31.0", + "eslint-plugin-jsx-a11y": "^6.10.2", + "eslint-plugin-no-null": "^1.0.2", + "eslint-plugin-no-type-assertion": "^1.3.0", + "eslint-plugin-prettier": "^5.5.5", + "eslint-plugin-promise": "^6.6.0", + "eslint-plugin-react": "^7.37.4", + "eslint-plugin-react-hooks": "^5.0.0", + "glob": "^10.5.0", + "lucide-react": "^1.8.0", + "papi-dts": "file:../paranext-core/lib/papi-dts", + "platform-bible-react": "file:../paranext-core/lib/platform-bible-react", + "postcss": "^8.5.3", + "postcss-loader": "^8.1.1", + "prettier": "^3.5.2", + "prettier-plugin-jsdoc": "^1.3.2", + "sass": "^1.85.0", + "sass-loader": "^16.0.5", + "shadcn": "^4.3.0", + "stylelint": "^16.17.0", + "stylelint-config-recommended": "^14.0.1", + "stylelint-config-sass-guidelines": "^12.1.0", + "swc-loader": "^0.2.6", + "tailwindcss": "^4.0.0", + "ts-node": "^10.9.2", + "tsconfig-paths": "^4.2.0", + "tsconfig-paths-webpack-plugin": "^4.2.0", + "tw-animate-css": "^1.4.0", + "typescript": "^5.8.3", + "webpack": "^5.105.2", + "webpack-cli": "^5.1.4", + "webpack-merge": "^6.0.1", + "zip-build": "^1.8.0" + }, + "overrides": { + "eslint-config-airbnb": { + "eslint-plugin-react-hooks": "$eslint-plugin-react-hooks" + } + }, + "volta": { + "node": "22.22.0" + } +} From ef1499ca11f3cce18855f8cffaa81be6b55c21b6 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 14:52:50 -0600 Subject: [PATCH 06/19] =?UTF-8?q?=E2=84=B9=20If=20these=20came=20in=20with?= =?UTF-8?q?=20a=20template=20merge,=20refresh=20merged-template-package.js?= =?UTF-8?q?on=20from=20the=20template=20commit=20that=20merge=20brought=20?= =?UTF-8?q?in,=20rather=20than=20acting=20on=20the=20lines=20above.=20Run?= =?UTF-8?q?=20npm=20run=20template:baseline=20while=20template/main=20stil?= =?UTF-8?q?l=20points=20at=20that=20commit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/check-dependency-scope.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 16cd6f36..a43901c1 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -43,7 +43,7 @@ const SHORT_COMMIT = MERGED_TEMPLATE_COMMIT.slice(0, 7); * every violation comes out inverted and advises undoing the merge. Nothing in the manifests tells * that case from real drift, so every failure carries the possibility. */ -const STALE_BASELINE_HINT = `If these came in with a template merge, refresh ${path.basename(MERGED_TEMPLATE_MANIFEST_PATH)} from the template commit that merge brought in, rather than acting on the lines above.`; +const STALE_BASELINE_HINT = `If these came in with a template merge, refresh ${path.basename(MERGED_TEMPLATE_MANIFEST_PATH)} from the template commit that merge brought in, rather than acting on the lines above. Run npm run template:baseline while template/main still points at that commit.`; /** * Version ranges this extension deliberately holds apart from the template's. Each entry records From ed6d4fac72d05d9c616afac7cfe0c756a0d3ad22 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 15:05:26 -0600 Subject: [PATCH 07/19] Record the template commit #204 actually merged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recorded id still named the state merged by #65. That template commit's manifest is byte-identical to #204's, so only the id lagged — the checked-in baseline copy needs no change. --- scripts/check-dependency-scope.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index a43901c1..d0ef1405 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -34,7 +34,7 @@ const MERGED_TEMPLATE_MANIFEST_PATH = path.join(__dirname, 'merged-template-pack * template state a reader can go and look at. Nothing resolves it — the copy beside it is what this * check reads — so the two move together or not at all. */ -const MERGED_TEMPLATE_COMMIT = '5f44a9d8e18908374962a482f74d2cc76270f91c'; +const MERGED_TEMPLATE_COMMIT = 'c2a2f07ce9faf1674340fba64e069f2e58a0eb09'; const SHORT_COMMIT = MERGED_TEMPLATE_COMMIT.slice(0, 7); From 95eb6fedb1fd2e1aaf04d4f2c16ab9cc12bdd750 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 17 Aug 2026 15:53:30 -0600 Subject: [PATCH 08/19] Leave the dependency baseline intact when git fails Refreshing it runs a script rather than a shell redirect, which truncated the file before git could fail. The check also compares peerDependencies and names the file it could not parse. --- README.md | 5 +-- package.json | 2 +- scripts/check-dependency-scope.cjs | 18 +++++++-- scripts/refresh-template-baseline.cjs | 53 +++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 scripts/refresh-template-baseline.cjs diff --git a/README.md b/README.md index c9962941..20a8f23e 100644 --- a/README.md +++ b/README.md @@ -317,14 +317,13 @@ git fetch template git merge template/main --allow-unrelated-histories ``` -Merging is also what moves the baseline `npm run lint:dependencies` compares dependency versions against, so in the same commit refresh that baseline from the template you just merged and record the commit it came from: +Merging is also what moves the baseline `npm run lint:dependencies` compares dependency versions against, so in the same commit refresh that baseline from the template you just merged: ```bash npm run template:baseline -git rev-parse template/main ``` -Run these before fetching the template again, so the baseline records the state you actually merged. Set `MERGED_TEMPLATE_COMMIT` at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs) to the commit the second command prints. Nothing resolves that id — it is there so the check's output names a template state you can go and look at — so move it and the copy together. +Run that before fetching the template again, so the baseline records the state you actually merged. Set `MERGED_TEMPLATE_COMMIT` at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs) to the commit it prints. Nothing resolves that id — it is there so the check's output names a template state you can go and look at — so move it and the copy together. For more information, read [the instructions on the wiki](https://github.com/paranext/paranext-extension-template/wiki/Merging-Template-Changes-into-Your-Extension). diff --git a/package.json b/package.json index 878fa6ec..18720963 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "core:install": "npm --prefix ../paranext-core install", "core:pull": "git -C ../paranext-core pull --ff-only", "core:update": "npm run core:pull && npm run core:install", - "template:baseline": "git show template/main:package.json > scripts/merged-template-package.json" + "template:baseline": "node ./scripts/refresh-template-baseline.cjs" }, "browserslist": [], "peerDependencies": { diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index d0ef1405..c1c1a92d 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -72,13 +72,25 @@ function isFileDependency(range) { return range !== undefined && range.startsWith('file:'); } -/** Runtime and development dependencies merged, since Dependabot scopes both as one npm ecosystem. */ +/** + * The manifest's dependency sections merged, since Dependabot scopes them as one npm ecosystem. + * `overrides` stays out: it pins transitive versions rather than naming packages this extension + * depends on, so the allow-list rule has nothing to say about it. + */ function collectDependencies(manifest) { - return { ...manifest.dependencies, ...manifest.devDependencies }; + return { ...manifest.dependencies, ...manifest.devDependencies, ...manifest.peerDependencies }; } +/** + * @throws When the manifest is missing or is not JSON, naming the file — the baseline copy collects + * conflict markers on a template merge as readily as any other file does. + */ function readDependencies(manifestPath) { - return collectDependencies(JSON.parse(fs.readFileSync(manifestPath, 'utf8'))); + try { + return collectDependencies(JSON.parse(fs.readFileSync(manifestPath, 'utf8'))); + } catch (error) { + throw new Error(`Could not read ${path.relative(REPO_ROOT, manifestPath)}: ${error.message}`); + } } /** diff --git a/scripts/refresh-template-baseline.cjs b/scripts/refresh-template-baseline.cjs new file mode 100644 index 00000000..49aee20b --- /dev/null +++ b/scripts/refresh-template-baseline.cjs @@ -0,0 +1,53 @@ +const { execFileSync } = require('child_process'); +const fs = require('fs'); +const path = require('path'); + +/** + * Copies the template's `package.json` over the baseline `npm run lint:dependencies` compares + * against, and reports the template commit the copy came from. README's update instructions run + * this in the same commit that merges the template. + */ + +const REPO_ROOT = path.join(__dirname, '..'); +const BASELINE_PATH = path.join(__dirname, 'merged-template-package.json'); + +/** The template state to copy from — a remote-tracking ref, so it moves only on `git fetch`. */ +const TEMPLATE_REF = 'template/main'; + +/** + * Runs git in the repo root and returns its stdout. + * + * @throws When git exits non-zero, carrying git's own stderr as the message. + */ +function git(...args) { + try { + return execFileSync('git', args, { + cwd: REPO_ROOT, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); + } catch (error) { + throw new Error(error.stderr?.trim() || error.message); + } +} + +let commit; +try { + commit = git('rev-parse', '--verify', TEMPLATE_REF).trim(); +} catch (error) { + console.error(`✗ Could not resolve ${TEMPLATE_REF}: ${error.message}`); + console.error( + 'ℹ Adding the template remote is a one-time step after cloning, and its refs need fetching before a merge. README\'s "To update this extension from the template" section has both commands.', + ); + process.exit(1); +} + +// Ordering is load-bearing: nothing touches the baseline until both git calls have succeeded, and +// the copy comes from the same commit reported below. +const manifest = git('show', `${commit}:package.json`); +fs.writeFileSync(BASELINE_PATH, manifest); + +console.log( + `✓ Copied package.json from ${TEMPLATE_REF} at ${commit.slice(0, 7)} into ${path.relative(REPO_ROOT, BASELINE_PATH)}`, +); +console.log(`ℹ Set MERGED_TEMPLATE_COMMIT in check-dependency-scope.cjs to ${commit}`); From 8b9380396f4041a60af473d22adfb85a6b534fb0 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 08:39:14 -0600 Subject: [PATCH 09/19] Refresh the recorded template commit alongside its copy npm run template:baseline now rewrites MERGED_TEMPLATE_COMMIT itself, so skipping a manual step can no longer leave the check naming a template state it did not compare against. Every failure path reports alike and writes nothing. --- README.md | 2 +- scripts/check-dependency-scope.cjs | 4 +- scripts/refresh-template-baseline.cjs | 77 +++++++++++++++++++++++---- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 20a8f23e..bdf7378d 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ Merging is also what moves the baseline `npm run lint:dependencies` compares dep npm run template:baseline ``` -Run that before fetching the template again, so the baseline records the state you actually merged. Set `MERGED_TEMPLATE_COMMIT` at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs) to the commit it prints. Nothing resolves that id — it is there so the check's output names a template state you can go and look at — so move it and the copy together. +Run that before fetching the template again, so the baseline records the state you actually merged. It writes both the copy and the `MERGED_TEMPLATE_COMMIT` id recorded beside it in [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs), so the two cannot drift apart. Nothing resolves that id — it is there so the check's output names a template state you can go and look at. For more information, read [the instructions on the wiki](https://github.com/paranext/paranext-extension-template/wiki/Merging-Template-Changes-into-Your-Extension). diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index c1c1a92d..64ab78c3 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -32,7 +32,9 @@ const MERGED_TEMPLATE_MANIFEST_PATH = path.join(__dirname, 'merged-template-pack /** * The commit {@link MERGED_TEMPLATE_MANIFEST_PATH} was copied from, recorded so that output names a * template state a reader can go and look at. Nothing resolves it — the copy beside it is what this - * check reads — so the two move together or not at all. + * check reads — so nothing here would catch the two disagreeing. `npm run template:baseline` writes + * both, which is what keeps them in step; set this by hand only to repair a refresh that went + * wrong. */ const MERGED_TEMPLATE_COMMIT = 'c2a2f07ce9faf1674340fba64e069f2e58a0eb09'; diff --git a/scripts/refresh-template-baseline.cjs b/scripts/refresh-template-baseline.cjs index 49aee20b..5e85689d 100644 --- a/scripts/refresh-template-baseline.cjs +++ b/scripts/refresh-template-baseline.cjs @@ -4,16 +4,39 @@ const path = require('path'); /** * Copies the template's `package.json` over the baseline `npm run lint:dependencies` compares - * against, and reports the template commit the copy came from. README's update instructions run - * this in the same commit that merges the template. + * against, and records the commit it came from in {@link CHECK_SCRIPT_PATH}. README's update + * instructions run this in the same commit that merges the template. + * + * Writing both is what holds them together. The copy is what the check reads and the commit is only + * what its output names, so nothing downstream would notice them disagreeing: a refresh that moved + * one and left the other would leave every run naming a template state it had not compared + * against. */ const REPO_ROOT = path.join(__dirname, '..'); const BASELINE_PATH = path.join(__dirname, 'merged-template-package.json'); +const CHECK_SCRIPT_PATH = path.join(__dirname, 'check-dependency-scope.cjs'); /** The template state to copy from — a remote-tracking ref, so it moves only on `git fetch`. */ const TEMPLATE_REF = 'template/main'; +/** + * The recorded commit's assignment in {@link CHECK_SCRIPT_PATH}, matched whole so the rewrite cannot + * land on another hex run in the file — substituting in the wrong place is the one failure this + * script would still report as a success. + */ +const RECORDED_COMMIT_ASSIGNMENT = /^(const MERGED_TEMPLATE_COMMIT = ')[0-9a-f]{40}(';)/m; + +/** + * Reports a failure and stops, in the shape they all share: what went wrong, then what to do about + * it. + */ +function fail(reason, hint) { + console.error(`✗ ${reason}`); + console.error(`ℹ ${hint}`); + process.exit(1); +} + /** * Runs git in the repo root and returns its stdout. * @@ -35,19 +58,51 @@ let commit; try { commit = git('rev-parse', '--verify', TEMPLATE_REF).trim(); } catch (error) { - console.error(`✗ Could not resolve ${TEMPLATE_REF}: ${error.message}`); - console.error( - 'ℹ Adding the template remote is a one-time step after cloning, and its refs need fetching before a merge. README\'s "To update this extension from the template" section has both commands.', + fail( + `Could not resolve ${TEMPLATE_REF}: ${error.message}`, + 'Adding the template remote is a one-time step after cloning, and its refs need fetching before a merge. README\'s "To update this extension from the template" section has both commands.', + ); +} + +const shortCommit = commit.slice(0, 7); + +let manifest; +try { + manifest = git('show', `${commit}:package.json`); +} catch (error) { + fail( + `Could not read package.json from ${TEMPLATE_REF} at ${shortCommit}: ${error.message}`, + `A commit carrying no package.json is not a template commit, so check where the last fetch left ${TEMPLATE_REF}.`, ); - process.exit(1); } -// Ordering is load-bearing: nothing touches the baseline until both git calls have succeeded, and -// the copy comes from the same commit reported below. -const manifest = git('show', `${commit}:package.json`); +let checkScript; +try { + checkScript = fs.readFileSync(CHECK_SCRIPT_PATH, 'utf8'); +} catch (error) { + fail( + `Could not read ${path.relative(REPO_ROOT, CHECK_SCRIPT_PATH)}: ${error.message}`, + 'The baseline and the commit recorded beside it are refreshed together, so this needs both files in place.', + ); +} + +if (!RECORDED_COMMIT_ASSIGNMENT.test(checkScript)) + fail( + `Found no MERGED_TEMPLATE_COMMIT assignment to rewrite in ${path.relative(REPO_ROOT, CHECK_SCRIPT_PATH)}`, + "The rewrite expects that constant to be a 40-character commit id assigned on one line, as in `const MERGED_TEMPLATE_COMMIT = '…';`. Restore that shape, or teach this script the new one.", + ); + +// Ordering is load-bearing: every read above has to succeed before either write below happens, so a +// run that fails leaves the baseline and the recorded commit as they were, and as each other. fs.writeFileSync(BASELINE_PATH, manifest); +fs.writeFileSync( + CHECK_SCRIPT_PATH, + checkScript.replace(RECORDED_COMMIT_ASSIGNMENT, `$1${commit}$2`), +); console.log( - `✓ Copied package.json from ${TEMPLATE_REF} at ${commit.slice(0, 7)} into ${path.relative(REPO_ROOT, BASELINE_PATH)}`, + `✓ Copied package.json from ${TEMPLATE_REF} at ${shortCommit} into ${path.relative(REPO_ROOT, BASELINE_PATH)}`, +); +console.log( + `✓ Recorded ${shortCommit} as MERGED_TEMPLATE_COMMIT in ${path.relative(REPO_ROOT, CHECK_SCRIPT_PATH)}`, ); -console.log(`ℹ Set MERGED_TEMPLATE_COMMIT in check-dependency-scope.cjs to ${commit}`); From 2cea1952d01f433c1a6c75dcc298a7b89cead9d5 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 09:13:12 -0600 Subject: [PATCH 10/19] Fail the dependency check with a hint, not a stack trace An unreadable baseline, manifest, or Dependabot config reached Node's default handler, so the conflict markers a template merge leaves in the baseline arrived with no route back. A config carrying no updates key hits the missing-ecosystem guard instead of throwing a TypeError. --- scripts/check-dependency-scope.cjs | 51 ++++++++++++++++++++++----- scripts/refresh-template-baseline.cjs | 11 +----- scripts/report-failure.cjs | 8 +++++ 3 files changed, 52 insertions(+), 18 deletions(-) create mode 100644 scripts/report-failure.cjs diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 64ab78c3..e225090d 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -1,6 +1,7 @@ const fs = require('fs'); const path = require('path'); const yaml = require('js-yaml'); +const { fail } = require('./report-failure.cjs'); /** * Cross-checks `package.json` against paranext-extension-template's and against @@ -47,6 +48,15 @@ const SHORT_COMMIT = MERGED_TEMPLATE_COMMIT.slice(0, 7); */ const STALE_BASELINE_HINT = `If these came in with a template merge, refresh ${path.basename(MERGED_TEMPLATE_MANIFEST_PATH)} from the template commit that merge brought in, rather than acting on the lines above. Run npm run template:baseline while template/main still points at that commit.`; +const UNREADABLE_BASELINE_HINT = + 'A template merge leaves conflict markers in this copy as readily as in any other file. Resolve them by hand, or run npm run template:baseline while template/main still points at the commit that merge brought in, which rewrites the copy outright.'; + +const UNREADABLE_MANIFEST_HINT = + 'Every npm command reads this file, so the rest of the toolchain is down alongside this check until it is readable again.'; + +const UNREADABLE_DEPENDABOT_CONFIG_HINT = + 'The allow and ignore lists this check holds package.json to live in that file, so there is nothing to check until it is readable.'; + /** * Version ranges this extension deliberately holds apart from the template's. Each entry records * both sides, so it covers that one divergence and no other: change either range and the entry @@ -98,13 +108,21 @@ function readDependencies(manifestPath) { /** * The dependency names the npm ecosystem entry allows and ignores. * - * @throws When the config declares no npm ecosystem, which would otherwise read as an empty scope - * that passes every check. + * @throws When the config is missing or is not YAML, and when it declares no npm ecosystem, which + * would otherwise read as an empty scope that passes every check. */ function readNpmScope() { - const config = yaml.load(fs.readFileSync(DEPENDABOT_CONFIG_PATH, 'utf8')); - const npmEntry = config.updates.find((entry) => entry['package-ecosystem'] === 'npm'); - if (!npmEntry) throw new Error(`No npm ecosystem entry in ${DEPENDABOT_CONFIG_PATH}`); + const configPath = path.relative(REPO_ROOT, DEPENDABOT_CONFIG_PATH); + + let config; + try { + config = yaml.load(fs.readFileSync(DEPENDABOT_CONFIG_PATH, 'utf8')); + } catch (error) { + throw new Error(`Could not read ${configPath}: ${error.message}`); + } + + const npmEntry = config?.updates?.find((entry) => entry['package-ecosystem'] === 'npm'); + if (!npmEntry) throw new Error(`No npm ecosystem entry in ${configPath}`); const dependencyNames = (entries) => (entries ?? []).map((entry) => entry['dependency-name']); return { allow: dependencyNames(npmEntry.allow), ignore: dependencyNames(npmEntry.ignore) }; @@ -163,9 +181,26 @@ function findViolations(ours, template, scope) { return violations; } -const templateDependencies = readDependencies(MERGED_TEMPLATE_MANIFEST_PATH); -const ours = readDependencies(OUR_MANIFEST_PATH); -const violations = findViolations(ours, templateDependencies, readNpmScope()); +/** + * Runs `read`, reporting whatever it throws as a failure with a way out of it. Left to Node's + * default handler, the same message arrives buried in a stack trace and carrying no hint at all. + */ +function readOrFail(read, hint) { + try { + return read(); + } catch (error) { + fail(error.message, hint); + } +} + +const templateDependencies = readOrFail( + () => readDependencies(MERGED_TEMPLATE_MANIFEST_PATH), + UNREADABLE_BASELINE_HINT, +); +const ours = readOrFail(() => readDependencies(OUR_MANIFEST_PATH), UNREADABLE_MANIFEST_HINT); +const scope = readOrFail(readNpmScope, UNREADABLE_DEPENDABOT_CONFIG_HINT); + +const violations = findViolations(ours, templateDependencies, scope); console.log( `Comparing package.json against the template at ${SHORT_COMMIT}, the commit this repo has merged`, diff --git a/scripts/refresh-template-baseline.cjs b/scripts/refresh-template-baseline.cjs index 5e85689d..3f18c238 100644 --- a/scripts/refresh-template-baseline.cjs +++ b/scripts/refresh-template-baseline.cjs @@ -1,6 +1,7 @@ const { execFileSync } = require('child_process'); const fs = require('fs'); const path = require('path'); +const { fail } = require('./report-failure.cjs'); /** * Copies the template's `package.json` over the baseline `npm run lint:dependencies` compares @@ -27,16 +28,6 @@ const TEMPLATE_REF = 'template/main'; */ const RECORDED_COMMIT_ASSIGNMENT = /^(const MERGED_TEMPLATE_COMMIT = ')[0-9a-f]{40}(';)/m; -/** - * Reports a failure and stops, in the shape they all share: what went wrong, then what to do about - * it. - */ -function fail(reason, hint) { - console.error(`✗ ${reason}`); - console.error(`ℹ ${hint}`); - process.exit(1); -} - /** * Runs git in the repo root and returns its stdout. * diff --git a/scripts/report-failure.cjs b/scripts/report-failure.cjs new file mode 100644 index 00000000..d2aa11e5 --- /dev/null +++ b/scripts/report-failure.cjs @@ -0,0 +1,8 @@ +/** Reports a failure and stops: what went wrong, then what to do about it. */ +function fail(reason, hint) { + console.error(`✗ ${reason}`); + console.error(`ℹ ${hint}`); + process.exit(1); +} + +module.exports = { fail }; From 081c9fe7788ba203f64c0138acc4dcf70d419d75 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 12:00:41 -0600 Subject: [PATCH 11/19] Scope the baseline ordering claim to read failures Reads-before-writes leaves the pair in step only when a read fails; a throw between the two writes still desynchronizes them. --- scripts/refresh-template-baseline.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/refresh-template-baseline.cjs b/scripts/refresh-template-baseline.cjs index 3f18c238..65d1a012 100644 --- a/scripts/refresh-template-baseline.cjs +++ b/scripts/refresh-template-baseline.cjs @@ -84,7 +84,7 @@ if (!RECORDED_COMMIT_ASSIGNMENT.test(checkScript)) ); // Ordering is load-bearing: every read above has to succeed before either write below happens, so a -// run that fails leaves the baseline and the recorded commit as they were, and as each other. +// read that fails leaves the baseline and the recorded commit as they were, and as each other. fs.writeFileSync(BASELINE_PATH, manifest); fs.writeFileSync( CHECK_SCRIPT_PATH, From 8f68a5531bae9f0211af23893480dcd3375e746b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 14:22:24 -0600 Subject: [PATCH 12/19] Hold Dependabot's group patterns to the allow list A pattern outliving the package it names went unreported. Allow-list wildcards are now rejected rather than misread as literal names, and a baseline refresh that fails mid-way says which write landed. --- README.md | 2 +- scripts/check-dependency-scope.cjs | 94 +++++++++++++++++++++++---- scripts/refresh-template-baseline.cjs | 32 +++++++-- 3 files changed, 110 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index bdf7378d..90c5d86e 100644 --- a/README.md +++ b/README.md @@ -335,7 +335,7 @@ npm run core:reinstall **Note:** The merge/squash commits created when updating this repo from the template are important; Git uses them to compare the files for future updates. If you edit this repo's Git history, please preserve these commits (do not squash them, for example) to avoid duplicated merge conflicts in the future. -Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml). A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. +Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml), whose grouping patterns it holds to that same allow list so a departing package cannot leave its grouping line behind. Entries on the allow and ignore lists are literal package names: the check reports a wildcard on either rather than guessing at what it covers. A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. ## Special features in this project diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index e225090d..1d7db027 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -7,7 +7,8 @@ const { fail } = require('./report-failure.cjs'); * Cross-checks `package.json` against paranext-extension-template's and against * `.github/dependabot.yml`, enforcing the rule the Dependabot config states in its header: a * package belongs on the allow list if, and only if, it is absent from the template's - * `package.json`. Exits non-zero on any violation. + * `package.json`. The group patterns are held to that allow list in turn, so a package leaving + * takes its grouping line with it. Exits non-zero on any violation. */ const REPO_ROOT = path.join(__dirname, '..'); @@ -44,7 +45,7 @@ const SHORT_COMMIT = MERGED_TEMPLATE_COMMIT.slice(0, 7); /** * A baseline left behind by a template merge reads that merge's own bumps as this extension's, so * every violation comes out inverted and advises undoing the merge. Nothing in the manifests tells - * that case from real drift, so every failure carries the possibility. + * that case from real drift, so every violation the comparison reports carries the possibility. */ const STALE_BASELINE_HINT = `If these came in with a template merge, refresh ${path.basename(MERGED_TEMPLATE_MANIFEST_PATH)} from the template commit that merge brought in, rather than acting on the lines above. Run npm run template:baseline while template/main still points at that commit.`; @@ -55,7 +56,7 @@ const UNREADABLE_MANIFEST_HINT = 'Every npm command reads this file, so the rest of the toolchain is down alongside this check until it is readable again.'; const UNREADABLE_DEPENDABOT_CONFIG_HINT = - 'The allow and ignore lists this check holds package.json to live in that file, so there is nothing to check until it is readable.'; + 'The allow, ignore, and group lists this check holds package.json to live in that file, so there is nothing to check until it is readable.'; /** * Version ranges this extension deliberately holds apart from the template's. Each entry records @@ -106,7 +107,8 @@ function readDependencies(manifestPath) { } /** - * The dependency names the npm ecosystem entry allows and ignores. + * The package names the npm ecosystem entry allows and ignores, and the patterns its groups + * collect, each paired with the group it came from. * * @throws When the config is missing or is not YAML, and when it declares no npm ecosystem, which * would otherwise read as an empty scope that passes every check. @@ -125,19 +127,79 @@ function readNpmScope() { if (!npmEntry) throw new Error(`No npm ecosystem entry in ${configPath}`); const dependencyNames = (entries) => (entries ?? []).map((entry) => entry['dependency-name']); - return { allow: dependencyNames(npmEntry.allow), ignore: dependencyNames(npmEntry.ignore) }; + const groups = Object.entries(npmEntry.groups ?? {}).flatMap(([group, definition]) => + (definition?.patterns ?? []).map((pattern) => ({ group, pattern })), + ); + return { + allow: dependencyNames(npmEntry.allow), + ignore: dependencyNames(npmEntry.ignore), + groups, + }; +} + +/** + * Wildcard entries on the allow and ignore lists, which this check has no matching for: it reads + * each entry on both as one package's name. Dependabot itself accepts wildcards there, so nothing + * else would report one as out of scope. + */ +function findUnsupportedWildcards(scope) { + return [ + { list: 'allow', names: scope.allow }, + { list: 'ignore', names: scope.ignore }, + ].flatMap(({ list, names }) => + names + .filter((name) => name.includes('*')) + .map( + (name) => + `${name}: a wildcard on Dependabot's ${list} list, which this check reads as literal package names — name each package it covers outright, or teach this check to match wildcards`, + ), + ); +} + +/** Whether a package name is one of those a Dependabot group pattern collects. */ +function matchesPattern(pattern, name) { + const anchored = pattern + .replace(/[.+?^${}()|[\]\\]/g, '\\$&') + .split('*') + .join('.*'); + return new RegExp(`^${anchored}$`).test(name); } -/** @returns {string[]} One line per violation; empty when the scoping rule holds. */ -function findViolations(ours, template, scope) { +/** + * Group patterns collecting nothing on the allow list. A group only ever collects updates + * Dependabot is already raising, so a pattern that matches nothing there is a line a departed + * package left behind. + */ +function findStaleGroupPatterns(scope) { + return scope.groups + .filter(({ pattern }) => !scope.allow.some((name) => matchesPattern(pattern, name))) + .map( + ({ group, pattern }) => + `${pattern}: in Dependabot's ${group} group but matches nothing on the allow list, so it groups no update`, + ); +} + +/** + * Where the manifests and the allow list disagree. + * + * @returns {string[]} One line per violation; empty when the scoping rule holds. + */ +function findManifestViolations(ours, template, scope) { const violations = []; const recordedByName = new Map(RECORDED_RANGE_DIVERGENCES.map((entry) => [entry.name, entry])); RECORDED_RANGE_DIVERGENCES.forEach((recorded) => { - if (ours[recorded.name] === recorded.ours && template[recorded.name] === recorded.template) + const ourRange = ours[recorded.name]; + const templateRange = template[recorded.name]; + if (ourRange === recorded.ours && templateRange === recorded.template) return; + if (ourRange === undefined && templateRange === undefined) { + violations.push( + `${recorded.name}: recorded as a divergence but in neither package.json — the entry in ${path.basename(__filename)} outlived the package it covers, so drop it`, + ); return; + } violations.push( - `${recorded.name}: recorded divergence is stale — it records template ${recorded.template} against ours ${recorded.ours} ("${recorded.reason}"), but the manifests now read template ${template[recorded.name] ?? '(absent)'} against ours ${ours[recorded.name] ?? '(absent)'}`, + `${recorded.name}: recorded divergence is stale — it records template ${recorded.template} against ours ${recorded.ours} ("${recorded.reason}"), but the manifests now read template ${templateRange ?? '(absent)'} against ours ${ourRange ?? '(absent)'}`, ); }); @@ -200,7 +262,14 @@ const templateDependencies = readOrFail( const ours = readOrFail(() => readDependencies(OUR_MANIFEST_PATH), UNREADABLE_MANIFEST_HINT); const scope = readOrFail(readNpmScope, UNREADABLE_DEPENDABOT_CONFIG_HINT); -const violations = findViolations(ours, templateDependencies, scope); +const wildcards = findUnsupportedWildcards(scope); + +// A wildcard reaches every other check as one package's name, so each package it covers would come +// out unlisted and the wildcard itself departed. Hold the rest until it is gone. +const configViolations = wildcards.length > 0 ? wildcards : findStaleGroupPatterns(scope); +const manifestViolations = + wildcards.length > 0 ? [] : findManifestViolations(ours, templateDependencies, scope); +const violations = [...configViolations, ...manifestViolations]; console.log( `Comparing package.json against the template at ${SHORT_COMMIT}, the commit this repo has merged`, @@ -215,7 +284,10 @@ if (templateOnly.length > 0) { if (violations.length > 0) { violations.forEach((violation) => console.error(`✗ ${violation}`)); - console.error(`ℹ ${STALE_BASELINE_HINT}`); + // The hint offers a stale baseline as the explanation, and only the comparison reads one. This + // repo owns the Dependabot config outright, so no template merge can have written what the rest + // report. + if (manifestViolations.length > 0) console.error(`ℹ ${STALE_BASELINE_HINT}`); process.exit(1); } diff --git a/scripts/refresh-template-baseline.cjs b/scripts/refresh-template-baseline.cjs index 65d1a012..468454f4 100644 --- a/scripts/refresh-template-baseline.cjs +++ b/scripts/refresh-template-baseline.cjs @@ -26,7 +26,20 @@ const TEMPLATE_REF = 'template/main'; * land on another hex run in the file — substituting in the wrong place is the one failure this * script would still report as a success. */ -const RECORDED_COMMIT_ASSIGNMENT = /^(const MERGED_TEMPLATE_COMMIT = ')[0-9a-f]{40}(';)/m; +const RECORDED_COMMIT_ASSIGNMENT = /^(const MERGED_TEMPLATE_COMMIT = ')([0-9a-f]{40})(';)/m; + +/** + * Writes one half of the refresh, reporting a failure as the state it leaves behind. Left to Node's + * default handler it arrives as a stack trace, which says that a write failed but not which half of + * the pair had already landed — the one disagreement nothing downstream reports. + */ +function writeOrFail(filePath, contents, hint) { + try { + fs.writeFileSync(filePath, contents); + } catch (error) { + fail(`Could not write ${path.relative(REPO_ROOT, filePath)}: ${error.message}`, hint); + } +} /** * Runs git in the repo root and returns its stdout. @@ -77,18 +90,25 @@ try { ); } -if (!RECORDED_COMMIT_ASSIGNMENT.test(checkScript)) +const recordedAssignment = checkScript.match(RECORDED_COMMIT_ASSIGNMENT); +if (!recordedAssignment) fail( `Found no MERGED_TEMPLATE_COMMIT assignment to rewrite in ${path.relative(REPO_ROOT, CHECK_SCRIPT_PATH)}`, "The rewrite expects that constant to be a 40-character commit id assigned on one line, as in `const MERGED_TEMPLATE_COMMIT = '…';`. Restore that shape, or teach this script the new one.", ); // Ordering is load-bearing: every read above has to succeed before either write below happens, so a -// read that fails leaves the baseline and the recorded commit as they were, and as each other. -fs.writeFileSync(BASELINE_PATH, manifest); -fs.writeFileSync( +// read that fails leaves the baseline and the recorded commit as they were, and as each other. The +// writes cannot be given that same guarantee. +writeOrFail( + BASELINE_PATH, + manifest, + 'Nothing was written, so the baseline and the recorded commit are still as they were and still in step. Re-running once the write can succeed is the whole repair.', +); +writeOrFail( CHECK_SCRIPT_PATH, - checkScript.replace(RECORDED_COMMIT_ASSIGNMENT, `$1${commit}$2`), + checkScript.replace(RECORDED_COMMIT_ASSIGNMENT, `$1${commit}$3`), + `Half of the refresh landed: ${path.relative(REPO_ROOT, BASELINE_PATH)} now holds the package.json from ${shortCommit}, while MERGED_TEMPLATE_COMMIT still records ${recordedAssignment[2].slice(0, 7)}. Re-run once the write can succeed, or check out ${path.relative(REPO_ROOT, BASELINE_PATH)} again to put the pair back in step.`, ); console.log( From 149a3811472fa19be8c2c38f567849be835f1edf Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 14:36:47 -0600 Subject: [PATCH 13/19] Group js-yaml with the rest of our dev tooling Renames the group to match its contents, and syncs the template manifest copy into the other two ignore files. --- .eslintignore | 4 ++++ .github/dependabot.yml | 9 ++++++--- .prettierignore | 2 +- .stylelintignore | 4 ++++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.eslintignore b/.eslintignore index 687a559a..e8c22a16 100644 --- a/.eslintignore +++ b/.eslintignore @@ -41,3 +41,7 @@ package-lock.json # Playwright test output e2e-tests/playwright-report e2e-tests/test-results + +# A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain +# `git show` redirect rather than a copy plus whatever our tooling would impose on it +scripts/merged-template-package.json diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 9015369a..525cddb7 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -44,9 +44,11 @@ updates: - dependency-name: ts-jest - dependency-name: ws groups: - # Test tooling moves together, so one PR per cycle rather than one per - # package. Major bumps stay ungrouped: those are worth reading on their own. - test-tooling: + # The tooling we add on top of the template moves together, so one PR per + # cycle rather than one per package. Runtime dependencies stay out: those + # ship to users. Major bumps stay ungrouped too, being worth reading on + # their own. + dev-tooling: applies-to: version-updates update-types: ['minor', 'patch'] patterns: @@ -57,6 +59,7 @@ updates: - eslint-plugin-jest - jest - jest-environment-jsdom + - js-yaml - ts-jest - ws diff --git a/.prettierignore b/.prettierignore index a418d8f5..66606e39 100644 --- a/.prettierignore +++ b/.prettierignore @@ -43,5 +43,5 @@ e2e-tests/playwright-report e2e-tests/test-results # A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain -# `git show` redirect rather than a copy plus whatever reformatting our config would impose +# `git show` redirect rather than a copy plus whatever our tooling would impose on it scripts/merged-template-package.json diff --git a/.stylelintignore b/.stylelintignore index 69c75a68..15d7b339 100644 --- a/.stylelintignore +++ b/.stylelintignore @@ -41,3 +41,7 @@ package-lock.json # Playwright test output e2e-tests/playwright-report e2e-tests/test-results + +# A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain +# `git show` redirect rather than a copy plus whatever our tooling would impose on it +scripts/merged-template-package.json From 939788c3428a64c32bfc86808c8947bf6b02e4bd Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Tue, 18 Aug 2026 14:55:50 -0600 Subject: [PATCH 14/19] Require the dev dependencies we add to be grouped The check verified that each group pattern matched something on the allow list, but not the reverse, so an addition could drift out of the group as js-yaml had. --- README.md | 2 +- scripts/check-dependency-scope.cjs | 46 +++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 90c5d86e..4dfe42f5 100644 --- a/README.md +++ b/README.md @@ -335,7 +335,7 @@ npm run core:reinstall **Note:** The merge/squash commits created when updating this repo from the template are important; Git uses them to compare the files for future updates. If you edit this repo's Git history, please preserve these commits (do not squash them, for example) to avoid duplicated merge conflicts in the future. -Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml), whose grouping patterns it holds to that same allow list so a departing package cannot leave its grouping line behind. Entries on the allow and ignore lists are literal package names: the check reports a wildcard on either rather than guessing at what it covers. A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. +Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml), whose grouping patterns and allow list it holds to each other: a departing package cannot leave its grouping line behind, and a dev dependency this extension adds cannot slip out of the group that spares it a pull request of its own. Entries on the allow and ignore lists are literal package names: the check reports a wildcard on either rather than guessing at what it covers. A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. ## Special features in this project diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 1d7db027..6337823c 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -7,8 +7,9 @@ const { fail } = require('./report-failure.cjs'); * Cross-checks `package.json` against paranext-extension-template's and against * `.github/dependabot.yml`, enforcing the rule the Dependabot config states in its header: a * package belongs on the allow list if, and only if, it is absent from the template's - * `package.json`. The group patterns are held to that allow list in turn, so a package leaving - * takes its grouping line with it. Exits non-zero on any violation. + * `package.json`. The group patterns and that allow list are held to each other in turn, so a + * package leaving takes its grouping line with it and an added dev dependency cannot slip out of + * the group that spares it a pull request of its own. Exits non-zero on any violation. */ const REPO_ROOT = path.join(__dirname, '..'); @@ -98,9 +99,9 @@ function collectDependencies(manifest) { * @throws When the manifest is missing or is not JSON, naming the file — the baseline copy collects * conflict markers on a template merge as readily as any other file does. */ -function readDependencies(manifestPath) { +function readManifest(manifestPath) { try { - return collectDependencies(JSON.parse(fs.readFileSync(manifestPath, 'utf8'))); + return JSON.parse(fs.readFileSync(manifestPath, 'utf8')); } catch (error) { throw new Error(`Could not read ${path.relative(REPO_ROOT, manifestPath)}: ${error.message}`); } @@ -179,6 +180,25 @@ function findStaleGroupPatterns(scope) { ); } +/** + * Allowed dev dependencies that no group collects. Grouping is what holds this extension's tooling + * to one pull request a cycle, and nothing about a package announces that it was meant to be in a + * group, so an addition drifts outside every pattern silently. + */ +function findUngroupedDevDependencies(manifest, scope) { + const devDependencies = manifest.devDependencies ?? {}; + return scope.allow + .filter( + (name) => + name in devDependencies && + !scope.groups.some(({ pattern }) => matchesPattern(pattern, name)), + ) + .map( + (name) => + `${name}: a dev dependency on the allow list that no group collects, so its updates arrive as a pull request of their own — add it to a group, or record here why it stays out`, + ); +} + /** * Where the manifests and the allow list disagree. * @@ -255,18 +275,24 @@ function readOrFail(read, hint) { } } -const templateDependencies = readOrFail( - () => readDependencies(MERGED_TEMPLATE_MANIFEST_PATH), +const templateManifest = readOrFail( + () => readManifest(MERGED_TEMPLATE_MANIFEST_PATH), UNREADABLE_BASELINE_HINT, ); -const ours = readOrFail(() => readDependencies(OUR_MANIFEST_PATH), UNREADABLE_MANIFEST_HINT); +const ourManifest = readOrFail(() => readManifest(OUR_MANIFEST_PATH), UNREADABLE_MANIFEST_HINT); const scope = readOrFail(readNpmScope, UNREADABLE_DEPENDABOT_CONFIG_HINT); +const templateDependencies = collectDependencies(templateManifest); +const ours = collectDependencies(ourManifest); + const wildcards = findUnsupportedWildcards(scope); -// A wildcard reaches every other check as one package's name, so each package it covers would come -// out unlisted and the wildcard itself departed. Hold the rest until it is gone. -const configViolations = wildcards.length > 0 ? wildcards : findStaleGroupPatterns(scope); +// The checks below read the allow list as literal names, so a wildcard leaves every package it +// covers unaccounted for and the wildcard itself looking departed. Hold them until it is gone. +const configViolations = + wildcards.length > 0 + ? wildcards + : [...findStaleGroupPatterns(scope), ...findUngroupedDevDependencies(ourManifest, scope)]; const manifestViolations = wildcards.length > 0 ? [] : findManifestViolations(ours, templateDependencies, scope); const violations = [...configViolations, ...manifestViolations]; From d0196375aebdb4aa33e7df3c1e0f63a2cb86b14b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 09:22:34 -0600 Subject: [PATCH 15/19] Report unreadable Dependabot entries, not a stack trace An entry naming no package crashed the check on `undefined`; an ignore entry whose package has left package.json now says so. --- README.md | 2 +- scripts/check-dependency-scope.cjs | 48 ++++++++++++++++++------------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 4dfe42f5..5197d410 100644 --- a/README.md +++ b/README.md @@ -335,7 +335,7 @@ npm run core:reinstall **Note:** The merge/squash commits created when updating this repo from the template are important; Git uses them to compare the files for future updates. If you edit this repo's Git history, please preserve these commits (do not squash them, for example) to avoid duplicated merge conflicts in the future. -Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml), whose grouping patterns and allow list it holds to each other: a departing package cannot leave its grouping line behind, and a dev dependency this extension adds cannot slip out of the group that spares it a pull request of its own. Entries on the allow and ignore lists are literal package names: the check reports a wildcard on either rather than guessing at what it covers. A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. +Dependabot covers only the packages this extension adds on top of the template, so that its updates never move a template-owned dependency ahead of the template. `npm run lint:dependencies` enforces that split: it compares `package.json` against the template's and reports any package whose version range has drifted, plus any mismatch between the extension's own packages and the allow list in [`.github/dependabot.yml`](.github/dependabot.yml), whose grouping patterns and allow list it holds to each other: a departing package cannot leave its grouping line behind, and a dev dependency this extension adds cannot slip out of the group that spares it a pull request of its own. Entries on the allow and ignore lists are literal package names: the check reports an entry it cannot read that way — a wildcard, or one selecting packages by dependency type — rather than guessing at what it covers. A version range this extension holds apart from the template's on purpose belongs in the recorded list at the top of [`scripts/check-dependency-scope.cjs`](scripts/check-dependency-scope.cjs). The template side of the comparison is [`scripts/merged-template-package.json`](scripts/merged-template-package.json), a verbatim copy of the template's manifest as of the commit this repo last merged, so a range difference means this extension moved the range: bumps the template makes between merges are ours to pick up at the next merge rather than a lint failure to fix now. Only a template merge moves that baseline; Dependabot's own updates never do, because its allow list covers only packages the template does not own. ## Special features in this project diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 6337823c..9bbc97e4 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -111,6 +111,10 @@ function readManifest(manifestPath) { * The package names the npm ecosystem entry allows and ignores, and the patterns its groups * collect, each paired with the group it came from. * + * An entry naming no package — Dependabot also selects by dependency type — leaves `undefined` in + * its place on the list rather than being dropped, since dropping it would widen the scope this + * check reads package.json against without saying so. + * * @throws When the config is missing or is not YAML, and when it declares no npm ecosystem, which * would otherwise read as an empty scope that passes every check. */ @@ -139,21 +143,23 @@ function readNpmScope() { } /** - * Wildcard entries on the allow and ignore lists, which this check has no matching for: it reads - * each entry on both as one package's name. Dependabot itself accepts wildcards there, so nothing - * else would report one as out of scope. + * Entries on the allow and ignore lists that this check cannot read as one package's name: + * wildcards, which it has no matching for, and entries naming no package at all. Dependabot accepts + * them, so nothing else would report one as out of scope, and an entry selecting packages by type + * covers a set this check cannot hold package.json to. */ -function findUnsupportedWildcards(scope) { +function findUnsupportedEntries(scope) { return [ { list: 'allow', names: scope.allow }, { list: 'ignore', names: scope.ignore }, ].flatMap(({ list, names }) => - names - .filter((name) => name.includes('*')) - .map( - (name) => - `${name}: a wildcard on Dependabot's ${list} list, which this check reads as literal package names — name each package it covers outright, or teach this check to match wildcards`, - ), + names.flatMap((name, index) => { + if (name === undefined) + return `${list} entry ${index + 1}: no dependency-name, so it names no package — name each package it covers outright, or teach this check to read the entry`; + if (name.includes('*')) + return `${name}: a wildcard on Dependabot's ${list} list, which this check reads as literal package names — name each package it covers outright, or teach this check to match wildcards`; + return []; + }), ); } @@ -255,9 +261,12 @@ function findManifestViolations(ours, template, scope) { scope.ignore.forEach((name) => { if (isFileDependency(ours[name])) return; - violations.push( - `${name}: on Dependabot's ignore list, which exists for file: dependencies — an ignore entry with another purpose needs this check updated`, - ); + if (!(name in ours)) + violations.push(`${name}: on Dependabot's ignore list but no longer in package.json`); + else + violations.push( + `${name}: on Dependabot's ignore list, which exists for file: dependencies — an ignore entry with another purpose needs this check updated`, + ); }); return violations; @@ -285,16 +294,17 @@ const scope = readOrFail(readNpmScope, UNREADABLE_DEPENDABOT_CONFIG_HINT); const templateDependencies = collectDependencies(templateManifest); const ours = collectDependencies(ourManifest); -const wildcards = findUnsupportedWildcards(scope); +const unsupportedEntries = findUnsupportedEntries(scope); -// The checks below read the allow list as literal names, so a wildcard leaves every package it -// covers unaccounted for and the wildcard itself looking departed. Hold them until it is gone. +// The checks below read the allow and ignore lists as literal names, so an entry they cannot read +// leaves every package it covers unaccounted for and the entry itself looking departed. Hold them +// until it is gone. const configViolations = - wildcards.length > 0 - ? wildcards + unsupportedEntries.length > 0 + ? unsupportedEntries : [...findStaleGroupPatterns(scope), ...findUngroupedDevDependencies(ourManifest, scope)]; const manifestViolations = - wildcards.length > 0 ? [] : findManifestViolations(ours, templateDependencies, scope); + unsupportedEntries.length > 0 ? [] : findManifestViolations(ours, templateDependencies, scope); const violations = [...configViolations, ...manifestViolations]; console.log( From 3b400677a031698f73847e532ce52ca48539cd12 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 09:56:41 -0600 Subject: [PATCH 16/19] Hold the check to one npm ecosystem entry A second entry scopes another directory's package.json, so merging its lists would misreport the root manifest. The baseline and allow-list comments now name `npm run template:baseline` and the `file:` carve-out they left out. --- .eslintignore | 4 ++-- .github/dependabot.yml | 5 ++-- .prettierignore | 4 ++-- .stylelintignore | 4 ++-- AGENTS.md | 2 +- scripts/check-dependency-scope.cjs | 37 ++++++++++++++++++++---------- 6 files changed, 35 insertions(+), 21 deletions(-) diff --git a/.eslintignore b/.eslintignore index e8c22a16..cbea34f2 100644 --- a/.eslintignore +++ b/.eslintignore @@ -42,6 +42,6 @@ package-lock.json e2e-tests/playwright-report e2e-tests/test-results -# A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain -# `git show` redirect rather than a copy plus whatever our tooling would impose on it +# A verbatim copy of the template's manifest, kept byte-for-byte so `npm run template:baseline` can +# write it straight from `git show` rather than a copy plus whatever our tooling would impose on it scripts/merged-template-package.json diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 525cddb7..72ea6a66 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -6,8 +6,9 @@ # as well as version updates, so nothing template-owned is touched either way. # # Keep the allow list in sync with package.json: an entry belongs here if, and -# only if, it is absent from the template's package.json. `npm run -# lint:dependencies` checks that. +# only if, it is absent from the template's package.json. The `file:` +# dependencies are the exception: the ignore list below skips them instead, for +# the reason recorded beside it. `npm run lint:dependencies` checks that. version: 2 updates: - package-ecosystem: npm diff --git a/.prettierignore b/.prettierignore index 66606e39..53cf3e5e 100644 --- a/.prettierignore +++ b/.prettierignore @@ -42,6 +42,6 @@ package-lock.json e2e-tests/playwright-report e2e-tests/test-results -# A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain -# `git show` redirect rather than a copy plus whatever our tooling would impose on it +# A verbatim copy of the template's manifest, kept byte-for-byte so `npm run template:baseline` can +# write it straight from `git show` rather than a copy plus whatever our tooling would impose on it scripts/merged-template-package.json diff --git a/.stylelintignore b/.stylelintignore index 15d7b339..3214a7f1 100644 --- a/.stylelintignore +++ b/.stylelintignore @@ -42,6 +42,6 @@ package-lock.json e2e-tests/playwright-report e2e-tests/test-results -# A verbatim copy of the template's manifest, kept byte-for-byte so refreshing it is a plain -# `git show` redirect rather than a copy plus whatever our tooling would impose on it +# A verbatim copy of the template's manifest, kept byte-for-byte so `npm run template:baseline` can +# write it straight from `git show` rather than a copy plus whatever our tooling would impose on it scripts/merged-template-package.json diff --git a/AGENTS.md b/AGENTS.md index 8a50af20..4e0ff592 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,7 @@ npm test -- path/to/file.test.ts # Run a single test file npm test -- --testNamePattern="pattern" # Run tests matching name ``` -Only a template merge moves the dependency baseline `npm run lint:dependencies` checks against; refresh it in that same commit with `npm run template:baseline` and point `MERGED_TEMPLATE_COMMIT` in [scripts/check-dependency-scope.cjs](scripts/check-dependency-scope.cjs) at the merged commit. [README.md](README.md) has the full procedure. +Only a template merge moves the dependency baseline `npm run lint:dependencies` checks against; refresh it in that same commit with `npm run template:baseline`, which writes both the copy and the `MERGED_TEMPLATE_COMMIT` id recorded beside it in [scripts/check-dependency-scope.cjs](scripts/check-dependency-scope.cjs). [README.md](README.md) has the full procedure. ## Architecture diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 9bbc97e4..1aa96ef1 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -7,9 +7,10 @@ const { fail } = require('./report-failure.cjs'); * Cross-checks `package.json` against paranext-extension-template's and against * `.github/dependabot.yml`, enforcing the rule the Dependabot config states in its header: a * package belongs on the allow list if, and only if, it is absent from the template's - * `package.json`. The group patterns and that allow list are held to each other in turn, so a - * package leaving takes its grouping line with it and an added dev dependency cannot slip out of - * the group that spares it a pull request of its own. Exits non-zero on any violation. + * `package.json` — `file:` dependencies excepted, since Dependabot skips those by way of its ignore + * list instead. The group patterns and that allow list are held to each other in turn, so a package + * leaving takes its grouping line with it and an added dev dependency cannot slip out of the group + * that spares it a pull request of its own. Exits non-zero on any violation. */ const REPO_ROOT = path.join(__dirname, '..'); @@ -56,8 +57,8 @@ const UNREADABLE_BASELINE_HINT = const UNREADABLE_MANIFEST_HINT = 'Every npm command reads this file, so the rest of the toolchain is down alongside this check until it is readable again.'; -const UNREADABLE_DEPENDABOT_CONFIG_HINT = - 'The allow, ignore, and group lists this check holds package.json to live in that file, so there is nothing to check until it is readable.'; +const UNSCOPED_DEPENDABOT_CONFIG_HINT = + 'The allow, ignore, and group lists this check holds package.json to live in that file, so there is nothing to check until one npm ecosystem entry there declares them.'; /** * Version ranges this extension deliberately holds apart from the template's. Each entry records @@ -115,8 +116,9 @@ function readManifest(manifestPath) { * its place on the list rather than being dropped, since dropping it would widen the scope this * check reads package.json against without saying so. * - * @throws When the config is missing or is not YAML, and when it declares no npm ecosystem, which - * would otherwise read as an empty scope that passes every check. + * @throws When the config is missing or is not YAML, and when it declares anything other than one + * npm ecosystem: none would read as an empty scope that passes every check, and a second scopes + * another directory's package.json, which is not the manifest this check opens. */ function readNpmScope() { const configPath = path.relative(REPO_ROOT, DEPENDABOT_CONFIG_PATH); @@ -128,8 +130,15 @@ function readNpmScope() { throw new Error(`Could not read ${configPath}: ${error.message}`); } - const npmEntry = config?.updates?.find((entry) => entry['package-ecosystem'] === 'npm'); - if (!npmEntry) throw new Error(`No npm ecosystem entry in ${configPath}`); + const npmEntries = (config?.updates ?? []).filter( + (entry) => entry['package-ecosystem'] === 'npm', + ); + if (npmEntries.length === 0) throw new Error(`No npm ecosystem entry in ${configPath}`); + if (npmEntries.length > 1) + throw new Error( + `${npmEntries.length} npm ecosystem entries in ${configPath}, and this check reads one — name the directory each covers here, or teach this check to pick out the one scoping the package.json beside it`, + ); + const [npmEntry] = npmEntries; const dependencyNames = (entries) => (entries ?? []).map((entry) => entry['dependency-name']); const groups = Object.entries(npmEntry.groups ?? {}).flatMap(([group, definition]) => @@ -163,7 +172,12 @@ function findUnsupportedEntries(scope) { ); } -/** Whether a package name is one of those a Dependabot group pattern collects. */ +/** + * Whether a package name is one of those a Dependabot group pattern collects. `*` is the only + * wildcard read and matching is case-sensitive, where Dependabot also takes `?` and character + * classes and folds case, so a pattern relying on any of that collects less here than it does + * there. + */ function matchesPattern(pattern, name) { const anchored = pattern .replace(/[.+?^${}()|[\]\\]/g, '\\$&') @@ -289,7 +303,7 @@ const templateManifest = readOrFail( UNREADABLE_BASELINE_HINT, ); const ourManifest = readOrFail(() => readManifest(OUR_MANIFEST_PATH), UNREADABLE_MANIFEST_HINT); -const scope = readOrFail(readNpmScope, UNREADABLE_DEPENDABOT_CONFIG_HINT); +const scope = readOrFail(readNpmScope, UNSCOPED_DEPENDABOT_CONFIG_HINT); const templateDependencies = collectDependencies(templateManifest); const ours = collectDependencies(ourManifest); @@ -328,4 +342,3 @@ if (violations.length > 0) { } console.log('✓ Dependabot scope matches the packages this extension adds to the template'); -process.exit(0); From da1914fa5f873e492684e22f515ccb3489b1c33c Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 11:29:46 -0600 Subject: [PATCH 17/19] Reject a package on both the allow and ignore lists Also record why peerDependencies stay in the comparison, and the inert allow entry a peer dependency beyond the template would be asked for. --- scripts/check-dependency-scope.cjs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 1aa96ef1..68d1643e 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -91,6 +91,12 @@ function isFileDependency(range) { * The manifest's dependency sections merged, since Dependabot scopes them as one npm ecosystem. * `overrides` stays out: it pins transitive versions rather than naming packages this extension * depends on, so the allow-list rule has nothing to say about it. + * + * `peerDependencies` stays in, so a package declared only there is still held to the template's + * range. It reads the allow-list rule wrong in return: Dependabot's npm updater raises no version + * update for a peer range, so a peer dependency beyond the template would be asked for an allow + * entry that can raise nothing. Nothing declares one; exempt the section from that rule alone if + * anything ever does. */ function collectDependencies(manifest) { return { ...manifest.dependencies, ...manifest.devDependencies, ...manifest.peerDependencies }; @@ -200,6 +206,20 @@ function findStaleGroupPatterns(scope) { ); } +/** + * Packages named on both the allow and the ignore list. Dependabot filters the allow list through + * the ignore list, so the pair leaves the allow entry raising nothing while reading as though the + * package were still receiving updates. + */ +function findPackagesOnBothLists(scope) { + return scope.allow + .filter((name) => scope.ignore.includes(name)) + .map( + (name) => + `${name}: on both Dependabot's allow and ignore lists, and ignoring wins, so the allow entry raises nothing — drop whichever of the two the package does not need`, + ); +} + /** * Allowed dev dependencies that no group collects. Grouping is what holds this extension's tooling * to one pull request a cycle, and nothing about a package announces that it was meant to be in a @@ -316,7 +336,11 @@ const unsupportedEntries = findUnsupportedEntries(scope); const configViolations = unsupportedEntries.length > 0 ? unsupportedEntries - : [...findStaleGroupPatterns(scope), ...findUngroupedDevDependencies(ourManifest, scope)]; + : [ + ...findPackagesOnBothLists(scope), + ...findStaleGroupPatterns(scope), + ...findUngroupedDevDependencies(ourManifest, scope), + ]; const manifestViolations = unsupportedEntries.length > 0 ? [] : findManifestViolations(ours, templateDependencies, scope); const violations = [...configViolations, ...manifestViolations]; From 1ae6606c049623ef09902e9e45b59281f20526c2 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 15:23:34 -0600 Subject: [PATCH 18/19] Report failures with a synchronous write `process.exit` does not wait for an asynchronous one, and stderr is asynchronous on a Windows terminal and on a POSIX pipe. Also match a recorded commit id at either object-format width so a refresh can read back what it wrote, fit the scope hint to every way that read fails, and drop a hardcoded count from the config's comment. --- .github/dependabot.yml | 2 +- scripts/check-dependency-scope.cjs | 10 +++++----- scripts/refresh-template-baseline.cjs | 7 ++++--- scripts/report-failure.cjs | 21 +++++++++++++++++---- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 72ea6a66..4f9033e8 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -15,7 +15,7 @@ updates: directory: / schedule: interval: monthly - # The three `file:` dependencies resolve against a sibling paranext-core + # The `file:` dependencies resolve against a sibling paranext-core # checkout, which exists on developer machines and in CI but not inside # Dependabot's container. npm's file fetcher resolves path dependencies # before any update is considered, and it consults `ignore` — never `allow` — diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 68d1643e..6ac4e7e5 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -1,7 +1,7 @@ const fs = require('fs'); const path = require('path'); const yaml = require('js-yaml'); -const { fail } = require('./report-failure.cjs'); +const { fail, failWith } = require('./report-failure.cjs'); /** * Cross-checks `package.json` against paranext-extension-template's and against @@ -58,7 +58,7 @@ const UNREADABLE_MANIFEST_HINT = 'Every npm command reads this file, so the rest of the toolchain is down alongside this check until it is readable again.'; const UNSCOPED_DEPENDABOT_CONFIG_HINT = - 'The allow, ignore, and group lists this check holds package.json to live in that file, so there is nothing to check until one npm ecosystem entry there declares them.'; + 'The allow, ignore, and group lists this check holds package.json to live in that file, declared by the one npm ecosystem entry scoping the directory package.json sits in.'; /** * Version ranges this extension deliberately holds apart from the template's. Each entry records @@ -357,12 +357,12 @@ if (templateOnly.length > 0) { } if (violations.length > 0) { - violations.forEach((violation) => console.error(`✗ ${violation}`)); + const lines = violations.map((violation) => `✗ ${violation}`); // The hint offers a stale baseline as the explanation, and only the comparison reads one. This // repo owns the Dependabot config outright, so no template merge can have written what the rest // report. - if (manifestViolations.length > 0) console.error(`ℹ ${STALE_BASELINE_HINT}`); - process.exit(1); + if (manifestViolations.length > 0) lines.push(`ℹ ${STALE_BASELINE_HINT}`); + failWith(lines); } console.log('✓ Dependabot scope matches the packages this extension adds to the template'); diff --git a/scripts/refresh-template-baseline.cjs b/scripts/refresh-template-baseline.cjs index 468454f4..ca2b84f1 100644 --- a/scripts/refresh-template-baseline.cjs +++ b/scripts/refresh-template-baseline.cjs @@ -24,9 +24,10 @@ const TEMPLATE_REF = 'template/main'; /** * The recorded commit's assignment in {@link CHECK_SCRIPT_PATH}, matched whole so the rewrite cannot * land on another hex run in the file — substituting in the wrong place is the one failure this - * script would still report as a success. + * script would still report as a success. The id is matched at either git object-format width, so + * what one run writes the next can still find. */ -const RECORDED_COMMIT_ASSIGNMENT = /^(const MERGED_TEMPLATE_COMMIT = ')([0-9a-f]{40})(';)/m; +const RECORDED_COMMIT_ASSIGNMENT = /^(const MERGED_TEMPLATE_COMMIT = ')([0-9a-f]{40,64})(';)/m; /** * Writes one half of the refresh, reporting a failure as the state it leaves behind. Left to Node's @@ -94,7 +95,7 @@ const recordedAssignment = checkScript.match(RECORDED_COMMIT_ASSIGNMENT); if (!recordedAssignment) fail( `Found no MERGED_TEMPLATE_COMMIT assignment to rewrite in ${path.relative(REPO_ROOT, CHECK_SCRIPT_PATH)}`, - "The rewrite expects that constant to be a 40-character commit id assigned on one line, as in `const MERGED_TEMPLATE_COMMIT = '…';`. Restore that shape, or teach this script the new one.", + "The rewrite expects that constant to be a commit id assigned on one line, as in `const MERGED_TEMPLATE_COMMIT = '…';`. Restore that shape, or teach this script the new one.", ); // Ordering is load-bearing: every read above has to succeed before either write below happens, so a diff --git a/scripts/report-failure.cjs b/scripts/report-failure.cjs index d2aa11e5..06c1cb12 100644 --- a/scripts/report-failure.cjs +++ b/scripts/report-failure.cjs @@ -1,8 +1,21 @@ +const fs = require('fs'); + +/** + * Reports a multi-line failure and stops. + * + * @param lines - Written as given, so each carries its own ✗ or ℹ marker. + */ +function failWith(lines) { + // Written synchronously because `process.exit` does not wait for an asynchronous write, and + // stderr is asynchronous on a Windows terminal and, by contract, on a POSIX pipe — which is what + // it is whenever CI captures a run. A report cut short names fewer problems than were found. + fs.writeSync(2, `${lines.join('\n')}\n`); + process.exit(1); +} + /** Reports a failure and stops: what went wrong, then what to do about it. */ function fail(reason, hint) { - console.error(`✗ ${reason}`); - console.error(`ℹ ${hint}`); - process.exit(1); + failWith([`✗ ${reason}`, `ℹ ${hint}`]); } -module.exports = { fail }; +module.exports = { fail, failWith }; From 562a0738868cb886fa3f50d7348edb0cf7f824d0 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 19 Aug 2026 15:51:37 -0600 Subject: [PATCH 19/19] Report Dependabot group keys this check cannot read `findUngroupedDevDependencies` reads a group as the packages its patterns collect. Dependabot also narrows a group by `exclude-patterns` and by `dependency-type`, and either one leaves a package the group drops still reading as collected, so the check that exists to catch an ungrouped dev dependency would pass over it in silence. Nothing declares either key today, which is what makes it a blind spot rather than a miss: the guarantee would lapse quietly the first time one arrived. Report any key beyond those the check reads, in the shape `findUnsupportedEntries` already uses for an allow or ignore entry it cannot read. `applies-to` and `update-types` stay unreported: both are declared today and narrow which updates a group collects rather than which packages, and packages are the axis the group checks reason about. An unread key holds back the two checks that open a group and no others, since the rest never read one. Both read the allow list as literal names too, so an unreadable allow entry still holds them as before. --- scripts/check-dependency-scope.cjs | 50 ++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/scripts/check-dependency-scope.cjs b/scripts/check-dependency-scope.cjs index 6ac4e7e5..42836222 100644 --- a/scripts/check-dependency-scope.cjs +++ b/scripts/check-dependency-scope.cjs @@ -115,8 +115,8 @@ function readManifest(manifestPath) { } /** - * The package names the npm ecosystem entry allows and ignores, and the patterns its groups - * collect, each paired with the group it came from. + * The package names the npm ecosystem entry allows and ignores, the patterns its groups collect, + * and the keys those groups declare, the last two each paired with the group they came from. * * An entry naming no package — Dependabot also selects by dependency type — leaves `undefined` in * its place on the list rather than being dropped, since dropping it would widen the scope this @@ -147,13 +147,18 @@ function readNpmScope() { const [npmEntry] = npmEntries; const dependencyNames = (entries) => (entries ?? []).map((entry) => entry['dependency-name']); - const groups = Object.entries(npmEntry.groups ?? {}).flatMap(([group, definition]) => + const definitions = Object.entries(npmEntry.groups ?? {}); + const groups = definitions.flatMap(([group, definition]) => (definition?.patterns ?? []).map((pattern) => ({ group, pattern })), ); + const groupKeys = definitions.flatMap(([group, definition]) => + Object.keys(definition ?? {}).map((key) => ({ group, key })), + ); return { allow: dependencyNames(npmEntry.allow), ignore: dependencyNames(npmEntry.ignore), groups, + groupKeys, }; } @@ -178,6 +183,28 @@ function findUnsupportedEntries(scope) { ); } +/** + * The group keys that leave a group readable by its patterns alone: the one this check reads, and + * those narrowing which updates a group collects rather than which packages. + */ +const READABLE_GROUP_KEYS = ['patterns', 'applies-to', 'update-types']; + +/** + * Keys a group declares that this check cannot read, each named with the group declaring it. + * Dependabot accepts more than {@link READABLE_GROUP_KEYS}, and a key narrowing which packages a + * group collects — `exclude-patterns` and `dependency-type` both do — leaves a package the group + * drops reading as collected, which {@link findUngroupedDevDependencies} would then pass over in + * silence. + */ +function findUnsupportedGroupKeys(scope) { + return scope.groupKeys + .filter(({ key }) => !READABLE_GROUP_KEYS.includes(key)) + .map( + ({ group, key }) => + `${key}: declared by Dependabot's ${group} group, and this check reads a group as the packages its patterns collect — a key narrowing that further leaves a package the group drops reading as grouped, so teach this check to read it, or drop the key`, + ); +} + /** * Whether a package name is one of those a Dependabot group pattern collects. `*` is the only * wildcard read and matching is case-sensitive, where Dependabot also takes `?` and character @@ -329,18 +356,23 @@ const templateDependencies = collectDependencies(templateManifest); const ours = collectDependencies(ourManifest); const unsupportedEntries = findUnsupportedEntries(scope); +const unsupportedGroupKeys = findUnsupportedGroupKeys(scope); + +// The checks here read a group as the packages its patterns collect, so a key narrowing that +// further leaves them reporting on a membership the group does not have. They read the allow list +// as literal names too, so an entry that cannot be read holds them back as well. +const groupViolations = + unsupportedGroupKeys.length > 0 + ? unsupportedGroupKeys + : [...findStaleGroupPatterns(scope), ...findUngroupedDevDependencies(ourManifest, scope)]; // The checks below read the allow and ignore lists as literal names, so an entry they cannot read // leaves every package it covers unaccounted for and the entry itself looking departed. Hold them // until it is gone. const configViolations = unsupportedEntries.length > 0 - ? unsupportedEntries - : [ - ...findPackagesOnBothLists(scope), - ...findStaleGroupPatterns(scope), - ...findUngroupedDevDependencies(ourManifest, scope), - ]; + ? [...unsupportedEntries, ...unsupportedGroupKeys] + : [...findPackagesOnBothLists(scope), ...groupViolations]; const manifestViolations = unsupportedEntries.length > 0 ? [] : findManifestViolations(ours, templateDependencies, scope); const violations = [...configViolations, ...manifestViolations];