Repository navigation
Commit 866683f
feat(spec)!: the build doors judge an approval node config against its declared contract, whole — an undeclared key or a refused value is refused with a location (#21893)
Fixes #21850
Clause-②: yes (narrowing)
The build doors now judge an `approval` node's `config` against the
contract the spec declares for it, `ApprovalNodeConfigSchema`, whole.
`escalation.bogusKey` and `escalation.timeoutHours: 0.5` are each
refused with a location at `FlowSchema.parse`, `objectstack validate`
and `objectstack compile`. Before this, both exited 0. The existing
alias text ("did you mean `timeout` → `timeoutHours`") stays in the
refusal. No plugin is loaded at build time, and no node type joins the
map unless the spec declares its contract.
**Draft.** Patch round 1 adds the one `domain:services` fixture line
that the claim revision now covers (see "The one `domain:services`
fixture" below).
## Two route changes, both measured before the edit
The dispatch route was to put `approval` into
`getBuiltinNodeConfigContracts` beside the 13 builtins. I tried exactly
that on the pristine base (`5e0b489bca`), rebuilt spec (the dist
preflight found the marker in 20 built files) and measured two problems:
1. **The builtin map is reconciled 1:1 against the builtin executors.**
`service-automation`'s `node-config-contract-ledger.test.ts` reads every
`parseNodeConfig` call in its own `builtin/` sources. With `approval` in
the map, 2 of its 5 tests went red: "the map names exactly the node
types an executor parses a config contract for" and "every builtin node
type is classified". That ledger is `domain:services` code, so this PR
cannot change it.
2. **The judge only checks for missing keys.** `flowNodeConfigRefusals`
keeps an issue only when the key it names is absent. The shipped
`flow-node-config-required-keys-refused` entry says the same thing. With
`approval` in the map, `FlowSchema` still accepted both card pins
(`success: true`). Only an approval node with no `approvers` was
refused.
What this PR does instead:
- **A declared contract map beside the builtin one.**
`getDeclaredPluginNodeConfigContracts()` is private to the module, holds
`[APPROVAL_NODE_TYPE, ApprovalNodeConfigSchema]`, and is built on first
use, never at module load. The same judge reads it. `approval.zod.ts`
imports nothing from `automation/` (zod, the membership-role leaf,
`lazySchema`, `strictObject`), so no import cycle is added.
- **That map is judged whole.** The approval executor
(`plugin-approvals`, `approval-node.ts`) runs `safeParse` on
`node.config` before it does anything else and fails the node on any
issue, so every issue the contract raises is refused:
- An undeclared key, or a refused value, gets the new closed-set code
`node-config-refused-by-contract` with `params: { nodeType, key }`. It
is anchored at the key: `escalation.bogusKey`, or one refusal per key
for top-level keys.
- The message wraps the contract's own sentence, including its
did-you-mean.
- A missing required key keeps `node-config-key-missing` or
`node-config-key-required-by-rule`.
- **The builtin arm does not change.** It still only checks for missing
keys. A control test pins it: an `http` node with an undeclared key
still parses.
- **`getBuiltinNodeConfigContracts` keeps its export, shape and
contents** (the 13 builtins). A caller who looks up `approval` gets
`undefined`. The approval contract is reachable through the exported
judge, `flowNodeConfigRefusals('approval', config)`.
## Census, before any edit (at `5e0b489bca`)
I wrote a census script that walks the TypeScript AST and checks every
literal approval node `config` with
`ApprovalNodeConfigSchema.safeParse`. Non-literal configs were read by
hand. Lit controls: the same search pattern finds `decision` nodes in
`app-crm` and `app-todo`, which author flows but no approval nodes, and
it finds the known showcase hit `dynamic-approval.flow.ts`.
- `examples/**`: 15 approval nodes, all in the showcase, all accepted.
- `content/docs/**`: 6 snippets, accepted (3 by the script, 3 by
reading).
- `skills/**`: 5 snippets, accepted.
- `packages/qa/dogfood` fixtures: 6 nodes, accepted.
- objectui at the pin `0abd4f9f87`, read-only: the designer's approval
seed `defaultNodeExtras('approval')` (`{ approvers: [{ type: 'manager'
}], behavior, lockRecord }`) is accepted. objectui's
`flow-canvas-seeds.spec-parse.test.tsx` parses seeds with
`FlowNodeSchema`, which never calls this judge. `flow-required-keys.ts`
asks the judge only whether some refusal names the probed path, and
nothing here changes that answer for a missing key.
- hotcrm: **NOT MEASURED**, because the session's permission check
denied the read-only clone.
No real writer is refused. In test code, 5 fixtures needed changes. They
are listed under "Fixtures" and under "The one `domain:services`
fixture".
## Reproduction, before and after (a scratch copy of the showcase)
I added an `escalation` block to the `co_sign` approval node in
`dynamic-approval.flow.ts`. The script proved each edit landed on disk
and restored the file byte for byte afterwards.
| variant | `5e0b489bca` (main) | this branch |
|:--|:--|:--|
| control `{ timeoutHours: 2, action: 'notify' }` | validate 0 · compile
0 | validate 0 · compile 0 |
| `{ timeoutHours: 2, action: 'notify', bogusKey: 1 }` | validate 0 (`✓
Validation passed`) · compile 0 (`✓ Build complete`), `bogusKey` written
to `dist/objectstack.json` | validate 1 · compile 2, `custom` at
`nodes.2.config.escalation.bogusKey` |
| `{ timeoutHours: 0.5, action: 'notify' }` | validate 0 · compile 0,
`0.5` written to the artifact | validate 1 · compile 2, `custom` at
`nodes.2.config.escalation.timeoutHours` |
Branch wording at the validate door: "This `approval` node's config is
refused at `escalation.bogusKey` by the approval contract: Unrecognized
key(s) on this approval escalation: `bogusKey`. …". For the `timeout`
alias, the contract's "Did you mean `timeout` → `timeoutHours`?" is
carried through, next to the `escalation.timeoutHours` key-missing
refusal.
## Doors pinned (`flow-approval-node-config-contract.test.ts`)
Each door has a valid approval node as its control:
- `FlowSchema` refuses both pins, and the alias, top-level undeclared
keys, a missing `approvers` and a rule finding (`onEmptyApprovers:
'fail'` together with `fallbackApprovers`).
- A sweep checks that the judge refuses exactly what the contract
refuses.
- `defineStack` refuses with `STACK_SCHEMA_INVALID` / 422 at
`flows.1.nodes.1.config.escalation.bogusKey`.
- `ObjectStackDefinitionSchema` refuses. This is the stack parse that
validate and compile run.
- The registered `flow` type schema used by the metadata save door
refuses.
- The artifact parse refuses.
- The `validateStackExpressions` door shows up in `@objectstack/lint`'s
run as `flow 'leave_approval' · node 'approve' (approval)
config.emptyApproverPolicy`.
**Ablation.** I committed the fix first, then deleted the
`[APPROVAL_NODE_TYPE, …]` entry with `scripts/ablation-replace.mjs`:
anchor count 1 → 0, blob `f915eb58bcd0` → `90e86f48dcc4`. The subject
resolves through `src` by relative import, so no build was involved.
- Prediction: 14 red, made up of 12 refusal tests in the new file plus 2
in `flow-slot-refusal-codes.test.ts` (the new code's pin, and "every
code is reached").
- Result: `Tests 14 failed | 26 passed (40)`, matching the prediction.
- Restore: blob equal to HEAD and `git diff HEAD` empty.
## The ADR-0087 kit
- **D3 entry.**
`entries/semantic/18.flow-approval-node-config-contract-refused.ts`,
with its `registry.ts` region regenerated by `gen:migration-registry`.
- **Step-18 rationale.** A fragment at **order 84**. I re-read
`origin/main` at `e085a8c3be` before opening this PR, and again at
`67c544ccca` in patch round 1: the highest order there is 83, and this
id is absent. If #21829 or #21848 also takes 84, the two fragments
render in id order, as the registry header allows.
- **No tombstone and no D2 conversion.** No key is removed, and a
refused node holds no intent that a rewrite could keep.
- **Changeset.** One BREAKING `minor` changeset for `@objectstack/spec`
with the `registered` marker and the `Clause-②` line, at the level the
precedents set (#20416, #21687). `check-adr-0087-registration`:
`[BREAKING+clause-②-narrowing] registered
flow-approval-node-config-contract-refused`.
- **Regeneration.** `check:generated` passes all 15 generated artifacts.
None changed apart from the registry region, because the public exports
did not change.
## Fixtures
These fixtures fed the narrowed rule. Each one was re-judged:
- `spec/.../flow-region-pause-and-end.test.ts`: the
`pausingNode('approval')` fixture had no config and is now refused for
missing `approvers`. Fix: it now declares the one key the contract
requires, `approvers`.
- `lint/src/runtime-gate.test.ts`,
`metadata-protocol/src/protocol.runtime-authoring-gate.test.ts`,
`objectql/src/plugin.authoring-channel.test.ts`: the "clean" approval
flow in all three is a copy of one worked example, and it carried
`emptyApproverPolicy: 'reject'`. That key was never declared by the
contract, and the executor would have refused the node on every run.
Fix: deleted the key. The flow is then the broken flow with its
expression fixed, which is what those tests mean by "clean". These files
are outside the original claim; claim revision round 1 covers them.
- `spec/.../flow-slot-refusal-codes.test.ts`: added a pin per new code
and approval rows in the sweep. Its message pins follow the file's
existing per-code convention.
## The one `domain:services` fixture (patch round 1)
`packages/services/service-automation/src/engine.test.ts`, test "says
nothing about a type a plugin registered AFTER the flow": its
`baseFlow('approval')` helper registered an approval node with no
`config`. `registerFlow` runs `FlowSchema.parse` first, so it now
refused that node with `nodes.1.config.approvers`. The test is about
sealing the node-type vocabulary, not about config. The claim revision
covers this file. The only change is one line in the helper: an
`approval` node now gets `config: { approvers: [{ type: 'user', value:
'u1' }] }`. That is the same disposition as `pausingNode` above. No
`service-automation` source changed. `service-automation` now passes
2110/2110.
## Relation to #21848 (#21848 remains open)
`AutomationEngine.registerFlow` runs `FlowSchema.parse` before anything
else (`engine.ts:4348`), so this PR also makes registration refuse
approval values like `timeoutHours: 0.5`, on every door that registers
through it. That overlaps #21848's done-when and does not contradict it.
Its seat should re-read what is left of its scope: package load paths
that do not go through `FlowSchema`, and its own registration pins. The
stable reuse point is the exported `flowNodeConfigRefusals`, not a
second lookup. `getBuiltinNodeConfigContracts().get('approval')` returns
`undefined`.
## Verification (final union at `76118d27fe`)
**Tests**
- `@objectstack/spec`: test 617 files / 18447 passed, exit 0; typecheck
exit 0. Measured at `3fc48ab07f`; patch round 1 changed no spec file.
- `@objectstack/lint`: before the fixture fix, the full suite had 2
failures, both in `runtime-gate`. After the fix, that file passes 46/46.
- `@objectstack/metadata-protocol`: before the fixture fix, 3 of the 4
files with approval fixtures passed. After it, the fourth passes too:
`protocol.runtime-authoring-gate.test.ts` 70/70.
- `@objectstack/objectql`: the full suite at `76118d27fe` passes, 374
files / 7464 tests, exit 0.
- `@objectstack/http-conformance`: the full suite at `76118d27fe`
passes, 8 files / 102 tests, exit 0.
- `@objectstack/plugin-approvals`: 895/895.
- `@objectstack/service-automation`: the full suite at `76118d27fe`
passes, 172 files / 2110 tests, exit 0. Typecheck exit 0.
- `@objectstack/cli`: `test/authoring-rule-command-parity.test.ts`
11/11, in its integration tier.
- Typecheck for lint, objectql and metadata-protocol: exit 0 each.
**Gates**
- `dispatch-gates --ran` at `76118d27fe`: 96 derived, 96 run, 0
NOT-MEASURED, 0 unrun. The patch round adds `check-tenant-audit-census`
and its `--self-test`, and both pass.
- `check:dual-build-cjs-loads`: exit 0 after a full build supplied the 8
missing `dist/` folders. 106 require entry points across 66 packages
load.
- `check-engine-split-ratio --days 90`: exit 2, refused on a shallow
clone (oldest visible commit 2026-09-20). It is recorded as run, and its
metric is not measured here.
- `check:type-check-debt`: exit 0, "none above its recorded number".
**ESLint, narrowed to the changed files and shown to cover them**
1. Population: ESLint's own `isPathIgnored` reports all 12 changed `.ts`
files as linted.
2. Count: `--format json` reports 12 files, 0 errors, 0 warnings, at
`76118d27fe`.
3. Untouched files: `eslint.config.mjs` enables no type-aware linting
(no `parserOptions.project`), so this diff cannot change the result for
any file it does not touch.
**Declared to CI:** the full `pnpm lint`, the dogfood suite (the census
found all 6 dogfood approval fixtures accepted), and the rest of the cli
integration tier.
## Acceptance notes (not filed)
- objectui's flow inspector (`flow-node-config.ts:996`) writes
`config.escalation.enabled`. Its `timeoutHours` field only shows while
`enabled` is `'true'`, so switching SLA escalation off can save
`escalation: { enabled: false }` with no `timeoutHours`. The contract
refuses that, and the executor already refused it on every run; it is
now refused at save, at `escalation.timeoutHours`. This comes from
reading the source; I did not drive the designer. Owner: none.
- When `defineFlow` throws while the CLI loads its config, the CLI
prints the raw ZodError JSON, with a path relative to the flow and no
flow name or file. This is existing behaviour for every `defineFlow`
refusal.
- Nothing in `plugin-approvals` checks the approval executor's
`safeParse` against the declared map, the way the builtin ledger does
for builtins. That check would live in `plugin-approvals`
(`domain:services`). #21848's PR is the natural place for it.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01T9u38rswFp5Rw8DswRUReJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b238856 commit 866683f
13 files changed
Lines changed: 677 additions & 13 deletions
File tree
- .changeset
- packages
- lint/src
- metadata-protocol/src
- objectql/src
- services/service-automation/src
- spec/src
- automation
- migrations
- entries/semantic
Lines changed: 41 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
48 | | - | |
49 | 48 | | |
50 | 49 | | |
51 | 50 | | |
| |||
Lines changed: 0 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
73 | | - | |
74 | 73 | | |
75 | 74 | | |
76 | 75 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
274 | 274 | | |
275 | 275 | | |
276 | 276 | | |
277 | | - | |
278 | 277 | | |
279 | 278 | | |
280 | 279 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2626 | 2626 | | |
2627 | 2627 | | |
2628 | 2628 | | |
2629 | | - | |
| 2629 | + | |
2630 | 2630 | | |
2631 | 2631 | | |
2632 | 2632 | | |
| |||
Lines changed: 265 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
0 commit comments