feat(host-cloudflare): configurable db seam — D1 default, Neon/Postgres via Hyperdrive opt-in#14
Conversation
…stgres 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
…re I/O isolation) + workerd e2e
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
WalkthroughCloudflare host 앱이 D1 기본, Hyperdrive/Postgres 옵트인으로 전환되고, 로컬 PGlite 개발 DB와 Postgres 연결/스키마 bring-up이 추가됐다. MCP 세션 수명과 워커 E2E 테스트도 새 Postgres 경로에 맞게 갱신됐다. ChangesHost Cloudflare Postgres opt-in
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces an opt-in Neon Postgres database seam over Cloudflare Hyperdrive as an alternative to the default D1 (SQLite) database, including a local development setup using PGlite and comprehensive integration tests. The review feedback highlights three medium-severity issues: recommending the use of Node.js's native process.kill instead of execSync to terminate stale local database processes, and advising to await the sql.end() calls in both ensurePostgresSchema and the database close method to ensure proper connection teardown and prevent resource leaks in the Cloudflare Workers environment.
Greptile SummaryThis PR makes the database seam of
Confidence Score: 5/5Safe to merge. All three issues raised in the previous review round have been addressed, the D1 default path is unchanged and still covered by its existing e2e, and the new Postgres path is covered by both a unit-level integration test and a full workerd e2e that explicitly reproduces the cross-request I/O isolation failure the design is meant to prevent. The boot-failure poison fix in worker.ts is correct and non-disruptive for D1 deployments. Connection lifecycle in postgres.ts is now consistent across both execution models: the DO path properly awaits sql.end in close, and schema bring-up failure cleans up the socket before re-throwing. The request-scoped connection model is validated end-to-end on the real workerd runtime. No files require special attention. The most structurally complex new file is src/db/postgres.ts, which has thorough inline commentary and is directly exercised by both postgres.test.ts and worker-postgres.e2e.node.test.ts. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Worker fetch] --> B{handlerPromise set?}
B -- No --> C[makeCloudflareApp]
B -- Yes --> D[await handlerPromise]
C --> E[selectDbSeam]
E --> F{isPostgresConfigured?}
F -- No --> G[D1 seam]
F -- Yes --> H{resolveConnectionString non-empty?}
H -- No --> I[warn + fallback to D1]
H -- Yes --> J[ensurePostgresSchema one-time DDL]
J --> K[Postgres seam]
G --> L[ExecutorApp.make no requestScoped]
K --> M[ExecutorApp.make with requestScoped]
D -- reject --> N[handlerPromise = null, next request retries]
D -- resolve --> O[serve request]
L --> O
M --> P{Request arrives}
P --> Q[acquire CfPgConnection]
Q --> R[run handler]
R --> S[release CfPgConnection sql.end]
subgraph MCP_DO [MCP Durable Object]
T[openSessionDb] --> U{isPostgresConfigured?}
U -- No --> V[D1 handle no-op close]
U -- Yes --> W[createPostgresExecutorDb long-lived]
W --> X[neutralize close, end = handle.close]
V --> X
X --> Y[session runs]
Y --> Z[DO teardown: end calls sql.end]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Worker fetch] --> B{handlerPromise set?}
B -- No --> C[makeCloudflareApp]
B -- Yes --> D[await handlerPromise]
C --> E[selectDbSeam]
E --> F{isPostgresConfigured?}
F -- No --> G[D1 seam]
F -- Yes --> H{resolveConnectionString non-empty?}
H -- No --> I[warn + fallback to D1]
H -- Yes --> J[ensurePostgresSchema one-time DDL]
J --> K[Postgres seam]
G --> L[ExecutorApp.make no requestScoped]
K --> M[ExecutorApp.make with requestScoped]
D -- reject --> N[handlerPromise = null, next request retries]
D -- resolve --> O[serve request]
L --> O
M --> P{Request arrives}
P --> Q[acquire CfPgConnection]
Q --> R[run handler]
R --> S[release CfPgConnection sql.end]
subgraph MCP_DO [MCP Durable Object]
T[openSessionDb] --> U{isPostgresConfigured?}
U -- No --> V[D1 handle no-op close]
U -- Yes --> W[createPostgresExecutorDb long-lived]
W --> X[neutralize close, end = handle.close]
V --> X
X --> Y[session runs]
Y --> Z[DO teardown: end calls sql.end]
end
Reviews (3): Last reviewed commit: "chore(host-cloudflare): apply AI code re..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/host-cloudflare/scripts/dev-db.ts (1)
39-57: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
execSync대신execFileSync인자 배열 사용 권장정적 분석(ast-grep, OpenGrep)이 동적 명령 문자열을 명령 주입 위험으로 표시합니다. 현재
PORT는Number()로 강제 변환되고pid는lsof출력에서 오므로 실제 주입 가능성은 낮지만,execFileSync("lsof", ["-ti",tcp:${PORT}, ...])처럼 인자 배열 형태로 바꾸면 경고를 제거하고 견고성도 높일 수 있습니다. dev 전용 스크립트라 우선순위는 낮습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/host-cloudflare/scripts/dev-db.ts` around lines 39 - 57, The `reapStaleDevDb` helper currently uses `execSync` with interpolated command strings, which triggers command-injection warnings from static analysis. Update the `lsof`, `ps`, and `kill` calls in `reapStaleDevDb` to use `execFileSync` with explicit argument arrays instead of shell-form strings, keeping the same logic for checking `PORT`, validating `dev-db.ts`, and reaping the stale PID.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/host-cloudflare/src/db/postgres.ts`:
- Around line 99-113: `ensurePostgresSchema` and the shared shutdown path used
by `createPostgresExecutorDb.close`/`McpSessionDO.end` are firing `sql.end()`
without awaiting or handling failures, which can leave rejected promises
unhandled and let teardown finish too early. Update the `finally` block in
`ensurePostgresSchema` and the `close` implementation to await `sql.end({
timeout: 0 })` and swallow any shutdown error with a catch, keeping the
termination behavior consistent across the Postgres lifecycle helpers.
---
Nitpick comments:
In `@apps/host-cloudflare/scripts/dev-db.ts`:
- Around line 39-57: The `reapStaleDevDb` helper currently uses `execSync` with
interpolated command strings, which triggers command-injection warnings from
static analysis. Update the `lsof`, `ps`, and `kill` calls in `reapStaleDevDb`
to use `execFileSync` with explicit argument arrays instead of shell-form
strings, keeping the same logic for checking `PORT`, validating `dev-db.ts`, and
reaping the stale PID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fa2b5b00-c64c-49a2-9ea9-cee0f8c64d65
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
apps/host-cloudflare/.gitignoreapps/host-cloudflare/README.mdapps/host-cloudflare/package.jsonapps/host-cloudflare/scripts/dev-db.tsapps/host-cloudflare/src/app.tsapps/host-cloudflare/src/config.tsapps/host-cloudflare/src/db/data-migrations.tsapps/host-cloudflare/src/db/index.test.tsapps/host-cloudflare/src/db/index.tsapps/host-cloudflare/src/db/postgres.test.tsapps/host-cloudflare/src/db/postgres.tsapps/host-cloudflare/src/mcp/session-durable-object.tsapps/host-cloudflare/src/worker-postgres.e2e.node.test.tsapps/host-cloudflare/wrangler.jsonc
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
chatbot-pf/reference-please(manual)
…ig 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.
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Confidence score: 2/5
- 가장 큰 리스크는
apps/host-cloudflare/src/db/index.ts의selectDbSeam→makeCloudflareApp초기화 경로에서ensurePostgresSchema(env)가 일시 실패할 때,worker.ts의 메모이즈된handlerPromise가 영구 reject 상태로 고정될 수 있다는 점입니다; 배포 직후 일시 네트워크 장애가 장기 장애로 증폭될 수 있으니, 초기화 재시도/백오프 또는 실패 시 재생성 가능한 메모이즈 전략으로 바꾼 뒤 머지하는 것이 안전합니다. apps/host-cloudflare/src/db/postgres.ts에서ensureDrizzleRuntimeSchemaFromTables/runCloudflarePostgresDataMigrations예외 시sql연결 정리가 누락되어 누적 리소스 누수가 발생할 수 있습니다; 실패 경로를ensurePostgresSchema와 동일하게try/finally로 감싸 연결 해제를 보장해 주세요.- 같은
apps/host-cloudflare/src/db/postgres.ts의close가sql.end()를 fire-and-forget으로 호출해, DO 종료 시 실제 연결 종료 전에 흐름이 끝나 연결 정리 타이밍이 불안정해질 수 있습니다;close가Promise를 반환하고 호출부에서await하도록 맞추면 종료 안정성이 올라갑니다. apps/host-cloudflare/scripts/dev-db.ts의 PID 레이스(lsof와ps사이 자연사)로 개발 스크립트가 불필요하게 실패할 수 있지만 운영 영향은 낮습니다; 빈 cmd PID는 건너뛰도록 처리하면 로컬 개발 경험 저하를 막을 수 있습니다.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 10 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
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)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/host-cloudflare/src/worker.ts`:
- Around line 25-29: `getHandler`의 catch 블록에서 `handlerPromise`를 무조건 null로 초기화하는
동시성 레이스가 있습니다. 실패한 프라미스와 현재 `handlerPromise`가 같은 경우에만 초기화하도록 조건을 추가해, 이미 재시도 중인
새 프라미스를 늦은 실패가 덮어쓰지 않게 수정하세요. `handlerPromise`와 `getHandler`를 기준으로 해당 비교 로직을 넣으면
됩니다.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 121b8e94-53a0-4f3d-a962-a8b94012a9c5
📒 Files selected for processing (3)
apps/host-cloudflare/scripts/dev-db.tsapps/host-cloudflare/src/db/postgres.tsapps/host-cloudflare/src/worker.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
chatbot-pf/reference-please(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/host-cloudflare/scripts/dev-db.ts
- apps/host-cloudflare/src/db/postgres.ts
| try { | ||
| return await handlerPromise; | ||
| } catch (err) { | ||
| handlerPromise = null; | ||
| throw err; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
동시성 레이스로 새 핸들러 프라미스가 잘못 초기화될 수 있습니다.
현재 catch에서 무조건 handlerPromise = null을 수행해, 같은 실패 프라미스를 기다리던 늦은 요청이 이미 재시도 중인 새 프라미스를 덮어쓸 수 있습니다. 실패한 프라미스 인스턴스와 동일할 때만 초기화해야 합니다.
수정 제안
const resolveHandler = async (
env: CloudflareEnv,
): Promise<(request: Request) => Promise<Response>> => {
if (!handlerPromise) {
handlerPromise = makeCloudflareApp(env).then(({ toWebHandler }) => toWebHandler().handler);
}
+ const currentPromise = 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;
+ return await currentPromise;
} catch (err) {
- handlerPromise = null;
+ if (handlerPromise === currentPromise) handlerPromise = null;
throw err;
}
// oxlint-enable executor/no-try-catch-or-throw
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| return await handlerPromise; | |
| } catch (err) { | |
| handlerPromise = null; | |
| throw err; | |
| if (!handlerPromise) { | |
| handlerPromise = makeCloudflareApp(env).then(({ toWebHandler }) => toWebHandler().handler); | |
| } | |
| const currentPromise = handlerPromise; | |
| try { | |
| return await currentPromise; | |
| } catch (err) { | |
| if (handlerPromise === currentPromise) handlerPromise = null; | |
| throw err; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/host-cloudflare/src/worker.ts` around lines 25 - 29, `getHandler`의 catch
블록에서 `handlerPromise`를 무조건 null로 초기화하는 동시성 레이스가 있습니다. 실패한 프라미스와 현재
`handlerPromise`가 같은 경우에만 초기화하도록 조건을 추가해, 이미 재시도 중인 새 프라미스를 늦은 실패가 덮어쓰지 않게
수정하세요. `handlerPromise`와 `getHandler`를 기준으로 해당 비교 로직을 넣으면 됩니다.
Source: Coding guidelines
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Adds optional Postgres DB seam with new dependencies, request-scoped connections, and schema bring-up. Core architecture changes require human review.
Re-trigger cubic
Summary
Makes
apps/host-cloudflare's database seam configurable: D1 (SQLite) staysthe zero-setup default, and a Neon/Postgres-over-Hyperdrive path activates
when the operator wires up credentials (a Hyperdrive binding, or a direct
DATABASE_URLbehindEXECUTOR_DIRECT_DATABASE_URL=true). The callers see asingle seam selector; nothing changes for existing D1 deployments.
Under Postgres the three D1 workarounds disappear: real interactive transactions
(no
interactiveTransactions: false), no 100-bound-parameter cap, and largevalues via Postgres TOAST. The Postgres path is modelled on the proven
apps/cloudpostgres.js + Hyperdrive seam, but keepshost-cloudflare's runtimeensurebring-up so the template stays self-provisioning (no out-of-bandmigration step).
Changes
src/db/index.ts:selectDbSeam(env)selector (D1 default; Postgres whencredentials present) and
createExecutorDb(env)for the MCP Durable Object.src/db/postgres.ts: postgres.js + Hyperdrive seam with runtimeensure.CfPgConnection,provided via
ExecutorApp.make'srequestScoped) so the socket'sacquire/release spans the whole request fiber. The DbProvider assembles the
FumaDB handle over it with a no-op close. Cloudflare Workers forbid
reusing an I/O object across request handlers, so a shared connection would
fail with "Cannot perform I/O on behalf of a different request" (and close
the socket after the first request). Schema is brought up once at boot.
idle/lifetime tuning); the DO base disposes it via its
end()contract.src/config.ts:HYPERDRIVE,DATABASE_URL,EXECUTOR_DIRECT_DATABASE_URLon
CloudflareEnv.src/app.ts/src/mcp/session-durable-object.ts: wire the selector; the DOhandle's
closeis neutralized so the per-engine-build scope can't end thesession connection early.
src/db/data-migrations.ts: Postgres data-migration is a no-op (a freshPostgres database has no legacy D1 data; the existing migration is SQLite-only).
wrangler.jsonc: optional, commented-outhyperdrivebinding (D1 defaultuntouched).
scripts/dev-db.ts: local PGlite dev DB on :5433 via the same runtimeensurepath;package.jsonaddspostgres+ PGlite dev deps anddev:db.README.md: "Neon Postgres (opt-in)" setup section.Test Plan
bun run typecheck(host-cloudflare)bun run lint(repo), 0 errorsbun run format:check(repo)vitest run(host-cloudflare), 27 passed, including:src/db/postgres.test.ts: PGlite-over-socket integration: write/readround-trip + transaction commit and rollback (proves the dropped D1
workarounds)
src/worker-postgres.e2e.node.test.ts: the real workerd runtime(wrangler
unstable_dev) against a PGlite socket, exercising db writes/readsacross SEPARATE requests + an MCP
tools/call. This reproduces and guardsagainst the cross-request I/O failure (verified: the pre-fix shared-connection
version returns 500
CONNECTION_ENDED; the request-scoped version passes).To try the Postgres path locally:
Related Issues
Closes #10
Follow-up tracked in #13 (decide runtime
ensurevs out-of-banddrizzle-kitmigrations; live Neon + real Hyperdrive validation; D1 to Postgres data
migration tooling).
Summary by CodeRabbit