fix(components): resolve the declared action type for a bar-hosted action:icon

.changeset/6306-action-icon-type-resolution.md

@@ -0,0 +1,43 @@
+---
+'@object-ui/components': patch
+---
+
+An `action:icon` hosted by an `action:bar` now reaches its handler. It forwarded the
+COMPONENT id as the action type, so the click resolved nothing at all — no error, no
+toast, a button that silently did nothing (objectui#6306, the objectstack#2169 "Mark Done
+does nothing" shape).
+
+`action:bar` does not route members through `SchemaRenderer`. It pulls each member's
+renderer off the registry and RENAMES the declared type as it spreads it onto the child:
+`type` becomes the component id (`'action:icon'`) and the real declaration moves to
+`actionType`. `action:button` has always resolved that pair when it forwards
+(`schema.actionType || schema.type`); `action:icon` read `schema.type` alone and dropped
+`actionType` entirely. `ActionRunner.execute` resolves its handler from
+`action.type || action.actionType || action.name`, and `'action:icon'` binds no registered
+handler and no builtin — for a declaration carrying `target` rather than `endpoint` it does
+not reach the legacy `navigate`/`api` fallback either, so it fell through to
+`executeActionSchema` and the authored action never ran.
+
+**The bug was a function of the layout, not the declaration.** One authored action executed
+or did nothing depending on which `component` the host picked for it — the same asymmetry
+objectui#5493 fixed on this renderer for `onSuccess`, one key over.
+
+`check:action-forward-parity` could not have caught this and its green run was never
+evidence: `type` **is** in the forward whitelist, and that gate diffs key PRESENCE against
+the owed set. This is a wrong-VALUE defect behind a present key, a class the gate has no
+opinion on by construction. The existing icon coverage could not catch it either — it
+rendered `action:icon` bar members three times and asserted only `visible`/`enabled`, never
+that a click reached a handler, which is exactly how this shipped.
+
+Pinned by `action-bar-member-type-resolution.test.tsx`, which executes clicks rather than
+inspecting props. Every row that reads the icon member's zero renders a sibling
+`action:button` member of the SAME declaration in the SAME bar and reads its one first, so
+a zero cannot be "the harness never executed anything". One row registers a trap handler
+keyed on the component id, making the unfixed behaviour a positive artefact (the trap
+fires) rather than only a missing call. A standalone row stays green in both worlds on
+purpose: it refuses a "fix" written as `schema.actionType` alone, which would trade this
+defect for its mirror image on the surface where `type` IS the action type.
+
+Scope is this one renderer. `type: schema` appears in exactly two files under
+`renderers/action/` — `action-button.tsx` (already correct) and `action-icon.tsx`;
+`action:group` and `action:menu` compose their members differently and are untouched.

packages/components/src/renderers/action/__tests__/action-bar-member-type-resolution.test.tsx

@@ -0,0 +1,200 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#6306 — an `action:icon` hosted by an `action:bar` forwarded the
+ * COMPONENT id as the action type, so the click resolved no handler and
+ * nothing happened: no error, no toast, the objectstack#2169 "Mark Done does
+ * nothing" shape.
+ *
+ * ## The composition, and where the two leaves disagreed
+ *
+ * `action:bar` does not route members through `SchemaRenderer`. It pulls each
+ * member's renderer off the registry and RENAMES the declared type as it
+ * spreads (`action-bar.tsx`):
+ *
+ * type: componentType, // 'action:button' | 'action:icon' | …
+ * actionType: action.type, // the real action type ('api', 'script', …)
+ *
+ * So on this host path the member's own `schema.type` is the component id and
+ * `schema.actionType` is the declaration. `action:button` resolves the pair
+ * when it forwards (`schema.actionType || schema.type`); `action:icon` read
+ * `schema.type` alone and dropped `actionType` entirely, handing the runner
+ * `type: 'action:icon'`. `ActionRunner.execute` resolves the handler from
+ * `action.type || action.actionType || action.name`, and `'action:icon'` binds
+ * nothing: no registered handler, no builtin, and — for a declaration carrying
+ * `target` rather than `endpoint` — no legacy `navigate`/`api` fallback either.
+ * It falls through to `executeActionSchema` and the authored action never runs.
+ *
+ * ## Why these rows are readings and not a dead probe
+ *
+ * Every row that reads the icon member's ZERO renders a sibling `action:button`
+ * member of the SAME declaration in the SAME bar and reads its ONE first. A
+ * harness that executes nothing at all — an unmounted renderer, a member pushed
+ * into the overflow menu, an assertion racing the async `execute` — reports
+ * zero on both members, so the control tells the two apart from the failure
+ * message alone. The two members differ in exactly one authored key,
+ * `component`; `name`/`label` differ only because they are the addressing
+ * handles, and the runner never consults them here (`type` is always truthy on
+ * this path, so the `|| action.name` leg is unreachable).
+ *
+ * The `component id never reaches the runner` row makes the defect two-sided
+ * rather than merely absent: it registers a TRAP handler keyed on the component
+ * id itself, so the unfixed renderer produces a positive artefact (the trap
+ * fires) instead of only a missing call. Nothing about the production path
+ * changes — the trap only gives the wrong value somewhere to land.
+ *
+ * ## The standalone row is a guard, not a duplicate
+ *
+ * Rendered on its own, `action:icon` never sees an `actionType` — its own
+ * registry `inputs` declare `type` as the action type. That row is green in
+ * both worlds ON PURPOSE: it is what refuses a "fix" written as
+ * `schema.actionType` alone, which would trade this defect for the mirror one.
+ *
+ * Scope note: `type: schema` appears in exactly TWO files under
+ * `renderers/action/` — `action-button.tsx` and `action-icon.tsx`. `action:group`
+ * and `action:menu` compose their members differently and are not part of this
+ * defect; that census is a control here rather than an assumption.
+ */
+
+import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
+import { render, screen, waitFor, fireEvent } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+import { ComponentRegistry } from '@object-ui/core';
+import type { ActionContext, ActionDef, ActionResult } from '@object-ui/core';
+import { ActionProvider } from '@object-ui/react';
+// Module-scope side-effect imports so the three renderers are in the registry
+// when `ComponentRegistry.get` runs — the light `dom` project does not load the
+// `@object-ui/components` graph, and `action:bar` resolves its members through
+// the registry at render time. Module scope, not a `beforeAll`, per
+// AGENTS.md §测试纪律.
+import '../action-button';
+import '../action-icon';
+import '../action-bar';
+
+/**
+ * The authored declaration. `target` and NOT `endpoint` — deliberately: an
+ * `endpoint` would let the runner's legacy `action.api || action.endpoint`
+ * fallback reach `executeAPI` even with an unresolved type, which would mask
+ * exactly the defect under test.
+ */
+const DECLARATION = {
+ type: 'api',
+ target: '/api/v1/tasks/mark_done',
+} as const;
+
+/** One declaration, mounted twice; `component` is the only variable. */
+const iconMember = { ...DECLARATION, name: 'mark_done_icon', label: 'Mark done icon', component: 'action:icon' };
+const buttonMember = { ...DECLARATION, name: 'mark_done_button', label: 'Mark done button', component: 'action:button' };
+
+let api: Mock<(action: ActionDef, ctx: ActionContext) => Promise<ActionResult>>;
+/** Keyed on the COMPONENT id — fires only if the unresolved type reaches the runner. */
+let trap: Mock<(action: ActionDef, ctx: ActionContext) => Promise<ActionResult>>;
+
+beforeEach(() => {
+ api = vi.fn(async () => ({ success: true }));
+ trap = vi.fn(async () => ({ success: true }));
+});
+
+/**
+ * The real `action:bar` host. BOTH ceilings are pinned high: the inline/overflow
+ * split reads `mobileMaxVisible ?? 1` when `useIsMobile()` is true, so pinning
+ * only `maxVisible` would leave the split at the mercy of the environment's
+ * viewport and could push a member into the overflow menu — a zero that is not
+ * about type resolution at all.
+ */
+function renderBar(handlers: Record<string, Mock<(a: ActionDef, c: ActionContext) => Promise<ActionResult>>>) {
+ const Bar = ComponentRegistry.get('action:bar');
+ if (!Bar) throw new Error('action:bar is not registered');
+ return render(
+ <ActionProvider handlers={handlers} onToast={vi.fn()}>
+ <Bar
+ schema={{
+ type: 'action:bar',
+ maxVisible: 10,
+ mobileMaxVisible: 10,
+ actions: [buttonMember, iconMember],
+ } as never}
+ />
+ </ActionProvider>,
+ );
+}
+
+const clickMember = (label: string) => fireEvent.click(screen.getByRole('button', { name: label }));
+
+/** The def the handler was handed — i.e. what actually reached the runner. */
+const defOf = (m: Mock<(a: ActionDef, c: ActionContext) => Promise<ActionResult>>, i = 0) =>
+ m.mock.calls[i][0];
+
+describe('action:bar member type resolution — action:icon (objectui#6306)', () => {
+ it('positive control — the action:button member reaches the api handler', async () => {
+ renderBar({ api });
+
+ clickMember('Mark done button');
+
+ await waitFor(() => expect(api).toHaveBeenCalledTimes(1));
+ expect(defOf(api).type).toBe('api');
+ });
+
+ it('the action:icon member of the same bar reaches the same handler', async () => {
+ renderBar({ api });
+
+ // The control first, in the same render: its ONE is what makes the icon
+ // member's count a reading rather than "the harness never executed".
+ clickMember('Mark done button');
+ await waitFor(() => expect(api).toHaveBeenCalledTimes(1));
+
+ clickMember('Mark done icon');
+ await waitFor(() => expect(api).toHaveBeenCalledTimes(2));
+ expect(defOf(api, 1).type).toBe('api');
+ });
+
+ it('the runner is handed the declared type, not the component id', async () => {
+ renderBar({ api, 'action:icon': trap });
+
+ clickMember('Mark done icon');
+
+ // Settle on EITHER path before reading, so the row is about WHICH handler
+ // resolved and never about async timing. Asserting the trap FIRST is what
+ // makes the unfixed world produce a naming artefact ("expected trap not to
+ // be called, but it was") instead of a generic missing call.
+ await waitFor(() => expect(api.mock.calls.length + trap.mock.calls.length).toBe(1));
+ expect(trap).not.toHaveBeenCalled();
+ expect(api).toHaveBeenCalledTimes(1);
+ expect(defOf(api).type).toBe('api');
+ });
+
+ it('both members of one declaration resolve to the same action type', async () => {
+ renderBar({ api });
+
+ clickMember('Mark done button');
+ clickMember('Mark done icon');
+
+ await waitFor(() => expect(api).toHaveBeenCalledTimes(2));
+ expect(api.mock.calls.map(([def]) => def.type)).toEqual(['api', 'api']);
+ });
+
+ it('regression guard — a standalone action:icon still resolves its own declared type', async () => {
+ // No host, so no `actionType` at all: `type` IS the action type here, as
+ // this renderer's own registry `inputs` declare. Green before and after the
+ // fix on purpose — it refuses a rewrite that reads `actionType` alone.
+ const Icon = ComponentRegistry.get('action:icon');
+ if (!Icon) throw new Error('action:icon is not registered');
+ render(
+ <ActionProvider handlers={{ api }} onToast={vi.fn()}>
+ <Icon schema={{ ...DECLARATION, name: 'standalone', label: 'Standalone icon' } as never} />
+ </ActionProvider>,
+ );
+
+ clickMember('Standalone icon');
+
+ await waitFor(() => expect(api).toHaveBeenCalledTimes(1));
+ expect(defOf(api).type).toBe('api');
+ });
+});

packages/components/src/renderers/action/action-icon.tsx

@@ -32,10 +32,13 @@ import { hasDeclaredVisibilityGate } from './visibility-gate';
  * `locations`, `enabled` and `size` are all modern-only keys this renderer
  * forwards, and the legacy `crud.ts` `ActionSchema` is `@deprecated` and pins
  * `type: 'action'` where this renderer's own registry `inputs` declare
- * `'script' | 'url' | 'modal' | 'flow' | 'api'`.
+ * `'script' | 'url' | 'modal' | 'flow' | 'api'`. `actionType` stays on the
+ * intersection for the same reason it does on `action:button`: it is the
+ * host-composed override this renderer reads FIRST
+ * (`schema.actionType || schema.type`), and it is not a `UIActionSchema` key.
  */
 export interface ActionIconProps {
- schema: UIActionSchema & { type: string; className?: string };
+ schema: UIActionSchema & { type: string; className?: string; actionType?: string };
  className?: string;
  context?: Record<string, any>;
  [key: string]: any;
@@ -96,7 +99,18 @@ const ActionIconRenderer = forwardRef<
  // (objectui#4281). `...localContext` is still merged last, so the object
  // reaching `execute` is unchanged.
  const forwarded: ActionDef = {
- type: schema.type,
+ // The host path renames the declared type (objectui#6306). `action:bar`
+ // does not route members through `SchemaRenderer` — it spreads the
+ // member onto this renderer's schema as `type: componentType,
+ // actionType: action.type`, so on that path `schema.type` is the
+ // COMPONENT id and `schema.actionType` is the declaration. Reading
+ // `schema.type` alone handed the runner `type: 'action:icon'`, which
+ // `ActionRunner.execute` resolves to no handler and no builtin: the
+ // click did nothing, with no error and no toast. Same resolution
+ // `action:button` has always had; the `|| schema.type` leg is what
+ // keeps the STANDALONE surface (where `type` IS the action type, as
+ // this renderer's own registry `inputs` declare) working.
+ type: schema.actionType || schema.type,
  name: schema.name,
  // See action-button.tsx — the param-collection dialog reads its title
  // and description off these (objectui#4192, measured on `action:menu`