Skip to content

Commit c5d51f0

Browse files
fix(studio): the Data pillar's Validations and Actions panels and ResourceEditPage keep a CEL hold with the draft (#27629) (#27697)
<!-- claude-projects-attribution: {"github_login":"os-bill"} --> _Requested by **Bill** · [project thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgCkKxLYbQUYZhmkBJUjE4nE)_ ## 维护者速读 - 改了什么:Studio 数据页的「校验规则」「动作」两个面板,以及元数据编辑页(ResourceEditPage),表达式写坏后离开标签页或换选中项,草稿仍然不保存;页面上出现一行提示点名是哪条规则 / 哪个动作 / 哪一项,带「Show me」跳回去修。 - 为什么改:按 #27485 裁决 A(6500685972,「同意」)第 3 条把「拦截跟草稿走」推广到兄弟面;以前一离开标签页或换选中,自动保存就把引擎拒收的表达式写进草稿,直到发布才报错且不点名。 - 风险与回滚:只改前端拦截判定,不动 `useDraftAutoSave` 的计时器与单飞;最坏情况是多拦一次(提示行 + Show me 可解),回滚即 revert 本 PR。 - 先看:`ui/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx`(`panelCelVerdicts` 一段)、`ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx`(`inspectorVerdicts` 与 `selectionSlicePath`)、两个新测试文件。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/app-shell/src/views/studio-design/DataPillar.panelHold-27629.test.tsx packages/app-shell/src/views/metadata-admin/ResourceEditPage.celHold-27629.test.tsx`,预期 8/8 通过。 - 需要你做的:无,席位落地。 Fixes #27629 Clause-②: no Metadata-protocol: none ## What changes The maintainer's ruling on #27485 (comment 6500685972, letter A) makes a CEL parse-fault hold belong to the draft, not to the open editor. PR #27617 did that for the Data pillar's field inspector; this PR ports the same shape to the sibling surfaces item 3 of that ruling names. One measured repro per surface came first (all failed on `main`, see Tests). - **Data pillar, Validations and Actions panels.** The panels now also report their verdicts per rule or action name (`onCelVerdictsChange`, pruned to the items in the draft; `onBlockingIssuesChange` is unchanged). `DataPillar` keeps each verdict in `panelCelVerdicts` with the stamp of what it judged (`panelItemCelStamp`: the rule or action itself plus the object's field names, the counterpart of `fieldCelStamp`), retaken while the panel is mounted. With the tab left, a kept verdict holds while the draft still carries that stamp; an edit to the item, its removal or the object's next load ends it. While the panel is mounted its own report is the live verdict and wins. The held line names the item (new clauses `engine.studio.held.celInvalidRule` / `celInvalidAction`), its "Show me" reopens it (the Validations panel gains the `focus` prop the Actions panel already had), and the rail switch's flush-or-ask, the Studio Publish refusal and the held-edit report all read the same `objHeld`, unchanged. - **Settings panel: left out, does not reproduce.** `ObjectDefaultInspector` mounts no CEL editor, so the panel never reports a verdict (its own header says so). It keeps the tab-stamped count it had. - **`ResourceEditPage`.** Each selection's last live inspector verdict is kept in `inspectorVerdicts` with the fingerprint of the draft slice it judged: `selectionSlicePath` reads the selection id as a path (a page block's `regions[0].components[0]`), else finds the object whose `id` / `name` / `key`, or map key, is the id; the default inspector and an unlocatable id fall back to the whole document. The fingerprint is `issueSliceFingerprint`, the one the server-refusal hold already uses. A kept verdict holds Save, the autosave and the shortcut while that slice is unchanged; the editor shows a held line (`engine.studio.held.inspectorInvalid`) naming the item, and "Show me" reselects it. Item change and the item's load clear the kept verdicts. - ⛔ Per ruling item 4: no dialog on close, no silent revert, no second autosave path, and `useDraftAutoSave.ts` is untouched. ## Tests - New `DataPillar.panelHold-27629.test.tsx` (5 cases): leaving Validations / Actions keeps the hold and the held line names the rule / action; "Show me" reopens it with its text and fixing it releases the hold; removing the rule ends it. - New `ResourceEditPage.celHold-27629.test.tsx` (3 cases): clearing or changing the selection keeps Save and the autosave held and names the block; "Show me" reselects it and fixing it releases. - `DataPillar.panelGate.test.tsx`: the case "expires the panel count when the author leaves the Rules tab" pinned the superseded tab stamp; flipped to "keeps the hold when the author leaves the Rules tab, and names the rule", citing the ruling. Readings, all on the final head `7484a827` (the work merged with `origin/main` at `15b099e7`) unless a line says otherwise: <details><summary>Repro on main (before the fix, tree 5d759b1 + the two new test files): 8 failed / 8</summary> ``` DataPillar.panelHold-27629: 5 failed (leaving Validations / Actions: client.save called 1 time; held line absent) ResourceEditPage.celHold-27629: 3 failed (save called 1 time after clearing or changing the selection; no held line) Test Files 2 failed (2) · Tests 8 failed (8) ``` </details> <details><summary>Affected suites (pre-merge head efe3f9a): 198 files / 1283 tests passed</summary> Every `DataPillar.*`, `StudioDesignSurface.*`, `ObjectValidationsPanel*`, `ObjectActionsPanel*`, `ObjectSettingsPanel*` test under `views/studio-design/` and every `ResourceEditPage.*` test under `views/metadata-admin/`, `--reporter=verbose`, under the verification lock: `Test Files 198 passed (198) · Tests 1283 passed (1283)`, lock VERDICT command-exit 0. The rest of the ui tree is left to CI `Test UI`. </details> <details><summary>After merging origin/main (head 7484a82)</summary> ``` pnpm install --frozen-lockfile :: exit 0 pnpm --filter '@object-ui/app-shell^...' build :: exit 0 pnpm --filter @object-ui/app-shell run type-check :: exit 0 vitest: DataPillar.panelHold-27629, ResourceEditPage.celHold-27629, DataPillar.panelGate, DataPillar.celGate, ResourceEditPage.celGate, ObjectValidationsPanel.celGate :: Test Files 6 passed (6) · Tests 27 passed (27) ``` </details> <details><summary>Ablations (one per surface, restored, blob == HEAD proven by ablation-replace)</summary> - `StudioDesignSurface.tsx`: the kept panel-verdict condition replaced by `if (false) {` → `leaving the Validations tab keeps the hold…` and `leaving the Actions tab keeps the hold…` both red (2 failed); restored, `git diff HEAD` empty. - `ResourceEditPage.tsx`: the kept inspector-verdict condition replaced by `if (false) {` → all 3 cases of `ResourceEditPage.celHold-27629` red; restored, `git diff HEAD` empty. - app-shell tests import `src` directly, so no dist build sits between the mutation and the run. </details> <details><summary>Derived gates (dispatch-gates --commands on this diff): 34 run, 34 exit 0, reconciled</summary> ``` node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:pm-changeset-deadline-census :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` `dispatch-gates --ran`: 34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN. Also `node scripts/check-new-cross-file-line-citations.mjs` (ui): 0 new citations, exit 0. Families CI runs beyond the derived set are read from this PR's check-runs. </details> ## Acceptance notes 1. **Residual 250 ms edge, named again, not closed.** The editors lint 250 ms after the last keystroke, so a `flush` (the Data rail switch) inside that window can still read the previous verdict, and a reopened held item reports nothing for about that long (its kept verdict stands meanwhile, so it stays held). Closing it needs a "lint pending" signal from the editors to the host; no such signal exists on this surface, and adding one reaches into every CEL editor, so it is out of this card's file surface. The timer path stays safe: a rising hold cancels the 1.5 s timer and a falling one re-arms a fresh one. 2. **`ResourceEditPage`'s slice locator is best-effort and fails toward release.** When a selection id is neither a path nor an item identity in the draft, the kept verdict is stamped against the whole document, so any edit ends the hold early (the old behaviour, narrowed). A wrongly located slice can over-hold; the held line's "Show me" reopens the item and its live verdict then wins, so Save is never wedged with nothing on screen. 3. **The inspector channel on `ResourceEditPage` is shared.** It carries CEL faults and the required-field refusal in a gated section (ruling 5831744213); both are now kept with the draft, since the count does not tell them apart. The held line's wording is neutral for that reason. 4. **Stale contract prose outside this card's surface.** `MetadataInspectorProps.onBlockingIssuesChange` in `inspector-registry.ts` still says hosts "must reset their own count when the selection changes"; both hosts now keep it with the draft (the field inspector since #27617). Not edited here (file not in the claim); a doc line for the seat to route. 5. **Commit trailer.** The first commit on this branch (`49b01042`) carries the dispatching seat's session id in its `Claude-Session:` trailer instead of this run's; later commits carry this run's. History is not rewritten (the branch is not `claude/issue-*`, so no force push). ## Verification Run the two new suites (command in 维护者速读). In a running Console: open Studio, Data, an object, Validations, set a rule's guard to `record.status ==`, switch to Form: the draft stays unsaved and the line "Not saved yet: an expression on the rule “…” has an error to fix" appears with Show me. --- _Generated by [Claude Code](https://claude.ai/code/session_01Utp6CYaLJTMXdChBaqK9KL)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent eda55b9 commit c5d51f0

