Skip to content

feat(host-cloudflare): configurable db seam — D1 default, Neon/Postgres via Hyperdrive opt-in#14

Open
amondnet wants to merge 4 commits into
mainfrom
amondnet/evaluate-neon-planetscale-hyperdrive-for-apps-ho
Open

feat(host-cloudflare): configurable db seam — D1 default, Neon/Postgres via Hyperdrive opt-in#14
amondnet wants to merge 4 commits into
mainfrom
amondnet/evaluate-neon-planetscale-hyperdrive-for-apps-ho

Conversation

@amondnet

@amondnet amondnet commented Jun 25, 2026

Copy link
Copy Markdown

Summary

Makes apps/host-cloudflare's database seam configurable: D1 (SQLite) stays
the zero-setup default
, and a Neon/Postgres-over-Hyperdrive path activates
when the operator wires up credentials (a Hyperdrive binding, or a direct
DATABASE_URL behind EXECUTOR_DIRECT_DATABASE_URL=true). The callers see a
single 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 large
values via Postgres TOAST. The Postgres path is modelled on the proven
apps/cloud postgres.js + Hyperdrive seam, but keeps host-cloudflare's runtime
ensure bring-up so the template stays self-provisioning (no out-of-band
migration step).

Changes

  • src/db/index.ts: selectDbSeam(env) selector (D1 default; Postgres when
    credentials present) and createExecutorDb(env) for the MCP Durable Object.
  • src/db/postgres.ts: postgres.js + Hyperdrive seam with runtime ensure.
    • HTTP: connection lives in a request-scoped service (CfPgConnection,
      provided via ExecutorApp.make's requestScoped) so the socket's
      acquire/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.
    • MCP Durable Object: one long-lived connection per session (matching cloud's
      idle/lifetime tuning); the DO base disposes it via its end() contract.
  • src/config.ts: HYPERDRIVE, DATABASE_URL, EXECUTOR_DIRECT_DATABASE_URL
    on CloudflareEnv.
  • src/app.ts / src/mcp/session-durable-object.ts: wire the selector; the DO
    handle's close is neutralized so the per-engine-build scope can't end the
    session connection early.
  • src/db/data-migrations.ts: Postgres data-migration is a no-op (a fresh
    Postgres database has no legacy D1 data; the existing migration is SQLite-only).
  • wrangler.jsonc: optional, commented-out hyperdrive binding (D1 default
    untouched).
  • scripts/dev-db.ts: local PGlite dev DB on :5433 via the same runtime
    ensure path; package.json adds postgres + PGlite dev deps and dev:db.
  • README.md: "Neon Postgres (opt-in)" setup section.

Test Plan

  • bun run typecheck (host-cloudflare)
  • bun run lint (repo), 0 errors
  • bun run format:check (repo)
  • vitest run (host-cloudflare), 27 passed, including:
    • existing D1 e2e on workerd, confirming the default path is unchanged
    • src/db/postgres.test.ts: PGlite-over-socket integration: write/read
      round-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/reads
      across SEPARATE requests + an MCP tools/call. This reproduces and guards
      against 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:

cd apps/host-cloudflare
bun run dev:db   # PGlite on :5433
bun run dev      # wrangler dev reads HYPERDRIVE.localConnectionString

Related Issues

Closes #10

Follow-up tracked in #13 (decide runtime ensure vs out-of-band drizzle-kit
migrations; live Neon + real Hyperdrive validation; D1 to Postgres data
migration tooling).

Summary by CodeRabbit

  • New Features
    • Cloudflare 호스트에서 기본 D1을 유지하면서, 옵트인으로 Neon(Postgres) + Hyperdrive를 사용할 수 있게 됐습니다.
    • 로컬 개발에서 Docker 없이 Postgres(PGlite)로 실행하는 개발 흐름이 추가됐습니다.
  • Bug Fixes
    • Postgres 설정이 불완전하면 자동으로 D1으로 안전하게 폴백하도록 개선했습니다.
    • 워커 핸들 초기화 실패 시 재시도가 가능하도록 안정화했습니다.
  • Documentation
    • Postgres(옵트인) 설정 절차와 로컬 연결 방법을 README에 구체화했습니다.
  • Tests
    • Postgres 연결, 요청 흐름, 트랜잭션/롤백 동작 및 E2E 검증이 추가됐습니다.

amondnet added 2 commits June 25, 2026 22:02
…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
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Cloudflare host 앱이 D1 기본, Hyperdrive/Postgres 옵트인으로 전환되고, 로컬 PGlite 개발 DB와 Postgres 연결/스키마 bring-up이 추가됐다. MCP 세션 수명과 워커 E2E 테스트도 새 Postgres 경로에 맞게 갱신됐다.

Changes

Host Cloudflare Postgres opt-in

