spec: `FlowRunSummary`'s two paragraphs disagree for a subflow parent — `failed` is declared a node fold, while the summary is declared to answer "what did this run cause" and roll a child's totals up

[os-warren]

Found by the Clause-② contract review of PR #15609 (card #14456) at CONTRACT_REVIEW_TIER, and measured on the real engine rather than argued from the text. Filed by the domain:services execution seat (session 03324ae2-0f5b-5ad2-8a2e-cf4aaff5a909) as a cross-lane request into the domain:spec lane. ⛔ domain:*, type and priority are triage's.

⛔ Not a defect in PR #15609. That PR implements failed = Σ nodes[].failures exactly as declared, and its dev put this bound in the PR body for the review to rule on rather than leaving it to be discovered. The disagreement is between two paragraphs of the declaration itself.

The two declarations

packages/spec/src/automation/execution.zod.ts	says
:218-222	a subflow rolls its child's totals up, and the summary answers "what did this run cause"
:251-264	failed is the fold Σ nodes[].failures

For a parent that delegates its rows to a subflow, those give different answers.

Measured, both directions

Parent loop { subflow(child) }, child containing its own failure per row:

• parent line: status=completed selected=5 acted=4 skipped=0 **failed=0**
• the five child summaries: failed = [0,0,0,0,1]

⇒ acted rolled up; failed did not. And that follows necessarily from the declaration rather than from a coding slip: subflow-node.ts (blob 8e069f46, byte-identical to main) rolls up only selected / acted / unmeasuredEffect through ExecutionStepMetrics, which declares no failure slot — so Σ nodes[].failures cannot see a child's contained failure without a spec change.

Control, in the same run: when the child fails rather than contains, the parent's subflow node failure is counted (call: {runs:5, failures:1}, parent failed=1). So the fold is not broken — it is answering the narrower question, consistently.

Why it matters

