Skip to content

Commit f296334

Browse files
fix(app-shell): a validation rule guard the publish refuses holds the Studio autosave (#27644) (#28027)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgVFfRuMa522AiLcTZSChFrD)_ Fixes #27644 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:Studio 数据页的「校验规则」标签保留草稿读回的服务端判定,每条规则的条件带上自己的判定路径(`validations.N.condition`),发布会拒收的条件就挡住自动保存。 - 为什么改:之前判定被丢掉,空值比较这类发布拒收的规则照样显示没问题并自动保存(PUT 200),直到点发布才被拒。 - 风险与回滚:只动控制台三个文件;每次自动保存后多一次草稿读取。回滚即还原本 PR,不涉及数据或元数据形状。 - 先看:`StudioDesignSurface.tsx` 的 DataPillar 加载和 `doSave` 两处、`ConditionBuilder.tsx` 行模式的 hold 计数、`ObjectValidationsPanel.tsx` 的 `ruleIndex`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/DataPillar.validationVerdictHold-27644.test.tsx`,预期 6 个用例全绿。 - 需要你做的:无,席位落地。 ## What changes - **`DataPillar`** keeps the object draft read's served `_diagnostics` and the body they judged (`editedItemDiagnostics`, the layered read's verdict and body without a pending draft), drops both on each object load, and re-reads the draft after each save, so the next verdict is the stored body's. A sequence counter keeps a stale re-read from landing over a later load. Both are provided to the Validations tab only, through the existing `DraftVerdictContext` / `DraftVerdictBodyContext`. - **`ObjectValidationsPanel`** hands each rule's guard its verdict path: `validations.N.GUARD`, where N is the rule's index in the draft's raw `validations` list (the index the lint locates by, since #27643) and GUARD is `condition`, or `when` on a conditional rule. A rule not yet in the draft gets none. - **`ConditionBuilder`** takes `verdictPath`. In raw mode it forwards it to `CelPredicateField`, whose #27612 `useServerHoldAt` already reports the served error through `onLintChange` while the text on screen is the judged text. In row mode no CEL editor is mounted, so the builder reads the same `useServerHoldAt` itself: it shows the served message under the rows and counts it in `onBlockingIssuesChange`. No second hold mechanism and no client copy of the rule: the server's errors are taken whole, at their exact path, through the #27612 helpers. The panel's verdict count already rides #27629's per-rule kept verdicts, so the hold also survives leaving the tab, and the held line names the rule. ## Why A guard the publish refuses (an ordering compare on a nullable field, `record.amount > 100`) read clean in the Validations tab and autosaved until Publish refused it. Measured on this branch before the fix: the stored refused guard round-trips as a ROW, so the raw-editor half alone would not hold the card's own shape (ablation 2 below). ## Tests - New `DataPillar.validationVerdictHold-27644.test.tsx`, pinned the way `PermissionMatrixEditor.serverVerdictHold-27482.test.tsx` is: a fake door decides the refusal and serves it in the draft read's `_diagnostics`; the CEL engine is stubbed to pass every guard, so every hold is the served verdict's. (a) stored refused draft: another edit to the rule is held, no PUT, the message stays; (b) the card flow: the refused guard saves once, its re-read holds the next edit; (c) editing the guard away releases, the save goes out and its clean re-read clears the message; (d) typed back to the judged text before the autosave fires: held again; (e) leaving the tab keeps the hold; control: the rule beside it shows none of it. - New `ConditionBuilder.serverHold-27644.test.tsx`: the row builder shows and reports the served error, editing the row away reports 0, and without `verdictPath` nothing is read. ## Acceptance notes - **Not reproduced live.** The pre-fix behaviour (reads "Valid CEL", PUT 200 until Publish) was not reproduced against a running stack; the ablations below are the before-state evidence. - **An envelope-form guard is not held.** `serverHoldAt` compares the judged body's value at the path with the text on screen, and a guard stored as an expression envelope (`dialect` + `source`) reads as an object there, so it never matches. No producer of envelope-form validation guards was found (none in `examples/`); the Studio writes a plain string unless the prior value was an envelope. Left as is, not filed. - **One extra draft read per autosave** in the Data pillar, as the Permission matrix already does after each package-door save. - `StudioDesignSurface.tsx` hunks: the shared import block (one name added) and `DataPillar` only; disjoint from #27988's `InterfacesPillar` hunks. ## Verification All readings on this branch, against `objectstack-ai/objectstack`. The final head is `479a7575`: this card's six files over `origin/main` `066ff51e`, which already carries #27988. <details><summary>New tests and ablations</summary> - `pnpm --dir ui exec vitest run packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.serverHold-27644.test.tsx packages/app-shell/src/views/studio-design/DataPillar.validationVerdictHold-27644.test.tsx`: Test Files 2 passed, Tests 9 passed. - Ablation 1, the served verdict dropped (`DraftVerdictContext.Provider value={objVerdict}` to `value={undefined}`, through `scripts/ablation-replace.mjs`, anchor 1 to 0): `DataPillar` (a) to (e) fail, the control passes (5 failed, 1 passed). Restored: blob equals HEAD, `git diff HEAD` empty. - Ablation 2, the row builder's count dropped (`raw ? celErrors : held.length` to `raw ? celErrors : 0`): both row-builder cases fail, and so do `DataPillar` (a) and (e), the cases that open the stored refused guard (4 failed, 5 passed). This is the measurement that the stored guard opens as a row. Restored the same way. </details> <details><summary>Package checks on the final head</summary> - `pnpm --filter @object-ui/app-shell type-check` (both `tsc` projects; `tsconfig.test.json` compiles the tests): exit 0. - 169 test files around the change (`ObjectValidationsPanel*`, `DataPillar*`, `StudioDesignSurface*`, `ConditionBuilder*`, `CelPredicateField*`, the `serverVerdict*` suites), including both new ones: Test Files 169 passed, Tests 1234 passed. - `eslint` on the five changed `ui/` files: 0 errors (the warnings are on lines this PR does not touch). - `pnpm check:new-line-citations`: 0 new citations. - Narrowed, declared: the whole `packages/app-shell/` suite (about 1950 files) was not completed locally; a run passed an hour on the shared box before the second `origin/main` merge, and was stopped. `Test UI` in CI runs it. </details> <details><summary>Derived gates</summary> `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 34 commands, the same list at every head. At `23227677` all 34 exit 0, including `node scripts/check-sdui-manifest.mjs`. The re-run on the final head is reported in the os-dev report on the card. </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CZjUkAAB57j1TQa8sE)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 13d85bb commit f296334

6 files changed

Lines changed: 465 additions & 11 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): a validation rule guard the publish refuses holds the Studio Data pillar's autosave (#27644)
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The Data pillar read the object draft and dropped its served `_diagnostics`, so a validation rule guard the publish refuses (an ordering compare on a nullable field, `record.amount > 100`) read clean and autosaved until Publish refused it. The pillar now keeps the draft read's verdict and the body it judged, re-reads both after each save, and hands them to the Validations tab. Each rule's guard carries its own verdict path (`validations.N.condition`, a conditional rule's `.when`), so the error the server located there is shown under the guard and holds the autosave while the guard still shows the judged text, in the raw CEL editor and in the row builder alike. Editing the guard releases the hold: that save goes out and its re-read brings the next verdict. No rule is restated in the console.
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#27644 — the served error at `verdictPath` counts as blocking in
5+
* the ROW builder too.
6+
*
7+
* The raw editor reports the server's errors through `CelPredicateField`'s
8+
* lint report (objectstack#27482), but the row builder mounts no CEL editor,
9+
* and a stored guard the publish refuses (`record.amount > 100`, an ordering
10+
* compare on a nullable field) round-trips as a row. So the builder reads the
11+
* same hold itself: while `value` is the judged text it shows the server's
12+
* message and reports it; once the row is edited away it reports none.
13+
*/
14+
15+
import '@testing-library/jest-dom/vitest';
16+
import * as React from 'react';
17+
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
18+
import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
19+
20+
const state = vi.hoisted(() => ({
21+
metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]), withPreviewDrafts() { return this; } },
22+
}));
23+
vi.mock('../useMetadata', () => ({
24+
useMetadataClient: () => state.metadataClient,
25+
}));
26+
27+
import { ConditionBuilder } from './ConditionBuilder';
28+
import { DraftVerdictBodyContext, DraftVerdictContext } from '../draftVerdict';
29+
import { __setCelFormulaLoader } from '../celAuthoring';
30+
31+
const REFUSED = 'record.amount > 100';
32+
const REFUSAL = "validation rule 'amount_cap' applies `>` to `record.amount`, which may be null";
33+
const PATH = 'validations.0.condition';
34+
const VERDICT = { valid: false, errors: [{ path: PATH, message: REFUSAL, code: 'expression-invalid' }] };
35+
const JUDGED = { validations: [{ name: 'amount_cap', condition: REFUSED }] };
36+
const FIELDS = [{ name: 'amount', label: 'Amount', type: 'number' }];
37+
38+
beforeEach(() => {
39+
__setCelFormulaLoader(() =>
40+
Promise.resolve({
41+
validateExpression: () => ({ ok: true, errors: [], warnings: [] }),
42+
introspectScope: () => ({ fields: ['amount'], roots: ['record'], functions: [] }),
43+
inferExpressionType: () => 'boolean' as const,
44+
}),
45+
);
46+
});
47+
afterEach(() => {
48+
cleanup();
49+
__setCelFormulaLoader(undefined);
50+
});
51+
52+
function Host({ verdictPath, onCount }: { verdictPath?: string; onCount: (n: number) => void }) {
53+
const [value, setValue] = React.useState(REFUSED);
54+
return (
55+
<DraftVerdictContext.Provider value={VERDICT}>
56+
<DraftVerdictBodyContext.Provider value={JUDGED}>
57+
<ConditionBuilder
58+
value={value}
59+
onCommit={setValue}
60+
fields={FIELDS}
61+
scope="record"
62+
verdictPath={verdictPath}
63+
onBlockingIssuesChange={onCount}
64+
/>
65+
</DraftVerdictBodyContext.Provider>
66+
</DraftVerdictContext.Provider>
67+
);
68+
}
69+
70+
describe('ConditionBuilder — the row builder counts the served hold (objectstack#27644)', () => {
71+
it('opens the refused guard as a row, shows the server message and reports it as blocking', async () => {
72+
const onCount = vi.fn();
73+
render(<Host verdictPath={PATH} onCount={onCount} />);
74+
// Row mode: the value input, no raw CEL editor.
75+
expect(screen.getByDisplayValue('100')).toBeInTheDocument();
76+
expect(screen.queryAllByRole('combobox').some((el) => el.tagName === 'TEXTAREA')).toBe(false);
77+
expect(screen.getByTestId('cel-server-error')).toHaveTextContent(REFUSAL);
78+
await waitFor(() => expect(onCount).toHaveBeenLastCalledWith(1));
79+
});
80+
81+
it('editing the row away releases: no message, 0 reported', async () => {
82+
const onCount = vi.fn();
83+
render(<Host verdictPath={PATH} onCount={onCount} />);
84+
await waitFor(() => expect(onCount).toHaveBeenLastCalledWith(1));
85+
86+
fireEvent.change(screen.getByDisplayValue('100'), { target: { value: '50' } });
87+
88+
await waitFor(() => expect(onCount).toHaveBeenLastCalledWith(0));
89+
expect(screen.queryByTestId('cel-server-error')).toBeNull();
90+
});
91+
92+
it('control: without a verdictPath the builder reads no verdict', async () => {
93+
const onCount = vi.fn();
94+
render(<Host onCount={onCount} />);
95+
await waitFor(() => expect(onCount).toHaveBeenCalled());
96+
expect(onCount).toHaveBeenLastCalledWith(0);
97+
expect(screen.queryByTestId('cel-server-error')).toBeNull();
98+
});
99+
});

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/ConditionBuilder.tsx‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,12 @@ import {
2626
Button, Input, Label,
2727
Select, SelectTrigger, SelectContent, SelectItem, SelectValue,
2828
} from '@object-ui/components';
29-
import { Plus, X, Code2, ListFilter } from 'lucide-react';
29+
import { AlertCircle, Plus, X, Code2, ListFilter } from 'lucide-react';
3030
import { BOOLEAN_VALUE_TYPES, expandEmptyOperator, type EmptyOperatorArm } from '@objectstack/spec/data';
3131
import { useObjectFields } from '../previews/useObjectFields.js';
3232
import { CelPredicateField } from '../CelPredicateField.js';
3333
import type { CelLintIssue } from '../celAuthoring.js';
34+
import { useServerHoldAt } from '../draftVerdict.js';
3435
import { t, tFormat, useMetadataLocale } from '../i18n.js';
3536

