From 69af7c0eee59ea69c714d47a3ba830781628ccfe Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:34:57 +0000 Subject: [PATCH 1/3] fix(lint): a validation rule's expression finding carries the slot's key path Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WGufzj2PNcHjESYvrviQf7 --- packages/lint/src/validate-expressions.ts | 49 +++++- ...e-expressions.validation-rule-path.test.ts | 145 ++++++++++++++++++ 2 files changed, 189 insertions(+), 5 deletions(-) create mode 100644 packages/lint/src/validate-expressions.validation-rule-path.test.ts diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 0faec702003..374859c9a61 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -165,7 +165,10 @@ export interface ExprIssue { * [#26858] The judged slot's key path in the stack, in the spelling the other * rules' findings carry (`flows[0].edges[1].condition`), beside the human * `where`. Set only where the slot has one: a top-level flow edge's - * `condition`. Absent, the finding is located by `where` alone. + * `condition`, and (#27643) an object validation rule's `condition` / `when` + * (`objects[0].validations[1].condition`), a nested branch's included + * (`objects[0].validations[1].then.condition`). Absent, the finding is + * located by `where` alone. */ path?: string; } @@ -573,6 +576,11 @@ interface RulePredicate { slot: 'condition' | 'when'; /** The name of the rule that carries it (the nested rule's own, below depth 0). */ ruleName: string; + /** + * [#27643] Its key path inside the top-level rule (`condition`, `when`, + * `then.condition`, `otherwise.then.when`), for a finding's `path`. + */ + key: string; } /** @@ -580,7 +588,7 @@ interface RulePredicate { * `conditional` rule's `then` / `otherwise` — the trap hides there just as * happily as at the top level. */ -function rulePredicates(rule: AnyRec, path: string, depth = 0): RulePredicate[] { +function rulePredicates(rule: AnyRec, path: string, depth = 0, keyPrefix = ''): RulePredicate[] { const out: RulePredicate[] = []; const name = typeof rule.name === 'string' ? rule.name : '?'; const here = path ? `${path} → '${name}'` : `'${name}'`; @@ -593,14 +601,18 @@ 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, slot: 'condition', ruleName: name }); + if (main != null) { + out.push({ label: `validation rule ${here}`, raw: main, depth, slot: 'condition', ruleName: name, key: `${keyPrefix}condition` }); + } if (rule.when != null) { - out.push({ label: `validation rule ${here} when-predicate`, raw: rule.when, depth, slot: 'when', ruleName: name }); + out.push({ + label: `validation rule ${here} when-predicate`, raw: rule.when, depth, slot: 'when', ruleName: name, key: `${keyPrefix}when`, + }); } 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}`, depth + 1)); + out.push(...rulePredicates(nested as AnyRec, `${here} ${branch}`, depth + 1, `${keyPrefix}${branch}.`)); } } return out; @@ -2955,6 +2967,11 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // ── Object validation-rule + formula predicates ──────────────────── for (const obj of objects) { const objectName = typeof obj.name === 'string' ? obj.name : undefined; + // [#27643] The object's index in the authored list, for a validation-rule + // finding's key path (`recordsOf` hands an array's own members back). A + // map-form `objects` has no index to spell, so its findings keep `where` + // alone, as a map-form `flows` collection's do (#26858). + const objectIndex = Array.isArray(stack.objects) ? stack.objects.indexOf(obj) : -1; // `validations` is the key `ObjectSchema` declares; `validationRules` is a // rejected alias of it (#5017) — see the `## Scope` table above. const validations = obj.validations; @@ -2963,6 +2980,18 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // position, so the door's findings and their order are the build's. for (const rule of recordsOf(validations)) { const where = `object '${objectName}' · validation '${(rule.name as string) ?? '?'}'`; + // [#27643] Every finding below also carries the judged slot's key path + // (`objects[0].validations[1].condition`), indexed over the lists the + // author wrote, in the spelling a flow edge's findings use (#26858), so + // a draft's `_diagnostics` reports it item-relative + // (`validations.1.condition`). `where` stays the human locator. + const ruleIndex = objectIndex >= 0 && Array.isArray(validations) ? validations.indexOf(rule) : -1; + const rulePath = ruleIndex >= 0 ? `objects[${objectIndex}].validations[${ruleIndex}]` : undefined; + const atSlot = (key: string, first: number): void => { + if (!rulePath) return; + const path = `${rulePath}.${key}`; + for (let k = first; k < issues.length; k++) issues[k] = { ...issues[k]!, path }; + }; // The declared predicate key is `condition` (see `rulePredicates`). // Validation predicates are `record`-scoped — no field flattening — so // bare refs are flagged (#1928). @@ -2973,8 +3002,12 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // `traversalHydration` is passed on those two slots here and nested below // (#22042), and on a formula field's `expression` (`judgeFieldFormula`, // #24197), and NOWHERE else. + let first = issues.length; check(where, rule.condition, objectName, 'record', undefined, true); + atSlot('condition', first); + first = issues.length; check(`${where} when`, (rule as AnyRec).when, objectName, 'record', undefined, true); + atSlot('when', first); 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 @@ -2989,7 +3022,9 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // meet the traversal checks. for (const p of predicates) { if (p.depth === 0) continue; + first = issues.length; check(`object '${objectName}' · ${p.label}`, p.raw, objectName, 'record', undefined, true); + atSlot(p.key, first); } // [#24720] The rule's root verdict, over every predicate it carries, // located where `check()` above located the same predicate. `parent` is @@ -3004,12 +3039,16 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression ? `object '${objectName}' · ${p.label}` : p.slot === 'when' ? `${where} when` : where; // `error`: a rule the runtime cannot evaluate must not pass a door; see the docblock. + first = issues.length; issues.push({ where: at, message, source: source!, severity: 'error' }); + atSlot(p.key, first); } // #4763 — null-guard gate over every predicate the rule carries, nested // `then`/`otherwise` branches included. for (const p of predicates) { + first = issues.length; checkNullGuards(`object '${objectName}' · ${p.label}`, p.label, p.raw, objectName); + atSlot(p.key, first); } } // Field-level formulas (computed fields) reference the same object. diff --git a/packages/lint/src/validate-expressions.validation-rule-path.test.ts b/packages/lint/src/validate-expressions.validation-rule-path.test.ts new file mode 100644 index 00000000000..6005eeafa0a --- /dev/null +++ b/packages/lint/src/validate-expressions.validation-rule-path.test.ts @@ -0,0 +1,145 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #27643 — an object validation rule's expression findings carry the slot's + * key path. + * + * `expression-invalid` is what publish refuses a validation rule's CEL with, + * and its finding was located by its `where` prose alone (`object 'X' · + * validation rule 'NAME'`). A draft's `_diagnostics` is item-relative and + * dotted (`validations.1.condition`), so Studio's rule editor could not ask + * whether the refusal was its own. Every finding the validation-rule pass + * pushes — `check()`, the `parent` root verdict and the null-guard gate, on + * the rule's `condition` and `when` and on a `conditional` rule's nested + * branches — now carries the key path in the spelling a flow edge's findings + * use (#26858), beside the unchanged `where` and message. + */ + +import { describe, expect, it } from 'vitest'; + +import { runRuntimeAuthoringRules } from './runtime-gate.js'; +import { validateStackExpressions } from './validate-expressions.js'; + +type AnyRec = Record; + +/** `amount` is nullable (no `required`, no `defaultValue`), so a bare compare on it is a null-guard finding. */ +function obj(validations: unknown, extra: AnyRec = {}): AnyRec { + return { + name: 'rule_probe', + label: 'Rule Probe', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + amount: { type: 'number', label: 'Amount' }, + }, + validations, + ...extra, + }; +} + +const CLEAN = { name: 'ok_rule', type: 'script', condition: 'record.name != null', message: 'ok' }; +/** A null-guard finding on the rule's own `condition`. */ +const NULL_UNSAFE = { name: 'amount_rule', type: 'script', condition: 'record.amount > 100', message: 'x' }; +/** A `check()` finding (bare reference) on the rule's own `condition`. */ +const BARE = { name: 'amount_bare', type: 'script', condition: 'amount > 1', message: 'x' }; + +const at = (issues: ReturnType, fragment: string) => + issues.filter((i) => i.where.includes(fragment)); + +describe('a validation rule\'s expression finding carries a key path (#27643)', () => { + it('null-guard gate on `condition`: indexed over the authored lists, where and message unchanged', () => { + const issues = validateStackExpressions({ objects: [obj([CLEAN, NULL_UNSAFE])] }); + const onRule = at(issues, "'amount_rule'"); + expect(onRule).toHaveLength(1); + expect(onRule[0]).toMatchObject({ + path: 'objects[0].validations[1].condition', + where: "object 'rule_probe' · validation rule 'amount_rule'", + }); + expect(onRule[0]!.message).toContain("validation rule 'amount_rule' applies `>` to `record.amount`"); + }); + + it('check() on `condition`: the bare-reference finding carries the same path', () => { + const issues = validateStackExpressions({ objects: [obj([BARE])] }); + expect(issues).toHaveLength(1); + expect(issues[0]).toMatchObject({ + path: 'objects[0].validations[0].condition', + where: "object 'rule_probe' · validation 'amount_bare'", + }); + expect(issues[0]!.message).toContain('bare reference `amount`'); + }); + + it('`when`: check() and the null-guard gate both carry `.when`', () => { + const gate = { + type: 'conditional', name: 'gate', message: 'x', when: 'sqrt(record.amount) > 1 && record.amount > 5', + then: { type: 'script', name: 'child', condition: 'record.amount != null && record.amount > 100', message: 'y' }, + }; + const issues = validateStackExpressions({ objects: [obj([gate])] }); + const viaCheck = issues.filter((i) => i.where === "object 'rule_probe' · validation 'gate' when"); + const viaNullGuard = issues.filter((i) => i.where === "object 'rule_probe' · validation rule 'gate' when-predicate"); + expect(viaCheck.length).toBeGreaterThan(0); + expect(viaNullGuard.length).toBeGreaterThan(0); + for (const i of [...viaCheck, ...viaNullGuard]) expect(i.path).toBe('objects[0].validations[0].when'); + expect(issues).toHaveLength(viaCheck.length + viaNullGuard.length); + }); + + it('nested `then` / `otherwise` branches: the path walks the branch keys', () => { + const gate = { + type: 'conditional', name: 'outer', message: 'x', when: 'record.name != null', + then: { type: 'script', name: 'inner', condition: 'record.amount < 1000', message: 'y' }, + otherwise: { + type: 'conditional', name: 'mid', message: 'm', when: 'record.amount > 5', + then: { type: 'script', name: 'deep', condition: 'sqrt(record.amount) > 1', message: 'd' }, + }, + }; + const issues = validateStackExpressions({ objects: [obj([gate])] }); + const byWhere = Object.fromEntries(issues.map((i) => [i.where, i.path])); + expect(byWhere).toEqual({ + "object 'rule_probe' · validation rule 'outer' then → 'inner'": 'objects[0].validations[0].then.condition', + "object 'rule_probe' · validation rule 'outer' otherwise → 'mid' when-predicate": 'objects[0].validations[0].otherwise.when', + "object 'rule_probe' · validation rule 'outer' otherwise → 'mid' then → 'deep'": 'objects[0].validations[0].otherwise.then.condition', + }); + }); + + it('the `parent` root verdict carries the slot path too', () => { + const rule = { name: 'paid_lock', type: 'script', condition: "parent.status == 'paid'", message: 'x' }; + const issues = validateStackExpressions({ objects: [obj([rule])] }); + const parentIssues = issues.filter((i) => i.message.includes('`parent`')); + expect(parentIssues.length).toBeGreaterThan(0); + for (const i of parentIssues) { + expect(i).toMatchObject({ path: 'objects[0].validations[0].condition', where: "object 'rule_probe' · validation 'paid_lock'" }); + } + }); + + it('indexes the raw lists: a null member before the object or the rule does not renumber it', () => { + const issues = validateStackExpressions({ objects: [null, obj([null, NULL_UNSAFE])] }); + const onRule = at(issues, "'amount_rule'"); + expect(onRule).toHaveLength(1); + expect(onRule[0]!.path).toBe('objects[1].validations[1].condition'); + }); + + it('a map-form `objects` collection has no index to spell, so the finding keeps where alone', () => { + const { name: _name, ...body } = obj([NULL_UNSAFE]); + const issues = validateStackExpressions({ objects: { rule_probe: body } }); + const onRule = at(issues, "'amount_rule'"); + expect(onRule).toHaveLength(1); + expect(onRule[0]!.path).toBeUndefined(); + }); + + it('a finding off a validation rule gains no path', () => { + const issues = validateStackExpressions({ + objects: [obj([], { fields: { amount: { type: 'formula', label: 'F', expression: 'amount * 2' } } })], + }); + expect(issues.length).toBeGreaterThan(0); + for (const i of issues) expect(i.path).toBeUndefined(); + }); + + it('reaches the publish gate: located at the rule\'s key path, the top-level index re-spelled by name', () => { + const result = runRuntimeAuthoringRules({ type: 'object', item: obj([CLEAN, NULL_UNSAFE]), context: { objects: [] } }); + const refusals = result.errors.filter((f) => f.rule === 'expression-invalid'); + expect(refusals).toHaveLength(1); + expect(refusals[0]).toMatchObject({ + path: 'objects.rule_probe.validations[1].condition', + where: "object 'rule_probe' · validation rule 'amount_rule'", + }); + }); +}); From 7363e13f347c8db1a808cf311114752cdaedfa62 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:37:32 +0000 Subject: [PATCH 2/3] test: validation-rule findings move from the prose path to the key path Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WGufzj2PNcHjESYvrviQf7 --- ...e.object-field-existence-spellings.test.ts | 12 +- ...time-gate.object-validation-writes.test.ts | 34 ++- packages/lint/src/validate-expressions.ts | 2 +- ...expressions.validation-rule-parent.test.ts | 10 +- .../protocol.runtime-authoring-gate.test.ts | 60 +++- .../src/protocol.zz-probe-27643.test.ts | 267 ++++++++++++++++++ 6 files changed, 363 insertions(+), 22 deletions(-) create mode 100644 packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts diff --git a/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts b/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts index d24f263e978..5a3483ccea2 100644 --- a/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts +++ b/packages/lint/src/runtime-gate.object-field-existence-spellings.test.ts @@ -66,6 +66,16 @@ const WHERE: Readonly> = { 'validation condition': "object 'fx_probe' · validation 'probe_rule'", }; +/** + * The door finding's `path`: the `where` prose on the two field slots, and on + * the validation rule (#27643) the judged slot's key path, its top-level index + * re-spelled by name at the door. + */ +const DOOR_PATH: Readonly> = { + ...WHERE, + 'validation condition': 'objects.fx_probe.validations[0].condition', +}; + const SLOTS = Object.keys(WHERE) as Slot[]; /** Every member spelling that names `f`; the dot row is the one that was already refused. */ @@ -103,7 +113,7 @@ describe('an undeclared field is refused in every member spelling, at both doors expect(door.rulesRun).toContain('validateStackExpressions'); const errs = expressionFindings(door.errors); expect(errs, dump(door)).toHaveLength(1); - expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE[slot], path: WHERE[slot] }); + expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE[slot], path: DOOR_PATH[slot] }); expect(errs[0]!.message).toContain('unknown field `zz_typo`'); }); } diff --git a/packages/lint/src/runtime-gate.object-validation-writes.test.ts b/packages/lint/src/runtime-gate.object-validation-writes.test.ts index 2d25a3c69eb..df0ebe3a6a1 100644 --- a/packages/lint/src/runtime-gate.object-validation-writes.test.ts +++ b/packages/lint/src/runtime-gate.object-validation-writes.test.ts @@ -111,6 +111,15 @@ const buildFindings = (body: unknown) => { const dump = (r: unknown) => JSON.stringify(r, null, 2); +/** + * [#27643] The build's findings as the door spells them: a validation rule's + * finding carries the judged slot's key path, and the door re-spells its + * top-level index by name (`objects[0]` → `objects.fx_rule`); every other key + * is the build's own. + */ +const asAtDoor = (fs: readonly T[]): T[] => + fs.map((f) => ({ ...f, path: f.path.replace(/^objects\[0\]/, 'objects.fx_rule') })); + describe('#22032 pass 1 — the object door gives the build\'s validation-rule verdict', () => { it('needs no registry change: `validateStackExpressions` is already on the object door', () => { expect(runtimeAuthoringRulesFor('object').map((r) => r.name)).toContain('validateStackExpressions'); @@ -123,7 +132,7 @@ describe('#22032 pass 1 — the object door gives the build\'s validation-rule v const errs = expressionFindings(result.errors); const where = "object 'fx_rule' · validation 'amount_root'"; expect(errs, dump(result)).toHaveLength(1); - expect(errs[0]).toMatchObject({ severity: 'error', where, path: where }); + expect(errs[0]).toMatchObject({ severity: 'error', where, path: 'objects.fx_rule.validations[0].condition' }); expect(errs[0]!.message).toContain('`sqrt` is not a callable name here'); }); @@ -171,7 +180,7 @@ describe('#22032 pass 1 — the object door gives the build\'s validation-rule v // Non-vacuous: the build refuses each of them. expect(atBuild.length, dump(rule)).toBeGreaterThan(0); - expect(atDoor, dump(rule)).toEqual(atBuild); + expect(atDoor, dump(rule)).toEqual(asAtDoor(atBuild)); } }); @@ -265,7 +274,11 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver 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.map((f) => f.path), dump(atBuild)).toEqual([ + 'objects[0].validations[0].then.condition', + 'objects[0].validations[0].otherwise.condition', + ]); + for (const f of atBuild) expect(f).toMatchObject({ severity: 'error' }); expect(atBuild[0]!.message).toContain('`sqrt` is not a callable name here'); expect(atBuild[1]!.message).toContain('bare reference `amont`'); }); @@ -276,7 +289,7 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver 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]))); + expect(atDoor).toEqual(asAtDoor(buildFindings(fxRule([NESTED_REFUSED])))); }); it('⭐ LIT — two levels down: a nested `conditional`\'s `when` and the rule below it are judged', () => { @@ -288,7 +301,11 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver ]); 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); + expect(atBuild.map((f) => f.path), dump(atBuild)).toEqual([ + 'objects[0].validations[0].then.when', + 'objects[0].validations[0].then.then.condition', + ]); + expect(expressionFindings(gateObject(fxRule([TWO_LEVEL])).errors)).toEqual(asAtDoor(atBuild)); }); it('⭐ CONTROL — valid nested predicates publish clean, two levels down and in `otherwise`', () => { @@ -326,7 +343,7 @@ describe('#22042 — a `conditional` rule\'s nested predicates meet the same ver const topWhen = buildFindings(fxRef([{ ...HYDRATION, when: MULTI_HOP, then: NESTED_VALID.otherwise, otherwise: undefined }])); 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); + expect(expressionFindings(gateObject(fxRef([HYDRATION])).errors)).toEqual(asAtDoor(atBuild)); }); it('a stored sibling\'s nested fault is not this write\'s to answer for (the differential)', () => { @@ -364,7 +381,8 @@ describe('#24468 — the object door refuses a rule hop to a field the target do const errs = expressionFindings(result.errors); expect(errs, dump(result)).toHaveLength(1); const where = "object 'fx_order' · validation 'north_only'"; - expect(errs[0]).toMatchObject({ severity: 'error', where, path: where }); + // [#27643] The second rule's `condition`, by key path; `where` stays the prose. + expect(errs[0]).toMatchObject({ severity: 'error', where, path: 'objects.fx_order.validations[1].condition' }); expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`'); expect(errs[0]!.message).toContain('the rule has no verdict'); }); @@ -428,7 +446,7 @@ describe('#24512 — the object door judges a `conditional` rule\'s `when` hop a 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]).toMatchObject({ rule: EXPRESSION_INVALID, severity: 'error', where, path: 'objects.fx_order.validations[0].when' }); expect(errs[0]!.message).toContain('unknown field `region` on `fx_account`'); expect(errs[0]!.message).toContain('the rule has no verdict'); }); diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 374859c9a61..a4aad9daff4 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -2985,7 +2985,7 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // author wrote, in the spelling a flow edge's findings use (#26858), so // a draft's `_diagnostics` reports it item-relative // (`validations.1.condition`). `where` stays the human locator. - const ruleIndex = objectIndex >= 0 && Array.isArray(validations) ? validations.indexOf(rule) : -1; + const ruleIndex = objectIndex >= 0 && Array.isArray(obj.validations) ? obj.validations.indexOf(rule) : -1; const rulePath = ruleIndex >= 0 ? `objects[${objectIndex}].validations[${ruleIndex}]` : undefined; const atSlot = (key: string, first: number): void => { if (!rulePath) return; diff --git a/packages/lint/src/validate-expressions.validation-rule-parent.test.ts b/packages/lint/src/validate-expressions.validation-rule-parent.test.ts index 6c84825acf7..7c1f8f04629 100644 --- a/packages/lint/src/validate-expressions.validation-rule-parent.test.ts +++ b/packages/lint/src/validate-expressions.validation-rule-parent.test.ts @@ -196,7 +196,8 @@ describe('#24720 — the object door gives the build\'s verdict', () => { expect(result.rulesRun).toContain('validateStackExpressions'); const errs = expressionFindings(result.errors); expect(errs, dump(result)).toHaveLength(1); - expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE, path: WHERE }); + // [#27643] Located at the judged slot's key path; `where` stays the prose. + expect(errs[0]).toMatchObject({ severity: 'error', where: WHERE, path: 'objects.fx_line.validations[0].condition' }); expect(errs[0]!.message).toContain("validation rule 'paid_lock' reads `parent`"); // No longer an advisory: the finding moved, it was not duplicated. expect(expressionFindings(result.advisories), dump(result)).toEqual([]); @@ -207,7 +208,12 @@ describe('#24720 — the object door gives the build\'s verdict', () => { const atDoor = expressionFindings(gateObject(line([script(PARENT)]), [header]).errors); expect(atBuild, dump(atBuild)).toHaveLength(1); - expect(atDoor, dump(atDoor)).toEqual(atBuild); + // [#27643] Equal once the door's by-name spelling of the top-level index + // is applied to the build's positional key path (`objects[1]`, after the header). + expect(atBuild[0]!.path).toBe('objects[1].validations[0].condition'); + expect(atDoor, dump(atDoor)).toEqual( + atBuild.map((f) => ({ ...f, path: f.path.replace(/^objects\[1\]/, 'objects.fx_line') })), + ); }); it('⭐ CONTROL — `parent` on a field\'s own `readonlyWhen` publishes clean, and the build agrees', () => { diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index 37f4a437086..46c097101db 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -1800,6 +1800,13 @@ describe('runtime authoring gate on OBJECT writes — the validation-rule verdic const VALID = 'record.amount != null && record.amount > 100'; /** Where the build locates a validation-rule finding — the rule the author edits. */ const WHERE = "object 'fx_rule' · validation 'amount_rule'"; + /** + * [#27643] The finding's key path: the judged slot, not the `WHERE` prose. + * The door re-spells the top-level index by name (`nameKeyFindingPath`); + * the draft read's `_diagnostics` spells it item-relative and dotted. + */ + const PATH = 'objects.fx_rule.validations[0].condition'; + const DIAGNOSTICS_PATH = 'validations.0.condition'; const ruleRows = (rows: Map) => Array.from(rows.values()).filter((r) => r.type === 'object' && r.name === 'fx_rule'); @@ -1825,7 +1832,7 @@ describe('runtime authoring gate on OBJECT writes — the validation-rule verdic expect(err.rulesRun).toContain('validateStackExpressions'); const issue = err.issues.find((i: any) => i.rule === EXPRESSION_INVALID); expect(issue, `issues: ${JSON.stringify(err.issues)}`).toBeDefined(); - expect(issue.path).toBe(WHERE); + expect(issue.path).toBe(PATH); expect(issue.where).toBe(WHERE); expect(issue.severity).toBe('error'); // The named subject: what the author typed, as the build names it. @@ -1848,7 +1855,25 @@ describe('runtime authoring gate on OBJECT writes — the validation-rule verdic expect(err.code).toBe('INVALID_METADATA'); const issue = err.issues.find((i: any) => i.rule === EXPRESSION_INVALID); expect(issue, `issues: ${JSON.stringify(err.issues)}`).toBeDefined(); - expect(issue.path).toBe(WHERE); + expect(issue.path).toBe(PATH); + expect(issue.where).toBe(WHERE); + }); + + it("[#27643] the draft read's `_diagnostics` locates each refusal at the rule's `condition`, item-relative", async () => { + for (const { condition } of REFUSED) { + const { protocol } = makeProtocol(); + await expect( + protocol.saveMetaItem({ type: 'object', name: 'fx_rule', item: fxRule(condition), mode: 'draft' }), + ).resolves.toMatchObject({ success: true }); + + const read: any = await protocol.getMetaItem({ type: 'object', name: 'fx_rule', state: 'draft' }); + const diagnostics = read?.item?._diagnostics; + + expect(diagnostics?.valid, condition).toBe(false); + const entries = (diagnostics?.errors ?? []).filter((e: any) => e.code === EXPRESSION_INVALID); + expect(entries, `${condition}: ${JSON.stringify(diagnostics)}`).toHaveLength(1); + expect(entries[0].path).toBe(DIAGNOSTICS_PATH); + } }); it('(b) a valid, guarded `condition` still saves, and the row lands', async () => { @@ -1876,9 +1901,13 @@ describe('runtime authoring gate on OBJECT writes — the validation-rule verdic expect(atDoor, condition).toHaveLength(1); // Compared key by key — the door reuses the build's call, so a reworded // or relocated door verdict is a second dialect, and red. - for (const key of ['rule', 'where', 'path', 'message', 'hint'] as const) { + for (const key of ['rule', 'where', 'message', 'hint'] as const) { expect(atDoor[0][key], `door and build disagree on '${key}' for ${condition}`).toBe(atBuild[0]![key]); } + // [#27643] The path too, once the door's by-name spelling of the + // top-level index is applied to the build's positional one. + expect(atBuild[0]!.path, condition).toBe('objects[0].validations[0].condition'); + expect(atDoor[0].path, condition).toBe(atBuild[0]!.path.replace(/^objects\[0\]/, 'objects.fx_rule')); } // And the valid rule is clean at the build too, not just at the door. expect(buildFindings(fxRule(VALID))).toEqual([]); @@ -2778,12 +2807,14 @@ describe('runtime authoring gate on OBJECT writes — a conditional rule\'s nest label: 'an unregistered function in `then`', rule: outer({ then: { type: 'script', name: 'inner', condition: 'sqrt(record.amount) > 1', message: 'I' }, otherwise: GUARDED }), where: "object 'fx_nested' · validation rule 'outer' then → 'inner'", + path: 'objects.fx_nested.validations[0].then.condition', subject: '`sqrt` is not a callable name here', }, { label: 'a bare field in `otherwise`', rule: outer({ then: GUARDED, otherwise: { type: 'script', name: 'other', condition: 'amont > 1', message: 'O' } }), where: "object 'fx_nested' · validation rule 'outer' otherwise → 'other'", + path: 'objects.fx_nested.validations[0].otherwise.condition', subject: 'bare reference `amont`', }, { @@ -2795,6 +2826,7 @@ describe('runtime authoring gate on OBJECT writes — a conditional rule\'s nest }, }), where: "object 'fx_nested' · validation rule 'outer' then → 'mid' then → 'deep'", + path: 'objects.fx_nested.validations[0].then.then.condition', subject: '`sqrt` is not a callable name here', }, ] as const; @@ -2816,7 +2848,7 @@ describe('runtime authoring gate on OBJECT writes — a conditional rule\'s nest .filter((f) => f.rule === EXPRESSION_INVALID); }; - for (const { label, rule, where, subject } of REFUSED) { + for (const { label, rule, where, path, subject } of REFUSED) { it(`(a) REFUSES an active save of ${label} with a 422 carrying the build's finding at the nested rule`, async () => { const { protocol, rows } = makeProtocol(); @@ -2830,7 +2862,8 @@ describe('runtime authoring gate on OBJECT writes — a conditional rule\'s nest expect(err.rulesRun).toContain('validateStackExpressions'); const issues = err.issues.filter((i: any) => i.rule === EXPRESSION_INVALID); expect(issues, `issues: ${JSON.stringify(err.issues)}`).toHaveLength(1); - expect(issues[0].path).toBe(where); + // [#27643] Located at the nested slot's key path; `where` stays the prose. + expect(issues[0].path).toBe(path); expect(issues[0].where).toBe(where); expect(issues[0].severity).toBe('error'); // The named subject: what the author typed, as the build names it. @@ -2853,7 +2886,7 @@ describe('runtime authoring gate on OBJECT writes — a conditional rule\'s nest expect(err.code).toBe('INVALID_METADATA'); const issue = err.issues.find((i: any) => i.rule === EXPRESSION_INVALID); expect(issue, `issues: ${JSON.stringify(err.issues)}`).toBeDefined(); - expect(issue.path).toBe(REFUSED[0].where); + expect(issue.path).toBe(REFUSED[0].path); }); it('(b) valid nested predicates — both branches, two levels down — still save, and the row lands', async () => { @@ -2881,9 +2914,11 @@ describe('runtime authoring gate on OBJECT writes — a conditional rule\'s nest expect(atDoor, label).toHaveLength(1); // Compared key by key — the door reuses the build's call, so a reworded // or relocated door verdict is a second dialect, and red. - for (const key of ['rule', 'where', 'path', 'message', 'hint'] as const) { + for (const key of ['rule', 'where', 'message', 'hint'] as const) { expect(atDoor[0][key], `door and build disagree on '${key}' for ${label}`).toBe(atBuild[0]![key]); } + // [#27643] The path too, once the door's by-name spelling of the top-level index is applied. + expect(atDoor[0].path, label).toBe(atBuild[0]!.path.replace(/^objects\[0\]/, 'objects.fx_nested')); } // And the valid rule is clean at the build too, not just at the door. expect(buildFindings(fxNested(VALID))).toEqual([]); @@ -4185,6 +4220,8 @@ describe('runtime authoring gate on OBJECT writes — a validations[] predicate const rule = (condition: string) => ({ name: 'paid_lock', type: 'script', condition, message: 'Locked.' }); /** Where the build locates it — the rule the author edits. */ const WHERE = "object 'fx_line' · validation 'paid_lock'"; + /** [#27643] The judged slot's key path, the top-level index re-spelled by name at the door. */ + const PATH = 'objects.fx_line.validations[0].condition'; const hostWithHeader = () => { const { engine, rows } = makeStubEngine(); @@ -4206,7 +4243,8 @@ describe('runtime authoring gate on OBJECT writes — a validations[] predicate expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); const issues = err.issues.filter((i: any) => i.rule === EXPRESSION_INVALID); expect(issues, `issues: ${JSON.stringify(err.issues)}`).toHaveLength(1); - expect(issues[0].path).toBe(WHERE); + expect(issues[0].path).toBe(PATH); + expect(issues[0].where).toBe(WHERE); expect(issues[0].severity).toBe('error'); expect(issues[0].message).toContain("validation rule 'paid_lock' reads `parent`"); expect(lineRows(rows)).toEqual([]); @@ -4216,9 +4254,11 @@ describe('runtime authoring gate on OBJECT writes — a validations[] predicate const atBuild = runAuthoringRules('build', { normalized: stack, parsed: stack }) .filter((f) => f.rule === EXPRESSION_INVALID); expect(atBuild).toHaveLength(1); - for (const key of ['rule', 'where', 'path', 'message', 'severity'] as const) { + for (const key of ['rule', 'where', 'message', 'severity'] as const) { expect(issues[0][key], `door and build disagree on '${key}'`).toBe(atBuild[0]![key]); } + // [#27643] The build spells the line at its index in this stack; the door by name. + expect(atBuild[0]!.path).toBe('objects[1].validations[0].condition'); }); it('(b) the same body still saves as a DRAFT, and its PROMOTION is refused', async () => { @@ -4232,7 +4272,7 @@ describe('runtime authoring gate on OBJECT writes — a validations[] predicate expect({ code: err?.code, status: err?.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); const issue = err.issues.find((i: any) => i.rule === EXPRESSION_INVALID); expect(issue, `issues: ${JSON.stringify(err.issues)}`).toBeDefined(); - expect(issue.path).toBe(WHERE); + expect(issue.path).toBe(PATH); expect(lineRows(rows).map((r) => r.state)).toEqual(['draft']); }); diff --git a/packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts b/packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts new file mode 100644 index 00000000000..8512885504d --- /dev/null +++ b/packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts @@ -0,0 +1,267 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #26707 — a draft's `_diagnostics` says what its publish will say. + * + * ## The defect, measured on `origin/main` before this file was written + * + * Six author-time rules ran only at publish. Each of the bodies below saved as + * a draft (200), its draft read answered `_diagnostics: { valid: true }`, and + * the publish refused it 422 `INVALID_METADATA` with the rule id in + * `issues[]`. In a package publish the refusal rolled every other draft of + * the package back with it, so an author who trusted "valid" lost unrelated + * work far from the edit that caused it. + * + * ## What is pinned + * + * - The draft still SAVES (#4463 D1 is unchanged): every fixture is stored. + * - Its read reports `valid: false` with the publish's own rule id (as the + * entry's `code`) and the publish's own message — compared against the + * publish run in the same test, never against a restatement of it. + * - The publish still refuses exactly as before. + * - A clean draft stays `valid: true`, and a draft whose reference resolves + * only inside its own package (the closure "Publish all" judges) is not + * reported: the verdict is the package publish's, not a stricter one. + * + * Harness: the package-aware multi-table stub engine of + * `protocol-publish-drafts-closure.test.ts` (kept local, the established + * shape). Nothing on the save, read or publish path is stubbed. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** The LIVE object universe: a deal with a formula field, and its account. */ +const liveDealObject = { + name: 'deal', + label: 'Deal', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + amount: { type: 'number', label: 'Amount' }, + deal_score: { + type: 'formula', + label: 'Score', + expression: { dialect: 'cel', source: 'record.amount * 2' }, + returnType: 'number', + }, + }, +}; + +/** The LIVE dataset a fixture dashboard binds. */ +const liveDealDataset = { + name: 'deals_live', + label: 'Deals', + object: 'deal', + dimensions: [{ name: 'deal_name', field: 'name' }], + measures: [{ name: 'deal_count', aggregate: 'count' }], +}; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; + updated_at?: string; + created_at?: string; +} + +interface HistoryRow { + id: string; + event_seq: number; + name: string; + type: string; + version: number; + operation_type: string; + metadata: string | null; + checksum: string | null; + previous_checksum: string | null; + change_note?: string | null; + source?: string | null; + organization_id: string | null; + recorded_by?: string | null; + recorded_at: string; +} + +// Overlay rows are keyed by (type, name, org, state, package_id) — the ADR-0048 key. +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +function matchesMetadataWhere(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (k === '$or') { + const clauses = v as Array>; + if (!clauses.some((c) => matchesMetadataWhere(r, c))) return false; + continue; + } + if (v === undefined) continue; + // The active read asks `type: { $in: [type, plural] }`; no other + // operator reaches this stub. + if (v && typeof v === 'object' && Array.isArray((v as { $in?: unknown }).$in)) { + if (!(v as { $in: unknown[] }).$in.includes((r as any)[k])) return false; + continue; + } + if ((r as any)[k] !== v) return false; + } + return true; +} + +function makeStubEngine() { + const rows = new Map(); + const historyRows: HistoryRow[] = []; + let nextId = 0; + + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + if (w.package_id !== undefined) { + const k = keyOf(w); + const r = rows.get(k); + return r ? { key: k, row: r } : null; + } + for (const [k, r] of rows) if (matchesMetadataWhere(r, w)) return { key: k, row: r }; + return null; + }; + + const matchesHistory = (h: HistoryRow, w: Record): boolean => { + if (w.organization_id !== undefined && h.organization_id !== w.organization_id) return false; + if (w.type !== undefined && h.type !== w.type) return false; + if (w.name !== undefined && h.name !== w.name) return false; + if (w.version !== undefined && h.version !== w.version) return false; + if (w.operation_type !== undefined && h.operation_type !== w.operation_type) return false; + return true; + }; + + const engine: any = { + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + if (table === 'sys_metadata_history') { + return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; + } + return findRow(opts.where)?.row ?? null; + }, + async find(table: string, opts: { where: Record; limit?: number }) { + const matched = table === 'sys_metadata_history' + ? historyRows.filter((h) => matchesHistory(h, opts.where)) + : Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; + if (table === 'sys_metadata_history') { + nextId += 1; + const h: HistoryRow = { id: `h_${nextId}`, ...(data as any) }; + historyRows.push(h); + return { id: h.id }; + } + nextId += 1; + const row = { id: `r_${nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + // The engine's judge-only filter admission (#20158), as the live engine + // answers it for a read scope on a formula field: refused, because a + // virtual field has no column a row filter can read. The engine's real + // verdicts are pinned in `packages/cli/test/rls-policy-authoring-admission.test.ts`. + judgeFilter: (_object: string, where: unknown) => + JSON.stringify(where).includes('deal_score') + ? { ok: false, code: 'FILTER_FIELD_NOT_STORED', status: 400, message: 'Field "deal_score" is a formula field and cannot be filtered.' } + : { ok: true }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + // The LIVE universe: the objects and the one dataset the fixtures + // resolve against. + listItems: (type: string) => { + if (type === 'object') return [liveDealObject]; + if (type === 'dataset') return [liveDealDataset]; + return []; + }, + // The base the drafts are bound to is INSTALLED — a flow may be + // saved only into a package the registry holds (#20863) — and its + // manifest declares no namespace → the ADR-0028 prefix pre-flight is + // skipped (legacy-grandfathered path); it declares no dependencies + // and no live object carries a package stamp, so + // `resolveWritePackageScope`'s closure removes nothing. + getPackage: (id: string) => (id === PKG ? { manifest: { id: PKG } } : undefined), + }, + }; + return { engine, rows, historyRows }; +} +const PKG = 'app.sales'; + + +import { appendFileSync } from 'node:fs'; +const OUT = process.env.PROBE_OUT!; +async function stageDraft(protocol: ObjectStackProtocolImplementation, type: string, item: { name: string }) { + return (protocol as any).saveMetaItem({ type, name: item.name, item, packageId: PKG, mode: 'draft' }); +} +const RULE_OBJ = { + name: 'rule_probe', + label: 'Rule Probe', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + amount: { type: 'number', label: 'Amount' }, + }, + validations: [ + { name: 'ok_rule', type: 'script', condition: 'record.name != null', message: 'ok' }, + { name: 'amount_rule', type: 'script', condition: 'record.amount > 100', message: 'Amount rule' }, + { name: 'cond_rule', type: 'conditional', when: 'record.amount > 5', + then: { name: 'inner', type: 'script', condition: 'record.amount < 1000', message: 'inner' }, message: 'c' }, + ], +}; +const FLOW = { + name: 'edge_cel', label: 'Edge CEL', type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'decide', type: 'decision', label: 'Decide' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'decide' }, + { id: 'e2', source: 'decide', target: 'end', type: 'conditional', condition: 'record.name ==' }, + ], +}; +describe('probe 27643', () => { + for (const [type, item] of [['object', RULE_OBJ], ['flow', FLOW]] as const) { + it(type, async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, type, item as any); + const read: any = await protocol.getMetaItem({ type, name: item.name, state: 'draft', packageId: PKG } as any); + const refusal: any = await protocol.publishMetaItem({ type, name: item.name, packageId: PKG } as any).then(() => null, (e: unknown) => e); + appendFileSync(OUT, JSON.stringify({ type, diagnostics: read.item._diagnostics, + publish422: refusal && { status: refusal.status, code: refusal.code, + issues: refusal.issues.map((i: any) => ({ rule: i.rule, path: i.path, where: i.where, message: i.message.slice(0, 160) })) } }, null, 1) + '\n'); + }); + } +}); From 9d9987f555e1a52d3b52bc8670e20376cd5c61af Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 16:40:03 +0000 Subject: [PATCH 3/3] chore: changeset for the validation-rule finding key path Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WGufzj2PNcHjESYvrviQf7 --- .../27643-validation-rule-finding-path.md | 16 ++ .../src/protocol.zz-probe-27643.test.ts | 267 ------------------ 2 files changed, 16 insertions(+), 267 deletions(-) create mode 100644 .changeset/27643-validation-rule-finding-path.md delete mode 100644 packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts diff --git a/.changeset/27643-validation-rule-finding-path.md b/.changeset/27643-validation-rule-finding-path.md new file mode 100644 index 00000000000..8587624ed97 --- /dev/null +++ b/.changeset/27643-validation-rule-finding-path.md @@ -0,0 +1,16 @@ +--- +'@objectstack/lint': patch +--- + +fix(lint): a validation rule's `expression-invalid` finding carries the slot's key path (`objects[0].validations[1].condition`) beside its `where` text + +Clause-②: no +Metadata-protocol: none + +An `expression-invalid` finding on an object validation rule's predicate used to be located by its `where` prose alone (`object 'invoice' · validation rule 'amount_rule'`), which is also what its `path` carried, so a reader keyed on paths could not address it. Its `path` is now the judged slot's key path in the spelling a flow edge's findings use, so a draft's `_diagnostics[].path` reports it item-relative (`validations.1.condition`, `validations.2.when`, `validations.2.then.condition`) and the publish refusal's `issues[].path` reads `objects.invoice.validations[1].condition`. + +- `where`, the message and the verdict are unchanged: nothing is accepted or refused differently. +- Every finding of the validation-rule pass moves: the expression check, the `parent` root verdict and the null-guard gate, on a rule's `condition` and `when` and on a `conditional` rule's nested `then` / `otherwise` branches. +- The object and the rule are indexed in the lists as authored (a `null` member before them does not renumber them). +- An object in a map-form `objects` collection keeps `where` as its `path`, as before. +- `os lint` prints the key path after `at` for these findings, where it printed the `where` text a second time. diff --git a/packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts b/packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts deleted file mode 100644 index 8512885504d..00000000000 --- a/packages/metadata-protocol/src/protocol.zz-probe-27643.test.ts +++ /dev/null @@ -1,267 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * #26707 — a draft's `_diagnostics` says what its publish will say. - * - * ## The defect, measured on `origin/main` before this file was written - * - * Six author-time rules ran only at publish. Each of the bodies below saved as - * a draft (200), its draft read answered `_diagnostics: { valid: true }`, and - * the publish refused it 422 `INVALID_METADATA` with the rule id in - * `issues[]`. In a package publish the refusal rolled every other draft of - * the package back with it, so an author who trusted "valid" lost unrelated - * work far from the edit that caused it. - * - * ## What is pinned - * - * - The draft still SAVES (#4463 D1 is unchanged): every fixture is stored. - * - Its read reports `valid: false` with the publish's own rule id (as the - * entry's `code`) and the publish's own message — compared against the - * publish run in the same test, never against a restatement of it. - * - The publish still refuses exactly as before. - * - A clean draft stays `valid: true`, and a draft whose reference resolves - * only inside its own package (the closure "Publish all" judges) is not - * reported: the verdict is the package publish's, not a stricter one. - * - * Harness: the package-aware multi-table stub engine of - * `protocol-publish-drafts-closure.test.ts` (kept local, the established - * shape). Nothing on the save, read or publish path is stubbed. - */ - -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; -import { ObjectStackProtocolImplementation } from './protocol.js'; - -/** The LIVE object universe: a deal with a formula field, and its account. */ -const liveDealObject = { - name: 'deal', - label: 'Deal', - sharingModel: 'private', - fields: { - name: { type: 'text', label: 'Name' }, - amount: { type: 'number', label: 'Amount' }, - deal_score: { - type: 'formula', - label: 'Score', - expression: { dialect: 'cel', source: 'record.amount * 2' }, - returnType: 'number', - }, - }, -}; - -/** The LIVE dataset a fixture dashboard binds. */ -const liveDealDataset = { - name: 'deals_live', - label: 'Deals', - object: 'deal', - dimensions: [{ name: 'deal_name', field: 'name' }], - measures: [{ name: 'deal_count', aggregate: 'count' }], -}; - -interface Row { - id: string; - type: string; - name: string; - organization_id: string | null; - package_id: string | null; - state: string; - metadata: string; - checksum?: string; - version?: number; - updated_at?: string; - created_at?: string; -} - -interface HistoryRow { - id: string; - event_seq: number; - name: string; - type: string; - version: number; - operation_type: string; - metadata: string | null; - checksum: string | null; - previous_checksum: string | null; - change_note?: string | null; - source?: string | null; - organization_id: string | null; - recorded_by?: string | null; - recorded_at: string; -} - -// Overlay rows are keyed by (type, name, org, state, package_id) — the ADR-0048 key. -function keyOf(w: Record) { - return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; -} - -function matchesMetadataWhere(r: Row, where: Record): boolean { - for (const [k, v] of Object.entries(where)) { - if (k === '$or') { - const clauses = v as Array>; - if (!clauses.some((c) => matchesMetadataWhere(r, c))) return false; - continue; - } - if (v === undefined) continue; - // The active read asks `type: { $in: [type, plural] }`; no other - // operator reaches this stub. - if (v && typeof v === 'object' && Array.isArray((v as { $in?: unknown }).$in)) { - if (!(v as { $in: unknown[] }).$in.includes((r as any)[k])) return false; - continue; - } - if ((r as any)[k] !== v) return false; - } - return true; -} - -function makeStubEngine() { - const rows = new Map(); - const historyRows: HistoryRow[] = []; - let nextId = 0; - - const findRow = (w: Record): { key: string; row: Row } | null => { - if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; - return null; - } - if (w.package_id !== undefined) { - const k = keyOf(w); - const r = rows.get(k); - return r ? { key: k, row: r } : null; - } - for (const [k, r] of rows) if (matchesMetadataWhere(r, w)) return { key: k, row: r }; - return null; - }; - - const matchesHistory = (h: HistoryRow, w: Record): boolean => { - if (w.organization_id !== undefined && h.organization_id !== w.organization_id) return false; - if (w.type !== undefined && h.type !== w.type) return false; - if (w.name !== undefined && h.name !== w.name) return false; - if (w.version !== undefined && h.version !== w.version) return false; - if (w.operation_type !== undefined && h.operation_type !== w.operation_type) return false; - return true; - }; - - const engine: any = { - async findOne(table: string, opts: { where: Record }) { - assertEngineFindOnePredicate(table, opts); - if (table === 'sys_metadata_history') { - return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; - } - return findRow(opts.where)?.row ?? null; - }, - async find(table: string, opts: { where: Record; limit?: number }) { - const matched = table === 'sys_metadata_history' - ? historyRows.filter((h) => matchesHistory(h, opts.where)) - : Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); - return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; - }, - async insert(table: string, data: Record) { - if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; - if (table === 'sys_metadata_history') { - nextId += 1; - const h: HistoryRow = { id: `h_${nextId}`, ...(data as any) }; - historyRows.push(h); - return { id: h.id }; - } - nextId += 1; - const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); - return { id: row.id }; - }, - async update(_t: string, data: Record, opts: { where: Record }) { - assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); - if (!found) return { id: null }; - const merged = { ...found.row, ...(data as any) }; - rows.delete(found.key); - rows.set(keyOf(merged), merged); - return { id: found.row.id }; - }, - async delete(_t: string, opts: { where: Record }) { - assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); - if (!found) return { deleted: 0 }; - rows.delete(found.key); - return { deleted: 1 }; - }, - async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { - return cb(undefined, { owned: true }); - }, - // The engine's judge-only filter admission (#20158), as the live engine - // answers it for a read scope on a formula field: refused, because a - // virtual field has no column a row filter can read. The engine's real - // verdicts are pinned in `packages/cli/test/rls-policy-authoring-admission.test.ts`. - judgeFilter: (_object: string, where: unknown) => - JSON.stringify(where).includes('deal_score') - ? { ok: false, code: 'FILTER_FIELD_NOT_STORED', status: 400, message: 'Field "deal_score" is a formula field and cannot be filtered.' } - : { ok: true }, - registry: { - registerItem: () => {}, - registerObject: () => {}, - // The LIVE universe: the objects and the one dataset the fixtures - // resolve against. - listItems: (type: string) => { - if (type === 'object') return [liveDealObject]; - if (type === 'dataset') return [liveDealDataset]; - return []; - }, - // The base the drafts are bound to is INSTALLED — a flow may be - // saved only into a package the registry holds (#20863) — and its - // manifest declares no namespace → the ADR-0028 prefix pre-flight is - // skipped (legacy-grandfathered path); it declares no dependencies - // and no live object carries a package stamp, so - // `resolveWritePackageScope`'s closure removes nothing. - getPackage: (id: string) => (id === PKG ? { manifest: { id: PKG } } : undefined), - }, - }; - return { engine, rows, historyRows }; -} -const PKG = 'app.sales'; - - -import { appendFileSync } from 'node:fs'; -const OUT = process.env.PROBE_OUT!; -async function stageDraft(protocol: ObjectStackProtocolImplementation, type: string, item: { name: string }) { - return (protocol as any).saveMetaItem({ type, name: item.name, item, packageId: PKG, mode: 'draft' }); -} -const RULE_OBJ = { - name: 'rule_probe', - label: 'Rule Probe', - sharingModel: 'private', - fields: { - name: { type: 'text', label: 'Name' }, - amount: { type: 'number', label: 'Amount' }, - }, - validations: [ - { name: 'ok_rule', type: 'script', condition: 'record.name != null', message: 'ok' }, - { name: 'amount_rule', type: 'script', condition: 'record.amount > 100', message: 'Amount rule' }, - { name: 'cond_rule', type: 'conditional', when: 'record.amount > 5', - then: { name: 'inner', type: 'script', condition: 'record.amount < 1000', message: 'inner' }, message: 'c' }, - ], -}; -const FLOW = { - name: 'edge_cel', label: 'Edge CEL', type: 'autolaunched', - nodes: [ - { id: 'start', type: 'start', label: 'Start' }, - { id: 'decide', type: 'decision', label: 'Decide' }, - { id: 'end', type: 'end', label: 'End' }, - ], - edges: [ - { id: 'e1', source: 'start', target: 'decide' }, - { id: 'e2', source: 'decide', target: 'end', type: 'conditional', condition: 'record.name ==' }, - ], -}; -describe('probe 27643', () => { - for (const [type, item] of [['object', RULE_OBJ], ['flow', FLOW]] as const) { - it(type, async () => { - const { engine } = makeStubEngine(); - const protocol = new ObjectStackProtocolImplementation(engine); - await stageDraft(protocol, type, item as any); - const read: any = await protocol.getMetaItem({ type, name: item.name, state: 'draft', packageId: PKG } as any); - const refusal: any = await protocol.publishMetaItem({ type, name: item.name, packageId: PKG } as any).then(() => null, (e: unknown) => e); - appendFileSync(OUT, JSON.stringify({ type, diagnostics: read.item._diagnostics, - publish422: refusal && { status: refusal.status, code: refusal.code, - issues: refusal.issues.map((i: any) => ({ rule: i.rule, path: i.path, where: i.where, message: i.message.slice(0, 160) })) } }, null, 1) + '\n'); - }); - } -});