Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .changeset/contained-failure-visibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Four changes populate the contract `@objectstack/spec` already declares:

- **`FlowRunSummary.failed`** — `summarizeRun` now folds `failed = Σ nodes[].failures` over the per-node array it publishes, so the run-level count can never disagree with the breakdown it summarizes. It counts every node execution that failed, contained or fatal; on a run that completed, all of them were contained.
- **`failed=N` on the run summary line** — `formatRunSummaryLine` prints the token whenever the count is present, `failed=0` included. That is the opposite of the `unmeasured` rule beside it and deliberate: `unmeasured` qualifies `acted`, while `failed` answers a question a completed run's line otherwise cannot be asked at all. Read `failed=0` precisely: **no node execution of this run failed**. It is the node fold and only that, so a `subflow` child's own contained failures stay on the child's summary rather than rolling up the way `acted` does — see #15617, where the declaration's two paragraphs are being reconciled.
- **Iteration through `try_catch`** — a step that ran in a `try` or `catch` region inside a loop body now carries the enclosing loop's `iteration`, with `regionKind` still `try` / `catch`. The step says which region ran it *and* which row it ran for. `parallel` branch tagging is unchanged.
- **Iteration through `try_catch`** — a step that ran in a `try` or `catch` region inside a loop body now carries the enclosing loop's `iteration`, with `regionKind` still `try` / `catch`. The step says which region ran it *and* which row it ran for. (`parallel` branch tagging was unchanged by *this* change; the entry below retires the `iteration` overload it left standing.)
- **`$error` binds the row** — the value bound to `errorVariable` (default `$error`) is the declared `TryCatchErrorValue`: `nodeId` and `message` as before, plus `iteration` and the loop's current `item` when the failure happened inside a loop body. A `subflow` / `map` child run has its own variable scope and therefore binds neither, so a parent's row identity never leaks into a child's `$error`.

**`failed` absent means "not tracked", never `0`.** Runs recorded before this change keep it absent — no migration and no default, the same convention `unmeasured` carries. Defaulting it to zero would tell an operator "nothing failed" about a run nobody measured. Absent, the summary line prints no `failed=` token at all; present-and-zero prints `failed=0`. The count rides in the persisted `summary_json`, including on a summary compacted past the size cap, where the per-node `failures` it folds are exactly what gets dropped.
16 changes: 16 additions & 0 deletions .changeset/region-tagger-branch-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@objectstack/service-automation": minor
---

The run step log tells a `parallel` branch apart from a `loop` row: `iteration` is the enclosing loop's iteration, always, and the branch index moves to `branch`.

The engine half of the ruling `@objectstack/spec` already declares (`ExecutionStepLogSchema.branch`). One field used to hold both meanings, told apart only by reading `regionKind` first, and `runRegion`'s tagger let the innermost region win outright — it skipped any step a nested region had already tagged. Together those two facts made `loop { body: [ parallel { branches } ] }` unreadable: every branch step recorded its branch index and **no** step of that branch recorded the row it ran for, so a per-row failure inside a branch was attributable to a branch and never to a row. That is the shape a fan-out inside a sweep has, and the one an operator most needs to read.

- **`branch` is written, and only inside a parallel branch.** `parallel` tags its branch regions with `branch: i` instead of `iteration: i`. A step outside a parallel branch carries no `branch` at all.
- **`iteration` is single-valued and carried through nesting.** `runRegion`'s tagger now splits what "innermost wins" governs. The IDENTITY fields — `parentNodeId`, `regionKind`, `retryAttempt` — answer *which region ran this step* and still belong to the innermost region outright; an enclosing region never relabels them. The INDEX fields — `iteration` and `branch` — answer *which pass of which region*, and nested regions contribute different ones that are both true of the same step, so an enclosing region now fills the index the inner region left undefined instead of being turned away at the door. A branch step inside a loop body therefore carries **both**: the row on `iteration`, the branch on `branch`.
- **`try` / `catch` inside a loop is unchanged**, deliberately. Such a region has no index of its own, so its steps keep carrying the enclosing loop's `iteration` with `regionKind` still naming the region, and gain no `branch`. It is the control arm of this change, not a subject of it.
- **Nested loops are unchanged too.** "Fill only what is undefined" still holds in both halves, so for `loop { loop }` the inner loop's `iteration` stands.

`StepLogEntry` (exported) gains `branch?: number`. It is not derived from the spec type, and is now held equal to it by a type-level pin rather than by a comment claiming they agree.

