From 3b10d9c2ca1a4c3a14a4369c20fb9da412e9ef78 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 21 Aug 2026 18:55:43 +0000
Subject: [PATCH 1/2] fix(app-shell): decide the marketplace admin refusal
 before the package load
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
---
 .../marketplace/MarketplacePackagePage.tsx    |  47 ++-
 ...MarketplacePackagePage.guardOrder.test.tsx | 301 ++++++++++++++++++
 .../MarketplacePage.guardOrder.test.tsx       |  60 ++--
 3 files changed, 361 insertions(+), 47 deletions(-)
 create mode 100644 packages/app-shell/src/console/marketplace/__tests__/MarketplacePackagePage.guardOrder.test.tsx

diff --git a/packages/app-shell/src/console/marketplace/MarketplacePackagePage.tsx b/packages/app-shell/src/console/marketplace/MarketplacePackagePage.tsx
index d02263420..132bf8f97 100644
--- a/packages/app-shell/src/console/marketplace/MarketplacePackagePage.tsx
+++ b/packages/app-shell/src/console/marketplace/MarketplacePackagePage.tsx
@@ -98,10 +98,21 @@ export function MarketplacePackagePage() {
   const marketplaceEnabled = isMarketplaceEnabled();
 
   const [data, setData] = useState<MarketplaceDetailResponse | null>(null);
-  // Seeded from the flag rather than settled by the effect: a runtime with no
-  // marketplace is not "loading" a package, it is done. Keeps the state
+  // Seeded from the runtime flag rather than settled by the effect: a runtime
+  // with no marketplace is not "loading" a package, it is done. Keeps the state
   // truthful even if the early return below is ever reordered -- a seeded
   // `true` with the fetch skipped would spin forever.
+  //
+  // Deliberately NOT `&& isAdmin`, even though objectui#5583 gates both fetches
+  // on `isAdmin` as well. `isAdmin` reads `activeMember`, which AuthProvider
+  // resolves asynchronously AFTER the session settles (`refreshActiveMember`),
+  // so an admin whose adminship comes from the org member row renders once as a
+  // non-admin before flipping. Seeding `false` there would leave that first
+  // admin render with `loading: false` and no data -- i.e. the destructive
+  // "failed to load" card, painted for a frame before the effect could raise
+  // the flag again. `true` means "this runtime has a marketplace, so a package
+  // answer is expected and none has arrived yet"; whether THIS viewer may see
+  // it is the separate question answered by the guard above the branch.
   const [loading, setLoading] = useState(marketplaceEnabled);
   const [error, setError] = useState<string | null>(null);
 
@@ -162,6 +173,10 @@ export function MarketplacePackagePage() {
     // are absent on this runtime too -- the probe would be one more guaranteed
     // 404 in the operator's network log.
     if (!marketplaceEnabled) return;
+    // The same argument one step further (objectui#5583): this viewer is
+    // refused before any CTA renders, so seeding one is work fired on behalf of
+    // a page we have already decided not to draw.
+    if (!isAdmin) return;
     const currentEnvId = getRuntimeConfig().defaultEnvironmentId ?? '';
     let cancelled = false;
     (async () => {
@@ -171,7 +186,7 @@ export function MarketplacePackagePage() {
       setCloudInstalledVersion(info.version);
     })();
     return () => { cancelled = true; };
-  }, [packageId, marketplaceEnabled]);
+  }, [packageId, marketplaceEnabled, isAdmin]);
 
   useEffect(() => {
     let cancelled = false;
@@ -182,6 +197,10 @@ export function MarketplacePackagePage() {
       // race the destructive card onto the screen before the disabled state
       // settles (objectui#5533).
       if (!marketplaceEnabled) return;
+      // Nor on behalf of a viewer this page refuses (objectui#5583).
+      // Authorization is not a function of whether the fetch succeeded, so it
+      // is settled before the request rather than after it.
+      if (!isAdmin) return;
       setLoading(true);
       setError(null);
       try {
@@ -194,7 +213,7 @@ export function MarketplacePackagePage() {
       }
     })();
     return () => { cancelled = true; };
-  }, [packageId, marketplaceEnabled]);
+  }, [packageId, marketplaceEnabled, isAdmin]);
 
   const openInstall = async () => {
     setInstallOpen(true);
@@ -518,6 +537,24 @@ export function MarketplacePackagePage() {
   // that exists for nobody is the same misdirection this fix removes.
   if (!marketplaceEnabled) return <MarketplaceDisabled />;
 
+  // Ahead of BOTH the loading and the load-failure branches below
+  // (objectui#5583): authorization is not a function of whether the fetch
+  // succeeded. Sitting behind them, this guard handed a non-admin whose package
+  // failed to load the destructive "Failed to load package" card carrying the
+  // server's own error message -- a diagnosis about a surface they are not
+  // allowed to use -- and reached the refusal only on the paths where the load
+  // happened to work. Both fetch effects above are gated on the same predicate,
+  // so the refusal also stops the page requesting on behalf of a viewer it has
+  // already decided to turn away: the discipline objectui#5533 established on
+  // this page for `features.marketplace`, applied to the other predicate that
+  // decides the same thing.
+  //
+  // The server remains the authority on what a non-admin may fetch; this only
+  // stops the client doing work it would throw away. It is also the ordering
+  // `MarketplacePage` carries after objectui#5557, so the sibling pages now
+  // answer one runtime the same way for every viewer.
+  if (!isAdmin) return <MarketplaceAccessDenied />;
+
   if (loading) {
     return (
       <div className="mx-auto w-full max-w-6xl flex flex-col gap-6 p-4 sm:p-6">
@@ -590,8 +627,6 @@ export function MarketplacePackagePage() {
     ? t(`marketplace.category.${pkg.category}` as any, { defaultValue: pkg.category })
     : null;
 
-  if (!isAdmin) return <MarketplaceAccessDenied />;
-
   return (
     <div className="mx-auto w-full max-w-6xl flex flex-col gap-6 p-4 sm:p-6">
       <Button variant="ghost" size="sm" className="self-start -ml-2 text-muted-foreground hover:text-foreground" onClick={() => navigate(`${basePath}/system/marketplace`)}>
diff --git a/packages/app-shell/src/console/marketplace/__tests__/MarketplacePackagePage.guardOrder.test.tsx b/packages/app-shell/src/console/marketplace/__tests__/MarketplacePackagePage.guardOrder.test.tsx
new file mode 100644
index 000000000..0d602c7a0
--- /dev/null
+++ b/packages/app-shell/src/console/marketplace/__tests__/MarketplacePackagePage.guardOrder.test.tsx
@@ -0,0 +1,301 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The package detail page decides "you are not an admin" BEFORE it fetches,
+ * and before it renders either the loading skeleton or the load-failure card
+ * (objectui#5583).
+ *
+ * ## The defect
+ *
+ * `MarketplacePackagePage` ordered its early returns like this:
+ *
+ *     if (!marketplaceEnabled) return (MarketplaceDisabled)
+ *     if (loading)             { ...skeleton... }
+ *     if (error || !data)      { ...destructive "Failed to load package"... }
+ *     ...
+ *     if (!isAdmin)            return (MarketplaceAccessDenied)
+ *
+ * with both fetch effects gated on `marketplaceEnabled` alone. On a runtime
+ * where the marketplace IS mounted, that produced two wrong answers for a
+ * non-admin who opened a package URL:
+ *
+ *  1. When the load failed, they got the destructive card carrying the
+ *     SERVER'S OWN error message — a diagnosis about a surface they are not
+ *     allowed to use — and never reached the refusal at all. Whether they were
+ *     refused or diagnosed came down to whether an unrelated request happened
+ *     to succeed.
+ *  2. `getMarketplacePackage` and `getCloudInstallationInfo` both fired on
+ *     behalf of a viewer the page had already decided to turn away.
+ *     objectui#5533 established on this same page that a request it knows it
+ *     will discard should be skipped rather than fired.
+ *
+ * ## What is NOT claimed
+ *
+ * That the refusal itself is wrong, or that the server stops being the
+ * authority on what a non-admin may fetch. This suite therefore carries a
+ * control for every claim it makes: the SAME failing load, seen by an ADMIN,
+ * must still produce the destructive card with the server's message intact. A
+ * "fix" that hoisted the refusal unconditionally, or deleted the failure
+ * branch, satisfies every non-admin case here and fails that one.
+ *
+ * ## Why these cases boot the REAL runtime-config module
+ *
+ * Same argument the sibling suites make (`MarketplacePage.guardOrder`,
+ * `*.disabledState`): `features.marketplace` is derived by the server from its
+ * own route table and is never inferred from the shape of a failure, so every
+ * case boots the genuine `initRuntimeConfig()` over a stubbed
+ * `GET /api/v1/runtime/config` and lets the real merge run.
+ *
+ * ## Why a separate suite from `MarketplacePackagePage.disabledState.test.tsx`
+ *
+ * That suite hard-mocks `useIsWorkspaceAdmin: () => true` at module scope, so
+ * every viewer in it is an admin and none of them can see this defect. The
+ * admin flag has to vary per case here — a different module mock, therefore a
+ * different file.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, screen, waitFor } from '@testing-library/react';
+
+/** The viewer's privilege, varied per case — the axis this suite exists for. */
+const viewer = vi.hoisted(() => ({ isAdmin: false }));
+
+vi.mock('react-router-dom', () => ({
+  useNavigate: () => vi.fn(),
+  useParams: () => ({ packageId: 'com.acme.crm' }),
+}));
+
+vi.mock('@object-ui/auth', () => ({
+  useIsWorkspaceAdmin: () => viewer.isAdmin,
+}));
+
+// `t` echoes the KEY, for the reason the sibling suites give: a `t` echoing
+// `defaultValue` renders identical text whether or not the key is wired, and
+// the claim under test is WHICH state — and so which string — the page reaches.
+vi.mock('@object-ui/i18n', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  useObjectTranslation: () => ({
+    t: (key: string, opts?: Record<string, unknown>) =>
+      typeof opts?.url === 'string' ? `«${key}:${opts.url}»` : `«${key}»`,
+    language: 'en',
+  }),
+}));
+
+const getMarketplacePackage = vi.fn();
+const getCloudInstallationInfo = vi.fn();
+const listLocalInstalls = vi.fn();
+
+vi.mock('../marketplaceApi', () => ({
+  getMarketplacePackage: (...a: unknown[]) => getMarketplacePackage(...a),
+  getCloudInstallationInfo: (...a: unknown[]) => getCloudInstallationInfo(...a),
+  listLocalInstalls: (...a: unknown[]) => listLocalInstalls(...a),
+  listMarketplacePackages: async () => ({ items: [] }),
+  listOrgPackages: async () => ({ items: [] }),
+  listInstalledPackages: async () => ({ items: [] }),
+  installPackage: async () => ({}),
+  installLocal: async () => ({}),
+  uninstallLocal: async () => ({}),
+  listCloudEnvironments: async () => [],
+  listInstallableOrgIds: async () => [],
+  cloudInstallDeepLink: () => '',
+  reseedSampleData: async () => ({ ok: true }),
+  purgeSampleData: async () => ({ ok: true }),
+  reseedLocalSampleData: async () => ({ ok: true }),
+  purgeLocalSampleData: async () => ({ ok: true }),
+}));
+
+vi.mock('../../../assistant/assistantBus', () => ({ emitMetadataRefresh: () => {} }));
+vi.mock('../../../providers/MetadataProvider', () => ({ useMetadata: () => ({ refresh: () => {} }) }));
+// Renders nothing here: it fetches its own suggestions on mount, which is a
+// second network surface this suite is not about.
+vi.mock('../../../components/SuggestedBindingsPanel', () => ({ SuggestedBindingsPanel: () => null }));
+
+import { initRuntimeConfig, resetRuntimeConfigForTesting } from '../../../runtime-config';
+import { MarketplacePackagePage } from '../MarketplacePackagePage';
+
+/** A `GET /api/v1/runtime/config` answer, as the server sends it. */
+type ConfigBody = Record<string, unknown>;
+
+const serverConfig = (cloudUrl: string, marketplace: boolean): ConfigBody => ({
+  cloudUrl,
+  singleEnvironment: true,
+  features: { installLocal: true, marketplace, aiStudio: true, autoPublishAiBuilds: true },
+  branding: { productName: 'ObjectOS', productShortName: 'ObjectOS' },
+});
+
+/** Boot the SPA against a runtime that answers with `body`. */
+async function bootOn(body: ConfigBody) {
+  resetRuntimeConfigForTesting();
+  vi.stubGlobal(
+    'fetch',
+    vi.fn(async () => ({ ok: true, status: 200, json: async () => body })),
+  );
+  await initRuntimeConfig();
+}
+
+/**
+ * The server's own words about a surface the viewer is not allowed to use.
+ * Asserted as a STRING, not just "some error card": the defect handed this
+ * exact text to a refused viewer.
+ */
+const SERVER_MESSAGE = 'HTTP 502: manifest store unreachable';
+
+const PACKAGE = {
+  package: {
+    id: 'pkg_1',
+    manifest_id: 'com.acme.crm',
+    display_name: 'Acme CRM',
+    description: 'A CRM.',
+    latest_version: null,
+  },
+  versions: [],
+};
+
+beforeEach(() => {
+  viewer.isAdmin = false;
+  getMarketplacePackage.mockReset();
+  getCloudInstallationInfo.mockReset();
+  listLocalInstalls.mockReset();
+  getMarketplacePackage.mockResolvedValue(PACKAGE);
+  getCloudInstallationInfo.mockResolvedValue(null);
+  listLocalInstalls.mockResolvedValue([]);
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  resetRuntimeConfigForTesting();
+});
+
+describe('a NON-ADMIN on a runtime that DOES mount a marketplace', () => {
+  beforeEach(async () => {
+    await bootOn(serverConfig('https://cloud.objectos.ai', true));
+  });
+
+  it('is refused on the FIRST render, without being walked through the skeleton', () => {
+    render(<MarketplacePackagePage />);
+
+    // Deliberately synchronous — no `findBy`, no `waitFor`. Under the retired
+    // ordering `loading` was seeded `true` on a marketplace-ON runtime, so the
+    // first commit was the skeleton and this query found nothing. Reaching the
+    // refusal with nothing awaited is the whole claim.
+    expect(screen.getByText('«marketplace.accessDenied.title»')).toBeInTheDocument();
+    expect(screen.getByText('«marketplace.accessDenied.description»')).toBeInTheDocument();
+  });
+
+  it('is refused rather than handed the server’s diagnosis when the load fails', async () => {
+    getMarketplacePackage.mockRejectedValue(new Error(SERVER_MESSAGE));
+
+    render(<MarketplacePackagePage />);
+
+    expect(screen.getByText('«marketplace.accessDenied.title»')).toBeInTheDocument();
+    // The retired answer: a destructive card about a surface this viewer may
+    // not use, quoting the server verbatim.
+    expect(screen.queryByText('«marketplace.load.packageFailed»')).toBeNull();
+    expect(screen.queryByText(SERVER_MESSAGE)).toBeNull();
+    // And it stays refused — the rejection settling later must not swap the
+    // refusal out for the error card.
+    await waitFor(() => expect(getMarketplacePackage).not.toHaveBeenCalled());
+    expect(screen.getByText('«marketplace.accessDenied.title»')).toBeInTheDocument();
+  });
+
+  it('issues neither of the requests whose answers it would have thrown away', async () => {
+    render(<MarketplacePackagePage />);
+
+    await waitFor(() =>
+      expect(screen.getByText('«marketplace.accessDenied.title»')).toBeInTheDocument(),
+    );
+    expect(getMarketplacePackage).not.toHaveBeenCalled();
+    expect(getCloudInstallationInfo).not.toHaveBeenCalled();
+  });
+});
+
+describe('CONTROL — the same runtime and the same failing load, seen by an ADMIN', () => {
+  // Scoped to exactly what the cases above vary: same config, same package id,
+  // same rejection. Only `isAdmin` differs, so these cases fail for any change
+  // that reaches the refusal by breaking the load path rather than by ordering.
+  beforeEach(async () => {
+    viewer.isAdmin = true;
+    await bootOn(serverConfig('https://cloud.objectos.ai', true));
+  });
+
+  it('still gets the destructive card, still carrying the server’s own message', async () => {
+    getMarketplacePackage.mockRejectedValue(new Error(SERVER_MESSAGE));
+
+    render(<MarketplacePackagePage />);
+
+    expect(await screen.findByText('«marketplace.load.packageFailed»')).toBeInTheDocument();
+    expect(screen.getByText(SERVER_MESSAGE)).toBeInTheDocument();
+    expect(screen.queryByText('«marketplace.accessDenied.title»')).toBeNull();
+  });
+
+  it('still issues both requests and still renders the package it loads', async () => {
+    render(<MarketplacePackagePage />);
+
+    expect(await screen.findByText('Acme CRM')).toBeInTheDocument();
+    expect(getMarketplacePackage).toHaveBeenCalledWith('com.acme.crm');
+    expect(getCloudInstallationInfo).toHaveBeenCalled();
+    expect(screen.queryByText('«marketplace.accessDenied.title»')).toBeNull();
+    expect(screen.queryByText('«marketplace.load.packageFailed»')).toBeNull();
+  });
+});
+
+describe('CONTROL — the ordering against `features.marketplace` is untouched', () => {
+  beforeEach(async () => {
+    // What the EE deploy template's factory default (`OS_CLOUD_URL=off`)
+    // produces: no browse mount, so the server's derived flag is false.
+    await bootOn(serverConfig('', false));
+  });
+
+  it('still tells a NON-ADMIN the runtime has no marketplace, not that they lack permission', () => {
+    // The guard moved up, but not past this one. objectui#5557 and
+    // objectui#5533 settled that "there is no marketplace here" is true of
+    // every viewer and outranks a permission answer; hoisting `!isAdmin` one
+    // line too far would re-break exactly that.
+    render(<MarketplacePackagePage />);
+
+    expect(screen.getByTestId('marketplace-disabled')).toBeInTheDocument();
+    expect(screen.queryByText('«marketplace.accessDenied.title»')).toBeNull();
+  });
+
+  it('still tells an ADMIN the same thing, and asks the marketplace for nothing', async () => {
+    viewer.isAdmin = true;
+
+    render(<MarketplacePackagePage />);
+
+    await waitFor(() =>
+      expect(screen.getByTestId('marketplace-disabled')).toBeInTheDocument(),
+    );
+    expect(getMarketplacePackage).not.toHaveBeenCalled();
+    expect(getCloudInstallationInfo).not.toHaveBeenCalled();
+  });
+});
+
+describe('an admin whose adminship resolves AFTER first paint', () => {
+  // Not hypothetical: `useIsWorkspaceAdmin` reads `activeMember`, which
+  // `AuthProvider.refreshActiveMember` resolves asynchronously after the
+  // session settles — so an admin holding the role through the org member row
+  // renders once as a non-admin. Gating the fetches on `isAdmin` without
+  // listing it as an effect dependency would leave that admin on a page that
+  // never requests anything.
+  beforeEach(async () => {
+    await bootOn(serverConfig('https://cloud.objectos.ai', true));
+  });
+
+  it('fetches once the flag flips, instead of sitting on a page that never loads', async () => {
+    const { rerender } = render(<MarketplacePackagePage />);
+
+    expect(screen.getByText('«marketplace.accessDenied.title»')).toBeInTheDocument();
+    expect(getMarketplacePackage).not.toHaveBeenCalled();
+
+    viewer.isAdmin = true;
+    rerender(<MarketplacePackagePage />);
+
+    expect(await screen.findByText('Acme CRM')).toBeInTheDocument();
+    expect(getMarketplacePackage).toHaveBeenCalledWith('com.acme.crm');
+    // The seed decision this depends on: `loading` stays seeded from
+    // `marketplaceEnabled` alone, so the first admin render shows the skeleton
+    // rather than the destructive "no data yet" card.
+    expect(screen.queryByText('«marketplace.load.packageFailed»')).toBeNull();
+  });
+});
diff --git a/packages/app-shell/src/console/marketplace/__tests__/MarketplacePage.guardOrder.test.tsx b/packages/app-shell/src/console/marketplace/__tests__/MarketplacePage.guardOrder.test.tsx
index 44dd7bb29..5aaf9ae3a 100644
--- a/packages/app-shell/src/console/marketplace/__tests__/MarketplacePage.guardOrder.test.tsx
+++ b/packages/app-shell/src/console/marketplace/__tests__/MarketplacePage.guardOrder.test.tsx
@@ -19,6 +19,13 @@
  * been reordered under objectui#5533, so the two sibling pages disagreed about
  * one runtime for exactly one class of viewer.
  *
+ * Its detail-page legs were written against an ordering that has since been
+ * retired: objectui#5583 moved that page's `!isAdmin` guard ahead of its
+ * `loading` and `error || !data` branches, so it no longer needs a package to
+ * resolve before it can refuse anyone. The legs below now compare both pages'
+ * answers from config and privilege alone, which is what "the same kind of
+ * answer" was always meant to mean.
+ *
  * ## Why a separate suite from `MarketplacePage.disabledState.test.tsx`
  *
  * That suite pins WHERE the disabled state comes from, and hard-mocks
@@ -102,8 +109,10 @@ vi.mock('../../../providers/MetadataProvider', () => ({ useMetadata: () => ({ re
 
 import { initRuntimeConfig, resetRuntimeConfigForTesting } from '../../../runtime-config';
 import { MarketplacePage } from '../MarketplacePage';
-// Read-only here: the already-fixed sibling, mounted to prove the two pages
-// agree. objectui#5557 changes nothing in it.
+// Mounted to prove the two pages agree. objectui#5557 changed nothing in it;
+// objectui#5583 later moved its `!isAdmin` guard ahead of its load branches,
+// which is why the marketplace-ON leg below no longer has to resolve a package
+// first.
 import { MarketplacePackagePage } from '../MarketplacePackagePage';
 
 /** A `GET /api/v1/runtime/config` answer, as the server sends it. */
@@ -152,29 +161,6 @@ function answerOf(ui: ReactElement): Answer {
   return answer;
 }
 
-/**
- * The answer a page gives once it has settled.
- *
- * Needed only on a marketplace-ON runtime, and only for the detail page: its
- * `!isAdmin` guard sits AFTER its `loading` and `error || !data` branches
- * (`MarketplacePackagePage.tsx`), so it reaches a refusal only once a package
- * has loaded. A page that settles on neither refusal times out here, which is
- * the correct direction of failure — "did not refuse" is what this pins.
- */
-async function settledAnswerOf(ui: ReactElement): Promise<Answer> {
-  const { unmount } = render(ui);
-  let answer: Answer = 'neither';
-  try {
-    await waitFor(() => {
-      answer = classify();
-      expect(answer).not.toBe('neither');
-    });
-  } finally {
-    unmount();
-  }
-  return answer;
-}
-
 beforeEach(() => {
   viewer.isAdmin = false;
   listMarketplacePackages.mockReset();
@@ -262,27 +248,19 @@ describe('the "Not claimed" boundary — a runtime that DOES have a marketplace'
     expect(screen.queryByText('«marketplace.title»')).toBeNull();
   });
 
-  it('and the detail page refuses that same non-admin the same way', async () => {
+  it('and the detail page refuses that same non-admin the same way', () => {
     // The sibling invariant, stated in the other direction: agreement must hold
     // on a marketplace-ON runtime too, or "the pages agree" would only mean
     // "both pages are off".
     //
-    // The detail page needs a package that LOADS to get there: its admin guard
-    // runs after `loading` and `error || !data`, so a 404 sends it to the
-    // destructive card and it never reaches a refusal at all.
-    getMarketplacePackage.mockResolvedValue({
-      package: {
-        id: 'pkg_1',
-        manifest_id: 'com.acme.crm',
-        display_name: 'Acme CRM',
-        description: 'A CRM.',
-        latest_version: null,
-      },
-      versions: [],
-    });
-
+    // Both answers are read WITHOUT letting a request settle, and the ambient
+    // `getMarketplacePackage` here is the `beforeEach` 404. Before
+    // objectui#5583 the detail page needed a package that LOADS to reach a
+    // refusal at all — a failing load sent it to the destructive card instead —
+    // so this leg had to stub a resolving package and await the settled answer.
+    // It no longer does, and not needing to is the point.
     const catalog = answerOf(<MarketplacePage />);
-    const detail = await settledAnswerOf(<MarketplacePackagePage />);
+    const detail = answerOf(<MarketplacePackagePage />);
 
     expect(catalog).toBe(detail);
     expect(catalog).toBe('access-denied');

From 25be3f5828f92615203febbfeb59bb700be502a7 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Fri, 21 Aug 2026 18:59:38 +0000
Subject: [PATCH 2/2] docs(changeset): declare the marketplace detail
 guard-order fix

Part of #5583
---
 ...rketplace-detail-admin-guard-order-5583.md | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 .changeset/marketplace-detail-admin-guard-order-5583.md

diff --git a/.changeset/marketplace-detail-admin-guard-order-5583.md b/.changeset/marketplace-detail-admin-guard-order-5583.md
new file mode 100644
index 000000000..b50d52248
--- /dev/null
+++ b/.changeset/marketplace-detail-admin-guard-order-5583.md
@@ -0,0 +1,39 @@
+---
+'@object-ui/app-shell': patch
+---
+
+The marketplace package detail page decides "you are not an admin" before it fetches,
+instead of after the load has already failed.
+
+`MarketplacePackagePage` ordered its early returns with the `!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 therefore 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 are not allowed to use came down
+to whether an unrelated request happened to succeed.
+
+The guard now sits ahead of both branches, and `getMarketplacePackage` and
+`getCloudInstallationInfo` are gated on `isAdmin` as well, so the page stops issuing
+requests on behalf of a viewer it has already decided to turn away. That is the
+discipline objectui#5533 established on this same page for `features.marketplace`,
+applied to the other predicate that decides the same thing. It is also the ordering
+`MarketplacePage` carries after objectui#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; this only stops the client doing work it would discard.
+
+Unchanged for an admin, deliberately and under test: a failing load still produces the
+destructive card with the server's message intact, and a successful one still renders
+the package. A "fix" that hoisted the refusal unconditionally, or that deleted the
+failure branch, would satisfy every non-admin assertion and fail those two.
+
+`loading` stays seeded from `marketplaceEnabled` alone rather than from
+`marketplaceEnabled && isAdmin`. `isAdmin` reads `activeMember`, which `AuthProvider`
+resolves asynchronously *after* the session settles, so an admin whose role comes from
+the org member row renders once as a non-admin before the flag flips. Seeding `false`
+there would leave that first admin render with `loading: false` and no data — the
+destructive card, painted for a frame before the effect could raise the flag again.
+`MarketplacePackagePage.guardOrder.test.tsx` pins the flip case for that reason, along
+with the ordering, the skipped requests, and the marketplace-off boundary the guard
+must not jump above.