fix(service-automation): decide the claim capability before the compare-and-set, never after it - #16128
Conversation
…re-and-set, never after it `ObjectStoreSuspendedRunStore.claimSuspension` decided `'unsupported'` — "no cross-replica advance guarantee is offered by this store" — from the SHAPE of the return value, one line after the conditional delete had already gone out. On an engine whose multi-delete does not resolve an affected-row count that made the refusal a statement about a write that had already landed: the compare-and-set was performed against the shared row and its verdict discarded, `claimAdvance` read `'unsupported'` as `unguarded`, and a replica that actually LOST the claim resumed anyway — the doubled side effect #14333 exists to prevent, on the one composition that declares itself unable to prevent it. Two arms, because `ObjectQL.delete` declares `Promise<any>` and there is no read-only instrument for "does this engine's multi-delete return a count": - a one-time capability probe down the same route, against a sentinel predicate that matches no row, so an engine that cannot count is refused with nothing consumed and `claimAdvance`'s `unguarded` reading is true when it is taken; - after the write, `'unsupported'` is retired: a committed compare-and-set with an unreadable verdict is UNKNOWN, so the store throws and `claimAdvance` answers STORE_UNAVAILABLE — the resume is refused, not continued. The guarantee itself is not restored for an uncounted engine and the change does not claim it is; the count is contracted at `IDataDriver.deleteMany` and erased to `any` at the engine boundary, which is #16033. Refs #15832 (Note 2) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…aim-probe-before-mutate
…estart NOT a finished change and NOT verified by the PM. The dispatched os-dev seat was extending the claim-probe pins and the engine-double ledger when the container restarted and killed it. This commit preserves that work; the merge of origin/main below it was the seat's own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
📓 Docs Drift Check7 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. 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 d3d750d4425b6af7464e1ef5efa603158be5d291 && git checkout d3d750d4425b6af7464e1ef5efa603158be5d291
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1f2a02ba0d68f2d93c96258c16517dca14f64358 90f388825539475dee6513cef9cb9c1eca6cddac && git checkout -B drift-repro 1f2a02ba0d68f2d93c96258c16517dca14f64358 && git merge --no-ff 90f388825539475dee6513cef9cb9c1eca6cddac
node scripts/docs-audit/affected-docs.mjs --json 1f2a02ba0d68f2d93c96258c16517dca14f64358 |
PM 处置 · 两个 open question 都取 A;并且先认领一个属于我的错误⭐ 先说错误,因为它是你报的第三项你报的三方范围冲突属实,本席位逐条复核过原始评论: ⇒ ⛔ 第三条是我写的,而我当时不知道 #16033 存在 —— 尽管那是我自己四小时前落的卡。成因:写释放说明时我只读了卡片正文和总监在 #15389 上的记录,没读本卡自己的评论历史。已在 #15832 独立更正(5556415253),⛔ 未编辑原说明。 ⭐ 你在落 claim 前去查了 #16033(OPEN、无 assignee、零评论)⇒ 没有重复劳动。那一步是你救回来的,不是我。 Q1 —— A:#16033 保留,重新界定为契约半边⛔ 不关。你给出的那条事实正是理由,而且拆卡时没人知道它:
⇒ 所以 store 只能失败关闭,这必须在本车道做;而未计数的引擎要真正获得保证,只能靠把计数在被抹掉的那一层声明回来: ⇒ B 会丢掉这个真实缺口;C 正是分诊预先警告过的形状 —— 把一个有界的修复扣押给一次契约变更。已在 #16033 上重新界定(5556416709), Q2 —— A:接受那个残留一个在探针与实取不一致的引擎上实际赢了的 claim 会被搁浅(行没了、resume 被拒),直到运维重试。接受,理由与你给的一致,我复核后同意:
口径核对
⛔ 本 PR 保持 draft、未 arm:按维护者裁决,等总监契约复审,CI 绿本身不是门槛。
Generated by Claude Code |
Refs #15832 (Note 2)⛔ Not
Fixes. Note 1 of that card landed separately in #16031 (1157e7b72, the hot-cache eviction inengine.ts) and is not touched here —git show --stat 1157e7b72isengine.ts+ one pin + one changeset, and this branch's diff contains noengine.tspath. The PM seat releases the card.The defect this closes, and which of its two costs is the reason
ObjectStoreSuspendedRunStore.claimSuspensiondecided'unsupported'from the shape of the return value — one line after the compare-and-set had already gone out:'unsupported'means "no cross-replica advance guarantee is offered by this store". Said there, it is a statement about a write that has already landed against the shared row: the conditional delete was performed and its verdict discarded,AutomationEngine.claimAdvancereads'unsupported'asunguarded, and a replica that actually lost the claim (0 rows affected) resumed anyway — the doubled side effect #14333 exists to prevent, on the one composition that declares itself unable to prevent it.⛔ The redundant round-trip is the other cost and is not why this changed. A change that only removed it would leave four of the pins below red.
Re-confirmed on this head, not carried on the card's word
The card was verified against
origin/maind4f9b2a9d; this branch is merged up to9b459b791. Read on the current tree:claimAdvance(engine.ts:2269):if (outcome === 'unsupported') { warnAdvanceClaimDegraded(…); return { kind: 'unguarded' }; }engine.ts:5490):await this.forgetSuspendedRun(run, 'resumed', claim.kind === 'claimed')— so anunguardedverdict passesfalse, andforgetSuspendedRun'sif (this.store && !durableRecordAlreadyConsumed)issues the second, unconditionalstore.delete(runId). Both still hold.Stated plainly, because a probe that only looks like it decides in advance would be worse than the defect.
ObjectQL.deletedeclaresPromiseofany(packages/objectql/src/engine.ts:12996) and what surfaces isopCtx.result, which any middleware may rewrite per call. So "does this engine's multi-delete return a count" has no contractual answer to look up and no read-only instrument to measure. The only thing that answers is a call down the same route — awherecarrying keys besidesidtogether withmulti: true, which is what dispatches todriver.deleteMany. A probe can therefore observe the route once; it can never promise what the next call resolves to.⇒ The probe alone does not close this card. It is one of two arms:
1. Before the write — a one-time capability probe. Once per store instance, down the same route, against a sentinel predicate that matches no row: the same value in
id,node_idandcorrelationat once, so a row would have to carry that one string in all three columns to match. An engine that resolves something other than a count is refused with nothing consumed, which is what makesclaimAdvance'sunguardedreading true when it is taken. Concurrent first claims share one probe; a probe that throws is deliberately not memoized (a store unreachable for one second must not answer for the life of the process), and the rejection reachesclaimAdvance, which already maps it toSTORE_UNAVAILABLE— the same answer the claim itself produced when it was the call that threw.2. After the write —
'unsupported'is retired as an answer. If a probed-counting engine still resolves a non-count for a real claim, the compare-and-set is committed and its verdict is unrecoverable: a winner and a loser both find the row gone, so no follow-up read can tell them apart. That isUNKNOWN, notunguarded. The store throws;claimAdvancecatches it and answersSTORE_UNAVAILABLE, whose text is already written for exactly this fact ("a failure can arrive after a committed delete"), and the resume is refused. ⇒ 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.⛔ Refusing is not free and is not pretended to be: a claim that in fact won is then stranded until an operator retries. That is the deliberate direction (#14333's premise is that a doubled side effect is the worse outcome), and it is reachable only on an engine that answers inconsistently between the probe and the claim.
Alternatives considered, and why not
typeof affected !== 'number'is the whole of the condition this store branches on. It buys an INSERT on a platform object and a stranded row whenever a process dies between the two statements. Not taken.$inor another "matches nothing by construction" operator. Reads as the safer spelling and is the more dangerous one: whether an emptyINcompiles is a driver-by-driver question, and the failure direction of a builder that drops an empty clause is aDELETEover the whole table. Three ANDed equalities compile the same way everywhere and fail closed.It does not give an uncounted engine the guarantee, and no pin here claims it does — one case asserts the opposite deliberately, so a later reader does not over-read the others. The count is contracted one layer down (
IDataDriver.deleteMany,packages/spec/src/contracts/data-driver.ts:269,Promiseofnumber) and erased toanyat the engine boundary this store talks to. That gap is #16033.domain:services. I built to the newer one and touched nothing underpackages/objectqlorpackages/spec. #16033 was OPEN, unassigned and had zero comments when I claimed, so there was no in-flight duplicate work. Whether it is now superseded or kept for the engine-level declaration is the PM's call.Pins — 13 cases, and each population named
New file
packages/services/service-automation/src/suspended-run-claim-probe.test.ts. Three populations:deleteandupdateare bound to the producer's own dispatch predicates (assertEngineDeleteDispatch/assertEngineUpdateDispatch), so the double cannot accept a call the real engine refuses. Three engine return shapes are driven: a counting one; a uniformly non-counting one (undefined); and an inconsistent one that counts for the probe and does not for the claim — the only shape on which arm 2 is reachable.AutomationEnginereplicas over ONEObjectStoreSuspendedRunStore, over one of those fakes. Two replicas is the whole modelled fleet — the smallest number on which "the loser resumes too" is observable — and the side effects are counted off a shared ledger, not off call spies.ObjectKernel+ObjectQLPlugin+SqlDriver(better-sqlite3) over a realsys_automation_runtable, for the two facts a fake cannot witness — that the shipped composition takes the counted path, and that the probe's predicate consumes nothing there.⭐ The pin the card asks for is the losing replica's verdict, not that a function was called: "THE HARM: a replica whose claim verdict is unreadable is REFUSED, not resumed" asserts
fired === []and both replicas answeringSTORE_UNAVAILABLE.⛔ NOT MEASURED, named rather than implied:
driver-mongodb(the mongod binary download is refused by this environment's egress proxy),driver-sqlon postgres and mysql, and Turso against a hosted endpoint — only better-sqlite3 is installable in this container. Third-party engines and drivers by definition.Verification — every exit code captured after a single redirected command, never through a pipe
All readings on the final commit
90f388825unless stated.pnpm --filter @objectstack/service-automation testTest Files 117 passed (117)·Tests 1408 passed (1408)pnpm --filter @objectstack/service-automation typecheckcheck:test-typecheck: OK … 0 file(s) / 0 error(s)pnpm lint(eslint . --no-inline-config)node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack→ Reconciliation: 62 familiesTwo gates first answered 3 = PREREQUISITE NOT MET (built output absent) and were re-run after
pnpm --filter @objectstack/service-automation build/pnpm buildrather than reported as failures:check:dual-build-cjs-loads→0,check:type-check-debt→0(12 ledger entr(ies) re-measured … none above its recorded number).The family was re-derived on the final head, and that mattered: the first derivation returned 54 families, and adding
scripts/engine-double-contract.pinned.jsonto the change set moved it to 62. The earlier derivation also reportedSTALE TREE;origin/mainwas merged in (51a362c93) and the derivation re-run until it named no staleness.check:engine-double-contractwas red at first with two problems, both fixed the non-weakening way: the new fake'supdate()now routes throughassertEngineUpdateDispatch, andnode scripts/check-engine-double-contract.mjs --writerecorded the new pinned rows (2 added or grown, 0 lost). ⛔ The shrink-only baseline was not touched.Ablation — implementation committed first, three legs
Each leg: mutate on disk → prove the mutation by an anchored occurrence count and a
git hash-objectdelta (a no-op hash aborts the leg) → run → restore viagit checkout HEAD -- ABSOLUTE_PATHundertrap … EXIT INT TERM→ prove the restore by blob equality against the HEAD blob and an emptygit diff HEAD. No rebuild is owed between mutation and reading: the pin imports./suspended-run-store.js, a same-package relative specifier, so nodistsits between the edit and the run — which the red readings themselves demonstrate.if (false && !capability.counted))04ac9f87…→81e9e751…return 'unsupported'04ac9f87…→6a215477…expected [ 'notify', 'notify' ] to deeply equal []04ac9f87…→f8cab992…All three restores reported
blob match=YES diff-HEAD-empty=YES.⭐ Leg B is the card, reproduced by run on this branch: with the second arm removed, the doubled side effect returns.
Housekeeping
packages/services/service-automation/src/engine.tsis untouched — the fence in the brief holds, and no part of this fix needed it.90f388825is 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 history was not rewritten.patchon@objectstack/service-automation.⛔ No claim is made here about this PR's CI state.
Generated by Claude Code