Uh oh!
There was an error while loading. Please reload this page.
feat(data-objectstack,app-shell,i18n): view config is explicitly org-wide — permission-gated write path + console wording - #6125
Merged
Conversation
…write path objectstack#7494's ruling (maintainer, 2026-08-12): sort / hiddenFields / columnState / rowHeight are ORG-WIDE view configuration, not per-user preferences. Gate `updateViewConfig` on the `manage_metadata` capability the repo already uses for metadata authoring, refuse before anything reaches the wire, surface the refusal instead of swallowing it, and say the scope in the View settings popover before the operator acts. Unknown capabilities fail OPEN (ADR-0066 doctrine, objectui#4656); a reported empty grant gates strictly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
`viewConfigPermissionDenied` adds one key to a live namespace and one correctly paired German „…“ span. The invariants themselves are untouched: de still shows open === close with rdq at 0, and none of the 116 retired objectView keys came back. Only the counts move, each with its provenance recorded inline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
Closes the loop from both ends: the real adapter's refusal is classified by the real guard the catch branches on, and the key that branch renders exists in the en pack. Also pins that the generic console.error SURVIVES for every other failure — the branch is an addition, not a replacement. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
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
|
yinlianghui
marked this pull request as ready for review
August 25, 2026 01:36
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 25, 2026
Merged
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
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#5232
Implements objectstack#7494's ruling (maintainer huangyiirene, 2026-08-12), verbatim:
All gate results below are from
c4f5dbf62, the branch head.The premise, re-derived — and one half of it was already done
The card pointed at
ObjectView.tsx ~1455for prose calling these "persisted user preferences … per-view personal config". That prose no longer exists, in eitherObjectView.tsx. A repo-wide search for the card's own quoted phrases returns three source matches, all code comments, in exactly the three files at issue — and all three already say the corrected thing, citing this same ruling ("ORG-WIDE shared, not a per-user preference"). The row-classification half left them that way.What was not done is the part the ruling actually asks for: no user-visible string anywhere told an operator that these settings apply to everyone. Correct comments are not UX. So the wording work here is on the rendered strings, not the docblocks.
Census method, stated because the card admitted its own was not exhaustive ("and any sibling prose"): (a)
git grepfor each quoted phrase across all tracked files; (b) grep the four ruled property names in label/tooltip/i18n contexts; (c) trace everypersistViewPatchcall site backwards to the component that emits it, then enumerate the i18n keys those components render. (c) is what found the real site —ViewSettingsPopover, the popover that actually hosts density and field visibility, whose header hint is the one sentence an operator reads before acting.The gate is on the write, not on the button
ObjectStackAdapter.updateViewConfignow refuses when the session's reported capability set lacksmanage_metadata, throwingViewConfigPermissionDeniedError. The check is the first statement in the method — beforeconnect(), before the payload is assembled — so a refused call puts nothing on the wire.Deliberately not the toolbar affordance: withholding the button leaves the method still accepting the call from anything else holding the adapter, whereas a gate on the write is inherited by every caller, present and future. No affordance was hidden or disabled in this PR.
Which permission, and why it was not minted. The ruling names a class ("视图设计类权限"), not a permission.
manage_metadatais what this repo already treats as metadata-authoring authority: it isHomePage'sAUTHORING_CAPABILITY, it is whatPackageFormDialogreads a server 403 for, andcapability.label.manage_metadatais already translated in all ten packs. Decisively — the gated write goes throughclient.meta.saveItem, the very same ADR-0005 metadata door the server already refuses without it. This applies the authority the server is already applying rather than inventing a parallel one.Unknown fails OPEN, and that is a judgement worth reading twice. It is the doctrine
PermissionContextValue.hasCapabilitiesstates for every ADR-0066 gate here (objectui#4656, framework#3923): the server enforces regardless, so a client-side denial on missing data cannot protect anything the server was not already protecting — it can only break a permitted user. Failing closed would have refused the write on every deployment predating ADR-0066 and every host with no permission provider, which is "break the write for everyone" wearing the costume of a security fix. A reported empty grant gates strictly; the two are never collapsed, which is the one distinctionMePermissionsProvidergoes out of its way to preserve.What an unpermitted caller gets — quoted
Not a silent no-op. The thrown error's message, verbatim:
And on screen,
persistViewPatch's catch previously swallowed every failure intoconsole.error. For a debounced toggle whose UI has already moved that would leave the operator looking at a density they did not get, discovering it on the next reload. A denied write now raises a toast:The generic
console.errordeliberately survives for every other failure — the branch is an addition, not a replacement, and that survival is pinned.Wording
list.viewSettingsHintnow reads "Grouping, color, density, and visible fields. Applies to everyone who uses this view." — in all ten packs, plus both inlinedefaultValuemirrors (ViewSettingsPopoverandListView's provider-less defaults map, which a test holds byte-identical to theenpack).Scoped deliberately to the popover header rather than sprayed across every tooltip: that header is where an operator reads a description before acting, and one accurate sentence there beats the same clause repeated on four controls.
Verification
Ablation — the gate deleted, restored under
trap … EXIT INT TERMwith a cwd-independentgit -C. My own edit was committed first, so the restore could not take it with the mutation. Mutation proven on disk by grepping the injected marker (1) and the removed text (0) separately, anchor uniqueness asserted before writing, landing site printed (index.ts:3873),git diff HEAD --statempty afterwards. No rebuild needed: the test imports./index, relative source.Predicted before running: the four refusal cells red, the two permit controls plus three pure-function cells green. Observed exactly that — 4 failed | 5 passed.
Which assertions would still pass on a revert, stated so nobody has to guess:
permits a session holding the capabilityandunreported capabilities fail OPENwould both still pass — they assert the write happens, which is what the code did before the gate. They are the controls, and they exist so "the gate works" cannot be satisfied by breaking the write for everyone; both directions run in the same file and the same run. Everything underrefusesfails on a revert. Every cell builds its ownmakeMetaStore()— a shared spy would let one case'ssaveItemsatisfy the next case's assertion.Gates (exit code captured before any pipe; script name echoed on each
type-checkso none was a zero-match no-op):pnpm --filter @object-ui/data-objectstack type-checkpnpm --filter @object-ui/app-shell type-checksystemPermissions/ guard import wrongpnpm --filter @object-ui/plugin-list type-checkpnpm --filter @object-ui/i18n type-checkpnpm exec vitest run packages/data-objectstack/src packages/i18n/src packages/plugin-list/srcTest Files 144 passed/Tests 2187 passedpnpm exec vitest run packages/app-shell/src packages/plugin-list/srcTest Files 566 passed/Tests 5732 passed | 1 skippedpnpm --filter '...@object-ui/data-objectstack' type-checkerror TSnpx eslint <merge-base delta>485 problems (0 errors, 485 warnings), all pre-existingpnpm check:i18n-keysenpnpm check:i18n-drift1 en value(s) changed …, 9 pack value(s) followednode scripts/check-changeset-presence.mjs14 source file(s) of 4 released package(s) changed … 1 changeset(s)node scripts/check-changeset-no-major.mjsmajorbumpnode scripts/check-changeset-fixed.mjsnode scripts/check-control-bytes.mjsscanned 5063 tracked text file(s)node scripts/check-type-check-coverage.mjspnpm check:phantom-depspnpm check:self-importpnpm check:esm-specifierspnpm check:spec-symbolsDownstream sweep direction, demonstrated rather than claimed:
'...@object-ui/data-objectstack'is the PREFIX form and resolves to 34 packages includingapp-shell,console,siteand everyplugin-*— the consumers a contract change lands on. The suffix form'@object-ui/data-objectstack...'resolves to onlycoreandtypes, the upstream deps. Zeroerror TSanywhere in the prefix run../examples/**was built alongside./packages/**, which is why noapps/examplesworkspace produced the unbuilt-closure red this sweep is known for.Gate set derived by enumerating each CI job's own step list rather than top-level script names.
Three ratchets moved, and why that is not a regression
Adding one key to a live namespace tripped three counts. The invariants are untouched and each count carries its provenance inline:
de-quote-pairing-3876— 52 → 53 correctly paired spans, and{ open: 53, close: 53, rdq: 0 }.rdqstaying at 0 is the load-bearing half: the new German value added a matched „…“ pair, not a stray closer that would have madeclose === opentrue for the wrong reason.objectView-config-keys-retired-4730— surviving keys 93 → 94, total 209 → 210. The retired half (116) does not move and is still pinned twice; splitting the two is the point, since folding a new key into the total would be indistinguishable from a retired key coming back.Scope
Per-user view config was not built — no storage, no fallback, no flag (parked upstream as objectstack#7611, v18).
persistViewPatchmerged-base freezing is a separate card and is untouched here. The row-classification half was already complete and is untouched. No sibling defects were found worth filing.Contended file — the HARD SERIAL condition, checked
Dispatch flagged
packages/data-objectstack/src/index.tsas contended by PR #6109 (per-column sortability), with the instruction to stop if the edit lands in or adjacent tofetchObjectSchemaFresh. It does not.updateViewConfigis a different method roughly 300 lines away, withgetObjects,getClient,getDiscovery,invalidateViewKeys,listViewOverridesandgetViewbetween them; my edits sit at its head and among the error classes near the top of the file. PR #6109 had not landed at branch time, so this branches fromorigin/mainate7957ab87— declared rather than assumed, since waiting on an unmerged sibling is not a terminal state this seat can reach.Re-measured after #6109 landed (it is now on
mainas490d9a93a):git merge-tree --write-tree origin/main HEADreturns exit 0, no conflict, against amainthat has also since taken #6117. The two changes touch the same file and do not overlap — which is what the dispatch's condition was asking, now answered by measurement rather than by reading line numbers. The branch is left un-merged so the gate results above keep pointing at the tree they were run on; the merge is the PM's to make.Generated by Claude Code