test(daemon): run the store suites on real PostgreSQL - #1080
Conversation
The daemon pool opens `LocalStore` over `PostgresSyncDatabase`, but every store test ran on SQLite only, so a SQLite-only construct reached a cluster before anything noticed. A `store-postgres` Vitest project now re-runs the store suites with `LocalStore` opened over the real pool store against a Testcontainers `postgres:16-alpine` — one container per run, one database per worker, a schema-wide sweep between tests. `test/store-postgres/backend.ts` is the seam: unarmed it leaves the SQLite run exactly as it was, armed it hands the suites `openPostgresLocalStore()`. `local-store-sql-portability.test.ts` is the cheap half: it reads the SQL text out of `local-store.ts` and fails on constructs the pool worker does not rewrite (two-argument `MAX`/`MIN`, `IFNULL`, `IIF`, `datetime()`, `INSERT OR REPLACE`, `GROUP_CONCAT`, `printf`, `TYPEOF`, `||`, comma `LIMIT`), so a statement no suite covers still fails fast. The PostgreSQL run caught one: the activation rendezvous joined its composite key and transcript coordinates with NUL, which PostgreSQL TEXT rejects outright, so every paired-activation write threw on the pool. The separator is now the unit separator; those rows are TTL-bounded, so no stored key outlives the change. Refs #1073, #1068
A dedicated Docker-bearing runner for the `store-postgres` project, off the critical path the way the control-plane integration shards are. The suites take about half a minute after the container boots, so the pool's SQL stops reaching a cluster untested.
There was a problem hiding this comment.
No blocking findings on a0b250710a49e9a050a612f60481d91a6632fdf5.
The PostgreSQL harness keeps each Vitest worker on a separate database, releases the schema advisory lock before the gated pool-store suites open their own connections, and prevents suite-level LocalStore.close() calls from closing the worker-wide backend. The activation rendezvous separator change is applied consistently to the production key/coordinate builders and their fixtures, and remains an opaque durable value as required by the rendezvous design.
Validation on this exact revision is green: Build, Check, Unit Test, Daemon Store (PostgreSQL), both integration shards, and Sandbox (Linux). The new PostgreSQL job reports 4 files passed with 99 tests passed and 10 SQLite-specific skips.
sent by review-bot (Codex · gpt-5.6-sol) · open in session
Summary
Closes #1073.
The daemon pool opens
LocalStoreoverPostgresSyncDatabase, but every store test ranagainst SQLite only, so a SQLite-only construct in
local-store.tsreached a clusterbefore anything noticed — #1068 caught a two-argument
MAX(x, y)in review, not in CI, andpostgres-pool-store.int.test.tswas gated onDATA_PLANE_TEST_DATABASE_URLand never ran.This adds the missing coverage in two layers: a Vitest project that re-runs the store
suites against a real
postgres:16-alpine, and a text check over the SQL itself for thestatements no suite reaches.
How the PostgreSQL project works
vitest.postgres.config.tsis a separate config file, not a second project insidevitest.config.ts, sovitest runand every targetedvitest run <file>already in CIstay Docker-free.
test/store-postgres/global-setup.tsboots onepostgres:16-alpineviaTestcontainers and creates one database per Vitest worker — the same shape as the
control-plane integration project.
test/store-postgres/setup.tsopens onePostgresSyncDatabaseper worker on thatworker's database, lets the first
LocalStorematerialize the schema through the realSQLite→PostgreSQL rewrite, releases the schema advisory lock, and truncates every store
table (plus rewinds the revision sequence) before each test.
test/store-postgres/backend.tsis the seam. Unarmed — the ordinary SQLite run — itreports
usingPostgresStore() === falseand the suites build theirnode:sqlitestoresexactly as before. Armed,
openPostgresLocalStore()hands them aLocalStoreover theworker's pool store, with a borrowed handle so a suite's
close()cannot take theworker-wide connection down.
Suites covered:
local-store,memory-capture-outbox,postgres-pool-store.int,postgres-transcript-org.int— 99 tests, 10 skipped. The skips are the suites that areabout SQLite internals and nothing else: schema versioning through
PRAGMA user_version,the v10 transcript-org migration replay, and two cases that reach for
DatabaseSyncdirectly to stage a race.
The gated
.intsuites now run for real: the setup file pointsDATA_PLANE_TEST_DATABASE_URLat the worker's database.ci: gate the daemon store suites on real PostgreSQLadds aDaemon Store (PostgreSQL)job on its own Docker-bearing runner, off the critical path, like the control-plane
integration shards.
What it caught
The activation rendezvous (
send-message-routing-rework.md§3.2/§8.6) joined its compositekey and its transcript coordinates with NUL, and PostgreSQL
TEXTrejects0x00outright:
invalid byte sequence for encoding "UTF8": 0x00. On the pool everyclaimActivationObservation/attachActivationEnvelopewrite threw, so paired activation— the agent-to-agent and relay-forwarded hand-off — could not settle at all. Eleven tests
failed on the first PostgreSQL run for that one reason.
The fix is portable, in the key builder rather than the worker: the separator is now the
unit separator (
U+001F). Those rows are TTL-bounded pairing records, so no stored keyoutlives the change.
The cheap half
test/local-store-sql-portability.test.tsparseslocal-store.ts, pulls out everySQL-shaped string and template literal, and fails on constructs the pool worker does not
rewrite: two-argument
MAX/MIN,IFNULL,IIF,datetime/strftime/julianday/unixepoch,INSERT OR REPLACE|ABORT|FAIL,GROUP_CONCAT,printf,TYPEOF,||concatenation and comma
LIMIT. A statement may opt out with a-- pg-portable-exempt:marker and a reason. It runs in the ordinary unit job, so a statement no suite covers still
fails fast, and it carries its own self-check so the patterns cannot rot into matching
nothing.
What the worker already translates —
INSERT OR IGNORE,BEGIN IMMEDIATE,INTEGER PRIMARY KEY AUTOINCREMENT,PRAGMA user_version,sqlite_master,LIMIT -1 OFFSET,length(CAST(x AS BLOB))— stays legal, and the check's header says thelist grows by making SQL portable, never by teaching the worker another function name.
Test plan
pnpm --filter @agentconnect.md/daemon test:store:postgres— 99 passed, 10 skipped,~23s after the container boots.
pnpm --filter @agentconnect.md/daemon exec vitest run test/local-store.test.ts test/memory-capture-outbox.test.ts test/local-store-permissions.test.ts test/local-store-sql-portability.test.ts test/daemon-agent-mention-routing.test.ts test/postgres-*.test.ts— the SQLite run, unchanged: 151 passed.pnpm --filter @agentconnect.md/daemon test— the pre-existing local failures(
acp-matrixlive runtimes, theshim-*macOS sandbox suites,workspace-git-runner-seam) reproduce on a clean checkout ofmainand are untouchedby this change.
pnpm --filter @agentconnect.md/daemon typecheck,pnpm lint,pnpm format:check.