Skip to content

Commit d7ccdfc

Browse files
refactor(service-automation): move the DAG traversal bodies into dag-traversal.ts (#25222 slice 3) (#25531)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuCEzLkAB12FfRHL9ipwfKPd)_ Part of #25222 Clause-②: no Metadata-protocol: none Ruling-ref: 6348891160 Slice 3 of the `engine.ts` split: the DAG traversal of `AutomationEngine`. The claim is 6373476307. The bodies of eight methods move, unedited, to the new module `packages/services/service-automation/src/dag-traversal.ts`: - node execution and traversal: `executeNode`, `traverseNext`, `runRegion`, `executeWithTimeout`; - condition and value evaluation: `celScope`, `valueEnvelopeRefusals`, `evaluateValueEnvelope`, `evaluateCondition`. The change is move-only, and the card stays open for the later slices. Branch: `claude/project-thread-nirtwt`, the harness-pinned branch for this session, restarted from `main` at `f38832be` (slice 2's merge, #25520). The branch name has no issue number, so the claim comment is the identity record. ## Scope - **Split in two.** The seat agreed a 12-body plan for this section. Generated whole, it measured 3,044 changed lines, over the 3,000-line bar, so it is cut in two. This PR is 2,062 changed lines. `retryExecution`, `executeWithoutRetry`, `seedDeclaredVariables` and `seedRunVariables` go in slice 4, on its own claim. - **The 33-site question.** The ruling on the 33 `this.FIELD.delete/insert/update(` sites in 21 methods is pending with Sales. This slice moves **0** of them, because none of the eight bodies is among the 21. `check-tenant-audit-census` and `check-system-context-census` give byte-identical output at base and head (see Readers). - **Return types.** All eight methods declare their return types. That includes the public `runRegion`, `evaluateValueEnvelope` and `evaluateCondition`, so the inferred-return-type `.d.ts` trap from #25467 does not apply. ## What stays in `engine.ts` - Every moved method keeps its name, docblock, signature and position in the class, because tsup emits each member, private ones included, into the published `.d.ts`. - The section's other members stay as they were: - the run log: `recordLog`, `finishRefusedRun` and `compactStepsForHistory`; - the node-type vocabulary members; - slice 1's registration-check stubs; - `validateNodeInputSchemas` and `setNodeError`; - the refusal and text-slot renderers `renderRefusalMessage`, `renderFlowTextSlot` and `translatedFlowTemplate`, which the spec translation ledger cites; - slice 4's four bodies. ## The shape (slices 1 and 2, one host interface) - `DagTraversalHost` types each of its 11 members as the class member itself (`AutomationEngine['m']`), so a renamed member is a compile error. There is no cast, and no member becomes public or protected. - Each moved function types `this` as the `Pick` of `DagTraversalHost` over exactly the members its own body reads. The generator read those members off each body's AST, in first-read order. A body that reads no member (`executeWithTimeout`, `celScope`, `valueEnvelopeRefusals`) is called directly, without a host. - Methods are passed as late-bound closures, `(...a) => this.m(...a)`, so a spy or test double set on the instance is still the one called. Recursion stays inside the class: `executeNode` and `traverseNext` reach each other through the host's closures. - Fields are passed by value. There are two: `logger`, assigned only in the constructor, and the `nodeExecutors` map, assigned once at its declaration. Neither is reassigned anywhere in the repo. So a host's field is the engine's field, including where an async body reads it after an `await`. - **Async methods.** The stubs of the four async methods (`executeNode`, `traverseNext`, `runRegion`, `executeWithTimeout`) drop `async` and return the moved function's promise, as #25223 did. The caller gets the very promise the body makes, with no extra await and no extra microtask. Dropping `async` is the only edit to those four signature lines, and the `.d.ts` does not print `async`. - The moved bodies keep saying `this.`, so the moved lines are unedited. ## Module-level declarations and imports - **Moved with the bodies.** Two helpers have no other code reader, and each moves with its docblock: - `runUserOf`, read by `evaluateCondition`. It is exported from the module (one word added), so the two `{@link runUserOf}` that stay in `engine.ts` resolve through `import { type runUserOf }`, as #25223 cut 6 did. - `decisionTakesEveryBranch`, read by `traverseNext`. - **Shared with code that stays, exported in place.** `FlowSuspendSignal`, `isSuspendSignal`, `FlowRefusalSignal` and `isRefusalSignal` each gain `export`, as #25223 did. `index.ts` re-exports `./engine.js` by name, so the package entry is unchanged; the export lists and the `.d.ts` below prove it. - **The module's value import of `./engine.js`.** It imports those four, `DEFAULT_BRANCH_LABEL` (already exported) and `AutomationEngine` by value. The class is imported only for its public static `MAX_NODE_REENTRIES`, read in `executeNode`'s runaway guard, as #25224 did for statics. Each of these names is read only inside a function body, at call time, never while a module is evaluating. So the import cycle with `./engine.js` is safe under ESM live bindings, and the tsup bundle is one file and has no cycle. - **Import lines.** - Six import statements that only moved code reads move verbatim, two of them with the comments above them: `createEvalUser`, `structuralConditionRefusal`, `FlowValueSlotSchema`/`VALUE_ENVELOPE_REFUSAL` (the #15137 comment), `ExpressionEngine`/`validateExpression` (the static-import comment), `readPartialSteps`, and `isRegionSuspensionRefusal`/`refuseRegionSuspension`. - Four other import lines drop the names only moved code read: `FlowEdgeParsed`, `findRegionEntry`, `isGuardRefusal` and `attachCarriedRefusal`. - The module adds imports of its own for the types and values its bodies read. ## Proof of move-only - **Moved block** (`git diff --color-moved=plain --color-moved-ws=allow-indentation-change f38832b HEAD` over the two files): 925 of the 938 deleted lines and 950 of the 1,122 added lines are detected as moved. - Deleted, not moved (13): the four narrowed import lines; `runUserOf`'s head, which gains `export`; the four signal declarations' heads, which gain `export`; and the four async methods' heads, which drop `async`. - Added, not moved (172): in `engine.ts`, the 12 replacement lines (the narrowed imports, the `export` heads and the heads without `async`), the module import with its comment, and the eight stub bodies; in the module, the license header, the module docblock, the imports, `runUserOf`'s `export` head, `DagTraversalHost` with its docblock, one pointer docblock per function and the rewritten signature heads. The rest are lines such as a blank or a closing brace that git does not pair with a moved block. - **An independent check** (a script over base `engine.ts`, head `engine.ts` and the module): - 975 of the module's 1,058 lines are in 13 verbatim runs of base `engine.ts` (the script reads 1,059, counting the empty string after the final newline into the last run): the eight method texts de-indented one level, the two helpers with their docblocks, and the moved import blocks with their comments. It names each of the other 83: 35 for the license header, the module docblock and the authored imports; 3 single-line imports moved verbatim (runs shorter than the script's three-line minimum); `runUserOf`'s head with `export`; 23 for the host docblock, the interface and the blank after it; 8 pointer docblocks; and 13 signature lines (eight `export function` heads and five `this` parameter lines, each a `Pick` of the host). - Every line deleted from `engine.ts` is in the module, apart from the 13 lines above (938 deleted, 33 of them blank). - 57 of the 64 lines added to `engine.ts` are found nowhere in base: the narrowed imports, the module import block, the four `export` heads, the four heads without `async`, and the stub lines. The other 7 are lines such as `logger: this.logger,` and `};` that base already contains elsewhere. - No string or template literal in the eight method bodies spans a line (checked on the TypeScript AST of base `engine.ts`), so de-indenting changed no string value. The two helpers and the import blocks moved at their own indentation. - **`.d.ts`**: see the next section. - **Bundles**: the ESM and CJS export lists are identical, 39 names each at base and head, and ESM equals CJS on each side. The four signals gain `export` inside the module graph only: `index.ts` re-exports `./engine.js` by name, so none of them reaches the package entry. The bundle JS bytes differ because code moved within the bundle. - **Tests**: `@objectstack/service-automation`'s suite passes the same at base and head, 199 files and 2628 tests. ## `.d.ts` reading (ruling 6348891160, item 2) Base is `dist/` built at `f38832be` in this container, after a full `turbo run build` (77 of 77 tasks). Head is `dist/` built at this PR's commit. - raw sha256, `index.d.ts` and `index.d.cts` (identical to each other on both sides): base `1a5162a4dd929399460a3936e22747a74cf0a8db24d398ffbab5d7ead7bf212e`, head `1a5162a4dd929399460a3936e22747a74cf0a8db24d398ffbab5d7ead7bf212e`. - normalised sha256, all four files: base `0df32585069f743135440763aabff8c23473231181297c8d164f5f7ee4f10232`, head `0df32585069f743135440763aabff8c23473231181297c8d164f5f7ee4f10232`. - raw diff, `index.d.ts` base vs head: **0 lines** (19,626 lines each side). This slice reorders no union, so the normalised diff is empty too. - the normaliser, the ruling's one-liner, verbatim: `node -e 'const s=require("fs").readFileSync(process.argv[1],"utf8");process.stdout.write(s.replace(/"[^"\n]*"(?:\s*\|\s*"[^"\n]*")+/g,r=>r.split(/\s*\|\s*/).sort().join(" | ")))' dist/index.d.ts | sha256sum` - positive controls on the head `index.d.ts`, line 4764, the private member whose body moved, one line each, then the same normaliser: - `private executeNode;` made public (`executeNode;`) gives `c779de31f261a001dedd376fa77768604dba9074b8abb5238ba902a67dad5f0b`. - the same member renamed (`private executeNodeRenamed;`) gives `844c29245b716d852a5a6e3bd5a111f3bb51b43956e9ada4aa6d9dc3a7813980`. - Both differ from `0df32585…`, so the normaliser hides neither kind of change. - Item 1 holds: (a) the declaration set and visibility are the same; (b) the files are byte-identical, before and after the normalisation; (c) the ESM and CJS export lists are identical; (d) there is no raw-diff line. ## Readers re-pointed - `src/flow-activation-ledger.test.ts`, the #10243 grep pins: `ENGINE_SOURCES` gains `./dag-traversal.ts`, so a moved body is not a place the retired `flowEnabled` map could come back unseen. **Readers examined and not re-pointed:** - `check-tenant-audit-census` (derived): output byte-identical to base `f38832be`, 238 write call sites certified (162 decidable; 2 provably without tenant context, 29 more unreadable), 23 prose figures held, every `UNTYPED_RECEIVERS` row matched. Its `--self-test` output is identical too. No moved body holds a `this.FIELD.delete/insert/update(` site. - `check-system-context-census` (derived): output byte-identical, 149 elevation read sites in 22 packages across 76 files, in 128 symbols. - `check:liveness` (spec lane, not derived, run by hand as `packages/spec`'s `tsx scripts/liveness/check-liveness.mts`): output identical at base and head, 753 (entry, cited file) pairs, 752 anchored, 1 exempt, and 982 of 982 symbol anchors resolve. No liveness row cites a method this slice moves. `flow.json` cites `#executeWithoutRetry` and `#retryExecution`, which stay (slice 4). - `docs/qa/platform-checklist/areas/automation.json:818` anchors `engine.ts#runRegion`. The method stays as a shell, and the `partialSteps` the anchor's note names is a parameter of its signature, which stays. `check:platform-checklist` (not derived, run by hand): output identical at base and head, 987 of 997 symbol anchors resolved, the same 10 on the named #16898 residual. - `check-adr-symbol-anchors` (not derived, run by hand): output identical at base and head, 2384 anchors across 142 records resolve. The two ADR symbol anchors into this file, `#variables` (ADR-0072) and `#canonicalizeStoredFlow` (ADR-0087), resolve on declarations that stay. - `check:adr-anchors` (decision letters, run by hand): `engine.ts` is not on the anchored surface. 1969 decision-letter citations at base and at head; the only difference in the output is the scanned file count, 6674 to 6675, the one new file. - `check:startup-registry-verdict` (not derived, run by hand because its docblock names this file): 46 seams, 40 read-only, none recording a verdict, at base and head. The scanned file count is 3446 at base and 3448 at head: the new module, and `packages/spec/src/migrations/registry.ts`, a git-ignored build output present only in the head worktree. - `text-slot-template.test.ts` walks all of `src/`, so it already reads the module. `builtin/node-config-contract-ledger.test.ts` reads only `src/builtin/`. The other test files that name `engine.ts` name it in prose only. - No `path:line` citation in the repo points into the moved ranges of base `engine.ts` (the module-level lines `:16` to `:180`, and the bodies `:11047` to `:12294`). Lines of `engine.ts` after base `:180` move up by 54, and lines after base `:12294` move up by 874. The one live `service-automation/src/engine.ts:44` string, in `packages/spec/scripts/liveness/evidence.test.ts`, is a realm-attribution fixture that `scanEvidence` parses without reading the file. ## Gates - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on this commit derives 59 families from the 3 changed paths (2,062 changed lines, under the 3,000-line human-merge threshold). All 59 ran in this container on this commit, each exit code captured before any pipe, and all 59 exit 0. `dispatch-gates.mjs --ran` reconciles: 59 derived, 59 run, 0 NOT-MEASURED, 0 UNRUN. - Run by hand at base and head, not derived: `check:liveness`, `check:platform-checklist`, `check-adr-symbol-anchors`, `check:adr-anchors` and `check:startup-registry-verdict`, with the readings in the readers section above. - `@objectstack/service-automation`'s `vitest run`: 199 of 199 files, 2628 of 2628 tests, the same at base and head. ```text node scripts/check-ci-filter-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0 node scripts/check-comment-mask-adoption.mjs :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-dts-emitted.mjs --self-test :: exit 0 node scripts/check-issue-citations.mjs :: exit 0 node scripts/check-keyed-text-bounds.mjs --self-test :: exit 0 node scripts/check-keyed-text-bounds.mjs :: exit 0 node scripts/check-platform-object-tenancy-census.mjs --self-test :: exit 0 node scripts/check-platform-object-tenancy-census.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0 node scripts/check-plugin-teardown-shape.mjs :: exit 0 node scripts/check-position-name-fold-loaders.mjs --self-test :: exit 0 node scripts/check-position-name-fold-loaders.mjs :: exit 0 node scripts/check-registry-log-declared.mjs --self-test :: exit 0 node scripts/check-registry-log-declared.mjs :: exit 0 node scripts/check-rest-log-spy-declared.mjs --self-test :: exit 0 node scripts/check-rest-log-spy-declared.mjs :: exit 0 node scripts/check-system-context-census.mjs --self-test :: exit 0 node scripts/check-system-context-census.mjs :: exit 0 node scripts/check-tenant-audit-census.mjs --self-test :: exit 0 node scripts/check-tenant-audit-census.mjs :: exit 0 node scripts/check-undeclared-dep-imports.mjs --self-test :: exit 0 node scripts/check-undeclared-dep-imports.mjs :: exit 0 node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0 node scripts/docs-audit/check-affected-docs.mjs :: exit 0 node scripts/docs-audit/check-drift-comment.mjs :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:doc-authoring :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:dts-closure :: exit 0 pnpm check:dual-build-cjs-loads :: exit 0 pnpm check:durability-log-level :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:error-status-conformance :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:issue-citations :: exit 0 pnpm check:lean-entry-closure :: exit 0 pnpm check:logger-receiver-detach :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:objectql-double-limit :: exit 0 pnpm check:org-identifier :: exit 0 pnpm check:page-declaration-shape :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:slot-lookup :: exit 0 pnpm check:sourcemap-no-sources-content :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` ## Acceptance notes - Nothing is outside this slice. The judgement calls are the split into two PRs and the four exports in place, both stated in the claim. - Slice 4 (`retryExecution`, `executeWithoutRetry`, `seedDeclaredVariables`, `seedRunVariables`) is claimed after this PR merges. The 33-site ruling decides the slices after that one. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Mnj4nkWuhJ18xEdZvb2cjE Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8ef2a96 commit d7ccdfc

3 files changed

Lines changed: 1123 additions & 939 deletions

File tree

0 commit comments

Comments
 (0)