From 4b69c9c085e4858b25a5e14b5ffbd505ae41a5cc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 17:00:53 +0000 Subject: [PATCH] feat(objectql): pin fake engines to ObjectQL.delete's dispatch contract (#4550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4550 records a failure mode with four instances: a test double LOOSER than the implementation it replaces converts a green suite into no suite at all — silently, and on exactly the paths a double was introduced for, which are the paths that were hard to test, which are usually where the contract is densest. This takes ONE slice of it, the one whose criterion is mechanically decidable: the ObjectQL engine's delete dispatch. `ObjectQL.delete(object, options)` is a total function from an options bag to three verdicts — `by-id` (scalar `where.id`), `multi` (`options.multi`), or a throw — so "is this double looser?" has a yes/no answer that does not require reading the test's intent. The producer's decision now lives in one place. `engine-delete-dispatch.ts` exports `resolveEngineDeleteDispatch` / `assertEngineDeleteDispatch` / `scalarDeleteId` / `ENGINE_DELETE_DISPATCH_CASES`, `ObjectQL.delete` itself reads it, and a fake engine calls it instead of mirroring it. #4434's fix mirrored the guard by hand into one fake; a mirror is a second copy of the contract and drifts the moment either side is edited — and the scalar test is the half a mirror drops (`where: { id: { $in: [...] } }` looks like an id and is a multi-row predicate). A double that imports the decision cannot be looser than the decision. `scripts/check-engine-double-contract.mjs` (wired into lint.yml's ESLint job, `--self-test` first per the repo's 10 other gates) finds all 39 fake ObjectQL engines by AST — separating them from the 39 DRIVER doubles, whose `delete` takes a scalar id and is a different contract — and requires the pinned call. 9 are converted here (objectql's five sys_metadata fakes, plugin-sharing's four, including the two that hand-mirrored); the other 30 sit in a measured, shrink-only baseline that reconciles in both directions. Proof it discriminates, not just that it is green: 1. `git show ba5ff2f28^:…/sharing-rule.test.ts` — the pre-#4434 fake — restored: the gate goes red naming that file and line. 2. With the fake pinned and the pre-#4434 `deleteRule` restored, the PRE-EXISTING test `deleteRule drops rule + all its grants` fails with `Delete requires an ID or options.multi=true` — the same error the running server answered 500 with. Had this gate existed, #4434 could not have shipped. Verified: objectql 111 files / 1757 tests, plugin-sharing 11 / 243, both typecheck clean, ESLint clean. @objectstack/spec ran 295/295 files green with the dispatch guard installed in every engine double, which is why its type-conformance witness is EXEMPT rather than DEBT. Refs #4550, #4434 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ --- .changeset/engine-double-delete-contract.md | 53 ++ .github/workflows/lint.yml | 17 + package.json | 3 +- .../src/engine-delete-dispatch.test.ts | 106 ++++ .../objectql/src/engine-delete-dispatch.ts | 168 ++++++ packages/objectql/src/engine.ts | 26 +- packages/objectql/src/index.ts | 19 + .../src/layered-overlay-integration.test.ts | 5 + .../src/protocol-publish-rollback.test.ts | 5 + .../src/protocol-save-meta-repo-path.test.ts | 5 + .../protocol-view-identity-overlay.test.ts | 5 + .../src/sys-metadata-repository.test.ts | 5 + .../plugin-sharing/src/boot-backfill.test.ts | 13 +- .../src/sharing-rule-provenance.test.ts | 4 + .../plugin-sharing/src/sharing-rule.test.ts | 37 +- .../src/sharing-service.test.ts | 4 + scripts/check-engine-double-contract.mjs | 557 ++++++++++++++++++ scripts/engine-double-contract.baseline.json | 238 ++++++++ 18 files changed, 1234 insertions(+), 36 deletions(-) create mode 100644 .changeset/engine-double-delete-contract.md create mode 100644 packages/objectql/src/engine-delete-dispatch.test.ts create mode 100644 packages/objectql/src/engine-delete-dispatch.ts create mode 100644 scripts/check-engine-double-contract.mjs create mode 100644 scripts/engine-double-contract.baseline.json diff --git a/.changeset/engine-double-delete-contract.md b/.changeset/engine-double-delete-contract.md new file mode 100644 index 0000000000..f06788f305 --- /dev/null +++ b/.changeset/engine-double-delete-contract.md @@ -0,0 +1,53 @@ +--- +"@objectstack/objectql": minor +--- + +feat(objectql): export the delete-dispatch contract so test doubles can be pinned to it (#4550) + +A test double that is **looser** than the implementation it replaces converts a +green suite into no suite at all — silently, and on exactly the paths a double +was introduced for, which are the paths that were hard to test, which are +usually where the contract is densest. #4434 is the worked example: +`DELETE /api/v1/sharing/rules/:idOrName` answered 500 for every rule and both +address forms it advertises, from the day it was written, while +`deleteRule drops rule + all its grants` asserted success against it the whole +time — against a fake engine whose `delete` accepted the one call shape +`ObjectQL.delete` refuses. + +`ObjectQL.delete`'s dispatch decision now lives in one exported place instead of +being re-derived by every fake: + +```ts +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; + +async delete(object: string, options?: any) { + assertEngineDeleteDispatch(options); // refuses what a real server refuses + … +} +``` + +New exports, all pure and side-effect free: + +- `resolveEngineDeleteDispatch(options)` → `{ kind: 'by-id', id }` | + `{ kind: 'multi' }` | `{ kind: 'reject', message }` — what the engine will do + with this call, without doing it. +- `assertEngineDeleteDispatch(options)` — throws exactly what the engine throws + on `reject`, returns the dispatch otherwise. This is the line a fake engine's + `delete` opens with. +- `scalarDeleteId(options)` — the SCALAR `where.id` or `undefined`. The half a + hand-written mirror drops: `where: { id: { $in: [...] } }` looks like an id + and is a multi-row predicate, so the engine rejects it without `multi`. +- `ENGINE_DELETE_REJECT_MESSAGE`, `ENGINE_DELETE_DISPATCH_CASES` — the message + and the shared conformance case-set, the same role + `packages/spec/src/data/*-conformance.ts` plays for drivers. + +`ObjectQL.delete` itself reads `resolveEngineDeleteDispatch`, so a double that +imports it cannot be looser than the engine, ever — that is the property, and +it is the one a hand-mirrored `if` can only have until somebody edits one side. +No runtime behaviour changes: the same three verdicts, over the same inputs, +proved case-by-case against the real engine in +`engine-delete-dispatch.test.ts`. + +Repo-side, `pnpm check:engine-double-contract` (wired into `lint.yml`) finds all +39 fake ObjectQL engines in the repo, holds new ones to this predicate, and +keeps the 30 not yet converted in a measured, shrink-only baseline. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index dcd8419ad0..8fecffd980 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -275,6 +275,23 @@ jobs: - name: Published-files whitelist guard run: pnpm check:published-files + # Engine test-double contract gate (#4550, from #4434). A test double + # LOOSER than the implementation it replaces turns a green suite into no + # suite at all, silently, on exactly the paths a double was introduced + # for. #4434 is the worked example: DELETE /sharing/rules/:idOrName + # answered 500 for every rule and both address forms from the day it was + # written, while `deleteRule drops rule + all its grants` asserted success + # against a fake engine that accepted the one call shape ObjectQL.delete + # refuses. This holds every fake ObjectQL engine's `delete` to the real + # dispatch predicate — imported from @objectstack/objectql, not + # hand-mirrored, so it cannot drift — with the pre-existing fakes in a + # shrink-only, measured baseline. Static AST only, so it needs no build + # and belongs in this job. Runs its own --self-test first: the detector + # can be broken while every fake is fine, and a scan that quietly stops + # matching would report OK while reading nothing (#4868's family). + - name: Engine test-double contract gate + run: pnpm check:engine-double-contract + typecheck: name: TypeScript Type Check runs-on: ubuntu-latest diff --git a/package.json b/package.json index 3b09b3343f..27a225cfd7 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,8 @@ "check:node-version": "node scripts/check-node-version.mjs", "check:published-files": "node scripts/check-published-files.mjs --self-test && node scripts/check-published-files.mjs", "check:type-check-coverage": "node scripts/check-type-check-coverage.mjs --self-test && node scripts/check-type-check-coverage.mjs", - "check:driver-conformance": "node scripts/check-driver-conformance.mjs --self-test && node scripts/check-driver-conformance.mjs" + "check:driver-conformance": "node scripts/check-driver-conformance.mjs --self-test && node scripts/check-driver-conformance.mjs", + "check:engine-double-contract": "node scripts/check-engine-double-contract.mjs --self-test && node scripts/check-engine-double-contract.mjs" }, "keywords": [ "objectstack", diff --git a/packages/objectql/src/engine-delete-dispatch.test.ts b/packages/objectql/src/engine-delete-dispatch.test.ts new file mode 100644 index 0000000000..3f3ecd3584 --- /dev/null +++ b/packages/objectql/src/engine-delete-dispatch.test.ts @@ -0,0 +1,106 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// objectstack#4550 — the shared delete-dispatch predicate must be the REAL +// engine's answer, not a second opinion that happens to agree today. +// +// A shared predicate that drifted from `ObjectQL.delete` would be worse than +// no predicate at all: every fake engine pinned to it would be confidently, +// uniformly wrong, and the gate over them would report success (route-ownership +// rule 3 — prefer failing to falling back). So this file does not test the +// predicate against a table of expectations written next to it. It drives the +// **real engine** with a recording driver over `ENGINE_DELETE_DISPATCH_CASES` +// and asserts the engine's observed behaviour equals the predicate's verdict, +// case by case. +// +// If someone changes the dispatch rule in `engine.ts` without changing +// `engine-delete-dispatch.ts`, this goes red here — the one place where both +// halves are in the room together. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { + ENGINE_DELETE_DISPATCH_CASES, + ENGINE_DELETE_REJECT_MESSAGE, + resolveEngineDeleteDispatch, + assertEngineDeleteDispatch, + scalarDeleteId, +} from './engine-delete-dispatch.js'; + +/** Records which driver entry point the engine chose, if any. */ +function makeRecordingDriver() { + const calls: Array<{ fn: 'delete' | 'deleteMany'; arg: unknown }> = []; + const driver: any = { + name: 'recording', + version: '0.0.0', + supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find() { return []; }, + async findOne() { return null; }, + async create(_o: string, data: Record) { return { id: 'r1', ...data }; }, + async update(_o: string, id: string, data: Record) { return { id, ...data }; }, + async delete(_o: string, id: string) { calls.push({ fn: 'delete', arg: id }); return true; }, + async deleteMany(_o: string, ast: unknown) { calls.push({ fn: 'deleteMany', arg: ast }); return 0; }, + async count() { return 0; }, + async bulkCreate() { return []; }, async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, calls }; +} + +async function makeEngine() { + const engine = new ObjectQL(); + const { driver, calls } = makeRecordingDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ name: 'task', fields: { title: { type: 'text' } } } as any); + return { engine, calls }; +} + +/** What the real engine actually did with this options bag. */ +async function observeEngine(options: unknown): Promise<'by-id' | 'multi' | 'reject'> { + const { engine, calls } = await makeEngine(); + try { + await engine.delete('task', options as any); + } catch (e) { + if ((e as Error).message === ENGINE_DELETE_REJECT_MESSAGE) return 'reject'; + throw e; + } + if (calls.length !== 1) { + throw new Error(`expected exactly one driver call, saw ${JSON.stringify(calls)}`); + } + return calls[0].fn === 'delete' ? 'by-id' : 'multi'; +} + +describe('engine delete dispatch — the shared predicate IS the engine (#4550)', () => { + it('has cases on both sides of the guard (an empty or one-sided set proves nothing)', () => { + const kinds = new Set(ENGINE_DELETE_DISPATCH_CASES.map((c) => c.expect)); + expect(kinds).toEqual(new Set(['by-id', 'multi', 'reject'])); + expect(ENGINE_DELETE_DISPATCH_CASES.filter((c) => c.expect === 'reject').length).toBeGreaterThan(3); + }); + + for (const c of ENGINE_DELETE_DISPATCH_CASES) { + it(`real engine agrees with the predicate: ${c.what} → ${c.expect}`, async () => { + expect(resolveEngineDeleteDispatch(c.options).kind, 'predicate').toBe(c.expect); + expect(await observeEngine(c.options), 'real ObjectQL.delete').toBe(c.expect); + }); + } + + it('rejects with the exact message a fake must reproduce', () => { + expect(() => assertEngineDeleteDispatch({ where: { rule_id: 'r1' } })) + .toThrow(ENGINE_DELETE_REJECT_MESSAGE); + // …and returns the dispatch (never `reject`) when the call is legal. + expect(assertEngineDeleteDispatch({ where: { id: 'a' } })).toEqual({ kind: 'by-id', id: 'a' }); + expect(assertEngineDeleteDispatch({ multi: true })).toEqual({ kind: 'multi' }); + }); + + it('scalarDeleteId treats operator objects and arrays as predicates, not ids', () => { + expect(scalarDeleteId({ where: { id: 'a' } })).toBe('a'); + expect(scalarDeleteId({ where: { id: 7 } })).toBe(7); + expect(scalarDeleteId({ where: { id: { $in: ['a'] } } })).toBeUndefined(); + expect(scalarDeleteId({ where: { id: ['a'] } })).toBeUndefined(); + expect(scalarDeleteId({ where: { id: null } })).toBeUndefined(); + expect(scalarDeleteId({ where: {} })).toBeUndefined(); + expect(scalarDeleteId(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/engine-delete-dispatch.ts b/packages/objectql/src/engine-delete-dispatch.ts new file mode 100644 index 0000000000..27366cabbe --- /dev/null +++ b/packages/objectql/src/engine-delete-dispatch.ts @@ -0,0 +1,168 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The **one** answer to "what does `ObjectQLEngine.delete` do with this call?" + * — extracted so that the engine and every test double that stands in for it + * read the same predicate rather than two hand-written approximations of it + * (objectstack#4550, from objectstack#4434). + * + * ## Why this is a shared module and not four lines inside `engine.ts` + * + * `#4434` shipped green. `DELETE /api/v1/sharing/rules/:idOrName` answered 500 + * for **both** address forms the route advertises, for every rule, from the day + * it was written — and `plugin-sharing`'s `deleteRule drops rule + all its + * grants` test asserted success against it the whole time. The route was not + * untested; it was tested against a **fake engine whose `delete` accepted a + * call the real engine refuses**. A predicate-shaped purge of + * `sys_record_share` (no scalar `where.id`, no `options.multi`) is precisely + * the one shape `delete()` throws on, and the fake happily deleted by + * predicate. + * + * The fix for #4434 mirrored the guard into that fake by hand. That closes one + * fake and starts a second copy of the contract — the failure mode this module + * exists to remove. A double that *imports the producer's own decision* cannot + * be looser than the producer, ever, which is the property the gate wants and + * the property a copy can only have until someone edits one side. + * + * Same reasoning as `packages/spec/src/data/*-conformance.ts` for drivers, and + * the same shape as objectstack#4455: **the scan and the validator must answer + * with one predicate.** + * + * ## The contract, normatively + * + * `delete(object, options)` dispatches on exactly one question — *does this + * call identify a single row by primary key?* + * + * - `options.where.id` is a **scalar** (`string` / `number` / `bigint`, not + * `null`) → `by-id`: routes to `driver.delete`, runs cascade-delete and the + * by-id RLS pre-image check. + * - otherwise, `options.multi` is truthy → `multi`: routes to + * `driver.deleteMany` with the middleware-composed AST. + * - otherwise → **`reject`**. The call names neither one row nor a bulk + * intent, and the engine throws rather than guessing. + * + * The scalar test is load-bearing and is the half a hand-written double most + * often drops: `where: { id: { $in: [...] } }` is a *multi-row predicate*, not + * an id. Treating it as an id would bind the operator object literally into + * `driver.delete(object, {$in: […]})` **and** skip both the row-scoping AST + * seeding (#2982) and the by-id pre-image check. So it is `reject` unless the + * caller also said `multi`. + * + * @see ObjectQL.delete in `engine.ts` — the only production caller. + * @see scripts/check-engine-double-contract.mjs — the gate that keeps doubles on it. + */ + +/** The message `delete()` throws when a call identifies neither one row nor a bulk intent. */ +export const ENGINE_DELETE_REJECT_MESSAGE = 'Delete requires an ID or options.multi=true'; + +/** What `ObjectQLEngine.delete` will do with a given options bag. */ +export type EngineDeleteDispatch = + /** A scalar `where.id` — `driver.delete`, cascade + by-id RLS pre-image. */ + | { readonly kind: 'by-id'; readonly id: string | number | bigint } + /** No single id but `options.multi` — `driver.deleteMany` with the composed AST. */ + | { readonly kind: 'multi' } + /** Neither — the engine throws `ENGINE_DELETE_REJECT_MESSAGE`. */ + | { readonly kind: 'reject'; readonly message: string }; + +/** The subset of `EngineDeleteOptions` the dispatch decision actually reads. */ +export interface EngineDeleteDispatchInput { + readonly where?: unknown; + readonly multi?: unknown; + readonly [k: string]: unknown; +} + +/** + * Extract the SCALAR `where.id`, or `undefined` when the call does not name one + * row by primary key. + * + * `null`, `undefined`, arrays, and operator objects (`{ $in: [...] }`, + * `{ $ne: … }`) all yield `undefined` — they are predicates over many rows, not + * a primary key. + */ +export function scalarDeleteId( + options?: EngineDeleteDispatchInput | null, +): string | number | bigint | undefined { + const where = options?.where; + if (!where || typeof where !== 'object') return undefined; + if (!('id' in (where as Record))) return undefined; + const whereId = (where as Record).id; + const t = typeof whereId; + if (whereId !== null && (t === 'string' || t === 'number' || t === 'bigint')) { + return whereId as string | number | bigint; + } + return undefined; +} + +/** + * Decide what `ObjectQLEngine.delete` does with `options`, without doing it. + * + * Pure and side-effect free, so a test double can call it to *classify* a call + * and then implement `by-id` / `multi` however its fixture stores rows — while + * being bound to the real engine's `reject` surface for free. + */ +export function resolveEngineDeleteDispatch( + options?: EngineDeleteDispatchInput | null, +): EngineDeleteDispatch { + const id = scalarDeleteId(options); + if (id !== undefined) return { kind: 'by-id', id }; + if (options?.multi) return { kind: 'multi' }; + return { kind: 'reject', message: ENGINE_DELETE_REJECT_MESSAGE }; +} + +/** + * Throw exactly what `ObjectQLEngine.delete` throws when a call is neither + * `by-id` nor `multi`; return the resolved dispatch otherwise. + * + * This is the line a fake engine's `delete` opens with. One call pins the fake + * to the producer's rejection surface, and — unlike a mirrored `if` — it cannot + * drift when the producer's rule changes. + * + * ```ts + * async delete(object: string, options?: any) { + * assertEngineDeleteDispatch(options); // refuses what a real server refuses + * … + * } + * ``` + */ +export function assertEngineDeleteDispatch( + options?: EngineDeleteDispatchInput | null, +): Exclude { + const dispatch = resolveEngineDeleteDispatch(options); + if (dispatch.kind === 'reject') throw new Error(dispatch.message); + return dispatch; +} + +/** + * The shared conformance case-set for the delete dispatch — the same role + * `packages/spec/src/data/*-conformance.ts` plays for drivers. + * + * Every case names a call shape and the verdict the **real engine** gives it. + * A double proved against these is proved against the producer, including the + * three shapes that look like an id and are not. + */ +export interface EngineDeleteDispatchCase { + /** What the shape is, in the words a failure message should use. */ + readonly what: string; + /** The options bag handed to `delete(object, options)`. */ + readonly options: EngineDeleteDispatchInput | undefined; + /** The verdict the engine gives it. */ + readonly expect: EngineDeleteDispatch['kind']; +} + +export const ENGINE_DELETE_DISPATCH_CASES: readonly EngineDeleteDispatchCase[] = [ + { what: 'scalar string id', options: { where: { id: 'rec_1' } }, expect: 'by-id' }, + { what: 'scalar number id', options: { where: { id: 42 } }, expect: 'by-id' }, + { what: 'scalar id alongside other predicates', options: { where: { id: 'rec_1', tenant: 't1' } }, expect: 'by-id' }, + { what: 'multi with a predicate', options: { where: { rule_id: 'r1' }, multi: true }, expect: 'multi' }, + { what: 'multi with no predicate at all', options: { multi: true }, expect: 'multi' }, + { what: 'multi alongside an $in id set', options: { where: { id: { $in: ['a', 'b'] } }, multi: true }, expect: 'multi' }, + // ── The rejects. Everything below is what #4434 shipped against a fake that + // accepted it, and what a running server answers 500 to. + { what: 'predicate on a non-id column, no multi', options: { where: { rule_id: 'r1' } }, expect: 'reject' }, + { what: '$in over ids, no multi (an operator object is NOT an id)', options: { where: { id: { $in: ['a', 'b'] } } }, expect: 'reject' }, + { what: 'array id, no multi', options: { where: { id: ['a', 'b'] } }, expect: 'reject' }, + { what: 'null id, no multi', options: { where: { id: null } }, expect: 'reject' }, + { what: 'empty where, no multi', options: { where: {} }, expect: 'reject' }, + { what: 'no options at all', options: undefined, expect: 'reject' }, + { what: 'multi explicitly false with a predicate', options: { where: { rule_id: 'r1' }, multi: false }, expect: 'reject' }, +]; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 8cc23d4073..f531cb3705 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -86,6 +86,11 @@ import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, Validat import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js'; import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; +import { + resolveEngineDeleteDispatch, + ENGINE_DELETE_REJECT_MESSAGE, + type EngineDeleteDispatchInput, +} from './engine-delete-dispatch.js'; import { applyHaving } from './having-filter.js'; import { auditDanglingReferences, @@ -5059,14 +5064,14 @@ export class ObjectQL implements IObjectQLEngine { // literally (driver.delete(object, {$in:[…]})) and both skip the #2982 AST // seeding below AND bypass the by-id RLS pre-image check. Leave `id` // undefined so the call routes to deleteMany with the scoped AST. - let id: any = undefined; - if (options?.where && typeof options.where === 'object' && 'id' in options.where) { - const whereId = (options.where as Record).id; - const t = typeof whereId; - if (whereId !== null && (t === 'string' || t === 'number' || t === 'bigint')) { - id = whereId; - } - } + // + // [#4550] The decision lives in `engine-delete-dispatch.ts` so the fake + // engines that stand in for this method import it instead of re-deriving + // it. #4434 shipped a dead REST route green because plugin-sharing's fake + // accepted the one shape the `reject` branch below refuses; a double that + // reads THIS predicate cannot be looser than this method. + const dispatch = resolveEngineDeleteDispatch(options as EngineDeleteDispatchInput | undefined); + const id: any = dispatch.kind === 'by-id' ? dispatch.id : undefined; const opCtx: OperationContext = { object, @@ -5134,7 +5139,10 @@ export class ObjectQL implements IObjectQLEngine { result = await driver.deleteMany(object, ast, hookContext.input.options as any); isPredicateWrite = true; } else { - throw new Error('Delete requires an ID or options.multi=true'); + // The `reject` verdict of resolveEngineDeleteDispatch, re-asked + // here because a beforeDelete hook may have cleared the id since + // (#4550 keeps the wording in one place either way). + throw new Error(ENGINE_DELETE_REJECT_MESSAGE); } hookContext.event = 'afterDelete'; diff --git a/packages/objectql/src/index.ts b/packages/objectql/src/index.ts index 59626d8e79..3cc6148caa 100644 --- a/packages/objectql/src/index.ts +++ b/packages/objectql/src/index.ts @@ -63,6 +63,25 @@ export type { } from './driver-connect-errors.js'; export type { InsertManyRowOutcome } from './engine.js'; +// [#4550] The delete-dispatch contract, exported so a TEST DOUBLE that stands +// in for the engine can import the producer's own decision rather than +// re-deriving it. A fake looser than the contract it replaces is how #4434 +// shipped a REST route that 500'd for every caller with its suite green. +// `scripts/check-engine-double-contract.mjs` is the gate that keeps new +// engine doubles on this. +export { + resolveEngineDeleteDispatch, + assertEngineDeleteDispatch, + scalarDeleteId, + ENGINE_DELETE_REJECT_MESSAGE, + ENGINE_DELETE_DISPATCH_CASES, +} from './engine-delete-dispatch.js'; +export type { + EngineDeleteDispatch, + EngineDeleteDispatchInput, + EngineDeleteDispatchCase, +} from './engine-delete-dispatch.js'; + // Export in-memory aggregation fallback (used by engine.aggregate when the // driver lacks native groupBy/aggregations support; also useful for tests). export { applyInMemoryAggregation, bucketDateValue } from './in-memory-aggregation.js'; diff --git a/packages/objectql/src/layered-overlay-integration.test.ts b/packages/objectql/src/layered-overlay-integration.test.ts index b6669c929c..e82fb92eec 100644 --- a/packages/objectql/src/layered-overlay-integration.test.ts +++ b/packages/objectql/src/layered-overlay-integration.test.ts @@ -19,6 +19,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { LayeredRepository, InMemoryRepository, hashSpec } from '@objectstack/metadata-core'; import type { MetaRef } from '@objectstack/metadata-core'; import { SysMetadataRepository } from '@objectstack/metadata-protocol'; +import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js'; interface Row { id: string; @@ -90,6 +91,10 @@ function makeFakeEngine() { return { id: found.row.id }; }, async delete(_t: string, opts: { where: Record }) { + // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double + // looser than the engine it stands in for is how #4434 shipped a REST + // route that answered 500 to every caller with its suite green. + assertEngineDeleteDispatch(opts); const found = findRow(opts.where); if (!found) return { deleted: 0 }; rows.delete(found.key); diff --git a/packages/objectql/src/protocol-publish-rollback.test.ts b/packages/objectql/src/protocol-publish-rollback.test.ts index 770bc654d0..91e0d77898 100644 --- a/packages/objectql/src/protocol-publish-rollback.test.ts +++ b/packages/objectql/src/protocol-publish-rollback.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js'; /** * Protocol-level coverage for the per-item draft / publish / rollback / @@ -133,6 +134,10 @@ function makeStubEngine() { return { id: found.row.id }; }, async delete(_t: string, opts: { where: Record }) { + // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double + // looser than the engine it stands in for is how #4434 shipped a REST + // route that answered 500 to every caller with its suite green. + assertEngineDeleteDispatch(opts); const found = findRow(opts.where); if (!found) return { deleted: 0 }; rows.delete(found.key); diff --git a/packages/objectql/src/protocol-save-meta-repo-path.test.ts b/packages/objectql/src/protocol-save-meta-repo-path.test.ts index 418d6e4fb6..fd67fe0a23 100644 --- a/packages/objectql/src/protocol-save-meta-repo-path.test.ts +++ b/packages/objectql/src/protocol-save-meta-repo-path.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from 'vitest'; import { hashSpec } from '@objectstack/metadata-core'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js'; /** * Repository write-path coverage (post PR-10d.6). @@ -70,6 +71,10 @@ function makeStubEngine() { return { id: found.row.id }; }, async delete(_t: string, opts: { where: Record }) { + // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double + // looser than the engine it stands in for is how #4434 shipped a REST + // route that answered 500 to every caller with its suite green. + assertEngineDeleteDispatch(opts); const found = findRow(opts.where); if (!found) return { deleted: 0 }; rows.delete(found.key); diff --git a/packages/objectql/src/protocol-view-identity-overlay.test.ts b/packages/objectql/src/protocol-view-identity-overlay.test.ts index 6d523d32c8..475acb958f 100644 --- a/packages/objectql/src/protocol-view-identity-overlay.test.ts +++ b/packages/objectql/src/protocol-view-identity-overlay.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js'; /** * #2555 — a console personalization PUT (grid column sort, inline edit, …) @@ -87,6 +88,10 @@ function makeStubEngine(registryViews: Record = {}) { return { id: found.row.id }; }, async delete(_t: string, opts: { where: Record }) { + // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double + // looser than the engine it stands in for is how #4434 shipped a REST + // route that answered 500 to every caller with its suite green. + assertEngineDeleteDispatch(opts); const found = findRow(opts.where); if (!found) return { deleted: 0 }; rows.delete(found.key); diff --git a/packages/objectql/src/sys-metadata-repository.test.ts b/packages/objectql/src/sys-metadata-repository.test.ts index 7b322308f9..87f11f39d8 100644 --- a/packages/objectql/src/sys-metadata-repository.test.ts +++ b/packages/objectql/src/sys-metadata-repository.test.ts @@ -11,6 +11,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { ConflictError, hashSpec } from '@objectstack/metadata-core'; import { SysMetadataRepository } from '@objectstack/metadata-protocol'; +import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js'; interface Row { id: string; @@ -119,6 +120,10 @@ function makeFakeEngine() { return { id: found.row.id }; }, async delete(_t: string, opts: { where: Record }) { + // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double + // looser than the engine it stands in for is how #4434 shipped a REST + // route that answered 500 to every caller with its suite green. + assertEngineDeleteDispatch(opts); const found = findRow(opts.where); if (!found) return { deleted: 0 }; rows.delete(found.key); diff --git a/packages/plugins/plugin-sharing/src/boot-backfill.test.ts b/packages/plugins/plugin-sharing/src/boot-backfill.test.ts index 7394cb6e35..625e29bd5d 100644 --- a/packages/plugins/plugin-sharing/src/boot-backfill.test.ts +++ b/packages/plugins/plugin-sharing/src/boot-backfill.test.ts @@ -12,6 +12,7 @@ */ import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { SharingService } from './sharing-service.js'; import { SharingRuleService } from './sharing-rule-service.js'; import { backfillRuleGrants, backfillRetiredAccessLevels } from './sharing-plugin.js'; @@ -58,13 +59,11 @@ function makeEngine() { return t[i]; }, async delete(o: string, opts?: any) { - // Mirror `ObjectQLEngine.delete`'s dispatch guard (#4434) — see the same - // note in sharing-rule.test.ts. A fake looser than the contract it - // stands in for is how a green suite ships a dead route. - const whereId = opts?.where && typeof opts.where === 'object' ? (opts.where as any).id : undefined; - const t0 = typeof whereId; - const scalarId = whereId != null && (t0 === 'string' || t0 === 'number' || t0 === 'bigint'); - if (!scalarId && !opts?.multi) throw new Error('Delete requires an ID or options.multi=true'); + // Pinned to `ObjectQLEngine.delete`'s own dispatch predicate (#4434, + // #4550) — see the longer note in sharing-rule.test.ts. This used to + // MIRROR the guard by hand; a mirror is a second copy of the contract, + // so it now calls the producer's exported decision instead. + assertEngineDeleteDispatch(opts); const t = ensure(o); const where = opts?.where ?? {}; for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); return { ok: true }; diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts index a9cb185583..c3fd86e26a 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-provenance.test.ts @@ -17,6 +17,7 @@ */ import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { SharingService } from './sharing-service.js'; import { SharingRuleService } from './sharing-rule-service.js'; import { bindRuleProvenanceStamp, SHARING_RULE_PROVENANCE_PACKAGE } from './sharing-rule-provenance.js'; @@ -53,6 +54,9 @@ function makeEngine() { return t[i]; }, async delete(o: string, opts?: any) { + // [#4550] Pinned to ObjectQL.delete's own dispatch predicate — see the + // note in sharing-rule.test.ts for what a looser fake cost (#4434). + assertEngineDeleteDispatch(opts); const t = ensure(o); const where = opts?.where ?? {}; for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); return { ok: true }; diff --git a/packages/plugins/plugin-sharing/src/sharing-rule.test.ts b/packages/plugins/plugin-sharing/src/sharing-rule.test.ts index 292aef923b..f438ff8590 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { SharingService } from './sharing-service.js'; import { SharingRuleService } from './sharing-rule-service.js'; import { TeamGraphService, expandPrincipal } from './team-graph.js'; @@ -59,7 +60,23 @@ function makeEngine() { return t[i]; }, async delete(o: string, opts?: any) { - assertDeletable(opts); + // Pinned to `ObjectQLEngine.delete`'s OWN dispatch predicate + // (objectstack#4434, objectstack#4550). + // + // The real engine routes a delete by SCALAR `where.id` to `driver.delete` + // and anything else to `driver.deleteMany` — but only when + // `options.multi` is set; otherwise it throws. This fake used to accept + // any `where`, so `deleteRule`'s predicate-shaped purge of + // `sys_record_share` passed here while the running server answered 500 to + // every `DELETE /sharing/rules/:idOrName`. A fake looser than the + // contract it stands in for is how a green suite ships a dead route. + // + // #4434 closed that by MIRRORING the guard here. A mirror is a second + // copy of the contract and drifts the moment either side is edited, so it + // now calls the producer's exported decision instead — the same function + // `ObjectQL.delete` dispatches on. `pnpm check:engine-double-contract` + // holds every fake engine to this. + assertEngineDeleteDispatch(opts); const t = ensure(o); const where = opts?.where ?? {}; for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); return { ok: true }; @@ -67,24 +84,6 @@ function makeEngine() { }; } -/** - * Mirror `ObjectQLEngine.delete`'s dispatch guard (objectstack#4434). - * - * The real engine routes a delete by SCALAR `where.id` to `driver.delete` and - * anything else to `driver.deleteMany` — but only when `options.multi` is set; - * otherwise it throws `'Delete requires an ID or options.multi=true'`. The fake - * used to accept any `where`, so `deleteRule`'s predicate-shaped purge of - * `sys_record_share` passed here while the running server answered 500 to every - * `DELETE /sharing/rules/:idOrName`. A fake looser than the contract it stands - * in for is how a green suite ships a dead route. - */ -function assertDeletable(opts?: any): void { - const whereId = opts?.where && typeof opts.where === 'object' ? (opts.where as any).id : undefined; - const t = typeof whereId; - const scalarId = whereId != null && (t === 'string' || t === 'number' || t === 'bigint'); - if (!scalarId && !opts?.multi) throw new Error('Delete requires an ID or options.multi=true'); -} - describe('TeamGraphService (flat — better-auth sys_team)', () => { let engine: ReturnType; beforeEach(() => { diff --git a/packages/plugins/plugin-sharing/src/sharing-service.test.ts b/packages/plugins/plugin-sharing/src/sharing-service.test.ts index c83a48bee6..80a649597b 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.test.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach } from 'vitest'; +import { assertEngineDeleteDispatch } from '@objectstack/objectql'; import { SharingService } from './sharing-service.js'; import { buildSharingMiddleware } from './sharing-plugin.js'; @@ -74,6 +75,9 @@ function makeFakeEngine(schemas: Record) { return table[i]; }, async delete(object: string, options?: any) { + // [#4550] Pinned to ObjectQL.delete's own dispatch predicate — see the + // note in sharing-rule.test.ts for what a looser fake cost (#4434). + assertEngineDeleteDispatch(options); const table = ensure(object); const id = options?.where?.id ?? options?.id; const i = table.findIndex(r => r.id === id); diff --git a/scripts/check-engine-double-contract.mjs b/scripts/check-engine-double-contract.mjs new file mode 100644 index 0000000000..ac8ab6dc5b --- /dev/null +++ b/scripts/check-engine-double-contract.mjs @@ -0,0 +1,557 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// check-engine-double-contract -- a fake ObjectQL engine's `delete` must be +// pinned to the real engine's dispatch contract, not a looser hand-written +// approximation of it (objectstack#4550, from objectstack#4434). +// +// node scripts/check-engine-double-contract.mjs +// node scripts/check-engine-double-contract.mjs --self-test +// +// ## The failure mode this exists for +// +// `DELETE /api/v1/sharing/rules/:idOrName` answered **500 for every rule and +// both address forms** from the day it was written. It was not untested: +// plugin-sharing's `deleteRule drops rule + all its grants` asserted success on +// it the whole time -- against a FAKE ENGINE whose `delete` accepted a call the +// real engine refuses. `deleteRule` purged `sys_record_share` with a +// predicate-shaped delete carrying neither a scalar `where.id` nor +// `options.multi`, which is precisely the one shape `ObjectQL.delete` throws +// on. The fake deleted by predicate happily, so the suite was green, the gate +// was green, and the route was dead. That is #4434. +// +// The general shape, and the reason this is a gate rather than a fixed test: +// **a test double looser than the implementation it replaces converts a green +// suite into no suite at all**, silently, on exactly the paths a double was +// introduced for -- which are the paths that were hard to test, which are +// usually the paths where the contract is densest. +// +// ## Slice: `delete` dispatch only, and only on ENGINE doubles +// +// #4550 lists four instances of the family. This gate takes exactly one of +// them, the one whose criterion is mechanically decidable with no judgment +// call: the engine's delete dispatch is a total function from an options bag to +// one of three verdicts, so "is this double looser?" has a yes/no answer that +// does not depend on reading the test's intent. +// +// Deliberately NOT covered, and why (each wants its own gate, not a vaguer +// version of this one -- a gate whose scope is fuzzy is indistinguishable from +// no gate to everyone downstream of it): +// +// - fixtures that disable a platform constraint in prose (`// FK enforcement +// is off in this harness`, #4441). The criterion is a comment, so it is +// both evadable by deleting the comment and unable to find the silent +// cases. That one wants a declared debt ledger, not a scanner. +// - stubbing the very thing under assertion (objectui#3129) and missing +// counterparts (objectui#3134). Both live in the `objectui` repo, which +// this script cannot see, and #3134 names no double at all. +// - every other method a fake engine offers (`find` filter semantics, +// `update`'s twin dispatch, unknown-option rejection). Same family, but +// each needs its own producer-side predicate extracted first. `delete` had +// one available because #4434 already paid for it. +// +// ## Invariants +// +// DISCOVERED the scan found engine doubles at all. Zero is not "a clean +// repo", it is a broken scan: PINNED iterates the discovered set, +// so a discovery that silently stops matching makes this script +// print OK while checking nothing -- the #4868 family, where a +// check runs, is green, and structurally cannot reach its subject. +// PINNED every discovered engine double's `delete` routes through +// `assertEngineDeleteDispatch` / `resolveEngineDeleteDispatch` +// from `@objectstack/objectql` -- the predicate the real +// `ObjectQL.delete` itself uses -- or its file carries a measured +// baseline entry. +// RECONCILED in both directions. A baseline entry for a file with no +// unguarded doubles left, for a file that no longer exists, or +// whose count is now lower, is an error. A ratchet that can only +// accrete rots into a list nobody trusts. +// +// ## Why "routes through the shared predicate" and not "mirrors the guard" +// +// The #4434 fix mirrored the engine's guard into that one fake by hand. That +// closes one fake and opens a second copy of the contract -- and the scalar +// test is the half a hand-written copy drops: `where: { id: { $in: [...] } }` +// LOOKS like an id and is a multi-row predicate, so the real engine rejects it +// without `multi` and a mirrored `if (!opts?.where?.id && !opts?.multi)` accepts +// it. Requiring the producer's own function removes the class: a double that +// imports the decision cannot be looser than the decision. Same reasoning as +// objectstack#4455 -- the scan and the validator must answer with ONE predicate. +// +// ## What this deliberately does NOT claim +// +// It checks that the shared predicate is CALLED, not that the double's by-id +// and multi branches then behave like the driver would. A gate cannot judge +// that, and one that pretended to would be the verifier that reports success +// while degrading. What it can do is make the rejection surface impossible to +// drift, which is the half that shipped #4434. + +import { readFileSync, readdirSync, existsSync } from 'node:fs'; +import { join, dirname, relative, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); +const BASELINE_PATH = join(ROOT, 'scripts', 'engine-double-contract.baseline.json'); +const SCAN_ROOTS = ['packages', 'examples']; + +/** The producer-side predicate a double must route through. */ +const PINNED_SYMBOLS = new Set(['assertEngineDeleteDispatch', 'resolveEngineDeleteDispatch']); +/** Where it may legitimately come from: the public export, or objectql's own relative path. */ +const PINNED_MODULES = [/^@objectstack\/objectql$/, /engine-delete-dispatch(\.js)?$/]; + +/** + * Members that mark an object literal as standing in for the ENGINE. + * + * The engine and the driver both have `delete`, so the sibling set is what + * separates them alongside the parameter test below: drivers speak + * `create`/`bulkCreate`/`checkHealth`, the engine speaks `insert`/`findOne`. + */ +const ENGINE_SIBLINGS = new Set([ + 'find', 'findOne', 'insert', 'update', 'count', 'aggregate', 'getSchema', 'registry', 'insertMany', +]); + +/** Parameter names that mean "this is the DRIVER's delete(object, id, options)". */ +const ID_PARAM = /^_*(id|recordId|ids|pk)$/i; + +// ── Discovery ─────────────────────────────────────────────────────────────── + +function walk(dir, out = []) { + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return out; + } + for (const e of entries) { + if (e.name === 'node_modules' || e.name === 'dist' || e.name === '.git' || e.name === '.cache') continue; + const p = join(dir, e.name); + if (e.isDirectory()) walk(p, out); + else if (/\.(test|spec)\.(ts|tsx|mts)$/.test(e.name)) out.push(p); + } + return out; +} + +function testFiles() { + const out = []; + for (const r of SCAN_ROOTS) walk(join(ROOT, r), out); + return out.sort(); +} + +/** A member's function-ish implementation, or null. */ +function implOf(member) { + if (ts.isMethodDeclaration(member) || ts.isMethodSignature(member)) return member; + if (ts.isPropertyAssignment(member)) { + const init = member.initializer; + if (init && (ts.isFunctionExpression(init) || ts.isArrowFunction(init))) return init; + return null; + } + if (ts.isPropertyDeclaration(member) && member.initializer) { + const init = member.initializer; + if (ts.isFunctionExpression(init) || ts.isArrowFunction(init)) return init; + return null; + } + if (ts.isShorthandPropertyAssignment(member)) return null; + return null; +} + +function memberName(member) { + const n = member.name; + if (!n) return null; + if (ts.isIdentifier(n) || ts.isStringLiteral(n)) return n.text; + return null; +} + +/** + * Is this `delete(a, b, …)` the ENGINE's shape (`object, options`) rather than + * the DRIVER's (`object, id, options`)? + * + * The second parameter is the whole question: the engine takes an options bag + * there, the driver takes a primary key. Judged on the name first (the repo + * writes `id` when it means one) and on a scalar type annotation second. + */ +function isEngineDeleteShape(fn) { + const params = fn.parameters ?? []; + if (params.length < 2) return false; + const second = params[1]; + const name = ts.isIdentifier(second.name) ? second.name.text : ''; + if (ID_PARAM.test(name)) return false; + const t = second.type ? second.type.getText() : ''; + if (/^(string|number|bigint|string \| number)$/.test(t.trim())) return false; + return true; +} + +/** Collect every identifier that is CALLED anywhere inside `node`. */ +function calleesIn(node) { + const names = new Set(); + const visit = (n) => { + if (ts.isCallExpression(n)) { + const e = n.expression; + if (ts.isIdentifier(e)) names.add(e.text); + else if (ts.isPropertyAccessExpression(e) && ts.isIdentifier(e.name)) names.add(e.name.text); + } + ts.forEachChild(n, visit); + }; + visit(node); + return names; +} + +/** + * LOCAL names in this file that are bound to one of the pinned predicates by an + * import from the producer. + * + * Keyed on the local binding (so `import { assertEngineDeleteDispatch as guard }` + * still counts) but only when the IMPORTED name is one of the pinned symbols — + * a same-named local look-alike must not qualify, since the whole property is + * that one predicate answers. + */ +function pinnedImportsOf(sourceFile) { + const found = new Set(); + for (const st of sourceFile.statements) { + if (!ts.isImportDeclaration(st) || !ts.isStringLiteral(st.moduleSpecifier)) continue; + const spec = st.moduleSpecifier.text; + if (!PINNED_MODULES.some((re) => re.test(spec))) continue; + const named = st.importClause?.namedBindings; + if (named && ts.isNamedImports(named)) { + for (const el of named.elements) { + const imported = (el.propertyName ?? el.name).text; + if (PINNED_SYMBOLS.has(imported)) found.add(el.name.text); + } + } + } + return found; +} + +/** Top-level function declarations / const-arrow functions, by name. */ +function localFunctions(sourceFile) { + const map = new Map(); + const visit = (n) => { + if (ts.isFunctionDeclaration(n) && n.name) map.set(n.name.text, n); + if (ts.isVariableStatement(n)) { + for (const d of n.declarationList.declarations) { + if (ts.isIdentifier(d.name) && d.initializer + && (ts.isArrowFunction(d.initializer) || ts.isFunctionExpression(d.initializer))) { + map.set(d.name.text, d.initializer); + } + } + } + ts.forEachChild(n, visit); + }; + visit(sourceFile); + return map; +} + +/** + * Every engine double in one file, with a verdict on whether its `delete` is + * pinned to the shared predicate. + * + * Pinning is accepted one level of indirection deep: a fake that opens with a + * local `assertDeletable(opts)` helper which itself calls the shared predicate + * is pinned. Two hops is not — at that point the gate would be guessing. + */ +function scanSource(fileName, text) { + const sf = ts.createSourceFile(fileName, text, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX); + const pinnedNames = pinnedImportsOf(sf); + const locals = localFunctions(sf); + const doubles = []; + + const bodyIsPinned = (fn) => { + if (pinnedNames.size === 0) return false; + const direct = calleesIn(fn); + for (const n of direct) if (pinnedNames.has(n)) return true; + for (const n of direct) { + const local = locals.get(n); + if (!local) continue; + for (const m of calleesIn(local)) if (pinnedNames.has(m)) return true; + } + return false; + }; + + const consider = (members, node) => { + const names = new Set(); + let del = null; + for (const m of members) { + const n = memberName(m); + if (!n) continue; + names.add(n); + if (n === 'delete') del = implOf(m); + } + if (!del) return; + const siblings = [...names].filter((n) => ENGINE_SIBLINGS.has(n)); + if (siblings.length < 2) return; + if (!isEngineDeleteShape(del)) return; + const line = sf.getLineAndCharacterOfPosition(node.getStart(sf)).line + 1; + doubles.push({ line, siblings: siblings.sort(), pinned: bodyIsPinned(del) }); + }; + + const visit = (n) => { + if (ts.isObjectLiteralExpression(n)) consider(n.properties, n); + else if (ts.isClassDeclaration(n) || ts.isClassExpression(n)) consider(n.members, n); + ts.forEachChild(n, visit); + }; + visit(sf); + return doubles; +} + +// ── Baseline ──────────────────────────────────────────────────────────────── + +function readBaseline() { + if (!existsSync(BASELINE_PATH)) return { entries: [] }; + return JSON.parse(readFileSync(BASELINE_PATH, 'utf8')); +} + +// ── Audit ─────────────────────────────────────────────────────────────────── + +function audit() { + const baseline = readBaseline(); + const byFile = new Map(baseline.entries.map((e) => [e.file, e])); + const errors = []; + const found = []; + + for (const abs of testFiles()) { + const rel = relative(ROOT, abs).split(sep).join('/'); + const text = readFileSync(abs, 'utf8'); + // Cheap pre-filter: no `delete` member, nothing to parse. + if (!/\bdelete\s*[(:]/.test(text)) continue; + const doubles = scanSource(abs, text); + if (doubles.length === 0) continue; + found.push({ file: rel, doubles }); + } + + // DISCOVERED + if (found.length === 0) { + errors.push( + 'DISCOVERED: the scan found no engine doubles anywhere. That is not a clean repo, it is a ' + + 'broken scan — PINNED iterates this set, so every other invariant passes vacuously and ' + + 'this script reports OK while reading nothing. Fix the discovery before trusting a green run.', + ); + } + + const seen = new Set(); + for (const { file, doubles } of found) { + const unguarded = doubles.filter((d) => !d.pinned); + const entry = byFile.get(file); + if (entry) seen.add(file); + + if (unguarded.length === 0) { + if (entry) { + errors.push( + `RECONCILED: ${file} has no unguarded engine double left, but the baseline still ` + + `records ${entry.unguarded}. Delete the entry in the same PR that fixed it.`, + ); + } + continue; + } + if (!entry) { + errors.push( + `PINNED: ${file} declares ${unguarded.length} engine double(s) whose delete() does not route ` + + `through assertEngineDeleteDispatch (line${unguarded.length > 1 ? 's' : ''} ` + + `${unguarded.map((d) => d.line).join(', ')}). A fake looser than ObjectQL.delete is how ` + + '#4434 shipped a dead REST route with its suite green. Open the fake\'s delete with ' + + "`assertEngineDeleteDispatch(options)` from '@objectstack/objectql' (add it as a " + + 'devDependency if the package lacks it), or add a MEASURED entry to ' + + 'scripts/engine-double-contract.baseline.json saying why not.', + ); + continue; + } + if (unguarded.length > entry.unguarded) { + errors.push( + `PINNED: ${file} now has ${unguarded.length} unguarded engine double(s), baseline records ` + + `${entry.unguarded}. The baseline is shrink-only — pin the new one rather than raising it.`, + ); + } else if (unguarded.length < entry.unguarded) { + errors.push( + `RECONCILED: ${file} is down to ${unguarded.length} unguarded engine double(s) from the ` + + `baseline's ${entry.unguarded}. Lower the number in the same PR, so the ratchet holds.`, + ); + } + } + + for (const entry of baseline.entries) { + if (seen.has(entry.file)) continue; + errors.push( + `RECONCILED: baseline entry for ${entry.file}, which declares no engine double any more ` + + '(file deleted, fake removed, or the shape changed). Delete the entry.', + ); + } + + return { found, baseline, errors }; +} + +function report() { + const { found, baseline, errors } = audit(); + const doubles = found.reduce((n, f) => n + f.doubles.length, 0); + const pinned = found.reduce((n, f) => n + f.doubles.filter((d) => d.pinned).length, 0); + + console.log( + `\nengine doubles: ${doubles} in ${found.length} test file(s) — ${pinned} pinned to ` + + `ObjectQL.delete's dispatch predicate, ${doubles - pinned} in the shrink-only baseline.\n`, + ); + + if (errors.length) { + for (const e of errors) console.error(` x ${e}`); + console.error(`\ncheck-engine-double-contract: ${errors.length} problem(s).\n`); + process.exit(1); + } + + for (const f of found.filter((f) => f.doubles.some((d) => d.pinned))) { + console.log(` pinned ${f.file}`); + } + + // Print the EXEMPT reasons, not only the count. An entry whose justification + // is never surfaced is how a ledger decays into a list nobody reads — and + // these rows are part of why this run is green. + const exempt = baseline.entries.filter((e) => e.kind === 'EXEMPT'); + if (exempt.length) console.log(''); + for (const e of exempt) { + console.log(` EXEMPT ${e.file}`); + console.log(` ${e.why}`); + } + console.log(''); + const debt = baseline.entries.filter((e) => e.kind !== 'EXEMPT').length; + console.log( + `check-engine-double-contract: OK — ${pinned} pinned, ${debt} in the DEBT ledger, ` + + `${exempt.length} exempt.\n`, + ); +} + +// ── Self-test ─────────────────────────────────────────────────────────────── +// +// A guard that cannot fail is not a guard (#4118). This drives the detector +// against synthetic sources on BOTH sides of every decision it makes, so a +// refactor that neuters it fails here instead of turning every future PR green. + +function selfTest() { + const failures = []; + const expect = (label, cond) => { if (!cond) failures.push(label); }; + + const IMPORT = "import { assertEngineDeleteDispatch } from '@objectstack/objectql';\n"; + const engineFake = (deleteBody, header = '') => `${header} +function makeEngine() { + return { + async find(o: string, opts?: any) { return []; }, + async insert(o: string, data: any) { return data; }, + async update(o: string, d: any) { return d; }, + async delete(o: string, opts?: any) { ${deleteBody} }, + }; +} +`; + + // ── Detection: an unpinned engine fake is found, a pinned one is not flagged. + let d = scanSource('a.test.ts', engineFake('return { ok: true };')); + expect('finds an unpinned engine double', d.length === 1 && d[0].pinned === false); + + d = scanSource('a.test.ts', engineFake('assertEngineDeleteDispatch(opts); return { ok: true };', IMPORT)); + expect('a directly pinned double is not flagged', d.length === 1 && d[0].pinned === true); + + // One level of indirection through a local helper counts; the helper must + // itself reach the shared predicate. + d = scanSource('a.test.ts', engineFake('assertDeletable(opts); return 1;', + IMPORT + 'function assertDeletable(o: any) { assertEngineDeleteDispatch(o); }\n')); + expect('a local helper that calls the predicate counts as pinned', d.length === 1 && d[0].pinned === true); + + d = scanSource('a.test.ts', engineFake('assertDeletable(opts); return 1;', + IMPORT + 'function assertDeletable(o: any) { if (!o?.where?.id && !o?.multi) throw new Error("x"); }\n')); + expect('a HAND-MIRRORED local helper does not count as pinned', + d.length === 1 && d[0].pinned === false); + + // Importing the symbol without calling it is not pinning — the #4434 fake + // would have passed a check that only looked at imports. + d = scanSource('a.test.ts', engineFake('return { ok: true };', IMPORT)); + expect('an unused import is not pinning', d.length === 1 && d[0].pinned === false); + + // ── Scope: the DRIVER's delete(object, id, options) is a different contract + // and must not be swept in, or the gate drowns in false positives. + const driverFake = ` +const driver = { + async find(o: string) { return []; }, + async create(o: string, d: any) { return d; }, + async update(o: string, id: string, d: any) { return d; }, + async delete(object: string, id: string) { return true; }, +}; +`; + expect('a driver double (delete by scalar id) is out of scope', scanSource('d.test.ts', driverFake).length === 0); + + const typedIdDriver = ` +const driver = { + async find(o: string) { return []; }, + async insert(o: string, d: any) { return d; }, + async delete(object: string, key: string) { return true; }, +}; +`; + expect('a scalar-typed second parameter is out of scope', + scanSource('d.test.ts', typedIdDriver).length === 0); + + // A lone `delete` with no engine siblings is a Map-ish or route helper. + const bare = 'const cache = { delete(k: string, o: any) { return true; } };\n'; + expect('an object with no engine siblings is out of scope', scanSource('c.test.ts', bare).length === 0); + + // ── Shape coverage: the fake shapes this repo actually writes. + const classFake = `${IMPORT} +class FakeEngine { + async find(o: string, q?: any) { return []; } + async insert(o: string, d: any) { return d; } + async delete(o: string, opts?: any) { assertEngineDeleteDispatch(opts); return 1; } +} +`; + d = scanSource('k.test.ts', classFake); + expect('a class-shaped fake engine is in scope and pinnable', d.length === 1 && d[0].pinned === true); + + const arrowFake = ` +const engine = { + find: async (o: string) => [], + insert: async (o: string, d: any) => d, + update: async (o: string, d: any) => d, + delete: async (o: string, opts: any) => ({ ok: true }), +}; +`; + d = scanSource('p.test.ts', arrowFake); + expect('an arrow-property fake engine is in scope', d.length === 1 && d[0].pinned === false); + + // The import must come from the producer. A same-named local function is not + // the contract — the whole point is that ONE predicate answers. + d = scanSource('q.test.ts', engineFake('assertEngineDeleteDispatch(opts); return 1;', + 'function assertEngineDeleteDispatch(o: any) { /* look-alike */ }\n')); + expect('a locally re-declared look-alike is not pinning', d.length === 1 && d[0].pinned === false); + + d = scanSource('r.test.ts', engineFake('assertEngineDeleteDispatch(opts); return 1;', + "import { assertEngineDeleteDispatch } from './my-helpers.js';\n")); + expect('the predicate imported from an unrelated module is not pinning', + d.length === 1 && d[0].pinned === false); + + // objectql's own tests import it by relative path; that IS the producer. + d = scanSource('s.test.ts', engineFake('assertEngineDeleteDispatch(opts); return 1;', + "import { assertEngineDeleteDispatch } from './engine-delete-dispatch.js';\n")); + expect("objectql's relative import of the producer counts", d.length === 1 && d[0].pinned === true); + + expect('a file with no fakes yields no doubles', + scanSource('empty.test.ts', 'export const x = 1;\n').length === 0); + + // Discovery must reach the real tree, and specifically must reach the fake + // #4434 was shipped past. Everything above is synthetic; this is the wiring. + // + // Deliberately NOT asserted here: that the real tree is clean, or that any + // particular fake is pinned. That is the job of the run this self-test gates, + // and duplicating it would make a genuine violation surface as a self-test + // failure — the least legible message available. + const { found } = audit(); + expect('discovers engine doubles in the real tree', found.length > 0); + expect( + 'discovery reaches the #4434 fake', + found.some((f) => f.file === 'packages/plugins/plugin-sharing/src/sharing-rule.test.ts'), + ); + + if (failures.length) { + for (const f of failures) console.error(` x self-test: ${f}`); + console.error(`\ncheck-engine-double-contract --self-test: ${failures.length} failure(s).\n`); + process.exit(1); + } + console.log( + 'OK self-test: separates engine doubles from driver doubles, accepts only the producer\'s ' + + 'predicate (direct or one helper deep), rejects unused imports, hand-mirrored guards and ' + + 'look-alikes, and proves discovery reaches the real tree.', + ); +} + +if (process.argv.includes('--self-test')) selfTest(); +else report(); diff --git a/scripts/engine-double-contract.baseline.json b/scripts/engine-double-contract.baseline.json new file mode 100644 index 0000000000..d4c446aeaa --- /dev/null +++ b/scripts/engine-double-contract.baseline.json @@ -0,0 +1,238 @@ +{ + "$comment": [ + "Measured baseline for scripts/check-engine-double-contract.mjs (#4550).", + "", + "Each entry is a test file that declares a fake ObjectQL engine whose `delete` does NOT", + "route through `assertEngineDeleteDispatch` from @objectstack/objectql — so it may accept", + "a call the real engine refuses, which is exactly how #4434 shipped a dead REST route with", + "its suite green.", + "", + "SHRINK-ONLY and hand-edited under review. The checker fails in BOTH directions: a file", + "whose count went down must have its entry lowered or deleted in the same PR, and a file", + "with no unguarded double left must lose its entry entirely. There is deliberately no", + "`--fix`/`--update` flag — a generator would let a new lax fake be admitted by 'just run", + "the update command', which is precisely how a gate stops meaning anything.", + "", + "`unguarded` is measured by the scan, not asserted by hand. `why` says what stands between", + "the file and being pinned; `closes` says what removes the entry.", + "", + "WHAT THESE ENTRIES DO AND DO NOT CLAIM. They record that the double is structurally", + "looser than the contract. They do NOT claim the looseness is currently harmless — proving", + "that per file means running each suite with the guard installed, and the one package where", + "that was done wholesale in this PR (@objectstack/spec, 295 files) passed. The rest is", + "unproven either way, which is the honest state and the reason each entry is DEBT rather", + "than EXEMPT." + ], + "entries": [ + { + "file": "packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/metadata-protocol 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/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/metadata-protocol 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/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/metadata-protocol 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/metadata-protocol/src/sys-metadata-repository.recorded-by.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/metadata-protocol 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/plugins/plugin-approvals/src/approval-actor-impersonation.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-node.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-override-audit.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-restart-resume.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-revise.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/plugins/plugin-approvals/src/approval-service.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/plugins/plugin-auth/src/auth-manager.optional-plugin-isolation.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/plugin-auth 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/plugins/plugin-reports/src/report-export-axis.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/plugin-reports 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/plugins/plugin-reports/src/report-service.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/plugin-reports 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/plugins/plugin-security/src/auto-org-admin-grant.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "HAND-MIRRORS the guard already (a copy of the #4434 fix's `if`), which is the second copy of the contract this gate exists to remove — but @objectstack/plugin-security 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/plugins/plugin-security/src/permission-set-projection.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/plugin-security 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/plugins/plugin-security/src/suggested-audience-bindings.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/plugin-security 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/plugins/plugin-webhooks/src/auto-enqueuer.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/plugin-webhooks 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/runtime/src/action-body-identity.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "The package DOES depend on @objectstack/objectql, so this is a one-line pin — deferred only because this PR's slice is the gate plus the package #4434 came from, and an unmeasured suite flipping red belongs in its own PR.", + "closes": "open the fake's delete with assertEngineDeleteDispatch(opts) and run the package's suite" + }, + { + "file": "packages/services/service-automation/src/builtin/crud-config-aliases.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/builtin/crud-filter-guard.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/builtin/crud-runas.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/fault-edge-guard-containment.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/plugin-suspended-run-wiring.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/record-lookup-expand.integration.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/runas-grant-resolution.integration.test.ts", + "unguarded": 1, + "kind": "DEBT", + "why": "@objectstack/service-automation 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-automation/src/suspended-run-store.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-automation 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/services/service-datasource/src/__tests__/datasource-admin-plugin.test.ts", + "unguarded": 1, + "kind": "DEBT", + "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-datasource/src/__tests__/datasource-secret-binder.test.ts", + "unguarded": 1, + "kind": "DEBT", + "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, + "kind": "EXEMPT", + "why": "Not a stand-in that code under test drives — it is a TYPE-CONFORMANCE witness that IDataEngine is implementable, asserting only `typeof engine.delete === 'function'`. And it cannot be pinned even in principle: @objectstack/objectql depends on @objectstack/spec, so the import would invert the dependency. Ran clean under the guard anyway (spec: 295/295 files passed with the dispatch guard installed in every engine double).", + "closes": "nothing — permanent" + } + ] +}