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 pin to
literally match the installed version. Dependabot regularly opens
lockfile-only PRs bumping these packages within the existing semver range,
which it can never satisfy (it has no awareness of the vendor copy or the
pin), so every such PR sits permanently red until a human manually runs the
refresh and pushes a fixup commit. PR #4565 was the latest instance.
Adds `--fix` to lint-vendored-deps.cjs (fixRow): mechanically re-copies the
upstream build artifact over the vendored .cjs/.d.cts twins and bumps the
manifest pin, preserving its range-operator style. It never touches a
hand-authored twin's declared type surface (js-yaml.d.cts) — a remaining
finding there means a real upstream API break, and --fix leaves it failing
rather than mask it.
Adds .github/workflows/dependabot-vendor-refresh.yml: on a same-repo
dependabot[bot] PR touching package.json/package-lock.json (same
defense-in-depth identity check as dependabot-auto-merge.yml), runs
`--fix` and, only on a clean result, commits and pushes the refresh back
to the PR branch via GSD_BOT_PR_TOKEN (falling back to GITHUB_TOKEN, same
pattern as auto-backmerge.yml) so the push re-triggers `synchronize` and
the real lint-vendored-deps check in test.yml genuinely re-passes. A
non-clean --fix result (real incompatibility) makes no commit, leaving
the actual failure visible for a human — this fixes the check's own
complaint, it does not bypass or weaken the check.
Closes #4573
Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
120
.github/workflows/dependabot-vendor-refresh.yml
vendored
Normal file
120
.github/workflows/dependabot-vendor-refresh.yml
vendored
Normal file
@@ -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.
|
||||
@@ -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 (`<same operator>` + `<new version>`) 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/<row.name>/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<string, string[]>} */
|
||||
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,
|
||||
|
||||
@@ -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}"`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user