fix(plugin-dashboard): a url header action goes through the action runner, and an omitted actionType runs as url (objectui#12275)

.changeset/12275-dashboard-header-action-runner.md

@@ -0,0 +1,17 @@
+---
+'@object-ui/plugin-dashboard': patch
+---
+
+A dashboard header action of type `url`, or of no declared type, now goes through the action runner, like every other header action (objectui#12275).
+
+The click used to navigate by a url check of its own and never reached the runner's `classifyNavigationTarget`, which decides what a click may open (ruling A+ on objectui#12191). So:
+
+- **A protocol-relative `//host` target is refused at the click.** It used to be opened with `location.assign`. A bare relative value such as `thanks` is refused too; it used to be pushed as a route. Under the console the runner's error toast reports the refusal; with no action provider the dashboard logs it.
+- **A `mailto:` target opens the mail client in place.** It used to throw in `history.pushState`.
+- **A header action with no `actionType` runs as a `url` action.** The contract and the dashboard's own action list read an omitted type as `url`, but the click dropped it with a "missing actionType" warning, so the button did nothing.
+- **An `https://` target opens the way every runner `url` action opens one**: through the host's navigation handler, which in the console opens it in a new tab. It used to replace the current page.
+- **An in-app path such as `/apps/...` still navigates in the app**: through the host's router under the console, and, with no action provider, by the same history push as before.
+
+Following a `url` header action raises no success toast, as before.
+
+Nothing is added to the package entry: no export, prop, type member or language-pack key.

packages/plugin-dashboard/src/DashboardRenderer.tsx

@@ -451,15 +451,25 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
     // name. `ActionDef.label` is a `string`, so the def carries the label
     // resolved against the active language, exactly as the button shows it.
     // Keyed on `language`, not on a memoised resolver's identity (AGENTS.md #10).
+    //
+    // `type` is the ONE reading of an omitted `actionType`: `url`, as the
+    // contract door reads it (objectui#12275). The click dispatches this def,
+    // so it never re-reads the authored `actionType`.
     const headerActionDefs = useMemo<ActionDef[]>(() => {
       const actions = schema.header?.actions ?? [];
       return actions.map((a) => {
         const label = pickLocalized(a.label, language) || undefined;
+        const type = (a.actionType as ActionDef['type']) || 'url';
         return {
           name: a.actionUrl || label,
-          type: (a.actionType as ActionDef['type']) || 'url',
+          type,
           target: a.actionUrl,
           label,
+          // Following a link is not a feedback-worthy action, so a `url` header
+          // action raises no success toast, as it raised none before it went
+          // through the runner (the html-tier anchor suppresses it the same
+          // way). The error toast stays: a refused target is reported.
+          ...(type === 'url' ? { toast: { showOnSuccess: false } } : {}),
         };
       });
     }, [schema.header?.actions, language]);
@@ -1208,32 +1218,39 @@ const DashboardRendererInner = forwardRef<HTMLDivElement, DashboardRendererProps
               {headerActions.map((action, i) => {
                 const Icon = resolveLucideIcon(action.icon);
                 const handleClick = async () => {
-                  const { actionType, actionUrl } = action;
-                  if (!actionType || !actionUrl) {
-                    console.warn('[DashboardRenderer] Header action missing actionType/actionUrl:', action);
+                  // Every header action goes through the ActionRunner, which
+                  // owns the type registry. This used to allow-list `modal` /
+                  // `script` only, so a `flow` header action (and `api` / `form`
+                  // / `navigation`) fell through to a warn and never dispatched —
+                  // a screen flow could not even be launched from a dashboard
+                  // (framework#3528). The runner reports an unknown type itself,
+                  // so there is nothing to second-guess here.
+                  //
+                  // `url` included (objectui#12275). It used to navigate here,
+                  // by a url grammar of its own: a protocol-relative `//host`
+                  // was assigned, `mailto:` threw in `pushState`, a bare
+                  // relative value was pushed as a route, and an omitted
+                  // `actionType` was dropped with a warning although the def
+                  // above reads it as `url`. The runner's `classifyNavigationTarget`
+                  // is the one answer to what a click may open (ruling A+ on
+                  // objectui#12191). ⛔ No scheme test of its own here.
+                  //
+                  // `actionUrl` is the name the def registered under.
+                  const result = await executeAction(action.actionUrl);
+                  if (!result?.success) {
+                    console.warn('[DashboardRenderer] action failed', result?.error);
                     return;
                   }
-                  if (actionType === 'url') {
-                    if (/^https?:\/\//.test(actionUrl) || actionUrl.startsWith('//')) {
-                      window.location.assign(actionUrl);
-                    } else {
-                      // SPA-friendly navigation: use history API + popstate so React Router picks it up.
-                      window.history.pushState({}, '', actionUrl);
-                      window.dispatchEvent(new PopStateEvent('popstate'));
-                    }
-                    return;
+                  // With no navigation handler (no `ActionProvider` above, so
+                  // no host router), the runner does not navigate to an
+                  // admitted in-app path: it hands it back as `redirect`.
+                  // Follow that hand-off for a `url` action, SPA-style, as this
+                  // click did before: history API + popstate so a router that
+                  // listens picks it up. `pushState` cannot leave the origin.
+                  if (result.redirect && headerActionDefs[i]?.type === 'url') {
+                    window.history.pushState({}, '', result.redirect);
+                    window.dispatchEvent(new PopStateEvent('popstate'));
                   }
-                  // Everything that is not a raw navigation goes through the
-                  // ActionRunner, which owns the type registry. This used to
-                  // allow-list `modal` / `script` only, so a `flow` header action
-                  // (and `api` / `form` / `navigation`) fell through to a warn and
-                  // never dispatched — a screen flow could not even be launched
-                  // from a dashboard (framework#3528). The runner reports an
-                  // unknown type itself, so there is nothing to second-guess here.
-                  // `actionUrl` is non-empty here (the guard above returns
-                  // otherwise), and it is the name the def registered under.
-                  const result = await executeAction(actionUrl);
-                  if (!result?.success) console.warn('[DashboardRenderer] action failed', result?.error);
                 };
                 return (
                   <Button key={i} variant="outline" size="sm" onClick={handleClick}>

packages/plugin-dashboard/src/__tests__/DashboardRenderer.headerActionUrl-12275.test.tsx

@@ -0,0 +1,236 @@
+/**
+ * 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.
+ */
+
+/**
+ * A dashboard header action of type `url`, or of no declared type, goes
+ * through the ActionRunner (objectui#12275).
+ *
+ * The click used to navigate by a url grammar of its own, and so bypassed the
+ * runner's `classifyNavigationTarget`, the one answer to what a click may open
+ * (ruling A+ on objectui#12191):
+ *
+ * - a protocol-relative `//host` was handed to `location.assign`;
+ * - `mailto:` was handed to `history.pushState`, which throws on it;
+ * - a bare relative value was pushed as a route;
+ * - an omitted `actionType`, which the contract and the renderer's own
+ *   `headerActionDefs` read as `url`, was dropped with a warning.
+ *
+ * The card's four pins, on a real render with the real runner, plus the
+ * in-app path the old `pushState` branch served:
+ *
+ * 1. `//cdn.example.com/x` is refused at the click;
+ * 2. `mailto:` opens (in place, as the runner opens it);
+ * 3. a bare relative value is refused;
+ * 4. an omitted-type header action with an `https://` target navigates;
+ * 5. an in-app `/apps/...` path still navigates in the app.
+ *
+ * Each runs twice: under an `ActionProvider` wired as the console wires it
+ * (`onNavigate` + `onToast`), and with no provider at all (a standalone embed,
+ * where the runner has no navigation handler and hands an in-app path back as
+ * `redirect`).
+ *
+ * "Refused" is read off the runner, not off prose: the error the dashboard
+ * surfaces is the one a bare runner answers for the same target, and that
+ * answer is a failure. No navigation sink is touched.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach, type Mock, type MockInstance } from 'vitest';
+import { render, screen, cleanup, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import type { DashboardComponentSchema } from '@object-ui/types';
+import { ActionRunner } from '@object-ui/core';
+import { ActionProvider } from '@object-ui/react';
+import { DashboardRenderer } from '../DashboardRenderer';
+
+const LABEL = 'Open';
+
+type HeaderAction = NonNullable<NonNullable<DashboardComponentSchema['header']>['actions']>[number];
+
+function dashboardWith(action: Omit<HeaderAction, 'label'>): DashboardComponentSchema {
+  return {
+    type: 'dashboard',
+    label: 'Ops',
+    widgets: [],
+    header: { actions: [{ label: LABEL, ...action }] },
+  } satisfies DashboardComponentSchema;
+}
+
+/** What a bare runner answers for a `url` action with this target. */
+async function runnerAnswer(target: string) {
+  return new ActionRunner({}).execute({ type: 'url', target });
+}
+
+let originalLocation: Location;
+let hrefSet: Mock<(url: string) => void>;
+let openSpy: MockInstance<typeof window.open>;
+let pushStateSpy: MockInstance<typeof window.history.pushState>;
+let warnSpy: MockInstance<typeof console.warn>;
+
+beforeEach(() => {
+  originalLocation = window.location;
+  hrefSet = vi.fn();
+  Object.defineProperty(window, 'location', {
+    writable: true,
+    // The runner opens `mailto:` by assigning `window.location.href`.
+    value: { ...originalLocation, set href(v: string) { hrefSet(v); } } as unknown as Location,
+  });
+  openSpy = vi.spyOn(window, 'open').mockImplementation(() => null);
+  pushStateSpy = vi.spyOn(window.history, 'pushState').mockImplementation(() => {});
+  warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+});
+
+afterEach(() => {
+  cleanup();
+  openSpy.mockRestore();
+  pushStateSpy.mockRestore();
+  warnSpy.mockRestore();
+  Object.defineProperty(window, 'location', { writable: true, value: originalLocation });
+});
+
+function renderUnderProvider(action: Omit<HeaderAction, 'label'>) {
+  const onNavigate = vi.fn();
+  const onToast = vi.fn();
+  render(
+    <ActionProvider onNavigate={onNavigate} onToast={onToast}>
+      <DashboardRenderer schema={dashboardWith(action)} />
+    </ActionProvider>,
+  );
+  return { onNavigate, onToast };
+}
+
+async function click() {
+  await userEvent.setup().click(screen.getByRole('button', { name: LABEL }));
+}
+
+/** No sink this header action could navigate through was touched. */
+function expectNoNavigation(onNavigate?: Mock) {
+  if (onNavigate) expect(onNavigate).not.toHaveBeenCalled();
+  expect(hrefSet).not.toHaveBeenCalled();
+  expect(openSpy).not.toHaveBeenCalled();
+  expect(pushStateSpy).not.toHaveBeenCalled();
+}
+
+const REFUSED: Array<[string, string]> = [
+  ['a protocol-relative //host', '//cdn.example.com/x'],
+  ['a bare relative value', 'thanks'],
+];
+
+describe('dashboard header url action, under an ActionProvider (objectui#12275)', () => {
+  it.each(REFUSED)('refuses %s at the click, with the runner\'s refusal', async (_what, target) => {
+    const expected = await runnerAnswer(target);
+    expect(expected.success).toBe(false);
+    expect(expected.error).toBeTruthy();
+
+    const { onNavigate, onToast } = renderUnderProvider({ actionUrl: target, actionType: 'url' });
+    await click();
+
+    await waitFor(() =>
+      expect(onToast).toHaveBeenCalledWith(expected.error, expect.objectContaining({ type: 'error' })),
+    );
+    expectNoNavigation(onNavigate);
+  });
+
+  it('opens mailto: in place, not through the router and not in a new tab', async () => {
+    const target = 'mailto:sales@example.com';
+    const { onNavigate, onToast } = renderUnderProvider({ actionUrl: target, actionType: 'url' });
+    await click();
+
+    await waitFor(() => expect(hrefSet).toHaveBeenCalledWith(target));
+    expect(hrefSet).toHaveBeenCalledTimes(1);
+    expect(onNavigate).not.toHaveBeenCalled();
+    expect(openSpy).not.toHaveBeenCalled();
+    expect(pushStateSpy).not.toHaveBeenCalled();
+    expect(onToast).not.toHaveBeenCalled();
+  });
+
+  it('navigates an omitted-type header action with an https:// target, as a url action', async () => {
+    const target = 'https://example.com/report';
+    const { onNavigate, onToast } = renderUnderProvider({ actionUrl: target });
+    await click();
+
+    await waitFor(() =>
+      expect(onNavigate).toHaveBeenCalledWith(target, expect.objectContaining({ external: true })),
+    );
+    expect(onNavigate).toHaveBeenCalledTimes(1);
+    // Following a link raises no success toast, as it raised none before.
+    expect(onToast).not.toHaveBeenCalled();
+    expect(warnSpy).not.toHaveBeenCalledWith(
+      expect.stringContaining('missing actionType'),
+      expect.anything(),
+    );
+  });
+
+  it('still navigates an in-app /apps/... path in the app, through the host router', async () => {
+    const target = '/apps/crm/dashboard/pipeline';
+    const { onNavigate, onToast } = renderUnderProvider({ actionUrl: target, actionType: 'url' });
+    await click();
+
+    await waitFor(() =>
+      expect(onNavigate).toHaveBeenCalledWith(target, expect.objectContaining({ external: false })),
+    );
+    expect(hrefSet).not.toHaveBeenCalled();
+    expect(openSpy).not.toHaveBeenCalled();
+    expect(pushStateSpy).not.toHaveBeenCalled();
+    expect(onToast).not.toHaveBeenCalled();
+  });
+});
+
+describe('dashboard header url action, with no ActionProvider (objectui#12275)', () => {
+  it.each(REFUSED)('refuses %s at the click, with the runner\'s refusal', async (_what, target) => {
+    const expected = await runnerAnswer(target);
+    expect(expected.success).toBe(false);
+
+    render(<DashboardRenderer schema={dashboardWith({ actionUrl: target, actionType: 'url' })} />);
+    await click();
+
+    await waitFor(() =>
+      expect(warnSpy).toHaveBeenCalledWith('[DashboardRenderer] action failed', expected.error),
+    );
+    expectNoNavigation();
+  });
+
+  it('opens mailto: in place', async () => {
+    const target = 'mailto:sales@example.com';
+    render(<DashboardRenderer schema={dashboardWith({ actionUrl: target, actionType: 'url' })} />);
+    await click();
+
+    await waitFor(() => expect(hrefSet).toHaveBeenCalledWith(target));
+    expect(hrefSet).toHaveBeenCalledTimes(1);
+    expect(openSpy).not.toHaveBeenCalled();
+    expect(pushStateSpy).not.toHaveBeenCalled();
+  });
+
+  it('navigates an omitted-type header action with an https:// target', async () => {
+    const target = 'https://example.com/report';
+    render(<DashboardRenderer schema={dashboardWith({ actionUrl: target })} />);
+    await click();
+
+    // With no host router the runner opens an external target itself.
+    await waitFor(() => expect(openSpy).toHaveBeenCalledWith(target, '_blank', 'noopener,noreferrer'));
+    expect(hrefSet).not.toHaveBeenCalled();
+    expect(pushStateSpy).not.toHaveBeenCalled();
+  });
+
+  it('follows the runner\'s redirect hand-off for an in-app /apps/... path, SPA-style', async () => {
+    const target = '/apps/crm/dashboard/pipeline';
+    const popstate = vi.fn();
+    window.addEventListener('popstate', popstate);
+    try {
+      render(<DashboardRenderer schema={dashboardWith({ actionUrl: target, actionType: 'url' })} />);
+      await click();
+
+      await waitFor(() => expect(pushStateSpy).toHaveBeenCalledWith({}, '', target));
+      expect(pushStateSpy).toHaveBeenCalledTimes(1);
+      expect(popstate).toHaveBeenCalledTimes(1);
+      expect(hrefSet).not.toHaveBeenCalled();
+      expect(openSpy).not.toHaveBeenCalled();
+    } finally {
+      window.removeEventListener('popstate', popstate);
+    }
+  });
+});

packages/plugin-dashboard/src/__tests__/DashboardRenderer.headerActions.test.tsx

@@ -15,7 +15,9 @@
  *    `api`, `form`, `navigation` — fell through to a `console.warn` and did
  *    nothing at all. A screen flow could not even be launched from a dashboard
  *    (framework#3528). The runner owns the type registry, so the renderer no
- *    longer second-guesses it.
+ *    longer second-guesses it. Since objectui#12275 that includes `url`, which
+ *    kept a url grammar of its own here until then; its navigation pins are in
+ *    `DashboardRenderer.headerActionUrl-12275.test.tsx`.
  *
  * 2. The header wrapper must cost zero pixels when every child is suppressed.
  *    It used to render whenever `header` was merely declared, while each child
@@ -53,7 +55,7 @@ function dashboardWith(actionType: string): DashboardComponentSchema {
 }
 
 describe('DashboardRenderer header actions', () => {
-  it.each(['flow', 'api', 'form'])('dispatches a %s header action to its handler', async (actionType) => {
+  it.each(['flow', 'api', 'form', 'url'])('dispatches a %s header action to its handler', async (actionType) => {
     const handler = vi.fn().mockResolvedValue({ success: true });
     render(
       <ActionProvider handlers={{ [actionType]: handler }}>
@@ -68,21 +70,6 @@ describe('DashboardRenderer header actions', () => {
     expect(handler.mock.calls[0][0]).toMatchObject({ type: actionType, target: 'convert_lead_wizard' });
   });
 
-  it('still navigates for a url header action instead of dispatching', async () => {
-    const handler = vi.fn();
-    const pushState = vi.spyOn(window.history, 'pushState');
-    render(
-      <ActionProvider handlers={{ url: handler }}>
-        <DashboardRenderer schema={dashboardWith('url')} />
-      </ActionProvider>,
-    );
-
-    await userEvent.setup().click(screen.getByRole('button', { name: /Convert Lead/i }));
-
-    await waitFor(() => expect(pushState).toHaveBeenCalled());
-    expect(handler).not.toHaveBeenCalled();
-    pushState.mockRestore();
-  });
 });
 
 /**