fix(objectql,lint)!: a conditional rule's when reads one hop through a reference, hydrated like a condition

.changeset/24512-conditional-when-hydration.md

@@ -0,0 +1,31 @@
+---
+'@objectstack/objectql': minor
+'@objectstack/lint': minor
+---
+
+fix(objectql,lint)!: a `conditional` validation rule's `when` reads one hop through a reference, the way a rule's `condition` does
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) No spec key, stored shape, export or authorable spelling is added, removed or renamed, so there is nothing for `objectstack migrate meta` to rewrite. A rule keeps its authored shape. What changes is how a `conditional` rule's `when` is judged: the engine now serves it one hop through a reference (it read the bare id before), and the validation-rule pass judges its hops as it judges a `condition`'s. Both narrow: the save door refuses four `when` shapes it accepted, and the runtime refuses a write it judged on an empty reference under a null-safe `when`. -->
+
+**BREAKING** (an accept-set narrowing, at the save door and at write time), shipped as `minor` under the launch-window convention for breaking changes.
+
+A `conditional` rule's `when` that reads through a lookup (`record.account.status == 'active'`) now works. The engine loads the related row for it, as it does for a `script` or `cross_field` rule's `condition`, and `when` picks `then` or `otherwise` on that row. Before, `when` met the stored id, so the read faulted and every write the rule guarded was refused with "The when-predicate reads 'status', which this object does not declare". A `when` nested in a `then` or `otherwise` is served the same way. A sibling rule keeps its own verdict when a `when` cannot be evaluated.
+
+`@objectstack/objectql`, at write time:
+
+- FROM: `when: record.account.status == 'active'` refused every write it guarded as unevaluable. TO: it is judged on the related row.
+- FROM: a null-safe spelling (`record.account.?status.orValue('') == 'active'`, `has(record.account.status) && …`) read the stored id and silently took `otherwise` for every account. TO: it is judged on the related row.
+- FROM: on a write whose reference is empty, a null-safe `when`, or one that tests `record.account != null` in the same expression, evaluated to false and judged `otherwise`. TO: the write is refused (`400 VALIDATION_FAILED`, `_record` `rule_violation`), as a `condition` reading through an empty reference is. The refusal names the guard.
+- A `when` that reads a second hop, a field the related object does not declare, or the reference both through it and as a value is refused with a sentence that names the related object and field, instead of the generic one. A `when` that faults on a delete's reference cleanup names the delete, as a `condition` does.
+- The guard spelling `when: record.account != null` reads the stored id, as before.
+
+`@objectstack/lint`, where a rule is written: `os validate`, `os build` and the object save door (`PUT /api/v1/meta/object/:name`) run the same pass, under the existing `EXPRESSION_INVALID` rule. They now judge a `when`, top level and nested, with the checks a `condition` gets. Each of these saved before (`200`) and is refused now (`422 INVALID_METADATA` at the save door), located at the `when` slot. At run time each refused every write it guarded, and the third also took `otherwise` on an empty reference:
+
+- FROM: a second hop (`record.account.owner.email != null`), saved. TO: refused.
+- FROM: a field the target does not declare (`record.account.region == 'north'`), saved. TO: refused, "unknown field `region` on `<target>`".
+- FROM: the reference read both through it and as a value (`record.account != null && record.account.status == 'active'`), saved. TO: refused, naming the guard.
+- FROM: a hop into a stored-metadata-body table, saved. TO: refused.
+
+What to do. To skip a rule while the reference is empty, keep the reference test in its own `when` and nest the rule that reads through it: a `conditional` whose `when` is `record.account != null`, with your `conditional` as its `then`. To refuse an empty reference, make the field `required: true`. Read a field the target declares, and read one hop.

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

@@ -39,8 +39,8 @@
  * 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
+ * relationship-traversal checks on a nested `condition` and — since #24512 —
+ * on a nested `when` (ObjectQL hydrates both). 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';
@@ -236,7 +236,7 @@ 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. */
+/** Reads more than one relationship hop — a shape `checkPredicate` and (#24512) `checkConditional` refuse. */
 const MULTI_HOP = 'record.account.owner.email != null';
 const HYDRATION = {
   type: 'conditional',
@@ -311,16 +311,21 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver
     ]);
   });
 
