feat(sync): bundle dictionary settings into the settings replica kind (#4096)

* feat(sync): bundle dictionary settings into the `settings` replica kind Adds three entries to the SETTINGS_WHITELIST so `providerOrder`, `providerEnabled`, and `webSearches` flow through the bundled settings replica with whole-field LWW. `defaultProviderId` (last- used tab) is deliberately excluded — it's per-device state. The customDictionaryStore exposes `applyRemoteDictionarySettings` so pulled values propagate into the in-memory mirror that the reader popup and the dictionary settings panel read from. Without this the mirror would stay stale until the next panel mount. Also fixes `saveCustomDictionaries`: it mutated the existing settings object in place and called `setSettings` with the same reference, so the replicaSettingsSync subscriber saw no change and never published. Build a fresh settings reference instead. Collapses the original PR 6 (`dict_provider_position`) and PR 7 (`dict_web_search`) plans, which proposed per-element CRDT rows with deterministic actor-id tiebreaks. Whole-field LWW is the right call given how rarely users edit these on two devices at once — same precedent as `customHighlightColors` and `customThemes` shipping through the bundled kind in PR 5. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(sync): make dict.id stable across devices (= contentId) Replaces the per-device `Math.random()` bundleDir as `dict.id` with the cross-device-stable `contentId`. `bundleDir` keeps tracking the device-local on-disk path, so on-disk layout is unchanged. Touch points: - 4 import paths in `dictionaryService.ts` + `buildLocalDictFromRow` in `replicaDictionaryApply.ts` set `id: contentId` instead of `id: bundleDir`. - Every `providerOrder` / `providerEnabled` entry that came from the dict store is now uniformly contentId-keyed, so the bundled `settings` replica syncs them across devices without any seam translation. Dicts with no contentId (very old, never synced) keep their bundleDir as id and remain local-only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 02:42:10 +08:00
parent 6e7c9d1395
commit 51a553dd89
10 changed files with 254 additions and 23 deletions
@@ -624,22 +624,35 @@ adapters for `dict_provider_pref`, `pref`, `theme`, `shortcut`,
 `annotation_rule`, etc.). The genuinely-different shapes (ordered
 collections, independent-record collections) ship as PR 6 and PR 7.

-### PR 6 — `dict_provider_position` (ordered list with per-position rows)
+### PR 6 — dictionary settings via the bundled `settings` whitelist

-The provider order is the only setting that genuinely needs per-element
-rows: concurrent rename + reorder must preserve both sides, which a
-single-field array can't do. Per-position rows keyed by
-`(position, actorId, replicaId)` with deterministic tiebreak.
-Migrates `dictionarySettings.providerOrder` off the single-array
-shape.
+Originally planned as two separate kinds (`dict_provider_position` for
+ordered providers, `dict_web_search` for custom web-search entries),
+but in practice the per-element CRDT machinery is over-engineered for
+data the user touches once a year from one device at a time.

-### PR 7 — `dict_web_search` (custom web-search entries)
+Collapsed into three new entries on the existing PR 5 whitelist:

-Collection of independent records — each with `id`, `name`,
-`urlTemplate`, plus tombstones. Per-record rows because users add /
-delete / rename entries independently across devices.
+- `dictionarySettings.providerOrder`
+- `dictionarySettings.providerEnabled`
+- `dictionarySettings.webSearches`

-### PR 8+ — incremental whitelist additions
+Same whole-field LWW semantics as `customHighlightColors` /
+`customThemes` (which are also `Record` / array shapes shipping
+through the bundled kind). `defaultProviderId` (last-used tab) is
+deliberately excluded — it's per-device state, not a preference.
+
+Concurrency cost on the rare double-edit: one side's reorder /
+toggle / web-search-add is lost, user redoes it. Acceptable given
+the edit frequency. The original per-position / per-record rows
+would have cost a new kind, a migration, and ~300 LOC of orchestration
+to handle a pathological case real users won't hit.
+
+The dictionary-side `customDictionaryStore` mirror exposes
+`applyRemoteDictionarySettings(patch)` so pulled values propagate
+into the popup + settings panel without a reload.
+
+### PR 7+ — incremental whitelist additions

 New scalar settings join the `settings` whitelist as needed (one-line
 PR + server schema bump). Future encrypted-field needs (AI API keys,
@@ -136,6 +136,16 @@ describe('SETTINGS_WHITELIST', () => {
      expect(key.length).toBeGreaterThan(0);
    }
  });
