quak files COLLECTION fetches that collection's files alone (closes #217)

`quak files COLLECTION` used to wait for a full refresh, which on an
empty cache fetched the files of every collection first. It now reads
the collection list, then calls the new `Library.refreshCollection()`.
That brings one collection up to date in the store as a refresh would.
Once the stored cursor is past 0, it only updates a collection the store
holds. For a new one, `quak files COLLECTION` waits for a refresh. A
refresh from a cursor of 0 replaces all the store holds, so collections
then sit in the server's order. `open()` awaits the first refresh while
the cursor is 0. The store lists what did not decrypt in a fixed order.

Model: opus-5-5

README.md

@@ -651,17 +651,17 @@ The CLI stores the snapshot at the platform-appropriate data directory via
 `0600`. The key material is stored in cleartext in the JSON; treat this file as
 you would treat the password itself. A missing file is reported as "not logged
 in"; a file that exists but is corrupt is reported as such, naming the bad
-field. When the refresh a command starts with gets HTTP 401 from the server,
+field. The server answers HTTP 401 when it no longer accepts the saved session's
-because it no longer accepts the saved session's token, the command prints one
+token. A command that gets that answer prints one line,
-line, `quak: the saved session is no longer valid; run "quak login"`, with no
+`quak: the saved session is no longer valid; run "quak login"`, with no stack
-stack trace. All three exit with status 3, which means the user must run
+trace. All three exit with status 3, which means the user must run `quak login`
-`quak login` again. `quak logout` is the exception: with no file it says there
+again. `quak logout` is the exception: with no file it says there is no session
-is no session and exits 0, and it handles a corrupt file or a failed server call
+and exits 0, and it handles a corrupt file or a failed server call as described
-as described below. `quak backup` meets an expired session on the refresh that
+below. `quak backup` meets an expired session on the refresh that starts every
-starts every run, before it touches any file but its lock (see "Backup layout").
+run, before it touches any file but its lock (see "Backup layout"). A session
-A session that stops working partway through a backup instead fails each
+that stops working partway through a backup instead fails each remaining file
-remaining file into `failures.json`, so that run exits 1 and the next one stops
+into `failures.json`, so that run exits 1 and the next one stops at its refresh
-at its refresh with status 3. No command but `quak login` ever prompts.
+with status 3. No command but `quak login` ever prompts.
 
 `quak logout` ends the session on the server, so the token in `session.json`
 stops working even in a copy of the file, and then deletes the file. If the
@@ -708,12 +708,14 @@ fetches no collection's files and leaves the cache as it is. `files` with a
 collection ID asks for the collection list the same way. It then asks for the
 files of that collection alone, and stores them in the cache. It asks for no
 files when the cache's copy of that collection is current. The other read
-commands, and `files` without a collection ID, wait for a refresh.
+commands, and `files` without a collection ID, wait for a refresh. So does
+`files` with the ID of a collection that is new since the last refresh.
 
-On an empty cache, as on a first run, that refresh fetches the files of every
+The first refresh fetches the files of every collection, one collection after
-collection, one collection after another. It stores nothing until it has them
+another. That includes any collection that `files` with a collection ID stored
-all, and on a large account that takes a while. Meanwhile the command logs a
+before it. The refresh stores nothing until it has them all, and on a large
-line on stderr for each collection whose files it has fetched, such as
+account that takes a while. Meanwhile the command logs a line on stderr for each
+collection whose files it has fetched, such as
 `Fetched the files of 3 of 120 collections`. It logs these lines with `--json`
 too. `backup` logs them during the refresh it starts with. stdout holds only the
 command's output.
@@ -777,9 +779,10 @@ its `Failed files:` list. Every other line is logged as it is without `--json`.
 exit status 1.
 
 `files <collectionID>` lists the files in that collection. `files` with no
-argument lists every file in the account, collection by collection, each file
+argument lists every file in the account once, collection by collection. The
-once: a file in several collections is listed with the first of them, which is
+collections come in the order the server first sent them in a refresh. A file in
-its `collectionID` in `--json`.
+several collections is listed with the first of them, which is its
+`collectionID` in `--json`.
 
 An ID given to `files`, `get`, `get-thumb` or
 `helper fix-missing-thumbnails --file` is written in decimal digits, no larger
@@ -1159,7 +1162,8 @@ In between, a refresh sends `"progress"` after each collection whose files it
 fetched. That event's `fetched` is the number of collections whose files the
 refresh has fetched so far. Its `total` is the number of collections whose files
 it will fetch in all. A refresh fetches the files of each collection that is new
-or changed since the cache's copy. On an empty cache, that is every collection.
+or changed since the cache's copy. On the first refresh, that is every
+collection.
 
 Concurrency is set through `pools`: construct
 `new RequestPools({ metadataConcurrency, contentConcurrency, thumbnailConcurrency })`
