Agree the test harness escape with resolveDatabaseUrl()'s variable order

.changeset/silent-plums-listen.md

@@ -0,0 +1,5 @@
+---
+'@opensaas/stack-core': patch
+---
+
+Fix the test harness's escape to read `DIRECT_DATABASE_URL`/`DATABASE_URL` in the same order `resolveDatabaseUrl()` does, so a `DIRECT_DATABASE_URL`-only environment runs the suite against that server instead of silently falling back to PGlite.

packages/core/src/db/dev-database.test.ts

@@ -9,6 +9,7 @@ import { DevDatabaseInUseError, startDevDatabase, type DevDatabase } from './dev
 import { processStartTime } from './process-identity.js'
 import { readDevDatabaseState, writeDevDatabaseState, type DevDatabaseState } from './state-file.js'
 import { resolveDatabaseUrl } from './url.js'
+import { readDatabaseEscape } from '../testing/escape.js'
 
 /**
  * The claim fields `readDevDatabaseState` returns for this process, for an
@@ -363,53 +364,26 @@ describe('startDevDatabase', () => {
   )
 })
 
-const POSTGRES_SCHEMES = new Set(['postgres:', 'postgresql:'])
-
-type Escape =
-  | { kind: 'absent' }
-  | { kind: 'postgres'; name: string; url: string }
-  | { kind: 'unusable'; name: string; url: string; fault: string }
-
-/**
- * `pg` parses any string it is handed, defaulting whatever it cannot read to
- * `localhost:5432`, so a connection string that names no Postgres surfaces as a
- * refused connection several frames from its cause. Classifying it here keeps
- * the misconfiguration named where it is made.
- */
-function readEscape(): Escape {
-  for (const name of ['DIRECT_DATABASE_URL', 'DATABASE_URL'] as const) {
-    const url = process.env[name]
-    if (url === undefined || url.length === 0) continue
-    let scheme: string
-    try {
-      scheme = new URL(url).protocol
-    } catch {
-      return { kind: 'unusable', name, url, fault: 'is not a URL' }
-    }
-    if (!POSTGRES_SCHEMES.has(scheme))
-      return { kind: 'unusable', name, url, fault: `names the \`${scheme}\` scheme, not Postgres` }
-    return { kind: 'postgres', name, url }
-  }
-  return { kind: 'absent' }
-}
-
 /**
- * The escape: with a Postgres `DATABASE_URL` set this suite exercises whatever
- * server it names, so CI can run on a real one while a developer machine runs
- * with nothing installed. Set to anything else the variable is a
- * misconfiguration, and this suite says so rather than dialling it.
+ * The escape: with a Postgres `DATABASE_URL`/`DIRECT_DATABASE_URL` set this
+ * suite exercises whatever server it names, so CI can run on a real one while
+ * a developer machine runs with nothing installed. Set to anything else the
+ * variable is a misconfiguration, and this suite says so rather than dialling
+ * it. `readDatabaseEscape()` is the harness's own classification — reusing it
+ * here is what keeps this suite's notion of "the escape" from drifting away
+ * from `resolveDatabaseUrl()`'s, the bug issue #1210 fixed.
  */
 describe('the resolved database', () => {
-  const escape = readEscape()
+  const escape = readDatabaseEscape()
   let projectRoot: string
   let database: DevDatabase | undefined
 
   beforeEach(async () => {
     projectRoot = mkdtempSync(path.join(tmpdir(), 'opensaas-dev-db-'))
     if (escape.kind === 'unusable')
       throw new Error(
-        `${escape.name} is set to \`${escape.url}\`, which ${escape.fault}. This suite dials the ` +
-          `URL the lookup reports, so leave the variable unset to exercise the dev database or ` +
+        `${escape.variable} is set to \`${escape.url}\`, which ${escape.fault}. This suite dials ` +
+          `the URL the lookup reports, so leave the variable unset to exercise the dev database or ` +
           `point it at a Postgres server.`,
       )
     database = escape.kind === 'absent' ? await startDevDatabase({ cwd: projectRoot }) : undefined

packages/core/src/db/url.ts

@@ -4,8 +4,13 @@ import { readDevDatabaseState, type DevDatabaseStateLocation } from './state-fil
  * The environment variables consulted, in order. `DIRECT_DATABASE_URL` wins so
  * a schema command reaches the direct connection rather than a pooler that
  * cannot run DDL (ADR-0003).
+ *
+ * This is the one place the list is written down — the test harness's escape
+ * (`@opensaas/stack-core/testing`'s `ESCAPE_VARIABLES`) imports it rather than
+ * keeping its own, so the two lookups cannot disagree about which variable
+ * counts (issue #1210).
  */
-const CONNECTION_VARIABLES = ['DIRECT_DATABASE_URL', 'DATABASE_URL'] as const
+export const CONNECTION_VARIABLES = ['DIRECT_DATABASE_URL', 'DATABASE_URL'] as const
 
 /** Thrown by {@link resolveDatabaseUrl} when neither remedy has been taken. */
 export class DatabaseUrlUnresolvedError extends Error {

packages/core/src/secured/capacity-gate.test.ts

@@ -9,7 +9,7 @@ import { originTripwire } from '../origin.js'
 import type { StackContext } from '../types/context.js'
 import type { AccessControlledDB } from '../access/types.js'
 import { createTestDatabase, ormClientFor, type TestDatabase } from '../testing/context.js'
-import { ESCAPE_VARIABLE, readDatabaseEscape } from '../testing/escape.js'
+import { ESCAPE_VARIABLES, readDatabaseEscape } from '../testing/escape.js'
 
 /**
  * The #614 capacity gate: N concurrent racers against a capacity-N slot admit
@@ -104,7 +104,7 @@ const escape = readDatabaseEscape()
 describe.skipIf(escape.kind !== 'postgres')(
   escape.kind === 'postgres'
     ? 'the #614 capacity gate under contention'
-    : `the #614 capacity gate under contention [escape-only: ${ESCAPE_VARIABLE} names no Postgres]`,
+    : `the #614 capacity gate under contention [escape-only: ${ESCAPE_VARIABLES.join('/')} names no Postgres]`,
   () => {
     let database: TestDatabase
     let racers: Racer[]

packages/core/src/secured/lock.test.ts

@@ -6,7 +6,7 @@ import type { OpenSaasConfig } from '../config/types.js'
 import type { Session } from '../access/types.js'
 import { integer, relationship, text } from '../fields/index.js'
 import { createTestDatabase, type TestDatabase } from '../testing/context.js'
-import { ESCAPE_VARIABLE, readDatabaseEscape } from '../testing/escape.js'
+import { ESCAPE_VARIABLES, readDatabaseEscape } from '../testing/escape.js'
 import {
   createRowLockLane,
   ROW_LOCK_MAX_KEYS,
@@ -470,7 +470,7 @@ const escape = readDatabaseEscape()
 describe.skipIf(escape.kind !== 'postgres')(
   escape.kind === 'postgres'
     ? 'forUpdate() under contention'
-    : `forUpdate() under contention [escape-only: ${ESCAPE_VARIABLE} names no Postgres]`,
+    : `forUpdate() under contention [escape-only: ${ESCAPE_VARIABLES.join('/')} names no Postgres]`,
   () => {
     let contended: TestDatabase
 

packages/core/src/secured/nearest.test.ts

@@ -12,7 +12,7 @@ import { ValidationError } from '../hooks/index.js'
 import { withOrigin } from '../origin.js'
 import { createTestDatabase, type TestDatabase } from '../testing/context.js'
 import { createPlanRecorder, type RecordedPlan } from '../testing/plans.js'
-import { ESCAPE_VARIABLE, readDatabaseEscape } from '../testing/escape.js'
+import { ESCAPE_VARIABLES, readDatabaseEscape } from '../testing/escape.js'
 
 const BOOT = 120_000
 
@@ -215,7 +215,9 @@ const available =
   })())
 
 describe.skipIf(!available)(
-  available ? 'nearest()' : `nearest() [skipped: the ${ESCAPE_VARIABLE} server has no pgvector]`,
+  available
+    ? 'nearest()'
+    : `nearest() [skipped: the ${ESCAPE_VARIABLES.join('/')} server has no pgvector]`,
   () => {
     beforeAll(async () => {
       database = await createTestDatabase(config, { middleware: [recorder.middleware] })

packages/core/src/secured/typed-read-surface.test.ts

@@ -6,7 +6,7 @@ import { checkbox, integer, relationship, text } from '../fields/index.js'
 import { withOrigin } from '../origin.js'
 import { createTestDatabase, type TestDatabase } from '../testing/context.js'
 import {
-  ESCAPE_VARIABLE,
+  ESCAPE_VARIABLES,
   probePgvectorAvailability,
   readDatabaseEscape,
 } from '../testing/escape.js'
@@ -394,7 +394,7 @@ describe('each promised member answers', () => {
   test.skipIf(!pgvectorAvailable)(
     pgvectorAvailable
       ? 'nearest returns the row and its score'
-      : `nearest returns the row and its score [skipped: the ${ESCAPE_VARIABLE} server has no pgvector]`,
+      : `nearest returns the row and its score [skipped: the ${ESCAPE_VARIABLES.join('/')} server has no pgvector]`,
     async () => {
       await seed('Article', { title: 'near', embedding: [1, 0, 0] })
       await seed('Article', { title: 'far', embedding: [0, 1, 0] })

packages/core/src/testing/context.test.ts

@@ -11,7 +11,7 @@ import {
   OrmCollectionMissingError,
   type TestDatabase,
 } from './context.js'
-import { ESCAPE_VARIABLE, probePgvectorAvailability, readDatabaseEscape } from './escape.js'
+import { ESCAPE_VARIABLES, probePgvectorAvailability, readDatabaseEscape } from './escape.js'
 import { ExtensionPackUnavailableError, loadExtensionPacks } from './extensions.js'
 import { createPlanRecorder } from './plans.js'
 
@@ -178,7 +178,7 @@ const vectorAvailable = escape.kind !== 'postgres' || (await probePgvectorAvaila
 
 const vectorSuite = vectorAvailable
   ? 'a pgvector-declaring config stands up on the default harness'
-  : `a pgvector-declaring config stands up [skipped: the ${ESCAPE_VARIABLE} server has no pgvector]`
+  : `a pgvector-declaring config stands up [skipped: the ${ESCAPE_VARIABLES.join('/')} server has no pgvector]`
 
 describe.skipIf(!vectorAvailable)(vectorSuite, () => {
   let database: TestDatabase
@@ -239,24 +239,9 @@ describe('the recording middleware', () => {
   )
 })
 
-describe('the DATABASE_URL escape', () => {
-  test('a set-but-unusable value is a misconfiguration, not a database', () => {
-    const saved = process.env[ESCAPE_VARIABLE]
-    process.env[ESCAPE_VARIABLE] = 'file:./dev.db'
-    try {
-      expect(readDatabaseEscape()).toEqual({
-        kind: 'unusable',
-        url: 'file:./dev.db',
-        fault: 'names the `file:` scheme, not Postgres',
-      })
-    } finally {
-      if (saved === undefined) delete process.env[ESCAPE_VARIABLE]
-      else process.env[ESCAPE_VARIABLE] = saved
-    }
-  })
-
+describe('the database escape', () => {
   test.skipIf(escape.kind !== 'postgres')(
-    `contention is observable on a real server [escape-only: ${ESCAPE_VARIABLE} names no Postgres]`,
+    `contention is observable on a real server [escape-only: ${ESCAPE_VARIABLES.join('/')} names no Postgres]`,
     async () => {
       const database = await createTestDatabase(blogConfig)
       try {

packages/core/src/testing/context.ts

@@ -16,7 +16,7 @@ import type { ContractData } from '../contract/types.js'
 import { getContext } from '../context/index.js'
 import { originTripwire } from '../origin.js'
 import type { StackContext } from '../types/context.js'
-import { ESCAPE_VARIABLE, requireUsableDatabaseEscape } from './escape.js'
+import { ESCAPE_VARIABLES, requireUsableDatabaseEscape } from './escape.js'
 import {
   contractPacks,
   loadExtensionPacks,
@@ -286,7 +286,7 @@ export class DevDatabaseUnavailableError extends Error {
     super(
       `The in-process test database could not start: ${cause instanceof Error ? cause.message : String(cause)}. ` +
         `PGlite is an optional peer of @opensaas/stack-core, so install ${DEV_DATABASE_PEERS.join(', ')} ` +
-        `as dev dependencies of this package, or set ${ESCAPE_VARIABLE} to a Postgres server and run the ` +
+        `as dev dependencies of this package, or set ${ESCAPE_VARIABLES.join(' or ')} to a Postgres server and run the ` +
         `suite against that instead.`,
     )
     this.name = 'DevDatabaseUnavailableError'

packages/core/src/testing/escape.test.ts

@@ -0,0 +1,116 @@
+import { afterEach, beforeEach, describe, expect, test } from 'vitest'
+import { CONNECTION_VARIABLES } from '../db/url.js'
+import {
+  ESCAPE_VARIABLES,
+  readDatabaseEscape,
+  requireUsableDatabaseEscape,
+  UnusableDatabaseEscapeError,
+} from './escape.js'
+
+describe('readDatabaseEscape', () => {
+  let saved: Record<string, string | undefined>
+
+  beforeEach(() => {
+    saved = Object.fromEntries(CONNECTION_VARIABLES.map((name) => [name, process.env[name]]))
+    for (const name of CONNECTION_VARIABLES) delete process.env[name]
+  })
+
+  afterEach(() => {
+    for (const name of CONNECTION_VARIABLES) {
+      const value = saved[name]
+      if (value === undefined) delete process.env[name]
+      else process.env[name] = value
+    }
+  })
+
+  test('agrees with resolveDatabaseUrl() on which variables it reads, in the same order', () => {
+    expect(ESCAPE_VARIABLES).toEqual(CONNECTION_VARIABLES)
+    expect(ESCAPE_VARIABLES[0]).toBe('DIRECT_DATABASE_URL')
+  })
+
+  test('is absent when neither variable is set', () => {
+    expect(readDatabaseEscape()).toEqual({ kind: 'absent' })
+  })
+
+  test('is postgres from DATABASE_URL alone', () => {
+    process.env.DATABASE_URL = 'postgres://someone@example.test:5432/app'
+    expect(readDatabaseEscape()).toEqual({
+      kind: 'postgres',
+      url: 'postgres://someone@example.test:5432/app',
+    })
+  })
+
+  test('is postgres from DIRECT_DATABASE_URL alone, rather than falling back to absent (#1210)', () => {
+    process.env.DIRECT_DATABASE_URL = 'postgres://direct@example.test:5432/app'
+    expect(readDatabaseEscape()).toEqual({
+      kind: 'postgres',
+      url: 'postgres://direct@example.test:5432/app',
+    })
+  })
+
+  test('prefers DIRECT_DATABASE_URL over DATABASE_URL, matching resolveDatabaseUrl()', () => {
+    process.env.DATABASE_URL = 'postgres://pooler@example.test:6543/app'
+    process.env.DIRECT_DATABASE_URL = 'postgres://direct@example.test:5432/app'
+    expect(readDatabaseEscape()).toEqual({
+      kind: 'postgres',
+      url: 'postgres://direct@example.test:5432/app',
+    })
+  })
+
+  test('a set-but-unusable DATABASE_URL is refused by name, not dialled', () => {
+    process.env.DATABASE_URL = 'file:./dev.db'
+    expect(readDatabaseEscape()).toEqual({
+      kind: 'unusable',
+      variable: 'DATABASE_URL',
+      url: 'file:./dev.db',
+      fault: 'names the `file:` scheme, not Postgres',
+    })
+  })
+
+  test('a set-but-unusable DIRECT_DATABASE_URL is refused by name too', () => {
+    process.env.DIRECT_DATABASE_URL = 'file:./dev.db'
+    expect(readDatabaseEscape()).toEqual({
+      kind: 'unusable',
+      variable: 'DIRECT_DATABASE_URL',
+      url: 'file:./dev.db',
+      fault: 'names the `file:` scheme, not Postgres',
+    })
+  })
+
+  test('an unusable DIRECT_DATABASE_URL wins over a usable DATABASE_URL, matching resolveDatabaseUrl() order', () => {
+    process.env.DATABASE_URL = 'postgres://pooler@example.test:6543/app'
+    process.env.DIRECT_DATABASE_URL = 'not a url at all'
+    expect(readDatabaseEscape()).toEqual({
+      kind: 'unusable',
+      variable: 'DIRECT_DATABASE_URL',
+      url: 'not a url at all',
+      fault: 'is not a URL',
+    })
+  })
+
+  test('requireUsableDatabaseEscape returns undefined when absent', () => {
+    expect(requireUsableDatabaseEscape()).toBeUndefined()
+  })
+
+  test('requireUsableDatabaseEscape returns the url when usable', () => {
+    process.env.DIRECT_DATABASE_URL = 'postgres://direct@example.test:5432/app'
+    expect(requireUsableDatabaseEscape()).toBe('postgres://direct@example.test:5432/app')
+  })
+
+  test('requireUsableDatabaseEscape throws naming the variable, the value and both remedies', () => {
+    process.env.DIRECT_DATABASE_URL = 'file:./dev.db'
+    try {
+      requireUsableDatabaseEscape()
+      expect.unreachable()
+    } catch (error) {
+      expect(error).toBeInstanceOf(UnusableDatabaseEscapeError)
+      if (!(error instanceof UnusableDatabaseEscapeError)) return
+      expect(error.variable).toBe('DIRECT_DATABASE_URL')
+      expect(error.url).toBe('file:./dev.db')
+      expect(error.message).toContain('DIRECT_DATABASE_URL')
+      expect(error.message).toContain('file:./dev.db')
+      expect(error.message).toMatch(/postgres:\/\//)
+      expect(error.message).toMatch(/unset it/)
+    }
+  })
+})