Skip to content

Commit 0814b89

Browse files
fix(lint)!: refuse a flow whose loop, parallel or try_catch region is malformed at publish (#29526)
domain:server#2 · os-steve · session_01ADHkmQxwoGb5fSnrFkpxmt <!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9SEsy6LaBvU7eiRnCrSgR6K)_ Fixes #29233 Clause-②: no (narrowing) Metadata-protocol: narrowed Skills-docs: none — no published skill teaches that a malformed region publishes; the automation skill's region rules (one entry, one exit, acyclic) stay true, and the publish door now refuses a region that breaks them earlier, in the engine's words. ## 维护者速读 - 改了什么:#29495 新增的 lint 规则 `validateFlowRegionShape`(`flow-region-shape-invalid`)现在也在运行时发布门上跑:循环体、并行分支或 try/catch 区域写坏了的流程,草稿读取显示 `valid: false`,所有发布入口都拒绝发布。 - 为什么改:以前这种流程草稿显示有效、发布成功,可引擎注册时拒收,流程永远不会运行,作者看不到任何错误。 - 风险与回滚:只改了注册表里一行(`surfaces: CLI_AND_RUNTIME`、`runtimeTypes: ['flow']`),判官就是引擎注册时调用的 `FlowSchema` + `validateControlFlow`,没有第二份判断;回滚即撤销本 PR。 - 先看:`packages/lint/src/authoring-rules.ts` 里 `validateFlowRegionShape` 那一行,再看 `packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts`。 - 两分钟验证:`pnpm --filter @objectstack/metadata-protocol exec vitest run src/runtime-authoring-gate.flow-region.test.ts`,预期 8 个用例全绿(需先 `pnpm --filter @objectstack/lint build`)。 - 需要你做的:无,席位落地。 ## What changes - `@objectstack/lint`: the registry row for `validateFlowRegionShape` (#29495, rule id `flow-region-shape-invalid`) moves from `CLI_ONLY` to `surfaces: CLI_AND_RUNTIME`, `runtimeTypes: ['flow']`, and its `surfaceReason` is retired. The runtime authoring gate (`evaluateRuntimeAuthoringGate`), which the metadata draft read and every publish door run, picks it up through `runRuntimeAuthoringRules` and discloses it in `rulesRun`. No gate-local rule and no second call: `runtime-authoring-gate.ts` is untouched. - The judge is the one the engine's registration uses (`canonicalizeFlow`): `FlowSchema`, then `validateControlFlow` from `@objectstack/spec/automation`. The message is "Flow 'NAME' would be refused at registration: " followed by the engine's sentence, word for word. - Changeset `.changeset/29233-flow-region-shape-publish.md`: `@objectstack/lint` `minor`, BREAKING banner with before/now, `adr-0087: not-required (no-migration-prescription)`. ### Accepted differences from a gate-local rule (seat ruling) 1. The finding is anchored on the flow (`flows[i]`, where `flow 'NAME'`), so the draft read's path is the item root `''`, not the container (`nodes.1`). The sentence names the region. Finer location is #29558. 2. The message carries the registry rule's prefix "Flow 'NAME' would be refused at registration: ". 3. One finding per flow (the engine's first refusal), as the engine itself throws once. 4. The publish doors judge the raw stored draft body; the engine converts first (`applyConversionsToFlow`). Landed as declared under seat ruling A; judging the converted body is #29557. Measurement below. ## Tests - `packages/metadata-protocol/src/runtime-authoring-gate.flow-region.test.ts` (new, pure gate, 8 cases): the card's loop body a → b → a (pins the engine's sentence once; where `flow 'rgnprobe_loop'`, path `flows[0]`, `rulesRun` contains `validateFlowRegionShape`), a one-branch parallel, a try region that closes on itself, a region its schema refuses ("invalid region"), a malformed region nested in another (sentence names both levels), two malformed containers giving one finding; controls: well-formed loop / parallel / try_catch (nested too) raise nothing, a draft is not gated. Every expected message is `validateControlFlow`'s own answer for the case. - `packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts` (real protocol doors): a CASES row for the card's loop (draft saves, draft read `valid:false` with the publish's own rule and message, per-item publish 422, package publish rolls back); a control that a well-formed loop, parallel and try_catch each read `valid:true` and publish (`publishedCount: 3`); a pin that the draft read carries the registry rule's message at the item root (`''`). - `packages/qa/dogfood/test/flow-region-malformed-publish.dogfood.test.ts` (showcase boot, REST): the card's table (draft PUT 200, draft GET `valid:false` with `code flow-region-shape-invalid`, `path ""` and the engine's sentence, publish-drafts `outcome: refused`, no active row); the per-item `POST /meta/flow/:name/publish` and a live `PUT` each 422 with no active row; control: well-formed loop, parallel and try_catch each read valid, publish, and reach `registration.state: registered`. - **Forced test edits** (claim amendment 6585054494): `packages/lint/src/runtime-gate.test.ts` (`brokenApprovalFlow` / `cleanApprovalFlow`) and `packages/lint/src/validate-flow-trigger-readiness.test.ts` (`apiFlow`, `startSecond`). Their runtime-gate fixtures were flows the flow schema refuses (no flow label, unlabelled nodes, a top-level `trigger` key, no edges), which the engine never registers and which this rule now reports on the gate. The fixtures are made schema-valid; no assertion changed. ## Registration / publish measurement (difference 4) - A stored body the flow schema refuses both raw and after conversion (no flow label, a node without a label, a top-level `trigger` key, an unknown flow type, `edges` missing) is refused by the engine's `registerFlow` too: the rule refuses nothing the engine would register. - A legacy D2 spelling (`get_record` `config.filters`) fails `FlowSchema` raw, passes once converted, and `registerFlow` REGISTERS it. Seeded as a stored legacy draft, it is refused at `publishMetaItem` and `publishPackageDrafts` with `[expression-invalid, flow-region-shape-invalid]`: `expression-invalid` already refuses it on main, so this PR adds no new refusal on that spelling. - NOT MEASURED: whether some other legacy spelling is refused by this rule alone. A new draft cannot carry a retired spelling (the draft save refuses it), so only rows stored before those retirements can reach this. ## Acceptance notes - `os validate`, `os build` and `os lint` already ran this rule (#29495); the CLI is unchanged. - The engine throws on the first malformed region of the whole flow, and so does this rule: one finding per flow. ## Verification Head `2cb755ef` (origin/main merged at f99a058, which carries #29495's merge 7f368f5). <details><summary>Red first (tests on the main code)</summary> On the earlier round the same door pins went red on main with the gate answering nothing (8 failed | 38 passed). On this head the ablation below puts main's registry row back (`surfaces: CLI_ONLY`) and the same 8 metadata-protocol cases fail, which is the red-first reading for the registry route. </details> <details><summary>Ablation (registry row back to CLI_ONLY, through lint dist)</summary> `scripts/ablation-replace.mjs` on `packages/lint/src/authoring-rules.ts` (row back to `surfaces: CLI_ONLY` with a marked `surfaceReason`), `pnpm --filter @objectstack/lint build`, `ablation-dist-preflight.mjs @objectstack/lint ABLATION_29233`: marker present in 4 built files. - `runtime-authoring-gate.flow-region.test.ts` + `protocol.draft-publish-verdict.test.ts`: `Tests 8 failed | 37 passed (45)`; the 8 are the six refusal cases of the pure-gate file and the CASES row plus the location pin; the controls and the draft case pass. - Dogfood file: `Tests 2 failed | 1 passed (3)`; the card case and the per-item/live case fail, the control passes. - Restored: blob == HEAD (488c91a), lint rebuilt, preflight `--absent` clean on all 20 built files, `git status --porcelain` empty. </details> <details><summary>Suites, gates, dogfood</summary> All on head 2cb755e after `pnpm install --frozen-lockfile` and a full packages build (78/78 tasks). - `pnpm --filter @objectstack/lint typecheck && test`: Test Files 192 passed (192), Tests 7885 passed (7885). - `pnpm --filter @objectstack/metadata-protocol typecheck && test`: Test Files 308 passed | 4 skipped (312), Tests 22240 passed | 26 skipped (22266). - Lint: `pnpm exec eslint --no-inline-config` (the root `pnpm lint` command) on the six touched TS files, exit 0. - Derived gates: `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` listed 63 families; all 63 run, exit 0. `--ran` reconciliation: "63 derived famil(ies) accounted for — 63 run, 0 NOT-MEASURED (a DERIVED zero)". - Full dogfood battery (`os-verify-lock.sh -c "pnpm --filter @objectstack/dogfood test"`): VERDICT command-exit 0; Test Files 398 passed | 3 skipped (401), Tests 3280 passed | 28 skipped (3308). </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmSfgX3fNF6rAXudbtz)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ADHkmSfgX3fNF6rAXudbtz --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent d3175d5 commit 0814b89

7 files changed

Lines changed: 479 additions & 12 deletions
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/lint': minor
3+
---
4+
5+
**BREAKING** fix(metadata)!: a flow whose loop, parallel or try_catch region is malformed is refused at publish, and its draft read says so, instead of publishing and then never registering
6+
7+
Clause-②: no (narrowing)
8+
Metadata-protocol: narrowed
9+
Skills-docs: none — no published skill teaches that a malformed region publishes; the automation skill's region rules (one entry, one exit, acyclic) stay true, and the publish door now refuses a region that breaks them earlier, in the engine's words.
10+
11+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped. The publish doors now refuse a flow body the automation engine already refuses to register, with the engine's own sentence, so a published body that could never run has nothing a migration could rewrite it to: the author's fix is the one the refusal names. -->
12+
13+
**BREAKING** accept-set narrowing at the runtime authoring gate, which the metadata draft read and every publish door run (`PUT /api/v1/meta/:type/:name` live, `POST …/publish`, `POST /api/v1/packages/:id/publish-drafts`). The `@objectstack/lint` authoring rule `validateFlowRegionShape` (`flow-region-shape-invalid`), which `os validate`, `os build` and `os lint` already run, now also runs on that gate for `flow` writes (`surfaces: CLI_AND_RUNTIME`, `runtimeTypes: ['flow']`).
14+
15+
- **Before.** A flow whose ADR-0031 region was malformed (a `loop` body, a `parallel` branch or a `try_catch` try or catch region, at any depth, with no single entry or exit node, a cycle inside it, a shape its region schema refuses, or a `parallel` block with fewer than two branches) saved as a draft, its draft read answered `_diagnostics: { valid: true }`, and it published (`outcome: 'published'`). The automation engine then refused to register it ("loop 'lp' body: region has no entry node …; region contains a cycle (region bodies must be acyclic)"), so the flow never ran, and only the by-name read's `registration` and a boot-log warning said why.
16+
- **Now.** The draft still saves (drafts are never gated). Its read answers `valid: false` with an error coded `flow-region-shape-invalid`, and every publish door refuses it `422 INVALID_METADATA` with that finding; a package publish refuses the batch (`outcome: 'refused'`). The message is "Flow 'NAME' would be refused at registration: " followed by the engine's registration refusal, word for word. The finding names the flow (one per flow, the engine's first refusal); the sentence names the region.
17+
- **One judge, one name.** The rule calls `FlowSchema`, then `validateControlFlow` (`@objectstack/spec/automation`), the two calls the engine's registration makes. A flow body the flow schema refuses is reported by this rule too ("could not be judged for its region shape"), as on the CLI.
18+
- **Unchanged.** A flow whose regions the engine accepts publishes and registers, as before.
19+
20+
**The remedy.** None for a flow that registers today: it publishes exactly as before. A flow this now refuses was already refused by the engine at registration and never ran; the refusal names the region and what is wrong with it. Give the region one entry node and one exit node with no cycle inside it, and give a `parallel` block at least two branches. A stored flow published before this change is re-registered exactly as before (still refused by the engine).

