Skip to content

Commit db6133a

Browse files
os-warrenclaude
andcommitted
chore(changeset): patch for the suspended-run cache eviction
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
1 parent 622120a commit db6133a

1 file changed

Lines changed: 64 additions & 0 deletions

File tree

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
fix(service-automation): evict a suspension consumed by another replica, so the run listings stop reporting phantoms (#15832)
6+
7+
`AutomationEngine` had exactly one eviction site for its `suspendedRuns`
8+
map, inside `forgetSuspendedRun` — and that runs in whichever process
9+
**consumes** the suspension. In a multi-replica deployment that is routinely
10+
not the process that parked it: replica A parks a run, replica B resumes it,
11+
and nothing ever removes A's entry. There is no invalidation channel from B
12+
to A.
13+
14+
The card that found this located the leak on `resumeInternal`'s
15+
`claim.kind === 'lost'` branch, which returns before that choke point. That
16+
branch does leak, but it is not the common shape: the **no-race** variant
17+
leaks identically — A parks, only B ever resumes, A never attempts a claim
18+
and there is no `'lost'` anywhere in the sequence — so an eviction hung on
19+
`'lost'` alone would have left the ordinary deployment untouched.
20+
21+
The retained snapshot was **not only memory**. Two readers handed it back:
22+
`listSuspendedRuns()` (synchronous, cache-only, and the one listing on the
23+
`AutomationService` spec contract) and `listSuspendedRunsDurable()` (which
24+
deliberately appends map entries the durable list lacks). Once the other
25+
replica **completed** the run, both reported a phantom — a finished run
26+
listed as suspended, whose `getSuspendedScreen()` answers `null`, so a
27+
consumer that listed and then opened got an entry it could not act on.
28+
29+
An entry is now dropped whenever this process holds a store-authoritative,
30+
per-id "no row" answer for it: the strict loader's store miss (which reaches
31+
`resume`, `hasSuspendedRun`, `cancelRun` and `getSuspendedScreen`), a lost
32+
advance claim, and a bounded per-id reconcile for the map-only entries of
33+
`listSuspendedRunsDurable()`.
34+
35+
**Nothing here moves the cache-only listing's contract.** The fix only ever
36+
*removes* entries. The spec says `listSuspendedRuns()` lists "the currently
37+
suspended (paused) runs awaiting a resume"; the engine's own docblock adds
38+
only that it may OMIT runs (those parked in a previous process lifetime),
39+
because it reads the cache alone. Under-reporting is therefore already
40+
inside the declared latitude, and over-reporting was never inside the
41+
promise. Neither listing becomes store-backed, and `listSuspendedRuns()`
42+
stays synchronous.
43+
44+
Three shapes are deliberately **never** evicted, each pinned by a control:
45+
no store attached (the map IS the authority); a run whose durable save
46+
failed (`cacheOnlySuspensions` — the store was never handed the row, so its
47+
silence says nothing about it); and a store read that THROWS (an outage
48+
means the run's existence is unknown, not gone). A failed `list()`
49+
enumeration likewise triggers no per-id reconcile — during an outage that
50+
would ask about every live run in the process.
51+
52+
**Residual, stated rather than implied.** Eviction is demand-driven: a
53+
phantom is cleared when this process next obtains the per-id answer for that
54+
run — any `resume` / `hasSuspendedRun` / `getSuspendedScreen`, or a
55+
`listSuspendedRunsDurable()` reconcile. A process that never looks at the
56+
run again keeps the entry until it does. With no invalidation channel
57+
between replicas, closing that last gap needs either a background sweep or a
58+
store-backed listing, and both are decisions above this change; the boundary
59+
is pinned by a `RESIDUAL` test rather than left to be discovered.
60+
61+
Note 2 of the same card — the `'unsupported'` branch deciding on the shape of
62+
a value the conditional delete has **already** been issued to obtain — is
63+
**not** addressed here: its honest fix is a declared return contract for the
64+
engine's multi-row delete, which lands in another package.

0 commit comments

Comments
 (0)