fix(pwa): restore installability lost in the Next.js migration - #235
Merged
Conversation
src/app.html carried `<link rel="manifest">` plus the apple-touch-icon and apple-mobile-web-app-* tags. It was deleted in f1a5ed9 and nothing in the Next root layout replaced it, so Chromium stopped offering "Install app" on desktop and Android and iOS "Add to Home Screen" stopped opening standalone. - layout.jsx: declare `manifest`, `appleWebApp`, `icons` and the msapplication tiles via the metadata export, and move viewport/theme-color to the `viewport` export. The hand-written <head> tags are dropped so nothing is emitted twice. Next 15 emits `mobile-web-app-capable` for appleWebApp.capable but no longer the apple-prefixed tag, which iOS < 15.4 still needs, so it is declared too. - manifest.json: add id/scope/display_override/categories/lang/dir and a 512px maskable icon; drop `version` and `splash_pages`, neither of which is a manifest member. `id` is pinned to start_url — the value Chromium already derives — so existing installs keep their identity. - sw.js: navigations are network-first with the cached shell as an offline fallback. Cache-first pinned the installed app to the HTML captured at install time, whose hashed Next chunks stop existing after the next deploy. Non-GET, cross-origin and /api/ requests are no longer intercepted. - pin @vitejs/plugin-react to ^5.2.0: v6 peers vite ^8 but vitest 4 ships vite 7, so loading vitest.config.js threw and no test could run. Verified against a dev server: the rendered <head> contains exactly one `<link rel="manifest" href="/manifest.json">`, one `mobile-web-app-capable` and one `apple-mobile-web-app-capable`, and /manifest.json, /sw.js and the 512px icon all return 200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan96 finding(s) HIGH/CRITICAL: 3 | MEDIUM: 17 | LOW: 76
…and 46 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
Contributor
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
ralyodio
added a commit
that referenced
this pull request
Aug 10, 2026
CI ran CodeQL, Semgrep, npm-audit and gitleaks but never vitest, so nobody noticed the suite could not start at all: vitest.config.js failed to load because @vitejs/plugin-react@6 peers vite ^8 while vitest 4 ships vite 7. That was fixed in #235; this wires the suite into CI so it stays runnable. Triaging all 139 test files in isolation gave 72 passing, 66 failing and 1 hanging. Two mechanical problems accounted for a third of the breakage: - 24 files did `import { describe, it } from 'mocha'` while the repo runs vitest, so those bindings were undefined and the file died on "Cannot read properties of undefined (reading 'describe')". Removed the import (vitest already sets globals: true) and renamed mocha's before/after to vitest's beforeAll/afterAll. - jsdom ships no IndexedDB, but the app stores private keys there, so every test touching key storage hit "indexedDB is not defined". tests/setup.js now imports fake-indexeddb/auto. That took it to 80 passing / 58 failing / 1 hanging. The remaining 59 are listed in tests/quarantine.js and excluded from the default suite: 19 have genuinely failing assertions, 13 import SvelteKit paths the migration deleted, 12 need a live Supabase, 5 use undefined globals, 1 has a parse error, 9 are assorted. tests/pwa-session-integration.test.js hangs rather than fails, which is what made `pnpm test` unusable. They are quarantined rather than deleted because each still documents intended behaviour worth porting, and a permanently red build gets ignored. Work them off with `pnpm test:quarantined`; deleting a line from the list is how a file rejoins CI. tests/QUARANTINE.md explains the categories and the fix for each. CI now runs `pnpm test:ci`: 80 files, 469 tests, ~33s, green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ralyodio
added a commit
that referenced
this pull request
Aug 11, 2026
* fix(icons): point the icon pipeline at public/, not static/
`pnpm icons:generate` wrote into `static/icons` and `pnpm icons:install` read
from there, but Next only serves `public/`. Regenerating icons updated a tree
nobody serves while the served one silently went stale — which is how the two
directories drifted apart in the first place.
- generate-icons.js reads ./public/favicon.svg and writes ./public/icons.
- It now emits every icon the app references, not a subset: the android-chrome-*
family (which is what public/manifest.json actually points at), the favicon-NxN
set, and the Windows mstile tiles including the non-square 310x150. Previously
regenerating refreshed the apple-touch icons and four icon-* sizes while
leaving the icons Chromium installs with untouched.
- install-desktop-icons.sh copies from public/. Its 16px and 32px fallbacks
looked for static/favicon-{16,32}.png, which never existed, so both branches
were dead; they now use public/icons/favicon-{16x16,32x32}.png.
- Dropped the `needsSolidBackground` branch. `background` only paints the
letterbox that `fit: 'contain'` adds, and favicon.svg is square, so it painted
nothing — the committed icons have always had transparent pixels. Making the
manifest's maskable 192/512 icons genuinely opaque needs `.flatten()` plus
safe-zone padding, which changes how the installed icon looks, so it is left
as a deliberate design decision rather than folded in here.
Running the generator now rewrites exactly the 38 PNGs already in public/icons
and creates no new files, so the script and the tree agree. Regenerated binaries
are not committed — this change is tooling only, no icon artwork changes.
tests/pwa-installability.test.js grows five checks tying the generator to the
manifest and layout: every icon either references must be one the generator
emits, and no tooling may point at static/ again. That is what caught the
missing mstile tiles.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* chore: delete the static/ SvelteKit holdover
static/ was a near-exact copy of public/ left behind by the Next.js migration.
Next only serves public/, so every one of these 56 files was dead weight — and
a trap, since editing static/manifest.json looks like it should do something.
Verified before removing: every file under static/ has a counterpart in public/,
including .well-known/security.txt (which prod serves 200). The icon tooling
that genuinely did read static/ was retargeted at public/ in the previous
commit, so nothing references it any more.
Also fixed while here:
- public/qryptchat.desktop hardcoded
`Icon=/home/ettinger/src/qrypt.chat/qryptochat-web/static/qryptchat.png`, an
absolute path into a developer's home directory (with a typo in the folder
name), so the installed desktop launcher had no icon for anyone else. The
installer registers the icon in the hicolor theme, so the entry just needs the
theme name: `Icon=qryptchat`.
- PWA_VIDEO_PLAYBACK_GUIDE.md told readers to edit static/manifest.json.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ci: run vitest, with the broken tests quarantined
CI ran CodeQL, Semgrep, npm-audit and gitleaks but never vitest, so nobody
noticed the suite could not start at all: vitest.config.js failed to load
because @vitejs/plugin-react@6 peers vite ^8 while vitest 4 ships vite 7. That
was fixed in #235; this wires the suite into CI so it stays runnable.
Triaging all 139 test files in isolation gave 72 passing, 66 failing and 1
hanging. Two mechanical problems accounted for a third of the breakage:
- 24 files did `import { describe, it } from 'mocha'` while the repo runs
vitest, so those bindings were undefined and the file died on
"Cannot read properties of undefined (reading 'describe')". Removed the
import (vitest already sets globals: true) and renamed mocha's before/after
to vitest's beforeAll/afterAll.
- jsdom ships no IndexedDB, but the app stores private keys there, so every
test touching key storage hit "indexedDB is not defined". tests/setup.js now
imports fake-indexeddb/auto.
That took it to 80 passing / 58 failing / 1 hanging. The remaining 59 are listed
in tests/quarantine.js and excluded from the default suite: 19 have genuinely
failing assertions, 13 import SvelteKit paths the migration deleted, 12 need a
live Supabase, 5 use undefined globals, 1 has a parse error, 9 are assorted.
tests/pwa-session-integration.test.js hangs rather than fails, which is what
made `pnpm test` unusable.
They are quarantined rather than deleted because each still documents intended
behaviour worth porting, and a permanently red build gets ignored. Work them off
with `pnpm test:quarantined`; deleting a line from the list is how a file
rejoins CI. tests/QUARANTINE.md explains the categories and the fix for each.
CI now runs `pnpm test:ci`: 80 files, 469 tests, ~33s, green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The regression
src/app.htmlcarried<link rel="manifest">plus the apple-touch-icon andapple-mobile-web-app-*tags. It was deleted inf1a5ed9("complete Next.js 15 migration cleanup and polish") and nothing in the Next root layout replaced it.The only reference to a manifest left anywhere in
src/waspwa-diagnostics.jschecking whether one existed. Without a linked manifest, Chromium will not offer "Install app" on desktop or Android, and iOS "Add to Home Screen" opens a plain bookmark instead of a standalone window.Cross-checked against
media-streamer, which installs fine: it has no service worker at all: its installability comes purely frommanifest: '/manifest.json'in the layout metadata. That confirmed the manifest link was the whole story.Changes
src/app/layout.jsx— declaremanifest,appleWebApp,iconsand the msapplication tiles through themetadataexport, and move viewport/theme-color to theviewportexport. The hand-written<head>tags are removed so nothing is emitted twice. Next 15 emitsmobile-web-app-capableforappleWebApp.capablebut no longer the apple-prefixed tag that iOS < 15.4 needs, so that one is declared explicitly.public/manifest.json— addid,scope,display_override,categories,lang,dirand a 512px maskable icon. Droppedversionandsplash_pages, neither of which is a manifest member.idis pinned tostart_url— the value Chromium already derives — so existing installs keep their identity rather than registering as a new app.public/sw.js— navigations are now network-first with the cached shell as an offline fallback. The old cache-first handler precached/at install time and never refreshed it, so every newly installed PWA would have been pinned to the install-day HTML, whose hashed Next chunks stop existing after the next deploy → white screen. Non-GET, cross-origin and/api/requests are no longer intercepted, so encrypted payloads can never come back from a cache. Cache bumpedv1→v2to evict stale shells.package.json— unrelated but blocking:@vitejs/plugin-react@6peersvite ^8while vitest 4 ships vite 7, sovitest.config.jsfailed to load withERR_PACKAGE_PATH_NOT_EXPORTEDand no test in the repo could run. Pinned to^5.2.0, which supports vite ^7. CI never runs vitest, which is why this went unnoticed. Lockfile churn is babel transitives that v5 needs; no runtime dependency version changed.Verification
Rendered against a dev server; the emitted
<head>contains exactly one of each:plus all nine apple-touch-icons.
/manifest.json,/sw.jsand/icons/android-chrome-512x512.pngall return 200.next buildcompiles successfully (page-data collection needs Supabase env, unrelated).New
tests/pwa-installability.test.js(15 tests) guards the manifest fields, icon files on disk, the layout wiring and the SW caching rules. Confirmed it actually catches the regression: deleting themanifest:line turns it red. Existing JSX component tests still pass under the pinned plugin.Note
static/is a leftover SvelteKit directory that Next does not serve; itsmanifest.jsonwas identical topublic/'s, so I kept them in sync. Worth deleting the whole directory separately.🤖 Generated with Claude Code