Skip to content

Commit e7a8bd4

Browse files
huangyiireneclaude
andauthored
test(service-automation): witness parallel fan-out structurally, not by a wall-clock bound (#19804)
Fixes #19766 Clause-②: no The parallel-branches case in `packages/services/service-automation/src/engine.test.ts` graded a real `Date.now()` window against a fixed 100 ms bound. This replaces that with a **structural** witness of the property — what the engine did, in what order — so the case can fail on the regression it names and cannot fail on runner speed. ## Premise verified at `origin/main` first (both halves true, measured) Base `fae870352e`. Both halves were measured, not argued, by lowering the bound to 1 ms through `scripts/ablation-replace.mjs` and reading the actual value vitest prints. | Run | The graded window | Verdict against the 100 ms bound | |:---|:---|:---| | engine unchanged (parallel fan-out) | `expected 13 to be less than 1` ⇒ **13 ms** | passes | | fan-out deliberately sequentialised | `expected 23 to be less than 1` ⇒ **23 ms** | **passes** | 1. **It could not fail on its own regression.** With `await Promise.all(parallelTasks)` replaced by a `for … await this.executeNode(…)` loop, the whole original case still passed: ``` ablation-replace: ok mutation landed: anchor 1 -> 0, blob bd3b534 -> 8458aedccd39 Test Files 1 passed (1) Tests 1 passed | 157 skipped (158) os-verify-lock: VERDICT command-exit 0 ``` 2. **It could fail on runner speed alone.** The graded quantity is real elapsed time: 13 ms measured on this (shared) box against a fixed 100 ms constant, so a correct parallel engine goes red on any runner that is roughly 8x slower through `engine.execute`, or on one scheduling stall of 87 ms. The bound sits above BOTH the parallel and the sequential path, which is why it separates nothing. ## The fix The registered executor — the observable this case already owned — now records an enter/exit trace of the branch bodies, and the case asserts: ```ts const firstReturn = trace.findIndex(event => event.startsWith('exit:')); const beforeFirstReturn = firstReturn === -1 ? trace : trace.slice(0, firstReturn); expect([...beforeFirstReturn].sort()).toEqual(['enter:branch_a', 'enter:branch_b']); ``` Everything preceding the FIRST branch return is a branch ENTRY, and both entries are there — true exactly when both bodies are in flight at once, which is what makes the fan-out parallel. `expect(elapsed).toBeLessThan(100)` and its `Date.now()` pair are gone; the case is not skipped, renamed, quarantined or weakened, and the flow fixture is untouched. ## Evidence direction 1 — it fires Deliberate regression: the unconditional fan-out in `packages/services/service-automation/src/engine.ts` runs one branch at a time. ``` node scripts/ablation-replace.mjs --file packages/services/service-automation/src/engine.ts \ --anchor 'THE PROMISE.ALL FAN-OUT BLOCK' --replacement 'A FOR-OF LOOP AWAITING EACH BRANCH' \ -- bash -c "OS_VERIFY_LOCK_SLOT=issue-19766 bash scripts/pm/os-verify-lock.sh -c \"pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 src/engine.test.ts -t 'should execute unconditional branches in parallel'\"" ``` ``` ablation-replace: ok mutation landed: anchor 1 -> 0, blob bd3b534 -> 8458aedccd39 FAIL src/engine.test.ts > AutomationEngine - Parallel Branch Execution > should execute unconditional branches in parallel AssertionError: expected [ 'enter:branch_a' ] to deeply equal [ 'enter:branch_a', 'enter:branch_b' ] - Expected + Received [ "enter:branch_a", - "enter:branch_b", ] ❯ src/engine.test.ts:2386:47 Test Files 1 failed (1) Tests 1 failed | 157 skipped (158) os-verify-lock: VERDICT command-exit 1 ``` Restore proven byte-exact by the same tool, on the absolute path, against HEAD rather than the index: ``` ablation-replace: restore /home/user/objectstack-issue-19766/packages/services/service-automation/src/engine.ts ablation-replace: blob after restore bd3b534 ablation-replace: blob at HEAD bd3b534 ablation-replace: git diff HEAD empty ablation-replace: ok restored: blob == HEAD (bd3b534) and `git diff HEAD` is empty ``` `git status --porcelain` after the run printed nothing. The mutation needed no rebuild to be read: `engine.test.ts` imports `./engine.js`, a package-local specifier vitest resolves to `src/engine.ts`, so no `dist/` sits between the mutation and the assertion. ## Evidence direction 2 — runner speed cannot reach it Argued from the assertion's shape, which is the stronger of the two forms the card allows: - It reads **no clock**: no `Date.now()`, no `performance.now()`, no duration, no bound, no timeout of its own. - Its only inputs are the ORDER of four events appended by the executor the engine calls. Order is decided by where `await` suspends, not by how long a suspension lasts. - A branch's `delay` can only stretch the wait between that branch's own entry and its own exit. It cannot move an entry across a sibling's return: under `Promise.all` over eagerly created tasks, both bodies are entered before either is awaited, on any machine, at any load. - The failure mode is likewise speed-free: a sequential fan-out emits `enter:branch_a, exit:branch_a, …` whatever the delays are. Observed green while the box was also running this repo's shared verify lock and sibling agent work: ``` pnpm --filter @objectstack/service-automation test Test Files 141 passed (141) Tests 1690 passed (1690) Duration 98.17s os-verify-lock: VERDICT command-exit 0 ``` ## Local verification | Scope | Command | Result | |:---|:---|:---| | dependency closure | `pnpm --filter '@objectstack/service-automation^...' build --concurrency=2` | exit 0 | | package tests | `pnpm --filter @objectstack/service-automation test` | 141 files / 1690 tests, exit 0 | | package typecheck | `pnpm --filter @objectstack/service-automation typecheck` | exit 0 (`tsc --noEmit` + `check:test-typecheck`, 0 errors) | | derived gate families | `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, all 53 run, reconciled with `--ran` | 51 exit 0 · 2 NOT MEASURED | | repo lint | `pnpm lint` (`eslint . --no-inline-config`, whole repo, not narrowed) | exit 0 | The two NOT MEASURED are `pnpm check:dual-build-cjs-loads` and `pnpm check:type-check-debt`: both exited **3**, `PREREQUISITE NOT MET` — each reads built output for the whole workspace and refused to measure without it. That is neither a pass nor a finding; CI builds the workspace and runs them there. Reconciliation: `53 derived, 51 run, 2 NOT-MEASURED, 0 UNRUN`. ## Changeset routing `skip-changeset`, ⛔ not an empty changeset. The diff is one test file, and this package publishes `files: ["dist","README.md","CHANGELOG.md"]`. Probed on the freshly built bundles, with a positive control: ``` dist/index.js enter:branch_a=0 AutomationEngine=24 dist/index.cjs enter:branch_a=0 AutomationEngine=25 dist/index.d.ts enter:branch_a=0 AutomationEngine=50 ``` A symbol introduced by this diff appears zero times in every shipped artefact while the control symbol hits; nothing published moves, so there is no release note to write. ## Acceptance notes Out of scope here, reported to the PM rather than filed: - The card recorded that it ran no census of other wall-clock assertions. A grep of this package finds **none left** after this change: `expect(elapsed)` has zero hits under `packages/services/service-automation/src`. - Repo-wide, `git grep -n "expect(elapsed)" -- 'packages/**/*.test.ts'` returns 10 hits in other packages. Most separate orders of magnitude (1 s against a 60 s acquire timeout, 2 s against a 5 s request), where the bound does discriminate. The one that shares this card's second defect is `packages/plugins/plugin-webhooks/src/auto-enqueuer.test.ts:558`, `expect(elapsed).toBeLessThan(20)` — a correct implementation reddens on a contended runner. Unlike this card's assertion it is not a phantom in the other direction: the regression it rules out would await a promise nothing resolves, so it hangs into the 5 s test timeout rather than passing. Not reproducible today, so it is noted, not filed. - `packages/services/service-automation/src/builtin/template-date-offset-dst.test.ts` — the real origin of the red this card was filed beside, per the filer's own correction — was ⛔ not touched: that is #19768, held serial behind this one. --- _Generated by [Claude Code](https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent ebc6afe commit e7a8bd4

1 file changed

Lines changed: 27 additions & 5 deletions

File tree

‎packages/services/service-automation/src/engine.test.ts‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2322,12 +2322,19 @@ describe('AutomationEngine - Parallel Branch Execution', () => {
23222322

23232323
it('should execute unconditional branches in parallel', async () => {
23242324
const executionOrder: string[] = [];
2325+
// Entry/exit trace of the branch bodies, recorded by the executor the
2326+
// engine calls — the observable that makes the fan-out's SHAPE readable.
2327+
// `executionOrder` alone records only that both branches ran, which a
2328+
// sequential engine satisfies just as well.
2329+
const trace: string[] = [];
23252330

23262331
engine.registerNodeExecutor({
23272332
type: 'script',
23282333
async execute(node) {
2334+
trace.push(`enter:${node.id}`);
23292335
const delay = (node.config as any)?.delay ?? 0;
23302336
await new Promise(r => setTimeout(r, delay));
2337+
trace.push(`exit:${node.id}`);
23312338
executionOrder.push(node.id);
23322339
return { success: true };
23332340
},
@@ -2351,17 +2358,32 @@ describe('AutomationEngine - Parallel Branch Execution', () => {
23512358
],
23522359
});
23532360

2354-
const start = Date.now();
23552361
const result = await engine.execute('parallel_flow');
2356-
const elapsed = Date.now() - start;
23572362

23582363
expect(result.success).toBe(true);
23592364
// Both branches should execute (order may vary in parallel)
23602365
expect(executionOrder).toContain('branch_a');
23612366
expect(executionOrder).toContain('branch_b');
2362-
// Parallel execution should be faster than sequential (10+10=20ms)
2363-
// Allow generous margin but expect it's faster than fully sequential
2364-
expect(elapsed).toBeLessThan(100); // generous but parallel should be ~15ms
2367+
2368+
// What makes this fan-out parallel is that both branch bodies are IN
2369+
// FLIGHT AT ONCE — a structural fact about what the engine did, in what
2370+
// order, and ⛔ never a wall-clock window. `Promise.all` over eagerly
2371+
// created tasks enters every branch body before awaiting any of them,
2372+
// so everything preceding the FIRST branch return is an entry, and both
2373+
// entries are among them. An engine that awaited the branches one at a
2374+
// time traces `enter:branch_a, exit:branch_a, enter:branch_b, …`, whose
2375+
// slice below holds a single entry — red at ANY runner speed.
2376+
//
2377+
// ⛔ A clocked bound cannot witness this property in either direction:
2378+
// the sequential path it claims to rule out (two 10 ms branches) fits
2379+
// inside any bound generous enough for a contended runner, so such an
2380+
// assertion is green on the regression AND red on a slow box. Nothing
2381+
// below reads a clock or picks a bound; a branch's delay only stretches
2382+
// the wait between its own entry and its own exit, and can never
2383+
// reorder an entry across a sibling's return.
2384+
const firstReturn = trace.findIndex(event => event.startsWith('exit:'));
2385+
const beforeFirstReturn = firstReturn === -1 ? trace : trace.slice(0, firstReturn);
2386+
expect([...beforeFirstReturn].sort()).toEqual(['enter:branch_a', 'enter:branch_b']);
23652387
});
23662388
});
23672389

0 commit comments

Comments
 (0)