fix(service-automation): on the synchronous path, a child run that refuses stops its parent, in subflow and in map alike

.changeset/18110-subflow-map-refused-rollup.md

@@ -0,0 +1,19 @@
+---
+"@objectstack/service-automation": minor
+---
+
+fix(service-automation): on the synchronous path, a child run that REFUSES stops its parent, in `subflow` and in `map` alike (#18110, #18555)
+
+**Clause-②: yes (widening)** — `NodeExecutionResult` is barrel-exported from this package's single entry point, and it gains two new optional members. Nothing previously accepted is refused and nothing is retired, so this is a widening of the published executor contract, not a narrowing. Contract-review tier.
+
+A child flow that runs to completion in one go and ends on an `end` node declaring `outcome: 'refused'` used to roll up to its parent as an ordinary success. `subflow-node.ts` branched only on `child.status === 'paused'` and `!child.success`; a refused child is neither (`{ success: true, status: 'refused' }` — *a refusal is a successful evaluation that says no*), so it fell through the success exit. The parent walked the node's out-edges, recorded `completed` and fired its **own** `successMessage` over the child's refusal — the author got the exact opposite of what they wrote, fail-open. `map-node.ts` had the identical branch set and the identical hole: a refusing row let every row after it through.
+
+- **New on `NodeExecutionResult`: `refuse?: boolean` and `refusalMessage?: string`.** The executor-facing half of the unwinding protocol `suspend?: boolean` already uses. A node that sets `refuse` terminates its run as `refused` — a terminal status this package has published since #15788, so **no new status value** and nothing authorable changes.
+- **`subflow` and `map` both set it** when their child run returns `status: 'refused'`. One channel, two call sites.
+- **The child's `selected` / `acted` / `unmeasuredEffect` rollup (#4354) survives the refusal**, because the engine throws the refusal signal from the same position it throws the suspend signal: after the node's success step is pushed, after its `childSteps` are folded and after its output is written back. A child that refused really can have written rows before it said no.
+- ⛔ **A refusal is still not a failure.** It does not consume retry budget, is not routable by a `fault` edge, and is not counted in `nodes[].failures`.
+- **Region-boundary diagnostic, text only**: the message a structured region raises when a refusal tries to cross it now names whichever node carried the refusal, instead of asserting it was an `end` node — which, for a refusing `subflow`/`map` inside a region, sent the author looking for a node that was not in their region. Region **semantics** are unchanged.
+
+**Scope — the RESUMED leg is not covered.** This fixes the path where the child run finishes inside the parent's own `engine.execute` call and its outcome is read from that return value. A child that durably PAUSES first — a nested `approval` / `screen` / `wait` — and only refuses when it is later resumed still reaches its parent through the resume machinery, which reads the child's outcome at different seams and does not consult `status: 'refused'` at any of them. Both of those seams pre-date this change and neither is a regression of it, but neither is closed by it either, and the resumed leg is the one a screen flow actually takes. A follow-up card covers it: #18714.
+
+For third-party node executors this is additive: an executor that never sets `refuse` behaves exactly as before.

packages/services/service-automation/src/builtin/map-node.ts

@@ -226,6 +226,49 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v
             },
           };
         }
+        if (child.status === 'refused') {
+          // [#18555] This item's child REFUSED — an `end` inside it declared
+          // `outcome: 'refused'`, a successful evaluation that says NO. The
+          // identical hole #18110 describes for `subflow`, in this second file:
+          // until this arm existed a refusing item fell through the synchronous
+          // completion below, its output was pushed into `state.results`, the
+          // batch carried on to the NEXT item, and the parent recorded
+          // `completed` and fired its own `successMessage`. For a `map` that is
+          // the worked "approve each row" shape answering *no* on row 3 and
+          // approving rows 4..n anyway — fail-open, and finishing green.
+          //
+          // ⛔ Not the failure arm above: a refusal is not a failure (⛔ no
+          // retry budget, ⛔ no `fault` routing, ⛔ not counted in
+          // `nodes[].failures`), and ⛔ not `state.results`: this item did not
+          // produce a result, it declined. The engine throws the refusal signal
+          // only after this step and these `metrics` are already in the run
+          // log, so the batch's #4354 totals survive the refusal.
+          //
+          // The totals are the COMPLETION rule, ⛔ not the failure one: the
+          // items that already ran keep theirs, and the refusing item's own
+          // `selected` / `acted` / `unmeasured` / `failed` roll up too, because
+          // a child that refused did not fail — nothing counts its `failed`
+          // a second time through `nodes[].failures` the way a failed child's
+          // would be. A refusing child really can have written rows before it
+          // said no.
+          //
+          // ⛔ The progress state is not advanced and not deleted: the run is
+          // terminating, nothing resumes it, and `started` is the resume
+          // program counter — moving it would claim this item completed.
+          return {
+            success: true,
+            refuse: true,
+            refusalMessage: child.refusalMessage,
+            metrics: {
+              selected: selected + (child.summary?.selected ?? 0),
+              acted: acted + (child.summary?.acted ?? 0),
+              ...(unmeasured || child.summary?.unmeasured ? { unmeasuredEffect: true } : {}),
+              ...(child.summary?.failed !== undefined
+                ? { failures: failures + child.summary.failed }
+                : rolledFailures()),
+            },
+          };
+        }
         // Synchronous completion — record and advance.
         state.started = idx + 1;
         state.results.push(child.output ?? null);

