Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/11789-cel-scope-verdict.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@object-ui/app-shell': patch
---

Studio's flow designer judges a record-triggered flow's Entry condition against the scope the engine binds, and shows one verdict per expression (objectui#11789).

- **`record` is in scope on the Start node.** The engine binds the whole `record` beside the record's flattened fields before it evaluates the start condition, so `record.status == 'done' && previous.status != 'done'` is as valid as `status == 'done' && previous.status != 'done'`. The Start node used to leave `record` out, and the editor showed "Valid CEL" together with "`record` is not a reference in scope at this step." An edge leaving the Start node reads the same scope, so its guard accepts `record` too.
- **`previous` follows the pre-image.** It is in scope on update, create-or-update and, new here, delete triggers, where the engine binds the deleted row as `previous`. It stays out on a create trigger, where the engine binds it only as `null`, so a member read of it fails when the flow runs. A schedule, manual or API flow gains neither `record` nor `previous`.
- **One verdict.** When the Entry condition names a reference that is not in scope at the node, the scope note replaces "Valid CEL" in the raw CEL editor instead of appearing below it. Other surfaces of that editor, the permission set's row-level security clauses among them, are unchanged.

**Clause-②: no.** No export, exported type or language-pack key changes. The editor's new optional input is internal to the package and is not reachable from its entry.
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* **One expression, one verdict — judged against the scope the engine binds**
* (objectui#11789).
*
* A record-triggered flow's Start node read "Valid CEL" and, directly below it,
* "`record` is not a reference in scope at this step." for the same entry
* condition. Two defects shared that one screen:
*
* 1. the scope Studio judged the Start node against had no `record`, while the
* engine binds it there: `seedRunVariables` seeds `record`, `$record`, the
* record's own fields and `previous` before the start-condition gate runs;
* 2. when a root really is out of scope, the raw CEL editor still said "Valid
* CEL" above the scope note, because its lint only knows the CEL scope roots.
* The scope verdict replaces the syntax verdict, so the panel says one thing.
*
* Measured through the WHOLE inspector, never a hand-built scope: a draft goes
* in, `useFlowScope` resolves it, and `FlowNodeConfigField` → `ConditionBuilder`
* → `CelPredicateField` render what an author sees. The per-trigger scope table
* itself is pinned in `inspectors/flow-scope.test.ts`.
*/

import type { ComponentProps } from 'react';
import { describe, it, expect, vi, afterEach, beforeAll } from 'vitest';
import { render, screen, cleanup, act, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

// Same network stubs as the sibling FlowNodeInspector suites: the engine
// config-schema hook (the inspector then uses its hardcoded field groups), the
// trigger object's field catalog, and the shared metadata client.
vi.mock('./previews/useFlowNodePalette', () => ({
useActionConfigSchemas: () => ({}),
useFlowNodePalette: () => [],
}));
const FIELDS = vi.hoisted(() => [
{ name: 'status', label: 'Status', type: 'text', hidden: false },
{ name: 'amount', label: 'Amount', type: 'number', hidden: false },
]);
vi.mock('./previews/useObjectFields', () => ({
useObjectFields: () => ({ fields: FIELDS, loading: false, error: null }),
}));
const state = vi.hoisted(() => ({
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) },
}));
vi.mock('./useMetadata', () => ({
useMetadataClient: () => state.metadataClient,
}));

/**
* The real CEL lint, observed. Every result the raw editor receives is kept, so
* a negative assertion below reads the editor AFTER its lint answered (and can
* show the lint answered "clean"), never a screen caught before the debounce.
*/
const lint = vi.hoisted(() => ({ results: [] as Array<Promise<unknown[]>> }));
vi.mock('./celAuthoring', async (importOriginal) => {
const real = await importOriginal<typeof import('./celAuthoring')>();
return {
...real,
lintCelPredicate: (...args: Parameters<typeof real.lintCelPredicate>) => {
const p = real.lintCelPredicate(...args);
lint.results.push(p);
return p;
},
};
});

import { FlowNodeInspector } from './inspectors/FlowNodeInspector';
import { CelPredicateField } from './CelPredicateField';
import { PermissionAdvancedFacets } from './PermissionAdvancedFacets';

afterEach(() => {
cleanup();
lint.results.length = 0;
});

beforeAll(() => {
for (const m of ['hasPointerCapture', 'setPointerCapture', 'releasePointerCapture'] as const) {
if (!Element.prototype[m]) {
// @ts-expect-error test shim
Element.prototype[m] = m === 'hasPointerCapture' ? () => false : () => {};
}
}
});

const VALID = 'Valid CEL';
const SCOPE_NOTE = /is not a reference in scope at this step|Not in scope:/;

function renderStartNode(config: Record<string, unknown>, variables?: unknown[]) {
const draft = {
...(variables ? { variables } : {}),
nodes: [{ id: 'start', type: 'start', label: 'When a lead changes', config }],
edges: [],
};
return render(
<FlowNodeInspector
type="flow"
name="lead_followup"
draft={draft}
selection={{ kind: 'node', id: 'start' }}
onPatch={vi.fn()}
onClearSelection={vi.fn()}
readOnly={false}
locale="en-US"
/>,
);
}

/** Switch the entry condition's row builder to its raw CEL editor — the
* editor the card was read off. */
async function openRawEditor(container: HTMLElement) {
const toggle = Array.from(container.querySelectorAll('button')).find((b) => b.textContent?.includes('Expression'));
expect(toggle, 'the entry condition opens in row mode with an Expression toggle').toBeTruthy();
await userEvent.click(toggle!);
}

/** Wait until the raw editor's lint has answered, and hand back its last answer. */
async function settledLint(): Promise<unknown[]> {
await waitFor(() => expect(lint.results.length).toBeGreaterThan(0), { timeout: 3000 });
const all = await Promise.all(lint.results);
await act(async () => {});
return all[all.length - 1];
}

const UPDATE_TRIGGER = { triggerType: 'record-after-update', objectName: 'crm_lead' };

describe('objectui#11789 a record-triggered Start node is judged against the scope the engine binds', () => {
it("`record.status == 'done' && previous.status != 'done'` on an update trigger reads only \"Valid CEL\"", async () => {
const { container } = renderStartNode({
...UPDATE_TRIGGER,
condition: "record.status == 'done' && previous.status != 'done'",
});
// Row mode first: no scope note under the rows either.
expect(screen.queryByText(SCOPE_NOTE)).toBeNull();
await openRawEditor(container);
expect(await screen.findByText(VALID, {}, { timeout: 3000 })).toBeInTheDocument();
expect(screen.queryByText(SCOPE_NOTE)).toBeNull();
});

it("control: the bare spelling `status == 'done' && previous.status != 'done'` is unchanged", async () => {
const { container } = renderStartNode({
...UPDATE_TRIGGER,
condition: "status == 'done' && previous.status != 'done'",
});
expect(screen.queryByText(SCOPE_NOTE)).toBeNull();
await openRawEditor(container);
expect(await screen.findByText(VALID, {}, { timeout: 3000 })).toBeInTheDocument();
expect(screen.queryByText(SCOPE_NOTE)).toBeNull();
});

it('a schedule-triggered Start node does not gain `record`: the scope note names it, and nothing says "Valid CEL"', () => {
// A declared variable makes the scope KNOWN at this node; with none, the
// ref check has no roots and stays silent by design ("scope unknown").
renderStartNode(
{ triggerType: 'schedule', schedule: { expression: '0 7 * * *' }, condition: "record.status == 'done'" },
[{ name: 'threshold', type: 'number' }],
);
expect(screen.getByText('`record` is not a reference in scope at this step.')).toBeInTheDocument();
expect(screen.queryByText(VALID)).toBeNull();
});
});

describe('objectui#11789 one verdict: an out-of-scope root replaces "Valid CEL"', () => {
it('a root the CEL lint accepts but the engine never binds here reads the scope note alone', async () => {
// `trigger` is a CEL scope root (the approval approver's submit-time
// snapshot), so the lint is clean; a flow's start condition binds no
// `trigger`, so the flow scope check is the verdict that holds.
const { container } = renderStartNode({
...UPDATE_TRIGGER,
condition: "status == 'done' && trigger.status != 'done'",
});
await openRawEditor(container);
const issues = await settledLint();
// The premise: the syntax verdict on its own WOULD be "Valid CEL".
expect(issues).toEqual([]);
expect(screen.getByText('`trigger` is not a reference in scope at this step.')).toBeInTheDocument();
expect(screen.queryByText(VALID)).toBeNull();
});

it('the editor itself: a host scope issue withholds "Valid CEL"; without one the clean verdict shows', async () => {
const t = (k: string) => k;
const { rerender } = render(
<CelPredicateField value="status == 'done'" onChange={() => {}} label="Entry" fieldNames={['status']} t={t} scopeIssue />,
);
expect(await settledLint()).toEqual([]);
expect(screen.queryByText('perm.cel.valid')).toBeNull();
rerender(<CelPredicateField value="status == 'done'" onChange={() => {}} label="Entry" fieldNames={['status']} t={t} />);
expect(await screen.findByText('perm.cel.valid')).toBeInTheDocument();
});

it("control: the permission matrix's RLS editor still reads \"Valid CEL\" for a clean clause", async () => {
const t = (k: string) => k;
const user = userEvent.setup();
render(
<PermissionAdvancedFacets
{...({
draft: {
rowLevelSecurity: [
{ name: 'p1', object: 'account', operation: 'all', using: 'organization_id == current_user.organization_id', check: '', enabled: true },
],
},
setDraft: () => {},
writable: true,
allSetNames: [] as string[],
loadObjectFields: async () => ['organization_id', 'owner_id'],
t,
} as unknown as ComponentProps<typeof PermissionAdvancedFacets>)}
/>,
);
await user.click(screen.getByText('perm.rls.title'));
expect(await screen.findByText('perm.cel.valid', {}, { timeout: 3000 })).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@ export interface CelPredicateFieldProps {
* on the inferred-result-type affordance.
*/
role?: 'predicate' | 'value';
/**
* Set by a host that judges this expression against a scope the lint cannot
* see, when that verdict found a problem — the flow inspector's in-scope
* references at the node, which name an out-of-scope root (objectui#11789).
* The host renders that verdict; this editor then withholds its "Valid CEL"
* affordance, so one expression reads one verdict. The lint itself, its
* findings and `onLintChange` are unaffected. Omitted on every surface that
* has no such second verdict (the permission matrix included).
*/
scopeIssue?: boolean;
/** Reports the current lint issues up so the editor can gate Save on errors. */
onLintChange?: (issues: CelLintIssue[]) => void;
/**
Expand Down Expand Up @@ -146,6 +156,7 @@ export function CelPredicateField({
roots,
slot,
role,
scopeIssue,
onLintChange,
onInferredTypeChange,
t,
Expand Down Expand Up @@ -342,7 +353,9 @@ export function CelPredicateField({

const errors = issues.filter((i) => i.severity === 'error');
const warnings = issues.filter((i) => i.severity === 'warning');
const clean = linted && !!value.trim() && issues.length === 0;
// "Valid CEL" only when nothing — the lint, or the host's scope verdict —
// says otherwise (objectui#11789).
const clean = linted && !!value.trim() && issues.length === 0 && !scopeIssue;
// 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ function initFrom(value: string): { rows: Row[]; join: '&&' | '||'; raw: boolean
return { rows: [], join: '&&', raw: !!value };
}

export function ConditionBuilder({ label, value, onCommit, objectName, fields: fieldsProp, disabled, onBlockingIssuesChange, subjects, scope, roots }: {
export function ConditionBuilder({ label, value, onCommit, objectName, fields: fieldsProp, disabled, onBlockingIssuesChange, subjects, scope, roots, scopeIssue }: {
label?: string;
value: string;
onCommit: (cel: string) => void;
Expand Down Expand Up @@ -598,6 +598,13 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f
* than a spelling.
*/
roots?: string[];
/**
* The host's scope verdict on `value` found a problem it renders itself
* (objectui#11789) — forwarded to `CelPredicateField`, which then withholds
* "Valid CEL" so the raw editor and the host's note cannot disagree. The row
* builder renders no verdict of its own, so it does not read this.
*/
scopeIssue?: boolean;
}) {
const { fields: hookFields } = useObjectFields(objectName);
const fields = fieldsProp ?? hookFields;
Expand Down Expand Up @@ -740,6 +747,7 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f
with no `scope` still forwards `undefined` here, so its offered
roots are the engine's own, unchanged. */
roots={offeredRoots}
scopeIssue={scopeIssue}
t={tLocal}
/>
{value && !parse(value) && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,11 +316,12 @@ describe('#6226 the flow entry field offers the FLATTENED trigger vocabulary', (
expect(opts).toContain('previous.amount');
});

it('does NOT offer `record.id` — the root this site does not bind', async () => {
it('does NOT offer `record.id` — one subject per value, in the bare spelling', async () => {
const opts = await subjectOptions(mountOneRow().container);
// The builder's record-scoped DEFAULT context would have put it here. The
// flow entry field declares its own (empty) context precisely so the editor
// cannot emit the one spelling its own sibling ref-check flags.
// flow entry field declares its own (empty) context: the engine binds
// `record` at this gate too (objectui#11789), but `record.FIELD` is the
// same value as the bare `FIELD` already offered, not a second subject.
expect(opts).not.toContain('record.id');
expect(opts.filter((o) => o.startsWith('record.'))).toEqual([]);
});
Expand Down
Loading
Loading