pneumo

M3: PWA, service worker and browser notifications (ENG-262) #25

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-262-m3-pwa-service-worker-and-browser-notifications

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-262-m3-pwa-service-worker-and-browser-notifications

Linear: https://linear.app/engineereddev/issue/ENG-262

What's in this PR (24 files, all under apps/web/ plus pnpm-lock.yaml):

Manifest (Pneumo, brand icons incl. maskable, theme/background from tokens, display: standalone, start_url: /); vite-plugin-pwa 2.0.0 in injectManifest mode with src/sw.ts; app-shell precache (22 entries) and offline navigation to the shell.
sw.ts + pure sw-logic.ts: push handler for inline push and ref payloads (token and contentKey from the IndexedDB key store), §10 notification options (title default, body, tag = push id, brand icon/badge, open link action), generic "New push" fallback without keys; notificationclick routing (open → openWindow; default → focus + show-push message, else /?push=<id>); pushsubscriptionchange re-subscribe + PUT.
Settings "Browser notifications" toggle (subscribe + PUT /api/devices/self/webpush, unsubscribe + DELETE, denied/unsupported states); sign-out unsubscribes; one-time Home prompt "Get notified in this browser"; "Reload to update" toast for a waiting worker.
Tests: vitest for sw-logic and the toggle; Playwright for manifest/SW control, a real encrypted payload through the SW push handler with notifications asserted, toggle persistence, the update flow, axe and screenshots.

Verify (lead, rebased on eca40d7):

mise run check:web → exit 0 (105 tests, build OK, dist/sw.js generated)
mise run e2e:web → exit 0 (27 passed)
Implementer: CDP Page.getInstallabilityErrors → [] (Lighthouse 12 has no PWA category).

Deviations: PWA tests use temporary persistent full-Chromium profiles (headless shell and incognito block the native APIs); synthetic notification clicks stand in for activation-gated focus/open. Real push-service delivery stays the M5 check on the owner's browser.

🤖 Generated with Claude Code

Commits 0

Could not load commits.

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, REQUEST_CHANGES #

The PWA work is correct and close to the spec. The service worker reuses session/store.ts (same IndexedDB store, non-extractable contentKey); inline and ref paths and the "New push" fallback behave correctly; click routing is tested with real crypto; nothing under /api is cached; toggle persistence is checked against the device record. check:web (105 tests) and e2e:web (27 passed, installability []) pass. No FIX-level bug.

Missing definition-of-done item

The new offline Settings placeholder (App.tsx:231-235, "Offline. Reconnect to load this browser's settings.") has no Playwright check, no axe scan and no screenshots. Suggested: in the offline-reload test in pwa.spec.ts, open Settings while offline, assert the text, then run captureAccessibleSettings as settings-offline.

FILE (score < 5, code left alone, recorded on ENG-275)

score=4 — NotificationPrompt renders outside .home-surface: wider than the Home column on desktop, and its h2 precedes the h1.
score=4 — badge is the full-colour /favicon.svg; Android Chrome shows a solid square. DESIGN.md says use the single-colour small mark.
score=4 — "On" reflects only the local subscription, never the server's webPush.
score=4 — the online handler after an offline start re-runs start(); a flaky first call replaces cached Home with "Connection unavailable" and restarts useHome.
score=4 — the Home prompt hides whenever permission isn't "default", so a new login in a browser that already granted permission gets no prompt.
score=2 — sign-out unsubscribes before deleteDevice; a failed DELETE leaves notifications silently off.
score=2 — show-push from an uncontrolled window is dropped (Shift+Reload tab).
score=2 — the "Reload to update" toast has no dismiss and covers history on mobile.
score=3 — CI note for ENG-278: the PWA tests need full Chromium (not the headless shell), and PushManager.subscribe likely needs outbound internet (FCM) on the runner.

Next: omp round 2 for the missing offline-Settings checks.

v1 1w ago

demi.dev 1w ago

pneumo-reviewer: round 2 (head a18c32d), APPROVE #

The round fixes the one missing definition-of-done item; no app source changed (apps/web/e2e/pwa.spec.ts +9/-1 and one README line).

DoD resolved: the offline-reload test (pwa.spec.ts:595) opens Settings while offline, asserts the heading and the status "Offline. Reconnect to load this browser's settings.", then runs axe and settings-offline-{light,dark,mobile} screenshots, all inside the setOffline(true) block.
Round 1 had no FIX findings; the nine FILE findings stay on ENG-275 (CI note also on ENG-278). No new findings.

Verify (reviewer and lead): mise run check:web exit 0 (105 tests); mise run e2e:web exit 0 (27 passed). CI 3/3 on a18c32d.

Versions

pneumo-reviewer: round 1, REQUEST_CHANGES #

pneumo-reviewer: round 2 (head a18c32d), APPROVE #