feat: PWA support with per-space offline reading#630
Conversation
Wiki spaces can now be saved for offline reading. A service worker bulk-precaches all published leaf pages, embedded images, and shared assets for a space; an LMS-style nudge prompts the user to install the app and save the current space; a /wiki-offline page lists and manages saved spaces. The existing SPA navigation fallback to full page loads means cached pages just work offline — no JSON caching needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WalkthroughThis PR implements comprehensive offline support for Frappe Wiki via a Progressive Web App approach. A new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wiki/api/wiki_space.py`:
- Around line 13-16: The guest-exposed get_space_manifest is performing
expensive descendant traversal and hashing (the code around the descendant
traversal/hashing block referenced at lines 32-68) and must be protected:
implement a caching layer keyed by space_route (use existing frappe.cache or
Redis) with a reasonable TTL to serve repeated requests without recomputing the
manifest, and add request-rate controls for guest users (e.g., per-IP or
per-guest token bucket limits via your app’s rate-limiter hooks) so heavy work
is throttled; alternatively restrict allow_guest=True for this endpoint and
require authenticated access for uncached manifest generation—update
get_space_manifest to first check cache, only compute on miss, and enforce the
rate limit before performing traversal/hashing.
In `@wiki/templates/wiki/layout.html`:
- Around line 233-238: Guard the API response before messaging the service
worker: after the fetch to the url and before reading data.message/manifest,
check res.ok and validate that data && data.message (the manifest) exists and
has the expected shape; if res.ok is false or manifest is missing/invalid, log
or handle the error and skip calling controller.postMessage. Update the block
around the fetch/res/json/manifest and controller.postMessage calls (referencing
url, fetch(...), res.ok, data.message, manifest, and controller.postMessage) to
perform these checks and fail-safe early instead of posting undefined.
In `@wiki/www/sw.js`:
- Around line 80-81: The PRECACHE_SPACE message handler dereferences
data.manifest.space.route without validating the payload; update the handler
that calls event.waitUntil(precacheSpace(...)) to first validate that data,
data.manifest, and data.manifest.space are objects and that
data.manifest.space.route is a non-empty string (or otherwise valid) before
calling precacheSpace; if validation fails, bail out gracefully and log or
postMessage an error instead of calling precacheSpace. Apply the same validation
pattern to the other message branch that accesses manifest.space.route (the
similar block around the 97-100 handling) so malformed/empty messages cannot
throw in the worker.
- Around line 99-111: The current per-space cacheName uses a hardcoded suffix
"-v1" which prevents purging of removed pages when a space updates; update the
cache naming or clear logic so cacheName incorporates the space manifest version
(e.g., use the manifest's version/hash with SPACE_CACHE_PREFIX and spaceRoute)
or explicitly delete the existing cacheName before repopulating. Locate the
cacheName declaration and the cleanup block (references: SPACE_CACHE_PREFIX,
spaceRoute, cacheName, caches.keys(), Promise.all) and change it to include the
manifest version (or call caches.delete(cacheName) prior to refill) so stale
assets are removed on updates.
In `@wiki/www/wiki-offline.html`:
- Around line 137-141: In refresh(), validate the fetch response and payload
before sending to the service worker: check res.ok and that data && data.message
is the expected manifest object/array (or has required keys) and only call
navigator.serviceWorker.controller.postMessage({ type: 'PRECACHE_SPACE',
manifest: data.message }) when those checks pass; otherwise handle the error
path (alert/log) and avoid posting (also ensure
navigator.serviceWorker.controller exists before posting).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a20018a-9de1-444e-bf98-1eda9610dfe6
📒 Files selected for processing (7)
wiki/api/wiki_space.pywiki/templates/wiki/includes/install_prompt.htmlwiki/templates/wiki/layout.htmlwiki/www/manifest.jsonwiki/www/offline.htmlwiki/www/sw.jswiki/www/wiki-offline.html
| @frappe.whitelist( | ||
| allow_guest=True, methods=["GET"] | ||
| ) # nosemgrep: frappe-semgrep-rules.rules.security.guest-whitelisted-method | ||
| def get_space_manifest(space_route: str) -> dict: |
There was a problem hiding this comment.
Guest manifest endpoint needs abuse controls before release.
Line 13 exposes this endpoint to guests, and Lines 32-68 do potentially heavy descendant traversal + hashing for every request. Without rate limiting and/or cached manifest reuse, this can be abused into avoidable load spikes.
Also applies to: 32-68
🧰 Tools
🪛 GitHub Actions: Linters / 0_Semgrep Rules.txt
[error] 13-16: semgrep reported a blocking finding (frappe-semgrep-rules.rules.security.guest-whitelisted-method): Whitelisted method accessible to guest should be manually reviewed by the security team. Location: @frappe.whitelist(allow_guest=True, methods=["GET"]) on def get_space_manifest(space_route: str) -> dict.
🪛 GitHub Actions: Linters / Semgrep Rules
[error] 13-15: Semgrep blocking rule fired (frappe-semgrep-rules.rules.security.guest-whitelisted-method): Whitelisted method accessible to guest should be manually reviewed by security team. Exiting with code 1.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiki/api/wiki_space.py` around lines 13 - 16, The guest-exposed
get_space_manifest is performing expensive descendant traversal and hashing (the
code around the descendant traversal/hashing block referenced at lines 32-68)
and must be protected: implement a caching layer keyed by space_route (use
existing frappe.cache or Redis) with a reasonable TTL to serve repeated requests
without recomputing the manifest, and add request-rate controls for guest users
(e.g., per-IP or per-guest token bucket limits via your app’s rate-limiter
hooks) so heavy work is throttled; alternatively restrict allow_guest=True for
this endpoint and require authenticated access for uncached manifest
generation—update get_space_manifest to first check cache, only compute on miss,
and enforce the rate limit before performing traversal/hashing.
| const url = `/api/method/wiki.api.wiki_space.get_space_manifest?space_route=${encodeURIComponent(spaceRoute)}`; | ||
| const res = await fetch(url, { credentials: 'same-origin' }); | ||
| const data = await res.json(); | ||
| const manifest = data.message; | ||
| const controller = navigator.serviceWorker.controller || (await navigator.serviceWorker.ready).active; | ||
| controller.postMessage({ type: 'PRECACHE_SPACE', manifest }); |
There was a problem hiding this comment.
Guard manifest API response before posting to the service worker.
If the API responds with an error payload, data.message can be undefined and trigger worker-side failures. Validate res.ok and manifest shape before postMessage.
Suggested fix
const res = await fetch(url, { credentials: 'same-origin' });
-const data = await res.json();
-const manifest = data.message;
+if (!res.ok) throw new Error(`Manifest request failed: ${res.status}`);
+const data = await res.json();
+const manifest = data?.message;
+if (!manifest?.space?.route || !Array.isArray(manifest?.pages)) {
+ throw new Error('Invalid manifest response');
+}
const controller = navigator.serviceWorker.controller || (await navigator.serviceWorker.ready).active;
controller.postMessage({ type: 'PRECACHE_SPACE', manifest });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const url = `/api/method/wiki.api.wiki_space.get_space_manifest?space_route=${encodeURIComponent(spaceRoute)}`; | |
| const res = await fetch(url, { credentials: 'same-origin' }); | |
| const data = await res.json(); | |
| const manifest = data.message; | |
| const controller = navigator.serviceWorker.controller || (await navigator.serviceWorker.ready).active; | |
| controller.postMessage({ type: 'PRECACHE_SPACE', manifest }); | |
| const url = `/api/method/wiki.api.wiki_space.get_space_manifest?space_route=${encodeURIComponent(spaceRoute)}`; | |
| const res = await fetch(url, { credentials: 'same-origin' }); | |
| if (!res.ok) throw new Error(`Manifest request failed: ${res.status}`); | |
| const data = await res.json(); | |
| const manifest = data?.message; | |
| if (!manifest?.space?.route || !Array.isArray(manifest?.pages)) { | |
| throw new Error('Invalid manifest response'); | |
| } | |
| const controller = navigator.serviceWorker.controller || (await navigator.serviceWorker.ready).active; | |
| controller.postMessage({ type: 'PRECACHE_SPACE', manifest }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiki/templates/wiki/layout.html` around lines 233 - 238, Guard the API
response before messaging the service worker: after the fetch to the url and
before reading data.message/manifest, check res.ok and validate that data &&
data.message (the manifest) exists and has the expected shape; if res.ok is
false or manifest is missing/invalid, log or handle the error and skip calling
controller.postMessage. Update the block around the fetch/res/json/manifest and
controller.postMessage calls (referencing url, fetch(...), res.ok, data.message,
manifest, and controller.postMessage) to perform these checks and fail-safe
early instead of posting undefined.
| if (data.type === 'PRECACHE_SPACE') { | ||
| event.waitUntil(precacheSpace(data.manifest, event.source)); |
There was a problem hiding this comment.
Validate PRECACHE_SPACE payload before dereferencing manifest fields.
manifest.space.route is accessed directly. A malformed/empty message will throw in the worker and can leave the UI stuck waiting for completion state.
Suggested fix
async function precacheSpace(manifest, client) {
+ if (!manifest?.space?.route || !Array.isArray(manifest?.pages)) {
+ client?.postMessage({
+ type: 'PRECACHE_DONE',
+ spaceRoute: manifest?.space?.route || '',
+ version: '',
+ failed: [{ error: 'Invalid manifest payload' }],
+ });
+ return;
+ }
const spaceRoute = manifest.space.route;Also applies to: 97-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiki/www/sw.js` around lines 80 - 81, The PRECACHE_SPACE message handler
dereferences data.manifest.space.route without validating the payload; update
the handler that calls event.waitUntil(precacheSpace(...)) to first validate
that data, data.manifest, and data.manifest.space are objects and that
data.manifest.space.route is a non-empty string (or otherwise valid) before
calling precacheSpace; if validation fails, bail out gracefully and log or
postMessage an error instead of calling precacheSpace. Apply the same validation
pattern to the other message branch that accesses manifest.space.route (the
similar block around the 97-100 handling) so malformed/empty messages cannot
throw in the worker.
| const cacheName = `${SPACE_CACHE_PREFIX}${spaceRoute}-v1`; | ||
|
|
||
| // Drop any older copy of this space so leftover URLs don't linger. | ||
| const existing = await caches.keys(); | ||
| await Promise.all( | ||
| existing | ||
| .filter( | ||
| (k) => | ||
| k.startsWith(`${SPACE_CACHE_PREFIX}${spaceRoute}-`) && | ||
| k !== cacheName, | ||
| ) | ||
| .map((k) => caches.delete(k)), | ||
| ); |
There was a problem hiding this comment.
Per-space cache version is static, so stale removed pages can survive updates.
Line 99 hardcodes -v1; when a space updates, removed routes/images are never purged from the current cache. Use manifest version in cache naming (or clear current cache before refill) so updates are exact.
Suggested fix
- const cacheName = `${SPACE_CACHE_PREFIX}${spaceRoute}-v1`;
+ const version = manifest.version || 'v1';
+ const cacheName = `${SPACE_CACHE_PREFIX}${spaceRoute}-${version}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cacheName = `${SPACE_CACHE_PREFIX}${spaceRoute}-v1`; | |
| // Drop any older copy of this space so leftover URLs don't linger. | |
| const existing = await caches.keys(); | |
| await Promise.all( | |
| existing | |
| .filter( | |
| (k) => | |
| k.startsWith(`${SPACE_CACHE_PREFIX}${spaceRoute}-`) && | |
| k !== cacheName, | |
| ) | |
| .map((k) => caches.delete(k)), | |
| ); | |
| const version = manifest.version || 'v1'; | |
| const cacheName = `${SPACE_CACHE_PREFIX}${spaceRoute}-${version}`; | |
| // Drop any older copy of this space so leftover URLs don't linger. | |
| const existing = await caches.keys(); | |
| await Promise.all( | |
| existing | |
| .filter( | |
| (k) => | |
| k.startsWith(`${SPACE_CACHE_PREFIX}${spaceRoute}-`) && | |
| k !== cacheName, | |
| ) | |
| .map((k) => caches.delete(k)), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiki/www/sw.js` around lines 99 - 111, The current per-space cacheName uses a
hardcoded suffix "-v1" which prevents purging of removed pages when a space
updates; update the cache naming or clear logic so cacheName incorporates the
space manifest version (e.g., use the manifest's version/hash with
SPACE_CACHE_PREFIX and spaceRoute) or explicitly delete the existing cacheName
before repopulating. Locate the cacheName declaration and the cleanup block
(references: SPACE_CACHE_PREFIX, spaceRoute, cacheName, caches.keys(),
Promise.all) and change it to include the manifest version (or call
caches.delete(cacheName) prior to refill) so stale assets are removed on
updates.
| const url = `/api/method/wiki.api.wiki_space.get_space_manifest?space_route=${encodeURIComponent(route)}`; | ||
| const res = await fetch(url, { credentials: 'same-origin' }); | ||
| const data = await res.json(); | ||
| navigator.serviceWorker.controller.postMessage({ type: 'PRECACHE_SPACE', manifest: data.message }); | ||
| } catch (e) { alert('Could not fetch latest manifest — are you online?'); } |
There was a problem hiding this comment.
refresh() should validate manifest responses before sending to SW.
Line 140 posts data.message directly. Add res.ok and payload checks to avoid invalid precache requests and silent failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@wiki/www/wiki-offline.html` around lines 137 - 141, In refresh(), validate
the fetch response and payload before sending to the service worker: check
res.ok and that data && data.message is the expected manifest object/array (or
has required keys) and only call
navigator.serviceWorker.controller.postMessage({ type: 'PRECACHE_SPACE',
manifest: data.message }) when those checks pass; otherwise handle the error
path (alert/log) and avoid posting (also ensure
navigator.serviceWorker.controller exists before posting).
1 participant
Summary
Adds Progressive Web App support to the wiki with a per-space, on-demand offline reading model — modeled on Kiwix / DevDocs.
/sw.js, scope/) precaches the app shell on install and stores each downloaded space in its own cache (wiki-space-<route>-vN). Fetch handler is cache-first for stored URLs, network-passthrough otherwise, with/offlineas the navigation fallback.wiki.api.wiki_space.get_space_manifest(space_route)returns the version, list of published-leaf-public pages, embedded/files/...image URLs, and shared CSS/JS asset URLs for a space.beforeinstallpromptfor Android-class browsers (modal → install + download), shows a Share → Add to Home Screen popover on iOS. Accepting in either flow kicks off the per-space bulk download via the SW.Saving for offline — N/Mand a briefAvailable offlineon completion./wiki-offlinelisting every saved space with version, page count, storage estimate, and Remove / Update buttons. Discoverable for desktop users (who don't get the nudge).How offline navigation works
The wiki already has client-side SPA navigation that POSTs to
get_page_dataand falls back towindow.location.hrefon error (sidebar.html). When offline, the POST fails → the existing fallback triggers a full nav → the SW serves the cached HTML. No JSON caching, no SPA changes needed.What's in scope for v1
What's deliberately out of scope (v2+)
Test plan
Saving for offline — N/M, thenAvailable offline./offlinefallback page./wiki-offline→ shows the downloaded space with Remove + Update buttons; remove clears the cache.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes