Uh oh!
There was an error while loading. Please reload this page.
Marketplace catalog page answers "no marketplace here" before "not an admin" (#5557) - #5582
Merged
os-sales merged 1 commit intoAug 21, 2026
Merged
Conversation
…re" before "not an admin" (#5557) MarketplacePage ordered its early returns admin-first, so on a runtime that mounts no marketplace (features.marketplace: false -- an OS_CLOUD_URL=off deployment) a non-admin was told they lack PERMISSION for a surface that exists for nobody, and the informational disabled state was unreachable for them. The runtime check now answers first: "this deployment has no marketplace" is true regardless of who is asking, and features.marketplace is public runtime config every client already reads. This is the ordering MarketplacePackagePage landed under #5533, so the sibling pages stop disagreeing for non-admins. Admin-first ordering stays correct where a marketplace exists, pinned by an explicit boundary case: a non-admin on a marketplace-ON runtime still gets MarketplaceAccessDenied. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
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 21, 2026 15:30
Uh oh!
There was an error while loading. Please reload this page.
os-sales
deleted the
claude/issue-5557-marketplace-runtime-before-admin
branch
August 21, 2026 15:31
This was referenced Aug 21, 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
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#5557
The defect
MarketplacePage— the marketplace catalog page — ordered its two early returns admin-first:So on a runtime that mounts no marketplace at all (
features.marketplace: false— anOS_CLOUD_URL=offdeployment, the EE deploy template's factory default) an unprivileged member was told they lack permission for a surface that exists for nobody. That answer sends them to ask an administrator for a grant that would not help them, and it left the informational disabled state built in #5504 unreachable for every non-admin.The fix
The runtime check answers first, because "this deployment has no marketplace" is true regardless of who is asking. That is the ordering
MarketplacePackagePagelanded under #5533, so the two sibling pages stop disagreeing about one runtime for the one class of viewer they still disagreed for.The retired code comment claimed the old order was deliberate — "a non-admin has no business reading this runtime's marketplace posture either way". The merged sibling rebuts it on the record, and this PR carries that rationale over:
features.marketplaceis public runtime config every client already reads, so the old order withheld nothing. It only misdirected.Boundary — what this PR does not claim
Admin-first ordering stays correct where a marketplace exists. On
features.marketplace: truea non-admin still getsMarketplaceAccessDenied; the catalog is an install surface. That boundary is a test assertion, not prose — it is the control for the whole change: without it, a "fix" that simply deleted the admin check would satisfy every other case in the file while handing an install surface to every member.Reverse-verification (ablation)
The pre-fix ordering was restored on disk and the suite re-run. No rebuild is involved: the tests import the subject as a relative source path (
from '../MarketplacePage'), so nothing resolves throughdist/.git diff --stat2 insertions/2 deletions; sha256fc7e9405…→60753dec…trap ... EXIT INT TERM; sha256 back tofc7e9405…,git statuscleanGates — all at
880929160pnpm --filter '@object-ui/app-shell^...' buildcommand-exit 0(run first; without it type-check reports TS2307 on every sibling)pnpm --filter @object-ui/app-shell type-checkTYPECHECK_EXIT=0—tsc --noEmit && tsc -p tsconfig.test.jsonpnpm --filter @object-ui/app-shell lintLINT_EXIT=0—2509 problems (0 errors, 2509 warnings)vitest run packages/app-shell/src/console/marketplace/Test Files 5 passed (5)·Tests 32 passed (32)check-control-bytesOK (scanned 4638 tracked text file(s); skipped 85 binary)check-changeset-presence2 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-majorNo changeset declares a major bump.check-changeset-fixedAll workspace packages are in the changeset fixed group.check-i18n-call-site-keyscheck-lint-coverage·check-type-check-coverage46/46 packages linted·41/41 packages compile their testsDeclared narrowing — vitest was run over
console/marketplace/rather than all ofpackages/app-shell, and here is why that cannot hide a failure. The changed function body can only execute where the real component is imported. Measured across every test file in the repo: exactly 3 import it for real, and all three are in the directory that was run. The only two others that reach the marketplace route replace it with a stub (vi.mock('../marketplace/MarketplacePage', …)), so the changed code cannot run in them. Invariance for untouched files: the change is a statement reorder inside one function body and alters no export signature, andtype-checkcompiled the whole package (both tsconfigs) green.Scope
MarketplacePage.tsx, one new test file, one changeset.MarketplacePackagePage.tsxis read-only here — it is the already-fixed sibling, mounted by the new tests only to prove the two pages agree.MarketplaceAccessDenied.tsxandMarketplaceDisabled.tsxare untouched: only which of the two the page reaches for changes, and in which order it decides.Why a new test file rather than growing
MarketplacePage.disabledState.test.tsx: that suite hard-mocksuseIsWorkspaceAdmin: () => trueat module scope, so every case in it is an admin and none of them can see this defect. The admin flag has to vary per case here, which is a different module mock and therefore a different file.Generated by Claude Code