Skip to content

Commit b722547

Browse files
claude[bot]claude
andauthored
fix(service-automation): subflow and map roll a COMPLETED child's contained failures into the run-level failed (#16314) (#18261)
Fixes #16314 Clause-②: no Services half of #15617's ruling (director seat, decision batch #55, maintainer 「同意」 on option 1, 2026-09-06). The spec half landed the slot on `68d5dfd0f`; this populates it. ## What moved `ExecutionStepMetrics.failures` is 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 the run-level `failed`. Nothing wrote it, so the engine's fold could not see a child's losses: a parent that delegated its rows reported `failed: 0` while `acted` had rolled up since #4354. Four producers, each read rather than assumed: | site | what it now reports | | :--- | :--- | | `builtin/subflow-node.ts` | a synchronous child's `summary.failed`, on the **non-failed** exit only | | `builtin/map-node.ts` | the accumulated `failed` of the items that COMPLETED during this entry — `map` does **not** share `subflow`'s roll-up path, so it needed its own | | `engine.ts` · `creditChildRun` | a child that PAUSED, whose parent step was written at suspend time; both call sites are completion paths, which is what puts them inside the declared rule | | `run-summary.ts` · `summarizeRun` | folds `metrics.failures` into `nodes[].failures` and so into `failed` | ## The measured target, driven on the real engine The card's shape from #15617 — 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 control, unchanged and pinned.** A child that **failed** rather than contained is the delegating step's own failure, counted once: `call: {runs: 5, failures: 1}`, parent `failed = 1`, with nothing of the child's own `failed` riding up. That is the one place the rule parts from `acted`'s, which does carry a failed child's writes — asserted beside it so neither direction can be made symmetric without a red test. ⚠️ One correction to the control's fixture, measured rather than assumed: a `loop` body is fail-fast, so an **unguarded** call to a failing child ends the loop at the third row (`call: {runs: 3}`) and the loop node records a failure of its own beside the call's, giving `failed = 2`. The control's declared numbers are the parent that CONTAINS — the same shape the contained-child case uses, differing only in which level contains. Both readings are in the test's own comments. **A delegating node's `status` does not move.** `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`. The fold takes the status verdict **before** it adds the roll-up; that ordering is load-bearing and has its own ablation leg below. PR #15609's narrowed wording — *"no node execution **of this run** failed"* — was true only while the declaration's two paragraphs disagreed. It is widened back here in `formatRunSummaryLine`'s comment and in `content/docs/automation/flows.mdx`. ## ⚠️ Fence — #18110 is NOT folded in, and the two are separable #18110 is a separate, still-ungraded card on the same file, in the opposite direction: a **`refused`** child (the run-OUTCOME sense — an `end` node saying no) rolled up as an ordinary success, the refusal reaching nobody. They are **not** mechanically inseparable, and the reason is mechanical rather than a judgement call: `selected` / `acted` / `unmeasuredEffect` already ride the exact exit a refused child takes today, and `failures` was added to that **same** exit. So this diff adds one total to an existing roll-up and decides nothing new about which child outcomes reach it. Nothing here pins the refused case in either direction — deliberately, so #18110's fix stays free to decide what a parent does with a refused child's totals, all four at once. ## Reverse verification — six ablation legs Each leg mutates one COMMITTED source file, proves the mutation reached disk by occurrence counts on the anchored text **plus** a blob-hash inequality against `HEAD` (never an editor's exit code), runs the suite, restores with `git checkout HEAD -- path` and proves restoration by blob hash equality with `git diff HEAD` empty. A `trap` on EXIT/INT/TERM restores every touched path by absolute path. Baseline and post-restore runs are both 16/16 green. | leg | mutation | result | | :--- | :--- | :--- | | A | the fold stops adding the roll-up | 9 failed / 7 passed | | B | roll-up added **before** the status verdict | 6 failed / 10 passed — incl. "reads `success` with `failures > 0`" and "`failures` may exceed `runs`" | | C | `subflow` stops rolling a completed child up | 2 failed / 14 passed — the measured target and the node status | | D | the asymmetry removed: a FAILED child's `failed` rides up too | 2 failed / 14 passed — exactly the two control tests | | E | `map` stops accumulating its completed items' failures | 1 failed / 15 passed — the `map` test | | F | `creditChildRun` stops crediting a PAUSED child | 2 failed / 14 passed — both resume directions | A first pass declared B/D/E VOID rather than green: its landing proof used `grep -cF` on a multi-line needle, which splits into per-line patterns and counts lines. The legs were re-run with a python-side occurrence count, and the void reading is reported rather than quietly replaced. No permanent ablation artefact is left behind; the worktree is clean and every restored blob matches `HEAD`. ## Verification - `pnpm --filter @objectstack/service-automation test` — **136 files / 1615 tests passed**. - `pnpm --filter @objectstack/service-automation typecheck` — clean (`tsc --noEmit` + `check:test-typecheck`, 0 files / 0 errors in the test-layer ledger). - **Derived gate families: 92 derived / 92 run / 0 NOT MEASURED / 0 UNRUN.** Derived by `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` from the merge-base change set, reconciled back with `--ran` carrying each command's own exit code captured before any pipe: *"92 derived famil(ies) accounted for — 92 run, 0 NOT-MEASURED (a DERIVED zero — all 92 recorded an exit code and none of them is 3)."* Three of them first answered **PREREQUISITE NOT MET** (`check:skill-examples`, `check:dual-build-cjs-loads` exit 3, `check:type-check-debt` exit 3) — ⛔ not passes; the workspace build closure was built and all three then exited 0. - `eslint . --no-inline-config` over the **whole** repo population — **6764 files, 0 errors, 0 warnings**. Not a narrowed run, so no narrowing needs declaring. - Control-character self-scan over the diff's seven files: no match (`check:nul-bytes` is also in the 92). - Not measured here, declared to CI: the required contexts on the merge group's affected set. ## Clause-②: no — derived, not predicted Derived by **reachability from the published entry**, with controls, ⛔ never from the word `export` and ⛔ never from a grep in `dist/index.js`. The package's `exports` declares one entry (`.` → `dist/index.js` / `dist/index.d.ts`) and `files` ships `dist`; the named-export set of that entry was read from the published type surface and cross-checked against the barrel chain in `src/index.ts`. - **Positive controls** — `summarizeRun`, `formatRunSummaryLine`, `AutomationEngine`, `registerLogicNodes`, `installBuiltinNodes`: REACHABLE, so the derivation can answer yes. - **Negative controls** — `registerSubflowNode` and `registerMapNode` are `export function` in their own modules and re-exported by `src/builtin/index.ts`, yet the root barrel's explicit list names six builtins and not those two ⇒ **unreachable by name**; `creditChildRun` (a `private` member) and a nonexistent identifier: also unreachable. So the derivation can answer no, and it is not a grep for `export`. - The delivered diff adds **zero** export statements outside the test file, and **zero** keys to any published payload — `metrics.failures`, `nodes[].failures` and `failed` are all keys `packages/spec` already declares. ⚠️ Declared honestly: what moves is the **value** on an already-published payload — a delegating parent's `failed` changes from "this run's own node failures" to "what this run caused". That is conformance to a declaration already on `main`, not a widened surface or a relaxed acceptance set. ## Acceptance notes - ⛔ `packages/spec` untouched — the contract said everything needed, including the failed-child boundary and the `status`-judged-on-own-executions rule, both of which this implements verbatim. - The card's re-check grep names `packages/services/service-automation/src/subflow-node.ts`; the file lives at `src/builtin/subflow-node.ts`. Path drift only — the premise holds. Noted, not filed. - `loop { subflow(failing child) }` unguarded answers `failed = 2` for one lost row: the loop node's own failed execution plus the subflow step's. Literally correct under *"total node executions that failed"*, and unchanged by this diff. Noted, not filed. - The refused-child roll-up is #18110's, left exactly as found. Noted, not filed here. Authored by Claude Code in session `session_01URLHobLUJB9K1ABV6ofdjj`. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7851fa3 commit b722547