3637
type Op = '==' | '!=' | '>' | '<' | '>=' | '<=' | 'truthy' | 'falsy';
@@ -820,7 +821,7 @@ function initFrom(value: string, valueOf: SubjectValueOf): { rows: Row[]; join:
820821
return { rows: [], join: '&&', raw: !!value };
821822
}
822823

823-
export function ConditionBuilder({ label, value, onCommit, objectName, fields: fieldsProp, disabled, onBlockingIssuesChange, subjects, scope, roots, scopeIssue, labelSubjects = false }: {
824+
export function ConditionBuilder({ label, value, onCommit, objectName, fields: fieldsProp, disabled, onBlockingIssuesChange, subjects, scope, roots, scopeIssue, labelSubjects = false, verdictPath }: {
824825
label?: string;
825826
value: string;
826827
onCommit: (cel: string) => void;
@@ -911,6 +912,15 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f
911912
* beside it. Off by default, so a mount that lists spellings keeps them.
912913
*/
913914
labelSubjects?: boolean;
915+
/**
916+
* objectstack#27644 — the item-relative path `value` is stored at in the body
917+
* the page's served verdict judged (`validations.1.condition`), forwarded to
918+
* `CelPredicateField`. The server's errors there count as blocking while
919+
* `value` is the text they judged, in either mode: the raw editor reports
920+
* them with its lint, and the row builder, which mounts no CEL editor, shows
921+
* and counts them itself. Omitted: no server verdict is read.
922+
*/
923+
verdictPath?: string;
914924
}) {
915925
const { fields: hookFields } = useObjectFields(objectName);
916926
const fields = fieldsProp ?? hookFields;
@@ -1063,7 +1073,11 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f
10631073
const errs = issues.filter((i) => i.severity === 'error').length;
10641074
setCelErrors((prev) => (prev === errs ? prev : errs));
10651075
}, []);
1066-
const blockingIssues = raw ? celErrors : 0;
1076+
// objectstack#27644 — the row builder has no editor to report the server's
1077+
// errors at this path, so it counts them here; the raw editor's report
1078+
// already carries them.
1079+
const held = useServerHoldAt(verdictPath, value || '');
1080+
const blockingIssues = raw ? celErrors : held.length;
10671081
// Held in a ref so an unmemoized parent callback cannot re-fire the effect.
10681082
const onBlockingIssuesChangeRef = React.useRef(onBlockingIssuesChange);
10691083
React.useEffect(() => {
@@ -1114,6 +1128,7 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f
11141128
roots are the engine's own, unchanged. */
11151129
roots={offeredRoots}
11161130
scopeIssue={scopeIssue}
1131+
verdictPath={verdictPath}
11171132
t={tLocal}
11181133
/>
11191134
{value && !parse(value, valueOf) && (
@@ -1205,6 +1220,18 @@ export function ConditionBuilder({ label, value, onCommit, objectName, fields: f
12051220
{compiled && (
12061221
<div className="rounded bg-muted/40 px-2 py-1 text-[10px] font-mono text-muted-foreground break-all">{compiled}</div>
12071222
)}
1223+
1224+
{/* objectstack#27644 — what the publish refuses in this condition, as served. */}
1225+
{held.length > 0 && (
1226+
<ul className="space-y-1" aria-live="polite">
1227+
{held.map((e, i) => (
1228+
<li key={i} className="flex items-start gap-1.5 text-[11px] text-destructive" data-testid="cel-server-error">
1229+
<AlertCircle className="mt-[1px] h-3 w-3 shrink-0" />
1230+
<span>{e.message}</span>
1231+
</li>
1232+
))}
1233+
</ul>
1234+
)}
12081235
</div>
12091236
);
12101237
}

0 commit comments

Comments
 (0)