Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/bulk-update-validation-rules.md
Original file line number Diff line number Diff line change
@@ -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).
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-<task> -b <branch> main && cd ../framework-<task> && 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.

Expand Down
6 changes: 5 additions & 1 deletion content/docs/references/data/validation.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
144 changes: 144 additions & 0 deletions packages/objectql/src/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading