fix(sync): WebDAV upload-after-enable and deletion propagation (#4856, #4860) (#4892)

* fix(sync): upload book files when Upload Book Files is enabled after first sync (#4856) Incremental sync decided what to push purely from `isLocalNewer` (`book.updatedAt` vs the shared index). A book's config/cover change over time, but its FILE is immutable per hash and only needs uploading once. After a first sync with "Upload Book Files" off, toggling it on never bumped `book.updatedAt`, so the book was skipped and its file never reached the remote. Record which book FILES are already on the remote in library.json (`uploadedHashes`) and split the push decision: config/cover stay gated on the incremental "changed locally" cursor, while a file is (re)uploaded only when syncBooks is on and its hash isn't recorded yet. This keeps an incremental "Sync now" O(changed) — once a file is recorded, later syncs skip it with no per-book HEAD probe, so large libraries don't pay an O(library) cost on every sync. Full Sync bypasses the record as an escape hatch for out-of-band drift. The record is additive and optional, so an old client that rewrites the index just drops it and the next new-client sync re-verifies each file once and re-records it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(sync): propagate WebDAV book deletions to peers and the server (#4860) A book deleted on one device only tombstoned itself in library.json — the deletion never reached other devices or the server: - peers kept the book: the reconcile pass skipped deletedAt entries, so a tombstone never removed the local copy; - the server kept the files: the per-hash directory was never GC'd; - the tombstone could vanish entirely: a device that had never seen the book rebuilt the index purely from its own library, dropping the tombstone and silently reviving the book for everyone. Fixes all three in engine.syncLibrary: - apply a peer's tombstone locally (LocalStore.deleteBookLocally removes the app-managed copy and persists the tombstone), with edit-wins-over-delete LWW so a book still being read isn't yanked; - GC the remote per-hash directory of tombstoned books, scoped to the dirs the discovery scan saw so removed dirs are never re-DELETEd; - union remote-only entries (chiefly tombstones) into the re-pushed index so a deletion can't be dropped by a device that never had the book. Books tombstoned mid-run are excluded from the push pass via the merged state so a just-deleted book isn't re-uploaded right before it is GC'd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 23:28:42 +09:00
parent 81802a7c72
commit df34de1c38
9 changed files with 590 additions and 42 deletions
@@ -44,6 +44,7 @@ beforeEach(() => {
      savedLibrary = books;
    }),
    generateCoverImageUrl: vi.fn(async () => 'blob:cover'),
+    deleteBook: vi.fn(async () => {}),
  } as unknown as AppService;
  envConfig = { getAppService: async () => appService } as unknown as EnvConfigType;
 });
@@ -84,4 +85,21 @@ describe('createAppLocalStore — library hydration (data-loss guard)', () => {
    expect(appService.loadLibraryBooks).not.toHaveBeenCalled();
    expect(savedLibrary!.map((b) => b.hash).sort()).toEqual(['a', 'b', 'c']);
  });
+
+  test('deleteBookLocally removes the managed copy and persists the tombstone (#4860)', async () => {
+    useLibraryStore.getState().setLibrary([makeBook('a'), makeBook('b')]);
+
+    await makeStore().deleteBookLocally(makeBook('a', { deletedAt: 500 }));
+
+    // The managed local copy is removed via the 'local' delete action.
+    expect(appService.deleteBook).toHaveBeenCalledWith(
+      expect.objectContaining({ hash: 'a' }),
+      'local',
+    );
+    // The tombstone is persisted and the other book survives.
+    expect(savedLibrary!.map((b) => b.hash).sort()).toEqual(['a', 'b']);
+    expect(savedLibrary!.find((b) => b.hash === 'a')!.deletedAt).toBe(500);
+    // The deleted book drops off the visible shelf.
+    expect(useLibraryStore.getState().visibleLibrary.map((b) => b.hash)).toEqual(['b']);
+  });
 });
