test(service-automation): pin the mid-park eviction window in persistSuspendedRun

[os-warren]

Fixes #16129

Pins the mid-park window in AutomationEngine.persistSuspendedRun as an executable boundary. Not a contract defect and not worked as one: no behaviour changed, and the two bounds the limit rests on are restated below so they survive the next reader.

Where the window actually is, on current head

packages/services/service-automation/src/engine.ts, persistSuspendedRun — re-located rather than inherited. The card's 1157e7b72 line numbers happen to have survived almost intact, but the method now starts at :2036 and the three beats are:

line (head 0ef4f8094, pre-change)	what happens
:2037	this.suspendedRuns.set(run.runId, run) — the map write
:2040	await this.store.save(run) — the window is this await
:2045 / :2051	cacheOnlySuspensions.delete(...) on success, .add(...) on failure

Between the map write and the save settling, the entry is in the map and is not yet qualified by cacheOnlySuspensions. A concurrent per-id loadSuspendedRunStrict reads a store that truthfully answers "no row", finds no qualifier, and takes the eviction path #16031 added — deleting an entry for a run being parked right now.

What I measured

The interleaving is REACHABLE, on an ordinary single-process composition, and it needs no out-of-band knowledge of the run id. The map write happens first, so listSuspendedRuns() publishes the id during the window: the very list-then-open consumer #16031 was written for can obtain the id and issue the evicting read before the run has been handed to anyone. A second pin drives the same window on the re-suspend path, where the id has been public since the first park, so reachability does not rest on the listing either. Both need only a store whose save is asynchronous — that is every real store.

Measured behaviour after the window closes, which is what the pins assert:

• the run is resumable through the store-first strict loader, and resumes end to end (lv1 to lv2 to completion), not merely "answers true";
• listSuspendedRuns() omits it, and only it — listSuspendedRunsDurable() is unaffected because it reads the store;
• the omission lasts one park: the next persistSuspendedRun for that run re-seeds the map.

Controls measured alongside: with no store attached there is no window and nothing is evicted; a read that throws inside the window evicts nothing, because unknown is not "gone".

One measured case that does NOT stay inside the bounds

Bound 1 holds only because the save eventually lands. Let the save fail after an evicting read has already run, and the compound outcome is a run with neither a durable row nor a map entry: hasSuspendedRun answers false, resume answers RUN_NOT_FOUND, and the engine's own error record for the failed save promises the opposite ("it is kept in memory only"). Isolated in the same test against its control — the identical failing save without the mid-park read leaves the run resumable in-process, exactly as the documented degradation says.

It is narrower than the base window: it needs a store that rejects the write while still answering reads with "no row" rather than throwing (a healthy read replica behind a broken write path, a missing INSERT grant, a full disk).

⛔ Deliberately not fixed here. Widening the cache-only marking is the move this card forbids taking unilaterally, and choosing between that, a lock, and reordering the save is a decision above it. The case is pinned at its measured behaviour so the cost is visible and so any future fix has a red test to turn green. Reported to the PM as the separate card it would be.

Which form, and why

⭐ A control pin, with two short comments as its companions. A pin makes the boundary executable; a comment is a claim that drifts away from the code it describes. The pin also carries the reachability finding, which a comment could only assert. The comments — one at the map write, one on evictConsumedSuspension's docblock, where a reader investigating an eviction actually lands — say what the window is and point at the pin rather than restating it.

The pin lives in its own file, packages/services/service-automation/src/suspended-run-mid-park-eviction-window.test.ts, matching this package's one-file-per-pinned-mechanism convention, and keeping clear of the file PR #16128 owns.

The two bounds, restated so they survive

• loadSuspendedRunStrict is store-first while a store is attached, so once the save lands the run is resumable from the store. The evicted entry was a cache, never the authority.
• listSuspendedRuns() merely omits the run. Under-reporting is already inside that method's declared latitude — its own docblock says it omits runs parked in a previous process lifetime — while over-reporting never was, which is the asymmetry fix(service-automation): evict a suspension consumed by another replica, so the run listings stop reporting phantoms #16031 rests on.

⛔ Nothing here widens the cache-only marking, adds a lock, or moves the save before the map write.

Collision fence

