perf(console): write app navigation at page and dashboard create and delete sites

.changeset/28680-navigation-writes-at-sites.md

@@ -0,0 +1,20 @@
+---
+'@object-ui/app-shell': patch
+---
+
+perf(console): write app navigation where a page or dashboard is created or deleted, and stop reading the whole `/meta/page` and `/meta/dashboard` lists on every app page
+
+Clause-②: no
+Metadata-protocol: none
+
+`NavigationSyncEffect` read both lists on every app page after the first paint, only to diff their name sets and add or remove navigation entries. The console's sites now write navigation themselves:
+
+- **The metadata editor's Publish** adds a package-less page or dashboard to every writable app the first time it is published (the draft was its only layer). A later publish of the same item is an update and writes nothing.
+- **The metadata editor's Delete** removes a package-less page or dashboard from every writable app when the delete leaves no baseline. A reset to a packaged baseline writes nothing.
+- **Publish all** asks each package-less page or dashboard draft's layers before publishing it, and adds the ones it creates to navigation.
+
+The diff's rules are kept: platform artifacts (a `sys_` name, or `_packageId` / `_provenance: 'package'`) never write navigation, write-protected apps are skipped, and nothing is written in draft preview. A create no longer adds a second entry to an app that already references the page or dashboard.
+
+What no longer reaches navigation: a package-less page or dashboard created or deleted outside this tab (a server-side agent, another tab, a raw REST call). The diff noticed one only when it appeared in this tab's list mid-session; it now reaches navigation only through its own writer. Items from a package install or a CLI deploy carry `_packageId` and never reached navigation through the diff.
+
+`NavigationSyncEffect` is removed. `@object-ui/app-shell` is private and nothing outside it imported the effect.

ui/packages/app-shell/src/chrome/CommandPalette.navLabelInherits-9868.test.tsx

