diff --git a/TODO.md b/TODO.md index bb337e6..796b57e 100644 --- a/TODO.md +++ b/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 diff --git a/docs/DATA-MODEL.md b/docs/DATA-MODEL.md index 0da12ba..1c997c6 100644 --- a/docs/DATA-MODEL.md +++ b/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. diff --git a/src/client/ReaderApp.tsx b/src/client/ReaderApp.tsx index 50b71b7..d07b414 100644 --- a/src/client/ReaderApp.tsx +++ b/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 }); }) diff --git a/src/client/state/reader/backups.ts b/src/client/state/reader/backups.ts index b71805d..4c94357 100644 --- a/src/client/state/reader/backups.ts +++ b/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, diff --git a/src/client/state/reader/open.ts b/src/client/state/reader/open.ts new file mode 100644 index 0000000..5ab86cf --- /dev/null +++ b/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(); + } +} diff --git a/test/reader-manual-backups.test.ts b/test/reader-manual-backups.test.ts index 97b0f01..2ffc595 100644 --- a/test/reader-manual-backups.test.ts +++ b/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]); +}); diff --git a/test/reader-open.test.ts b/test/reader-open.test.ts new file mode 100644 index 0000000..1c05c6a --- /dev/null +++ b/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(); +});