packages/services/service-automation/src/builtin/map-refused-rollup.test.ts

@@ -0,0 +1,228 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #18555 — a refusing child inside a `map` unit must STOP the parent.
+ *
+ * The identical hole #18110 describes for `subflow`, in a second file:
+ * `map-node.ts` branched on `child.status === 'paused'` and `!child.success`
+ * and had no `refused` arm, so a refused child (`{ success: true, status:
+ * 'refused' }`) fell through the synchronous-completion path — its output was
+ * pushed into `state.results`, the batch carried on to the NEXT item, and the
+ * parent recorded `completed` and fired its own `successMessage`.
+ *
+ * ⭐ For a `map` that is the worked "approve each row" shape answering *no* on
+ * one row and approving every row after it anyway. Fail-open, and finishing
+ * green.
+ *
+ * ⚠️ Pinned SEPARATELY from the `subflow` arm on purpose: one channel
+ * (`NodeExecutionResult.refuse`), two call sites, and deleting either site must
+ * fail a test by itself. `subflow-refused-rollup.test.ts` is the other half —
+ * neither file's green covers the other's arm.
+ *
+ * ⚠️ Direction, predicted before running. "The defect" tests FAIL against the
+ * unfixed executor; the CONTROL is green on both sides on purpose.
+ *
+ * ⚠️ `refused` here is the run OUTCOME — *a refusal is a successful evaluation
+ * that says no* — ⛔ NOT this package's GUARD refusal, which is a FAILURE.
+ */
+
+import { describe, it, expect, beforeEach } from 'vitest';
+import { AutomationEngine } from '../engine.js';
+import type { NodeExecutor } from '../engine.js';
+import { registerMapNode } from './map-node.js';
+
+function silentLogger(): any {
+  return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } };
+}
+function pluginCtx(): any {
+  return { logger: silentLogger(), getService() { return undefined; } };
+}
+
+/** The parent's own completion toast — it must never ride an item's refusal. */
+const PARENT_TOAST = 'Batch complete!';
+/**
+ * The authored refusal template. `{val}` is the child's own declared INPUT
+ * variable (the mapped item, handed down as `params.val`) — what makes the
+ * rendered reason per-item. ⛔ Not the parent's `{item}` iterator: that lives in
+ * the PARENT's variable map and resolves to the empty string down here.
+ */
+const REFUSAL_TEMPLATE = 'Refused: {val} is not eligible';
+/** Per-item #4354 work, so the rollup on the refusal path is assertable. */
+const ITEM_METRICS = { selected: 2, acted: 1 } as const;
+
+/**
+ * Child flow: judge the item, then leave by one of TWO ends — a plain
+ * completion, or an `end` declaring `outcome: 'refused'`. Routed by the
+ * judge's `branchLabel`, so WHICH item refuses is a property of the data.
+ */
+function childFlow() {
+  return {
+    name: 'per_item', label: 'Per item', type: 'autolaunched',
+    variables: [{ name: 'val', type: 'text', isInput: true }],
+    nodes: [
+      { id: 'c_start', type: 'start', label: 'Start' },
+      { id: 'c_judge', type: 'judge', label: 'Judge' },
+      { id: 'c_ok', type: 'end', label: 'Ok' },
+      { id: 'c_no', type: 'end', label: 'No', config: { outcome: 'refused', message: REFUSAL_TEMPLATE } },
+    ],
+    edges: [
+      { id: 'ce0', source: 'c_start', target: 'c_judge' },
+      { id: 'ce1', source: 'c_judge', target: 'c_ok', label: 'allow' },
+      { id: 'ce2', source: 'c_judge', target: 'c_no', label: 'deny' },
+    ],
+  };
+}
+
+/** start -> map(per_item over {items}) -> recorder -> end, with the parent toast. */
+function parentFlow() {
+  return {
+    name: 'batch', label: 'batch', type: 'autolaunched',
+    successMessage: PARENT_TOAST,
+    variables: [{ name: 'items', type: 'list', isInput: true }],
+    nodes: [
+      { id: 'start', type: 'start', label: 'Start' },
+      {
+        id: 'each', type: 'map', label: 'For each',
+        config: {
+          flowName: 'per_item', collection: '{items}',
+          iteratorVariable: 'item', input: { val: '{item}' }, outputVariable: 'mapped',
+        },
+      },
+      { id: 'after', type: 'downstream', label: 'After' },
+      { id: 'fin', type: 'end', label: 'Fin' },
+    ],
+    edges: [
+      { id: 'e0', source: 'start', target: 'each' },
+      { id: 'e1', source: 'each', target: 'after' },
+      { id: 'e2', source: 'after', target: 'fin' },
+    ],
+  };
+}
+
+describe('#18555 — a refusing child inside a `map` stops the parent', () => {
+  let engine: AutomationEngine;
+  let judged: string[];
+  let ran: string[];
+  /** The item value the judge refuses. `null` = refuse nothing (the control). */
+  let deny: string | null;
+
+  beforeEach(() => {
+    engine = new AutomationEngine(silentLogger());
+    judged = [];
+    ran = [];
+    deny = 'b';
+    registerMapNode(engine, pluginCtx());
+
+    engine.registerNodeExecutor({
+      type: 'judge',
+      async execute(_node, _variables, context) {
+        const val = String((context as any)?.params?.val);
+        judged.push(val);
+        return {
+          success: true,
+          branchLabel: deny !== null && val === deny ? 'deny' : 'allow',
+          metrics: { ...ITEM_METRICS },
+        };
+      },
+    } as NodeExecutor);
+    // The node AFTER the map. Its presence in `ran` IS the "the parent walked
+    // on" assertion — ⛔ not a proxy for it.
+    engine.registerNodeExecutor({
+      type: 'downstream',
+      async execute() {
+        ran.push('downstream');
+        return { success: true };
+      },
+    } as NodeExecutor);
+
+    engine.registerFlow('per_item', childFlow() as never);
+    engine.registerFlow('batch', parentFlow() as never);
+  });
+
+  const runBatch = () => engine.execute('batch', { params: { items: ['a', 'b', 'c'] } } as never);
+
+  async function newestRun(flowName: string) {
+    const runs = await engine.listRuns(flowName, { limit: 5 });
+    expect(runs.length).toBeGreaterThan(0);
+    return engine.getRun(runs[0]!.id);
+  }
+
+  describe('the defect', () => {
+    it('the parent run REFUSES — it does not record `completed`', async () => {
+      const result = await runBatch();
+
+      expect(result.success).toBe(true);           // a refusal is a successful evaluation
+      expect(result.status).toBe('refused');
+      expect((await newestRun('batch'))?.status).toBe('refused');
+    });
+
+    it("the refusing item's rendered reason reaches the parent's caller", async () => {
+      const result = await runBatch();
+
+      // Interpolated in the CHILD against the child's own variables (the
+      // iterator value), passed through — ⛔ not re-rendered by the map node.
+      expect(result.refusalMessage).toBe('Refused: b is not eligible');
+      expect((await newestRun('batch'))?.refusalMessage).toBe('Refused: b is not eligible');
+    });
+
+    it("the parent's own `successMessage` stays silent over the refusal", async () => {
+      const result = await runBatch();
+
+      expect(result.successMessage).toBeUndefined();
+    });
+
+    it('the batch STOPS at the refusing item — later items never run', async () => {
+      // ⭐ The `map`-specific half of the defect: "approve each row" answering
+      // no on row 2 and approving row 3 anyway.
+      await runBatch();
+
+      expect(judged).toEqual(['a', 'b']);
+      expect(judged).not.toContain('c');
+    });
+
+    it('downstream nodes do NOT run — the map node is not walked past', async () => {
+      await runBatch();
+
+      expect(ran).toEqual([]);
+    });
+
+    it("preserves the batch's #4354 rollup on the refusal path (`selected` / `acted`)", async () => {
+      // ⭐ The property the unwinding POSITION exists for: the signal is thrown
+      // after this node's success step and its metrics are already in the run
+      // log. Items that already ran really did write rows, and so did the
+      // refusing item before it said no — both count. Two items x {2,1}.
+      const result = await runBatch();
+
+      // ⛔ The refusal assertion belongs IN this test, not next door: without it
+      // the totals below are equally true of the unfixed engine, which rolled
+      // the same metrics up and then carried on to the next item.
+      expect(result.status).toBe('refused');
+      expect(result.summary).toBeDefined();
+      expect(result.summary).toMatchObject({ selected: 4, acted: 2 });
+      expect(result.summary!.nodes.find((n) => n.nodeId === 'each')).toMatchObject({
+        selected: 4, acted: 2,
+      });
+    });
+  });
+
+  describe('the control — an ordinary batch still rolls up as an ordinary success', () => {
+    // ⛔ Mandatory, not decoration: every assertion above is also satisfied by
+    // a map that had started refusing EVERY batch.
+    it('a batch with no refusing item completes, fires the toast, and walks on', async () => {
+      deny = null;
+
+      const result = await runBatch();
+
+      expect(result.success).toBe(true);
+      expect(result.status).toBeUndefined();        // the terminal-success exit stamps none
+      expect(result.refusalMessage).toBeUndefined();
+      expect(result.successMessage).toBe(PARENT_TOAST);
+      expect(judged).toEqual(['a', 'b', 'c']);      // every item ran
+      expect(ran).toEqual(['downstream']);
+      expect((await newestRun('batch'))?.status).toBe('completed');
+      // Three items this time — the refusal arm added a branch, ⛔ it did not
+      // move the totals.
+      expect(result.summary).toMatchObject({ selected: 6, acted: 3 });
+    });
+  });
+});