+
+  test('includes the dictionary settings paths (PR 6 — whole-field LWW)', () => {
+    expect(SETTINGS_WHITELIST).toContain('dictionarySettings.providerOrder');
+    expect(SETTINGS_WHITELIST).toContain('dictionarySettings.providerEnabled');
+    expect(SETTINGS_WHITELIST).toContain('dictionarySettings.webSearches');
+  });
+
+  test('does NOT sync dictionarySettings.defaultProviderId (per-device last-used tab)', () => {
+    expect(SETTINGS_WHITELIST).not.toContain('dictionarySettings.defaultProviderId');
+  });
 });

 describe('readPath / writePath', () => {
@@ -132,7 +132,9 @@ describe('buildLocalDictFromRow', () => {
    });
    const dict = buildLocalDictFromRow(row, 'local-bundle-1');
    expect(dict).not.toBe(null);
-    expect(dict!.id).toBe('local-bundle-1');
+    // dict.id is now the cross-device contentId (= replica_id), and
+    // bundleDir keeps the device-local on-disk path. (PR 6 Option B)
+    expect(dict!.id).toBe('content-hash-abc');
    expect(dict!.contentId).toBe('content-hash-abc');
    expect(dict!.kind).toBe('mdict');
    expect(dict!.name).toBe('Webster');
@@ -277,4 +277,70 @@ describe('applyRemoteSettings', () => {
    expect(useSettingsStore.getState().settings).toBe(before);
    expect(useSettingsStore.getState().saveSettings).not.toHaveBeenCalled();
  });
+
+  test('propagates dictionarySettings into useCustomDictionaryStore mirror', async () => {
+    const { useCustomDictionaryStore } = await import('@/store/customDictionaryStore');
+    useCustomDictionaryStore.setState({
+      ...useCustomDictionaryStore.getState(),
+      settings: {
+        providerOrder: ['local-x'],
+        providerEnabled: { 'local-x': true },
+        defaultProviderId: 'local-x',
+        webSearches: [],
+      },
+    });
+    const env = makeEnvConfig();
+    applyRemoteSettings(env, {
+      name: 'singleton',
+      patch: {
+        dictionarySettings: {
+          providerOrder: ['remote-y'],
+          providerEnabled: { 'remote-y': true },
+          webSearches: [{ id: 'web:remote-y', name: 'Y', urlTemplate: 'https://y/?q=%WORD%' }],
+        },
+      } as unknown as Partial<SystemSettings>,
+    });
+    const dictMirror = useCustomDictionaryStore.getState().settings;
+    expect(dictMirror.providerOrder).toEqual(['remote-y']);
+    expect(dictMirror.providerEnabled).toEqual({ 'remote-y': true });
+    expect(dictMirror.webSearches).toEqual([
+      { id: 'web:remote-y', name: 'Y', urlTemplate: 'https://y/?q=%WORD%' },
+    ]);
+    expect(dictMirror.defaultProviderId).toBe('local-x');
+  });
+
+  test('deep-merges dictionarySettings without clobbering local fields', () => {
+    const env = makeEnvConfig();
+    useSettingsStore.setState({
+      ...useSettingsStore.getState(),
+      settings: makeSettings({
+        dictionarySettings: {
+          providerOrder: ['local-x'],
+          providerEnabled: { 'local-x': true },
+          defaultProviderId: 'local-x',
+          webSearches: [],
+        },
+      } as Partial<SystemSettings>),
+    });
+    applyRemoteSettings(env, {
+      name: 'singleton',
+      patch: {
+        dictionarySettings: {
+          providerOrder: ['remote-y'],
+          providerEnabled: { 'remote-y': true },
+          // defaultProviderId omitted — must NOT be cleared by the merge.
+          webSearches: [{ id: 'web:remote-y', name: 'Y', urlTemplate: 'https://y/?q=%WORD%' }],
+        },
+      } as unknown as Partial<SystemSettings>,
+    });
+    const merged = useSettingsStore.getState().settings.dictionarySettings;
+    expect(merged.providerOrder).toEqual(['remote-y']);
+    expect(merged.providerEnabled).toEqual({ 'remote-y': true });
+    expect(merged.webSearches).toEqual([
+      { id: 'web:remote-y', name: 'Y', urlTemplate: 'https://y/?q=%WORD%' },
+    ]);
+    // defaultProviderId is per-device — preserved by the deep-merge even
+    // when the remote patch omits it.
+    expect(merged.defaultProviderId).toBe('local-x');
+  });
 });
@@ -293,3 +293,93 @@ describe('customDictionaryStore — web search CRUD', () => {
    expect(call[3]).toBe('epoch-1');
  });
 });
+
+describe('customDictionaryStore — saveCustomDictionaries reference identity (PR 6)', () => {
+  it('replaces useSettingsStore.settings with a NEW reference so subscribers fire', async () => {
+    // Seed the settings store with a real reducer so setSettings actually
+    // writes the new reference back.
+    type SettingsState = ReturnType<typeof useSettingsStore.getState>;
+    useSettingsStore.setState({
+      settings: {
+        customDictionaries: [],
+        dictionarySettings: {
+          providerOrder: ['a', 'b'],
+          providerEnabled: { a: true, b: true },
+          webSearches: [],
+        },
+      } as unknown as SettingsState['settings'],
+      setSettings: (s: SettingsState['settings']) => useSettingsStore.setState({ settings: s }),
+      saveSettings: vi.fn().mockResolvedValue(undefined),
+    } as unknown as SettingsState);
+
+    useCustomDictionaryStore.setState({
+      ...useCustomDictionaryStore.getState(),
+      dictionaries: [],
+      settings: {
+        providerOrder: ['b', 'a'], // reordered locally
+        providerEnabled: { a: true, b: true },
+        webSearches: [],
+      },
+    });
+
+    const before = useSettingsStore.getState().settings;
+    await useCustomDictionaryStore
+      .getState()
+      .saveCustomDictionaries({ name: 'env' } as unknown as EnvConfigType);
+    const after = useSettingsStore.getState().settings;
+
+    // The whole point: the post-save settings reference must be NEW
+    // so the replicaSettingsSync subscriber sees state.settings !==
+    // prev.settings and runs the publish diff. Mutating in place
+    // bypasses the subscriber and the reorder never syncs.
+    expect(after).not.toBe(before);
+    // …and the new reference reflects the reorder.
+    expect(after.dictionarySettings.providerOrder).toEqual(['b', 'a']);
+  });
+});
+
+describe('customDictionaryStore — applyRemoteDictionarySettings (PR 6)', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    useCustomDictionaryStore.setState({
+      dictionaries: [],
+      settings: {
+        providerOrder: ['local-x'],
+        providerEnabled: { 'local-x': true },
+        defaultProviderId: 'local-x',
+        webSearches: [],
+      },
+    });
+  });
+
+  it('overlays the remote dictionarySettings patch onto the in-memory mirror', () => {
+    const { applyRemoteDictionarySettings } = useCustomDictionaryStore.getState();
+    applyRemoteDictionarySettings({
+      providerOrder: ['remote-y'],
+      providerEnabled: { 'remote-y': true },
+      webSearches: [{ id: 'web:remote-y', name: 'Y', urlTemplate: 'https://y/?q=%WORD%' }],
+    });
+    const after = useCustomDictionaryStore.getState().settings;
+    expect(after.providerOrder).toEqual(['remote-y']);
+    expect(after.providerEnabled).toEqual({ 'remote-y': true });
+    expect(after.webSearches).toEqual([
+      { id: 'web:remote-y', name: 'Y', urlTemplate: 'https://y/?q=%WORD%' },
+    ]);
+  });
+
+  it('preserves defaultProviderId (per-device, not in remote patch)', () => {
+    const { applyRemoteDictionarySettings } = useCustomDictionaryStore.getState();
+    applyRemoteDictionarySettings({
+      providerOrder: ['remote-y'],
+      providerEnabled: { 'remote-y': true },
+    });
+    const after = useCustomDictionaryStore.getState().settings;
+    expect(after.defaultProviderId).toBe('local-x');
+  });
+
+  it('does NOT call publishReplicaUpsert (this is a pull, not a local edit)', () => {
+    const { applyRemoteDictionarySettings } = useCustomDictionaryStore.getState();
+    applyRemoteDictionarySettings({ providerOrder: ['remote-y'] });
+    expect(mockPublishReplicaUpsert).not.toHaveBeenCalled();
+  });
+});
@@ -292,7 +292,7 @@ async function importStarDictBundle(
  const contentId = await computeDictionaryContentId(ifoFile, stardictFilenames);

  return {
-    id: bundleDir,
+    id: contentId,
    contentId,
    kind: 'stardict',
    name,
@@ -372,7 +372,7 @@ async function importMdictBundle(fs: FileSystem, group: MDictGroup): Promise<Imp
  const contentId = await computeDictionaryContentId(mdxFile, mdictFilenames);

  return {
-    id: bundleDir,
+    id: contentId,
    contentId,
    kind: 'mdict',
    name,
@@ -448,7 +448,7 @@ async function importDictBundle(fs: FileSystem, group: DictGroup): Promise<Impor
  const contentId = await computeDictionaryContentId(dictFile, dictFilenames);

  return {
-    id: bundleDir,
+    id: contentId,
    contentId,
    kind: 'dict',
    name,
@@ -494,7 +494,7 @@ async function importSlobBundle(fs: FileSystem, group: SlobGroup): Promise<Impor
  const contentId = await computeDictionaryContentId(slobFile, [group.slob.name]);

  return {
-    id: bundleDir,
+    id: contentId,
    contentId,
    kind: 'slob',
    name,
@@ -26,7 +26,9 @@ export const SETTINGS_REPLICA_ID = 'singleton';
 *     across devices.
 *   * Collection settings already synced via dedicated kinds
 *     (`customFonts`, `customTextures`, `customDictionaries`,
- *     `opdsCatalogs`, `dictionarySettings`).
+ *     `opdsCatalogs`). Note: `dictionarySettings` sub-fields
+ *     (providerOrder / providerEnabled / webSearches) ARE bundled
+ *     here — see entries below.
 */
 export const SETTINGS_WHITELIST = [
  'globalReadSettings.customThemes',
@@ -34,6 +36,13 @@ export const SETTINGS_WHITELIST = [
  'globalReadSettings.userHighlightColors',
  'globalReadSettings.defaultHighlightLabels',
  'globalReadSettings.customTtsHighlightColors',
+  // Dictionary preferences. Whole-field LWW — concurrent edits on
+  // different devices may lose one side, but in practice users don't
+  // edit these on two devices at once. `defaultProviderId` is
+  // deliberately excluded: it's the last-used tab, per-device state.
+  'dictionarySettings.providerOrder',
+  'dictionarySettings.providerEnabled',
+  'dictionarySettings.webSearches',
  // External integrations. Server URL + identifiers sync as plaintext;
  // the credential fields are listed in `encryptedFields` below so the
  // publish/pull middleware wraps them in cipher envelopes.
@@ -99,7 +99,9 @@ export const buildLocalDictFromRow = (
  if (!fields.name || !fields.kind) return null;

  const dict: ImportedDictionary = {
-    id: bundleDir,
+    // dict.id is the cross-device-stable contentId (= replica_id).
+    // bundleDir stays as the device-local on-disk path.
+    id: row.replica_id,
    contentId: row.replica_id,
    kind: fields.kind,
    name: fields.name,
@@ -38,6 +38,7 @@ import {
 } from '@/services/sync/adapters/settings';
 import { cryptoSession } from '@/libs/crypto/session';
 import { ensurePassphraseUnlocked } from '@/services/sync/passphraseGate';
+import { useCustomDictionaryStore } from '@/store/customDictionaryStore';

 const ENCRYPTED_PATHS: ReadonlySet<string> = new Set(SETTINGS_ENCRYPTED_FIELDS);

@@ -274,6 +275,16 @@ export const applyRemoteSettings = (
  const merged: SystemSettings = mergeSettings(settings, record.patch);
  setSettings(merged);
  saveSettings(envConfig, merged);
+
+  // Mirror dictionarySettings into the customDictionaryStore so the
+  // dictionary panel + reader popup re-render with the remote values
+  // immediately. Without this, those views read from the store's own
+  // cache (only refreshed on `loadCustomDictionaries` mount).
+  if (record.patch.dictionarySettings) {
+    useCustomDictionaryStore
+      .getState()
+      .applyRemoteDictionarySettings(record.patch.dictionarySettings);
+  }
 };

 const mergeSettings = (current: SystemSettings, patch: Partial<SystemSettings>): SystemSettings => {
@@ -297,6 +308,12 @@ const mergeSettings = (current: SystemSettings, patch: Partial<SystemSettings>):
  if (patch.hardcover) {
    out.hardcover = { ...current.hardcover, ...patch.hardcover };
  }
+  if (patch.dictionarySettings) {
+    // `defaultProviderId` (last-used tab) is per-device — not in the
+    // whitelist, so the remote patch never sends it. Spread-with-current
+    // preserves it when the remote updates the synced sub-fields.
+    out.dictionarySettings = { ...current.dictionarySettings, ...patch.dictionarySettings };
+  }
  return out;
 };

@@ -114,6 +114,14 @@ interface DictionaryStoreState {
  /** Soft-delete a custom web search and remove from order/enabled. */
  removeWebSearch(id: string): boolean;

+  /**
+   * Mirror an inbound dictionarySettings patch from the bundled
+   * `settings` replica into the in-memory store so the dictionary
+   * panel and the reader popup pick up the change without a reload.
+   * Pull-side only — no publish, no save.
+   */
+  applyRemoteDictionarySettings(patch: Partial<DictionarySettings>): void;
+
  /** Hydrate from `settings.customDictionaries` + `settings.dictionarySettings` + check on-disk availability. */
  loadCustomDictionaries(envConfig: EnvConfigType): Promise<void>;
  /** Persist current state back into settings (which then syncs to cloud). */
@@ -436,6 +444,12 @@ export const useCustomDictionaryStore = create<DictionaryStoreState>((set, get)
    return true;
  },

+  applyRemoteDictionarySettings: (patch) => {
+    set((state) => ({
+      settings: { ...state.settings, ...patch },
+    }));
+  },
+
  loadCustomDictionaries: async (envConfig) => {
    try {
      const { settings } = useSettingsStore.getState();
@@ -449,6 +463,7 @@ export const useCustomDictionaryStore = create<DictionaryStoreState>((set, get)
          return exists ? dict : { ...dict, unavailable: true };
        }),
      );
+
      // Merge defaults to back-fill any missing keys (e.g. new builtin added in a release).
      // For providerOrder, we append any newly-defaulted ids (like the
      // built-in web searches added in this release) so existing users see
@@ -480,10 +495,17 @@ export const useCustomDictionaryStore = create<DictionaryStoreState>((set, get)
    try {
      const { settings, setSettings, saveSettings } = useSettingsStore.getState();
      const { dictionaries, settings: dictSettings } = get();
-      settings.customDictionaries = dictionaries.map(toSettingsDict);
-      settings.dictionarySettings = dictSettings;
-      setSettings(settings);
-      saveSettings(envConfig, settings);
+      // Build a NEW settings object — Zustand subscribers (notably
+      // replicaSettingsSync.initSettingsSync) compare references to
+      // detect changes, so mutating the existing object in place
+      // bypasses the bundled-settings publish path entirely.
+      const next = {
+        ...settings,
+        customDictionaries: dictionaries.map(toSettingsDict),
+        dictionarySettings: dictSettings,
+      };
+      setSettings(next);
+      saveSettings(envConfig, next);
    } catch (error) {
      console.error('Failed to save custom dictionaries settings:', error);
      throw error;