‎packages/lint/src/authoring-rules.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1981,11 +1981,12 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [
19811981
input: 'parsed',
19821982
commands: ALL,
19831983
source: 'packages/lint/src/validate-flow-region-shape.ts',
1984-
surfaces: CLI_ONLY,
1985-
surfaceReason:
1986-
'Not yet on the runtime publish gate: whether a flow write whose region the engine refuses is refused at '
1987-
+ 'publish is the server lane\'s open question for the metadata write path, decided there; this entry '
1988-
+ 'brings the three authoring commands to the engine\'s registration verdict.',
1984+
// [#29233] Runtime publish gate (#4463): a flow whose region the engine
1985+
// refuses to register can only ever publish as a flow that never runs, so
1986+
// the draft read and every publish door refuse it here, through this one
1987+
// row, rather than through a gate-local copy of the same judgement.
1988+
surfaces: CLI_AND_RUNTIME,
1989+
runtimeTypes: ['flow'],
19891990
run: (stack) => validateFlowRegionShape(stack),
19901991
},
19911992
// The same predicate over a declarative connector's four header/env maps

‎packages/lint/src/runtime-gate.test.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,28 +21,35 @@ import { validatePredicatePathRefs } from './validate-predicate-path-refs.js';
2121
import { validateWidgetBindings } from './validate-widget-bindings.js';
2222