Layer / File(s) Summary
Opt-in config and docs
apps/host-cloudflare/src/config.ts, apps/host-cloudflare/package.json, apps/host-cloudflare/README.md, apps/host-cloudflare/wrangler.jsonc
CloudflareEnv에 Hyperdrive/Postgres 환경 필드가 추가되고, dev:db 스크립트와 Postgres/PGlite 의존성이 더해지며, README와 Wrangler 주석이 D1 기본/Hyperdrive opt-in 흐름을 설명한다.
Local PGlite dev server
apps/host-cloudflare/.gitignore, apps/host-cloudflare/scripts/dev-db.ts
.dev-db/가 무시되고, Bun 스크립트가 PGlite DB 생성·스키마 적용·TCP 소켓 노출·정리 종료를 수행한다.
DB seam selection and app wiring
apps/host-cloudflare/src/db/index.ts, apps/host-cloudflare/src/app.ts, apps/host-cloudflare/src/worker.ts, apps/host-cloudflare/src/db/index.test.ts
Postgres 구성 여부와 연결 문자열 해석 로직이 추가되고, selectDbSeam(env) 결과에 따라 ExecutorApp가 D1 또는 Postgres provider로 조립되며, worker 초기화 실패 시 handler promise를 재설정하는 동작과 분기 로직이 테스트로 검증된다.
Postgres connector and bring-up
apps/host-cloudflare/src/db/data-migrations.ts, apps/host-cloudflare/src/db/postgres.ts, apps/host-cloudflare/src/db/postgres.test.ts
Postgres 연결 문자열, 런타임 스키마 캐시, R2 blob store, schema bring-up, request-scoped/long-lived DB handle이 추가되고, PGlite 기반 통합 테스트가 CRUD와 트랜잭션 원자성을 확인한다.
MCP session and worker E2E
apps/host-cloudflare/src/mcp/session-durable-object.ts, apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts
MCP 세션 DB 생성이 createExecutorDb로 전환되고 close/end 수명이 조정되며, 워커 E2E가 직접 Postgres 경로, 반복 쓰기, MCP execute 호출을 검증한다.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • #13: ensurePostgresSchemaselectDbSeam/createPostgresExecutorDb 경로가 런타임 schema ensure 전략을 구현하고 README에서 문서화합니다.
  • #10: apps/host-cloudflare의 db seam을 D1에서 Neon/Postgres + Hyperdrive로 평가/전환하는 목적과 이 PR의 변경 범위가 일치합니다.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목이 host-cloudflare의 DB seam을 D1 기본 + Hyperdrive 기반 Postgres 옵트인으로 바꾸는 핵심 변경을 정확히 요약합니다.
