fix(service-automation): a durable PAUSED run is visible to listRuns and run-detail after a cold restart (#8050)

.changeset/paused-run-visibility-after-restart.md

@@ -0,0 +1,39 @@
+---
+"@objectstack/service-automation": patch
+---
+
+fix(service-automation): a durable PAUSED run is visible to `listRuns` and run-detail after a cold restart (#8050)
+
+After a process restart, a run parked at an `approval` / `screen` / `wait` node
+disappeared from the automation API while remaining fully durable:
+
+| read | before | after |
+| :--- | :--- | :--- |
+| `GET /automation/:name/runs` | 200, **zero rows** | the parked run |
+| `GET /automation/:name/runs?status=paused` | 200, **zero rows** | the parked run |
+| `GET /automation/:name/runs/:runId` | **404** `RESOURCE_NOT_FOUND` | 200, `status: 'paused'` |
+
+`sys_automation_run` holds two disjoint row families — terminal history rows
+(`run_`-prefixed, written on completion) and live suspension rows (keyed by the
+raw run id, status `paused`). `AutomationEngine.listRuns` merged the in-memory
+ring buffer with the first family only, and `getRun` fell back to the first
+family only. Before a restart the gap is invisible because a paused run is still
+in the ring; after one, the ring is empty and the suspension rows had no reader.
+
+The sharp edge was `?status=paused`. #7359 had just made that a real filter, and
+with no post-restart producer of a `paused` entry it could never match a row —
+so the one query an operator reaches for when asking "what is in flight?" was
+structurally guaranteed to answer "nothing pending".
+
+This is a read-path change only. Nothing about persistence moves: suspension
+rows keep their own id space, lifecycle and retention exemption, and are **not**
+reshaped into history rows. Durability was never the defect — a parked run
+already served `…/runs/:runId/screen` and resumed cleanly across a restart, and
+still does.
+
+Merge precedence is now stated explicitly: durable paused → durable history →
+in-memory ring, weakest first. A paused row is the only source that can be stale
+(the delete on completion is best-effort), so a terminal row or ring entry for
+the same run id is later evidence and wins — a finished run is never reported as
+still waiting. The paused read is best-effort like the history read beside it: a
+store outage degrades the listing and logs the shortfall rather than throwing.

content/docs/automation/flows.mdx

@@ -711,6 +711,14 @@ panel. Recent runs are held in an in-memory ring buffer; terminal runs
 history with a bounded step log, so `listRuns` / `getRun` still report a run's
 status, steps, and failure reason after a restart or ring-buffer eviction.
 
+Runs still **in flight** survive the same way. A run parked at an `approval`,
+`screen` or `wait` node is persisted as a live suspension row, and both
+`listRuns` (including `?status=paused`) and `getRun` read those rows back — so
+after a restart the Runs view shows what is *waiting*, not only what finished.
+A paused run reports its trigger attribution and its variable snapshot exactly
+as it did before the restart; it carries no `durationMs`, because a suspension
+records when the run started, not when it parked.
+
 ### Run summaries
 
 A run that reports `success: true` has not told you it did its job. A scheduled

packages/plugins/plugin-approvals/src/approval-service.ts

@@ -118,10 +118,13 @@ export interface ApprovalResumeSurface {
  * a decision from being recorded against a run that can never advance
  * (#4420). Read-only; it never consumes the suspension.
  *
- * Distinct from {@link getRun}, which reports on the execution LOG: a run
- * suspended by a PREVIOUS process resolves to `null` there even when its
- * state is durable, so it cannot tell "waiting for a human" from "dead".
- * This asks the suspension store itself.
+ * Still distinct from {@link getRun}, but no longer on the axis this comment
+ * used to name: since #8050 `getRun` also sees a run suspended by a PREVIOUS
+ * process (it resolved `null` there before, unable to tell "waiting for a
+ * human" from "dead"). The difference that remains is the one a pre-flight
+ * turns on — this asks the suspension store and REJECTS when it cannot be
+ * read, where `getRun` degrades an outage to `null`. A caller about to WRITE
+ * must not accept a degraded read.
  *
  * Rejects when the durable store cannot be read — existence is then
  * unknown, and callers must not read an outage as a dead run. Optional: an
@@ -3023,10 +3026,14 @@ export class ApprovalService implements IApprovalService {
  * class of failure stayed silent.
  *
  * It also could not have answered the question even if it looked: its
- * liveness oracle is `getRun`, which reads the execution LOG, and after a
- * restart that returns `null` for a perfectly ALIVE suspended run. It treats
- * `null` as alive (conservative, correct) — but that means it has no way to
- * say "this run is really gone".
+ * liveness oracle is `getRun`, which treats both `null` and `paused` as alive
+ * (conservative, correct) — so it has no way to say "this run is really
+ * gone". (Until #8050 that oracle was weaker still: after a restart it
+ * returned `null` for a perfectly ALIVE suspended run, so "alive" and
+ * "unknown" were the same answer. It now reports such a run as `paused`,
+ * which does not change any branch here — both already meant "leave alone" —
+ * but it is why the sweep below needs `hasSuspendedRun` as a second oracle
+ * rather than a sharper reading of the first.)
  *
  * So this uses BOTH oracles, and a row must fail both to be reported:
  *

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

@@ -2556,6 +2556,68 @@ export class AutomationEngine implements IAutomationService {
  const limit = options?.limit ?? 20;
  const inMem = this.executionLogs.filter(l => l.flowName === flowName);
 
+ // [#8050] Durable PAUSED rows — the arm this merge was missing.
+ //
+ // `sys_automation_run` holds TWO disjoint row families: the terminal
+ // history rows `recordTerminal` writes (id `run_` + runId, status
+ // completed/failed) and the LIVE suspension rows `save` writes (id =
+ // the raw runId, status `paused`). The history arm below reads the
+ // first family; nothing here read the second. Before a restart that is
+ // invisible, because a paused run is still in `executionLogs` — so
+ // every in-process test of this method passes. After a restart the ring
+ // is empty and the paused rows had no reader at all, which is the
+ // defect: an operator who restarts the process can enumerate what
+ // FINISHED and what is IN FLIGHT vanishes — the strictly more urgent
+ // half. It also structurally emptied #7359's just-enforced
+ // `?status=paused`: with no producer of a `paused` entry after a
+ // restart, that filter could never match a row, and the one query
+ // reached for here always answered "nothing pending".
+ //
+ // Read-side only, by construction: this rehydrates the row the suspend
+ // path already writes. No column, prefix or lifecycle changes — the
+ // paused row is NOT reshaped into a history row, because the two have
+ // different lifetimes (a paused row is live resumable state, deleted on
+ // completion and exempt from the age sweep; a history row is a
+ // tombstone). Unifying them would trade an observability gap for a
+ // persistence-semantics change.
+ //
+ // Skipped when the caller filters for a status a paused row can never
+ // have: `suspendedRunToLogEntry` always yields `paused`, and a run that
+ // has since finished is answered by the fresher history/ring entry that
+ // outranks it in the merge below — so the arm cannot change the result
+ // of `?status=failed`, only its cost. `store.list()` is a table scan of
+ // every paused row in the deployment (see its own contract), so not
+ // paying it on the monitoring queries is worth the one-line guard.
+ const wantsPaused = options?.status === undefined || options.status === 'paused';
+ let durablePaused: ExecutionLogEntry[] = [];
+ if (this.store && wantsPaused) {
+ try {
+ const rows = await this.store.list();
+ durablePaused = rows
+ .filter(r => r.flowName === flowName)
+ .map(r => this.suspendedRunToLogEntry(r));
+ } catch (err) {
+ // #6499 — driver text to the structured slot; `warn(message,
+ // meta?)`, meta SECOND (no `Error` slot on `warn`).
+ //
+ // #4632 verdict: FUNCTIONAL — `warn`, for the same reason as
+ // the history arm below and `listSuspendedRunsDurable`. Nothing
+ // claimed-persisted failed to land: the paused rows are intact
+ // and still resumable by id (`resume` reads them through
+ // `loadSuspendedRun`, a different door that is unaffected by
+ // this failure). What degrades is this observability read, back
+ // to exactly the pre-#8050 answer.
+ this.logger.warn(
+ `[Automation] paused-run read failed for '${flowName}' — the Runs listing DEGRADES to the ` +
+ `in-memory ring buffer plus terminal history, so runs parked by a previous process are ` +
+ `missing and '?status=paused' can report an empty result for a flow that has runs ` +
+ `waiting. The rows themselves are untouched and still resumable by id. Fix the store ` +
+ `failure in this record's meta.`,
+ describeThrownForLog(err),
+ );
+ }
+ }
+
  // Merge durable run history so the "Runs" view survives a restart and
  // ring-buffer eviction. In-memory entries are the freshest (they carry
  // full step detail); durable rows backfill runs the process no longer
@@ -2588,7 +2650,28 @@ export class AutomationEngine implements IAutomationService {
  );
  }
  }
+ // Dedupe by run id, weakest source first — the same run legitimately
+ // appears in more than one of these (#8050):
+ //
+ // 1. durable PAUSED — the run parked, and the row is still there.
+ // 2. durable HISTORY — the run reached a terminal state.
+ // 3. in-memory ring — this process executed it.
+ //
+ // The order is a precedence claim, not an accident. Paused loses to
+ // both because it is the only one that can be STALE while the others
+ // cannot: `forgetSuspendedRun` deletes the paused row on completion,
+ // but that delete is best-effort (a store outage swallows it), so a
+ // finished run can leave its paused row behind. A terminal row or a
+ // terminal ring entry for the same id is therefore strictly later
+ // evidence, and letting the paused row win would report a completed run
+ // as still waiting — the exact defect `run-history.test.ts`'s "latest
+ // entry wins" block pins for `getRun`. There is no symmetric hazard:
+ // within a process the ring is written in the same breath as the paused
+ // row (`persistSuspendedRun` then `recordLog`, and again on re-suspend),
+ // so it is never the older of the two; across a restart the ring is
+ // empty and cannot mask anything.
  const byId = new Map<string, ExecutionLogEntry>();
+ for (const e of durablePaused) byId.set(e.id, e);
  for (const e of durable) byId.set(e.id, e);
  for (const e of inMem) byId.set(e.id, e); // freshest wins
 
@@ -2669,6 +2752,47 @@ export class AutomationEngine implements IAutomationService {
  };
  }
 
+ /**
+ * Rehydrate a durably-stored {@link SuspendedRun} into the `paused`
+ * {@link ExecutionLogEntry} the Runs surfaces expect (#8050) — the
+ * suspension-row twin of {@link runRecordToLogEntry}.
+ *
+ * Deliberately reconstructs the SAME entry the two `status: 'paused'`
+ * `recordLog` sites write, from the same inputs, so that whether a paused
+ * run is read before or after a restart is invisible to the caller:
+ *
+ * - `trigger` goes through {@link buildRunTrigger} on the persisted
+ * `context_json`, not through the flattened `trigger_*` columns. Those
+ * columns exist for FILTERING (#7533) and drop `type` to `null` where
+ * the log entry says `'manual'`; the context is what the ring entry was
+ * built from, so reusing the chokepoint reproduces it exactly rather
+ * than approximating it.
+ * - `variables` is carried because #7639 made it part of what a PAUSED run
+ * discloses on run-detail, and the row has held the same snapshot all
+ * along (`variables_json` is written from the very object handed to the
+ * log entry). Dropping it here would have re-opened #7639 for exactly
+ * the runs an operator most needs it for — the ones that outlived the
+ * process.
+ *
+ * `durationMs` / `completedAt` are absent because a suspension row records
+ * no pause instant — only `started_at` / `start_time`. Absent reads as "not
+ * recorded", which is what the schema's `optional()` means; inventing an
+ * age-since-start here would publish a number that grows every time the row
+ * is read and is not the "time spent executing" the ring entry reports.
+ */
+ private suspendedRunToLogEntry(run: SuspendedRun): ExecutionLogEntry {
+ return {
+ id: run.runId,
+ flowName: run.flowName,
+ flowVersion: run.flowVersion,
+ status: 'paused',
+ startedAt: run.startedAt,
+ trigger: buildRunTrigger(run.context),
+ steps: run.steps ?? [],
+ variables: run.variables ?? {},
+ };
+ }
+
  async getRun(runId: string): Promise<ExecutionLogEntry | null> {
  // LAST entry wins, not the first: a run that pauses and later finishes
  // records TWO entries under the same run id ('paused', then
@@ -2712,6 +2836,38 @@ export class AutomationEngine implements IAutomationService {
  );
  }
  }
+ // [#8050] …and the PAUSED fallback, so run-detail and `listRuns` answer
+ // out of one story. The card measured both surfaces failing together
+ // after a restart — list returning zero rows AND this method 404ing —
+ // and fixing only the list would have swapped a visible gap for an
+ // inconsistency between two reads of the same run.
+ //
+ // AFTER the terminal probe, matching the merge order in `listRuns`: a
+ // paused row can outlive the run it describes (the delete on completion
+ // is best-effort), so a terminal row for the same id is later evidence
+ // and must win. Trying this first would resurrect #3456's "paused
+ // forever" for any run whose cleanup delete was lost.
+ //
+ // This does NOT make a nonexistent run findable: `store.load` answers
+ // `null` for an unknown id exactly as `loadTerminal` does, so the route
+ // above still returns its 404 `RESOURCE_NOT_FOUND` envelope for one.
+ if (this.store) {
+ try {
+ const suspended = await this.store.load(runId);
+ if (suspended) return this.suspendedRunToLogEntry(suspended);
+ } catch (err) {
+ // #6499 / #4632: same verdict as the terminal probe above —
+ // FUNCTIONAL, so `warn`. The suspension row is intact and the
+ // run stays parked and resumable; what degrades is this read.
+ this.logger.warn(
+ `[Automation] durable paused-run lookup failed for '${runId}' — this read DEGRADES to null, ` +
+ `so a run that is parked and resumable reports as if it had never run, and the caller ` +
+ `cannot tell the two apart. The suspension row is untouched. Fix the store failure in ` +
+ `this record's meta.`,
+ describeThrownForLog(err),
+ );
+ }
+ }
  return null;
  }
 
@@ -3510,9 +3666,11 @@ export class AutomationEngine implements IAutomationService {
  * against a run that can no longer advance (#4420).
  *
  * THROWS when the durable store cannot be read — an outage means "unknown",
- * and a caller must not act on it as if the run were gone. Contrast
- * {@link getRun}, which reports on the execution LOG and returns null for a
- * run suspended by a previous process even when its state is durable.
+ * and a caller must not act on it as if the run were gone. That is the one
+ * axis {@link getRun} still differs on: since #8050 it, too, sees a run
+ * suspended by a previous process, but as an OBSERVABILITY read it degrades
+ * a store failure to `null` with a warning rather than throwing. Use this
+ * one before writing anything of your own; use `getRun` to display.
  */
  async hasSuspendedRun(runId: string): Promise<boolean> {
  return (await this.loadSuspendedRunStrict(runId)) !== null;