objectstack-ai
diff --git a/‎.changeset/action-doubled-redirect-refusal.md‎
Lines changed: 53 additions & 0 deletions b/‎.changeset/action-doubled-redirect-refusal.md‎
Lines changed: 53 additions & 0 deletions
diff --git a/‎packages/runtime/src/action-execution.ts‎
Lines changed: 58 additions & 0 deletions b/‎packages/runtime/src/action-execution.ts‎
Lines changed: 58 additions & 0 deletions
diff --git a/‎packages/runtime/src/domains/actions.ts‎
Lines changed: 7 additions & 0 deletions b/‎packages/runtime/src/domains/actions.ts‎
Lines changed: 7 additions & 0 deletions
diff --git a/‎packages/runtime/src/http-dispatcher.actions-doubled-redirect.test.ts‎
Lines changed: 167 additions & 0 deletions b/‎packages/runtime/src/http-dispatcher.actions-doubled-redirect.test.ts‎
Lines changed: 167 additions & 0 deletions
@@ -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. -->
@@ -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
 
@@ -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
 
@@ -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();
+    });
+});