Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/flow-action-record-id-seeding.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
"@objectstack/runtime": patch
---

fix(actions): seed a flow action's params with the row id, like the trigger route does (#3915 follow-up)

#3915 gave the REST `/actions/:object/:action` route its flow dispatch and
documented it as "equivalent to `POST /api/v1/automation/:target/trigger`,
without having to know the flow name". A real run showed that claim did not
hold: the params bag carried the subject record's fields — so `id` — but never
`recordId`. The CRM's own `crm_convert_lead` action declares
`recordIdParam: 'recordId'` and its flow reads `{recordId}`, so invoking it
through the actions endpoint reached the automation engine and then died at its
first node:

```
Flow 'crm_convert_lead_wizard' failed: Node 'get_lead' failed: get_record:
refusing to run — 1 filter condition(s) resolved to nothing … `{recordId}` (at id)
```

while the identical run through `/automation/crm_convert_lead_wizard/trigger`
paused normally on its first screen. Only a live invocation surfaced it — the
unit tests mock `automation.execute`, so they pinned the call shape without
noticing the bag was missing the key flows actually read.

`dispatchFlowAction` now seeds the row id under the same keys
`domains/automation.ts` seeds for the trigger route — `recordId` and the
`<objectName>Id` camelCase alias — plus the action's own declared
`recordIdParam` (sourced from `recordIdField`, default `id`) when it names a
third key. Explicit action params still win over every seed, and the seeding
applies to the MCP `run_action` path too, which shared the same gap. A declared
`recordIdParam` that no dispatcher honoured was the `declared ≠ enforced` shape
in miniature.
75 changes: 70 additions & 5 deletions packages/runtime/src/action-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,66 @@ export function flowActionUnavailableError(action: any): string {
return `Action '${action?.name ?? 'unknown'}' is a flow but no automation service is available`;
}

/**
* The params bag a flow action hands the automation engine.
*
* Three seeds, weakest first — each only fills a key the stronger one left
* unset:
* 1. the subject record's fields, which populate a flow's named `isInput`
* variables the way the record-change trigger does;
* 2. the row id under the keys a flow author actually writes —
* `recordId` and the `<objectName>Id` camelCase alias — the SAME two
* `POST /automation/:name/trigger` seeds (`domains/automation.ts`), plus
* the action's own declared `recordIdParam` (seeded from `recordIdField`,
* default `id`) when it names a third key;
* 3. the caller's explicit action params, which win outright.
*
* Seed 2 is the one #3915's first pass missed, and only a real run caught it:
* the params bag carried the record's `id` but never `recordId`, so the CRM's
* own `crm_convert_lead` action — which declares `recordIdParam: 'recordId'`
* and whose flow reads `{recordId}` — reached the engine and died at its first
* node ("1 filter condition(s) resolved to nothing"), while the identical run
* through `/automation/crm_convert_lead_wizard/trigger` succeeded. A declared
* `recordIdParam` that nothing honours is the `declared ≠ enforced` shape in
* miniature.
*/
export function seedFlowActionParams(deps: ActionExecutionDeps,
action: any,
input: {
objectName: string;
record: Record<string, unknown>;
params: Record<string, unknown>;
recordId?: string;
},
): Record<string, unknown> {
const { objectName, record, params, recordId } = input;
const seeded: Record<string, unknown> = { ...record };

// `recordIdField` names the row field whose value seeds the key (default
// `id`) — a declaration may want a non-id value (spec: `token` for
// revoke-session). Fall back to the explicit recordId when the record
// never loaded (a record-less / new-record invocation).
const idField: string = typeof action?.recordIdField === 'string' && action.recordIdField
? action.recordIdField
: 'id';
const rowId: unknown = record?.[idField] ?? (idField === 'id' ? recordId : undefined);

if (rowId != null) {
const keys = new Set<string>(['recordId']);
if (objectName && objectName !== 'global') {
keys.add(`${objectName.replace(/_([a-z])/g, (_m: string, c: string) => c.toUpperCase())}Id`);
}
if (typeof action?.recordIdParam === 'string' && action.recordIdParam) {
keys.add(action.recordIdParam);
}
for (const key of keys) {
if (seeded[key] === undefined) seeded[key] = rowId;
}
}

return { ...seeded, ...params };
}

