diff --git a/scripts/bump-cache-busting.js b/scripts/bump-cache-busting.js index 3b56ddfe..8086a0c5 100644 --- a/scripts/bump-cache-busting.js +++ b/scripts/bump-cache-busting.js @@ -29,6 +29,8 @@ function hashFor(assetPath) { so one pass can leave stale hashes. Repeat until a pass makes no changes. */ /* References that had no stamp before this run. */ const unstamped = []; +/* References whose stamp does not match the content it names. */ +const stale = []; /* Manifests whose entryVersions no longer match their entry files. */ const staleManifests = []; @@ -41,6 +43,23 @@ function findUnstamped(text, file) { return found; } +/* A stamp that names content the file no longer has. The browser keeps serving + the cached copy, so the page runs a mix of versions after an upgrade. This is + the same failure an unstamped reference causes, and it has to fail the check + for the same reason. */ +function findStale(text, file) { + const found = []; + const re = new RegExp(REF_RE.source, 'g'); + for (let m = re.exec(text); m !== null; m = re.exec(text)) { + if (!m[3]) continue; /* reported as unstamped instead */ + const want = `?v=${hashFor(m[2])}`; + if (m[3] !== want) { + found.push(`${path.relative(UI_DIR, file)} references ${m[2]}${m[3]}, but its content is ${want}`); + } + } + return found; +} + /* --check must not write. A check that modifies the tree makes CI pass by fixing the thing it reports. */ const CHECK_ONLY = process.argv.includes('--check'); @@ -59,6 +78,7 @@ while (filesChangedThisPass !== 0) { for (const file of files) { const original = fs.readFileSync(file, 'utf8'); unstamped.push(...findUnstamped(original, file)); + if (CHECK_ONLY) stale.push(...findStale(original, file)); const updated = original.replace( REF_RE, (_match, quote, assetPath) => `${quote}${assetPath}?v=${hashFor(assetPath)}`, @@ -130,12 +150,12 @@ if (CHECK_ONLY) { if (staleManifests.length) { console.log(`bump-cache-busting: ${staleManifests.length} widget manifest(s) will be stamped by the build`); } - const problems = [...new Set(unstamped)]; + const problems = [...new Set([...unstamped, ...stale])]; if (problems.length) { console.error('bump-cache-busting --check failed:'); for (const p of problems) console.error(` ${p}`); console.error('Run `node scripts/bump-cache-busting.js` and commit the result.'); process.exit(1); } - console.log('bump-cache-busting: every asset reference is stamped'); + console.log('bump-cache-busting: every asset reference is stamped and current'); } diff --git a/ui/test/cache-busting.test.mjs b/ui/test/cache-busting.test.mjs index e0fd3be0..5d23ad1f 100644 --- a/ui/test/cache-busting.test.mjs +++ b/ui/test/cache-busting.test.mjs @@ -17,7 +17,9 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; +import { spawnSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); @@ -90,6 +92,37 @@ test('the build script can see an unstamped reference', () => { } }); +/* A stamp naming content the file no longer has is the same failure as a + missing one: the browser serves the cached copy and the page runs a mix of + versions. --check reported those files in a count and passed anyway, so an + edited stylesheet shipped behind a stale stamp. Run against a copy of the + tree, since the script rewrites what it is pointed at. */ +test('--check fails on a stamp that no longer matches its file', () => { + const repo = path.resolve(root, '..'); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sy-stamp-')); + try { + for (const dir of ['ui', 'scripts']) { + fs.cpSync(path.join(repo, dir), path.join(tmp, dir), { recursive: true }); + } + const script = path.join(tmp, 'scripts', 'bump-cache-busting.js'); + const run = () => spawnSync(process.execPath, [script, '--check'], { encoding: 'utf8' }); + + assert.equal(run().status, 0, 'the committed tree should pass before it is broken'); + + const index = path.join(tmp, 'ui', 'index.html'); + const before = fs.readFileSync(index, 'utf8'); + const broken = before.replace(/(\/css\/dashboard\.css\?v=)[0-9a-f]+/, '$1deadbeef'); + assert.notEqual(broken, before, 'the fixture reference was not found'); + fs.writeFileSync(index, broken); + + const r = run(); + assert.equal(r.status, 1, 'a stale stamp has to fail the check'); + assert.match(`${r.stdout}${r.stderr}`, /deadbeef/, 'the failure should name the stale reference'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } +}); + /* The checks moved into a composite action so the release build runs the same set as a pull request; see .github/actions/checks. Asserted there rather than in test.yml, which now just calls it. */