fix(app-shell): Studio CEL editors agree with the server verdict (builder null guard, stale RLS verdict)

.changeset/27452-cel-field-verdict-on-judged-text.md

@@ -0,0 +1,12 @@
+---
+'@object-ui/app-shell': patch
+---
+
+fix(app-shell): a CEL field shows the server's verdict only on the text it judged
+
+Clause-②: no
+Metadata-protocol: none
+
+In Studio's permission-set editor, an RLS `using` or `check` clause kept showing the previous draft's server error under text the author had since changed. An edit the CEL syntax check holds is never saved, so the draft is not read again and the server's verdict on the old text stayed on screen beside the new error: two different errors at once, one of them about text no longer in the field.
+
+The clause now shows the server's errors on the same terms on which they hold the save: only while it still shows the text the server judged. Editing the text away hides them, and the next save's re-read brings the new verdict. Typing the judged text back shows the verdict again, because it describes that text.

.changeset/27513-condition-builder-null-guard.md

@@ -0,0 +1,12 @@
+---
+'@object-ui/app-shell': patch
+---
+
+fix(app-shell): the condition builder writes a null-safe comparison on a field
+
+Clause-②: no
+Metadata-protocol: none
+
+In Studio's no-code condition builder, a row such as "`record.estimated_hours` greater than `40`" compiled to `record.estimated_hours > 40`. On a field that may be empty, CEL has no `>` for a missing value, so the server refused the rule at publish ("Guard it with '!= null'") and held back every other draft in the package, while the builder had said "Valid CEL".
+
+A greater than, less than, at least or at most row on a field of the object now compiles behind a null test, `record.estimated_hours != null && record.estimated_hours > 40`: the shape the "Number can't be negative" preset already writes and the one the server's message prescribes. It is accepted at publish and evaluates to false on a record with no value. Equals and not equals are unchanged, as are rows on a context subject. A stored condition is never rewritten on open: a comparison saved without the guard reopens as the same row and keeps its text until the author edits that row.

ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx

@@ -54,7 +54,7 @@ import {
   type CelSuggestionKind,
   type CelValueType,
 } from './celAuthoring.js';
-import { useServerErrorsAt, useServerHoldAt, type ServerDiagnosticEntry } from './draftVerdict.js';
+import { useServerHoldAt, type ServerDiagnosticEntry } from './draftVerdict.js';
 
 const LINT_DEBOUNCE_MS = 250;
 
