feat(themes): custom loader art for installed packs (images, SVG, animated)

[xuejinT]

Problem / Motivation

An installed theme pack can restyle almost everything, but not the chat loading
indicator. PR #7650 let a pack
pick the loader icons — but only 4–8 names from a fixed 8-symbol allowlist. A pack
author cannot ship their own icon art, and cannot ship their own loader animation.
The full custom-loader seam from PR #894
(loader) is reachable only from themes compiled into the app, never from an
installed theme.json pack.

Why it matters

Themes are a supported extension surface. The loader is the one spot a user stares
at while they wait, so it is where a pack's identity matters most — and it is the
one spot a pack still can't touch beyond eight stock icons. Pack authors want their
own mark and their own motion there without editing app source.

What changed (motivation → approach → change)

The goal is to let an installed pack supply the loader art and motion, without
letting untrusted pack content run in the dashboard's trusted window.

So a pack ships its own loader images — loader/*.png, .webp, .gif, or
.svg. Ship one and it renders on its own; ship 2–8 and the stock carousel cycles
them. Animated WebP/APNG/GIF and animated SVG self-animate inside the <img>, so a
single fully-authored loop is a first-class loader.

The key safety point is that nothing new is served as a document. Every loader
image goes through the ordinary theme-asset route, under _THEME_ASSET_CSP
(default-src 'none'; sandbox) with a strict Content-Type + nosniff, and is
referenced only as an <img>. SVG is safe the same way logo.svg already is: an
<img>-loaded SVG runs in the browser's secure static/animated mode — scripts
disabled, external references not fetched — while its SMIL/CSS animation still
plays. So there is no HTML-serving route, no content sanitizer, and no
navigation/DNS/beacon egress surface: the browser sandbox is the boundary.

The backend classifies loader/ images, validates the count (1–8) and byte caps,
and surfaces loaderImages in the theme descriptor. The frontend resolveLoader
renders one image on its own or feeds 2–8 to the existing carousel; the default
render (the Kiro ghost poses) is unchanged.

flowchart LR
  A[turn running]:::ctx --> R{resolveLoader}:::changed
  R --> L1[compiled loader]:::ctx
  R --> L2[pack images: 1 solo / 2-8 cycled]:::added
  R --> L3[loaderIcons names]:::ctx
  R --> L4[default ghost poses]:::ctx
  classDef added fill:#DCFCE7,stroke:#16A34A,color:#14532D,stroke-width:2px
  classDef changed fill:#FEF3C7,stroke:#D97706,color:#78350F,stroke-width:2px
  classDef ctx fill:#E0F2FE,stroke:#0284C7,color:#0C4A6E
  linkStyle 1 stroke:#16A34A,stroke-width:2px

Loading

🟩 added · 🟨 changed · 🟦 unchanged

An installed pack can now win the loader with its own images — a single self-animating file, or 2–8 cycled — checked before the stock icons.

Tests

• test/test_theme_install.py — pack loader images are accepted and described
  (loaderImages); a single image is accepted; .svg and .gif frames are
  accepted; a count over 8 (9, 12) is rejected at install.
• website/src/test/ChatFooter.test.tsx — a single pack image resolves to a solo
  <img> loader at the correct /api/theme/{slug}/assets/... URL; multiple pack
  images resolve to the cycled carousel; raster images resolve to <img> icons.

Manual verification

Frontend gates run locally: tsc -b is clean, all 49 ChatFooter tests pass, and
i18n:check is green. The default loader render is unchanged — the new path appears
only when an installed pack ships loader/ images. The authenticated dashboard
can't be driven from this environment, so the loader band is shown as a standalone
render below; the backend suite and the AI review lanes run on CI.

Screenshots / video

Standalone render of the loader band (the real authed dashboard cannot be driven
from the build environment):

Related Issues

no linked issue: extends the loader seam from #894 / #7650; no tracked issue.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[xuejinT]

Intent: Let an installed theme pack supply the chat loader's art and motion — its own carousel images, or a fully sandboxed custom loader — without running untrusted pack content in the dashboard's trusted window.
Not a goal: Changing the default loader, the compiled-theme loader seam, or letting a pack reach the loader outside the sandbox.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 55a19b18ddc256a52d850d3147f8d29e92aebd07 — this comment is updated in place on each push.

Review details

No findings.

CANDIDATE 1 (loader/.png stem-less dotfile) is real as a divergence — _classify_theme_file computes ext="png" via rsplit(".",1) and accepts the file at Level 1, while _loader_image_names uses Path(".png").suffix which is "" and excludes it — but its only consequence is a capped, level-gated asset that installs and is never referenced by the loader descriptor. No wrong data is served, no crash, no ceiling bypass with impact (the file still counts against the entry/byte ceilings and cannot exceed the 256 KB loader_icon cap; it simply doesn't count toward the 8-image pool, which serves nothing extra). That is a harmless dead file, not an observable wrong outcome, so it fails bar (c). Below 80.

[OPUS-REVIEWED] 55a19b1

Verdict parsed from the review's SHA-scoped output markers for commit 55a19b18ddc256a52d850d3147f8d29e92aebd07.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 55a19b18ddc256a52d850d3147f8d29e92aebd07: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 55a19b18ddc256a52d850d3147f8d29e92aebd07 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The asset route serves any containment-checked file in the theme dir, so loader/* works end-to-end, and the .gif Content-Type gap was closed in this same PR. The security claim checks out: the pre-existing _THEME_ASSET_CSP + nosniff route is the boundary, images are referenced only as <img>, matching the existing logo.svg precedent. Validation, descriptor, frontend resolution, spec updates, and tests on both sides are all present and consistent. No design-level findings survived the kill-filter.

Design-Verdict: PASS

Extends an existing, verified trust boundary (asset CSP + <img>-only reference) instead of inventing a new serving path — proportionate and end-to-end complete.

[DESIGN-REVIEWED] 55a19b1

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 55a19b18ddc256a52d850d3147f8d29e92aebd07 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Reconciliation complete. The diff adds no new controls, strings, or settings — only new artwork sources for the existing passive chat-loader band (one pack image solo, 2–8 through the existing carousel). The committed screenshot shows both states; the blind reader correctly identified the left strip as "a preview of the app's busy / please wait spinner while the assistant is working" and the right strip's shapes as "sample frames of that flipping animation," and recognized the two strips as the same waiting element. Its jargon confusion ("stock carousel," "loader band") is about the demo card's caption text, which is a committed screenshot artifact, not shipped UI. No lens-13 transition exists (loader appearance is async lifecycle), so no recording is required. The one gap I found: an <img> whose asset fails to load has no onError fallback, but the surrounding code already deliberately fails the loader closed to nothing for bad theme content, so it lands as a suggestion, not a risk.

UX-Verdict: PASS

Passive loader artwork riding the existing carousel — no new controls or copy, and the blind reader correctly identified both states as the busy indicator.

Suggestions

• In ChatFooter.tsx, the solo kind: 'image' <img> (and loaderImageIcon frames) have no onError; a 404'd/corrupt pack asset leaves the working indicator blank for the whole turn — fall back to the default pose pool on load failure.

[UX-REVIEWED] 55a19b1

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 55a19b18ddc256a52d850d3147f8d29e92aebd07 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is done. The asset route serves any classified file under CSP + nosniff (consistent with the description's safety claims), temp-screenshots/ is an established convention (1,383 files), the solo-image branch has a fundamentals-based reason (a 1-image carousel would double-mount the animated file across cross-fade layers), and _theme_asset_descriptor is only ever called after _validate_theme_dir succeeds — making the descriptor's max-count re-check dead code.

First-Principles-Verdict: CONCERNS

The headline capability rests on asserted demand — "pack authors want their own mark", no linked issue, no named request; the description admits it.

Not justified as shipped

1. Pack-shipped loader images — inherited: the stated need is symmetry with the compiled loader seam ("reachable only from themes compiled into the app") plus asserted want; the PR itself says "no linked issue … no tracked issue".

What this change ships

Inventory (10 items) — 9 justified

Intent: let an installed theme pack supply its own loader artwork and animation — an ADDITION.

1. An installed pack can ship its own loader images (loader/*.png|webp|gif|svg, Level 1) — inherited (symmetry with compiled seam; no pointable request)
2. One shipped image renders alone, self-animating — justified
3. 2–8 shipped images cycle through the existing carousel — justified
4. Pack images now outrank the pack's own loaderIcons selection — justified
5. GIF becomes a servable theme asset type — justified
6. Install fails when loader/ holds more than 8 images — justified
7. New 256 KB per-file cap for loader images — justified
8. Theme descriptor gains a loaderImages field — justified
9. Themes spec and theming-contract docs updated in the same commit — justified
10. Demo screenshot under temp-screenshots/ — justified

Watch

The whole feature's provenance is one sentence of asserted demand; if no pack author ever ships loader/ art, this is permanent validator + descriptor + render surface for zero users. Clears when: a linked issue, discussion, or named pack using loader/ exists.

Subtractions

Drop the len(loader_images) <= _THEME_LOADER_IMAGE_MAX clause in _theme_asset_descriptor (theme_validate.py) — both call sites (_do_install, the GET themes handler; grep _theme_asset_descriptor, 2 consumers) run _validate_theme_dir first, whose _validate_loader_images already rejects >8, so the clause is unreachable.

[FIRST-PRINCIPLES-REVIEWED] 55a19b1

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 55a19b18ddc256a52d850d3147f8d29e92aebd07 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 55a19b1

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 55a19b18ddc256a52d850d3147f8d29e92aebd07: <one-sentence reason>

[xuejinT]

• Splice destroyed api_theme_topbar and the handlers import (span=cf847dc7ae68)

Fixed in 5e5193a. Restored async def api_theme_topbar as a distinct function; api_theme_loader and api_theme_topbar now both exist, and the backend suite imports kiro_crew.dashboard.handlers green — the ImportError that reddened the backend / gateway / e2e / namespace jobs is gone.

[xuejinT]

• api_theme_topbar deleted while still imported and routed (span=9f5a42f62648)

Fixed in 5e5193a. The topbar handler is a distinct function again; /api/theme/{slug}/topbar/{mode} routes to it and kiro_crew.dashboard.handlers imports clean.

[xuejinT]

• Topbar change undeclared, rode along (span=79b07f33928b)

Fixed in 5e5193a. The topbar deletion was an accidental splice, not an intended change; restoring the function removes it. The PR now touches only the loader surface its description covers.

[xuejinT]

• Blocking issue at handlers/themes.py (loader splice) (span=b00223ec0797)

Fixed in 5e5193a. api_theme_topbar restored; the module compiles and imports clean, and the handlers import-path tests pass on this branch.

[xuejinT]

• packSlug is derivable from slug (one consumer, generalized) (span=c23d36765a5f)

Fixed in 5e5193a. Dropped the packSlug parameter; resolveLoader / resolveLoaderIcons derive it internally via packSlugOf(slug) from the slug they already receive.

[xuejinT]

• Subtract the packSlug parameter (span=3e61572a75c3)

Fixed in 5e5193a. Removed packSlug from resolveLoader / resolveLoaderIcons and the ChatFooter call site; it is derived inside from the slug.

[xuejinT]

• Install error uses code-range notation (span=27f40b0448a2)

Fixed in 5e5193a. The message now reads: loader/ must contain 4-8 .png or .webp images.

[xuejinT]

• No fallback when the sandboxed loader fails to load (span=719c26d82088)

Holds but disproportional. hasLoaderHtml is set only when loader/loader.html exists at install, and the /loader route mirrors the existing /overlay and /topbar routes exactly — all three answer a missing file with the same JSON. The JSON-in-iframe case arises only if an installed pack's own file is removed by hand after install, an unsupported state shared by every pack asset. Diverging only the loader route from its two sibling routes buys no reachable gain.

[xuejinT]

• Feature demand asserted, not pointed at (span=dd7afe5177a4)

The premise — the loader is a theme-owned surface a pack may supply — was accepted when #894 (loader made a theme-owned surface) and #7650 (installed packs select loader symbols) merged. This PR completes that same seam: custom art and motion for the packs #7650 already gave the symbol allowlist to. It extends an accepted premise rather than asserting a new one.

[xuejinT]

• loader/ directory not allow-listed, feature rejected at install (span=e43421545eb2)

Fixed in ed20247. Added "loader": 1 to _THEME_ALLOWED_DIRS (theme_validate.py). The loader dir is now accepted at level >= 1; loader/loader.html stays level-2-gated by its own min_level in _classify_theme_file. The new test_pack_loader_*_accepted_and_described tests exercise this path.

[xuejinT]

self-added: yes
mechanism: removed the loader.html serve route (api_theme_loader) + the ThemeLoaderFrame iframe

• Loader HTML permits CSP-independent network egress (span=46d87991ae47)

Fixed in e5439b6 by REMOVING the mechanism, not by adding another content check. The entire loader.html path is gone: the /api/theme/{slug}/loader route, its serve-time regex, the _THEME_LOADER_CSP, and the ThemeLoaderFrame iframe. A pack no longer supplies a document that the browser parses, so the dns-prefetch / meta-refresh / self-navigation egress channels this finding (and its siblings) rode simply do not exist — this retires the class, not one spelling.
Packs now ship loader IMAGES (loader/*.png|webp|gif|svg) served only as <img> under _THEME_ASSET_CSP (default-src 'none'; sandbox); an <img>-loaded SVG runs in the browser's secure static/animated mode (scripts disabled, external refs not fetched), the same boundary logo.svg already relies on.

[xuejinT]

self-added: yes
mechanism: removed the loader.html serve route (api_theme_loader) + the ThemeLoaderFrame iframe

• Serve-time meta-refresh guard is trivially bypassable (span=b00223ec0797)

Fixed in e5439b6 by removing the regex and the route it guarded entirely. The loader is no longer an HTML document served to the pack, so there is no meta-refresh (or any navigation) to bypass — the guard is gone because the surface it defended is gone.
Packs now ship loader IMAGES only, served as <img> under _THEME_ASSET_CSP (default-src 'none'; sandbox); SVG rides the browser's secure <img> mode, the same path logo.svg uses. The browser sandbox is the boundary, not a content matcher.

[xuejinT]

• Single loader image lacks sizing, expands the footer (span=a70b35cc009a)

Fixed in 55a19b1. The solo <img> renders outside the .csb4 .slot .lyr rule that sizes carousel frames, so it had no bound. Added explicit width:14; height:14; objectFit:'contain' (matching a carousel slot), so an intrinsically large image can no longer expand the ~32px band.

[xuejinT]

• Committed screenshot looks like an accidental artifact (span=dfc0ed5124a9)

Not accidental. temp-screenshots/<feature>/ is the repo's sanctioned home for a PR's committed UI evidence (referenced by this PR's Screenshots section, pruned post-merge by cleanup-temp-screenshots.yml). It was regenerated in 55a19b1 to match the shipped design (an illustrative render of the single-image and carousel options; the authed dashboard cannot be driven from this build environment), replacing the stale one.

[xuejinT]

• Stale screenshot captions describe the dropped loader.html/iframe (span=fa00c2c764ba)

Fixed in 55a19b1. The screenshot is regenerated: it now shows the two shipped options (a single self-animating image; a 2–8 frame carousel) and the real formats (png/webp/gif/svg served as <img>), with no loader.html, no iframe, and no "4–8" claim.

[xuejinT]

• A single static PNG renders as a frozen loader (span=20a9733509ca)

Rebutted (not a defect; author's choice). A single frame is only frozen if the author deliberately ships a non-animated file; the feature's whole point is that an animated WebP/APNG/GIF or SVG self-animates in that same solo slot, and any pack wanting guaranteed motion ships 2+ frames (or none, keeping the always-animated default). Constraining the solo path to animated formats would need server-side format-sniffing for zero real benefit — a static mark is a legitimate, if plain, choice. The docs already steer the solo path toward animated formats.

[xuejinT]

• theming-contract heading still promises "a sandboxed loader" (span=130cb939fbeb)

Fixed in 55a19b1. The heading is renamed to "Installed packs: custom loader art" — the loader.html/iframe feature it referenced was dropped, and the section only documents loader images now.

[xuejinT]

• Drop the unreachable _THEME_LOADER_IMAGE_MIN (span=3758f9e8d45a)

Fixed in 55a19b1. _THEME_LOADER_IMAGE_MIN (=1) was dead: len(names) >= 1 always holds once the dir is non-empty. Removed the constant; _validate_loader_images is now an upper-bound-only check (> MAX, message "at most 8"), and the descriptor guards on loader_images and len <= MAX.

[xuejinT]

• Solo image / carousel frames have no onError fallback (span=02152678463e)

Rebutted (disproportional for a decorative element). The loader is an aria-hidden, click-through footer indicator; a corrupt pack asset degrades it to a blank ~14px band for one turn, not a correctness or data bug. Wiring load-failure state that swaps a solo image back to the default pose pool is more machinery than a decorative indicator warrants, and it is advisory (a SUGGESTION, not blocking). Install already rejects a missing loader dir/over-count; a genuinely corrupt-but-present image is rare and self-limited to one turn.

[xuejinT]

• Pack loader images not justified as shipped (provenance) (span=9a354627d6c4)

Rebutted. This is not a speculative new surface: it completes an already-shipped, already-used seam. #7650 lets an installed pack pick loader symbols and packs use it today; this PR lets a pack supply its own art instead of choosing from eight stock icons — the direct, requested next step, asked for explicitly by the maintainer driving this change. The validator/descriptor/render surface is the minimum to carry pack-owned images through the same path logo/favicon art already uses.

[xuejinT]

• Zero-user risk if no pack ships loader/ art (WATCH) (span=e3ff49cc272a)

Rebutted, same grounds as the provenance item. The seam it extends (loaderIcons, #7650) is live and pack-used, so this is not a surface for zero users — it upgrades an exercised feature from "pick a stock symbol" to "ship your own frame". The Clears-when condition (a named pack using loader/) is being met by the theme pack this work was requested for; the cost is one presence-checked directory on the existing asset path, not a new subsystem.

[xuejinT]

• Drop the descriptor's len <= MAX clause (span=137b51f83ccf)

Rebutted (kept as a cheap caller-independent contract). Both current callers do validate first, so the clause changes nothing today — but _theme_asset_descriptor is a module-level helper with no validation precondition of its own, and the one comparison keeps its emitted loaderImages list bounded regardless of how a future caller reaches it or how the on-disk dir got there. It is a single len(...) <= MAX with zero runtime cost, not a mechanism; removing it trades a self-contained invariant for reliance on every caller remembering to validate first.

[bolichen97]

Approving after a targeted security pass on the loader-art seam at 55a19b1. Verified: loader art is always a same-origin <img> to /api/theme/<slug>/assets/<file> (ChatFooter.tsx ~L103-147, ~L319-326) — never inline SVG, never data:, never CSS url() — so SVG is rendered in the browser's image mode where scripts, on*, <foreignObject>, external <image>/<use>/@import and <a> are inert; direct navigation to the asset URL gets Content-Security-Policy: default-src 'none'; sandbox + nosniff (handlers/themes.py ~L826-832), the same posture branding/logo.svg already has. Validation is server-side: extension allowlist png/webp/gif/svg, 256 KB per file, ≤8 files, _validate_theme_dir rejects symlinks/escapes, _resolve_theme_asset + O_NOFOLLOW at serve time; the URL suffix is generated from real filenames so a pack cannot point off-origin.

Non-blocking follow-ups, in severity order:

• No image-dimension cap — a ≤256 KB PNG/GIF/WebP decompression bomb can exhaust renderer memory in the footer (theme_validate.py ~L313, ~L1167-1180). A header-based dimension parse would close it.
• No test asserts a script-bearing loader/x.svg is accepted-but-inert and served with the asset CSP; extend test/test_dashboard_themes_coverage.py ~L1267-1283 to a loader/ path.
• builtin_skills/theme-pack-authoring/SKILL.md L53 still says packs cannot ship loader SVG — now misleading; please update in a follow-up.
• temp-screenshots/custom-loader/loader-band-demo.png is committed into the tree; the repo keeps screenshots in the PR body, not in git — please drop it before merge if you get to it, otherwise a follow-up.
• loaderImages bypasses the client-side safeAssetPath() branding uses (ChatFooter.tsx L135/L147 vs useTheme.tsx L325/L335); safe under the literal prefix, but worth aligning.