diff --git a/.changeset/29051-cli-flow-undeclared-cycle.md b/.changeset/29051-cli-flow-undeclared-cycle.md new file mode 100644 index 00000000000..ea92126a5b1 --- /dev/null +++ b/.changeset/29051-cli-flow-undeclared-cycle.md @@ -0,0 +1,19 @@ +--- +'@objectstack/cli': patch +--- + +**BREAKING** fix(cli)!: `os validate` and `os build` refuse a flow whose top-level graph closes a cycle with no `type: 'back'` edge, as the automation engine refuses it at registration + +Clause-②: no (narrowing) +Metadata-protocol: none +Skills-docs: none — `skills/objectstack-automation/references/state-machines-and-approvals.md` already teaches that a cycle-closing edge MUST be typed `'back'` and that an unmarked cycle is refused; that stays true, and the CLI now refuses the same flow before it ships. + + + +**BREAKING** accept-set narrowing at the CLI's author-time doors (`@objectstack/cli`). + +- **Before.** A flow whose top-level graph had a cycle with no edge typed `back` (ADR-0044), such as `start → push → check → revise → push`, passed `os validate` and `os build` with exit 0. The engine then refused to register it at boot ("Flow contains a cycle: …"), logged a warning, and ran without it. +- **Now.** Both commands refuse it, exit 1, and `os build` writes no artifact. The row is `VALIDATION_ERROR`, located at the flow (`flows[N]`, or `packages[K].manifest.flows[N]` in an assembled artifact), and its message is the engine's registration refusal, word for word. The judge is `findUndeclaredFlowCycle` from `@objectstack/metadata-core`, the one the engine's registration and the runtime publish gate call, so all three refuse exactly the same graphs. It is a CLI step, not an `@objectstack/lint` rule, since lint does not depend on metadata-core; the rule table runs first, so a flow with both a malformed region (`flow-region-shape-invalid`) and an undeclared cycle is refused for the region first, and for the cycle once the region is fixed (the engine names the cycle first). +- **Unchanged.** A cycle with any of its edges typed `back` validates and builds, as before. Cycles inside a `loop` / `parallel` / `try_catch` region are judged by the region check, as before. + +**The remedy.** None for a flow that registers today. A flow this now refuses never ran: if the loop is intended, set `type: 'back'` on the cycle-closing edge (ADR-0044); otherwise remove or redirect that edge. diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 7db48c0ee42..5fadf9a6d07 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -84,6 +84,10 @@ import { findOverrideDeclarationRefusals } from '../utils/override-declarations. // (`judgeExtensionFieldSuffix`). import { findExtensionFieldSuffixRefusals } from '../utils/extension-field-suffix.js'; +// [#29051] An undeclared top-level flow cycle — the walk `os validate` step +// 3a-vii runs, over the automation engine's own judge. +import { findFlowCycleRefusals } from '../utils/flow-undeclared-cycle.js'; + export default class Compile extends Command { static override description = 'Compile ObjectStack configuration to JSON artifact'; @@ -841,6 +845,27 @@ export default class Compile extends Command { // rule table at step 3b since #29049 (`flow-region-shape-invalid`), // the same verdict `os validate` and `os lint` give. + // 3b-iii-quater. [#29051] An undeclared top-level flow cycle (ADR-0044), + // at build time — the SAME call `os validate` makes at its step + // 3a-vii: the engine's registration judge (`findUndeclaredFlowCycle`). + // Before this the build emitted a flow the engine then refused at + // boot; now the door that ships refuses it and writes no artifact. + const cycleRefusals = findFlowCycleRefusals(result.data as Record); + if (cycleRefusals.length > 0) { + if (flags.json) { + await emitJson({ success: false, errors: cycleRefusals, warnings: warningsSoFar(), conversions: conversionNotices }, 0, { compact: true }); + this.exit(1); + } + const n = cycleRefusals.length; + console.log(''); + printError(`The automation engine would refuse to register ${n} flow${n > 1 ? 's' : ''} for a cycle no back edge declares (ADR-0044)`); + printBulletList( + cycleRefusals.map((r) => `${r.path} [${r.code}] ${r.message}`), + { noun: 'flow cycle 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`). diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 1c4c07c104d..68bac14cbc9 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -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'; +// [#29051] An undeclared top-level flow cycle (ADR-0044): the automation +// engine's own judge (`findUndeclaredFlowCycle`, `@objectstack/metadata-core`). +import { findFlowCycleRefusals } from '../utils/flow-undeclared-cycle.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. @@ -756,6 +759,36 @@ export default class Validate extends Command { // `@objectstack/lint`'s `validate-flow-region-shape.ts`), so all // three authoring commands give the engine's registration verdict. + // 3a-vii. [#29051] An undeclared cycle in a flow's TOP-LEVEL graph + // (ADR-0044): a cycle that survives setting the `type: 'back'` edges + // aside. The verdict is the engine's registration judge + // (`findUndeclaredFlowCycle`, `@objectstack/metadata-core`), the + // one the publish doors run too, so a flow green here is one boot + // registers; see `utils/flow-undeclared-cycle.ts`. It is a CLI step, + // not a rule-table entry, because `@objectstack/lint` does not + // depend on metadata-core. + const cycleRefusals = findFlowCycleRefusals(result.data as Record); + if (cycleRefusals.length > 0) { + if (flags.json) { + await emitJson({ + valid: false, + errors: cycleRefusals, + warnings: warningsSoFar(), + conversions: conversionNotices, + duration: timer.elapsed(), + }); + this.exit(1); + } + const n = cycleRefusals.length; + console.log(''); + printError(`The automation engine would refuse to register ${n} flow${n > 1 ? 's' : ''} for a cycle no back edge declares (ADR-0044)`); + printBulletList( + cycleRefusals.map((r) => `${r.path} [${r.code}] ${r.message}`), + { noun: 'flow cycle 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 diff --git a/packages/cli/src/utils/flow-undeclared-cycle.test.ts b/packages/cli/src/utils/flow-undeclared-cycle.test.ts new file mode 100644 index 00000000000..594cff49636 --- /dev/null +++ b/packages/cli/src/utils/flow-undeclared-cycle.test.ts @@ -0,0 +1,48 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29051] `findFlowCycleRefusals`: an undeclared cycle in a flow's TOP-LEVEL + * graph (ADR-0044) is a row, located at the flow, in the engine's sentence. + * + * The cases are the judge's own corpus (`@objectstack/metadata-core/testing`), + * the one the engine's registration and the runtime publish gate are driven + * from, so the CLI refuses exactly the graphs they refuse. What the doors do + * with a row is pinned end to end in `test/validate-flow-region-shape.test.ts`; + * here the subject is the row. + */ + +import { describe, expect, it } from 'vitest'; + +import { undeclaredFlowCycleMessage } from '@objectstack/metadata-core'; +import { FLOW_CYCLE_CORPUS } from '@objectstack/metadata-core/testing'; + +import { findFlowCycleRefusals } from './flow-undeclared-cycle.js'; + +const located = (rows: ReturnType) => rows.map(({ path, code, flow }) => ({ path, code, flow })); + +describe('[#29051] a top-level cycle not declared back is refused, as the engine refuses it', () => { + for (const c of FLOW_CYCLE_CORPUS) { + it(c.title, () => { + const rows = findFlowCycleRefusals({ flows: [c.flow] }); + if (c.cycle === null) { + expect(rows).toEqual([]); + return; + } + expect(located(rows)).toEqual([{ path: 'flows[0]', code: 'VALIDATION_ERROR', flow: c.flow.name }]); + expect(rows[0]!.message).toBe(`Flow '${c.flow.name}' would be refused at registration: ${undeclaredFlowCycleMessage(c.cycle)}`); + }); + } + + it('an artifact\'s packages[] entry: located under its own entry, and the top level is not walked', () => { + const refused = FLOW_CYCLE_CORPUS.find((c) => c.cycle !== null)!; + const passes = FLOW_CYCLE_CORPUS.find((c) => c.cycle === null)!; + const rows = findFlowCycleRefusals({ + flows: [refused.flow], + packages: [ + { manifest: { id: 'com.example.a', flows: [passes.flow] } }, + { manifest: { id: 'com.example.b', flows: [passes.flow, refused.flow] } }, + ], + }); + expect(located(rows)).toEqual([{ path: 'packages[1].manifest.flows[1]', code: 'VALIDATION_ERROR', flow: refused.flow.name }]); + }); +}); diff --git a/packages/cli/src/utils/flow-undeclared-cycle.ts b/packages/cli/src/utils/flow-undeclared-cycle.ts new file mode 100644 index 00000000000..5f733ce3ccf --- /dev/null +++ b/packages/cli/src/utils/flow-undeclared-cycle.ts @@ -0,0 +1,75 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #29051 — an undeclared cycle in a flow's TOP-LEVEL graph, at `os validate` + * and `os build` (ADR-0044). + * + * The automation engine refuses to register a flow whose graph, once its + * `type: 'back'` edges are set aside, still has a cycle. It asks + * `findUndeclaredFlowCycle` (`@objectstack/metadata-core`) at registration, and + * so does the runtime authoring gate every publish door runs. Before this + * module `os validate` and `os build` passed such a flow with exit 0; it then + * published or booted, the engine refused it, and it never ran. + * + * The verdict here is that same call on the same parsed body (`FlowSchema`, + * then the judge), so there is no second copy of the DFS, and the message is + * the engine's sentence, verbatim, behind the flow's name. It lives in the CLI, + * not in the `@objectstack/lint` rule table beside `flow-region-shape-invalid`, + * because lint does not depend on `@objectstack/metadata-core`. + * + * Order: the engine judges the cycle before the ADR-0031 region shape. The + * commands run the rule table first, so a flow that has both a malformed + * region and an undeclared cycle is refused for its region; once that is + * fixed, this step refuses the cycle. Both are refusals; only which one an + * author reads first differs. + * + * Walked the way the other CLI flow walks are: each `packages[]` entry's flows, + * or the stack's own flows when it declares no packages. + * + * A flow `FlowSchema` refuses here is not judged: `flow-region-shape-invalid` + * already refused it at the rule-table step, which exits before this one, so + * no flow reaches this walk unjudged. + */ + +import { findUndeclaredFlowCycle } from '@objectstack/metadata-core'; +import { FlowSchema } from '@objectstack/spec/automation'; + +import { artifactPackages } from './artifact-packages.js'; + +/** One refused flow, located. */ +export interface FlowCycleRefusalRow { + /** 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 '' would be refused at registration: ` followed by the engine's refusal, verbatim. */ + message: string; +} + +/** Every flow of the parsed stack the engine would refuse to register for an undeclared top-level cycle. */ +export function findFlowCycleRefusals(parsed: Record): FlowCycleRefusalRow[] { + 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: FlowCycleRefusalRow[] = []; + for (const { flows, at } of subjects) { + if (!Array.isArray(flows)) continue; + flows.forEach((raw, i) => { + const result = FlowSchema.safeParse(raw); + if (!result.success) return; + const cycle = findUndeclaredFlowCycle(result.data); + if (!cycle) return; + const name = result.data.name; + rows.push({ + path: `${at}flows[${i}]`, + code: 'VALIDATION_ERROR', + flow: name, + message: `Flow '${name}' would be refused at registration: ${cycle.message}`, + }); + }); + } + return rows; +} diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 632d6647001..770ea80c9cc 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -213,6 +213,14 @@ 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', + // [#29051] An undeclared cycle in a flow's top-level graph (ADR-0044), + // judged by the automation engine's own registration judge + // (`findUndeclaredFlowCycle`, `@objectstack/metadata-core`), which the + // publish doors run too. Not a registry rule: `@objectstack/lint` does not + // depend on metadata-core, so the rule table cannot reach the judge without + // a new package edge, and a copy of the walk is what the ruling forbids. So + // `os lint` does not give this verdict; validate and build do. + 'findFlowCycleRefusals', ]; /** diff --git a/packages/cli/test/validate-flow-region-shape.test.ts b/packages/cli/test/validate-flow-region-shape.test.ts index 8096d5b8e47..24b3d407998 100644 --- a/packages/cli/test/validate-flow-region-shape.test.ts +++ b/packages/cli/test/validate-flow-region-shape.test.ts @@ -31,6 +31,8 @@ import { fileURLToPath } from 'node:url'; import Compile from '../src/commands/compile'; import Lint from '../src/commands/lint'; import Validate from '../src/commands/validate'; +import { undeclaredFlowCycleMessage } from '@objectstack/metadata-core'; +import { FLOW_CYCLE_CORPUS } from '@objectstack/metadata-core/testing'; import { defineStackSource, linkSpec } from './helpers/define-stack-fixture.js'; const CLI_ROOT = resolve(fileURLToPath(import.meta.url), '..', '..'); @@ -151,6 +153,10 @@ const CASES: Record; refusal?: string } stack: assembledWith(flow('rgn_ok', guard(fanOut(true))), flow('rgn_ops_lead_time', guard(fanOut(false)))), refusal: "try_catch 'guard' catch: region must be single-exit but has 3: x, y, z", }, + // [#29051] The flow-cycle judge's whole corpus in one stack, and the cases it + // must pass on their own. + cycles: { stack: stackWith(FLOW_CYCLE_CORPUS.map((c) => c.flow)) }, + cyclesDeclared: { stack: stackWith(FLOW_CYCLE_CORPUS.filter((c) => c.cycle === null).map((c) => c.flow)) }, }; let root = ''; @@ -251,3 +257,48 @@ describe('[#28893] validate, build and lint refuse a flow region the engine refu expect(issues.filter((i) => i.rule === RULE)).toEqual([]); }, 60_000); }); + +/** + * [#29051] A flow whose TOP-LEVEL graph closes a cycle with no `type: 'back'` + * edge (ADR-0044). The engine refuses it at registration with + * `findUndeclaredFlowCycle` (`@objectstack/metadata-core`); before this, + * `os validate` and `os build` passed it with exit 0 and the flow never ran. + * Driven from the judge's own corpus, so the doors refuse exactly the cases + * the engine and the publish gate refuse, each at its own flow, in the + * engine's sentence. + */ +describe('[#29051] validate and build refuse an undeclared top-level flow cycle, with the engine\'s sentence', () => { + const refused = FLOW_CYCLE_CORPUS + .map((c, i) => ({ c, i })) + .filter(({ c }) => c.cycle !== null) + .map(({ c, i }) => ({ path: `flows[${i}]`, code: 'VALIDATION_ERROR', flow: c.flow.name, cycle: undeclaredFlowCycleMessage(c.cycle!) })); + const rowsOf = (errors: Array>) => + errors.filter((r) => String(r.message).includes('Flow contains a cycle')); + const expectRefused = (errors: Array>) => { + const rows = rowsOf(errors); + expect(rows.map(({ path, code, flow }) => ({ path, code, flow })), JSON.stringify(errors, null, 2)) + .toEqual(refused.map(({ path, code, flow }) => ({ path, code, flow }))); + rows.forEach((row, k) => expect(String(row.message)).toContain(refused[k]!.cycle)); + }; + + it('validate exits 1 with one row per refused corpus flow; build refuses the same and writes nothing', async () => { + const validated = await validate('cycles', '--json'); + expect(validated.code, validated.stdout).toBe(1); + expectRefused(json(validated, 'validate cycles').errors as Array>); + + const built = await build('cycles', '--json'); + expect(built.code, built.stdout).toBe(1); + const payload = json(built, 'build cycles'); + expect(payload.success).toBe(false); + expectRefused(payload.errors as Array>); + expect(existsSync(fixtures.cycles.dist)).toBe(false); + }, 60_000); + + it('CONTROL: the corpus flows whose loops are declared back (and the acyclic ones) validate and build', async () => { + const validated = await validate('cyclesDeclared', '--json'); + expect(validated.code, validated.stdout).toBe(0); + const built = await build('cyclesDeclared', '--json'); + expect(json(built, 'build cyclesDeclared').success, built.stdout).toBe(true); + expect(built.code).toBe(0); + }, 60_000); +}); diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 7d5ad3dacb7..bc6ffa7acc8 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -875,6 +875,19 @@ export default defineConfig({ find: /^create-objectstack\/created-summary$/, replacement: path.resolve(__dirname, '../create-objectstack/src/created-summary.ts'), }, + // [#29051] `FLOW_CYCLE_CORPUS`, the case table the flow-cycle judge, the + // engine's registration and the publish gate are driven from, pins + // `os validate` / `os build` to the same graphs + // (`src/utils/flow-undeclared-cycle.test.ts`, + // `test/validate-flow-region-shape.test.ts`). The subpath's `exports` + // entry is built from `src/testing.ts`; aliased so the corpus the tests + // read is the checkout's, not a stale `dist/testing.js`. The bare + // `@objectstack/metadata-core` specifier has its own anchored entry + // below (the #20158 block); this one covers only the `/testing` subpath. + { + find: /^@objectstack\/metadata-core\/testing$/, + replacement: path.resolve(__dirname, '../metadata-core/src/testing.ts'), + }, // `test/serve-node-env-production-default.e2e.test.ts` (#11113) writes a // FIXTURE config file whose text is `import { AuthPlugin } from // '@objectstack/plugin-auth'` — real code, but code the fixture's own