**Reading a run recorded before this change.** `iteration` on a `regionKind: 'parallel-branch'` step written by an older engine is a BRANCH index, not a row — the same absent-versus-zero care the run summary's other counters need. Nothing is migrated and nothing is defaulted: a step with no `branch` key is either a pre-change record or a step that ran outside a parallel branch, and `regionKind` is what tells those apart. Bumped `minor` rather than `major` to match the contract half of the same ruling, which shipped its declaration change that way.
18 changes: 16 additions & 2 deletions docs/qa/platform-checklist/areas/automation.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
"title": "Flow Runs render loop/region iterations as a nested execution tree",
"since": "v16",
"status": "active",
"revision": 2,
"revision": 3,
"priority": "P1",
"surface": "mixed",
"personas": [
Expand All @@ -85,7 +85,8 @@
"fixtures": {
"app": "showcase",
"requires": [
"showcase_batch_reminders (examples/app-showcase/src/automation/flows/index.ts BatchRemindersFlow) — an autolaunched loop flow with a `tasks` list input, runnable on demand via the trigger route"
"showcase_batch_reminders (examples/app-showcase/src/automation/flows/index.ts BatchRemindersFlow) — an autolaunched loop flow with a `tasks` list input, runnable on demand via the trigger route",
"a `loop` whose body holds a two-branch `parallel` — the only shape that exercises both index keys on one step, needed by the loop { parallel } clause. ⚠️ NOT PRESENT in examples/app-showcase: it carries `loop` (showcase_batch_reminders) and `parallel` (showcase_fan_out_notify) as SEPARATE flows and nests neither, so that clause scores blocked(fixture) until such a flow lands (filed as #16356). The clause is written now because what it pins is settled — maintainer ruling 2026-09-03 — and a clause missing from this item is exactly what let the `iteration` overload sit unmeasured"
]
},
"steps": [
Expand All @@ -95,6 +96,7 @@
"record every step's nodeId, nodeType, status, parentNodeId, iteration, regionKind (ExecutionStepLogSchema #1505 region tags)",
"open the flow in the Studio flow-designer (metadata-admin) and its Runs panel (FlowRunsPanel) — NOT the developer Flow Runs page — and expand the newest run",
"screenshot the expanded step tree showing the per-iteration children under the loop node",
"nesting run (needs the loop { parallel } fixture above): trigger that flow over at least two rows, GET its newest run detail, and record every branch step's parentNodeId, iteration, branch and regionKind — both index keys are read off the SAME step, which is the whole point of the clause",
"contrast run: trigger again with {\"params\": {\"tasks\": []}} and capture the loop step of that run"
],
"acceptance": [
Expand All @@ -110,6 +112,12 @@
"verify": "assert the three send_reminder steps each carry {parentNodeId:'loop_tasks', iteration: 0|1|2, regionKind:'loop-body'} and no two share an iteration; top-level steps (start/loop_tasks/end) carry NO parentNodeId",
"evidence": "step-log excerpt with the tags"
},
{
"clause": "loop { parallel }: a branch step carries BOTH indices, one meaning each — the enclosing loop's row on `iteration`, its own branch position on `branch` — and `iteration` never carries a branch index. Maintainer ruling 2026-09-03: \"`iteration` is single-valued: the zero-based iteration of the enclosing loop, carried through any nesting … The branch index lives on a new optional `branch` key, present only on steps inside a `parallel` branch.\" Before it, one field held both meanings and the innermost region won outright, so every branch step of a loop { parallel } recorded its branch and NO step of that branch recorded the row — a per-row failure inside a branch was attributable to a branch and never to the row",
"oracle": "api",
"verify": "from the nesting run's detail take the steps with regionKind='parallel-branch' and assert THREE things, because any two of them still pass on a broken engine: (a) each carries a `branch` equal to its branch position, `branch: 0` INCLUDED — a falsy check anywhere on the way silently drops the first branch; (b) each carries an `iteration` equal to the ROW it ran for, so across N rows x M branches every (iteration, branch) pair appears exactly once — the pre-fix engine wrote a CONSTANT iteration per branch, which is why counting distinct values is not enough on its own; (c) the enclosing `parallel` container step itself reads regionKind='loop-body' with the row on `iteration` and NO `branch` of its own. Cross-check that every recorded step still parses under ExecutionStepLogSchema",
"evidence": "step-log excerpt for one full row showing both branch steps with their (iteration, branch) pairs, plus the enclosing parallel container step"
},
{
"clause": "the designer Runs panel renders the iterations as a nested tree (per-iteration children folded under the loop node, labeled 1-based), not a flat list",
"oracle": "screenshot",
Expand Down Expand Up @@ -154,6 +162,12 @@
"date": "2026-08-07",
"change": "expanded to deep-test contract: concrete steps, multi-clause acceptance, negatives, variants",
"ref": "claude/platform-test-checklist-ocwugl"
},
{
"revision": 3,
"date": "2026-09-06",
"change": "added the loop { parallel } clause. The item pinned parentNodeId/iteration/regionKind on a plain loop body and said nothing about the nested case — the one shape where two enclosing regions each have an index of their own. It was unwritable while `iteration` was overloaded, because there was no correct reading to assert: the branch index displaced the row. The maintainer ruling of 2026-09-03 made `iteration` single-valued and gave the branch index its own `branch` key, and the engine's runRegion tagger now carries an outer region's index through nesting instead of discarding it for a step an inner region already tagged. Also records, rather than hides, the fixture gap the clause exposes: showcase nests neither construct in the other",
"ref": "#15230"
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,9 +331,17 @@ describe('#14456 — a contained per-iteration failure is visible, attributed an
}
});

