Skip to content

Commit f16b492

Browse files
os-zhuangclaude
andauthored
fix(spec): trim dead 'delete' member from validation-rule events enum (#3189)
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 Claude-Session: https://claude.ai/code/session_01VCUSMJBsX14C3RQdWFw7N7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent b659111 commit f16b492

9 files changed

Lines changed: 95 additions & 32 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
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.

content/docs/data-modeling/validation.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ All validation types share these base properties:
6969
| `active` | `boolean` | optional | Is the rule active (default: `true`) |
7070
| `severity` | `enum` | optional | `'error'` (blocks save), `'warning'`, `'info'` (default: `'error'`) |
7171
| `message` | `string` || User-facing error message |
72-
| `events` | `enum[]` | optional | When to run: `'insert'`, `'update'`, `'delete'` (default: `['insert', 'update']`) |
72+
| `events` | `enum[]` | optional | When to run: `'insert'`, `'update'` (default: `['insert', 'update']`). No `'delete'` — guard deletions with a `beforeDelete` lifecycle hook |
7373
| `priority` | `number` | optional | Execution order (0-9999, lower runs first, default: 100) |
7474
| `tags` | `string[]` | optional | Categorization tags |
7575

content/docs/references/data/validation.mdx

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@ no I/O. Everything advertised here runs on the write path (see
2323

2424
`objectql/src/validation/rule-validator.ts`) — insert, single-id update, and multi-row
2525

26-
(`multi: true`) update, where the evaluator runs once per matched row (#3106). One known gap:
26+
(`multi: true`) update, where the evaluator runs once per matched row (#3106); nothing is a
2727

28-
rules declaring `events: ['delete']` are not yet evaluated on delete (tracked separately).
28+
silent no-op. The `events` enum admits only `insert`/`update` for this reason — see the
29+
30+
`delete` note under "Deliberately NOT validation rules" below.
2931

3032
The system supports these validation types:
3133

@@ -65,6 +67,14 @@ the form layer, or enforce the underlying invariant with a `unique` index / life
6567

6668
extension point for arbitrary validation code.
6769

70+
- **Delete-time guards** (`events: ['delete']`) → a `beforeDelete` lifecycle hook. The evaluator
71+
72+
only runs on the insert/update write path (a delete carries no record payload to validate), so
73+
74+
a `delete` event was a proven silent no-op — the enum value was removed rather than left
75+
76+
advertised-but-unenforced (#3184; see docs/audits/2026-06-validationschema-property-liveness.md).
77+
6878
## Salesforce Comparison
6979

7080
ObjectStack validation rules are inspired by Salesforce validation rules but enhanced:
@@ -131,7 +141,7 @@ const result = ConditionalValidation.parse(data);
131141
| **label** | `string` | optional | Human-readable label for the rule listing |
132142
| **description** | `string` | optional | Administrative notes explaining the business reason |
133143
| **active** | `boolean` | optional | |
134-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
144+
| **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 |
135145
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
136146
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
137147
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -154,7 +164,7 @@ const result = ConditionalValidation.parse(data);
154164
| **label** | `string` | optional | Human-readable label for the rule listing |
155165
| **description** | `string` | optional | Administrative notes explaining the business reason |
156166
| **active** | `boolean` | optional | |
157-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
167+
| **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 |
158168
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
159169
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
160170
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -176,7 +186,7 @@ const result = ConditionalValidation.parse(data);
176186
| **label** | `string` | optional | Human-readable label for the rule listing |
177187
| **description** | `string` | optional | Administrative notes explaining the business reason |
178188
| **active** | `boolean` || |
179-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` || Validation contexts |
189+
| **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 |
180190
| **priority** | `integer` || Execution priority (lower runs first, default: 100) |
181191
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
182192
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` || |
@@ -199,7 +209,7 @@ const result = ConditionalValidation.parse(data);
199209
| **label** | `string` | optional | Human-readable label for the rule listing |
200210
| **description** | `string` | optional | Administrative notes explaining the business reason |
201211
| **active** | `boolean` || |
202-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` || Validation contexts |
212+
| **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 |
203213
| **priority** | `integer` || Execution priority (lower runs first, default: 100) |
204214
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
205215
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` || |
@@ -221,7 +231,7 @@ const result = ConditionalValidation.parse(data);
221231
| **label** | `string` | optional | Human-readable label for the rule listing |
222232
| **description** | `string` | optional | Administrative notes explaining the business reason |
223233
| **active** | `boolean` | optional | |
224-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
234+
| **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 |
225235
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
226236
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
227237
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -242,7 +252,7 @@ const result = ConditionalValidation.parse(data);
242252
| **label** | `string` | optional | Human-readable label for the rule listing |
243253
| **description** | `string` | optional | Administrative notes explaining the business reason |
244254
| **active** | `boolean` || |
245-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` || Validation contexts |
255+
| **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 |
246256
| **priority** | `integer` || Execution priority (lower runs first, default: 100) |
247257
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
248258
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` || |
@@ -273,7 +283,7 @@ This schema accepts one of the following structures:
273283
| **label** | `string` | optional | Human-readable label for the rule listing |
274284
| **description** | `string` | optional | Administrative notes explaining the business reason |
275285
| **active** | `boolean` | optional | |
276-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
286+
| **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 |
277287
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
278288
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
279289
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -295,7 +305,7 @@ This schema accepts one of the following structures:
295305
| **label** | `string` | optional | Human-readable label for the rule listing |
296306
| **description** | `string` | optional | Administrative notes explaining the business reason |
297307
| **active** | `boolean` | optional | |
298-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
308+
| **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 |
299309
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
300310
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
301311
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -319,7 +329,7 @@ This schema accepts one of the following structures:
319329
| **label** | `string` | optional | Human-readable label for the rule listing |
320330
| **description** | `string` | optional | Administrative notes explaining the business reason |
321331
| **active** | `boolean` | optional | |
322-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
332+
| **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 |
323333
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
324334
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
325335
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -343,7 +353,7 @@ This schema accepts one of the following structures:
343353
| **label** | `string` | optional | Human-readable label for the rule listing |
344354
| **description** | `string` | optional | Administrative notes explaining the business reason |
345355
| **active** | `boolean` | optional | |
346-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
356+
| **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 |
347357
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
348358
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
349359
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -366,7 +376,7 @@ This schema accepts one of the following structures:
366376
| **label** | `string` | optional | Human-readable label for the rule listing |
367377
| **description** | `string` | optional | Administrative notes explaining the business reason |
368378
| **active** | `boolean` | optional | |
369-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
379+
| **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 |
370380
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
371381
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
372382
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |
@@ -389,7 +399,7 @@ This schema accepts one of the following structures:
389399
| **label** | `string` | optional | Human-readable label for the rule listing |
390400
| **description** | `string` | optional | Administrative notes explaining the business reason |
391401
| **active** | `boolean` | optional | |
392-
| **events** | `Enum<'insert' \| 'update' \| 'delete'>[]` | optional | Validation contexts |
402+
| **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 |
393403
| **priority** | `integer` | optional | Execution priority (lower runs first, default: 100) |
394404
| **tags** | `string[]` | optional | Categorization tags (e.g., "compliance", "billing") |
395405
| **severity** | `Enum<'error' \| 'warning' \| 'info'>` | optional | |

docs/design/builder-ui.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ field editor is shared by both Records (selected column) and Fields (selected fi
224224
union of six declarative rule types (`packages/spec/src/data/validation.zod.ts`):
225225
`script` (CEL predicate → fails when true), `cross_field`, `format`,
226226
`state_machine`, `json`, `conditional`. Each carries `message`, `severity`
227-
(error/warning/info), and `events` (insert/update/delete).
227+
(error/warning/info), and `events` (insert/update).
228228

229229
- **Field-level** (required / unique / format) → configured in the **field editor**
230230
(they belong to the field).

packages/metadata-protocol/src/protocol.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ const HAND_CRAFTED_SCHEMAS: Record<string, Record<string, unknown>> = {
198198
additionalProperties: true,
199199
},
200200
// Validation rules live inside `object.validations[]`. The canonical
201-
// ValidationRuleSchema is a discriminated union of 9 variants; the
201+
// ValidationRuleSchema is a discriminated union of 6 variants; the
202202
// generic SchemaForm renderer treats unions as opaque JSON, so we
203203
// ship a *flat* form-friendly schema covering the common base
204204
// properties plus every variant-specific field as optional. Save-time
@@ -215,13 +215,10 @@ const HAND_CRAFTED_SCHEMAS: Record<string, Record<string, unknown>> = {
215215
type: 'string',
216216
enum: [
217217
'script',
218-
'unique',
219218
'state_machine',
220219
'format',
221220
'cross_field',
222-
'json',
223-
'async',
224-
'custom',
221+
'json_schema',
225222
'conditional',
226223
],
227224
default: 'script',
@@ -230,7 +227,7 @@ const HAND_CRAFTED_SCHEMAS: Record<string, Record<string, unknown>> = {
230227
active: { type: 'boolean', default: true },
231228
events: {
232229
type: 'array',
233-
items: { type: 'string', enum: ['insert', 'update', 'delete'] },
230+
items: { type: 'string', enum: ['insert', 'update'] },
234231
default: ['insert', 'update'],
235232
},
236233
priority: { type: 'number', default: 100, minimum: 0, maximum: 9999 },

packages/objectql/src/validation/rule-validator.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,10 @@
3939
* (uniqueness → DB index, async → form layer, custom → lifecycle hook). The
4040
* engine invokes this evaluator on insert, single-id update, and — per matched
4141
* row — multi-row (`multi: true`) update (#3106), so no declared rule is a
42-
* silent no-op on the write path. Known gap: rules declaring
43-
* `events: ['delete']` are never evaluated (`engine.delete` does not call this
44-
* evaluator; tracked separately — see below).
42+
* silent no-op on the write path. The `events` enum admits only `insert`/`update`
43+
* to match: `delete` carries no record payload to validate, so it was removed
44+
* from the spec rather than advertised-but-unenforced (#3184). Guard deletions
45+
* with a `beforeDelete` lifecycle hook instead.
4546
*
4647
* ## Execution-control semantics (from `BaseValidationSchema`)
4748
*
@@ -86,7 +87,7 @@ interface BaseRule {
8687
name: string;
8788
message: string;
8889
active?: boolean;
89-
events?: Array<'insert' | 'update' | 'delete'>;
90+
events?: Array<'insert' | 'update'>;
9091
priority?: number;
9192
severity?: 'error' | 'warning' | 'info';
9293
}

packages/spec/src/data/validation.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,3 +1116,45 @@ describe('ValidationRule - priority property', () => {
11161116
})).toThrow();
11171117
});
11181118
});
1119+
1120+
describe('ValidationRule - events property (#3184)', () => {
1121+
it('should accept insert / update events', () => {
1122+
const rule = ScriptValidationSchema.parse({
1123+
type: 'script',
1124+
name: 'on_write',
1125+
message: 'x',
1126+
condition: 'true',
1127+
events: ['insert', 'update'],
1128+
});
1129+
expect(rule.events).toEqual(['insert', 'update']);
1130+
});
1131+
1132+
it('should default events to insert + update', () => {
1133+
const rule = ScriptValidationSchema.parse({
1134+
type: 'script',
1135+
name: 'default_events',
1136+
message: 'x',
1137+
condition: 'true',
1138+
});
1139+
expect(rule.events).toEqual(['insert', 'update']);
1140+
});
1141+
1142+
it("should reject events: ['delete'] — the evaluator never runs on delete; guard deletions with a beforeDelete hook", () => {
1143+
expect(() => ScriptValidationSchema.parse({
1144+
type: 'script',
1145+
name: 'on_delete',
1146+
message: 'x',
1147+
condition: 'true',
1148+
events: ['delete'],
1149+
})).toThrow();
1150+
1151+
// also rejected via the discriminated-union entry point
1152+
expect(() => ValidationRuleSchema.parse({
1153+
type: 'script',
1154+
name: 'on_delete_union',
1155+
message: 'x',
1156+
condition: 'true',
1157+
events: ['insert', 'delete'],
1158+
})).toThrow();
1159+
});
1160+
});

0 commit comments

Comments
 (0)