From 4cc5e9ad27ad85b65c23f84d00a2b13417adbb98 Mon Sep 17 00:00:00 2001 From: fforres Date: Sun, 18 Oct 2026 01:05:03 -0700 Subject: [PATCH 1/5] perf(host-cloudflare): open a current D1 database with one read Fold the compatibility version and the ordered data-migration ids into the schema fingerprint and skip the compatibility check and migration ledger read when the stored fingerprint matches. The marker is still written only after DDL, compatibility and data migrations succeed. Claude-Session: https://claude.ai/code/session_019ZE3j9YzRKxT4LaiDCBv6S --- apps/host-cloudflare/src/db/d1.test.ts | 49 ++++++++++++++++++- apps/host-cloudflare/src/db/d1.ts | 26 ++++++---- .../host-cloudflare/src/db/data-migrations.ts | 10 ++++ 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/apps/host-cloudflare/src/db/d1.test.ts b/apps/host-cloudflare/src/db/d1.test.ts index 83f9788090..9d1d860ff7 100644 --- a/apps/host-cloudflare/src/db/d1.test.ts +++ b/apps/host-cloudflare/src/db/d1.test.ts @@ -5,6 +5,7 @@ import { collectTables, type SqliteDataMigrationClient } from "@executor-js/sdk" import { createSqliteTestFumaDb } from "@executor-js/sdk/testing"; import { createD1ExecutorDb } from "./d1"; +import { cloudflareDataMigrationIds } from "./data-migrations"; const makeRecordingD1 = ( client: SqliteDataMigrationClient, @@ -79,10 +80,54 @@ describe("createD1ExecutorDb", () => { const secondStatements = statements.slice(secondStart); expect(secondStatements.filter(isRuntimeSchemaStatement)).toEqual([]); - expect(secondStatements.some((sql) => sql.includes("SELECT name FROM data_migration"))).toBe( - true, + + await sqlite.close(); + }); + + it("opens a current database with exactly one D1 query", async () => { + const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); + const { db, statements } = makeRecordingD1(sqlite.client); + const first = await createD1ExecutorDb(db, undefined); + await first.close(); + + const secondStart = statements.length; + const second = await createD1ExecutorDb(db, undefined); + await second.close(); + + const secondStatements = statements.slice(secondStart); + expect(secondStatements).toHaveLength(1); + expect(secondStatements[0]).toContain("SELECT fingerprint"); + + await sqlite.close(); + }); + + it("runs the data migrations again when the stored fingerprint predates the migration list", async () => { + const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); + const { db, statements } = makeRecordingD1(sqlite.client); + const first = await createD1ExecutorDb(db, undefined); + await first.close(); + const ids = cloudflareDataMigrationIds(); + const missing = ids[ids.length - 1]; + await sqlite.client.execute({ + sql: "DELETE FROM data_migration WHERE name = ?", + args: [missing], + }); + await sqlite.client.execute( + `UPDATE private_executor_cloudflare_schema_fingerprint + SET fingerprint = 'older' + WHERE id = 'runtime-schema'`, ); + const secondStart = statements.length; + const second = await createD1ExecutorDb(db, undefined); + await second.close(); + + expect( + statements.slice(secondStart).some((sql) => sql.includes("SELECT name FROM data_migration")), + ).toBe(true); + const ledger = await sqlite.client.execute(`SELECT name FROM data_migration`); + expect(ledger.rows.map((row) => row.name)).toContain(missing); + await sqlite.close(); }); diff --git a/apps/host-cloudflare/src/db/d1.ts b/apps/host-cloudflare/src/db/d1.ts index 5763670987..b25f48b4af 100644 --- a/apps/host-cloudflare/src/db/d1.ts +++ b/apps/host-cloudflare/src/db/d1.ts @@ -14,7 +14,11 @@ import { import { makeR2BlobStore } from "@executor-js/cloudflare/blob-store"; import { CLOUDFLARE_NAMESPACE, CLOUDFLARE_SCHEMA_VERSION } from "../config"; -import { prepareCloudflareD1Data } from "./data-migrations"; +import { + CLOUDFLARE_D1_COMPATIBILITY_VERSION, + cloudflareDataMigrationIds, + prepareCloudflareD1Data, +} from "./data-migrations"; import { withD1TransientRetry } from "./d1-retry"; const SCHEMA_FINGERPRINT_TABLE = `private_${CLOUDFLARE_NAMESPACE}_schema_fingerprint`; @@ -105,19 +109,21 @@ export const createD1ExecutorDb = async ( // exposes one. A generated fingerprint lets every Worker/DO isolate prove the // expected generated schema was already prepared with one primary read instead of // replaying dozens of idempotent CREATE/ALTER statements on every database - // open. The marker is stored only after the ensure, data migrations, and any - // required post-compatibility ensure succeed. Data migrations retain their - // own ledger and still run below on every open. - const expectedFingerprint = await schemaFingerprint( - createDrizzleRuntimeSchemaSqlFromTables(options), - ); + // open. The fingerprint also covers the compatibility-check version and the + // ordered data-migration ids, so a current database opens with that one read + // and skips both. The marker is stored only after the ensure, compatibility + // check, data migrations, and any required post-compatibility ensure succeed. + // Data migrations keep their own ledger for databases that need them. + const expectedFingerprint = await schemaFingerprint([ + ...createDrizzleRuntimeSchemaSqlFromTables(options), + `compatibility:${CLOUDFLARE_D1_COMPATIBILITY_VERSION}`, + `data-migrations:${cloudflareDataMigrationIds().join(",")}`, + ]); const preparedFingerprint = await readPreparedSchemaFingerprint(db); const requiresSchemaPreparation = preparedFingerprint !== expectedFingerprint; if (requiresSchemaPreparation) { await ensureDrizzleRuntimeSchemaFromTables({ run: (query) => drizzleDb.run(query) }, options); - } - const migrationResult = await prepareCloudflareD1Data(db, blobs); - if (requiresSchemaPreparation) { + const migrationResult = await prepareCloudflareD1Data(db, blobs); // Compatibility migrations can rebuild a legacy table. Re-run the // idempotent ensure before stamping so the database matches the generated // schema after migration, including newer nullable columns the legacy diff --git a/apps/host-cloudflare/src/db/data-migrations.ts b/apps/host-cloudflare/src/db/data-migrations.ts index 1efb4340dd..a91a0256ed 100644 --- a/apps/host-cloudflare/src/db/data-migrations.ts +++ b/apps/host-cloudflare/src/db/data-migrations.ts @@ -228,6 +228,10 @@ const copyProviderServiceSplitBlobsToR2 = ( }), }); +// Bump when `ensureCloudflareD1SchemaCompatibility` gains a repair step, so +// databases stamped by the previous logic run it once more. +export const CLOUDFLARE_D1_COMPATIBILITY_VERSION = 1; + const cloudflareDataMigrations = (bucket: R2Bucket | undefined): readonly SqliteDataMigration[] => [ { name: googleOpenApiOwnershipDataMigration.name, @@ -256,6 +260,12 @@ const cloudflareDataMigrations = (bucket: R2Bucket | undefined): readonly Sqlite encryptedSecretsRepartitionDataMigration, ]; +// Ordered ids of every data migration this build knows. The ledger already +// makes a migration run once, and the bucket only decides how it copies blobs, +// so the ids alone describe what a fully prepared database looks like. +export const cloudflareDataMigrationIds = (): readonly string[] => + cloudflareDataMigrations(undefined).map((migration) => migration.name); + export const prepareCloudflareD1Data = ( db: D1Database, bucket: R2Bucket | undefined, From e16593e882660020a9aef6ae47a42d522c798ecf Mon Sep 17 00:00:00 2001 From: fforres Date: Sun, 18 Oct 2026 01:05:35 -0700 Subject: [PATCH 2/5] perf(host-cloudflare): load QuickJS on first code execution instead of every app build The QuickJS executor now loads the Workers WASM-inlined variant right before its first execute, so building the app, the cron reconcile and MCP sessions that never run code skip the load. Claude-Session: https://claude.ai/code/session_019ZE3j9YzRKxT4LaiDCBv6S --- apps/host-cloudflare/src/app.ts | 6 --- apps/host-cloudflare/src/execution.test.ts | 37 +++++++++++++++++++ apps/host-cloudflare/src/execution.ts | 25 ++++++++++++- .../src/mcp/session-durable-object.ts | 4 -- 4 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 apps/host-cloudflare/src/execution.test.ts diff --git a/apps/host-cloudflare/src/app.ts b/apps/host-cloudflare/src/app.ts index af62a386a5..828492e4cb 100644 --- a/apps/host-cloudflare/src/app.ts +++ b/apps/host-cloudflare/src/app.ts @@ -26,7 +26,6 @@ import { ErrorCaptureLive } from "./observability"; import { cloudflareAccountMiddleware } from "./account/account-provider"; import { makeCloudflareApprovalHandler } from "./mcp"; import { makeCloudflareMcpAgentHandler } from "./mcp/agent-handler"; -import { preloadQuickJs } from "./quickjs"; // =========================================================================== // The Cloudflare host, as ONE `ExecutorApp.make` call: the 4th app alongside @@ -67,7 +66,6 @@ export const reconcileInternalIntegrationsOnce = (env: CloudflareEnv): Promise preloadQuickJs()); const handle = yield* Effect.promise(() => openDb(env)); const executor = yield* makeScopedExecutor( accountId, @@ -94,10 +92,6 @@ export const makeCloudflareApp = async ( ) => { const plugins = makeCloudflarePlugins(config.secretKey); - // Load the Workers-compatible (WASM-inlined) QuickJS variant before any - // executor is built, the default variant cannot fetch its .wasm on Workers. - await preloadQuickJs(); - // Open and idempotently bring up the D1 schema once. This is the long-lived // handle the per-request scoped executor reads through the DbProvider seam. const handle = await openDb(env); diff --git a/apps/host-cloudflare/src/execution.test.ts b/apps/host-cloudflare/src/execution.test.ts new file mode 100644 index 0000000000..f0eb33b707 --- /dev/null +++ b/apps/host-cloudflare/src/execution.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect } from "effect"; + +import { withLazyPreload } from "./execution"; + +const makeInner = (calls: string[]) => ({ + execute: (code: string) => { + calls.push(`execute:${code}`); + return Effect.succeed({ result: code }); + }, +}); + +describe("withLazyPreload", () => { + it.effect("loads nothing until the first execute, then runs the code", () => + Effect.gen(function* () { + const calls: string[] = []; + const preload = () => { + calls.push("preload"); + return Promise.resolve(); + }; + const executor = withLazyPreload( + // oxlint-disable-next-line executor/no-double-cast -- test double: only execute is used + makeInner(calls) as unknown as Parameters[0], + preload, + ); + + expect(calls).toEqual([]); + + // oxlint-disable-next-line executor/no-double-cast -- test double: the invoker is never called + const invoker = {} as unknown as Parameters[1]; + const outcome = yield* executor.execute("1 + 1", invoker); + + expect(calls).toEqual(["preload", "execute:1 + 1"]); + expect(outcome).toEqual({ result: "1 + 1" }); + }), + ); +}); diff --git a/apps/host-cloudflare/src/execution.ts b/apps/host-cloudflare/src/execution.ts index a63afea50b..ccbcf3db04 100644 --- a/apps/host-cloudflare/src/execution.ts +++ b/apps/host-cloudflare/src/execution.ts @@ -1,4 +1,4 @@ -import { Effect, Layer } from "effect"; +import { Effect, Layer, type Context } from "effect"; import { CodeExecutorProvider, @@ -32,11 +32,32 @@ import { makeCloudflarePlugins } from "./plugins"; export { makeExecutionStack } from "@executor-js/api/server"; export { EngineDecoratorNoop }; +type CodeExecutor = Context.Service.Shape; + +// QuickJS must have the Workers WASM-inlined variant loaded before it runs +// (the default variant cannot fetch its .wasm on Workers). Executors are built +// synchronously, so the load happens on the first `execute`, the async boundary +// right before code runs. Building the app or serving a request that never +// executes code does not load QuickJS. The load is memoized per isolate. +export const withLazyPreload = ( + executor: CodeExecutor, + preload: () => Promise, +): CodeExecutor => ({ + ...executor, + execute: (code, toolInvoker) => + Effect.promise(preload).pipe(Effect.flatMap(() => executor.execute(code, toolInvoker))), +}); + +const preloadQuickJsOnDemand = (): Promise => + import("./quickjs").then((m) => m.preloadQuickJs()); + export const CloudflareCodeExecutorProvider: Layer.Layer = Layer.sync( CodeExecutorProvider, () => { const loader = (env as { LOADER?: WorkerLoader }).LOADER; - return loader ? makeDynamicWorkerExecutor({ loader }) : makeQuickJsExecutor(); + return loader + ? makeDynamicWorkerExecutor({ loader }) + : withLazyPreload(makeQuickJsExecutor(), preloadQuickJsOnDemand); }, ); diff --git a/apps/host-cloudflare/src/mcp/session-durable-object.ts b/apps/host-cloudflare/src/mcp/session-durable-object.ts index ceaeb3f552..dd112494d0 100644 --- a/apps/host-cloudflare/src/mcp/session-durable-object.ts +++ b/apps/host-cloudflare/src/mcp/session-durable-object.ts @@ -30,7 +30,6 @@ import type { ResumeResponse } from "@executor-js/execution"; import { loadConfig, type CloudflareConfig, type CloudflareEnv } from "../config"; import { createD1ExecutorDb } from "../db/d1"; import { makeCloudflareExecutionStackLayer, makeExecutionStack } from "../execution"; -import { preloadQuickJs } from "../quickjs"; // --------------------------------------------------------------------------- // Cloudflare (self-host) MCP Session Durable Object — the host-cloudflare @@ -146,9 +145,6 @@ export class McpSessionDO extends McpAgentSessionDOBase preloadQuickJs()); const { engine, executor } = yield* makeExecutionStack( sessionMeta.userId, sessionMeta.organizationId, From 49c5a6c72671a3de6cae00b5727153ca6a736d74 Mon Sep 17 00:00:00 2001 From: fforres Date: Sun, 18 Oct 2026 01:16:29 -0700 Subject: [PATCH 3/5] test(host-cloudflare): cover lazy QuickJS at the provider and D1 open edge cases Claude-Session: https://claude.ai/code/session_019ZE3j9YzRKxT4LaiDCBv6S --- apps/host-cloudflare/src/db/d1.test.ts | 81 ++++++++++++++++++- apps/host-cloudflare/src/execution.test.ts | 93 +++++++++++++++++++++- apps/host-cloudflare/src/quickjs.test.ts | 50 ++++++++++++ 3 files changed, 220 insertions(+), 4 deletions(-) create mode 100644 apps/host-cloudflare/src/quickjs.test.ts diff --git a/apps/host-cloudflare/src/db/d1.test.ts b/apps/host-cloudflare/src/db/d1.test.ts index 9d1d860ff7..bc55fd201e 100644 --- a/apps/host-cloudflare/src/db/d1.test.ts +++ b/apps/host-cloudflare/src/db/d1.test.ts @@ -1,11 +1,40 @@ import { describe, expect, it } from "@effect/vitest"; +// oxlint-disable-next-line executor/no-vitest-import -- vi.mock hoisting needs the vitest import +import { vi } from "vitest"; import type { D1Database } from "@cloudflare/workers-types"; import { collectTables, type SqliteDataMigrationClient } from "@executor-js/sdk"; import { createSqliteTestFumaDb } from "@executor-js/sdk/testing"; -import { createD1ExecutorDb } from "./d1"; -import { cloudflareDataMigrationIds } from "./data-migrations"; +const { extraMigrationIds, prepareSpy } = vi.hoisted(() => ({ + extraMigrationIds: [] as string[], + prepareSpy: vi.fn(), +})); + +vi.mock("./data-migrations", async (importOriginal) => { + const original = await importOriginal(); + return { + ...original, + cloudflareDataMigrationIds: () => [ + ...original.cloudflareDataMigrationIds(), + ...extraMigrationIds, + ], + prepareCloudflareD1Data: (...args: Parameters) => { + prepareSpy(); + return original.prepareCloudflareD1Data(...args); + }, + }; +}); + +const { createD1ExecutorDb } = await import("./d1"); +const { cloudflareDataMigrationIds } = await import("./data-migrations"); + +const readMarker = async (client: SqliteDataMigrationClient): Promise => { + const result = await client.execute( + `SELECT fingerprint FROM private_executor_cloudflare_schema_fingerprint WHERE id = 'runtime-schema'`, + ); + return String(result.rows[0]?.fingerprint); +}; const makeRecordingD1 = ( client: SqliteDataMigrationClient, @@ -301,4 +330,52 @@ describe("createD1ExecutorDb", () => { await sqlite.close(); }); + + it("prepares the database and rewrites the marker when a data migration id is added", async () => { + const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); + const { db, statements } = makeRecordingD1(sqlite.client); + const first = await createD1ExecutorDb(db, undefined); + await first.close(); + const oldMarker = await readMarker(sqlite.client); + + extraMigrationIds.push("9999-added-migration"); + prepareSpy.mockClear(); + const secondStart = statements.length; + const second = await createD1ExecutorDb(db, undefined); + await second.close(); + extraMigrationIds.length = 0; + + expect(prepareSpy).toHaveBeenCalledTimes(1); + expect( + statements.slice(secondStart).some((sql) => sql.includes("SELECT name FROM data_migration")), + ).toBe(true); + expect(await readMarker(sqlite.client)).not.toBe(oldMarker); + + await sqlite.close(); + }); + + it("keeps the old marker when data preparation fails so the next open retries", async () => { + const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); + const { db, failWhen } = makeRecordingD1(sqlite.client); + const first = await createD1ExecutorDb(db, undefined); + await first.close(); + const oldMarker = await readMarker(sqlite.client); + + extraMigrationIds.push("9999-added-migration"); + failWhen((sql) => sql.includes("SELECT name FROM data_migration")); + await expect(createD1ExecutorDb(db, undefined)).rejects.toBeDefined(); + + expect(await readMarker(sqlite.client)).toBe(oldMarker); + + failWhen(null); + prepareSpy.mockClear(); + const retried = await createD1ExecutorDb(db, undefined); + await retried.close(); + extraMigrationIds.length = 0; + + expect(prepareSpy).toHaveBeenCalledTimes(1); + expect(await readMarker(sqlite.client)).not.toBe(oldMarker); + + await sqlite.close(); + }); }); diff --git a/apps/host-cloudflare/src/execution.test.ts b/apps/host-cloudflare/src/execution.test.ts index f0eb33b707..8ba6c748ae 100644 --- a/apps/host-cloudflare/src/execution.test.ts +++ b/apps/host-cloudflare/src/execution.test.ts @@ -1,7 +1,23 @@ import { describe, expect, it } from "@effect/vitest"; -import { Effect } from "effect"; +// oxlint-disable-next-line executor/no-vitest-import -- vi.mock hoisting needs the vitest import +import { beforeEach, vi } from "vitest"; +import { Effect, Exit } from "effect"; -import { withLazyPreload } from "./execution"; +import { CodeExecutorProvider } from "@executor-js/api/server"; +import { env } from "cloudflare:workers"; + +const { preloadQuickJs } = vi.hoisted(() => ({ + preloadQuickJs: vi.fn((): Promise => Promise.resolve()), +})); + +// oxlint-disable-next-line executor/no-promise-reject -- test boundary: a rejected dynamic import is a Promise rejection +const failLoad = (): Promise => Promise.reject("wasm load failed"); + +const workerEnv = env as Record; + +vi.mock("./quickjs", () => ({ preloadQuickJs })); + +const { CloudflareCodeExecutorProvider, withLazyPreload } = await import("./execution"); const makeInner = (calls: string[]) => ({ execute: (code: string) => { @@ -35,3 +51,76 @@ describe("withLazyPreload", () => { }), ); }); + +const makeInvoker = () => + // oxlint-disable-next-line executor/no-double-cast -- test double: the code under test never calls a tool + ({}) as unknown as Parameters["execute"]>[1]; + +const buildProvider = CodeExecutorProvider.asEffect().pipe( + Effect.provide(CloudflareCodeExecutorProvider), +); + +describe("CloudflareCodeExecutorProvider", () => { + beforeEach(() => { + preloadQuickJs.mockReset(); + preloadQuickJs.mockImplementation(() => Promise.resolve()); + delete workerEnv.LOADER; + }); + + it.effect("does not load QuickJS when the provider is built", () => + Effect.gen(function* () { + yield* buildProvider; + + expect(preloadQuickJs).not.toHaveBeenCalled(); + }), + ); + + it.effect("asks for the QuickJS load on the first execute and again on each later one", () => + Effect.gen(function* () { + const executor = yield* buildProvider; + expect(preloadQuickJs).toHaveBeenCalledTimes(0); + + yield* executor.execute("1 + 1", makeInvoker()); + expect(preloadQuickJs).toHaveBeenCalledTimes(1); + + yield* executor.execute("2 + 2", makeInvoker()); + expect(preloadQuickJs).toHaveBeenCalledTimes(2); + }), + ); + + it.effect("fails execute without running code when the QuickJS load rejects", () => + Effect.gen(function* () { + preloadQuickJs.mockImplementation(failLoad); + const executor = yield* buildProvider; + + const exit = yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); + + expect(Exit.isFailure(exit)).toBe(true); + expect(preloadQuickJs).toHaveBeenCalledTimes(1); + }), + ); + + it.effect("asks for the load again on the next execute after a failed load", () => + Effect.gen(function* () { + preloadQuickJs.mockImplementationOnce(failLoad); + const executor = yield* buildProvider; + + yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); + const retry = yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); + + expect(preloadQuickJs).toHaveBeenCalledTimes(2); + expect(Exit.isSuccess(retry)).toBe(true); + }), + ); + + it.effect("never loads QuickJS when a LOADER binding is present", () => + Effect.gen(function* () { + workerEnv.LOADER = {}; + const executor = yield* buildProvider; + + yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); + + expect(preloadQuickJs).not.toHaveBeenCalled(); + }), + ); +}); diff --git a/apps/host-cloudflare/src/quickjs.test.ts b/apps/host-cloudflare/src/quickjs.test.ts new file mode 100644 index 0000000000..31932d58ad --- /dev/null +++ b/apps/host-cloudflare/src/quickjs.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "@effect/vitest"; +// oxlint-disable-next-line executor/no-vitest-import -- vi.mock hoisting needs the vitest import +import { beforeEach, vi } from "vitest"; + +const { newQuickJSWASMModuleFromVariant, setQuickJSModule } = vi.hoisted(() => ({ + newQuickJSWASMModuleFromVariant: vi.fn(), + setQuickJSModule: vi.fn(), +})); + +vi.mock("quickjs-emscripten-core", () => ({ + newQuickJSWASMModuleFromVariant, + newVariant: (base: unknown) => base, +})); +vi.mock("@jitl/quickjs-wasmfile-release-sync", () => ({ default: {} })); +vi.mock("./quickjs-engine.wasm", () => ({ default: {} })); +vi.mock("@executor-js/runtime-quickjs", () => ({ setQuickJSModule })); + +describe("preloadQuickJs", () => { + beforeEach(() => { + vi.resetModules(); + newQuickJSWASMModuleFromVariant.mockReset(); + setQuickJSModule.mockReset(); + }); + + it("builds the module once and shares it across calls", async () => { + newQuickJSWASMModuleFromVariant.mockResolvedValue({ id: "module" }); + const { preloadQuickJs } = await import("./quickjs"); + + await preloadQuickJs(); + await preloadQuickJs(); + + expect(newQuickJSWASMModuleFromVariant).toHaveBeenCalledTimes(1); + expect(setQuickJSModule).toHaveBeenCalledTimes(1); + expect(setQuickJSModule).toHaveBeenCalledWith({ id: "module" }); + }); + + // Documents current behavior: the rejected promise is memoized, so a failed + // load is never retried within the isolate. + it("keeps returning the same rejection after a failed load", async () => { + newQuickJSWASMModuleFromVariant.mockRejectedValue("wasm load failed"); + const { preloadQuickJs } = await import("./quickjs"); + + await expect(preloadQuickJs()).rejects.toBe("wasm load failed"); + newQuickJSWASMModuleFromVariant.mockResolvedValue({ id: "module" }); + await expect(preloadQuickJs()).rejects.toBe("wasm load failed"); + + expect(newQuickJSWASMModuleFromVariant).toHaveBeenCalledTimes(1); + expect(setQuickJSModule).not.toHaveBeenCalled(); + }); +}); From e8dc40ed2c737b5f06743b88c23ccd449b834cee Mon Sep 17 00:00:00 2001 From: fforres Date: Sun, 18 Oct 2026 01:22:45 -0700 Subject: [PATCH 4/5] fix(runtime-quickjs): resolve the QuickJS module lazily through a host loader Hosts that cannot fetch the default WASM register a loader that runs on the first resolve and is memoized, with a rejected load retried. The Cloudflare worker registers its WASM-inlined variant once at module scope, so the artifact smoke render in the MCP session Durable Object works again without instantiating QuickJS at startup. Claude-Session: https://claude.ai/code/session_019ZE3j9YzRKxT4LaiDCBv6S --- apps/host-cloudflare/src/execution.test.ts | 126 ++---------------- apps/host-cloudflare/src/execution.ts | 25 +--- apps/host-cloudflare/src/quickjs.test.ts | 50 ------- apps/host-cloudflare/src/quickjs.ts | 20 +-- apps/host-cloudflare/src/worker.ts | 8 ++ e2e/cloudflare/artifact-smoke-render.test.ts | 32 +++++ packages/kernel/runtime-quickjs/src/index.ts | 32 ++++- .../runtime-quickjs/src/module-loader.test.ts | 89 +++++++++++++ .../runtime-quickjs/src/sealed-bundle.ts | 5 +- 9 files changed, 182 insertions(+), 205 deletions(-) delete mode 100644 apps/host-cloudflare/src/quickjs.test.ts create mode 100644 e2e/cloudflare/artifact-smoke-render.test.ts create mode 100644 packages/kernel/runtime-quickjs/src/module-loader.test.ts diff --git a/apps/host-cloudflare/src/execution.test.ts b/apps/host-cloudflare/src/execution.test.ts index 8ba6c748ae..ff8b54f18e 100644 --- a/apps/host-cloudflare/src/execution.test.ts +++ b/apps/host-cloudflare/src/execution.test.ts @@ -1,126 +1,28 @@ import { describe, expect, it } from "@effect/vitest"; // oxlint-disable-next-line executor/no-vitest-import -- vi.mock hoisting needs the vitest import -import { beforeEach, vi } from "vitest"; -import { Effect, Exit } from "effect"; +import { vi } from "vitest"; +import { Effect } from "effect"; import { CodeExecutorProvider } from "@executor-js/api/server"; -import { env } from "cloudflare:workers"; -const { preloadQuickJs } = vi.hoisted(() => ({ - preloadQuickJs: vi.fn((): Promise => Promise.resolve()), -})); - -// oxlint-disable-next-line executor/no-promise-reject -- test boundary: a rejected dynamic import is a Promise rejection -const failLoad = (): Promise => Promise.reject("wasm load failed"); - -const workerEnv = env as Record; - -vi.mock("./quickjs", () => ({ preloadQuickJs })); - -const { CloudflareCodeExecutorProvider, withLazyPreload } = await import("./execution"); - -const makeInner = (calls: string[]) => ({ - execute: (code: string) => { - calls.push(`execute:${code}`); - return Effect.succeed({ result: code }); - }, -}); - -describe("withLazyPreload", () => { - it.effect("loads nothing until the first execute, then runs the code", () => - Effect.gen(function* () { - const calls: string[] = []; - const preload = () => { - calls.push("preload"); - return Promise.resolve(); - }; - const executor = withLazyPreload( - // oxlint-disable-next-line executor/no-double-cast -- test double: only execute is used - makeInner(calls) as unknown as Parameters[0], - preload, - ); - - expect(calls).toEqual([]); - - // oxlint-disable-next-line executor/no-double-cast -- test double: the invoker is never called - const invoker = {} as unknown as Parameters[1]; - const outcome = yield* executor.execute("1 + 1", invoker); +const { loadQuickJs } = vi.hoisted(() => ({ loadQuickJs: vi.fn() })); - expect(calls).toEqual(["preload", "execute:1 + 1"]); - expect(outcome).toEqual({ result: "1 + 1" }); - }), - ); -}); - -const makeInvoker = () => - // oxlint-disable-next-line executor/no-double-cast -- test double: the code under test never calls a tool - ({}) as unknown as Parameters["execute"]>[1]; +vi.mock("./quickjs", () => ({ loadQuickJs })); +vi.mock("./mcp", () => ({ McpExecutionOwnerDirectoryDO: class {}, McpSessionDO: class {} })); +vi.mock("./app", () => ({ + makeCloudflareApp: vi.fn(), + reconcileInternalIntegrationsOnce: vi.fn(), +})); -const buildProvider = CodeExecutorProvider.asEffect().pipe( - Effect.provide(CloudflareCodeExecutorProvider), -); +await import("./worker"); +const { CloudflareCodeExecutorProvider } = await import("./execution"); describe("CloudflareCodeExecutorProvider", () => { - beforeEach(() => { - preloadQuickJs.mockReset(); - preloadQuickJs.mockImplementation(() => Promise.resolve()); - delete workerEnv.LOADER; - }); - - it.effect("does not load QuickJS when the provider is built", () => + it.effect("does not load QuickJS when the worker starts or the provider is built", () => Effect.gen(function* () { - yield* buildProvider; - - expect(preloadQuickJs).not.toHaveBeenCalled(); - }), - ); - - it.effect("asks for the QuickJS load on the first execute and again on each later one", () => - Effect.gen(function* () { - const executor = yield* buildProvider; - expect(preloadQuickJs).toHaveBeenCalledTimes(0); - - yield* executor.execute("1 + 1", makeInvoker()); - expect(preloadQuickJs).toHaveBeenCalledTimes(1); - - yield* executor.execute("2 + 2", makeInvoker()); - expect(preloadQuickJs).toHaveBeenCalledTimes(2); - }), - ); - - it.effect("fails execute without running code when the QuickJS load rejects", () => - Effect.gen(function* () { - preloadQuickJs.mockImplementation(failLoad); - const executor = yield* buildProvider; - - const exit = yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); - - expect(Exit.isFailure(exit)).toBe(true); - expect(preloadQuickJs).toHaveBeenCalledTimes(1); - }), - ); - - it.effect("asks for the load again on the next execute after a failed load", () => - Effect.gen(function* () { - preloadQuickJs.mockImplementationOnce(failLoad); - const executor = yield* buildProvider; - - yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); - const retry = yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); - - expect(preloadQuickJs).toHaveBeenCalledTimes(2); - expect(Exit.isSuccess(retry)).toBe(true); - }), - ); - - it.effect("never loads QuickJS when a LOADER binding is present", () => - Effect.gen(function* () { - workerEnv.LOADER = {}; - const executor = yield* buildProvider; - - yield* Effect.exit(executor.execute("1 + 1", makeInvoker())); + yield* CodeExecutorProvider.asEffect().pipe(Effect.provide(CloudflareCodeExecutorProvider)); - expect(preloadQuickJs).not.toHaveBeenCalled(); + expect(loadQuickJs).not.toHaveBeenCalled(); }), ); }); diff --git a/apps/host-cloudflare/src/execution.ts b/apps/host-cloudflare/src/execution.ts index ccbcf3db04..a63afea50b 100644 --- a/apps/host-cloudflare/src/execution.ts +++ b/apps/host-cloudflare/src/execution.ts @@ -1,4 +1,4 @@ -import { Effect, Layer, type Context } from "effect"; +import { Effect, Layer } from "effect"; import { CodeExecutorProvider, @@ -32,32 +32,11 @@ import { makeCloudflarePlugins } from "./plugins"; export { makeExecutionStack } from "@executor-js/api/server"; export { EngineDecoratorNoop }; -type CodeExecutor = Context.Service.Shape; - -// QuickJS must have the Workers WASM-inlined variant loaded before it runs -// (the default variant cannot fetch its .wasm on Workers). Executors are built -// synchronously, so the load happens on the first `execute`, the async boundary -// right before code runs. Building the app or serving a request that never -// executes code does not load QuickJS. The load is memoized per isolate. -export const withLazyPreload = ( - executor: CodeExecutor, - preload: () => Promise, -): CodeExecutor => ({ - ...executor, - execute: (code, toolInvoker) => - Effect.promise(preload).pipe(Effect.flatMap(() => executor.execute(code, toolInvoker))), -}); - -const preloadQuickJsOnDemand = (): Promise => - import("./quickjs").then((m) => m.preloadQuickJs()); - export const CloudflareCodeExecutorProvider: Layer.Layer = Layer.sync( CodeExecutorProvider, () => { const loader = (env as { LOADER?: WorkerLoader }).LOADER; - return loader - ? makeDynamicWorkerExecutor({ loader }) - : withLazyPreload(makeQuickJsExecutor(), preloadQuickJsOnDemand); + return loader ? makeDynamicWorkerExecutor({ loader }) : makeQuickJsExecutor(); }, ); diff --git a/apps/host-cloudflare/src/quickjs.test.ts b/apps/host-cloudflare/src/quickjs.test.ts deleted file mode 100644 index 31932d58ad..0000000000 --- a/apps/host-cloudflare/src/quickjs.test.ts +++ /dev/null @@ -1,50 +0,0 @@ -import { describe, expect, it } from "@effect/vitest"; -// oxlint-disable-next-line executor/no-vitest-import -- vi.mock hoisting needs the vitest import -import { beforeEach, vi } from "vitest"; - -const { newQuickJSWASMModuleFromVariant, setQuickJSModule } = vi.hoisted(() => ({ - newQuickJSWASMModuleFromVariant: vi.fn(), - setQuickJSModule: vi.fn(), -})); - -vi.mock("quickjs-emscripten-core", () => ({ - newQuickJSWASMModuleFromVariant, - newVariant: (base: unknown) => base, -})); -vi.mock("@jitl/quickjs-wasmfile-release-sync", () => ({ default: {} })); -vi.mock("./quickjs-engine.wasm", () => ({ default: {} })); -vi.mock("@executor-js/runtime-quickjs", () => ({ setQuickJSModule })); - -describe("preloadQuickJs", () => { - beforeEach(() => { - vi.resetModules(); - newQuickJSWASMModuleFromVariant.mockReset(); - setQuickJSModule.mockReset(); - }); - - it("builds the module once and shares it across calls", async () => { - newQuickJSWASMModuleFromVariant.mockResolvedValue({ id: "module" }); - const { preloadQuickJs } = await import("./quickjs"); - - await preloadQuickJs(); - await preloadQuickJs(); - - expect(newQuickJSWASMModuleFromVariant).toHaveBeenCalledTimes(1); - expect(setQuickJSModule).toHaveBeenCalledTimes(1); - expect(setQuickJSModule).toHaveBeenCalledWith({ id: "module" }); - }); - - // Documents current behavior: the rejected promise is memoized, so a failed - // load is never retried within the isolate. - it("keeps returning the same rejection after a failed load", async () => { - newQuickJSWASMModuleFromVariant.mockRejectedValue("wasm load failed"); - const { preloadQuickJs } = await import("./quickjs"); - - await expect(preloadQuickJs()).rejects.toBe("wasm load failed"); - newQuickJSWASMModuleFromVariant.mockResolvedValue({ id: "module" }); - await expect(preloadQuickJs()).rejects.toBe("wasm load failed"); - - expect(newQuickJSWASMModuleFromVariant).toHaveBeenCalledTimes(1); - expect(setQuickJSModule).not.toHaveBeenCalled(); - }); -}); diff --git a/apps/host-cloudflare/src/quickjs.ts b/apps/host-cloudflare/src/quickjs.ts index 9d84484ed6..fbf8995e1e 100644 --- a/apps/host-cloudflare/src/quickjs.ts +++ b/apps/host-cloudflare/src/quickjs.ts @@ -9,7 +9,7 @@ import baseVariant from "@jitl/quickjs-wasmfile-release-sync"; // monorepo-root node_modules path — see scripts/vendor-quickjs-wasm.ts. import wasmModule from "./quickjs-engine.wasm"; -import { setQuickJSModule } from "@executor-js/runtime-quickjs"; +import type { QuickJSWASMModule } from "quickjs-emscripten"; // --------------------------------------------------------------------------- // QuickJS-on-Workers WASM loading. @@ -18,18 +18,10 @@ import { setQuickJSModule } from "@executor-js/runtime-quickjs"; // build (its `./emscripten-module` export has a `workerd` condition wrangler // selects) — that build expects the WASM module to be supplied rather than // fetched/compiled at runtime. `newVariant(base, { wasmModule })` hands it the -// statically-imported, pre-compiled module, and `setQuickJSModule` makes every -// `makeQuickJsExecutor()` reuse it. Preloaded once per isolate. +// statically-imported, pre-compiled module. The worker entry registers this +// through `setQuickJSModuleLoader`, so the runtime builds it on the first +// execution and reuses it for the rest of the isolate. // --------------------------------------------------------------------------- -let preloaded: Promise | null = null; - -export const preloadQuickJs = (): Promise => { - if (!preloaded) { - const variant = newVariant(baseVariant, { wasmModule }); - preloaded = newQuickJSWASMModuleFromVariant(variant).then((mod) => { - setQuickJSModule(mod); - }); - } - return preloaded; -}; +export const loadQuickJs = (): Promise => + newQuickJSWASMModuleFromVariant(newVariant(baseVariant, { wasmModule })); diff --git a/apps/host-cloudflare/src/worker.ts b/apps/host-cloudflare/src/worker.ts index 6ed8951936..9a59c8b5a2 100644 --- a/apps/host-cloudflare/src/worker.ts +++ b/apps/host-cloudflare/src/worker.ts @@ -1,5 +1,7 @@ import { WorkerEntrypoint } from "cloudflare:workers"; +import { setQuickJSModuleLoader } from "@executor-js/runtime-quickjs"; + import { makeCloudflareApp, reconcileInternalIntegrationsOnce } from "./app"; import { cloudflareAccessConfigErrorMessage, @@ -9,6 +11,12 @@ import { } from "./config"; import { mcpResourceFromPath } from "./mcp/resource"; +// Workers cannot fetch the default QuickJS .wasm, so every QuickJS user in this +// script (the fetch handlers and the MCP Durable Objects) resolves the +// WASM-inlined variant through this loader. The import stays dynamic so the +// engine is not instantiated or evaluated at startup. +setQuickJSModuleLoader(() => import("./quickjs").then((m) => m.loadQuickJs())); + // The MCP Durable Object classes, bound in wrangler.jsonc. They must be exported // at the Worker entry module scope for the runtime to find them. export { McpExecutionOwnerDirectoryDO, McpSessionDO } from "./mcp"; diff --git a/e2e/cloudflare/artifact-smoke-render.test.ts b/e2e/cloudflare/artifact-smoke-render.test.ts new file mode 100644 index 0000000000..1b0fb320a4 --- /dev/null +++ b/e2e/cloudflare/artifact-smoke-render.test.ts @@ -0,0 +1,32 @@ +// Cloudflare self-host: create-artifact runs its smoke render in QuickJS inside +// workerd. The render fails open when the sandbox is unavailable, so a created +// artifact proves nothing; a component that throws on first render being +// REFUSED proves the WASM-inlined QuickJS actually ran. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; + +import { scenario } from "../src/scenario"; +import { Mcp, Target } from "../src/services"; + +scenario( + "Cloudflare · create-artifact refuses a component that throws on first render", + { timeout: 120_000 }, + Effect.gen(function* () { + const target = yield* Target; + const mcp = yield* Mcp; + const identity = yield* target.newIdentity(); + const session = mcp.session(identity); + const suffix = randomBytes(4).toString("hex"); + + const refused = yield* session.call("create-artifact", { + code: `function App() { throw new Error("boom-${suffix}"); }`, + title: `Throws ${suffix}`, + description: "Should never be saved", + }); + + expect(refused.ok, "create-artifact refuses a first-render throw").toBe(false); + expect(refused.text, "the refusal carries the sandbox's own error").toContain(`boom-${suffix}`); + }), +); diff --git a/packages/kernel/runtime-quickjs/src/index.ts b/packages/kernel/runtime-quickjs/src/index.ts index d8a8e48dd3..5d517b964b 100644 --- a/packages/kernel/runtime-quickjs/src/index.ts +++ b/packages/kernel/runtime-quickjs/src/index.ts @@ -35,8 +35,32 @@ export const setQuickJSModule = (mod: QuickJSWASMModule) => { preloadedModule = mod; }; -const resolveQuickJS = (): Promise => - preloadedModule ? Promise.resolve(preloadedModule) : getQuickJS(); +export type QuickJSModuleLoader = () => Promise; + +let moduleLoader: QuickJSModuleLoader | null = null; +let loadingModule: Promise | null = null; + +// Register a lazy source for the module (Workers must supply a WASM-inlined +// variant). The loader first runs on the first resolve, so hosts pay the +// instantiation cost only when code actually executes. +export const setQuickJSModuleLoader = (loader: QuickJSModuleLoader) => { + moduleLoader = loader; + loadingModule = null; +}; + +const resolveQuickJS = (): Promise => { + if (preloadedModule) return Promise.resolve(preloadedModule); + if (!moduleLoader) return getQuickJS(); + if (!loadingModule) { + // oxlint-disable-next-line executor/no-promise-catch -- boundary: this module has no Effect context; reset the memo on failure so the next resolve retries instead of caching a permanent rejection + loadingModule = moduleLoader().catch((err: unknown) => { + loadingModule = null; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: rethrow so the caller still observes the original failure + throw err; + }); + } + return loadingModule; +}; /** * Run a sealed, self-contained bundle in a QuickJS sandbox. @@ -44,8 +68,8 @@ const resolveQuickJS = (): Promise => * A sibling of `makeQuickJsExecutor` for validation work rather than user * execution — no tool bridge, no code recovery, no metering above it. See * `sealed-bundle.ts` for why that separation matters. The WASM module is - * resolved the same way, so a host that called `setQuickJSModule` (Workers must) - * is honoured here too. + * resolved the same way, so a host that called `setQuickJSModule` or + * `setQuickJSModuleLoader` (Workers must) is honoured here too. */ export const executeSealedBundle = ( options: SealedBundleOptions, diff --git a/packages/kernel/runtime-quickjs/src/module-loader.test.ts b/packages/kernel/runtime-quickjs/src/module-loader.test.ts new file mode 100644 index 0000000000..225191e507 --- /dev/null +++ b/packages/kernel/runtime-quickjs/src/module-loader.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it } from "@effect/vitest"; +// oxlint-disable-next-line executor/no-vitest-import -- vi.resetModules gives each test fresh module-level loader state +import { beforeEach, vi } from "vitest"; +import * as Effect from "effect/Effect"; +import { getQuickJS } from "quickjs-emscripten"; + +// The loader seam is module-level state, so every test imports a fresh copy. +const freshRuntime = () => import("./index"); + +const run = (runtime: Awaited>) => + Effect.runPromise(runtime.executeSealedBundle({ bundle: "globalThis.value = 1;", call: "'ok'" })); + +describe("setQuickJSModuleLoader", () => { + beforeEach(() => { + vi.resetModules(); + }); + + it("does not call the loader until the first resolve", async () => { + const runtime = await freshRuntime(); + let calls = 0; + runtime.setQuickJSModuleLoader(() => { + calls += 1; + return getQuickJS(); + }); + + expect(calls).toBe(0); + await run(runtime); + expect(calls).toBe(1); + }); + + it("calls the loader once across many resolves", async () => { + const runtime = await freshRuntime(); + let calls = 0; + runtime.setQuickJSModuleLoader(() => { + calls += 1; + return getQuickJS(); + }); + + await Promise.all([run(runtime), run(runtime), run(runtime)]); + await run(runtime); + + expect(calls).toBe(1); + }); + + it("prefers a preloaded module over the loader", async () => { + const runtime = await freshRuntime(); + let calls = 0; + runtime.setQuickJSModuleLoader(() => { + calls += 1; + return getQuickJS(); + }); + runtime.setQuickJSModule(await getQuickJS()); + + await run(runtime); + + expect(calls).toBe(0); + }); + + it("retries the loader on the next resolve after a rejected load", async () => { + const runtime = await freshRuntime(); + let calls = 0; + runtime.setQuickJSModuleLoader(() => { + calls += 1; + return calls === 1 ? Promise.reject(new Error("wasm load failed")) : getQuickJS(); + }); + + await expect(run(runtime)).rejects.toThrow("wasm load failed"); + await expect(run(runtime)).resolves.toBe("ok"); + + expect(calls).toBe(2); + }); + + it("serves the sealed bundle runner and the executor from the same loader", async () => { + const runtime = await freshRuntime(); + let calls = 0; + runtime.setQuickJSModuleLoader(() => { + calls += 1; + return getQuickJS(); + }); + + await run(runtime); + const executed = await Effect.runPromise( + runtime.makeQuickJsExecutor().execute("return 1 + 2", { invoke: () => Effect.void }), + ); + + expect(executed.result).toBe(3); + expect(calls).toBe(1); + }); +}); diff --git a/packages/kernel/runtime-quickjs/src/sealed-bundle.ts b/packages/kernel/runtime-quickjs/src/sealed-bundle.ts index 56619df68b..0d30ff738c 100644 --- a/packages/kernel/runtime-quickjs/src/sealed-bundle.ts +++ b/packages/kernel/runtime-quickjs/src/sealed-bundle.ts @@ -75,8 +75,9 @@ const DEFAULT_MAX_STACK_SIZE_BYTES = 2 * 1024 * 1024; const BUNDLE_FILENAME = "executor-sealed-bundle.js"; const CALL_FILENAME = "executor-sealed-call.js"; -/** Set by `setQuickJSModule` in this package's main entry. Read lazily so a - * host that preloads a custom WASM module (Workers must) is honoured. */ +/** Supplied by this package's main entry (`setQuickJSModule` or + * `setQuickJSModuleLoader`). Read lazily so a host that provides a custom WASM + * module (Workers must) is honoured. */ type ModuleResolver = () => Promise; const errorMessage = (cause: unknown): string => From 665ec64104aa15877aa0b07f2f70c1471d7e29a5 Mon Sep 17 00:00:00 2001 From: fforres Date: Sun, 18 Oct 2026 01:22:45 -0700 Subject: [PATCH 5/5] refactor(host-cloudflare): derive the D1 compatibility fingerprint and simplify tests The fingerprint now hashes the compatibility repair source instead of a hand-bumped version, is computed once per isolate, and is read in parallel with the stored marker. The D1 tests assert through the marker row and recorded statements instead of a module mock. Claude-Session: https://claude.ai/code/session_019ZE3j9YzRKxT4LaiDCBv6S --- apps/host-cloudflare/src/db/d1.test.ts | 203 +++++++----------- apps/host-cloudflare/src/db/d1.ts | 28 ++- .../host-cloudflare/src/db/data-migrations.ts | 16 +- 3 files changed, 112 insertions(+), 135 deletions(-) diff --git a/apps/host-cloudflare/src/db/d1.test.ts b/apps/host-cloudflare/src/db/d1.test.ts index bc55fd201e..b9a95b0b8e 100644 --- a/apps/host-cloudflare/src/db/d1.test.ts +++ b/apps/host-cloudflare/src/db/d1.test.ts @@ -1,33 +1,11 @@ import { describe, expect, it } from "@effect/vitest"; -// oxlint-disable-next-line executor/no-vitest-import -- vi.mock hoisting needs the vitest import -import { vi } from "vitest"; import type { D1Database } from "@cloudflare/workers-types"; import { collectTables, type SqliteDataMigrationClient } from "@executor-js/sdk"; import { createSqliteTestFumaDb } from "@executor-js/sdk/testing"; -const { extraMigrationIds, prepareSpy } = vi.hoisted(() => ({ - extraMigrationIds: [] as string[], - prepareSpy: vi.fn(), -})); - -vi.mock("./data-migrations", async (importOriginal) => { - const original = await importOriginal(); - return { - ...original, - cloudflareDataMigrationIds: () => [ - ...original.cloudflareDataMigrationIds(), - ...extraMigrationIds, - ], - prepareCloudflareD1Data: (...args: Parameters) => { - prepareSpy(); - return original.prepareCloudflareD1Data(...args); - }, - }; -}); - -const { createD1ExecutorDb } = await import("./d1"); -const { cloudflareDataMigrationIds } = await import("./data-migrations"); +import { createD1ExecutorDb } from "./d1"; +import { cloudflareDataMigrationIds } from "./data-migrations"; const readMarker = async (client: SqliteDataMigrationClient): Promise => { const result = await client.execute( @@ -93,21 +71,39 @@ const isRuntimeSchemaStatement = (sql: string): boolean => { return !normalized.includes("DATA_MIGRATION"); }; +const ranDataMigrations = (statements: readonly string[]): boolean => + statements.some((sql) => sql.includes("SELECT name FROM data_migration")); + +const setMarker = (client: SqliteDataMigrationClient, fingerprint: string) => + client.execute({ + sql: `UPDATE private_executor_cloudflare_schema_fingerprint SET fingerprint = ? WHERE id = 'runtime-schema'`, + args: [fingerprint], + }); + +// Opens the database once so it is current, then opens it again and returns +// only what the second open sent to D1. +const openTwice = async ( + db: D1Database, + statements: string[], + between: () => Promise = async () => {}, +): Promise => { + const first = await createD1ExecutorDb(db, undefined); + await first.close(); + await between(); + const start = statements.length; + const second = await createD1ExecutorDb(db, undefined); + await second.close(); + return statements.slice(start); +}; + describe("createD1ExecutorDb", () => { it("does not repeat runtime schema DDL after the current schema was prepared", async () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); const { db, statements } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - const firstSchemaStatements = statements.filter(isRuntimeSchemaStatement); - expect(firstSchemaStatements.length).toBeGreaterThan(0); - - const secondStart = statements.length; - const second = await createD1ExecutorDb(db, undefined); - await second.close(); - const secondStatements = statements.slice(secondStart); + const secondStatements = await openTwice(db, statements); + expect(statements.filter(isRuntimeSchemaStatement).length).toBeGreaterThan(0); expect(secondStatements.filter(isRuntimeSchemaStatement)).toEqual([]); await sqlite.close(); @@ -116,44 +112,44 @@ describe("createD1ExecutorDb", () => { it("opens a current database with exactly one D1 query", async () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); const { db, statements } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - const secondStart = statements.length; - const second = await createD1ExecutorDb(db, undefined); - await second.close(); + const secondStatements = await openTwice(db, statements); - const secondStatements = statements.slice(secondStart); expect(secondStatements).toHaveLength(1); expect(secondStatements[0]).toContain("SELECT fingerprint"); await sqlite.close(); }); + it("records every known data migration in the ledger", async () => { + const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); + const { db } = makeRecordingD1(sqlite.client); + + const handle = await createD1ExecutorDb(db, undefined); + await handle.close(); + + const ledger = await sqlite.client.execute(`SELECT name FROM data_migration`); + expect(ledger.rows.map((row) => row.name)).toEqual( + expect.arrayContaining([...cloudflareDataMigrationIds]), + ); + + await sqlite.close(); + }); + it("runs the data migrations again when the stored fingerprint predates the migration list", async () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); const { db, statements } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - const ids = cloudflareDataMigrationIds(); - const missing = ids[ids.length - 1]; - await sqlite.client.execute({ - sql: "DELETE FROM data_migration WHERE name = ?", - args: [missing], + const missing = cloudflareDataMigrationIds[cloudflareDataMigrationIds.length - 1]; + + const secondStatements = await openTwice(db, statements, async () => { + await sqlite.client.execute({ + sql: "DELETE FROM data_migration WHERE name = ?", + args: [missing], + }); + await setMarker(sqlite.client, "older"); }); - await sqlite.client.execute( - `UPDATE private_executor_cloudflare_schema_fingerprint - SET fingerprint = 'older' - WHERE id = 'runtime-schema'`, - ); - const secondStart = statements.length; - const second = await createD1ExecutorDb(db, undefined); - await second.close(); - - expect( - statements.slice(secondStart).some((sql) => sql.includes("SELECT name FROM data_migration")), - ).toBe(true); + expect(ranDataMigrations(secondStatements)).toBe(true); const ledger = await sqlite.client.execute(`SELECT name FROM data_migration`); expect(ledger.rows.map((row) => row.name)).toContain(missing); @@ -164,19 +160,10 @@ describe("createD1ExecutorDb", () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); const { db, statements } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - await sqlite.client.execute( - `UPDATE private_executor_cloudflare_schema_fingerprint - SET fingerprint = 'stale' - WHERE id = 'runtime-schema'`, + const secondStatements = await openTwice(db, statements, () => + setMarker(sqlite.client, "stale"), ); - const secondStart = statements.length; - const second = await createD1ExecutorDb(db, undefined); - await second.close(); - const secondStatements = statements.slice(secondStart); - expect( secondStatements.some((sql) => sql.startsWith('CREATE TABLE IF NOT EXISTS "integration"')), ).toBe(true); @@ -186,20 +173,14 @@ describe("createD1ExecutorDb", () => { it("adds the compiled artifact columns to a database that predates them", async () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); - const { db } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - for (const column of ["compiled_fingerprint", "compiled_js", "compiled_css"]) { - await sqlite.client.execute(`ALTER TABLE "artifact" DROP COLUMN "${column}"`); - } - await sqlite.client.execute( - `UPDATE private_executor_cloudflare_schema_fingerprint - SET fingerprint = 'stale' - WHERE id = 'runtime-schema'`, - ); + const { db, statements } = makeRecordingD1(sqlite.client); - const second = await createD1ExecutorDb(db, undefined); - await second.close(); + await openTwice(db, statements, async () => { + for (const column of ["compiled_fingerprint", "compiled_js", "compiled_css"]) { + await sqlite.client.execute(`ALTER TABLE "artifact" DROP COLUMN "${column}"`); + } + await setMarker(sqlite.client, "stale"); + }); const columns = await sqlite.client.execute(`PRAGMA table_info("artifact")`); const names = columns.rows.map((row) => String(row.name)); @@ -215,11 +196,7 @@ describe("createD1ExecutorDb", () => { const { db } = makeRecordingD1(sqlite.client); const prepared = await createD1ExecutorDb(db, undefined); await prepared.close(); - await sqlite.client.execute( - `UPDATE private_executor_cloudflare_schema_fingerprint - SET fingerprint = 'stale' - WHERE id = 'runtime-schema'`, - ); + await setMarker(sqlite.client, "stale"); const handles = await Promise.all([ createD1ExecutorDb(db, undefined), @@ -331,50 +308,36 @@ describe("createD1ExecutorDb", () => { await sqlite.close(); }); - it("prepares the database and rewrites the marker when a data migration id is added", async () => { + it("rewrites the marker after preparing a database whose marker is out of date", async () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); const { db, statements } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - const oldMarker = await readMarker(sqlite.client); + let currentMarker = ""; - extraMigrationIds.push("9999-added-migration"); - prepareSpy.mockClear(); - const secondStart = statements.length; - const second = await createD1ExecutorDb(db, undefined); - await second.close(); - extraMigrationIds.length = 0; + const secondStatements = await openTwice(db, statements, async () => { + currentMarker = await readMarker(sqlite.client); + await setMarker(sqlite.client, "older"); + }); - expect(prepareSpy).toHaveBeenCalledTimes(1); - expect( - statements.slice(secondStart).some((sql) => sql.includes("SELECT name FROM data_migration")), - ).toBe(true); - expect(await readMarker(sqlite.client)).not.toBe(oldMarker); + expect(ranDataMigrations(secondStatements)).toBe(true); + expect(await readMarker(sqlite.client)).toBe(currentMarker); await sqlite.close(); }); it("keeps the old marker when data preparation fails so the next open retries", async () => { const sqlite = await createSqliteTestFumaDb({ tables: collectTables() }); - const { db, failWhen } = makeRecordingD1(sqlite.client); - const first = await createD1ExecutorDb(db, undefined); - await first.close(); - const oldMarker = await readMarker(sqlite.client); - - extraMigrationIds.push("9999-added-migration"); - failWhen((sql) => sql.includes("SELECT name FROM data_migration")); - await expect(createD1ExecutorDb(db, undefined)).rejects.toBeDefined(); - - expect(await readMarker(sqlite.client)).toBe(oldMarker); - - failWhen(null); - prepareSpy.mockClear(); - const retried = await createD1ExecutorDb(db, undefined); - await retried.close(); - extraMigrationIds.length = 0; + const { db, statements, failWhen } = makeRecordingD1(sqlite.client); + let currentMarker = ""; + await openTwice(db, statements, async () => { + currentMarker = await readMarker(sqlite.client); + await setMarker(sqlite.client, "older"); + failWhen((sql) => sql.includes("SELECT name FROM data_migration")); + await expect(createD1ExecutorDb(db, undefined)).rejects.toBeDefined(); + expect(await readMarker(sqlite.client)).toBe("older"); + failWhen(null); + }); - expect(prepareSpy).toHaveBeenCalledTimes(1); - expect(await readMarker(sqlite.client)).not.toBe(oldMarker); + expect(await readMarker(sqlite.client)).toBe(currentMarker); await sqlite.close(); }); diff --git a/apps/host-cloudflare/src/db/d1.ts b/apps/host-cloudflare/src/db/d1.ts index b25f48b4af..3b7d0a41b9 100644 --- a/apps/host-cloudflare/src/db/d1.ts +++ b/apps/host-cloudflare/src/db/d1.ts @@ -15,8 +15,8 @@ import { makeR2BlobStore } from "@executor-js/cloudflare/blob-store"; import { CLOUDFLARE_NAMESPACE, CLOUDFLARE_SCHEMA_VERSION } from "../config"; import { - CLOUDFLARE_D1_COMPATIBILITY_VERSION, cloudflareDataMigrationIds, + ensureCloudflareD1SchemaCompatibility, prepareCloudflareD1Data, } from "./data-migrations"; import { withD1TransientRetry } from "./d1-retry"; @@ -30,6 +30,22 @@ const schemaFingerprint = async (statements: readonly string[]): Promise return Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, "0")).join(""); }; +// Every open in an isolate uses the same fixed options, so the hash is +// computed once. Hashing the compatibility function's source makes a change to +// the repair code change the fingerprint without a hand-bumped version. +let expectedFingerprintCache: Promise | null = null; + +const expectedSchemaFingerprint = ( + options: Parameters[0], +): Promise => { + expectedFingerprintCache ??= schemaFingerprint([ + ...createDrizzleRuntimeSchemaSqlFromTables(options), + `compatibility:${ensureCloudflareD1SchemaCompatibility.toString()}`, + `data-migrations:${cloudflareDataMigrationIds.join(",")}`, + ]); + return expectedFingerprintCache; +}; + const isMissingFingerprintTable = (error: unknown): boolean => { for (let current = error, depth = 0; current != null && depth < 8; depth += 1) { const record = @@ -109,17 +125,15 @@ export const createD1ExecutorDb = async ( // exposes one. A generated fingerprint lets every Worker/DO isolate prove the // expected generated schema was already prepared with one primary read instead of // replaying dozens of idempotent CREATE/ALTER statements on every database - // open. The fingerprint also covers the compatibility-check version and the + // open. The fingerprint also covers the compatibility-check source and the // ordered data-migration ids, so a current database opens with that one read // and skips both. The marker is stored only after the ensure, compatibility // check, data migrations, and any required post-compatibility ensure succeed. // Data migrations keep their own ledger for databases that need them. - const expectedFingerprint = await schemaFingerprint([ - ...createDrizzleRuntimeSchemaSqlFromTables(options), - `compatibility:${CLOUDFLARE_D1_COMPATIBILITY_VERSION}`, - `data-migrations:${cloudflareDataMigrationIds().join(",")}`, + const [expectedFingerprint, preparedFingerprint] = await Promise.all([ + expectedSchemaFingerprint(options), + readPreparedSchemaFingerprint(db), ]); - const preparedFingerprint = await readPreparedSchemaFingerprint(db); const requiresSchemaPreparation = preparedFingerprint !== expectedFingerprint; if (requiresSchemaPreparation) { await ensureDrizzleRuntimeSchemaFromTables({ run: (query) => drizzleDb.run(query) }, options); diff --git a/apps/host-cloudflare/src/db/data-migrations.ts b/apps/host-cloudflare/src/db/data-migrations.ts index a91a0256ed..6476c4c82d 100644 --- a/apps/host-cloudflare/src/db/data-migrations.ts +++ b/apps/host-cloudflare/src/db/data-migrations.ts @@ -228,10 +228,6 @@ const copyProviderServiceSplitBlobsToR2 = ( }), }); -// Bump when `ensureCloudflareD1SchemaCompatibility` gains a repair step, so -// databases stamped by the previous logic run it once more. -export const CLOUDFLARE_D1_COMPATIBILITY_VERSION = 1; - const cloudflareDataMigrations = (bucket: R2Bucket | undefined): readonly SqliteDataMigration[] => [ { name: googleOpenApiOwnershipDataMigration.name, @@ -260,11 +256,15 @@ const cloudflareDataMigrations = (bucket: R2Bucket | undefined): readonly Sqlite encryptedSecretsRepartitionDataMigration, ]; -// Ordered ids of every data migration this build knows. The ledger already +// Ordered names of every data migration this build knows. The ledger already // makes a migration run once, and the bucket only decides how it copies blobs, -// so the ids alone describe what a fully prepared database looks like. -export const cloudflareDataMigrationIds = (): readonly string[] => - cloudflareDataMigrations(undefined).map((migration) => migration.name); +// so the names alone describe what a fully prepared database looks like. +export const cloudflareDataMigrationIds: readonly string[] = [ + googleOpenApiOwnershipDataMigration.name, + providerServiceSplitDataMigration.name, + openApiNdjsonOutputDataMigration.name, + encryptedSecretsRepartitionDataMigration.name, +]; export const prepareCloudflareD1Data = ( db: D1Database,