Skip to content

Commit 4e090ec

Browse files
os-warrenclaude
andauthored
fix(service-automation): decide the claim capability before the compare-and-set, never after it (#16128)
* fix(service-automation): decide the claim capability before the compare-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 * wip(#15832): preserve in-progress Note 2 pin work after a container restart 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d5d8d50 commit 4e090ec

5 files changed

Lines changed: 772 additions & 10 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
`ObjectStoreSuspendedRunStore` no longer announces "no cross-replica advance guarantee is offered" *after* it has issued the guarded delete.
6+
7+
`claimSuspension` is the cross-replica half of the resume idempotency guard: it removes the `sys_automation_run` row only if the run is still parked where this replica read it, and the affected-row count names the winner. The refusal for an engine that does not resolve such a count was decided on the SHAPE of the return value — one line after the compare-and-set had already gone out. On such an engine that made the refusal a statement about a write that had already landed: the conditional delete was performed against the shared row and its verdict discarded, `AutomationEngine.claimAdvance` read `'unsupported'` as `unguarded`, and a replica that **actually lost** the claim (0 rows affected) resumed anyway — running every downstream side effect a second time, on the one composition that declares itself unable to prevent that.
8+
9+
The capability question is now settled before anything is claimed, and `'unsupported'` is retired as an answer once the row has been touched:
10+
11+
- **A one-time capability probe, before the compare-and-set.** Once per store instance, `claimSuspension` issues one delete down the very route the claim takes (`multi: true` with a `where` carrying keys besides `id`, which is what dispatches to `driver.deleteMany`) against a sentinel predicate that matches no row — the same value in `id`, `node_id` and `correlation` at once. An engine that resolves something other than a count is refused with **nothing consumed**, so `claimAdvance`'s `unguarded` reading is true when it is taken. Concurrent first claims share one probe, and a probe that *throws* is deliberately not memoized: a store that was unreachable for one second must not answer for the life of the process.
12+
- **After the write, an unreadable verdict is `STORE_UNAVAILABLE`, not `unguarded`.** 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. The store throws instead of answering `'unsupported'`; `claimAdvance` already maps that to `STORE_UNAVAILABLE`, whose text is written for exactly this fact ("a failure can arrive after a committed delete"), and the resume is **refused** rather than continued. A claim that in fact won is then stranded until an operator retries — the deliberate direction, since a doubled side effect is the worse outcome.
13+
14+
**What this does not do, stated so it is not read into it.** It does not give an uncounted engine the guarantee. `ObjectQL.delete` declares `Promise<any>`, so "does a multi-delete return a count" has no contractual answer to look up and no read-only instrument to measure — a probe can observe the route once, never promise what the next call resolves to. Closing that gap belongs to the engine boundary, where the count is contracted one layer down (`IDataDriver.deleteMany`, `Promise<number>`) and erased to `any` on the way up. On such a composition the store still degrades to an unguarded resume; what changed is that it says so before consuming anything, and the run's durable row is still removed by the consumption choke point exactly as before.
15+
16+
Every measured shipped composition already resolves a count (memory, sql/better-sqlite3, sqlite-wasm, turso local and remote transport, sql with the security plugin composed), so the observable cost there is one extra `DELETE … WHERE` that matches nothing, once per process. It emits no hook dispatch, no realtime event and no row change: the per-row before phase is "zero matched rows is zero dispatches", the after phase iterates the same empty set, and `publishBulkDataEvent` returns at `matched === 0` by design.

0 commit comments

Comments
 (0)