Uh oh!
There was an error while loading. Please reload this page.
fix(service-automation): route the last three suspended-run readers through the shared loader - #14650
Conversation
…ers through the shared loader `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map over the shared `SuspendedRunStore`, which #13617 had already made authoritative for the resume path. Each now takes one answer to "where is this run parked", with the degrading or strict loader chosen per site so the recorded #4632 / #6299 degradation posture is preserved by choice rather than re-derived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
… module `AutomationContext` is declared in `@objectstack/spec/contracts` and re-imported by `engine.ts` as a type — it is not re-exported from there, so importing it from `./engine.js` added a fourth error (TS2459) to a package whose type-check debt ledger is frozen at 3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin da191b53496857088142d233a3b34da26653db98 && git checkout da191b53496857088142d233a3b34da26653db98
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 20b883918aa787e65299e72b85c2b3396aa2006f 89f5f8d15dacac7b65cd1198ff48339d5bd36a18 && git checkout -B drift-repro 20b883918aa787e65299e72b85c2b3396aa2006f && git merge --no-ff 89f5f8d15dacac7b65cd1198ff48339d5bd36a18
node scripts/docs-audit/affected-docs.mjs --json 20b883918aa787e65299e72b85c2b3396aa2006f
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#14332
cancelRun,failAncestorsandlistSuspendedRunsDurablewere the three readers of suspended-run state that #13617 deliberately left on the per-processsuspendedRunsmap. All three now take one answer to "where is this run parked", through the existingloadSuspendedRun/loadSuspendedRunStrictpair — with the degrading or strict loader chosen per site, so each site's recorded #4632 / #6299 degradation posture is preserved by choice rather than re-derived.Ruling of record
No maintainer ruling is owed on this card — triage graded it dispatchable and fixed its shape. Quoted verbatim from triage (14332#issuecomment-5503565404):
The per-site loader judgment
This is the deliverable, not the mechanical replacement. Each site's docblock now carries the same judgment in prose.
cancelRunloadSuspendedRunStrict)false— "already terminal / unknown", its documented idempotent successerrornullunder its own best-effortwarnand silently downgraded the #4632 verdict. Its degradation decides a WRITE, so the strict form plus a local catch is the only shape that preserves both the return value and the level. Same shaperesumeInternaluses.failAncestorsloadSuspendedRun)if (!parent) return;), unchanged.catch(() => null)did. One thing gained: that silent swallow is now recorded, at the loader's declared best-effortwarn.listSuspendedRunsDurablewarn(#4632 FUNCTIONAL)store.list()is capped and best-effort (ObjectStoreSuspendedRunStorereads at most 1000pausedrows) and the same merge is reached on the degraded path, so "absent from the list" is not the "the store answered and has no row" thatloadSuspendedRunStrictrests on. The durable row wins a collision; map entries the listing does not carry are still included, deliberately.Site 3's comment
In-memory entries win — they are the freshest copyis corrected in place, together with the docblock sentence that repeated it (The in-memory cache takes precedence on id collisions).Premise checks — verified on
origin/mainat 13bf05d, before the first editAll hold;
premise_still_valid: true.cancelRunreadthis.suspendedRuns.get(runId) ?? nulland only thenstore.load— map-first. HOLDS.failAncestorswalked$parentRunIdwiththis.suspendedRuns.get(parentId) ?? (this.store ? await this.store.load(parentId).catch(() => null) : null). HOLDS.listSuspendedRunsDurablemerged the durable list under the map with the commentIn-memory entries win — they are the freshest copy. HOLDS.loadSuspendedRunStrictis store-authoritative with thecacheOnlySuspensionsfallback for a failed durable save;loadSuspendedRunis its degrading form; thegetSuspendedScreendocblock still statesone loader, two callers. HOLDS.packages/spec/**andcontent/docs/releases/**untouched. HOLDS — the diff is 3 files:engine.ts, one new test file, one changeset.PM hypotheses
suspended-run-store-consume-log-cause.test.ts, 20 tests) stay green untouched. Two consequences are stated in the docblocks, the changeset and the pins, not left to be discovered: (a)cancelRun— while a store is configured this process's map is no longer an answer, so a store outage reaches the existingerrorrecord even for a run this replica holds, where the old cache-first read cancelled from the local snapshot; that snapshot is the defect. (b)failAncestors— a store failure during the ancestor walk was swallowed in total silence by.catch(() => null)and is now recorded by the loader atwarn. No newerror-level site anywhere (the [Decision] plugin-sharing's refused-backfill report lands atwarnwhere AGENTS.md puts it aterror— and the card that was supposed to carry the level is CLOSED #13398-class ruling).git grep -n 'listSuspendedRunsDurable' -- 'packages/**' 'examples/**' 'apps/**' ':!*.test.ts' ':!*.spec.ts'returns onlyengine.ts's own definition and comments plus CHANGELOG prose — no consumer. Positive controls on the same command shape:hasSuspendedRunreturns a real non-test consumer (packages/plugins/plugin-approvals/src/approval-service.ts:2704), andlistSuspendedRunsreturns its spec-contract declaration (packages/spec/src/contracts/automation-service.ts:589) — so the scan does find consumers where they exist.no. Declared from the actual diff:git diff -U0 origin/main...HEAD | grep exportmatches exactly one line, and it is prose inside the changeset (No signature, export or return-shape change...), not an export statement. No new exported symbol, no signature change on the three readers, no accept set moved. Changeset ispatch. Noneeds:contract-review.engine.tsis confined tocancelRun,failAncestors,listSuspendedRunsDurableand their docblocks.Tests
New pins:
packages/services/service-automation/src/multi-replica-suspended-run-readers.test.ts(11 tests), a sibling of the #13617 harness rather than more cases inside it. Reason recorded in its docblock:multi-replica-resume-staleness.test.tscarries a REVERT-PROOF ledger for one named mutation and its measured 4 red / 4 green split; adding these cases would falsify those counts and merge two different mutations' revert-proofs into one statement. The two files share only the two-engines-over-one-store shape.Every site-1 and site-2 pin asserts on the
onSuspensionReleasednotification (which node executor was told its pause is over), never on the row: the row delete is by id and is correct either way, which is the whole point of the card.All runs below were taken at commit da4db89f5 and carry to the final head 89f5f8d because the two commits have a byte-identical tree: both are
514564fde37f75fd96c8f58e09ae139bc5ce0801, andgit diff --stat da4db89f5 89f5f8d15prints nothing. The extra commit exists only because this branch had already been pushed when a type-only import was corrected, and force-pushing is forbidden — so the amend was re-landed as a child commit rather than rewritten. Everything ran underscripts/pm/os-verify-lock.sh, exit captured before any pipe.Full package suite —
pnpm --filter @objectstack/service-automation test:Type-check — the package has no
typecheckscript (it is a shrink-only DEBT ledger entry, frozen at 3), so apnpm --filter ... typecheckwould have matched zero scripts and exited 0 having measured nothing. Measured instead with the dependency closure built (pnpm --filter '@objectstack/service-automation^...' build, the world the ledger is measured in),tsc --noEmit -p tsconfig.json --listFiles:Exactly the ledger's frozen 3, all pre-existing, none in the changed files;
--listFilesconfirms the new test file is in the program (this run is the reason a fourth error — a type imported from the wrong module — was caught and fixed before the head was final).Repo-wide lint — no narrowing was needed, the whole population ran:
Gates
Derived on the final head from the merge-base change set, not a hand-written list:
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsatda4db89f5(--repoasserted and held against this checkout'sorigin) — 36 commands over the 3 changed paths. The derivation is a property of the tree, which is byte-identical at the final head89f5f8d15(see Tests).33 green, 0 red, 3 NOT MEASURED. The three are prerequisite refusals (exit code 3, which each script's own verdict text distinguishes from a finding's 1), not failures:
node scripts/check-test-completeness.mjs—PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named.Its own text says to record NOT MEASURED when the family is run locally.pnpm check:dual-build-cjs-loads—PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/.(49 packages unbuilt; needs a fullpnpm build.)pnpm check:type-check-debt—PREREQUISITE NOT MET ... --re-measure cannot run: 35 workspace dependenc(ies) ... have no built type entry point on disk. Its siblingcheck:type-check-coverageran green (OK — 69/79 workspace packages type-checked ... 10 in the DEBT ledger), and the targetedtscreading above answers this gate's substantive question for the one package this PR touches, in the built-closure world the ledger uses.Also green outside the derived family:
pnpm check:nul-bytes(OK (scanned 7992 text file(s) ... no raw ASCII control bytes)), plus a direct control-byte scan of both changed files.Merge preflight before opening:
git merge-tree --write-tree --name-only origin/main HEADlists no conflicting paths —content/docs/permissions/system-context.mdxis not among them, so no regeneration is owed.Ablation — three, one per site, on the committed tree
Each mutation restores that site's map-first read, is line-neutral, is proven on disk by anchored occurrence counts before the run, and is restored with
git checkout HEAD -- ABSOLUTE_PATHinside atrap ... EXIT INT TERM, the restore proven bygit hash-objectagainst the HEAD blob and an emptygit diff HEAD. Because the pins import./engine.jsrelatively (vitest resolves that tosrc/engine.ts, not the package'sdist/throughexports), no rebuild is interposed; the reds themselves are the proof the mutation reached the code under test.HEAD blob for
engine.ts=f2c33d8b57cea0dce4054de3b8f1ee0ac9b88e6a; every restore leg read back the same hash and an emptygit diff HEAD.run = this.suspendedRuns.get(runId) ?? await this.loadSuspendedRunStrict(runId)THE BUG: a stale replica must not tell the wrong node executor its pause is over(teardown nameslv1while the run is parked atlv2) andNEW REACH(a map hit means the unreadable store is never read, so the cancel proceeds from the snapshot)const parent = this.suspendedRuns.get(parentId) ?? await this.loadSuspendedRun(parentId)THE BUG: a stale parent must not be failed at a node it has already leftand the unreadable-ancestor control, red for site 1's reasonbyId.has(r.runId)guard so the map overwrites againTHE BUG: the durable row wins an id collision, the stale in-process copy does not(the listing reportslv1, one level stale)The controls that stay green under all three are the postures each site had before: the no-store cases where the map IS the authority, the failed-durable-save cases the store's silence says nothing about, and the unlistable store's documented short list.
One reading was voided rather than reported: the first attempt at the site-1 mutation used a one-line anchor that occurs twice in
engine.ts(the other inresumeInternal), the guard refused at anchor count 2, and the run was re-anchored on a unique three-line block. A second attempt was voided by the on-disk proof itself before any test ran. Both voided attempts are named here because a silently re-run ablation is the defect one layer up.Scope
Diff is 3 files. Untouched as required:
packages/spec/**,content/docs/releases/**,skills/**, the store implementation, PR #14567's subflow delegation block, and the region #14333 is queued for. No out-of-scope findings were filed — nothing outside the card's own surface was found.🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code
Generated by Claude Code