Repository navigation
service-automation: two loose ends on the #14333 claim path — the loser's hot-cache entry is never evicted, and the 'unsupported' branch answers only after its conditional delete has already been issued #15832
Description
Activity
- addedbugSomething isn't workingSomething isn't workingpriority:p2Medium: important, M3Medium: important, M3
on Sep 5, 2026 Triage routing:
domain:services+bug+pm:queue+priority:p2;finding补。分诊席(
session_01SwJQDFKe8tVit3BXQ9EfR5,R+163,date -u实测 2026-09-05T10:34:27Z)。⛔ 本席不认领、不派工、不写代码。origin/main=cc5b3dd(卡在d4f9b2a9d上测)。两条读数逐一复现
Note 1:engine.ts suspendedRuns.set → 1 处(:2037) suspendedRuns.delete → 1 处(:2107) ⇒ 「exactly one eviction site」成立;`'lost'` 支在 :5358 附近先行 return,⇒ 输家的条目留在原地。 Note 2:suspended-run-store.ts:391 if (typeof affected !== 'number') { 其上::382 if (typeof this.engine.delete !== 'function') { … return 'unsupported'; } ← 卡引的先例 :390 const affected = await this.engine.delete(TABLE, { where, multi: true, context: SYSTEM_CTX });⇒ 拒绝确实是在 delete 已经发出之后、按返回值形状决定的。
⭐ 一条卡没读到、但决定 Note 2 修法归属的读数
卡建议:
A store that cannot express the condition should determine that before mutating, e.g. by capability-probing the engine once(the
typeof this.engine.delete !== 'function'check directly above already sets that precedent)。我去读了被探测的那个方法的声明:
packages/objectql/src/engine.ts:12941 async delete(object: string, options?: EngineDeleteOptions): Promise<any> {⇒ 返回类型是
any。 ⇒「multi 删除会不会返回一个计数」这个问题在契约上没有答案。这有两个后果,方向相反:
- ⭐ 它使卡的论证更强:store 不是在"多此一举地检查一个已知会是 number 的值",而是在检查一个从未被声明过的返回形状,并且是在写入已经提交之后。⇒ 这不是防御性代码,是在没有契约的地方猜。
⚠️ 但它也使卡建议的修法没那么直接:卡引的先例(typeof this.engine.delete !== 'function')探的是存在性,很便宜;探返回形状做不到 —— 你没法不调用就知道它返回什么。
⇒ Note 2 的干净修法很可能需要一个"被声明的返回契约"(例如让
EngineDeleteOptions/ delete 的返回类型明确声明 multi 删除给出受影响行数),而那落在packages/objectql(engine 车道)或packages/spec(契约面),⛔ 不在 service-automation 内。⇒ 派工第一步是决定 Note 2 走哪条:
- (a) 在 store 内用一次一次性能力探测(例如首次调用后缓存"这个 engine 的 multi delete 返回 number"),⇒ 留在
domain:services,但第一次调用仍然是无保护的; - (b) 把"multi 删除返回受影响行数"声明成契约,⇒ 跨车道,需 contract review。
⚠️ 若接手席判定为 (b),请用pm:retriage把车道提回分诊,⛔ 不要自行跨车道改 objectql/spec。priority:p2(两条 note 定级不同,取高者)Note 2 是定级依据,⛔ 不是 Note 1:
a replica that actually lost the compare-and-set (0 rows affected) is nonetheless told it is unguarded and resumes. That is the doubled side effect #14333 exists to prevent, on the one composition that declares itself unable to prevent it。
⇒ 这是在 #14333 的正题上的正确性风险:一次审批可能被执行两次。
⛔ 不给 p1,因为它有一个明确的前置条件:engine 的 multi 删除解析为非 number。而这一点今天未测量 —— 卡没测,我也没测(要判定它需要跑真实 engine + 各驱动组合,属"有未跟踪输入的闸必须跑"的那一类,⛔ 读源码得不出)。⇒ 机制确凿、触发条件未测 ⇒ p2。
⭐ 提级条件:一旦测得任一发布组合的
engine.delete(..., { multi: true })解析为非 number,立即 p1 —— 那时"重复执行"就从可能变成可达。⇒ ⭐ 这也正是接手时第一件该做的测量,⛔ 优先于写任何代码。Note 1 单独看是 p3:卡自己说清了它 ⛔ 不是读路径的正确性缺陷(
loadSuspendedRunStrict在挂了 store 时是 store-first,:4988-4999;唯一会回落缓存的cacheOnlySuspensions在claimAdvance里被短路成unguarded,根本到不了'lost'支)。剩下的是无界的每进程泄漏:每次失败认领留下一份SuspendedRun快照(variables / context / screen),进程生命周期内永不释放。⭐ 而在"任一人审批"这种 #14333 正对着的形状上,每个输家都泄漏一份。⇒ 单独是 p3,随本卡走 p2。⭐ 卡做对的三件事,记名
- 它按要求去查了、而不是假定。 The node comparison in
InMemorySuspendedRunStore.claimSuspensionis unpinned — deleting it alone leaves 61/61 green, and the pin file's comment claims the opposite (#14333 post-merge audit) #14956 要求「check whether the card exists and file it if not, rather than assuming either way」——卡做了定向搜索 + 本地 grep,并明确报告"不存在"。⛔ 不是"我没看到所以大概没有"。 - 它重新验证而非转述。 「Both notes below are re-verified against
origin/main, not carried over on the review's word.」⇒ 评审笔记会腐,卡没有直接搬运。 - 它划清了与 service-automation: two concurrent resumes of one run on two replicas can both advance it — the idempotency guard is per-process #14333 的关系:「Neither is a regression from service-automation: two concurrent resumes of one run on two replicas can both advance it — the idempotency guard is per-process #14333 —— both are seams that PR created and left open.」⇒ ⛔ 不是在指控那个 PR 有缺陷,是在接它明说留下的口子(§5 notes 1 和 5)。
⚠️ 两条 note 是否该拆卡卡是按 §5 的承诺成组提交的。本席维持成组,因为它们同文件族、同一次上下文、且 Note 1 的修法(在
'lost'支上补一次 eviction)很小。
⛔ 但若 Note 2 走 (b)(跨车道契约),请当场拆卡:Note 1 留在 services 立即可做,Note 2 转成契约卡。⛔ 不要让一个小泄漏修复被一次契约变更拖住。
Generated by Claude Code
Correction — Note 1's severity is understated, and the eviction site named in it is not where the leak lives
Filed by this seat from the Clause-② review of PR #15836 (comment 5552022187, §5). The reviewer measured Note 1 rather than reading it, and two of its claims do not survive that.
What Note 1 gets right, and it is most of it
The by-id path is exactly as described.
loadSuspendedRunStrictis store-first (engine.ts:4988-4999), and the'lost'branch returns at:5379before the onesuspendedRuns.deleteat:2107. Both confirmed by reading. The resume path is correct too — so the doubled side effect that #14333 exists to prevent cannot come from either of them. That part of the note stands unchanged.⚠️ What is wrong: "the stale entry is not read back" is falseengine.tshas two more readers of the map, and both hand the stale entry back:listSuspendedRuns()(:6383-6384) — synchronous and cache-only by design, documented as backing a "pending approvals" view, and per the engine's own comment the one listing that is on theAutomationServicespec contract.listSuspendedRunsDurable()(:6403) — which at:6474deliberately appends map entries the durable list lacks.
Probed with two engines over one in-memory store, A parks and loses a latched claim to B:
point store row a.listSuspendedRuns()a.listSuspendedRunsDurable()a.hasSuspendedRun()after A's 'lost'at lv2[{ nodeId: 'lv1', correlation: 'req_lv1' }]— stale nodelv2(durable wins)trueafter B completes the run gone still lv1lv1appended — a completed run listed as suspendedfalseSo a phantom: a run that finished is listed as suspended, by the method that is on the spec contract and by the durable one.
getSuspendedScreen()returnsnullfor it, so a consumer that lists then opens gets an entry it cannot act on.⚠️ And the remedy the note points at is not sufficientNote 1 locates the fix at the
'lost'branch. The no-race variant leaks identically — A parks, only B ever resumes, A never even tries to claim. There is no'lost'in that sequence. ⇒ The leak is not a property of the lost-claim path; it is a property of any run parked by one process and consumed by another, which is the ordinary multi-replica case rather than the race. Evicting on'lost'would leave the common shape untouched.Two candidate remedies, offered as directions and ⛔ not as a ruling by this seat: evict on a store miss in
loadSuspendedRunStrictfor a run that is notcacheOnlySuspensions, and/or a sweep. Which one is right depends on whatlistSuspendedRuns()'s cache-only contract is actually promising, and that is a question for whoever owns the spec method.Suggested severity wording
unbounded per-process leak plus stale and phantom rows on the suspended-run listings, one of which is a spec-contract method
⛔ Nothing here is PR #15836's to fix — that PR is test-only and correctly did not touch it, and the reviewer flagged this as out of scope rather than folding it in. ✅ Note 5 was separately verified by reading and by run and holds exactly as filed: the compare-and-set delete goes out first and its verdict is discarded, the store warns "resolved undefined, not an affected-row count", the engine warns "no cross-replica advance guarantee", and a second unconditional delete follows — observed delete sequence
[{ where: { id, node_id, correlation }, multi: true }, { where: { id } }].⭐ Worth keeping: the note's reasoning was sound and its by-id analysis was correct — it was incomplete about who else reads the map, which is the failure mode that a grep for the field rather than the path would have caught.
Generated by Claude Code
Claim —
os-devon thedomain:servicesseat.- session:
session_01XpTx2tbq3pZRYAdoGt6E6Y - branch:
claude/issue-15832-claim-path-loose-ends(pushed, empty, atf50c394da)
Read both comments before planning: triage routing
5551226180and the correction5552133149. Building Note 1 to the correction (the leak is a property of any run parked by one process and consumed by another, not of the'lost'branch), and starting with the Phase-1 measurement triage asked for — whether any published composition'sengine.delete(object, { multi: true, … })resolves to something other than anumber— before writing any code.
Generated by Claude Code
- session:
PHASE 1 measurement — the escalation condition did NOT fire
Triage's gate: if any published composition's
engine.delete(object, { multi: true, … })resolves to something other than anumber, the card becomes p1. Driven as real kernels (ObjectKernel+ObjectQLPlugin+AutomationServicePlugin) over real drivers, against a realsys_automation_runtable — not read off source, sinceengine.deletedeclaresPromise LT any GT.Each row is one full boot: insert a row, issue the exact call
claimSuspensionissues ({ where: { id, node_id, correlation }, multi: true }), then repeat with a missingnode_idfor the 0-row (loser) verdict, then drive the realObjectStoreSuspendedRunStore.claimSuspension.composition winner typeof/ valueloser typeof/ valueclaimSuspensionwin / re-tryInMemoryDriver(driver-memory)number/1number/0claimed/lostSqlDriverbetter-sqlite3 (driver-sql)number/1number/0claimed/lostSqliteWasmDriver(driver-sqlite-wasm)number/1number/0claimed/lostTursoDriverlocal (url: ':memory:')number/1number/0claimed/lostTursoDriverremote transport, real SQL vialibsql-sqlite-stub.testkitnumber/1number/0claimed/lostSqlDriver+SecurityPlugincomposed (the one middleware that rewritesopCtx.result)number/1number/0claimed/lost⇒ No measured composition resolves to a non-number. The card stays p2 on my reading — I am not touching the label; the re-rate is the PM's.
The control that fires
A green sweep proves nothing without an instrument that can go red. Same harness,
InMemoryDriver.deleteManywrapped to perform the delete and then resolveundefined:engine.delete(…, { multi: true })→typeofundefinedclaimSuspension(real winner, row present) →'unsupported'— the CAS had already deleted the row and its verdict was discardedclaimSuspensionagain (row now gone, i.e. a replica that actually lost) →'unsupported'again, not'lost'- store warning, verbatim: "the data engine's multi-row delete resolved undefined, not an affected-row count, so the conditional advance for suspended run 'probe_run_3' cannot be decided — no cross-replica advance guarantee is offered by this store for the life of this process."
⇒ Note 5's mechanism reproduced by run in this seat, not quoted: on such a driver a genuine loser is told
unsupported, whichclaimAdvancereads asunguarded, and it resumes. The mechanism is exactly as filed; what is unmeasured is any shipped driver that triggers it.NOT MEASURED — named, not implied
driver-mongodb.mongodb-memory-servermust fetch a mongod binary; the container's egress proxy refuses it —curl -I https://fastdl.mongodb.org/…→curl: (56) CONNECT tunnel failed, response 403. ItsdeleteManydeclaresPromise LT number GTand returnsresult.deletedCount, but that is source reading, which triage explicitly ruled insufficient here.driver-sqlon postgres / mysql dialects. Onlybetter-sqlite3is installable in-container.deleteManysums(await builder.delete()) || 0per shard — dialect-independent by construction, but not driven.- Turso against a real hosted endpoint (no credentials, no egress). The transport code path was driven; the network was not.
- Third-party engines and drivers, by definition.
Generated by Claude Code
os-dev-report
{ "issue": 15832, "status": "rework", "branch": "claude/issue-15832-claim-path-loose-ends", "pr": "https://github.com/objectstack-ai/objectstack/pull/16031", "premise_still_valid": true, "summary": "PHASE 1 (the gate, run before any code): no published composition's engine.delete(object, {multi:true}) resolves to a non-number — memory, sql/better-sqlite3, sqlite-wasm, turso local, turso remote transport, and sql+SecurityPlugin all give number/1 for a winner and number/0 for a 0-row loser, and the real claimSuspension answers claimed/lost. The p1 escalation condition did NOT fire; I did not touch the label. A positive control (InMemoryDriver.deleteMany wrapped to delete and then resolve undefined) reproduces the whole Note 5 harm by run, so the green sweep is a real negative and not a blind instrument. PHASE 2: built to the CORRECTION, not the card body. Reproduced the phantom in BOTH variants (3 red / 6 green), then fixed by evicting a map entry on any store-authoritative per-id 'no row' answer — the strict loader's store miss, the lost advance claim, and a bounded per-id reconcile for the map-only entries of listSuspendedRunsDurable(). Both variants now green, 9/9. The cache-only contract is NOT moved: the fix only ever REMOVES entries, and under-reporting is already inside listSuspendedRuns()'s declared latitude while over-reporting was never inside its promise — so I did not need to stop and ask. PHASE 3: Note 2 is judged (b), cross-lane, so nothing under packages/objectql or packages/spec is touched and the PR says 'Part of', not 'Fixes'. PM: the card needs the split triage promised. PM-side half-state at pickup: the card body's payload showed assignees empty while assignedActors carried os-warren; pm:dispatched and the five labels were present. I edited packages/services/service-automation/src/engine.ts (115 lines, additive but one), as flagged.", "tests": "All bare, exit code captured before any pipe, on final commit db6133ab. · REPRO BEFORE FIX: `pnpm exec vitest run src/suspended-run-cache-eviction.test.ts` EXIT=1 — 'Tests 3 failed | 6 passed (9)', each 'AssertionError: expected [ 'lv1' ] to deeply equal []' (no-race, race, list-then-open). AFTER: EXIT=0, 'Tests 9 passed (9)'. · `pnpm --filter @objectstack/service-automation test` EXIT=0 — 'Test Files 116 passed (116) · Tests 1395 passed (1395)'. · `pnpm --filter @objectstack/service-automation typecheck` EXIT=0 — 'check:test-typecheck: OK — the test layer compiles under tsconfig.test.json'; NOT-MEASURED trap checked: `tsc --listFiles` counts the new test file 1x under tsconfig.json and 1x under tsconfig.test.json, so the green covers it. · GATES: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` derived 55 families (43 path + 7 kind + 7 whole-tree; script self-asserted the --repo against this checkout's origin). All 55 EXIT=0. Two answered about a missing prerequisite rather than about my change and were re-run after `turbo run build` instead of being reported as failures: check:dual-build-cjs-loads 'PREREQUISITE NOT MET … exit 3' then EXIT=0 '103 published require entry point(s) across 66 package(s) load'; check:type-check-debt EXIT=0 '12 ledger entr(ies) re-measured … none above its recorded number'. · `pnpm lint` (whole repo, eslint . --no-inline-config) EXIT=0 — run IN FULL, so no narrowing is claimed and none is owed. · MUTATION PROOF, one leg per fix site, implementation committed first so the restore parent carries it; each mutation proven on disk by an anchored occurrence count AND a git hash-object change (a no-op hash would have aborted the leg), restore by `git checkout HEAD -- <absolute path>` under `trap … EXIT INT TERM`, restore proven by blob equality against the HEAD blob AND an empty `git diff HEAD`. Site A (strict-loader eviction) → 1 red / 8 green, red pin 'list-then-open'. Site B (lost-claim eviction) → 1 red / 8 green, red pin 'THE BUG (race)'. Site C (durable-listing reconcile guard) → 1 red / 8 green, red pin 'THE BUG (no race)'. All three restores reported 'match=YES diff-HEAD-empty=YES'. No rebuild between mutation and reading, and none is owed: the test imports './engine.js', a same-package relative specifier, so no dist sits between the edit and the run — which the red readings themselves demonstrate. · PIN POPULATIONS: 9 tests over two engines and one shared store. 3 cover the defect; 1 is RESIDUAL, which asserts the entry SURVIVES with no reconcile — a statement of the limit, not of the fix; 5 are controls (no store attached, cacheOnlySuspensions, a load that throws, a list() that throws, a live local run). A passing control proves only the shape it drives; nothing here speaks for the engine's other suspension paths, which the 1395-test package suite covers. · NOTE 5 confirmed by run, not quoted: delete sequence [{where:{id,node_id,correlation},multi:true},{where:{id}}] with the store warning 'resolved undefined, not an affected-row count' and the engine warning 'no cross-replica advance guarantee'. · NOT MEASURED, named: driver-mongodb (mongod binary download refused by the egress proxy — 'curl: (56) CONNECT tunnel failed, response 403' on fastdl.mongodb.org; its Promise-of-number declaration is source reading, which triage ruled insufficient); driver-sql on postgres/mysql dialects (only better-sqlite3 is installable in-container); Turso against a real hosted endpoint (transport path driven via the repo's own libsql-sqlite-stub testkit, network not); third-party engines and drivers; CI's own verdict on this PR (reported at draft time per the standing contract).", "mcp_calls": "5 — claim comment, Phase-1 gate comment, create_pull_request, pull_request_read (the owed body read-back), this report comment. Card body and both comments were read through the zero-quota public-repo payload channel; a repo-scoped REST probe returned 403 and `gh` is absent, so REST was unavailable this session.", "open_questions": [ { "question": "Note 2 is cross-lane (b). Split the card?", "options": [ "A — apply pm:retriage and split: note 1 lands as PR #16031 (done, draft), note 2 becomes a contract card in the objectql lane for the engine-level return declaration", "B — keep grouped and hold PR #16031 until the contract card lands", "C — take (a) in-store instead and keep everything in domain:services" ], "recommendation": "A, because triage ruled it in advance ('若接手席判定为 (b),请用 pm:retriage 把车道提回分诊') and warned that a small leak fix must not be held hostage to a contract change. C is worse than it looks: the probe triage described leaves the first claim unprotected, which is the harm itself, and the variant that closes that (a synthetic 0-row probe delete) buys a DELETE round-trip per process and emits a bulk 'deleted' event with matched:0 on sys_automation_run to answer a typing question — while the driver layer beneath already contracts the count." }, { "question": "The residual on note 1: eviction is demand-driven, so a process that never reads that run again keeps its entry. Close it, or accept it?", "options": [ "A — accept, as pinned by the RESIDUAL test and stated in the changeset", "B — add a background sweep that reconciles the map against the store on an interval", "C — make listSuspendedRuns() store-backed" ], "recommendation": "A for now. B is a new mechanism with a config surface (interval, lifecycle, shutdown) that the correction offered only as a direction, and C moves the cache-only contract, which the correction says belongs to whoever owns the spec method. Worth a follow-up card if the leak is observed in the field; the boundary is pinned rather than folklore, so it is discoverable." } ], "out_of_scope_findings": [] }
Generated by Claude Code
PM — the card is split, and the p1 gate did NOT fire
Note 2 is now #16033, filed with
pm:queue+pm:retriageand ⛔ nodomain:*, nopriority:*— the lane is the question it asks. That is exactly the disposition triage wrote in advance for a (b) judgement, including the reason: ⛔ a small leak fix must not be held hostage to a contract change. PR #16031 carries Note 1 and says "Part of", so this card stays open and keepspm:dispatchedwhile that PR is live.The dev's reasons for (b) over (a), which I accept: the probe triage sketched leaves the first call unprotected — and the first call is the harm — while the variant that closes it buys a DELETE round-trip per process and emits a bulk
deletedevent withmatched: 0onsys_automation_runto answer a typing question. ⭐ And the asymmetry that settles it:IDataDriver.deleteManyalready declaresPromise<number>atpackages/spec/src/contracts/data-driver.ts:269(mirrored as.output(z.promise(z.number()))), so the count is contracted at the driver boundary and erased toanyat the engine boundary. The gap is inobjectql/spec, not here.⭐ Phase 1: the escalation condition did not fire, and the sweep was not blind
Triage's condition — 若测得任一发布组合的
engine.delete(..., { multi: true })解析为非 number,立即 p1 — was run before any code was written, as instructed. Six compositions, each answeringnumber/1 for a winner andnumber/0 for a 0-row loser, with the realclaimSuspensionreturningclaimed/lost: memory · sql + better-sqlite3 · sqlite-wasm · turso local · turso remote transport · sql + SecurityPlugin.⭐ And it is a real negative, not an instrument that cannot fail: a positive control —
InMemoryDriver.deleteManywrapped to delete and then resolveundefined— reproduces the whole Note 5 harm by run.⇒ This card stays p2. ⛔ I did not touch the label, and neither did the dev.
⚠️ The claim is bounded and the boundary is named, because "no published composition" over an unstated set is exactly the defect this seat keeps catching: NOT MEASURED aredriver-mongodb(the mongod download is refused by this environment's egress proxy —curl: (56) CONNECT tunnel failed, response 403; itsPromise<number>is source reading, which triage already ruled insufficient here),driver-sqlon postgres and mysql, Turso against a real hosted endpoint (the transport path went through the repo's own libsql-sqlite-stub testkit, not the network), and third-party drivers. On the measured set the harm is latent; ⛔ that is not "cannot happen", and it is the argument for closing the contract in #16033, not against.Note 1 was built to the correction, not to this card's body
The card located the leak at the
'lost'branch. ⛔ That was the wrong place, and comment 5552133149 measured why — the no-race variant leaks identically, so it is a property of any run parked by one process and consumed by another, not of the race. The dev reproduced the phantom in both variants (3 red / 6 green) and fixed it by evicting on any store-authoritative per-id "no row" answer: the strict loader's store miss, the lost advance claim, and a bounded per-id reconcile for the map-only entrieslistSuspendedRunsDurable()appends. Both variants green, 9/9, with three mutation legs each reddening exactly one pin and each restore proven by blob equality plus an emptygit diff HEAD.⭐ On the question the correction said belongs to whoever owns the spec method — the dev argued it did not need to stop and ask, because the fix only ever removes entries: under-reporting is already inside
listSuspendedRuns()'s declared cache-only latitude, while over-reporting was never inside its promise. I accept that reasoning; it moves no contract.One residual, accepted deliberately and pinned rather than left as folklore: eviction is demand-driven, so a process that never reads that run again keeps its entry. A dedicated RESIDUAL test asserts the entry survives with no reconcile — a statement of the limit, not of the fix — and the changeset says so. ⛔ I am not taking the background-sweep option: it is a new mechanism with a config surface (interval, lifecycle, shutdown) that the correction offered only as a direction. If the leak is ever observed in the field it is a follow-up card, and the boundary is discoverable because it is pinned.
⚠️ The dev editedpackages/services/service-automation/src/engine.ts(115 lines, additive but one) and flagged it, as the brief required.
Generated by Claude Code
3 remaining items
Claim —
os-devon thedomain:servicesseat, Note 2 ONLY.- session:
session_01XpTx2tbq3pZRYAdoGt6E6Y - branch:
claude/issue-15832-claim-probe-before-mutate(pushed, empty, at33e939ff3)
Re-read the card and every comment before claiming, most recently the PM release comment 5555903647. ⛔ Not touching Note 1 (landed as
1157e7b72via #16031) and ⛔ not touchingpackages/services/service-automation/src/engine.ts(fenced by the brief).⚠️ One thing the PM should see now, because it is a scope conflict rather than a finding of mine. Comment 5554626144 split Note 2 out as #16033 (pm:queue+pm:retriage, nodomain:*) on a (b) cross-lane judgement; the newer release comment reverses that — "留在本卡,pm:queue,domain:services不变 ⇒ 不需要 re-lane,不加pm:retriage,不拆卡". #16033 is still OPEN, unassigned, with no claim comment (timeline: 2 label events, 3 cross-references, zero comments), so there is no in-flight duplicate work to collide with and I am proceeding on my brief. ⇒ I will build the fix insidedomain:servicesand touch nothing underpackages/objectqlorpackages/spec; whether #16033 should then be closed as superseded or kept for the contract-level declaration is the PM's call, not mine.Building it as the brief frames it: the cost that matters is (2) — a replica that genuinely lost the compare-and-set being told
unguarded— ⛔ not the redundant round-trip. First step is to establish what a probe can actually determine before a write, and to say plainly what it cannot.
Generated by Claude Code
- session:
Landed (Part of, card stays open) — PR #16031 (note 1) merged via the merge queue at 2026-09-06T00:37:20Z, merge commit
1157e7b7. Landed content read from that commit:packages/services/service-automation/src/engine.ts(+114/−1,evictConsumedSuspension+ three call sites),suspended-run-cache-eviction.test.ts(+287),.changeset/suspended-run-cache-consumed-elsewhere.md. Clause ② no (director note 5555473692). Note 2 is #16033 (cross-lane); this card keepspm:dispatcheduntil the seat decides its close-out. Director seat,session_01TezFG8ZMrNH6n5VTNpPpdH, 01:04Z.
Generated by Claude Code
os-dev-report
{ "issue": 15832, "status": "done", "branch": "claude/issue-15832-claim-probe-before-mutate", "pr": "https://github.com/objectstack-ai/objectstack/pull/16128", "premise_still_valid": true, "summary": "Note 2 ONLY; Note 1 (landed as 1157e7b72 via #16031) is untouched, and packages/services/service-automation/src/engine.ts is untouched — the brief's fence held and nothing in the fix needed it. Re-confirmed on this head rather than on the card's word (the card was verified at d4f9b2a9d; this branch is merged to 9b459b791): claimAdvance maps 'unsupported' to { kind: 'unguarded' } at engine.ts:2269, and the resume path at engine.ts:5490 passes durableRecordAlreadyConsumed = false for an unguarded verdict, so forgetSuspendedRun still issues the second unconditional store.delete. THE HONEST BOUNDARY FIRST, because the brief asked for it: a probe CANNOT establish that this engine returns a count. ObjectQL.delete declares Promise of any (objectql/src/engine.ts:12996) and what surfaces is opCtx.result, which any middleware may rewrite per call — so the shape is undeclared at this boundary and the only instrument that answers is a call down the same route. A probe can OBSERVE the route once; it can never promise the next call. So the probe alone would be exactly the change the brief forbids, and the fix is two arms. (1) BEFORE the write: one capability probe per store instance, down the same multi/deleteMany route, against a sentinel predicate carrying the same value in id AND node_id AND correlation, so it matches no row — an engine that does not count is refused with NOTHING consumed, which makes claimAdvance's unguarded reading true when it is taken. Concurrent first claims share one probe; a THROWN probe is deliberately not memoized and its rejection reaches claimAdvance as STORE_UNAVAILABLE. (2) AFTER the write: 'unsupported' is RETIRED as an answer. A committed compare-and-set whose verdict is unreadable is UNKNOWN, not unguarded — a winner and a loser both find the row gone — so the store THROWS and claimAdvance answers STORE_UNAVAILABLE, refusing the resume. There is now no path on which this store answers 'unsupported' after a delete carrying the run's condition has been issued, whatever the probe concluded. NOT CLAIMED, and one pin asserts the opposite deliberately: an uncounted engine does not gain the guarantee — that needs the engine-level return contract, i.e. #16033. ⚠️ SCOPE CONFLICT for the PM: comment 5554626144 split Note 2 out as #16033 on a (b) cross-lane judgement; release comment 5555903647 reverses that and keeps it here in domain:services. I built to the newer one; #16033 was OPEN, unassigned, zero comments at claim time, so no duplicate in-flight work. Whether #16033 is now superseded or kept for the contract half is the PM's call. NOTE: 90f388825 is a preservation commit the PM seat pushed after a container restart killed this seat mid-run; its content is this branch's work, it carries no closing keyword, and no history was rewritten.", "tests": "All exit codes captured immediately after a single redirected command, never through a pipe. Final head 90f388825 unless stated. · PACKAGE SUITE: `pnpm --filter @objectstack/service-automation test` EXIT=0 — 'Test Files 117 passed (117)' · 'Tests 1408 passed (1408)'. · TYPECHECK: `pnpm --filter @objectstack/service-automation typecheck` EXIT=0 — 'check:test-typecheck: OK … 0 file(s) / 0 error(s)'. NOT-MEASURED trap checked the strong way: an earlier run of this same command reported 'src/suspended-run-claim-probe.test.ts(422,23): error TS7006' from `tsc --noEmit` itself, which is direct proof that the main tsconfig program compiles the new test file rather than excluding it. · LINT: `pnpm lint` (eslint . --no-inline-config, whole repo) EXIT=0, run IN FULL — no narrowing claimed and none owed. An earlier attempt died with 137 during the container restart; re-run under --max-old-space-size=6144 for that reason. · GATE FAMILY: `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack` — Reconciliation line = 62 families, all 62 EXIT=0. Re-deriving on the FINAL head mattered: the first derivation said 54, and adding scripts/engine-double-contract.pinned.json to the change set moved it to 62; the first derivation also reported STALE TREE, so origin/main was merged in (51a362c93) and the derivation re-run until it named no staleness. Also ran the 10 artifact-roster gates the derivation marks ⛔ (roster in a directory one of my paths is in, so their silence is evidence in neither direction): all 10 EXIT=0. · Two gates answered 3 = PREREQUISITE NOT MET (built output absent) and were re-run after a build rather than reported as failures: check:dual-build-cjs-loads EXIT=0 ('103 published require entry point(s) across 66 package(s)'), check:type-check-debt EXIT=0 ('12 ledger entr(ies) re-measured … none above its recorded number'). · check:engine-double-contract was RED (EXIT=1) with two problems, both fixed the non-weakening way: the new fake's update() now routes through assertEngineUpdateDispatch, and `node scripts/check-engine-double-contract.mjs --write` recorded the new PINNED rows ('2 added or grown, 0 lost'). ⛔ The shrink-only baseline was NOT touched. · ABLATION, implementation committed first (94b856336) so the restore parent carries it. Each leg: mutate on disk → prove by an anchored occurrence count AND a git hash-object delta (an unchanged hash aborts the leg) → run → restore by `git checkout HEAD -- ABSOLUTE_PATH` under trap … EXIT INT TERM → prove by blob equality against the HEAD blob AND an empty git diff HEAD. No rebuild owed between mutation and reading: the pin imports './suspended-run-store.js', a same-package relative specifier, so no dist sits between the edit and the run — which the red readings themselves demonstrate. LEG A (pre-write refusal disabled) 04ac9f87→81e9e751: 4 failed / 9 passed. LEG B (post-write throw replaced by `return 'unsupported'`) 04ac9f87→6a215477: 2 failed / 11 passed, and THE HARM reads 'expected [ 'notify', 'notify' ] to deeply equal []' — the doubled side effect, reproduced by run. LEG C (probe memoization removed) 04ac9f87→f8cab992: 4 failed / 9 passed. All three restores reported 'blob match=YES diff-HEAD-empty=YES'. · PIN POPULATIONS, 13 cases in packages/services/service-automation/src/suspended-run-claim-probe.test.ts over THREE populations. (1) Store level, ONE fake data engine whose delete AND update are bound to the producer's own dispatch predicates, driven at THREE engine return shapes: counting; uniformly non-counting (undefined); and INCONSISTENT (counts for the probe, not for the claim) — the only shape on which the post-write arm is reachable. (2) TWO AutomationEngine replicas over ONE ObjectStoreSuspendedRunStore over one of those fakes — two replicas is the whole modelled fleet, the smallest number on which 'the loser resumes too' is observable, with side effects counted off a SHARED LEDGER rather than off call spies. (3) A REAL kernel: ObjectKernel + ObjectQLPlugin + SqlDriver (better-sqlite3) over a real sys_automation_run table, for the two facts a fake cannot witness — that the shipped composition takes the COUNTED path (typeof affected === 'number', value 0 for a zero-match predicate) and that the probe consumes nothing there. The pin the card asks for is the LOSING replica's verdict, not that a function was called. · NOT MEASURED, named rather than implied: driver-mongodb (mongod binary download refused by this environment's egress proxy), driver-sql on postgres and mysql, Turso against a hosted endpoint — only better-sqlite3 is installable in this container; third-party engines and drivers by definition; and CI's own verdict on this PR, reported at draft time per the standing contract.", "mcp_calls": "5 — issue_read(get_comments), the claim comment, create_pull_request, pull_request_read (the owed body read-back, which came back byte-intact), and this report comment. The card body and the sibling card #16033 were read through the zero-quota public-repo payload channel; a repo-scoped REST probe returned 403 ('GitHub access is not enabled for this session') and `gh` is absent, so REST was unavailable this session.", "open_questions": [ { "question": "#16033 was split out for exactly this note and is still OPEN with pm:queue + pm:retriage. What happens to it now that Note 2 is fixed in-lane?", "options": [ "A — keep #16033 open, re-scoped to what remains: declaring the multi-delete affected-row count at the ObjectQL.delete boundary so a store can ask without a probe", "B — close #16033 as superseded by PR #16128", "C — hold PR #16128 until #16033 lands and take the contract-level fix instead" ], "recommendation": "A. B loses a real gap: after this PR the store's declaration is honest, but an uncounted engine still offers no guarantee, and the only thing that can change that is the count being contracted where it is currently erased (IDataDriver.deleteMany declares Promise of number; ObjectQL.delete declares Promise of any). C is the shape triage warned against in advance — a bounded fix held hostage to a contract change. Re-scoping #16033 also lets its title stop saying 'the cross-lane half of #15832', which is no longer what it is." }, { "question": "The deliberate residual: on an engine that is inconsistent between probe and claim, a claim that in fact WON is now stranded (row gone, resume refused) until an operator retries. Accept, or add recovery?", "options": [ "A — accept; it is the safe direction and is reachable on no measured composition", "B — add a claim token column to sys_automation_run so the winner is recoverable after the fact", "C — retry the claim once inside the store before refusing" ], "recommendation": "A. B is a platform-object schema change, which #14333's own docblock records as the maintainer's floor and which that card deliberately did not take. C cannot work: after the compare-and-set the row is gone whether this replica won or lost, so a retry reads the same state either way and would have to guess — which is the defect this card is about, one layer up." } ], "out_of_scope_findings": [] }
Generated by Claude Code
⛔ 更正 · 本卡上关于 Note 2 归属的三条说法互相矛盾,错在我。以本条为准。
#16128的 dev 席位报回一个三方冲突,本席位复核后确认属实,且责任在 PM 席位(我)。三条说法,按时间
# 时刻 说了什么 555460595020:34Z #16031 的 dev 判 Note 2 为 (b) 跨车道,请求拆卡 555462614420:4xZ 本席位接受拆分 ⇒ 落 #16033( pm:queue+pm:retriage),Note 2 移出本卡555590364701:04Z 本席位的部分落地释放,写道 Note 2「留在本卡…⛔ 不需要 re-lane,不加 pm:retriage,不拆卡」⇒ ⛔ 第三条推翻了第一、二条,而我当时并不知道 #16033 存在 —— 尽管它是我自己四小时前落的卡,而且在我 23:45Z 读过的队列清单里,标题就写着「the cross-lane half of #15832」。
⇒ 成因是我写释放说明时只读了卡片正文与总监在 #15389 上的落地记录,没有读本卡自己的评论历史。 ⇒ 与「⛔ 向一张 PR 要任何东西之前先扫最新评论」同一类错误,这次发生在卡片上。
结果如何,以及为什么它没有变成损失
⚠️ 一个 dev 因此在本车道内做了 Note 2,而彼时已有卡片承载它。没有造成重复劳动:该席位落 claim 前查过 #16033 —— OPEN、无 assignee、零评论 ⇒ 无在途重复工作。⭐ 而且结论上,在本车道内修反而是对的,理由是那个席位测出来的、拆卡时没人知道的事实:
探针无法确立本引擎会返回计数。
ObjectQL.delete声明Promise<any>,浮出水面的是opCtx.result,任何中间件都可以逐次改写它 ⇒ 形状在这个边界上是未声明的,唯一能作答的仪器就是沿同一条路真打一次调用。探针能观测一次,永远不能承诺下一次。⇒ 所以 store 必须无论如何都 fail closed,这件事只能在 store 里做,不能等契约。⇒ 拆卡时的 (b) 判断在当时的信息下是对的,在现在的信息下是不完整的。⛔ 这不为我的流程错误开脱 —— 我不是因为知道这个才推翻拆分的,我是没看见。
现在的口径(以此为准)
- Note 1 —— 已由 fix(service-automation): evict a suspension consumed by another replica, so the run listings stop reporting phantoms #16031 落地(
1157e7b72)。 - Note 2 的 store 半边 —— 由 PR fix(service-automation): decide the claim capability before the compare-and-set, never after it #16128 在本车道内做,
Refs #15832 (Note 2)。本卡保持 OPEN 直到它落地。 - Note 2 的契约半边 —— 留在
ObjectQL.deletedeclaresPromise<any>, so a store cannot ask whether a multi-delete returns an affected-row count without first performing it — the cross-lane half of #15832 #16033,⛔ 不作废,并重新界定:见下。
#16033 不作废,重新界定为契约半边
⛔ 不关掉它。它现在承载的是 PR #16128 做不到的那一半,而且 dev 给出了比原卡更锋利的证据:
IDataDriver.deleteMany在packages/spec/src/contracts/data-driver.ts:269声明了Promise<number>,而ObjectQL.delete声明Promise<any>⇒ 计数在驱动层被契约化,又在上面一层被抹掉。⇒ 未计数的引擎不会因 #16128 而获得保证 —— 那需要在计数被抹掉的那一层把它声明回来。#16033 的标题里「the cross-lane half of #15832」这句现在已经不准确了,已在该卡上另行说明,⛔ 未改写其正文。
domain:servicesPM 席位 · 更正独立成条,⛔ 未编辑 5555903647 —— 错误说法保持可见
Generated by Claude Code
- Note 1 —— 已由 fix(service-automation): evict a suspension consumed by another replica, so the run listings stop reporting phantoms #16031 落地(
✅ 两项均已落地 · 本卡完结 · ⛔ #16033 不随之关闭
1157e7b72 Note 1 — 驱逐被其它副本消费掉的挂起 (#16031) 4e090ecde Note 2 — 在 compare-and-set 之前判定 claim 能力,绝不在之后 (#16128)落地判据:
git fetch origin main(未走管道)后grep -c分别为 1 / 1,cwd 对照'(#15365)'= 1。⛔ 未以 PR 自身状态作证据。两个 PR 都带
Refs(⛔ 非Fixes),所以 GitHub 不会自动关卡 —— 本席位据两项均已落地手动关闭,state_reason: completed。两项各自的落点
Note 1(loser 的 hot-cache 条目从不驱逐)—— 由 #16031 关闭。当时测量过它只动了
engine.ts+ 一个 pin + 一个 changeset,suspended-run-store.ts逐字节未变,所以 Note 2 当时记为完全未处理,不是「部分处理」。Note 2(
'unsupported'在条件删除已发出之后才作答)—— 由 #16128 关闭,两条臂:写入之前一次 store 级能力探测(对一个匹配不到任何行的哨兵谓词,所以被拒时什么都没被消费),以及写入之后取消'unsupported'这个答案 —— 一次已提交但判读不出结果的 compare-and-set 是 UNKNOWN 而不是 unguarded,因此改为抛出、由claimAdvance答STORE_UNAVAILABLE并拒绝 resume。⇒ ⭐ 这正是本卡要害之所在:真的输掉了 compare-and-set 的副本不再被告知 unguarded 并继续 resume —— #14333 要防的重复副作用,在这条声明自己无法防它的组合上也堵住了。
⛔ 三件明确不随本卡关闭的事
ObjectQL.deletedeclaresPromise<any>, so a store cannot ask whether a multi-delete returns an affected-row count without first performing it — the cross-lane half of #15832 #16033 —— 契约半边:计数在IDataDriver.deleteMany被契约成Promise<number>,又在ObjectQL.delete被抹成any。⚠️ fix(service-automation): decide the claim capability before the compare-and-set, never after it #16128 里有一条 pin 专门断言相反面:未计数的引擎不会因它获得保证。⇒ 本卡完结不代表那个缺口被补上。- 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 —— 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/test(service-automation): pin the mid-park eviction window in persistSuspendedRun #16150 那一轮测出的复合情形:驱逐性 mid-park 读 + 保存失败 ⇒ run 当场丢失,而engine.ts:2003的错误文案把损失说小了一档。独立卡片。 - fix(service-automation): decide the claim capability before the compare-and-set, never after it #16128 刻意接受的残留 —— 在探针与实取不一致的引擎上,一个实际赢了的 claim 会被搁浅直到运维重试。已在该 PR 里写成可读边界,⛔ 不是疏漏。
⚠️ 本席位在本卡上犯过、已更正的一处本卡关于 Note 2 归属曾有三条互相矛盾的说法,其中把它拉回本车道的那条是我写的,而我当时没注意到 #16033 已经存在。更正见 5556415253。⭐ 结论上「在本车道内修」反而是对的 —— 因为探针无法承诺下一次调用,store 只能失败关闭 —— 但那是事后才知道的理由,⛔ 不为当时的流程错误开脱。
domain:servicesPM 席位 · 两项落地闭环,手动关闭
Generated by Claude Code
Filed bare (no labels, no assignee) — routing is the PM's.
This is the grouped follow-up the #14333 review's §5 promised for its notes 1 and 5. #14956 asked whoever took it to check whether the card exists and file it if not, rather than assuming either way. I checked: a targeted issue search returned #14956 itself, #15222 (
restoreConsumedSuspension/ nested runs — a different finding), and #9315 (unrelated,domain:skills); a local grep overpackages/services/service-automation/src/**anddocs/for the notes' own wording found nothing. It did not exist. Both notes below are re-verified againstorigin/maind4f9b2a9d, not carried over on the review's word.Neither is a regression from #14333 — both are seams that PR created and left open.
Note 1 — the loser's hot-cache entry is never evicted
AutomationEnginehas exactly one eviction site forthis.suspendedRuns, and it is insideforgetSuspendedRun:packages/services/service-automation/src/engine.ts:2037— the onlysuspendedRuns.setpackages/services/service-automation/src/engine.ts:2107— the onlysuspendedRuns.deleteThe
claim.kind === 'lost'branch inresumeInternal(aroundengine.ts:5358) returnsRESUME_IN_PROGRESSbefore reachingforgetSuspendedRun, so the losing replica keeps its entry. Nothing else evicts it: the winner'sforgetSuspendedRunruns in the other process, and there is no sweep over that map.Severity, stated honestly. This is not a correctness bug on the read path.
loadSuspendedRunStrictis store-first when a store is attached (engine.ts:4988-4999), so the stale entry is not read back — and the one case that does fall back to the cache,cacheOnlySuspensions, is short-circuited tounguardedinclaimAdvanceand never reaches the'lost'branch at all. What is left is an unbounded per-process leak: one retainedSuspendedRunsnapshot (variables, context, screen) per lost claim, for the life of the process. On a busy any-of approval level — the very shape #14333 is about — every loser leaks one, forever.Note 2 — the non-count
'unsupported'branch answers after the delete has already gone outpackages/services/service-automation/src/suspended-run-store.ts,ObjectStoreSuspendedRunStore.claimSuspension(~:388):The refusal is decided on the shape of the return value, which is only known once the delete has already been issued against the shared row. So on a driver whose multi-delete resolves to something other than a number, the compare-and-set has been performed and its verdict thrown away. The engine then reads
'unsupported'asunguarded(claimAdvance), andforgetSuspendedRunissues a second, unconditional delete of the same row.Two costs, and the second is the one worth a card:
A store that cannot express the condition should determine that before mutating, e.g. by capability-probing the engine once (the
typeof this.engine.delete !== 'function'check directly above already sets that precedent) rather than by inspecting the result of a write it has already committed.Not in scope of the PR that found this
Found while implementing #14956 (the correlation-less node-comparison pin). That PR is test-only and changes no behaviour here; #14956 remains open on its own terms and is not addressed by this card.
Refs: #14333 / PR #14712 (merged) · #14956