fix(app-shell): the default-inspector family and its panel hosts gate Save on CEL errors (#4527)

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

@@ -0,0 +1,15 @@
+---
+'@object-ui/app-shell': patch
+---
+
+The default-inspector family and its panel hosts gate Save on CEL errors — a hook guard, an action predicate or a validation rule that does not parse no longer saves
+
+#4306 gave the SCOPED inspectors a way to say "what I am showing is not saveable" (`MetadataInspectorProps.onBlockingIssuesChange`), and #4527's first half wired the shared CEL editors to it. That left the other half of the console still publishing malformed expressions, for a structural reason rather than an oversight: there are TWO inspector registries, and only one of them had the channel. `MetadataDefaultInspectorProps` — the contract every "no selection" inspector is rendered through — had no such member, so the hook guard, an action's `visible` / `disabled` predicates and a view's conditional-formatting rules on the home panel rendered their inline parse errors while Save stayed writable, and no host could pass a callback that did not exist.
+
+`MetadataDefaultInspectorProps` now carries the same optional `onBlockingIssuesChange`. `HookDefaultInspector` reports its guard; `ActionDefaultInspector` aggregates its two predicate editors through a per-site map, because two editors lint independently and a shared counter would hand back a writable Save the moment one of two broken predicates was fixed; the view home panel already aggregated and now has a contract to report through.
+
+The hosts that own the buttons hold and expire those counts. The metadata editor gates its no-selection branch as well as its scoped one, stamping each so neither reads the other's verdict. Studio's design pillar gates its rail at last — that was an unfinished edge of #4306 rather than new ground, since the same malformed-CEL publish was reachable there with the gate inert, including for the field inspector. The Data pillar gains a second count for its panel family: the validations, actions and settings panels write through the object draft and own no Save, so their faults have to reach the pillar's button, and the count is stamped with the panel tab because only one panel is mounted at a time and a tab the author has left can never retract its verdict. The hooks panel is the one panel that writes on its own (`client.save('hook', …)`), so it gates its own per-hook Save.
+
+Every count is DERIVED from what it describes rather than repaired by a reset effect, and pruned by what still exists: a deleted validation rule or action drops out of the total immediately, so a fault can never wedge Save shut with no editor left on screen to fix it in. A faulty rule the author merely navigates away from stays counted, because it is still in the document and saving would still publish it.
+
+Also wired: the object validations panel, a sixth `ConditionBuilder` consumer that the original report did not list. Still deferred by ruling: `widgets.tsx`'s condition widget, a `SchemaForm` widget needing widget-context plumbing.

packages/app-shell/src/views/metadata-admin/ResourceEditPage.defaultGate.test.tsx

@@ -0,0 +1,153 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The metadata editor's Save must refuse a DEFAULT-inspector fault too —
+ * objectui#4527 phase 2.
+ *
+ * PR #4547 gated this host on the SCOPED inspector's verdict
+ * ({@link file://./ResourceEditPage.celGate.test.tsx}). With no selection the
+ * editor renders the registered DEFAULT inspector instead, and that branch
+ * passed no `onBlockingIssuesChange` — `MetadataDefaultInspectorProps` had no
+ * such member to pass. A view's conditional-formatting rules live on exactly
+ * that branch, so a rule whose CEL does not parse saved and published.
+ *
+ * ## Why `view`, measured rather than assumed
+ *
+ * This host only reaches a default inspector when the type ALSO has a canvas
+ * preview: the panel that renders it sits inside the `PreviewComponent` branch,
+ * and a type without one falls through to a plain `SchemaForm`. Of the types
+ * that have both, `view` is the one whose default inspector mounts CEL
+ * (`ViewDefaultInspector` -> `ViewVariantInspector` -> the formatting editor),
+ * so it is the type this gate is actually reachable through. The `hook` and
+ * `action` default inspectors are NOT reachable from this host at all — they
+ * are edited through the Studio panels and gated in their own suites.
+ *
+ * Only the canvas is stubbed, and only so that a preview exists and no
+ * selection is emitted; everything under test — the registered default
+ * inspector, the real formatting editor, this host's real hold and its real
+ * Save button — is the shipping code.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
+import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
+
+const viewDef = {
+  name: 'invoices',
+  label: 'Invoices',
+  list: {
+    type: 'grid',
+    object: 'invoice',
+    columns: ['status', 'amount'],
+    conditionalFormatting: [{ condition: '', style: {} }],
+  },
+};
+
+const mockClient = {
+  list: vi.fn(async () => []),
+  listDrafts: vi.fn(async () => []),
+  layered: vi.fn(async () => ({ effective: viewDef, code: viewDef, editable: true })),
+  getDraft: vi.fn(async () => null),
+  get: vi.fn(async () => null),
+  saveDraft: vi.fn(async () => ({})),
+};
+
+vi.mock('./useMetadata', async (importOriginal) => {
+  const mod = await importOriginal<typeof import('./useMetadata')>();
+  return {
+    ...mod,
+    useMetadataClient: () => mockClient,
+    useMetadataTypes: () => ({
+      entries: [{ type: 'view', name: 'view', label: 'View', allowOrgOverride: true }],
+    }),
+  };
+});
+
+import { MetadataResourceEditPage } from './ResourceEditPage';
+import { registerBuiltinInspectors } from './inspectors';
+import { registerMetadataPreview, getMetadataPreview } from './preview-registry';
+import { __setCelFormulaLoader } from './celAuthoring';
+
+registerBuiltinInspectors();
+
+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,
+    }),
+  );
+}
+
+/**
+ * Canvas stand-in: exists so the host takes its split-editor branch, and emits
+ * no selection, so the DEFAULT inspector is what fills the rail.
+ */
+function StubViewCanvas() {
+  return <div data-testid="stub-view-canvas" />;
+}
+
+const realViewPreview = getMetadataPreview('view');
+
+beforeEach(() => {
+  stubEngine();
+  registerMetadataPreview('view', StubViewCanvas as never);
+});
+
+afterEach(() => {
+  cleanup();
+  __setCelFormulaLoader(undefined);
+  if (realViewPreview) registerMetadataPreview('view', realViewPreview);
+});
+
+/** The Save icon button, identified by its title in either state. */
+const saveButton = () =>
+  screen.getByRole('button', { name: /Save \(⌘S\)|Fix the CEL syntax errors before saving\./ });
+
+const ruleBox = () =>
+  screen.getByTestId('cf-rule-0').querySelector('[role="combobox"]') as HTMLTextAreaElement;
+
+/** Open the view editor and hand back its first formatting rule's CEL box. */
+async function openRule() {
+  render(
+    <MemoryRouter initialEntries={['/metadata/view/invoices']}>
+      <MetadataResourceEditPage type="view" name="invoices" />
+    </MemoryRouter>,
+  );
+  await screen.findByTestId('cf-rule-0');
+  return ruleBox();
+}
+
+describe('MetadataResourceEditPage — Save is gated on the DEFAULT inspector’s CEL verdict (#4527)', () => {
+  it('refuses a formatting rule whose condition does not parse', async () => {
+    const box = await openRule();
+
+    // A valid condition first: dirties the draft (so Save is live at all) and
+    // pins the must-not-change half — a good condition never blocks.
+    fireEvent.change(box, { target: { value: "record.status == 'overdue'" } });
+    await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 });
+
+    fireEvent.change(box, { target: { value: 'record.amount >' } });
+    await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
+    expect(saveButton()).toHaveAttribute('title', 'Fix the CEL syntax errors before saving.');
+  });
+
+  it('re-enables Save once the condition parses again', async () => {
+    const box = await openRule();
+
+    fireEvent.change(box, { target: { value: 'record.amount >' } });
+    await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
+
+    fireEvent.change(box, { target: { value: "record.status == 'overdue'" } });
+    await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 });
+  });
+});

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

