diff --git a/.changeset/24720-validation-rule-parent.md b/.changeset/24720-validation-rule-parent.md new file mode 100644 index 00000000000..2efe9027f54 --- /dev/null +++ b/.changeset/24720-validation-rule-parent.md @@ -0,0 +1,22 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): `os validate`, `os build` and the object save door warn on a `validations[]` rule whose predicate reads `parent` (#24720) + +Clause-②: no + +A `validations[]` rule's predicates — a `script` or `cross_field` rule's `condition`, a `conditional` rule's `when`, and the same slots on the rules nested in its `then` / `otherwise` — are evaluated by the server against `record` and `previous` only. The master-detail header is bound as `parent` for a field's own `readonlyWhen` and `requiredWhen` alone, on an object with exactly one `master_detail`. A rule that read `parent`, such as `parent.status == 'paid'`, passed `os validate` silently, because `parent` is a root the platform declares. At runtime the predicate faulted (`Unknown variable: parent`) and every write the rule judged was refused as unevaluable, the seed's own rows at boot included. + +The build's expression rule (`validateStackExpressions`) now reports such a predicate at `warning`, located where the rule's other findings are (`object 'NAME' · validation 'RULE'`, `… when`, or the nested rule's label). The message names the slot, the rule and the root. The object save door runs the same pass and lists the same finding among its advisories: the same rule id (`expression-invalid`), location and message. + +**Nothing new is refused in 18.x.** A warning fails neither `os build` nor `os validate`, and a publish-mode object save that carries such a rule is still admitted. No ADR-0087 conversion can rewrite a predicate that reads `parent`, so under the post-GA rule for such narrowings only this warning ships in 18.x. The refusal, the same finding at `error`, is planned for 19.0. It will also cover a rule marked `active: false`, a rule whose `events` a stack never fires, and a `conditional` branch whose `when` is never true, because the verdict judges the rule as written. + +**Remedy.** To lock or require a field on the header's state, write the predicate on that field's own `readonlyWhen` or `requiredWhen`, where `parent` is bound. For any other check, read a column the detail object declares, and denormalise the header value you need onto the detail. + +**Unchanged.** + +- The runtime is unchanged: a `validations[]` rule still binds `record` and `previous` and no `parent`. If the runtime comes to bind `parent` there, this verdict is withdrawn in that same change. +- `parent` on a field's own `readonlyWhen` / `requiredWhen` is still accepted with no finding, as before, and so is a `record` column spelled like the root (`record.parent_code`). +- No example or package object in this repository authors `parent` in a `validations[]` rule. +- No public export or signature moves. diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 2f0c605228c..eff83c96b2e 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -552,6 +552,10 @@ interface RulePredicate { raw: unknown; /** 0 for the rule itself; 1 inside its `then` / `otherwise`, and so on down. */ depth: number; + /** The key the predicate is written under on its own rule. */ + slot: 'condition' | 'when'; + /** The name of the rule that carries it (the nested rule's own, below depth 0). */ + ruleName: string; } /** @@ -572,9 +576,9 @@ function rulePredicates(rule: AnyRec, path: string, depth = 0): RulePredicate[] // author who wrote both `condition` and a rejected alias had their canonical // predicate short-circuited away and the alias validated instead (#5017). const main = rule.condition; - if (main != null) out.push({ label: `validation rule ${here}`, raw: main, depth }); + if (main != null) out.push({ label: `validation rule ${here}`, raw: main, depth, slot: 'condition', ruleName: name }); if (rule.when != null) { - out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, depth }); + out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, depth, slot: 'when', ruleName: name }); } for (const branch of ['then', 'otherwise'] as const) { const nested = rule[branch]; @@ -1710,6 +1714,73 @@ function optionVisibleWhenUserMemberIssue( }; } +/** + * [#24720] The root verdict for a `validations[]` rule's own predicates — a + * `script` / `cross_field` rule's `condition`, a `conditional` rule's `when`, + * and the same two slots on every rule nested in its `then` / `otherwise`: a + * predicate that reads `parent` is reported, at `warning` (18.x). + * + * ## Why it is reported + * + * ObjectQL evaluates every one of those predicates in `ruleScope(record, + * previous)` (`rule-validator.ts`: `checkPredicate`, `checkConditional`), which + * binds `record` and `previous` and no master-detail header. The header is + * bound as `parent` for a field's own `readonlyWhen` / `requiredWhen` only + * ({@link FIELD_RULE_BOUND_ROOTS}), on an object with exactly one + * `master_detail`. So the predicate faults (`Unknown variable: parent`) on + * every write it judges, and an unevaluable rule fails closed: the write is + * refused, the seed's own rows at boot included. Nothing else named it, + * because `parent` is in `SCOPE_ROOTS`, which the strict env declares, so the + * bare-reference check stays silent. The field-rule slots one level up accept + * the same text, which is what makes it the shape an author copies. + * + * ## Why a warning, and when it becomes a refusal + * + * A rule the runtime cannot evaluate must not pass a door (ADR-0049), so the + * end state is `error`. But refusing it narrows what `os build` and the object + * save door accept (a rule marked `active: false`, or a `conditional` branch + * whose `when` is never true, builds and writes today), and no ADR-0087 + * conversion can rewrite a predicate that reads `parent`. Under the standing + * post-GA ruling (甲), a narrowing no conversion carries ships only its loud + * half in 18.x: this verdict reports at `warning`, which neither fails + * `os build` nor refuses a save (the door lists it among its advisories). The + * refusal — the same finding at `error` — is planned for 19.0. + * + * ## Why `parent` alone + * + * This is the one root the record's own field rules bind and its validation + * rules do not: the credible copy, the one the runtime's own diagnostics name. + * Whether the other platform-wide roots the rule evaluator binds no more of + * (the acting user's among them) earn the same verdict is not decided here, + * because neither the runtime nor its callers were measured for them. + * + * ⛔ This verdict is withdrawn in the same change that binds `parent` for a + * `validations[]` rule, never before. + * + * `null` = nothing to report: the source does not parse (the syntax pass owns + * that), or it reads no `parent`. + */ +function validationRuleParentIssue( + objectName: string | undefined, + ruleName: string, + slot: 'condition' | 'when', + source: string, +): string | null { + if (!readsParentRoot(source)) return null; + const owner = objectName ? `'${objectName}'` : 'this object'; + return ( + `\`${slot}\` of validation rule '${ruleName}' reads \`parent\`, but the server evaluates a ` + + `\`validations[]\` rule against \`record\` and \`previous\` only, so \`parent\` is unbound there. ` + + `The rule can never be evaluated, and the server REFUSES every write it judges — a rule that ` + + `cannot be evaluated fails closed, seed rows included. This is a warning today; ` + + `\`os build\` and the object save door will refuse it from 19.0. The master-detail header is bound as ` + + `\`parent\` only for a field's own \`readonlyWhen\` and \`requiredWhen\`, on an object with exactly ` + + `one \`master_detail\`: to lock or require a field on the header's state, write the predicate on ` + + `that field's \`readonlyWhen\` or \`requiredWhen\` instead. For any other check, read a column ` + + `${owner} declares (denormalise the header value you need onto ${owner}).` + ); +} + /** * [#20078] The master object of an object with exactly ONE `master_detail` * relationship — the record the field level binds as `parent` — or `undefined` @@ -2848,6 +2919,21 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression if (p.depth === 0) continue; check(`object '${objectName}' · ${p.label}`, p.raw, objectName, 'record', undefined, true); } + // [#24720] The rule's root verdict, over every predicate it carries, + // located where `check()` above located the same predicate. `parent` is + // a `SCOPE_ROOTS` member, so `check()` never reports a bare reference to + // it and the two verdicts stay disjoint; see + // {@link validationRuleParentIssue}. + for (const p of predicates) { + const source = celSourceOf(p.raw); + const message = source ? validationRuleParentIssue(objectName, p.ruleName, p.slot, source) : null; + if (!message) continue; + const at = p.depth > 0 + ? `object '${objectName}' · ${p.label}` + : p.slot === 'when' ? `${where} when` : where; + // `warning` in 18.x, `error` from 19.0: see the docblock's ruling. + issues.push({ where: at, message, source: source!, severity: 'warning' }); + } // #4763 — null-guard gate over every predicate the rule carries, nested // `then`/`otherwise` branches included. for (const p of predicates) { diff --git a/packages/lint/src/validate-expressions.validation-rule-parent.test.ts b/packages/lint/src/validate-expressions.validation-rule-parent.test.ts new file mode 100644 index 00000000000..1031c85ae69 --- /dev/null +++ b/packages/lint/src/validate-expressions.validation-rule-parent.test.ts @@ -0,0 +1,205 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #24720 — a `validations[]` rule's predicate that reads `parent` is reported + * at authoring, at `os validate` / `os build` and at the object save door, at + * `warning` in 18.x (the standing post-GA ruling 甲: a narrowing no ADR-0087 + * conversion carries ships its loud half first; the refusal comes in 19.0). + * + * ## The state this closes + * + * ObjectQL evaluates every `validations[]` predicate — a `script` / + * `cross_field` rule's `condition` (`checkPredicate`), a `conditional` rule's + * `when` (`checkConditional`), and the rules nested in its `then` / + * `otherwise` — in `ruleScope(record, previous)`: `record` and `previous`, and + * no master-detail header. The header is bound as `parent` for a field's own + * `readonlyWhen` / `requiredWhen` only. `parent` is a `SCOPE_ROOTS` member, so + * the bare-reference check was silent on it, and a rule reading + * `parent.status` passed every door — then faulted (`Unknown variable: + * parent`) on every write it judged, which the server refuses as unevaluable, + * the seed's own rows included. + * + * ## The verdict + * + * One root verdict over every predicate the rule carries, located where + * `check()` locates the same predicate, reporting `parent` at warning, with the + * prescription that is true for it: the field-rule slots where `parent` IS + * bound, or a denormalised column. The runtime is unchanged. + */ +import { describe, expect, it } from 'vitest'; +import { EXPRESSION_INVALID, runAuthoringRules } from './authoring-rules.js'; +import { runRuntimeAuthoringRules } from './runtime-gate.js'; +import { validateStackExpressions } from './validate-expressions.js'; + +const PARENT = "parent.status == 'paid'"; + +const header = { + name: 'fx_header', + label: 'Header', + sharingModel: 'private', + fields: { name: { type: 'text', label: 'Name' }, status: { type: 'text', label: 'Status' } }, +}; + +/** A detail with exactly ONE `master_detail` — the shape on which the field-rule slots bind `parent`. */ +const line = ( + validations: unknown[], + extraFields: Record = {}, + masters: number = 1, +) => ({ + name: 'fx_line', + label: 'Line', + sharingModel: 'private', + fields: { + ...(masters >= 1 ? { hdr: { type: 'master_detail', label: 'Header', reference: 'fx_header' } } : {}), + ...(masters >= 2 ? { hdr2: { type: 'master_detail', label: 'Header 2', reference: 'fx_header' } } : {}), + amount: { type: 'number', label: 'Amount' }, + parent_code: { type: 'text', label: 'Parent code' }, + ...extraFields, + }, + validations, +}); + +const script = (condition: string, name = 'paid_lock') => ({ name, type: 'script', condition, message: 'x' }); + +const stackOf = (...objects: unknown[]) => ({ objects }); + +const expressionFindings = (fs: readonly T[]): T[] => + fs.filter((f) => f.rule === EXPRESSION_INVALID); + +const commandFindings = (command: 'validate' | 'build', ...objects: unknown[]) => { + const stack = stackOf(...objects); + return expressionFindings(runAuthoringRules(command, { normalized: stack, parsed: stack })); +}; + +const gateObject = (item: unknown, context: unknown[]) => + runRuntimeAuthoringRules({ type: 'object', item, context: { objects: context } }); + +const dump = (r: unknown) => JSON.stringify(r, null, 2); + +const WHERE = "object 'fx_line' · validation 'paid_lock'"; + +describe('#24720 — a `validations[]` predicate reading `parent` is reported at authoring', () => { + it('⭐ LIT — a `script` rule\'s `condition` reading `parent` is reported at warning, located at the rule', () => { + const issues = validateStackExpressions(stackOf(header, line([script(PARENT)]))); + + expect(issues, dump(issues)).toHaveLength(1); + expect(issues[0]).toMatchObject({ where: WHERE, severity: 'warning', source: PARENT }); + expect(issues[0]!.message).toContain("validation rule 'paid_lock' reads `parent`"); + }); + + it('⭐ the prescription names the field-rule slots where `parent` IS bound, and the denormalise remedy', () => { + const [issue] = validateStackExpressions(stackOf(header, line([script(PARENT)]))); + + expect(issue!.message).toContain('`readonlyWhen`'); + expect(issue!.message).toContain('`requiredWhen`'); + expect(issue!.message).toContain("denormalise the header value you need onto 'fx_line'"); + }); + + it('⭐ the `os validate` and `os build` commands both give the finding, at warning: nothing they gate on', () => { + for (const command of ['validate', 'build'] as const) { + const stack = stackOf(header, line([script(PARENT)])); + const all = runAuthoringRules(command, { normalized: stack, parsed: stack }); + const found = expressionFindings(all); + expect(found, `${command}: ${dump(found)}`).toHaveLength(1); + expect(found[0]).toMatchObject({ severity: 'warning', where: WHERE }); + // The 18.x half of the ruling: no `error` anywhere, so the command does not fail. + expect(all.filter((f) => f.severity === 'error'), `${command}: ${dump(all)}`).toEqual([]); + } + }); + + it('⭐ a `cross_field` rule\'s `condition` is judged the same way', () => { + const rule = { name: 'paid_lock', type: 'cross_field', fields: ['amount'], condition: PARENT, message: 'x' }; + const issues = validateStackExpressions(stackOf(header, line([rule]))); + + expect(issues, dump(issues)).toHaveLength(1); + expect(issues[0]).toMatchObject({ where: WHERE, severity: 'warning' }); + }); + + it('⭐ a `conditional` rule\'s `when`, and the rules in its `then` / `otherwise`, are judged at their own locations', () => { + const rule = { + type: 'conditional', + name: 'gate', + when: PARENT, + message: 'x', + then: { type: 'script', name: 'inner', condition: "parent.status != 'void'", message: 'y' }, + otherwise: { type: 'script', name: 'other', condition: 'record.amount != null && record.amount > 0', message: 'z' }, + }; + const issues = validateStackExpressions(stackOf(header, line([rule]))); + + expect(issues.map((i) => i.where), dump(issues)).toEqual([ + "object 'fx_line' · validation 'gate' when", + "object 'fx_line' · validation rule 'gate' then → 'inner'", + ]); + for (const i of issues) { + expect(i.severity).toBe('warning'); + expect(i.message).toContain('reads `parent`'); + } + }); + + it('gives one finding per predicate however often it reads `parent`', () => { + const issues = validateStackExpressions( + stackOf(header, line([script(`${PARENT} && parent.name != ''`)])), + ); + expect(issues, dump(issues)).toHaveLength(1); + }); + + it('refuses it whatever the object\'s master count: no validation rule binds `parent` on any object', () => { + for (const masters of [0, 2]) { + const issues = validateStackExpressions(stackOf(header, line([script(PARENT)], {}, masters))); + const here = issues.filter((i) => i.where === WHERE); + expect(here, `${masters} masters: ${dump(issues)}`).toHaveLength(1); + expect(here[0]!.severity).toBe('warning'); + } + }); + + it('⭐ CONTROL — `parent` on the field\'s own `readonlyWhen` / `requiredWhen` is still accepted', () => { + for (const key of ['readonlyWhen', 'requiredWhen']) { + const issues = validateStackExpressions( + stackOf(header, line([], { note: { type: 'text', label: 'Note', [key]: PARENT } })), + ); + expect(issues, `${key}: ${dump(issues)}`).toEqual([]); + } + }); + + it('⭐ CONTROL — `record` / `previous` reads, a column spelled like the root and the string `parent` pass', () => { + for (const condition of [ + 'record.amount != null && record.amount > 100', + 'previous != null && previous.amount != null && previous.amount > 100', + "record.parent_code == 'a'", + "record.parent_code == 'parent'", + ]) { + const issues = validateStackExpressions(stackOf(header, line([script(condition)]))); + expect(issues, `${condition}: ${dump(issues)}`).toEqual([]); + } + }); +}); + +describe('#24720 — the object door gives the build\'s verdict', () => { + it('⭐ LIT — the measured body is ADMITTED at the door with the finding as an advisory, located at the rule', () => { + const result = gateObject(line([script(PARENT)]), [header]); + + expect(result.rulesRun).toContain('validateStackExpressions'); + expect(result.errors, dump(result)).toEqual([]); + const advs = expressionFindings(result.advisories); + expect(advs, dump(result)).toHaveLength(1); + expect(advs[0]).toMatchObject({ severity: 'warning', where: WHERE, path: WHERE }); + expect(advs[0]!.message).toContain("validation rule 'paid_lock' reads `parent`"); + }); + + it('⭐ PARITY — the door finding IS the build finding', () => { + const atBuild = commandFindings('build', header, line([script(PARENT)])); + const atDoor = expressionFindings(gateObject(line([script(PARENT)]), [header]).advisories); + + expect(atBuild, dump(atBuild)).toHaveLength(1); + expect(atDoor, dump(atDoor)).toEqual(atBuild); + }); + + it('⭐ CONTROL — `parent` on a field\'s own `readonlyWhen` publishes clean, and the build agrees', () => { + const body = line([], { note: { type: 'text', label: 'Note', readonlyWhen: PARENT } }); + const result = gateObject(body, [header]); + + expect(expressionFindings(result.errors), dump(result)).toEqual([]); + expect(expressionFindings(result.advisories), dump(result)).toEqual([]); + expect(commandFindings('build', header, body)).toEqual([]); + }); +});