Skip to content

Commit 19bca8c

Browse files
os-helpclaude
andauthored
feat(lint): refuse a path-shaped ==/!= right-hand side at publish time (#7659) (#7691)
The metadata-editing form renderer resolves paths on the LEFT of `==` / `!=` only; the right side goes to a literal parser whose tail returns 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. #7010's `predicate-path-unresolved` cannot see it: both paths resolve, so there is nothing for it to report. The renderer's own diagnostic (objectui#4049) is dev-mode only and fires after the metadata is stored, so an AI author or a CI pipeline publishing forms never sees it either. New sibling rule `predicate-rhs-path-shaped` in the same file, using the consumer's grammar verbatim (verified against objectui@37cd8e4, not against the issue text) — one grammar, two enforcement points. `error` on a dotted chain (no reading under which it worked); `warning` on a bare unquoted word, which compares as the literal text today and would be a false build error to refuse. The two path-resolution rules are untouched: a predicate that is both unresolvable and path-shaped on the right reports twice, because both statements are true and their fixes differ. `in`'s array parse (objectui#4266) is not folded in. Corpus over the shipped METADATA_FORM_REGISTRY: 0 at both severities, with reverse verification at 45 comparisons. Claude-Session: https://claude.ai/code/session_01KBKThMnWto2SromcUVAxJk Co-authored-by: Claude <noreply@anthropic.com>
1 parent 06b43cc commit 19bca8c

6 files changed

Lines changed: 476 additions & 8 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): refuse a path-shaped right-hand side in a metadata-form predicate (#7659)
6+
7+
The metadata-editing form renderer supports a declared subset of predicate
8+
expressions in which the RIGHT side of `==` / `!=` is a **literal**, never a
9+
resolved path. Only the left side resolves; the right side goes to a literal
10+
parser whose tail hands back anything it does not recognise verbatim. So
11+
`data.a == data.b` compares `data.a`'s value against the seven-character
12+
**string** `"data.b"` — false however equal the two sides are, and
13+
`data.a != data.b` correspondingly true. An `==` predicate written that way
14+
hides the element on every row, and nothing says why.
15+
16+
Nothing at the publish door could see it. #7010's `predicate-path-unresolved`
17+
asks whether a path RESOLVES; `data.a == data.b` answers yes twice and walks
18+
through. The renderer's own diagnostic (objectui#4049) is dev-mode only and
19+
fires at render time — after the metadata is stored — so an AI author or a CI
20+
pipeline publishing forms never sees it.
21+
22+
**New rule — `predicate-rhs-path-shaped`**, a sibling of the two path-resolution
23+
rules in `validate-predicate-path-refs.ts`, exported from the package root and
24+
run by `os build` / `os lint` / `os validate` and at the runtime `view` publish
25+
gate. It reports a `==` / `!=` right-hand side that is an unquoted identifier
26+
chain, using the same grammar the renderer warns on
27+
(`/^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/`) — one grammar,
28+
two enforcement points. The message names both sanctioned spellings: quote the
29+
literal, or restructure so the path is on the left and a literal on the right.
30+
31+
**Two severities, on one id:**
32+
33+
- **`error` for a dotted chain** (`data.a == data.b`, or the same with the sides
34+
swapped). Nobody writes a dotted identifier chain meaning the literal text of
35+
it, so there is no reading under which this worked — the same bar
36+
`predicate-path-unresolved` already gates on.
37+
- **`warning` for a bare single word** (`status == active`). This one *works*
38+
today: it compares against the literal string `"active"`, which is very likely
39+
what the author meant, and the renderer's ruling preserved that deliberately.
40+
It is outside the declared subset all the same and stops working when this
41+
surface moves to the real CEL evaluator, so it is reported — but refusing a
42+
`view` write over metadata that renders correctly would be a false build error.
43+
44+
Measured over the shipped `METADATA_FORM_REGISTRY` (17 forms, 46 predicates):
45+
**0** findings at either severity, with reverse verification (rewriting each
46+
`== 'literal'` into `== data.__rhs__` reports all 45 comparisons).
47+
48+
Deliberately unchanged: the two path-resolution rules. A predicate that is both
49+
unresolvable and path-shaped on the right reports twice — both statements are
50+
true and their fixes differ. `in`'s array parse is a distinct defect
51+
(objectui#4266) and is not folded in.

packages/lint/src/authoring-rules.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,9 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
822822
// in ONE edit, on the maintainer's 2026-08-10 ruling, sequenced after #4717's
823823
// `advisories` channel landed (PR #7435). Before that move a `view` written
824824
// through Studio / REST `/meta` / MCP — the only door most tenants have, and
825-
// the door AI authors use — was judged by NONE of the family's six rule ids.
825+
// the door AI authors use — was judged by NONE of the family's rule ids (six
826+
// at the time of the move; seven since #7659 added
827+
// `predicate-rhs-path-shaped` inside the second entry).
826828
//
827829
// They move together on purpose, and the two entries carry one comment because
828830
// they are one wall: #7214's implementer wired its own rule here alone and then
@@ -870,6 +872,17 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
870872
// object's addressable path set is NOT closed (lookup traversal, system
871873
// columns, formula outputs), and an `error` gate over an open set generates
872874
// false build errors. See the rule's module note.
875+
//
876+
// #7659 adds a THIRD id here, `predicate-rhs-path-shaped`, which is not a
877+
// resolution question at all: the metadata-admin renderer resolves paths only
878+
// on the LEFT of `==` / `!=` and hands the right side to its literal parser,
879+
// so `data.a == data.b` resolves both sides cleanly, passes the two rules
880+
// above, and still compares against the string "data.b" — a constant verdict.
881+
// It carries `error` on a dotted chain (no reading under which it worked) and
882+
// `warning` on a bare word (`status == active` compares as the text today, so
883+
// refusing it would fail a build over metadata that renders correctly). The
884+
// per-finding severity is what gates, exactly as `lintFlowPatterns` has worked
885+
// since #3760; the entry's `gating` tier is unchanged because it already was.
873886
{
874887
name: 'validatePredicatePathRefs',
875888
tier: 'gating',

packages/lint/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ export {
179179
validatePredicatePathRefs,
180180
PREDICATE_PATH_UNRESOLVED,
181181
PREDICATE_PATH_UNROOTED,
182+
PREDICATE_RHS_PATH_SHAPED,
182183
} from './validate-predicate-path-refs.js';
183184
export type {
184185
PredicatePathFinding,

packages/lint/src/runtime-gate.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,40 @@ describe('the views[] visibility-predicate family at the runtime publish gate (#
320320
expect(errors.map((e) => e.rule)).toContain('predicate-path-unrooted');
321321
});
322322

323+
it('REFUSES a path on the RIGHT of `==` — which resolves cleanly and is broken anyway', () => {
324+
// #7659, and the measurement that justifies the id existing: `data.type` and
325+
// `data.label` are both keys of `FieldSchema`, so the rule directly above is
326+
// silent here BY CONSTRUCTION. The renderer resolves only the left side and
327+
// parses the right as a literal, so this compares against the string
328+
// "data.label" and is false on every row.
329+
const { errors } = gateView(schemaBoundForm('data.type == data.label'));
330+
const f = errors.find((e) => e.rule === 'predicate-rhs-path-shaped');
331+
expect(f, 'the right-hand position never evaluates a path').toBeDefined();
332+
expect(f!.severity).toBe('error');
333+
expect(
334+
errors.map((e) => e.rule),
335+
"#7214's check has nothing to say here — that silence is why this rule exists",
336+
).not.toContain('predicate-path-unresolved');
337+
});
338+
339+
it('sends a BARE unquoted word down the advisory channel, not the refusal one', () => {
340+
// The second severity the same id carries. `== active` is compared as the
341+
// literal string "active" today — very likely what the author meant — so
342+
// this rule does not refuse the write over metadata that renders correctly.
343+
//
344+
// The write IS refused, by `visibility-bare-identifier` from the sibling
345+
// file, which reads `active` as a dropped binding root. Both findings are
346+
// true about the token and they prescribe DIFFERENT fixes (`data.active` vs
347+
// `'active'`), so this pins the pair rather than asserting a clean `errors`
348+
// list that would go stale the moment either side moved.
349+
const result = gateView(schemaBoundForm('data.type == active'));
350+
const f = result.advisories.find((a) => a.rule === 'predicate-rhs-path-shaped');
351+
expect(f, 'the subset boundary must still reach the author').toBeDefined();
352+
expect(f!.severity).toBe('warning');
353+
expect(result.errors.map((e) => e.rule)).not.toContain('predicate-rhs-path-shaped');
354+
expect(result.errors.map((e) => e.rule)).toContain('visibility-bare-identifier');
355+
});
356+
323357
it('reports a MISLAYERED root through the advisory channel, not a refusal', () => {
324358
// The one family member that is `warning` on every surface, so it must not
325359
// 422 — and must not be silent either. #4717's `advisories` channel (the

packages/lint/src/validate-predicate-path-refs.test.ts

Lines changed: 177 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

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

@@ -261,6 +263,120 @@ describe('validatePredicatePathRefs — traversal reach', () => {
261263
});
262264
});
263265

266+
// ────────────────────────────────────────────────────────────────────────────
267+
// #7659 — the RIGHT-hand side of `==` / `!=`
268+
// ────────────────────────────────────────────────────────────────────────────
269+
//
270+
// A different question from everything above, and the reason it needed its own
271+
// rule rather than a widening of #7214: `data.a == data.b` RESOLVES on both
272+
// sides, so both rules above are silent on it by construction. The first two
273+
// tests pin that pair — the silence, and a control proving the walk that went
274+
// silent can still see.
275+
describe('validatePredicatePathRefs — path-shaped right-hand side (#7659)', () => {
276+
const rhs = (source: string) =>
277+
run(form([{ label: 'S', fields: [{ field: 'name', visibleWhen: source }] }]));
278+
279+
it('reports a path on the RIGHT of `==` — the case #7214 is silent on', () => {
280+
// Both sides resolve against DemoSchema (`name`, `type`), so neither
281+
// resolution limb has anything to say. Asserted as an EQUALITY on the rule
282+
// set rather than a `toContain`: if a resolution limb ever started firing
283+
// here, this card's premise would be gone and the test must say so.
284+
const findings = rhs('data.name == data.type');
285+
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
286+
expect(findings[0].severity).toBe('error');
287+
expect(findings[0].path).toBe('views[0].sections[0].fields[0].visibleWhen');
288+
expect(findings[0].message).toContain('`data.type`');
289+
expect(findings[0].message).toMatch(/literal string "data\.type"/);
290+
expect(findings[0].hint).toMatch(/quote it/);
291+
});
292+
293+
it('proves that silence is #7214 unable to SEE this, not a walk that reports nothing', () => {
294+
// Same predicate, one segment misspelled: the resolution limb must wake up.
295+
// Without this, the assertion above could be measuring a dead walk.
296+
expect(rhs('data.name == data.tpye').map((f) => f.rule).sort())
297+
.toEqual([PREDICATE_PATH_UNRESOLVED, PREDICATE_RHS_PATH_SHAPED]);
298+
});
299+
300+
it('reports `!=` the same way', () => {
301+
const findings = rhs('data.name != data.type');
302+
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
303+
expect(findings[0].message).toContain('`!=`');
304+
});
305+
306+
it('reports a path on the right even when the LITERAL is on the left', () => {
307+
// Sides swapped: the renderer resolves the left and parses the right, so
308+
// this is the same defect and the fix is to swap them back.
309+
expect(rhs("'grid' == data.type").map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
310+
});
311+
312+
it('reports a BARE unquoted word, at `warning` — it works today by accident', () => {
313+
// `... == active` compares against the literal string "active", which is
314+
// very likely what the author meant; objectui#4049's ruling preserved that
315+
// deliberately. Reported, because it is outside the declared subset and dies
316+
// when CEL lands; not gated, because refusing a `view` write over metadata
317+
// that renders correctly is a false build error.
318+
const findings = rhs('data.name == active');
319+
expect(findings.map((f) => f.rule)).toEqual([PREDICATE_RHS_PATH_SHAPED]);
320+
expect(findings[0].severity).toBe('warning');
321+
expect(findings[0].message).toContain('`active`');
322+
});
323+
324+
it('reports every comparison in a compound predicate', () => {
325+
const findings = rhs("data.name == data.type && data.type == 'formula' || data.name != data.type");
326+
expect(findings.map((f) => f.rule)).toEqual([
327+
PREDICATE_RHS_PATH_SHAPED,
328+
PREDICATE_RHS_PATH_SHAPED,
329+
]);
330+
});
331+
332+
// ── Negative controls. Each is either a spelling the rule's own hint
333+
// recommends, or a literal form `parseLiteral` returns from BEFORE its
334+
// path-shaped tail — so the renderer is silent on it too.
335+
it.each([
336+
['a quoted literal RHS', "data.type == 'formula'"],
337+
['a double-quoted literal RHS', 'data.type == "formula"'],
338+
['a numeric RHS', 'data.name == 3'],
339+
['a negative numeric RHS', 'data.name != -3'],
340+
['a boolean RHS', 'data.enable.search == true'],
341+
['a boolean RHS, negated', 'data.enable.search != false'],
342+
['a null RHS', 'data.type == null'],
343+
// The restructuring the hint recommends — PATH on the left, literal on the
344+
// right. If the message tells authors to write this, writing it must not be
345+
// reported, or the rule sends them in a circle.
346+
['the sanctioned restructuring (path LEFT, literal right)', "data.type != 'formula'"],
347+
['a bare truthy check with no comparison at all', 'data.enable.search'],
348+
['an `in` membership test (objectui#4266, deliberately not folded in)', "data.type in ['a','b']"],
349+
// Reachable through `parseLiteral`'s tail in the renderer, but NOT
350+
// path-shaped under the shared grammar — the consumer stays silent on both.
351+
['an indexed RHS', 'data.type == data.rows[0]'],
352+
['a call-result RHS', 'data.type == size(data.tags)'],
353+
])('is silent on %s', (_label, source) => {
354+
expect(rhs(source)).toEqual([]);
355+
});
356+
357+
it('leaves a comprehension macro BODY alone, and still walks its receiver', () => {
358+
// The interim evaluator supports no macros at all, so a comparison inside
359+
// one is not a statement about this subset. The receiver is still walked —
360+
// the second case carries a real finding in the same predicate.
361+
expect(rhs('data.tags.all(t, t == data.type)')).toEqual([]);
362+
expect(rhs('data.name == data.type && data.tags.all(t, t == data.type)').map((f) => f.rule))
363+
.toEqual([PREDICATE_RHS_PATH_SHAPED]);
364+
});
365+
366+
it('gives no verdict on a source the canonical front end refuses', () => {
367+
// `$` is in the shared grammar and NOT in CEL's identifier syntax, so this
368+
// never parses. One broken predicate, one finding — and that one is
369+
// `visibility-predicate-syntax`'s (#6253), from the sibling file.
370+
expect(rhs('data.name == $b')).toEqual([]);
371+
});
372+
373+
it('emits the id the published barrel exports', async () => {
374+
const barrel = await import('./index.js');
375+
expect(barrel.PREDICATE_RHS_PATH_SHAPED).toBe('predicate-rhs-path-shaped');
376+
expect(rhs('data.name == data.type')[0].rule).toBe(barrel.PREDICATE_RHS_PATH_SHAPED);
377+
});
378+
});
379+
264380
describe('registry wiring', () => {
265381
it('is registered in AUTHORING_RULES as a gating rule on all three commands', () => {
266382
const entry = AUTHORING_RULES.find((r) => r.name === 'validatePredicatePathRefs');
@@ -354,6 +470,66 @@ describe('#7010 corpus — shipped METADATA_FORM_REGISTRY', () => {
354470
).toEqual([]);
355471
});
356472

473+
// #7659's own corpus measurement, on the same population and through the same
474+
// production entry point. The rule above is asserted at zero for its two ids;
475+
// this one asserts zero for the third at BOTH severities, which is what a new
476+
// `error` finding costs before it may land. Measured on `origin/main@5823d59`:
477+
// 0 and 0. A non-zero `error` count would have been a STOP.
478+
it('reports NOTHING on the RHS rule over the shipped forms, at either severity', () => {
479+
const rhsFindings = validatePredicatePathRefs(shippedStack)
480+
.filter((f) => f.rule === PREDICATE_RHS_PATH_SHAPED);
481+
expect(rhsFindings.filter((f) => f.severity === 'error').map((f) => f.path)).toEqual([]);
482+
expect(rhsFindings.filter((f) => f.severity === 'warning').map((f) => f.path)).toEqual([]);
483+
});
484+
485+
it('sees the shipped corpus (reverse verification for the RHS rule)', () => {
486+
// The anti-vacuity direction, and the reason it is written against the
487+
// SHIPPED predicates rather than a fixture: rewriting each real
488+
// `== 'literal'` comparison into `== data.__rhs__` turns it into exactly the
489+
// defect, so the assertion is an EQUALITY on the number of rewritten
490+
// comparisons rather than a floor any subset would satisfy. Zero here would
491+
// mean the measurement above was a green gate over nothing (#4984).
492+
//
493+
// The rewrite is anchored on the OPERATOR, not on "any quoted string": a
494+
// literal inside `data.type in ['a','b']` belongs to `in`, whose array parse
495+
// is objectui#4266 and deliberately not this rule's, so rewriting those too
496+
// would have inflated the expected count past what this rule answers for.
497+
const corrupted = structuredClone(shippedStack) as { views: unknown[] };
498+
let comparisons = 0;
499+
const rewrite = (node: unknown): void => {
500+
if (Array.isArray(node)) {
501+
for (const child of node) rewrite(child);
502+
return;
503+
}
504+
if (!node || typeof node !== 'object') return;
505+
const rec = node as Record<string, unknown>;
506+
for (const key of ['visibleWhen', 'visibleOn']) {
507+
const value = rec[key];
508+
const source = typeof value === 'string' ? value
509+
: value && typeof value === 'object'
510+
&& typeof (value as Record<string, unknown>).source === 'string'
511+
? ((value as Record<string, unknown>).source as string)
512+
: undefined;
513+
if (source === undefined) continue;
514+
const swapped = source.replace(/(==|!=)(\s*)'[^']*'/g, (_m, op, gap) => {
515+
comparisons++;
516+
return `${op}${gap}data.__rhs__`;
517+
});
518+
if (swapped === source) continue;
519+
if (typeof value === 'string') rec[key] = swapped;
520+
else (value as Record<string, unknown>).source = swapped;
521+
}
522+
for (const value of Object.values(rec)) rewrite(value);
523+
};
524+
rewrite(corrupted.views);
525+
expect(comparisons, 'no shipped predicate carries an `==`/`!=` literal comparison').toBe(45);
526+
527+
const rhsFindings = validatePredicatePathRefs(corrupted)
528+
.filter((f) => f.rule === PREDICATE_RHS_PATH_SHAPED);
529+
expect(rhsFindings).toHaveLength(comparisons);
530+
expect(new Set(rhsFindings.map((f) => f.severity))).toEqual(new Set(['error']));
531+
});
532+
357533
it('catches the pre-#6254 bare spellings when they are restored (reverse verification)', () => {
358534
// The reverse direction is RED-on-restore: #6254 rewrote 16 predicates in
359535
// `object.form.ts` from `type ...` to `data.type ...`. Restoring the bare

0 commit comments

Comments
 (0)