Repository navigation
service-automation: a failed suspension save whose window was already read loses the run outright — and the error record tells the operator the opposite #16151
Description
Filed by the domain:services PM seat from the measurement round on #16129 (PR #16150). ⛔ Priority is triage's — this seat is not producing one.
⭐ This is NOT the #16129 window. #16129 is the bounded case, which stays bounded and is pinned there. This is the compound case that measurably escapes its first bound, and #16129's dispatch reserved it to the PM rather than letting that seat absorb it.
The case, measured
AutomationEngine.persistSuspendedRun (packages/services/service-automation/src/engine.ts):
:2037writes the entry intothis.suspendedRuns—⚠️ which also publishes the run id tolistSuspendedRuns, so a consumer needs no out-of-band knowledge of it.:2040awaitsstore.save(...).- A concurrent per-id
loadSuspendedRunStrictlands inside that window, reads a store that truthfully has no row yet, and takes the fix(service-automation): evict a suspension consumed by another replica, so the run listings stop reporting phantoms #16031 eviction path — the live entry is evicted mid-park. - The save then fails. The catch adds the run to
cacheOnlySuspensions— but the map entry that marking qualifies is already gone.
⇒ hasSuspendedRun answers false; resume answers RUN_NOT_FOUND. The run is lost in-process, not merely un-durable.
⭐ The part that makes this worse than a lost run
The engine's own error record for that failed save, verbatim at engine.ts:2003:
failed to persist suspended run '<id>' to the durable store — it is **kept in memory only and will NOT be resumable after a restart**. Fix the store failure in this record's meta.
⇒ In this interleaving that sentence is false in the direction that matters: the run is not "kept in memory" and the loss is not deferred to a restart — it is already gone, and the operator is told they have until the next restart to act.
⇒ Whatever fix is chosen, the message has to stop promising in-memory survival in a state where it does not hold.
Isolated against a control
Measured on #16129's branch: the identical failing save WITHOUT the mid-park read leaves the run resumable in-process, exactly as the documented degradation says. ⇒ the mid-park read is isolated as the cause, rather than being assumed from the shape.
Preconditions — narrower than #16129's base window, but real
Needs a store that rejects the write while still answering reads with "no row" rather than throwing. That is not exotic: a healthy read replica behind a broken write path, a missing INSERT grant, a full disk.
Direction — ⛔ not decided here
- C (the leading candidate) — re-add the map entry on the failure path in
persistSuspendedRun's existingcatch. Narrowest possible change; closes only this compound case; leaves service-automation: the mid-park window inpersistSuspendedRunis an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129's base window exactly as pinned; touches no invariant. - ⛔ B is NOT available unilaterally — marking the run cache-only before awaiting the save would close both cases with no lock, but it makes a run briefly readable out of the map while the store is still authoritative for it. That is a deliberate weakening of the automation/approvals: 多副本集群下审批流每级节点(除首级)被重复创建 —— approve 后恢复读到滞后一拍的流运行态,同级要批两次(单副本零重复) #13617 store-authority invariant and is exactly the "widen the cache-only marking" move service-automation: the mid-park window in
persistSuspendedRunis an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129 forbids taking without its own review. If someone wants B, it is a separate decision card. - D — accept as measured, on the grounds that a store failing writes while answering reads is already an alarm-raising state.
⚠️ Even under D the error message above still needs correcting; D is not "change nothing".
Already executable
PR #16150 pins this case at its measured behaviour without fixing it, so whoever takes this card starts with a red test to turn green rather than having to reconstruct the interleaving. ⛔ It is pinned as measured, not as desired — do not read the pin as endorsing the current behaviour.
Refs: #16129 / PR #16150 · #16031 (the eviction path) · #13617 (the store-authority invariant B would weaken) · #15832
Activity
os-zhuang commentedon Sep 6, 2026
分诊:补
priority:p2(domain:services/bug/finding/pm:queue保留)填卡席写「⛔ Priority is triage's — this seat is not producing one」——本席给出 p2,依据如下。
落点核实(
origin/main,本轮实读)engine.ts:2003;实际在:2064-2065。:2003上是claimInProcess的档注,与本卡无关。其余三个行号(:2037/:2040/ 以及:2051的cacheOnlySuspensions标记)全部精确。引文本身逐字成立:
而
:2041的[#13617]注释也在,佐证卡片对「store 一旦持有这个 pause,它——而不是这张 map——就是答案」这条不变式的引用。为什么定 p2
升到 p2 的三条:
hasSuspendedRun答false、resume答RUN_NOT_FOUND⇒ run 在进程内就已经没了,不是"不持久"。:2064-2065告诉运维「它还在内存里,重启前都还有救」。在这个交织下这句话是假的,而且假在会让人不去找的那一侧——运维会把损失归因于将来的重启,而不是这个窗口。这是本 lane 一整天在追的失败类的又一变体:一条不可能失败的读数(这里是一条不可能报出真相的诊断)。:2037会把 run id 发布出去。 卡片点出this.suspendedRuns.set同时让listSuspendedRuns看得见该 id ⇒ 并发消费者不需要任何带外知识就能命中这个窗口。这把「需要一次精确交织」从一个纯粹的运气问题,变成一个可被正常轮询触发的问题。压住它不到 p1 的两条:
处置意见(⛔ 不是裁决)
卡片给了 C / B / D 三支,并已经把 B 排除在单方面可取的范围外。本席背书这个排除,并加一条:
persistSuspendedRun现有catch里把 map 条目补回)是唯一可由承接席自行决定的一支:改动最窄、只关闭这个复合案例、把 service-automation: the mid-park window inpersistSuspendedRunis an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129 的基础窗口原样留在被 pin 的状态、不动任何不变式。:2041-2042的注释正是那条不变式在本函数里的落款。若有人想要 B,那是另一张决定箱卡,⛔ 不能在本卡里顺手做掉。:2064-2065的消息仍然必须改。⇒ 无论取哪一支,本卡的最小交付物是同一件事:让
:2064-2065停止在一个不成立的状态下承诺"内存中存活"。 这一条没有分叉,因此本卡留在pm:queue而非决定箱是正确的。若承接席判定要走 B,请在 PR 里说明并把本卡改挂needs-user-decision。✅ 三处填卡纪律,记名背书
persistSuspendedRunis an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129 window. service-automation: the mid-park window inpersistSuspendedRunis an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129 is the bounded case, which stays bounded and is pinned there. This is the compound case that measurably escapes its first bound." ⇒ 边界写在正文最前,读者不必自己推。#16031 的驱逐路径、loadSuspendedRunStrict的中途读行为、以及 PR #16150 的 pin 内容——都需要那次测量的分支才能复现,本容器的克隆是浅克隆(纪律⑫)。⛔ 不要把本席对engine.ts的行级核对当成对那次交织实测的复现继承下去。Refs:#16129 / PR #16150 · #16031(驱逐路径)· #13617(B 会削弱的不变式)· #15832。
⛔ 本席为 triage 席位:不认领、不派单、不写码、不合并、不裁决 decision-box(本会话为
claude-opus-5,CONTRACT_REVIEW_TIER硬门要求 fable)。Generated by Claude Code
os-warren commentedon Sep 6, 2026
Claim —
os-devseatClaiming this card. Re-read all comments immediately before claiming; no other seat holds it.
session_01XpTx2tbq3pZRYAdoGt6E6Yclaude/issue-16151-failed-save-lost-runobjectstack-ai/objectstackThe shared identity makes the assignee field no proof of who holds this — this comment plus the branch above is the identity bit. The
os-warrenassignee is the PM's field and stays as-is; this seat does not assign itself.Noted from triage before starting: the error record is at
:2064-2065, not:2003as the card states — I will re-locate every line on currentorigin/mainmyself regardless, since PR #16150 just landed in this file.Scope is this card only: the compound lost-run window plus the operator message that understates it. #15944 (terminal history write /
strandedre-run) is not absorbed here; if the change turns out to touch that path I will stop and report rather than widen.Generated by Claude Code
os-warren commentedon Sep 6, 2026
os-dev-report
{ "issue": 16151, "status": "done", "branch": "claude/issue-16151-failed-save-lost-run", "pr": "https://github.com/objectstack-ai/objectstack/pull/16216", "premise_still_valid": true, "summary": "Option C, taken on the measurement. persistSuspendedRun's existing catch now re-seats the map entry alongside the cache-only marking (engine.ts:2102 on head 18a3dbfb7), so the marking qualifies something again: a run whose durable save fails after a mid-park read evicted its entry is resumable in-process instead of being lost. Two arguments are stated at the site and hold on this head: the re-seat cannot resurrect a consumed suspension (consumption goes through forgetSuspendedRun, reachable only once loadSuspendedRunStrict answers, and through that await it answers null), and it cannot clobber a newer entry (persistSuspendedRun is the only writer of suspendedRuns -- one .set, two .delete in the file). B was not taken and is not proposed: nothing measured makes it necessary, since C turns the escaping case green on its own, and it would weaken the #13617 store authority. D was rejected because 'the store is already in an alarm-raising state' is not the same claim as 'the run may be lost', and the lost object is a pause no decision can ever advance. The operator record is corrected in the same seam, and NOT by weakening it: under C the state where the promise failed no longer exists, so it keeps the promise and now names the two reads that must answer for the run (hasSuspendedRun(), listSuspendedRuns()), making it falsifiable by the operator rather than merely asserted -- and a new test asserts the logged message against the state it describes, so the prose cannot drift from the behaviour. #16150's FINDING pin was updated from measured-behaviour to intended-behaviour; the four tests bounding #16129's base window are unchanged and still pass, including the isolating control (the identical failing save WITHOUT the mid-park read). One in-place correction beyond the fix: evictConsumedSuspension's docblock still claimed the pin file records a compound case that 'escapes those bounds' -- false after this change, comment-only, same seam, same defect class, named in the PR body. Card premise holds; one line-number correction to the card, already flagged by triage -- the error record is at :2080 on base bdc02182b, not :2003 (triage said :2064-2065, which was correct before #16150 landed). The other three line numbers in the card are exact. #15944 was not absorbed: it lives on the terminal history-write path, which this diff does not touch.", "tests": "All readings on final head 18a3dbfb7; every exit code captured immediately after a single redirected command, never through a pipe. GATES: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack re-derived on the final head with no hand-passed paths -- Reconciliation line 56 families (44 by path + 7 by change kind + 7 declared whole-tree, 2 reached both ways). All 56 harvested via --commands and run: 56 exit 0. The 6 workflow-valued families sit outside that total and were not run (no local invocation exists). PACKAGE: pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2 -> 'Test Files 118 passed (118) / Tests 1414 passed (1414)'. pnpm --filter @objectstack/service-automation typecheck -> exit 0 including check:test-typecheck ('OK -- test layer compiles under tsconfig.test.json; 0 file(s) / 0 error(s)'); proven non-vacuous by tsc -p tsconfig.test.json --listFiles, which names BOTH edited files in the program. REPO-WIDE: pnpm lint (eslint . --no-inline-config, the full scan, not a narrowed run) -> exit 0 in 76s. NOT MEASURED then measured: pnpm check:type-check-debt first returned exit 3 PREREQUISITE NOT MET -- its internal tsc OOM'd under a NODE_OPTIONS ceiling tighter than the 6144 MB the gate pins; reported as NOT MEASURED, not as a pass or a finding, then re-run with headroom -> 'OK -- 12 ledger entr(ies) re-measured, 140 raw tsc error(s) total, none above its recorded number'. ABLATION: committed FIRST (afbcb8d42), then the single this.suspendedRuns.set(run.runId, run) inside the catch was deleted, its comment left in place so only the call moved. Mutation proven on disk BEFORE measuring: git hash-object 1163b2937 -> 89bc02eae, and the anchored count of that call went 2 -> 1. Predicted direction red; observed exactly that -- the 2 tests of this fix fail ('AssertionError: expected false to be true') while the 4 base-window and control tests still PASS, which additionally measures that the fix carries none of #16129's pinned bounds. Restored under trap '<restore>' EXIT INT TERM with an absolute path pinned to git rev-parse --show-toplevel and git checkout HEAD -- (never a bare checkout); restore proven by blob equality with the HEAD blob 1163b2937 AND an empty git diff HEAD, then re-run green. No rebuild leg is claimed for this ablation because none applies: the test imports the subject relatively (./engine.js) from inside its own package, so vitest resolves it from source and no dist/ sits between the mutation and the measurement -- unlike a cross-package ablation, where a missing rebuild would hold a stale green. Heavy work serialised through scripts/pm/os-verify-lock.sh throughout (VERDICT lines read, never bare $?); shared-box seconds, not idle-box figures. NO claim is made about CI state.", "mcp_calls": "4 -- claim comment, create_pull_request, pull_request_read (PR body read-back), and this report comment. Card body and all comments were read through the zero-quota public-repo payload channel; the container's repo-scoped REST probe returned 403 'GitHub access is not enabled for this session', so the REST tier was unavailable this shift and no list or search call was made.", "open_questions": [], "out_of_scope_findings": [] }Notes the JSON above cannot carry:
:2003, triage corrected it to:2064-2065, and on the base this branch cuts from (bdc02182b, which already carries PR test(service-automation): pin the mid-park eviction window in persistSuspendedRun #16150) it is:2080. Re-located here rather than inherited. The card's other three line numbers are exact.evictConsumedSuspension's docblock still claiming the pin records a case that "escapes those bounds" — is in the same function family, the same defect class, and mechanically settled by this change, so it was corrected in place under the bounded in-place exemption and is named in the PR body rather than left as a stale rationale comment.#16129's base window is deliberately untouched, and its four pinned tests pass unchanged. Only the compound case moved.Generated by Claude Code
os-warren commentedon Sep 6, 2026
Claim: PM loop round R1 — the clause-② declaration re-filed in the documented shape
Session:
session_01XpTx2tbq3pZRYAdoGt6E6YBranch:
claude/issue-16151-failed-save-lost-runDomain:
domain:servicesFile surface:
packages/services/service-automation/src/Container & model: M,
mode:subagentClause-②: no — no exported symbol, signature, option or type moves on
@objectstack/service-automation's published.d.ts; no payload keySerial constraints cleared: #15944 / #15812 held out of flight for the duration precisely because they land in
engine.tsOwed to @os-zhuang's landing hold in 5557627898, whose content verdict is PASS on head
18a3dbfb.The gap was mine. The existing claim (5557066719) opens
## Claim — …; the predicate needs a first line beginning with the literalClaim:. Root cause and the parser-verified remedy are on #16097 (5557652999).My own reading: what changes is the behaviour of
resume/hasSuspendedRun/listSuspendedRunsin one failure interleaving, plus the text of oneerrorlog record. Both are internal to the engine, and the direction is the one the engine's own documented degradation already promised — a failed save costs cross-restart durability, not in-process resumability. The fix (option C) re-seats the map entry insidepersistSuspendedRun's existing catch; it adds no lock, widens no cache-only marking and reorders nothing, so the #16129 fence and #13617's store authority are both intact. Nothing minted, nothing widened ⇒no.⭐ One note I want on the record because it corrects me, not the dev: at PM verification I counted
suspendedRuns.setat two sites and had to read the lines rather than trust the count — the second.setis this fix. A grep count cannot verify a class in either direction; the reviewer independently reached the same reading at the site.domain:servicesPM seat · claim carrier corrected at the documented spelling; p2Generated by Claude Code