From 0a08c9fcf480dd2a881daff865c272dfb6ef6f2f Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 25 Jun 2026 22:02:06 +0900 Subject: [PATCH 1/4] feat(host-cloudflare): make db seam configurable (D1 default, Neon/Postgres via Hyperdrive opt-in) D1 (SQLite) stays the zero-setup default; a Hyperdrive binding (or a direct DATABASE_URL behind EXECUTOR_DIRECT_DATABASE_URL=true) switches the seam to Neon Postgres over Cloudflare Hyperdrive, modelled on apps/cloud's proven postgres.js + Hyperdrive path but keeping host-cloudflare's runtime `ensure` bring-up (no out-of-band migration step). Under Postgres the three D1 workarounds disappear: interactive transactions and unbounded parameters are restored, and the schema bring-up runs with the full drizzle handle (transactional DDL). The R2 blob seam is preserved as optional. - src/db/index.ts: createExecutorDb(env) factory + isPostgresConfigured selector - src/db/postgres.ts: postgres.js + Hyperdrive handle with runtime ensure - src/config.ts: HYPERDRIVE / DATABASE_URL / EXECUTOR_DIRECT_DATABASE_URL env - app.ts + session-durable-object.ts: switch callers to createExecutorDb - data-migrations.ts: Postgres data-migration no-op (no legacy D1 data) - wrangler.jsonc: optional (commented) hyperdrive binding - scripts/dev-db.ts: PGlite dev DB on :5433 via runtime ensure - tests: selector unit tests + PGlite-over-socket integration (round-trip + transaction commit/rollback proving the dropped D1 workarounds) - README: Neon Postgres opt-in section Refs #10 --- apps/host-cloudflare/README.md | 35 +++++- apps/host-cloudflare/package.json | 4 + apps/host-cloudflare/scripts/dev-db.ts | 91 ++++++++++++++ apps/host-cloudflare/src/app.ts | 10 +- apps/host-cloudflare/src/config.ts | 19 ++- .../host-cloudflare/src/db/data-migrations.ts | 13 ++ apps/host-cloudflare/src/db/index.test.ts | 69 +++++++++++ apps/host-cloudflare/src/db/index.ts | 38 ++++++ apps/host-cloudflare/src/db/postgres.test.ts | 116 ++++++++++++++++++ apps/host-cloudflare/src/db/postgres.ts | 93 ++++++++++++++ .../src/mcp/session-durable-object.ts | 9 +- apps/host-cloudflare/wrangler.jsonc | 19 +++ bun.lock | 3 + 13 files changed, 509 insertions(+), 10 deletions(-) create mode 100644 apps/host-cloudflare/scripts/dev-db.ts create mode 100644 apps/host-cloudflare/src/db/index.test.ts create mode 100644 apps/host-cloudflare/src/db/index.ts create mode 100644 apps/host-cloudflare/src/db/postgres.test.ts create mode 100644 apps/host-cloudflare/src/db/postgres.ts diff --git a/apps/host-cloudflare/README.md b/apps/host-cloudflare/README.md index 1770dbbe8..1d4a54779 100644 --- a/apps/host-cloudflare/README.md +++ b/apps/host-cloudflare/README.md @@ -7,7 +7,7 @@ paths, different injected providers: | Seam | Cloudflare provider | | ------------ | ---------------------------------------------------------------- | | **identity** | Cloudflare Access JWT (`Cf-Access-Jwt-Assertion`) — no app login | -| **db** | D1 (SQLite) via the shared FumaDB assembly | +| **db** | D1 (SQLite) by default; Neon/Postgres via Hyperdrive (opt-in) | | **engine** | QuickJS-WASM, in-Worker (no extra binding) | | **mcp** | Access-JWT auth + the shared in-process session store | | **account** | `/account/me` from the Access principal (members/keys → Access) | @@ -77,6 +77,39 @@ running `wrangler dev` if you need live data. `ENABLE_DEV_AUTH` is a dev-only escape hatch — never set it in a deployed environment (it disables the Access gate). +## Neon Postgres (opt-in; D1 is the default) + +D1 (SQLite) is the default db seam — zero external dependencies, auto-provisioned +by `wrangler deploy`, co-located with the Worker. That is the right default for +this single-tenant template. Swap to Neon Postgres over Cloudflare Hyperdrive +when you need real interactive transactions, no bound-parameter cap, large +values without the ~1-2MB D1 ceiling, or to scale past D1's 10GB limit. The +Worker auto-selects Postgres when a Hyperdrive binding is present — no code +change (see `src/db/index.ts`). + +```bash +# 1. Create a Neon project (https://neon.tech) and copy its connection string. +# 2. Create a Hyperdrive config pointing at it: +bunx wrangler hyperdrive create executor-pg \ + --connection-string="postgresql://USER:PASS@EP.neon.tech/neondb?sslmode=require" +# 3. In wrangler.jsonc, uncomment the "hyperdrive" block and set "id" to the id +# printed above. (Leave the d1_databases block; it just goes unused.) +# 4. Deploy. The schema is provisioned automatically on first boot (runtime +# ensure, same code path as D1), so there is no separate migration step. +bun run deploy +``` + +Local dev against Postgres (PGlite stands in for Neon, no Docker): + +```bash +bun run dev:db # PGlite over a socket at postgresql://…@127.0.0.1:5433/postgres +bun run dev # wrangler dev reads HYPERDRIVE.localConnectionString +``` + +The R2 `BLOBS` bucket stays useful under Postgres (large blob offload) and is +optional either way. Migrating existing data from a live D1 deployment to +Postgres is out of scope — a fresh Postgres database starts empty. + ## Notes - The QuickJS engine WASM is vendored into `src/quickjs-engine.wasm` (Workers diff --git a/apps/host-cloudflare/package.json b/apps/host-cloudflare/package.json index 9ece894d1..72f0e848f 100644 --- a/apps/host-cloudflare/package.json +++ b/apps/host-cloudflare/package.json @@ -7,6 +7,7 @@ "deploy": "vite build && wrangler deploy", "dev": "wrangler dev", "dev:web": "vite dev", + "dev:db": "bun run scripts/dev-db.ts", "typecheck": "tsgo --noEmit", "cf-typegen": "wrangler types", "deploy:setup": "bash scripts/deploy.sh", @@ -38,6 +39,7 @@ "drizzle-orm": "catalog:", "effect": "catalog:", "jose": "^5.9.6", + "postgres": "^3.4.9", "quickjs-emscripten-core": "0.31.0", "react": "catalog:", "react-dom": "catalog:" @@ -45,6 +47,8 @@ "devDependencies": { "@cloudflare/workers-types": "^4.20250410.0", "@effect/vitest": "catalog:", + "@electric-sql/pglite": "^0.4.4", + "@electric-sql/pglite-socket": "^0.1.4", "@executor-js/vite-plugin": "workspace:*", "@tailwindcss/vite": "catalog:", "@tanstack/router-plugin": "^1.167.12", diff --git a/apps/host-cloudflare/scripts/dev-db.ts b/apps/host-cloudflare/scripts/dev-db.ts new file mode 100644 index 000000000..cce7878d3 --- /dev/null +++ b/apps/host-cloudflare/scripts/dev-db.ts @@ -0,0 +1,91 @@ +// --------------------------------------------------------------------------- +// Local dev Postgres via PGlite — no Docker, no install +// --------------------------------------------------------------------------- +// +// Exposes an in-process PGlite instance over a TCP socket so Hyperdrive's +// `localConnectionString` can connect to it like a real Postgres server, for +// exercising the opt-in Postgres seam (src/db/postgres.ts) under `wrangler dev`. +// +// Unlike apps/cloud (which applies generated drizzle-kit migrations), the +// Cloudflare host brings its schema up at RUNTIME via +// `ensureDrizzleRuntimeSchemaFromTables`, so this script does the same here — +// the dev DB and the deployed Worker share one schema bring-up code path. + +import { execSync } from "node:child_process"; +import { setTimeout as sleep } from "node:timers/promises"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { PGlite } from "@electric-sql/pglite"; +import { PGLiteSocketServer } from "@electric-sql/pglite-socket"; +import { drizzle } from "drizzle-orm/pglite"; +import { + createDrizzleRuntimeSchemaFromTables, + ensureDrizzleRuntimeSchemaFromTables, +} from "@executor-js/fumadb/adapters/drizzle"; +import { collectTables } from "@executor-js/api/server"; + +import { CLOUDFLARE_NAMESPACE, CLOUDFLARE_SCHEMA_VERSION } from "../src/config"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +// Port + data dir default to the dev values but are env-overridable so a second +// throwaway instance can run alongside `bun dev`. +const PORT = Number(process.env.DEV_DB_PORT ?? 5433); +const DB_PATH = process.env.DEV_DB_PATH + ? resolve(process.env.DEV_DB_PATH) + : resolve(__dirname, "../.dev-db"); + +// Reap any orphan dev-db from a previous run that didn't shut down cleanly — +// otherwise the new instance can't bind to PORT. +function reapStaleDevDb() { + const out = execSync(`lsof -ti tcp:${PORT} -sTCP:LISTEN 2>/dev/null || true`, { + encoding: "utf8", + }); + const pids = out.trim().split("\n").filter(Boolean); + if (pids.length === 0) return false; + + for (const pid of pids) { + const cmd = execSync(`ps -p ${pid} -o args= 2>/dev/null || true`, { + encoding: "utf8", + }).trim(); + if (!cmd.includes("dev-db.ts")) { + console.error(`[dev-db] Port ${PORT} is held by an unexpected process (pid ${pid}): ${cmd}`); + console.error(`[dev-db] Refusing to kill it. Free the port and retry.`); + process.exit(1); + } + console.log(`[dev-db] Reaping stale dev-db (pid ${pid})`); + execSync(`kill -KILL ${pid}`); + } + return true; +} + +if (reapStaleDevDb()) { + // Give the kernel a beat to release the socket before we try to bind. + await sleep(200); +} + +console.log(`[dev-db] Starting PGlite at ${DB_PATH}`); +const db = await PGlite.create(DB_PATH); + +console.log("[dev-db] Ensuring runtime schema (provider=postgresql)"); +const options = { + tables: collectTables(), + namespace: CLOUDFLARE_NAMESPACE, + version: CLOUDFLARE_SCHEMA_VERSION, + provider: "postgresql" as const, +}; +const schema = createDrizzleRuntimeSchemaFromTables(options); +await ensureDrizzleRuntimeSchemaFromTables(drizzle(db, { schema }), options); + +const server = new PGLiteSocketServer({ db, port: PORT, host: "127.0.0.1" }); +await server.start(); +console.log(`[dev-db] Listening on postgresql://postgres:postgres@127.0.0.1:${PORT}/postgres`); + +const shutdown = async () => { + console.log("\n[dev-db] Shutting down"); + await server.stop(); + await db.close(); + process.exit(0); +}; + +process.on("SIGINT", shutdown); +process.on("SIGTERM", shutdown); diff --git a/apps/host-cloudflare/src/app.ts b/apps/host-cloudflare/src/app.ts index 9c284a48a..fab3c27b4 100644 --- a/apps/host-cloudflare/src/app.ts +++ b/apps/host-cloudflare/src/app.ts @@ -5,7 +5,7 @@ import { dbProviderLayer, ExecutorApp, textFailureStrategy } from "@executor-js/ import { loadConfig, type CloudflareEnv } from "./config"; import { makeCloudflarePlugins } from "./plugins"; -import { createD1ExecutorDb } from "./db/d1"; +import { createExecutorDb } from "./db"; import { cloudflareAccessIdentityLayer } from "./auth/cloudflare-access"; import { CloudflareCodeExecutorProvider, @@ -40,9 +40,11 @@ export const makeCloudflareApp = async (env: CloudflareEnv) => { // executor is built — the default variant can't fetch its .wasm on Workers. await preloadQuickJs(); - // Open + idempotently bring up the D1 schema once (the long-lived handle the - // per-request scoped executor reads through the DbProvider seam). - const dbHandle = await createD1ExecutorDb(env.DB, env.BLOBS); + // Open + idempotently bring up the schema once (the long-lived handle the + // per-request scoped executor reads through the DbProvider seam). The seam is + // D1 by default; a Hyperdrive binding / DATABASE_URL switches it to Postgres + // (see ./db/index.ts). + const dbHandle = await createExecutorDb(env); const identityLayer = cloudflareAccessIdentityLayer(config); // MCP runs through the `MCP_SESSION` Durable Object (cross-isolate sessions); // each session DO opens its own D1 handle, so it takes `env`, not `dbHandle`. diff --git a/apps/host-cloudflare/src/config.ts b/apps/host-cloudflare/src/config.ts index 140d904f8..00d46a7d4 100644 --- a/apps/host-cloudflare/src/config.ts +++ b/apps/host-cloudflare/src/config.ts @@ -1,4 +1,9 @@ -import type { D1Database, DurableObjectNamespace, R2Bucket } from "@cloudflare/workers-types"; +import type { + D1Database, + DurableObjectNamespace, + Hyperdrive, + R2Bucket, +} from "@cloudflare/workers-types"; import { isValidOrgSlug } from "@executor-js/api"; import { missingPublicOriginWarning, resolvePublicOrigin } from "@executor-js/sdk/public-origin"; @@ -21,6 +26,18 @@ export interface CloudflareEnv { readonly DB: D1Database; /** R2 bucket binding — holds values too large for a D1 row (~1-2MB cap). */ readonly BLOBS?: R2Bucket; + /** Optional Hyperdrive binding for Neon/Postgres. When present the db seam + * switches from D1 to Postgres-over-Hyperdrive (see `src/db/index.ts`). + * Provision with `wrangler hyperdrive create` and add the binding to + * `wrangler.jsonc` — absent by default, so D1 stays the zero-setup default. */ + readonly HYPERDRIVE?: Hyperdrive; + /** Direct Postgres connection string. Only used when + * `EXECUTOR_DIRECT_DATABASE_URL === "true"` (keeps Hyperdrive the default + * path); also serves as the fallback when a Hyperdrive binding is absent. */ + readonly DATABASE_URL?: string; + /** Set to "true" to connect to `DATABASE_URL` directly instead of through + * Hyperdrive. For local dev / admin scripts only — mirrors `apps/cloud`. */ + readonly EXECUTOR_DIRECT_DATABASE_URL?: string; /** MCP session Durable Object namespace — one addressable isolate per MCP * session (the DO id IS the session id), so a session survives across the * Worker's stateless isolates. */ diff --git a/apps/host-cloudflare/src/db/data-migrations.ts b/apps/host-cloudflare/src/db/data-migrations.ts index 45d87a88b..15bd1bfa0 100644 --- a/apps/host-cloudflare/src/db/data-migrations.ts +++ b/apps/host-cloudflare/src/db/data-migrations.ts @@ -1,4 +1,5 @@ import { Effect } from "effect"; +import type { Sql } from "postgres"; import type { D1Database, D1DatabaseSession, R2Bucket } from "@cloudflare/workers-types"; import { @@ -88,3 +89,15 @@ export const runCloudflareDataMigrations = ( Effect.runPromise( runSqliteDataMigrations(d1DataMigrationClient(db), cloudflareDataMigrations(bucket)), ); + +// Postgres (Neon) path — see ./postgres.ts. A fresh Postgres deployment has no +// legacy D1 data, so there is nothing to migrate. The existing data migration +// (the Google OpenAPI ownership move + its R2 blob copy) is SQLite-specific +// (`json_extract`/`json_type`, D1 sessions) and only relevant to deployments +// that previously ran on D1. Moving existing data D1 -> Postgres is out of +// scope (a manual export/import); if a Postgres-native data migration is ever +// needed, add a Postgres ledger here modelled on apps/cloud's code migrations. +export const runCloudflarePostgresDataMigrations = async ( + _sql: Sql, + _bucket: R2Bucket | undefined, +): Promise => []; diff --git a/apps/host-cloudflare/src/db/index.test.ts b/apps/host-cloudflare/src/db/index.test.ts new file mode 100644 index 000000000..72e34c9ce --- /dev/null +++ b/apps/host-cloudflare/src/db/index.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "@effect/vitest"; +import type { Hyperdrive } from "@cloudflare/workers-types"; + +import type { CloudflareEnv } from "../config"; +import { isPostgresConfigured } from "./index"; +import { resolveConnectionString } from "./postgres"; + +// The base env carries only what the db selector reads; the rest of +// CloudflareEnv is irrelevant to these pure decisions, so cast a minimal shape. +const env = (overrides: Partial): CloudflareEnv => + // oxlint-disable-next-line executor/no-double-cast -- test: minimal env shape; only the db-selector fields are read + overrides as unknown as CloudflareEnv; + +const fakeHyperdrive = (connectionString: string): Hyperdrive => + // oxlint-disable-next-line executor/no-double-cast -- test: only Hyperdrive.connectionString is read by the selector + ({ connectionString }) as unknown as Hyperdrive; + +const NEON = "postgresql://user:pass@ep-x.neon.tech/neondb?sslmode=require"; +const DIRECT = "postgresql://postgres:postgres@127.0.0.1:5433/postgres"; + +describe("isPostgresConfigured", () => { + it("is false with no Postgres credentials (D1 default)", () => { + expect(isPostgresConfigured(env({}))).toBe(false); + }); + + it("is true when a Hyperdrive binding is present", () => { + expect(isPostgresConfigured(env({ HYPERDRIVE: fakeHyperdrive(NEON) }))).toBe(true); + }); + + it("is true when DATABASE_URL + EXECUTOR_DIRECT_DATABASE_URL=true", () => { + expect( + isPostgresConfigured(env({ DATABASE_URL: DIRECT, EXECUTOR_DIRECT_DATABASE_URL: "true" })), + ).toBe(true); + }); + + it("is false when DATABASE_URL is set without the direct escape hatch", () => { + // A bare DATABASE_URL must NOT silently flip the seam to Postgres — the + // operator has to opt in explicitly (HYPERDRIVE binding, or the flag). + expect(isPostgresConfigured(env({ DATABASE_URL: DIRECT }))).toBe(false); + }); +}); + +describe("resolveConnectionString", () => { + it("prefers the Hyperdrive connection string", () => { + expect( + resolveConnectionString(env({ HYPERDRIVE: fakeHyperdrive(NEON), DATABASE_URL: DIRECT })), + ).toBe(NEON); + }); + + it("uses DATABASE_URL directly only behind EXECUTOR_DIRECT_DATABASE_URL=true", () => { + expect( + resolveConnectionString( + env({ + HYPERDRIVE: fakeHyperdrive(NEON), + DATABASE_URL: DIRECT, + EXECUTOR_DIRECT_DATABASE_URL: "true", + }), + ), + ).toBe(DIRECT); + }); + + it("falls back to DATABASE_URL when no Hyperdrive binding exists", () => { + expect(resolveConnectionString(env({ DATABASE_URL: DIRECT }))).toBe(DIRECT); + }); + + it("returns empty string when nothing is configured (the fallback-to-D1 signal)", () => { + expect(resolveConnectionString(env({}))).toBe(""); + }); +}); diff --git a/apps/host-cloudflare/src/db/index.ts b/apps/host-cloudflare/src/db/index.ts new file mode 100644 index 000000000..1cddcca7e --- /dev/null +++ b/apps/host-cloudflare/src/db/index.ts @@ -0,0 +1,38 @@ +import type { ExecutorDbHandle } from "@executor-js/api/server"; + +import type { CloudflareEnv } from "../config"; +import { createD1ExecutorDb } from "./d1"; +import { createPostgresExecutorDb, resolveConnectionString } from "./postgres"; + +// --------------------------------------------------------------------------- +// DB seam selector. D1 (SQLite) is the default — the template's +// zero-external-dependency single-Worker premise. The Postgres seam +// (Neon via Hyperdrive, see ./postgres.ts) activates ONLY when the operator has +// wired up credentials: a Hyperdrive binding, or a direct DATABASE_URL behind +// EXECUTOR_DIRECT_DATABASE_URL=true. Presence of credentials IS the signal — no +// separate provider flag — so doing nothing keeps D1. +// --------------------------------------------------------------------------- + +export const isPostgresConfigured = (env: CloudflareEnv): boolean => { + if (env.HYPERDRIVE) return true; + if (env.EXECUTOR_DIRECT_DATABASE_URL === "true" && env.DATABASE_URL) return true; + return false; +}; + +export const createExecutorDb = async (env: CloudflareEnv): Promise => { + if (isPostgresConfigured(env)) { + // Guard against a partial Postgres config (e.g. DATABASE_URL set but neither + // a Hyperdrive binding nor EXECUTOR_DIRECT_DATABASE_URL=true): rather than + // let postgres.js fail with an opaque connect error, warn loudly and fall + // back to D1 so a misconfigured deploy is obvious but still boots. + if (resolveConnectionString(env)) { + return createPostgresExecutorDb(env); + } + console.error( + "[executor-cloudflare] Postgres looked configured but no usable connection string was found " + + "(set EXECUTOR_DIRECT_DATABASE_URL=true to use DATABASE_URL directly, or bind a Hyperdrive " + + "config in wrangler.jsonc). Falling back to D1.", + ); + } + return createD1ExecutorDb(env.DB, env.BLOBS); +}; diff --git a/apps/host-cloudflare/src/db/postgres.test.ts b/apps/host-cloudflare/src/db/postgres.test.ts new file mode 100644 index 000000000..998642097 --- /dev/null +++ b/apps/host-cloudflare/src/db/postgres.test.ts @@ -0,0 +1,116 @@ +// --------------------------------------------------------------------------- +// Postgres seam integration test +// --------------------------------------------------------------------------- +// +// Exercises createPostgresExecutorDb end-to-end against a real Postgres wire +// protocol: an in-process PGlite exposed over a TCP socket (the same approach +// apps/cloud's test harness uses), reached through postgres.js exactly as the +// deployed Worker reaches Neon through Hyperdrive. This proves: +// - the runtime `ensure` bring-up works under provider "postgresql" +// - a write/read round-trip works through the FumaDB orm +// - REAL interactive transactions work (the D1 `interactiveTransactions: +// false` workaround is gone): a committed tx persists both rows, a failed +// tx rolls BOTH back — atomicity D1 could not give. +// +// PGlite is in-process (no Docker / external DB), so this runs unconditionally. + +import { afterAll, beforeAll, describe, expect, it } from "@effect/vitest"; +import { PGlite } from "@electric-sql/pglite"; +import { PGLiteSocketServer } from "@electric-sql/pglite-socket"; +import { withQueryContext } from "@executor-js/fumadb/query"; + +import type { ExecutorDbHandle } from "@executor-js/api/server"; + +import type { CloudflareEnv } from "../config"; +import { createPostgresExecutorDb } from "./postgres"; + +const PORT = Number(process.env.HOST_CF_TEST_DB_PORT ?? 5437); + +let pglite: PGlite; +let server: PGLiteSocketServer; +let handle: ExecutorDbHandle; +// The executor tables carry a tenant policy (see core-schema.ts); the scoped +// executor binds the tenant context via withQueryContext, so the test does the +// same to write through the orm. +let db: ExecutorDbHandle["db"]; + +const integration = (slug: string) => ({ + tenant: "org_1", + slug, + plugin_id: "openapi", + name: null, + description: `desc-${slug}`, + config: { kind: "test" }, + can_remove: true, + can_refresh: false, + created_at: new Date(), + updated_at: new Date(), +}); + +beforeAll(async () => { + pglite = await PGlite.create(); + server = new PGLiteSocketServer({ db: pglite, port: PORT, host: "127.0.0.1" }); + await server.start(); + + // oxlint-disable-next-line executor/no-double-cast -- test: only the db-connection fields are read by createPostgresExecutorDb + const env = { + DATABASE_URL: `postgresql://postgres:postgres@127.0.0.1:${PORT}/postgres`, + EXECUTOR_DIRECT_DATABASE_URL: "true", + } as unknown as CloudflareEnv; + handle = await createPostgresExecutorDb(env); + db = withQueryContext(handle.db, { tenant: "org_1", subject: undefined }); +}); + +afterAll(async () => { + await handle?.close(); + await server?.stop(); + await pglite?.close(); +}); + +describe("createPostgresExecutorDb", () => { + it("brings up the schema and round-trips a row through the orm", async () => { + const created = await db.create("integration", integration("round-trip")); + expect(created.slug).toBe("round-trip"); + + const found = await db.findFirst("integration", { + where: (b) => b("slug", "=", "round-trip"), + }); + expect(found?.plugin_id).toBe("openapi"); + }); + + it("commits a real interactive transaction (no D1 auto-commit workaround)", async () => { + await db.transaction(async (tx) => { + await tx.create("integration", integration("tx-a")); + await tx.create("integration", integration("tx-b")); + }); + + const a = await db.findFirst("integration", { + where: (b) => b("slug", "=", "tx-a"), + }); + const b = await db.findFirst("integration", { + where: (b) => b("slug", "=", "tx-b"), + }); + expect(a).not.toBeNull(); + expect(b).not.toBeNull(); + }); + + it("rolls back BOTH writes when a transaction throws (atomicity)", async () => { + await expect( + db.transaction(async (tx) => { + await tx.create("integration", integration("rollback-1")); + await tx.create("integration", integration("rollback-2")); + // oxlint-disable-next-line executor/no-try-catch-or-throw, executor/no-error-constructor -- test: the tx callback must throw to exercise transaction rollback + throw new Error("boom"); + }), + ).rejects.toThrow("boom"); + + const one = await db.findFirst("integration", { + where: (b) => b("slug", "=", "rollback-1"), + }); + const two = await db.findFirst("integration", { + where: (b) => b("slug", "=", "rollback-2"), + }); + expect(one).toBeNull(); + expect(two).toBeNull(); + }); +}); diff --git a/apps/host-cloudflare/src/db/postgres.ts b/apps/host-cloudflare/src/db/postgres.ts new file mode 100644 index 000000000..0cb665793 --- /dev/null +++ b/apps/host-cloudflare/src/db/postgres.ts @@ -0,0 +1,93 @@ +import { drizzle } from "drizzle-orm/postgres-js"; +import postgres from "postgres"; +import { + createDrizzleRuntimeSchemaFromTables, + ensureDrizzleRuntimeSchemaFromTables, +} from "@executor-js/fumadb/adapters/drizzle"; + +import { + collectTables, + createExecutorFumaDb, + type ExecutorDbHandle, +} from "@executor-js/api/server"; +import { makeR2BlobStore } from "@executor-js/cloudflare/blob-store"; + +import { CLOUDFLARE_NAMESPACE, CLOUDFLARE_SCHEMA_VERSION, type CloudflareEnv } from "../config"; +import { runCloudflarePostgresDataMigrations } from "./data-migrations"; + +// --------------------------------------------------------------------------- +// Postgres (Neon via Hyperdrive) DbProvider handle — the opt-in alternative to +// the D1 seam (see ./d1.ts). Selected by ./index.ts when a Hyperdrive binding +// or a direct DATABASE_URL is present; D1 stays the default. +// +// This mirrors the proven postgres.js + Hyperdrive path apps/cloud already runs +// (apps/cloud/src/db/db.ts) but keeps host-cloudflare's RUNTIME `ensure` +// bring-up (no out-of-band migration step), so the template stays +// self-provisioning. Unlike D1, Postgres supports transactional DDL and has no +// 100-bound-parameter cap, so the two D1 workarounds +// (`interactiveTransactions: false`, `maxBoundParameters: 100`) are dropped and +// the schema bring-up runs with the FULL drizzle handle (DDL wrapped in a tx). +// --------------------------------------------------------------------------- + +// Resolve the Postgres connection string, mirroring apps/cloud/src/db/db.ts. +// Production prefers the Hyperdrive binding's connection string; a direct +// DATABASE_URL is used only behind the explicit EXECUTOR_DIRECT_DATABASE_URL +// escape hatch (so a stray secret can't silently bypass Hyperdrive), and is the +// final fallback when no Hyperdrive binding exists. +export const resolveConnectionString = (env: CloudflareEnv): string => { + if (env.EXECUTOR_DIRECT_DATABASE_URL === "true" && env.DATABASE_URL) { + return env.DATABASE_URL; + } + return env.HYPERDRIVE?.connectionString || env.DATABASE_URL || ""; +}; + +export const createPostgresExecutorDb = async (env: CloudflareEnv): Promise => { + // postgres.js (not `pg`): Workers forbid sharing I/O across requests, and + // `pg`'s CloudflareSocket hangs when reused. `max: 1` is right for Hyperdrive + // (it pools behind the proxy); options match apps/cloud/src/db/db.ts. No + // explicit `ssl` — Hyperdrive terminates TLS to the backend. + const sql = postgres(resolveConnectionString(env), { + max: 1, + idle_timeout: 0, + max_lifetime: 60, + connect_timeout: 10, + fetch_types: false, + prepare: true, + onnotice: () => undefined, + }); + + const options = { + tables: collectTables(), + namespace: CLOUDFLARE_NAMESPACE, + version: CLOUDFLARE_SCHEMA_VERSION, + provider: "postgresql" as const, + }; + + const schema = createDrizzleRuntimeSchemaFromTables(options); + const drizzleDb = drizzle(sql, { schema }); + + // Pass the FULL drizzle handle (not D1's run-only view): Postgres supports + // transactional DDL, so the idempotent `CREATE TABLE IF NOT EXISTS` bring-up + // runs atomically. Postgres has no legacy D1 data, so the data-migration is a + // no-op (see ./data-migrations.ts). + await ensureDrizzleRuntimeSchemaFromTables(drizzleDb, options); + await runCloudflarePostgresDataMigrations(sql, env.BLOBS); + + const { db: fumaDb, fuma } = createExecutorFumaDb(drizzleDb, options); + + return { + db: fumaDb, + fuma, + // Fire-and-forget, like apps/cloud: the Worker/DO request context ends + // before sql.end() resolves. In the Worker happy path this is never called + // (isolate eviction is Cloudflare-managed); in the MCP DO it ends the + // per-session connection at session teardown. + close: async () => { + void sql.end({ timeout: 0 }); + }, + // R2 stays useful for multi-MB blobs even under Postgres (TOAST handles + // large values, but the R2 seam keeps them out of table rows). Falls back + // to the FumaDB `blob` table when no bucket is bound. + blobs: env.BLOBS ? makeR2BlobStore(env.BLOBS) : undefined, + }; +}; diff --git a/apps/host-cloudflare/src/mcp/session-durable-object.ts b/apps/host-cloudflare/src/mcp/session-durable-object.ts index 26caac4df..495715829 100644 --- a/apps/host-cloudflare/src/mcp/session-durable-object.ts +++ b/apps/host-cloudflare/src/mcp/session-durable-object.ts @@ -11,7 +11,7 @@ import { } from "@executor-js/cloudflare/mcp/durable-object"; import { loadConfig, type CloudflareConfig, type CloudflareEnv } from "../config"; -import { createD1ExecutorDb } from "../db/d1"; +import { createExecutorDb } from "../db"; import { makeCloudflareExecutionStackLayer, makeExecutionStack } from "../execution"; import { preloadQuickJs } from "../quickjs"; @@ -31,8 +31,9 @@ import { preloadQuickJs } from "../quickjs"; // was invisible to the next; the DO id == session id routes them all back). // --------------------------------------------------------------------------- -// The long-lived D1 handle, adapted to the base's `end` contract. D1 owns its -// own lifecycle (the binding is the connection), so `end` is `close` — a no-op. +// The long-lived db handle, adapted to the base's `end` contract. `end` +// delegates to the handle's `close`: for D1 that's a no-op (the binding owns its +// lifecycle); for Postgres it ends the per-session connection (sql.end()). type CfSessionDbHandle = ExecutorDbHandle & { readonly end: () => Promise }; export class McpSessionDO extends McpSessionDOBase { @@ -49,7 +50,7 @@ export class McpSessionDO extends McpSessionDOBase { } protected override async openSessionDb(): Promise { - const handle = await createD1ExecutorDb(this.cfEnv.DB, this.cfEnv.BLOBS); + const handle = await createExecutorDb(this.cfEnv); return { ...handle, end: () => handle.close() }; } diff --git a/apps/host-cloudflare/wrangler.jsonc b/apps/host-cloudflare/wrangler.jsonc index aec20421f..00497e3be 100644 --- a/apps/host-cloudflare/wrangler.jsonc +++ b/apps/host-cloudflare/wrangler.jsonc @@ -33,6 +33,25 @@ "bucket_name": "executor-blobs", }, ], + // --------------------------------------------------------------------------- + // OPTIONAL: Neon Postgres via Hyperdrive (opt-in; D1 above is the default). + // The Worker auto-selects Postgres when a HYPERDRIVE binding is present (see + // src/db/index.ts). To enable: + // 1. Create a Neon project (https://neon.tech) and copy its connection string. + // 2. wrangler hyperdrive create executor-pg \ + // --connection-string="postgresql://USER:PASS@EP.neon.tech/neondb?sslmode=require" + // 3. Uncomment the block below and set "id" to the Hyperdrive id from step 2. + // 4. `bun run deploy`. The schema is provisioned automatically on first boot. + // For local dev, run `bun run dev:db` (PGlite on :5433) alongside `bun run dev`; + // the localConnectionString below points wrangler at it. + // --------------------------------------------------------------------------- + // "hyperdrive": [ + // { + // "binding": "HYPERDRIVE", + // "id": "", + // "localConnectionString": "postgresql://postgres:postgres@127.0.0.1:5433/postgres", + // }, + // ], // The MCP session Durable Object: one addressable isolate per MCP session (the // DO id IS the session id) so a session survives across the Worker's stateless // isolates — without it, `tools/list` after `initialize` can land on a fresh diff --git a/bun.lock b/bun.lock index 1074dcb03..60464a038 100644 --- a/bun.lock +++ b/bun.lock @@ -189,6 +189,7 @@ "drizzle-orm": "catalog:", "effect": "catalog:", "jose": "^5.9.6", + "postgres": "^3.4.9", "quickjs-emscripten-core": "0.31.0", "react": "catalog:", "react-dom": "catalog:", @@ -196,6 +197,8 @@ "devDependencies": { "@cloudflare/workers-types": "^4.20250410.0", "@effect/vitest": "catalog:", + "@electric-sql/pglite": "^0.4.4", + "@electric-sql/pglite-socket": "^0.1.4", "@executor-js/vite-plugin": "workspace:*", "@tailwindcss/vite": "catalog:", "@tanstack/router-plugin": "^1.167.12", From 8ec707e5e4f30d1bf0ba8038617ddee065e2c72d Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 25 Jun 2026 22:35:18 +0900 Subject: [PATCH 2/4] fix(host-cloudflare): request-scope the Postgres connection (Cloudflare I/O isolation) + workerd e2e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HTTP db seam shared one postgres.js connection across requests (memoized in the per-isolate app), which Cloudflare Workers forbid ("Cannot perform I/O on behalf of a different request") and which also closed the socket after the first request. Mirror apps/cloud: the connection lives in a request-scoped service (CfPgConnection, provided via ExecutorApp.make's `requestScoped`) so its acquire/release spans the whole request fiber, and the DbProvider assembles the FumaDB handle over it with a no-op close. The schema is brought up once at boot (ensurePostgresSchema), never per request. The MCP Durable Object keeps one long-lived connection per session (matching cloud's longer idle/lifetime tuning); its handle's `close` is neutralized in openSessionDb so the per-engine-build scope can't end the session connection early (the DO base disposes it via `end()`). Adds worker-postgres.e2e.node.test.ts: boots the real worker on workerd (wrangler unstable_dev) against a PGlite socket and drives db writes/reads across SEPARATE requests + an MCP tools/call — exactly the cross-request path a node-only test cannot exercise. This reproduces (and now guards against) the CONNECTION_ENDED / I/O-isolation failure. Refs #10 --- apps/host-cloudflare/.gitignore | 1 + apps/host-cloudflare/src/app.ts | 75 +++++--- apps/host-cloudflare/src/db/index.ts | 65 +++++-- apps/host-cloudflare/src/db/postgres.ts | 171 +++++++++++++----- .../src/mcp/session-durable-object.ts | 8 +- .../src/worker-postgres.e2e.node.test.ts | 167 +++++++++++++++++ 6 files changed, 397 insertions(+), 90 deletions(-) create mode 100644 apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts diff --git a/apps/host-cloudflare/.gitignore b/apps/host-cloudflare/.gitignore index 726b7f10f..6ebef4d24 100644 --- a/apps/host-cloudflare/.gitignore +++ b/apps/host-cloudflare/.gitignore @@ -1,4 +1,5 @@ dist/ +.dev-db/ .dev.vars .wrangler/ wrangler.preview.jsonc diff --git a/apps/host-cloudflare/src/app.ts b/apps/host-cloudflare/src/app.ts index fab3c27b4..9f7b82e27 100644 --- a/apps/host-cloudflare/src/app.ts +++ b/apps/host-cloudflare/src/app.ts @@ -1,11 +1,10 @@ -import { Effect } from "effect"; import { HttpEffect, HttpRouter } from "effect/unstable/http"; -import { dbProviderLayer, ExecutorApp, textFailureStrategy } from "@executor-js/api/server"; +import { ExecutorApp, textFailureStrategy } from "@executor-js/api/server"; import { loadConfig, type CloudflareEnv } from "./config"; import { makeCloudflarePlugins } from "./plugins"; -import { createExecutorDb } from "./db"; +import { selectDbSeam } from "./db"; import { cloudflareAccessIdentityLayer } from "./auth/cloudflare-access"; import { CloudflareCodeExecutorProvider, @@ -40,35 +39,36 @@ export const makeCloudflareApp = async (env: CloudflareEnv) => { // executor is built — the default variant can't fetch its .wasm on Workers. await preloadQuickJs(); - // Open + idempotently bring up the schema once (the long-lived handle the - // per-request scoped executor reads through the DbProvider seam). The seam is - // D1 by default; a Hyperdrive binding / DATABASE_URL switches it to Postgres - // (see ./db/index.ts). - const dbHandle = await createExecutorDb(env); + // The db seam: D1 by default (one memoized handle); a Hyperdrive binding / + // DATABASE_URL switches it to Postgres, where the schema is brought up once + // here and each request gets a fresh connection in its own fiber scope via + // `requestScoped` (Cloudflare forbids sharing a socket across requests). + const seam = await selectDbSeam(env); const identityLayer = cloudflareAccessIdentityLayer(config); // MCP runs through the `MCP_SESSION` Durable Object (cross-isolate sessions); - // each session DO opens its own D1 handle, so it takes `env`, not `dbHandle`. + // each session DO opens its own db handle, so it takes `env`, not a handle. const mcp = makeCloudflareMcpSeams(config, env); - const { appLayer, toWebHandler } = ExecutorApp.make({ - plugins, - providers: { - identity: identityLayer, - db: dbProviderLayer(Effect.succeed(dbHandle)), - engine: { codeExecutor: CloudflareCodeExecutorProvider }, // decorator defaults to no-op - plugins: { - provider: makeCloudflarePluginsProvider(config), - config: makeCloudflareHostConfig(config), - }, - errorCapture: ErrorCaptureLive, - // The account API (`/api/account/*`) backs the shared multiplayer shell's - // auth context; `me` reflects the Access principal. Members/keys are - // Access-managed, so the rest of the surface is stubbed. - account: cloudflareAccountMiddleware(config), - // The MCP serving envelope: Access-JWT auth + the shared in-process session - // store over the QuickJS engine. - mcp: { auth: mcp.auth, sessions: mcp.sessions, reporter: mcp.reporter }, + // Everything but the db provider + (Postgres-only) request scope is identical + // across the two seams. + const commonProviders = { + identity: identityLayer, + engine: { codeExecutor: CloudflareCodeExecutorProvider }, // decorator defaults to no-op + plugins: { + provider: makeCloudflarePluginsProvider(config), + config: makeCloudflareHostConfig(config), }, + errorCapture: ErrorCaptureLive, + // The account API (`/api/account/*`) backs the shared multiplayer shell's + // auth context; `me` reflects the Access principal. Members/keys are + // Access-managed, so the rest of the surface is stubbed. + account: cloudflareAccountMiddleware(config), + // The MCP serving envelope: Access-JWT auth + the shared in-process session + // store over the QuickJS engine. + mcp: { auth: mcp.auth, sessions: mcp.sessions, reporter: mcp.reporter }, + }; + const common = { + plugins, extensions: { routes: [ // Browser approval of paused MCP executions: the console resume page @@ -77,9 +77,26 @@ export const makeCloudflareApp = async (env: CloudflareEnv) => { HttpRouter.add("*", "/api/mcp-sessions/*", HttpEffect.fromWebHandler(mcp.approvalHandler)), ], }, - config: { mountPrefix: "/api", failure: textFailureStrategy }, + config: { mountPrefix: "/api" as const, failure: textFailureStrategy }, boot: identityLayer, - }); + }; + + // Postgres provides its per-request connection through `requestScoped` so the + // socket's acquire/release spans the whole request fiber; D1 needs no request + // scope. Two `make` calls (rather than a conditional field) keep the residual + // types exact: Postgres's `db` carries a `CfPgConnection` requirement that + // only `requestScoped` satisfies. + const { appLayer, toWebHandler } = + seam.kind === "postgres" + ? ExecutorApp.make({ + ...common, + providers: { ...commonProviders, db: seam.db }, + requestScoped: seam.requestScoped, + }) + : ExecutorApp.make({ + ...common, + providers: { ...commonProviders, db: seam.db }, + }); return { appLayer, toWebHandler }; }; diff --git a/apps/host-cloudflare/src/db/index.ts b/apps/host-cloudflare/src/db/index.ts index 1cddcca7e..cc8805724 100644 --- a/apps/host-cloudflare/src/db/index.ts +++ b/apps/host-cloudflare/src/db/index.ts @@ -1,8 +1,17 @@ -import type { ExecutorDbHandle } from "@executor-js/api/server"; +import { Effect, Layer } from "effect"; + +import { DbProvider, dbProviderLayer, type ExecutorDbHandle } from "@executor-js/api/server"; import type { CloudflareEnv } from "../config"; import { createD1ExecutorDb } from "./d1"; -import { createPostgresExecutorDb, resolveConnectionString } from "./postgres"; +import { + CfPgConnection, + createPostgresExecutorDb, + ensurePostgresSchema, + makeCfPgConnectionLayer, + postgresDbProviderLayer, + resolveConnectionString, +} from "./postgres"; // --------------------------------------------------------------------------- // DB seam selector. D1 (SQLite) is the default — the template's @@ -19,20 +28,48 @@ export const isPostgresConfigured = (env: CloudflareEnv): boolean => { return false; }; +const warnPostgresMisconfigured = (): void => { + console.error( + "[executor-cloudflare] Postgres looked configured but no usable connection string was found " + + "(set EXECUTOR_DIRECT_DATABASE_URL=true to use DATABASE_URL directly, or bind a Hyperdrive " + + "config in wrangler.jsonc). Falling back to D1.", + ); +}; + +// Single handle for the MCP Durable Object (one long-lived connection held for +// the session). The HTTP path uses selectDbSeam instead. export const createExecutorDb = async (env: CloudflareEnv): Promise => { if (isPostgresConfigured(env)) { - // Guard against a partial Postgres config (e.g. DATABASE_URL set but neither - // a Hyperdrive binding nor EXECUTOR_DIRECT_DATABASE_URL=true): rather than - // let postgres.js fail with an opaque connect error, warn loudly and fall - // back to D1 so a misconfigured deploy is obvious but still boots. - if (resolveConnectionString(env)) { - return createPostgresExecutorDb(env); - } - console.error( - "[executor-cloudflare] Postgres looked configured but no usable connection string was found " + - "(set EXECUTOR_DIRECT_DATABASE_URL=true to use DATABASE_URL directly, or bind a Hyperdrive " + - "config in wrangler.jsonc). Falling back to D1.", - ); + if (resolveConnectionString(env)) return createPostgresExecutorDb(env); + warnPostgresMisconfigured(); } return createD1ExecutorDb(env.DB, env.BLOBS); }; + +// The HTTP db wiring, as a discriminated union the app composes into +// ExecutorApp.make. Postgres carries a `requestScoped` connection layer (the +// socket lives in the request fiber's scope — Cloudflare forbids sharing it +// across requests); the DbProvider reads it with a no-op close. D1 has no +// request scope (the binding is not a socket) and reuses one memoized handle. +export type CloudflareDbSeam = + | { readonly kind: "d1"; readonly db: Layer.Layer } + | { + readonly kind: "postgres"; + readonly db: Layer.Layer; + readonly requestScoped: Layer.Layer; + }; + +export const selectDbSeam = async (env: CloudflareEnv): Promise => { + if (isPostgresConfigured(env) && resolveConnectionString(env)) { + // Bring the schema up ONCE per isolate; per-request connections never DDL. + await ensurePostgresSchema(env); + return { + kind: "postgres", + db: postgresDbProviderLayer(env), + requestScoped: makeCfPgConnectionLayer(env), + }; + } + if (isPostgresConfigured(env)) warnPostgresMisconfigured(); + const handle = await createD1ExecutorDb(env.DB, env.BLOBS); + return { kind: "d1", db: dbProviderLayer(Effect.succeed(handle)) }; +}; diff --git a/apps/host-cloudflare/src/db/postgres.ts b/apps/host-cloudflare/src/db/postgres.ts index 0cb665793..3c8d49027 100644 --- a/apps/host-cloudflare/src/db/postgres.ts +++ b/apps/host-cloudflare/src/db/postgres.ts @@ -1,5 +1,6 @@ +import { Context, Effect, Layer } from "effect"; import { drizzle } from "drizzle-orm/postgres-js"; -import postgres from "postgres"; +import postgres, { type Sql } from "postgres"; import { createDrizzleRuntimeSchemaFromTables, ensureDrizzleRuntimeSchemaFromTables, @@ -8,6 +9,7 @@ import { import { collectTables, createExecutorFumaDb, + DbProvider, type ExecutorDbHandle, } from "@executor-js/api/server"; import { makeR2BlobStore } from "@executor-js/cloudflare/blob-store"; @@ -16,19 +18,49 @@ import { CLOUDFLARE_NAMESPACE, CLOUDFLARE_SCHEMA_VERSION, type CloudflareEnv } f import { runCloudflarePostgresDataMigrations } from "./data-migrations"; // --------------------------------------------------------------------------- -// Postgres (Neon via Hyperdrive) DbProvider handle — the opt-in alternative to -// the D1 seam (see ./d1.ts). Selected by ./index.ts when a Hyperdrive binding -// or a direct DATABASE_URL is present; D1 stays the default. +// Postgres (Neon via Hyperdrive) DbProvider — the opt-in alternative to the D1 +// seam (see ./d1.ts). Selected by ./index.ts when a Hyperdrive binding or a +// direct DATABASE_URL is present; D1 stays the default. // -// This mirrors the proven postgres.js + Hyperdrive path apps/cloud already runs -// (apps/cloud/src/db/db.ts) but keeps host-cloudflare's RUNTIME `ensure` -// bring-up (no out-of-band migration step), so the template stays -// self-provisioning. Unlike D1, Postgres supports transactional DDL and has no -// 100-bound-parameter cap, so the two D1 workarounds -// (`interactiveTransactions: false`, `maxBoundParameters: 100`) are dropped and -// the schema bring-up runs with the FULL drizzle handle (DDL wrapped in a tx). +// Modelled on the proven postgres.js + Hyperdrive path apps/cloud runs +// (apps/cloud/src/db/db.ts + fuma.ts + mcp/session-durable-object.ts). The +// critical constraint is that Cloudflare Workers forbid reusing an I/O object +// across request handlers, so the two host execution models differ: +// +// - HTTP (stateless Worker): the connection lives in a REQUEST-SCOPED service +// (`CfPgConnection`, acquired/released per request via `requestScoped` in +// ExecutorApp.make). The DbProvider just assembles the FumaDB handle over +// it and its `close` is a NO-OP — releasing the socket is the request +// scope's job, NOT the executor-build scope's (which closes before the +// handler runs). The schema is brought up ONCE at boot +// (`ensurePostgresSchema`), never per request. +// - MCP Durable Object (one persistent isolate per session): ONE long-lived +// connection held for the session (`createPostgresExecutorDb`); the DO base +// disposes it via its `end()` contract. +// +// host-cloudflare keeps RUNTIME `ensure` (no out-of-band migration step). Unlike +// D1, Postgres has transactional DDL and no 100-bound-parameter cap, so the D1 +// workarounds (`interactiveTransactions: false`, `maxBoundParameters: 100`) are +// dropped and the bring-up runs with the FULL drizzle handle (DDL in a tx). // --------------------------------------------------------------------------- +// postgres.js options. Shared base; the two lifetimes differ only in +// socket-hygiene timeouts, matching apps/cloud (db.ts vs mcp/session-do.ts). +const BASE_OPTS = { + max: 1, + connect_timeout: 10, + fetch_types: false, + prepare: true, + onnotice: () => undefined, +} as const; +// Per-request (HTTP): no idle reaping; the request scope closes it explicitly. +const EPHEMERAL_OPTS = { idle_timeout: 0, max_lifetime: 60 } as const; +// Per-session (DO): hold the socket across the session's requests, recycling it +// on a short idle/lifetime so an idle session doesn't pin a backend connection. +const LONG_LIVED_OPTS = { idle_timeout: 5, max_lifetime: 120 } as const; + +type Lifetime = "ephemeral" | "long-lived"; + // Resolve the Postgres connection string, mirroring apps/cloud/src/db/db.ts. // Production prefers the Hyperdrive binding's connection string; a direct // DATABASE_URL is used only behind the explicit EXECUTOR_DIRECT_DATABASE_URL @@ -41,53 +73,100 @@ export const resolveConnectionString = (env: CloudflareEnv): string => { return env.HYPERDRIVE?.connectionString || env.DATABASE_URL || ""; }; -export const createPostgresExecutorDb = async (env: CloudflareEnv): Promise => { - // postgres.js (not `pg`): Workers forbid sharing I/O across requests, and - // `pg`'s CloudflareSocket hangs when reused. `max: 1` is right for Hyperdrive - // (it pools behind the proxy); options match apps/cloud/src/db/db.ts. No - // explicit `ssl` — Hyperdrive terminates TLS to the backend. - const sql = postgres(resolveConnectionString(env), { - max: 1, - idle_timeout: 0, - max_lifetime: 60, - connect_timeout: 10, - fetch_types: false, - prepare: true, - onnotice: () => undefined, +const makeSql = (env: CloudflareEnv, lifetime: Lifetime): Sql => + postgres(resolveConnectionString(env), { + ...BASE_OPTS, + ...(lifetime === "ephemeral" ? EPHEMERAL_OPTS : LONG_LIVED_OPTS), }); - const options = { - tables: collectTables(), - namespace: CLOUDFLARE_NAMESPACE, - version: CLOUDFLARE_SCHEMA_VERSION, - provider: "postgresql" as const, - }; +// The runtime Drizzle schema is pure and env-independent (derived from the fixed +// executor table set), so build it once and reuse it across every connection. +const options = { + tables: collectTables(), + namespace: CLOUDFLARE_NAMESPACE, + version: CLOUDFLARE_SCHEMA_VERSION, + provider: "postgresql" as const, +}; +let cachedSchema: Record | undefined; +const runtimeSchema = (): Record => + (cachedSchema ??= createDrizzleRuntimeSchemaFromTables(options)); + +const blobStore = (env: CloudflareEnv) => (env.BLOBS ? makeR2BlobStore(env.BLOBS) : undefined); - const schema = createDrizzleRuntimeSchemaFromTables(options); - const drizzleDb = drizzle(sql, { schema }); +// Boot-once schema bring-up on a short-lived connection that is closed +// immediately. The HTTP path runs this once per isolate (see ./index.ts) so the +// per-request connections never run DDL. +export const ensurePostgresSchema = async (env: CloudflareEnv): Promise => { + const sql = makeSql(env, "ephemeral"); + const drizzleDb = drizzle(sql, { schema: runtimeSchema() }); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: release the boot connection even if DDL throws + try { + // Full drizzle handle (not D1's run-only view): Postgres supports + // transactional DDL, so the idempotent `CREATE TABLE IF NOT EXISTS` bring-up + // runs atomically. Postgres has no legacy D1 data, so the data-migration is + // a no-op (see ./data-migrations.ts). + await ensureDrizzleRuntimeSchemaFromTables(drizzleDb, options); + await runCloudflarePostgresDataMigrations(sql, env.BLOBS); + } finally { + void sql.end({ timeout: 0 }); + } +}; - // Pass the FULL drizzle handle (not D1's run-only view): Postgres supports - // transactional DDL, so the idempotent `CREATE TABLE IF NOT EXISTS` bring-up - // runs atomically. Postgres has no legacy D1 data, so the data-migration is a - // no-op (see ./data-migrations.ts). +// ---- HTTP path: request-scoped connection + no-op-close DbProvider ---------- + +interface CfPgConnectionShape { + readonly sql: Sql; + readonly db: ReturnType; +} + +// The per-request postgres socket, mirroring apps/cloud's `DbService`. Provided +// via `requestScoped` so its acquire/release spans the whole request fiber +// (Cloudflare I/O isolation), not the executor-build scope. +export class CfPgConnection extends Context.Service()( + "@executor-js/host-cloudflare/CfPgConnection", +) {} + +export const makeCfPgConnectionLayer = (env: CloudflareEnv): Layer.Layer => + Layer.effect(CfPgConnection)( + Effect.acquireRelease( + Effect.sync((): CfPgConnectionShape => { + const sql = makeSql(env, "ephemeral"); + return { sql, db: drizzle(sql, { schema: runtimeSchema() }) }; + }), + ({ sql }) => + // oxlint-disable-next-line executor/no-promise-catch -- boundary: postgres.js close is best-effort at request-scope end + Effect.promise(() => sql.end({ timeout: 0 }).catch(() => undefined)), + ), + ); + +// Assemble the FumaDB handle over the request-scoped connection. `close` is a +// no-op: CfPgConnection (request scope) owns the socket lifecycle. +export const postgresDbProviderLayer = ( + env: CloudflareEnv, +): Layer.Layer => + Layer.effect(DbProvider)( + Effect.map(CfPgConnection.asEffect(), ({ db }): ExecutorDbHandle => { + const { db: fumaDb, fuma } = createExecutorFumaDb(db, options); + return { db: fumaDb, fuma, close: async () => {}, blobs: blobStore(env) }; + }), + ); + +// ---- MCP Durable Object path: one long-lived connection per session --------- + +// One long-lived connection for the session, with the schema brought up on it +// (idempotent, once per session DO). The DO base manages teardown via `end()`. +export const createPostgresExecutorDb = async (env: CloudflareEnv): Promise => { + const sql = makeSql(env, "long-lived"); + const drizzleDb = drizzle(sql, { schema: runtimeSchema() }); await ensureDrizzleRuntimeSchemaFromTables(drizzleDb, options); await runCloudflarePostgresDataMigrations(sql, env.BLOBS); - const { db: fumaDb, fuma } = createExecutorFumaDb(drizzleDb, options); - return { db: fumaDb, fuma, - // Fire-and-forget, like apps/cloud: the Worker/DO request context ends - // before sql.end() resolves. In the Worker happy path this is never called - // (isolate eviction is Cloudflare-managed); in the MCP DO it ends the - // per-session connection at session teardown. close: async () => { void sql.end({ timeout: 0 }); }, - // R2 stays useful for multi-MB blobs even under Postgres (TOAST handles - // large values, but the R2 seam keeps them out of table rows). Falls back - // to the FumaDB `blob` table when no bucket is bound. - blobs: env.BLOBS ? makeR2BlobStore(env.BLOBS) : undefined, + blobs: blobStore(env), }; }; diff --git a/apps/host-cloudflare/src/mcp/session-durable-object.ts b/apps/host-cloudflare/src/mcp/session-durable-object.ts index 495715829..1dd2abf63 100644 --- a/apps/host-cloudflare/src/mcp/session-durable-object.ts +++ b/apps/host-cloudflare/src/mcp/session-durable-object.ts @@ -51,7 +51,13 @@ export class McpSessionDO extends McpSessionDOBase { protected override async openSessionDb(): Promise { const handle = await createExecutorDb(this.cfEnv); - return { ...handle, end: () => handle.close() }; + // The base owns this connection's lifecycle for the whole session and + // disposes it via `end()` at runtime teardown. Neutralize the handle's own + // `close` so the per-engine-build DbProvider scope (built once when the MCP + // server is created) does NOT end the session connection early — for D1 + // `close` was already a no-op, but the Postgres connection must survive the + // build scope and live for the session. `end` runs the real teardown. + return { ...handle, close: async () => {}, end: () => handle.close() }; } protected override resolveSessionMeta(token: McpSessionInit): Effect.Effect { diff --git a/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts b/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts new file mode 100644 index 000000000..8333a3780 --- /dev/null +++ b/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts @@ -0,0 +1,167 @@ +import { randomUUID } from "node:crypto"; +import { existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { afterAll, beforeAll, describe, expect, it } from "@effect/vitest"; +import { PGlite } from "@electric-sql/pglite"; +import { PGLiteSocketServer } from "@electric-sql/pglite-socket"; +import { unstable_dev, type Unstable_DevWorker } from "wrangler"; + +// --------------------------------------------------------------------------- +// End-to-end test for the Cloudflare host's POSTGRES seam on the REAL workerd +// runtime (wrangler `unstable_dev`/Miniflare). A local PGlite stands in for Neon +// over a TCP socket; the worker is booted with DATABASE_URL + +// EXECUTOR_DIRECT_DATABASE_URL=true so the db seam (selectDbSeam / createExecutorDb) +// takes the Postgres branch (no Hyperdrive binding needed for the direct path). +// +// The point this test pins that a node-only test CANNOT: Cloudflare Workers +// forbid reusing an I/O object across request handlers. The HTTP db seam opens a +// FRESH postgres connection per request scope; if it instead shared one +// connection across requests (the pre-fix bug), the SECOND db-touching request +// would fail with "Cannot perform I/O on behalf of a different request". So the +// assertions deliberately span multiple sequential requests. +// --------------------------------------------------------------------------- + +const dir = fileURLToPath(new URL(".", import.meta.url)); +const runId = randomUUID().slice(0, 8); +const PORT = Number(process.env.HOST_CF_E2E_DB_PORT ?? 5438); + +const SPEC = JSON.stringify({ + openapi: "3.0.0", + info: { title: "Test", version: "1.0.0" }, + servers: [{ url: "https://example.com" }], + paths: { + "/ping": { get: { operationId: "ping", responses: { "200": { description: "ok" } } } }, + }, +}); + +describe("cloudflare host POSTGRES e2e (workerd/miniflare + PGlite)", () => { + let pglite: PGlite; + let server: PGLiteSocketServer; + let worker: Unstable_DevWorker; + + beforeAll(async () => { + pglite = await PGlite.create(); + server = new PGLiteSocketServer({ db: pglite, port: PORT, host: "127.0.0.1" }); + await server.start(); + + // wrangler's assets validation needs ./dist/index.html present even though + // this test only drives run_worker_first API paths (mirrors the D1 e2e). + const distIndex = resolve(dir, "../dist/index.html"); + if (!existsSync(distIndex)) { + mkdirSync(resolve(dir, "../dist"), { recursive: true }); + writeFileSync(distIndex, "executor"); + } + + worker = await unstable_dev(resolve(dir, "worker.ts"), { + config: resolve(dir, "../wrangler.jsonc"), + ip: "127.0.0.1", + local: true, + experimental: { disableExperimentalWarning: true }, + vars: { + EXECUTOR_SECRET_KEY: "test-secret-key-0123456789abcdef", + ENABLE_DEV_AUTH: "true", + // The direct-connection escape hatch: take the Postgres branch without a + // Hyperdrive binding, pointing at the local PGlite socket. + DATABASE_URL: `postgresql://postgres:postgres@127.0.0.1:${PORT}/postgres`, + EXECUTOR_DIRECT_DATABASE_URL: "true", + }, + }); + }, 120_000); + + afterAll(async () => { + await worker?.stop(); + await server?.stop(); + await pglite?.close(); + }); + + it("adds an OpenAPI source then reads it back across SEPARATE requests (per-request Postgres connection)", async () => { + const slug = `pgapi-${runId}`; + // Request 1: a db write — opens a fresh connection, writes, releases it. + const add = await worker.fetch("/api/openapi/specs", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + spec: { kind: "blob", value: SPEC }, + slug, + description: "PG API", + baseUrl: "https://example.com", + }), + }); + expect(add.status).toBe(200); + const added = (await add.json()) as { toolCount: number }; + expect(added.toolCount).toBeGreaterThan(0); + + // Request 2: a db read on a NEW request. With a shared cross-request + // connection this is exactly where workerd throws; with a per-request + // connection it just works. + const got = await worker.fetch(`/api/openapi/integrations/${slug}`); + expect(got.status).toBe(200); + const integration = (await got.json()) as { slug: string } | null; + expect(integration?.slug).toBe(slug); + }, 90_000); + + it("survives many sequential db-touching requests (no I/O-context reuse error)", async () => { + // Several independent requests in a row, each must acquire+release its own + // connection. A shared connection would fail on the 2nd here. + for (let i = 0; i < 3; i++) { + const slug = `pgloop-${runId}-${i}`; + const add = await worker.fetch("/api/openapi/specs", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + spec: { kind: "blob", value: SPEC }, + slug, + description: `loop ${i}`, + baseUrl: "https://example.com", + }), + }); + expect(add.status).toBe(200); + } + }, 90_000); + + it("invokes the execute tool over MCP against Postgres (DO long-lived connection)", async () => { + const accept = "application/json, text/event-stream"; + const rpc = (sessionId: string | null, body: unknown) => + worker.fetch("/mcp", { + method: "POST", + headers: { + "content-type": "application/json", + accept, + ...(sessionId ? { "mcp-session-id": sessionId } : {}), + }, + body: JSON.stringify(body), + }); + + const init = await rpc(null, { + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "t", version: "1" }, + }, + }); + expect(init.status).toBe(200); + const sessionId = init.headers.get("mcp-session-id"); + expect(sessionId).toBeTruthy(); + + await rpc(sessionId, { jsonrpc: "2.0", method: "notifications/initialized" }); + + // tools/call on a follow-up request: the DO holds ONE long-lived Postgres + // connection for the session across these separate requests. + const call = await rpc(sessionId, { + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { name: "execute", arguments: { code: "export default 6 * 7" } }, + }); + expect(call.status).toBe(200); + const result = (await call.json()) as { + result?: { structuredContent?: { result?: number } }; + }; + expect(result.result?.structuredContent?.result).toBe(42); + }, 90_000); +}); From c69d503af29dc4b415601ab11a10b495dbaafe32 Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Thu, 25 Jun 2026 23:07:38 +0900 Subject: [PATCH 3/4] chore(host-cloudflare): address review (remove em-dashes, pin misconfig fallback) Code review findings on PR #14: - Remove em-dashes from comments/docs added by this PR (AGENTS.md: never use em-dashes in code comments or docs), replacing them with colons/commas/ parentheses. Pre-existing em-dashes in untouched files are left as-is. - Add a unit test pinning the partial-misconfig guard: a HYPERDRIVE binding with an empty connectionString is not-usable, so selectDbSeam/createExecutorDb fall back to D1. --- apps/host-cloudflare/README.md | 6 +++--- apps/host-cloudflare/scripts/dev-db.ts | 6 +++--- apps/host-cloudflare/src/config.ts | 4 ++-- apps/host-cloudflare/src/db/data-migrations.ts | 2 +- apps/host-cloudflare/src/db/index.test.ts | 18 +++++++++++++++++- apps/host-cloudflare/src/db/index.ts | 8 ++++---- apps/host-cloudflare/src/db/postgres.test.ts | 2 +- apps/host-cloudflare/src/db/postgres.ts | 4 ++-- .../src/mcp/session-durable-object.ts | 2 +- .../src/worker-postgres.e2e.node.test.ts | 2 +- 10 files changed, 35 insertions(+), 19 deletions(-) diff --git a/apps/host-cloudflare/README.md b/apps/host-cloudflare/README.md index 1d4a54779..491814663 100644 --- a/apps/host-cloudflare/README.md +++ b/apps/host-cloudflare/README.md @@ -79,12 +79,12 @@ environment (it disables the Access gate). ## Neon Postgres (opt-in; D1 is the default) -D1 (SQLite) is the default db seam — zero external dependencies, auto-provisioned +D1 (SQLite) is the default db seam: zero external dependencies, auto-provisioned by `wrangler deploy`, co-located with the Worker. That is the right default for this single-tenant template. Swap to Neon Postgres over Cloudflare Hyperdrive when you need real interactive transactions, no bound-parameter cap, large values without the ~1-2MB D1 ceiling, or to scale past D1's 10GB limit. The -Worker auto-selects Postgres when a Hyperdrive binding is present — no code +Worker auto-selects Postgres when a Hyperdrive binding is present, with no code change (see `src/db/index.ts`). ```bash @@ -108,7 +108,7 @@ bun run dev # wrangler dev reads HYPERDRIVE.localConnectionString The R2 `BLOBS` bucket stays useful under Postgres (large blob offload) and is optional either way. Migrating existing data from a live D1 deployment to -Postgres is out of scope — a fresh Postgres database starts empty. +Postgres is out of scope: a fresh Postgres database starts empty. ## Notes diff --git a/apps/host-cloudflare/scripts/dev-db.ts b/apps/host-cloudflare/scripts/dev-db.ts index cce7878d3..df428bc12 100644 --- a/apps/host-cloudflare/scripts/dev-db.ts +++ b/apps/host-cloudflare/scripts/dev-db.ts @@ -1,5 +1,5 @@ // --------------------------------------------------------------------------- -// Local dev Postgres via PGlite — no Docker, no install +// Local dev Postgres via PGlite (no Docker, no install) // --------------------------------------------------------------------------- // // Exposes an in-process PGlite instance over a TCP socket so Hyperdrive's @@ -8,7 +8,7 @@ // // Unlike apps/cloud (which applies generated drizzle-kit migrations), the // Cloudflare host brings its schema up at RUNTIME via -// `ensureDrizzleRuntimeSchemaFromTables`, so this script does the same here — +// `ensureDrizzleRuntimeSchemaFromTables`, so this script does the same here: // the dev DB and the deployed Worker share one schema bring-up code path. import { execSync } from "node:child_process"; @@ -34,7 +34,7 @@ const DB_PATH = process.env.DEV_DB_PATH ? resolve(process.env.DEV_DB_PATH) : resolve(__dirname, "../.dev-db"); -// Reap any orphan dev-db from a previous run that didn't shut down cleanly — +// Reap any orphan dev-db from a previous run that didn't shut down cleanly, // otherwise the new instance can't bind to PORT. function reapStaleDevDb() { const out = execSync(`lsof -ti tcp:${PORT} -sTCP:LISTEN 2>/dev/null || true`, { diff --git a/apps/host-cloudflare/src/config.ts b/apps/host-cloudflare/src/config.ts index 00d46a7d4..92506802c 100644 --- a/apps/host-cloudflare/src/config.ts +++ b/apps/host-cloudflare/src/config.ts @@ -29,14 +29,14 @@ export interface CloudflareEnv { /** Optional Hyperdrive binding for Neon/Postgres. When present the db seam * switches from D1 to Postgres-over-Hyperdrive (see `src/db/index.ts`). * Provision with `wrangler hyperdrive create` and add the binding to - * `wrangler.jsonc` — absent by default, so D1 stays the zero-setup default. */ + * `wrangler.jsonc`. Absent by default, so D1 stays the zero-setup default. */ readonly HYPERDRIVE?: Hyperdrive; /** Direct Postgres connection string. Only used when * `EXECUTOR_DIRECT_DATABASE_URL === "true"` (keeps Hyperdrive the default * path); also serves as the fallback when a Hyperdrive binding is absent. */ readonly DATABASE_URL?: string; /** Set to "true" to connect to `DATABASE_URL` directly instead of through - * Hyperdrive. For local dev / admin scripts only — mirrors `apps/cloud`. */ + * Hyperdrive. For local dev / admin scripts only; mirrors `apps/cloud`. */ readonly EXECUTOR_DIRECT_DATABASE_URL?: string; /** MCP session Durable Object namespace — one addressable isolate per MCP * session (the DO id IS the session id), so a session survives across the diff --git a/apps/host-cloudflare/src/db/data-migrations.ts b/apps/host-cloudflare/src/db/data-migrations.ts index 15bd1bfa0..bddde80ac 100644 --- a/apps/host-cloudflare/src/db/data-migrations.ts +++ b/apps/host-cloudflare/src/db/data-migrations.ts @@ -90,7 +90,7 @@ export const runCloudflareDataMigrations = ( runSqliteDataMigrations(d1DataMigrationClient(db), cloudflareDataMigrations(bucket)), ); -// Postgres (Neon) path — see ./postgres.ts. A fresh Postgres deployment has no +// Postgres (Neon) path, see ./postgres.ts. A fresh Postgres deployment has no // legacy D1 data, so there is nothing to migrate. The existing data migration // (the Google OpenAPI ownership move + its R2 blob copy) is SQLite-specific // (`json_extract`/`json_type`, D1 sessions) and only relevant to deployments diff --git a/apps/host-cloudflare/src/db/index.test.ts b/apps/host-cloudflare/src/db/index.test.ts index 72e34c9ce..08afc41a4 100644 --- a/apps/host-cloudflare/src/db/index.test.ts +++ b/apps/host-cloudflare/src/db/index.test.ts @@ -34,12 +34,28 @@ describe("isPostgresConfigured", () => { }); it("is false when DATABASE_URL is set without the direct escape hatch", () => { - // A bare DATABASE_URL must NOT silently flip the seam to Postgres — the + // A bare DATABASE_URL must NOT silently flip the seam to Postgres; the // operator has to opt in explicitly (HYPERDRIVE binding, or the flag). expect(isPostgresConfigured(env({ DATABASE_URL: DIRECT }))).toBe(false); }); }); +describe("partial-misconfig guard (selectDbSeam / createExecutorDb fall back to D1)", () => { + // selectDbSeam and createExecutorDb both gate the Postgres branch on + // `isPostgresConfigured(env) && resolveConnectionString(env)`. The only state + // where the first is true but the second is empty is a HYPERDRIVE binding + // whose connectionString is empty: then the guard is falsy and the seam + // falls back to D1 (with a warning). Pin that combined contract here without + // booting a real D1 handle. + it("treats an empty-connectionString Hyperdrive binding as not-usable -> D1", () => { + const e = env({ HYPERDRIVE: fakeHyperdrive("") }); + expect(isPostgresConfigured(e)).toBe(true); + expect(resolveConnectionString(e)).toBe(""); + // The guard selectDbSeam/createExecutorDb apply: + expect(isPostgresConfigured(e) && resolveConnectionString(e) !== "").toBe(false); + }); +}); + describe("resolveConnectionString", () => { it("prefers the Hyperdrive connection string", () => { expect( diff --git a/apps/host-cloudflare/src/db/index.ts b/apps/host-cloudflare/src/db/index.ts index cc8805724..693b5ddaa 100644 --- a/apps/host-cloudflare/src/db/index.ts +++ b/apps/host-cloudflare/src/db/index.ts @@ -14,12 +14,12 @@ import { } from "./postgres"; // --------------------------------------------------------------------------- -// DB seam selector. D1 (SQLite) is the default — the template's +// DB seam selector. D1 (SQLite) is the default: the template's // zero-external-dependency single-Worker premise. The Postgres seam // (Neon via Hyperdrive, see ./postgres.ts) activates ONLY when the operator has // wired up credentials: a Hyperdrive binding, or a direct DATABASE_URL behind -// EXECUTOR_DIRECT_DATABASE_URL=true. Presence of credentials IS the signal — no -// separate provider flag — so doing nothing keeps D1. +// EXECUTOR_DIRECT_DATABASE_URL=true. Presence of credentials IS the signal, no +// separate provider flag, so doing nothing keeps D1. // --------------------------------------------------------------------------- export const isPostgresConfigured = (env: CloudflareEnv): boolean => { @@ -48,7 +48,7 @@ export const createExecutorDb = async (env: CloudflareEnv): Promise { // The base owns this connection's lifecycle for the whole session and // disposes it via `end()` at runtime teardown. Neutralize the handle's own // `close` so the per-engine-build DbProvider scope (built once when the MCP - // server is created) does NOT end the session connection early — for D1 + // server is created) does NOT end the session connection early. For D1 // `close` was already a no-op, but the Postgres connection must survive the // build scope and live for the session. `end` runs the real teardown. return { ...handle, close: async () => {}, end: () => handle.close() }; diff --git a/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts b/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts index 8333a3780..6dbea185f 100644 --- a/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts +++ b/apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts @@ -78,7 +78,7 @@ describe("cloudflare host POSTGRES e2e (workerd/miniflare + PGlite)", () => { it("adds an OpenAPI source then reads it back across SEPARATE requests (per-request Postgres connection)", async () => { const slug = `pgapi-${runId}`; - // Request 1: a db write — opens a fresh connection, writes, releases it. + // Request 1: a db write that opens a fresh connection, writes, releases it. const add = await worker.fetch("/api/openapi/specs", { method: "POST", headers: { "content-type": "application/json" }, From c3c8f567283a48ed28c1645b0c4a040fd6deedba Mon Sep 17 00:00:00 2001 From: Minsu Lee Date: Fri, 26 Jun 2026 00:08:19 +0900 Subject: [PATCH 4/4] chore(host-cloudflare): apply AI code review suggestions Address bot review findings on PR #14: - worker.ts: clear the memoized handlerPromise on boot failure so a transient Postgres/Hyperdrive outage during schema bring-up does not permanently poison the isolate; the next request reattempts boot instead of replaying the rejection. (greptile/cubic P1) - postgres.ts createPostgresExecutorDb: release the long-lived session socket if schema bring-up throws, preventing a connection leak on Neon when the DB is momentarily unreachable at session start. (greptile/cubic) - postgres.ts: await sql.end(...).catch(() => undefined) in ensurePostgresSchema finally and in the DO session close (the DO base's end() delegates here, so it should await real teardown) instead of fire-and-forget. (gemini/coderabbit/cubic) - dev-db.ts: use process.kill instead of a kill subshell, and skip a PID that exited between lsof and ps rather than misreporting it as an unexpected process. (gemini/cubic) --- apps/host-cloudflare/scripts/dev-db.ts | 12 +++++++++++- apps/host-cloudflare/src/db/postgres.ts | 23 +++++++++++++++++++---- apps/host-cloudflare/src/worker.ts | 13 +++++++++++-- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/apps/host-cloudflare/scripts/dev-db.ts b/apps/host-cloudflare/scripts/dev-db.ts index df428bc12..0230a30c3 100644 --- a/apps/host-cloudflare/scripts/dev-db.ts +++ b/apps/host-cloudflare/scripts/dev-db.ts @@ -47,13 +47,23 @@ function reapStaleDevDb() { const cmd = execSync(`ps -p ${pid} -o args= 2>/dev/null || true`, { encoding: "utf8", }).trim(); + // The PID can exit between `lsof` and `ps` (the stale instance shutting down + // on its own): an empty cmd means it is already gone and the port is free, so + // skip it rather than misreading it as an unexpected process and bailing. + if (cmd === "") continue; if (!cmd.includes("dev-db.ts")) { console.error(`[dev-db] Port ${PORT} is held by an unexpected process (pid ${pid}): ${cmd}`); console.error(`[dev-db] Refusing to kill it. Free the port and retry.`); process.exit(1); } console.log(`[dev-db] Reaping stale dev-db (pid ${pid})`); - execSync(`kill -KILL ${pid}`); + // process.kill (a Node built-in, no subshell) instead of `kill -KILL`; + // tolerate ESRCH if the process exited between the check above and here. + try { + process.kill(Number(pid), "SIGKILL"); + } catch { + // already gone + } } return true; } diff --git a/apps/host-cloudflare/src/db/postgres.ts b/apps/host-cloudflare/src/db/postgres.ts index 6903c9100..2fc61d13d 100644 --- a/apps/host-cloudflare/src/db/postgres.ts +++ b/apps/host-cloudflare/src/db/postgres.ts @@ -108,7 +108,11 @@ export const ensurePostgresSchema = async (env: CloudflareEnv): Promise => await ensureDrizzleRuntimeSchemaFromTables(drizzleDb, options); await runCloudflarePostgresDataMigrations(sql, env.BLOBS); } finally { - void sql.end({ timeout: 0 }); + // Await the boot connection's close (best-effort) so it is released before + // per-request connections open; .catch keeps a close failure from masking a + // DDL error thrown from the try. + // oxlint-disable-next-line executor/no-promise-catch -- boundary: postgres.js close is best-effort + await sql.end({ timeout: 0 }).catch(() => undefined); } }; @@ -158,14 +162,25 @@ export const postgresDbProviderLayer = ( export const createPostgresExecutorDb = async (env: CloudflareEnv): Promise => { const sql = makeSql(env, "long-lived"); const drizzleDb = drizzle(sql, { schema: runtimeSchema() }); - await ensureDrizzleRuntimeSchemaFromTables(drizzleDb, options); - await runCloudflarePostgresDataMigrations(sql, env.BLOBS); + // oxlint-disable executor/no-try-catch-or-throw -- boundary: release the session connection if schema bring-up throws; a momentary DB outage at session start would otherwise leak the long-lived socket (idle_timeout 5s) on Neon's backend + try { + await ensureDrizzleRuntimeSchemaFromTables(drizzleDb, options); + await runCloudflarePostgresDataMigrations(sql, env.BLOBS); + } catch (err) { + // oxlint-disable-next-line executor/no-promise-catch -- boundary: best-effort release on the failure path + await sql.end({ timeout: 0 }).catch(() => undefined); + throw err; + } + // oxlint-enable executor/no-try-catch-or-throw const { db: fumaDb, fuma } = createExecutorFumaDb(drizzleDb, options); return { db: fumaDb, fuma, + // The DO base's end() delegates to this close for real session teardown, so + // await the socket close (best-effort) rather than fire-and-forget. close: async () => { - void sql.end({ timeout: 0 }); + // oxlint-disable-next-line executor/no-promise-catch -- boundary: postgres.js close is best-effort at session teardown + await sql.end({ timeout: 0 }).catch(() => undefined); }, blobs: blobStore(env), }; diff --git a/apps/host-cloudflare/src/worker.ts b/apps/host-cloudflare/src/worker.ts index 228b60a55..5a740aeac 100644 --- a/apps/host-cloudflare/src/worker.ts +++ b/apps/host-cloudflare/src/worker.ts @@ -15,11 +15,20 @@ export { McpSessionDO } from "./mcp"; let handlerPromise: Promise<(request: Request) => Promise> | null = null; -const resolveHandler = (env: CloudflareEnv): Promise<(request: Request) => Promise> => { +const resolveHandler = async ( + env: CloudflareEnv, +): Promise<(request: Request) => Promise> => { if (!handlerPromise) { handlerPromise = makeCloudflareApp(env).then(({ toWebHandler }) => toWebHandler().handler); } - return handlerPromise; + // oxlint-disable executor/no-try-catch-or-throw -- boundary: a boot failure (e.g. Postgres unreachable during schema bring-up) must not permanently poison the isolate; the memoized promise would replay the same rejection for every later request. Clear the memo on failure so the next request reattempts boot. D1 boots never hit this (a built-in binding does not fail); the Postgres path makes it a real network failure mode. + try { + return await handlerPromise; + } catch (err) { + handlerPromise = null; + throw err; + } + // oxlint-enable executor/no-try-catch-or-throw }; export default {