From 7b64f963bcdc26d73df71a118b8ddae3ae006f16 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 23:31:44 +0000 Subject: [PATCH 1/2] =?UTF-8?q?docs(spec):=20HookContext.input=20=E5=A5=91?= =?UTF-8?q?=E7=BA=A6=E8=A1=A8=E6=94=B9=E6=88=90=E5=BC=95=E6=93=8E=E7=9C=9F?= =?UTF-8?q?=E6=AD=A3=E6=9E=84=E9=80=A0=E7=9A=84=E5=BD=A2=E7=8A=B6=20(#5273?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `HookContext.input` table named three keys no producer sets: `ast` on bulk update AND bulk delete, and `doc` on insert. `input` is `z.record(z.string(), z.unknown())` — an open shape — so Zod validated none of it and the prose was the only contract an author could read. - Bulk writes carry no `ast`. The row-scoping predicate lives on the engine-internal `OperationContext.ast` (#2982) so middleware-composed filters bind the driver call where no handler can widen them. Deleted the "the row-scoping predicate is carried in `input.ast`" sentence. - `input.id` on a bulk before-event is present but `undefined` (the engine builds `{ id, … }` with shorthand), not absent — documented as such, since `'id' in input` answers true. - Documented the post-#5038 per-row after-event shape: `after*` on a bulk write dispatches once per matched row on a single-record-shaped context, so `input.id` IS bound there. - insert builds `{ data }`, not `{ doc }`. Kept: before-events still fire once per batch, and there is no `*Many` event. No engine change. The `ast` special-case in `hook-wrappers.ts` deliberately stays — it is live on the READ path, where `input.ast` is real and a handler may rewrite it. Pinned in `packages/objectql/src/hook-input-shape-contract.test.ts`: spec cannot execute a dispatch (objectql depends on spec, so a spec-side test would invert the dependency), so the facts are asserted next to the engine that produces them. `beforeFind` is the positive control (#4865) — it really does carry `ast`, so "no ast on writes" is a measurement rather than a vacuous pass. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --- .../src/hook-input-shape-contract.test.ts | 320 ++++++++++++++++++ packages/spec/src/data/hook.zod.ts | 41 ++- 2 files changed, 355 insertions(+), 6 deletions(-) create mode 100644 packages/objectql/src/hook-input-shape-contract.test.ts diff --git a/packages/objectql/src/hook-input-shape-contract.test.ts b/packages/objectql/src/hook-input-shape-contract.test.ts new file mode 100644 index 0000000000..1f40a7e9e1 --- /dev/null +++ b/packages/objectql/src/hook-input-shape-contract.test.ts @@ -0,0 +1,320 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5273] The `HookContext.input` shape table in `packages/spec` is TRUE of + * this engine. + * + * `packages/spec/src/data/hook.zod.ts` documents, per operation, the exact + * `input` a handler receives. That table is the whole contract: `input` itself + * is `z.record(z.string(), z.unknown())` — an open shape by design, so Zod + * validates NOTHING about which keys are present, and the prose is the only + * thing an author (human or AI) can read to learn what to reach for. Which is + * why it drifted silently: three keys it named had no producer left. + * + * - `update (bulk)` / `delete (bulk)` were documented as carrying + * `{ ast: QueryAST, ... }`, with the table repeating below it that "the + * row-scoping predicate is carried in `input.ast`". The engine has never + * put an AST on a WRITE context: the bulk predicate lives on the internal + * `OperationContext.ast` (#2982) precisely so middleware-composed row + * filters bind the driver call where no handler can widen them. So the one + * field the docs pointed at resolved `undefined`. + * - `insert` was documented as `{ doc: Record, ... }`; the engine builds + * `{ data: row, ... }`. (`trigger-record-change` still carries a defensive + * `input.doc` alias read for that reason — filed separately, not fixed + * here.) + * + * The table was also silent about #5038: since ADR-0058's bulk-write addendum + * the `after*` events on a bulk write fire PER MATCHED ROW on a + * single-record-shaped context, so `input.id` — documented as absent on bulk + * writes — is in fact bound on every after-event a bulk write dispatches. + * + * ## Why this file lives in objectql + * + * The defect is in spec's prose, but prose is unassertable and `packages/spec` + * cannot execute a hook dispatch: objectql depends on spec, so a spec-side + * test importing the engine would invert the dependency. The FACTS the prose + * claims are pinned here instead, next to the engine that produces them and + * next to #5038's own `bulk-write-per-row-hooks.test.ts`. + * + * ## Reading the assertions + * + * Hooks are registered with `engine.registerHook` — the RAW context, so what + * is asserted is what the engine constructs, not a view of it. The declarative + * (metadata `Hook`) path additionally wraps `ctx.input` in the flat-input + * proxy of `hook-wrappers.ts`; the last describe pins that an author on THAT + * path sees the same answer, since the false `input.ast` sentence was aimed at + * exactly those authors. + * + * `beforeFind` is the POSITIVE CONTROL (#4865): it really does carry + * `input.ast`, so "no `ast` on the write paths" is a measurement, not an + * assertion that would pass against an engine that had stopped setting `ast` + * anywhere at all. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; +import type { Hook, HookContext } from '@objectstack/spec/data'; + +const TASK_FIELDS = { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + status: { name: 'status', label: 'Status', type: 'text' as const }, +}; +const taskObject = { name: 'task', label: 'Task', fields: TASK_FIELDS }; + +const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} }; + +/* ──────────────────────────────────────────────────────────────────────────── + * 1. The row-scoping predicate is NOT on `input` (the deleted claim) + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#5273] a bulk write carries no `ast` on `input`', () => { + it('POSITIVE CONTROL — a read DOES carry `input.ast`', async () => { + // Without this, every "no ast" assertion below would also pass against an + // engine that had stopped building read contexts correctly. + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeFind', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + await engine.find('task', {} as any); + + expect(seen).toHaveLength(1); + expect('ast' in seen[0]!).toBe(true); + expect(seen[0]!.ast).toBeDefined(); + }); + + it('`beforeUpdate` on a bulk write has no `ast` key', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]); + await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any); + + expect(seen).toHaveLength(1); // before* fires ONCE for the whole batch + expect('ast' in seen[0]!).toBe(false); + expect(seen[0]!.ast).toBeUndefined(); + }); + + it('`beforeDelete` on a bulk write has no `ast` key', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]); + await engine.delete('task', { multi: true, where: { status: 'todo' } } as any); + + expect(seen).toHaveLength(1); + expect('ast' in seen[0]!).toBe(false); + expect(seen[0]!.ast).toBeUndefined(); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 2. `input.id` — present-but-undefined on the batch, bound per row after + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#5273] `input.id` on a bulk write', () => { + it('`beforeUpdate` leaves `id` undefined (the key exists; nothing binds it)', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]); + await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any); + + // The engine builds `{ id, data, options }` with the shorthand `id`, so the + // KEY is there while the value is not. Documented as `{ id: undefined, … }` + // rather than "no id" because `'id' in input` answers true. + expect('id' in seen[0]!).toBe(true); + expect(seen[0]!.id).toBeUndefined(); + expect(seen[0]!.data).toEqual({ status: 'done' }); + expect(seen[0]!.options).toBeDefined(); + }); + + it('`afterUpdate` fires per matched row, each naming its own `id`', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('afterUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + const rows = await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]); + await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any); + + expect(seen).toHaveLength(2); + expect(seen.map((i) => i.id).sort()).toEqual(rows.map((r) => r.id).sort()); + // Single-record shape: the payload rides along, exactly as on a single-id + // write, so a handler needs no bulk-aware branch. + for (const input of seen) expect(input.data).toEqual({ status: 'done' }); + }); + + it('`afterDelete` fires per matched row, each naming its own `id`', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('afterDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + const rows = await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]); + await engine.delete('task', { multi: true, where: { status: 'todo' } } as any); + + expect(seen).toHaveLength(2); + expect(seen.map((i) => i.id).sort()).toEqual(rows.map((r) => r.id).sort()); + // A delete has no post-state, so no payload rides along. + for (const input of seen) expect('data' in input).toBe(false); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 3. The rest of the table, so the whole thing is measured and not just the + * two rows #5273 named + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#5273] the single-record rows of the table', () => { + it('insert carries `data` — never `doc`', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + await engine.insert('task', { title: 'a', status: 'todo' } as any); + + expect(seen).toHaveLength(1); + expect(seen[0]!.data).toMatchObject({ title: 'a', status: 'todo' }); + expect('doc' in seen[0]!).toBe(false); + }); + + it('a batch insert builds ONE context per row (#2922)', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + await engine.insert('task', [{ title: 'a' }, { title: 'b' }] as any); + + expect(seen).toHaveLength(2); + expect(seen.map((i) => (i.data as any).title)).toEqual(['a', 'b']); + }); + + it('a single-id update binds `id` and `data`', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + const [row] = await seedTasks(engine, [{ title: 'a', status: 'todo' }]); + await engine.update('task', { status: 'done' }, { where: { id: row.id } } as any); + + expect(seen[0]!.id).toBe(row.id); + expect(seen[0]!.data).toEqual({ status: 'done' }); + expect('ast' in seen[0]!).toBe(false); + }); + + it('a single-id delete binds `id` and carries no `data`', async () => { + const seen: Array> = []; + const { engine } = await boot(); + engine.registerHook('beforeDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); + + const [row] = await seedTasks(engine, [{ title: 'a', status: 'todo' }]); + await engine.delete('task', { where: { id: row.id } } as any); + + expect(seen[0]!.id).toBe(row.id); + expect('data' in seen[0]!).toBe(false); + expect('ast' in seen[0]!).toBe(false); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * 4. The declarative path sees the same answer + * ──────────────────────────────────────────────────────────────────────────── */ + +describe('[#5273] a metadata-declared hook reads the same shape', () => { + it('`ctx.input.ast` is undefined on a bulk update through the flat-input proxy', async () => { + // The deleted sentence told THIS author to read `input.ast`. The proxy + // passes `ast` through to the wrapper rather than folding it into `data`, + // so the read is faithful — there is simply nothing behind it on a write. + const seen: unknown[] = []; + const { engine } = await boot(); + bindHooksToEngine( + engine, + [{ + name: 'reads_ast', object: 'task', events: ['beforeUpdate'], priority: 100, + handler: (ctx: HookContext) => { seen.push((ctx.input as any).ast); }, + } as unknown as Hook], + { packageId: 'app:test', logger: silentLogger }, + ); + + await seedTasks(engine, [{ title: 'a', status: 'todo' }]); + await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any); + + expect(seen).toEqual([undefined]); + }); +}); + +/* ──────────────────────────────────────────────────────────────────────────── + * Harness — a memory driver just wide enough for the dispatch paths above. + * ──────────────────────────────────────────────────────────────────────────── */ + +async function seedTasks(engine: ObjectQL, rows: Record[]): Promise { + const written = await engine.insert('task', rows as any); + return Array.isArray(written) ? written : [written]; +} + +function makeMemoryDriver(): any { + const stores = new Map>>(); + const storeFor = (o: string) => { + let s = stores.get(o); + if (!s) { s = new Map(); stores.set(o, s); } + return s; + }; + let nextId = 0; + const matches = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k.startsWith('$')) continue; + const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (expected ?? null)) return false; + } + return true; + }; + const d: any = { + name: 'memory', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async execute() { return null; }, async syncSchema() {}, + async find(o: string, ast: any) { + return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + }, + async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; }, + async create(o: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row = { ...data, id }; storeFor(o).set(id, row); return row; + }, + async update(o: string, id: string, data: Record) { + const s = storeFor(o); const cur = s.get(id); if (!cur) return null; + const u = { ...cur, ...data, id }; s.set(id, u); return u; + }, + async upsert(o: string, data: any) { const id = data.id; return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data); }, + async delete(o: string, id: string) { return storeFor(o).delete(id); }, + async count(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)).length; }, + async bulkCreate(o: string, rows: any[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async updateMany(o: string, ast: any, data: Record) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + for (const r of rows) storeFor(o).set(r.id as string, { ...r, ...data, id: r.id }); + return rows.length; + }, + async deleteMany(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + for (const r of rows) storeFor(o).delete(r.id as string); + return rows.length; + }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return d; +} + +async function boot(): Promise<{ engine: ObjectQL; driver: any }> { + const engine = new ObjectQL(); + const driver = makeMemoryDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(taskObject as any); + return { engine, driver }; +} diff --git a/packages/spec/src/data/hook.zod.ts b/packages/spec/src/data/hook.zod.ts index bc514f8720..c010ba36a6 100644 --- a/packages/spec/src/data/hook.zod.ts +++ b/packages/spec/src/data/hook.zod.ts @@ -311,17 +311,46 @@ export const HookContextSchema = lazySchema(() => z.object({ * Input Parameters (Mutable) * Modify this to change the behavior of the operation. * + * These shapes are exactly what the engine BUILDS — `packages/objectql`'s + * `engine.ts` is the only producer of a `HookContext`. A key this table + * lists but no producer sets reads back `undefined` at every call site, and + * a contract statement is the one place that cannot be discovered as false: + * #5273 was three such keys (`ast` on both bulk writes, `doc` on insert) + * standing in the table long after the engine had stopped agreeing. Pinned + * against the real engine in + * `packages/objectql/src/hook-input-shape-contract.test.ts` — the assertions + * live there because only objectql can execute a dispatch, and spec must not + * depend on it. + * * - find (also fires for findOne): { ast: QueryAST, options: DriverOptions } - * - insert: { doc: Record, options: DriverOptions } + * - insert (one context per row, batch inserts included): { data: Record, options: DriverOptions } * - update (single id): { id: ID, data: Record, options: DriverOptions } - * - update (bulk, multi:true): { ast: QueryAST, data: Record, options: DriverOptions } + * - update (bulk, multi:true) — before: { id: undefined, data: Record, options: DriverOptions } + * - update (bulk, multi:true) — after, PER MATCHED ROW: { id: ID, data: Record, options: DriverOptions } * - delete (single id): { id: ID, options: DriverOptions } - * - delete (bulk, multi:true): { ast: QueryAST, options: DriverOptions } + * - delete (bulk, multi:true) — before: { id: undefined, options: DriverOptions } + * - delete (bulk, multi:true) — after, PER MATCHED ROW: { id: ID, options: DriverOptions } * * A bulk (`multi: true`) update/delete fires the SAME `beforeUpdate`/ - * `beforeDelete` events as a single-id write — the row-scoping predicate is - * carried in `input.ast` (no per-row `id`). There is no separate `*Many` - * event. + * `beforeDelete` events as a single-id write, ONCE for the whole batch; + * there is no separate `*Many` event. `input.id` is present but `undefined` + * there — binding it is precisely the test the engine dispatches on, so a + * `before*` handler that sets it REROUTES the write onto the single-id path. + * + * The row-scoping predicate is NOT reachable from `input` at all. It lives + * on the engine-internal `OperationContext.ast` (#2982) so that the filters + * middleware composes onto it — RLS write policies, the sharing plugin's + * editable-rows filter — bind the driver call itself, where no handler can + * widen them. A bulk write therefore hands hooks no queryable predicate: + * scope the batch through `options.where` at the CALLER, or work per row on + * the `after*` events below. + * + * Since #5038 (ADR-0058's bulk-write addendum) the `after*` events on a bulk + * write dispatch ONCE PER MATCHED ROW, each on a single-record-shaped + * context — `input.id` names that row, `previous` is its pre-image and + * `result` its post-state — so a handler written for a single-id write needs + * no bulk-aware branch of its own. The batch-level context keeps the + * affected COUNT as `result` and is what the call itself resolves (#4639). */ input: z.record(z.string(), z.unknown()).describe('Mutable input parameters'), From dc6bfbf7a92e5ed9051a2a46d9188de79b3e8ad8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 00:03:58 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(objectql):=20=E6=AD=A3=E6=8E=A7?= =?UTF-8?q?=E7=9A=84=20find=20=E9=80=89=E9=A1=B9=E4=B8=8D=E5=86=8D?= =?UTF-8?q?=E6=93=A6=E6=88=90=20`any`,=E8=BF=87=20#4918=20query-options=20?= =?UTF-8?q?=E6=A3=98=E8=BD=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:query-options-erasure`(PR #5600 / #4918,在本分支切出之后落地) 把测试面计入只增不减的棘轮。本分支新增的正控里 `engine.find('task', {} as any)` 是其中一处擦除,使测试面 267 → 268 而红。 该调用点是**合约内**形状——`find(object, query?: EngineQueryOptions)` 的空查询——不是「断言引擎拒绝未知选项」那类刻意越契约的输入,所以按 门禁处方的第 1 条直接给它正确类型(这里等于去掉断言,签名本就能推 断),而不是写 `as unknown as EngineQueryOptions`。 基线文件未动:抬高天花板是「reviewed edit, not a remedy」,这里修的是 站点本身。 验证(合并 origin/main 后): - node scripts/check-query-options-erasure-ratchet.mjs → 267,at the ceiling,no files added - @objectstack/objectql test → 1934 passed (120 files) - @objectstack/spec + @objectstack/objectql typecheck → Done Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --- packages/objectql/src/hook-input-shape-contract.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/objectql/src/hook-input-shape-contract.test.ts b/packages/objectql/src/hook-input-shape-contract.test.ts index 1f40a7e9e1..961bf1312c 100644 --- a/packages/objectql/src/hook-input-shape-contract.test.ts +++ b/packages/objectql/src/hook-input-shape-contract.test.ts @@ -77,7 +77,11 @@ describe('[#5273] a bulk write carries no `ast` on `input`', () => { const { engine } = await boot(); engine.registerHook('beforeFind', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' }); - await engine.find('task', {} as any); + // No `as any` on the options: `find(object, query?: EngineQueryOptions)` + // already infers an empty query, and erasing it would add a site to the + // #4918 query-options ratchet (`check:query-options-erasure`) for no gain — + // this call is in-contract, not a deliberate off-contract probe. + await engine.find('task', {}); expect(seen).toHaveLength(1); expect('ast' in seen[0]!).toBe(true);