From 1eda6678bcf39bd62e5e86d611550e0e5abe96a3 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sat, 13 Jun 2026 16:56:55 -0400 Subject: [PATCH 1/5] chore(quick-260613-ndv-01): add globalSetup for familysync_test isolation - Create apps/api/test/global-setup.ts: root-provisions + grants + migrates familysync_test (local only); no-op when process.env.CI is truthy (T-ndv-04) - Update apps/api/vitest.config.ts: wire globalSetup; add CI-gated test.env override (DB_NAME=familysync_test, DB_HOST) so workers never touch dev DB --- apps/api/test/global-setup.ts | 103 ++++++++++++++++++++++++++++++++++ apps/api/vitest.config.ts | 14 +++++ 2 files changed, 117 insertions(+) create mode 100644 apps/api/test/global-setup.ts diff --git a/apps/api/test/global-setup.ts b/apps/api/test/global-setup.ts new file mode 100644 index 0000000..d6ffd01 --- /dev/null +++ b/apps/api/test/global-setup.ts @@ -0,0 +1,103 @@ +/** + * Vitest globalSetup — provisions and migrates familysync_test for local runs. + * + * Runs ONCE in the main vitest process before any test file. + * Under CI (process.env.CI truthy), returns immediately — CI provisions its own + * `familysync` service DB and must not be touched. + * + * Local flow: + * 1. Root-connects → CREATE DATABASE IF NOT EXISTS familysync_test + * 2. GRANT ALL PRIVILEGES ON familysync_test.* to the app user + * 3. App-user connects → drizzle migrate() applies committed SQL migrations + * + * Security: + * - Root password read from process.env.DB_ROOT_PASSWORD (default 'root' matches dev compose only) + * - App user validated against /^[A-Za-z0-9_]+$/ before GRANT interpolation (T-ndv-03) + * - This file never runs in CI (T-ndv-04) + */ + +import mysql from 'mysql2/promise'; +import { drizzle } from 'drizzle-orm/mysql2'; +import { migrate } from 'drizzle-orm/mysql2/migrator'; +import { fileURLToPath } from 'node:url'; + +const TEST_DB = 'familysync_test'; + +export default async function setup(): Promise { + const isCI = !!process.env.CI; + if (isCI) { + // CI provisions its own database via the mariadb service + db:migrate step. + // Do nothing here so the CI flow is completely unaffected (T-ndv-04). + return; + } + + // ── Connection params (dev defaults match docker-compose.dev.yml) ────────── + const host = process.env.DB_HOST ?? '127.0.0.1'; + const port = Number(process.env.DB_PORT ?? 3306); + const appUser = process.env.DB_USER ?? 'familysync'; + const appPassword = process.env.DB_PASSWORD ?? ''; + + // ── Root creds — read from env; dev default 'root' only for convenience ──── + const rootUser = process.env.DB_ROOT_USER ?? 'root'; + const rootPassword = process.env.DB_ROOT_PASSWORD ?? 'root'; + + // ── Security: validate appUser before interpolating into GRANT ────────────── + if (!/^[A-Za-z0-9_]+$/.test(appUser)) { + throw new Error( + `[global-setup] DB_USER '${appUser}' contains characters not allowed in a GRANT statement. Aborting.`, + ); + } + + // ── Step 1–3: Root connection — CREATE DATABASE + GRANT ──────────────────── + const rootConn = await mysql.createConnection({ + host, + port, + user: rootUser, + password: rootPassword, + // No database selected — we are creating one + }); + + try { + await rootConn.query(`CREATE DATABASE IF NOT EXISTS \`${TEST_DB}\``); + + // GRANT does not accept parameterised identifiers; user is validated above (T-ndv-03). + // Try @'%' first (Docker/network access); fall back to @'localhost' if the user was + // created with a localhost host qualifier. + try { + await rootConn.query( + `GRANT ALL PRIVILEGES ON \`${TEST_DB}\`.* TO '${appUser}'@'%'`, + ); + } catch { + // If @'%' fails (user only exists as @'localhost'), try that form. + await rootConn.query( + `GRANT ALL PRIVILEGES ON \`${TEST_DB}\`.* TO '${appUser}'@'localhost'`, + ); + } + + await rootConn.query('FLUSH PRIVILEGES'); + } finally { + await rootConn.end(); + } + + // ── Step 4: Apply committed migrations to familysync_test ────────────────── + const appConn = await mysql.createConnection({ + host, + port, + user: appUser, + password: appPassword, + database: TEST_DB, + multipleStatements: true, // required by drizzle migrator for multi-statement SQL files + }); + + try { + const db = drizzle({ client: appConn, mode: 'default' }); + const migrationsFolder = fileURLToPath( + new URL('../src/db/migrations', import.meta.url), + ); + await migrate(db, { migrationsFolder }); + } finally { + await appConn.end(); + } + + console.log('[global-setup] provisioned + migrated familysync_test'); +} diff --git a/apps/api/vitest.config.ts b/apps/api/vitest.config.ts index 608eb54..f6c17e1 100644 --- a/apps/api/vitest.config.ts +++ b/apps/api/vitest.config.ts @@ -1,14 +1,28 @@ import { defineConfig } from 'vitest/config'; +// Under CI the job-level env already sets DB_NAME=familysync and DB_HOST=mariadb +// (the ephemeral service container). Do not override those — CI provisions and +// migrates its own DB via the db:migrate step (T-ndv-04). +// Locally, force workers to connect to the isolated test DB so the dev +// `familysync` DB is never mutated by the test suite (T-ndv-02). +const isCI = !!process.env.CI; + export default defineConfig({ test: { environment: 'node', globals: true, + globalSetup: ['./test/global-setup.ts'], setupFiles: ['./test/setup.ts'], // Disable parallel file execution so concurrent DB tests do not interfere // via the shared MariaDB. The global afterEach in test/setup.ts truncates // list tables; running test files in parallel causes FK violations when one // file's afterEach deletes rows that another file's test is still using. fileParallelism: false, + env: isCI + ? {} + : { + DB_NAME: 'familysync_test', + DB_HOST: process.env.DB_HOST ?? '127.0.0.1', + }, }, }); From f39bd308b29dd514fdb80fc5e3f288500a9bab07 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sat, 13 Jun 2026 17:02:50 -0400 Subject: [PATCH 2/5] chore(quick-260613-ndv-02): clean-slate comment in setup.ts + README local-test docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update apps/api/test/setup.ts header: clarify tests run against familysync_test (provisioned by global-setup.ts), document users-cleanup decision (intact across tests), and note CI-vs-local env difference - Add apps/api/README.md "Running API tests locally" section: documents the test DB isolation, run command, DB_ROOT_PASSWORD requirement, and CI no-op behaviour - Fix apps/api/test/global-setup.ts: switch from drizzle({ client, mode }) to drizzle(pool, { mode }) — drizzle-orm@0.45.2 isConfig() has a tautological OR in the `mode` branch that always returns false, causing the combined-config form to pass the config object as the client (client.query is not a function); two-arg form routes correctly; 244/244 tests pass against familysync_test --- apps/api/README.md | 20 ++++++++++++++++++++ apps/api/test/global-setup.ts | 16 +++++++++++++--- apps/api/test/setup.ts | 27 +++++++++++++++++---------- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/apps/api/README.md b/apps/api/README.md index 6ac07dd..18aa160 100644 --- a/apps/api/README.md +++ b/apps/api/README.md @@ -144,6 +144,26 @@ pnpm --filter @familysync/api typecheck Integration tests that hit MariaDB require a running dev DB with `DB_HOST=127.0.0.1` and credentials from your `.env`. See [../../docs/TESTING.md](../../docs/TESTING.md) for the full setup. +## Running API tests locally + +Local test runs use a dedicated `familysync_test` database so the dev `familysync` database is never mutated. `test/global-setup.ts` creates and migrates `familysync_test` automatically on the first run. + +**Prerequisites:** + +- Dev MariaDB running and port-bound (`127.0.0.1:3306`) — start with `docker compose -f docker-compose.yml -f docker-compose.dev.yml up -d mariadb` +- `.env` sourced in your shell (provides `DB_PASSWORD`, `DB_ROOT_PASSWORD`, and other credentials) + +**Run command:** + +```bash +set -a; source .env; set +a +DB_HOST=127.0.0.1 pnpm --filter @familysync/api test +``` + +`DB_ROOT_PASSWORD` must be set in `.env` for the one-time `CREATE DATABASE` / `GRANT` that provisions `familysync_test`. Subsequent runs skip the provisioning step if the database already exists (`CREATE DATABASE IF NOT EXISTS`). + +**CI is unaffected.** `test/global-setup.ts` returns immediately when `CI` is set (the CI `api` job provisions its own `familysync` service DB and runs `db:migrate` before the test step). The `test.env` DB override in `vitest.config.ts` is also a no-op under CI. + ## Further reading - [Architecture](../../docs/ARCHITECTURE.md) — system overview and component diagram diff --git a/apps/api/test/global-setup.ts b/apps/api/test/global-setup.ts index d6ffd01..ec67000 100644 --- a/apps/api/test/global-setup.ts +++ b/apps/api/test/global-setup.ts @@ -80,23 +80,33 @@ export default async function setup(): Promise { } // ── Step 4: Apply committed migrations to familysync_test ────────────────── - const appConn = await mysql.createConnection({ + // Use a Pool (not a Connection) — drizzle-orm/mysql2 session.all() calls + // client.execute() and transaction() calls client.getConnection(), both of + // which are pool methods. createPool with connectionLimit:1 is the minimal form. + const appPool = mysql.createPool({ host, port, user: appUser, password: appPassword, database: TEST_DB, + connectionLimit: 1, multipleStatements: true, // required by drizzle migrator for multi-statement SQL files }); try { - const db = drizzle({ client: appConn, mode: 'default' }); + // Pass pool as the first arg + config as the second. Do NOT use the + // { client: pool, mode } combined-config form — drizzle-orm@0.45.2 has a + // bug in isConfig() where the `mode` branch always returns false (its OR + // condition is a tautology), so the combined form falls through to + // construct({ client, mode }, undefined) and the session client becomes + // the plain config object (no .query()). The two-arg form is safe. + const db = drizzle(appPool, { mode: 'default' }); const migrationsFolder = fileURLToPath( new URL('../src/db/migrations', import.meta.url), ); await migrate(db, { migrationsFolder }); } finally { - await appConn.end(); + await appPool.end(); } console.log('[global-setup] provisioned + migrated familysync_test'); diff --git a/apps/api/test/setup.ts b/apps/api/test/setup.ts index 2787a5d..f6af415 100644 --- a/apps/api/test/setup.ts +++ b/apps/api/test/setup.ts @@ -1,18 +1,25 @@ /** - * Vitest global test setup for apps/api. + * Vitest per-file test setup for apps/api. * - * Establishes shared test infrastructure for API tests: - * - DB pool access via the existing client.ts (DB_HOST/DB_NAME from env) - * - Per-test cleanup for list tables (truncate between tests so state is isolated) + * Tests now run against the isolated `familysync_test` database, which is + * auto-provisioned and migrated by `test/global-setup.ts` before the suite + * starts. The dev `familysync` database is never touched by a local test run. * - * Usage: - * This file is referenced in vitest.config.ts via test.setupFiles. - * Pure-logic tests (listEmitter, fractional rank) do NOT require DB — the - * cleanup function is a no-op when the tables are empty. + * Cleanup strategy: + * - afterEach truncates list/push tables in FK-safe order so each test + * starts with a clean slate for those tables. + * - `users` is intentionally left intact across tests within a single run. + * Many tests seed user id=1 once and reuse it; deleting users between tests + * would break FK-dependent rows mid-suite. The globalSetup provides a fresh + * migrated `familysync_test` at run start, so `users` starts empty and any + * seed inserted by the first test that needs it persists for the session. + * If a specific test leaks `users` rows that affect another test, scope a + * targeted delete inside that test's own beforeEach/afterEach instead. * * Environment: - * Set DB_HOST, DB_USER, DB_PASSWORD, DB_NAME in the test environment. - * Tests run against a local MariaDB dev database; production data is never touched. + * DB_HOST, DB_USER, DB_PASSWORD, and DB_NAME=familysync_test are injected by + * vitest.config.ts (test.env) for local runs. Under CI, DB_NAME=familysync is + * preserved from the job-level env and globalSetup is a no-op. */ import { afterEach } from 'vitest'; From 24cb7569bf5c6cba0f5faa1268d228dcb893d712 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sat, 13 Jun 2026 17:08:46 -0400 Subject: [PATCH 3/5] fix(quick-260613-ndv): reset familysync_test each run (truncate-all) CREATE DATABASE IF NOT EXISTS reuses the prior run's data, so the test DB would itself accumulate users run-over-run (the same flaky list_shares fan-out the dev DB suffered). Truncate every table (except the drizzle migration ledger) at globalSetup start, FK-safe, so each run is a clean deterministic slate. Verified: familysync_test users 186 -> 93 across a run (reset, not doubled); dev familysync untouched at 3. --- apps/api/test/global-setup.ts | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/apps/api/test/global-setup.ts b/apps/api/test/global-setup.ts index ec67000..0249048 100644 --- a/apps/api/test/global-setup.ts +++ b/apps/api/test/global-setup.ts @@ -105,9 +105,32 @@ export default async function setup(): Promise { new URL('../src/db/migrations', import.meta.url), ); await migrate(db, { migrationsFolder }); + + // Clean slate each run: truncate every table (except drizzle's migration + // ledger) so familysync_test does NOT accumulate rows across runs. Without + // this, CREATE DATABASE IF NOT EXISTS reuses the prior run's data and the + // users table grows unbounded — re-creating the slow/flaky list_shares + // fan-out the dev DB used to suffer, just in the test DB. FK-safe via the + // session FOREIGN_KEY_CHECKS toggle (scoped to this one connection). + const conn = await appPool.getConnection(); + try { + const [rows] = await conn.query( + `SELECT table_name AS t FROM information_schema.tables + WHERE table_schema = ? AND table_type = 'BASE TABLE' + AND table_name <> '__drizzle_migrations'`, + [TEST_DB], + ); + await conn.query('SET FOREIGN_KEY_CHECKS = 0'); + for (const { t } of rows as Array<{ t: string }>) { + await conn.query(`TRUNCATE TABLE \`${t}\``); + } + await conn.query('SET FOREIGN_KEY_CHECKS = 1'); + } finally { + conn.release(); + } } finally { await appPool.end(); } - console.log('[global-setup] provisioned + migrated familysync_test'); + console.log('[global-setup] provisioned + migrated + reset familysync_test'); } From e687cb96e7f8517e165276a61c500a755483f7e5 Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sat, 13 Jun 2026 17:09:55 -0400 Subject: [PATCH 4/5] docs(quick-260613-ndv): plan + summary + state for test-DB isolation --- .planning/STATE.md | 3 +- .../260613-ndv-PLAN.md | 156 ++++++++++++++++++ .../260613-ndv-SUMMARY.md | 117 +++++++++++++ 3 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 .planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-PLAN.md create mode 100644 .planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-SUMMARY.md diff --git a/.planning/STATE.md b/.planning/STATE.md index 29bcb3a..7105e07 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -28,7 +28,7 @@ See: .planning/PROJECT.md (updated 2026-06-10) Phase: 13 Plan: Not started Status: Phase complete — ready for verification -Last activity: 2026-06-13 +Last activity: 2026-06-13 - Completed quick task 260613-ndv: isolated local apps/api tests to familysync_test (dev DB no longer polluted) ### ✅ Resolved Checkpoint — Phase 15 Plan 15-03 Task 2 (human-action) @@ -226,6 +226,7 @@ Recent decisions affecting current work: | 260611-tfc | Fix WR-01 (13-REVIEW): apps/pwa/src/sw.ts notificationclick openWindow fallback was unreachable when client.focus() rejects (window closed between matchAll/focus) or client.navigate() resolves null — chained a navigate-result check + a .catch, both falling through to self.clients.openWindow(url). lint/format:check/typecheck green, build emits sw.js, 191/191 pwa tests | 2026-06-12 | af78ccc | Verified | [260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi](./quick/260611-tfc-fix-wr-01-sw-ts-notificationclick-openwi/) | | 260613-dmw | Exclude `.gitea/**` from the CI `changes` `code` paths-filter so workflow-only PRs skip the heavy api/harness jobs (treated like docs) while fast-checks + gate still run. Single `- '!.gitea/**'` negation appended after the yml/yaml globs (index 11 vs 5). Rides along on the Phase 16 branch / PR #15. | 2026-06-13 | 2d329a9 | | [260613-dmw-exclude-gitea-workflow-config-changes-fr](./quick/260613-dmw-exclude-gitea-workflow-config-changes-fr/) | | 260613-fp9 | `.gitea`/`.planning`-only pushes to main no longer trigger the Docker publish — added `paths-ignore: ['.gitea/**', '.planning/**']` under `on.push` in `.gitea/workflows/publish.yml` (skips only when EVERY changed file matches; mixed code+docs pushes still publish). `.dockerignore` already excludes `.planning` so the image is byte-identical. Done in isolated worktree (phase-10 agent held main tree). | 2026-06-13 | cd5a88c | | [260613-fp9-gitea-and-planning-pushes-should-not-tri](./quick/260613-fp9-gitea-and-planning-pushes-should-not-tri/) | +| 260613-ndv | Isolate local apps/api integration tests to a dedicated `familysync_test` DB so test runs stop polluting the dev `familysync` DB. New CI-gated vitest globalSetup root-provisions (CREATE DATABASE + GRANT) + migrates + truncate-resets `familysync_test` each run; `vitest.config.ts` forces `DB_NAME=familysync_test` for local workers (no-op under CI, so CI's `familysync` service DB + db:migrate are untouched). Verified: dev `familysync` users stays 3 across a run, `familysync_test` resets (186→93, not doubled), 244/244 tests pass (flaky list_shares timeout gone), typecheck 0. Branch off main. | 2026-06-13 | 07d5161 | Verified | [260613-ndv-wire-apps-api-integration-tests-to-a-ded](./quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/) | ## Deferred Items diff --git a/.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-PLAN.md b/.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-PLAN.md new file mode 100644 index 0000000..808e288 --- /dev/null +++ b/.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-PLAN.md @@ -0,0 +1,156 @@ +--- +phase: quick-260613-ndv +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - apps/api/test/global-setup.ts + - apps/api/vitest.config.ts + - apps/api/test/setup.ts + - apps/api/README.md +autonomous: true +requirements: [QUICK-260613-ndv] +must_haves: + truths: + - "Local `pnpm --filter @familysync/api test` runs against `familysync_test`, never the dev `familysync` DB" + - "After a full local test run, the dev `familysync` DB `users` row count is unchanged" + - "The new globalSetup is a no-op under CI (process.env.CI truthy), so CI keeps DB_NAME=familysync and its own migrate flow" + - "`familysync_test` is auto-provisioned (CREATE DATABASE + GRANT to the app user) and migrated before tests run" + - "`pnpm --filter @familysync/api typecheck` exits 0" + artifacts: + - path: "apps/api/test/global-setup.ts" + provides: "vitest globalSetup: root-provisions + migrates familysync_test (local only), no-op under CI" + min_lines: 40 + - path: "apps/api/vitest.config.ts" + provides: "globalSetup wired + CI-gated test.env.DB_NAME/DB_HOST override" + contains: "globalSetup" + key_links: + - from: "apps/api/vitest.config.ts" + to: "apps/api/test/global-setup.ts" + via: "test.globalSetup config field" + pattern: "globalSetup" + - from: "apps/api/vitest.config.ts" + to: "apps/api/src/db/client.ts" + via: "test.env.DB_NAME=familysync_test sets the env the pool reads at module-eval" + pattern: "DB_NAME" +--- + + +Wire the apps/api integration test suite to a dedicated LOCAL test database (`familysync_test`) so local test runs stop polluting the dev DB (`familysync`). A vitest `globalSetup` provisions and migrates `familysync_test` once per run (local only), and vitest's `test.env` forces `DB_NAME=familysync_test` for the test workers — both CI-gated so the existing CI `api` job (its own `familysync` service DB + `db:migrate`) is untouched. + +Purpose: The operator currently runs api tests against the live dev DB, mutating real dev rows (lists, users) and causing the flaky `lists.test.ts > re-populates list_shares` timeout against dirty state. Isolating to `familysync_test` makes local runs deterministic and non-destructive. + +Output: `apps/api/test/global-setup.ts` (new), `apps/api/vitest.config.ts` (globalSetup + CI-gated env override), an optional `test/setup.ts` clean-slate adjustment, and a short README note on running api tests locally. + + + +@$HOME/.claude/gsd-core/workflows/execute-plan.md +@$HOME/.claude/gsd-core/templates/summary.md + + + +@.planning/STATE.md +@apps/api/vitest.config.ts +@apps/api/test/setup.ts +@apps/api/src/db/client.ts +@apps/api/drizzle.config.ts +@apps/api/package.json +@docker-compose.dev.yml +@docker-compose.yml +@.gitea/workflows/ci.yml + +Already-established facts (do NOT re-derive): +- `src/db/client.ts` builds the pool from `process.env` at module-eval time (no dotenv in code): host=DB_HOST??'localhost', port=DB_PORT??3306, user=DB_USER??'familysync', password=DB_PASSWORD??'', database=DB_NAME??'familysync'. So forcing the test DB = ensuring the test workers see `DB_NAME=familysync_test` BEFORE client.ts is imported. +- vitest config is the right place for the env override: `test.env` is applied to the test worker processes before any module (including client.ts) loads. The config file itself runs in the main process where `process.env.CI` is readable — so the override value can be computed conditionally at config-load time. +- `globalSetup` runs ONCE in the main vitest process before any test file. It is the place to root-provision + migrate `familysync_test`. globalSetup runs in a SEPARATE process from the test workers, so mutating `process.env.DB_NAME` inside globalSetup does NOT reach the workers — the worker DB_NAME must come from `test.env` in the config, not from globalSetup. +- Dev MariaDB is reachable at 127.0.0.1:3306. The `familysync` app user has ALL on `familysync.*` but only USAGE on `*.*` — it CANNOT create databases. Provisioning `familysync_test` needs ROOT. Dev compose root password is `${DB_ROOT_PASSWORD}` from `.env` (docker-compose.yml line 41); the operator's run command does `set -a; source .env; set +a`, so `DB_ROOT_PASSWORD` is already in the shell env for local runs. +- Migrations: committed SQL lives in `apps/api/src/db/migrations/` (`0000_baseline.sql` + `meta/`). `drizzle-orm/mysql2/migrator`'s `migrate()` is installed (drizzle-orm@0.45.2) and applies that folder programmatically. +- Tests live in `apps/api/tests/` (NOT `src/`). Per-test cleanup in `test/setup.ts` truncates list/push tables via `afterEach` (does NOT touch `users`). +- CI `api` job: sets job-level env `DB_NAME=familysync` against a fresh `mariadb:11` service, runs `db:migrate` itself, then `pnpm --filter @familysync/api test`. CI is already isolated. It must keep `DB_NAME=familysync` and skip the local provisioning entirely — gate via `process.env.CI`. + + + + + + Task 1: Add CI-gated globalSetup that provisions and migrates familysync_test + apps/api/test/global-setup.ts, apps/api/vitest.config.ts + +Create `apps/api/test/global-setup.ts` exporting a default async `setup()` function (vitest globalSetup contract). At the top, read `const isCI = !!process.env.CI;` and, if `isCI` is truthy, `return` immediately — CI provisions and migrates its own `familysync` service DB and must not be touched. + +For the local (non-CI) branch: + 1. Resolve the test DB name from a single constant `const TEST_DB = process.env.DB_NAME ?? 'familysync_test';` — but DO NOT rely on the shell DB_NAME here; default to the literal `'familysync_test'` so this file is self-describing (the worker override is set in vitest.config.ts, see Task wiring below). Use `const TEST_DB = 'familysync_test';`. + 2. Read DB connection params from env with dev defaults that match docker-compose: host `process.env.DB_HOST ?? '127.0.0.1'`, port `Number(process.env.DB_PORT ?? 3306)`, app user `process.env.DB_USER ?? 'familysync'`, app password `process.env.DB_PASSWORD ?? ''`. + 3. Read ROOT creds from env with dev defaults: `const rootUser = process.env.DB_ROOT_USER ?? 'root';` and `const rootPassword = process.env.DB_ROOT_PASSWORD ?? 'root';`. NEVER hardcode a production secret — `DB_ROOT_PASSWORD` is already in the operator's shell (sourced from `.env`); the `'root'` default matches the dev compose convention only. + 4. Open a ROOT connection via `mysql.createConnection` from `mysql2/promise` (host/port from step 2, user/password = root creds, NO database selected). Run, in order: `CREATE DATABASE IF NOT EXISTS \`familysync_test\``; `GRANT ALL PRIVILEGES ON \`familysync_test\`.* TO ?@'%'` binding the app user (use a parameterized identifier carefully — GRANT does not accept `?` for identifiers, so build the statement as `` `GRANT ALL PRIVILEGES ON \`familysync_test\`.* TO '${appUser}'@'%'` `` after validating `appUser` matches `/^[A-Za-z0-9_]+$/` to avoid injection); then `FLUSH PRIVILEGES`. Close the root connection. Wrap GRANT in try/catch — if the app user was created as `@'localhost'` rather than `@'%'`, also attempt the `@'localhost'` grant; ignore "operation not allowed" only if the user already has the privilege. + 5. Apply committed migrations to `familysync_test` programmatically: open a `mysql.createConnection` (or `createPool`) as the APP user against database `familysync_test`, wrap with `drizzle(client, { mode: 'default' })` from `drizzle-orm/mysql2`, and call `await migrate(db, { migrationsFolder: })` from `drizzle-orm/mysql2/migrator`. Resolve the migrations folder relative to this file using `fileURLToPath(new URL('../src/db/migrations', import.meta.url))` so it is path-independent of cwd. Close the connection/pool after migrate resolves. + 6. Log a single line `console.log('[global-setup] provisioned + migrated familysync_test')` so the operator can confirm the local branch ran. +Use `import mysql from 'mysql2/promise'`, `import { drizzle } from 'drizzle-orm/mysql2'`, `import { migrate } from 'drizzle-orm/mysql2/migrator'`, `import { fileURLToPath } from 'node:url'`. No fenced code in this plan — follow the named imports and statement order above. + +Then wire it into `apps/api/vitest.config.ts`: + - Add `globalSetup: ['./test/global-setup.ts']` to the `test` block. + - Add a CI-gated env override so the test WORKERS connect to `familysync_test` locally but keep CI's values: at the top of the config module compute `const isCI = !!process.env.CI;` and set `env: isCI ? {} : { DB_NAME: 'familysync_test', DB_HOST: process.env.DB_HOST ?? '127.0.0.1' }` inside `test`. This is the load-bearing override — `test.env` is applied to worker processes before `client.ts` is imported, so the pool reads `familysync_test`. Under CI the override is empty, so the job-level `DB_NAME=familysync` and `DB_HOST=mariadb` are preserved untouched. Keep the existing `fileParallelism: false` and `setupFiles: ['./test/setup.ts']`. + + + cd /home/luc/Projects/familysync && grep -q "globalSetup" apps/api/vitest.config.ts && grep -q "familysync_test" apps/api/vitest.config.ts && grep -q "process.env.CI" apps/api/test/global-setup.ts && grep -q "migrate(" apps/api/test/global-setup.ts && pnpm --filter @familysync/api typecheck + + globalSetup file exists, returns early under CI, root-provisions + grants + migrates familysync_test locally; vitest.config.ts wires globalSetup and applies a CI-gated DB_NAME=familysync_test worker override; typecheck exits 0. + + + + Task 2: Make per-test cleanup clean-slate (fold users) and prove isolation end-to-end + apps/api/test/setup.ts, apps/api/README.md + +The flaky `lists.test.ts > re-populates list_shares` times out against dirty/leftover state. Make the test DB deterministic between tests. In `apps/api/test/setup.ts`, keep the existing `afterEach` FK-safe truncation of `listItems`, `listShares`, `pushSubscriptions`, `lists`. Because this now runs against the isolated `familysync_test` DB (never dev data), it is safe to also reset `users` to a known baseline IF tests depend on user rows. + +Decision (justify in the SUMMARY): do NOT blanket-`delete(users)` in `afterEach` — many tests seed user id=1 (dev user) once and reuse it; deleting users between tests would break FK-dependent rows mid-suite and add churn. Instead, leave `users` cleanup OUT of `afterEach` (matching current behavior) and rely on globalSetup's fresh-migrated `familysync_test` for a clean baseline at run start. If any test currently leaks `users` rows in a way that affects another test, scope a targeted delete inside that test's own setup rather than globally. Add a clarifying comment in `test/setup.ts` updating the file header: tests now run against `familysync_test` (provisioned by `test/global-setup.ts`), not the dev DB; `afterEach` truncates list/push tables only; `users` is left intact across tests within a run. + +Update `apps/api/README.md` (create a short `## Running API tests locally` section if absent): document that local api tests run against `familysync_test`, auto-provisioned by `test/global-setup.ts`; the run command is `set -a; source .env; set +a; DB_HOST=127.0.0.1 pnpm --filter @familysync/api test`; note that `DB_ROOT_PASSWORD` must be present in `.env` for the one-time CREATE DATABASE/GRANT, and that CI is unaffected because globalSetup no-ops when `CI` is set. + + + cd /home/luc/Projects/familysync && set -a && source .env && set +a && DEV_BEFORE=$(DB_HOST=127.0.0.1 node --input-type=module -e "import mysql from 'mysql2/promise'; const c=await mysql.createConnection({host:'127.0.0.1',port:3306,user:process.env.DB_USER,password:process.env.DB_PASSWORD,database:'familysync'}); const [r]=await c.query('SELECT COUNT(*) n FROM users'); console.log(r[0].n); await c.end();") && DB_HOST=127.0.0.1 pnpm --filter @familysync/api test && DEV_AFTER=$(DB_HOST=127.0.0.1 node --input-type=module -e "import mysql from 'mysql2/promise'; const c=await mysql.createConnection({host:'127.0.0.1',port:3306,user:process.env.DB_USER,password:process.env.DB_PASSWORD,database:'familysync'}); const [r]=await c.query('SELECT COUNT(*) n FROM users'); console.log(r[0].n); await c.end();") && echo "dev users before=$DEV_BEFORE after=$DEV_AFTER" && [ "$DEV_BEFORE" = "$DEV_AFTER" ] && DB_HOST=127.0.0.1 node --input-type=module -e "import mysql from 'mysql2/promise'; const c=await mysql.createConnection({host:'127.0.0.1',port:3306,user:process.env.DB_USER,password:process.env.DB_PASSWORD,database:'familysync_test'}); const [r]=await c.query('SHOW TABLES'); console.log('familysync_test tables:', r.length); await c.end();" + + Full local api test suite passes against familysync_test; dev `familysync` users count is identical before and after the run; `familysync_test` exists with migrated tables; the previously-flaky list_shares test no longer times out. README documents the local run flow. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| test process → dev MariaDB (root) | globalSetup connects as ROOT to provision a DB; root creds cross into a Node test process | +| vitest config → test workers | DB_NAME/DB_HOST override decides which DB the suite mutates | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation Plan | +|-----------|----------|-----------|-------------|-----------------| +| T-ndv-01 | Information Disclosure | root DB creds in global-setup.ts | mitigate | Read root creds from `process.env.DB_ROOT_USER/DB_ROOT_PASSWORD` with dev-only defaults; never hardcode prod secret; dev default `'root'` matches docker-compose.dev convention and is used only on the non-CI branch | +| T-ndv-02 | Tampering | accidental write to dev `familysync` DB | mitigate | `test.env.DB_NAME=familysync_test` forces workers off the dev DB; verify step captures dev `users` count before/after and asserts equality | +| T-ndv-03 | Elevation of Privilege | SQL injection via interpolated user/db identifiers in GRANT | mitigate | DB name is a fixed literal `familysync_test`; app user validated against `/^[A-Za-z0-9_]+$/` before interpolation into GRANT | +| T-ndv-04 | Denial of Service | globalSetup runs root provisioning in CI and breaks the isolated CI flow | mitigate | `if (process.env.CI) return;` first line of globalSetup; config env override is `{}` under CI — both CI-gated | +| T-ndv-SC | Tampering | npm/pip/cargo installs | accept | No new packages installed; uses already-installed `mysql2`, `drizzle-orm`, `drizzle-orm/mysql2/migrator` (verified present) | + + + +- `apps/api/test/global-setup.ts` returns early when `process.env.CI` is truthy (grep + read). +- `apps/api/vitest.config.ts` declares `globalSetup` and a CI-gated `DB_NAME=familysync_test` worker override; `fileParallelism:false` and `setupFiles` retained. +- Local run: `set -a; source .env; set +a; DB_HOST=127.0.0.1 pnpm --filter @familysync/api test` passes. +- Dev `familysync` `users` row count is identical before and after the run (captured in the Task 2 verify). +- `familysync_test` exists and has migrated tables after the run. +- `pnpm --filter @familysync/api typecheck` exits 0. +- CI inspection (no CI run): job-level `DB_NAME=familysync` + `db:migrate` step unchanged; globalSetup no-ops and the config env override is empty under `CI=true`, so CI keeps its own DB. + + + +- Local api tests run exclusively against `familysync_test`, proven by the dev `familysync` users count being unchanged and `familysync_test` containing the seeded/migrated tables. +- `familysync_test` is auto-created, granted to the app user, and migrated by globalSetup with no manual operator setup beyond `DB_ROOT_PASSWORD` being in `.env`. +- The CI `api` (and `harness`) jobs are demonstrably unaffected by code inspection: globalSetup early-returns and the env override is empty when `CI` is set. +- `typecheck` passes; the previously-flaky `lists.test.ts > re-populates list_shares` no longer times out. + + + +Create `.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-SUMMARY.md` when done. + diff --git a/.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-SUMMARY.md b/.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-SUMMARY.md new file mode 100644 index 0000000..0c8eec9 --- /dev/null +++ b/.planning/quick/260613-ndv-wire-apps-api-integration-tests-to-a-ded/260613-ndv-SUMMARY.md @@ -0,0 +1,117 @@ +--- +phase: quick-260613-ndv +plan: "01" +subsystem: api/testing +tags: [test-isolation, mariadb, drizzle, vitest, globalSetup] +dependency_graph: + requires: [] + provides: [familysync_test DB provisioning, local test isolation] + affects: [apps/api test suite, CI api job (unaffected — CI-gated)] +tech_stack: + added: [] + patterns: [vitest globalSetup, drizzle-orm/mysql2 two-arg form] +key_files: + created: + - apps/api/test/global-setup.ts + modified: + - apps/api/vitest.config.ts + - apps/api/test/setup.ts + - apps/api/README.md +decisions: + - "D-ndv-pool-form: use drizzle(pool, { mode }) not drizzle({ client: pool, mode }) — drizzle-orm@0.45.2 isConfig() has a tautological OR in the mode branch that always returns false; combined-config form falls through to construct(configObj, undefined) making the config object itself the session client" + - "D-ndv-users-intact: do not delete users in afterEach — tests seed user id=1 once and reuse across test files; users starts empty in familysync_test at run start; per-test user leaks scoped to those tests' own setup" +metrics: + duration: "~15 min" + completed: "2026-06-13" + tasks_completed: 2 + files_changed: 4 +--- + +# Quick Task 260613-ndv: Test DB Isolation Summary + +**One-liner:** vitest globalSetup provisions + migrates `familysync_test` via root MariaDB connection (CI-gated no-op); `test.env` forces workers to `DB_NAME=familysync_test`; 244 tests pass without touching the dev DB. + +## Tasks Completed + +| Task | Name | Commit | Key Files | +|------|------|--------|-----------| +| 1 | Add CI-gated globalSetup (provision + migrate familysync_test) | 8453b97 | apps/api/test/global-setup.ts (new), apps/api/vitest.config.ts | +| 2 | Clean-slate comment in setup.ts + README local-test docs | 4740d86 | apps/api/test/setup.ts, apps/api/README.md, apps/api/test/global-setup.ts (bug fix) | + +## What Was Built + +### apps/api/test/global-setup.ts (new, 108 lines) + +Vitest `globalSetup` that runs once in the main process before any test file: + +- **CI gate:** `if (process.env.CI) return` — CI provisions its own `familysync` service DB via `db:migrate`, completely unaffected. +- **Local flow:** + 1. Root connection (`DB_ROOT_USER`/`DB_ROOT_PASSWORD` with dev defaults `root`/`root`) → `CREATE DATABASE IF NOT EXISTS familysync_test` + 2. `GRANT ALL PRIVILEGES ON familysync_test.* TO ''@'%'` with `appUser` validated against `/^[A-Za-z0-9_]+$/` (T-ndv-03); falls back to `@'localhost'` grant if `@'%'` fails. + 3. `FLUSH PRIVILEGES`, close root connection. + 4. App-user pool → `drizzle(pool, { mode: 'default' })` → `migrate(db, { migrationsFolder })` applies committed SQL from `apps/api/src/db/migrations/`. + 5. Logs `[global-setup] provisioned + migrated familysync_test`. + +### apps/api/vitest.config.ts (modified) + +- Added `globalSetup: ['./test/global-setup.ts']` +- Added CI-gated `test.env`: locally sets `DB_NAME=familysync_test` and `DB_HOST=127.0.0.1`; under `CI` the env override is `{}` so job-level `DB_NAME=familysync` / `DB_HOST=mariadb` are preserved. +- Retained `fileParallelism: false` and `setupFiles: ['./test/setup.ts']`. + +### apps/api/test/setup.ts (modified) + +Updated file header to document: +- Tests now run against `familysync_test` (not dev `familysync`) +- `afterEach` truncates list/push tables only; `users` is intentionally left intact within a run +- Rationale for the `users` decision (no per-test deletion — tests seed id=1 once and reuse it) + +### apps/api/README.md (modified) + +Added `## Running API tests locally` section documenting: +- Dev MariaDB prerequisite + run command (`set -a; source .env; set +a; DB_HOST=127.0.0.1 pnpm --filter @familysync/api test`) +- `DB_ROOT_PASSWORD` requirement in `.env` for one-time provisioning +- CI unaffected note + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] drizzle-orm@0.45.2 isConfig() mode branch tautology** +- **Found during:** Task 1 verification (first test run) +- **Issue:** `drizzle({ client: pool, mode: 'default' })` — the `{ client, mode }` combined config form — triggers a bug in `drizzle-orm@0.45.2/utils.js` `isConfig()`. The `mode` branch has: `if (data["mode"] !== "default" || data["mode"] !== "planetscale" || ...)` which is a tautological OR (always true for any mode value), so `isConfig` returns `false`. The call falls through to `construct(configObj, undefined)`, making the config object itself the session client. `client.query is not a function`. +- **Fix:** Switched to `drizzle(pool, { mode: 'default' })` (two-arg form, which hits the `construct(params[0], params[1])` branch directly — correct behavior). Added comment in the code explaining the drizzle-orm bug. +- **Files modified:** `apps/api/test/global-setup.ts` +- **Commit:** 4740d86 + +## Verification Results + +| Check | Result | +|-------|--------| +| `grep -q "globalSetup" apps/api/vitest.config.ts` | PASS | +| `grep -q "familysync_test" apps/api/vitest.config.ts` | PASS | +| `grep -q "process.env.CI" apps/api/test/global-setup.ts` | PASS | +| `grep -q "migrate(" apps/api/test/global-setup.ts` | PASS | +| `pnpm --filter @familysync/api typecheck` | PASS (exit 0) | +| Full local test run (244 tests, 25 files) | PASS | +| Dev `familysync` users count before=3, after=3 | PASS | +| `familysync_test` tables after run | PASS (10 tables: all schema tables + __drizzle_migrations) | +| CI code inspection: globalSetup early-returns, env override is {} | PASS (confirmed by code) | + +## Known Stubs + +None. + +## Threat Flags + +None — no new network endpoints, auth paths, or file access patterns introduced. The root DB credential usage is scoped exclusively to the local non-CI branch of globalSetup and reads from env (T-ndv-01 mitigated as designed). + +## Self-Check: PASSED + +- `apps/api/test/global-setup.ts` exists: confirmed +- `apps/api/vitest.config.ts` updated: confirmed +- `apps/api/test/setup.ts` updated: confirmed +- `apps/api/README.md` updated: confirmed +- Commit 8453b97 exists: confirmed (Task 1) +- Commit 4740d86 exists: confirmed (Task 2) +- 244/244 tests pass against familysync_test: confirmed +- Dev DB users count unchanged (3 before, 3 after): confirmed From 4517432dca94705994f7538f48f64b134e42dfbb Mon Sep 17 00:00:00 2001 From: Lucas Berger Date: Sat, 13 Jun 2026 17:12:52 -0400 Subject: [PATCH 5/5] style(quick-260613-ndv): prettier-format global-setup.ts --- apps/api/test/global-setup.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/apps/api/test/global-setup.ts b/apps/api/test/global-setup.ts index 0249048..5f092b4 100644 --- a/apps/api/test/global-setup.ts +++ b/apps/api/test/global-setup.ts @@ -64,14 +64,10 @@ export default async function setup(): Promise { // Try @'%' first (Docker/network access); fall back to @'localhost' if the user was // created with a localhost host qualifier. try { - await rootConn.query( - `GRANT ALL PRIVILEGES ON \`${TEST_DB}\`.* TO '${appUser}'@'%'`, - ); + await rootConn.query(`GRANT ALL PRIVILEGES ON \`${TEST_DB}\`.* TO '${appUser}'@'%'`); } catch { // If @'%' fails (user only exists as @'localhost'), try that form. - await rootConn.query( - `GRANT ALL PRIVILEGES ON \`${TEST_DB}\`.* TO '${appUser}'@'localhost'`, - ); + await rootConn.query(`GRANT ALL PRIVILEGES ON \`${TEST_DB}\`.* TO '${appUser}'@'localhost'`); } await rootConn.query('FLUSH PRIVILEGES'); @@ -101,9 +97,7 @@ export default async function setup(): Promise { // construct({ client, mode }, undefined) and the session client becomes // the plain config object (no .query()). The two-arg form is safe. const db = drizzle(appPool, { mode: 'default' }); - const migrationsFolder = fileURLToPath( - new URL('../src/db/migrations', import.meta.url), - ); + const migrationsFolder = fileURLToPath(new URL('../src/db/migrations', import.meta.url)); await migrate(db, { migrationsFolder }); // Clean slate each run: truncate every table (except drizzle's migration