-  it('the traversal checks follow the evaluator per slot: ON for a nested `condition`, OFF for a nested `when`', () => {
+  it('the traversal checks follow the evaluator per slot: ON for a nested `condition` and, since #24512, 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.
+    // `checkPredicate` refuses a read deeper than one hop at any depth, and
+    // `checkConditional` evaluates a `when` on the same hydrated scope, so the
+    // build says so for both slots…
+    expect(atBuild.map((f) => f.where), dump(atBuild)).toEqual([
+      "object 'fx_rule' · validation rule 'outer' then → 'inner'",
+      "object 'fx_rule' · validation rule 'outer' otherwise → 'mid' when-predicate",
+    ]);
+    for (const f of atBuild) expect(f.message).toContain('ONE hop');
+    // …and at the top-level `when` site too.
     const topWhen = buildFindings(fxRef([{ ...HYDRATION, when: MULTI_HOP, then: NESTED_VALID.otherwise, otherwise: undefined }]));
-    expect(topWhen, dump(topWhen)).toEqual([]);
+    expect(topWhen.map((f) => f.where), dump(topWhen)).toEqual(["object 'fx_rule' · validation 'outer' when"]);
+    expect(topWhen[0]!.message).toContain('ONE hop');
     expect(expressionFindings(gateObject(fxRef([HYDRATION])).errors)).toEqual(atBuild);
   });
 
@@ -382,3 +387,68 @@ describe('#24468 — the object door refuses a rule hop to a field the target do
     expect(expressionFindings(result.advisories), dump(result)).toEqual([]);
   });
 });
+
+describe('#24512 — the object door judges a `conditional` rule\'s `when` hop as it judges a `condition`\'s', () => {
+  /** The lookup's target, delivered by ANOTHER package the written one depends on. */
+  const account = {
+    name: 'fx_account', label: 'Account', sharingModel: 'private', _packageId: 'com.example.accounts',
+    fields: { status: { type: 'text', label: 'Status' } },
+  };
+  const order = (validations: unknown[]) => ({
+    name: 'fx_order',
+    label: 'Order',
+    sharingModel: 'private',
+    fields: {
+      amount: { type: 'number', label: 'Amount' },
+      account: { type: 'lookup', label: 'Account', reference: 'fx_account' },
+    },
+    validations,
+  });
+  const gate = (when: string, name = 'gate') => ({
+    type: 'conditional', name, when, message: 'x',
+    then: { type: 'script', name: 'cap', condition: 'record.amount != null && record.amount > 100', message: 'y' },
+  });
+  const gateOrder = (validations: unknown[]) => runRuntimeAuthoringRules({
+    type: 'object',
+    item: order(validations),
+    context: { objects: [account] },
+    packageScope: { packageId: 'com.example.orders', dependencies: ['com.example.accounts'] },
+  });
+
+  it('the fixtures are spec-valid: each refusal below is the expression verdict, not the schema\'s', () => {
+    for (const when of ["record.account.region == 'north'", 'record.account.owner.email != null', "record.account.status == 'open'"]) {
+      const parsed = ObjectSchema.safeParse(order([gate(when), { type: 'conditional', name: 'outer', when: 'record.amount != null', message: 'x', then: gate(when, 'inner') }]));
+      expect(parsed.success, dump(parsed.error?.issues)).toBe(true);
+    }
+  });
+
+  it('⭐ LIT — a `when` reading a field the target does not declare is REFUSED under EXPRESSION_INVALID, naming the slot, the field and the target', () => {
+    const result = gateOrder([gate("record.account.region == 'north'")]);
+
+    const errs = expressionFindings(result.errors);
+    expect(errs, dump(result)).toHaveLength(1);
+    const where = "object 'fx_order' · validation 'gate' when";
+    expect(errs[0]).toMatchObject({ rule: EXPRESSION_INVALID, severity: 'error', where, path: where });
+    expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`');
+    expect(errs[0]!.message).toContain('the rule has no verdict');
+  });
+
+  it('⭐ LIT — a `when` reading a second hop is REFUSED, and a nested `when` is judged too', () => {
+    const result = gateOrder([{
+      type: 'conditional', name: 'outer', when: 'record.amount != null', message: 'x',
+      then: gate('record.account.owner.email != null', 'inner'),
+    }]);
+
+    const errs = expressionFindings(result.errors);
+    expect(errs.map((e) => e.where), dump(result)).toEqual(["object 'fx_order' · validation rule 'outer' then → 'inner' when-predicate"]);
+    expect(errs[0]).toMatchObject({ rule: EXPRESSION_INVALID, severity: 'error' });
+    expect(errs[0]!.message).toContain('ONE hop');
+  });
+
+  it('⭐ CONTROL — a `when` reading a field the target declares publishes clean', () => {
+    const result = gateOrder([gate("record.account.status == 'open'")]);
+
+    expect(expressionFindings(result.errors), dump(result)).toEqual([]);
+    expect(expressionFindings(result.advisories), dump(result)).toEqual([]);
+  });
+});

packages/lint/src/validate-expressions.rule-one-hop.test.ts

@@ -71,3 +71,58 @@ describe('[#24468] a validation rule reads one hop through a reference', () => {
     expect(errorsOf("record.account.region == 'north'", 'hop_elsewhere')).toEqual([]);
   });
 });
+
+/**
+ * [#24512] A `conditional` rule's `when` is hydrated one hop by ObjectQL's
+ * `checkConditional`, as a `condition` is by `checkPredicate`, so the same hop
+ * checks judge it — at the top level and in a nested `conditional` — under the
+ * same `expression-invalid` rule.
+ */
+describe('[#24512] a `conditional` rule\'s `when` reads one hop through a reference', () => {
+  const WHEN_WHERE = "object 'hop_order' · validation 'gate' when";
+  const NESTED_WHERE = "object 'hop_order' · validation rule 'outer' then → 'gate' when-predicate";
+  const gate = (when: string) => ({
+    name: 'gate', type: 'conditional', severity: 'error', message: 'x', when,
+    then: { name: 'cap', type: 'script', severity: 'error', message: 'y', condition: 'record.amount != null && record.amount > 100' },
+  });
+  const withRule = (rule: Record<string, unknown>) => ({ ...order('true'), validations: [rule] });
+  const errorsAt = (rule: Record<string, unknown>) =>
+    validateStackExpressions(specValid({ objects: [account, withRule(rule)] }))
+      .filter((i) => (i.severity ?? 'error') === 'error');
+  const outer = (when: string) => ({
+    name: 'outer', type: 'conditional', severity: 'error', message: 'x', when: 'record.amount != null', then: gate(when),
+  });
+
+  it('admits a `when` reading a field the target declares, top level and nested', () => {
+    expect(errorsAt(gate("record.account.status == 'active'"))).toEqual([]);
+    expect(errorsAt(outer("record.account.status == 'active'"))).toEqual([]);
+  });
+
+  it('refuses a `when` reading a field the target does not declare, naming the slot, the field and the target', () => {
+    for (const [rule, where] of [[gate("record.account.region == 'north'"), WHEN_WHERE], [outer("record.account.region == 'north'"), NESTED_WHERE]] as const) {
+      const errors = errorsAt(rule);
+      expect(errors.map((e) => e.where)).toEqual([where]);
+      expect(errors[0].message).toMatch(/^unknown field `region` on `hop_account`/);
+      expect(errors[0].message).toContain('through the reference `record.account`');
+    }
+  });
+
+  it('refuses a `when` reading a second hop, naming the slot', () => {
+    for (const [rule, where] of [[gate("record.account.owner.email == 'x'"), WHEN_WHERE], [outer("record.account.owner.email == 'x'"), NESTED_WHERE]] as const) {
+      const errors = errorsAt(rule);
+      expect(errors.map((e) => e.where)).toEqual([where]);
+      expect(errors[0].message).toContain('read through more than one relationship hop');
+    }
+  });
+
+  it('refuses a `when` reading the reference both through it and as a value, naming the guard', () => {
+    const errors = errorsAt(gate("record.account != null && record.account.status == 'active'"));
+    expect(errors.map((e) => e.where)).toEqual([WHEN_WHERE]);
+    expect(errors[0].message).toContain('is read BOTH through the relationship');
+    expect(errors[0].message).toContain('`conditional` rule whose `when` is `record.account != null`');
+  });
+
+  it('CONTROL — the guard spelling reads the stored id and is admitted', () => {
+    expect(errorsAt(gate('record.account != null'))).toEqual([]);
+  });
+});

packages/lint/src/validate-expressions.ts

@@ -544,8 +544,6 @@ function celSourceOf(raw: unknown): string | undefined {
 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;
 }
@@ -568,9 +566,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, slot: 'condition', depth });
+  if (main != null) out.push({ label: `validation rule ${here}`, raw: main, depth });
   if (rule.when != null) {
-    out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, slot: 'when', depth });
+    out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, depth });
   }
   for (const branch of ['then', 'otherwise'] as const) {
     const nested = rule[branch];
@@ -1946,7 +1944,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
   /**
    * [#24316] Refuse a one-hop read through a reference into a
    * stored-metadata-body table, at the two sites that serve a hop: a
-   * validation rule's hydrated `condition` ({@link check} with
+   * validation rule's hydrated `condition` or `when` ({@link check} with
    * `traversalHydration`) and a formula field's `expression`
    * ({@link judgeFieldFormula}). ObjectQL's one-hop loader never reads that
    * hop, so as written the field reads null or the rule refuses every write;
@@ -1984,8 +1982,8 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
    *
    * [#24468] The two sites that serve a hop share it: a formula field's
    * `expression` ({@link judgeFieldFormula}) and a validation rule's hydrated
-   * `condition` ({@link check} with `traversalHydration`), where the rule has
-   * no verdict and refuses every write it judges.
+   * `condition` or `when` ({@link check} with `traversalHydration`), where the
+   * rule has no verdict and refuses every write it judges.
    *
    * Judged against the target's field index — its declared fields and the
    * system columns the platform injects on it, the set the registry carries —
@@ -2163,7 +2161,8 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
     /**
      * [#18682] Opt this ONE site in to the relationship-traversal conflict
      * checks. True only where the traversal is actually SERVED — object
-     * validation rules, which ObjectQL's `checkPredicate` hydrates (a formula
+     * validation rules' `condition` and `when`, which ObjectQL's
+     * `checkPredicate` and `checkConditional` hydrate (#24512) (a formula
      * `expression`, the other served site, is judged by `judgeFieldFormula`,
      * which does not go through this helper). Everywhere
      * else the checks' prescription ("write `record.<fk>.id`") is false,
@@ -2787,15 +2786,13 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
       // bare refs are flagged (#1928).
       // [#18682] The sites where a relationship traversal is SERVED: these are
       // the `script` / `cross_field` conditions ObjectQL's `checkPredicate`
-      // hydrates. `traversalHydration` is passed here, on a nested
-      // `condition` below (#22042), and on a formula field's `expression`
-      // (`judgeFieldFormula`, #24197), and NOWHERE else.
+      // hydrates, and (#24512) a `conditional` rule's `when`, which
+      // `checkConditional` evaluates on the same hydrated scope.
+      // `traversalHydration` is passed on those two slots here and nested below
+      // (#22042), and on a formula field's `expression` (`judgeFieldFormula`,
+      // #24197), 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');
+      check(`${where} when`, (rule as AnyRec).when, objectName, 'record', undefined, true);
       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
@@ -2804,15 +2801,13 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression
       // 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`.
+      // Hydration follows the evaluator, as at the top level: ObjectQL's
+      // `collectPredicateRelationships` reaches a nested `condition` and a
+      // nested `when` alike, and both are evaluated hydrated, so both slots
+      // meet the traversal checks.
       for (const p of predicates) {
         if (p.depth === 0) continue;
-        check(`object '${objectName}' · ${p.label}`, p.raw, objectName, 'record', undefined, p.slot === 'condition');
+        check(`object '${objectName}' · ${p.label}`, p.raw, objectName, 'record', undefined, true);
       }
       // #4763 — null-guard gate over every predicate the rule carries, nested
       // `then`/`otherwise` branches included.