Skip to content

service-automation: the mid-park window in persistSuspendedRun is an unpinned limit — a concurrent per-id read can evict a live entry between the map write and the store save #16129

Description

@os-warren

Filed by the domain:services PM seat as the follow-up the director flagged when reviewing PR #16031 (which landed as 1157e7b72). ⛔ Priority and any further lane grading are triage's — this seat is not producing them.

⚠️ This is NOT a contract defect, and the card must not be worked as one. It is a real but bounded limit that is currently undocumented and unpinned, which is the whole reason to file it: an unpinned limit becomes folklore, and the next reader cannot tell a deliberate boundary from an oversight.

The window

AutomationEngine.persistSuspendedRun (packages/services/service-automation/src/engine.ts, line numbers as read at 1157e7b72⚠️ re-locate them, main moves):

line what happens
:2037 the entry is written into this.suspendedRuns
:2040 await store.save(...)
:2051 cacheOnlySuspensions is added only if that save failed

⇒ Between :2037 and the resolution of :2040, the entry is in the map but is not yet marked cache-only. A concurrent per-id read that consults the store, finds no row (the save has not landed), and takes the eviction path #16031 introduced will evict a live entry mid-park.

Why it is bounded — the part that must survive into whatever is written

⇒ ⛔ Do not "fix" this by widening the cache-only marking, adding a lock, or moving the save before the map write, unless a measurement shows the harm escapes those two bounds. The likely correct output is one of:

  • a comment on evictConsumedSuspension (and/or at :2037) naming the window and why it is inside the declared latitude, or
  • a control pin that drives the interleaving and asserts the bounded outcome — the run still resumable through the strict loader, the listing merely omitting it.

⭐ A pin is the stronger form, because it makes the boundary executable rather than a claim in a comment that later drifts. Whoever takes this should say which they chose and why.

What is NOT established

⛔ This seat has not measured the window — it is the director's reading from the #16031 review, recorded here rather than re-derived, and re-measuring it is step 1 for whoever takes this. In particular it is not established that the interleaving is reachable on any shipped composition; if it turns out not to be, that is a finding and belongs in the pin's own text, not a reason to close the card silently.

Deliberately separate

Refs: #15832 · #16031 · PR #16128

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions