Skip to content

🐛 Fixed post card assets going missing until restart (minimal disk recovery)#29129

Draft
sagzy wants to merge 1 commit into
mainfrom
fix-card-assets-recovery-minimal
Draft

🐛 Fixed post card assets going missing until restart (minimal disk recovery)#29129
sagzy wants to merge 1 commit into
mainfrom
fix-card-assets-recovery-minimal

Conversation

@sagzy

@sagzy sagzy commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

ref https://linear.app/ghost/issue/ONC-1865

Note: This is one of two alternative fixes for the same incident, opened for comparative review. The other is the in-memory serving approach: #29128. Both supersede #29056, whose review surfaced races in the recovery-on-ENOENT design (torn reads from non-atomic writes, response-cache poisoning, throttle 404ing concurrent requests, rebuild loops for never-produced files).

Problem

/public/cards.min.css and /public/cards.min.js are minified at runtime into content/public/ and served by reading them back from disk. If the built files go missing at runtime, or the content folder is unwritable (EACCES), every request 404s until the process restarts — ~486 production sites accumulated in this state during Jun 25–29.

Solution: keep disk serving, fix the root causes

This keeps the existing serve-from-disk architecture and makes the smallest set of changes that fix the failure modes at the layer that owns them:

  • Atomic writes — the minifier writes to a temp file in the same directory and renames it into place, so a concurrent reader sees either the old or the new complete file, never a torn one.
  • All rebuild policy in one place — new AssetsMinificationBase.ensureLoaded(): concurrent callers join a single in-flight build; a 10s retry backoff (also covering builds that resolve without producing assets, e.g. swallowed EACCES) stops a persistently failing build from minifying per request; invalidate() no longer forgets an in-flight build — a generation counter re-runs load() with the new config instead of allowing two concurrent minifier runs.
  • Dumb recovery in the file middleware — on ENOENT for a built file it calls the service's rebuild hook once and retries the read once. No throttle at this layer: a request arriving during an in-flight rebuild joins it instead of 404ing.
  • hasFile() gate — files the current build legitimately never produces (theme card config yielding no output) 404 fast instead of triggering a rebuild per request.
  • No permanent memory cache for built files — runtime-mutable files are re-read from disk per request (page-cache-warm, small), so a bad read can never be pinned until restart; ETag/Cache-Control still enable client/CDN caching.
  • serveMiddleware() no longer lets a rejected load() escape as an unhandled rejection (which left requests hanging under Express 4) — this also fixes the hang for AdminAuthAssets.

Testing

  • New unit tests for minifier atomicity (destination preserved on failed rename, temp cleanup, error rethrow), ensureLoaded() (join-in-flight, backoff after failure and after resolved-but-not-ready builds, invalidate resets backoff, mid-build invalidate re-runs load with no concurrency), and the recovery path (rebuild→200, still-missing→404 with exactly one rebuild, rebuild rejection logged→404, static files never rebuild, built files not memory-cached while static still are).
  • ghost/core unit suite: 7185 passed; the 3 failures are pre-existing on main (verified by baseline run).

🤖 Generated with Claude Code

ref https://linear.app/ghost/issue/ONC-1865

- cards.min.css/js are generated into content/public at runtime; when the
  files went missing or the folder was unwritable, every request 404d
  until reboot because nothing ever retried the build
- a missing generated file now triggers one shared rebuild and a single
  retried read; concurrent requests join the in-flight build instead of
  404ing
- build policy (single-flight, 10s retry backoff, rebuild-on-theme-change)
  lives on the assets service so every entry point shares one throttle
- the minifier now writes atomically (temp file + rename) so a request can
  never read a half-written asset, and built files are no longer pinned in
  the middleware memory cache where a bad read would stick until restart

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9861256d-1088-4b65-98f9-102cdc98102d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-card-assets-recovery-minimal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 2701a79

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 45s View ↗
nx run ghost:test:integration ✅ Succeeded 2m 47s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 56s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 22s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗
nx run-many -t lint -p ghost ✅ Succeeded 35s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 29s View ↗
nx run @tryghost/admin:build ✅ Succeeded 5s View ↗
Additional runs (2) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-06 09:49:10 UTC

// whose card config legitimately yields no output must 404 fast instead
// of triggering a rebuild per request.
const rebuildCardAssets = type => () => (cardAssets.hasFile(type) ? cardAssets.ensureLoaded() : Promise.resolve());
siteApp.get('/public/cards.min.css', cardAssets.serveMiddleware(), createPublicFileMiddleware('built', 'public/cards.min.css', 'text/css', config.get('caching:publicAssets:maxAge'), {rebuild: rebuildCardAssets('css')}));
// of triggering a rebuild per request.
const rebuildCardAssets = type => () => (cardAssets.hasFile(type) ? cardAssets.ensureLoaded() : Promise.resolve());
siteApp.get('/public/cards.min.css', cardAssets.serveMiddleware(), createPublicFileMiddleware('built', 'public/cards.min.css', 'text/css', config.get('caching:publicAssets:maxAge'), {rebuild: rebuildCardAssets('css')}));
siteApp.get('/public/cards.min.js', cardAssets.serveMiddleware(), createPublicFileMiddleware('built', 'public/cards.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'), {rebuild: rebuildCardAssets('js')}));
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.

2 participants