Skip to content

Commit 60f48e6

Browse files
committed
test(service-automation): pin the region index keys BEFORE the tagger change (#15230)
Red-first. On this unmodified tree the three ruling pins read: loop { parallel } RED — branch=undefined on every branch step, and `iteration` carries the BRANCH index (0/1) instead of the row (0..2): the loop's index is discarded. bare parallel RED — expected undefined to be +0 at `branch`. loop { try_catch } GREEN — the control arm; try/catch already forwards the enclosing loop's iteration at its own call site. spec refusal GREEN — `branch` already refuses -1 / 1.5 at the `branch` path (the contract half landed with PR #15227). The type-level pin is invisible to vitest (esbuild strips types) and is measured by tsc: TS2344 `Type '"branch"' is not assignable to type 'keyof StepLogEntry'`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent 3e270d4 commit 60f48e6

1 file changed

Lines changed: 296 additions & 0 deletions

File tree

Lines changed: 296 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,296 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #15230 — the region index keys a step log entry carries, and what each one
5+
* means. The engine half of #14414; the contract half is `ExecutionStepLogSchema`
6+
* (`packages/spec/src/automation/execution.zod.ts`), already landed.
7+
*
8+
* Maintainer ruling, 2026-09-03 (recorded on #14414):
9+
*
10+
* `iteration` is single-valued: the zero-based iteration of the enclosing
11+
* loop, carried through any nesting … The branch index lives on a new
12+
* optional `branch` key, present only on steps inside a `parallel` branch.
13+
* The engine's `runRegion` tagger stops discarding the outer region's index
14+
* for steps the inner region already tagged.
15+
*
16+
* The three cases the ruling names are pinned here, plus the two guards that
17+
* keep them honest:
18+
*
19+
* 1. `loop { parallel }` — a branch step carries BOTH the loop's `iteration`
20+
* and its own `branch`. This is the shape that was unreadable before: the
21+
* branch index won outright and no step of that branch recorded the row,
22+
* so a per-row failure inside a branch was attributable to a branch and
23+
* never to the row.
24+
* 2. `loop { try_catch }` — UNCHANGED. A try/catch region has no index of its
25+
* own, so its steps carry the enclosing loop's `iteration` and no
26+
* `branch`, with `regionKind` still naming the region. Control arm: this
27+
* must read the same before and after the tagger change.
28+
* 3. `parallel` NOT inside a loop — `branch` is written and `iteration` is
29+
* ABSENT. `iteration` no longer doubles as the branch index, so there is
30+
* nothing for it to say here.
31+
*
32+
* Guards:
33+
* - every record the engine produced parses under the spec schema at THIS
34+
* head, and `branch` refuses negative / fractional values at the `branch`
35+
* path (⛔ not a positives-only gate);
36+
* - a TYPE-LEVEL pin holding the engine's local `StepLogEntry` region keys
37+
* equal to the spec type's. The engine interface is NOT derived from the
38+
* spec — the card asks for the two to be kept in step "by a pin, not by
39+
* prose", and this is that pin. ⚠️ It is compiled by
40+
* `pnpm --filter @objectstack/service-automation typecheck` (both
41+
* `tsconfig.json` and `tsconfig.test.json` reach `src/**\/*.test.ts`) and
42+
* is INVISIBLE to `vitest`, which strips types — a green `pnpm test` says
43+
* nothing about it.
44+
*/
45+
46+
import { describe, it, expect, beforeEach } from 'vitest';
47+
import { ExecutionStepLogSchema } from '@objectstack/spec/automation';
48+
import type { ExecutionStepLog } from '@objectstack/spec/automation';
49+
import { AutomationEngine } from '../engine.js';
50+
import type { NodeExecutor, StepLogEntry } from '../engine.js';
51+
import { registerLoopNode } from './loop-node.js';
52+
import { registerParallelNode } from './parallel-node.js';
53+
import { registerTryCatchNode } from './try-catch-node.js';
54+
55+
// ── Type-level pin: engine `StepLogEntry` ↔ spec `ExecutionStepLog` ─────────
56+
//
57+
// Region-grouping keys only. The engine's interface legitimately carries
58+
// engine-local fields the contract does not (`nodeLabel`, `warnings`), and the
59+
// contract carries fields the engine never writes (`input`, `output`), so
60+
// pinning the WHOLE shape would pin noise. What must not drift is the set of
61+
// keys this card is about and the type of each.
62+
type RegionKeys = 'parentNodeId' | 'iteration' | 'branch' | 'regionKind' | 'retryAttempt';
63+
64+
// Both `Pick`s fail to compile if either side is missing a key — that is half
65+
// the pin. `Eq` is the invariant (not merely assignable) comparison, so a
66+
// widening on one side is caught too.
67+
type Eq<A, B> = (<G>() => G extends A ? 1 : 2) extends <G>() => G extends B ? 1 : 2 ? true : false;
68+
type Assert<T extends true> = T;
69+
export type _RegionKeysInStep = Assert<
70+
Eq<Pick<StepLogEntry, RegionKeys>, Pick<ExecutionStepLog, RegionKeys>>
71+
>;
72+
73+
function silentLogger(): any {
74+
return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } };
75+
}
76+
function ctx(): any {
77+
return { logger: silentLogger(), getService() { throw new Error('none'); } };
78+
}
79+
80+
/** Assert every step the run produced is a legal `ExecutionStepLog` at this head. */
81+
function expectParsesUnderSpec(steps: StepLogEntry[]): void {
82+
expect(steps.length).toBeGreaterThan(0);
83+
for (const step of steps) {
84+
const parsed = ExecutionStepLogSchema.safeParse(step);
85+
if (!parsed.success) {
86+
throw new Error(
87+
`step ${step.nodeId} does not parse under ExecutionStepLogSchema: `
88+
+ JSON.stringify(parsed.error.issues),
89+
);
90+
}
91+
}
92+
}
93+
94+
describe('#15230 — `iteration` is the loop\'s, `branch` is the parallel branch\'s', () => {
95+
let engine: AutomationEngine;
96+
97+
beforeEach(() => {
98+
engine = new AutomationEngine(silentLogger());
99+
registerLoopNode(engine, ctx());
100+
registerParallelNode(engine, ctx());
101+
registerTryCatchNode(engine, ctx());
102+
engine.registerNodeExecutor({
103+
type: 'touch',
104+
async execute() { return { success: true }; },
105+
} as NodeExecutor);
106+
engine.registerNodeExecutor({
107+
type: 'boom',
108+
async execute(node) { throw new Error(`boom from ${node.id}`); },
109+
} as NodeExecutor);
110+
});
111+
112+
// ── Ruling pin 1: `loop { parallel }` ──────────────────────────────────
113+
it('loop { parallel }: a branch step carries the loop\'s `iteration` AND its own `branch`', async () => {
114+
engine.registerFlow('lp', {
115+
name: 'lp', label: 'Loop over parallel', type: 'autolaunched',
116+
variables: [{ name: 'rows', type: 'list', isInput: true }],
117+
nodes: [
118+
{ id: 'start', type: 'start', label: 'Start' },
119+
{
120+
id: 'each', type: 'loop', label: 'For each row',
121+
config: {
122+
collection: '{rows}', iteratorVariable: 'row', indexVariable: 'idx',
123+
body: {
124+
nodes: [{
125+
id: 'fan', type: 'parallel', label: 'Fan out',
126+
config: {
127+
branches: [
128+
{ name: 'A', nodes: [{ id: 'leafA', type: 'touch', label: 'A' }], edges: [] },
129+
{ name: 'B', nodes: [{ id: 'leafB', type: 'touch', label: 'B' }], edges: [] },
130+
],
131+
},
132+
}],
133+
edges: [],
134+
},
135+
},
136+
},
137+
{ id: 'end', type: 'end', label: 'End' },
138+
],
139+
edges: [
140+
{ id: 'e1', source: 'start', target: 'each' },
141+
{ id: 'e2', source: 'each', target: 'end' },
142+
],
143+
} as never);
144+
145+
const result = await engine.execute('lp', { params: { rows: ['r0', 'r1', 'r2'] } });
146+
expect(result.success).toBe(true);
147+
const steps = (await engine.listRuns('lp'))[0].steps as StepLogEntry[];
148+
149+
const branchSteps = steps.filter(s => s.regionKind === 'parallel-branch');
150+
// 3 rows x 2 branches.
151+
expect(branchSteps).toHaveLength(6);
152+
153+
// Each branch step names BOTH indices: which row, and which branch.
154+
expect(
155+
branchSteps.map(s => `${s.nodeId}@iteration=${s.iteration}/branch=${s.branch}`).sort(),
156+
).toEqual([
157+
'leafA@iteration=0/branch=0',
158+
'leafA@iteration=1/branch=0',
159+
'leafA@iteration=2/branch=0',
160+
'leafB@iteration=0/branch=1',
161+
'leafB@iteration=1/branch=1',
162+
'leafB@iteration=2/branch=1',
163+
]);
164+
165+
// The innermost container still wins the IDENTITY fields — carrying the
166+
// outer index through must not relabel which region ran the step.
167+
for (const s of branchSteps) expect(s.parentNodeId).toBe('fan');
168+
169+
// The parallel container step itself is a loop-body step: it has the
170+
// row, and no branch of its own.
171+
const fanSteps = steps.filter(s => s.nodeId === 'fan');
172+
expect(fanSteps).toHaveLength(3);
173+
for (const s of fanSteps) {
174+
expect(s.regionKind).toBe('loop-body');
175+
expect(s.parentNodeId).toBe('each');
176+
expect(s.branch).toBeUndefined();
177+
}
178+
expect(fanSteps.map(s => s.iteration)).toEqual([0, 1, 2]);
179+
180+
expectParsesUnderSpec(steps);
181+
});
182+
183+
// ── Ruling pin 2: `loop { try_catch }` — the CONTROL ARM, unchanged ─────
184+
it('loop { try_catch }: try/catch steps keep the loop\'s `iteration` and gain no `branch`', async () => {
185+
engine.registerFlow('ltc', {
186+
name: 'ltc', label: 'Loop over try_catch', type: 'autolaunched',
187+
variables: [{ name: 'rows', type: 'list', isInput: true }],
188+
nodes: [
189+
{ id: 'start', type: 'start', label: 'Start' },
190+
{
191+
id: 'each', type: 'loop', label: 'For each row',
192+
config: {
193+
collection: '{rows}', iteratorVariable: 'row',
194+
body: {
195+
nodes: [{
196+
id: 'guard', type: 'try_catch', label: 'Guard',
197+
config: {
198+
try: { nodes: [{ id: 'risky', type: 'boom', label: 'Risky' }], edges: [] },
199+
catch: { nodes: [{ id: 'recover', type: 'touch', label: 'Recover' }], edges: [] },
200+
},
201+
}],
202+
edges: [],
203+
},
204+
},
205+
},
206+
{ id: 'end', type: 'end', label: 'End' },
207+
],
208+
edges: [
209+
{ id: 'e1', source: 'start', target: 'each' },
210+
{ id: 'e2', source: 'each', target: 'end' },
211+
],
212+
} as never);
213+
214+
const result = await engine.execute('ltc', { params: { rows: ['r0', 'r1'] } });
215+
expect(result.success).toBe(true);
216+
const steps = (await engine.listRuns('ltc'))[0].steps as StepLogEntry[];
217+
218+
const trySteps = steps.filter(s => s.regionKind === 'try');
219+
const catchSteps = steps.filter(s => s.regionKind === 'catch');
220+
expect(trySteps).toHaveLength(2);
221+
expect(catchSteps).toHaveLength(2);
222+
223+
// A try/catch region has no index of its own: the row comes through
224+
// `iteration`, `regionKind` still names the region, `branch` is absent.
225+
for (const s of [...trySteps, ...catchSteps]) {
226+
expect(s.parentNodeId).toBe('guard');
227+
expect(s.branch).toBeUndefined();
228+
}
229+
expect(trySteps.map(s => s.iteration)).toEqual([0, 1]);
230+
expect(catchSteps.map(s => s.iteration)).toEqual([0, 1]);
231+
232+
expectParsesUnderSpec(steps);
233+
});
234+
235+
// ── Ruling pin 3: `parallel` NOT inside a loop ─────────────────────────
236+
it('bare parallel: a branch step writes `branch` and NO `iteration`', async () => {
237+
engine.registerFlow('bare', {
238+
name: 'bare', label: 'Bare parallel', type: 'autolaunched',
239+
nodes: [
240+
{ id: 'start', type: 'start', label: 'Start' },
241+
{
242+
id: 'fan', type: 'parallel', label: 'Fan out',
243+
config: {
244+
branches: [
245+
{ name: 'A', nodes: [{ id: 'leafA', type: 'touch', label: 'A' }], edges: [] },
246+
{ name: 'B', nodes: [{ id: 'leafB', type: 'touch', label: 'B' }], edges: [] },
247+
],
248+
},
249+
},
250+
{ id: 'end', type: 'end', label: 'End' },
251+
],
252+
edges: [
253+
{ id: 'e1', source: 'start', target: 'fan' },
254+
{ id: 'e2', source: 'fan', target: 'end' },
255+
],
256+
} as never);
257+
258+
const result = await engine.execute('bare');
259+
expect(result.success).toBe(true);
260+
const steps = (await engine.listRuns('bare'))[0].steps as StepLogEntry[];
261+
262+
const leafA = steps.find(s => s.nodeId === 'leafA');
263+
const leafB = steps.find(s => s.nodeId === 'leafB');
264+
expect(leafA?.regionKind).toBe('parallel-branch');
265+
expect(leafB?.regionKind).toBe('parallel-branch');
266+
expect(leafA?.branch).toBe(0);
267+
expect(leafB?.branch).toBe(1);
268+
// No enclosing loop ⇒ nothing for `iteration` to say. Before #15230 it
269+
// carried the branch index; that overload is what the ruling removed.
270+
expect(leafA?.iteration).toBeUndefined();
271+
expect(leafB?.iteration).toBeUndefined();
272+
273+
expectParsesUnderSpec(steps);
274+
});
275+
276+
// ── Guard: the contract refuses illegal `branch` values ─────────────────
277+
it('the spec refuses a negative or fractional `branch`, at the `branch` path', () => {
278+
const base = {
279+
nodeId: 'leafA', nodeType: 'touch', status: 'success' as const,
280+
startedAt: '2026-09-06T00:00:00.000Z',
281+
parentNodeId: 'fan', regionKind: 'parallel-branch',
282+
};
283+
284+
expect(ExecutionStepLogSchema.safeParse({ ...base, branch: 0 }).success).toBe(true);
285+
expect(ExecutionStepLogSchema.safeParse({ ...base, branch: 7 }).success).toBe(true);
286+
287+
for (const bad of [-1, 1.5]) {
288+
const parsed = ExecutionStepLogSchema.safeParse({ ...base, branch: bad });
289+
expect(parsed.success).toBe(false);
290+
// The refusal must land ON `branch` — a schema that rejected the
291+
// whole record for some other reason would read the same at the
292+
// `success` boolean.
293+
expect(parsed.success === false && parsed.error.issues.some(i => i.path[0] === 'branch')).toBe(true);
294+
}
295+
});
296+
});

0 commit comments

Comments
 (0)