fix(library): stop bookshelf context menu shuffling its order (#4389) (#4421)

The bookshelf right-click menu built itself with un-awaited
`Menu.append()` calls. Each append is an async IPC round-trip to the
Tauri backend, so the concurrent fire-and-forget requests resolved in
non-deterministic order on the Rust side and the native menu items
landed shuffled on every open (only reproducible on the native app,
invisible in jsdom).

Build the items in order and create the menu in a single
`await Menu.new({ items })` call for both the book and group handlers.
Order and conditional inclusion are unchanged.

Extract the order/inclusion logic into a pure `getBookContextMenuItemIds`
helper in `libraryUtils.ts` so the deterministic ordering is unit-tested
without mounting the component or mocking Tauri.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/readest-app/src/__tests__/app/library/book-context-menu.test.ts

@@ -0,0 +1,94 @@
+import { describe, expect, it } from 'vitest';
+
+import { getBookContextMenuItemIds } from '@/app/library/utils/libraryUtils';
+import { Book } from '@/types/book';
+
+const createBook = (overrides: Partial<Book> = {}): Book => ({
+  hash: 'hash-1',
+  format: 'EPUB',
+  title: 'Test Book',
+  author: 'Test Author',
+  createdAt: 0,
+  updatedAt: 0,
+  ...overrides,
+});
+
+describe('getBookContextMenuItemIds', () => {
+  it('returns a deterministic order for a local downloaded book', () => {
+    const book = createBook({ downloadedAt: 1 });
+    expect(getBookContextMenuItemIds(book)).toEqual([
+      'select',
+      'group',
+      'markFinished',
+      'showDetails',
+      'showInFinder',
+      'upload',
+      'share',
+      'delete',
+    ]);
+  });
+
+  it('shows "Mark as Unread" + "Clear Status" for a finished book', () => {
+    const book = createBook({ downloadedAt: 1, readingStatus: 'finished' });
+    expect(getBookContextMenuItemIds(book)).toEqual([
+      'select',
+      'group',
+      'markUnread',
+      'clearStatus',
+      'showDetails',
+      'showInFinder',
+      'upload',
+      'share',
+      'delete',
+    ]);
+  });
+
+  it('shows "Mark as Finished" + "Clear Status" for an unread book', () => {
+    const book = createBook({ downloadedAt: 1, readingStatus: 'unread' });
+    expect(getBookContextMenuItemIds(book)).toEqual([
+      'select',
+      'group',
+      'markFinished',
+      'clearStatus',
+      'showDetails',
+      'showInFinder',
+      'upload',
+      'share',
+      'delete',
+    ]);
+  });
+
+  it('offers Download (not Upload) for a cloud-only book', () => {
+    const book = createBook({ uploadedAt: 1 });
+    expect(getBookContextMenuItemIds(book)).toEqual([
+      'select',
+      'group',
+      'markFinished',
+      'showDetails',
+      'showInFinder',
+      'download',
+      'share',
+      'delete',
+    ]);
+  });
+
+  it('omits download/upload/share for a book that is neither downloaded nor uploaded', () => {
+    const book = createBook({ filePath: '/some/external/file.epub' });
+    expect(getBookContextMenuItemIds(book)).toEqual([
+      'select',
+      'group',
+      'markFinished',
+      'showDetails',
+      'showInFinder',
+      'delete',
+    ]);
+  });
+
+  it('produces the same order on repeated calls and never duplicates an item (issue #4389)', () => {
+    const book = createBook({ downloadedAt: 1, uploadedAt: 1, readingStatus: 'finished' });
+    const first = getBookContextMenuItemIds(book);
+    const second = getBookContextMenuItemIds(book);
+    expect(second).toEqual(first);
+    expect(new Set(first).size).toBe(first.length);
+  });
+});

apps/readest-app/src/app/library/components/BookshelfItem.tsx

@@ -6,7 +6,7 @@ import { useSettingsStore } from '@/store/settingsStore';
 import { useTranslation } from '@/hooks/useTranslation';
 import { useAppRouter } from '@/hooks/useAppRouter';
 import { useLongPress } from '@/hooks/useLongPress';
-import { Menu, MenuItem } from '@tauri-apps/api/menu';
+import { Menu, type MenuItemOptions } from '@tauri-apps/api/menu';
 import { revealItemInDir } from '@tauri-apps/plugin-opener';
 import { eventDispatcher } from '@/utils/event';
 import { getOSPlatform } from '@/utils/misc';
@@ -16,6 +16,10 @@ import { LibraryCoverFitType, LibraryViewModeType } from '@/types/settings';
 import { BOOK_UNGROUPED_ID, BOOK_UNGROUPED_NAME } from '@/services/constants';
 import { FILE_REVEAL_LABELS, FILE_REVEAL_PLATFORMS } from '@/utils/os';
 import { Book, BooksGroup, ReadingStatus } from '@/types/book';
+import {
+  getBookContextMenuItemIds,
+  type BookContextMenuItemId,
+} from '@/app/library/utils/libraryUtils';
 import { md5Fingerprint } from '@/utils/md5';
 import BookItem from './BookItem';
 import GroupItem from './GroupItem';
@@ -214,144 +218,127 @@ const BookshelfItem: React.FC<BookshelfItemProps> = ({
     const osPlatform = getOSPlatform();
     const fileRevealLabel =
       FILE_REVEAL_LABELS[osPlatform as FILE_REVEAL_PLATFORMS] || FILE_REVEAL_LABELS.default;
-    const selectBookMenuItem = await MenuItem.new({
-      text: itemSelected ? _('Deselect Book') : _('Select Book'),
-      action: async () => {
-        if (!isSelectMode) handleSetSelectMode(true);
-        toggleSelection(book.hash);
-      },
-    });
-    const groupBooksMenuItem = await MenuItem.new({
-      text: _('Group Books'),
-      action: async () => {
-        if (!isSelectMode) handleSetSelectMode(true);
-        if (!itemSelected) {
+    // Build every item up front, then create the menu from the ordered subset
+    // in a single Menu.new({ items }) call. Appending items one-by-one with
+    // un-awaited Menu.append() promises races on the Tauri IPC boundary and
+    // shuffles the order on every open (issue #4389).
+    const itemOptions: Record<BookContextMenuItemId, MenuItemOptions> = {
+      select: {
+        text: itemSelected ? _('Deselect Book') : _('Select Book'),
+        action: async () => {
+          if (!isSelectMode) handleSetSelectMode(true);
           toggleSelection(book.hash);
-        }
-        handleGroupBooks();
+        },
       },
-    });
-    const markAsFinishedMenuItem = await MenuItem.new({
-      text: _('Mark as Finished'),
-      action: async () => {
-        handleUpdateReadingStatus(book, 'finished');
+      group: {
+        text: _('Group Books'),
+        action: async () => {
+          if (!isSelectMode) handleSetSelectMode(true);
+          if (!itemSelected) {
+            toggleSelection(book.hash);
+          }
+          handleGroupBooks();
+        },
       },
-    });
-    const markAsUnreadMenuItem = await MenuItem.new({
-      text: _('Mark as Unread'),
-      action: async () => {
-        handleUpdateReadingStatus(book, 'unread');
+      markFinished: {
+        text: _('Mark as Finished'),
+        action: async () => {
+          handleUpdateReadingStatus(book, 'finished');
+        },
       },
-    });
-    const clearStatusMenuItem = await MenuItem.new({
-      text: _('Clear Status'),
-      action: async () => {
-        handleUpdateReadingStatus(book, undefined);
+      markUnread: {
+        text: _('Mark as Unread'),
+        action: async () => {
+          handleUpdateReadingStatus(book, 'unread');
+        },
       },
-    });
-    const showBookInFinderMenuItem = await MenuItem.new({
-      text: _(fileRevealLabel),
-      action: async () => {
-        const folder = `${settings.localBooksDir}/${book.hash}`;
-        revealItemInDir(folder);
+      clearStatus: {
+        text: _('Clear Status'),
+        action: async () => {
+          handleUpdateReadingStatus(book, undefined);
+        },
       },
-    });
-    const showBookDetailsMenuItem = await MenuItem.new({
-      text: _('Show Book Details'),
-      action: async () => {
-        showBookDetailsModal(book);
+      showDetails: {
+        text: _('Show Book Details'),
+        action: async () => {
+          showBookDetailsModal(book);
+        },
       },
-    });
-    const downloadBookMenuItem = await MenuItem.new({
-      text: _('Download Book'),
-      action: async () => {
-        handleBookDownload(book, { queued: true });
+      showInFinder: {
+        text: _(fileRevealLabel),
+        action: async () => {
+          const folder = `${settings.localBooksDir}/${book.hash}`;
+          revealItemInDir(folder);
+        },
       },
-    });
-    const uploadBookMenuItem = await MenuItem.new({
-      text: _('Upload Book'),
-      action: async () => {
-        handleBookUpload(book);
+      download: {
+        text: _('Download Book'),
+        action: async () => {
+          handleBookDownload(book, { queued: true });
+        },
       },
-    });
-    const shareBookMenuItem = await MenuItem.new({
-      text: _('Share Book'),
-      action: async () => {
-        // Bookshelf.tsx hosts the dialog; we dispatch and let it route
-        // unauthenticated users into the login flow first.
-        eventDispatcher.dispatch('show-share-dialog', { book });
+      upload: {
+        text: _('Upload Book'),
+        action: async () => {
+          handleBookUpload(book);
+        },
       },
-    });
-    const deleteBookMenuItem = await MenuItem.new({
-      text: _('Delete'),
-      action: async () => {
-        eventDispatcher.dispatch('delete-books', { ids: [book.hash] });
+      share: {
+        text: _('Share Book'),
+        action: async () => {
+          // Bookshelf.tsx hosts the dialog; we dispatch and let it route
+          // unauthenticated users into the login flow first.
+          eventDispatcher.dispatch('show-share-dialog', { book });
+        },
       },
-    });
-    const menu = await Menu.new();
-    menu.append(selectBookMenuItem);
-    menu.append(groupBooksMenuItem);
-    if (book.readingStatus === 'finished') {
-      menu.append(markAsUnreadMenuItem);
-    } else {
-      menu.append(markAsFinishedMenuItem);
-    }
-    // show "Clear Status" option when book has an explicit status set
-    if (book.readingStatus === 'finished' || book.readingStatus === 'unread') {
-      menu.append(clearStatusMenuItem);
-    }
-    menu.append(showBookDetailsMenuItem);
-    menu.append(showBookInFinderMenuItem);
-    if (book.uploadedAt && !book.downloadedAt) {
-      menu.append(downloadBookMenuItem);
-    }
-    if (!book.uploadedAt && book.downloadedAt) {
-      menu.append(uploadBookMenuItem);
-    }
-    // Share is offered for any local-or-uploaded book; the dialog will trigger
-    // an upload first if the book hasn't been pushed yet.
-    if (book.downloadedAt || book.uploadedAt) {
-      menu.append(shareBookMenuItem);
-    }
-    menu.append(deleteBookMenuItem);
-    menu.popup();
+      delete: {
+        text: _('Delete'),
+        action: async () => {
+          eventDispatcher.dispatch('delete-books', { ids: [book.hash] });
+        },
+      },
+    };
+    const items = getBookContextMenuItemIds(book).map((id) => itemOptions[id]);
+    const menu = await Menu.new({ items });
+    await menu.popup();
   };
 
   const groupContextMenuHandler = async (group: BooksGroup) => {
     if (!appService?.hasContextMenu) return;
-    const selectGroupMenuItem = await MenuItem.new({
-      text: itemSelected ? _('Deselect Group') : _('Select Group'),
-      action: async () => {
-        if (!isSelectMode) handleSetSelectMode(true);
-        toggleSelection(group.id);
-      },
-    });
-    const groupBooksMenuItem = await MenuItem.new({
-      text: _('Group Books'),
-      action: async () => {
-        if (!isSelectMode) handleSetSelectMode(true);
-        if (!itemSelected) {
+    // Single Menu.new({ items }) call keeps the order deterministic — see the
+    // note in bookContextMenuHandler about the Menu.append() IPC race (#4389).
+    const items: MenuItemOptions[] = [
+      {
+        text: itemSelected ? _('Deselect Group') : _('Select Group'),
+        action: async () => {
+          if (!isSelectMode) handleSetSelectMode(true);
           toggleSelection(group.id);
-        }
-        handleGroupBooks();
+        },
       },
-    });
-    const deleteGroupMenuItem = await MenuItem.new({
-      text: _('Delete'),
-      action: async () => {
-        // Dispatch the constituent book hashes — `group.books` is the
-        // rendered rollup and already includes books from nested sub-
-        // folders, so the deletion path doesn't need to re-derive what
-        // belongs to the group from the id alone.
-        const ids = group.books.filter((book) => !book.deletedAt).map((book) => book.hash);
-        eventDispatcher.dispatch('delete-books', { ids });
+      {
+        text: _('Group Books'),
+        action: async () => {
+          if (!isSelectMode) handleSetSelectMode(true);
+          if (!itemSelected) {
+            toggleSelection(group.id);
+          }
+          handleGroupBooks();
+        },
       },
-    });
-    const menu = await Menu.new();
-    menu.append(selectGroupMenuItem);
-    menu.append(groupBooksMenuItem);
-    menu.append(deleteGroupMenuItem);
-    menu.popup();
+      {
+        text: _('Delete'),
+        action: async () => {
+          // Dispatch the constituent book hashes — `group.books` is the
+          // rendered rollup and already includes books from nested sub-
+          // folders, so the deletion path doesn't need to re-derive what
+          // belongs to the group from the id alone.
+          const ids = group.books.filter((book) => !book.deletedAt).map((book) => book.hash);
+          eventDispatcher.dispatch('delete-books', { ids });
+        },
+      },
+    ];
+    const menu = await Menu.new({ items });
+    await menu.popup();
   };
 
   // eslint-disable-next-line react-hooks/exhaustive-deps

apps/readest-app/src/app/library/utils/libraryUtils.ts

@@ -583,3 +583,41 @@ export const createGroupSorter =
 
     return 0;
   };
+
+export type BookContextMenuItemId =
+  | 'select'
+  | 'group'
+  | 'markFinished'
+  | 'markUnread'
+  | 'clearStatus'
+  | 'showDetails'
+  | 'showInFinder'
+  | 'download'
+  | 'upload'
+  | 'share'
+  | 'delete';
+
+/**
+ * Resolve the ordered list of context-menu item ids for a book from its state.
+ *
+ * The native menu MUST be built from this list in a single `Menu.new({ items })`
+ * call. Appending items one at a time with un-awaited `Menu.append()` promises
+ * races on the Tauri IPC boundary, so the items land in a non-deterministic
+ * order and the menu appears to shuffle on every open (issue #4389).
+ */
+export const getBookContextMenuItemIds = (book: Book): BookContextMenuItemId[] => {
+  const ids: BookContextMenuItemId[] = ['select', 'group'];
+  ids.push(book.readingStatus === 'finished' ? 'markUnread' : 'markFinished');
+  // "Clear Status" is offered only when the book has an explicit status set.
+  if (book.readingStatus === 'finished' || book.readingStatus === 'unread') {
+    ids.push('clearStatus');
+  }
+  ids.push('showDetails', 'showInFinder');
+  if (book.uploadedAt && !book.downloadedAt) ids.push('download');
+  if (!book.uploadedAt && book.downloadedAt) ids.push('upload');
+  // Share is offered for any local-or-uploaded book; the dialog uploads first
+  // if the book hasn't been pushed yet.
+  if (book.downloadedAt || book.uploadedAt) ids.push('share');
+  ids.push('delete');
+  return ids;
+};