feat(service-automation)!: decline to arm a save_notice that runs after a subflow whose child can suspend

.changeset/28840-subflow-notice-arming.md

@@ -0,0 +1,19 @@
+---
+'@objectstack/service-automation': patch
+'@objectstack/spec': patch
+---
+
+feat(service-automation)!: a flow whose `save_notice` runs after a `subflow` / `map` whose child can suspend is registered but not armed (#28840)
+
+Clause-②: yes (narrowing)
+Metadata-protocol: none
+
+**BREAKING** for a record-triggered flow that places a `save_notice` node after a `subflow` or `map` node whose child flow can suspend the run.
+
+- **Before.** The flow parse refuses a `save_notice` that an `approval`, `wait`, `screen` or `approval_revise` node reaches (#28806), but it cannot see into a `subflow` / `map` child. A flow that asked the approval in a subflow and told the saver after it registered and armed. Its child suspended it, the write answered, and on the resume the `save_notice` was refused by name, so every decision stranded the run (`RESUME_FAILED`).
+- **Now.** The arming gate in `activateFlowTrigger` declines such a flow, beside its disabled-subflow half (ADR-0126 §7.3). "Can suspend" means the child holds a node of an always-suspending type reachable from its start, or calls a child that can suspend, read through every call with cycle protection. The flow still registers, is not armed, and warns once, naming the `save_notice`, the `subflow` / `map` node and the child's suspending node. `GET /automation/_status` reports it `bound: false` with that reason and the new `unboundCause` value `notice_after_suspending_subflow` (added to `FLOW_UNBOUND_CAUSES` in `@objectstack/spec/contracts`). Whenever a flow is registered, every flow whose calls reach it is judged again, so the decline does not depend on load order, and a child republished without its suspension arms the caller again.
+- **Not declined.** A `subflow` / `map` whose child cannot suspend, and a `save_notice` reachable only before the call. Registration, the save door and `objectstack validate` accept the same flows as before.
+- **The fix.** Move the `save_notice` ahead of the `subflow` / `map` node the reason names, or tell the person after it with a `notify` node, or republish the child without the node that suspends. The flow arms on its own once the shape is gone.
+- **Who moves.** No shipped flow has the shape: the showcase's one `save_notice` (`showcase_task_completed`) has no `subflow` or `map` node, the dogfood fixtures carry no `save_notice`, and HotCRM's `main` carries none.
+
+<!-- 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: a flow that registered still registers and is only left unarmed by the runtime, with its own reason naming what to move, and the one contract change is a new value in the runtime status cause list, which no author writes and no migration could rewrite -->

packages/plugins/plugin-approvals/src/save-notice-approval-round-trip.integration.test.ts

@@ -20,7 +20,20 @@
  * `subflow` that asks the approval: the parse cannot see that a subflow
  * pauses (it does only when its child does), so that is where the run-time
  * refusal is still the guard. The parse refusal itself is pinned at
- * `registerFlow` in the second case.
+ * `registerFlow` in the last case.
+ *
+ * [#28840] That subflow shape is now declined by the arming gate: a
+ * `save_notice` after a `subflow` whose child can suspend registers but is
+ * NOT armed, so the create no longer starts it. The first case pins that
+ * decline through the real record-change trigger: the flow registers, the
+ * create fires nothing, and `/_status` names the `subflow` node and the
+ * child's `approval` node. The resumed leg stays reachable through a run
+ * started directly (`automation.execute`) with the context the record-change
+ * trigger hands a run, the write's notice channel included, so the second
+ * case still pins that the stored run drops the channel and its resumed
+ * `save_notice` refuses by name. What the direct start skips is the trigger's
+ * hand-off of the channel, which the record-change trigger's own suite pins
+ * (#28251).
  *
  * The executor's own suite pins the run-time refusal with a hand-built
  * context. This suite pins it through the real round trip, so a store that
@@ -43,7 +56,8 @@
  *
  * Ablated: with the executor's no-channel refusal replaced by a success, the
  * resumed-leg assertions go red (`save-notice-node.ts`), and are green again
- * once it is restored.
+ * once it is restored. With the arming gate's save-notice half removed, the
+ * first case goes red (the flow is armed and the create starts it).
  */
 
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
@@ -89,6 +103,9 @@ const deal = {
  * the graph is accepted, the parent suspends with its child, persists, and
  * resumes from storage when the decision completes the child. The resumed
  * parent leg is where the second `save_notice` meets the run-time refusal.
+ *
+ * [#28840] The arming gate declines this graph, so the create never starts
+ * it; the run-time case starts it directly.
  */
 const CHILD = 'deal_manager_review';
 const child = {
@@ -205,15 +222,42 @@ describe('[#28786] a save_notice before an approval answers the write, and one o
     try { await kernel?.shutdown(); } catch { /* noop */ }
   });
 
-  it('the create hears the sentence, the run is persisted without the channel, and the resumed save_notice refuses', async () => {
-    // ── The write: the save_notice ahead of the approval answers it.
+  it('[#28840] the flow registers but is not armed: the create starts nothing, and /_status names the subflow node and the child\'s approval', async () => {
+    const automation = kernel.getService<AutomationEngine>('automation');
+    expect(automation.getFlow(FLOW), 'the flow is registered').toBeDefined();
+    const row = automation.getFlowRuntimeStates().find((s) => s.name === FLOW);
+    expect(row).toMatchObject({ enabled: true, bound: false, unboundCause: 'notice_after_suspending_subflow' });
+    expect(row?.reason).toContain("`save_notice` node 'tell_after' is reached after `subflow` node 'review'");
+    expect(row?.reason).toContain(`child flow '${CHILD}' can suspend the run at \`approval\` node 'ask'`);
+
     const heard: unknown[] = [];
-    const created: any = await ql.insert(
+    await ql.insert(
       deal.name,
-      { name: 'Acme' },
+      { name: 'Globex' },
       { context: { userId: SUBMITTER }, onNotice: (message: unknown) => { heard.push(message); } } as any,
     );
-    expect(heard, "the create's own answer carried the notice").toEqual(['Sent Acme to your manager for approval']);
+    expect(heard, 'no run answered the create').toEqual([]);
+    expect(await runRows(), 'the create started no run').toEqual([]);
+  });
+
+  it('a run started with the write\'s channel is persisted without it, and its resumed save_notice refuses by name', async () => {
+    const automation = kernel.getService<AutomationEngine>('automation');
+    const created: any = await ql.insert(deal.name, { name: 'Acme' }, { context: SYSTEM });
+
+    // ── The run, started as the record-change trigger starts one: the record,
+    // the writer, and the write's notice channel, whose `set` answers the write.
+    const heard: unknown[] = [];
+    const started: any = await automation.execute(FLOW, {
+      record: { ...created },
+      previous: undefined,
+      object: deal.name,
+      event: 'insert',
+      userId: SUBMITTER,
+      params: { ...created },
+      saveNotice: { set: (message: string) => { heard.push(message); } },
+    } as any);
+    expect(started.success, 'the run suspended at its subflow, it did not fail').toBe(true);
+    expect(heard, 'the save_notice ahead of the subflow set the sentence').toEqual(['Sent Acme to your manager for approval']);
 
     // ── Suspended and persisted: one live row, its context stored as JSON.
     const [pause] = await runRows();

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

@@ -221,6 +221,7 @@ import {
     rejudgeSubflowCallers,
     refuseCustomerAuthoredToggle,
 } from './subflow-guards.js';
+import { describeSubflowNoticeTrap, subflowNoticeTrap } from './subflow-notice-guard.js';
 // [#25222] The consumed-suspension restore's bodies: journaling a consumed
 // suspension, resolving its witnesses, and restoring, peeking at or inspecting
 // one. Each `AutomationEngine` method of the same name calls its body.
@@ -2904,6 +2905,16 @@ export class AutomationEngine implements IAutomationService {
      * {@link unregisterFlow} drops it with the flow.
      */
     private readonly subflowDeclinedFlows = new Map<string, string>();
+    /**
+     * [#28840] Flows {@link activateFlowTrigger} declined to arm because a
+     * `save_notice` of theirs runs after a `subflow` / `map` node whose child
+     * flow can suspend, each with the reason `/_status` and the binding audit
+     * read. A record for {@link subflowDeclinedFlows}'s reasons: deleted the
+     * moment the flow gets past the gate, re-asked by
+     * {@link rejudgeSubflowCallers} whenever a flow its calls reach is
+     * registered, and dropped with the flow by {@link withdrawFlow}.
+     */
+    private readonly subflowNoticeDeclinedFlows = new Map<string, string>();
     /** Connectors registered by integration plugins, keyed by connector name (ADR-0018 §Addendum). */
     private connectors = new Map<string, RegisteredConnector>();
     /** Connector provider factories keyed by provider name (ADR-0097 §2 — `openapi`/`mcp`/`rest`/…). */
@@ -3880,6 +3891,14 @@ export class AutomationEngine implements IAutomationService {
         // trigger would change nothing, so "no trigger registered" would name
         // a remedy that cannot work.
         if (this.declineOntoDisabledSubflows(flowName, resolved.triggerType)) return;
+        // [#28840] The save-notice half, a decline for the same reasons: a
+        // `save_notice` after a `subflow` / `map` node whose child can suspend
+        // is refused by name when the suspended run resumes, after the write
+        // answered, and a decision that resumes it is stranded. The flow parse
+        // cannot see the child (#28806), which may register after this flow,
+        // so the gate asks here and {@link rejudgeSubflowCallers} asks again
+        // when the child arrives or changes.
+        if (this.declineOntoSuspendingSubflowNotice(flowName, resolved.triggerType)) return;
         const trigger = this.triggers.get(resolved.triggerType);
         if (!trigger) return;
         try {
@@ -4629,6 +4648,7 @@ export class AutomationEngine implements IAutomationService {
             flowVersionHistory: this.flowVersionHistory,
             policyDisabledFlows: this.policyDisabledFlows,
             subflowDeclinedFlows: this.subflowDeclinedFlows,
+            subflowNoticeDeclinedFlows: this.subflowNoticeDeclinedFlows,
             flowRefusals: this.flowRefusals,
             logger: this.logger,
         };
@@ -4759,6 +4779,7 @@ export class AutomationEngine implements IAutomationService {
             boundFlowTriggers: this.boundFlowTriggers,
             policyDisabledFlows: this.policyDisabledFlows,
             subflowDeclinedFlows: this.subflowDeclinedFlows,
+            subflowNoticeDeclinedFlows: this.subflowNoticeDeclinedFlows,
             triggers: this.triggers,
         };
         return describeUnboundReason.call(host, name, resolved);
@@ -5489,6 +5510,48 @@ export class AutomationEngine implements IAutomationService {
         return true;
     }
 
+    /**
+     * [#28840] The arming gate's save-notice half, asked by
+     * {@link activateFlowTrigger} of an enabled flow it is about to arm: does a
+     * `save_notice` of it run after a `subflow` / `map` node whose child flow
+     * can suspend ({@link subflowNoticeTrap}: a pausing node reachable from the
+     * child's start, or a child of the child that can suspend, transitively)?
+     * Then it is declined: recorded on {@link subflowNoticeDeclinedFlows} for
+     * `/_status` and the binding audit, and said once as a `warn` naming the
+     * notice, the call node and where the child suspends.
+     *
+     * Any flow, packaged or authored here: the stranded decision is the same.
+     * ⛔ A decline, never a refusal, for {@link declineOntoDisabledSubflows}'s
+     * reason and one of its own: which flows suspend depends on what else is
+     * registered, so `registerFlow` cannot refuse it without depending on load
+     * order.
+     *
+     * #4632 verdict: FUNCTIONAL, `warn`, as the disabled-subflow decline: the
+     * flow is visibly smaller than declared and `/_status` says why. Said once
+     * per decline reason.
+     *
+     * @returns `true` when the flow is declined and must not be armed.
+     */
+    private declineOntoSuspendingSubflowNotice(flowName: string, triggerType: string): boolean {
+        const trap = subflowNoticeTrap(this.flows, flowName);
+        if (!trap) {
+            this.subflowNoticeDeclinedFlows.delete(flowName);
+            return false;
+        }
+        const why = describeSubflowNoticeTrap(trap);
+        const reason =
+            `not armed — ${why}, and it is armed; it is also armed once '${trap.child}' is republished without a ` +
+            `node that suspends`;
+        const previous = this.subflowNoticeDeclinedFlows.get(flowName);
+        this.subflowNoticeDeclinedFlows.set(flowName, reason);
+        if (previous === reason) return true;
+        this.logger.warn(
+            `Flow '${flowName}' is registered but NOT armed on trigger '${triggerType}' — ${why}. It is armed the ` +
+                `moment the \`save_notice\` moves, or '${trap.child}' is republished without a node that suspends.`,
+        );
+        return true;
+    }
+
     /**
      * [#20725, ADR-0126 §7.3] `subflow` just changed state — (re)registered,
      * switched on or off, or read off the ledger at boot — so ask the arming
@@ -5505,12 +5568,22 @@ export class AutomationEngine implements IAutomationService {
      * Every other caller is left exactly as it is: an armed caller whose
      * subflows are all enabled is not stopped and restarted, and a caller
      * unbound for a reason of its own is not re-offered here.
+     *
+     * [#28840] And the save-notice half of the gate, for every flow whose
+     * calls reach `subflow`, directly or through other flows, packaged or
+     * not: whether a child can suspend is read through its own calls, so a
+     * change two calls away moves the answer. A caller that half declined is
+     * re-offered, and an armed caller it would now decline is unbound and
+     * offered again, both through the one gate.
      */
     private rejudgeSubflowCallers(subflow: string): void {
         // The body lives in ./subflow-guards.ts (#25222).
         const host: ThisParameterType<typeof rejudgeSubflowCallers> = {
             boundFlowTriggers: this.boundFlowTriggers,
             subflowDeclinedFlows: this.subflowDeclinedFlows,
+            subflowNoticeDeclinedFlows: this.subflowNoticeDeclinedFlows,
+            flows: this.flows,
+            subflowTargets: (...a) => this.subflowTargets(...a),
             packagedSubflowCallers: (...a) => this.packagedSubflowCallers(...a),
             disabledPackagedSubflows: (...a) => this.disabledPackagedSubflows(...a),
             deactivateFlowTrigger: (...a) => this.deactivateFlowTrigger(...a),

packages/services/service-automation/src/flow-definitions.ts

@@ -74,6 +74,7 @@ export interface FlowDefinitionsHost {
     readonly boundFlowTriggers: AutomationEngine['boundFlowTriggers'];
     readonly policyDisabledFlows: AutomationEngine['policyDisabledFlows'];
     readonly subflowDeclinedFlows: AutomationEngine['subflowDeclinedFlows'];
+    readonly subflowNoticeDeclinedFlows: AutomationEngine['subflowNoticeDeclinedFlows'];
     readonly triggers: AutomationEngine['triggers'];
     readonly flowCredentialSource: AutomationEngine['flowCredentialSource'];
     readonly flowVersionHistory: AutomationEngine['flowVersionHistory'];
@@ -287,9 +288,9 @@ export function getFlowRuntimeStates(this: Pick<FlowDefinitionsHost, 'flows' | '
     });
 }
 
-/** The body of {@link AutomationEngine.describeUnboundReason}, moved unedited; that method's docblock is its contract. */
+/** The body of {@link AutomationEngine.describeUnboundReason}; that method's docblock is its contract. */
 export function describeUnboundReason(
-    this: Pick<FlowDefinitionsHost, 'isFlowEnabled' | 'boundFlowTriggers' | 'policyDisabledFlows' | 'subflowDeclinedFlows' | 'triggers'>,
+    this: Pick<FlowDefinitionsHost, 'isFlowEnabled' | 'boundFlowTriggers' | 'policyDisabledFlows' | 'subflowDeclinedFlows' | 'subflowNoticeDeclinedFlows' | 'triggers'>,
     name: string,
     resolved: { triggerType: string } | undefined,
 ): { reason: string; cause: FlowUnboundCause } | undefined {
@@ -304,6 +305,12 @@ export function describeUnboundReason(
     // child off, the trigger would not arm it either.
     const declined = this.subflowDeclinedFlows.get(name);
     if (declined !== undefined) return { reason: declined, cause: 'disabled_subflow' };
+    // [#28840] The gate's save-notice half declined it: a `save_notice` runs
+    // after a `subflow` / `map` node whose child can suspend. Its own cause,
+    // for its own owner and remedy (the author moves the notice, or the child
+    // stops suspending).
+    const noticeDeclined = this.subflowNoticeDeclinedFlows.get(name);
+    if (noticeDeclined !== undefined) return { reason: noticeDeclined, cause: 'notice_after_suspending_subflow' };
     return this.triggers.has(resolved.triggerType)
         ? {
             reason: `trigger '${resolved.triggerType}' is registered but binding failed — see earlier warnings`,
@@ -415,8 +422,8 @@ export function unregisterConnectorProvider(this: Pick<FlowDefinitionsHost, 'con
     this.logger.info(`Connector provider unregistered: ${providerKey}`);
 }
 
-/** The body of {@link AutomationEngine.withdrawFlow}, moved unedited; that method's docblock is its contract. */
-export function withdrawFlow(this: Pick<FlowDefinitionsHost, 'deactivateFlowTrigger' | 'flows' | 'flowStatusDisabled' | 'flowVersionHistory' | 'policyDisabledFlows' | 'subflowDeclinedFlows' | 'flowRefusals' | 'logger'>, name: string): void {
+/** The body of {@link AutomationEngine.withdrawFlow}; that method's docblock is its contract. */
+export function withdrawFlow(this: Pick<FlowDefinitionsHost, 'deactivateFlowTrigger' | 'flows' | 'flowStatusDisabled' | 'flowVersionHistory' | 'policyDisabledFlows' | 'subflowDeclinedFlows' | 'subflowNoticeDeclinedFlows' | 'flowRefusals' | 'logger'>, name: string): void {
     this.deactivateFlowTrigger(name);
     this.flows.delete(name);
     // [ADR-0126 §7.2] `flowLedgerDisabled` is deliberately NOT cleared. It
@@ -433,6 +440,7 @@ export function withdrawFlow(this: Pick<FlowDefinitionsHost, 'deactivateFlowTrig
     // an in-process bind attempt that no longer has a subject.
     this.policyDisabledFlows.delete(name);
     this.subflowDeclinedFlows.delete(name);
+    this.subflowNoticeDeclinedFlows.delete(name);
     // [#23316] A withdrawal ends a refusal as it ends a registration: the
     // record's subject is gone. A host that withdraws BECAUSE it refused
     // records the refusal after this — see {@link recordFlowRefusal}.