@@ -10,8 +10,8 @@
  * The ⌘K palette names an app navigation entry the way the sidebar does,
  * including an entry with NO `label` (objectui#9868, contract review finding 1).
  *
- * Since objectui#9868 `NavigationSyncEffect` writes every page and dashboard
- * created in the console WITHOUT a `label` — absent means "inherit the target's
+ * Since objectui#9868 the console writes every page and dashboard created in
+ * it into navigation WITHOUT a `label` (`useNavigationWrites`, objectstack#28680) — absent means "inherit the target's
  * current label at render time". The palette read `resolveKeyedI18nLabel(
  * item.label, t)`, which answers `undefined` for an absent label, so such an
  * entry drew a BLANK row whose cmdk filter value read `page undefined my_page`.
@@ -53,7 +53,7 @@ const APP = {
   name: 'crm',
   label: 'CRM',
   navigation: [
-    // What NavigationSyncEffect writes for a page / dashboard since objectui#9868.
+    // What the console's create sites write for a page / dashboard since objectui#9868.
     { id: 'nav_page', type: 'page', pageName: 'my_page' },
     { id: 'nav_dash', type: 'dashboard', dashboardName: 'sales_overview' },
     { id: 'nav_board', type: 'object', objectName: 'customer', viewName: 'board' },

ui/packages/app-shell/src/chrome/CommandPalette.tsx

@@ -178,7 +178,8 @@ function CommandPaletteBody(props: CommandPaletteProps & { gates: NavMemberGates
   const { t, language } = useObjectTranslation();
   // A nav entry's text, the way the sidebar and `nav:menu` name it: an entry
   // with NO `label` shows its target's current label, else its target's machine
-  // name (objectui#9868 — `NavigationSyncEffect` writes such entries). A present
+  // name (objectui#9868 — the console's page and dashboard create sites write
+  // such entries through `useNavigationWrites`). A present
   // label renders as authored — an inline locale map in the viewer's `language`,
   // the locale the sidebar passes (objectui#11299).
   const targetLabel = useNavTargetLabel();

ui/packages/app-shell/src/console/AppContent.tsx

@@ -57,7 +57,6 @@ import { ObjectNotFoundState } from '../views/ObjectView.js';
 import { KeyboardShortcutsDialog } from '../chrome/KeyboardShortcutsDialog.js';
 import { OnboardingWalkthrough } from '../chrome/OnboardingWalkthrough.js';
 import { RouteFader } from '../chrome/RouteFader.js';
-import { NavigationSyncEffect } from '../hooks/useNavigationSync.js';
 import { AppNoAccessEmptyState } from './AppNoAccessEmptyState.js';
 import { appNamesOnlyUnreadableObjects, appServesNoNavigation } from './appServesNoNavigation.js';
 import { resolveDeclaredLandingRoute, type LandingVisibilityEvaluator } from './appDeclaredLanding.js';
@@ -1452,7 +1451,6 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps =
         currentUserId={user?.id ?? null}
         currentOrgId={activeOrganization?.id ?? null}
       >
-      <NavigationSyncEffect />
       <ConsoleLayout
         activeAppName={activeApp.name}
         activeApp={activeApp}

ui/packages/app-shell/src/console/__tests__/AppContent.routeChunkPreload-12980.test.tsx

@@ -70,7 +70,6 @@ vi.mock('../../layout/ConsoleLayout', () => ({
 vi.mock('../../chrome/CommandPalette', () => ({ CommandPalette: () => null }));
 vi.mock('../../chrome/KeyboardShortcutsDialog', () => ({ KeyboardShortcutsDialog: () => null }));
 vi.mock('../../chrome/OnboardingWalkthrough', () => ({ OnboardingWalkthrough: () => null }));
-vi.mock('../../hooks/useNavigationSync', () => ({ NavigationSyncEffect: () => null }));
 vi.mock('@object-ui/i18n', async (importOriginal) => ({
   ...(await importOriginal<Record<string, unknown>>()),
   useObjectTranslation: () => ({

ui/packages/app-shell/src/hooks/__tests__/chromeReads.afterFirstPaint-12980.test.tsx

@@ -2,12 +2,10 @@
  * ObjectUI
  * Copyright (c) 2024-present ObjectStack Inc.
  *
- * objectui#12980, part 3 — two shell readers that mount UNDER the boot splash
- * wait for the page's first paint, not the splash's:
+ * objectui#12980, part 3 — shell readers that mount UNDER the boot splash
+ * wait for the page's first paint, not the splash's (objectstack#28680 retired a
+ * third, `NavigationSyncEffect`):
  *
- *   - `NavigationSyncEffect` reads the `pages` / `dashboards` lists (lazy
- *     getters: reading one starts its `/meta/:type` read) only once the splash
- *     is gone and a frame has painted;
  *   - the bell's Activity feed, until the bell asks for it, does not even ask
  *     whether `sys_activity` exists: inside an app that answer is its own
  *     by-name read through the app's object scope, which used to go out in
@@ -39,7 +37,6 @@ vi.mock('@object-ui/auth', async (importOriginal) => ({
 }));
 vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } }));
 
-import { NavigationSyncEffect } from '../useNavigationSync';
 import { useSignedInUserLocale } from '../useUserLocale';
 import { __resetFirstPaint, useSharedActivityFeed } from '../sharedUserFeeds';
 import { ExpressionProvider } from '../../providers/ExpressionProvider';
@@ -73,51 +70,6 @@ afterEach(() => {
   vi.restoreAllMocks();
 });
 
-describe('NavigationSyncEffect reads the page and dashboard lists after the first paint (objectui#12980)', () => {
-  function metadataRecording(read: Set<string>): MetadataContextValue {
-    return {
-      apps: [{ name: 'crm', label: 'CRM', navigation: [] }],
-      get pages() {
-        read.add('page');
-        return [];
-      },
-      get dashboards() {
-        read.add('dashboard');
-        return [];
-      },
-      objects: [],
-      reports: [],
-      loading: false,
-      error: null,
-      refresh: async () => {},
-      invalidate: () => {},
-      ensureType: async () => [],
-      getItem: async () => null,
-      getItemsByType: () => [],
-      getTypeStatus: () => 'ready',
-    } as unknown as MetadataContextValue;
-  }
-
-  it('reads neither list while the boot splash is up, and both once it is gone', async () => {
-    const read = new Set<string>();
-    const adapter = { getClient: () => ({ meta: { saveItem: vi.fn() } }) } as unknown as React.ComponentProps<
-      typeof AdapterCtx.Provider
-    >['value'];
-    render(
-      <AdapterCtx.Provider value={adapter}>
-        <MetadataCtx.Provider value={metadataRecording(read)}>
-          <NavigationSyncEffect />
-        </MetadataCtx.Provider>
-      </AdapterCtx.Provider>,
-    );
-    await someFrames();
-    expect([...read]).toEqual([]);
-
-    dropSplash();
-    await waitFor(() => expect([...read].sort()).toEqual(['dashboard', 'page']));
-  });
-});
-
 describe('useSignedInUserLocale reads the user row after the first paint (objectui#12980)', () => {
   function Probe() {
     useSignedInUserLocale();

ui/packages/app-shell/src/hooks/__tests__/useNavigationSync.labelInherits-9868.test.tsx

@@ -16,17 +16,18 @@
  * means "resolve at render time", so an unnamed entry is written with NO
  * `label` key; an author label is still written, verbatim.
  *
- * Asserted on the real writer — `NavigationSyncEffect`, the one caller in the
- * console, which never passes a label — and, for the author-label control, on
- * `useNavigationSync().syncPageCreated` / `syncDashboardCreated` with one.
+ * Asserted on the real writer — `useNavigationWrites().created`, the door the
+ * console's create sites call (objectstack#28680; it replaced
+ * `NavigationSyncEffect`), which never passes a label — and, for the
+ * author-label control, on `useNavigationSync().syncPageCreated` /
+ * `syncDashboardCreated` with one.
  */
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import React, { useEffect } from 'react';
 import { render, waitFor } from '@testing-library/react';
 import { MetadataCtx, AdapterCtx, type MetadataContextValue } from '@object-ui/react';
-import { NavigationSyncEffect, useNavigationSync } from '../useNavigationSync';
-import { __markFirstPainted } from '../sharedUserFeeds';
+import { useNavigationSync, useNavigationWrites } from '../useNavigationSync';
 
 vi.mock('sonner', () => ({
   toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() },
@@ -64,29 +65,32 @@ const adapterFor = (saveItem: ReturnType<typeof vi.fn>) =>
 const writtenEntries = (saveItem: ReturnType<typeof vi.fn>) =>
   saveItem.mock.calls.flatMap(([, , schema]) => schema.navigation as Record<string, unknown>[]);
 
-const flush = () => new Promise((r) => setTimeout(r, 0));
-
 beforeEach(() => {
   vi.clearAllMocks();
-  // objectui#12980 — the effect reads nothing before the console's first
-  // paint; these cases are about the diff, so they start past it.
-  __markFirstPainted();
 });
 
-describe('objectui#9868 — NavigationSyncEffect writes unnamed entries with NO label', () => {
+describe('objectui#9868 — the create sites write unnamed entries with NO label', () => {
   it('a new page and a new dashboard land in navigation without a `label` key', async () => {
     const saveItem = vi.fn().mockResolvedValue({});
-    const tree = (world: World) => (
+    function Site() {
+      const { created } = useNavigationWrites();
+      useEffect(() => {
+        void (async () => {
+          await created('page', 'my_page', { name: 'my_page' });
+          await created('dashboard', 'my_dash', { name: 'my_dash', label: 'My Dashboard' });
+        })();
+        // Once, on mount: the two creates are the case.
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+      }, []);
+      return null;
+    }
+    render(
       <AdapterCtx.Provider value={adapterFor(saveItem)}>
-        <MetadataCtx.Provider value={metaValue(world)}>
-          <NavigationSyncEffect />
+        <MetadataCtx.Provider value={metaValue({ pages: [], dashboards: [] })}>
+          <Site />
         </MetadataCtx.Provider>
-      </AdapterCtx.Provider>
+      </AdapterCtx.Provider>,
     );
-    const { rerender } = render(tree({ pages: [], dashboards: [] }));
-    await flush();
-
-    rerender(tree({ pages: [{ name: 'my_page' }], dashboards: [{ name: 'my_dash', label: 'My Dashboard' }] }));
     await waitFor(() => expect(saveItem).toHaveBeenCalledTimes(2));
 
     const [page, dash] = [

ui/packages/app-shell/src/hooks/__tests__/useNavigationSync.standardEntryLocalizes-11201.test.tsx

@@ -17,8 +17,9 @@
  * would be relying on the renderer's translate-if-equal-to-name convention,
  * which B retires. This pins the other half, end to end on the real code:
  *
- *   1. the writer — `NavigationSyncEffect`, the console's one caller of the
- *      sync — stores the new dashboard's entry with NO `label` key;
+ *   1. the writer — `useNavigationWrites().created`, the door the console's
+ *      create sites call (objectstack#28680) — stores the new dashboard's entry
+ *      with NO `label` key;
  *   2. that STORED entry, handed to the sidebar's `NavigationRenderer` wired as
  *      `UnifiedSidebar` wires it (`useNavTargetLabel` plus the active
  *      `language` as `locale`, objectui#11299), under a real `I18nProvider`
@@ -31,7 +32,7 @@
 
 import '@testing-library/jest-dom/vitest';
 import { describe, it, expect, vi, beforeAll } from 'vitest';
-import React from 'react';
+import React, { useEffect } from 'react';
 import { render, screen, waitFor, within, cleanup } from '@testing-library/react';
 import { MemoryRouter } from 'react-router-dom';
 import type { TranslationData } from '@objectstack/spec/system';
@@ -49,8 +50,7 @@ import {
   transformSpecTranslations,
   useObjectTranslation,
 } from '@object-ui/i18n';
-import { NavigationSyncEffect } from '../useNavigationSync';
-import { __markFirstPainted } from '../sharedUserFeeds';
+import { useNavigationWrites } from '../useNavigationSync';
 import { useNavTargetLabel } from '../useNavTargetLabel';
 
 vi.mock('sonner', () => ({
@@ -89,27 +89,28 @@ function metaValue(dashboards: unknown[]): MetadataContextValue {
   };
 }
 
-const flush = () => new Promise((r) => setTimeout(r, 0));
-
 /** Drive the real writer through a dashboard creation; return the entry it stored. */
 async function syncedDashboardEntry(): Promise<Record<string, unknown>> {
   const saveItem = vi.fn().mockResolvedValue({});
   const adapter = { getClient: () => ({ meta: { saveItem } }) } as unknown as React.ComponentProps<
     typeof AdapterCtx.Provider
   >['value'];
-  const tree = (dashboards: unknown[]) => (
+  function Site() {
+    const { created } = useNavigationWrites();
+    useEffect(() => {
+      void created('dashboard', DASHBOARD.name, DASHBOARD);
+      // Once, on mount: the create is the case.
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, []);
+    return null;
+  }
+  const { unmount } = render(
     <AdapterCtx.Provider value={adapter}>
-      <MetadataCtx.Provider value={metaValue(dashboards)}>
-        <NavigationSyncEffect />
+      <MetadataCtx.Provider value={metaValue([DASHBOARD])}>
+        <Site />
       </MetadataCtx.Provider>
-    </AdapterCtx.Provider>
+    </AdapterCtx.Provider>,
   );
-  // objectui#12980 — the effect reads nothing before the console's first
-  // paint; this case is about the stored entry, so it starts past it.
-  __markFirstPainted();
-  const { rerender, unmount } = render(tree([]));
-  await flush();
-  rerender(tree([DASHBOARD]));
   await waitFor(() => expect(saveItem).toHaveBeenCalledTimes(1));
   unmount();
   const [type, name, schema] = saveItem.mock.calls[0];