diff --git a/docs/analysis/2026-09-21-notification-preference-order.md b/docs/analysis/2026-09-21-notification-preference-order.md new file mode 100644 index 000000000..1a18d13d0 --- /dev/null +++ b/docs/analysis/2026-09-21-notification-preference-order.md @@ -0,0 +1,39 @@ +# Notification preference mutation ordering + +Status: stacked draft PR #3343, 2026-09-21. Parent: PR #3340. + +## Reproduced defect + +Every `updatePreferences` call previously started transport immediately. The API carries no expected revision and the persisted notification-preference row has no configured concurrency token. Two saves from one client could therefore commit or settle in an order different from the user’s submissions. + +A queued correction also needs explicit error ownership. An independent inbox read may fail while the next preference save waits; starting that queued save must not erase the unrelated inbox receipt. + +## Contract + +- One preference-mutation lane preserves this client’s submission order. +- The first save starts transport synchronously. Later saves wait for their predecessor, regardless of success or failure. +- Every queued save owns a loading token from submission through settlement. +- Immediately before transport, queued work rechecks the credential epoch inherited from #3340. Token, identity, authentication or demo replacement clears queue registration and prevents old intent from running with later credentials. +- Error receipts carry the operation token that produced them. Queued start retires only its own predecessor’s receipt; an independent inbox failure remains visible. +- Successful saves retain #3340’s preference-read invalidation. + +This preserves one client’s order only. It does not claim cross-device concurrency safety or add a server-side revision precondition. + +## Test-first evidence + +Test-only child head: `4f89c6f2969d5dbad923841b4984ef152daef824`. + +A supplemental runner transpiled and executed the actual parent and corrected production stores with framework/API/session boundaries stubbed: + +- parent #3340: **1/5 passed**; only the existing loading-token control passed; +- corrected child: **5/5 passed**. + +The four parent failures demonstrated eager second transport, failed-predecessor overlap, queued old-credential transport, and lack of a real queued boundary for independent-error preservation. + +The committed Pinia suite covers immediate first transport, serialization, failed-predecessor continuation, token replacement, queued loading, and preservation of an independent inbox failure. + +## Verification and remaining gates + +The corrected module transpiles under TypeScript 5.8.3 with zero diagnostics. Exact-head Pinia/Vitest, lint, project typecheck, build, full hosted CI and independent review remain required. After #3340 lands, retarget to current `main`, verify the child-only diff and requalify. + +No merge, release or deployment qualification is claimed by this note. diff --git a/frontend/taskdeck-web/src/store/notificationStore.ts b/frontend/taskdeck-web/src/store/notificationStore.ts index 5fd3296d1..47a0c3c0c 100644 --- a/frontend/taskdeck-web/src/store/notificationStore.ts +++ b/frontend/taskdeck-web/src/store/notificationStore.ts @@ -33,9 +33,16 @@ export const useNotificationStore = defineStore('notifications', () => { observedMutationGeneration: number } + interface PreferenceMutationTail { + promise: Promise + ownerToken: symbol + } + let sessionEpoch = 0 let notificationMutationGeneration = 0 let preferenceMutationGeneration = 0 + let errorOwner: symbol | null = null + let preferenceMutationTail: PreferenceMutationTail | null = null const activeLoadingOperations = new Set() const readOwners = new Map() @@ -45,6 +52,12 @@ export const useNotificationStore = defineStore('notifications', () => { function clearError(): void { error.value = null + errorOwner = null + } + + function recordError(owner: OperationOwner, message: string): void { + error.value = message + errorOwner = owner.token } function beginOperation( @@ -133,6 +146,7 @@ export const useNotificationStore = defineStore('notifications', () => { preferenceMutationGeneration = 0 activeLoadingOperations.clear() readOwners.clear() + preferenceMutationTail = null notifications.value = [] preferences.value = null loading.value = false @@ -168,7 +182,7 @@ export const useNotificationStore = defineStore('notifications', () => { } catch (e: unknown) { if (ownsRead('notifications', owner)) { const msg = getErrorDisplay(e, 'Failed to load notifications').message - error.value = msg + recordError(owner, msg) toast.error(msg) } throw e @@ -192,7 +206,7 @@ export const useNotificationStore = defineStore('notifications', () => { } catch (e: unknown) { if (ownsSession(owner)) { const msg = getErrorDisplay(e, 'Failed to mark notification as read').message - error.value = msg + recordError(owner, msg) toast.error(msg) } throw e @@ -219,7 +233,7 @@ export const useNotificationStore = defineStore('notifications', () => { } catch (e: unknown) { if (ownsSession(owner)) { const msg = getErrorDisplay(e, 'Failed to mark all notifications as read').message - error.value = msg + recordError(owner, msg) toast.error(msg) } throw e @@ -242,7 +256,7 @@ export const useNotificationStore = defineStore('notifications', () => { } catch (e: unknown) { if (ownsRead('preferences', owner)) { const msg = getErrorDisplay(e, 'Failed to load notification preferences').message - error.value = msg + recordError(owner, msg) toast.error(msg) } throw e @@ -253,27 +267,41 @@ export const useNotificationStore = defineStore('notifications', () => { async function updatePreferences(dto: UpdateNotificationPreferenceRequest) { guardDemoMutation() + const predecessor = preferenceMutationTail const owner = beginOperation('update-preferences', { ownsLoading: true, - clearExistingError: true, + clearExistingError: predecessor === null, }) - try { - const updated = await notificationsApi.updatePreferences(dto) - if (!ownsSession(owner)) return updated + let release!: () => void + const tail = new Promise((resolve) => { release = resolve }) + preferenceMutationTail = { promise: tail, ownerToken: owner.token } - recordPreferenceMutation() - preferences.value = updated - toast.success('Notification preferences saved') - return updated - } catch (e: unknown) { - if (ownsSession(owner)) { - const msg = getErrorDisplay(e, 'Failed to save notification preferences').message - error.value = msg - toast.error(msg) + try { + if (predecessor) await predecessor.promise + if (!ownsSession(owner)) return undefined + + if (predecessor && errorOwner === predecessor.ownerToken) clearError() + + try { + const updated = await notificationsApi.updatePreferences(dto) + if (!ownsSession(owner)) return updated + + recordPreferenceMutation() + preferences.value = updated + toast.success('Notification preferences saved') + return updated + } catch (e: unknown) { + if (ownsSession(owner)) { + const msg = getErrorDisplay(e, 'Failed to save notification preferences').message + recordError(owner, msg) + toast.error(msg) + } + throw e } - throw e } finally { finishOperation(owner) + release() + if (preferenceMutationTail?.promise === tail) preferenceMutationTail = null } } diff --git a/frontend/taskdeck-web/src/tests/store/notificationPreferenceMutationOrder.spec.ts b/frontend/taskdeck-web/src/tests/store/notificationPreferenceMutationOrder.spec.ts new file mode 100644 index 000000000..4bfe99b80 --- /dev/null +++ b/frontend/taskdeck-web/src/tests/store/notificationPreferenceMutationOrder.spec.ts @@ -0,0 +1,232 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' +import { notificationsApi } from '../../api/notificationsApi' +import { useNotificationStore } from '../../store/notificationStore' +import { useSessionStore } from '../../store/sessionStore' +import type { + NotificationItem, + NotificationPreference, + UpdateNotificationPreferenceRequest, +} from '../../types/notifications' + +const toastMocks = vi.hoisted(() => ({ + error: vi.fn(), + success: vi.fn(), + info: vi.fn(), + warning: vi.fn(), +})) + +vi.mock('../../utils/demoMode', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, isDemoMode: false } +}) + +vi.mock('../../api/notificationsApi', () => ({ + notificationsApi: { + getNotifications: vi.fn(), + markAsRead: vi.fn(), + markAllRead: vi.fn(), + getPreferences: vi.fn(), + updatePreferences: vi.fn(), + }, +})) + +vi.mock('../../api/authApi', () => ({ + authApi: { + login: vi.fn(), + register: vi.fn(), + changePassword: vi.fn(), + refreshToken: vi.fn(), + exchangeOAuthCode: vi.fn(), + exchangeOidcCode: vi.fn(), + }, +})) + +vi.mock('../../store/toastStore', () => ({ + useToastStore: () => toastMocks, +})) + +vi.mock('../../composables/useErrorMapper', () => ({ + getErrorDisplay: (error: unknown, fallback: string) => ({ + message: error instanceof Error ? error.message : fallback, + }), +})) + +function deferred() { + let resolve!: (value: T) => void + let reject!: (reason: unknown) => void + const promise = new Promise((yes, no) => { + resolve = yes + reject = no + }) + return { promise, resolve, reject } +} + +function token(suffix: string): string { + const body = btoa(JSON.stringify({ exp: 1893456000 })) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/g, '') + return `header.${body}.${suffix}` +} + +function preferences(enabled: boolean): NotificationPreference { + return { + userId: 'user-a', + inAppChannelEnabled: true, + mentionImmediateEnabled: enabled, + mentionDigestEnabled: !enabled, + assignmentImmediateEnabled: true, + assignmentDigestEnabled: false, + proposalOutcomeImmediateEnabled: true, + proposalOutcomeDigestEnabled: false, + createdAt: '2026-09-21T00:00:00Z', + updatedAt: '2026-09-21T00:00:01Z', + } +} + +function request(enabled: boolean): UpdateNotificationPreferenceRequest { + const value = preferences(enabled) + return { + inAppChannelEnabled: value.inAppChannelEnabled, + mentionImmediateEnabled: value.mentionImmediateEnabled, + mentionDigestEnabled: value.mentionDigestEnabled, + assignmentImmediateEnabled: value.assignmentImmediateEnabled, + assignmentDigestEnabled: value.assignmentDigestEnabled, + proposalOutcomeImmediateEnabled: value.proposalOutcomeImmediateEnabled, + proposalOutcomeDigestEnabled: value.proposalOutcomeDigestEnabled, + } +} + +async function flushQueue() { + await Promise.resolve() + await Promise.resolve() +} + +describe('notification preference mutation ordering', () => { + let session: ReturnType + let store: ReturnType + + beforeEach(() => { + setActivePinia(createPinia()) + session = useSessionStore() + session.userId = 'user-a' + session.token = token('old') + store = useNotificationStore() + vi.clearAllMocks() + }) + + it('starts the first save immediately and serializes the second intent', async () => { + const first = deferred() + const second = deferred() + vi.mocked(notificationsApi.updatePreferences) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updatePreferences(request(true)) + const secondRequest = store.updatePreferences(request(false)) + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(1) + + first.resolve(preferences(true)) + await firstRequest + await flushQueue() + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(2) + + second.resolve(preferences(false)) + await secondRequest + expect(store.preferences?.mentionImmediateEnabled).toBe(false) + }) + + it('continues with the queued save after its predecessor fails', async () => { + const first = deferred() + const second = deferred() + vi.mocked(notificationsApi.updatePreferences) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updatePreferences(request(true)) + const secondRequest = store.updatePreferences(request(false)) + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(1) + + first.reject(new Error('first save failed')) + await expect(firstRequest).rejects.toThrow('first save failed') + await flushQueue() + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(2) + + second.resolve(preferences(false)) + await secondRequest + expect(store.preferences?.mentionImmediateEnabled).toBe(false) + }) + + it('does not start queued old-credential intent after token rotation', async () => { + const first = deferred() + const second = deferred() + vi.mocked(notificationsApi.updatePreferences) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updatePreferences(request(true)) + const secondRequest = store.updatePreferences(request(false)) + const callsBeforeRotation = vi.mocked(notificationsApi.updatePreferences).mock.calls.length + + session.token = token('new') + first.resolve(preferences(true)) + second.resolve(preferences(false)) + await Promise.all([firstRequest, secondRequest]) + + expect(callsBeforeRotation).toBe(1) + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(1) + expect(store.preferences).toBeNull() + expect(store.loading).toBe(false) + }) + + it('keeps loading true from queued submission through final settlement', async () => { + const first = deferred() + const second = deferred() + vi.mocked(notificationsApi.updatePreferences) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + + const firstRequest = store.updatePreferences(request(true)) + const secondRequest = store.updatePreferences(request(false)) + expect(store.loading).toBe(true) + + first.resolve(preferences(true)) + await firstRequest + await flushQueue() + expect(store.loading).toBe(true) + + second.resolve(preferences(false)) + await secondRequest + expect(store.loading).toBe(false) + }) + + it('does not erase an inbox failure when queued preference work starts', async () => { + const first = deferred() + const second = deferred() + const inbox = deferred() + vi.mocked(notificationsApi.updatePreferences) + .mockReturnValueOnce(first.promise) + .mockReturnValueOnce(second.promise) + vi.mocked(notificationsApi.getNotifications).mockReturnValue(inbox.promise) + + const firstRequest = store.updatePreferences(request(true)) + const secondRequest = store.updatePreferences(request(false)) + const inboxRequest = store.fetchNotifications() + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(1) + + inbox.reject(new Error('inbox failed')) + await expect(inboxRequest).rejects.toThrow('inbox failed') + expect(store.error).toBe('inbox failed') + + first.resolve(preferences(true)) + await firstRequest + await flushQueue() + expect(notificationsApi.updatePreferences).toHaveBeenCalledTimes(2) + expect(store.error).toBe('inbox failed') + + second.resolve(preferences(false)) + await secondRequest + expect(store.error).toBe('inbox failed') + }) +})