From 93a6bf0af7de4126261beaff77a81e6170d1780b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 04:04:56 +0000 Subject: [PATCH 1/3] wip(service-automation): roll a completed child's contained failures into the delegating node's failure count Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude --- content/docs/automation/flows.mdx | 4 +- .../src/builtin/map-node.ts | 36 ++++++++++- .../src/builtin/subflow-node.ts | 54 +++++++++++++---- .../services/service-automation/src/engine.ts | 15 +++++ .../service-automation/src/run-summary.ts | 59 +++++++++++++++---- 5 files changed, 142 insertions(+), 26 deletions(-) diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index c65998223cd..2c0847fb4d4 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -1124,8 +1124,8 @@ run in `listRuns` / `getRun`, and in the log: | `acted` | Records **created / updated / deleted**, plus effects dispatched (notifications delivered) | | `skipped` | Node executions a **closed gate** prevented — one per loop iteration whose conditional edge evaluated false | | `unmeasured` | Executions that reached something the platform **cannot count** — see below | -| `failed` | Node executions **of this run** that failed — on a completed run every one of them was contained, caught by a `try_catch` or routed down a `fault` edge, so the run went on; the sum of `nodes[].failures`. `failed=0` therefore reads "no node execution of this run failed", which is narrower than "nothing failed anywhere": a `subflow` child's own contained failures are counted on the CHILD's summary, not folded up here the way `acted` is (that inconsistency in the declaration is [#15617](https://github.com/objectstack-ai/objectstack/issues/15617)). Absent on a run that did not track it, which is not zero | -| `nodes[]` | Per-node terminal status with `runs` / `failures` / `skipped` and its own selected/acted | +| `failed` | Node executions that failed — on a completed run every one of them was contained, caught by a `try_catch` or routed down a `fault` edge, so the run went on; the sum of `nodes[].failures`. The fold **includes what a delegating node rolled up from its child**: a `subflow` child, or a `map` item, that COMPLETED while containing failures reports them on the delegating step, exactly as `acted` already rode up, so a parent whose child lost a row does not read `failed=0`. A child that **failed** rather than contained is the delegating step's own failure, counted once there, and its own `failed` stays on the child's run row. `failed=0` therefore reads "nothing this run caused failed, subflows included". Absent on a run that did not track it, which is not zero | +| `nodes[]` | Per-node terminal status with `runs` / `failures` / `skipped` and its own selected/acted. `status` is judged on the node's **own** executions, so a delegating node that ran fine and rolled a child's contained failures up reads `success` with `failures > 0` — and on such a node `failures` may exceed `runs` | | `gates[]` | Which gates closed and how often, most-skipped first | The counts come from the node executors themselves — `get_record` reports what diff --git a/packages/services/service-automation/src/builtin/map-node.ts b/packages/services/service-automation/src/builtin/map-node.ts index 0f4c526d077..6ec4ee9326d 100644 --- a/packages/services/service-automation/src/builtin/map-node.ts +++ b/packages/services/service-automation/src/builtin/map-node.ts @@ -148,6 +148,16 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v let selected = 0; let acted = 0; let unmeasured = false; + // #15617 — the contained failures of the items that COMPLETED during this + // entry, on their own rule (see the three exits below). `tracked` is the + // presence bit the other totals do not need: `ExecutionStepMetrics` + // declares an absent `failures` as "delegated nothing, or the child + // tracked no count", never zero, so an entry that ran no item — or only + // items recorded before the count existed — reports nothing here rather + // than a `0` that would claim a measurement nobody took. + let failures = 0; + let failuresTracked = false; + const rolledFailures = (): { failures?: number } => (failuresTracked ? { failures } : {}); // Drive items in order. Synchronous items advance inline; a pausing item // suspends the run and is resumed via re-entry. @@ -187,7 +197,11 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v variables.set(stateKey, state); return { success: true, suspend: true, correlation: `map:${child.runId}`, - metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}) }, + // The pausing item has done nothing yet; what rides out here is what + // the items BEFORE it contained. The engine credits the pausing + // item's own totals to this same step when its child run bubbles + // back (AutomationEngine.creditChildRun). + metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}), ...rolledFailures() }, }; } if (!child.success) { @@ -195,11 +209,20 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v success: false, error: `map '${node.id}': item ${idx} (subflow '${flowName}') failed: ${child.error ?? 'unknown error'}`, // Items that already succeeded wrote real rows; a later item's - // failure must not erase them from the run's totals. + // failure must not erase them from the run's totals — and the + // failing item's own writes count too, for the same reason. + // + // ⛔ `failures` is the one total that does NOT take the failing + // item's contribution (#15617): a child that FAILED is THIS step's + // own failure, counted once through `nodes[].failures`, and its own + // `failed` — contained and fatal alike — stays on its run row. + // Rolling it up would count one loss twice. The items that already + // completed keep theirs, which is what the accumulator holds. metrics: { selected: selected + (child.summary?.selected ?? 0), acted: acted + (child.summary?.acted ?? 0), ...(unmeasured || child.summary?.unmeasured ? { unmeasuredEffect: true } : {}), + ...rolledFailures(), }, }; } @@ -211,6 +234,13 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v // One uncountable effect anywhere in the batch makes the batch's // `acted` incomplete — the flag rides out with this entry's metrics. if (child.summary?.unmeasured) unmeasured = true; + // #15617 — this item went on and contained its failures, so they are + // this run's to answer for. `undefined` is "the child tracked no + // count", so it neither adds nor flips the presence bit. + if (child.summary?.failed !== undefined) { + failures += child.summary.failed; + failuresTracked = true; + } } // All items done — the collection is exhausted, so this is the node's @@ -240,7 +270,7 @@ export function registerMapNode(engine: AutomationEngine, ctx: PluginContext): v return { success: true, output: { results: state.results, count: state.results.length }, - metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}) }, + metrics: { selected, acted, ...(unmeasured ? { unmeasuredEffect: true } : {}), ...rolledFailures() }, }; }, }); diff --git a/packages/services/service-automation/src/builtin/subflow-node.ts b/packages/services/service-automation/src/builtin/subflow-node.ts index 8e069f462dc..ea77e607424 100644 --- a/packages/services/service-automation/src/builtin/subflow-node.ts +++ b/packages/services/service-automation/src/builtin/subflow-node.ts @@ -129,29 +129,63 @@ export function registerSubflowNode(engine: AutomationEngine, ctx: PluginContext // delegates its writes to a subflow reports `acted: 0` and reads as a // broken sweep. The child keeps its own run row with its own summary; // the parent's answers "what did this run cause", subflows included. + // + // These three ride up from a child that completed AND one that failed + // alike — a child that wrote rows before it died really did write them. + // `failures` (#15617) does NOT, which is why it is added past the failure + // exit below and not here; the two rules are deliberately different and + // `ExecutionStepMetrics.failures` names `acted` as the contrast. const rolled = child.summary ? { - metrics: { - selected: child.summary.selected, - acted: child.summary.acted, - // An uncountable effect inside the child is uncountable for the - // parent too — the parent's `acted` cannot be read as complete. - ...(child.summary.unmeasured ? { unmeasuredEffect: true } : {}), - }, + selected: child.summary.selected, + acted: child.summary.acted, + // An uncountable effect inside the child is uncountable for the + // parent too — the parent's `acted` cannot be read as complete. + ...(child.summary.unmeasured ? { unmeasuredEffect: true } : {}), } - : {}; + : undefined; if (!child.success) { // A failed child may still have written rows before it died — carry its // counts so the parent's summary does not understate what happened. - return { success: false, error: `subflow '${flowName}' failed: ${child.error ?? 'unknown error'}`, ...rolled }; + // + // ⛔ And no `failures`: a child that FAILED — whether or not it also + // contained failures before it failed — is THIS step's own failure, + // counted once through `nodes[].failures` as it always was. Its own + // `failed`, contained and fatal alike, stays on the child's run row; + // rolling it up here would count one loss twice. + return { + success: false, + error: `subflow '${flowName}' failed: ${child.error ?? 'unknown error'}`, + ...(rolled ? { metrics: rolled } : {}), + }; } + // #15617 — the child went on and CONTAINED its failures. Those steps + // live in the child's log, so until this slot existed the parent's fold + // could not see them and a parent whose child lost a row read + // `failed: 0` — the misreading the run-level `failed` was added to + // prevent (#13681), one level up. Rolled up here, it folds into this + // node's `failures` and so into the run-level `failed`. + // + // 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.) + // + // Absent, never zero: a child summary with no `failed` is a row recorded + // before the count existed, and `0` would claim it was measured. + const metrics = rolled && child.summary?.failed !== undefined + ? { ...rolled, failures: child.summary.failed } + : rolled; + // Bare output variable (like the assignment node, the executor may write // directly to the parent variable map). if (outVar) variables.set(outVar, child.output ?? null); - return { success: true, output: { output: child.output ?? null }, ...rolled }; + return { success: true, output: { output: child.output ?? null }, ...(metrics ? { metrics } : {}) }; }, }); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 86fede46cfb..c6e5ebc9765 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -5881,6 +5881,12 @@ export class AutomationEngine implements IAutomationService { * The credit lands on the LAST step for the node, which is the entry that * suspended awaiting this child — so a `map` re-entering once per item * credits each item to its own step and nothing is counted twice. + * + * Both call sites are COMPLETION paths — the up-bubble is raised only from + * a child that completed, and the down-delegation path returns on + * `!childRes.success` before reaching here — which is what lets #15617's + * `failures` ride this seam under exactly its declared rule: the contained + * failures of a child that COMPLETED, never a failed child's own `failed`. */ private creditChildRun(steps: StepLogEntry[], nodeId: string, child: FlowRunSummary | undefined): void { if (!child) return; @@ -5898,6 +5904,15 @@ export class AutomationEngine implements IAutomationService { // its own run row, and the question this feeds — "is the // parent's `acted` complete?" — is boolean either way. ...(prior.unmeasuredEffect || child.unmeasured ? { unmeasuredEffect: true } : {}), + // #15617 — the pausing item's own contained failures, which + // the synchronous path reports through the executor's + // `metrics`. Written only when one of the two sides actually + // tracked a count: an absent `failures` means "not tracked", + // and a `0` written here would claim a measurement of a + // child recorded before the count existed. + ...(prior.failures !== undefined || child.failed !== undefined + ? { failures: (prior.failures ?? 0) + (child.failed ?? 0) } + : {}), }, }; return; diff --git a/packages/services/service-automation/src/run-summary.ts b/packages/services/service-automation/src/run-summary.ts index aea3f2b90fd..df3519278f2 100644 --- a/packages/services/service-automation/src/run-summary.ts +++ b/packages/services/service-automation/src/run-summary.ts @@ -47,6 +47,17 @@ import type { StepLogEntry } from './engine.js'; * including through its subflows — otherwise a sweep that delegates its writes * would report `acted: 0` and trip the very detector this exists to feed. * + * #15617 puts the failure count on that same footing, each total on its own + * rule: `selected` / `acted` ride up from a completed and a failed child alike, + * `unmeasured` as one per-execution flag, and `failures` — the contained + * failures of a child that COMPLETED and went on — through + * `metrics.failures`. A child that FAILED is the delegating step's own failure, + * counted once through `nodes[].failures` as it always was; nothing of its own + * `failed` rides up, which is the one place the rule parts from `acted`'s. + * Until that slot existed a parent whose child lost a row read `failed: 0`, + * which is the misreading the run-level count was added to prevent, one level + * up. Node `status` is untouched by the roll-up — see the fold below. + * * #14456 adds the run-level `failed` counter — `Σ nodes[].failures` — which is * the count a GREEN run hides. `loop { body: [ try_catch { try, catch } ] }` is * the containment spelling for a per-iteration failure that must not end the @@ -68,6 +79,12 @@ export function summarizeRun(steps: readonly StepLogEntry[]): FlowRunSummary { let acted = 0; let skipped = 0; let unmeasured = 0; + // #15617 — what a DELEGATING execution rolled up from a child run that + // COMPLETED while containing failures. Held apart from `node.failures` + // until the status verdict below is taken, because the two answer + // different questions: `FlowRunNodeSummary.status` is judged on this + // node's OWN executions, while its `failures` publishes own + rolled-up. + const rolledUp = new Map(); for (const step of steps) { let node = nodes.get(step.nodeId); @@ -130,6 +147,13 @@ export function summarizeRun(steps: readonly StepLogEntry[]): FlowRunSummary { node.unmeasured = (node.unmeasured ?? 0) + 1; unmeasured += 1; } + if (metrics?.failures !== undefined) { + // A `subflow` / `map` step whose child COMPLETED while containing + // failures (#15617). Summed like `selected` / `acted` — a `map` + // re-entering once per item reports each entry's own share on its + // own step, so the fold adds them rather than replacing. + rolledUp.set(step.nodeId, (rolledUp.get(step.nodeId) ?? 0) + metrics.failures); + } } // #14456 — `failed = Σ nodes[].failures`, stated as a fold over the SAME @@ -140,7 +164,18 @@ export function summarizeRun(steps: readonly StepLogEntry[]): FlowRunSummary { // Worst outcome wins: one failed iteration makes the node's run-level // status `failure`, and `runs`/`failures` carry the nuance. A node that // only ever got skipped never ran at all. + // + // ⚠️ ORDER IS LOAD-BEARING (#15617): the verdict is taken while + // `node.failures` still holds this node's OWN failed executions only. + // `FlowRunNodeSummary.status` declares exactly that — "a delegating + // node whose child completed while containing failures reads `success` + // here with `failures > 0`" — so a `subflow` step that ran fine and + // delegated to a child that lost a row must not be recoloured + // `failure`. Add the roll-up after, never before. node.status = node.failures > 0 ? 'failure' : node.runs > 0 ? 'success' : 'skipped'; + // On a delegating node this may now exceed `runs`, which the field + // declares: it is no longer only this node's own failed executions. + node.failures += rolledUp.get(node.nodeId) ?? 0; failed += node.failures; } @@ -201,17 +236,19 @@ export function formatRunSummaryLine( // asked at all — a completed run says nothing about the rows it lost — so // the token has to be there to be read. // - // What `failed=0` says, exactly: NO NODE EXECUTION OF THIS RUN FAILED. - // That is narrower than "nothing failed", and the difference is a - // `subflow`: the fold this prints is `Sigma nodes[].failures` over THIS - // run's own nodes, so a child run that CONTAINED failures of its own - // reports them on the child's summary and the parent still prints - // `failed=0` — measured, alongside the control where a child that FAILS - // rather than contains does reach the parent's count through the - // `subflow` node's own failure step. `acted` rolls a child's totals up - // and this does not; the declaration says both things in two paragraphs - // and is being reconciled in #15617. Until it is, this line is the node - // fold, and only that. + // What `failed=0` says, exactly: NOTHING THIS RUN CAUSED FAILED, + // subflows included. #15617 reconciled the two paragraphs that used to + // disagree here, and this line was narrowed to "no node execution OF THIS + // RUN failed" only while they did. The fold this prints is + // `Sigma nodes[].failures`, and a delegating node's `failures` now carries + // what a `subflow` child — or a `map` item — CONTAINED while completing, + // rolled up through `metrics.failures` the way `acted` already rode up. So + // a parent whose child lost a row prints the loss instead of `failed=0`. + // + // The one boundary the roll-up does not cross, unchanged and measured as + // the control: a child that FAILED rather than contained is the delegating + // step's OWN failure, counted once through that node's failure step, and + // its own `failed` stays on the child's run row. // // A line with no token at all is a different reading again — the older // "not tracked". A run summarized by `summarizeRun` always carries the From e13fd0c594c494625c226a4baa772f92cce8e128 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 04:14:06 +0000 Subject: [PATCH 2/3] test(service-automation): drive #15617's measured target and its control for the contained-failure rollup Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude --- .../builtin/contained-failure-rollup.test.ts | 515 ++++++++++++++++++ 1 file changed, 515 insertions(+) create mode 100644 packages/services/service-automation/src/builtin/contained-failure-rollup.test.ts diff --git a/packages/services/service-automation/src/builtin/contained-failure-rollup.test.ts b/packages/services/service-automation/src/builtin/contained-failure-rollup.test.ts new file mode 100644 index 00000000000..0cead3d785e --- /dev/null +++ b/packages/services/service-automation/src/builtin/contained-failure-rollup.test.ts @@ -0,0 +1,515 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #16314 — the services half of #15617's ruling (maintainer 「同意」 on option 1). +// +// `ExecutionStepMetrics.failures` is the slot the spec half landed: a node that +// DELEGATES to a child run — a `subflow`, or each item of a `map` — reports the +// failures that child CONTAINED, and the fold `failed = Σ nodes[].failures` +// therefore answers "what did this run cause", subflows included. Before it, a +// parent whose child lost a row read `failed: 0` while `acted` had rolled up +// all along — the misreading the run-level count exists to prevent (#13681), +// one level up. +// +// The measured target these tests drive is the card's, from #15617: +// +// parent `loop { subflow(child) }`, one child failing per five records +// → parent status=completed selected=5 acted=4 skipped=0 failed=0 (before) +// → parent ... failed=1 (ruled) +// → the five child summaries carry failed = [0,0,0,0,1] (unchanged) +// +// and its CONTROL, which must keep answering exactly as it does today: a child +// that FAILS rather than contains is the delegating step's OWN failure, counted +// once through `nodes[].failures` — `call: {runs: 5, failures: 1}`, parent +// `failed = 1` — with nothing of the child's own `failed` riding up. That is the +// one place this rule parts from `acted`'s, which carries a failed child's +// writes, and the asymmetry is the easiest thing to get backwards. + +import { describe, it, expect } from 'vitest'; +import { AutomationEngine } from '../engine.js'; +import type { NodeExecutor, StepLogEntry } from '../engine.js'; +import type { AutomationContext } from '@objectstack/spec/contracts'; +import type { FlowRunSummary } from '@objectstack/spec/automation'; +import { FlowRunSummarySchema } from '@objectstack/spec/automation'; +import { InMemorySuspendedRunStore } from '../suspended-run-store.js'; +import { registerLoopNode } from './loop-node.js'; +import { registerTryCatchNode } from './try-catch-node.js'; +import { registerLogicNodes } from './logic-nodes.js'; +import { registerSubflowNode } from './subflow-node.js'; +import { registerMapNode } from './map-node.js'; +import { summarizeRun } from '../run-summary.js'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; + +/** + * `resumeAuthority: 'any'` is what a pausing fixture has had to declare since + * #5561 — these tests continue their pause through the public `resume` door. + * Nothing here is about the resume gate; the fixture states the posture it + * relies on, the same declaration the pausing built-ins carry. + */ +const HOLD_DESCRIPTOR = defineActionDescriptor({ + type: 'hold', version: '1.0.0', name: 'Hold', + supportsPause: true, resumeAuthority: 'any', +}); + +function silentLogger(): any { + const l: any = { info() {}, warn() {}, error() {}, debug() {} }; + l.child = () => l; + return l; +} +const pluginCtx = (logger: any) => ({ logger, getService() { return undefined; } }) as any; + +/** The five rows of the measurement; the THIRD is the ownerless one. */ +const ROWS = ['c1', 'c2', 'c3', 'c4', 'c5']; +const OWNERLESS = 2; + +const AT = '2026-09-15T00:00:00.000Z'; +const step = (over: Partial & { nodeId: string }): StepLogEntry => ({ + nodeType: 'noop', + status: 'success', + startedAt: AT, + ...over, +}); + +interface Harness { + engine: AutomationEngine; + /** Every row the per-row work node was handed, in order. */ + ran: string[]; +} + +/** + * The work node fails on the OWNERLESS row and only on it. Driven by call + * order rather than by a param, because `loop`, `map` and `subflow` are all + * sequential here — one counter reproduces "one row in five" without plumbing + * a record through two variable scopes, which is not what is under test. + */ +function harness(): Harness { + const logger = silentLogger(); + const engine = new AutomationEngine(logger, new InMemorySuspendedRunStore()); + const ctx = pluginCtx(logger); + registerLoopNode(engine, ctx); + registerTryCatchNode(engine, ctx); + registerLogicNodes(engine, ctx); + registerSubflowNode(engine, ctx); + registerMapNode(engine, ctx); + + const ran: string[] = []; + + // Stands in for the sweep query — seeds the collection and reports + // `selected` the way a real `get_record` node does (#4354). + engine.registerNodeExecutor({ + type: 'seed', + async execute(_node, variables) { + variables.set('cases', ROWS); + return { success: true, metrics: { selected: ROWS.length } }; + }, + } as NodeExecutor); + + engine.registerNodeExecutor({ + type: 'work', + async execute() { + const n = ran.length; + ran.push(ROWS[n] ?? `extra_${n}`); + if (n === OWNERLESS) { + return { success: false, error: `notify: at least one recipient is required (${ROWS[n]})` }; + } + return { success: true, metrics: { acted: 1 } }; + }, + } as NodeExecutor); + + // A child that CONTAINS its failure: the work node throws inside a + // `try_catch`, the handler runs, the child run completes. + engine.registerFlow('contained_child', { + name: 'contained_child', label: 'contained_child', type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'guard', type: 'try_catch', label: 'Guarded', + config: { + try: { nodes: [{ id: 'work', type: 'work', label: 'Work' }], edges: [] }, + catch: { nodes: [{ id: 'handled', type: 'assignment', label: 'Handled' }], edges: [] }, + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'guard' }, + { id: 'e2', source: 'guard', target: 'end' }, + ], + } as never); + + // Always fails — the single-row spelling, for the durable-pause case where + // "one row in five" would only add noise. + engine.registerNodeExecutor({ + type: 'boom', + async execute() { + return { success: false, error: 'notify: at least one recipient is required (paused row)' }; + }, + } as NodeExecutor); + + engine.registerNodeExecutor({ + type: 'hold', + descriptor: HOLD_DESCRIPTOR, + async execute() { return { success: true, suspend: true, correlation: 'held' }; }, + } as NodeExecutor); + + // A child that PAUSES first and contains its failure only after the resume. + // Its parent's step was written at suspend time, before the child had done + // anything, so the count reaches the parent through the engine's + // `creditChildRun` seam rather than through the executor's `metrics`. + engine.registerFlow('paused_contained_child', { + name: 'paused_contained_child', label: 'paused_contained_child', type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'hold', type: 'hold', label: 'Hold' }, + { + id: 'guard', type: 'try_catch', label: 'Guarded', + config: { + try: { nodes: [{ id: 'boom', type: 'boom', label: 'Boom' }], edges: [] }, + catch: { nodes: [{ id: 'handled', type: 'assignment', label: 'Handled' }], edges: [] }, + }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'hold' }, + { id: 'e2', source: 'hold', target: 'guard' }, + { id: 'e3', source: 'guard', target: 'end' }, + ], + } as never); + + // The control's child: the same work node with NO containment, so the + // child run itself FAILS. + engine.registerFlow('failing_child', { + name: 'failing_child', label: 'failing_child', type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'work', type: 'work', label: 'Work' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'work' }, + { id: 'e2', source: 'work', target: 'end' }, + ], + } as never); + + return { engine, ran }; +} + +/** + * `loop { subflow() }` over the five rows — the card's shape. + * + * `guardBody` wraps the `subflow` call in the parent's own `try_catch`, which + * is what the control needs and MEASURED rather than assumed: a `loop` body is + * fail-fast, so an unguarded call to a child that FAILS ends the loop at the + * third row (`call: {runs: 3}`) and the loop node records a failure of its own + * beside the call's, making `failed = 2`. The control's declared numbers — + * `call: {runs: 5, failures: 1}`, parent `failed = 1` — are the CONTAINED + * parent, which is the shape the contained-child case uses too, so the two read + * against one fixture and differ only in which level contains. + */ +function registerLoopParent( + engine: AutomationEngine, + name: string, + childFlow: string, + guardBody = false, +): void { + const call = { id: 'call', type: 'subflow', label: 'Call', config: { flowName: childFlow } }; + const body = guardBody + ? { + nodes: [{ + id: 'guard_parent', type: 'try_catch', label: 'Guarded', + config: { + try: { nodes: [call], edges: [] }, + catch: { nodes: [{ id: 'handled_parent', type: 'assignment', label: 'Handled' }], edges: [] }, + }, + }], + edges: [], + } + : { nodes: [call], edges: [] }; + + engine.registerFlow(name, { + name, label: name, type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'query', type: 'seed', label: 'Query' }, + { + id: 'each', type: 'loop', label: 'Each case', + config: { collection: '{cases}', iteratorVariable: 'currentCase', body }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'query' }, + { id: 'e2', source: 'query', target: 'each' }, + { id: 'e3', source: 'each', target: 'end' }, + ], + } as never); +} + +/** `map` over the five rows, one child run per item. */ +function registerMapParent(engine: AutomationEngine, name: string, childFlow: string): void { + engine.registerFlow(name, { + name, label: name, type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'query', type: 'seed', label: 'Query' }, + { + id: 'each', type: 'map', label: 'Each case', + config: { collection: '{cases}', flowName: childFlow, iteratorVariable: 'currentCase' }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'query' }, + { id: 'e2', source: 'query', target: 'each' }, + { id: 'e3', source: 'each', target: 'end' }, + ], + } as never); +} + +const nodeOf = (summary: FlowRunSummary, nodeId: string) => + summary.nodes.find((n) => n.nodeId === nodeId); + +describe('#16314 — a delegating node rolls its COMPLETED child\'s contained failures up', () => { + it('the measured target: `loop { subflow }`, one child failing per five rows, answers `failed = 1`', async () => { + const { engine, ran } = harness(); + registerLoopParent(engine, 'parent', 'contained_child'); + + const res = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + const summary = res.summary as FlowRunSummary; + + // Every row ran and the run completed — containment worked, which is + // the premise the count is about. + expect(res.success).toBe(true); + expect(ran).toEqual(ROWS); + expect(summary.selected).toBe(5); + expect(summary.acted).toBe(4); + expect(summary.skipped).toBe(0); + + // The half that did not exist: the parent answers for what its children + // lost. `0` here was the card's measurement. + expect(summary.failed).toBe(1); + // …and it still agrees with the breakdown it is declared to fold. + expect(summary.failed).toBe(summary.nodes.reduce((n, node) => n + node.failures, 0)); + }); + + it('the delegating node reads `success` with `failures > 0` — the verdict is its OWN executions', async () => { + const { engine } = harness(); + registerLoopParent(engine, 'parent', 'contained_child'); + + const res = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + const call = nodeOf(res.summary as FlowRunSummary, 'call'); + + // `FlowRunNodeSummary.status` is declared judged on the node's own + // executions: this `subflow` step ran five times and never failed, so + // colouring it `failure` because a child lost a row would be a second, + // contradictory answer to a question the schema already settles. + expect(call).toMatchObject({ status: 'success', runs: 5, failures: 1 }); + }); + + it('the CHILD rows are untouched — `failed = [0,0,0,0,1]` stays on them', async () => { + const { engine } = harness(); + registerLoopParent(engine, 'parent', 'contained_child'); + + await engine.execute('parent', { event: 'schedule' } as AutomationContext); + const children = await engine.listRuns('contained_child'); + + // Oldest-first, so the ownerless row is the third child. + const failedByChild = [...children] + .sort((a, b) => String(a.startedAt).localeCompare(String(b.startedAt))) + .map((r) => r.summary?.failed); + expect(failedByChild).toEqual([0, 0, 1, 0, 0]); + // The child keeps its own run row; the roll-up does not move the count, + // it publishes it a second time under the question the parent answers. + expect(failedByChild.reduce((a, b) => (a ?? 0) + (b ?? 0), 0)).toBe(1); + }); + + it('the run summary still satisfies the published contract', async () => { + const { engine } = harness(); + registerLoopParent(engine, 'parent', 'contained_child'); + + const res = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + + // A full parse, not an unrecognized-keys check: what moved is a VALUE + // (`failures` may now exceed `runs` on a delegating node), so the whole + // judgement has to stay green. + const parsed = FlowRunSummarySchema.safeParse(res.summary); + expect(parsed.success).toBe(true); + }); + + it('`map` rolls its per-item children up the same way — measured, not assumed', async () => { + const { engine, ran } = harness(); + registerMapParent(engine, 'sweep', 'contained_child'); + + const res = await engine.execute('sweep', { event: 'schedule' } as AutomationContext); + const summary = res.summary as FlowRunSummary; + + expect(res.success).toBe(true); + expect(ran).toEqual(ROWS); + // `map` does NOT share `subflow`'s roll-up path — it accumulates its + // items' totals itself — so this is a second implementation of the same + // rule and needs its own pin. + expect(summary.failed).toBe(1); + expect(nodeOf(summary, 'each')).toMatchObject({ status: 'success', runs: 1, failures: 1 }); + expect(summary.acted).toBe(4); + }); +}); + +describe('#16314 — the control: a child that FAILED is the delegating step\'s own failure, counted once', () => { + it('`loop { subflow }` over a failing child keeps answering exactly as before', async () => { + const { engine } = harness(); + registerLoopParent(engine, 'parent', 'failing_child', true); + + const res = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + const summary = res.summary as FlowRunSummary; + const call = nodeOf(summary, 'call'); + + // The card's control numbers, verbatim: the subflow node's own failure + // is what counts, once. + expect(call).toMatchObject({ runs: 5, failures: 1, status: 'failure' }); + expect(summary.failed).toBe(1); + }); + + it('nothing of the failed child\'s own `failed` rides up — one loss is never counted twice', async () => { + const { engine } = harness(); + registerLoopParent(engine, 'parent', 'failing_child', true); + + const res = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + const summary = res.summary as FlowRunSummary; + const children = await engine.listRuns('failing_child'); + const failedChild = children.find((r) => r.status === 'failed'); + + // The failed child's run row carries its own failure… + expect(failedChild?.summary?.failed).toBeGreaterThanOrEqual(1); + // …and the parent counts ONE, not that number plus its own step. The + // symmetric-looking implementation (roll every child's `failed` up) + // fails here and nowhere else. + expect(summary.failed).toBe(1); + expect(nodeOf(summary, 'call')?.failures).toBe(1); + }); + + it('a failed child\'s WRITES still ride up, which is where the two rules part', async () => { + const { engine } = harness(); + registerLoopParent(engine, 'parent', 'failing_child', true); + + const res = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + + // `acted` is declared to carry a failed child's rows — it wrote them. + // Asserted beside the `failures` control so a future edit cannot make + // the two rules identical in either direction without turning one of + // these two tests red. + expect((res.summary as FlowRunSummary).acted).toBeGreaterThan(0); + }); + + it('`map` on a failing item does not roll that item\'s contained failures up either', async () => { + const { engine } = harness(); + registerMapParent(engine, 'sweep', 'failing_child'); + + const res = await engine.execute('sweep', { event: 'schedule' } as AutomationContext); + const summary = res.summary as FlowRunSummary; + + // v1 `map` is fail-fast: the failing item fails the map, and the map + // step's own failure is the one count. The items before it keep their + // (zero) contained failures. + expect(nodeOf(summary, 'each')).toMatchObject({ failures: 1, status: 'failure' }); + expect(summary.failed).toBe(1); + }); +}); + +describe('#16314 — a child that PAUSED and then contained a failure is credited too', () => { + /** `subflow` straight off `start`, so the parent's only step is the call. */ + function pausingParent(engine: AutomationEngine): void { + engine.registerFlow('parent', { + name: 'parent', label: 'parent', type: 'autolaunched', runAs: 'system', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'call', type: 'subflow', label: 'Call', config: { flowName: 'paused_contained_child' } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'call' }, + { id: 'e2', source: 'call', target: 'end' }, + ], + } as never); + } + + it('credits it when the CHILD is resumed and bubbles up', async () => { + const { engine } = harness(); + pausingParent(engine); + + const paused = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + expect(paused.status).toBe('paused'); + + // What an approval service / wait timer does: it holds the child's id. + const [childRun] = await engine.listRuns('paused_contained_child', { limit: 1 }); + const done = await engine.resume(childRun.id); + expect(done.success).toBe(true); + + const parent = await engine.getRun(paused.runId!); + expect(parent!.status).toBe('completed'); + // The parent's step for this node was written at SUSPEND time, before + // the child had run anything, so this number can only arrive through + // the engine's own credit seam — the executor's `metrics` never see it. + expect(parent!.summary!.failed).toBe(1); + expect(nodeOf(parent!.summary!, 'call')).toMatchObject({ status: 'success', failures: 1 }); + }); + + it('credits it when the PARENT is resumed and delegates down', async () => { + const { engine } = harness(); + pausingParent(engine); + + const paused = await engine.execute('parent', { event: 'schedule' } as AutomationContext); + expect(paused.status).toBe('paused'); + + // What a UI holding the launch run id does. + const done = await engine.resume(paused.runId!); + expect(done.success).toBe(true); + expect((done.summary as FlowRunSummary).failed).toBe(1); + }); +}); + +// ── The fold itself, without an engine ────────────────────────────────────── + +describe('#16314 — `summarizeRun` folds `metrics.failures` without recolouring the node', () => { + it('adds the roll-up to the node\'s own failures and so to the run-level `failed`', () => { + const s = summarizeRun([ + step({ nodeId: 'call', nodeType: 'subflow', metrics: { acted: 4, failures: 1 } }), + ]); + expect(s.failed).toBe(1); + expect(s.nodes[0]).toMatchObject({ nodeId: 'call', runs: 1, failures: 1, status: 'success' }); + }); + + it('sums across executions — a `map` re-entering per item reports its own share per step', () => { + const s = summarizeRun([ + step({ nodeId: 'each', nodeType: 'map', metrics: { failures: 1 } }), + step({ nodeId: 'each', nodeType: 'map', metrics: { failures: 2 } }), + ]); + expect(s.nodes[0]).toMatchObject({ runs: 2, failures: 3, status: 'success' }); + expect(s.failed).toBe(3); + }); + + it('a node with its OWN failure keeps `status: failure`, and the two counts add', () => { + const s = summarizeRun([ + step({ nodeId: 'call', nodeType: 'subflow', status: 'failure' }), + step({ nodeId: 'call', nodeType: 'subflow', metrics: { failures: 2 } }), + ]); + expect(s.nodes[0]).toMatchObject({ runs: 2, failures: 3, status: 'failure' }); + expect(s.failed).toBe(3); + }); + + it('an absent `failures` adds nothing — absent is "not tracked", never zero', () => { + const s = summarizeRun([ + step({ nodeId: 'call', nodeType: 'subflow', metrics: { acted: 1 } }), + ]); + expect(s.failed).toBe(0); + expect(s.nodes[0]).toMatchObject({ failures: 0, status: 'success' }); + }); + + it('`failures` may exceed `runs` on a delegating node, as the field declares', () => { + const s = summarizeRun([ + step({ nodeId: 'call', nodeType: 'subflow', metrics: { failures: 7 } }), + ]); + const node = s.nodes[0]; + expect(node.failures).toBeGreaterThan(node.runs); + expect(node.status).toBe('success'); + }); +}); From 6938723d7d5684419bf09bfb7a9750e1c356a20a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 04:18:17 +0000 Subject: [PATCH 3/3] chore(changeset): contained-failure rollup on the delegating node's failure count Claude-Session: https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj Co-authored-by: Claude --- ...-contained-failure-rollup-services-half.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .changeset/16314-contained-failure-rollup-services-half.md diff --git a/.changeset/16314-contained-failure-rollup-services-half.md b/.changeset/16314-contained-failure-rollup-services-half.md new file mode 100644 index 00000000000..5895d1b2ffd --- /dev/null +++ b/.changeset/16314-contained-failure-rollup-services-half.md @@ -0,0 +1,28 @@ +--- +'@objectstack/service-automation': patch +--- + +fix(service-automation): a delegating node rolls its COMPLETED child's contained failures into the run-level `failed` (#16314) + +The services half of #15617's ruling (maintainer 「同意」 on option 1, decision batch #55). The spec half landed the slot: `ExecutionStepMetrics.failures`, declared as *"node executions that failed inside a child run this execution delegated to and went on from"*, folding into `nodes[].failures` and so into `FlowRunSummary.failed`. Until this, nothing populated it — the engine's fold could not see a child's losses, so a parent that delegated its rows reported `failed: 0` while its children lost them. `acted` had rolled up since #4354; the failure count had not, and the two paragraphs of the declaration disagreed for exactly that shape. + +**What moves on the wire.** For a run whose `subflow` or `map` child COMPLETED while containing failures, the delegating node's `nodes[].failures` and the run-level `failed` grow by the child's own `failed` — and the summary line prints it. The measured target from #15617, driven on the real engine: + +``` +parent loop { subflow(child) }, one child failing per five rows + before status=completed selected=5 acted=4 skipped=0 failed=0 + after status=completed selected=5 acted=4 skipped=0 failed=1 + children failed = [0, 0, 1, 0, 0] (unchanged — the child keeps its own row) +``` + +**The boundary, unchanged and pinned as the control.** A child that **failed** rather than contained is the delegating step's own failure, counted once through `nodes[].failures` exactly as it always was: `call: {runs: 5, failures: 1}`, parent `failed = 1`, with nothing of the child's own `failed` riding up. That is the one place this rule parts from `acted`'s, which does carry a failed child's writes. Implementing the symmetric-looking version would count one loss twice, and the control test is red on it. + +**A delegating node's `status` is unaffected.** `FlowRunNodeSummary.status` is declared judged on the node's OWN executions, so a `subflow` step that ran fine and rolled a child's losses up reads `success` with `failures > 0` — and on such a node `failures` may exceed `runs`, as the field declares. The fold takes the status verdict before it adds the roll-up. + +Three producers, each measured rather than assumed: `subflow-node.ts` (synchronous child), `map-node.ts` (per-item children — it does **not** share `subflow`'s roll-up path and needed its own), and `AutomationEngine.creditChildRun` (a child that PAUSED, whose parent step was written at suspend time; both the child-resume up-bubble and the parent-resume down-delegation are completion paths, which is what puts them inside the declared rule). + +`failed` keeps its convention: absent is "not tracked", never zero — an absent `metrics.failures` means the execution delegated nothing or the child tracked no count, and nothing writes a `0` that would claim a measurement. + +PR #15609's narrowed wording — *"no node execution **of this run** failed"* — was true only while the paragraphs disagreed, and is widened back here in the summary-line comment and in `content/docs/automation/flows.mdx`: `failed=0` now reads *"nothing this run caused failed, subflows included"*. + +No API moves: no new export, no new key on any published payload, and the node executors' `NodeExecutionResult.metrics` shape is the spec's already-published one.