packages/services/service-automation/src/builtin/subflow-node.ts

@@ -171,9 +171,10 @@ export function registerSubflowNode(engine: AutomationEngine, ctx: PluginContext
       // Deliberately the SAME exit the three totals above already leave by, so
       // this adds one total to an existing rollup and decides nothing new
       // about which child outcomes reach it. (A `refused` child — the run
-      // OUTCOME sense, an `end` node saying no — reaches this exit today and
-      // has since refusals existed; whether a parent should go on from one at
-      // all is a separate open question about this node, not this slot's.)
+      // OUTCOME sense, an `end` node saying no — used to reach this exit as an
+      // ordinary success; #18110 answered the open question this comment left
+      // and gives it the arm below, which leaves by this same exit with the
+      // same totals.)
       //
       // Absent, never zero: a child summary with no `failed` is a row recorded
       // before the count existed, and `0` would claim it was measured.
@@ -185,7 +186,43 @@ export function registerSubflowNode(engine: AutomationEngine, ctx: PluginContext
       // directly to the parent variable map).
       if (outVar) variables.set(outVar, child.output ?? null);
 
-      return { success: true, output: { output: child.output ?? null }, ...(metrics ? { metrics } : {}) };
+      const rollup = {
+        success: true as const,
+        output: { output: child.output ?? null },
+        ...(metrics ? { metrics } : {}),
+      };
+
+      // [#18110] The child REFUSED — an `end` inside it declared
+      // `outcome: 'refused'`, which is a successful evaluation that says NO.
+      //
+      // Until this arm existed every child status other than `paused` fell
+      // through the success exit below, so the parent walked straight down this
+      // node's out-edges, recorded `completed` and fired its OWN
+      // `successMessage` over the child's refusal: the author got the opposite
+      // of what they wrote, and fail-open — a refusing gate (approval,
+      // eligibility, a precondition) that lets the run through is the one kind
+      // of wrong nobody notices, because the flow finishes green.
+      //
+      // ⛔ NOT folded into the failure arm above. A refusal is not a failure:
+      // it must not consume retry budget, must not be routable by a `fault`
+      // edge, and must not be counted in `nodes[].failures` — all of which it
+      // would inherit from `success: false`. `refuse` is the channel for
+      // exactly this (`NodeExecutionResult.refuse`), and the engine throws its
+      // signal only after this step and these `metrics` are already in the run
+      // log — which is why the child's #4354 totals survive on the refusal path
+      // instead of being lost to the unwind.
+      //
+      // The envelope is the success one PLUS the refusal, deliberately: the
+      // child's output really was produced and the nodes before its refusing
+      // `end` really ran, so withholding them here would make the parent's
+      // answer depend on how the child ended rather than on what it did —
+      // the same reasoning `finishRefusedRun` applies to a refused run's own
+      // declared outputs.
+      if (child.status === 'refused') {
+        return { ...rollup, refuse: true, refusalMessage: child.refusalMessage };
+      }
+
+      return rollup;
     },
   });