diff --git a/.changeset/15556-subflow-parent-strand-on-decide.md b/.changeset/15556-subflow-parent-strand-on-decide.md new file mode 100644 index 0000000000..9fc02c9c73 --- /dev/null +++ b/.changeset/15556-subflow-parent-strand-on-decide.md @@ -0,0 +1,32 @@ +--- +"@objectstack/service-automation": minor +"@objectstack/plugin-approvals": minor +--- + +An approval `decide()` that resumes a subflow CHILD now tells the caller when that resume bubbles into a PARENT run that stranded — instead of answering full success with nothing to distinguish it from a healthy composition (#15556; the #16472 family ruling, decision batch #76, option A). + +**The composition.** A parent flow parks at a `subflow` node whose child hosts the `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. The parent lands on the engine's `'stranded'` exit — it consumed its suspension and is now terminal, repairable only by an operator's `restoreConsumedSuspension` — and `bubbleToParent` already logged that at `error` (unchanged by this fix). What the caller was TOLD did not: `resumed: true`, no `resumeError`, and a `runId` naming the healthy child — identical to what a fully healthy composition answers. + +``` +FROM service.decide(requestId, { decision: 'approve' }, ctx) + -> { finalized: true, decision: 'approve', runId: '', resumed: true } + // identical to a healthy composition's answer — no caller can tell + +TO service.decide(requestId, { decision: 'approve' }, ctx) + -> { finalized: true, decision: 'approve', runId: '', resumed: true, + resumeError: "RESUME_FAILED: … its own flow run '' resumed, but the " + + "subflow parent above it — run '' — consumed its suspension " + + "and is now stranded: ", + resumeFailure: { code: 'RESUME_FAILED', runId: '', status: 'stranded', repairable: true } } +``` + +**Additive only — no migration.** `ApprovalDecisionResult.resumeFailure` was already declared (and pinned) in `@objectstack/spec` ahead of this card; this fix is the first producer that fills it. No existing field changes shape, no status code moves (the door still never throws for this shape — `AGENTS.md`'s "a failure handed to the caller" answer does not apply here, since before this fix no caller was told at all), and the door's `error` log line is untouched. A consumer that already ignores unknown fields sees no difference; a consumer that reads `resumeFailure` can now tell a bubbled parent strand from a clean resume without diffing `runId` against a durable run history. + +**What did not move, on purpose.** `RESUME_IN_PROGRESS` / `STORE_UNAVAILABLE` bubble outcomes stay the functional degradation they always were (`warn`, unreported on `resumeFailure`) — the #16472 ruling is scoped to the one exit the engine calls `'stranded'`. The sibling `recall` door (`ApprovalRecallResult.resumeFailure`, #15970) is a separate card and is not touched here. + +**New public surface — the reason for `minor` on both packages, not `patch`.** Getting the parent's strand from the engine to the approvals door without touching `packages/spec` or the wire-visible `AutomationResult` (which a raw REST `POST …/resume` also serves verbatim, so a field there would leak an undeclared key onto every subflow resume, not only an approvals-mediated one) needed a small new internal channel: + +- `@objectstack/service-automation`: `AutomationEngine` gains a new public method, `takeSubflowParentStrand(childRunId: string): SubflowParentStrand | undefined` — read-once (deletes on read), populated only by `bubbleToParent`'s `'stranded'` exit. `SubflowParentStrand` is a new exported interface (`{ runId, repairable: true, error }`). +- `@objectstack/plugin-approvals`: `ApprovalResumeSurface` (already exported from the package entry) gains a matching optional member, `takeSubflowParentStrand?(childRunId): { runId, repairable, error } | undefined`. + +Both are additive and optional; nothing existing changes shape or behaviour. Neither reaches any wire payload — `AutomationResult`, the REST resume door's response, and every other published contract are byte-for-byte unchanged. diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index 162b1b825b..9a68d3685b 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -47,6 +47,7 @@ import type { ApprovalResubmitResult, ApprovalStatus, ApprovalCancelReason, + ResumeFailureReport, } from '@objectstack/spec/contracts'; // [#7135] The full `resolveAuthzContext` envelope — what `IApprovalService` // declares for every one of these context parameters since #6523 (the #6206 @@ -226,6 +227,26 @@ export interface ApprovalResumeSurface { | { repairable: true } | { repairable: false; reason: 'RUN_SUSPENDED' | 'SNAPSHOT_DROPPED' | 'NO_CONSUMED_SUSPENSION' } >; + /** + * [#15556; the #16472 family ruling] The subflow PARENT strand that + * `childRunId`'s own completion bubbled into, if the engine's up-bubble + * hit the `'stranded'` exit — read, and CLEARED, by + * {@link ApprovalService.resumeRecordedOutcome} right after a resume it + * issued reports success, so a decision whose OWN run advanced can still + * tell the caller a run further up the chain did not. + * + * ⚠️ Declares a method `AutomationEngine` ALREADY implements publicly + * (`takeSubflowParentStrand`); it widens no wire surface — the engine's + * generic `resume()` / `AutomationResult` answer is UNCHANGED by this + * member's existence, which is exactly why a caller must ask for it by + * name instead of finding it riding the resume result. + * + * `runId` here is the PARENT's — never `childRunId` itself, and never the + * run a caller of `decide`/`resumeRecordedOutcome` was resuming. Optional: + * an engine that predates this member simply never reports a bubbled + * strand, exactly as before this card. + */ + takeSubflowParentStrand?(childRunId: string): { runId: string; repairable: boolean; error: string } | undefined; } /** What {@link ApprovalResumeSurface.inspectConsumedSuspension} answers. */ @@ -668,8 +689,10 @@ export interface StrandedContinuationSignal { * Outcome of {@link ApprovalService.continueRestoredRun} (#15389). * * ⚠️ Deliberately its own shape rather than a reuse of `ApprovalDecisionResult`: - * that contract is the subject of an OPEN maintainer ruling on #15556, and this - * card must not pre-empt it. Nothing here changes what `decide` answers. + * the #16472 family ruling settled that contract for `decide` (#15556) and + * `recall` (#15970) by name, and this repair-replay verb is neither of those + * doors — reusing their shape here would answer a question nobody asked. + * Nothing here changes what `decide` answers. */ export interface ApprovalContinuationResult { /** True when the restored pause was consumed and the flow moved on. */ @@ -3113,11 +3136,19 @@ export class ApprovalService implements IApprovalService { * how an approval could be recorded, reported as resumed, and leave its flow * stranded forever (#4420). The thrown error carries {@link resumeCodeOf}'s * `resumeCode` so callers can tell a benign duplicate from a dead run. + * + * [#15556; the #16472 family ruling] On the SUCCESS path this also asks + * {@link ApprovalResumeSurface.takeSubflowParentStrand} whether resuming + * `runId` bubbled into a parent that then stranded — a fact the engine's + * own `resume()` answer never carries (its `AutomationResult` is unchanged + * by this card, on purpose: that value can also reach a raw REST resume + * caller verbatim, and this is not a wire member). `undefined` on every + * other outcome, exactly like the surface member itself. */ private async serviceResume( runId: string, signal: { output?: Record; branchLabel?: string }, - ): Promise { + ): Promise<{ runId: string; repairable: boolean; error: string } | undefined> { const result = await this.automation!.resume!(runId, { ...signal, [RESUME_AUTHORITY_SERVICE]: true }); const reported = result as { success?: boolean; code?: string; error?: string; status?: string } | undefined; @@ -3132,6 +3163,7 @@ export class ApprovalService implements IApprovalService { err.resumeStatus = reported.status; throw err; } + return this.automation?.takeSubflowParentStrand?.(runId); } /** The engine failure code behind a {@link serviceResume} rejection, if any. */ @@ -3279,6 +3311,26 @@ export class ApprovalService implements IApprovalService { * happen", which is the misreading that makes a caller retry or escalate * against a decision that IS durable. * + * ## A resume that SUCCEEDED can still carry a strand (#15556) + * + * #13807 above is about THIS run failing to resume. A DIFFERENT run can + * strand as a side effect of this one succeeding: `runId` parks inside a + * subflow, so resuming it can bubble into a PARENT that consumed ITS OWN + * suspension and then failed downstream (`AutomationEngine.bubbleToParent`, + * `service-automation`). Before the #16472 family ruling that fact reached + * nobody — `resumed: true`, no `resumeError`, and the `runId` this method + * returns names the run that is genuinely fine, never the stranded parent. + * The ruling (option A): the door's status code still does not move — this + * method still returns normally — but `resumeFailure` on the return value + * carries the PARENT's `runId` and `repairable`, read via + * {@link serviceResume}'s post-success + * {@link ApprovalResumeSurface.takeSubflowParentStrand} check, and + * `resumeError` carries the same event in prose. Absent on every OTHER + * success — a plain resume, or one whose subflow parent (if any) advanced + * cleanly — so a caller reading this member sees a report only when there + * is one to make, exactly {@link ApprovalDecisionResult.resumeFailure}'s + * absence rule. + * * @param what - how the recorded outcome reads in the error, e.g. * `"the approve decision"`. * @param decision - the outcome label for the machine-readable envelope @@ -3293,11 +3345,35 @@ export class ApprovalService implements IApprovalService { what: string, signal: { output?: Record; branchLabel?: string }, decision: string, - ): Promise<{ resumed: boolean; resumeError?: string }> { + ): Promise<{ resumed: boolean; resumeError?: string; resumeFailure?: ResumeFailureReport }> { const missing = this.missingRunCapability(runId, requestId, what, 'resume'); if (missing) return { resumed: false, resumeError: missing }; try { - await this.serviceResume(runId, signal); + const bubbleStrand = await this.serviceResume(runId, signal); + if (bubbleStrand) { + // #15556: this door's OWN resume succeeded — `runId` really did + // advance — but the subflow parent it bubbled into did not. Told on + // BOTH halves of the ONE telling (spec docblock, `ApprovalDecisionResult`): + // `resumeFailure` for a machine, `resumeError` for a human, never + // gated on `resumed`, which stays `true` here. ⛔ No NEW log line: the + // #16472 ruling left logging alone — `bubbleToParent`'s own `error` + // line (`service-automation`) already said this ONCE, with the + // consequence and the fix, per AGENTS.md's "say it once" durability + // rule; a second statement here would be the same event twice. + return { + resumed: true, + resumeError: + `RESUME_FAILED: ${what} was recorded on request ${requestId} and its own flow run '${runId}' ` + + `resumed, but the subflow parent above it — run '${bubbleStrand.runId}' — consumed its ` + + `suspension and is now stranded: ${bubbleStrand.error}`, + resumeFailure: { + code: 'RESUME_FAILED', + runId: bubbleStrand.runId, + status: 'stranded', + repairable: bubbleStrand.repairable, + }, + }; + } return { resumed: true }; } catch (err: any) { const reason = err?.message ?? String(err); @@ -3360,6 +3436,7 @@ export class ApprovalService implements IApprovalService { let resumed = false; let resumeError: string | undefined; + let resumeFailure: ResumeFailureReport | undefined; // No `typeof this.automation?.resume === 'function'` guard here (#4420): // skipping the call when no engine is attached is precisely how a decision // against a parked run returned 200 / `resumed: false` with nothing logged. @@ -3383,6 +3460,7 @@ export class ApprovalService implements IApprovalService { ); resumed = outcome.resumed; resumeError = outcome.resumeError; + resumeFailure = outcome.resumeFailure; } return { @@ -3392,6 +3470,11 @@ export class ApprovalService implements IApprovalService { runId: result.runId, resumed, ...(resumeError ? { resumeError } : {}), + // [#15556; #16472 ruling] Additive — see the field's own docblock in + // `@objectstack/spec/contracts`'s absence rule: omitted entirely rather + // than `undefined`, so a consumer that merely checks `'resumeFailure' + // in result` reads presence correctly. + ...(resumeFailure ? { resumeFailure } : {}), }; } 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 index 3a5c751fa6..5a78ed5108 100644 --- a/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts +++ b/packages/plugins/plugin-approvals/src/subflow-hosted-approval-strand.test.ts @@ -1,12 +1,14 @@ // 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. + * #15556 — 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. + * file is that drive, with its controls in the same run. It reproduced the + * defect, and now pins the fix (#16472 family ruling, maintainer 2026-09-07, + * decision batch #76, option A). * * ## The composition * @@ -15,30 +17,32 @@ * 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 + * ## What was measured, and what the ruling fixed * - * 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). + * MEASURED, unchanged by this fix: the parent lands on the engine's stranded + * exit — `{ success: false, status: 'stranded' }`, journalled and repairable + * — and `bubbleToParent` logs that at `error`, naming the run and the repair + * verb (`subflow-bubble-strand-log-level.test.ts` in `service-automation` + * holds those pins, both directions; the #16472 ruling explicitly leaves the + * log alone). * - * ⚠️ 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. + * FIXED here: before this ruling the decision door's resume-facing answer was + * IDENTICAL to a healthy composition's — no caller could tell the two apart, + * and the `runId` it handed back named the CHILD, which completed, never the + * stranded parent. The door's status code still does not move (`resumed` + * stays `true` — the CHILD really did resume) but the answer now carries the + * strand behind it: `resumeFailure` names the PARENT's `runId` and + * `repairable`, and `resumeError` tells the same fact in prose. Read off + * `AutomationEngine.takeSubflowParentStrand` (added for this card), which + * `serviceResume` consults right after its own resume reports success. * - * ## The control that makes the reading trustworthy + * ## The controls that make 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. + * `CONTROL A` drives the healthy composition through the SAME door in the + * same run — proof that the two answers now genuinely DIVERGE, not that this + * test's plumbing merely stopped checking. `CONTROL B` drives the #13807 + * shape — no subflow, the child's own branch throws — where the door still + * throws `RESUME_FAILED` with its stranded envelope, unaffected by this card. */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -54,10 +58,11 @@ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as any; 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. + * The resume-facing answer a caller reads, minus the run id — CONTROL A's + * shape: a plain healthy resume with nothing behind it to tell. Before the + * fix this was ALSO the stranded composition's answer, byte for byte — the + * defect this file reproduced. It no longer is; the stranded test asserts + * its own, divergent shape below instead of reusing this constant. */ const FULL_SUCCESS = { finalized: true, decision: 'approve', resumed: true, resumeError: undefined }; @@ -203,7 +208,7 @@ describe('#15556 — an approval hosted in a subflow child, whose parent bubble finalized: r.finalized, decision: r.decision, resumed: r.resumed, resumeError: r.resumeError, }); - it('the parent STRANDS while the door answers full success', async () => { + it('the parent STRANDS, and the door now tells the caller so on `resumeFailure`', async () => { throwOn.after_sub = DOWNSTREAM_FAILURE; const automation = boot(); @@ -253,18 +258,41 @@ describe('#15556 — an approval hosted in a subflow child, whose parent bubble .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); + // FIXED — see the file header. The door still does not throw (#13807's + // status code does not move) and `resumed` stays `true` — this decision's + // OWN run, the child, really did resume. But it no longer reads as a + // clean success: `resumeFailure` names the PARENT, never the healthy + // child `runId` still names, and `resumeError` tells the same fact in + // prose. + expect(outcome.ok, 'the door still does not throw for this shape').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(answer.finalized).toBe(true); + expect(answer.decision).toBe('approve'); + expect(answer.resumed, "this decision's own run — the child — really did resume").toBe(true); + expect(answer.runId, 'the id handed back is still 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). + // ── The machine-readable half: the PARENT's id, never the child's. + expect(answer.resumeFailure).toEqual({ + code: 'RESUME_FAILED', + runId: parentRunId, + status: 'stranded', + repairable: true, + }); + // ── The human-readable half — presence decided by the telling, never by + // `resumed`, which is `true` right here (the spec docblock's rule). + expect(answer.resumeError).toContain('RESUME_FAILED'); + expect(answer.resumeError).toContain(parentRunId); + expect(answer.resumeError).toContain(DOWNSTREAM_FAILURE); + + // ── And the two answers now genuinely DIVERGE — no longer the shared + // `FULL_SUCCESS` literal CONTROL A asserts below. + expect(resumeFacing(answer)).not.toEqual(FULL_SUCCESS); + + // The one LOG artefact the operator gets, at the level AGENTS.md's + // durability rule requires, naming the run and the repair verb — the + // #16472 ruling left this log line alone; it is now a SIBLING to + // `resumeFailure`, not this card's only telling. const durability = logger.lines.filter( (l: any) => l.level === 'error' && String(l.msg).includes('STRANDED'), ); @@ -276,7 +304,7 @@ describe('#15556 — an approval hosted in a subflow child, whose parent bubble }); - it('CONTROL A — the healthy composition answers IDENTICALLY, which is the defect', async () => { + it('CONTROL A — a healthy composition answers plain success, with nothing behind it to tell', async () => { const automation = boot(); const started = await automation.execute('deal_parent', { object: 'crm_deal', record: { id: 'd1', amount: 100 }, userId: 'submitter', @@ -291,11 +319,15 @@ describe('#15556 — an approval hosted in a subflow child, whose parent bubble 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. + // ⭐ Before the fix this was the SAME literal the stranded composition + // asserted — a caller comparing the two answers had nothing to compare. + // Kept here as the reverse control: it still holds for a run that really + // has nothing to report, which is what proves the stranded test's new + // divergent shape is about the strand and not a plumbing change that + // fires unconditionally. expect(resumeFacing(answer)).toEqual(FULL_SUCCESS); expect(answer.runId).toBe(req.flow_run_id); + expect(answer.resumeFailure, 'nothing to report — absence is the correct reading here').toBeUndefined(); }); it("CONTROL B — the #13807 shape still throws at this door, so the harness is live", async () => { diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 50fb3b3cae..bba6374154 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -1504,6 +1504,28 @@ export interface ConsumedSuspension { persisted?: 'pending' | 'landed' | 'failed'; } +/** + * [#15556] What {@link AutomationEngine.bubbleToParent} records — under the + * CHILD's own run id — when resuming that child's completion up-bubbles into + * a PARENT that consumed its suspension and then failed downstream. Read once + * via {@link AutomationEngine.takeSubflowParentStrand}: exported only so the + * class's public method can name its return type in this file's declaration + * output, never re-exported from this package's index — the shape a consumer + * reads is this file's own return value, not a type it imports. + * + * Recorded on exactly the arm {@link AutomationResult.status} calls + * `'stranded'` — never on `RESUME_IN_PROGRESS` / `STORE_UNAVAILABLE`, which + * stay the functional degradation #15556's ruling left alone. + */ +export interface SubflowParentStrand { + /** The run that is actually stranded — the PARENT, never the child that just completed. */ + runId: string; + /** Always `true`: the one arm that records an entry is the one the engine journals a repair snapshot for. */ + repairable: true; + /** The parent's own downstream failure text, verbatim — `resumeInternal`'s `error` for that run. */ + error: string; +} + /** * Why {@link AutomationEngine.restoreConsumedSuspension} declined (#13909). * @@ -2165,6 +2187,21 @@ export class AutomationEngine implements IAutomationService { * holds a whole suspension, and the durable copy is the record. */ private consumedSuspensions = new Map(); + /** + * [#15556] The subflow PARENT strand a child's completion bubbled into, + * keyed by the CHILD run id — i.e. exactly the run id a caller resuming + * the child already holds. {@link bubbleToParent} writes an entry only on + * its `'stranded'` exit (never on the tolerated `RESUME_IN_PROGRESS` / + * `STORE_UNAVAILABLE` arms, which stay functional per #15556's ruling); + * {@link takeSubflowParentStrand} is the one reader, and it deletes on + * read so an entry nobody asks for does not accumulate forever. + * + * Bounded by {@link MAX_CONSUMED_SUSPENSIONS} for the same reason that + * bound exists one field up — a caller that never asks (a bare `resume()` + * with no subflow-aware consumer) must not leak memory across restarts of + * the same long-lived process. + */ + private subflowParentStrands = new Map(); /** * [#13909] Run ids currently mid-RESTORE — the same synchronous in-process * guard shape as {@link resuming}, so two operators racing the verb produce @@ -6657,6 +6694,17 @@ export class AutomationEngine implements IAutomationService { * the parent's own completion bubbles multi-level chains. Best-effort — * a failed parent continuation is logged, never thrown back at the * caller who resumed the child. + * + * [#15556] Best-effort at the ENGINE layer only, since this call never + * throws either way: on the `'stranded'` exit — the one #15556's ruling + * names — it also records a {@link SubflowParentStrand} under the CHILD's + * own run id in {@link subflowParentStrands}, so the caller who resumed + * that child (`resumeInternal` returns before this method's caller sees + * anything beyond `success: true`) can retrieve it via + * {@link takeSubflowParentStrand} and tell its OWN caller the truth. The + * two tolerated arms below (`RESUME_IN_PROGRESS` / `STORE_UNAVAILABLE`) + * record nothing — #15556's ruling is scoped to the strand, and those two + * stay the functional degradation the old verdict already had right. */ private async bubbleToParent( run: SuspendedRun, @@ -6722,14 +6770,26 @@ export class AutomationEngine implements IAutomationService { // 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. + // #15556 family ruling (#16472, maintainer 2026-09-07, decision + // batch #76, option A): the LOG half stays exactly as it was + // (below, unchanged) — the door's status code does not move — + // and the strand is ALSO recorded here, under the CHILD's own + // run id, so `takeSubflowParentStrand` can hand it to whichever + // caller resumed that child. Not a widening of what this + // method tells ITS OWN caller (still `void`, still never + // thrown) — a sibling side-channel, read only by a caller that + // asks for it by name. if (parentRes.status === 'stranded') { + this.subflowParentStrands.set(run.runId, { + runId: parentRunId, + repairable: true, + error: parentRes.error ?? 'unknown error', + }); + while (this.subflowParentStrands.size > MAX_CONSUMED_SUSPENSIONS) { + const oldest = this.subflowParentStrands.keys().next().value; + if (oldest === undefined) break; + this.subflowParentStrands.delete(oldest); + } // THIRD argument per the `Logger` contract // (`error(message, error?, meta?)`); the `Error` slot stays // empty on purpose (#5575). The message owes the two things @@ -6776,6 +6836,32 @@ export class AutomationEngine implements IAutomationService { } } + /** + * [#15556] Read — and clear — the {@link SubflowParentStrand} + * {@link bubbleToParent} recorded for `childRunId`'s most recent + * completion, if any. `undefined` on every other outcome: no parent, a + * parent that resumed cleanly, or a parent bubble that hit the tolerated + * `RESUME_IN_PROGRESS` / `STORE_UNAVAILABLE` arms (#15556's ruling leaves + * those functional, not reported here). + * + * Delete-on-read on purpose: this is a hand-off to the ONE caller that + * resumed `childRunId` and is about to answer its OWN caller, not a + * durable record — the durable half is the parent's own consumed- + * suspension journal and terminal history row, both already readable via + * {@link inspectConsumedSuspension} / {@link getRun} against the + * PARENT's run id, which this method is what hands a caller in the first + * place (nothing else names it). + * + * ⚠️ Never populated for `childRunId`'s OWN failure — only for a PARENT + * this child's completion bubbled into. A child that itself stranded is + * reported on `childRunId`'s own {@link AutomationResult}, unchanged. + */ + takeSubflowParentStrand(childRunId: string): SubflowParentStrand | undefined { + const found = this.subflowParentStrands.get(childRunId); + if (found) this.subflowParentStrands.delete(childRunId); + return found; + } + /** * Terminally fail a suspended run: consume its continuation and record a * `failed` log so it stops surfacing as resumable. Used when a subflow 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 index 45a1b16e7d..bdee9477db 100644 --- 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 @@ -38,8 +38,14 @@ * 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. + * This is the LOG half only, and it is UNCHANGED by the #16472 family ruling + * that closed #15556's open decision: the door's status code does not move, + * so what the child's resumer is TOLD still reads `resumed: true` here. What + * changed lives one package over, in `plugin-approvals`'s + * `subflow-hosted-approval-strand.test.ts` — the decision result now ALSO + * carries the strand as `resumeFailure`, read via + * `AutomationEngine.takeSubflowParentStrand` (added for exactly this), a + * sibling channel to the log line pinned below, not a replacement for it. */ import { describe, it, expect, beforeEach } from 'vitest';