From 3e91a8f2af6ce04af6d5e793782d79d764ab8bed Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Sat, 15 Aug 2026 16:40:19 +0000 Subject: [PATCH] fix(#751,#752): the retention window counted deploys, and 30 of them was 3.5 days MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production rendered unstyled for the eighth time on 2026-08-15. The origin was healthy — every route served CSS 200, all 13 retained stylesheets resolved, and the post-deploy detector was green. The visitor was holding a document the window no longer covered. RETAIN_GENERATIONS was raised 5 -> 30 in #650 on the reasoning that "30 covers a normal working week even at an unusually high merge rate". Measured against the deploy log for the six days after: 40 successful deploys, 19 in a single day. 30 generations was ~3.5 days, and the live ledger topped out at age 25. The comment above that constant already diagnosed the error one iteration earlier — the window protects how long a tab has been open, which is unrelated to deploy count. The fix acted on the diagnosis by changing the NUMBER and keeping the UNIT, so the same mismatch survived in a new parameterisation. So state it in the unit the risk is in. ASSET_AGES.txt now carries a first-seen timestamp per file, retention drops on elapsed days against RETAIN_DAYS (14), and the generation counter survives only as a diagnostic. A retained asset keeps its ORIGINAL date — restamping it would make the window never expire anything while looking healthy, so that has its own test. RETAIN_MAX_FILES (800) is a runaway backstop, and it says so loudly when it engages, because a count deciding coverage means the day window no longer is. Nothing could have caught the shortfall. check-retained-assets.mjs asks whether every promised file is served; all 13 were. It never asked whether the promise was wide enough. It does now, and that assertion fails rather than warns — this file's own header records how a warning inside a green check went unnoticed. It is dormant during the ledger's first fortnight, because a freshly-retimed ledger legitimately spans zero days, so RETENTION_RETIMED_AT exists purely so the floor can be exercised before then: a check nobody has seen go red is not yet a check. StylesheetGuard is the recovery that depends on no number, and it was disarming itself for the life of a tab after one recovery — stranding exactly the visitors it exists for, since holding a document for days is what expires its assets. It already stored a timestamp; only the read side threw it away. Now it re-arms after an hour, which still stops a genuine loop (those retry in seconds). Verified by mutation, not by assertion count. Reverting the guard to once-per-tab fails the new re-arm case in both harnesses; disabling it entirely trips the positive control, which reports that the second-load result proves nothing rather than quietly passing. The window floor was driven red at a 2-day ledger and green at 20, with the ramp case proving it stays quiet when a narrow window is correct. The StylesheetGuard unit test asserted `sessionStorage.getItem(...) return` by regex — a shape match that passed for the bug being fixed. The throttle is sessionStorage plus arithmetic, which jsdom runs faithfully, so it is executed now instead of pattern-matched. None of this is the fix. #635 is: a CDN serving HTML no-cache, which GitHub Pages cannot be configured to do. Closes #751 Closes #752 Refs #635 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/deploy.yml | 34 ++-- CLAUDE.md | 56 ++++-- .../__tests__/check-retained-assets.test.js | 126 +++++++++++- .../__tests__/retain-previous-assets.test.js | 189 +++++++++++++++--- scripts/check-stale-html.mjs | 92 ++++++++- scripts/ci/check-retained-assets.mjs | 89 +++++++++ scripts/retain-previous-assets.mjs | 176 +++++++++++++--- .../StylesheetGuard/StylesheetGuard.test.tsx | 68 ++++++- .../StylesheetGuard/StylesheetGuard.tsx | 33 ++- 9 files changed, 744 insertions(+), 119 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index b56f659f..b49a2b90 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -201,26 +201,32 @@ jobs: "${SITE_URL:-https://scripthammer.com}" env: SITE_URL: ${{ vars.NEXT_PUBLIC_SITE_URL }} - # How many generations back an asset is carried. Bounds _next/static so - # chaining cannot grow forever. + # HOW LONG an asset is carried. Stated in days because the exposure is a + # duration: how long a visitor may still be holding a document. # - # WAS 5, AND THE REASONING WAS WRONG. It was sized against "deploys per 10 - # minutes" — the HTML cache-control window. But the thing being protected - # is not a 10-minute window, it is HOW LONG SOMEONE'S TAB HAS BEEN OPEN, - # and those are unrelated quantities. On 2026-08-09 production rendered - # unstyled for the SIXTH time (#438, #467, #476, #548): five deploys landed - # across 22 hours — well-paced by the 10-minute rule — and a visitor - # returning the next day asked for CSS that had just aged out. Retention - # worked exactly as designed; the design measured the wrong thing. + # IT COUNTED DEPLOYS TWICE, AND WAS WRONG BOTH TIMES (#751). 5 was sized + # against "deploys per 10 minutes" — the HTML cache-control window — and + # #650 correctly identified that as the wrong quantity when production went + # unstyled a SIXTH time. But its fix kept counting deploys: 30, justified as + # "a normal working week even at an unusually high merge rate". Then 40 + # deploys landed in the following 6 days (19 in one day), making 30 + # generations ~3.5 days, and production went unstyled an EIGHTH time on + # 2026-08-15. Retention worked exactly as designed, twice; the design + # measured a quantity nobody had measured, twice. # - # 30 covers a normal working week even at an unusually high merge rate. - # Measured cost: 3-5 hashed files per generation (17 files bought the old - # 5), so ~100 small files against a 131-page export. + # A duration cannot be invalidated by the merge rate. Two weeks covers a + # holiday-length absence. Expected cost at the measured ~7 deploys/day: + # ~300-500 small files against a 131-page export (17 files bought the old + # 5 generations, ~100 bought 30). RETAIN_MAX_FILES backstops a runaway. + # + # `check-retained-assets.mjs` in smoke.yml FAILS if the live window is + # narrower than this. That assertion is the part that was missing — nothing + # ever compared the promise against its intended width. # # This is a MITIGATION. The recovery that does not depend on any number is # the self-heal in src/app/layout.tsx; the actual fix is #635 — a CDN # serving HTML `no-cache`, which GitHub Pages cannot be configured to do. - RETAIN_GENERATIONS: '30' + RETAIN_DAYS: '14' - name: Upload artifact uses: actions/upload-pages-artifact@v3 diff --git a/CLAUDE.md b/CLAUDE.md index 9cb6ec58..ce8934e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -423,33 +423,47 @@ The rules below still describe `e2e.yml` and remain correct for it. ### Merging several PRs in quick succession can serve production with no CSS -**Why**: GitHub Pages serves HTML with `cache-control: max-age=600`, and every deploy -deletes the previous build's content-hashed CSS and JS. So for **ten minutes** after a -deploy, a visitor can be holding HTML whose stylesheets no longer exist — a white page, -no nav, the logo at its natural size, with a perfectly correct DOM. Reported from -production four times (#438, #467, #476, #548). - -`scripts/retain-previous-assets.mjs` carries old assets forward so that visitor still -resolves. It now chains across `RETAIN_GENERATIONS` (5) deploys — but before #548 it -spanned exactly **one**, which is only enough at one deploy per cache window. Six PRs -merged in 35 minutes, two of them 57 seconds apart, put real visitors two generations -back and broke the site for them. +**Why**: GitHub Pages serves HTML with `cache-control: max-age=600` and cannot be told +otherwise, while every deploy deletes the previous build's content-hashed CSS and JS. A +returning visitor can therefore hold HTML whose stylesheets no longer exist — a white +page, no nav, the logo at its natural size, with a perfectly correct DOM. Reported from +production **eight times** (#438, #467, #476, #548, #650, and three more through +2026-08-15). The open ticket is **#635**; it stays open until the cause is gone. + +**The window is measured in DAYS, and getting that unit wrong is the recurring bug.** +`scripts/retain-previous-assets.mjs` carries old assets forward, bounded by `RETAIN_DAYS` +(14) in `deploy.yml`. It was previously bounded by a deploy count, and that was mis-sized +twice — 5 (against the 10-minute cache window) and then 30 (against an assumed merge +rate, which turned out to be 40 deploys in 6 days, so ~3.5 days). **Never restate this +window in deploys**: converting requires a merge rate nobody measures, and +`retain-previous-assets.test.js` now fails if `RETAIN_GENERATIONS` reappears (#751). **The trap is that docs-only PRs feel free.** `e2e.yml` has `paths-ignore` for `**/*.md`, `docs/**` and `.gitignore`, so markdown PRs skip the ~1-hour E2E mutex that paces everything else — and nothing else paces them. That is exactly how six merges landed in half an hour. -**Now guarded by**: `scripts/check-stale-html.mjs` drives A → B → C and asserts a -visitor holding A's HTML is still styled after **two** deploys, with a negative control -that fails if one-generation retention ever stops breaking (i.e. if the harness has quietly -stopped simulating a deploy). `scripts/__tests__/retain-previous-assets.test.js` asserts -the chaining and the generation cap. Both run in CI — the first in the required -`accessibility` check, the second via `pnpm test:scripts`. - -**Still worth pacing merges.** The guards make a burst survivable, not free: retention is -capped at 5 generations, so more than five deploys inside one 10-minute window is still -outside what anything protects. +**Guarded by three things, which check different questions:** + +- `scripts/check-stale-html.mjs` (required `accessibility` check) drives A → B → C in a + real chromium and asserts a visitor holding A's HTML is still styled after two deploys, + with a negative control that fails if the harness stops simulating a deploy. It also + proves `StylesheetGuard` fires, stays inert on a healthy page, and re-arms after an hour + but not immediately (#752). +- `scripts/ci/check-retained-assets.mjs` (post-deploy `smoke.yml`) reads the live ledger + and asserts both that every promised file is served **and that the window is still as + wide as `RETAIN_DAYS`**. The second assertion exists because the first was green on the + night production went unstyled for the eighth time. +- `scripts/__tests__/*` via `pnpm test:scripts` for the chaining, the window and the unit. + +**The client-side backstop**: `StylesheetGuard` (in every page) detects a page whose +same-origin stylesheets all have zero rules and re-fetches at a fresh URL. It is the only +recovery that does not depend on a number being right — but it ships _inside_ the HTML, +so a document cached before it existed has no guard at all. + +**Still worth pacing merges**, and still worth remembering that none of this is the fix. +The fix is #635 — a CDN serving HTML `no-cache`, which GitHub Pages cannot be configured +to do. ### NEVER bypass commit hooks (no `--no-verify`) diff --git a/scripts/__tests__/check-retained-assets.test.js b/scripts/__tests__/check-retained-assets.test.js index fa7a526a..1cceced9 100644 --- a/scripts/__tests__/check-retained-assets.test.js +++ b/scripts/__tests__/check-retained-assets.test.js @@ -16,9 +16,30 @@ function retainedEntries(extra = []) { ]; } -function runProbe(baseUrl) { +/** + * An `ASSET_AGES.txt` body whose oldest entry is `spanDays` old (#751). + * + * The probe reads this ledger to judge whether the retention WINDOW is wide enough, + * which is a separate question from whether the files are reachable — and the one + * nothing asked on the night production went unstyled an eighth time. + */ +function agesFor(entries, spanDays) { + const now = Date.now(); + return entries + .map((rel, i) => { + const age = + i === 0 ? spanDays : (spanDays * (entries.length - i)) / entries.length; + const when = new Date(now - age * 86400000).toISOString(); + return `${i} ${when} ${rel.replace(/^\/+/, '')}`; + }) + .join('\n'); +} + +function runProbe(baseUrl, env = {}) { return new Promise((resolve, reject) => { - const child = spawn(process.execPath, [SCRIPT, baseUrl]); + const child = spawn(process.execPath, [SCRIPT, baseUrl], { + env: { ...process.env, ...env }, + }); let stdout = ''; let stderr = ''; child.stdout.on('data', (chunk) => { @@ -50,6 +71,10 @@ test('accepts a CDN-style 206 ranged GET when HEAD is unavailable', async (t) => response.end(entries.join('\n')); return; } + if (request.url === '/_next/static/ASSET_AGES.txt') { + response.end(agesFor(entries, 20)); + return; + } if (request.method === 'HEAD') { response.writeHead(405).end(); return; @@ -73,6 +98,10 @@ test('fails and names a missing retained stylesheet', async (t) => { response.end(entries.join('\n')); return; } + if (request.url === '/_next/static/ASSET_AGES.txt') { + response.end(agesFor(entries, 20)); + return; + } if (request.url === missing) { response.writeHead(404).end(); return; @@ -88,3 +117,96 @@ test('fails and names a missing retained stylesheet', async (t) => { assert.match(output, /removed\.css/); assert.match(output, /STYLESHEETS/); }); + +/** + * THE WINDOW ASSERTION (#751). + * + * Every check above asks whether retained files are REACHABLE. On 2026-08-15 all 13 + * retained stylesheets were reachable and production was unstyled anyway, because + * the window they represented had shrunk to ~3.5 days while the config claimed a + * week. Reachability cannot see that; only these can. + * + * `RETENTION_RETIMED_AT` is backdated here because the floor is deliberately dormant + * during the ledger's first fortnight — without the override these would be testing + * the ramp, not the assertion. + */ +const PAST_RAMP = { + RETENTION_RETIMED_AT: '2026-01-01T00:00:00Z', + RETAIN_DAYS: '14', +}; + +const serveLedger = (entries, spanDays) => (request, response) => { + if (request.url === '/_next/static/ASSET_MANIFEST.txt') { + response.end(entries.join('\n')); + return; + } + if (request.url === '/_next/static/ASSET_AGES.txt') { + response.end(agesFor(entries, spanDays)); + return; + } + response.writeHead(200).end(); +}; + +test('fails when the retention window has collapsed below RETAIN_DAYS', async (t) => { + const entries = retainedEntries(['/_next/static/css/app.css']); + const server = await startServer(serveLedger(entries, 2)); + t.after(() => server.close()); + + const result = await runProbe(server.baseUrl, PAST_RAMP); + const output = result.stdout + result.stderr; + + assert.equal(result.code, 1, output); + assert.match(output, /covers only 2\.0 day\(s\)/); +}); + +test('passes when the window is at full width — the harness can reach success', async (t) => { + // Without this the test above passes just as well against a probe that fails on + // everything, which is the vacuous shape this repo keeps getting bitten by. + const entries = retainedEntries(['/_next/static/css/app.css']); + const server = await startServer(serveLedger(entries, 20)); + t.after(() => server.close()); + + const result = await runProbe(server.baseUrl, PAST_RAMP); + const output = result.stdout + result.stderr; + + assert.equal(result.code, 0, output); + assert.match(output, /full width/); +}); + +test('stays quiet during the ramp, when a narrow window is correct', async (t) => { + const entries = retainedEntries(['/_next/static/css/app.css']); + const server = await startServer(serveLedger(entries, 2)); + t.after(() => server.close()); + + // Same 2-day ledger as the failing case; only the retime date differs. + const result = await runProbe(server.baseUrl, { + RETENTION_RETIMED_AT: new Date().toISOString(), + RETAIN_DAYS: '14', + }); + const output = result.stdout + result.stderr; + + assert.equal(result.code, 0, output); + assert.match(output, /still ramping/); +}); + +test('fails when the age ledger is missing entirely', async (t) => { + const entries = retainedEntries(['/_next/static/css/app.css']); + const server = await startServer((request, response) => { + if (request.url === '/_next/static/ASSET_MANIFEST.txt') { + response.end(entries.join('\n')); + return; + } + if (request.url === '/_next/static/ASSET_AGES.txt') { + response.writeHead(404).end(); + return; + } + response.writeHead(200).end(); + }); + t.after(() => server.close()); + + const result = await runProbe(server.baseUrl, PAST_RAMP); + const output = result.stdout + result.stderr; + + assert.equal(result.code, 1, output); + assert.match(output, /age ledger/i); +}); diff --git a/scripts/__tests__/retain-previous-assets.test.js b/scripts/__tests__/retain-previous-assets.test.js index 54da7065..e8ec1312 100644 --- a/scripts/__tests__/retain-previous-assets.test.js +++ b/scripts/__tests__/retain-previous-assets.test.js @@ -51,8 +51,14 @@ function makeGeneration(dir, tag) { return dir; } -/** Run the real script: retain from `liveDir` (served over HTTP) into `outDir`. */ -async function retain(outDir, liveDir, generations = 5) { +/** + * Run the real script: retain from `liveDir` (served over HTTP) into `outDir`. + * + * `days` is the retention window (#751). It used to be a generation count; the + * default is deliberately wide so the chaining tests below exercise chaining rather + * than expiry, which is what they are about. + */ +async function retain(outDir, liveDir, days = 14) { const server = createServer((req, res) => { const rel = decodeURIComponent((req.url ?? '/').split('?')[0]); const file = path.join(liveDir, rel === '/' ? 'index.html' : rel); @@ -87,7 +93,7 @@ async function retain(outDir, liveDir, generations = 5) { ], { encoding: 'utf8', - env: { ...process.env, RETAIN_GENERATIONS: String(generations) }, + env: { ...process.env, RETAIN_DAYS: String(days) }, } ); return stdout; @@ -106,6 +112,7 @@ const manifestOf = (dir) => .split('\n') .filter(Boolean); +/** Path -> generation count. The generation number is now a diagnostic (#751). */ const agesOf = (dir) => new Map( fs @@ -113,11 +120,49 @@ const agesOf = (dir) => .split('\n') .filter(Boolean) .map((l) => { - const m = l.match(/^(\d+)\s+(.+)$/); - return [m[2], Number(m[1])]; + const m = l.match(/^(\d+)\s+(\S+T\S+Z)\s+(.+)$/); + return [m[3], Number(m[1])]; }) ); +/** Path -> first-seen epoch ms. This is what retention actually decides on. */ +const bornOf = (dir) => + new Map( + fs + .readFileSync(path.join(dir, '_next/static/ASSET_AGES.txt'), 'utf8') + .split('\n') + .filter(Boolean) + .map((l) => { + const m = l.match(/^(\d+)\s+(\S+T\S+Z)\s+(.+)$/); + return [m[3], Date.parse(m[2])]; + }) + ); + +/** + * Rewrite one entry's first-seen timestamp in a built directory's ledger. + * + * Retention is now measured in days, and a test cannot wait days. Backdating the + * ledger is the only thing being simulated — the script reads it exactly as it would + * read a genuinely old one. + */ +function backdate(dir, rel, days) { + const p = path.join(dir, '_next/static/ASSET_AGES.txt'); + const when = new Date(Date.now() - days * 86400000).toISOString(); + const out = fs + .readFileSync(p, 'utf8') + .split('\n') + .map((l) => { + const m = l.match(/^(\d+)\s+(\S+T\S+Z)\s+(.+)$/); + return m && m[3] === rel ? `${m[1]} ${when} ${m[3]}` : l; + }) + .join('\n'); + assert.ok( + out.includes(when), + `backdate() failed to match ${rel} in the ledger` + ); + fs.writeFileSync(p, out); +} + describe('retain-previous-assets: chaining across a burst (#548)', () => { before(() => { fs.rmSync(WORK, { recursive: true, force: true }); @@ -273,42 +318,117 @@ describe('retain-previous-assets: chaining across a burst (#548)', () => { ); }); - it('stops carrying an asset past RETAIN_GENERATIONS, so _next/static stays bounded', async () => { - // The cap is the only thing bounding chained retention. Without it every + it('drops an asset once it is older than RETAIN_DAYS, so _next/static stays bounded', async () => { + // The window is the only thing bounding chained retention. Without it every // deploy would accumulate forever, which is the obvious failure mode of the // fix and therefore worth a test of its own. const dirs = ['a4', 'b4', 'c4'].map((n, i) => makeGeneration(path.join(WORK, n), `gen-${'abc'[i]}`) ); - await retain(dirs[1], dirs[0], 1); // cap of 1: A's file is age 1, still kept + await retain(dirs[1], dirs[0], 14); assert.ok(fs.existsSync(path.join(dirs[1], '_next/static/css/gen-a.css'))); - await retain(dirs[2], dirs[1], 1); // A would become age 2 — past the cap + // Age A's stylesheet past the window in B's published ledger, exactly as real + // elapsed time would. B's own file stays new. + backdate(dirs[1], '_next/static/css/gen-a.css', 40); + + await retain(dirs[2], dirs[1], 14); assert.ok( !fs.existsSync(path.join(dirs[2], '_next/static/css/gen-a.css')), - 'an asset past RETAIN_GENERATIONS must not be carried forward' + 'an asset older than RETAIN_DAYS must not be carried forward' ); assert.ok( fs.existsSync(path.join(dirs[2], '_next/static/css/gen-b.css')), - 'the generation still inside the cap must survive' + 'an asset still inside the window must survive' + ); + }); + + it('keeps an asset across MANY deploys while it is still inside the window', async () => { + // The whole point of #751: deploy COUNT must not decide expiry. Under the old + // generation cap this asset died on deploy 6; here it survives ten because it + // is only a day old. + const dirs = Array.from({ length: 11 }, (_, i) => + makeGeneration(path.join(WORK, `burst${i}`), `burst-${i}`) + ); + for (let i = 1; i < dirs.length; i++) + await retain(dirs[i], dirs[i - 1], 14); + + const last = dirs[dirs.length - 1]; + assert.ok( + fs.existsSync(path.join(last, '_next/static/css/burst-0.css')), + 'a one-day-old asset must survive ten deploys — expiring it on a deploy ' + + 'count is exactly the #751 defect' + ); + assert.strictEqual( + agesOf(last).get('_next/static/css/burst-0.css'), + 10, + 'the generation counter should still be counting, as a diagnostic' + ); + }); + + it('carries the ORIGINAL first-seen date forward, never restamping it', async () => { + // If a retained asset were redated on each deploy its age would reset every + // time and the window would never expire anything — retention would grow + // without bound while looking healthy. + const dirs = ['a6', 'b6', 'c6'].map((n, i) => + makeGeneration(path.join(WORK, n), `gen-${'abc'[i]}`) + ); + await retain(dirs[1], dirs[0], 14); + backdate(dirs[1], '_next/static/css/gen-a.css', 10); + const before = bornOf(dirs[1]).get('_next/static/css/gen-a.css'); + + await retain(dirs[2], dirs[1], 14); + const after = bornOf(dirs[2]).get('_next/static/css/gen-a.css'); + assert.strictEqual( + after, + before, + 'the first-seen timestamp must survive a deploy unchanged' + ); + }); + + it('reads the pre-#751 two-field ledger and stamps those entries now', async () => { + // The live ledger on the deploy that ships this has no timestamps. Refusing to + // parse it would reset retention to zero on the very deploy meant to fix it. + const dirs = ['a7', 'b7'].map((n, i) => + makeGeneration(path.join(WORK, n), `gen-${'ab'[i]}`) + ); + fs.writeFileSync( + path.join(dirs[0], '_next/static/ASSET_AGES.txt'), + '3 _next/static/css/gen-a.css\n' + ); + + const out = await retain(dirs[1], dirs[0], 14); + assert.match(out, /without a timestamp/, 'the ramp should announce itself'); + assert.ok( + fs.existsSync(path.join(dirs[1], '_next/static/css/gen-a.css')), + 'an undated legacy entry must be carried, not dropped' + ); + const born = bornOf(dirs[1]).get('_next/static/css/gen-a.css'); + assert.ok( + Date.now() - born < 5 * 60_000, + 'a legacy entry should be stamped now, giving it a full window' ); }); }); /** - * THE WINDOW IS A DURATION, NOT A BURST COUNT (#650). + * THE WINDOW IS A DURATION, AND MUST BE EXPRESSED AS ONE (#650, #751). + * + * This was `RETAIN_GENERATIONS` and it was mis-sized twice, both times by stating + * the window in deploys while the risk is in days: * - * `RETAIN_GENERATIONS` was 5, chosen against "deploys per 10 minutes" — the HTML - * cache-control window. But what it protects is how long a visitor's tab has been - * open, which is unrelated. On 2026-08-09 production rendered unstyled for the - * SIXTH time: five deploys across 22 hours, well-paced by the 10-minute rule, and - * someone returning the next day asked for CSS that had just aged out. + * 5 — sized against "deploys per 10 minutes", the HTML cache-control window. + * #650 identified that as the wrong quantity when production went unstyled a + * sixth time on 2026-08-09. + * 30 — its replacement, justified as "a normal working week even at an unusually + * high merge rate". 40 deploys landed in the next 6 days, 19 in one day, so + * it was really ~3.5 days. Production went unstyled an eighth time. * - * This pins the number so it cannot quietly drift back to a burst-sized one. The - * client-side recovery in src/components/StylesheetGuard.tsx is what makes the - * failure survivable regardless; this keeps it rare. + * The unit is the fix. A day is a day no matter how often anyone merges, so these + * assertions pin the UNIT as much as the number — a value in generations cannot + * satisfy them at all. */ -describe('RETAIN_GENERATIONS is sized for a returning visitor', () => { +describe('RETAIN_DAYS is sized for a returning visitor', () => { const deployYml = fs.readFileSync( path.join(__dirname, '..', '..', '.github', 'workflows', 'deploy.yml'), 'utf8' @@ -317,20 +437,29 @@ describe('RETAIN_GENERATIONS is sized for a returning visitor', () => { it('is set in deploy.yml at all', () => { assert.match( deployYml, - /RETAIN_GENERATIONS:\s*'?\d+'?/, - 'deploy.yml no longer sets RETAIN_GENERATIONS — retention would fall back to ' + - 'the script default and nothing would say so' + /RETAIN_DAYS:\s*'?\d+'?/, + 'deploy.yml no longer sets RETAIN_DAYS — retention would fall back to the ' + + 'script default and nothing would say so' ); }); - it('covers a working week of deploys, not a single burst', () => { - const m = deployYml.match(/RETAIN_GENERATIONS:\s*'?(\d+)'?/); + it('covers a fortnight, so a holiday-length absence is inside the window', () => { + const m = deployYml.match(/RETAIN_DAYS:\s*'?(\d+)'?/); const n = Number(m[1]); assert.ok( - n >= 30, - `RETAIN_GENERATIONS is ${n}. Below 30 a visitor returning after a normal ` + - `working week loses their stylesheets — that is #650, reported from ` + - `production six times. Raise it back, or make the case in the issue first.` + n >= 14, + `RETAIN_DAYS is ${n}. Below 14 a visitor returning from a week or two away ` + + `loses their stylesheets — that is #635, reported from production eight ` + + `times. Raise it back, or make the case in the issue first.` + ); + }); + + it('has not reverted to counting deploys', () => { + assert.ok( + !/RETAIN_GENERATIONS:/.test(deployYml), + 'deploy.yml sets RETAIN_GENERATIONS again. That unit is the #751 defect: it ' + + 'converts to a duration only via a merge rate nobody measures. Express the ' + + 'window in days.' ); }); }); diff --git a/scripts/check-stale-html.mjs b/scripts/check-stale-html.mjs index 0a22d8d6..7437ca63 100644 --- a/scripts/check-stale-html.mjs +++ b/scripts/check-stale-html.mjs @@ -523,11 +523,14 @@ if (!burstBroke) // ── PAST THE RETENTION CAP, THE CLIENT MUST RECOVER ITSELF (#650) ─────────── // -// Everything above proves retention survives a burst. It cannot survive -// FOREVER — RETAIN_GENERATIONS bounds it, and the bound is counted in DEPLOYS -// while the exposure is how long a visitor's tab has been open. Those are -// unrelated, which is how production rendered unstyled a sixth time on -// 2026-08-09 with retention working exactly as designed. +// Everything above proves retention survives a burst. It cannot survive FOREVER: +// `RETAIN_DAYS` bounds it, and a visitor away longer than that is outside it. +// +// The bound used to be counted in DEPLOYS, which is a different quantity from the +// exposure it protects — how long a visitor's tab has been open. That mismatch put +// production unstyled a sixth time on 2026-08-09 and an eighth on 2026-08-15, both +// with retention working exactly as designed (#751). Stating the window as a +// duration removes the conversion, but not the bound. // // So src/components/StylesheetGuard.tsx recovers the page when every same-origin // stylesheet came back empty. This asserts that guard fires on the real @@ -620,6 +623,85 @@ if (!guardMatch) { failures.push( 'the stylesheet guard reloaded on a single dead sheet — too eager' ); + + // RE-ARMING (#752). The cases above each use a fresh context, so they say + // nothing about the rule that actually decides a SECOND recovery. That rule was + // "never" — once per tab, forever — which stranded exactly the visitors this + // guard exists for: a tab open long enough for its assets to expire is a tab + // likely to have recovered once already. + // + // Both directions are asserted, because the change is only correct if it moved + // one of them and left the other alone. One context, two loads: sessionStorage + // is per-tab, so reusing it is the whole point. + const runGuardTwice = async ({ ageOutMs }) => { + const statuses = [404, 404]; + const ctx = await browser.newContext(); + const page = await ctx.newPage(); + statuses.forEach((_, i) => + page.route(`**/g${i}.css`, (r) => r.fulfill({ status: 404, body: '' })) + ); + await page.route('**/guard.html*', (r) => + r.fulfill({ + status: 200, + contentType: 'text/html', + body: guardPage(statuses), + }) + ); + + await page.goto('http://guard.test/guard.html', { waitUntil: 'load' }); + await page.waitForTimeout(1200); + const first = /_r=/.test(page.url()); + + // Backdate the stored recovery so the next load sees an OLD one. Faking the + // clock rather than waiting an hour, and the only thing being faked is the + // passage of time. + if (ageOutMs) { + await page.evaluate((ms) => { + sessionStorage.setItem( + 'sh-stylesheet-recovered', + String(Date.now() - ms) + ); + }, ageOutMs); + } + + // A URL the guard has not already rewritten, so `_r=` can only appear if it + // recovered a second time. + await page.goto('http://guard.test/guard.html?second=1', { + waitUntil: 'load', + }); + await page.waitForTimeout(1200); + const second = /_r=/.test(page.url()); + await ctx.close(); + return { first, second }; + }; + + // Positive control: this harness must be able to reach success at all. Without + // it, an anti-loop assertion passes just as well when the guard never fires. + const reArmed = await runGuardTwice({ ageOutMs: 2 * 3600000 }); + console.log( + ` broke again an hour on -> ${reArmed.second ? 'recovered again (correct)' : 'STILL DISARMED (wrong)'}` + ); + if (!reArmed.first) + failures.push( + 're-arm harness never recovered on its FIRST load, so its second-load result ' + + 'proves nothing — fix the harness before reading the assertion below' + ); + if (reArmed.first && !reArmed.second) + failures.push( + 'the stylesheet guard did not re-arm after an hour — a tab that recovered ' + + 'once stays unstyled forever, which is #752' + ); + + // And the reason the limit exists at all: a genuine loop retries in seconds. + const looped = await runGuardTwice({ ageOutMs: 0 }); + console.log( + ` broke again immediately-> ${looped.second ? 'RELOADED AGAIN (loop risk)' : 'blocked (correct)'}` + ); + if (looped.second) + failures.push( + 'the stylesheet guard recovered twice in a row with no delay — that is a ' + + 'reload loop, which is strictly worse than an unstyled page' + ); } await browser.close(); diff --git a/scripts/ci/check-retained-assets.mjs b/scripts/ci/check-retained-assets.mjs index 00742c4c..f14f7918 100644 --- a/scripts/ci/check-retained-assets.mjs +++ b/scripts/ci/check-retained-assets.mjs @@ -40,8 +40,30 @@ const BASE = ( 'https://scripthammer.com' ).replace(/\/$/, ''); const MANIFEST = `${BASE}/_next/static/ASSET_MANIFEST.txt`; +const AGES = `${BASE}/_next/static/ASSET_AGES.txt`; const CONCURRENCY = 12; +/** Must match `RETAIN_DAYS` in .github/workflows/deploy.yml (#751). */ +const RETAIN_DAYS = Number(process.env.RETAIN_DAYS ?? 14); + +/** + * When the day-based ledger shipped (#751). + * + * A freshly-retimed ledger spans zero days and widens by about a day per day, so a + * short span means "ramping" for the first RETAIN_DAYS and "collapsed" ever after. + * Nothing IN the ledger can tell those apart — both look like recent timestamps — + * so the window floor stays dormant until enough wall-clock time has passed for a + * healthy ledger to have filled. Failing during the ramp would train people to + * ignore this check in the two weeks before it can first mean anything. + * + * Overridable ONLY so the floor can be exercised before the ramp elapses — a check + * nobody has seen go red is not yet a check, and this one is dormant by design for + * its first two weeks. Nothing in CI sets it. + */ +const RETIMED_AT = Date.parse( + process.env.RETENTION_RETIMED_AT ?? '2026-08-15T00:00:00Z' +); + /** * HEAD, falling back to a ranged GET whenever HEAD cannot prove the asset is * served. Some CDNs reject HEAD while serving GET, and a valid ranged response @@ -145,6 +167,73 @@ if (missing.length) { process.exit(1); } +/** + * IS THE PROMISE WIDE ENOUGH? (#751) + * + * Everything above asks whether the retained files are reachable. All 13 stylesheets + * were, on the night production went unstyled for the eighth time — the check was + * green and correct, and the window it was vouching for had quietly shrunk to about + * three and a half days because the cap counted deploys instead of days. + * + * So this asks the other question, the one nothing asked: does the ledger actually + * span the coverage we intend to sell? A window that has collapsed passes every + * reachability assertion ever written, which is precisely why it needs its own. + * + * Ramp: a freshly-retimed ledger legitimately spans zero days, and grows by roughly a + * day per day. Failing during that would be crying wolf on a correct deploy, so the + * floor only applies once the ledger is old enough to have reached full width. + */ +const agesRes = await fetch(AGES, { redirect: 'follow' }); +if (!agesRes.ok) { + console.error( + `::error::${AGES} returned ${agesRes.status}. Without the age ledger the next ` + + `deploy cannot date what it carries, so retention silently restarts.` + ); + process.exit(1); +} + +const dated = []; +for (const line of (await agesRes.text()).split('\n')) { + const m = line.trim().match(/^(\d+)\s+(\S+T\S+Z)\s+(.+)$/); + if (m) { + const t = Date.parse(m[2]); + if (Number.isFinite(t)) dated.push(t); + } +} + +if (!dated.length) { + console.log( + `\n age ledger carries no timestamps yet — pre-#751 format, still ramping. ` + + `Window unverifiable until the next deploy.` + ); +} else { + const now = Date.now(); + const spanDays = (now - Math.min(...dated)) / 86_400_000; + const rampDaysElapsed = (now - RETIMED_AT) / 86_400_000; + console.log( + `\n window ${spanDays.toFixed(1)} day(s) of coverage, target ${RETAIN_DAYS}` + ); + + // A day of slack: the oldest asset ages out mid-window, so a healthy ledger + // oscillates just under the target rather than sitting exactly on it. + if (spanDays + 1 >= RETAIN_DAYS) { + console.log(` window is at full width.`); + } else if (rampDaysElapsed < RETAIN_DAYS) { + console.log( + ` still ramping (day ${rampDaysElapsed.toFixed(1)} of ${RETAIN_DAYS} since ` + + `the ledger was retimed) — the floor is not asserted yet.` + ); + } else { + console.error( + `\n::error::retention covers only ${spanDays.toFixed(1)} day(s), but ` + + `RETAIN_DAYS is ${RETAIN_DAYS}. A visitor returning after ` + + `${spanDays.toFixed(1)} days gets an unstyled page. This is the failure ` + + `mode of #635 and the exact shortfall that shipped it an 8th time.` + ); + process.exit(1); + } +} + console.log( '\n OK — every asset the deploy promised to retain is still served.' ); diff --git a/scripts/retain-previous-assets.mjs b/scripts/retain-previous-assets.mjs index cd4c866a..055a3b0a 100644 --- a/scripts/retain-previous-assets.mjs +++ b/scripts/retain-previous-assets.mjs @@ -55,7 +55,8 @@ if (!outDir || !liveBase) { const BASE = liveBase.replace(/\/$/, ''); const ASSET_RE = /(?:href|src)="([^"]*\/_next\/static\/[^"]+)"/g; /** Chunk paths appear as bare strings inside the runtime, not as attributes. */ -const CHUNK_RE = /["'`]([^"'`]*\/_next\/static\/(?:chunks|css)\/[^"'`]+\.(?:js|css))["'`]/g; +const CHUNK_RE = + /["'`]([^"'`]*\/_next\/static\/(?:chunks|css)\/[^"'`]+\.(?:js|css))["'`]/g; async function get(url) { try { @@ -152,11 +153,44 @@ const wanted = new Set(); /** Generations-since-introduced for files this run retained. Filled in below. */ const ages = new Map(); +/** First-seen timestamp (epoch ms) for files this run retained. Filled in below. */ +const firstSeen = new Map(); + /** - * How many generations back an asset is carried. Bounds `_next/static` so - * chaining cannot grow it forever (#548). + * HOW LONG an asset is carried, and the only rule that decides what is dropped (#751). + * + * THIS USED TO COUNT DEPLOYS AND THAT WAS THE BUG, TWICE. The exposure being + * protected is how long a visitor may hold a document — a duration. Deploy count is + * a different quantity, and converting between them requires knowing the merge rate, + * which nobody measured either time: + * + * - 5 was sized against "deploys per 10 minutes", the HTML cache-control window. + * #650 correctly identified that as measuring the wrong thing. + * - 30 replaced it, justified as "a normal working week even at an unusually high + * merge rate" — and then 40 deploys landed in the next 6 days, 19 of them in one + * day. 30 generations was ~3.5 days. Production went unstyled for the 8th time. + * + * So the cap is now stated in the unit the risk is actually in. Two weeks covers a + * holiday-length absence, and no assumption about merge rate can invalidate it. + */ +const RETAIN_DAYS = Number(process.env.RETAIN_DAYS ?? 14); + +/** + * Hard backstop on how many previous-build files are carried, independent of age. + * + * Time alone does not bound the chain: a burst of deploys inside the window grows + * `_next/static` without limit. When more candidates survive the age rule than this, + * the NEWEST are kept — the oldest are the ones fewest visitors can still be holding. + * + * Measured cost for scale: 17 files bought 5 generations, ~100 bought 30. 800 is far + * above the ~300-500 expected at 14 days, so it is a runaway guard rather than a + * second cap doing routine work. When it engages it says so loudly, because that + * means the age window is no longer the thing deciding coverage. */ -const RETAIN_GENERATIONS = Number(process.env.RETAIN_GENERATIONS ?? 5); +const RETAIN_MAX_FILES = Number(process.env.RETAIN_MAX_FILES ?? 800); + +const DAY_MS = 86_400_000; +const NOW = Date.now(); /** * Publish `ASSET_MANIFEST.txt` + `ASSET_AGES.txt` describing what is on disk. @@ -182,7 +216,10 @@ async function publishManifest() { const p = join(dir, e.name); if (e.isDirectory()) { await walk(p); - } else if (e.name !== 'ASSET_MANIFEST.txt' && e.name !== 'ASSET_AGES.txt') { + } else if ( + e.name !== 'ASSET_MANIFEST.txt' && + e.name !== 'ASSET_AGES.txt' + ) { published.push(p.slice(outDir.length + 1)); } } @@ -195,15 +232,30 @@ async function publishManifest() { } published.sort(); - await writeFile(join(staticRoot, 'ASSET_MANIFEST.txt'), published.join('\n') + '\n'); + await writeFile( + join(staticRoot, 'ASSET_MANIFEST.txt'), + published.join('\n') + '\n' + ); await writeFile( join(staticRoot, 'ASSET_AGES.txt'), - // Anything not carried in by the retain loop is this build's own output: age 0. - published.map((rel) => `${ages.get(rel) ?? 0} ${rel}`).join('\n') + '\n' + // ` `. The timestamp is what decides + // retention (#751); the generation count is kept as a diagnostic, because it is + // what makes a runaway merge rate legible in the logs. + // + // Anything not carried in by the retain loop is this build's own output: age 0, + // first seen now. + published + .map( + (rel) => + `${ages.get(rel) ?? 0} ${new Date(firstSeen.get(rel) ?? NOW).toISOString()} ${rel}` + ) + .join('\n') + '\n' ); + const oldest = [...firstSeen.values()].reduce((a, b) => Math.min(a, b), NOW); console.log( `manifest lists ${published.length} file(s) — this build's output plus ` + - `${ages.size} retained, carried up to ${RETAIN_GENERATIONS} generation(s)` + `${ages.size} retained, carried up to ${RETAIN_DAYS} day(s); oldest retained ` + + `asset is ${((NOW - oldest) / DAY_MS).toFixed(1)} day(s) old` ); } @@ -230,11 +282,16 @@ if (manifest) { .map((l) => l.trim()) .filter((l) => l.startsWith('_next/static/')); for (const l of lines) wanted.add(`/${l}`); - console.log(`manifest from the live build: ${wanted.size} file(s) — complete list`); + console.log( + `manifest from the live build: ${wanted.size} file(s) — complete list` + ); } const pages = manifest ? [] : await livePages(); -if (!manifest) console.log('no manifest on the live build — falling back to crawling its HTML'); +if (!manifest) + console.log( + 'no manifest on the live build — falling back to crawling its HTML' + ); if (pages === null) { // Non-zero so this is visibly a failure. The workflow step uses // `continue-on-error`, so the deploy still ships — but the step is marked @@ -259,7 +316,9 @@ if (!manifest) `read ${read}/${pages.length} live page(s); collected ${wanted.size} asset reference(s)` ); if (!manifest && read === 0) { - console.log('::error::listed pages but read none of them — retention is a NO-OP'); + console.log( + '::error::listed pages but read none of them — retention is a NO-OP' + ); await publishManifest(); process.exit(1); } @@ -274,7 +333,9 @@ if (!manifest && read < pages.length) { // manifest already gave the complete list. const seed = manifest ? [] : [...wanted].filter((u) => u.endsWith('.js')); for (const u of seed) { - const res = await get(u.startsWith('http') ? u : `${new URL(BASE).origin}${u}`); + const res = await get( + u.startsWith('http') ? u : `${new URL(BASE).origin}${u}` + ); if (!res) continue; const js = await res.text(); for (const m of js.matchAll(CHUNK_RE)) wanted.add(m[1]); @@ -282,33 +343,68 @@ for (const u of seed) { console.log(`after one transitive pass: ${wanted.size} reference(s)`); /** - * GENERATION AGES (#548). + * THE AGE LEDGER (#548, retimed in #751). * - * `ASSET_AGES.txt` on the live build maps each published file to how many - * deploys ago it was introduced. Retaining a file bumps its age by one; past - * RETAIN_GENERATIONS it is dropped, which is what keeps chained retention from - * growing `_next/static` forever. + * `ASSET_AGES.txt` on the live build records, for each published file, when it was + * first seen and how many deploys ago that was. Retaining a file carries its + * ORIGINAL timestamp forward unchanged — that is what makes the window a duration + * rather than a deploy count, and it is the whole fix. The generation number rides + * along as a diagnostic only; nothing is dropped because of it. * - * Absent on the currently-live build (the first deploy after this lands), every - * retained file simply starts at age 1 — the ramp is one deploy, same as #476's. + * Absent on the currently-live build, every retained file starts dated now — the + * ramp is one deploy, same as #476's. The same applies per-entry to the old + * two-field lines, which is why the parser below still reads them. */ const liveAges = new Map(); +const liveFirstSeen = new Map(); +let undated = 0; const agesRes = await get(`${BASE}/_next/static/ASSET_AGES.txt`); if (agesRes) { for (const line of (await agesRes.text()).split('\n')) { - const m = line.trim().match(/^(\d+)\s+(.+)$/); - if (m) liveAges.set(m[2], Number(m[1])); + // New format ` `, and the OLD ` ` it replaces. + // Both are parsed for exactly one deploy — the currently-live ledger predates + // the timestamp, and refusing to read it would reset retention to zero on the + // very deploy that introduces the fix. + const dated = line.trim().match(/^(\d+)\s+(\S+T\S+Z)\s+(.+)$/); + if (dated) { + liveAges.set(dated[3], Number(dated[1])); + const t = Date.parse(dated[2]); + if (Number.isFinite(t)) liveFirstSeen.set(dated[3], t); + continue; + } + const legacy = line.trim().match(/^(\d+)\s+(.+)$/); + if (legacy) { + liveAges.set(legacy[2], Number(legacy[1])); + undated++; + } } - console.log(`live age table: ${liveAges.size} entry(ies)`); + console.log( + `live age table: ${liveAges.size} entry(ies)` + + (undated + ? `, ${undated} without a timestamp — stamped now (one-deploy ramp)` + : '') + ); } else { - console.log('no age table on the live build — retained files start at age 1'); + console.log( + 'no age table on the live build — retained files start at age 1, dated now' + ); } let retained = 0; let alreadyPresent = 0; let failed = 0; let tooOld = 0; +let overflowed = 0; +/** + * SELECT BEFORE DOWNLOADING (#751). + * + * The age rule and the file-count backstop both decide what NOT to fetch, so both + * have to run before any request — an expired asset should cost nothing. The count + * backstop additionally needs to compare candidates against each other, which a + * single streaming loop cannot do. + */ +const candidates = []; for (const ref of wanted) { // Strip any basePath so the on-disk location matches the build output. const path = ref.startsWith('http') ? new URL(ref).pathname : ref; @@ -325,15 +421,30 @@ for (const ref of wanted) { /* not in the new build — that is exactly what we retain */ } - // Age it forward, and stop carrying it once it is beyond the cap. This is the - // only thing bounding the chain, so it runs BEFORE the download — an expired - // asset should cost no request at all. - const age = (liveAges.get(rel) ?? 0) + 1; - if (age > RETAIN_GENERATIONS) { + // Unknown to the live ledger means first sighting: it is dated now, so it gets a + // full window rather than being dropped for having no history. + const born = liveFirstSeen.get(rel) ?? NOW; + const ageDays = (NOW - born) / DAY_MS; + if (ageDays > RETAIN_DAYS) { tooOld++; continue; } + candidates.push({ rel, dest, born, age: (liveAges.get(rel) ?? 0) + 1 }); +} +// Newest first, so the backstop drops the assets fewest visitors can still be holding. +candidates.sort((a, b) => b.born - a.born); +if (candidates.length > RETAIN_MAX_FILES) { + overflowed = candidates.length - RETAIN_MAX_FILES; + candidates.length = RETAIN_MAX_FILES; + console.log( + `::warning::${overflowed} asset(s) dropped by the ${RETAIN_MAX_FILES}-file backstop ` + + `rather than by age. Coverage is no longer ${RETAIN_DAYS} days — raise ` + + 'RETAIN_MAX_FILES or slow the merge rate.' + ); +} + +for (const { rel, dest, born, age } of candidates) { // Fetch against BASE, not the origin. `path` carries whatever prefix the LIVE // HTML uses, and `rel` is prefix-free — so joining `rel` to BASE is the only // combination correct in both directions. Using the origin plus the live path @@ -348,12 +459,16 @@ for (const ref of wanted) { await mkdir(dirname(dest), { recursive: true }); await writeFile(dest, buf); ages.set(rel, age); + firstSeen.set(rel, born); retained++; } console.log( `\nretained ${retained} previous-build asset(s); ${alreadyPresent} already in the new build; ` + - `${failed} unreachable; ${tooOld} past ${RETAIN_GENERATIONS} generation(s)` + `${failed} unreachable; ${tooOld} past ${RETAIN_DAYS} day(s)` + + (overflowed + ? `; ${overflowed} past the ${RETAIN_MAX_FILES}-file backstop` + : '') ); if (wanted.size === 0) { console.log( @@ -380,4 +495,3 @@ if (retained === 0) { } await publishManifest(); - diff --git a/src/components/subatomic/StylesheetGuard/StylesheetGuard.test.tsx b/src/components/subatomic/StylesheetGuard/StylesheetGuard.test.tsx index 94affbf0..4eeab737 100644 --- a/src/components/subatomic/StylesheetGuard/StylesheetGuard.test.tsx +++ b/src/components/subatomic/StylesheetGuard/StylesheetGuard.test.tsx @@ -3,18 +3,23 @@ import { describe, it, expect } from 'vitest'; import StylesheetGuard from './StylesheetGuard'; /** - * These assert the SHAPE of the emitted script, not its runtime behaviour. + * Mostly the SHAPE of the emitted script — plus the one part that is real logic. * * jsdom cannot reproduce the bug: it does not fetch stylesheets, so a `` * never 404s and `document.styleSheets` never carries an empty external sheet. * The behavioural proof lives in `scripts/check-stale-html.mjs`, which drives a - * real Chromium, covers healthy / all-dead / one-dead / no-sheets, and is + * real Chromium, covers healthy / all-dead / one-dead / re-arm / anti-loop, and is * mutation-proven — disabling the guard fails the required `accessibility` check. * * What is worth pinning HERE is the set of properties whose loss would make that * runtime guard dangerous or dead, and which a careless edit could remove without - * any browser noticing: the loop stopper, the deferral to `load`, and the fact - * that it navigates rather than reloads. + * any browser noticing: the deferral to `load`, and the fact that it navigates + * rather than reloads. + * + * The THROTTLE is different — it is `sessionStorage` plus arithmetic, which jsdom + * executes faithfully — so it is run rather than pattern-matched. `new Function` is + * safe here: its only input is this component's own compiled-in template literal, + * which is exactly what ships, and executing what ships is the point. */ describe('StylesheetGuard', () => { const scriptText = () => { @@ -43,13 +48,62 @@ describe('StylesheetGuard', () => { expect(script?.textContent?.length ?? 0).toBeGreaterThan(0); }); - it('cannot loop — it records a sessionStorage flag and bails when set', () => { + /** + * THE THROTTLE IS REAL LOGIC, SO TEST IT AS LOGIC (#752). + * + * The stylesheet detection needs a browser, but the decision about whether to arm + * at all is just `sessionStorage` and clock arithmetic — jsdom runs that exactly + * as a browser would. Running it beats matching its source: this was a regex + * against `sessionStorage.getItem(...) return`, which passed for the once-per-tab + * bug and would have passed for any rearrangement that broke the rule. + * + * Arming is observable: a script that intends to act registers a `load` listener, + * and a throttled one returns before it can. + */ + const armsAfter = (stored: string | null) => { + sessionStorage.clear(); + if (stored !== null) + sessionStorage.setItem('sh-stylesheet-recovered', stored); + const listeners: string[] = []; + const original = window.addEventListener; + window.addEventListener = ((type: string, ...rest: unknown[]) => { + listeners.push(type); + return (original as unknown as (...a: unknown[]) => void).call( + window, + type, + ...rest + ); + }) as typeof window.addEventListener; + try { + new Function(scriptText())(); + } finally { + window.addEventListener = original; + } + return listeners.includes('load'); + }; + + it('arms on a tab that has never recovered', () => { + // The positive control. Without it every assertion below passes just as well + // against a script that never arms at all. + expect(armsAfter(null)).toBe(true); + }); + + it('cannot loop — it stays disarmed right after a recovery', () => { // A reload loop is strictly worse than an unstyled page, so this is the single // most important property of the script. + expect(armsAfter(String(Date.now()))).toBe(false); + }); + + it('re-arms an hour after the last recovery (#752)', () => { + // It used to be once per tab forever, which stranded exactly the long-lived + // tabs this guard exists for. + expect(armsAfter(String(Date.now() - 2 * 3600_000))).toBe(true); + }); + + it('records the recovery time, not a boolean', () => { const s = scriptText(); expect(s).toContain('sh-stylesheet-recovered'); - expect(s).toMatch(/sessionStorage\.getItem\([^)]*\)\)?\s*return/); - expect(s).toContain('sessionStorage.setItem'); + expect(s).toContain('sessionStorage.setItem(KEY, String(Date.now()))'); }); it('waits for load, not DOMContentLoaded', () => { diff --git a/src/components/subatomic/StylesheetGuard/StylesheetGuard.tsx b/src/components/subatomic/StylesheetGuard/StylesheetGuard.tsx index 19a2ba7f..74e87068 100644 --- a/src/components/subatomic/StylesheetGuard/StylesheetGuard.tsx +++ b/src/components/subatomic/StylesheetGuard/StylesheetGuard.tsx @@ -5,14 +5,18 @@ * and cannot be configured otherwise, while every deploy replaces content-hashed * CSS. A returning visitor can therefore hold HTML whose stylesheets no longer * exist: white page, no nav, the logo at its natural size, DOM perfectly correct. - * Reported from live production six times — #438, #467, #476, #548, and again on - * 2026-08-09. + * Reported from live production eight times — #438, #467, #476, #548, #650, and + * three more through 2026-08-15. * * `scripts/retain-previous-assets.mjs` carries old assets forward and is why this is - * rare, but it is bounded by `RETAIN_GENERATIONS` — and that bound is counted in - * DEPLOYS while the exposure is HOW LONG A TAB HAS BEEN OPEN. Those are unrelated, - * so no value of it closes the hole. This does: it notices the page is unstyled and - * fetches the current HTML. + * rare. It is now bounded in DAYS rather than deploys (#751), which fixes a real + * mis-measurement but still leaves a bound — a visitor gone longer than the window + * is outside it. This has no bound: it notices the page is unstyled and fetches the + * current HTML, whatever the reason the assets went away. + * + * ONE RECOVERY PER HOUR, not per tab (#752). The original limit was per tab for the + * life of the tab, which disarmed the visitors most exposed to the bug — a tab open + * long enough for its assets to expire is one that has probably recovered before. * * WHY THE DETECTOR COUNTS RULES. Two earlier detectors were written and a browser * refuted both before either shipped: @@ -45,10 +49,21 @@ const stylesheetGuard = ` (function () { try { - // At most ONE recovery per tab. A reload loop is strictly worse than an - // unstyled page — a visitor can at least read an unstyled page. + // At most one recovery per HOUR per tab (#752). A reload loop is strictly + // worse than an unstyled page — a visitor can at least read an unstyled page + // — so a second attempt is refused while the first is still recent. + // + // This used to be once per TAB, forever, which stranded exactly the visitors + // the guard exists for: holding a document for days is what makes its assets + // expire, and a tab open for days is one that has likely recovered before. + // Recovering on Monday must not disarm Friday. + // + // An hour separates the two cases cleanly. A genuine loop re-fires in + // seconds and is still stopped after one attempt; no real deploy-and-return + // cycle repeats inside an hour. var KEY = 'sh-stylesheet-recovered'; - if (sessionStorage.getItem(KEY)) return; + var last = Number(sessionStorage.getItem(KEY) || 0); + if (last && Date.now() - last < 3600000) return; // On 'load', deliberately: at DOMContentLoaded a stylesheet may still be in // flight and would read as missing, turning a slow network into a reload.