diff --git a/.changeset/17541-resume-door-consults-inspection.md b/.changeset/17541-resume-door-consults-inspection.md new file mode 100644 index 00000000000..ca026b6dbf6 --- /dev/null +++ b/.changeset/17541-resume-door-consults-inspection.md @@ -0,0 +1,64 @@ +--- +'@objectstack/spec': minor +'@objectstack/runtime': minor +--- + +the resume door's `repairable` is answered by the engine on the exits that stamp no status — `IAutomationService` declares the read-only `inspectConsumedSuspension` (#17541) + +Clause-②: yes (widening) + +The resume route's `400 FLOW_FAILED` details computed `repairable` as the single +expression `status === 'stranded'`. That word is stamped on exactly one exit — +the run that consumed its OWN pause and then threw downstream. The subflow +DELEGATION exit stamps nothing on purpose: a caller resumes the PARENT, the +signal is forwarded down, the child strands, and the parent frame answers +`{ success: false, error, durationMs }`, because nothing re-arms an ancestor by +resuming it and stamping `'stranded'` there would send an operator to retry a +recovery that cannot succeed. + +Since the nested-chain restore landed, that parent's consumed pause IS +journalled and one `restoreConsumedSuspension(parentRunId)` re-arms the whole +chain leaf-first. So the wire answered `repairable: false` about a run the +operator verb WILL repair, and a client written exactly as the reference page +instructs closed it as terminal. Measured through the HTTP route, before and +after, on the same parked delegation: + +```json +before 400 { "error": { "code": "FLOW_FAILED", + "details": { "runId": "run_…", "repairable": false } } } +after 400 { "error": { "code": "FLOW_FAILED", + "details": { "runId": "run_…", "repairable": true } } } +``` + +…while at that same instant the engine answered +`inspectConsumedSuspension(runId) → { repairable: true, witness: 'journal' }` +and `restoreConsumedSuspension(runId) → { restored: true, chain: [child, parent] }`. + +**`@objectstack/spec` — additive, `minor`.** `IAutomationService` declares the +optional read-only member `inspectConsumedSuspension(runId)`, which +`AutomationEngine` already implements publicly: would the restore verb have a +consumed suspension to put back for this run? It re-arms nothing and reads the +same two witnesses that verb reads, so what it calls repairable IS what that +verb restores. The declared result is deliberately narrower than the +implementation's, the way `restoreConsumedSuspension`'s already is — `reason` is +typed as the string the implementation answers, not as an enumeration this +contract would have to keep in step, and the engine's wider type satisfies it +under `implements`. `ResumeFailureDetailsSchema.repairable`'s `.describe()` is +rewritten to the truth and the generated reference page regenerated with it. No +key is added, renamed or retired on any wire schema. + +**`@objectstack/runtime` — the door.** On a `400 FLOW_FAILED` whose result +carries a `status`, that stamp still decides, and the engine is not consulted at +all. On a result that carries none, the door asks the declared member and relays +its `repairable`. Both ways of not getting an answer are FAIL-CLOSED: a service +that declares no inspection member answers `false` exactly as it did before, and +an inspection that REJECTS (a store it could not read) answers `false` and says +so once at `warn` — an unreadable store is UNKNOWN, not "nothing to restore", +and it is never allowed to replace the `400` the caller asked for with a `500`. + +⛔ The fence is untouched: a cascade-failed ancestor is still never STAMPED +`'stranded'`. Its repairability is carried by the journal and REPORTED by the +inspection, which is exactly why the door asks instead of reading a word. ⛔ And +no new `AutomationResult.status` member is minted for this exit — there is +nothing new for a client to learn, and `details.repairable` is the member a +client was already told to branch on. diff --git a/content/docs/api/client-sdk.mdx b/content/docs/api/client-sdk.mdx index 6886baf7126..dda5cdc53c4 100644 --- a/content/docs/api/client-sdk.mdx +++ b/content/docs/api/client-sdk.mdx @@ -767,17 +767,22 @@ try { if (!isApiError(err) || err.code !== 'FLOW_FAILED') throw err; const verdict = ResumeFailureDetailsSchema.safeParse(err.details); if (verdict.success && verdict.data.repairable) { - // verdict.data.status === 'stranded' — verdict.data.runId names the run - // an operator can re-arm; offer that instead of closing as terminal. + // verdict.data.runId names the run an operator can re-arm; offer that + // instead of closing as terminal. Read `repairable` alone: `status` is + // absent on the subflow delegation failure, which is repairable without + // ever carrying the word `stranded`. } } ``` `repairable` is always present on the resume door's `400` — `false` on a plain terminal failure, deliberately, so an absent member reads as an older server -rather than as "not repairable". `status` is the engine's own stamp -(`'stranded'` or `'failed'`), forwarded when the engine set one and never -invented by the door. The code stays `FLOW_FAILED`: there is no `FLOW_STRANDED` +rather than as "not repairable". It is not a reading of `status`: where the +engine stamped one, that stamp decides; where it stamped none — the subflow +delegation failure — the door asks the engine's read-only inspection, so +`repairable: true` can arrive with no `status` beside it. `status` is the +engine's own stamp (`'stranded'` or `'failed'`), forwarded when the engine set +one and never invented by the door. The code stays `FLOW_FAILED`: there is no `FLOW_STRANDED` sibling, and the trigger door's `400` carries `errorMessage` / `summary` only — it never resumes, so "repairable" has no referent there. diff --git a/content/docs/automation/flows.mdx b/content/docs/automation/flows.mdx index 3c791cb9464..028a2468083 100644 --- a/content/docs/automation/flows.mdx +++ b/content/docs/automation/flows.mdx @@ -790,11 +790,18 @@ per-node `summary`: "errorMessage": "…", "summary": { … } } } } ``` -Branch on `details.repairable`, never on the message: it is `true` exactly when -`status` is `stranded`, and it is **always present** on this route's `400` — -`false` on a plain terminal failure, so an absent member reads as an older -server rather than as "not repairable". `status` is the engine's own stamp, -forwarded when it set one and never invented by the route. The code does not +Branch on `details.repairable`, never on the message. It is **always present** +on this route's `400`, and it is the engine's own answer rather than a reading +of the status word: where the engine stamped a `status`, that stamp decides +(`stranded`); where it stamped none, the route asks the engine's read-only +inspection and relays the verdict. The second case is a real exit, not a +fallback — a subflow **delegation** failure carries no status, because nothing +re-arms an ancestor by resuming it, and yet the ancestor's consumed pause is +journalled and one operator restore re-arms that whole chain, so it answers +`repairable: true` with no `status` beside it. `false` on a plain terminal +failure, so an absent member reads as an older server rather than as "not +repairable". `status` is the engine's own stamp, forwarded when it set one and +never invented by the route. The code does not change (there is no `FLOW_STRANDED`), and the trigger route's `400` carries `errorMessage` / `summary` only — it never resumes, so the question has no referent there. The shape is `ResumeFailureDetailsSchema` in diff --git a/content/docs/references/api/automation-api.mdx b/content/docs/references/api/automation-api.mdx index e23785f3158..c2f7671dbc7 100644 --- a/content/docs/references/api/automation-api.mdx +++ b/content/docs/references/api/automation-api.mdx @@ -583,7 +583,7 @@ const result = AutomationApiErrorCode.parse(data); | :--- | :--- | :--- | :--- | | **runId** | `string` | ✅ | The run the resume was addressed to - the run that failed, and on the `stranded` arm the run an operator verb can re-arm. Named so a caller acts on an identifier instead of parsing one out of the message | | **status** | `Enum<'failed' \| 'stranded'>` | optional | The engine's own lifecycle verdict for the run, forwarded verbatim when the producer stamped one and never synthesised by the door - absent when the engine reported no status (a subflow child that failed terminally, an engine that predates the discriminator). `stranded` is the terminally-failed-but-repairable run of `AutomationResult.status`; `failed` says the run ran and was rejected. These two terminal-failure members of that union are the only ones that can reach a 400 | -| **repairable** | `boolean` | ✅ | Whether the engine says this run can still be re-armed by an operator verb - `true` exactly when `status` is `stranded`, derived from the engine's discriminator and never from the message text. Always present on this arm: `false` is the honest answer for every other exit, the ones that report no status included, because an absent member would be indistinguishable from a server that predates this field, and promising a repair verb that will refuse is worse than promising nothing | +| **repairable** | `boolean` | ✅ | Whether the engine says this run can still be re-armed by an operator verb. Answered from the engine, two ways, and never from the message text: where the engine stamped a `status`, that word decides (`stranded` is the run whose OWN pause a resume consumed before a downstream node threw); where it stamped none, the door asks the engine's read-only inspection (`IAutomationService.inspectConsumedSuspension`) and relays its verdict. The second half is not a fallback but the honest answer for a real exit: a subflow DELEGATION failure carries no status, because nothing re-arms an ancestor by resuming it, and yet the ancestor's consumed pause is journalled and the restore verb re-arms that chain as one unit. Always present on this arm: an absent member would be indistinguishable from a server that predates this field. Every way of not getting an answer is fail-closed - a service that declares no inspection member, and a store the inspection could not read - because promising a repair verb that will refuse is worse than promising nothing | --- diff --git a/packages/runtime/src/domains/automation-resume-delegation-repairable.test.ts b/packages/runtime/src/domains/automation-resume-delegation-repairable.test.ts new file mode 100644 index 00000000000..752eec45222 --- /dev/null +++ b/packages/runtime/src/domains/automation-resume-delegation-repairable.test.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #17541 — the resume door's `repairable` is answered by the ENGINE on the + * exits that stamp no `status`, instead of being read off a stamp that is not + * there (the maintainer's ruling on this card, 2026-09-17, letter 1). + * + * ## The defect, as a wire reading + * + * `repairable` used to be the single expression `status === 'stranded'`. That + * word is stamped on exactly one exit — the run that consumed its OWN pause + * and then threw downstream. The subflow **delegation** exit deliberately + * stamps nothing: a caller resumes the PARENT, `resumeInternal` forwards the + * signal down, the child strands, and the parent frame answers + * `{ success: false, error, durationMs }`, because nothing re-arms an ancestor + * by resuming it and stamping `'stranded'` there would send an operator to + * retry a recovery that cannot succeed. Since #15222 that parent's consumed + * pause IS journalled and `restoreConsumedSuspension(parentRunId)` re-arms the + * chain as one unit — so the wire answered `repairable: false` about a run the + * operator verb WILL repair, and a client written exactly as the docs instruct + * closed it as terminal. + * + * ## What this file pins, and what it deliberately does NOT + * + * This is the DOOR's shaping, with a fake service, so every arm is reachable + * by name — including the two that a real engine will not produce on demand (a + * host with no inspection member; a store the inspection cannot read). The + * end-to-end fact, driven through the real engine and the HTTP route, is + * `@objectstack/verify`'s `automation-resume-delegation-repairable.test.ts`; + * the engine's own halves are `service-automation`'s + * `nested-strand-chain-restore.test.ts` and + * `consumed-suspension-inspection.test.ts`. + * + * ⛔ The fence #15222 was dispatched with is untouched and is not re-pinned + * here: a cascade-failed ancestor is still never STAMPED `'stranded'`. Its + * repairability is carried by the journal and REPORTED by the inspection, + * which is precisely why the door has to ask instead of reading a word. + * + * ⛔ And the door asks a member the CONTRACT declares — `IAutomationService` + * gained the optional read-only `inspectConsumedSuspension` for this card. + * Reaching for an undeclared member would be the fail-open shape the route's + * own `501` arm exists to prevent, so every way of not getting an answer here + * is FAIL-CLOSED: no member, and a rejected read, both answer `false`. + * + * The sibling file `automation-resume-stranded-details.test.ts` is the CONTROL + * for this change and is deliberately left untouched: every non-delegation + * exit it pins — the stamped `'stranded'`, the stamped `'failed'`, the + * status-less arm on a service with no inspection member, the regex control, + * the trigger door's absent member — still answers exactly what it did. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; + +import { ResumeFailureDetailsSchema } from '@objectstack/spec/api'; +import type { AutomationResult, IAutomationService } from '@objectstack/spec/contracts'; + +import { HttpDispatcher } from '../http-dispatcher.js'; + +const CTX = { request: {}, executionContext: { userId: 'user_1' } } as any; +const RESUME = '/parent_flow/runs/run_parent/resume'; + +/** + * The engine's answer on the delegation exit, measured through the real + * engine while this card was written: the parent frame carries no `status`, + * no `errorMessage` and no `summary` — only the subflow failure text. + */ +const DELEGATION_FRAME: AutomationResult = { + success: false, + error: "subflow run 'run_child' (child_flow) failed: update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found", + durationMs: 7, +}; + +type Inspect = NonNullable; + +function makeDispatcher(resumeResult: AutomationResult, inspect?: Inspect) { + const spies = { + resume: vi.fn(async () => resumeResult), + ...(inspect ? { inspectConsumedSuspension: vi.fn(inspect) } : {}), + }; + const services: Record = { automation: spies }; + const resolve = (name: string) => services[name]; + const kernel: any = { + getService: resolve, + getServiceAsync: async (name: string) => resolve(name), + context: { getService: resolve }, + }; + return { dispatcher: new HttpDispatcher(kernel), spies }; +} + +afterEach(() => { vi.restoreAllMocks(); }); + +describe('#17541 — the status-less resume exit asks the engine whether the run is still repairable', () => { + it('DELEGATION — a frame with no status answers repairable: true when the engine holds the consumed suspension', async () => { + const { dispatcher, spies } = makeDispatcher( + DELEGATION_FRAME, + async (runId: string) => ({ repairable: true, runId }), + ); + + const result = await dispatcher.handleAutomation(RESUME, 'POST', {}, CTX); + + // The door asked about the run it was addressed to — the path's + // `:runId`, which on this exit IS the run whose pause was consumed. + expect(spies.inspectConsumedSuspension).toHaveBeenCalledWith('run_parent'); + expect(result.response?.status).toBe(400); + const error = result.response?.body?.error; + expect(error?.code).toBe('FLOW_FAILED'); + // ⛔ Still no `status`: the door relays the producer's stamp and the + // producer stamped none. `repairable` is the answer, not the word. + expect(error?.details).toEqual({ runId: 'run_parent', repairable: true }); + expect(ResumeFailureDetailsSchema.safeParse(error?.details).success).toBe(true); + // Not prose: the message names the subflow failure, not the verdict. + expect(error?.message).not.toMatch(/repairable/i); + }); + + it('FIRING CONTROL — the same status-less arm answers false when the engine says the suspension is gone', async () => { + // Without this the pin above would pass over a door that simply + // flipped the status-less arm to `true` and never asked anything. + const { dispatcher, spies } = makeDispatcher( + DELEGATION_FRAME, + async (runId: string) => ({ repairable: false, runId, reason: 'NO_CONSUMED_SUSPENSION' }), + ); + + const result = await dispatcher.handleAutomation(RESUME, 'POST', {}, CTX); + + expect(spies.inspectConsumedSuspension).toHaveBeenCalledWith('run_parent'); + expect(result.response?.status).toBe(400); + expect(result.response?.body?.error?.details).toEqual({ runId: 'run_parent', repairable: false }); + }); + + it('FAIL-CLOSED — a service that declares no inspection member answers false, exactly as it did before this card', async () => { + const { dispatcher } = makeDispatcher(DELEGATION_FRAME); + + const result = await dispatcher.handleAutomation(RESUME, 'POST', {}, CTX); + + expect(result.response?.status).toBe(400); + expect(result.response?.body?.error?.details).toEqual({ runId: 'run_parent', repairable: false }); + }); + + it('FAIL-CLOSED — an inspection that REJECTS still answers the 400 it was asked for, with repairable false and one warning', async () => { + // An unreadable store is UNKNOWN, not "nothing to restore" — so it is + // said out loud once. ⛔ And it never replaces the answer: the caller + // asked about a run that failed, and turning a store outage into a + // 500 would withhold that in order to report a detail. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { dispatcher } = makeDispatcher( + DELEGATION_FRAME, + async () => { throw new Error('connection reset'); }, + ); + + const result = await dispatcher.handleAutomation(RESUME, 'POST', {}, CTX); + + expect(result.response?.status).toBe(400); + expect(result.response?.body?.error?.code).toBe('FLOW_FAILED'); + expect(result.response?.body?.error?.details).toEqual({ runId: 'run_parent', repairable: false }); + const said = warn.mock.calls.map(c => String(c[0])).filter(l => l.includes('inspectConsumedSuspension')); + expect(said).toHaveLength(1); + expect(said[0]).toContain('connection reset'); + expect(said[0]).toContain('run_parent'); + }); + + it('CONTROL — a stamped `stranded` is answered by the STAMP: repairable true, and the engine is never asked', async () => { + const { dispatcher, spies } = makeDispatcher( + { success: false, error: 'tail blew up', durationMs: 12, status: 'stranded' }, + async (runId: string) => ({ repairable: false, runId, reason: 'NO_CONSUMED_SUSPENSION' }), + ); + + const result = await dispatcher.handleAutomation(RESUME, 'POST', {}, CTX); + + // The inspection would have answered `false` here. It is not consulted, + // so the stamp is demonstrably what decided — and the pre-existing + // exits keep costing exactly one engine call. + expect(spies.inspectConsumedSuspension).not.toHaveBeenCalled(); + expect(result.response?.body?.error?.details).toEqual({ + runId: 'run_parent', status: 'stranded', repairable: true, + }); + }); + + it('CONTROL — a stamped `failed` is answered by the STAMP: repairable false, and the engine is never asked', async () => { + const { dispatcher, spies } = makeDispatcher( + { success: false, error: 'rejected', durationMs: 3, status: 'failed' }, + async (runId: string) => ({ repairable: true, runId }), + ); + + const result = await dispatcher.handleAutomation(RESUME, 'POST', {}, CTX); + + // The inspection would have answered `true` here — a stamped `failed` + // is a run that ran and was rejected, and no reading of a leftover + // snapshot is allowed to overturn the producer's own verdict. + expect(spies.inspectConsumedSuspension).not.toHaveBeenCalled(); + expect(result.response?.body?.error?.details).toEqual({ + runId: 'run_parent', status: 'failed', repairable: false, + }); + }); +}); diff --git a/packages/runtime/src/domains/automation.ts b/packages/runtime/src/domains/automation.ts index 803bd86097f..b75d05346b4 100644 --- a/packages/runtime/src/domains/automation.ts +++ b/packages/runtime/src/domains/automation.ts @@ -1385,15 +1385,39 @@ function isTerminalFailureStatus(status: AutomationResult['status']): status is * is neither forwarded under a schema that refuses it nor turned into * anything else. `TERMINAL_FAILURE_STATUSES` is `satisfies`-bound to the * schema's enum, so a member the spec drops reds this file. - * - `repairable` — `status === 'stranded'`, and ALWAYS present on this arm. - * Present-and-false on the plain terminal exit is a deliberate contract, - * not an implementation detail: an ABSENT member would be - * indistinguishable from a server that predates this field, and - * `StrandedDecisionDetails.repairable` (`@objectstack/types`, the approvals - * door's carrier) already fixed the vocabulary — `false` is the honest - * answer for every other exit, including the ones that report no status - * at all, because promising a repair verb that will refuse is worse than - * promising nothing. + * - `repairable` — whether an operator verb can still re-arm this run, and + * ALWAYS present on this arm. It is answered from TWO sources, chosen by + * what the engine stamped, and neither of them is this door's opinion: + * when the engine stamped a `status`, that word decides (`'stranded'` is + * the run whose own pause was consumed and which the restore verb takes); + * when it stamped NONE, the door ASKS the engine + * (`IAutomationService.inspectConsumedSuspension`, #15358) and relays its + * `repairable`. [#17541] The status-less exit is the subflow DELEGATION + * one — a caller resumes the PARENT, the signal is forwarded down, the + * child strands, and the parent frame answers with no `status` because + * nothing re-arms an ancestor by resuming it. Since #15222 that parent's + * consumed pause IS journalled and `restoreConsumedSuspension` re-arms the + * whole chain as one unit, so `status === 'stranded'` answered `false` + * about a run the operator verb WILL repair, and a client written exactly + * as the docs instruct closed it as terminal. ⛔ The fence #15222 was + * dispatched with is untouched: the ancestor is still never STAMPED + * `'stranded'`, because that word names an exit it did not take — its + * repairability is carried by the journal and reported by the inspection. + * Present-and-false is a deliberate contract, not an implementation + * detail: an ABSENT member would be indistinguishable from a server that + * predates this field, and `StrandedDecisionDetails.repairable` + * (`@objectstack/types`, the approvals door's carrier) already fixed the + * vocabulary. So every way of not getting an answer is FAIL-CLOSED here — + * a service that declares no `inspectConsumedSuspension`, and a store the + * inspection could not read — because promising a repair verb that will + * refuse is worse than promising nothing. + * + * ⛔ The door asks a member the CONTRACT declares, never one it merely hopes + * the host has: `inspectConsumedSuspension` is optional on + * `IAutomationService` exactly as `resume` and `restoreConsumedSuspension` + * are, and this arm reads it through that optionality. Reaching for an + * undeclared member is the fail-open shape the `501` arm below exists to + * prevent. * * ⛔ Not reused from `@objectstack/types`: `strandedDecisionDetails` / * `strandedDecisionFailure` are an all-four-or-nothing envelope whose @@ -1409,15 +1433,66 @@ function isTerminalFailureStatus(status: AutomationResult['status']): status is * both doors: the trigger door in `automation-resume-stranded-details.test.ts`, * `/actions` in `actions-flow-dispatch-status.test.ts` (#9585's artefacts pin). */ -function resumeFailureDetails(runId: string, result: AutomationResult): ResumeFailureDetails { +async function resumeFailureDetails( + deps: DomainHandlerDeps, + automationService: IAutomationService, + runId: string, + result: AutomationResult, +): Promise { const status = isTerminalFailureStatus(result.status) ? result.status : undefined; return { runId, - repairable: status === 'stranded', + repairable: status === undefined + ? await consumedSuspensionSurvives(deps, automationService, runId) + : status === 'stranded', ...(status !== undefined ? { status } : {}), }; } +/** + * [#17541] The engine's answer to "can an operator verb still re-arm this + * run?", for the resume exits that stamp no `status` — read through the + * declared optional member `IAutomationService.inspectConsumedSuspension` + * (#15358), which re-arms nothing and reads the same witnesses + * `restoreConsumedSuspension` reads, so what it calls repairable IS what that + * verb restores. + * + * Both ways of failing to get an answer are FAIL-CLOSED, and they are + * different facts kept apart on purpose: + * + * - the service declares no such member — an automation capability with no + * inspection half, which is the shape this door has answered `false` for + * since #15221 and goes on answering `false` for, silently: there is + * nothing to report about a host that simply does not have the member; + * - the inspection REJECTED — a store it could not read. That is UNKNOWN, + * ⛔ not "nothing to restore", so it is said once at `warn` naming what + * the caller is not being told. Functional, not durability: the caller + * gets a visibly smaller answer about a run that did fail, and nothing + * that claimed to persist failed to land. + * + * ⛔ The rejection is never allowed to REPLACE the answer. What reaches this + * helper is a run that consumed its pause and ran, and the `400` describing + * it is the thing the caller asked for; turning a store outage into a `500` + * would withhold that in order to report a detail. + */ +async function consumedSuspensionSurvives( + deps: DomainHandlerDeps, + automationService: IAutomationService, + runId: string, +): Promise { + try { + return (await automationService.inspectConsumedSuspension?.(runId))?.repairable === true; + } catch (e) { + (deps.logger ?? console).warn( + `[Automation] resume door: inspectConsumedSuspension('${runId}') could not be read ` + + `(${e instanceof Error ? e.message : String(e)}), so this 400 reports ` + + '`repairable: false` for a run whose consumed suspension may well survive — ' + + 'ask the operator restore verb itself before treating the run as terminal.', + ); + return false; + } +} + /** * Handles Automation requests * path: sub-path after /automation/ @@ -2248,8 +2323,9 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // "repair waiting" as one and the same 400. The #16472 // ruling (option A) carries it here, in the details of the // EXISTING code: `runId`, `status` (verbatim, when stamped) - // and `repairable` (`status === 'stranded'`, always present — - // false on the plain terminal exit, deliberately, see + // and `repairable` (always present; [#17541] the stamped + // exits are answered by the stamp and the status-LESS ones by + // asking the engine's `inspectConsumedSuspension`, see // `resumeFailureDetails`), declared once as // `ResumeFailureDetailsSchema` in `@objectstack/spec/api`. // ⛔ No `FLOW_STRANDED` sibling code: the console treats @@ -2257,13 +2333,14 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str // wants to branch reads `details.repairable`, never a regex // over the message. if (result?.success === false) { + const verdict = await resumeFailureDetails(deps, automationService, parts[2], result); return { handled: true, response: deps.error(result.error ?? 'Flow run failed', 400, { code: 'FLOW_FAILED', ...(result.errorMessage !== undefined ? { errorMessage: result.errorMessage } : {}), ...(result.summary !== undefined ? { summary: result.summary } : {}), - ...resumeFailureDetails(parts[2], result), + ...verdict, }), }; } diff --git a/packages/spec/src/api/automation-api.zod.ts b/packages/spec/src/api/automation-api.zod.ts index c2abbdd6e13..6d74947a32d 100644 --- a/packages/spec/src/api/automation-api.zod.ts +++ b/packages/spec/src/api/automation-api.zod.ts @@ -454,12 +454,21 @@ export const ResumeFailureDetailsSchema = lazySchema(() => z.object({ ), repairable: z.boolean().describe( 'Whether the engine says this run can still be re-armed by an operator ' - + 'verb - `true` exactly when `status` is `stranded`, derived from the ' - + 'engine\'s discriminator and never from the message text. Always present ' - + 'on this arm: `false` is the honest answer for every other exit, the ones ' - + 'that report no status included, because an absent member would be ' - + 'indistinguishable from a server that predates this field, and promising ' - + 'a repair verb that will refuse is worse than promising nothing', + + 'verb. Answered from the engine, two ways, and never from the message ' + + 'text: where the engine stamped a `status`, that word decides ' + + '(`stranded` is the run whose OWN pause a resume consumed before a ' + + 'downstream node threw); where it stamped none, the door asks the ' + + 'engine\'s read-only inspection ' + + '(`IAutomationService.inspectConsumedSuspension`) and relays its ' + + 'verdict. The second half is not a fallback but the honest answer for a ' + + 'real exit: a subflow DELEGATION failure carries no status, because ' + + 'nothing re-arms an ancestor by resuming it, and yet the ancestor\'s ' + + 'consumed pause is journalled and the restore verb re-arms that chain as ' + + 'one unit. Always present on this arm: an absent member would be ' + + 'indistinguishable from a server that predates this field. Every way of ' + + 'not getting an answer is fail-closed - a service that declares no ' + + 'inspection member, and a store the inspection could not read - because ' + + 'promising a repair verb that will refuse is worse than promising nothing', ), })); export type ResumeFailureDetails = z.input; diff --git a/packages/spec/src/contracts/automation-service.ts b/packages/spec/src/contracts/automation-service.ts index 8d9231d401b..a551cf4f6da 100644 --- a/packages/spec/src/contracts/automation-service.ts +++ b/packages/spec/src/contracts/automation-service.ts @@ -906,4 +906,82 @@ export interface IAutomationService { /** One sentence naming what was observed — always present, both ways. */ reason: string; }>; + + /** + * **The READ-ONLY half of the operator exit** (#15358; ruled B′, + * 2026-09-07): would {@link restoreConsumedSuspension} have a consumed + * suspension to put back for this run? It re-arms nothing, moves nothing + * and records nothing — asking is free, and asking twice answers twice. + * + * It is read from the SAME witnesses the restore verb reads, which is the + * whole reason a door may act on it: what this member calls repairable IS + * what that verb restores. ⚠️ It judges what SURVIVES, never whether + * re-arming would be safe at this instant — the restore verb keeps its own + * pre-flight. + * + * **Why a door needs it, and no status stamp will do.** The resume door's + * `400 FLOW_FAILED` details carry `repairable` + * (`ResumeFailureDetailsSchema`, `api/automation-api.zod.ts`). On the exit + * that consumed its OWN pause and then threw, `AutomationResult.status` is + * `'stranded'` and that word answers the question by itself. On the subflow + * DELEGATION exit it cannot: a caller resumes the PARENT, the signal is + * forwarded down, the child strands, and the parent frame answers with no + * `status` at all — deliberately, because nothing re-arms an ancestor by + * resuming it and stamping `'stranded'` there would send an operator to + * retry a recovery that cannot succeed. Since #15222 that parent's consumed + * pause IS journalled and the restore verb re-arms the whole chain as one + * unit, so the run is repairable while carrying no word that says so. The + * door asks THIS member for those exits instead of inferring an answer + * from the absence of a stamp. + * + * ⛔ **Asked per run; never inferred from a run's position in a subflow + * tree.** A cascade-failed ancestor is journalled exactly when the + * descendant whose failure consumed its pause is itself repairable, so + * "is an ancestor" predicts nothing. + * + * **The result is deliberately NARROWER than the implementation's**, for + * the reason {@link restoreConsumedSuspension}'s is (#16495, route (i)): + * the engine answers its own wider type — the pause a restore would + * re-arm, which of the two witnesses answered, when the suspension was + * consumed, and a closed refusal vocabulary whose members a consumer that + * branches on `repairable` alone reads wrong in both directions. That type + * lives with the engine and is not moved here; `reason` is typed as the + * string the implementation answers, not as an enumeration this contract + * would have to keep in step, and the wider type satisfies this one under + * `implements`. A consumer that needs the vocabulary itself is a spec + * card, never a widening at a call site. + * + * **Optional, deliberately**, and its absence is FAIL-CLOSED for a door: + * a service that does not declare this member cannot be asked, and a door + * that cannot ask answers `repairable: false` — ⛔ never `true`, because + * promising a repair verb that will refuse is worse than promising + * nothing, and ⛔ never by dropping the member, which would be + * indistinguishable from a server that predates the field. Absence of the + * discriminator is not evidence of anything. + * + * @param runId - The run to ask about (its `sys_automation_run` row) + * @returns `repairable: true` when a consumed-suspension snapshot is held + * for the run and the restore verb would put it back; otherwise + * `repairable: false` with `reason` naming what the implementation + * observed. `runId` is echoed both ways. + * @throws when a store cannot be read. An unreadable store is UNKNOWN, + * ⛔ not "nothing to restore" — a verdict that turned an outage into an + * absence would send an operator to give up on a run that is repairable. + * A door that must still answer treats the rejection the way it treats + * absence, fail-closed, and says so where it does it. + */ + inspectConsumedSuspension?(runId: string): Promise<{ + /** + * `true` exactly when a consumed suspension survives for this run and + * {@link restoreConsumedSuspension} would put it back. + */ + repairable: boolean; + /** The run id, echoed. */ + runId: string; + /** + * What the implementation observed when it answers `false` (its own + * closed vocabulary — see above); absent on the `true` arm. + */ + reason?: string; + }>; } diff --git a/packages/verify/src/automation-resume-delegation-repairable.test.ts b/packages/verify/src/automation-resume-delegation-repairable.test.ts new file mode 100644 index 00000000000..9b73f43ad5e --- /dev/null +++ b/packages/verify/src/automation-resume-delegation-repairable.test.ts @@ -0,0 +1,224 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #17541 — the subflow DELEGATION exit tells the truth about repair ON THE + * WIRE, driven through the real engine and the real route. + * + * ## The reading this file exists for + * + * `POST /automation/parent_flow/runs/:parentRunId/resume` on a parked + * delegation whose child then throws answered, before this card: + * + * ```json + * 400 { "success": false, "error": { "code": "FLOW_FAILED", + * "message": "subflow run 'run_…' (child_flow) failed: …", + * "details": { "runId": "run_…", "repairable": false } } } + * ``` + * + * …while the very same parent run was, at that same instant, + * `inspectConsumedSuspension → { repairable: true, witness: 'journal' }` and + * `restoreConsumedSuspension → { restored: true, chain: [child, parent] }`. + * The wire told an operator not to retry a repair that works. + * + * ## Why the delegation exit carries no status, and why that is right + * + * A caller resumes the PARENT; `resumeInternal` forwards the signal down; the + * child strands with `skipBubble`; the parent frame sees a failed child with + * no retryable code and fails itself. It stamps NOTHING, deliberately — + * nothing re-arms an ancestor by resuming it, so stamping `'stranded'` there + * would send an operator to retry a recovery that cannot succeed (the fence + * #15222 was dispatched with, and it is untouched by this card). Since #15222 + * the ancestor's consumed pause is journalled anyway, and ONE restore re-arms + * the whole chain leaf-first. So the run is repairable while carrying no word + * that says so — which is why the door must ASK the engine + * (`IAutomationService.inspectConsumedSuspension`, #15358) rather than read a + * stamp that is not there. + * + * ## The controls + * + * Two, and each fires: + * + * - the **non-delegation** stranded exit — resume the CHILD directly — still + * answers `status: 'stranded'` with `repairable: true`, byte for byte what + * it answered before this card. The change is confined to the arm that + * reports no status. + * - a delegation whose leaf is **not** repairable answers `repairable: + * false` on that same status-less arm. Without it this file would pass + * over a door that simply flipped the arm to `true` and asked nothing. + * + * Engine-level halves, not re-pinned here: + * `service-automation`'s `nested-strand-chain-restore.test.ts` (the chain) and + * `consumed-suspension-inspection.test.ts` (the read-only verdict). The + * door's own shaping, arm by arm with a fake service: + * `packages/runtime`'s `automation-resume-delegation-repairable.test.ts`. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; + +import { HttpDispatcher } from '@objectstack/runtime'; +import { + AutomationEngine, + InMemorySuspendedRunStore, + installBuiltinNodes, +} from '@objectstack/service-automation'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import { ResumeFailureDetailsSchema } from '@objectstack/spec/api'; + +const CTX = { request: {}, executionContext: { userId: 'user_1' } } as never; + +const DOWNSTREAM_FAILURE = 'update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found'; + +function createTestLogger(): never { + const logger = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => logger }; + return logger as never; +} + +/** Leaf: parks at `pauser`, then runs a node that can be made to throw. */ +const CHILD = { + name: 'child_flow', label: 'Child', type: 'autolaunched', + nodes: [ + { id: 'cstart', type: 'start', label: 'Start' }, + { id: 'park', type: 'pauser', label: 'Park' }, + { id: 'cwrite', type: 'cmark', label: 'Write' }, + { id: 'cend', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'c1', source: 'cstart', target: 'park' }, + { id: 'c2', source: 'park', target: 'cwrite' }, + { id: 'c3', source: 'cwrite', target: 'cend' }, + ], +}; + +/** Parent: parks at its `subflow` node while the child is suspended. */ +const PARENT = { + name: 'parent_flow', label: 'Parent', type: 'autolaunched', + nodes: [ + { id: 'pstart', type: 'start', label: 'Start' }, + { id: 'sub', type: 'subflow', label: 'Sub', config: { flowName: 'child_flow' } }, + { id: 'after', type: 'pmark', label: 'After' }, + { id: 'pend', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'p1', source: 'pstart', target: 'sub' }, + { id: 'p2', source: 'sub', target: 'after' }, + { id: 'p3', source: 'after', target: 'pend' }, + ], +}; + +describe('#17541 — the delegation exit answers repairable from the engine, on the wire', () => { + let engine: AutomationEngine; + let dispatcher: HttpDispatcher; + let childThrows: string | undefined; + + beforeEach(() => { + childThrows = undefined; + const logger = createTestLogger(); + engine = new AutomationEngine(logger, new InMemorySuspendedRunStore()); + installBuiltinNodes(engine, { logger, getService() { throw new Error('none'); } } as never); + engine.registerNodeExecutor({ + type: 'pauser', + descriptor: defineActionDescriptor({ + type: 'pauser', version: '1.0.0', name: 'pauser', + supportsPause: true, resumeAuthority: 'any', + }), + async execute() { return { success: true, suspend: true }; }, + } as never); + engine.registerNodeExecutor({ + type: 'cmark', + async execute() { + if (childThrows) throw new Error(childThrows); + return { success: true }; + }, + } as never); + engine.registerNodeExecutor({ type: 'pmark', async execute() { return { success: true }; } } as never); + engine.registerFlow('child_flow', CHILD as never); + engine.registerFlow('parent_flow', PARENT as never); + + const services: Record = { automation: engine }; + const resolve = (name: string): unknown => services[name]; + const kernel = { + getService: resolve, + getServiceAsync: async (name: string): Promise => resolve(name), + context: { getService: resolve }, + }; + dispatcher = new HttpDispatcher(kernel as never); + }); + + /** Park the two-level tree through the trigger door; hand back both ids. */ + async function park(): Promise<{ parentRunId: string; childRunId: string }> { + const started = await dispatcher.handleAutomation('/parent_flow/trigger', 'POST', {}, CTX); + expect(started.response?.status).toBe(200); + expect(started.response?.body?.data?.status, 'the parent parks awaiting its child').toBe('paused'); + const parentRunId = started.response?.body?.data?.runId as string; + const childRunId = engine.listSuspendedRuns().find(r => r.flowName === 'child_flow')!.runId; + expect(typeof parentRunId).toBe('string'); + expect(typeof childRunId).toBe('string'); + return { parentRunId, childRunId }; + } + + it('DELEGATION — the 400 answers repairable: true with NO status, and the operator verb accepts exactly that run', async () => { + const { parentRunId, childRunId } = await park(); + childThrows = DOWNSTREAM_FAILURE; + + const res = await dispatcher.handleAutomation(`/parent_flow/runs/${parentRunId}/resume`, 'POST', {}, CTX); + + expect(res.response?.status).toBe(400); + const error = res.response?.body?.error; + expect(error?.code).toBe('FLOW_FAILED'); + // The verdict, on the wire. ⛔ Still no `status` — the producer stamped + // none and the door does not invent one; `repairable` is the answer. + expect(error?.details).toEqual({ runId: parentRunId, repairable: true }); + expect(error?.details?.status).toBeUndefined(); + expect(ResumeFailureDetailsSchema.safeParse(error?.details).success).toBe(true); + // The message names the failure, never the verdict — no regex client. + expect(error?.message).toContain(DOWNSTREAM_FAILURE); + expect(error?.message).not.toMatch(/repairable/i); + + // ONE FACT, STATED TWICE: `repairable: true` on the wire ⇔ the operator + // verb accepts THIS run. A wire that promised a repair the verb refuses + // would be worse than the under-report it replaces. + expect(await engine.hasSuspendedRun(parentRunId), 'the delegation consumed the parent pause').toBe(false); + const restored = await engine.restoreConsumedSuspension(parentRunId, { requestedBy: 'ops' }); + expect(restored.restored).toBe(true); + expect(restored.chain?.map(m => m.runId), 'leaf-first, the whole chain').toEqual([childRunId, parentRunId]); + + // …and the repair the wire pointed at really completes the tree. + childThrows = undefined; + const again = await dispatcher.handleAutomation(`/parent_flow/runs/${parentRunId}/resume`, 'POST', {}, CTX); + expect(again.response?.status).toBe(200); + expect((await engine.getRun(parentRunId))?.status).toBe('completed'); + }); + + it('CONTROL — the non-delegation stranded exit is unchanged: status stranded, repairable true', async () => { + // The up-bubble path: the caller resumes the CHILD, whose own pause was + // consumed, so the engine stamps `'stranded'` and the stamp answers. + const { childRunId } = await park(); + childThrows = DOWNSTREAM_FAILURE; + + const res = await dispatcher.handleAutomation(`/child_flow/runs/${childRunId}/resume`, 'POST', {}, CTX); + + expect(res.response?.status).toBe(400); + expect(res.response?.body?.error?.code).toBe('FLOW_FAILED'); + expect(res.response?.body?.error?.details).toMatchObject({ + runId: childRunId, status: 'stranded', repairable: true, + }); + }); + + it('FIRING CONTROL — a delegation whose leaf is NOT repairable answers repairable: false on the same status-less arm', async () => { + // The child's flow is gone, so its resume is the engine's terminal + // "this pause is gone for good" class: nothing is journalled for the + // ancestor, and the restore verb has nothing to put back. The door + // must relay THAT — it is the same arm, and it answers `false`. + const { parentRunId } = await park(); + engine.unregisterFlow('child_flow'); + + const res = await dispatcher.handleAutomation(`/parent_flow/runs/${parentRunId}/resume`, 'POST', {}, CTX); + + expect(res.response?.status).toBe(400); + expect(res.response?.body?.error?.details).toEqual({ runId: parentRunId, repairable: false }); + expect(res.response?.body?.error?.details?.status).toBeUndefined(); + const refused = await engine.restoreConsumedSuspension(parentRunId); + expect(refused.restored, 'and the wire agreed with the verb').toBe(false); + expect(refused.refusal).toBe('NO_CONSUMED_SUSPENSION'); + }); +});