@@ -1214,17 +1218,18 @@ waits for it to end. When a refresh is already running, background or not,
 `fresh()` waits for that one. Its answer can then come from requests made before
 the call. Only when none is running does it start one. A refresh that fails
 rejects the caller (default reads stay silent and keep serving the last good
-copy). Every CLI read command calls `fresh()`, except `quak collections` and
+copy). Every CLI read command calls `fresh()`, except `quak collections`.
-`quak files` with a collection ID.
+`quak files` with a collection ID calls it only when `lib.refreshCollection()`
+resolves to `false`.
 
 `await lib.freshCollections()` is the fresh read of the collection list alone.
 It sends one collections request. It answers with every live collection that
 decrypts, in the server's order, as the `Collection` objects that
 `listCollections()` returns. It fetches no collection's files and stores
-nothing, not even the list. A refresh fetches a collection's files only when the
+nothing, not even the list. A refresh from a cursor past 0 fetches a
-collection is new or has changed since the stored copy. So a collection stored
+collection's files only when the collection is new or has changed since the
-without its files would never get them. `quak collections` uses
+stored copy. So does `lib.refreshCollection()`. Neither would fetch the files of
-`freshCollections()`.
+a collection stored without them. `quak collections` uses `freshCollections()`.
 
 `await lib.refreshCollection(collection)` brings one collection up to date in
 the cache, given its `Collection` from `freshCollections()`. It does for that
@@ -1232,8 +1237,19 @@ collection what a refresh does for a changed one, and fetches no other
 collection's files. It fetches the collection's files changed since the cache's
 copy, or all of them when the cache has no copy. It then stores and saves them.
 It sends no request when the cache's copy has the same `updationTime` or a later
-one. It also sends none when a refresh has already dropped the collection from
+one. It resolves to `true` when the cache then holds the collection.
-the cache, because the server deleted it or it did not decrypt.
+
+`lib.refreshCollection()` adds a collection the cache does not hold only while
+the refresh cursor is 0. The cursor moves past 0 once a refresh receives the
+account's collections. After that, for a collection the cache does not hold, it
+sends no request and resolves to `false`. Such a collection is new since that
+refresh, or the refresh dropped it. Only a refresh may then add it, after the
+collections the cache holds.
+
+A refresh from a cursor of 0 replaces everything the cache holds. It fetches the
+files of every collection, even those `lib.refreshCollection()` stored. It
+stores the collections in the server's order. The cache then holds what it would
+had `lib.refreshCollection()` never run.
 
 `lib.refreshCollection()` leaves the refresh cursor where it was, so the next
 refresh still fetches the files of every other collection that changed. It never

TODO.md

@@ -33,9 +33,13 @@ declares one.
   one collection up to date through the new `lib.refreshCollection()`. That
   stores and saves its files as a refresh would, and leaves the refresh cursor
   where it was. When the stored copy is current, it asks for none of the files.
-  It never runs at the same time as a refresh. A library opened on a cache that
+  It never runs at the same time as a refresh. Once a refresh has received the
-  only it wrote awaits its first refresh. `quak files` with no argument still
+  account's collections, it stores no collection the cache does not hold, and
-  waits for a refresh.
+  `quak files COLLECTION` waits for a refresh instead. A refresh from a cursor
+  of 0 replaces all the cache holds, so collections keep the server's order. A
+  library opened on a cache that only `lib.refreshCollection()` wrote awaits its
+  first refresh. `quak files` with no argument still waits for a refresh. The
+  store lists what did not decrypt by collection ID and file ID.
 
 - 2026-10-22: The JSON `quak backup` and `quak backup-metadata` write for a file
   holds `keywords` in place of the CLIP embedding in its ML data

src/cli-commands.ts

