objectstack-ai
diff --git a/‎.changeset/engine-update-dispatch-predicate.md‎
Lines changed: 21 additions & 0 deletions b/‎.changeset/engine-update-dispatch-predicate.md‎
Lines changed: 21 additions & 0 deletions
diff --git a/‎packages/objectql/src/engine-update-dispatch.test.ts‎
Lines changed: 145 additions & 0 deletions b/‎packages/objectql/src/engine-update-dispatch.test.ts‎
Lines changed: 145 additions & 0 deletions
@@ -0,0 +1,21 @@
+---
+'@objectstack/objectql': patch
+---
+
+`ObjectQL.update` 的三分支派发抽成生产者侧唯一判定 `engine-update-dispatch.ts`
+
+`delete` 的派发决策自 #4550 起就是一份共享判定(`resolveEngineDeleteDispatch`),任何顶替引擎的测试替身都能 import 它,因此结构上不可能比引擎更宽松。`update` 的同款三分支——标量 `where.id` → 按 id;`options.multi` → `driver.updateMany`;否则抛错——此前只是 `engine.ts` 里的一个内联字面量 throw,既没有导出的常量也没有可复用的函数。后果不是理论上的:#5393 给 flow 的 `update_record` / `delete_record` 补真实契约测试时,delete 侧能把假引擎钉死在生产者契约上,update 侧只能退而断言执行器交出的 options 包,并在文件头写明「不对引擎会不会接受它发表第二份意见」——因为唯一的替代做法是在 fake 里手抄一遍判定,而手抄必然漏掉 `where: { id: { $in: [...] } }` 看着像 id 实为谓词这一半(#4434 正是这样带着全绿的测试发布了一条对每个调用者都回 500 的路由)。同一个执行器的两个写入动词,一个能被绑定到生产者契约、另一个结构上不能,而谓词 update 的破坏性并不低——它覆盖每一行匹配记录的字段。
+
+本次新增:
+
+- `packages/objectql/src/engine-update-dispatch.ts`,导出 `resolveEngineUpdateDispatch` / `assertEngineUpdateDispatch` / `scalarUpdateId` / `ENGINE_UPDATE_REJECT_MESSAGE` / `ENGINE_UPDATE_DISPATCH_CASES`,均从 `@objectstack/objectql` 公开导出;
+- `ObjectQL.update` **自身改用它**——生产者与判定必须是同一份,否则只是第二份副本。
+
+这是**行为保持的重构**:三分支语义、`$in` 谓词判定、拒绝消息文本一字未改(`Update requires an ID or options.multi=true`,现在是导出常量 `ENGINE_UPDATE_REJECT_MESSAGE`)。判定里有两处刻意照抄而非「改良」了生产者的现状,并在模块头与测试中写明:
+
+1. `data.id` **不做标量测试**,只要为真就直接作为 id,且优先于 `where` 与 `multi`;
+2. 分支按**真值**而非 `!== undefined`,所以 `where: { id: 0 }` 不走按 id 路径。
+
+比生产者更「聪明」的判定就是第二份意见,正是 #4550 消除的东西;这两点该改的时候会在两个文件里一起改,现在那是一次编辑而不是两次。
+
+新测试 `engine-update-dispatch.test.ts` 不去对照写在旁边的期望表,而是用记录型 driver 驱动**真实引擎**跑完 `ENGINE_UPDATE_DISPATCH_CASES`,逐例断言引擎的实际行为等于判定的裁决——两半唯一同处一室的地方。
@@ -0,0 +1,145 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// objectstack#5480 — the shared update-dispatch predicate must be the REAL
+// engine's answer, not a second opinion that happens to agree today.
+//
+// Exactly the argument `engine-delete-dispatch.test.ts` makes for `delete`,
+// and it matters more here rather than less: a shared predicate that drifted
+// from `ObjectQL.update` would make every fake engine pinned to it confidently,
+// uniformly wrong, while the gate over them reported success. 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_UPDATE_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-update-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_UPDATE_DISPATCH_CASES,
+  ENGINE_UPDATE_REJECT_MESSAGE,
+  resolveEngineUpdateDispatch,
+  assertEngineUpdateDispatch,
+  scalarUpdateId,
+} from './engine-update-dispatch.js';
+
+/** Records which driver entry point the engine chose, if any. */
+function makeRecordingDriver() {
+  const calls: Array<{ fn: 'update' | 'updateMany'; 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>) { calls.push({ fn: 'update', arg: id }); return { id, ...data }; },
+    async updateMany(_o: string, ast: unknown) { calls.push({ fn: 'updateMany', arg: ast }); return 0; },
+    async delete() { return true; },
+    async deleteMany() { 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' }, tenant: { type: 'text' } },
+  } as any);
+  return { engine, calls };
+}
+
+/** What the real engine actually did with this `(data, options)` pair. */
+async function observeEngine(data: unknown, options: unknown): Promise<'by-id' | 'multi' | 'reject'> {
+  const { engine, calls } = await makeEngine();
+  try {
+    await engine.update('task', data as any, options as any);
+  } catch (e) {
+    if ((e as Error).message === ENGINE_UPDATE_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 === 'update' ? 'by-id' : 'multi';
+}
+
+describe('engine update dispatch — the shared predicate IS the engine (#5480)', () => {
+  it('has cases on both sides of the guard (an empty or one-sided set proves nothing)', () => {
+    const kinds = new Set(ENGINE_UPDATE_DISPATCH_CASES.map((c) => c.expect));
+    expect(kinds).toEqual(new Set(['by-id', 'multi', 'reject']));
+    expect(ENGINE_UPDATE_DISPATCH_CASES.filter((c) => c.expect === 'reject').length).toBeGreaterThan(3);
+  });
+
+  for (const c of ENGINE_UPDATE_DISPATCH_CASES) {
+    it(`real engine agrees with the predicate: ${c.what} → ${c.expect}`, async () => {
+      expect(resolveEngineUpdateDispatch(c.data, c.options).kind, 'predicate').toBe(c.expect);
+      expect(await observeEngine(c.data, c.options), 'real ObjectQL.update').toBe(c.expect);
+    });
+  }
+
+  it('rejects with the exact message a fake must reproduce', () => {
+    expect(() => assertEngineUpdateDispatch({ title: 'x' }, { where: { tenant: 't1' } }))
+      .toThrow(ENGINE_UPDATE_REJECT_MESSAGE);
+    // …and returns the dispatch (never `reject`) when the call is legal.
+    expect(assertEngineUpdateDispatch({ title: 'x' }, { where: { id: 'a' } })).toEqual({ kind: 'by-id', id: 'a' });
+    expect(assertEngineUpdateDispatch({ id: 'a' }, undefined)).toEqual({ kind: 'by-id', id: 'a' });
+    expect(assertEngineUpdateDispatch({ title: 'x' }, { multi: true })).toEqual({ kind: 'multi' });
+  });
+
+  it('scalarUpdateId treats operator objects and arrays as predicates, not ids', () => {
+    expect(scalarUpdateId({ where: { id: 'a' } })).toBe('a');
+    expect(scalarUpdateId({ where: { id: 7 } })).toBe(7);
+    expect(scalarUpdateId({ where: { id: { $in: ['a'] } } })).toBeUndefined();
+    expect(scalarUpdateId({ where: { id: ['a'] } })).toBeUndefined();
+    expect(scalarUpdateId({ where: { id: null } })).toBeUndefined();
+    expect(scalarUpdateId({ where: {} })).toBeUndefined();
+    expect(scalarUpdateId(undefined)).toBeUndefined();
+  });
+
+  // ── The two places `update` is NOT `delete`. Both are pinned here rather
+  //    than left to the reader, because they are exactly what a hand-copied
+  //    guard gets wrong in the OTHER direction: too strict, and the double
+  //    then refuses a call the producer accepts.
+  it('data.id outranks where and multi, and is NOT scalar-tested (the producer\'s rule, verbatim)', () => {
+    expect(resolveEngineUpdateDispatch({ id: 'rec_1' }, { where: { id: { $in: ['a'] } }, multi: true }))
+      .toEqual({ kind: 'by-id', id: 'rec_1' });
+    // An operator object parked in the PAYLOAD is taken as an id — the engine
+    // does exactly this today, so the predicate must say so too. Improving on
+    // the producer here would make this module a second opinion, which is the
+    // thing #4550 removed. Tracked as #5748; when it is fixed it is fixed in
+    // both files at once, which is now one edit instead of two, and this
+    // assertion is what tells the next author to turn BOTH halves over.
+    const operatorInPayload = resolveEngineUpdateDispatch({ id: { $in: ['a', 'b'] } }, { multi: true });
+    expect(operatorInPayload.kind).toBe('by-id');
+  });
+
+  it('branches on TRUTHINESS, so a falsy scalar id does not identify a row', () => {
+    expect(resolveEngineUpdateDispatch({ title: 'x' }, { where: { id: 0 } }).kind).toBe('reject');
+    expect(resolveEngineUpdateDispatch({ title: 'x' }, { where: { id: '' } }).kind).toBe('reject');
+    expect(resolveEngineUpdateDispatch({ id: 0, title: 'x' }, { multi: true }).kind).toBe('multi');
+    // …while `scalarUpdateId` still reports the raw scalar it found. The two
+    // answer different questions and only `resolveEngineUpdateDispatch`
+    // answers the engine's.
+    expect(scalarUpdateId({ where: { id: 0 } })).toBe(0);
+  });
+
+  it('reads data UNGUARDED, exactly like the producer', () => {
+    // `ObjectQL.update` opens with `data.id`, so a missing payload is a
+    // TypeError there. A double kinder than the producer about it would hide
+    // the producer's behaviour.
+    expect(() => resolveEngineUpdateDispatch(undefined as any, { multi: true })).toThrow(TypeError);
+  });
+});