Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .changeset/predicate-rhs-path-shaped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
"@objectstack/lint": minor
---

feat(lint): refuse a path-shaped right-hand side in a metadata-form predicate (#7659)

The metadata-editing form renderer supports a declared subset of predicate
expressions in which the RIGHT side of `==` / `!=` is a **literal**, never a
resolved path. Only the left side resolves; the right side goes to a literal
parser whose tail hands back anything it does not recognise verbatim. So
`data.a == data.b` compares `data.a`'s value against the seven-character
**string** `"data.b"` — false however equal the two sides are, and
`data.a != data.b` correspondingly true. An `==` predicate written that way
hides the element on every row, and nothing says why.

Nothing at the publish door could see it. #7010's `predicate-path-unresolved`
asks whether a path RESOLVES; `data.a == data.b` answers yes twice and walks
through. The renderer's own diagnostic (objectui#4049) is dev-mode only and
fires at render time — after the metadata is stored — so an AI author or a CI
pipeline publishing forms never sees it.

**New rule — `predicate-rhs-path-shaped`**, a sibling of the two path-resolution
rules in `validate-predicate-path-refs.ts`, exported from the package root and
run by `os build` / `os lint` / `os validate` and at the runtime `view` publish
gate. It reports a `==` / `!=` right-hand side that is an unquoted identifier
chain, using the same grammar the renderer warns on
(`/^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/`) — one grammar,
two enforcement points. The message names both sanctioned spellings: quote the
literal, or restructure so the path is on the left and a literal on the right.

**Two severities, on one id:**

- **`error` for a dotted chain** (`data.a == data.b`, or the same with the sides
swapped). Nobody writes a dotted identifier chain meaning the literal text of
it, so there is no reading under which this worked — the same bar
`predicate-path-unresolved` already gates on.
- **`warning` for a bare single word** (`status == active`). This one *works*
today: it compares against the literal string `"active"`, which is very likely
what the author meant, and the renderer's ruling preserved that deliberately.
It is outside the declared subset all the same and stops working when this
surface moves to the real CEL evaluator, so it is reported — but refusing a
`view` write over metadata that renders correctly would be a false build error.

Measured over the shipped `METADATA_FORM_REGISTRY` (17 forms, 46 predicates):
**0** findings at either severity, with reverse verification (rewriting each
`== 'literal'` into `== data.__rhs__` reports all 45 comparisons).

Deliberately unchanged: the two path-resolution rules. A predicate that is both
unresolvable and path-shaped on the right reports twice — both statements are
true and their fixes differ. `in`'s array parse is a distinct defect
(objectui#4266) and is not folded in.
15 changes: 14 additions & 1 deletion packages/lint/src/authoring-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
// in ONE edit, on the maintainer's 2026-08-10 ruling, sequenced after #4717's
// `advisories` channel landed (PR #7435). Before that move a `view` written
// through Studio / REST `/meta` / MCP — the only door most tenants have, and
// the door AI authors use — was judged by NONE of the family's six rule ids.
// the door AI authors use — was judged by NONE of the family's rule ids (six
// at the time of the move; seven since #7659 added
// `predicate-rhs-path-shaped` inside the second entry).
//
// They move together on purpose, and the two entries carry one comment because
// they are one wall: #7214's implementer wired its own rule here alone and then
Expand Down Expand Up @@ -870,6 +872,17 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
// object's addressable path set is NOT closed (lookup traversal, system
// columns, formula outputs), and an `error` gate over an open set generates
// false build errors. See the rule's module note.
//
// #7659 adds a THIRD id here, `predicate-rhs-path-shaped`, which is not a
// resolution question at all: the metadata-admin renderer resolves paths only
// on the LEFT of `==` / `!=` and hands the right side to its literal parser,
// so `data.a == data.b` resolves both sides cleanly, passes the two rules
// above, and still compares against the string "data.b" — a constant verdict.
// It carries `error` on a dotted chain (no reading under which it worked) and
// `warning` on a bare word (`status == active` compares as the text today, so
// refusing it would fail a build over metadata that renders correctly). The
// per-finding severity is what gates, exactly as `lintFlowPatterns` has worked
// since #3760; the entry's `gating` tier is unchanged because it already was.
{
name: 'validatePredicatePathRefs',
tier: 'gating',
Expand Down
1 change: 1 addition & 0 deletions packages/lint/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export {
validatePredicatePathRefs,
PREDICATE_PATH_UNRESOLVED,
PREDICATE_PATH_UNROOTED,
PREDICATE_RHS_PATH_SHAPED,
} from './validate-predicate-path-refs.js';
export type {
PredicatePathFinding,
Expand Down
34 changes: 34 additions & 0 deletions packages/lint/src/runtime-gate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,40 @@ describe('the views[] visibility-predicate family at the runtime publish gate (#
expect(errors.map((e) => e.rule)).toContain('predicate-path-unrooted');
});

it('REFUSES a path on the RIGHT of `==` — which resolves cleanly and is broken anyway', () => {
// #7659, and the measurement that justifies the id existing: `data.type` and
// `data.label` are both keys of `FieldSchema`, so the rule directly above is
// silent here BY CONSTRUCTION. The renderer resolves only the left side and
// parses the right as a literal, so this compares against the string
// "data.label" and is false on every row.
const { errors } = gateView(schemaBoundForm('data.type == data.label'));
const f = errors.find((e) => e.rule === 'predicate-rhs-path-shaped');
expect(f, 'the right-hand position never evaluates a path').toBeDefined();
expect(f!.severity).toBe('error');
expect(
errors.map((e) => e.rule),
"#7214's check has nothing to say here — that silence is why this rule exists",
).not.toContain('predicate-path-unresolved');
});

it('sends a BARE unquoted word down the advisory channel, not the refusal one', () => {
// The second severity the same id carries. `== active` is compared as the
// literal string "active" today — very likely what the author meant — so
// this rule does not refuse the write over metadata that renders correctly.
//
// The write IS refused, by `visibility-bare-identifier` from the sibling
// file, which reads `active` as a dropped binding root. Both findings are
// true about the token and they prescribe DIFFERENT fixes (`data.active` vs
// `'active'`), so this pins the pair rather than asserting a clean `errors`
// list that would go stale the moment either side moved.
const result = gateView(schemaBoundForm('data.type == active'));
const f = result.advisories.find((a) => a.rule === 'predicate-rhs-path-shaped');
expect(f, 'the subset boundary must still reach the author').toBeDefined();
expect(f!.severity).toBe('warning');
expect(result.errors.map((e) => e.rule)).not.toContain('predicate-rhs-path-shaped');
expect(result.errors.map((e) => e.rule)).toContain('visibility-bare-identifier');
});

it('reports a MISLAYERED root through the advisory channel, not a refusal', () => {
// The one family member that is `warning` on every surface, so it must not
// 422 — and must not be silent either. #4717's `advisories` channel (the
Expand Down
178 changes: 177 additions & 1 deletion packages/lint/src/validate-predicate-path-refs.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* Tests for the #7010 predicate PATH-resolution gate.
* Tests for the #7010 predicate PATH-resolution gate, and for #7659's sibling
* rule about the SHAPE of a `==` / `!=` right-hand side.
*
* The load-bearing block is `#6254 corpus` at the bottom. Everything above it is
* unit coverage over a hand-built schema; that block runs the rule over the
Expand All @@ -20,6 +21,7 @@ import {
validatePredicatePathRefs,
PREDICATE_PATH_UNRESOLVED,
PREDICATE_PATH_UNROOTED,
PREDICATE_RHS_PATH_SHAPED,
} from './validate-predicate-path-refs.js';
import { AUTHORING_RULES } from './authoring-rules.js';

Expand Down Expand Up @@ -261,6 +263,120 @@ describe('validatePredicatePathRefs — traversal reach', () => {
});
});

// ────────────────────────────────────────────────────────────────────────────
// #7659 — the RIGHT-hand side of `==` / `!=`
// ────────────────────────────────────────────────────────────────────────────
//
// A different question from everything above, and the reason it needed its own
// rule rather than a widening of #7214: `data.a == data.b` RESOLVES on both
// sides, so both rules above are silent on it by construction. The first two
// tests pin that pair — the silence, and a control proving the walk that went
// silent can still see.
describe('validatePredicatePathRefs — path-shaped right-hand side (#7659)', () => {
const rhs = (source: string) =>
run(form([{ label: 'S', fields: [{ field: 'name', visibleWhen: source }] }]));

it('reports a path on the RIGHT of `==` — the case #7214 is silent on', () => {
// Both sides resolve against DemoSchema (`name`, `type`), so neither
// resolution limb has anything to say. Asserted as an EQUALITY on the rule
// set rather than a `toContain`: if a resolution limb ever started firing
// here, this card's premise would be gone and the test must say so.
const findings = rhs('data.name == data.type');
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
expect(findings[0].severity).toBe('error');
expect(findings[0].path).toBe('views[0].sections[0].fields[0].visibleWhen');
expect(findings[0].message).toContain('`data.type`');
expect(findings[0].message).toMatch(/literal string "data\.type"/);
expect(findings[0].hint).toMatch(/quote it/);
});

it('proves that silence is #7214 unable to SEE this, not a walk that reports nothing', () => {
// Same predicate, one segment misspelled: the resolution limb must wake up.
// Without this, the assertion above could be measuring a dead walk.
expect(rhs('data.name == data.tpye').map((f) => f.rule).sort())
.toEqual([PREDICATE_PATH_UNRESOLVED, PREDICATE_RHS_PATH_SHAPED]);
});

it('reports `!=` the same way', () => {
const findings = rhs('data.name != data.type');
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
expect(findings[0].message).toContain('`!=`');
});

it('reports a path on the right even when the LITERAL is on the left', () => {
// Sides swapped: the renderer resolves the left and parses the right, so
// this is the same defect and the fix is to swap them back.
expect(rhs("'grid' == data.type").map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
});

it('reports a BARE unquoted word, at `warning` — it works today by accident', () => {
// `... == active` compares against the literal string "active", which is
// very likely what the author meant; objectui#4049's ruling preserved that
// deliberately. Reported, because it is outside the declared subset and dies
// when CEL lands; not gated, because refusing a `view` write over metadata
// that renders correctly is a false build error.
const findings = rhs('data.name == active');
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
expect(findings[0].severity).toBe('warning');
expect(findings[0].message).toContain('`active`');
});

it('reports every comparison in a compound predicate', () => {
const findings = rhs("data.name == data.type && data.type == 'formula' || data.name != data.type");
expect(findings.map((f) => f.rule)).toEqual([
PREDICATE_RHS_PATH_SHAPED,
PREDICATE_RHS_PATH_SHAPED,
]);
});

// ── Negative controls. Each is either a spelling the rule's own hint
// recommends, or a literal form `parseLiteral` returns from BEFORE its
// path-shaped tail — so the renderer is silent on it too.
it.each([
['a quoted literal RHS', "data.type == 'formula'"],
['a double-quoted literal RHS', 'data.type == "formula"'],
['a numeric RHS', 'data.name == 3'],
['a negative numeric RHS', 'data.name != -3'],
['a boolean RHS', 'data.enable.search == true'],
['a boolean RHS, negated', 'data.enable.search != false'],
['a null RHS', 'data.type == null'],
// The restructuring the hint recommends — PATH on the left, literal on the
// right. If the message tells authors to write this, writing it must not be
// reported, or the rule sends them in a circle.
['the sanctioned restructuring (path LEFT, literal right)', "data.type != 'formula'"],
['a bare truthy check with no comparison at all', 'data.enable.search'],
['an `in` membership test (objectui#4266, deliberately not folded in)', "data.type in ['a','b']"],
// Reachable through `parseLiteral`'s tail in the renderer, but NOT
// path-shaped under the shared grammar — the consumer stays silent on both.
['an indexed RHS', 'data.type == data.rows[0]'],
['a call-result RHS', 'data.type == size(data.tags)'],
])('is silent on %s', (_label, source) => {
expect(rhs(source)).toEqual([]);
});

it('leaves a comprehension macro BODY alone, and still walks its receiver', () => {
// The interim evaluator supports no macros at all, so a comparison inside
// one is not a statement about this subset. The receiver is still walked —
// the second case carries a real finding in the same predicate.
expect(rhs('data.tags.all(t, t == data.type)')).toEqual([]);
expect(rhs('data.name == data.type && data.tags.all(t, t == data.type)').map((f) => f.rule))
.toEqual([PREDICATE_RHS_PATH_SHAPED]);
});

it('gives no verdict on a source the canonical front end refuses', () => {
// `$` is in the shared grammar and NOT in CEL's identifier syntax, so this
// never parses. One broken predicate, one finding — and that one is
// `visibility-predicate-syntax`'s (#6253), from the sibling file.
expect(rhs('data.name == $b')).toEqual([]);
});

it('emits the id the published barrel exports', async () => {
const barrel = await import('./index.js');
expect(barrel.PREDICATE_RHS_PATH_SHAPED).toBe('predicate-rhs-path-shaped');
expect(rhs('data.name == data.type')[0].rule).toBe(barrel.PREDICATE_RHS_PATH_SHAPED);
});
});

describe('registry wiring', () => {
it('is registered in AUTHORING_RULES as a gating rule on all three commands', () => {
const entry = AUTHORING_RULES.find((r) => r.name === 'validatePredicatePathRefs');
Expand Down Expand Up @@ -354,6 +470,66 @@ describe('#7010 corpus — shipped METADATA_FORM_REGISTRY', () => {
).toEqual([]);
});

// #7659's own corpus measurement, on the same population and through the same
// production entry point. The rule above is asserted at zero for its two ids;
// this one asserts zero for the third at BOTH severities, which is what a new
// `error` finding costs before it may land. Measured on `origin/main@5823d59`:
// 0 and 0. A non-zero `error` count would have been a STOP.
it('reports NOTHING on the RHS rule over the shipped forms, at either severity', () => {
const rhsFindings = validatePredicatePathRefs(shippedStack)
.filter((f) => f.rule === PREDICATE_RHS_PATH_SHAPED);
expect(rhsFindings.filter((f) => f.severity === 'error').map((f) => f.path)).toEqual([]);
expect(rhsFindings.filter((f) => f.severity === 'warning').map((f) => f.path)).toEqual([]);
});

it('sees the shipped corpus (reverse verification for the RHS rule)', () => {
// The anti-vacuity direction, and the reason it is written against the
// SHIPPED predicates rather than a fixture: rewriting each real
// `== 'literal'` comparison into `== data.__rhs__` turns it into exactly the
// defect, so the assertion is an EQUALITY on the number of rewritten
// comparisons rather than a floor any subset would satisfy. Zero here would
// mean the measurement above was a green gate over nothing (#4984).
//
// The rewrite is anchored on the OPERATOR, not on "any quoted string": a
// literal inside `data.type in ['a','b']` belongs to `in`, whose array parse
// is objectui#4266 and deliberately not this rule's, so rewriting those too
// would have inflated the expected count past what this rule answers for.
const corrupted = structuredClone(shippedStack) as { views: unknown[] };
let comparisons = 0;
const rewrite = (node: unknown): void => {
if (Array.isArray(node)) {
for (const child of node) rewrite(child);
return;
}
if (!node || typeof node !== 'object') return;
const rec = node as Record<string, unknown>;
for (const key of ['visibleWhen', 'visibleOn']) {
const value = rec[key];
const source = typeof value === 'string' ? value
: value && typeof value === 'object'
&& typeof (value as Record<string, unknown>).source === 'string'
? ((value as Record<string, unknown>).source as string)
: undefined;
if (source === undefined) continue;
const swapped = source.replace(/(==|!=)(\s*)'[^']*'/g, (_m, op, gap) => {
comparisons++;
return `${op}${gap}data.__rhs__`;
});
if (swapped === source) continue;
if (typeof value === 'string') rec[key] = swapped;
else (value as Record<string, unknown>).source = swapped;
}
for (const value of Object.values(rec)) rewrite(value);
};
rewrite(corrupted.views);
expect(comparisons, 'no shipped predicate carries an `==`/`!=` literal comparison').toBe(45);

const rhsFindings = validatePredicatePathRefs(corrupted)
.filter((f) => f.rule === PREDICATE_RHS_PATH_SHAPED);
expect(rhsFindings).toHaveLength(comparisons);
expect(new Set(rhsFindings.map((f) => f.severity))).toEqual(new Set(['error']));
});

it('catches the pre-#6254 bare spellings when they are restored (reverse verification)', () => {
// The reverse direction is RED-on-restore: #6254 rewrote 16 predicates in
// `object.form.ts` from `type ...` to `data.type ...`. Restoring the bare
Expand Down
Loading
Loading