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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
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
19 changes: 19 additions & 0 deletions .changeset/wise-pumas-attack.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
'@object-ui/app-shell': patch
---

metadata-admin: diagnose a path on the right-hand side of `==` / `!=` in a visibility predicate

The predicate evaluator resolves paths only on the LEFT of `==` / `!=`. The right-hand side goes
through `parseLiteral`, which hands back anything it does not recognise as a literal verbatim — so
`data.a == data.b` compares the value of `data.a` against the seven-character string `"data.b"` and
is false however equal the two sides are, with nothing in the console. objectstack#6936's
unresolved-path warning cannot see this: it hangs on `resolveValue`, which the right side never
enters.

A dev-mode `console.warn` now fires when that tail returns something path-shaped (a dot-separated
identifier chain — the same grammar the left side accepts), naming the text, the predicate carrying
it, and the boundary. **No semantics change**: `data.a == data.b` still evaluates false, and the
before/after verdicts are pinned identical. The semantic fix belongs to publish-time predicate
validation (objectstack#7010) and to the real CEL runtime this file stands in for (ROADMAP M9), with
which this diagnostic retires.
104 changes: 104 additions & 0 deletions packages/app-shell/src/views/metadata-admin/predicate.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,3 +346,107 @@ describe('the 16 vanished Studio sub-fields (objectstack#6936 reading)', () => {
expect(warn).toHaveBeenCalled();
});
});

/* ── 7. a path on the RIGHT of ==/!= is a string literal — say so (#4049) ── */

/**
* objectui#4049. `parseLiteral`'s tail `return s` hands back any input it does
* not recognise as a literal, verbatim. The LEFT side of `==` / `!=` resolves
* paths; the RIGHT side does not — so `data.a == data.b` compares the value of
* `data.a` against the seven-character string "data.b" and is FALSE however
* equal the two sides are, with nothing in the console. objectstack#6936's
* warning cannot see this: it hangs on `resolveValue`, which the right side
* never enters (measured — 0 warnings for the whole truth table below).
*
* Maintainer/PM ruling on this card: option B ONLY — a dev-mode diagnostic at
* that tail, ZERO semantic change. The verdicts are therefore pinned IDENTICAL
* before and after (§7.3); the console merely stops being silent.
*/
describe('a path-shaped right-hand side is diagnosed, not resolved (objectui#4049)', () => {
/* 7.1 — it fires, and it names both halves */

it('`data.a == data.b` warns, naming the right-hand text and the predicate', () => {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).toHaveBeenCalledTimes(1);
// Both halves or the warning sends nobody anywhere — same bar as #6936.
expect(warnings()).toContain('`data.b`');
expect(warnings()).toContain('data.a == data.b');
});

it('`!=` routes through the same tail', () => {
expect(evaluatePredicate('data.a != data.b', scope({ a: 'x', b: 'x' }))).toBe(true);
expect(warnings()).toContain('`data.b`');
});

it('an UNQUOTED bare identifier fires — the A-direction regression shape', () => {
// `data.type == text` "works" today by accident: the row happens to hold the
// string "text". Option A (resolving the right side) would have flipped it
// to fail-open true. It stays exactly as it was — and stops being silent, so
// the accident is discoverable instead of load-bearing.
expect(evaluatePredicate('data.type == text', scope({ type: 'text' }))).toBe(true);
expect(warnings()).toContain('`text`');
});

it('warns ONCE per (right-hand text, predicate) pair', () => {
for (let i = 0; i < 5; i++) evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
expect(warn).toHaveBeenCalledTimes(1);
});

it('but a different predicate carrying the same text gets its own warning', () => {
evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }));
evaluatePredicate('data.c == data.b', scope({ b: 'x', c: 'x' }));
expect(warn).toHaveBeenCalledTimes(2);
});

it('the diagnostic is dev-mode only', () => {
const prev = process.env.NODE_ENV;
process.env.NODE_ENV = 'production';
try {
expect(evaluatePredicate('data.a == data.b', scope({ a: 'x', b: 'x' }))).toBe(false);
expect(warn).not.toHaveBeenCalled();
} finally {
process.env.NODE_ENV = prev;
}
});

/* 7.2 — controls: real literals stay silent */

it.each([
["a quoted string", "data.type == 'text'", { type: 'text' }],
['a double-quoted string', 'data.type == "text"', { type: 'text' }],
['a number', 'data.n == 42', { n: 42 }],
['a negative number', 'data.n == -1', { n: -1 }],
['a decimal', 'data.n == 1.5', { n: 1.5 }],
['true', 'data.flag == true', { flag: true }],
['false', 'data.flag == false', { flag: false }],
['null', 'data.v == null', { v: null }],
['an array', "data.type in ['text','number']", { type: 'text' }],
['a quoted string that CONTAINS dots', "data.v == 'a.b.c'", { v: 'a.b.c' }],
])('%s on the right side fires nothing', (_label, expr, row) => {
evaluatePredicate(expr as string, scope(row as Record<string, unknown>));
expect(warn).not.toHaveBeenCalled();
});

