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.

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<void>((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<void>((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<boolean>((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', () => {

src/device-sync/app/lifecycle.ts

@@ -180,6 +180,25 @@ function oneAtATime<T>(registryId: string, step: () => Promise<T>): Promise<T> {
   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<T>(registryIds: readonly string[], step: () => Promise<T>): Promise<T> {
+  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<string, Promise<boolean>>();
+
+/**
+ * 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<T>(registryId: string, step: () => Promise<T>, released: T): Promise<T> {
+  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<EnableOutcome> {
-  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<EnableOutcome> {
@@ -230,7 +249,7 @@ async function enable(registryId: string, authority: Authority): Promise<EnableO
 
 /** Automatic sync back on after the user kept syncing instead of losing changes. */
 export function resumeDeviceSync(registryId: string, authority: Authority): Promise<void> {
-  return oneAtATime(registryId, () => resume(registryId, authority));
+  return accountStep(registryId, () => resume(registryId, authority), undefined);
 }
 
 async function resume(registryId: string, authority: Authority): Promise<void> {
@@ -316,7 +335,8 @@ export function disableDeviceSync(
   authority: Authority,
   options: { force?: boolean } = {},
 ): Promise<TurnOffResult> {
-  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<TurnOffResult> {
@@ -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<boolean> = confirmSignOutLosingChanges,
 ): Promise<boolean> {
-  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(