From a0e211a9b92a55597988e7d5e4ccb3b6f604618e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:08:10 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(lint):=20flow-region-shape-invalid=20?= =?UTF-8?q?=E2=80=94=20os=20lint=20refuses=20a=20flow=20region=20the=20eng?= =?UTF-8?q?ine=20refuses=20to=20register=20(#29049)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ADR-0031 region check that os validate and os build ran through a CLI-only walk (flow-region-shape.ts) is now an authoring rule calling the spec's validateControlFlow, so all three commands judge it from the registry; the CLI walk is retired. Claude-Session: https://claude.ai/code/session_01ADHkmQjqkXDjc5jhjFfHt3 Co-authored-by: Claude --- content/docs/deployment/cli.mdx | 2 +- .../docs/deployment/validating-metadata.mdx | 2 +- .../build-with-claude-code.mdx | 2 +- content/docs/ui/react-pages.mdx | 2 +- packages/cli/src/commands/compile.ts | 28 +---- packages/cli/src/commands/validate.ts | 36 +----- .../cli/src/utils/flow-region-shape.test.ts | 48 -------- packages/cli/src/utils/flow-region-shape.ts | 95 --------------- .../authoring-rule-command-parity.test.ts | 52 +------- .../test/validate-build-gate-parity.test.ts | 9 -- .../test/validate-flow-region-shape.test.ts | 55 +++++++-- packages/lint/src/authoring-rules.ts | 22 ++++ packages/lint/src/index.ts | 8 ++ packages/lint/src/rule-explanations.ts | 24 ++++ .../src/validate-flow-region-shape.test.ts | 101 ++++++++++++++++ .../lint/src/validate-flow-region-shape.ts | 113 ++++++++++++++++++ 16 files changed, 332 insertions(+), 267 deletions(-) delete mode 100644 packages/cli/src/utils/flow-region-shape.test.ts delete mode 100644 packages/cli/src/utils/flow-region-shape.ts create mode 100644 packages/lint/src/validate-flow-region-shape.test.ts create mode 100644 packages/lint/src/validate-flow-region-shape.ts diff --git a/content/docs/deployment/cli.mdx b/content/docs/deployment/cli.mdx index 2a62db5b983..0a73ed21e0f 100644 --- a/content/docs/deployment/cli.mdx +++ b/content/docs/deployment/cli.mdx @@ -771,7 +771,7 @@ os compile --json # JSON output for CI pipelines → Normalizing stack definition... → Lowering inline handlers... → Validating protocol compliance... - → Running author-time rules (60)... + → Running author-time rules (61)... → Checking that every required capability has a provider installable in this edition... → Collecting package docs (ADR-0046)... 0 collected → Writing artifact... diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index 0abaac3bf18..e69eda4f3b6 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -760,7 +760,7 @@ A clean run walks the registry and reports timing: Config: /path/to/support-desk/objectstack.config.ts Load time: 21ms → Validating against ObjectStack Protocol... - → Running author-time rules (60)... + → Running author-time rules (61)... → Checking that every required capability has a provider installable in this edition... → Checking package docs (ADR-0046)... diff --git a/content/docs/getting-started/build-with-claude-code.mdx b/content/docs/getting-started/build-with-claude-code.mdx index 72149b6088c..05d789ed946 100644 --- a/content/docs/getting-started/build-with-claude-code.mdx +++ b/content/docs/getting-started/build-with-claude-code.mdx @@ -310,7 +310,7 @@ visible: 'status != "resolved"' ◆ Validate ──────────────────────────────────────── → Validating against ObjectStack Protocol... - → Running author-time rules (60)... + → Running author-time rules (61)... ✗ Author-time rules failed (1 issue) • stack · action 'resolve_ticket' visible: bare reference `status` — a diff --git a/content/docs/ui/react-pages.mdx b/content/docs/ui/react-pages.mdx index c2b3cb0f278..664b648789c 100644 --- a/content/docs/ui/react-pages.mdx +++ b/content/docs/ui/react-pages.mdx @@ -404,7 +404,7 @@ objectstack validate ──────────────────────────────────────── → Loading configuration... → Validating against ObjectStack Protocol... - → Running author-time rules (60)... + → Running author-time rules (61)... → Checking that every required capability has a provider installable in this edition... → Checking package docs (ADR-0046)... diff --git a/packages/cli/src/commands/compile.ts b/packages/cli/src/commands/compile.ts index 9a450afe6b2..7db48c0ee42 100644 --- a/packages/cli/src/commands/compile.ts +++ b/packages/cli/src/commands/compile.ts @@ -83,9 +83,6 @@ 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'; @@ -840,28 +837,9 @@ 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); - 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-iii-ter. [#28893] Flow region shape (ADR-0031) is judged by the + // rule table at step 3b since #29049 (`flow-region-shape-invalid`), + // the same verdict `os validate` and `os lint` give. // 3b-quater. ADR-0141 D2, author side — every `manifest.dependencies` // range of the packages one install act would deliver, judged by the diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index aa7682a7b0e..1c4c07c104d 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -81,9 +81,6 @@ 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. @@ -754,35 +751,10 @@ 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); - 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); - } + // 3a-vi. [#28893] Flow region shape (ADR-0031) is judged by the rule + // table above since #29049 (`flow-region-shape-invalid`, + // `@objectstack/lint`'s `validate-flow-region-shape.ts`), so all + // three authoring commands give the engine's registration verdict. // 3b. [#3366] Installable-provider preflight — the shift-left of the // `serve`-time capability check. `os validate` previously only checked diff --git a/packages/cli/src/utils/flow-region-shape.test.ts b/packages/cli/src/utils/flow-region-shape.test.ts deleted file mode 100644 index 29b46230948..00000000000 --- a/packages/cli/src/utils/flow-region-shape.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [#29059] `findFlowRegionRefusals` never passes a flow it could not judge. - * - * The walk parses each flow with `FlowSchema` before it asks - * `validateControlFlow` for the region verdict. The stack schema has already - * parsed every flow it reads, and `FlowSchema` is idempotent on its own output, - * so no authored stack reaches the refused-parse branch today; the cases below - * hand the walk bodies the schema refuses, which is the only way to force it. - * Before #29059 the branch was a bare `return`: no row, so `os validate` and - * `os build` passed a flow whose region shape nobody had judged. - * - * What the door does 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 { findFlowRegionRefusals } from './flow-region-shape.js'; - -/** A body `FlowSchema` refuses: `nodes` must be an array. */ -const unparseable = (name?: string) => ({ ...(name === undefined ? {} : { name }), label: 'Broken', type: 'autolaunched', nodes: 'start', edges: [] }); - -const located = (rows: ReturnType) => rows.map(({ path, code, flow }) => ({ path, code, flow })); - -describe('[#29059] a flow the flow schema refuses is a row, never a silent pass', () => { - it('the stack\'s own flows: the row names the flow and its path', () => { - const rows = findFlowRegionRefusals({ flows: [unparseable('rgn_broken')] }); - expect(located(rows)).toEqual([{ path: 'flows[0]', code: 'VALIDATION_ERROR', flow: 'rgn_broken' }]); - expect(rows[0]!.message).toContain("'rgn_broken'"); - // The schema's own issue rides the row, so the author is told what it refused. - expect(rows[0]!.message).toContain('nodes'); - }); - - it('a body with no string name is named by its path', () => { - const rows = findFlowRegionRefusals({ flows: [unparseable()] }); - expect(located(rows)).toEqual([{ path: 'flows[0]', code: 'VALIDATION_ERROR', flow: 'flows[0]' }]); - }); - - it('an artifact\'s packages[] entry: located under its own entry', () => { - const rows = findFlowRegionRefusals({ - flows: [unparseable('rgn_top_level_not_walked')], - packages: [{ manifest: { id: 'com.example.a', flows: [] } }, { manifest: { id: 'com.example.b', flows: [unparseable('rgn_broken')] } }], - }); - expect(located(rows)).toEqual([{ path: 'packages[1].manifest.flows[0]', code: 'VALIDATION_ERROR', flow: 'rgn_broken' }]); - }); -}); diff --git a/packages/cli/src/utils/flow-region-shape.ts b/packages/cli/src/utils/flow-region-shape.ts deleted file mode 100644 index e6ebf20c648..00000000000 --- a/packages/cli/src/utils/flow-region-shape.ts +++ /dev/null @@ -1,95 +0,0 @@ -// 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 (`: `) 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 `FlowSchema` refuses HERE is a row, never a skip (#29059). The stack - * schema has already parsed every flow this walk reads, and `FlowSchema` is - * idempotent on its own output today, so the branch cannot be reached by an - * author. If that ever stops being true the region verdict was never computed, - * and a silent `return` would pass the flow: the false-clean direction this - * gate exists to close. The row says the flow could not be judged, with the - * schema's issues, and the door refuses on it like any other row. - * - * 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; its `path` when the body carries no string name. */ - flow: string; - /** - * `Flow '' …: ` followed by the engine's refusal, verbatim — or, when - * `FlowSchema` refuses the parsed body, the schema's issues (#29059). - */ - message: string; -} - -/** Every flow of the parsed stack the engine would refuse to register for its region shape. */ -export function findFlowRegionRefusals(parsed: Record): 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) { - const declared = (raw as { name?: unknown } | null)?.name; - const name = typeof declared === 'string' && declared !== '' ? declared : `${at}flows[${i}]`; - const issues = result.error.issues - .map((issue) => `${issue.path.length > 0 ? issue.path.join('.') : '(root)'}: ${issue.message}`) - .join('; '); - rows.push({ - path: `${at}flows[${i}]`, - code: 'VALIDATION_ERROR', - flow: name, - message: `Flow '${name}' could not be judged for its region shape: the flow schema refused the parsed body (${issues})`, - }); - 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; -} diff --git a/packages/cli/test/authoring-rule-command-parity.test.ts b/packages/cli/test/authoring-rule-command-parity.test.ts index 51d0574e2d0..2f01449b98e 100644 --- a/packages/cli/test/authoring-rule-command-parity.test.ts +++ b/packages/cli/test/authoring-rule-command-parity.test.ts @@ -32,7 +32,6 @@ import { fileURLToPath } from 'node:url'; import { AUTHORING_COMMANDS, runAuthoringRules, type AuthoringCommand } from '@objectstack/lint'; import { childEnv } from './helpers/serve-process.js'; import { defineStackSource, linkSpec } from './helpers/define-stack-fixture.js'; -import { findFlowRegionRefusals } from '../src/utils/flow-region-shape.js'; const cliBin = join(fileURLToPath(new URL('.', import.meta.url)), '..', 'bin', 'run-dev.js'); @@ -141,39 +140,14 @@ const CASES: ReadonlyArray<{ rule: string; blindTo: readonly AuthoringCommand[]; actions: [{ name: 'log_time', type: 'form', target: 'parity_task.default' }], }), }, -]; - -/** - * 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; `gate` is the check the two doors call. - * - * The pin below measures two things on the SAME stack, in process: `gate` - * refuses it with a row carrying `marker` (so the fixture really plants the - * defect), and the rule registry, run as each `blindTo` command runs it, - * reports no finding carrying `marker`. It measures the registry, not the - * `os lint` command. Closing the gap (registering the check as an authoring - * rule) reds the second half here and the case moves into `CASES`; a fixture - * that stops planting the defect reds the first half (#29059) instead of - * leaving the blindness assertion vacuously green. - * - * - 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 (`findFlowRegionRefusals`, `utils/flow-region-shape.ts`, pinned - * end to end by `validate-flow-region-shape.test.ts`). - */ -const OUTSIDE_THE_TABLE: ReadonlyArray<{ - check: string; - marker: string; - blindTo: readonly AuthoringCommand[]; - gate: (stack: Record) => ReadonlyArray<{ message: string }>; - stack: Record; -}> = [ + // [#29049] Was the one row of an `OUTSIDE_THE_TABLE` table: `os validate` + // and `os build` refused it through a CLI-only walk (#28893) and `os lint`, + // which runs only this registry, passed it. The planted defect is a + // `try_catch` catch region that is not single-exit, which the automation + // engine refuses to register; the rule is that same call. { - check: 'flow region shape (#28893)', - marker: 'single-exit', + rule: 'flow-region-shape-invalid', blindTo: ['lint'], - gate: findFlowRegionRefusals, stack: withBaseline({ objects: [{ name: 'parity_task', label: 'Task', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name' } } }], flows: [{ @@ -222,20 +196,6 @@ 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) => { - // The fixture plants the defect: the doors' own gate refuses it with the - // marker, or the blindness below would hold of a stack nothing refuses. - const refused = testCase.gate(testCase.stack).filter((r) => r.message.includes(testCase.marker)); - expect(refused, JSON.stringify(testCase.gate(testCase.stack), null, 2)).not.toEqual([]); - 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 diff --git a/packages/cli/test/validate-build-gate-parity.test.ts b/packages/cli/test/validate-build-gate-parity.test.ts index 5587cb769c8..632d6647001 100644 --- a/packages/cli/test/validate-build-gate-parity.test.ts +++ b/packages/cli/test/validate-build-gate-parity.test.ts @@ -213,15 +213,6 @@ 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', ]; /** diff --git a/packages/cli/test/validate-flow-region-shape.test.ts b/packages/cli/test/validate-flow-region-shape.test.ts index d5f427f9c59..1930a24fe7c 100644 --- a/packages/cli/test/validate-flow-region-shape.test.ts +++ b/packages/cli/test/validate-flow-region-shape.test.ts @@ -14,6 +14,12 @@ * The engine then refused it at `registerFlow` ("region must be single-exit"), * logged at WARN, and the boot went on without the flow, so its trigger never * fired. Both commands run in-process (`Command.run`), not as spawned CLIs. + * + * [#29049] The verdict moved into the authoring-rule table + * (`flow-region-shape-invalid`, `@objectstack/lint`), so `os lint` gives it + * too: each fixture below is run through all three commands. A refusal is now + * a rule finding (`rule`, `where`, `path`, `message`, `hint`) in the rule + * table's `errors`, not a separate gate's `VALIDATION_ERROR` row. */ import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; @@ -22,6 +28,7 @@ import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import Compile from '../src/commands/compile'; +import Lint from '../src/commands/lint'; import Validate from '../src/commands/validate'; import { defineStackSource, linkSpec } from './helpers/define-stack-fixture.js'; @@ -29,7 +36,7 @@ const CLI_ROOT = resolve(fileURLToPath(import.meta.url), '..', '..'); interface Run { code: number; stdout: string } -async function run(command: typeof Compile | typeof Validate, argv: string[]): Promise { +async function run(command: typeof Compile | typeof Validate | typeof Lint, argv: string[]): Promise { const chunks: string[] = []; const spy = vi.spyOn(process.stdout, 'write').mockImplementation(((chunk: unknown, ...rest: unknown[]) => { chunks.push(String(chunk)); @@ -38,9 +45,13 @@ async function run(command: typeof Compile | typeof Validate, argv: string[]): P return true; }) as never); const exitCodeBefore = process.exitCode; + process.exitCode = undefined; let code = 0; try { await command.run(argv, { root: CLI_ROOT }); + // `os lint --json` reports its verdict through `process.exitCode` + // (`emitJson`), not an oclif exit. + code = Number(process.exitCode ?? 0); } catch (error) { const exit = (error as { oclif?: { exit?: unknown } }).oclif?.exit; if (typeof exit !== 'number') throw error; @@ -162,10 +173,13 @@ afterAll(() => { const validate = (label: string, ...flags: string[]) => run(Validate, [fixtures[label].config, ...flags]); const build = (label: string, ...flags: string[]) => run(Compile, [fixtures[label].config, '--output', fixtures[label].artifact, ...flags]); +const lint = (label: string) => run(Lint, [fixtures[label].config, '--json', '--skip-i18n']); + +const RULE = 'flow-region-shape-invalid'; -describe('[#28893] validate and build refuse a flow region the engine refuses, with its message', () => { +describe('[#28893] validate, build and lint refuse a flow region the engine refuses, with its message', () => { for (const label of ['duly', 'nested', 'singleBranch'] as const) { - it(`${label}: validate exits 1 naming the flow and the engine's refusal; build refuses the same and writes nothing`, async () => { + it(`${label}: validate exits 1 naming the flow and the engine's refusal; build refuses the same and writes nothing; lint refuses it`, async () => { const { refusal } = CASES[label]; const flowName = (CASES[label].stack.flows as Array<{ name: string }>)[0]!.name; const validated = await validate(label, '--json'); @@ -173,34 +187,56 @@ describe('[#28893] validate and build refuse a flow region the engine refuses, w const errors = json(validated, `validate ${label}`).errors as Array>; const row = errors.find((r) => String(r.message).includes(refusal!)); expect(row, JSON.stringify(errors, null, 2)).toBeDefined(); + expect(row!.rule).toBe(RULE); expect(String(row!.message)).toContain(flowName); + // [#29049] Reported once: the retired CLI-only walk does not echo it. + expect(errors.filter((r) => String(r.message).includes(refusal!))).toHaveLength(1); const built = await build(label, '--json'); expect(built.code).toBe(1); const payload = json(built, `build ${label}`); expect(payload.success).toBe(false); - expect((payload.errors as Array>).some((r) => String(r.message).includes(refusal!))).toBe(true); + const builtRows = (payload.errors as Array>).filter((r) => String(r.message).includes(refusal!)); + expect(builtRows.map((r) => r.rule)).toEqual([RULE]); expect(existsSync(fixtures[label].dist)).toBe(false); + + // [#29049] `os lint`, blind to it before: exit 1, the same rule and message. + const linted = await lint(label); + expect(linted.code, linted.stdout).toBe(1); + const lintReport = json(linted, `lint ${label}`); + const lintRows = (lintReport.issues as Array>).filter((i) => i.rule === RULE); + expect(lintRows.map((i) => i.severity), JSON.stringify(lintReport.issues, null, 2)).toEqual(['error']); + expect(String(lintRows[0]!.message)).toContain(refusal!); + expect(String(lintRows[0]!.message)).toContain(flowName); }, 60_000); } - it('[#29059] an assembled artifact: the refused flow is located in its own packages[] entry, and build writes nothing', async () => { + // [#29049] Forced edit: the CLI-only walk located the row in its own entry + // (`packages[1].manifest.flows[0]`, `code`, `flow`). The rule table judges + // the artifact's union of flows first, as it judges every other collection, + // so the finding sits at the union path and names the flow in `where`. + it('[#29059] an assembled artifact: the refused flow in the second packages[] entry is refused once, and build writes nothing', async () => { const { refusal } = CASES.assembled; - const expected = { path: 'packages[1].manifest.flows[0]', code: 'VALIDATION_ERROR', flow: 'rgn_ops_lead_time' }; + const expected = { rule: RULE, where: "flow 'rgn_ops_lead_time'", path: 'flows[1]' }; const validated = await validate('assembled', '--json'); expect(validated.code, validated.stdout).toBe(1); const errors = json(validated, 'validate assembled').errors as Array>; const rows = errors.filter((r) => String(r.message).includes(refusal!)); - expect(rows.map(({ path, code, flow }) => ({ path, code, flow })), JSON.stringify(errors, null, 2)).toEqual([expected]); + expect(rows.map(({ rule, where, path }) => ({ rule, where, path })), JSON.stringify(errors, null, 2)).toEqual([expected]); const built = await build('assembled', '--json'); expect(built.code, built.stdout).toBe(1); const payload = json(built, 'build assembled'); expect(payload.success).toBe(false); const builtRows = (payload.errors as Array>).filter((r) => String(r.message).includes(refusal!)); - expect(builtRows.map(({ path, code, flow }) => ({ path, code, flow }))).toEqual([expected]); + expect(builtRows.map(({ rule, where, path }) => ({ rule, where, path }))).toEqual([expected]); expect(existsSync(fixtures.assembled.dist)).toBe(false); + + const linted = await lint('assembled'); + expect(linted.code, linted.stdout).toBe(1); + const lintRows = (json(linted, 'lint assembled').issues as Array>).filter((i) => i.rule === RULE); + expect(lintRows.map(({ path }) => path)).toEqual(['flows[1]']); }, 60_000); it('CONTROL: the same flow with the join restored validates and builds', async () => { @@ -209,5 +245,8 @@ describe('[#28893] validate and build refuse a flow region the engine refuses, w const built = await build('joined', '--json'); expect(json(built, 'build joined').success, built.stdout).toBe(true); expect(built.code).toBe(0); + const linted = await lint('joined'); + const issues = json(linted, 'lint joined').issues as Array>; + expect(issues.filter((i) => i.rule === RULE)).toEqual([]); }, 60_000); }); diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 9f774754e77..2d2c58cbbcf 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -150,6 +150,7 @@ import { validateRuleSchemaFormats } from './validate-rule-schema-formats.js'; import { validateActionLocations } from './validate-action-locations.js'; import { lintFlowPatterns } from './lint-flow-patterns.js'; import { lintFlowCredentialLiterals } from './lint-flow-credential-literals.js'; +import { validateFlowRegionShape } from './validate-flow-region-shape.js'; import { lintLivenessProperties } from './lint-liveness-properties.js'; import { lintAutonumberFormats } from './lint-autonumber-formats.js'; import { lintViewRefs } from './lint-view-refs.js'; @@ -1951,6 +1952,27 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ hint: f.hint, })), }, + // [#29049] ADR-0031 region shape: a flow whose `loop` body, `parallel` + // branches or `try_catch` regions are not single-entry/single-exit and + // acyclic at any depth, or whose `parallel` has one branch, is a flow the + // automation engine refuses to register, so it never runs. The verdict is + // the engine's own call (`FlowSchema`, then the spec's `validateControlFlow`); + // `os validate` and `os build` gated on it through a CLI-only walk + // (#28893), which left `os lint` blind. Registered here, all three commands + // judge it, and the CLI walk is retired. + { + name: 'validateFlowRegionShape', + tier: 'gating', + input: 'parsed', + commands: ALL, + source: 'packages/lint/src/validate-flow-region-shape.ts', + surfaces: CLI_ONLY, + surfaceReason: + 'Not yet on the runtime publish gate: whether a flow write whose region the engine refuses is refused at ' + + 'publish is the server lane\'s open question for the metadata write path, decided there; this entry ' + + 'brings the three authoring commands to the engine\'s registration verdict.', + run: (stack) => validateFlowRegionShape(stack), + }, // The spec-liveness loop on the author side: a property the ledger marks // dead, live-elsewhere or experimental is set hopefully and does nothing // here. Ledger-driven (those verdicts warn on their own; any other row only diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index de575fbc3ad..ac2c534a8ff 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -725,6 +725,14 @@ export { } from './validate-customization-namespace.js'; export type { CustomizationNamespaceFinding } from './validate-customization-namespace.js'; +// [#29049] ADR-0031 region shape: a flow the automation engine refuses to +// register, judged with the spec's `validateControlFlow` itself. +export { + validateFlowRegionShape, + FLOW_REGION_SHAPE_INVALID, +} from './validate-flow-region-shape.js'; +export type { FlowRegionShapeFinding } from './validate-flow-region-shape.js'; + // [#14105] The layer BELOW the chart/widget binding rules above: a dataset's // own `include[]`, `dimensions[].field`, `measures[].field` and filter KEYS, // resolved against the object graph. Its base-object half is diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index eaa9868844a..9c18d9ead6d 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -5420,6 +5420,29 @@ const NAMESPACE_NO_CUSTOMIZATION_PAIR_EXPLANATION: RuleExplanation = { ], }; +// The verdict is `validateControlFlow` from `@objectstack/spec/automation`, the +// call the automation engine's `registerFlow` makes; the rule file holds no copy. +const FLOW_REGION_SHAPE_INVALID_EXPLANATION: RuleExplanation = { + rule: 'flow-region-shape-invalid', + covers: 'what makes a flow region one the engine registers', + paragraphs: [ + 'A `loop` body, each `parallel` branch and a `try_catch` try or catch region is a small graph of its ' + + 'own (ADR-0031). The automation engine runs it from its one entry node to its one exit node, so when ' + + 'it registers a flow it refuses a region that has more than one entry or exit, or a cycle, at any ' + + 'depth, and a `parallel` with fewer than two branches. A refused flow is not registered: the server ' + + 'logs a warning at boot and runs without it, so its trigger never fires.', + 'The rule asks the engine\'s own question: each flow is parsed with the flow schema and handed to the ' + + 'same region check the engine runs when it registers the flow, and the message is the engine\'s ' + + 'refusal behind the flow\'s name. Like the engine it reports a flow\'s first malformed region, so a ' + + 'flow with two may need two fixes. A flow the flow schema refuses cannot be judged and is reported ' + + 'as such, with the schema\'s issues.', + 'Not judged: a cycle in the flow\'s top-level graph, outside any region. The engine refuses that too, ' + + 'with a check this rule cannot reach.', + 'To fix a multi-exit region, add one node every exit of the region leads to; to fix a cycle, remove ' + + 'the edge that leads back; give a `parallel` a second branch or replace it with its one branch.', + ], +}; + export const RULE_EXPLANATIONS: Readonly> = Object.freeze({ [FIELD_NO_CONSUMERS_EXPLANATION.rule]: FIELD_NO_CONSUMERS_EXPLANATION, [SECURITY_OWD_UNSET_EXPLANATION.rule]: SECURITY_OWD_UNSET_EXPLANATION, @@ -5631,6 +5654,7 @@ export const RULE_EXPLANATIONS: Readonly> = Obje [REPORT_DATE_RANGE_DIMENSION_AMBIGUOUS_EXPLANATION.rule]: REPORT_DATE_RANGE_DIMENSION_AMBIGUOUS_EXPLANATION, [REPORT_DATE_RANGE_DIMENSION_NONE_EXPLANATION.rule]: REPORT_DATE_RANGE_DIMENSION_NONE_EXPLANATION, [NAMESPACE_NO_CUSTOMIZATION_PAIR_EXPLANATION.rule]: NAMESPACE_NO_CUSTOMIZATION_PAIR_EXPLANATION, + [FLOW_REGION_SHAPE_INVALID_EXPLANATION.rule]: FLOW_REGION_SHAPE_INVALID_EXPLANATION, }); /** The explanation for `rule`, or `undefined` when the rule has none. Exact id match. */ diff --git a/packages/lint/src/validate-flow-region-shape.test.ts b/packages/lint/src/validate-flow-region-shape.test.ts new file mode 100644 index 00000000000..19fc98b1eb0 --- /dev/null +++ b/packages/lint/src/validate-flow-region-shape.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#29049] `flow-region-shape-invalid`: the automation engine's registration + * verdict on a flow's ADR-0031 regions, as an authoring rule. + * + * The subject here is the finding. That the three commands gate on it is + * pinned in `@objectstack/cli`'s `authoring-rule-command-parity.test.ts`, and + * `os validate` / `os build` end to end in its + * `validate-flow-region-shape.test.ts`. + * + * The refused-parse cases moved here from the retired CLI walk's own test + * (#29059): a flow the flow schema refuses is a finding, never a silent pass. + */ + +import { describe, expect, it } from 'vitest'; +import { validateControlFlow, FlowSchema } from '@objectstack/spec/automation'; + +import { FLOW_REGION_SHAPE_INVALID, validateFlowRegionShape } from './validate-flow-region-shape.js'; +import { AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js'; + +const n = (id: string, type = 'assignment', extra: Record = {}) => ({ id, type, label: id, ...extra }); +const e = (id: string, source: string, target: string) => ({ id, source, target, type: 'default' }); + +/** d → x, y: one entry, two exits. With `join`, x, y → j closes it. */ +const fanOut = (join: boolean) => ({ + nodes: [n('d'), n('x'), n('y'), ...(join ? [n('j')] : [])], + edges: [e('r1', 'd', 'x'), e('r2', 'd', 'y'), ...(join ? [e('r3', 'x', 'j'), e('r4', 'y', 'j')] : [])], +}); + +const flow = (name: string, middle: Record) => ({ + name, label: name, type: 'autolaunched', runAs: 'system', status: 'active', + nodes: [n('start', 'start'), middle, n('end', 'end')], + edges: [e('e1', 'start', String(middle.id)), e('e2', String(middle.id), 'end')], +}); + +const guard = (catchRegion: Record) => + n('guard', 'try_catch', { config: { try: { nodes: [n('work')], edges: [] }, catch: catchRegion } }); + +/** What the engine's own call says about `raw` — the message the finding must carry. */ +const engineRefusal = (raw: unknown): string => { + try { + validateControlFlow(FlowSchema.parse(raw)); + } catch (err) { + return (err as Error).message; + } + throw new Error('the engine registers this flow'); +}; + +/** A body `FlowSchema` refuses: `nodes` must be an array. */ +const unparseable = (name?: string) => ({ ...(name === undefined ? {} : { name }), label: 'Broken', type: 'autolaunched', nodes: 'start', edges: [] }); + +describe('[#29049] flow-region-shape-invalid', () => { + it('a multi-exit region: one error carrying the engine\'s refusal behind the flow\'s name', () => { + const bad = flow('rgn_lead_time', guard(fanOut(false))); + const findings = validateFlowRegionShape({ flows: [flow('rgn_ok', guard(fanOut(true))), bad] }); + expect(findings.map(({ severity, rule, where, path }) => ({ severity, rule, where, path }))).toEqual([ + { severity: 'error', rule: FLOW_REGION_SHAPE_INVALID, where: "flow 'rgn_lead_time'", path: 'flows[1]' }, + ]); + expect(findings[0]!.message).toBe(`Flow 'rgn_lead_time' would be refused at registration: ${engineRefusal(bad)}`); + expect(findings[0]!.message).toContain("try_catch 'guard' catch: region must be single-exit"); + }); + + it('a parallel with one branch is refused with the engine\'s sentence', () => { + const bad = flow('rgn_fan', n('fan', 'parallel', { config: { branches: [{ nodes: [n('only')], edges: [] }] } })); + const [finding] = validateFlowRegionShape({ flows: [bad] }); + expect(finding?.message).toContain("parallel 'fan': a parallel block needs at least 2 branches"); + }); + + it('CONTROL: the same flow with the join restored has no finding, nor does a stack with no flows', () => { + expect(validateFlowRegionShape({ flows: [flow('rgn_lead_time', guard(fanOut(true)))] })).toEqual([]); + expect(validateFlowRegionShape({})).toEqual([]); + }); + + describe('[#29059] a flow the flow schema refuses is a finding, never a silent pass', () => { + it('names the flow, its path and the schema\'s issue', () => { + const [finding, ...rest] = validateFlowRegionShape({ flows: [unparseable('rgn_broken')] }); + expect(rest).toEqual([]); + expect(finding).toMatchObject({ severity: 'error', rule: FLOW_REGION_SHAPE_INVALID, where: "flow 'rgn_broken'", path: 'flows[0]' }); + expect(finding!.message).toContain('could not be judged'); + expect(finding!.message).toContain('nodes'); + }); + + it('a body with no string name is named by its path', () => { + const [finding] = validateFlowRegionShape({ flows: [unparseable()] }); + expect(finding).toMatchObject({ where: "flow 'flows[0]'", path: 'flows[0]' }); + }); + }); + + it('is registered as a gating rule on all three commands, and the registry run reports it', () => { + const entry = AUTHORING_RULES.find((r) => r.name === 'validateFlowRegionShape'); + expect(entry).toMatchObject({ tier: 'gating', input: 'parsed', commands: ['validate', 'build', 'lint'] }); + const stack = { flows: [flow('rgn_lead_time', guard(fanOut(false)))] }; + for (const command of ['validate', 'build', 'lint'] as const) { + const rules = runAuthoringRules(command, { normalized: stack, ...(command === 'lint' ? {} : { parsed: stack }) }) + .filter((f) => f.severity === 'error') + .map((f) => f.rule); + expect(rules, command).toContain(FLOW_REGION_SHAPE_INVALID); + } + }); +}); diff --git a/packages/lint/src/validate-flow-region-shape.ts b/packages/lint/src/validate-flow-region-shape.ts new file mode 100644 index 00000000000..4deac76f066 --- /dev/null +++ b/packages/lint/src/validate-flow-region-shape.ts @@ -0,0 +1,113 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `flow-region-shape-invalid` — a flow the automation engine refuses to + * register for its ADR-0031 region shape, refused at every authoring command. + * + * ## What it reports + * + * One `error` per 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. The engine's `registerFlow` refuses such a flow: it logs + * `[Automation] failed to register flow` and the boot goes on without it, so + * its trigger never fires. + * + * ## ⛔ The verdict is the engine's — nothing here restates the region rules + * + * Each flow goes through the same two calls `registerFlow` makes: `FlowSchema`, + * then the spec's `validateControlFlow` (`@objectstack/spec/automation`). The + * finding's message is the engine's refusal, verbatim, behind the flow's name. + * Like the engine, it stops at a flow's first malformed region; every flow is + * judged. + * + * ## A flow `FlowSchema` refuses is a finding, never a skip + * + * The region verdict is computed on the parsed flow. Under `os validate` and + * `os build` the stack schema has already parsed every flow, and `FlowSchema` + * accepts its own output, so a refused parse cannot happen there today; if + * that ever stops being true, a silent skip would pass a flow nobody judged, + * the false-clean direction this rule exists to close. Under `os lint`, which + * never parses, the flow really can fail the schema, and the engine refuses to + * register it too. Either way the finding says the flow could not be judged + * and carries the schema's issues. + * + * ## 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'; + +type AnyRec = Record; + +/** The rule id every finding of this rule carries. */ +export const FLOW_REGION_SHAPE_INVALID = 'flow-region-shape-invalid'; + +export interface FlowRegionShapeFinding { + /** Always an error: the engine refuses the flow. */ + severity: 'error'; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `flow 'lead_time'`. */ + where: string; + /** Config path of the flow, e.g. `flows[0]`. */ + path: string; + /** `Flow '' would be refused at registration: ` and the engine's refusal. */ + message: string; + /** How to fix it. */ + hint: string; +} + +const HINT = + 'Give the region one entry and one exit (add one node every exit of the region leads to), remove any edge ' + + 'that loops back inside it, and give a parallel at least two branches.'; + +const SCHEMA_HINT = + 'Fix the flow so the flow schema accepts it (`os validate` names each issue); its regions are judged then.'; + +/** + * Every flow of `stack.flows` the automation engine would refuse to register + * for its region shape, one finding each. Pure. + */ +export function validateFlowRegionShape(stack: AnyRec): FlowRegionShapeFinding[] { + const flows = stack.flows; + if (!Array.isArray(flows)) return []; + const findings: FlowRegionShapeFinding[] = []; + flows.forEach((raw, i) => { + const path = `flows[${i}]`; + const result = FlowSchema.safeParse(raw); + if (!result.success) { + const declared = (raw as { name?: unknown } | null)?.name; + const name = typeof declared === 'string' && declared !== '' ? declared : path; + const issues = result.error.issues + .map((issue) => `${issue.path.length > 0 ? issue.path.join('.') : '(root)'}: ${issue.message}`) + .join('; '); + findings.push({ + severity: 'error', + rule: FLOW_REGION_SHAPE_INVALID, + where: `flow '${name}'`, + path, + message: `Flow '${name}' could not be judged for its region shape: the flow schema refused it (${issues})`, + hint: SCHEMA_HINT, + }); + return; + } + try { + validateControlFlow(result.data); + } catch (err) { + const name = result.data.name; + findings.push({ + severity: 'error', + rule: FLOW_REGION_SHAPE_INVALID, + where: `flow '${name}'`, + path, + message: `Flow '${name}' would be refused at registration: ${err instanceof Error ? err.message : String(err)}`, + hint: HINT, + }); + } + }); + return findings; +} From 96f4483d43c66dd238532f48586dffc599536321 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:19:38 +0000 Subject: [PATCH 2/3] fix(lint): flow-region-shape-invalid skips a non-record flows[] entry like every registry reader Claude-Session: https://claude.ai/code/session_01ADHkmQjqkXDjc5jhjFfHt3 Co-authored-by: Claude --- packages/lint/src/validate-flow-region-shape.test.ts | 6 ++++++ packages/lint/src/validate-flow-region-shape.ts | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/lint/src/validate-flow-region-shape.test.ts b/packages/lint/src/validate-flow-region-shape.test.ts index 19fc98b1eb0..2e62db5ce1c 100644 --- a/packages/lint/src/validate-flow-region-shape.test.ts +++ b/packages/lint/src/validate-flow-region-shape.test.ts @@ -81,6 +81,12 @@ describe('[#29049] flow-region-shape-invalid', () => { expect(finding!.message).toContain('nodes'); }); + it('a non-record entry is no flow and draws nothing; the flow after it keeps its own index', () => { + const bad = flow('rgn_lead_time', guard(fanOut(false))); + const findings = validateFlowRegionShape({ flows: [null, 'x', bad] }); + expect(findings.map(({ path }) => path)).toEqual(['flows[2]']); + }); + it('a body with no string name is named by its path', () => { const [finding] = validateFlowRegionShape({ flows: [unparseable()] }); expect(finding).toMatchObject({ where: "flow 'flows[0]'", path: 'flows[0]' }); diff --git a/packages/lint/src/validate-flow-region-shape.ts b/packages/lint/src/validate-flow-region-shape.ts index 4deac76f066..46bdaf4b745 100644 --- a/packages/lint/src/validate-flow-region-shape.ts +++ b/packages/lint/src/validate-flow-region-shape.ts @@ -32,6 +32,12 @@ * register it too. Either way the finding says the flow could not be judged * and carries the schema's issues. * + * An entry that is not a record at all (an empty YAML list item, a string) is + * skipped, as every registry reader skips it (`non-record-object-entry.test.ts`): + * no author wrote a flow there, and `os validate` refuses the collection with + * the stack schema's own sentence. Its index is kept, so the paths of the + * flows around it stay the author's. + * * ## Not judged here * * A cycle in the flow's TOP-LEVEL graph. The engine refuses it with @@ -43,6 +49,8 @@ import { FlowSchema, validateControlFlow } from '@objectstack/spec/automation'; type AnyRec = Record; +const isRec = (v: unknown): v is AnyRec => !!v && typeof v === 'object' && !Array.isArray(v); + /** The rule id every finding of this rule carries. */ export const FLOW_REGION_SHAPE_INVALID = 'flow-region-shape-invalid'; @@ -77,10 +85,11 @@ export function validateFlowRegionShape(stack: AnyRec): FlowRegionShapeFinding[] if (!Array.isArray(flows)) return []; const findings: FlowRegionShapeFinding[] = []; flows.forEach((raw, i) => { + if (!isRec(raw)) return; const path = `flows[${i}]`; const result = FlowSchema.safeParse(raw); if (!result.success) { - const declared = (raw as { name?: unknown } | null)?.name; + const declared = raw.name; const name = typeof declared === 'string' && declared !== '' ? declared : path; const issues = result.error.issues .map((issue) => `${issue.path.length > 0 ? issue.path.join('.') : '(root)'}: ${issue.message}`) From 300aafafcc97f7151fbbed6be6539bf21bf6ac47 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 15:23:19 +0000 Subject: [PATCH 3/3] test(cli): os build --json reports the region refusal under issues; changeset; correct the two pending region changesets Claude-Session: https://claude.ai/code/session_01ADHkmQjqkXDjc5jhjFfHt3 Co-authored-by: Claude --- ...893-flow-region-shape-at-validate-build.md | 2 +- .../29049-flow-region-shape-lint-rule.md | 20 +++++++++++++++++++ .../29059-flow-region-gate-hardening.md | 2 +- .../test/validate-flow-region-shape.test.ts | 7 ++++--- 4 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 .changeset/29049-flow-region-shape-lint-rule.md diff --git a/.changeset/28893-flow-region-shape-at-validate-build.md b/.changeset/28893-flow-region-shape-at-validate-build.md index 75a008f3a1e..de6fd715df3 100644 --- a/.changeset/28893-flow-region-shape-at-validate-build.md +++ b/.changeset/28893-flow-region-shape-at-validate-build.md @@ -9,6 +9,6 @@ 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 '' would be refused at registration: : ` (`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. +Both commands now hand each flow to the automation engine's own registration verdict: `FlowSchema`, then the spec's `validateControlFlow`. A refused flow is reported by the author-time rule `flow-region-shape-invalid` as `Flow '' would be refused at registration: : ` (exit 1), and the build writes nothing to `dist/`; `os lint` reports it too. 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. diff --git a/.changeset/29049-flow-region-shape-lint-rule.md b/.changeset/29049-flow-region-shape-lint-rule.md new file mode 100644 index 00000000000..288c392ac8e --- /dev/null +++ b/.changeset/29049-flow-region-shape-lint-rule.md @@ -0,0 +1,20 @@ +--- +'@objectstack/lint': patch +'@objectstack/cli': patch +--- + +feat(lint): `os lint` refuses a flow whose structured region the automation engine refuses to register (`flow-region-shape-invalid`, ADR-0031) + +Clause-②: no (narrowing) +Metadata-protocol: none + +**BREAKING** accept-set narrowing at `os lint` (`@objectstack/cli`, through the `@objectstack/lint` rule table): 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 `os lint` (exit 1) instead of passing. `os validate` and `os build` already refused it; their verdicts are unchanged. + +- **One rule for all three commands.** The check is now the author-time rule `flow-region-shape-invalid` (`validateFlowRegionShape` and `FLOW_REGION_SHAPE_INVALID`, exported from `@objectstack/lint`). It calls the automation engine's own registration verdict, `FlowSchema` then the spec's `validateControlFlow`, and its message is the engine's refusal behind the flow's name. `os explain flow-region-shape-invalid` prints the full reasoning. +- **What changes at `os validate` and `os build`.** They refuse the same flows as before, now from the rule table, so the refusal prints under `Author-time rules failed` and appears once. Under `--json` it is a rule finding (`rule`, `where`, `path`, `message`, `hint`), in `errors` for `os validate` and in `issues` for `os build`. A flow in a multi-package artifact is located at its place in the artifact's flows (`flows[N]`), with the flow's name in `where`. +- **A flow the flow schema refuses** is reported by the same rule as one that could not be judged, with the schema's issues, never passed. `os validate` and `os build` refuse such a flow at the schema step already; `os lint`, which does not parse, now reports it here. +- **Not judged:** a cycle in a flow's top-level graph, outside any region. + +**The remedy.** Fix the flow the way the engine's sentence says: add one node every exit of the region leads to, remove the edge that loops back inside the region, or give the `parallel` a second branch. + + diff --git a/.changeset/29059-flow-region-gate-hardening.md b/.changeset/29059-flow-region-gate-hardening.md index b797e894b35..1750bf72274 100644 --- a/.changeset/29059-flow-region-gate-hardening.md +++ b/.changeset/29059-flow-region-gate-hardening.md @@ -7,5 +7,5 @@ fix(cli): the os validate / os build flow-region check never passes a flow it co Clause-②: no Metadata-protocol: none -- **No silent pass.** The flow-region check `os validate` and `os build` run (the automation engine's registration verdict, ADR-0031) parses each flow with `FlowSchema` before it judges the regions. When that parse failed it used to skip the flow with no row, so the flow passed unjudged. It now refuses it with a `VALIDATION_ERROR` row at the flow's `path`, naming the flow and the schema's issues. No authored stack reaches this branch today: the stack schema has already parsed every flow the check reads, and `FlowSchema` accepts its own output. Nothing refused or accepted today changes. +- **No silent pass.** The flow-region check `os validate` and `os build` run (the automation engine's registration verdict, ADR-0031) parses each flow with `FlowSchema` before it judges the regions. When that parse failed it used to skip the flow with no row, so the flow passed unjudged. It now refuses it with a `flow-region-shape-invalid` finding at the flow's `path`, naming the flow and the schema's issues. No authored stack reaches this branch today: the stack schema has already parsed every flow the check reads, and `FlowSchema` accepts its own output. Nothing refused or accepted today changes. - **Refused flows in the boot banner.** When the automation engine refuses a declared flow at registration, boot still continues and the per-flow reason is still under Boot diagnostics. The `os serve` / `os dev` banner now also prints one line beside the declared count, for example `⚠ Flows: 2 flow(s) declared, 1 refused at registration`, read from the engine's own record (`getFlowRegistration`). It prints even when every declared flow was refused and nothing registered. A boot with nothing refused prints no such line. diff --git a/packages/cli/test/validate-flow-region-shape.test.ts b/packages/cli/test/validate-flow-region-shape.test.ts index 1930a24fe7c..8096d5b8e47 100644 --- a/packages/cli/test/validate-flow-region-shape.test.ts +++ b/packages/cli/test/validate-flow-region-shape.test.ts @@ -19,7 +19,8 @@ * (`flow-region-shape-invalid`, `@objectstack/lint`), so `os lint` gives it * too: each fixture below is run through all three commands. A refusal is now * a rule finding (`rule`, `where`, `path`, `message`, `hint`) in the rule - * table's `errors`, not a separate gate's `VALIDATION_ERROR` row. + * table's failure envelope (`errors` under `os validate --json`, `issues` under + * `os build --json`), not a separate gate's `VALIDATION_ERROR` row. */ import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; @@ -196,7 +197,7 @@ describe('[#28893] validate, build and lint refuse a flow region the engine refu expect(built.code).toBe(1); const payload = json(built, `build ${label}`); expect(payload.success).toBe(false); - const builtRows = (payload.errors as Array>).filter((r) => String(r.message).includes(refusal!)); + const builtRows = (payload.issues as Array>).filter((r) => String(r.message).includes(refusal!)); expect(builtRows.map((r) => r.rule)).toEqual([RULE]); expect(existsSync(fixtures[label].dist)).toBe(false); @@ -229,7 +230,7 @@ describe('[#28893] validate, build and lint refuse a flow region the engine refu expect(built.code, built.stdout).toBe(1); const payload = json(built, 'build assembled'); expect(payload.success).toBe(false); - const builtRows = (payload.errors as Array>).filter((r) => String(r.message).includes(refusal!)); + const builtRows = (payload.issues as Array>).filter((r) => String(r.message).includes(refusal!)); expect(builtRows.map(({ rule, where, path }) => ({ rule, where, path }))).toEqual([expected]); expect(existsSync(fixtures.assembled.dist)).toBe(false);