Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions .claude/skills/update-web-assets/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
---
name: update-web-assets
description: >
Update the third-party JavaScript vendored into the app for the artifact,
diagram, and math WebViews (KaTeX, mermaid, marked, highlight.js, Tailwind,
Babel, React). Checks what is outdated, reads each pin's rationale before
proposing a bump, re-downloads from the official source, verifies the lock,
and runs the gates. Ends by naming what must be checked on a device, because
a broken WebView asset fails silently. Use when bumping a vendored web
library, adding a new one, or investigating whether a rendering bug comes
from a stale pin.
allowed-tools: Bash, Read, Edit, Write, Glob, Grep, WebFetch, AskUserQuestion
argument-hint: "[asset-id ...] (optional, defaults to checking all of them)"
---

# Update vendored web assets

The artifact, diagram and math renderers are WebViews, and every script they execute ships
inside the app. `scripts/web-assets.json` is the registry; `scripts/vendor-web-assets.py`
moves the bytes; `scripts/web-assets.lock.json` records a sha256 per file and CI verifies it.

**Read `scripts/web-assets.json` before doing anything.** Every entry carries a
`pin_reason`, and several of them are load-bearing rather than informational — two pins
must NOT be moved to the newest version, and the reason is in the file, not in this skill.

## What makes this different from a Gradle dependency bump

A wrong version here does not fail the build, fail a test, or throw anything Kotlin can
catch. The failure happens inside the WebView, and the usual shape of it is a blank box or
a feature that quietly stops working. Both libraries this system replaced were already
broken that way before anyone noticed:

- **marked** deleted its `highlight` option in v5. Passing one to `setOptions` is accepted
and silently ignored, so syntax highlighting had been dead with no error anywhere.
- **highlight.js** was being loaded from a URL that served CommonJS. In a browser
`<script>` that throws immediately and never defines `hljs`.

So: the gates below prove the wiring is *consistent*, and only a device proves it *renders*.
Do not report a bump as done on green gates alone.

## Phase 0 — Establish what is stale

```bash
scripts/vendor-web-assets.py --check # tree matches the lock (should be clean first)
scripts/vendor-web-assets.py --outdated # what npm has that we don't
scripts/vendor-web-assets.py --list # what each asset is for, and who uses it
```

`--outdated` prints each pin's `pin_reason` next to any version it reports. Read it. It is
advisory output about a registry that has opinions.

If `--check` fails before you have changed anything, stop and report it — someone
hand-edited a vendored file, or a repin was left half-applied. That is a finding to raise,
not a precondition to quietly re-sync away.

## Phase 1 — Decide, per asset

For each asset the user named (or each one `--outdated` flags, if they named none):

1. Read its `pin_reason` in the registry.
2. If the reason forbids or constrains the bump, say so and **do not** bump it. Two known
standing constraints, both explained in full in the registry:
- **mermaid** must stay on the v10 line (v11+ is ESM-only and fails in Android WebView).
- **react** / **react-dom** must move together and are bounded by UMD availability.
3. Check the upstream changelog for breaking changes in the range, especially anything
touching the entry point path the registry's `files` block names. A package that
reorganises its `dist/` is the most likely way a bump fails, and `--sync` will refuse
with a clear error if a declared path no longer exists.
4. For a major version, use AskUserQuestion rather than deciding alone.

`tailwind` is pinned to a URL, not npm, because Tailwind v3 never published a browser
build. `--bump` refuses it deliberately; repinning it means editing both the version and
every file URL in the registry by hand, and moving to `@tailwindcss/browser` means moving
to v4, which is a behaviour change for artifacts (the registry explains which utilities
changed).

## Phase 2 — Apply

```bash
scripts/vendor-web-assets.py --bump <id> <version>
```

This repins the registry, wipes that asset's directory, re-downloads from the official
tarball, rewrites the lock, and regenerates `VendoredWebAssets.kt`. The wipe is deliberate:
a repin that drops a file must not leave the old one behind to be served.

Adding a **new** asset instead: add a registry entry (with a real `why`, `pin_reason` and
`used_by`), run `--sync`, then reference it from the renderer by a path relative to the
document base — never an absolute URL.

If the bump changes the entry point path or an API the renderer calls, update the HTML
builder in the same pass. The builders are:

