Skip to content

Commit 5563bfb

Browse files
claude[bot]claude
andauthored
fix(service-automation): answer a delegated subflow child refusal as a refusal, not a terminal failure (#14567)
* test(service-automation): pin the subflow child-refusal delegation contract (#14379) Red half of the reproduction: a parent resume delegated to a child paused on a screen with a `required` field answers a code-less envelope, fails the parent and orphans the still-paused child. The negative control (a child that really ran and threw) is green on both sides. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * fix(service-automation): propagate a delegated subflow child's refusal instead of failing the parent (#14379) The subflow delegation block read every `!childRes.success` as a child that ran and died. A retryable refusal — the codes `resumeInternal` itself answers for a resume that never ran — left the child parked where it was, but consumed the PARENT's pause, recorded a failure, and answered a code-less envelope the transport maps to `400 FLOW_FAILED`; the corrected retry then answered `RUN_NOT_FOUND`. Branch on the child's own `code` (producer-first, per the triage ruling), return the child's envelope with the code intact and both pauses untouched, and reserve `failSuspendedRun` for a child that genuinely ran and failed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 35383c4 commit 5563bfb

3 files changed

Lines changed: 350 additions & 0 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
Answer a delegated subflow child's retryable refusal as a refusal, not as a terminal child failure.
6+
7+
A run paused at a `subflow` node forwards a resume down to the child it is parked on — the screen-flow path, where the caller holds one stable run id (the parent's) and posts every wizard step to it. When the child *refused* that bag (`INVALID_SCREEN_INPUT` for a missing `required` field, `INVALID_SIGNAL`, `RESUME_IN_PROGRESS`, `STORE_UNAVAILABLE`), the delegation read it as a child that ran and died: it failed the parent run, consumed the parent's suspension, orphaned the still-paused child, and answered a **code-less** envelope, which a transport maps to `400 FLOW_FAILED`. The corrected retry on the same run id then answered `RUN_NOT_FOUND` — one mistyped form field destroyed a running workflow.
8+
9+
The delegation now branches on the child's own `code`. A refusal is returned verbatim with its `code` intact (and the parent's `durationMs`), **both** pauses left live, so the corrected submission still lands on the same parent run id. A child that genuinely ran and failed still fails the parent exactly as before.
Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* A subflow parent's resume must answer a delegated child's RETRYABLE REFUSAL
5+
* as a refusal — not as a terminal child failure (#14379).
6+
*
7+
* The screen-flow path gives the caller ONE stable run id: the parent's. Every
8+
* wizard step is posted to it, and `resumeInternal`'s subflow delegation block
9+
* forwards the bag down to the child the parent is parked on. When the child
10+
* REFUSES that bag — `INVALID_SCREEN_INPUT` for a missing `required` field,
11+
* `INVALID_SIGNAL` for a reserved variable name — nothing ran and the child's
12+
* pause is deliberately still live (#4477: "a rejected bag leaves the pause
13+
* live and the legitimate submission still lands").
14+
*
15+
* The delegation block used to read every `!childRes.success` as a child that
16+
* RAN and DIED: it called `failSuspendedRun` on the parent and answered a
17+
* CODE-LESS `{ success: false, error }`. One mistyped form field therefore
18+
* destroyed the run — the parent's suspension consumed and a failure recorded,
19+
* the still-paused child orphaned with nothing to bubble into, the caller told
20+
* `400 FLOW_FAILED` ("it ran and was rejected") for something that never ran,
21+
* and their corrected retry on the same run id answered `RUN_NOT_FOUND`.
22+
*
23+
* The discriminator is PRODUCER-FIRST (triage ruling, 2026-09-02): the child's
24+
* own `code` being one of the refusal codes the engine itself answers — ⛔ not
25+
* "is the child's suspension still live", which is a second store read whose
26+
* answer can race and which infers intent from state. `failSuspendedRun` is
27+
* reserved for a child that genuinely ran and failed, which the last test here
28+
* is the negative control for.
29+
*/
30+
31+
import { describe, it, expect, beforeEach } from 'vitest';
32+
import { AutomationEngine } from '../engine.js';
33+
import type { NodeExecutor } from '../engine.js';
34+
import { installBuiltinNodes } from './index.js';
35+
import { defineActionDescriptor } from '@objectstack/spec/automation';
36+
37+
function silentLogger() {
38+
return { info() {}, warn() {}, error() {}, debug() {}, child() { return silentLogger(); } } as any;
39+
}
40+
function ctx() {
41+
return { logger: silentLogger(), getService() { return undefined; } } as any;
42+
}
43+
44+
/**
45+
* `resumeAuthority: 'any'` on the fixture pausers: the resume gate (#5561)
46+
* follows the linked-run chain to the CHILD's node, so the type the child
47+
* parks on is what a resume of the parent is judged against. These tests are
48+
* about delegation mechanics, not the gate (`resume-authority-gate.test.ts`
49+
* owns that), so the fixtures state the posture they rely on.
50+
*/
51+
const openPauser = (type: string) => defineActionDescriptor({
52+
type, version: '1.0.0', name: type,
53+
supportsPause: true, resumeAuthority: 'any',
54+
});
55+
56+
/** The child's screen declares exactly one unconditional required field. */
57+
const REQUIRED_KIND = [{ name: 'kind', label: 'Kind', type: 'text', required: true }];
58+
59+
/** A child flow that parks on a real `screen` node and exports what it collected. */
60+
const screenChild = (name: string, tail: Array<Record<string, unknown>> = []) => ({
61+
name,
62+
label: name,
63+
type: 'screen',
64+
status: 'active',
65+
version: 1,
66+
variables: [{ name: 'kind', type: 'text', isOutput: true }],
67+
nodes: [
68+
{ id: 'start', type: 'start', label: 'Start' },
69+
{ id: 'ask', type: 'screen', label: 'Ask', config: { fields: REQUIRED_KIND } },
70+
...tail,
71+
{ id: 'end', type: 'end', label: 'End' },
72+
],
73+
edges: [
74+
{ id: 'e1', source: 'start', target: 'ask', type: 'default' },
75+
...tail.map((n, i) => ({
76+
id: `t${i}`,
77+
source: i === 0 ? 'ask' : (tail[i - 1] as { id: string }).id,
78+
target: (n as { id: string }).id,
79+
type: 'default',
80+
})),
81+
{
82+
id: 'e2',
83+
source: tail.length ? (tail[tail.length - 1] as { id: string }).id : 'ask',
84+
target: 'end',
85+
type: 'default',
86+
},
87+
],
88+
});
89+
90+
/** A child flow that parks on a pause declaring NO screen contract. */
91+
const openChild = (name: string) => ({
92+
name,
93+
label: name,
94+
type: 'autolaunched',
95+
status: 'active',
96+
version: 1,
97+
variables: [{ name: 'kind', type: 'text', isOutput: true }],
98+
nodes: [
99+
{ id: 'start', type: 'start', label: 'Start' },
100+
{ id: 'hold', type: 'openpauser', label: 'Hold' },
101+
{ id: 'end', type: 'end', label: 'End' },
102+
],
103+
edges: [
104+
{ id: 'e1', source: 'start', target: 'hold', type: 'default' },
105+
{ id: 'e2', source: 'hold', target: 'end', type: 'default' },
106+
],
107+
});
108+
109+
/** The parent: start → subflow(child) → recorder → end. */
110+
const parentFlow = (childName: string) => ({
111+
name: 'parent_flow',
112+
label: 'Parent Flow',
113+
type: 'autolaunched',
114+
status: 'active',
115+
version: 1,
116+
nodes: [
117+
{ id: 'ps', type: 'start', label: 'Start' },
118+
{ id: 'call', type: 'subflow', label: 'Call Child', config: { flowName: childName, outputVariable: 'childOut' } },
119+
{ id: 'rec', type: 'recorder', label: 'Record' },
120+
{ id: 'pe', type: 'end', label: 'End' },
121+
],
122+
edges: [
123+
{ id: 'p1', source: 'ps', target: 'call', type: 'default' },
124+
{ id: 'p2', source: 'call', target: 'rec', type: 'default' },
125+
{ id: 'p3', source: 'rec', target: 'pe', type: 'default' },
126+
],
127+
});
128+
129+
describe('subflow delegation: a child REFUSAL is answered as a refusal (#14379)', () => {
130+
let engine: AutomationEngine;
131+
let captured: unknown[];
132+
133+
beforeEach(() => {
134+
engine = new AutomationEngine(silentLogger());
135+
installBuiltinNodes(engine, ctx());
136+
captured = [];
137+
// Downstream of the parent's subflow node: proves the parent really
138+
// continued and what the child's output mapped to.
139+
engine.registerNodeExecutor({
140+
type: 'recorder',
141+
async execute(_node, variables) {
142+
captured.push(variables.get('childOut'));
143+
return { success: true };
144+
},
145+
} as NodeExecutor);
146+
// A pause that declares no screen contract (for the INVALID_SIGNAL arm).
147+
engine.registerNodeExecutor({
148+
type: 'openpauser',
149+
descriptor: openPauser('openpauser'),
150+
async execute() { return { success: true, suspend: true }; },
151+
} as NodeExecutor);
152+
// Terminal child failure, downstream of the child's screen.
153+
engine.registerNodeExecutor({
154+
type: 'boomer',
155+
async execute() { throw new Error('boom in the child'); },
156+
} as NodeExecutor);
157+
});
158+
159+
/** Start the parent and return `[parentRunId, childRunId]`. */
160+
async function startPair(): Promise<[string, string]> {
161+
const started = await engine.execute('parent_flow', {} as any);
162+
expect(started.status).toBe('paused');
163+
const parentRunId = started.runId!;
164+
const child = engine.listSuspendedRuns().find((r) => r.runId !== parentRunId)!;
165+
expect(child).toBeDefined();
166+
return [parentRunId, child.runId];
167+
}
168+
169+
describe('INVALID_SCREEN_INPUT — the child screen refuses the bag', () => {
170+
beforeEach(() => {
171+
engine.registerFlow('child_flow', screenChild('child_flow') as any);
172+
engine.registerFlow('parent_flow', parentFlow('child_flow') as any);
173+
});
174+
175+
it('answers the child refusal with its code and leaves BOTH pauses intact', async () => {
176+
const [parentRunId, childRunId] = await startPair();
177+
178+
const res = await engine.resume(parentRunId, { variables: {} });
179+
180+
// ADR-0112 envelope: the code the child produced, propagated intact.
181+
// A code-less envelope is what made the transport answer
182+
// `400 FLOW_FAILED` for something that never ran.
183+
expect(res.success).toBe(false);
184+
expect(res.code).toBe('INVALID_SCREEN_INPUT');
185+
// The child's own actionable text, not "subflow run '…' failed:".
186+
expect(res.error).toMatch(/^Invalid screen input: /);
187+
expect(res.error).toContain('"kind"');
188+
expect(res.error).toMatch(/required/i);
189+
// Nothing was consumed on either level.
190+
expect(await engine.hasSuspendedRun(parentRunId)).toBe(true);
191+
expect(await engine.hasSuspendedRun(childRunId)).toBe(true);
192+
// The parent still surfaces the child's screen, unchanged.
193+
expect((await engine.getSuspendedScreen(parentRunId))?.nodeId).toBe('ask');
194+
expect(captured).toEqual([]); // the parent did NOT continue
195+
});
196+
197+
it('completes the corrected retry on the SAME parent run id', async () => {
198+
const [parentRunId, childRunId] = await startPair();
199+
expect((await engine.resume(parentRunId, { variables: {} })).code).toBe('INVALID_SCREEN_INPUT');
200+
201+
const good = await engine.resume(parentRunId, { variables: { kind: 'normal' } });
202+
203+
expect(good.success).toBe(true);
204+
expect(good.code).toBeUndefined();
205+
expect(good.status).toBeUndefined(); // ran to completion
206+
expect(captured).toEqual([{ kind: 'normal' }]); // child output mapped into the parent
207+
expect(await engine.hasSuspendedRun(parentRunId)).toBe(false);
208+
expect(await engine.hasSuspendedRun(childRunId)).toBe(false);
209+
});
210+
211+
it('refuses the signal-less gesture the same way, both pauses intact', async () => {
212+
// #13648 normalises an absent signal to `{}` at the public door, so
213+
// `resume(parentRunId)` lands on this same delegation path.
214+
const [parentRunId, childRunId] = await startPair();
215+
216+
const bare = await engine.resume(parentRunId);
217+
218+
expect(bare.success).toBe(false);
219+
expect(bare.code).toBe('INVALID_SCREEN_INPUT');
220+
expect(await engine.hasSuspendedRun(parentRunId)).toBe(true);
221+
expect(await engine.hasSuspendedRun(childRunId)).toBe(true);
222+
// And the corrected retry still lands.
223+
const good = await engine.resume(parentRunId, { variables: { kind: 'late' } });
224+
expect(good.success).toBe(true);
225+
expect(captured).toEqual([{ kind: 'late' }]);
226+
});
227+
});
228+
229+
describe('INVALID_SIGNAL — a second refusal code on the same path', () => {
230+
beforeEach(() => {
231+
engine.registerFlow('child_flow', openChild('child_flow') as any);
232+
engine.registerFlow('parent_flow', parentFlow('child_flow') as any);
233+
});
234+
235+
it('answers the child refusal with its code and leaves BOTH pauses intact', async () => {
236+
const [parentRunId, childRunId] = await startPair();
237+
238+
const res = await engine.resume(parentRunId, { variables: { $sneaky: 1 } });
239+
240+
expect(res.success).toBe(false);
241+
expect(res.code).toBe('INVALID_SIGNAL');
242+
expect(res.error).toMatch(/engine-internal variables/);
243+
expect(await engine.hasSuspendedRun(parentRunId)).toBe(true);
244+
expect(await engine.hasSuspendedRun(childRunId)).toBe(true);
245+
expect(captured).toEqual([]);
246+
247+
// The legitimate submission still lands on the same parent run id.
248+
const good = await engine.resume(parentRunId, { variables: { kind: 'ok' } });
249+
expect(good.success).toBe(true);
250+
expect(captured).toEqual([{ kind: 'ok' }]);
251+
});
252+
});
253+
254+
describe('negative control — a child that genuinely RAN and FAILED', () => {
255+
beforeEach(() => {
256+
engine.registerFlow('child_flow', screenChild('child_flow', [{ id: 'boom', type: 'boomer', label: 'Boom' }]) as any);
257+
engine.registerFlow('parent_flow', parentFlow('child_flow') as any);
258+
});
259+
260+
it('still fails the parent terminally, with the envelope shape unchanged', async () => {
261+
const [parentRunId, childRunId] = await startPair();
262+
263+
// The screen ACCEPTS this bag; the node after it throws.
264+
const res = await engine.resume(parentRunId, { variables: { kind: 'normal' } });
265+
266+
expect(res.success).toBe(false);
267+
expect(res.code).toBeUndefined(); // a terminal child failure carries none — unchanged
268+
expect(res.error).toMatch(/^subflow run '.*' \(child_flow\) failed: /);
269+
expect(res.error).toContain('boom in the child');
270+
// Both suspensions are consumed: the parent was failed, the child ran.
271+
expect(await engine.hasSuspendedRun(parentRunId)).toBe(false);
272+
expect(await engine.hasSuspendedRun(childRunId)).toBe(false);
273+
expect(captured).toEqual([]);
274+
});
275+
});
276+
});

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,38 @@ function isSuspendSignal(err: unknown): err is FlowSuspendSignal {
961961
*/
962962
class InputSchemaViolationError extends Error {}
963963

964+
/**
965+
* The refusal codes {@link AutomationEngine.resumeInternal} answers for a
966+
* resume that NEVER RAN — the run is untouched, nothing executed, and the
967+
* identical call succeeds once its cause is corrected or has passed (#14379).
968+
*
969+
* Read by the subflow delegation path, which forwards a parent's resume down
970+
* to the child the parent is parked on: a child answering one of these has
971+
* REFUSED, not failed, so the parent must answer the refusal rather than
972+
* record a failure. The set is the PRODUCER's own vocabulary — the codes this
973+
* file returns from that one method — which is why it is a closed list here
974+
* and not a predicate over state (triage ruling 2026-09-02: branch on the
975+
* child's `code`, ⛔ never on "is the child's suspension still live", a second
976+
* store read whose answer can race and which infers intent from state).
977+
*
978+
* ⛔ `RUN_NOT_FOUND` is deliberately absent, though `resumeInternal` returns it
979+
* too: it is the engine's terminal "this pause is gone for good" class (#8684)
980+
* — no suspension, an unregistered flow, or a node edited away underneath a
981+
* parked run — which a transport answers **404** and which no retry can fix.
982+
* A child in that state can never continue, so its parent cannot either.
983+
*/
984+
const RETRYABLE_RESUME_REFUSAL_CODES: ReadonlySet<string> = new Set([
985+
'INVALID_SCREEN_INPUT',
986+
'INVALID_SIGNAL',
987+
'RESUME_IN_PROGRESS',
988+
'STORE_UNAVAILABLE',
989+
] satisfies ReadonlyArray<NonNullable<AutomationResult['code']>>);
990+
991+
/** Whether an {@link AutomationResult} code names a resume that never ran. */
992+
function isRetryableResumeRefusal(code: AutomationResult['code']): boolean {
993+
return code !== undefined && RETRYABLE_RESUME_REFUSAL_CODES.has(code);
994+
}
995+
964996
/**
965997
* Marks a {@link ResumeSignal} the ENGINE built for its own continuations —
966998
* the subflow output mapping and the `map` item handoff. Module-private and
@@ -4817,6 +4849,39 @@ export class AutomationEngine implements IAutomationService {
48174849
screen: childRes.screen,
48184850
};
48194851
}
4852+
// [#14379] A child REFUSAL is not a child failure. The
4853+
// codes in {@link RETRYABLE_RESUME_REFUSAL_CODES} are this
4854+
// method's own answers for a resume that never ran: the
4855+
// child's screen contract is checked BEFORE
4856+
// `forgetSuspendedRun` precisely so "a rejected bag leaves
4857+
// the pause live and the legitimate submission still lands"
4858+
// (#4477), so the child is parked exactly where it was.
4859+
//
4860+
// Reading those as a terminal failure consumed the PARENT's
4861+
// pause over a mistyped form field — and the screen-flow
4862+
// path is where a caller holds ONE stable run id, the
4863+
// parent's, and posts every wizard step to it. The run was
4864+
// gone, the still-paused child orphaned with nothing left
4865+
// to bubble into, the caller told `400 FLOW_FAILED` ("it
4866+
// ran and was rejected") for something that never ran, and
4867+
// their corrected retry on that same id answered
4868+
// `RUN_NOT_FOUND`.
4869+
//
4870+
// The child's envelope is answered VERBATIM but for the
4871+
// parent's `durationMs`. Propagating the `code` is half the
4872+
// fix: a code-less envelope is exactly what forced the
4873+
// transport onto `400 FLOW_FAILED`, and leaving both pauses
4874+
// alive while still answering one repairs the state and
4875+
// leaves the caller equally misled. The child's `error` is
4876+
// the actionable half — `Screen field "kind" is required` —
4877+
// where the failure text below names neither the problem
4878+
// nor anything a caller can act on. Nothing else moves:
4879+
// neither pause is consumed, and the parent's surfaced
4880+
// screen needs no refresh because the child did not
4881+
// advance.
4882+
if (!childRes.success && isRetryableResumeRefusal(childRes.code)) {
4883+
return { ...childRes, durationMs: Date.now() - run.startTime };
4884+
}
48204885
if (!childRes.success) {
48214886
const error = `subflow run '${childRunId}' (${childRun.flowName}) failed: ${childRes.error ?? 'unknown error'}`;
48224887
await this.failSuspendedRun(run, error);

0 commit comments

Comments
 (0)