@@ -0,0 +1,225 @@
+import { describe, expect, test, vi } from 'vitest';
+
+import type { Book } from '@/types/book';
+import { FileSyncEngine } from '@/services/sync/file/engine';
+import type { FileSyncProvider } from '@/services/sync/file/provider';
+import type { LocalStore } from '@/services/sync/file/localStore';
+import type { RemoteLibraryIndex } from '@/services/sync/file/wire';
+
+/**
+ * #4860: WebDAV deletions must propagate. Three behaviours are exercised here:
+ *   1. a peer's tombstone deletes the book locally (edit-wins-over-delete guard),
+ *   2. the deleted book's remote hash directory is GC'd off the server,
+ *   3. a tombstone for a book this device never had survives the index re-push
+ *      (otherwise the deletion would silently vanish from library.json).
+ */
+
+const makeBook = (hash: string, overrides: Partial<Book> = {}): Book => ({
+  hash,
+  format: 'EPUB',
+  title: `Book ${hash}`,
+  sourceTitle: `Book ${hash}`,
+  author: 'A',
+  createdAt: 1,
+  updatedAt: 1,
+  ...overrides,
+});
+
+type Captured = { writes: { path: string; body: string }[]; deletedDirs: string[] };
+
+const fakeProvider = (
+  opts: Partial<FileSyncProvider> & { captured?: Captured } = {},
+): FileSyncProvider => ({
+  rootPath: '/',
+  readText: opts.readText ?? (async () => null),
+  readBinary: opts.readBinary ?? (async () => null),
+  head: opts.head ?? (async () => null),
+  list: opts.list ?? (async () => []),
+  writeText:
+    opts.writeText ??
+    (async (path: string, body: string) => {
+      opts.captured?.writes.push({ path, body });
+    }),
+  writeBinary: opts.writeBinary ?? (async () => {}),
+  ensureDir: opts.ensureDir ?? (async () => {}),
+  deleteDir:
+    opts.deleteDir ??
+    (async (path: string) => {
+      opts.captured?.deletedDirs.push(path);
+    }),
+  uploadStream: opts.uploadStream,
+  downloadStream: opts.downloadStream,
+});
+
+const fakeStore = (opts: Partial<LocalStore> = {}): LocalStore => ({
+  loadConfig: opts.loadConfig ?? (async () => null),
+  saveBookConfig: opts.saveBookConfig ?? (async () => {}),
+  loadBookFile: opts.loadBookFile ?? (async () => null),
+  resolveLocalBookPath: opts.resolveLocalBookPath ?? (async () => null),
+  saveBookFile: opts.saveBookFile ?? (async () => {}),
+  prepareLocalBookPath: opts.prepareLocalBookPath ?? (async () => '/local/dst'),
+  loadBookCover: opts.loadBookCover ?? (async () => null),
+  saveBookCover: opts.saveBookCover ?? (async () => {}),
+  addBookToLibrary: opts.addBookToLibrary ?? (async () => {}),
+  updateBookMetadata: opts.updateBookMetadata ?? (async () => {}),
+  deleteBookLocally: opts.deleteBookLocally ?? (async () => {}),
+});
+
+const makeIndex = (books: Book[]): RemoteLibraryIndex => ({
+  schemaVersion: 1,
+  updatedAt: 1,
+  books,
+});
+
+const libraryWrite = (captured: Captured) =>
+  captured.writes.find((w) => w.path.endsWith('library.json'));
+
+describe('FileSyncEngine.syncLibrary — deletion propagation (#4860)', () => {
+  test('deletes a book locally when a peer tombstoned it', async () => {
+    const captured: Captured = { writes: [], deletedDirs: [] };
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 100, deletedAt: 200 })]))
+          : null,
+      captured,
+    });
+    const deleteBookLocally = vi.fn<(b: Book) => Promise<void>>(async () => {});
+    const store = fakeStore({ deleteBookLocally });
+
+    const res = await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100 })],
+      { strategy: 'silent', syncBooks: false, deviceId: 'd' },
+    );
+
+    expect(deleteBookLocally).toHaveBeenCalledTimes(1);
+    expect(deleteBookLocally.mock.calls[0]![0].hash).toBe('h1');
+    expect(deleteBookLocally.mock.calls[0]![0].deletedAt).toBe(200);
+    expect(res.booksDeleted).toBe(1);
+
+    // The tombstone must be carried into the re-pushed index.
+    const idx = libraryWrite(captured);
+    expect(idx).toBeDefined();
+    const parsed = JSON.parse(idx!.body) as RemoteLibraryIndex;
+    const h1 = parsed.books.find((b) => b.hash === 'h1');
+    expect(h1?.deletedAt).toBe(200);
+  });
+
+  test('does not re-push the config of a book it just peer-deleted', async () => {
+    const captured: Captured = { writes: [], deletedDirs: [] };
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 100, deletedAt: 200 })]))
+          : p.endsWith('config.json')
+            ? JSON.stringify({ schemaVersion: 1, bookHash: 'h1', config: {}, booknotes: [] })
+            : null,
+      captured,
+    });
+    // The book file is still on disk after a 'local' delete removes the copy, so
+    // loadConfig returns a real config — the push loop must still skip it.
+    const store = fakeStore({
+      loadConfig: async () => ({ updatedAt: 1, booknotes: [] }),
+      loadBookFile: async () => ({ bytes: new ArrayBuffer(8), size: 8 }),
+    });
+
+    const res = await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100 })],
+      { strategy: 'silent', syncBooks: true, deviceId: 'd' },
+    );
+
+    expect(res.booksDeleted).toBe(1);
+    expect(res.configsUploaded).toBe(0);
+    expect(res.filesUploaded).toBe(0);
+    expect(captured.writes.filter((w) => w.path.endsWith('config.json'))).toHaveLength(0);
+  });
+
+  test('does not delete locally when the local copy was edited after the deletion', async () => {
+    const captured: Captured = { writes: [], deletedDirs: [] };
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 50, deletedAt: 100 })]))
+          : null,
+      captured,
+    });
+    const deleteBookLocally = vi.fn<(b: Book) => Promise<void>>(async () => {});
+    const store = fakeStore({ deleteBookLocally });
+
+    const res = await new FileSyncEngine(provider, store).syncLibrary(
+      // Local edit (updatedAt 200) is newer than the remote deletion (100).
+      [makeBook('h1', { updatedAt: 200 })],
+      { strategy: 'silent', syncBooks: false, deviceId: 'd' },
+    );
+
+    expect(deleteBookLocally).not.toHaveBeenCalled();
+    expect(res.booksDeleted).toBe(0);
+  });
+
+  test('GCs the remote hash directory of a locally-deleted book', async () => {
+    const captured: Captured = { writes: [], deletedDirs: [] };
+    const provider = fakeProvider({
+      readText: async () => null, // fresh remote index
+      list: async (path: string) =>
+        path.endsWith('/books')
+          ? [{ name: 'h1', path: '/Readest/books/h1', isDirectory: true }]
+          : [],
+      captured,
+    });
+    const store = fakeStore();
+
+    await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100, deletedAt: 100 })],
+      { strategy: 'silent', syncBooks: true, deviceId: 'd' },
+    );
+
+    expect(captured.deletedDirs).toContain('/Readest/books/h1');
+  });
+
+  test('does not GC a hash dir that is no longer on the server', async () => {
+    const captured: Captured = { writes: [], deletedDirs: [] };
+    const provider = fakeProvider({
+      readText: async () => null,
+      list: async () => [], // the books dir is empty — nothing to GC
+      captured,
+    });
+    const store = fakeStore();
+
+    await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100, deletedAt: 100 })],
+      { strategy: 'silent', syncBooks: true, deviceId: 'd' },
+    );
+
+    expect(captured.deletedDirs).toHaveLength(0);
+  });
+
+  test('preserves a remote tombstone for a book this device never had', async () => {
+    const captured: Captured = { writes: [], deletedDirs: [] };
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(
+              makeIndex([
+                makeBook('h1', { updatedAt: 100 }),
+                makeBook('h2', { updatedAt: 100, deletedAt: 300 }),
+              ]),
+            )
+          : null,
+      captured,
+    });
+    const store = fakeStore();
+
+    // Local library only has h1; it has never seen h2.
+    await new FileSyncEngine(provider, store).syncLibrary([makeBook('h1', { updatedAt: 100 })], {
+      strategy: 'silent',
+      syncBooks: false,
+      deviceId: 'd',
+    });
+
+    const idx = libraryWrite(captured);
+    expect(idx).toBeDefined();
+    const parsed = JSON.parse(idx!.body) as RemoteLibraryIndex;
+    const h2 = parsed.books.find((b) => b.hash === 'h2');
+    expect(h2?.deletedAt).toBe(300);
+  });
+});
@@ -92,6 +92,7 @@ const makeStore = (overrides: Partial<LocalStore> = {}): LocalStore => ({
  saveBookCover: async () => {},
  addBookToLibrary: async () => {},
  updateBookMetadata: async () => {},
+  deleteBookLocally: async () => {},
  ...overrides,
 });

