Skip to content

Serve unhashed static files no-store, not no-cache - #390

Merged
Babissimo merged 1 commit into
mainfrom
fix/unhashed-static-no-store
Sep 15, 2026
Merged

Babissimo merged 1 commit into
mainfrom
fix/unhashed-static-no-store

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

Why

#388 switched every static file outside /assets/ to Cache-Control: no-cache, expecting the edge to cache-and-revalidate and the browser to revalidate with a 304. The edge half holds. The browser half does not: the zone's Browser Cache TTL is four hours, and with Origin Cache Control on (the default on this plan) Cloudflare stamps that on any cacheable response whose own max-age is lower. The moment #388 deployed, staging answered cache-control: max-age=14400 for theme-boot.js and app.css. A browser would sit on a stale copy for four hours after a deploy, and the two smoke probes asserting no-cache failed in run 34972146296, which skipped the production deploy. The pipeline is red until this lands.

What

no-store, no-cache for the unhashed files, the policy index.html already has: this zone passes it through untouched (cf-cache-status: DYNAMIC, header intact, verified on staging today). Neither the edge nor the browser keeps a copy, so a deploy shows through at once. The smoke probes and the rendered-config test assert no-store; the spa.conf comment records why no-cache alone is not enough, as a standing constraint.

The price is a browser re-fetch on every visit rather than a 304, about 450 KB for the data explorer including its vendored libraries, on a low-traffic public page. The hashed /assets/ policy is unchanged.

Verified

  • pytest tests/test_nginx_static_cache.py tests/test_nginx_rewrite_ordering.py tests/test_towers_vhost_coverage.py: 10 passed.
  • Rendered the template (TLS off) and ran nginx -t on it in nginx:alpine: syntax ok; the rendered config carries the no-store, no-cache directive twelve times, once for index.html and once for the catch-all in each of the six SPA vhosts.
  • pre-commit run --all-files clean; bash -n and shellcheck (warning level) clean.
  • Zone settings read through the Cloudflare API: browser_cache_ttl = 14400, plan Free.
  • The staging chain never runs on a PR, so the probes first pass for real on the merge run. Active nodes > 0 also failed in run 34972146296; that is the known staging fleet reconnect race, unrelated.

After this deploys, the edge still holds copies stored under the old headers until they expire; those get purged by hand once, and the smoke probes bypass them with a never-seen query string.

ClickUp Edge keeps unhashed static files for a week, so a deploy of the data explorer is invisible until purged.

🤖 Generated with Claude Code

#388 switched every static file outside /assets/ to `no-cache`, on the
reading that Cloudflare would cache and revalidate at the edge while the
browser revalidated with a 304. Half of that is what happens. The edge
does revalidate, but the zone's Browser Cache TTL is four hours, and
with Origin Cache Control on Cloudflare stamps that on any cacheable
response whose own max-age is lower: staging answered
`cache-control: max-age=14400` for theme-boot.js and app.css the moment
#388 deployed. A browser would sit on a stale copy for four hours after
a deploy, and the two smoke probes that assert `no-cache` failed, which
skipped the production deploy.

`no-store, no-cache` is the form this zone passes through untouched;
index.html has been served that way throughout and arrives intact as
DYNAMIC. So the unhashed files take the same policy as the page that
loads them. The price is that a browser re-fetches them on every visit
rather than revalidating, about 450 KB for the data explorer including
its vendored libraries, on a low-traffic public page. The smoke probes
and the rendered-config test now assert `no-store`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 1m 31s —— View job


Code Review

Summary: This is a small, well-scoped fix. #388 moved unhashed static files to Cache-Control: no-cache, but on this zone (Origin Cache Control on, Browser Cache TTL 4h, Free plan) Cloudflare stamps its own TTL onto any response whose own max-age is lower/absent, so no-cache alone didn't stop the browser from caching. Switching to no-store, no-cache — the same policy index.html already uses — fixes that, and the four changed files (nginx config, the rendered-config test, the staging smoke probes, and the data-explorer README) are all consistent with each other. No leftover no-cache-only assertions remain anywhere in the repo (checked backend/tests/, deploy/, docs/, frontend/e2e/).

Specific checks:

  • deploy/nginx/snippets/spa.conf: the new catch-all location matches index.html's existing "no-store, no-cache" header exactly — good consistency, and the comment correctly explains why no-cache alone isn't sufficient on this zone (a non-obvious operational constraint worth recording).
  • backend/tests/test_nginx_static_cache.py: assertion updated to check for "no-store" (substring match against the rendered no-store, no-cache value), consistent with the config change.
  • deploy/staging-smoke-test.sh: probe checks and their descriptive names ("...is not cached") were both updated together; check_header_value does a substring match (grep -qF), so "no-store" correctly matches the full no-store, no-cache header value.
  • data-explorer/README.md: docs updated to match the new behavior (no more mention of 304 revalidation for these files).

Minor nit (non-blocking): backend/tests/test_nginx_static_cache.py:36 — the test function is still named test_every_other_static_file_is_revalidated, but the behavior it now asserts (no-store) is the opposite of revalidation — the file is never cached/stored at all, browser or edge, rather than cached-and-revalidated. Worth a rename (e.g. test_every_other_static_file_is_never_cached) in a follow-up so the test name doesn't mislead future readers, though the docstring above it does already explain the corrected behavior.

No security, correctness, or performance concerns. The tradeoff called out in the PR description (full re-fetch instead of a 304 on every visit, ~450KB for the data explorer) is reasonable for a low-traffic public page and is the right fix for the stated bug (stale browser cache for up to 4h after a deploy, and the two failing smoke probes in run 34972146296).

I was unable to execute pytest/nginx -t myself in this sandboxed environment (Bash tool calls requiring elevated approval aren't available here), so I relied on static analysis of the diff and cross-referencing the rendered config logic in tests/nginx_helpers.py. The PR description's own verification (10 passed, nginx -t clean, Cloudflare zone settings confirmed) looks credible and consistent with the code.

@Babissimo
Babissimo merged commit 9bcff8d into main Sep 15, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant