Skip to content

Commit 2116dde

Browse files
committed
fix(automation): answer real HTTP status codes on both trigger routes
POST /api/v1/automation/:name/trigger and the legacy POST /api/v1/automation/trigger/:name both ended `deps.success(result)` unconditionally, so a flow that ran and failed came back as HTTP 200 wrapping an inner {success:false} — the double envelope #3962 ruled out for /actions and #8684 closed on the resume route. Producer first: the engine stamps `status: 'failed'` on the exits that dispatched a run and were rejected (the same verdict it already writes to the run log); its never-dispatched exits carry no status. The route reads that verdict rather than sniffing summary/durationMs, and both doors now answer through one shared mapper. - ran and failed (incl. the retry-strategy exit) -> 400 FLOW_FAILED, with the author's errorMessage and the run summary in error.details - unknown flow -> 404, through the same registry probe /:name/toggle and GET /:name use, before anything is dispatched The ruling's disabled (409) and no-start-node (422) rows are NOT implemented: both are never-dispatched exits and the closed AutomationResult.code union has no honest member to tell them apart. Escalated on the card rather than guessed; they keep today's behaviour and are pinned as unchanged.
1 parent bc6434b commit 2116dde

8 files changed

Lines changed: 724 additions & 11 deletions

File tree

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
---
2+
'@objectstack/service-automation': minor
3+
'@objectstack/runtime': minor
4+
'@objectstack/client': minor
5+
---
6+
7+
**BREAKING** — both automation `trigger` routes answer real HTTP status codes for a flow
8+
that ran and failed, instead of HTTP 200 wrapping an inner `{success: false}`.
9+
10+
This is the second and wider half of the migration the resume route shipped in the same
11+
release (#8684, merged 2026-08-17): one SDK-visible behaviour change, one migration note.
12+
The resume flip touched the screen-flow runner; this one touches the door every app
13+
dispatches flows through.
14+
15+
Until now a flow driven to a node failure answered:
16+
17+
```
18+
HTTP 200
19+
{"success":true,"data":{"success":false,"error":"Node 'create_opportunity' failed: …"}}
20+
```
21+
22+
The run genuinely failed; the transport reported success. A scripted or integration caller
23+
that branches on the HTTP status alone read a failed run as a successful one. This applies
24+
the `/actions` ruling (business failures must not ride HTTP 200 inside a double envelope)
25+
to `POST /api/v1/automation/:name/trigger` and to the legacy
26+
`POST /api/v1/automation/trigger/:name` — the shape `client.automation.trigger()` calls.
27+
Both doors answer through one mapper, so they cannot drift.
28+
29+
What changes on the wire:
30+
31+
- **A flow that ran and then failed ⇒ `400` with `error.code: 'FLOW_FAILED'`.** The node
32+
failure stays the human-readable `error.message`. The flow author's own `errorMessage`
33+
travels in `error.details.errorMessage` — one documented location, the same one the
34+
console reads — and the run's per-node accounting in `error.details.summary`. A flow
35+
whose `errorHandling.strategy` is `retry` answers the same way once its attempts are
36+
exhausted. `durationMs` is no longer carried on this response.
37+
- **A flow name the deployment does not hold ⇒ `404`,** answered before anything is
38+
dispatched, through the same registry probe `POST /:name/toggle` and `GET /:name` use.
39+
- **Unchanged:** a successful run still answers 200 with its result, and a run that PAUSED
40+
at a `screen` node still answers 200 with the next screen — a pause is not a failure.
41+
- **Also unchanged, pending a ruling:** a DISABLED flow and one with no start node still
42+
answer 200 with the inner failure. Both are exits that never dispatched anything, and
43+
telling them apart needs a producer-side classification the closed
44+
`AutomationResult.code` union cannot yet express. Tracked on #9378.
45+
46+
**`@objectstack/service-automation`:** `execute()` now stamps `status: 'failed'` on the
47+
results of runs that dispatched and were rejected — the same lifecycle verdict it already
48+
writes to the run log. Its never-dispatched exits carry no `status`, which is what lets a
49+
transport answer the two classes differently without inspecting the result's internals.
50+
51+
**`@objectstack/client`:** `client.automation.trigger()`, `client.automation.execute()` and
52+
`client.project(id).automation.execute()` now **reject** on a failed run instead of
53+
resolving with `{ success: false, error }` — the SDK throws on every non-2xx before
54+
unwrapping. Callers that inspected the resolved value must move to a `catch`:
55+
56+
```ts
57+
try {
58+
await client.automation.execute(flow, { params });
59+
} catch (err: any) {
60+
err.code; // 'FLOW_FAILED' (400) — the run ran and failed
61+
err.httpStatus; // 400 | 404
62+
err.message; // the node failure, verbatim
63+
err.details?.errorMessage; // the flow author's own message, when the flow declares one
64+
err.details?.summary; // which node failed
65+
}
66+
```
67+
68+
Raw-HTTP callers that treated `2xx` as success and never opened the inner envelope now see
69+
the failure they were already being told about, one level up.
70+
71+
<!-- adr-0087: not-required (no-migration-prescription) retires no metadata surface: no Zod schema, no authorable key, no stored sys_metadata row changes shape, so `objectstack migrate meta` has nothing to rewrite and no ledger entry can be written for it. What changes is an HTTP status plus an SDK method's promise contract, and the only channel that reaches those consumers is this changeset itself. -->

packages/client/src/client.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,6 +1354,90 @@ describe('ObjectStackClient.automation', () => {
13541354
expect(err.message).toMatch(/no longer exists in flow/);
13551355
});
13561356

1357+
// [#9378] BREAKING, and the wide half of the same flip: DISPATCHING a flow.
1358+
// `client.automation.trigger()` (legacy route) and `.execute()` used to
1359+
// resolve with `{ success: false, error }` under HTTP 200 for a run that
1360+
// ran and failed — every app dispatches flows through this door, so a
1361+
// caller that never opened the inner envelope read every failed run as a
1362+
// successful one. The route answers 400 `FLOW_FAILED` now and this SDK's
1363+
// fetch layer throws on non-2xx before any unwrapping, so both surfaces
1364+
// REJECT. No SDK code changed; the contract did, and these are its pins.
1365+
//
1366+
// Both spellings are pinned, not one: `trigger()` reads `res.json()` while
1367+
// `execute()` reads `unwrapResponse()`, so a regression in either unwrap
1368+
// path would be invisible from the other's test.
1369+
const failedRunBody = {
1370+
success: false,
1371+
error: {
1372+
code: 'FLOW_FAILED',
1373+
message: "Node 'create_opportunity' failed: Amount must be greater than zero",
1374+
httpStatus: 400,
1375+
details: {
1376+
errorMessage: 'We could not create the opportunity — check the amount and try again.',
1377+
summary: { nodes: [{ nodeId: 'create_opportunity', status: 'failure' }] },
1378+
},
1379+
},
1380+
};
1381+
1382+
it('should reject with FLOW_FAILED when a triggered flow runs and fails (legacy trigger)', async () => {
1383+
const { client } = createMockClient(failedRunBody, 400);
1384+
1385+
const err: any = await client.automation
1386+
.trigger('my_flow', { amount: 0 })
1387+
.then(() => { throw new Error('expected the failed trigger to reject'); }, (e) => e);
1388+
1389+
// The classification, not merely that it threw: an SDK rejecting with a
1390+
// bare `Error` would satisfy `.rejects.toThrow()` while losing
1391+
// everything a caller branches on.
1392+
expect(err.code).toBe('FLOW_FAILED');
1393+
expect(err.httpStatus).toBe(400);
1394+
expect(err.message).toMatch(/Node 'create_opportunity' failed/);
1395+
// The flow author's own text keeps its one documented location — the
1396+
// ADR-0112 envelope has no `data`, and the console reads it from here.
1397+
expect(err.details?.errorMessage)
1398+
.toBe('We could not create the opportunity — check the amount and try again.');
1399+
});
1400+
1401+
it('should reject with FLOW_FAILED when execute() dispatches a flow that fails', async () => {
1402+
const { client } = createMockClient(failedRunBody, 400);
1403+
1404+
const err: any = await client.automation
1405+
.execute('my_flow', { params: { amount: 0 } })
1406+
.then(() => { throw new Error('expected the failed execute to reject'); }, (e) => e);
1407+
1408+
expect(err.code).toBe('FLOW_FAILED');
1409+
expect(err.httpStatus).toBe(400);
1410+
expect(err.details?.summary?.nodes?.[0]?.status).toBe('failure');
1411+
});
1412+
1413+
it('should reject with 404 when the triggered flow does not exist', async () => {
1414+
const { client } = createMockClient({
1415+
success: false,
1416+
error: { code: 'RESOURCE_NOT_FOUND', message: "Flow 'no_such_flow' not found", httpStatus: 404 },
1417+
}, 404);
1418+
1419+
const err: any = await client.automation
1420+
.execute('no_such_flow', {})
1421+
.then(() => { throw new Error('expected the unknown flow to reject'); }, (e) => e);
1422+
1423+
expect(err.httpStatus).toBe(404);
1424+
expect(err.code).not.toBe('FLOW_FAILED');
1425+
expect(err.message).toContain('no_such_flow');
1426+
});
1427+
1428+
it('should still resolve when a triggered flow succeeds', async () => {
1429+
// The other half of the contract: a successful dispatch is untouched,
1430+
// including the paused screen-flow shape the runner drives.
1431+
const { client } = createMockClient({
1432+
success: true,
1433+
data: { success: true, status: 'paused', runId: 'run_1', screen: { nodeId: 'collect', fields: [] } },
1434+
}, 200);
1435+
1436+
const result: any = await client.automation.execute('my_flow', {});
1437+
expect(result.status).toBe('paused');
1438+
expect(result.runId).toBe('run_1');
1439+
});
1440+
13571441
it('should fetch the screen a paused run awaits', async () => {
13581442
const { client, fetchMock } = createMockClient({
13591443
success: true,

packages/client/src/index.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3016,6 +3016,28 @@ export class ObjectStackClient {
30163016
automation = {
30173017
/**
30183018
* Trigger a named automation flow (legacy endpoint)
3019+
*
3020+
* **BREAKING since #9378 — a failed run REJECTS instead of resolving.**
3021+
* A flow that ran and then failed used to come back as a resolved
3022+
* `{ success: false, error }` riding HTTP 200, so a caller that did not
3023+
* open the inner envelope read a failed run as a successful one. The
3024+
* route answers **400** `FLOW_FAILED` now (inheriting #3962's ruling for
3025+
* `/actions`, applied to the resume route by #8684), and every non-2xx
3026+
* throws out of this SDK's fetch layer — so this promise **rejects**:
3027+
*
3028+
* ```ts
3029+
* try { await client.automation.trigger(flow, payload); }
3030+
* catch (err: any) {
3031+
* err.code; // 'FLOW_FAILED'
3032+
* err.httpStatus; // 400
3033+
* err.message; // the node failure, verbatim
3034+
* err.details?.errorMessage; // the flow author's `errorMessage`
3035+
* err.details?.summary; // per-node accounting of the failed run
3036+
* }
3037+
* ```
3038+
*
3039+
* A flow name the deployment does not hold rejects with **404** instead.
3040+
* A run that PAUSED at a screen node is not a failure and still resolves.
30193041
*/
30203042
trigger: async (triggerName: string, payload: any) => {
30213043
const route = this.getRoute('automation');
@@ -3168,7 +3190,15 @@ export class ObjectStackClient {
31683190
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(name)}`);
31693191
return this.unwrapResponse(res) as Promise<T>;
31703192
},
3171-
/** Execute (trigger) a flow with an execution context. */
3193+
/**
3194+
* Execute (trigger) a flow with an execution context.
3195+
*
3196+
* **BREAKING since #9378**: a run that ran and failed now REJECTS with
3197+
* `400` `FLOW_FAILED` (author text on `err.details.errorMessage`, per-node
3198+
* accounting on `err.details.summary`) instead of resolving with an inner
3199+
* `{ success: false }` under HTTP 200; an unknown flow rejects with `404`.
3200+
* See `automation.trigger` for the full shape — both call the same door.
3201+
*/
31723202
execute: async <T = any>(name: string, ctx?: Record<string, any>): Promise<T> => {
31733203
const route = this.getRoute('automation');
31743204
const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(name)}/trigger`, {
@@ -5342,6 +5372,12 @@ export class ScopedProjectClient {
53425372
/**
53435373
* Execute (trigger) a flow by name. The request body is forwarded as the
53445374
* automation execution context (e.g. `{ params, trigger }`).
5375+
*
5376+
* Mirrors the unscoped `client.automation.execute` — including its
5377+
* **breaking #9378 behaviour**: a run that ran and then failed REJECTS with
5378+
* `400` `FLOW_FAILED` (author text on `err.details.errorMessage`) instead of
5379+
* resolving with an inner `{ success: false }` under HTTP 200, and an
5380+
* unknown flow rejects with `404`. See that method for the full shape.
53455381
*/
53465382
execute: async <T = any>(name: string, ctx?: Record<string, any>): Promise<T> => {
53475383
const res = await this.parent._fetch(this.url(`/automation/${encodeURIComponent(name)}/trigger`), {

packages/runtime/src/domain-handler-registry.test.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,12 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
620620
*/
621621
it('both trigger routes translate the body and forward the caller identity', async () => {
622622
const execute = vi.fn().mockResolvedValue({ success: true });
623-
const automation = { execute, listFlows: vi.fn(), getFlow: vi.fn() };
623+
// [#9378] `getFlow` resolves the flow rather than `undefined`: both
624+
// trigger doors now answer 404 through the same shared existence probe
625+
// `POST /:name/toggle` and `GET /:name` use, so a fixture whose
626+
// registry claims the flow does not exist never reaches `execute` —
627+
// which is this test's subject.
628+
const automation = { execute, listFlows: vi.fn(), getFlow: vi.fn().mockResolvedValue({ name: 'nurture' }) };
624629
const ctx: any = {
625630
executionContext: {
626631
userId: 'u-1',
@@ -668,7 +673,9 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
668673
it('never calls a non-contract `trigger` method, even when one exists', async () => {
669674
const trigger = vi.fn().mockResolvedValue({ success: true });
670675
const execute = vi.fn().mockResolvedValue({ success: true });
671-
const automation = { trigger, execute, listFlows: vi.fn(), getFlow: vi.fn() };
676+
// [#9378] See the note on the fixture above: the trigger door consults
677+
// the shared existence probe before dispatching.
678+
const automation = { trigger, execute, listFlows: vi.fn(), getFlow: vi.fn().mockResolvedValue({ name: 'nurture' }) };
672679

673680
const result = await makeDispatcher({ automation, auth })
674681
.dispatch('POST', '/automation/trigger/nurture', {}, {}, {} as any);

0 commit comments

Comments
 (0)