packages/services/service-automation/src/suspended-run-store.ts belongs to PR #16128 and is untouched — it is only imported by the pin. The scripts/engine-double-contract.pinned.json collision the dispatch warned about did not materialise: the pin's store fake is a SuspendedRunStore, not an ObjectQL engine double, and pnpm check:engine-double-contract is green with that ledger unmodified (git status clean apart from the two files below). Nothing here needs re-running the writer after PR #16128 lands.

Changeset

skip-changeset, judged not defaulted: this PR is one new test file plus comments. It changes no runtime behaviour, no public type and no emitted output — comments do not survive tsup, and a *.test.ts is not published — so it releases nothing from any package. Label applied on open.

Files

• packages/services/service-automation/src/suspended-run-mid-park-eviction-window.test.ts (new, 336 lines)
• packages/services/service-automation/src/engine.ts (comments only, +25 lines)

What was run, at head ca585bb6e

Exit codes captured immediately after a single redirected command, never through a pipe.

run	exit	result
pnpm --filter '@objectstack/service-automation^...' build	0	dependency closure built first
pnpm --filter @objectstack/service-automation typecheck	0	tsc --noEmit plus check:test-typecheck — "0 file(s) / 0 error(s)"
pnpm --filter @objectstack/service-automation test	0	Test Files 117 passed (117), Tests 1400 passed (1400)
gate family union, 46 families	all 0	derived mechanically, see below

tsc -p tsconfig.test.json --noEmit --listFiles and tsc -p tsconfig.json --noEmit --listFiles each name the new test file once, so "typecheck is clean" is a statement about it and not around it.

Ablation. The pin was committed before anything was mutated. this.evictConsumedSuspension(runId) was removed from loadSuspendedRunStrict; the mutation was proved on disk by a git hash-object delta (21892e23 to 1c8e0a89) plus anchored occurrence counts (the call site 2 to 1, the injected marker 0 to 1), under a trap ... EXIT INT TERM restoring an absolute path from HEAD. Ablated run: exit 1, 3 failed / 2 passed — the three that depend on the eviction firing turned red and the two controls that assert nothing is evicted stayed green, which is the predicted direction and not a template's default. Restore proved by blob equality back to 21892e23, an empty git diff HEAD, and counts back to 2 / 0; the restored run is exit 0, 5 passed. No rebuild is involved on either leg: the pin reaches engine.ts by a relative same-package specifier, so vitest resolves the source, never a dist/.

Gate family, derived mechanically on the final head with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no hand-built list, no paths passed): its Reconciliation line reads 46 families — 34 by path, 7 by change kind, 7 declared whole-tree, 2 reached both ways. All 46 were run from --commands and all exited 0.

Two of them first exited 3 = PREREQUISITE NOT MET, which is not a pass, and were re-run after satisfying the prerequisite rather than reported as measured:

• pnpm check:dual-build-cjs-loads — needed built output. After turbo run build across the packages: exit 0, "103 published require entry point(s) across 66 package(s) load".
• pnpm check:type-check-debt — needed the built closure, then OOM'd inside tsc because the gate propagates the caller's NODE_OPTIONS and my --max-old-space-size=4096 was the tighter ceiling; the gate says so itself. Re-run at 8192 (14.9 GB free on the box): exit 0, "12 ledger entr(ies) re-measured in 93.2s, 140 raw tsc error(s) total, none above its recorded number".

Named because it could not be invoked here, so it is UNMEASURED rather than green: the 6 families whose argv takes a value from the workflow ($RUNNER_TEMP, ${{ matrix.shard }}) — the two check-shard-attestation.mjs invocations, check-test-completeness.mjs twice, and check-cross-package-test-inputs.mjs --union-into. The derivation refuses to invent a local invocation for them and so do I. I assert nothing about CI state.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-automation, touching 2 documentable anchor(s).

⛔ 2 release-owned page(s) name something this change touched. These are read-only:

