db proxy in packages/database uses require() in ESM context — silently broken at runtime #74

📝 Bug Description

packages/database/src/client.ts:24 uses CommonJS require() inside a file that is published as ESM ("type": "module" in packages/database/package.json). At runtime, require("@workspace/env/server") throws ReferenceError: require is not defined the first time getDb() runs. The fallback branch _db = {} as ReturnType<typeof drizzle> is dead code — the throw happens before it can run.

Net effect: the db proxy exports an empty object that throws on every method call. Production callers either catch the error and return a degraded response (the readiness route, Better Auth's session lookup) or never exercise db at all (the /templates route, which calls enrich directly).

This is silently broken in production today, not just in tests. Every call site that depends on a real DB round-trip is failing under the hood — only the error-handling wrappers make it look like it works.

Discovered while implementing the ADR-016 integration test harness (PR #70, merged). The readiness test surfaced a 503 that the test initially diagnosed as a cold pool. The test now documents the bug (see packages/api/tests/integration/system/health.test.ts) but does not fix it — the fix is out of scope for the harness PR.

📋 Steps to Reproduce

1. Spin up the API in a clean environment with DATABASE_URL set.
2. Hit any route that touches db (e.g. GET /api/v1/ready).
3. Inspect the server-side logs and the response.
4. The route returns a degraded response (503, empty body, etc.) and the throw is swallowed.

A minimal reproduction in a Node REPL:

import('./packages/database/dist/index.js').then(({ db }) => {
  // First access triggers getDb() which throws ReferenceError
  db.execute('SELECT 1').catch(err => console.log(err.message))
  // → ReferenceError: require is not defined
})

✅ Expected Behavior

db.execute("SELECT 1") should run against the Postgres pool configured in getDb() and return a result.

❌ Actual Behavior

db.execute("SELECT 1") throws ReferenceError: require is not defined because getDb() calls require("@workspace/env/server") which is invalid in ESM. The if (!url) { _db = {} } branch is unreachable.

Production impact:

• GET /api/v1/ready returns 503 with { status: "not ready" } instead of { status: "ready" }.
• Better Auth's session() middleware returns null (the session lookup throws → swallowed → null), which makes every request look unauthenticated to downstream code.
• Any future route that depends on db and lacks an error-swallowing wrapper will 500.

🎯 Area

area:database (per the issue template's dropdown options — note: area:* labels are not currently configured on the repo, so this is informational only).

📄 Logs / Error Messages

ReferenceError: require is not defined
    at getDb (packages/database/src/client.ts:24:35)
    at Object.get (packages/database/src/client.ts:50:21)
    at db.execute (packages/database/src/client.ts:50:21)
    at /api/v1/ready (packages/api/src/http/routes/http.ts:49:11)

The error is currently swallowed by the readiness route's try/catch and the Better Auth internal error handling, so it never appears in production logs.

🌍 Additional Context

Discovered by: the ADR-016 integration test harness (PR #70, merged on 2026-08-19).

Reproduction commit: the bug existed in packages/database/src/client.ts since the ESM migration. The comment at line 22 ("Dynamic import to avoid ESM circular dependency issues") suggests the author was aware that require was a workaround, but the ESM migration never replaced it with a proper import.

Why this bug was not caught earlier:

• The pre-existing test suite (packages/auth/tests/, contract tests) does not exercise the /api/v1/ready route through the full Hono chain. It tests the auth library in isolation.
• The packages/api/tests/unit/observability.test.ts test does hit /api/v1/ready but the test was authored against a pre-ESM world. The require would have thrown there too, but the test passed because the error was swallowed.
• The packages/api/tests/integration/system/health.test.ts test (added in PR docs(adr): ADR-015 + ADR-016 + ADR-017 — Hono routing, API CI tests, real-network enrich #70) documents the 503 path with a comment but does not fix it — the fix is this issue's scope.

Why the fix is not in PR #70: the integration test harness PR documents and surfaces the bug but does not fix it, because the fix touches production code (packages/database/src/client.ts) and the harness PR is scoped to test-side changes. Mixing the two would have inflated the diff and obscured the review.

Fix scope: packages/database/src/client.ts only.

Effort: ~10 minutes of code + 1 commit + 1 PR.

Proposed fix:

-    const envModule = require("@workspace/env/server") as {
-      serverEnv: { DATABASE_URL: string | undefined }
-    }
-    const url = envModule.serverEnv.DATABASE_URL
-
-    if (!url) {
-      // CLI context: return a passthrough object so imports don't crash.
-      // Real usage always has DATABASE_URL set.
-      _db = {} as ReturnType<typeof drizzle>
-    } else {
-      const pool = postgres(url, { ... })
-      _db = drizzle(pool, { schema })
-    }
+    const { serverEnv } = await import("@workspace/env/server")
+    const url = serverEnv.DATABASE_URL
+    if (!url) {
+      throw new Error("DATABASE_URL is required for @workspace/database")
+    }
+    const pool = postgres(url, { ... })
+    _db = drizzle(pool, { schema })

The fix:

1. Replaces require() with a proper ESM dynamic import() (the function is already async — the change is trivial).
2. Removes the dead _db = {} as ReturnType<typeof drizzle> branch — the throw at the top of getDb() was already unreachable when url is undefined.
3. Throws a clear error message instead of a silent empty-object return, so any future caller that forgets DATABASE_URL fails fast at boot.

Verification:

• GET /api/v1/ready returns 200 with { status: "ready" } after the fix (the readiness test in PR docs(adr): ADR-015 + ADR-016 + ADR-017 — Hono routing, API CI tests, real-network enrich #70 passes 200 instead of skipping or 503).
• Better Auth's session() middleware returns the actual session (or null) instead of throwing through the proxy.

Related:

• ADR-016 in PR docs(adr): ADR-015 + ADR-016 + ADR-017 — Hono routing, API CI tests, real-network enrich #70 documents the readiness test's 503 path as 'a documented bug in packages/database/src/client.ts that is out of scope here.'
• ADR-015 / ADR-016 / ADR-017 (PR docs(adr): ADR-015 + ADR-016 + ADR-017 — Hono routing, API CI tests, real-network enrich #70) all pass CI today despite the bug — the test harness is correct, the production code is wrong.

✅ Implementation report — PR #75

Status: all CI checks passing (Build, Type Check, Lint, Test (Unit), Test (Integration), Drift, Squawk, check-changeset, Vercel previews). Branch impl/74-fix-database-esm-require targets staging.

What was actually done

PR #75 ships 6 file changes, not the 4 originally planned. The original fix worked; the extras were forced by a chain of latent bugs that the strict readiness assertion surfaced for the first time.

1. packages/database/src/client.ts — replaced require("@workspace/env/server") with a static import { serverEnv } from "@workspace/env/server". Replaced the dead _db = {} as ReturnType<typeof drizzle> branch with an explicit throw new Error("DATABASE_URL is required for @workspace/database") so a missing env fails loudly at boot instead of via a swallowed proxy throw. Did NOT use await import(...) from the issue body — that would have forced getDb() async and broken the synchronous Proxy.get trap and the drizzleAdapter(db, …) contract in packages/auth/src/auth.ts:55.

2. packages/eslint-config/base.js — dropped the @typescript-eslint/no-require-imports allowlist that existed only to permit this one require().

3. packages/env/src/loader.ts — JSDoc-only: removed the reference to the require() path in packages/database/src/client.ts.

4. packages/api/tests/integration/system/health.test.ts — tightened the /api/v1/ready integration test from a 503-tolerant if/else to a strict expect(res.status).toBe(200).

5. turbo.json (not in original plan) — added an env allowlist to the test task (DATABASE_URL, TEST_DATABASE_URL, BETTER_AUTH_SECRET, AUTH_SECRET, BETTER_AUTH_URL, ALLOWED_ORIGINS, RATE_LIMIT_PER_MINUTE, RESEND_*, MAIL_TRANSPORT, GITHUB_TOKEN). Mirrors the existing build task's allowlist.

6. packages/auth/tests/session.test.ts (not in original plan) — moved ctx.test.getAuthHeaders({ userId }) before ctx.test.deleteUser(user.id) in 'should save and delete a user'. The helper inserts a session row keyed by user_id, which violated the session.user_id -> user.id FK once the user row was gone.

Problems encountered and solutions

Problem 1 — CI red after the strict assertion

After the initial commit (just the four originally planned files), Test (Integration) failed with AssertionError: expected 503 to be 200 on GET /api/v1/ready. Temporary debug log showed:

[db-debug] process.env.DATABASE_URL= undefined   serverEnv.DATABASE_URL= undefined
[ready] db.execute failed: Error: DATABASE_URL is required for @workspace/database

Initial (wrong) hypothesis: vitest pool / test.env

I first assumed the issue was a Vitest 4 + GitHub Actions worker-thread env-propagation problem (well-known across vitest-dev/vitest#8769, #9683, and elsewhere). Tried three variants, none of which moved the needle:

• pool: "forks" in packages/vitest-config/src/index.ts — did not change the worker env.
• test.env allowlist in the vitest config, forwarding env at config-load time — did not change the worker env.

Problem 2 — debug log in globalSetup revealed the real cause

Added a debug log in packages/api/tests/globalSetup.ts to check the main vitest process (where globalSetup runs):

[api-tests-debug] main process process.env.DATABASE_URL=undefined

The main process also saw DATABASE_URL as undefined. So the env was being stripped before vitest started — not inside vitest's worker pool.

The actual culprit: turbo.json had no env field on the test task. Turbo's default is to pass only globalEnv + globalPassThroughEnv (CI, NODE_ENV, TURBO_TOKEN, TURBO_TEAM, GITHUB_ACTIONS, VERCEL_URL). Everything else — including DATABASE_URL — was silently stripped before turbo spawned the vitest process. The build task already had the allowlist; the test task didn't.

Solution — added an env allowlist to the test task mirroring the build task. After the fix, globalSetup saw the env correctly, the integration tests ran for real, and /api/v1/ready returned 200.

Problem 3 — second failure: latent test-order bug

With the env fix in place, packages/auth/tests/session.test.ts > 'should save and delete a user' started failing with:

PostgresError: insert or update on table "session" violates foreign key constraint "session_user_id_user_id_fk"
Key (user_id)=(...) is not present in table "user"

The test called ctx.test.deleteUser(user.id) and then ctx.test.getAuthHeaders({ userId: user.id }). getAuthHeaders inserts a session row keyed by user_id, which violates the FK once the user is gone.

This was a latent test bug that had been silently skipped in CI: the describe's hasDatabase gate was false because turbo stripped DATABASE_URL (Problem 2 above), so the integration tests were skipped. With Problem 2 fixed, the test ran for the first time and exposed the bug.

Solution — moved ctx.test.getAuthHeaders({ userId: user.id }) to before ctx.test.deleteUser(user.id). The session is now created against an existing user; the post-delete getSession call returns null as the test originally intended.

Final state

PR #75 — all CI checks green. The branch carries several debug(...) commits from the diagnosis phase. These only contained console.error lines that were removed before the test assertions were tightened, so they leave no trace in the production code paths.

Key takeaways

1. The proposed fix in this issue (await import(...)) would have broken drizzleAdapter and the synchronous Proxy.get trap. The static import is correct because serverEnv is already a lazy Proxy in packages/env/src/server.ts — no circular dependency exists, the old comment was outdated.
2. turbo.json had a latent bug: test task missing the env allowlist that build already had. Worth a future repo-wide audit of every turbo task's env field.
3. The pre-existing test-order bug in session.test.ts had been hidden for a long time by the missing turbo allowlist. Worth a future sweep for similar silently-skipped describes across the test suite.

Related

• PR fix(database): replace ESM-incompatible require() with static import #75: fix(database): replace ESM-incompatible require() with static import #75
• PR docs(adr): ADR-015 + ADR-016 + ADR-017 — Hono routing, API CI tests, real-network enrich #70 (ADR-016 integration test harness that surfaced this bug)
• ADR-016 (readiness test docs the 503 path)