|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | +// |
| 3 | +// #15616 — a `map` node inside a `loop` body ran its collection ONCE. |
| 4 | +// |
| 5 | +// `map` tracks its progress through the collection in `<nodeId>.$mapState`, |
| 6 | +// which it wrote into the flow's **shared** variable scope and never removed. |
| 7 | +// A `loop` body region runs in that same scope (`runRegion` is handed the |
| 8 | +// caller's map, deliberately — the iterator variable and body mutations have to |
| 9 | +// be visible), so the state written by iteration 1 was still there when |
| 10 | +// iteration 2 entered the map: `started === collection.length`, nothing left to |
| 11 | +// start, return success. Iterations 2..n ran nothing, every map step reported |
| 12 | +// `success`, and the run finished `completed`. |
| 13 | +// |
| 14 | +// The measurement these tests reproduce, on the real `AutomationEngine`: |
| 15 | +// **5 iterations × 2 items ⇒ 2 child runs instead of 10**, with `failed = 0`. |
| 16 | +// That last clause is why this needed its own card rather than riding #14456: |
| 17 | +// nothing throws, nothing is caught, so `FlowRunSummary.failed` — the counter |
| 18 | +// built to expose silently-contained failures — reports a clean run over it. |
| 19 | +// |
| 20 | +// ⚠️ The lifetime is the point, not the key. `$mapState` MUST survive a durable |
| 21 | +// pause: a `map` whose per-item child run paused resumes by re-entering this |
| 22 | +// node and reading that state back. What it must not do is survive the node's |
| 23 | +// own completion. Both halves are pinned here — the last test fails if the fix |
| 24 | +// is spelled as an unconditional delete. |
| 25 | + |
| 26 | +import { describe, it, expect } from 'vitest'; |
| 27 | +import { AutomationEngine } from '../engine.js'; |
| 28 | +import type { NodeExecutor } from '../engine.js'; |
| 29 | +import { defineActionDescriptor } from '@objectstack/spec/automation'; |
| 30 | +import { InMemorySuspendedRunStore } from '../suspended-run-store.js'; |
| 31 | +import { registerLoopNode } from './loop-node.js'; |
| 32 | +import { registerMapNode } from './map-node.js'; |
| 33 | + |
| 34 | +function silentLogger(): any { |
| 35 | + const l: any = { info() {}, warn() {}, error() {}, debug() {} }; |
| 36 | + l.child = () => l; |
| 37 | + return l; |
| 38 | +} |
| 39 | +const pluginCtx = (logger: any) => ({ logger, getService() { throw new Error('none'); } }) as any; |
| 40 | + |
| 41 | +/** The card's fixture: five loop iterations, two mapped items each. */ |
| 42 | +const ROWS = ['r1', 'r2', 'r3', 'r4', 'r5']; |
| 43 | +const CELLS = ['a', 'b']; |
| 44 | + |
| 45 | +interface Harness { |
| 46 | + engine: AutomationEngine; |
| 47 | + /** One entry per CHILD RUN that actually executed, in order: `row:cell`. */ |
| 48 | + ran: string[]; |
| 49 | + /** Per loop iteration: what the body observed after the map node returned. */ |
| 50 | + observed: Array<{ results: unknown; stateKeyPresent: boolean }>; |
| 51 | +} |
| 52 | + |
| 53 | +/** |
| 54 | + * `loop { body: [ map { flowName: cell_flow }, probe ] }` over the real engine. |
| 55 | + * |
| 56 | + * `probe` sits after the map INSIDE the body region, so it reads the same |
| 57 | + * shared scope the map just wrote — which is how the state key's lifetime is |
| 58 | + * observed directly rather than inferred from the child-run count. |
| 59 | + */ |
| 60 | +function setup(): Harness { |
| 61 | + const logger = silentLogger(); |
| 62 | + const engine = new AutomationEngine(logger); |
| 63 | + registerLoopNode(engine, pluginCtx(logger)); |
| 64 | + registerMapNode(engine, pluginCtx(logger)); |
| 65 | + |
| 66 | + const ran: string[] = []; |
| 67 | + const observed: Array<{ results: unknown; stateKeyPresent: boolean }> = []; |
| 68 | + |
| 69 | + // The child flow's only node: records that this child run happened. |
| 70 | + engine.registerNodeExecutor({ |
| 71 | + type: 'cellmark', |
| 72 | + async execute(_node, variables, context) { |
| 73 | + const p = (context as any)?.params ?? {}; |
| 74 | + ran.push(`${p.row}:${p.cell}`); |
| 75 | + variables.set('result', `${p.row}:${p.cell}`); |
| 76 | + return { success: true }; |
| 77 | + }, |
| 78 | + } as NodeExecutor); |
| 79 | + |
| 80 | + // Loop-body probe, downstream of the map in the SAME region scope. |
| 81 | + engine.registerNodeExecutor({ |
| 82 | + type: 'probe', |
| 83 | + async execute(_node, variables) { |
| 84 | + observed.push({ |
| 85 | + results: variables.get('cellResults'), |
| 86 | + stateKeyPresent: variables.has('per_cell.$mapState'), |
| 87 | + }); |
| 88 | + return { success: true }; |
| 89 | + }, |
| 90 | + } as NodeExecutor); |
| 91 | + |
| 92 | + engine.registerFlow('cell_flow', { |
| 93 | + name: 'cell_flow', |
| 94 | + label: 'Cell', |
| 95 | + type: 'autolaunched', |
| 96 | + variables: [{ name: 'result', type: 'text', isOutput: true }], |
| 97 | + nodes: [ |
| 98 | + { id: 'cs', type: 'start', label: 'Start' }, |
| 99 | + { id: 'cm', type: 'cellmark', label: 'Mark' }, |
| 100 | + { id: 'ce', type: 'end', label: 'End' }, |
| 101 | + ], |
| 102 | + edges: [ |
| 103 | + { id: 'c1', source: 'cs', target: 'cm' }, |
| 104 | + { id: 'c2', source: 'cm', target: 'ce' }, |
| 105 | + ], |
| 106 | + } as never); |
| 107 | + |
| 108 | + engine.registerFlow('sweep_flow', { |
| 109 | + name: 'sweep_flow', |
| 110 | + label: 'Sweep', |
| 111 | + type: 'autolaunched', |
| 112 | + variables: [ |
| 113 | + { name: 'rows', type: 'list', isInput: true }, |
| 114 | + { name: 'cells', type: 'list', isInput: true }, |
| 115 | + ], |
| 116 | + nodes: [ |
| 117 | + { id: 'ss', type: 'start', label: 'Start' }, |
| 118 | + { |
| 119 | + id: 'sweep', type: 'loop', label: 'For each row', |
| 120 | + config: { |
| 121 | + collection: '{rows}', |
| 122 | + iteratorVariable: 'row', |
| 123 | + body: { |
| 124 | + nodes: [ |
| 125 | + { |
| 126 | + id: 'per_cell', type: 'map', label: 'For each cell', |
| 127 | + config: { |
| 128 | + flowName: 'cell_flow', |
| 129 | + collection: '{cells}', |
| 130 | + iteratorVariable: 'cell', |
| 131 | + input: { row: '{row}', cell: '{cell}' }, |
| 132 | + outputVariable: 'cellResults', |
| 133 | + }, |
| 134 | + }, |
| 135 | + { id: 'probe', type: 'probe', label: 'Probe' }, |
| 136 | + ], |
| 137 | + edges: [{ id: 'be', source: 'per_cell', target: 'probe' }], |
| 138 | + }, |
| 139 | + }, |
| 140 | + }, |
| 141 | + { id: 'se', type: 'end', label: 'End' }, |
| 142 | + ], |
| 143 | + edges: [ |
| 144 | + { id: 's1', source: 'ss', target: 'sweep' }, |
| 145 | + { id: 's2', source: 'sweep', target: 'se' }, |
| 146 | + ], |
| 147 | + } as never); |
| 148 | + |
| 149 | + return { engine, ran, observed }; |
| 150 | +} |
| 151 | + |
| 152 | +describe('#15616 — a `map` in a `loop` body runs its collection on EVERY iteration', () => { |
| 153 | + it("runs 5 iterations x 2 items as 10 child runs (the card's measurement: it was 2)", async () => { |
| 154 | + const { engine, ran } = setup(); |
| 155 | + |
| 156 | + const result = await engine.execute('sweep_flow', { params: { rows: ROWS, cells: CELLS } }); |
| 157 | + |
| 158 | + expect(result.success).toBe(true); |
| 159 | + // The defect's signature was `ran.length === 2` — row r1 only, with |
| 160 | + // rows r2..r5 contributing nothing at all. |
| 161 | + expect(ran).toEqual([ |
| 162 | + 'r1:a', 'r1:b', 'r2:a', 'r2:b', 'r3:a', 'r3:b', 'r4:a', 'r4:b', 'r5:a', 'r5:b', |
| 163 | + ]); |
| 164 | + expect(ran).toHaveLength(ROWS.length * CELLS.length); |
| 165 | + }); |
| 166 | + |
| 167 | + it('reports the run green with `failed = 0` either way — the counter cannot see this defect', async () => { |
| 168 | + const { engine, ran } = setup(); |
| 169 | + |
| 170 | + const result = await engine.execute('sweep_flow', { params: { rows: ROWS, cells: CELLS } }); |
| 171 | + const runs = await engine.listRuns('sweep_flow'); |
| 172 | + |
| 173 | + // Both halves of the card's point, asserted together: the run really is |
| 174 | + // clean (nothing throws, nothing is caught, so #14456's fold reports 0) |
| 175 | + // AND the work really happened. Before the fix the first half held and |
| 176 | + // the second did not — which is exactly why `failed` could not be the |
| 177 | + // instrument that caught it. |
| 178 | + expect(runs[0]?.status).toBe('completed'); |
| 179 | + expect(result.summary?.failed).toBe(0); |
| 180 | + expect(ran).toHaveLength(10); |
| 181 | + }); |
| 182 | + |
| 183 | + it('collects a FRESH result set per iteration, and leaves no progress state behind', async () => { |
| 184 | + const { engine, observed } = setup(); |
| 185 | + |
| 186 | + await engine.execute('sweep_flow', { params: { rows: ROWS, cells: CELLS } }); |
| 187 | + |
| 188 | + expect(observed).toHaveLength(ROWS.length); |
| 189 | + // Each iteration's `outputVariable` holds that iteration's two items — |
| 190 | + // not the first iteration's results re-read, and not an accumulation. |
| 191 | + expect(observed.map(o => o.results)).toEqual( |
| 192 | + ROWS.map(r => [{ result: `${r}:a` }, { result: `${r}:b` }]), |
| 193 | + ); |
| 194 | + // The mechanism itself: once the collection is exhausted the node's |
| 195 | + // progress state is gone from the shared scope, so the next entry to |
| 196 | + // this node starts from zero. This is the assertion that fails on the |
| 197 | + // unfixed engine even if the child-run count somehow did not. |
| 198 | + expect(observed.map(o => o.stateKeyPresent)).toEqual(ROWS.map(() => false)); |
| 199 | + }); |
| 200 | +}); |
| 201 | + |
| 202 | +/** |
| 203 | + * The other half of the lifetime — and the reason "delete the state key" is |
| 204 | + * only correct on the node's TERMINAL paths. |
| 205 | + * |
| 206 | + * A `map` whose per-item child run pauses suspends the parent at this node and |
| 207 | + * is re-entered when the child completes; the re-entry reads its progress back |
| 208 | + * out of the suspend-time snapshot. `resumeInternal` rebuilds the scope with |
| 209 | + * `new Map(Object.entries(run.variables))`, so the ONLY write that can reach a |
| 210 | + * resume is the one the node makes before returning `suspend: true`. An |
| 211 | + * unconditional delete removes it and the resumed map restarts the collection |
| 212 | + * from item 0 — re-running every item that already ran. |
| 213 | + * |
| 214 | + * (A pausing `map` is unreachable from inside a `loop` body: `runRegion` |
| 215 | + * converts a durable pause inside a structured region into an error. So this |
| 216 | + * fixture is a TOP-LEVEL map, which is where the resume path is live.) |
| 217 | + */ |
| 218 | +describe('#15616 — the progress state still survives a durable pause (the half that must NOT change)', () => { |
| 219 | + function pausingSetup() { |
| 220 | + const logger = silentLogger(); |
| 221 | + const engine = new AutomationEngine(logger); |
| 222 | + registerMapNode(engine, pluginCtx(logger)); |
| 223 | + // The durable store is read directly below: `listSuspendedRuns()` |
| 224 | + // deliberately projects away `variables`, and the snapshot is the exact |
| 225 | + // object `resumeInternal` rebuilds the scope from. |
| 226 | + const store = new InMemorySuspendedRunStore(); |
| 227 | + engine.setSuspendedRunStore(store); |
| 228 | + |
| 229 | + const ran: string[] = []; |
| 230 | + let doneResults: unknown; |
| 231 | + let stateKeyAfterCompletion: boolean | undefined; |
| 232 | + |
| 233 | + engine.registerNodeExecutor({ |
| 234 | + type: 'pauser', |
| 235 | + descriptor: defineActionDescriptor({ |
| 236 | + type: 'pauser', version: '1.0.0', name: 'pauser', |
| 237 | + supportsPause: true, resumeAuthority: 'any', |
| 238 | + }), |
| 239 | + async execute() { return { success: true, suspend: true }; }, |
| 240 | + } as NodeExecutor); |
| 241 | + engine.registerNodeExecutor({ |
| 242 | + type: 'cellmark', |
| 243 | + async execute(_node, variables, context) { |
| 244 | + const p = (context as any)?.params ?? {}; |
| 245 | + ran.push(String(p.cell)); |
| 246 | + variables.set('result', String(p.cell)); |
| 247 | + return { success: true }; |
| 248 | + }, |
| 249 | + } as NodeExecutor); |
| 250 | + engine.registerNodeExecutor({ |
| 251 | + type: 'after', |
| 252 | + async execute(_node, variables) { |
| 253 | + doneResults = variables.get('cellResults'); |
| 254 | + stateKeyAfterCompletion = variables.has('per_cell.$mapState'); |
| 255 | + return { success: true }; |
| 256 | + }, |
| 257 | + } as NodeExecutor); |
| 258 | + |
| 259 | + engine.registerFlow('cell_flow', { |
| 260 | + name: 'cell_flow', label: 'Cell', type: 'autolaunched', |
| 261 | + variables: [{ name: 'result', type: 'text', isOutput: true }], |
| 262 | + nodes: [ |
| 263 | + { id: 'cs', type: 'start', label: 'Start' }, |
| 264 | + { id: 'cp', type: 'pauser', label: 'Pause' }, |
| 265 | + { id: 'cm', type: 'cellmark', label: 'Mark' }, |
| 266 | + { id: 'ce', type: 'end', label: 'End' }, |
| 267 | + ], |
| 268 | + edges: [ |
| 269 | + { id: 'c1', source: 'cs', target: 'cp' }, |
| 270 | + { id: 'c2', source: 'cp', target: 'cm' }, |
| 271 | + { id: 'c3', source: 'cm', target: 'ce' }, |
| 272 | + ], |
| 273 | + } as never); |
| 274 | + engine.registerFlow('batch_flow', { |
| 275 | + name: 'batch_flow', label: 'Batch', type: 'autolaunched', |
| 276 | + variables: [{ name: 'cells', type: 'list', isInput: true }], |
| 277 | + nodes: [ |
| 278 | + { id: 'bs', type: 'start', label: 'Start' }, |
| 279 | + { |
| 280 | + id: 'per_cell', type: 'map', label: 'For each cell', |
| 281 | + config: { |
| 282 | + flowName: 'cell_flow', collection: '{cells}', iteratorVariable: 'cell', |
| 283 | + input: { cell: '{cell}' }, outputVariable: 'cellResults', |
| 284 | + }, |
| 285 | + }, |
| 286 | + { id: 'af', type: 'after', label: 'After' }, |
| 287 | + { id: 'be', type: 'end', label: 'End' }, |
| 288 | + ], |
| 289 | + edges: [ |
| 290 | + { id: 'b1', source: 'bs', target: 'per_cell' }, |
| 291 | + { id: 'b2', source: 'per_cell', target: 'af' }, |
| 292 | + { id: 'b3', source: 'af', target: 'be' }, |
| 293 | + ], |
| 294 | + } as never); |
| 295 | + |
| 296 | + return { |
| 297 | + engine, store, ran, |
| 298 | + results: () => doneResults, |
| 299 | + stateKeyAfterCompletion: () => stateKeyAfterCompletion, |
| 300 | + }; |
| 301 | + } |
| 302 | + |
| 303 | + const childRunId = (engine: AutomationEngine) => |
| 304 | + engine.listSuspendedRuns().find(r => r.flowName === 'cell_flow')?.runId; |
| 305 | + |
| 306 | + it('carries `$mapState` into the suspend snapshot and resumes the collection where it left off', async () => { |
| 307 | + const h = pausingSetup(); |
| 308 | + |
| 309 | + // Item 0 pauses → the parent parks at the map node. |
| 310 | + const first = await h.engine.execute('batch_flow', { params: { cells: ['a', 'b', 'c'] } }); |
| 311 | + expect(first.status).toBe('paused'); |
| 312 | + |
| 313 | + // The state is in the SNAPSHOT the resume will rebuild the scope from — |
| 314 | + // already advanced past the in-flight item (ADR-0019). |
| 315 | + const parked = h.engine.listSuspendedRuns().find(r => r.flowName === 'batch_flow')!; |
| 316 | + expect(parked.nodeId).toBe('per_cell'); |
| 317 | + const snapshot = await h.store.load(parked.runId); |
| 318 | + expect(snapshot!.variables['per_cell.$mapState']).toMatchObject({ started: 1 }); |
| 319 | + |
| 320 | + // Drive the three items through. Each resume re-enters the map, which |
| 321 | + // must read its progress back — not restart from item 0. |
| 322 | + await h.engine.resume(childRunId(h.engine)!); |
| 323 | + await h.engine.resume(childRunId(h.engine)!); |
| 324 | + await h.engine.resume(childRunId(h.engine)!); |
| 325 | + |
| 326 | + // Every item ran EXACTLY once, in order. |
| 327 | + expect(h.ran).toEqual(['a', 'b', 'c']); |
| 328 | + expect(h.results()).toEqual([{ result: 'a' }, { result: 'b' }, { result: 'c' }]); |
| 329 | + // …and once the collection is exhausted the state is gone again. |
| 330 | + expect(h.stateKeyAfterCompletion()).toBe(false); |
| 331 | + expect(h.engine.listSuspendedRuns()).toHaveLength(0); |
| 332 | + }); |
| 333 | +}); |
0 commit comments