…ge load
MarketplacePackagePage placed its `!isAdmin` guard after both the loading
branch and the `error || !data` branch, and gated its two fetch effects on
`features.marketplace` alone. On a runtime that mounts a marketplace, a
non-admin who opened a package URL was walked through the fetch and the
skeleton, and — when the load failed — was handed the destructive "Failed to
load package" card carrying the server's own error message instead of the
refusal. Whether they were refused or diagnosed came down to whether an
unrelated request happened to succeed.
Move the guard ahead of both branches and gate `getMarketplacePackage` and
`getCloudInstallationInfo` on `isAdmin` too, so the page stops requesting on
behalf of a viewer it has already decided to turn away (the discipline #5533
established on this page for `features.marketplace`). This is the ordering
MarketplacePage carries after #5557; the sibling pages now answer one runtime
the same way for every viewer. The server remains the authority on what a
non-admin may fetch.
`loading` stays seeded from `marketplaceEnabled` alone, deliberately: `isAdmin`
reads `activeMember`, which AuthProvider resolves asynchronously after the
session, so an org-role admin renders once as a non-admin — seeding `false`
there would paint the destructive card for a frame before the effect could
raise the flag again.
Part of #5583
Fixes#5583
What changed
packages/app-shell/src/console/marketplace/MarketplacePackagePage.tsxdecided!isAdminafter both the loading branch and theerror || !databranch, and gatedits two fetch effects on
features.marketplacealone. On a runtime that mounts amarketplace, a non-admin who opened a package URL was walked through the fetch and the
skeleton, and — when the load failed — was handed the destructive "Failed to load
package" card carrying the server's own error message. Whether that viewer was refused
or handed a diagnosis about a surface they may not use came down to whether an
unrelated request happened to succeed.
The guard now sits ahead of both branches, and
getMarketplacePackage/getCloudInstallationInfoare gated onisAdmintoo, so the page stops requesting onbehalf of a viewer it has already decided to turn away — the discipline #5533
established on this page for
features.marketplace, applied to the other predicatethat decides the same thing. It is also the ordering
MarketplacePagecarries after#5557, so the two sibling pages now answer one runtime the same way for every viewer.
The server remains the authority on what a non-admin may fetch.
Line numbers: re-verified on the merged ref, not assumed
The card cited
:593/:521/:531againstf2158ec86and the triage note warnedthey would have drifted once #5582 landed. Measured on
origin/mainat8c87f0583(with #5582 merged), they had not moved — the branches were found by their code and
sat at exactly the cited lines:
8c87f0583if (!marketplaceEnabled) return <MarketplaceDisabled />if (loading) {if (error || !data) {if (!isAdmin) return <MarketplaceAccessDenied />Reorder, or behaviour change? Both — stated precisely
Pure reorder with respect to this page's own load state.
isAdmincomes fromuseIsWorkspaceAdmin(), which derives only fromuseAuth()'sactiveMember/user.It reads none of
data,loadingorerror, so the guard was never waiting on thefetch for its input — it was only sequenced behind it.
A real behaviour change in what a viewer sees, which is the point of the card: a
non-admin gets the refusal on the first commit instead of the skeleton, and instead of
the error card on a failed load. On a load that succeeds, the outcome was already
MarketplaceAccessDenied— this only stops the detour. Two requests are no longerissued for that viewer.
One coupling it removes, worth naming.
AuthProvider.refreshActiveMemberresolvesactiveMemberasynchronously after the session settles, so an admin whose role comesfrom the org member row renders once as a non-admin. Today the package fetch
incidentally masks that window with a skeleton — for as long as an unrelated request
happens to take. It is not an auth gate, and
MarketplacePagehas had no such masksince #5557. Consequence handled here rather than inherited:
loadingstays seededfrom
marketplaceEnabledalone, deliberately notmarketplaceEnabled && isAdmin.Seeding
falsewould leave that first admin render withloading: falseand no data —i.e. the destructive card, painted for a frame before the effect could raise the flag
again. The residual (a real admin briefly seeing a refusal, on this and every other
useIsWorkspaceAdmin()surface) is filed separately as #5619 — it is a property of thehook's bare-boolean shape, not of this ordering.
Tests
New:
packages/app-shell/src/console/marketplace/__tests__/MarketplacePackagePage.guardOrder.test.tsx.The non-admin cases assert synchronously after
render()— nofindBy, nowaitFor— because under the retired orderingloadingwas seededtrueon amarketplace-ON runtime and the first commit was the skeleton. Reaching the refusal with
nothing awaited is the whole claim.
Controls, scoped to exactly what the probes vary (same config, same package id, same
rejection; only
isAdmindiffers):server's message intact, and on a good load still renders the package and still
issues both requests — a "fix" that hoisted the refusal unconditionally or deleted
the failure branch passes every non-admin case and fails these;
marketplace — the guard moved up, but must not jump above that one (Console:
OS_CLOUD_URL=offrenders the marketplace as a red load-failure with wrong guidance, and the home page still promotes the marketplace entry #5504 / Marketplace package detail page still red-errors on a runtime with no marketplace — the disabled state stops at the catalog page #5533 /Marketplace catalog page tells a NON-ADMIN "access denied" on a runtime that has no marketplace at all #5557);
isAdmininboth effect dependency arrays.
Fixture triage on
MarketplacePage.guardOrder.test.tsx(#5557's suite): itssettledAnswerOfhelper and its stubbed resolving package existed only because thedetail page could not refuse anyone until a package loaded. Its header and its
detail-page leg asserted that ordering as live fact. That leg now reads both pages'
answers with
answerOfagainst the ambient 404 — strictly stronger, and no longer acomment that describes a defect this PR removes.
Reverse verification
From the committed state,
MarketplacePackagePage.tsxwas reverted toorigin/main(restore under an
EXIT/INT/TERMtrap). Mutation confirmed on disk before measuring —guard back at line 593,
0occurrences of the injectedif (!isAdmin) return;,0occurrences of the
marketplaceEnabled, isAdmin]dependency, guard once again sittingafter
categoryLabel:The 10 that stayed green are exactly the controls — the admin legs and the
marketplace-off legs — which is the discriminating pattern, not merely "it went red".
Green union, at
25be3f582(final commit)pnpm vitest run --maxWorkers=2from the repo root (package-cwd is refused, #3378), over the provable superset belowTest Files 8 passed (8)·Tests 122 passed (122)type-checkpnpm --filter @object-ui/app-shell type-check(tsc --noEmit && tsc -p tsconfig.test.json)VERDICT command-exit 0eslint .insidepackages/app-shell— the exact commandturbo run lintinvokes for this packagecheck-lint-coveragenode scripts/check-lint-coverage.mjs46/46 packages linted, 0 with outstanding errorscheck-control-bytesnode scripts/check-control-bytes.mjsOK (scanned 4672 tracked text file(s))check-changeset-presencenode scripts/check-changeset-presence.mjs3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-majornode scripts/check-changeset-no-major.mjsNo changeset declares a major bumpcheck-changeset-fixednode scripts/check-changeset-fixed.mjsprivatePackages declared: version=true, tag=falsecheck-i18n-call-site-keysnode scripts/check-i18n-call-site-keys.mjsEvery in-scope call-site key resolves against the en pack (2918 keys)Every heavy command ran through the shared verification lock; all reported
VERDICT command-exit 0.Why the test run is a superset and not a sample.
packages/app-shellhas ~487 testfiles and its full suite exceeds this container's foreground cap. The suites that can
observe this change are the ones that render the component, and that set is closed by
reading the importers: the only non-test importers are
src/index.ts(a re-export, norender, no module-level side effect) and
AppContent.tsx's lazy route — and the onetest that drives that route,
AppContent.pseudoRouteSegments.test.tsx,vi.mocks thecomponent out to a stub div, so the real module never loads there. That leaves the three
suites in
console/marketplace/__tests__/that import it directly. All six files inthat directory were run, plus
AppContent.pseudoRouteSegments.test.tsx(proving itsmock still resolves) and
packages/i18n/.../marketplace-preview-namespace-3546.test.tsx,which reads this file as text and so is sensitive to the edit even though it renders
nothing.
Changeset — measured, not assumed
.changeset/marketplace-detail-admin-guard-order-5583.md(patch,@object-ui/app-shell).Both legs built at the real
dist/path and compared uncompressed, so no gzip headercarries a filename into the number:
origin/mainsource)Exactly one emitted file differs —
dist/console/marketplace/MarketplacePackagePage.js,42,591 → 45,097 bytes (+2,506). No
.d.tsdiffers (0 of them), so the criterionread literally as "does
dist/*.d.tsmove?" answers no while the shipped JS plainlydoes; the emitted delta is the behaviour change plus the comments this package's build
preserves. The changeset gate is independently decisive here — it guards source of a
versioned package, and
@object-ui/app-shell@17.6.0is one.Filed, not fixed here
useIsWorkspaceAdmincollapses "not resolved yet" intofalse, so an org-role admin renders once as a non-admin — every admin-gated surface can refuse or hide from a real admin #5619 —useIsWorkspaceAdminhas no "not resolved yet" answer, so an org-roleadmin renders once as a non-admin across all 11 call sites. Predates this PR
(
MarketplacePagehas had the unmasked shape since Marketplace catalog page tells a NON-ADMIN "access denied" on a runtime that has no marketplace at all #5557); the one consequence localto this page is handled above by the
loadingseed.MarketplacePackagePage's third request —listLocalInstalls— is still ungated: it fires on a marketplace-off runtime and for a refused viewer #5620 — this page's third request,listLocalInstalls, is gated onfeatures.installLocalalone and still fires on a marketplace-off runtime and for arefused viewer. Same class as Marketplace package detail page still red-errors on a runtime with no marketplace — the disabled state stops at the catalog page #5533, different flag. Left alone because the intended
shape is not pinned by anything in the tree —
features.installLocalis a separatedeployment axis and local-kernel traffic is not marketplace-proxy traffic.
Not claimed
That the refusal itself is wrong, or that the server stops being the authority on what
a non-admin may fetch. This only stops the client doing work it would discard. The
catalog page's
features.marketplaceordering (#5557) is untouched and pinned by acontrol here.
Generated by Claude Code
Generated by Claude Code