From b2668bf97c3ea0a0688d14b3290a753926f07e44 Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Sun, 16 Aug 2026 17:17:53 -0400 Subject: [PATCH] Fix: fail the cache-busting check on a stale stamp --check computed the correct hash for every reference and noticed the file differed, but only counted it. Only a reference with no ?v= at all failed the check, so an edited stylesheet or module shipped behind a stamp naming content it no longer had, and browsers kept serving the cached copy. Report those references and fail on them, naming the reference and the hash its content now has. Cover it with a test that copies the tree, breaks one stamp and asserts the check exits non-zero. --- scripts/bump-cache-busting.js | 24 ++++++++++++++++++++++-- ui/test/cache-busting.test.mjs | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) 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. */