From b958d052876a3e3e6165537f460739427d420922 Mon Sep 17 00:00:00 2001
From: siddharthd <siddharthdevilz@gmail.com>
Date: Fri, 21 Aug 2026 00:52:54 +1000
Subject: [PATCH] test(db): resolve the integration DSN at run time instead of
 storing an IP (#161)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

setup-test-db.sh wrote .env.test once with the Postgres container's bridge IP
baked in, and Docker reassigns those on recreation. The file rotted silently:
its stored 172.22.0.18 belongs to **gitea** as of today, and an earlier drift
had the suite pointing its TRUNCATE at **postgres-pantry** — which it would
have wiped had the credentials happened to match.

Root-cause fix: stop persisting a host at all.

- scripts/resolve-test-db.sh re-derives the DSN from the live container and
  rewrites .env.test. Resolution order is TEST_DATABASE_URL, then the
  container's published host port, then the bridge IP. `docker port
  postgres-personal` reports 127.0.0.1:5432 since the 2026-08-17 PG17
  migration, so loopback is what gets used here — and a loopback address
  cannot be reassigned to another container, which is the whole point.
  (Note this supersedes the smarthome CLAUDE.md's "postgres-personal publishes
  no host port"; that is now stale.) The bridge-IP fallback is kept for a host
  where the port is not published, and the script never prints the password.
- Wired as `pretest:integration`, so npm runs it before every integration run.
  .env.test becomes a generated artifact rather than stored configuration.
- setup-test-db.sh now only creates the database and copies the schema, then
  delegates to the resolver.
- helpers.ts::assertTestDatabase stays and its message now points at the
  resolver rather than at `docker inspect`. It still earns its place: running
  vitest directly bypasses the pretest hook, and a stale .env.test would then
  still be read.

Docs: CLAUDE.md gains a Testing section carrying this, the prisma-generate
trap for a fresh checkout or worktree, and the frozen-clock rule for tests
whose subject is defined relative to "now". AGENTS.md updated for the same, and
for lint now being blocking.
---
 AGENTS.md                            |  9 ++--
 CLAUDE.md                            | 43 ++++++++++++++++++
 package.json                         |  1 +
 scripts/resolve-test-db.sh           | 66 ++++++++++++++++++++++++++++
 scripts/setup-test-db.sh             | 32 ++++++--------
 src/__tests__/integration/helpers.ts | 22 ++++++----
 6 files changed, 143 insertions(+), 30 deletions(-)
 create mode 100755 scripts/resolve-test-db.sh

diff --git a/AGENTS.md b/AGENTS.md
index 6f74b2d..6a83ded 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -13,10 +13,11 @@ Keep data flow consistent: API routes call query functions in `src/lib/queries.t
 - `npm ci` installs the locked dependency set (Node 22 is used in CI).
 - `npm run dev` starts the local Next.js development server.
 - `npm run build` creates a production build; `npm start` serves it.
-- `npm run lint` runs the Next.js ESLint configuration. Existing lint debt makes CI lint advisory, but new code should pass.
+- `npm run lint` runs the Next.js ESLint configuration. **Blocking in CI since #79** — the pre-existing debt is cleared, so it must stay at zero errors (warnings are still allowed).
+- `npx tsc --noEmit` typechecks, including the test files. Also blocking in CI.
 - `npm test` runs fast unit tests.
-- `npm run test:setup` prepares the PostgreSQL test database using `.env.test`.
-- `npm run test:integration` runs database-backed tests.
+- `npm run test:setup` creates/refreshes the `personal_test` database (run it once, and after any migration).
+- `npm run test:integration` runs database-backed tests. It re-resolves the DSN from the live container first (`scripts/resolve-test-db.sh`), so `.env.test` must never carry a hand-written host — see `CLAUDE.md` → Testing.
 - `npm run test:all` runs both test suites.
 
 ## Coding Style & Naming Conventions
@@ -33,4 +34,4 @@ Add schema changes as the next numbered `prisma/migrations/NNNN_description/migr
 
 ## Commits & Pull Requests
 
-History follows concise Conventional Commit-style subjects such as `feat(rules): preview rule changes`, `fix(trips): ...`, and `docs: ...`. Keep commits focused. Pull requests should explain behavior and data-model impact, link related issues, list validation commands, and include screenshots for UI changes. Ensure unit tests and the production build pass; call out any known lint warnings or migration steps.
+History follows concise Conventional Commit-style subjects such as `feat(rules): preview rule changes`, `fix(trips): ...`, and `docs: ...`. Keep commits focused. Pull requests should explain behavior and data-model impact, link related issues, list validation commands, and include screenshots for UI changes. Ensure lint, typecheck, unit tests and the production build pass; call out any migration steps.
diff --git a/CLAUDE.md b/CLAUDE.md
index c95d318..ddaace4 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -59,6 +59,49 @@ docker exec postgres-personal psql -U personal -d personal
 docker logs finance -f
 ```
 
+## Testing
+
+```bash
+npm test                  # unit — pure logic, no database
+npm run test:integration  # database-backed; resolves the DSN itself (see below)
+npm run test:all          # both
+npm run test:setup        # once, and after any migration: creates/refreshes personal_test
+npm run lint              # blocking in CI since #79 — 0 errors, warnings allowed
+npx tsc --noEmit          # blocking in CI; nothing else compiles the test files
+```
+
+**The integration DSN is resolved at run time — never hand-write a host into
+`.env.test`.** `scripts/resolve-test-db.sh` runs automatically as
+`pretest:integration` and rewrites that file from the live container before
+every run. It prefers the container's **published host port** (`docker port
+postgres-personal` → `127.0.0.1:5432`, published since the 2026-08-17 PG17
+migration) and falls back to the bridge IP only where that is absent. Set
+`TEST_DATABASE_URL` to override it entirely.
+
+This replaces the old "refresh the container IP" instruction, which described
+the defect rather than a workaround (#161). `setup-test-db.sh` used to bake the
+bridge IP into `.env.test` once, and Docker reassigns those on recreation: by
+2026-08-21 the stored `172.22.0.18` belonged to **gitea**, and an earlier drift
+had pointed the suite at **postgres-pantry** — whose database it would have
+TRUNCATEd had the credentials happened to match. A loopback address cannot be
+reassigned to another container, which is why the published port wins.
+
+**Two guards, and both matter.** `helpers.ts::assertTestDatabase` refuses to
+truncate anything whose `current_database()` is not `personal_test`, and the
+suite runs `fileParallelism: false` because every file shares that one database
+and `resetDB()` TRUNCATEs it.
+
+**A fresh checkout or worktree needs `npx prisma generate` first.** The client is
+generated into the gitignored `src/generated/prisma`, so without it every test
+that reaches `src/lib/db.ts` fails on a missing package — which reads as a code
+error rather than a setup one. CI does this explicitly for the same reason.
+
+**Freeze the clock in any test whose subject is defined relative to "now".**
+`analytics-priority0.test.ts` uses `vi.useFakeTimers({ toFake: ["Date"] })` —
+only `Date`, because `pg` needs real timers for connection handling. Without it
+the fixtures drift out of the routes' rolling windows over time and the
+assertions quietly stop testing anything.
+
 ## Architecture
 
 ### Key Files
diff --git a/package.json b/package.json
index f76e951..66546e0 100644
--- a/package.json
+++ b/package.json
@@ -10,6 +10,7 @@
     "test": "vitest run --config vitest.config.ts",
     "test:watch": "vitest --config vitest.config.ts",
     "test:setup": "bash scripts/setup-test-db.sh",
+    "pretest:integration": "bash scripts/resolve-test-db.sh",
     "test:integration": "vitest run --config vitest.integration.config.ts",
     "test:all": "npm test && npm run test:integration"
   },
diff --git a/scripts/resolve-test-db.sh b/scripts/resolve-test-db.sh
new file mode 100755
index 0000000..0299849
--- /dev/null
+++ b/scripts/resolve-test-db.sh
@@ -0,0 +1,66 @@
+#!/bin/bash
+# Resolve the integration-test DSN at run time and (re)write .env.test.
+#
+# Why this exists (#161): .env.test used to be written ONCE by
+# setup-test-db.sh with the Postgres container's bridge IP baked in. Container
+# IPs are reassigned on recreation, so the file rotted silently — by
+# 2026-08-21 its 172.22.0.18 belonged to *gitea*, and an earlier drift had the
+# suite pointing its TRUNCATE at postgres-pantry. It only failed safe because
+# the credentials happened not to match.
+#
+# The fix is to stop persisting a host at all: this runs as `pretest:integration`,
+# so the DSN is re-derived from the live container immediately before every run.
+#
+# Resolution order:
+#   1. An explicit TEST_DATABASE_URL in the environment wins (CI, remote DB).
+#   2. The container's published host port (`docker port`) — postgres-personal
+#      publishes 127.0.0.1:5432 since the 2026-08-17 PG17 migration. Preferred
+#      because a loopback address cannot be reassigned to another container.
+#   3. The container's bridge IP, for a host where the port is not published.
+#
+# helpers.ts::assertTestDatabase is the backstop: it refuses to TRUNCATE
+# anything whose current_database() is not `personal_test`.
+
+set -euo pipefail
+
+PG_CONTAINER=${PG_CONTAINER:-postgres-personal}
+PG_USER=${PG_USER:-personal}
+PG_PASS=${PG_PASS:-personalpassword123}
+TEST_DB=${TEST_DB:-personal_test}
+
+ENV_FILE="$(cd "$(dirname "$0")/.." && pwd)/.env.test"
+
+if [ -n "${TEST_DATABASE_URL:-}" ]; then
+  DSN="$TEST_DATABASE_URL"
+  SOURCE="TEST_DATABASE_URL"
+else
+  if ! docker inspect "$PG_CONTAINER" >/dev/null 2>&1; then
+    echo "ERROR: container '$PG_CONTAINER' not found. Start it, or set TEST_DATABASE_URL." >&2
+    exit 1
+  fi
+
+  # `docker port` prints e.g. "127.0.0.1:5432" (possibly several lines).
+  PUBLISHED=$(docker port "$PG_CONTAINER" 5432/tcp 2>/dev/null | head -1 || true)
+
+  if [ -n "$PUBLISHED" ]; then
+    HOSTPORT="$PUBLISHED"
+    SOURCE="published port"
+  else
+    IP=$(docker inspect "$PG_CONTAINER" \
+      --format '{{range .NetworkSettings.Networks}}{{.IPAddress}} {{end}}' 2>/dev/null \
+      | tr ' ' '\n' | grep -v '^$' | head -1)
+    if [ -z "$IP" ]; then
+      echo "ERROR: '$PG_CONTAINER' has neither a published 5432 nor a bridge IP." >&2
+      exit 1
+    fi
+    HOSTPORT="$IP:5432"
+    SOURCE="bridge IP"
+  fi
+
+  DSN="postgresql://$PG_USER:$PG_PASS@$HOSTPORT/$TEST_DB"
+fi
+
+printf 'DATABASE_URL=%s\n' "$DSN" > "$ENV_FILE"
+
+# Never print the password.
+echo "resolve-test-db: ${DSN%%://*}://***@${DSN#*@}  (via $SOURCE)"
diff --git a/scripts/setup-test-db.sh b/scripts/setup-test-db.sh
index 1471b32..da47659 100755
--- a/scripts/setup-test-db.sh
+++ b/scripts/setup-test-db.sh
@@ -1,25 +1,24 @@
 #!/bin/bash
-# Creates the personal_test database and writes .env.test
-# Run once before integration tests: npm run test:setup
+# Creates the personal_test database and copies the production schema into it.
+# Run once (and after any migration) before integration tests: npm run test:setup
+#
+# It no longer writes a host into .env.test. That was the #161 defect: the file
+# was written once with the container's bridge IP and then rotted as the IP was
+# reassigned. scripts/resolve-test-db.sh now re-derives the DSN before every
+# integration run (wired as `pretest:integration`), and this script just calls it
+# at the end so a fresh setup leaves a usable .env.test behind.
 
 set -e
 
-PG_CONTAINER=postgres-personal
-PG_USER=personal
-PG_PASS=personalpassword123
-TEST_DB=personal_test
+PG_CONTAINER=${PG_CONTAINER:-postgres-personal}
+PG_USER=${PG_USER:-personal}
+TEST_DB=${TEST_DB:-personal_test}
 
-# Discover the container's bridge IP (accessible from the host on Linux)
-PG_HOST=$(docker inspect "$PG_CONTAINER" \
-  --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' 2>/dev/null | head -1)
-
-if [ -z "$PG_HOST" ]; then
+if ! docker inspect "$PG_CONTAINER" >/dev/null 2>&1; then
   echo "ERROR: Could not find container '$PG_CONTAINER'"
   exit 1
 fi
 
-echo "Postgres container at $PG_HOST:5432"
-
 # Create test database (ignore error if it already exists)
 docker exec "$PG_CONTAINER" psql -U "$PG_USER" -d postgres \
   -c "CREATE DATABASE $TEST_DB;" 2>/dev/null || true
@@ -31,9 +30,6 @@ docker exec "$PG_CONTAINER" pg_dump -U "$PG_USER" --schema-only "$PG_USER" \
 
 echo "Schema ready."
 
-# Write .env.test
-cat > "$(dirname "$0")/../.env.test" << EOF
-DATABASE_URL=postgresql://$PG_USER:$PG_PASS@$PG_HOST:5432/$TEST_DB
-EOF
+bash "$(dirname "$0")/resolve-test-db.sh"
 
-echo ".env.test written — integration tests can now run with: npm run test:integration"
+echo "Integration tests can now run with: npm run test:integration"
diff --git a/src/__tests__/integration/helpers.ts b/src/__tests__/integration/helpers.ts
index be7b254..f57cac9 100644
--- a/src/__tests__/integration/helpers.ts
+++ b/src/__tests__/integration/helpers.ts
@@ -30,12 +30,17 @@ export function mockDbWithPool(p: Pool) {
 /**
  * Refuse to truncate anything that is not the test database.
  *
- * `DATABASE_URL` in `.env.test` names the Postgres container by IP, and
- * container IPs move on recreation: 172.22.0.47 stopped being
- * `postgres-personal` and became `postgres-pantry`, so the suite spent a while
- * pointing its TRUNCATE at another app's database. It only failed safe because
- * the credentials happened not to match — had they matched, this would have
- * wiped pantry-app.
+ * `.env.test` used to name the Postgres container by bridge IP, and Docker
+ * reassigns those on recreation: 172.22.0.47 stopped being `postgres-personal`
+ * and became `postgres-pantry`, so the suite spent a while pointing its
+ * TRUNCATE at another app's database. It only failed safe because the
+ * credentials happened not to match — had they matched, this would have wiped
+ * pantry-app.
+ *
+ * `scripts/resolve-test-db.sh` (wired as `pretest:integration`) now re-derives
+ * the DSN from the live container before every run, preferring the published
+ * loopback port. This check stays anyway: running vitest directly bypasses the
+ * pretest hook, and a stale `.env.test` would still be read.
  *
  * Checked once per process, before the first truncate.
  */
@@ -47,8 +52,9 @@ async function assertTestDatabase(pool: Pool) {
   if (db !== "personal_test") {
     throw new Error(
       `Refusing to truncate: connected to "${db}", expected "personal_test". ` +
-      `Check DATABASE_URL in .env.test — the Postgres container IP may have changed ` +
-      `(docker inspect postgres-personal --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}').`
+      `Re-resolve the DSN with \`bash scripts/resolve-test-db.sh\` (or just run ` +
+      `\`npm run test:integration\`, which does it for you), and run ` +
+      `\`npm run test:setup\` if personal_test does not exist yet.`
     );
   }
   targetVerified = true;