| Asset | Built in |
|---|---|
| katex | `androidMain/components/LatexBlock.kt`, `iosMain/components/PlatformMediaComponents.ios.kt` |
| mermaid | `androidMain/components/MermaidDiagram.kt`, `commonMain/…/artifact/MermaidWebContent.kt`, `iosMain/components/PlatformMediaComponents.ios.kt` |
| marked, marked-highlight, highlight | `commonMain/…/artifact/MarkdownWebContent.kt` |
| tailwind, babel, react, react-dom | `commonMain/…/artifact/ArtifactWebContent.kt` |

Note KaTeX and mermaid each have **two** independent HTML builders (Android and iOS) that
do not share code. Changing one and not the other is the easiest mistake to make here.

## Phase 3 — Gates

```bash
scripts/vendor-web-assets.py --check
./gradlew :feature:chat:testDebugUnitTest --tests '*VendoredAssetReferenceTest*' \
--tests '*ReactArtifactRenderTest*'
./gradlew :feature:chat:detekt :feature:chat:detektMetadataCommonMain
./gradlew :app:assembleDebug
```

`VendoredAssetReferenceTest` is the one that matters most: it asserts that no document
references a remote origin, that no CSP lets one execute, and that **every path a document
references exists in the generated manifest** — which is what catches a bump that renamed a
dist file, since the symptom otherwise is a blank WebView.

Per the repo's workflow preference, skip iOS builds unless asked; if asked, stop at the
Gradle framework link (`./gradlew :feature:chat:compileKotlinIosSimulatorArm64`).

## Phase 4 — Report, and name the device check

Report: which assets moved, from and to; the APK size delta if it is material; anything in
a `pin_reason` you deliberately did not act on.

Then state plainly that the gates cannot confirm rendering, and name what to look at for
the assets that actually moved:

| Asset | What to look at on a device |
|---|---|
| katex | a message containing `$$x^2$$` — check glyphs AND that fonts loaded (no fallback serif) |
| mermaid | a ```mermaid block in a message, and a mermaid artifact opened fullscreen |
| marked / marked-highlight / highlight | a markdown artifact with a fenced code block — highlighting present, theme matches light/dark |
| tailwind | an HTML artifact using utility classes |
| 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 |

Do not open a PR. Per the repo's convention, stop at local commits and let the user device-test first.
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ jobs:
- name: Setup Gradle
uses: gradle/actions/setup-gradle@v6.3.0

# The WebView renderers execute these files. Nothing else notices if one is
# hand-edited, replaced, or left behind by a repin -- the app just runs it.
- name: Verify vendored web assets
run: scripts/vendor-web-assets.py --check

- name: Run lint
run: ./gradlew detekt detektMetadataCommonMain :app:lint --continue

Expand Down
11 changes: 11 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,15 @@ Each module has its own `CLAUDE.md` with specific guidance.
- **`UPSTREAM_VERSION`** — Tracks which official tag/commit this mobile build is based on. Updated by the `/sync-upstream` skill.
- **`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.
- **`/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).
- **`scripts/web-assets.json`** — registry of the third-party JavaScript the artifact/diagram/math
WebViews execute (KaTeX, mermaid, marked, highlight.js, Tailwind, Babel, React). All of it is
**vendored into the app**; none of it is fetched at render time. This is required for F-Droid,
which rejects apps that download executable code without explicit opt-in consent — a
`<script src="https://cdn…">` in a WebView is exactly that. It also closes a silent-drift hole:
an unversioned CDN URL served whatever the CDN resolved that day, and two libraries had already
broken that way without failing a build or a test. `scripts/vendor-web-assets.py` downloads the
pins and CI verifies the tree against a sha256 lock. **Repin via `/update-web-assets`**, never by
editing a vendored file. Adding a new WebView dependency means adding a registry entry in the
same PR. See `feature/chat/CLAUDE.md` for how a page resolves them per platform.

- **`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.
55 changes: 51 additions & 4 deletions feature/chat/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -363,17 +363,64 @@ existing upload/usage path already handles them.
reloads its WebView on every content change (mermaid recreates its view outright). The real
preview mounts once, at settle, where the streaming→final swap replaces the subtree anyway.
- Supported types: `text/html`, `image/svg+xml`, `application/vnd.react`, `application/vnd.mermaid`, `text/markdown`/`text/md`, `text/plain`, `application/vnd.code-html`
- `MermaidWebContent` renders Mermaid diagrams via CDN mermaid.js with zoom controls and dark theme
- `MarkdownWebContent` renders Markdown via CDN marked.js + highlight.js with GFM and syntax highlighting
- HTML/React/SVG templates include Tailwind CDN, theme CSS vars, and error handling
- 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)
- `MermaidWebContent` renders Mermaid diagrams with zoom controls and dark theme
- `MarkdownWebContent` renders Markdown (marked + highlight.js) with GFM and syntax highlighting
- HTML/React/SVG templates include Tailwind, theme CSS vars, and error handling
- 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)
- `ArtifactPanel` supports fullscreen Dialog mode, version switching, loading indicator, and WebView error overlay
- `ArtifactButton` shows type-specific icons and subtitle (e.g. "Mermaid Diagram", "React Component")
- `ContentPartRenderer` wires `groupArtifactVersions()` to pass version lists to ArtifactButton/ArtifactPanel
- `ArtifactVersionNav` provides prev/next arrows with "v2/3" indicator
- `ArtifactDownloadHelper` shares artifacts via FileProvider temp file + system share sheet. Maps 25+ language extensions including `.mmd` for Mermaid. Sanitizes filenames to 100 chars
- **Gotcha**: FileProvider authority must match app's declared authority in AndroidManifest

