fix(app-shell)!: a modal action is client-side only — drop the server fallthrough (objectstack#3959) (#2973)

modalActionHandler fell through to serverActionHandler when the action's
target resolved to neither a page nor an object, documented as 'how a modal
action bound to engine.registerAction(...) still runs'. It never ran: the
framework's headlessActionTypeError rejects type:'modal' over REST with a 400,
since a modal action has no server dispatch. The fallthrough only converted an
authoring mistake into a confusing round-trip, and it let apps ship handlers
no declaration could address.

An unresolvable target is now reported as such, naming the action, the dud
target and the way out. To collect input and run server-side, declare
type:'script' with params.

Pairs with objectstack#3974 (merged).

Author: os-zhuang

.changeset/modal-actions-are-client-only.md

@@ -0,0 +1,19 @@
+---
+'@object-ui/app-shell': major
+---
+
+**[objectstack#3959] A `type: 'modal'` action is client-side only — the server fallthrough is removed.**
+
+`modalActionHandler` fell through to `serverActionHandler` when the action's
+target resolved to neither a page nor an object, documented as "how a modal
+action bound to `engine.registerAction(...)` still runs". It never ran: the
+framework's `headlessActionTypeError` rejects `type: 'modal'` over REST with a
+400, because a modal action has no server dispatch. The fallthrough only turned
+an authoring mistake — a target naming no page — into a confusing round-trip,
+and it let apps ship handlers that no declaration could address (app-todo's
+`deferTask` / `setReminder` sat dead for exactly this reason).
+
+An unresolvable target is now reported as what it is, naming the action, the
+dud target, and the way out. To collect input and then run server-side,
+declare `type: 'script'` with `params` — the runner collects the same dialog
+and the handler runs with those values.

packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx

@@ -463,7 +463,7 @@ describe('useConsoleActionRuntime — authenticated handlers', () => {
  * mounted it. Both now run this rule: render `target` when it names a page (or
  * object), else complete the action server-side.
  */
-describe('modalActionHandler — open the target, else run it server-side', () => {
+describe('modalActionHandler — a modal action is CLIENT-SIDE ONLY (objectstack#3959)', () => {
   it('opens the resolved target client-side and never POSTs to /actions', async () => {
     resolveTargetSpy.mockResolvedValue({ content: { name: 'log_call', type: 'utility' } });
     const { result } = renderHook(() =>
@@ -479,11 +479,14 @@ describe('modalActionHandler — open the target, else run it server-side', () =
     expect(authFetchSpy).not.toHaveBeenCalled();
   });
 
-  it('falls back to the server action when the target names no page or object', async () => {
-    // The modal was the param dialog the runner already collected — the action
-    // still has to run, so it goes to its registered server handler.
+  // This test used to assert the opposite — that an unresolvable target fell
+  // back to POSTing /actions, "how a modal action bound to registerAction still
+  // runs". It never ran: the framework rejects type:'modal' over REST with a
+  // 400 (headlessActionTypeError), so the fallthrough turned an authoring
+  // mistake into a confusing round-trip and let apps ship handlers no
+  // declaration could address (objectstack#3959).
+  it('reports an unresolvable target instead of POSTing to /actions', async () => {
     resolveTargetSpy.mockResolvedValue(null);
-    authFetchSpy.mockResolvedValue({ ok: true, json: async () => ({ success: true, data: { ok: 1 } }) });
     const { result } = renderHook(() =>
       useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'crm_call' }),
     );
@@ -496,9 +499,11 @@ describe('modalActionHandler — open the target, else run it server-side', () =
     });
 
     expect(modalHandlerSpy).not.toHaveBeenCalled();
-    expect(authFetchSpy).toHaveBeenCalledTimes(1);
-    expect(authFetchSpy.mock.calls[0][0]).toContain('/api/v1/actions/crm_call/log_call');
-    expect(r.success).toBe(true);
+    expect(authFetchSpy).not.toHaveBeenCalled();
+    expect(r.success).toBe(false);
+    // The message must name the action, the dud target, and the way out.
+    expect(String(r.error)).toContain('log_call');
+    expect(String(r.error)).toMatch(/type:'script' with params/);
   });
 
   it('prefers an inline `modal` descriptor over `target`', async () => {

packages/app-shell/src/hooks/useConsoleActionRuntime.tsx

@@ -662,20 +662,36 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons
   const { modalHandler, modalElement, resolveModalTarget } = useActionModal(dataSource);
 
   /**
-   * `type: 'modal'` dispatch. The action's `target` names the page to open
-   * (spec: "the modal/page name to open"), so try to render it client-side
-   * first. When it names neither a page nor an object there is nothing to
-   * render — the modal was the param dialog the runner already collected — so
-   * complete the action through its server-side handler, which is how a modal
-   * action bound to `engine.registerAction(...)` (or an inline `body`) still
-   * runs.
+   * `type: 'modal'` dispatch — CLIENT-SIDE ONLY. The action's `target` names
+   * the page to open (spec: "the modal/page name to open"); rendering it is
+   * the whole of what a modal action does.
+   *
+   * [objectstack#3959] This used to fall through to `serverActionHandler` when
+   * the target resolved to neither a page nor an object, documented as "how a
+   * modal action bound to `engine.registerAction(...)` still runs". It never
+   * ran: the framework's `headlessActionTypeError` rejects `type: 'modal'`
+   * over REST with a 400, because a modal has no server dispatch. The
+   * fallthrough only converted an authoring mistake — a target naming no
+   * page — into a confusing round-trip, and it let apps ship handlers no
+   * declaration could address (app-todo's `deferTask` / `setReminder` sat dead
+   * for exactly this reason).
+   *
+   * An unresolvable target is now reported as what it is. To collect input and
+   * then run server-side, declare `type: 'script'` with `params`: the runner
+   * collects the same dialog and the handler runs with those values.
    */
-  const modalActionHandler = useCallback(async (action: ActionDef, context?: ActionContext): Promise<ActionResult> => {
+  const modalActionHandler = useCallback(async (action: ActionDef, _context?: ActionContext): Promise<ActionResult> => {
     const schema = (action as any).modal ?? action.target ?? (action as any).params?.schema;
     const descriptor = schema != null ? await resolveModalTarget(schema) : null;
     if (descriptor) return modalHandler(descriptor);
-    return serverActionHandler(action, context);
-  }, [resolveModalTarget, modalHandler, serverActionHandler]);
+    return {
+      success: false,
+      error:
+        `Action "${action.name}" is type:'modal' but its target ` +
+        `${schema != null ? `"${String(schema)}" ` : ''}names no page or object to open. ` +
+        `Point it at a page, or use type:'script' with params to collect input and run a handler.`,
+    };
+  }, [resolveModalTarget, modalHandler]);
 
   const actionProviderProps = useMemo(() => ({
     context: {