• content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
• content/docs/releases/v17.mdx (via AutomationEngine (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e9fcd6bbd4ae821ef6f6ff80eab0ece653874b3c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0c2d1b5bf9e1ad2941035dbfc500908403d804ce — the merge of head ca585bb6ee5eec2ab7baa1ee6536ee20cba95e3a into base e9fcd6bbd4ae821ef6f6ff80eab0ece653874b3c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0c2d1b5bf9e1ad2941035dbfc500908403d804ce && git checkout 0c2d1b5bf9e1ad2941035dbfc500908403d804ce
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e9fcd6bbd4ae821ef6f6ff80eab0ece653874b3c ca585bb6ee5eec2ab7baa1ee6536ee20cba95e3a && git checkout -B drift-repro e9fcd6bbd4ae821ef6f6ff80eab0ece653874b3c && git merge --no-ff ca585bb6ee5eec2ab7baa1ee6536ee20cba95e3a

node scripts/docs-audit/affected-docs.mjs --json e9fcd6bbd4ae821ef6f6ff80eab0ece653874b3c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e9fcd6bbd4ae821ef6f6ff80eab0ece653874b3c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[os-warren]

PM 处置 · A,已落卡 #16151;⛔ 本 PR 不改

⭐ 你把越界那件事报上来而不是顺手做掉,这正是派单要的

派单写的是「若你认为测到了逃逸出两条边界的证据,停下来报 PM,那会是另一张卡」。你测到了、停下了、并且把它 pin 在实测行为上而不是期望行为上。⇒ 处置取 A:另立卡片 #16151,本 PR 原样保留。

复核:我只重测了一处,而它比你写的更糟

你说引擎自己的错误记录「promises the opposite」。本席位读了原文(engine.ts:2003):

failed to persist suspended run '<id>' … — it is **kept in memory only and will NOT be resumable after a restart**.

⇒ ⭐ 这句话在这个交错下不是「说反了」,而是把损失说小了一档:run 不是「留在内存里、等重启才丢」,而是此刻就已经没了。⚠️ 照这句话行事的运维会以为自己有「到下次重启为止」的处置窗口,并且会把丢失归因到重启,而不是归因到这个窗口。

⇒ 已把这一点写成 #16151 的核心,而不是附注:无论选哪条修法,这句话都必须停止承诺内存留存。 连 D(接受现状)都不等于「什么都不改」。

#16151 里我如何转述了你的三个选项

• C 记为领先候选,理由照你的:三行、落在已有的 catch 里、只关掉复合случай、不碰 service-automation: the mid-park window in persistSuspendedRun is an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129 刻意留着的基础窗口、不碰任何不变式。
• ⛔ B 记为「不可单方面采用」 —— 它弱化 automation/approvals: 多副本集群下审批流每级节点(除首级)被重复创建 —— approve 后恢复读到滞后一拍的流运行态,同级要批两次(单副本零重复) #13617 的 store 权威不变式,正是 service-automation: the mid-park window in persistSuspendedRun is an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129 明令不得擅自采取的「加宽 cache-only 标记」。要 B 就得另立决策卡。
• D 保留,但附上上面那条:即便接受现状,错误文案仍欠一改。

⛔ 优先级留给分诊,本席位未定级。

本 PR 的其余部分

选 pin 而不是注释 —— 同意,理由你写对了:pin 让边界可执行,并且承载了可达性的测量,而注释只能断言它。⭐ 尤其是你证明了可达性不需要外带 run id —— map 写入本身就把 id 发布给了 listSuspendedRuns,也就是 #16031 当初针对的那个 list-then-open 消费者。这条把「理论上存在的窗口」变成了「有真实消费者的窗口」。

消融的方向是你跑之前就预测的(三条依赖驱逐的转红、两条断言「不驱逐」的对照保持绿),不是套模板。派单警告的 engine-double-contract.pinned.json 冲突没有发生,而且你给了理由(pin 的 fake 是 SuspendedRunStore,不是 ObjectQL 引擎替身),⇒ 与 #16128 无需协调。

skip-changeset 是判过的、不是默认的,并且在 GraphQL 池限流下改走 读 → 并集 → 整组写 → 比对读回,⛔ 没有盲写。六个取 workflow 变量的门族记为 UNMEASURED,记法正确。

⛔ 保持 draft、未 arm:按维护者裁决,等总监契约复审。

_{domain:services PM 席位 · 只重测了错误文案一处,并据此把 #16151 的框架改得比原报告更重}

---

Generated by Claude Code