@@ -133,6 +133,12 @@ export interface CelPredicateFieldProps {
    * on what publish will refuse exactly as on a parse fault. Once the author
    * edits the text away the verdict no longer describes it and holds nothing:
    * the save goes out, and its re-read brings the next verdict.
+   *
+   * objectstack#27452 — the message is shown on the same terms as the hold:
+   * only while the text on screen is the text the verdict judged. A save the
+   * lint holds never re-reads, so a verdict on the previous text stays served;
+   * it is not this text's, and is not shown under it. A page that provides no
+   * judged body shows no server verdict here.
    */
   verdictPath?: string;
   /**
@@ -404,12 +410,12 @@ export function CelPredicateField({
   const errors = issues.filter((i) => i.severity === 'error');
   const warnings = issues.filter((i) => i.severity === 'warning');
   // objectstack#26858 — the server's errors at this editor's path, each one
-  // the lint does not already say in the same words.
-  const atPath = useServerErrorsAt(verdictPath);
-  const serverErrors = atPath.filter((e) => !issues.some((iss) => issueText(iss, t) === e.message));
+  // the lint does not already say in the same words. objectstack#27452 — the
+  // held ones: a verdict on text no longer on screen describes nothing here.
+  const serverErrors = held.filter((e) => !issues.some((iss) => issueText(iss, t) === e.message));
   // "Valid CEL" only when nothing — the lint, the host's scope verdict
   // (objectui#11789) or the server's verdict at this path — says otherwise.
-  const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue && atPath.length === 0;
+  const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue && held.length === 0;
   // Result-type affordance (role="value"): shown once the expression parses,
   // even alongside warnings — the type is what dataset measure eligibility
   // keys off, so the author should see it whenever it is known.
@@ -437,7 +443,7 @@ export function CelPredicateField({
           aria-expanded={open}
           aria-autocomplete="list"
           aria-controls={open ? listId : undefined}
-          aria-invalid={errors.length > 0 || atPath.length > 0 || undefined}
+          aria-invalid={errors.length > 0 || held.length > 0 || undefined}
           onChange={(e) => {
             onChange(e.target.value);
             refreshAc();
@@ -450,7 +456,7 @@ export function CelPredicateField({
           }}
           className={cn(
             'font-mono text-xs',
-            (errors.length > 0 || atPath.length > 0) && 'border-destructive focus-visible:ring-destructive/40',
+            (errors.length > 0 || held.length > 0) && 'border-destructive focus-visible:ring-destructive/40',
           )}
         />
         {open && suggestions.length > 0 && (

ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.serverVerdictHold-27482.test.tsx

@@ -172,3 +172,43 @@ describe('a server error at a clause path holds the autosave (objectstack#27482)
     expect(server.sent).toHaveLength(0);
   });
 });
+
+/**
+ * objectstack#27452 — the verdict on the previous text is not shown under new
+ * text. An edit the lint holds never saves, so nothing re-reads the draft and
+ * the served verdict stays the one on the stored text; the clause shows only
+ * the error that applies to what it holds.
+ */
+describe('a lint-held edit does not show the previous text\'s server verdict (objectstack#27452)', () => {
+  /** A parse fault: the client lint holds it on its own. */
+  const HELD_BY_LINT = 'f_total >';
+
+  it('the server message leaves with its text; the client error and the hold stay, no PUT', async () => {
+    const server: Server = { draft: policy(REFUSED_USING), draftReads: 0, sent: [] };
+    await openPackageDoor(server);
+    await screen.findByText(REFUSAL);
+    const reads = server.draftReads;
+
+    edit(USING_PLACEHOLDER, HELD_BY_LINT);
+
+    await waitFor(() => expect(screen.getByTestId('perm-autosave-held')).toBeInTheDocument(), AUTOSAVE);
+    await waitFor(() => expect(screen.getByPlaceholderText(USING_PLACEHOLDER)).toHaveAttribute('aria-invalid', 'true'));
+    expect(screen.queryByText(REFUSAL)).toBeNull();
+    await pause(PAST_THE_PAUSE);
+    expect(server.sent).toHaveLength(0);
+    expect(server.draftReads).toBe(reads);
+    expect(screen.queryByText(REFUSAL)).toBeNull();
+  });
+
+  it('control: typed back to the judged text, the server message is shown again', async () => {
+    const server: Server = { draft: policy(REFUSED_USING), draftReads: 0, sent: [] };
+    await openPackageDoor(server);
+    await screen.findByText(REFUSAL);
+
+    edit(USING_PLACEHOLDER, HELD_BY_LINT);
+    await waitFor(() => expect(screen.queryByText(REFUSAL)).toBeNull());
+    edit(USING_PLACEHOLDER, REFUSED_USING);
+
+    expect(await screen.findByText(REFUSAL)).toBeInTheDocument();
+  });
+});

ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx

@@ -0,0 +1,177 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * objectstack#27513 — the builder's ordering comparison on a field compiles
+ * behind a null guard.
+ *
+ * The defect, as the release walkthrough measured it: on a Validations tab,
+ * "Reject the save when… `record.estimated_hours` greater than `40`" on an
+ * optional Number field saved as `record.estimated_hours > 40`. CEL has no `>`
+ * overload for `null`, so the server refused the rule at publish and held the
+ * whole package back. The "Number can't be negative" preset already writes the
+ * guarded shape (`record.x != null && record.x < 0`); the builder now writes it
+ * too.
+ *
+ * The verdicts here are read off the CEL the builder EMITS, judged by the
+ * server's own null-guard rule (`@objectstack/lint`'s
+ * `findUnguardedNullableOperands`, the gate the publish runs) and evaluated by
+ * the engine the server evaluates a validation rule with. ⛔ Neither is called
+ * from the builder: it decides nothing about nullability and guards every
+ * ordering comparison on a catalog field.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, afterEach, beforeAll, vi } from 'vitest';
+import { render, screen, cleanup, fireEvent } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { ExpressionEngine } from '@objectstack/formula';
+import { findUnguardedNullableOperands } from '@objectstack/lint';
+
+// objectui#4697 — ConditionBuilder calls useObjectFields(objectName)
+// unconditionally even when `fields` is supplied; keep that fetch off the network.
+const state = vi.hoisted(() => ({
+  metadataClient: {
+    get: vi.fn(async (): Promise<unknown> => undefined),
+    list: vi.fn(async () => [] as unknown[]),
+    withPreviewDrafts() { return this; },
+  },
+}));
+vi.mock('../useMetadata', () => ({
+  useMetadataClient: () => state.metadataClient,
+}));
+
+import { ConditionBuilder, RECORD_CONDITION_SUBJECTS } from './ConditionBuilder';
+
+afterEach(cleanup);
+
+beforeAll(() => {
+  // Radix Select probes pointer-capture APIs the test DOM lacks.
+  for (const m of ['hasPointerCapture', 'setPointerCapture', 'releasePointerCapture'] as const) {
+    if (!Element.prototype[m]) {
+      // @ts-expect-error test shim
+      Element.prototype[m] = m === 'hasPointerCapture' ? () => false : () => {};
+    }
+  }
+});
+
+/** The card's Work Order: an optional Number and a Picklist. */
+const FIELDS = [
+  { name: 'estimated_hours', label: 'Estimated Hours', type: 'number' },
+  { name: 'status', label: 'Status', type: 'select' },
+];
+/** What the server's index calls nullable on that object: neither is required or defaulted. */
+const NULLABLE = { nullableFields: new Set(['estimated_hours', 'status']) };
+
+const GUARDED = 'record.estimated_hours != null && record.estimated_hours > 40';
+const UNGUARDED = 'record.estimated_hours > 40';
+
+/** Evaluate `source` exactly as objectql's validation-rule evaluator does. */
+function evaluate(source: string, record: Record<string, unknown>) {
+  const res = ExpressionEngine.evaluate<boolean>({ dialect: 'cel', source }, { record, previous: undefined });
+  return res.ok ? { ok: true, value: res.value } : { ok: false };
+}
+
+/** The Validations tab's mount: record scope, the server host's subjects. */
+function Harness({ initial }: { initial: string }) {
+  const [v, setV] = React.useState(initial);
+  return (
+    <div>
+      <ConditionBuilder
+        value={v}
+        onCommit={setV}
+        fields={FIELDS}
+        scope="record"
+        subjects={{ context: RECORD_CONDITION_SUBJECTS }}
+      />
+      <pre data-testid="committed">{v}</pre>
+    </div>
+  );
+}
+
+const committed = () => screen.getByTestId('committed').textContent ?? '';
+const comboboxes = (c: HTMLElement) => Array.from(c.querySelectorAll('[role="combobox"]')) as HTMLElement[];
+/** The raw editor's textarea, present only when the builder fell back to it. */
+const rawEditor = (c: HTMLElement) => c.querySelector('textarea');
+
+async function pick(trigger: HTMLElement, option: string) {
+  await userEvent.click(trigger);
+  await userEvent.click(await screen.findByRole('option', { name: option }));
+}
+
+describe('an ordering comparison on a field compiles behind a null guard (objectstack#27513)', () => {
+  it('the card flow — Add condition, the optional number, greater than, 40 — writes the guarded shape', async () => {
+    const { container } = render(<Harness initial="" />);
+    fireEvent.click(screen.getByRole('button', { name: /Add condition/ }));
+    await pick(comboboxes(container)[0], 'record.estimated_hours');
+    await pick(comboboxes(container)[1], 'greater than');
+    fireEvent.change(screen.getByPlaceholderText('value'), { target: { value: '40' } });
+
+    expect(committed()).toBe(GUARDED);
+    // The server's null-guard gate accepts it, and the engine evaluates it on
+    // a row with no hours instead of faulting.
+    expect(findUnguardedNullableOperands(committed(), NULLABLE)).toEqual([]);
+    expect(evaluate(committed(), { estimated_hours: null, status: 'new' })).toEqual({ ok: true, value: false });
+    expect(evaluate(committed(), { estimated_hours: 41, status: 'new' })).toEqual({ ok: true, value: true });
+    expect(evaluate(committed(), { estimated_hours: 40, status: 'new' })).toEqual({ ok: true, value: false });
+  });
+
+  it('INSTRUMENT CONTROL — the emission this replaces is refused by the same gate and faults in the same engine', () => {
+    expect(findUnguardedNullableOperands(UNGUARDED, NULLABLE)).toMatchObject([
+      { operand: 'record.estimated_hours', operator: '>' },
+    ]);
+    expect(evaluate(UNGUARDED, { estimated_hours: null, status: 'new' })).toEqual({ ok: false });
+  });
+
+  it('the guarded text reopens as the one row it compiles from', () => {
+    const { container } = render(<Harness initial={GUARDED} />);
+    expect(rawEditor(container)).toBeNull();
+    // One row: its subject and operator, and no join (two rows would read the
+    // guard as an "is not empty" row of its own).
+    expect(comboboxes(container)).toHaveLength(2);
+    expect((screen.getByPlaceholderText('value') as HTMLInputElement).value).toBe('40');
+    expect(committed()).toBe(GUARDED);
+  });
+
+  it('beside another row the guarded row is parenthesised, and the OR join round-trips', async () => {
+    const { container } = render(<Harness initial="record.status == 'new' || record.estimated_hours == 40" />);
+    await pick(comboboxes(container).at(-1)!, 'greater than');
+    const cel = committed();
+    expect(cel).toBe(`record.status == 'new' || (${GUARDED})`);
+    expect(findUnguardedNullableOperands(cel, NULLABLE)).toEqual([]);
+
+    cleanup();
+    const again = render(<Harness initial={cel} />);
+    expect(rawEditor(again.container)).toBeNull();
+    expect(screen.getAllByPlaceholderText('value')).toHaveLength(2);
+  });
+
+  it('a stored comparison written without the guard keeps its spelling until the author edits that row', () => {
+    const { container } = render(<Harness initial={`${UNGUARDED} && record.status == 'new'`} />);
+    // Adopted as rows, not rewritten: the builder writes nothing on mount.
+    expect(rawEditor(container)).toBeNull();
+    expect(committed()).toBe(`${UNGUARDED} && record.status == 'new'`);
+
+    // Editing the other row leaves this one's bytes alone.
+    fireEvent.change(screen.getAllByPlaceholderText('value')[1], { target: { value: 'done' } });
+    expect(committed()).toBe(`${UNGUARDED} && record.status == 'done'`);
+
+    // Editing this row writes it the way the builder writes.
+    fireEvent.change(screen.getAllByPlaceholderText('value')[0], { target: { value: '50' } });
+    expect(committed()).toBe(
+      "(record.estimated_hours != null && record.estimated_hours > 50) && record.status == 'done'",
+    );
+  });
+
+  it('equality needs no guard: CEL compares against null without a fault', async () => {
+    const { container } = render(<Harness initial="record.estimated_hours > 40" />);
+    await pick(comboboxes(container).at(-1)!, 'equals');
+    expect(committed()).toBe('record.estimated_hours == 40');
+    expect(findUnguardedNullableOperands(committed(), NULLABLE)).toEqual([]);
+  });
+
+  it('a subject that names no catalog field is not guarded', async () => {
+    const { container } = render(<Harness initial="record.id == 5" />);
+    await pick(comboboxes(container).at(-1)!, 'greater than');
+    expect(committed()).toBe('record.id > 5');
+  });
+});