fix(sw): the service worker had never installed, and updates raced the reload

Two defects behind "the new version is ready" not going away.

1. THE WORKER NEVER INSTALLED — in any environment, ever.

   install did `cache.addAll([...PRECACHE, SHELL])`. PRECACHE is derived from
   the build's own outputs, which already contain index.html, and SHELL is
   index.html again. addAll() rejects the ENTIRE install on a duplicate
   request:

     InvalidStateError: Cache.addAll(): duplicate requests (.../index.html)

   So install failed on every load, the worker went redundant, and nothing
   ever activated. Registration itself succeeds and the failure is inside the
   install event, so nothing surfaced it — which is why offline behaviour was
   listed as "never verified" rather than "broken". All 11 precached URLs
   return 200; the list was the only fault. Deduping now lives in
   src/swPrecache.ts, at the point of use, so no future addition to either
   list can reintroduce it.

   Verified in a real browser for the first time: the worker installs,
   activates, claims the page, and caches 11 entries. A rebuild is then
   detected, the prompt appears, clicking Reload applies it, stale caches are
   dropped and the prompt does not return.

2. THE DEV WORKER IGNORED THE LIFECYCLE — my own regression, from 53707b1.

   That commit made the dev server serve an inert sw.js to silence a MIME
   error. Inert included the lifecycle: no skipWaiting, no clients.claim, no
   message handler. A worker like that can sit in `waiting` forever and can
   never be applied, so the prompt returns on every load and the Reload button
   cannot possibly clear it. This is the likeliest cause of the reported
   symptom, since with defect 1 a production build could never have shown that
   prompt at all. The dev worker still caches nothing — that would fight HMR —
   but now honours install, activate and SKIP_WAITING.

3. applyUpdate RACED ITS OWN RELOAD — real, measured, but not reproducible on
   demand, so it is hardening rather than a proven fix.

   It posted SKIP_WAITING and called window.location.reload() on the next
   line. Measured in Chrome: at that instant the new worker was still
   `installed` and the controller had NOT changed — it changed 2 ms later. The
   window is wider in practice because activate deletes every stale cache
   before claiming clients. When the reload wins, the reloaded page still finds
   registration.waiting and shows the prompt again. It now waits for
   controllerchange, with a timeout backstop so a worker that ignores
   SKIP_WAITING cannot leave the button dead. The sequencing is pure and
   tested (src/ui/swUpdate.ts): listener attached before the message, reload
   exactly once, unsubscribed after.

Tooling: scripts/screenshot.ts gains --profile (a fresh Chrome profile has no
registered worker, so nothing stateful across loads could be reproduced at all)
and --eval-after-file (observe a flow that ends in a navigation from the other
side of it).

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

scripts/screenshot.ts

@@ -5,6 +5,7 @@
  *   bun scripts/screenshot.ts <url> <out.png> [--w=1280] [--h=900]
  *                             [--wait=1500] [--full] [--click=<selector>]
  *                             [--eval=<js>|--eval-file=<path>] [--dark] [--reload]
+ *                             [--profile=<dir>] [--eval-after-file=<path>]
  *
  * Why this exists: ARCHITECTURE.md and TODO.md both say nothing here can run a
  * browser, so appearance has never been verified. Chrome IS available, but its
