From a2552ab363000c7a2cbad2b5414062ac8e7a7494 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 02:35:54 +0000 Subject: [PATCH 1/2] fix(objectql): enforce validation rules, requiredWhen and option visibleWhen on multi-row updates (#3106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bulk branch of engine.update (options.multi → driver.updateMany) never called evaluateValidationRules, so every object-level rule (script, state_machine, format, cross_field, json_schema, conditional), field-level requiredWhen, and per-option visibleWhen check was a silent no-op there — the only signal was a warning that skipped format/json_schema-only schemas entirely. The engine now reads the row-scoped match set once (the same middleware- composed AST the write binds, shared with the #3042 readonlyWhen strip) and evaluates the stripped payload against each matched row's prior state; any error-severity violation throws ValidationError (annotated with the failing record id) before updateMany writes anything. Schemas needing no prior state are evaluated once against the payload with no fetch; rule-free schemas (messaging outboxes, settings rows) hit the evaluator's early return and are unaffected. Deliberate behavior change: bulk writes that previously slipped past declared rules now throw. Also corrects the two doc comments that overstated coverage (rule-validator.ts, validation.zod.ts) — both now name the remaining events:['delete'] gap, which is tracked separately — and refreshes the AGENTS.md PD #10 example that described the pre-fix state. Closes #3106 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VCUSMJBsX14C3RQdWFw7N7 --- .changeset/bulk-update-validation-rules.md | 6 + AGENTS.md | 2 +- packages/objectql/src/engine.test.ts | 144 ++++++++++++++++++ packages/objectql/src/engine.ts | 60 ++++++-- .../objectql/src/plugin.integration.test.ts | 106 +++++++++++++ .../objectql/src/validation/rule-validator.ts | 13 +- packages/spec/src/data/validation.zod.ts | 4 +- 7 files changed, 314 insertions(+), 21 deletions(-) create mode 100644 .changeset/bulk-update-validation-rules.md diff --git a/.changeset/bulk-update-validation-rules.md b/.changeset/bulk-update-validation-rules.md new file mode 100644 index 0000000000..a15675d285 --- /dev/null +++ b/.changeset/bulk-update-validation-rules.md @@ -0,0 +1,6 @@ +--- +"@objectstack/objectql": patch +"@objectstack/spec": patch +--- + +Enforce validation rules, `requiredWhen`, and per-option `visibleWhen` on multi-row updates (#3106). The bulk branch of `engine.update` (`options.multi` → `driver.updateMany`) previously never called `evaluateValidationRules`, so every object-level rule (`script`, `state_machine`, `format`, `cross_field`, `json_schema`, `conditional`), field-level `requiredWhen`, and per-option `visibleWhen` check was a silent no-op there. The engine now reads the row-scoped match set (the same AST the write binds, one query shared with the `readonlyWhen` bulk strip) and evaluates the payload against each matched row's prior state; any error-severity violation rejects the whole batch with `ValidationError` (annotated with the failing record id) before anything is written. Schemas needing no prior state (`format`/`json_schema`-only) are evaluated once against the payload with no fetch, and rule-free schemas are unaffected. Behavior change: bulk writes that previously slipped past declared rules now throw. Doc comments in `rule-validator.ts` and `validation.zod.ts` no longer overstate coverage and name the remaining `events: ['delete']` gap (tracked separately). diff --git a/AGENTS.md b/AGENTS.md index e2f54c12f8..60a9ab2062 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -71,7 +71,7 @@ Other scripts: `objectui:bump` (pull only), `objectui:build`, `objectui:clean`. - Pre-existing vars that don't fit (`OS_METADATA_WRITABLE`, `OS_EAGER_SCHEMAS`, `OS_SERVER_TIMING`) are **debt, not precedent** — new vars follow this rule; rename old ones via the deprecation helper below when touched. When renaming a legacy var, use `readEnvWithDeprecation('OS_NEW', 'LEGACY')` from `@objectstack/types` (keeps legacy working one release). Third-party exceptions kept as-is: `NODE_ENV`, `HOME`, `OPENAI_API_KEY`, `TURSO_*`, OAuth `*_CLIENT_ID/SECRET`, `RESEND_API_KEY`, `POSTMARK_TOKEN`, `AI_GATEWAY_*`, `SMTP_*`. See #1382. -10. **File issues for out-of-scope findings — don't silently expand scope or leave them buried.** When you hit a bug, gap, or unenforced capability that's unrelated to the current task, or too large to fix in scope, open a GitHub issue (`gh issue create`) with a clear repro/decision and link it from your PR. Corollary: **never advertise or demo a capability the runtime doesn't actually deliver** (declared ≠ enforced) — fix it, trim it, or file an issue, but don't fake coverage. Example: the spec once declared 9 validation-rule types while the write-path validator enforced only 3 (`state_machine`/`script`/`cross_field`); the gap was filed as #1475 rather than demoed in the showcase, then closed by **trimming** what could never be enforced (`unique`/`async`/`custom`) and **implementing** the rest — the spec now declares 6 and `rule-validator.ts` handles all 6. Note how narrow that claim stays even so: the evaluator is wired into insert and single-id update only, so a bulk `updateMany` still skips every rule — silently, and for `format`/`json_schema` without even the warning (#3106). A `case` label is not enforcement; check the **call site**. +10. **File issues for out-of-scope findings — don't silently expand scope or leave them buried.** When you hit a bug, gap, or unenforced capability that's unrelated to the current task, or too large to fix in scope, open a GitHub issue (`gh issue create`) with a clear repro/decision and link it from your PR. Corollary: **never advertise or demo a capability the runtime doesn't actually deliver** (declared ≠ enforced) — fix it, trim it, or file an issue, but don't fake coverage. Example: the spec once declared 9 validation-rule types while the write-path validator enforced only 3 (`state_machine`/`script`/`cross_field`); the gap was filed as #1475 rather than demoed in the showcase, then closed by **trimming** what could never be enforced (`unique`/`async`/`custom`) and **implementing** the rest — the spec now declares 6 and `rule-validator.ts` handles all 6. Note how narrow that claim stayed even so: the evaluator was wired into insert and single-id update only, so a bulk `updateMany` silently skipped every rule — a second `declared ≠ enforced` gap one layer down, at the **call site** rather than the `switch`; filed as #3106 and closed by evaluating the bulk match set per row. A `case` label is not enforcement; check the **call site**. 11. **Worktree-first — never edit on the shared `main` checkout.** This repo is edited by **multiple agents at once**; the shared `main` tree has its HEAD switched and reset *under you*, silently clobbering uncommitted work. Before your **first file edit**, you MUST be in a dedicated worktree on a feature branch: `git worktree add ../framework- -b main && cd ../framework- && pnpm install`. A PreToolUse hook (`.claude/hooks/guard-main-checkout.sh`) **enforces** this — it blocks `Edit`/`Write`/`NotebookEdit` unless the edited file is in a dedicated **worktree** — a feature branch on the *shared* checkout is **not** enough (it still gets switched under you) — and it checks the **edited file's own repo**, so sibling repos (`objectui`/`cloud`) you touch are covered too (override for a deliberate non-task fix with `OS_ALLOW_MAIN_EDITS=1`). Full playbook below. 12. **Contract-first — fix the metadata, not the runtime.** This is a metadata-driven framework: `packages/spec` is the one contract between metadata *producers* and the runtime/renderers that *consume* it. When a piece of metadata "doesn't work," ask **first**: *is it spec-compliant? is this the long-term-correct direction?* If the metadata is wrong, fix it at the **producer** and **reject it at authoring/publish** (validation / lint) so the error surfaces loudly — do **not** add a lenient alias or `??` fallback in the consumer (a node executor, the REST layer, a renderer) to tolerate off-spec input. A tolerant fallback fossilizes the wrong convention into a second de-facto contract, dilutes the spec, and hides the producer's bug — one strict contract beats N dialects. This is an **internal** contract (we own both ends), so "be liberal in what you accept" (Postel) does **not** apply — that's for untrusted boundaries. Change the **spec** only when the spec itself is genuinely wrong, and then deliberately (edit the Zod schema + migrate), never by accreting consumer-side fallbacks. The existing `cfg.filter ?? cfg.filters` / `cfg.objectName ?? cfg.object` in the flow executors are **debt to pay down, not a pattern to copy**. *Worked example:* an AI-authored `create_record` used `fieldValues` / `today()` / `{{trigger.record.id}}` while the executor reads `fields` / `{TODAY()}` / `{record.id}` → the fix was correcting the authoring skill + a publish-gate lint that rejects the wrong shape (cloud#688), **not** a `cfg.fields ?? cfg.fieldValues` runtime alias (framework#2419, rejected). Strengthens #5. diff --git a/packages/objectql/src/engine.test.ts b/packages/objectql/src/engine.test.ts index 22b3092b06..6a098ef9a7 100644 --- a/packages/objectql/src/engine.test.ts +++ b/packages/objectql/src/engine.test.ts @@ -698,6 +698,150 @@ describe('ObjectQL Engine', () => { }); }); + describe('Bulk update validation enforcement (#3106)', () => { + beforeEach(async () => { + engine.registerDriver(mockDriver, true); + await engine.init(); + (mockDriver as any).updateMany = vi.fn().mockResolvedValue(2); + }); + + it('enforces a prior-free format rule on multi without fetching rows', async () => { + // format / json_schema need nothing from the prior record, so the + // bulk path must evaluate them against the payload with NO fetch. + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'acct', + fields: { email: { type: 'text' } }, + validations: [{ type: 'format', name: 'email_format', message: 'email must be a valid email', field: 'email', format: 'email' }], + } as any); + + await expect( + engine.update('acct', { email: 'not-an-email' }, { where: { status: 'active' }, multi: true } as any), + ).rejects.toThrow(/valid email/); + expect((mockDriver as any).updateMany).not.toHaveBeenCalled(); + expect(mockDriver.find).not.toHaveBeenCalled(); + }); + + it('lets a valid payload through the same format rule', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'acct', + fields: { email: { type: 'text' } }, + validations: [{ type: 'format', name: 'email_format', message: 'email must be a valid email', field: 'email', format: 'email' }], + } as any); + + await engine.update('acct', { email: 'a@example.com' }, { where: { status: 'active' }, multi: true } as any); + expect((mockDriver as any).updateMany).toHaveBeenCalledTimes(1); + }); + + it('evaluates a state_machine rule per matched row and rejects the whole batch when one row violates', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'task', + fields: { status: { type: 'select' } }, + validations: [{ type: 'state_machine', name: 'status_flow', message: 'illegal status transition', field: 'status', transitions: { pending: ['in_flight'], done: ['archived'] } }], + } as any); + // Rows are fetched with the SAME row-scoped ast the write binds. + vi.mocked(mockDriver.find).mockResolvedValue([ + { id: 'a', status: 'pending' }, // pending → in_flight is legal + { id: 'b', status: 'done' }, // done → in_flight is not + ] as any); + + await expect( + engine.update('task', { status: 'in_flight' }, { where: { status: { $in: ['pending', 'done'] } }, multi: true } as any), + ).rejects.toThrow(/illegal status transition \(record b\)/); + expect(mockDriver.find).toHaveBeenCalledTimes(1); + const [obj, ast] = vi.mocked(mockDriver.find).mock.calls[0]; + expect(obj).toBe('task'); + expect((ast as any).where).toEqual({ status: { $in: ['pending', 'done'] } }); + expect((mockDriver as any).updateMany).not.toHaveBeenCalled(); + }); + + it('proceeds when every matched row passes the state_machine rule', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'task', + fields: { status: { type: 'select' } }, + validations: [{ type: 'state_machine', name: 'status_flow', message: 'illegal status transition', field: 'status', transitions: { pending: ['in_flight'], done: ['archived'] } }], + } as any); + vi.mocked(mockDriver.find).mockResolvedValue([ + { id: 'a', status: 'pending' }, + { id: 'b', status: 'pending' }, + ] as any); + + await engine.update('task', { status: 'in_flight' }, { where: { status: 'pending' }, multi: true } as any); + expect((mockDriver as any).updateMany).toHaveBeenCalledTimes(1); + }); + + it('does not fetch rows for a rule-free schema (outbox/settings bulk writes stay zero-cost)', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any); + + await engine.update('task', { status: 'done' }, { where: { status: 'pending' }, multi: true } as any); + expect(mockDriver.find).not.toHaveBeenCalled(); + expect((mockDriver as any).updateMany).toHaveBeenCalledTimes(1); + }); + + it('enforces requiredWhen against each matched row\'s merged record', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'ticket', + fields: { + status: { type: 'select' }, + resolution: { type: 'text', requiredWhen: "record.status == 'closed'" }, + }, + } as any); + vi.mocked(mockDriver.find).mockResolvedValue([ + { id: 'a', status: 'open', resolution: 'fixed' }, // merged has a resolution + { id: 'b', status: 'open', resolution: null }, // merged is missing it + ] as any); + + await expect( + engine.update('ticket', { status: 'closed' }, { where: { status: 'open' }, multi: true } as any), + ).rejects.toThrow(/resolution is required \(record b\)/); + expect((mockDriver as any).updateMany).not.toHaveBeenCalled(); + }); + + it('enforces per-option visibleWhen against each matched row (cascade gating)', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'case', + fields: { + tier: { type: 'select' }, + priority: { + type: 'select', + options: [ + { value: 'urgent', visibleWhen: "record.tier == 'gold'" }, + { value: 'normal' }, + ], + }, + }, + } as any); + vi.mocked(mockDriver.find).mockResolvedValue([ + { id: 'a', tier: 'gold' }, // 'urgent' is available here + { id: 'b', tier: 'silver' }, // hidden option submitted for this row + ] as any); + + await expect( + engine.update('case', { priority: 'urgent' }, { where: { status: 'open' }, multi: true } as any), + ).rejects.toThrow(/option 'urgent' is not available \(record b\)/); + expect((mockDriver as any).updateMany).not.toHaveBeenCalled(); + }); + + it('shares ONE row fetch between the readonlyWhen strip and rule evaluation', async () => { + vi.mocked(SchemaRegistry.getObject).mockReturnValue({ + name: 'invoice', + fields: { + amount: { type: 'number', readonlyWhen: 'record.locked == true' }, + }, + validations: [{ type: 'cross_field', name: 'amount_cap', message: 'amount exceeds limit', condition: 'record.amount > record.limit', fields: ['amount'] }], + } as any); + vi.mocked(mockDriver.find).mockResolvedValue([ + { id: 'a', locked: false, amount: 10, limit: 100 }, + ] as any); + + await engine.update('invoice', { amount: 50 }, { where: { status: 'draft' }, multi: true } as any); + expect(mockDriver.find).toHaveBeenCalledTimes(1); + expect((mockDriver as any).updateMany).toHaveBeenCalledTimes(1); + // The unlocked conditional field survives the strip. + const [, , data] = (mockDriver as any).updateMany.mock.calls[0]; + expect(data).toHaveProperty('amount', 50); + }); + }); + describe('Expand Related Records', () => { beforeEach(async () => { engine.registerDriver(mockDriver, true); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 06f55b6475..6e10270ffb 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -30,7 +30,7 @@ import { ExpressionEngine } from '@objectstack/formula'; import type { Expression } from '@objectstack/spec'; import { isAggregatedViewContainer, expandViewContainer } from '@objectstack/spec'; import { bindHooksToEngine } from './hook-binder.js'; -import { validateRecord, normalizeMultiValueFields, coerceBooleanFields } from './validation/record-validator.js'; +import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError } from './validation/record-validator.js'; import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; @@ -2442,13 +2442,6 @@ export class ObjectQL implements IDataEngine { await this.encryptSecretFields(object, hookContext.input.data as Record, opCtx.context, hookContext.input.options); normalizeMultiValueFields(updateSchema, hookContext.input.data as Record); validateRecord(updateSchema, hookContext.input.data as Record, 'update'); - // Multi-row update: per-row prior state is not fetched for the - // object-level state_machine / cross_field / script rules (they - // need one prior per row, and a single bulk payload cannot diverge - // per row). Warn so the gap stays visible. - if (needsPriorRecord(updateSchema as any)) { - this.logger.warn('Object-level validation rules (state_machine/cross_field/script) are not enforced on multi-row updates', { object }); - } // [#2982] Consume the middleware-composed AST seeded above, so // the injected row-scoping (RLS write filter, sharing's // editable-rows filter) actually binds the driver operation. Fail @@ -2463,17 +2456,28 @@ export class ObjectQL implements IDataEngine { `(a hook cleared the target id after the security filter was composed).`, ); } + // [#3106] Validation rules, `requiredWhen` and per-option + // `visibleWhen` are PER ROW on a bulk update, exactly like the + // `readonlyWhen` strip below: one payload, N prior states. Read + // the row-scoped match set ONCE with the SAME AST the write binds + // (shared with the [#3042] strip), and only when the schema + // actually needs prior state — `needsPriorRecord` subsumes + // `hasReadonlyWhenInPayload` (readonlyWhen fields count toward + // it), so a rule-free schema still pays nothing here. + const rulesNeedRows = needsPriorRecord(updateSchema as any); + const payloadHasReadonlyWhen = hasReadonlyWhenInPayload(updateSchema as any, hookContext.input.data as Record); + let priorRows: Record[] | null = null; + if (rulesNeedRows || payloadHasReadonlyWhen) { + priorRows = await driver.find(object, ast, hookContext.input.options as any) as Record[]; + } // [#3042] Enforce conditional `readonlyWhen` on the bulk path too. // Unlike static `readonly` (below), a `readonlyWhen` lock is PER - // ROW, so read the row-scoped match set with the SAME AST the write - // binds (one query, and only when the payload actually writes a - // `readonlyWhen` field) and drop any field locked in ≥1 matched row - // — a bulk write can't keep it for some rows and drop it for others, - // so a field locked in any target row is fail-safe-dropped for all - // (narrow `where` to reach the unlocked rows). Symmetric with the + // ROW — drop any field locked in ≥1 matched row: a bulk write + // can't keep it for some rows and drop it for others, so a field + // locked in any target row is fail-safe-dropped for all (narrow + // `where` to reach the unlocked rows). Symmetric with the // single-id `stripReadonlyWhenFields`; INSERT stays exempt. - if (hasReadonlyWhenInPayload(updateSchema as any, hookContext.input.data as Record)) { - const priorRows = await driver.find(object, ast, hookContext.input.options as any); + if (payloadHasReadonlyWhen) { hookContext.input.data = stripReadonlyWhenFieldsMulti(updateSchema as any, hookContext.input.data as Record, priorRows, this.logger) as any; } // [#2948] Same static-`readonly` write guard on the bulk path — @@ -2483,6 +2487,30 @@ export class ObjectQL implements IDataEngine { if (!opCtx.context?.isSystem) { hookContext.input.data = stripReadonlyFields(updateSchema as any, hookContext.input.data as Record, suppliedKeys, this.logger) as any; } + // [#3106] Same enforcement the single-id branch runs at its + // `evaluateValidationRules` call, applied per matched row: any + // error-severity violation rejects the WHOLE batch before + // `updateMany` writes anything (all-or-nothing, like the strip's + // locked-in-any-row rule). Runs on the stripped payload, in the + // single-id branch's order. Warning-severity violations may log + // once per matched row — accepted. With no prior-dependent rules + // the payload-only evaluation covers format / json_schema / + // non-prior conditional at zero fetch cost. + const bulkEvalUser = this.buildEvalUser(opCtx.context); + if (rulesNeedRows) { + for (const row of priorRows ?? []) { + try { + evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: row, logger: this.logger, currentUser: bulkEvalUser }); + } catch (err) { + if (err instanceof ValidationError && row?.id != null) { + throw new ValidationError(err.fields.map((f) => ({ ...f, message: `${f.message} (record ${String(row.id)})` }))); + } + throw err; + } + } + } else { + evaluateValidationRules(updateSchema as any, hookContext.input.data as Record, 'update', { previous: null, logger: this.logger, currentUser: bulkEvalUser }); + } result = await driver.updateMany(object, ast, hookContext.input.data as Record, hookContext.input.options as any); } else { throw new Error('Update requires an ID or options.multi=true'); diff --git a/packages/objectql/src/plugin.integration.test.ts b/packages/objectql/src/plugin.integration.test.ts index 790f66ea41..4074c4257a 100644 --- a/packages/objectql/src/plugin.integration.test.ts +++ b/packages/objectql/src/plugin.integration.test.ts @@ -1451,4 +1451,110 @@ describe('ObjectQLPlugin - Metadata Service Integration', () => { expect(bulkUpdates[0].tax_rate).toBe(999); // legitimate bulk edit unaffected }); }); + + // #3106 — object-level validation rules, `requiredWhen` and per-option + // `visibleWhen` must be enforced on the BULK (updateMany) path too. The + // engine reads the matched rows once (shared with the #3042 strip) and + // evaluates the payload against each row's prior state. + describe('validation rule enforcement on multi-row UPDATE (#3106)', () => { + async function bootWithValidationCapture(priorRows: Record[], obj: ObjectSchema) { + const bulkUpdates: Record[] = []; + // Count only reads of the object under test — kernel bootstrap issues + // its own driver.find calls (sys_metadata restore, authored-hook resync). + let findCalls = 0; + const mockDriver = { + name: 'vr-capture', version: '1.0.0', + connect: async () => {}, disconnect: async () => {}, + find: async (o: string) => { if (o === obj.name) { findCalls += 1; return priorRows; } return []; }, + findOne: async () => null, + create: async (_o: string, d: any) => ({ id: 'rec-1', ...d }), + update: async (_o: string, _i: any, d: any) => ({ id: _i, ...d }), + updateMany: async (_o: string, _ast: any, d: any) => { bulkUpdates.push({ ...d }); return [{ ...d }]; }, + delete: async () => true, syncSchema: async () => {}, + }; + await kernel.use({ + name: 'vr-capture-plugin', type: 'driver', version: '1.0.0', + init: async (ctx) => { ctx.registerService('driver.vr-capture', mockDriver); }, + }); + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + const objectql = kernel.getService('objectql') as any; + objectql.registry.registerObject(obj, 'test', 'test'); + return { objectql, bulkUpdates, getFindCalls: () => findCalls }; + } + + it('rejects the whole batch on a prior-free format rule, with no row fetch', async () => { + const { objectql, bulkUpdates, getFindCalls } = await bootWithValidationCapture([], { + name: 'vr_acct', label: 'VR Acct', datasource: 'vr-capture', + fields: { email: { name: 'email', label: 'Email', type: 'text' } }, + validations: [{ type: 'format', name: 'email_format', message: 'email must be a valid email', field: 'email', format: 'email' }] as any, + }); + await expect(objectql.update( + 'vr_acct', + { email: 'not-an-email' }, + { multi: true, where: { status: 'active' }, context: { userId: 'user-9' } }, + )).rejects.toThrow(/valid email/); + expect(bulkUpdates.length).toBe(0); // nothing written + expect(getFindCalls()).toBe(0); // format needs no prior state + }); + + it('evaluates a state_machine rule per matched row: one illegal row rejects, all-legal proceeds', async () => { + const schema: ObjectSchema = { + name: 'vr_task', label: 'VR Task', datasource: 'vr-capture', + fields: { status: { name: 'status', label: 'Status', type: 'text' } }, + validations: [{ type: 'state_machine', name: 'status_flow', message: 'illegal status transition', field: 'status', transitions: { pending: ['in_flight'], done: ['archived'] } }] as any, + }; + const bad = await bootWithValidationCapture( + [{ id: 'a', status: 'pending' }, { id: 'b', status: 'done' }], + schema, + ); + await expect(bad.objectql.update( + 'vr_task', + { status: 'in_flight' }, + { multi: true, where: { status: { $in: ['pending', 'done'] } }, context: { userId: 'user-9' } }, + )).rejects.toThrow(/illegal status transition \(record b\)/); + expect(bad.bulkUpdates.length).toBe(0); + }); + + it('accepts the batch when every matched row transitions legally', async () => { + const schema: ObjectSchema = { + name: 'vr_task', label: 'VR Task', datasource: 'vr-capture', + fields: { status: { name: 'status', label: 'Status', type: 'text' } }, + validations: [{ type: 'state_machine', name: 'status_flow', message: 'illegal status transition', field: 'status', transitions: { pending: ['in_flight'], done: ['archived'] } }] as any, + }; + const ok = await bootWithValidationCapture( + [{ id: 'a', status: 'pending' }, { id: 'b', status: 'pending' }], + schema, + ); + await ok.objectql.update( + 'vr_task', + { status: 'in_flight' }, + { multi: true, where: { status: 'pending' }, context: { userId: 'user-9' } }, + ); + expect(ok.bulkUpdates.length).toBe(1); + expect(ok.bulkUpdates[0].status).toBe('in_flight'); + }); + + it('shares ONE row fetch between the readonlyWhen strip and per-row rule evaluation', async () => { + const { objectql, bulkUpdates, getFindCalls } = await bootWithValidationCapture( + [{ id: 'a', status: 'draft', amount: 10, limit: 100 }], + { + name: 'vr_invoice', label: 'VR Invoice', datasource: 'vr-capture', + fields: { + status: { name: 'status', label: 'Status', type: 'text' }, + amount: { name: 'amount', label: 'Amount', type: 'number', readonlyWhen: "record.status == 'paid'" } as any, + }, + validations: [{ type: 'cross_field', name: 'amount_cap', message: 'amount exceeds limit', condition: 'record.amount > record.limit', fields: ['amount'] }] as any, + }, + ); + await objectql.update( + 'vr_invoice', + { amount: 50 }, + { multi: true, where: { status: 'draft' }, context: { userId: 'user-9' } }, + ); + expect(getFindCalls()).toBe(1); // strip + validation share the read + expect(bulkUpdates.length).toBe(1); + expect(bulkUpdates[0].amount).toBe(50); // unlocked row: edit goes through + }); + }); }); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index 9326d1df18..ef0c72512e 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -34,8 +34,12 @@ * * Every variant declared by `ValidationRuleSchema` is enforced here — the * schema deliberately excludes anything that would need I/O or a handler model - * (uniqueness → DB index, async → form layer, custom → lifecycle hook), so - * there are no silent no-ops. + * (uniqueness → DB index, async → form layer, custom → lifecycle hook). The + * engine invokes this evaluator on insert, single-id update, and — per matched + * row — multi-row (`multi: true`) update (#3106), so no declared rule is a + * silent no-op on the write path. Known gap: rules declaring + * `events: ['delete']` are never evaluated (`engine.delete` does not call this + * evaluator; tracked separately — see below). * * ## Execution-control semantics (from `BaseValidationSchema`) * @@ -61,7 +65,10 @@ * the record's prior state. The engine fetches it once (see * `engine.update`) and threads it in via `opts.previous`. On `insert` there * is no prior state, so `state_machine` is a no-op (the field-level select - * check already constrains the initial value to a declared option). + * check already constrains the initial value to a declared option). On a + * multi-row update the engine reads the row-scoped match set (the same AST the + * write binds, shared with the `readonlyWhen` bulk strip) and calls the + * evaluator once per matched row — one payload, N priors (#3106). */ import { ExpressionEngine } from '@objectstack/formula'; diff --git a/packages/spec/src/data/validation.zod.ts b/packages/spec/src/data/validation.zod.ts index 0e799ca7da..649e0c2e20 100644 --- a/packages/spec/src/data/validation.zod.ts +++ b/packages/spec/src/data/validation.zod.ts @@ -15,7 +15,9 @@ import { ExpressionInputSchema } from '../shared/expression.zod'; * A validation rule is a **deterministic, synchronous, side-effect-free predicate over a single * record** — it must be decidable from the incoming write (and, on update, the prior record) with * no I/O. Everything advertised here runs on the write path (see - * `objectql/src/validation/rule-validator.ts`); nothing is a silent no-op. + * `objectql/src/validation/rule-validator.ts`) — insert, single-id update, and multi-row + * (`multi: true`) update, where the evaluator runs once per matched row (#3106). One known gap: + * rules declaring `events: ['delete']` are not yet evaluated on delete (tracked separately). * * The system supports these validation types: * From 605f296ba3df9aeda25d5436e51a68f6d433c1f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 03:08:49 +0000 Subject: [PATCH 2/2] docs(spec): regenerate validation reference for the updated module doc comment The validation.zod.ts module doc comment feeds the generated content/docs/references/data/validation.mdx; regenerate so `check:docs` passes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VCUSMJBsX14C3RQdWFw7N7 --- content/docs/references/data/validation.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/content/docs/references/data/validation.mdx b/content/docs/references/data/validation.mdx index 4b1c90eb24..ab0ceb2aef 100644 --- a/content/docs/references/data/validation.mdx +++ b/content/docs/references/data/validation.mdx @@ -21,7 +21,11 @@ record** — it must be decidable from the incoming write (and, on update, the p no I/O. Everything advertised here runs on the write path (see -`objectql/src/validation/rule-validator.ts`); nothing is a silent no-op. +`objectql/src/validation/rule-validator.ts`) — insert, single-id update, and multi-row + +(`multi: true`) update, where the evaluator runs once per matched row (#3106). One known gap: + +rules declaring `events: ['delete']` are not yet evaluated on delete (tracked separately). The system supports these validation types: