Skip to content

Commit dce2d16

Browse files
committed
feat(spec,runtime): refuse the doubled post-success navigation channel (#11519)
A type:'script' action could carry two post-success destinations — the declared ActionSchema.onSuccess block and the handler-returned { redirectUrl } — with the spec ruling neither, leaving renderer-side precedence to decide silently (interim: declared wins, objectui#5933). Maintainer ruling 2026-08-24: refuse the doubled channel; no precedence field. Measured static knowability partitions the fix: - Statically knowable half: opensInNewTab: true is the schema-visible marker of the handler-redirect channel, so onSuccess beside it on a script action is refused at authoring time by a new refine, with guidance naming both channels and the remedy. - Runtime-only remainder: a handler that returns redirectUrl with no marker is diagnosed loudly at the dispatch seam (doubledPostSuccessNavigationWarning), wired at both surfaces that hold the declaration and the handler result — the REST /actions route and the MCP run_action bridge. Observe-only: the wire is untouched and the interim renderer precedence stays the decider until the author takes the remedy. Single-channel cases (only onSuccess, only opensInNewTab, opensInNewTab + newTabUrl) stay accepted byte-identically, pinned; the corpus was measured at zero doubled producers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rxnd8cyFnoU8V5y21PaTsy
1 parent dce6a01 commit dce2d16

5 files changed

Lines changed: 422 additions & 0 deletions

File tree

‎packages/runtime/src/action-execution.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,11 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps,
12441244
if (!dispatch.dispatched) {
12451245
throw new Error(`No handler registered for action '${name}' on '${objectName}'`);
12461246
}
1247+
// [#11519] Same doubled post-success-navigation diagnostic as the REST
1248+
// seam — the defect is a property of the authored action + handler pair,
1249+
// observable wherever the two meet. Observe-only; the result is untouched.
1250+
const doubled = doubledPostSuccessNavigationWarning(deps, action, dispatch.result, objectName);
1251+
if (doubled) console.warn(doubled);
12471252
return { ok: true, action: action.name, objectName, ...(recordId ? { recordId } : {}), result: dispatch.result ?? null };
12481253
}
12491254

@@ -1358,6 +1363,59 @@ export function isActionNotRegisteredError(err: any): boolean {
13581363
}
13591364

13601365

1366+
/**
1367+
* [#11519] The DOUBLED post-success-navigation diagnostic — the runtime half
1368+
* of the maintainer's 2026-08-24 ruling (refuse the doubled channel; ⛔ no
1369+
* `precedence` contract field).
1370+
*
1371+
* Two channels can name a post-success destination for one `type: 'script'`
1372+
* action: the declared `ActionSchema.onSuccess` block, and the
1373+
* handler-returned `{ redirectUrl }` convention. The statically-knowable half
1374+
* (`onSuccess` beside `opensInNewTab: true`, the schema-visible marker of the
1375+
* handler-redirect channel) is refused at parse time by `@objectstack/spec`.
1376+
* This helper covers the remainder no schema can see — "the handler returns
1377+
* `redirectUrl`" is runtime-only knowledge (`target` names an opaque registry
1378+
* entry; `HookBodySchema` declares no return contract) — at the one seam
1379+
* where both channels are finally in hand: the script dispatch, holding the
1380+
* resolved declaration AND the handler's return value.
1381+
*
1382+
* Returns the warning text on the doubled case, `null` otherwise; the caller
1383+
* logs it (the `actionPermissionError` string-or-null convention). It only
1384+
* OBSERVES — the result still reaches the client intact, and the interim
1385+
* renderer precedence (declared `onSuccess` wins, objectui#5933) still
1386+
* decides the navigation until the author takes the remedy the warning
1387+
* names. `warn`, not `error`, by the degradation-log-level rule: nothing
1388+
* claimed-persisted is lost, and the system is visibly navigating — to the
1389+
* declared destination.
1390+
*
1391+
* Both dispatch surfaces call it — the REST `/actions` route and the MCP
1392+
* `run_action` bridge — because the defect it names is a property of the
1393+
* AUTHORED action + handler pair, observable wherever the two meet, not of
1394+
* whichever caller happened to invoke it.
1395+
*/
1396+
export function doubledPostSuccessNavigationWarning(
1397+
_deps: ActionExecutionDeps,
1398+
actionDef: any,
1399+
result: unknown,
1400+
objectName?: string,
1401+
): string | null {
1402+
const navigate: unknown = actionDef?.onSuccess?.navigate;
1403+
if (typeof navigate !== 'string' || navigate.length === 0) return null;
1404+
if (!result || typeof result !== 'object' || Array.isArray(result)) return null;
1405+
const redirectUrl: unknown = (result as Record<string, unknown>).redirectUrl;
1406+
if (typeof redirectUrl !== 'string' || redirectUrl.length === 0) return null;
1407+
const where = objectName ? `${objectName}/${actionDef?.name ?? '<unnamed>'}` : String(actionDef?.name ?? '<unnamed>');
1408+
return (
1409+
`[action-contract] Action '${where}': the handler returned \`redirectUrl\` while the action `
1410+
+ 'also declares `onSuccess.navigate` — two post-success destinations for one success '
1411+
+ '(#11519). The DECLARED `onSuccess` wins and the handler\'s `redirectUrl` is ignored '
1412+
+ '(interim renderer precedence, objectui#5933). Fix the action, not the renderer: keep '
1413+
+ '`onSuccess` and stop returning `redirectUrl` from the handler, or drop `onSuccess` and '
1414+
+ 'let the handler return drive the navigation. There is no `precedence` field, by ruling.'
1415+
);
1416+
}
1417+
1418+
13611419
/**
13621420
* [ADR-0110 D2] Run a script/body action through the engine's handler
13631421
* registry: rotate the derived key candidates across the object-key rotation

‎packages/runtime/src/domains/actions.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,13 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string
432432
response: deps.error(`Action '${actionName}' on object '${objectName}' not found`, 404),
433433
};
434434
}
435+
// [#11519] Doubled post-success navigation — the handler returned
436+
// `redirectUrl` while the declaration carries `onSuccess`. The one
437+
// seam holding both channels; observe LOUDLY, never rewrite the wire
438+
// (the interim renderer precedence, declared wins per objectui#5933,
439+
// stays the decider until the author takes the remedy).
440+
const doubled = actionExec.doubledPostSuccessNavigationWarning(deps, actionDef, result, objectName);
441+
if (doubled) console.warn(doubled);
435442
// [#3962] Single wrap: `data` is the handler's return value, exactly as
436443
// every other domain serializes. The former inner `{success, data}`
437444
// envelope existed only to carry a failure signal at HTTP 200; failures
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* REST `/actions/:object/:action` — the DOUBLED post-success-navigation
5+
* diagnostic (#11519, maintainer ruling 2026-08-24).
6+
*
7+
* Two channels can name a post-success destination for one `type: 'script'`
8+
* action: the declared `ActionSchema.onSuccess` block, and the
9+
* handler-returned `{ redirectUrl }`. The statically-knowable half
10+
* (`onSuccess` + `opensInNewTab: true`) is refused at parse time by
11+
* `@objectstack/spec`; this file pins the RUNTIME half — the case no schema
12+
* can see, because "the handler returns `redirectUrl`" is runtime-only
13+
* knowledge (`target` names an opaque registry entry, `HookBodySchema`
14+
* declares no return contract). The seam where the two channels finally meet
15+
* is the script dispatch: the resolved declaration (carrying `onSuccess`) and
16+
* the handler's return value are both in hand, so the doubled case is
17+
* diagnosed LOUDLY there instead of being resolved silently by renderer-side
18+
* precedence.
19+
*
20+
* The diagnostic never alters the wire: the handler's return value still
21+
* reaches the client intact, and the interim renderer precedence (declared
22+
* `onSuccess` wins, objectui#5933) still decides the navigation until the
23+
* author takes the remedy the warning names.
24+
*/
25+
26+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
27+
import { HttpDispatcher } from './http-dispatcher.js';
28+
import { doubledPostSuccessNavigationWarning } from './action-execution.js';
29+
30+
const scriptAction = {
31+
name: 'open_portal',
32+
label: 'Open portal',
33+
objectName: 'crm_lead',
34+
type: 'script',
35+
target: 'openPortal',
36+
onSuccess: { navigate: '/apps/crm/leads/${result.id}', openIn: 'self' },
37+
};
38+
39+
function makeDispatcher(opts: { objectDef?: any; handlerResult?: unknown } = {}) {
40+
const executeAction = vi.fn(async () => opts.handlerResult ?? { ran: 'script' });
41+
const objectDef = opts.objectDef ?? { name: 'crm_lead', actions: [scriptAction] };
42+
const ql: any = {
43+
executeAction,
44+
getSchema: (name: string) => (name === objectDef.name ? objectDef : undefined),
45+
registry: {
46+
getObject: (name: string) => (name === objectDef.name ? objectDef : undefined),
47+
getItem: () => undefined,
48+
},
49+
find: vi.fn(async () => []),
50+
insert: vi.fn(), update: vi.fn(), delete: vi.fn(),
51+
};
52+
const metadata: any = {
53+
load: vi.fn(async () => null),
54+
loadDiagnosed: vi.fn(async () => ({ data: null, degraded: false, errors: [] })),
55+
listObjects: vi.fn(async () => [objectDef]),
56+
getObject: vi.fn(async () => objectDef),
57+
};
58+
const kernel: any = {
59+
context: {
60+
getService: (n: string) =>
61+
n === 'objectql' || n === 'data' ? ql
62+
: n === 'metadata' ? metadata
63+
: null,
64+
},
65+
};
66+
return { dispatcher: new HttpDispatcher(kernel), executeAction };
67+
}
68+
69+
const ctxFor = (): any => ({
70+
request: {},
71+
environmentId: 'platform',
72+
executionContext: { userId: 'u1', systemPermissions: [] },
73+
});
74+
75+
describe('REST /actions — doubled post-success navigation diagnostic (#11519)', () => {
76+
let warnSpy: ReturnType<typeof vi.spyOn>;
77+
beforeEach(() => {
78+
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
79+
});
80+
afterEach(() => {
81+
warnSpy.mockRestore();
82+
});
83+
84+
it('warns LOUDLY when the handler returns redirectUrl while the action declares onSuccess', async () => {
85+
const { dispatcher } = makeDispatcher({
86+
handlerResult: { redirectUrl: 'https://idp.example.com/handoff' },
87+
});
88+
89+
const res = await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
90+
91+
expect(res.response.status).toBe(200);
92+
const doubled = warnSpy.mock.calls
93+
.map((c) => c.join(' '))
94+
.filter((line) => line.includes('[action-contract]'));
95+
expect(doubled).toHaveLength(1);
96+
// The warning names the action, BOTH channels, the interim winner and
97+
// the remedy — that is what "loud" means here.
98+
expect(doubled[0]).toContain("'crm_lead/open_portal'");
99+
expect(doubled[0]).toContain('onSuccess');
100+
expect(doubled[0]).toContain('redirectUrl');
101+
expect(doubled[0]).toContain('objectui#5933');
102+
expect(doubled[0]).toContain('#11519');
103+
});
104+
105+
it('does NOT alter the wire — the handler return value still reaches the client intact', async () => {
106+
const { dispatcher } = makeDispatcher({
107+
handlerResult: { redirectUrl: 'https://idp.example.com/handoff', ticket: 't_1' },
108+
});
109+
110+
const res = await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
111+
112+
// Single wrap (#3962): `data` IS the handler's return value. The
113+
// diagnostic observes; the interim renderer precedence (declared wins,
114+
// objectui#5933) stays the decider until the remedy is taken.
115+
expect(res.response.body.data).toEqual({
116+
redirectUrl: 'https://idp.example.com/handoff',
117+
ticket: 't_1',
118+
});
119+
});
120+
121+
it('stays SILENT when only onSuccess is declared (handler returns no redirectUrl)', async () => {
122+
const { dispatcher } = makeDispatcher({ handlerResult: { ok: true } });
123+
124+
await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
125+
126+
expect(warnSpy.mock.calls.map((c) => c.join(' '))
127+
.filter((line) => line.includes('[action-contract]'))).toHaveLength(0);
128+
});
129+
130+
it('stays SILENT when only the handler-redirect channel is used (no onSuccess declared)', async () => {
131+
const single = { ...scriptAction, onSuccess: undefined };
132+
const { dispatcher } = makeDispatcher({
133+
objectDef: { name: 'crm_lead', actions: [single] },
134+
handlerResult: { redirectUrl: 'https://idp.example.com/handoff' },
135+
});
136+
137+
await dispatcher.handleActions('/crm_lead/open_portal', 'POST', {}, ctxFor());
138+
139+
expect(warnSpy.mock.calls.map((c) => c.join(' '))
140+
.filter((line) => line.includes('[action-contract]'))).toHaveLength(0);
141+
});
142+
});
143+
144+
describe('doubledPostSuccessNavigationWarning — predicate pins (#11519)', () => {
145+
const deps: any = {};
146+
const decl = { name: 'open_portal', onSuccess: { navigate: '/x', openIn: 'self' } };
147+
148+
it('fires exactly on the doubled pair', () => {
149+
const msg = doubledPostSuccessNavigationWarning(deps, decl, { redirectUrl: '/y' }, 'crm_lead');
150+
expect(msg).toBeTruthy();
151+
expect(msg).toContain('[action-contract]');
152+
});
153+
154+
it.each([
155+
['no declaration', undefined, { redirectUrl: '/y' }],
156+
['declaration without onSuccess', { name: 'a' }, { redirectUrl: '/y' }],
157+
['onSuccess without navigate', { onSuccess: {} }, { redirectUrl: '/y' }],
158+
['non-object result', decl, 'https://x'],
159+
['array result', decl, [{ redirectUrl: '/y' }]],
160+
['result without redirectUrl', decl, { ok: true }],
161+
['empty redirectUrl', decl, { redirectUrl: '' }],
162+
['non-string redirectUrl', decl, { redirectUrl: 42 }],
163+
['null result', decl, null],
164+
])('stays null on %s', (_label, actionDef, result) => {
165+
expect(doubledPostSuccessNavigationWarning(deps, actionDef, result, 'crm_lead')).toBeNull();
166+
});
167+
});
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
// #11519 — the doubled post-success-navigation channel (maintainer ruling
2+
// 2026-08-24, recorded on the card: refuse the doubled channel; ⛔ no
3+
// `precedence` contract field).
4+
//
5+
// Two independent channels can name a post-success destination for ONE
6+
// `type: 'script'` action: the DECLARED `onSuccess` block, and the
7+
// HANDLER-RETURNED `{ redirectUrl }`. The handler's return value is
8+
// runtime-only in general (a `target` names an opaque registry entry;
9+
// `HookBodySchema` declares no return contract) — but the action-level
10+
// `opensInNewTab` flag IS a schema-visible declaration of the handler-redirect
11+
// channel: its contract is "pre-open a tab, then drive it to the handler's
12+
// returned `redirectUrl`". So the statically-knowable doubled case is
13+
// `onSuccess` + `opensInNewTab: true` on one script action, and THAT pair is
14+
// refused at authoring time. The runtime-only remainder (a handler that
15+
// returns `redirectUrl` with no marker declared) is covered by the loud
16+
// dispatch-seam diagnostic in `@objectstack/runtime` (`action-execution.ts`),
17+
// not by this schema.
18+
import { describe, it, expect } from 'vitest';
19+
import { ActionSchema } from './action.zod';
20+
import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas';
21+
22+
const base = { name: 'open_sso_portal', label: 'Open SSO portal' };
23+
24+
describe('ActionSchema — doubled post-success navigation (#11519)', () => {
25+
describe('refusal pin — the statically-knowable doubled declaration', () => {
26+
const doubled = {
27+
...base,
28+
type: 'script' as const,
29+
target: 'ssoOpen',
30+
opensInNewTab: true,
31+
onSuccess: { navigate: '/apps/account/sys_account' },
32+
};
33+
34+
it('refuses onSuccess beside opensInNewTab on a type:script action', () => {
35+
const r = ActionSchema.safeParse(doubled);
36+
expect(r.success).toBe(false);
37+
});
38+
39+
it('names BOTH channels and the remedy, and records the interim winner', () => {
40+
const r = ActionSchema.safeParse(doubled);
41+
expect(r.success).toBe(false);
42+
const msg = r.error!.issues.map((i) => i.message).join('\n');
43+
// Both channels, by name.
44+
expect(msg).toContain('onSuccess');
45+
expect(msg).toContain('opensInNewTab');
46+
expect(msg).toContain('redirectUrl');
47+
// The remedy: one destination, declared in one place.
48+
expect(msg).toMatch(/drop|remove|keep/i);
49+
// The interim renderer precedence this refusal supersedes at authoring
50+
// time (declared wins, objectui#5933) is recorded so an author hitting
51+
// the error understands what happens to metadata published before it.
52+
expect(msg).toContain('objectui#5933');
53+
});
54+
55+
it('is refused through the registered `action` metadata schema too (the parsing door)', () => {
56+
const schema = getMetadataTypeSchema('action');
57+
expect(schema).toBeDefined();
58+
const r = schema!.safeParse(doubled);
59+
expect(r.success).toBe(false);
60+
});
61+
});
62+
63+
describe('single-channel pins — each channel alone stays accepted byte-identically', () => {
64+
it('only onSuccess on a script action: accepted, output unchanged', () => {
65+
const out = ActionSchema.parse({
66+
...base,
67+
type: 'script',
68+
target: 'cloneVersion',
69+
onSuccess: { navigate: '/apps/mfg/task_version/${result.id}' },
70+
}) as Record<string, unknown>;
71+
// The exact parse output this input produced BEFORE the refusal landed —
72+
// materialized defaults included. A byte drift here means the narrowing
73+
// touched an accepted case.
74+
expect(out).toEqual({
75+
name: 'open_sso_portal',
76+
label: 'Open SSO portal',
77+
type: 'script',
78+
target: 'cloneVersion',
79+
refreshAfter: false,
80+
onSuccess: { navigate: '/apps/mfg/task_version/${result.id}', openIn: 'self' },
81+
});
82+
});
83+
84+
it('only opensInNewTab (handler-redirect channel) on a script action: accepted, output unchanged', () => {
85+
const out = ActionSchema.parse({
86+
...base,
87+
type: 'script',
88+
target: 'ssoOpen',
89+
opensInNewTab: true,
90+
}) as Record<string, unknown>;
91+
expect(out).toEqual({
92+
name: 'open_sso_portal',
93+
label: 'Open SSO portal',
94+
type: 'script',
95+
target: 'ssoOpen',
96+
refreshAfter: false,
97+
opensInNewTab: true,
98+
});
99+
});
100+
101+
it('opensInNewTab + newTabUrl (zero-roundtrip variant) without onSuccess: accepted', () => {
102+
const r = ActionSchema.safeParse({
103+
...base,
104+
type: 'script',
105+
target: 'ssoOpen',
106+
opensInNewTab: true,
107+
newTabUrl: '/sso-open?recordId={recordId}',
108+
});
109+
expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true);
110+
});
111+
});
112+
113+
describe('scope pins — exactly the ruled pair, nothing wider', () => {
114+
it('an explicit opensInNewTab: false beside onSuccess is NOT the marker — accepted', () => {
115+
// `false` declares the handler-redirect channel is NOT in use; only
116+
// `true` marks it. The pair with `false` carries one destination.
117+
const r = ActionSchema.safeParse({
118+
...base,
119+
type: 'script',
120+
target: 'cloneVersion',
121+
opensInNewTab: false,
122+
onSuccess: { navigate: '/x' },
123+
});
124+
expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true);
125+
});
126+
127+
it('the pair on a type:api action stays accepted — the ruling scopes the refusal to type:script', () => {
128+
// #11519's ruled sentence is about a `type: 'script'` action whose
129+
// HANDLER can return `redirectUrl`; an api action has no script handler.
130+
// Recorded as a deliberate scope boundary, not an oversight — widening
131+
// it is a new decision, not a drive-by.
132+
const r = ActionSchema.safeParse({
133+
...base,
134+
type: 'api',
135+
target: '/api/v1/actions/x/y',
136+
opensInNewTab: true,
137+
onSuccess: { navigate: '/x' },
138+
});
139+
expect(r.success, JSON.stringify((r as { error?: unknown }).error)).toBe(true);
140+
});
141+
});
142+
});

0 commit comments

Comments
 (0)