root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FN-9133: Use bounded PostgreSQL DDL for reliability fixtures

Browse Source 
Reliability fixtures now manage PostgreSQL databases through owned, deadline-bounded maintenance connections.

- Replace psql child-process DDL with postgres.js maintenance clients and forced cleanup.
- Remove redundant pre-create drops and preserve idempotent teardown behavior.
- Add a live-connection cleanup contract test and document audit measurements and policy.

Files changed:
 .../postgres-reliability-helper-ddl-audit.md       | 48 +++++++++++
 docs/testing.md                                    |  2 +-
 packages/engine/package.json                       |  5 +-
 .../_helpers-pg-ddl-contract.pg.test.ts            | 66 +++++++++++++++
 .../__tests__/reliability-interactions/_helpers.ts | 93 +++++++++++++---------
 pnpm-lock.yaml                                     |  3 +
 6 files changed, 176 insertions(+), 41 deletions(-)

Fusion-Task-Id: FN-9133

Fusion-Task-Lineage: 5a7f511d-abb4-4468-a97a-04331f60d245

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-08-16 16:11:21 -07:00
parent 2160f7500c
commit 1d7dd3e7b8
6 changed files with 176 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
docs/solutions/test-failures/postgres-reliability-helper-ddl-audit.md Normal file
View File

@@ -0,0 +1,48 @@
---
category: test-failures
module: testing
problem_type: PostgreSQL fixture cleanup and DDL contention
applies_when: Engine reliability tests create isolated PostgreSQL databases under loaded worker fan-out.
tags: [postgresql, testing, reliability, ddl, cleanup]
---
# PostgreSQL reliability helper DDL audit
FN-9133 audited `packages/engine/src/__tests__/reliability-interactions/_helpers.ts`, the contained PostgreSQL fixture helper used by the engine reliability lane.
## Inventory
Before the fix, each fixture used `psql ... -f -` for all database DDL, issued a redundant unique-name `DROP DATABASE IF EXISTS` before `CREATE DATABASE`, and used a best-effort cleanup drop without `WITH (FORCE)`. That was three explicit database statements per fixture (drop/create/drop), plus a full schema baseline. The helper's runtime pool permits five connections, so a live connection could make the cleanup drop fail and leave a `fusion_rel_%` database.
The audit's source census found 53 `makeReliabilityFixture(`, 3 `makePgTaskStore(`, and 3 `createPgLayer(` occurrences (including declarations) in this checkout. A repository scan for `CREATE DATABASE` across engine, dashboard, and CLI sources returned only this helper, so the divergence was contained.
## Measurements
Each measurement ran `engine-reliability` with retained output, then queried `pg_database` for `fusion_rel_%` and used `pgrep -x psql` for the child census.
| phase/run | workers | result | wall seconds | leaked databases | live psql |
|---|---|---|---:|---:|---:|
| baseline default | computed default | 100 files / 548 tests passed | 52.85 | 1 | 0 |
| baseline elevated 1 | 12 | passed | 38.62 | 0 | 0 |
| baseline elevated 2 | 12 | passed | 35.20 | 0 | 0 |
| post default | computed default | passed | 49.01 | 0 | 0 |
| post elevated 1 | 12 | passed | 45.77 | 0 | 0 |
| post elevated 2 | 12 | passed | 48.71 | 0 | 0 |
Baseline variation was 35.20/38.62/52.85 seconds (min/median/max); post-change variation was 45.77/48.71/49.01. This is not a timing-improvement claim. The acceptance band was three green post-change runs, zero database leaks, zero psql children, and no run slower than the 52.85-second baseline maximum; all runs met it.
## Verdict
### Admission/deferred drain: do not copy
FN-9130 measured uniform and drop-only K=4 in-hook admission as regressions, with unstable watchdog counts even on unchanged ungated code. FN-9133 therefore does **not** add an advisory admission gate or deferred drain. FN-9134 owns the open structural alternative.
### Maintenance-connection contract: conform
The helper now uses a short-lived owned `postgres.js` maintenance client with a server-side statement timeout, a JavaScript deadline, and forced socket close. It no longer needs a `psql` binary gate. Cleanup uses `DROP DATABASE IF EXISTS ... WITH (FORCE)`, and the redundant pre-create drop is removed; the explicit DDL minimum is now create plus forced cleanup drop.
The regression test holds an independent open transaction, runs helper cleanup, verifies that `pg_database` no longer contains the fixture, and calls cleanup twice. A temporary local revert failed the test with the retained `fusion_rel_74100_1_dobb29` database. The test also pins the absence of `exec`, `execSync`, or `spawnSync` psql DDL launch sites.
## Policy preservation
No test or hook timeout, worker/fan-out setting, `exclude`, retry, `.skip` policy (other than the standard `hasPg` conditional test gate), or quarantine ledger changed. The changed lane, targeted contract test, lint, `pnpm verify:fast`, and `pnpm build` passed.

