From fcfc7b7624b653d9ea22b84d6c0fad84b5513904 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:38:45 +0000 Subject: [PATCH 1/3] fix(app-shell): a CEL field shows the server verdict only on the text it judged The RLS clause editor kept showing the previous draft's server error under newly edited text: a save the client lint holds never re-reads the draft, so the served verdict on the stored text stayed, and the field read it by path alone. The field now shows the server's errors on the same terms the save hold already used (useServerHoldAt): only while the text on screen is the value the served body holds at that path. Editing the text away hides it; the next save's re-read brings the new one. Claude-Session: https://claude.ai/code/session_01Utp6CZVdZdKe4J1nCNL23t Co-authored-by: Claude --- .../27452-cel-field-verdict-on-judged-text.md | 12 ++++++ .../metadata-admin/CelPredicateField.tsx | 20 ++++++---- ...rixEditor.serverVerdictHold-27482.test.tsx | 40 +++++++++++++++++++ .../serverVerdictAtPath-26858.test.tsx | 14 ++++++- 4 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 .changeset/27452-cel-field-verdict-on-judged-text.md diff --git a/.changeset/27452-cel-field-verdict-on-judged-text.md b/.changeset/27452-cel-field-verdict-on-judged-text.md new file mode 100644 index 00000000000..61c9c20bd5d --- /dev/null +++ b/.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. diff --git a/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx b/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx index bab4d9cae39..9071579c459 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/CelPredicateField.tsx +++ b/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 && ( diff --git a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.serverVerdictHold-27482.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.serverVerdictHold-27482.test.tsx index bc4c66d3ee3..a02ad4954b8 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/PermissionMatrixEditor.serverVerdictHold-27482.test.tsx +++ b/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(); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/serverVerdictAtPath-26858.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/serverVerdictAtPath-26858.test.tsx index 60322f8cc75..c497a2d8131 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/serverVerdictAtPath-26858.test.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/serverVerdictAtPath-26858.test.tsx @@ -30,7 +30,7 @@ vi.mock('./previews/useObjectFields', () => ({ import { PermissionAdvancedFacets } from './PermissionAdvancedFacets'; import { FlowEdgeInspector } from './inspectors/FlowEdgeInspector'; -import { DraftVerdictContext, serverErrorsAt, type ServerDiagnostics } from './draftVerdict'; +import { DraftVerdictBodyContext, DraftVerdictContext, serverErrorsAt, type ServerDiagnostics } from './draftVerdict'; import type { MetadataSelection } from './preview-registry'; afterEach(cleanup); @@ -69,7 +69,17 @@ async function renderRls(served: ServerDiagnostics | undefined, policies = 1) { } as unknown as React.ComponentProps)} /> ); - render(served === undefined ? facets : {facets}); + // The page provides the body its verdict judged, the draft on screen + // (objectstack#27452: the field shows a verdict only on the text it judged). + render( + served === undefined ? ( + facets + ) : ( + + {facets} + + ), + ); await user.click(screen.getByText('perm.rls.title')); } From 436625b9339150884168ada7eb6d7be7793306d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:41:29 +0000 Subject: [PATCH 2/3] fix(app-shell): the condition builder guards an ordering comparison with != null A no-code row "optional number greater than 40" compiled to `record.estimated_hours > 40`; CEL has no `>` overload for null, so the server refused the validation rule at publish and held the package back while the builder said "Valid CEL". An ordering row (> < >= <=) whose subject names a catalog field now compiles to `S != null && S op V`, the shape the "can't be negative" preset writes, and reads back as that one row. The builder decides nothing about nullability: the guard is well typed on every field. A stored comparison without the guard keeps its spelling until the author edits that row, so nothing is rewritten on open. Claude-Session: https://claude.ai/code/session_01Utp6CZVdZdKe4J1nCNL23t Co-authored-by: Claude --- .../27513-condition-builder-null-guard.md | 12 ++ .../ConditionBuilder.nullGuard-27513.test.tsx | 175 ++++++++++++++++++ .../inspectors/ConditionBuilder.tsx | 72 ++++++- 3 files changed, 253 insertions(+), 6 deletions(-) create mode 100644 .changeset/27513-condition-builder-null-guard.md create mode 100644 ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx diff --git a/.changeset/27513-condition-builder-null-guard.md b/.changeset/27513-condition-builder-null-guard.md new file mode 100644 index 00000000000..56b6a062e3e --- /dev/null +++ b/.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. diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx new file mode 100644 index 00000000000..830a98a475b --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx @@ -0,0 +1,175 @@ +// 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 => 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) { + const res = ExpressionEngine.evaluate({ 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 ( +
+ + 
{v}
+
+ ); +} + +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(); + 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(); + expect(rawEditor(container)).toBeNull(); + expect((screen.getByPlaceholderText('value') as HTMLInputElement).value).toBe('40'); + expect(screen.getAllByPlaceholderText('value')).toHaveLength(1); + expect(committed()).toBe(GUARDED); + }); + + it('beside another row the guarded row is parenthesised, and the OR join round-trips', async () => { + const { container } = render(); + 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(); + 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(); + // 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(); + 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(); + await pick(comboboxes(container).at(-1)!, 'greater than'); + expect(committed()).toBe('record.id > 5'); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx index 6121d1e240a..f2683be26e0 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx @@ -40,13 +40,28 @@ type Op = '==' | '!=' | '>' | '<' | '>=' | '<=' | 'truthy' | 'falsy'; * (`88b15fddc`) — see {@link fmtValue}. */ type Quote = '"' | "'"; -interface Row { subject: string; op: Op; value: string; quote?: Quote } +/** + * One row of the builder. `guard` is set on an ordering comparison that + * compiles behind a null test (`S != null && S > V`, see {@link compileRow}): + * on every row the author builds or edits on a catalog field, and on a stored + * row written in that shape. A stored comparison written without one keeps + * its spelling until the author edits that row (the {@link Quote} rule). + */ +interface Row { subject: string; op: Op; value: string; quote?: Quote; guard?: boolean } type OpChoice = { value: Op; labelKey: string } | { value: Op; symbol: string }; /** The operators that compare with a value: a catalogue key for the worded * ones, a bare symbol for `≥` / `≤`, which read the same in every locale * (objectui#10586). */ +/** + * The comparisons CEL has no `null` overload for (objectstack#27513). The + * server refuses one whose operand may be null unless an explicit `!= null` + * test guards it (`@objectstack/lint`'s null-guard gate, which `has()` does not + * satisfy). `==` / `!=` evaluate cleanly against `null` and need none. + */ +const ORDERING_OPS: ReadonlySet = new Set(['>', '<', '>=', '<=']); + const COMPARE_OPS: ReadonlyArray = [ { value: '==', labelKey: 'engine.inspector.condition.op.equals' }, { value: '!=', labelKey: 'engine.inspector.condition.op.notEquals' }, @@ -634,7 +649,25 @@ function compileRow(r: Row, valueOf: SubjectValueOf): { cel: string; compound: b const clauses = emptyCheckClauses(r.subject, declared, empty); return { cel: clauses.join(empty ? ' || ' : ' && '), compound: clauses.length > 1 }; } - return { cel: `${r.subject} ${r.op} ${fmtValue(r.value, r.quote)}`, compound: false }; + const cmp = `${r.subject} ${r.op} ${fmtValue(r.value, r.quote)}`; + // objectstack#27513 — an ordering comparison on a field faults on a `null` + // value and the server refuses it at publish; behind `!= null` it is well + // typed for every value. The same guard the "can't be negative" preset + // writes, and the one the server's own message prescribes. + if (r.guard && ORDERING_OPS.has(r.op)) return { cel: `${r.subject} != null && ${cmp}`, compound: true }; + return { cel: cmp, compound: false }; +} + +/** + * Whether the author's row compiles behind the null guard (objectstack#27513): + * an ordering comparison whose subject names a catalog field. ⛔ Not whether + * the field may be null — that is the server's verdict to give, and the guard + * is well typed on a field that can never be null too. A context subject + * (`user.id`) or a flow variable is not a catalog field and is left as it is. + */ +function withGuard(r: Row, fieldSubjects: ReadonlySet): Row { + const guard = ORDERING_OPS.has(r.op) && fieldSubjects.has(r.subject); + return r.guard === guard ? r : { ...r, guard }; } /** Compile rows → CEL. Rows without a subject are skipped (in-progress). */ @@ -718,10 +751,23 @@ function parseEmptyCheck(text: string, valueOf: SubjectValueOf): Row | null { return compileRow(row, valueOf).cel === text ? row : null; } +/** + * Read `text` back as a guarded ordering comparison row (`S != null && S > V`), + * or `null` (objectstack#27513). Accepts exactly the text {@link compileRow} + * emits for such a row: one subject on both sides, the guard first. + */ +function parseGuardedCompare(text: string): Row | null { + const m = /^([a-zA-Z_][\w.]*)\s*!=\s*null\s*&&\s*([a-zA-Z_][\w.]*)\s*(>=|<=|>|<)\s*(.+)$/.exec(text); + if (!m || m[1] !== m[2]) return null; + const v = unfmtValue(m[4]); + const row: Row = { subject: m[1], op: m[3] as Op, value: v.value, quote: v.quote, guard: true }; + return compileRow(row, NOTHING_DECLARED).cel === text ? row : null; +} + /** Read one AND/OR term back as a row, or `null` when it is not a row's shape. */ function parseTerm(p: string, valueOf: SubjectValueOf): Row | null { const group = enclosedGroup(p); - if (group !== null) return parseEmptyCheck(group, valueOf); + if (group !== null) return parseEmptyCheck(group, valueOf) ?? parseGuardedCompare(group); const check = parseEmptyCheck(p, valueOf); if (check) return check; const cmp = /^([a-zA-Z_][\w.]*)\s*(==|!=|>=|<=|>|<)\s*(.+)$/.exec(p); @@ -745,7 +791,7 @@ function parse( if (!s) return { rows: [], join: '&&' }; // A lone row whose check is several clauses compiles without parentheses, // so the whole text is tried as that one row before it is split. - const whole = parseEmptyCheck(s, valueOf); + const whole = parseEmptyCheck(s, valueOf) ?? parseGuardedCompare(s); if (whole) return { rows: [whole], join: '&&' }; const split = splitTopLevel(s); if (!split) return null; @@ -911,6 +957,16 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f [fields, fieldPrefix, includePrevious], ); const valueOf: SubjectValueOf = (subject) => subjectValues.get(subject); + /** Every subject spelling that names a catalog field, hidden and untyped + * ones included — what an ordering row is guarded on (objectstack#27513). */ + const fieldSubjects = React.useMemo(() => { + const out = new Set(); + for (const f of fields) { + out.add(`${fieldPrefix}${f.name}`); + if (includePrevious) out.add(`previous.${f.name}`); + } + return out; + }, [fields, fieldPrefix, includePrevious]); const declaredKey = [...subjectValues].map(([s, v]) => `${s}=${v}`).sort().join('|'); // Held in a ref so the effects below read this render's catalog without // listing a function that is new every render. @@ -967,9 +1023,13 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f }; const update = (nextRows: Row[], nextJoin: '&&' | '||' = join) => { editedSinceRead.current = true; - setRowsState(nextRows); + // A row this edit built or changed is a new object; it compiles as the + // builder writes it (objectstack#27513). The rows it left alone keep + // their spelling. + const settled = nextRows.map((r) => (rows.includes(r) ? r : withGuard(r, fieldSubjects))); + setRowsState(settled); setJoin(nextJoin); - emit(nextRows, nextJoin); + emit(settled, nextJoin); }; /* ─── Blocking CEL verdicts → the inspector's aggregate (objectui#4527) ─── From 232f261a2b43ec5cc7cd81667849860b74df1763 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 15:48:57 +0000 Subject: [PATCH 3/3] test(app-shell): pin that the guarded comparison reopens as one row Claude-Session: https://claude.ai/code/session_01Utp6CZVdZdKe4J1nCNL23t Co-authored-by: Claude --- .../inspectors/ConditionBuilder.nullGuard-27513.test.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx index 830a98a475b..8a0b9acf8b7 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.nullGuard-27513.test.tsx @@ -125,8 +125,10 @@ describe('an ordering comparison on a field compiles behind a null guard (object it('the guarded text reopens as the one row it compiles from', () => { const { container } = render(); 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(screen.getAllByPlaceholderText('value')).toHaveLength(1); expect(committed()).toBe(GUARDED); });