Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): the permission matrix honors allowRuntimeCreate — and the read-only banner tells the truth (#4446) - #4519
Merged
Conversation
… the read-only banner tells the truth (#4446) The writability switch read `allowOrgOverride` alone, so a writable package rendered read-only while the server accepted the package-door write. It now reads the disjunction `allowOrgOverride || allowRuntimeCreate` off the raw server entry — the repo's own convention and the server's own predicate. The package-level `readOnly` gate is untouched and still dominant. 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
|
yinlianghui
marked this pull request as ready for review
August 13, 2026 03:37
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes#4446
Phase 1 — pin provenance: DESCRIPTION, not decision
The card warned that two suites pin the current behaviour on purpose and that a decision there would make this a ruling request rather than a patch. Traced both, plus the upstream thread. Verdict: they describe the code, and the one on-record ruling in the area points the same way this PR goes.
PermissionMatrixEditor.readonly.test.tsx:153— introduced by PR #2570 (57a61b097, 2026-07-16). That PR's own change was purely additive: it added the host gate (&& !readOnly) so the Studio Access pillar could lock a read-only package. It inheritedwritable = allowOrgOverrideand described it — "内部writable = allowOrgOverride是 type 级(环境 OS_METADATA_WRITABLE)概念" — which is precisely the mis-attribution this card is about. Its case 4 pins that the two gates keep separate wording ("两个闸门语义分离"), not that the type gate must be strict. No ruling.PermissionMatrixEditor.readonlyHeaderBadge.test.tsx:17-19— introduced by PR #4341 (f046f885a), the B-half of objectstack#5768 / #4036. Its header does carry decision language ("a fix that made anything editable would be the wrong fix"), but it is scoped to the ruling it quotes —Studio 维持包级只读, the package-level gate. Measured: every case in that suite drives the host gate (readOnlytrue/absent) withallowOrgOverride: truethroughout. It contains noallowOrgOverride: falsecase, so it never renders the shape it describes in prose and does not pin the type-gate formula.The upstream ruling. objectstack#5768's 裁决 of 2026-08-06 (maintainer-authorized, quoted verbatim and untranslated):
That server-side tightening has since landed — objectstack#6483, 2026-08-08 maintainer ruling — and it deliberately closed one door while keeping the other open. From the
permissionentry's own rationale inpackages/spec/src/kernel/metadata-plugin.zod.ts:So there is no ruling that
permissionmust gate stricter than the server. The ruling closed the overlay door and preserved the runtime-create door; this card asks the editor to consult exactly the door that was preserved. A1 (opening overlay editing) stays untouched and still deferred.Phase 2 — the fix
1. The switch reads the disjunction.
allowOrgOverrideis permission to OVERLAY a code-shipped item per org;allowRuntimeCreateis permission to AUTHOR at runtime — and authoring is what this editor's Save does under apackageId(mode: 'draft'+packageId, ADR-0086 P0/P2,PermissionMatrixEditor.tsx:567). The server's own gate is that same disjunction, refusing only when both are false (!isOverlayAllowed && !isRuntimeCreateAllowed, insaveMetaItemandpromoteDraftForPublish).permissionsits exactly in the gap:allowOrgOverride: false,allowRuntimeCreate: true.Read off the raw server entry, as
DirectoryPage:171/175,EmbeddedItemEditor:93andResourceEditPage:1332all do, and asuseMetadata.tsdocuments on the field itself ("UI affordances … should activate when either flag is true"). Note the card's suggested spellingresolved.allowRuntimeCreatecould not have worked:resolveResourceConfigforwardsallowOrgOverrideonly, so that read would have been silentlyundefined.resolveResourceConfigis now unused here and its import is dropped.The host gate is unchanged and still dominant, so
Studio 维持包级只读holds exactly as before.2. The badge/controls divergence, resolved rather than half-migrated.
PageShell'sWritabilityBadgeis read-only exactly whenreadOnly || (!allowOrgOverride && !allowRuntimeCreate); the controls were read-only whenreadOnly || !allowOrgOverride. The gap is precisely!allowOrgOverride && allowRuntimeCreate— today'spermission, where the screen showed a "create-only" badge above 207 dead checkboxes. The badge already consulted the full disjunction; the controls were the outlier. Fixing the controls makes the two predicates byte-identical, which a new four-state table pins so neither side can drift again.PageShellitself is untouched — passing!writableinto it would have collapsed the inputs but made a TYPE-gate lock claim the PACKAGE as its reason (that branch's tooltip isengine.studio.pkg.readonlyHint), trading the divergence for a fresh lie.3. Banner honesty, and the measurement behind it. The old caption blamed a deployment env var for a per-type registry declaration — and it had no reachable honest case at all.
OS_METADATA_WRITABLEdoes not sit besideallowOrgOverride, it flips it:getMetaTypesemitsallowOrgOverride: base.allowOrgOverride || isEnvOverridden. So whenever the hatch is on for a type, that type is writable and this badge never renders. There is therefore no state to keep the old wording for. It now names the per-type declaration that actually locked the surface and keeps the env var as the documented remedy, in the hint — the same place the server's own 403 text puts it. New rows go through the metadata-admin defaults maps (EN + ZH), the channel this surface uses.Red-first
Predicted directions were written down before the revert; the run matched exactly — 3 red, 13 green. Reverting only the two source files to
origin/mainand keeping the tests:(The second line reads
expected+ the rendered div element +to be nullin the real output. The element is transcribed as[ div element ]here because GitHub's body sanitizer strips an angle bracket followed by a letter as an HTML tag at rest, and ate it verbatim on the first save of this description.)The misleading caption, captured verbatim from that run:
The 13 that stay green are green on purpose, and include the three must-not-change pins the ruling named: both flags false stays read-only (with the honest caption), the
readOnlyprop still wins overallowRuntimeCreate: true, and anallowOrgOverridepackage is byte-identical to before.Verification
pnpm --filter '@object-ui/app-shell^...' build→ exit 0.pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/metadata-admin/→ 145 files / 1521 passed, 1 skipped.pnpm exec vitest run --maxWorkers=2 packages/app-shell/src/views/studio-design/→ 20 files / 127 passed (the Access pillar hosts this editor).tsc --noEmit→ exit 0;tsc -p tsconfig.test.json→ exit 0. (This package has notsconfig.typetests.json; its second project istsconfig.test.json.)eslinton the four changed files → 0 errors, 24 warnings, all pre-existing: the identical count is produced by the same command onorigin/main.check-control-bytesOK (4226 files),check:i18n-keysOK,check:i18n-driftOK (0 en values changed — that gate scopes topackages/i18n/src/locales, and these rows live in the app-shell defaults maps)..d.tsmeasured both ways: built@object-ui/app-shellwith the fix and again with the two source files reverted, hashing every emitted.d.ts. Identical —37dfb635b78d3db7941734cf6b4f23d6a9fbb7623f1eacfaadb33919f2dbe6aeboth times. No public type surface change, so the changeset is patch.Out of scope, filed separately
#4518 —
PermissionMatrixEditormodels only the server's type tier, not its artifact tier (ResourceEditPage:1332models both). On a multi-environment kernel an env-scope edit of a code-declared permission set can now offer Save and get a 403. Not folded in here: the ruling scoped this card to the type-tier disjunction and to trusting the entry's flags without a new probe, and adding artifact awareness would have re-locked the card's own headline case, which the QA run measured returning 200.Generated by Claude Code