Register push only for the signed-in account's Burrows (#1238)

Author: nedtwigg

docs/specs/pocket-app.md

@@ -117,6 +117,8 @@ Installed means `navigator.standalone === true` (iOS) or the standard `(display-
 
 **Registering another Burrow, or retrying that POST, reuses the scope's existing `PushSubscription`** when its `applicationServerKey` matches the Relay's VAPID key byte-for-byte; a new endpoint is minted only when the key differs (rationale). **When it does rotate, the Relay drops that device's other Burrow rows in the same mutation** and its response lists what survived, which makes a committed POST whose response was lost self-repairing (rationale).
 
+**Enable registers, and the offer counts, only the signed-in account's paired Burrows its list still names**: another account's record is never a target, since the session's Relay holds none of that account's Burrows.
+
 **Obsolete delivery mappings are retired, durably.** A `pairing-required` transition, a re-pair that mints a new id, and an explicit **Remove** each write the old `{ burrowId, accountId, deliveryId }` (the record's account) to `PendingDeliveryDeletionV1` *before* the record forgets it (rationale), then call the idempotent deletion route; tombstones retry until a Relay answer clears them. **Only a session of the tombstone's account sends or clears it** (rationale). **This deletes the delivery row alone** — never the scope's shared `PushSubscription`, and never another Burrow's row.
 
 Source of truth: `getPushAvailability` / `subscribeToPushInBrowser` in `lib/src/remote/client/push-subscribe.ts`; `pushNoticeState` in `lib/src/remote/pocket-app/App.tsx`; `PocketClient.subscribeToPush` / `retirePendingDeletions` in `lib/src/remote/client/pocket-client.ts`; `pushEndpointFingerprint` in `remote-lib-common/src/security/push.ts`.

lib/src/remote/pocket-app/App.push.test.tsx

@@ -35,6 +35,8 @@ const fake = vi.hoisted(() => ({
   retirePendingDeletions: vi.fn<() => Promise<void>>(),
   /** Paired Burrows the Relay's list no longer names: removed rows. */
   unlisted: [] as string[],
+  /** Burrows paired under an earlier account, which this session's list never names. */
+  previousAccount: [] as string[],
   /** Every client call, in order, so "before" can be asserted rather than assumed. */
   order: [] as string[],
 }));
@@ -68,9 +70,15 @@ vi.mock('../client/pocket-client', async (importOriginal) => ({
     setOnTransportChanged = () => undefined;
     close = () => undefined;
     openSocket = async () => undefined;
-    listKnownBurrows = async () => KNOWN;
+    listKnownBurrows = async () =>
+      KNOWN.map((record) =>
+        fake.previousAccount.includes(record.burrowId) ? { ...record, accountId: 'previous' } : record,
+      );
     listBurrows = async () =>
-      KNOWN.filter((record) => !fake.unlisted.includes(record.burrowId)).map((record) => ({
+      KNOWN.filter(
+        (record) =>
+          !fake.unlisted.includes(record.burrowId) && !fake.previousAccount.includes(record.burrowId),
+      ).map((record) => ({
         burrowId: record.burrowId,
         label: record.label,
         online: true,
@@ -134,6 +142,7 @@ beforeEach(() => {
   fake.availability = 'ready';
   fake.order = [];
   fake.unlisted = [];
+  fake.previousAccount = [];
   fake.subscribeInBrowser.mockReset();
   fake.retirePendingDeletions.mockReset().mockResolvedValue(undefined);
   fake.listPushSubscribedBurrows.mockReset().mockResolvedValue([]);
@@ -264,6 +273,30 @@ describe('the one Enable on the Burrows view', () => {
     expect(buttonNamed(container, ENABLE)).toBeNull();
   });
 
+  /**
+   * Records outlive an account switch, but the Relay scopes registration to the
+   * session's account and refuses a Burrow another account holds. Registering
+   * one would stop the loop before this account's own Burrows, and re-offer an
+   * Enable that can never finish.
+   */
+  it('skips a Burrow paired under another account, and keeps its row', async () => {
+    fake.previousAccount = ['burrow-1'];
+    fake.subscribeInBrowser.mockResolvedValue({ endpoint: 'https://push.example/abc' });
+    fake.subscribeToPush.mockImplementation(async (burrowId) => {
+      if (fake.previousAccount.includes(burrowId)) throw new Error('unknown burrow');
+      return { burrowIds: [...registered.add(burrowId)] };
+    });
+    await openBurrows();
+
+    expect(rowFor(container, 'First laptop')).not.toBeNull();
+    await click(container, ENABLE);
+
+    expect(fake.subscribeToPush.mock.calls.map(([burrowId]) => burrowId)).toEqual(['burrow-2']);
+    expect(alertText(container)).toBeNull();
+    expect(container.textContent).toContain('Push notifications on.');
+    expect(buttonNamed(container, ENABLE)).toBeNull();
+  });
+
   /**
    * The readback is the only thing that says which Burrows hold a row. A read
    * that threw learned nothing — and empty is not nothing — so the card

lib/src/remote/pocket-app/App.tsx

@@ -92,14 +92,21 @@ export interface BurrowView {
    * Absent is listed.
    */
   removed?: boolean;
+  /**
+   * The record was paired under an account other than the signed-in one, so
+   * this session's Relay does not hold the Burrow. The row keeps its offline
+   * face; absent is the signed-in account's.
+   */
+  otherAccount?: boolean;
 }
 
 /**
- * A Burrow push can be registered against: paired, and still on the Relay's
- * list, whose `pushSubscribe` refuses a Burrow it no longer holds.
+ * A Burrow push can be registered against: paired, of the signed-in account,
+ * and still on the Relay's list, whose `pushSubscribe` refuses a Burrow the
+ * session's account does not hold.
  */
 function isPushTarget(burrow: BurrowView): boolean {
-  return !burrow.needsPairing && !burrow.removed;
+  return !burrow.needsPairing && !burrow.removed && !burrow.otherAccount;
 }
 
 /** What a removed Burrow's row says, by who serves this Pocket. */
@@ -352,8 +359,9 @@ export default function App({
    * The Burrows list: the pinned records, with online state stamped on from the
    * Relay, and a record of the signed-in account the Relay's list no longer
    * names marked removed — only ever off a list that was read, since a failed
-   * read throws. **Another account's record is never marked removed**: this
-   * session's list names only its own account's Burrows. **A Burrow with no
+   * read throws. **Another account's record is never marked removed**, only
+   * `otherAccount`: this session's list names only its own account's Burrows,
+   * so it can say nothing about that one. **A Burrow with no
    * record is not shown** — the Relay's list is discovery, and a row for a
    * computer this phone holds no key for would offer an action that cannot
    * exist. Answers the rows it painted.
@@ -362,13 +370,13 @@ export default function App({
     const account = client.accountId;
     const [records, enrolled] = await Promise.all([client.listKnownBurrows(), client.listBurrows()]);
     const online = new Map(enrolled.map((burrow) => [burrow.burrowId, burrow.online]));
-    const views = records.map((record) =>
-      toBurrowView(
-        record,
-        online.get(record.burrowId) ?? false,
-        account !== null && record.accountId === account && !online.has(record.burrowId),
-      ),
-    );
+    const views = records.map((record) => {
+      const ours = account !== null && record.accountId === account;
+      return toBurrowView(record, online.get(record.burrowId) ?? false, {
+        removed: ours && !online.has(record.burrowId),
+        otherAccount: !ours,
+      });
+    });
     setBurrows(views);
     setPhase({ at: 'burrows' });
     // Owed deletions retry here: this runs after every sign-in and on every
@@ -748,13 +756,18 @@ async function retireUnlessExpired(client: PocketClient, setupToken: string): Pr
 }
 
 /** One pinned record as the list renders it. */
-function toBurrowView(record: KnownBurrowSummary, online: boolean, removed = false): BurrowView {
+function toBurrowView(
+  record: KnownBurrowSummary,
+  online: boolean,
+  { removed, otherAccount }: Pick<BurrowView, 'removed' | 'otherAccount'> = {},
+): BurrowView {
   return {
     burrowId: record.burrowId,
     label: record.label || record.burrowId,
     online,
     needsPairing: record.authorization.state !== 'paired',
     ...(removed ? { removed } : {}),
+    ...(otherAccount ? { otherAccount } : {}),
   };
 }