fix(sidebar): write a read finish back when another tab removed it unheard

changelog.d/seen-write-stale-cache.md

@@ -0,0 +1,4 @@
+### Fixed
+- A finish read on another device no longer turns unread again in a browser where another tab
+  removed its record while no sidebar was open: the read record is written back instead of
+  being skipped as already stored.

src/lib/seenFinishes.test.ts

@@ -0,0 +1,67 @@
+import { afterEach, beforeEach, expect, it } from "bun:test";
+import { loadSeen, receiveSeen, saveSeen, seenKey, subscribeSeen } from "./seenFinishes.ts";
+
+// A finish this page acknowledged must reach storage again once another tab removed it while no
+// subscriber here listened for storage events (#1539); a subscriber's listener keeps the cache true.
+const descriptors = new Map<string, PropertyDescriptor | undefined>();
+let data: Map<string, string>;
+let writes: number;
+
+beforeEach(() => {
+  for (const key of ["window", "localStorage"]) descriptors.set(key, Object.getOwnPropertyDescriptor(globalThis, key));
+  data = new Map();
+  writes = 0;
+  const events = new EventTarget();
+  Object.defineProperty(globalThis, "window", { configurable: true, value: {
+    addEventListener: events.addEventListener.bind(events), removeEventListener: events.removeEventListener.bind(events),
+  } });
+  // a new object each test: the module drops what it cached for another storage
+  Object.defineProperty(globalThis, "localStorage", { configurable: true, value: {
+    get length() { return data.size; }, key: (index: number) => [...data.keys()][index] ?? null,
+    getItem: (key: string) => data.get(key) ?? null,
+    setItem: (key: string, value: string) => { writes++; data.set(key, value); },
+    removeItem: (key: string) => { data.delete(key); },
+  } });
+});
+
+afterEach(() => {
+  for (const [key, descriptor] of descriptors) {
+    if (descriptor) Object.defineProperty(globalThis, key, descriptor);
+    else Reflect.deleteProperty(globalThis, key);
+  }
+  descriptors.clear();
+});
+
+const finish = { pc: "local", epoch: "e1", pane: "w1:p1", id: 3, answer: "done" };
+const key = seenKey(finish.pc, finish.epoch, finish.pane);
+
+it("stores a finish again that another tab removed while nothing here listened", () => {
+  receiveSeen([finish]);
+  expect(data.has(key)).toBe(true);
+  data.delete(key);
+  receiveSeen([finish]);
+  expect(JSON.parse(data.get(key) ?? "null")).toMatchObject({ id: 3, answer: "done" });
+  expect(loadSeen(finish.pc, finish.epoch)[finish.pane]).toBe(3);
+});
+
+it("reads storage for a repeated acknowledgement only while no subscriber hears other tabs", () => {
+  const stop = subscribeSeen(() => {});
+  try {
+    saveSeen(finish.pc, finish.epoch, finish.pane, finish.id, finish.answer);
+    let reads = 0;
+    const getItem = localStorage.getItem.bind(localStorage);
+    localStorage.getItem = (name) => { reads++; return getItem(name); };
+    saveSeen(finish.pc, finish.epoch, finish.pane, finish.id, finish.answer);
+    expect(reads).toBe(0);
+  } finally { stop(); }
+});
+
+it("keeps a newer finish it holds when storage lost it, and writes nothing while storage still has it", () => {
+  saveSeen(finish.pc, finish.epoch, finish.pane, 5, "later");
+  const before = writes;
+  saveSeen(finish.pc, finish.epoch, finish.pane, 3, "done");
+  expect(writes).toBe(before);
+  data.delete(key);
+  saveSeen(finish.pc, finish.epoch, finish.pane, 3, "done");
+  expect(JSON.parse(data.get(key) ?? "null")).toMatchObject({ id: 5, answer: "later" });
+});

src/lib/seenFinishes.ts

@@ -268,11 +268,16 @@ function write(pc: string, epoch: string, pane: string, id: number, answer?: str
   hydrate();
   const key = seenKey(pc, epoch, pane);
   const held = records.get(key);
+  const storage = storageNow();
   if (held?.stored && (held.id > id || (held.id === id && (held.answer !== undefined || answer === undefined)))) {
-    return { id: held.id, changes: [] };
+    // A subscriber's storage listener keeps `stored` true to what other tabs did. With none, another
+    // tab may have removed the key since it was read: look, and write the record back if so.
+    if (listeningWindow !== null || !storage) return { id: held.id, changes: [] };
+    let current: StoredFinish | null = null;
+    try { current = parse(storage.getItem(key)); } catch { return { id: held.id, changes: [] }; }
+    if (current !== null && current.id >= held.id) return { id: held.id, changes: [] };
   }
 
-  const storage = storageNow();
   let saved: StoredFinish | null = null;
   let read = false;
   try {