From 5ce131517bf3e09af2f1eb22348bd974e03323dc Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:07:25 -0400 Subject: [PATCH 1/8] fix(notifications): refuse an app message before it spends the day window The minute window is spent first, so a request the minute window refuses between the read and the spend leaves the day window untouched. --- .../NotificationDriver.appMessage.test.ts | 82 ++++++++++++++++++- .../notification/NotificationDriver.ts | 17 ++-- 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts b/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts index 712900197..52dcea270 100644 --- a/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts +++ b/src/backend/drivers/notification/NotificationDriver.appMessage.test.ts @@ -17,7 +17,7 @@ * along with this program. If not, see . */ -import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { v4 as uuidv4 } from 'uuid'; import { makeActor, type Actor } from '../../core/actor.js'; import { runWithContext } from '../../core/context.js'; @@ -28,6 +28,42 @@ import type { IConfig } from '../../types.js'; import { peekRateLimit } from '../../core/http/middleware/rateLimit.js'; import type { NotificationDriver } from './NotificationDriver.js'; +/** + * Charges to a held key wait here until the test lets them go, so two requests + * can both be read before either is spent. Every charge is recorded by key. + */ +const charges = vi.hoisted(() => ({ + held: new Set(), + waiting: [] as Array<() => void>, + made: [] as string[], +})); + +vi.mock('../../core/http/middleware/rateLimit.js', async (importOriginal) => { + const actual = + await importOriginal< + typeof import('../../core/http/middleware/rateLimit.js') + >(); + return { + ...actual, + checkRateLimit: async (key: string, ...rest: unknown[]) => { + if (charges.held.has(key)) { + await new Promise((go) => charges.waiting.push(go)); + } + charges.made.push(key); + return ( + actual.checkRateLimit as (...a: unknown[]) => Promise + )(key, ...rest); + }, + }; +}); + +const until = async (ready: () => boolean): Promise => { + for (let i = 0; i < 400 && !ready(); i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + if (!ready()) throw new Error('timed out'); +}; + const makeUser = async (server: PuterServer): Promise => { const username = `am-${Math.random().toString(36).slice(2, 10)}`; const created = await server.stores.user.create({ @@ -347,6 +383,50 @@ describe('app.message with live delivery over the socket wire', () => { ).resolves.toMatchObject({ uid: expect.any(String) }); }); + it('spends the day window only on a request the minute window admits', async () => { + const holder = await makeUser(state.server); + const app = await makeApp(state.server, holder.user.id!); + const minuteKey = `notif:app-message:minute:${holder.user.id}:${app.uid}`; + const dayKey = `notif:app-message:day:${holder.user.id}:${app.uid}`; + for (let i = 0; i < 2; i++) { + await withActor(asApp(holder, app), () => + state.driver.create(message()), + ); + } + + charges.held.add(minuteKey).add(dayKey); + charges.made.length = 0; + let settled = false; + const outcome = Promise.allSettled([ + withActor(asApp(holder, app), () => state.driver.create(message())), + withActor(asApp(holder, app), () => state.driver.create(message())), + ]).finally(() => { + settled = true; + }); + try { + // Both requests read the windows as open before either spends. + await until(() => charges.waiting.length === 2); + while (!settled) { + await until(() => settled || charges.waiting.length > 0); + charges.waiting.shift()?.(); + } + } finally { + charges.held.clear(); + } + + const results = await outcome; + expect(results.map((r) => r.status).sort()).toEqual([ + 'fulfilled', + 'rejected', + ]); + const refused = results.find((r) => r.status === 'rejected') as + PromiseRejectedResult | undefined; + expect(refused?.reason).toMatchObject({ statusCode: 429 }); + // The refused request left the day window alone. + expect(charges.made.filter((k) => k === dayKey)).toEqual([dayKey]); + expect(await peekRateLimit(dayKey, 4, 24 * 60 * 60_000)).toBe(true); + }); + it("reads back the app's own messages and no other app's", async () => { const holder = await makeUser(state.server); const app = await makeApp(state.server, holder.user.id!); diff --git a/src/backend/drivers/notification/NotificationDriver.ts b/src/backend/drivers/notification/NotificationDriver.ts index 887a31e6d..7e58f3a80 100644 --- a/src/backend/drivers/notification/NotificationDriver.ts +++ b/src/backend/drivers/notification/NotificationDriver.ts @@ -340,23 +340,28 @@ export class NotificationDriver extends PuterDriver { }); } - /** Both windows are read before either is spent, so a refusal costs none. */ + /** + * Both windows are read before either is spent, so a refusal costs none. + * The minute window is spent first: it is the one most likely to run out + * between the read and the spend, and a refusal there leaves the day + * alone. + */ async #withinAppMessageLimits( userId: number, appUid: string, ): Promise { const configured = this.config.notificationAppMessageLimits ?? {}; const all: Array<[string, number, number]> = [ - [ - `notif:app-message:day:${userId}:${appUid}`, - configured.perDay ?? APP_MESSAGE_RATE_LIMITS.perDay, - DAY_MS, - ], [ `notif:app-message:minute:${userId}:${appUid}`, configured.perMinute ?? APP_MESSAGE_RATE_LIMITS.perMinute, MINUTE_MS, ], + [ + `notif:app-message:day:${userId}:${appUid}`, + configured.perDay ?? APP_MESSAGE_RATE_LIMITS.perDay, + DAY_MS, + ], ]; const windows = all.filter(([, limit]) => limit > 0); for (const [key, limit, windowMs] of windows) { From f0d7a69dd4da4f2516e86dbad8970063d496f114 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:07:27 -0400 Subject: [PATCH 2/8] fix(notifications): cap the sender name on app messages The app's title or name is cut to the title length, counting code points, before it is stored on the message. --- src/backend/services/notification/appMessage.test.ts | 10 ++++++++++ src/backend/services/notification/appMessage.ts | 7 ++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/backend/services/notification/appMessage.test.ts b/src/backend/services/notification/appMessage.test.ts index 48d4df0f1..717362010 100644 --- a/src/backend/services/notification/appMessage.test.ts +++ b/src/backend/services/notification/appMessage.test.ts @@ -212,4 +212,14 @@ describe('appMessageSender', () => { ).toEqual({ uid: 'app-1', name: 'chat' }); expect(appMessageSender('app-1', null)).toEqual({ uid: 'app-1' }); }); + + it('caps the name at the title length, counting an emoji once', () => { + const long = '😀'.repeat(500); + const { name } = appMessageSender('app-1', { title: long }); + expect([...name!].length).toBe(120); + expect(name).toBe('😀'.repeat(120)); + expect(appMessageSender('app-1', { title: 'a'.repeat(300) }).name).toBe( + 'a'.repeat(120), + ); + }); }); diff --git a/src/backend/services/notification/appMessage.ts b/src/backend/services/notification/appMessage.ts index a79155b65..a13bb84a4 100644 --- a/src/backend/services/notification/appMessage.ts +++ b/src/backend/services/notification/appMessage.ts @@ -69,7 +69,12 @@ export const appMessageSender = ( const name = [row?.title, row?.name].find( (v): v is string => typeof v === 'string' && v.trim() !== '', ); - if (name !== undefined) sender.name = cleanText(name).trim(); + if (name !== undefined) { + sender.name = [...cleanText(name)] + .slice(0, APP_MESSAGE_TITLE_MAX) + .join('') + .trim(); + } const icon = row?.icon; if ( typeof icon === 'string' && From bbf375db14cbe45e771dc7e0aeba72eae82bc3c7 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:10:56 -0400 Subject: [PATCH 3/8] fix(notifications): notif.written hands out a copy with the audience Listeners get their own copy of the value, so what the socket carries is what was written. Both emits name the audience, and an update names the row's app when the row or the value says which. --- src/backend/clients/event/types.ts | 10 +- .../notification/NotificationService.test.ts | 96 +++++++++++++++++-- .../notification/NotificationService.ts | 14 ++- 3 files changed, 105 insertions(+), 15 deletions(-) diff --git a/src/backend/clients/event/types.ts b/src/backend/clients/event/types.ts index d54e4abce..99036d2ac 100644 --- a/src/backend/clients/event/types.ts +++ b/src/backend/clients/event/types.ts @@ -552,13 +552,17 @@ export type EventMap = { /** Events handler runs behind whatever created it. */ handlerDepth?: number; }; - /** Every non-silent write or update, only on the node that wrote it. */ + /** + * Every non-silent write or update, only on the node that wrote it. The + * value is the listener's own copy of what the socket carries. + */ 'notif.written': { userId: number; uid: string; type: string; - /** The row's app. Absent on an update, which keeps the row's own. */ - appUid?: string | null; + audience: string; + /** The row's app; null when it names none or an update could not tell. */ + appUid: string | null; value: Record; /** An earlier notification rewritten in place. */ updated?: true; diff --git a/src/backend/services/notification/NotificationService.test.ts b/src/backend/services/notification/NotificationService.test.ts index 68f8d14af..ab7065f2a 100644 --- a/src/backend/services/notification/NotificationService.test.ts +++ b/src/backend/services/notification/NotificationService.test.ts @@ -3,18 +3,19 @@ * * This file is part of Puter. * - * Puter is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. + * Puter is free software: you can redistribute it and/or modify it under the + * terms of the GNU Affero General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) any + * later version. * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more + * details. * * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . + * along with this program. If not, see + * [https://www.gnu.org/licenses/](https://www.gnu.org/licenses/). */ import { v4 as uuidv4 } from 'uuid'; @@ -361,6 +362,7 @@ describe("NotificationService — 'notif.written'", () => { userId: u.id, uid: rows[i][0].uid, type: 'app.events.ended', + audience: 'app-user', appUid, value: { title: 'ended', type: 'app.events.ended' }, })); @@ -404,11 +406,82 @@ describe("NotificationService — 'notif.written'", () => { userId: user.id, uid, type: 'share.received', + audience: 'account', + appUid: null, value: { title: 'two shares', type: 'share.received' }, updated: true, }); written.stop(); }); + + it("names the row's app on a rewrite", async () => { + const user = await makeUser(); + const uid = uuidv4(); + const appUid = `app-${uuidv4()}`; + await server.stores.notification.insert({ + userId: user.id, + value: { title: 'one', type: 'share.received' }, + uid, + type: 'share.received', + audience: 'account', + appUid, + }); + const written = collect('notif.written'); + + await notifications.notifyUpdate( + uid, + user.id, + { title: 'two' }, + { type: 'share.received' }, + ); + await waitFor(() => written.seen.length === 1); + + expect(written.seen[0]).toMatchObject({ uid, appUid, updated: true }); + written.stop(); + }); + + it('hands each listener its own copy of the value', async () => { + const user = await makeUser(); + const handler = (_k: string, data: unknown) => { + const value = (data as { value: Record }).value; + value.title = 'changed by a listener'; + (value.fields as Record).path = '/elsewhere'; + }; + server.clients.event.on('notif.written', handler); + const pushed = collect('outer.gui.notif.message'); + try { + const uid = await notifications.notify( + [user.id], + { title: 'one share', fields: { path: '/a' } }, + { type: 'share.received' }, + ); + await notifications.notifyUpdate( + uid!, + user.id, + { title: 'two shares', fields: { path: '/b' } }, + { type: 'share.received' }, + ); + await waitFor(() => pushed.seen.length === 2); + } finally { + server.clients.event.off?.('notif.written', handler); + pushed.stop(); + } + + expect( + pushed.seen.map((p) => (p as NotifPush).response.notification), + ).toEqual([ + { + title: 'one share', + fields: { path: '/a' }, + type: 'share.received', + }, + { + title: 'two shares', + fields: { path: '/b' }, + type: 'share.received', + }, + ]); + }); }); describe('NotificationService.markAcknowledged', () => { @@ -608,7 +681,10 @@ describe('NotificationService — shown on delivery', () => { { type: 'share.received' }, ); - const [row] = await server.stores.notification.listByUserId(user.id, {}); + const [row] = await server.stores.notification.listByUserId( + user.id, + {}, + ); expect(row.uid).toBe(uid); expect(row.shown).toBeTruthy(); diff --git a/src/backend/services/notification/NotificationService.ts b/src/backend/services/notification/NotificationService.ts index c12962a23..5f661c4c3 100644 --- a/src/backend/services/notification/NotificationService.ts +++ b/src/backend/services/notification/NotificationService.ts @@ -170,14 +170,16 @@ export class NotificationService extends PuterService { } if (!opts.silent) { + // Listeners get their own copy; the socket sends `payload`. this.clients.event.emit( 'notif.written', { userId, uid, type: registered.type, + audience: registered.audience, appUid, - value: payload, + value: structuredClone(payload), }, {}, ); @@ -229,13 +231,21 @@ export class NotificationService extends PuterService { // to anything deduplicating on `event.id`. this.#socket().message(userId, uid, payload); this.#markDelivered(uid, userId); + const row = await this.stores.notification.getByUid(uid, { + userId, + }); + const app = payload.app as { uid?: unknown } | undefined; this.clients.event.emit( 'notif.written', { userId, uid, type: registered.type, - value: payload, + audience: registered.audience, + appUid: + row?.app_uid ?? + (typeof app?.uid === 'string' ? app.uid : null), + value: structuredClone(payload), updated: true, }, {}, From fe8acf007cd275d5e3256b8d11cb75d3467f1eec Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:11:46 -0400 Subject: [PATCH 4/8] fix(notifications): read the app off the notification on an update --- src/backend/services/notification/NotificationService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/services/notification/NotificationService.ts b/src/backend/services/notification/NotificationService.ts index 5f661c4c3..a9835c005 100644 --- a/src/backend/services/notification/NotificationService.ts +++ b/src/backend/services/notification/NotificationService.ts @@ -234,7 +234,7 @@ export class NotificationService extends PuterService { const row = await this.stores.notification.getByUid(uid, { userId, }); - const app = payload.app as { uid?: unknown } | undefined; + const app = notification.app as { uid?: unknown } | undefined; this.clients.event.emit( 'notif.written', { From b3dbfdbd7be2fb654d5cfe5ca1ae341fdf2234e9 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:17:40 -0400 Subject: [PATCH 5/8] fix(gui): route push taps through the stored notification A tap from the worker names only a notification; the desktop and the dashboard read the signed-in user's own row and route from its app, target and data, as a tap from the URL already did. Without a row the dashboard opens the list and the desktop does nothing. --- .../UI/Dashboard/UIDashboardNotifications.js | 21 ++--- src/gui/src/UI/UIDesktop.js | 31 ++++--- src/gui/src/helpers/notificationTap.js | 84 ++++++++----------- src/gui/src/helpers/notificationTap.test.js | 81 +++++++++++++----- src/gui/src/helpers/pushWorker.js | 3 +- src/gui/src/helpers/pushWorker.test.js | 10 +-- 6 files changed, 129 insertions(+), 101 deletions(-) diff --git a/src/gui/src/UI/Dashboard/UIDashboardNotifications.js b/src/gui/src/UI/Dashboard/UIDashboardNotifications.js index 1d5d6b6a1..36ff9a775 100644 --- a/src/gui/src/UI/Dashboard/UIDashboardNotifications.js +++ b/src/gui/src/UI/Dashboard/UIDashboardNotifications.js @@ -26,7 +26,7 @@ import { handleNotificationTap, openAppForTap, setPushTapHandler, - storedTapItem, + storedTapHandler, tapRoute, } from '../../helpers/notificationTap.js'; import { showLocalBanner, startPushWorker } from '../../helpers/pushWorker.js'; @@ -498,6 +498,15 @@ export default function UIDashboardNotifications ({ $el_window, socket }) { void handleNotificationTap({ uid: entry.uid, notification: entry.notification, appUid: entry.appUid }, tapSurface); }; + const actOnStored = storedTapHandler({ + fetchRow: getNotification, + surface: tapSurface, + found: (uid) => void acknowledge(uid).catch((err) => { + console.warn('Could not acknowledge notification:', err); + }), + missing: () => open(), + }); + const actOnPush = async (push) => { await reveal_dashboard(); if ( ! loaded ) await refresh(); @@ -506,15 +515,7 @@ export default function UIDashboardNotifications ({ $el_window, socket }) { actOn(entry); return; } - const tap = push.stored ? await storedTapItem(push.uid, getNotification) : { uid: push.uid, push }; - if ( ! tap ) { - open(); - return; - } - void acknowledge(push.uid).catch((err) => { - console.warn('Could not acknowledge notification:', err); - }); - await handleNotificationTap(tap, tapSurface); + await actOnStored(push); }; // -- Toasts ------------------------------------------------------------- diff --git a/src/gui/src/UI/UIDesktop.js b/src/gui/src/UI/UIDesktop.js index cd6a0da91..7d8552ef2 100644 --- a/src/gui/src/UI/UIDesktop.js +++ b/src/gui/src/UI/UIDesktop.js @@ -51,7 +51,7 @@ import { handleNotificationTap, openAppForTap, setPushTapHandler, - storedTapItem, + storedTapHandler, } from '../helpers/notificationTap.js'; import { showLocalBanner, startPushWorker } from '../helpers/pushWorker.js'; import { get_app_by_uid } from '../helpers/getAppByUid.js'; @@ -251,26 +251,25 @@ async function UIDesktop (options) { }; // The desktop has no notification list, so other types only clear. - const tap_notification = (tap) => handleNotificationTap(tap, { + const tap_surface = { share: (notification) => share_notification_click(notification)?.(), openApp: (route) => openAppForTap(route, { getApp: get_app_by_uid, launch: launch_app }), - }); + }; + const tap_notification = (tap) => handleNotificationTap(tap, tap_surface); const toasts_for = (uid) => $(elementsWithData($('.notification').get(), 'uid', uid)); - setPushTapHandler(async (push) => { - let tap = { uid: push.uid, push }; - if ( push.stored ) { - tap = await storedTapItem(push.uid, getNotification); - if ( ! tap ) return; - } - toasts_for(push.uid).remove(); - update_tab_notif_count_badge(); - void markNotificationAcknowledged(push.uid).catch((err) => { - console.warn('Could not acknowledge notification:', err); - }); - return tap_notification(tap); - }); + setPushTapHandler(storedTapHandler({ + fetchRow: getNotification, + surface: tap_surface, + found: (uid) => { + toasts_for(uid).remove(); + update_tab_notif_count_badge(); + void markNotificationAcknowledged(uid).catch((err) => { + console.warn('Could not acknowledge notification:', err); + }); + }, + })); startPushWorker(); /** diff --git a/src/gui/src/helpers/notificationTap.js b/src/gui/src/helpers/notificationTap.js index 31b3056f0..36dbbb866 100644 --- a/src/gui/src/helpers/notificationTap.js +++ b/src/gui/src/helpers/notificationTap.js @@ -38,11 +38,10 @@ const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : */ /** + * A tap from the worker or the URL names only a notification; what it leads + * to is read off the signed-in user's own row. * @typedef {Object} PushTap * @property {string} uid - * @property {{ app?: string, path?: string }} [target] - * @property {unknown} [data] - * @property {true} [stored] - Only the uid is known; act on the stored row */ /** @@ -52,60 +51,37 @@ const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : */ /** - * @param {{ uid?: string, notification?: Object|null, appUid?: string|null, push?: PushTap }} tap + * @param {{ uid?: string, notification?: Object|null, appUid?: string|null }} tap * @returns {TapRoute} */ -export const tapRoute = ({ uid, notification, appUid, push }) => { - if ( notification && typeof notification === 'object' ) { - if ( notification.type === APP_MESSAGE_TYPE ) { - const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); - if ( ! app ) return { kind: 'panel' }; - const target = typeof notification.target === 'string' ? notification.target : undefined; - return { kind: 'app', appUid: app, click: { uid, target, data: notification.data } }; - } - if ( isShareNotification(notification) ) return { kind: 'share', notification }; - return { kind: 'panel' }; - } - const app = nonEmpty(push?.target?.app); - if ( app ) { - return { kind: 'app', appUid: app, click: { uid, target: push.target.path, data: push.data } }; - } - const path = push?.target?.path; - if ( typeof path === 'string' && path.startsWith('/') ) { - return { - kind: 'share', - notification: { type: 'share.received', fields: { target: { path } } }, - }; +export const tapRoute = ({ uid, notification, appUid }) => { + if ( ! notification || typeof notification !== 'object' ) return { kind: 'panel' }; + if ( notification.type === APP_MESSAGE_TYPE ) { + const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); + if ( ! app ) return { kind: 'panel' }; + const target = typeof notification.target === 'string' ? notification.target : undefined; + return { kind: 'app', appUid: app, click: { uid, target, data: notification.data } }; } + if ( isShareNotification(notification) ) return { kind: 'share', notification }; return { kind: 'panel' }; }; +/** @returns {PushTap|null} */ +const tapFor = (uid) => ( + typeof uid === 'string' && uid.length <= UID_MAX && UID_PATTERN.test(uid) ? { uid } : null +); + /** * @param {unknown} raw - The worker's `notification` object - * @returns {PushTap|null} + * @returns {PushTap|null} The notification it names; nothing else it carries */ -export const normalizePushTap = (raw) => { - if ( ! raw || typeof raw !== 'object' ) return null; - if ( ! nonEmpty(raw.uid) || raw.uid.length > UID_MAX ) return null; - const target = {}; - if ( nonEmpty(raw.target?.app) ) target.app = raw.target.app; - if ( nonEmpty(raw.target?.path) ) target.path = raw.target.path; - return { - uid: raw.uid, - ...(Object.keys(target).length ? { target } : {}), - ...(raw.data !== undefined ? { data: raw.data } : {}), - }; -}; +export const normalizePushTap = (raw) => (raw && typeof raw === 'object' ? tapFor(raw.uid) : null); /** * @param {URLSearchParams} [params] - * @returns {PushTap|null} A tap naming only a notification uid + * @returns {PushTap|null} */ -export const pushTapFromParams = (params) => { - const uid = params?.get?.(PUSH_TAP_PARAM); - if ( typeof uid !== 'string' || uid.length > UID_MAX || ! UID_PATTERN.test(uid) ) return null; - return { uid, stored: true }; -}; +export const pushTapFromParams = (params) => tapFor(params?.get?.(PUSH_TAP_PARAM)); /** * @param {string} uid @@ -205,6 +181,21 @@ export const handleNotificationTap = async (tap, { share, panel, openApp }) => { return panel?.(); }; +/** + * @param {{ fetchRow: (uid: string) => Promise, + * surface: { share: Function, panel?: Function, openApp: Function }, + * found?: (uid: string) => unknown, missing?: () => unknown }} deps + * `found` runs before routing a row that was read; `missing` instead of + * routing when there is none + * @returns {(tap: PushTap) => Promise} + */ +export const storedTapHandler = ({ fetchRow, surface, found, missing }) => async ({ uid }) => { + const tap = await storedTapItem(uid, fetchRow); + if ( ! tap ) return missing?.(); + found?.(uid); + return handleNotificationTap(tap, surface); +}; + /** @type {((tap: PushTap) => unknown) | null} */ let pushTapHandler = null; /** @type {PushTap[]} */ @@ -223,8 +214,5 @@ const deliverPushTap = (tap) => { else pendingPushTaps.push(tap); }; -/** @param {unknown} raw - The worker's `notification` object */ +/** @param {unknown} raw - The worker's `notification` object, or a tap from the URL */ export const dispatchPushTap = (raw) => deliverPushTap(normalizePushTap(raw)); - -/** @param {PushTap} tap - From {@link pushTapFromParams} */ -export const dispatchStoredTap = (tap) => deliverPushTap(tap?.stored ? tap : null); diff --git a/src/gui/src/helpers/notificationTap.test.js b/src/gui/src/helpers/notificationTap.test.js index e8d9bffd2..f84063c51 100644 --- a/src/gui/src/helpers/notificationTap.test.js +++ b/src/gui/src/helpers/notificationTap.test.js @@ -24,12 +24,15 @@ import { normalizePushTap, openAppForTap, pushTapFromParams, + storedTapHandler, storedTapItem, tapRoute, withoutNotificationClick, } from './notificationTap.js'; const UID = '0b6a2c1e-6f1d-4a57-9c1e-3d2f8a9b7c10'; +const OTHER_UID = '7d3e9f10-2b4c-4d5e-8f6a-1b2c3d4e5f60'; +const THIRD_UID = 'c1d2e3f4-5a6b-4c7d-8e9f-0a1b2c3d4e5f'; const appMessage = (over = {}) => ({ type: 'app.message', @@ -75,24 +78,22 @@ describe('tapRoute', () => { .toEqual({ kind: 'panel' }); }); - it('reads a push tap: an app target is the app, a path is a shared item', () => { + it('is the list without a notification, whatever else the tap carries', () => { expect(tapRoute({ uid: 'n1', push: { uid: 'n1', target: { app: 'app-1', path: '/rooms/7' }, data: 5 } })) - .toEqual({ kind: 'app', appUid: 'app-1', click: { uid: 'n1', target: '/rooms/7', data: 5 } }); - expect(tapRoute({ uid: 'n1', push: { uid: 'n1', target: { path: '/alice/Shared/a.txt' } } })).toEqual({ - kind: 'share', - notification: { type: 'share.received', fields: { target: { path: '/alice/Shared/a.txt' } } }, - }); - expect(tapRoute({ uid: 'n1', push: { uid: 'n1' } })).toEqual({ kind: 'panel' }); + .toEqual({ kind: 'panel' }); + expect(tapRoute({ uid: 'n1', target: { app: 'app-1' }, notification: null })).toEqual({ kind: 'panel' }); + expect(tapRoute({ uid: 'n1' })).toEqual({ kind: 'panel' }); }); }); describe('normalizePushTap', () => { - it('keeps the expected fields and drops the rest', () => { - expect(normalizePushTap({ uid: 'n1', target: { app: 'a', path: 7, x: 1 }, data: null, extra: 1 })) - .toEqual({ uid: 'n1', target: { app: 'a' }, data: null }); + it('keeps the notification uid and nothing else', () => { + expect(normalizePushTap({ uid: UID, target: { app: 'a', path: '/x' }, data: null, extra: 1 })) + .toEqual({ uid: UID }); expect(normalizePushTap({ uid: '' })).toBeNull(); - expect(normalizePushTap({ uid: 'x'.repeat(65) })).toBeNull(); - expect(normalizePushTap('n1')).toBeNull(); + expect(normalizePushTap({ uid: 'n1' })).toBeNull(); + expect(normalizePushTap({ uid: `${UID}${'0'.repeat(40)}` })).toBeNull(); + expect(normalizePushTap(UID)).toBeNull(); }); }); @@ -100,7 +101,7 @@ describe('pushTapFromParams', () => { const params = (value) => new URLSearchParams({ 'puter.notification': value }); it('reads a notification uid, and nothing else, from the URL', () => { - expect(pushTapFromParams(params(UID))).toEqual({ uid: UID, stored: true }); + expect(pushTapFromParams(params(UID))).toEqual({ uid: UID }); }); it('takes nothing that is not a notification uid', () => { @@ -251,17 +252,57 @@ describe('handleNotificationTap', () => { }); }); +describe('storedTapHandler', () => { + const surface = () => ({ share: vi.fn(), panel: vi.fn(), openApp: vi.fn() }); + const row = { uid: UID, value: appMessage({ target: '/rooms/9', data: { room: 9 } }), app_uid: 'app-row' }; + const rows = async (uid) => (uid === UID ? row : null); + + it('routes from the row the tap names, after `found`', async () => { + const deps = { fetchRow: vi.fn(rows), surface: surface(), found: vi.fn(), missing: vi.fn() }; + await storedTapHandler(deps)({ uid: UID }); + expect(deps.fetchRow).toHaveBeenCalledWith(UID); + expect(deps.found).toHaveBeenCalledWith(UID); + expect(deps.surface.openApp).toHaveBeenCalledWith({ + kind: 'app', + appUid: 'app-row', + click: { uid: UID, target: '/rooms/9', data: { room: 9 } }, + }); + expect(deps.missing).not.toHaveBeenCalled(); + }); + + it('opens nothing for a row the user cannot read, running `missing` instead', async () => { + const deps = { fetchRow: vi.fn(rows), surface: surface(), found: vi.fn(), missing: vi.fn() }; + await storedTapHandler(deps)({ uid: OTHER_UID }); + expect(deps.missing).toHaveBeenCalledOnce(); + expect(deps.found).not.toHaveBeenCalled(); + expect(deps.surface.openApp).not.toHaveBeenCalled(); + expect(deps.surface.panel).not.toHaveBeenCalled(); + expect(deps.surface.share).not.toHaveBeenCalled(); + }); +}); + describe('push tap dispatch', () => { it('holds taps until a handler is set, then hands them over in order', async () => { - const { dispatchPushTap, dispatchStoredTap, setPushTapHandler } = await import('./notificationTap.js'); - dispatchPushTap({ uid: 'n1' }); + const { dispatchPushTap, setPushTapHandler } = await import('./notificationTap.js'); + dispatchPushTap({ uid: UID }); dispatchPushTap({ uid: '' }); - dispatchStoredTap({ uid: UID, stored: true }); - dispatchStoredTap({ uid: UID }); + dispatchPushTap({ uid: 'n2' }); + dispatchPushTap({ uid: OTHER_UID, stored: true }); const handler = vi.fn(); setPushTapHandler(handler); - expect(handler.mock.calls).toEqual([[{ uid: 'n1' }], [{ uid: UID, stored: true }]]); - dispatchPushTap({ uid: 'n3' }); - expect(handler).toHaveBeenLastCalledWith({ uid: 'n3' }); + expect(handler.mock.calls).toEqual([[{ uid: UID }], [{ uid: OTHER_UID }]]); + dispatchPushTap({ uid: THIRD_UID }); + expect(handler).toHaveBeenLastCalledWith({ uid: THIRD_UID }); + }); + + it('hands a worker tap over as the notification it names, nothing more', async () => { + const { dispatchPushTap, setPushTapHandler, storedTapHandler: make } = await import('./notificationTap.js'); + const openApp = vi.fn(); + const fetchRow = vi.fn(async () => null); + setPushTapHandler(make({ fetchRow, surface: { share: vi.fn(), openApp } })); + dispatchPushTap({ uid: OTHER_UID, target: { app: 'app-1', path: '/rooms/7' }, data: { room: 7 } }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(fetchRow).toHaveBeenCalledWith(OTHER_UID); + expect(openApp).not.toHaveBeenCalled(); }); }); diff --git a/src/gui/src/helpers/pushWorker.js b/src/gui/src/helpers/pushWorker.js index e0783189c..b1236a6d9 100644 --- a/src/gui/src/helpers/pushWorker.js +++ b/src/gui/src/helpers/pushWorker.js @@ -23,7 +23,6 @@ import { httpsIconFor, isShareNotification } from '../UI/Dashboard/notificationC import { APP_MESSAGE_TYPE, dispatchPushTap, - dispatchStoredTap, normalizePushTap, pushTapFromParams, PUSH_TAP_PARAM, @@ -144,7 +143,7 @@ export const startPushWorker = async ({ if ( query?.has?.(PUSH_TAP_PARAM) ) { const opened = pushTapFromParams(query); clearParam(PUSH_TAP_PARAM); - if ( opened ) dispatchStoredTap(opened); + if ( opened ) dispatchPushTap(opened); } // Leave an existing worker: removing it ends this browser's subscription. diff --git a/src/gui/src/helpers/pushWorker.test.js b/src/gui/src/helpers/pushWorker.test.js index 7e2359f27..7cd5d25d3 100644 --- a/src/gui/src/helpers/pushWorker.test.js +++ b/src/gui/src/helpers/pushWorker.test.js @@ -210,14 +210,14 @@ describe('startPushWorker', () => { expect(nav.serviceWorker.register).toHaveBeenCalledWith('/sw.js', { scope: '/' }); const source = { postMessage: vi.fn() }; - nav.listeners.message({ data: { type: 'other', notification: { uid: 'x' } }, source }); - nav.listeners.message({ data: { type: 'puter-notification-click', id: 'click-0', notification: { uid: '' } }, source }); + nav.listeners.message({ data: { type: 'other', notification: { uid: UID } }, source }); + nav.listeners.message({ data: { type: 'puter-notification-click', id: 'click-0', notification: { uid: 'n1' } }, source }); nav.listeners.message({ - data: { type: 'puter-notification-click', id: 'click-1', notification: { uid: 'n1', target: { app: 'a' } } }, + data: { type: 'puter-notification-click', id: 'click-1', notification: { uid: UID, target: { app: 'a' } } }, source, }); expect(source.postMessage.mock.calls).toEqual([[{ type: 'puter-notification-click-ack', id: 'click-1' }]]); - expect(handler.mock.calls).toEqual([[{ uid: 'n1', target: { app: 'a' } }]]); + expect(handler.mock.calls).toEqual([[{ uid: UID }]]); }); it('registers nothing inside a frame', async () => { @@ -245,7 +245,7 @@ describe('startPushWorker', () => { clearParam, }); expect(clearParam).toHaveBeenCalledWith('puter.notification'); - expect(handler).toHaveBeenCalledWith({ uid: UID, stored: true }); + expect(handler).toHaveBeenCalledWith({ uid: UID }); }); it('clears a malformed parameter and hands nothing over', async () => { From 5f7ab80721dfb947e7ca81e3a9780ed575a441f6 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:17:48 -0400 Subject: [PATCH 6/8] fix(gui): local banner tags carry the app An app message's group is tagged under its app, as the server tags a push, so two apps using the same group name keep separate banners. --- src/gui/src/helpers/pushWorker.js | 5 ++++- src/gui/src/helpers/pushWorker.test.js | 12 +++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/gui/src/helpers/pushWorker.js b/src/gui/src/helpers/pushWorker.js index b1236a6d9..8e462ef43 100644 --- a/src/gui/src/helpers/pushWorker.js +++ b/src/gui/src/helpers/pushWorker.js @@ -68,9 +68,12 @@ const nonEmpty = (value) => (typeof value === 'string' && value.trim() !== '' ? * @returns {Object} The push payload shape `sw.js` shows */ export const pushMessageFor = (uid, notification, appUid = null) => { - const message = { uid, group: nonEmpty(notification.group) ?? uid }; + const group = nonEmpty(notification.group); + const message = { uid, group: group ?? uid }; if ( notification.type === APP_MESSAGE_TYPE ) { const app = nonEmpty(appUid) ?? nonEmpty(notification.app?.uid); + // An app's group is its own, as the server names it on a push. + if ( app && group ) message.group = `${app}:${group}`; if ( notification.sensitive === true ) { return { ...message, title: nonEmpty(notification.app?.name), ...(app ? { target: { app } } : {}) }; } diff --git a/src/gui/src/helpers/pushWorker.test.js b/src/gui/src/helpers/pushWorker.test.js index 7cd5d25d3..c33dfe80f 100644 --- a/src/gui/src/helpers/pushWorker.test.js +++ b/src/gui/src/helpers/pushWorker.test.js @@ -74,7 +74,7 @@ describe('pushMessageFor', () => { app: { uid: 'app-1', name: 'Chat' }, })).toEqual({ uid: 'n1', - group: 'room-7', + group: 'app-1:room-7', title: 'New reply', body: 'see you', icon: 'https://example.com/i.png', @@ -102,6 +102,16 @@ describe('pushMessageFor', () => { }); }); + it("tags an app message's group with its app, so two apps never share one", () => { + const message = (app, over = {}) => ({ type: 'app.message', title: 't', group: 'room-7', app: { uid: app }, ...over }); + expect(pushMessageFor('n1', message('app-1')).group).toBe('app-1:room-7'); + expect(pushMessageFor('n1', message('app-2')).group).toBe('app-2:room-7'); + expect(pushMessageFor('n1', message('app-1', { sensitive: true })).group).toBe('app-1:room-7'); + expect(pushMessageFor('n1', message('app-b'), 'app-a').group).toBe('app-a:room-7'); + expect(pushMessageFor('n1', message('app-1', { group: undefined })).group).toBe('n1'); + expect(pushMessageFor('n2', { ...share, group: 'room-7' }).group).toBe('room-7'); + }); + it("uses the row's app uid over the one the message names", () => { expect(pushMessageFor('n1', { type: 'app.message', title: 't' }, 'app-9').target).toEqual({ app: 'app-9' }); expect(pushMessageFor('n1', { type: 'app.message', title: 't', app: { uid: 'app-b' } }, 'app-a').target) From 7aa4d91ae950b5695aa82b18e1d16d9da8a58b9d Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:17:57 -0400 Subject: [PATCH 7/8] fix(gui): a tap delivered twice routes once The page remembers the notifications it handed over in the last ten seconds and drops a repeat, whichever way it arrives. --- src/gui/src/helpers/notificationTap.js | 15 ++++++++++++++- src/gui/src/helpers/notificationTap.test.js | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/gui/src/helpers/notificationTap.js b/src/gui/src/helpers/notificationTap.js index 36dbbb866..3c9420f3c 100644 --- a/src/gui/src/helpers/notificationTap.js +++ b/src/gui/src/helpers/notificationTap.js @@ -27,6 +27,8 @@ export const PUSH_TAP_PARAM = 'puter.notification'; const UID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; const UID_MAX = 64; +// A tap for a uid acted on this recently is the same tap again. +const TAP_REPEAT_MS = 10_000; const nonEmpty = (value) => (typeof value === 'string' && value !== '' ? value : undefined); @@ -200,6 +202,8 @@ export const storedTapHandler = ({ fetchRow, surface, found, missing }) => async let pushTapHandler = null; /** @type {PushTap[]} */ const pendingPushTaps = []; +/** @type {Map} uid → when it was last handed over */ +const recentTaps = new Map(); /** @param {(tap: PushTap) => unknown} handler */ export const setPushTapHandler = (handler) => { @@ -207,9 +211,18 @@ export const setPushTapHandler = (handler) => { for ( const tap of pendingPushTaps.splice(0) ) void handler(tap); }; +const seenLately = (uid, now) => { + for ( const [seen, at] of recentTaps ) { + if ( now - at >= TAP_REPEAT_MS ) recentTaps.delete(seen); + } + if ( recentTaps.has(uid) ) return true; + recentTaps.set(uid, now); + return false; +}; + /** @param {PushTap|null} tap */ const deliverPushTap = (tap) => { - if ( ! tap ) return; + if ( ! tap || seenLately(tap.uid, Date.now()) ) return; if ( pushTapHandler ) void pushTapHandler(tap); else pendingPushTaps.push(tap); }; diff --git a/src/gui/src/helpers/notificationTap.test.js b/src/gui/src/helpers/notificationTap.test.js index f84063c51..74a9488d6 100644 --- a/src/gui/src/helpers/notificationTap.test.js +++ b/src/gui/src/helpers/notificationTap.test.js @@ -282,6 +282,10 @@ describe('storedTapHandler', () => { }); describe('push tap dispatch', () => { + afterEach(() => { + vi.useRealTimers(); + }); + it('holds taps until a handler is set, then hands them over in order', async () => { const { dispatchPushTap, setPushTapHandler } = await import('./notificationTap.js'); dispatchPushTap({ uid: UID }); @@ -305,4 +309,21 @@ describe('push tap dispatch', () => { expect(fetchRow).toHaveBeenCalledWith(OTHER_UID); expect(openApp).not.toHaveBeenCalled(); }); + + it('hands the same notification over once within ten seconds', async () => { + vi.useFakeTimers(); + const { dispatchPushTap, setPushTapHandler } = await import('./notificationTap.js'); + const handler = vi.fn(); + setPushTapHandler(handler); + dispatchPushTap({ uid: UID }); + dispatchPushTap({ uid: UID }); + dispatchPushTap({ uid: OTHER_UID }); + expect(handler.mock.calls).toEqual([[{ uid: UID }], [{ uid: OTHER_UID }]]); + vi.advanceTimersByTime(9_999); + dispatchPushTap({ uid: UID }); + expect(handler).toHaveBeenCalledTimes(2); + vi.advanceTimersByTime(1); + dispatchPushTap({ uid: UID }); + expect(handler).toHaveBeenCalledTimes(3); + }); }); From 7e78a4feef502e13d584585b013da27106eaf9c5 Mon Sep 17 00:00:00 2001 From: Juan Castro Date: Tue, 27 Oct 2026 14:18:07 -0400 Subject: [PATCH 8/8] fix(gui): the service worker never reuses cached imports The worker is registered with updateViaCache set to none, so the scripts it imports are fetched fresh on every update check. --- src/gui/src/helpers/pushWorker.js | 3 ++- src/gui/src/helpers/pushWorker.test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gui/src/helpers/pushWorker.js b/src/gui/src/helpers/pushWorker.js index 8e462ef43..73cb78d62 100644 --- a/src/gui/src/helpers/pushWorker.js +++ b/src/gui/src/helpers/pushWorker.js @@ -157,7 +157,8 @@ export const startPushWorker = async ({ dispatchPushTap(event.data.notification); }); try { - await nav.serviceWorker.register(WORKER_URL, { scope: '/' }); + // Scripts the worker imports are fetched fresh on each update check. + await nav.serviceWorker.register(WORKER_URL, { scope: '/', updateViaCache: 'none' }); } catch ( err ) { console.warn('Could not register the service worker:', err); } diff --git a/src/gui/src/helpers/pushWorker.test.js b/src/gui/src/helpers/pushWorker.test.js index c33dfe80f..d233a46f6 100644 --- a/src/gui/src/helpers/pushWorker.test.js +++ b/src/gui/src/helpers/pushWorker.test.js @@ -217,7 +217,7 @@ describe('startPushWorker', () => { mod.setPushTapHandler(handler); const nav = makeNav(); await start(mod, { nav }); - expect(nav.serviceWorker.register).toHaveBeenCalledWith('/sw.js', { scope: '/' }); + expect(nav.serviceWorker.register).toHaveBeenCalledWith('/sw.js', { scope: '/', updateViaCache: 'none' }); const source = { postMessage: vi.fn() }; nav.listeners.message({ data: { type: 'other', notification: { uid: UID } }, source });