fix(app-shell): all CEL-hosting inspectors block Save on parse faults (#4527)

.changeset/inspector-family-cel-save-gate-4527.md

@@ -0,0 +1,13 @@
+---
+'@object-ui/app-shell': patch
+---
+
+All CEL-hosting inspectors block Save on parse faults — a page block or formatting rule whose condition does not parse no longer saves
+
+`CelPredicateField` has always reported its lint verdict upward through `onLintChange`, but only the RLS policy editor listened. #4306 fixed the field inspector and shipped the channel the rest of the console needed — `MetadataInspectorProps.onBlockingIssuesChange`, named for blocking issues rather than for CEL precisely so the remaining sites could be wired against it. This wires the two shared editors that discarded the verdict: `ConditionBuilder` (the no-code predicate builder's raw-expression mode) and `ConditionalFormattingEditor` (one condition per formatting rule). Each gains an optional callback surfacing its blocking-error count, and the inspectors above them aggregate and report through the contract, so the host that owns Save refuses to write.
+
+Both counts are DERIVED from what they describe rather than repaired by reset effects, because the editors can vanish while their last verdict was "1 error" and nothing would ever retract it. `ConditionBuilder`'s CEL editor exists only in raw mode — an externally-changed value that round-trips as a simple predicate flips the builder back to rows in the same commit, unmounting the editor and cancelling its pending lint — so the count is read as 0 whenever the raw editor is not mounted. `ConditionalFormattingEditor` keys a per-rule map and counts only rules that still exist: a shared counter would let whichever rule linted last overwrite the others, so fixing one of two broken rules would hand back a writable Save while the other was still malformed, and a deleted rule's remembered error would wedge Save shut with no editor on screen to fix it. Only `severity: 'error'` counts; advisory warnings never block.
+
+Wired this way, `PageBlockInspector` gates the metadata editor's Save on a page block's `visibleWhen`, and `ViewVariantInspector` gates it on a view's `conditionalFormatting` rules (forwarded through `ViewInspector`, the scoped router — without that hop the channel stops one component short of the editor and the wiring is inert).
+
+Three of the sites named in the original report are NOT wired here, because the channel does not reach them: `HookDefaultInspector`, `ActionDefaultInspector` and the view's home panel are `MetadataDefaultInspectorProps` components, whose contract has no blocking-issues member, and `widgets.tsx`'s condition widget is a `SchemaForm` widget rather than an inspector at all. Wiring those needs a second contract decision and edits to the hosts, so they are left for a follow-up rather than guessed at.

packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.celGate.test.tsx

@@ -0,0 +1,172 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * `ConditionalFormattingEditor` must REPORT its CEL verdicts upward, so the
+ * inspector above it can aggregate and the host that owns Save can refuse to
+ * publish a parse fault — objectui#4527, the same ungated-Save family as #4306.
+ *
+ * This editor mounts one `CelPredicateField` PER RULE, and passed no
+ * `onLintChange` to any of them: a formatting condition that does not parse
+ * rendered its inline error and Save stayed writable. `ViewVariantInspector`
+ * reaches this file.
+ *
+ * ## The two cases that decide the implementation
+ *
+ *  - **Per-rule map, not a running total.** Rules lint independently and
+ *    asynchronously, so one shared counter lets whichever reported last
+ *    overwrite the others: fixing one of two broken rules would hand back a
+ *    writable Save while the other was still malformed. A shared counter
+ *    passes every single-rule case and fails only this one.
+ *  - **Prune by derivation.** Deleting a rule unmounts its editor, which can
+ *    never report `0` afterwards, so a remembered count would hold Save shut
+ *    with no editor on screen to fix it. The total counts only rules that
+ *    still exist (`i < drafts.length`) rather than being repaired by a reset
+ *    effect (#4527 ruling item 2, mirroring #4306).
+ *
+ * The engine is stubbed deterministically — the live lint is
+ * `CelPredicateField.test.tsx`'s job; this suite tests WIRING.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import * as React from 'react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
+
+import {
+  ConditionalFormattingEditor,
+  type ConditionalFormattingRuleDraft,
+} from './ConditionalFormattingEditor';
+import { __setCelFormulaLoader } from './celAuthoring';
+
+afterEach(() => {
+  cleanup();
+  __setCelFormulaLoader(undefined);
+});
+
+const DANGLING = /[*+\-/&|=<>]\s*$/;
+
+function stubEngine() {
+  __setCelFormulaLoader(() =>
+    Promise.resolve({
+      validateExpression: (_role: string, input: unknown) => {
+        const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? '');
+        return DANGLING.test(src)
+          ? { ok: false, errors: [{ message: 'Parse error: expression ends after an operator' }], warnings: [] }
+          : { ok: true, errors: [], warnings: [] };
+      },
+      introspectScope: () => ({ fields: ['status', 'amount'], roots: ['record'], functions: ['has'] }),
+      inferExpressionType: () => 'boolean' as const,
+    }),
+  );
+}
+
+const t = (k: string) => k;
+
+/** Controlled harness — the editor is controlled, so edits must round-trip. */
+function Harness({
+  initial,
+  report,
+}: {
+  initial: ConditionalFormattingRuleDraft[];
+  report: (count: number) => void;
+}) {
+  const [rules, setRules] = React.useState<ConditionalFormattingRuleDraft[]>(initial);
+  return (
+    <ConditionalFormattingEditor
+      rules={rules}
+      onChange={setRules}
+      objectName="invoice"
+      fieldNames={['status', 'amount']}
+      t={t}
+      onBlockingIssuesChange={report}
+    />
+  );
+}
+
+function renderEditor(initial: ConditionalFormattingRuleDraft[]) {
+  const report = vi.fn();
+  render(<Harness initial={initial} report={report} />);
+  const current = () => report.mock.calls.at(-1)?.[0] as number | undefined;
+  return { report, current };
+}
+
+/** The rule `i`'s CEL box (`CelPredicateField` renders a combobox TEXTAREA). */
+const ruleBox = (i: number) =>
+  screen
+    .getByTestId(`cf-rule-${i}`)
+    .querySelector('[role="combobox"]') as HTMLTextAreaElement;
+
+describe('ConditionalFormattingEditor — blocking CEL issues are reported upward (#4527)', () => {
+  it('counts a formatting condition that does not parse', async () => {
+    stubEngine();
+    const { current } = renderEditor([{ condition: "record.status == 'a'", style: {} }]);
+    fireEvent.change(ruleBox(0), { target: { value: 'record.status ==' } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+  });
+
+  it('reports a clean rule as zero, so a valid condition never blocks Save', async () => {
+    stubEngine();
+    const { current } = renderEditor([{ condition: '', style: {} }]);
+    fireEvent.change(ruleBox(0), { target: { value: "record.status == 'overdue'" } });
+    await waitFor(() => expect(current()).toBe(0), { timeout: 3000 });
+  });
+
+  /**
+   * DECISIVE — the per-rule map. One shared counter passes both cases above
+   * and fails here: clearing ONE of two faulty rules would drop the total to 0
+   * and hand back a Save button that still publishes the other fault.
+   */
+  it('keeps each rule independent — fixing one leaves the other counted', async () => {
+    stubEngine();
+    const { current } = renderEditor([
+      { condition: '', style: {} },
+      { condition: '', style: {} },
+    ]);
+
+    fireEvent.change(ruleBox(0), { target: { value: 'record.status ==' } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+
+    fireEvent.change(ruleBox(1), { target: { value: 'record.amount >' } });
+    await waitFor(() => expect(current()).toBe(2), { timeout: 3000 });
+
+    // Fix only the first — the second must still hold Save closed.
+    fireEvent.change(ruleBox(0), { target: { value: "record.status == 'overdue'" } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+  });
+
+  /**
+   * DECISIVE — the prune. Deleting the faulty rule unmounts its editor, so
+   * nothing will ever report `0` for it; a remembered count wedges Save shut.
+   */
+  it('drops the count when the faulty rule is deleted, so Save cannot wedge shut', async () => {
+    stubEngine();
+    const { current } = renderEditor([
+      { condition: "record.status == 'ok'", style: {} },
+      { condition: '', style: {} },
+    ]);
+
+    fireEvent.change(ruleBox(1), { target: { value: 'record.amount >' } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+
+    fireEvent.click(screen.getByTestId('cf-remove-1'));
+    await waitFor(() => expect(screen.queryByTestId('cf-rule-1')).toBeNull(), { timeout: 3000 });
+    await waitFor(() => expect(current()).toBe(0), { timeout: 3000 });
+  });
+
+  it('stays mountable with no reporter attached — the prop is optional', async () => {
+    stubEngine();
+    render(
+      <ConditionalFormattingEditor
+        rules={[{ condition: 'record.status ==', style: {} }]}
+        onChange={() => {}}
+        objectName="invoice"
+        fieldNames={['status']}
+        t={t}
+      />,
+    );
+    await waitFor(
+      () => expect(screen.getByText(/Parse error: expression ends after an operator/)).toBeInTheDocument(),
+      { timeout: 3000 },
+    );
+  });
+});

packages/app-shell/src/views/metadata-admin/ConditionalFormattingEditor.tsx

@@ -28,6 +28,7 @@ import * as React from 'react';
 import { Button, Input, cn } from '@object-ui/components';
 import { Plus, Trash2, ChevronUp, ChevronDown } from 'lucide-react';
 import { CelPredicateField } from './CelPredicateField';
+import type { CelLintIssue } from './celAuthoring';
 
 /**
  * Scope roots bound at RUNTIME for a row predicate, advertised to autocomplete.
@@ -132,6 +133,17 @@ export interface ConditionalFormattingEditorProps {
   disabled?: boolean;
   /** i18n resolver (`(key) => string`). */
   t: (key: string) => string;
+  /**
+   * Report how many BLOCKING author-time issues this editor is showing — rule
+   * conditions that do not parse (objectui#4527). The inspector above
+   * aggregates and hands the total to the host through
+   * `MetadataInspectorProps.onBlockingIssuesChange`, because the Save button
+   * belongs to the host, not here.
+   *
+   * Optional: mount sites with no Save to gate simply omit it. Fires whenever
+   * the aggregate changes, `0` when every rule is clean.
+   */
+  onBlockingIssuesChange?: (count: number) => void;
 }
 
 /** A native color swatch + free-text value (hex / CSS / Tailwind), like the
@@ -178,13 +190,51 @@ export function ConditionalFormattingEditor({
   fieldNames,
   disabled,
   t,
+  onBlockingIssuesChange,
 }: ConditionalFormattingEditorProps) {
   // Normalize the persisted rules to the authoring shape once per input change.
   const drafts = React.useMemo<ConditionalFormattingRuleDraft[]>(
     () => (Array.isArray(rules) ? rules.map(normalizeRule) : []),
     [rules],
   );
 
+  /* ─── Blocking CEL verdicts → the inspector's aggregate (objectui#4527) ───
+   *
+   * Errors are counted PER RULE rather than into one running total: each rule
+   * mounts its own `CelPredicateField`, and they lint independently and
+   * asynchronously, so a shared counter would let whichever reported last
+   * overwrite the others — fixing one of two broken rules would hand back a
+   * writable Save while the other was still malformed.
+   *
+   * The total is DERIVED against the rule list rather than repaired by a reset
+   * effect: a deleted rule's editor is gone and can never report `0` for
+   * itself, so counting a verdict it left behind would wedge Save shut with no
+   * editor on screen to fix it. Indices at or past the current length are
+   * therefore simply not counted. */
+  const [celErrors, setCelErrors] = React.useState<Record<number, number>>({});
+  const reportCel = React.useCallback((index: number, issues: CelLintIssue[]) => {
+    // Only `error` blocks Save; `warning` is advisory, matching #4306.
+    const errs = issues.filter((i) => i.severity === 'error').length;
+    setCelErrors((prev) => (prev[index] === errs ? prev : { ...prev, [index]: errs }));
+  }, []);
+  const ruleCount = drafts.length;
+  const blockingIssues = React.useMemo(() => {
+    let total = 0;
+    for (const [index, count] of Object.entries(celErrors)) {
+      if (Number(index) >= ruleCount) continue; // pruned: the rule is gone
+      total += count;
+    }
+    return total;
+  }, [celErrors, ruleCount]);
+  // Held in a ref so an unmemoized parent callback cannot re-fire the effect.
+  const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange);
+  React.useEffect(() => {
+    onBlockingIssuesChangeRef.current = onBlockingIssuesChange;
+  });
+  React.useEffect(() => {
+    onBlockingIssuesChangeRef.current?.(blockingIssues);
+  }, [blockingIssues]);
+
   const commit = (next: ConditionalFormattingRuleDraft[]) => onChange(next);
 
   const setRule = (i: number, patch: Partial<ConditionalFormattingRuleDraft>) => {
@@ -283,6 +333,7 @@ export function ConditionalFormattingEditor({
             scope="flattened"
             roots={ROW_PREDICATE_ROOTS}
             onChange={(v) => setRule(i, { condition: v })}
+            onLintChange={(issues) => reportCel(i, issues)}
             t={t}
             id={`cf-condition-${i}`}
           />