@@ -131,8 +131,9 @@ const readLibraryClient = (client: Client): LibraryClient => ({
 // Open a library for a single point command. The aggressive background
 // precache (issue #48) is off, because a one-shot `collections` or `get` must
 // not start downloading the whole account. No refresh starts as it opens or on
-// a timer. Each command asks for the refresh it needs, and `collections` and
+// a timer. Each command asks for the refresh it needs. `collections` needs none
-// `files COLLECTION` need none (issues #210 and #217). While a refresh runs,
+// (issue #210). `files COLLECTION` needs one only when `refreshCollection()`
+// resolves to `false` (issue #217). While a refresh runs,
 // each collection whose files it has fetched is logged, so a large account
 // shows progress rather than nothing.
 const openReadLibrary = (ctx: CliContext, client: Client): Promise<Library> =>

src/cli-read.ts

@@ -12,16 +12,20 @@
 // `files COLLECTION` reads the collection list the same way, for the same
 // reason. It then brings that one collection up to date through
 // `Library.refreshCollection()` (issue #217), and fetches no other collection's
-// files.
+// files. Once a refresh has received the account's collections,
+// `refreshCollection()` stores no collection the library does not hold.
+// `files COLLECTION` then forces a refresh instead.
 //
-// `collections` lists in the server's order and `files` in the library's own
+// `collections` lists in the server's order. `files` lists in the library's
-// enumeration order (`listFiles()`), the orders the pre-library CLI printed,
+// own enumeration order: `listCollections()`, then `listFiles()` for each.
-// rather than the `albums`/`photos` projection's newest-first order, which
+// Those are the orders the pre-library CLI printed. The `albums`/`photos`
-// re-sorts the rows. The field values still come from each record's raw
+// projection would re-sort the rows newest first. After a refresh, the
-// metadata via `cli-output.ts`. `files` without a collection ID lists every
+// library holds collections in the order the server first sent them. The
-// file in the account once (issue #211): the library keeps a record of a file
+// field values still come from each record's raw metadata via
-// for each collection that holds it, and only the first collection's record is
+// `cli-output.ts`. `files` without a collection ID lists every file in the
-// listed.
+// account once (issue #211).
+// The library keeps a record of a file for each collection that holds it, and
+// only the first collection's record is listed.
 
 import type { Collection, EnteFile } from "./model/types.js";
 import type { Photo, PhotosAPI } from "./library/index.js";
@@ -33,7 +37,8 @@ import type { Photo, PhotosAPI } from "./library/index.js";
 export interface FreshReadLibrary {
     fresh(): Promise<unknown>;
     freshCollections(): Promise<Collection[]>;
-    refreshCollection(collection: Collection): Promise<void>;
+    refreshCollection(collection: Collection): Promise<boolean>;
+    getCollection(id: number): Collection | undefined;
     listCollections(): Collection[];
     listFiles(collectionID: number): EnteFile[];
     getFileByID(fileID: number): EnteFile | undefined;
@@ -46,12 +51,14 @@ export const freshCollections = (
     lib: FreshReadLibrary,
 ): Promise<Collection[]> => lib.freshCollections();
 
-// The files of one collection, in enumeration order, current as of one
+// The files of one collection, in enumeration order. `undefined` (not an empty
+// list) when the collection does not exist, so the caller can tell "no such
+// collection" from "an empty collection". They are current as of one
 // collections request and the requests that bring that collection alone up to
-// date. `undefined` (not an empty list) when the collection does not exist, so
+// date. When `refreshCollection()` resolves to `false`, they are current as of
-// the caller can tell "no such collection" from "an empty collection". Without
+// a forced refresh instead. Without a collection ID, every file in the account,
-// a collection ID, every file in the account, current as of a forced refresh.
+// current as of a forced refresh. Each file is listed once, as held by the
-// Each file is listed once, as held by the first collection that holds it.
+// first collection that holds it.
 export const freshFiles = async (
     lib: FreshReadLibrary,
     collectionID?: number,
@@ -60,7 +67,10 @@ export const freshFiles = async (
         const collections = await lib.freshCollections();
         const collection = collections.find((c) => c.id === collectionID);
         if (!collection) return undefined;
-        await lib.refreshCollection(collection);
+        if (!(await lib.refreshCollection(collection))) {
+            await lib.fresh();
+            if (!lib.getCollection(collectionID)) return undefined;
+        }
         return lib.listFiles(collectionID);
     }
     await lib.fresh();

src/library/index.ts

@@ -25,9 +25,13 @@
 // `quak collections` uses it. `refreshCollection()` brings one collection
 // from that list up to date in the store (issue #217). It fetches that
 // collection's files alone, as a refresh would. `quak files COLLECTION` uses
-// it. It never runs at the same time as a refresh. A caller that asks for each
+// it. It never runs at the same time as a refresh. Once the stored cursor is
-// refresh it needs can also open the library with `refreshOnOpen: false`, so
+// past 0, it only updates a collection the store holds, and only a refresh
-// that `open()` starts none.
+// adds one. A refresh from a stored cursor of 0 replaces all the store holds.
+// So after a refresh, the store holds collections in the order the server
+// first sent them. A caller that asks for
+// each refresh it needs can also open the library with `refreshOnOpen: false`,
+// so that `open()` starts none.
 //
 // A refresh reports through `onProgress` each collection whose files it has
 // fetched, so a large account, whose first refresh fetches the files of every
@@ -638,10 +642,10 @@ export class Library {
     // The fresh read of the collection list alone (issue #210): every live
     // collection that decrypts, in the server's order, from one collections
     // request. Unlike a refresh, it fetches no collection's files, which on a
-    // large account take long. It stores nothing. A refresh fetches a
+    // large account take long. It stores nothing. A refresh from a stored
-    // collection's files only when the collection is new or has changed since
+    // cursor past 0 fetches a collection's files only when the collection is
-    // the stored copy. A collection stored here without its files would never
+    // new or has changed since the stored copy. So does `refreshCollection()`.
-    // have them fetched.
+    // Neither would fetch the files of a collection stored here without them.
     async freshCollections(): Promise<Collection[]> {
         if (this.closed) throw new Error("the library is closed");
         const { collections } = await this.client.collectionsSince({
@@ -653,16 +657,25 @@ export class Library {
     // Bring one collection up to date in the store (issue #217), given its
     // record from `freshCollections()`. It does for that collection what a
     // refresh does for a changed one. It fetches no other collection's files.
+    // It resolves to whether the store then holds the collection.
+    //
+    // It adds a collection the store does not hold only while the stored
+    // cursor is 0, before any refresh has received a collection. After that,
+    // it stores nothing for such a collection and resolves to false. Only a
+    // refresh may then add it, after the collections the store holds.
+    //
     // It runs in the same slot as a refresh, so the two never run at the same
     // time. It first waits for a running refresh or `refreshCollection()` to
     // end. While its own work runs, a fresh read or `backup()` waits for it,
     // and the background loop skips its turn.
-    async refreshCollection(collection: Collection): Promise<void> {
+    async refreshCollection(collection: Collection): Promise<boolean> {
         while (this.cycle) await this.cycle.catch(() => {});
         if (this.closed) throw new Error("the library is closed");
-        await this.startCycle(false, () =>
+        let held = false;
-            this.refreshCollectionOnce(collection),
+        await this.startCycle(false, async () => {
-        );
+            held = await this.refreshCollectionOnce(collection);
+        });
+        return held;
     }
 
     // Back up every in-scope file to `opts.downloadDirectory`, or else the
@@ -819,7 +832,13 @@ export class Library {
     // Fetch every change since the stored cursor, then commit. All network
     // reads happen before any store mutation, so a fetch that throws leaves the
     // store untouched and the previous snapshot intact.
+    //
+    // From a stored cursor of 0, the server lists every live collection. The
+    // refresh then replaces all the store holds, and counts every collection
+    // as new. Only `refreshCollection()` can have stored anything before it.
+    // So the store ends as it would had `refreshCollection()` never run.
     private async refreshOnce(): Promise<void> {
+        const replace = this.store.collectionsSinceTime === 0;
         const page = await this.client.collectionsSince({
             sinceTime: this.store.collectionsSinceTime,
         });
@@ -827,16 +846,19 @@ export class Library {
         // Stage per-collection file diffs. A collection's files are
         // re-enumerated only when its updationTime has advanced past the cached
         // copy; an unchanged album's file list cannot have changed. New
-        // collections enumerate from the beginning of time. Each collection
+        // collections enumerate from the beginning of time. When the refresh
+        // replaces the store, every collection counts as new. Each collection
         // whose files are fetched is reported, with how many there are to
         // fetch.
+        const storedCopy = (id: number): Collection | undefined =>
+            replace ? undefined : this.store.getCollection(id);
         const changedCollections = page.collections.filter((collection) => {
-            const known = this.store.getCollection(collection.id);
+            const known = storedCopy(collection.id);
             return !known || collection.updationTime > known.updationTime;
         });
         const filePages: { collectionID: number; page: FilesPage }[] = [];
         for (const collection of changedCollections) {
-            const known = this.store.getCollection(collection.id);
+            const known = storedCopy(collection.id);
             const filePage = await this.client.filesSince({
                 collectionID: collection.id,
                 collectionKey: collection.key,
@@ -851,26 +873,23 @@ export class Library {
             });
         }
 
-        await this.commit(page, filePages);
+        await this.commit(page, filePages, replace);
     }
 
     // `refreshCollection()`'s work, run in the slot: fetch the files of one
-    // collection changed since the stored copy, then commit.
+    // collection changed since the stored copy, then commit. It resolves to
-    private async refreshCollectionOnce(collection: Collection): Promise<void> {
+    // whether the store then holds the collection.
+    private async refreshCollectionOnce(
+        collection: Collection,
+    ): Promise<boolean> {
         const known = this.store.getCollection(collection.id);
+        // A refresh has received the account's collections, and this one is
+        // not stored. It is new since then, or a refresh dropped it. Leave it
+        // to the next refresh, which adds new collections after the stored
+        // ones, in the server's order.
+        if (!known && this.store.collectionsSinceTime > 0) return false;
         // The stored copy is as new or newer, so there is nothing to fetch.
-        if (known && collection.updationTime <= known.updationTime) return;
+        if (known && collection.updationTime <= known.updationTime) return true;
-        // No copy is stored, yet the stored cursor has reached this
-        // `updationTime`. A refresh has then seen this version or a later one,
-        // and dropped the collection: it was deleted, or did not decrypt.
-        // Storing it would bring it back, and no later refresh would see it
-        // again until the server's copy changes.
-        if (
-            !known &&
-            collection.updationTime <= this.store.collectionsSinceTime
-        ) {
-            return;
-        }
         const filePage = await this.client.filesSince({
             collectionID: collection.id,
             collectionKey: collection.key,
@@ -885,17 +904,22 @@ export class Library {
                 cursor: this.store.collectionsSinceTime,
             },
             [{ collectionID: collection.id, page: filePage }],
+            false,
         );
+        return true;
     }
 
     // Commit a collections page and the file pages fetched for its collections
-    // to the store. A collection or file that does not decrypt is left out, as
+    // to the store. With `replace`, the page lists every live collection, so
-    // if deleted. The reason is kept in the store. No later refresh sees it
+    // the commit first deletes every stored collection. A collection or file
-    // again until the server's copy changes. Last, save the store when it
+    // that does not decrypt is left out, as if deleted. The reason is kept in
-    // holds changes not yet saved.
+    // the store. Until the server's copy changes, only a refresh from a stored
+    // cursor of 0 fetches it again. Last, save the store when it holds changes
+    // not yet saved.
     private async commit(
         page: CollectionsPage,
         filePages: { collectionID: number; page: FilesPage }[],
+        replace: boolean,
     ): Promise<void> {
         // The network work is done, so change the store now. `changed` says
         // whether this commit changed anything.
@@ -906,6 +930,13 @@ export class Library {
             changed = true;
         }
 
+        if (replace) {
+            for (const { id } of this.store.listCollections()) {
+                this.store.deleteCollection(id);
+                changed = true;
+            }
+        }
+
         for (const id of page.deleted) {
             if (this.store.getCollection(id)) {
                 this.store.deleteCollection(id);

src/library/precache.ts

@@ -122,7 +122,7 @@ export class Precache {
     }
 
     // Recompute the fill orders and the pinned set from the current projection.
-    // Called at open and after every refresh that changes the library.
+    // Called at open and after every change to the store.
     update(records: DerivedRecords): void {
         const photos = [...records.photos.values()].sort(
             (a, b) => b.takenAt - a.takenAt || b.fileID - a.fileID,

src/library/records.ts

@@ -71,13 +71,14 @@ export interface LibrarySnapshot {
     takenAt: number;
 }
 
+// What one refresh or `refreshCollection()` changed in the records.
 export interface LibraryChange {
-    // Full records for albums/photos added or changed by the refresh.
+    // Full records for albums/photos added or changed.
     albumsChanged: AlbumRecord[];
     photosChanged: PhotoRecord[];
     fileIDsRemoved: number[];
     albumIDsRemoved: number[];
-    // Wall-clock milliseconds of the refresh that produced this change.
+    // Wall-clock milliseconds when the store changed.
     refreshedAt: number;
 }
 

src/library/store.ts

@@ -3,7 +3,7 @@
 // The store keeps one `metadata.json` file holding the account's server
 // state: the user id, a schema version, the cursor for the incremental
 // collections listing, the decrypted collection and file records, and the
-// collections and files the refresh could not decrypt. The whole file is read
+// collections and files that did not decrypt. The whole file is read
 // into RAM on load and rewritten as a whole on save; there is no partial
 // update and no lock file. A separate refresh unit populates the store from
 // the server — this module only stores what it is given.
@@ -34,9 +34,11 @@ const FILE_MODE = 0o600;
 type StoredCollection = Omit<Collection, "key"> & { key: string };
 type StoredFile = Omit<EnteFile, "key"> & { key: string };
 
-// A collection, or with `fileID` a file in it, that the last refresh to fetch
+// A collection, or with `fileID` a file in it, that did not decrypt or parse,
-// it could not decrypt or parse, and why. It is kept until a later refresh
+// and why. A refresh or `refreshCollection()` records it on fetching such a
-// fetches it again, because a refresh fetches only what changed.
+// copy. A later fetch drops it on getting a copy that decrypts, or on learning
+// that the server deleted it. Each fetch asks only for what changed, so the
+// store keeps the record between fetches.
 export interface Undecryptable {
     collectionID: number;
     fileID?: number;
@@ -222,8 +224,14 @@ export class MetadataStore {
         this.files.delete(fileKey(collectionID, fileID));
     }
 
+    // Sorted by collection ID, then file ID, so the order does not depend on
+    // when each record was fetched.
     listUndecryptable(): Undecryptable[] {
-        return [...this.undecryptable];
+        return [...this.undecryptable].sort(
+            (a, b) =>
+                a.collectionID - b.collectionID ||
+                (a.fileID ?? 0) - (b.fileID ?? 0),
+        );
     }
 
     // Replaces any earlier record of the same collection or file.

test/cli/commands.test.ts

@@ -124,20 +124,35 @@ const FILES: Record<number, EnteFile[]> = {
 // thumbnail as empty. `withNewFile` adds new.jpg (102) to Vacation, advancing
 // the collection's updationTime as the server does. `beachInWork` adds
 // beach.jpg (100) to Work as well, so that one file is in both albums.
-// `refreshError` makes listing collections fail with that message.
+// `newAlbums` adds the albums A (3) and B (4), in that order, which both hold
+// shared.jpg (300). `refreshError` makes listing collections fail with that
+// message.
 const fakeClient = (
     opts: {
         failID?: number;
         emptyThumbID?: number;
         withNewFile?: boolean;
         beachInWork?: boolean;
+        newAlbums?: boolean;
         refreshError?: string;
     } = {},
 ) => {
-    const collections = opts.withNewFile
+    let collections = opts.withNewFile
         ? [{ ...COLLECTIONS[0], updationTime: 2 }, COLLECTIONS[1]]
         : COLLECTIONS;
     let files = FILES;
+    if (opts.newAlbums) {
+        collections = [
+            ...collections,
+            { ...collection(3, "A"), updationTime: 2 },
+            { ...collection(4, "B"), updationTime: 2 },
+        ];
+        files = {
+            ...files,
+            3: [file(300, 3, "shared.jpg")],
+            4: [file(300, 4, "shared.jpg")],
+        };
+    }
     if (opts.withNewFile) {
         files = {
             ...files,
@@ -569,6 +584,57 @@ describe("files", () => {
         ]);
         expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS);
     });
+
+    // `files 2` on an empty cache stores Work alone. The refresh that `files`
+    // then waits for must still store Vacation first, as the server lists it.
+    it("without a collection ID lists in the server's order after files 2 on an empty cache", async () => {
+        const ctx = context(fakeClient({ beachInWork: true }));
+        expect(await filesCommand(ctx, "2", {})).toBe(0);
+        stdout.text = "";
+        stderr.text = "";
+
+        expect(await filesCommand(ctx, undefined, { json: true })).toBe(0);
+        expect(
+            JSON.parse(stdout.text).map(
+                (row: { id: number; collectionID: number }) => [
+                    row.id,
+                    row.collectionID,
+                ],
+            ),
+        ).toEqual([
+            [100, 1],
+            [101, 1],
+            [200, 2],
+        ]);
+        expect(stderr.text).toBe(FETCHED_BOTH_ALBUMS);
+    });
+
+    // After a refresh, the server adds A and then B. `files 4` must not store B
+    // ahead of A, so it waits for a refresh, which stores both in that order.
+    it("without a collection ID lists new albums in the server's order after files with the second one's ID", async () => {
+        expect(await filesCommand(context(), undefined, {})).toBe(0);
+        const ctx = context(fakeClient({ newAlbums: true }));
+        stdout.text = "";
+
+        expect(await filesCommand(ctx, "4", {})).toBe(0);
+        expect(stdout.text).toBe("300\timage\tshared.jpg\n");
+        stdout.text = "";
+
+        expect(await filesCommand(ctx, undefined, { json: true })).toBe(0);
+        expect(
+            JSON.parse(stdout.text).map(
+                (row: { id: number; collectionID: number }) => [
+                    row.id,
+                    row.collectionID,
+                ],
+            ),
+        ).toEqual([
+            [100, 1],
+            [101, 1],
+            [200, 2],
+            [300, 3],
+        ]);
+    });
 });
 
 describe("get and get-thumb", () => {
@@ -1387,12 +1453,12 @@ describe("backup-metadata --exif", () => {
     });
 });
 
-// Each test first runs `files 1` so the cache holds Vacation as it was, then
+// Each test first runs `files` so the cache holds the account as it was, then
-// adds a file to Vacation on the server. (`collections` would not do: it reads
+// changes the server under it. (`collections` would not do: it reads the
-// the collection list alone and stores nothing.)
+// collection list alone and stores nothing.)
 describe("backup-metadata and the thumbnail helpers refresh first", () => {
     beforeEach(async () => {
-        expect(await filesCommand(context(), "1", {})).toBe(0);
+        expect(await filesCommand(context(), undefined, {})).toBe(0);
         stdout.text = "";
         stderr.text = "";
     });
@@ -1701,8 +1767,8 @@ describe("an account with many albums", () => {
         expect(stderr.text).toBe("");
         expect(paths).toEqual(["/collections/v2?sinceTime=0"]);
 
-        // It stored nothing that would make a refresh skip an album's files:
+        // It stored nothing that would make `refreshCollection()` skip an
-        // `files` after it still finds the album's photo.
+        // album's files: `files` after it still finds the album's photo.
         stdout.text = "";
         expect(await filesCommand(ctx, "7", {})).toBe(0);
         expect(stdout.text).toBe("700\timage\tphoto-7.jpg\n");

test/cli/log.test.ts

@@ -234,8 +234,8 @@ describe("-q", () => {
         });
     });
 
-    // The refresh's progress is left out; the line saying why the command
+    // The line saying why the command failed is the fatal one, and it is
-    // failed is the fatal one, and it is printed.
+    // printed.
     it("prints only the line a failing command exits with", async () => {
         expect(await quak("-q", "files", "9")).toEqual({
             code: 1,

test/cli/read.test.ts

@@ -11,8 +11,10 @@
  * (`freshCollections()`, issue #210). It must not wait for a refresh, which
  * would fetch the files of every collection. `files COLLECTION` reads that list
  * too, then brings that one collection up to date (`refreshCollection()`,
- * issue #217). The stand-in serves a collection's files once `fresh()` or
+ * issue #217). The stand-in serves a collection's files once `fresh()` has run,
- * `refreshCollection()` for that collection has run.
+ * or once `refreshCollection()` has resolved to `true` for that collection.
+ * When `refreshCollection()` resolves to `false`, `files COLLECTION` forces a
+ * refresh instead.
  *
  * `collections` and `files` also list in the library's enumeration order
  * (`listCollections`/`listFiles`) — the order the pre-library CLI printed — not
@@ -67,14 +69,18 @@ const file = (
 
 // A library that reveals its records only after `fresh()` has been awaited, and
 // serves them in the enumeration order it was given. `freshCollections()`
-// answers the collection list in that order without a refresh.
+// answers the collection list in that order without a refresh. When
-// `refreshCollection()` reveals the files of that one collection.
+// `refreshCollection()` resolves to true, it reveals the files of that one
+// collection.
 // `photos.byID` returns a stand-in `Photo` carrying just the fileID the helper
 // passes through.
 class FakeLibrary implements FreshReadLibrary {
     freshCalls = 0;
     freshCollectionsCalls = 0;
     refreshCollectionIDs: number[] = [];
+    // What `refreshCollection()` resolves to. With false it reveals nothing,
+    // as a library that has had a refresh does for a collection it lacks.
+    refreshCollectionHolds = true;
     private refreshed = false;
 
     constructor(
@@ -93,8 +99,14 @@ class FakeLibrary implements FreshReadLibrary {
         return this.collections;
     }
 
-    async refreshCollection(collection: Collection): Promise<void> {
+    async refreshCollection(collection: Collection): Promise<boolean> {
         this.refreshCollectionIDs.push(collection.id);
+        return this.refreshCollectionHolds;
+    }
+
+    getCollection(id: number): Collection | undefined {
+        if (!this.refreshed) return undefined;
+        return this.collections.find((c) => c.id === id);
     }
 
     listCollections(): Collection[] {
@@ -103,7 +115,9 @@ class FakeLibrary implements FreshReadLibrary {
 
     listFiles(collectionID: number): EnteFile[] {
         const current =
-            this.refreshed || this.refreshCollectionIDs.includes(collectionID);
+            this.refreshed ||
+            (this.refreshCollectionHolds &&
+                this.refreshCollectionIDs.includes(collectionID));
         return current
             ? this.files.filter((f) => f.collectionID === collectionID)
             : [];
@@ -216,6 +230,19 @@ describe("CLI read helpers (issue #36 amendment, issue #52)", () => {
         ]);
     });
 
+    it("freshFiles forces a refresh when refreshCollection resolves to false", async () => {
+        const lib = new FakeLibrary(
+            [collection(1, 100)],
+            [file(10, 1, 1_700_000_000_000_000)],
+        );
+        lib.refreshCollectionHolds = false;
+
+        const rows = await freshFiles(lib, 1);
+        expect(lib.refreshCollectionIDs).toEqual([1]);
+        expect(lib.freshCalls).toBe(1);
+        expect(rows?.map((f) => f.id)).toEqual([10]);
+    });
+
     it("freshFiles returns undefined for an unknown collection", async () => {
         const lib = new FakeLibrary([collection(1, 100)], []);
         const rows = await freshFiles(lib, 999);

test/library/fresh.test.ts

@@ -20,9 +20,11 @@
  *  5. `refreshCollection()` brings one collection up to date in the store
  *     (issue #217). It leaves the stored cursor where it was, so the next
  *     refresh still fetches the files of every other collection that changed.
- *     It never runs at the same time as a refresh, and never brings back a
+ *     It never runs at the same time as a refresh. After a refresh, it stores
- *     collection that a refresh deleted. `close()` waits for it. A library
+ *     no collection the store does not hold: not a new one, and not one a
- *     opened on a cache that only it wrote awaits its first refresh.
+ *     refresh deleted. `close()` waits for it. A library opened on a cache that
+ *     only it wrote awaits its first refresh. That refresh replaces what it
+ *     stored, in the server's order.
  *
  * The client is the same metadata-only mock the background-refresh tests use:
  * no crypto, no network, scripted pages, and a record of each call. A long
@@ -314,10 +316,10 @@ describe("Library.freshCollections", () => {
         rmSync(dir, { recursive: true, force: true });
     });
 
-    // A refresh fetches a collection's files only when the collection is new
+    // A refresh from a stored cursor past 0 fetches a collection's files only
-    // or its updationTime has advanced past the stored copy's. Had the
+    // when the collection is new or its updationTime has advanced past the
-    // collection list been stored, the next refresh would take every
+    // stored copy's. So does `refreshCollection()`. Neither would fetch the
-    // collection for one it already holds the files of, and fetch none.
+    // files of a collection stored here without them.
     it("asks for the collection list alone and stores nothing", async () => {
         const client = new MockClient();
         const page = {
@@ -419,7 +421,7 @@ describe("Library.refreshCollection", () => {
             });
 
             // Collection 1's files alone, since the stored copy.
-            await lib.refreshCollection(collection(1, 110));
+            expect(await lib.refreshCollection(collection(1, 110))).toBe(true);
             expect(client.filesCalls).toEqual([
                 { collectionID: 1, sinceTime: 100 },
             ]);
@@ -501,7 +503,7 @@ describe("Library.refreshCollection", () => {
             await lib.fresh();
             client.filesCalls.length = 0;
 
-            await lib.refreshCollection(listed);
+            expect(await lib.refreshCollection(listed)).toBe(false);
             expect(client.filesCalls).toEqual([]);
             expect(lib.getCollection(1)).toBeUndefined();
         } finally {
@@ -509,6 +511,21 @@ describe("Library.refreshCollection", () => {
         }
     });
 
+    // A refresh has stored collection 1. Collection 2 is new on the server
+    // since then. Only the next refresh may add it, after collection 1.
+    it("stores nothing for a collection that is new since the last refresh", async () => {
+        const { client, lib } = await openSeeded(cacheDirectory);
+        try {
+            client.filesCalls.length = 0;
+
+            expect(await lib.refreshCollection(collection(2, 110))).toBe(false);
+            expect(client.filesCalls).toEqual([]);
+            expect(lib.getCollection(2)).toBeUndefined();
+        } finally {
+            await lib.close();
+        }
+    });
+
     // The caller may remove the cache directory once `close()` resolves, so
     // the cache write must have landed by then.
     it("close() waits for a refreshCollection() in flight", async () => {
@@ -538,12 +555,13 @@ describe("Library.refreshCollection", () => {
         await refreshing;
     });
 
-    // `quak files 1` on an empty cache stores collection 1 alone. A library
+    // `quak files 2` on an empty cache stores collection 2 alone. A library
-    // opened later on that cache must not serve it as the whole account.
+    // opened later on that cache must not serve it as the whole account. Its
+    // first refresh replaces what the cache holds, in the server's order.
     it("open() awaits the first refresh on a cache that only refreshCollection() wrote", async () => {
         const client = new MockClient();
-        client.filesFor(1, {
+        client.filesFor(2, {
-            files: [file(1001, 1, 90)],
+            files: [file(2001, 2, 90)],
             deleted: [],
             cursor: 90,
         });
@@ -553,7 +571,7 @@ describe("Library.refreshCollection", () => {
             refreshOnOpen: false,
             refreshIntervalSeconds: SLOW_INTERVAL,
         });
-        await first.refreshCollection(collection(1, 100));
+        expect(await first.refreshCollection(collection(2, 100))).toBe(true);
         await first.close();
 
         client.collectionsQueue.push({
@@ -561,11 +579,17 @@ describe("Library.refreshCollection", () => {
             deleted: [],
             cursor: 100,
         });
+        client.filesFor(1, {
+            files: [file(1001, 1, 90)],
+            deleted: [],
+            cursor: 90,
+        });
         client.filesFor(2, {
             files: [file(2001, 2, 90)],
             deleted: [],
             cursor: 90,
         });
+        client.filesCalls.length = 0;
         const lib = await Library.open({
             client,
             cacheDirectory,
@@ -574,6 +598,11 @@ describe("Library.refreshCollection", () => {
         try {
             expect(lib.status().lastRefreshAt).toBeDefined();
             expect(lib.listCollections().map((c) => c.id)).toEqual([1, 2]);
+            // It fetches collection 2's files again, from the beginning.
+            expect(client.filesCalls).toEqual([
+                { collectionID: 1, sinceTime: 0 },
+                { collectionID: 2, sinceTime: 0 },
+            ]);
             expect(lib.listFiles(2).map((f) => f.id)).toEqual([2001]);
         } finally {
             await lib.close();

test/library/library.test.ts

@@ -20,9 +20,9 @@
  * 5. `close()` stops the timer and is idempotent, and its promise resolves
  *    only once an in-flight refresh has written the cache file.
  * 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
+ * 7. `open()` branches on the stored cursor. At 0, as on an empty cache, it
- *    (it has nothing to serve yet); an existing cache serves its copy at once
+ *    awaits the first refresh. Past 0 it serves the cache's copy at once and
- *    and refreshes in the background, so a slow or dead server never stalls
+ *    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.

test/library/snapshot.test.ts

@@ -4,7 +4,8 @@
  * These are the surface the GUI consumes across Electron IPC. `snapshot()` is
  * synchronous — it reads the in-RAM store and projects it into plain records
  * (no keys) — and `subscribe({ onChange })` delivers a `LibraryChange` whenever
- * a background refresh actually changes the derived records. The contracts:
+ * a refresh or `refreshCollection()` changes the derived records. The
+ * contracts:
  *
  *  1. `snapshot()` deduplicates a file across memberships into one record with
  *     every album id, orders photos newest first, and carries no key material.

test/library/store.test.ts

@@ -172,7 +172,7 @@ describe("MetadataStore", () => {
         expect(store.listCollections()).toEqual([]);
     });
 
-    it("loads an empty store when the list of what the refresh could not decrypt is damaged", async () => {
+    it("loads an empty store when the list of what did not decrypt is damaged", async () => {
         // A backup reads that list on every run, so a damaged one that loaded
         // would fail every backup until the cache was deleted. A string is
         // damaged too, though JavaScript can loop over its characters.
@@ -266,4 +266,23 @@ describe("MetadataStore", () => {
         expect(store.getFile(12345, 67890)).toBeUndefined();
         expect(store.getFile(55555, 67890)?.key).toEqual(inB.key);
     });
+
+    // A backup reports these in this order. It must not depend on whether a
+    // refresh or `Library.refreshCollection()` fetched each one first.
+    it("lists what did not decrypt by collection ID, then file ID", async () => {
+        const store = await MetadataStore.load(path);
+        store.putUndecryptable({ collectionID: 2, fileID: 7, error: "a" });
+        store.putUndecryptable({ collectionID: 3, error: "b" });
+        store.putUndecryptable({ collectionID: 1, fileID: 9, error: "c" });
+        store.putUndecryptable({ collectionID: 2, fileID: 5, error: "d" });
+
+        expect(
+            store.listUndecryptable().map((r) => [r.collectionID, r.fileID]),
+        ).toEqual([
+            [1, 9],
+            [2, 5],
+            [2, 7],
+            [3, undefined],
+        ]);
+    });
 });