Skip to content

Commit 5d3667e

Browse files
fix(cli)!: os validate and os build refuse a flow whose region the automation engine refuses to register (#29043)
domain:server#5 · os-project-manager · session_01FEkQ5oc56xChD6Wrc4cGSt <!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X8dNy8r76WjFsVdvaSJbXXR)_ Fixes #28893 Clause-②: no (narrowing) Metadata-protocol: none Skills-docs: none — no file under `skills/**` teaches what `os validate` / `os build` refuse about flow regions; the region rules themselves are unchanged. ## 维护者速读 - 问题:`os validate` 和 `os build` 放行一个 `try_catch` / `loop` / `parallel` 区域不是单入口单出口(或单分支 parallel)的 flow,build 还照常产出 artifact;引擎在 `registerFlow` 里用 `validateControlFlow` 拒绝它,启动只记一条 `[Automation] failed to register flow` 警告,然后不带这个 flow 继续运行,触发器从不触发。 - 现在:两条命令对每个 flow 调用引擎注册时的同一判定(`FlowSchema`,再 spec 导出的 `validateControlFlow`),任意深度;拒绝信息就是引擎原文(`<where>: <errors>`)前面加 flow 名,退出码 1,build 不写 `dist/`。规则没有第二份拷贝,未改 `packages/spec/**`。 - 这是作者期的收窄(`Clause-②: no (narrowing)`,标题带 `!`):新拒绝的每个 flow 本来就注册失败、从未运行,运行中的 flow 不受影响。 - 未做(已列入 out_of_scope):`os lint` 仍看不见这个检查(parity 测试里以 `blindTo: ['lint']` 钉住);顶层图的环(`detectCycles` 在 service-automation 内且未导出)validate 仍放行;启动横幅不计"注册失败"数,只在 Boot diagnostics 里逐条出现。 ## What changes - **`packages/cli/src/utils/flow-region-shape.ts`** (new): `findFlowRegionRefusals(parsed)` walks each `packages[]` entry's flows, or the stack's own flows when it declares no packages (the walk `findExtensionFieldSuffixRefusals` takes), and hands each one to the engine's registration verdict: `FlowSchema.safeParse`, then `validateControlFlow` from `@objectstack/spec/automation`. A refusal is a row `{ path, code: 'VALIDATION_ERROR', flow, message }` whose message is `Flow '<name>' would be refused at registration: ` followed by the engine's message verbatim. Like the engine it stops at a flow's first malformed region; every flow is judged. A flow the schema refuses is left to the schema step. - **`commands/validate.ts`** step 3a-vi and **`commands/compile.ts`** (`os build`) step 3b-iii-ter call it beside the `__c` reservation gate, ahead of every artifact write, with the same two faces as the gates around them: the command's `--json` failure envelope (`errors` rows), and the text face (`printError` + one bullet per flow, exit 1). - **`test/validate-build-gate-parity.test.ts`**: `findFlowRegionRefusals` joins `SHARED_NON_REGISTRY_GATES`, the ledger of gates both commands run. Placement (A), in lane, following #28727: a `cli/src/utils` judge over the spec's exported function. No `packages/spec/**`, `packages/lint/**` or `service-automation` edit. ## Tests - `test/validate-flow-region-shape.test.ts` (new, in-process `Validate` / `Compile`, 4 cases): - the duly shape (`try_catch 'guard'` whose catch region is `d → x, y, z` with no join): validate `--json` exits 1 with a row carrying `try_catch 'guard' catch: region must be single-exit but has 3: x, y, z` and the flow name; build `--json` exits 1, `success: false`, the same row, and no `dist/`; - nested (a `loop` whose body is that fan-out, inside a `try_catch` try region): the same, with `try_catch 'outer' try → loop 'sweep' body: …`; - single-branch `parallel`: `parallel 'fan': a parallel block needs at least 2 branches`, both commands; - control: the joined region (`x, y, z → j`) validates and builds. - On `origin/main` (`ff881050`): the three refused cases red (validate exited 0, build succeeded), the control green. - Ablations, each landed on disk then restored: the call in `validate.ts` replaced by `[]` → 3 red; the call in `compile.ts` replaced by `[]` → 3 red (`expected +0 to be 1`). - `test/authoring-rule-command-parity.test.ts`: per the seat's ruling, `os lint`'s blindness is recorded as `blindTo: ['lint']`. It sits in a second table, `OUTSIDE_THE_TABLE`, not in `CASES`: every `CASES` row asserts the rule gates on all three commands through `runAuthoringRules`, and this check is not an authoring rule, so a `CASES` row would be red by construction. The new row ASSERTS the blindness (`runAuthoringRules('lint', …)` carries no `single-exit` finding), so closing the gap reds it and the case moves into `CASES`. - One `os validate` run (real CLI, text face) over the duly fixture: ``` base (ff88105) head → Running author-time rules (60)... → Running author-time rules (60)... → Checking that every required capability… → Checking package docs (ADR-0046)... ✗ The automation engine would refuse to register 1 flow for a malformed region (ADR-0031) ✓ Validation passed (166ms) • flows[0] [VALIDATION_ERROR] Flow 'rgn_lead_time' would be refused at registration: Logic: 1 Flows try_catch 'guard' catch: region must be single-exit but has 3: x, y, z — add one node exit 0 with an edge from each of x, y, z to it, so it is the region's only exit; … exit 1 ``` ## Acceptance notes - **Clause-② reading.** The order asked for `yes (narrowing)` if the gates read it so. `clause2-line.mjs` reads `yes (narrowing)` as "widens one surface and narrows another"; nothing here widens (no new export, schema key or accepted input; `flow` on the error row is a new member of an existing row list, like `rule` / `packageId` on its neighbours). So the line is `no (narrowing)`, the spelling #28727 used for the same kind of change; the changeset carries the `**BREAKING**` banner and an ADR-0087 `not-required` disposition. - **Boot side (ruling 3), measured, not changed.** The banner's `Flows:` line (`utils/format.ts` `printAutomationSummary`, ~1718) counts registered flows (`flowCount` = `getFlowRuntimeStates().length`), bound flows and drafts. `declaredFlowCount` (`commands/serve.ts` ~5708, `resolveStackCollection(config, 'flows')`) is read only for the engine-disabled line. A refused flow is in neither count; it surfaces only as its `[Automation] failed to register flow` line in Boot diagnostics. A refused count would be one more `parts.push` beside the draft count, but no one-line, same-file version is exact: `declaredFlowCount - flowCount` is moved by runtime overlay rows and same-named contenders too, and the engine exposes refusals only per name (`getFlowRegistration(name)`), so an exact count needs a new summary field collected in `serve.ts`. Left out, reported. Whether boot continues is untouched. - **Not closed here, reported**: the lint-rule parity (`os lint` blind, B, domain:devx), and a cycle in the flow's TOP-LEVEL graph, which `os validate` still passes (`start → a → b → a → end`, exit 0): the engine refuses it with `detectCycles` (`packages/services/service-automation/src/flow-registration-checks.ts`), which is exported from that file but not from the package index. Region bodies are cycle-checked, by `validateControlFlow`. - Seat 3's files (`subflow-guards.ts`, `region-suspension-refusal.ts`, the arming gate) are untouched. ## Verification Gates: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 62 commands on the diff; all 62 exit 0, and `--ran` reconciles 62 derived, 62 run, 0 NOT-MEASURED. `pnpm --filter @objectstack/cli typecheck` (runs `check:test-typecheck`): exit 0. `pnpm --filter @objectstack/spec check:error-code-provenance`: exit 0. Scoped eslint over the six changed `.ts` files: exit 0. `pnpm --filter @objectstack/cli exec vitest run --project unit`, after merging `main` (`00d0668d`, nothing under `packages/cli`, `packages/spec` or `packages/lint`): first run 368/369 files, 1 red: `validate-build-gate-parity.test.ts` "every call site … is classified" named `findFlowRegionRefusals`. Fixed by adding it to `SHARED_NON_REGISTRY_GATES` (a gate both commands run); re-run on the final head: 369/369 files, 5201/5201 tests, VERDICT command-exit 0. The `integration` tier (where `authoring-rule-command-parity.test.ts` lives, for its spawn case) is declared to CI; its in-process rule cases, the new pin included, ran locally green (10 passed). Dogfood battery (owed for the `(narrowing)` arm): after building the `@objectstack/dogfood...` closure (68/68 tasks), `pnpm --filter @objectstack/dogfood test` under the lock, VERDICT command-exit 0 · Test Files 383 passed | 3 skipped (386) · Tests 3163 passed | 28 skipped (3191). It ran on `832bd06d`; the final head adds the ledger row in a `packages/cli` test file and one more merge of `main` (`ab085a04`: under `packages/spec` only `shared/region-currency.*` and a save-notice placement change with a `FlowSchema` comment, nothing the region check reads). --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5kV6ptpTnU6Hjb2mE4)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 636432c commit 5d3667e

7 files changed

Lines changed: 381 additions & 0 deletions
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

‎packages/cli/test/validate-build-gate-parity.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,15 @@ const SHARED_NON_REGISTRY_GATES: readonly string[] = [
213213
// (#28727), so the row moved here — deleted there rather than reworded, for
214214
// the reason the `runPerPackageAuthoringRules` row above gives.
215215
'findExtensionFieldSuffixRefusals',
216+
// [#28893] Flow region shape (ADR-0031): every `loop` body, `parallel`
217+
// branch and `try_catch` region single-entry/single-exit and acyclic, and a
218+
// `parallel` with two branches, judged by the automation engine's own
219+
// registration call (`FlowSchema`, then the spec's `validateControlFlow`).
220+
// Wired into both commands in one card. Not a registry rule today, so
221+
// `os lint` is blind to it: the parity test pins that gap
222+
// (`authoring-rule-command-parity.test.ts`, `OUTSIDE_THE_TABLE`), and moving
223+
// it into `authoring-rules.ts` is the reported follow-up.
224+
'findFlowRegionRefusals',
216225
];
217226

218227
/**

0 commit comments

Comments
 (0)