feat(test): isolated-database test run as the trustworthy default (#614) - #623
Merged
Conversation
A full `bun test` against the shared dogfood database returns a different failure count each time on unmodified code. Measured on this branch, three consecutive runs: 37 → 41 → 37 fail. The same tree through the new entry point: 3710 pass / 0 fail, twice, identical. Shared mutable state is the contaminant, not the tests. `bun run test:isolated` (scripts/test-isolated.ts) creates a uniquely-named database, migrates it, points OPENBRAIN_TEST_DATABASE_URL at it, runs `bun test` with the caller's arguments, and drops the database on exit — including on SIGINT/SIGTERM/uncaught exception. A failed drop prints the orphan's name and the exact dropdb line rather than leaking quietly. No new mechanism is invented: creation is CI's `createdb -E UTF8 -T template0` (.github/workflows/ci.yml:68, by way of lane-bootstrap --fresh-db), migrations run through the existing `bun run migrate` path, and `bun test` itself is unchanged. Drops are prefix-guarded to `ob_isolated_`, so this file cannot name a database it did not create. AGENTS.md names it as the default and records that full-suite counts from the dogfood database are not evidence. Acceptance gate: scripts/done-means/614-isolated-test-run.sh — entry point runs the pg test against a printed non-dogfood database (and proves it was not silently skipped), the database is gone after exit, an interrupt does not leak silently, and AGENTS.md names the command. Closes #614 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bun run test:isolated(scripts/test-isolated.ts): creates a uniquely-named database, migrates it, pointsOPENBRAIN_TEST_DATABASE_URLat it, runsbun testwith the caller's arguments, prints the database name, and drops the database on exit — including on SIGINT/SIGTERM/uncaught exception.createdb -E UTF8 -T template0(.github/workflows/ci.yml:68, by way ofscripts/lane-bootstrap.ts:368--fresh-db), migrations run through the existingbun run migratepath withDB_NAMEoverridden (scripts/lane-bootstrap.ts:385), and the tests' existingOPENBRAIN_TEST_DATABASE_URLconvention is what is fed.bun testitself is unchanged and the dogfood database is never touched.AGENTS.mdnames it as the trustworthy default and records that full-suite counts from the dogfood database are not evidence (Full bun test run against the shared dogfood DB is untrustworthy as a gate #614). Barebun teststays documented for quick single-file iteration.scripts/done-means/614-isolated-test-run.sh.Headline evidence — the same tree, both ways
Run on this branch, unmodified code, back to back:
bun test→ dogfoodopen_brain_local_20260724bun run test:isolated→ fresh DBThe dogfood failure count moves on code that did not change, which is exactly #614's claim (49 → 43 → 40 observed 2026-08-06; 37 → 41 → 37 here). The isolated path is identical across runs and 3.5× faster. This does not fix the leakage — #613 is one concrete mechanism and is a separate lane — it makes the contaminated path stop being the default.
RLVR transcripts
RED, before
scripts/test-isolated.tsexisted (script exit 1):GREEN, after (script exit 0):
Check 1 asserts the pg test actually ran (>0 passing tests in a file that
describe.skips itself without the env var), not merely that the run was green — the silent-skip trap makes "0 fail" otherwise indistinguishable from "nothing ran". Check 3 sends SIGINT to the running process group after the tool has printed its database name, then asserts the database is gone, accepting a loud orphan (name plusdropdbline printed) as an alternative pass but failing a silent one.Verification
bunx tsc --noEmitclean; full suite 3710 pass / 35 skip / 0 fail through the new entry point (twice);scripts/done-means/614-isolated-test-run.shPASS.python/openbrain-memory/is touched.Post-run check
select datname from pg_database where datname like 'ob_isolated_%'returned zero rows — nothing leaked across every run in this lane.Critical Self-Review
dropdbcall. It is the only destructive operation, so it is constrained three ways — the name comes from one variable set bymakeDbName()seconds earlier in the same process, every generated name carries a fixedob_isolated_prefix, andteardown()refuses outright to drop a name lacking that prefix. There is no code path by which it can name the dogfood database or any database this process did not create, and no recursive or forced filesystem delete anywhere in the file..env/environment Postgres role cancreatedb— if it cannot, the run fails loudly atdb-createand creates nothing; (b) that migrations are the complete schema setup — verified by the pg tests actually passing, which they would not against an unmigrated database; (c) that no test hard-codes the dogfood database name and thereby escapes the isolation — not exhaustively audited, but the 0-fail result argues against any material case.scripts/test-isolated.tsitself; the done-means script is the behavioural test and covers creation, execution, clean teardown, and interrupt teardown end to end against real Postgres. Not covered: SIGKILL (uncatchable by construction — such a run leaves an orphan whose name was printed before creation, which is why the name is printed first), and a teardown failure while the cluster is unreachable (the loud-orphan branch is reasoned, not exercised).src/changes; nothing ships to core01.docs/downstream-rollout.mdapplies to MCP tool/schema/protocol/client-facing changes; this is a developer test-invocation script, apackage.jsonscript entry, and documentation. No consumer of Open Brain can observe it.bunx tsc --noEmitcaught TS5076 (??mixed with||without parentheses) in the orphan-message construction; fixed before commit. The RED transcript above was captured before the entry point existed, so the gate is proven to discriminate rather than to pass vacuously.parity-source-registry-*fixtures behind (Test suites leave parity-source-registry-* fixtures behind; any global producer makes them live #613) are untouched and remain that lane's scope, as instructed. A SIGKILLed run still orphans a database (loudly). The dogfood failures are now routed around rather than diagnosed; the ~37–41 failing tests on that database are not individually explained here.docs/sme/updated or [x] not applicable because: no new review finding — this implements an already-filed, already-diagnosed issue (Full bun test run against the shared dogfood DB is untrustworthy as a gate #614) whose pattern is recorded in that issue and in Test suites leave parity-source-registry-* fixtures behind; any global producer makes them live #613'sdocs/sme/correctness.mdentry.Review Gate
docs/sme/or explicitly marked not applicableContract Parity
scripts/test-isolated.tsis a local developer entry point with no runtime counterpart to keep in parity.Downstream Rollout
docs/downstream-rollout.mdNotes/evidence:
.github/workflows/ci.yml:68(createdb -E UTF8 -T template0),scripts/lane-bootstrap.ts:368(same call, already borrowed for--fresh-db),scripts/lane-bootstrap.ts:385(bun run migratewithDB_NAMEoverridden).