fix(lint)!: a conditional validation rule's nested then / otherwise predicate meets the build's expression verdict, at os build and the object save door (#22042)

.changeset/22042-nested-validation-predicate-verdict.md

@@ -0,0 +1,30 @@
+---
+"@objectstack/lint": minor
+"@objectstack/metadata-protocol": minor
+---
+
+fix(lint)!: a `conditional` validation rule's nested `then` / `otherwise` predicate meets the same expression verdict as the rule's own (#22042)
+
+Clause-②: no (narrowing)
+
+A `conditional` validation rule applies its `then` rule when its `when` holds and its `otherwise` rule when it does not, and the rule validator evaluates either branch as a rule of its own. `os build` judged a rule's own `condition` and `when` with the shared `validateExpression` validator, but reached the predicates inside `then` / `otherwise` with the null-guard check alone. So a nested `condition` that called an unregistered function, such as `sqrt(record.amount) > 1`, or read a bare field, such as `amont > 1`, passed `os build`. The object save door gives the build's verdict, so `PUT /api/v1/meta/object/:name` stored it, and the rule then refused every write it judged, because a validation rule that cannot be evaluated fails closed. The same predicate one level up was refused at both doors.
+
+The build's expression rule (`validateStackExpressions`) now runs the same check on every predicate nested in a `conditional` rule, at every depth: each nested `condition`, and the `when` of a `conditional` nested inside a branch. A nested `condition` also gets the relationship-traversal checks, because ObjectQL hydrates a one-hop read there, as it does at the top level. A nested `when` does not, because the evaluator never hydrates a `when`, as at the top level. A nested finding is located at the nested rule, the location the null-guard check already gave it: `object 'OBJECT' · validation rule 'OUTER' then → 'INNER'`, with `when-predicate` appended for a nested `when`. A rule's own `condition` and `when` keep their findings and their location (`object 'OBJECT' · validation 'NAME'`) and are judged once.
+
+**BREAKING — what moves for consumers.**
+
+- `os build`, `os validate` and `os lint` now refuse, at `error`, a stack whose `conditional` validation rule carries a nested predicate the shared validator refuses. The validator's warnings on a nested predicate are now reported too, and at the save door they ride the response as advisories.
+- An object write in publish mode that carries such a rule answered 200. It now answers `422 INVALID_METADATA`, with an `expression-invalid` issue located at the nested rule. This covers `PUT /api/v1/meta/object/:name` (and `saveMetaItem` in publish mode) and the promotion of a draft (`POST /api/v1/meta/object/:name/publish`, `publishMetaItem`).
+- The verdict is the one a rule's own `condition` already got: an unknown function, a field the object does not declare, a bare field reference (`amount` instead of `record.amount`), a syntax error, and, for a nested `condition`, a reference field read both through the relationship and as a value, or a read deeper than one hop.
+
+**Remedy.** Fix the nested predicate the way the same predicate is fixed at the top level: the message names the unknown function or field and the position. Qualify field reads as `record.FIELD`, and use one of the functions `introspectScope` lists. Saving the object as a draft (`mode: 'draft'`) is still allowed, because drafts are never gated; publishing that draft is judged.
+
+**Unchanged.**
+
+- Stored rows are not migrated, and they are not refused on read. An object stored before this change keeps loading until it is next saved, and that save is judged.
+- A rule's own `condition` and `when` are judged exactly as before, at the same location; the null-guard check over every predicate is unchanged.
+- `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns a refusal into a logged write.
+- Measured before crossing: this repository ships one `conditional` validation rule with nested predicates (`showcase_account.churn_reason_consistency`, two nested `condition`s), among 21 validation rules on the 118 objects it ships. Both have 0 refusals and 0 advisories, at the build and at the door.
+- No public export or signature moves. `validateStackExpressions(stack)` keeps its signature, and no registry entry changes.
+
+<!-- adr-0087: not-required (no-migration-prescription) a refusal, at os build and at the object save door, of a nested conditional validation predicate the published validator already refuses one level up: no authorable key, spelling, export or stored shape moves, and no stored row is read, rewritten or converted. A stored object whose nested predicate the validator refuses keeps loading until it is next saved, and the repair is the author's edit of the predicate, which no ledger entry can derive. The other categories are closed on facts: the packages publish (not unpublished); no ADR-0087 id covers this verdict (not already-registered); and the change is a validator verdict, not a declaration (not runtime-interface-only or type-surface-only). -->

packages/lint/src/runtime-gate.object-validation-writes.test.ts

@@ -30,8 +30,21 @@
  * The protocol-level half — the same verdict through the real `saveMetaItem`
  * and `publishMetaItem` — is the #22032 block of
  * `packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts`.
+ *
+ * ## #22042 — one level down
+ *
+ * A `conditional` rule's `then` / `otherwise` is a rule the evaluator runs,
+ * yet only the null-guard gate reached its predicates: the same unregistered
+ * function or bare field the top level refuses published clean one level
+ * down, in `os build` and at this door alike. The pass now runs the same
+ * `check()` on every nested predicate, at the location the null-guard gate
+ * already gives it (`validation rule 'outer' then → 'inner'`), with the
+ * relationship-traversal checks on a nested `condition` (ObjectQL hydrates it)
+ * and not on a nested `when` (it does not). The second describe block below
+ * pins it; its protocol half is the #22042 block of the same protocol file.
  */
 import { describe, expect, it } from 'vitest';
+import { ObjectSchema } from '@objectstack/spec/data';
 import { EXPRESSION_INVALID, runAuthoringRules } from './authoring-rules.js';
 import { runRuntimeAuthoringRules, runtimeAuthoringRulesFor } from './runtime-gate.js';
 
@@ -173,3 +186,148 @@ describe('#22032 pass 1 — the object door gives the build\'s validation-rule v
     expect(expressionFindings(result.errors), dump(result)).toEqual([]);
   });
 });
+
+/** #22042 — the card's two bodies, one level down: an unregistered function in `then`, a bare field in `otherwise`. */
+const NESTED_REFUSED = {
+  type: 'conditional',
+  name: 'outer',
+  when: "record.status == 'open'",
+  message: 'x',
+  then: { type: 'script', name: 'inner', condition: 'sqrt(record.amount) > 1', message: 'y' },
+  otherwise: { type: 'script', name: 'other', condition: 'amont > 1', message: 'z' },
+};
+const THEN_WHERE = "object 'fx_rule' · validation rule 'outer' then → 'inner'";
+const OTHERWISE_WHERE = "object 'fx_rule' · validation rule 'outer' otherwise → 'other'";
+
+/** Two levels: a bare field in a nested `conditional`'s own `when`, an unregistered function one level below it. */
+const TWO_LEVEL = {
+  type: 'conditional',
+  name: 'outer',
+  when: "record.status == 'open'",
+  message: 'x',
+  then: {
+    type: 'conditional',
+    name: 'mid',
+    when: 'amount > 1',
+    message: 'y',
+    then: { type: 'script', name: 'deep', condition: 'sqrt(record.amount) > 1', message: 'z' },
+  },
+};
+
+/** Valid, guarded predicates in both branches and two levels down. */
+const NESTED_VALID = {
+  type: 'conditional',
+  name: 'outer',
+  when: "record.status == 'open'",
+  message: 'x',
+  then: {
+    type: 'conditional',
+    name: 'mid',
+    when: 'record.amount != null',
+    message: 'y',
+    // Guarded in its own source: the null-guard gate does not credit the enclosing `when`.
+    then: { type: 'script', name: 'deep', condition: 'record.amount != null && record.amount > 100', message: 'z' },
+  },
+  otherwise: { type: 'script', name: 'other', condition: 'record.amount != null && record.amount < 0', message: 'w' },
+};
+
+/** A probe object with a reference field, for the per-slot traversal checks. */
+const fxRef = (validations: unknown[]) => {
+  const base = fxRule(validations);
+  return { ...base, fields: { ...base.fields, account: { type: 'lookup', label: 'Account', reference: 'fx_rule' } } };
+};
+/** Reads more than one relationship hop — a shape `checkPredicate` refuses, and `checkConditional` never judges. */
+const MULTI_HOP = 'record.account.owner.email != null';
+const HYDRATION = {
+  type: 'conditional',
+  name: 'outer',
+  when: "record.status == 'open'",
+  message: 'x',
+  then: { type: 'script', name: 'inner', condition: MULTI_HOP, message: 'y' },
+  otherwise: {
+    type: 'conditional',
+    name: 'mid',
+    when: MULTI_HOP,
+    message: 'z',
+    then: { type: 'script', name: 'deep', condition: 'record.amount != null', message: 'w' },
+  },
+};
+
+describe('#22042 — a `conditional` rule\'s nested predicates meet the same verdict, at the build and at the door', () => {
+  it('the fixtures are spec-valid: each refusal below is the expression verdict, not the schema\'s', () => {
+    for (const body of [fxRule([NESTED_REFUSED]), fxRule([TWO_LEVEL]), fxRule([NESTED_VALID]), fxRef([HYDRATION])]) {
+      const parsed = ObjectSchema.safeParse(body);
+      expect(parsed.success, dump(parsed.error?.issues)).toBe(true);
+    }
+  });
+
+  it('⭐ LIT — an unregistered function in `then` and a bare field in `otherwise` are REFUSED by `os build`, located at the nested rule', () => {
+    const atBuild = buildFindings(fxRule([NESTED_REFUSED]));
+
+    expect(atBuild.map((f) => f.where), dump(atBuild)).toEqual([THEN_WHERE, OTHERWISE_WHERE]);
+    for (const f of atBuild) expect(f).toMatchObject({ severity: 'error', path: f.where });
+    expect(atBuild[0]!.message).toContain('`sqrt` is not a callable name here');
+    expect(atBuild[1]!.message).toContain('bare reference `amont`');
+  });
+
+  it('⭐ LIT — the object door REFUSES the same body, and its findings ARE the build\'s', () => {
+    const result = gateObject(fxRule([NESTED_REFUSED]));
+
+    expect(result.rulesRun).toContain('validateStackExpressions');
+    const atDoor = expressionFindings(result.errors);
+    expect(atDoor.map((f) => f.where), dump(result)).toEqual([THEN_WHERE, OTHERWISE_WHERE]);
+    expect(atDoor).toEqual(buildFindings(fxRule([NESTED_REFUSED])));
+  });
+
+  it('⭐ LIT — two levels down: a nested `conditional`\'s `when` and the rule below it are judged', () => {
+    const atBuild = buildFindings(fxRule([TWO_LEVEL]));
+
+    expect(atBuild.map((f) => f.where), dump(atBuild)).toEqual([
+      "object 'fx_rule' · validation rule 'outer' then → 'mid' when-predicate",
+      "object 'fx_rule' · validation rule 'outer' then → 'mid' then → 'deep'",
+    ]);
+    expect(atBuild[0]!.message).toContain('bare reference `amount`');
+    expect(atBuild[1]!.message).toContain('`sqrt` is not a callable name here');
+    expect(expressionFindings(gateObject(fxRule([TWO_LEVEL])).errors)).toEqual(atBuild);
+  });
+
+  it('⭐ CONTROL — valid nested predicates publish clean, two levels down and in `otherwise`', () => {
+    const result = gateObject(fxRule([NESTED_VALID]));
+
+    expect(expressionFindings(result.errors), dump(result)).toEqual([]);
+    expect(expressionFindings(result.advisories), dump(result)).toEqual([]);
+    expect(buildFindings(fxRule([NESTED_VALID]))).toEqual([]);
+  });
+
+  it('each predicate is judged ONCE: the rule\'s own `condition` / `when` keep their location and are not re-judged as nested', () => {
+    // A top-level `condition` — one finding, at the rule's own location only.
+    const top = buildFindings(fxRule([UNREGISTERED]));
+    expect(top.map((f) => f.where), dump(top)).toEqual(["object 'fx_rule' · validation 'amount_root'"]);
+    // A faulting top-level `when` beside a faulting nested `then` — one finding each.
+    const both = buildFindings(fxRule([{ ...WHEN, then: NESTED_REFUSED.then }]));
+    expect(both.map((f) => f.where), dump(both)).toEqual([
+      "object 'fx_rule' · validation 'gate' when",
+      "object 'fx_rule' · validation rule 'gate' then → 'inner'",
+    ]);
+  });
+
+  it('the traversal checks follow the evaluator per slot: ON for a nested `condition`, OFF for a nested `when`', () => {
+    const atBuild = buildFindings(fxRef([HYDRATION]));
+
+    // `checkPredicate` refuses a read deeper than one hop at any depth, so the build says so…
+    expect(atBuild.map((f) => f.where), dump(atBuild)).toEqual(["object 'fx_rule' · validation rule 'outer' then → 'inner'"]);
+    expect(atBuild[0]!.message).toContain('ONE hop');
+    // …and `checkConditional` never hydrates a `when`, so the same source there earns no
+    // traversal prescription — exactly as the top-level `when` site is opted out.
+    const topWhen = buildFindings(fxRef([{ ...HYDRATION, when: MULTI_HOP, then: NESTED_VALID.otherwise, otherwise: undefined }]));
+    expect(topWhen, dump(topWhen)).toEqual([]);
+    expect(expressionFindings(gateObject(fxRef([HYDRATION])).errors)).toEqual(atBuild);
+  });
+
+  it('a stored sibling\'s nested fault is not this write\'s to answer for (the differential)', () => {
+    const sibling = { ...fxRule([NESTED_REFUSED, TWO_LEVEL]), name: 'fx_sibling' };
+    const result = runRuntimeAuthoringRules({ type: 'object', item: fxRule([NESTED_VALID]), context: { objects: [sibling] } });
+
+    expect(expressionFindings(result.errors), dump(result)).toEqual([]);
+  });
+});

packages/lint/src/validate-expressions.ts

@@ -481,13 +481,23 @@ function celSourceOf(raw: unknown): string | undefined {
   return undefined;
 }
 
+/** One predicate a validation rule carries — see {@link rulePredicates}. */
+interface RulePredicate {
+  label: string;
+  raw: unknown;
+  /** `condition` (a `script` / `cross_field` rule's) or `when` (a `conditional` rule's). */
+  slot: 'condition' | 'when';
+  /** 0 for the rule itself; 1 inside its `then` / `otherwise`, and so on down. */
+  depth: number;
+}
+
 /**
  * Every predicate a validation rule carries, including the ones nested inside a
  * `conditional` rule's `then` / `otherwise` — the trap hides there just as
  * happily as at the top level.
  */
-function rulePredicates(rule: AnyRec, path: string): Array<{ label: string; raw: unknown }> {
-  const out: Array<{ label: string; raw: unknown }> = [];
+function rulePredicates(rule: AnyRec, path: string, depth = 0): RulePredicate[] {
+  const out: RulePredicate[] = [];
   const name = typeof rule.name === 'string' ? rule.name : '?';
   const here = path ? `${path} → '${name}'` : `'${name}'`;
   // `condition` is the declared predicate key on every validation-rule variant
@@ -499,12 +509,14 @@ function rulePredicates(rule: AnyRec, path: string): Array<{ label: string; raw:
   // 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 });
-  if (rule.when != null) out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when });
+  if (main != null) out.push({ label: `validation rule ${here}`, raw: main, slot: 'condition', depth });
+  if (rule.when != null) {
+    out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, slot: 'when', depth });
+  }
   for (const branch of ['then', 'otherwise'] as const) {
     const nested = rule[branch];
     if (nested && typeof nested === 'object' && !Array.isArray(nested)) {
-      out.push(...rulePredicates(nested as AnyRec, `${here} ${branch}`));
+      out.push(...rulePredicates(nested as AnyRec, `${here} ${branch}`, depth + 1));
     }
   }
   return out;
@@ -1885,18 +1897,37 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
       // The declared predicate key is `condition` (see `rulePredicates`).
       // Validation predicates are `record`-scoped — no field flattening — so
       // bare refs are flagged (#1928).
-      // [#18682] The two sites where a relationship traversal is SERVED: these
-      // are the `script` / `cross_field` conditions ObjectQL's `checkPredicate`
-      // hydrates. `traversalHydration` is passed here and NOWHERE else.
+      // [#18682] The sites where a relationship traversal is SERVED: these are
+      // the `script` / `cross_field` conditions ObjectQL's `checkPredicate`
+      // hydrates. `traversalHydration` is passed here and on a nested
+      // `condition` below (#22042), and NOWHERE else.
       check(where, rule.condition, objectName, 'record', undefined, true);
       // `conditional` rules carry a nested `when` predicate (record-scoped).
       // ⚠️ `when` is evaluated by `checkConditional` WITHOUT hydration today, so
       // it is opted OUT: a traversal there faults, and the conflict checks'
       // prescription would not repair it.
       check(`${where} when`, (rule as AnyRec).when, objectName, 'record');
+      const predicates = rulePredicates(rule, '');
+      // [#22042] The same verdict one level down, and every level below it: a
+      // `conditional` rule's `then` / `otherwise` is a rule the evaluator runs
+      // (`checkConditional` hands the branch to `evaluateRule`), so its
+      // predicates meet `check()` exactly as the two calls above do — located
+      // at the label `rulePredicates` builds, the location the null-guard gate
+      // below already gives the same predicate. Depth 0 is skipped: the rule's
+      // own `condition` / `when` met `check()` above, under their own location.
+      // Hydration follows the evaluator per slot, as at the top level: a
+      // nested `condition` is a `script` / `cross_field` rule's, which
+      // ObjectQL's `collectPredicateRelationships` reaches inside a
+      // `conditional` and `checkPredicate` hydrates; a nested `when` is a
+      // nested `conditional`'s, which `checkConditional` evaluates WITHOUT
+      // hydration, so it is opted out like the top-level `when`.
+      for (const p of predicates) {
+        if (p.depth === 0) continue;
+        check(`object '${objectName}' · ${p.label}`, p.raw, objectName, 'record', undefined, p.slot === 'condition');
+      }
       // #4763 — null-guard gate over every predicate the rule carries, nested
       // `then`/`otherwise` branches included.
-      for (const p of rulePredicates(rule, '')) {
+      for (const p of predicates) {
         checkNullGuards(`object '${objectName}' · ${p.label}`, p.label, p.raw, objectName);
       }
     }