/**
* Dispatch a `type: 'flow'` action through the automation service.
*
Expand All @@ -333,18 +393,25 @@ export function flowActionUnavailableError(action: any): string {
* what lets a `runAs: 'user'` flow enforce RLS as the invoker instead of
* falling into the user-less UNSCOPED path (#2849, ADR-0049 / #1888; mirrors
* the record-change trigger's context shape).
*
* The params bag is seeded exactly like `POST /automation/:name/trigger`
* (`domains/automation.ts`) — see {@link seedFlowActionParams}. Invoking a
* flow ACTION and triggering its flow directly must land the same run, or
* "the actions endpoint dispatches flows for you" is a claim the runtime
* doesn't keep.
*/
export async function dispatchFlowAction(deps: ActionExecutionDeps,
action: any,
wiring: {
objectName: string;
record: Record<string, unknown>;
params: Record<string, unknown>;
recordId?: string;
ec: any;
envId?: string;
},
): Promise<any> {
const { objectName, record, params, ec, envId } = wiring;
const { objectName, record, params, recordId, ec, envId } = wiring;
const automation = await resolveAutomationService(deps, envId);
if (!automation) {
throw new Error(flowActionUnavailableError(action));
Expand All @@ -358,9 +425,7 @@ export async function dispatchFlowAction(deps: ActionExecutionDeps,
...(Array.isArray(ec?.positions) && ec.positions.length ? { positions: ec.positions } : {}),
...(Array.isArray(ec?.permissions) && ec.permissions.length ? { permissions: ec.permissions } : {}),
...(ec?.tenantId ? { tenantId: ec.tenantId } : {}),
// Record fields seed flows' named `isInput` variables (like the
// record-change trigger); explicit action params win on clash.
params: { ...record, ...params },
params: seedFlowActionParams(deps, action, { objectName, record, params, recordId }),
});
if (result && typeof result === 'object' && 'success' in result && result.success === false) {
throw new Error(`Flow '${action.target}' failed: ${result.error ?? 'unknown error'}`);
Expand Down Expand Up @@ -645,7 +710,7 @@ export async function invokeBusinessAction(deps: ActionExecutionDeps,

// ── flow dispatch ── (shared with the REST /actions route, #3915)
if (action.type === 'flow') {
const result = await dispatchFlowAction(deps, action, { objectName, record, params, ec, envId });
const result = await dispatchFlowAction(deps, action, { objectName, record, params, recordId, ec, envId });
return { ok: true, action: action.name, objectName, ...(recordId ? { recordId } : {}), result };
}

Expand Down
1 change: 1 addition & 0 deletions packages/runtime/src/domains/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string
objectName,
record,
params: reqParams,
recordId,
ec,
envId: _context?.environmentId,
});
Expand Down
89 changes: 89 additions & 0 deletions packages/runtime/src/http-dispatcher.actions-type-dispatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,95 @@ describe('REST /actions — flow dispatch (#3915)', () => {
expect(res.response.body.data).toEqual({ success: true, data: { success: true, output: { converted: true } } });
});

// ── params seeding ── the half a mocked automation service could not
// catch. Found by invoking the CRM's real `crm_convert_lead` against a
// running server: the bag carried the record's `id` but never `recordId`,
// so the flow's `get_lead` node died on `{recordId}` resolving to nothing
// while `/automation/crm_convert_lead_wizard/trigger` ran the same flow
// fine. Invoking the ACTION and triggering its FLOW must land the same run.
it('seeds the row id under `recordId` and the `<objectName>Id` alias, like the trigger route', async () => {
const execute = vi.fn(async () => ({ success: true }));
const { dispatcher } = makeDispatcher({
objectDef: { name: 'crm_lead', actions: [flowAction] },
automation: { execute },
record: { id: 'lead_1', company: 'Radium Labs' },
});

await dispatcher.handleActions('/crm_lead/convert_lead/lead_1', 'POST', {}, ctxFor());

expect(execute.mock.calls[0]?.[1]).toMatchObject({
params: { id: 'lead_1', recordId: 'lead_1', crmLeadId: 'lead_1', company: 'Radium Labs' },
});
});

it('honours a declared `recordIdParam` naming a key of its own', async () => {
const execute = vi.fn(async () => ({ success: true }));
const { dispatcher } = makeDispatcher({
objectDef: {
name: 'crm_lead',
actions: [{ ...flowAction, recordIdParam: 'leadToConvert' }],
},
automation: { execute },
record: { id: 'lead_1' },
});

await dispatcher.handleActions('/crm_lead/convert_lead/lead_1', 'POST', {}, ctxFor());

expect((execute.mock.calls[0]?.[1] as any).params).toMatchObject({
leadToConvert: 'lead_1',
recordId: 'lead_1',
});
});

it('seeds from `recordIdField` when the declaration wants a non-id value', async () => {
const execute = vi.fn(async () => ({ success: true }));
const { dispatcher } = makeDispatcher({
objectDef: {
name: 'crm_lead',
actions: [{ ...flowAction, recordIdParam: 'token', recordIdField: 'session_token' }],
},
automation: { execute },
record: { id: 'lead_1', session_token: 'tok_abc' },
});

await dispatcher.handleActions('/crm_lead/convert_lead/lead_1', 'POST', {}, ctxFor());

expect((execute.mock.calls[0]?.[1] as any).params.token).toBe('tok_abc');
});

it('lets an explicit param win over every seeded key', async () => {
const execute = vi.fn(async () => ({ success: true }));
const { dispatcher } = makeDispatcher({
objectDef: { name: 'crm_lead', actions: [flowAction] },
automation: { execute },
record: { id: 'lead_1' },
});

await dispatcher.handleActions(
'/crm_lead/convert_lead/lead_1',
'POST',
{ params: { recordId: 'explicit_override' } },
ctxFor(),
);

expect((execute.mock.calls[0]?.[1] as any).params.recordId).toBe('explicit_override');
});

it('seeds `recordId` from the URL even when the record never loaded', async () => {
// New-record / unreadable-record invocations pass an empty record; the
// flow still needs the id the caller named.
const execute = vi.fn(async () => ({ success: true }));
const { dispatcher } = makeDispatcher({
objectDef: { name: 'crm_lead', actions: [flowAction] },
automation: { execute },
// no `record` → the best-effort load returns nothing
});

await dispatcher.handleActions('/crm_lead/convert_lead/lead_404', 'POST', {}, ctxFor());

expect((execute.mock.calls[0]?.[1] as any).params.recordId).toBe('lead_404');
});

it('forwards the caller identity so a `runAs: user` flow enforces RLS as the invoker', async () => {
const execute = vi.fn(async () => ({ success: true }));
const { dispatcher } = makeDispatcher({
Expand Down
Loading