fix(reader): open the save under the reader lock, and cap automatic backups

Follow-up to the 2026-10-18 review of the v1→v2 migration. Start-up
(finishing a pending restore, then migrating or reconciling) now runs under
the lock every save takes, so a second tab's save cannot be overwritten;
where no lock exists it opens unlocked, as before. The automatic backups get
a size cap like Back up now's: past it the oldest are dropped, never the
newest, leaving the main save room to grow.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

TODO.md

@@ -249,23 +249,23 @@ note only says where to start.
     `EventForm`, `src/client/components/TaskForm.tsx`). Review them as a reader would — field
     order, defaults, copy, validation messages, how a cadence and a measure are chosen, phone
     width — against `docs/DESIGN.md` and `docs/PRODUCT.md`.
-- [ ] Follow up the 2026-10-18 pre-ship review of the v1→v2 reader migration. A schema-guardian review
+- [x] Follow up the 2026-10-18 pre-ship review of the v1→v2 reader migration. A schema-guardian review
   found nothing blocking: event IDs, day keys and the feed contract are unchanged, and v1 keys are
-  never removed, so a rollback finds them byte-identical. These gaps remain, all low severity:
-  - Orphaned v1 entries (marks or amounts for a deleted `myevent:`/`mytask:`, or any entry whose game
-    can't be resolved) are not carried into v2 account data (`state/reader/migration.ts`,
-    `gameOf`). They survive in `legacyRaw`, `gacha-tracker:v2:migration-recovery` and the untouched v1
-    keys. Consider a catch-all bucket so a restored custom item finds them.
-  - Importing a v1 file whose region differs from the reader's account lands its marks in a new
-    `account:import-N`, not the visible account (`state/reader/import.ts`, `export.ts`). Kept, but
-    out of sight.
-  - `initializeReader` reconciles and writes back outside `withReaderLock`, with no compare-and-set
-    (`state/reader/store.ts`). A second tab's write in that window can be overwritten.
-  - Storage pressure: every save carries `legacyRaw`, and every automatic backup embeds the state,
-    `legacyRaw` and the v1 keys again, with no size cap (only manual backups are capped). Heavy
-    readers could reach the ~5 MB quota. Failures are reported, not silent.
-  - After a rollback, anything done in the new build lives only in v2 and isn't shown by the old app.
-    It's stranded, not lost.
+  never removed, so a rollback finds them byte-identical. Done 2026-10-18:
+  - Start-up (finishing a pending restore, then migrating or reconciling) now runs under the reader
+    lock every save takes (`state/reader/open.ts`), so a second tab's save cannot be overwritten.
+    Where no lock exists (no Web Locks, no IndexedDB) it opens unlocked, as before.
+  - The automatic backups have a size cap, `AUTO_BACKUP_BYTES`, like Back up now's: past it the
+    oldest snapshots are dropped, never the newest (`state/reader/backups.ts`). The main save still
+    carries `legacyRaw`, which reconciling and rollback need.
+  - Left as they are, by the agent's call: orphaned v1 entries (marks for a deleted
+    `myevent:`/`mytask:`, or with no resolvable game) stay out of v2 account data, as nothing can
+    show them; they survive in `legacyRaw`, `v2:migration-recovery` and the v1 keys. After a
+    rollback, anything done in the new build is in v2 only: stranded for the old app, not lost.
+- [ ] Decide where an imported v1 file's marks go when its region differs from the reader's account.
+  Today they land in a new `account:import-N`, kept but out of sight (`state/reader/import.ts`,
+  `export.ts`). Options: merge into the visible account for that game, or show the imported account
+  as a second server. Needs the owner's call (found by the 2026-10-18 migration review).
 - [x] Get readers to try the layout choices, not just be told about them (owner, 2026-10-18). Done
   2026-10-18. The tour is now forced once for everyone, including new readers and anyone who saw the
   earlier tour. Its "Pick a layout" and Timeline steps switch the page behind in place, and the

docs/DATA-MODEL.md

@@ -797,12 +797,16 @@ importedExtensions as well as mergeable known fields. Existing same-server setti
 Migration preserves raw v1 keys in place and in migration recovery, creates one account per game at
 the saved region and maps old follows/pins/logs. Newly offered games remain off. Startup compares
 legacyRaw with surviving v1 keys to carry older-tab deltas into the original account without
-resurrecting newer v2 deletions. Parser aliases carry progress, ticks, amounts and archive/seen IDs.
+resurrecting newer v2 deletions. Startup (finishing a pending restore, then migrating or reconciling)
+runs under the same reader lock as every save (`state/reader/open.ts`), so another tab's save
+cannot land between its read and its write. Parser aliases carry progress, ticks, amounts and archive/seen IDs.
 
 Keys outside the envelope:
 
 - gacha-tracker:v2:migration-recovery — raw pre-migration bytes.
