fix(automation): resume a paused run from the shared store, not from a stale per-replica snapshot

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**

.changeset/tall-moons-refuse.md

@@ -0,0 +1,30 @@
+---
+'@objectstack/service-automation': patch
+---
+
+Resume a paused flow run from the shared store, not from the replica's own memory of it
+
+On a multi-replica deployment over one database, approving a level of a multi-level
+approval flow could re-create the level that was just approved instead of opening the
+next one — so the same approver had to approve each level twice, and a three-level flow
+produced five approval requests. Landing the same stale read on the final level rolled
+the run back to the previous one and left it parked forever instead of completing.
+
+The engine kept paused runs in a per-process map and read that map before the durable
+`sys_automation_run` row, so a replica that had handled the run earlier answered from
+its own snapshot of the node the run was parked at — a snapshot nothing invalidates.
+Whichever replica the next decision reached then traversed forward from a node the run
+had already left. A single replica never showed it, because there is only one map and
+it is never behind.
+
+The resume path is now store-authoritative: with a `SuspendedRunStore` configured, the
+store answers where a run is parked, and the in-memory map is consulted only for a run
+whose durable save failed (the existing degradation, which keeps such a run resumable
+in-process and reports the lost durability at `error`). The ordering of the resume
+itself is unchanged — the suspension is still consumed before downstream traversal.
+
+Two consequences worth knowing: every resume now reads the store, so an unreadable
+store is reported as `STORE_UNAVAILABLE` for a run this process parked itself rather
+than being served a possibly-stale snapshot; and the approvals pre-flight
+(`hasSuspendedRun`) is answered from the same authoritative read, so a decision is no
+longer recorded against a run that another replica has already advanced or finished.

packages/services/service-automation/src/builtin/wait-node.test.ts

