fix: renew push for every connected account, not only the active one #1054

The background resync only touched the active client, so accounts
turned on from the all-accounts panel lapsed when the server's push
subscription expired after seven days. The resync now walks every
connected login in turn, active one first, and runs again once the
background logins finish connecting. Single-account behaviour is
unchanged.

The panel also lists which accounts failed to enable and why, instead
of only a count.

components/__tests__/push-notification-prompt.test.tsx

@@ -17,7 +17,12 @@ const mocks = vi.hoisted(() => ({
     username: "alice@example.com",
     isAuthenticated: true,
     isDemoMode: false,
+    connectedAccountsRevision: 0,
+    // Login id -> client, as the auth store keeps it for the account switcher.
+    connected: new Map<string, { getAccountId: () => string }>(),
+    getAllConnectedClients: () => new Map(mocks.authState.connected),
   },
+  accounts: [] as { id: string; username: string }[],
   settingsState: { emailNotificationsEnabled: true, pushRelayUrl: "" },
   policyState: {
     loaded: true,
@@ -34,8 +39,15 @@ vi.mock("next/navigation", () => ({
 }));
 
 vi.mock("@/stores/auth-store", () => ({
-  useAuthStore: (selector: (state: typeof mocks.authState) => unknown) =>
-    selector(mocks.authState),
+  useAuthStore: Object.assign(
+    (selector: (state: typeof mocks.authState) => unknown) =>
+      selector(mocks.authState),
+    { getState: () => mocks.authState },
+  ),
+}));
+
+vi.mock("@/stores/account-store", () => ({
+  useAccountStore: { getState: () => ({ accounts: mocks.accounts }) },
 }));
 
 vi.mock("@/stores/settings-store", () => ({
@@ -82,6 +94,9 @@ describe("PushNotificationPrompt", () => {
     mocks.supported = true;
     mocks.enabled = false;
     mocks.authState.client = { getAccountId: () => "account-1" };
+    mocks.authState.connectedAccountsRevision = 0;
+    mocks.authState.connected = new Map([["alice@mail.example.com", mocks.authState.client]]);
+    mocks.accounts = [{ id: "alice@mail.example.com", username: "alice@example.com" }];
     mocks.authState.username = "alice@example.com";
     mocks.authState.isAuthenticated = true;
     mocks.authState.isDemoMode = false;
@@ -188,6 +203,63 @@ describe("PushNotificationPrompt", () => {
     expect(mocks.enableWebPush).not.toHaveBeenCalled();
   });
 
+  it("renews push for every connected login, not only the active one", async () => {
+    // Push for bob was enabled from the all-accounts panel; he never becomes
+    // the active account, so only this background pass keeps his
+    // registration from hitting the server's 7-day expiry.
+    const bob = { getAccountId: () => "account-2" };
+    mocks.authState.connected.set("bob@mail.example.com", bob);
+    mocks.accounts.push({ id: "bob@mail.example.com", username: "bob@example.com" });
+
+    render(<PushNotificationPrompt />);
+    await advancePromptDelay();
+
+    expect(mocks.resyncWebPush).toHaveBeenCalledTimes(2);
+    expect(mocks.resyncWebPush.mock.calls[0][0]).toMatchObject({
+      client: mocks.authState.client,
+      accountLabel: "alice@example.com",
+    });
+    expect(mocks.resyncWebPush.mock.calls[1][0]).toMatchObject({
+      client: bob,
+      accountLabel: "bob@example.com",
+      relayBaseUrl: "https://notifications.relay.bulwarkmail.org",
+    });
+  });
+
+  it("renews the other logins again when the tab returns to the foreground", async () => {
+    const bob = { getAccountId: () => "account-2" };
+    mocks.authState.connected.set("bob@mail.example.com", bob);
+    render(<PushNotificationPrompt />);
+    await advancePromptDelay();
+    mocks.resyncWebPush.mockClear();
+
+    await act(async () => {
+      document.dispatchEvent(new Event("visibilitychange"));
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    expect(mocks.resyncWebPush.mock.calls.map(([p]) => p.client)).toEqual([
+      mocks.authState.client,
+      bob,
+    ]);
+  });
+
+  it("reaches logins that finish connecting after the first pass", async () => {
+    const { rerender } = render(<PushNotificationPrompt />);
+    await advancePromptDelay();
+    expect(mocks.resyncWebPush).toHaveBeenCalledTimes(1);
+
+    // The background restore connects bob and bumps the revision.
+    const bob = { getAccountId: () => "account-2" };
+    mocks.authState.connected.set("bob@mail.example.com", bob);
+    mocks.authState.connectedAccountsRevision = 1;
+    rerender(<PushNotificationPrompt />);
+    await advancePromptDelay();
+
+    expect(mocks.resyncWebPush.mock.calls.map(([p]) => p.client)).toContain(bob);
+  });
+
   it("does not touch push in demo mode", async () => {
     mocks.authState.isDemoMode = true;
     render(<PushNotificationPrompt />);

components/push-notification-prompt.tsx

@@ -4,6 +4,7 @@ import { useCallback, useEffect, useLayoutEffect, useState } from "react";
 import { usePathname } from "next/navigation";
 import { useTranslations } from "next-intl";
 import { Bell, Loader2, X } from "@/components/icons";
+import { useAccountStore } from "@/stores/account-store";
 import { useAuthStore } from "@/stores/auth-store";
 import { usePolicyStore } from "@/stores/policy-store";
 import { useSettingsStore } from "@/stores/settings-store";
@@ -26,6 +27,37 @@ function dismissedKey(accountId: string): string {
   return `${DISMISSED_KEY_PREFIX}${accountId}`;
 }
 
+interface ResyncTarget {
+  client: NonNullable<ReturnType<typeof useAuthStore.getState>["client"]>;
+  accountLabel?: string;
+}
+
+/**
+ * Every login whose push registration this tab should keep alive: the active
+ * one first, then each other connected login. Push can be enabled for a login
+ * that is not on screen (the all-accounts panel in settings), and Stalwart
+ * expires a PushSubscription after 7 days, so renewing only the active login
+ * would let the others lapse. The other clients are the ones the auth store
+ * already keeps for the account switcher, so this opens no new connection;
+ * resyncWebPush itself skips logins without push in a local check.
+ */
+function resyncTargets(
+  activeClient: ResyncTarget["client"],
+  activeLabel: string | undefined,
+): ResyncTarget[] {
+  const targets: ResyncTarget[] = [{ client: activeClient, accountLabel: activeLabel }];
+  const auth = useAuthStore.getState();
+  const others = auth.getAllConnectedClients?.() ?? new Map();
+  if (others.size === 0) return targets;
+  const accounts = useAccountStore.getState().accounts;
+  for (const [loginId, other] of others) {
+    if (!other || other === activeClient) continue;
+    const account = accounts.find((a) => a.id === loginId);
+    targets.push({ client: other, accountLabel: account?.username });
+  }
+  return targets;
+}
+
 function isExcludedPath(pathname: string): boolean {
   return /(^|\/)settings(?:\/|$)/.test(pathname)
     || /(^|\/)setup(?:\/|$)/.test(pathname)
@@ -58,6 +90,11 @@ export function PushNotificationPrompt() {
   const username = useAuthStore((state) => state.username);
   const isAuthenticated = useAuthStore((state) => state.isAuthenticated);
   const isDemoMode = useAuthStore((state) => state.isDemoMode);
+  // Bumped when the background logins finish connecting: the renewal below
+  // re-runs then so it reaches them too.
+  const connectedAccountsRevision = useAuthStore(
+    (state) => state.connectedAccountsRevision,
+  );
   const emailNotificationsEnabled = useSettingsStore(
     (state) => state.emailNotificationsEnabled,
   );
@@ -121,21 +158,30 @@ export function PushNotificationPrompt() {
   // live registration stale.
   useEffect(() => {
     if (!policyLoaded || !isAuthenticated || !client || !accountId || isDemoMode) return;
-    const resync = () => {
-      void resyncWebPush({
-        client,
-        relayBaseUrl,
-        accountLabel: username ?? undefined,
-        inboxOnly: pushNotifyInboxOnly,
-      });
+    let cancelled = false;
+    const resync = async () => {
+      // One login at a time: they share the browser's single
+      // PushSubscription, and a parallel re-create would race on it.
+      for (const target of resyncTargets(client, username ?? undefined)) {
+        if (cancelled) return;
+        await resyncWebPush({
+          client: target.client,
+          relayBaseUrl,
+          accountLabel: target.accountLabel,
+          inboxOnly: pushNotifyInboxOnly,
+        });
+      }
     };
-    resync();
+    void resync();
     // A long-open tab comes back to the foreground: renew the subscription
     // before the server's 7-day expiry (resyncWebPush runs at most daily).
-    const onVisible = () => { if (document.visibilityState === 'visible') resync(); };
+    const onVisible = () => { if (document.visibilityState === 'visible') void resync(); };
     document.addEventListener('visibilitychange', onVisible);
-    return () => document.removeEventListener('visibilitychange', onVisible);
-  }, [accountId, client, isAuthenticated, isDemoMode, policyLoaded, pushNotifyInboxOnly, relayBaseUrl, username]);
+    return () => {
+      cancelled = true;
+      document.removeEventListener('visibilitychange', onVisible);
+    };
+  }, [accountId, client, connectedAccountsRevision, isAuthenticated, isDemoMode, policyLoaded, pushNotifyInboxOnly, relayBaseUrl, username]);
 
   useEffect(() => {
     let cancelled = false;

components/settings/__tests__/multi-account-push.test.tsx

@@ -1,4 +1,4 @@
-import { render, waitFor } from '@testing-library/react';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 import { describe, it, expect, vi } from 'vitest';
 import { MultiAccountPushSettings } from '../multi-account-push';
 import { useAccountStore } from '@/stores/account-store';
@@ -6,7 +6,8 @@ import { useAuthStore } from '@/stores/auth-store';
 import * as webPush from '@/lib/web-push';
 
 vi.mock('next-intl', () => ({
-  useTranslations: () => (key: string) => key,
+  useTranslations: () => (key: string, values?: Record<string, unknown>) =>
+    values ? `${key} ${JSON.stringify(values)}` : key,
 }));
 
 vi.mock('../settings-section', async (importOriginal) => ({
@@ -44,4 +45,38 @@ describe('MultiAccountPushSettings', () => {
     expect(webPush.isWebPushEnabled).toHaveBeenCalledWith('jmap-a');
     expect(webPush.isWebPushEnabled).not.toHaveBeenCalledWith('a@mail.example.com');
   });
+
+  it('names each account the bulk enable left off, with the reason', async () => {
+    const clients: Record<string, { getAccountId: () => string }> = {
+      'a@mail.example.com': { getAccountId: () => 'jmap-a' },
+      'b@mail.example.com': { getAccountId: () => 'jmap-b' },
+      'c@mail.example.com': { getAccountId: () => 'jmap-c' },
+    };
+    useAccountStore.setState({
+      accounts: [
+        { id: 'a@mail.example.com', username: 'a', email: 'a@example.com', displayName: 'Alice', isConnected: true },
+        { id: 'b@mail.example.com', username: 'b', email: 'b@example.com', displayName: 'Bob', isConnected: true },
+        { id: 'c@mail.example.com', username: 'c', email: 'c@example.com', displayName: 'Carol', isConnected: true },
+      ] as never,
+    });
+    useAuthStore.setState({ getClientForAccount: ((id: string) => clients[id]) as never });
+    vi.mocked(webPush.enableWebPushForAccounts).mockResolvedValueOnce({
+      enabled: ['a@mail.example.com'],
+      failed: [
+        { accountId: 'b@mail.example.com', error: new Error('relay unreachable') },
+        { accountId: 'c@mail.example.com', error: 'timeout' },
+      ],
+    });
+
+    render(<MultiAccountPushSettings />);
+    await waitFor(() => expect(screen.getByText('select_all')).toBeInTheDocument());
+    fireEvent.click(screen.getByLabelText('select_all'));
+    fireEvent.click(screen.getByText('enable_selected'));
+
+    const alert = await screen.findByRole('alert');
+    expect(screen.getByText(/result_partial/).textContent).toContain('"enabled":1,"failed":2');
+    expect(alert.textContent).toContain('Bob (b@example.com): relay unreachable');
+    expect(alert.textContent).toContain('Carol (c@example.com): timeout');
+    expect(alert.textContent).not.toContain('Alice');
+  });
 });

components/settings/multi-account-push.tsx

@@ -10,6 +10,18 @@ import { useAuthStore } from '@/stores/auth-store';
 import { useSettingsStore } from '@/stores/settings-store';
 import { disableWebPush, enableWebPushForAccounts, isWebPushEnabled, isWebPushSupported } from '@/lib/web-push';
 
+interface FailedAccount {
+  id: string;
+  name: string;
+  email: string;
+  reason: string;
+}
+
+function failureReason(error: unknown): string {
+  if (error instanceof Error && error.message) return error.message;
+  return String(error);
+}
+
 /**
  * Turn push on or off for several accounts at once.
  *
@@ -27,6 +39,9 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri
   const [chosen, setChosen] = useState<Set<string>>(new Set());
   const [busy, setBusy] = useState(false);
   const [notice, setNotice] = useState('');
+  // Which accounts the last bulk enable left off, and why: a bare count
+  // leaves the user guessing which login to retry.
+  const [failures, setFailures] = useState<FailedAccount[]>([]);
 
   const connected = accounts.filter(a => a.isConnected && useAuthStore.getState().getClientForAccount(a.id));
 
@@ -56,7 +71,7 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri
     const auth = useAuthStore.getState();
     const targets = connected.filter(a => chosen.has(a.id));
     if (!targets.length) return;
-    setBusy(true); setNotice('');
+    setBusy(true); setNotice(''); setFailures([]);
     try {
       if (action === 'enable') {
         const outcome = await enableWebPushForAccounts(
@@ -66,6 +81,15 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri
         setNotice(outcome.failed.length
           ? t('result_partial', { enabled: outcome.enabled.length, failed: outcome.failed.length })
           : t('result_enabled', { count: outcome.enabled.length }));
+        setFailures(outcome.failed.map(({ accountId, error }) => {
+          const account = targets.find(a => a.id === accountId);
+          return {
+            id: accountId,
+            name: account ? (account.displayName || account.label || account.username) : accountId,
+            email: account ? (account.email || account.username) : '',
+            reason: failureReason(error),
+          };
+        }));
       } else {
         // Teardown swallows its own failures, so every account ends up local-off.
         for (const account of targets) {
@@ -112,6 +136,13 @@ export function MultiAccountPushSettings({ relayBaseUrl }: { relayBaseUrl?: stri
         </Button>
       </div>
       {notice && <p className="text-sm text-muted-foreground">{notice}</p>}
+      {failures.length > 0 && <ul role="alert" className="space-y-1 text-sm text-destructive">
+        {failures.map(f => <li key={f.id}>
+          <span className="font-medium">{f.name}</span>
+          {f.email && f.email !== f.name && <span> ({f.email})</span>}
+          <span>: {f.reason}</span>
+        </li>)}
+      </ul>}
     </div>
   </SettingsSection>;
 }