Skip to content

Commit 60b3120

Browse files
committed
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
1 parent 7fbc7e5 commit 60b3120

2 files changed

Lines changed: 67 additions & 9 deletions

File tree

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)