2
docs/testing.md
View File

@@ -23,7 +23,7 @@ Gate membership is the explicit allow-list in `packages/engine/vitest.config.ts`
**PostgreSQL and unit gate policy:** `packages/core`'s `test:pg-gate` intentionally runs `handoff-to-review-atomicity.pg.test.ts` and `task-lifecycle-e2e.pg.test.ts`, preserving atomic-handoff and lifecycle real-backend canaries. `sync-workflow-ir-is-always-default.pg.test.ts` was evicted under the merge-gate flake rule; its coverage remains in the non-blocking core suite (see the [observed suite-only flakes register](solutions/test-failures/suite-only-flakes-observed-register.md#6-sync-workflow-ir-default-canary-setup-hook)). `test:unit-gate` runs `task-merge.test.ts`, `legacy-adoption.test.ts`, `no-hardcoded-lifecycle-columns.test.ts`, and `sync-workflow-ir-callsite-allowlist.test.ts`. Every other former PG gate member remains enabled and discovered by the non-blocking command `pnpm --filter @fusion/core test` (default config: `src/**/*.test.ts`, no PG quarantine exclusions). `scripts/__tests__/engine-vitest-gate-policy.test.mjs` pins the exact two PG and four unit files, all waits, CI-shape ordering, and every engine/static member.
<!-- FNXC:PgTestBootstrap 2026-08-16-18:59: PostgreSQL integration fixtures must use pg-test-harness bootstrap primitives so reachability, bounded maintenance DDL, and forced cleanup do not drift between files under forked loaded runs. Select createEmptyPgTestDatabase when the test proves first application/upgrades; use a baselined clone only when the schema-present state itself is the contract. -->
**PostgreSQL fixture bootstrap:** Do not hand-roll per-file `CREATE DATABASE` or drop helpers. Use `createEmptyPgTestDatabase` for migration-application and upgrade contracts, and `createBaselinedPgTestDatabase` only when an already-applied schema is the intended fixture state. Both keep database lifecycle and cleanup behavior aligned with the shared harness.
**PostgreSQL fixture bootstrap:** Do not hand-roll per-file `CREATE DATABASE` or drop helpers. Use `createEmptyPgTestDatabase` for migration-application and upgrade contracts, and `createBaselinedPgTestDatabase` only when an already-applied schema is the intended fixture state. Both keep database lifecycle and cleanup behavior aligned with the shared harness. Engine reliability fixtures follow the same bounded maintenance-connection and forced-cleanup contract; see [the reliability helper DDL audit](solutions/test-failures/postgres-reliability-helper-ddl-audit.md).
<!-- FNXC:EngineTests 2026-07-08-03:00: FN-7667 decouples the engine-core gate's module graph from full-barrel growth so new feature modules don't silently inflate every gate fork's transform/import cost. -->
**Gate-safe `@fusion/core` barrel:** the `engine-core` project resolves `@fusion/core` to `packages/core/src/index.gate.ts` (a project-scoped `resolve.alias`, not the root map), not the full `packages/core/src/index.ts` barrel. `index.gate.ts` is a byte-for-byte copy of the full barrel minus the `export ... from` statements for modules added to the barrel after the last re-audit baseline — i.e. it re-exports everything the full barrel does except genuinely new, gate-irrelevant feature modules (diffed against the prior baseline commit's barrel, not hand-picked from what gate *test* files import — production modules under test pull in far more of the barrel transitively than their own imports suggest). `engine-default`/`engine-reliability`/`engine-slow` are unaffected and keep resolving the full barrel. `@fusion/engine` is untouched (no gate file imports it). When adding a new barrel module that no gate test needs, mirror the exclusion in `index.gate.ts` rather than letting gate wall-time grow — see the FNXC comment at the top of `index.gate.ts` and `packages/engine/vitest.config.ts`'s `engine-core` project for the audit procedure.

5
packages/engine/package.json
View File

@@ -46,14 +46,15 @@
"cron-parser": "^5.5.0",
"esbuild": "^0.25.12",
"node-pty": "npm:@homebridge/node-pty-prebuilt-multiarch@^0.13.1",
"playwright-core": "^1.60.0",
"proper-lockfile": "^4.1.2",
"typebox": "^1.0.0",
"playwright-core": "^1.60.0"
"typebox": "^1.0.0"
},
"devDependencies": {
"@types/node": "^25.5.0",
"@types/proper-lockfile": "^4.1.4",
"@vitest/coverage-v8": "^4.1.10",
"postgres": "3.4.9",
"typescript": "^5.7.0",
"vitest": "^4.1.10"
},