10 files changed

Lines changed: 787 additions & 43 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
'@object-ui/i18n': patch
4+
---
5+
6+
fix(studio): leaving the Validations or Actions tab, or the metadata editor's selection, keeps an expression that does not parse unsaved and names it (#27629)
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
- The Data pillar's Validations and Actions panels held the object's draft while a rule's guard or an action's predicate did not parse, but only while their tab was open: leaving the tab ended the hold, and the autosave sent the broken expression as the object's draft. The hold now belongs to the draft, as the field inspector's does since #27485: it lasts while the draft still carries the rule or action the editor judged, and ends when that item is edited, removed or the object reloads. While it lasts, the pillar shows the held line ("Not saved yet: an expression on the rule “Rule A” has an error to fix…") with a **Show me** that reopens the rule or action, and a switch to another object asks first.
12+
- The metadata editor (`ResourceEditPage`) did the same with its selection: clearing or changing it re-enabled Save and the autosave over a broken predicate in the item it had open. The hold now stays with the draft slice the inspector judged, the editor shows a held line naming the item, and **Show me** reselects it.
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#27629 — the metadata editor keeps a scoped inspector's CEL hold
5+
* with the DRAFT, not with the selection.
6+
*
7+
* The maintainer's ruling on #27485 (comment 6500685972, 2026-10-29T15:17Z,
8+
* letter A) makes a CEL parse-fault hold belong to the draft: it lasts while
9+
* the draft still carries the text the editor judged; editing that text,
10+
* removing the item or reloading ends it; while it holds, a held line names the
11+
* item with a "Show me" that reopens it. Item 3 asks for the sibling surfaces;
12+
* this suite pins `MetadataResourceEditPage`. Before this card the inspector's
13+
* count was stamped with the selection, so clearing or changing the selection
14+
* released Save and the autosave while the broken predicate stayed in the draft.
15+
*
16+
* Set up as {@link file://./ResourceEditPage.celGate.test.tsx}: only the page
17+
* canvas is stubbed (to emit selections); the block inspector, its
18+
* ConditionBuilder and this host's gating are real, the CEL engine is stubbed.
19+
*/
20+
21+
import '@testing-library/jest-dom/vitest';
22+
import * as React from 'react';
23+
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
24+
import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react';
25+
import { MemoryRouter } from 'react-router-dom';
26+
27+
const PAGE = {
28+
name: 'home',
29+
label: 'Home',
30+
type: 'home',
31+
template: 'default',
32+
regions: [
33+
{
34+
name: 'main',
35+
components: [
36+
{ type: 'text', id: 'b1', visibleWhen: 'record.amount > 10' },
37+
{ type: 'text', id: 'b2' },
38+
],
39+
},
40+
],
41+
};
42+
43+
const mockClient = {
44+
list: vi.fn(async () => []),
45+
listDrafts: vi.fn(async () => []),
46+
layered: vi.fn(async () => ({ effective: PAGE, code: PAGE, editable: true })),
47+
getDraft: vi.fn(async () => null),
48+
get: vi.fn(async () => null),
49+
save: vi.fn(async () => ({})),
50+
saveDraft: vi.fn(async () => ({})),
51+
};
52+
53+
vi.mock('./useMetadata', async (importOriginal) => {
54+
const mod = await importOriginal<typeof import('./useMetadata')>();
55+
return {
56+
...mod,
57+
useMetadataClient: () => mockClient,
58+
useMetadataTypes: () => ({
59+
entries: [{ type: 'page', name: 'page', label: 'Page', allowEnvironmentOverlay: true }],
60+
}),
61+
};
62+
});
63+
64+
import { MetadataResourceEditPage } from './ResourceEditPage';
65+
import { registerBuiltinInspectors } from './inspectors';
66+
import { registerMetadataPreview, getMetadataPreview, type MetadataSelection } from './preview-registry';
67+
import { __setCelFormulaLoader } from './celAuthoring';
68+
69+
registerBuiltinInspectors();
70+
71+
const DANGLING = /[*+\-/&|=<>]\s*$/;
72+
73+
function stubEngine() {
74+
__setCelFormulaLoader(() =>
75+
Promise.resolve({
76+
validateExpression: (_role: string, input: unknown) => {
77+
const src = typeof input === 'string' ? input : String((input as { source?: string })?.source ?? '');
78+
return DANGLING.test(src)
79+
? { ok: false, errors: [{ message: 'Parse error: expression ends after an operator' }], warnings: [] }
80+
: { ok: true, errors: [], warnings: [] };
81+
},
82+
introspectScope: () => ({ fields: ['amount'], roots: ['record'], functions: ['has'] }),
83+
inferExpressionType: () => 'boolean' as const,
84+
}),
85+
);
86+
}
87+
88+
/** Canvas stand-in: emits the selections the real canvas emits on click and on an empty click. */
89+
function StubPageCanvas({
90+
onSelectionChange,
91+
}: {
92+
onSelectionChange?: (next: MetadataSelection | null) => void;
93+
}) {
94+
return (
95+
<>
96+
<button type="button" onClick={() => onSelectionChange?.({ kind: 'block', id: 'regions[0].components[0]', label: 'Intro text' })}>
97+
select the first block
98+
</button>
99+
<button type="button" onClick={() => onSelectionChange?.({ kind: 'block', id: 'regions[0].components[1]', label: 'Footer text' })}>
100+
select the second block
101+
</button>
102+
<button type="button" onClick={() => onSelectionChange?.(null)}>
103+
clear the selection
104+
</button>
105+
</>
106+
);
107+
}
108+
109+
const realPagePreview = getMetadataPreview('page');
110+
111+
beforeEach(() => {
112+
stubEngine();
113+
registerMetadataPreview('page', StubPageCanvas as never);
114+
});
115+
116+
afterEach(() => {
117+
cleanup();
118+
__setCelFormulaLoader(undefined);
119+
mockClient.save.mockClear();
120+
mockClient.saveDraft.mockClear();
121+
if (realPagePreview) registerMetadataPreview('page', realPagePreview);
122+
});
123+
124+
const saveButton = () =>
125+
screen.getByRole('button', { name: /Save \(⌘S\)|Fix the issues shown in the inspector before saving\./ });
126+
127+
/** Past the autosave's 1.5 s debounce. */
128+
const outlastDebounce = () => new Promise((r) => setTimeout(r, 2300));
129+
130+
async function openFirstBlockCel(): Promise<HTMLTextAreaElement> {
131+
fireEvent.click(await screen.findByRole('button', { name: 'select the first block' }));
132+
fireEvent.click(await screen.findByText('Expression'));
133+
return screen.getAllByRole('combobox').find((el) => el.tagName === 'TEXTAREA') as HTMLTextAreaElement;
134+
}
135+
136+
async function breakFirstBlock(): Promise<void> {
137+
render(
138+
<MemoryRouter initialEntries={['/metadata/page/home']}>
139+
<MetadataResourceEditPage type="page" name="home" />
140+
</MemoryRouter>,
141+
);
142+
const box = await openFirstBlockCel();
143+
fireEvent.change(box, { target: { value: 'record.amount >' } });
144+
await waitFor(() => expect(saveButton()).toBeDisabled(), { timeout: 4000 });
145+
}
146+
147+
const LEAVERS: Array<[string, () => void]> = [
148+
['clearing the selection', () => fireEvent.click(screen.getByRole('button', { name: 'clear the selection' }))],
149+
['selecting another block', () => fireEvent.click(screen.getByRole('button', { name: 'select the second block' }))],
150+
];
151+
152+
describe('MetadataResourceEditPage — the CEL hold belongs to the draft, not to the selection (objectstack#27629)', () => {
153+
it.each(LEAVERS)('%s keeps Save and the autosave held, and the held line names the block', async (_how, leave) => {
154+
await breakFirstBlock();
155+
leave();
156+
await outlastDebounce();
157+
expect(mockClient.save).not.toHaveBeenCalled();
158+
expect(mockClient.saveDraft).not.toHaveBeenCalled();
159+
expect(saveButton()).toBeDisabled();
160+
expect(screen.getByTestId('inspector-held-message')).toHaveTextContent('Intro text');
161+
});
162+
163+
it('"Show me" reopens the block with its text, and fixing it there releases the hold', async () => {
164+
await breakFirstBlock();
165+
fireEvent.click(screen.getByRole('button', { name: 'clear the selection' }));
166+
await waitFor(() => expect(screen.getByTestId('inspector-held')).toBeInTheDocument(), { timeout: 3000 });
167+
168+
fireEvent.click(screen.getByRole('button', { name: 'Show me' }));
169+
const box = await waitFor(() => {
170+
const el = screen.queryAllByRole('combobox').find((e) => e.tagName === 'TEXTAREA') as HTMLTextAreaElement | undefined;
171+
expect(el).toBeTruthy();
172+
return el!;
173+
});
174+
expect(box.value).toBe('record.amount >');
175+
fireEvent.change(box, { target: { value: 'record.amount > 20' } });
176+
await waitFor(() => expect(saveButton()).toBeEnabled(), { timeout: 4000 });
177+
expect(screen.queryByTestId('inspector-held')).toBeNull();
178+
});
179+
});

0 commit comments

Comments
 (0)