fix(sw): await caches.match() before || fallback so offline HTML actually shows

nesquena · claude · nesquena · commit ecf2f93a214f · 2026-04-23T15:04:02.000-07:00
The offline-navigation fallback was dead code:

    return caches.match('./') || new Response('&lt;html&gt;...&lt;/html&gt;', ...);

`caches.match()` returns a Promise, and Promise objects are always truthy
in a `||` check — so the `new Response(...)` branch was never taken. On
actual offline, `caches.match('./')` resolves to undefined (no cache hit
for the root), the SW returns undefined, and the browser falls back to
its own default offline page. The custom "Hermes requires a server
connection" HTML was unreachable.

Fix by threading the match through `.then()` so the resolved value (not
the Promise object) feeds the `||`:

    return caches.match('./').then((cached) =&gt; cached || new Response(...));

Added 13 regression tests in tests/test_pwa_manifest_sw.py covering:
- manifest.json validity + required PWA fields + icon existence
- sw.js cache-version placeholder + API/stream bypass + correct offline
  pattern (explicitly rejects the broken `|| new Response` shape so it
  can't regress)
- /manifest.json + /sw.js routes serve correct Content-Type,
  Cache-Control, Service-Worker-Allowed headers and inject WEBUI_VERSION
- index.html links manifest, registers SW, has iOS PWA meta tags

Co-Authored-By: Claude Opus 4.7 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/static/sw.js b/static/sw.js
@@ -87,15 +87,18 @@ self.addEventListener('fetch', (event) => {
         }
         return response;
       }).catch(() => {
-        // Offline fallback for navigation requests
+        // Offline fallback for navigation requests.
+        // Note: caches.match() returns a Promise (always truthy in a `||` check),
+        // so we must await/then to unwrap it — otherwise the `new Response(...)`
+        // branch is dead code and the browser falls back to its default offline page.
         if (event.request.mode === 'navigate') {
-          return caches.match('./') || new Response(
+          return caches.match('./').then((cached) => cached || new Response(
             '<html><body style="font-family:sans-serif;padding:2rem;background:#1a1a1a;color:#ccc">' +
             '<h2>You are offline</h2>' +
             '<p>Hermes requires a server connection. Please check your network and try again.</p>' +
             '</body></html>',
             { headers: { 'Content-Type': 'text/html' } }
-          );
+          ));
         }
       });
     })
