From 1b0b4cfaa4bcc34271523eda369336ab1a71e2eb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 09:58:33 +0000 Subject: [PATCH 1/4] wip(#15556): reproduction probe for the subflow-hosted approval bubble Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/subflow-bubble-strand.test.ts | 246 ++++++++++++++++++ 1 file changed, 246 insertions(+) create mode 100644 packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts diff --git a/packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts b/packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts new file mode 100644 index 0000000000..e04977f31c --- /dev/null +++ b/packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts @@ -0,0 +1,246 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * REPRODUCTION PROBE for #15556 — an approval hosted inside a SUBFLOW CHILD. + * Not a deliverable yet: this file exists to measure what the decision door + * actually answers when `bubbleToParent` fails, with its controls in the + * same run. + */ + +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; + +/** 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 probe — approval inside a subflow child, 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]; + + it('MEASUREMENT — parent bubble fails: what does the door answer?', async () => { + throwOn.after_sub = 'update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found'; + const automation = boot(); + + const started = await automation.execute('deal_parent', { + object: 'crm_deal', record: { id: 'd1', amount: 100 }, userId: 'submitter', + } as never); + // eslint-disable-next-line no-console + console.log('PROBE started =', JSON.stringify(started)); + const parentRunId = (started as any).runId as string; + + const req = await pendingRequest(); + // eslint-disable-next-line no-console + console.log('PROBE request =', JSON.stringify(req && { id: req.id, run: req.flow_run_id, status: req.status })); + const childRunId = req?.flow_run_id as string; + expect(childRunId, 'the request must name the CHILD run').toBeTruthy(); + expect(childRunId).not.toBe(parentRunId); + expect(await automation.hasSuspendedRun(parentRunId)).toBe(true); + + // Capture the envelope `bubbleToParent` receives for the PARENT resume — + // the thing the swallowing catch throws away. + 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 })); + + // eslint-disable-next-line no-console + console.log('PROBE door =', JSON.stringify(outcome.ok ? outcome.r : { threw: outcome.e.message, details: strandedDecisionDetails(outcome.e) })); + // eslint-disable-next-line no-console + console.log('PROBE marks =', JSON.stringify(marks)); + // eslint-disable-next-line no-console + console.log('PROBE parent suspended?', await automation.hasSuspendedRun(parentRunId)); + // eslint-disable-next-line no-console + console.log('PROBE parent resume =', JSON.stringify(await automation.resume(parentRunId))); + const parentRow = await automation.getRun(parentRunId); + // eslint-disable-next-line no-console + console.log('PROBE parent run row =', JSON.stringify(parentRow && { + status: (parentRow as any).status, error: (parentRow as any).error, + consumedSuspension: Boolean((parentRow as any).consumedSuspension), + })); + // eslint-disable-next-line no-console + console.log('PROBE child run row =', JSON.stringify(await automation.getRun(childRunId).then(r => r && { status: (r as any).status }))); + // eslint-disable-next-line no-console + console.log('PROBE request row =', JSON.stringify((await data.find('sys_approval_request', { where: { id: req.id } }))[0]?.status)); + // eslint-disable-next-line no-console + console.log('PROBE parent bubble envelope =', JSON.stringify(bubbled.map(b => ({ success: b.success, code: b.code, status: b.status, error: b.error })))); + // eslint-disable-next-line no-console + console.log('PROBE parent restore =', JSON.stringify(await automation.restoreConsumedSuspension(parentRunId, { requestedBy: 'probe' }))); + // eslint-disable-next-line no-console + console.log('PROBE log lines =', JSON.stringify(logger.lines.filter((l: any) => l.level !== 'debug' && l.level !== 'info').map((l: any) => [l.level, l.msg]))); + }); + + it('CONTROL A — same composition, parent downstream node healthy', async () => { + const automation = boot(); + const started = await automation.execute('deal_parent', { + object: 'crm_deal', record: { id: 'd2', amount: 100 }, userId: 'submitter', + } as never); + const parentRunId = (started as any).runId as string; + const req = await pendingRequest(); + 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 })); + // eslint-disable-next-line no-console + console.log('CTRL-A door =', JSON.stringify(outcome.ok ? outcome.r : { threw: outcome.e.message })); + // eslint-disable-next-line no-console + console.log('CTRL-A marks =', JSON.stringify(marks)); + // eslint-disable-next-line no-console + console.log('CTRL-A parent run row =', JSON.stringify(await automation.getRun(parentRunId).then(r => r && { status: (r as any).status }))); + }); + + it('CONTROL B — no subflow: the #13807 shape still throws at this door', async () => { + 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 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 })); + // eslint-disable-next-line no-console + console.log('CTRL-B door =', JSON.stringify(outcome.ok ? outcome.r : { threw: outcome.e.message, details: strandedDecisionDetails(outcome.e) })); + }); +}); From ea169b352778c96c86a429e1fbe752047a1590a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 10:15:40 +0000 Subject: [PATCH 2/4] fix(automation): report a subflow parent stranded by a failed up-bubble at `error`, not `warn` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An approval hosted in a subflow child was driven end to end for the first time (the card was filed NOT MEASURED). It reproduces: the child's resume completes, `bubbleToParent` resumes the parent, the parent's downstream node throws, and `resumeInternal` answers the #13937 stranded exit — `{ success: false, status: 'stranded' }`, no `code` — which consumed the parent's suspension, journalled a repair snapshot and recorded the run `failed`. Nothing in the engine moves it again; only `restoreConsumedSuspension` can. The seam's #4632 verdict was FUNCTIONAL for the whole arm, on the enumeration "the parent either failed terminally or stays visibly parked and resumable". The measurement falsifies it: a stranded parent is neither, the approval row is durably terminal, and the decision door answered its caller success. That is AGENTS.md's DURABILITY class verbatim, and the rule's third legal answer (a failure handed to the CALLER) does not apply because no caller is told. The level is now graded by the engine's own discriminator, never by this seam's guess: `status === 'stranded'` reports at `error`, naming the run and the repair verb; every other exit stays `warn` unchanged, because escalating a parent that is still parked is how `error` becomes unreadable. ⚠️ The log half only. What the child's resumer — and through it the approvals decision door — is TOLD still reads as full success; making that truthful moves a public contract and is #15556's open decision. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/subflow-bubble-stranded-parent.md | 22 ++ ...=> subflow-hosted-approval-strand.test.ts} | 183 ++++++++++---- .../services/service-automation/src/engine.ts | 83 ++++++- .../subflow-bubble-strand-log-level.test.ts | 231 ++++++++++++++++++ 4 files changed, 455 insertions(+), 64 deletions(-) create mode 100644 .changeset/subflow-bubble-stranded-parent.md rename packages/plugins/plugin-approvals/src/{subflow-bubble-strand.test.ts => subflow-hosted-approval-strand.test.ts} (50%) create mode 100644 packages/services/service-automation/src/subflow-bubble-strand-log-level.test.ts diff --git a/.changeset/subflow-bubble-stranded-parent.md b/.changeset/subflow-bubble-stranded-parent.md new file mode 100644 index 0000000000..7eb5047f32 --- /dev/null +++ b/.changeset/subflow-bubble-stranded-parent.md @@ -0,0 +1,22 @@ +--- +'@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, because on those exits the parent +is still parked and resumable. + +⚠️ This is the log half only. What the child's resumer is told is unchanged. diff --git a/packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts b/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts similarity index 50% rename from packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts rename to packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts index e04977f31c..3a5c751fa6 100644 --- a/packages/plugins/plugin-approvals/src/subflow-bubble-strand.test.ts +++ b/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts @@ -1,10 +1,44 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * REPRODUCTION PROBE for #15556 — an approval hosted inside a SUBFLOW CHILD. - * Not a deliverable yet: this file exists to measure what the decision door - * actually answers when `bubbleToParent` fails, with its controls in the - * same run. + * #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'; @@ -16,6 +50,17 @@ 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 }> = []; @@ -116,7 +161,7 @@ const PARENT = { ], }; -describe('#15556 probe — approval inside a subflow child, parent bubble fails', () => { +describe('#15556 — an approval hosted in a subflow child, whose parent bubble fails', () => { let data: ReturnType; let service: ApprovalService; let logger: ReturnType; @@ -153,27 +198,30 @@ describe('#15556 probe — approval inside a subflow child, parent bubble fails' const pendingRequest = async () => (await data.find('sys_approval_request', { where: { status: 'pending' } }))[0]; - it('MEASUREMENT — parent bubble fails: what does the door answer?', async () => { - throwOn.after_sub = 'update_record(crm_leave_request) failed: Record 9SEmlyRfw8D9-J7Z not found'; + /** 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); - // eslint-disable-next-line no-console - console.log('PROBE started =', JSON.stringify(started)); 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(); - // eslint-disable-next-line no-console - console.log('PROBE request =', JSON.stringify(req && { id: req.id, run: req.flow_run_id, status: req.status })); const childRunId = req?.flow_run_id as string; - expect(childRunId, 'the request must name the CHILD run').toBeTruthy(); + expect(childRunId, 'the request names the CHILD run, never the parent').toBeTruthy(); expect(childRunId).not.toBe(parentRunId); expect(await automation.hasSuspendedRun(parentRunId)).toBe(true); - // Capture the envelope `bubbleToParent` receives for the PARENT resume — - // the thing the swallowing catch throws away. + // 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[]) => { @@ -186,61 +234,88 @@ describe('#15556 probe — approval inside a subflow child, parent bubble fails' .decide(req.id, { decision: 'approve', actorId: 'u1' }, SYSTEM_CTX) .then(r => ({ ok: true as const, r }), (e: Error) => ({ ok: false as const, e })); - // eslint-disable-next-line no-console - console.log('PROBE door =', JSON.stringify(outcome.ok ? outcome.r : { threw: outcome.e.message, details: strandedDecisionDetails(outcome.e) })); - // eslint-disable-next-line no-console - console.log('PROBE marks =', JSON.stringify(marks)); - // eslint-disable-next-line no-console - console.log('PROBE parent suspended?', await automation.hasSuspendedRun(parentRunId)); - // eslint-disable-next-line no-console - console.log('PROBE parent resume =', JSON.stringify(await automation.resume(parentRunId))); - const parentRow = await automation.getRun(parentRunId); - // eslint-disable-next-line no-console - console.log('PROBE parent run row =', JSON.stringify(parentRow && { - status: (parentRow as any).status, error: (parentRow as any).error, - consumedSuspension: Boolean((parentRow as any).consumedSuspension), - })); - // eslint-disable-next-line no-console - console.log('PROBE child run row =', JSON.stringify(await automation.getRun(childRunId).then(r => r && { status: (r as any).status }))); - // eslint-disable-next-line no-console - console.log('PROBE request row =', JSON.stringify((await data.find('sys_approval_request', { where: { id: req.id } }))[0]?.status)); - // eslint-disable-next-line no-console - console.log('PROBE parent bubble envelope =', JSON.stringify(bubbled.map(b => ({ success: b.success, code: b.code, status: b.status, error: b.error })))); - // eslint-disable-next-line no-console - console.log('PROBE parent restore =', JSON.stringify(await automation.restoreConsumedSuspension(parentRunId, { requestedBy: 'probe' }))); - // eslint-disable-next-line no-console - console.log('PROBE log lines =', JSON.stringify(logger.lines.filter((l: any) => l.level !== 'debug' && l.level !== 'info').map((l: any) => [l.level, l.msg]))); + // ── 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 — same composition, parent downstream node healthy', async () => { + 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: 'd2', amount: 100 }, userId: 'submitter', + 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 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 })); - // eslint-disable-next-line no-console - console.log('CTRL-A door =', JSON.stringify(outcome.ok ? outcome.r : { threw: outcome.e.message })); - // eslint-disable-next-line no-console - console.log('CTRL-A marks =', JSON.stringify(marks)); - // eslint-disable-next-line no-console - console.log('CTRL-A parent run row =', JSON.stringify(await automation.getRun(parentRunId).then(r => r && { status: (r as any).status }))); + + 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 — no subflow: the #13807 shape still throws at this door', async () => { + 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 outcome = await service + + const err = 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 })); - // eslint-disable-next-line no-console - console.log('CTRL-B door =', JSON.stringify(outcome.ok ? outcome.r : { threw: outcome.e.message, details: strandedDecisionDetails(outcome.e) })); + .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 f8fc575ee2..0a165bee01 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'); + }); +}); From 853eefcc5d029fe2d75a0adc94737e99fb1ff24a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 13:04:26 +0000 Subject: [PATCH 3/4] tooling(pm): record the #15556 reproduction's engine double in the pinned ledger `check:engine-double-contract` reported RETAINED on both write verbs for the new test file: its fake engine routes delete()/update() through ObjectQL's own dispatch predicates, but the pinned ledger had no row for the file, so the coverage protected nothing. Written by `--write`, as the gate instructs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- scripts/engine-double-contract.pinned.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 0314dcd082..6092f206d3 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2166,6 +2166,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", From 4e91f34bead02771d04050dbe0ba8b2be25097fe Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 14:06:43 +0000 Subject: [PATCH 4/4] docs(changeset): state the narrowing's reason as the discriminator argument, and name the two exits that are not healthy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changeset for this PR justifies leaving every non-stranded parent-resume exit at `warn`. The justification it now gives is the one this seam can actually stand behind: those exits carry no `'stranded'` discriminator, and `'stranded'` is the single exit that journals a repair snapshot — so it is the single exit an operator can act on, and grading by the engine's own verdict is what keeps `error` readable rather than skimmed. That is a claim about what the seam KNOWS, and the changeset now says so, plus the two exits that are known NOT to have left the parent healthy: * a THROWN parent resume carries no discriminator at all, and #15555 documents a window in which a throw landing between the journal and the stamp hides a parent that is genuinely stranded. This seam leaves that arm at `warn` deliberately and that card tracks it. * the CLAIM-PATH store failure states in its own envelope text that whether the suspension was consumed is UNKNOWN, and settles it by retry — which an up-bubble has no retrier to perform. "Not consumed" is the guarantee of the strict-load store failure alone, never of every store failure. Text only: no source, no pins and no ledger move, so the measured behaviour and both mutation legs are untouched. A blanket "the parent is still parked and resumable" would have read as a platform guarantee across all three exits, and release notes are built from this file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/subflow-bubble-stranded-parent.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/.changeset/subflow-bubble-stranded-parent.md b/.changeset/subflow-bubble-stranded-parent.md index 7eb5047f32..dc14066cb6 100644 --- a/.changeset/subflow-bubble-stranded-parent.md +++ b/.changeset/subflow-bubble-stranded-parent.md @@ -16,7 +16,20 @@ from the outside, which is the durability class. `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, because on those exits the parent -is still parked and resumable. +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.