From abac3078b9b58ae2b9d49ef88e39487a7a20cb58 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 15:45:09 +0000 Subject: [PATCH 1/2] test(service-queue): pin db-queue-adapter's engine double to ObjectQL.delete's dispatch predicate (#5198) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DEBT entry for `db-queue-adapter.test.ts` in `scripts/engine-double-contract.baseline.json` said pinning was blocked on a devDependency change. #5192 already made that change (it needed `@objectstack/objectql` to pin the new `job-queue-retention.test.ts` fake), so the entry's stated blocker no longer existed and its `why` would have misled the next reader. Replace the hand-mirrored `if (opts?.where?.id == null) throw` in the fake engine with `assertEngineDeleteDispatch(opts)` and route on the returned verdict, then delete the ledger entry. The mirror was looser than the producer in BOTH directions, which is why it is replaced rather than corrected in place: - it ACCEPTED `where: { id: { $in: [...] } }` and `where: { id: ['a','b'] }` without `multi` — multi-row predicates the real engine rejects; - it THREW on `{ multi: true }`, which the real engine accepts. Also drive the fake against the producer's own published case-set (`ENGINE_DELETE_DISPATCH_CASES`) so the double's fidelity is measured, not assumed: the gate proves the predicate is CALLED, these tests prove the call is answered. Restoring the mirror turns 5 of the 14 new tests red and the gate red with a PINNED error — both directions verified. Refs #5192, #5179, #4550, #4434 --- .../src/db-queue-adapter.test.ts | 102 ++++++++++++++++-- scripts/engine-double-contract.baseline.json | 7 -- 2 files changed, 94 insertions(+), 15 deletions(-) diff --git a/packages/services/service-queue/src/db-queue-adapter.test.ts b/packages/services/service-queue/src/db-queue-adapter.test.ts index 565b2ceb59..1ddb8edfea 100644 --- a/packages/services/service-queue/src/db-queue-adapter.test.ts +++ b/packages/services/service-queue/src/db-queue-adapter.test.ts @@ -1,6 +1,11 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach } from 'vitest'; +import { + ENGINE_DELETE_DISPATCH_CASES, + ENGINE_DELETE_REJECT_MESSAGE, + assertEngineDeleteDispatch, +} from '@objectstack/objectql'; import { DbQueueAdapter } from './db-queue-adapter.js'; /** @@ -54,15 +59,26 @@ function makeFakeEngine() { return r; }, async delete(table: string, opts: any) { - // Real-engine contract: the target id lives at `where.id` — there is no - // top-level `id` option. The mock used to accept `opts.id`, a signature - // the real engine rejects, which is exactly how the adapter's broken - // `{ id }` bags stayed green (#4371 option-2 survey). - const id = opts?.where?.id; - if (id == null) throw new Error('Delete requires an ID or options.multi=true'); + // [#4550/#5198] Opened with ObjectQL.delete's OWN dispatch predicate. + // What stood here was a hand-mirrored `if (opts?.where?.id == null)` — + // written for #4371 to stop the mock accepting the top-level `{ id }` + // bags the real engine rejects, and correct about that. But a mirror is a + // second copy of the contract, and it was looser than the producer in + // both directions: `where: { id: { $in: [...] } }` only LOOKS like an id + // (a multi-row predicate the engine refuses without `multi`) and the + // mirror waved it through, while `{ multi: true }` with no `where` is a + // shape the engine ACCEPTS and the mirror threw on. A double that imports + // the decision cannot drift from it; the same predicate already opens the + // sibling `job-queue-retention.test.ts` fake in this package. + const dispatch = assertEngineDeleteDispatch(opts); const t = tables.get(table) ?? []; - tables.set(table, t.filter((r) => r.id !== id)); - return { id }; + if (dispatch.kind === 'multi') { + const keep = t.filter((r) => !matches(r, opts?.where ?? {})); + tables.set(table, keep); + return t.length - keep.length; // drivers report a deleted count + } + tables.set(table, t.filter((r) => r.id !== dispatch.id)); + return { id: dispatch.id }; }, }; } @@ -232,3 +248,73 @@ describe('DbQueueAdapter', () => { expect(await adapter.getQueueSize('mix')).toBe(0); }); }); + +// --------------------------------------------------------------------------- +// The double itself, measured against the producer (#4550 / #5198) +// --------------------------------------------------------------------------- +// +// Every assertion above is only worth what this fake's fidelity is worth: a +// double that accepts a call the real engine refuses turns a green suite into +// no suite at all, on exactly the path the double was introduced for (#4434). +// So the fake is driven against ObjectQL.delete's OWN published case-set here, +// rather than trusted because its `delete` now names the right function. +// +// This is what the deleted `scripts/engine-double-contract.baseline.json` DEBT +// entry bought, and why the entry could go: the gate proves the predicate is +// CALLED, and these two tests prove the call is answered — the by-id and multi +// branches route by verdict, and every shape the engine rejects the fake +// rejects, with the producer's own message. + +describe('makeFakeEngine().delete conforms to ObjectQL.delete (#4550)', () => { + it.each(ENGINE_DELETE_DISPATCH_CASES.map((c) => [c.what, c] as const))( + 'agrees with the engine on %s', + async (_what, c) => { + const engine = makeFakeEngine(); + engine.tables.set('sys_job_queue', [{ id: 'rec_1', rule_id: 'r1' }]); + const call = engine.delete('sys_job_queue', c.options as any); + if (c.expect === 'reject') { + // Not merely "throws": the same message a real server answers with, so + // the fake's rejection surface cannot drift from the producer's. + await expect(call).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE); + // …and a refused call must not have deleted anything on its way out. + expect(engine.tables.get('sys_job_queue')).toHaveLength(1); + return; + } + await expect(call).resolves.toBeDefined(); + }, + ); + + it('routes by the verdict, not by guessing at `where`', async () => { + const engine = makeFakeEngine(); + const rows = () => engine.tables.get('sys_job_queue') ?? []; + + // by-id: the scalar id is the ONLY row removed, siblings survive. + engine.tables.set('sys_job_queue', [{ id: 'a' }, { id: 'b' }]); + expect(await engine.delete('sys_job_queue', { where: { id: 'a' } })).toEqual({ id: 'a' }); + expect(rows().map((r: any) => r.id)).toEqual(['b']); + + // multi: the predicate matches many, and the fake reports the count a + // driver's `deleteMany` reports. + engine.tables.set('sys_job_queue', [ + { id: 'a', status: 'completed' }, + { id: 'b', status: 'completed' }, + { id: 'c', status: 'pending' }, + ]); + expect( + await engine.delete('sys_job_queue', { where: { status: 'completed' }, multi: true }), + ).toBe(2); + expect(rows().map((r: any) => r.id)).toEqual(['c']); + + // `where: { id: { $in: […] } }` only LOOKS like an id: it is a multi-row + // predicate, so without `multi` the engine rejects it — the exact case a + // hand-mirrored `if (opts?.where?.id == null)` waves through, and the reason + // the mirror had to go rather than be corrected in place. (This fake's + // `matches` is equality-only by design — no fixture here sends an operator + // predicate — so what is pinned is the dispatch verdict, not `$in` matching.) + engine.tables.set('sys_job_queue', [{ id: 'a' }, { id: 'b' }, { id: 'c' }]); + await expect( + engine.delete('sys_job_queue', { where: { id: { $in: ['a', 'b'] } } }), + ).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE); + expect(rows()).toHaveLength(3); + }); +}); diff --git a/scripts/engine-double-contract.baseline.json b/scripts/engine-double-contract.baseline.json index 539da11399..32938c004a 100644 --- a/scripts/engine-double-contract.baseline.json +++ b/scripts/engine-double-contract.baseline.json @@ -241,13 +241,6 @@ "why": "@objectstack/service-datasource does not depend on @objectstack/objectql. Pinning needs a devDependency + lockfile change, which is a separate reviewable act.", "closes": "add @objectstack/objectql to devDependencies, then open the fake's delete with assertEngineDeleteDispatch(opts)" }, - { - "file": "packages/services/service-queue/src/db-queue-adapter.test.ts", - "unguarded": 1, - "kind": "DEBT", - "why": "HAND-MIRRORS the guard already, which is the second copy of the contract this gate exists to remove — but @objectstack/service-queue does not depend on @objectstack/objectql, so replacing the copy with the producer's predicate needs a devDependency change.", - "closes": "add @objectstack/objectql to devDependencies, then replace the mirrored `if` with assertEngineDeleteDispatch(options)" - }, { "file": "packages/spec/src/contracts/data-engine.test.ts", "unguarded": 1, From badff7ce21aaf79455a7eb74564ff057890289cc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 15:49:06 +0000 Subject: [PATCH 2/2] test(service-queue): keep #4371's top-level `{ id }` rejection explicitly pinned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hand-mirrored `if` this PR removed was originally written for #4371 — a mock that accepted `delete(table, { id })` when the real engine reads `where.id`. `assertEngineDeleteDispatch` still refuses that bag, but it is not one of ENGINE_DELETE_DISPATCH_CASES, so nothing in the file asserted it any more. Assert it directly rather than let the property lapse with the code that used to carry it. --- .../services/service-queue/src/db-queue-adapter.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/services/service-queue/src/db-queue-adapter.test.ts b/packages/services/service-queue/src/db-queue-adapter.test.ts index 1ddb8edfea..cc08ceb2ec 100644 --- a/packages/services/service-queue/src/db-queue-adapter.test.ts +++ b/packages/services/service-queue/src/db-queue-adapter.test.ts @@ -316,5 +316,14 @@ describe('makeFakeEngine().delete conforms to ObjectQL.delete (#4550)', () => { engine.delete('sys_job_queue', { where: { id: { $in: ['a', 'b'] } } }), ).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE); expect(rows()).toHaveLength(3); + + // The property the removed mirror was ORIGINALLY written for (#4371): a + // top-level `{ id }` bag is not an address — the id lives at `where.id`. + // It is not one of ENGINE_DELETE_DISPATCH_CASES, so it is asserted here + // rather than left to lapse with the code that used to carry it. + await expect( + engine.delete('sys_job_queue', { id: 'a' } as any), + ).rejects.toThrow(ENGINE_DELETE_REJECT_MESSAGE); + expect(rows()).toHaveLength(3); }); });