Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/specs/pocket-app.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
37 changes: 35 additions & 2 deletions lib/src/remote/pocket-app/App.push.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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[],
}));
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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([]);
Expand Down Expand Up @@ -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
Expand Down
39 changes: 26 additions & 13 deletions lib/src/remote/pocket-app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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 } : {}),
};
}

Expand Down
Loading