Skip to content

Commit 7843663

Browse files
os-litantclaude
andauthored
feat(spec)!: a structured region body refuses a pause-capable node and an 'end' node (#18688)
Part of #15646 Clause-②: yes The flow accept set shrinks for five node types inside region bodies — shapes the runtime never honoured. Ruling D clause 4 states it verbatim. ⚠️ **The runtime half is NOT in this PR.** A region-contained node that *durably suspends* must fail the run with a named error — only the run can know that — and ruling D assigns it to a separate `domain:services` card. ⇒ this PR lands with `Part of`, ⛔ not `Fixes`; **#15646 stays open until the runtime half lands.** --- ## ⚠️ SEAT BANNER — ruling **D** superseded the route this body argues for Everything below the horizontal rule was written when the card was ruled **C**, and it argues for **route A** ("Recommendation: A, as implemented"). ⛔ **That is no longer what this PR does.** It is kept unedited as the record of how the decision was reached — ⛔ deleting it would erase the evidence the later ruling was made on. **What this PR does NOW**, per ruling D (batch #153 item 1, comment `5724940095`, maintainer 「其他同意」): - Inside `loop` / `parallel` branch / `try_catch` (try **and** catch) bodies at any depth, `FlowSchema.superRefine` refuses **five** node types: `screen`, `wait`, `approval`, `approval_revise`, and `end`. - ⛔ **`map` and `subflow` are NOT refused by type.** They pause exactly when the child flow their `config.flowName` names pauses — a *different metadata record*, not in hand at parse. Refusing them by type would also refuse `loop { map(synchronous child) }`, which runs correctly today. - `packages/spec` keeps its published identifier `FLOW_PAUSE_CAPABLE_NODE_TYPES`; only its **contents** narrow. - **`packages/services` is untouched by this round** — measured, zero paths. The 5-tests-in-3-files cost the section below describes **does not occur**: the whole package runs **138 files / 1652 tests, all passing**, with a false-green control proving the test read the rebuilt artifact and not a stale `dist`. ⚠️ **Corrected by the seat, and it is a DECLARED DEVIATION from ruling D's letter — ⛔ not a clean pass.** 「zero paths」 is true of **this round's commits** and ⛔ NOT of the PR's cumulative diff: `packages/services/service-automation/src/end-node-refused-outcome.test.ts` (+57/−35) is in the diff, from the earlier round's commit `87973cab8d1`. Ruling D clause 1 says 「`packages/services` untouched; **the 5 tests** and #15616's suite stand」 — and #15788's region-`end` case **was one of those 5**. ⇒ the ruling's premise 「B breaks nothing」 was **false for that one case**: clause 1 itself orders `end` refused at parse, and `registerFlow` parses, so the old run-time assertion is unreachable by construction. What was done: the fixture is **byte-identical**, case count **12 → 12**, and the assertion is **strengthened** (region path, message text, and that nothing registered) so it fails again the day the shape becomes declarable. ⛔ No test deleted, skipped or quarantined; ⛔ no engine source moved; #15616's suite and the other three files are untouched and green. The at-tier review measured all of this and ruled it non-blocking — but it is a deviation and it is stated here rather than buried. **CI on `6de9d662f6df`: 32 success, 3 skipped, 0 failure, 0 pending.** ### ⛔ Two corrections the `domain:spec` seat owes on its own record 1. ⭐ **The seat ruled "keep the published name" on a premise that is FALSE.** It told the round that renaming `FLOW_PAUSE_CAPABLE_NODE_TYPES` removes a *published* export and therefore forces a major. Measured since: main's `packages/spec/api-surface/automation.json` greps **0** for that name (lit controls `FLOW_BUILTIN_NODE_TYPES` and `FLOW_STRUCTURAL_NODE_TYPES` = 1 each; dark control = 0), and the branch greps 1. ⇒ **the constant is introduced by this PR and is on no consumer's import path**; the gate's 「1 breaking (removed)」 was computed against the branch's own earlier snapshot. The *decision* stands and costs nothing — a second name would be cost without benefit — but ⛔ the record must not carry 「a removed published export」 as a fact about consumers. The round measured this and told the seat; the seat re-measured and confirms it. 2. ⛔ **THIS CORRECTION WAS ITSELF WRONG, and the seat withdraws it.** It claimed `--pair 18688` re-measured 「exit 0」 at this head. **That reading came from a STALE INSTRUMENT.** The shared checkout's `check-clause2-carriers.mjs` is blob `ccd5ad7c9a00` and contains **0** occurrences of rule **C8**; `origin/main`'s and this head's is blob `3a270ef2eb5f` and contains **18** (lit control `C1`: 50 vs 51, so the reader works). C8 landed on `main` at 01:41Z via #18859 and the shared checkout never had it. ⇒ every `--pair` reading this seat took today was taken with a script that cannot see C8. Re-taken with `origin/main`'s script: **#18688 exit 4 on C8** — this seat held **two live `Claim:` comments** on #15646 (`5722016855`, `5728277407`), which the protocol forbids. Repaired as C8 prescribes: `Release:` (`5729634742`) then ONE fresh `Claim:` (`5729639847`). **`--pair 18688` now exits 0** — `claim.selected` 1, `claim.rejected` 2. The at-tier review caught this; the seat re-measured and confirms it. --- Route **C**, as ruled. Director seat, summon #24, batch #145 item 5 — #15646 (comment) (maintainer 「同意,其他也同意」), with the batch #146 scope addition — #15646 (comment) (maintainer 「146 同意」), which attached #3267's 禁 ruling and absorbed #18112 into this card. One PR, one changeset, two refusals in one rule family. ## 🛑 Read this first — this PR is NOT ready to land, and the reason is a measured decision, not a bug `packages/spec` is green end to end. **5 tests in 3 `packages/services/service-automation` files now fail**, and every one of them fails for the same reason: the fixture can no longer be REGISTERED, because `AutomationEngine.registerFlow` parses through `FlowSchema.parse` (`engine.ts:3941`) and this rule refuses the shape. ⚠️ **Corrected by the `domain:spec` seat after a classification round — the table below replaces one that named 5 tests in 3 files.** That earlier count was taken by running **three named files**; CI runs `pnpm --filter @objectstack/service-automation test`, the whole package, and a named-file subset cannot see this class of breakage. `os-dev.md:56` reserves this body to the PR-open write, so the round named the wording and the seat writes it. | File | Failing | Card | What it pins | | --- | --- | --- | --- | | `src/builtin/contained-failure-rollup.test.ts` | **7** | #16314 | the contained-failure rollup fold over `loop { subflow }` — ⚠️ **absent from the earlier table entirely**; it predates this branch's base (`git merge-base --is-ancestor` exit 0), so this is a measurement gap, ⛔ not drift | | `src/builtin/map-in-loop-iteration-state.test.ts` | 3 | #15616 | `loop { body: [ map, probe ] }` over a **non-pausing** child: 5 iterations x 2 items ⇒ 10 child runs, `failed = 0` either way, a fresh result set per iteration | | `src/builtin/contained-failure-visibility.test.ts` | 1 | #14456 | a parent run's row identity does not leak into a `subflow` child; the region shape is the **vehicle**, not the subject | | `src/end-node-refused-outcome.test.ts` | 0 (was 1) | #15788 | ⭐ **fixed on this branch** — see below | **Measured with the package suite:** at `e10b395cee`, **12 failed / 1627 passed (1639)** across **4 files**. After the fix below, at `87973cab8d1`: **11 failed / 1628 passed**. ⭐ **One of the twelve was never blocked on the open question, and it is repaired here.** #15788's region-`end` case sits in **both** candidate populations — this body defines route B as the unconditionally pausing types **plus `end`** — so no answer to the question below moves it. It is re-homed to the registration refusal: the fixture is unchanged byte for byte, and the case now asserts the ZodError's located path, its message and prescription, and that **nothing registered**. ⛔ Not a deletion — it fails again the day the shape becomes declarable. **The 11 are mutually exclusive with route A, and that is measured rather than argued.** Ablating `FLOW_PAUSE_CAPABLE_NODE_TYPES` to route B's definition turns **all 11 green with nothing else moving**; route A on the same four files is 11 red. ⚠️ Method note that is load-bearing: `service-automation` resolves `@objectstack/spec` through **`dist`**, so the ablation was rebuilt and verified present in 18 built artifacts before anything was read — an unrebuilt ablation would have gone green and proved nothing. Restored afterwards, verified absent from all 216 artifacts, whole-tree porcelain empty. ⭐ **The 11 are NOT one cost.** 3 of them (#15616) are free: under route A the shape becomes undeclarable, so the defect is unreachable and the regression suite converts to a refusal pin — mechanically, the same conversion performed above for #15788; that file's second describe (a TOP-LEVEL pausing map) is untouched and green, so the durable-pause half keeps its coverage. The other 8 (#16314, #14456) are a genuine re-home onto a top-level delegating node, and `loop { subflow }` over five rows with one failing is the shape #15617's ruling **named**, so any re-home must record that the measurement no longer runs on it. ⛔ **Not repaired here.** The dispatch fences `packages/services` ("the engine's runtime refusal stays exactly as it is") — ⚠️ and note precisely what that fence claims: it is true of the **diff**, which touches no `packages/services` file. Read as a claim about **effect** it is false, because the parse refusal changes what those suites can register. The changeset carries the same correction, and two of these three are other cards' regression suites: deleting or re-homing #15616's and #15788's coverage is a decision, not a fixture edit. **Two of them are also evidence about the rule itself**, which is the open question below. ### The open question: does the narrowing take a shape that WORKS with it? The ruling's population is "a node that **can durably pause** (`map` / `subflow` **with a pausing child**, approval-class nodes)". Measured: `map` and `subflow` pause **exactly when the child flow they NAME pauses** — a different metadata record — so "with a pausing child" is **not decidable at parse**. Only two spellings are: - **A — judge the node TYPE** (what this PR implements). Closes this card's own reproduction, covers all three region kinds, and is the only reading under which C is the *complete* fix the ruling's own reasoning requires. **Cost, measured:** it also refuses `loop { map(synchronous child) }` — a shape that runs correctly today and was deliberately fixed 12 days ago by #15616 / PR #15648, whose regression suite is 3 of the 5 failures above. - **B — judge only the UNCONDITIONALLY pausing types** (`screen` / `wait` / `approval` / `approval_revise`) plus `end`. Refuses nothing that works today, and the 3 `map` failures disappear. **Cost:** this card's own reproduction — `loop { try_catch { map(pausing child) } }` — stays declarable and stays silently green, so the card is not closed. There is no third reading available to a parse. **Recommendation: A, as implemented** — #3267 is ruled 禁 ("structured regions do not support durable pause"), and a shape whose legality lives in a record the author is not editing, revocable by editing that record, is not a contract. Under A the five tests are re-homed (a top-level `map`, a top-level `end`) or retired with a statement, in this PR or a follow-up, once the seat says the coverage may move. ## Step Zero — the ruling's precondition, answered before any code was written > **First step, before writing**: prove the nesting is statically decidable at parse/validate time. **Answer: YES for the nesting and for the node vocabulary this rule judges, with two boundaries that are declared rather than discovered.** What was measured, on this branch's base `7f7b8557df`: 1. **The nesting is decidable, and a refusing layer already exists.** `collectFlowGraphs` (`packages/spec/src/automation/control-flow.zod.ts`) yields the top-level graph plus every region body, depth first, with a `scope` label and a `path` that anchors a Zod issue where the author wrote the node. `FlowSchema`'s `superRefine` already walks exactly that and refuses on it — the #16134 one-node-id-space rule. The PM seat's clue held: there is no *refusing* layer for this shape, but the walk and the refusal machinery are both live and in the same file. 2. **The pausing vocabulary is statically declared for the built-in set.** Derived by reading the shipped `defineActionDescriptor` literals, not by recall: `supportsPause: true` appears on `screen` / `wait` / `subflow` / `map` (`packages/services/service-automation/src/builtin/`) and `approval` / `approval_revise` (`packages/plugins/plugin-approvals/src/`) — six, the same six the ADR-0044 `resumeAuthority` default-flip migration entry names in its own prose. They are published here as `FLOW_PAUSE_CAPABLE_NODE_TYPES`. 3. **`end` is fully static** — `FLOW_STRUCTURAL_NODE_TYPES`, a node type the engine handles with no executor at all. **What is NOT decidable, and what this rule does about it.** Whether a given node *will* pause is not decidable at parse, in two different ways, and both are stated in the docblock, in the changeset and in the ADR-0087 entry: - **`map` / `subflow` pause exactly when the child flow they NAME pauses** (`map.config.flowName`, an opaque reference to another metadata record). So the rule judges the node **TYPE**, not the run. That is wider than the runs that actually broke — a region-nested `map` over a synchronous child parsed green before and is refused now — and it is deliberate: the old shape's legality lived in a record the author is not editing and could be revoked by editing that record. "Legal until somebody adds a `wait` to the child flow" is not a contract. - **A plugin-registered pausing type is invisible to a parse.** ADR-0018 left the node-type namespace open (`FlowNodeSchema.type` is a validated `string`), and a parse has no registry. Pinned as a boundary test so it moves deliberately. - **`MAX_REGION_DEPTH` (32).** The walk stops there. ⚠️ Unlike #16134's duplicate-id rule, there is **no second spec refusal behind the ceiling** for this rule — `analyzeRegion` says nothing about pausing nodes — so past depth 32 the engine's run-time refusal is the only one. Measured and pinned at nesting 32 (refused) / 33 (not judged), and stated in the changeset rather than left for an author to find. ## What changed `FlowSchema.superRefine` gains one walk over `collectFlowGraphs`, skipping the flow's own graph, that raises a `custom` issue anchored at `[...regionPath, 'nodes', i, 'type']` for: - **a pause-capable node** in a region body — the message names the node, the region scope (`loop 'sweep' body → try_catch 'guard' try`), why a region body cannot host it, and the fix; - **an `end` node** in a region body, whatever its `outcome` — an `end` there was a no-op, and a refusing one was converted into a region error at the same boundary (#15788). The ruled prescription is the message: a region body cannot end the run; put the `end` on the top-level graph. `FLOW_PAUSE_CAPABLE_NODE_TYPES` is the new export (`api-surface` / `export-origins` regenerated). The two approval entries are the declared constants `APPROVAL_NODE_TYPE` / `APPROVAL_REVISE_NODE_TYPE`, so a rename cannot desynchronise them. ⛔ `packages/services` is untouched — this is authoring-time enforcement only. ⛔ No engine rollback seam (route A, no card filed, per the ruling). ⛔ No runtime detection in `map` (route B, refused). ⛔ #15617's `failed` fold is not addressed. ## Tests New file `packages/spec/src/automation/flow-region-pause-and-end.test.ts` — every case fails without the rule: - both refusals × all three region kinds: `loop` body, `try_catch` try **and** catch, `parallel` branch. A rule covering `loop` only is route B wearing C's clothes; the `try_catch` catch arm and the `parallel` branch arm are the two route B could never see, and each has its own case. - all six pause-capable types, table-driven off the exported constant. - the card's own reproduction, `loop { try_catch { map } }`, refused with the chained region path. - **negative tests, the over-reach guard**: every pause-capable type and an `end` still parse on the **top-level graph**; every non-pausing type still parses inside a region; a node merely *named* `end` or `wait` in a region still parses (the rule judges `type`, not `id`). - both declared boundaries pinned: the plugin-contributed pausing type, and the depth-32/33 seam. - `defineFlow` and `formatZodError` renderings. Two existing cases pinned the behaviour this rule replaces and were **replaced rather than re-spelled**, each saying so in its own comment: `end-node-outcome.test.ts`'s region-nested `end` (its subject — an `end`-in-region whose *config* is judged one door later — no longer exists) and `flow.test.ts`'s BPMN `waitEventConfig` region case (now asserts the earlier refusal *and* keeps the region-contract half it actually exists to measure). The `requireTypeScopedConfig` docblock that asserted a nested block-less `wait` parses green was corrected in the same edit. ## Verification Measured on `e10b395cee`. Heavy runs go through `scripts/pm/os-verify-lock.sh`; every exit code below is read from the wrapper's own `VERDICT command-exit` line or captured into a variable **before** any pipe — never `$?` after one. | Command | Verdict | | --- | --- | | `pnpm --filter @objectstack/spec build` | `command-exit 0` | | `pnpm --filter @objectstack/spec test` (whole package) | `command-exit 0` — **486 files, 13895 tests, 0 failed** | | `pnpm --filter @objectstack/spec typecheck` (`tsc --noEmit` + `check:scripts-typecheck` + `check:test-typecheck`) | `command-exit 0` | | `pnpm --filter @objectstack/spec check:generated` | `command-exit 0` — all 15 artifacts up to date | | `pnpm lint` (whole repo, `eslint . --no-inline-config`) | `exit 0` — run in full, so nothing here is a narrowing | | `dispatch-gates.mjs --ran` reconciliation | `exit 0` — **85 derived, 81 run, 4 NOT-MEASURED, 0 UNRUN** | | `@objectstack/service-automation` — the 3 files whose fixtures feed this rule | `exit 1` — **5 failed / 24 passed**, see the section at the top | **Reverse verification (one-shot, restored).** The rule's own early-exit was mutated (`graph.path.length === 0` → `>= 0`), and the mutation was proved on disk before anything was read from the run — anchor grep 1 → 0, marker grep 0 → 1, blob `044bbbba` → `56a1c350`: - **ablated** — `flow-region-pause-and-end.test.ts`: **20 failed / 7 passed**. The 20 are exactly the refusal assertions; the 7 that survive are the over-reach guards and the two boundary pins, which must stay green with the rule absent. That split is itself the reading: a rule that also broke the negative cases would be refusing too much. - **restored** — `git checkout HEAD --` the file, blob back to `044bbbba`, `git diff HEAD` clean, `git status` empty, same file **27/27 passed**. No `dist` preflight applies: the test imports `./flow.zod` by relative source path, so the subject never resolves through `packages/spec/dist`. A restore `trap` was armed for the whole window. **The four NOT-MEASURED gates** are `check:doc-formula-expressions`, `check:dual-build-cjs-loads`, `check:lean-entry-closure` and `check:type-check-debt` — each exited **3, PREREQUISITE NOT MET**, printing in its own words that nothing was measured. All four read built output across packages this diff does not touch and need a repo-wide build; CI's `Build Core` and `Lint & Repo Gates` are where they run. ⛔ Not green, not red — unrun. **Not measured, stated:** CI convergence on this PR (the report is filed at the end of local verification); the branch has not been merged forward since `7f7b8557df`, so `main`'s newer commits are tested by CI and the queue rather than here. **Review-gate reading, not an action.** `scripts/pm/check-clause2-carriers.mjs --pair 18688` exits **4** on two rows, both belonging to the claiming seat and ⛔ neither touched here: **C1** — card #15646 carries `needs:contract-review` while this PR does not (the gate is a dual carrier); **C2** — no comment on the card's thread is a machine-legible claim comment (none has a first line beginning `Claim:` carrying the `Clause-②:` line), so the declaration limb has nothing to read. The declaration itself is at the top of this body and in the changeset. ## Acceptance notes Observations from this card's reading, recorded here and **not** filed — none is a reproducible defect, a contract violation, or an authoring trap: - The ruling's own parenthetical, "`map` / `subflow` **with a pausing child**", describes the defect population rather than a decidable rule population, and its "a shape the runtime never honoured" is exact for `end`, `screen`, `wait` and the approval pair but not for a `map` over a synchronous child, which runs today. The PR takes the capability reading — the only one that makes C the complete fix the ruling's own reasoning requires — and the changeset states the cost in the author's own terms. Noted so a reviewer reads the widening deliberately rather than discovering it. - The card body and the batch #145 ruling both say #15617 is open; it is **closed / completed**. Nothing here depends on it. - `engine.ts:9937` in the ruling reads `engine.ts:9970` on this tree — line numbers are clues, and this one was re-read rather than trusted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LvwGppdonww4zGLWZo5rho --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 75c0dac commit 7843663

11 files changed

Lines changed: 785 additions & 81 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
**BREAKING for authored metadata** — an ADR-0031 structured region body (`loop.config.body`, a `parallel` branch, `try_catch`'s `try` / `catch`) now refuses two node populations at parse: a node whose TYPE parks the run on every execution, and an `end` node (#15646, absorbing #18112).
6+
7+
Clause-②: yes
8+
9+
The flow accept set shrinks for five node types inside region bodies — shapes the runtime never honoured. Both refusals are the authoring-time enforcement of a limit the engine already holds at run time and #3267 ruled 禁: **a region body runs synchronously inside the enclosing run, so it can neither park that run nor terminate it.**
10+
11+
```
12+
✗ nodes.1.config.body.nodes.0.type: A `approval` node may not sit inside a structured region —
13+
`loop 'sweep' body → try_catch 'guard' try` is a region body and the `approval` node `sign_off`
14+
is inside it. A region body runs synchronously and cannot durably pause …
15+
```
16+
17+
**What is refused**
18+
19+
- **A node that pauses on EVERY execution**`screen`, `wait`, `approval`, `approval_revise`.
20+
- **An `end` node**, whatever its `outcome`. An `end` in a region was a no-op, and a refusing one was converted into a region error at the boundary; neither is what the author wrote.
21+
22+
**⛔ What is deliberately NOT refused: `subflow` and `map`.** Their shipped executors also declare `supportsPause: true`, but they pause exactly when the child flow their `config.flowName` names pauses — a **different metadata record**, not in hand while this flow is parsed. Refusing them by type would also refuse `loop { map(synchronous child) }`, a shape that runs correctly today and is covered by an existing regression suite. A parse-time rule refuses what is statically wrong; a region-contained node that actually suspends is a fact only the run holds. **Nothing an author wrote with a region-nested `map` or `subflow` needs editing for this release.**
23+
24+
**Why it was silent, measured.** The engine converts a suspension raised inside a region into an error — but the executor has already written its progress state into the ENCLOSING scope by then. Contain that error in a `try_catch` and the residue is read back as progress by the next entry to the same node. On a real `AutomationEngine`, `loop { try_catch { map(pausing child) } }` over 3 iterations × 2 items: not one item's subflow completed, only two of three iterations reached the catch, and iteration 3 read `started === collection.length`, ran nothing, and returned `success` with `summary.failed = 0`. ⚠️ Read that for the MECHANISM, not for this change's reach — the shape it was measured on is a `map`, and making that run's refusal loud is a separate change to the automation engine, not this one.
25+
26+
### Migration — FROM → TO
27+
28+
| You wrote | Write instead |
29+
| --- | --- |
30+
| `loop { body: [ …, end ] }` | `loop { body: [ … ] } → end` — give the region a normal exit and put the terminator, with its `outcome` / `message`, on the top-level graph |
31+
| `loop { body: [ wait ] }` | a top-level `wait`, with the top-level graph as the repeating construct — a region body cannot park the run, so the nested form never waited |
32+
| `parallel { branches: [ [ approval ] , … ] }` | put the `approval` on the top-level graph and fan out around it, or split the branch's pausing half into a `subflow` the top-level graph calls |
33+
34+
The one-line fix is always the same: **move the node onto the top-level graph and route the region's exit to it.** ⛔ Not mechanically convertible — hoisting a node out of a region is a graph rewrite (new edges, a changed exit, sometimes a deleted container) and which shape the author meant is an intent no artifact records, so this ships as an ADR-0087 D3 structured TODO rather than a D2 conversion.
35+
36+
<!-- adr-0087: registered structured-region-body-pause-and-end-refused -->
37+
38+
**⚠️ Two boundaries this refusal does not reach, stated rather than discovered.** A pausing node type contributed by a **plugin** is not refused: ADR-0018 left the node-type namespace open and a parse has no registry. A region nested past **`MAX_REGION_DEPTH` (32)** is not judged: the parse walk stops there, and unlike a duplicate node id there is no second spec refusal behind it. For both, the engine's run-time refusal is the only one — unchanged by this change, and not fixed by it.
39+
40+
⛔ No engine source is edited. What the refusal does to the run time is stated rather than left to be discovered: `AutomationEngine.registerFlow` and the ADR-0087 stored-row rehydration seam both go through `FlowSchema.parse` (`canonicalizeStoredFlow`), so a flow carrying a refused shape no longer registers or rehydrates — it is met at LOAD, not at the region boundary, and a stored row that carries one stops loading until it is rewritten. The engine's own run-time refusals for these shapes stay in place but are reachable only through the two boundaries above; for the `end` arm those are the only remaining path, because the refusal signal it answers is raised at exactly one site — an `end` node whose `outcome` is `refused`.
41+
42+
**Published surface.** `FLOW_PAUSE_CAPABLE_NODE_TYPES` is published with the four types above. ⚠️ Read its contents, not its name: it is the UNCONDITIONALLY pausing set, not every type that can pause — `subflow` and `map` declare `supportsPause: true` and are deliberately absent, for the reason above. The identifier is unchanged, so this release removes no export.

packages/services/service-automation/src/end-node-refused-outcome.test.ts

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -384,48 +384,70 @@ describe('#15788 — the region boundary, made loud', () => {
384384
* throw as the try region FAILING: the author's refusal would run the catch
385385
* handler and the run would still record `completed`.
386386
*
387-
* ⛔ Nothing an author had is narrowed. Before #15788 an `end` inside a
388-
* region was a no-op whatever its `outcome`, so this shape has never once
389-
* been honoured; whether a refusal should instead propagate out of a region
390-
* is a real question the #14945 ruling does not answer.
387+
* ⚠️ #15646 moved that refusal one door EARLIER, and this case moved with
388+
* it. `FlowSchema` now refuses an `end` node inside any ADR-0031 region
389+
* body at parse, and `registerFlow` parses (`canonicalizeStoredFlow` →
390+
* `FlowSchema.parse`), so the fixture below no longer registers at all and
391+
* the run this case used to drive is unreachable. The fixture is
392+
* deliberately unchanged, so this case still fails the day the shape
393+
* becomes declarable again — a re-home, ⛔ not a deletion.
394+
*
395+
* ⚠️ What it no longer covers, stated rather than left to be discovered:
396+
* `runRegion`'s own `isRefusalSignal` arm. `FlowRefusalSignal` is raised at
397+
* exactly one site — an `end` node whose parsed `outcome` is `refused` —
398+
* and the class is not exported, so with the parse refusal in place no
399+
* authored flow reaches that arm except past `MAX_REGION_DEPTH` (32),
400+
* where the spec's region walk stops. The engine code stays as the refusal
401+
* for that one remaining seam, and nothing in this package exercises it.
402+
*
403+
* ⛔ Nothing an author had is narrowed by either door. Before #15788 an
404+
* `end` inside a region was a no-op whatever its `outcome`, so this shape
405+
* has never once been honoured; whether a refusal should instead propagate
406+
* out of a region is a real question the #14945 ruling does not answer.
391407
*/
392-
it('a refusing `end` inside a `loop` body fails the run loudly instead of vanishing', async () => {
408+
it('a refusing `end` inside a `loop` body is refused at REGISTRATION, before any run', () => {
393409
const { engine } = engineWithStore();
394410
installBuiltinNodes(engine, { logger: createTestLogger(), getService: () => undefined } as never);
395-
engine.registerFlow('in_region', {
396-
name: 'in_region', label: 'in_region', type: 'autolaunched',
397-
successMessage: SUCCESS_TEXT,
398-
nodes: [
399-
{ id: 'start', type: 'start', label: 'Start' },
400-
{
401-
id: 'sweep', type: 'loop', label: 'Sweep',
402-
config: {
403-
collection: '{items}',
404-
iteratorVariable: 'item',
405-
body: {
406-
nodes: [{ id: 'nope', type: 'end', label: 'Nope', config: { outcome: 'refused', message: REFUSAL_TEMPLATE } }],
407-
edges: [],
411+
412+
let caught: unknown;
413+
try {
414+
engine.registerFlow('in_region', {
415+
name: 'in_region', label: 'in_region', type: 'autolaunched',
416+
successMessage: SUCCESS_TEXT,
417+
nodes: [
418+
{ id: 'start', type: 'start', label: 'Start' },
419+
{
420+
id: 'sweep', type: 'loop', label: 'Sweep',
421+
config: {
422+
collection: '{items}',
423+
iteratorVariable: 'item',
424+
body: {
425+
nodes: [{ id: 'nope', type: 'end', label: 'Nope', config: { outcome: 'refused', message: REFUSAL_TEMPLATE } }],
426+
edges: [],
427+
},
408428
},
409429
},
410-
},
411-
],
412-
edges: [{ id: 'e0', source: 'start', target: 'sweep' }],
413-
} as never);
414-
415-
// `items` rides on the trigger record, which the engine flattens into
416-
// the variable map — so `{items}` resolves without declaring an input.
417-
const result = await engine.execute('in_region', {
418-
event: 'manual', object: 'account', record: { ...ACME, items: [1] },
419-
} as unknown as AutomationContext);
430+
],
431+
edges: [{ id: 'e0', source: 'start', target: 'sweep' }],
432+
} as never);
433+
} catch (err) {
434+
caught = err;
435+
}
420436

421-
expect(result.success).toBe(false);
422-
expect(result.status).toBe('failed');
437+
// Refused, and refused by the REGION rule specifically — a bare "it
438+
// threw" would be satisfied by any unrelated parse error.
439+
const issues = (caught as { issues?: Array<{ path: Array<string | number>; message: string }> } | undefined)?.issues ?? [];
440+
const refusal = issues.find((i) => i.message.includes('may not sit inside a structured region'));
441+
expect(refusal, `no region refusal among ${JSON.stringify(issues)}`).toBeDefined();
442+
// Anchored where the author wrote the node, not on the container.
443+
expect(refusal!.path).toEqual(['nodes', 1, 'config', 'body', 'nodes', 0, 'type']);
423444
// The message names the shape and the one-line fix, per "absence must
424445
// be loud" — ⛔ not a bare stringified sentinel.
425-
expect(result.error).toContain('structured region');
426-
expect(result.error).toContain("outcome: 'refused'");
427-
// ⛔ And it is NOT recorded as a refusal: the run did not refuse, the
428-
// engine declined to honour a shape it cannot express.
429-
expect(result.refusalMessage).toBeUndefined();
446+
expect(refusal!.message).toContain('An `end` node may not sit inside a structured region');
447+
expect(refusal!.message).toContain('Put the `end` on the top-level graph');
448+
449+
// …and the refusal is terminal: nothing was registered, so no later
450+
// call can reach the shape by another door.
451+
expect(engine.getFlowVersionHistory('in_region')).toEqual([]);
430452
});
431453
});

packages/spec/api-surface-declarations/automation.txt

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
# excluded: documentation drift is `check:docs`'s axis, not this one.
1313
#
1414
# entry: ./automation
15-
# exported names: 273
16-
# declarations: 279
15+
# exported names: 274
16+
# declarations: 280
1717
#
1818
# GENERATED — ⛔ never hand-edited. Regenerate after a real build:
1919
# pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec gen:api-surface-declarations
@@ -215,8 +215,8 @@ declare const ActionRefSchema: z.ZodUnion<readonly [z.ZodString, z.ZodObject<{
215215

216216
// ── ApprovalDecision (const) ──
217217
declare const ApprovalDecision: z.ZodEnum<{
218-
reject: "reject";
219218
approve: "approve";
219+
reject: "reject";
220220
}>;
221221

222222
// ── ApprovalDecision (type) ──
@@ -311,8 +311,8 @@ declare const ApprovalNodeConfigSchema: z.ZodObject<{
311311
lockRecord: z.ZodDefault<z.ZodBoolean>;
312312
approvalStatusField: z.ZodOptional<z.ZodString>;
313313
onEmptyApprovers: z.ZodDefault<z.ZodEnum<{
314-
fail: "fail";
315314
fallback: "fallback";
315+
fail: "fail";
316316
auto_approve: "auto_approve";
317317
admin_rescue: "admin_rescue";
318318
}>>;
@@ -988,6 +988,9 @@ declare const FLOW_BUILTIN_NODE_TYPES: readonly string[];
988988
// ── FLOW_NODE_EXPRESSION_PATHS (const) ──
989989
declare const FLOW_NODE_EXPRESSION_PATHS: readonly FlowNodeExpressionPath[];
990990

991+
// ── FLOW_PAUSE_CAPABLE_NODE_TYPES (const) ──
992+
declare const FLOW_PAUSE_CAPABLE_NODE_TYPES: readonly string[];
993+
991994
// ── FLOW_REGION_CONFIG_KEYS (const) ──
992995
declare const FLOW_REGION_CONFIG_KEYS: ReadonlySet<string>;
993996

packages/spec/api-surface/automation.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@
111111
"ExecutionStepSkipReasonSchema (const)",
112112
"FLOW_BUILTIN_NODE_TYPES (const)",
113113
"FLOW_NODE_EXPRESSION_PATHS (const)",
114+
"FLOW_PAUSE_CAPABLE_NODE_TYPES (const)",
114115
"FLOW_REGION_CONFIG_KEYS (const)",
115116
"FLOW_REGION_SLOTS (const)",
116117
"FLOW_REGION_SLOTS_BY_TYPE (const)",

packages/spec/export-origins/automation.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
"ExecutionStepSkipReasonSchema": "src/automation/execution.zod.ts#ExecutionStepSkipReasonSchema (const)",
108108
"FLOW_BUILTIN_NODE_TYPES": "src/automation/flow.zod.ts#FLOW_BUILTIN_NODE_TYPES (const)",
109109
"FLOW_NODE_EXPRESSION_PATHS": "src/automation/flow-node-expression-paths.ts#FLOW_NODE_EXPRESSION_PATHS (const)",
110+
"FLOW_PAUSE_CAPABLE_NODE_TYPES": "src/automation/flow.zod.ts#FLOW_PAUSE_CAPABLE_NODE_TYPES (const)",
110111
"FLOW_REGION_CONFIG_KEYS": "src/automation/region-slots.ts#FLOW_REGION_CONFIG_KEYS (const)",
111112
"FLOW_REGION_SLOTS": "src/automation/region-slots.ts#FLOW_REGION_SLOTS (const)",
112113
"FLOW_REGION_SLOTS_BY_TYPE": "src/automation/region-slots.ts#FLOW_REGION_SLOTS_BY_TYPE (const)",

packages/spec/src/automation/end-node-outcome.test.ts

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import { describe, it, expect } from 'vitest';
2222
import { EndConfigSchema } from './builtin-node-config.zod';
2323
import { FlowSchema, FlowNodeSchema, defineFlow, type Flow } from './flow.zod';
24-
import { validateControlFlow } from './control-flow.zod';
2524
import { ExecutionLogSchema, ExecutionStatus } from './execution.zod';
2625
import { formatZodError } from '../shared/error-map.zod';
2726

@@ -218,13 +217,15 @@ describe('FlowSchema applies the `end` contract — the structural node\'s only
218217
expect(issues?.map((i) => [i.code, i.path])).toEqual([['custom', ['nodes', 1, 'config', 'message']]]);
219218
});
220219

221-
it('a region-nested `end` is checked at the region door: the flow parse leaves the region raw, validateControlFlow refuses it by name', () => {
222-
// `parseFlowNodeRegions` deliberately leaves a region it cannot parse
223-
// untouched (the registration walk owns nested diagnostics, #4389), so the
224-
// FLOW parse alone does not surface a nested refusal — the same boundary
225-
// every other nested node key has. `validateControlFlow` re-parses the
226-
// region through `FlowNodeSchema`, where this contract now lives, and
227-
// throws with the same sentence.
220+
it('a region-nested `end` is refused by the FLOW parse itself (#15646/#18112) — the region-door reading this test used to pin is unreachable, because the shape is gone', () => {
221+
// ⚠️ REPLACED, not re-spelled. This case used to assert that the flow parse
222+
// was GREEN here and that `validateControlFlow` was the door — a true
223+
// reading of `parseFlowNodeRegions` leaving a refused region raw (#4389).
224+
// #15646 removes its subject: an `end` node inside a structured region body
225+
// is refused at parse, whatever its `config`, so there is no longer a
226+
// region-nested `end` whose CONFIG can be judged one door later. Keeping the
227+
// old assertion by weakening it would have pinned an `end`-in-region shape
228+
// that is now undeclarable.
228229
const nested: Flow = {
229230
name: 'nested_refusal',
230231
label: 'Nested refusal',
@@ -249,16 +250,31 @@ describe('FlowSchema applies the `end` contract — the structural node\'s only
249250
],
250251
};
251252
const parsed = FlowSchema.safeParse(nested);
252-
expect(parsed.success).toBe(true);
253-
if (!parsed.success) return;
254-
let message = '';
255-
try {
256-
validateControlFlow(parsed.data);
257-
} catch (error) {
258-
message = (error as Error).message;
259-
}
260-
expect(message).toContain("loop 'each' body");
261-
expect(message).toContain("`outcome: 'refused'` requires a `message`");
253+
expect(parsed.success).toBe(false);
254+
if (parsed.success) return;
255+
expect(parsed.error.issues.map((i) => [i.code, i.path])).toEqual([
256+
['custom', ['nodes', 1, 'config', 'body', 'nodes', 0, 'type']],
257+
]);
258+
expect(parsed.error.issues[0].message).toContain(
259+
"An `end` node may not sit inside a structured region — `loop 'each' body` is a region body and the `end` node `inner_end` is inside it",
260+
);
261+
262+
// CONTROL — the identical `end` node, identical malformed config, on the
263+
// TOP-LEVEL graph: refused by the `end` CONFIG contract instead, at
264+
// `config.message`. So the reading above is the region rule firing, not this
265+
// fixture being malformed in some way that would fail anywhere.
266+
const topLevel = FlowSchema.safeParse({
267+
...nested,
268+
nodes: [
269+
{ id: 'start', type: 'start', label: 'Start' },
270+
{ id: 'inner_end', type: 'end', label: 'Inner end', config: { outcome: 'refused' } },
271+
],
272+
edges: [{ id: 'e1', source: 'start', target: 'inner_end' }],
273+
} as Flow);
274+
expect(topLevel.success).toBe(false);
275+
if (topLevel.success) return;
276+
expect(topLevel.error.issues.map((i) => i.path)).toEqual([['nodes', 1, 'config', 'message']]);
277+
expect(topLevel.error.issues[0].message).toContain("`outcome: 'refused'` requires a `message`");
262278
});
263279
});
264280

0 commit comments

Comments
 (0)