Skip to content

Commit f5b6b3f

Browse files
fix(lint)!: page/view visibleWhen refuses a has() argument that is not a field selection (#25313)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wg52sU6eSQpjfDKQEaQYrvSX)_ Fixes #25281 Clause-②: no (narrowing) Metadata-protocol: narrowed Executes ruling `6351260886` on #25281, letter A. Draft until the maintainer ratifies that ruling; the seat runs the contract review and the landing. ## What CEL defines `has()` in one form, `has(e.f)`, with `f` a field name after a dot. The engine refuses any other argument as `has() invalid argument` on every evaluation, whatever the record holds. The page and view `visibleWhen` door (`validateVisibilityPredicates`, so `os validate`, `os build`, `os lint`, `defineStack` and the page and view save doors) did not judge the argument, so `has(status)` and `has(record.items[0])` published clean and the console drew the guarded element for every user. - **`@objectstack/lint`**: a new gating arm, `visibility-predicate-has-argument` (`error`), in the existing family, at the same site and path as its siblings. It fires on a `has()` call whose single argument is not a `.` field selection, reports the first such call per predicate, quotes the argument and the engine's wording, and gives one hint line with the spelling the definition admits: - a bare name: `has(record.status)` or `'status' in record`, with the root the layer binds (`data` on a metadata-editing form); a bare root (`has(record)`) is told `has(record.FIELD)`, never `record.record`; - an index as the last step: `size(record.items) > 0`, or `'k' in e` when the index is a string key; - `e.?f`: `has(e.f)`; anything else: the one form, with the layer root. No null-guard advice is repeated (`validate-null-guards` owns it). `os explain visibility-predicate-has-argument` carries the reasoning. - **The oracle (mechanism (b) of the ruling)**: the canonical AST this rule already walks for the #16118 mask. A `has` call with exactly one argument whose node is not `.` is refused. After `rewriteIndexedHas` (#25267) the engine refuses exactly those arguments, and the suite pins the equivalence against `celEngine.compile` in both directions: 22 refused shapes and 10 accepted shapes. Mechanism (b) was chosen over (a) because the engine reports one error per source: in `bogusFn(1) && has(status)` a compile-based helper would see only the unknown function, while the AST walk sees the `has()` call too. - **The #16118 mask stays load-bearing.** `has(status)` alone yields ONE finding (this one). `has(status) && status == 'x'` keeps its `visibility-bare-identifier` finding on the unguarded `status`, beside the new one. - **`@objectstack/spec`, ADR-0087 kit**: the D3 semantic entry `visibility-predicate-has-argument-refused` (step 18) and a `REFUSED_STORED_SHAPES` detector. `os migrate meta --stored` lists a stored page component, form section or section field carrying the shape under that id. No D2 conversion: the test the author meant is not recorded. `packages/spec` cannot parse CEL, so the detector reads the source through a package-internal token reading (`conversions/cel-has-argument.ts`). A lint-side test (`validate-visibility-predicates.stored-listing.test.ts`) holds the listing, the page save door and `celEngine.compile` to one set over 29 predicates. - **`@objectstack/formula`**: comment only. The STILL OPEN note above `firstUndeclaredReference` records that the page/view door now reports the argument. The first-error masking it describes is unchanged. ## Pins rewritten as part of the ruling (not silently) - The module note: the rule list (five gating rules), §Function existence point 2 ("narrows by exactly TWO axes"), the CEL-TYPE blind-spot bullet ("Function existence and the `has()` argument shape excepted"), and a new §`has()` argument shape that quotes the ruling and states what it does not change. - `validate-visibility-predicates.test.ts`: row 3 (`has(status)` alone), "several bare `has()` arguments" and the comprehension row flip from `[]` to the one `has()` finding, with no bare-identifier finding. The "#13594 narrows this pin by exactly one axis" block becomes two axes and adds `has(status)` beside `totallyBogusFn(1,2)`. ## Unchanged (#13594's scope discipline, verbatim) `has(e.f)` for any `e`, including the indexed and call receivers #25267 lowers; every `dyn` predicate; `type == 'grid'`, `type(record.x) == string`, `upper(1, 2)`, `1 + 'a'` (pinned: no `has()` finding); a `has(` inside a string literal; a member call `x.has(y)`; `has()` and `has(a, b)`, which the engine refuses under another message; the render-side fail-open (ADR-0137 D3). ## Measured before the first edit - HotCRM producers: **0** at `4166f21` (shallow clone). The 550 `has(` occurrences hold no bare-name and no index-last argument in a CEL string. The only hits are comment prose (`(!has(x) || isBlank(x))` in two flow docblocks, one test comment). - This repository: **0** at the base `5801a4f`. Every `has(NAME)` / `has(e[i])` hit is TypeScript `Map`/`Set` `.has(`, a code comment, CHANGELOG or docs prose. - Base `5801a4f`: `has(status)` and `has(record.items[0])` on a form field draw no finding from `validateVisibilityPredicates`, and `celEngine.compile` answers `has() invalid argument` on both. The suite at the base pinned that silence (rows 898–926). ## Tests Run on this branch, each under `scripts/pm/os-verify-lock.sh`: - Before the merge of `main` (head `ce74b8efc`): `@objectstack/lint` 154 files / 7164 tests, `@objectstack/formula` 52 / 1376, `@objectstack/spec` 710 / 21577 (1 todo), all passing; the three packages type-check. - After merging `main` (#25299, head `7f0e0e6fe`): `@objectstack/spec` `src/conversions` + `src/migrations` 26 files / 1036 tests, `@objectstack/lint` 154 / 7164, both passing; spec and lint type-check. The merge kept both refused-shape describes; the new detector reads the stack alone and does not use the context parameter #25279 added. - New suites: `validate-visibility-predicates.test.ts` › `visibility-predicate-has-argument (#25281)` (acceptance pair, engine equivalence both ways, prescriptions, mask, first-only, boundaries, carriers); `validate-visibility-predicates.stored-listing.test.ts` (engine, page door and stored listing over 18 refused + 11 accepted predicates); `conversions/cel-has-argument.test.ts`; the `#25281` describe in `refused-shapes.test.ts`; two rows and a view-door assertion in `runtime-gate.page-visibility-predicate-writes.test.ts`. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives 92 families, the same list before and after the merge. On head `7f0e0e6fe` all 92 were run and each exited 0; `--ran` reconciliation: "92 derived famil(ies) accounted for — 92 run, 0 NOT-MEASURED". This includes `check-empty-changeset`, `check-adr-0087-registration` and `check-changeset-no-major`. (On the pre-merge head `check:dual-build-cjs-loads` exited 3 for a missing dist; with the dists built it exits 0.) ## Dogfood battery `pnpm exec turbo run build --filter='@objectstack/dogfood^...' && pnpm --filter @objectstack/dogfood test` on head `7f0e0e6fe`: 289 files passed, 3 skipped (292); 2491 tests passed, 18 skipped (2509). `VERDICT command-exit 0`. ## Acceptance notes - The detector in `packages/spec` reads tokens, not the engine's AST. A predicate that is also broken CEL can be listed there while the door names its syntax fault first. Both send the owner to the same predicate. - Not addressed here: on the object, flow, action, sharing-rule and hook surfaces, `validateExpression` already refuses both shapes (`invalid-cel`, `has() invalid argument`). Its prescription there is the generic "predicates are bare CEL", not a `has()` spelling. Noted, not filed: the verdict is right and the surface is outside this card. --- _Generated by [Claude Code](https://claude.ai/code/session_01Jc59n5NWaMp1khDwL7Fici)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5f4a3f0 commit f5b6b3f

13 files changed

Lines changed: 1088 additions & 29 deletions
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
'@objectstack/lint': minor
3+
'@objectstack/spec': minor
4+
---
5+
6+
fix(lint)!: a page or view `visibleWhen` that hands `has()` an argument other than a field selection is refused at publish
7+
8+
Clause-②: no (narrowing)
9+
10+
Metadata-protocol: narrowed
11+
12+
<!-- adr-0087: registered visibility-predicate-has-argument-refused -->
13+
14+
**BREAKING** (what an author can declare narrows), shipped as `minor` under the launch-window convention for breaking changes.
15+
16+
CEL defines `has()` in one form, `has(e.f)`, with `f` a field name after a dot. The engine refuses any other argument as `has() invalid argument` on every evaluation, whatever the record holds. Every other predicate surface already refused it through `validateExpression`. The page and view `visibleWhen` door did not, so `has(status)` and `has(record.items[0])` published clean, and the console, which renders a faulting predicate as visible, drew the guarded element for every user.
17+
18+
- **FROM:** a `visibleWhen` on a page component, a form view section or a section field could hand `has()` a name alone (`has(status)`), an index as the last step (`has(record.items[0])`), or any other argument that is not a field selection (`has(record.?a)`, `has(upper(record.x))`, a literal, an operator), and `os validate`, `os build`, `os lint` and the page and view save doors accepted it.
19+
- **TO:** each is refused as `visibility-predicate-has-argument`, an error, at those doors (the save doors answer `422 INVALID_METADATA`). The finding quotes the argument and the engine's wording, and its hint names the spelling to write. `os explain visibility-predicate-has-argument` says why.
20+
- **Unchanged:** `has(e.f)` for any `e`, including a selection over an indexed or called receiver (`has(record.items[0].name)`); every other predicate, and nothing about the types that flow through one is judged. A `has(status) && status == 'x'` keeps its `visibility-bare-identifier` finding on the unguarded `status`, beside the new one. A stored page or view row in the refused shape still loads and is served; its next save is refused, and `os migrate meta --stored` lists each such predicate under `visibility-predicate-has-argument-refused`.
21+
- **The fix for an author:** write `has(record.status)` or `'status' in record` for a field (`data` instead of `record` on a metadata-editing form), and `size(record.items) > 0` to ask whether a list has a first element (`'k' in e` for a bracketed string key).

‎packages/formula/src/cel-engine.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -404,9 +404,13 @@ let recordScopeEnv: Environment | undefined;
404404
* faults `has() invalid argument`. `@objectstack/lint`'s
405405
* `validate-visibility-predicates` masks `has(…)` spans at its own call site
406406
* (#16118) and that mask stays load-bearing; no other consumer has one.
407-
* A select whose receiver is an index or a call (`has(a[0].b)`) is NOT in
408-
* this class: the spec admits it, and {@link rewriteIndexedHas} lowers it
409-
* before this check runs (#25267).
407+
* The page/view door now REPORTS the argument itself, under
408+
* `visibility-predicate-has-argument` (#25281); on every other surface
409+
* `validateExpression`'s compile pass refuses it. Neither changes what this
410+
* helper answers: a `has()` first error still masks the references behind
411+
* it here. A select whose receiver is an index or a call (`has(a[0].b)`) is
412+
* NOT in this class: the spec admits it, and {@link rewriteIndexedHas}
413+
* lowers it before this check runs (#25267).
410414
*
411415
* ⇒ A `null` from this helper is still "nothing was reported", never "the
412416
* source is clean". ⛔ Do not close the remaining classes by widening the regex

‎packages/lint/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ export {
267267
VISIBILITY_PREDICATE_SYNTAX,
268268
VISIBILITY_PREDICATE_OVER_BUDGET,
269269
VISIBILITY_PREDICATE_UNKNOWN_FUNCTION,
270+
VISIBILITY_PREDICATE_HAS_ARGUMENT,
270271
} from './validate-visibility-predicates.js';
271272
export type {
272273
VisibilityFinding,

‎packages/lint/src/rule-explanations.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,23 @@ const VISIBILITY_PREDICATE_UNKNOWN_FUNCTION_EXPLANATION: RuleExplanation = {
853853
],
854854
};
855855

856+
const VISIBILITY_PREDICATE_HAS_ARGUMENT_EXPLANATION: RuleExplanation = {
857+
rule: 'visibility-predicate-has-argument',
858+
covers: 'what `has()` accepts as its argument',
859+
paragraphs: [
860+
'CEL defines the macro in one form, `has(e.f)`: a presence test on field `f` of whatever `e` ' +
861+
'evaluates to, where `f` is a plain field name written after a dot. Any other argument — a name ' +
862+
'alone (`has(status)`), an index as the last step (`has(record.items[0])`), a call, a literal or ' +
863+
'an operator — is refused by the engine as `has() invalid argument` on every evaluation, whatever ' +
864+
'the record holds. On a view or page surface the console then falls OPEN and the element renders ' +
865+
'unconditionally, exactly like one carrying no predicate at all.',
866+
'Write the test the definition admits: `has(record.status)` or `\'status\' in record` for a field ' +
867+
'(`data` instead of `record` on a metadata-editing form), and `size(record.items) > 0` to ask whether ' +
868+
'a list has a first element. `has()` over a selection whose receiver is indexed or called, such as ' +
869+
'`has(record.items[0].name)`, is accepted.',
870+
],
871+
};
872+
856873
const VISIBILITY_BARE_IDENTIFIER_EXPLANATION: RuleExplanation = {
857874
rule: 'visibility-bare-identifier',
858875
covers: 'why every value is read through a namespace',
@@ -4772,6 +4789,7 @@ export const RULE_EXPLANATIONS: Readonly<Record<string, RuleExplanation>> = Obje
47724789
[VISIBILITY_PREDICATE_OVER_BUDGET_EXPLANATION.rule]: VISIBILITY_PREDICATE_OVER_BUDGET_EXPLANATION,
47734790
[VISIBILITY_PREDICATE_SYNTAX_EXPLANATION.rule]: VISIBILITY_PREDICATE_SYNTAX_EXPLANATION,
47744791
[VISIBILITY_PREDICATE_UNKNOWN_FUNCTION_EXPLANATION.rule]: VISIBILITY_PREDICATE_UNKNOWN_FUNCTION_EXPLANATION,
4792+
[VISIBILITY_PREDICATE_HAS_ARGUMENT_EXPLANATION.rule]: VISIBILITY_PREDICATE_HAS_ARGUMENT_EXPLANATION,
47754793
[VISIBILITY_BARE_IDENTIFIER_EXPLANATION.rule]: VISIBILITY_BARE_IDENTIFIER_EXPLANATION,
47764794
[HOOK_BODY_WRITE_UNKNOWN_FIELD_EXPLANATION.rule]: HOOK_BODY_WRITE_UNKNOWN_FIELD_EXPLANATION,
47774795
[HOOK_BODY_WRITE_UNPROVISIONED_ANCHOR_EXPLANATION.rule]: HOOK_BODY_WRITE_UNPROVISIONED_ANCHOR_EXPLANATION,

‎packages/lint/src/runtime-gate.page-visibility-predicate-writes.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
VISIBILITY_BARE_IDENTIFIER,
4444
VISIBILITY_PREDICATE_OVER_BUDGET,
4545
VISIBILITY_PREDICATE_SYNTAX,
46+
VISIBILITY_PREDICATE_HAS_ARGUMENT,
4647
VISIBILITY_PREDICATE_UNKNOWN_FUNCTION,
4748
VISIBILITY_ROOT_MISLAYERED,
4849
} from './validate-visibility-predicates.js';
@@ -59,6 +60,7 @@ const FAMILY_IDS = new Set<string>([
5960
VISIBILITY_BARE_IDENTIFIER,
6061
VISIBILITY_PREDICATE_OVER_BUDGET,
6162
VISIBILITY_PREDICATE_SYNTAX,
63+
VISIBILITY_PREDICATE_HAS_ARGUMENT,
6264
VISIBILITY_PREDICATE_UNKNOWN_FUNCTION,
6365
VISIBILITY_ROOT_MISLAYERED,
6466
PREDICATE_PATH_UNRESOLVED,
@@ -90,6 +92,9 @@ const REFUSED = [
9092
// A wrong reference: no binding root resolves `status`.
9193
{ body: "status == 'open'", rule: VISIBILITY_BARE_IDENTIFIER, subject: '`status`' },
9294
{ body: 'record.status ==', rule: VISIBILITY_PREDICATE_SYNTAX, subject: 'not valid CEL' },
95+
// [#25281] A `has()` argument the engine refuses: a bare name, an index as the last step.
96+
{ body: 'has(status)', rule: VISIBILITY_PREDICATE_HAS_ARGUMENT, subject: '`status`' },
97+
{ body: 'has(record.items[0])', rule: VISIBILITY_PREDICATE_HAS_ARGUMENT, subject: '`record.items[0]`' },
9398
] as const;
9499

95100
/**
@@ -198,6 +203,10 @@ describe('#24491 — the page door gives the build\'s visibility-predicate verdi
198203
expect(fn.map((f) => [f.rule, f.path])).toEqual([
199204
[VISIBILITY_PREDICATE_UNKNOWN_FUNCTION, 'views[0].form.sections[0].fields[0]'],
200205
]);
206+
// [#25281] The `has()` argument arm reaches the view door the same way.
207+
expect(family(gateView(view('has(status)')).errors).map((f) => [f.rule, f.path])).toEqual([
208+
[VISIBILITY_PREDICATE_HAS_ARGUMENT, 'views[0].form.sections[0].fields[0]'],
209+
]);
201210
// The path half is live on the view surface it was built for.
202211
const path = family(gateView(view("data.tpye == 'text'", { provider: 'schema', schemaId: 'field' })).errors);
203212
expect(path.map((f) => f.rule)).toContain(PREDICATE_PATH_UNRESOLVED);
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25281] The stored-shape listing and the refusing door name ONE set.
5+
*
6+
* `os migrate meta --stored` lists a stored page or view whose `visibleWhen`
7+
* hands `has()` an argument that is not a field selection, under the D3 entry
8+
* `visibility-predicate-has-argument-refused`. The listing lives in
9+
* `@objectstack/spec`, which cannot parse CEL, so it reads the source on its
10+
* own (`conversions/cel-has-argument.ts`). The refusal is this package's
11+
* `visibility-predicate-has-argument`, whose verdict is the engine's AST. This
12+
* file holds the two equal over every shape the engine was measured on: a
13+
* predicate is listed exactly when the door refuses it under that id, and the
14+
* door refuses exactly what `celEngine.compile` refuses as `has() invalid
15+
* argument`.
16+
*/
17+
import { describe, expect, it } from 'vitest';
18+
import { applyConversionsToStoredItem, type ConversionTodoNotice } from '@objectstack/spec';
19+
import { celEngine } from '@objectstack/formula';
20+
21+
import { runRuntimeAuthoringRules } from './runtime-gate.js';
22+
import { VISIBILITY_PREDICATE_HAS_ARGUMENT } from './validate-visibility-predicates.js';
23+
24+
const ENTRY_ID = 'visibility-predicate-has-argument-refused';
25+
26+
const page = (visibleWhen: string) => ({
27+
name: 'pipeline_home',
28+
label: 'Pipeline',
29+
type: 'app',
30+
regions: [{ name: 'main', components: [{ type: 'page:header', properties: { title: 'Pipeline' }, visibleWhen }] }],
31+
});
32+
33+
/** Does the stored pass list this page under the entry? */
34+
function listed(visibleWhen: string): boolean {
35+
const todos: ConversionTodoNotice[] = [];
36+
applyConversionsToStoredItem('page', page(visibleWhen), { onTodo: (t) => todos.push(t) });
37+
return todos.some((t) => t.conversionId === ENTRY_ID);
38+
}
39+
40+
/** Does the page save door refuse it under the rule? */
41+
function refusedAtDoor(visibleWhen: string): boolean {
42+
const result = runRuntimeAuthoringRules({ type: 'page', item: page(visibleWhen), context: { objects: [] } });
43+
return result.errors.some((f) => f.rule === VISIBILITY_PREDICATE_HAS_ARGUMENT);
44+
}
45+
46+
function engineRefuses(visibleWhen: string): boolean {
47+
const compiled = celEngine.compile(visibleWhen);
48+
return !compiled.ok && compiled.error.message.split('\n')[0].trim() === 'has() invalid argument';
49+
}
50+
51+
const REFUSED = [
52+
'has(status)',
53+
'has(record)',
54+
'has(record.items[0])',
55+
"has(record['k'])",
56+
'has(record.items[size(record.items) - 1])',
57+
'has(record.?a)',
58+
'has(record.a.?b)',
59+
'has(1)',
60+
"has('x')",
61+
'has(true)',
62+
'has(record.x.size())',
63+
'has(upper(record.x))',
64+
'has(-record.a)',
65+
'has(record.a == 1)',
66+
'has(record.a ? record.b : record.c)',
67+
'has(record.x) || has(record.items[0])',
68+
'record.items.exists(i, has(i))',
69+
'has (record)',
70+
];
71+
72+
const ACCEPTED = [
73+
'has(record.a)',
74+
'has(record.a.b.c.d)',
75+
'has((record.a))',
76+
'has(record.items[0].x)',
77+
"has(record['a'].b)",
78+
'has(dyn(record).a)',
79+
'has(record.a.?b.c)',
80+
'record.items.exists(i, has(i.x))',
81+
"has(record.a) && record.a != ''",
82+
"record.note == 'has(x)'",
83+
'record.tags.has(record.a)',
84+
];
85+
86+
describe('#25281 — the stored listing, the save door and the engine agree on `has()` arguments', () => {
87+
it.each(REFUSED)('%s — the engine refuses it, the door refuses it, the stored pass lists it', (predicate) => {
88+
expect(engineRefuses(predicate), 'engine').toBe(true);
89+
expect(refusedAtDoor(predicate), 'door').toBe(true);
90+
expect(listed(predicate), 'listing').toBe(true);
91+
});
92+
93+
it.each(ACCEPTED)('%s — none of the three refuses or lists it', (predicate) => {
94+
expect(engineRefuses(predicate), 'engine').toBe(false);
95+
expect(refusedAtDoor(predicate), 'door').toBe(false);
96+
expect(listed(predicate), 'listing').toBe(false);
97+
});
98+
});

0 commit comments

Comments
 (0)