From b31314b6dc699f20472a485fff8a5f19eb454a06 Mon Sep 17 00:00:00 2001 From: Contentrain Date: Wed, 29 Jul 2026 19:51:44 +0300 Subject: [PATCH] fix(cdn): publish build artifacts in an order a reader can trust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ordering bugs in executeCDNBuild left windows where the CDN advertised something it had not published. `_manifest.json` went up FIRST. It is the content version pointer — consumers key freshness off its commitSha — so for the entire rest of the build it named a commit whose content, bundles and media manifest were still uploading. Objects go up one at a time: measured on staging, a full rebuild wrote the manifest at 11:01:19 and the last bundle at 11:03:04, a 105-second window (~24s on a selective build). A consumer reading in there pinned the new commitSha to pre-build bodies and, if it caches per commit, never re-read them. This is the invariant #153 protected — the manifest must not outrun the bundle — but that fix covered a build that SKIPPED the bundle, not every build that simply had not written it yet. The manifest now goes last, after every artifact it describes, so a build that dies midway leaves the old manifest pointing at the old, complete content instead of a half-written snapshot. `_media_manifest.json` was written AFTER the stale-object sweep, so it was never in uploadedPaths when the sweep ran: every full rebuild deleted it and re-uploaded it a moment later. The delivery SDK throws on any non-2xx and caches the media manifest for the lifetime of the instance, so a consumer booting inside that window stays broken until it is replaced. It now runs before the sweep, which also keeps the inverse honest — a project with no media assets uploads nothing, the path stays out of uploadedPaths, and the sweep still collects a stale manifest. Both are covered by tests that fail against the previous ordering. --- server/utils/cdn-builder.ts | 166 +++++++++++++++++++-------------- tests/unit/cdn-builder.test.ts | 81 ++++++++++++++++ 2 files changed, 175 insertions(+), 72 deletions(-) diff --git a/server/utils/cdn-builder.ts b/server/utils/cdn-builder.ts index 95b4c33..ebc6480 100644 --- a/server/utils/cdn-builder.ts +++ b/server/utils/cdn-builder.ts @@ -232,13 +232,13 @@ export async function executeCDNBuild(options: BuildOptions): Promise m.id)) // A selective build whose diff touches no content models (a pure code - // push: only app/, server/, etc.) must be a true no-op. Uploading the - // manifest here would advance `_manifest.json.commitSha` to the code - // commit while the bundle block below is skipped (targetModels empty) — - // leaving `_manifest.json.commitSha` ahead of every `_bundle/*.json`. - // Consumers that key content freshness off the manifest then read stale/ - // empty content until a full rebuild re-aligns them. The manifest tracks - // the CONTENT version, so a content-less push must not bump it. + // push: only app/, server/, etc.) must be a true no-op. Running the rest + // would advance `_manifest.json.commitSha` to the code commit while the + // bundle block is skipped (targetModels empty), leaving the manifest ahead + // of every `_bundle/*.json`. Consumers that key content freshness off the + // manifest then read stale/empty content until a full rebuild re-aligns + // them (#153). The manifest tracks the CONTENT version, so a content-less + // push must not bump it — returning here also saves a wasted build cycle. // fullRebuild (manual trigger) and config/model-def changes never reach // here: the former skips the `else` branch above, the latter make // getAffectedModels non-empty. @@ -255,33 +255,7 @@ export async function executeCDNBuild(options: BuildOptions): Promise ({ - id: m.id, - name: m.name, - kind: m.kind, - domain: m.domain, - i18n: m.i18n, - })), - } - const manifestData = JSON.stringify(manifest, null, 2) - await cdn.putObject(projectId, '_manifest.json', manifestData, 'application/json') - uploadedPaths.add('_manifest.json') - filesUploaded++ - totalSizeBytes += Buffer.byteLength(manifestData) - - // 5. Upload model index + definitions + // 4. Upload model index + definitions const modelSummaries = models.map(m => ({ id: m.id, name: m.name, @@ -305,7 +279,7 @@ export async function executeCDNBuild(options: BuildOptions): Promise 0) { + // Build media manifest + const mediaManifest: Record, meta: Record }> = {} + for (const asset of mediaAssets) { + mediaManifest[asset.originalPath] = { + original: asset.originalPath, + variants: Object.fromEntries(Object.entries(asset.variants).map(([k, v]) => [k, v.path])), + meta: { + width: asset.width, + height: asset.height, + format: asset.format, + size: asset.size, + blurhash: asset.blurhash, + alt: asset.alt, + }, + } + } + const mediaManifestData = JSON.stringify({ version: '1', assets: mediaManifest }, null, 2) + await cdn.putObject(projectId, '_media_manifest.json', mediaManifestData, 'application/json') + uploadedPaths.add('_media_manifest.json') + filesUploaded++ + totalSizeBytes += Buffer.byteLength(mediaManifestData) + } + } + } + catch { + // Media manifest generation is non-fatal + } + + // 8. Manifest — published LAST, after every artifact it describes. + // + // `_manifest.json` is the CONTENT VERSION POINTER: consumers key freshness + // off its commitSha. Publishing it first (as this used to) advertised a + // commit whose content, bundles and media manifest were still uploading — + // measured at ~105s on a full rebuild, since every object goes up one at a + // time. A consumer reading in that window pinned the new commitSha to + // pre-build bodies and, if it caches per commit, never re-read them. Same + // invariant #153 protected (manifest must not outrun the bundle); that fix + // covered a build that skipped the bundle, this covers every build that + // simply hadn't written it yet. A build that dies midway now leaves the old + // manifest pointing at the old, complete content instead of a half-written + // snapshot. + progress({ phase: 'upload', message: 'Uploading manifest...', current: targetModels.length, total: targetModels.length }) + const manifest = { + version: '1', + commitSha, + builtAt: new Date().toISOString(), + branch, + config: { + stack: config.stack, + locales: config.locales, + domains: config.domains, + }, + models: models.map(m => ({ + id: m.id, + name: m.name, + kind: m.kind, + domain: m.domain, + i18n: m.i18n, + })), + } + const manifestData = JSON.stringify(manifest, null, 2) + await cdn.putObject(projectId, '_manifest.json', manifestData, 'application/json') + uploadedPaths.add('_manifest.json') + filesUploaded++ + totalSizeBytes += Buffer.byteLength(manifestData) + + // 9. Diff-based stale object cleanup progress({ phase: 'cleanup', message: 'Cleaning stale objects...' }) try { if (options.fullRebuild || !options.changedPaths?.length) { @@ -520,41 +576,7 @@ export async function executeCDNBuild(options: BuildOptions): Promise 0) { - // Build media manifest - const mediaManifest: Record, meta: Record }> = {} - for (const asset of mediaAssets) { - mediaManifest[asset.originalPath] = { - original: asset.originalPath, - variants: Object.fromEntries(Object.entries(asset.variants).map(([k, v]) => [k, v.path])), - meta: { - width: asset.width, - height: asset.height, - format: asset.format, - size: asset.size, - blurhash: asset.blurhash, - alt: asset.alt, - }, - } - } - const manifestData = JSON.stringify({ version: '1', assets: mediaManifest }, null, 2) - await cdn.putObject(projectId, '_media_manifest.json', manifestData, 'application/json') - uploadedPaths.add('_media_manifest.json') - filesUploaded++ - totalSizeBytes += Buffer.byteLength(manifestData) - } - } - } - catch { - // Media manifest generation is non-fatal - } - - // 9. Purge edge cache + // 10. Purge edge cache progress({ phase: 'done', message: `Build complete — ${filesUploaded} uploaded, ${filesDeleted} deleted`, current: targetModels.length, total: targetModels.length }) await cdn.purgeCache(projectId) diff --git a/tests/unit/cdn-builder.test.ts b/tests/unit/cdn-builder.test.ts index 8002310..232265e 100644 --- a/tests/unit/cdn-builder.test.ts +++ b/tests/unit/cdn-builder.test.ts @@ -300,6 +300,87 @@ describe('cdn builder', () => { expect(result.filesDeleted).toBe(2) }) + // `_manifest.json` is the content version pointer — consumers key freshness + // off its commitSha. It used to go up FIRST, so for the whole upload (~105s on + // a measured full rebuild, one object at a time) it advertised a commit whose + // content and bundles were still in flight. A consumer reading in that window + // pinned the new commitSha to pre-build bodies. + it('publishes _manifest.json after every artifact it describes', async () => { + const { git, provider } = seedProject('order-proj') + + const result = await executeCDNBuild({ + projectId: 'order-proj', + buildId: 'b', + git, + cdn: provider, + contentRoot: '', + commitSha: 's', + branch: 'main', + fullRebuild: true, + }) + + expect(result.error).toBeUndefined() + const written = vi.mocked(provider.putObject).mock.calls.map(c => c[1] as string) + expect(written).toContain('_manifest.json') + // Nothing the manifest points at may be written after it. + expect(written.at(-1)).toBe('_manifest.json') + expect(written.indexOf('_manifest.json')).toBeGreaterThan(written.indexOf('content/faq/en.json')) + expect(written.indexOf('_manifest.json')).toBeGreaterThan(written.indexOf('_bundle/en.json')) + }) + + // The sweep deletes every build-owned object outside uploadedPaths. The media + // manifest used to be written AFTER it, so each full rebuild deleted it and + // re-uploaded it a moment later — a window where the path 404s. The delivery + // SDK throws on non-2xx and caches the media manifest for the life of the + // instance, so a consumer booting inside that window stays broken. + it('never deletes _media_manifest.json during a full rebuild that has media', async () => { + const { git, provider, objects } = seedProject('media-proj') + objects.set('media-proj:_media_manifest.json', '{"version":"1","assets":{}}') + vi.stubGlobal('useMediaProvider', () => ({ + listAssets: async () => ({ + assets: [{ originalPath: 'media/original/keep.webp', variants: {}, width: 1, height: 1, format: 'webp', size: 3, blurhash: null, alt: null }], + }), + })) + + const result = await executeCDNBuild({ + projectId: 'media-proj', + buildId: 'b', + git, + cdn: provider, + contentRoot: '', + commitSha: 's', + branch: 'main', + fullRebuild: true, + }) + + expect(result.error).toBeUndefined() + const deleted = vi.mocked(provider.deleteObject).mock.calls.map(c => c[1] as string) + expect(deleted).not.toContain('_media_manifest.json') + expect(objects.has('media-proj:_media_manifest.json')).toBe(true) + }) + + // The inverse still has to hold: with no media assets nothing is uploaded, so + // a leftover manifest is genuinely stale and the sweep must collect it. + it('sweeps a stale _media_manifest.json when the project has no media assets', async () => { + const { git, provider, objects } = seedProject('nomedia-proj') + objects.set('nomedia-proj:_media_manifest.json', '{"version":"1","assets":{}}') + vi.stubGlobal('useMediaProvider', () => ({ listAssets: async () => ({ assets: [] }) })) + + const result = await executeCDNBuild({ + projectId: 'nomedia-proj', + buildId: 'b', + git, + cdn: provider, + contentRoot: '', + commitSha: 's', + branch: 'main', + fullRebuild: true, + }) + + expect(result.error).toBeUndefined() + expect(objects.has('nomedia-proj:_media_manifest.json')).toBe(false) + }) + it('preserves media/* when a build runs with empty changedPaths (webhook empty-commits path)', async () => { const { git, provider, objects } = seedProject('proj2')