Skip to content

Commit 19955fb

Browse files
fix(service-automation): settle the run row when a resume is refused because its flow or parked node is gone (#24003)
Fixes #23963 Clause-②: no ## What changed `resumeInternal` has two never-dispatched exits for a suspension that is real but can never continue: the run's flow is no longer registered, or the node it parked on was edited away. The engine already classifies both as the terminal `RUN_NOT_FOUND` class, and a transport answers them 404. Until now both exits returned without touching the run, so its `sys_automation_run` row kept `status: 'paused'` through every later resume, an uninstall and a cold restart. Retention never pruned it, because retention's `onlyWhen` scope holds terminal statuses only. Both exits now return through one new method, `settleUnresumableSuspension` (`packages/services/service-automation/src/engine.ts:9016`, called at `:7799` and `:7803`). In the same call that answers the refusal, it does three things: 1. It consumes the parked row through `claimAdvance` (`:9019`). That is the compare-and-set the resume path already takes (`:8117`): the store deletes the row only while it is still parked at the node this replica read. 2. It releases the suspension through `forgetSuspendedRun(run, 'failed', claimed)` (`:9038`), the one consumption choke point, so a `wait` node's armed job is torn down too. 3. It records a `failed` history row whose `error` is the refusal's own text, through `recordLog`. The answer is byte-identical: same `code`, same `error` text. A second resume finds nothing parked and answers `RUN_NOT_FOUND` (`No suspended run 'ID'`). It writes nothing. **Why `failed` and not `cancelled`.** `failSuspendedRun` sets the precedent for an ancestor that can never be resumed. `cancelled` (ADR-0044) records a deliberate act, and `restoreConsumedSuspension` refuses it as one. Nobody decided to end this run. It can never continue, and `error` is the column that says why. `failed` is already in retention's `onlyWhen` list (`sys-automation-run.object.ts:67`), so the row ages out by the existing rule. Retention is unchanged. **Which refusals do NOT record anything.** - **Before the host seals the boot** (`:9018`). The guard is `nodeTypeVocabularySealed`, which `AutomationServicePlugin` sets at `kernel:bootstrapped` (`plugin.ts:1592`), strictly after every flow bind of the boot. Before that, an absent flow can be one that is not registered yet. Measured on this branch: the plugin re-arms overdue wait timers by calling `resume` inside `start()` (`plugin.ts:1649`), with the engine unsealed. A flow its provider binds during `kernel:ready` is absent at that moment. Without this guard, the plugin's own boot recorded such a run `failed` a moment before its flow arrived (ablation A below). Before the seal, the refusal is answered exactly as before and nothing is recorded (AGENTS.md "Startup registry reads"). An embedded host that never seals keeps the previous behaviour. - **A claim another consumer won** (`lost`). A concurrent resume or cancel already consumed the row and wrote its own outcome, so nothing is written here. The stale snapshot is evicted, as on the resume path. - **A claim the store cannot answer** (`unavailable`). The resume answers `STORE_UNAVAILABLE` (503), as the resume path's claim does (`:9029`). Answering `RUN_NOT_FOUND` there would close the matter for the caller while the row could still read `paused`. A retry that finds the run still parked settles it. The history write is guarded the way the completion paths guard theirs. The suspension is already consumed when it runs, so a throw out of `recordLog` is logged at `error`, with the consequence and the fix stated. It does not turn the refusal into a rejected promise. ## Tests New file `packages/services/service-automation/src/resume-gone-suspension-settle.test.ts`, 8 cases: - **Full stack.** `ObjectKernel` + `ObjectQLPlugin` + `AutomationServicePlugin` over a sqlite file, with the real `ObjectStoreSuspendedRunStore` and a run parked on a real `screen` node. Four cases: - flow removed; - parked node edited away; - a second resume answers the same code and leaves the settled row deep-equal; - a cold restart onto a flow that disappeared while the process was down settles on its first resume. Each case reads `sys_automation_run` back: the paused row is gone when `resume` returns, and the terminal row reads `failed` with the refusal text in `error`. The status is asserted against `SysAutomationRun.lifecycle.retention.onlyWhen.status.$in`, read off the object, not restated. - **Engine level.** A resume before the seal leaves the row parked, and the run continues once its flow registers. A claim that throws answers `STORE_UNAVAILABLE` with the row untouched, and the retry settles it. A claim lost to a concurrent `cancelRun` leaves only the cancel's `cancelled` record. - **Plugin boot order.** Two process lifetimes over one sqlite file, with a real `wait` node. A plugin composed after automation binds the flow in its own `kernel:ready` handler. A positive control asserts that the boot-time re-arm resume actually reached the flow-gone exit. After the boot the row is still `paused`, and a resume completes it. **Red at base** (`4153a7089f`, the pins against the unchanged engine): 6 failed, 2 passed. The first failure: `the paused row must not survive the refusal: expected { …(32) } to be null`. The 2 that passed are the two guards, which pin behaviour that must not change. **Green:** the file 8/8. Package suite `pnpm --filter @objectstack/service-automation exec vitest run --maxWorkers=2`: 189 files, 2533 tests passed. `pnpm --filter @objectstack/service-automation typecheck` (tsc plus `check:test-typecheck`): exit 0. Both on the merged head `0a1fd3171a`. The new file appears in both tsc programs (`--listFiles`, `tsconfig.json` and `tsconfig.test.json`). **Ablations.** The fix was committed first (`af6e1b4d2d`). Each mutation went through `scripts/ablation-replace.mjs` (anchor hit 1 to 0, blob changed) and was restored and proven: blob `a664529755` == HEAD, `git diff HEAD` empty. The subject is imported from `src`, so no `dist` leg applies. | Ablation | Direction | Result | |---|---|---| | A. Seal guard deleted | Predicted the two guards red | 2 failed, 6 passed. Plugin boot order: `the run must still be parked after the boot: expected undefined to be 'paused'` | | B. Settle disabled (unconditional early return) | Predicted the six settle pins red | 6 failed, 2 passed | | C. `claimAdvance` replaced by an `unguarded` claim | Predicted the two claim pins red | 2 failed, 6 passed. Lost claim: `expected [ [ 'failed', …(1) ] ] to deeply equal [ [ 'cancelled', …(1) ] ]` | ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `0a1fd3171a` derived 66 commands. All 66 were run, each exit captured before any pipe, and reconciled with `--ran`: 65 at exit 0, and 1 NOT MEASURED. `pnpm check:dual-build-cjs-loads` exited 3, `PREREQUISITE NOT MET`: it reads built output for the whole tree and this worktree built only this package's closure. That family is CI's reading. Other verdicts: - `check:durability-log-level`: 48 durability-critical catch seams, all loud. - `check:nul-bytes`: OK. - `check:doc-authoring`: clean. - `check-adr-0087-registration`: no declared-breaking changeset. - `check-empty-changeset`: one declaring changeset. - `check:engine-double-contract`: OK. ## Acceptance notes - **Remaining case, not filed.** An overdue `wait` run whose flow disappeared while the process was down is not settled by the boot re-arm, which runs before the seal. The row stays `paused`, as before, until any later resume settles it. The re-arm ignores the resume's answer (`rearmSuspendedWaitTimers` counts `rearmed++` on any non-throwing result), so a refused overdue run is also counted in its "Re-armed N" line. Moving the re-arm after `kernel:bootstrapped`, or checking its result, would close both. That is `plugin.ts` and `wait-node.ts`, outside this card's surface. Carrier: none. - **Withdrawal-time settle.** The card's optional `withdrawFlow` settle is not done. This change is the resume-time floor the card names. - **Subflow children.** A parent parked at a `subflow` whose own flow is gone is now settled `failed`. Its child run is its own run with its own flow, and it stays resumable by its own id, as before. - **No dogfood pin.** The HTTP status mapping (`RUN_NOT_FOUND` to 404, `STORE_UNAVAILABLE` to 503, `packages/runtime/src/domains/automation.ts`) is unchanged, and the full-stack pins read the row on a real store. - **Branch state.** `origin/main` was merged at `0a1fd3171a`, after PR 23954 (the `engine.ts` resolver-retirement hunks) landed. It merged without conflict. Four later `main` commits touch `objectql`, `spec` and `cli`, not `service-automation`. --- _Generated by [Claude Code](https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 92fd343 commit 19955fb

3 files changed

Lines changed: 613 additions & 2 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
"@objectstack/service-automation": patch
3+
---
4+
5+
Clause-②: no
6+
7+
fix(service-automation): a resume refused because the run's flow or parked node is gone now settles the run's `sys_automation_run` row
8+
9+
- **What changed.** A resume refused with `RUN_NOT_FOUND` because the run's flow is no longer registered, or because the node it parked on was edited away, now settles the run in the same call. The paused row is consumed and a `failed` history row records the refusal's own text in `error`. Before, the row kept `status: 'paused'` through every later resume, an uninstall and a cold restart. Run lists and paused-run counts kept showing a run the engine already knew could never continue, and retention never aged the row out, because it prunes terminal statuses only.
10+
- **Unchanged:** the answer. It has the same code and the same text, and a transport still answers 404. A second resume answers `RUN_NOT_FOUND` again (`No suspended run`) and writes nothing. Retention is unchanged: `failed` was already in its scope, and paused rows are still never pruned.
11+
- **When nothing is recorded.** Before the host seals the boot, a missing flow may be one that is not registered yet. `AutomationServicePlugin` re-arms overdue wait timers in `start()`, before flows bound at `kernel:ready` arrive. So before the seal the refusal records nothing, as before. The host seals with `sealNodeTypeVocabulary()`, which the plugin calls at `kernel:bootstrapped`. An embedded host that never calls it keeps the previous behaviour.
12+
- **Concurrency.** The settle consumes the row through the same compare-and-set a resume takes. If a concurrent resume or cancel consumed the row first, the call writes nothing. If the store cannot answer the compare-and-set, the resume answers `STORE_UNAVAILABLE` (503), and a retry settles the run.

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

Lines changed: 115 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7786,13 +7786,24 @@ export class AutomationEngine implements IAutomationService {
77867786
// which of the three causes it was, verbatim on the wire. A distinct
77877787
// code would be vocabulary nothing reads — add one the day a caller
77887788
// needs to branch on the difference.
7789+
//
7790+
// [#23963] And the verdict is carried onto the RUN, not only onto
7791+
// the answer: both exits go through
7792+
// {@link settleUnresumableSuspension}, which moves the parked row to
7793+
// `failed` with this same text in `error`. Answering "gone for
7794+
// good" while the row kept `status: 'paused'` left it resumable on
7795+
// every surface that reads the row, and outside retention, whose
7796+
// scope is terminal statuses only.
77897797
const flow = this.flows.get(run.flowName);
77907798
if (!flow) {
7791-
return { success: false, code: 'RUN_NOT_FOUND', error: `Flow '${run.flowName}' not found for run '${runId}'` };
7799+
return this.settleUnresumableSuspension(run, `Flow '${run.flowName}' not found for run '${runId}'`);
77927800
}
77937801
const node = flow.nodes.find(n => n.id === run.nodeId);
77947802
if (!node) {
7795-
return { success: false, code: 'RUN_NOT_FOUND', error: `Suspended node '${run.nodeId}' no longer exists in flow '${run.flowName}'` };
7803+
return this.settleUnresumableSuspension(
7804+
run,
7805+
`Suspended node '${run.nodeId}' no longer exists in flow '${run.flowName}'`,
7806+
);
77967807
}
77977808

77987809
// #4354 — up-bubble: the child that just finished did work this run
@@ -8956,6 +8967,108 @@ export class AutomationEngine implements IAutomationService {
89568967
}, run.context, consumed?.run);
89578968
}
89588969

