diff --git a/app/api/settings/route.ts b/app/api/settings/route.ts index d3ff0a0e5..265f7bc9d 100644 --- a/app/api/settings/route.ts +++ b/app/api/settings/route.ts @@ -9,6 +9,7 @@ import { saveUserSettings, loadUserSettings, deleteUserSettings } from '@/lib/se import { configManager } from '@/lib/admin/config-manager'; import { hasSessionSecret } from '@/lib/auth/session-secret'; import { MAX_ACCOUNT_SLOTS } from '@/lib/account-utils'; +import { mergeStoredTemplates } from '@/lib/template-merge'; function classifyError(error: unknown): { message: string; status: number } { const code = (error as NodeJS.ErrnoException).code; @@ -176,7 +177,10 @@ export async function POST(request: NextRequest) { } } - await saveUserSettings(accountName, serverUrl, filteredSettings); + // Templates are merged into the stored blob rather than replaced, so a + // stale tab or device cannot drop templates another one added. + const stored = await loadUserSettings(accountName, serverUrl); + await saveUserSettings(accountName, serverUrl, mergeStoredTemplates(stored, filteredSettings)); return NextResponse.json({ ok: true }); } catch (error) { const message = error instanceof Error ? error.message : 'Unknown error'; diff --git a/lib/__tests__/settings-route-template-merge.test.ts b/lib/__tests__/settings-route-template-merge.test.ts new file mode 100644 index 000000000..746dd01da --- /dev/null +++ b/lib/__tests__/settings-route-template-merge.test.ts @@ -0,0 +1,119 @@ +// @vitest-environment node +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { NextRequest } from 'next/server'; + +// Every push carries the pushing tab's whole template list. The route used to +// store it as is, so a tab or device that had not loaded since another one +// added a template wiped that template for everyone. Templates are now merged +// into the stored blob. + +vi.mock('@/lib/auth/session-secret', () => ({ + getSessionSecret: () => 's'.repeat(64), + hasSessionSecret: () => true, +})); +vi.mock('@/lib/logger', () => ({ + logger: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} }, +})); +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { + ensureLoaded: async () => {}, + get: (key: string, fallback: unknown) => (key === 'settingsSyncEnabled' ? true : fallback), + getPolicy: () => ({ restrictions: {} }), + }, +})); + +const jar = new Map(); +vi.mock('next/headers', () => ({ + cookies: async () => ({ + get: (name: string) => (jar.has(name) ? { name, value: jar.get(name)! } : undefined), + set: (name: string, value: string) => { jar.set(name, value); }, + delete: (name: string) => { jar.delete(name); }, + getAll: () => [...jar].map(([name, value]) => ({ name, value })), + }), +})); + +const store = new Map>(); +vi.mock('@/lib/settings-sync', () => ({ + loadUserSettings: async (username: string, serverUrl: string) => store.get(`${username}|${serverUrl}`) ?? null, + saveUserSettings: async (username: string, serverUrl: string, settings: Record) => { + store.set(`${username}|${serverUrl}`, settings); + }, + deleteUserSettings: async () => {}, +})); + +const SERVER = 'https://mail.example.org'; +const USER = 'alice@example.org'; +const KEY = `${USER}|${SERVER}`; + +function template(id: string, updatedAt = '2026-10-01T00:00:00.000Z') { + return { id, name: id, subject: '', body: '', isHTML: false, category: '', isFavorite: false, createdAt: updatedAt, updatedAt }; +} + +async function push(settings: Record) { + const { POST } = await import('@/app/api/settings/route'); + const res = await POST(new NextRequest('https://webmail.example/api/settings', { + method: 'POST', + headers: { 'sec-fetch-site': 'same-origin', 'content-type': 'application/json' }, + body: JSON.stringify({ username: USER, serverUrl: SERVER, settings }), + })); + expect(res.status).toBe(200); + return store.get(KEY)!; +} + +function ids(saved: Record) { + return (saved.templates as { id: string }[]).map((t) => t.id).sort(); +} + +beforeEach(async () => { + store.clear(); + jar.clear(); + process.env.SETTINGS_SYNC_ENABLED = 'true'; + const { setStalwartAuthContextInStore } = await import('@/lib/stalwart/auth-context'); + setStalwartAuthContextInStore({ + get: (n: string) => (jar.has(n) ? { name: n, value: jar.get(n)! } : undefined), + set: (n: string, v: string) => { jar.set(n, v); }, + delete: (n: string) => { jar.delete(n); }, + } as never, 0, { serverUrl: SERVER, authHeader: 'Bearer t', username: USER }); +}); + +describe('settings route merges templates into the stored blob', () => { + it('keeps a template another device added when a stale client pushes', async () => { + store.set(KEY, { theme: 'light', templates: [template('a'), template('b')], deletedTemplateIds: {} }); + + const saved = await push({ theme: 'dark', templates: [template('a')], deletedTemplateIds: {} }); + + expect(saved.theme).toBe('dark'); + expect(ids(saved)).toEqual(['a', 'b']); + }); + + it('still removes a template the pushing client deleted', async () => { + store.set(KEY, { templates: [template('a'), template('b')], deletedTemplateIds: {} }); + + const saved = await push({ templates: [template('a')], deletedTemplateIds: { b: new Date().toISOString() } }); + + expect(ids(saved)).toEqual(['a']); + expect(Object.keys(saved.deletedTemplateIds as object)).toEqual(['b']); + }); + + it('takes the newer edit of the same template', async () => { + store.set(KEY, { templates: [{ ...template('a', '2026-10-02T00:00:00.000Z'), name: 'newer' }], deletedTemplateIds: {} }); + + const saved = await push({ templates: [{ ...template('a'), name: 'older' }], deletedTemplateIds: {} }); + + expect((saved.templates as { name: string }[])[0].name).toBe('newer'); + }); + + it('keeps stored templates when a push carries none', async () => { + store.set(KEY, { templates: [template('a')], deletedTemplateIds: {} }); + + const saved = await push({ theme: 'dark' }); + + expect(ids(saved)).toEqual(['a']); + }); + + it('stores the first push as is', async () => { + const saved = await push({ templates: [template('a')], deletedTemplateIds: {} }); + + expect(ids(saved)).toEqual(['a']); + }); +}); diff --git a/lib/template-merge.ts b/lib/template-merge.ts new file mode 100644 index 000000000..1377c34aa --- /dev/null +++ b/lib/template-merge.ts @@ -0,0 +1,118 @@ +import type { EmailTemplate } from './template-types'; + +/** + * Tombstones older than this are pruned from the synced blob. A device that + * stays offline longer than this may resurrect a deletion - the accepted + * trade-off for keeping the blob bounded. + */ +export const TEMPLATE_TOMBSTONE_TTL_MS = 90 * 24 * 60 * 60 * 1000; + +export interface SyncedTemplateState { + templates: T[]; + /** Template id -> ISO time it was deleted. */ + deletedTemplateIds: Record; +} + +/** + * Merges a synced template state into the local one. Per template id the + * newer `updatedAt` wins (ties keep the local copy); a deletion tombstone + * beats a template unless the template was edited after the deletion, which + * resurrects it and clears the tombstone. Merging (rather than replacing) + * keeps a stale per-account blob from wiping templates created under another + * account or on another device. + */ +export function mergeSyncedTemplates( + local: SyncedTemplateState, + incoming: SyncedTemplateState, + now: Date = new Date() +): SyncedTemplateState { + const cutoff = now.getTime() - TEMPLATE_TOMBSTONE_TTL_MS; + + // Union of tombstones, newest deletion time per id, pruned by TTL. + const tombstones: Record = {}; + for (const source of [local.deletedTemplateIds, incoming.deletedTemplateIds]) { + for (const [id, deletedAt] of Object.entries(source)) { + if (Date.parse(deletedAt) < cutoff) continue; + if (!tombstones[id] || Date.parse(deletedAt) > Date.parse(tombstones[id])) { + tombstones[id] = deletedAt; + } + } + } + + const byId = new Map(); + for (const t of local.templates) byId.set(t.id, t); + for (const t of incoming.templates) { + const existing = byId.get(t.id); + if (!existing || Date.parse(t.updatedAt) > Date.parse(existing.updatedAt)) { + byId.set(t.id, t); + } + } + + const templates: T[] = []; + for (const t of byId.values()) { + const deletedAt = tombstones[t.id]; + if (deletedAt !== undefined) { + if (Date.parse(t.updatedAt) <= Date.parse(deletedAt)) continue; + delete tombstones[t.id]; + } + templates.push(t); + } + + return { templates, deletedTemplateIds: tombstones }; +} + +type RawTemplate = Record & { id: string; updatedAt: string }; + +function rawTemplates(value: unknown): RawTemplate[] | null { + if (!Array.isArray(value)) return null; + return value.filter( + (t): t is RawTemplate => + typeof t === 'object' && t !== null && + typeof (t as Record).id === 'string' && + typeof (t as Record).updatedAt === 'string' + ); +} + +function rawTombstones(value: unknown): Record { + if (typeof value !== 'object' || value === null || Array.isArray(value)) return {}; + const out: Record = {}; + for (const [id, deletedAt] of Object.entries(value as Record)) { + if (typeof deletedAt === 'string' && !Number.isNaN(Date.parse(deletedAt))) out[id] = deletedAt; + } + return out; +} + +/** + * Server-side counterpart of the client merge, applied when a settings blob + * is saved. A push carries the pushing tab's whole template list, so without + * this a tab or device that has not loaded since another one added a template + * overwrites the stored blob and the template is gone for every device that + * loads afterwards. A push without a `templates` array (a client whose + * template store had not registered yet, or an older build) keeps the stored + * templates instead of dropping them. + * + * Returns the blob to store; the template entries are passed through as they + * are, clients validate them when they apply a blob. + */ +export function mergeStoredTemplates( + stored: Record | null, + incoming: Record +): Record { + const storedTemplates = stored ? rawTemplates(stored.templates) : null; + if (!storedTemplates) return incoming; + + const incomingTemplates = rawTemplates(incoming.templates); + if (!incomingTemplates) { + return { + ...incoming, + templates: stored!.templates, + deletedTemplateIds: stored!.deletedTemplateIds ?? {}, + }; + } + + const merged = mergeSyncedTemplates( + { templates: storedTemplates, deletedTemplateIds: rawTombstones(stored!.deletedTemplateIds) }, + { templates: incomingTemplates, deletedTemplateIds: rawTombstones(incoming.deletedTemplateIds) } + ); + return { ...incoming, ...merged }; +} diff --git a/lib/template-utils.ts b/lib/template-utils.ts index 114fa84b2..f509c488e 100644 --- a/lib/template-utils.ts +++ b/lib/template-utils.ts @@ -166,18 +166,13 @@ export interface ImportResult { // below, synced templates keep their ids so the same template converges to a // single entry across devices instead of duplicating on every server load. -/** - * Tombstones older than this are pruned from the synced blob. A device that - * stays offline longer than this may resurrect a deletion - the accepted - * trade-off for keeping the blob bounded. - */ -export const TEMPLATE_TOMBSTONE_TTL_MS = 90 * 24 * 60 * 60 * 1000; - -export interface SyncedTemplateState { - templates: EmailTemplate[]; - /** Template id -> ISO time it was deleted. */ - deletedTemplateIds: Record; -} +// The merge itself lives in template-merge so the settings API route can use +// it without pulling DOMPurify into the server bundle. +export { + mergeSyncedTemplates, + TEMPLATE_TOMBSTONE_TTL_MS, + type SyncedTemplateState, +} from './template-merge'; /** * Structurally validates a `templates` value from a synced settings blob or @@ -235,54 +230,6 @@ export function parseTemplateTombstones(value: unknown): Record return out; } -/** - * Merges a synced template state into the local one. Per template id the - * newer `updatedAt` wins (ties keep the local copy); a deletion tombstone - * beats a template unless the template was edited after the deletion, which - * resurrects it and clears the tombstone. Merging (rather than replacing) - * keeps a stale per-account blob from wiping templates created under another - * account or on another device. - */ -export function mergeSyncedTemplates( - local: SyncedTemplateState, - incoming: SyncedTemplateState, - now: Date = new Date() -): SyncedTemplateState { - const cutoff = now.getTime() - TEMPLATE_TOMBSTONE_TTL_MS; - - // Union of tombstones, newest deletion time per id, pruned by TTL. - const tombstones: Record = {}; - for (const source of [local.deletedTemplateIds, incoming.deletedTemplateIds]) { - for (const [id, deletedAt] of Object.entries(source)) { - if (Date.parse(deletedAt) < cutoff) continue; - if (!tombstones[id] || Date.parse(deletedAt) > Date.parse(tombstones[id])) { - tombstones[id] = deletedAt; - } - } - } - - const byId = new Map(); - for (const t of local.templates) byId.set(t.id, t); - for (const t of incoming.templates) { - const existing = byId.get(t.id); - if (!existing || Date.parse(t.updatedAt) > Date.parse(existing.updatedAt)) { - byId.set(t.id, t); - } - } - - const templates: EmailTemplate[] = []; - for (const t of byId.values()) { - const deletedAt = tombstones[t.id]; - if (deletedAt !== undefined) { - if (Date.parse(t.updatedAt) <= Date.parse(deletedAt)) continue; - delete tombstones[t.id]; - } - templates.push(t); - } - - return { templates, deletedTemplateIds: tombstones }; -} - export function importTemplates(json: string): ImportResult { const errors: string[] = []; diff --git a/stores/__tests__/settings-store-sync.test.ts b/stores/__tests__/settings-store-sync.test.ts index 4bfba4472..c4d689ea1 100644 --- a/stores/__tests__/settings-store-sync.test.ts +++ b/stores/__tests__/settings-store-sync.test.ts @@ -185,3 +185,84 @@ describe('template sync (#825)', () => { expect(exported.deletedTemplateIds).toEqual({}); }); }); + +describe('template push after a server load', () => { + const USER = 'a@example.com'; + const SERVER = 'https://mail-a.example.com'; + const remote = { + id: 'remote-1', + name: 'From server', + subject: '', + body: '', + category: '', + isFavorite: false, + createdAt: '2026-01-01T00:00:00Z', + updatedAt: '2026-01-01T00:00:00Z', + }; + + function serverReturns(settings: Record | null) { + apiFetch.mockImplementation(async (_url: string, init?: RequestInit) => + new Response(JSON.stringify(init?.method === 'POST' ? { ok: true } : { settings }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + ); + } + + function addLocal() { + return useTemplateStore.getState().addTemplate({ + name: 'Local only', subject: '', body: '', category: '', isFavorite: false, + }); + } + + async function login() { + await useSettingsStore.getState().loadFromServer(USER, SERVER); + useSettingsStore.getState().enableSync(USER, SERVER); + await useSettingsStore.getState().flushSync(); + return apiFetch.mock.calls + .map((call) => call[1] as RequestInit | undefined) + .filter((init) => init?.method === 'POST') + .map((init) => JSON.parse(init!.body as string)); + } + + beforeEach(() => { + apiFetch.mockReset(); + useSettingsStore.getState().disableSync(); + useSettingsStore.setState({ settingsSyncDisabled: false }); + useTemplateStore.setState({ templates: [], recentTemplateIds: [], deletedTemplateIds: {} }); + }); + + it('pushes a local template the server blob lacks', async () => { + serverReturns({ templates: [remote], deletedTemplateIds: {} }); + const local = addLocal(); + + const pushes = await login(); + + expect(pushes).toHaveLength(1); + const ids = pushes[0].settings.templates.map((t: { id: string }) => t.id).sort(); + expect(ids).toEqual([local.id, 'remote-1'].sort()); + }); + + it('pushes local templates when the server has no blob yet', async () => { + serverReturns(null); + addLocal(); + + const pushes = await login(); + + expect(pushes).toHaveLength(1); + expect(pushes[0].settings.templates).toHaveLength(1); + }); + + it('does not push when the server blob already matches', async () => { + serverReturns({ templates: [remote], deletedTemplateIds: {} }); + + expect(await login()).toHaveLength(0); + }); + + it('does not push when the user turned sync off', async () => { + serverReturns({ templates: [], deletedTemplateIds: {}, settingsSyncDisabled: true }); + addLocal(); + + expect(await login()).toHaveLength(0); + }); +}); diff --git a/stores/__tests__/template-store-sync.test.ts b/stores/__tests__/template-store-sync.test.ts index b6c7be271..c2712d52c 100644 --- a/stores/__tests__/template-store-sync.test.ts +++ b/stores/__tests__/template-store-sync.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { useTemplateStore } from '../template-store'; import type { EmailTemplate } from '@/lib/template-types'; @@ -91,3 +91,28 @@ describe('applySyncedState', () => { expect(state.deletedTemplateIds).toEqual({ a: deletedAt }); }); }); + +describe('another tab writing templates', () => { + it('reloads the list so this tab does not overwrite the other tab\'s template', async () => { + useTemplateStore.setState({ templates: [makeTemplate({ id: 'mine' })] }); + + // What the other tab's persist middleware writes after adding a template. + const otherTab = JSON.stringify({ + state: { + templates: [makeTemplate({ id: 'mine' }), makeTemplate({ id: 'theirs' })], + recentTemplateIds: [], + deletedTemplateIds: {}, + }, + version: 1, + }); + window.localStorage.setItem('template-storage', otherTab); + window.dispatchEvent(new StorageEvent('storage', { + key: 'template-storage', + newValue: otherTab, + storageArea: window.localStorage, + })); + await vi.waitFor(() => { + expect(useTemplateStore.getState().templates.map((t) => t.id)).toEqual(['mine', 'theirs']); + }); + }); +}); diff --git a/stores/settings-store.ts b/stores/settings-store.ts index 33df6a566..07e49541f 100644 --- a/stores/settings-store.ts +++ b/stores/settings-store.ts @@ -30,6 +30,12 @@ let syncServerUrl: string | null = null; let syncTimeout: ReturnType | null = null; let pendingSync: SettingsSyncJob | null = null; let isLoadingFromServer = false; +// Set by loadFromServer when this browser holds templates (or deletions) the +// server blob lacks; enableSync then pushes once instead of waiting for the +// next unrelated settings change. +let pushTemplatesAfterLoad = false; +// Wired up in the window-only init block below; null during SSR. +let requestSync: (() => void) | null = null; const SYNC_DEBOUNCE_MS = 2000; @@ -69,6 +75,32 @@ export function registerTemplateSyncBridge( subscribe(() => onTemplateStoreChange?.()); } +/** Order-independent fingerprint of a template list plus its tombstones. */ +function templateStateKey(templates: unknown, deletedTemplateIds: unknown): string { + const list = Array.isArray(templates) + ? templates + .filter((t): t is { id: string; updatedAt?: unknown } => + typeof t === 'object' && t !== null && typeof (t as { id?: unknown }).id === 'string') + .map((t) => `${t.id}@${String(t.updatedAt)}`) + .sort() + : []; + const tombstones = isPlainRecord(deletedTemplateIds) ? Object.keys(deletedTemplateIds).sort() : []; + return JSON.stringify([list, tombstones]); +} + +/** + * Whether the local template state differs from what a server blob carries, + * so the blob needs a push. False when there is nothing local to push. + */ +function localTemplatesAhead(serverSettings: Record | null): boolean { + const local = templateSyncBridge?.getSyncedState(); + if (!local) return false; + if (local.templates.length === 0 && Object.keys(local.deletedTemplateIds).length === 0) return false; + if (!serverSettings) return true; + return templateStateKey(local.templates, local.deletedTemplateIds) !== + templateStateKey(serverSettings.templates, serverSettings.deletedTemplateIds); +} + async function syncSettingsJob(job: SettingsSyncJob, retries = 1): Promise { syncLog('Syncing settings to server for', job.username); const res = await apiFetch('/api/settings', { @@ -1147,6 +1179,10 @@ export const useSettingsStore = create()( syncServerUrl = serverUrl; syncEnabled = true; syncLog('Settings sync enabled for', username); + if (pushTemplatesAfterLoad) { + pushTemplatesAfterLoad = false; + if (!get().settingsSyncDisabled) requestSync?.(); + } }, flushSync: async () => { @@ -1174,6 +1210,7 @@ export const useSettingsStore = create()( loadFromServer: async (username: string, serverUrl: string) => { try { syncLog('Loading settings from server for', username); + pushTemplatesAfterLoad = false; const res = await apiFetch('/api/settings', { headers: { 'x-settings-username': username, @@ -1188,6 +1225,7 @@ export const useSettingsStore = create()( const { settings } = await res.json(); if (!settings) { syncLog('No server settings found yet'); + pushTemplatesAfterLoad = localTemplatesAhead(null); return false; } if (settings && typeof settings === 'object') { @@ -1198,6 +1236,7 @@ export const useSettingsStore = create()( serverAccountId: generateAccountId(username, serverUrl), }); isLoadingFromServer = false; + pushTemplatesAfterLoad = localTemplatesAhead(settings); syncLog('Settings loaded from server successfully'); // The per-account preferred sender identity (#507) is re-applied by // applyPreferredIdentity() in auth-store, invoked from the @@ -1407,6 +1446,8 @@ if (typeof window !== 'undefined') { }, SYNC_DEBOUNCE_MS); }; + requestSync = triggerSync; + // Auto-sync settings to server on any state change let prevSyncDisabled = useSettingsStore.getState().settingsSyncDisabled; useSettingsStore.subscribe(() => { diff --git a/stores/template-store.ts b/stores/template-store.ts index 00b60539c..894d0add6 100644 --- a/stores/template-store.ts +++ b/stores/template-store.ts @@ -200,6 +200,17 @@ export const useTemplateStore = create()( ) ); +// Every tab keeps its own in-memory copy and writes the whole list back to +// localStorage on any change, so a second tab that never saw a template +// created in the first one would overwrite it (and push the shorter list). +// Pick up the other tab's write instead. +if (typeof window !== 'undefined') { + window.addEventListener('storage', (event) => { + if (event.key !== 'template-storage' || event.storageArea !== window.localStorage) return; + void useTemplateStore.persist.rehydrate(); + }); +} + // Registers templates with the cross-device settings sync (#825): they are // included in the synced blob / settings export, applied back on server loads // and file imports, and template changes schedule a sync push. Registration