fix(app-shell): the default-inspector family and its panel hosts gate Save on CEL errors (#4527) - #4558
Merged
Conversation
… Save on CEL errors (#4527) There are TWO inspector registries and #4306 extended only one. MetadataInspectorProps carried onBlockingIssuesChange; MetadataDefaultInspectorProps did not, so every "no selection" inspector rendered its CEL parse error while Save stayed writable -- and no host could pass a callback that did not exist. MetadataDefaultInspectorProps now carries the same optional member. HookDefaultInspector reports its guard; ActionDefaultInspector aggregates its two predicate editors through a per-site map (two editors lint independently, so a shared counter would hand back a writable Save the moment one of two broken predicates was fixed); the view home panel already aggregated and now has a contract to report through. Hosts hold and expire the counts. The metadata editor gates its no-selection branch as well as its scoped one, each stamped so neither reads the other's verdict. Studio's design pillar gates its rail -- an unfinished edge of #4306, since the same malformed-CEL publish was reachable there with the gate inert. The Data pillar gains a second count for its panel family (validations / actions / settings write through the object draft and own no Save), stamped with the panel tab because only one panel is mounted at a time. The hooks panel writes on its own and gates its own per-hook Save. Every count is DERIVED from what it describes and pruned by what still exists: a deleted rule or action drops out immediately, so a fault cannot wedge Save shut with no editor left to fix it in. A faulty rule merely navigated away from stays counted -- it is still in the document and saving would still publish it. Also wired ObjectValidationsPanel, a sixth ConditionBuilder consumer the original report did not list. widgets.tsx's ConditionWidget stays deferred by ruling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3) — and with this, the CEL-gate family (#4306 → #4547 → here) is closed except the explicitly-deferred ConditionWidget.
Auto-merge armed (squash) — landing verified per the merge-queue discipline. Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 13, 2026 09:02
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4527.
Phase 2, following PR #4547 and the phase-2 ruling.
Fixesthis time: every site the ruling names is wired.widgets.tsx's ConditionWidget remains explicitly deferred by that ruling and does not block closure.The structural cause
There are two inspector registries and #4306 extended only one.
MetadataInspectorPropscarriedonBlockingIssuesChange;MetadataDefaultInspectorProps— the contract every "no selection" inspector renders through — had no such member. So the whole default family showed its CEL parse errors and saved anyway, and no host could pass a callback that did not exist. That contract now carries the same optional member.HookDefaultInspectorActionDefaultInspectorvisible/disabledViewVariantInspectorhome pathObjectValidationsPanelHosts wired: metadata editor's no-selection branch, Studio design pillar (both its scoped and default rail renders), Data pillar's panel family, and the hooks panel's own button.
Two measured corrections to the ruling's model
Both changed what the work actually is, so they are stated rather than smoothed over.
1. Three of the four panel hosts do not own a Save. Only
ObjectHooksPanelwrites on its own (client.save('hook', …)).ObjectActionsPanel,ObjectValidationsPanelandObjectSettingsPanelwrite through the object draft — their own headers say so — and all three are rendered insideDataPillar. So they report upward and the pillar holds a second count beside the field-inspector count #4306 gave it, stamped with the panel tab: exactly one panel is mounted at a time, so a tab the author has left can never retract its verdict.2.
ObjectSettingsPanelis inert today, and is wired anyway — declared, not hidden. It rendersObjectDefaultInspector, which mounts no CEL editor at all, so nothing downstream can ever call the callback. It is wired to keep the host family uniform (the next CEL editor added there is gated by construction), and only its forwarding is covered — there is no verdict to produce. Flagged so nobody reads it as tested behaviour.A third, smaller one: the metadata editor reaches a default inspector only for types that also have a canvas preview, because that panel lives inside the
PreviewComponentbranch; a type without one falls through to a plainSchemaForm.viewis the type where that branch actually hosts CEL, which is why the host test uses it — and whyhook/actiondefault inspectors are not reachable from that host at all, but from the Studio panels, where they are gated in their own suites.Red-first
Predicted the split in writing before running. Predicted signature, inherited from #4306/#4547: the reporter is never called, so the count a host would hold is
undefinedand every asserted count fails, the0cases included. Measured against unfixed code, verbatim:One prediction was wrong, and informatively so. I predicted the view home path would go red at runtime. It did not:
ViewDefaultInspectorspreads{...props}intoViewVariantInspector, so once #4547 taught the variant inspector to aggregate, the callback already reached it. The home path's defect was purely the missing type — no host could pass the prop without a compile error, and none did. Those two render cases are therefore reported as pins, not red signals, and the actually-red assertion for that path is a type-level one that fails to compile until the contract carries the channel.Reverse verification removed the fix with
git diff+git checkout --(nevergit stash— objectui#3430) and re-ran all seven suites: 6 files failed, 20 failed / 2 passed — the 2 passes being exactly the two spread-carried home-path cases above. Restored withgit applyand sha256 verified on all 9 files.Verification
tsc --noEmitandtsc -p tsconfig.test.json).origin/maincontent, 0 errors. New test files lint clean.check-control-bytesOK; plus an explicitgrep -naPself-scan of every changed file including the untracked ones (the gate only scans tracked files) — zero control bytes.check:phantom-depsOK, changeset gates OK.Changeset
@object-ui/app-shell: patch, by.d.tsmeasurement both ways withdist/andtsconfig.tsbuildinfocleared between builds.dist/index.d.tsis byte-identical before and after. The ruling forecast minor on the assumption thatMetadataDefaultInspectorPropsis entry-reachable like its sibling; measurement says it is not — the package entry re-exportsMetadataInspectorPropsfrominspector-registryand nothing at all fromdefault-inspector-registry, and the package declares no subpath exports. That asymmetry is also why #4536 graded minor and this does not.Surface
inspector-registry.tsis not touched: the type extended here lives in the siblingdefault-inspector-registry.ts, so the conditional authorization for that file was not needed or used.ObjectFieldInspector.tsx, the permission editors, plugin packages andcontent/docs/releases/all untouched.ResourceEditPage.tsxandStudioDesignSurface.tsxwere edited under the lifted phase-1 restriction; noclient.get(call site was modified.Generated by Claude Code