Linked Issues check ✅ Passed [#10] D1 기본 유지, Hyperdrive/Neon Postgres 옵트인, 스키마 bring-up, 트랜잭션·파라미터·대용량 제한 완화가 요구와 맞습니다.
Out of Scope Changes check ✅ Passed README, wrangler 설정, 테스트, 로컬 PGlite 스크립트 등은 새 DB seam을 지원하는 범위 내 변경으로 보입니다.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch amondnet/evaluate-neon-planetscale-hyperdrive-for-apps-ho
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch amondnet/evaluate-neon-planetscale-hyperdrive-for-apps-ho

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread apps/host-cloudflare/scripts/dev-db.ts Outdated
Comment thread apps/host-cloudflare/src/db/postgres.ts Outdated
Comment thread apps/host-cloudflare/src/db/postgres.ts Outdated
@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the database seam of apps/host-cloudflare configurable: D1 (SQLite) remains the zero-setup default, and a Neon/Postgres-over-Hyperdrive path activates automatically when the operator wires up a Hyperdrive binding or a DATABASE_URL behind EXECUTOR_DIRECT_DATABASE_URL=true. All three D1 workarounds (disabled interactive transactions, 100-bound-parameter cap, small value ceiling) are dropped on the Postgres path.

  • src/db/index.ts + src/db/postgres.ts: new selectDbSeam selector, CfPgConnection request-scoped Effect service (HTTP path), and createPostgresExecutorDb long-lived connection (MCP Durable Object path); connection lifecycle and schema bring-up correctly isolated for each execution model.
  • src/worker.ts: boot failure now clears the memoized handlerPromise so a transient Postgres outage at isolate start does not permanently poison the isolate; subsequent requests reattempt boot.
  • Tests: 27 Vitest passes including a full workerd/Miniflare e2e that explicitly guards against the "Cannot perform I/O on behalf of a different request" Cloudflare failure mode the request-scoped connection design is meant to prevent.

Confidence Score: 5/5

Safe 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

Filename Overview
apps/host-cloudflare/src/db/postgres.ts New Postgres seam: request-scoped CfPgConnection layer (HTTP) and long-lived createPostgresExecutorDb (DO). All three previously flagged connection-lifecycle issues are addressed: DO close now awaits sql.end, schema-bring-up failure properly cleans up the long-lived socket via try/catch, and the no-op DbProvider close correctly decouples the executor-build scope from the session connection.
apps/host-cloudflare/src/db/index.ts selectDbSeam and createExecutorDb selectors are correct. Partial-misconfig guard (isPostgresConfigured true but resolveConnectionString empty) correctly falls back to D1 with a warning. Unit tests cover all decision branches.
apps/host-cloudflare/src/worker.ts Boot-failure retry fix: handlerPromise is cleared on rejection so transient Postgres outages at isolate boot do not permanently poison the isolate. Concurrent requests during a failure share the same boot attempt and all fail together, then the next wave reattempts cleanly.
apps/host-cloudflare/src/mcp/session-durable-object.ts openSessionDb correctly neutralizes handle.close (so the executor-build scope cannot end the session connection) and wires the real teardown through end, matching the DO base contract for both D1 (no-op close) and Postgres (sql.end) paths.
apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts Workerd/Miniflare e2e against PGlite: explicitly covers separate-request db reads/writes (guarding the cross-request I/O isolation constraint), sequential writes loop, and MCP DO long-lived connection path. Directly reproduces the pre-fix failure mode.
apps/host-cloudflare/scripts/dev-db.ts Local PGlite dev server with stale-process reaping. Shares the same runtime ensureDrizzleRuntimeSchemaFromTables code path as the deployed Worker. Dev-only script; lsof/ps reliance and 200ms settle sleep are acceptable for this purpose.
apps/host-cloudflare/src/config.ts HYPERDRIVE, DATABASE_URL, EXECUTOR_DIRECT_DATABASE_URL all optional, keeping D1 as zero-setup default. The explicit EXECUTOR_DIRECT_DATABASE_URL=true guard prevents a stray DATABASE_URL secret from silently bypassing Hyperdrive.
apps/host-cloudflare/src/db/data-migrations.ts Postgres no-op migration stub is correct: a fresh Postgres deployment has no legacy D1 data, and the existing SQLite-specific migration uses json_extract/D1 sessions that cannot run on Postgres.
apps/host-cloudflare/src/app.ts Two ExecutorApp.make calls (one per seam kind) keep residual types exact: the Postgres branch's db layer carries a CfPgConnection requirement that is only satisfied by the requestScoped parameter, preserving Effect's type safety.

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
Loading
%%{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
Loading

Reviews (3): Last reviewed commit: "chore(host-cloudflare): apply AI code re..." | Re-trigger Greptile

Comment thread apps/host-cloudflare/src/db/index.ts
Comment thread apps/host-cloudflare/src/db/postgres.ts
Comment thread apps/host-cloudflare/src/db/postgres.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)이 동적 명령 문자열을 명령 주입 위험으로 표시합니다. 현재 PORTNumber()로 강제 변환되고 pidlsof 출력에서 오므로 실제 주입 가능성은 낮지만, 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

📥 Commits

Reviewing files that changed from the base of the PR and between e12cfb2 and 8ec707e.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • apps/host-cloudflare/.gitignore
  • apps/host-cloudflare/README.md
  • apps/host-cloudflare/package.json
  • apps/host-cloudflare/scripts/dev-db.ts
  • apps/host-cloudflare/src/app.ts
  • apps/host-cloudflare/src/config.ts
  • apps/host-cloudflare/src/db/data-migrations.ts
  • apps/host-cloudflare/src/db/index.test.ts
  • apps/host-cloudflare/src/db/index.ts
  • apps/host-cloudflare/src/db/postgres.test.ts
  • apps/host-cloudflare/src/db/postgres.ts
  • apps/host-cloudflare/src/mcp/session-durable-object.ts
  • apps/host-cloudflare/src/worker-postgres.e2e.node.test.ts
  • apps/host-cloudflare/wrangler.jsonc
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • chatbot-pf/reference-please (manual)

Comment thread apps/host-cloudflare/src/db/postgres.ts
…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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found and verified against the latest diff

Confidence score: 2/5

  • 가장 큰 리스크는 apps/host-cloudflare/src/db/index.tsselectDbSeammakeCloudflareApp 초기화 경로에서 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.tsclosesql.end()를 fire-and-forget으로 호출해, DO 종료 시 실제 연결 종료 전에 흐름이 끝나 연결 정리 타이밍이 불안정해질 수 있습니다; closePromise를 반환하고 호출부에서 await하도록 맞추면 종료 안정성이 올라갑니다.
  • apps/host-cloudflare/scripts/dev-db.ts의 PID 레이스(lsofps 사이 자연사)로 개발 스크립트가 불필요하게 실패할 수 있지만 운영 영향은 낮습니다; 빈 cmd PID는 건너뛰도록 처리하면 로컬 개발 경험 저하를 막을 수 있습니다.

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/host-cloudflare/src/db/postgres.ts
Comment thread apps/host-cloudflare/src/db/index.ts
Comment thread apps/host-cloudflare/src/db/postgres.ts Outdated
Comment thread apps/host-cloudflare/scripts/dev-db.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread apps/host-cloudflare/src/db/postgres.ts Outdated
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)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between c69d503 and c3c8f56.

📒 Files selected for processing (3)
  • apps/host-cloudflare/scripts/dev-db.ts
  • apps/host-cloudflare/src/db/postgres.ts
  • apps/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

Comment on lines +25 to +29
try {
return await handlerPromise;
} catch (err) {
handlerPromise = null;
throw err;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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.

Suggested change
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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Evaluate Neon/PlanetScale + Hyperdrive for apps/host-cloudflare db seam (vs D1)

1 participant