@@ -375,13 +375,20 @@ describe('wait timer teardown when the pause ends another way (#5512)', () => {
  * durable store unreadable, so per #4420 the pause is emphatically NOT gone —
  * that cancelled the only thing left that would ever wake the run.
  *
- * Reachability is not equal across the two sites, and these tests are built to
- * say so rather than to look symmetric: `resumeInternal` reads the durable store
- * only on a hot-cache MISS, and a run that paused in this process stays cached
- * for the life of its suspension. So the end-to-end specimen below is the
- * **re-arm** callback (fresh process, empty cache, store consulted for real);
- * the arming callback's branch is latent by construction and is pinned at the
- * handler level, with the code injected rather than provoked.
+ * Reachability was not equal across the two sites when these tests were built,
+ * and they were shaped to say so rather than to look symmetric: `resumeInternal`
+ * read the durable store only on a MISS of the engine's in-memory map, and a run
+ * that paused in this process was answered from memory for the life of its
+ * suspension. So the end-to-end specimen below is the **re-arm** callback (fresh
+ * process, empty map, store consulted for real), while the arming callback's
+ * branch was latent by construction and is pinned at the handler level, with the
+ * code injected rather than provoked.
+ *
+ * [#13617] The asymmetry is gone — a store-backed engine now reads the store on
+ * every resume — but the arming-path specimen below is unchanged on purpose: it
+ * runs on an engine with NO store at all, where there is no store read to fail,
+ * so what it pins is still the handler's branch and the arming site's routing
+ * through it, not a reachability claim.
  */
 describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)', () => {
  /** A logger that keeps its `error` lines so the diagnostic can be asserted. */
@@ -520,12 +527,11 @@ describe('wait timer one-shot vs. a shot that never consumed the pause (#5529)',
 
  it('arming path: the same handler keeps the job armed on STORE_UNAVAILABLE', async () => {
  // The arming callback shares one handler with the re-arm callback, so this
- // pins the branch on THAT site too. The code is injected, not provoked: a run
- // that paused in this process is in the engine's hot cache, so its own resume
- // never reads the durable store and cannot produce STORE_UNAVAILABLE here.
- // Fabricating a cache miss to "prove" otherwise would pin a scenario the
- // engine does not have — what is verified is the handler's branch, and that
- // the arming site routes through it rather than keeping its own `finally`.
+ // pins the branch on THAT site too. The code is injected, not provoked, and
+ // [#13617] did not change that: this engine is built with NO store, so no
+ // resume of it can produce STORE_UNAVAILABLE however it reads. What is
+ // verified is the handler's branch, and that the arming site routes through
+ // it rather than keeping its own `finally`.
  const { ctx, scheduled, cancelled } = fakeJobCtx();
  const engine = new AutomationEngine(silentLogger());
  const ran: string[] = [];

packages/services/service-automation/src/builtin/wait-node.ts

@@ -82,13 +82,16 @@ interface WaitTimerLogger {
  * found" and the only remaining path is the next boot's overdue re-arm pass.
  * Both remedies are named in the log line for that reason.
  *
- * Reachability differs by site, and the honest note is that they are not equal.
- * `resumeInternal` reads the durable store only on a hot-cache miss, and a run
- * that paused in *this* process is cached for as long as the suspension lives —
- * so the **re-arm** callback (a fresh process, empty cache) is where
- * `STORE_UNAVAILABLE` is genuinely reachable today, while the arming callback's
- * branch is latent by construction. It is shared anyway rather than special-cased:
- * a second spelling of "settle the one-shot" is exactly the drift #5512 collapsed.
+ * Reachability was once unequal by site, and this note used to say so: while
+ * `resumeInternal` read the durable store only on a miss of the engine's
+ * in-memory map, a run that paused in *this* process was answered from memory
+ * for the life of its suspension, so only the **re-arm** callback (a fresh
+ * process, empty map) could genuinely produce `STORE_UNAVAILABLE`. [#13617]
+ * ended that: the resume path is store-authoritative whenever a
+ * `SuspendedRunStore` is configured, so BOTH callbacks read the store and both
+ * reach this branch — the arming site is no longer latent by construction. The
+ * handler was shared before that was true and stays shared: a second spelling
+ * of "settle the one-shot" is exactly the drift #5512 collapsed.
  */
 function makeWaitTimerJobHandler(
  engine: Pick<AutomationEngine, 'resume'>,

packages/services/service-automation/src/engine.ts

@@ -1622,11 +1622,31 @@ export class AutomationEngine implements IAutomationService {
  private readonly runSummaryLog: RunSummaryLogLevel;
  private logger: Logger;
  /**
- * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache —
- * mirrored to {@link store} when one is configured, so a pause survives a
- * process restart. See {@link SuspendedRun}.
+ * Runs paused at a node, keyed by runId (ADR-0019). Process-local copy of
+ * the pause — mirrored to {@link store} when one is configured, so a pause
+ * survives a process restart. See {@link SuspendedRun}.
+ *
+ * [#13617] NOT a read-through cache sitting in front of the store. When a
+ * store is configured the STORE is the authority and this map answers only
+ * for the runs it never accepted ({@link cacheOnlySuspensions}). Reading
+ * this map first is what made a multi-replica approval flow re-create every
+ * level — the mechanism is in {@link loadSuspendedRunStrict}.
  */
  private suspendedRuns = new Map<string, SuspendedRun>();
+ /**
+ * [#13617] Runs whose durable save FAILED, so {@link store} holds no row
+ * for them and its "no such run" says nothing about them. These are the
+ * only runs {@link loadSuspendedRunStrict} will answer out of
+ * {@link suspendedRuns} while a store is configured — which is what keeps
+ * {@link persistSuspendedRun}'s documented degradation (a save failure
+ * costs cross-restart durability, not in-process resumability) working.
+ *
+ * Written by {@link persistSuspendedRun} — added when a save throws,
+ * cleared when one lands — and dropped alongside the cache entry by
+ * {@link forgetSuspendedRun}, the single choke point every consumption
+ * passes through, so it is bounded by the map it qualifies.
+ */
+ private cacheOnlySuspensions = new Set<string>();
  /**
  * Optional durable backing for {@link suspendedRuns}. When set, suspended
  * runs are persisted on suspend and rehydrated on resume after a restart;
@@ -1788,7 +1808,17 @@ export class AutomationEngine implements IAutomationService {
  if (this.store) {
  try {
  await this.store.save(run);
+ // [#13617] The store now holds this pause, so it — not this map
+ // — is the answer for it. Cleared here and not only on the
+ // failure path: a re-suspend whose save lands after an earlier
+ // one failed must stop being read out of memory.
+ this.cacheOnlySuspensions.delete(run.runId);
  } catch (err) {
+ // [#13617] The store was never given the row, so its "no such
+ // run" is silence about this run rather than an answer. This is
+ // what lets `loadSuspendedRunStrict` keep serving it from the
+ // map — the in-process resumability the message below promises.
+ this.cacheOnlySuspensions.add(run.runId);
  // #6499 — the cause is the datasource DRIVER's own text, so it
  // goes to the logger's STRUCTURED slot, never spliced into the
  // message; see `forgetSuspendedRun`'s catch below for the full
@@ -1831,6 +1861,10 @@ export class AutomationEngine implements IAutomationService {
  */
  private async forgetSuspendedRun(run: SuspendedRun, reason: SuspensionReleaseReason): Promise<void> {
  this.suspendedRuns.delete(run.runId);
+ // [#13617] The qualifier goes with the entry it qualifies — this is the
+ // one choke point every consumption passes through, so nothing can leave
+ // a run marked "the store never took this" after its map entry is gone.
+ this.cacheOnlySuspensions.delete(run.runId);
  if (this.store) {
  try {
  await this.store.delete(run.runId);
@@ -4521,12 +4555,52 @@ export class AutomationEngine implements IAutomationService {
  }
 
  /** {@link loadSuspendedRun} without the degradation: a store read failure
- * THROWS instead of reading as "no such run". */
+ * THROWS instead of reading as "no such run".
+ *
+ * [#13617] STORE-AUTHORITATIVE. When a {@link SuspendedRunStore} is
+ * configured, the store answers and {@link suspendedRuns} answers only for
+ * a run the store never accepted ({@link cacheOnlySuspensions}). It used
+ * to be the other way round — this process's map first, the store only on
+ * a miss — which is a correct read for exactly one deployment shape: a
+ * single process. Put several replicas behind a load balancer over one
+ * database and that map is a per-replica snapshot of the node a run was
+ * parked at THE LAST TIME THIS REPLICA TOUCHED IT, and nothing invalidates
+ * it, because there is no invalidation channel to it at all.
+ *
+ * The measured shape, a multi-level approval flow: replica A parks the run
+ * at `lv1` and keeps it in its map. The `lv1` decision round-robins to
+ * replica B, which advances the run to `lv2` in the store and in B's map;
+ * A's map still says `lv1`. The `lv2` decision lands back on A, which read
+ * its own map, resumed from `lv1`, and traversed to `lv2` a SECOND time —
+ * the same level re-created as a fresh pending request tens of
+ * milliseconds after the first one completed, so one approver approves
+ * every level twice. Land the same one-beat-stale read on the FINAL level
+ * and the run rolls back to the previous one instead of terminating. A
+ * single replica shows zero duplicates because there is one map and it is
+ * never behind.
+ *
+ * Both callers that must not be wrong funnel through here: `resumeInternal`
+ * (which node does this resume continue from) and {@link hasSuspendedRun}
+ * (the approvals pre-flight that decides whether to record a decision at
+ * all), so one seam settles both.
+ *
+ * ⛔ NOT a re-ordering of the resume path. The suspension is still consumed
+ * before `traverseNext` and {@link forgetSuspendedRun} is untouched —
+ * which ordering is right is #13937's question, and unruled. This changes
+ * only WHICH suspension is read, never when it is consumed. */
  private async loadSuspendedRunStrict(runId: string): Promise<SuspendedRun | null> {
- const cached = this.suspendedRuns.get(runId);
- if (cached) return cached;
- if (!this.store) return null;
- return await this.store.load(runId);
+ if (!this.store) return this.suspendedRuns.get(runId) ?? null;
+ const stored = await this.store.load(runId);
+ if (stored) return stored;
+ // The store has no row. For every run it ever accepted that IS the
+ // answer — including the runs this process advanced past, whose stale
+ // map entries are the whole defect above. The lone exception is a run
+ // whose durable save failed here: the store was never handed that row,
+ // so its silence says nothing about it, and `persistSuspendedRun`
+ // deliberately keeps such a run resumable in-process (it reports the
+ // lost durability at `error`).
+ if (this.cacheOnlySuspensions.has(runId)) return this.suspendedRuns.get(runId) ?? null;
+ return null;
  }
 
  /**