@@ -40,6 +41,10 @@ const chrome = Bun.spawn(
     "--hide-scrollbars",
     "--force-device-scale-factor=1",
     "--remote-debugging-port=0",
+    // A fresh profile each run means empty IndexedDB, empty caches and NO
+    // registered service worker — which makes anything stateful across loads
+    // (an update prompt, a migration, an install) impossible to reproduce.
+    ...(flag("profile") ? [`--user-data-dir=${flag("profile")}`] : []),
     `--window-size=${width},${height}`,
     ...(has("dark") ? ["--force-dark-mode", "--enable-features=WebContentsForceDark"] : []),
     "about:blank",
@@ -171,6 +176,21 @@ if (clickSelector) {
   await Bun.sleep(700);
 }
 
+// Runs after the reload, so a flow that ends in a navigation can be observed
+// on the other side of it — an update prompt applying itself, a migration.
+const evalAfterFile = flag("eval-after-file");
+if (evalAfterFile) {
+  const source = await Bun.file(evalAfterFile).text();
+  const result = await send("Runtime.evaluate", {
+    expression: source,
+    awaitPromise: true,
+  });
+  console.log(
+    "eval-after:",
+    JSON.stringify(result?.result?.value ?? result?.result?.description ?? null),
+  );
+}
+
 const shot = await send("Page.captureScreenshot", {
   format: "png",
   captureBeyondViewport: has("full"),

src/index.ts

@@ -37,9 +37,24 @@ const server = serve({
     // clients and handles no fetches keeps registration honest in dev without
     // caching anything, which would fight HMR.
     [`${basePath}sw.js`]: () =>
-      new Response("/* dev: intentionally does nothing */\n", {
-        headers: { "content-type": "text/javascript", "cache-control": "no-cache" },
-      }),
+      new Response(
+        // Caches nothing — that would fight HMR — but it DOES honour the
+        // lifecycle, so the update prompt's Reload button behaves in dev the
+        // way it behaves in production instead of only ever hitting the
+        // timeout backstop.
+        [
+          "/* dev worker: no caching, real lifecycle */",
+          'self.addEventListener("install", () => self.skipWaiting());',
+          'self.addEventListener("activate", (e) => e.waitUntil(self.clients.claim()));',
+          'self.addEventListener("message", (e) => {',
+          '  if (e.data && e.data.type === "SKIP_WAITING") self.skipWaiting();',
+          "});",
+          "",
+        ].join("\n"),
+        {
+          headers: { "content-type": "text/javascript", "cache-control": "no-cache" },
+        },
+      ),
     // The @font-face rules are injected by frontend.tsx; these serve the
     // files those rules point at.
     ...Object.fromEntries(

src/sw.ts

@@ -1,6 +1,8 @@
 /// <reference lib="webworker" />
 declare const self: ServiceWorkerGlobalScope;
 
+import { precacheUrls } from "./swPrecache.ts";
+
 // Both are replaced at build time by Bun.build's `define`.
 const PRECACHE: string[] = JSON.parse(process.env.PRECACHE_MANIFEST ?? "[]");
 const VERSION = process.env.SW_VERSION ?? "dev";
@@ -10,8 +12,10 @@ const CACHE = `budget2-${VERSION}`;
 const SHELL = `${BASE}index.html`;
 
 self.addEventListener("install", (event) => {
+  // Deduped: addAll() rejects the whole install on a repeated request, and the
+  // build's own outputs already contain the shell. See src/swPrecache.ts.
   event.waitUntil(
-    caches.open(CACHE).then((cache) => cache.addAll([...PRECACHE, SHELL])),
+    caches.open(CACHE).then((cache) => cache.addAll(precacheUrls(PRECACHE, SHELL))),
   );
 });
 

src/swPrecache.test.ts

@@ -0,0 +1,53 @@
+import { test, expect, describe } from "bun:test";
+import { precacheUrls } from "./swPrecache.ts";
+
+describe("the service worker's precache list", () => {
+  /**
+   * The bug this exists for: `cache.addAll()` rejects the WHOLE install with
+   * InvalidStateError if the list contains the same request twice. The build's
+   * precache list is derived from its own outputs, which already include
+   * index.html, and the worker then appended the shell — index.html again. So
+   * install failed every time, the worker went redundant, and no service
+   * worker ever activated. Nothing reported it: registration itself succeeds,
+   * and the failure is inside the install event.
+   */
+  test("the shell is not repeated when the build already emitted it", () => {
+    expect(precacheUrls(["/index.html", "/index-abc.js"], "/index.html")).toEqual([
+      "/index.html",
+      "/index-abc.js",
+    ]);
+  });
+
+  test("the shell is added when the build did not emit it", () => {
+    expect(precacheUrls(["/index-abc.js"], "/index.html")).toEqual([
+      "/index-abc.js",
+      "/index.html",
+    ]);
+  });
+
+  test("any duplicate is removed, not just the shell", () => {
+    expect(precacheUrls(["/a.js", "/b.css", "/a.js"], "/index.html")).toEqual([
+      "/a.js",
+      "/b.css",
+      "/index.html",
+    ]);
+  });
+
+  test("order is preserved, so the shell is not fetched before the assets", () => {
+    expect(precacheUrls(["/z.js", "/a.css"], "/index.html")).toEqual([
+      "/z.js",
+      "/a.css",
+      "/index.html",
+    ]);
+  });
+
+  test("a subpath deploy is not a special case", () => {
+    expect(
+      precacheUrls(["/budget2.0/index.html", "/budget2.0/x.js"], "/budget2.0/index.html"),
+    ).toEqual(["/budget2.0/index.html", "/budget2.0/x.js"]);
+  });
+
+  test("an empty build still precaches the shell", () => {
+    expect(precacheUrls([], "/index.html")).toEqual(["/index.html"]);
+  });
+});

src/swPrecache.ts

@@ -0,0 +1,20 @@
+/**
+ * The exact list the service worker precaches on install.
+ *
+ * Separate from src/sw.ts so it can be tested: importing sw.ts outside a
+ * worker would run its `self.addEventListener` calls.
+ *
+ * Deduplication is the whole point. `cache.addAll()` rejects the ENTIRE
+ * install with InvalidStateError if the same request appears twice, and the
+ * build's precache list is derived from its own outputs — which already
+ * include index.html — while the worker also needs the shell by name. Those
+ * two facts collided silently: install failed on every load, the worker went
+ * redundant, and no service worker ever activated. Registration itself
+ * succeeds, so nothing surfaced it.
+ *
+ * Deduping here rather than in build.ts means no future addition to either
+ * list can reintroduce the collision.
+ */
+export function precacheUrls(precache: readonly string[], shell: string): string[] {
+  return [...new Set([...precache, shell])];
+}

src/ui/registerSw.ts

@@ -1,4 +1,5 @@
 import { BASE_PATH, withBase } from "./basePath.ts";
+import { requestUpdate } from "./swUpdate.ts";
 
 let waiting: ServiceWorker | null = null;
 
@@ -32,8 +33,32 @@ export function registerServiceWorker(onUpdateReady: () => void): void {
     });
 }
 
+/**
+ * Reloads only once the new worker is actually in control. Posting
+ * SKIP_WAITING and reloading on the next line raced activation, and when the
+ * reload won, the reloaded page still found a waiting worker and showed the
+ * update prompt again — forever. See src/ui/swUpdate.ts for the measurement.
+ */
 export function applyUpdate(): void {
-  waiting?.postMessage({ type: "SKIP_WAITING" });
+  const target = waiting;
   waiting = null;
-  window.location.reload();
+
+  if (!target) {
+    window.location.reload();
+    return;
+  }
+
+  requestUpdate({
+    postSkipWaiting: () => target.postMessage({ type: "SKIP_WAITING" }),
+    onControllerChange: (listener) => {
+      navigator.serviceWorker.addEventListener("controllerchange", listener);
+      return () =>
+        navigator.serviceWorker.removeEventListener("controllerchange", listener);
+    },
+    setTimer: (fn, ms) => {
+      const id = setTimeout(fn, ms);
+      return () => clearTimeout(id);
+    },
+    reload: () => window.location.reload(),
+  });
 }

src/ui/swUpdate.test.ts

@@ -0,0 +1,100 @@
+import { test, expect, describe } from "bun:test";
+import { requestUpdate, type UpdateHandle } from "./swUpdate.ts";
+
+function fakeHandle() {
+  const events: string[] = [];
+  let controllerListener: (() => void) | null = null;
+  let timer: { fn: () => void; ms: number } | null = null;
+  let unsubscribed = false;
+
+  const handle: UpdateHandle = {
+    postSkipWaiting() {
+      events.push("postSkipWaiting");
+    },
+    onControllerChange(listener) {
+      events.push("listen");
+      controllerListener = listener;
+      return () => {
+        unsubscribed = true;
+      };
+    },
+    setTimer(fn, ms) {
+      timer = { fn, ms };
+      return () => {};
+    },
+    reload() {
+      events.push("reload");
+    },
+  };
+
+  return {
+    handle,
+    events,
+    get unsubscribed() {
+      return unsubscribed;
+    },
+    fireControllerChange: () => controllerListener?.(),
+    fireTimeout: () => timer?.fn(),
+    get timerMs() {
+      return timer?.ms;
+    },
+  };
+}
+
+describe("applying a waiting service worker", () => {
+  /**
+   * The bug: applyUpdate posted SKIP_WAITING and called reload() on the very
+   * next line. Measured in Chrome, the controller had NOT changed at that
+   * instant — it changed 2 ms later — so the reload raced activation. When the
+   * reload won, the fresh page still saw registration.waiting and showed
+   * "A new version is ready" again, and clicking Reload re-entered the same
+   * race. The prompt never went away.
+   */
+  test("does not reload until the new worker actually takes control", () => {
+    const f = fakeHandle();
+    requestUpdate(f.handle);
+    expect(f.events).toEqual(["listen", "postSkipWaiting"]);
+    expect(f.events).not.toContain("reload");
+  });
+
+  test("listens before posting, or a fast activation is missed entirely", () => {
+    const f = fakeHandle();
+    requestUpdate(f.handle);
+    expect(f.events.indexOf("listen")).toBeLessThan(f.events.indexOf("postSkipWaiting"));
+  });
+
+  test("reloads once the controller changes", () => {
+    const f = fakeHandle();
+    requestUpdate(f.handle);
+    f.fireControllerChange();
+    expect(f.events).toEqual(["listen", "postSkipWaiting", "reload"]);
+  });
+
+  /**
+   * A worker that ignores SKIP_WAITING — the deliberately inert one the dev
+   * server serves, or a genuinely broken build — must not leave the Reload
+   * button dead. Reload anyway; the user asked for it.
+   */
+  test("reloads anyway if the worker never takes control", () => {
+    const f = fakeHandle();
+    requestUpdate(f.handle, 1500);
+    expect(f.timerMs).toBe(1500);
+    f.fireTimeout();
+    expect(f.events).toEqual(["listen", "postSkipWaiting", "reload"]);
+  });
+
+  test("reloads exactly once when both the change and the timeout fire", () => {
+    const f = fakeHandle();
+    requestUpdate(f.handle);
+    f.fireControllerChange();
+    f.fireTimeout();
+    expect(f.events.filter((e) => e === "reload")).toHaveLength(1);
+  });
+
+  test("stops listening once it has reloaded", () => {
+    const f = fakeHandle();
+    requestUpdate(f.handle);
+    f.fireControllerChange();
+    expect(f.unsubscribed).toBe(true);
+  });
+});

src/ui/swUpdate.ts

@@ -0,0 +1,52 @@
+/**
+ * Sequencing for "apply the waiting service worker, then reload".
+ *
+ * Pure, with every browser touchpoint injected, because the bug it fixes is
+ * entirely about ORDER and order is what a test can pin. The old code was:
+ *
+ *     waiting.postMessage({ type: "SKIP_WAITING" });
+ *     window.location.reload();
+ *
+ * Measured in Chrome: at the moment of that reload the new worker was still
+ * `installed` and the controller had not changed — it changed 2 ms later. So
+ * the reload raced activation. When the reload won, the fresh page load still
+ * found `registration.waiting` and showed "A new version is ready" again, and
+ * pressing Reload re-entered the same race. The prompt never went away.
+ *
+ * The window is wider than 2 ms in practice: the worker's `activate` handler
+ * deletes every stale cache before calling `clients.claim()`.
+ */
+export interface UpdateHandle {
+  /** Tells the waiting worker to activate now. */
+  postSkipWaiting(): void;
+  /** Subscribes to the page's controller changing. Returns an unsubscribe. */
+  onControllerChange(listener: () => void): () => void;
+  setTimer(fn: () => void, ms: number): () => void;
+  reload(): void;
+}
+
+/**
+ * `timeoutMs` is a backstop, not a delay: a worker that ignores SKIP_WAITING —
+ * the deliberately inert one the dev server serves, or a broken build — must
+ * not leave the Reload button doing nothing at all. It reloads anyway, because
+ * that is what the user asked for.
+ */
+export function requestUpdate(handle: UpdateHandle, timeoutMs = 3000): void {
+  let reloaded = false;
+
+  const finish = () => {
+    if (reloaded) return;
+    reloaded = true;
+    stopListening();
+    cancelTimer();
+    handle.reload();
+  };
+
+  // Subscribe BEFORE posting. Activation can complete in a couple of
+  // milliseconds, and a listener attached afterwards would miss it and fall
+  // through to the timeout every time.
+  const stopListening = handle.onControllerChange(finish);
+  const cancelTimer = handle.setTimer(finish, timeoutMs);
+
+  handle.postSkipWaiting();
+}