Merge pull request #144 from Coffey-Labs/push-survive-deploy

Keep push alive across a deploy, not just across a week

web/src/lib/__tests__/storage.test.ts

@@ -75,6 +75,7 @@ describe("device-trusted storage", () => {
     saveJson(accountKey("acct1", "recent"), [{ email: "[email protected]" }]);
     store.set("ihasmail:lastUser", "[email protected]");
     store.set("ihasmail:pushDeviceId", "ihasmail-abc");
+    store.set("ihasmail:pushEnabled", "1");
 
     clearSignedInData();
 
@@ -84,6 +85,14 @@ describe("device-trusted storage", () => {
     // Kept on purpose: prefills sign-in, and only a trusted device wrote it.
     expect(store.get("ihasmail:lastUser")).toBe("[email protected]");
     expect(store.get("ihasmail:pushDeviceId")).toBe("ihasmail-abc");
+    /*
+     * Kept for the ending that is not a sign-out. A deploy expires every
+     * session, and that path clears local data without unsubscribing -- there
+     * is no session left to unsubscribe with. Losing the flag there would
+     * strand a live subscription with nothing renewing it, and the switch in
+     * Settings would still say background notifications were on.
+     */
+    expect(store.get("ihasmail:pushEnabled")).toBe("1");
   });
 
   it("clears everything, lastUser included, for an untrusted sign-in", () => {

web/src/lib/__tests__/webpush.test.ts

@@ -1,4 +1,4 @@
-import { afterEach, describe, expect, it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { client } from "@/jmap/client";
 import {
   applicationServerKey,
@@ -8,10 +8,14 @@ import {
   needsRenewal,
   RENEW_WITHIN_MS,
   subscriptionPayload,
+  pushEnabledHere,
+  setPushEnabledHere,
   supportsEmailPush,
+  unsubscribeThisDevice,
   webPushAvailable,
   type JmapPushSubscription,
 } from "@/lib/webpush";
+import { setDeviceTrusted } from "@/lib/storage";
 import type { JmapSession } from "@/jmap/types";
 
 /**
@@ -240,3 +244,62 @@ describe("needsRenewal", () => {
     expect(needsRenewal([sub(MINE, "whenever")], MINE, NOW)).toBe(true);
   });
 });
+
+/**
+ * Whether push is on *in this browser* is the flag the renewal on app start
+ * keys off, so the two endings that can clear it have to be told apart.
+ *
+ * Signing out clears it, alongside destroying the subscription itself: a
+ * browser left notifying for a mailbox nobody is signed into is somebody
+ * else's mail on a shared machine. A session merely expiring must not, because
+ * that path -- which is what a deploy does to everyone at once -- leaves the
+ * subscription registered and has no session left to remove it with. That half
+ * is enforced by `KEEP_ON_SIGN_OUT` and tested in storage.test.ts.
+ */
+describe("remembering that push is on here", () => {
+  let store: Map<string, string>;
+
+  beforeEach(() => {
+    store = new Map();
+    Object.defineProperty(globalThis, "localStorage", {
+      configurable: true,
+      value: {
+        getItem: (k: string) => store.get(k) ?? null,
+        setItem: (k: string, v: string) => void store.set(k, v),
+        removeItem: (k: string) => void store.delete(k),
+      },
+    });
+    setDeviceTrusted(true);
+  });
+
+  afterEach(() => {
+    setDeviceTrusted(false);
+    Reflect.deleteProperty(globalThis, "localStorage");
+  });
+
+  it("round-trips, and is off until something turns it on", () => {
+    expect(pushEnabledHere()).toBe(false);
+    setPushEnabledHere(true);
+    expect(pushEnabledHere()).toBe(true);
+    setPushEnabledHere(false);
+    expect(pushEnabledHere()).toBe(false);
+  });
+
+  it("stays off on a device nobody said was theirs", () => {
+    // Push is refused there anyway; reading the flag as set would start the
+    // renewal trying on every load for a subscription that cannot exist.
+    setPushEnabledHere(true);
+    setDeviceTrusted(false);
+    expect(pushEnabledHere()).toBe(false);
+  });
+
+  it("is cleared by signing out, even when the server end cannot be reached", () => {
+    setPushEnabledHere(true);
+    vi.spyOn(client, "call").mockRejectedValue(new Error("offline"));
+    return unsubscribeThisDevice().then(() => {
+      // The subscription may well survive at the server; this browser must
+      // still stop believing it has push, or renewal would resurrect it.
+      expect(pushEnabledHere()).toBe(false);
+    });
+  });
+});

web/src/lib/storage.ts

@@ -22,8 +22,18 @@ const PREFIX = "ihasmail:";
  * - `deviceTrusted` is how the next boot knows to read at all.
  * - `pushDeviceId` is a random id for this browser, so re-subscribing replaces
  *   rather than accumulates. The subscription itself is removed on sign-out.
+ * - `pushEnabled` records that background notifications were switched on here,
+ *   and is what the renewal on app start keys off. It is kept because this
+ *   function runs on two different endings and only one of them is a sign-out:
+ *   a *deploy* expires every session, and the handler for that clears local
+ *   data without removing the push subscription, because there is no longer a
+ *   session to remove it with. Dropping the flag there would leave the
+ *   subscription registered, the switch still reading as on, and nothing
+ *   renewing it -- so push would go quiet a week after every deploy, which is
+ *   the exact failure the renewal exists to prevent. Signing out for real
+ *   clears it directly, in `unsubscribeThisDevice`, alongside the subscription.
  */
-const KEEP_ON_SIGN_OUT = ["lastUser", "deviceTrusted", "pushDeviceId"];
+const KEEP_ON_SIGN_OUT = ["lastUser", "deviceTrusted", "pushDeviceId", "pushEnabled"];
 
 const TRUST_KEY = `${PREFIX}deviceTrusted`;