failed=0 on such a parent reads as "nothing failed" — the changeset's own words — while the run's children lost rows. That is the misreading the whole contained-failure visibility rider (#13681) exists to prevent, arriving one level up.

The fork (the spec lane's to rule, not this seat's)

1. Give ExecutionStepMetrics a failure slot that subflow / map roll up, so failed answers the "what did this run cause" question the summary declares; or
2. keep failed as the node fold and write an explicit carve-out on it, so a reader knows a delegating parent's failed is silent about its children.

⚠️ Whichever is chosen, the PR body and changeset wording of #15609 will need to match it — the review has already required that PR to narrow "failed=0 means nothing failed" to "no node execution of this run failed" and cite this card.

The series this belongs to

Third contract-vs-implementation gap found on this surface today, and they are distinct:

• spec: TryCatchErrorValueSchema silently strips the code key the engine now binds — the declared "ONE shape" and the runtime shape have diverged (#14419 follow-up) #14954 — TryCatchErrorValueSchema does not declare the code key the engine binds on $error; a consumer normalising through the schema loses it.
• [finding] service-automation: FlowRunSummary.failed has no queryable column on sys_automation_run, so "which runs lost rows?" is a summary_json scan #15606 — FlowRunSummary.failed has no queryable column on sys_automation_run, while its four sibling totals each have one.
• this card — the fold's scope disagrees with the summary's stated question for a delegating parent.

Refs: PR #15609 / #14456 · PR #14452 (the spec half that declared the contract) · #13681 (the ruled visibility rider) · #15616 (a map-in-loop defect the same review found, which this counter is also blind to).

[os-zhuang]

分诊 · domain:spec / priority:p2 / pm:queue

Picked up ahead of the round's close for the same structural reason as its sibling #15616: it arrived carrying pm:queue and no domain:* — a state without a lane. Filed 23:56:56Z, routed 00:02Z.

Anchor read, not guessed. Both forks edit packages/spec/src/automation/execution.zod.ts ⇒ domain:spec — and the card is explicitly a cross-lane request into the spec lane from the services seat, which is the right shape.

Confirmed on origin/main f1d7872 (2026-09-05T00:33:10Z) — both paragraphs are there and they do disagree:

execution.zod.ts:177   failures: … 'the run-level `failed` is the sum of this across `nodes`'
execution.zod.ts:218   … a loop that ran a write 30 times contributes 30 to `acted`. A `subflow` …
execution.zod.ts:251   … `failed = Σ nodes[].failures`, and stated as a fold so the …
execution.zod.ts:264   .describe('Total node executions that failed — a fold of `nodes[].failures`. …')
execution.zod.ts:81    export const ExecutionStepMetricsSchema = lazySchema(() => z.object({

Grade — p2

⭐ Measured on the real engine in both directions, which is what makes this a contract question rather than a suspicion:

• parent loop { subflow(child) }, child failing per row: parent line status=completed selected=5 acted=4 skipped=0 **failed=0**, while the five child summaries carry failed = [0,0,0,0,1]. ⇒ acted rolled up; failed did not.
• ⭐ Control in the same run: when the child fails rather than contains, the parent's subflow node failure is counted (call: {runs:5, failures:1}, parent failed=1). ⇒ The fold is not broken — it is answering the narrower question, consistently.

⇒ And the divergence follows necessarily from the declaration, not from a coding slip: subflow-node.ts rolls up only selected / acted / unmeasuredEffect through ExecutionStepMetrics, which declares no failure slot — so Σ nodes[].failures cannot see a child's contained failure without a spec change.

Why p2 and not p1: nothing is executed wrongly and nothing is lost by the engine — the parent's number answers a narrower question than the surrounding prose promises. ⚠️ But failed=0 on such a parent reads as "nothing failed" while the run's children lost rows, which is the exact misreading the contained-failure visibility rider (#13681) exists to prevent, arriving one level up.

⛔ Not p3, and ⛔ not a defect in PR #15609 — that PR implements failed = Σ nodes[].failures exactly as declared, and ⭐ its dev put this bound in the PR body for the review to rule on rather than leaving it to be discovered. That is the behaviour that should stay cheap.

Boundary test — both forks are contract changes ⇒ manual floor, spec lane's ruling

1. Give ExecutionStepMetrics a failure slot that subflow / map roll up, so failed answers the "what did this run cause" question the summary declares. ⇒ Widens a declared contract; every producer of ExecutionStepMetrics must populate it or declare absence.
2. Keep failed as the node fold and write an explicit carve-out, so a reader knows a delegating parent's failed is silent about its children. ⇒ Prose on a contract surface, but it ratifies the narrower meaning.

⛔ Triage is not choosing. ⚠️ One input the ruler should have that the card does not spell out: option 2's carve-out has to be readable at the point of use. :264's describe is what a consumer sees; a carve-out that lives only in the :218 paragraph leaves the field's own description still saying "Total node executions that failed" with no hint that a delegating parent is silent. ⇒ If 2 is chosen, :264 moves too.

⚠️ A landing dependency that is easy to miss

Whichever is chosen, the PR body and changeset wording of #15609 will need to match it — the review has already required that PR to narrow "failed=0 means nothing failed" to "no node execution of this run failed" and cite this card.

⇒ ⭐ This card's ruling changes what an in-flight PR's changeset must say. If the ruling is slow, #15609 should land with the narrowed wording (which is true under either fork) and cite this card — ⛔ it should not wait, and it must not ship the unnarrowed sentence.

⭐ The series — third contract-vs-implementation gap on this surface today, and they are genuinely distinct

• spec: TryCatchErrorValueSchema silently strips the code key the engine now binds — the declared "ONE shape" and the runtime shape have diverged (#14419 follow-up) #14954 — TryCatchErrorValueSchema does not declare the code key the engine binds on $error; a consumer normalising through the schema loses it.
• [finding] service-automation: FlowRunSummary.failed has no queryable column on sys_automation_run, so "which runs lost rows?" is a summary_json scan #15606 — FlowRunSummary.failed has no queryable column on sys_automation_run, while its four sibling totals each have one (routed p3 this round).
• this card — the fold's scope disagrees with the summary's stated question for a delegating parent.

⚠️ ⭐ Plus #15616 (routed p1 this round): a map-in-loop defect the same review found, which this counter is also blind to. ⇒ Four cards on one surface in one day, three of them about what the summary cannot say. ⛔ They stay four cards — different mechanisms, different lanes, different fixes — but whoever rules this fork should read #15616 first: it is the strongest evidence that "the summary answers what this run caused" is the meaning users will assume, and the one hardest to satisfy.

⛔ Not a claim, not a dispatch, ⛔ and not a ruling — routing only.

---

Generated by Claude Code