diff --git a/.changeset/actions-flow-dispatch-status-table.md b/.changeset/actions-flow-dispatch-status-table.md new file mode 100644 index 0000000000..b13cc27dfa --- /dev/null +++ b/.changeset/actions-flow-dispatch-status-table.md @@ -0,0 +1,22 @@ +--- +'@objectstack/runtime': minor +--- + +`POST /api/v1/actions/:object/:action` answers the flow-dispatch status table instead of one blanket `400 FLOW_FAILED` (#9446). + +**What a caller sees differently.** A `type: 'flow'` action whose dispatch is REFUSED no longer reports a failed run. Three answers changed: + +| the flow behind the action | before | now | +|---|---|---| +| is not registered | `400` `FLOW_FAILED` | `404` `RESOURCE_NOT_FOUND` | +| is switched off | `400` `FLOW_FAILED` | `409` `FLOW_DISABLED` | +| has no `start` node | `400` `FLOW_FAILED` | `422` `FLOW_NO_START_NODE` | +| ran and was rejected | `400` `FLOW_FAILED` | `400` `FLOW_FAILED` (unchanged) | + +These are the same four rows `POST /api/v1/automation/:name/trigger` has answered since #9378 + #9415, and they now come from one shared definition both doors read, so the two cannot drift apart again. + +**Behaviourally breaking for a caller that branches on the status or the code.** Every one of these was a `400` before, so a caller treating `400` as "the run failed" was being told something false in three of the four cases: nothing had dispatched and no node had executed. A client that lumps all four together keeps working — they are all still refusals, all still `success: false` with no inner envelope — but one that reports "the flow failed" on a `400` should now distinguish. **Retry semantics differ per row**, which is the practical reason to: `409 FLOW_DISABLED` is reversible operational state (enable the flow and the identical request succeeds), while `404` and `422 FLOW_NO_START_NODE` are authoring defects that no retry fixes. `400 FLOW_FAILED` remains terminal, exactly as the console already treats it. + +**Unchanged on purpose.** A successful run still answers `200` with the single `data` wrap (#3962). The `400 FLOW_FAILED` message keeps its existing wording (`Flow '' failed: …`), which names the flow the action dispatches — the trigger route's URL carries that name and this route's does not. A `success: false` result the automation engine did not classify still refuses with `400 FLOW_FAILED` rather than falling back to `200 {success:true,data:{success:false}}` — the double envelope #3962 removed from this route. + +**Not in scope.** Declared endpoints (`type: 'flow'` endpoints, `endpoint-executor.ts`) still answer `200` for every outcome. That door converges in its own change (#9462), where the envelope flip is a breaking change for consumers of the current double envelope and is sequenced against them. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 536c470b14..4fed46c324 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -1310,6 +1310,14 @@ to do it — never the message text: The first three never dispatched anything: no node executed, no record was written, and there is no run to look up. Only `400` describes a run. +**The same table answers at the action door.** Invoking a `type: 'flow'` +[action](/docs/ui/actions) through `POST /api/v1/actions/:object/:action` +dispatches the same flow through the same service call, and it classifies the +outcome the same way — the table above has one definition that both doors read, +so they cannot answer one engine outcome differently. Declared endpoints +(`type: 'flow'`) are the remaining exception: they still answer `200` for every +outcome, so read `data.success` there rather than the status. + ``` POST /api/v1/automation/order_approval/trigger diff --git a/content/docs/protocol/kernel/http-protocol.mdx b/content/docs/protocol/kernel/http-protocol.mdx index ec86a8bfca..3b9b1260b5 100644 --- a/content/docs/protocol/kernel/http-protocol.mdx +++ b/content/docs/protocol/kernel/http-protocol.mdx @@ -1216,7 +1216,7 @@ declaration to shadow a built-in route: | Endpoint declares | Answer | |:---|:---| | `type: 'object_operation'` | delegated to the same `callData` binding that serves `/api/v1/data/{object}` — byte-identical `data` | -| `type: 'flow'` | delegated to the same automation pipeline as `POST /api/v1/automation/{name}/trigger` — the same execution context builder and the same `execute` call, so the run itself is identical. **The response is not**: the trigger route classifies a refused or failed run into real status codes (404 / 409 `FLOW_DISABLED` / 422 `FLOW_NO_START_NODE` / 400 `FLOW_FAILED`), while this seam still answers `200` with the result in `data` for every outcome ([#9446](https://github.com/objectstack-ai/objectstack/issues/9446)). Read `data.success` here, not the status | +| `type: 'flow'` | delegated to the same automation pipeline as `POST /api/v1/automation/{name}/trigger` — the same execution context builder and the same `execute` call, so the run itself is identical. **The response is not**: the trigger route classifies a refused or failed run into real status codes (404 / 409 `FLOW_DISABLED` / 422 `FLOW_NO_START_NODE` / 400 `FLOW_FAILED`), and `POST /api/v1/actions/{object}/{action}` answers that same table since #9446, while this seam still answers `200` with the result in `data` for every outcome ([#9462](https://github.com/objectstack-ai/objectstack/issues/9462)). Read `data.success` here, not the status | | `authRequired: true` (or omitted) + anonymous caller | `401` `UNAUTHENTICATED`, the same envelope every seam answers | | `rateLimit` armed and exhausted | `429` + `Retry-After`, never with a cache directive | | `cacheTtl: 30` on a successful GET | `Cache-Control: private, max-age=30` — `private` is a security rule, not tuning: any response can be RLS-trimmed | diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index ccc5856f5a..766d7e5b44 100644 --- a/content/docs/ui/actions.mdx +++ b/content/docs/ui/actions.mdx @@ -343,7 +343,7 @@ The endpoint dispatches on the **declared `type`**, exactly like the MCP | `type` | Over REST | |:---|:---| | `script` | Runs the registered handler / inline body. | -| `flow` | Runs `target` on the automation engine, with your identity forwarded (a `runAs: 'user'` flow enforces RLS as you). Dispatches the same flow as `POST /api/v1/automation/:target/trigger`, without having to know the flow name. ⚠️ It does **not** answer the same way: any unsuccessful outcome comes back as **400** `FLOW_FAILED`, where the trigger route separates a run that failed (400) from one that was never dispatched (404 / 409 / 422) — see [#9446](https://github.com/objectstack-ai/objectstack/issues/9446). | +| `flow` | Runs `target` on the automation engine, with your identity forwarded (a `runAs: 'user'` flow enforces RLS as you). Dispatches the same flow as `POST /api/v1/automation/:target/trigger`, without having to know the flow name — **and answers the same way**: a run that ran and was rejected is **400** `FLOW_FAILED`, while a dispatch that never happened is separated out (**404** unknown flow / **409** `FLOW_DISABLED` / **422** `FLOW_NO_START_NODE`). See [Run a flow via API](/docs/automation/flows#run-a-flow-via-api) for the full table — it is one table, read by both doors. | | `api` | **400** — it dispatches on `target`; call that endpoint directly. | | `url` / `modal` / `form` | **400** — client-side navigation; there is nothing for the server to run. | diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index de070d1881..b0f9b7ccff 100644 --- a/packages/runtime/src/action-execution.ts +++ b/packages/runtime/src/action-execution.ts @@ -19,6 +19,15 @@ import { validateActionParams, type ActionSession, type ResolvedActionParam } fr import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { IObjectQLEngine, ServiceSlotContract, ServiceSlotContracts } from '@objectstack/spec/contracts'; import { checkApiExposure } from './api-exposure.js'; +// [#9446] The ONE #9378 status table. Imported rather than re-read here: this +// door's blanket `FLOW_FAILED` was the second of three readings of one engine +// result, and a second definition of the rule is what let the doors diverge. +import { + classifyFlowRefusal, + flowIsUnknown, + flowNotFoundMessage, + FLOW_NOT_FOUND_STATUS, +} from './flow-dispatch-status.js'; // [#5138] The ONE 404 envelope a single-record path answers. Imported rather // than re-spelled so `callData`'s ObjectQL fallback and the protocol service it // falls back FROM cannot disagree about what "this id names no row" looks like. @@ -561,10 +570,33 @@ export function seedFlowActionParams(_deps: ActionExecutionDeps, * The ONE implementation both headless surfaces share — the MCP `run_action` * tool and the REST `/actions/:object/:action` route (#3915, which is exactly * the asymmetry that let this branch exist on only one of them). Throws on a - * missing automation service and converts a `{ success: false }` engine result - * into a throw so both callers report failure the same way; returns the raw + * missing automation service and converts a refused or failed dispatch into a + * throw so both callers report failure the same way; returns the raw * automation result otherwise. * + * [#9446] **The refusal it throws is the #9378 table, read from the ONE + * definition** (`./flow-dispatch-status.js`) that the trigger door reads too: + * + * | engine exit | this door answers | + * |------------------------|----------------------------| + * | flow not found | `404` | + * | flow disabled | `409` `FLOW_DISABLED` | + * | flow has no start node | `422` `FLOW_NO_START_NODE` | + * | ran and failed | `400` `FLOW_FAILED` | + * + * Maintainer ruling (2026-08-18, verbatim 「同意」): the table is a property of + * the flow-dispatch CONTRACT, not of the trigger route, so this door converges + * on it rather than keeping its own reading. It used to map EVERY + * `success: false` to `400 FLOW_FAILED` under a comment asserting "The flow + * RAN and rejected" — a false statement for the two never-dispatched exits it + * caught, told to a caller whose only machine-readable signal is that code. + * + * The throw carries `status` and `code` and the route serves them through + * `errorFromThrown`; `error.details` is whatever `resolveThrownHttpError` + * reads off a thrown value, so the trigger door's `errorMessage` / `summary` + * details do NOT ride this door — see the shared module's note on what the + * table deliberately does not answer. + * * Forwarding the caller's identity (rather than just executing the flow) is * 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 @@ -593,6 +625,16 @@ export async function dispatchFlowAction(deps: ActionExecutionDeps, if (!automation) { throw new Error(flowActionUnavailableError(action)); } + // [#9446] Row 1 of the table, answered by the SAME optional `getFlow` + // registry probe the trigger door uses — the engine's not-found exit + // carries no classification, so this is the only way to read it that is not + // a regex over its message. A service that omits `getFlow` cannot be asked + // and dispatches as before. + if (await flowIsUnknown(automation, action.target)) { + const err: any = new Error(flowNotFoundMessage(action.target)); + err.status = FLOW_NOT_FOUND_STATUS; + throw err; + } // Pass a proper AutomationContext (the engine never read the former // `triggerData` envelope). const result: any = await automation.execute(action.target, { @@ -604,11 +646,40 @@ export async function dispatchFlowAction(deps: ActionExecutionDeps, ...(ec?.tenantId ? { tenantId: ec.tenantId } : {}), params: seedFlowActionParams(deps, action, { objectName, record, params, recordId }), }); + // [#9446] Rows 2-4, read off the PRODUCER's classification through the one + // shared table. What stood here mapped every `success: false` to + // `400 FLOW_FAILED` under a comment claiming "the flow RAN and rejected" — + // false for two of the exits it caught, and the producer's own `code` was + // available and ignored. A disabled flow invoked through an action told the + // caller a run had failed when no node ever executed. + const refusal = classifyFlowRefusal(action.target, result); + if (refusal) { + const err: any = new Error( + // The ran-and-failed row keeps THIS door's wording, byte for byte: + // it has been on the wire since #3962, the ruling is about status + // and code, and re-labelling a message nobody asked about would be + // an unruled change riding along. It also names the flow, which + // this door needs and the trigger door does not — the flow name is + // in that route's URL and is nowhere in this one. The two + // never-dispatched rows are NEW here, so they take the shared + // table's message: the producer's own words, exactly as the + // trigger door serves them. + refusal.code === 'FLOW_FAILED' + ? `Flow '${action.target}' failed: ${result.error ?? 'unknown error'}` + : refusal.message, + ); + err.status = refusal.status; + err.code = refusal.code; + throw err; + } + // An UNCLASSIFIED `success: false` still refuses, and `FLOW_FAILED` stays + // its answer — deliberately NOT the trigger door's 200. This route settled + // in #3962 that failures speak HTTP, so the alternative residual here is + // the `200 {success:true,data:{success:false}}` double envelope that + // ruling removed. `FLOW_FAILED` is what this exit has answered all along; + // narrowing which refusals reach it is this card's change, re-labelling + // the residual is not. if (result && typeof result === 'object' && 'success' in result && result.success === false) { - // The flow RAN and rejected — a deliberate business rejection, served - // as a 400 (#3962). Tagging the status/code here (rather than relying - // on the route's name heuristic) keeps the semantic `FLOW_FAILED` on - // the wire for callers that branch on `err.code`. const err: any = new Error(`Flow '${action.target}' failed: ${result.error ?? 'unknown error'}`); err.status = 400; err.code = 'FLOW_FAILED'; diff --git a/packages/runtime/src/actions-flow-dispatch-status.test.ts b/packages/runtime/src/actions-flow-dispatch-status.test.ts new file mode 100644 index 0000000000..0379bc66d3 --- /dev/null +++ b/packages/runtime/src/actions-flow-dispatch-status.test.ts @@ -0,0 +1,318 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9446 — `POST /api/v1/actions/:object/:action` with `type: 'flow'` answers + * the #9378 status table, the same one the trigger routes answer. + * + * `dispatchFlowAction` (`action-execution.ts`) mapped **every** + * `success: false` result to one answer: + * + * ```ts + * err.status = 400; + * err.code = 'FLOW_FAILED'; + * ``` + * + * under a comment asserting *"The flow RAN and rejected"* — false for two of + * the four exits it caught. A DISABLED flow invoked through an action came + * back as `400 FLOW_FAILED`, telling the caller a run had failed when no node + * ever executed, and the producer's own `result.code` was available and + * ignored. Maintainer ruling, 2026-08-18, verbatim 「同意」: the table is a + * property of the flow-dispatch CONTRACT, not of the trigger route, and this + * door converges on it now. + * + * ## What this file pins, and why in two halves + * + * A suite that only asserted the new codes would stay green under a regression + * that made every exit answer ONE code again — every row would still "have" + * its code if they all shared it. So both halves are pinned: + * + * 1. each row answers its own status AND its own code, and + * 2. the rows are DISTINGUISHABLE from each other — asserted as a set, so a + * collapse back to one answer reddens here whatever that one answer is. + * + * The second describe block pins the convergence itself: the same engine + * result, driven through BOTH doors, answers the same status and the same + * code. That is the ruling's actual content — a per-door assertion can be + * satisfied by two copies of a rule, and two copies drifting apart is the + * defect this card exists to close. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { AutomationResult } from '@objectstack/spec/contracts'; + +import { HttpDispatcher } from './http-dispatcher.js'; + +const FLOW = 'crm_convert_lead_wizard'; + +const flowAction = { + name: 'convert_lead', + label: 'Convert Lead', + objectName: 'crm_lead', + type: 'flow', + target: FLOW, +}; + +/** + * A dispatcher serving both doors from ONE automation service — the shape the + * real engine presents: `getFlow` reads the same flow map `execute` reads + * (`engine.ts`), so it resolves `null` for exactly the names `execute` would + * answer "not found" for, and `execute` returns an `AutomationResult` rather + * than throwing. + */ +function makeDispatcher(opts: { + result?: AutomationResult; + flows?: string[]; + omitGetFlow?: boolean; +} = {}) { + const names = opts.flows ?? [FLOW]; + const held = new Map(names.map((n) => [n, { name: n }])); + const execute = vi.fn(async (): Promise => opts.result ?? { success: true, output: {} }); + const getFlow = vi.fn(async (name: string) => held.get(name) ?? null); + const automation: Record = opts.omitGetFlow ? { execute } : { execute, getFlow }; + + const objectDef = { name: 'crm_lead', actions: [flowAction] }; + const executeAction = vi.fn(async () => ({ ran: 'script' })); + 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 resolve = (n: string) => + n === 'objectql' || n === 'data' ? ql + : n === 'metadata' ? metadata + : n === 'automation' ? automation + : null; + const kernel: any = { + getService: resolve, + getServiceAsync: async (n: string) => resolve(n), + context: { getService: resolve }, + }; + return { dispatcher: new HttpDispatcher(kernel), execute, getFlow, executeAction }; +} + +const CTX: any = { + request: {}, + environmentId: 'platform', + executionContext: { userId: 'u1', systemPermissions: [] }, +}; + +/** Invoke the flow ACTION — door 2. */ +const viaAction = (d: HttpDispatcher, action = 'convert_lead') => + d.handleActions(`/crm_lead/${action}`, 'POST', {}, CTX); + +/** Trigger the same flow directly — door 1, the reference implementation. */ +const viaTrigger = (d: HttpDispatcher, flow = FLOW) => + d.handleAutomation(`/${flow}/trigger`, 'POST', {}, CTX); + +/** The engine's ran-and-failed exit: `status: 'failed'` is the producer's verdict. */ +const RAN_AND_FAILED: AutomationResult = { + success: false, + status: 'failed', + error: "Node 'create_opportunity' failed: Amount must be greater than zero", + durationMs: 45, +}; + +/** The engine's two never-dispatched exits, each stamped with its own code (#9415). */ +const DISABLED: AutomationResult = { + success: false, code: 'FLOW_DISABLED', error: `Flow '${FLOW}' is disabled`, +}; +const NO_START_NODE: AutomationResult = { + success: false, code: 'FLOW_NO_START_NODE', error: 'Flow has no start node', +}; + +describe('#9446 — /actions answers the four-row flow-dispatch table', () => { + it('row 1: a flow the service does not hold is 404, and is never dispatched', async () => { + const { dispatcher, execute, getFlow } = makeDispatcher({ flows: [] }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(404); + // Named, so the caller knows WHICH name failed to resolve — and that it + // is the FLOW behind the action, not the action itself, that is missing. + expect(res.response.body.error.message).toContain(FLOW); + // The same shared probe the trigger door uses, asked with the action's + // declared target; the engine is never asked to run a name nothing holds. + expect(getFlow).toHaveBeenCalledWith(FLOW); + expect(execute).not.toHaveBeenCalled(); + // ⛔ Never the old blanket answer: nothing ran, so "the flow failed" is + // a false statement about what happened. + expect(res.response.body.error.code).not.toBe('FLOW_FAILED'); + }); + + it('row 2: a disabled flow is 409 FLOW_DISABLED, not a failed run', async () => { + const { dispatcher } = makeDispatcher({ result: DISABLED }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(409); + expect(res.response.body.error.code).toBe('FLOW_DISABLED'); + expect(res.response.body.error.httpStatus).toBe(409); + // The engine's own words survive — an operator needs to know WHICH flow + // was refused and why enabling it will help. + expect(res.response.body.error.message).toBe(DISABLED.error); + expect(res.response.body.error.code).not.toBe('FLOW_FAILED'); + // The #3962 single wrap holds: no inner envelope for a status-blind + // caller to misread. + expect(res.response.body.data).toBeUndefined(); + expect(res.response.body.success).toBe(false); + }); + + it('row 3: a flow with no start node is 422 FLOW_NO_START_NODE', async () => { + const { dispatcher } = makeDispatcher({ result: NO_START_NODE }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(422); + expect(res.response.body.error.code).toBe('FLOW_NO_START_NODE'); + expect(res.response.body.error.httpStatus).toBe(422); + expect(res.response.body.error.message).toBe(NO_START_NODE.error); + expect(res.response.body.error.code).not.toBe('FLOW_FAILED'); + expect(res.response.body.data).toBeUndefined(); + expect(res.response.body.success).toBe(false); + }); + + it('row 4: a run that dispatched and was rejected is still 400 FLOW_FAILED', async () => { + const { dispatcher } = makeDispatcher({ result: RAN_AND_FAILED }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(400); + expect(res.response.body.error.code).toBe('FLOW_FAILED'); + expect(res.response.body.error.httpStatus).toBe(400); + // This door names the flow in its message and the trigger door does + // not, deliberately: the flow name is in the trigger route's URL and is + // nowhere in this one — a caller here asked for an ACTION. + expect(res.response.body.error.message).toContain(FLOW); + expect(res.response.body.error.message).toContain("Node 'create_opportunity' failed"); + }); + + it('the four rows are DISTINGUISHABLE — the half a per-row assertion cannot see', async () => { + // A regression that collapsed the table back to one answer would leave + // every per-row assertion above satisfiable by that one answer if it + // happened to be the row's own. Asserted as a SET, it cannot. + const answers = await Promise.all( + [ + makeDispatcher({ flows: [] }), + makeDispatcher({ result: DISABLED }), + makeDispatcher({ result: NO_START_NODE }), + makeDispatcher({ result: RAN_AND_FAILED }), + ].map(async ({ dispatcher }) => { + const res: any = await viaAction(dispatcher); + return { status: res.response.status, code: res.response.body.error.code }; + }), + ); + + expect(answers.map((a) => a.status)).toEqual([404, 409, 422, 400]); + expect(new Set(answers.map((a) => a.status)).size).toBe(4); + // Codes too: two rows sharing a status would still be two different + // facts, and the SDK branches on `code`. + expect(new Set(answers.map((a) => a.code)).size).toBe(4); + // Exactly ONE row may claim the flow ran. + expect(answers.filter((a) => a.code === 'FLOW_FAILED')).toHaveLength(1); + }); + + it('classifies off the producer\'s verdict, never off `summary` / `durationMs`', async () => { + // A refused dispatch carrying the incidental fields of a failed run is + // still a refused dispatch. If this door sniffed shape instead of + // reading the classification, it would answer 400 here. + const { dispatcher } = makeDispatcher({ + result: { + ...DISABLED, + durationMs: 45, + summary: { + selected: 0, acted: 0, skipped: 0, unmeasured: 0, + nodes: [{ nodeId: 'n', nodeType: 'create_record', status: 'failure', runs: 1, failures: 1 }], + } as AutomationResult['summary'], + }, + }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(409); + expect(res.response.body.error.code).toBe('FLOW_DISABLED'); + }); + + it('a successful run still answers 200 with the single #3962 wrap', async () => { + const { dispatcher } = makeDispatcher({ result: { success: true, output: { converted: true } } }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(200); + expect(res.response.body.success).toBe(true); + expect(res.response.body.data).toEqual({ success: true, output: { converted: true } }); + }); + + it('a service without `getFlow` keeps the probe optional and still answers the result rows', async () => { + // `getFlow?` is optional on `IAutomationService`. One that omits it + // cannot be asked whether a flow exists, so this dispatches rather than + // inventing a 404 — exactly as the trigger door behaves. + const { dispatcher, execute } = makeDispatcher({ omitGetFlow: true, result: DISABLED }); + + const res: any = await viaAction(dispatcher); + + expect(execute).toHaveBeenCalledTimes(1); + expect(res.response.status).toBe(409); + expect(res.response.body.error.code).toBe('FLOW_DISABLED'); + }); + + it('an UNCLASSIFIED refusal still speaks HTTP here — never the #3962 double envelope', async () => { + // The one place the two doors answer differently, and on purpose: the + // trigger door leaves an unclassified `success: false` at 200 (it never + // promotes an exit it was not told about), while this route settled in + // #3962 that failures speak HTTP. So the residual stays `400 + // FLOW_FAILED` — what this exit has always answered — rather than + // regressing to `200 {success:true,data:{success:false}}`. + const { dispatcher } = makeDispatcher({ result: { success: false, error: 'something odd' } }); + + const res: any = await viaAction(dispatcher); + + expect(res.response.status).toBe(400); + expect(res.response.body.error.code).toBe('FLOW_FAILED'); + expect(res.response.body.data).toBeUndefined(); + }); +}); + +describe('#9446 — both doors read ONE table, so they cannot drift', () => { + // The ruling's actual content. Per-door assertions are satisfiable by two + // copies of a rule; two copies drifting apart is the defect being closed, + // and only a comparison can see it. + for (const [label, result, status, code] of [ + ['a disabled flow', DISABLED, 409, 'FLOW_DISABLED'], + ['a flow with no start node', NO_START_NODE, 422, 'FLOW_NO_START_NODE'], + ['a run that ran and failed', RAN_AND_FAILED, 400, 'FLOW_FAILED'], + ] as Array<[string, AutomationResult, number, string]>) { + it(`${label}: /actions and /automation/:name/trigger answer the same status and code`, async () => { + const { dispatcher } = makeDispatcher({ result }); + + const action: any = await viaAction(dispatcher); + const trigger: any = await viaTrigger(dispatcher); + + expect(action.response.status).toBe(status); + expect(trigger.response.status).toBe(status); + expect(action.response.body.error.code).toBe(code); + expect(trigger.response.body.error.code).toBe(code); + }); + } + + it('an unknown flow is 404 at both doors', async () => { + const { dispatcher } = makeDispatcher({ flows: [] }); + + const action: any = await viaAction(dispatcher); + const trigger: any = await viaTrigger(dispatcher); + + expect(action.response.status).toBe(404); + expect(trigger.response.status).toBe(404); + expect(action.response.body.error.code).toBe(trigger.response.body.error.code); + }); +}); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index ec33ece265..669c75469b 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -22,6 +22,13 @@ import { ExecutionStatus } from '@objectstack/spec/automation'; import { ListRunsRequestSchema } from '@objectstack/spec/api'; import { parseEnumParam, parseIntegerParam, parseStringParam } from '../query-param.js'; import { capabilityUnavailable } from './unavailable.js'; +// [#9446] The ONE #9378 status table, now shared with the `/actions` door. +import { + classifyFlowRefusal, + flowIsUnknown, + flowNotFoundMessage, + FLOW_NOT_FOUND_STATUS, +} from '../flow-dispatch-status.js'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; @@ -483,6 +490,16 @@ function flowDefinitionRefusal(err: any): unknown { * | flow has no start node | never dispatched | `422` `FLOW_NO_START_NODE` | * | ran and failed (incl. the retry-strategy exits) | ran, rejected | `400` `FLOW_FAILED` | * + * [#9446] **The table itself now lives in `../flow-dispatch-status.js`** — one + * definition, read by this door and by `/actions` (`action-execution.ts`) — + * because the maintainer ruled (2026-08-18, verbatim 「同意」) that it is a + * property of the flow-dispatch CONTRACT rather than of this route. Everything + * below about WHY each row answers as it does is unchanged and is still the + * reference for it; what moved is the READING, so no door can drift from the + * rule while claiming to implement it. Two things stay here because the table + * deliberately does not answer them: the `errorMessage` / `summary` details on + * the 400 arm, and the 200 an UNCLASSIFIED refusal still gets at this door. + * * **404 is answered by the registry probe, not by reading the result.** It is * the SAME `getFlow` probe `POST /:name/toggle` (#7535) and `GET /:name` use, * so no two doors can disagree about which flows exist — and existence is a @@ -545,37 +562,28 @@ async function respondToFlowTrigger( body: any, context: HttpProtocolContext, ): Promise { - if (typeof automationService.getFlow === 'function') { - const existing = await automationService.getFlow(flowName); - if (!existing) { - return { handled: true, response: deps.error(`Flow '${flowName}' not found`, 404) }; - } - } - const result = await automationService.execute(flowName, buildAutomationContext(body, context)); - if (result?.success === false && result.code === 'FLOW_DISABLED') { + if (await flowIsUnknown(automationService, flowName)) { return { handled: true, - response: deps.error(result.error ?? `Flow '${flowName}' is disabled`, 409, { - code: 'FLOW_DISABLED', - }), + response: deps.error(flowNotFoundMessage(flowName), FLOW_NOT_FOUND_STATUS), }; } - if (result?.success === false && result.code === 'FLOW_NO_START_NODE') { - return { - handled: true, - response: deps.error(result.error ?? `Flow '${flowName}' has no start node`, 422, { - code: 'FLOW_NO_START_NODE', - }), - }; - } - if (result?.success === false && result.status === 'failed') { - return { - handled: true, - response: deps.error(result.error ?? 'Flow run failed', 400, { - code: 'FLOW_FAILED', + const result = await automationService.execute(flowName, buildAutomationContext(body, context)); + const refusal = classifyFlowRefusal(flowName, result); + if (refusal) { + // The run's own artefacts ride the 400 arm ONLY — they describe a run + // that happened. A never-dispatched refusal has no author failure text + // and no node log to point at, so emitting either there would be this + // door inventing run evidence for a run that never started. + const runDetails = refusal.code === 'FLOW_FAILED' + ? { ...(result.errorMessage !== undefined ? { errorMessage: result.errorMessage } : {}), ...(result.summary !== undefined ? { summary: result.summary } : {}), - }), + } + : {}; + return { + handled: true, + response: deps.error(refusal.message, refusal.status, { code: refusal.code, ...runDetails }), }; } return { handled: true, response: deps.success(result) }; diff --git a/packages/runtime/src/flow-dispatch-status.ts b/packages/runtime/src/flow-dispatch-status.ts new file mode 100644 index 0000000000..6a72b3bdba --- /dev/null +++ b/packages/runtime/src/flow-dispatch-status.ts @@ -0,0 +1,158 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The #9378 status table for a flow dispatched through + * `IAutomationService.execute` — ONE definition, read by every door. + * + * | engine exit | reality | answer | + * |------------------------|------------------|----------------------------| + * | flow not found | never dispatched | `404` | + * | flow disabled | never dispatched | `409` `FLOW_DISABLED` | + * | flow has no start node | never dispatched | `422` `FLOW_NO_START_NODE` | + * | ran and failed | ran, rejected | `400` `FLOW_FAILED` | + * + * ## Why the table is a module and not a mapper inside one route + * + * Three doors dispatch a flow through that one service method — the two + * `trigger` routes (`domains/automation.ts`), the `/actions` route plus the + * MCP `run_action` bridge (`action-execution.ts`), and declared endpoints + * (`endpoint-executor.ts`) — and each answered from its own reading of the + * result. That is how one engine exit got three answers: a DISABLED flow was + * `409 FLOW_DISABLED` at the trigger door and `400 FLOW_FAILED` at `/actions` + * — "the flow ran and rejected", a false statement about a dispatch that never + * happened — while the endpoint door served every outcome as `200`. + * + * The maintainer ruled (2026-08-18, verbatim 「同意」 to the triage + * recommendation on #9446) that this table is a property of the flow-dispatch + * CONTRACT rather than of the trigger route, converged in stages. A second + * copy of it is therefore a defect by construction, and each stage is a door + * deleting its own copy in favour of this one — which is also why this file is + * a module in `src/` rather than an export of either door: a rule two doors + * must agree on cannot live inside one of them. + * + * ## What this table does NOT answer, and why each door still owns it + * + * **The envelope.** The trigger door RETURNS a built response and can carry + * `errorMessage` / `summary` in `error.details`; `/actions` THROWS, and a + * throw's structured context is only what `resolveThrownHttpError` + * (`@objectstack/types`) reads off the thrown value. Status and code are the + * contract the #9378 ruling settled; the payload beside them is not. + * + * **What an UNCLASSIFIED `success: false` means.** {@link classifyFlowRefusal} + * returns `undefined` for a refusal the producer did not classify, and the two + * doors answer that differently on purpose: the trigger door leaves it at + * today's `200` (it never PROMOTES an exit it was not told about), while + * `/actions` refuses it, because `200 {success:true,data:{success:false}}` is + * exactly the double envelope #3962 ruled out for that route. Both readings + * are stated at their door. + * + * ## Read the producer's verdict — never sniff (PD #12) + * + * `code` says WHY a dispatch was refused; `status: 'failed'` says how a run + * that started ended. Those two fields are the whole input. `summary`, + * `durationMs` and the message text are never consulted — a refused dispatch + * that happens to carry a failed run's incidental fields is still a refused + * dispatch, and a regex over the engine's prose is the tolerant-consumer shape + * the platform forbids. + * + * ⚠️ **Ordering: the never-dispatched arms come FIRST.** They are exclusive of + * the `status: 'failed'` arm today (a refused dispatch has no lifecycle + * verdict), so the order is not load-bearing for correctness — but it states + * the intended precedence, and it keeps a future producer that stamped both by + * mistake from being reported as a run that failed, which is the wrong of the + * two answers. + */ + +import type { AutomationResult } from '@objectstack/spec/contracts'; + +/** + * The codes this table answers with. All three are ADR-0112 registered under + * `@objectstack/runtime` in `ERROR_CODE_LEDGER` — ⛔ nothing here mints one, + * and a fourth row would be a spec-seat widening, never a call-site decision + * (the #9384 ruling). + */ +export type FlowRefusalCode = 'FLOW_DISABLED' | 'FLOW_NO_START_NODE' | 'FLOW_FAILED'; + +/** One row of the table, resolved against a real result. */ +export interface FlowRefusal { + /** The HTTP status this row answers with. */ + readonly status: 400 | 409 | 422; + /** The ADR-0112 `error.code` this row answers with. */ + readonly code: FlowRefusalCode; + /** + * The producer's own words when it wrote any, else this row's default. + * The engine names the flow in its disabled message and does not in its + * start-node one, so the defaults name it for both — an operator reading a + * refusal needs to know WHICH flow was refused. + */ + readonly message: string; +} + +/** + * The table's first row. Answered by a registry probe rather than by reading a + * result, because the engine's not-found exit carries neither a `code` nor a + * `status` — telling it apart from any other unclassified refusal would take a + * regex over its message, which is the one thing this table refuses to do. + */ +export const FLOW_NOT_FOUND_STATUS = 404; + +/** The 404 row's message. Named, so the caller knows WHICH name failed to resolve. */ +export function flowNotFoundMessage(flowName: string): string { + return `Flow '${flowName}' not found`; +} + +/** + * Whether this automation service can be asked about `flowName` and answers + * that it holds no such flow — the SAME optional `getFlow` probe + * `POST /:name/toggle` (#7535) and `GET /:name` use, so no two doors can + * disagree about which flows exist. + * + * `getFlow` is optional on `IAutomationService`. An implementation that omits + * it cannot be asked, so this answers `false` — "no evidence of absence" — and + * the caller dispatches as before rather than inventing a 404 it has no + * grounds for. + */ +export async function flowIsUnknown(automation: unknown, flowName: string): Promise { + const svc = automation as { getFlow?: (name: string) => Promise } | null | undefined; + if (typeof svc?.getFlow !== 'function') return false; + return !(await svc.getFlow(flowName)); +} + +/** + * The three result-borne rows: which HTTP answer this engine result declares, + * or `undefined` when the producer classified nothing (see the module note on + * why that is deliberately not a row). + * + * `flowName` is used ONLY to fill a row's default message when the producer + * wrote none; it never affects the classification. + */ +export function classifyFlowRefusal( + flowName: string, + result: AutomationResult | null | undefined, +): FlowRefusal | undefined { + if (!result || typeof result !== 'object' || result.success !== false) return undefined; + const message = typeof result.error === 'string' && result.error ? result.error : undefined; + + // ── never dispatched: the producer says WHICH refusal (#9415) ────────── + if (result.code === 'FLOW_DISABLED') { + return { + status: 409, + code: 'FLOW_DISABLED', + message: message ?? `Flow '${flowName}' is disabled`, + }; + } + if (result.code === 'FLOW_NO_START_NODE') { + return { + status: 422, + code: 'FLOW_NO_START_NODE', + message: message ?? `Flow '${flowName}' has no start node`, + }; + } + + // ── dispatched and rejected: the producer's lifecycle verdict (#9378) ── + if (result.status === 'failed') { + return { status: 400, code: 'FLOW_FAILED', message: message ?? 'Flow run failed' }; + } + + return undefined; +}