Serve firmware update data from static assets instead of D1#347
Conversation
Precompute the lookup data at build time (yarn build:data) into per-manufacturer JSON shards plus a manifest, deployed atomically with the worker via Workers Static Assets. The API routes now look up devices in memory from memoized shards instead of querying D1, and the separate post-deploy upload step (and its flaky admin upload protocol) is gone from CI. The manifest version uses the same content hash as the previous upload flow, so existing edge cache keys remain valid across the cutover. Verified against production: 300+ request pairs across API v1-v4, including region filtering, $if conditions and multi-vendor batches, returned byte-identical responses. The D1 code, admin routes and database are left in place for rollback and will be removed in a follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Moves firmware-update lookup data from D1 to bundled Workers Static Assets, generated at build time and read in-memory at runtime to reduce deploy flakiness and remove the post-deploy upload step.
Changes:
- Add a build step (
yarn build:data) that compilesfirmwares/**into sharded JSON plus a manifest underdist/data. - Replace D1 lookups in the API handlers with static-asset lookups via
src/lib/dataOperations.ts(memoized per isolate). - Update deploy/test workflows and configuration to build and ship the static assets with the worker.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| wrangler.toml | Configures Workers Static Assets binding (DATA) to ship prebuilt data. |
| src/worker.ts | Adds DATA: Fetcher binding to the worker environment. |
| src/routes/api.ts | Switches API lookups from D1 to asset-backed dataOperations. |
| src/maintenance/build-data.ts | New build script to shard configs + write manifest/shards to dist/data. |
| src/lib/dataOperations.ts | New runtime lookup layer that fetches/parses manifest and shards from assets. |
| src/lib/dataOperations.test.ts | Adds tests for asset-backed lookups and $if evaluation. |
| src/lib/dataFormat.ts | Defines manifest/shard formats and shard path helpers. |
| package.json | Adds build:data and wires it into dev and test:ci. |
| .gitignore | Ignores generated dist/ output. |
| .github/workflows/test-and-release.yml | Builds static data before deploying; removes upload step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AlCalzone
left a comment
There was a problem hiding this comment.
deep-review — Serve firmware update data from static assets instead of D1
Verdict: Ship with minor changes. No blockers — the D1→Static-Assets move is the idiomatic Cloudflare pattern and directly kills the flaky post-deploy upload. The core lookup path is well-tested and prod-verified. I'd address the two Majors worth gating on — the empty-build guard and failure logging — before leaning on this in prod; the other two Majors (build-pipeline tests, serial v4 fetch) are strong follow-ups. 11 inline findings below.
Top 3 risks
- A failed/empty config read at build time ships an empty dataset that returns HTTP 200 "no updates" for every device, with no error signal.
- No log on any data-fetch failure path, so a partial deploy is indistinguishable from an unknown vendor.
- v4 batch fetches manufacturers serially and lost all edge caching.
Strengths
- Public lookup surface preserved → minimal
api.tschurn. run_worker_first = truecorrectly keeps shards off the public router (verified: unmatched paths 404).- Failures deliberately not memoized → transient errors self-heal.
- In-flight promise dedup via
Map/??=is correct; deterministic.sort()for hash + layout is the right instinct. WeakMap<Fetcher>holds up — CF guaranteesenvis identity-stable per isolate, so parse-once-per-isolate works as intended.
Theme — the cutover is less seamless than the description claims. Two independent mechanisms break the "cache keys stay valid / byte-identical responses" story: the .sort() changes the version hash vs. the retired upload.ts (see build-data.ts:44), and the schema's file key order differs from the old D1 reconstruction (see dataOperations.ts:137). Both are one-time and self-healing with no correctness or client impact — worth either accepting-and-rewording or just knowing.
FYI / context (no action required in this PR)
- Retirement debris (tracked, Phase 4).
registerAdminis still wired (src/app.ts:29) so/admin/config/uploadstays live;cachedD1Operations.tsand thed1Operationslookup path are now dead;CONFIG_FILES+ theuploadscript are orphaned. Security-relevant piece: prioritize deleting the prodADMIN_SECRETso the still-live, now-untested privileged endpoint (fail-closed auth, writes to a D1 nobody reads) isn't a stale surface. - Rollback story. "D1 kept for rollback" has no live runtime switch — rollback = redeploy the previous worker, and D1 goes stale immediately since CI no longer uploads. Worth documenting as "redeploy previous version."
compatibility_date(2022-08-28) is stale but not a runtime risk here (Static Assets is deploy-time/wrangler, not gated on it). Bump for hygiene + retest.undefined as any(dataOperations.ts:139-140) and thepath-browserify+node:pathmix (build-data.ts:4,15) both mirror existingd1Operations.ts/upload.tsconventions — not new smells.- v1/v2 result ordering shifts to sorted-filename order; only observable when multiple configs match one device (rare); v3/v4 sort explicitly. Not a regression.
- Per-isolate shard cache is unbounded (no eviction) — fine at today's 344 KB / 16 shards; revisit if the catalog grows an order of magnitude.
Coverage: all 8 dimensions run (correctness, security, DRY, design, performance, operability, tests, readability); all 10 files covered. .gitignore, package.json (no new runtime deps), worker.ts reviewed directly — clean.
- Fail build:data on implausibly small datasets instead of shipping an empty catalog - Extract shared config discovery/hash/shard logic to dataBuild.ts, reuse it in check_configs.ts, and cover it with unit tests - Throw on non-404 asset fetch errors and log all failure paths, including manifest-listed shards that are missing - Guard against malformed manifests without a shards array - Prefetch distinct manufacturer shards in parallel for v4 batches - Restore edge caching for v4, keyed on data version + hash of the normalized (deduplicated, sorted) device list - Keep the file key order of the D1 path for stable response bodies - Reword "Database empty" to "Firmware update data not available" - Test failure recovery (no memoized failures), malformed JSON, and concurrent shard fetch dedup Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…view) Build a manufacturerId->shard map from the parallel prefetch and index into it in the matching loop, instead of calling getShard again and relying on its internal memoization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deploys were slow and flaky: the post-deploy upload replicated the firmware definitions into D1 through 10 HTTP requests and ~2,500 sequential statements, and a single 30 s D1 stall failed the job with no retry.
The data now ships with the worker as Workers Static Assets instead:
yarn build:datacompiles the config files into per-manufacturer JSON shards plus a manifest, deployed atomically withwrangler deploy. The separate upload step is gone from CI.src/lib/dataOperations.tsreplaces the D1 lookups: shards are parsed once per isolate and lookups run in memory. Edge caching for API responses is unchanged (v4 now gets its own edge cache keyed on data version + normalized device list). The version hash computation is the same as before but now runs over a deterministically sorted file list, so the version value changes once at cutover — existing edge cache entries are orphaned and simply expire.Verified by diffing local responses against production: 300+ request pairs across API v1–v4 (all manufacturers, regions,
$ifconditions, unknown devices) returned identical responses (parsed-JSON comparison).The D1 code and database stay in place for rollback; removal is a follow-up PR.
🤖 Generated with Claude Code