Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .changeset/28893-flow-region-shape-at-validate-build.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
'@objectstack/cli': patch
---

fix(cli): `os validate` and `os build` refuse a flow whose structured region the automation engine refuses to register (ADR-0031, #28893)

Clause-②: no (narrowing)
Metadata-protocol: none

**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.

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.

<!-- 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). -->
26 changes: 26 additions & 0 deletions packages/cli/src/commands/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ import { findOverrideDeclarationRefusals } from '../utils/override-declarations.
// `os validate` step 3a-iv runs, over the save and install doors' own judge
// (`judgeExtensionFieldSuffix`).
import { findExtensionFieldSuffixRefusals } from '../utils/extension-field-suffix.js';
// [#28893] The ADR-0031 region-shape check — the walk `os validate` step 3a-vi
// runs, over the automation engine's own registration verdict.
import { findFlowRegionRefusals } from '../utils/flow-region-shape.js';

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

// 3b-iii-ter. [#28893] Flow region shape (ADR-0031), at build time — the
// SAME call `os validate` makes at its step 3a-vi: the engine's
// registration verdict (`FlowSchema`, then the spec's
// `validateControlFlow`). Before this the build emitted a flow the
// engine then refused at boot, logging a warning and running without
// it; now the door that ships refuses it and writes no artifact. Same
// two faces as the gates above.
const regionRefusals = findFlowRegionRefusals(result.data as Record<string, unknown>);
if (regionRefusals.length > 0) {
if (flags.json) {
await emitJson({ success: false, errors: regionRefusals, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true });
this.exit(1);
}
const n = regionRefusals.length;
console.log('');
printError(`The automation engine would refuse to register ${n} flow${n > 1 ? 's' : ''} for a malformed region (ADR-0031)`);
printBulletList(
regionRefusals.map((r) => `${r.path} [${r.code}] ${r.message}`),
{ noun: 'flow region refusal(s)', remedy: JSON_FULL_LIST_REMEDY },
);
this.exit(1);
}

// 3b-quater. ADR-0141 D2, author side — every `manifest.dependencies`
// range of the packages one install act would deliver, judged by the
// install's own predicate (`utils/dependency-range-check.ts`).
Expand Down
33 changes: 33 additions & 0 deletions packages/cli/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ import { findExtensionFieldSuffixRefusals } from '../utils/extension-field-suffi
// [ADR-0146 D10, second rule] The field-patch target check's walk; the verdict
// is the marketplace install door's own judge.
import { findFieldPatchTargetRefusals } from '../utils/field-patch-targets.js';
// [#28893] The ADR-0031 region-shape check: the automation engine's own
// registration verdict (`FlowSchema` then the spec's `validateControlFlow`).
import { findFlowRegionRefusals } from '../utils/flow-region-shape.js';
// A field `picklist` that names no picklist the stack declares — refused, or
// reported when the declaring package depends on packages outside the stack.
// Walked the way the load path registers, like the refusal above.
Expand Down Expand Up @@ -751,6 +754,36 @@ export default class Validate extends Command {
this.exit(1);
}

// 3a-vi. [#28893] Flow region shape (ADR-0031): every `loop` body,
// `parallel` branch and `try_catch` region, at every depth, must be
// single-entry/single-exit and acyclic, and a `parallel` needs two
// branches. The verdict is the engine's registration call
// (`FlowSchema`, then the spec's `validateControlFlow`), so a flow
// green here is one boot registers instead of logging
// `[Automation] failed to register flow` and running without it; see
// `utils/flow-region-shape.ts`.
const regionRefusals = findFlowRegionRefusals(result.data as Record<string, unknown>);
if (regionRefusals.length > 0) {
if (flags.json) {
await emitJson({
valid: false,
errors: regionRefusals,
warnings: warningsSoFar(),
conversions: conversionNotices,
duration: timer.elapsed(),
});
this.exit(1);
}
const n = regionRefusals.length;
console.log('');
printError(`The automation engine would refuse to register ${n} flow${n > 1 ? 's' : ''} for a malformed region (ADR-0031)`);
printBulletList(
regionRefusals.map((r) => `${r.path} [${r.code}] ${r.message}`),
{ noun: 'flow region refusal(s)', remedy: JSON_FULL_LIST_REMEDY },
);
this.exit(1);
}

// 3b. [#3366] Installable-provider preflight — the shift-left of the
// `serve`-time capability check. `os validate` previously only checked
// the `requires` tokens against the vocabulary (ADR-0066), never
Expand Down
72 changes: 72 additions & 0 deletions packages/cli/src/utils/flow-region-shape.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #28893 — the ADR-0031 region-shape check at `os validate` and `os build`.
*
* The automation engine refuses to register a flow whose structured regions
* (`loop` bodies, `parallel` branches, `try_catch` try/catch regions, at every
* depth) are not single-entry/single-exit and acyclic, or whose `parallel` has
* fewer than two branches: `registerFlow` parses the body with `FlowSchema`
* and calls the spec's `validateControlFlow`. Before this module validate and
* build passed such a flow, and the first sign of it was a boot warning
* (`[Automation] failed to register flow`) with the flow silently absent.
*
* The verdict here is that same call — `FlowSchema` then `validateControlFlow`
* from `@objectstack/spec/automation` — so there is no second copy of the
* region rules, and its message (`<where>: <errors>`) is the engine's,
* verbatim, behind the flow's name. Like the engine it stops at a flow's first
* malformed region; each flow is judged.
*
* Walked the way `findExtensionFieldSuffixRefusals` (`./extension-field-suffix.ts`)
* walks: each `packages[]` entry's flows, or the stack's own flows when it
* declares no packages. A flow the schema refuses is left to the schema step,
* which has already reported it.
*
* Not judged here: a cycle in the flow's TOP-LEVEL graph. The engine refuses it
* with `detectCycles`, which lives in `@objectstack/service-automation` and is
* not exported; region bodies ARE cycle-checked, by `validateControlFlow`.
*/

