Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): gate object fields on the spec-declared hidden / visibleWhen keys, not the refused visible - #6657
Merged
Conversation
…keys Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
…o prior owners of isFieldVisible Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 28, 2026 10:12
This was referenced Aug 28, 2026
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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#6514
Maintainer ruling, 2026-08-27, Option A: the two app-shell call sites stop gating on the
spec-refused
fields[].visibleand read the contract's DECLARED keys instead — the statichidden(INVERTED polarity, per the spec's own guidance) and the predicatevisibleWhen— with the dead-key read deleted. Option B (adding field-level
visibleto the spec)was declined. Dispatch seat
8ca04858-ea8e-5b85-9182-de59aa49e00c.What changed
RecordFormPageandAppContent's global record-form modal both filtered an object'sfields with
evaluateVisibility(f.visible, expressionEvaluator).FieldSchemais astrictObjectandvisibleis not one of its keys — it is prose inFIELD_KEY_GUIDANCEthat REFUSES the spelling, deliberately not an alias, because "this surface declares BOTH
forms and the two answers have opposite polarity". The gate was therefore unreachable
through the authoring surface.
Both sites now call one shared helper,
isObjectFieldVisible(new, inproviders/ExpressionProvider.tsx— a module both already imported, so no new module edgeis created):
One helper rather than two edited expressions, deliberately: the polarity is the whole
risk of this card, and two copies of an inversion are two chances to get it backwards.
Composition is AND.
visibleWhenis documented as "shown only when TRUE (elsehidden)" — a necessary condition, never a licence to un-hide a statically hidden field.
This is the same shape
@object-ui/core'sresolveFieldRuleStatealready uses one tierdown, where the static and the predicate OR into the restrictive verdict
(
readonly || readonlyWhen,required || requiredWhen). Precedent, not invention.The census, re-run on today's ref, WITH a positive control
The zero-usage measurement was taken at
d09e13fd7over 104 files. Re-run at frameworkorigin/mainaef1b7e641e7cead90f35db7cd1e06c46a021944— 113*.object.*files now— with the control in the SAME query shape, so a zero is a measurement and not a broken
pattern:
visiblelabelrequiredreadonlysearchableThe controls prove the query reaches field level.
visible:does occur 22 times as a codekey across those files — every one of them under
actions, zero underfields— plus16 mentions in comments/prose. Zero nested below field level (per-option and similar) as
well. The finding holds on today's ref.
Also measured and worth stating:
hiddenandvisibleWhenare themselves at 0field-level occurrences in the framework's shipped objects. The declared keys have no
authored pull today either; what changes here is that they now WORK when authored, which
is what "the platform says what it means" costs.
Both-direction pins, per call site, red before the fix
visible->hiddenis an INVERSION, and an inversion read backwards raises no error. Itproduces a field that disappears with no diagnostic, or one that LEAKS to a user who must
not see it. So each key carries a hiding case AND its complement, at BOTH sites:
packages/app-shell/src/views/RecordFormPage.declaredVisibilityKeys.test.tsxpackages/app-shell/src/console/__tests__/AppContent.declaredVisibilityKeys.test.tsxLeg 1 — against the pre-fix code: 12 failed / 4 passed (16). The 12 red are the six
per site:
hidden: trueabsent, falsevisibleWhenabsent, the per-user predicate,the AND composition, and both dead-key cases. The 4 green are the complements
(
hidden: falsepresent, TRUEvisibleWhenpresent) — they cannot be red before the fix,because nothing hid them then.
Leg 2 — against a deliberately INVERTED implementation: 16 failed / 0 passed. This is
the leg that makes the complements load-bearing. Both polarities in
isFieldVisiblewereflipped (
=== trueto!== true, and the predicate negated) and the complements go redwith
expected [] to include 'plain_note'— inverted logic hid everything, exactly thefailure mode the card names. A suite asserting only the hiding half would have passed this
leg.
The mutation was confirmed on disk before the run, not inferred from an editor exit code:
2 injected markers present, 0 occurrences of the original guard remaining, and the blob
hash moved (
c0dc6e21toba43b49f). Restore proven the same way:git diff HEADempty,0 residual markers, and the file's blob hash back to
c0dc6e21, identical toHEAD:...ExpressionProvider.tsx. The mutation script carried atraprestore onEXIT INT TERMwith absolute paths. Nodistis involved — vitest resolves these packagesto
src— so there is no stale-build leg to report. Both legs were re-run at the finalcommit, so every number here is anchored to the same tree.
After the fix: 43 files / 404 tests passed. The suite selection is a DECLARED narrowing
and it was derived, not guessed: every
*.test.ts(x)inpackages/app-shellwhose subjectimports a changed symbol (
ExpressionProvider,evaluateVisibility,isObjectFieldVisible)or renders a changed component (
RecordFormPage,AppContent) — 43 of them, which coversboth new suites, both triaged suites, every
providers/suite including the fault-diagnosticpins, and the sidebar/palette wrappers of
evaluateVisibility. A full-package run wasstarted earlier and is deliberately NOT quoted: the tree was edited while it was in flight,
so it is not a clean measurement. CI runs the whole farm across its four shards regardless.
Fixture triage — two existing suites carried the dead key
These authored
visible: cel(...)on object fields because that was the key the page readwhen they were written. Only the CARRIER key moved; every predicate text, user fixture and
assertion is unchanged, and neither pin is weakened:
views/RecordFormPage.predicateScope.test.tsx(4 fixtures) — its subject is which ROOTSthe evaluator binds, which is independent of the key carrying the predicate.
providers/expressionUser.mountParity.test.tsx(3 fixtures) — its subject is the SHAPEof the identity a mount site publishes; likewise independent.
Both files gained a note saying why the spelling moved.
Also in the diff, and why
The helper is named
isObjectFieldVisible, not the shorterisFieldVisible, because thatshorter name is already twice taken by functions answering different questions —
views/metadata-admin/inspectors/flow-node-config.tsgates a flow node's CONFIG inputs onnode type, and
apps/console'sFormPageevaluates the VIEW-level field predicate. Onlythe new symbol was renamed; neither prior owner is touched.
evaluateVisibilitygained an optional third parameter,authoredKey, defaulting to'visible'. Strictly additive — every existing 2-argument caller is byte-for-byteunaffected — and it exists because this change would otherwise make the fault diagnostic
LIE: a
visibleWhenfault at a field site would have printedvisible, sending an authorto grep for a key
FieldSchemarefuses. The doc paragraph onAPP_SHELL_VISIBLE_SURFACEthat described nav and field faults sharing one dedupe entry was corrected in the same
stroke, since
keyis part of the dedupe tuple and they no longer do.Fenced boundaries
current_userbinding is added at field level. The ruling's sub-clause isbinding, and nothing here reaches it:
isFieldVisibleadds no roots, it handsvisibleWhento the evaluator the caller already built. The spec's documentedfault-open at this tier is unchanged.
objectui#6493 are exactly as they were.
views/metadata-admin/predicate.tswas never reached — that file is held by anotherdev (objectui#6617) and nothing here goes near it.
Confidence gaps the analysis declared
Both were probed rather than silently closed.
object FIELD's
.visiblewere these two call sites; everything else namedvisibleisActionDef.visible, action params, or a component key, all legal at their own tier.@object-ui/types'BaseFieldMetadatadeclares novisible, so a third-partyTypeScript plugin reading
field.visiblewould be reading an undeclared key.But a real hazard turned up next door: the framework's published docs TEACH a
field-level
visiblein three mutually incompatible shapes, none spec-valid(
concept.mdxsection 3,layout-dsl.mdx"Device-Specific Visibility"). Recorded asobjectstack#12935 — out of scope here, docs-only, and in the other repo.
visibledata in customer deployments, written past schema validation. Notmeasurable from this container; no customer data is reachable. What objectstack#12935
adds is the most plausible SOURCE of any such data, which is the honest partial answer.
Verification
--filter="...@object-ui/app-shell", the prefix form):39/39 turbo tasks successful, covering
@object-ui/console,@object-ui/example-byo-backend-consoleand@object-ui/example-console-starteras wellas app-shell's own build. All four touched test files were confirmed present in the
tsconfig.test.jsonprogram via--listFiles, so "typecheck is clean" genuinely coversthem.
npx vitest runplus explicit file paths), never the package-scoped form.check:control-bytesOK (5497 files) ·check:vi-mock-specifiersOK ·check:spec-symbolsOK ·
check:designer-field-key-parityOK ·check:self-importOK ·check:phantom-depsOK ·
check:i18n-keysOK ·check:esm-specifiersOK ·check:readme-exportsOK ·check-changeset-fixedOK ·check-changeset-no-majorOK.check:eager-closureis NOT MEASURED locally, not red: it needsapps/console/dist/eager-closure.jsonfrom a consolevite buildand says so itself("a broken gauge, not a passing budget"). CI builds it. The diff adds no module edge at
all — both call sites already imported this module — so the eager closure is
structurally unchanged.
eslint .inpackages/app-shell, the package containingevery changed source file. Population read from eslint's own config; count read from
--format json: 1008 files, 0 errors, all 7 touched files present in the report byname. Invariance for untouched files:
eslint.config.jsdeclares noprojectService/parserOptions.project, so type-aware linting is off and this diff cannot move theverdict on a file it does not touch. CI runs the repo-wide farm regardless.
Gate union, lint, type-check, both pin legs and the ablation were all run at
HEAD=f8d36099.Generated by Claude Code