fix(service-automation)!: the subflow guards see a subflow or map call inside a region body

.changeset/28846-region-nested-subflow-calls.md

@@ -0,0 +1,18 @@
+---
+'@objectstack/service-automation': patch
+---
+
+fix(service-automation)!: the subflow guards see a `subflow` / `map` call inside a region body (#28846)
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+
+**BREAKING** for a packaged flow that calls a packaged child from inside a `loop` body, a `parallel` branch or a `try_catch` slot.
+
+- **Before.** The ADR-0126 §7.3 guards read a flow's calls from its top-level nodes only. A `subflow` or `map` node inside a region body was invisible to them. A packaged caller was armed onto a disabled packaged child and failed mid-run at that node, and the child could be switched off or removed while an enabled caller still called it.
+- **Now.** The guards read calls at every region depth, through the spec's region walk (`collectFlowGraphs`). A packaged caller whose region body calls a disabled packaged child registers but is not armed: it warns, and `/_status` reports `unboundCause: 'disabled_subflow'` naming the child. Switching off or removing a packaged child that an enabled packaged caller calls from a region body is refused with `DELETE_RESTRICTED` (409), naming the caller. Re-arming when the child is enabled or republished `active` works as it does for a top-level call.
+- **Not changed.** The save-notice half of the arming gate (#28840) still reads top-level calls only. A region cannot carry a durable pause, so a region-nested call whose child suspends fails the run at the region boundary before anything after it runs. Registration, the save door and `objectstack validate` accept the same flows as before.
+- **The fix.** Enable the child, or publish it `active`, and the caller arms. To switch off or remove a child, switch off its callers first, as the refusal names.
+- **Who moves.** HotCRM's `campaign_enrollment` calls two active children from loop bodies. It still arms. Switching off or removing those children while it is enabled is now refused, as it already was for a top-level call. Showcase and dogfood calls are all top-level.
+
+<!-- adr-0087: not-required (no-migration-prescription) no metadata key is added, removed, renamed or re-shaped and the save door accepts the same flows: the runtime guards now read calls they already should have read, the remedy is an administrator's enable or a republish named in the warning and the refusal, and no migration could rewrite that choice -->

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

@@ -5215,6 +5215,13 @@ export class AutomationEngine implements IAutomationService {
      * `engine.execute`. Scanning only `subflow` would let a `map` caller break
      * exactly the way §7.3 exists to prevent.
      *
+     * [#28846] At EVERY region depth: a `subflow` / `map` node inside a
+     * `loop` body, a `parallel` branch or a `try_catch` slot calls its child
+     * on each pass exactly as a top-level one does, so a guard that read only
+     * the top-level `nodes` armed a caller onto a disabled child and let that
+     * child be switched off under it. The regions are read through
+     * `collectFlowGraphs`, the spec's one region walk, never a second walker.
+     *
      * ⛔ The historical `flow` alias for `config.flowName` is NOT read here.
      * ADR-0087 D2's conversion `flow-node-subflow-flow-alias` canonicalizes it
      * at load, so only the canonical key reaches a registered definition;

packages/services/service-automation/src/subflow-guard-region-calls.test.ts

@@ -0,0 +1,239 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// [#28846] The subflow guards see a `subflow` / `map` node inside a region
+// body (a `loop` body, a `parallel` branch, a `try_catch` slot).
+//
+// ADR-0126 §7.3 keeps a packaged caller from being armed onto a disabled
+// packaged child, and keeps that child from being switched off or removed
+// while an armed packaged caller still calls it. Each guard reads the caller's
+// calls through `subflowTargets`, which read only the top-level `nodes`: a
+// call inside a region body was invisible, so the caller armed onto a disabled
+// child and failed mid-run at that node, the late, inexplicable failure §7.3
+// refuses to cause. `subflowTargets` now reads every region depth through
+// `collectFlowGraphs`, the spec's one region walk, so every guard sees the
+// same calls.
+//
+// The save-notice half of the arming gate (#28840) is deliberately NOT moved
+// by this: a region body cannot carry a durable pause, so a region-nested call
+// whose child suspends fails the run at the region boundary before anything
+// after the container runs. That walk keeps reading top-level calls only, and
+// two pins below hold it there.
+
+import { describe, it, expect, vi } from 'vitest';
+import { AutomationEngine } from './engine.js';
+import type { FlowTrigger, FlowTriggerBinding } from './engine.js';
+import { InMemoryFlowActivationStore } from './flow-activation-store.js';
+import { registerSubflowNode } from './builtin/subflow-node.js';
+import { registerMapNode } from './builtin/map-node.js';
+import type { AutomationContext } from '@objectstack/spec/contracts';
+import { withLoaderSetFromPull } from './loader-set.test-support.js';
+
+function createTestLogger(): any {
+    const l: any = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() };
+    l.child = () => l;
+    return l;
+}
+
+const declineWarnings = (logger: any, caller: string): string[] =>
+    logger.warn.mock.calls
+        .map((c: unknown[]) => String(c[0]))
+        .filter((m: string) => m.includes(`Flow '${caller}'`) && m.includes('NOT armed'));
+
+function recordingTrigger() {
+    const bound = new Set<string>();
+    const trigger: FlowTrigger = {
+        type: 'record_change',
+        start(binding: FlowTriggerBinding, _cb: (ctx: AutomationContext) => Promise<void>) {
+            bound.add(binding.flowName);
+        },
+        stop(flowName: string) {
+            bound.delete(flowName);
+        },
+    };
+    return { trigger, isBound: (n: string) => bound.has(n) };
+}
+
+const nodeCtx = { logger: createTestLogger(), getService: () => undefined } as any;
+
+function engineWithLedger() {
+    const logger = createTestLogger();
+    const engine = withLoaderSetFromPull(new AutomationEngine(logger));
+    engine.setFlowActivationStore(new InMemoryFlowActivationStore());
+    registerSubflowNode(engine, nodeCtx);
+    registerMapNode(engine, nodeCtx);
+    const trigger = recordingTrigger();
+    engine.registerTrigger(trigger.trigger);
+    return { engine, logger, isBound: trigger.isBound };
+}
+
+const stateOf = (engine: AutomationEngine, name: string) =>
+    engine.getFlowRuntimeStates().find((s) => s.name === name);
+
+type Node = Record<string, unknown>;
+type Region = 'loop' | 'parallel' | 'try_catch';
+type CallType = 'subflow' | 'map';
+
+const END: Node = { id: 'end', type: 'end', label: 'End' };
+const start = (trigger: boolean): Node => ({
+    id: 'start', type: 'start', label: 'Start',
+    config: trigger ? { objectName: 'lead', triggerType: 'record-after-create' } : {},
+});
+const call = (flowName: string, type: CallType = 'subflow'): Node => ({
+    id: 'step', type, label: `Call ${flowName}`,
+    config: { flowName, ...(type === 'map' ? { collection: [1] } : {}) },
+});
+const NOOP: Node = { id: 'noop', type: 'assignment', label: 'Nothing', config: {} };
+
+/** `inner` as the one node of a region body of `kind`, in a container node `id`. */
+function region(kind: Region, inner: Node, id = 'box'): Node {
+    const body = { nodes: [inner], edges: [] };
+    if (kind === 'loop') {
+        return { id, type: 'loop', label: 'Box', config: { collection: [1], iteratorVariable: 'item', body } };
+    }
+    if (kind === 'parallel') {
+        return {
+            id, type: 'parallel', label: 'Box',
+            config: { branches: [{ name: 'a', ...body }, { name: 'b', nodes: [{ ...NOOP }], edges: [] }] },
+        };
+    }
+    return { id, type: 'try_catch', label: 'Box', config: { try: body, catch: { nodes: [{ ...NOOP }], edges: [] } } };
+}
+
+function chain(name: string, nodes: Node[], extra: Record<string, unknown> = {}) {
+    return {
+        name,
+        label: name,
+        type: 'autolaunched',
+        nodes,
+        edges: nodes.slice(1).map((n, i) => ({ id: `e${i}`, source: nodes[i].id as string, target: n.id as string })),
+        ...extra,
+    };
+}
+
+/** Shipped by a code package (ADR-0029 D9.6 provenance). */
+const PACKAGED = { _packageId: 'crm' };
+
+/** A packaged child flow `start → end`, active unless `status` says otherwise. */
+const child = (name: string, status?: string) =>
+    chain(name, [start(false), END], { ...PACKAGED, ...(status !== undefined ? { status } : {}) });
+
+/** A packaged record-triggered caller whose only call to `target` sits inside a region body. */
+const regionCaller = (name: string, target: string, kind: Region, type: CallType = 'subflow') =>
+    chain(name, [start(true), region(kind, call(target, type)), END], PACKAGED);
+
+const CASES: Array<[Region, CallType]> = [
+    ['loop', 'subflow'], ['loop', 'map'],
+    ['parallel', 'subflow'], ['parallel', 'map'],
+    ['try_catch', 'subflow'], ['try_catch', 'map'],
+];
+
+describe('the subflow guards read a call inside a region body (#28846)', () => {
+    it.each(CASES)('arming: a caller whose %s body holds a `%s` call to a disabled packaged child is declined, naming the child', (kind, type) => {
+        const { engine, logger, isBound } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step', 'obsolete'));
+        engine.registerFlow('vendor_process', regionCaller('vendor_process', 'shared_step', kind, type));
+
+        expect(isBound('vendor_process')).toBe(false);
+        const row = stateOf(engine, 'vendor_process');
+        expect(row).toMatchObject({ enabled: true, bound: false, unboundCause: 'disabled_subflow' });
+        expect(row?.reason).toContain("'shared_step' (its definition's status is 'obsolete')");
+        expect(declineWarnings(logger, 'vendor_process')).toHaveLength(1);
+    });
+
+    it('arming: a child republished `obsolete` under an armed caller that calls it from a loop body disarms the caller; republished `active` arms it again', () => {
+        const { engine, isBound } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step'));
+        engine.registerFlow('vendor_process', regionCaller('vendor_process', 'shared_step', 'loop'));
+        expect(isBound('vendor_process')).toBe(true);
+
+        engine.registerFlow('shared_step', child('shared_step', 'obsolete'));
+        expect(isBound('vendor_process')).toBe(false);
+        expect(stateOf(engine, 'vendor_process')?.unboundCause).toBe('disabled_subflow');
+
+        engine.registerFlow('shared_step', child('shared_step'));
+        expect(isBound('vendor_process')).toBe(true);
+        expect(stateOf(engine, 'vendor_process')?.unboundCause).toBeUndefined();
+    });
+
+    it('arming: a call two regions deep (a `parallel` branch inside a `loop` body) is read too', () => {
+        const { engine, isBound } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step', 'obsolete'));
+        engine.registerFlow(
+            'vendor_process',
+            chain('vendor_process', [start(true), region('loop', region('parallel', call('shared_step'), 'fan')), END], PACKAGED),
+        );
+        expect(isBound('vendor_process')).toBe(false);
+        expect(stateOf(engine, 'vendor_process')?.unboundCause).toBe('disabled_subflow');
+    });
+
+    it('removal: a child an armed packaged caller calls from a try_catch slot is refused with DELETE_RESTRICTED / 409, naming the caller', () => {
+        const { engine } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step'));
+        engine.registerFlow('vendor_process', regionCaller('vendor_process', 'shared_step', 'try_catch'));
+
+        let thrown: any;
+        try {
+            engine.unregisterFlow('shared_step');
+        } catch (e) {
+            thrown = e;
+        }
+        expect(thrown, 'removing the child was accepted').toBeDefined();
+        expect(thrown.code).toBe('DELETE_RESTRICTED');
+        expect(thrown.status).toBe(409);
+        expect(thrown.subflowCallers).toEqual(['vendor_process']);
+        expect(thrown.message).toContain("Flow 'shared_step' cannot be removed");
+        expect(engine.getFlow('shared_step'), 'nothing was removed').toBeDefined();
+    });
+
+    it('switch-off: a child an enabled packaged caller calls from a parallel branch is refused with DELETE_RESTRICTED / 409, naming the caller', async () => {
+        const { engine } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step'));
+        engine.registerFlow('vendor_process', regionCaller('vendor_process', 'shared_step', 'parallel', 'map'));
+
+        const thrown: any = await engine.toggleFlow('shared_step', false).then(() => undefined, (e: unknown) => e);
+        expect(thrown, 'switching the child off was accepted').toBeDefined();
+        expect(thrown.code).toBe('DELETE_RESTRICTED');
+        expect(thrown.status).toBe(409);
+        expect(thrown.subflowCallers).toEqual(['vendor_process']);
+        expect(thrown.message).toContain("Flow 'shared_step' cannot be disabled");
+    });
+
+    it('CONTROL: a caller whose loop body calls an ACTIVE packaged child is armed, with no warning', () => {
+        const { engine, logger, isBound } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step'));
+        engine.registerFlow('vendor_process', regionCaller('vendor_process', 'shared_step', 'loop'));
+        expect(isBound('vendor_process')).toBe(true);
+        expect(stateOf(engine, 'vendor_process')?.unboundCause).toBeUndefined();
+        expect(declineWarnings(logger, 'vendor_process')).toEqual([]);
+    });
+
+    it('CONTROL: a top-level call to a disabled packaged child is declined as before', () => {
+        const { engine, isBound } = engineWithLedger();
+        engine.registerFlow('shared_step', child('shared_step', 'obsolete'));
+        engine.registerFlow('vendor_process', chain('vendor_process', [start(true), call('shared_step'), END], PACKAGED));
+        expect(isBound('vendor_process')).toBe(false);
+        expect(stateOf(engine, 'vendor_process')?.unboundCause).toBe('disabled_subflow');
+    });
+});
+
+describe('the save-notice half of the arming gate keeps reading top-level calls (#28840, #28846)', () => {
+    const WAIT: Node = { id: 'hold', type: 'wait', label: 'Hold', waitEventConfig: { eventType: 'timer', timerDuration: 'PT1H' } };
+    const NOTICE: Node = { id: 'tell', type: 'save_notice', label: 'Tell the saver', config: { message: 'Saved' } };
+
+    it('a save_notice after a region whose body calls a suspending child is not declined on that call: the region cannot carry the pause, so the run fails at the region, before the notice', () => {
+        const { engine, isBound } = engineWithLedger();
+        engine.registerFlow('deal_review', chain('deal_review', [start(false), WAIT, END]));
+        engine.registerFlow('deal_submit', chain('deal_submit', [start(true), region('loop', call('deal_review')), NOTICE, END]));
+        expect(isBound('deal_submit')).toBe(true);
+        expect(stateOf(engine, 'deal_submit')?.unboundCause).toBeUndefined();
+    });
+
+    it('a child whose only call to a suspending flow sits in a region body does not count as suspending', () => {
+        const { engine, isBound } = engineWithLedger();
+        engine.registerFlow('deal_ask', chain('deal_ask', [start(false), WAIT, END]));
+        engine.registerFlow('deal_review', chain('deal_review', [start(false), region('loop', call('deal_ask')), END]));
+        engine.registerFlow('deal_submit', chain('deal_submit', [start(true), call('deal_review'), NOTICE, END]));
+        expect(isBound('deal_submit')).toBe(true);
+        expect(stateOf(engine, 'deal_submit')?.unboundCause).toBeUndefined();
+    });
+});

packages/services/service-automation/src/subflow-guards.ts

@@ -13,6 +13,7 @@
  * no longer `async`: it returns the promise the moved function makes.
  */
 
+import { collectFlowGraphs } from '@objectstack/spec/automation';
 import type { FlowParsed } from '@objectstack/spec/automation';
 import { describeTurnedOffPackagedItem } from '@objectstack/types';
 import type { AutomationEngine } from './engine.js';
@@ -57,15 +58,16 @@ export function describeDisabledFlow(this: Pick<SubflowGuardsHost, 'flowLedgerDi
     return `Flow '${flowName}' is disabled`;
 }
 
-/** The body of {@link AutomationEngine.subflowTargets}, moved unedited; that method's docblock is its contract. */
+/** The body of {@link AutomationEngine.subflowTargets}; that method's docblock is its contract. */
 export function subflowTargets(flow: FlowParsed): string[] {
-    const nodes = (flow as { nodes?: unknown }).nodes;
-    if (!Array.isArray(nodes)) return [];
     const targets: string[] = [];
-    for (const node of nodes) {
-        const n = node as { type?: unknown; config?: { flowName?: unknown } } | null;
-        if (!n || (n.type !== 'subflow' && n.type !== 'map')) continue;
-        if (typeof n.config?.flowName === 'string') targets.push(n.config.flowName);
+    // [#28846] The top-level graph and every region body, in walk order.
+    for (const graph of collectFlowGraphs(flow)) {
+        for (const node of graph.nodes) {
+            const n = node as { type?: unknown; config?: { flowName?: unknown } } | null;
+            if (!n || (n.type !== 'subflow' && n.type !== 'map')) continue;
+            if (typeof n.config?.flowName === 'string') targets.push(n.config.flowName);
+        }
     }
     return targets;
 }

packages/services/service-automation/src/subflow-notice-guard.ts

@@ -19,9 +19,12 @@
  * is repeated here rather than exported from `@objectstack/spec`, which keeps
  * no internal entry: the order prefers that to a new public helper.
  *
- * Only top-level `subflow` / `map` nodes are read, the ones
- * {@link AutomationEngine.subflowTargets} answers, so the gate and the
- * re-judge read the same calls.
+ * Only top-level `subflow` / `map` nodes are read, here and in a child: a
+ * call inside a region body cannot suspend the run, because a region cannot
+ * carry a durable pause and the run fails at its boundary instead (#28846).
+ * The re-judge finds its callers through
+ * {@link AutomationEngine.subflowTargets}, which reads every region depth, so
+ * it re-asks a superset of the flows this reading can move.
  */
 
 import { FLOW_PAUSE_CAPABLE_NODE_TYPES, collectFlowGraphs } from '@objectstack/spec/automation';

packages/spec/src/automation/flow.zod.ts

@@ -1640,7 +1640,11 @@ export const FlowSchema = lazySchema(() => strictObject(
   // stands only in a flow a create or update record trigger starts — anywhere
   // else no write waits on the run. Refused at each such node, at any region
   // depth; the reasons are `save-notice-placement.ts`'s header. [#28806] Also
-  // refused where a node that always suspends the run reaches it.
+  // refused where a node that always suspends the run reaches it. A notice
+  // after a `subflow` / `map` whose child can suspend is not refused here: the
+  // engine's arming gate declines that flow (#28840,
+  // `notice_after_suspending_subflow`), and the run-time refusal by name is the
+  // residue.
   for (const refusal of saveNoticePlacementRefusals(flow, FLOW_PAUSE_CAPABLE_NODE_TYPES)) {
     ctx.addIssue({ code: 'custom', path: [...refusal.path], message: refusal.message });
   }