### Everything a WebView executes is bundled (no CDNs)

Every script and stylesheet these renderers load ships inside the APK and the iOS bundle.
Nothing is fetched at render time. That is a **hard requirement**, not a preference:
F-Droid's inclusion policy rejects apps that download executable code without explicit
opt-in consent, and a `<script src="https://cdn…">` in a WebView is exactly that. It is
also why the CSPs in these documents name no remote origin — `img-src https:` is the one
deliberate exception, because a remote `<img>` in an artifact is content the user asked to
see rather than code the app chose to run.

What is vendored, at which version, and why each pin sits where it does is in
`scripts/web-assets.json`; `scripts/vendor-web-assets.py` moves the bytes and CI runs
`--check` against a sha256 lock. Repin through **`/update-web-assets`**, never by editing a
vendored file. ~2.5 MB compressed in the APK, mermaid and Babel being two thirds of it.

**How a page finds its assets.** Each document references its scripts *relatively*
(`katex/katex.min.js`), so the only platform-specific part is the document base URL, from
`webAssetBaseUrl()` (`components/web/`). Android resolves Compose Resources in place and
serves `file:///android_asset/…` — which stays readable with `allowFileAccess = false`, so
that setting stays off. iOS **cannot**: WKWebView will not load local subresources for a
page passed to `loadHTMLString`, whatever base URL it gets, so the assets are copied out of
the read-only framework bundle into caches on first use and pages are written next to them
and loaded via `loadFileURL(…, allowingReadAccessTo:)` (`loadVendoredHtml`). The generated
`VendoredWebAssets.FILES` manifest is what tells the copier what to copy, and the copy's
completion marker is written last so a crash mid-copy redoes it rather than trusting it.
Both hosts render nothing until the base URL resolves: a page loaded against a wrong base
renders unstyled and scriptless instead of failing.

**React's module problem.** React 18 publishes no browser-ready ESM — only CommonJS and
UMD — which is the entire reason this used to resolve `import 'react'` through esm.sh. The
UMD builds load as plain scripts and the runner generates small blob modules re-exporting
`window.React` / `window.ReactDOM`, then injects an import map pointing at them *before*
the first dynamic import (hence the classic-script-wrapping-an-async-IIFE shape — a
`type="module"` runner would have resolved its own imports too early). A bare specifier
that is **not** one of the bundled four resolves to a module that throws naming the
package. Third-party npm imports (recharts, lucide-react) therefore no longer work; that
is the known cost of this pass, and the follow-up is an opt-in consent toggle that maps
them back to a CDN for users who accept it.

**Two things this replaced were silently broken**, which is the argument for pinning in
general: markdown syntax highlighting had never worked (marked deleted its `highlight`
option in v5 and ignores one passed to `setOptions`; separately the CDN URL served
CommonJS that cannot define `hljs` in a browser), and the unversioned Babel URL had drifted
onto a major version nothing here was tested against. Neither broke a build or a test.
`VendoredAssetReferenceTest` now pins both halves — no remote origin, and every referenced
path exists in the manifest.

## Media Players
- `VideoContentPlayer` uses ExoPlayer (media3) — 16:9 aspect ratio Card, lifecycle-aware release
- `AudioContentPlayer` uses MediaPlayer — play/pause + seekbar, 250ms polling for progress
Expand Down
Loading