2323
/** The issue's measured example: an approval flow whose expression approver is broken CEL. */
24+
// [#29233] Spelled as a body the flow schema accepts (a `type`, node labels,
25+
// an edge list, no top-level `trigger`), since the publish gate now runs the
26+
// region-shape rule, which refuses a flow the schema refuses. Each case below
27+
// still isolates the one thing it pins: the approver expression.
2428
const brokenApprovalFlow = {
2529
name: 'leave_approval',
2630
label: 'Leave approval',
27-
trigger: { type: 'record_change', object: 'leave_request', events: ['create'] },
31+
type: 'autolaunched',
2832
nodes: [
29-
{ id: 'start', type: 'start' },
33+
{ id: 'start', type: 'start', label: 'Start' },
3034
{
3135
id: 'approve',
3236
type: 'approval',
37+
label: 'Approve',
3338
config: { approvers: [{ type: 'expression', value: 'record.owner ==' }] },
3439
},
3540
],
41+
edges: [{ id: 'e1', source: 'start', target: 'approve' }],
3642
};
3743

3844
/** The same flow with an approver expression that parses and uses a legal root. */
3945
const cleanApprovalFlow = {
4046
...brokenApprovalFlow,
4147
nodes: [
42-
{ id: 'start', type: 'start' },
48+
{ id: 'start', type: 'start', label: 'Start' },
4349
{
4450
id: 'approve',
4551
type: 'approval',
52+
label: 'Approve',
4653
config: {
4754
approvers: [{ type: 'expression', value: 'current.owner' }],
4855
},

‎packages/lint/src/validate-flow-trigger-readiness.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,13 +1099,17 @@ describe('validateFlowTriggerReadiness', () => {
10991099
config: Record<string, unknown>,
11001100
overrides: Record<string, unknown> = {},
11011101
): Record<string, unknown> {
1102+
// [#29233] A label on the flow and its nodes: the runtime publish gate
1103+
// the cases below cross now runs the region-shape rule, which refuses a
1104+
// body the flow schema refuses.
11021105
return {
11031106
name: 'inbound_order',
1107+
label: 'Inbound order',
11041108
type: 'api',
11051109
status: 'active',
11061110
nodes: [
1107-
{ id: 'start', type: 'start', config },
1108-
{ id: 'end', type: 'end' },
1111+
{ id: 'start', type: 'start', label: 'Start', config },
1112+
{ id: 'end', type: 'end', label: 'End' },
11091113
],
11101114
edges: [{ id: 'e1', source: 'start', target: 'end' }],
11111115
...overrides,
@@ -1298,8 +1302,8 @@ describe('validateFlowTriggerReadiness', () => {
12981302
// it `flows[0].nodes[1].config.secret`, the rule's own finding path.
12991303
const startSecond = apiFlow({ hookId: 'intake' }, {
13001304
nodes: [
1301-
{ id: 'end', type: 'end' },
1302-
{ id: 'start', type: 'start', config: { hookId: 'intake' } },
1305+
{ id: 'end', type: 'end', label: 'End' },
1306+
{ id: 'start', type: 'start', label: 'Start', config: { hookId: 'intake' } },
13031307
],
13041308
});
13051309
const at = (paths: string[]) =>

‎packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,57 @@ const CASES: Array<{ rule: string; type: string; item: { name: string } & Record
460460
],
461461
},
462462
},
463+
{
464+
// [#29233] A loop whose body closes on itself (body a → b → a, the
465+
// card's reproduction). It saved, read valid:true and PUBLISHED, and
466+
// the engine's region check then refused to register it.
467+
rule: 'flow-region-shape-invalid',
468+
type: 'flow',
469+
item: regionFlow('region_loop', {
470+
id: 'lp',
471+
type: 'loop',
472+
label: 'Loop',
473+
config: {
474+
collection: [1, 2],
475+
iteratorVariable: 'row',
476+
body: {
477+
nodes: [
478+
{ id: 'step_a', type: 'assignment', label: 'A' },
479+
{ id: 'step_b', type: 'assignment', label: 'B' },
480+
],
481+
edges: [
482+
{ id: 'b1', source: 'step_a', target: 'step_b' },
483+
{ id: 'b2', source: 'step_b', target: 'step_a' },
484+
],
485+
},
486+
},
487+
}),
488+
},
463489
];
464490

491+
/** [#29233] start → CONTAINER → end, the container's region carrying the case. */
492+
function regionFlow(name: string, container: Record<string, unknown>) {
493+
return {
494+
name,
495+
label: name,
496+
type: 'autolaunched',
497+
nodes: [
498+
{ id: 'start', type: 'start', label: 'Start' },
499+
container,
500+
{ id: 'end', type: 'end', label: 'End' },
501+
],
502+
edges: [
503+
{ id: 'e1', source: 'start', target: container.id as string },
504+
{ id: 'e2', source: container.id as string, target: 'end' },
505+
],
506+
};
507+
}
508+
509+
/** [#29233] One straight body of a single step, the well-formed region shape. */
510+
function oneStep(id: string) {
511+
return { nodes: [{ id, type: 'assignment', label: id }], edges: [] };
512+
}
513+
465514
/** Stage one env-wide, package-bound draft (Studio's autosave shape). */
466515
async function stageDraft(protocol: ObjectStackProtocolImplementation, type: string, item: { name: string }) {
467516
return (protocol as any).saveMetaItem({ type, name: item.name, item, packageId: PKG, mode: 'draft' });
@@ -541,6 +590,58 @@ describe('a draft reads the verdict its publish gives (#26707)', () => {
541590
expect(published).toMatchObject({ success: true, failedCount: 0 });
542591
});
543592

593+
it('[#29233] a well-formed loop, parallel and try_catch region each still read valid:true and publish', async () => {
594+
const { engine } = makeStubEngine();
595+
const protocol = new ObjectStackProtocolImplementation(engine);
596+
const flows = [
597+
regionFlow('ok_loop', {
598+
id: 'lp', type: 'loop', label: 'Loop',
599+
config: { collection: [1, 2], iteratorVariable: 'row', body: oneStep('loop_step') },
600+
}),
601+
regionFlow('ok_parallel', {
602+
id: 'fan', type: 'parallel', label: 'Fan',
603+
config: { branches: [oneStep('left_step'), oneStep('right_step')] },
604+
}),
605+
regionFlow('ok_try', {
606+
id: 'guard', type: 'try_catch', label: 'Guard',
607+
config: { try: oneStep('try_step'), catch: oneStep('catch_step') },
608+
}),
609+
];
610+
for (const flow of flows) await stageDraft(protocol, 'flow', flow);
611+
612+
for (const flow of flows) expect(await draftDiagnostics(protocol, 'flow', flow.name)).toEqual({ valid: true });
613+
const published: any = await protocol.publishPackageDrafts({ packageId: PKG });
614+
expect(published).toMatchObject({ success: true, publishedCount: 3, failedCount: 0 });
615+
});
616+
617+
it('[#29233] the region verdict on the draft read is the registry rule\'s: the engine\'s sentence, at the item root', async () => {
618+
const { engine } = makeStubEngine();
619+
const protocol = new ObjectStackProtocolImplementation(engine);
620+
// A one-branch parallel block nested in a try region. The registry rule
621+
// judges the flow as one unit (as the engine does) and anchors the
622+
// flow itself, so the item-relative path is the item root; the
623+
// sentence names the region.
624+
const flow = regionFlow('nested_region', {
625+
id: 'guard', type: 'try_catch', label: 'Guard',
626+
config: {
627+
try: {
628+
nodes: [{ id: 'fan', type: 'parallel', label: 'Fan', config: { branches: [oneStep('only_step')] } }],
629+
edges: [],
630+
},
631+
catch: oneStep('catch_step'),
632+
},
633+
});
634+
await stageDraft(protocol, 'flow', flow);
635+
636+
const diagnostics = await draftDiagnostics(protocol, 'flow', flow.name);
637+
expect(diagnostics.valid).toBe(false);
638+
expect((diagnostics.errors ?? []).filter((e) => e.code === 'flow-region-shape-invalid')).toEqual([{
639+
path: '',
640+
message: "Flow 'nested_region' would be refused at registration: parallel 'fan': a parallel block needs at least 2 branches",
641+
code: 'flow-region-shape-invalid',
642+
}]);
643+
});
644+
544645
it('[#29025] the same loop with its closing edge typed back still reads valid:true and publishes', async () => {
545646
const { engine } = makeStubEngine();
546647
const protocol = new ObjectStackProtocolImplementation(engine);

0 commit comments

Comments
 (0)