feat(objectql): 把假引擎钉在 ObjectQL.delete 的真实契约上 (#4550)

.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.

.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

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",

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<string, unknown>) { return { id: 'r1', ...data }; },
+    async update(_o: string, id: string, data: Record<string, unknown>) { 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();
+  });
+});

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<string, unknown>))) return undefined;
+  const whereId = (where as Record<string, unknown>).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<EngineDeleteDispatch, { kind: 'reject' }> {
+  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' },
+];