feat(spec): declare the settings visible grammar the evaluator actually implements (#7327)

claude · claude · commit 7fbc7e52aa5f · 2026-08-10T08:31:30.000Z
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
diff --git a/.changeset/settings-visible-grammar-declared.md b/.changeset/settings-visible-grammar-declared.md
@@ -0,0 +1,66 @@
+---
+"@objectstack/spec": minor
+---
+
+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)
+
+Both `visible` slots on a settings manifest — specifier-level and
+manifest-level — were typed `ExpressionInputSchema`, the shared expression
+input whose bare-string arm normalises to `dialect: 'cel'`. Nothing has ever
+evaluated them as CEL. Their only two readers are the console's client-side
+`new Function(...)` over the raw string and, since #7310, the server-side
+`evaluateVisibility` in `@objectstack/service-settings`, which implements a
+deliberately tiny closed grammar. So the declared dialect and the evaluated
+dialect disagreed, and the disagreement was **not** cosmetic: `===` and `!==`,
+which the bundled manifests use throughout, are not CEL operators at all.
+
+**The measurement decided which side moves.** #7169 counted the corpus — 94
+`visible` predicates across the 10 bundled manifests, 27 distinct sources.
+Wiring the *declared* CEL into evaluation breaks **93 of 94**, syntactically
+and totally, plus every manifest stored outside this repo. Narrowing the
+*declaration* to the grammar already evaluated breaks **1**, and #7310's
+relational-operator extension had already absorbed that one, taking it to
+**0**. The maintainer's 2026-08-10 ruling took the second direction, and
+#7071's ruling on `ExpressionInput` ("each protocol keeps its own spelling")
+named this narrowing as the follow-up.
+
+**After:** both slots accept the grammar the evaluator implements and nothing
+else — a single root `data` with one level of member access, the operators
+`||` `&&` `!` and `===` `!==` `==` `!=` `>=` `<=` `>` `<`, parentheses, and
+string / number / `true` / `false` / `null` literals, optionally wrapped in
+`${…}`. A bare string and a `{ 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 set of accepted `source` strings
+narrows.
+
+An author who reaches for real CEL is now told so where it is cheap to fix:
+
+```
+Unsupported `visible` predicate "data.provider in ['smtp', 'resend']":
+unsupported identifier "in" — the only root is `data`. A settings `visible`
+predicate is not CEL: … Rewrite CEL membership as an `||` chain
+(`${data.x === 'a' || data.x === 'b'}`); function calls, macros and member
+paths deeper than one level have no equivalent here.
+```
+
+Previously that predicate passed every publish-time gate and then failed the
+tenant's next save — and before #7310, did not even fail: it silently switched
+off `required`, `options`, `pattern`, `valueDomain` and the value window on its
+key. #7310's save-time refusal stays exactly where it is, as defense in depth:
+this is the producer-side check, that is the consumer-side check.
+
+**The two sides are pinned to each other**, because a second statement of one
+grammar is exactly the drift that caused #7169 in the first place.
+`service-settings/src/settings-visibility-declaration.pin.test.ts` asserts that
+"the schema accepts it" and "the evaluator can parse it" are the same bit, over
+an in-grammar / out-of-grammar table *and* over the real bundled corpus — which
+it re-measures at 10 manifests / 94 predicates, 0 refused.
+
+**Upgrading:** every bundled manifest is unaffected (measured, 0 refusals). A
+third-party manifest is affected only if it carries a `visible` predicate the
+save path already could not evaluate; the refusal names the predicate, the
+reason and the supported grammar. `minor` rather than `major` follows the
+repo's precedent for narrowing acceptance on one authorable key
+(`action-param-strict-unknown-keys`, `chart-aggregate-groupby-strict`) — this
+removes no authorable surface with reachable behaviour, so it is not the
+`major` class of #6188 / #6815.
diff --git a/content/docs/references/system/settings-manifest.mdx b/content/docs/references/system/settings-manifest.mdx
@@ -87,7 +87,7 @@ const result = ResolvedSettingValueSchema.parse(data);
 | **category** | `string` | optional | Settings hub category |
 | **order** | `number` | optional | Display order |
 | **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) |
-| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Whole-manifest visibility |
+| **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. |
 | **featureFlag** | `string` | optional | Gate manifest visibility on a feature flag |
 | **beta** | `boolean` | optional | Show a Beta chip on the page |
 
@@ -119,7 +119,7 @@ const result = ResolvedSettingValueSchema.parse(data);
 | **description** | `string` | optional | Help text |
 | **icon** | `string` | optional | Icon name (Lucide) |
 | **default** | `any` | optional | Default value |
-| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility expression |
+| **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. |
 | **required** | `boolean` | optional | Required field |
 | **encrypted** | `boolean` | optional | Encrypt value at rest (forced true for password) |
 | **scope** | `Enum<'global' \| 'tenant' \| 'user'>` | optional | Override manifest scope for this key |
diff --git a/packages/services/service-settings/src/settings-visibility-declaration.pin.test.ts b/packages/services/service-settings/src/settings-visibility-declaration.pin.test.ts
@@ -0,0 +1,163 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// The producer/consumer pin for settings `visible` (#7327, the alignment half
+// of #7169).
+//
+// There are two checks on a `visible` predicate and they live in different
+// packages. The DECLARATION — `SettingsManifestSchema` in `@objectstack/spec`
+// — refuses an unsupported predicate at publish/parse. The EVALUATOR —
+// `evaluateVisibility` in this package — refuses it at save (#7310, fail
+// closed). Those two agreeing is not a nice-to-have: #7169 was caused by a
+// producer and a consumer disagreeing about what parses, and answering it by
+// adding a SECOND grammar statement would have reproduced the bug class if the
+// two were left free to drift.
+//
+// So this file asserts the pairing directly, from the consumer side (spec
+// cannot import a service — Prime Directive #2, the spec declares, it does not
+// execute). Two facts:
+//
+//   1. The corpus. Every bundled manifest still parses. This is the #7169
+//      measurement (94 predicates / 10 manifests) carried onto the declaration
+//      side: narrowing that refused even one of them would brick publishing of
+//      a builtin namespace, and the whole reason direction (b) was chosen over
+//      direction (a) is that this number is 0 rather than 93.
+//   2. The agreement table. For a spread of in-grammar and out-of-grammar
+//      sources, "the schema accepts it" and "the evaluator can parse it" are
+//      the same bit.
+
+import { describe, it, expect } from 'vitest';
+import { SettingsManifestSchema, SpecifierSchema } from '@objectstack/spec/system';
+import { builtinSettingsManifests } from './manifests/index.js';
+import { evaluateVisibility, visibilitySource } from './visibility-eval.js';
+
+/** Wrap a predicate in the smallest specifier that carries one. */
+const specifierWith = (visible: unknown) => ({
+  type: 'text' as const,
+  key: 'some_key',
+  label: 'Some key',
+  visible,
+});
+
+const declarationAccepts = (visible: unknown): boolean =>
+  SpecifierSchema.safeParse(specifierWith(visible)).success;
+
+const evaluatorParses = (visible: unknown): boolean => {
+  try {
+    evaluateVisibility(visible, {});
+    return true;
+  } catch {
+    return false;
+  }
+};
+
+describe('settings `visible` — declaration ⇄ evaluator', () => {
+  it('the corpus: every bundled manifest parses against the narrowed declaration', () => {
+    // The #7169 corpus, on the declaration side. Expected refusals: 0.
+    const refusals: string[] = [];
+    for (const manifest of builtinSettingsManifests as Array<Record<string, any>>) {
+      const parsed = SettingsManifestSchema.safeParse(manifest);
+      if (!parsed.success) {
+        refusals.push(`${manifest.namespace}: ${parsed.error.issues.map((i) => `${i.path.join('.')} — ${i.message}`).join(' | ')}`);
+      }
+    }
+    expect(refusals).toEqual([]);
+  });
+
+  it('the corpus is still the size the ruling measured — 10 manifests, 94 predicates', () => {
+    // If a bundled manifest gains or loses a `visible`, this number moves and
+    // the reader is sent back to the measurement rather than trusting a stale
+    // one. It is a tripwire on the premise, not a rule about how many
+    // predicates a manifest may have — move it deliberately.
+    const predicates = (builtinSettingsManifests as Array<Record<string, any>>).flatMap((m) => [
+      ...(typeof m.visible === 'undefined' ? [] : [m.visible]),
+      ...(m.specifiers ?? []).filter((s: any) => typeof s.visible !== 'undefined').map((s: any) => s.visible),
+    ]);
+    expect(builtinSettingsManifests).toHaveLength(10);
+    expect(predicates).toHaveLength(94);
+  });
+
+  it('every bundled predicate is accepted by BOTH sides', () => {
+    for (const manifest of builtinSettingsManifests as Array<Record<string, any>>) {
+      for (const spec of manifest.specifiers ?? []) {
+        if (typeof spec.visible === 'undefined') continue;
+        const where = `${manifest.namespace}.${spec.key ?? '(layout)'}: ${visibilitySource(spec.visible)}`;
+        expect(declarationAccepts(spec.visible), `declaration refused ${where}`).toBe(true);
+        expect(evaluatorParses(spec.visible), `evaluator refused ${where}`).toBe(true);
+      }
+    }
+  });
+
+  describe('the two sides answer the same bit', () => {
+    const IN_GRAMMAR = [
+      // Every shape the bundled corpus actually uses.
+      "${data.provider === 'smtp'}",
+      "${data.provider !== 'memory'}",
+      '${data.email_password_enabled !== false}',
+      '${data.mfa_required === true}',
+      '${data.lockout_threshold > 0}',
+      "${data.provider === 'resend' || data.provider === 'postmark'}",
+      "${data.embedder_provider && data.embedder_provider !== 'none'}",
+      "${data.provider !== 'memory' && data.title_generation_enabled !== false}",
+      // Reachable but unused by the bundled set — declared is declared.
+      '${!data.disabled}',
+      '${data.count <= 10 && (data.a == 1 || data.b != null)}',
+      '${data.ratio >= 0.5}',
+      "${data.mode < 'z'}",
+      // Both accepted envelopes, and the unwrapped spelling.
+      "data.provider === 'smtp'",
+      { dialect: 'cel' as const, source: "${data.provider === 'smtp'}" },
+      { dialect: 'cel' as const, source: "data.provider === 'smtp'" },
+    ];
+
+    const OUT_OF_GRAMMAR = [
+      // Real CEL an author would reasonably reach for — the case #7327 exists
+      // to move from a save-time failure to a publish-time one.
+      "${data.provider in ['smtp', 'resend']}",
+      '${size(data.recipients) > 0}',
+      "${data.provider.startsWith('s')}",
+      '${data.nested.field === 1}',
+      "${current_user.role === 'admin'}",
+      "${has(data.provider) ? 'a' : 'b'}",
+      // Malformed rather than mis-dialected.
+      "${data.provider === 'smtp'",
+      '${data.provider ===}',
+      "${data.a === 'x'} && ${data.b === 'y'}",
+      '${data.provider === }{',
+      '${-5 > data.x}',
+    ];
+
+    const labelled = (xs: unknown[]) => xs.map((v) => [JSON.stringify(v), v] as const);
+
+    it.each(labelled(IN_GRAMMAR))('accepts %s on both sides', (_label, visible) => {
+      expect(evaluatorParses(visible)).toBe(true);
+      expect(declarationAccepts(visible)).toBe(true);
+    });
+
+    it.each(labelled(OUT_OF_GRAMMAR))('refuses %s on both sides', (_label, visible) => {
+      expect(evaluatorParses(visible)).toBe(false);
+      expect(declarationAccepts(visible)).toBe(false);
+    });
+  });
+
+  it('the publish-time refusal prescribes the grammar instead of just naming a violation', () => {
+    const result = SpecifierSchema.safeParse(specifierWith("${data.provider in ['smtp', 'resend']}"));
+    expect(result.success).toBe(false);
+    const message = result.success ? '' : result.error.issues.map((i) => i.message).join('\n');
+    // The offending source, why, and — the point of the card — what to write.
+    expect(message).toContain("data.provider in ['smtp', 'resend']");
+    expect(message).toContain('unsupported identifier "in"');
+    expect(message).toContain('not CEL');
+    expect(message).toContain('`===`');
+    expect(message).toContain('one-level member access');
+  });
+
+  it('the manifest-level slot carries the same grammar as the specifier-level one', () => {
+    const base = {
+      namespace: 'demo',
+      label: 'Demo',
+      specifiers: [{ type: 'text' as const, key: 'some_key', label: 'Some key' }],
+    };
+    expect(SettingsManifestSchema.safeParse({ ...base, visible: "${data.tier === 'pro'}" }).success).toBe(true);
+    expect(SettingsManifestSchema.safeParse({ ...base, visible: "${data.tier in ['pro']}" }).success).toBe(false);
+  });
+});
diff --git a/packages/spec/src/system/settings-manifest.test.ts b/packages/spec/src/system/settings-manifest.test.ts
@@ -302,6 +302,96 @@ describe('Specifier.valueDomain (#5933)', () => {
   });
 });
 
+describe('`visible` — the settings visibility grammar (#7327)', () => {
+  const withVisible = (visible: unknown) =>
+    SpecifierSchema.safeParse({ type: 'text', key: 'smtp_host', label: 'Host', visible });
+  const firstMessage = (visible: unknown): string => {
+    const result = withVisible(visible);
+    return result.success ? '' : result.error.issues[0].message;
+  };
+
+  // The shapes the ten bundled manifests are actually written in. The
+  // cross-package half — that this schema and `evaluateVisibility` accept the
+  // same set, measured against the real corpus — is pinned from the consumer
+  // side, in `service-settings/src/settings-visibility-declaration.pin.test.ts`.
+  it.each([
+    "${data.provider === 'smtp'}",
+    "${data.provider !== 'memory'}",
+    '${data.email_password_enabled !== false}',
+    '${data.mfa_required === true}',
+    '${data.lockout_threshold > 0}',
+    "${data.provider === 'resend' || data.provider === 'postmark'}",
+    "${data.embedder_provider && data.embedder_provider !== 'none'}",
+    '${!data.disabled}',
+    '${data.count <= 10 && (data.a == 1 || data.b != null)}',
+    '${data.ratio >= 0.5}',
+  ])('accepts %s', (visible) => {
+    expect(withVisible(visible).success).toBe(true);
+  });
+
+  it('accepts the unwrapped spelling and the `{ dialect, source }` envelope', () => {
+    expect(withVisible("data.provider === 'smtp'").success).toBe(true);
+    expect(withVisible({ dialect: 'cel', source: "${data.provider === 'smtp'}" }).success).toBe(true);
+    // An `ast`-only envelope is opaque at this layer — nothing to walk.
+    expect(withVisible({ dialect: 'cel', ast: { kind: 'opaque' } }).success).toBe(true);
+  });
+
+  it('normalises a bare string to the canonical envelope, unchanged by the narrowing', () => {
+    const parsed = SpecifierSchema.parse({
+      type: 'text', key: 'smtp_host', label: 'Host', visible: "${data.provider === 'smtp'}",
+    });
+    expect(parsed.visible).toEqual({ dialect: 'cel', source: "${data.provider === 'smtp'}" });
+  });
+
+  // The point of #7327: this slot never was CEL, and an author who wrote CEL
+  // here used to be told so only by a failing tenant save (#7169 / PR #7310).
+  it.each([
+    ["${data.provider in ['smtp', 'resend']}", 'unsupported identifier "in"'],
+    ['${size(data.recipients) > 0}', 'unsupported identifier "size"'],
+    ["${current_user.role === 'admin'}", 'unsupported identifier "current_user.role"'],
+    ['${data.nested.field === 1}', 'unsupported reference "data.nested.field"'],
+    ["${data.provider.startsWith('s')}", 'unsupported reference "data.provider.startsWith"'],
+  ])('refuses CEL %s', (visible, detail) => {
+    expect(withVisible(visible).success).toBe(false);
+    expect(firstMessage(visible)).toContain(detail);
+  });
+
+  it.each([
+    ['${data.provider ===}', 'unexpected end of expression'],
+    ["${data.a === 'x'} && ${data.b === 'y'}", 'unexpected character "}"'],
+    ['${-5 > data.x}', 'unexpected character "-"'],
+    ["${(data.a === 'x'}", 'missing closing parenthesis'],
+    ["${data.a 'x'}", 'trailing tokens'],
+  ])('refuses malformed %s', (visible, detail) => {
+    expect(withVisible(visible).success).toBe(false);
+    expect(firstMessage(visible)).toContain(detail);
+  });
+
+  it('prescribes the grammar rather than only naming the violation', () => {
+    // Every part an author needs to rewrite the predicate without leaving the
+    // error: the offending source, the reason, and the grammar itself.
+    const message = firstMessage("${data.provider in ['smtp']}");
+    expect(message).toContain("data.provider in ['smtp']");
+    expect(message).toContain('unsupported identifier "in"');
+    expect(message).toContain('not CEL');
+    expect(message).toContain('one-level member access');
+    expect(message).toContain('`===` `!==` `==` `!=` `>=` `<=` `>` `<`');
+    expect(message).toContain("`${data.x === 'a' || data.x === 'b'}`");
+  });
+
+  it('applies to the manifest-level slot too', () => {
+    const base = {
+      namespace: 'demo',
+      label: 'Demo',
+      specifiers: [{ type: 'text', key: 'smtp_host', label: 'Host' }],
+    };
+    expect(SettingsManifestSchema.safeParse({ ...base, visible: "${data.tier === 'pro'}" }).success).toBe(true);
+    const refused = SettingsManifestSchema.safeParse({ ...base, visible: "${data.tier in ['pro']}" });
+    expect(refused.success).toBe(false);
+    expect(refused.success ? [] : refused.error.issues[0].path).toEqual(['visible']);
+  });
+});
+
 describe('valueDomain membership definitions — the measurements service-settings must implement', () => {
   // These pin the TSDoc on `SpecifierValueDomainSchema`. `packages/spec` does
   // not enforce a domain (Prime Directive #2) — but the two halves have to agree
diff --git a/packages/spec/src/system/settings-manifest.zod.ts b/packages/spec/src/system/settings-manifest.zod.ts
