Fix: fail the cache-busting check on a stale stamp

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');
 }

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. */