feat(spec,runtime): refuse the doubled post-success navigation channel on script actions

.changeset/action-doubled-redirect-refusal.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/spec": minor
+"@objectstack/runtime": patch
+---
+
+feat(spec,runtime): refuse the doubled post-success navigation channel on a `type: 'script'` action (#11519)
+
+**BREAKING** accept-set narrowing on `ActionSchema`, shipped as `minor` under
+the repo's launch-window convention for breaking changes.
+
+Two independent channels could name a post-success destination for one
+`type: 'script'` action: the declared `onSuccess` block (`{ navigate, openIn }`,
+validated and visible in metadata) and the handler-returned `{ redirectUrl }`
+convention (runtime-only). The spec ruled each surface's default in isolation
+and said nothing about an action carrying both — so the renderer had to pick,
+and the pick lived only in one renderer's implementation (declared `onSuccess`
+wins, objectstack-ai/objectui#5933). Maintainer ruling 2026-08-24: refuse the
+doubled channel; ⛔ no `precedence` contract field.
+
+The measured static-knowability partition:
+
+- **Authoring-time refine (spec):** "the handler can return `redirectUrl`" is
+ runtime-only in general (`target` names an opaque registry entry;
+ `HookBodySchema` declares no return contract) — but `opensInNewTab: true` is
+ a schema-visible declaration of the handler-redirect channel (its contract is
+ "pre-open a tab, then drive it to the handler's returned `redirectUrl`").
+ A `type: 'script'` action declaring `onSuccess` beside `opensInNewTab: true`
+ is now **rejected at parse time**, with guidance naming both channels and the
+ remedy. Previously the pair parsed clean and one declaration was silently
+ dead at render.
+- **Dispatch-seam diagnostic (runtime):** the runtime-only remainder — a
+ handler that actually returns `{ redirectUrl }` while the action declares
+ `onSuccess` — now logs a loud `[action-contract]` warning at both dispatch
+ surfaces (the REST `/actions` route and the MCP `run_action` bridge), naming
+ the action, both channels, the interim winner and the remedy. Observe-only:
+ the wire is untouched and the interim renderer precedence stands until the
+ author takes the remedy.
+
+Single-channel declarations are untouched and pinned byte-identically: only
+`onSuccess`, only `opensInNewTab` (with or without `newTabUrl`), and
+`opensInNewTab: false` beside `onSuccess` all parse exactly as before. The
+corpus was measured at zero doubled producers (this repo's examples and
+platform metadata, objectui metadata, and the cloud SSO handoff producers per
+the #11519 measurement), so no shipped metadata is affected.
+
+**Migration.** An action refused by the new refine must pick its one
+destination: keep `onSuccess` and drop `opensInNewTab` (and stop returning
+`redirectUrl` from the handler), or keep `opensInNewTab` + the handler
+redirect and drop `onSuccess`. Which channel is right is an authoring decision
+the metadata cannot make for you, and zero such actions exist in any measured
+corpus.
+
+<!-- adr-0087: not-required (no-migration-prescription) A validity narrowing over a pair of existing keys: no key is removed, renamed or re-shaped, so there is no tombstone and nothing mechanical for `objectstack migrate meta` to rewrite. The refusal is the channel that reaches an affected author, at the parse site, carrying the remedy; choosing which of the two declared destinations to keep is an authoring decision no migration entry can perform on an upgrader's behalf — and the measured population of affected sources is zero in every corpus. -->

packages/runtime/src/action-execution.ts

@@ -1244,6 +1244,11 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps,
  if (!dispatch.dispatched) {
  throw new Error(`No handler registered for action '${name}' on '${objectName}'`);
  }
+ // [#11519] Same doubled post-success-navigation diagnostic as the REST
+ // seam — the defect is a property of the authored action + handler pair,
+ // observable wherever the two meet. Observe-only; the result is untouched.
+ const doubled = doubledPostSuccessNavigationWarning(deps, action, dispatch.result, objectName);
+ if (doubled) console.warn(doubled);
  return { ok: true, action: action.name, objectName, ...(recordId ? { recordId } : {}), result: dispatch.result ?? null };
 }
 
@@ -1358,6 +1363,59 @@ export function isActionNotRegisteredError(err: any): boolean {
 }
 
 
+/**
+ * [#11519] The DOUBLED post-success-navigation diagnostic — the runtime half
+ * of the maintainer's 2026-08-24 ruling (refuse the doubled channel; ⛔ no
+ * `precedence` contract field).
+ *
+ * Two channels can name a post-success destination for one `type: 'script'`
+ * action: the declared `ActionSchema.onSuccess` block, and the
+ * handler-returned `{ redirectUrl }` convention. The statically-knowable half
+ * (`onSuccess` beside `opensInNewTab: true`, the schema-visible marker of the
+ * handler-redirect channel) is refused at parse time by `@objectstack/spec`.
+ * This helper covers the remainder no schema can see — "the handler returns
+ * `redirectUrl`" is runtime-only knowledge (`target` names an opaque registry
+ * entry; `HookBodySchema` declares no return contract) — at the one seam
+ * where both channels are finally in hand: the script dispatch, holding the
+ * resolved declaration AND the handler's return value.
+ *
+ * Returns the warning text on the doubled case, `null` otherwise; the caller
+ * logs it (the `actionPermissionError` string-or-null convention). It only
+ * OBSERVES — the result still reaches the client intact, and the interim
+ * renderer precedence (declared `onSuccess` wins, objectui#5933) still
+ * decides the navigation until the author takes the remedy the warning
+ * names. `warn`, not `error`, by the degradation-log-level rule: nothing
+ * claimed-persisted is lost, and the system is visibly navigating — to the
+ * declared destination.
+ *
+ * Both dispatch surfaces call it — the REST `/actions` route and the MCP
+ * `run_action` bridge — because the defect it names is a property of the
+ * AUTHORED action + handler pair, observable wherever the two meet, not of
+ * whichever caller happened to invoke it.
+ */
+export function doubledPostSuccessNavigationWarning(
+ _deps: ActionExecutionDeps,
+ actionDef: any,
+ result: unknown,
+ objectName?: string,
+): string | null {
+ const navigate: unknown = actionDef?.onSuccess?.navigate;
+ if (typeof navigate !== 'string' || navigate.length === 0) return null;
+ if (!result || typeof result !== 'object' || Array.isArray(result)) return null;
+ const redirectUrl: unknown = (result as Record<string, unknown>).redirectUrl;
+ if (typeof redirectUrl !== 'string' || redirectUrl.length === 0) return null;
+ const where = objectName ? `${objectName}/${actionDef?.name ?? '<unnamed>'}` : String(actionDef?.name ?? '<unnamed>');
+ return (
+ `[action-contract] Action '${where}': the handler returned \`redirectUrl\` while the action `
+ + 'also declares `onSuccess.navigate` — two post-success destinations for one success '
+ + '(#11519). The DECLARED `onSuccess` wins and the handler\'s `redirectUrl` is ignored '
+ + '(interim renderer precedence, objectui#5933). Fix the action, not the renderer: keep '
+ + '`onSuccess` and stop returning `redirectUrl` from the handler, or drop `onSuccess` and '
+ + 'let the handler return drive the navigation. There is no `precedence` field, by ruling.'
+ );
+}
+
+
 /**
  * [ADR-0110 D2] Run a script/body action through the engine's handler
  * registry: rotate the derived key candidates across the object-key rotation

packages/runtime/src/domains/actions.ts

@@ -432,6 +432,13 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string
  response: deps.error(`Action '${actionName}' on object '${objectName}' not found`, 404),
  };
  }
+ // [#11519] Doubled post-success navigation — the handler returned
+ // `redirectUrl` while the declaration carries `onSuccess`. The one
+ // seam holding both channels; observe LOUDLY, never rewrite the wire
+ // (the interim renderer precedence, declared wins per objectui#5933,
+ // stays the decider until the author takes the remedy).
+ const doubled = actionExec.doubledPostSuccessNavigationWarning(deps, actionDef, result, objectName);
+ if (doubled) console.warn(doubled);
  // [#3962] Single wrap: `data` is the handler's return value, exactly as
  // every other domain serializes. The former inner `{success, data}`
  // envelope existed only to carry a failure signal at HTTP 200; failures

packages/runtime/src/http-dispatcher.actions-doubled-redirect.test.ts

@@ -0,0 +1,167 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * REST `/actions/:object/:action` — the DOUBLED post-success-navigation
+ * diagnostic (#11519, maintainer ruling 2026-08-24).
+ *
+ * Two channels can name a post-success destination for one `type: 'script'`
+ * action: the declared `ActionSchema.onSuccess` block, and the
+ * handler-returned `{ redirectUrl }`. The statically-knowable half
+ * (`onSuccess` + `opensInNewTab: true`) is refused at parse time by
+ * `@objectstack/spec`; this file pins the RUNTIME half — the case no schema
+ * can see, because "the handler returns `redirectUrl`" is runtime-only
+ * knowledge (`target` names an opaque registry entry, `HookBodySchema`
+ * declares no return contract). The seam where the two channels finally meet
+ * is the script dispatch: the resolved declaration (carrying `onSuccess`) and
+ * the handler's return value are both in hand, so the doubled case is
+ * diagnosed LOUDLY there instead of being resolved silently by renderer-side
+ * precedence.
+ *
+ * The diagnostic never alters the wire: the handler's return value still
+ * reaches the client intact, and the interim renderer precedence (declared
+ * `onSuccess` wins, objectui#5933) still decides the navigation until the
+ * author takes the remedy the warning names.
+ */
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { HttpDispatcher } from './http-dispatcher.js';
+import { doubledPostSuccessNavigationWarning } from './action-execution.js';
+
+const scriptAction = {
+ name: 'open_portal',
+ label: 'Open portal',
+ objectName: 'crm_lead',
+ type: 'script',
+ target: 'openPortal',
+ onSuccess: { navigate: '/apps/crm/leads/${result.id}', openIn: 'self' },
+};
+
+function makeDispatcher(opts: { objectDef?: any; handlerResult?: unknown } = {}) {
+ const executeAction = vi.fn(async () => opts.handlerResult ?? { ran: 'script' });
+ const objectDef = opts.objectDef ?? { name: 'crm_lead', actions: [scriptAction] };
+ const ql: any = {
+ executeAction,
+ getSchema: (name: string) => (name === objectDef.name ? objectDef : undefined),
+ registry: {
+ getObject: (name: string) => (name === objectDef.name ? objectDef : undefined),
+ getItem: () => undefined,
+ },
+ find: vi.fn(async () => []),
+ insert: vi.fn(), update: vi.fn(), delete: vi.fn(),
+ };
+ const metadata: any = {
+ load: vi.fn(async () => null),
+ loadDiagnosed: vi.fn(async () => ({ data: null, degraded: false, errors: [] })),
+ listObjects: vi.fn(async () => [objectDef]),
+ getObject: vi.fn(async () => objectDef),
+ };
+ const kernel: any = {
+ context: {
+ getService: (n: string) =>
+ n === 'objectql' || n === 'data' ? ql
+ : n === 'metadata' ? metadata
+ : null,
+ },
+ };
+ return { dispatcher: new HttpDispatcher(kernel), executeAction };
+}
+
+const ctxFor = (): any => ({
+ request: {},
+ environmentId: 'platform',
+ executionContext: { userId: 'u1', systemPermissions: [] },
+});
+
+describe('REST /actions — doubled post-success navigation diagnostic (#11519)', () => {
+ let warnSpy: ReturnType<typeof vi.spyOn>;
+ beforeEach(() => {
+ warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+ });
+ afterEach(() => {
+ warnSpy.mockRestore();
+ });
+
+ it('warns LOUDLY when the handler returns redirectUrl while the action declares onSuccess', async () => {
+ const { dispatcher } = makeDispatcher({
+ handlerResult: { redirectUrl: 'https://idp.example.com/handoff' },
+ });
+
+ const res = await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
+
+ expect(res.response?.status).toBe(200);
+ const doubled = warnSpy.mock.calls
+ .map((c: unknown[]) => c.join(' '))
+ .filter((line: string) => line.includes('[action-contract]'));
+ expect(doubled).toHaveLength(1);
+ // The warning names the action, BOTH channels, the interim winner and
+ // the remedy — that is what "loud" means here.
+ expect(doubled[0]).toContain("'crm_lead/open_portal'");
+ expect(doubled[0]).toContain('onSuccess');
+ expect(doubled[0]).toContain('redirectUrl');
+ expect(doubled[0]).toContain('objectui#5933');
+ expect(doubled[0]).toContain('#11519');
+ });
+
+ it('does NOT alter the wire — the handler return value still reaches the client intact', async () => {
+ const { dispatcher } = makeDispatcher({
+ handlerResult: { redirectUrl: 'https://idp.example.com/handoff', ticket: 't_1' },
+ });
+
+ const res = await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
+
+ // Single wrap (#3962): `data` IS the handler's return value. The
+ // diagnostic observes; the interim renderer precedence (declared wins,
+ // objectui#5933) stays the decider until the remedy is taken.
+ expect(res.response?.body.data).toEqual({
+ redirectUrl: 'https://idp.example.com/handoff',
+ ticket: 't_1',
+ });
+ });
+
+ it('stays SILENT when only onSuccess is declared (handler returns no redirectUrl)', async () => {
+ const { dispatcher } = makeDispatcher({ handlerResult: { ok: true } });
+
+ await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
+
+ expect(warnSpy.mock.calls.map((c: unknown[]) => c.join(' '))
+ .filter((line: string) => line.includes('[action-contract]'))).toHaveLength(0);
+ });
+
+ it('stays SILENT when only the handler-redirect channel is used (no onSuccess declared)', async () => {
+ const single = { ...scriptAction, onSuccess: undefined };
+ const { dispatcher } = makeDispatcher({
+ objectDef: { name: 'crm_lead', actions: [single] },
+ handlerResult: { redirectUrl: 'https://idp.example.com/handoff' },
+ });
+
+ await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
+
+ expect(warnSpy.mock.calls.map((c: unknown[]) => c.join(' '))
+ .filter((line: string) => line.includes('[action-contract]'))).toHaveLength(0);
+ });
+});
+
+describe('doubledPostSuccessNavigationWarning — predicate pins (#11519)', () => {
+ const deps: any = {};
+ const decl = { name: 'open_portal', onSuccess: { navigate: '/x', openIn: 'self' } };
+
+ it('fires exactly on the doubled pair', () => {
+ const msg = doubledPostSuccessNavigationWarning(deps, decl, { redirectUrl: '/y' }, 'crm_lead');
+ expect(msg).toBeTruthy();
+ expect(msg).toContain('[action-contract]');
+ });
+
+ it.each([
+ ['no declaration', undefined, { redirectUrl: '/y' }],
+ ['declaration without onSuccess', { name: 'a' }, { redirectUrl: '/y' }],
+ ['onSuccess without navigate', { onSuccess: {} }, { redirectUrl: '/y' }],
+ ['non-object result', decl, 'https://x'],
+ ['array result', decl, [{ redirectUrl: '/y' }]],
+ ['result without redirectUrl', decl, { ok: true }],
+ ['empty redirectUrl', decl, { redirectUrl: '' }],
+ ['non-string redirectUrl', decl, { redirectUrl: 42 }],
+ ['null result', decl, null],
+ ])('stays null on %s', (_label, actionDef, result) => {
+ expect(doubledPostSuccessNavigationWarning(deps, actionDef, result, 'crm_lead')).toBeNull();
+ });
+});