From 55ca12adbde3ef789b7d5bc420f41d11430f4687 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 3 Nov 2026 08:43:35 +0100 Subject: [PATCH] fix: say when a start stays on the local copy, queue its sends, paint it only with folders kept and fill the From list after the swap --- lib/jmap/offline-hooks.ts | 2 + lib/offline/__tests__/review-fixes.test.ts | 23 +++++++ lib/offline/offline-mode.ts | 12 +++- lib/offline/state.ts | 15 +++++ lib/store/__tests__/mail-answers.test.ts | 26 ++++++++ lib/store/mail/calls.ts | 2 + lib/store/mail/methods.ts | 3 + .../__tests__/auth-store-local-copy.test.ts | 66 ++++++++++++++++++- stores/auth-store.ts | 19 +++++- 9 files changed, 160 insertions(+), 8 deletions(-) diff --git a/lib/jmap/offline-hooks.ts b/lib/jmap/offline-hooks.ts index b908eafb7..753b4273d 100644 --- a/lib/jmap/offline-hooks.ts +++ b/lib/jmap/offline-hooks.ts @@ -22,6 +22,8 @@ export interface OfflineClientRef { getServerUrl(): string; getUsername(): string; getAccountId(): string; + /** Answers from this device only: a local copy on screen at a start. */ + isOfflineOnly?(): boolean; } export interface OfflineHooks { diff --git a/lib/offline/__tests__/review-fixes.test.ts b/lib/offline/__tests__/review-fixes.test.ts index c516f61e5..11bd3cfd5 100644 --- a/lib/offline/__tests__/review-fixes.test.ts +++ b/lib/offline/__tests__/review-fixes.test.ts @@ -28,6 +28,7 @@ import { flushQueue, settleStaleSends, SENDING_STALE_MS } from '../flush'; import { claimForSending, holdFile, queueSend, readQueue } from '../queue'; import { forgetLegacyMove } from '../migrate-legacy'; import { consumeLost } from '../lost'; +import { getOutboxHooks } from '../outbox-hooks'; import { shellFiles } from '../shell'; import { isOfflineNow, OFFLINE_ACCOUNTS_KEY, setServerDown, useOfflineStore, writeEnabled } from '../state'; @@ -283,6 +284,28 @@ describe('11: starting from the device', () => { writeEnabled(['acc']); expect(await offlineSessionFor('acc')).toMatchObject({ primaryAccounts: { 'urn:ietf:params:jmap:mail': JMAP_ACCOUNT } }); }); + + it('doesn\'t offer a quick start before the store has its folders', async () => { + store = new InProcessStore(new FakeJmapMail()); + setStoreBackend(store); + writeEnabled(['acc']); + expect(await offlineSessionFor('acc', { withFolders: true })).toBeNull(); + await store.sync('acc'); + expect(await offlineSessionFor('acc', { withFolders: true })).toMatchObject({ primaryAccounts: { 'urn:ietf:params:jmap:mail': JMAP_ACCOUNT } }); + }); +}); + +describe('a send on the local copy', () => { + it('queues, before the swap too, while the probes say the server is up', () => { + writeEnabled(['acc']); + stop = startOfflineMode(); + useAuthStore.setState({ client: fakeClient('https://mail.example.org', { isOfflineOnly: () => true }) as never }); + const client = useAuthStore.getState().client as never; + expect(isOfflineNow('https://mail.example.org')).toBe(false); + expect(getOutboxHooks()?.shouldQueue(client)).toBe(true); + useAuthStore.setState({ client: fakeClient() as never }); + expect(getOutboxHooks()?.shouldQueue(useAuthStore.getState().client as never)).toBe(false); + }); }); describe('12: one server down doesn’t hold another’s mail', () => { diff --git a/lib/offline/offline-mode.ts b/lib/offline/offline-mode.ts index 03b0e140d..4ad4994f8 100644 --- a/lib/offline/offline-mode.ts +++ b/lib/offline/offline-mode.ts @@ -54,7 +54,7 @@ import { type PolicyFingerprint, } from './queue'; import { clearHandoffs } from '@/lib/launch-handoff'; -import { clearEvicted, EVICTED_KEEP_MS, isEvicted, isOfflineNow, OFFLINE_ACCOUNTS_KEY, readEnabledFrom, readEvicted, visibleAccounts, serverKey, setServerDown, useOfflineStore, writeEnabled, type OutboxRow } from './state'; +import { clearEvicted, EVICTED_KEEP_MS, forgetMailSnapshot, isEvicted, isOfflineNow, OFFLINE_ACCOUNTS_KEY, readEnabledFrom, readEvicted, visibleAccounts, serverKey, setServerDown, useOfflineStore, writeEnabled, type OutboxRow } from './state'; import { flushQueue, settleStaleSends } from './flush'; import { LEGACY_DB_NAME, moveLegacyOutbox } from './migrate-legacy'; import { deleteShellCaches, tellServiceWorker } from './shell'; @@ -160,6 +160,7 @@ const storeHooks: StorePageHooks = { onEvent(event) { if (event.type === 'synced') { useOfflineStore.setState((s) => ({ lastSyncedAt: { ...s.lastSyncedAt, [event.account]: event.at } })); + forgetMailSnapshot(event.account); void refreshUsage(); } if (event.type === 'error' && event.area === STORAGE_ERROR_AREA) { @@ -392,6 +393,8 @@ const outbox: OutboxHooks = { shouldQueue(client) { const account = accountForClient(client); if (!account || !offlineActive(account)) return false; + // A local copy has no way to the server, whatever the probes last said. + if (client.isOfflineOnly?.()) return true; return isOfflineNow(client.getServerUrl()) || (typeof navigator !== 'undefined' && navigator.onLine === false); }, async queueEmail(client, args) { @@ -981,7 +984,7 @@ export async function serverUnreachable(serverUrl: string): Promise { const SESSION_READ_TIMEOUT_MS = 5_000; /** The session kept for an account, for starting from the local copy. */ -export async function offlineSessionFor(account: string): Promise { +export async function offlineSessionFor(account: string, options: { withFolders?: boolean } = {}): Promise { // The organisation's last word: the policy kept from the last fetch (the // fetch itself fails without a network), never the defaults. const policy = usePolicyStore.getState(); @@ -992,7 +995,10 @@ export async function offlineSessionFor(account: string): Promise(account, 'mail.session'), + (async () => { + if (options.withFolders && !(await getStore().area(account, 'mail.hasFolders'))) return null; + return getStore().area(account, 'mail.session'); + })(), new Promise((resolve) => setTimeout(() => resolve(null), SESSION_READ_TIMEOUT_MS)), ]); } catch { diff --git a/lib/offline/state.ts b/lib/offline/state.ts index 4508c07e5..5a86ca647 100644 --- a/lib/offline/state.ts +++ b/lib/offline/state.ts @@ -119,6 +119,21 @@ export function visibleAccounts(): string[] { return useOfflineStore.getState().enabled.filter((account) => !(account in evicted)); } +/** + * Drops the plain-text list kept in localStorage for a quick start (email-store) + * once the store can do that job. Until an account's first sync finishes it's + * what stays on screen, so turning the store on alone doesn't remove it. + */ +export function forgetMailSnapshot(account: string): void { + try { + const raw = localStorage.getItem('email-snapshot'); + if (raw && (JSON.parse(raw) as { account?: unknown } | null)?.account !== account) return; + localStorage.removeItem('email-snapshot'); + } catch { + localStorage.removeItem('email-snapshot'); + } +} + export function serverKey(url: string): string { return url.replace(/\/+$/, '').toLowerCase(); } diff --git a/lib/store/__tests__/mail-answers.test.ts b/lib/store/__tests__/mail-answers.test.ts index 90a36a9b8..6461af6f4 100644 --- a/lib/store/__tests__/mail-answers.test.ts +++ b/lib/store/__tests__/mail-answers.test.ts @@ -263,6 +263,32 @@ describe('Email/query filters and sorts', () => { }); }); +describe('a store without its mail yet', () => { + it('says so rather than answering an empty list, until the first sync has finished', async () => { + const server = new FakeJmapMail(); + server.addEmail({ id: 'one' }); + const { engine } = engineFor(server); + const inbox = Object.keys((server.emails.get('one')!.mailboxIds) as Record)[0]; + const query = () => one(engine, ['Email/query', { filter: { inMailbox: inbox } }, 'q']); + // No answer: the client reports it can't tell, and the list on screen stays. + expect(await query()).toBeUndefined(); + await engine.requestSync(); + expect((await query())![1].ids).toEqual(['one']); + // After a finished sync, an empty folder really is empty. + server.destroyEmail('one'); + await engine.requestSync(); + expect((await query())![1].ids).toEqual([]); + }); + + it('tells a start whether there are folders to show', async () => { + const server = new FakeJmapMail(); + const { engine } = engineFor(server); + expect(await engine.areaCall('mail.hasFolders', {})).toBe(false); + await engine.requestSync(); + expect(await engine.areaCall('mail.hasFolders', {})).toBe(true); + }); +}); + describe('keyword case, as measured on both Stalwart versions', () => { it('keeps two keywords differing only in case, and sync doesn\'t stop on them', async () => { const server = new FakeJmapMail(); diff --git a/lib/store/mail/calls.ts b/lib/store/mail/calls.ts index 429b55dd5..5916c7ce6 100644 --- a/lib/store/mail/calls.ts +++ b/lib/store/mail/calls.ts @@ -59,4 +59,6 @@ export const MAIL_CALLS: Readonly> = { 'mail.usage': usage, /** The session kept with the account, for a start from the local copy. */ 'mail.session': (ctx) => ctx.session, + /** Whether a start may show the store: it has a folder list from the server. */ + 'mail.hasFolders': (ctx) => ctx.db.get("SELECT 1 AS yes FROM sync_state WHERE type = 'Mailbox' AND state IS NOT NULL") !== undefined, }; diff --git a/lib/store/mail/methods.ts b/lib/store/mail/methods.ts index 43c8c87a1..8462563e4 100644 --- a/lib/store/mail/methods.ts +++ b/lib/store/mail/methods.ts @@ -317,6 +317,9 @@ const emailQuery: MethodHandler = (ctx, args) => { ).map((row) => String(row.id)); // Stalwart's queryState is the Email state. Absent until the first sync, as allowed. const queryState = lastState(ctx, jmap, 'Email'); + // Nothing here doesn't mean nothing there: an empty folder of a first sync + // that hasn't finished mustn't paint over what's on screen. + if (ids.length === 0 && position === 0 && !queryState) throw new MethodError('serverUnavailable', 'Mail not kept yet'); const result: MethodResult = { accountId: jmap, canCalculateChanges: false, diff --git a/stores/__tests__/auth-store-local-copy.test.ts b/stores/__tests__/auth-store-local-copy.test.ts index 15881cc85..ee514fb01 100644 --- a/stores/__tests__/auth-store-local-copy.test.ts +++ b/stores/__tests__/auth-store-local-copy.test.ts @@ -7,11 +7,15 @@ const offline = vi.hoisted(() => ({ order: [] as string[], session: { apiUrl: 'x' } as unknown, gone: false, + ready: true, + // A local copy with no identities kept answers an empty list. + localIdentities: Promise.resolve([]) as Promise, + releaseLocal: () => {}, })); vi.mock('@/lib/offline/offline-mode', () => ({ registerOfflineHooks: () => { offline.order.push('hooks'); }, - offlineSessionFor: async () => offline.session, + offlineSessionFor: async (_account: string, options?: { withFolders?: boolean }) => (options?.withFolders && !offline.ready ? null : offline.session), serverUnreachable: async () => offline.gone, markStartedOffline: () => { offline.order.push('startedOffline'); }, })); @@ -33,7 +37,9 @@ vi.mock('@/lib/jmap/client', () => { async connect() { offline.order.push('connect'); await connectGate; } getSessionUsername() { return this.username; } getUsername() { return this.username; } - async getIdentities() { return [{ id: 'a', email: this.username, name: this.username, mayDelete: false }]; } + async getIdentities() { + return this.offlineOnly ? offline.localIdentities : [{ id: 'a', email: this.username, name: this.username, mayDelete: false }]; + } getAuthHeader() { return 'Basic eA=='; } onConnectionChange() {} onRateLimit() {} @@ -88,11 +94,13 @@ beforeEach(async () => { offline.order.length = 0; offline.session = { apiUrl: 'x' }; offline.gone = false; + offline.ready = true; + offline.localIdentities = Promise.resolve([]); connectGate = Promise.resolve(); sessionAnswer = goodSession; useAccountStore.setState({ accounts: [ACCOUNT] as never, activeAccountId: ACCOUNT.id, defaultAccountId: ACCOUNT.id }); useOfflineStore.setState({ enabled: [ACCOUNT.id], startedOffline: false }); - useAuthStore.setState({ client: null, isAuthenticated: false, activeAccountId: ACCOUNT.id }); + useAuthStore.setState({ client: null, isAuthenticated: false, activeAccountId: ACCOUNT.id, identities: [], primaryIdentity: null }); }); type ClientLike = { isOfflineOnly(): boolean }; @@ -128,6 +136,23 @@ it('stays on the local copy when the server can\'t be reached', async () => { expect(useAccountStore.getState().getAccountById(ACCOUNT.id)).toBeTruthy(); }); +it('stays on the local copy, and says so, when the server answers a start with a 5xx or 429', async () => { + for (const status of [502, 429]) { + ACCOUNT = makeAccount(); + offline.order.length = 0; + useAccountStore.setState({ accounts: [ACCOUNT] as never, activeAccountId: ACCOUNT.id, defaultAccountId: ACCOUNT.id }); + useOfflineStore.setState({ enabled: [ACCOUNT.id] }); + useAuthStore.setState({ client: null, isAuthenticated: false, activeAccountId: ACCOUNT.id }); + sessionAnswer = () => new Response('{}', { status }); + // The probe passes: the server itself is there. + offline.gone = false; + await useAuthStore.getState().checkAuth(); + await vi.waitFor(() => expect(offline.order).toContain('startedOffline')); + expect(current()?.isOfflineOnly()).toBe(true); + expect(useAccountStore.getState().getAccountById(ACCOUNT.id)).toBeTruthy(); + } +}); + it('goes the usual way when the store has no copy', async () => { offline.session = null; await useAuthStore.getState().checkAuth(); @@ -142,6 +167,41 @@ it('doesn\'t touch the store for an account without it', async () => { expect(current()?.isOfflineOnly()).toBe(false); }); +it('goes the usual way while the store has no folders yet', async () => { + offline.ready = false; + await useAuthStore.getState().checkAuth(); + expect(offline.order).not.toContain('local client'); + expect(current()?.isOfflineOnly()).toBe(false); +}); + +it('still starts on an empty store when the server can\'t be reached', async () => { + offline.ready = false; + offline.gone = true; + sessionAnswer = () => new Response('{}', { status: 503 }); + await useAuthStore.getState().checkAuth(); + await vi.waitFor(() => expect(current()?.isOfflineOnly()).toBe(true)); +}); + +it('fills the From list from the server after the swap, when the local copy had none', async () => { + let open: () => void = () => {}; + connectGate = new Promise((resolve) => { open = resolve; }); + await useAuthStore.getState().checkAuth(); + await vi.waitFor(() => expect(current()?.isOfflineOnly()).toBe(true)); + expect(useAuthStore.getState().identities).toEqual([]); + open(); + await vi.waitFor(() => expect(useAuthStore.getState().identities.map((identity) => identity.email)).toEqual([ACCOUNT.username])); + expect(useAuthStore.getState().primaryIdentity?.email).toBe(ACCOUNT.username); +}); + +it('doesn\'t let a late local answer overwrite the server\'s From list', async () => { + offline.localIdentities = new Promise((resolve) => { offline.releaseLocal = () => resolve([{ id: 'old', email: 'stale@x.test', name: 'Stale', mayDelete: false }]); }); + await useAuthStore.getState().checkAuth(); + await vi.waitFor(() => expect(useAuthStore.getState().identities.map((identity) => identity.email)).toEqual([ACCOUNT.username])); + offline.releaseLocal(); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(useAuthStore.getState().identities.map((identity) => identity.email)).toEqual([ACCOUNT.username]); +}); + it('leaves the login path to a refused account, and keeps its data', async () => { sessionAnswer = () => new Response('{}', { status: 401 }); await useAuthStore.getState().checkAuth(); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index c891ab06c..1fae88b7b 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -1368,7 +1368,8 @@ async function startLocalCopyClient(accountId: string, serverUrl: string, userna if (IS_LITE || skipLocalCopy || !useOfflineStore.getState().enabled.includes(accountId)) return null; try { const offline = await import('@/lib/offline/offline-mode'); - const session = await offline.offlineSessionFor(accountId); + // A store that has no folders yet has nothing to show: the snapshot stays up. + const session = await offline.offlineSessionFor(accountId, { withFolders: true }); return session ? JMAPClient.fromOfflineSession(serverUrl, username, session) : null; } catch { return null; @@ -2916,7 +2917,7 @@ export const useAuthStore = create()( impersonated: session.impersonated === true, }); void syncAccountDisplayName(account.id, client); - } else if (res.status >= 500) { + } else if (res.status >= 500 || res.status === 429) { throw new TransientAuthError('Session restore failed', res.status); } else { throw new Error(`Session cookie missing: ${res.status}`); @@ -2994,6 +2995,9 @@ export const useAuthStore = create()( if (!real) { if (useAccountStore.getState().getAccountById(entry.id)) { clients.set(entry.id, local); + // A 5xx or a rate limit, with the server up otherwise. The app stays + // on the local copy, and says so, and leaves it when the server answers. + void import('@/lib/offline/offline-mode').then((offline) => offline.markStartedOffline()).catch(() => {}); return; } if (get().activeAccountId !== entry.id) return; @@ -3006,6 +3010,15 @@ export const useAuthStore = create()( if (get().activeAccountId !== entry.id) return; initializeFeatureStores(real); set({ client: real, ...getClientRateLimitState(real), connectionLost: false, error: null }); + // The local copy may have had none yet, and the server's are newer. + void real.getIdentities() + .then((raw) => { + if (get().client !== real) return; + const { identities, primaryIdentity } = loadIdentities(raw, entry.username); + set({ identities, primaryIdentity }); + applyPreferredIdentity(entry.id); + }) + .catch((err) => debug.error('Failed to load identities after the local copy:', err)); }; // A few at a time rather than one after the other: with many logins @@ -3050,6 +3063,8 @@ export const useAuthStore = create()( // spending a serial round trip before the UI unblocks. const identitiesLoaded = targetClient.getIdentities() .then((raw) => { + // The real client took over meanwhile, and asks for itself. + if (localCopy && get().client !== null && get().client !== targetClient) return; const { identities, primaryIdentity } = loadIdentities(raw, targetAccount.username); set({ identities, primaryIdentity }); })