From 2f292ece0b1b3bebe8713a47d10d2f0f8ef16579 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 21:50:18 +0000 Subject: [PATCH 1/3] fix(objectql): refuse undeclared update fields at the schema, before beforeUpdate (#8738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The insert path's declared-field door (#8682) applied to the second write verb — the same function, given its second caller and a verb-neutral name, rather than a second predicate to drift against. The card filed this half as INFERRED and asked for a reproduction first. Both claims reproduce on origin/main @ e5eeb499c: an undeclared key reached driver.update on the by-id branch and driver.updateMany on the predicate branch and was refused THERE, after beforeUpdate had run and stamped a derived value onto the payload the driver then rejected. Unlike insert there is no autonumber, so nothing durable is consumed — the hook side effect and declared = enforced (PD #10) are the case, and the hook run is what the suite pins. The wire answer is unchanged: the same 400 INVALID_FIELD with the same message. #8737's three no-opinion cases (absent map, empty map, id/created_at/updated_at) are reused unchanged and pinned as controls. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --- .changeset/undeclared-update-field-door.md | 11 + .../engine-undeclared-update-field.test.ts | 347 ++++++++++++++++++ packages/objectql/src/engine.ts | 71 +++- 3 files changed, 423 insertions(+), 6 deletions(-) create mode 100644 .changeset/undeclared-update-field-door.md create mode 100644 packages/objectql/src/engine-undeclared-update-field.test.ts diff --git a/.changeset/undeclared-update-field-door.md b/.changeset/undeclared-update-field-door.md new file mode 100644 index 0000000000..b7a7fdee89 --- /dev/null +++ b/.changeset/undeclared-update-field-door.md @@ -0,0 +1,11 @@ +--- +"@objectstack/objectql": patch +--- + +Refuse undeclared fields on update at the schema, before the `beforeUpdate` hooks run (#8738) + +**An undeclared key on `engine.update(...)` is now refused by the object's field map, before anything runs for a request that was already going to be refused.** Previously it travelled the whole update path and was refused at the very end by the driver — measured on both branches of the verb: `driver.update` on the by-id path and `driver.updateMany` on the predicate path each received the mistyped key. The `beforeUpdate` hooks ran first, so a hook that stamps a ledger, calls out, or derives a field executed for a write that was then rejected; in the reproduction the hook's derived value travelled into the statement the driver refused. + +**What a caller observes changing.** The refusal itself does not move: an undeclared update key was already rejected, and the client-facing answer is deliberately unchanged — the same `400 INVALID_FIELD` with the same message, `field` and `object`, which `@objectstack/rest` re-emits verbatim. What changes is where the refusal is decided, and therefore what the error carries **inside the process**: an in-process caller of `ObjectQL.update()` that caught the old failure saw the driver's raw error (no `code`, no `status`, its message containing the bound SQL statement) and now sees the ADR-0112 envelope (`code: 'INVALID_FIELD'`, `status: 400`) with a message naming the field. An in-process caller matching on the driver's SQL text — rather than on the envelope — is the one shape that has to change. The write no longer costs a driver round-trip either: the pre-update read is skipped along with the hooks. + +The door is the same one `insert()` has carried since #8682 — one condition, one implementation, now with two callers — including its three deliberate no-opinion cases, which are unchanged and reused rather than re-derived: an absent field map, a field map the door sees as empty, and `id` / `created_at` / `updated_at` when a declaration omits them. Schema drift (a declared field whose physical column is missing) stays the driver's to refuse, as before. Nothing is widened; `declared = enforced` (Prime Directive #10) is restored on the second write verb. diff --git a/packages/objectql/src/engine-undeclared-update-field.test.ts b/packages/objectql/src/engine-undeclared-update-field.test.ts new file mode 100644 index 0000000000..22af020c06 --- /dev/null +++ b/packages/objectql/src/engine-undeclared-update-field.test.ts @@ -0,0 +1,347 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #8738 — an undeclared field must be refused by the SCHEMA on the UPDATE path +// too, before the `beforeUpdate` hooks run. The sibling of #8682's insert door +// (`engine-undeclared-field-preflight.test.ts`), and deliberately the same +// door: one condition, one implementation, two callers. +// +// ## The card filed this half as INFERRED — here is the measurement +// +// #8738 says in as many words that nobody had run an update-path reproduction, +// and asks for one before anyone implements. Run on `origin/main` @ `e5eeb499c` +// with a real `ObjectQL` and the recording driver below, one mistyped key: +// +// by-id driver.update received `zzz_nonexistent_field` → refused THERE +// multi driver.updateMany received it likewise → refused THERE +// hooks `beforeUpdate` ran FIRST on both branches +// payload {name, zzz_nonexistent_field, description:'derived-for-bad'} +// ← `description` is the HOOK's derived value, not the caller's: it +// was computed for, and travelled with, a request the server had +// already decided to refuse. +// envelope the thrown error carried NO `code` and NO `status` — the driver's +// raw string, which `mapDataError` translated at the REST boundary. +// +// Both inferred claims reproduce. The premise stands. +// +// ## What the pin is, and what it deliberately is NOT +// +// The insert half pinned an AUTONUMBER GAP, because an insert issues a sequence +// value that a refused request consumed permanently. **Update has no such +// observable** — no autonumber, nothing durable consumed — so the card is +// milder by exactly that much, and the pin has to be the thing that IS at +// stake: the HOOK RUN. `beforeUpdate` stamping a ledger, calling out, or +// deriving a field for a request that is then refused is the whole defect here, +// so `hookRuns` is asserted directly rather than inferred from a counter. +// +// A suite that only asserted "an undeclared key is refused" would be satisfied +// by a door that refuses everything, so every case below has its positive +// twin: declared keys still update on both branches, and each of the three +// no-opinion cases is pinned as a CONTROL that must pass with the door removed +// as well as with it in place. + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; + +/** Records everything that reached the driver — presence is the point. */ +function makeRecordingDriver(missingColumns: readonly string[] = []) { + const writes: Array<{ fn: string; data: Record }> = []; + const stored: Record = { id: 'row-1', name: 'stored', description: 'd' }; + 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 [{ ...stored }]; }, + async findOne() { return { ...stored }; }, + async create(object: string, data: Record) { + writes.push({ fn: 'create', data: { ...data } }); + return { id: 'rec_1', ...data }; + }, + async update(object: string, id: string, data: Record) { + writes.push({ fn: 'update', data: { ...data } }); + const bad = missingColumns.find((c) => c in data); + // The shape knex produces: the bound statement, then ` - `, then the + // database's own diagnostic. + if (bad) throw new Error(`update \`${object}\` set \`${bad}\` = 'v' where \`id\` = '${id}' - table ${object} has no column named ${bad}`); + return { ...stored, ...data, id }; + }, + async updateMany(object: string, _ast: unknown, data: Record) { + writes.push({ fn: 'updateMany', data: { ...data } }); + const bad = missingColumns.find((c) => c in data); + if (bad) throw new Error(`update \`${object}\` set \`${bad}\` = 'v' - table ${object} has no column named ${bad}`); + return 1; + }, + async delete() { return true; }, + async deleteMany() { return 0; }, + async count() { return 1; }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => driver.create(object, r))); + }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, writes }; +} + +function silentLogger() { + const logger: any = { + trace() {}, debug() {}, info() {}, warn() {}, error() {}, fatal() {}, + child() { return logger; }, + }; + return logger; +} + +/** + * The `beforeUpdate` hook DERIVES a value the caller never sent — the card's + * `period_label = 'Q3 2026'` shape — so "the hook ran" is a fact about app + * behaviour reaching the statement, not merely a counter ticking. + */ +async function makeEngine(options: { + missingColumns?: readonly string[]; + /** `'declared'` (default) · `'none'` (registry-less) */ + registration?: 'declared' | 'none'; + /** + * Replace what `getObject('acct')` answers, AFTER a normal registration. + * + * Necessary rather than decorative, and it is the shape the real fixtures + * have: `registerObject({ fields: {} })` does NOT leave the map empty — the + * registry INJECTS `organization_id`, `created_at`, `created_by`, + * `updated_at`, `updated_by`, `owner_id` and `owning_business_unit_id` on + * top, measured here. So an object registered with an empty map is not an + * object whose map the door SEES as empty, and a control built that way + * would pass for a reason that has nothing to do with the rule it claims to + * pin. The 15 `fields: {}` fixtures #8737 triaged carry a registry STUB, not + * a registration — this reproduces that, and only for `acct`. + */ + stubFields?: Record; +} = {}) { + const engine = new ObjectQL({ logger: silentLogger() }); + const { driver, writes } = makeRecordingDriver(options.missingColumns ?? []); + engine.registerDriver(driver, true); + await engine.init(); + const registration = options.registration ?? 'declared'; + if (registration === 'declared') { + engine.registry.registerObject({ + name: 'acct', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + name: { name: 'name', type: 'text' }, + description: { name: 'description', type: 'text' }, + }, + } as any, 'test'); + } + if (options.stubFields) { + const inner = engine.registry.getObject.bind(engine.registry); + (engine.registry as any).getObject = (name: string) => + (name === 'acct' ? { name: 'acct', fields: options.stubFields } : inner(name)); + } + const hookRuns: string[] = []; + engine.registerHook('beforeUpdate', (ctx: any) => { + hookRuns.push(String(ctx.input.data?.name ?? '?')); + ctx.input.data.description = `derived-for-${ctx.input.data?.name}`; + }, { object: 'acct' }); + return { engine, writes, hookRuns }; +} + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e; + } + return null; +} + +describe('#8738 — the declared-field door on update()', () => { + describe('the ordering claim — the card`s actual subject', () => { + it('by-id: the beforeUpdate hook does NOT run for a payload carrying an undeclared key', async () => { + const { engine, hookRuns } = await makeEngine(); + + await refusalOf(() => engine.update('acct', { id: 'row-1', name: 'bad', zzz_nonexistent_field: 'x' } as any)); + + // `['bad']` on `origin/main`: the hook ran, and its derived `description` + // reached the statement the driver then rejected. A hook is not a pure + // function — it stamps ledgers and calls out — so running it for a + // refused request is a side effect, not a wasted cycle. + expect(hookRuns).toEqual([]); + }); + + it('multi: the beforeUpdate hook does NOT run either — the predicate branch has the same hole', async () => { + const { engine, hookRuns } = await makeEngine(); + + await refusalOf(() => engine.update( + 'acct', + { name: 'bad', zzz_nonexistent_field: 'x' } as any, + { where: { name: 'stored' }, multi: true } as any, + )); + + expect(hookRuns).toEqual([]); + }); + + it('the hook still runs — and is still the last word — when every key is declared', async () => { + // The other direction of the ordering pin: the door refuses a payload, it + // does not suppress the hook phase. Without this, "the hook did not run" + // is satisfied by a door that refuses everything. + const { engine, writes, hookRuns } = await makeEngine(); + + await engine.update('acct', { id: 'row-1', name: 'ok' } as any); + + expect(hookRuns).toEqual(['ok']); + expect(writes).toHaveLength(1); + expect(writes[0].data.description).toBe('derived-for-ok'); + }); + }); + + describe('the refusal', () => { + it('by-id: nothing reaches the driver', async () => { + const { engine, writes } = await makeEngine(); + + await refusalOf(() => engine.update('acct', { id: 'row-1', name: 'bad', zzz_nonexistent_field: 'x' } as any)); + + // Zero, not one: the pre-update read is skipped too. A refused write + // should not cost a driver round-trip, and `previous` / the not-found + // gate / the `readonlyWhen` gate — the read's three consumers — are all + // downstream of a payload this door never lets through. + expect(writes).toHaveLength(0); + }); + + it('multi: nothing reaches the driver', async () => { + const { engine, writes } = await makeEngine(); + + await refusalOf(() => engine.update( + 'acct', + { name: 'bad', zzz_nonexistent_field: 'x' } as any, + { where: { name: 'stored' }, multi: true } as any, + )); + + expect(writes).toHaveLength(0); + }); + + it('refuses in the ADR-0112 envelope, with the wire answer unchanged', async () => { + const { engine } = await makeEngine(); + + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', name: 'bad', zzz_nonexistent_field: 'x' } as any)); + + // `code` AND `status` — the envelope, not merely "it threw". On + // `origin/main` both were `undefined` here: what the engine threw was the + // driver's raw string, and only `mapDataError` at the REST boundary gave + // it a shape. + expect(refusal?.code).toBe('INVALID_FIELD'); + expect(refusal?.status).toBe(400); + expect(refusal?.field).toBe('zzz_nonexistent_field'); + expect(refusal?.object).toBe('acct'); + // Byte-identical to what `mapDataError`'s driver-string branch produced + // for the same mistake, so the caller reads exactly what it read before — + // the refusal moved, the answer did not. + expect(refusal?.message).toBe("Unknown field 'zzz_nonexistent_field' on object 'acct'"); + }); + + it('names every undeclared key, not only the first', async () => { + const { engine } = await makeEngine(); + + const refusal = await refusalOf(() => engine.update('acct', { + id: 'row-1', name: 'bad', zzz_one: 1, zzz_two: 2, + } as any)); + + expect(refusal?.field).toBe('zzz_one'); + expect(refusal?.fields).toEqual(['zzz_one', 'zzz_two']); + }); + + it('a key holding `undefined` is still an undeclared key', async () => { + // `{ ...partial }` is how this arrives from code rather than from JSON, + // and a mistyped key is a mistyped key whatever it holds. + const { engine } = await makeEngine(); + + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', zzz_typo: undefined } as any)); + + expect(refusal?.code).toBe('INVALID_FIELD'); + expect(refusal?.field).toBe('zzz_typo'); + }); + }); + + describe('declared keys still update normally', () => { + it('by-id: a declared payload lands on the driver untouched', async () => { + const { engine, writes } = await makeEngine(); + + await engine.update('acct', { id: 'row-1', name: 'renamed' } as any); + + expect(writes).toHaveLength(1); + expect(writes[0].fn).toBe('update'); + expect(writes[0].data.name).toBe('renamed'); + }); + + it('multi: a declared payload lands on the driver untouched', async () => { + const { engine, writes } = await makeEngine(); + + await engine.update( + 'acct', + { name: 'renamed' } as any, + { where: { name: 'stored' }, multi: true } as any, + ); + + expect(writes).toHaveLength(1); + expect(writes[0].fn).toBe('updateMany'); + expect(writes[0].data.name).toBe('renamed'); + }); + }); + + // The three no-opinion cases are #8737's, reused rather than re-derived — + // settled rules from the sibling card. Each is a CONTROL: it asserts the door + // has NO verdict, so it must pass with the door removed as well as with it in + // place, and a reverse verification that turned one of these red would mean + // the door had grown an opinion it is not allowed to have. + describe('where the door deliberately has NO opinion (reused from #8737)', () => { + it('a registry-less host gets no verdict — the driver stays the backstop', async () => { + const { engine, writes } = await makeEngine({ registration: 'none', missingColumns: ['zzz_nonexistent_field'] }); + + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', zzz_nonexistent_field: 'x' } as any)); + + expect(writes).toHaveLength(1); + expect(String(refusal?.message)).toContain('has no column named zzz_nonexistent_field'); + }); + + it('an EMPTY field map gets no verdict — an absence is not a prohibition', async () => { + // A real registered object always carries at least its primary key and + // the registry's injected audit columns, so a map the door sees as EMPTY + // means the host did not fill it in. Refusing everything on that reading + // would be a verdict made from an absence. + const { engine, writes } = await makeEngine({ stubFields: {}, missingColumns: ['zzz_nonexistent_field'] }); + + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', zzz_nonexistent_field: 'x' } as any)); + + expect(writes).toHaveLength(1); + expect(String(refusal?.message)).toContain('has no column named zzz_nonexistent_field'); + }); + + it('`id` / `created_at` / `updated_at` pass even when the declaration omits them', async () => { + // Mirrors the three names `find()` / `findOne()` already add to their + // known set: platform-provisioned rather than authored, so a key accepted + // by a read is not refused by a write. Stubbed rather than registered + // for the reason `stubFields` records — the registry would otherwise + // inject `created_at` / `updated_at` itself and the case would prove + // nothing about the door. `id` is the one name the registry does NOT + // inject, so it is the door's tolerance being read here, and only its. + const { engine, writes } = await makeEngine({ + stubFields: { name: { name: 'name', type: 'text' } }, + }); + + const refusal = await refusalOf(() => engine.update('acct', { + id: 'row-1', name: 'ok', created_at: '2026-01-01T00:00:00.000Z', updated_at: '2026-01-02T00:00:00.000Z', + } as any)); + + expect(refusal).toBeNull(); + expect(writes).toHaveLength(1); + }); + + it('schema drift — a DECLARED field whose column is missing — still reaches the driver', async () => { + // The door's scope is the SCHEMA's field map, so drift is invisible to it + // by construction and stays the driver's to refuse. `mapDataError`'s + // driver-string branch is still needed and still fires. + const { engine, writes } = await makeEngine({ missingColumns: ['description'] }); + + const refusal = await refusalOf(() => engine.update('acct', { id: 'row-1', description: 'v' } as any)); + + expect(writes).toHaveLength(1); + expect(String(refusal?.message)).toContain('has no column named description'); + }); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 95017f1f53..eac1ff8e33 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -994,9 +994,16 @@ function assertProjectionHasNoDottedPaths( } /** - * [#8682] The DECLARED-FIELD DOOR on the insert path: a key the object does not - * declare is refused by the SCHEMA, before anything is produced for a request - * that is about to be refused anyway. + * [#8682 / #8738] The DECLARED-FIELD DOOR on the WRITE paths — `insert()` and, + * since #8738, `update()`: a key the object does not declare is refused by the + * SCHEMA, before anything is produced for a request that is about to be refused + * anyway. + * + * ONE condition, ONE implementation, deliberately — the two verbs sit in the + * same file and share the shape of the problem, so #8738 gave this function its + * second caller rather than a second predicate to drift against. The verdict is + * a question about a payload and an object's field map; neither term knows + * which verb asked. * * ## What ran before this door existed * @@ -1022,6 +1029,25 @@ function assertProjectionHasNoDottedPaths( * so "it ran and we threw the result away" is not a no-op, it is a side effect * of a request the server had already decided to refuse. * + * ## The UPDATE path had the same hole (#8738), measured the same way + * + * #8738 filed the update half as INFERRED, not measured, and asked for it to be + * reproduced before anyone implemented against it. It was, on `origin/main` @ + * `e5eeb499c`, with a real engine and a recording driver — and it reproduces on + * BOTH branches of the verb: + * + * - by-id `driver.update` received `zzz_nonexistent_field` and refused it; + * - multi `driver.updateMany` received it and refused it likewise; + * - `beforeUpdate` RAN first on both, and its derived value reached the + * statement — the recorded payload was `{ name, zzz_nonexistent_field, + * description: 'derived-for-bad' }`, where `description` is the hook's, not + * the caller's. The card's `period_label = 'Q3 2026'` shape, one verb over. + * + * What it does NOT have is the insert path's durable damage: no autonumber is + * issued on update, so nothing user-visible is consumed by the refused request. + * The hook side effect and `declared = enforced` (PD #10) are the whole case — + * which is why the pin here is the HOOK, not a sequence gap. + * * ## Why HERE and not one step later * * This runs as the first act inside the middleware body: after middleware (a @@ -1034,6 +1060,17 @@ function assertProjectionHasNoDottedPaths( * Like those two it throws without logging — the caller is told exactly what is * wrong, and a client typo is not a server fault to record at ERROR. * + * `update()` places it by the same rule, which lands it one step EARLIER in + * that verb's body than a naive reading suggests: first act inside the + * middleware body, so ahead of the prior-record read as well as the hooks and + * both strips. Ahead of the read is not incidental — a refused write should not + * cost a driver round-trip either, and the read exists to serve `previous`, the + * `readonlyWhen` gate and the not-found gate, none of which a refused payload + * reaches. It is also ahead of the dispatch ladder's own `reject` verdict, so a + * call that is BOTH mis-keyed and missing its `id`/`multi` is answered on the + * payload: the ladder's message is about how to address rows and would send an + * author hunting for the wrong defect, while `INVALID_FIELD` names the typo. + * * ## The verdict is the SCHEMA's field map, and the wire answer is unchanged * * `INVALID_FIELD` + 400, and the message is byte-identical to the one @@ -1075,7 +1112,7 @@ function assertProjectionHasNoDottedPaths( */ const PLATFORM_PROVISIONED_COLUMNS = ['id', 'created_at', 'updated_at'] as const; -function undeclaredInsertFieldErrors( +function undeclaredWriteFieldErrors( object: string, schema: { fields?: unknown } | undefined, rows: readonly unknown[], @@ -8084,14 +8121,14 @@ export class ObjectQL implements IObjectQLEngine { // untouched, hooks run after and may override. const nowSnap = new Date(); const isBatch = Array.isArray(opCtx.data); - // [#8682] The declared-field door — see `undeclaredInsertFieldErrors` for + // [#8682] The declared-field door — see `undeclaredWriteFieldErrors` for // what used to run below it for a request that was already refused. // FIRST, so nothing downstream (defaults, summary seeding, the hooks, the // secret writes, validation, the autonumber) happens for a row the schema // rejects. Non-partial callers get the refusal thrown here; the // partial-success path (`insertMany`) carries it per row into `rowErrors` // below, where the culled rows also skip the hooks and every producer. - const undeclaredPerRow = undeclaredInsertFieldErrors( + const undeclaredPerRow = undeclaredWriteFieldErrors( object, this._registry.getObject(object) as { fields?: unknown } | undefined, isBatch ? (opCtx.data as unknown[]) : [opCtx.data], @@ -8817,6 +8854,28 @@ export class ObjectQL implements IObjectQLEngine { }; await this.executeWithMiddleware(opCtx, async () => { + // [#8738] The declared-field door, the insert path's (#8682) applied to + // the second write verb — same function, not a second predicate. First + // act inside the middleware body: after middleware (which may rewrite + // `data`), and before the prior-record read, the `beforeUpdate` hooks, + // both readonly strips, validation and the statement. + // + // Measured on `origin/main` before it existed — the update half of the + // card was filed as INFERRED and this is what the reproduction found: + // `zzz_nonexistent_field` reached `driver.update` (and `updateMany` on + // the multi branch) and was refused THERE, after `beforeUpdate` had run + // and stamped a derived value onto the payload the driver then rejected. + // + // Single-row by construction: `update()` takes one payload, so there is + // no partial-row mode to carry the verdict into — unlike `insert()`, the + // refusal is simply thrown. One element in, one verdict out. + const undeclared = undeclaredWriteFieldErrors( + object, + this._registry.getObject(object) as { fields?: unknown } | undefined, + [opCtx.data], + )[0]; + if (undeclared) throw undeclared; + const hookContext: HookContext = { object, event: 'beforeUpdate', From 4832398a10fd3b72852fe30f3055fdcfd24362e6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 22:26:49 +0000 Subject: [PATCH 2/3] test(runtime): exercise the #4271 driver split through bodies on both verbs (#8738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one red in the downstream sweep that was NOT a fixture field-map defect. `undeclared-field-write-driver-split` pinned the schemaless family PERSISTING an undeclared key on update by calling engine.update() with the typo in the CALLER's payload — on the file's stated reasoning that a beforeUpdate body "would only add the flat-input envelope to the thing under test". The declared-field door falsifies that equivalence, and the same thing is already true of insert on main: measured here, a caller-supplied undeclared key on the MEMORY driver is refused INVALID_FIELD/400 with nothing persisted, by #8737's insert door. The file survived that only because its insert arm injects the key through a hook body, which runs after the door. So both update cases now carry a real beforeUpdate body, matching the insert arm: the key is added below the engine's validation, still reaches the driver, and the split the file exists to pin is measured on both verbs (SQL: `no such column: stagee`, whole write lost; memory: persisted alongside the declared key). The caller-payload half is pinned separately as what it now is — a schema refusal on BOTH families, with no split to observe. The three prose surfaces the file guards (the two lint messages and content/docs/automation/hook-bodies.mdx "What still happens at runtime") describe BODY writes, which the door does not touch, so they stay accurate as written. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --- ...eld-write-driver-split.integration.test.ts | 117 ++++++++++++++++-- 1 file changed, 105 insertions(+), 12 deletions(-) diff --git a/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts b/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts index ba82f312d6..fa9ec5567f 100644 --- a/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts +++ b/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts @@ -33,12 +33,24 @@ * `content/docs/automation/hook-bodies.mdx` all describe this split; if any of * them drifts back to "silently never lands", one half of this file fails. * - * The insert cases run the FULL chain — real QuickJS sandbox, real hook body, - * real engine, real driver — so link 1 is proved rather than assumed: if - * `applyMutationsToInput` ever learned to filter, the SQL insert would stop - * throwing. The update cases go straight at the engine, because that is where - * `validateRecord`'s update branch lives and a `beforeUpdate` body would only - * add the flat-input envelope to the thing under test. + * Every case runs the FULL chain — real QuickJS sandbox, real hook body, real + * engine, real driver — so link 1 is proved rather than assumed: if + * `applyMutationsToInput` ever learned to filter, the SQL write would stop + * throwing. + * + * [#8738] The update cases used to go straight at the engine instead, on the + * reasoning that a `beforeUpdate` body "would only add the flat-input envelope + * to the thing under test". That equivalence is gone, and its loss is what this + * file now has to say out loud: the DECLARED-FIELD DOOR (#8682 on insert, + * #8738 on update) refuses a key the CALLER names before any hook runs, so a + * caller payload no longer stands in for a body mutation. Both update cases + * therefore carry a real `beforeUpdate` body, matching the insert arm, and the + * caller-payload case is pinned separately at the foot of the file — where the + * answer is a schema refusal on BOTH families and there is no split at all. + * + * Which half is which matters for reading the three prose surfaces above: they + * describe what a BODY write does, and a body write is exactly the half the + * door does not touch. */ /** @@ -162,6 +174,34 @@ const CORRECT_HOOK = { body: { language: 'js', source: `ctx.input.stage = 'won';` }, }; +/** + * [#8738] The same authoring mistake on the UPDATE verb, and it has to be a + * BODY rather than a caller payload — which is a change of METHOD, not of + * subject. + * + * These two cases used to call `engine.update(...)` with the typo in the + * caller's own payload, on the file's stated reasoning that "a `beforeUpdate` + * body would only add the flat-input envelope to the thing under test". The + * declared-field door falsifies that equivalence: since #8682 on insert and + * #8738 on update, a key the CALLER names is refused by the schema before any + * hook runs, so a caller payload no longer stands in for a body mutation — it + * tests the door instead, and the driver split it is supposed to reach is + * never exercised. + * + * The subject is unchanged and still measured on both families: a key a BODY + * writes is added AFTER the door and still reaches the driver verbatim, so + * `applyMutationsToInput` → `validateRecord`'s `if (!def) continue` → the + * driver is intact, and it is what `content/docs/automation/hook-bodies.mdx` + * ("What still happens at runtime") and the two lint messages describe. The + * caller-payload half now has its own cases below, pinning the door. + */ +const UPDATE_TYPO_HOOK = { + name: 'deal_stage_typo_update', + object: 'deal', + events: ['beforeUpdate'], + body: { language: 'js', source: `ctx.input.stagee = 'won';` }, +}; + describe('#4271 an undeclared field written by an L2 body — the real runtime split', () => { let engine: ObjectQL | null = null; let dir: string | null = null; @@ -218,11 +258,13 @@ describe('#4271 an undeclared field written by an L2 body — the real runtime s }, 30000); it('fails an UPDATE the same way, and leaves the row untouched', async () => { - const e = await bootSql(); + const e = await bootSql(UPDATE_TYPO_HOOK); const row = await e.insert('deal', { stage: 'open', amount: 10 }); - // `validateRecord`'s update branch `continue`s past the unknown key - // rather than rejecting it, so the driver is what refuses the write. - await expect(e.update('deal', { id: row.id, stagee: 'won' } as any)) + // The caller's payload is entirely DECLARED, so the door passes it; the + // body then adds the typo, and `validateRecord`'s update branch + // `continue`s past the unknown key rather than rejecting it, so the + // driver is still what refuses the write. + await expect(e.update('deal', { id: row.id, stage: 'negotiating' } as any)) .rejects.toThrow(/stagee/); const after: any = (await e.find('deal', { where: { id: row.id } } as any))[0]; expect(after.stage).toBe('open'); @@ -255,11 +297,62 @@ describe('#4271 an undeclared field written by an L2 body — the real runtime s }, 30000); it('persists it on UPDATE too', async () => { - const e = await bootMemory(); + const e = await bootMemory(UPDATE_TYPO_HOOK); const row = await e.insert('deal', { stage: 'open', amount: 10 }); - await e.update('deal', { id: row.id, stagee: 'won' } as any); + await e.update('deal', { id: row.id, stage: 'negotiating' } as any); const stored: any = (await e.find('deal', { where: { id: row.id } } as any))[0]; expect(stored.stagee).toBe('won'); + // The declared key of the same write landed as well — the body's typo + // costs the schemaless family nothing, which is the half of the split + // that makes "it fails" the wrong thing to tell an author here. + expect(stored.stage).toBe('negotiating'); + }, 30000); + }); + + // ─── The other half of the same question: who refuses a CALLER's typo ────── + + /** + * [#8682 / #8738] The declared-field door, and the reason the cases above had + * to move to bodies. + * + * The driver split is a fact about keys that arrive BELOW the engine's own + * validation — which is what a body mutation is, and what a caller payload + * has stopped being. A key the caller names is now refused by the object's + * FIELD MAP, before the hooks, before the statement, and — the point that + * decides these two cases — before any driver is consulted at all. So there + * is no split to observe: the verdict is a schema verdict, and both families + * get the identical ADR-0112 envelope. + * + * Pinned here rather than left implicit because this file is where a reader + * comes to learn what happens to an undeclared write, and half an answer + * ("SQL fails, schemaless persists") is what sent the old lint message + * wrong in the first place. + */ + describe('a CALLER-supplied undeclared key — the schema refuses, on both families', () => { + it('SQL: refused before the driver, and the row is untouched', async () => { + const e = await bootSql(); + const row = await e.insert('deal', { stage: 'open', amount: 10 }); + + const err: any = await e.update('deal', { id: row.id, stagee: 'won' } as any).catch((x: unknown) => x); + + expect(err?.code).toBe('INVALID_FIELD'); + expect(err?.status).toBe(400); + const after: any = (await e.find('deal', { where: { id: row.id } } as any))[0]; + expect(after.stage).toBe('open'); + }, 30000); + + it('schemaless: refused too — the door is a schema verdict, not a driver one', async () => { + // The one case in this file where the two families AGREE, and it is not + // a coincidence: nothing here ever reaches a driver to disagree. + const e = await bootMemory(); + const row = await e.insert('deal', { stage: 'open', amount: 10 }); + + const err: any = await e.update('deal', { id: row.id, stagee: 'won' } as any).catch((x: unknown) => x); + + expect(err?.code).toBe('INVALID_FIELD'); + expect(err?.status).toBe(400); + const stored: any = (await e.find('deal', { where: { id: row.id } } as any))[0]; + expect(stored).not.toHaveProperty('stagee'); }, 30000); }); }); From 62648ff4a536089ce1636c861a276b701c7cd3a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 23:08:24 +0000 Subject: [PATCH 3/3] test(objectql,runtime): type the new options bags and drop an unused parameter (#8738) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ratchets the new test code moved, both fixed at the author's end rather than by raising a ledger: - check:query-options-erasure — the test surface grew 240 → 242. The two new `as any` options bags are not deliberately off-contract input, so they are TYPED (EngineUpdateOptions for the predicate branch, EngineQueryOptions for the read-back) instead of erased. Back at the 240 ceiling. - check:type-check-debt — objectql's TEST_DEBT re-measured 355 → 356 on one TS6133 (an unused `object` parameter in the recording driver's `create`). Renamed to `_object`; re-measured at 355, matching the ledger, with zero errors attributable to this card's files. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XeQRiAa7vYRVX5Fog7Zby8 --- .../src/engine-undeclared-update-field.test.ts | 16 ++++++++++++---- ...-field-write-driver-split.integration.test.ts | 13 +++++++++++-- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/engine-undeclared-update-field.test.ts b/packages/objectql/src/engine-undeclared-update-field.test.ts index 22af020c06..9a97ef48bc 100644 --- a/packages/objectql/src/engine-undeclared-update-field.test.ts +++ b/packages/objectql/src/engine-undeclared-update-field.test.ts @@ -41,6 +41,14 @@ import { describe, it, expect } from 'vitest'; import { ObjectQL } from './engine.js'; +import type { EngineUpdateOptions } from '@objectstack/spec/data'; + +/** + * The predicate branch's options bag, TYPED rather than cast — the payload is + * what these cases are about, and an `as any` here would erase the contract on + * the argument that decides which branch of `update()` runs. + */ +const MULTI_OPTIONS: EngineUpdateOptions = { where: { name: 'stored' }, multi: true }; /** Records everything that reached the driver — presence is the point. */ function makeRecordingDriver(missingColumns: readonly string[] = []) { @@ -51,7 +59,7 @@ function makeRecordingDriver(missingColumns: readonly string[] = []) { async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, async find() { return [{ ...stored }]; }, async findOne() { return { ...stored }; }, - async create(object: string, data: Record) { + async create(_object: string, data: Record) { writes.push({ fn: 'create', data: { ...data } }); return { id: 'rec_1', ...data }; }, @@ -171,7 +179,7 @@ describe('#8738 — the declared-field door on update()', () => { await refusalOf(() => engine.update( 'acct', { name: 'bad', zzz_nonexistent_field: 'x' } as any, - { where: { name: 'stored' }, multi: true } as any, + MULTI_OPTIONS, )); expect(hookRuns).toEqual([]); @@ -210,7 +218,7 @@ describe('#8738 — the declared-field door on update()', () => { await refusalOf(() => engine.update( 'acct', { name: 'bad', zzz_nonexistent_field: 'x' } as any, - { where: { name: 'stored' }, multi: true } as any, + MULTI_OPTIONS, )); expect(writes).toHaveLength(0); @@ -275,7 +283,7 @@ describe('#8738 — the declared-field door on update()', () => { await engine.update( 'acct', { name: 'renamed' } as any, - { where: { name: 'stored' }, multi: true } as any, + MULTI_OPTIONS, ); expect(writes).toHaveLength(1); diff --git a/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts b/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts index fa9ec5567f..a180e30dfb 100644 --- a/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts +++ b/packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts @@ -148,6 +148,15 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { InMemoryDriver } from '@objectstack/driver-memory'; import { hookBodyRunnerFactory } from './body-runner.js'; import { QuickJSScriptRunner } from './quickjs-runner.js'; +import type { EngineQueryOptions } from '@objectstack/spec/data'; + +/** + * The read-back query, TYPED rather than cast. The `as any` reads elsewhere in + * this file predate the `query-options-erasure` ratchet and are counted as + * grandfathered residue; new ones are not, and there is no reason for these + * two to be erased — the options bag is an ordinary `where`. + */ +const rowById = (id: unknown): EngineQueryOptions => ({ where: { id } }); /** `stagee` is the typo under test; `stage` is the field that exists. */ const DEAL = { @@ -337,7 +346,7 @@ describe('#4271 an undeclared field written by an L2 body — the real runtime s expect(err?.code).toBe('INVALID_FIELD'); expect(err?.status).toBe(400); - const after: any = (await e.find('deal', { where: { id: row.id } } as any))[0]; + const after: any = (await e.find('deal', rowById(row.id)))[0]; expect(after.stage).toBe('open'); }, 30000); @@ -351,7 +360,7 @@ describe('#4271 an undeclared field written by an L2 body — the real runtime s expect(err?.code).toBe('INVALID_FIELD'); expect(err?.status).toBe(400); - const stored: any = (await e.find('deal', { where: { id: row.id } } as any))[0]; + const stored: any = (await e.find('deal', rowById(row.id)))[0]; expect(stored).not.toHaveProperty('stagee'); }, 30000); });