Skip to content

test(server): pin the PostgreSQL agent store contract - PgAgentStore (Closes #1658) - #1932

Merged
dmitrii-f-t27 merged 1 commit into
queenfrom
zcode/tests-1658-pg-agent-store
Oct 13, 2026
Merged

dmitrii-f-t27 merged 1 commit into
queenfrom
zcode/tests-1658-pg-agent-store

Conversation

@dmitrii-f-t27

Copy link
Copy Markdown
Collaborator

Closes #1658

What changed

One new file: trios/agent-server/apps/server/tests/api/pg-agent-store.test.ts (the path the issue's Success Criteria names). The subject pg-agent-store.ts is untouched.

The single exported class is exercised (7 it( blocks) through a recording fake of the createQueenPool seam - the pool interface the store uses (query, end, on):

  • Lifecycle (2 tests): queries before connect answer PgAgentStore not connected; connect builds exactly one pool for the DSN (a second connect is a no-op); disconnect ends it and further queries are refused.
  • Statements (5 tests): the upsert's ON CONFLICT (id) DO UPDATE with the card fields and the JSON metadata params; the heartbeat's touch-count answer (true on rowCount 1, false on 0); the offline UPDATE and the DELETE with their params; the row-to-card mapping with the online filter measured in the WHERE clause (and its absence for listAgents(false)); the matrix view read and the prune answering the ids it took offline with the interval in the SQL.

The mock is process-global in bun's shared CI process, so it is wrapped in the repo's proven snapshot-and-restore pattern from queen-roadmap.test.ts: the real module is copied to a plain object BEFORE the mock and re-installed in afterAll. Measured: without the restore, the full tests/api run shows 55 failures in the later queen-roadmap, task-queue and vault suites; with it, the run is back to the 8 pre-existing local vault failures (missing age CLI on this machine - CI installs it). No database, no network, no container.

How it was verified (FR-004)

  • Passing run against the unmodified subject: 7 pass / 0 fail / 23 expect() calls, Ran 7 tests across 1 file.
  • Deliberately broken copy (connect un-nulls the pool and re-creates with a mutated DSN; heartbeat always true; the online filter inverted to offline; the prune ids prefixed): 4 fail out of 7, quoted raw above. The subject was restored byte-identical (git checkout) and the suite passes again.
  • The full tests/api one-process run with this suite is 2150 tests / 8 fail - identical to the baseline failure set (the local vault/age failures), proving the snapshot-and-restore contains the mock. The file is biome ci-clean and contributes no typecheck error (local monorepo typecheck: base set, 0 mentions of the new file).

Not verified

  • Nothing beyond the quoted local runs; the CI Tests / server-api job re-runs the suite on this SHA.

Dmitrii Fedorov (@dmitrii-f-t27)

The first suite for pg-agent-store.ts: the class exercised
(7 tests) through a recording fake of the createQueenPool seam,
snapshotted-and-restored with the repo's proven queen-roadmap
pattern (the real module copied before the mock and re-installed
in afterAll - measured: without the restore the shared CI process
shows 55 failures in the later queen, task-queue and vault
suites). Pinned: the not-connected refusal, the one-pool lifecycle,
the upsert ON CONFLICT statement with its JSON metadata params, the
heartbeat touch-count answer, the offline/delete statements, the
row-to-card mapping with the online filter in the WHERE clause, the
matrix view read and the prune id answer. No database, no network,
no container; the subject is untouched.

Closes #1658
@tri-claw

tri-claw Bot commented Oct 13, 2026

Copy link
Copy Markdown

Summary by t27-bees

Worth a look

  • trios/agent-server/apps/server/tests/api/pg-agent-store.test.ts (global pools array and nextResult variable): Test isolation is broken. The pools array accumulates across all tests (each connect() pushes a new record) and nextResult is a single mutable object overwritten by tests like heartbeat and pruneOffline. If tests run in parallel (Bun default) or out of order, they will read each other's pool records and result stubs. No beforeEach/afterEach resets this state.

  • trios/agent-server/apps/server/tests/api/pg-agent-store.test.ts:123 (pool.calls[pool.calls.length - 1] in listAgents test): Relies on the last call in the current pool being the listAgents query. If any helper or setup code issues a query on the same pool before listAgents, the assertion checks the wrong call. The test creates a fresh store/pool so it holds today, but it's a fragile coupling to call order.

  • trios/agent-server/apps/server/tests/api/pg-agent-store.test.ts:148 (pool.calls[pool.calls.length - 1] in listMatrix/pruneOffline test): Same fragility β€” the test does two operations on the same pool (listMatrix then pruneOffline) and checks the last call for each, but if listMatrix issues multiple queries the pruneOffline check could accidentally inspect the listMatrix call.

  • trios/agent-server/apps/server/tests/api/pg-agent-store.test.ts:38 (afterAll mock restore only): The mock is restored but pools and nextResult are not cleared. If another test file in the same process imports this file's side effects (e.g., via a shared setup), it inherits the polluted globals. The repo's snapshot pattern handles the module mock, not the test's own mutable state.

  • trios/agent-server/apps/server/tests/api/pg-agent-store.test.ts:12 (const realQueenPool = { ...(await import(...)) }): The spread of a namespace import (import * as ns) copies only own enumerable properties at snapshot time. If queen-pool exports a default or non-enumerable members, they are lost in the restore. The pattern works for the current export shape but is not robust to module structure changes.


Read: 1 files, +254 -0, the whole diff. Head ec72497. Model: openai-compatible/nvidia/nemotron-3-ultra-550b-a55b. Commands: @t27-bees review | summary | help | pause | resume. Free for every repository.

@github-actions

Copy link
Copy Markdown

βœ… Tests passed β€” 3551/3621

Suite Passed Failed Skipped
βœ… agent 186/186 0 0
βœ… build 9/9 0 0
βœ… cdp-protocol 5/5 0 0
βœ… eval 93/93 0 0
βœ… server-agent 280/280 0 0
βœ… server-api 2152/2221 0 69
βœ… server-browser 6/6 0 0
βœ… server-integration 10/11 0 1
βœ… server-lib 279/279 0 0
βœ… server-pglive 166/166 0 0
βœ… server-root 68/68 0 0
βœ… server-sim 8/8 0 0
βœ… server-skills 31/31 0 0
βœ… server-tools 244/244 0 0
βœ… shared 14/14 0 0

View workflow run

@dmitrii-f-t27

Copy link
Copy Markdown
Collaborator Author

Checks note, before merge:

  • runner / Biome and runner / Typecheck are red on the base branch queen with pre-existing failures (reference base run https://github.com/gHashTag/trios/actions/runs/38087839871 on dfb6d97). On this PR's SHA: the Typecheck log has the 19 base error TS lines and 0 mentions of the new pg-agent-store.test.ts; the Biome log has 0 diagnostics for the new file. All Tests / * suites including server-pglive are green on this SHA, and the mock is snapshot-and-restored around this file (the full api run stays at the pre-existing failure set), so it cannot affect sibling suites.

Dmitrii Fedorov (@dmitrii-f-t27)

@dmitrii-f-t27
dmitrii-f-t27 merged commit 3fade42 into queen Oct 13, 2026
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant