The flaky test that was a tenant-isolation bug

A database test in our suite failed about one run in eight. Here is the full path from symptom to cause, including the three hypotheses that were wrong, because the wrong ones are the useful part.

The symptom

✖ a pending approval is returned with its platform and action type
  AssertionError: 0 !== 1

And sometimes, instead:

  AssertionError: '—' !== 'gmail'

Same test, two different failures. That should have been the first clue — one bug producing two shapes usually means the failure is upstream of both assertions.

Wrong hypothesis 1: test contention

The obvious read: something else was hammering the database. We had a dashboard running against the same Postgres container.

Evidence killed it. The tests use a different database, connections were 7 of 100, and query round trips were 0.07s. Three consecutive runs passed with the dashboard up.

Wrong hypothesis 2: a CREATE DATABASE race

Each test file drops and recreates its database in before. Fourteen files run in parallel and Postgres serialises CREATE DATABASE. Plausible.

Also wrong — each suite already uses its own database name, so they were never competing for the same one.

Wrong hypothesis 3: test ordering

Maybe a later test decided the approval before the read ran. But the tests are sequential — no concurrency flag — and the seed happens in a before hook.

The actual cause

Our repository wraps every tenant-scoped read like this:

await connection.query("begin");
await connection.query("select set_config('app.tenant_id', $1, true)", [tenantId]);
const result = await work(scope);
await connection.query("commit");

The connection is a single long-lived psql session, not a pool. So when two of these overlap:

A: begin          -- opens A's transaction
B: begin          -- no-op, Postgres warns, B is now inside A's transaction
A: commit         -- ends the transaction B is still using
B: select ...     -- runs with no transaction at all

That last line is the bug. set_config(..., true) is transaction-local — which is exactly what you want, because it stops a pooled connection leaking one tenant's context into another request. But when the transaction ends underneath a caller, the setting goes with it. Every row-level security policy then sees an empty tenant and matches nothing.

Hence 0 !== 1. And when the outer read landed inside a valid context while the joined read did not, '—' !== 'gmail'.

Why this is not a flaky test

A retry would have made it green. It would also have shipped a path where one caller's query can execute under another caller's tenant context — which is the isolation boundary the whole system rests on.

The test was not unreliable. It was intermittently reporting a real defect.

The fix

Serialise callers per connection, so a transaction cannot be opened inside another:

const queues = new WeakMap<SqlConnection, Promise<unknown>>();

const predecessor = queues.get(connection) ?? Promise.resolve();
let release!: () => void;
const turn = new Promise<void>((r) => { release = r; });
queues.set(connection, turn);
await predecessor.catch(() => undefined);
try {
  return await runTransaction(connection, ctx, work);
} finally {
  release();
}

Three details that matter:

• The queue slot is claimed before the first await, or two callers entering together read the same predecessor and run as one.
• The predecessor's rejection is swallowed. A failed request handed the connection back; inheriting its error would fail an unrelated caller.
• release() is in a finally. An unreleased slot stalls every caller behind it — a deadlock is a worse outcome than the bug.

What I would do differently

Reproduce first. I asserted 'test contention' twice before running a loop that would have told me, and both times the story was plausible enough that I nearly stopped there.

The loop that found it is six lines, and it asserts the run completed — because a half-killed test run reports as 'clean' and looks exactly like a pass.