// ── The fence: `parallel` is untouched ─────────────────────────────────

it('leaves `parallel` branch tagging exactly as it was — the #14414 fence', async () => {
// ── `parallel` inside a loop: both indices, one meaning each ───────────
//
// This case was the #14414 FENCE while the overload stood: it asserted that
// #14456 had left `parallel` alone, with the branch index still occupying
// `iteration`. #15230 is the card that retired the overload (maintainer
// ruling 2026-09-03), so the fence is spent and what it guarded has moved:
// the assertion below now reads the two indices apart. It stays in this
// suite because #14456's subject — attributing a contained per-row failure
// to its ROW — is exactly what a branch step could not do before.

it('loop { parallel }: a branch step carries the row on `iteration` and the branch on `branch` (#15230)', async () => {
setup(CASES);
engine.registerFlow('par', {
name: 'par', label: 'par', type: 'autolaunched', runAs: 'system',
Expand Down Expand Up @@ -370,13 +378,23 @@ describe('#14456 — a contained per-iteration failure is visible, attributed an
await engine.execute('par', { event: 'schedule' } as AutomationContext);
const run = (await engine.listRuns('par'))[0];

// A branch step still carries its BRANCH index on `iteration` and
// `regionKind: 'parallel-branch'`, five times over (once per row) — the
// pre-existing overload #14414 owns. Nothing here changed it.
// Five rows x two branches, `regionKind: 'parallel-branch'` throughout.
const branchSteps = run.steps.filter((s) => s.regionKind === 'parallel-branch');
expect(branchSteps).toHaveLength(10);
expect(branchSteps.filter((s) => s.nodeId === 'flag').every((s) => s.iteration === 0)).toBe(true);
expect(branchSteps.filter((s) => s.nodeId === 'noop').every((s) => s.iteration === 1)).toBe(true);

const flagSteps = branchSteps.filter((s) => s.nodeId === 'flag');
const noopSteps = branchSteps.filter((s) => s.nodeId === 'noop');

// The BRANCH index is constant per branch and lives on `branch`.
expect(flagSteps.every((s) => s.branch === 0)).toBe(true);
expect(noopSteps.every((s) => s.branch === 1)).toBe(true);

// The ROW comes through `iteration`, carried down from the enclosing
// loop — the reading that did not exist while the branch index sat
// there. Each branch ran once per row, so each sees every row exactly
// once.
expect(flagSteps.map((s) => s.iteration)).toEqual([0, 1, 2, 3, 4]);
expect(noopSteps.map((s) => s.iteration)).toEqual([0, 1, 2, 3, 4]);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,18 @@ describe('parallel block executor (ADR-0031)', () => {
const stepA = runs[0].steps.find(s => s.nodeId === 'a');
const stepB = runs[0].steps.find(s => s.nodeId === 'b');
expect(stepA?.parentNodeId).toBe('par');
expect(stepA?.iteration).toBe(0);
expect(stepA?.branch).toBe(0);
expect(stepA?.regionKind).toBe('parallel-branch');
expect(stepB?.parentNodeId).toBe('par');
expect(stepB?.iteration).toBe(1);
expect(stepB?.branch).toBe(1);
expect(stepB?.regionKind).toBe('parallel-branch');
// #15230: the branch index moved off `iteration`, which is now single-valued
// — the enclosing LOOP's row. This `parallel` sits in no loop, so there is
// nothing for it to report and it stays absent. Asserting the absence is the
// point: re-spelling the assertion alone would leave the retired overload
// free to come back as a second writer.
expect(stepA?.iteration).toBeUndefined();
expect(stepB?.iteration).toBeUndefined();
});

it('joins only after the slowest branch completes', async () => {
Expand Down
17 changes: 14 additions & 3 deletions packages/services/service-automation/src/builtin/parallel-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,22 @@ export function registerParallelNode(engine: AutomationEngine, ctx: PluginContex
try {
// Implicit join: continue once when ALL branches have completed.
// #1479: each branch returns its body steps, tagged with the branch index.
//
// #15230: the branch index goes on `branch`, NOT on `iteration`. It
// used to share `iteration` with the loop's row index — one field, two
// meanings, told apart only by reading `regionKind` first — and because
// the tagger let the innermost region win outright, a branch step
// inside a loop body recorded the branch and lost the row entirely.
// `iteration` is now single-valued (the enclosing loop's, carried
// through nesting by `runRegion`'s tagger), so nothing here writes it:
// a branch step inside a loop body gets the row from the loop's own
// tagging pass, and a bare `parallel` leaves it absent because there is
// no enclosing loop to report. Maintainer ruling 2026-09-03.
branchSteps = await Promise.all(
branches.map((branch, i) =>
engine.runRegion(branch, variables, context ?? ({} as AutomationContext), {
branches.map((branchRegion, i) =>
engine.runRegion(branchRegion, variables, context ?? ({} as AutomationContext), {
parentNodeId: node.id,
iteration: i,
branch: i,
regionKind: 'parallel-branch',
}),
),
Expand Down
Loading
Loading