Conversation
Product specs for the JS Asset Proxy feature (#603). Includes the original PRD and two engineering design documents covering: - JS Asset Proxy: config schema, KV data model, fetch/cache pipeline, HTML head injection, security hardening, and Fastly provisioning steps - JS Asset Auditor: Claude Code slash command that sweeps a publisher page via Chrome DevTools MCP and generates/diffs js-assets.toml Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Well-structured engineering spec for JS asset proxying through Fastly Compute. The phased approach, KV metadata/body separation, and Auditor-as-skill design are strong. Main concerns are cross-document inconsistencies (PRD vs eng spec) and missing details needed for correct implementation (compression strategy, cache invalidation, TLS verification).
Blocking
❓ question
- PRD and eng spec disagree on config file location: PRD §9 defines config as new sections in
trusted-server.tomlwith[backends.*]; eng spec uses a separatejs-assets.tomlwith dynamic backends. Fundamental contradiction — update PRD to match eng spec or annotate it as superseded. - Slug derivation algorithm differs: PRD (line 100) derives from publisher ID; eng spec (line 126) derives from
sha256(domain + origin_url). Different algorithms produce different slugs. PRD examples are inconsistent with the eng spec hash-based approach. - Stale-on-error inconsistency: PRD (line 201) returns 502 for origin failures, with stale fallback only for expired KV entries. Eng spec (line 167) makes stale fallback conditional on KV existence for all origin failures, but doesn't cover non-200/304 responses. Align and make explicit for all failure cases.
See inline comments for additional blocking items on specific lines.
Non-blocking
🤔 thinking
- No timeout for origin fetches:
BackendConfig::from_url()uses the default 15s first-byte timeout. PRD targets p99 < 300ms. Consider documenting a configurablefetch_timeout_ms.
🌱 seedling
- No CSP guidance: None of the three docs mention
Content-Security-Policyimplications. Publishers with strictscript-srcdirectives need to allow the new asset domain. Worth documenting as a deployment consideration.
⛏ nitpick
- Inconsistent terminology: "TS", "TS Lite", "Trusted Server" used interchangeably. "TS Lite" appears in PRD but is never defined and absent from eng spec.
👍 praise
- Phased approach: Three phases with clear file lists, each independently testable, logical dependency order. Auditor delivery timed between Phase 1 and 2 is particularly thoughtful.
- KV metadata/body separation: Reading metadata first to avoid WASM heap cost shows excellent understanding of platform constraints.
- Auditor as Claude Code skill: Pragmatic choice — avoids bloating the production binary, leverages existing Chrome DevTools MCP, diff mode with commented-out TOML is a nice safety mechanism.
CI Status
- cargo fmt: PASS
- cargo test: PASS
- vitest: PASS
- format-typescript: PASS
- format-docs: FAIL — new markdown files need formatting (
cd docs && npm run format:write) - CodeQL: PASS
- integration tests: PASS
|
|
||
| ### 6.1 KV store | ||
|
|
||
| **Store name:** `js_asset_store` (configured in `trusted-server.toml` alongside `ec_store` and `partner_store`) |
There was a problem hiding this comment.
❓ question — ec_store and partner_store do not exist in the codebase. Current KV stores are configured per-feature inline (e.g., synthetic.counter_store, consent.consent_store). There is no [kv_stores] section in trusted-server.toml. This misleads implementers into thinking a centralized KV pattern already exists.
Fix: Remove "alongside ec_store and partner_store" or note this is a new pattern being introduced.
|
|
||
| When TS is operating as a full HTML proxy (HTML proxy enabled in `trusted-server.toml`), it injects the asset's `<script>` tag automatically using the existing `html_processor.rs` head injection pipeline. | ||
|
|
||
| The injection point is the start of `<head>`, before the TS core bundle, consistent with how integration head injectors work today (`head_inserts()` in `integration_registry.rs`). This ensures the vendor JS is available as early as possible in page parse order. |
There was a problem hiding this comment.
♻️ refactor — File is actually registry.rs in crates/trusted-server-core/src/integrations/, not integration_registry.rs.
|
|
||
| ### 9.1 Backend configuration | ||
|
|
||
| Each distinct origin hostname requires a declared backend in `trusted-server.toml` (Fastly or similar Compute requires backends to be declared at deploy time): |
There was a problem hiding this comment.
♻️ refactor — The engineering spec explicitly resolves [backends.*] as "Removed" (dynamic backends via BackendConfig::from_url() instead). This entire section is superseded. Either strike it or annotate as superseded by the eng spec.
|
|
||
| ### 5.3 Why opaque and not content-hash | ||
|
|
||
| Content-hash URLs (e.g., `tsjs-core.abc123.js`) are appropriate for assets TS generates itself, where the hash is computed at build time. Third-party JS has content that changes independently — using a publisher-token-derived slug decouples the URL from the content version. The ETag mechanism handles freshness. |
There was a problem hiding this comment.
⛏ nitpick — The TS bundle does not actually use content-hash filenames. Current path is /static/tsjs=tsjs-unified.min.js with ETag-based caching via serve_static_with_etag(). The comparison is slightly misleading about current behavior.
| - `fastly.toml` — add `[[local_server.kv_stores.js_asset_store]]` entry | ||
| - `crates/trusted-server-core/src/settings.rs` — add `KvStores`, `JsAsset`, extend `Settings` | ||
| - `crates/trusted-server-adapter-fastly/src/main.rs` — add asset path routing before publisher fallback | ||
| - `crates/trusted-server-adapter-fastly/build.rs` — include `js-assets.toml` |
There was a problem hiding this comment.
❓ question — This file does not exist. The build.rs that loads trusted-server.toml is at crates/trusted-server-core/build.rs. The adapter crate has no build.rs. Implementers will look in the wrong place.
Fix: crates/trusted-server-core/build.rs
|
|
||
| **Scope:** | ||
| - `handle_js_asset()` handler wired into route dispatch | ||
| - Cold path: `BackendConfig::from_url()` → origin fetch → compress body to brotli → write compressed bytes + metadata to KV |
There was a problem hiding this comment.
❓ question — Two gaps in the origin fetch strategy:
-
certificate_check:BackendConfig::from_url()requires acertificate_check: boolparameter. The existingsettings.proxy.certificate_checkapplies only to publisher origin proxy. Where does the JS asset proxy get this setting? -
Accept-Encoding/ compression: The spec says "compress body to brotli" but does not address what happens when the origin already returns compressed content (Content-Encoding: brorgzip). The PRD (line 158) says "matching the origin Content-Encoding", which contradicts always storing as brotli.
Fix: Specify Accept-Encoding: br on origin fetches to get brotli directly. Document behavior when origin returns a different encoding. Add a note about TLS verification policy.
|
|
||
| **Caching layers — all aligned at 1h:** | ||
| - **KV store TTL (1h):** how long TS keeps its cached copy before issuing a conditional GET to the vendor origin | ||
| - **Fastly CDN edge cache (s-maxage=3600 via `public, max-age=3600`):** Fastly serves the asset from edge POP without hitting the TS WASM at all |
There was a problem hiding this comment.
❓ question — Cache-Control: public, max-age=3600 enables Fastly HTTP edge cache in front of KV, creating a dual-cache that can diverge:
- No
Varyheader specified — risk if the service ever serves multiple publishers - No
Surrogate-Keyfor targeted edge purging - A KV metadata-only update (304 path) does not purge the Fastly edge cache, which continues serving the previous version until its own max-age expires
Is the dual-cache intentional? If KV should be the sole cache layer, consider s-maxage=0 or Cache-Control: private, max-age=3600. If dual-cache is intended, document the implications and add Surrogate-Key for purging.
| ## Config Contract: `js-assets.toml` (new file) | ||
|
|
||
| Separate from `trusted-server.toml` to avoid bloat. Loaded via the same `include_bytes!` / `build.rs` pattern as the main config. | ||
|
|
There was a problem hiding this comment.
🤔 thinking — The current build.rs is tightly coupled to a single TOML file (trusted-server.toml -> env merge -> target/trusted-server-out.toml -> include_bytes!). Adding js-assets.toml via the same pattern requires either extending build.rs to merge both files into one output, or a separate include_bytes! with its own loading path. Worth specifying which approach so Phase 1 does not start with an architectural decision.
| pub js_asset_store: String, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Deserialize, Serialize, Validate)] |
There was a problem hiding this comment.
🤔 thinking — JsAsset derives Validate but no #[validate(...)] attributes are shown. The slug, path, and origin_url fields would benefit from validation annotations (URL format, path prefix, slug pattern). Without them the Validate derive is a no-op and malformed config entries would only fail at runtime.
|
|
||
| Path segments matching either pattern are replaced with `*`: | ||
| - Semver: `\d+\.\d+[\.\d-]*` (e.g., `1.19.8-hcskhn`) | ||
| - Hash-like: `[a-f0-9]{6,}` or `[A-Za-z0-9]{8,}` between path separators |
There was a problem hiding this comment.
🌱 seedling — The hash-like pattern [A-Za-z0-9]{8,} would false-positive on common vendor path segments like analytics (9 chars), openrtb2 (8 chars), prebidjs (8 chars). Worth noting as a known limitation requiring operator review of wildcard substitutions.
2 participants
Summary
Engineering spec for the JS Asset Proxy feature — brings third-party JS delivery inside the publisher's operational boundary by proxying vendor assets through Fastly Compute.
Closes #604
Docs added
docs/js-asset-proxy-prd.md— original product requirementsdocs/superpowers/specs/2026-04-01-js-asset-proxy-design.md— engineering spec covering config schema, KV data model, 3-phase implementation plan, caching layers, degraded behavior, and Fastly KV store provisioningTest plan