From ef96acd05f68c135e19b34e58b08d13a5fde2f56 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sun, 27 Sep 2026 10:03:20 +0200 Subject: [PATCH] fix: finish turning sync on or off before signing an account out #34 Turning device sync on or off runs one step after the other per account, but signing out did not wait its turn. A sign-out that started while a turn-on was still setting up (or queued behind a slow turn-off) found no Android account yet and went ahead; the turn-on then created the Android account, stored it and requested a sync for the signed-out account, and the next check posted "Sign in to keep syncing". A turn-off still tearing down could likewise store its preferences after the sign-out had forgotten the account. The release before a sign-out is now a step in the queue of every account it releases, taken in a fixed order: it waits for the steps under way and releases what they set up. A turn-on or turn-off asked for while the sign-out runs comes after it, and does nothing once the account was released; if the user stays signed in it goes ahead. --- .../__tests__/app/lifecycle.test.ts | 59 +++++++++++++++++++ src/device-sync/app/lifecycle.ts | 58 +++++++++++++----- 2 files changed, 103 insertions(+), 14 deletions(-) diff --git a/src/device-sync/__tests__/app/lifecycle.test.ts b/src/device-sync/__tests__/app/lifecycle.test.ts index c61d4a85..b95b0bb1 100644 --- a/src/device-sync/__tests__/app/lifecycle.test.ts +++ b/src/device-sync/__tests__/app/lifecycle.test.ts @@ -375,6 +375,65 @@ describe('sign-out', () => { expect(confirm).toHaveBeenCalledWith(4); expect(h.state.accounts).toEqual([]); }); + + it('waits for a turn-on in progress, then releases what it set up', async () => { + let finishEnsure: () => void = () => undefined; + vi.mocked(native.ensureAndroidAccount).mockImplementationOnce(async (name: string, registryId: string) => { + h.state.log.push(`ensure ${name}`); + await new Promise((resolve) => { finishEnsure = resolve; }); + h.state.accounts.push({ name, registryId, sync: {} }); + return true; + }); + const turningOn = enableDeviceSync(ALICE, CONTACTS_AUTHORITY); + await vi.waitFor(() => expect(h.state.log).toContain('ensure alice')); + + const signingOut = releaseDeviceSyncBeforeSignOut([ALICE], vi.fn(async () => true)); + finishEnsure(); + + expect(await signingOut).toBe(true); + await turningOn; + expect(teardown).toHaveBeenCalledWith(ALICE, 'alice', CONTACTS_AUTHORITY, { uploadFirst: true, timeoutMs: TEARDOWN_TIMEOUT_MS }); + expect(h.state.accounts).toEqual([]); + expect(useDeviceSyncStore.getState().accounts[ALICE]).toBeUndefined(); + }); + + it('lets a turn-on or turn-off asked for during the sign-out set nothing up for the account', async () => { + await enableDeviceSync(ALICE, CONTACTS_AUTHORITY); + let finishTeardown: () => void = () => undefined; + teardown.mockImplementationOnce(async () => { + await new Promise((resolve) => { finishTeardown = resolve; }); + return { pending: 0 }; + }); + const signingOut = releaseDeviceSyncBeforeSignOut([ALICE], vi.fn(async () => true)); + await vi.waitFor(() => expect(teardown).toHaveBeenCalled()); + + const turningOn = enableDeviceSync(ALICE, CALENDAR_AUTHORITY); + const turningOff = disableDeviceSync(ALICE, CONTACTS_AUTHORITY); + finishTeardown(); + + expect(await signingOut).toBe(true); + expect((await turningOn).kind).toBe('failed'); + expect(await turningOff).toEqual({ done: true, pending: 0 }); + expect(h.state.accounts).toEqual([]); + expect(h.state.log.filter((l) => l.startsWith('ensure') || l.startsWith('sync'))).toEqual(['ensure alice', `sync alice ${CONTACTS_AUTHORITY} {"manual":true}`]); + expect(useDeviceSyncStore.getState().accounts[ALICE]).toBeUndefined(); + }); + + it('lets a turn-on asked for during the sign-out go ahead once the user chose to stay signed in', async () => { + await enableDeviceSync(ALICE, CONTACTS_AUTHORITY); + teardown.mockImplementation(async (_r: string, _n: string, authority: string) => ({ pending: authority === CONTACTS_AUTHORITY ? 1 : 0 })); + let answer: (signOut: boolean) => void = () => undefined; + const confirm = vi.fn(() => new Promise((resolve) => { answer = resolve; })); + const signingOut = releaseDeviceSyncBeforeSignOut([ALICE], confirm); + await vi.waitFor(() => expect(confirm).toHaveBeenCalled()); + + const turningOn = enableDeviceSync(ALICE, CALENDAR_AUTHORITY); + answer(false); + + expect(await signingOut).toBe(false); + expect(await turningOn).toEqual({ kind: 'enabled', accountName: 'alice' }); + expect(h.find('alice')?.sync[CALENDAR_AUTHORITY].automatic).toBe(true); + }); }); describe('reconcile', () => { diff --git a/src/device-sync/app/lifecycle.ts b/src/device-sync/app/lifecycle.ts index e873595e..4daba392 100644 --- a/src/device-sync/app/lifecycle.ts +++ b/src/device-sync/app/lifecycle.ts @@ -180,6 +180,25 @@ function oneAtATime(registryId: string, step: () => Promise): Promise { return next; } +/** One step in the queue of every one of the accounts, taken in a fixed order so two such steps never wait for each other. */ +function oneAtATimeForAll(registryIds: readonly string[], step: () => Promise): Promise { + const [first, ...rest] = registryIds; + return first === undefined ? step() : oneAtATime(first, () => oneAtATimeForAll(rest, step)); +} + +// Sign-outs under way, per app account: true once they released it. +const signOuts = new Map>(); + +/** + * A turn-on or turn-off of one app account, in its queue. One asked for + * while the account signs out runs after the sign-out, and does nothing + * (`released`) when that released the account. + */ +function accountStep(registryId: string, step: () => Promise, released: T): Promise { + const signOut = signOuts.get(registryId); + return oneAtATime(registryId, async () => ((await signOut) ? released : step())); +} + // ─── Enable ──────────────────────────────────────────── export type EnableOutcome = @@ -193,7 +212,7 @@ export type EnableOutcome = * asked for the permissions (and, for calendars, who reminds the user). */ export function enableDeviceSync(registryId: string, authority: Authority): Promise { - return oneAtATime(registryId, () => enable(registryId, authority)); + return accountStep(registryId, () => enable(registryId, authority), { kind: 'failed', message: 'The account was signed out' }); } async function enable(registryId: string, authority: Authority): Promise { @@ -230,7 +249,7 @@ async function enable(registryId: string, authority: Authority): Promise { - return oneAtATime(registryId, () => resume(registryId, authority)); + return accountStep(registryId, () => resume(registryId, authority), undefined); } async function resume(registryId: string, authority: Authority): Promise { @@ -316,7 +335,8 @@ export function disableDeviceSync( authority: Authority, options: { force?: boolean } = {}, ): Promise { - return oneAtATime(registryId, () => disable(registryId, authority, options)); + // Released for signing out meanwhile: nothing of it is left on the device. + return accountStep(registryId, () => disable(registryId, authority, options), { done: true, pending: 0 }); } async function disable(registryId: string, authority: Authority, options: { force?: boolean }): Promise { @@ -363,21 +383,31 @@ async function disable(registryId: string, authority: Authority, options: { forc * rows and removes their Android accounts, while the credentials are still * there. When changes could not be uploaded, `confirm` asks whether to sign * out anyway. Resolves false when the user chose to stay signed in; their - * accounts then keep syncing. + * accounts then keep syncing. It is a step of the accounts' own turn-ons and + * turn-offs: it waits for those under way, and those asked for meanwhile find + * the accounts released and do nothing. */ -export async function releaseDeviceSyncBeforeSignOut( +export function releaseDeviceSyncBeforeSignOut( registryIds: readonly string[], confirm: (pending: number) => Promise = confirmSignOutLosingChanges, ): Promise { - if (!deviceSyncAvailable() || registryIds.length === 0) return true; - try { - return await release(registryIds, confirm); - } catch (err) { - // Never keep the user from signing out over a bug here; an Android - // account left behind is suspended by the next reconcile. - console.warn('[device-sync] release before sign-out failed', errorMessage(err)); - return true; - } + if (!deviceSyncAvailable() || registryIds.length === 0) return Promise.resolve(true); + const ids = [...new Set(registryIds)].sort(); + const signOut = oneAtATimeForAll(ids, async () => { + try { + return await release(registryIds, confirm); + } catch (err) { + // Never keep the user from signing out over a bug here; an Android + // account left behind is suspended by the next reconcile. + console.warn('[device-sync] release before sign-out failed', errorMessage(err)); + return true; + } + }); + for (const id of ids) signOuts.set(id, signOut); + void signOut.then(() => { + for (const id of ids) if (signOuts.get(id) === signOut) signOuts.delete(id); + }); + return signOut; } async function release(