fix(service-automation): a triggered run carries the flow author's successMessage / errorMessage — execute() and both retry exits (#9414) (#9514)

* fix(service-automation): execute() carries the flow author's terminal messages (#9414)

`AutomationResult` declares `successMessage` / `errorMessage` as a general
terminal-result feature, but `resumeInternal` was the only producer. A flow
dispatched through `POST /api/v1/automation/:name/trigger` carried neither,
so the author's own text reached a caller only when the run happened to
pause and be resumed — and the console consumer that reads
`error.details.errorMessage` (#9413, objectui `flowResponse.ts`) got nothing
on every non-screen flow.

Four terminal exits now produce the pair: `execute()`'s success and failure
returns, both of `executeWithoutRetry`'s, and `retryExecution`'s exhausted
exit — a different exit from `execute()`'s own, reached by exactly the runs
that failed repeatedly.

Paused, skipped and never-dispatched exits keep carrying neither, and that
boundary is pinned rather than only described.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza

* test(verify): pin the trigger route's terminal messages end-to-end at the documented wire path (#9414)

`content/docs/automation/flows.mdx` promises that a 400 from
`POST /api/v1/automation/:name/trigger` carries the flow author's own
`errorMessage` at `error.details.errorMessage`. That sentence was false
before the engine repair — the field was always absent at the source on
this route — so the docs described the declaration while the
implementation never honoured it.

The engine pins assert `AutomationResult`; the route pins drive a scripted
result. Neither proves the documented sentence. `@objectstack/verify` is
the one package depending on both `@objectstack/runtime` and
`@objectstack/service-automation`, so a real engine is driven through a
real `HttpDispatcher` here and the response body is read at the doc's own
path and key — on both exits, plus the "when the flow declares one" half.

One case drives a deliberately hostile string (braces, padding, `&`,
quotes) and asserts byte identity: `flows.mdx` documents both fields as
plain strings with `{var}` explicitly NOT interpolated, so templating,
trimming or escaping on the way out would contradict the shipped contract.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza

---------

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

Author: os-project-manager

.changeset/automation-execute-terminal-messages.md

@@ -0,0 +1,51 @@
+---
+"@objectstack/service-automation": patch
+---
+
+fix(service-automation): a triggered run carries the flow author's `successMessage` / `errorMessage` — `execute()` and both retry exits, symmetric with `resume()` (#9414)
+
+`AutomationResult` declares `successMessage` / `errorMessage` as a general
+terminal-result feature — *"Friendly terminal messages copied from the flow
+definition (`flow.successMessage` / `flow.errorMessage`) … `successMessage` is
+set on terminal success, `errorMessage` on failure."* One producer honoured it.
+`resumeInternal` set both on its terminal returns; `execute()` set neither, on
+either exit, and neither did `executeWithoutRetry` or `retryExecution`.
+
+So a flow's own words reached a caller **only if the run happened to pause and
+be resumed**. A flow dispatched straight through
+`POST /api/v1/automation/:name/trigger` — or the legacy `trigger/:name` that
+`client.automation.trigger()` calls — carried nothing, though the flow declared
+the text and the contract said it was set. One declaration, two behaviours
+decided by *route* rather than by authoring.
+
+**The consumer was already there and already reading.** The trigger route
+carries `errorMessage` into `error.details.errorMessage` (#9413), which is the
+one place the console reads it from (objectui `flowResponse.ts`, #4899) — and on
+the trigger path it was **always absent at the source**, so every non-screen flow
+showed the raw node error instead of the sentence its author wrote.
+
+Four terminal exits now produce the pair, which is every exit a triggered run can
+leave through:
+
+- `execute()` terminal success → `successMessage`; terminal failure →
+  `errorMessage`, **beside** the raw `error` rather than instead of it (the
+  transport folds `error` into the ADR-0112 message and carries the author's text
+  in `details`).
+- `executeWithoutRetry()` — both exits. `retryExecution` returns this result
+  verbatim when a later attempt succeeds, so without it `successMessage` would
+  depend on *which attempt* happened to work.
+- `retryExecution()`'s **exhausted** exit, which is a different exit from
+  `execute()`'s own failure return — a flow under `errorHandling.strategy:
+  'retry'` never reaches that one. A repair stopping at `execute()` would have
+  left the message missing for exactly the runs most likely to need it.
+
+**Nothing else gained a message, deliberately.** The paused return is not
+terminal; the skip exits (`condition_not_met`, `reentrancy_loop_guard`) return
+`success: true` for a run that executed no node; the never-dispatched exits
+(flow not found / disabled / no start node) have no lifecycle verdict at all and
+must not acquire a second channel implying one. Those boundaries are pinned, not
+just described.
+
+No new keys and no contract edit — the pair was declared, documented and
+consumed already; this is the production half catching up (ADR-0049
+enforce-or-remove, restoration direction).

packages/services/service-automation/src/engine.ts

@@ -3239,6 +3239,28 @@ export class AutomationEngine implements IAutomationService {
                 success: true,
                 output,
                 durationMs,
+                // [#9414] The flow author's own completion text, on the exit
+                // every TRIGGERED run leaves through. `AutomationResult`
+                // declares this pair as a general terminal-result feature —
+                // "`successMessage` is set on terminal success, `errorMessage`
+                // on failure" — but `resumeInternal` was the only producer, so
+                // the author's words reached a caller only when the run
+                // happened to pause and be resumed. A flow dispatched straight
+                // through `POST /api/v1/automation/:name/trigger` (or the
+                // legacy `trigger/:name` the SDK calls) carried nothing. Same
+                // declaration, two behaviours decided by ROUTE rather than by
+                // authoring — the shape ADR-0049's enforce-or-remove exists to
+                // stop, and this is the restoration half of it, not a new key.
+                //
+                // ⚠️ TERMINAL exits only, which is the whole boundary. The
+                // paused return below carries neither (the run has not
+                // finished; `resumeInternal` stamps the message when it later
+                // ends). Neither do the SKIP exits above — `condition_not_met`
+                // and `reentrancy_loop_guard` return `success: true` for a run
+                // that executed no node, so stamping "Opportunity created!" on
+                // one would be a toast about work nobody did. They carry no
+                // `summary` for exactly the same reason.
+                successMessage: flow.successMessage,
                 // #4354 — hand the counts back synchronously so a caller
                 // (a `subflow` roll-up, a runtime test asserting the sweep wrote
                 // something) never has to re-read the run to learn what it did.
@@ -3309,7 +3331,7 @@ export class AutomationEngine implements IAutomationService {
 
             // Error handling strategy
             if (flow.errorHandling?.strategy === 'retry') {
-                return this.retryExecution(flowName, context, startTime, flow.errorHandling);
+                return this.retryExecution(flowName, context, startTime, flow.errorHandling, flow.errorMessage);
             }
             return {
                 success: false,
@@ -3350,6 +3372,16 @@ export class AutomationEngine implements IAutomationService {
                 // documented as the run's lifecycle verdict, and already the
                 // value recorded in the log.
                 status: 'failed',
+                // [#9414] The author's failure text, BESIDE the raw `error`
+                // rather than instead of it. The transport folds `error` into
+                // the ADR-0112 message and carries this one in
+                // `error.details.errorMessage` — the single place the console
+                // reads it from (objectui `flowResponse.ts`, PR #4899). The
+                // envelope has no `data`, so a producer that leaves this absent
+                // does not degrade to a plainer toast: the consumer falls back
+                // to the raw node error text, which is what every non-screen
+                // flow showed until now.
+                errorMessage: flow.errorMessage,
                 // A failed run's counts matter MORE, not less: they say how far
                 // it got before dying — how many rows it had already written.
                 summary: logged.summary,
@@ -6117,12 +6149,20 @@ export class AutomationEngine implements IAutomationService {
      * the parameter as the parsed shape is what keeps a second set of defaults
      * from growing back here: a knob the spec stops defaulting becomes a
      * compile error, not a silent engine-side guess.
+     *
+     * `flowErrorMessage` is the author's terminal failure text
+     * (`flow.errorMessage`), passed for the same reason `errorHandling` is
+     * passed rather than re-read: `execute()` already holds the parsed flow,
+     * and the exhausted exit below must report the definition THIS dispatch
+     * started under — not whatever a hot-reload re-registered under the same
+     * name while the loop slept between attempts (#9414).
      */
     private async retryExecution(
         flowName: string,
         context: AutomationContext | undefined,
         startTime: number,
         errorHandling: NonNullable<FlowParsed['errorHandling']>,
+        flowErrorMessage: string | undefined,
     ): Promise<AutomationResult> {
         // `maxRetries >= 1` is guaranteed under `strategy: 'retry'` — the schema
         // refuses the zero-attempt spelling of "retry" (#4247), so reaching this
@@ -6159,7 +6199,20 @@ export class AutomationEngine implements IAutomationService {
         // same lifecycle verdict, and a transport answers it the same way.
         // Without it, a flow whose author chose `errorHandling.strategy:
         // 'retry'` would be the ONE failure shape that still rode HTTP 200.
-        return { success: false, error: lastError, durationMs: Date.now() - startTime, status: 'failed' };
+        //
+        // [#9414] `errorMessage` for the same structural reason: this is a
+        // DIFFERENT terminal exit from `execute()`'s own failure return — a
+        // flow under `strategy: 'retry'` is handed off above and never reaches
+        // that one — so a repair that stopped at `execute()` would leave the
+        // author's message missing for precisely the runs most likely to need
+        // it, the ones that failed over and over.
+        return {
+            success: false,
+            error: lastError,
+            durationMs: Date.now() - startTime,
+            status: 'failed',
+            errorMessage: flowErrorMessage,
+        };
     }
 
     /**
@@ -6290,7 +6343,13 @@ export class AutomationEngine implements IAutomationService {
 
             // #4354 — a retried run reports its own attempt's counts, not the
             // failed one's: `retryExecution` returns THIS result on success.
-            return { success: true, output, durationMs, summary: logged.summary };
+            //
+            // [#9414] …and because `retryExecution` returns THIS result, this
+            // is the exit a run that succeeded on attempt 2+ leaves through.
+            // The author's completion text has to be produced here as well, or
+            // `successMessage` would be a function of which attempt happened to
+            // work — the same route-dependent shape the fix is removing.
+            return { success: true, output, durationMs, successMessage: flow.successMessage, summary: logged.summary };
         } catch (err: unknown) {
             const errorMessage = err instanceof Error ? err.message : String(err);
             const durationMs = Date.now() - startTime;
@@ -6311,7 +6370,19 @@ export class AutomationEngine implements IAutomationService {
             // same ran-and-failed exit as the two above and is classified the
             // same: a selective classification is the one a later reader
             // mistakes for a rule.
-            return { success: false, error: errorMessage, durationMs, status: 'failed', summary: logged.summary };
+            //
+            // [#9414] `errorMessage: flow.errorMessage` rides along on the same
+            // argument, and note the two names are NOT the same thing here:
+            // the local `errorMessage` is the raw thrown text, `flow.errorMessage`
+            // is what the author wrote for a human to read.
+            return {
+                success: false,
+                error: errorMessage,
+                durationMs,
+                status: 'failed',
+                errorMessage: flow.errorMessage,
+                summary: logged.summary,
+            };
         }
     }
 }