ReviewOS

also looking at this

stacks/stx

fix(images): name delivery variants independently of the output path

#1961
Merged chris wants to merge fix/stable-image-delivery-names into main
2 files +102 -2
8 days ago

The bug

ts-images folds storage.cacheNamespace into the content hash that names every delivery variant, and its local adapter builds that namespace out of the absolute output directory:

cacheNamespace: `local:${outDir}:${baseUrl}`

For a checkout that lives in one place forever this is invisible. For anything deploying atomic releases it is not — the same tree is served from a new absolute path every deploy, so identical bytes with identical encode options land under a different filename each time:

releases/<sha-1>/…/_stx/images  ->  hero-6f21e0aa-44338d78039b57e4-640.webp
releases/<sha-2>/…/_stx/images  ->  hero-6f21e0aa-301f828bdc402f4a-640.webp

The readable slug and the 8-char path hash (both from stx's own catalogName) are stable. The 16-char delivery hash is not.

What it costs

Every stat() misses, the previous release's output is never read back, and each deploy re-encodes the entire public directory while the old generation stays on disk.

Measured on a site with 201 source images:

Cold pass, first response~211s
Warm pass, same path~20s, 0 variants regenerated
Delivery dir over four deploys1,984 → 2,596 → 5,171 → 5,917 files

The server binds its port about a second after start and then answers nothing until the pass completes, so a liveness probe with any sane patience restarts it mid-pass and the work begins again. That directory never converges, and nothing in it is ever read twice.

The fix

stx passes its own local storage adapter. Its namespace describes the URL space a variant is published into rather than the directory this particular release happens to write it to, so two builds of the same bytes agree on the filename — which is what made the on-disk cache reusable in the first place.

Two smaller things came along with owning the adapter:

  • Writes go through a temp file and a rename. The delivery directory is commonly shared between a running server and a build; a reader arriving mid-write now sees the old file or the new one, never a truncated one.
  • A zero-byte file is treated as absent. That is what an interrupted encode leaves behind, and the old path would serve it as a permanently broken image.

Tests

Two added to packages/stx/test/builtins/image-delivery.test.ts:

  • variants are named the same from any output directory, and the fingerprints match — this one fails without the adapter (verified by reverting the single line and re-running)
  • a second pass over the same directory reuses what is on disk, asserted on untouched mtimes rather than just names
6 pass, 0 fail

No new typecheck errors (the two in that file pre-date this change).

Note on where the fix lives

The root cause is arguably localStorage in ts-images/delivery — an absolute path in a cache namespace is surprising for any consumer, not just stx. Fixing it there would help everyone and need no change here. This PR fixes it in stx because stx can do it today without a dependency release, and because the namespace it wants is genuinely more specific than the generic adapter can guess.

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.