Merge the review fixes for the service worker and the Loki note

.env.production.example

@@ -524,13 +524,14 @@ API_TOKEN_HMAC_KEY="<openssl rand -hex 32>"
 # Optional log shipping to a Loki instance. LOKI_ENDPOINT turns shipping on.
 # Either form works: the base URL (http://loki:3100) or the full push URL
 # (http://loki:3100/loki/api/v1/push). A Loki on your own network is fine,
-# private addresses and internal hostnames included; only link-local and
-# cloud-metadata addresses are refused. The username/password pair is only
-# needed when your Loki sits behind basic auth. When a push fails (Loki down,
-# wrong path, rejected credentials) the app writes one line to stderr naming
-# the Loki origin, the reason and how many events were dropped, at most once
-# every five minutes per reason; the password never appears in it. Leave all
-# three unset and logs stay on stdout for the Docker log driver to rotate.
+# private addresses and internal hostnames included; only link-local,
+# cloud-metadata and the unspecified address (0.0.0.0, ::) are refused. The
+# username/password pair is only needed when your Loki sits behind basic
+# auth. When a push fails (Loki down, wrong path, rejected credentials) the
+# app writes one line to stderr naming the Loki origin, the reason and how
+# many events were dropped, at most once every five minutes per reason; the
+# password never appears in it. Leave all three unset and logs stay on stdout
+# for the Docker log driver to rotate.
 # LOKI_ENDPOINT=""
 # LOKI_USERNAME=""
 # LOKI_PASSWORD=""

public/sw.js

@@ -399,6 +399,14 @@ function isForeignNavigationResponse(response) {
  * an auth gateway, a redirect) proves nothing, and the worker resumes normal
  * service.
  *
+ * One gateway shape still reads as "another app": a same-origin auth gateway
+ * that answers an expired session with its sign-in page and status 200 (nginx
+ * `auth_request` with `error_page 401 = /oauth2/sign_in` does this). Both the
+ * navigation and this probe then come back as HTML. The probe sends the
+ * gateway cookie (`credentials: "same-origin"`), so a valid gateway session
+ * reaches HealthLog; with an expired one the verdict is wrong, which is why
+ * `retireFromForeignOrigin` never unregisters while a push subscription exists.
+ *
  * The probe is the worker's own `fetch`, which never passes through this
  * worker's fetch handler; `no-store` keeps the HTTP cache out of it and
  * `redirect: "manual"` keeps a login redirect from reading as an answer.
