Skip to content

Commit 066dd3b

Browse files
os-trumpclaude
andauthored
fix(runtime): route the flow param seeder through the single object-less action-key predicate (#15018)
* test(runtime): pin the object-less action-key agreement and the ADR-0104 D2 param gate Both functions were ablated repo-wide first. `seedFlowActionParams` turned out to be pinned already — indirectly, through the REST route — but only on its object-BOUND leg; `enforceActionParams` had no pin at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * fix(runtime): route the flow param seeder through the single object-less predicate Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * chore(changeset): patch for the object-less action-key predicate convergence Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b63052d commit 066dd3b

5 files changed

Lines changed: 291 additions & 2 deletions
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): route the flow param seeder through the single object-less predicate (#14864)
6+
7+
`isObjectLessActionKey` (`@objectstack/objectql`) is the canonical answer to
8+
"is this routed object the object-less placeholder": the canonical
9+
`GLOBAL_ACTION_OBJECT_KEY`, the legacy `'*'`, or nothing at all.
10+
`dispatchFlowAction` asks it directly when it decides whether to hand the
11+
automation service an `object` at all — and then, three lines later, handed the
12+
same `objectName` to `seedFlowActionParams`, which answered the same question
13+
with a second, narrower comparison of its own (`objectName !==
14+
GLOBAL_ACTION_OBJECT_KEY`).
15+
16+
The two parted on exactly one input, `'*'`. A request routed at the legacy
17+
wildcard — `POST /actions/*/<action>/<id>`, which resolves today because
18+
`actionHandlerObjectKeys` deliberately probes `'*'` last so a handler user code
19+
registered against it still resolves — was object-less to the automation
20+
envelope (no `object` sent) and object-BOUND to the params bag, which seeded a
21+
nonsense `'*Id'` key beside `recordId`. Same dispatch, two answers.
22+
23+
The empty-string half was never part of the divergence: the `objectName &&`
24+
truthiness leg of the old guard already covered it, and `undefined` with it.
25+
`'*'` was the whole of it.
26+
27+
**Direction.** The guard is widened onto the shared predicate rather than
28+
`isObjectLessActionKey` being narrowed. `'*'` is *unused today*, not *dead*:
29+
nothing first-party registers under it, but it is a deliberately-honoured
30+
legacy read path with its own docblock, reachable through the public
31+
`engine.registerAction(objectName, …)` surface that user code calls. Retiring
32+
it is a compatibility decision about someone else's package, not a tidy-up this
33+
fix is entitled to make.
34+
35+
**Coverage.** Both functions this touches were ablated repo-wide first rather
36+
than grepped, because a grep scoped to the file you expect a pin in cannot see
37+
a pin living elsewhere:
38+
39+
- `seedFlowActionParams` gutted → 5 tests red. It was pinned all along,
40+
indirectly, through the REST route — but every case there routes at a real
41+
object, so the object-LESS leg, where the two predicates actually disagreed,
42+
was the unpinned part. Now pinned, over the whole predicate domain.
43+
- `enforceActionParams` replaced with an unconditional `return null` → 3143
44+
passed, 0 failed. The ADR-0104 D2 gate could stop existing with nothing in
45+
the repo noticing. Its validator is well pinned in `@objectstack/spec`; the
46+
runtime gate around it was not, and that gate is what keeps an AI/MCP
47+
caller's plausible-but-wrong bag out of an action body. Now pinned.

packages/runtime/src/action-execution.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,16 @@ export function seedFlowActionParams(_deps: ActionExecutionDeps,
638638

639639
if (rowId != null) {
640640
const keys = new Set<string>(['recordId']);
641-
if (objectName && objectName !== GLOBAL_ACTION_OBJECT_KEY) {
641+
// [#14864] ONE predicate for "object-less", the same one
642+
// `dispatchFlowAction` asks three lines from here before it decides
643+
// whether to hand the automation service an `object` at all. This used
644+
// to be a second, narrower comparison (`objectName !==
645+
// GLOBAL_ACTION_OBJECT_KEY`), and the two parted on exactly one input:
646+
// a route resolved at the legacy `'*'` was object-less to the envelope
647+
// and object-BOUND here, so the bag grew a nonsense `'*Id'` alias. The
648+
// empty-string leg was never the divergence — the `objectName &&`
649+
// truthiness test this replaces already covered it.
650+
if (!isObjectLessActionKey(objectName)) {
642651
keys.add(`${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`);
643652
}
644653
if (typeof action?.recordIdParam === 'string' && action.recordIdParam) {
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* "Object-less" has ONE answer inside `action-execution.ts` (#14864).
5+
*
6+
* `isObjectLessActionKey` (`@objectstack/objectql`) is the canonical predicate:
7+
* the routed object is object-less when it is the canonical
8+
* `GLOBAL_ACTION_OBJECT_KEY`, the legacy `'*'`, or nothing at all.
9+
* `dispatchFlowAction` asks it directly when it decides whether to hand the
10+
* automation service an `object` at all — and then, on the very next line,
11+
* hands the same `objectName` to `seedFlowActionParams`, which used to answer
12+
* the same question with a second, narrower comparison of its own.
13+
*
14+
* The two parted on exactly one input, `'*'`: the automation envelope treated a
15+
* `'*'` route as object-less and omitted `object`, while the params bag treated
16+
* it as a real object and seeded a nonsense `'*Id'` alias key beside
17+
* `recordId`. One dispatch, two answers, three lines apart.
18+
*
19+
* ## Why the pin sits HERE and not only on the route
20+
*
21+
* `seedFlowActionParams` was NOT unpinned — `http-dispatcher.actions-type-
22+
* dispatch.test.ts` covers its whole seeding ladder, indirectly, through the
23+
* REST route, without ever naming it. What that file never does is route at an
24+
* object-LESS key: every case there is `/crm_lead/...`. So the ladder was
25+
* pinned and the object-less leg of it was not, which is why the divergence
26+
* survived. This file pins the leg, at the level the two predicates actually
27+
* meet: one function, the whole `isObjectLessActionKey` domain, one bag.
28+
*
29+
* ## The arms, and which one is the control
30+
*
31+
* The `OBJECT_FUL` case is an ANTI-VACUITY CONTROL, not a pin: it asserts the
32+
* alias key IS seeded for a real object. If a future edit makes
33+
* `seedFlowActionParams` seed nothing at all, the negative assertions below
34+
* would all pass for the wrong reason, and this control is what fails instead.
35+
* ⛔ A red here is not this file's finding — read the object-less arm first.
36+
*/
37+
38+
import { describe, it, expect } from 'vitest';
39+
import { GLOBAL_ACTION_OBJECT_KEY, isObjectLessActionKey } from '@objectstack/objectql';
40+
import { seedFlowActionParams, type ActionExecutionDeps } from './action-execution.js';
41+
42+
/** `seedFlowActionParams` ignores its first parameter — see its signature. */
43+
const NO_DEPS = undefined as unknown as ActionExecutionDeps;
44+
45+
const ROW_ID = 'row_1';
46+
47+
/**
48+
* The `<objectName>Id` camelCase alias `seedFlowActionParams` seeds for an
49+
* object-bound route, derived the way the function derives it rather than
50+
* hard-coded — a hard-coded copy would go stale in silence the day the
51+
* spelling changes, which is the same failure this whole card is about.
52+
*/
53+
const aliasKeyFor = (objectName: string): string =>
54+
`${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`;
55+
56+
const seedFor = (objectName: string): Record<string, unknown> =>
57+
seedFlowActionParams(NO_DEPS, { name: 'convert_lead', type: 'flow' }, {
58+
objectName,
59+
record: {},
60+
params: {},
61+
recordId: ROW_ID,
62+
});
63+
64+
/**
65+
* Every string spelling `isObjectLessActionKey` accepts. The table is asserted
66+
* against the predicate itself below, so narrowing the predicate (retiring
67+
* `'*'`, say) fails HERE with a readable message instead of quietly leaving a
68+
* row that no longer describes anything.
69+
*/
70+
const OBJECT_LESS_KEYS: readonly string[] = [GLOBAL_ACTION_OBJECT_KEY, '*', ''];
71+
72+
/** A real object — the control's route, and the one the REST pin already uses. */
73+
const OBJECT_FUL = 'crm_lead';
74+
75+
describe('object-less action key — one predicate, one answer (#14864)', () => {
76+
it('anti-vacuity control: an object-BOUND route still seeds its alias key', () => {
77+
// Positive control. Every negative below is a claim that a key is
78+
// absent; without this, deleting the seeding branch outright would
79+
// turn them all green.
80+
expect(isObjectLessActionKey(OBJECT_FUL)).toBe(false);
81+
const bag = seedFor(OBJECT_FUL);
82+
expect(bag[aliasKeyFor(OBJECT_FUL)]).toBe(ROW_ID);
83+
expect(bag.recordId).toBe(ROW_ID);
84+
});
85+
86+
it('the table below describes exactly what the predicate accepts', () => {
87+
// Guards the table, not the code: a narrowed predicate must come here
88+
// and say so rather than leaving an inert row behind.
89+
for (const key of OBJECT_LESS_KEYS) {
90+
expect(isObjectLessActionKey(key), `${JSON.stringify(key)} is no longer object-less`).toBe(true);
91+
}
92+
});
93+
94+
it.each(OBJECT_LESS_KEYS.map((key) => ({ key, label: JSON.stringify(key) })))(
95+
'seeds no object alias for the object-less key $label',
96+
({ key }) => {
97+
const bag = seedFor(key);
98+
// The row id still reaches the flow — this is about the ALIAS only.
99+
expect(bag.recordId).toBe(ROW_ID);
100+
expect(
101+
Object.keys(bag),
102+
`seedFlowActionParams seeded the alias key ${JSON.stringify(aliasKeyFor(key))} for the `
103+
+ `object-less route ${JSON.stringify(key)}. isObjectLessActionKey() calls that route `
104+
+ `object-less and dispatchFlowAction omits \`object\` from the automation envelope for `
105+
+ `it, so the params bag must not invent an object alias either (#14864).`,
106+
).not.toContain(aliasKeyFor(key));
107+
},
108+
);
109+
110+
it('every object-less spelling lands the SAME bag as the canonical key', () => {
111+
// The agreement stated as one assertion: which object-less spelling a
112+
// caller routed at must not be observable in the flow's params.
113+
const canonical = seedFor(GLOBAL_ACTION_OBJECT_KEY);
114+
for (const key of OBJECT_LESS_KEYS) {
115+
expect(seedFor(key), `routing at ${JSON.stringify(key)} produced a different params bag`)
116+
.toEqual(canonical);
117+
}
118+
});
119+
});

packages/runtime/src/action-owner-key-single-source.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,20 @@ describe('standalone-action owner key — half C: no bare literal (#14678)', ()
150150
// exactly the wrong reason — the can-never-fail property this whole
151151
// file was written to replace. Both controls are positive assertions
152152
// against text the converged file must carry.
153+
//
154+
// [#14864] The second control used to be the `seedFlowActionParams`
155+
// comparison `objectName !== GLOBAL_ACTION_OBJECT_KEY`. That guard is
156+
// gone — it was one of the two rival answers to "is this route
157+
// object-less", and it now delegates to `isObjectLessActionKey` like
158+
// its neighbours. Re-anchored rather than deleted, and deliberately
159+
// onto a site this file's own subject does not move: the warn-once log
160+
// key in `enforceActionParams`, which is the SECOND of the three bare
161+
// literals #14678 converged and is untouched by the predicate work.
162+
// ⛔ Do not re-anchor a control onto the thing the next change is most
163+
// likely to edit — a control that moves with its subject stops being a
164+
// control.
153165
expect(src).toContain('GLOBAL_ACTION_OBJECT_KEY');
154-
expect(src).toContain('objectName !== GLOBAL_ACTION_OBJECT_KEY');
166+
expect(src).toContain('where.objectName ?? GLOBAL_ACTION_OBJECT_KEY');
155167

156168
for (const literal of BARE_LITERALS) {
157169
expect(
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `enforceActionParams` — the ADR-0104 D2 gate itself, not its validator
5+
* (#14864).
6+
*
7+
* ## What was measured, and why this file exists
8+
*
9+
* The card that produced this file claimed neither `seedFlowActionParams` nor
10+
* `enforceActionParams` was named by any test. Grep cannot settle that — a pin
11+
* can live in a file that never names the function — so both were ABLATED
12+
* instead, repo-wide against `packages/runtime`'s 217 files / 3143 tests:
13+
*
14+
* - `seedFlowActionParams`, gutted → **5 tests red** in
15+
* `http-dispatcher.actions-type-dispatch.test.ts`. Pinned all along,
16+
* indirectly, through the REST route. The claim was wrong about it.
17+
* - `enforceActionParams`, replaced with an unconditional `return null` (the
18+
* gate accepting every bag) → **3143 passed, 0 failed**. Nothing in the repo
19+
* noticed the param contract had stopped existing.
20+
*
21+
* The VALIDATOR is thoroughly pinned — `@objectstack/spec`'s
22+
* `action-params.test.ts` covers `validateActionParams` case by case. What had
23+
* no pin is the runtime GATE wrapped around it, and the gate is where the
24+
* decisions live that the validator never makes: the param-less pass-through,
25+
* the strict-by-default rejection, and the `OS_ALLOW_LAX_ACTION_PARAMS` escape
26+
* hatch. A green validator says nothing about whether anything still calls it.
27+
*
28+
* That gap matters more than a missing unit test usually does: this gate is
29+
* what stops an AI/MCP caller's plausible-but-wrong bag from reaching an
30+
* action body (#3438), and its only other mention outside the source is a
31+
* MANUALLY-run platform-checklist clause. So it is pinned here at the level
32+
* the ablation showed to be empty.
33+
*/
34+
35+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
36+
import { enforceActionParams, type ActionExecutionDeps } from './action-execution.js';
37+
38+
/** `enforceActionParams` reaches nothing on `deps` — it only forwards it. */
39+
const NO_DEPS = undefined as unknown as ActionExecutionDeps;
40+
41+
/** No parent object schema: an object-less action carrying inline params only. */
42+
const NO_OBJECT = undefined;
43+
44+
const WHERE = { objectName: 'crm_lead', actionName: 'convert_lead' };
45+
46+
const REQUIRES_TITLE = {
47+
name: 'convert_lead',
48+
params: [{ name: 'title', type: 'text', required: true }],
49+
};
50+
51+
describe('enforceActionParams — the ADR-0104 D2 gate (#14864)', () => {
52+
beforeEach(() => {
53+
vi.unstubAllEnvs();
54+
});
55+
56+
afterEach(() => {
57+
vi.unstubAllEnvs();
58+
vi.restoreAllMocks();
59+
});
60+
61+
it('anti-vacuity control: a CONFORMING bag against declared params is accepted', () => {
62+
// Positive control for the rejection below. Without it, a gate that
63+
// rejected everything, or one that had stopped resolving params at
64+
// all, would still satisfy "rejects a bad bag".
65+
expect(enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, { title: 'Hi' }, WHERE)).toBeNull();
66+
});
67+
68+
it('rejects a bag that violates the declared contract, naming the param', () => {
69+
const error = enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, {}, WHERE);
70+
expect(error).toContain('Invalid action params');
71+
expect(error).toContain('title');
72+
});
73+
74+
it('passes an action that declares NO params straight through', () => {
75+
// The documented compatibility leg: nothing to validate against, so a
76+
// param-less action is untouched however odd its bag looks.
77+
expect(enforceActionParams(NO_DEPS, { name: 'ping' }, NO_OBJECT, { anything: 1 }, WHERE)).toBeNull();
78+
expect(enforceActionParams(NO_DEPS, { name: 'ping', params: [] }, NO_OBJECT, { anything: 1 }, WHERE)).toBeNull();
79+
});
80+
81+
it('is STRICT by default — no environment variable needed to reject (#3438)', () => {
82+
vi.stubEnv('OS_ALLOW_LAX_ACTION_PARAMS', '');
83+
expect(enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, {}, WHERE)).toContain('Invalid action params');
84+
});
85+
86+
it('`OS_ALLOW_LAX_ACTION_PARAMS=1` accepts the same bag instead, and warns', () => {
87+
// The opt-OUT of a check that ships ON (Prime Directive #9): the flag
88+
// must change the ANSWER, not merely the log line — a flag that only
89+
// logs would leave the rejection in place and strand the caller it was
90+
// added to unblock.
91+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
92+
vi.stubEnv('OS_ALLOW_LAX_ACTION_PARAMS', '1');
93+
94+
// A dedup key this suite has not warned on yet — `warnActionParamsOnce`
95+
// keys on `objectName/actionName` and its Set is module-global, so a
96+
// reused key would make the warn assertion pass or fail on test order.
97+
const where = { objectName: 'crm_lead', actionName: 'lax_probe' };
98+
expect(enforceActionParams(NO_DEPS, REQUIRES_TITLE, NO_OBJECT, {}, where)).toBeNull();
99+
expect(warn).toHaveBeenCalledTimes(1);
100+
expect(String(warn.mock.calls[0]?.[0])).toContain('OS_ALLOW_LAX_ACTION_PARAMS=1');
101+
});
102+
});

0 commit comments

Comments
 (0)