8970+
/**
8971+
* [#23963] Answer a resume refused as GONE FOR GOOD, and settle the run it
8972+
* names in the same act.
8973+
*
8974+
* {@link resumeInternal} reaches this from its two never-dispatched exits:
8975+
* the run's flow is no longer registered, or the node it parked on was
8976+
* edited away. Both are the terminal `RUN_NOT_FOUND` class (#8684). The
8977+
* answer is unchanged, and the run now moves to the state the answer
8978+
* describes: the parked row is consumed and a `failed` history row records
8979+
* `cause`, the same text the caller is answered. `failed` and not
8980+
* `cancelled`, for {@link failSuspendedRun}'s reason: nobody decided to end
8981+
* this run (ADR-0044's `cancelled`, which a restore refuses as a deliberate
8982+
* act). It can never continue, and `error` is the column that says why.
8983+
* Retention's `onlyWhen` scope already holds `failed`, so the row ages out
8984+
* like any other history row and nothing there changes.
8985+
*
8986+
* Three guards, each against a different way the verdict could be wrong:
8987+
*
8988+
* - **The boot is still filling the flow registry.** Before the host
8989+
* seals the boot, an absent flow means "not registered YET": the plugin
8990+
* re-arms overdue wait timers in its `start()`, and those resume runs
8991+
* before its `kernel:ready` handler binds the inline app flows they
8992+
* belong to. Recording the verdict there would kill a run whose flow
8993+
* arrives a moment later (AGENTS.md "Startup registry reads"). The host
8994+
* declares the boot complete with {@link sealNodeTypeVocabulary}, which
8995+
* `AutomationServicePlugin` calls at `kernel:bootstrapped`, strictly
8996+
* after every flow bind of the boot. Before the seal this answers the
8997+
* refusal and records nothing, which is exactly what shipped before.
8998+
* After it, a flow can leave only by an explicit withdrawal. An
8999+
* embedded host that never seals keeps the pre-#23963 behaviour.
9000+
* - **Another consumer got there first.** The row is consumed through
9001+
* {@link claimAdvance}, the compare-and-set the resume path takes: the
9002+
* store removes the row only while it is still parked where this
9003+
* replica read it. `lost` means a concurrent resume or cancel already
9004+
* consumed it and wrote its own outcome, so nothing is written here.
9005+
* The answer stays the refusal this replica reached.
9006+
* - **The store cannot say whether the row was consumed.** A claim that
9007+
* throws answers `STORE_UNAVAILABLE`, as the resume path's claim does.
9008+
* That tells the caller the row may still read `paused`, and a retry
9009+
* settles it. Answering `RUN_NOT_FOUND` there would tell the caller the
9010+
* matter is closed while the row it describes stayed parked.
9011+
*
9012+
* The history write is guarded the way the completion paths guard theirs
9013+
* (#16274): the run is already consumed when it runs, so a throw out of
9014+
* it must not turn the refusal into a rejected promise.
9015+
*/
9016+
private async settleUnresumableSuspension(run: SuspendedRun, cause: string): Promise<AutomationResult> {
9017+
const refusal: AutomationResult = { success: false, code: 'RUN_NOT_FOUND', error: cause };
9018+
if (!this.nodeTypeVocabularySealed) return refusal;
9019+
const claim = await this.claimAdvance(run);
9020+
if (claim.kind === 'lost') {
9021+
// [#15832] The store answered and holds no row where this replica
9022+
// read one, so whatever snapshot this process still holds is stale.
9023+
this.evictConsumedSuspension(run.runId);
9024+
return refusal;
9025+
}
9026+
if (claim.kind === 'unavailable') {
9027+
return {
9028+
success: false,
9029+
code: 'STORE_UNAVAILABLE',
9030+
error:
9031+
`Run '${run.runId}' can never continue (${cause}), but the durable suspended-run store was ` +
9032+
`unreachable while settling it, and whether the store consumed the suspension is UNKNOWN (a ` +
9033+
`failure can arrive after a committed delete), so its record may still read paused. Retry once ` +
9034+
`the store is available: a retry that finds the run still parked settles it, and one that finds ` +
9035+
`the suspension gone answers RUN_NOT_FOUND: ${claim.message}`,
9036+
};
9037+
}
9038+
await this.forgetSuspendedRun(run, 'failed', claim.kind === 'claimed');
9039+
try {
9040+
this.recordLog({
9041+
id: run.runId,
9042+
flowName: run.flowName,
9043+
flowVersion: run.flowVersion,
9044+
status: 'failed',
9045+
startedAt: run.startedAt,
9046+
completedAt: new Date().toISOString(),
9047+
durationMs: Date.now() - run.startTime,
9048+
trigger: buildRunTrigger(run.context),
9049+
steps: run.steps,
9050+
error: cause,
9051+
}, run.context);
9052+
} catch (bookkeeping) {
9053+
// #4632 verdict: DURABILITY, so `error`. The suspension is consumed
9054+
// and the caller is answered the truthful refusal, so nothing reads
9055+
// as wrong from the outside. But the `failed` row that says why
9056+
// never landed, and nothing retries it. Same sink and same level as
9057+
// the #16274 completion guard. THIRD argument per
9058+
// `error(message, error?, meta?)`; the `Error` slot stays empty
9059+
// (#5575) and the thrown text rides the structured slot (#6499).
9060+
this.logger.error(
9061+
`[Automation] run '${run.runId}' of flow '${run.flowName}' can never continue and its suspension ` +
9062+
`was consumed, but recording its terminal 'failed' history threw, so no history row says why: ` +
9063+
`the caller was answered RUN_NOT_FOUND, the run no longer reads as paused, and nothing retries ` +
9064+
`the write. Fix the history failure in this record's meta.`,
9065+
undefined,
9066+
describeThrownForLog(bookkeeping),
9067+
);
9068+
}
9069+
return refusal;
9070+
}
9071+
89599072
/**
89609073
* Cancel a suspended run (ADR-0044): consume its continuation and record a
89619074
* terminal `cancelled` log so it stops surfacing as resumable. The

0 commit comments

Comments
 (0)