v2.4.0: a re-ordered download takes the new order on its next online start

CONVENTIONS.md

@@ -1773,7 +1773,21 @@ are asked, at most 10,000 rows.
   is a new download). An answer that reached the cache before the book was downloaded (while its
   files did, or before launch's `hydrate`) is taken as it becomes downloaded (`withCachedChapters`).
   The audio is never touched, and a queue already playing keeps its chapters until the book is
-  next started.
+  next started. The same files in another order (server v2.4.0 plays them in natural order, "2-"
+  before "10-") are taken **only by a start that loads the engine in that order**
+  (`adoptStartedChapters`, from `playBook` once its load landed and it still owns the engine;
+  saved at once; a start given no chapters plays the download's own files and chapters), never
+  from a background answer, and a start's answer in the manifest's own order is left to the
+  background refresh: JS sees only what it loaded, not a book the Android service or a Cast
+  receiver holds before `adoptLoaded` / the receiver adoption, which map the engine's track index
+  through the manifest. So an offline start, Android's no-JS car start
+  and `adoptLoaded` keep the manifest's order until an online start plays the new one. The
+  chapters carry the order (every queue over a download orders its files by them and finds each by
+  path); `manifest.files` itself never moves, since a file's index there is its name on disk
+  (`fileName`, rebuilt by `relocateEntry` at every launch and by a retry). A place saved on the old
+  timeline reads as moved once the new order is taken (as the server's re-index moves any
+  whole-book place: no remap). A downloaded book whose chapters change gets its Android car play
+  spec rebuilt (`car-controller.ts`).
 - **Offline companion** (`src/downloads/offline-meta.ts`): a downloaded book keeps its community
   metadata (asked by `metaEnabledFor`, so a book with content generated on its server too; the `/meta` envelope, plus the previous work(s) the envelope doesn't hold) as
   `meta.json` beside its audio (`engine.writeText`: the book's folder on native, its Cache API

src/car/car-controller.test.ts

@@ -515,6 +515,27 @@ describe('startCarSync', () => {
     expect(e.play).toMatchObject({ startIndex: 1, positionInTrack: 50, rate: 1.25 });
   });
 
+  it("rebuilds a downloaded book's spec when its chapters change (its files in a new order)", async () => {
+    Object.defineProperty(Platform, 'OS', { value: 'android', configurable: true });
+    connect(['c1']);
+    mockClients.c1 = fakeClient({});
+    useDownloads.setState({ entries: { 'c1:2:D': downloaded('c1', 'D') } });
+    stop = startCarSync();
+    await settle();
+    const played = () => tab(lastSnapshot(), 'downloads')!.items[0].play!.tracks.map((t) => t.url);
+    expect(played()).toEqual(['file:///d/1.mp3', 'file:///d/2.mp3']);
+
+    // The server now plays them the other way round (`refreshedChapters`): same status.
+    const reordered = downloaded('c1', 'D');
+    reordered.manifest.chapters = {
+      ...twoFileChapters,
+      files: twoFileChapters.files.map((f) => ({ ...f, seq: 1 - f.seq })),
+    };
+    useDownloads.setState({ entries: { 'c1:2:D': reordered } });
+    await settle(SETTLE_MS + MIN_GAP_MS);
+    expect(played()).toEqual(['file:///d/2.mp3', 'file:///d/1.mp3']);
+  });
+
   it('leaves Up next out where the default server keeps no queue', async () => {
     connect(['c1']);
     mockClients.c1 = fakeClient({});

src/car/car-controller.ts

@@ -40,7 +40,7 @@ import { startBookInPlace } from '@/components/player/start-book';
 import { queueConnectionId } from '@/components/upnext/up-next-model';
 import { statusSignature } from '@/downloads/downloads-view';
 import { downloadedEntryOf, useDownloads } from '@/downloads/store';
-import type { DownloadManifest } from '@/downloads/types';
+import type { DownloadEntry, DownloadManifest } from '@/downloads/types';
 import i18n from '@/i18n';
 import { bootstrapPlayback } from '@/lib/bootstrap';
 import { resolveConnectionId } from '@/lib/connection-id';
@@ -979,6 +979,24 @@ export function carSyncReady(): Promise<void> {
   return ready;
 }
 
+/** Whether a book downloaded in both registries has other chapters in `next` (a chapters
+ * refresh, `refreshedChapters`, or a start's, `adoptStartedChapters`, which can also give
+ * its files a new play order): its play spec is built from them (`playSpec`), and a
+ * refresh changes no download's status. */
+function chaptersChanged(
+  next: Record<string, DownloadEntry>,
+  prev: Record<string, DownloadEntry>,
+): boolean {
+  return Object.entries(next).some(([k, e]) => {
+    const was = prev[k];
+    return (
+      e.status === 'downloaded' &&
+      was?.status === 'downloaded' &&
+      e.manifest.chapters !== was.manifest.chapters
+    );
+  });
+}
+
 /**
  * Start the car sync (idempotent: callers share one; each returned stop releases one, and the
  * last releases everything). Native only; a no-op on the web and on a binary without the car
@@ -1016,7 +1034,7 @@ export function startCarSync(): () => void {
     useDownloads.subscribe((s, prev) => {
       if (s.entries === prev.entries) return;
       const sig = statusSignature(s.entries);
-      if (sig === downloadsSig) return;
+      if (sig === downloadsSig && !chaptersChanged(s.entries, prev.entries)) return;
       downloadsSig = sig;
       request(false);
     }),

src/downloads/store.test.ts

@@ -132,10 +132,12 @@ jest.mock('@/api/hooks', () => ({
 /* eslint-disable import/first */
 import { resolveClient } from '@/api/connection-clients';
 import { engine } from '@/downloads/engine';
+import { bookFileSpecs } from '@/playback/book-queue';
 import {
   downloadedCountFor,
   downloadedEntryOf,
   downloadKey,
+  adoptStartedChapters,
   isDeclined,
   refreshedChapters,
   reviveEntry,
@@ -544,6 +546,15 @@ describe("refreshing a downloaded book's chapters", () => {
     chapters_source: 'community',
     ...over,
   });
+  /** The order a downloaded book's queue plays its files in: its chapters'. */
+  const playOrder = (manifest: DownloadManifest) =>
+    bookFileSpecs(manifest.book, manifest.chapters ?? undefined).map((s) => s.path);
+  /** The query cache holds `data` as A/Book's chapters answer. */
+  const cacheHolds = (data: ChaptersResponse) =>
+    mockGetQueryData.mockImplementation((k: unknown) =>
+      Array.isArray(k) && k[0] === 'chapters' && k[3] === 'A/Book' ? data : undefined,
+    );
+  afterEach(() => mockGetQueryData.mockReset());
 
   describe('refreshedChapters', () => {
     it('takes the fresh chapters when the audio files are the same', () => {
@@ -558,8 +569,46 @@ describe("refreshing a downloaded book's chapters", () => {
     it('leaves the manifest alone when the files differ (a re-download)', () => {
       const e = downloadedEntry();
       expect(refreshedChapters(e.manifest, fresh(['A/Book/01.mp3']))).toBeNull();
-      expect(refreshedChapters(e.manifest, fresh(['A/Book/02.mp3', 'A/Book/01.mp3']))).toBeNull();
       expect(refreshedChapters(e.manifest, fresh(['A/Book/01.mp3', 'A/Book/03.mp3']))).toBeNull();
+      expect(refreshedChapters(e.manifest, fresh(['A/Book/01.mp3', 'A/Book/01.mp3']))).toBeNull();
+      expect(
+        refreshedChapters(e.manifest, fresh(['A/Book/01.mp3', 'A/Book/02.mp3', 'A/Book/03.mp3'])),
+      ).toBeNull();
+    });
+
+    it('never takes the same files in another order (only a start that plays it does)', () => {
+      const e = downloadedEntry();
+      e.manifest.chapters = fresh(undefined, { chapters_source: undefined });
+      expect(refreshedChapters(e.manifest, fresh(['A/Book/02.mp3', 'A/Book/01.mp3']))).toBeNull();
+    });
+
+    it('keeps each file on its own audio across a launch once its book took a new order', async () => {
+      // Two extensions: a file named for another one's index would be missing on disk, and
+      // the launch would drop the book and delete its folder.
+      const paths = ['A/Book/01.mp3', 'A/Book/02.m4a'];
+      const e = downloadedEntry();
+      e.manifest.chapters = fresh(paths, { chapters_source: undefined });
+      e.manifest.files = [
+        { relPath: paths[0], localUri: 'live:c1:2:A/Book:0.mp3' },
+        { relPath: paths[1], localUri: 'live:c1:2:A/Book:1.m4a' },
+      ];
+      const onDisk = new Set(e.manifest.files.map((f) => f.localUri));
+      mockEngine.fileExists.mockImplementation(async (uri: string) => onDisk.has(uri));
+      const key = downloadKey('c1', 2, 'A/Book');
+      useDownloads.setState({ entries: { [key]: e } });
+      adoptStartedChapters(
+        { connectionId: 'c1', libraryId: 2, path: 'A/Book' },
+        fresh([paths[1], paths[0]]),
+      );
+      await seed({ [key]: useDownloads.getState().entries[key] });
+
+      await useDownloads.getState().hydrate();
+
+      const after = useDownloads.getState().entries[key];
+      expect(after?.status).toBe('downloaded');
+      expect(mockEngine.removeBook).not.toHaveBeenCalled();
+      expect(after.manifest.files).toEqual(e.manifest.files);
+      expect(playOrder(after.manifest)).toEqual([paths[1], paths[0]]);
     });
 
     it('leaves the manifest alone when a file at the same path changed size (replaced in place)', () => {
@@ -657,6 +706,57 @@ describe("refreshing a downloaded book's chapters", () => {
       expect(useDownloads.getState().entries[key]).toBe(queued);
     });
 
+    describe('a new file order', () => {
+      const key = downloadKey('c1', 2, 'A/Book');
+      const ref = { connectionId: 'c1', libraryId: 2, path: 'A/Book' };
+      const reordered = () => fresh(['A/Book/02.mp3', 'A/Book/01.mp3']);
+      const order = () => playOrder(useDownloads.getState().entries[key].manifest);
+      beforeEach(() => {
+        // Downloaded from the server's old answer (its own chapters, in the old order).
+        const e = downloadedEntry();
+        e.manifest.chapters = fresh(undefined, { chapters_source: undefined });
+        useDownloads.setState({ entries: { [key]: e } });
+      });
+
+      it('never comes from a background answer: JS cannot see what a native engine holds', async () => {
+        const e = useDownloads.getState().entries[key];
+        answer(reordered());
+        expect(useDownloads.getState().entries[key]).toBe(e);
+        // Nor from one the cache held when launch read the registry.
+        await seed({ [key]: e });
+        cacheHolds(reordered());
+        await useDownloads.getState().hydrate();
+        expect(useDownloads.getState().entries[key].manifest.chapters).toEqual(e.manifest.chapters);
+      });
+
+      it('is taken by a start that played it, with its chapters, nothing downloaded again', async () => {
+        const e = useDownloads.getState().entries[key];
+        adoptStartedChapters(ref, reordered());
+        const now = useDownloads.getState().entries[key];
+        expect(now.manifest.chapters).toEqual(reordered());
+        expect(order()).toEqual(['A/Book/02.mp3', 'A/Book/01.mp3']);
+        // The local files stay as the download wrote them (a file's index is its name on disk).
+        expect(now.manifest.files).toBe(e.manifest.files);
+        // Saved at once: a JS restart right after the start must find the order it played.
+        expect((await readPersisted())[key]?.manifest.chapters).toEqual(reordered());
+      });
+
+      it('is not taken from a start whose chapters name other files, or another size', () => {
+        const e = useDownloads.getState().entries[key];
+        adoptStartedChapters(ref, fresh(['A/Book/02.mp3', 'A/Book/03.mp3']));
+        adoptStartedChapters(ref, fresh(['A/Book/02.mp3']));
+        const resized = reordered();
+        resized.files = resized.files.map((f) =>
+          f.rel_path === 'A/Book/01.mp3' ? { ...f, size: 2 } : f,
+        );
+        adoptStartedChapters(ref, resized);
+        adoptStartedChapters({ ...ref, path: 'B/Book' }, reordered());
+        // The same order is the background refresh's to take, not a start's (it can be older).
+        adoptStartedChapters(ref, fresh());
+        expect(useDownloads.getState().entries[key]).toBe(e);
+      });
+    });
+
     it('stops listening once unsubscribed', () => {
       const e = downloadedEntry();
       const key = downloadKey('c1', 2, 'A/Book');
@@ -671,11 +771,6 @@ describe("refreshing a downloaded book's chapters", () => {
   // before (while the files did, or before launch read the registry) is taken from the cache.
   describe('an answer that landed before the book was downloaded', () => {
     const key = downloadKey('c1', 2, 'A/Book');
-    const cacheHolds = (data: ChaptersResponse) =>
-      mockGetQueryData.mockImplementation((k: unknown) =>
-        Array.isArray(k) && k[0] === 'chapters' && k[3] === 'A/Book' ? data : undefined,
-      );
-    afterEach(() => mockGetQueryData.mockReset());
 
     it('is taken when the download finishes', async () => {
       mockResolveClient.mockReturnValue({