From e8bec83528fe79c5f212833aee6e02c559344f4d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 03:05:44 +0000 Subject: [PATCH 1/2] feat(app-shell): wire navigation action items to the console action runtime (framework#4509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `type: 'action'` nav item rendered, gated like any other item, and did nothing when clicked. NavigationRenderer dispatches such a click to an `onAction` prop it expects the host to supply — it deliberately never reads `item.actionDef` itself — and no shipped sidebar supplied it. So `actionDef.actionName` reached no dispatcher: an author could put an action in the menu, watch it render with icon and label, and never learn that clicking it was a no-op. The framework's liveness ledger recorded this as the single gap in the AppSchema navigation surface. `useNavActionDispatch` resolves the nav item's actionName against `action` metadata at click time — the same source DeclaredActionsBar reads for a record toolbar — and dispatches the resolved def through useAction(). UnifiedSidebar passes it. No new provider: the sidebar already renders inside ConsoleShell's GlobalActionRuntimeProvider, so nav actions get the fully-wired console runner with its confirm/param/result/navigate dialogs. A declared `params` array becomes the runner's param-dialog input; the nav item's own `actionDef.params` rides as the value bag, so a menu entry can pre-fill the action it launches. Nav actions are inherently global — ActionNavItemSchema is strict with exactly { actionName, params? } and carries no objectName — so resolution is by name and no record context rides along. Behaviour change: a shell passing no `onAction` no longer renders action items at all, rather than rendering them dead. This mirrors the existing capability guards and makes the omission diagnosable — a missing prop now reads as "my item is gone", which leads to the prop, instead of "clicking does nothing", which for three releases led nowhere. Every dispatch-time failure warns and toasts instead of returning silently, since silence is the bug being fixed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01E5CYr5SDwe85gH2Jr5KSgu --- .changeset/nav-action-dispatch.md | 39 +++++ .../__tests__/useNavActionDispatch.test.tsx | 133 ++++++++++++++++++ packages/app-shell/src/hooks/index.ts | 1 + .../src/hooks/useNavActionDispatch.ts | 105 ++++++++++++++ packages/app-shell/src/index.ts | 1 + .../app-shell/src/layout/UnifiedSidebar.tsx | 7 + packages/layout/src/NavigationRenderer.tsx | 21 ++- .../NavigationRenderer.actions.test.tsx | 80 +++++++++++ 8 files changed, 386 insertions(+), 1 deletion(-) create mode 100644 .changeset/nav-action-dispatch.md create mode 100644 packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx create mode 100644 packages/app-shell/src/hooks/useNavActionDispatch.ts create mode 100644 packages/layout/src/__tests__/NavigationRenderer.actions.test.tsx diff --git a/.changeset/nav-action-dispatch.md b/.changeset/nav-action-dispatch.md new file mode 100644 index 000000000..53e6cfb53 --- /dev/null +++ b/.changeset/nav-action-dispatch.md @@ -0,0 +1,39 @@ +--- +"@object-ui/layout": minor +"@object-ui/app-shell": minor +--- + +Navigation `action` items actually run now (framework#4509). + +A `type: 'action'` nav item rendered, gated like any other item, and did +**nothing** when clicked. `NavigationRenderer` dispatches such a click to an +`onAction` prop it expects the host shell to supply — it deliberately never +reads `item.actionDef` itself — and no shipped sidebar supplied that prop. So +`actionDef.actionName` reached no dispatcher: an author could put an action in +the menu, watch it render with its icon and label, and never find out that +clicking it was a no-op. The framework's liveness ledger recorded this as the +single gap in the AppSchema navigation surface. + +**New `useNavActionDispatch`** (`@object-ui/app-shell`) resolves the nav item's +`actionName` against `action` metadata at click time — the same source +`DeclaredActionsBar` reads for a record toolbar — and dispatches the resolved +definition through `useAction()`. `UnifiedSidebar` now passes it. No new +provider is involved: the sidebar already renders inside `ConsoleShell`'s +`GlobalActionRuntimeProvider`, so nav actions get the fully-wired console runner +including the confirm, param-collection, result and navigate dialogs. A declared +`params` array becomes the runner's param-dialog input, and the nav item's own +`actionDef.params` is passed as the value bag, so a menu entry can pre-fill the +action it launches. + +Nav actions are inherently **global**: `ActionNavItemSchema` is strict with +exactly `{ actionName, params? }` and carries no `objectName`, so resolution is +by name alone and no record context rides along. + +**Behaviour change:** a shell that passes no `onAction` no longer renders +`action` items at all, instead of rendering them dead. This mirrors the existing +capability guards — an item the host cannot serve is hidden — and it makes the +omission diagnosable: a missing prop now shows up as "my action item is gone", +which leads to the prop, rather than "clicking does nothing", which for three +releases led nowhere. Every failure at dispatch time (an unnamed item, an +unresolvable action, a throwing action) warns and toasts instead of returning +silently. diff --git a/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx b/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx new file mode 100644 index 000000000..af5f3e6f6 --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx @@ -0,0 +1,133 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * useNavActionDispatch — nav `action` items reach the action runtime + * (framework#4509). + * + * The contract has three parts, and the third is the one the bug was made of: + * resolve `actionDef.actionName` against `action` metadata, dispatch the + * resolved def through the runner, and FAIL LOUDLY when either step comes up + * empty — a silent return would reproduce the dead click through a new route. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { renderHook, act } from '@testing-library/react'; + +const execute = vi.fn(); +const getItem = vi.fn(); +const toastError = vi.fn(); + +vi.mock('sonner', () => ({ toast: { error: (...a: unknown[]) => toastError(...a) } })); +vi.mock('@object-ui/react', () => ({ useAction: () => ({ execute }) })); +vi.mock('../../providers/MetadataProvider', () => ({ useMetadata: () => ({ getItem }) })); + +import { useNavActionDispatch } from '../useNavActionDispatch'; + +const navItem = (over: Record = {}) => ({ + id: 'nav_export', + type: 'action', + label: 'Export Data', + actionDef: { actionName: 'export_data' }, + ...over, +}) as never; + +async function dispatch(item: unknown) { + const { result } = renderHook(() => useNavActionDispatch()); + await act(async () => { + result.current(item as never); + // The handler is fire-and-forget by design (the renderer's onClick is + // synchronous); let its promise chain settle. + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +beforeEach(() => { + execute.mockReset().mockResolvedValue(undefined); + getItem.mockReset(); + toastError.mockReset(); + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +describe('useNavActionDispatch', () => { + it('resolves the action by name and dispatches the resolved definition', async () => { + getItem.mockResolvedValue({ name: 'export_data', type: 'api', target: '/exports', label: 'Export' }); + + await dispatch(navItem()); + + expect(getItem).toHaveBeenCalledWith('action', 'export_data'); + expect(execute).toHaveBeenCalledTimes(1); + const [dispatched] = execute.mock.calls[0]; + // The whole def rides the dispatch — the runner reads type/target/label. + expect(dispatched).toMatchObject({ name: 'export_data', type: 'api', target: '/exports' }); + }); + + it("moves a declared `params` ARRAY to `actionParams` (the runner's dialog input)", async () => { + getItem.mockResolvedValue({ + name: 'export_data', + type: 'api', + params: [{ name: 'format', type: 'text' }], + }); + + await dispatch(navItem()); + + const [dispatched] = execute.mock.calls[0]; + expect(dispatched.actionParams).toEqual([{ name: 'format', type: 'text' }]); + // `params` on the dispatch is the VALUE bag, not the param declarations — + // leaving the array there would make the runner treat it as values. + expect(Array.isArray(dispatched.params)).toBe(false); + }); + + it("passes the nav item's own actionDef.params through as the value bag", async () => { + getItem.mockResolvedValue({ name: 'export_data', type: 'api' }); + + await dispatch(navItem({ actionDef: { actionName: 'export_data', params: { format: 'csv' } } })); + + const [dispatched] = execute.mock.calls[0]; + expect(dispatched.params).toEqual({ format: 'csv' }); + }); + + it('warns and toasts — without dispatching — when the action is not defined', async () => { + getItem.mockResolvedValue(null); + + await dispatch(navItem()); + + expect(execute).not.toHaveBeenCalled(); + expect(toastError).toHaveBeenCalledTimes(1); + expect(String(toastError.mock.calls[0][0])).toContain('export_data'); + }); + + it('warns and toasts when the nav item names no action at all', async () => { + await dispatch(navItem({ actionDef: undefined })); + + expect(getItem).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + expect(toastError).toHaveBeenCalledTimes(1); + }); + + it('surfaces a resolution failure instead of swallowing it', async () => { + getItem.mockRejectedValue(new Error('offline')); + + await dispatch(navItem()); + + expect(execute).not.toHaveBeenCalled(); + expect(toastError).toHaveBeenCalledTimes(1); + }); + + it('surfaces a failure thrown by the action itself', async () => { + getItem.mockResolvedValue({ name: 'export_data', type: 'api' }); + execute.mockRejectedValue(new Error('boom')); + + await dispatch(navItem()); + + expect(toastError).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/app-shell/src/hooks/index.ts b/packages/app-shell/src/hooks/index.ts index 200f61c51..14eb43c98 100644 --- a/packages/app-shell/src/hooks/index.ts +++ b/packages/app-shell/src/hooks/index.ts @@ -1,6 +1,7 @@ export { useFavorites, type FavoriteItem } from './useFavorites'; export { useActionModal, type ModalDescriptor } from './useActionModal'; export { useMetadataService } from './useMetadataService'; +export { useNavActionDispatch } from './useNavActionDispatch'; export { useNavPins } from './useNavPins'; export { useNavigationSync, diff --git a/packages/app-shell/src/hooks/useNavActionDispatch.ts b/packages/app-shell/src/hooks/useNavActionDispatch.ts new file mode 100644 index 000000000..669261e43 --- /dev/null +++ b/packages/app-shell/src/hooks/useNavActionDispatch.ts @@ -0,0 +1,105 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * useNavActionDispatch — make `type: 'action'` navigation items actually run. + * + * ## The disconnect this closes (framework#4509) + * `NavigationRenderer` renders an `action` nav item, gates it like any other + * (permissions, capabilities, visibility), and dispatches the click to an + * `onAction` prop it expects the HOST to supply — it never reads + * `item.actionDef` itself. No shipped shell supplied that prop, so + * `actionDef.actionName` reached no dispatcher: the item rendered, looked + * enabled, and did nothing on click. The framework liveness ledger recorded it + * as the one gap in the AppSchema navigation walk. + * + * ## Resolution + * A nav item names an action (`actionDef.actionName`); it does not carry the + * definition. `ActionEngine.executeAction` only knows names that were + * registered with it, which a sidebar has no way to arrange — so the name is + * resolved against `action` metadata at click time, the same source + * `DeclaredActionsBar` reads for a record's toolbar. + * + * Note the scope: `ActionNavItemSchema` is `.strict()` with exactly + * `{ actionName, params? }` — there is no `objectName`, so a nav action is + * inherently a GLOBAL action (ADR-0110's `'*'` scope). Resolution is by name + * alone, and an object-scoped action is not addressable from a nav item. + * + * Dispatch goes through `useAction()`, which under `GlobalActionRuntimeProvider` + * (ConsoleShell) is the fully-wired console runner — api/flow/script handlers + * plus the confirm, param-collection, result and navigate dialogs. The sidebar + * already renders inside that provider, so no new wiring is needed. + * + * ## Failure is loud + * Every failure path warns and toasts rather than returning silently. Silence + * is precisely the bug being fixed: a nav item that names an action nobody + * defined should say so, not reproduce the dead click through a different + * route. + */ + +import { useCallback } from 'react'; +import { toast } from 'sonner'; +import { useAction } from '@object-ui/react'; +import type { NavigationItem } from '@object-ui/types'; +import { useMetadata } from '../providers/MetadataProvider'; + +type ActionDefLike = Record & { name?: string; params?: unknown }; + +export function useNavActionDispatch(): (item: NavigationItem) => void { + const { execute } = useAction(); + const { getItem } = useMetadata(); + + return useCallback((item: NavigationItem) => { + void (async () => { + const actionDef = (item as { actionDef?: { actionName?: string; params?: Record } }).actionDef; + const actionName = actionDef?.actionName; + + // The spec requires `actionDef` on this variant, but objectui's own + // NavigationItem type keeps it optional (the two shapes are reconciled by + // the navigation-spec-parity test, not by the compiler), so a nav item + // can reach here naming nothing. + if (!actionName) { + console.warn('[nav] action item has no actionDef.actionName — nothing to dispatch', item?.id); + toast.error('This menu item is not configured to run an action.'); + return; + } + + let def: ActionDefLike | null = null; + try { + def = (await getItem('action', actionName)) as ActionDefLike | null; + } catch (err) { + console.warn(`[nav] failed to resolve action '${actionName}'`, err); + toast.error(`Could not load action "${actionName}".`); + return; + } + + if (!def) { + console.warn(`[nav] action '${actionName}' is not defined — nav item ${item?.id} cannot run`); + toast.error(`Action "${actionName}" is not defined.`); + return; + } + + try { + // Same dispatch shape as DeclaredActionsBar: forward the whole def + // (type/target/confirmText/successMessage/…), and move a `params` + // ARRAY out of the way into `actionParams`, which is the runner's + // param-dialog input. `params` on the dispatch is the value bag, so the + // nav item's own `actionDef.params` lands there — that is how an author + // pre-fills an action from the menu entry. + const { params: declaredParams, ...rest } = def; + const dispatch: Record = { ...rest }; + const staticParams = Array.isArray(declaredParams) ? declaredParams : []; + if (staticParams.length > 0) dispatch.actionParams = staticParams; + if (actionDef?.params && typeof actionDef.params === 'object') { + dispatch.params = { ...(actionDef.params as Record) }; + } + // No `_rowRecord`: a sidebar click carries no record context. An action + // that interpolates `{id}` is object-scoped and was never addressable + // from a nav item anyway. + await execute(dispatch as never); + } catch (err) { + console.warn(`[nav] action '${actionName}' failed`, err); + toast.error(`Action "${actionName}" failed to run.`); + } + })(); + }, [execute, getItem]); +} diff --git a/packages/app-shell/src/index.ts b/packages/app-shell/src/index.ts index af2f2c179..933e95922 100644 --- a/packages/app-shell/src/index.ts +++ b/packages/app-shell/src/index.ts @@ -151,6 +151,7 @@ export type { export { useFavorites, useMetadataService, + useNavActionDispatch, useNavPins, useNavigationSync, NavigationSyncEffect, diff --git a/packages/app-shell/src/layout/UnifiedSidebar.tsx b/packages/app-shell/src/layout/UnifiedSidebar.tsx index f19fa4066..d6ef7586d 100644 --- a/packages/app-shell/src/layout/UnifiedSidebar.tsx +++ b/packages/app-shell/src/layout/UnifiedSidebar.tsx @@ -47,6 +47,7 @@ import { useAuth, useIsWorkspaceAdmin } from '@object-ui/auth'; import { useRecentItems } from '../hooks/useRecentItems'; import { useFavorites } from '../hooks/useFavorites'; import { useNavPins } from '../hooks/useNavPins'; +import { useNavActionDispatch } from '../hooks/useNavActionDispatch'; import { resolveI18nLabel, matchAppBySegment, appRouteSegment } from '../utils'; import { useObjectTranslation, useObjectLabel } from '@object-ui/i18n'; // useObjectLabel provides appLabel/appDescription for convention-based @@ -152,6 +153,11 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { const { context, currentAppName } = useNavigationContext(); const { user, activeOrganization } = useAuth(); const isWorkspaceAdmin = useIsWorkspaceAdmin(); + // `type: 'action'` nav items dispatch through here (framework#4509). The + // sidebar renders inside ConsoleShell's GlobalActionRuntimeProvider, so this + // resolves to the fully-wired console runner — confirm/param/result dialogs + // included — with no provider of its own. + const dispatchNavAction = useNavActionDispatch(); // Swipe-from-left-edge gesture to open sidebar on mobile React.useEffect(() => { @@ -464,6 +470,7 @@ export function UnifiedSidebar({ activeAppName }: UnifiedSidebarProps) { : resolveNavGroupLabel(activeApp.name, itemId, fallback) ) : undefined} resolveViewLabel={(objectName, viewName, fallback) => resolveNavViewLabel(objectName, viewName, fallback)} + onAction={dispatchNavAction} t={t} templateContext={{ currentUserId: user?.id ?? null, currentOrgId: activeOrganization?.id ?? null, contextValues }} /> diff --git a/packages/layout/src/NavigationRenderer.tsx b/packages/layout/src/NavigationRenderer.tsx index a5b9e48fd..dd725c1a5 100644 --- a/packages/layout/src/NavigationRenderer.tsx +++ b/packages/layout/src/NavigationRenderer.tsx @@ -122,7 +122,19 @@ export interface NavigationRendererProps { /** Optional runtime-capability checker for `requiresObject` / `requiresService` */ checkCapability?: CapabilityChecker; - /** Called when an `action`-type item is clicked */ + /** + * Called when an `action`-type item is clicked. + * + * A shell that renders `action` items MUST supply this: the renderer has no + * dispatcher of its own and never reads `item.actionDef` — unpacking + * `actionName` / `params` and invoking the action is the host's job (see + * `useNavActionDispatch` in `@objectstack/app-shell`). Omitting it does not + * degrade to an inert button; action items are **not rendered at all**, + * because a nav entry that looks clickable and silently does nothing is worse + * than an absent one (framework#4509 — every shipped sidebar omitted this + * prop, so `actionDef.actionName` reached no dispatcher and every such item + * dead-clicked). + */ onAction?: (item: NavigationItem) => void; // --- P1.7 Navigation Enhancements --- @@ -958,6 +970,13 @@ function NavigationItemRenderer({ // --- Action --- if (item.type === 'action') { + // No dispatcher, no button (framework#4509). This mirrors the capability + // guards above: an item the host cannot actually serve is hidden, not + // rendered dead. It also makes the omission visible to whoever adds a new + // shell — a missing `onAction` shows up as "my action item vanished", + // which leads to the prop, instead of "clicking does nothing", which for + // three releases led nowhere. + if (!onAction) return null; const Icon = resolveIcon(item.icon); const actionLabel = resolveLabel(item.label, tProp); return ( diff --git a/packages/layout/src/__tests__/NavigationRenderer.actions.test.tsx b/packages/layout/src/__tests__/NavigationRenderer.actions.test.tsx new file mode 100644 index 000000000..0c44a9ae8 --- /dev/null +++ b/packages/layout/src/__tests__/NavigationRenderer.actions.test.tsx @@ -0,0 +1,80 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `type: 'action'` navigation items — the framework#4509 dead-click. + * + * The renderer owns two halves of this contract and neither was covered: + * it must hand the WHOLE item to the host (it never unpacks `actionDef` + * itself), and it must not render an action item at all when no host handler + * exists — an enabled-looking button that silently does nothing is the bug. + */ + +import { describe, it, expect, vi } from 'vitest'; +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { NavigationItem } from '@object-ui/types'; +import { SidebarProvider } from '@object-ui/components'; +import { NavigationRenderer } from '../NavigationRenderer'; + +const actionItem: NavigationItem = { + id: 'nav_export', + type: 'action', + label: 'Export Data', + icon: 'download', + actionDef: { actionName: 'export_data', params: { format: 'csv' } }, +}; + +const objectItem: NavigationItem = { + id: 'nav_accounts', + type: 'object', + label: 'Accounts', + objectName: 'account', +}; + +function renderNav(items: NavigationItem[], props: Record = {}) { + return render( + + + + + , + ); +} + +describe('NavigationRenderer — action items', () => { + it('dispatches the whole item to onAction so the host can read actionDef', () => { + const onAction = vi.fn(); + renderNav([actionItem], { onAction }); + + fireEvent.click(screen.getByText('Export Data')); + + expect(onAction).toHaveBeenCalledTimes(1); + // The renderer deliberately does NOT unpack actionDef — resolving the name + // and invoking the action is the host's job, so the host needs the item. + const [dispatched] = onAction.mock.calls[0]; + expect(dispatched.id).toBe('nav_export'); + expect(dispatched.actionDef).toEqual({ actionName: 'export_data', params: { format: 'csv' } }); + }); + + it('renders nothing for an action item when the shell supplies no handler', () => { + // This is the framework#4509 regression guard: every shipped sidebar + // omitted `onAction`, so these items rendered, gated, and dead-clicked. + renderNav([actionItem]); + + expect(screen.queryByText('Export Data')).toBeNull(); + }); + + it('hides only the action item — the rest of the tree still renders', () => { + renderNav([objectItem, actionItem]); + + expect(screen.getByText('Accounts')).toBeTruthy(); + expect(screen.queryByText('Export Data')).toBeNull(); + }); +}); From 8d806cbb25cf4940706b5818b37f12f9fb7b2141 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 08:28:43 +0000 Subject: [PATCH 2/2] fix(app-shell): drop the dead initializer flagged by no-useless-assignment (#4509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `let def = null` is never read — both the try and the catch either assign or return — so ESLint's no-useless-assignment failed the Lint job. Declare without the initializer. Also removes an unused React import from the new hook test. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01E5CYr5SDwe85gH2Jr5KSgu --- .../app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx | 1 - packages/app-shell/src/hooks/useNavActionDispatch.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx b/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx index af5f3e6f6..6d2adb530 100644 --- a/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/useNavActionDispatch.test.tsx @@ -17,7 +17,6 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import React from 'react'; import { renderHook, act } from '@testing-library/react'; const execute = vi.fn(); diff --git a/packages/app-shell/src/hooks/useNavActionDispatch.ts b/packages/app-shell/src/hooks/useNavActionDispatch.ts index 669261e43..827a35c92 100644 --- a/packages/app-shell/src/hooks/useNavActionDispatch.ts +++ b/packages/app-shell/src/hooks/useNavActionDispatch.ts @@ -63,7 +63,7 @@ export function useNavActionDispatch(): (item: NavigationItem) => void { return; } - let def: ActionDefLike | null = null; + let def: ActionDefLike | null; try { def = (await getItem('action', actionName)) as ActionDefLike | null; } catch (err) {