@@ -508,7 +508,14 @@ function MetadataResourceEditPageImpl({
   // component that has gone away cannot retract its last verdict, and a host
   // that waited for one would wedge Save shut.
   const [blockingReport, setBlockingReport] = React.useState({ key: '', count: 0 });
-  const selectionKey = selection ? `${type}:${name}:${selection.kind}:${selection.id}` : '';
+  // Covers BOTH inspector branches. With no selection the editor renders the
+  // registered DEFAULT inspector instead, and that surface hosts CEL too (a
+  // hook's guard, an action's predicates, a view's formatting rules) — so it
+  // gets its own stamp rather than sharing the scoped one (objectui#4527).
+  // Distinct keys are what stop one branch's verdict from gating the other.
+  const selectionKey = selection
+    ? `${type}:${name}:${selection.kind}:${selection.id}`
+    : `${type}:${name}:default`;
   const inspectorBlocking = blockingReport.key === selectionKey ? blockingReport.count : 0;
   React.useEffect(() => {
     if (!editing) setSelection(null);
@@ -2349,6 +2356,9 @@ function MetadataResourceEditPageImpl({
                               }))
                             }
                             onSelectionChange={setSelection}
+                            onBlockingIssuesChange={(count) =>
+                              setBlockingReport({ key: selectionKey, count })
+                            }
                             readOnly={formReadOnly}
                             locale={locale}
                             serverSchema={entry?.schema as Record<string, unknown> | undefined}

packages/app-shell/src/views/metadata-admin/default-inspector-registry.ts

@@ -33,6 +33,26 @@ export interface MetadataDefaultInspectorProps {
    * scoped inspector for that selection.
    */
   onSelectionChange?: (next: MetadataSelection | null) => void;
+  /**
+   * Report how many BLOCKING author-time issues the inspector is currently
+   * showing — e.g. a CEL expression that does not parse (objectui#4527).
+   *
+   * Symmetric with `MetadataInspectorProps.onBlockingIssuesChange` (#4306), and
+   * deliberately the SAME shape: the default (no-selection) inspectors host CEL
+   * editors too — the hook guard, an action's visible/disabled predicates, a
+   * view's conditional formatting — and the host owns Save, so only the host
+   * can refuse to write. Without this member no host could pass the callback at
+   * all, which is why the whole default-inspector family published malformed
+   * expressions while the scoped family already refused them.
+   *
+   * Fires whenever the aggregate changes, `0` when everything is clean.
+   *
+   * Optional — an inspector with nothing to block on simply never calls it.
+   * Hosts must expire their own count when the inspected item changes or the
+   * inspector unmounts rather than waiting for a final `0`, since a component
+   * that has gone away cannot report anything.
+   */
+  onBlockingIssuesChange?: (count: number) => void;
   /** Whether the host is in edit mode. False → disable inputs. */
   readOnly: boolean;
   /** Active UI locale for i18n. */

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

@@ -0,0 +1,143 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The Action inspector must REPORT its blocking CEL verdicts upward, so the
+ * host that owns Save can refuse to publish a predicate that does not parse —
+ * objectui#4527 phase 2.
+ *
+ * This inspector mounts TWO ConditionBuilders — "Visible when" (`visible`) and
+ * "Disabled when" (`disabled`) — and is a DEFAULT inspector, so before phase 2
+ * it had no channel to report through at all.
+ *
+ * ## The decisive case
+ *
+ * Two editors report independently and asynchronously, so a single shared
+ * counter lets whichever linted last overwrite the other: fixing "Visible when"
+ * while "Disabled when" is still malformed would hand back a writable Save.
+ * A shared counter passes every single-editor case below and fails only
+ * {@link https://github.com/objectstack-ai/objectui/issues/4527 the both-faulty
+ * case} — which is why the count is a per-SITE map, exactly as
+ * ObjectFieldInspector's is.
+ */
+
+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, within } from '@testing-library/react';
+
+import { ActionDefaultInspector } from './ActionDefaultInspector';
+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'], roots: ['record'], functions: ['has'] }),
+      inferExpressionType: () => 'boolean' as const,
+    }),
+  );
+}
+
+/**
+ * Stateful harness — the inspector is CONTROLLED, so committed predicates must
+ * round-trip through the draft or the next keystroke reverts the last one and
+ * the editors never lint what the author typed.
+ */
+function Harness({ report }: { report: (n: number) => void }) {
+  const [draft, setDraft] = React.useState<Record<string, unknown>>({
+    name: 'approve',
+    label: 'Approve',
+    type: 'script',
+  });
+  return (
+    <ActionDefaultInspector
+      type="action"
+      name="approve"
+      draft={draft}
+      onPatch={(patch) => setDraft((d) => ({ ...d, ...patch }))}
+      readOnly={false}
+      locale={'en-US' as never}
+      onBlockingIssuesChange={report}
+    />
+  );
+}
+
+function renderInspector() {
+  const report = vi.fn();
+  render(<Harness report={report} />);
+  const current = () => report.mock.calls.at(-1)?.[0] as number | undefined;
+  return { report, current };
+}
+
+/**
+ * A ConditionBuilder's own root, located by its label — the two builders are
+ * otherwise identical, so every interaction must be scoped to one of them.
+ */
+function builder(label: string): HTMLElement {
+  return screen.getByText(label).parentElement!.parentElement! as HTMLElement;
+}
+
+/** Switch one builder into raw CEL mode and hand back its editor. */
+function rawEditorFor(label: string): HTMLTextAreaElement {
+  const root = builder(label);
+  fireEvent.click(within(root).getByText('Expression'));
+  return within(root)
+    .getAllByRole('combobox')
+    .find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement;
+}
+
+describe('ActionDefaultInspector — blocking CEL issues reach the host (#4527)', () => {
+  it('counts a "Visible when" predicate that does not parse', async () => {
+    stubEngine();
+    const { current } = renderInspector();
+    fireEvent.change(rawEditorFor('Visible when'), { target: { value: 'record.status ==' } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+  });
+
+  it('counts a "Disabled when" predicate that does not parse', async () => {
+    stubEngine();
+    const { current } = renderInspector();
+    fireEvent.change(rawEditorFor('Disabled when'), { target: { value: 'record.amount >' } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+  });
+
+  it('reports clean predicates as zero, so valid conditions never block Save', async () => {
+    stubEngine();
+    const { current } = renderInspector();
+    fireEvent.change(rawEditorFor('Visible when'), { target: { value: "record.status == 'open'" } });
+    await waitFor(() => expect(current()).toBe(0), { timeout: 3000 });
+  });
+
+  /**
+   * DECISIVE — the per-site map. One shared counter passes every case above
+   * and fails here: fixing one editor would drop the total to 0 while the
+   * other predicate is still malformed.
+   */
+  it('keeps each editor independent — fixing one leaves the other counted', async () => {
+    stubEngine();
+    const { current } = renderInspector();
+    const visible = rawEditorFor('Visible when');
+    const disabled = rawEditorFor('Disabled when');
+
+    fireEvent.change(visible, { target: { value: 'record.status ==' } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+
+    fireEvent.change(disabled, { target: { value: 'record.amount >' } });
+    await waitFor(() => expect(current()).toBe(2), { timeout: 3000 });
+
+    fireEvent.change(visible, { target: { value: "record.status == 'open'" } });
+    await waitFor(() => expect(current()).toBe(1), { timeout: 3000 });
+  });
+});