You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 369bcbe
Browse filesBrowse the repository at this point in the historyBrowse files
feat(spec): DecisionConfigSchema declares an optional mode: 'exclusive' | 'inclusive' (contract half of the #15429 ruling) (#20162)
Fixes#19867
Clause-②: yes
The contract half of the maintainer ruling on #15429 (record 5793803317,
item 2), landed first by that ruling's own split order: spec key first,
then the engine semantics and the `os migrate meta` conversion together
in one PR, then the docs. This PR carries the spec key ONLY. The
traversal change, the conversion, the lint hint, the status-quo pin
rewrite and `flows.mdx` all stay with #15429 and are not touched here.
Ruling text, verbatim (item 2):
> **显式包容**:要「所有成立的分支都走」,作者必须在判断节点上**显式声明**(工作名 `mode: 'inclusive'`,对应
BPMN 包容网关、n8n 的那个开关)。这是 `DecisionConfigSchema` 上一个可选键 ⇒ `packages/spec`
契约变更,`Clause-②: yes`,实现 PR 走 `needs:contract-review`。
## What changes
`DecisionConfigSchema`
(`packages/spec/src/automation/schemaless-node-config.zod.ts`) gains one
optional key:
```ts
mode: z.enum(['exclusive', 'inclusive'], {
error: (issue) => (issue.code === 'invalid_value' ? decisionModePrescription(issue.input) : undefined),
}).optional()
```
- **Accepted**: `mode` omitted, `'exclusive'`, `'inclusive'`, and either
one next to a `conditions` list.
- **Refused**: every other value, at path `['mode']`, issue code
`invalid_value`, with one prescription.
- **No `.default('exclusive')`, on purpose.** A default would change the
parsed output every consumer of this schema sees: `parse({})` would
start returning `{ mode: 'exclusive' }`. "Omitted means exclusive" lives
in the contract's prose and in the reader that will honour it. It is
pinned: `parse({})` returns `{}` with no `mode` key, and the published
JSON Schema carries no `default`.
- **Types**: `DecisionConfig['mode']` and `DecisionConfigParsed['mode']`
are both `'exclusive' | 'inclusive' | undefined`.
### New `.describe()` text (quoted in full)
> Declares how many out-edges an edge-branched decision takes when more
than one out-edge condition holds: 'exclusive' = only the first, in the
order the edges are declared (what an omitted mode means); 'inclusive' =
every one that holds. Declared ahead of the engine change that reads it:
until that lands, an edge-branched decision takes every out-edge whose
condition holds, whatever this says. A conditions list is first-match on
its own.
### New refusal prescription (quoted in full, rendered for input
`'all'`)
> `mode: 'all'` is not a decision mode. `mode` is the closed pair
'exclusive' | 'inclusive'. 'exclusive' declares that only the FIRST
out-edge whose condition holds is taken, in the order the edges are
declared (the BPMN exclusive gateway), and is what an omitted `mode`
means. 'inclusive' declares that EVERY out-edge whose condition holds is
taken (the BPMN inclusive gateway). Write one of the two, or omit the
key for exclusive.
This is one message for every wrong value, not a did-you-mean. The
likely wrong values (`'all'`, `'first'`, `'parallel'`, `true`) are not
typos an edit distance can reach. They are the same idea written the way
another engine writes it, so the message explains what each legal value
means. The message is a schema-level `error`, so it also wins over the
`objectStackErrorMap` a validator may pass per parse. The ablation below
shows that without it, that map's generic "Invalid value" text would
take its place.
### Docblocks, and why this wording (the measured choice the dispatch
asked for)
The card and triage note 2 ask that the text not claim first-match
before the engine does. I measured the window first:
- `logic-nodes.ts` (the decision executor) reads `config.conditions` and
nothing else.
- The engine's traversal (`engine.ts`, the conditional-edge loop) reads
no decision config key at all.
- So at this head nothing reads `mode`. An edge-branched decision takes
every out-edge whose condition holds, one after another, whatever `mode`
says. `decision-overlapping-edge-conditions.pin.test.ts` pins exactly
that as the status quo, and it stays green here (numbers below).
The wording therefore does three things:
1. **It states what each value declares.** It does not say what the
engine does.
2. **It says in the author-facing text itself that nothing reads the key
yet**, and what the engine does in the meantime. This is in the
`.describe()`, the docblock and the changeset.
3. **It removes the existing false statement instead of repeating it.**
Before this PR, the module header and the `DecisionConfigSchema`
docblock both called the edge-branched shape "a plain BPMN exclusive
gateway". That is the #15429 defect written as fact. Both sentences are
rewritten. The module header's "`conditions` is its only key" becomes
false with this PR, so that sentence is rewritten too.
On scope: `mode` speaks about the out-edges. The ruling addresses
exactly that shape: its item 1 is about a decision with no
`config.conditions`, and its conversion rewrites exactly those
decisions. A `conditions` list is already first-match
(`logic-nodes.ts`), and the ruling leaves it so. What `mode:
'inclusive'` should do next to a `conditions` list is NOT decided here.
It is an open question for #15429 (see Acceptance notes). This PR
neither forbids the combination nor gives it a meaning.
The docblock paragraph that starts "Declared ahead of its enforcement"
says it is rewritten together with #15429's engine change. That is where
the ruling's item 5 puts the rewrite.
## Tests (pinned at `5fbb7ed8`)
New `describe('DecisionConfigSchema.mode …')` in
`packages/spec/src/automation/schemaless-node-config.test.ts`:
- **Accepts**: `{}` is returned as `{}` with no `mode` injected; `{
mode: 'exclusive' }`; `{ mode: 'inclusive' }`; `mode` next to
`conditions`.
- **Refuses**: `'all'`, `'first'`, `'parallel'`, `'Inclusive'`, `true`
and `null`. Each case checks all three parts of the refusal: exactly one
issue, `code === 'invalid_value'`, `path` equal to `['mode']`, and the
prescription (the echoed input, the closed pair, and what an omitted key
means). Each also checks the message carries no tracker number.
- **Error-map precedence**: the prescription survives `safeParse(…, {
error: objectStackErrorMap })`.
- **tsc door**: `expectTypeOf` pins the closed pair on both
`DecisionConfig` and `DecisionConfigParsed`.
- **JSON Schema**: `mode` is `enum: ['exclusive', 'inclusive']` with no
`default`, not `required`, and there is no top-level
`anyOf`/`oneOf`/`allOf`.
- **Key-set pin updated on purpose**: `['conditions']` becomes
`['conditions', 'mode']`. The pin's comment now names the cross-repo
consequence (see Acceptance notes).
Runs (all through `os-verify-lock`; exit codes captured before any
pipe):
| What | Result |
|:--|:--|
| `@objectstack/spec` full suite (`vitest run --project local`) | **540
files / 15,807 passed, 2 todo** |
| `@objectstack/spec` `typecheck` (tsc, scripts program, test program) |
exit 0. The test file is in `tsconfig.test.json`'s program
(`--listFilesOnly`: 1 hit among 511 test files). `check:test-typecheck`
holds this file at its 1 pre-existing pinned signature, so the new
`expectTypeOf` lines add no errors |
| Consumer: `@objectstack/service-automation`, full | **145 files /
1,729 passed**. It includes `config-expression-ledger.test.ts` (the only
test there reading `getSchemalessNodeConfigJsonSchemas()`),
`decision-overlapping-edge-conditions.pin.test.ts` (the status-quo pin:
still green, so no behaviour moved), `decision-branch-routing.test.ts`,
`logic-nodes.test.ts` and `config-schemas.test.ts` |
| Consumer: `@objectstack/metadata-protocol`, full | **188 files passed,
3 skipped (env-gated) / 2,686 tests passed, 19 skipped**. It includes
`reference-sites.derivation.test.ts`: `reference-sites.ts` walks
`SCHEMALESS_NODE_CONFIG_SCHEMAS`, and an enum is not a name-shaped site
|
| `@objectstack/lint` | not a consumer: nothing in `packages/lint`
imports `DecisionConfigSchema` or `SCHEMALESS_NODE_CONFIG_SCHEMAS`
(repo-wide `git grep` of both names) |
**Ablation (one-time proof, no permanent file).** Done after the commit,
through `scripts/ablation-replace.mjs`. The mutation replaced the
schema-level `error` with `error: () => undefined`. Anchor hits went 1 →
0 and the blob changed `67cb1f11` → `02b7fc37`. The file then read **7
failed / 30 passed**: the six refusal cases plus the error-map case.
After the restore, `git hash-object` equals the HEAD blob `67cb1f11`,
and `git diff HEAD` is empty.
**Reverse verification against the REBUILT
`dist/automation/index.d.ts`.** A scratch program outside the repo
assigned `{ mode: 'all' }` to `DecisionConfig`. tsc exited 2 with
`TS2322: Type '"all"' is not assignable to type '"inclusive" |
"exclusive" | undefined'`. The control leg, with only `{ mode:
'inclusive' }`, exited 0. From `service-automation`'s own resolution of
`@objectstack/spec/automation` (the rebuilt dist), `{ mode: 'all' }` is
refused with `invalid_value` at `['mode']`, and `{ mode: 'inclusive' }`
is accepted.
### Generated artifacts
Regenerated, not hand-edited:
- `packages/spec/authorable-surface/automation.json`:
+`automation/DecisionConfig:mode` (from the build).
- `content/docs/references/automation/schemaless-node-config.mdx`: from
`gen:docs`, after `check:generated` named it the one stale artifact.
`authorable-surface.base.json` is untouched. `check:generated` then
passes all 15 artifacts, including `check:liveness` and
`check:authorable-surface`.
### Gates
Derived from the actual change set with `dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` at `5fbb7ed8`: 107 commands, every
one run with its exit code written to disk. `--ran` reconciliation:
**107 derived, 105 run green, 2 NOT MEASURED, 0 UNRUN.**
- NOT MEASURED: `check:dual-build-cjs-loads`, reason: PREREQUISITE (exit
3). It needs a dist for every workspace package, which means a
whole-workspace build. In its place I loaded every CJS `require` entry
of `@objectstack/spec` itself: 18 loaded, 0 failed.
- NOT MEASURED: `check:type-check-debt`, reason: PREREQUISITE (exit 3).
It needs the whole-workspace `.d.ts` closure (lint.yml builds
`./packages/*` first). This PR touches no DEBT-ledgered package's
TypeScript.
- Also run: the roster gates whose rosters sit under my paths
(`check:authz-resolver`, `check:error-code-casing`,
`check:filter-alias-parity`, `check:error-code-provenance`), all exit 0.
- The level axis of `check-changeset-no-major` was driven offline with
`--event` on a `pull_request` payload carrying THIS body. It exited 0
and printed: `LEVEL AXIS: this PR declares clause-② yes, and no package
whose packages/**/src/** it moves is graded patch` (the changeset grades
`@objectstack/spec` `minor`).
## Acceptance notes
- **objectui, measured read-only at the `.objectui-sha` pin
`f8a9d0fb`.** `flow-node-config.spec-reconciliation.test.ts` reconciles
its hand-written decision form against `DecisionConfigSchema.shape` in
both directions. At the pin, the decision form offers only `conditions`
(plus the legacy-gated `condition`). So once objectui installs a
published spec that carries `mode`, its "read by the executor but not
offered by the designer form" assertion reds for `decision`. It stays
red until the form offers `mode`.
- This does NOT touch this repo's CI: the Console Pin Gate builds
objectui at the pin and runs none of its tests.
- No objectui edit here. The objectui half is best landed with or after
#15429's engine change, so the designer does not offer a switch the
engine ignores.
- The key-set pin in this repo now names this consequence.
- **Open question for #15429, not decided here**: what `mode:
'inclusive'` means on a decision that ALSO declares `conditions`. That
shape already takes one branch by label, and the ruling's item 1 and its
conversion address only the edge-branched shape. The options are
refusing the combination (at `os validate` / registration) or giving it
a meaning. Refusing is the tighter contract.
- **Where the closed pair binds today**: decision config is export-only
(nothing parses it at run time), so the pair is enforced by `tsc`, the
published JSON Schema and a direct parse. Those are the same doors
`conditions` has. A stored flow carrying `mode: 'bogus'` is not refused
at run time by this PR. That parse belongs with the engine change that
reads the key.
- **Main moved** by 2 commits since the branch point (`9401b842`,
`49144fcc`, in `packages/rest` and `packages/core` only). There is no
overlap with this diff, and the merge queue rebuilds on the current
main.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Rjy9MeetSfq34PKn81CRiN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
`DecisionConfigSchema` declares an optional `mode: 'exclusive' | 'inclusive'` — the contract half of the #15429 ruling. The author of a `decision` node that routes on its out-edges can now declare whether it takes only the first out-edge whose condition holds or every one of them — with taking every one as the value that must be written down, the way BPMN separates the exclusive gateway from the inclusive one and n8n's Switch keeps "send to all matching outputs" behind an off-by-default toggle (#19867).
6
+
7
+
Clause-②: yes (widening) — one new OPTIONAL key on a published, strict node-config schema, so the set of accepted configs grows. Nothing previously accepted is refused, no key is renamed or retired, and the parsed output of an existing config is unchanged (the key has no `.default()`).
8
+
9
+
-**`'exclusive'`** — only the first out-edge whose condition holds, in the order the edges are declared; this is what an omitted `mode` means. **`'inclusive'`** — every out-edge whose condition holds. Any other value is refused at `mode` with a prescription naming both members' meanings.
10
+
-**⚠️ Declared ahead of its enforcement, on purpose.** The ruling's split order lands this key first, then the engine semantics together with the `os migrate meta` conversion in one change, then the docs. Until that second step ships, nothing reads `mode`: an edge-branched decision still takes EVERY out-edge whose condition holds, whatever `mode` says. The key's own description says so, and the conversion that writes `mode: 'inclusive'` onto every decision relying on today's behaviour ships in the same change as the new traversal, so no flow changes behaviour silently.
11
+
-**A `conditions` list is unaffected**: it is ordered first-match on its own, and `mode` speaks about the out-edges.
12
+
-**Where it binds today**: `decision` config stays export-only (nothing parses it at run time), so the closed pair is enforced by `tsc`, by the published JSON Schema and by a direct parse — the same doors `conditions` has.
|**mode**|`Enum<'exclusive' \| 'inclusive'>`| optional | Declares how many out-edges an edge-branched decision takes when more than one out-edge condition holds: 'exclusive' = only the first, in the order the edges are declared (what an omitted mode means); 'inclusive' = every one that holds. Declared ahead of the engine change that reads it: until that lands, an edge-branched decision takes every out-edge whose condition holds, whatever this says. A conditions list is first-match on its own. |
0 commit comments