Skip to content

Commit 42cc219

Browse files
os-zhuangclaude
andauthored
feat(spec): declare the settings visible grammar the evaluator actually implements (#7327) (#7387)
* feat(spec): declare the settings `visible` grammar the evaluator actually implements (#7327) Both settings-manifest `visible` slots — specifier-level and manifest-level — were typed `ExpressionInputSchema`, whose bare-string arm normalises to `dialect: 'cel'`. Nothing has ever evaluated them as CEL: their only readers are the console's client-side `new Function(...)` and, since #7310, the server-side `evaluateVisibility`, which implements a small closed grammar. `===` / `!==` — used throughout the bundled manifests — are not CEL at all. #7169 measured which side should move: routing the declared CEL into evaluation breaks 93 of the 94 bundled predicates, narrowing the declaration breaks 1, and #7310's relational-operator extension had already taken that 1 to 0. Per the maintainer's 2026-08-10 ruling (and #7071's "each protocol keeps its own spelling"), the declaration moves. Both slots now accept exactly the evaluated grammar: single root `data`, one level of member access, `|| && !`, `=== !== == != >= <= > <`, parentheses and string/number/bool/null literals, optionally `${...}`-wrapped. Bare string and `{ dialect, source }` envelope are both still accepted and a bare string still normalises to the canonical envelope, so the wire shape does not move — only the accepted `source` strings narrow. Real CEL (`data.x in [...]`, `size(data.y) > 0`, `data.a.b == 1`) is refused at publish/parse with a message naming the offending source, the reason, and the grammar that would work. #7310's save-time refusal stays as defense in depth. A second statement of one grammar is the drift that caused #7169, so the two are pinned to each other: `settings-visibility-declaration.pin.test.ts` asserts "the schema accepts it" and "the evaluator can parse it" are the same bit, over an in/out-of-grammar table and over the real corpus — re-measured at 10 manifests / 94 predicates, 0 refused. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdPj3S347aPWapzTuHCb4N * fix(qa): keep the settings `visible` slot in the ADR-0058 expression ratchet, classified honestly (#7327) The expression-surface conformance ratchet discovers surfaces by matching `<key>: ExpressionInputSchema` textually, so narrowing the two settings `visible` slots onto their own schema dropped them out of the scan and turned their ledger entry stale — a live predicate surface silently leaving the ledger, which is the #1887 class the ledger exists to catch. Discovery now reads a registered list of expression-declaring schema names rather than one hardcoded name, with the failure mode written down: a slot narrowed onto its own schema must register that schema on the same commit. The classification is corrected while it is being moved. `settings-manifest visible` sat under `cel-ui` — `dialect: 'cel'`, enforced by the SchemaRenderer and celEngine — and is evaluated by neither. It gets its own `settings-visibility` row naming `evaluateVisibility`, its closed grammar, and its fail-closed policy (#7310), proved by the producer/consumer pin. `ExprDialect` gains a member for it: the ledger records what a surface IS, and spelling this one `cel` would restate in the ledger the exact claim #7327 removes from the schema. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdPj3S347aPWapzTuHCb4N --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent dadd1ad commit 42cc219

7 files changed

Lines changed: 616 additions & 14 deletions

File tree

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): the settings-manifest `visible` slots declare the grammar they are actually evaluated with, instead of claiming CEL (#7327, the alignment half of #7169)
6+
7+
Both `visible` slots on a settings manifest — specifier-level and
8+
manifest-level — were typed `ExpressionInputSchema`, the shared expression
9+
input whose bare-string arm normalises to `dialect: 'cel'`. Nothing has ever
10+
evaluated them as CEL. Their only two readers are the console's client-side
11+
`new Function(...)` over the raw string and, since #7310, the server-side
12+
`evaluateVisibility` in `@objectstack/service-settings`, which implements a
13+
deliberately tiny closed grammar. So the declared dialect and the evaluated
14+
dialect disagreed, and the disagreement was **not** cosmetic: `===` and `!==`,
15+
which the bundled manifests use throughout, are not CEL operators at all.
16+
17+
**The measurement decided which side moves.** #7169 counted the corpus — 94
18+
`visible` predicates across the 10 bundled manifests, 27 distinct sources.
19+
Wiring the *declared* CEL into evaluation breaks **93 of 94**, syntactically
20+
and totally, plus every manifest stored outside this repo. Narrowing the
21+
*declaration* to the grammar already evaluated breaks **1**, and #7310's
22+
relational-operator extension had already absorbed that one, taking it to
23+
**0**. The maintainer's 2026-08-10 ruling took the second direction, and
24+
#7071's ruling on `ExpressionInput` ("each protocol keeps its own spelling")
25+
named this narrowing as the follow-up.
26+
27+
**After:** both slots accept the grammar the evaluator implements and nothing
28+
else — a single root `data` with one level of member access, the operators
29+
`||` `&&` `!` and `===` `!==` `==` `!=` `>=` `<=` `>` `<`, parentheses, and
30+
string / number / `true` / `false` / `null` literals, optionally wrapped in
31+
`${…}`. A bare string and a `{ dialect, source }` envelope are both still
32+
accepted, and a bare string still normalises to the canonical envelope, so
33+
**the wire shape does not move** — only the set of accepted `source` strings
34+
narrows.
35+
36+
An author who reaches for real CEL is now told so where it is cheap to fix:
37+
38+
```
39+
Unsupported `visible` predicate "data.provider in ['smtp', 'resend']":
40+
unsupported identifier "in" — the only root is `data`. A settings `visible`
41+
predicate is not CEL: … Rewrite CEL membership as an `||` chain
42+
(`${data.x === 'a' || data.x === 'b'}`); function calls, macros and member
43+
paths deeper than one level have no equivalent here.
44+
```
45+
46+
Previously that predicate passed every publish-time gate and then failed the
47+
tenant's next save — and before #7310, did not even fail: it silently switched
48+
off `required`, `options`, `pattern`, `valueDomain` and the value window on its
49+
key. #7310's save-time refusal stays exactly where it is, as defense in depth:
50+
this is the producer-side check, that is the consumer-side check.
51+
52+
**The two sides are pinned to each other**, because a second statement of one
53+
grammar is exactly the drift that caused #7169 in the first place.
54+
`service-settings/src/settings-visibility-declaration.pin.test.ts` asserts that
55+
"the schema accepts it" and "the evaluator can parse it" are the same bit, over
56+
an in-grammar / out-of-grammar table *and* over the real bundled corpus — which
57+
it re-measures at 10 manifests / 94 predicates, 0 refused.
58+
59+
**Upgrading:** every bundled manifest is unaffected (measured, 0 refusals). A
60+
third-party manifest is affected only if it carries a `visible` predicate the
61+
save path already could not evaluate; the refusal names the predicate, the
62+
reason and the supported grammar. `minor` rather than `major` follows the
63+
repo's precedent for narrowing acceptance on one authorable key
64+
(`action-param-strict-unknown-keys`, `chart-aggregate-groupby-strict`) — this
65+
removes no authorable surface with reachable behaviour, so it is not the
66+
`major` class of #6188 / #6815.

content/docs/references/system/settings-manifest.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ const result = ResolvedSettingValueSchema.parse(data);
8787
| **category** | `string` | optional | Settings hub category |
8888
| **order** | `number` | optional | Display order |
8989
| **specifiers** | `{ type: Enum<'group' \| 'child_pane' \| 'info_banner' \| 'title_value' \| 'text' \| 'textarea' \| … +13 more>; id?: string; key?: string; label: string \| Record<string, string>; … }[]` || Page contents (ordered) |
90-
| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Whole-manifest visibility |
90+
| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Whole-manifest visibility. Grammar is NOT CEL: root `data` with one-level member access, `\|\|` `&&` `!`, `===` `!==` `==` `!=` `>=` `<=` `>` `<`, parentheses and string/number/bool/null literals, optionally wrapped in `${...}`; bare string or `{ dialect, source }` envelope. |
9191
| **featureFlag** | `string` | optional | Gate manifest visibility on a feature flag |
9292
| **beta** | `boolean` | optional | Show a Beta chip on the page |
9393

@@ -119,7 +119,7 @@ const result = ResolvedSettingValueSchema.parse(data);
119119
| **description** | `string` | optional | Help text |
120120
| **icon** | `string` | optional | Icon name (Lucide) |
121121
| **default** | `any` | optional | Default value |
122-
| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility expression |
122+
| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility expression evaluated against the namespace value map, e.g. `${data.provider === 'smtp'}`. Hidden specifiers are not rendered and their values are not validated. Grammar is NOT CEL: root `data` with one-level member access, `\|\|` `&&` `!`, `===` `!==` `==` `!=` `>=` `<=` `>` `<`, parentheses and string/number/bool/null literals, optionally wrapped in `${...}`; bare string or `{ dialect, source }` envelope. |
123123
| **required** | `boolean` | optional | Required field |
124124
| **encrypted** | `boolean` | optional | Encrypt value at rest (forced true for password) |
125125
| **scope** | `Enum<'global' \| 'tenant' \| 'user'>` | optional | Override manifest scope for this key |

packages/qa/dogfood/test/expression-conformance.ledger.ts

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,24 @@ import type { ConformanceRow } from '@objectstack/verify';
1515
// the celEngine). There is NO silent fallback from compile to interpret.
1616
//
1717
// The companion test (`expression-conformance.test.ts`) RE-DISCOVERS every
18-
// `ExpressionInputSchema` field declaration in `packages/spec/src` (plus the RLS
18+
// expression-declaring field in `packages/spec/src` (plus the RLS
1919
// `using`/`check` string predicates) and asserts each is `covers`-ed by exactly
2020
// one row. A NEW expression surface that nobody classified — the #1887 class of
21-
// "declared-but-unwired predicate" — breaks the build.
21+
// "declared-but-unwired predicate" — breaks the build. Discovery is by SCHEMA
22+
// NAME (`EXPRESSION_INPUT_SCHEMAS` in that file), so a slot narrowed onto its
23+
// own schema must register that schema there or it drops out of the scan.
2224

2325
export type ExprMode = 'compile' | 'interpret';
24-
export type ExprDialect = 'cel' | 'cron' | 'template' | 'js';
26+
/**
27+
* What a surface is ACTUALLY evaluated as — deliberately not the spec's
28+
* `ExpressionDialect` enum. `js` outlives its retirement from that enum
29+
* (#3278), and `settings-visibility` never was in it: the settings manifest
30+
* `visible` slots carry a closed hand-rolled grammar with its own evaluator
31+
* (#7169 / #7327). A ledger that could only spell the three declared dialects
32+
* would have to record the settings slot as `cel`, which is the exact
33+
* misclassification it exists to surface.
34+
*/
35+
export type ExprDialect = 'cel' | 'cron' | 'template' | 'js' | 'settings-visibility';
2536
export type ExprState = 'enforced' | 'experimental' | 'removed';
2637
/** ADR-0058 D5 fail-policy tiers. */
2738
export type FailPolicy = 'compile-error' | 'fail-closed' | 'fail-soft-log' | 'throw';
@@ -143,9 +154,34 @@ export const EXPRESSION_SURFACE: ExprSurface[] = [
143154
// this new surface). Interpreted by the objectui page:tabs renderer to
144155
// omit the whole tab (header + panel) when FALSE.
145156
'ui/component.zod.ts:visibleWhen',
146-
'system/settings-manifest.zod.ts:visible',
157+
// `system/settings-manifest.zod.ts:visible` used to sit here. It never
158+
// belonged: this row's dialect is `cel` and its enforcement is the
159+
// SchemaRenderer + celEngine, and the settings slot is evaluated by
160+
// neither. Split out as `settings-visibility` in #7327.
147161
],
148162
},
163+
{
164+
id: 'settings-visibility',
165+
summary: 'settings-manifest `visible` — specifier + whole-manifest gating (a closed non-CEL grammar)',
166+
// The one row in this ledger whose dialect is NOT one of the spec's three.
167+
// That is the finding, not an oversight: the slot was typed
168+
// `ExpressionInputSchema` — which labels its contents CEL — while its only
169+
// two evaluators read a small hand-rolled grammar. Measured in #7169 over
170+
// the 94 bundled predicates: routing them through CEL breaks 93 (`===` and
171+
// `!==` are not CEL operators at all), so the maintainer's 2026-08-10
172+
// ruling moved the DECLARATION rather than the evaluator. Classifying this
173+
// `cel` under `cel-ui` would restate here the exact claim #7327 removed
174+
// from the schema.
175+
dialect: 'settings-visibility', mode: 'interpret', state: 'enforced', failPolicy: 'fail-closed',
176+
enforcement:
177+
'service-settings `evaluateVisibility` (visibility-eval.ts), called from `SettingsService.validatePatch` — a closed grammar: single root `data`, one-level member access, `|| && !`, `=== !== == != >= <= > <`, parens, and string/number/bool/null literals, optionally `${…}`-wrapped, as a bare string or a `{dialect, source}` envelope. Fail-closed since #7310: a predicate outside the grammar REFUSES the save (SettingsValidationError, HTTP 400) instead of skipping the specifier — `visible` gates every other check on the key (`required`, `options`, `pattern`, `valueDomain`, the value window), so skipping it switched all of them off at once. The console evaluates the same string client-side through `new Function(...)`. Since #7327 the spec DECLARES that same grammar (`SettingsVisibilityInputSchema`), so it is refused at publish/parse too',
178+
covers: ['system/settings-manifest.zod.ts:visible'],
179+
// Proof is the producer/consumer pin rather than a runtime fixture: the
180+
// failure mode this surface actually has is the two sides disagreeing about
181+
// what parses, which is what that file measures — over the real bundled
182+
// corpus and an in/out-of-grammar table.
183+
proof: 'packages/services/service-settings/src/settings-visibility-declaration.pin.test.ts',
184+
},
149185
{
150186
id: 'cel-action-param-option-visible',
151187
summary: "action param option-list per-option gating (params[].options[].visibleWhen, #5016)",

packages/qa/dogfood/test/expression-conformance.test.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22
//
33
// ADR-0058 D7 — the Expression Surface Conformance ledger is a CHECKED artifact.
44
// Refactored onto the reusable ADR-0060 `checkLedger` helper: one call asserts
5-
// the shared invariants AND the ratchet (re-discover every ExpressionInputSchema
6-
// field in packages/spec/src + the RLS using/check predicates; fail if any is
7-
// unclassified). The expression-specific invariants (mode/dialect/fail-policy,
5+
// the shared invariants AND the ratchet (re-discover every expression-declaring
6+
// field in packages/spec/src — see EXPRESSION_INPUT_SCHEMAS — plus the RLS
7+
// using/check predicates; fail if any is unclassified). Discovery is by SCHEMA
8+
// NAME, so a slot that moves to a narrower schema leaves the scan unless that
9+
// schema is registered: #7327 is the worked example. The
10+
// expression-specific invariants (mode/dialect/fail-policy,
811
// compile rows name the canonical compiler) stay here.
912

1013
import { describe, expect, it } from 'vitest';
@@ -20,7 +23,26 @@ const SPEC_SRC = join(REPO_ROOT, 'packages/spec/src');
2023

2124
const MODES = new Set(['compile', 'interpret']);
2225
const FAIL_POLICIES = new Set(['compile-error', 'fail-closed', 'fail-soft-log', 'throw']);
23-
const DIALECTS = new Set(['cel', 'cron', 'template', 'js']);
26+
// `settings-visibility` is not one of the spec's `ExpressionDialect` members on
27+
// purpose (#7327): it is a closed non-CEL grammar with its own evaluator, and
28+
// the ledger's job is to say what a surface IS, not what its schema used to
29+
// claim. See the `settings-visibility` row.
30+
const DIALECTS = new Set(['cel', 'cron', 'template', 'js', 'settings-visibility']);
31+
32+
/**
33+
* Schemas that DECLARE an expression surface. `ExpressionInputSchema` is the
34+
* shared one; a slot whose accepted grammar is narrower gets its own schema and
35+
* must be listed here too, or the ratchet silently stops watching it.
36+
*
37+
* That is not hypothetical — it is how this scan behaves by construction, and
38+
* #7327 hit it: narrowing the settings `visible` slots off `ExpressionInputSchema`
39+
* dropped them out of discovery and turned their ledger entry stale. A new
40+
* narrowed alias belongs in this list on the same commit that introduces it.
41+
*/
42+
const EXPRESSION_INPUT_SCHEMAS = ['ExpressionInputSchema', 'SettingsVisibilityInputSchema'];
43+
const DECLARES_EXPRESSION = new RegExp(
44+
String.raw`^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*:\s*(?:${EXPRESSION_INPUT_SCHEMAS.join('|')})\b`,
45+
);
2446

2547
/** Re-discover every expression surface in the spec — the SAME scan the ledger encodes. */
2648
function discoverSurfaces(): Set<string> {
@@ -34,7 +56,7 @@ function discoverSurfaces(): Set<string> {
3456
else if (ent.isFile() && ent.name.endsWith('.zod.ts')) {
3557
const rel = relative(SPEC_SRC, p);
3658
for (const line of readFileSync(p, 'utf8').split('\n')) {
37-
const m = line.match(/^\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*:\s*ExpressionInputSchema\b/);
59+
const m = line.match(DECLARES_EXPRESSION);
3860
if (m) found.add(`${rel}:${m[1]}`);
3961
}
4062
}

0 commit comments

Comments
 (0)