feat(push): VAPID Web Push voor queue-berichten #81
No reviewers
Labels
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
janpeter/scrum4me-workers!81
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/vapid-queue-push"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
OS-niveau notificaties op Mac en iPhone wanneer een s4m-queue-bericht binnenkomt of van status wisselt, ook met gesloten tab of app.
NIET MERGEN tot het testverslag erin staat. De deploy draait bewust vanaf deze branch, zodat de commits van taak 14/15 (AC1-AC9-bewijs) nog in dezelfde merge mee kunnen. Zou deze PR nu gemerged worden, dan bereikt dat verslag
mainnooit.Wat erin zit:
lib/push/payload.ts— envelope naar notificatie; nieuw bericht =previous_status === null(een requeue is dus geen nieuw bericht)lib/push/server.ts— verzenden, filter opapp_scope=workers+ rol ADMIN, ensendOnegeeft de HTTP-status terug: dat is de bron van het 201-bewijslib/push/queue-listener.ts+instrumentation.ts— persistente LISTEN met advisory lock, server-side keepalives en per message-id geserialiseerde sendsDockerfile—COPY instrumentation.ts ./vóór de build; zonder die regel zit de listener niet in de image terwijl de UI gewoon werktactions/push.ts,public/sw.js,lib/push/client.ts,components/push-toggle.tsx,proxy.ts(/sw.jsin PUBLIC_PATHS),app/layout.tsx(de Toaster ontbrak volledig)Ontwerp:
docs/superpowers/specs/2026-08-10-vapid-queue-push-notifications-design.md(vijf reviewrondes, dubbele GO).Plan:
docs/superpowers/plans/2026-08-10-vapid-queue-push-notifications.md.Voorwaarden uit de app_scope-keten zijn afgerond: scrum4me-shared #46 en Scrum4Me #146 gemerged, migratie toegepast en geverifieerd.
Het antwoord keert de route om (from = verzoek.to), dus AC4 verwachtte de verkeerde titel. Dat legde een echt gat bloot: de melding waar JP op wacht ('heb ik antwoord?') werd door geen enkel criterium geraakt. Mapping benoemt nu beide spiegelflows; AC9 toegevoegd voor de JP-naar-agent-flow. Verder: /sw.js toevoegen aan proxy.ts PUBLIC_PATHS (deny-by-default whitelist), en to_model='jp' i.p.v. hardcoded mac:jp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Verdict: COMMENT
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings:
lib/push/client.ts:50:unsubscribeFromPush()zegt de browser-subscription eerst lokaal op en roept daarna pasunsubscribeFromPushAction()aan. Als die server action faalt, is de client al uitgeschreven maar blijft de DB-rij bestaan. Dat levert stale subscriptions en misleidende UI/toast-status op; draai de volgorde om of maak de server cleanup expliciet retrybaar/idempotent voordat de browser-state definitief wordt aangepast.De rest van de diff volgt de admin-only policy (
requireWorkersAdminop actions/page), gebruikt de shared-submodule voor schemawijziging in plaats van lokale migrations, documenteert de VAPID-runtimekeuzes, en bevat gerichte Vitest-dekking voor payload-mapping, server actions, listener-lifecycle en proxy-public path.Verdict: APPROVED
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
De implementatie volgt de admin-only workers-standaard (
requireWorkersAdminop page/actions), gebruikt de canonical shared-schema route via submodule-bump in plaats van lokale migraties, en voegt passende testdekking toe voor server actions, payload-mapping, listener-lifecycle, push-send gedrag en proxy-public-paths.Server-side bewijs compleet: alle vier de bewijs-ids afgeleverd met status=201 en verzonden=1/1, alle negatieve controles schoon. De overgang van verzonden=0/0 naar 1/1 in dezelfde logstroom sluit '201 op nul ontvangers' uit. Bevinding: een bericht levert een melding per statuswisseling, niet per bericht — tot drie per bericht. Dat is ontwerpgedrag ('alle events'), maar nu meetbaar en dus een keuze die JP moet maken. De mismatch-hypothese uit de spec is onwaarschijnlijk geworden: push_subscriptions bevatte nul rijen, dus er is nooit een abonnement opgeslagen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>APPROVED
geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
/sw.jsis publiek, VAPID wordt runtime gelezen zonderNEXT_PUBLIC_,instrumentation.tswordt in de Docker-build meegenomen, subscriptions worden metapp_scope='workers'gescheiden, en de server actions blijven admin-only. De toegevoegde tests dekken payload-mapping, listener-lifecycle, send/delete-gedrag, proxy-uitzondering en action-auth.Verdict: COMMENT
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
app/api/push/resubscribe/route.ts:63: de cleanup vanoldEndpointfiltert alleen opendpoint+app_scope, niet opuser_id. Een geauthenticeerde workers-admin kan daardoor via een gemanipuleerde body een workers-pushsubscription van een andere admin verwijderen als het endpoint bekend is. Dit is geen functionele blocker voor aflevering, maar het doorbreekt de ownership-scope dieunsubscribeFromPushActionwel hanteert. Voeguser_id: session.userIdtoe of documenteer expliciet waarom admins elkaars subscriptions mogen opruimen.Samenvatting
De diff is breed maar coherent: VAPID-config blijft server-side, de public key wordt request-time doorgegeven,
/sw.jsis publiek bereikbaar voor service-worker updates, de listener gebruikt advisory locking om dubbele sends te vermijden, en de push-payload/server/listener lagen hebben gerichte tests. Er zijn ook docs en een testverslag toegevoegd. Door bovenstaande ownership-opmerking kies ikCOMMENTin plaats vanAPPROVED.COMMENT
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
public/sw.js:39/app/api/push/resubscribe/route.ts:27: depushsubscriptionchange-handler post naar een route dierequireWorkersAdmin()vereist. Dat is correct vanuit admin-only/auth-perspectief, maar beperkt precies de herstelcase waarvoor deze handler bedoeld is: subscription-rotatie kan plaatsvinden terwijl er geen geldige workers-sessie meer is, waarna de route401geeft en het nieuwe endpoint niet wordt opgeslagen. De reguliere subscribe-flow blijft werken zodra de admin de UI opnieuw opent, dus dit is geen blokkade, maar de code/docs suggereren nu sterker herstel dan de implementatie kan garanderen.Verdict
Geen blokkerende findings. De hoofdflow houdt zich aan de admin-only productregel (
requireWorkersAdminop page/action/route), gebruikt de gedeeldepush_subscriptions-scope expliciet, voegt gerichte tests toe voor actions, payloadmapping, listener en server-send, en documenteert de VAPID/deployvoorwaarden. Door bovenstaande betrouwbaarheidskanttekening kies ikCOMMENTin plaats vanAPPROVED.APPROVED
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
Reviewnotities
De diff volgt de relevante productstandaarden uit de docs: workers blijft admin-only (
requireWorkersAdminop page/action/route), schemawijzigingen lopen viascrum4me-shared/designated migrator in plaats van eigen workers-migraties, en de Dockerfile neemtinstrumentation.tsmee zodat de listener in standalone builds beschikbaar is.Testdekking is aanwezig voor de belangrijkste risico's: VAPID payload-mapping/filtering, push-send foutclassificatie en secret-safe logging, queue-listener serialisatie/reconnect, server actions, resubscribe-route en proxy-public-path voor
/sw.js. Docs zijn bijgewerkt voor configuratie, operationele regels en troubleshooting.Verdict: REQUEST_CHANGES
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
docs/reports/2026-08-10-vapid-push-testverslag.md:35: het eigen testverslag markeert AC5 (AC2+AC3+AC4 op iPhone met gesloten PWA) nog als Nog te doen. Voor deze PR is gesloten-PWA/iPhone-aflevering juist een kernclaim van de Web Push-feature; zonder dit bewijs blijft een belangrijk productiepad onverifieerd.docs/reports/2026-08-10-vapid-push-testverslag.md:37: AC7 (Twee listeners → precies één push per envelope) staat nog als Nog te doen. De implementatie gebruikt een deployment-breed advisory lock om dubbele sends te voorkomen; zonder expliciete multi-listener-verificatie is een centraal architectuurrisico nog open.docs/reports/2026-08-10-vapid-push-testverslag.md:38: AC8 (Uitgelogd → 307, geen rij, geen push) staat nog als Nog te doen. De feature voegt nieuwe subscribe/resubscribe/test-push ingangen toe; auth-/sessiegedrag moet aantoonbaar dicht zijn voordat dit mergebaar is.Samenvatting
De diff volgt de relevante productstandaarden grotendeels: runtime VAPID zonder
NEXT_PUBLIC_,instrumentation.tswordt in Docker meegenomen,/sw.jsis publiek via de proxy, subscriptions zijn gescopeerd metapp_scope='workers', en er is gerichte unit-testdekking voor payloadmapping, server-actions, resubscribe, listener-lifecycle en proxy. De PR is alleen niet approvebaar zolang het meegeleverde acceptatie-/testverslag nog expliciet open E2E-criteria bevat.APPROVED
Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.
Findings
De diff volgt de relevante productstandaarden: de nieuwe page/action/route-paden gebruiken
requireWorkersAdmin, er zijn geen lokale workers-migraties toegevoegd maar alleen een shared-submodule bump, en de PR bevat gerichte Vitest-dekking voor server actions, resubscribe-route, payload-mapping, listenergedrag, push-serverfouten en proxy-uitzondering. De Dockerfile neemtinstrumentation.tsexpliciet mee, wat past bij de Next.js runtime-aanpak voor de queue-listener.