diff --git a/.github/workflows/dependabot-vendor-refresh.yml b/.github/workflows/dependabot-vendor-refresh.yml new file mode 100644 index 000000000..07036bd89 --- /dev/null +++ b/.github/workflows/dependabot-vendor-refresh.yml @@ -0,0 +1,120 @@ +name: Dependabot Vendor Refresh + +# #4573: scripts/lint-vendored-deps.cjs gates gsd-core/bin/lib/vendor/{js-yaml.cjs,re2js.cjs} +# for byte-freshness against node_modules, and requires package.json's +# devDependencies pin to literally match the installed version. Dependabot +# regularly opens lockfile-only PRs that bump these packages within the +# existing semver range — it never touches the vendor copy or the manifest +# pin, so the lint-vendored-deps check in test.yml correctly (but +# unhelpfully) flags every such PR as stale, and a human has to manually run +# the refresh command and push a fixup commit. +# +# This workflow does that refresh automatically: on a Dependabot PR that +# touches package.json/package-lock.json, it runs +# `node scripts/lint-vendored-deps.cjs --fix`, which mechanically re-copies +# the upstream build artifact over the vendored .cjs (and, for +# upstream-verbatim twins, their .d.cts files) and rewrites the +# package.json pin to match — see fixRow() in scripts/lint-vendored-deps.cjs +# for exactly what it does and does not touch (it never hand-edits a +# hand-authored twin like js-yaml.d.cts; a genuine upstream API break there +# is left for a human). +# +# Trust boundary: same-repo Dependabot PRs only. The `if:` guard below +# checks both github.actor and pull_request.user.login — defense-in-depth, +# mirroring dependabot-auto-merge.yml's own rationale (github.actor alone +# can't be forged to a different login, but pairing it with +# pull_request.user.login is GitHub's documented hardening pattern). This +# workflow never checks out or executes a fork's code: Dependabot PRs that +# only touch package.json/package-lock.json originate same-repo, and the +# checkout below pins `ref` to the PR head SHA of that same-repo branch. +# +# Why pushing here is not a bypass of the real check: the push (via +# GSD_BOT_PR_TOKEN, falling back to GITHUB_TOKEN — see +# auto-backmerge.yml's "Open or update PR" step for the same fallback +# pattern) lands a new commit on the PR branch, which re-triggers this +# workflow's own `synchronize` trigger AND the `pull_request: synchronize` +# trigger on test.yml. The real lint-vendored-deps check in test.yml then +# re-runs against the fixed commit and genuinely passes — this workflow +# fixes the actual drift the check complains about, it does not silence, +# skip, or override the check itself. + +on: + pull_request: + types: [opened, synchronize] + branches: [next] + paths: + - package.json + - package-lock.json + +concurrency: + group: dependabot-vendor-refresh-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: write + +jobs: + refresh-vendor: + # Defense-in-depth: check both the triggering actor and the PR author, + # same rationale as dependabot-auto-merge.yml. + if: | + github.actor == 'dependabot[bot]' && + github.event.pull_request.user.login == 'dependabot[bot]' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + # persist-credentials: false — the write-capable token must not sit on + # disk while the --fix step below runs `npm ci` and requires the + # freshly-installed, Dependabot-proposed (unreviewed) vendor package. + # The token is only reintroduced, transiently, in the push step, after + # that require() has already run. + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event.pull_request.head.sha }} + token: ${{ secrets.GSD_BOT_PR_TOKEN || secrets.GITHUB_TOKEN }} + persist-credentials: false + + - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + with: + node-version: 24 + + - name: Install dependencies + run: npm ci --ignore-scripts + + - name: Run lint-vendored-deps --fix + id: fix + run: | + set +e + node scripts/lint-vendored-deps.cjs --fix + echo "exit_code=$?" >> "$GITHUB_OUTPUT" + set -e + + - name: Check for changes + id: diff + run: | + if [ -n "$(git status --porcelain)" ]; then + echo "dirty=true" >> "$GITHUB_OUTPUT" + else + echo "dirty=false" >> "$GITHUB_OUTPUT" + fi + + - name: Commit and push the mechanical refresh + if: steps.fix.outputs.exit_code == '0' && steps.diff.outputs.dirty == 'true' + env: + GH_TOKEN: ${{ secrets.GSD_BOT_PR_TOKEN || secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add -A -- gsd-core/bin/lib/vendor package.json + git commit -m "chore: refresh vendored deps to match dependency bump" + git remote set-url origin "https://x-access-token:${GH_TOKEN}@github.com/${{ github.repository }}.git" + git push origin HEAD:${{ github.head_ref }} + + # Intentionally a no-op when the --fix step did not exit 0 (findings + # remain after --fix, e.g. a hand-authored twin's declared-export + # finding): a remaining finding means a real upstream API break that + # only a human can resolve. Auto-committing here would either mask + # that break behind a green re-run or ship something wrong. The real + # lint-vendored-deps check in test.yml still runs on the original + # commit and reports the failure to the PR exactly as it does today. diff --git a/scripts/lint-vendored-deps.cjs b/scripts/lint-vendored-deps.cjs index a3f60de60..7aa73260c 100644 --- a/scripts/lint-vendored-deps.cjs +++ b/scripts/lint-vendored-deps.cjs @@ -161,8 +161,24 @@ function compareFiles(relA, relB) { * @param {string} spec * @returns {string} */ +const PIN_OPERATOR_RE = /^(\^|~|>=|<=|>|<|=)?/; + function stripRangeOperator(spec) { - return String(spec || '').trim().replace(/^[\^~]|^>=|^<=|^>|^<|^=/, '').trim(); + return String(spec || '').trim().replace(PIN_OPERATOR_RE, '').trim(); +} + +/** + * The leading range-operator token (if any) a package.json dependency spec + * was written with — the inverse half of stripRangeOperator, needed by + * `fixRow` to rebuild a pin (`` + ``) that + * preserves the author's original range style instead of collapsing every + * pin to an exact version. + * @param {string} spec + * @returns {string} the operator (e.g. "^", "~", ">="), or "" for an exact pin + */ +function pinOperatorPrefix(spec) { + const m = String(spec || '').trim().match(PIN_OPERATOR_RE); + return (m && m[1]) || ''; } /** @@ -211,6 +227,26 @@ function checkHandAuthoredTwin(row) { return findings; } +/** + * Read a row's pin state: the package.json devDependencies spec for + * `row.name` and, if `node_modules//package.json` exists, its + * installed version. Shared by checkRow (compares) and fixRow (rewrites) so + * the two can never silently diverge on how a pin is read. + * @param {VendoredPackage} row + * @returns {{pinnedSpec: string | undefined, installedVersion: string | undefined}} + */ +function readPinState(row) { + const pkgPath = path.join(ROOT, 'package.json'); + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + const pinnedSpec = pkg.devDependencies && pkg.devDependencies[row.name]; + const installedPkgPath = path.join(ROOT, 'node_modules', row.name, 'package.json'); + let installedVersion; + if (fs.existsSync(installedPkgPath)) { + installedVersion = JSON.parse(fs.readFileSync(installedPkgPath, 'utf8')).version; + } + return { pinnedSpec, installedVersion }; +} + /** * Run all applicable freshness checks for one vendored package row. * @param {VendoredPackage} row @@ -235,31 +271,92 @@ function checkRow(row) { findings.push(...checkHandAuthoredTwin(row)); } - const pkgPath = path.join(ROOT, 'package.json'); - const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); - const pinnedSpec = pkg.devDependencies && pkg.devDependencies[row.name]; + const { pinnedSpec, installedVersion } = readPinState(row); if (!pinnedSpec) { findings.push(`package.json devDependencies.${row.name} is missing`); + } else if (installedVersion === undefined) { + findings.push(`node_modules/${row.name}/package.json does not exist (run npm install)`); } else { - const installedPkgPath = path.join(ROOT, 'node_modules', row.name, 'package.json'); - if (!fs.existsSync(installedPkgPath)) { - findings.push(`node_modules/${row.name}/package.json does not exist (run npm install)`); - } else { - const installed = JSON.parse(fs.readFileSync(installedPkgPath, 'utf8')); - const pinned = stripRangeOperator(pinnedSpec); - if (pinned !== installed.version) { - findings.push( - `package.json devDependencies.${row.name} ("${pinnedSpec}" -> "${pinned}") != ` - + `node_modules/${row.name}/package.json version ("${installed.version}")`, - ); - } + const pinned = stripRangeOperator(pinnedSpec); + if (pinned !== installedVersion) { + findings.push( + `package.json devDependencies.${row.name} ("${pinnedSpec}" -> "${pinned}") != ` + + `node_modules/${row.name}/package.json version ("${installedVersion}")`, + ); } } return findings; } +/** + * Mechanically resolve a vendored package's byte/pin drift: copy the + * upstream .cjs (and, for `upstream-verbatim` rows, the .d.cts twins) over + * the vendored copy, and bump the package.json pin to the installed + * version, preserving the original range-operator prefix. Then re-runs + * `checkRow` and returns whatever findings remain. + * + * This NEVER hand-edits a `hand-authored` twin (e.g. js-yaml.d.cts) — that + * file encodes a human's deliberate judgment about which exports are safe + * to expose, and only a human can tell whether a declared-export-missing + * finding is mechanical drift or a real upstream API break. If one remains + * after this runs, that is by design: the caller must not treat it as + * fixed. + * @param {VendoredPackage} row + * @returns {string[]} findings remaining after the fix (empty when fully resolved) + */ +function fixRow(row) { + fs.copyFileSync(resolvePath(row.upstreamCjs), resolvePath(row.vendoredCjs)); + + if (row.twinKind === 'upstream-verbatim' && row.upstreamDts) { + if (row.vendoredDts) fs.copyFileSync(resolvePath(row.upstreamDts), resolvePath(row.vendoredDts)); + if (row.srcTwin) fs.copyFileSync(resolvePath(row.upstreamDts), resolvePath(row.srcTwin)); + } + + const { pinnedSpec, installedVersion } = readPinState(row); + if (pinnedSpec && installedVersion !== undefined) { + const newPin = `${pinOperatorPrefix(pinnedSpec)}${installedVersion}`; + if (newPin !== pinnedSpec) { + const pkgPath = path.join(ROOT, 'package.json'); + const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + pkg.devDependencies[row.name] = newPin; + fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); + } + } + + return checkRow(row); +} + function main() { + if (process.argv.includes('--fix')) { + /** @type {Record} */ + const remaining = {}; + for (const row of VENDORED) { + const findings = fixRow(row); + if (findings.length > 0) remaining[row.name] = findings; + } + + const names = VENDORED.map((row) => row.name).join(', '); + + if (Object.keys(remaining).length === 0) { + process.stdout.write( + `ok lint-vendored-deps --fix: gsd-core/bin/lib/vendor/{${names}} refreshed and now match node_modules and their pinned versions\n`, + ); + return 0; + } + + const detail = Object.entries(remaining) + .map(([name, findings]) => ` ${name}:\n${findings.map((f) => ` ${f}`).join('\n')}`) + .join('\n'); + throw new ExitError( + 1, + `lint-vendored-deps --fix: mechanical drift refreshed, but the following row(s)\n` + + 'still have findings that --fix cannot resolve automatically — these need a\n' + + 'human, not just a re-run of --fix:\n' + + detail, + ); + } + const findings = []; for (const row of VENDORED) { findings.push(...checkRow(row)); @@ -288,9 +385,11 @@ if (require.main === module) runMain(main); module.exports = { compareFiles, stripRangeOperator, + pinOperatorPrefix, VENDORED, buildRefreshCommand, checkRow, + fixRow, declaredValueExports, checkHandAuthoredTwin, resolvePath, diff --git a/tests/lint-vendored-deps-manifest.test.cjs b/tests/lint-vendored-deps-manifest.test.cjs index 6277ffc72..5dd7b4540 100644 --- a/tests/lint-vendored-deps-manifest.test.cjs +++ b/tests/lint-vendored-deps-manifest.test.cjs @@ -28,6 +28,8 @@ const { compareFiles, checkRow, stripRangeOperator, + pinOperatorPrefix, + fixRow, declaredValueExports, checkHandAuthoredTwin, resolvePath, @@ -268,3 +270,88 @@ describe('G3: the hand-authored js-yaml type twin is excluded from byte-compare, assert.doesNotMatch(content, /export function loadAll\(/, 'loadAll must stay undeclared per the narrowed surface'); }); }); + +describe('#4573: pinOperatorPrefix / fixRow — mechanical --fix for Dependabot-range vendor drift', () => { + test('pinOperatorPrefix extracts the leading range-operator token, or "" for an exact pin', () => { + assert.equal(pinOperatorPrefix('^4.3.1'), '^'); + assert.equal(pinOperatorPrefix('~4.3.1'), '~'); + assert.equal(pinOperatorPrefix('4.3.1'), ''); + assert.equal(pinOperatorPrefix('>=4.3.1'), '>='); + }); + + test('regression guard: stripRangeOperator is unchanged for the same inputs after the PIN_OPERATOR_RE refactor', () => { + assert.equal(stripRangeOperator('^4.3.1'), '4.3.1'); + assert.equal(stripRangeOperator('~4.3.1'), '4.3.1'); + assert.equal(stripRangeOperator('4.3.1'), '4.3.1'); + assert.equal(stripRangeOperator('>=4.3.1'), '4.3.1'); + }); + + test('fixRow resolves mechanical .cjs drift: mutated vendored js-yaml.cjs is byte-restored to match node_modules', (t) => { + const row = jsYamlRow(); + const vendoredAbs = path.join(REPO_ROOT, row.vendoredCjs); + const upstreamAbs = path.join(REPO_ROOT, row.upstreamCjs); + const original = fs.readFileSync(vendoredAbs, 'utf8'); + fs.writeFileSync(vendoredAbs, `${original}\n// mutated for test\n`); + t.after(() => { + // Safety net: fixRow copies FROM upstream, so the vendored file should + // already be back in its original clean state — but re-copy from + // upstream regardless in case an assertion above threw before fixRow + // completed, so this test never leaves the tree dirty. + fs.copyFileSync(upstreamAbs, vendoredAbs); + }); + + const findings = fixRow(row); + assert.deepEqual(findings, [], `expected fixRow to leave zero findings, got: ${JSON.stringify(findings)}`); + assert.ok( + fs.readFileSync(vendoredAbs).equals(fs.readFileSync(upstreamAbs)), + 'expected the vendored .cjs to byte-equal node_modules/js-yaml/dist/js-yaml.js after fixRow', + ); + }); + + test('fixRow does NOT mask a genuine hand-authored-twin incompatibility: a fake declared export still surfaces after --fix', (t) => { + const row = jsYamlRow(); + const srcTwinAbs = path.join(REPO_ROOT, row.srcTwin); + const original = fs.readFileSync(srcTwinAbs, 'utf8'); + fs.writeFileSync( + srcTwinAbs, + `${original}\nexport function thisFixRowExportDoesNotExistAtRuntime(): void;\n`, + ); + t.after(() => { + fs.writeFileSync(srcTwinAbs, original); + }); + + const findings = fixRow(row); + assert.ok( + findings.some((f) => f.includes('thisFixRowExportDoesNotExistAtRuntime')), + `expected the hand-authored-twin finding to survive fixRow, got: ${JSON.stringify(findings)}`, + ); + }); + + test('fixRow preserves the pin\'s original range-operator style when rewriting package.json', (t) => { + const row = jsYamlRow(); + const pkgPath = path.join(REPO_ROOT, 'package.json'); + const installedPkgPath = path.join(REPO_ROOT, 'node_modules', 'js-yaml', 'package.json'); + const installedVersion = JSON.parse(fs.readFileSync(installedPkgPath, 'utf8')).version; + + const originalContent = fs.readFileSync(pkgPath, 'utf8'); + t.after(() => { + fs.writeFileSync(pkgPath, originalContent); + }); + + const pkg = JSON.parse(originalContent); + const stalePin = installedVersion === '4.0.0' ? '~4.0.1' : '~4.0.0'; + pkg.devDependencies['js-yaml'] = stalePin; + fs.writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`); + + const findings = fixRow(row); + assert.deepEqual(findings, [], `expected fixRow to leave zero findings, got: ${JSON.stringify(findings)}`); + + const after = JSON.parse(fs.readFileSync(pkgPath, 'utf8')); + const pinnedAfter = after.devDependencies['js-yaml']; + assert.equal( + pinnedAfter, + `~${installedVersion}`, + `expected fixRow to preserve the "~" range-operator style from the stale pin and correct the version to the installed one, got pin "${pinnedAfter}"`, + ); + }); +});