66
packages/engine/src/__tests__/reliability-interactions/_helpers-pg-ddl-contract.pg.test.ts Normal file
View File

@@ -0,0 +1,66 @@
/*
FNXC:ReliabilityFixtures 2026-08-16-22:30:
FN-9133 pins the reliability helper's bounded maintenance-connection contract.
A live extra database connection must not survive cleanup, and DDL must never
create a psql child that can outlive Vitest's subprocess guard.
*/
import { readFile } from "node:fs/promises";
import { describe, expect, it } from "vitest";
import { createConnectionSetFromUrl, drizzleSql, type ResolvedBackend } from "@fusion/core";
import postgres from "postgres";
import { createPgLayer, hasPg } from "./_helpers.js";
const PG_TEST_URL_BASE = process.env.FUSION_PG_TEST_URL_BASE ?? "postgresql://localhost:5432";
function maintenanceUrl(): string {
const url = new URL(PG_TEST_URL_BASE);
url.pathname = "/postgres";
return url.toString();
}
function connectionBackend(url: string): ResolvedBackend {
return {
mode: "external",
runtimeUrl: url,
migrationUrl: url,
migrationUrlOverridden: false,
};
}
const pgDescribe = hasPg ? describe : describe.skip;
pgDescribe("FN-9133 reliability PostgreSQL DDL contract", () => {
it("force-drops a live fixture database, is idempotent, and has no psql DDL spawn", async () => {
const fixture = await createPgLayer();
const heldClient = postgres(`${PG_TEST_URL_BASE}/${fixture.dbName}`, {
max: 1,
prepare: false,
onnotice: () => {},
});
const heldConnection = await heldClient.reserve();
const maint = await createConnectionSetFromUrl(connectionBackend(maintenanceUrl()), {
poolMax: 1,
connectTimeoutSeconds: 5,
});
try {
// A reserved connection with an open transaction stays attached until FORCE ends it.
await heldConnection.unsafe("BEGIN");
await expect(fixture.cleanup()).resolves.toBeUndefined();
await expect(fixture.cleanup()).resolves.toBeUndefined();
const rows = await maint.runtime.execute(drizzleSql`
SELECT datname FROM pg_database WHERE datname = ${fixture.dbName}
`);
expect(rows).toEqual([]);
const source = await readFile(new URL("./_helpers.ts", import.meta.url), "utf8");
expect(source).not.toMatch(/(?:exec|execSync|spawnSync)\(\s*["'`]psql\b/);
} finally {
// FORCE may already have closed this socket; do not issue a follow-up query on it.
heldConnection.release();
void heldClient.end({ timeout: 0 }).catch(() => {});
await maint.close().catch(() => {});
await fixture.cleanup();
}
});
});

93
packages/engine/src/__tests__/reliability-interactions/_helpers.ts
View File

@@ -1,7 +1,8 @@
import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { execSync, spawnSync, exec } from "node:child_process";
import { execSync, spawnSync } from "node:child_process";
import postgres from "postgres";
import { Worker } from "node:worker_threads";
import {
AgentStore, DEFAULT_SETTINGS, TaskStore, type Settings, type Task,
@@ -94,40 +95,51 @@ function probeTcpReachable(host: string, port: number, timeoutMs = 1500): boolea
}
/*
FNXC:PgTestGuard 2026-07-14-07:10:
hasPg must verify BOTH that the PostgreSQL server is TCP-reachable AND that the
psql CLI binary is installed. adminExecAsync() shells out to psql for DDL
(CREATE/DROP DATABASE). Without this check, a runner with Postgres reachable
but psql missing would pass the gate and fail inside fixture creation with
spawn ENOENT instead of skipping cleanly.
*/
const hasPsql = spawnSync("psql", ["--version"], { stdio: "pipe" }).status === 0;
FNXC:ReliabilityFixtures 2026-08-16-22:30:
FN-9133 aligns reliability-fixture DDL with the core harness after its documented
2026-07-18 orphaned `psql -f -` subprocess-guard incident. PostgreSQL reachability
is sufficient now that an owned postgres.js maintenance connection runs DDL; never
reintroduce a psql binary requirement or shell child for CREATE/DROP DATABASE.
export const hasPg = process.env.FUSION_PG_TEST_SKIP !== "1" && hasPsql && (() => {
FN-9133 deliberately does not copy an advisory admission gate or deferred drain.
FN-9130 measured those in-hook designs as regressions, and FN-9134 owns the open
contention alternative.
*/
export const hasPg = process.env.FUSION_PG_TEST_SKIP !== "1" && (() => {
if (!PG_TEST_URL_BASE) return false;
const { host, port } = parseProbeTarget(PG_TEST_URL_BASE);
return probeTcpReachable(host, port);
})();
function adminExecAsync(statement: string, timeoutMs = 15_000): Promise<void> {
const { promise, resolve, reject } = Promise.withResolvers<void>();
const maintUrl = new URL(PG_TEST_URL_BASE);
maintUrl.pathname = "/postgres";
const child = exec(
`psql "${maintUrl.toString()}" -v ON_ERROR_STOP=1 -f -`,
{ stdio: ["pipe", "pipe", "pipe"], env: process.env, timeout: timeoutMs },
(error, _stdout, stderr) => {
if (error) {
reject(new Error(`adminExec psql failed: ${error.message}\nstderr: ${stderr}`));
return;
}
resolve();
},
);
if (child.stdin) {
child.stdin.end(statement);
async function adminExecAsync(statement: string, timeoutMs = 15_000): Promise<void> {
let timedOut = false;
let timeoutHandle: ReturnType<typeof setTimeout> | undefined;
let client: ReturnType<typeof postgres> | undefined;
try {
await Promise.race([
(async () => {
const maintUrl = new URL(PG_TEST_URL_BASE);
maintUrl.pathname = "/postgres";
client = postgres(maintUrl.toString(), { max: 1, prepare: false, onnotice: () => {} });
// Cancel on the server before the JS deadline and own the socket for force-close.
await client.unsafe(`SET statement_timeout = ${Math.max(1_000, timeoutMs - 500)}`);
await client.unsafe(statement);
})(),
new Promise<never>((_, reject) => {
timeoutHandle = setTimeout(() => {
timedOut = true;
void client?.end({ timeout: 0 }).catch(() => {});
reject(new Error(`adminExec timed out after ${timeoutMs}ms: ${statement}`));
}, timeoutMs);
}),
]);
} catch (error) {
if (timedOut) throw error;
throw new Error(`adminExec failed: ${error instanceof Error ? error.message : String(error)}\nstatement: ${statement}`);
} finally {
if (timeoutHandle) clearTimeout(timeoutHandle);
await client?.end({ timeout: 5 }).catch(() => {});
}
return promise;
}
let relDbCounter = 0;
@@ -140,17 +152,13 @@ export type PgLayerFixture = {
/**
* Create one isolated PostgreSQL schema layer for a reliability test.
* Callers must use {@link hasPg} before invoking this helper because DDL uses
* the `psql` binary as well as a TCP-reachable PostgreSQL server.
* Callers must use {@link hasPg} before invoking this helper because DDL needs
* a TCP-reachable PostgreSQL maintenance database.
*/
export async function createPgLayer(): Promise<PgLayerFixture> {
relDbCounter += 1;
const dbName = `fusion_rel_${process.pid}_${relDbCounter}_${Math.random().toString(36).slice(2, 8)}`;
try {
await adminExecAsync(`DROP DATABASE IF EXISTS "${dbName}"`);
} catch {
// may not exist — safe to ignore
}
// The pid/counter/random name is new for every fixture; a pre-create DROP only adds contention.
await adminExecAsync(`CREATE DATABASE "${dbName}"`);
const testUrl = `${PG_TEST_URL_BASE}/${dbName}`;
const backend: ResolvedBackend = {
@@ -184,15 +192,24 @@ export async function createPgLayer(): Promise<PgLayerFixture> {
layer,
dbName,
cleanup: async () => {
try { await layer.close(); } catch { /* best-effort */ }
try { await adminExecAsync(`DROP DATABASE IF EXISTS "${dbName}"`); } catch { /* best-effort */ }
try {
await layer.close();
} catch (error) {
console.warn(`[reliability-fixtures] failed to close ${dbName}`, error);
}
try {
await adminExecAsync(`DROP DATABASE IF EXISTS "${dbName}" WITH (FORCE)`);
} catch (error) {
// Preserve best-effort teardown without concealing a persistent database leak.
console.warn(`[reliability-fixtures] failed to drop ${dbName}`, error);
}
},
};
}
/*
FNXC:PgMigrationQuarantine 2026-07-16-10:30:
VAL-REMOVAL-005 removed AgentStore's SQLite runtime path, so multi-node claim and handoff tests must construct TaskStore and every sibling AgentStore with one shared AsyncDataLayer. Reliability callers gate with hasGit && hasPg; integration callers compose hasPg ? pgDescribe : describe.skip because DDL requires both reachable PostgreSQL and psql, but integration tests do not require Git.
VAL-REMOVAL-005 removed AgentStore's SQLite runtime path, so multi-node claim and handoff tests must construct TaskStore and every sibling AgentStore with one shared AsyncDataLayer. Reliability callers gate with hasGit && hasPg; integration callers compose hasPg ? pgDescribe : describe.skip because DDL requires reachable PostgreSQL, but integration tests do not require Git.
*/
export async function makePgTaskStore(): Promise<{
rootDir: string;

3
pnpm-lock.yaml generated
View File

@@ -607,6 +607,9 @@ importers:
'@vitest/coverage-v8':
specifier: ^4.1.10
version: 4.1.10(vitest@4.1.10)
postgres:
specifier: 3.4.9
version: 3.4.9
typescript:
specifier: ^5.7.0
version: 5.9.3