diff --git a/.changeset/subflow-bubble-stranded-parent.md b/.changeset/subflow-bubble-stranded-parent.md new file mode 100644 index 0000000000..dc14066cb6 --- /dev/null +++ b/.changeset/subflow-bubble-stranded-parent.md @@ -0,0 +1,35 @@ +--- +'@objectstack/service-automation': patch +--- + +automation: a subflow parent left STRANDED by a failed up-bubble is reported at `error`, not `warn` + +When an approval (or any pause) sits inside a subflow child, resuming the child +bubbles up to the parent. If the parent's own continuation then fails on the +engine's stranded exit — its suspension consumed, a repair snapshot journalled, +the run recorded `failed` — nothing but a `warn` said so, while the child's +resumer (an approvals decision door, a wait timer) was told the resume +succeeded. Persisted state and runtime state disagree and nothing looks broken +from the outside, which is the durability class. + +`bubbleToParent` now grades that record by the engine's own +`AutomationResult.status` discriminator: `'stranded'` is reported at `error`, +naming the parent run and the `restoreConsumedSuspension` verb that repairs it. +Every other parent-resume failure — a concurrent resume, an unreachable store, +a thrown resume — stays at `warn` unchanged, on a narrower ground: those exits +carry no `'stranded'` discriminator. `'stranded'` is the one exit that journals +a repair snapshot, so it is the one an operator can act on, and grading by the +engine's own verdict is what keeps `error` readable. + +⚠️ That is a statement about what this seam can KNOW, not a guarantee that +every other exit left the parent healthy. Two exits are known not to be: + +- a **thrown** parent resume carries no discriminator at all, and #15555 + documents a window in which a throw between the journal and the stamp hides a + parent that IS stranded. Left at `warn` deliberately, for that card; +- the **claim-path** store failure reports, in its own envelope text, that + whether the suspension was consumed is UNKNOWN — it relies on a retry to + settle it, and an up-bubble has no retrier. ("Not consumed" is the guarantee + of the strict-load store failure only, not of every store failure.) + +⚠️ This is the log half only. What the child's resumer is told is unchanged. diff --git a/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts b/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts new file mode 100644 index 0000000000..3a5c751fa6 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts @@ -0,0 +1,321 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15556 — the REPRODUCTION: an approval hosted inside a SUBFLOW CHILD, whose + * parent's continuation fails. + * + * The card was filed NOT MEASURED — the seam and its swallowing `catch` were + * found by reading `engine.ts`, and nobody had driven the composition. This + * file is that drive, with its controls in the same run, and it reproduces. + * + * ## The composition + * + * `deal_parent` parks at a `subflow` node; the child `deal_approval` parks at + * an `approval` node, so the approvals row names the CHILD run. The decision + * door resumes the child, the child completes, `bubbleToParent` resumes the + * parent, and the parent's own downstream node throws. + * + * ## What is measured, and what is only characterised + * + * MEASURED FACT, now fixed engine-side: the parent lands on the engine's + * stranded exit — `{ success: false, status: 'stranded' }`, journalled and + * repairable — and `bubbleToParent` logged that at `warn`. The level is now + * graded by that discriminator (`subflow-bubble-strand-log-level.test.ts` in + * `service-automation` holds the pins, both directions). + * + * ⚠️ CHARACTERISED, NOT BLESSED: the decision door still answers full success. + * Its resume-facing answer is IDENTICAL to the one a healthy composition + * produces, so no caller can tell the two apart, and the `runId` it hands back + * names the CHILD — which completed — never the stranded parent. Making that + * truthful moves a public contract (`AutomationResult`, + * `ApprovalDecisionResult`) and is #15556's open decision, the sibling one + * level up of the #13807 ruling (maintainer 2026-09-04, decision batch #37). + * ⛔ The assertions below record what the door does TODAY; whatever ruling + * lands must turn them red on purpose. + * + * ## The control that makes the reading trustworthy + * + * `CONTROL` drives the #13807 shape through the SAME door in the same run — no + * subflow, the child's own branch throws — and the door throws `RESUME_FAILED` + * with its stranded envelope. So the absence of a throw above is a fact about + * the composition, not about a mis-wired harness. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AutomationEngine, InMemorySuspendedRunStore, installBuiltinNodes } from '@objectstack/service-automation'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { strandedDecisionDetails } from '@objectstack/types'; +import { ApprovalService } from './approval-service.js'; +import { registerApprovalNode } from './approval-node.js'; + +const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as any; + +/** The card's own downstream failure text, in shape. */ +const DOWNSTREAM_FAILURE = 'update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found'; + +/** + * The resume-facing answer a caller reads, minus the run id. Asserted by BOTH + * the stranded composition and the healthy one — that shared literal, and not + * a value smuggled between tests, is what carries the claim that the two are + * indistinguishable at the door. + */ +const FULL_SUCCESS = { finalized: true, decision: 'approve', resumed: true, resumeError: undefined }; + +/** Records every level so the "only artefact is a log line" claim is measurable. */ +function recordingLogger() { + const lines: Array<{ level: string; msg: string; meta?: unknown }> = []; + const mk = (level: string) => (msg: string, meta?: unknown) => { lines.push({ level, msg, meta }); }; + const self: any = { + lines, + info: mk('info'), warn: mk('warn'), error: mk('error'), debug: mk('debug'), + child() { return self; }, + }; + return self; +} + +function makeFakeEngine() { + const tables = new Map(); + const rows = (o: string) => (tables.get(o) ?? (tables.set(o, []), tables.get(o)!)); + const matches = (row: any, where: any) => Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake engine: unsupported filter operator ${k}`); + if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(row[k]); + if (v && typeof v === 'object' && '$ne' in (v as any)) return row[k] !== (v as any).$ne; + return row[k] === v; + }); + return { + tables, + async find(object: string, opts: any = {}) { + const where = opts.where ?? opts.filter ?? {}; + const out = rows(object).filter(r => matches(r, where)); + const start = opts.offset ?? 0; + const page = typeof opts.limit === 'number' ? out.slice(start, start + opts.limit) : out.slice(start); + return page.map(r => ({ ...r })); + }, + async insert(object: string, data: any) { rows(object).push({ ...data }); return { ...data }; }, + async update(object: string, data: any, options?: any) { + const dispatch = assertEngineUpdateDispatch(data, options); + const table = rows(object); + if (dispatch.kind === 'multi') { + let n = 0; + for (let i = 0; i < table.length; i++) { + if (matches(table[i], options?.where)) { table[i] = { ...table[i], ...data }; n++; } + } + return { updated: n }; + } + const i = table.findIndex(r => r.id === dispatch.id); + if (i >= 0) table[i] = { ...table[i], ...data }; + return i >= 0 ? { ...table[i] } : null; + }, + async delete(object: string, options?: any) { + const dispatch = assertEngineDeleteDispatch(options); + const table = rows(object); + if (dispatch.kind === 'multi') { + const survivors = table.filter(r => !matches(r, options?.where)); + const deleted = table.length - survivors.length; + table.splice(0, table.length, ...survivors); + return { deleted }; + } + const i = table.findIndex(r => r.id === dispatch.id); + if (i >= 0) table.splice(i, 1); + return { id: dispatch.id }; + }, + }; +} + +/** The CHILD: an approval node, exactly the #13807 fixture. */ +const CHILD = { + name: 'deal_approval', + label: 'Deal Approval', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'approve_step', type: 'approval', label: 'Manager Approval', config: { approvers: [{ type: 'user', value: 'u1' }] } }, + { id: 'on_approved', type: 'mark', label: 'Approved' }, + { id: 'mark_rejected', type: 'mark', label: 'Rejected' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'approve_step' }, + { id: 'e2', source: 'approve_step', target: 'on_approved', label: 'approve' }, + { id: 'e3', source: 'approve_step', target: 'mark_rejected', label: 'reject' }, + { id: 'e4', source: 'on_approved', target: 'end' }, + { id: 'e5', source: 'mark_rejected', target: 'end' }, + ], +}; + +/** The PARENT: hosts the child in a `subflow` node, then does more work. */ +const PARENT = { + name: 'deal_parent', + label: 'Deal Parent', + type: 'autolaunched', + nodes: [ + { id: 'pstart', type: 'start', label: 'Start' }, + { id: 'sub', type: 'subflow', label: 'Run the approval subflow', config: { flowName: 'deal_approval', outputVariable: 'subOut' } }, + { id: 'after_sub', type: 'mark', label: 'After the subflow' }, + { id: 'pend', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'p1', source: 'pstart', target: 'sub' }, + { id: 'p2', source: 'sub', target: 'after_sub' }, + { id: 'p3', source: 'after_sub', target: 'pend' }, + ], +}; + +describe('#15556 — an approval hosted in a subflow child, whose parent bubble fails', () => { + let data: ReturnType; + let service: ApprovalService; + let logger: ReturnType; + let marks: string[]; + let throwOn: Record; + + beforeEach(() => { + marks = []; + throwOn = {}; + logger = recordingLogger(); + data = makeFakeEngine(); + service = new ApprovalService({ engine: data as any, logger }); + }); + + function boot() { + const automation = new AutomationEngine(logger, new InMemorySuspendedRunStore()); + installBuiltinNodes(automation, { logger, getService() { throw new Error('none'); } } as any); + registerApprovalNode(automation, service, logger); + automation.registerNodeExecutor({ + type: 'mark', + async execute(node: any) { + const boom = throwOn[node.id]; + if (boom) throw new Error(boom); + marks.push(node.id); + return { success: true }; + }, + } as never); + automation.registerFlow('deal_approval', CHILD as never); + automation.registerFlow('deal_parent', PARENT as never); + service.attachAutomation(automation); + return automation; + } + + const pendingRequest = async () => + (await data.find('sys_approval_request', { where: { status: 'pending' } }))[0]; + + /** The resume-facing projection of the door's answer — everything a caller reads about the run. */ + const resumeFacing = (r: any) => ({ + finalized: r.finalized, decision: r.decision, resumed: r.resumed, resumeError: r.resumeError, + }); + + it('the parent STRANDS while the door answers full success', async () => { + throwOn.after_sub = DOWNSTREAM_FAILURE; + const automation = boot(); + + const started = await automation.execute('deal_parent', { + object: 'crm_deal', record: { id: 'd1', amount: 100 }, userId: 'submitter', + } as never); + const parentRunId = (started as any).runId as string; + expect((started as any).status, 'the parent parks at its subflow node').toBe('paused'); + + const req = await pendingRequest(); + const childRunId = req?.flow_run_id as string; + expect(childRunId, 'the request names the CHILD run, never the parent').toBeTruthy(); + expect(childRunId).not.toBe(parentRunId); + expect(await automation.hasSuspendedRun(parentRunId)).toBe(true); + + // The envelope `bubbleToParent` receives for the PARENT — the thing this + // card is about. Captured through `resumeInternal` because the up-bubble + // never goes through the public `resume` door. + const bubbled: any[] = []; + const realInternal = (automation as any).resumeInternal.bind(automation); + (automation as any).resumeInternal = async (...args: any[]) => { + const r = await realInternal(...args); + if (args[0] === parentRunId) bubbled.push(r); + return r; + }; + + const outcome = await service + .decide(req.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX) + .then(r => ({ ok: true as const, r }), (e: Error) => ({ ok: false as const, e })); + + // ── The parent's resume answers the #13937 discriminator, and no `code`. + expect(bubbled.length).toBe(1); + expect(bubbled[0].success).toBe(false); + expect(bubbled[0].status, "the producer's own verdict").toBe('stranded'); + expect(bubbled[0].code, 'and it names no code at all').toBeUndefined(); + expect(bubbled[0].error).toBe(DOWNSTREAM_FAILURE); + + // ── The parent really is dead, and really is repairable. + expect(await automation.hasSuspendedRun(parentRunId)).toBe(false); + expect((await automation.resume(parentRunId)).code).toBe('RUN_NOT_FOUND'); + expect((await automation.getRun(parentRunId))?.status).toBe('failed'); + + // ── The child is fine, and so is the decision: both halves of the + // divergence are real, which is what makes it a divergence. + expect((await automation.getRun(childRunId))?.status).toBe('completed'); + expect(marks, 'the child advanced; the parent died on the node after the subflow') + .toEqual(['on_approved']); + expect((await data.find('sys_approval_request', { where: { id: req.id } }))[0].status).toBe('approved'); + + // ⚠️ CHARACTERISED, NOT BLESSED — see the file header. The door does not + // throw, reports `resumed: true`, carries no `resumeError`, and the run it + // names is the CHILD, which completed. Nothing in the response reaches the + // stranded parent. + expect(outcome.ok, 'today the door does not throw').toBe(true); + const answer = outcome.ok ? outcome.r : (undefined as never); + expect(resumeFacing(answer)).toEqual(FULL_SUCCESS); + expect(answer.runId, 'the id handed back is the CHILD — the run that is fine').toBe(childRunId); + expect(strandedDecisionDetails(answer as unknown)).toBeUndefined(); + + // The one artefact the operator gets, at the level AGENTS.md's durability + // rule requires, naming the run and the repair verb (#15556's shipped half). + const durability = logger.lines.filter( + (l: any) => l.level === 'error' && String(l.msg).includes('STRANDED'), + ); + expect(durability.length, 'exactly one, at `error`').toBe(1); + expect(durability[0].msg).toContain(`restoreConsumedSuspension('${parentRunId}')`); + + // …and the repair verb it promises actually works. + expect((await automation.restoreConsumedSuspension(parentRunId, { requestedBy: 'ops' })).restored).toBe(true); + + }); + + it('CONTROL A — the healthy composition answers IDENTICALLY, which is the defect', async () => { + const automation = boot(); + const started = await automation.execute('deal_parent', { + object: 'crm_deal', record: { id: 'd1', amount: 100 }, userId: 'submitter', + } as never); + const parentRunId = (started as any).runId as string; + const req = await pendingRequest(); + + const answer = await service.decide(req.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX); + + // The parent ran to completion this time — the only thing that changed. + expect(marks).toEqual(['on_approved', 'after_sub']); + expect((await automation.getRun(parentRunId))?.status).toBe('completed'); + expect(logger.lines.filter((l: any) => l.level === 'error')).toEqual([]); + + // ⭐ The sharpest statement of the defect: the SAME literal the stranded + // composition asserted. A caller comparing the two answers has nothing to + // compare — only the run ids differ, and both name a healthy child. + expect(resumeFacing(answer)).toEqual(FULL_SUCCESS); + expect(answer.runId).toBe(req.flow_run_id); + }); + + it("CONTROL B — the #13807 shape still throws at this door, so the harness is live", async () => { + // Without this control, "the door did not throw" above would be + // indistinguishable from a door that was never wired to throw at all. + throwOn.on_approved = 'the child branch blew up'; + const automation = boot(); + await automation.execute('deal_approval', { + object: 'crm_deal', record: { id: 'd3', amount: 100 }, userId: 'submitter', + } as never); + const req = await pendingRequest(); + + const err = await service + .decide(req.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX) + .then(() => null, (e: Error) => e); + + expect(err, 'the direct shape is reported — this door can fail').toBeTruthy(); + expect(err?.message).toMatch(/^RESUME_FAILED/); + expect(strandedDecisionDetails(err)).toEqual({ + finalized: true, decision: 'approve', runId: req.flow_run_id, repairable: true, + }); + }); +}); diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 3a44f8da78..102faf78f2 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -5740,16 +5740,79 @@ export class AutomationEngine implements IAutomationService { // `forgetSuspendedRun`'s catch above for the full mechanism // (#6299). // - // #4632 verdict: FUNCTIONAL — stays `warn`: no false success - // is recorded anywhere — the parent either failed terminally - // (recorded in run history) or stays visibly parked and - // resumable — and the child's own completion, which is what - // its resumer was told, is genuine. - this.logger.warn( - `[automation] subflow run '${run.runId}' completed but resuming parent '${parentRunId}' ` + - `failed — the parent's failure envelope is in this record's meta.`, - { error: parentRes.error ?? 'unknown error' }, - ); + // [#15556] The #4632 verdict is taken PER OUTCOME here, graded + // by the engine's own discriminator and never by this seam's + // guess at what went wrong upstream. + // + // The old verdict was FUNCTIONAL for the whole arm, on this + // enumeration: "the parent either failed terminally (recorded + // in run history) or stays visibly parked and resumable". A + // reproduction of the composition the enumeration never + // covered — a parent parked at a `subflow` node whose child + // hosts an approval, resumed by the approvals decision door — + // measured a THIRD outcome: `resumeInternal` answers + // `{ success: false, status: 'stranded' }` (and no `code`), + // because the parent consumed its suspension and then threw + // downstream. That exit journals a repair snapshot and records + // the parent `failed`, so the parent is neither parked nor + // merely failed: nothing in the engine will ever move it again + // and only {@link restoreConsumedSuspension} can re-arm it. + // + // That is AGENTS.md's DURABILITY class verbatim — persisted + // state and runtime state disagree and nothing looks broken + // from the outside: the approval row is durably terminal, the + // child's own resume genuinely succeeded, and the decision + // door therefore answered its caller success. ⛔ And the rule's + // third legal answer (a failure handed to the CALLER is not a + // degradation) does NOT apply: measured, no caller is told. + // + // ⛔ Deliberately NOT the whole arm. `RESUME_IN_PROGRESS` (a + // replica is already advancing the parent) and + // `STORE_UNAVAILABLE` (the parent's suspension was not + // consumed, so it stays parked and the identical resume works + // once the store recovers) are exactly the functional cases + // the old verdict was right about, and escalating those is how + // `error` becomes unreadable. `status === 'stranded'` is the + // ONE exit that journalled a snapshot, so it is the one an + // operator can and must act on. + // + // ⚠️ This is the LOG half only. What the child's resumer — and + // through it the approvals decision door — is TOLD is + // unchanged and still reads as full success; making that + // truthful moves a public contract (`AutomationResult`, + // `ApprovalDecisionResult`) and is #15556's open decision, the + // sibling one level up of the #13807 ruling (2026-09-04, + // decision batch #37). ⛔ Not decided here. + if (parentRes.status === 'stranded') { + // THIRD argument per the `Logger` contract + // (`error(message, error?, meta?)`); the `Error` slot stays + // empty on purpose (#5575). The message owes the two things + // AGENTS.md's durability rule asks of an `error`: the + // CONSEQUENCE, concretely, and the FIX. + this.logger.error( + `[automation] subflow run '${run.runId}' completed, but its parent run ` + + `'${parentRunId}' is STRANDED — the parent consumed its suspension and then failed ` + + `downstream, so no resume, timer or restart will move it again, while this child's ` + + `resumer (an approvals decision door, a wait timer) was told the resume SUCCEEDED and ` + + `nothing else reports the parent. Repair it with ` + + `restoreConsumedSuspension('${parentRunId}') and re-issue the continuation. The ` + + `parent's failure envelope is in this record's meta.`, + undefined, + { error: parentRes.error ?? 'unknown error', parentRunId, status: parentRes.status }, + ); + } else { + // #4632 verdict: FUNCTIONAL — stays `warn`, unchanged: on + // every other exit nothing claimed-persisted fails to land + // — the parent either failed terminally (recorded in run + // history) or stays visibly parked and resumable — and the + // child's own completion, which is what its resumer was + // told, is genuine. + this.logger.warn( + `[automation] subflow run '${run.runId}' completed but resuming parent '${parentRunId}' ` + + `failed — the parent's failure envelope is in this record's meta.`, + { error: parentRes.error ?? 'unknown error' }, + ); + } } } catch (err) { // #6499 — thrown text to the structured slot; see diff --git a/packages/services/service-automation/src/subflow-bubble-strand-log-level.test.ts b/packages/services/service-automation/src/subflow-bubble-strand-log-level.test.ts new file mode 100644 index 0000000000..45a1b16e7d --- /dev/null +++ b/packages/services/service-automation/src/subflow-bubble-strand-log-level.test.ts @@ -0,0 +1,231 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15556 — `bubbleToParent`'s #4632 verdict, graded per outcome. + * + * ## What was measured + * + * A parent flow parked at a `subflow` node whose child hosts an approval. The + * child's resume completes, `bubbleToParent` resumes the parent, and the + * parent's own continuation throws downstream. `resumeInternal` answers that + * exit with `{ success: false, status: 'stranded' }` and **no `code`** — the + * #13937 discriminator, stamped on the one exit that journals a repair + * snapshot. `bubbleToParent` received it and logged it at `warn`. + * + * The old verdict's enumeration was "the parent either failed terminally + * (recorded in run history) or stays visibly parked and resumable". A stranded + * parent is neither: nothing in the engine moves it again and only + * `restoreConsumedSuspension` can re-arm it, while the approval row is durably + * terminal and the child's resumer was told the resume succeeded. That is + * AGENTS.md's DURABILITY class — persisted state and runtime state disagree, + * nothing looks broken from the outside — and the rule's third legal answer + * (a failure handed to the CALLER) does not apply, because measurably no + * caller is told. + * + * ## What these pins hold + * + * The grading, in BOTH directions, in one run: + * + * - a stranded parent → `error`, naming the run and the repair verb; + * - a parent failure the engine does NOT call stranded → `warn`, unchanged — + * `RESUME_IN_PROGRESS` / `STORE_UNAVAILABLE` are the functional cases the + * old verdict was right about, and escalating them is how `error` becomes + * unreadable; + * - a THROWN parent resume → `warn`, unchanged. + * + * The last two are the reverse controls: without them a pin that only watched + * the stranded case would stay green if the whole arm were escalated. The + * sibling pins for the same two seams live in `engine-residual-log-cause.test.ts` + * (sites 12 and 13), which is where the #6499 message-shape half is held. + * + * ⚠️ This is the LOG half only. What the child's resumer is TOLD is unchanged + * and still reads as full success; see `#15556` for that open decision. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { AutomationEngine } from './engine.js'; +import type { NodeExecutor } from './engine.js'; +import { InMemorySuspendedRunStore } from './suspended-run-store.js'; +import { registerSubflowNode } from './builtin/subflow-node.js'; +import { defineActionDescriptor } from '@objectstack/spec/automation'; +import type { AutomationContext } from '@objectstack/spec/contracts'; + +/** Every argument of every call, so the `error(message, error?, meta?)` slot layout is itself under test (#5575). */ +type Line = { level: string; args: unknown[] }; +function recordingLogger() { + const lines: Line[] = []; + const mk = (level: string) => (...args: unknown[]) => { lines.push({ level, args }); }; + const self: any = { + lines, + info: mk('info'), warn: mk('warn'), error: mk('error'), debug: mk('debug'), + child() { return self; }, + }; + return self as { lines: Line[] } & Record; +} + +const CHILD = { + name: 'child_flow', + label: 'Child', + type: 'autolaunched', + nodes: [ + { id: 'cstart', type: 'start', label: 'Start' }, + { id: 'park', type: 'pauser', label: 'Park' }, + { id: 'cend', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'c1', source: 'cstart', target: 'park' }, + { id: 'c2', source: 'park', target: 'cend' }, + ], +}; + +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: 'mark', 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' }, + ], +}; + +const DOWNSTREAM_FAILURE = 'update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found'; + +describe('#15556 — bubbleToParent grades its #4632 verdict by the engine\'s own discriminator', () => { + let logger: ReturnType; + let engine: AutomationEngine; + let afterThrows: string | undefined; + + const bubbleLines = () => logger.lines.filter( + l => typeof l.args[0] === 'string' && (l.args[0] as string).includes("subflow run"), + ); + + beforeEach(() => { + afterThrows = undefined; + logger = recordingLogger(); + engine = new AutomationEngine(logger as never, new InMemorySuspendedRunStore()); + registerSubflowNode(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 NodeExecutor); + engine.registerNodeExecutor({ + type: 'mark', + async execute() { + if (afterThrows) throw new Error(afterThrows); + return { success: true }; + }, + } as NodeExecutor); + engine.registerFlow('child_flow', CHILD as never); + engine.registerFlow('parent_flow', PARENT as never); + }); + + /** Park the parent at its `subflow` node and hand back both run ids. */ + async function park() { + const started = await engine.execute('parent_flow', {} as AutomationContext); + expect(started.status).toBe('paused'); + const parentRunId = started.runId!; + const parked = engine.listSuspendedRuns(); + const parent = parked.find(r => r.runId === parentRunId); + const child = parked.find(r => r.flowName === 'child_flow'); + expect(parent?.correlation, 'the parent parks correlated to its child') + .toBe(`subflow:${child?.runId}`); + return { parentRunId, childRunId: child!.runId }; + } + + it('a STRANDED parent is reported at `error`, naming the run and its repair verb', async () => { + afterThrows = DOWNSTREAM_FAILURE; + const { parentRunId, childRunId } = await park(); + + const childRes = await engine.resume(childRunId); + + // The condition being graded, established rather than assumed: the + // child genuinely completed and the parent is genuinely unreachable. + expect(childRes.success, "the child's own completion is genuine").toBe(true); + expect(await engine.hasSuspendedRun(parentRunId)).toBe(false); + expect((await engine.resume(parentRunId)).code).toBe('RUN_NOT_FOUND'); + expect((await engine.getRun(parentRunId))?.status).toBe('failed'); + // …and it IS repairable, which is what makes the repair verb in the + // message a promise the platform keeps. + expect((await engine.restoreConsumedSuspension(parentRunId)).restored).toBe(true); + + const lines = bubbleLines(); + expect(lines.length, 'exactly one record for this bubble').toBe(1); + expect(lines[0].level, 'durability, not functional').toBe('error'); + const [message, errorSlot, meta] = lines[0].args as [string, unknown, Record]; + expect(message).toContain('STRANDED'); + expect(message).toContain(`'${parentRunId}'`); + expect(message, 'the FIX the durability rule owes').toContain( + `restoreConsumedSuspension('${parentRunId}')`, + ); + expect(message, 'the CONSEQUENCE the durability rule owes') + .toContain('was told the resume SUCCEEDED'); + expect(message, "#6499 — the failing node's text never reaches the message").not.toContain(DOWNSTREAM_FAILURE); + expect(message, 'one physical line').not.toContain('\n'); + // #5575 — the `Error` slot stays empty and the diagnostics ride the + // THIRD argument; a meta passed second would still render, so pinning + // the position is the only thing that keeps the contract shape. + expect(errorSlot).toBeUndefined(); + expect(meta).toMatchObject({ error: DOWNSTREAM_FAILURE, parentRunId, status: 'stranded' }); + }); + + it('REVERSE CONTROL — a healthy parent continuation logs nothing at all', async () => { + const { parentRunId, childRunId } = await park(); + + expect((await engine.resume(childRunId)).success).toBe(true); + + expect(bubbleLines(), 'nothing failed — nothing to report').toEqual([]); + expect((await engine.getRun(parentRunId))?.status).toBe('completed'); + }); + + it('REVERSE CONTROL — a parent failure the engine does NOT call stranded stays `warn`', async () => { + // The `engine-residual-log-cause.test.ts` site-12 shape: a reported + // failure with an envelope and NO `status`. The parent's suspension was + // never consumed on such an exit, so it stays parked and resumable — + // functional, and escalating it is exactly the over-application + // AGENTS.md warns trains everyone to skim `error`. + const { parentRunId, childRunId } = await park(); + const eng = engine as unknown as { resumeInternal: (runId: string, ...rest: unknown[]) => Promise }; + const real = eng.resumeInternal.bind(engine); + eng.resumeInternal = async (runId: string, ...rest: unknown[]) => + runId === parentRunId + ? { success: false, code: 'RESUME_IN_PROGRESS', error: 'another replica is resuming it' } + : real(runId, ...rest); + + expect((await engine.resume(childRunId)).success).toBe(true); + + const lines = bubbleLines(); + expect(lines.length).toBe(1); + expect(lines[0].level, 'functional — unchanged').toBe('warn'); + expect(lines[0].args[0]).toContain('failed — the parent'); + }); + + it('REVERSE CONTROL — a THROWN parent resume stays `warn`', async () => { + // Site 13's shape. A throw never carries the discriminator, so this arm + // has no measured stranding behind it and its verdict is untouched. + const { parentRunId, childRunId } = await park(); + const eng = engine as unknown as { resumeInternal: (runId: string, ...rest: unknown[]) => Promise }; + const real = eng.resumeInternal.bind(engine); + eng.resumeInternal = async (runId: string, ...rest: unknown[]) => { + if (runId === parentRunId) throw new Error('the parent resume blew up'); + return real(runId, ...rest); + }; + + expect((await engine.resume(childRunId)).success).toBe(true); + + const lines = bubbleLines(); + expect(lines.length).toBe(1); + expect(lines[0].level, 'functional — unchanged').toBe('warn'); + expect(lines[0].args[0]).toContain('threw — the thrown failure'); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index b3739ef73c..00d33445d1 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2171,6 +2171,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-approvals/src/team-approver-org-screen.test.ts", "verb": "delete",