Skip to content

Commit 828ec00

Browse files
authored
feat(chat): bundle the WebView renderers' JavaScript into the app (#368)
The artifact, diagram and math renderers loaded KaTeX, mermaid, marked, highlight.js, Tailwind, Babel and React from public CDNs at render time. Vendoring them fixes what the app executes at build time, lets the renderers work offline, and satisfies F-Droid's rule against downloading executable code without explicit opt-in consent. Part of #288.
1 parent c09ec96 commit 828ec00

65 files changed

Lines changed: 8320 additions & 150 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
---
2+
name: update-web-assets
3+
description: >
4+
Update the third-party JavaScript vendored into the app for the artifact,
5+
diagram, and math WebViews (KaTeX, mermaid, marked, highlight.js, Tailwind,
6+
Babel, React). Checks what is outdated, reads each pin's rationale before
7+
proposing a bump, re-downloads from the official source, verifies the lock,
8+
and runs the gates. Ends by naming what must be checked on a device, because
9+
a broken WebView asset fails silently. Use when bumping a vendored web
10+
library, adding a new one, or investigating whether a rendering bug comes
11+
from a stale pin.
12+
allowed-tools: Bash, Read, Edit, Write, Glob, Grep, WebFetch, AskUserQuestion
13+
argument-hint: "[asset-id ...] (optional, defaults to checking all of them)"
14+
---
15+
16+
# Update vendored web assets
17+
18+
The artifact, diagram and math renderers are WebViews, and every script they execute ships
19+
inside the app. `scripts/web-assets.json` is the registry; `scripts/vendor-web-assets.py`
20+
moves the bytes; `scripts/web-assets.lock.json` records a sha256 per file and CI verifies it.
21+
22+
**Read `scripts/web-assets.json` before doing anything.** Every entry carries a
23+
`pin_reason`, and several of them are load-bearing rather than informational — two pins
24+
must NOT be moved to the newest version, and the reason is in the file, not in this skill.
25+
26+
## What makes this different from a Gradle dependency bump
27+
28+
A wrong version here does not fail the build, fail a test, or throw anything Kotlin can
29+
catch. The failure happens inside the WebView, and the usual shape of it is a blank box or
30+
a feature that quietly stops working. Both libraries this system replaced were already
31+
broken that way before anyone noticed:
32+
33+
- **marked** deleted its `highlight` option in v5. Passing one to `setOptions` is accepted
34+
and silently ignored, so syntax highlighting had been dead with no error anywhere.
35+
- **highlight.js** was being loaded from a URL that served CommonJS. In a browser
36+
`<script>` that throws immediately and never defines `hljs`.
37+
38+
So: the gates below prove the wiring is *consistent*, and only a device proves it *renders*.
39+
Do not report a bump as done on green gates alone.
40+
41+
## Phase 0 — Establish what is stale
42+
43+
```bash
44+
scripts/vendor-web-assets.py --check # tree matches the lock (should be clean first)
45+
scripts/vendor-web-assets.py --outdated # what npm has that we don't
46+
scripts/vendor-web-assets.py --list # what each asset is for, and who uses it
47+
```
48+
49+
`--outdated` prints each pin's `pin_reason` next to any version it reports. Read it. It is
50+
advisory output about a registry that has opinions.
51+
52+
If `--check` fails before you have changed anything, stop and report it — someone
53+
hand-edited a vendored file, or a repin was left half-applied. That is a finding to raise,
54+
not a precondition to quietly re-sync away.
55+
56+
## Phase 1 — Decide, per asset
57+
58+
For each asset the user named (or each one `--outdated` flags, if they named none):
59+
60+
1. Read its `pin_reason` in the registry.
61+
2. If the reason forbids or constrains the bump, say so and **do not** bump it. Two known
62+
standing constraints, both explained in full in the registry:
63+
- **mermaid** must stay on the v10 line (v11+ is ESM-only and fails in Android WebView).
64+
- **react** / **react-dom** must move together and are bounded by UMD availability.
65+
3. Check the upstream changelog for breaking changes in the range, especially anything
66+
touching the entry point path the registry's `files` block names. A package that
67+
reorganises its `dist/` is the most likely way a bump fails, and `--sync` will refuse
68+
with a clear error if a declared path no longer exists.
69+
4. For a major version, use AskUserQuestion rather than deciding alone.
70+
71+
`tailwind` is pinned to a URL, not npm, because Tailwind v3 never published a browser
72+
build. `--bump` refuses it deliberately; repinning it means editing both the version and
73+
every file URL in the registry by hand, and moving to `@tailwindcss/browser` means moving
74+
to v4, which is a behaviour change for artifacts (the registry explains which utilities
75+
changed).
76+
77+
## Phase 2 — Apply
78+
79+
```bash
80+
scripts/vendor-web-assets.py --bump <id> <version>
81+
```
82+
83+
This repins the registry, wipes that asset's directory, re-downloads from the official
84+
tarball, rewrites the lock, and regenerates `VendoredWebAssets.kt`. The wipe is deliberate:
85+
a repin that drops a file must not leave the old one behind to be served.
86+
87+
Adding a **new** asset instead: add a registry entry (with a real `why`, `pin_reason` and
88+
`used_by`), run `--sync`, then reference it from the renderer by a path relative to the
89+
document base — never an absolute URL.
90+
91+
If the bump changes the entry point path or an API the renderer calls, update the HTML
92+
builder in the same pass. The builders are:
93+
94+
| Asset | Built in |
95+
|---|---|
96+
| katex | `androidMain/components/LatexBlock.kt`, `iosMain/components/PlatformMediaComponents.ios.kt` |
97+
| mermaid | `androidMain/components/MermaidDiagram.kt`, `commonMain/…/artifact/MermaidWebContent.kt`, `iosMain/components/PlatformMediaComponents.ios.kt` |
98+
| marked, marked-highlight, highlight | `commonMain/…/artifact/MarkdownWebContent.kt` |
99+
| tailwind, babel, react, react-dom | `commonMain/…/artifact/ArtifactWebContent.kt` |
100+
101+
Note KaTeX and mermaid each have **two** independent HTML builders (Android and iOS) that
102+
do not share code. Changing one and not the other is the easiest mistake to make here.
103+
104+
## Phase 3 — Gates
105+
106+
```bash
107+
scripts/vendor-web-assets.py --check
108+
./gradlew :feature:chat:testDebugUnitTest --tests '*VendoredAssetReferenceTest*' \
109+
--tests '*ReactArtifactRenderTest*'
110+
./gradlew :feature:chat:detekt :feature:chat:detektMetadataCommonMain
111+
./gradlew :app:assembleDebug
112+
```
113+
114+
`VendoredAssetReferenceTest` is the one that matters most: it asserts that no document
115+
references a remote origin, that no CSP lets one execute, and that **every path a document
116+
references exists in the generated manifest** — which is what catches a bump that renamed a
117+
dist file, since the symptom otherwise is a blank WebView.
118+
119+
Per the repo's workflow preference, skip iOS builds unless asked; if asked, stop at the
120+
Gradle framework link (`./gradlew :feature:chat:compileKotlinIosSimulatorArm64`).
121+
122+
## Phase 4 — Report, and name the device check
123+
124+
Report: which assets moved, from and to; the APK size delta if it is material; anything in
125+
a `pin_reason` you deliberately did not act on.
126+
127+
Then state plainly that the gates cannot confirm rendering, and name what to look at for
128+
the assets that actually moved:
129+
130+
| Asset | What to look at on a device |
131+
|---|---|
132+
| katex | a message containing `$$x^2$$` — check glyphs AND that fonts loaded (no fallback serif) |
133+
| mermaid | a ```mermaid block in a message, and a mermaid artifact opened fullscreen |
134+
| marked / marked-highlight / highlight | a markdown artifact with a fenced code block — highlighting present, theme matches light/dark |
135+
| tailwind | an HTML artifact using utility classes |
136+
| babel / react / react-dom | a React artifact that uses hooks; then one importing an unbundled package, which must show the naming error rather than a blank box |
137+
138+
Do not open a PR. Per the repo's convention, stop at local commits and let the user device-test first.

.github/workflows/ci.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ jobs:
3232
- name: Setup Gradle
3333
uses: gradle/actions/setup-gradle@v6.3.0
3434

35+
# The WebView renderers execute these files. Nothing else notices if one is
36+
# hand-edited, replaced, or left behind by a repin -- the app just runs it.
37+
- name: Verify vendored web assets
38+
run: scripts/vendor-web-assets.py --check
39+
3540
- name: Run lint
3641
run: ./gradlew detekt detektMetadataCommonMain :app:lint --continue
3742

CLAUDE.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,4 +65,15 @@ Each module has its own `CLAUDE.md` with specific guidance.
6565
- **`UPSTREAM_VERSION`** — Tracks which official tag/commit this mobile build is based on. Updated by the `/sync-upstream` skill.
6666
- **`backendTargetVersion`** (root `version.properties`) — single source of truth for the targeted backend; must match the tag in `UPSTREAM_VERSION` (without `v` prefix). A core/common Gradle task code-generates `BackendVersion.SUPPORTED_BACKEND_VERSION` from it, and `release.yml` reads it for release notes. Edit the property, not the constant.
6767
- **`/sync-upstream`** — Claude Code skill to diff upstream releases, identify gaps, propose changes, and implement them with user approval. Uses Agent Teams (investigator, android-expert, implementer, verifier).
68+
- **`scripts/web-assets.json`** — registry of the third-party JavaScript the artifact/diagram/math
69+
WebViews execute (KaTeX, mermaid, marked, highlight.js, Tailwind, Babel, React). All of it is
70+
**vendored into the app**; none of it is fetched at render time. This is required for F-Droid,
71+
which rejects apps that download executable code without explicit opt-in consent — a
72+
`<script src="https://cdn…">` in a WebView is exactly that. It also closes a silent-drift hole:
73+
an unversioned CDN URL served whatever the CDN resolved that day, and two libraries had already
74+
broken that way without failing a build or a test. `scripts/vendor-web-assets.py` downloads the
75+
pins and CI verifies the tree against a sha256 lock. **Repin via `/update-web-assets`**, never by
76+
editing a vendored file. Adding a new WebView dependency means adding a registry entry in the
77+
same PR. See `feature/chat/CLAUDE.md` for how a page resolves them per platform.
78+
6879
- **`scripts/mirrors.json`** — registry of upstream constants the client copies by hand, because the server never serves them (which providers take documents natively, which MIME types the parser extracts, which feedback reasons the write route accepts). These drift **silently**: nothing fails to decode and nothing errors, so a sync's ordinary diff sweep reads them as inert constant edits. `scripts/check-mirrors.py` diffs each watched region between two upstream revisions and names the Kotlin file to reconcile; `/sync-upstream` runs it at Phase 0. **Adding a hardcoded mirror means adding a registry entry in the same PR** — a mirror nobody registered is one nobody will notice going stale.

feature/chat/CLAUDE.md

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,17 +363,64 @@ existing upload/usage path already handles them.
363363
reloads its WebView on every content change (mermaid recreates its view outright). The real
364364
preview mounts once, at settle, where the streaming→final swap replaces the subtree anyway.
365365
- Supported types: `text/html`, `image/svg+xml`, `application/vnd.react`, `application/vnd.mermaid`, `text/markdown`/`text/md`, `text/plain`, `application/vnd.code-html`
366-
- `MermaidWebContent` renders Mermaid diagrams via CDN mermaid.js with zoom controls and dark theme
367-
- `MarkdownWebContent` renders Markdown via CDN marked.js + highlight.js with GFM and syntax highlighting
368-
- HTML/React/SVG templates include Tailwind CDN, theme CSS vars, and error handling
369-
- React artifacts compile in-browser (Babel) and load as a real ES module; the artifact's `import`/`export` run verbatim against a generated import map that resolves every bare package via an ESM CDN (no source rewriting, no per-library handling)
366+
- `MermaidWebContent` renders Mermaid diagrams with zoom controls and dark theme
367+
- `MarkdownWebContent` renders Markdown (marked + highlight.js) with GFM and syntax highlighting
368+
- HTML/React/SVG templates include Tailwind, theme CSS vars, and error handling
369+
- React artifacts compile in-browser (Babel) and load as a real ES module; the artifact's `import`/`export` run verbatim against an import map the runner builds over the bundled React/ReactDOM globals (no source rewriting)
370370
- `ArtifactPanel` supports fullscreen Dialog mode, version switching, loading indicator, and WebView error overlay
371371
- `ArtifactButton` shows type-specific icons and subtitle (e.g. "Mermaid Diagram", "React Component")
372372
- `ContentPartRenderer` wires `groupArtifactVersions()` to pass version lists to ArtifactButton/ArtifactPanel
373373
- `ArtifactVersionNav` provides prev/next arrows with "v2/3" indicator
374374
- `ArtifactDownloadHelper` shares artifacts via FileProvider temp file + system share sheet. Maps 25+ language extensions including `.mmd` for Mermaid. Sanitizes filenames to 100 chars
375375
- **Gotcha**: FileProvider authority must match app's declared authority in AndroidManifest
376376

377+
### Everything a WebView executes is bundled (no CDNs)
378+
379+
Every script and stylesheet these renderers load ships inside the APK and the iOS bundle.
380+
Nothing is fetched at render time. That is a **hard requirement**, not a preference:
381+
F-Droid's inclusion policy rejects apps that download executable code without explicit
382+
opt-in consent, and a `<script src="https://cdn…">` in a WebView is exactly that. It is
383+
also why the CSPs in these documents name no remote origin — `img-src https:` is the one
384+
deliberate exception, because a remote `<img>` in an artifact is content the user asked to
385+
see rather than code the app chose to run.
386+
387+
What is vendored, at which version, and why each pin sits where it does is in
388+
`scripts/web-assets.json`; `scripts/vendor-web-assets.py` moves the bytes and CI runs
389+
`--check` against a sha256 lock. Repin through **`/update-web-assets`**, never by editing a
390+
vendored file. ~2.5 MB compressed in the APK, mermaid and Babel being two thirds of it.
391+
392+
**How a page finds its assets.** Each document references its scripts *relatively*
393+
(`katex/katex.min.js`), so the only platform-specific part is the document base URL, from
394+
`webAssetBaseUrl()` (`components/web/`). Android resolves Compose Resources in place and
395+
serves `file:///android_asset/…` — which stays readable with `allowFileAccess = false`, so
396+
that setting stays off. iOS **cannot**: WKWebView will not load local subresources for a
397+
page passed to `loadHTMLString`, whatever base URL it gets, so the assets are copied out of
398+
the read-only framework bundle into caches on first use and pages are written next to them
399+
and loaded via `loadFileURL(…, allowingReadAccessTo:)` (`loadVendoredHtml`). The generated
400+
`VendoredWebAssets.FILES` manifest is what tells the copier what to copy, and the copy's
401+
completion marker is written last so a crash mid-copy redoes it rather than trusting it.
402+
Both hosts render nothing until the base URL resolves: a page loaded against a wrong base
403+
renders unstyled and scriptless instead of failing.
404+
405+
**React's module problem.** React 18 publishes no browser-ready ESM — only CommonJS and
406+
UMD — which is the entire reason this used to resolve `import 'react'` through esm.sh. The
407+
UMD builds load as plain scripts and the runner generates small blob modules re-exporting
408+
`window.React` / `window.ReactDOM`, then injects an import map pointing at them *before*
409+
the first dynamic import (hence the classic-script-wrapping-an-async-IIFE shape — a
410+
`type="module"` runner would have resolved its own imports too early). A bare specifier
411+
that is **not** one of the bundled four resolves to a module that throws naming the
412+
package. Third-party npm imports (recharts, lucide-react) therefore no longer work; that
413+
is the known cost of this pass, and the follow-up is an opt-in consent toggle that maps
414+
them back to a CDN for users who accept it.
415+
416+
**Two things this replaced were silently broken**, which is the argument for pinning in
417+
general: markdown syntax highlighting had never worked (marked deleted its `highlight`
418+
option in v5 and ignores one passed to `setOptions`; separately the CDN URL served
419+
CommonJS that cannot define `hljs` in a browser), and the unversioned Babel URL had drifted
420+
onto a major version nothing here was tested against. Neither broke a build or a test.
421+
`VendoredAssetReferenceTest` now pins both halves — no remote origin, and every referenced
422+
path exists in the manifest.
423+
377424
## Media Players
378425
- `VideoContentPlayer` uses ExoPlayer (media3) — 16:9 aspect ratio Card, lifecycle-aware release
379426
- `AudioContentPlayer` uses MediaPlayer — play/pause + seekbar, 250ms polling for progress

0 commit comments

Comments
 (0)