it('a dotted NON-identifier is not called a path (grammar boundary)', () => {
// `1.2.3` reaches the same tail (via resolveValue's literal shortcut for
// digit-leading operands) and is likewise compared as text — but it is a
// malformed NUMBER, not a path. Reporting it as one would be a false claim,
// so the grammar is the dot-separated identifier chain the left side
// accepts, not "contains a dot".
expect(evaluatePredicate('data.v == 1.2.3', scope({ v: '1.2.3' }))).toBe(true);
expect(warn).not.toHaveBeenCalled();
});

/* 7.3 — the zero-semantics proof: verdicts identical to pre-change */

it.each([
['data.a == data.b', { a: 'x', b: 'x' }, false], // equal values, still FALSE
['data.a != data.b', { a: 'x', b: 'x' }, true], // equal values, still TRUE
['data.a == data.b', { a: 'x', b: 'y' }, false], // right for the wrong reason
["data.a == 'x'", { a: 'x', b: 'y' }, true], // literal control
['data.type == text', { type: 'text' }, true], // unquoted, unchanged
['data.a == data.b', { a: 'data.b' }, true], // the literal-text comparison itself
])('%s over %j is still %s — the diagnostic changes no verdict', (expr, row, expected) => {
expect(evaluatePredicate(expr as string, scope(row as Record<string, unknown>))).toBe(expected);
});
});
102 changes: 90 additions & 12 deletions packages/app-shell/src/views/metadata-admin/predicate.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -80,6 +80,30 @@
* `false && <unresolvable>` is `false` and `true || <unresolvable>` is `true` —
* the unresolvable half is short-circuited away, absorbed exactly as CEL absorbs
* an erroring branch, and no warning fires because nothing needed it.
*
* ## A path on the RIGHT of `==` / `!=` is a string literal (objectui#4049)
*
* Only the LEFT side resolves. The right side goes to `parseLiteral`, whose tail
* hands back anything it does not recognise as a literal VERBATIM — so
* `data.a == data.b` compares the value of `data.a` against the seven-character
* string "data.b". It is FALSE however equal the two sides are, and
* `data.a != data.b` is correspondingly TRUE. The subset list above is honest
* about this (`path == 'literal'`, never `path == path`), but the boundary was
* enforced by silence: objectstack#6936's warning cannot reach it, because that
* one hangs on `resolveValue` and the right side never enters it.
*
* Ruling on objectui#4049: **option B only — a dev-mode diagnostic here, zero
* semantic change.** Resolving the right side (option A) was rejected: it would
* flip `data.type == text`, the unquoted-string spelling that works today by
* accident, into a fail-open `true`. The verdicts stay bit-for-bit what they
* were and are pinned that way in `predicate.test.ts` §7.3; all that changes is
* that the console stops being silent about the subset boundary.
*
* The semantic fix belongs to the producer — publish-time validation of
* predicate expressions (objectstack#7010) — and to the real CEL runtime: as the
* header says, this file is an interim stand-in for `@objectstack/formula`, so
* **this diagnostic retires with the file** when ROADMAP M9 lands CEL. Do not
* grow it into a second evaluator.
*/

export function evaluatePredicate(
Expand All@@ -90,7 +114,7 @@ export function evaluatePredicate(
const source = typeof expr === 'string' ? expr : expr.source;
if (!source) return true;
try {
return evalExpr(source.trim(), ctx);
return evalExpr(source.trim(), ctx, source);
} catch (err) {
// Fail-open either way; an unresolvable path additionally gets a name.
if (err instanceof UnresolvedPathError) warnUnresolvedPath(err.path, source);
Expand DownExpand Up@@ -119,9 +143,18 @@ class UnresolvedPathError extends Error {
// `warnOnUnknownActionKeys` in `@object-ui/core` (`actions/actionKeys.ts`).
const warnedUnresolvedPaths = new Set<string>();

/** Reset the warn-once memo. Exported for tests. */
/**
* The same warn-once discipline for the right-hand-literal diagnostic
* (objectui#4049), keyed on (right-hand text, predicate) for the same reason:
* keyed on the text alone, a form with fifteen predicates comparing against
* `data.b` would report one of them and hide the rest.
*/
const warnedPathShapedLiterals = new Set<string>();

/** Reset the warn-once memos. Exported for tests. */
export function resetPredicateWarnings(): void {
warnedUnresolvedPaths.clear();
warnedPathShapedLiterals.clear();
}

const isDev = (): boolean =>
Expand All@@ -144,42 +177,79 @@ function warnUnresolvedPath(path: string, source: string): void {
);
}

/**
* The identifier grammar the LEFT side accepts: a root identifier followed by
* dot-separated segments (`text`, `data.type`, `data.config.kind`) — i.e. the
* shape `resolveValue` would have resolved had this text been on the other side
* of the operator.
*
* Deliberately NOT "contains a dot". `1.2.3` also reaches `parseLiteral`'s tail
* (via the digit-leading literal shortcut in `resolveValue`) and is likewise
* compared as text, but it is a malformed NUMBER, not a path; announcing it as
* a path would be a false statement about the author's code. Quoted strings,
* numbers, booleans, null and arrays never reach the tail at all — they return
* from their own branches above.
*/
const PATH_SHAPED_LITERAL = /^[A-Za-z_$][A-Za-z0-9_$]*(?:\.[A-Za-z_$][A-Za-z0-9_$]*)*$/;

function warnPathShapedLiteral(text: string, source: string): void {
if (!isDev()) return;
const memo = `${text}::${source}`;
if (warnedPathShapedLiterals.has(memo)) return;
warnedPathShapedLiterals.add(memo);
console.warn(
`[metadata-admin] visibility predicate \`${source}\` compares against \`${text}\`, which looks ` +
`like a path but is being used as the literal string "${text}" — this evaluator resolves ` +
'paths only on the LEFT of `==` / `!=`; the right-hand side is always a literal (supported ' +
"subset: `path == 'literal'`). So `data.a == data.b` is FALSE even when both sides hold the " +
'same value, and `data.a != data.b` is TRUE — the verdict does not depend on the right-hand ' +
"path at all. If you meant the text, quote it (`== 'text'`). If you meant to compare two " +
'paths, that is outside this evaluator\'s subset: it is an interim stand-in for ' +
'`@objectstack/formula` until CEL lands (ROADMAP M9), and predicate expressions are ' +
'validated at publish time (objectstack#7010). objectui#4049.',
);
}

function evalExpr(
expr: string,
ctx: { data: Record<string, unknown> },
// The WHOLE predicate, threaded down unchanged so a diagnostic raised deep in
// a sub-expression can name the predicate the author actually wrote — the
// same pairing objectstack#6936's warning makes via UnresolvedPathError.
source: string,
): boolean {
// Handle || (lowest precedence)
const orParts = splitTopLevel(expr, '||');
if (orParts.length > 1) {
return orParts.some((p) => evalExpr(p.trim(), ctx));
return orParts.some((p) => evalExpr(p.trim(), ctx, source));
}
// Handle &&
const andParts = splitTopLevel(expr, '&&');
if (andParts.length > 1) {
return andParts.every((p) => evalExpr(p.trim(), ctx));
return andParts.every((p) => evalExpr(p.trim(), ctx, source));
}
// Handle negation
if (expr.startsWith('!')) {
return !evalExpr(expr.slice(1).trim(), ctx);
return !evalExpr(expr.slice(1).trim(), ctx, source);
}
// Handle 'in'
const inMatch = expr.match(/^(.+?)\s+in\s+(\[.*\])$/);
if (inMatch) {
const left = resolveValue(inMatch[1].trim(), ctx);
const right = parseLiteral(inMatch[2]);
const left = resolveValue(inMatch[1].trim(), ctx, source);
const right = parseLiteral(inMatch[2], source);
return Array.isArray(right) && right.includes(left as never);
}
// Handle == / != (CEL-style loose equality: null == undefined)
const eqMatch = expr.match(/^(.+?)\s*(==|!=)\s*(.+)$/);
if (eqMatch) {
const left = resolveValue(eqMatch[1].trim(), ctx);
const right = parseLiteral(eqMatch[3].trim());
const left = resolveValue(eqMatch[1].trim(), ctx, source);
const right = parseLiteral(eqMatch[3].trim(), source);
const nullish = (v: unknown) => v === null || v === undefined;
const equal = nullish(left) && nullish(right) ? true : left === right;
return eqMatch[2] === '==' ? equal : !equal;
}
// Bare truthy check
return Boolean(resolveValue(expr, ctx));
return Boolean(resolveValue(expr, ctx, source));
}

function splitTopLevel(expr: string, op: string): string[] {
Expand DownExpand Up@@ -221,10 +291,11 @@ function splitTopLevel(expr: string, op: string): string[] {
function resolveValue(
path: string,
ctx: { data: Record<string, unknown> },
source: string,
): unknown {
// Allow literals on the left side too.
if (/^['"]/.test(path) || /^-?\d/.test(path) || path === 'true' || path === 'false' || path === 'null') {
return parseLiteral(path);
return parseLiteral(path, source);
}
const segs = path.split('.');
// The root identifier must be a name the scope actually declares. `hasOwn`,
Expand All@@ -245,7 +316,7 @@ function resolveValue(
return cur;
}

function parseLiteral(raw: string): unknown {
function parseLiteral(raw: string, source: string): unknown {
const s = raw.trim();
if (s === 'true') return true;
if (s === 'false') return false;
Expand All@@ -266,5 +337,12 @@ function parseLiteral(raw: string): unknown {
return [];
}
}
// The tail: `s` is not a literal this evaluator recognises, so it is handed
// back as itself and compared as text. Every route to this line carries the
// diagnostic (objectui#4049) — the right side of `==` / `!=`, and the
// digit-leading literal shortcut in `resolveValue` used by `in`'s left side,
// a bare truthy check and the left of `==`. NOTE the verdict is untouched:
// `s` is still returned verbatim, exactly as before.
if (PATH_SHAPED_LITERAL.test(s)) warnPathShapedLiteral(s, source);
return s;
}
Loading