Skip to content

Commit 36cac5a

Browse files
refactor(service-automation): move the run-history bodies into run-history.ts (#25222 slice 5) (#25551)
<!-- 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 This is slice 5 of the `engine.ts` split. The claim is comment 6376387561, and the seat agreed the slice on the remaining-bodies table (comment 6376268510). The bodies of nine `AutomationEngine` methods move, unedited, into a new module, `packages/services/service-automation/src/run-history.ts`: - `listRuns`, `listRunsPage`, `runRecordToLogEntry`, `suspendedRunToLogEntry`, `getRun`, `listSuspendedRuns` and `readSuspendedRuns`, which read runs and suspended runs back; - `recordLog` and `finishRefusedRun`, which record a run's log entry and finish a refused run. The change is move-only, and the card stays open for the later slices. The branch is `claude/project-thread-nirtwt`, which the harness pins for this session. It was restarted from `main` at `673ebfd4` after slice 4 (#25542) merged. The branch name has no issue number, so the claim comment is the identity record. ## Scope - **The 33-site question.** The ruling on the 33 `this.FIELD.delete/insert/update(` sites in 21 methods is still pending with Sales. This slice moves **0** of those sites, because none of the nine bodies holds one. `check-tenant-audit-census` and `check-system-context-census` give byte-identical output at base and head (see Readers). - **Return types.** Each of the nine methods declares its return type, so no public body with an inferred return type moves. - **None of the bodies that must stay moves.** Those are the ones the remaining-bodies table marks as stays: - for liveness, `retryExecution` and `getConnectorDescriptors`; - for private statics, `resolveCanonicalDescriptor`, `resolveResumeAuthority` and `resolveEffectiveSuspension`; - for the startup-registry gate, `knownNodeTypes`, `getUnknownNodeTypeAudit`, `sealNodeTypeVocabulary` and `warnUnknownNodeTypes`. - **Left in place.** `listSuspendedRunsDurable`, `getSuspendedScreen` and `compactStepsForHistory` are one line each, so a stub would be no shorter than the body, and they stay in the class. - **Size.** 1,522 changed lines, under the 3,000-line bar: `engine.ts` +80 / -659, the new module +781, and the test 1 / 1. ## 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 line comments between `recordLog`'s docblock and its signature (`[#10101]`, `[#13909]`) stay above the signature too. ## The shape (slices 1 to 4) - **The host interface.** `RunHistoryHost` holds the 14 members these bodies read, each typed 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 `RunHistoryHost` over exactly the members its own body reads, read off each body's AST in first-read order. - `runRecordToLogEntry` and `suspendedRunToLogEntry` read no member, so they are 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. There are six: `listRunsPage`, `suspendedRunToLogEntry`, `runRecordToLogEntry`, `evictConsumedSuspension`, `compactStepsForHistory` and `recordLog`. - **`store` goes through a getter.** This is the first time the series uses one, and it follows the seat's shape call. - `setSuspendedRunStore` reassigns `store`, so passing it by value could hand a body a store the engine has since replaced. - The stub builds `const readStore = () => this.store;` and the host member `get store() { return readStore(); }`, the form #25223 cut 1 (PR #25290) uses for non-readonly fields. Each read of `this.store` in the body reads the engine's current store, as it did inside the class. - No moved body writes `store`, or any other field: the generator refuses a `this.X =`, `op=`, `++` or `--` in a moved body, and found none. So no get/set pair (#25508) is needed. - **Seven fields go by value**: `executionLogs`, `logger`, `suspendedRuns`, `cacheOnlySuspensions`, `maxLogSize`, `runSummaryLog` and `consumedSuspensions`. The seat asked for proof that none is reassigned after construction: - The declarations (`engine.ts` at this head): - `private executionLogs: ExecutionLogEntry[] = [];` - `private readonly maxLogSize: number;` - `private readonly runSummaryLog: RunSummaryLogLevel;` - `private logger: Logger;` - `private suspendedRuns = new Map…` - `private cacheOnlySuspensions = new Set…` - `private consumedSuspensions = new Map…` - `git grep -nE "this\.(executionLogs|logger|suspendedRuns|cacheOnlySuspensions|maxLogSize|runSummaryLog|consumedSuspensions)\s*=[^=]" -- packages/services/service-automation/src` finds 3 lines, all in the constructor: `this.logger = logger;`, `this.maxLogSize = …` and `this.runSummaryLog = …`. - Across the repo, `git grep -nE "\.(executionLogs|suspendedRuns|cacheOnlySuspensions|maxLogSize|runSummaryLog|consumedSuspensions)\s*(=[^=>]|\?\?=)"` finds those 2 constructor lines and one test, `src/run-summary.test.ts:295`, which overwrites `runSummaryLog` with `'off'` before the run starts. - The host is built afresh on each call, and `recordLog`, the one body that reads `runSummaryLog`, is synchronous, so the body reads the value the test set. That test passes at head. - The one body that changes a field's contents is `recordLog`. It changes them in place, with `this.executionLogs.push(…)` and `.splice(…)` on the engine's own array, and never rebinds the field. - **Async.** The stubs of the four async methods (`listRuns`, `listRunsPage`, `getRun` and `readSuspendedRuns`) drop `async` and return the moved function's promise, as in slices 3 and 4. - The caller gets the very promise the body makes. - Dropping `async` is the only edit to those 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 - **Nothing is newly exported.** Every top-level name the bodies read is already exported from `engine.ts`: `buildRunTrigger`, `isTerminalRunStatus`, `ExecutionLogEntry`, `RunRecord`, `StepLogEntry` and `SuspendedRun`. - The module imports `buildRunTrigger` and `isTerminalRunStatus` by value, and the rest, `AutomationEngine` included, as types. - Both values are read only inside function bodies, at call time, and a comment above the import says so, as in slice 3. - **The module's other imports** use the same specifiers as `engine.ts`'s own imports: - the types `ExecutionStatus`, `FlowParsed`, `AutomationContext`, `AutomationResult` and `RunListResult`; - the values `summarizeRun`, `formatRunSummaryLine` and `describeThrownForLog`. - **One `engine.ts` import line changes.** `formatRunSummaryLine` was read only by `recordLog`, so `import { summarizeRun, formatRunSummaryLine } from './run-summary.js';` keeps `summarizeRun` alone. That is a moved line: the module carries it verbatim. - Every other name is still read in `engine.ts` too. - `engine.ts` gains the `./run-history.js` import, after the `./dag-traversal.js` one, with a 3-line comment naming its bodies. - **Authored text.** The module docblock and the host interface's docblock are new. The host docblock says why `store` is a getter. ## Proof of move-only - **Moved block.** This is `git diff --color-moved=plain --color-moved-ws=allow-indentation-change 673ebfd HEAD` over `engine.ts` and `run-history.ts`. It detects 655 of the 659 deleted lines and 684 of the 861 added lines as moved. - **Deleted, not moved (4):** the signature heads of `listRuns`, `listRunsPage`, `getRun` and `readSuspendedRuns`, each of which drops `async`. - **Added, not moved (177):** - the module's docblock, imports and host interface; - the nine pointer docblocks and rewritten function heads; - the `./run-history.js` import and its comment; - the four heads without `async`; - the nine stub bodies; - lines git does not pair with a moved block. - **An independent check.** A script compared base and head `engine.ts` against the new module. - **Module lines.** 698 of the module's 782 lines are in 9 verbatim runs of base `engine.ts`: the nine method texts, de-indented one level. The script names each of the other 84: - 62 head lines: the copyright line, the docblock, the imports and their comment, the host docblock and the 14-member host interface; - the 9 pointer docblocks; - 13 signature lines: nine `export function` heads, and four `this` parameter lines, each a `Pick` of the host. - **Deleted lines.** Every line deleted from `engine.ts` (659 in all, 6 of them blank) is among the module's lines, apart from the 4 async heads above. - **Added lines.** Of the 80 lines added to `engine.ts`, 66 are found nowhere in base: the narrowed `./run-summary.js` import, the `./run-history.js` import with its comment, the four heads without `async`, and the stub lines. The other 14, such as `logger: this.logger,` and `};`, are lines base already contains elsewhere. - **Literals.** No string or template literal in the nine method bodies spans a line, checked on the TypeScript AST of base `engine.ts`. So de-indenting changed no string value. - **`.d.ts`**: see the next section. - **Bundles.** The ESM and CJS export lists are identical: 39 names each at base and at head, and ESM equals CJS on each side. The bundle JS bytes differ only because code moved within the bundle. - **Tests.** `@objectstack/service-automation`'s suite passes the same at base and head, 199 of 199 files and 2628 of 2628 tests. ## `.d.ts` reading (ruling 6348891160, item 2) The merge base is `673ebfd4`, the head of `main`, so the merge-base reading and the base reading are one reading. - **Base** is `dist/`, built at `673ebfd4` in this container, in a worktree of that commit. - **Head** is `dist/`, built at this PR's commit. The package's dependencies were built by `turbo run build --filter=@objectstack/service-automation` (31 of 31 tasks), and both sides resolve the same built dependencies. The readings: - **Raw sha256**, `index.d.ts` and `index.d.cts`, which are 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**, with 19,626 lines on 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 line 4276 of the head `index.d.ts`, a private member whose body moved. Each control changes that one line, then runs the same normaliser: - `private recordLog;` made public (`recordLog;`) gives `0ec48a1a498558b9a4c85d40790ef91f63014441ec38485bc40b564ad81f8c3b`. - The same member renamed (`private recordLogRenamed;`) gives `ad22516615ca006846b45d71c4647406ba6465f5fd420778a8d709e5517473ca`. - 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 **One reader re-pointed.** `src/flow-activation-ledger.test.ts`'s `ENGINE_SOURCES` holds the #10243 grep pins, which are absence checks. It gains `./run-history.ts`, so a moved body is not a place the retired mechanism could come back unseen. That 1-line edit is the PR's third file. **Readers examined.** The base for each is `673ebfd4`, in a worktree of that commit. - **`check-tenant-audit-census`** (derived): output byte-identical to base. - It certifies 238 write call sites (162 decidable; 2 provably without tenant context, 29 more unreadable). - It holds 23 prose figures and matches every `UNTYPED_RECEIVERS` row. - Its `--self-test` output is identical too. - **`check-system-context-census`** (derived): output byte-identical. It finds 149 elevation read sites in 22 packages across 77 files, in 129 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. - 982 of 982 symbol anchors resolve. - No `packages/spec/liveness` row cites any of the nine methods. - **`check:platform-checklist`** (not derived; run by hand): output identical at base and head. 987 of 997 symbol anchors resolve; the other 10 are the same ones, 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. - **`check:adr-anchors`** (decision letters; run by hand at the merge base and the head): output byte-identical. It reads 84 anchored files, 52279 citations across 6684 files, and 1956 decision-letter citations. Neither `engine.ts` nor `run-history.ts` is on the anchored surface. - **`check:startup-registry-verdict`** (not derived; run by hand at both ends, and on the hand-gate list from this slice on, as the seat asked): 46 seams, 40 of them read-only, none recording a verdict, at base and at head. - Its `--list` is the same at both ends apart from line numbers. - None of its four engine bodies moves: `knownNodeTypes`, `getUnknownNodeTypeAudit`, `sealNodeTypeVocabulary` and `warnUnknownNodeTypes`. - The scanned file count is 3454 at base and 3455 at head; the extra file is `run-history.ts`. - **`path:line` citations.** The only one into this package's `engine.ts` is `service-automation/src/engine.ts:44`, in `packages/spec/scripts/liveness/evidence.test.ts`. It is a realm-attribution fixture that `scanEvidence` parses without reading the file. - Lines of `engine.ts` after base `:129` move down by 14 until the first moved body. - Lines after the last moved body move up by 579, since the file goes from 11,686 to 11,107 lines. ## Gates - **Derived.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` on this commit derives 59 families from the 3 changed paths: 1,522 changed lines, under the 3,000-line human-merge threshold. - All of them ran in this container on this commit, with 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. - **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. - **Package checks.** `@objectstack/service-automation`'s `vitest run` gives 199 of 199 files and 2628 of 2628 tests, the same at base and head. `tsc --noEmit`, and ESLint on the three files, pass. - **Open PRs**, re-listed before this PR was opened: 11, and none of them touches `service-automation/src`, the readers above or their ledgers. #25548 and #25545 edit `scripts/check-dispatcher-error-vocabulary.mjs` and `scripts/check-system-context-census.mjs`, but only rows for `plugin-auth` and `plugin-security`. The Version Packages PR #24876 touches only the package's `CHANGELOG.md` and `package.json`. `main` has moved to `ab596f82` (#25547, a `cli` test), which touches none of these files. ```text node scripts/check-ci-filter-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-adoption.mjs :: exit 0 node scripts/check-comment-mask-adoption.mjs --self-test :: 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 :: exit 0 node scripts/check-keyed-text-bounds.mjs --self-test :: exit 0 node scripts/check-platform-object-tenancy-census.mjs :: exit 0 node scripts/check-platform-object-tenancy-census.mjs --self-test :: exit 0 node scripts/check-plugin-teardown-shape.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0 node scripts/check-position-name-fold-loaders.mjs :: exit 0 node scripts/check-position-name-fold-loaders.mjs --self-test :: exit 0 node scripts/check-registry-log-declared.mjs :: exit 0 node scripts/check-registry-log-declared.mjs --self-test :: exit 0 node scripts/check-rest-log-spy-declared.mjs :: exit 0 node scripts/check-rest-log-spy-declared.mjs --self-test :: exit 0 node scripts/check-system-context-census.mjs :: exit 0 node scripts/check-system-context-census.mjs --self-test :: exit 0 node scripts/check-tenant-audit-census.mjs :: exit 0 node scripts/check-tenant-audit-census.mjs --self-test :: exit 0 node scripts/check-undeclared-dep-imports.mjs :: exit 0 node scripts/check-undeclared-dep-imports.mjs --self-test :: 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 - The slice is the seat's agreed list, with no judgement call outside it. - This is the first host getter in the series: `store`, in the #25223 cut 1 form. No get/set pair is needed. - After this slice, the remaining-bodies table leaves about 1,420 movable lines in bodies over 3 lines. Those are slices 6 and 7, and they need no ruling. The 21 site-bearing bodies, and the five startup-registry bodies tied to `registerFlow`, wait on the 33-site ruling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Mnj4nkWuhJ18xEdZvb2cjE Co-authored-by: Claude <noreply@anthropic.com>
1 parent b531016 commit 36cac5a

3 files changed

Lines changed: 862 additions & 660 deletions

File tree

0 commit comments

Comments
 (0)