diff --git a/src/platform/adapters/fs/veryfront/adapter.ts b/src/platform/adapters/fs/veryfront/adapter.ts index d1dd535300..76a1f515c2 100644 --- a/src/platform/adapters/fs/veryfront/adapter.ts +++ b/src/platform/adapters/fs/veryfront/adapter.ts @@ -130,7 +130,11 @@ export class VeryfrontFSAdapter implements FSAdapter { /** Resolves when file list initialization is complete (for coordinating reads) */ private fileListReadyResolve: (() => void) | null = null; /** Single-flight background rewarm when the file list cache disappears */ - private fileListWarmupPromise: Promise | null = null; + // Resolves with the files it fetched, so a caller that waited does not have + // to depend on the cache write having succeeded -- writes are skipped + // entirely when caching is disabled, and can fail on a backend cache. + private fileListWarmupPromise: Promise | null> | null = + null; private fileListWarmupKey: string | null = null; /** Single-flight foreground refresh when a branch preview read misses a newly pushed file. */ private branchMissRecoveryPromise: Promise | null = null; @@ -663,7 +667,7 @@ export class VeryfrontFSAdapter implements FSAdapter { } const warmupContext = this.contentContext; - let warmupPromise: Promise | null = null; + let warmupPromise: Promise | null> | null = null; warmupPromise = (async () => { try { const existing = await this.cache.getAsync>( @@ -676,7 +680,7 @@ export class VeryfrontFSAdapter implements FSAdapter { cacheKey: effectiveCacheKey, fileCount: existing.length, }); - return; + return existing; } logger.debug("Starting file list warmup", { @@ -704,12 +708,16 @@ export class VeryfrontFSAdapter implements FSAdapter { totalFiles: files.length, filesWithContent: files.filter((file) => file.content).length, }); + + return files; } catch (error) { logger.warn("File list warmup failed", { reason, cacheKey: effectiveCacheKey, error: error instanceof Error ? error.message : String(error), }); + + return null; } finally { if (warmupPromise && this.fileListWarmupPromise === warmupPromise) { this.fileListWarmupPromise = null; @@ -720,7 +728,8 @@ export class VeryfrontFSAdapter implements FSAdapter { this.fileListWarmupPromise = warmupPromise; this.fileListWarmupKey = effectiveCacheKey; - this.readOps.setFileListReadyPromise(warmupPromise); + // That collaborator only needs completion, not the payload. + this.readOps.setFileListReadyPromise(warmupPromise.then(() => {})); } private markSourceSnapshotChanged( @@ -1040,7 +1049,16 @@ export class VeryfrontFSAdapter implements FSAdapter { return this.projectData; } - async getAllSourceFiles(): Promise> { + /** + * @param options.waitForWarmup wait for an in-flight file-list fetch instead + * of answering empty. Off by default: most callers can proceed without the + * list and must not pay for the fetch, but a caller that has no other way to + * obtain it -- CSP derivation on a release-backed context, where nothing else + * populates the cache -- would otherwise read empty on every request forever. + */ + async getAllSourceFiles( + options: { waitForWarmup?: boolean } = {}, + ): Promise> { if (!this.contentContext) { logger.debug("getAllSourceFiles called without contentContext", { initialized: this.initialized, @@ -1055,7 +1073,22 @@ export class VeryfrontFSAdapter implements FSAdapter { "getAllSourceFiles miss", ); const cacheKey = cached?.cacheKey; - const files = cached?.files; + let files = cached?.files; + + // A miss schedules a warmup and returns immediately, which is right for + // callers that can proceed without the list. This one cannot: nothing else + // populates it for a release-backed context, so returning early meant the + // list was empty on every request for the life of the process. Wait for the + // fetch this read just started, then look again. + if (options.waitForWarmup && cacheKey && !files?.length && this.fileListWarmupPromise) { + // Take what the fetch returned rather than re-reading the cache: with + // caching disabled, or a failed backend write, the cache keeps nothing + // and correctness would depend on a write that never happened. + const fetched = await this.fileListWarmupPromise; + files = fetched?.length + ? fetched + : await this.cache.getAsync<{ path: string; content?: string }[]>(cacheKey); + } if (!cacheKey || !files?.length) { logger.debug("getAllSourceFiles cache miss or empty", { diff --git a/src/platform/adapters/fs/veryfront/multi-project-adapter.ts b/src/platform/adapters/fs/veryfront/multi-project-adapter.ts index b93c93fdf8..61e285347b 100644 --- a/src/platform/adapters/fs/veryfront/multi-project-adapter.ts +++ b/src/platform/adapters/fs/veryfront/multi-project-adapter.ts @@ -316,10 +316,12 @@ export class MultiProjectFSAdapter implements FSAdapter { } } - async getAllSourceFiles(): Promise> { + async getAllSourceFiles( + options: { waitForWarmup?: boolean } = {}, + ): Promise> { try { const adapter = await this.getAdapter(); - const files = (await adapter.getAllSourceFiles?.()) ?? []; + const files = (await adapter.getAllSourceFiles?.(options)) ?? []; if (files.length === 0) { logger.debug("getAllSourceFiles returned empty", { diff --git a/src/server/runtime-handler/derive-project-csp.test.ts b/src/server/runtime-handler/derive-project-csp.test.ts index 8adf5f282a..c12fae5c06 100644 --- a/src/server/runtime-handler/derive-project-csp.test.ts +++ b/src/server/runtime-handler/derive-project-csp.test.ts @@ -35,7 +35,8 @@ function createHostedAdapter(options: { requireInitialization: boolean }) { return Promise.resolve(SOURCE); }, getContentContext: () => null, - getSourceSnapshotVersion: () => 7, + // Async, like MultiProjectFSAdapter's. + getSourceSnapshotVersion: () => Promise.resolve(7), }; const fs = { @@ -93,6 +94,49 @@ describe("server/runtime-handler/deriveProjectCspOrigins", () => { assertEquals(derived?.["img-src"], ["https://images.unsplash.com"]); }); + it("re-derives when the snapshot moves under a fixed release", async () => { + // The wrapper's `getSourceSnapshotVersion` is async, and template-stringifying + // it wrote the literal "[object Promise]" into every key. Two releases would + // still differ by their id prefix, so only a moving snapshot under one fixed + // identity can see this: with the promise stringified, both calls share a key + // and the second is served from cache instead of re-reading the source. + __clearDerivedCspCacheForTests(); + + let snapshot = 1; + let reads = 0; + let source = SOURCE; + const underlying = { + ensureSourceSnapshotFresh: () => Promise.resolve(), + getAllSourceFiles: () => { + reads += 1; + return Promise.resolve(source); + }, + getContentContext: () => null, + getSourceSnapshotVersion: () => Promise.resolve(snapshot), + }; + const adapter = { + fs: { + isVeryfrontAdapter: true, + isMultiProjectMode: true, + getUnderlyingAdapter: () => underlying, + ensureSourceSnapshotFresh: () => Promise.resolve(), + runWithContext: (_s: string, _t: string, run: () => Promise) => run(), + }, + } as unknown as RuntimeAdapter; + + const first = await deriveProjectCspOrigins({ ...PRODUCTION, adapter }); + assertEquals(first?.["img-src"], ["https://images.unsplash.com"]); + assertEquals(reads, 1); + + // Same release, new content pushed under it. + snapshot = 2; + source = [{ path: "pages/index.tsx", content: '' }]; + + const second = await deriveProjectCspOrigins({ ...PRODUCTION, adapter }); + assertEquals(reads, 2, "a moved snapshot must not be served from the previous key"); + assertEquals(second?.["img-src"], ["https://cdn.example.com"]); + }); + it("returns nothing rather than throwing when the adapter cannot host a tenant", async () => { __clearDerivedCspCacheForTests(); const derived = await deriveProjectCspOrigins({ diff --git a/src/server/runtime-handler/project-runtime-context.ts b/src/server/runtime-handler/project-runtime-context.ts index d63460a056..0c160dee0c 100644 --- a/src/server/runtime-handler/project-runtime-context.ts +++ b/src/server/runtime-handler/project-runtime-context.ts @@ -595,9 +595,11 @@ export async function deriveProjectCspOrigins(args: { const underlying = typeof fs.getUnderlyingAdapter === "function" ? fs.getUnderlyingAdapter() as { - getAllSourceFiles?: () => Promise>; + getAllSourceFiles?: ( + options?: { waitForWarmup?: boolean }, + ) => Promise>; getContentContext?: () => ResolvedContentContext | null; - getSourceSnapshotVersion?: () => number; + getSourceSnapshotVersion?: () => number | Promise; ensureSourceSnapshotFresh?: (reason?: string) => Promise; } : undefined; @@ -617,8 +619,11 @@ export async function deriveProjectCspOrigins(args: { // under them changes, so the adapter's snapshot generation is what actually // moves when a preview is pushed to. Without it a preview would serve a // derivation from before the push until the entry is evicted. + // Awaited: the multi-project wrapper's version of this is async, and + // template-stringifying the promise put the literal "[object Promise]" in + // every key, collapsing all snapshots to one value. const snapshot = typeof underlying.getSourceSnapshotVersion === "function" - ? underlying.getSourceSnapshotVersion() + ? await underlying.getSourceSnapshotVersion() : 0; const contentVersion = `${ resolveStyleContentVersion(underlying.getContentContext?.() ?? null, { @@ -631,7 +636,9 @@ export async function deriveProjectCspOrigins(args: { return await getDerivedCspOrigins({ projectScope: args.projectSlug, contentVersion, - loadSourceFiles: () => underlying.getAllSourceFiles!(), + // Nothing else populates the file list for a release-backed context, so + // this read must wait for the fetch rather than answer empty forever. + loadSourceFiles: () => underlying.getAllSourceFiles!({ waitForWarmup: true }), }); };