refactor(actions): navigation becomes a declared alias of url, sharing one navigator (#2944) (#2994)

The last open item of #2944. `ActionRunner` dispatched a seventh action type,
`navigation`, that `@objectstack/spec`'s `ActionType` does not contain. The issue
asked to promote it upstream or delete the case. Neither, as stated.

Promoting is wrong: the spec already names this operation — `type: 'url'`, with
`openIn` for the new-tab choice — so a seventh type would put a second spec name
on one operation, which is the failure this audit exists to describe, committed
one level up. Deleting is worse because it is silent: `{ type: 'navigation',
to: … }` is authored today (`element:button` CTAs), and without the case the
action falls through to `executeActionSchema`, which returns `{ success: true }`
— a green toast that navigates nowhere (#2960's trap).

The alias had already drifted, which is the actual finding. `executeNavigation`
was quietly the weaker of two implementations of one concept: no `${param.X}`
interpolation, `openIn` ignored, no `/api/…` full-page short-circuit. So
`{ type: 'navigation', to: '/x?p=${param.p}' }` shipped the literal
`${param.p}` while the identical `url` action resolved it.

So it stays and stops being dialect. `ObjectUiLocalActionType` /
`OBJECTUI_LOCAL_ACTION_TYPES` declare it as objectui's own alias of `url` — the
treatment #2985 gave the page-visualization names — and both names now route
through one `navigateTo`. `url` gains `replace` pass-through, the one modifier
only the alias carried; `replace` is omitted from the `NavigationHandler`
options when unset, so hosts see the shape they already saw.

The guard is structural rather than another assertion: built-in dispatch is a
table typed `Record<RunnableActionType, …>` instead of a `switch`, so an
`ActionType` the spec ADDS stops compiling until an executor exists for it —
#2942's "validates at save, renders nothing at run time" class, made a build
error for actions. `spec-derived-unions.test.ts` additionally asserts
`navigation` is absent from the spec enum, so the day it is adopted upstream the
test fails and names the alias to retire.

Also covers the `/api/` short-circuit and `apiBase` promotion, which had none —
the project runs in a node env with no `window`, which is why — and corrects the
audit doc's Direction B row, which still prescribed promoting.

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: os-zhuang

.changeset/navigation-action-alias.md

@@ -0,0 +1,44 @@
+---
+"@object-ui/types": minor
+"@object-ui/core": minor
+---
+
+refactor(actions): `navigation` becomes a named alias of the spec's `url`, sharing one navigator (#2944)
+
+The last open item of #2944: `ActionRunner` dispatched a seventh action type,
+`navigation`, that `@objectstack/spec`'s `ActionType` does not contain. The issue
+asked for a decision — promote it upstream or delete the case. Neither, as stated.
+
+- **Promoting it is wrong.** The spec already has `url` for "go to a location",
+  with `openIn` for the new-tab/same-tab choice. A seventh type would put a
+  second spec name on one operation, which is the exact failure the #2901 audit
+  is named after: *a second definition of the vocabulary exists, and the renderer
+  is faithful to the wrong one*.
+- **Deleting it is worse, because it is silent.** `{ type: 'navigation', to: … }`
+  is authored today (`element:button` CTAs). Without the case the action falls
+  through to `executeActionSchema`, which returns `{ success: true }` — a green
+  toast that navigates nowhere. That is #2960's trap.
+
+So it stays, but stops being dialect. `ObjectUiLocalActionType` /
+`OBJECTUI_LOCAL_ACTION_TYPES` in `@object-ui/types` declare it as objectui's own
+alias of `url` — the same treatment #2985 gave `PageVisualizationAlias` — and the
+runner routes both names through one navigator.
+
+**The alias had already drifted, which is the point.** `executeNavigation` was
+quietly the weaker of the two implementations: no `${param.X}` / `${ctx.X}`
+interpolation, `openIn` ignored, and no `/api/…` full-page short-circuit (the
+redirect-dance case `url` exists to handle). An author who wrote
+`{ type: 'navigation', to: '/x?p=${param.p}' }` shipped the literal `${param.p}`,
+while the identical `url` action resolved it. Both names now behave identically;
+`url` in turn gains `replace` pass-through, the one modifier only the alias had.
+
+Additive only. `replace` is omitted from the `NavigationHandler` options object
+when unset, so hosts see the option shape they already saw.
+
+The new guard is structural rather than another assertion. The runner's built-in
+dispatch is a table typed `Record<RunnableActionType, …>` instead of a `switch`,
+so an `ActionType` the spec **adds** stops compiling until an executor exists for
+it — the Tier-2 "validates at save, renders nothing at run time" failure (#2942)
+becomes a build error for actions. `spec-derived-unions.test.ts` additionally
+asserts `navigation` is *absent* from the spec enum, so the day it is adopted
+upstream, the test fails and names the alias to retire.

docs/audits/2901-spec-enum-renderer-coverage.md

@@ -2,7 +2,7 @@
 
 Audited against `@objectstack/spec@16.0.0-rc.0` and `objectui@cd09a7b90`.
 **Re-validated against `17.0.0-rc.0`** — see the addendum at the end, which also
-carries three corrections. Read it before acting on any row below.
+carries four corrections. Read it before acting on any row below.
 
 Every row below was confirmed by reading the dispatch code. Nothing was exercised at
 runtime, so symptoms are read off the code path, not observed. Rows the auditors could
@@ -165,7 +165,7 @@ value corruption rather than disclosure.
 
 | Name | Location | Call |
 |---|---|---|
-| `navigation` action type | [ActionRunner.ts:539](../../packages/core/src/actions/ActionRunner.ts) | Fully implemented (`executeNavigation`). **Promote** — it is a real capability the spec doesn't name. |
+| `navigation` action type | [ActionRunner.ts](../../packages/core/src/actions/ActionRunner.ts) | ~~Promote~~ — **resolved as a declared alias instead.** See the correction below. |
 | `combo` chart type | [normalizeChartSchema.ts:63](../../packages/plugin-charts/src/normalizeChartSchema.ts) | Drawn at `AdvancedChartImpl.tsx:819`. Promote or delete. |
 | `system` theme mode | ThemeProvider ×2 | Delete — the spec name is `auto`, and `react/ThemeContext.tsx` already gets it right. |
 | `scale-fade` preset | [useAnimation.ts:71](../../packages/react/src/hooks/useAnimation.ts) | Delete with the hyphen/underscore fix. |
@@ -358,6 +358,33 @@ On a list whose only narrowing is that filter, the user sees rows they had asked
 to exclude. It is not a permission bypass — row-scoping `$and`-composes as a
 separate arm and survives — but it ranks with Tier 1, not Tier 2.
 
+## Correction 4 — "promote `navigation` upstream" was the wrong call
+
+Direction B told the reader to promote the `navigation` action type into the spec,
+on the grounds that it is "a real capability the spec doesn't name". The capability
+is real; the framing was not. The spec already names it — `type: 'url'`, with
+`openIn` for the new-tab choice — so promoting `navigation` would have put a second
+spec name on one operation. That is the failure this audit exists to describe,
+committed one level up.
+
+The row *also* under-read the cost of the stated alternative. Deleting the case
+does not surface an error: the action falls through to `executeActionSchema`, which
+returns `{ success: true }`, so the user gets a success toast and no navigation —
+#2960's trap, reached by way of a cleanup.
+
+What was missing from the row is that the alias had **already drifted**, which is
+the ordinary outcome of two implementations of one concept. `executeNavigation` did
+no `${param.X}` interpolation, ignored `openIn`, and skipped the `/api/…`
+full-page short-circuit — so `{ type: 'navigation', to: '/x?p=${param.p}' }`
+shipped the literal `${param.p}` while the identical `url` action resolved it.
+
+Resolved by declaring it (`ObjectUiLocalActionType`) and routing both names
+through one navigator — the same treatment #2985 gave the page-visualization names.
+Generalizing: a "renderer-local dialect" row needs a third option next to promote
+and delete — **alias to the spec name it duplicates** — and picking between the
+three starts with asking whether the spec really lacks the concept or just spells
+it differently.
+
 ## Status of the items above
 
 | Item | Status |
@@ -366,7 +393,9 @@ separate arm and survives — but it ranks with Tier 1, not Tier 2.
 | `before`/`after`, `'not in'`, + 8 legacy spellings | Fixed — #2974 |
 | `DensityModeSchema` inert | Resolved upstream (removed in spec 17) |
 | Packages lacking the `@objectstack/spec` devDep | **6** — `plugin-list` and `data-objectstack` gained it in #2974; `plugin-charts`, `plugin-dashboard`, `plugin-report`, `components`, `mobile` and `fields` remain. (The body above says 6 because it counted a 13-package "needs a guard" list that omitted `fields`; adding `fields` and removing the two now fixed leaves 6 either way.) |
-| Everything else | Open — #2941 (Tier 1), #2942 (Tier 2), #2943 (Tier 3), #2944 (forks), #2945 (vocabularies) |
+| The five shadowing forks | Fixed — #2985 (closes #2944) |
+| `navigation` action type | Resolved as a declared alias, not promoted — see Correction 4 |
+| Everything else | Open — #2941 (Tier 1), #2942 (Tier 2), #2943 (Tier 3), #2945 (vocabularies) |
 
 One method note for anyone repeating this: the original run read a checkout that
 was behind `origin/main`, which produced a spurious "objectui resolves two spec

packages/core/src/actions/ActionRunner.ts

@@ -10,11 +10,18 @@
  * @object-ui/core - Action Runner
  * 
  * Executes actions defined in ActionSchema and EventHandler.
- * Supports all spec v2.0.1 action types: script, url, modal, flow, api.
+ *
+ * Dispatches every `@objectstack/spec` `ActionType` — `script`, `url`, `modal`,
+ * `flow`, `api`, `form` — plus the one declared renderer-local alias,
+ * `navigation` (see `ObjectUiLocalActionType`). The table that does it is typed
+ * against that vocabulary, so this list cannot go stale the way the old comment
+ * had (it still claimed five types after `form` shipped).
+ *
  * Features: conditional execution, confirmation, toast notifications,
  * redirect handling, action chaining, custom handler registration.
  */
 
+import type { RunnableActionType } from '@object-ui/types';
 import { ExpressionEvaluator } from '../evaluator/ExpressionEvaluator';
 import { globalUndoManager, type UndoableOperation } from './UndoManager';
 
@@ -76,7 +83,8 @@ export interface ApiConfig {
  * Compatible with both UIActionSchema (spec v2.0.1) and legacy crud.ts ActionSchema.
  */
 export interface ActionDef {
-  /** Action type identifier: 'script' | 'url' | 'modal' | 'flow' | 'api' | 'navigation' | custom */
+  /** Action type identifier — a `RunnableActionType` (the spec's six plus the
+   *  `navigation` alias of `url`) or a custom type with a registered handler. */
   type?: string;
   /** Legacy action type field */
   actionType?: string;
@@ -359,6 +367,33 @@ export class ActionRunner {
   private paramCollectionHandler: ParamCollectionHandler | null;
   private resultDialogHandler: ResultDialogHandler | null;
 
+  /**
+   * Built-in dispatch, one entry per runnable action type.
+   *
+   * A table rather than a `switch` because the type annotation is the guard:
+   * `Record<RunnableActionType, …>` stops compiling the moment
+   * `@objectstack/spec` adds an `ActionType` this runner has no executor for.
+   * That is the whole Tier-2 failure class for actions — a spec name that
+   * validates at save and then does nothing at run time (#2942) — converted
+   * into a build error. It also rejects the reverse: an executor keyed by a
+   * name that is neither in the spec nor a declared local alias.
+   *
+   * `navigation` is that declared alias (see `ObjectUiLocalActionType`); it
+   * shares `url`'s navigator.
+   */
+  private readonly builtinExecutors: Record<
+    RunnableActionType,
+    (action: ActionDef) => Promise<ActionResult>
+  > = {
+    script: (action) => this.executeScript(action),
+    url: (action) => this.executeUrl(action),
+    modal: (action) => this.executeModal(action),
+    flow: (action) => this.executeFlow(action),
+    api: (action) => this.executeAPI(action),
+    form: (action) => this.executeForm(action),
+    navigation: (action) => this.executeNavigation(action),
+  };
+
   constructor(context: ActionContext = {}) {
     this.context = withIdentityAlias(context);
     this.evaluator = new ExpressionEvaluator(this.context);
@@ -520,43 +555,24 @@ export class ActionRunner {
         return result;
       }
 
-      // Built-in action execution by type
+      // Built-in action execution by type.
       let result: ActionResult;
-
-      switch (actionType) {
-        case 'script':
-          result = await this.executeScript(action);
-          break;
-        case 'url':
-          result = await this.executeUrl(action);
-          break;
-        case 'modal':
-          result = await this.executeModal(action);
-          break;
-        case 'flow':
-          result = await this.executeFlow(action);
-          break;
-        case 'api':
-          result = await this.executeAPI(action);
-          break;
-        case 'form':
-          result = await this.executeForm(action);
-          break;
-        case 'navigation':
-          result = await this.executeNavigation(action);
-          break;
-        default:
-          // Legacy fallback: check for navigate, api, or onClick
-          if (action.navigate) {
-            result = await this.executeNavigation(action);
-          } else if (action.api || action.endpoint) {
-            result = await this.executeAPI(action);
-          } else if (action.onClick) {
-            await action.onClick();
-            result = { success: true };
-          } else {
-            result = await this.executeActionSchema(action);
-          }
+      const builtin = actionType
+        ? this.builtinExecutors[actionType as RunnableActionType]
+        : undefined;
+
+      if (builtin) {
+        result = await builtin(action);
+      } else if (action.navigate) {
+        // Legacy fallback: check for navigate, api, or onClick
+        result = await this.executeNavigation(action);
+      } else if (action.api || action.endpoint) {
+        result = await this.executeAPI(action);
+      } else if (action.onClick) {
+        await action.onClick();
+        result = { success: true };
+      } else {
+        result = await this.executeActionSchema(action);
       }
 
       await this.handlePostExecution(action, result);
@@ -797,13 +813,34 @@ export class ActionRunner {
     if (!rawUrl) {
       return { success: false, error: 'No URL provided for url action' };
     }
+    return this.navigateTo(action, rawUrl, action);
+  }
 
+  /**
+   * The one navigator behind `type: 'url'` and its `navigation` alias.
+   *
+   * Both names mean "go to a location", and they had two implementations that
+   * drifted: only this one interpolated `${param.X}`, promoted `/api/…` to the
+   * `apiBase`, short-circuited a redirect-dance URL to a full-page load, and
+   * honoured `openIn`. A `navigation` action was quietly the weaker of the two
+   * (#2944 item 3). Sharing the body is what keeps the alias honest.
+   *
+   * `action` supplies the interpolation context (`params`) and `openIn`;
+   * `source` supplies the navigation modifiers, which live on the nested
+   * `action.navigate` block for the legacy shape and on the action itself
+   * otherwise.
+   */
+  private async navigateTo(
+    action: ActionDef,
+    rawUrl: unknown,
+    source: ActionDef,
+  ): Promise<ActionResult> {
     // Apply target ${param.X} / ${ctx.X} interpolation FIRST — the
     // ExpressionEvaluator that follows would otherwise see unresolved
     // `param.provider` and either error or substitute undefined. Doing it
     // here also URL-encodes values (required for query-position params
     // like `?provider=foo+bar`).
-    const interpolated = this.interpolateTarget(rawUrl, action);
+    const interpolated = this.interpolateTarget(rawUrl as string, action);
     let url = this.evaluator.evaluate(interpolated) as string;
 
     if (!this.isValidUrl(url)) {
@@ -824,7 +861,10 @@ export class ActionRunner {
       url = `${apiBase}${url}`;
     }
 
-    const isExternal = url.startsWith('http://') || url.startsWith('https://');
+    // `source.external` is the `navigation` shape's explicit override; the
+    // scheme heuristic covers every other case.
+    const isExternal =
+      Boolean(source.external) || url.startsWith('http://') || url.startsWith('https://');
     // Same-origin API endpoints (most commonly the auth provider's
     // `/api/v1/auth/sign-in/social` redirect dance) issue server-side
     // 302s that must be followed by the *browser*, not the SPA router.
@@ -842,15 +882,25 @@ export class ActionRunner {
       return { success: true };
     }
     // `openIn` is the first-class, declarative switch (ActionSchema.openIn);
-    // it wins over the legacy `params.newTab` escape hatch and the
-    // external-URL heuristic.
+    // it wins over the legacy `navigate.newTab` / `params.newTab` escape
+    // hatches and the external-URL heuristic.
+    const openIn = source.openIn ?? action.openIn;
     const newTab =
-      action.openIn === 'new-tab' ? true
-        : action.openIn === 'self' ? false
-          : (action.params?.newTab ?? isExternal);
+      openIn === 'new-tab' ? true
+        : openIn === 'self' ? false
+          : (source.newTab ?? action.params?.newTab ?? isExternal);
+
+    // History semantics, the one thing the `navigation` shape carries that
+    // `ActionSchema` has no field for. Omitted from the handler call when
+    // unset so hosts see the same options object they always did.
+    const replace = source.replace ?? action.replace;
 
     if (this.navigationHandler) {
-      this.navigationHandler(url, { external: isExternal, newTab });
+      this.navigationHandler(url, {
+        external: isExternal,
+        newTab,
+        ...(replace === undefined ? {} : { replace: Boolean(replace) }),
+      });
       return { success: true };
     }
 
@@ -926,9 +976,6 @@ export class ActionRunner {
     return result;
   }
 
-  /**
-   * Execute navigation action
-   */
   /**
    * `form` action — open a FormView as a routed page (`/forms/:name`, per the
    * spec). `target` is the FormView name. Without this case the action fell
@@ -954,37 +1001,23 @@ export class ActionRunner {
     return { success: true, redirect: to };
   }
 
+  /**
+   * `navigation` — objectui's alias of the spec's `url` (`ObjectUiLocalActionType`).
+   *
+   * Kept because `{ type: 'navigation', to: … }` is authored today
+   * (`element:button` CTAs) and because deleting the case would not fail loudly:
+   * the action would land in `executeActionSchema` and report success while
+   * navigating nowhere. All it does now is resolve the alias's own spelling of
+   * the target and hand off to the shared navigator, so the two names cannot
+   * drift apart again. Prefer `type: 'url'` + `openIn` in new metadata.
+   */
   private async executeNavigation(action: ActionDef): Promise<ActionResult> {
-    const nav = action.navigate || action;
-    const to = this.evaluator.evaluate(nav.to || nav.target) as string;
-
-    if (!this.isValidUrl(to)) {
-      return {
-        success: false,
-        error: 'Invalid URL scheme. Only http://, https://, and relative URLs are allowed.',
-      };
-    }
-
-    const isExternal = nav.external || (typeof to === 'string' && (
-      to.startsWith('http://') || to.startsWith('https://')
-    ));
-
-    if (this.navigationHandler) {
-      this.navigationHandler(to, {
-        external: isExternal,
-        newTab: nav.newTab ?? isExternal,
-        replace: nav.replace,
-      });
-      return { success: true };
-    }
-
-    if (isExternal) {
-      window.open(to, '_blank', 'noopener,noreferrer');
-    } else {
-      return { success: true, redirect: to };
+    const nav = (action.navigate || action) as ActionDef;
+    const rawUrl = nav.to || nav.target || nav.redirect;
+    if (!rawUrl) {
+      return { success: false, error: 'No URL provided for navigation action' };
     }
-
-    return { success: true };
+    return this.navigateTo(action, rawUrl, nav);
   }
 
   /**