Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/20168-decision-mode-beside-conditions-refused.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@objectstack/spec": patch
---

`DecisionConfigSchema` refuses `mode` on a `decision` that also declares a non-empty `conditions` list (#20168). `mode` belongs to the edge-branched decision alone: a `conditions` list is first-match on its own, so a `mode` beside one would be accepted and never read, and `mode: 'inclusive'` there would promise every matching branch while the run takes one.

Clause-②: no

This corrects a key that has not been released yet, so it narrows no published accept set. Timing, measured when this landed: the npm registry's `latest` `@objectstack/spec` is `17.4.0`, and its `json-schema/automation/DecisionConfig.json` declares `conditions` only, with `additionalProperties: false`. No `mode` was published. The Version Packages PR (`chore: version packages`) is open and unmerged. `mode` reaches its first release together with this refusal, so no ADR-0087 entry is owed.

- **The refusal**: one issue at `mode`, for either member. It reads "`mode: 'inclusive'` is not valid on a decision that declares a `conditions` list — `mode` belongs to the edge-branched decision alone." and names the two ways out:
- delete `mode` and keep the list;
- or move the branches onto the out-edges (a `condition` on each branch edge, `isDefault: true` on the fallback), delete `conditions`, and keep `mode`.
- **Left alone**: `mode` on an empty `conditions` list, `mode` with `conditions` absent, and a `conditions` list with no `mode` all parse as before. A `mode` outside `'exclusive' | 'inclusive'` still gets its own value refusal first.
- **Where it binds**: the doors that parse `DecisionConfigSchema`. Today that is a direct parse, including the `SCHEMALESS_NODE_CONFIG_SCHEMAS.decision` handle. `decision` config is still export-only, so a flow's registration and `os validate` do not run it yet. The published JSON Schema cannot state the rule, because no arm of the closed refinement projection fits it. `automation/DecisionConfig` therefore joins `dropped-refinements.baseline.json` and carries the site as `x-dropped-refinements`.
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ const result = DecisionConditionSchema.parse(data);
| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **conditions** | `{ label: string; expression: string }[]` | optional | Ordered decision branches (first true expression wins; omit to branch purely on edge conditions) |
| **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. |
| **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. Refused beside a non-empty conditions list, which is first-match on its own: delete mode there, or move the branches onto the out-edges, delete conditions, and keep mode. |

### Nested Shape: `DecisionConfig.conditions[number]`

Expand Down
9 changes: 7 additions & 2 deletions packages/spec/dropped-refinements.baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
"description": "Shrink-only ledger of every PUBLISHED JSON Schema that is STILL WIDER than the Zod type it was generated from, because a rule written as `.refine()` reaches the runtime and not the file (#18670). `z.toJSONSchema()` has no arm for a `custom` check: a plain record, the same record with a `.refine()`, and the same record with an ABORTING `.refine()` all project byte-identically (measured on zod 4.4.3, the version packages/spec resolves). So a document one of these files ACCEPTS can still be refused at parse time, and an author -- or an AI -- validating against packages/spec/json-schema/** finds out a release later. Each `sites` path is a position under that schema at which a refinement is dropped; the same paths are written onto the artifact itself as `x-dropped-refinements`. Item 2 closed the first patterns: a refinement DECLARED through the closed list in src/shared/refinement-projection.ts is emitted into the published file, reads `projected` rather than `dropped`, and its row LEAVES this ledger in the same PR -- which is why the ledger shrinks and never grows on a repair. Every refinement outside that closed list stays here, and adding an arm to the list is a public-contract decision, not a refactor. Hand-edited on purpose and with no `gen:` script: a generator would let a new gap be admitted by running a command instead of by a decision, which is the silence this ledger exists to end. Adding, removing or moving a site fails packages/spec/scripts/build-schemas.ts until the line moves with it, and the failure prints the corrected entry in full. ⛔ Do not delete or weaken a refinement to shorten this file -- the runtime rule is correct; it is the projection that is silent, and the remedy is to teach the closed list a NAMED pattern, never to drop the rule.",
"measured": {
"zod": "4.4.3",
"publishedSchemasWithDroppedRefinements": 211,
"droppedRefinementSites": 604,
"publishedSchemasWithDroppedRefinements": 212,
"droppedRefinementSites": 605,
"refinementSitesThatDidProject": 369,
"refinementSitesWithNoJsonFormToCompare": 0
},
Expand Down Expand Up @@ -413,6 +413,11 @@
"fields.valueType"
]
},
"automation/DecisionConfig": {
"sites": [
""
]
},
"automation/EndConfig": {
"sites": [
""
Expand Down
138 changes: 132 additions & 6 deletions packages/spec/src/automation/schemaless-node-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
* guard. `decision` is the exception — it stays export-only (nothing parses it
* at run time), so its pins below bind the authoring doors only: `tsc`, the
* published JSON Schema and a direct parse. Its `mode` key is declared ahead of
* the engine change that reads it (#15429).
* the engine change that reads it (#15429), and is refused beside a non-empty
* `conditions` list (ruling 5856786357 on #20168) — a refinement, so of those
* doors it binds the direct parse and is declared dropped in the JSON Schema.
*
* The structural assertions at the bottom guard the downstream walkers that a
* union-shaped contract would have broken, which is why #4343 converged the
Expand All @@ -25,6 +27,7 @@ import { objectStackErrorMap } from '../shared/error-map.zod.js';
import {
DecisionConditionSchema,
DecisionConfigSchema,
SCHEMALESS_NODE_CONFIG_SCHEMAS,
ScriptConfigSchema,
SubflowConfigSchema,
getSchemalessNodeConfigJsonSchemas,
Expand Down Expand Up @@ -244,11 +247,8 @@ describe('DecisionConfigSchema.mode (#15429 item 2 — the contract half, declar
expect('mode' in omitted, 'an omitted mode must not come back as a parsed default').toBe(false);
expect(DecisionConfigSchema.parse({ mode: 'exclusive' })).toEqual({ mode: 'exclusive' });
expect(DecisionConfigSchema.parse({ mode: 'inclusive' })).toEqual({ mode: 'inclusive' });
// …and alongside a branch list, which the key does not forbid.
expect(DecisionConfigSchema.parse({
mode: 'exclusive',
conditions: [{ label: 'big', expression: 'amount > 100000' }],
})).toEqual({ mode: 'exclusive', conditions: [{ label: 'big', expression: 'amount > 100000' }] });
// Beside a NON-EMPTY branch list the key is refused (ruling 5856786357 on
// #20168) — pinned in the describe block below, with its controls.
});

it('types the key as the closed pair at the tsc door', () => {
Expand Down Expand Up @@ -303,6 +303,132 @@ describe('DecisionConfigSchema.mode (#15429 item 2 — the contract half, declar
});
});

/**
* Ruling 5856786357 on #20168 (letter A): `mode` belongs to the edge-branched
* decision alone, so a decision declaring a NON-EMPTY `conditions` list AND
* `mode` is refused at `mode`, with the ruled prescription — delete `mode`
* (the list is first-match on its own), or move the branches onto the
* out-edges, delete `conditions`, and keep `mode`.
*
* Key-vs-value note: the rule judges the KEY beside a non-empty list, whatever
* member it names, so every refusal below is a full `safeParse` failure
* located at `mode`, and every control a full `safeParse` success that
* round-trips — never mere absence of one issue code.
*/
describe('DecisionConfigSchema — `mode` beside a non-empty `conditions` list is refused', () => {
const BRANCH = { label: 'big', expression: 'amount > 100000' } as const;
const BRANCHES = [BRANCH, { label: 'small', expression: 'amount <= 100000' }] as const;

/**
* The ruled prescription on the message: the wording is the contract here
* (the ruling names both ways out and #15429's acceptance list carries it),
* so the first sentence is read verbatim and each remedy is required.
*/
function expectRuledRefusal(message: string, mode: 'exclusive' | 'inclusive'): void {
expect(message.startsWith(
`\`mode: '${mode}'\` is not valid on a decision that declares a \`conditions\` list — `
+ '`mode` belongs to the edge-branched decision alone.',
)).toBe(true);
expect(message).toContain('A `conditions` list is first-match on its own');
expect(message, 'remedy 1: drop mode').toContain('Either delete `mode` and keep the list');
expect(message, 'remedy 2: branches onto the edges, keep mode')
.toContain('move the branches onto the out-edges');
expect(message).toContain('delete `conditions`, and keep `mode`');
expect(message, 'a prescription an author is shown carries no tracker number').not.toMatch(/#\d{3,5}/);
}

type Issue = { code: string; path: PropertyKey[]; message: string };
const issuesOf = (result: { success: boolean; error?: { issues: ReadonlyArray<Issue> } }): ReadonlyArray<Issue> =>
(result.success ? [] : result.error!.issues);

it.each(['inclusive', 'exclusive'] as const)(
'refuses `mode: %j` beside a one-entry list — one custom issue at [mode], with the ruled prescription',
(mode) => {
const result = DecisionConfigSchema.safeParse({ conditions: [BRANCH], mode });
expect(result.success).toBe(false);
const issues = issuesOf(result);
expect(issues).toHaveLength(1);
expect(issues[0]!.code).toBe('custom');
expect(issues[0]!.path).toEqual(['mode']);
expectRuledRefusal(issues[0]!.message, mode);
},
);

it.each(['inclusive', 'exclusive'] as const)('refuses `mode: %j` beside a multi-entry list alike', (mode) => {
const issues = issuesOf(DecisionConfigSchema.safeParse({ mode, conditions: BRANCHES }));
expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['mode']]]);
expectRuledRefusal(issues[0]!.message, mode);
});

it('is the same refusal through `SCHEMALESS_NODE_CONFIG_SCHEMAS.decision` — the handle a registration-time reader looks up by node type', () => {
// #15429's registration reader and metadata-protocol's reference walk both
// reach this contract by node type rather than by its export name, so the
// pin is taken through that door too.
const issues = issuesOf(SCHEMALESS_NODE_CONFIG_SCHEMAS.decision.safeParse({ conditions: [BRANCH], mode: 'inclusive' }));
expect(issues.map((i) => [i.code, i.path])).toEqual([['custom', ['mode']]]);
expectRuledRefusal(issues[0]!.message, 'inclusive');
});

it('keeps its prescription under the ObjectStack error map a validator may pass per parse', () => {
const result = DecisionConfigSchema.safeParse({ conditions: [BRANCH], mode: 'inclusive' }, { error: objectStackErrorMap });
const issues = issuesOf(result);
expect(issues).toHaveLength(1);
expect(issues[0]!.path).toEqual(['mode']);
expectRuledRefusal(issues[0]!.message, 'inclusive');
});

it('a mode OUTSIDE the closed pair beside a list gets the value refusal first — one issue, never both', () => {
// The enum refusal is a base-type issue, so the object's refinement does
// not run over it: the author fixes the value, then meets this rule.
const issues = issuesOf(DecisionConfigSchema.safeParse({ conditions: [BRANCH], mode: 'all' }));
expect(issues).toHaveLength(1);
expect(issues[0]!.code).toBe('invalid_value');
expect(issues[0]!.path).toEqual(['mode']);
expect(issues[0]!.message).toContain("`mode: 'all'` is not a decision mode");
});

describe('CONTROLS — what the refusal must leave alone', () => {
it.each(['inclusive', 'exclusive'] as const)('accepts `mode: %j` beside an EMPTY list, and it round-trips', (mode) => {
const result = DecisionConfigSchema.safeParse({ conditions: [], mode });
expect(result.success).toBe(true);
expect(result.data).toEqual({ conditions: [], mode });
expect(DecisionConfigSchema.parse(result.data)).toEqual(result.data);
});

it.each(['inclusive', 'exclusive'] as const)('accepts `mode: %j` with `conditions` absent, and it round-trips', (mode) => {
const result = DecisionConfigSchema.safeParse({ mode });
expect(result.success).toBe(true);
expect(result.data).toEqual({ mode });
expect('conditions' in result.data!).toBe(false);
expect(DecisionConfigSchema.parse(result.data)).toEqual(result.data);
});

it('accepts a non-empty list with NO mode, and it round-trips', () => {
for (const conditions of [[BRANCH], BRANCHES]) {
const result = DecisionConfigSchema.safeParse({ conditions });
expect(result.success, JSON.stringify(conditions)).toBe(true);
expect(result.data).toEqual({ conditions });
expect('mode' in result.data!).toBe(false);
expect(DecisionConfigSchema.parse(result.data)).toEqual(result.data);
}
});

it('following either remedy parses', () => {
const refused: Record<string, unknown> = { conditions: [...BRANCHES], mode: 'inclusive' };
expect(DecisionConfigSchema.safeParse(refused).success).toBe(false);
// Remedy 1 — delete `mode`, keep the list.
const listOnly = { ...refused };
delete listOnly.mode;
expect(DecisionConfigSchema.parse(listOnly)).toEqual({ conditions: BRANCHES });
// Remedy 2 — the branches move onto the out-edges (outside this config),
// `conditions` is deleted, and `mode` stays.
const modeOnly = { ...refused };
delete modeOnly.conditions;
expect(DecisionConfigSchema.parse(modeOnly)).toEqual({ mode: 'inclusive' });
});
});
});

describe('structural contract — what the downstream walkers require', () => {
it('keeps the tombstoned keys IN the shape, so the ratchet can see them retired', () => {
// A `retiredKey()` is still a property. Deleting it outright would read as
Expand Down
Loading
Loading