fix(spec): refuse requiresFeature composed with a blank-source CEL visible (#17631)

.changeset/17631-requires-feature-blank-source.md

@@ -0,0 +1,23 @@
+---
+'@objectstack/spec': patch
+---
+
+fix(spec): `requiresFeature` refuses a blank-`source` CEL `visible` instead of composing a predicate that can never parse (#17631)
+
+Clause-②: no
+
+`lowerRequiresFeature` lowers the `requiresFeature: '<flag>'` sugar into the canonical `visible` CEL predicate, and its own docblock states the ADR-0078 rule it enforces: a composition that could never take effect is a loud parse error, not a silent one. The guard that enforced it tested the TYPE of `source` (`typeof existing.source !== 'string'`), so a whitespace-only `source` — legal on `ExpressionSchema`, which is the persistence contract and whose `min(1)` whitespace clears — passed it and the gate was composed AROUND a blank operand:
+
+```
+visible: { dialect: 'cel', source: '   ' } + requiresFeature: 'organization'
+  →  { dialect: 'cel', source: '(   ) && features.organization != false' }
+```
+
+That predicate parses on no scope at all (`celEngine.evaluate` answers `kind: parse`, `Unexpected token: RPAREN`), so at render the gate faults instead of gating: fail-soft surfaces show the element regardless of the flag, fail-closed surfaces hide it regardless of the flag. Either way the flag decides nothing — the parses-clean-changes-nothing arrival the guard exists to reject, produced by the guard's own composition step.
+
+The lowering now refuses a `source` that is blank after trimming, on the same leg as the AST-only refusal one line above, with a refusal that names the composition it would have produced and both exits (drop the blank `visible` and the sugar emits the gate alone; or write the predicate the gate should compose with). The notion of blank is `source.trim()` — the one the engine's own helpers apply — so a `source` that is merely padded around real text still composes verbatim.
+
+- **Refused at the producer, not tolerated at a consumer.** No renderer gains a fallback for the unparseable predicate; the lowering stops emitting it.
+- **Both slots that compose the sugar inherit it** — `ActionSchema.visible` and `ActionParamSchema.visible` — because the rule lives in the shared lowering rather than in either slot's declaration.
+- **`ExpressionSchema` / `ExpressionInputSchema` are NOT narrowed.** They remain the persistence contract, and a blank-`source` `visible` with no `requiresFeature` beside it still parses exactly as before. What is refused is the COMPOSITION, which is the thing that could never work.
+- **Nothing that functioned stops functioning.** The only authoring this refuses is one whose output faulted at CEL parse on every scope, so the migration is the refusal's own prescription and there is no working shape to port.

packages/spec/src/kernel/public-auth-features.test.ts

@@ -147,4 +147,36 @@ describe('lowerRequiresFeature', () => {
       expect(issues[0]).toMatchObject({ code: 'custom', path: ['requiresFeature'] });
     }
   });
+
+  // The same ADR-0078 leg, one step further in: a blank `source` is a STRING,
+  // so it passes the type test above and used to compose into
+  // `(   ) && <gate>` — a predicate no CEL parse accepts on any scope, so the
+  // flag decided nothing at render. Same shape as the pin above: kind and
+  // subject, never the wording.
+  it('rejects a CEL visible whose source is blank after trimming (ADR-0078)', () => {
+    for (const visible of [
+      { dialect: 'cel', source: '   ' },
+      { dialect: 'cel', source: '' },
+      { dialect: 'cel', source: '\n\t' },
+    ]) {
+      const { ctx, issues } = noIssues();
+      const out = lowerRequiresFeature({ requiresFeature: 'admin' as const, visible }, ctx);
+      expect(issues).toHaveLength(1);
+      expect(issues[0]).toMatchObject({ code: 'custom', path: ['requiresFeature'] });
+      // Refused, not composed — no `(   ) && …` envelope is produced.
+      expect(out.visible).toEqual(visible);
+    }
+  });
+
+  // The boundary the refusal must NOT cross: a source that is merely PADDED is
+  // authored, and still composes verbatim (the engine trims its own input).
+  it('still composes a source that has leading/trailing whitespace around real text', () => {
+    const { ctx, issues } = noIssues();
+    const out = lowerRequiresFeature(
+      { requiresFeature: 'admin' as const, visible: { dialect: 'cel', source: ' a ' } },
+      ctx,
+    );
+    expect(issues).toHaveLength(0);
+    expect(out.visible).toEqual({ dialect: 'cel', source: '( a ) && features.admin == true' });
+  });
 });

packages/spec/src/kernel/public-auth-features.ts

@@ -333,6 +333,15 @@ type WithRequiresFeature = {
  *   Drop one of the two rather than shipping a gate that reads as load-bearing.
  * - Existing `visible` that is non-CEL or AST-only → loud parse error
  *   (ADR-0078 no-silently-inert); write the combined predicate by hand.
+ * - Existing CEL `visible` whose `source` is blank after trimming → loud parse
+ *   error, for the same reason one step further in. `source` is `min(1)` on the
+ *   persistence contract and whitespace clears it, so a blank one is a *string*
+ *   and would compose: the result parenthesises nothing (`( ) && <gate>`) and
+ *   faults at CEL parse on every scope, so the gate decides nothing wherever
+ *   the consuming surface is fail-soft and hides the element regardless of the
+ *   flag wherever it is fail-closed — the inert arrival again, reached through
+ *   the one spelling that passes the type test above. The notion of blank is
+ *   `source.trim()`, the one the engine's own helpers apply.
  *
  * Designed as a zod `.transform((v, ctx) => lowerRequiresFeature(v, ctx))`
  * appended after the schema's refinements.
@@ -376,6 +385,30 @@ export function lowerRequiresFeature<T extends WithRequiresFeature>(
     });
     return rest as Omit<T, 'requiresFeature'>;
   }
+  if (existing.source.trim().length === 0) {
+    ctx.addIssue({
+      code: 'custom',
+      path: ['requiresFeature'],
+      // ⚠ Deliberately does NOT interpolate `gate`. Doing so puts
+      // `featureGatePredicate` — and through it the whole
+      // `PUBLIC_AUTH_FEATURES` registry — in a customer-facing message
+      // position, and `check:doc-authoring`'s per-module fixed point then
+      // sweeps that registry's INTERNAL `notes` / `exempt.reason` prose as
+      // customer-facing text (measured: green at the base commit, three
+      // pre-existing strings flagged with the interpolation in). The concrete
+      // gate is one `featureGatePredicate` call away for anyone who wants it.
+      message:
+        '`requiresFeature` composes only with a CEL `visible` carrying a NON-BLANK `source`; this '
+        + '`source` is blank after trimming, so composing the feature gate onto it would parenthesise '
+        + 'nothing — the predicate would read `( ) && ` followed by the gate — which no CEL parse '
+        + 'accepts on any scope. The gate would fault at evaluation instead of gating: the element is '
+        + 'shown regardless of the flag where the consuming surface is fail-soft and hidden regardless '
+        + 'of it where it is fail-closed, so the flag decides nothing — the inert arrival ADR-0078 '
+        + 'rejects. Drop the blank `visible` and `requiresFeature` emits the gate alone, or put the '
+        + 'predicate the gate should compose with in `source`.',
+    });
+    return rest as Omit<T, 'requiresFeature'>;
+  }
   return {
     ...rest,
     visible: { ...existing, source: `(${existing.source}) && ${gate}` },