-- gacha-tracker:v2:backups — at most 7 daily + 4 weekly + 6 monthly UTC bucket snapshots.
+- gacha-tracker:v2:backups — at most 7 daily + 4 weekly + 6 monthly UTC bucket snapshots, and at
+  most `AUTO_BACKUP_BYTES` (1.5M UTF-16 units) together: past it the oldest are dropped, never the
+  newest. Back up now copies (`v2:manual-backups`) have their own 20-copy, 1.5M cap.
 - gacha-tracker:v2:restore-recovery — the complete pre-restore export.
 - gacha-tracker:v2:restore-pending — prepared replacement, resumed before mounting on startup.
 

src/client/ReaderApp.tsx

@@ -16,13 +16,9 @@ import {
   type FeedState,
   type RefreshOutcome,
 } from "./api.ts";
-import {
-  initializeReader,
-  createReaderStore,
-  type ReaderStore,
-} from "./state/reader/store.ts";
+import { createReaderStore, type ReaderStore } from "./state/reader/store.ts";
+import { openReader } from "./state/reader/open.ts";
 import { ReaderContext, useReader } from "./state/reader/useReader.ts";
-import { finishPendingRestore } from "./state/reader/restore.ts";
 import { saveBackup } from "./state/reader/backups.ts";
 import {
   pairKey,
@@ -128,15 +124,8 @@ export function ReaderApp() {
       .then(setCodes)
       .catch(() => {});
     void fetchFeed(ac.signal)
-      .then((feed) => {
-        const recovery = finishPendingRestore(localStorage);
-        if (!recovery.ok) {
-          setError(
-            "Your interrupted restore could not be recovered. Free browser storage and reload; saved data has been retained.",
-          );
-          return;
-        }
-        const initialized = initializeReader(
+      .then(async (feed) => {
+        const opened = await openReader(
           localStorage,
           [
             ...new Set([
@@ -147,13 +136,16 @@ export function ReaderApp() {
           new Date().toISOString(),
           mintAccountId,
         );
-        if (!initialized.ok) {
+        if (ac.signal.aborted) return;
+        if (!opened.ok) {
           setError(
-            "Your saved data could not be opened safely. It has been retained. Check browser storage access and reload.",
+            opened.reason === "restore"
+              ? "Your interrupted restore could not be recovered. Free browser storage and reload; saved data has been retained."
+              : "Your saved data could not be opened safely. It has been retained. Check browser storage access and reload.",
           );
           return;
         }
-        const created = createReaderStore(localStorage, initialized.state);
+        const created = createReaderStore(localStorage, opened.state);
         setStore(created);
         setFeed({ status: "ready", feed });
       })

src/client/state/reader/backups.ts

@@ -60,8 +60,19 @@ export function saveBackup(
     [...previous, { id: mintLocalId(), at, payload }],
     Date.parse(at),
   );
+  // Oldest first, but never the newest: one recent copy is worth more than
+  // the room, and past the cap the main save is what would run out.
+  while (next.length > 1 && JSON.stringify(next).length > AUTO_BACKUP_BYTES)
+    next.pop();
   return tryWriteJson(storage, READER_KEYS.backups, next);
 }
+/**
+ * The most the automatic history may take in local storage, in UTF-16 units.
+ * Each snapshot embeds the state, `legacyRaw` and the v1 keys, and up to 17
+ * are retained; for a heavy reader that alone could fill the origin's ~5MB.
+ * Together with `MANUAL_BACKUP_BYTES` this leaves the main save room to grow.
+ */
+export const AUTO_BACKUP_BYTES = 1_500_000;
 /**
  * Which part of the retained history a snapshot belongs to, with the same
  * windows `retainBackups` keeps: the last 7 days, the 4 weeks before them,

src/client/state/reader/open.ts

@@ -0,0 +1,35 @@
+import type { LaneId } from "../../../shared/custom.ts";
+import { withReaderLock } from "./lock.ts";
+import { finishPendingRestore } from "./restore.ts";
+import { initializeReader } from "./store.ts";
+import type { AccountId, ReaderState } from "./schema.ts";
+
+/**
+ * Open the reader's save at start-up: finish an interrupted restore, then
+ * migrate or reconcile and write back. Both read the envelope and write it
+ * again, so they run under the lock every save takes; otherwise a second
+ * tab's save landing between the read and the write would be overwritten.
+ */
+export async function openReader(
+  storage: Storage,
+  games: readonly LaneId[],
+  at: string,
+  mint: () => AccountId,
+): Promise<
+  { ok: true; state: ReaderState } | { ok: false; reason: "restore" | "open" }
+> {
+  const open = () => {
+    if (!finishPendingRestore(storage).ok)
+      return { ok: false, reason: "restore" } as const;
+    const opened = initializeReader(storage, games, at, mint);
+    return opened.ok ? opened : ({ ok: false, reason: "open" } as const);
+  };
+  // Both steps catch their own errors, so a throw here is the lock itself
+  // being unavailable (no Web Locks and no IndexedDB, as in some private
+  // windows). Opening unlocked then is what start-up did before the lock.
+  try {
+    return await withReaderLock(open);
+  } catch {
+    return open();
+  }
+}

test/reader-manual-backups.test.ts

@@ -1,5 +1,6 @@
 import { expect, test } from "bun:test";
 import {
+  AUTO_BACKUP_BYTES,
   deleteManualBackup,
   MANUAL_BACKUP_BYTES,
   MANUAL_BACKUP_LIMIT,
@@ -88,3 +89,36 @@ test("the list refuses a copy that would push it past its size budget", () => {
   });
   expect(readManualBackups(storage)).toEqual([]);
 });
+
+test("the automatic history drops its oldest copies past its size cap, never the newest", () => {
+  const storage = memory();
+  // A heavy reader: about 300k units of v1 data embedded in every snapshot.
+  storage.setItem(
+    "gacha-tracker:v1:progress",
+    JSON.stringify("x".repeat(300_000)),
+  );
+  const day = 86_400_000;
+  for (let i = 0; i < 10; i++)
+    expect(
+      saveBackup(
+        storage,
+        state,
+        new Date(Date.parse(at) + i * day).toISOString(),
+      ).ok,
+    ).toBe(true);
+  const kept = readBackups(storage);
+  expect(storage.getItem(READER_KEYS.backups)!.length).toBeLessThanOrEqual(
+    AUTO_BACKUP_BYTES,
+  );
+  expect(kept.length).toBeGreaterThan(1);
+  expect(kept.length).toBeLessThan(7);
+  expect(kept[0]!.at).toBe(new Date(Date.parse(at) + 9 * day).toISOString());
+
+  storage.setItem(
+    "gacha-tracker:v1:progress",
+    JSON.stringify("x".repeat(AUTO_BACKUP_BYTES)),
+  );
+  const last = new Date(Date.parse(at) + 10 * day).toISOString();
+  expect(saveBackup(storage, state, last).ok).toBe(true);
+  expect(readBackups(storage).map((b) => b.at)).toEqual([last]);
+});

test/reader-open.test.ts

@@ -0,0 +1,73 @@
+import { expect, test } from "bun:test";
+import { openReader } from "../src/client/state/reader/open.ts";
+import { READER_KEYS } from "../src/client/state/reader/schema.ts";
+import { MemoryStorage } from "./helpers/reader.ts";
+
+const at = "2026-10-18T12:00:00.000Z";
+
+test("start-up reads and writes the save inside the reader lock", async () => {
+  const s = new MemoryStorage();
+  const seen: string[] = [];
+  const locks = {
+    request: async (name: string, run: () => unknown) => {
+      seen.push(`lock ${name}`);
+      const value = await run();
+      seen.push(`release, saved: ${s.getItem(READER_KEYS.reader) !== null}`);
+      return value;
+    },
+  };
+  const original = Object.getOwnPropertyDescriptor(globalThis, "navigator");
+  Object.defineProperty(globalThis, "navigator", {
+    value: { locks },
+    configurable: true,
+  });
+  try {
+    const opened = await openReader(s, ["genshin"], at, () => "account:one");
+    expect(opened.ok).toBe(true);
+  } finally {
+    if (original) Object.defineProperty(globalThis, "navigator", original);
+    else delete (globalThis as { navigator?: unknown }).navigator;
+  }
+  expect(seen).toEqual([
+    "lock gacha-tracker:v2:reader",
+    "release, saved: true",
+  ]);
+});
+
+test("an interrupted restore that cannot be read stops start-up, keeping the save", async () => {
+  const s = new MemoryStorage();
+  s.setItem(READER_KEYS.pending, JSON.stringify({ at, payload: {} }));
+  const opened = await openReader(s, ["genshin"], at, () => "account:one");
+  expect(opened).toEqual({ ok: false, reason: "restore" });
+  expect(s.getItem(READER_KEYS.pending)).not.toBeNull();
+  expect(s.getItem(READER_KEYS.reader)).toBeNull();
+});
+
+test("an unreadable save is reported, not replaced", async () => {
+  const s = new MemoryStorage();
+  s.setItem(READER_KEYS.reader, JSON.stringify({ version: 2 }));
+  const opened = await openReader(s, ["genshin"], at, () => "account:one");
+  expect(opened).toEqual({ ok: false, reason: "open" });
+  expect(s.getItem(READER_KEYS.reader)).toBe(JSON.stringify({ version: 2 }));
+});
+
+test("with no lock available, start-up still opens the save", async () => {
+  const s = new MemoryStorage();
+  const original = Object.getOwnPropertyDescriptor(globalThis, "navigator");
+  Object.defineProperty(globalThis, "navigator", {
+    value: {
+      locks: {
+        request: () => Promise.reject(new Error("unavailable")),
+      },
+    },
+    configurable: true,
+  });
+  try {
+    const opened = await openReader(s, ["genshin"], at, () => "account:one");
+    expect(opened.ok).toBe(true);
+  } finally {
+    if (original) Object.defineProperty(globalThis, "navigator", original);
+    else delete (globalThis as { navigator?: unknown }).navigator;
+  }
+  expect(s.getItem(READER_KEYS.reader)).not.toBeNull();
+});