@@ -409,7 +417,7 @@ async function confirmForeignOrigin() {
     response = await fetch("/api/version", {
       cache: "no-store",
       redirect: "manual",
-      credentials: "omit",
+      credentials: "same-origin",
     });
   } catch {
     return false;
@@ -456,13 +464,38 @@ function checkForeignOrigin() {
   return foreignOriginCheck;
 }
 
+/**
+ * Whether this registration holds a Web Push subscription. Unregistering
+ * destroys the subscription and nothing in the app re-creates it (the push
+ * card only reads it), so reminders would stop without notice. A missing
+ * `pushManager` or a throwing lookup counts as "has one": keeping a worker
+ * that only passes requests through costs nothing, losing reminders does.
+ */
+async function hasPushSubscription() {
+  try {
+    const pushManager = self.registration.pushManager;
+    if (!pushManager || typeof pushManager.getSubscription !== "function") {
+      return true;
+    }
+    return (await pushManager.getSubscription()) !== null;
+  } catch {
+    return true;
+  }
+}
+
 /**
  * Leave an origin that no longer serves HealthLog: delete every HealthLog
  * cache (only ours, matched by name, never another app's CacheStorage) and
  * unregister, so the next load of that origin runs without this worker. The
- * `foreignOrigin` flag set by the caller covers the requests the current page
- * still makes before it unloads. Each step is best-effort and independent: a
- * failed cache delete must not keep the worker registered.
+ * `foreignOrigin` flag set by the caller stays set, so the requests the
+ * current page still makes go to the network. Each step is best-effort and
+ * independent: a failed cache delete must not keep the worker registered.
+ *
+ * With a push subscription the worker does not unregister: the verdict may
+ * be a gateway's 200 sign-in page (see `confirmForeignOrigin`), and
+ * unregistering would destroy the subscription. It deletes the caches and
+ * stays in pass-through for the rest of its life instead, so another app on
+ * the origin still gets network content. The next worker start probes again.
  */
 async function retireFromForeignOrigin() {
   try {
@@ -475,6 +508,7 @@ async function retireFromForeignOrigin() {
   } catch {
     // CacheStorage unavailable; unregistering still matters more.
   }
+  if (await hasPushSubscription()) return;
   try {
     await self.registration.unregister();
   } catch {

src/__tests__/service-worker.test.ts

@@ -136,6 +136,8 @@ interface SwHarness {
   cacheStorage: FakeCacheStorage;
   navPreload: { enabled: boolean; enable: () => Promise<void> };
   unregisterCalls: { count: number };
+  /** What `pushManager.getSubscription()` does; default: no subscription. */
+  push: { getSubscription: () => Promise<unknown> };
   lifetimePromises: Promise<unknown>[];
   context: Record<string, unknown>;
 }
@@ -145,6 +147,7 @@ function bootServiceWorker(): SwHarness {
   const cacheStorage = new FakeCacheStorage();
   const lifetimePromises: Promise<unknown>[] = [];
   const unregisterCalls = { count: 0 };
+  const push = { getSubscription: async (): Promise<unknown> => null };
   const navPreload = {
     enabled: false,
     enable: async () => {
@@ -167,6 +170,9 @@ function bootServiceWorker(): SwHarness {
         unregisterCalls.count += 1;
         return true;
       },
+      pushManager: {
+        getSubscription: () => push.getSubscription(),
+      },
       showNotification: async (title: string, opts: { tag?: string }) => {
         shown.push({ title, tag: opts?.tag, closed: false });
       },
@@ -219,6 +225,7 @@ function bootServiceWorker(): SwHarness {
     cacheStorage,
     navPreload,
     unregisterCalls,
+    push,
     lifetimePromises,
     context,
   };
@@ -803,7 +810,7 @@ describe("sw.js — foreign origin (#847)", () => {
     expect(harness.unregisterCalls.count).toBe(1);
   });
 
-  it("probes with no-store and a manual redirect", async () => {
+  it("probes with no-store, a manual redirect and same-origin credentials", async () => {
     const harness = bootServiceWorker();
     let probeInit: RequestInit | undefined;
     harness.context.fetch = async (
@@ -826,6 +833,8 @@ describe("sw.js — foreign origin (#847)", () => {
 
     expect(probeInit?.cache).toBe("no-store");
     expect(probeInit?.redirect).toBe("manual");
+    // A valid same-origin gateway cookie must reach /api/version.
+    expect(probeInit?.credentials).toBe("same-origin");
   });
 
   it("does not retire when a header-stripping proxy hides the marker from a real HealthLog", async () => {
@@ -924,6 +933,59 @@ describe("sw.js — foreign origin (#847)", () => {
     expect(harness.unregisterCalls.count).toBe(1);
   });
 
+  it("keeps a worker that holds a push subscription: caches deleted, no unregister, pass-through", async () => {
+    const harness = bootServiceWorker();
+    await seedHealthlogCaches(harness);
+    harness.push.getSubscription = async () => ({
+      endpoint: "https://push.example/abc",
+    });
+    // A gateway's 200 sign-in page for both the navigation and the probe.
+    routeFetch(harness, foreignHtml, foreignHtml);
+
+    await dispatchNavigationFetch(harness, "/");
+    await Promise.all(harness.lifetimePromises);
+
+    expect(harness.unregisterCalls.count).toBe(0);
+    expect([...harness.cacheStorage.stores.keys()]).toEqual([
+      "next-app-runtime",
+    ]);
+    expect(
+      dispatchAnyFetch(harness, "/_next/static/chunks/app.js").responded,
+    ).toBe(false);
+    expect(
+      dispatchAnyFetch(harness, "/about", { accept: "text/html" }).responded,
+    ).toBe(false);
+  });
+
+  it.each([
+    [
+      "the subscription lookup throws",
+      (h: SwHarness) => {
+        h.push.getSubscription = async () => {
+          throw new Error("push service unavailable");
+        };
+      },
+    ],
+    [
+      "pushManager is missing",
+      (h: SwHarness) => {
+        delete (h.context.self as { registration: { pushManager?: unknown } })
+          .registration.pushManager;
+      },
+    ],
+  ])("does not unregister when %s", async (_label, arrange) => {
+    const harness = bootServiceWorker();
+    await seedHealthlogCaches(harness);
+    arrange(harness);
+    routeFetch(harness, foreignHtml, foreignHtml);
+
+    await dispatchNavigationFetch(harness, "/");
+    await Promise.all(harness.lifetimePromises);
+
+    expect(harness.unregisterCalls.count).toBe(0);
+    expect(harness.cacheStorage.stores.has(CURRENT_STATIC_CACHE)).toBe(false);
+  });
+
   it("still unregisters when deleting the caches fails", async () => {
     const harness = bootServiceWorker();
     harness.cacheStorage.keys = async () => {