Skip to content

Commit aebfa4f

Browse files
os-zhuangclaude
andauthored
chore(core): deprecate ObjectValidationEngine — enforcement stays single-implementation on the server (#3110) (#3111)
Validation rules are enforcement, and enforcement lives on the server (`objectql`'s rule-validator). objectui already draws that line for the predicates it DOES evaluate client-side: `evaluator/fieldRules.ts` handles the presentation predicates by delegating to the canonical `ExpressionEngine`, "rather than re-implementing a parallel evaluator" (ADR-0036). This engine was that parallel evaluator, on the enforcement side of the line. #3103 converged its semantics onto the server rule-for-rule with eight mutation-tested gates, and STILL left a known divergence: the server carries ADR-0113's legacy-violation exemption (reject only when the merged state violates AND this write worsens it); this engine does not. Editing an unrelated field on a legacy row would be blocked here and accepted there. One careful pass still left a gap — mirroring cross-repo behaviour is structurally unreliable, not unreliable-this-time. Nothing removed, no behaviour change. Callers keep working; the barrels keep re-exporting, because publishing a deprecated API is not wiring. The decision ships as a MECHANISM, not a comment. What #3103 deleted was a doc comment claiming spec canonicity that had been false for fifteen majors; shipping its successor as another comment would repeat the mistake one level up (#3017). `validation-engine-stays-unwired.test.ts` scans `packages/*/src` and fails if a production module references the engine, naming the file and the issue to reverse first. It strips comments and string bodies before matching, so prose that merely names the engine is not a wiring, and it self-checks that the scan reached a plausible tree — a guard that silently scans zero files is the objectstack#4115 failure mode. Three mutations verified it: a direct import, an indirect namespace access, and a broken scan path all turn it red. Pre-submit feedback moves to a server validate-only (dry-run) write — objectstack#4372 — which also covers `unique` and `json_schema`, the two rule kinds a client can never check. The five spec-derived rule TYPES in @object-ui/types are untouched: they anchor objectstack#4115's ledger and are independent of whether objectui ships an engine. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 912496d commit aebfa4f

5 files changed

Lines changed: 316 additions & 10 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@object-ui/core": patch
3+
---
4+
5+
chore(core): deprecate `ObjectValidationEngine` — rule enforcement stays single-implementation on the server (#3110)
6+
7+
`ObjectValidationEngine` / `defaultObjectValidationEngine` / `validateRecord` are
8+
now `@deprecated`. **Nothing is removed and no behaviour changes** — existing
9+
callers keep working exactly as they did after #3103.
10+
11+
**Why.** Object-level validation rules are *enforcement*, and enforcement is
12+
single-implementation on the server (`objectql`'s rule-validator). objectui
13+
already draws that line for the predicates it *does* evaluate client-side:
14+
`evaluator/fieldRules.ts` handles the presentation predicates (`visibleWhen` /
15+
`readonlyWhen` / `requiredWhen`) by delegating to the canonical
16+
`ExpressionEngine`, "rather than re-implementing a parallel evaluator"
17+
(ADR-0036). This engine was that parallel evaluator, on the enforcement side.
18+
19+
#3103 converged its semantics onto the server rule-for-rule, with eight
20+
mutation-tested gates — and still left a known divergence: the server carries
21+
ADR-0113's legacy-violation exemption (reject only when the merged state violates
22+
*and* this write makes it worse), which this engine does not implement. Editing
23+
an unrelated field on a legacy row would be blocked here and accepted there. One
24+
careful pass still left a gap, which is the argument: mirroring cross-repo
25+
behaviour is structurally unreliable, not unreliable-this-time.
26+
27+
**What to use instead.** Let the write fail and render the server's rejection —
28+
it is already structured (`field` / `code` / `message`, plus a label since
29+
objectstack#3957). For pre-submit feedback, the answer is a validate-only
30+
(dry-run) write on the server: identical UX, zero parity risk, and it covers the
31+
two rule kinds a client can never check — `unique` (needs the database) and
32+
`json_schema` (ajv lives server-side).
33+
34+
**The decision is a mechanism, not a comment.** What #3103 removed was a doc
35+
comment claiming spec canonicity that had been false for fifteen majors;
36+
shipping its successor as another comment would repeat the mistake one level up.
37+
`validation-engine-stays-unwired.test.ts` scans `packages/*/src` and fails if a
38+
production module starts referencing the engine, naming the file and the issue to
39+
reverse first. Barrels still re-export it — publishing a deprecated API is not
40+
wiring — and host applications are free to keep importing it.
41+
42+
The five spec-derived rule TYPES in `@object-ui/types` are unaffected: they are
43+
the anchor for objectstack#4115's ledger and are independent of whether objectui
44+
ships an engine.
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `ObjectValidationEngine` stays unwired — the mechanism behind #3110.
11+
*
12+
* The decision was: validation rules are ENFORCEMENT, enforcement is
13+
* single-implementation on the server, and objectui renders the server's
14+
* rejection rather than predicting it. `evaluator/fieldRules.ts` draws the same
15+
* line for the presentation predicates it DOES evaluate client-side — it
16+
* delegates to the canonical `ExpressionEngine` "rather than re-implementing a
17+
* parallel evaluator" (ADR-0036). This engine is that parallel evaluator.
18+
*
19+
* #3103 converged it onto the server rule-for-rule and still left a known
20+
* divergence (ADR-0113's legacy-violation exemption), which is the whole
21+
* argument: mirroring cross-repo behaviour is structurally unreliable.
22+
*
23+
* Why a test and not just the `@deprecated` tag: the thing this replaced was a
24+
* doc comment claiming spec canonicity that had been false for fifteen majors
25+
* (#3103). Shipping the successor decision as another comment would repeat the
26+
* mistake one level up — a rule that is not a check is not a rule (#3017,
27+
* objectstack#4115). So the ban is compiled here: wire the engine into a
28+
* production module and this file names the file, the line, and the issue to
29+
* reverse first.
30+
*
31+
* This is deliberately a REACHABILITY ban, not a mention ban. Nothing stops a
32+
* host application from importing the deprecated engine — it is still exported,
33+
* still functional. What is banned is objectui itself growing a client-side
34+
* enforcement path again.
35+
*/
36+
37+
import { describe, it, expect } from 'vitest';
38+
import { readdirSync, readFileSync, statSync } from 'node:fs';
39+
import { join, resolve, relative } from 'node:path';
40+
41+
/** `packages/` — three levels up from `packages/core/src/validation/__tests__`. */
42+
const PACKAGES_DIR = resolve(__dirname, '../../../..');
43+
44+
/**
45+
* The engine's own module, its barrels, and its tests. A barrel re-export is not
46+
* a wiring: it publishes the deprecated API for host applications, which is
47+
* exactly what "deprecated but not removed" means.
48+
*/
49+
const ALLOWED = [
50+
'core/src/validation/validators/object-validation-engine.ts',
51+
'core/src/validation/validators/index.ts',
52+
'core/src/validation/index.ts',
53+
];
54+
55+
/** The engine's module path, and the entry points a caller would name. */
56+
const ENGINE_REFERENCE =
57+
/object-validation-engine|ObjectValidationEngine|defaultObjectValidationEngine/;
58+
59+
/**
60+
* Strip comments and string bodies before looking for a reference.
61+
*
62+
* Prose is not a wiring: `@object-ui/types` names the engine in a doc comment
63+
* explaining which defaults it applies, and that must not read as a production
64+
* import. Scanning the CODE (rather than the import statements alone) still
65+
* catches the indirect route — `import * as core` followed by
66+
* `core.ObjectValidationEngine` — which an import-list regex would miss.
67+
*
68+
* String bodies go too, so a URL or an error message quoting the name is prose
69+
* as well. Template-literal substitutions are kept, since that is real code.
70+
*/
71+
function stripCommentsAndStrings(src: string): string {
72+
let out = '';
73+
let i = 0;
74+
while (i < src.length) {
75+
const two = src.slice(i, i + 2);
76+
if (two === '//') {
77+
while (i < src.length && src[i] !== '\n') i++;
78+
continue;
79+
}
80+
if (two === '/*') {
81+
i += 2;
82+
while (i < src.length && src.slice(i, i + 2) !== '*/') i++;
83+
i += 2;
84+
continue;
85+
}
86+
const ch = src[i];
87+
if (ch === '"' || ch === "'" || ch === '`') {
88+
const quote = ch;
89+
i++;
90+
while (i < src.length) {
91+
if (src[i] === '\\') {
92+
i += 2;
93+
continue;
94+
}
95+
// Keep `${…}` substitutions — they are code, not prose.
96+
if (quote === '`' && src.slice(i, i + 2) === '${') {
97+
let depth = 1;
98+
i += 2;
99+
const start = i;
100+
while (i < src.length && depth > 0) {
101+
if (src[i] === '{') depth++;
102+
else if (src[i] === '}') depth--;
103+
if (depth > 0) i++;
104+
}
105+
out += src.slice(start, i);
106+
i++;
107+
continue;
108+
}
109+
if (src[i] === quote) break;
110+
i++;
111+
}
112+
i++;
113+
// A module specifier is a string, so preserve the ones that name the
114+
// engine's module — an `import … from '…/object-validation-engine'` must
115+
// still be caught.
116+
continue;
117+
}
118+
out += ch;
119+
i++;
120+
}
121+
return out;
122+
}
123+
124+
/** Module specifiers are strings, so they are checked separately. */
125+
const IMPORT_SPECIFIER = /(?:from|import|require)\s*\(?\s*['"]([^'"]+)['"]/g;
126+
127+
function referencesEngine(src: string): boolean {
128+
if (ENGINE_REFERENCE.test(stripCommentsAndStrings(src))) return true;
129+
for (const [, specifier] of src.matchAll(IMPORT_SPECIFIER)) {
130+
if (specifier.includes('object-validation-engine')) return true;
131+
}
132+
return false;
133+
}
134+
135+
function sourceFiles(): string[] {
136+
const out: string[] = [];
137+
const walk = (dir: string) => {
138+
for (const entry of readdirSync(dir, { withFileTypes: true })) {
139+
const full = join(dir, entry.name);
140+
if (entry.isDirectory()) {
141+
// Tests are not a production path, and `dist` is build output.
142+
if (['node_modules', 'dist', '__tests__'].includes(entry.name)) continue;
143+
walk(full);
144+
continue;
145+
}
146+
if (!/\.tsx?$/.test(entry.name) || /\.d\.ts$/.test(entry.name)) continue;
147+
if (/\.(test|stories)\.tsx?$/.test(entry.name)) continue;
148+
out.push(full);
149+
}
150+
};
151+
152+
for (const pkg of readdirSync(PACKAGES_DIR)) {
153+
const src = join(PACKAGES_DIR, pkg, 'src');
154+
try {
155+
if (!statSync(src).isDirectory()) continue;
156+
} catch {
157+
continue;
158+
}
159+
walk(src);
160+
}
161+
return out;
162+
}
163+
164+
describe('the deprecated ObjectValidationEngine stays out of production paths (#3110)', () => {
165+
it('scans a plausible source tree — so a passing result means something', () => {
166+
// Without this, a broken path silently scans zero files and the ban above
167+
// reports success forever. That failure mode is the one objectstack#4115
168+
// was filed about, so the guard guards itself.
169+
const files = sourceFiles();
170+
expect(files.length).toBeGreaterThan(500);
171+
expect(
172+
files.some((f) => f.endsWith('core/src/validation/validators/object-validation-engine.ts'))
173+
).toBe(true);
174+
});
175+
176+
it('is referenced only by its own module and the barrels that publish it', () => {
177+
const offenders = sourceFiles()
178+
.filter((file) => referencesEngine(readFileSync(file, 'utf8')))
179+
.map((file) => relative(PACKAGES_DIR, file))
180+
.filter((rel) => !ALLOWED.includes(rel))
181+
.sort();
182+
183+
expect(
184+
offenders,
185+
offenders.length === 0
186+
? ''
187+
: `A production module now references the deprecated ObjectValidationEngine:\n` +
188+
offenders.map((o) => ` - packages/${o}`).join('\n') +
189+
`\n\n` +
190+
`#3110 decided objectui does NOT run object-level validation rules on the\n` +
191+
`client: enforcement is single-implementation on the server, and mirroring\n` +
192+
`it drifts (see ADR-0113's legacy-violation exemption, still divergent).\n\n` +
193+
`For pre-submit feedback, use a validate-only (dry-run) write instead — it\n` +
194+
`also covers \`unique\` and \`json_schema\`, which a client cannot check.\n\n` +
195+
`If you are deliberately reversing the decision (e.g. offline writes),\n` +
196+
`reverse it in #3110 first, land the spec conformance fixtures it names,\n` +
197+
`then update ALLOWED here with the reason.`
198+
).toEqual([]);
199+
});
200+
});

packages/core/src/validation/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@
22
* @object-ui/core - Validation Module
33
*
44
* Phase 3.5: Validation engine
5+
*
56
* Object-level validation. The rule vocabulary is owned by
67
* `@objectstack/spec/data` and derived in `@object-ui/types`; canonicity is
78
* carried by that derivation and its parity gate, not by this comment.
9+
*
10+
* The object-level rule ENGINE (`validators/`) is @deprecated (#3110) — the
11+
* server is the single implementation of rule enforcement. Field/record SHAPE
12+
* validation (`schema-validator.js`, `validation-engine.js`) is unaffected: it
13+
* is client-side form validation, not a mirror of a server rule.
814
*/
915

1016
export * from './validation-engine.js';

packages/core/src/validation/validators/index.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,15 @@
88

99
/**
1010
* @object-ui/core - Validators
11-
*
12-
* Validators for the spec-owned object-level rule vocabulary.
13-
*
11+
*
12+
* Publishes the @deprecated `ObjectValidationEngine` (#3110). Object-level
13+
* validation rules are enforcement, and enforcement is single-implementation on
14+
* the server (`objectql`'s rule-validator); objectui renders the server's
15+
* rejection rather than predicting it, and pre-submit feedback belongs in a
16+
* validate-only (dry-run) write. This barrel keeps the API exported for host
17+
* applications that already depend on it — re-exporting is not wiring, and
18+
* `validation-engine-stays-unwired.test.ts` enforces that distinction.
19+
*
1420
* @module validators
1521
* @packageDocumentation
1622
*/

packages/core/src/validation/validators/object-validation-engine.ts

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,55 @@
77
*/
88

99
/**
10-
* @object-ui/core - Object-Level Validation Engine
10+
* @object-ui/core - Object-Level Validation Engine — **DEPRECATED** (#3110)
1111
*
12-
* A CLIENT-SIDE PRE-CHECK of the rules an object declares in
12+
* A client-side pre-check of the rules an object declares in
1313
* `ObjectSchema.validations`. The authority is the server
1414
* (`objectql/src/validation/rule-validator.ts`), which evaluates the same rules
15-
* on the write path; this engine exists to surface a violation before the round
16-
* trip, never to invent one. That framing sets every semantic below — a
17-
* pre-check that disagrees with the server is worse than no pre-check, because
18-
* it either blocks writes the server would accept or greenlights ones it
19-
* rejects.
15+
* on the write path.
16+
*
17+
* ## Why this is deprecated rather than finished
18+
*
19+
* Validation rules are ENFORCEMENT, and enforcement is single-implementation on
20+
* the server by design. `evaluator/fieldRules.ts` draws that line explicitly:
21+
* PRESENTATION predicates (`visibleWhen` / `readonlyWhen` / `requiredWhen`) are
22+
* evaluated client-side by delegating to the canonical `ExpressionEngine`,
23+
* "rather than re-implementing a parallel evaluator" (ADR-0036). This module is
24+
* that parallel evaluator, on the enforcement side of the line.
25+
*
26+
* #3103 converged its semantics onto the server rule-for-rule, with eight
27+
* mutation-tested gates — and it STILL left a known divergence: the server
28+
* carries ADR-0113's legacy-violation exemption (reject only when the merged
29+
* state violates AND this write makes it worse, so a pre-existing violation on
30+
* an old row passes), and this engine does not. Editing an unrelated field on a
31+
* legacy row would be blocked here and accepted there. That is the argument in
32+
* one line: mirroring cross-repo behaviour is structurally unreliable, not
33+
* unreliable-this-time.
34+
*
35+
* ## What to use instead
36+
*
37+
* Let the write fail and render the server's rejection — it is already
38+
* structured (`field` / `code` / `message`, plus a label since objectstack#3957).
39+
* If pre-submit feedback is wanted, the answer is a **validate-only (dry-run)
40+
* write** on the server: identical UX, zero parity risk, and it also covers the
41+
* two rule kinds a client can never check — `unique` (needs the database) and
42+
* `json_schema` (ajv lives server-side).
43+
*
44+
* Nothing here has been removed or changed in behaviour; existing callers keep
45+
* working. `validation-engine-stays-unwired.test.ts` fails if a production
46+
* module starts importing this one, so the decision is a mechanism rather than
47+
* this comment (#3017).
48+
*
49+
* ## When to revisit
50+
*
51+
* Offline writes (`OfflineConfig`). With no server to ask, a client evaluator
52+
* stops being redundant and becomes necessary. The precondition then is
53+
* conformance fixtures shipped by `@objectstack/spec` — golden
54+
* (rule, record, verdict) triples both repos run in CI — because that is the
55+
* only thing that turns cross-repo parity into a mechanism instead of a habit.
56+
* Reverse the decision in #3110 before reversing it here.
57+
*
58+
* ## Semantics (unchanged, for as long as this exists)
2059
*
2160
* The rule types come from `@object-ui/types`, which derives them from
2261
* `@objectstack/spec/data` (objectstack#4115). Semantics mirrored from the
@@ -340,6 +379,13 @@ class SimpleExpressionEvaluator implements ValidationExpressionEvaluator {
340379
/**
341380
* Object-Level Validation Engine — the client pre-check described at the top of
342381
* this module.
382+
*
383+
* @deprecated (#3110) Validation rules are enforcement, and enforcement is
384+
* single-implementation on the server (`objectql`'s rule-validator). Render the
385+
* server's rejection instead; for pre-submit feedback, use a validate-only
386+
* (dry-run) write. Still functional and still correct as of #3103 — but it
387+
* cannot stay in step with the server without a conformance-fixture mechanism
388+
* that does not exist yet. See the module header.
343389
*/
344390
export class ObjectValidationEngine {
345391
private expressionEvaluator: ValidationExpressionEvaluator;
@@ -872,11 +918,15 @@ function matchesNamedFormat(format: FormatValidation['format'], str: string): bo
872918

873919
/**
874920
* Default instance
921+
*
922+
* @deprecated (#3110) See `ObjectValidationEngine`.
875923
*/
876924
export const defaultObjectValidationEngine = new ObjectValidationEngine();
877925

878926
/**
879927
* Convenience function to validate a record
928+
*
929+
* @deprecated (#3110) See `ObjectValidationEngine`.
880930
*/
881931
export async function validateRecord(
882932
rules: ObjectValidationRule[],

0 commit comments

Comments
 (0)