Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/olive-parrots-attend.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
---
'@objectstack/service-automation': patch
---

**The last three readers of suspended-run state read the shared store, not this replica's memory of it.** #13617 made the resume path store-authoritative; `cancelRun`, `failAncestors` and `listSuspendedRunsDurable` still preferred the per-process `suspendedRuns` map, so on a replica holding a stale entry each acted on the node a run was parked at the last time THIS replica touched it. All three now take one answer to "where is this run parked", through the existing `loadSuspendedRun` / `loadSuspendedRunStrict` pair, with the degrading or strict loader chosen per site so each recorded degradation posture is preserved by choice rather than re-derived.

- **`cancelRun` — the strict loader.** Before: a stale replica cancelled from its own snapshot; the row deletion is by id and was right either way, but `forgetSuspendedRun` told the executor of the node in the SNAPSHOT that its pause was over, so the live node's pause stayed armed and a node the run had already left was released a second time. Now the shared row decides which pause is torn down. The strict loader is deliberate: "not found" still returns `false` (already terminal / unknown) exactly as before, and an unreadable store still lands on this seam's own #4632 DURABILITY record at `error` — the degrading loader would have answered `null` under its best-effort `warn` and silently downgraded that verdict. ⚠️ One consequence, stated: while a store is configured this process's map is no longer an answer, so a store outage now reaches that `error` record even for a run this replica is holding, where the old cache-first read cancelled from the local snapshot.
- **`failAncestors` — the degrading loader.** Before: a stale parent was failed at a node it had already left (#13617's own harm shape, one level up). The degrading loader is deliberate: this walk runs inside the catch arm already handling a run's failure, so it must not throw, and "a store failure reads as no ancestor here and stops the walk" is exactly the posture the bare `.catch(() => null)` had. The one thing gained beyond the fix: that silent swallow is now recorded, at the loader's declared best-effort `warn` — no new `error` seam.
- **`listSuspendedRunsDurable` — the merge direction, and the comment.** The durable row now wins an id collision; the comment claiming "In-memory entries win — they are the freshest copy" is corrected, since it is true of exactly one deployment shape. Map entries the durable listing does not carry are still included, deliberately: `store.list()` is a capped, best-effort enumeration (at most 1000 `paused` rows) and the same merge is reached on the degraded path, so "absent from the list" is not the per-id "the store answered and has no row" the strict loader rests on.

No signature, export or return-shape change on any of the three.
160 changes: 107 additions & 53 deletions packages/services/service-automation/src/engine.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5330,54 +5330,73 @@ export class AutomationEngine implements IAutomationService {
* indistinguishable to the caller, so the run may still be parked and
* resumable. That path is reported at `error` (#4632/#6299) precisely
* because nothing above it can tell the difference; see the catch below.
*
* [#14332] WHERE THE RUN IS READ FROM: {@link loadSuspendedRunStrict} —
* the same store-authoritative read `resumeInternal` takes, and the STRICT
* loader by deliberate choice rather than the degrading
* {@link loadSuspendedRun}. NOT FOUND (the store answered and holds no row
* for this id) reads as "already terminal / unknown" and returns `false`,
* exactly as before; a store that cannot be READ throws out of the loader
* into the catch below, which keeps this site's own #4632 DURABILITY record
* at `error`. The degrading loader would have answered `null` under its own
* best-effort `warn` and silently downgraded that verdict — the posture is
* preserved here by picking the loader that preserves it.
*
* ⚠️ The consequence of a store-authoritative read, stated rather than left
* to be discovered: while a store is configured this process's map is no
* longer an answer, so a store outage reaches the `error` record above even
* for a run THIS replica is holding — where the old cache-first read
* cancelled from the local snapshot instead. That snapshot is the defect:
* the row delete is by id and is therefore right either way, but
* {@link forgetSuspendedRun} notifies the executor of the node recorded on
* the SNAPSHOT, so a stale replica tore down the pause of a node the run had
* already left and left the live one's armed.
*/
async cancelRun(runId: string, reason?: string): Promise<boolean> {
let run = this.suspendedRuns.get(runId) ?? null;
if (!run && this.store) {
try {
run = await this.store.load(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
let run: SuspendedRun | null = null;
try {
run = await this.loadSuspendedRunStrict(runId);
} catch (err) {
// #6299 — same family, same mechanism as `forgetSuspendedRun`
// above: the driver's uncontrolled text goes to the structured
// slot so the record stays one physical line.
//
// #4632 verdict: DURABILITY — raised from `warn` to `error`. The
// failed read is silently turned into "no such suspended run"
// and this method returns `false`, which its own contract
// documents as idempotent success (already terminal / unknown),
// so the cancellation is SKIPPED while the call reads clean. The
// only in-repo caller measures the cost: plugin-approvals'
// revise-window recall
// (`packages/plugins/plugin-approvals/src/approval-service.ts`)
// never reads the boolean at all — it only catches a THROW, and
// grades that throw `error` with "the run may be stranded"
// (#4420). A store-read failure produces precisely that stranded
// run WITHOUT firing that alarm: the request is marked
// `recalled`, the record lock is released, `resumeError` stays
// undefined — and the run stays parked in the store, to be
// re-armed and resumed by the next restart, inside a flow whose
// approval has already been withdrawn.
//
// This is why #6230's verdict must not be copied here.
// `loadSuspendedRun` is a DECLARED best-effort reader for
// incidental callers (a gate lookup, a screen fetch), and
// `resumeInternal` takes the strict form exactly where the
// difference matters. `cancelRun` has no strict alternative, and
// its degradation decides a WRITE.
//
// THIRD argument (`error(message, error?, meta?)`), `Error` slot
// deliberately empty (#5575).
this.logger.error(
`[automation] cancelRun('${runId}') could not read the durable suspended-run store, so the ` +
`cancellation was SKIPPED and reported as idempotent success — this call returns false, which ` +
`its callers read as "no such suspended run". The run is NOT cancelled: if it is parked in the ` +
`store it stays parked, and the next restart re-arms and resumes it while the caller has ` +
`already recorded the cancellation. Fix the store failure in this record's meta, then re-issue ` +
`cancelRun('${runId}').`,
undefined,
describeThrownForLog(err),
);
}
if (!run) return false;
await this.forgetSuspendedRun(run, 'cancelled');
Expand DownExpand Up@@ -5747,9 +5766,19 @@ export class AutomationEngine implements IAutomationService {
let parentId = (context as Record<string, unknown> | undefined)?.$parentRunId;
let hops = 0;
while (typeof parentId === 'string' && parentId && hops++ < 32) {
const parent =
this.suspendedRuns.get(parentId) ??
(this.store ? await this.store.load(parentId).catch(() => null) : null);
// [#14332] The DEGRADING loader, by deliberate choice: this walk runs
// inside the catch arm that is already handling a run's failure, so it
// must not throw, and its recorded posture is exactly
// `loadSuspendedRun`'s — a store failure reads as "no ancestor here"
// and stops the walk. What changes is only WHICH suspension is read:
// the shared store's, not this replica's memory of where the parent
// was last parked. The old `??` chain had #13617's own harm shape —
// a stale parent failed at a node it had already left, so
// `forgetSuspendedRun` released the wrong node's pause. The one thing
// gained beyond that: the bare `.catch(() => null)` swallowed a store
// failure in total silence, and the loader records it (at `warn`,
// its declared best-effort level — no new `error` seam here).
const parent = await this.loadSuspendedRun(parentId);
if (!parent) return;
await this.failSuspendedRun(parent, `subflow descendant failed: ${error}`);
parentId = (parent.context as Record<string, unknown> | undefined)?.$parentRunId;
Expand All@@ -5776,9 +5805,14 @@ export class AutomationEngine implements IAutomationService {

/**
* Like {@link listSuspendedRuns} but includes runs held only in the durable
* {@link SuspendedRunStore} (e.g. suspended before a restart). The in-memory
* cache takes precedence on id collisions. Falls back to the in-memory list
* when no store is configured.
* {@link SuspendedRunStore} (e.g. suspended before a restart). Falls back to
* the in-memory list when no store is configured.
*
* [#14332] The DURABLE row wins an id collision — the store is the shared
* answer to "where is this run parked" and this process's map is only its
* own memory of it. A run present in the map but absent from the durable
* listing is still included, because a capped or failed enumeration is not
* the per-id "no row" the strict loader rests on; see the merge below.
*/
async listSuspendedRunsDurable(): Promise<Array<{ runId: string; flowName: string; nodeId: string; correlation?: string }>> {
const byId = new Map<string, { runId: string; flowName: string; nodeId: string; correlation?: string }>();
Expand DownExpand Up@@ -5831,8 +5865,28 @@ export class AutomationEngine implements IAutomationService {
);
}
}
// In-memory entries win — they are the freshest copy.
// [#14332] The DURABLE row wins a collision. The comment this replaces
// said the opposite — "In-memory entries win — they are the freshest
// copy" — which is true of exactly one deployment shape, a single
// process. Put several replicas over one store and this map is a
// per-replica snapshot of the node a run was parked at THE LAST TIME
// THIS REPLICA TOUCHED IT, with no invalidation channel to it at all
// (the mechanism is in {@link loadSuspendedRunStrict}), so preferring it
// reported a run at a node it had already left.
//
// Map entries the durable list does not carry are still appended, and
// that is NOT the strict loader's rule being softened: a LIST is not a
// per-id answer. `store.list()` is a capped, best-effort enumeration
// (`ObjectStoreSuspendedRunStore` reads at most 1000 `paused` rows) and
// this line is also reached on the DEGRADED path above, where the
// enumeration failed outright and `byId` is empty. So "absent from the
// list" is not the evidence "the store answered and has no row" is,
// which is what {@link loadSuspendedRunStrict} rests on when it lets
// only {@link cacheOnlySuspensions} answer out of the map. Applying that
// qualifier here would let a truncated or failed enumeration silently
// drop live runs from an operability listing.
for (const r of this.suspendedRuns.values()) {
if (byId.has(r.runId)) continue;
byId.set(r.runId, { runId: r.runId, flowName: r.flowName, nodeId: r.nodeId, correlation: r.correlation });
}
return [...byId.values()];
Expand Down
Loading
Loading