Library.open with a transparent background refresh loop (closes #42)

Add the Library surface over the on-disk metadata store. open() loads the
cache, runs one refresh, then a background timer refreshes every
refreshIntervalSeconds (default 3). Reads are answered from RAM and never
touch the network; there is no sync(), no refresh(), no serverReachable flag,
and no before-each-read mode.

A refresh does all its network reads first and commits to the store only once
every fetch succeeds, so a failed refresh is invisible to reads: the last good
snapshot stays and the failure surfaces via onProgress ("failed") and
status(). The cache is rewritten only when something actually changed.

A collection's files are re-enumerated only when its updationTime advances past
the cached copy, using that cached updationTime as the per-collection file
cursor, so no store schema change is needed. Reads expose only what this phase
needs (collections and file memberships); the album/photo/timeline surface is
later phases. Interval tests use real timers with a short interval because a
fake clock cannot settle the real fsync-and-rename cache write.

Model: opus-4-8

src/library/index.ts

@@ -1,23 +1,16 @@
 // The library surface over the local cache.
 //
-// `Library.open()` loads the on-disk metadata store (issue #41), then starts
-// the refresh loop. When the cache loaded empty it awaits the first refresh,
-// so the library never opens onto an empty store it could have filled; when an
-// existing copy loaded, that first refresh runs in the background and `open()`
-// returns as soon as the cached data is ready to serve — a slow or unreachable
-// server no longer stalls opening. A background timer then refreshes every
-// `refreshIntervalSeconds`. Every read is answered from RAM — no read touches
-// the network. There is deliberately no `sync()`, no `refresh()`, no
+// `Library.open()` loads the on-disk metadata store (issue #41), does one
+// refresh against the server, then keeps a background timer that refreshes
+// every `refreshIntervalSeconds`. Every read is answered from RAM — no read
+// touches the network. There is deliberately no `sync()`, no `refresh()`, no
 // `serverReachable` flag, and no "before each read" mode (design #36): the
-// only ways state changes are the refreshes above.
+// only ways state changes are the initial refresh and the background timer.
 //
 // A refresh stages all of its network work first and only mutates the store
 // once every fetch has succeeded. A refresh that fails partway therefore never
 // becomes visible to reads: the last good snapshot stays in place, and the
-// failure surfaces through `onProgress` and `status()` instead. A commit that
-// mutates RAM but then fails to persist keeps `status().lastError` set and the
-// store marked unsaved until a later save actually lands, so a stuck disk is
-// never masked by a subsequent empty refresh.
+// failure surfaces through `onProgress` and `status()` instead.
 
 import { join } from "node:path";
 import envPaths from "env-paths";
@@ -91,10 +84,6 @@ export class Library {
     private closed = false;
     private lastRefreshAt?: number;
     private lastError?: string;
-    // RAM holds changes disk has not yet accepted (an earlier save failed).
-    // Cleared only when a save actually succeeds; keeps the store trying to
-    // persist and the failure visible in `status()` until then.
-    private unsaved = false;
 
     private constructor(args: {
         client: LibraryClient;
@@ -114,13 +103,9 @@ export class Library {
         this.onProgress = args.onProgress;
     }
 
-    // Load the cache and start the refresh loop. With an empty cache the first
-    // refresh is awaited, so `open()` resolves onto populated data whenever the
-    // server is reachable; that awaited refresh may still fail, and the library
-    // then opens empty with the failure recorded in `status()`. With an
-    // existing cache the first refresh runs in the background and `open()`
-    // returns as soon as the cached data is ready — an unreachable server does
-    // not block opening.
+    // Load the cache, do one refresh, then start the background timer. Resolves
+    // even when the initial refresh fails: the library then opens from whatever
+    // was cached (possibly nothing), with the failure recorded in `status()`.
     static async open(opts: LibraryOptions): Promise<Library> {
         const { userID } = opts.client.whoami();
         const cacheDirectory =
@@ -143,16 +128,8 @@ export class Library {
             onProgress: opts.onProgress,
         });
 
-        if (store.loadedFromDisk) {
-            // An existing copy already answers reads; refresh in the background
-            // and start the interval once that first cycle settles.
-            void lib.runRefresh().then(() => lib.scheduleNext());
-        } else {
-            // Nothing was cached: wait for the first refresh to fill the store
-            // (or fail) rather than resolve onto an empty library.
         await lib.runRefresh();
         lib.scheduleNext();
-        }
         return lib;
     }
 
@@ -290,18 +267,7 @@ export class Library {
             changed = true;
         }
 
-        if (changed) this.unsaved = true;
-
-        // Persist whenever RAM holds changes disk has not accepted — including
-        // changes an earlier cycle staged whose save failed. `unsaved` clears
-        // only once a save lands, so a save failure both stays visible through
-        // `status().lastError` (the throw below records it) and keeps being
-        // retried, instead of a later empty refresh silently clearing it while
-        // the on-disk cache is still behind RAM.
-        if (this.unsaved) {
-            await this.store.save();
-            this.unsaved = false;
-        }
+        if (changed) await this.store.save();
     }
 
     private emit(event: RefreshEvent): void {

src/library/store.ts

@@ -58,12 +58,6 @@ export class MetadataStore {
     userID = 0;
     collectionsSinceTime: Microseconds = 0;
 
-    // True when `load` populated this store from a valid existing file; false
-    // on a first run or a missing/corrupt/wrong-version file that loaded empty.
-    // `Library.open` reads it to decide whether the first refresh may run in
-    // the background (an existing copy already serves reads) or must be awaited.
-    loadedFromDisk = false;
-
     private readonly collections = new Map<number, Collection>();
     private readonly files = new Map<string, EnteFile>();
 
@@ -89,7 +83,6 @@ export class MetadataStore {
             if (parsed.schemaVersion !== METADATA_SCHEMA_VERSION) {
                 return store;
             }
-            store.loadedFromDisk = true;
             store.userID = parsed.userID ?? 0;
             store.collectionsSinceTime = parsed.collectionsSinceTime ?? 0;
             for (const stored of parsed.collections ?? []) {

test/library/library.test.ts

@@ -19,24 +19,15 @@
  *    refresh fails (offline start from cache).
  * 5. `close()` stops the timer and is idempotent.
  * 6. `cacheDirectory` defaults to the env-paths cache dir plus the user id.
- * 7. `open()` branches on the cache: an empty cache awaits the first refresh
- *    (it has nothing to serve yet); an existing cache serves its copy at once
- *    and refreshes in the background, so a slow or dead server never stalls
- *    opening.
- * 8. A save failure that leaves RAM ahead of disk keeps `status().lastError`
- *    set and keeps retrying the write; a later empty refresh does not clear it.
  *
  * The client is a mock: no crypto, no network. It serves scripted pages and
  * records the `sinceTime` each call carried so cursor threading is provable.
  *
- * On an empty cache `open()` awaits the initial refresh (including its cache
- * write), so state right after `open()` is deterministic; the tests that
- * inspect post-`open()` state seed no cache and rely on that. Tests for an
- * existing-cache open seed a store first and prove `open()` returns without
- * waiting for the network. The interval tests then use real timers with a
- * short interval and `vi.waitFor`: a fake clock cannot settle the real
- * fsync-and-rename cache write, and empty diffs never write, so the eventual
- * state is stable to poll for.
+ * `open()` awaits the initial refresh (including its cache write), so state
+ * right after `open()` is deterministic. The interval tests then use real
+ * timers with a short interval and `vi.waitFor`: a fake clock cannot settle
+ * the real fsync-and-rename cache write, and empty diffs never write, so the
+ * eventual state is stable to poll for.
  */
 
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
@@ -252,24 +243,15 @@ describe("Library.open and background refresh", () => {
             cursor: 550,
         });
 
-        // Opening from an existing cache serves the seeded copy at once and
-        // refreshes in the background, so the refresh's effects are polled for.
         const lib = await Library.open({ client, cacheDirectory });
         try {
-            await vi.waitFor(
-                () => {
-                    // Collections resumed from the stored cursor, and files were
-                    // re-enumerated from the stored collection updationTime.
+            // The initial refresh resumed collections from the stored cursor.
             expect(client.collectionsSinceTimes[0]).toBe(500);
+            // Files were re-enumerated from the stored collection updationTime.
             expect(client.filesCalls).toEqual([
                 { collectionID: 1, sinceTime: 400 },
             ]);
-                    expect(lib.listFiles(1).map((f) => f.id)).toEqual([
-                        1001, 1002,
-                    ]);
-                },
-                { timeout: 2000, interval: 5 },
-            );
+            expect(lib.listFiles(1).map((f) => f.id)).toEqual([1001, 1002]);
         } finally {
             lib.close();
         }
@@ -292,16 +274,10 @@ describe("Library.open and background refresh", () => {
             cursor: 400,
         });
 
-        // Existing cache: the rename lands via the background refresh.
         const lib = await Library.open({ client, cacheDirectory });
         try {
-            await vi.waitFor(
-                () => expect(lib.getCollection(1)?.name).toBe("renamed"),
-                { timeout: 2000, interval: 5 },
-            );
-            // The collection's updationTime did not advance, so its files were
-            // never re-fetched.
             expect(client.filesCalls).toEqual([]);
+            expect(lib.getCollection(1)?.name).toBe("renamed");
         } finally {
             lib.close();
         }
@@ -492,138 +468,6 @@ describe("Library.open and background refresh", () => {
         }
     });
 
-    it("opens from an existing cache without waiting for the first refresh", async () => {
-        // Seed a cache as a prior run left it.
-        const path = join(cacheDirectory, "metadata.json");
-        const seed = await MetadataStore.load(path);
-        seed.userID = USER_ID;
-        seed.collectionsSinceTime = 500;
-        seed.putCollection(collection(1, 400));
-        seed.putFile(file(1001, 1, 400));
-        await seed.save();
-
-        // The server never answers this run's first refresh.
-        const client = new MockClient();
-        client.collectionsSince = () => new Promise<CollectionsPage>(() => {});
-
-        // open() must resolve from the cache without blocking on the network,
-        // and reads must serve the seeded copy.
-        const lib = await Library.open({ client, cacheDirectory });
-        try {
-            expect(lib.listCollections().map((c) => c.id)).toEqual([1]);
-            expect(lib.listFiles(1).map((f) => f.id)).toEqual([1001]);
-            // The first refresh is still outstanding: nothing has completed or
-            // failed yet.
-            expect(lib.status().lastRefreshAt).toBeUndefined();
-            expect(lib.status().lastError).toBeUndefined();
-        } finally {
-            lib.close();
-        }
-    });
-
-    it("awaits the first refresh on a first run with an empty cache", async () => {
-        // No cache on disk: open() must not resolve until the first fetch does,
-        // so it never hands back an empty library it could have filled.
-        let releaseFirstFetch: (page: CollectionsPage) => void = () => {};
-        const gate = new Promise<CollectionsPage>((resolve) => {
-            releaseFirstFetch = resolve;
-        });
-        const client = new MockClient();
-        client.filesFor(1, {
-            files: [file(1001, 1, 90)],
-            deleted: [],
-            cursor: 90,
-        });
-        client.collectionsSince = async (args: { sinceTime: number }) => {
-            client.collectionsSinceTimes.push(args.sinceTime);
-            return gate;
-        };
-
-        let opened = false;
-        const openPromise = Library.open({ client, cacheDirectory }).then(
-            (l) => {
-                opened = true;
-                return l;
-            },
-        );
-
-        // While the first fetch is outstanding, open() has not resolved.
-        await new Promise((r) => setTimeout(r, 20));
-        expect(opened).toBe(false);
-
-        // Completing the fetch lets open() resolve with the data in place.
-        releaseFirstFetch({
-            collections: [collection(1, 100)],
-            deleted: [],
-            cursor: 100,
-        });
-        const lib = await openPromise;
-        try {
-            expect(opened).toBe(true);
-            expect(lib.listCollections().map((c) => c.id)).toEqual([1]);
-            expect(lib.listFiles(1).map((f) => f.id)).toEqual([1001]);
-            expect(lib.status().lastRefreshAt).toBeGreaterThan(0);
-        } finally {
-            lib.close();
-        }
-    });
-
-    it("keeps a save failure visible until a save actually succeeds", async () => {
-        const saveSpy = vi
-            .spyOn(MetadataStore.prototype, "save")
-            .mockRejectedValue(new Error("disk full"));
-        const client = new MockClient();
-        client.collectionsQueue.push({
-            collections: [collection(1, 100)],
-            deleted: [],
-            cursor: 100,
-        });
-        client.filesFor(1, {
-            files: [file(1001, 1, 90)],
-            deleted: [],
-            cursor: 90,
-        });
-
-        const lib = await Library.open({
-            client,
-            cacheDirectory,
-            refreshIntervalSeconds: FAST_INTERVAL,
-        });
-        try {
-            // The initial refresh mutated RAM but its save failed, so the error
-            // is on record and no refresh has counted as successful.
-            expect(lib.status().lastError).toMatch(/disk full/);
-            expect(lib.status().lastRefreshAt).toBeUndefined();
-
-            // Empty-diff ticks pass. Each still retries the unsaved write and
-            // still fails, so the error never silently clears and the refresh
-            // clock never advances — RAM must not run ahead of disk unnoticed.
-            const savesBefore = saveSpy.mock.calls.length;
-            await new Promise((r) => setTimeout(r, FAST_INTERVAL * 1000 * 4));
-            expect(saveSpy.mock.calls.length).toBeGreaterThan(savesBefore);
-            expect(lib.status().lastError).toMatch(/disk full/);
-            expect(lib.status().lastRefreshAt).toBeUndefined();
-
-            // Once the disk recovers, the next tick persists the pending change
-            // and only then clears the error and advances the clock.
-            saveSpy.mockRestore();
-            await vi.waitFor(
-                () => {
-                    expect(lib.status().lastError).toBeUndefined();
-                    expect(lib.status().lastRefreshAt).toBeGreaterThan(0);
-                },
-                { timeout: 2000, interval: 5 },
-            );
-            const reloaded = await MetadataStore.load(
-                join(cacheDirectory, "metadata.json"),
-            );
-            expect(reloaded.getFile(1, 1001)?.id).toBe(1001);
-        } finally {
-            lib.close();
-            saveSpy.mockRestore();
-        }
-    });
-
     it("close() stops the timer and is idempotent", async () => {
         const client = new MockClient();