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
12 changes: 12 additions & 0 deletions .changeset/27482-server-verdict-holds-cel-autosave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@object-ui/app-shell': patch
---

fix(app-shell): a server error at an RLS clause's path holds the permission-set autosave

Clause-②: no
Metadata-protocol: none

In Studio's permission-set editor, an RLS `using` or `check` clause that the publish will refuse (for example a `using` that reads a formula field, refused as `rls-predicate-unenforceable`) showed the server's message on the field but kept autosaving: the message was displayed, and the save gate never counted it. Now the server's errors at the clause's own path hold the autosave, the same way a CEL syntax error does, while the clause still shows the text that verdict judged.

The verdict describes the stored draft and nothing else, and no door judges a text without saving it. So an edit that changes the clause's text is not held: it saves, and that save's re-read brings the next verdict, which holds or releases from there. The editor decides nothing itself: it takes the server's errors as served and restates no publish rule.
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import {
type CelSuggestionKind,
type CelValueType,
} from './celAuthoring.js';
import { useServerErrorsAt } from './draftVerdict.js';
import { useServerErrorsAt, useServerHoldAt, type ServerDiagnosticEntry } from './draftVerdict.js';

const LINT_DEBOUNCE_MS = 250;

Expand Down Expand Up @@ -124,11 +124,21 @@ export interface CelPredicateFieldProps {
* the page holds (`rowLevelSecurity.0.using`). When the page's served verdict
* (`DraftVerdictContext`) has an error at exactly that path — what publish
* will refuse — the editor shows the server's message and withholds "Valid
* CEL". The lint, its findings and `onLintChange` are unaffected, as with
* `scopeIssue`. Omitted: no server verdict is read.
* CEL". The lint and its findings are unaffected, as with `scopeIssue`.
* Omitted: no server verdict is read.
*
* objectstack#27482 — and while the text on screen is the text that verdict
* judged (`DraftVerdictBodyContext`), each of those errors is also reported
* through `onLintChange` as a blocking error, so the host's Save gate holds
* 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.
*/
verdictPath?: string;
/** Reports the current lint issues up so the editor can gate Save on errors. */
/**
* Reports the current lint issues up so the editor can gate Save on errors —
* plus the server's errors at {@link verdictPath} on the judged text.
*/
onLintChange?: (issues: CelLintIssue[]) => void;
/**
* `role="value"` only: reports the inferred result type after each
Expand Down Expand Up @@ -206,14 +216,42 @@ export function CelPredicateField({
const fieldsKey = fieldNames.join(',');
const rootsKey = roots ? roots.join(',') : '';

// objectstack#27482 — the server's errors at this editor's path while the
// text on screen is the text they judged; they ride the lint report up.
const held = useServerHoldAt(verdictPath, value);
const heldKey = held.map((e) => e.message).join('\n');

// Keep the reporters out of the lint effect deps (parent may not memoize them).
const onLintChangeRef = React.useRef(onLintChange);
const onInferredTypeChangeRef = React.useRef(onInferredTypeChange);
const heldRef = React.useRef<readonly ServerDiagnosticEntry[]>(held);
React.useEffect(() => {
onLintChangeRef.current = onLintChange;
onInferredTypeChangeRef.current = onInferredTypeChange;
heldRef.current = held;
});

// The last lint result, so a verdict that arrives or lapses between two
// lint passes is reported against it.
const lintRef = React.useRef<CelLintIssue[]>([]);
const report = React.useCallback((lint: CelLintIssue[]) => {
lintRef.current = lint;
const server = heldRef.current;
onLintChangeRef.current?.(
server.length === 0
? lint
: [...lint, ...server.map((e): CelLintIssue => ({ severity: 'error', message: e.message }))],
);
}, []);
// Keyed on the messages, not on the array (rebuilt every render): report
// when the hold starts, changes or lapses, never on mount with none.
const reportedHeldRef = React.useRef('');
React.useEffect(() => {
if (heldKey === reportedHeldRef.current) return;
reportedHeldRef.current = heldKey;
report(lintRef.current);
}, [heldKey, report]);

/* Debounced lint (and, for role="value", result-type inference). */
React.useEffect(() => {
let cancelled = false;
Expand All @@ -223,7 +261,7 @@ export function CelPredicateField({
if (cancelled) return;
setIssues(res);
setLinted(true);
onLintChangeRef.current?.(res);
report(res);
});
if (role === 'value') {
inferCelValueType(value, hint).then((res) => {
Expand All @@ -246,7 +284,7 @@ export function CelPredicateField({
setIssues([]);
setLinted(false);
setInferredType(null);
onLintChangeRef.current?.([]);
report([]);
if (role === 'value') onInferredTypeChangeRef.current?.(null);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* objectstack#27482 — a server error at an RLS clause's own path holds the
* package door's autosave, the way a CEL parse fault does.
*
* objectstack#26858 made the clause SHOW the draft read's verdict; it never
* reached `celErrorCount`, so an expression the publish refuses (a USING that
* reads a formula field) kept autosaving with the server's message on screen.
* The verdict judges the stored draft and nothing else, so it holds while the
* clause still shows the text it judged, and lets the edit that changes that
* text go out: only that save brings the next verdict. ⛔ The fake door below
* is the only place the refusal is decided; the editor restates no rule.
*/

import '@testing-library/jest-dom/vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { MemoryRouter } from 'react-router-dom';

const REFUSED_USING = 'f_total > 5';
const ACCEPTED_USING = 'owner_id == current_user.id';
const REFUSAL = "rowLevelSecurity 'hot_deals' using reads 'f_total', a formula field the database cannot filter on";

const SET = {
name: 'sales_perms',
label: 'Sales',
objects: { a_account: { allowRead: true } },
fields: {},
};

interface Server {
draft: Record<string, unknown> | null;
draftReads: number;
/** Every body a save sent. */
sent: Array<Record<string, unknown>>;
}

/** The door's publish verdict on a stored draft: it refuses the one USING clause. */
function verdictOf(body: Record<string, unknown>) {
const rows = (body.rowLevelSecurity ?? []) as Array<{ using?: string }>;
const errors = rows.flatMap((r, i) =>
r.using === REFUSED_USING
? [{ path: `rowLevelSecurity.${i}.using`, message: REFUSAL, code: 'rls-predicate-unenforceable' }]
: [],
);
return errors.length ? { valid: false, errors } : { valid: true };
}

// `withPreviewDrafts` answers the client itself, not a promise (objectstack#26840).
type FakeClient = Record<string, (...args: never[]) => unknown>;

function makeClient(server: Server): FakeClient {
return {
layered: async () => ({ effective: SET, code: null, overlay: null, overlayScope: null }),
getDraft: async () => {
server.draftReads += 1;
return server.draft
? { type: 'permission', name: 'sales_perms', item: { ...server.draft, _draft: true, _diagnostics: verdictOf(server.draft) } }
: null;
},
withPreviewDrafts() { return this; },
list: async (type: string) => (type === 'object' ? [{ item: { name: 'a_account' } }] : []),
get: async () => null,
save: async (_type: string, _name: string, payload: Record<string, unknown>) => {
const body = JSON.parse(JSON.stringify(payload)) as Record<string, unknown>;
server.sent.push(body);
server.draft = body;
return payload;
},
};
}

let clientImpl: FakeClient;

vi.mock('./useMetadata', () => ({
useMetadataClient: () => clientImpl,
useMetadataTypes: () => ({
loading: false,
error: null,
entries: [{ type: 'permission', label: 'Permission', allowEnvironmentOverlay: true }],
}),
}));

vi.mock('./AssignedUsersSection', () => ({ AssignedUsersSection: () => null }));

import { PermissionMatrixEditPage } from './PermissionMatrixEditor';

afterEach(cleanup);

const USING_PLACEHOLDER = 'organization_id == current_user.organization_id';
const CHECK_PLACEHOLDER = 'optional — defaults to USING';
/** The shared autosave fires 1.5s after the last edit. */
const AUTOSAVE = { timeout: 5000 };
/** Longer than the autosave's pause. */
const PAST_THE_PAUSE = 2500;
const pause = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));

const policy = (using: string) => ({
...SET,
rowLevelSecurity: [{ name: 'hot_deals', object: 'a_account', operation: 'select', using, enabled: true }],
});

async function openPackageDoor(server: Server) {
clientImpl = makeClient(server);
render(
<MemoryRouter>
<PermissionMatrixEditPage type="permission" name="sales_perms" packageId="app.a" />
</MemoryRouter>,
);
await screen.findByText('a_account');
fireEvent.click(screen.getByRole('button', { name: /Row-Level Security/ }));
}

const edit = (placeholder: string, value: string) =>
fireEvent.change(screen.getByPlaceholderText(placeholder), { target: { value } });

describe('a server error at a clause path holds the autosave (objectstack#27482)', () => {
it('(a) a stored draft the publish refuses: another edit is held, no PUT, the server message stays', async () => {
const server: Server = { draft: policy(REFUSED_USING), draftReads: 0, sent: [] };
await openPackageDoor(server);
await screen.findByText(REFUSAL);

edit(CHECK_PLACEHOLDER, ACCEPTED_USING);

await waitFor(() => expect(screen.getByTestId('perm-autosave-held')).toBeInTheDocument(), AUTOSAVE);
await pause(PAST_THE_PAUSE);
expect(server.sent).toHaveLength(0);
expect(screen.getByText(REFUSAL)).toBeInTheDocument();
});

it('(b) the card flow: the refused clause saves once, its re-read holds the next edit', async () => {
const server: Server = { draft: policy(ACCEPTED_USING), draftReads: 0, sent: [] };
await openPackageDoor(server);
await screen.findByText('Valid CEL', {}, { timeout: 3000 });

// No client copy of the rule: the lint passes it and the first save goes out.
edit(USING_PLACEHOLDER, REFUSED_USING);
await waitFor(() => expect(server.sent).toHaveLength(1), AUTOSAVE);
expect(await screen.findByText(REFUSAL, {}, AUTOSAVE)).toBeInTheDocument();

edit(CHECK_PLACEHOLDER, ACCEPTED_USING);
await waitFor(() => expect(screen.getByTestId('perm-autosave-held')).toBeInTheDocument(), AUTOSAVE);
await pause(PAST_THE_PAUSE);
expect(server.sent).toHaveLength(1);
});

it('(c) editing the clause away releases: the save goes out and its clean re-read clears the verdict', async () => {
const server: Server = { draft: policy(REFUSED_USING), draftReads: 0, sent: [] };
await openPackageDoor(server);
await screen.findByText(REFUSAL);

edit(USING_PLACEHOLDER, ACCEPTED_USING);

await waitFor(() => expect(server.sent).toHaveLength(1), AUTOSAVE);
await waitFor(() => expect(screen.queryByText(REFUSAL)).toBeNull(), AUTOSAVE);
expect(await screen.findByTestId('perm-saved-at', {}, AUTOSAVE)).toBeInTheDocument();
expect(screen.queryByTestId('perm-autosave-held')).toBeNull();
});

it('(d) typed back to the judged text before the autosave fires: held again, no PUT', async () => {
const server: Server = { draft: policy(REFUSED_USING), draftReads: 0, sent: [] };
await openPackageDoor(server);
await screen.findByText(REFUSAL);

edit(CHECK_PLACEHOLDER, ACCEPTED_USING);
edit(USING_PLACEHOLDER, 'f_total > 50');
edit(USING_PLACEHOLDER, REFUSED_USING);

await waitFor(() => expect(screen.getByTestId('perm-autosave-held')).toBeInTheDocument(), AUTOSAVE);
await pause(PAST_THE_PAUSE);
expect(server.sent).toHaveLength(0);
});
});
Loading
Loading