From b21888fd7a9867fe3db218ac326d164de5739e69 Mon Sep 17 00:00:00 2001 From: timmy Date: Tue, 11 Aug 2026 09:57:05 +0000 Subject: [PATCH] feat: mark Web Push updates read (Closes #561) --- frontend/service-worker.js | 62 +++++++++++--- src/push_notifications.py | 1 + tests/test_push_notifications.py | 1 + tests/test_service_worker.py | 140 ++++++++++++++++++++++++++++++- 4 files changed, 187 insertions(+), 17 deletions(-) diff --git a/frontend/service-worker.js b/frontend/service-worker.js index b30f115..d703175 100644 --- a/frontend/service-worker.js +++ b/frontend/service-worker.js @@ -4,6 +4,7 @@ const CACHE = 'stackchain-dashboard-shell-v88'; const OFFLINE_LEASE_URL = new URL(BASE + '__offline-session-lease', self.location.origin).href; const OUTAGE_STATUSES = new Set([500, 502, 503, 504]); const NAVIGATION_TIMEOUT_MS = self.__STACKCHAIN_NAVIGATION_TIMEOUT_MS || 4000; +const PUSH_ACTION_TIMEOUT_MS = self.__STACKCHAIN_PUSH_ACTION_TIMEOUT_MS || 8000; const SHELL = [ BASE, BASE + 'manifest.webmanifest', @@ -276,26 +277,61 @@ self.addEventListener('push', event => { catch (_error) { return; } const route = String(payload.route || ''); const tag = String(payload.tag || ''); + const notificationId = Number(payload.notification_id); if (!/^#\/my-work\/update\/\d+$/.test(route) || !/^stackchain-update-\d+$/.test(tag)) return; - event.waitUntil(self.registration.showNotification('New work update', { + const options = { body: 'Tap to review it in Stackchain.', tag, data: {route}, - })); + }; + if ( + Number.isSafeInteger(notificationId) && notificationId > 0 + && route === '#/my-work/update/' + notificationId + && tag === 'stackchain-update-' + notificationId + ) { + options.actions = [{ action: 'mark-read', title: 'Mark read' }]; + options.data.notificationId = notificationId; + } + event.waitUntil(self.registration.showNotification('New work update', options)); }); -self.addEventListener('notificationclick', event => { - event.notification.close(); - const route = String(event.notification.data?.route || ''); - if (!route.startsWith('#/my-work/')) return; +async function openWorkRoute(route) { const target = new URL(BASE + route, self.location.origin).href; - event.waitUntil((async () => { - const windows = await self.clients.matchAll({ type: 'window', includeUncontrolled: true }); - const client = windows.find(candidate => candidate.url.startsWith(self.location.origin + BASE)); - if (!client) return self.clients.openWindow(target); - if (client.navigate) await client.navigate(target); - return client.focus(); - })()); + const windows = await self.clients.matchAll({ type: 'window', includeUncontrolled: true }); + const client = windows.find(candidate => candidate.url.startsWith(self.location.origin + BASE)); + if (!client) return self.clients.openWindow(target); + if (client.navigate) await client.navigate(target); + return client.focus(); +} + +self.addEventListener('notificationclick', event => { + const route = String(event.notification.data?.route || ''); + if (event.action === 'mark-read') { + const notificationId = Number(event.notification.data?.notificationId); + if ( + !Number.isSafeInteger(notificationId) || notificationId <= 0 + || route !== '#/my-work/update/' + notificationId + ) return; + event.waitUntil((async () => { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), PUSH_ACTION_TIMEOUT_MS); + try { + await fetchJson(BASE + 'api/v1/notifications/' + notificationId + '/read', { + method: 'PATCH', headers: { Accept: 'application/json' }, signal: controller.signal, + }); + event.notification.close(); + } catch (_error) { + await openWorkRoute(route); + event.notification.close(); + } finally { + clearTimeout(timeout); + } + })()); + return; + } + event.notification.close(); + if (!route.startsWith('#/my-work/')) return; + event.waitUntil(openWorkRoute(route)); }); self.addEventListener('fetch', event => { diff --git a/src/push_notifications.py b/src/push_notifications.py index a8cafde..9a84c8a 100644 --- a/src/push_notifications.py +++ b/src/push_notifications.py @@ -103,6 +103,7 @@ async def dispatch_unread_updates( "body": "Tap to review it in Stackchain.", "route": f"#/my-work/update/{thread_id}", "tag": f"stackchain-update-{thread_id}", + "notification_id": thread_id, }, separators=(",", ":"), ) diff --git a/tests/test_push_notifications.py b/tests/test_push_notifications.py index 7eef745..626cd4d 100644 --- a/tests/test_push_notifications.py +++ b/tests/test_push_notifications.py @@ -141,6 +141,7 @@ async def test_dispatch_sends_one_privacy_safe_deep_link_per_new_thread(tmp_path "body": "Tap to review it in Stackchain.", "route": "#/my-work/update/42", "tag": "stackchain-update-42", + "notification_id": 42, } assert "private/repo" not in json.dumps(sent) assert "Secret title" not in json.dumps(sent) diff --git a/tests/test_service_worker.py b/tests/test_service_worker.py index 745074f..419358e 100644 --- a/tests/test_service_worker.py +++ b/tests/test_service_worker.py @@ -41,6 +41,7 @@ const context = {{ self: {{ location: {{ href: 'https://forge.example/dashboard/service-worker.js', origin: 'https://forge.example' }}, __STACKCHAIN_NAVIGATION_TIMEOUT_MS: 15, + __STACKCHAIN_PUSH_ACTION_TIMEOUT_MS: 15, addEventListener: (name, handler) => {{ listeners[name] = handler; }}, skipWaiting: async () => {{ state.skipped = true; }}, clients: {{ @@ -104,10 +105,11 @@ async function dispatchMessage(data, ports = []) {{ listeners.message({{ data, ports, waitUntil: promise => {{ pending = promise; }} }}); if (pending) await pending; }} -async function dispatchNotificationClick(route) {{ +async function dispatchNotificationClick(route, action = '', notificationId = null) {{ let pending; listeners.notificationclick({{ - notification: {{data: {{route}}, close: () => {{ state.notificationClosed = true; }}}}, + action, + notification: {{data: {{route, notificationId}}, close: () => {{ state.notificationClosed = true; }}}}, waitUntil: promise => {{ pending = promise; }}, }}); if (pending) await pending; @@ -386,7 +388,7 @@ def test_inbound_push_renders_generic_update_and_tap_opens_existing_workflow(): """ await dispatchPush({ title:'New work update', body:'Tap to review it in Stackchain.', - tag:'stackchain-update-42', route:'#/my-work/update/42', + tag:'stackchain-update-42', route:'#/my-work/update/42', notification_id:42, repository:'must-not-render', }); await dispatchNotificationClick('#/my-work/update/42'); @@ -399,7 +401,11 @@ def test_inbound_push_renders_generic_update_and_tap_opens_existing_workflow(): "options": { "body": "Tap to review it in Stackchain.", "tag": "stackchain-update-42", - "data": {"route": "#/my-work/update/42"}, + "actions": [{"action": "mark-read", "title": "Mark read"}], + "data": { + "route": "#/my-work/update/42", + "notificationId": 42, + }, }, }] assert result["opened"] == [ @@ -408,6 +414,132 @@ def test_inbound_push_renders_generic_update_and_tap_opens_existing_workflow(): assert "must-not-render" not in json.dumps(result["notifications"]) +def test_push_mark_read_action_confirms_authenticated_mutation_without_opening_app(): + result = run_worker_scenario( + """ + const calls = []; + context.fetch = async (request, options = {}) => { + const url = String(request.url || request); + const headers = new Headers(options.headers || {}); + calls.push({url, method:String(options.method || 'GET'), csrf:headers.get('X-CSRF-Token')}); + if (url.endsWith('/api/v1/session')) { + return new Response(JSON.stringify({csrf_token:'session-proof'}), { + status:200, headers:{'Content-Type':'application/json'}, + }); + } + return new Response(JSON.stringify({ok:true}), { + status:200, headers:{'Content-Type':'application/json'}, + }); + }; + await dispatchNotificationClick('#/my-work/update/42', 'mark-read', 42); + process.stdout.write(JSON.stringify({state,calls})); +""" + ) + + assert result["calls"] == [ + { + "url": "https://forge.example/dashboard/api/v1/session", + "method": "GET", + "csrf": None, + }, + { + "url": "https://forge.example/dashboard/api/v1/notifications/42/read", + "method": "PATCH", + "csrf": "session-proof", + }, + ] + assert result["state"]["notificationClosed"] is True + assert result["state"]["opened"] == [] + assert result["state"]["focused"] == [] + + +def test_push_mark_read_failure_opens_existing_update_reader(): + result = run_worker_scenario( + """ + state.clientList = [{ + url:'https://forge.example/dashboard/', + navigate:async function(url){ this.url=url; }, + focus:async function(){ state.focused.push(this.url); }, + }]; + context.fetch = async (request, options = {}) => { + const url = String(request.url || request); + if (url.endsWith('/api/v1/session')) { + return new Response(JSON.stringify({csrf_token:'session-proof'}), { + status:200, headers:{'Content-Type':'application/json'}, + }); + } + return new Response(JSON.stringify({error:'unavailable'}), { + status:503, headers:{'Content-Type':'application/json'}, + }); + }; + await dispatchNotificationClick('#/my-work/update/42', 'mark-read', 42); + process.stdout.write(JSON.stringify(state)); +""" + ) + + assert result["focused"] == [ + "https://forge.example/dashboard/#/my-work/update/42" + ] + assert result["opened"] == [] + assert result["notificationClosed"] is True + + +def test_push_mark_read_rejects_mismatched_notification_identity(): + result = run_worker_scenario( + """ + const calls = []; + context.fetch = async (request, options = {}) => { + calls.push({url:String(request.url || request), method:String(options.method || 'GET')}); + return new Response(JSON.stringify({csrf_token:'session-proof'}), { + status:200, headers:{'Content-Type':'application/json'}, + }); + }; + await dispatchNotificationClick('#/my-work/update/42', 'mark-read', 43); + process.stdout.write(JSON.stringify({state,calls})); +""" + ) + + assert result["calls"] == [] + assert result["state"].get("notificationClosed") is not True + assert result["state"]["opened"] == [] + + +def test_push_mark_read_timeout_aborts_request_and_opens_update_reader(): + result = run_worker_scenario( + """ + state.clientList = [{ + url:'https://forge.example/dashboard/', + navigate:async function(url){ this.url=url; }, + focus:async function(){ state.focused.push(this.url); }, + }]; + context.fetch = async (request, options = {}) => { + const url = String(request.url || request); + if (url.endsWith('/api/v1/session')) { + return new Response(JSON.stringify({csrf_token:'session-proof'}), { + status:200, headers:{'Content-Type':'application/json'}, + }); + } + return new Promise((resolve, reject) => { + options.signal?.addEventListener('abort', () => { + state.actionFetchAborted = true; + reject(new Error('aborted')); + }, {once:true}); + }); + }; + dispatchNotificationClick('#/my-work/update/42', 'mark-read', 42); + await new Promise(resolve => setTimeout(resolve, 50)); + process.stdout.write(JSON.stringify(state)); + process.exit(0); +""" + ) + + assert result["actionFetchAborted"] is True + assert result["focused"] == [ + "https://forge.example/dashboard/#/my-work/update/42" + ] + assert result["notificationClosed"] is True + + def test_background_mutations_obtain_session_bound_csrf_proof(): source = WORKER.read_text()