test(harness): unset the DB env keys instead of assigning undefined

backend/TESTING.md

@@ -74,7 +74,9 @@ Tier-1 setup logs `[tier1] Connected to real MongoDB …` and `[tier1] Connected
 | `generateTestToken(userId)` | signs with `process.env.JWT_SECRET` | same |
 | `createTestUser / Pod / Message` | Mongoose model instantiation | same |
 
-The branch is controlled by `process.env.INTEGRATION_TEST === 'true'`. `__tests__/setup.js` reads this at suite start and populates `MONGO_URI` / `PG_*` defaults when set; when unset, it nulls them so accidental real-DB connections fail loudly.
+The branch is controlled by `process.env.INTEGRATION_TEST === 'true'`. `__tests__/setup.js` reads this at suite start and populates `MONGO_URI` / `PG_*` defaults when set; when unset, it **deletes** them (`delete process.env.PG_HOST`) so accidental real-DB connections fail loudly. Assigning `undefined` does not do that: Node stores the **string** `'undefined'`, which is truthy, and every `if (process.env.PG_HOST)` guard in the backend then takes the configured branch — including `config/db-pg.ts:75`, which builds a `Pool` for a host named `undefined` instead of leaving `pool` null (#1873).
+
+**`process.env` is per test FILE, not per test worker.** `jest-environment-node` gives each file its own detached copy (`jest-util`'s `createProcessObject`), so two files in one `--runInBand` worker — same pid — never observe each other's writes. Two consequences: no cross-file cleanup is owed for a `process.env` write inside a file, and a variable set in one file cannot be inherited by the next. The copy is also detached from the *real* environment, which is why writing `process.env.TMPDIR` does not move `os.tmpdir()` under Jest (see `docs/development/review-checklist.md` rule 31) — **a child process needs its env passed explicitly**; `child_process`'s default env reads the real environment, not the test file's copy.
 
 ## Authoring rules
 

backend/__tests__/setup.js

@@ -31,7 +31,15 @@ if (process.env.INTEGRATION_TEST) {
   process.env.PG_PASSWORD = process.env.PG_PASSWORD || 'postgres';
   process.env.PG_SSL_ENABLED = 'false';
 } else {
-  // In-memory mode — no real DB connections (default for unit tests)
-  process.env.PG_HOST = undefined;
-  process.env.MONGO_URI = undefined;
+  // In-memory mode — no real DB connections (default for unit tests).
+  // DELETE the keys; do not assign `undefined`. Assignment stores the STRING
+  // 'undefined', which is truthy, so every `if (process.env.PG_HOST)` guard in
+  // the backend takes the configured branch — 25 reads across 11 runtime
+  // modules, including server.ts:68/:378, podController.ts:22/:462/:663,
+  // authController.ts:171 and agentsRuntime.ts:3188 — and db-pg.ts:75 builds a
+  // real Pool for host 'undefined' instead of taking connectPG's not-configured
+  // path at :106-108. Measured: PG_HOST read back as the string "undefined" at
+  // module scope in a file that never touched it.
+  delete process.env.PG_HOST;
+  delete process.env.MONGO_URI;
 }

backend/__tests__/unit/config/init-pg-db.test.js

@@ -1,4 +1,11 @@
+/* eslint-disable import/no-unresolved, import/extensions */
 jest.mock('fs');
+// The unit under test is initializeDatabase; its pool dependency is stubbed. This
+// suite previously reached for a REAL Pool object, which existed only because
+// setup.js left PG_HOST as the truthy string 'undefined' and db-pg.ts:75 built a
+// Pool for that host — so it broke the moment the harness stopped lying about the
+// host, not because initializeDatabase changed (TASK-128).
+jest.mock('../../../config/db-pg', () => ({ pool: { connect: jest.fn() } }));
 const fs = require('fs');
 const { pool } = require('../../../config/db-pg');
 

backend/__tests__/unit/config/setupEnvAbsence.test.js

@@ -0,0 +1,23 @@
+/* eslint-disable import/no-unresolved, import/extensions */
+// setup.js runs before every test file (jest `setupFiles`). process.env is per
+// test FILE (measured), so what this file observes is setup.js's own effect and
+// nothing another suite did.
+const { pool } = require('../../../config/db-pg');
+
+describe('unit-suite DB env (setup.js in-memory mode)', () => {
+  it('does not store the string "undefined" in PG_HOST / MONGO_URI', () => {
+    // The defect signature: `process.env.PG_HOST = undefined` stores the STRING
+    // 'undefined', which is truthy and satisfies every `if (process.env.PG_HOST)`
+    // guard in the backend.
+    expect(process.env.PG_HOST).not.toBe('undefined');
+    expect(process.env.MONGO_URI).not.toBe('undefined');
+  });
+
+  it('builds no Pool for the host string "undefined"', () => {
+    // The consequence (db-pg.ts:75): a truthy host string constructs a real Pool.
+    // Asserted on the pool's own config rather than on `pool === null`, so a
+    // developer with a real PG_HOST in the environment still passes.
+    const host = pool && pool.options && pool.options.host;
+    expect(host).not.toBe('undefined');
+  });
+});

backend/services/agentIdentityService.ts

@@ -580,12 +580,12 @@ class AgentIdentityService {
   }
 
   static async syncUserToPostgreSQL(user: InstanceType<typeof User>): Promise<boolean> {
-    const pgHost = process.env.PG_HOST;
-    // Some test harnesses clear process.env with `undefined`, which Node
-    // serializes as the literal string "undefined". Treat that the same as
-    // an unconfigured PostgreSQL mirror rather than reporting a false sync
-    // failure to an otherwise successful profile save.
-    if (!PGMessage || !pgHost || pgHost === 'undefined' || !dbPg) return true;
+    // `!PG_HOST` is the unconfigured-mirror case: config/db-pg.ts:75 builds a
+    // Pool only when PG_HOST is set, so there is nothing to sync into. A test
+    // harness that stored the literal string "undefined" made that guard truthy
+    // anyway, so this used to name the string explicitly; __tests__/setup.js
+    // deletes the key instead and the compensation is gone.
+    if (!PGMessage || !process.env.PG_HOST || !dbPg) return true;
     try {
       const { pool } = dbPg;
       const checkQuery = 'SELECT _id FROM users WHERE _id = $1';

backend/tsconfig.build.json

@@ -11,7 +11,6 @@
     "**/*.test.js",
     "**/*.spec.ts",
     "jest.config.js",
-    "jest.setup.js",
     "cleanup-test-data.ts",
     "test-discord-commands.ts",
     "test-discord-integration.ts",

docs/development/review-checklist.md

@@ -98,7 +98,7 @@ And then note where the fix landed. Someone hit the audience gap, felt it, and b
 
 30. **A question about what a rendered value MEANS is answered from the store, not from the renderer.** A renderer's fallback chain tells you which inputs *could* have produced a string; it cannot tell you which one did, and reading the chain to reason about the cause settles nothing. Enumerate the chain from source — each `||` is a hypothesis with a different mechanism — then measure the inputs in the store. Prefer a decisive experiment to more reading where one exists: a single write plus a single re-capture falsified a membership hypothesis in about a minute (and the probe was reverted, so the committed evidence still matches its seed). When the answer is "the artifact is a harness artifact", say so **with the artifact**: a screenshot cannot distinguish a fixture row from live data, so any evidence built from seeded records must name its seed and the fixture-shaped fields in the same breath as the image, or a reader will read the fixture as usage. Rider: a hand-seeded row is not equivalent to a posted one — the real write path also creates the identity row and the membership, so a hand-inserted record reproduces neither. *(Earned: 2026-09-19, the #1758 evidence — the seeded card rendered as `Unknown User` and the file-worthy hypothesis was pod membership; the measurement refuted it (added the member, the page read `2 members`, the label unchanged). The cause was the PG mirror: `backend/models/pg/Message.ts:214,261` LEFT JOINs `users` for `u.username`, and the hand-inserted card's author has no row there because it never went through `agentIdentityService.ts:618` (`INSERT INTO users`) inside `getOrCreateAgentUser`, which `agentMessageService.ts:1245–1249` calls together with `ensureAgentInPod` before posting. Nothing filed — and four seeded `tool_calls` rows read as live usage by a human until their provenance was stated.)*
 
-31. **A flake fix is proven by making the defect reproducible, not by rerunning the flake — and an assertion that counts a process-global resource must watch the subject's own identity.** "It passed ten times" is not evidence about a flaky assertion: the flake's rate bounds its cost, never whether it is fixed. The reproducible form is to mutate the guarded behaviour into existence and watch the same assertion go red, which also proves the assertion measures the thing it names. The follow-up question is what the assertion counts: a count over `os.tmpdir()` contents, a socket table or a lock directory is process- or machine-global state a sibling test or another actor can perturb, so the assertion must watch the *subject's own* identity — this run's path, this seat's row — not the pool it lives in. One negative result worth carrying, because it looks like an escape hatch and is not: setting `process.env.TMPDIR` does **not** redirect `os.tmpdir()` inside jest, though a bare `node` v20 does follow it — mechanism unexplained, do not build isolation on it. *(Earned: TASK-073/#1769 — the codex adapter creates its temp dir at `codex.js:442` and removes it in a `finally` at `:506`; the test now watches the directory that call created, so dropping that `rm` reddens exactly that test, with an in-seam `existsSync` control so it cannot pass against an adapter that never creates the directory. Control for the flake itself: branch 3/15, main 3/15, alone 0/10 — the rate was pre-existing; the instrument was the fix.)*
+31. **A flake fix is proven by making the defect reproducible, not by rerunning the flake — and an assertion that counts a process-global resource must watch the subject's own identity.** "It passed ten times" is not evidence about a flaky assertion: the flake's rate bounds its cost, never whether it is fixed. The reproducible form is to mutate the guarded behaviour into existence and watch the same assertion go red, which also proves the assertion measures the thing it names. The follow-up question is what the assertion counts: a count over `os.tmpdir()` contents, a socket table or a lock directory is process- or machine-global state a sibling test or another actor can perturb, so the assertion must watch the *subject's own* identity — this run's path, this seat's row — not the pool it lives in. One negative result worth carrying, because it looks like an escape hatch and is not: setting `process.env.TMPDIR` does **not** redirect `os.tmpdir()` inside jest, though a bare `node` v20 does follow it — the mechanism is the per-file `process.env`: a test file's env is `jest-util`'s detached `createProcessObject` copy, so its writes never reach the real environment, while `os.tmpdir()` (POSIX native `getTempDir()`) and `child_process`'s default env both read the real one, so an `execSync` child does not see a variable the test file set. Do not build isolation on it — and pass env explicitly to any child that needs it. *(Reason replaced 2026-09-25; the conclusion is unchanged and only the "unexplained" rider is withdrawn. Measured on TASK-128 by @wren and reproduced on this box: Jest 29.7 gives each test file its own `process.env` — two files in one `--runInBand` worker, same pid, neither saw the other's writes — on Node 22 and on Node 20.18.1. See `backend/TESTING.md`.)* *(Earned: TASK-073/#1769 — the codex adapter creates its temp dir at `codex.js:442` and removes it in a `finally` at `:506`; the test now watches the directory that call created, so dropping that `rm` reddens exactly that test, with an in-seam `existsSync` control so it cannot pass against an adapter that never creates the directory. Control for the flake itself: branch 3/15, main 3/15, alone 0/10 — the rate was pre-existing; the instrument was the fix.)*
 
 ## Merge mechanics