From 20099de2f54670e0e1dafe586be7e52108f679a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 18:00:57 +0000 Subject: [PATCH 1/5] wip: #13909 slice 2 operator exit verb --- .../src/consumed-suspension-restore.test.ts | 591 ++++++++++++++++++ .../services/service-automation/src/engine.ts | 546 +++++++++++++++- .../src/suspended-run-store.ts | 114 ++++ 3 files changed, 1248 insertions(+), 3 deletions(-) create mode 100644 packages/services/service-automation/src/consumed-suspension-restore.test.ts diff --git a/packages/services/service-automation/src/consumed-suspension-restore.test.ts b/packages/services/service-automation/src/consumed-suspension-restore.test.ts new file mode 100644 index 0000000000..7b131ffd6e --- /dev/null +++ b/packages/services/service-automation/src/consumed-suspension-restore.test.ts @@ -0,0 +1,591 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #13909 slice 2 — **the operator exit from a run a resume left terminally + * unresumable**, and every one of its refusals. + * + * ## The state under test + * + * `AutomationEngine.resumeInternal` consumes the suspension BEFORE running the + * downstream nodes (`forgetSuspendedRun(run, 'resumed')` precedes + * `traverseNext`), so a node that merely THROWS throws with the pause already + * gone and the catch arm records the run `failed`. Slice 1 measured that this + * is terminal: `resume` answers `RUN_NOT_FOUND`, `cancelRun` is a no-op on it, + * the REST run surface has no cancel or retry route, and none of the engine's + * public methods moved such a run out. + * + * ⛔ **This file changes nothing about that ordering.** Which ordering is right + * is #13937, unruled and in the maintainer's hands. What is pinned here is the + * EXIT: `restoreConsumedSuspension` puts the consumed suspension back so the + * run is resumable again, under the ordering exactly as it is. + * + * ## What is pinned, and why each one is here + * + * 1. **The exit works end to end** — a run in the terminal post-resume-failure + * state is moved back to resumable AND can then actually be resumed to + * completion. Half of that (a `restored: true` that leaves the run no more + * resumable than before) would be a verb that reports success and delivers + * nothing. + * 2. **Every refusal separately, each with its OWN reason.** The reasons ARE + * the deliverable: an operator whose repair is declined has to be able to + * tell "this run is fine" from "this run is beyond this verb" from "I could + * not read the store", because the remedy differs for each. A single "it + * refuses bad input" test would cover none of that. + * 3. ⭐ **The in-flight resume.** "Suspension gone, no terminal row yet" is + * exactly what a live resume looks like from outside, and re-arming one of + * those races it. Pinned by calling the verb from INSIDE the downstream + * node, i.e. at the one instant the race is real. + * 4. **Idempotence**, both halves: sequential (the second caller finds a live + * suspension) and concurrent (one `restored: true`, one refusal) — and in + * both cases exactly ONE suspension exists afterwards. + * 5. **The trace** — the whole reason this class stayed silent is that nothing + * recorded it, so an exit that is itself invisible repeats the defect. + * 6. **Across a restart** — the deployment shape ADR-0019 exists for. An exit + * that only works inside the process lifetime that stranded the run would + * answer almost never: these runs are found hours later, by a sweep or a + * support ticket, in some other process. + * 7. **Verbatim, and the snapshot's own lifecycle** — what goes back is the + * pause as it stood, and a run that is restored and then finishes clears + * its own snapshot instead of staying restorable forever. + */ + +import { describe, it, expect, vi } from 'vitest'; + +import { AutomationEngine, type SuspendedRunStore } from './engine.js'; +import { InMemorySuspendedRunStore } from './suspended-run-store.js'; +import type { AutomationContext } from '@objectstack/spec/contracts'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; + +const silent = { info() {}, warn() {}, error() {}, debug() {} } as never; + +/** + * `resumeAuthority: 'any'` is required of a pausing fixture since #5561 — these + * tests continue their pause through the public `resume` door. Nothing here is + * about the resume gate (`resume-authority-gate.test.ts` owns that). + */ +const pauser = (type: string) => defineActionDescriptor({ + type, version: '1.0.0', name: type, + supportsPause: true, resumeAuthority: 'any', +}); + +const plain = (type: string) => defineActionDescriptor({ type, version: '1.0.0', name: type }); + +/** start → pause (suspends) → after (the node that throws) → end. */ +function flowDef(name: string) { + return { + name, label: name, type: 'autolaunched', + variables: [{ name: 'ticket', type: 'text', isInput: true, isOutput: true }], + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'pause', type: 'pause_here', label: 'Pause' }, + { id: 'after', type: 'after_pause', label: 'After' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'pause' }, + { id: 'e2', source: 'pause', target: 'after' }, + { id: 'e3', source: 'after', target: 'end' }, + ], + }; +} + +/** A flow with no pause at all — for the never-suspended refusal. */ +function noPauseFlow(name: string) { + return { + name, label: name, type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'boom', type: 'always_throws', label: 'Boom' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'boom' }, + { id: 'e2', source: 'boom', target: 'end' }, + ], + }; +} + +function registerPauser(engine: AutomationEngine) { + engine.registerNodeExecutor({ + type: 'pause_here', + descriptor: pauser('pause_here'), + async execute() { + return { success: true, suspend: true, correlation: 'approval:req_1', output: { stage: 'awaiting' } }; + }, + } as never); +} + +/** + * The downstream node. `throwUntil` counts the resumes that must fail — the + * FIRST resume strands the run, and a later one (after the operator repaired + * whatever broke) has to be able to finish it, which is what makes the exit an + * exit rather than a status change. + */ +function registerDownstream(engine: AutomationEngine, state: { throws: boolean; onEnter?: () => Promise }) { + engine.registerNodeExecutor({ + type: 'after_pause', + descriptor: plain('after_pause'), + async execute() { + if (state.onEnter) await state.onEnter(); + if (state.throws) throw new Error('downstream node blew up'); + return { success: true, output: { done: true } }; + }, + } as never); +} + +const ctx = { event: 'test', record: { id: 'rec_1' }, params: { ticket: 'TKT-9' } } as unknown as AutomationContext; + +/** Drive a run into the exact state this card is about. Returns its id. */ +async function strandRun(engine: AutomationEngine, flowName = 'strand_flow'): Promise { + const started = await engine.execute(flowName, ctx); + expect(started.status).toBe('paused'); + const runId = started.runId as string; + const failed = await engine.resume(runId); + // The mechanism, restated as an assertion rather than assumed: the run is + // terminal and the pause is gone. + expect(failed.success).toBe(false); + expect(await engine.hasSuspendedRun(runId)).toBe(false); + // …and it is TERMINAL — the existing doors do not move it. + const again = await engine.resume(runId); + expect(again.code).toBe('RUN_NOT_FOUND'); + expect(await engine.cancelRun(runId)).toBe(false); + return runId; +} + +function newEngine(store?: SuspendedRunStore, logger: unknown = silent, throws = true) { + const engine = new AutomationEngine(logger as never, store); + const state = { throws }; + registerPauser(engine); + registerDownstream(engine, state); + engine.registerFlow('strand_flow', flowDef('strand_flow') as never); + return { engine, state }; +} + +describe('#13909 — restoreConsumedSuspension: the operator exit', () => { + it('moves a terminally-failed run back to resumable, and it then actually resumes', async () => { + const { engine, state } = newEngine(new InMemorySuspendedRunStore()); + const runId = await strandRun(engine); + + const restored = await engine.restoreConsumedSuspension(runId, { + requestedBy: 'ops@example.com', + reason: 'downstream service was down', + }); + + expect(restored.restored).toBe(true); + expect(restored.refusal).toBeUndefined(); + expect(restored.nodeId).toBe('pause'); + expect(restored.flowName).toBe('strand_flow'); + expect(restored.consumedAt).toEqual(expect.any(String)); + + // Back to resumable — measured, not inferred from the return value. + expect(await engine.hasSuspendedRun(runId)).toBe(true); + + // ⭐ And it RESUMES. Half an exit — a run that reports restorable and + // then cannot be resumed — would be worse than none. + state.throws = false; + const finished = await engine.resume(runId); + expect(finished.success).toBe(true); + expect(finished.error).toBeUndefined(); + expect(await engine.hasSuspendedRun(runId)).toBe(false); + expect((await engine.getRun(runId))?.status).toBe('completed'); + }); + + it('restores the pause VERBATIM — the pause\'s variables and step log, not the failed attempt\'s', async () => { + const store = new InMemorySuspendedRunStore(); + const { engine } = newEngine(store); + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + + const atPause = await store.load(runId); + expect(atPause).not.toBeNull(); + const stepsAtPause = atPause!.steps.length; + + // Resume WITH a signal, so the failed attempt's variable map differs + // from the pause's own — if the snapshot were taken after the fold, the + // key below would be present. + await engine.resume(runId, { output: { verdict: 'approved' } } as never); + expect(await engine.hasSuspendedRun(runId)).toBe(false); + + const restored = await engine.restoreConsumedSuspension(runId); + expect(restored.restored).toBe(true); + + const back = await store.load(runId); + expect(back).not.toBeNull(); + expect(back!.nodeId).toBe('pause'); + expect(back!.correlation).toBe('approval:req_1'); + // The pause's own state, unchanged… + expect(back!.variables.ticket).toBe('TKT-9'); + expect(back!.variables['pause.stage']).toBe('awaiting'); + // …and the resume signal is deliberately NOT folded back in: replaying + // it would re-decide on the operator's behalf, and the branch a signal + // routes down is not part of a suspension at all. + expect(back!.variables['pause.verdict']).toBeUndefined(); + // The failed attempt's steps are not part of what goes back. + expect(back!.steps.length).toBe(stepsAtPause); + }); + + it('records the run as paused again, so the repair is not invisible', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const runId = await strandRun(engine); + expect((await engine.getRun(runId))?.status).toBe('failed'); + + await engine.restoreConsumedSuspension(runId); + + const after = await engine.getRun(runId); + expect(after?.status).toBe('paused'); + // #7639's shape: a paused entry carries the variable snapshot. + expect(after?.variables?.ticket).toBe('TKT-9'); + }); +}); + +describe('#13909 — the refusals, each earned by its own observation', () => { + it('refuses a run that is still SUSPENDED — it is already resumable', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + + const res = await engine.restoreConsumedSuspension(runId); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('RUN_SUSPENDED'); + expect(res.reason).toContain('already resumable'); + // Refusing is not enough — it must not have minted a second pause. + expect((await (new InMemorySuspendedRunStore()).list()).length).toBe(0); + expect(await engine.hasSuspendedRun(runId)).toBe(true); + }); + + it('⭐ refuses a run whose resume is IN FLIGHT — the case that races a live resume', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + + // Called from INSIDE the downstream node: the suspension is already + // consumed, no terminal row is written yet, and `resuming` is set. This + // is the only instant at which the race is real. + let seen: Awaited> | undefined; + registerDownstream(engine, { + throws: true, + onEnter: async () => { seen = await engine.restoreConsumedSuspension(runId); }, + }); + + await engine.resume(runId); + + expect(seen).toBeDefined(); + expect(seen!.restored).toBe(false); + expect(seen!.refusal).toBe('RESUME_IN_PROGRESS'); + expect(seen!.reason).toContain('not decided yet'); + // The run took its normal course; nothing was re-armed underneath it. + expect(await engine.hasSuspendedRun(runId)).toBe(false); + }); + + it('refuses a COMPLETED run', async () => { + const { engine, state } = newEngine(new InMemorySuspendedRunStore()); + state.throws = false; + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + const done = await engine.resume(runId); + expect(done.success).toBe(true); + + const res = await engine.restoreConsumedSuspension(runId); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('RUN_COMPLETED'); + expect(res.reason).toContain('completed'); + }); + + it('refuses a CANCELLED run — a restore would undo a deliberate decision', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + expect(await engine.cancelRun(runId, 'submitter withdrew')).toBe(true); + + const res = await engine.restoreConsumedSuspension(runId); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('RUN_CANCELLED'); + expect(res.reason).toContain('cancelled'); + }); + + it('refuses a run that NEVER suspended — distinct from every other failure', async () => { + const engine = new AutomationEngine(silent, new InMemorySuspendedRunStore()); + engine.registerNodeExecutor({ + type: 'always_throws', + descriptor: plain('always_throws'), + async execute() { throw new Error('never paused, just broke'); }, + } as never); + engine.registerFlow('no_pause_flow', noPauseFlow('no_pause_flow') as never); + + const res0 = await engine.execute('no_pause_flow', ctx); + expect(res0.success).toBe(false); + const runId = res0.runId ?? (await engine.listRuns('no_pause_flow'))[0]?.id; + expect(runId).toBeTruthy(); + + const res = await engine.restoreConsumedSuspension(runId as string); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('NO_CONSUMED_SUSPENSION'); + expect(res.reason).toContain('never suspended'); + // ⛔ And it does NOT claim to know which of the two causes it was. + expect(res.reason).toContain('no longer held'); + }); + + it('refuses an UNKNOWN run', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const res = await engine.restoreConsumedSuspension('run_does_not_exist'); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('RUN_NOT_FOUND'); + }); + + it('refuses — rather than guesses — when the suspended-run store cannot be read', async () => { + const inner = new InMemorySuspendedRunStore(); + const store: SuspendedRunStore = { + save: (r) => inner.save(r), + load: async () => { throw new Error('connection reset'); }, + delete: (id) => inner.delete(id), + list: () => inner.list(), + recordTerminal: (r) => inner.recordTerminal(r), + loadTerminal: (id) => inner.loadTerminal(id), + }; + const engine = new AutomationEngine(silent, store); + registerPauser(engine); + registerDownstream(engine, { throws: true }); + engine.registerFlow('strand_flow', flowDef('strand_flow') as never); + + const res = await engine.restoreConsumedSuspension('run_whatever'); + expect(res.restored).toBe(false); + // ⛔ NOT 'RUN_NOT_FOUND': reading an outage as "no suspension" is the one + // mistake that would put a second resumable pause on a live run. + expect(res.refusal).toBe('STORE_UNAVAILABLE'); + }); + + it('refuses when the run-history read fails — "unknown" is not "nothing to restore"', async () => { + const inner = new InMemorySuspendedRunStore(); + const store: SuspendedRunStore = { + save: (r) => inner.save(r), + load: (id) => inner.load(id), + delete: (id) => inner.delete(id), + list: () => inner.list(), + recordTerminal: (r) => inner.recordTerminal(r), + loadTerminal: async () => { throw new Error('history table unreachable'); }, + }; + const engine = new AutomationEngine(silent, store); + registerPauser(engine); + registerDownstream(engine, { throws: true }); + engine.registerFlow('strand_flow', flowDef('strand_flow') as never); + + // A run this engine never saw, so the hot journal cannot answer for it. + const res = await engine.restoreConsumedSuspension('run_elsewhere'); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('STORE_UNAVAILABLE'); + }); +}); + +describe('#13909 — idempotence: one pause, whoever asks and however often', () => { + it('a second SEQUENTIAL restore finds the run already suspended', async () => { + const store = new InMemorySuspendedRunStore(); + const { engine } = newEngine(store); + const runId = await strandRun(engine); + + const first = await engine.restoreConsumedSuspension(runId); + const second = await engine.restoreConsumedSuspension(runId); + + expect(first.restored).toBe(true); + expect(second.restored).toBe(false); + expect(second.refusal).toBe('RUN_SUSPENDED'); + // The invariant the card asks for, measured on the store itself. + expect((await store.list()).filter(r => r.runId === runId).length).toBe(1); + }); + + it('two CONCURRENT restores produce one restore, one refusal, and one pause', async () => { + const store = new InMemorySuspendedRunStore(); + const { engine } = newEngine(store); + const runId = await strandRun(engine); + + const [a, b] = await Promise.all([ + engine.restoreConsumedSuspension(runId, { requestedBy: 'alice' }), + engine.restoreConsumedSuspension(runId, { requestedBy: 'bob' }), + ]); + + const restored = [a, b].filter(r => r.restored); + const refused = [a, b].filter(r => !r.restored); + expect(restored.length).toBe(1); + expect(refused.length).toBe(1); + expect(refused[0].refusal).toBe('RESTORE_IN_PROGRESS'); + expect((await store.list()).filter(r => r.runId === runId).length).toBe(1); + }); + + it('does not itself traverse — the continuation stays an ordinary resume', async () => { + const store = new InMemorySuspendedRunStore(); + const engine = new AutomationEngine(silent, store); + registerPauser(engine); + let entries = 0; + registerDownstream(engine, { throws: true, onEnter: async () => { entries++; } }); + engine.registerFlow('strand_flow', flowDef('strand_flow') as never); + + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + await engine.resume(runId); + expect(entries).toBe(1); + + await engine.restoreConsumedSuspension(runId); + await engine.restoreConsumedSuspension(runId); + // Two invocations, zero extra traversals: the verb re-arms and stops. + expect(entries).toBe(1); + }); +}); + +describe('#13909 — the trace', () => { + it('records the restore where an operator can find it afterwards', async () => { + const warn = vi.fn(); + const logger = { info() {}, warn, error() {}, debug() {} }; + const { engine } = newEngine(new InMemorySuspendedRunStore(), logger); + const runId = await strandRun(engine); + + await engine.restoreConsumedSuspension(runId, { + requestedBy: 'ops@example.com', + reason: 'restarted the billing service', + }); + + const record = warn.mock.calls.find(([msg]) => String(msg).includes('RESTORED')); + expect(record).toBeDefined(); + const [message, meta] = record as [string, Record]; + + // The handles the engine controls stay in the message… + expect(message).toContain(runId); + expect(message).toContain('strand_flow'); + expect(message).toContain("node 'pause'"); + // …and the two consequences an operator must know are stated, not implied. + expect(message).toContain('NOT replayed'); + expect(message).toContain('undone'); + + // Who / why / the original failure are in the STRUCTURED slot. All three + // are uncontrolled text (#6299 family): a newline in any of them would + // split one record into several physical lines of which only the first + // is greppable. + expect(meta.requestedBy).toBe('ops@example.com'); + expect(meta.restoreReason).toBe('restarted the billing service'); + expect(meta.failure).toContain('downstream node blew up'); + expect(meta.consumedAt).toEqual(expect.any(String)); + expect(message).not.toContain('ops@example.com'); + expect(message).not.toContain('restarted the billing service'); + }); + + it('says so when nobody recorded who asked', async () => { + const warn = vi.fn(); + const logger = { info() {}, warn, error() {}, debug() {} }; + const { engine } = newEngine(new InMemorySuspendedRunStore(), logger); + const runId = await strandRun(engine); + + await engine.restoreConsumedSuspension(runId); + + const record = warn.mock.calls.find(([msg]) => String(msg).includes('RESTORED')); + expect((record as [string, Record])[1].requestedBy).toBe('not recorded'); + }); +}); + +describe('#13909 — across a restart: the deployment shape this exists for', () => { + it('a SECOND engine restores a run the FIRST one stranded', async () => { + // One store, two engines — the file\'s established way of simulating a + // process restart (suspend on A, act on B). These runs are found hours + // later, by a sweep or a support ticket, in some other process; an exit + // that only worked inside the lifetime that stranded the run would + // answer almost never. + const store = new InMemorySuspendedRunStore(); + const { engine: engineA } = newEngine(store); + const runId = await strandRun(engineA); + + const { engine: engineB, state: stateB } = newEngine(store); + // Engine B has no in-memory journal for this run at all… + const restored = await engineB.restoreConsumedSuspension(runId, { requestedBy: 'ops' }); + expect(restored.restored).toBe(true); + expect(restored.nodeId).toBe('pause'); + + // …and what it put back is a real, resumable suspension. + expect(await engineB.hasSuspendedRun(runId)).toBe(true); + const back = await store.load(runId); + expect(back!.variables.ticket).toBe('TKT-9'); + expect(back!.correlation).toBe('approval:req_1'); + expect(back!.nodeType).toBe('pause_here'); + + stateB.throws = false; + const finished = await engineB.resume(runId); + expect(finished.success).toBe(true); + }); + + it('a run that is restored and then finishes CLEARS its snapshot', async () => { + const store = new InMemorySuspendedRunStore(); + const { engine, state } = newEngine(store); + const runId = await strandRun(engine); + + // The failed terminal row carries the snapshot… + expect((await store.loadTerminal(runId))?.consumedSuspension).toBeDefined(); + + await engine.restoreConsumedSuspension(runId); + state.throws = false; + expect((await engine.resume(runId)).success).toBe(true); + + // …and the completing run's terminal record replaces it, so nothing is + // left that a later operator could restore a second time. + const terminal = await store.loadTerminal(runId); + expect(terminal?.status).toBe('completed'); + expect(terminal?.consumedSuspension).toBeUndefined(); + + const fresh = new AutomationEngine(silent, store); + const res = await fresh.restoreConsumedSuspension(runId); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('RUN_COMPLETED'); + }); + + it('a store-less engine still exits IN PROCESS, and says so honestly once evicted', async () => { + // No store at all: the in-memory journal is the only copy. This is the + // historical default and the shape unit tests run in. + const { engine, state } = newEngine(undefined); + const runId = await strandRun(engine); + + const restored = await engine.restoreConsumedSuspension(runId); + expect(restored.restored).toBe(true); + state.throws = false; + expect((await engine.resume(runId)).success).toBe(true); + }); +}); + +describe('#13909 — what this slice deliberately does NOT do', () => { + it('leaves AutomationResult.status and the run\'s recorded status alone', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + const failed = await engine.resume(runId); + + // ⛔ No new platform status is minted for the condition — naming it is + // an explicit same-batch sub-item of #13937, because what it should be + // called depends on which resume-ordering shape is ruled. + expect(failed.status).toBeUndefined(); + expect((await engine.getRun(runId))?.status).toBe('failed'); + }); + + it('leaves the resume ordering exactly as it is', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const started = await engine.execute('strand_flow', ctx); + const runId = started.runId as string; + + // The consumption still precedes the traversal: measured from inside + // the downstream node, the suspension is already gone. If a future + // change made the pause survive a downstream throw (#13937 shape 2), + // THIS is the assertion that should be reconsidered — deliberately, not + // by accident. + let suspendedDuringTraversal: boolean | undefined; + registerDownstream(engine, { + throws: true, + onEnter: async () => { suspendedDuringTraversal = await engine.hasSuspendedRun(runId); }, + }); + await engine.resume(runId); + expect(suspendedDuringTraversal).toBe(false); + }); + + it('does not restore a run automatically — only an explicit call does', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + const runId = await strandRun(engine); + + // Nothing sweeps, nothing retries: the run stays exactly where the + // failed resume left it until somebody asks. + await new Promise(r => setTimeout(r, 10)); + expect(await engine.hasSuspendedRun(runId)).toBe(false); + expect((await engine.getRun(runId))?.status).toBe('failed'); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 1a3bf67f43..5722c217af 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -617,6 +617,21 @@ export const DEFAULT_MAX_EXECUTION_LOG_SIZE = 1000; */ export const MAX_PERSISTED_HISTORY_STEPS = 200; +/** + * Cap on the in-memory {@link AutomationEngine.consumedSuspensions} journal — + * the hot half of the operator restore path (#13909). + * + * Bounded because each entry holds a whole suspension (variables, steps, + * context), and SMALL because it is a cache, not the record: with a + * {@link SuspendedRunStore} configured, the authoritative copy rides the run's + * own terminal history row and outlives both this map and the process. A + * store-less engine keeps only the newest N, and + * {@link AutomationEngine.restoreConsumedSuspension} answers + * `NO_CONSUMED_SUSPENSION` for an evicted one rather than implying the run was + * never suspended. + */ +export const MAX_CONSUMED_SUSPENSIONS = 50; + /** * Level the one-line-per-terminal-run summary is logged at (#4354), or `'off'`. * @@ -1150,6 +1165,125 @@ export interface RunRecord { * absent summary must never be read as "this run did nothing". */ summary?: FlowRunSummary; + /** + * [#13909] The suspension this run's resume CONSUMED before the downstream + * node threw — written only on that one path, so its presence is itself the + * statement "this `failed` run had a pause and no longer has one". + * + * It rides the terminal row because that is the only durable artefact the + * failure leaves: `forgetSuspendedRun` deleted the paused row before + * `traverseNext` ran, and this row's `variables_json` / `context_json` / + * `screen_json` columns already exist and were simply never written on + * terminal rows (`sys_automation_run` says so at the field itself). No new + * column, no new status value, no new table. + * + * Cleared by the next terminal record for the same run — a restored run + * that resumes to completion upserts this row without a snapshot — so + * "restorable" cannot outlive the condition it describes. + * + * ⛔ Not a run state. See {@link ConsumedSuspension}. + */ + consumedSuspension?: SuspendedRun; +} + +/** + * A suspension that a resume CONSUMED and then failed downstream of — the input + * {@link AutomationEngine.restoreConsumedSuspension} puts back (#13909). + * + * `resumeInternal` consumes the suspension **before** running downstream nodes, + * so a node that merely throws throws with the pause already gone and the catch + * arm records the run `failed`. There is then no suspension left to resume — + * and, before this journal existed, nothing anywhere to rebuild one from: the + * durable paused row is deleted at consumption and the terminal history row + * carries no `variables` / `context` / `screen` at all. + * + * ⛔ **This is not a run state and not a status.** The run is `failed`, + * {@link AutomationResult.status} is unchanged, and nothing here names the + * condition — naming it is an explicit same-batch sub-item of #13937, because + * what it should be called depends on which resume-ordering shape is ruled. + * This is a SNAPSHOT of a deleted row, kept so a deliberate operator action can + * restore it. + */ +export interface ConsumedSuspension { + /** + * The suspension exactly as it stood when the resume consumed it — + * VERBATIM: the resume signal is not folded in and the failed attempt's + * steps are not included. See + * {@link AutomationEngine.restoreConsumedSuspension} for why that is the + * only shape that is safe to put back. + */ + run: SuspendedRun; + /** When the resume that consumed it recorded its downstream failure. */ + consumedAt: string; + /** The downstream failure that left the run terminal and unresumable. */ + error: string; +} + +/** + * Why {@link AutomationEngine.restoreConsumedSuspension} declined (#13909). + * + * Every value is EARNED BY A SPECIFIC OBSERVATION, not by a catch-all "bad + * input" arm: an operator whose repair is refused has to be able to tell "this + * run is fine" from "this run is beyond this verb" from "I could not read the + * store", because the remedy differs for each. + * + * - `'RESTORE_IN_PROGRESS'` — another restore of this run is already running + * in this process. The in-process half of the idempotence guarantee. + * - `'RESUME_IN_PROGRESS'` — a resume is running for this run RIGHT NOW. + * ⚠️ The subtle one: "suspension gone, no terminal row yet" is exactly what + * a resume in flight looks like, so re-arming one of those races the live + * resume — the traversal would finish and record `completed` while a paused + * row it knows nothing about survives. + * - `'RUN_SUSPENDED'` — a live suspension already exists. The run is already + * resumable, so there is nothing to restore; this is also what a second + * restore of an already-restored run answers, in this process or after a + * restart. + * - `'STORE_UNAVAILABLE'` — the durable store could not be read, so whether a + * suspension is live is UNKNOWN. Refused rather than guessed: reading an + * outage as "no suspension" is the one mistake that mints a second pause. + * - `'RUN_COMPLETED'` — the run finished. Nothing to exit from. + * - `'RUN_CANCELLED'` — the run was cancelled (ADR-0044). A restore would + * undo a decision somebody made on purpose. + * - `'NO_CONSUMED_SUSPENSION'` — the run exists and is terminal, but no + * consumed suspension is available for it: it never paused, or its snapshot + * is gone (no store configured and the in-memory journal evicted it, or a + * later terminal record cleared it). Deliberately does NOT claim which — + * nothing in the engine can tell those apart, and the result's `reason` + * names the status actually observed instead of overclaiming. + * - `'RUN_NOT_FOUND'` — no record of this run at all. + */ +export type SuspensionRestoreRefusal = + | 'RESTORE_IN_PROGRESS' + | 'RESUME_IN_PROGRESS' + | 'RUN_SUSPENDED' + | 'STORE_UNAVAILABLE' + | 'RUN_COMPLETED' + | 'RUN_CANCELLED' + | 'NO_CONSUMED_SUSPENSION' + | 'RUN_NOT_FOUND'; + +/** + * Outcome of {@link AutomationEngine.restoreConsumedSuspension} (#13909). + * + * Deliberately NOT an {@link AutomationResult}: this verb does not execute a + * flow, and its refusal vocabulary is its own. Folding it into the platform + * result type would put eight new codes into a contract every transport reads + * — and would mint platform vocabulary for a condition #13937 has not yet + * ruled the shape of. + */ +export interface SuspensionRestoreResult { + /** `true` only when a suspension was actually put back by THIS call. */ + restored: boolean; + runId: string; + /** Absent exactly when `restored` is `true`. */ + refusal?: SuspensionRestoreRefusal; + /** One sentence naming what was observed — always present, both ways. */ + reason: string; + /** The restored run's flow / node, when one was restored. */ + flowName?: string; + nodeId?: string; + /** When the resume that consumed the restored suspension failed. */ + consumedAt?: string; } export interface SuspendedRunStore { @@ -1500,6 +1634,25 @@ export class AutomationEngine implements IAutomationService { * duplicate `resume(runId)` can't re-enter and double-run side effects. */ private resuming = new Set(); + /** + * [#13909] Suspensions this process consumed on a resume that then failed + * downstream — the hot half of {@link restoreConsumedSuspension}, mirrored + * to the run's terminal history row when a {@link store} is configured + * (exactly the {@link suspendedRuns} + {@link store} pairing one field up). + * + * Bounded by {@link MAX_CONSUMED_SUSPENSIONS}, oldest first: each entry + * holds a whole suspension, and the durable copy is the record. + */ + private consumedSuspensions = new Map(); + /** + * [#13909] Run ids currently mid-RESTORE — the same synchronous in-process + * guard shape as {@link resuming}, so two operators racing the verb produce + * one restored pause and one refusal instead of two `restored: true` + * answers. Across processes the guarantee is carried by the paused row + * itself: a suspension is keyed by run id, so a second restore finds one + * live and refuses `RUN_SUSPENDED`. + */ + private restoring = new Set(); /** * Optional persisted dispatch-claim ledger (#10220). When set, `claim()` * checks-and-records against `sys_flow_dispatch` so trigger dispatch dedup @@ -4632,7 +4785,19 @@ export class AutomationEngine implements IAutomationService { // signal above is pure in-memory work, not downstream work.) // This is also where the paused node learns its pause is over and // disarms what it armed on entry (#5512) — see forgetSuspendedRun. - await this.forgetSuspendedRun(run, 'resumed'); + // [#13909] How long the step log was AT THE PAUSE, read one line + // after the consumption and before anything downstream runs. + // `steps` below is the SAME array `traverseNext` appends to, so by + // the time the catch arm builds a restore snapshot the pause's own + // step log is no longer recoverable from it — this integer is what + // trims it back. An `int` on every resume, and nothing else: the + // snapshot itself is built only on the failure path. + // + // ⛔ Deliberately NOT a change to the ordering. The consumption + // still precedes the traversal, `forgetSuspendedRun` is untouched + // and `hasSuspendedRun` still answers false for the whole traversal + // window. Which ordering is right is #13937's, and unruled. + const stepCountAtPause = run.steps.length; const steps = run.steps; const context = run.context; @@ -4727,6 +4892,20 @@ export class AutomationEngine implements IAutomationService { const errorMessage = err instanceof Error ? err.message : String(err); const durationMs = Date.now() - run.startTime; + // [#13909] THE seam this card is about. The pause was consumed + // above, this node merely threw, and the record below makes the + // run terminal — so from here on nothing in the engine can move + // it: `resume` answers RUN_NOT_FOUND, `cancelRun` is a no-op on + // it, and none of the other public methods takes it anywhere. + // Journal the suspension that was consumed so a DELIBERATE + // operator action can put it back + // ({@link restoreConsumedSuspension}). + // + // ⛔ Not a repair, and nothing here re-arms anything: the run + // stays `failed`, no suspension exists after this line, and no + // caller's result changes. It is the evidence a repair needs, + // written at the only moment it still exists. + const consumed = this.journalConsumedSuspension(run, stepCountAtPause, errorMessage); const logged = this.recordLog({ id: runId, flowName: run.flowName, @@ -4738,7 +4917,7 @@ export class AutomationEngine implements IAutomationService { trigger: buildRunTrigger(context), steps, error: errorMessage, - }, context); + }, context, consumed.run); // Subflow chain: a child failing terminally fails every // ancestor awaiting it — they can never be resumed otherwise. // The delegation path handles its own level (skipBubble). @@ -5046,6 +5225,348 @@ export class AutomationEngine implements IAutomationService { return true; } + /** + * Record the suspension a resume consumed before its downstream node threw + * (#13909) — the one and only producer of a {@link ConsumedSuspension}. + * + * VERBATIM, and that word is load-bearing: + * + * - `run.variables` is the pause's OWN snapshot. The resume's signal was + * folded into a separate `Map` built from it, never into this object, so + * what is journalled is the state at the pause, not the state the failed + * attempt was working from. It is the same object the durable paused row + * was written from at suspend time. + * - `run.steps` is the live array `traverseNext` appended to, so it is + * trimmed back to `stepCountAtPause` — the failed attempt's steps are + * NOT part of the thing an operator puts back. + * - Everything else (`nodeId`, `nodeType`, `context`, `correlation`, + * `screen`, `startedAt`, `startTime`) is carried across untouched. + * + * Shallow by design, not lazily: a JSON clone here could throw on a + * circular value INSIDE a catch arm that is already handling a failure, and + * the fields it would deep-copy are exactly the ones the durable store + * already round-tripped through JSON at suspend time. + */ + private journalConsumedSuspension( + run: SuspendedRun, + stepCountAtPause: number, + error: string, + ): ConsumedSuspension { + const consumed: ConsumedSuspension = { + run: { ...run, steps: run.steps.slice(0, stepCountAtPause) }, + consumedAt: new Date().toISOString(), + error, + }; + this.consumedSuspensions.set(run.runId, consumed); + // Oldest first — `Map` iterates in insertion order, and re-`set`ting an + // existing key keeps its original position, which is what we want: a + // run that strands twice does not jump the queue ahead of one that has + // been waiting for an operator longer. + while (this.consumedSuspensions.size > MAX_CONSUMED_SUSPENSIONS) { + const oldest = this.consumedSuspensions.keys().next().value; + if (oldest === undefined) break; + this.consumedSuspensions.delete(oldest); + } + return consumed; + } + + /** Build a refusal from {@link restoreConsumedSuspension}. */ + private refuseRestore( + runId: string, + refusal: SuspensionRestoreRefusal, + reason: string, + ): SuspensionRestoreResult { + return { restored: false, runId, refusal, reason }; + } + + /** + * **The operator exit from a run a resume left terminally unresumable** + * (#13909, deliverable 2). Puts back the suspension that resume consumed, + * so the run is resumable again. + * + * ## The state this is an exit from + * + * `resumeInternal` consumes the suspension **before** running downstream + * nodes, so a node that merely throws throws with the pause already gone + * and the catch arm records the run `failed`. That state is terminal: + * {@link resume} answers `RUN_NOT_FOUND` (there is no suspension left), + * {@link cancelRun} is a no-op on it, and nothing else moves it either. A + * deployment could enter that state and never leave it. This verb is the + * leaving. + * + * ⚠️ It is a REPAIR, not a prevention: whether the pause should survive a + * downstream throw at all is #13937, unruled and in the maintainer's hands. + * This method changes no resume semantics for any pausing node type — the + * ordering, {@link forgetSuspendedRun} and `traverseNext` are all exactly + * as they were — and it stays useful whichever way that card is ruled, + * because the runs already stuck today are not released by changing what + * FUTURE resumes do. + * + * ## Deliberate — never automatic + * + * Nothing calls this on its own. There is no retry, no sweeper, no + * self-healing arm anywhere in the engine: an operator (or an admin door + * standing in for one) asks for this run, by id, on purpose. A machine that + * re-armed strandings by itself would re-run the node that threw, forever, + * with nobody deciding it should. + * + * ## Verbatim — the pause goes back as it was, and nothing else + * + * The restored suspension is the one that was consumed, exactly: the + * pause's own variables, its step log as of the pause, its node, its + * correlation, its screen. Two consequences an operator must know, and both + * are in the trace this writes: + * + * - ⚠️ **The resume signal is NOT replayed.** The approval decision or + * screen submission that accompanied the failed resume is not folded + * back in; the continuation must be re-issued. Replaying it would mean + * re-deciding on the operator's behalf, and the branch a signal routes + * down (`signal.branchLabel`) is not part of a suspension at all — a + * resume with the signal dropped would silently take a different edge. + * - ⚠️ **The failed attempt is NOT undone.** Whatever the downstream nodes + * did before one of them threw stands; this re-arms a pause, it does not + * roll a transaction back. That is why re-deciding is the right default + * and why this is an operator's call rather than the engine's. + * + * Restoring the pause exactly as it stood is also the shape that does not + * pre-empt #13937: it is precisely the state a resume-ordering change would + * have left behind, so this composes with that ruling instead of racing it. + * + * ## Idempotence — carried by the paused row, not by a flag + * + * A suspension is keyed by run id, so a restore cannot produce two + * resumable pauses however many operators ask: the second call finds one + * live and answers `RUN_SUSPENDED`. That holds across processes and across + * a restart, because the paused row is durable. {@link restoring} adds the + * in-process half — two callers racing in one process get one + * `restored: true` and one `RESTORE_IN_PROGRESS`, rather than two calls + * both claiming the restore. + * + * And it cannot produce two traversals, by construction: **this verb does + * not resume.** It re-arms the pause and stops. The continuation is an + * ordinary {@link resume} afterwards, through the same authority gate, the + * same screen validation and the same `resuming` guard as any other. + * + * ## The trace + * + * A restore is recorded at `warn` with the run, flow, node, when the + * suspension was consumed, who asked and why. #4632's vocabulary grades + * this FUNCTIONAL — nothing claimed-persisted failed to land — but the + * level is `warn` rather than `info` on purpose: this moves a run the + * platform had already recorded as terminally failed, and an exit that + * leaves no mark is the same silence that let this whole class go + * unnoticed. Operator-supplied text (`requestedBy` / `reason`) and the + * original thrown message ride the STRUCTURED slot, never the message — + * none of the three is controlled by us and a newline in any of them would + * split one record into several physical lines of which only the first is + * greppable (the #6299 family). + * + * @param options.requestedBy - Who asked. Logged; `not recorded` when + * absent, which is itself something an operator can find. + * @param options.reason - Why. Logged the same way. + * @returns Never throws: every outcome — including an unreadable store — is + * a {@link SuspensionRestoreResult} naming what was observed. + */ + async restoreConsumedSuspension( + runId: string, + options?: { requestedBy?: string; reason?: string }, + ): Promise { + // Synchronously, before the first await — the same shape as `resuming` + // in `resumeInternal`: a guard set after an await is not a guard. + if (this.restoring.has(runId)) { + return this.refuseRestore( + runId, + 'RESTORE_IN_PROGRESS', + `Another restore of run '${runId}' is already in progress in this process`, + ); + } + // ⚠️ The subtle one. `resuming` is set before `resumeInternal`'s first + // await and cleared in its `finally`, so this window is exactly + // "suspension consumed, outcome not yet decided" — which is what a + // resume IN FLIGHT looks like, and is indistinguishable from the + // stranded state by any other reading. Re-arming one of these races the + // live resume: its traversal would finish and record `completed` while + // a paused row it knows nothing about survives, and the next restart + // would resume an already-finished run. + if (this.resuming.has(runId)) { + return this.refuseRestore( + runId, + 'RESUME_IN_PROGRESS', + `Run '${runId}' is being resumed right now — its outcome is not decided yet`, + ); + } + this.restoring.add(runId); + try { + // Already resumable? STRICT read: an unreadable store must not read + // as "no suspension" here. That degradation is harmless for a + // display path and is the one mistake that mints a second pause on + // this one. + let live: SuspendedRun | null; + try { + live = await this.loadSuspendedRunStrict(runId); + } catch (err) { + this.logger.warn( + `[automation] restoreConsumedSuspension('${runId}') could not read the durable suspended-run ` + + `store, so whether this run is still parked is UNKNOWN — the restore was REFUSED rather ` + + `than guessed, because reading an outage as "no suspension" is what would put a second ` + + `resumable pause on a live run. Nothing was written. Fix the store failure in this ` + + `record's meta, then re-issue the restore.`, + describeThrownForLog(err), + ); + return this.refuseRestore( + runId, + 'STORE_UNAVAILABLE', + `Durable suspended-run store unreachable for run '${runId}' — whether it is still suspended is unknown`, + ); + } + if (live) { + return this.refuseRestore( + runId, + 'RUN_SUSPENDED', + `Run '${runId}' is suspended at node '${live.nodeId}' and already resumable — nothing to restore`, + ); + } + + // The journal: this process's hot copy first, then the durable + // copy on the run's own terminal history row. One reader, two + // sources — the same pairing `resume` itself uses for suspensions. + let consumed = this.consumedSuspensions.get(runId); + if (!consumed && this.store?.loadTerminal) { + let terminal: RunRecord | null; + try { + terminal = await this.store.loadTerminal(runId); + } catch (err) { + this.logger.warn( + `[automation] restoreConsumedSuspension('${runId}') could not read the durable run-history ` + + `row, so whether a consumed suspension is recoverable for this run is UNKNOWN — the ` + + `restore was REFUSED rather than reported as "nothing to restore", which is the answer ` + + `an operator would act on by giving up. Nothing was written. Fix the store failure in ` + + `this record's meta, then re-issue the restore.`, + describeThrownForLog(err), + ); + return this.refuseRestore( + runId, + 'STORE_UNAVAILABLE', + `Durable run-history unreachable for run '${runId}' — whether a consumed suspension survives is unknown`, + ); + } + if (terminal?.consumedSuspension) { + consumed = { + run: terminal.consumedSuspension, + consumedAt: terminal.finishedAt ?? terminal.startedAt, + error: terminal.error ?? '', + }; + } + } + + if (!consumed) { + // Nothing to restore — say WHICH nothing. The remedy differs for + // every one of these and a single "bad run" refusal would send an + // operator looking for the wrong thing. + const logged = await this.getRun(runId); + if (!logged) { + return this.refuseRestore(runId, 'RUN_NOT_FOUND', `No run '${runId}' is known`); + } + if (logged.status === 'completed') { + return this.refuseRestore( + runId, + 'RUN_COMPLETED', + `Run '${runId}' completed — there is no unresumable state to exit`, + ); + } + if (logged.status === 'cancelled') { + return this.refuseRestore( + runId, + 'RUN_CANCELLED', + `Run '${runId}' was cancelled — restoring it would undo a deliberate decision`, + ); + } + // ⛔ Deliberately does not claim WHICH: "it never paused" and + // "its snapshot is gone" are indistinguishable from here, and + // guessing would hand an operator a false certainty. The status + // actually observed is named instead. + return this.refuseRestore( + runId, + 'NO_CONSUMED_SUSPENSION', + `Run '${runId}' is recorded '${logged.status}' and no consumed suspension is available for it ` + + `— it never suspended, or its snapshot is no longer held`, + ); + } + + // Claim, then re-arm. No await between the claim and the write. + this.consumedSuspensions.delete(runId); + await this.persistSuspendedRun(consumed.run); + + // The run IS paused again, so the run log has to say so — otherwise + // the repair is invisible on every surface that reads it and the + // Runs view keeps calling a resumable run `failed`. This is the + // SAME record the resume path writes when a run re-suspends at a + // downstream node (`status: 'paused'`, carrying the variable + // snapshot, #7639), for the same reason, and `getRun`'s + // last-entry-wins order is what makes it the answer. + // + // ⚠️ Deliberately NOT terminal, so it neither writes a history row + // nor clears the snapshot the row carries: until this run reaches a + // real terminal state, "it failed mid-resume and was put back" is + // the truth, and both halves of it should survive a restart. + // + // ⚠️ Measured limitation, stated rather than papered over: after a + // restart the DURABLE surfaces still read this run `failed`, because + // `getRun` / `listRuns` deliberately let a terminal row win over a + // paused one (a paused row can outlive the run it describes). The + // in-process log entry and the `warn` above are the trace this slice + // ships; a durable status that survives the restart is naming work, + // and naming is #13937's same-batch sub-item. + this.recordLog({ + id: runId, + flowName: consumed.run.flowName, + flowVersion: consumed.run.flowVersion, + status: 'paused', + startedAt: consumed.run.startedAt, + durationMs: Date.now() - consumed.run.startTime, + trigger: buildRunTrigger(consumed.run.context), + steps: consumed.run.steps, + variables: consumed.run.variables, + }, consumed.run.context); + + // ⚠️ Uncontrolled text — the operator's own words and the original + // thrown message — goes to the STRUCTURED slot, never the message + // (#6299 family). `warn(message, meta?)`: meta is the SECOND + // argument; `warn` has no `Error` slot. + this.logger.warn( + `[automation] run '${runId}' of flow '${consumed.run.flowName}': an operator RESTORED the ` + + `suspension its resume had consumed at node '${consumed.run.nodeId}', so the run is resumable ` + + `again. ⚠️ Nothing the failed attempt already did was undone, and the original resume signal ` + + `was NOT replayed — the continuation must be re-issued. Who asked, why, and the failure this ` + + `is an exit from are in this record's meta.`, + { + runId, + flowName: consumed.run.flowName, + nodeId: consumed.run.nodeId, + nodeType: consumed.run.nodeType, + correlation: consumed.run.correlation, + consumedAt: consumed.consumedAt, + requestedBy: options?.requestedBy ?? 'not recorded', + restoreReason: options?.reason ?? 'not recorded', + failure: consumed.error, + }, + ); + + return { + restored: true, + runId, + reason: + `Restored the suspension run '${runId}' consumed at node '${consumed.run.nodeId}' — the run is ` + + `resumable again; re-issue the continuation`, + flowName: consumed.run.flowName, + nodeId: consumed.run.nodeId, + consumedAt: consumed.consumedAt, + }; + } finally { + this.restoring.delete(runId); + } + } + /** * Walk a failed run's `$parentRunId` chain and fail each suspended * ancestor (see {@link failSuspendedRun}). Bounded so a corrupt context @@ -5185,7 +5706,20 @@ export class AutomationEngine implements IAutomationService { // tenant (the ruled fallback). Threaded as a parameter rather than read // off the entry because `ExecutionLogEntry` deliberately keeps the // published `trigger` block's shape (`ExecutionLogSchema`). - private recordLog(entry: ExecutionLogEntry, context?: AutomationContext): ExecutionLogEntry { + // [#13909] `consumedSuspension` is the suspension a resume consumed before + // the downstream node threw, passed ONLY by that one call site and copied + // straight onto the {@link RunRecord} below. It is deliberately a parameter + // rather than a field of {@link ExecutionLogEntry}: that interface is + // served verbatim by `GET /automation/:name/runs/:runId`, so carrying a + // whole variable map and run context on it would publish a paused run's + // internals for every failed run — a disclosure change with no card behind + // it, and the exact widening #7639 refused when it added `variables` to the + // `paused` sites only. + private recordLog( + entry: ExecutionLogEntry, + context?: AutomationContext, + consumedSuspension?: SuspendedRun, + ): ExecutionLogEntry { // #4354 — fold the run's outcome BEFORE anything downstream trims the // step log. History compaction keeps 200 steps; the summary must count // all 5000, or a long sweep's `acted` would shrink with its step log and @@ -5267,6 +5801,12 @@ export class AutomationEngine implements IAutomationService { nodeId: lastStep?.nodeId, steps: this.compactStepsForHistory(entry.steps), summary: entry.summary, + // [#13909] Present only on the resume-consumed-then-failed + // path. On every other terminal record it is `undefined`, and + // the store writes explicit NULLs for it — so a run that is + // restored and then finishes CLEARS its own snapshot instead of + // leaving a stale one an operator could restore a second time. + consumedSuspension, }; void this.store.recordTerminal(record).catch((err) => { // #6499 — driver text to the structured slot; see diff --git a/packages/services/service-automation/src/suspended-run-store.ts b/packages/services/service-automation/src/suspended-run-store.ts index 0bf011c33a..ae1ccc9ffe 100644 --- a/packages/services/service-automation/src/suspended-run-store.ts +++ b/packages/services/service-automation/src/suspended-run-store.ts @@ -58,6 +58,19 @@ const OVERFLOW_PRUNE_BATCH = 50; * tail is halved until it fits — the newest steps carry the failure. */ const MAX_STEPS_JSON_BYTES = 64 * 1024; +/** + * [#13909] Byte cap for the consumed-suspension snapshot a terminal row + * carries (`variables_json` + `context_json` + `screen_json` together). + * + * ⛔ Over budget the snapshot is DROPPED, never truncated — and that asymmetry + * with `steps_json` above is the whole point. A halved step tail is still an + * honest, smaller observation; half a variable map is a run that would resume + * from state it was never in. `restoreConsumedSuspension` then answers + * `NO_CONSUMED_SUSPENSION` for the run, which is true, instead of restoring a + * corrupt pause that looks perfectly healthy. + */ +const MAX_CONSUMED_SUSPENSION_JSON_BYTES = 256 * 1024; + /** Byte cap for a terminal row's persisted `summary_json` (#4354). Generous * relative to the shape it holds — one entry per node that ran, one per gate * that closed — so only a pathological flow ever trips it. */ @@ -327,6 +340,20 @@ export class ObjectStoreSuspendedRunStore implements SuspendedRunStore { skipped_count: record.summary?.skipped ?? null, unmeasured_count: record.summary?.unmeasured ?? null, summary_json: record.summary ? serializeSummaryBounded(record.summary) : null, + // [#13909] The suspension this run's resume consumed before the + // downstream node threw, in the columns that already exist for exactly + // this state and were simply never written on terminal rows. + // + // ALWAYS all four keys, `null` when there is no snapshot: this is an + // UPSERT, and a restored run that later finishes must CLEAR what it + // carried. Omitting the keys would leave a stale snapshot behind on the + // updated row, which an operator could restore a second time — the run + // would go back to a pause it has already left. + // + // `node_type` rides along because the resume authority gate (#3801) keys + // on it: a suspension restored without it is one a `resumeAuthority` + // check can only fall back on the live flow for. + ...serializeConsumedSuspension(record.consumedSuspension, this.logger), }; const existing = await this.engine.find(TABLE, { where: { id }, limit: 1, context: SYSTEM_CTX, @@ -428,6 +455,14 @@ export class ObjectStoreSuspendedRunStore implements SuspendedRunStore { // steps are compacted (200 max), so recomputing would report a // 5000-iteration sweep as having acted a couple of hundred times. summary: parseJson(row.summary_json, undefined), + // [#13909] Rebuilt only when the row actually carries the resumable + // state. `variables_json` is the discriminator: it is written on a + // terminal row by nothing but the consumed-suspension path, so its + // presence IS the statement "this failed run had a pause and no longer + // has one". A pre-#13909 row has none and rebuilds to `undefined`, which + // `restoreConsumedSuspension` reports honestly rather than as a run that + // never suspended. + consumedSuspension: deserializeConsumedSuspension(row), }; } @@ -526,6 +561,85 @@ export class ObjectStoreSuspendedRunStore implements SuspendedRunStore { } } +/** + * [#13909] The `sys_automation_run` columns that carry a terminal row's + * consumed-suspension snapshot — or explicit `null`s for all of them. + * + * Refuses rather than truncates when the snapshot is over + * {@link MAX_CONSUMED_SUSPENSION_JSON_BYTES}: a partial variable map would + * restore a run into a state it was never in, and would do it silently. The + * drop is logged with the size so an operator who finds the run unrestorable + * learns WHY instead of concluding it never suspended. + */ +function serializeConsumedSuspension( + run: SuspendedRun | undefined, + logger?: MinimalLogger, +): Record { + const empty = { + variables_json: null, + context_json: null, + screen_json: null, + node_type: null, + correlation: null, + }; + if (!run) return empty; + const variables_json = JSON.stringify(run.variables ?? {}); + const context_json = JSON.stringify(run.context ?? {}); + const screen_json = run.screen ? JSON.stringify(run.screen) : null; + const bytes = variables_json.length + context_json.length + (screen_json?.length ?? 0); + if (bytes > MAX_CONSUMED_SUSPENSION_JSON_BYTES) { + logger?.warn?.( + `[automation] run '${run.runId}': the suspension its resume consumed is ${bytes} bytes, over the ` + + `${MAX_CONSUMED_SUSPENSION_JSON_BYTES}-byte row budget, so it was NOT persisted and this run cannot be ` + + `restored after a restart. It was dropped rather than truncated on purpose — half a variable map would ` + + `restore the run into a state it was never in.`, + ); + return empty; + } + // `correlation` is part of the resumable state, not decoration: a run parked + // at a `subflow:`/`map:` node resumes down a DIFFERENT path on it, and a + // pausing plugin finds its external row through it. A snapshot restored + // without it is a different pause. + return { + variables_json, + context_json, + screen_json, + node_type: run.nodeType ?? null, + correlation: run.correlation ?? null, + }; +} + +/** + * [#13909] Rebuild a terminal row's consumed-suspension snapshot, or + * `undefined` when the row carries none. + * + * Keyed off `variables_json`, which no other terminal-row writer populates — + * see the call site. `correlation` and `node_type` come back from their own + * columns, written by the same helper. `steps` come from the row's own `steps_json`: they are the + * step log AS OF THE PAUSE (the engine trims the failed attempt's steps off the + * snapshot before recording), bounded by the same cap every terminal row's + * steps are. + */ +function deserializeConsumedSuspension(row: any): SuspendedRun | undefined { + if (row.variables_json == null || row.variables_json === '') return undefined; + const startedAt = row.started_at ?? row.created_at ?? ''; + const rawId = String(row.id ?? ''); + return { + runId: rawId.startsWith(HISTORY_PREFIX) ? rawId.slice(HISTORY_PREFIX.length) : rawId, + flowName: String(row.flow_name ?? ''), + flowVersion: typeof row.flow_version === 'number' ? row.flow_version : undefined, + nodeId: String(row.node_id ?? ''), + nodeType: row.node_type ?? undefined, + variables: parseJson>(row.variables_json, {}), + steps: parseJson(row.steps_json, []), + context: parseJson(row.context_json, {}), + startedAt, + startTime: typeof row.start_time === 'number' ? row.start_time : (Date.parse(startedAt) || Date.now()), + correlation: row.correlation ?? undefined, + screen: parseJson(row.screen_json, undefined as any), + }; +} + /** * JSON-encode a terminal run's step log under the {@link MAX_STEPS_JSON_BYTES} * cap. The engine already bounds step COUNT (and strips stacks); this bounds From 15a2dd08777c59faccac79554559e3187955e7ed Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 18:05:35 +0000 Subject: [PATCH 2/5] fix(automation): restore the dropped forgetSuspendedRun call --- .../services/service-automation/src/engine.ts | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 5722c217af..883fc7fb00 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -4779,25 +4779,29 @@ export class AutomationEngine implements IAutomationService { }; } + // [#13909] How long the step log was AT THE PAUSE, read before + // anything downstream runs. `steps` below is the SAME array + // `traverseNext` appends to, so by the time the catch arm builds a + // restore snapshot the pause's own step log is no longer + // recoverable from it — this integer is what trims it back. One + // `int` on every resume and nothing else: the snapshot itself is + // built only on the failure path. + // + // ⛔ Deliberately NOT a change to the ordering. The consumption + // below still precedes the traversal, `forgetSuspendedRun` is + // untouched, and `hasSuspendedRun` still answers false for the + // whole traversal window (pinned in + // `consumed-suspension-restore.test.ts`). Which ordering is right + // is #13937's, and unruled. + const stepCountAtPause = run.steps.length; + // Consume the suspension *before* running downstream work — a run // resumes exactly once per pause, and a duplicate resume after a // partial restart must not double-run side effects. (Folding the // signal above is pure in-memory work, not downstream work.) // This is also where the paused node learns its pause is over and // disarms what it armed on entry (#5512) — see forgetSuspendedRun. - // [#13909] How long the step log was AT THE PAUSE, read one line - // after the consumption and before anything downstream runs. - // `steps` below is the SAME array `traverseNext` appends to, so by - // the time the catch arm builds a restore snapshot the pause's own - // step log is no longer recoverable from it — this integer is what - // trims it back. An `int` on every resume, and nothing else: the - // snapshot itself is built only on the failure path. - // - // ⛔ Deliberately NOT a change to the ordering. The consumption - // still precedes the traversal, `forgetSuspendedRun` is untouched - // and `hasSuspendedRun` still answers false for the whole traversal - // window. Which ordering is right is #13937's, and unruled. - const stepCountAtPause = run.steps.length; + await this.forgetSuspendedRun(run, 'resumed'); const steps = run.steps; const context = run.context; From 4c949ca3950ce0e0ca19f35af194f70b5ee1e45f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 18:31:30 +0000 Subject: [PATCH 3/5] feat(service-automation): an operator can put back a suspension a failed resume consumed (#13909) --- .../automation-consumed-suspension-restore.md | 48 +++++++++++++++++++ .../backfill-platform-row-organizations.ts | 9 ++-- 2 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 .changeset/automation-consumed-suspension-restore.md diff --git a/.changeset/automation-consumed-suspension-restore.md b/.changeset/automation-consumed-suspension-restore.md new file mode 100644 index 0000000000..7620347186 --- /dev/null +++ b/.changeset/automation-consumed-suspension-restore.md @@ -0,0 +1,48 @@ +--- +"@objectstack/service-automation": minor +"@objectstack/plugin-approvals": patch +--- + +feat(service-automation): an operator can put back a suspension a failed resume consumed (#13909) + +A run that was resumed and whose downstream node merely **threw** was +terminally unresumable, and nothing anywhere could move it. The engine consumes +the suspension *before* running downstream nodes, so such a node throws with +the pause already gone and the catch arm records the run `failed`: `resume` +then answers `RUN_NOT_FOUND`, `cancelRun` is a no-op, and none of the engine's +other public methods takes the run anywhere. A deployment could enter that +state and never leave it. + +`AutomationEngine.restoreConsumedSuspension(runId, { requestedBy, reason })` +is the exit. It puts the consumed suspension back — verbatim, as it stood at +the pause — so the run is resumable again through an ordinary `resume`, with +the same authority gate, the same screen validation and the same idempotency +guard as any other. + +- **Deliberate, never automatic.** Nothing calls it on its own: no retry, no + sweeper. An operator asks for one run, by id. +- **Safe to refuse, with the reason named.** A run that is still suspended, one + whose resume is *in flight*, one that completed, one that was cancelled, one + that never suspended, and an unknown id each get their own refusal — as does + an unreadable store, which is refused rather than guessed at. +- **Idempotent.** A suspension is keyed by run id, so however many operators + ask there is one resumable pause and no extra traversal — the verb re-arms + and stops. Two racing callers in one process get one restore and one refusal. +- **It leaves a trace.** The restore is logged with the run, flow, node, when + the suspension was consumed, who asked and why, and the run is recorded + `paused` again so the repair is not invisible. Across a restart the exit + still works: the consumed suspension rides the run's own terminal history row + (in `sys_automation_run` columns that already existed), and a run that is + restored and then finishes clears it. + +⚠️ A repair, not a prevention. The failed attempt's side effects are **not** +undone and the original resume signal is **not** replayed — the continuation +must be re-issued. Whether the pause should survive a downstream throw at all +is a separate, unruled decision (#13937); this change leaves the resume +ordering, `forgetSuspendedRun` and `traverseNext` exactly as they are, mints no +new run status, and works whichever way that is ruled — the runs already stuck +today are not released by changing what future resumes do. + +`plugin-approvals` carries a comment correction only: its organization backfill +documented `context_json` as never written on terminal rows, which this change +makes false for that one class of row. No behaviour change there. diff --git a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts index b4d0c269c7..750a343dcf 100644 --- a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts +++ b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts @@ -168,9 +168,12 @@ export const BACKFILL_TARGETS: readonly BackfillTarget[] = [ subjectObjectField: 'trigger_object', subjectIdField: 'trigger_record_id', // `context_json` is the serialized AutomationContext; the trigger record - // sits at `.record`. Written on paused rows only — `recordTerminal` does - // not persist it, so terminal rows resolve from the live subject or not - // at all. + // sits at `.record`. Written on every paused row, and — since #13909 — + // on the one class of TERMINAL row that carries a restorable suspension + // (a run whose resume consumed its pause and then failed downstream). + // Every other terminal row still has none, so those still resolve from the + // live subject or not at all. Nothing here needs to branch on which: a row + // that HAS the snapshot uses it, exactly as a paused row does. snapshotField: 'context_json', snapshotPath: ['record'], statusField: 'status', From fc5ceb49eb5c6fada48f7f7935e06bd74e4925a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 23:58:42 +0000 Subject: [PATCH 4/5] fix(service-automation): publish the restore vocabulary at the barrel and true up the sys_automation_run declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contract-review rework (ruling recorded on PR 13951; behaviour unchanged): - index.ts: export type SuspensionRestoreResult / SuspensionRestoreRefusal in the engine.js type block. The verb was already barrel-reachable, so the eight refusal values were published de facto while the union was unnameable de jure — no exhaustive switch, no annotated result, no typed handler parameter. ConsumedSuspension deliberately stays unexported (it appears in no barrel-reachable signature). - consumed-suspension-restore.test.ts: barrel-import pin. The existing suite imports from './engine.js', which is why the gap had no witness. The pin annotates a real result with the BARREL type and writes the exhaustive switch (never-typed default), so removing the export breaks tsc; the runtime half pins the verb on the class the barrel itself exports. - sys-automation-run.object.ts: the declaration asserted invariants this PR falsified. One-class corrections only — the node_type description and the trigger-attribution comment now carve out the consumed-suspension class of terminal row (the same correction shape this PR already applied in plugin-approvals' backfill), and variables_json's description names the presence-discriminator the store's deserializer keys off. - engine.ts: the NO_CONSUMED_SUSPENSION docblock no longer claims "exists and is terminal" — the arm also answers on a non-terminal logged status, and the reason string was already honest about that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs --- .../src/consumed-suspension-restore.test.ts | 63 +++++++++++++++++++ .../services/service-automation/src/engine.ts | 11 ++-- .../services/service-automation/src/index.ts | 10 +++ .../src/sys-automation-run.object.ts | 13 ++-- 4 files changed, 88 insertions(+), 9 deletions(-) diff --git a/packages/services/service-automation/src/consumed-suspension-restore.test.ts b/packages/services/service-automation/src/consumed-suspension-restore.test.ts index 7b131ffd6e..183141f9ef 100644 --- a/packages/services/service-automation/src/consumed-suspension-restore.test.ts +++ b/packages/services/service-automation/src/consumed-suspension-restore.test.ts @@ -52,6 +52,15 @@ import { describe, it, expect, vi } from 'vitest'; import { AutomationEngine, type SuspendedRunStore } from './engine.js'; +// ⛔ BARREL imports, on purpose — everything else in this file imports from +// './engine.js', which is exactly why the missing barrel export had no witness +// (#13951 contract review, finding 1). These lines and the pin at the bottom +// of this file are that witness; see the describe block for what fails where. +import { + AutomationEngine as BarrelAutomationEngine, + type SuspensionRestoreResult, + type SuspensionRestoreRefusal, +} from './index.js'; import { InMemorySuspendedRunStore } from './suspended-run-store.js'; import type { AutomationContext } from '@objectstack/spec/contracts'; import { defineActionDescriptor } from '@objectstack/spec/automation'; @@ -589,3 +598,57 @@ describe('#13909 — what this slice deliberately does NOT do', () => { expect((await engine.getRun(runId))?.status).toBe('failed'); }); }); + +/** + * The exhaustive switch the export exists to make writable (#13951 finding 1): + * an operator surface maps each refusal to its remedy, because the remedy + * differs for each. Compile-time exhaustive — the `default` arm types the + * scrutinee `never`, so growing {@link SuspensionRestoreRefusal} without + * extending every consumer is a tsc error here, which is exactly the + * protection a consumer could not buy while the union was unnameable. + */ +function remedyFor(refusal: SuspensionRestoreRefusal): string { + switch (refusal) { + case 'RESTORE_IN_PROGRESS': return 'wait: this process is already restoring it'; + case 'RESUME_IN_PROGRESS': return 'wait: its outcome is not decided yet'; + case 'RUN_SUSPENDED': return 'nothing to do: it is already resumable'; + case 'STORE_UNAVAILABLE': return 'fix the store failure, then re-issue the restore'; + case 'RUN_COMPLETED': return 'nothing to do: it finished'; + case 'RUN_CANCELLED': return 'ask who cancelled it before undoing their decision'; + case 'NO_CONSUMED_SUSPENSION': return 'read the reason: it names the status observed'; + case 'RUN_NOT_FOUND': return 'check the run id'; + default: { + const unhandled: never = refusal; + return unhandled; + } + } +} + +// Barrel nameability (#13951 contract review, finding 1). The refusal +// vocabulary was published de facto — a consumer could call the +// barrel-reachable verb and receive the eight values at runtime — but +// unnameable de jure: the barrel exported neither result type, and the exports +// map publishes only '.', so no exhaustive switch was writable. This block +// does, with BARREL names only, the things the finding says a consumer must be +// able to do, and it FAILS if the barrel export is removed — split across the +// two channels that actually check each half: +// - the type half breaks at `tsc --noEmit` (TS2305 on the type-only imports +// above; vitest transpiles without type-checking, so tsc IS its witness); +// - the runtime half (the verb on the class the barrel itself exports) breaks +// right here in vitest. +describe('#13951 — the restore vocabulary is nameable from the barrel', () => { + it('publishes the verb on the same class the barrel exports', () => { + expect(BarrelAutomationEngine).toBe(AutomationEngine); + expect(typeof BarrelAutomationEngine.prototype.restoreConsumedSuspension).toBe('function'); + }); + + it('a consumer can annotate the result and switch exhaustively over the refusal', async () => { + const { engine } = newEngine(new InMemorySuspendedRunStore()); + // The annotation is the point: this is the line the missing export + // made unwritable. + const res: SuspensionRestoreResult = await engine.restoreConsumedSuspension('no-such-run'); + expect(res.restored).toBe(false); + expect(res.refusal).toBe('RUN_NOT_FOUND'); + expect(remedyFor(res.refusal!)).toBe('check the run id'); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 883fc7fb00..2bf9a94e46 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -1244,10 +1244,13 @@ export interface ConsumedSuspension { * - `'RUN_COMPLETED'` — the run finished. Nothing to exit from. * - `'RUN_CANCELLED'` — the run was cancelled (ADR-0044). A restore would * undo a decision somebody made on purpose. - * - `'NO_CONSUMED_SUSPENSION'` — the run exists and is terminal, but no - * consumed suspension is available for it: it never paused, or its snapshot - * is gone (no store configured and the in-memory journal evicted it, or a - * later terminal record cleared it). Deliberately does NOT claim which — + * - `'NO_CONSUMED_SUSPENSION'` — the run exists, but no consumed suspension + * is available for it: it never paused, or its snapshot is gone (no store + * configured and the in-memory journal evicted it, or a later terminal + * record cleared it). Usually a terminal (`failed`) run, but this arm also + * answers when the log's last word on the run is non-terminal + * (`running` / `pending`) — the fall-through does not require terminality. + * Deliberately does NOT claim which cause — * nothing in the engine can tell those apart, and the result's `reason` * names the status actually observed instead of overclaiming. * - `'RUN_NOT_FOUND'` — no record of this run at all. diff --git a/packages/services/service-automation/src/index.ts b/packages/services/service-automation/src/index.ts index 259152fd2f..4a0af058ea 100644 --- a/packages/services/service-automation/src/index.ts +++ b/packages/services/service-automation/src/index.ts @@ -34,6 +34,16 @@ export type { // one it cannot derive — the shadowed definition is not in the flow map. FlowContender, FlowShadowingRecord, + // [#13909] The operator exit verb's result vocabulary. The method + // (`AutomationEngine.restoreConsumedSuspension`) was already + // barrel-reachable, so consumers received these values at runtime; without + // the type names they could not annotate a result, type a handler + // parameter, or write an exhaustive switch over the refusal union — and + // the refusals exist precisely to be branched on (the remedy differs for + // each). `ConsumedSuspension` stays unexported on purpose: it appears in + // no barrel-reachable signature. + SuspensionRestoreResult, + SuspensionRestoreRefusal, } from './engine.js'; // [#11997] ADR-0005 overlay precedence for same-named flow definitions. The boot diff --git a/packages/services/service-automation/src/sys-automation-run.object.ts b/packages/services/service-automation/src/sys-automation-run.object.ts index 1fec35a9bd..dfc9199513 100644 --- a/packages/services/service-automation/src/sys-automation-run.object.ts +++ b/packages/services/service-automation/src/sys-automation-run.object.ts @@ -125,7 +125,7 @@ export const SysAutomationRun = ObjectSchema.create({ label: 'Node Type', required: false, maxLength: 255, - description: 'Registry type of the node a suspended run paused at (approval / screen / wait / …). Keys the resume authorization gate (#3801) — captured at suspend time rather than re-read from a flow that may have been republished since. Null on rows written before the gate shipped, and on terminal history rows.', + description: 'Registry type of the node a suspended run paused at (approval / screen / wait / …). Keys the resume authorization gate (#3801) — captured at suspend time rather than re-read from a flow that may have been republished since. Null on rows written before the gate shipped, and on terminal history rows — except (since #13909) the one class of terminal row that carries a restorable consumed suspension (a run whose resume consumed its pause and then failed downstream), which keeps the paused node\'s type so a restore re-arms the gate.', group: 'State', }), @@ -163,9 +163,12 @@ export const SysAutomationRun = ObjectSchema.create({ // filter on `trigger_object` + `trigger_record_id`; "was last night's // failure storm scheduled or record-driven?" is a group-by on // `trigger_type`. Folded into `context_json` they would be legible one row - // at a time and unqueryable in aggregate — and `context_json` is not even - // written on terminal history rows, which is how the durable copy of the - // run log ended up strictly less informative than the in-memory one. + // at a time and unqueryable in aggregate — and `context_json` is not + // written on terminal history rows (except, since #13909, the one class of + // terminal row that carries a restorable consumed suspension — a run whose + // resume consumed its pause and then failed downstream; every other + // terminal row still has none), which is how the durable copy of the run + // log ended up strictly less informative than the in-memory one. trigger_type: Field.text({ label: 'Trigger Type', required: false, @@ -230,7 +233,7 @@ export const SysAutomationRun = ObjectSchema.create({ variables_json: Field.textarea({ label: 'Variables', required: false, - description: 'JSON snapshot of the flow variable map at suspend time.', + description: 'JSON snapshot of the flow variable map at suspend time. On a terminal row its PRESENCE is the discriminator (#13909): nothing but the consumed-suspension path writes it there, so variables_json present on a completed/failed row ⇔ the row carries a restorable suspension — the store\'s deserializer keys off exactly this.', group: 'State', }), From f422d353a47903e24282f415bce247084a3c7e64 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 00:51:20 +0000 Subject: [PATCH 5/5] fix(service-automation): keep tracker ids out of the declaration's runtime strings check:doc-authoring is right: the two descriptions the rework corrected carried '#13909' inside runtime string prose, which reaches operators who cannot resolve a tracker id. The carve-out text stays; the ids move to adjacent comments (the reader who can resolve them reads the source). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016ZC5rNQj3WEet5HAmmAkMs --- .../src/sys-automation-run.object.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/services/service-automation/src/sys-automation-run.object.ts b/packages/services/service-automation/src/sys-automation-run.object.ts index dfc9199513..69d085b960 100644 --- a/packages/services/service-automation/src/sys-automation-run.object.ts +++ b/packages/services/service-automation/src/sys-automation-run.object.ts @@ -121,11 +121,14 @@ export const SysAutomationRun = ObjectSchema.create({ group: 'State', }), + // [#13909] The terminal-row carve-out in this description: the restore + // verb re-arms the resume authorization gate from this column, so the one + // terminal-row class that stays restorable keeps it. node_type: Field.text({ label: 'Node Type', required: false, maxLength: 255, - description: 'Registry type of the node a suspended run paused at (approval / screen / wait / …). Keys the resume authorization gate (#3801) — captured at suspend time rather than re-read from a flow that may have been republished since. Null on rows written before the gate shipped, and on terminal history rows — except (since #13909) the one class of terminal row that carries a restorable consumed suspension (a run whose resume consumed its pause and then failed downstream), which keeps the paused node\'s type so a restore re-arms the gate.', + description: 'Registry type of the node a suspended run paused at (approval / screen / wait / …). Keys the resume authorization gate (#3801) — captured at suspend time rather than re-read from a flow that may have been republished since. Null on rows written before the gate shipped, and on terminal history rows — except the one class of terminal row that carries a restorable consumed suspension (a run whose resume consumed its pause and then failed downstream), which keeps the paused node\'s type so a restore re-arms the gate.', group: 'State', }), @@ -230,10 +233,13 @@ export const SysAutomationRun = ObjectSchema.create({ group: 'Trigger', }), + // [#13909] The presence-discriminator named in this description lives in + // ObjectStoreSuspendedRunStore.deserializeConsumedSuspension — one writer, + // one reader, this column is the key for both. variables_json: Field.textarea({ label: 'Variables', required: false, - description: 'JSON snapshot of the flow variable map at suspend time. On a terminal row its PRESENCE is the discriminator (#13909): nothing but the consumed-suspension path writes it there, so variables_json present on a completed/failed row ⇔ the row carries a restorable suspension — the store\'s deserializer keys off exactly this.', + description: 'JSON snapshot of the flow variable map at suspend time. On a terminal row its PRESENCE is the discriminator: nothing but the consumed-suspension path writes it there, so variables_json present on a completed/failed row ⇔ the row carries a restorable suspension — the store\'s deserializer keys off exactly this.', group: 'State', }),