@@ -58,6 +58,7 @@ const fakeStore = (opts: Partial<LocalStore> = {}): LocalStore => ({
  saveBookCover: opts.saveBookCover ?? (async () => {}),
  addBookToLibrary: opts.addBookToLibrary ?? (async () => {}),
  updateBookMetadata: opts.updateBookMetadata ?? (async () => {}),
+  deleteBookLocally: opts.deleteBookLocally ?? (async () => {}),
 });

 describe('FileSyncEngine.pushBookFile — streaming upload', () => {
@@ -163,10 +164,11 @@ describe('FileSyncEngine.syncLibrary — receive strategy is pull-only', () => {
  });
 });

-const makeIndex = (books: Book[]): RemoteLibraryIndex => ({
+const makeIndex = (books: Book[], uploadedHashes?: string[]): RemoteLibraryIndex => ({
  schemaVersion: 1,
  updatedAt: 1,
  books,
+  ...(uploadedHashes ? { uploadedHashes } : {}),
 });

 const makeEnvelope = (over: Partial<RemoteBookConfig> = {}): RemoteBookConfig => ({
@@ -273,6 +275,150 @@ describe('FileSyncEngine.syncLibrary — incremental diff (default)', () => {
    expect(configWrites(captured)).toHaveLength(0);
  });

+  // #4856: enabling "Upload Book Files" AFTER the first (config-only) sync must
+  // upload the file even though the book's config is already in sync with the
+  // index (its updatedAt is unchanged, so the incremental cursor would skip it).
+  test('uploads the file when syncBooks is enabled after a config-only first sync', async () => {
+    const captured: Captured = { writes: [] };
+    const binaryWrites: string[] = [];
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 100 })]))
+          : null,
+      head: async () => null, // no book file on the remote yet
+      writeBinary: async (path: string) => {
+        binaryWrites.push(path);
+      },
+      captured,
+    });
+    const store = fakeStore({
+      loadConfig: async () => ({ updatedAt: 1, booknotes: [] }),
+      loadBookFile: async () => ({ bytes: new ArrayBuffer(10), size: 10 }),
+    });
+
+    const res = await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100 })],
+      {
+        strategy: 'silent',
+        syncBooks: true,
+        deviceId: 'd',
+      },
+    );
+
+    expect(res.filesUploaded).toBe(1);
+    expect(binaryWrites.some((p) => p.includes('/Readest/books/h1/'))).toBe(true);
+    // The config is already in sync, so it must NOT be re-pushed.
+    expect(configWrites(captured)).toHaveLength(0);
+    // The upload is recorded so the next incremental sync skips the probe.
+    const idx = JSON.parse(
+      captured.writes.find((w) => w.path.endsWith('library.json'))!.body,
+    ) as RemoteLibraryIndex;
+    expect(idx.uploadedHashes).toContain('h1');
+  });
+
+  test('skips the file when the remote already has a same-size copy (syncBooks on)', async () => {
+    const captured: Captured = { writes: [] };
+    const binaryWrites: string[] = [];
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 100 })]))
+          : null,
+      head: async () => ({ size: 10 }), // remote already has the file, same size
+      writeBinary: async (path: string) => {
+        binaryWrites.push(path);
+      },
+      captured,
+    });
+    const store = fakeStore({
+      loadConfig: async () => ({ updatedAt: 1, booknotes: [] }),
+      loadBookFile: async () => ({ bytes: new ArrayBuffer(10), size: 10 }),
+    });
+
+    const res = await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100 })],
+      {
+        strategy: 'silent',
+        syncBooks: true,
+        deviceId: 'd',
+      },
+    );
+
+    expect(res.filesUploaded).toBe(0);
+    expect(res.filesAlreadyInSync).toBe(1);
+    expect(binaryWrites).toHaveLength(0);
+    expect(configWrites(captured)).toHaveLength(0);
+    // The freshly-verified file is now recorded so the next sync skips it.
+    const idx = JSON.parse(
+      captured.writes.find((w) => w.path.endsWith('library.json'))!.body,
+    ) as RemoteLibraryIndex;
+    expect(idx.uploadedHashes).toContain('h1');
+  });
+
+  // #4856 perf: once a file is recorded in the index, an incremental sync must
+  // NOT HEAD-probe it again — the steady state stays O(changed), not O(library).
+  test('does not probe an already-recorded file (stays O(changed))', async () => {
+    const captured: Captured = { writes: [] };
+    const head = vi.fn(async () => null);
+    const loadBookFile = vi.fn(async () => ({ bytes: new ArrayBuffer(10), size: 10 }));
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 100 })], ['h1']))
+          : null,
+      head,
+      captured,
+    });
+    const store = fakeStore({
+      loadConfig: async () => ({ updatedAt: 1, booknotes: [] }),
+      loadBookFile,
+    });
+
+    const res = await new FileSyncEngine(provider, store).syncLibrary(
+      [makeBook('h1', { updatedAt: 100 })],
+      { strategy: 'silent', syncBooks: true, deviceId: 'd' },
+    );
+
+    // No file work at all: no HEAD probe, no bytes read, no upload.
+    expect(head).not.toHaveBeenCalled();
+    expect(loadBookFile).not.toHaveBeenCalled();
+    expect(res.filesUploaded).toBe(0);
+    expect(res.filesAlreadyInSync).toBe(0);
+    expect(res.booksSynced).toBe(0);
+    // The recorded hash is preserved across the re-push.
+    const idx = JSON.parse(
+      captured.writes.find((w) => w.path.endsWith('library.json'))!.body,
+    ) as RemoteLibraryIndex;
+    expect(idx.uploadedHashes).toContain('h1');
+  });
+
+  test('fullSync re-probes a recorded file (drift escape hatch)', async () => {
+    const head = vi.fn(async () => ({ size: 10 }));
+    const provider = fakeProvider({
+      readText: async (p) =>
+        p.endsWith('library.json')
+          ? JSON.stringify(makeIndex([makeBook('h1', { updatedAt: 100 })], ['h1']))
+          : null,
+      head,
+      captured: { writes: [] },
+    });
+    const store = fakeStore({
+      loadConfig: async () => ({ updatedAt: 1, booknotes: [] }),
+      loadBookFile: async () => ({ bytes: new ArrayBuffer(10), size: 10 }),
+    });
+
+    await new FileSyncEngine(provider, store).syncLibrary([makeBook('h1', { updatedAt: 100 })], {
+      strategy: 'silent',
+      syncBooks: true,
+      deviceId: 'd',
+      fullSync: true,
+    });
+
+    // Full Sync bypasses the record and re-verifies the file.
+    expect(head).toHaveBeenCalled();
+  });
+
  test('fullSync re-pushes an in-sync book', async () => {
    const captured: Captured = { writes: [] };
    const provider = fakeProvider({
@@ -59,4 +59,16 @@ describe('wire envelope (frozen)', () => {
    expect(ok?.books).toHaveLength(1);
    expect(ok?.updatedAt).toBe(5);
  });
+
+  test('parseRemoteLibraryIndex preserves the optional uploadedHashes record (#4856)', () => {
+    const parsed = parseRemoteLibraryIndex(
+      JSON.stringify({ schemaVersion: 1, books: [book], updatedAt: 5, uploadedHashes: ['h1'] }),
+    );
+    expect(parsed?.uploadedHashes).toEqual(['h1']);
+    // Legacy index without the field parses fine (treated as empty by the engine).
+    const legacy = parseRemoteLibraryIndex(
+      JSON.stringify({ schemaVersion: 1, books: [book], updatedAt: 5 }),
+    );
+    expect(legacy?.uploadedHashes).toBeUndefined();
+  });
 });
@@ -130,4 +130,22 @@ export const createAppLocalStore = ({
    // new title / author / cover show up without a reload.
    await useLibraryStore.getState().updateBook(envConfig, book);
  },
+
+  deleteBookLocally: async (book) => {
+    // Remove this device's managed copy of the book file (cloudService.deleteBook
+    // with 'local' only ever touches app-managed Books/<hash>/ sources; an
+    // in-place / external original is left untouched). The tombstone itself is
+    // set by the engine before this call — we just persist it.
+    try {
+      await appService.deleteBook(book, 'local');
+    } catch (e) {
+      console.warn('createAppLocalStore: local book delete failed', book.hash, e);
+    }
+    book.coverImageUrl = null;
+    book.syncedAt = Date.now();
+    if (!useLibraryStore.getState().libraryLoaded) {
+      useLibraryStore.getState().setLibrary(await appService.loadLibraryBooks());
+    }
+    await useLibraryStore.getState().updateBook(envConfig, book);
+  },
 });
@@ -68,6 +68,8 @@ export interface SyncLibraryResult {
  filesAlreadyInSync: number;
  coversUploaded: number;
  booksDownloaded: number;
+  /** Local books removed because a peer's tombstone propagated to this device (#4860). */
+  booksDeleted: number;
  /** Already-local books whose metadata was refreshed from a newer index copy (#4756). */
  metadataUpdated: number;
  /** Distinct books that had any sync activity (pushed, downloaded, or reconciled). */
@@ -363,6 +365,7 @@ export class FileSyncEngine {
      filesAlreadyInSync: 0,
      coversUploaded: 0,
      booksDownloaded: 0,
+      booksDeleted: 0,
      metadataUpdated: 0,
      booksSynced: 0,
      failures: 0,
@@ -413,6 +416,20 @@ export class FileSyncEngine {
      return (book.updatedAt ?? 0) > (remote.updatedAt ?? 0);
    };

+    // File-upload cursor (#4856): the index records which book FILES already
+    // live on the remote. A book's file is immutable per hash, so once recorded
+    // it never needs re-checking — this keeps an incremental sync O(changed)
+    // by skipping the per-book HEAD probe for already-mirrored files instead of
+    // probing every book each run. Seeded from the pulled index and carried
+    // forward (plus this run's uploads) into the re-pushed index. Empty in send
+    // mode / on a fresh remote, so the first sync verifies every file once.
+    const uploadedHashes = new Set<string>(remoteIndex?.uploadedHashes ?? []);
+    // A file needs (re)uploading only when syncBooks is on and the remote copy
+    // isn't recorded yet. Full Sync bypasses the record as an escape hatch for
+    // drift (e.g. a file deleted out-of-band via the browse pane).
+    const needsFilePush = (book: Book): boolean =>
+      options.syncBooks && (fullSync || !uploadedHashes.has(book.hash));
+
    const remoteBooksToDownload: Book[] = [];
    // The remote source of truth for a book's on-disk filename is the per-hash
    // directory listing — NOT the book's title (which may be stale). We always
@@ -474,6 +491,43 @@ export class FileSyncEngine {
      });
    }

+    // Deletion propagation (#4860): a book a peer tombstoned in the shared index
+    // must be removed from this device too, not just hidden on the origin. Apply
+    // the deletion with edit-wins-over-delete semantics — only when it is newer
+    // than any local change, so a device that kept reading a book after another
+    // device deleted it keeps its copy (and the live row re-revives the tombstone
+    // on the next push).
+    if (canPull && remoteIndex && remoteIndex.books) {
+      const remoteDeletions = remoteIndex.books.filter((rb) => {
+        if (!rb.deletedAt) return false;
+        const local = allBooksMap.get(rb.hash);
+        return !!local && !local.deletedAt && (rb.deletedAt ?? 0) > (local.updatedAt ?? 0);
+      });
+      await runPool(remoteDeletions, concurrency, async (rb) => {
+        const local = allBooksMap.get(rb.hash)!;
+        const deleted: Book = {
+          ...local,
+          deletedAt: rb.deletedAt,
+          downloadedAt: null,
+          coverDownloadedAt: null,
+          updatedAt: Math.max(local.updatedAt ?? 0, rb.updatedAt ?? 0),
+        };
+        try {
+          await this.store.deleteBookLocally(deleted);
+          // Keep the tombstone in allBooksMap so the index re-push carries it.
+          allBooksMap.set(rb.hash, deleted);
+          result.booksDeleted += 1;
+          syncedHashes.add(rb.hash);
+        } catch (e) {
+          console.warn('file sync: local delete failed', rb.hash, e);
+        }
+      });
+    }
+
+    // Hash directories that still exist on the remote. Populated by the discovery
+    // scan below and reused by the deleted-book GC before the index re-push.
+    const remoteHashDirs = new Set<string>();
+
    if (canPull) {
      const candidateHashes = new Set<string>();

@@ -495,7 +549,9 @@ export class FileSyncEngine {
        const booksDirPath = `${buildBasePath(this.provider.rootPath)}/${SYNC_BOOKS_DIR}`;
        const dirEntries = await this.provider.list(booksDirPath);
        for (const entry of dirEntries) {
-          if (entry.isDirectory && !allBooksMap.has(entry.name)) {
+          if (!entry.isDirectory) continue;
+          remoteHashDirs.add(entry.name);
+          if (!allBooksMap.has(entry.name)) {
            candidateHashes.add(entry.name);
          }
        }
@@ -608,6 +664,9 @@ export class FileSyncEngine {
            await this.store.addBookToLibrary(rb);
            result.booksDownloaded += 1;
            syncedHashes.add(rb.hash);
+            // We just pulled its bytes, so the file is on the remote — record it
+            // so a later push-side sync doesn't HEAD-probe it back.
+            uploadedHashes.add(rb.hash);
          } else {
            // No bytes returned (typically a 404 we couldn't resolve).
            result.failures += 1;
@@ -635,10 +694,23 @@ export class FileSyncEngine {
    // Books we just downloaded already exist on the remote — don't re-push
    // them. Only push books already present in the caller-supplied library.
    const downloadedHashes = new Set(remoteBooksToDownload.map((b) => b.hash));
-    // Incremental (default): push only books that changed locally since the
-    // last index push. Full-sync re-checks everything.
+    // A book's config/cover only need pushing when it changed locally since the
+    // last index push (incremental; full-sync re-checks everything). Its FILE,
+    // by contrast, is immutable per hash and only needs uploading when the
+    // remote copy is missing per the index's uploaded-file record (`needsFilePush`)
+    // — which catches the user enabling "Upload Book Files" only after the first
+    // (config-only) sync (#4856) without a per-book probe once files are recorded.
+    const configChanged = (b: Book): boolean => fullSync || isLocalNewer(b);
+    // Consult the merged state, not the caller's raw book: a book a peer just
+    // tombstoned in this same run is now deletedAt in allBooksMap even though
+    // the caller's array copy isn't — pushing it would re-upload a book we are
+    // about to GC (#4860).
+    const isEffectivelyDeleted = (b: Book): boolean => !!(allBooksMap.get(b.hash) ?? b).deletedAt;
    const booksToPush = books.filter(
-      (b) => !b.deletedAt && !downloadedHashes.has(b.hash) && (fullSync || isLocalNewer(b)),
+      (b) =>
+        !isEffectivelyDeleted(b) &&
+        !downloadedHashes.has(b.hash) &&
+        (configChanged(b) || needsFilePush(b)),
    );
    result.totalBooks = booksToPush.length;

@@ -654,51 +726,57 @@ export class FileSyncEngine {
        pushStarted += 1;
        let phase: SyncFailureEntry['phase'] = 'upload-config';
        try {
-          const config = await this.store.loadConfig(book);
-          if (config) {
-            // Mirror the reader hook's pull-merge-push discipline so a manual
-            // "Sync now" can't blind-overwrite state this device hasn't pulled
-            // yet. Only in two-way ('silent') mode — 'send' keeps the blind
-            // push. A failed pull-merge falls back to the local config.
-            let configToPush = config;
-            if (canPull) {
-              try {
-                const pull = await this.pullBookConfig(book, config);
-                if (pull.applied && pull.mergedConfig) {
-                  configToPush = pull.mergedConfig;
-                  // Persist the merged superset locally so this device
-                  // converges too, not just the remote.
-                  await this.store.saveBookConfig(book, pull.mergedConfig);
+          if (configChanged(book)) {
+            const config = await this.store.loadConfig(book);
+            if (config) {
+              // Mirror the reader hook's pull-merge-push discipline so a manual
+              // "Sync now" can't blind-overwrite state this device hasn't pulled
+              // yet. Only in two-way ('silent') mode — 'send' keeps the blind
+              // push. A failed pull-merge falls back to the local config.
+              let configToPush = config;
+              if (canPull) {
+                try {
+                  const pull = await this.pullBookConfig(book, config);
+                  if (pull.applied && pull.mergedConfig) {
+                    configToPush = pull.mergedConfig;
+                    // Persist the merged superset locally so this device
+                    // converges too, not just the remote.
+                    await this.store.saveBookConfig(book, pull.mergedConfig);
+                  }
+                } catch (e) {
+                  console.warn('file sync: config pull-merge failed', book.hash, e);
                }
-              } catch (e) {
-                console.warn('file sync: config pull-merge failed', book.hash, e);
              }
-            }
-            await this.pushBookConfig(book, configToPush, options.deviceId);
-            result.configsUploaded += 1;
-            syncedHashes.add(book.hash);
-          }
-          // Covers ride along with the config-level sync, NOT with syncBooks:
-          // the receiving device can't regenerate them without the book bytes.
-          // Failures here are warnings, not hard failures.
-          try {
-            const coverResult = await this.pushBookCover(book);
-            if (coverResult.uploaded) {
-              result.coversUploaded += 1;
+              await this.pushBookConfig(book, configToPush, options.deviceId);
+              result.configsUploaded += 1;
              syncedHashes.add(book.hash);
            }
-          } catch (e) {
-            console.warn('file sync: cover failed', book.hash, e);
+            // Covers ride along with the config-level sync, NOT with syncBooks:
+            // the receiving device can't regenerate them without the book bytes.
+            // Failures here are warnings, not hard failures.
+            try {
+              const coverResult = await this.pushBookCover(book);
+              if (coverResult.uploaded) {
+                result.coversUploaded += 1;
+                syncedHashes.add(book.hash);
+              }
+            } catch (e) {
+              console.warn('file sync: cover failed', book.hash, e);
+            }
          }
-          if (options.syncBooks) {
+          if (needsFilePush(book)) {
            phase = 'upload-file';
            const fileResult = await this.pushBookFile(book);
            if (fileResult.uploaded) {
              result.filesUploaded += 1;
              syncedHashes.add(book.hash);
+              uploadedHashes.add(book.hash);
            } else if (fileResult.reason === 'remote-matches') {
              result.filesAlreadyInSync += 1;
+              uploadedHashes.add(book.hash);
            }
+            // 'no-source' → the file isn't on this device; leave it unrecorded
+            // so a device that does have it can upload and record it later.
          }
        } catch (e) {
          result.failures += 1;
@@ -713,16 +791,47 @@ export class FileSyncEngine {
      });
    }

-    // Push the merged index whenever we're allowed to write, even if no
-    // binaries moved this turn (keeps library.json authoritative). Soft-deleted
-    // books' per-hash dirs are intentionally NOT GC'd here — that's the manual
-    // cleanup sweep's job (see deleteRemoteBookDir).
+    // The final index whenever we're allowed to write, even if no binaries
+    // moved this turn (keeps library.json authoritative). Union in any remote
+    // entries this device never materialised (chiefly peers' tombstones):
+    // rebuilding purely from allBooksMap would drop a deletion for a book we
+    // never had, silently reviving it for every other device (#4860).
    if (canPush) {
+      const indexByHash = new Map(allBooksMap);
+      if (remoteIndex?.books) {
+        for (const rb of remoteIndex.books) {
+          if (!indexByHash.has(rb.hash)) indexByHash.set(rb.hash, rb);
+        }
+      }
+
+      // GC the remote per-hash directory of every tombstoned book whose files
+      // still linger on the server (#4860). Scoped to dirs the discovery scan
+      // actually saw, so a dir removed on a previous sync is never re-DELETEd
+      // and 'send' mode (which never lists) is a safe no-op. This is what makes
+      // a deletion reclaim server space instead of leaving orphaned book files.
+      const dirsToGc = Array.from(remoteHashDirs).filter(
+        (hash) => indexByHash.get(hash)?.deletedAt,
+      );
+      await runPool(dirsToGc, concurrency, async (hash) => {
+        try {
+          await deleteRemoteBookDir(this.provider, hash);
+        } catch (e) {
+          console.warn('file sync: failed to GC deleted book dir', hash, e);
+        }
+      });
+
      try {
        const newIndex: RemoteLibraryIndex = {
          schemaVersion: 1,
-          books: Array.from(allBooksMap.values()),
+          books: Array.from(indexByHash.values()),
          updatedAt: Date.now(),
+          // Carry the uploaded-file record forward so the next incremental sync
+          // stays O(changed). Keep only hashes that still map to a live indexed
+          // book so the set can't grow unbounded with tombstoned / evicted books.
+          uploadedHashes: Array.from(uploadedHashes).filter((hash) => {
+            const b = indexByHash.get(hash);
+            return !!b && !b.deletedAt;
+          }),
        };
        await this.pushLibraryIndex(newIndex);
      } catch (e) {
@@ -50,4 +50,11 @@ export interface LocalStore {
  addBookToLibrary(book: Book): Promise<void>;
  /** Persist refreshed metadata for a book already in the local library. */
  updateBookMetadata(book: Book): Promise<void>;
+  /**
+   * Apply a peer's deletion locally: remove this device's managed copy of the
+   * book file and persist the tombstone (`book.deletedAt`) so the book drops
+   * off the shelf and stops being re-uploaded. External / in-place sources are
+   * never touched — only the app-managed `Books/<hash>/` copy is removed.
+   */
+  deleteBookLocally(book: Book): Promise<void>;
 }
@@ -96,6 +96,18 @@ export interface RemoteLibraryIndex {
  schemaVersion: 1;
  books: Book[];
  updatedAt: number;
+  /**
+   * Hashes whose book FILE (not just config/cover) is confirmed present on the
+   * remote. A book's file is immutable per hash, so once uploaded it never
+   * needs re-checking — recording it here lets an incremental "Sync now" skip
+   * the per-book HEAD probe for already-mirrored files and stay O(changed)
+   * instead of O(library) when "Upload Book Files" is on (#4856).
+   *
+   * Optional + additive: a legacy/absent value is treated as empty, so an old
+   * client that rewrites the index simply drops it and the next new-client sync
+   * re-verifies each file once (a bounded, self-healing HEAD) and re-records it.
+   */
+  uploadedHashes?: string[];
 }

 export const parseRemoteLibraryIndex = (raw: string | null): RemoteLibraryIndex | null => {