Skip to content

Commit 44a030b

Browse files
fix(cli): os validate --target allows the ADR-0031 parallel / try_catch containers, and the restriction profile docblocks state the reader rules (#31277)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XAhN7jUu184vzuXHSCNx6Hb)_ Fixes #31239 Fixes #31264 Clause-②: yes (widening) Metadata-protocol: none ## 维护者速读 - **改了什么**:`os validate --target` 读限制配置时,把引擎自带的 `parallel` / `try_catch` 容器节点当作结构节点(和 `start` / `end` 一样)一律放行,容器里面的每个节点照常按白名单判。spec 里 `allowedFlowNodeTypes` 与 `allowCode` 两段注释写明读者规则,其中 `allowCode` 一段写明 `devPlugins` 不按代码判(#31276 已让 `os build` 不再把它写进产物)。另有单元测试与一个 patch changeset。 - **为什么改**:白名单只能写 `FlowNodeAction` 里的类型,而这两个容器不在其中,于是禁止代码的环境把它们当插件节点拒掉,而且任何配置都放行不了(#31239 第 1 项)。您已选定方案 C(#31354)。 - **风险与回滚**:只放宽一个本地校验命令,以前被拒的流程现在通过;不改任何 schema 形状或取值集合,不碰运行时。cloud 是否从 `manifest_json` 读 `devPlugins` 仍未测,见 Acceptance notes。回滚即 revert。 - **先看**:`packages/cli/src/utils/restriction-profile.ts` 的 `FLOW_CONTAINER_NODE_TYPES` 与 `judgeNodeType` 首行;`packages/spec/src/system/restriction-profile.zod.ts` 两段「Reader rules」注释。 - **两分钟验证**:`pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 src/utils/restriction-profile.test.ts`,预期 23 个全部通过,其中两条是本卡新增(容器放行、`loop` 仍按白名单判)。 - **需要你做的**:无,席位落地。 ## What changes 1. **Item 1, option C.** Ruled C by the maintainer (decision card #31354, comment 6717120942, 2026-11-08). `packages/cli/src/utils/restriction-profile.ts`: reader rule 1 now covers `FLOW_CONTAINER_NODE_TYPES`, derived from the published `PARALLEL_NODE_TYPE` / `TRY_CATCH_NODE_TYPE` constants (`@objectstack/spec/automation`). Those two containers are always allowed, like `start` / `end`, and every node inside their regions is still judged, because `collectFlowGraphs` walks into `branches[]`, `try` and `catch`. `loop` (`LOOP_NODE_TYPE`) is already a `FlowNodeAction` id, so `allowedFlowNodeTypes` keeps judging it like any built-in. No vocabulary change: `allowedFlowNodeTypes` is still `z.array(FlowNodeAction)`. 2. **Item 3.** `packages/spec/src/system/restriction-profile.zod.ts`, JSDoc only (no `.describe()` text, no shape): the `allowedFlowNodeTypes` docblock states the reader rules. `start` / `end` and the two containers are structural and always allowed. A `script` node is also subject to `allowCode`. A type outside the profile's vocabulary cannot be named by the list, so `allowCode` governs it (ruling 6710350208); the docblock no longer says such a type comes from a code plugin, since the vocabulary is set to grow (#31344). 3. **Item 2, the spec half of the `devPlugins` work.** Before the first round the thread measured that `os compile` wrote `devPlugins` into `dist/objectstack.json` and `os package publish` snapshots that file into `sys_package_version.manifest_json`, so the seat moved the carve-out to the other card (claim amendment 6712791333). PR #31276 (merged as `ea053b4a`) made the artifact write delete `devPlugins` beside `demoStaff` and added the reader's `NEVER_REACHES_TARGET` carve-out. This branch merges `origin/main` (no rebase), keeps that carve-out next to `FLOW_CONTAINER_NODE_TYPES` in the one conflict, and writes the sentence #31276 left owed into the `allowCode` docblock: `devPlugins` is not judged by `allowCode`, because only a local development boot loads it and `os build` leaves it out of the artifact a target installs, while `UNMANAGED_INSTALL_KEY_DISPOSITIONS` still classifies it as `code` for an unmanaged install. Checked against the merged tree: `compile.ts` deletes `finalBundle.devPlugins`, and the table still reads `devPlugins: 'code'`. 4. `.changeset/restriction-profile-adr-0031-containers.md`: `@objectstack/cli` and `@objectstack/spec` patch (the spec JSDoc ships in its `.d.ts`), both declaration lines at the head of the body. ## Tests `packages/cli/src/utils/restriction-profile.test.ts`, two new cases under "the reader rules": - `parallel / try_catch are always allowed`: a flow holding both containers, with a `notify` node in a parallel branch, a `get_record` in `try` and a `notify` in `catch`. Under `allowCode: false` and a list of `['get_record']`, no row names a container and exactly the two `notify` nodes are refused by `allowedFlowNodeTypes` at their region paths. With `notify` added to the list, nothing is refused. The case also pins that neither id is in `FLOW_BUILTIN_NODE_TYPES`. - `loop is a FlowNodeAction id`: a `loop` not in the list is refused by `allowedFlowNodeTypes`. <details><summary>Ablation (one-off, not kept)</summary> `node scripts/ablation-replace.mjs` on `restriction-profile.ts`, replacing the rule-1 line with the pre-change `if (FLOW_STRUCTURAL_NODE_TYPES.includes(type)) return undefined;` (anchor 1 → 0, blob `dbc0daaabec5` → `2b0aa2d61236`): the container case fails (`Tests 1 failed | 22 passed (23)`). Restored: blob equals HEAD `dbc0daaabec5`, `git diff HEAD` empty. The CLI unit suite runs `src/`, so no dist leg applies. </details> ## Acceptance notes - The order asked whether `loop` is already a `FlowNodeAction`: it is (`flow.zod.ts`, `'loop', // For Each`), so only `parallel` and `try_catch` needed the rule. - Open residual, not measured: whether `objectstack-ai/cloud` reads `devPlugins` out of `sys_package_version.manifest_json`. That repository is not reachable from this session (as #31276 also recorded), and an artifact built before #31276 still carries the key. The docblock speaks for what `os build` writes now. - The claim's line reads `Clause-②: yes`; this body and the changeset write `yes (widening)`, the same value with the arm the order expects (the profile now admits graphs it refused). ## Verification Head `2a6b5bfe` adds one JSDoc-only commit on `e7693b99` (the ruled wording of the outside-the-vocabulary rule): `pnpm --filter @objectstack/spec run typecheck` exit 0, `src/system/restriction-profile.test.ts` 27/27, cli `src/utils/restriction-profile.test.ts` 24/24, `check:docs`, `check:api-surface`, `check-changeset-no-major` and `check-nul-bytes` exit 0. Local, on the merged head `e7693b99` (merge of `origin/main` `59d9815a`, which carries #31276, plus the docblock commit), after `pnpm --filter '@objectstack/cli...' build`: - `dispatch-gates --commands`: 83 derived, 81 exit 0, 2 NOT MEASURED (`check:dual-build-cjs-loads` and `check:i18n-coverage`, both exit 3 PREREQUISITE NOT MET: they need every package's `dist`, CI's). `--ran` reconciles 83 accounted, 0 unrun. - `pnpm --filter @objectstack/spec test`: 821 files / 25964 tests passed (1 todo). `pnpm --filter @objectstack/spec run typecheck` exit 0. - `pnpm --filter @objectstack/cli exec vitest run --project unit`: 401 files / 5666 tests passed, #31276's reader case and these two container cases together. `test/validate-target-restriction-profile.test.ts` and `test/build-demo-staff-artifact.test.ts` (the door and the artifact drop): 2 files / 11 tests passed. `pnpm --filter @objectstack/cli run typecheck` exit 0. - First round (head `405b3d48`): `check:generated` 13 of 14 current, `test-typecheck-debt.json` stale only from `tsc` dying under the default heap; `check:test-typecheck` with a 4 GB heap OK, nothing regenerated. CI runs the full set. --- _Generated by [Claude Code](https://claude.ai/code/session_01FEkQ5kxRsYPLNnBt5AWYj1)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01FEkQ5kxRsYPLNnBt5AWYj1 <!-- signed: domain:spec#1 · os-project-manager · session_01FEkQ5mw7HFYn9o4LEGMQdK --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent eff90af commit 44a030b

4 files changed

Lines changed: 105 additions & 4 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/cli': patch
3+
'@objectstack/spec': patch
4+
---
5+
6+
fix(cli): `os validate --target` no longer refuses the engine's `parallel` and `try_catch` containers
7+
8+
Clause-②: yes (widening)
9+
Metadata-protocol: none
10+
11+
A restriction profile's `allowedFlowNodeTypes` can only list `FlowNodeAction` ids, and the ADR-0031 containers `parallel` and `try_catch` are engine built-ins that are not among them. Under `allowCode: false`, `os validate --target` refused them as if a code plugin had registered them, and no profile could allow them. They are now structural, like `start` and `end`: always allowed, while every node inside their branches, `try` and `catch` regions is still judged. `loop` is a `FlowNodeAction` id and is still judged by the list.
12+
13+
The `allowedFlowNodeTypes` docblock on `RestrictionProfileSchema` now states the reader rules: `start` / `end` and the two containers are always allowed, a `script` node is also subject to `allowCode`, and a node type outside the vocabulary is governed by `allowCode`. The `allowCode` docblock states which keys a reader takes as code, and that `devPlugins` is not one of them: `os build` leaves it out of the artifact a target installs. No schema shape or value set changes.

‎packages/cli/src/utils/restriction-profile.test.ts‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
1111
import { tmpdir } from 'node:os';
1212
import { join } from 'node:path';
1313
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
14-
import { FLOW_BUILTIN_NODE_TYPES } from '@objectstack/spec/automation';
14+
import { FLOW_BUILTIN_NODE_TYPES, LOOP_NODE_TYPE, PARALLEL_NODE_TYPE, TRY_CATCH_NODE_TYPE } from '@objectstack/spec/automation';
1515

1616
import {
1717
judgeRestrictionProfile,
@@ -142,6 +142,46 @@ describe('judgeRestrictionProfile — the reader rules', () => {
142142
expect(rowsOf(profile({ allowedFlowNodeTypes: [] }), stack)).toEqual([]);
143143
});
144144

145+
/** One flow holding both ADR-0031 containers that FlowNodeAction does not name, each with a node inside. */
146+
const CONTAINERS = {
147+
manifest: STACK.manifest,
148+
flows: [flow('rp_blocks', [
149+
node('start', 'start'),
150+
node('fan', PARALLEL_NODE_TYPE, { config: { branches: [{ nodes: [node('send', 'notify')], edges: [] }] } }),
151+
node('guard', TRY_CATCH_NODE_TYPE, {
152+
config: { try: { nodes: [node('fetch', 'get_record')], edges: [] }, catch: { nodes: [node('warn', 'notify')], edges: [] } },
153+
}),
154+
node('end', 'end'),
155+
])],
156+
};
157+
158+
it('parallel / try_catch are always allowed — by a list that omits them AND under allowCode: false — while their nodes are judged', () => {
159+
expect(FLOW_BUILTIN_NODE_TYPES).not.toContain(PARALLEL_NODE_TYPE);
160+
expect(FLOW_BUILTIN_NODE_TYPES).not.toContain(TRY_CATCH_NODE_TYPE);
161+
const rows = rowsOf(profile({ allowCode: false, allowedFlowNodeTypes: ['get_record'] }), CONTAINERS);
162+
expect(rows.filter((r) => r.where.endsWith(`(${PARALLEL_NODE_TYPE})`) || r.where.endsWith(`(${TRY_CATCH_NODE_TYPE})`))).toEqual([]);
163+
expect(rows.map(({ rule, path }) => ({ rule, path }))).toEqual([
164+
{ rule: 'allowedFlowNodeTypes', path: 'flows[0].nodes[1].config.branches[0].nodes[0]' },
165+
{ rule: 'allowedFlowNodeTypes', path: 'flows[0].nodes[2].config.catch.nodes[0]' },
166+
]);
167+
expect(rowsOf(profile({ allowCode: false, allowedFlowNodeTypes: ['get_record', 'notify'] }), CONTAINERS)).toEqual([]);
168+
});
169+
170+
it('loop is a FlowNodeAction id, so allowedFlowNodeTypes judges it — it is not a structural container', () => {
171+
expect(FLOW_BUILTIN_NODE_TYPES).toContain(LOOP_NODE_TYPE);
172+
const stack = {
173+
manifest: STACK.manifest,
174+
flows: [flow('rp_each', [
175+
node('start', 'start'),
176+
node('each', LOOP_NODE_TYPE, { config: { body: { nodes: [node('send', 'notify')], edges: [] } } }),
177+
node('end', 'end'),
178+
])],
179+
};
180+
expect(rowsOf(profile({ allowedFlowNodeTypes: ['notify'] }), stack)).toEqual([
181+
{ rule: 'allowedFlowNodeTypes', where: "flow 'rp_each' › node 'each' (loop)", path: 'flows[0].nodes[1]' },
182+
]);
183+
});
184+
145185
it('a type outside FlowNodeAction is governed by allowCode: refused naming allowCode when it is false', () => {
146186
const rows = judgeRestrictionProfile(STACK, profile({ allowCode: false })).filter((r) => r.where.includes('(approval)'));
147187
expect(rows.map((r) => r.rule)).toEqual(['allowCode']);

‎packages/cli/src/utils/restriction-profile.ts‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,13 @@
4343
*
4444
* 1. `start` / `end` (`FLOW_STRUCTURAL_NODE_TYPES`) are always allowed: the
4545
* engine handles them without an executor, and a profile that had to list
46-
* them to allow any flow at all would be a trap.
46+
* them to allow any flow at all would be a trap. So are the ADR-0031
47+
* containers `parallel` and `try_catch` ({@link FLOW_CONTAINER_NODE_TYPES}):
48+
* engine built-ins that are not `FlowNodeAction` ids, so no profile could
49+
* name them, and reader rule 2 would otherwise refuse them as code. A
50+
* container only holds regions; every node inside it is still judged.
51+
* (`loop`, the third container, IS a `FlowNodeAction` id, so
52+
* `allowedFlowNodeTypes` judges it like any built-in.)
4753
* 2. A node type outside the spec's built-in registry (`FlowNodeAction` /
4854
* `FLOW_BUILTIN_NODE_TYPES`) cannot be named by `allowedFlowNodeTypes`, so
4955
* it is governed by `allowCode`, and its refusal names that rule.
@@ -59,6 +65,8 @@ import { isAbsolute, join, resolve } from 'node:path';
5965
import {
6066
FLOW_BUILTIN_NODE_TYPES,
6167
FLOW_STRUCTURAL_NODE_TYPES,
68+
PARALLEL_NODE_TYPE,
69+
TRY_CATCH_NODE_TYPE,
6270
collectFlowGraphs,
6371
type FlowNodeParsed,
6472
} from '@objectstack/spec/automation';
@@ -150,6 +158,14 @@ export function loadRestrictionProfile(
150158
return { profile: parsed.data, file };
151159
}
152160

161+
/**
162+
* Reader rule 1, second half: the ADR-0031 containers the engine ships that
163+
* `FlowNodeAction` does not name, read off the spec's own type-id constants.
164+
* Structural like `start` / `end`: always allowed, while the nodes in their
165+
* regions are judged (`collectFlowGraphs` walks into every region).
166+
*/
167+
const FLOW_CONTAINER_NODE_TYPES: readonly string[] = [PARALLEL_NODE_TYPE, TRY_CATCH_NODE_TYPE];
168+
153169
/**
154170
* The keys the spec classifies as code that never reach a target, so
155171
* `allowCode` does not read them. `devPlugins` is loaded only by a local
@@ -300,8 +316,8 @@ function judgeNodeType(
300316
allowCode: boolean,
301317
allowed: ReadonlySet<string>,
302318
): { rule: RestrictionRule; message: string } | undefined {
303-
// Reader rule 1: structural types are always allowed.
304-
if (FLOW_STRUCTURAL_NODE_TYPES.includes(type)) return undefined;
319+
// Reader rule 1: structural types and the ADR-0031 containers are always allowed.
320+
if (FLOW_STRUCTURAL_NODE_TYPES.includes(type) || FLOW_CONTAINER_NODE_TYPES.includes(type)) return undefined;
305321
// Reader rule 3: a `script` node calls code; under `allowCode: false` that rule alone refuses it.
306322
if (type === 'script' && !allowCode) {
307323
return {

‎packages/spec/src/system/restriction-profile.zod.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,18 @@ export const RestrictionProfileSchema = lazySchema(() => strictObject(
6666
* named handler `functions`, and code plugins.
6767
*
6868
* `false` refuses every one of them; `true` allows them.
69+
*
70+
* ## Reader rule
71+
*
72+
* Besides `hooks`, a reader takes what is code from the spec's one
73+
* classification of a package body's keys, `UNMANAGED_INSTALL_KEY_DISPOSITIONS`
74+
* (`@objectstack/spec/kernel`, disposition `code`), and a key that carries
75+
* nothing refuses nothing. `devPlugins` is not judged by `allowCode`: it
76+
* is loaded only by a local development boot, and `os build` leaves it out
77+
* of the artifact a target installs, so it never reaches a target;
78+
* `UNMANAGED_INSTALL_KEY_DISPOSITIONS` still classifies it as `code` for an
79+
* unmanaged install. A `script` flow node is also subject to this rule (see
80+
* `allowedFlowNodeTypes`).
6981
*/
7082
allowCode: z
7183
.boolean()
@@ -91,6 +103,26 @@ export const RestrictionProfileSchema = lazySchema(() => strictObject(
91103
* forbidden; a type the registry does not name is refused here.
92104
*
93105
* An empty array allows no built-in node type.
106+
*
107+
* ## Reader rules
108+
*
109+
* A reader judges every node of every flow, at every region depth, by
110+
* these rules, and names exactly one dimension per refused node:
111+
*
112+
* - `start` / `end` are structural and always allowed, listed or not: the
113+
* engine runs them without an executor, so a profile never has to list
114+
* them for a flow to run at all.
115+
* - The ADR-0031 containers `parallel` and `try_catch` are structural too
116+
* and always allowed: engine built-ins that are not `FlowNodeAction` ids,
117+
* so no profile could list them. A container only holds regions, and
118+
* every node inside them is still judged. (`loop`, the third container,
119+
* is a `FlowNodeAction` id and is judged by this list.)
120+
* - A `script` node calls a registered function, so it is also subject to
121+
* `allowCode`: under `allowCode: false` it is refused by `allowCode`
122+
* alone, and under `allowCode: true` by this list.
123+
* - A node type outside the profile's vocabulary cannot be named by this
124+
* list, so `allowCode` governs it: refused under `allowCode: false`,
125+
* allowed under `allowCode: true`.
94126
*/
95127
allowedFlowNodeTypes: z
96128
.array(FlowNodeAction)

0 commit comments

Comments
 (0)