7 files changed

Lines changed: 685 additions & 26 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation): a delegating node rolls its COMPLETED child's contained failures into the run-level `failed` (#16314)
6+
7+
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.
8+
9+
**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:
10+
11+
```
12+
parent loop { subflow(child) }, one child failing per five rows
13+
before status=completed selected=5 acted=4 skipped=0 failed=0
14+
after status=completed selected=5 acted=4 skipped=0 failed=1
15+
children failed = [0, 0, 1, 0, 0] (unchanged — the child keeps its own row)
16+
```
17+
18+
**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.
19+
20+
**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.
21+
22+
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).
23+
24+
`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.
25+
26+
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"*.
27+
28+
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.

content/docs/automation/flows.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,8 +1124,8 @@ run in `listRuns` / `getRun`, and in the log:
11241124
| `acted` | Records **created / updated / deleted**, plus effects dispatched (notifications delivered) |
11251125
| `skipped` | Node executions a **closed gate** prevented — one per loop iteration whose conditional edge evaluated false |
11261126
| `unmeasured` | Executions that reached something the platform **cannot count** — see below |
1127-
| `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 |
1128-
| `nodes[]` | Per-node terminal status with `runs` / `failures` / `skipped` and its own selected/acted |
1127+
| `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 |
1128+
| `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` |
11291129
| `gates[]` | Which gates closed and how often, most-skipped first |
11301130

11311131
The counts come from the node executors themselves — `get_record` reports what

0 commit comments

Comments
 (0)