fix(automation): answer real HTTP status codes on both trigger routes (#9413)

* 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.

* test(dogfood): migrate the flow-runAs write leg to the ruled 400 FLOW_FAILED

The `runAs:'user'` WRITE leg triggers a flow whose `update_record` node is
refused at the record layer, so the run fails — and since the trigger route
answers real HTTP status codes it answers 400 FLOW_FAILED instead of 200
wrapping an inner {success:false}. `memberTrigger`'s blanket `< 300` was
written against the old contract and is the only dogfood consumer this
change touches.

The semantic the file pins is unchanged and still primary: the run executes
AS the member, so the admin's note stays 'new'. What moves is the transport
expectation, and it is asserted precisely rather than as a widened band —
status 400, error.code FLOW_FAILED, the node-first access-refusal text, and
the `touch` node's failure entry in error.details.summary. A 403 or 500 here
would mean de-elevation broke differently and must not pass.

The READ leg is deliberately NOT migrated: an RLS-scoped read is FILTERED,
not refused, so that run still succeeds with an empty `found` and keeps its
`< 300` expectation. Measured locally, not assumed. That leg also gains
discrimination for free: until now a failed run and an empty read were
indistinguishable there, because both left `found` falsy under HTTP 200.

Verified locally on a built workspace closure:
  vitest run test/flow-runas.dogfood.test.ts            → 5 passed (was 1 failed)
  flow-node · flow-function-effect · flow-durable-suspend ·
  showcase-declarative-mcp                              → 17 passed
  showcase-anonymous-deny-surfaces · authz-conformance  → 46 passed
  pnpm --filter @objectstack/dogfood typecheck          → green

Every other dogfood trigger caller expects the run to SUCCEED (or asserts an
anonymous 401), so none is touched by this contract. No changeset change:
@objectstack/dogfood is private internal QA and the wire change is already
documented in .changeset/automation-trigger-status-unification.md.

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/automation-trigger-status-unification.md

@@ -0,0 +1,71 @@
+---
+'@objectstack/service-automation': minor
+'@objectstack/runtime': minor
+'@objectstack/client': minor
+---
+
+**BREAKING** — both automation `trigger` routes answer real HTTP status codes for a flow
+that ran and failed, instead of HTTP 200 wrapping an inner `{success: false}`.
+
+This is the second and wider half of the migration the resume route shipped in the same
+release (#8684, merged 2026-08-17): one SDK-visible behaviour change, one migration note.
+The resume flip touched the screen-flow runner; this one touches the door every app
+dispatches flows through.
+
+Until now a flow driven to a node failure answered:
+
+```
+HTTP 200
+{"success":true,"data":{"success":false,"error":"Node 'create_opportunity' failed: …"}}
+```
+
+The run genuinely failed; the transport reported success. A scripted or integration caller
+that branches on the HTTP status alone read a failed run as a successful one. This applies
+the `/actions` ruling (business failures must not ride HTTP 200 inside a double envelope)
+to `POST /api/v1/automation/:name/trigger` and to the legacy
+`POST /api/v1/automation/trigger/:name` — the shape `client.automation.trigger()` calls.
+Both doors answer through one mapper, so they cannot drift.
+
+What changes on the wire:
+
+- **A flow that ran and then failed ⇒ `400` with `error.code: 'FLOW_FAILED'`.** The node
+  failure stays the human-readable `error.message`. The flow author's own `errorMessage`
+  travels in `error.details.errorMessage` — one documented location, the same one the
+  console reads — and the run's per-node accounting in `error.details.summary`. A flow
+  whose `errorHandling.strategy` is `retry` answers the same way once its attempts are
+  exhausted. `durationMs` is no longer carried on this response.
+- **A flow name the deployment does not hold ⇒ `404`,** answered before anything is
+  dispatched, through the same registry probe `POST /:name/toggle` and `GET /:name` use.
+- **Unchanged:** a successful run still answers 200 with its result, and a run that PAUSED
+  at a `screen` node still answers 200 with the next screen — a pause is not a failure.
+- **Also unchanged, pending a ruling:** a DISABLED flow and one with no start node still
+  answer 200 with the inner failure. Both are exits that never dispatched anything, and
+  telling them apart needs a producer-side classification the closed
+  `AutomationResult.code` union cannot yet express. Tracked on #9378.
+
+**`@objectstack/service-automation`:** `execute()` now stamps `status: 'failed'` on the
+results of runs that dispatched and were rejected — the same lifecycle verdict it already
+writes to the run log. Its never-dispatched exits carry no `status`, which is what lets a
+transport answer the two classes differently without inspecting the result's internals.
+
+**`@objectstack/client`:** `client.automation.trigger()`, `client.automation.execute()` and
+`client.project(id).automation.execute()` now **reject** on a failed run instead of
+resolving with `{ success: false, error }` — the SDK throws on every non-2xx before
+unwrapping. Callers that inspected the resolved value must move to a `catch`:
+
+```ts
+try {
+  await client.automation.execute(flow, { params });
+} catch (err: any) {
+  err.code;                   // 'FLOW_FAILED' (400) — the run ran and failed
+  err.httpStatus;             // 400 | 404
+  err.message;                // the node failure, verbatim
+  err.details?.errorMessage;  // the flow author's own message, when the flow declares one
+  err.details?.summary;       // which node failed
+}
+```
+
+Raw-HTTP callers that treated `2xx` as success and never opened the inner envelope now see
+the failure they were already being told about, one level up.
+
+<!-- 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

@@ -1354,6 +1354,90 @@ describe('ObjectStackClient.automation', () => {
         expect(err.message).toMatch(/no longer exists in flow/);
     });
 
+    // [#9378] BREAKING, and the wide half of the same flip: DISPATCHING a flow.
+    // `client.automation.trigger()` (legacy route) and `.execute()` used to
+    // resolve with `{ success: false, error }` under HTTP 200 for a run that
+    // ran and failed — every app dispatches flows through this door, so a
+    // caller that never opened the inner envelope read every failed run as a
+    // successful one. The route answers 400 `FLOW_FAILED` now and this SDK's
+    // fetch layer throws on non-2xx before any unwrapping, so both surfaces
+    // REJECT. No SDK code changed; the contract did, and these are its pins.
+    //
+    // Both spellings are pinned, not one: `trigger()` reads `res.json()` while
+    // `execute()` reads `unwrapResponse()`, so a regression in either unwrap
+    // path would be invisible from the other's test.
+    const failedRunBody = {
+        success: false,
+        error: {
+            code: 'FLOW_FAILED',
+            message: "Node 'create_opportunity' failed: Amount must be greater than zero",
+            httpStatus: 400,
+            details: {
+                errorMessage: 'We could not create the opportunity — check the amount and try again.',
+                summary: { nodes: [{ nodeId: 'create_opportunity', status: 'failure' }] },
+            },
+        },
+    };
+
+    it('should reject with FLOW_FAILED when a triggered flow runs and fails (legacy trigger)', async () => {
+        const { client } = createMockClient(failedRunBody, 400);
+
+        const err: any = await client.automation
+            .trigger('my_flow', { amount: 0 })
+            .then(() => { throw new Error('expected the failed trigger to reject'); }, (e) => e);
+
+        // The classification, not merely that it threw: an SDK rejecting with a
+        // bare `Error` would satisfy `.rejects.toThrow()` while losing
+        // everything a caller branches on.
+        expect(err.code).toBe('FLOW_FAILED');
+        expect(err.httpStatus).toBe(400);
+        expect(err.message).toMatch(/Node 'create_opportunity' failed/);
+        // The flow author's own text keeps its one documented location — the
+        // ADR-0112 envelope has no `data`, and the console reads it from here.
+        expect(err.details?.errorMessage)
+            .toBe('We could not create the opportunity — check the amount and try again.');
+    });
+
+    it('should reject with FLOW_FAILED when execute() dispatches a flow that fails', async () => {
+        const { client } = createMockClient(failedRunBody, 400);
+
+        const err: any = await client.automation
+            .execute('my_flow', { params: { amount: 0 } })
+            .then(() => { throw new Error('expected the failed execute to reject'); }, (e) => e);
+
+        expect(err.code).toBe('FLOW_FAILED');
+        expect(err.httpStatus).toBe(400);
+        expect(err.details?.summary?.nodes?.[0]?.status).toBe('failure');
+    });
+
+    it('should reject with 404 when the triggered flow does not exist', async () => {
+        const { client } = createMockClient({
+            success: false,
+            error: { code: 'RESOURCE_NOT_FOUND', message: "Flow 'no_such_flow' not found", httpStatus: 404 },
+        }, 404);
+
+        const err: any = await client.automation
+            .execute('no_such_flow', {})
+            .then(() => { throw new Error('expected the unknown flow to reject'); }, (e) => e);
+
+        expect(err.httpStatus).toBe(404);
+        expect(err.code).not.toBe('FLOW_FAILED');
+        expect(err.message).toContain('no_such_flow');
+    });
+
+    it('should still resolve when a triggered flow succeeds', async () => {
+        // The other half of the contract: a successful dispatch is untouched,
+        // including the paused screen-flow shape the runner drives.
+        const { client } = createMockClient({
+            success: true,
+            data: { success: true, status: 'paused', runId: 'run_1', screen: { nodeId: 'collect', fields: [] } },
+        }, 200);
+
+        const result: any = await client.automation.execute('my_flow', {});
+        expect(result.status).toBe('paused');
+        expect(result.runId).toBe('run_1');
+    });
+
     it('should fetch the screen a paused run awaits', async () => {
         const { client, fetchMock } = createMockClient({
             success: true,

packages/client/src/index.ts

@@ -3016,6 +3016,28 @@ export class ObjectStackClient {
   automation = {
       /**
        * Trigger a named automation flow (legacy endpoint)
+       *
+       * **BREAKING since #9378 — a failed run REJECTS instead of resolving.**
+       * A flow that ran and then failed used to come back as a resolved
+       * `{ success: false, error }` riding HTTP 200, so a caller that did not
+       * open the inner envelope read a failed run as a successful one. The
+       * route answers **400** `FLOW_FAILED` now (inheriting #3962's ruling for
+       * `/actions`, applied to the resume route by #8684), and every non-2xx
+       * throws out of this SDK's fetch layer — so this promise **rejects**:
+       *
+       * ```ts
+       * try { await client.automation.trigger(flow, payload); }
+       * catch (err: any) {
+       *   err.code;                    // 'FLOW_FAILED'
+       *   err.httpStatus;              // 400
+       *   err.message;                 // the node failure, verbatim
+       *   err.details?.errorMessage;   // the flow author's `errorMessage`
+       *   err.details?.summary;        // per-node accounting of the failed run
+       * }
+       * ```
+       *
+       * A flow name the deployment does not hold rejects with **404** instead.
+       * A run that PAUSED at a screen node is not a failure and still resolves.
        */
       trigger: async (triggerName: string, payload: any) => {
           const route = this.getRoute('automation');
@@ -3168,7 +3190,15 @@ export class ObjectStackClient {
           const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(name)}`);
           return this.unwrapResponse(res) as Promise<T>;
       },
-      /** Execute (trigger) a flow with an execution context. */
+      /**
+       * Execute (trigger) a flow with an execution context.
+       *
+       * **BREAKING since #9378**: a run that ran and failed now REJECTS with
+       * `400` `FLOW_FAILED` (author text on `err.details.errorMessage`, per-node
+       * accounting on `err.details.summary`) instead of resolving with an inner
+       * `{ success: false }` under HTTP 200; an unknown flow rejects with `404`.
+       * See `automation.trigger` for the full shape — both call the same door.
+       */
       execute: async <T = any>(name: string, ctx?: Record<string, any>): Promise<T> => {
           const route = this.getRoute('automation');
           const res = await this.fetch(`${this.baseUrl}${route}/${encodeURIComponent(name)}/trigger`, {
@@ -5342,6 +5372,12 @@ export class ScopedProjectClient {
     /**
      * Execute (trigger) a flow by name. The request body is forwarded as the
      * automation execution context (e.g. `{ params, trigger }`).
+     *
+     * Mirrors the unscoped `client.automation.execute` — including its
+     * **breaking #9378 behaviour**: a run that ran and then failed REJECTS with
+     * `400` `FLOW_FAILED` (author text on `err.details.errorMessage`) instead of
+     * resolving with an inner `{ success: false }` under HTTP 200, and an
+     * unknown flow rejects with `404`. See that method for the full shape.
      */
     execute: async <T = any>(name: string, ctx?: Record<string, any>): Promise<T> => {
       const res = await this.parent._fetch(this.url(`/automation/${encodeURIComponent(name)}/trigger`), {

packages/qa/dogfood/test/flow-runas.dogfood.test.ts

@@ -19,6 +19,13 @@
 // object the member cannot read or write directly:
 //   • system flows succeed on the admin's note  → elevation is REAL,
 //   • user flows are RLS-denied on the same note → de-elevation is REAL.
+// The two user-mode legs are denied DIFFERENTLY, and both shapes are asserted:
+// the WRITE is refused at the record layer, so the run fails and the trigger
+// route answers 400 `FLOW_FAILED` (#9378 — it rode HTTP 200 with an inner
+// `{success:false}` until then); the READ is filtered by RLS, so the run
+// SUCCEEDS with an empty `found`. Neither is a status band: a write leg that
+// started answering 403/500, or a read leg that started failing outright, is a
+// different bug and must not pass here.
 // Before the #1888 fix the user flows wrongly succeed (CRUD nodes passed no
 // identity → security skipped) → this file is RED; after the fix → GREEN.
 
@@ -61,7 +68,18 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
     return (j.record ?? j).status;
   }
 
-  /** Trigger a flow as the restricted member; returns the inner AutomationResult. */
+  /**
+   * Trigger a flow as the restricted member and require the run to have
+   * COMPLETED; returns the inner AutomationResult.
+   *
+   * [#9378] Since the trigger route answers real HTTP status codes, this helper
+   * is also the discriminator it could not be before: a run that fails now
+   * comes back 400 and is rejected HERE. Until then a failed run rode HTTP 200
+   * with `{success:false}` inside, so the read leg below — which only checks
+   * that `found` is falsy — passed identically whether the RLS-scoped read
+   * returned EMPTY (the thing it means to prove) or the run DIED before
+   * reading anything at all.
+   */
   async function memberTrigger(flow: string, noteId: string): Promise<{ success?: boolean; output?: any }> {
     const res = await stack.apiAs(memberToken, 'POST', `/automation/${flow}/trigger`, { params: { noteId } });
     expect(res.status, `trigger ${flow} HTTP failed: ${res.status} ${await res.clone().text()}`).toBeLessThan(300);
@@ -70,6 +88,50 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
     return body.data ?? {};
   }
 
+  /**
+   * Trigger a flow as the restricted member and require the run to have RUN AND
+   * FAILED on a record-access refusal — the de-elevation proof's write leg.
+   *
+   * [#9378] The transport contract this asserts, in full rather than by status
+   * band: the route answers **400** `FLOW_FAILED` (ADR-0112 envelope), the node
+   * failure is `error.message` verbatim, and the per-node accounting in
+   * `error.details.summary` names WHICH node failed. Asserting the band
+   * (`>= 400`) or the throw alone would keep passing if the refusal turned into
+   * a 403 authorization verdict or a 500 fault — both of which would mean the
+   * de-elevation broke in a different way, and both of which this file exists
+   * to catch.
+   *
+   * Before #9378 this same run answered `HTTP 200 {"success":true,"data":{
+   * "success":false,…}}`, so the ONLY visible trace of the denial was the
+   * record staying unchanged. That assertion is still below and still the
+   * primary proof; this one is the transport half that used to be invisible.
+   */
+  async function memberTriggerExpectingAccessRefusal(flow: string, noteId: string, failingNodeId: string) {
+    const res = await stack.apiAs(memberToken, 'POST', `/automation/${flow}/trigger`, { params: { noteId } });
+    const text = await res.clone().text();
+    expect(res.status, `trigger ${flow} should be 400 FLOW_FAILED: ${res.status} ${text}`).toBe(400);
+
+    const body = (await res.json()) as {
+      success?: boolean;
+      data?: unknown;
+      error?: { code?: string; message?: string; httpStatus?: number; details?: { summary?: { nodes?: any[] } } };
+    };
+    expect(body.success).toBe(false);
+    expect(body.error?.code, `expected FLOW_FAILED: ${text}`).toBe('FLOW_FAILED');
+    expect(body.error?.httpStatus).toBe(400);
+    // The double envelope is GONE, not re-labelled: nothing is left for a
+    // status-blind caller to misread as a successful run.
+    expect(body.data).toBeUndefined();
+    // The failure is the RLS refusal on the note, named node-first — not a
+    // generic "flow failed", which would pass while the run died of anything.
+    expect(body.error?.message).toContain(`Node '${failingNodeId}' failed`);
+    expect(body.error?.message).toMatch(/do not have access to this record/i);
+    // Which node failed survives the envelope change — the reason `summary`
+    // rides in `details` at all.
+    const failed = body.error?.details?.summary?.nodes?.find((n) => n?.nodeId === failingNodeId);
+    expect(failed?.status, `no failure entry for node '${failingNodeId}' in ${text}`).toBe('failure');
+  }
+
   it('precondition: the automation service is wired and a flow is registered', async () => {
     const res = await stack.apiAs(memberToken, 'GET', '/automation/runas_system_touch');
     expect(res.status, `automation service not wired: ${res.status}`).toBe(200);
@@ -102,7 +164,13 @@ describe('objectstack verify FLOW: runAs identity enforcement (#flow-runas)', ()
 
   it("runAs:'user' DE-ELEVATES — member-triggered user flow is RLS-DENIED on the same record", async () => {
     const id = await adminCreateNote('user-touch');
-    await memberTrigger('runas_user_touch', id);
+    // The de-elevated run reaches the record layer as the MEMBER and the write
+    // is refused there, so the run fails on its `touch` node — surfaced since
+    // #9378 as 400 `FLOW_FAILED` instead of a 200 wrapping the inner failure.
+    // The refusal text is asserted in the helper: this leg proves the identity
+    // switch is real, so "denied because of WHO ran it" is the load-bearing
+    // part, not merely "something went wrong".
+    await memberTriggerExpectingAccessRefusal('runas_user_touch', id, 'touch');
     // The run executed as the member; the by-id write to the admin's note is
     // RLS-denied, so the record is unchanged. (Before the fix it would read
     // 'touched-user' — the privilege-boundary surprise this gate pins.)

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

@@ -620,7 +620,12 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
      */
     it('both trigger routes translate the body and forward the caller identity', async () => {
         const execute = vi.fn().mockResolvedValue({ success: true });
-        const automation = { execute, listFlows: vi.fn(), getFlow: vi.fn() };
+        // [#9378] `getFlow` resolves the flow rather than `undefined`: both
+        // trigger doors now answer 404 through the same shared existence probe
+        // `POST /:name/toggle` and `GET /:name` use, so a fixture whose
+        // registry claims the flow does not exist never reaches `execute` —
+        // which is this test's subject.
+        const automation = { execute, listFlows: vi.fn(), getFlow: vi.fn().mockResolvedValue({ name: 'nurture' }) };
         const ctx: any = {
             executionContext: {
                 userId: 'u-1',
@@ -668,7 +673,9 @@ describe('HttpDispatcher extracted domains (PR-6: automation)', () => {
     it('never calls a non-contract `trigger` method, even when one exists', async () => {
         const trigger = vi.fn().mockResolvedValue({ success: true });
         const execute = vi.fn().mockResolvedValue({ success: true });
-        const automation = { trigger, execute, listFlows: vi.fn(), getFlow: vi.fn() };
+        // [#9378] See the note on the fixture above: the trigger door consults
+        // the shared existence probe before dispatching.
+        const automation = { trigger, execute, listFlows: vi.fn(), getFlow: vi.fn().mockResolvedValue({ name: 'nurture' }) };
 
         const result = await makeDispatcher({ automation, auth })
             .dispatch('POST', '/automation/trigger/nurture', {}, {}, {} as any);