From 8a1e3f7d78296f7fafd03efd6df96fbe2fda1ba5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 06:51:54 +0000 Subject: [PATCH] fix(spec): trim dead 'delete' member from validation-rule events enum (#3184) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule evaluator only runs on the insert/update write path — engine.delete never invokes evaluateValidationRules — so a rule declaring events:['delete'] was a silent no-op (flagged in #3106 and the 2026-06 validation liveness audit, and actively taught by the objectstack-data skill). Guard deletions with a beforeDelete lifecycle hook instead. Narrows the enum and its hand-written mirrors in lockstep: the BaseValidationRuleShape type, objectql's BaseRule, and the metadata-protocol JSON-schema form helper (whose stale `type` enum still listing the removed unique/async/custom variants and `json`→`json_schema` is corrected in the same pass). Updates the module doc comments (delete now lives under "Deliberately NOT validation rules"), the published skill, the hand-written validation doc, and regenerates content/docs/references/data/validation.mdx. No shipped metadata declares events:['delete']; any off-spec metadata that did now fails loudly at os validate / registration instead of parsing and silently doing nothing (contract-first, PD #12). Adds a spec test asserting the rejection. Closes #3184 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VCUSMJBsX14C3RQdWFw7N7 --- .changeset/trim-validation-delete-event.md | 5 +++ content/docs/data-modeling/validation.mdx | 2 +- content/docs/references/data/validation.mdx | 38 ++++++++++------- docs/design/builder-ui.md | 2 +- packages/metadata-protocol/src/protocol.ts | 9 ++-- .../objectql/src/validation/rule-validator.ts | 9 ++-- packages/spec/src/data/validation.test.ts | 42 +++++++++++++++++++ packages/spec/src/data/validation.zod.ts | 15 ++++--- skills/objectstack-data/rules/validation.md | 5 ++- 9 files changed, 95 insertions(+), 32 deletions(-) create mode 100644 .changeset/trim-validation-delete-event.md diff --git a/.changeset/trim-validation-delete-event.md b/.changeset/trim-validation-delete-event.md new file mode 100644 index 0000000000..67dca51bcb --- /dev/null +++ b/.changeset/trim-validation-delete-event.md @@ -0,0 +1,5 @@ +--- +"@objectstack/spec": patch +--- + +Remove the dead `'delete'` member from the validation-rule `events` enum (#3184). The rule evaluator only runs on the insert/update write path — `engine.delete` never invokes it — so a rule declaring `events: ['delete']` was a silent no-op (flagged in #3106 and `docs/audits/2026-06-validationschema-property-liveness.md`). The enum now admits only `insert`/`update`; guard deletions with a `beforeDelete` lifecycle hook instead. No shipped metadata declares `events: ['delete']`; any off-spec metadata that did now fails loudly at `os validate` / registration rather than parsing and doing nothing. Also narrows the two hand-written mirrors (`rule-validator.ts` `BaseRule`, `metadata-protocol` JSON-schema form helper — whose stale `type` enum listing removed `unique`/`async`/`custom` variants is corrected in the same pass), updates the doc comments, the published data skill, and the hand-written validation doc. diff --git a/content/docs/data-modeling/validation.mdx b/content/docs/data-modeling/validation.mdx index 5ea64e470e..7c1d688c6e 100644 --- a/content/docs/data-modeling/validation.mdx +++ b/content/docs/data-modeling/validation.mdx @@ -69,7 +69,7 @@ All validation types share these base properties: | `active` | `boolean` | optional | Is the rule active (default: `true`) | | `severity` | `enum` | optional | `'error'` (blocks save), `'warning'`, `'info'` (default: `'error'`) | | `message` | `string` | ✅ | User-facing error message | -| `events` | `enum[]` | optional | When to run: `'insert'`, `'update'`, `'delete'` (default: `['insert', 'update']`) | +| `events` | `enum[]` | optional | When to run: `'insert'`, `'update'` (default: `['insert', 'update']`). No `'delete'` — guard deletions with a `beforeDelete` lifecycle hook | | `priority` | `number` | optional | Execution order (0-9999, lower runs first, default: 100) | | `tags` | `string[]` | optional | Categorization tags | diff --git a/content/docs/references/data/validation.mdx b/content/docs/references/data/validation.mdx index ab0ceb2aef..03bafdd28d 100644 --- a/content/docs/references/data/validation.mdx +++ b/content/docs/references/data/validation.mdx @@ -23,9 +23,11 @@ no I/O. Everything advertised here runs on the write path (see `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: +(`multi: true`) update, where the evaluator runs once per matched row (#3106); nothing is a -rules declaring `events: ['delete']` are not yet evaluated on delete (tracked separately). +silent no-op. The `events` enum admits only `insert`/`update` for this reason — see the + +`delete` note under "Deliberately NOT validation rules" below. The system supports these validation types: @@ -65,6 +67,14 @@ the form layer, or enforce the underlying invariant with a `unique` index / life extension point for arbitrary validation code. +- **Delete-time guards** (`events: ['delete']`) → a `beforeDelete` lifecycle hook. The evaluator + +only runs on the insert/update write path (a delete carries no record payload to validate), so + +a `delete` event was a proven silent no-op — the enum value was removed rather than left + +advertised-but-unenforced (#3184; see docs/audits/2026-06-validationschema-property-liveness.md). + ## Salesforce Comparison ObjectStack validation rules are inspired by Salesforce validation rules but enhanced: @@ -131,7 +141,7 @@ const result = ConditionalValidation.parse(data); | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -154,7 +164,7 @@ const result = ConditionalValidation.parse(data); | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -176,7 +186,7 @@ const result = ConditionalValidation.parse(data); | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | ✅ | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | ✅ | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | ✅ | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | ✅ | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | ✅ | | @@ -199,7 +209,7 @@ const result = ConditionalValidation.parse(data); | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | ✅ | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | ✅ | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | ✅ | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | ✅ | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | ✅ | | @@ -221,7 +231,7 @@ const result = ConditionalValidation.parse(data); | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -242,7 +252,7 @@ const result = ConditionalValidation.parse(data); | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | ✅ | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | ✅ | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | ✅ | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | ✅ | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | ✅ | | @@ -272,7 +282,7 @@ This schema accepts one of the following structures: | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -294,7 +304,7 @@ This schema accepts one of the following structures: | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -317,7 +327,7 @@ This schema accepts one of the following structures: | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -341,7 +351,7 @@ This schema accepts one of the following structures: | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -364,7 +374,7 @@ This schema accepts one of the following structures: | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | @@ -387,7 +397,7 @@ This schema accepts one of the following structures: | **label** | `string` | optional | Human-readable label for the rule listing | | **description** | `string` | optional | Administrative notes explaining the business reason | | **active** | `boolean` | optional | | -| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts | +| **events** | `Enum<'insert' \| 'update'>[]` | optional | Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook | | **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) | | **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") | | **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | | diff --git a/docs/design/builder-ui.md b/docs/design/builder-ui.md index 508d91a76e..ba8523aca8 100644 --- a/docs/design/builder-ui.md +++ b/docs/design/builder-ui.md @@ -224,7 +224,7 @@ field editor is shared by both Records (selected column) and Fields (selected fi union of six declarative rule types (`packages/spec/src/data/validation.zod.ts`): `script` (CEL predicate → fails when true), `cross_field`, `format`, `state_machine`, `json`, `conditional`. Each carries `message`, `severity` -(error/warning/info), and `events` (insert/update/delete). +(error/warning/info), and `events` (insert/update). - **Field-level** (required / unique / format) → configured in the **field editor** (they belong to the field). diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 296b94244e..97962ed01a 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -198,7 +198,7 @@ const HAND_CRAFTED_SCHEMAS: Record> = { additionalProperties: true, }, // Validation rules live inside `object.validations[]`. The canonical - // ValidationRuleSchema is a discriminated union of 9 variants; the + // ValidationRuleSchema is a discriminated union of 6 variants; the // generic SchemaForm renderer treats unions as opaque JSON, so we // ship a *flat* form-friendly schema covering the common base // properties plus every variant-specific field as optional. Save-time @@ -215,13 +215,10 @@ const HAND_CRAFTED_SCHEMAS: Record> = { type: 'string', enum: [ 'script', - 'unique', 'state_machine', 'format', 'cross_field', - 'json', - 'async', - 'custom', + 'json_schema', 'conditional', ], default: 'script', @@ -230,7 +227,7 @@ const HAND_CRAFTED_SCHEMAS: Record> = { active: { type: 'boolean', default: true }, events: { type: 'array', - items: { type: 'string', enum: ['insert', 'update', 'delete'] }, + items: { type: 'string', enum: ['insert', 'update'] }, default: ['insert', 'update'], }, priority: { type: 'number', default: 100, minimum: 0, maximum: 9999 }, diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index ef0c72512e..251d7d74d2 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -37,9 +37,10 @@ * (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). + * silent no-op on the write path. The `events` enum admits only `insert`/`update` + * to match: `delete` carries no record payload to validate, so it was removed + * from the spec rather than advertised-but-unenforced (#3184). Guard deletions + * with a `beforeDelete` lifecycle hook instead. * * ## Execution-control semantics (from `BaseValidationSchema`) * @@ -83,7 +84,7 @@ interface BaseRule { name: string; message: string; active?: boolean; - events?: Array<'insert' | 'update' | 'delete'>; + events?: Array<'insert' | 'update'>; priority?: number; severity?: 'error' | 'warning' | 'info'; } diff --git a/packages/spec/src/data/validation.test.ts b/packages/spec/src/data/validation.test.ts index 9d726c81ba..59c0097777 100644 --- a/packages/spec/src/data/validation.test.ts +++ b/packages/spec/src/data/validation.test.ts @@ -1116,3 +1116,45 @@ describe('ValidationRule - priority property', () => { })).toThrow(); }); }); + +describe('ValidationRule - events property (#3184)', () => { + it('should accept insert / update events', () => { + const rule = ScriptValidationSchema.parse({ + type: 'script', + name: 'on_write', + message: 'x', + condition: 'true', + events: ['insert', 'update'], + }); + expect(rule.events).toEqual(['insert', 'update']); + }); + + it('should default events to insert + update', () => { + const rule = ScriptValidationSchema.parse({ + type: 'script', + name: 'default_events', + message: 'x', + condition: 'true', + }); + expect(rule.events).toEqual(['insert', 'update']); + }); + + it("should reject events: ['delete'] — the evaluator never runs on delete; guard deletions with a beforeDelete hook", () => { + expect(() => ScriptValidationSchema.parse({ + type: 'script', + name: 'on_delete', + message: 'x', + condition: 'true', + events: ['delete'], + })).toThrow(); + + // also rejected via the discriminated-union entry point + expect(() => ValidationRuleSchema.parse({ + type: 'script', + name: 'on_delete_union', + message: 'x', + condition: 'true', + events: ['insert', 'delete'], + })).toThrow(); + }); +}); diff --git a/packages/spec/src/data/validation.zod.ts b/packages/spec/src/data/validation.zod.ts index 649e0c2e20..250e99c41d 100644 --- a/packages/spec/src/data/validation.zod.ts +++ b/packages/spec/src/data/validation.zod.ts @@ -16,8 +16,9 @@ import { ExpressionInputSchema } from '../shared/expression.zod'; * 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`) — 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). + * (`multi: true`) update, where the evaluator runs once per matched row (#3106); nothing is a + * silent no-op. The `events` enum admits only `insert`/`update` for this reason — see the + * `delete` note under "Deliberately NOT validation rules" below. * * The system supports these validation types: * @@ -42,7 +43,11 @@ import { ExpressionInputSchema } from '../shared/expression.zod'; * the form layer, or enforce the underlying invariant with a `unique` index / lifecycle hook. * - **Custom handler** → a `beforeInsert` / `beforeUpdate` lifecycle hook, the typed, supported * extension point for arbitrary validation code. - * + * - **Delete-time guards** (`events: ['delete']`) → a `beforeDelete` lifecycle hook. The evaluator + * only runs on the insert/update write path (a delete carries no record payload to validate), so + * a `delete` event was a proven silent no-op — the enum value was removed rather than left + * advertised-but-unenforced (#3184; see docs/audits/2026-06-validationschema-property-liveness.md). + * * ## Salesforce Comparison * * ObjectStack validation rules are inspired by Salesforce validation rules but enhanced: @@ -87,7 +92,7 @@ const BaseValidationSchema = z.object({ // Execution Control active: z.boolean().default(true), - events: z.array(z.enum(['insert', 'update', 'delete'])).default(['insert', 'update']).describe('Validation contexts'), + events: z.array(z.enum(['insert', 'update'])).default(['insert', 'update']).describe('Write contexts the rule runs on. `delete` is intentionally absent — the evaluator only runs on the insert/update write path; guard deletions with a `beforeDelete` lifecycle hook'), priority: z.number().int().min(0).max(9999).default(100).describe('Execution priority (lower runs first, default: 100)'), // Classification @@ -225,7 +230,7 @@ export interface BaseValidationRuleShape { label?: string; description?: string; active?: boolean; - events?: ('insert' | 'update' | 'delete')[]; + events?: ('insert' | 'update')[]; priority?: number; tags?: string[]; severity?: 'error' | 'warning' | 'info'; diff --git a/skills/objectstack-data/rules/validation.md b/skills/objectstack-data/rules/validation.md index 6204732cf2..85523797d6 100644 --- a/skills/objectstack-data/rules/validation.md +++ b/skills/objectstack-data/rules/validation.md @@ -265,9 +265,12 @@ severity: 'info' // Informational only events: ['insert'] // Only on create events: ['update'] // Only on update events: ['insert', 'update'] // On create and update (default) -events: ['delete'] // Only on delete ``` +> Validation rules run only on the insert/update write path — there is no `'delete'` +> event (a delete carries no record payload to validate). To block or guard a deletion, +> use a `beforeDelete` lifecycle hook instead (see `references/data-hooks.md`). + ### Priority ```typescript