diff --git a/apps/readest-app/.claude/plans/vivid-orbiting-thimble.md b/apps/readest-app/.claude/plans/vivid-orbiting-thimble.md index 94ddb2ee..3df8f514 100644 --- a/apps/readest-app/.claude/plans/vivid-orbiting-thimble.md +++ b/apps/readest-app/.claude/plans/vivid-orbiting-thimble.md @@ -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, diff --git a/apps/readest-app/src/__tests__/services/sync/adapters/settings.test.ts b/apps/readest-app/src/__tests__/services/sync/adapters/settings.test.ts index 2bdcf5b2..6bdef6ab 100644 --- a/apps/readest-app/src/__tests__/services/sync/adapters/settings.test.ts +++ b/apps/readest-app/src/__tests__/services/sync/adapters/settings.test.ts @@ -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', () => { diff --git a/apps/readest-app/src/__tests__/services/sync/replicaDictionaryApply.test.ts b/apps/readest-app/src/__tests__/services/sync/replicaDictionaryApply.test.ts index a67d89db..5701f5bc 100644 --- a/apps/readest-app/src/__tests__/services/sync/replicaDictionaryApply.test.ts +++ b/apps/readest-app/src/__tests__/services/sync/replicaDictionaryApply.test.ts @@ -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'); diff --git a/apps/readest-app/src/__tests__/services/sync/replicaSettingsSync.test.ts b/apps/readest-app/src/__tests__/services/sync/replicaSettingsSync.test.ts index 55c355e2..f57eb450 100644 --- a/apps/readest-app/src/__tests__/services/sync/replicaSettingsSync.test.ts +++ b/apps/readest-app/src/__tests__/services/sync/replicaSettingsSync.test.ts @@ -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, + }); + 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), + }); + 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, + }); + 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'); + }); }); diff --git a/apps/readest-app/src/__tests__/store/custom-dictionary-store.test.ts b/apps/readest-app/src/__tests__/store/custom-dictionary-store.test.ts index 88fbd0b9..a8abde47 100644 --- a/apps/readest-app/src/__tests__/store/custom-dictionary-store.test.ts +++ b/apps/readest-app/src/__tests__/store/custom-dictionary-store.test.ts @@ -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; + 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(); + }); +}); diff --git a/apps/readest-app/src/services/dictionaries/dictionaryService.ts b/apps/readest-app/src/services/dictionaries/dictionaryService.ts index 0eb0c553..77db43b5 100644 --- a/apps/readest-app/src/services/dictionaries/dictionaryService.ts +++ b/apps/readest-app/src/services/dictionaries/dictionaryService.ts @@ -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 = 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 => { @@ -297,6 +308,12 @@ const mergeSettings = (current: SystemSettings, patch: Partial): 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; }; diff --git a/apps/readest-app/src/store/customDictionaryStore.ts b/apps/readest-app/src/store/customDictionaryStore.ts index 931d7e5d..db045d3a 100644 --- a/apps/readest-app/src/store/customDictionaryStore.ts +++ b/apps/readest-app/src/store/customDictionaryStore.ts @@ -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): void; + /** Hydrate from `settings.customDictionaries` + `settings.dictionarySettings` + check on-disk availability. */ loadCustomDictionaries(envConfig: EnvConfigType): Promise; /** Persist current state back into settings (which then syncs to cloud). */ @@ -436,6 +444,12 @@ export const useCustomDictionaryStore = create((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((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((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;