diff --git a/tests/test_pwa_manifest_sw.py b/tests/test_pwa_manifest_sw.py
@@ -0,0 +1,152 @@
+"""Regression tests for PWA support (manifest + service worker).
+
+Covers:
+- manifest.json is valid JSON with required PWA fields
+- sw.js has the `__CACHE_VERSION__` placeholder the server replaces at request time
+- sw.js offline-fallback uses a resolved promise (not `caches.match() || fallback`
+  which is broken — Promise objects are always truthy in `||` checks, so the
+  fallback Response would never be used)
+- /manifest.json, /manifest.webmanifest, /sw.js routes serve correct Content-Type
+"""
+import json
+import re
+from pathlib import Path
+
+
+ROOT = Path(__file__).resolve().parent.parent
+MANIFEST = ROOT / "static" / "manifest.json"
+SW = ROOT / "static" / "sw.js"
+INDEX = ROOT / "static" / "index.html"
+ROUTES = ROOT / "api" / "routes.py"
+
+
+class TestManifest:
+    def test_manifest_is_valid_json(self):
+        data = json.loads(MANIFEST.read_text(encoding="utf-8"))
+        assert isinstance(data, dict)
+
+    def test_manifest_has_required_pwa_fields(self):
+        data = json.loads(MANIFEST.read_text(encoding="utf-8"))
+        for field in ("name", "start_url", "display", "icons"):
+            assert field in data, f"manifest.json missing required field: {field}"
+        assert data["display"] == "standalone", (
+            "manifest.display must be 'standalone' for installable PWA"
+        )
+        assert isinstance(data["icons"], list) and len(data["icons"]) > 0, (
+            "manifest.icons must be a non-empty list"
+        )
+
+    def test_manifest_icons_reference_existing_files(self):
+        data = json.loads(MANIFEST.read_text(encoding="utf-8"))
+        for icon in data["icons"]:
+            src = icon.get("src", "")
+            if src.startswith("http"):
+                continue  # external icon, skip
+            # Paths are relative to the app root (where manifest is served)
+            # 'static/favicon.svg' or './static/favicon.svg' both valid
+            clean = src.lstrip("./")
+            p = ROOT / clean
+            assert p.exists(), f"manifest.json references missing icon: {src}"
+
+
+class TestServiceWorker:
+    def test_sw_has_cache_version_placeholder(self):
+        src = SW.read_text(encoding="utf-8")
+        assert "__CACHE_VERSION__" in src, (
+            "sw.js must contain __CACHE_VERSION__ placeholder for the server "
+            "handler at /sw.js to replace with WEBUI_VERSION at request time"
+        )
+
+    def test_sw_bypasses_api_and_stream(self):
+        src = SW.read_text(encoding="utf-8")
+        assert "/api/" in src, "SW must bypass /api/* (no cached auth/session responses)"
+        assert "/stream" in src, "SW must bypass streaming endpoints"
+
+    def test_sw_offline_fallback_awaits_caches_match(self):
+        """caches.match() returns a Promise (always truthy in `||`), so the pattern
+        `caches.match('./') || new Response(...)` is broken — the fallback Response
+        is dead code and the browser falls back to its default offline page.
+
+        The correct pattern chains the match through .then() or awaits it so the
+        resolved value is what gets the `||` fallback.
+        """
+        src = SW.read_text(encoding="utf-8")
+        # Must not use the broken shape
+        broken_pattern = re.compile(
+            r"caches\.match\([^)]*\)\s*\|\|\s*new\s+Response",
+            re.DOTALL,
+        )
+        assert not broken_pattern.search(src), (
+            "sw.js offline fallback uses `caches.match('./') || new Response(...)` "
+            "which is dead code — caches.match() returns a Promise that's always "
+            "truthy. Use `.then((cached) => cached || new Response(...))` instead."
+        )
+        # Positive assertion that SOME form of the working pattern is present
+        has_then = ".then(" in src and "cached" in src
+        has_await = "await caches.match" in src
+        assert has_then or has_await, (
+            "sw.js must await/then the caches.match() result before applying the fallback"
+        )
+
+    def test_sw_never_caches_api_responses(self):
+        """Defensive: the SW must not cache responses from /api/* paths.
+        Currently enforced by early-return before the shell-asset cache block."""
+        src = SW.read_text(encoding="utf-8")
+        # Look for the early-return pattern in the fetch handler
+        assert "return;" in src and "/api/" in src, (
+            "SW fetch handler must early-return for /api/* paths (no caching)"
+        )
+
+
+class TestPWARoutes:
+    def test_manifest_route_serves_correct_content_type(self):
+        src = ROUTES.read_text(encoding="utf-8")
+        # The handler block for /manifest.json
+        idx = src.find('"/manifest.json"')
+        assert idx != -1, "routes.py must handle /manifest.json"
+        block = src[idx:idx + 800]
+        assert "application/manifest+json" in block, (
+            "manifest.json route must serve Content-Type: application/manifest+json"
+        )
+        assert "no-store" in block or "Cache-Control" in block, (
+            "manifest.json should have Cache-Control: no-store so updates are picked up"
+        )
+
+    def test_sw_route_injects_cache_version(self):
+        src = ROUTES.read_text(encoding="utf-8")
+        idx = src.find('"/sw.js"')
+        assert idx != -1, "routes.py must handle /sw.js"
+        block = src[idx:idx + 1000]
+        assert "__CACHE_VERSION__" in block, (
+            "sw.js route must replace __CACHE_VERSION__ with the current WEBUI_VERSION"
+        )
+        assert "WEBUI_VERSION" in block, (
+            "sw.js route must import and use WEBUI_VERSION for cache busting"
+        )
+
+    def test_sw_route_sets_service_worker_allowed(self):
+        src = ROUTES.read_text(encoding="utf-8")
+        idx = src.find('"/sw.js"')
+        block = src[idx:idx + 1000]
+        assert "Service-Worker-Allowed" in block, (
+            "sw.js route must set Service-Worker-Allowed header so the SW can control "
+            "the expected scope"
+        )
+
+
+class TestIndexHtmlIntegration:
+    def test_index_links_manifest(self):
+        src = INDEX.read_text(encoding="utf-8")
+        assert 'rel="manifest"' in src, "index.html must link to manifest.json"
+
+    def test_index_registers_service_worker(self):
+        src = INDEX.read_text(encoding="utf-8")
+        assert "serviceWorker" in src and "register" in src, (
+            "index.html must register the service worker"
+        )
+
+    def test_index_has_ios_pwa_meta_tags(self):
+        src = INDEX.read_text(encoding="utf-8")
+        assert "apple-mobile-web-app-capable" in src, (
+            "index.html should include Apple PWA meta tags for iOS home-screen support"
+        )