import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation';

import { artifactPackages } from './artifact-packages.js';

/** One refused flow, located. */
export interface FlowRegionRefusalRow {
/** Where the flow sits in the parsed stack, e.g. `flows[0]` or `packages[1].manifest.flows[0]`. */
path: string;
/** The code the CLI's other author-time refusals carry. */
code: 'VALIDATION_ERROR';
/** The flow's name. */
flow: string;
/** `Flow '<name>' …: ` followed by the engine's refusal, verbatim. */
message: string;
}

/** Every flow of the parsed stack the engine would refuse to register for its region shape. */
export function findFlowRegionRefusals(parsed: Record<string, unknown>): FlowRegionRefusalRow[] {
const entries = artifactPackages(parsed);
const subjects = entries.length > 0
? entries.map(({ index, body }) => ({ flows: body.flows, at: `packages[${index}].manifest.` }))
: [{ flows: parsed.flows, at: '' }];
const rows: FlowRegionRefusalRow[] = [];
for (const { flows, at } of subjects) {
if (!Array.isArray(flows)) continue;
flows.forEach((raw, i) => {
const result = FlowSchema.safeParse(raw);
if (!result.success) return;
try {
validateControlFlow(result.data);
} catch (err) {
const name = result.data.name;
rows.push({
path: `${at}flows[${i}]`,
code: 'VALIDATION_ERROR',
flow: name,
message: `Flow '${name}' would be refused at registration: ${err instanceof Error ? err.message : String(err)}`,
});
}
});
}
return rows;
}
57 changes: 57 additions & 0 deletions packages/cli/test/authoring-rule-command-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,53 @@ const CASES: ReadonlyArray<{ rule: string; blindTo: readonly AuthoringCommand[];
},
];

/**
* Checks `os validate` and `os build` gate on OUTSIDE the rule table, so
* `os lint`, which runs only the table, is blind to them. `blindTo` is the same
* documentation as above; the pin below asserts the blindness, so closing it
* (registering the check as an authoring rule) reds here and the case moves
* into `CASES`.
*
* - flow region shape (#28893): a `try_catch` region that is not single-exit.
* validate and build refuse it with the automation engine's own registration
* verdict (`utils/flow-region-shape.ts`, pinned by
* `validate-flow-region-shape.test.ts`); `os lint` passes it.
*/
const OUTSIDE_THE_TABLE: ReadonlyArray<{ check: string; marker: string; blindTo: readonly AuthoringCommand[]; stack: Record<string, unknown> }> = [
{
check: 'flow region shape (#28893)',
marker: 'single-exit',
blindTo: ['lint'],
stack: withBaseline({
objects: [{ name: 'parity_task', label: 'Task', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name' } } }],
flows: [{
name: 'parity_region', label: 'Region', type: 'record_change', runAs: 'system', status: 'active',
nodes: [
{ id: 'start', type: 'start', label: 'Start', config: { objectName: 'parity_task', triggerType: 'record-after-create' } },
{
id: 'guard', type: 'try_catch', label: 'Guard',
config: {
try: { nodes: [{ id: 'work', type: 'assignment', label: 'Work' }], edges: [] },
catch: {
nodes: ['d', 'x', 'y'].map((id) => ({ id, type: 'assignment', label: id })),
edges: [
{ id: 'dx', source: 'd', target: 'x', type: 'default' },
{ id: 'dy', source: 'd', target: 'y', type: 'default' },
],
},
},
},
{ id: 'end', type: 'end', label: 'End' },
],
edges: [
{ id: 'e1', source: 'start', target: 'guard', type: 'default' },
{ id: 'e2', source: 'guard', target: 'end', type: 'default' },
],
}],
}),
},
];

describe('every authoring command reaches the same verdict (#4409)', () => {
it.each(CASES.map((c) => [c.rule, c] as const))('%s gates on all three commands', (_rule, testCase) => {
for (const command of AUTHORING_COMMANDS) {
Expand All @@ -160,6 +207,16 @@ describe('every authoring command reaches the same verdict (#4409)', () => {
}
});

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) => {
for (const command of testCase.blindTo) {
const findings = runAuthoringRules(command, {
normalized: testCase.stack,
...(command === 'lint' ? {} : { parsed: testCase.stack }),
});
expect(findings.filter((f) => f.message.includes(testCase.marker))).toEqual([]);
}
});

it('the case table still covers every command as a blind spot', () => {
// Guard the guard: if the table drifted to cover only one direction, the
// suite above would keep passing while testing half the problem. The issue
Expand Down
9 changes: 9 additions & 0 deletions packages/cli/test/validate-build-gate-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,15 @@ const SHARED_NON_REGISTRY_GATES: readonly string[] = [
// (#28727), so the row moved here — deleted there rather than reworded, for
// the reason the `runPerPackageAuthoringRules` row above gives.
'findExtensionFieldSuffixRefusals',
// [#28893] Flow region shape (ADR-0031): every `loop` body, `parallel`
// branch and `try_catch` region single-entry/single-exit and acyclic, and a
// `parallel` with two branches, judged by the automation engine's own
// registration call (`FlowSchema`, then the spec's `validateControlFlow`).
// Wired into both commands in one card. Not a registry rule today, so
// `os lint` is blind to it: the parity test pins that gap
// (`authoring-rule-command-parity.test.ts`, `OUTSIDE_THE_TABLE`), and moving
// it into `authoring-rules.ts` is the reported follow-up.
'findFlowRegionRefusals',
];

/**
Expand Down
Loading
Loading