Skip to content

Commit 839ba45

Browse files
committed
fix(cli): os validate and os build refuse a flow whose region the engine refuses to register
Both commands now hand each flow to the automation engine's registration verdict (FlowSchema, then the spec's validateControlFlow) through utils/flow-region-shape.ts, and refuse it with the engine's message behind the flow's name. Before, they passed a flow whose try_catch, loop or parallel region was not single-entry/single-exit (or a single-branch parallel), and boot logged a warning and ran without it. os lint stays blind to the check; the parity test pins that gap. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ff88105 commit 839ba45

6 files changed

Lines changed: 372 additions & 0 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
fix(cli): `os validate` and `os build` refuse a flow whose structured region the automation engine refuses to register (ADR-0031, #28893)
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: none
9+
10+
**BREAKING** accept-set narrowing at `os validate` and `os build` (`@objectstack/cli`): a stack carrying a flow whose `loop` body, `parallel` branches or `try_catch` region is not single-entry/single-exit or is cyclic, at any depth, or whose `parallel` has fewer than two branches, now fails validation and the build instead of passing and emitting an artifact.
11+
12+
Both commands now call `findFlowRegionRefusals`, which hands each flow (each `packages[]` entry's flows, or the stack's own) to the automation engine's own registration verdict: `FlowSchema`, then the spec's `validateControlFlow`. A refused flow prints `Flow '<name>' would be refused at registration: <where>: <the engine's message>` (`VALIDATION_ERROR`, exit 1), and the build writes nothing to `dist/`. Before, both passed it, and the engine refused it at boot with an `[Automation] failed to register flow` warning while the server ran without the flow. What the engine registers is unchanged. The fix is the engine's own sentence, e.g. add one node every exit of the region leads to.
13+
14+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: `os validate` and `os build` now run the check `registerFlow` already runs, with the same function (`validateControlFlow` from `@objectstack/spec/automation`) and its message. Every flow they now refuse was already refused at registration and never ran, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone; the remedy is the engine's own sentence (close the region with one exit node, or give the parallel a second branch). -->

‎packages/cli/src/commands/compile.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ import { findOverrideDeclarationRefusals } from '../utils/override-declarations.
8383
// `os validate` step 3a-iv runs, over the save and install doors' own judge
8484
// (`judgeExtensionFieldSuffix`).
8585
import { findExtensionFieldSuffixRefusals } from '../utils/extension-field-suffix.js';
86+
// [#28893] The ADR-0031 region-shape check — the walk `os validate` step 3a-vi
87+
// runs, over the automation engine's own registration verdict.
88+
import { findFlowRegionRefusals } from '../utils/flow-region-shape.js';
8689

8790
export default class Compile extends Command {
8891
static override description = 'Compile ObjectStack configuration to JSON artifact';
@@ -837,6 +840,29 @@ export default class Compile extends Command {
837840
this.exit(1);
838841
}
839842

843+
// 3b-iii-ter. [#28893] Flow region shape (ADR-0031), at build time — the
844+
// SAME call `os validate` makes at its step 3a-vi: the engine's
845+
// registration verdict (`FlowSchema`, then the spec's
846+
// `validateControlFlow`). Before this the build emitted a flow the
847+
// engine then refused at boot, logging a warning and running without
848+
// it; now the door that ships refuses it and writes no artifact. Same
849+
// two faces as the gates above.
850+
const regionRefusals = findFlowRegionRefusals(result.data as Record<string, unknown>);
851+
if (regionRefusals.length > 0) {
852+
if (flags.json) {
853+
await emitJson({ success: false, errors: regionRefusals, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
854+
this.exit(1);
855+
}
856+
const n = regionRefusals.length;
857+
console.log('');
858+
printError(`The automation engine would refuse to register ${n} flow${n > 1 ? 's' : ''} for a malformed region (ADR-0031)`);
859+
printBulletList(
860+
regionRefusals.map((r) => `${r.path} [${r.code}] ${r.message}`),
861+
{ noun: 'flow region refusal(s)', remedy: JSON_FULL_LIST_REMEDY },
862+
);
863+
this.exit(1);
864+
}
865+
840866
// 3b-quater. ADR-0141 D2, author side — every `manifest.dependencies`
841867
// range of the packages one install act would deliver, judged by the
842868
// install's own predicate (`utils/dependency-range-check.ts`).

‎packages/cli/src/commands/validate.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ import { findExtensionFieldSuffixRefusals } from '../utils/extension-field-suffi
8181
// [ADR-0146 D10, second rule] The field-patch target check's walk; the verdict
8282
// is the marketplace install door's own judge.
8383
import { findFieldPatchTargetRefusals } from '../utils/field-patch-targets.js';
84+
// [#28893] The ADR-0031 region-shape check: the automation engine's own
85+
// registration verdict (`FlowSchema` then the spec's `validateControlFlow`).
86+
import { findFlowRegionRefusals } from '../utils/flow-region-shape.js';
8487
// A field `picklist` that names no picklist the stack declares — refused, or
8588
// reported when the declaring package depends on packages outside the stack.
8689
// Walked the way the load path registers, like the refusal above.
@@ -751,6 +754,36 @@ export default class Validate extends Command {
751754
this.exit(1);
752755
}
753756

757+
// 3a-vi. [#28893] Flow region shape (ADR-0031): every `loop` body,
758+
// `parallel` branch and `try_catch` region, at every depth, must be
759+
// single-entry/single-exit and acyclic, and a `parallel` needs two
760+
// branches. The verdict is the engine's registration call
761+
// (`FlowSchema`, then the spec's `validateControlFlow`), so a flow
762+
// green here is one boot registers instead of logging
763+
// `[Automation] failed to register flow` and running without it; see
764+
// `utils/flow-region-shape.ts`.
765+
const regionRefusals = findFlowRegionRefusals(result.data as Record<string, unknown>);
766+
if (regionRefusals.length > 0) {
767+
if (flags.json) {
768+
await emitJson({
769+
valid: false,
770+
errors: regionRefusals,
771+
warnings: warningsSoFar(),
772+
conversions: conversionNotices,
773+
duration: timer.elapsed(),
774+
});
775+
this.exit(1);
776+
}
777+
const n = regionRefusals.length;
778+
console.log('');
779+
printError(`The automation engine would refuse to register ${n} flow${n > 1 ? 's' : ''} for a malformed region (ADR-0031)`);
780+
printBulletList(
781+
regionRefusals.map((r) => `${r.path} [${r.code}] ${r.message}`),
782+
{ noun: 'flow region refusal(s)', remedy: JSON_FULL_LIST_REMEDY },
783+
);
784+
this.exit(1);
785+
}
786+
754787
// 3b. [#3366] Installable-provider preflight — the shift-left of the
755788
// `serve`-time capability check. `os validate` previously only checked
756789
// the `requires` tokens against the vocabulary (ADR-0066), never
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #28893 — the ADR-0031 region-shape check at `os validate` and `os build`.
5+
*
6+
* The automation engine refuses to register a flow whose structured regions
7+
* (`loop` bodies, `parallel` branches, `try_catch` try/catch regions, at every
8+
* depth) are not single-entry/single-exit and acyclic, or whose `parallel` has
9+
* fewer than two branches: `registerFlow` parses the body with `FlowSchema`
10+
* and calls the spec's `validateControlFlow`. Before this module validate and
11+
* build passed such a flow, and the first sign of it was a boot warning
12+
* (`[Automation] failed to register flow`) with the flow silently absent.
13+
*
14+
* The verdict here is that same call — `FlowSchema` then `validateControlFlow`
15+
* from `@objectstack/spec/automation` — so there is no second copy of the
16+
* region rules, and its message (`<where>: <errors>`) is the engine's,
17+
* verbatim, behind the flow's name. Like the engine it stops at a flow's first
18+
* malformed region; each flow is judged.
19+
*
20+
* Walked the way `findExtensionFieldSuffixRefusals` (`./extension-field-suffix.ts`)
21+
* walks: each `packages[]` entry's flows, or the stack's own flows when it
22+
* declares no packages. A flow the schema refuses is left to the schema step,
23+
* which has already reported it.
24+
*
25+
* Not judged here: a cycle in the flow's TOP-LEVEL graph. The engine refuses it
26+
* with `detectCycles`, which lives in `@objectstack/service-automation` and is
27+
* not exported; region bodies ARE cycle-checked, by `validateControlFlow`.
28+
*/
29+
30+
import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation';
31+
32+
import { artifactPackages } from './artifact-packages.js';
33+
34+
/** One refused flow, located. */
35+
export interface FlowRegionRefusalRow {
36+
/** Where the flow sits in the parsed stack, e.g. `flows[0]` or `packages[1].manifest.flows[0]`. */
37+
path: string;
38+
/** The code the CLI's other author-time refusals carry. */
39+
code: 'VALIDATION_ERROR';
40+
/** The flow's name. */
41+
flow: string;
42+
/** `Flow '<name>' …: ` followed by the engine's refusal, verbatim. */
43+
message: string;
44+
}
45+
46+
/** Every flow of the parsed stack the engine would refuse to register for its region shape. */
47+
export function findFlowRegionRefusals(parsed: Record<string, unknown>): FlowRegionRefusalRow[] {
48+
const entries = artifactPackages(parsed);
49+
const subjects = entries.length > 0
50+
? entries.map(({ index, body }) => ({ flows: body.flows, at: `packages[${index}].manifest.` }))
51+
: [{ flows: parsed.flows, at: '' }];
52+
const rows: FlowRegionRefusalRow[] = [];
53+
for (const { flows, at } of subjects) {
54+
if (!Array.isArray(flows)) continue;
55+
flows.forEach((raw, i) => {
56+
const result = FlowSchema.safeParse(raw);
57+
if (!result.success) return;
58+
try {
59+
validateControlFlow(result.data);
60+
} catch (err) {
61+
const name = result.data.name;
62+
rows.push({
63+
path: `${at}flows[${i}]`,
64+
code: 'VALIDATION_ERROR',
65+
flow: name,
66+
message: `Flow '${name}' would be refused at registration: ${err instanceof Error ? err.message : String(err)}`,
67+
});
68+
}
69+
});
70+
}
71+
return rows;
72+
}

‎packages/cli/test/authoring-rule-command-parity.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,53 @@ const CASES: ReadonlyArray<{ rule: string; blindTo: readonly AuthoringCommand[];
142142
},
143143
];
144144

145+
/**
146+
* Checks `os validate` and `os build` gate on OUTSIDE the rule table, so
147+
* `os lint`, which runs only the table, is blind to them. `blindTo` is the same
148+
* documentation as above; the pin below asserts the blindness, so closing it
149+
* (registering the check as an authoring rule) reds here and the case moves
150+
* into `CASES`.
151+
*
152+
* - flow region shape (#28893): a `try_catch` region that is not single-exit.
153+
* validate and build refuse it with the automation engine's own registration
154+
* verdict (`utils/flow-region-shape.ts`, pinned by
155+
* `validate-flow-region-shape.test.ts`); `os lint` passes it.
156+
*/
157+
const OUTSIDE_THE_TABLE: ReadonlyArray<{ check: string; marker: string; blindTo: readonly AuthoringCommand[]; stack: Record<string, unknown> }> = [
158+
{
159+
check: 'flow region shape (#28893)',
160+
marker: 'single-exit',
161+
blindTo: ['lint'],
162+
stack: withBaseline({
163+
objects: [{ name: 'parity_task', label: 'Task', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name' } } }],
164+
flows: [{
165+
name: 'parity_region', label: 'Region', type: 'record_change', runAs: 'system', status: 'active',
166+
nodes: [
167+
{ id: 'start', type: 'start', label: 'Start', config: { objectName: 'parity_task', triggerType: 'record-after-create' } },
168+
{
169+
id: 'guard', type: 'try_catch', label: 'Guard',
170+
config: {
171+
try: { nodes: [{ id: 'work', type: 'assignment', label: 'Work' }], edges: [] },
172+
catch: {
173+
nodes: ['d', 'x', 'y'].map((id) => ({ id, type: 'assignment', label: id })),
174+
edges: [
175+
{ id: 'dx', source: 'd', target: 'x', type: 'default' },
176+
{ id: 'dy', source: 'd', target: 'y', type: 'default' },
177+
],
178+
},
179+
},
180+
},
181+
{ id: 'end', type: 'end', label: 'End' },
182+
],
183+
edges: [
184+
{ id: 'e1', source: 'start', target: 'guard', type: 'default' },
185+
{ id: 'e2', source: 'guard', target: 'end', type: 'default' },
186+
],
187+
}],
188+
}),
189+
},
190+
];
191+
145192
describe('every authoring command reaches the same verdict (#4409)', () => {
146193
it.each(CASES.map((c) => [c.rule, c] as const))('%s gates on all three commands', (_rule, testCase) => {
147194
for (const command of AUTHORING_COMMANDS) {
@@ -160,6 +207,16 @@ describe('every authoring command reaches the same verdict (#4409)', () => {
160207
}
161208
});
162209

210+
it.each(OUTSIDE_THE_TABLE.map((c) => [c.check, c] as const))('%s: a known gap, os lint is still blind to it', (_check, testCase) => {
211+
for (const command of testCase.blindTo) {
212+
const findings = runAuthoringRules(command, {
213+
normalized: testCase.stack,
214+
...(command === 'lint' ? {} : { parsed: testCase.stack }),
215+
});
216+
expect(findings.filter((f) => f.message.includes(testCase.marker))).toEqual([]);
217+
}
218+
});
219+
163220
it('the case table still covers every command as a blind spot', () => {
164221
// Guard the guard: if the table drifted to cover only one direction, the
165222
// suite above would keep passing while testing half the problem. The issue

0 commit comments

Comments
 (0)