fix(app-shell): a failed package-list refresh is not a deletion (objectui#7821)

.changeset/7821-manage-refresh-failure-is-not-deletion.md

@@ -0,0 +1,29 @@
+---
+'@object-ui/app-shell': patch
+---
+
+A failed package-list refresh no longer reads as "the package was deleted" and no
+longer evicts the author from the Studio (objectui#7821).
+
+`onManageChanged` — the callback the Studio's `PackageDetailSheet` fires after every
+package lifecycle action (disable / duplicate / delete / publish / manifest edit) —
+refreshed the list into a local `list` initialised to `[]` and swallowed the rejection
+under a comment reading "keep the stale list". That is true of the `pkgs` state, which
+is simply not written, and false of the local, which stayed `[]`. So after a failed
+`GET /api/v1/packages` the `!list.some(...)` check three lines down was
+unconditionally true, the code took the branch labelled `// Deleted`, and — when the
+managed package was the one under the editor — navigated away with `list[0]`
+undefined, i.e. to `/home`. One transient 503, network blip or auth expiry threw the
+author out of the editor with no toast and no confirmation, while the package was
+still there.
+
+The local now starts as `null` — "the refresh told us nothing" — and only a list that
+actually came back, without the managed package in it, is read as a deletion. A
+failure draws no inference at all: no navigation. It is reported instead, through the
+posture this surface already has (objectui#7368): `formatMetadataError` on the shared
+`studio-package-list` sonner id, and recorded so the switcher reads `failed` rather
+than presenting a now-stale list as current. A real deletion navigates exactly as
+before — to the first surviving package, or `/home` when none is left.
+
+Still a `.catch` and still no retry: one 503 must not take the Studio down, and no
+retry policy has been ruled on.

packages/app-shell/src/views/studio-design/StudioDesignSurface.packageDeletionInference.test.tsx

@@ -0,0 +1,300 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * objectui#7821 — `onManageChanged` read a FAILED package-list refresh as
+ * "the package was deleted" and evicted the author out of the Studio.
+ *
+ * The callback runs after every package lifecycle action fired from the
+ * `PackageDetailSheet` (disable / duplicate / delete / publish / manifest
+ * edit). It refreshed the list into a LOCAL `list`, initialised to `[]`, and
+ * swallowed the rejection under a comment reading "keep the stale list" —
+ * true of the `pkgs` state, which is simply not written, and false of the
+ * local, which stayed `[]`. Three lines later `!list.some((p) => p.id ===
+ * managedId)` was therefore UNCONDITIONALLY true after a failure, so the code
+ * took the branch labelled `// Deleted`; when the managed package was the one
+ * under the editor it navigated away, and `list[0]` being `undefined` made
+ * the destination `/home`.
+ *
+ * Net effect: one transient `GET /api/v1/packages` failure — a 503 from the
+ * durable half, a network blip, an auth expiry — threw the author out of the
+ * editor, with no toast and no confirmation, while the package was still
+ * there. That is the sibling defect of objectui#7368 escalated: not a
+ * swallowed failure, but a swallowed failure that then decides the OPPOSITE
+ * of the truth.
+ *
+ * These pins are behavioural, and there are three of them because the fix has
+ * two ways to be wrong:
+ *   1. a FAILED refresh causes NO navigation (the defect), and
+ *   2. it is still REPORTED, through this file's existing objectui#7368
+ *      posture (`formatMetadataError` on the shared sonner id, recorded so
+ *      the trigger reads `failed`) — a fix must not buy pin 1 with silence;
+ *   3. a REAL deletion — the list comes back successfully WITHOUT the package
+ *      — still navigates exactly as before. ⛔ Not optional: pinning only 1
+ *      and 2 lets the fix degrade into "never navigate", which strands the
+ *      author on a package that no longer exists.
+ *
+ * ⛔ The `.catch` is deliberately still a `.catch` (one 503 must not take the
+ * Studio down — objectui#7368's ruling) and there is deliberately still no
+ * retry (count / backoff / what-after-giving-up are unruled policy).
+ */
+
+import '@testing-library/jest-dom/vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom';
+
+const PACKAGE_ID = 'app.b2r4';
+const SIBLING_ID = 'app.other';
+const START_PATH = `/studio/${PACKAGE_ID}/interfaces`;
+
+const row = (id: string) => ({ id, name: id, writable: true, namespace: id.split('.')[1] });
+
+// The package list refresh is the system under test — every case installs its
+// own resolution/rejection for the refresh that `onManageChanged` performs.
+const fetchPackagesMock = vi.fn();
+vi.mock('./packages-io', async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>();
+  return { ...actual, fetchPackages: (...args: unknown[]) => fetchPackagesMock(...args) };
+});
+
+// The report channel (objectui#7368's posture). Captured rather than rendered
+// so a pin can read both the message and the shared sonner id.
+const toastError = vi.fn();
+vi.mock('sonner', () => ({
+  toast: {
+    error: (...args: unknown[]) => toastError(...args),
+    success: vi.fn(),
+    info: vi.fn(),
+    dismiss: vi.fn(),
+  },
+  Toaster: () => null,
+}));
+
+/**
+ * The lifecycle sheet, stubbed down to the one thing this file is about: the
+ * `onChanged` callback it fires after a lifecycle action. Driving the real
+ * sheet's delete/disable/publish buttons would test THOSE, and the defect is
+ * in the surface's reaction, which every one of them reaches the same way.
+ */
+vi.mock('../metadata-admin/PackagesPage', async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>();
+  return {
+    ...actual,
+    PackageDetailSheet: ({
+      pkg,
+      open,
+      onChanged,
+    }: {
+      pkg: { manifest?: { id?: string } } | null;
+      open: boolean;
+      onChanged: () => void | Promise<void>;
+    }) =>
+      open ? (
+        <div data-testid="pkg-sheet" data-managed-id={pkg?.manifest?.id ?? ''}>
+          <button type="button" data-testid="lifecycle-ran" onClick={() => void onChanged()}>
+            a lifecycle action ran
+          </button>
+        </div>
+      ) : null,
+  };
+});
+
+const mockClient = {
+  list: vi.fn(async () => []),
+  listDrafts: vi.fn(async () => []),
+  layered: vi.fn(async (_t: string, name: string) => ({ effective: { name } })),
+  getDraft: vi.fn(async () => null),
+  get: vi.fn(async () => undefined),
+  save: vi.fn(async () => ({})),
+};
+
+vi.mock('../metadata-admin/useMetadata', async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>();
+  return {
+    ...actual,
+    useMetadataClient: () => mockClient,
+    useMetadataTypes: () => ({ loading: false, error: null, entries: [] }),
+  };
+});
+
+vi.mock('@object-ui/react', async (importOriginal) => {
+  const actual = await importOriginal<Record<string, unknown>>();
+  return { ...actual, useAdapter: () => ({}) };
+});
+
+// Rail siblings / docks irrelevant to the top bar — keep the render light.
+vi.mock('../../components/SuggestedBindingsPanel', () => ({ SuggestedBindingsPanel: () => null }));
+vi.mock('../metadata-admin/AccessExplainPanel', () => ({ AccessExplainPanel: () => null }));
+vi.mock('./StudioAiCopilot', () => ({ StudioChatDock: () => null }));
+vi.mock('../../preview/DraftChangesPanel', () => ({ DraftChangesPanel: () => null }));
+
+import { StudioDesignSurface } from './StudioDesignSurface';
+
+// jsdom ships neither of these; useIsMobile / useIsWideViewport and the Radix
+// popover's floating-ui measurement need them.
+window.matchMedia = ((query: string) => ({
+  matches: false,
+  media: query,
+  onchange: null,
+  addEventListener: () => {},
+  removeEventListener: () => {},
+  addListener: () => {},
+  removeListener: () => {},
+  dispatchEvent: () => false,
+})) as unknown as typeof window.matchMedia;
+(globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver =
+  (globalThis as unknown as { ResizeObserver?: unknown }).ResizeObserver ??
+  class {
+    observe() {}
+    unobserve() {}
+    disconnect() {}
+  };
+
+/** Raw-`fetch` calls to the bare packages endpoint — that is `fetchFullPackage`,
+ *  the managed-snapshot refresh `onManageChanged` performs AFTER the
+ *  deletion decision. Counting it gives the negative pin a positive event to
+ *  wait for: the callback provably ran past the branch that used to navigate. */
+let snapshotFetches = 0;
+
+beforeEach(() => {
+  fetchPackagesMock.mockReset();
+  toastError.mockReset();
+  snapshotFetches = 0;
+  vi.stubGlobal(
+    'fetch',
+    vi.fn(async (input: unknown) => {
+      const url = String(input);
+      if (url === '/api/v1/packages') {
+        snapshotFetches += 1;
+        // What `fetchFullPackage` reads: the full installed record.
+        return { ok: true, json: async () => [{ manifest: { id: PACKAGE_ID, name: PACKAGE_ID } }] };
+      }
+      // The pending-drafts counter and the automation status probe.
+      return { ok: true, json: async () => [] };
+    }) as unknown as typeof fetch,
+  );
+});
+
+afterEach(() => {
+  cleanup();
+  vi.unstubAllGlobals();
+});
+
+function LocationProbe() {
+  return <div data-testid="location">{useLocation().pathname}</div>;
+}
+
+function renderSurface() {
+  return render(
+    <MemoryRouter initialEntries={[START_PATH]}>
+      <LocationProbe />
+      <Routes>
+        <Route path="/studio/:packageId/:tab" element={<StudioDesignSurface />} />
+        <Route path="/home" element={<div data-testid="home-page" />} />
+      </Routes>
+    </MemoryRouter>,
+  );
+}
+
+const trigger = () => screen.getByTitle('Switch / create package');
+const where = () => screen.getByTestId('location').textContent;
+
+/**
+ * Open the lifecycle sheet the way the author does — switcher trigger →
+ * "Package info & settings" — and hand back its "a lifecycle action ran"
+ * button. Every case shares this drive, so the ONLY difference between the
+ * pins below is what the refresh does.
+ */
+async function openLifecycleSheet(initial = [row(PACKAGE_ID)]): Promise<HTMLElement> {
+  fetchPackagesMock.mockResolvedValue(initial);
+  renderSurface();
+  await waitFor(() => expect(trigger()).toHaveAttribute('data-pkg-list-state', 'loaded'));
+  fireEvent.click(trigger());
+  fireEvent.click(await screen.findByText('Package info & settings'));
+  const sheet = await screen.findByTestId('pkg-sheet');
+  expect(sheet).toHaveAttribute('data-managed-id', PACKAGE_ID);
+  toastError.mockReset();
+  return screen.getByTestId('lifecycle-ran');
+}
+
+describe('Studio package lifecycle — a failed refresh is not a deletion (#7821)', () => {
+  it('FAILED refresh: the author is NOT evicted — no navigation at all', async () => {
+    const lifecycle = await openLifecycleSheet();
+    const before = snapshotFetches;
+
+    // The refresh that follows the lifecycle action rejects.
+    fetchPackagesMock.mockRejectedValue(new Error('Service Unavailable'));
+    fireEvent.click(lifecycle);
+
+    // Wait until the callback has SETTLED, whichever way it goes: either it
+    // navigated (the defect — the `/home` route mounts) or it went on to
+    // refresh the managed snapshot (what follows the deletion decision). Both
+    // arms are observable, so the assertions below are not racing a pending
+    // navigation, and the red lands on the eviction itself rather than on a
+    // timeout.
+    await waitFor(() =>
+      expect(snapshotFetches > before || screen.queryByTestId('home-page') !== null).toBe(true),
+    );
+
+    expect(where()).toBe(START_PATH);
+    expect(screen.queryByTestId('home-page')).not.toBeInTheDocument();
+    // Still in the editor, still on the same package.
+    expect(trigger()).toHaveTextContent(PACKAGE_ID);
+  });
+
+  it('FAILED refresh: the failure is still REPORTED through this file\'s own posture', async () => {
+    const lifecycle = await openLifecycleSheet();
+
+    fetchPackagesMock.mockRejectedValue(new Error('Service Unavailable'));
+    fireEvent.click(lifecycle);
+
+    // objectui#7368's channel, reused: the error's own message, on the one
+    // shared sonner id, so an outage that rejects several call sites is one
+    // toast rather than a stack.
+    await waitFor(() => expect(toastError).toHaveBeenCalled());
+    expect(toastError.mock.calls[0][0]).toBe('Service Unavailable');
+    expect((toastError.mock.calls[0][1] as { id?: string } | undefined)?.id).toBe('studio-package-list');
+
+    // …and recorded, so the switcher stops presenting the now-stale list as
+    // though it were current.
+    await waitFor(() => expect(trigger()).toHaveAttribute('data-pkg-list-state', 'failed'));
+    // ⛔ Never a throw: the top bar is still a working trigger.
+    fireEvent.click(trigger());
+    expect(await screen.findByText('Packages (apps)')).toBeInTheDocument();
+  });
+
+  it('REAL deletion, nothing left: still navigates to /home (behaviour unchanged)', async () => {
+    const lifecycle = await openLifecycleSheet();
+
+    // The list came back — successfully — and the package is gone from it.
+    fetchPackagesMock.mockResolvedValue([]);
+    fireEvent.click(lifecycle);
+
+    expect(await screen.findByTestId('home-page')).toBeInTheDocument();
+    expect(where()).toBe('/home');
+    // A successful refresh is not a failure: nothing was reported.
+    expect(toastError).not.toHaveBeenCalled();
+  });
+
+  it('REAL deletion, a sibling survives: still navigates to that sibling (behaviour unchanged)', async () => {
+    const lifecycle = await openLifecycleSheet([row(PACKAGE_ID), row(SIBLING_ID)]);
+
+    fetchPackagesMock.mockResolvedValue([row(SIBLING_ID)]);
+    fireEvent.click(lifecycle);
+
+    await waitFor(() => expect(where()).toBe(`/studio/${SIBLING_ID}/interfaces`));
+    expect(screen.queryByTestId('home-page')).not.toBeInTheDocument();
+  });
+
+  it('successful refresh, package still there: no navigation, and an earlier failure clears', async () => {
+    const lifecycle = await openLifecycleSheet();
+
+    fetchPackagesMock.mockResolvedValue([row(PACKAGE_ID)]);
+    const before = snapshotFetches;
+    fireEvent.click(lifecycle);
+
+    await waitFor(() => expect(snapshotFetches).toBeGreaterThan(before));
+    expect(where()).toBe(START_PATH);
+    expect(trigger()).toHaveAttribute('data-pkg-list-state', 'loaded');
+  });
+});

packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx

@@ -388,16 +388,45 @@ function PackageSwitcher({
   // snapshot (so an edit shows immediately). If the managed package was the one
   // we're editing and it's now gone (deleted), jump to another package / home.
   const onManageChanged = React.useCallback(async () => {
-    let list: PkgEntry[] = [];
+    /**
+     * `null` means "the refresh did not tell us anything", which is NOT the
+     * same fact as "the server listed the packages and yours is not among
+     * them" (objectui#7821). Those two used to be the SAME value: `list`
+     * started as `[]` and the `.catch` left it that way — the comment there
+     * said "keep the stale list", true of the `pkgs` state, which simply is
+     * not written, but never of this local. So after a failed
+     * `GET /api/v1/packages` the `!list.some(...)` below was unconditionally
+     * true, the code took the branch labelled `// Deleted`, and with `list[0]`
+     * undefined a transient 503 evicted the author from the editor to
+     * `/home` — no toast, no confirmation, package still there. Absence of
+     * evidence is not evidence of deletion.
+     */
+    let list: PkgEntry[] | null = null;
     try {
       list = await fetchPackages();
       setPkgs(list);
-    } catch {
-      /* keep the stale list */
+      // A list we did receive is current, so it also clears an earlier
+      // failure — otherwise the trigger would keep reading `failed` over
+      // names that are now fresh.
+      setPkgsErr(null);
+    } catch (e) {
+      // Reported through this surface's EXISTING posture (objectui#7368):
+      // `formatMetadataError` on the shared sonner id (one outage, one toast)
+      // and recorded, so the trigger reads `failed` rather than showing a
+      // stale list as though it were current. ⛔ Still not a `throw` — one
+      // 503 must not take the Studio down — and ⛔ still no retry, whose
+      // count / backoff / give-up state nobody has ruled on.
+      const message = formatMetadataError(e);
+      setPkgsErr(message);
+      toast.error(message, { id: PACKAGE_LIST_TOAST_ID });
     }
     const managedId = manage?.manifest.id;
     if (!managedId) return;
-    if (!list.some((p) => p.id === managedId)) {
+    // A list we actually received, that does not contain the managed package,
+    // is the ONLY evidence of deletion. `list === null` draws no inference
+    // either way: the author stays put, and the managed snapshot below is
+    // still refreshed (that call reports its own outcome).
+    if (list !== null && !list.some((p) => p.id === managedId)) {
       // Deleted — only navigate away if it was the package we're editing.
       if (managedId === packageId) {
         const next = list[0];