feat(web+appview): theme caching layer (ATB-56) (#96)
* feat(web+appview): theme caching layer (ATB-56)
Add in-memory TTL cache for resolved theme data on the web server to
avoid redundant AppView API calls on every page request.
- New ThemeCache class (theme-cache.ts): TTL entries for policy (single)
and themes (keyed by uri:colorScheme to keep light/dark isolated)
- resolveTheme now accepts an optional ThemeCache; checks cache before
each fetch, populates after successful CID validation; stale CID on
cache hit falls through to a fresh fetch rather than serving stale data
- createThemeMiddleware creates one ThemeCache at startup (shared across
all requests); accepts configurable cacheTtlMs (default 5 min)
- THEME_CACHE_TTL_MS env var exposed via WebConfig.themeCacheTtlMs
- AppView theme endpoints now set Cache-Control: public, max-age=300;
GET /api/themes/:rkey also sets ETag from the theme record CID
* fix(web+appview): address code review feedback on theme caching (ATB-56)
Critical: Cache-Control on GET /themes was set before DB queries, causing
CDNs to cache error responses for 5 minutes. Moved to immediately before
each success return, matching the existing pattern in GET /:rkey.
Important fixes:
- Add THEME_CACHE_TTL_MS to turbo.json env array (Turbo blocks env vars
not declared here, causing tests to receive NaN TTL via turbo)
- Guard parseInt result with Number.isNaN fallback in config (invalid
env value would produce an immortal cache with no operator feedback)
- Add ThemeCache.deleteTheme(): evict stale entry when CID mismatch is
detected so failed re-fetches don't loop per-request indefinitely
- CachedTheme.tokens: Record<string,string> (was unknown) — eliminates
downstream casts and prevents numeric token values entering the cache
- Remove unused re-export of cache types from theme-resolution.ts
Suggestions applied:
- JSDoc on CachedPolicy.availableThemes[].cid explaining live vs pinned refs
- getPolicy()/getTheme() now return Readonly<T> to prevent external mutation
- Comment on ThemeCache construction in middleware explaining why it must
be outside the request handler
Test additions:
- 503 from GET /themes must NOT include Cache-Control header (regression)
- stale CID + failed fresh fetch: eviction means next request retries cleanly
- cache repopulated after stale-CID recovery: third call makes no fetches
- deleteTheme() targeted eviction tests
- Fixed misleading comment in policy-cache-hit test