fix(app-shell): inspectors can block Save — the field inspector gates on CEL errors (#4306) by yinlianghui · Pull Request #4536 · objectstack-ai/objectui

yinlianghui · 2026-08-13T05:52:49Z

record.est_hours * in a formula field showed its inline parse error and saved anyway — PUT 200, success toast, and publishing made the malformed expression the live field definition. The RLS editor refused the same class of input, so one console disagreed with itself.

Why this needed a contract, not a one-file fix

The card's stated root cause did not survive measurement, and the PM ruling that governs this PR was issued on the corrected picture (measurement: #4306 comment 5276045542, ruling: comment 5276058655).

CelPredicateField has always reported findings through onLintChange(issues) — it has no onCelErrorsChange prop at all. onCelErrorsChange is the aggregating parent's prop on PermissionAdvancedFacets, consumed by PermissionMatrixEditor. Both ends live inside the permission editor, which owns its own Save button.

The field inspector is a registry component. The button it must gate belongs to a host — the Studio Data pillar's "Save draft", or the metadata editor's Save — reachable only through MetadataInspectorProps, which had no error channel, and there is no context alternative (zero createContext hits in metadata-admin and studio-design). So there was no existing channel to connect to; the fix is to add one.

Ruling compliance

Prop named for blocking issues, not CEL. MetadataInspectorProps gains optional onBlockingIssuesChange(count). Optional, so all existing inspectors stay valid. Named generally so the five other inspectors with the same gap (CEL blocking errors are dropped on the floor in ConditionBuilder and ConditionalFormattingEditor too — same ungated-Save family as #4306 #4527, gated on this landing) wire against this contract rather than a renamed one.
Per-site map on the PermissionAdvancedFacets precedent. The four editors are keyed formula / visibleWhen / readonlyWhen / requiredWhen with identity-preserving writes and a summed memo. Four editors lint independently and asynchronously, so one shared counter would let whichever reported last overwrite the others — fixing one of two broken rules would hand back a writable Save.
Both hosts wired in this PR, reusing perm.cel.saveBlocked verbatim ("Fix the CEL syntax errors before saving."). No i18n edit was needed: the key already exists in both maps (en 1148, zh 2935) and both hosts already import t from metadata-admin/i18n. Its perm. prefix is now a misnomer — recorded as a naming smell, not renamed here.
Sub-decision A — host-owned reset.

One declared deviation on mechanism, not behavior

Both the stale-verdict prune and the host reset are done by derivation rather than reset effects. The map is stamped with the field it describes; each host stamps its count with the selection it came from. Behavior is exactly what the ruling specifies — the count expires on selection change and on unmount, and the formula verdict cannot outlive its editor — but it is stronger in two ways: an effect-based reset leaves a one-render window in which Save is still gated by an editor that is already gone, and the effect version added 4 react-hooks/set-state-in-effect warnings over baseline. The derived version adds zero warnings.

Beyond the ruling: all three save doors

The metadata editor has three ways to save, and gating only the button would have left two open — the autosave timer would have written the malformed definition a second later, and so would the keyboard shortcut. The button, the autosave effect and ⌘S are all gated.

Red-first evidence

Predicted in writing before running: inspector suite all red (reporter never called); host suite all red at its first toBeDisabled, with the valid-formula half passing even unfixed as the must-not-change pin.

Measured against unfixed sources — 11 failed / 11:

8 inspector cases: AssertionError: expected undefined to be 1 (and to be +0) — nothing was ever reported.
3 host cases: expect(element).toBeDisabled() → "Received element is not disabled" on the Save draft button. That is the card's symptom reproduced as a test.

Reverse verification after the fix, by patch-file revert of the four source files (never git stash — shared stack) and re-run: 11 failed again, split 8 + 3 on the same two signatures; restored with git apply and sha256 verified byte-identical on all four files.

Pins (must-not-change)

A valid formula keeps Save enabled — asserted before the fault in the host test, and green both before and after the fix.
Each of the three conditional-rule editors gates independently, one case each.
The decisive per-site case: two sites faulty, fix one, the other still holds Save closed (1 to 2 to 1). A shared counter passes every single-site case and fails this one.
Clearing the fault re-enables Save.
The RLS editor's own gate is untouched — PermissionAdvancedFacets.tsx, PermissionMatrixEditor.tsx and permission-slice.ts were read-only references ([security] An RLS policy authored in Studio's package door is silently discarded — Save succeeds, no row filter is ever persisted #4302 owns that surface), and their suites stay green.

Verification

New suites: 11/11 green. Regression: metadata-admin + studio-design, 168 files / 1671 tests passed, 1 skipped.
Both tsc passes for app-shell (tsc --noEmit, tsc -p tsconfig.test.json) — clean.
ESLint on all six changed files: 0 errors, and per-file warnings exactly at the origin/main baseline (58/12/15/0; both new test files 0).
check-control-bytes, check-changeset-presence, check-changeset-no-major, check-changeset-fixed, check-phantom-dependencies — all green.
.d.ts measured both ways with clean builds (dist + tsbuildinfo removed between): exactly two files differ — the added optional prop in inspector-registry.d.ts and the destructured parameter list in ObjectFieldInspector.d.ts. No other API movement. Graded minor per the entry-reachable-additive precedent, since MetadataInspectorProps is re-exported from the package entry.

Generated by Claude Code

… on CEL errors (#4306) A formula that fails to parse showed its inline error and saved anyway: `record.est_hours *` left "Save draft" enabled, PUT returned 200 with a success toast, and publishing made the malformed expression the live field definition. The RLS editor refused the same input, so one console disagreed with itself. `CelPredicateField` always reported findings through `onLintChange`, but only `PermissionAdvancedFacets` listened, and both ends of that channel live inside the permission editor which owns its own Save button. The field inspector is a registry component whose Save belongs to a host, reached only through `MetadataInspectorProps` — which had no way to say "what I am showing is not saveable". So there was no channel to wire, and adding one is the fix. - `MetadataInspectorProps` gains an optional `onBlockingIssuesChange(count)`, named for blocking issues rather than CEL so the five other inspectors with the same gap (#4527) wire against this contract instead of a renamed one. - `ObjectFieldInspector` aggregates its four CEL editors into a per-site map, not one running total: four editors lint independently, so a shared counter would let fixing one of two broken rules hand back a writable Save. - Stale verdicts are settled by derivation, not repair — the map is stamped with the field it describes, and the formula site counts only while the field is a formula, so no render window exists in which Save is gated by an editor already gone. - Both hosts hold the count, disable their own Save with the message the RLS editor already shows, and stamp it with the selection so it expires on selection change and unmount. In the metadata editor all three save doors are gated — button, autosave timer and ⌘S. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3

vercel · 2026-08-13T05:52:57Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectui	Ignored		Aug 13, 2026 5:52am

github-actions · 2026-08-13T05:54:12Z

✅ Console Performance Budget

Metric	Value	Budget
Main entry (gzip)	24.7 KB	350 KB
Entry file	`index-DT2H3BDq.js`	—
Status	PASS	—

📦 Bundle Size Report

Package	Size	Gzipped
app-shell (index.js)	9.56KB	3.59KB
app-shell (runtime-config.js)	7.42KB	2.32KB
app-shell (types.js)	0.01KB	0.04KB
app-shell (urlParams.js)	8.92KB	3.41KB
auth (AuthContext.js)	0.31KB	0.24KB
auth (AuthGuard.js)	1.17KB	0.53KB
auth (AuthProvider.js)	25.13KB	5.40KB
auth (AuthShell.js)	3.49KB	1.40KB
auth (ForgotPasswordForm.js)	12.21KB	3.45KB
auth (LoginForm.js)	18.13KB	5.39KB
auth (PreviewBanner.js)	0.90KB	0.50KB
auth (RegisterForm.js)	6.64KB	2.21KB
auth (SocialSignInButtons.js)	9.60KB	3.89KB
auth (UserMenu.js)	3.40KB	1.22KB
auth (auth-gate-events.js)	1.29KB	0.66KB
auth (authStyles.js)	5.04KB	1.72KB
auth (createAuthClient.js)	38.46KB	10.17KB
auth (createAuthenticatedFetch.js)	6.34KB	2.43KB
auth (index.js)	2.35KB	1.07KB
auth (org-roles.js)	6.66KB	2.78KB
auth (phone-identifier.js)	1.11KB	0.66KB
auth (types.js)	0.59KB	0.35KB
auth (useAuth.js)	5.02KB	0.88KB
auth (useIsWorkspaceAdmin.js)	1.61KB	0.85KB
collaboration (CommentThread.js)	26.07KB	7.56KB
collaboration (LiveCursors.js)	3.17KB	1.27KB
collaboration (PresenceAvatars.js)	6.49KB	2.64KB
collaboration (PresenceProvider.js)	2.79KB	1.13KB
collaboration (index.js)	1.65KB	0.73KB
collaboration (useCollaborationTranslation.js)	6.05KB	2.52KB
collaboration (useCommentSearch.js)	1.98KB	0.88KB
collaboration (useConflictResolution.js)	7.75KB	1.86KB
collaboration (useMentionNotifications.js)	1.81KB	0.68KB
collaboration (usePresence.js)	6.33KB	1.84KB
collaboration (useRealtimeSubscription.js)	7.91KB	2.01KB
components (index.js)	489.32KB	108.45KB
core (index.js)	3.37KB	1.34KB
create-plugin (index.js)	10.08KB	3.26KB
data-objectstack (index.js)	156.23KB	42.29KB
fields (index.js)	230.14KB	57.12KB
i18n (LocalizationContext.js)	1.76KB	0.96KB
i18n (currency.js)	1.22KB	0.64KB
i18n (i18n.js)	4.32KB	1.77KB
i18n (index.js)	3.35KB	1.38KB
i18n (pickLocalized.js)	3.69KB	1.73KB
i18n (provider.js)	23.12KB	7.62KB
i18n (useDisplayLocale.js)	2.84KB	1.45KB
i18n (useObjectLabel.js)	27.59KB	6.63KB
i18n (useSafeTranslation.js)	7.77KB	3.13KB
layout (index.js)	38.98KB	10.85KB
mobile (MobileProvider.js)	0.92KB	0.49KB
mobile (ResponsiveContainer.js)	0.94KB	0.38KB
mobile (breakpoints.js)	1.51KB	0.70KB
mobile (createOfflineDataSource.js)	5.61KB	1.74KB
mobile (index.js)	1.50KB	0.62KB
mobile (offlineQueue.js)	3.91KB	1.35KB
mobile (pwa.js)	0.97KB	0.49KB
mobile (serviceWorker.js)	1.48KB	0.62KB
mobile (serviceWorkerSource.js)	3.41KB	1.48KB
mobile (useBreakpoint.js)	1.54KB	0.65KB
mobile (useGesture.js)	6.96KB	1.98KB
mobile (useOfflineSync.js)	1.99KB	0.72KB
mobile (usePullToRefresh.js)	2.53KB	0.85KB
mobile (useResponsive.js)	0.71KB	0.42KB
mobile (useResponsiveConfig.js)	1.36KB	0.63KB
mobile (useSpecGesture.js)	4.32KB	1.64KB
mobile (useTouchTarget.js)	1.01KB	0.54KB
permissions (MePermissionsProvider.js)	8.75KB	3.06KB
permissions (PermissionContext.js)	0.31KB	0.25KB
permissions (PermissionGuard.js)	0.89KB	0.45KB
permissions (PermissionProvider.js)	3.67KB	1.12KB
permissions (evaluator.js)	4.41KB	1.44KB
permissions (index.js)	0.91KB	0.41KB
permissions (store.js)	0.91KB	0.42KB
permissions (useFieldPermissions.js)	1.28KB	0.52KB
permissions (usePermissions.js)	1.55KB	0.71KB
plugin-ai (index.js)	15.75KB	3.80KB
plugin-calendar (index.js)	46.86KB	12.91KB
plugin-charts (index.js)	62.07KB	17.65KB
plugin-chatbot (index.js)	181.21KB	43.14KB
plugin-dashboard (index.js)	120.95KB	31.53KB
plugin-designer (index.js)	212.58KB	42.83KB
plugin-detail (index.js)	239.88KB	59.99KB
plugin-editor (index.js)	2.46KB	1.10KB
plugin-form (index.js)	114.58KB	27.68KB
plugin-gantt (index.js)	164.14KB	39.98KB
plugin-grid (index.js)	189.28KB	50.29KB
plugin-kanban (index.js)	48.62KB	13.42KB
plugin-list (index.js)	111.13KB	27.12KB
plugin-map (index.js)	18.16KB	5.81KB
plugin-markdown (index.js)	13.72KB	4.69KB
plugin-report (index.js)	41.16KB	10.96KB
plugin-timeline (index.js)	26.25KB	7.53KB
plugin-tree (index.js)	8.50KB	2.88KB
plugin-view (index.js)	84.08KB	20.55KB
providers (DataSourceProvider.js)	0.75KB	0.39KB
providers (MetadataProvider.js)	1.37KB	0.59KB
providers (ThemeProvider.js)	1.90KB	0.85KB
providers (UploadProvider.js)	11.71KB	3.53KB
providers (index.js)	0.44KB	0.22KB
providers (types.js)	0.01KB	0.04KB
react-runtime (index.js)	5.67KB	2.37KB
react (LazyPluginLoader.js)	3.77KB	1.33KB
react (SchemaRenderer.js)	23.73KB	7.96KB
react (data-invalidation.js)	5.05KB	2.08KB
react (index.js)	1.23KB	0.66KB
react (spec-input.js)	0.20KB	0.18KB
sdui-parser (codegen.js)	4.09KB	1.74KB
sdui-parser (index.js)	4.47KB	2.03KB
sdui-parser (parse.js)	10.04KB	2.82KB
sdui-parser (types.js)	0.29KB	0.24KB
sdui-parser (validate.js)	4.69KB	1.48KB
types (ai.js)	0.20KB	0.17KB
types (api-types.js)	0.20KB	0.18KB
types (app.js)	2.87KB	0.99KB
types (base.js)	0.20KB	0.18KB
types (blocks.js)	0.20KB	0.18KB
types (complex.js)	0.20KB	0.18KB
types (crud.js)	0.20KB	0.18KB
types (dashboard-filter-alias.js)	6.23KB	2.74KB
types (data-display.js)	0.20KB	0.18KB
types (data-protocol.js)	0.20KB	0.19KB
types (data.js)	0.20KB	0.18KB
types (designer.js)	1.87KB	0.85KB
types (disclosure.js)	0.20KB	0.18KB
types (error-code.js)	1.54KB	0.88KB
types (feedback.js)	0.20KB	0.18KB
types (field-types.js)	0.20KB	0.18KB
types (form.js)	0.20KB	0.18KB
types (http-retry.js)	4.32KB	2.02KB
types (index.js)	3.05KB	1.52KB
types (layout.js)	0.20KB	0.18KB
types (managed-by.js)	0.19KB	0.18KB
types (mobile.js)	2.59KB	1.31KB
types (navigation.js)	0.20KB	0.18KB
types (objectql.js)	0.20KB	0.18KB
types (overlay.js)	0.20KB	0.18KB
types (permissions.js)	0.20KB	0.18KB
types (plugin-scope.js)	0.20KB	0.18KB
types (record-components.js)	0.20KB	0.19KB
types (record-semantics.js)	1.28KB	0.67KB
types (registry.js)	0.20KB	0.18KB
types (reports.js)	0.20KB	0.18KB
types (spec-report.js)	5.05KB	1.93KB
types (system-fields.js)	3.33KB	1.54KB
types (theme.js)	0.20KB	0.18KB
types (ui-action.js)	3.40KB	1.71KB
types (views.js)	0.20KB	0.18KB
types (widget.js)	0.20KB	0.18KB

Size Limits

✅ Core packages should be < 50KB gzipped
✅ Component packages should be < 100KB gzipped
⚠️ Plugin packages should be < 150KB gzipped

yinlianghui · 2026-08-13T06:05:17Z

PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)

The escalate-then-implement sequence worked exactly as designed: the card's false root-cause sentence was refuted by measurement BEFORE any code, the Option A ruling was issued on the corrected picture, and this PR implements that ruling — a typed, generically-named optional reporter on MetadataInspectorProps, per-site aggregation on the measured PermissionAdvancedFacets precedent, both hosts gated.
The decisive two-site test is the review's favorite artifact: visibleWhen + readonlyWhen faulty → 2, fix one → 1 (NOT 0). A shared counter passes every single-site case and fails exactly there — the test proves the map is load-bearing.
Both declared deviations are accepted as strictly-better mechanism with behavior as ruled: (1) derivation-with-stamps instead of reset effects — no one-render gated-by-a-ghost window, zero eslint delta (and the +4 of the effect version was MEASURED before the refactor, which is the right way to make this call); (2) ResourceEditPage's THREE save doors all gated — the autosave timer would have written the malformed definition a second later, so gating only the button would have recreated the divergence Option C was rejected for.
Red-first 11/11 on the predicted split, host reds are the card's literal Save-draft symptom, reverse verification run twice with sha256-verified restores. Zero pins moved; 168-file regression green; RLS editor and [security] An RLS policy authored in Studio's package door is silently discarded — Save succeeds, no row filter is ever persisted #4302's surface untouched (read-only), as contracted.
.d.ts: exactly the one-prop contract growth, entry-reachable → MINOR as ruled. CI 20/20 green on per-job conclusions.
The process failure (detached pacing timers, turn ended with no wake source) was caught, owned, root-caused and corrected in one round — recorded here so the pattern stays banned: the background-timer affordance is not a substitute for foreground CI polling.
CEL blocking errors are dropped on the floor in ConditionBuilder and ConditionalFormattingEditor too — same ungated-Save family as #4306 #4527 (the five remaining inspectors through ConditionBuilder / ConditionalFormattingEditor) is now unblocked by this contract and will be dispatched next.

Auto-merge armed (squash).

Generated by Claude Code

…objectstack-ai#4527) (objectstack-ai#4547) CelPredicateField has always reported its lint verdict through onLintChange, but ConditionBuilder and ConditionalFormattingEditor passed no listener, so a predicate that does not parse rendered its inline error and Save still saved -- the objectui#4306 defect, one editor family over. Both editors gain an optional callback surfacing their blocking-error count (severity 'error' only), and the inspectors above them aggregate and report through MetadataInspectorProps.onBlockingIssuesChange, the channel PR objectstack-ai#4536 shipped. PageBlockInspector gates on a page block's visibleWhen; ViewVariantInspector gates on a view's conditionalFormatting rules, forwarded through ViewInspector on the scoped path. Counts are DERIVED from what they describe, never repaired by reset effects. ConditionBuilder's CEL editor exists only in raw mode and can vanish while its last verdict was an error -- the adopt effect flips back to rows in the same commit, unmounting it and cancelling its pending lint -- so the count is read as 0 whenever that editor is not mounted. ConditionalFormattingEditor keys a per-rule map and counts only rules that still exist: a shared counter would let whichever rule linted last overwrite the others, and a deleted rule's remembered error would wedge Save shut with no editor left to fix it in. Three sites named in the report are NOT wired: HookDefaultInspector, ActionDefaultInspector and the view's home panel are MetadataDefaultInspectorProps components whose contract has no blocking-issues member, and widgets.tsx's condition widget is a SchemaForm widget rather than an inspector. Wiring those needs a second contract decision plus edits to the hosts, so they are escalated rather than guessed. Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-authored-by: Claude <noreply@anthropic.com>

github-actions Bot added the tests label Aug 13, 2026

yinlianghui marked this pull request as ready for review August 13, 2026 06:05

yinlianghui added this pull request to the merge queue Aug 13, 2026

Merged via the queue into main with commit 553099c Aug 13, 2026
21 checks passed

yinlianghui deleted the claude/issue-4306-field-inspector-cel-gate branch August 13, 2026 06:05

Navigation Menu

Uh oh!

fix(app-shell): inspectors can block Save — the field inspector gates on CEL errors (#4306) - #4536

fix(app-shell): inspectors can block Save — the field inspector gates on CEL errors (#4306)#4536
yinlianghui merged 1 commit into
mainfrom
claude/issue-4306-field-inspector-cel-gate

yinlianghui commented Aug 13, 2026

Uh oh!

vercel Bot commented Aug 13, 2026

Uh oh!

github-actions Bot commented Aug 13, 2026

Uh oh!

yinlianghui commented Aug 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

yinlianghui commented Aug 13, 2026

Why this needed a contract, not a one-file fix

Ruling compliance

One declared deviation on mechanism, not behavior

Beyond the ruling: all three save doors

Red-first evidence

Pins (must-not-change)

Verification

Uh oh!

vercel Bot commented Aug 13, 2026

Uh oh!

github-actions Bot commented Aug 13, 2026

✅ Console Performance Budget

📦 Bundle Size Report

Size Limits

Uh oh!

yinlianghui commented Aug 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants